[RFC V1 0/8] imx5 clock port to Mike's clkv3

[RFC V1 0/8] imx5 clock port to Mike's clkv3

[Richard Zhao]

This work took Sascha's common clk work as a start, and port to Mike's
generic clk v3 patch.

clock tree topology:
.
??? ckih1
??? ??? ssi_lp_apm
??? ckih2
??? ckil
??? dummy
??? ??? emi_fast_gate
??? osc
    ??? lp_apm
    ??? ??? periph_apm
    ??? ??? step_clk
    ??? pll1
    ??? ??? pll1_sw
    ???     ??? cpu_podf
    ??? pll2
    ??? ??? pll2_sw
    ??? ??? ??? esdhc1_sel
    ??? ??? ??? ??? esdhc1_pred
    ??? ??? ???     ??? esdhc1_podf
    ??? ??? ???         ??? esdhc1_per_gate
    ??? ??? ???         ??? esdhc3_sel
    ??? ??? ???         ??? ??? esdhc2_per_gate
    ??? ??? ???         ??? esdhc4_sel
    ??? ??? ???             ??? esdhc4_per_gate
    ??? ??? ??? esdhc2_sel
    ??? ??? ??? ??? esdhc2_pred
    ??? ??? ???     ??? esdhc2_podf
    ??? ??? ???         ??? esdhc3_per_gate
    ??? ??? ??? main_bus
    ??? ??? ??? ??? ahb_root
    ??? ??? ??? ??? ??? ipg
    ??? ??? ??? ???     ??? ahb_max
    ??? ??? ??? ???     ??? ahbmux1
    ??? ??? ??? ???     ??? aips_tz1
    ??? ??? ??? ???     ??? aips_tz2
    ??? ??? ??? ???     ??? cspi_ipg_gate
    ??? ??? ??? ???     ??? ecspi1_ipg_gate
    ??? ??? ??? ???     ??? ecspi2_ipg_gate
    ??? ??? ??? ???     ??? esdhc1_ipg_gate
    ??? ??? ??? ???     ??? esdhc2_ipg_gate
    ??? ??? ??? ???     ??? esdhc3_ipg_gate
    ??? ??? ??? ???     ??? esdhc4_ipg_gate
    ??? ??? ??? ???     ??? fec_gate
    ??? ??? ??? ???     ??? gpt_gate
    ??? ??? ??? ???     ??? gpt_ipg_gate
    ??? ??? ??? ???     ??? iim_gate
    ??? ??? ??? ???     ??? pwm1_ipg_gate
    ??? ??? ??? ???     ??? pwm2_ipg_gate
    ??? ??? ??? ???     ??? sdma_gate
    ??? ??? ??? ???     ??? ssi1_ipg_gate
    ??? ??? ??? ???     ??? ssi2_ipg_gate
    ??? ??? ??? ???     ??? ssi3_ipg_gate
    ??? ??? ??? ???     ??? uart1_ipg_gate
    ??? ??? ??? ???     ??? uart2_ipg_gate
    ??? ??? ??? ???     ??? uart3_ipg_gate
    ??? ??? ??? ???     ??? usboh3_ahb_gate
    ??? ??? ??? ??? axi_a
    ??? ??? ??? ??? ??? ddr_root
    ??? ??? ??? ??? axi_b
    ??? ??? ??? ??? ??? arm_axi
    ??? ??? ??? ??? ??? gpu
    ??? ??? ??? ??? ??? gpu2d
    ??? ??? ??? ??? ??? ipu_hsp
    ??? ??? ??? ??? ??? ??? ipu_gate
    ??? ??? ??? ??? ??? vpu_axi_root
    ??? ??? ??? ??? emi_sel
    ??? ??? ??? ??? ??? emi_slow_podf
    ??? ??? ??? ???     ??? emi_slow_gate
    ??? ??? ??? ???     ??? nfc_podf
    ??? ??? ??? ???         ??? nfc_gate
    ??? ??? ??? ??? perclk_lp_apm
    ??? ??? ???     ??? perclk_pred1
    ??? ??? ???         ??? perclk_pred2
    ??? ??? ???             ??? perclk_podf
    ??? ??? ???                 ??? ipg_perclk
    ??? ??? ???                     ??? i2c1_gate
    ??? ??? ???                     ??? i2c2_gate
    ??? ??? ???                     ??? pwm1_hf_gate
    ??? ??? ???                     ??? pwm2_hf_gate
    ??? ??? ??? uart_sel
    ??? ???     ??? uart_pred
    ??? ???         ??? uart_root
    ??? ???             ??? uart1_per_gate
    ??? ???             ??? uart2_per_gate
    ??? ???             ??? uart3_per_gate
    ??? ??? step_pll2_div
    ??? pll3
    ??? ??? pll3_sw
    ??? ??? ??? ecspi_sel
    ??? ??? ??? ??? ecspi_pred
    ??? ??? ???     ??? ecspi_podf
    ??? ??? ???         ??? ecspi1_per_gate
    ??? ??? ???         ??? ecspi2_per_gate
    ??? ??? ??? ipu_di0_sel
    ??? ??? ??? ??? ipu_di0_gate
    ??? ??? ??? ipu_di1_sel
    ??? ??? ??? ??? ipu_di1_gate
    ??? ??? ??? ssi1_clk_sel
    ??? ??? ??? ??? ssi1_clk_pred
    ??? ??? ???     ??? ssi1_clk
    ??? ??? ???         ??? ssi1_gate
    ??? ??? ???         ??? ssi3_clk
    ??? ??? ???             ??? ssi3_gate
    ??? ??? ??? ssi2_clk_sel
    ??? ??? ??? ??? ssi2_clk_pred
    ??? ??? ???     ??? ssi2_clk
    ??? ??? ???         ??? ssi2_gate
    ??? ??? ??? usboh3_sel
    ??? ??? ??? ??? usboh3_pred
    ??? ??? ???     ??? usboh3_podf
    ??? ??? ???         ??? usboh3_gate
    ??? ??? ??? usb_phy_pred
    ??? ???     ??? usb_phy_podf
    ??? ??? step_pll3_div
    ??? pll4
    ??? ??? pll4_sw
    ???     ??? tve_ext_sel
    ???         ??? tve_pred
    ???             ??? tve_gate
    ???             ??? tve_sel
    ??? usb_phy_sel
        ??? mx53_usb_phy1_gate
        ??? mx53_usb_phy2_gate


