[U-Boot] [i.MX8MM+CCF 11/41] clk: fixed_rate: export clk_fixed_rate

[Lukasz Majewski]

On Tue, 7 May 2019 13:27:45 +0000
Peng Fan <peng.fan at nxp.com> wrote:

> > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export
> > clk_fixed_rate
> > 
> > Hi Peng,
> >   
> > > Export the structure for others to use.
> > >
> > > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > > ---
> > >  drivers/clk/clk_fixed_rate.c | 8 +-------
> > > include/linux/clk-provider.h | 7 +++++++
> > >  2 files changed, 8 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk_fixed_rate.c
> > > b/drivers/clk/clk_fixed_rate.c index 089f060a23..069e643fbc 100644
> > > --- a/drivers/clk/clk_fixed_rate.c
> > > +++ b/drivers/clk/clk_fixed_rate.c
> > > @@ -6,13 +6,7 @@
> > >  #include <common.h>
> > >  #include <clk-uclass.h>
> > >  #include <dm.h>
> > > -
> > > -struct clk_fixed_rate {
> > > -	struct clk clk;
> > > -	unsigned long fixed_rate;
> > > -};
> > > -
> > > -#define to_clk_fixed_rate(dev)	((struct clk_fixed_rate
> > > *)dev_get_platdata(dev)) +#include <linux/clk-provider.h>
> > >
> > >  static ulong clk_fixed_rate_get_rate(struct clk *clk)  { diff
> > > --git a/include/linux/clk-provider.h
> > > b/include/linux/clk-provider.h index 3ed0db86d2..b2bed768b6 100644
> > > --- a/include/linux/clk-provider.h
> > > +++ b/include/linux/clk-provider.h
> > > @@ -112,6 +112,13 @@ struct clk_fixed_factor {  #define
> > > to_clk_fixed_factor(_clk) container_of(_clk, struct
> > > clk_fixed_factor,\ clk)
> > >
> > > +struct clk_fixed_rate {
> > > +	struct clk clk;
> > > +	unsigned long fixed_rate;
> > > +};  
> > 
> > I think that this struct shall stay where it was. Moreover, the
> > clk-provider.h is not the API to be used by other parts of the
> > clock API.
> > 
> > The clk_fixed_rate shall be accessed via get_rate() only and in
> > IMX6Q it is available in early SPL (parsed from dts /clocks
> > property - the 24MHz OSC) 
> > > +
> > > +#define to_clk_fixed_rate(dev)	((struct clk_fixed_rate
> > > *)dev_get_platdata(dev)) +
> > >  int clk_register(struct clk *clk, const char *drv_name,
> > >  		 ulong drv_data, const char *name,
> > >  		 const char *parent_name);  
> > 
> > Please explain why iMX8MM needs such global export?  
> 
> In clk-imx8mm.c, first configure ARM clk to osc24M fixed clk to
> change pll clock.
> +       /* Configure ARM to osc24M */
> +       clk_get_by_id(IMX8MM_CLK_A53_SRC, &clkp);
> +       uclass_get_device_by_name(UCLASS_CLK, "clock-osc-24m", &devp);
> +       clkp1 = &to_clk_fixed_rate(devp)->clk;
> +       clk_set_parent(clkp, clkp1);

This code looks a bit strange to me. Why imx8mm sets parent here?

In the imx6q I write "osc" as a part of the clock tree:

        clk_dm(IMX6QDL_CLK_PLL2,
               imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2_bus", "osc",
                             base + 0x30, 0x1));

And here the parent is set as "osc".

Moreover, the "osc" or in your case "clock-osc-24m" shall be available
in "clocks" DTS node and hence parsed / setup from there (and be
available in the clk uclass list).

> +
> +       /* Configure ARM PLL to 1.2GHz */
> +       clk_get_by_id(IMX8MM_ARM_PLL, &clkp1);
> +       clk_set_rate(clkp1, 1200000000UL);

Shouldn't this be set in DTS ? As it is now - it seems like a hardcoded
value for early SPL clock setup. Am I right?

> +       clk_get_by_id(IMX8MM_ARM_PLL_OUT, &clkp1);
> +       clk_enable(clkp1);
> +       clk_set_parent(clkp, clkp1);
> +
> +       /* Configure DIV to 1.2GHz */
> +       clk_get_by_id(IMX8MM_CLK_A53_DIV, &clkp1);
> +       clk_set_rate(clkp1, 1200000000UL);
> 
> Regards,
> Peng.
> 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma at denx.de  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190508/91c383b9/attachment.sig>