[PATCH 0/8] clk: sunxi: Add support for the Audio PLL

[PATCH 0/8] clk: sunxi: Add support for the Audio PLL

[Maxime Ripard]

Hi,

This serie adds support for the PLL2 aka the Audio PLL on the
Allwinner A10 and the later SoCs.

This is the first stepping stone to get the audio support merged.

Thanks!
Maxime

Emilio L?pez (5):
  clk: sunxi: codec clock support
  clk: sunxi: mod1 clock support
  ARM: sunxi: Add PLL2 support
  ARM: sunxi: Add codec clock support
  ARM: sun7i: Add mod1 clock nodes

Maxime Ripard (3):
  clk: sunxi: factors: Add m_start parameters
  clk: sunxi: factors: Add a parameter for N and M zeros
  clk: sunxi: Add a driver for the PLL2

 arch/arm/boot/dts/sun4i-a10.dtsi           |  18 +++
 arch/arm/boot/dts/sun5i.dtsi               |  18 +++
 arch/arm/boot/dts/sun7i-a20.dtsi           |  73 ++++++++++++
 drivers/clk/sunxi/Makefile                 |   3 +
 drivers/clk/sunxi/clk-a10-codec.c          |  45 ++++++++
 drivers/clk/sunxi/clk-a10-mod1.c           |  85 ++++++++++++++
 drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
 drivers/clk/sunxi/clk-factors.c            |  17 ++-
 drivers/clk/sunxi/clk-factors.h            |   5 +
 drivers/clk/sunxi/clk-mod0.c               |   2 +
 drivers/clk/sunxi/clk-sun8i-mbus.c         |   2 +
 drivers/clk/sunxi/clk-sun9i-core.c         |   6 +
 drivers/clk/sunxi/clk-sunxi.c              |  10 ++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
 14 files changed, 512 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c
 create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
 create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h

-- 
2.3.6

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Maxime Ripard]

Some clocks start incrementing the m factor at 0. Add a parameter to handle
it just like we did for the N factor.

Since the behaviour until now was to assume that the m factor was starting
at 1, we also need to fix the other users.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
 drivers/clk/sunxi/clk-factors.h    |  2 ++
 drivers/clk/sunxi/clk-mod0.c       |  2 ++
 drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
 drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
 drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
 6 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 8c20190a3e9f..100a711c3e3d 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	/* Get each individual factor if applicable */
 	if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		n = FACTOR_GET(config->nshift, config->nwidth, reg);
+
 	if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		k = FACTOR_GET(config->kshift, config->kwidth, reg);
+
 	if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		m = FACTOR_GET(config->mshift, config->mwidth, reg);
+	else
+		/* Make sure we don't get a division by zero */
+		m = 1;
+
 	if (config->pwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		p = FACTOR_GET(config->pshift, config->pwidth, reg);
 
 	/* Calculate the rate */
-	rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
+	rate = parent_rate * (n + config->n_start);
+	rate *= k + 1;
+	rate >>= p;
+	rate /= m + config->m_start;
 
 	return rate;
 }
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 171085ab5513..735d756d2923 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -16,6 +16,8 @@ struct clk_factors_config {
 	u8 mwidth;
 	u8 pshift;
 	u8 pwidth;
+
+	u8 m_start;
 	u8 n_start;
 };
 
diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index ec8f5a1fca09..eefa9be4078b 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -66,6 +66,8 @@ static struct clk_factors_config sun4i_a10_mod0_config = {
 	.mwidth = 4,
 	.pshift = 16,
 	.pwidth = 2,
+
+	.m_start = 1,
 };
 
 static const struct factors_data sun4i_a10_mod0_data = {
diff --git a/drivers/clk/sunxi/clk-sun8i-mbus.c b/drivers/clk/sunxi/clk-sun8i-mbus.c
index 14cd026064bf..66fbf14a0f8a 100644
--- a/drivers/clk/sunxi/clk-sun8i-mbus.c
+++ b/drivers/clk/sunxi/clk-sun8i-mbus.c
@@ -55,6 +55,8 @@ static void sun8i_a23_get_mbus_factors(u32 *freq, u32 parent_rate,
 static struct clk_factors_config sun8i_a23_mbus_config = {
 	.mshift = 0,
 	.mwidth = 3,
+
+	.m_start = 1,
 };
 
 static const struct factors_data sun8i_a23_mbus_data __initconst = {
diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
index d8da77d72861..97e87ca4a8fc 100644
--- a/drivers/clk/sunxi/clk-sun9i-core.c
+++ b/drivers/clk/sunxi/clk-sun9i-core.c
@@ -78,6 +78,8 @@ static struct clk_factors_config sun9i_a80_pll4_config = {
 	.nwidth = 8,
 	.pshift = 16,
 	.pwidth = 1,
+
+	.m_start = 1,
 };
 
 static const struct factors_data sun9i_a80_pll4_data __initconst = {
@@ -137,6 +139,8 @@ static void sun9i_a80_get_gt_factors(u32 *freq, u32 parent_rate,
 static struct clk_factors_config sun9i_a80_gt_config = {
 	.mshift = 0,
 	.mwidth = 2,
+
+	.m_start = 1,
 };
 
 static const struct factors_data sun9i_a80_gt_data __initconst = {
@@ -294,6 +298,8 @@ static struct clk_factors_config sun9i_a80_apb1_config = {
 	.mwidth = 5,
 	.pshift = 16,
 	.pwidth = 2,
+
+	.m_start = 1,
 };
 
 static const struct factors_data sun9i_a80_apb1_data __initconst = {
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e1e2bd189b6..6df869050986 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -616,6 +616,8 @@ static struct clk_factors_config sun4i_pll1_config = {
 	.mwidth = 2,
 	.pshift = 16,
 	.pwidth = 2,
+
+	.m_start = 1,
 };
 
 static struct clk_factors_config sun6i_a31_pll1_config = {
@@ -625,6 +627,8 @@ static struct clk_factors_config sun6i_a31_pll1_config = {
 	.kwidth = 2,
 	.mshift = 0,
 	.mwidth = 2,
+
+	.m_start = 1,
 	.n_start = 1,
 };
 
@@ -637,6 +641,8 @@ static struct clk_factors_config sun8i_a23_pll1_config = {
 	.mwidth = 2,
 	.pshift = 16,
 	.pwidth = 2,
+
+	.m_start = 1,
 	.n_start = 1,
 };
 
@@ -665,6 +671,8 @@ static struct clk_factors_config sun4i_apb1_config = {
 	.mwidth = 5,
 	.pshift = 16,
 	.pwidth = 2,
+
+	.m_start = 1,
 };
 
 /* user manual says "n" but it's really "p" */
@@ -673,6 +681,8 @@ static struct clk_factors_config sun7i_a20_out_config = {
 	.mwidth = 5,
 	.pshift = 20,
 	.pwidth = 2,
+
+	.m_start = 1,
 };
 
 static const struct factors_data sun4i_pll1_data __initconst = {
-- 
2.3.6

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Chen-Yu Tsai]

On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Some clocks start incrementing the m factor at 0. Add a parameter to handle
> it just like we did for the N factor.
>
> Since the behaviour until now was to assume that the m factor was starting
> at 1, we also need to fix the other users.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
>  drivers/clk/sunxi/clk-factors.h    |  2 ++
>  drivers/clk/sunxi/clk-mod0.c       |  2 ++
>  drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
>  drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
>  drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
>  6 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> index 8c20190a3e9f..100a711c3e3d 100644
> --- a/drivers/clk/sunxi/clk-factors.c
> +++ b/drivers/clk/sunxi/clk-factors.c
> @@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>         /* Get each individual factor if applicable */
>         if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>                 n = FACTOR_GET(config->nshift, config->nwidth, reg);
> +
>         if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>                 k = FACTOR_GET(config->kshift, config->kwidth, reg);
> +
>         if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>                 m = FACTOR_GET(config->mshift, config->mwidth, reg);
> +       else
> +               /* Make sure we don't get a division by zero */
> +               m = 1;

What happens when mwidth is valid, m_start = 0, and m = 0?

Other than that, this one looks good.

ChenYu

> +
>         if (config->pwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>                 p = FACTOR_GET(config->pshift, config->pwidth, reg);
>
>         /* Calculate the rate */
> -       rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
> +       rate = parent_rate * (n + config->n_start);
> +       rate *= k + 1;
> +       rate >>= p;
> +       rate /= m + config->m_start;
>
>         return rate;
>  }
> diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
> index 171085ab5513..735d756d2923 100644
> --- a/drivers/clk/sunxi/clk-factors.h
> +++ b/drivers/clk/sunxi/clk-factors.h
> @@ -16,6 +16,8 @@ struct clk_factors_config {
>         u8 mwidth;
>         u8 pshift;
>         u8 pwidth;
> +
> +       u8 m_start;
>         u8 n_start;
>  };
>
> diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
> index ec8f5a1fca09..eefa9be4078b 100644
> --- a/drivers/clk/sunxi/clk-mod0.c
> +++ b/drivers/clk/sunxi/clk-mod0.c
> @@ -66,6 +66,8 @@ static struct clk_factors_config sun4i_a10_mod0_config = {
>         .mwidth = 4,
>         .pshift = 16,
>         .pwidth = 2,
> +
> +       .m_start = 1,
>  };
>
>  static const struct factors_data sun4i_a10_mod0_data = {
> diff --git a/drivers/clk/sunxi/clk-sun8i-mbus.c b/drivers/clk/sunxi/clk-sun8i-mbus.c
> index 14cd026064bf..66fbf14a0f8a 100644
> --- a/drivers/clk/sunxi/clk-sun8i-mbus.c
> +++ b/drivers/clk/sunxi/clk-sun8i-mbus.c
> @@ -55,6 +55,8 @@ static void sun8i_a23_get_mbus_factors(u32 *freq, u32 parent_rate,
>  static struct clk_factors_config sun8i_a23_mbus_config = {
>         .mshift = 0,
>         .mwidth = 3,
> +
> +       .m_start = 1,
>  };
>
>  static const struct factors_data sun8i_a23_mbus_data __initconst = {
> diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
> index d8da77d72861..97e87ca4a8fc 100644
> --- a/drivers/clk/sunxi/clk-sun9i-core.c
> +++ b/drivers/clk/sunxi/clk-sun9i-core.c
> @@ -78,6 +78,8 @@ static struct clk_factors_config sun9i_a80_pll4_config = {
>         .nwidth = 8,
>         .pshift = 16,
>         .pwidth = 1,
> +
> +       .m_start = 1,
>  };
>
>  static const struct factors_data sun9i_a80_pll4_data __initconst = {
> @@ -137,6 +139,8 @@ static void sun9i_a80_get_gt_factors(u32 *freq, u32 parent_rate,
>  static struct clk_factors_config sun9i_a80_gt_config = {
>         .mshift = 0,
>         .mwidth = 2,
> +
> +       .m_start = 1,
>  };
>
>  static const struct factors_data sun9i_a80_gt_data __initconst = {
> @@ -294,6 +298,8 @@ static struct clk_factors_config sun9i_a80_apb1_config = {
>         .mwidth = 5,
>         .pshift = 16,
>         .pwidth = 2,
> +
> +       .m_start = 1,
>  };
>
>  static const struct factors_data sun9i_a80_apb1_data __initconst = {
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 7e1e2bd189b6..6df869050986 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -616,6 +616,8 @@ static struct clk_factors_config sun4i_pll1_config = {
>         .mwidth = 2,
>         .pshift = 16,
>         .pwidth = 2,
> +
> +       .m_start = 1,
>  };
>
>  static struct clk_factors_config sun6i_a31_pll1_config = {
> @@ -625,6 +627,8 @@ static struct clk_factors_config sun6i_a31_pll1_config = {
>         .kwidth = 2,
>         .mshift = 0,
>         .mwidth = 2,
> +
> +       .m_start = 1,
>         .n_start = 1,
>  };
>
> @@ -637,6 +641,8 @@ static struct clk_factors_config sun8i_a23_pll1_config = {
>         .mwidth = 2,
>         .pshift = 16,
>         .pwidth = 2,
> +
> +       .m_start = 1,
>         .n_start = 1,
>  };
>
> @@ -665,6 +671,8 @@ static struct clk_factors_config sun4i_apb1_config = {
>         .mwidth = 5,
>         .pshift = 16,
>         .pwidth = 2,
> +
> +       .m_start = 1,
>  };
>
>  /* user manual says "n" but it's really "p" */
> @@ -673,6 +681,8 @@ static struct clk_factors_config sun7i_a20_out_config = {
>         .mwidth = 5,
>         .pshift = 20,
>         .pwidth = 2,
> +
> +       .m_start = 1,
>  };
>
>  static const struct factors_data sun4i_pll1_data __initconst = {
> --
> 2.3.6
>

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Maxime Ripard]

On Thu, May 14, 2015 at 05:12:07PM +0800, Chen-Yu Tsai wrote:
> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Some clocks start incrementing the m factor at 0. Add a parameter to handle
> > it just like we did for the N factor.
> >
> > Since the behaviour until now was to assume that the m factor was starting
> > at 1, we also need to fix the other users.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
> >  drivers/clk/sunxi/clk-factors.h    |  2 ++
> >  drivers/clk/sunxi/clk-mod0.c       |  2 ++
> >  drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
> >  drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
> >  drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
> >  6 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> > index 8c20190a3e9f..100a711c3e3d 100644
> > --- a/drivers/clk/sunxi/clk-factors.c
> > +++ b/drivers/clk/sunxi/clk-factors.c
> > @@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
> >         /* Get each individual factor if applicable */
> >         if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >                 n = FACTOR_GET(config->nshift, config->nwidth, reg);
> > +
> >         if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >                 k = FACTOR_GET(config->kshift, config->kwidth, reg);
> > +
> >         if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >                 m = FACTOR_GET(config->mshift, config->mwidth, reg);
> > +       else
> > +               /* Make sure we don't get a division by zero */
> > +               m = 1;
> 
> What happens when mwidth is valid, m_start = 0, and m = 0?

That's a very good question. A division by zero in the kernel, I'd
say.

But I don't think we can end up in such a case today, and it's
somewhat expected that it will happen, and no clock have looked at can
actually end up in such a case.

> Other than that, this one looks good.

Thanks!

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/d7510e57/attachment.sig>

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Chen-Yu Tsai]

On Fri, May 15, 2015 at 3:43 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Thu, May 14, 2015 at 05:12:07PM +0800, Chen-Yu Tsai wrote:
>> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Some clocks start incrementing the m factor at 0. Add a parameter to handle
>> > it just like we did for the N factor.
>> >
>> > Since the behaviour until now was to assume that the m factor was starting
>> > at 1, we also need to fix the other users.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
>> >  drivers/clk/sunxi/clk-factors.h    |  2 ++
>> >  drivers/clk/sunxi/clk-mod0.c       |  2 ++
>> >  drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
>> >  drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
>> >  drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
>> >  6 files changed, 32 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
>> > index 8c20190a3e9f..100a711c3e3d 100644
>> > --- a/drivers/clk/sunxi/clk-factors.c
>> > +++ b/drivers/clk/sunxi/clk-factors.c
>> > @@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>> >         /* Get each individual factor if applicable */
>> >         if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>> >                 n = FACTOR_GET(config->nshift, config->nwidth, reg);
>> > +
>> >         if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>> >                 k = FACTOR_GET(config->kshift, config->kwidth, reg);
>> > +
>> >         if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>> >                 m = FACTOR_GET(config->mshift, config->mwidth, reg);
>> > +       else
>> > +               /* Make sure we don't get a division by zero */
>> > +               m = 1;
>>
>> What happens when mwidth is valid, m_start = 0, and m = 0?
>
> That's a very good question. A division by zero in the kernel, I'd
> say.
>
> But I don't think we can end up in such a case today, and it's
> somewhat expected that it will happen, and no clock have looked at can
> actually end up in such a case.

It's possible if the bootloader left the clock in an invalid state.
How about just returning 0, indicating an invalid rate, early?

>> Other than that, this one looks good.
>
> Thanks!
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Maxime Ripard]

On Fri, May 15, 2015 at 03:48:38PM +0800, Chen-Yu Tsai wrote:
> On Fri, May 15, 2015 at 3:43 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Thu, May 14, 2015 at 05:12:07PM +0800, Chen-Yu Tsai wrote:
> >> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > Some clocks start incrementing the m factor at 0. Add a parameter to handle
> >> > it just like we did for the N factor.
> >> >
> >> > Since the behaviour until now was to assume that the m factor was starting
> >> > at 1, we also need to fix the other users.
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> >  drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
> >> >  drivers/clk/sunxi/clk-factors.h    |  2 ++
> >> >  drivers/clk/sunxi/clk-mod0.c       |  2 ++
> >> >  drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
> >> >  drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
> >> >  drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
> >> >  6 files changed, 32 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> >> > index 8c20190a3e9f..100a711c3e3d 100644
> >> > --- a/drivers/clk/sunxi/clk-factors.c
> >> > +++ b/drivers/clk/sunxi/clk-factors.c
> >> > @@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
> >> >         /* Get each individual factor if applicable */
> >> >         if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >> >                 n = FACTOR_GET(config->nshift, config->nwidth, reg);
> >> > +
> >> >         if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >> >                 k = FACTOR_GET(config->kshift, config->kwidth, reg);
> >> > +
> >> >         if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >> >                 m = FACTOR_GET(config->mshift, config->mwidth, reg);
> >> > +       else
> >> > +               /* Make sure we don't get a division by zero */
> >> > +               m = 1;
> >>
> >> What happens when mwidth is valid, m_start = 0, and m = 0?
> >
> > That's a very good question. A division by zero in the kernel, I'd
> > say.
> >
> > But I don't think we can end up in such a case today, and it's
> > somewhat expected that it will happen, and no clock have looked at can
> > actually end up in such a case.
> 
> It's possible if the bootloader left the clock in an invalid state.
> How about just returning 0, indicating an invalid rate, early?

The value set in the register might be zero, but that will always
really mean 1, either through m_start or m_zero. That would be a bug
in the driver itself.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/66fe4a44/attachment.sig>

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Chen-Yu Tsai]

On Fri, May 15, 2015 at 4:05 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, May 15, 2015 at 03:48:38PM +0800, Chen-Yu Tsai wrote:
>> On Fri, May 15, 2015 at 3:43 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > On Thu, May 14, 2015 at 05:12:07PM +0800, Chen-Yu Tsai wrote:
>> >> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
>> >> <maxime.ripard@free-electrons.com> wrote:
>> >> > Some clocks start incrementing the m factor at 0. Add a parameter to handle
>> >> > it just like we did for the N factor.
>> >> >
>> >> > Since the behaviour until now was to assume that the m factor was starting
>> >> > at 1, we also need to fix the other users.
>> >> >
>> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> >> > ---
>> >> >  drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
>> >> >  drivers/clk/sunxi/clk-factors.h    |  2 ++
>> >> >  drivers/clk/sunxi/clk-mod0.c       |  2 ++
>> >> >  drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
>> >> >  drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
>> >> >  drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
>> >> >  6 files changed, 32 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
>> >> > index 8c20190a3e9f..100a711c3e3d 100644
>> >> > --- a/drivers/clk/sunxi/clk-factors.c
>> >> > +++ b/drivers/clk/sunxi/clk-factors.c
>> >> > @@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>> >> >         /* Get each individual factor if applicable */
>> >> >         if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>> >> >                 n = FACTOR_GET(config->nshift, config->nwidth, reg);
>> >> > +
>> >> >         if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>> >> >                 k = FACTOR_GET(config->kshift, config->kwidth, reg);
>> >> > +
>> >> >         if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
>> >> >                 m = FACTOR_GET(config->mshift, config->mwidth, reg);
>> >> > +       else
>> >> > +               /* Make sure we don't get a division by zero */
>> >> > +               m = 1;
>> >>
>> >> What happens when mwidth is valid, m_start = 0, and m = 0?
>> >
>> > That's a very good question. A division by zero in the kernel, I'd
>> > say.
>> >
>> > But I don't think we can end up in such a case today, and it's
>> > somewhat expected that it will happen, and no clock have looked at can
>> > actually end up in such a case.
>>
>> It's possible if the bootloader left the clock in an invalid state.
>> How about just returning 0, indicating an invalid rate, early?
>
> The value set in the register might be zero, but that will always
> really mean 1, either through m_start or m_zero. That would be a bug
> in the driver itself.

Maybe you should switch the order of these 2 patches to make it clear?
Or maybe squash them? As it currently is, m_start = 0 and m = 0 with
this patch only is a bug.

ChenYu

[PATCH 1/8] clk: sunxi: factors: Add m_start parameters

[Maxime Ripard]

On Fri, May 15, 2015 at 05:11:16PM +0800, Chen-Yu Tsai wrote:
> On Fri, May 15, 2015 at 4:05 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Fri, May 15, 2015 at 03:48:38PM +0800, Chen-Yu Tsai wrote:
> >> On Fri, May 15, 2015 at 3:43 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > On Thu, May 14, 2015 at 05:12:07PM +0800, Chen-Yu Tsai wrote:
> >> >> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> >> >> <maxime.ripard@free-electrons.com> wrote:
> >> >> > Some clocks start incrementing the m factor at 0. Add a parameter to handle
> >> >> > it just like we did for the N factor.
> >> >> >
> >> >> > Since the behaviour until now was to assume that the m factor was starting
> >> >> > at 1, we also need to fix the other users.
> >> >> >
> >> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> >> > ---
> >> >> >  drivers/clk/sunxi/clk-factors.c    | 11 ++++++++++-
> >> >> >  drivers/clk/sunxi/clk-factors.h    |  2 ++
> >> >> >  drivers/clk/sunxi/clk-mod0.c       |  2 ++
> >> >> >  drivers/clk/sunxi/clk-sun8i-mbus.c |  2 ++
> >> >> >  drivers/clk/sunxi/clk-sun9i-core.c |  6 ++++++
> >> >> >  drivers/clk/sunxi/clk-sunxi.c      | 10 ++++++++++
> >> >> >  6 files changed, 32 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> >> >> > index 8c20190a3e9f..100a711c3e3d 100644
> >> >> > --- a/drivers/clk/sunxi/clk-factors.c
> >> >> > +++ b/drivers/clk/sunxi/clk-factors.c
> >> >> > @@ -56,15 +56,24 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
> >> >> >         /* Get each individual factor if applicable */
> >> >> >         if (config->nwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >> >> >                 n = FACTOR_GET(config->nshift, config->nwidth, reg);
> >> >> > +
> >> >> >         if (config->kwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >> >> >                 k = FACTOR_GET(config->kshift, config->kwidth, reg);
> >> >> > +
> >> >> >         if (config->mwidth != SUNXI_FACTORS_NOT_APPLICABLE)
> >> >> >                 m = FACTOR_GET(config->mshift, config->mwidth, reg);
> >> >> > +       else
> >> >> > +               /* Make sure we don't get a division by zero */
> >> >> > +               m = 1;
> >> >>
> >> >> What happens when mwidth is valid, m_start = 0, and m = 0?
> >> >
> >> > That's a very good question. A division by zero in the kernel, I'd
> >> > say.
> >> >
> >> > But I don't think we can end up in such a case today, and it's
> >> > somewhat expected that it will happen, and no clock have looked at can
> >> > actually end up in such a case.
> >>
> >> It's possible if the bootloader left the clock in an invalid state.
> >> How about just returning 0, indicating an invalid rate, early?
> >
> > The value set in the register might be zero, but that will always
> > really mean 1, either through m_start or m_zero. That would be a bug
> > in the driver itself.
> 
> Maybe you should switch the order of these 2 patches to make it clear?
> Or maybe squash them? As it currently is, m_start = 0 and m = 0 with
> this patch only is a bug.

We don't have any clock with m_start = 0 at this point. All the clocks
have m_start = 1, which is safe.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/69274997/attachment-0001.sig>

[PATCH 2/8] clk: sunxi: factors: Add a parameter for N and M zeros

[Maxime Ripard]

On some clocks, N and M set to 0 in the register actually mean a value of
'1', but any other value doesn't have that shift anymore.

Add parameters to the factors clock to handle this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-factors.c | 6 ++++++
 drivers/clk/sunxi/clk-factors.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 100a711c3e3d..632926a13c26 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -69,6 +69,12 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	if (config->pwidth != SUNXI_FACTORS_NOT_APPLICABLE)
 		p = FACTOR_GET(config->pshift, config->pwidth, reg);
 
+	if (!n && config->n_zero)
+		n = config->n_zero;
+
+	if (!m && config->m_zero)
+		m = config->m_zero;
+
 	/* Calculate the rate */
 	rate = parent_rate * (n + config->n_start);
 	rate *= k + 1;
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index 735d756d2923..fb182d9ccde6 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -19,6 +19,9 @@ struct clk_factors_config {
 
 	u8 m_start;
 	u8 n_start;
+
+	u8 n_zero;
+	u8 m_zero;
 };
 
 struct factors_data {
-- 
2.3.6

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Maxime Ripard]

The PLL2 on the A10 and later SoCs is the clock used for all the audio
related operations.

This clock has a somewhat complex output tree, with three outputs (2X, 4X
and 8X) with a fixed divider from the base clock, and an output (1X) with a
post divider.

However, we can simplify things since the 1X divider can be fixed, and we
end up by having a base clock not exposed to any device (or at least
directly, since the 4X output doesn't have any divider), and 4 fixed
divider clocks that will be exposed.

This clock seems to have been introduced, at least in this form, in the
revision B of the A10, but we don't have any information on the clock used
on the revision A.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile                 |   1 +
 drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
 3 files changed, 230 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
 create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 058f273d6154..eb36c38d4120 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -4,6 +4,7 @@
 
 obj-y += clk-sunxi.o clk-factors.o
 obj-y += clk-a10-hosc.o
+obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
 obj-y += clk-sun8i-mbus.o
diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
new file mode 100644
index 000000000000..4d0369626dba
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-pll2.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ * Emilio L?pez <emilio@elopez.com.ar>
+ *
+ * Copyright 2015 Maxime Ripard
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
+
+#include "clk-factors.h"
+
+#define SUN4I_PLL2_ENABLE		31
+#define SUN4I_PLL2_POST_DIV		26
+#define SUN4I_PLL2_POST_DIV_MASK	0xF
+#define SUN4I_PLL2_N			8
+#define SUN4I_PLL2_N_MASK		0x7F
+#define SUN4I_PLL2_PRE_DIV		0
+#define SUN4I_PLL2_PRE_DIV_MASK		0x1F
+
+#define SUN4I_PLL2_POST_DIV_VALUE	21
+#define SUN4I_PLL2_PRE_DIV_VALUE	4
+
+#define SUN4I_PLL2_OUTPUTS		4
+
+static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
+					    u8 *n, u8 *k, u8 *m, u8 *p)
+{
+	/*
+	 * Normalize the frequency to a multiple of (24 MHz / Fixed
+	 * PRE-DIV)
+	 */
+	*freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
+
+	/* We were called to round the frequency, we can return */
+	if (!n)
+		return;
+
+	*n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
+
+	/*
+	 * Even though the pre-divider can be changed, we don't really
+	 * care and we can just fix it to 4.
+	 */
+	*m = SUN4I_PLL2_PRE_DIV_VALUE;
+}
+
+static struct clk_factors_config sun4i_a10_pll2_base_config = {
+	.mshift = SUN4I_PLL2_PRE_DIV,
+	.mwidth = 5,
+	.nshift = SUN4I_PLL2_N,
+	.nwidth = 7,
+
+	.m_zero = 1,
+	.n_zero = 1,
+};
+
+static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
+	.enable = SUN4I_PLL2_ENABLE,
+	.table = &sun4i_a10_pll2_base_config,
+	.getter = sun4i_a10_get_pll2_base_factors,
+	.name = "pll2-base",
+};
+
+static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
+
+static void __init sun4i_pll2_setup(struct device_node *node)
+{
+	const char *clk_name = node->name, *parent;
+	struct clk_onecell_data *clk_data;
+	struct clk **clks, *base_clk;
+	void __iomem *reg;
+	u32 val;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
+	if (!clk_data)
+		goto err_unmap;
+
+	clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
+	if (!clks)
+		goto err_free_data;
+
+	base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
+					  &sun4i_a10_pll2_lock, reg);
+	if (!base_clk) {
+		pr_err("Couldn't register the base factor clock\n");
+		goto err_free_array;
+	}
+
+	parent = __clk_get_name(base_clk);
+
+	/*
+	 * PLL2-1x
+	 *
+	 * This is supposed to have a post divider, but we won't need
+	 * to use it, we just need to initialise it to 4, and use a
+	 * fixed divider.
+	 */
+	val = readl(reg);
+	val &= ~(SUN4I_PLL2_POST_DIV_MASK << SUN4I_PLL2_POST_DIV);
+	val |= SUN4I_PLL2_POST_DIV_VALUE << SUN4I_PLL2_POST_DIV;
+	writel(val, reg);
+
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_1X, &clk_name);
+	clks[SUN4I_A10_PLL2_1X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 4);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_1X]));
+
+	/*
+	 * PLL2-2x
+	 *
+	 * This clock doesn't use the post divider, and really is just
+	 * a fixed divider from the PLL2 base clock.
+	 */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_2X, &clk_name);
+	clks[SUN4I_A10_PLL2_2X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 2);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_2X]));
+
+	/* PLL2-4x */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_4X, &clk_name);
+	clks[SUN4I_A10_PLL2_4X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 1);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_4X]));
+
+	/* PLL2-8x */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_8X, &clk_name);
+	clks[SUN4I_A10_PLL2_8X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    2, 1);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_8X]));
+
+	clk_data->clks = clks;
+	clk_data->clk_num = SUN4I_PLL2_OUTPUTS;
+	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+	return;
+
+err_free_array:
+	kfree(clks);
+err_free_data:
+	kfree(clk_data);
+err_unmap:
+	iounmap(reg);
+}
+CLK_OF_DECLARE(sun4i_pll2, "allwinner,sun4i-a10-b-pll2-clk", sun4i_pll2_setup);
diff --git a/include/dt-bindings/clock/sun4i-a10-pll2.h b/include/dt-bindings/clock/sun4i-a10-pll2.h
new file mode 100644
index 000000000000..071c8112d531
--- /dev/null
+++ b/include/dt-bindings/clock/sun4i-a10-pll2.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2015 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
+#define __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
+
+#define SUN4I_A10_PLL2_1X	0
+#define SUN4I_A10_PLL2_2X	1
+#define SUN4I_A10_PLL2_4X	2
+#define SUN4I_A10_PLL2_8X	3
+
+#endif /* __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_ */
-- 
2.3.6

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Chen-Yu Tsai]