Thanks
Richard

[RFC V1 1/8] clk: support static parent

[Mike Turquette]

Hi Richard,

Thanks for porting your platform so quickly!

On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
> ?drivers/clk/clk.c | ? 10 ++++------
> ?1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 85dabdb31..ed557c9 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -519,13 +519,11 @@ void clk_init(struct device *dev, struct clk *clk)
>
> ? ? ? ?mutex_lock(&prepare_lock);
>
> - ? ? ? if (clk->ops->get_parent) {
> + ? ? ? if (clk->ops->get_parent)
> ? ? ? ? ? ? ? ?clk->parent = clk->ops->get_parent(clk);
> - ? ? ? ? ? ? ? if (clk->parent)
> - ? ? ? ? ? ? ? ? ? ? ? hlist_add_head(&clk->child_node,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &clk->parent->children);
> - ? ? ? } else
> - ? ? ? ? ? ? ? clk->parent = NULL;
> + ? ? ? if (clk->parent)
> + ? ? ? ? ? ? ? hlist_add_head(&clk->child_node,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &clk->parent->children);

I'll have to NACK this.  I don't think we should trust .parent in
struct clk for initialization.

Rather, if your clk has a fixed parent then I would prefer for you to
put that parent inside your struct clk_hw_whatever and have your
.get_parent callback return clk_hw_whatever->fixed_parent.

The reason for this is that some folks with mux clks might be tempted
to populate .parent statically, but maybe the bootloader changed it
and now it can't be trusted.  I'd prefer to rule out this possibility
entirely by always relying on a .get_parent function.

Check out the way that the simple fixed-rate clk and simple gateable
clk does this in drivers/clk/clk-basic.c

Regards,
Mike

>
> ? ? ? ?if (clk->ops->recalc_rate)
> ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> --
> 1.7.5.4
>
>
>

[RFC V1 1/8] clk: support static parent

[Richard Zhao]

On Wed, Nov 23, 2011 at 02:11:40PM -0800, Mike Turquette wrote:
> Hi Richard,
> 
> Thanks for porting your platform so quickly!
My pleasure!
> 
> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> > ?drivers/clk/clk.c | ? 10 ++++------
> > ?1 files changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 85dabdb31..ed557c9 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -519,13 +519,11 @@ void clk_init(struct device *dev, struct clk *clk)
> >
> > ? ? ? ?mutex_lock(&prepare_lock);
> >
> > - ? ? ? if (clk->ops->get_parent) {
> > + ? ? ? if (clk->ops->get_parent)
> > ? ? ? ? ? ? ? ?clk->parent = clk->ops->get_parent(clk);
If it has .get_parent, we use .get_parent to get parent.
> > - ? ? ? ? ? ? ? if (clk->parent)
> > - ? ? ? ? ? ? ? ? ? ? ? hlist_add_head(&clk->child_node,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &clk->parent->children);
> > - ? ? ? } else
> > - ? ? ? ? ? ? ? clk->parent = NULL;
> > + ? ? ? if (clk->parent)
I assume .parent is set to zero when struct clk alloc memory. So the
value .parent is either get from .get_parent or a fixed parent.
> > + ? ? ? ? ? ? ? hlist_add_head(&clk->child_node,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &clk->parent->children);
> 
> I'll have to NACK this.  I don't think we should trust .parent in
> struct clk for initialization.
> 
> Rather, if your clk has a fixed parent then I would prefer for you to
> put that parent inside your struct clk_hw_whatever and have your
> .get_parent callback return clk_hw_whatever->fixed_parent.
Yes, It's what I did on your v2 patch, which struct clk is allocated by
the framework. But now we don't have to repeat the work clk by clk.
> 
> The reason for this is that some folks with mux clks might be tempted
> to populate .parent statically, but maybe the bootloader changed it
> and now it can't be trusted.  I'd prefer to rule out this possibility
> entirely by always relying on a .get_parent function.
This patch didn't remove .get_parent call. We trust .get_parent most.
If .get_parent must not be NULL, clocks without mux have to duplicate
the redundant get_parent function.

