diff mbox

[1/7] clk: sunxi: Add post clk divider for factor clocks

Message ID 1410000448-9999-2-git-send-email-wens@csie.org (mailing list archive)
State Not Applicable
Headers show

Commit Message

Chen-Yu Tsai Sept. 6, 2014, 10:47 a.m. UTC
Some factor clocks, mostly PLLs, have an extra fixed divider just before
the clock output. Add an option to the factor clk driver config data to
specify this divider.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-factors.c | 3 +++
 drivers/clk/sunxi/clk-factors.h | 1 +
 2 files changed, 4 insertions(+)

Comments

Maxime Ripard Sept. 11, 2014, 8:36 p.m. UTC | #1
On Sat, Sep 06, 2014 at 06:47:22PM +0800, Chen-Yu Tsai wrote:
> Some factor clocks, mostly PLLs, have an extra fixed divider just before
> the clock output. Add an option to the factor clk driver config data to
> specify this divider.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime
Emilio López Sept. 13, 2014, 2:43 p.m. UTC | #2
Hi,

El 06/09/14 a las 07:47, Chen-Yu Tsai escibió:
> Some factor clocks, mostly PLLs, have an extra fixed divider just before
> the clock output. Add an option to the factor clk driver config data to
> specify this divider.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>   drivers/clk/sunxi/clk-factors.c | 3 +++
>   drivers/clk/sunxi/clk-factors.h | 1 +
>   2 files changed, 4 insertions(+)
>
> diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> index 2057c8a..435111d 100644
> --- a/drivers/clk/sunxi/clk-factors.c
> +++ b/drivers/clk/sunxi/clk-factors.c
> @@ -64,6 +64,9 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>   	/* Calculate the rate */
>   	rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
>
> +	if (config->post_div)
> +		rate /= config->post_div;
> +
>   	return rate;
>   }
>
> diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
> index d2d0efa..ce70c65 100644
> --- a/drivers/clk/sunxi/clk-factors.h
> +++ b/drivers/clk/sunxi/clk-factors.h
> @@ -16,6 +16,7 @@ struct clk_factors_config {
>   	u8 pshift;
>   	u8 pwidth;
>   	u8 n_start;
> +	u8 post_div;
>   };
>
>   struct clk_factors {
>

For the record, I liked your solution on[1] more, as it's in line with 
what we're doing on the other sunxi platforms, instead of adding 
features in factors to cover for some cases. But it's your and Maxime's 
call, as I haven't written any of the sun6i code so far.

Cheers!

Emilio

[1] https://patchwork.kernel.org/patch/4228541/
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen-Yu Tsai Sept. 16, 2014, 8:11 a.m. UTC | #3
Hi,

On Sat, Sep 13, 2014 at 10:43 PM, Emilio López <emilio@elopez.com.ar> wrote:
> Hi,
>
> El 06/09/14 a las 07:47, Chen-Yu Tsai escibió:
>
>> Some factor clocks, mostly PLLs, have an extra fixed divider just before
>> the clock output. Add an option to the factor clk driver config data to
>> specify this divider.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>   drivers/clk/sunxi/clk-factors.c | 3 +++
>>   drivers/clk/sunxi/clk-factors.h | 1 +
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/clk/sunxi/clk-factors.c
>> b/drivers/clk/sunxi/clk-factors.c
>> index 2057c8a..435111d 100644
>> --- a/drivers/clk/sunxi/clk-factors.c
>> +++ b/drivers/clk/sunxi/clk-factors.c
>> @@ -64,6 +64,9 @@ static unsigned long clk_factors_recalc_rate(struct
>> clk_hw *hw,
>>         /* Calculate the rate */
>>         rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m +
>> 1);
>>
>> +       if (config->post_div)
>> +               rate /= config->post_div;
>> +
>>         return rate;
>>   }
>>
>> diff --git a/drivers/clk/sunxi/clk-factors.h
>> b/drivers/clk/sunxi/clk-factors.h
>> index d2d0efa..ce70c65 100644
>> --- a/drivers/clk/sunxi/clk-factors.h
>> +++ b/drivers/clk/sunxi/clk-factors.h
>> @@ -16,6 +16,7 @@ struct clk_factors_config {
>>         u8 pshift;
>>         u8 pwidth;
>>         u8 n_start;
>> +       u8 post_div;
>>   };
>>
>>   struct clk_factors {
>>
>
> For the record, I liked your solution on[1] more, as it's in line with what
> we're doing on the other sunxi platforms, instead of adding features in
> factors to cover for some cases. But it's your and Maxime's call, as I
> haven't written any of the sun6i code so far.

I'm OK either way. It's really up to Maxime whether he likes the DT
representation.

> Cheers!
>
> Emilio
>
> [1] https://patchwork.kernel.org/patch/4228541/
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard Sept. 16, 2014, 3:57 p.m. UTC | #4
Hi Emilio,

On Sat, Sep 13, 2014 at 11:43:46AM -0300, Emilio López wrote:
> Hi,
> 
> El 06/09/14 a las 07:47, Chen-Yu Tsai escibió:
> >Some factor clocks, mostly PLLs, have an extra fixed divider just before
> >the clock output. Add an option to the factor clk driver config data to
> >specify this divider.
> >
> >Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >---
> >  drivers/clk/sunxi/clk-factors.c | 3 +++
> >  drivers/clk/sunxi/clk-factors.h | 1 +
> >  2 files changed, 4 insertions(+)
> >
> >diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> >index 2057c8a..435111d 100644
> >--- a/drivers/clk/sunxi/clk-factors.c
> >+++ b/drivers/clk/sunxi/clk-factors.c
> >@@ -64,6 +64,9 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
> >  	/* Calculate the rate */
> >  	rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
> >
> >+	if (config->post_div)
> >+		rate /= config->post_div;
> >+
> >  	return rate;
> >  }
> >
> >diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
> >index d2d0efa..ce70c65 100644
> >--- a/drivers/clk/sunxi/clk-factors.h
> >+++ b/drivers/clk/sunxi/clk-factors.h
> >@@ -16,6 +16,7 @@ struct clk_factors_config {
> >  	u8 pshift;
> >  	u8 pwidth;
> >  	u8 n_start;
> >+	u8 post_div;
> >  };
> >
> >  struct clk_factors {
> >
> 
> For the record, I liked your solution on[1] more, as it's in line
> with what we're doing on the other sunxi platforms, instead of
> adding features in factors to cover for some cases. But it's your
> and Maxime's call, as I haven't written any of the sun6i code so
> far.

No, you still wrote most of the clock support, so your opinion is
always valuable (and valued).

Thing is, unlike what was done in the sun4i driver where there was a
"real" technical issue that was preventing us from using only
fixed-factor, we're not in such a case in sun6i (and later,
apparently).