Hi,

On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The PLL2 on the A10 and later SoCs is the clock used for all the audio
> related operations.
>
> This clock has a somewhat complex output tree, with three outputs (2X, 4X
> and 8X) with a fixed divider from the base clock, and an output (1X) with a
> post divider.
>
> However, we can simplify things since the 1X divider can be fixed, and we
> end up by having a base clock not exposed to any device (or at least
> directly, since the 4X output doesn't have any divider), and 4 fixed
> divider clocks that will be exposed.
>
> This clock seems to have been introduced, at least in this form, in the
> revision B of the A10, but we don't have any information on the clock used
> on the revision A.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/Makefile                 |   1 +
>  drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
>  include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
>  3 files changed, 230 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
>  create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h
>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index 058f273d6154..eb36c38d4120 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -4,6 +4,7 @@
>
>  obj-y += clk-sunxi.o clk-factors.o
>  obj-y += clk-a10-hosc.o
> +obj-y += clk-a10-pll2.o
>  obj-y += clk-a20-gmac.o
>  obj-y += clk-mod0.o
>  obj-y += clk-sun8i-mbus.o
> diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
> new file mode 100644
> index 000000000000..4d0369626dba
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-a10-pll2.c
> @@ -0,0 +1,176 @@
> +/*
> + * Copyright 2013 Emilio L?pez
> + * Emilio L?pez <emilio@elopez.com.ar>
> + *
> + * Copyright 2015 Maxime Ripard
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/clock/sun4i-a10-pll2.h>
> +
> +#include "clk-factors.h"
> +
> +#define SUN4I_PLL2_ENABLE              31
> +#define SUN4I_PLL2_POST_DIV            26
> +#define SUN4I_PLL2_POST_DIV_MASK       0xF
> +#define SUN4I_PLL2_N                   8
> +#define SUN4I_PLL2_N_MASK              0x7F
> +#define SUN4I_PLL2_PRE_DIV             0
> +#define SUN4I_PLL2_PRE_DIV_MASK                0x1F
> +
> +#define SUN4I_PLL2_POST_DIV_VALUE      21
> +#define SUN4I_PLL2_PRE_DIV_VALUE       4
> +
> +#define SUN4I_PLL2_OUTPUTS             4
> +
> +static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
> +                                           u8 *n, u8 *k, u8 *m, u8 *p)
> +{
> +       /*
> +        * Normalize the frequency to a multiple of (24 MHz / Fixed
> +        * PRE-DIV)
> +        */
> +       *freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
> +
> +       /* We were called to round the frequency, we can return */
> +       if (!n)
> +               return;
> +
> +       *n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
> +
> +       /*
> +        * Even though the pre-divider can be changed, we don't really
> +        * care and we can just fix it to 4.
> +        */
> +       *m = SUN4I_PLL2_PRE_DIV_VALUE;
> +}
> +
> +static struct clk_factors_config sun4i_a10_pll2_base_config = {
> +       .mshift = SUN4I_PLL2_PRE_DIV,
> +       .mwidth = 5,
> +       .nshift = SUN4I_PLL2_N,
> +       .nwidth = 7,
> +
> +       .m_zero = 1,
> +       .n_zero = 1,
> +};
> +
> +static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
> +       .enable = SUN4I_PLL2_ENABLE,
> +       .table = &sun4i_a10_pll2_base_config,
> +       .getter = sun4i_a10_get_pll2_base_factors,
> +       .name = "pll2-base",
> +};
> +
> +static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
> +
> +static void __init sun4i_pll2_setup(struct device_node *node)
> +{
> +       const char *clk_name = node->name, *parent;
> +       struct clk_onecell_data *clk_data;
> +       struct clk **clks, *base_clk;
> +       void __iomem *reg;
> +       u32 val;
> +
> +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +       if (IS_ERR(reg))
> +               return;
> +
> +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> +       if (!clk_data)
> +               goto err_unmap;
> +
> +       clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
> +       if (!clks)
> +               goto err_free_data;
> +
> +       base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
> +                                         &sun4i_a10_pll2_lock, reg);