Thanks
Richard
> 
> Check out the way that the simple fixed-rate clk and simple gateable
> clk does this in drivers/clk/clk-basic.c
> 
> Regards,
> Mike
> 
> >
> > ? ? ? ?if (clk->ops->recalc_rate)
> > ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> > --
> > 1.7.5.4
> >
> >
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 1/8] clk: support static parent

[Mike Turquette]

On Wed, Nov 23, 2011 at 4:30 PM, Richard Zhao <richard.zhao@linaro.org> wrote:
> On Wed, Nov 23, 2011 at 02:11:40PM -0800, Mike Turquette wrote:
>> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> This patch didn't remove .get_parent call. We trust .get_parent most.
> If .get_parent must not be NULL, clocks without mux have to duplicate
> the redundant get_parent function.

I've taken this in for V4 to accommodate DT clk binding support.  I've
put a comment in clk_init concerning use of .get_parent versus
statically initializing .parent.

Thanks,
Mike

>
> Thanks
> Richard
>>
>> Check out the way that the simple fixed-rate clk and simple gateable
>> clk does this in drivers/clk/clk-basic.c
>>
>> Regards,
>> Mike
>>
>> >
>> > ? ? ? ?if (clk->ops->recalc_rate)
>> > ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
>> > --
>> > 1.7.5.4
>> >
>> >
>> >
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
>

[RFC V1 1/8] clk: support static parent

[Richard Zhao]

On Wed, Nov 30, 2011 at 12:41:05PM -0800, Mike Turquette wrote:
> On Wed, Nov 23, 2011 at 4:30 PM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > On Wed, Nov 23, 2011 at 02:11:40PM -0800, Mike Turquette wrote:
> >> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > This patch didn't remove .get_parent call. We trust .get_parent most.
> > If .get_parent must not be NULL, clocks without mux have to duplicate
> > the redundant get_parent function.
> 
> I've taken this in for V4 to accommodate DT clk binding support.  I've
> put a comment in clk_init concerning use of .get_parent versus
> statically initializing .parent.
Thanks.
I also did some basic work for imx5 clk dts. I wrote a small tool to convert
my patch to dts. I'll continue to do it once your send out v4 patch.

Richard
> 
> Thanks,
> Mike
> 
> >
> > Thanks
> > Richard
> >>
> >> Check out the way that the simple fixed-rate clk and simple gateable
> >> clk does this in drivers/clk/clk-basic.c
> >>
> >> Regards,
> >> Mike
> >>
> >> >
> >> > ? ? ? ?if (clk->ops->recalc_rate)
> >> > ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);

[RFC V1 2/8] clk: pass parent rate if recalc_rate is NULL

[Mike Turquette]

On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
> ?drivers/clk/clk.c | ? ?4 ++++
> ?1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed557c9..2d8422f 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -256,6 +256,8 @@ static void clk_recalc_rates(struct clk *clk)
>
> ? ? ? ?if (clk->ops->recalc_rate)
> ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> + ? ? ? else if (clk->parent)
> + ? ? ? ? ? ? ? clk->rate = clk->parent->rate;

My code at Linaro Connect did this but Saravana expressed that this
was not a safe assumption to make.  I suppose it depends on how many
clks you want to model, but don't provide good support with
clk_hw_ops.

I'm inclined to leave it out if it is making assumptions that work for
one platform but not another.  Saravana can you clarify if I
understood you correctly?

Regards,
Mike

>
> ? ? ? ?if (old_rate == clk->rate)
> ? ? ? ? ? ? ? ?return;
> @@ -527,6 +529,8 @@ void clk_init(struct device *dev, struct clk *clk)
>
> ? ? ? ?if (clk->ops->recalc_rate)
> ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> + ? ? ? else if (clk->parent)
> + ? ? ? ? ? ? ? clk->rate = clk->parent->rate;
> ? ? ? ?else
> ? ? ? ? ? ? ? ?clk->rate = 0;
>
> --
> 1.7.5.4
>
>
>

[RFC V1 2/8] clk: pass parent rate if recalc_rate is NULL

[Richard Zhao]

On Wed, Nov 23, 2011 at 02:17:50PM -0800, Mike Turquette wrote:
> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> > ?drivers/clk/clk.c | ? ?4 ++++
> > ?1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index ed557c9..2d8422f 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -256,6 +256,8 @@ static void clk_recalc_rates(struct clk *clk)
> >
> > ? ? ? ?if (clk->ops->recalc_rate)
> > ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> > + ? ? ? else if (clk->parent)
> > + ? ? ? ? ? ? ? clk->rate = clk->parent->rate;
> 
> My code at Linaro Connect did this but Saravana expressed that this
> was not a safe assumption to make.  I suppose it depends on how many
> clks you want to model, but don't provide good support with
> clk_hw_ops.
clk A --\              -- clk A1
          Shared Gate /