PLL6 has only one output, which is then directly multiplied by
fixed-factors, without any (pre|post)-dividers for any of them.

That means that following what you did for the sun4i would just
register 3 "dumbs" fixed-factors, that we couldn't reference from DT,
or through a cryptic index (which is not even documented in our
bindings).

I'd be fine either way, I just prefer the solution that has less code
and is more explicit.

Maxime
Chen-Yu Tsai Sept. 24, 2014, 3:35 p.m. UTC | #5
Hi Maxime, Emilio,

On Tue, Sep 16, 2014 at 11:57 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Emilio,
>
> On Sat, Sep 13, 2014 at 11:43:46AM -0300, Emilio López wrote:
>> Hi,
>>
>> El 06/09/14 a las 07:47, Chen-Yu Tsai escibió:
>> >Some factor clocks, mostly PLLs, have an extra fixed divider just before
>> >the clock output. Add an option to the factor clk driver config data to
>> >specify this divider.
>> >
>> >Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> >---
>> >  drivers/clk/sunxi/clk-factors.c | 3 +++
>> >  drivers/clk/sunxi/clk-factors.h | 1 +
>> >  2 files changed, 4 insertions(+)
>> >
>> >diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
>> >index 2057c8a..435111d 100644
>> >--- a/drivers/clk/sunxi/clk-factors.c
>> >+++ b/drivers/clk/sunxi/clk-factors.c
>> >@@ -64,6 +64,9 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>> >     /* Calculate the rate */
>> >     rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
>> >
>> >+    if (config->post_div)
>> >+            rate /= config->post_div;
>> >+
>> >     return rate;
>> >  }
>> >
>> >diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
>> >index d2d0efa..ce70c65 100644
>> >--- a/drivers/clk/sunxi/clk-factors.h
>> >+++ b/drivers/clk/sunxi/clk-factors.h
>> >@@ -16,6 +16,7 @@ struct clk_factors_config {
>> >     u8 pshift;
>> >     u8 pwidth;
>> >     u8 n_start;
>> >+    u8 post_div;
>> >  };
>> >
>> >  struct clk_factors {
>> >
>>
>> For the record, I liked your solution on[1] more, as it's in line
>> with what we're doing on the other sunxi platforms, instead of
>> adding features in factors to cover for some cases. But it's your
>> and Maxime's call, as I haven't written any of the sun6i code so
>> far.
>
> No, you still wrote most of the clock support, so your opinion is
> always valuable (and valued).
>
> Thing is, unlike what was done in the sun4i driver where there was a
> "real" technical issue that was preventing us from using only
> fixed-factor, we're not in such a case in sun6i (and later,
> apparently).
>
> PLL6 has only one output, which is then directly multiplied by
> fixed-factors, without any (pre|post)-dividers for any of them.
>
> That means that following what you did for the sun4i would just
> register 3 "dumbs" fixed-factors, that we couldn't reference from DT,
> or through a cryptic index (which is not even documented in our
> bindings).
>
> I'd be fine either way, I just prefer the solution that has less code
> and is more explicit.