Why aren't you using divs_clk for this? It seems right for the job.

ChenYu

> +       if (!base_clk) {
> +               pr_err("Couldn't register the base factor clock\n");
> +               goto err_free_array;
> +       }
> +
> +       parent = __clk_get_name(base_clk);
> +
> +       /*
> +        * PLL2-1x
> +        *
> +        * This is supposed to have a post divider, but we won't need
> +        * to use it, we just need to initialise it to 4, and use a
> +        * fixed divider.
> +        */
> +       val = readl(reg);
> +       val &= ~(SUN4I_PLL2_POST_DIV_MASK << SUN4I_PLL2_POST_DIV);
> +       val |= SUN4I_PLL2_POST_DIV_VALUE << SUN4I_PLL2_POST_DIV;
> +       writel(val, reg);
> +
> +       of_property_read_string_index(node, "clock-output-names",
> +                                     SUN4I_A10_PLL2_1X, &clk_name);
> +       clks[SUN4I_A10_PLL2_1X] = clk_register_fixed_factor(NULL, clk_name,
> +                                                           parent,
> +                                                           CLK_SET_RATE_PARENT,
> +                                                           1, 4);
> +       WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_1X]));
> +
> +       /*
> +        * PLL2-2x
> +        *
> +        * This clock doesn't use the post divider, and really is just
> +        * a fixed divider from the PLL2 base clock.
> +        */
> +       of_property_read_string_index(node, "clock-output-names",
> +                                     SUN4I_A10_PLL2_2X, &clk_name);
> +       clks[SUN4I_A10_PLL2_2X] = clk_register_fixed_factor(NULL, clk_name,
> +                                                           parent,
> +                                                           CLK_SET_RATE_PARENT,
> +                                                           1, 2);
> +       WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_2X]));
> +
> +       /* PLL2-4x */
> +       of_property_read_string_index(node, "clock-output-names",
> +                                     SUN4I_A10_PLL2_4X, &clk_name);
> +       clks[SUN4I_A10_PLL2_4X] = clk_register_fixed_factor(NULL, clk_name,
> +                                                           parent,
> +                                                           CLK_SET_RATE_PARENT,
> +                                                           1, 1);
> +       WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_4X]));
> +
> +       /* PLL2-8x */
> +       of_property_read_string_index(node, "clock-output-names",
> +                                     SUN4I_A10_PLL2_8X, &clk_name);
> +       clks[SUN4I_A10_PLL2_8X] = clk_register_fixed_factor(NULL, clk_name,
> +                                                           parent,
> +                                                           CLK_SET_RATE_PARENT,
> +                                                           2, 1);
> +       WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_8X]));
> +
> +       clk_data->clks = clks;
> +       clk_data->clk_num = SUN4I_PLL2_OUTPUTS;
> +       of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> +
> +       return;
> +
> +err_free_array:
> +       kfree(clks);
> +err_free_data:
> +       kfree(clk_data);
> +err_unmap:
> +       iounmap(reg);
> +}
> +CLK_OF_DECLARE(sun4i_pll2, "allwinner,sun4i-a10-b-pll2-clk", sun4i_pll2_setup);
> diff --git a/include/dt-bindings/clock/sun4i-a10-pll2.h b/include/dt-bindings/clock/sun4i-a10-pll2.h
> new file mode 100644
> index 000000000000..071c8112d531
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun4i-a10-pll2.h
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright 2015 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful,
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
> +#define __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
> +
> +#define SUN4I_A10_PLL2_1X      0
> +#define SUN4I_A10_PLL2_2X      1
> +#define SUN4I_A10_PLL2_4X      2
> +#define SUN4I_A10_PLL2_8X      3
> +
> +#endif /* __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_ */
> --
> 2.3.6
>

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Maxime Ripard]

On Thu, May 14, 2015 at 05:43:51PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The PLL2 on the A10 and later SoCs is the clock used for all the audio
> > related operations.
> >
> > This clock has a somewhat complex output tree, with three outputs (2X, 4X
> > and 8X) with a fixed divider from the base clock, and an output (1X) with a
> > post divider.
> >
> > However, we can simplify things since the 1X divider can be fixed, and we
> > end up by having a base clock not exposed to any device (or at least
> > directly, since the 4X output doesn't have any divider), and 4 fixed
> > divider clocks that will be exposed.
> >
> > This clock seems to have been introduced, at least in this form, in the
> > revision B of the A10, but we don't have any information on the clock used
> > on the revision A.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/sunxi/Makefile                 |   1 +
> >  drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
> >  include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
> >  3 files changed, 230 insertions(+)
> >  create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
> >  create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h
> >
> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> > index 058f273d6154..eb36c38d4120 100644
> > --- a/drivers/clk/sunxi/Makefile
> > +++ b/drivers/clk/sunxi/Makefile
> > @@ -4,6 +4,7 @@
> >
> >  obj-y += clk-sunxi.o clk-factors.o
> >  obj-y += clk-a10-hosc.o
> > +obj-y += clk-a10-pll2.o
> >  obj-y += clk-a20-gmac.o
> >  obj-y += clk-mod0.o
> >  obj-y += clk-sun8i-mbus.o
> > diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
> > new file mode 100644
> > index 000000000000..4d0369626dba
> > --- /dev/null
> > +++ b/drivers/clk/sunxi/clk-a10-pll2.c
> > @@ -0,0 +1,176 @@
> > +/*
> > + * Copyright 2013 Emilio L?pez
> > + * Emilio L?pez <emilio@elopez.com.ar>
> > + *
> > + * Copyright 2015 Maxime Ripard
> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +
> > +#include <dt-bindings/clock/sun4i-a10-pll2.h>
> > +
> > +#include "clk-factors.h"
> > +
> > +#define SUN4I_PLL2_ENABLE              31
> > +#define SUN4I_PLL2_POST_DIV            26
> > +#define SUN4I_PLL2_POST_DIV_MASK       0xF
> > +#define SUN4I_PLL2_N                   8
> > +#define SUN4I_PLL2_N_MASK              0x7F
> > +#define SUN4I_PLL2_PRE_DIV             0
> > +#define SUN4I_PLL2_PRE_DIV_MASK                0x1F
> > +
> > +#define SUN4I_PLL2_POST_DIV_VALUE      21
> > +#define SUN4I_PLL2_PRE_DIV_VALUE       4
> > +
> > +#define SUN4I_PLL2_OUTPUTS             4
> > +
> > +static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
> > +                                           u8 *n, u8 *k, u8 *m, u8 *p)
> > +{
> > +       /*
> > +        * Normalize the frequency to a multiple of (24 MHz / Fixed
> > +        * PRE-DIV)
> > +        */
> > +       *freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
> > +
> > +       /* We were called to round the frequency, we can return */
> > +       if (!n)
> > +               return;
> > +
> > +       *n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
> > +
> > +       /*
> > +        * Even though the pre-divider can be changed, we don't really
> > +        * care and we can just fix it to 4.
> > +        */
> > +       *m = SUN4I_PLL2_PRE_DIV_VALUE;
> > +}
> > +
> > +static struct clk_factors_config sun4i_a10_pll2_base_config = {
> > +       .mshift = SUN4I_PLL2_PRE_DIV,
> > +       .mwidth = 5,
> > +       .nshift = SUN4I_PLL2_N,
> > +       .nwidth = 7,
> > +
> > +       .m_zero = 1,
> > +       .n_zero = 1,
> > +};
> > +
> > +static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
> > +       .enable = SUN4I_PLL2_ENABLE,
> > +       .table = &sun4i_a10_pll2_base_config,
> > +       .getter = sun4i_a10_get_pll2_base_factors,
> > +       .name = "pll2-base",
> > +};
> > +
> > +static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
> > +
> > +static void __init sun4i_pll2_setup(struct device_node *node)
> > +{
> > +       const char *clk_name = node->name, *parent;
> > +       struct clk_onecell_data *clk_data;
> > +       struct clk **clks, *base_clk;
> > +       void __iomem *reg;
> > +       u32 val;
> > +
> > +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> > +       if (IS_ERR(reg))
> > +               return;
> > +
> > +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> > +       if (!clk_data)
> > +               goto err_unmap;
> > +
> > +       clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
> > +       if (!clks)
> > +               goto err_free_data;
> > +
> > +       base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
> > +                                         &sun4i_a10_pll2_lock, reg);
> 
> Why aren't you using divs_clk for this? It seems right for the job.

As far as I know, divs_clk can only handle a single divider, and not
two subsequent dividers like this one uses.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/2f099d3d/attachment-0001.sig>

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Chen-Yu Tsai]