clk B --/             \-- clk B1
A.rate == A1.rate && B.rate == B1.rate is true but A.rate != B.rate

It's the only case I can find out not to pass parent rate. But I don't
think we need to support it at first stage.

Saravana, is it what you mean?

Thanks
Richard
> 
> I'm inclined to leave it out if it is making assumptions that work for
> one platform but not another.  Saravana can you clarify if I
> understood you correctly?
> 
> Regards,
> Mike
> 
> >
> > ? ? ? ?if (old_rate == clk->rate)
> > ? ? ? ? ? ? ? ?return;
> > @@ -527,6 +529,8 @@ void clk_init(struct device *dev, struct clk *clk)
> >
> > ? ? ? ?if (clk->ops->recalc_rate)
> > ? ? ? ? ? ? ? ?clk->rate = clk->ops->recalc_rate(clk);
> > + ? ? ? else if (clk->parent)
> > + ? ? ? ? ? ? ? clk->rate = clk->parent->rate;
> > ? ? ? ?else
> > ? ? ? ? ? ? ? ?clk->rate = 0;
> >
> > --
> > 1.7.5.4
> >
> >
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 0/8] imx5 clock port to Mike's clkv3

[Shawn Guo]

On Wed, Nov 23, 2011 at 07:12:27PM +0800, Richard Zhao wrote:
> This work took Sascha's common clk work as a start, and port to Mike's
> generic clk v3 patch.
> 
It seems that the patches did not reach the mailing list but only
this cover letter.

-- 
Regards,
Shawn

[RFC V1 0/8] imx5 clock port to Mike's clkv3

[Richard Zhao]

On Thu, Nov 24, 2011 at 01:16:26PM +0800, Shawn Guo wrote:
> On Wed, Nov 23, 2011 at 07:12:27PM +0800, Richard Zhao wrote:
> > This work took Sascha's common clk work as a start, and port to Mike's
> > generic clk v3 patch.
> > 
> It seems that the patches did not reach the mailing list but only
> this cover letter.
It seems they're blocked.

quote:
Your mail to 'linux-arm-kernel' with the subject

   [RFC V1 2/8] clk: pass parent rate if recalc_rate is NULL

Is being held until the list moderator can review it for approval.

The reason it is being held:

   Message has a suspicious header

Thanks
Richard
> 
> -- 
> Regards,
> Shawn
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Mike Turquette]

On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> +#define DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
> + ? ? ? struct clk_divider _name = { \
> + ? ? ? ? ? ? ? .clk = { \
> + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_divider_ops, \
> + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
> + ? ? ? ? ? ? ? ? ? ? ? .flags = _flags, \
> + ? ? ? ? ? ? ? }, \
> + ? ? ? ? ? ? ? .reg = (_reg), \
> + ? ? ? ? ? ? ? .shift = (_shift), \
> + ? ? ? ? ? ? ? .width = (_width), \
> + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> + ? ? ? }
> +
> +#define DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
> + ? ? ? struct clk_mux _name = { \
> + ? ? ? ? ? ? ? .clk = { \
> + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_mux_ops, \
> + ? ? ? ? ? ? ? }, \
> + ? ? ? ? ? ? ? .reg = (_reg), \
> + ? ? ? ? ? ? ? .shift = (_shift), \
> + ? ? ? ? ? ? ? .width = (_width), \
> + ? ? ? ? ? ? ? .clks = (_clks), \
> + ? ? ? ? ? ? ? .num_clks = ARRAY_SIZE(_clks), \
> + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> + ? ? ? }
> +
> +#define DEFINE_CLK_FIXED(_name, _rate) \
> + ? ? ? struct clk_hw_fixed _name = { \
> + ? ? ? ? ? ? ? .clk = { \
> + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_hw_fixed_ops, \
> + ? ? ? ? ? ? ? }, \
> + ? ? ? ? ? ? ? .fixed_rate = (_rate), \
> + ? ? ? }
> +
> ?#endif /* CONFIG_GENERIC_CLK */
> ?#endif /* __ASSEMBLY__ */
> ?#endif /* __ASM_ARCH_MXC_CLOCK_H__ */
> --
> 1.7.5.4
>
>
>

Can you move these into clk.h/approriate header?  Just add _lock to it
and it'll work fine for your code.

Thanks,
Mike

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Richard Zhao]

