Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580256 > unrolled thread

[PATCH 0/5] clk: sunxi-ng: Add support for A83T CCU

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] clk: sunxi-ng: Add support for A83T CCU Chen-Yu Tsai <wens@csie.org> - 2017-02-14 04:40 +0100
    [PATCH 2/5] clk: sunxi-ng: gate: Support common pre-dividers Chen-Yu Tsai <wens@csie.org> - 2017-02-14 04:40 +0100
      Re: [PATCH 2/5] clk: sunxi-ng: gate: Support common pre-dividers Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-14 10:40 +0100
    [PATCH 3/5] clk: sunxi-ng: Add compatible string for A83T CCU to bindings Chen-Yu Tsai <wens@csie.org> - 2017-02-14 04:40 +0100
    Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-14 11:00 +0100
      Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU Chen-Yu Tsai <wens@csie.org> - 2017-02-14 11:30 +0100
        Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-15 11:00 +0100

#1580256 — [PATCH 0/5] clk: sunxi-ng: Add support for A83T CCU

Hi everyone,

This is yet another series that adds support for the A83T CCU.
The A83T CCU has a mix of new styled (like the A80) clocks at
old (like A3x) offsets. Some differences include:

  - D1/D2 style PLL clocks
  - divisible audio module clocks
  - new timing mode for mmc2 module clock

Patch 1 fixes the mp style clock to take into account pre-dividers
for the .set_rate and .recalc_rate callbacks.

Patch 2 makes the gate clocks support common pre-dividers. This is
used to make the HSIC 12M clock have the right clock rate.

Patch 3 adds a compatible string for the A83T CCU to the sunxi-ccu
bindings.

Patch 4 adds the driver for the A83T CCU.

Patch 5 adds the CCU device nodes, and fixes up any existing clock
phandles in the dtsi.

Let me know what you think.


Regards
ChenYu


Chen-Yu Tsai (5):
  clk: sunxi-ng: mp: Adjust parent rate for pre-dividers
  clk: sunxi-ng: gate: Support common pre-dividers
  clk: sunxi-ng: Add compatible string for A83T CCU to bindings
  clk: sunxi-ng: Add driver for A83T CCU
  ARM: dts: sun8i-a83t: Add CCU device nodes

 .../devicetree/bindings/clock/sunxi-ccu.txt        |   1 +
 arch/arm/boot/dts/sun8i-a83t.dtsi                  |  18 +-
 drivers/clk/sunxi-ng/Kconfig                       |  10 +
 drivers/clk/sunxi-ng/Makefile                      |   1 +
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c              | 898 +++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.h              |  65 ++
 drivers/clk/sunxi-ng/ccu_gate.c                    |  47 ++
 drivers/clk/sunxi-ng/ccu_mp.c                      |   8 +
 include/dt-bindings/clock/sun8i-a83t-ccu.h         | 138 ++++
 include/dt-bindings/reset/sun8i-a83t-ccu.h         |  98 +++
 10 files changed, 1282 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
 create mode 100644 drivers/clk/sunxi-ng/ccu-sun8i-a83t.h
 create mode 100644 include/dt-bindings/clock/sun8i-a83t-ccu.h
 create mode 100644 include/dt-bindings/reset/sun8i-a83t-ccu.h

-- 
2.11.0

[toc] | [next] | [standalone]

#1580260 — [PATCH 2/5] clk: sunxi-ng: gate: Support common pre-dividers

Some clock gates have a pre-divider between the source input and the
gate itself. A notable example is the HSIC 12 MHz clock found on the
A83T, which has the 24 MHz main oscillator as its input, and a /2
pre-divider.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/ccu_gate.c | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu_gate.c b/drivers/clk/sunxi-ng/ccu_gate.c
index 8a81f9d4a89f..cd069d5da215 100644
--- a/drivers/clk/sunxi-ng/ccu_gate.c
+++ b/drivers/clk/sunxi-ng/ccu_gate.c
@@ -75,8 +75,55 @@ static int ccu_gate_is_enabled(struct clk_hw *hw)
 	return ccu_gate_helper_is_enabled(&cg->common, cg->enable);
 }
 