On Fri, May 15, 2015 at 3:45 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Thu, May 14, 2015 at 05:43:51PM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The PLL2 on the A10 and later SoCs is the clock used for all the audio
>> > related operations.
>> >
>> > This clock has a somewhat complex output tree, with three outputs (2X, 4X
>> > and 8X) with a fixed divider from the base clock, and an output (1X) with a
>> > post divider.
>> >
>> > However, we can simplify things since the 1X divider can be fixed, and we
>> > end up by having a base clock not exposed to any device (or at least
>> > directly, since the 4X output doesn't have any divider), and 4 fixed
>> > divider clocks that will be exposed.
>> >
>> > This clock seems to have been introduced, at least in this form, in the
>> > revision B of the A10, but we don't have any information on the clock used
>> > on the revision A.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  drivers/clk/sunxi/Makefile                 |   1 +
>> >  drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
>> >  include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
>> >  3 files changed, 230 insertions(+)
>> >  create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
>> >  create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h
>> >
>> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> > index 058f273d6154..eb36c38d4120 100644
>> > --- a/drivers/clk/sunxi/Makefile
>> > +++ b/drivers/clk/sunxi/Makefile
>> > @@ -4,6 +4,7 @@
>> >
>> >  obj-y += clk-sunxi.o clk-factors.o
>> >  obj-y += clk-a10-hosc.o
>> > +obj-y += clk-a10-pll2.o
>> >  obj-y += clk-a20-gmac.o
>> >  obj-y += clk-mod0.o
>> >  obj-y += clk-sun8i-mbus.o
>> > diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
>> > new file mode 100644
>> > index 000000000000..4d0369626dba
>> > --- /dev/null
>> > +++ b/drivers/clk/sunxi/clk-a10-pll2.c
>> > @@ -0,0 +1,176 @@
>> > +/*
>> > + * Copyright 2013 Emilio L?pez
>> > + * Emilio L?pez <emilio@elopez.com.ar>
>> > + *
>> > + * Copyright 2015 Maxime Ripard
>> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation; either version 2 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + */
>> > +
>> > +#include <linux/clk-provider.h>
>> > +#include <linux/of.h>
>> > +#include <linux/of_address.h>
>> > +#include <linux/slab.h>
>> > +
>> > +#include <dt-bindings/clock/sun4i-a10-pll2.h>
>> > +
>> > +#include "clk-factors.h"
>> > +
>> > +#define SUN4I_PLL2_ENABLE              31
>> > +#define SUN4I_PLL2_POST_DIV            26
>> > +#define SUN4I_PLL2_POST_DIV_MASK       0xF
>> > +#define SUN4I_PLL2_N                   8
>> > +#define SUN4I_PLL2_N_MASK              0x7F
>> > +#define SUN4I_PLL2_PRE_DIV             0
>> > +#define SUN4I_PLL2_PRE_DIV_MASK                0x1F
>> > +
>> > +#define SUN4I_PLL2_POST_DIV_VALUE      21

Another thing:

This says 21, which obviously exceeds the mask defined above,
but the comment in the section below (in the original code)
says 4. I assume the comment was right, based on the user
manual?

Maybe it should be swapped with SUN4I_PLL2_PRE_DIV_VALUE?
I get PRE_DIV = 21 for generating a clock close to 22.5792 MHz.

Seems AC97 might need another frequency, but the manual isn't
clear, and we can handle it when we add AC97 anyway.

>> > +#define SUN4I_PLL2_PRE_DIV_VALUE       4
>> > +
>> > +#define SUN4I_PLL2_OUTPUTS             4
>> > +
>> > +static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
>> > +                                           u8 *n, u8 *k, u8 *m, u8 *p)
>> > +{
>> > +       /*
>> > +        * Normalize the frequency to a multiple of (24 MHz / Fixed
>> > +        * PRE-DIV)
>> > +        */
>> > +       *freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
>> > +
>> > +       /* We were called to round the frequency, we can return */
>> > +       if (!n)
>> > +               return;
>> > +
>> > +       *n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
>> > +
>> > +       /*
>> > +        * Even though the pre-divider can be changed, we don't really
>> > +        * care and we can just fix it to 4.
>> > +        */
>> > +       *m = SUN4I_PLL2_PRE_DIV_VALUE;
>> > +}
>> > +
>> > +static struct clk_factors_config sun4i_a10_pll2_base_config = {
>> > +       .mshift = SUN4I_PLL2_PRE_DIV,
>> > +       .mwidth = 5,
>> > +       .nshift = SUN4I_PLL2_N,
>> > +       .nwidth = 7,
>> > +
>> > +       .m_zero = 1,
>> > +       .n_zero = 1,
>> > +};
>> > +
>> > +static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
>> > +       .enable = SUN4I_PLL2_ENABLE,
>> > +       .table = &sun4i_a10_pll2_base_config,
>> > +       .getter = sun4i_a10_get_pll2_base_factors,
>> > +       .name = "pll2-base",
>> > +};
>> > +
>> > +static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
>> > +
>> > +static void __init sun4i_pll2_setup(struct device_node *node)
>> > +{
>> > +       const char *clk_name = node->name, *parent;
>> > +       struct clk_onecell_data *clk_data;
>> > +       struct clk **clks, *base_clk;
>> > +       void __iomem *reg;
>> > +       u32 val;
>> > +
>> > +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
>> > +       if (IS_ERR(reg))
>> > +               return;
>> > +
>> > +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>> > +       if (!clk_data)
>> > +               goto err_unmap;
>> > +
>> > +       clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
>> > +       if (!clks)
>> > +               goto err_free_data;
>> > +
>> > +       base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
>> > +                                         &sun4i_a10_pll2_lock, reg);
>>
>> Why aren't you using divs_clk for this? It seems right for the job.
>
> As far as I know, divs_clk can only handle a single divider, and not
> two subsequent dividers like this one uses.

I thought we were setting the post divider to 4 so the outputs match
the names? Maybe we could get it in as is (with the above comment
addressed), and refactor it later. I'm still stuck on factors_clk
stuff....


ChenYu

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Maxime Ripard]

On Fri, May 15, 2015 at 05:15:32PM +0800, Chen-Yu Tsai wrote:
> On Fri, May 15, 2015 at 3:45 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Thu, May 14, 2015 at 05:43:51PM +0800, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > The PLL2 on the A10 and later SoCs is the clock used for all the audio
> >> > related operations.
> >> >
> >> > This clock has a somewhat complex output tree, with three outputs (2X, 4X
> >> > and 8X) with a fixed divider from the base clock, and an output (1X) with a
> >> > post divider.
> >> >
> >> > However, we can simplify things since the 1X divider can be fixed, and we
> >> > end up by having a base clock not exposed to any device (or at least
> >> > directly, since the 4X output doesn't have any divider), and 4 fixed
> >> > divider clocks that will be exposed.
> >> >
> >> > This clock seems to have been introduced, at least in this form, in the
> >> > revision B of the A10, but we don't have any information on the clock used
> >> > on the revision A.
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> >  drivers/clk/sunxi/Makefile                 |   1 +
> >> >  drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
> >> >  include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
> >> >  3 files changed, 230 insertions(+)
> >> >  create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
> >> >  create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h
> >> >
> >> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> >> > index 058f273d6154..eb36c38d4120 100644
> >> > --- a/drivers/clk/sunxi/Makefile
> >> > +++ b/drivers/clk/sunxi/Makefile
> >> > @@ -4,6 +4,7 @@
> >> >
> >> >  obj-y += clk-sunxi.o clk-factors.o
> >> >  obj-y += clk-a10-hosc.o
> >> > +obj-y += clk-a10-pll2.o
> >> >  obj-y += clk-a20-gmac.o
> >> >  obj-y += clk-mod0.o
> >> >  obj-y += clk-sun8i-mbus.o
> >> > diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
> >> > new file mode 100644
> >> > index 000000000000..4d0369626dba
> >> > --- /dev/null
> >> > +++ b/drivers/clk/sunxi/clk-a10-pll2.c
> >> > @@ -0,0 +1,176 @@
> >> > +/*
> >> > + * Copyright 2013 Emilio L?pez
> >> > + * Emilio L?pez <emilio@elopez.com.ar>
> >> > + *
> >> > + * Copyright 2015 Maxime Ripard
> >> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > + *
> >> > + * This program is free software; you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation; either version 2 of the License, or
> >> > + * (at your option) any later version.
> >> > + *
> >> > + * This program is distributed in the hope that it will be useful,
> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> > + * GNU General Public License for more details.
> >> > + */
> >> > +
> >> > +#include <linux/clk-provider.h>
> >> > +#include <linux/of.h>
> >> > +#include <linux/of_address.h>
> >> > +#include <linux/slab.h>
> >> > +
> >> > +#include <dt-bindings/clock/sun4i-a10-pll2.h>
> >> > +
> >> > +#include "clk-factors.h"
> >> > +
> >> > +#define SUN4I_PLL2_ENABLE              31
> >> > +#define SUN4I_PLL2_POST_DIV            26
> >> > +#define SUN4I_PLL2_POST_DIV_MASK       0xF
> >> > +#define SUN4I_PLL2_N                   8
> >> > +#define SUN4I_PLL2_N_MASK              0x7F
> >> > +#define SUN4I_PLL2_PRE_DIV             0
> >> > +#define SUN4I_PLL2_PRE_DIV_MASK                0x1F
> >> > +
> >> > +#define SUN4I_PLL2_POST_DIV_VALUE      21
> 
> Another thing:
> 
> This says 21, which obviously exceeds the mask defined above,
> but the comment in the section below (in the original code)
> says 4. I assume the comment was right, based on the user
> manual?
> 
> Maybe it should be swapped with SUN4I_PLL2_PRE_DIV_VALUE?
> I get PRE_DIV = 21 for generating a clock close to 22.5792 MHz.

You're right. POST_DIV_VALUE and PRE_DIV_VALUE are inverted....

I wonder how that was even working...

> Seems AC97 might need another frequency, but the manual isn't
> clear, and we can handle it when we add AC97 anyway.

I'm not even sure there's a board out there with an AC97 codec...

> 
> >> > +#define SUN4I_PLL2_PRE_DIV_VALUE       4
> >> > +
> >> > +#define SUN4I_PLL2_OUTPUTS             4
> >> > +
> >> > +static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
> >> > +                                           u8 *n, u8 *k, u8 *m, u8 *p)
> >> > +{
> >> > +       /*
> >> > +        * Normalize the frequency to a multiple of (24 MHz / Fixed
> >> > +        * PRE-DIV)
> >> > +        */
> >> > +       *freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
> >> > +
> >> > +       /* We were called to round the frequency, we can return */
> >> > +       if (!n)
> >> > +               return;
> >> > +
> >> > +       *n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
> >> > +
> >> > +       /*
> >> > +        * Even though the pre-divider can be changed, we don't really
> >> > +        * care and we can just fix it to 4.
> >> > +        */
> >> > +       *m = SUN4I_PLL2_PRE_DIV_VALUE;
> >> > +}
> >> > +
> >> > +static struct clk_factors_config sun4i_a10_pll2_base_config = {
> >> > +       .mshift = SUN4I_PLL2_PRE_DIV,
> >> > +       .mwidth = 5,
> >> > +       .nshift = SUN4I_PLL2_N,
> >> > +       .nwidth = 7,
> >> > +
> >> > +       .m_zero = 1,
> >> > +       .n_zero = 1,
> >> > +};
> >> > +
> >> > +static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
> >> > +       .enable = SUN4I_PLL2_ENABLE,
> >> > +       .table = &sun4i_a10_pll2_base_config,
> >> > +       .getter = sun4i_a10_get_pll2_base_factors,
> >> > +       .name = "pll2-base",
> >> > +};
> >> > +
> >> > +static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
> >> > +
> >> > +static void __init sun4i_pll2_setup(struct device_node *node)
> >> > +{
> >> > +       const char *clk_name = node->name, *parent;
> >> > +       struct clk_onecell_data *clk_data;
> >> > +       struct clk **clks, *base_clk;
> >> > +       void __iomem *reg;
> >> > +       u32 val;
> >> > +
> >> > +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> >> > +       if (IS_ERR(reg))
> >> > +               return;
> >> > +
> >> > +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> >> > +       if (!clk_data)
> >> > +               goto err_unmap;
> >> > +
> >> > +       clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
> >> > +       if (!clks)
> >> > +               goto err_free_data;
> >> > +
> >> > +       base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
> >> > +                                         &sun4i_a10_pll2_lock, reg);
> >>
> >> Why aren't you using divs_clk for this? It seems right for the job.
> >
> > As far as I know, divs_clk can only handle a single divider, and not
> > two subsequent dividers like this one uses.
> 
> I thought we were setting the post divider to 4 so the outputs match
> the names? Maybe we could get it in as is (with the above comment
> addressed), and refactor it later. I'm still stuck on factors_clk
> stuff....

We are, but the pre-div is considered as the M factor, and the N
factor as N, so we can't really consider it as a single divider.

Thinking a bit more about this, what we could do though, is splitting
the pll2-base clock in half, one that would expose a single divider
using clk-divider for the pre-divider, then a factor clock for the N
factor, and then multiple dividers for the post-divider and other
outputs.