On Mon, Nov 28, 2011 at 06:36:33PM -0800, Mike Turquette wrote:
> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > +#define DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
> > + ? ? ? struct clk_divider _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_divider_ops, \
> > + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
> > + ? ? ? ? ? ? ? ? ? ? ? .flags = _flags, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .reg = (_reg), \
> > + ? ? ? ? ? ? ? .shift = (_shift), \
> > + ? ? ? ? ? ? ? .width = (_width), \
> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> > + ? ? ? }
> > +
> > +#define DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
> > + ? ? ? struct clk_mux _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_mux_ops, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .reg = (_reg), \
> > + ? ? ? ? ? ? ? .shift = (_shift), \
> > + ? ? ? ? ? ? ? .width = (_width), \
> > + ? ? ? ? ? ? ? .clks = (_clks), \
> > + ? ? ? ? ? ? ? .num_clks = ARRAY_SIZE(_clks), \
> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> > + ? ? ? }
> > +
> > +#define DEFINE_CLK_FIXED(_name, _rate) \
> > + ? ? ? struct clk_hw_fixed _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_hw_fixed_ops, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .fixed_rate = (_rate), \
> > + ? ? ? }
> > +
> > ?#endif /* CONFIG_GENERIC_CLK */
> > ?#endif /* __ASSEMBLY__ */
> > ?#endif /* __ASM_ARCH_MXC_CLOCK_H__ */
> > --
> > 1.7.5.4
> >
> >
> >
> 
> Can you move these into clk.h/approriate header?  Just add _lock to it
> and it'll work fine for your code.
Yes, I can add _lock and share the macros in common header. But I still
need a wraper for imx to eliminate _lock, right?

Thanks for review
Richard
> 
> Thanks,
> Mike
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Mike Turquette]

On Mon, Nov 28, 2011 at 7:09 PM, Richard Zhao <richard.zhao@linaro.org> wrote:
> On Mon, Nov 28, 2011 at 06:36:33PM -0800, Mike Turquette wrote:
>> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
>> > +#define DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
>> > + ? ? ? struct clk_divider _name = { \
>> > + ? ? ? ? ? ? ? .clk = { \
>> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
>> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_divider_ops, \
>> > + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
>> > + ? ? ? ? ? ? ? ? ? ? ? .flags = _flags, \
>> > + ? ? ? ? ? ? ? }, \
>> > + ? ? ? ? ? ? ? .reg = (_reg), \
>> > + ? ? ? ? ? ? ? .shift = (_shift), \
>> > + ? ? ? ? ? ? ? .width = (_width), \
>> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
>> > + ? ? ? }
>> > +
>> > +#define DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
>> > + ? ? ? struct clk_mux _name = { \
>> > + ? ? ? ? ? ? ? .clk = { \
>> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
>> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_mux_ops, \
>> > + ? ? ? ? ? ? ? }, \
>> > + ? ? ? ? ? ? ? .reg = (_reg), \
>> > + ? ? ? ? ? ? ? .shift = (_shift), \
>> > + ? ? ? ? ? ? ? .width = (_width), \
>> > + ? ? ? ? ? ? ? .clks = (_clks), \
>> > + ? ? ? ? ? ? ? .num_clks = ARRAY_SIZE(_clks), \
>> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
>> > + ? ? ? }
>> > +
>> > +#define DEFINE_CLK_FIXED(_name, _rate) \
>> > + ? ? ? struct clk_hw_fixed _name = { \
>> > + ? ? ? ? ? ? ? .clk = { \
>> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
>> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_hw_fixed_ops, \
>> > + ? ? ? ? ? ? ? }, \
>> > + ? ? ? ? ? ? ? .fixed_rate = (_rate), \
>> > + ? ? ? }
>> > +
>> > ?#endif /* CONFIG_GENERIC_CLK */
>> > ?#endif /* __ASSEMBLY__ */
>> > ?#endif /* __ASM_ARCH_MXC_CLOCK_H__ */
>> > --
>> > 1.7.5.4
>> >
>> >
>> >
>>
>> Can you move these into clk.h/approriate header? ?Just add _lock to it
>> and it'll work fine for your code.
> Yes, I can add _lock and share the macros in common header. But I still

On second thought you don't need to resubmit with these in clk.h; I'll
absorb them into clk.h with my next common clk patchset.

> need a wraper for imx to eliminate _lock, right?

No.  Taking code from your patch 8/8:

+static DEFINE_CLK_DIVIDER(perclk_pred1, &perclk_lp_apm.clk, 0,
MXC_CCM_CBCDR, 6, 2);
+static DEFINE_CLK_DIVIDER(perclk_pred2, &perclk_pred1.clk, 0,
MXC_CCM_CBCDR, 3, 3);
+static DEFINE_CLK_DIVIDER(perclk_podf, &perclk_pred2.clk,
CLK_PARENT_SET_RATE, MXC_CCM_CBCDR, 0, 3);

becomes,

+static DEFINE_CLK_DIVIDER(perclk_pred1, &perclk_lp_apm.clk, 0,
MXC_CCM_CBCDR, 6, 2, &imx_ccm_lock);
+static DEFINE_CLK_DIVIDER(perclk_pred2, &perclk_pred1.clk, 0,
MXC_CCM_CBCDR, 3, 3, &imx_ccm_lock);
+static DEFINE_CLK_DIVIDER(perclk_podf, &perclk_pred2.clk,
CLK_PARENT_SET_RATE, MXC_CCM_CBCDR, 0, 3, &imx_ccm_lock);

Regards,
Mike

>
> Thanks for review
> Richard
>>
>> Thanks,
>> Mike
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
>

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Richard Zhao]