+static unsigned long ccu_gate_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	struct ccu_gate *cg = hw_to_ccu_gate(hw);
+	unsigned long rate = parent_rate;
+
+	if (cg->common.features & CCU_FEATURE_ALL_PREDIV)
+		rate /= cg->common.prediv;
+
+	return rate;
+}
+
+static long ccu_gate_round_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long *prate)
+{
+	struct ccu_gate *cg = hw_to_ccu_gate(hw);
+	int div = 1;
+
+	if (cg->common.features & CCU_FEATURE_ALL_PREDIV)
+		div = cg->common.prediv;
+
+	if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
+		unsigned long best_parent = rate;
+
+		if (cg->common.features & CCU_FEATURE_ALL_PREDIV)
+			best_parent *= div;
+		*prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
+	}
+
+	return *prate / div;
+}
+
+static int ccu_gate_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	/*
+	 * We must report success but we can do so unconditionally because
+	 * clk_factor_round_rate returns values that ensure this call is a
+	 * nop.
+	 */
+
+	return 0;
+}
+
 const struct clk_ops ccu_gate_ops = {
 	.disable	= ccu_gate_disable,
 	.enable		= ccu_gate_enable,
 	.is_enabled	= ccu_gate_is_enabled,
+	.round_rate	= ccu_gate_round_rate,
+	.set_rate	= ccu_gate_set_rate,
+	.recalc_rate	= ccu_gate_recalc_rate,
 };
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1580431 — Re: [PATCH 2/5] clk: sunxi-ng: gate: Support common pre-dividers

[Multipart message — attachments visible in raw view] — view raw

On Tue, Feb 14, 2017 at 11:35:23AM +0800, Chen-Yu Tsai wrote:
> Some clock gates have a pre-divider between the source input and the
> gate itself. A notable example is the HSIC 12 MHz clock found on the
> A83T, which has the 24 MHz main oscillator as its input, and a /2
> pre-divider.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1580262 — [PATCH 3/5] clk: sunxi-ng: Add compatible string for A83T CCU to bindings

The A83T clock control unit is a hybrid of some new style clock designs
from the A80, and old style layout from the other Allwinner SoCs.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
index bae5668cf427..a6144f913f17 100644
--- a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt
@@ -6,6 +6,7 @@ Required properties :
 		- "allwinner,sun6i-a31-ccu"
 		- "allwinner,sun8i-a23-ccu"
 		- "allwinner,sun8i-a33-ccu"
+		- "allwinner,sun8i-a83t-ccu"
 		- "allwinner,sun8i-h3-ccu"
 		- "allwinner,sun8i-v3s-ccu"
 		- "allwinner,sun9i-a80-ccu"
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1580441 — Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU

[Multipart message — attachments visible in raw view] — view raw

On Tue, Feb 14, 2017 at 11:35:25AM +0800, Chen-Yu Tsai wrote:
> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */

Is that even working?

I'm not quite sure we want to do that. We might model it as a NP clock
with a variable prediv?

> +/* Use a separate clock for the pre-divider on the AHB1 PLL-PERIPH input */
> +static SUNXI_CCU_M(pll_periph_ahb1_clk, "pll-periph-ahb1", "pll-periph",
> +		   0x054, 6, 2, 0);
> +
> +static const char * const ahb1_parents[] = { "osc16M-d512", "osc24M",
> +					     "pll-periph-ahb1",
> +					     "pll-periph-ahb1" };
> +static struct ccu_div ahb1_clk = {
> +	.div		= _SUNXI_CCU_DIV_FLAGS(4, 2, CLK_DIVIDER_POWER_OF_TWO),
> +	.mux		= _SUNXI_CCU_MUX(12, 2),
> +	.common		= {
> +		.reg		= 0x054,
> +		.hw.init	= CLK_HW_INIT_PARENTS("ahb1",
> +						      ahb1_parents,
> +						      &ccu_div_ops,
> +						      0),
> +	},
> +};

What's different from a pre divider only for a given index here?

> +/*
> + * MMC2 supports what's called the "new timing mode". The CCU and the MMC
> + * controller must be in sync about which mode is used. The new mode moves
> + * the clock delay controls (and possibly the delay lines) into the MMC
> + * block. Also, the output of the clock is divided by 2. The output and
> + * sample phase clocks are unused under this mode.
> + *
> + * This new mode seems to be preferred. Hence we force this clock to the
> + * new mode. And we don't add the phase clocks.
> + */

I'm sorry, but I said this several times, this isn't working. We
should model it properly, and not hack this around in the clock
driver.

As you say in your comment, the MMC driver needs to be aware about
which mode is used, in order to also set a bit in one of its registers
accordingly, and modify its sampling behaviour.