I wonder if that could not even be made that way for all the factors
clock. That would reduce the needed logic in the drivers a lot.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/b700b123/attachment.sig>

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Chen-Yu Tsai]

On Fri, May 15, 2015 at 10:44 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, May 15, 2015 at 05:15:32PM +0800, Chen-Yu Tsai wrote:
>> On Fri, May 15, 2015 at 3:45 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > On Thu, May 14, 2015 at 05:43:51PM +0800, Chen-Yu Tsai wrote:
>> >> Hi,
>> >>
>> >> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
>> >> <maxime.ripard@free-electrons.com> wrote:
>> >> > The PLL2 on the A10 and later SoCs is the clock used for all the audio
>> >> > related operations.
>> >> >
>> >> > This clock has a somewhat complex output tree, with three outputs (2X, 4X
>> >> > and 8X) with a fixed divider from the base clock, and an output (1X) with a
>> >> > post divider.
>> >> >
>> >> > However, we can simplify things since the 1X divider can be fixed, and we
>> >> > end up by having a base clock not exposed to any device (or at least
>> >> > directly, since the 4X output doesn't have any divider), and 4 fixed
>> >> > divider clocks that will be exposed.
>> >> >
>> >> > This clock seems to have been introduced, at least in this form, in the
>> >> > revision B of the A10, but we don't have any information on the clock used
>> >> > on the revision A.
>> >> >
>> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> >> > ---
>> >> >  drivers/clk/sunxi/Makefile                 |   1 +
>> >> >  drivers/clk/sunxi/clk-a10-pll2.c           | 176 +++++++++++++++++++++++++++++
>> >> >  include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++++
>> >> >  3 files changed, 230 insertions(+)
>> >> >  create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
>> >> >  create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h
>> >> >
>> >> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> >> > index 058f273d6154..eb36c38d4120 100644
>> >> > --- a/drivers/clk/sunxi/Makefile
>> >> > +++ b/drivers/clk/sunxi/Makefile
>> >> > @@ -4,6 +4,7 @@
>> >> >
>> >> >  obj-y += clk-sunxi.o clk-factors.o
>> >> >  obj-y += clk-a10-hosc.o
>> >> > +obj-y += clk-a10-pll2.o
>> >> >  obj-y += clk-a20-gmac.o
>> >> >  obj-y += clk-mod0.o
>> >> >  obj-y += clk-sun8i-mbus.o
>> >> > diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
>> >> > new file mode 100644
>> >> > index 000000000000..4d0369626dba
>> >> > --- /dev/null
>> >> > +++ b/drivers/clk/sunxi/clk-a10-pll2.c
>> >> > @@ -0,0 +1,176 @@
>> >> > +/*
>> >> > + * Copyright 2013 Emilio L?pez
>> >> > + * Emilio L?pez <emilio@elopez.com.ar>
>> >> > + *
>> >> > + * Copyright 2015 Maxime Ripard
>> >> > + * Maxime Ripard <maxime.ripard@free-electrons.com>
>> >> > + *
>> >> > + * This program is free software; you can redistribute it and/or modify
>> >> > + * it under the terms of the GNU General Public License as published by
>> >> > + * the Free Software Foundation; either version 2 of the License, or
>> >> > + * (at your option) any later version.
>> >> > + *
>> >> > + * This program is distributed in the hope that it will be useful,
>> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >> > + * GNU General Public License for more details.
>> >> > + */
>> >> > +
>> >> > +#include <linux/clk-provider.h>
>> >> > +#include <linux/of.h>
>> >> > +#include <linux/of_address.h>
>> >> > +#include <linux/slab.h>
>> >> > +
>> >> > +#include <dt-bindings/clock/sun4i-a10-pll2.h>
>> >> > +
>> >> > +#include "clk-factors.h"
>> >> > +
>> >> > +#define SUN4I_PLL2_ENABLE              31
>> >> > +#define SUN4I_PLL2_POST_DIV            26
>> >> > +#define SUN4I_PLL2_POST_DIV_MASK       0xF
>> >> > +#define SUN4I_PLL2_N                   8
>> >> > +#define SUN4I_PLL2_N_MASK              0x7F
>> >> > +#define SUN4I_PLL2_PRE_DIV             0
>> >> > +#define SUN4I_PLL2_PRE_DIV_MASK                0x1F
>> >> > +
>> >> > +#define SUN4I_PLL2_POST_DIV_VALUE      21
>>
>> Another thing:
>>
>> This says 21, which obviously exceeds the mask defined above,
>> but the comment in the section below (in the original code)
>> says 4. I assume the comment was right, based on the user
>> manual?
>>
>> Maybe it should be swapped with SUN4I_PLL2_PRE_DIV_VALUE?
>> I get PRE_DIV = 21 for generating a clock close to 22.5792 MHz.
>
> You're right. POST_DIV_VALUE and PRE_DIV_VALUE are inverted....
>
> I wonder how that was even working...
>
>> Seems AC97 might need another frequency, but the manual isn't
>> clear, and we can handle it when we add AC97 anyway.
>
> I'm not even sure there's a board out there with an AC97 codec...

We can handle it when and if we add AC97. :p

>>
>> >> > +#define SUN4I_PLL2_PRE_DIV_VALUE       4
>> >> > +
>> >> > +#define SUN4I_PLL2_OUTPUTS             4
>> >> > +
>> >> > +static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
>> >> > +                                           u8 *n, u8 *k, u8 *m, u8 *p)
>> >> > +{
>> >> > +       /*
>> >> > +        * Normalize the frequency to a multiple of (24 MHz / Fixed
>> >> > +        * PRE-DIV)
>> >> > +        */
>> >> > +       *freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
>> >> > +
>> >> > +       /* We were called to round the frequency, we can return */
>> >> > +       if (!n)
>> >> > +               return;
>> >> > +
>> >> > +       *n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
>> >> > +
>> >> > +       /*
>> >> > +        * Even though the pre-divider can be changed, we don't really
>> >> > +        * care and we can just fix it to 4.
>> >> > +        */
>> >> > +       *m = SUN4I_PLL2_PRE_DIV_VALUE;
>> >> > +}
>> >> > +
>> >> > +static struct clk_factors_config sun4i_a10_pll2_base_config = {
>> >> > +       .mshift = SUN4I_PLL2_PRE_DIV,
>> >> > +       .mwidth = 5,
>> >> > +       .nshift = SUN4I_PLL2_N,
>> >> > +       .nwidth = 7,
>> >> > +
>> >> > +       .m_zero = 1,
>> >> > +       .n_zero = 1,
>> >> > +};
>> >> > +
>> >> > +static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
>> >> > +       .enable = SUN4I_PLL2_ENABLE,
>> >> > +       .table = &sun4i_a10_pll2_base_config,
>> >> > +       .getter = sun4i_a10_get_pll2_base_factors,
>> >> > +       .name = "pll2-base",
>> >> > +};
>> >> > +
>> >> > +static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
>> >> > +
>> >> > +static void __init sun4i_pll2_setup(struct device_node *node)
>> >> > +{
>> >> > +       const char *clk_name = node->name, *parent;
>> >> > +       struct clk_onecell_data *clk_data;
>> >> > +       struct clk **clks, *base_clk;
>> >> > +       void __iomem *reg;
>> >> > +       u32 val;
>> >> > +
>> >> > +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
>> >> > +       if (IS_ERR(reg))
>> >> > +               return;
>> >> > +
>> >> > +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
>> >> > +       if (!clk_data)
>> >> > +               goto err_unmap;
>> >> > +
>> >> > +       clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
>> >> > +       if (!clks)
>> >> > +               goto err_free_data;
>> >> > +
>> >> > +       base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
>> >> > +                                         &sun4i_a10_pll2_lock, reg);
>> >>
>> >> Why aren't you using divs_clk for this? It seems right for the job.
>> >
>> > As far as I know, divs_clk can only handle a single divider, and not
>> > two subsequent dividers like this one uses.
>>
>> I thought we were setting the post divider to 4 so the outputs match
>> the names? Maybe we could get it in as is (with the above comment
>> addressed), and refactor it later. I'm still stuck on factors_clk
>> stuff....
>
> We are, but the pre-div is considered as the M factor, and the N
> factor as N, so we can't really consider it as a single divider.

Isn't pre-div used by all the clocks?

The manual lists:

1X = 48*N / pre-div / post-div / 2
2X = 48*N / pre-div / 4
4X = 48*N / pre-div / 2
8X = 48*N / pre-div

So couldn't you have a base factor clock as 24*N / pre-div,
and the outputs as follows?

1X = base / post-div
2X = base / 2
4X = base
8X = base * 2

> Thinking a bit more about this, what we could do though, is splitting
> the pll2-base clock in half, one that would expose a single divider
> using clk-divider for the pre-divider, then a factor clock for the N
> factor, and then multiple dividers for the post-divider and other
> outputs.
>
> I wonder if that could not even be made that way for all the factors
> clock. That would reduce the needed logic in the drivers a lot.