On Tue, Nov 29, 2011 at 11:22:23AM -0800, Mike Turquette wrote:
> On Mon, Nov 28, 2011 at 7:09 PM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > On Mon, Nov 28, 2011 at 06:36:33PM -0800, Mike Turquette wrote:
> >> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> >> > +#define DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
> >> > + ? ? ? struct clk_divider _name = { \
> >> > + ? ? ? ? ? ? ? .clk = { \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_divider_ops, \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .flags = _flags, \
> >> > + ? ? ? ? ? ? ? }, \
> >> > + ? ? ? ? ? ? ? .reg = (_reg), \
> >> > + ? ? ? ? ? ? ? .shift = (_shift), \
> >> > + ? ? ? ? ? ? ? .width = (_width), \
> >> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> >> > + ? ? ? }
> >> > +
> >> > +#define DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
> >> > + ? ? ? struct clk_mux _name = { \
> >> > + ? ? ? ? ? ? ? .clk = { \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_mux_ops, \
> >> > + ? ? ? ? ? ? ? }, \
> >> > + ? ? ? ? ? ? ? .reg = (_reg), \
> >> > + ? ? ? ? ? ? ? .shift = (_shift), \
> >> > + ? ? ? ? ? ? ? .width = (_width), \
> >> > + ? ? ? ? ? ? ? .clks = (_clks), \
> >> > + ? ? ? ? ? ? ? .num_clks = ARRAY_SIZE(_clks), \
> >> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> >> > + ? ? ? }
> >> > +
> >> > +#define DEFINE_CLK_FIXED(_name, _rate) \
> >> > + ? ? ? struct clk_hw_fixed _name = { \
> >> > + ? ? ? ? ? ? ? .clk = { \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> >> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_hw_fixed_ops, \
> >> > + ? ? ? ? ? ? ? }, \
> >> > + ? ? ? ? ? ? ? .fixed_rate = (_rate), \
> >> > + ? ? ? }
> >> > +
> >> > ?#endif /* CONFIG_GENERIC_CLK */
> >> > ?#endif /* __ASSEMBLY__ */
> >> > ?#endif /* __ASM_ARCH_MXC_CLOCK_H__ */
> >> > --
> >> > 1.7.5.4
> >> >
> >> >
> >> >
> >>
> >> Can you move these into clk.h/approriate header? ?Just add _lock to it
> >> and it'll work fine for your code.
> > Yes, I can add _lock and share the macros in common header. But I still
> 
> On second thought you don't need to resubmit with these in clk.h; I'll
> absorb them into clk.h with my next common clk patchset.
Do you mean you also adopt divider and mux clock patch?
> 
> > need a wraper for imx to eliminate _lock, right?
> 
> No.  Taking code from your patch 8/8:
> 
> +static DEFINE_CLK_DIVIDER(perclk_pred1, &perclk_lp_apm.clk, 0,
> MXC_CCM_CBCDR, 6, 2);
> +static DEFINE_CLK_DIVIDER(perclk_pred2, &perclk_pred1.clk, 0,
> MXC_CCM_CBCDR, 3, 3);
> +static DEFINE_CLK_DIVIDER(perclk_podf, &perclk_pred2.clk,
> CLK_PARENT_SET_RATE, MXC_CCM_CBCDR, 0, 3);
> 
> becomes,
> 
> +static DEFINE_CLK_DIVIDER(perclk_pred1, &perclk_lp_apm.clk, 0,
> MXC_CCM_CBCDR, 6, 2, &imx_ccm_lock);
> +static DEFINE_CLK_DIVIDER(perclk_pred2, &perclk_pred1.clk, 0,
> MXC_CCM_CBCDR, 3, 3, &imx_ccm_lock);
> +static DEFINE_CLK_DIVIDER(perclk_podf, &perclk_pred2.clk,
> CLK_PARENT_SET_RATE, MXC_CCM_CBCDR, 0, 3, &imx_ccm_lock);
understood.

Thanks
Richard
> 
> Regards,
> Mike
> 
> >
> > Thanks for review
> > Richard
> >>
> >> Thanks,
> >> Mike
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
> >
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Mike Turquette]

On Tue, Nov 29, 2011 at 10:18 PM, Richard Zhao <richard.zhao@linaro.org> wrote:
> Do you mean you also adopt divider and mux clock patch?

I'd like to do that and roll them into the existing clk-basic.c.  Are
you OK with this approach?

Regards,
Mike

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Richard Zhao]

On Wed, Nov 30, 2011 at 08:22:42AM -0800, Mike Turquette wrote:
> On Tue, Nov 29, 2011 at 10:18 PM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > Do you mean you also adopt divider and mux clock patch?
> 
> I'd like to do that and roll them into the existing clk-basic.c.  Are
> you OK with this approach?
Sure, glad you take it.
> 
> Regards,
> Mike
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 7/8] ARM i.MX: prepare common clk support

[Richard Zhao]