What's the verdict on this series?

ChenYu
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard Sept. 27, 2014, 7:07 a.m. UTC | #6
On Wed, Sep 24, 2014 at 11:35:58PM +0800, Chen-Yu Tsai wrote:
> Hi Maxime, Emilio,
> 
> On Tue, Sep 16, 2014 at 11:57 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi Emilio,
> >
> > On Sat, Sep 13, 2014 at 11:43:46AM -0300, Emilio López wrote:
> >> Hi,
> >>
> >> El 06/09/14 a las 07:47, Chen-Yu Tsai escibió:
> >> >Some factor clocks, mostly PLLs, have an extra fixed divider just before
> >> >the clock output. Add an option to the factor clk driver config data to
> >> >specify this divider.
> >> >
> >> >Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> >---
> >> >  drivers/clk/sunxi/clk-factors.c | 3 +++
> >> >  drivers/clk/sunxi/clk-factors.h | 1 +
> >> >  2 files changed, 4 insertions(+)
> >> >
> >> >diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> >> >index 2057c8a..435111d 100644
> >> >--- a/drivers/clk/sunxi/clk-factors.c
> >> >+++ b/drivers/clk/sunxi/clk-factors.c
> >> >@@ -64,6 +64,9 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
> >> >     /* Calculate the rate */
> >> >     rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
> >> >
> >> >+    if (config->post_div)
> >> >+            rate /= config->post_div;
> >> >+
> >> >     return rate;
> >> >  }
> >> >
> >> >diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
> >> >index d2d0efa..ce70c65 100644
> >> >--- a/drivers/clk/sunxi/clk-factors.h
> >> >+++ b/drivers/clk/sunxi/clk-factors.h
> >> >@@ -16,6 +16,7 @@ struct clk_factors_config {
> >> >     u8 pshift;
> >> >     u8 pwidth;
> >> >     u8 n_start;
> >> >+    u8 post_div;
> >> >  };
> >> >
> >> >  struct clk_factors {
> >> >
> >>
> >> For the record, I liked your solution on[1] more, as it's in line
> >> with what we're doing on the other sunxi platforms, instead of
> >> adding features in factors to cover for some cases. But it's your
> >> and Maxime's call, as I haven't written any of the sun6i code so
> >> far.
> >
> > No, you still wrote most of the clock support, so your opinion is
> > always valuable (and valued).
> >
> > Thing is, unlike what was done in the sun4i driver where there was a
> > "real" technical issue that was preventing us from using only
> > fixed-factor, we're not in such a case in sun6i (and later,
> > apparently).
> >
> > PLL6 has only one output, which is then directly multiplied by
> > fixed-factors, without any (pre|post)-dividers for any of them.
> >
> > That means that following what you did for the sun4i would just
> > register 3 "dumbs" fixed-factors, that we couldn't reference from DT,
> > or through a cryptic index (which is not even documented in our
> > bindings).
> >
> > I'd be fine either way, I just prefer the solution that has less code
> > and is more explicit.
> 
> What's the verdict on this series?