The new timing is preferred, but our previous clock implementations
didn't hardcode it, so we can't even rely on that behaviour to always
write it in our driver.

This is not something specific to the A83T, but is found in all the
SoCs since the A23, so we need to come up with a good solution to
address that.

I'm not sure what a good solution would be though. One would be to
just have a private function of our own to switch in the new mode (if
relevant, because only the MMC2 controllers have it), but that would
lead to troubles with !sunxi-ng. Not something we can't deal with, but
some extra precautions should be taken (make sure to protect the call
through an ifdef / IS_DEFINED, check that the sunxi-ng driver has been
probed, etc.)

Or we could introduce a new clk_ops function pointer, but I'm not sure
if Mike and Stephen are going to be happy with that.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1580458 — Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU

On Tue, Feb 14, 2017 at 5:58 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Feb 14, 2017 at 11:35:25AM +0800, Chen-Yu Tsai wrote:
>> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */
>
> Is that even working?

Looking at the nkmp clock code, only .recalc_rate will work properly though.
Maybe I could fix up the code so it handles zero width factors.

> I'm not quite sure we want to do that. We might model it as a NP clock
> with a variable prediv?

There's no NP clock type yet. And a problem with a variable prediv is that
it doesn't participate in factor calculation. It's effectively fixed.

I did this for the A80 as well though. Fixing up the NKMP clock might be
easier.

>
>> +/* Use a separate clock for the pre-divider on the AHB1 PLL-PERIPH input */
>> +static SUNXI_CCU_M(pll_periph_ahb1_clk, "pll-periph-ahb1", "pll-periph",
>> +                0x054, 6, 2, 0);
>> +
>> +static const char * const ahb1_parents[] = { "osc16M-d512", "osc24M",
>> +                                          "pll-periph-ahb1",
>> +                                          "pll-periph-ahb1" };
>> +static struct ccu_div ahb1_clk = {
>> +     .div            = _SUNXI_CCU_DIV_FLAGS(4, 2, CLK_DIVIDER_POWER_OF_TWO),
>> +     .mux            = _SUNXI_CCU_MUX(12, 2),
>> +     .common         = {
>> +             .reg            = 0x054,
>> +             .hw.init        = CLK_HW_INIT_PARENTS("ahb1",
>> +                                                   ahb1_parents,
>> +                                                   &ccu_div_ops,
>> +                                                   0),
>> +     },
>> +};
>
> What's different from a pre divider only for a given index here?

The variable pre-divider is shared for both pll-periph mux inputs.
This is one way to handle it. The other would be to extend ccu_mux
to handle multiple variable pre-dividers. I don't really want to do
that if this is the only instance that needs it though.

>
>> +/*
>> + * MMC2 supports what's called the "new timing mode". The CCU and the MMC
>> + * controller must be in sync about which mode is used. The new mode moves
>> + * the clock delay controls (and possibly the delay lines) into the MMC
>> + * block. Also, the output of the clock is divided by 2. The output and
>> + * sample phase clocks are unused under this mode.
>> + *
>> + * This new mode seems to be preferred. Hence we force this clock to the
>> + * new mode. And we don't add the phase clocks.
>> + */
>
> I'm sorry, but I said this several times, this isn't working. We
> should model it properly, and not hack this around in the clock
> driver.
>
> As you say in your comment, the MMC driver needs to be aware about
> which mode is used, in order to also set a bit in one of its registers
> accordingly, and modify its sampling behaviour.
>
> The new timing is preferred, but our previous clock implementations
> didn't hardcode it, so we can't even rely on that behaviour to always
> write it in our driver.

Correct. With the A83T there has never been a merged clock driver though.
I realize this is a one off thing.

> This is not something specific to the A83T, but is found in all the
> SoCs since the A23, so we need to come up with a good solution to
> address that.
>
> I'm not sure what a good solution would be though. One would be to
> just have a private function of our own to switch in the new mode (if
> relevant, because only the MMC2 controllers have it), but that would
> lead to troubles with !sunxi-ng. Not something we can't deal with, but
> some extra precautions should be taken (make sure to protect the call
> through an ifdef / IS_DEFINED, check that the sunxi-ng driver has been
> probed, etc.)

If the custom function route is acceptable, I'll come up with something.

Regards
ChenYu

>
> Or we could introduce a new clk_ops function pointer, but I'm not sure
> if Mike and Stephen are going to be happy with that.
>
> Thanks,
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1581173 — Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU

[Multipart message — attachments visible in raw view] — view raw

On Tue, Feb 14, 2017 at 06:26:39PM +0800, Chen-Yu Tsai wrote:
> On Tue, Feb 14, 2017 at 5:58 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Tue, Feb 14, 2017 at 11:35:25AM +0800, Chen-Yu Tsai wrote:
> >> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */
> >
> > Is that even working?
> 
> Looking at the nkmp clock code, only .recalc_rate will work properly though.
> Maybe I could fix up the code so it handles zero width factors.
> 
> > I'm not quite sure we want to do that. We might model it as a NP clock
> > with a variable prediv?
> 
> There's no NP clock type yet. And a problem with a variable prediv is that
> it doesn't participate in factor calculation. It's effectively fixed.
> 
> I did this for the A80 as well though. Fixing up the NKMP clock might be
> easier.

Then maybe we just need a NMP clock type then. What I'm really afraid
of is that we'll just end up in a clk-factors situation that was
simply impossible to maintain without breaking anything, hence why we
had different clock types then.

> >
> >> +/* Use a separate clock for the pre-divider on the AHB1 PLL-PERIPH input */
> >> +static SUNXI_CCU_M(pll_periph_ahb1_clk, "pll-periph-ahb1", "pll-periph",
> >> +                0x054, 6, 2, 0);
> >> +
> >> +static const char * const ahb1_parents[] = { "osc16M-d512", "osc24M",
> >> +                                          "pll-periph-ahb1",
> >> +                                          "pll-periph-ahb1" };
> >> +static struct ccu_div ahb1_clk = {
> >> +     .div            = _SUNXI_CCU_DIV_FLAGS(4, 2, CLK_DIVIDER_POWER_OF_TWO),
> >> +     .mux            = _SUNXI_CCU_MUX(12, 2),
> >> +     .common         = {
> >> +             .reg            = 0x054,
> >> +             .hw.init        = CLK_HW_INIT_PARENTS("ahb1",
> >> +                                                   ahb1_parents,
> >> +                                                   &ccu_div_ops,
> >> +                                                   0),
> >> +     },
> >> +};
> >
> > What's different from a pre divider only for a given index here?
> 
> The variable pre-divider is shared for both pll-periph mux inputs.
> This is one way to handle it. The other would be to extend ccu_mux
> to handle multiple variable pre-dividers. I don't really want to do
> that if this is the only instance that needs it though.

Every addition we made was only needed by one instance at first :)

We are working that way for fixed pre-dividers already, I don't see
why we can't have it for variable ones too.

> >> +/*
> >> + * MMC2 supports what's called the "new timing mode". The CCU and the MMC
> >> + * controller must be in sync about which mode is used. The new mode moves
> >> + * the clock delay controls (and possibly the delay lines) into the MMC
> >> + * block. Also, the output of the clock is divided by 2. The output and
> >> + * sample phase clocks are unused under this mode.
> >> + *
> >> + * This new mode seems to be preferred. Hence we force this clock to the
> >> + * new mode. And we don't add the phase clocks.
> >> + */
> >
> > I'm sorry, but I said this several times, this isn't working. We
> > should model it properly, and not hack this around in the clock
> > driver.
> >
> > As you say in your comment, the MMC driver needs to be aware about
> > which mode is used, in order to also set a bit in one of its registers
> > accordingly, and modify its sampling behaviour.
> >
> > The new timing is preferred, but our previous clock implementations
> > didn't hardcode it, so we can't even rely on that behaviour to always
> > write it in our driver.
> 
> Correct. With the A83T there has never been a merged clock driver though.
> I realize this is a one off thing.
> 
> > This is not something specific to the A83T, but is found in all the
> > SoCs since the A23, so we need to come up with a good solution to
> > address that.
> >
> > I'm not sure what a good solution would be though. One would be to
> > just have a private function of our own to switch in the new mode (if
> > relevant, because only the MMC2 controllers have it), but that would
> > lead to troubles with !sunxi-ng. Not something we can't deal with, but
> > some extra precautions should be taken (make sure to protect the call
> > through an ifdef / IS_DEFINED, check that the sunxi-ng driver has been
> > probed, etc.)
> 
> If the custom function route is acceptable, I'll come up with something.

I think it would be a great start yes. I'll try to discuss it with
Mike and Stephen at ELC and see what they think about that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web