On Mon, Nov 28, 2011 at 06:36:33PM -0800, Mike Turquette wrote:
> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > +#define DEFINE_CLK_DIVIDER(_name, _parent, _flags, _reg, _shift, _width) \
> > + ? ? ? struct clk_divider _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_divider_ops, \
> > + ? ? ? ? ? ? ? ? ? ? ? .parent = _parent, \
> > + ? ? ? ? ? ? ? ? ? ? ? .flags = _flags, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .reg = (_reg), \
> > + ? ? ? ? ? ? ? .shift = (_shift), \
> > + ? ? ? ? ? ? ? .width = (_width), \
> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> > + ? ? ? }
> > +
> > +#define DEFINE_CLK_MUX(_name, _reg, _shift, _width, _clks) \
> > + ? ? ? struct clk_mux _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_mux_ops, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .reg = (_reg), \
> > + ? ? ? ? ? ? ? .shift = (_shift), \
> > + ? ? ? ? ? ? ? .width = (_width), \
> > + ? ? ? ? ? ? ? .clks = (_clks), \
> > + ? ? ? ? ? ? ? .num_clks = ARRAY_SIZE(_clks), \
> > + ? ? ? ? ? ? ? .lock = &imx_ccm_lock, \
> > + ? ? ? }
> > +
> > +#define DEFINE_CLK_FIXED(_name, _rate) \
> > + ? ? ? struct clk_hw_fixed _name = { \
> > + ? ? ? ? ? ? ? .clk = { \
> > + ? ? ? ? ? ? ? ? ? ? ? .name = #_name, \
> > + ? ? ? ? ? ? ? ? ? ? ? .ops = &clk_hw_fixed_ops, \
> > + ? ? ? ? ? ? ? }, \
> > + ? ? ? ? ? ? ? .fixed_rate = (_rate), \
> > + ? ? ? }
> > +
> > ?#endif /* CONFIG_GENERIC_CLK */
> > ?#endif /* __ASSEMBLY__ */
> > ?#endif /* __ASM_ARCH_MXC_CLOCK_H__ */
> > --
> > 1.7.5.4
> >
> >
> >
> 
> Can you move these into clk.h/approriate header?  Just add _lock to it
> and it'll work fine for your code.
Yes, I can add _lock and share the macros in common header. But I still
need a wraper for imx to eliminate _lock, right?

Thanks for review
Richard
> 
> Thanks,
> Mike
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 3/8] clk: Add support for simple dividers

[Mike Turquette]