If Emilio prefers to have a single clock node, fine. But please update
the DT bindings documentation.

Maxime
Chen-Yu Tsai Sept. 27, 2014, 7:23 a.m. UTC | #7
On Sat, Sep 27, 2014 at 3:07 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Wed, Sep 24, 2014 at 11:35:58PM +0800, Chen-Yu Tsai wrote:
>> Hi Maxime, Emilio,
>>
>> On Tue, Sep 16, 2014 at 11:57 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Hi Emilio,
>> >
>> > On Sat, Sep 13, 2014 at 11:43:46AM -0300, Emilio López wrote:
>> >> Hi,
>> >>
>> >> El 06/09/14 a las 07:47, Chen-Yu Tsai escibió:
>> >> >Some factor clocks, mostly PLLs, have an extra fixed divider just before
>> >> >the clock output. Add an option to the factor clk driver config data to
>> >> >specify this divider.
>> >> >
>> >> >Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> >> >---
>> >> >  drivers/clk/sunxi/clk-factors.c | 3 +++
>> >> >  drivers/clk/sunxi/clk-factors.h | 1 +
>> >> >  2 files changed, 4 insertions(+)
>> >> >
>> >> >diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
>> >> >index 2057c8a..435111d 100644
>> >> >--- a/drivers/clk/sunxi/clk-factors.c
>> >> >+++ b/drivers/clk/sunxi/clk-factors.c
>> >> >@@ -64,6 +64,9 @@ static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
>> >> >     /* Calculate the rate */
>> >> >     rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
>> >> >
>> >> >+    if (config->post_div)
>> >> >+            rate /= config->post_div;
>> >> >+
>> >> >     return rate;
>> >> >  }
>> >> >
>> >> >diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
>> >> >index d2d0efa..ce70c65 100644
>> >> >--- a/drivers/clk/sunxi/clk-factors.h
>> >> >+++ b/drivers/clk/sunxi/clk-factors.h
>> >> >@@ -16,6 +16,7 @@ struct clk_factors_config {
>> >> >     u8 pshift;
>> >> >     u8 pwidth;
>> >> >     u8 n_start;
>> >> >+    u8 post_div;
>> >> >  };
>> >> >
>> >> >  struct clk_factors {
>> >> >
>> >>
>> >> For the record, I liked your solution on[1] more, as it's in line
>> >> with what we're doing on the other sunxi platforms, instead of
>> >> adding features in factors to cover for some cases. But it's your
>> >> and Maxime's call, as I haven't written any of the sun6i code so
>> >> far.
>> >
>> > No, you still wrote most of the clock support, so your opinion is
>> > always valuable (and valued).
>> >
>> > Thing is, unlike what was done in the sun4i driver where there was a
>> > "real" technical issue that was preventing us from using only
>> > fixed-factor, we're not in such a case in sun6i (and later,
>> > apparently).
>> >
>> > PLL6 has only one output, which is then directly multiplied by
>> > fixed-factors, without any (pre|post)-dividers for any of them.
>> >
>> > That means that following what you did for the sun4i would just
>> > register 3 "dumbs" fixed-factors, that we couldn't reference from DT,
>> > or through a cryptic index (which is not even documented in our
>> > bindings).
>> >
>> > I'd be fine either way, I just prefer the solution that has less code
>> > and is more explicit.
>>
>> What's the verdict on this series?
>
> If Emilio prefers to have a single clock node, fine. But please update
> the DT bindings documentation.

OK. Then this series needs a bit of work. I'll pick the orignal
divs clocks patch from patchwork (as I didn't keep one) and fix
up the DT.


ChenYu
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index 2057c8a..435111d 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -64,6 +64,9 @@  static unsigned long clk_factors_recalc_rate(struct clk_hw *hw,
 	/* Calculate the rate */
 	rate = (parent_rate * (n + config->n_start) * (k + 1) >> p) / (m + 1);
 
+	if (config->post_div)
+		rate /= config->post_div;
+
 	return rate;
 }
 
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index d2d0efa..ce70c65 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -16,6 +16,7 @@  struct clk_factors_config {
 	u8 pshift;
 	u8 pwidth;
 	u8 n_start;
+	u8 post_div;
 };
 
 struct clk_factors {