Taking apart what should be an integrated clock into separate parts
kind of makes the clock tree ugly. :(


ChenYu

[PATCH 3/8] clk: sunxi: Add a driver for the PLL2

[Maxime Ripard]

On Tue, May 19, 2015 at 03:58:09PM +0800, Chen-Yu Tsai wrote:
> >> >> > +#define SUN4I_PLL2_PRE_DIV_VALUE       4
> >> >> > +
> >> >> > +#define SUN4I_PLL2_OUTPUTS             4
> >> >> > +
> >> >> > +static void sun4i_a10_get_pll2_base_factors(u32 *freq, u32 parent_rate,
> >> >> > +                                           u8 *n, u8 *k, u8 *m, u8 *p)
> >> >> > +{
> >> >> > +       /*
> >> >> > +        * Normalize the frequency to a multiple of (24 MHz / Fixed
> >> >> > +        * PRE-DIV)
> >> >> > +        */
> >> >> > +       *freq = round_down(*freq, parent_rate / SUN4I_PLL2_PRE_DIV_VALUE);
> >> >> > +
> >> >> > +       /* We were called to round the frequency, we can return */
> >> >> > +       if (!n)
> >> >> > +               return;
> >> >> > +
> >> >> > +       *n = *freq * SUN4I_PLL2_PRE_DIV_VALUE / parent_rate;
> >> >> > +
> >> >> > +       /*
> >> >> > +        * Even though the pre-divider can be changed, we don't really
> >> >> > +        * care and we can just fix it to 4.
> >> >> > +        */
> >> >> > +       *m = SUN4I_PLL2_PRE_DIV_VALUE;
> >> >> > +}
> >> >> > +
> >> >> > +static struct clk_factors_config sun4i_a10_pll2_base_config = {
> >> >> > +       .mshift = SUN4I_PLL2_PRE_DIV,
> >> >> > +       .mwidth = 5,
> >> >> > +       .nshift = SUN4I_PLL2_N,
> >> >> > +       .nwidth = 7,
> >> >> > +
> >> >> > +       .m_zero = 1,
> >> >> > +       .n_zero = 1,
> >> >> > +};
> >> >> > +
> >> >> > +static const struct factors_data sun4i_a10_pll2_base_data __initconst = {
> >> >> > +       .enable = SUN4I_PLL2_ENABLE,
> >> >> > +       .table = &sun4i_a10_pll2_base_config,
> >> >> > +       .getter = sun4i_a10_get_pll2_base_factors,
> >> >> > +       .name = "pll2-base",
> >> >> > +};
> >> >> > +
> >> >> > +static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
> >> >> > +
> >> >> > +static void __init sun4i_pll2_setup(struct device_node *node)
> >> >> > +{
> >> >> > +       const char *clk_name = node->name, *parent;
> >> >> > +       struct clk_onecell_data *clk_data;
> >> >> > +       struct clk **clks, *base_clk;
> >> >> > +       void __iomem *reg;
> >> >> > +       u32 val;
> >> >> > +
> >> >> > +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> >> >> > +       if (IS_ERR(reg))
> >> >> > +               return;
> >> >> > +
> >> >> > +       clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> >> >> > +       if (!clk_data)
> >> >> > +               goto err_unmap;
> >> >> > +
> >> >> > +       clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
> >> >> > +       if (!clks)
> >> >> > +               goto err_free_data;
> >> >> > +
> >> >> > +       base_clk = sunxi_factors_register(node, &sun4i_a10_pll2_base_data,
> >> >> > +                                         &sun4i_a10_pll2_lock, reg);
> >> >>
> >> >> Why aren't you using divs_clk for this? It seems right for the job.
> >> >
> >> > As far as I know, divs_clk can only handle a single divider, and not
> >> > two subsequent dividers like this one uses.
> >>
> >> I thought we were setting the post divider to 4 so the outputs match
> >> the names? Maybe we could get it in as is (with the above comment
> >> addressed), and refactor it later. I'm still stuck on factors_clk
> >> stuff....
> >
> > We are, but the pre-div is considered as the M factor, and the N
> > factor as N, so we can't really consider it as a single divider.
> 
> Isn't pre-div used by all the clocks?
> 
> The manual lists:
> 
> 1X = 48*N / pre-div / post-div / 2
> 2X = 48*N / pre-div / 4
> 4X = 48*N / pre-div / 2
> 8X = 48*N / pre-div
> 
> So couldn't you have a base factor clock as 24*N / pre-div,
> and the outputs as follows?
> 
> 1X = base / post-div
> 2X = base / 2
> 4X = base
> 8X = base * 2

Good thing that it's what was done in this patch then :)

> > Thinking a bit more about this, what we could do though, is splitting
> > the pll2-base clock in half, one that would expose a single divider
> > using clk-divider for the pre-divider, then a factor clock for the N
> > factor, and then multiple dividers for the post-divider and other
> > outputs.
> >
> > I wonder if that could not even be made that way for all the factors
> > clock. That would reduce the needed logic in the drivers a lot.
> 
> Taking apart what should be an integrated clock into separate parts
> kind of makes the clock tree ugly. :(

But it would remove the amount of code that we maintain, extend and
debug by our own in favor of common, well-tested and factored code.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150519/d1a3d2a0/attachment.sig>

[PATCH 4/8] clk: sunxi: codec clock support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

The codec clock on sun4i, sun5i and sun7i is a simple gate with PLL2 as
parent. Add a driver for such a clock.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile        |  1 +
 drivers/clk/sunxi/clk-a10-codec.c | 45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index eb36c38d4120..6fa845e13067 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
+obj-y += clk-a10-codec.o
 obj-y += clk-a10-hosc.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
new file mode 100644
index 000000000000..aaeccf8cde39
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-codec.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ *
+ * Emilio L?pez <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define SUN4I_CODEC_GATE	31
+
+static void __init sun4i_codec_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name, *parent_name;
+	void __iomem *reg;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	clk = clk_register_gate(NULL, clk_name, parent_name,
+				CLK_SET_RATE_PARENT, reg,
+				SUN4I_CODEC_GATE, 0, NULL);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(sun4i_codec, "allwinner,sun4i-a10-codec-clk",
+	       sun4i_codec_clk_setup);
-- 
2.3.6

[PATCH 4/8] clk: sunxi: codec clock support

[Chen-Yu Tsai]

On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> From: Emilio L?pez <emilio@elopez.com.ar>
>
> The codec clock on sun4i, sun5i and sun7i is a simple gate with PLL2 as
> parent. Add a driver for such a clock.
>
> Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/Makefile        |  1 +
>  drivers/clk/sunxi/clk-a10-codec.c | 45 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-a10-codec.c
>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index eb36c38d4120..6fa845e13067 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -3,6 +3,7 @@
>  #
>
>  obj-y += clk-sunxi.o clk-factors.o
> +obj-y += clk-a10-codec.o
>  obj-y += clk-a10-hosc.o
>  obj-y += clk-a10-pll2.o
>  obj-y += clk-a20-gmac.o
> diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
> new file mode 100644
> index 000000000000..aaeccf8cde39
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-a10-codec.c
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright 2013 Emilio L?pez
> + *
> + * Emilio L?pez <emilio@elopez.com.ar>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>

This one is not needed.

Otherwise:

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +#define SUN4I_CODEC_GATE       31
> +
> +static void __init sun4i_codec_clk_setup(struct device_node *node)
> +{
> +       struct clk *clk;
> +       const char *clk_name = node->name, *parent_name;
> +       void __iomem *reg;
> +
> +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +       if (IS_ERR(reg))
> +               return;
> +
> +       of_property_read_string(node, "clock-output-names", &clk_name);
> +       parent_name = of_clk_get_parent_name(node, 0);
> +
> +       clk = clk_register_gate(NULL, clk_name, parent_name,
> +                               CLK_SET_RATE_PARENT, reg,
> +                               SUN4I_CODEC_GATE, 0, NULL);
> +
> +       if (!IS_ERR(clk))
> +               of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +}
> +CLK_OF_DECLARE(sun4i_codec, "allwinner,sun4i-a10-codec-clk",
> +              sun4i_codec_clk_setup);
> --
> 2.3.6
>

[PATCH 4/8] clk: sunxi: codec clock support

[Maxime Ripard]

On Thu, May 14, 2015 at 07:03:07PM +0800, Chen-Yu Tsai wrote:
> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > From: Emilio L?pez <emilio@elopez.com.ar>
> >
> > The codec clock on sun4i, sun5i and sun7i is a simple gate with PLL2 as
> > parent. Add a driver for such a clock.
> >
> > Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/sunxi/Makefile        |  1 +
> >  drivers/clk/sunxi/clk-a10-codec.c | 45 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 46 insertions(+)
> >  create mode 100644 drivers/clk/sunxi/clk-a10-codec.c
> >
> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> > index eb36c38d4120..6fa845e13067 100644
> > --- a/drivers/clk/sunxi/Makefile
> > +++ b/drivers/clk/sunxi/Makefile
> > @@ -3,6 +3,7 @@
> >  #
> >
> >  obj-y += clk-sunxi.o clk-factors.o
> > +obj-y += clk-a10-codec.o
> >  obj-y += clk-a10-hosc.o
> >  obj-y += clk-a10-pll2.o
> >  obj-y += clk-a20-gmac.o
> > diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
> > new file mode 100644
> > index 000000000000..aaeccf8cde39
> > --- /dev/null
> > +++ b/drivers/clk/sunxi/clk-a10-codec.c
> > @@ -0,0 +1,45 @@
> > +/*
> > + * Copyright 2013 Emilio L?pez
> > + *
> > + * Emilio L?pez <emilio@elopez.com.ar>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/clkdev.h>
> 
> This one is not needed.

Right, thanks!

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/664fcc84/attachment-0001.sig>

[PATCH 5/8] clk: sunxi: mod1 clock support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

The module 1 type of clocks consist of a gate and a mux and are used on
the audio blocks to mux and gate the PLL2 outputs for AC97, IIS or
SPDIF. This commit adds support for them on the sunxi clock driver.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile       |  1 +
 drivers/clk/sunxi/clk-a10-mod1.c | 85 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 6fa845e13067..960eeabc375f 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -5,6 +5,7 @@
 obj-y += clk-sunxi.o clk-factors.o
 obj-y += clk-a10-codec.o
 obj-y += clk-a10-hosc.o
+obj-y += clk-a10-mod1.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
diff --git a/drivers/clk/sunxi/clk-a10-mod1.c b/drivers/clk/sunxi/clk-a10-mod1.c
new file mode 100644
index 000000000000..6fc1b7a6de00
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-mod1.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ *
+ * Emilio L?pez <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+static DEFINE_SPINLOCK(mod1_lock);
+
+#define SUN4I_MOD1_ENABLE	31
+#define SUN4I_MOD1_MUX		16
+#define SUN4I_MOD1_MUX_WIDTH	2
+#define SUN4I_MOD1_MAX_PARENTS	4
+
+static void __init sun4i_mod1_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	struct clk_mux *mux;
+	struct clk_gate *gate;
+	const char *parents[4];
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	int i = 0;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		goto err_unmap;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		goto err_free_mux;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+	reg = of_iomap(node, 0);
+
+	while (i < SUN4I_MOD1_MAX_PARENTS &&
+	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
+		i++;
+
+	gate->reg = reg;
+	gate->bit_idx = SUN4I_MOD1_ENABLE;
+	gate->lock = &mod1_lock;
+	mux->reg = reg;
+	mux->shift = SUN4I_MOD1_MUX;
+	mux->mask = BIT(SUN4I_MOD1_MUX_WIDTH) - 1;
+	mux->lock = &mod1_lock;
+
+	clk = clk_register_composite(NULL, clk_name, parents, i,
+				     &mux->hw, &clk_mux_ops,
+				     NULL, NULL,
+				     &gate->hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk))
+		goto err_free_gate;
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+	return;
+
+err_free_gate:
+	kfree(gate);
+err_free_mux:
+	kfree(mux);
+err_unmap:
+	iounmap(reg);
+}
+CLK_OF_DECLARE(sun4i_mod1, "allwinner,sun4i-a10-mod1-clk",
+	       sun4i_mod1_clk_setup);
-- 
2.3.6

[PATCH 5/8] clk: sunxi: mod1 clock support

[Chen-Yu Tsai]

On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> From: Emilio L?pez <emilio@elopez.com.ar>
>
> The module 1 type of clocks consist of a gate and a mux and are used on
> the audio blocks to mux and gate the PLL2 outputs for AC97, IIS or
> SPDIF. This commit adds support for them on the sunxi clock driver.
>
> Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/Makefile       |  1 +
>  drivers/clk/sunxi/clk-a10-mod1.c | 85 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c
>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index 6fa845e13067..960eeabc375f 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -5,6 +5,7 @@
>  obj-y += clk-sunxi.o clk-factors.o
>  obj-y += clk-a10-codec.o
>  obj-y += clk-a10-hosc.o
> +obj-y += clk-a10-mod1.o
>  obj-y += clk-a10-pll2.o
>  obj-y += clk-a20-gmac.o
>  obj-y += clk-mod0.o
> diff --git a/drivers/clk/sunxi/clk-a10-mod1.c b/drivers/clk/sunxi/clk-a10-mod1.c
> new file mode 100644
> index 000000000000..6fc1b7a6de00
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-a10-mod1.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright 2013 Emilio L?pez
> + *
> + * Emilio L?pez <emilio@elopez.com.ar>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +static DEFINE_SPINLOCK(mod1_lock);
> +
> +#define SUN4I_MOD1_ENABLE      31
> +#define SUN4I_MOD1_MUX         16
> +#define SUN4I_MOD1_MUX_WIDTH   2
> +#define SUN4I_MOD1_MAX_PARENTS 4
> +
> +static void __init sun4i_mod1_clk_setup(struct device_node *node)
> +{
> +       struct clk *clk;
> +       struct clk_mux *mux;
> +       struct clk_gate *gate;
> +       const char *parents[4];
> +       const char *clk_name = node->name;
> +       void __iomem *reg;
> +       int i = 0;
> +
> +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +       if (IS_ERR(reg))
> +               return;
> +
> +       mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +       if (!mux)
> +               goto err_unmap;
> +
> +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +       if (!gate)
> +               goto err_free_mux;
> +
> +       of_property_read_string(node, "clock-output-names", &clk_name);
> +       reg = of_iomap(node, 0);

Is this left over from some change?

ChenYu

> +
> +       while (i < SUN4I_MOD1_MAX_PARENTS &&
> +              (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
> +               i++;
> +
> +       gate->reg = reg;
> +       gate->bit_idx = SUN4I_MOD1_ENABLE;
> +       gate->lock = &mod1_lock;
> +       mux->reg = reg;
> +       mux->shift = SUN4I_MOD1_MUX;
> +       mux->mask = BIT(SUN4I_MOD1_MUX_WIDTH) - 1;
> +       mux->lock = &mod1_lock;
> +
> +       clk = clk_register_composite(NULL, clk_name, parents, i,
> +                                    &mux->hw, &clk_mux_ops,
> +                                    NULL, NULL,
> +                                    &gate->hw, &clk_gate_ops, 0);
> +       if (IS_ERR(clk))
> +               goto err_free_gate;
> +
> +       of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +
> +       return;
> +
> +err_free_gate:
> +       kfree(gate);
> +err_free_mux:
> +       kfree(mux);
> +err_unmap:
> +       iounmap(reg);
> +}
> +CLK_OF_DECLARE(sun4i_mod1, "allwinner,sun4i-a10-mod1-clk",
> +              sun4i_mod1_clk_setup);
> --
> 2.3.6
>

[PATCH 5/8] clk: sunxi: mod1 clock support

[Maxime Ripard]

On Thu, May 14, 2015 at 09:43:19PM +0800, Chen-Yu Tsai wrote:
> On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > From: Emilio L?pez <emilio@elopez.com.ar>
> >
> > The module 1 type of clocks consist of a gate and a mux and are used on
> > the audio blocks to mux and gate the PLL2 outputs for AC97, IIS or
> > SPDIF. This commit adds support for them on the sunxi clock driver.
> >
> > Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/sunxi/Makefile       |  1 +
> >  drivers/clk/sunxi/clk-a10-mod1.c | 85 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 86 insertions(+)
> >  create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c
> >
> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> > index 6fa845e13067..960eeabc375f 100644
> > --- a/drivers/clk/sunxi/Makefile
> > +++ b/drivers/clk/sunxi/Makefile
> > @@ -5,6 +5,7 @@
> >  obj-y += clk-sunxi.o clk-factors.o
> >  obj-y += clk-a10-codec.o
> >  obj-y += clk-a10-hosc.o
> > +obj-y += clk-a10-mod1.o
> >  obj-y += clk-a10-pll2.o
> >  obj-y += clk-a20-gmac.o
> >  obj-y += clk-mod0.o
> > diff --git a/drivers/clk/sunxi/clk-a10-mod1.c b/drivers/clk/sunxi/clk-a10-mod1.c
> > new file mode 100644
> > index 000000000000..6fc1b7a6de00
> > --- /dev/null
> > +++ b/drivers/clk/sunxi/clk-a10-mod1.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + * Copyright 2013 Emilio L?pez
> > + *
> > + * Emilio L?pez <emilio@elopez.com.ar>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/clkdev.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +
> > +static DEFINE_SPINLOCK(mod1_lock);
> > +
> > +#define SUN4I_MOD1_ENABLE      31
> > +#define SUN4I_MOD1_MUX         16
> > +#define SUN4I_MOD1_MUX_WIDTH   2
> > +#define SUN4I_MOD1_MAX_PARENTS 4
> > +
> > +static void __init sun4i_mod1_clk_setup(struct device_node *node)
> > +{
> > +       struct clk *clk;
> > +       struct clk_mux *mux;
> > +       struct clk_gate *gate;
> > +       const char *parents[4];
> > +       const char *clk_name = node->name;
> > +       void __iomem *reg;
> > +       int i = 0;
> > +
> > +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> > +       if (IS_ERR(reg))
> > +               return;
> > +
> > +       mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> > +       if (!mux)
> > +               goto err_unmap;
> > +
> > +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > +       if (!gate)
> > +               goto err_free_mux;
> > +
> > +       of_property_read_string(node, "clock-output-names", &clk_name);
> > +       reg = of_iomap(node, 0);
> 
> Is this left over from some change?

Yes :/

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150515/e2024d88/attachment.sig>

[PATCH 6/8] ARM: sunxi: Add PLL2 support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit adds the PLL2 definition to the sun4i, sun5i and sun7i device
trees. PLL2 is used to clock audio devices.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 9 +++++++++
 arch/arm/boot/dts/sun5i.dtsi     | 9 +++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 1d7fd68bea1d..2ed3a0b43131 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -198,6 +198,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk at 01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk at 01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-pll1-clk";
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 8c04f240f2e9..b291c165966f 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -107,6 +107,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk at 01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk at 01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-pll1-clk";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 4163ade867cb..7c850dc1b197 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -203,6 +203,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk at 01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk at 01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun7i-a20-pll4-clk";
-- 
2.3.6

[PATCH 7/8] ARM: sunxi: Add codec clock support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit adds the codec clock definition to the sun4i, sun5i and
sun7i device trees. The codec clock is used in the analog codec block.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 9 +++++++++
 arch/arm/boot/dts/sun5i.dtsi     | 9 +++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 2ed3a0b43131..f4dead8cde2c 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -50,6 +50,7 @@
 
 #include <dt-bindings/thermal/thermal.h>
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -457,6 +458,14 @@
 			clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
 			clock-output-names = "spi3";
 		};
+
+		codec_clk: clk at 01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
 	};
 
 	/*
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index b291c165966f..38cb77c378ec 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -49,6 +49,7 @@
 
 #include "skeleton.dtsi"
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -298,6 +299,14 @@
 			clock-output-names = "usb_ohci0", "usb_phy";
 		};
 
+		codec_clk: clk at 01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
+
 		mbus_clk: clk at 01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun5i-a13-mbus-clk";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 7c850dc1b197..8a5418a0e795 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -52,6 +52,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/thermal.h>
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -472,6 +473,14 @@
 			clock-output-names = "spi3";
 		};
 
+		codec_clk: clk at 01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
+
 		mbus_clk: clk at 01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun5i-a13-mbus-clk";
-- 
2.3.6

[PATCH 7/8] ARM: sunxi: Add codec clock support

[Chen-Yu Tsai]

On Sat, May 2, 2015 at 7:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> From: Emilio L?pez <emilio@elopez.com.ar>
>
> This commit adds the codec clock definition to the sun4i, sun5i and
> sun7i device trees. The codec clock is used in the analog codec block.
>
> Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

> ---
>  arch/arm/boot/dts/sun4i-a10.dtsi | 9 +++++++++
>  arch/arm/boot/dts/sun5i.dtsi     | 9 +++++++++
>  arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++
>  3 files changed, 27 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 2ed3a0b43131..f4dead8cde2c 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -50,6 +50,7 @@
>
>  #include <dt-bindings/thermal/thermal.h>
>
> +#include <dt-bindings/clock/sun4i-a10-pll2.h>
>  #include <dt-bindings/dma/sun4i-a10.h>
>  #include <dt-bindings/pinctrl/sun4i-a10.h>
>
> @@ -457,6 +458,14 @@
>                         clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
>                         clock-output-names = "spi3";
>                 };
> +
> +               codec_clk: clk at 01c20140 {
> +                       #clock-cells = <0>;
> +                       compatible = "allwinner,sun4i-a10-codec-clk";
> +                       reg = <0x01c20140 0x4>;
> +                       clocks = <&pll2 SUN4I_A10_PLL2_1X>;
> +                       clock-output-names = "codec";
> +               };
>         };
>
>         /*
> diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
> index b291c165966f..38cb77c378ec 100644
> --- a/arch/arm/boot/dts/sun5i.dtsi
> +++ b/arch/arm/boot/dts/sun5i.dtsi
> @@ -49,6 +49,7 @@
>
>  #include "skeleton.dtsi"
>
> +#include <dt-bindings/clock/sun4i-a10-pll2.h>
>  #include <dt-bindings/dma/sun4i-a10.h>
>  #include <dt-bindings/pinctrl/sun4i-a10.h>
>
> @@ -298,6 +299,14 @@
>                         clock-output-names = "usb_ohci0", "usb_phy";
>                 };
>
> +               codec_clk: clk at 01c20140 {
> +                       #clock-cells = <0>;
> +                       compatible = "allwinner,sun4i-a10-codec-clk";
> +                       reg = <0x01c20140 0x4>;
> +                       clocks = <&pll2 SUN4I_A10_PLL2_1X>;
> +                       clock-output-names = "codec";
> +               };
> +
>                 mbus_clk: clk at 01c2015c {
>                         #clock-cells = <0>;
>                         compatible = "allwinner,sun5i-a13-mbus-clk";
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 7c850dc1b197..8a5418a0e795 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -52,6 +52,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/thermal/thermal.h>
>
> +#include <dt-bindings/clock/sun4i-a10-pll2.h>
>  #include <dt-bindings/dma/sun4i-a10.h>
>  #include <dt-bindings/pinctrl/sun4i-a10.h>
>
> @@ -472,6 +473,14 @@
>                         clock-output-names = "spi3";
>                 };
>
> +               codec_clk: clk at 01c20140 {
> +                       #clock-cells = <0>;
> +                       compatible = "allwinner,sun4i-a10-codec-clk";
> +                       reg = <0x01c20140 0x4>;
> +                       clocks = <&pll2 SUN4I_A10_PLL2_1X>;
> +                       clock-output-names = "codec";
> +               };
> +
>                 mbus_clk: clk at 01c2015c {
>                         #clock-cells = <0>;
>                         compatible = "allwinner,sun5i-a13-mbus-clk";
> --
> 2.3.6
>

[PATCH 8/8] ARM: sun7i: Add mod1 clock nodes

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit adds all the mod1 clocks available on A20 to its device
tree. This list was created by looking at the A20 user manual.

Not-signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 55 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 8a5418a0e795..8111458f364b 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -456,6 +456,39 @@
 			clock-output-names = "ir1";
 		};
 
+		iis0_clk: clk at 01c200b8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200b8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis0";
+		};
+
+		ac97_clk: clk at 01c200bc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200bc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "ac97";
+		};
+
+		spdif_clk: clk at 01c200c0 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200c0 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "spdif";
+		};
+
 		usb_clk: clk at 01c200cc {
 			#clock-cells = <1>;
 		        #reset-cells = <1>;
@@ -473,6 +506,28 @@
 			clock-output-names = "spi3";
 		};
 
+		iis1_clk: clk at 01c200d8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200d8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis1";
+		};
+
+		iis2_clk: clk at 01c200dc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200dc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis2";
+		};
+
 		codec_clk: clk at 01c20140 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-codec-clk";
-- 
2.3.6