On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> +#define account_for_rounding_errors ? ?1
> +
> +static int clk_divider_bestdiv(struct clk *clk, unsigned long rate,
> + ? ? ? ? ? ? ? unsigned long *best_parent_rate)
> +{
> + ? ? ? struct clk_divider *divider = to_clk_divider(clk);
> + ? ? ? int i, bestdiv = 0;
> + ? ? ? unsigned long parent_rate, best = 0, now, maxdiv;
> +
> + ? ? ? maxdiv = (1 << divider->width);
> +
> + ? ? ? if (divider->flags & CLK_DIVIDER_FLAG_ONE_BASED)
> + ? ? ? ? ? ? ? maxdiv--;
> +
> + ? ? ? if (!(clk->flags & CLK_PARENT_SET_RATE)) {
> + ? ? ? ? ? ? ? parent_rate = clk->parent->rate;
> + ? ? ? ? ? ? ? bestdiv = parent_rate / rate;
> + ? ? ? ? ? ? ? bestdiv = bestdiv == 0 ? 1 : bestdiv;
> + ? ? ? ? ? ? ? bestdiv = bestdiv > maxdiv ? maxdiv : bestdiv;
> + ? ? ? ? ? ? ? goto out;
> + ? ? ? }
> +
> + ? ? ? /*
> + ? ? ? ?* The maximum divider we can use without overflowing
> + ? ? ? ?* unsigned long in rate * i below
> + ? ? ? ?*/
> + ? ? ? maxdiv = min(ULONG_MAX / rate, maxdiv);
> +
> + ? ? ? for (i = 1; i <= maxdiv; i++) {
> + ? ? ? ? ? ? ? parent_rate = clk_round_rate(clk->parent,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (rate + account_for_rounding_errors) * i);

How about just ((rate + 1) * i) with a comment above explaining why?
This removes the awkward #define above.

Also, I'd like to roll this into clk-basic.c for the V4 series.  Any objections?

Thanks,
Mike

[RFC V1 3/8] clk: Add support for simple dividers

[Richard Zhao]

On Wed, Nov 30, 2011 at 12:43:10PM -0800, Mike Turquette wrote:
> On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > +#define account_for_rounding_errors ? ?1
> > +
> > +static int clk_divider_bestdiv(struct clk *clk, unsigned long rate,
> > + ? ? ? ? ? ? ? unsigned long *best_parent_rate)
> > +{
> > + ? ? ? struct clk_divider *divider = to_clk_divider(clk);
> > + ? ? ? int i, bestdiv = 0;
> > + ? ? ? unsigned long parent_rate, best = 0, now, maxdiv;
> > +
> > + ? ? ? maxdiv = (1 << divider->width);
> > +
> > + ? ? ? if (divider->flags & CLK_DIVIDER_FLAG_ONE_BASED)
> > + ? ? ? ? ? ? ? maxdiv--;
> > +
> > + ? ? ? if (!(clk->flags & CLK_PARENT_SET_RATE)) {
> > + ? ? ? ? ? ? ? parent_rate = clk->parent->rate;
> > + ? ? ? ? ? ? ? bestdiv = parent_rate / rate;
> > + ? ? ? ? ? ? ? bestdiv = bestdiv == 0 ? 1 : bestdiv;
> > + ? ? ? ? ? ? ? bestdiv = bestdiv > maxdiv ? maxdiv : bestdiv;
> > + ? ? ? ? ? ? ? goto out;
> > + ? ? ? }
The code is originally from Sascha. I think it again.

Sascha, why don't we use the below code?

	for (i = 1; i <= maxdiv; i++) {
		int div;
		parent_rate = clk_round_rate(clk->parent, rate * i);
		div = parent_rate / rate;
		div = div > maxdiv ? maxdiv : div;
		div = div < 1 ? 1 : div;
		now = parent_rate / div;

		if (now <= rate && now >= best) {
			bestdiv = div;
			best = now;
			best_parent_rate = parent_rate;
		}
	}


> > +
> > + ? ? ? /*
> > + ? ? ? ?* The maximum divider we can use without overflowing
> > + ? ? ? ?* unsigned long in rate * i below
> > + ? ? ? ?*/
> > + ? ? ? maxdiv = min(ULONG_MAX / rate, maxdiv);
> > +
> > + ? ? ? for (i = 1; i <= maxdiv; i++) {
> > + ? ? ? ? ? ? ? parent_rate = clk_round_rate(clk->parent,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (rate + account_for_rounding_errors) * i);
> 
> How about just ((rate + 1) * i) with a comment above explaining why?
> This removes the awkward #define above.
> 
> Also, I'd like to roll this into clk-basic.c for the V4 series.  Any objections?
Go ahead.

Thanks
Richard
> 
> Thanks,
> Mike
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC V1 3/8] clk: Add support for simple dividers

[Sascha Hauer]

On Thu, Dec 01, 2011 at 03:42:59PM +0800, Richard Zhao wrote:
> On Wed, Nov 30, 2011 at 12:43:10PM -0800, Mike Turquette wrote:
> > On Wed, Nov 23, 2011 at 3:12 AM, Richard Zhao <richard.zhao@linaro.org> wrote:
> > > +#define account_for_rounding_errors ? ?1
> > > +
> > > +static int clk_divider_bestdiv(struct clk *clk, unsigned long rate,
> > > + ? ? ? ? ? ? ? unsigned long *best_parent_rate)
> > > +{
> > > + ? ? ? struct clk_divider *divider = to_clk_divider(clk);
> > > + ? ? ? int i, bestdiv = 0;
> > > + ? ? ? unsigned long parent_rate, best = 0, now, maxdiv;
> > > +
> > > + ? ? ? maxdiv = (1 << divider->width);
> > > +
> > > + ? ? ? if (divider->flags & CLK_DIVIDER_FLAG_ONE_BASED)
> > > + ? ? ? ? ? ? ? maxdiv--;
> > > +
> > > + ? ? ? if (!(clk->flags & CLK_PARENT_SET_RATE)) {
> > > + ? ? ? ? ? ? ? parent_rate = clk->parent->rate;
> > > + ? ? ? ? ? ? ? bestdiv = parent_rate / rate;
> > > + ? ? ? ? ? ? ? bestdiv = bestdiv == 0 ? 1 : bestdiv;
> > > + ? ? ? ? ? ? ? bestdiv = bestdiv > maxdiv ? maxdiv : bestdiv;
> > > + ? ? ? ? ? ? ? goto out;
> > > + ? ? ? }
> The code is originally from Sascha. I think it again.
> 
> Sascha, why don't we use the below code?
> 
> 	for (i = 1; i <= maxdiv; i++) {
> 		int div;
> 		parent_rate = clk_round_rate(clk->parent, rate * i);
> 		div = parent_rate / rate;
> 		div = div > maxdiv ? maxdiv : div;
> 		div = div < 1 ? 1 : div;
> 		now = parent_rate / div;
> 
> 		if (now <= rate && now >= best) {
> 			bestdiv = div;
> 			best = now;
> 			best_parent_rate = parent_rate;
> 		}
> 	}

Yes, why not ;)

I can't remember the exact problem, but it came in with cascaded
dividers which wouldn't work without this. I've never been very
proud of this account_for_rounding_errors, so feel free to remove
it and see what happens. If this becomes a problem we can search
for a better solution.


> 
> 
> > > +
> > > + ? ? ? /*
> > > + ? ? ? ?* The maximum divider we can use without overflowing
> > > + ? ? ? ?* unsigned long in rate * i below
> > > + ? ? ? ?*/
> > > + ? ? ? maxdiv = min(ULONG_MAX / rate, maxdiv);
> > > +
> > > + ? ? ? for (i = 1; i <= maxdiv; i++) {
> > > + ? ? ? ? ? ? ? parent_rate = clk_round_rate(clk->parent,
> > > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (rate + account_for_rounding_errors) * i);
> > 
> > How about just ((rate + 1) * i) with a comment above explaining why?
> > This removes the awkward #define above.
> > 
> > Also, I'd like to roll this into clk-basic.c for the V4 series.  Any objections?
> Go ahead.
> 
> Thanks
> Richard
> > 
> > Thanks,
> > Mike
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |