| Message ID | 20190430103056.32537-12-peng.fan@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | i.MX8MM + CCF | expand |
Hi Peng, > Export the structure for others to use. > > Signed-off-by: Peng Fan <peng.fan@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? 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@denx.de
> 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@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); + + /* Configure ARM PLL to 1.2GHz */ + clk_get_by_id(IMX8MM_ARM_PLL, &clkp1); + clk_set_rate(clkp1, 1200000000UL); + 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@denx.de
On Tue, 7 May 2019 13:27:45 +0000 Peng Fan <peng.fan@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@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@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@denx.de
> -----Original Message----- > From: Lukasz Majewski [mailto:lukma@denx.de] > Sent: 2019年5月8日 14:46 > To: Peng Fan <peng.fan@nxp.com> > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > <uboot-imx@nxp.com>; sjg@chromium.org; jagan@amarulasolutions.com; > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export clk_fixed_rate > > On Tue, 7 May 2019 13:27:45 +0000 > Peng Fan <peng.fan@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@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? The A53 clk could not change on the fly. There is a mux here, one is PLL, one is OSC, And there are others. If we want to change the pll clock which is currently being used by A53, we need first switch the A53 clk to source from OSC, then change pll clock, then switch A53 clk back to PLL. > > 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? You mean get freq from cpu opp and set cpu freq? I do not have good idea how to set in DTS. Thanks, Peng. > > > + 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@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@denx.de
On Wed, 8 May 2019 06:51:46 +0000 Peng Fan <peng.fan@nxp.com> wrote: > > -----Original Message----- > > From: Lukasz Majewski [mailto:lukma@denx.de] > > Sent: 2019年5月8日 14:46 > > To: Peng Fan <peng.fan@nxp.com> > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > <uboot-imx@nxp.com>; sjg@chromium.org; jagan@amarulasolutions.com; > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > clk_fixed_rate > > > > On Tue, 7 May 2019 13:27:45 +0000 > > Peng Fan <peng.fan@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@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? > > The A53 clk could not change on the fly. There is a mux here, one is > PLL, one is OSC, And there are others. If we want to change the pll > clock which is currently being used by A53, we need first switch the > A53 clk to source from OSC, then change pll clock, then switch A53 > clk back to PLL. The above description looks like a "standard" procedure for bypassing PLL when it is going to be locked. The same is also performed on IMX6Q: https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx6q.c#L88 But I've not ported that part from the original Linux source code. > > > > > 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? > > You mean get freq from cpu opp and set cpu freq? I do not have good > idea how to set in DTS. I'm thinking about following description of the "osc" fixed clock: https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/imx6qdl.dtsi#L64 and http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl.dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 The "fixed-clock" code has been adjusted to comply with the above DTS (which is provided in pre-reloc SPL - even before console and ddr setup). (I had to use debug uart for debugging). > > Thanks, > Peng. > > > > > > + 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@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@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@denx.de
> -----Original Message----- > From: Lukasz Majewski [mailto:lukma@denx.de] > Sent: 2019年5月8日 15:40 > To: Peng Fan <peng.fan@nxp.com> > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > <uboot-imx@nxp.com>; sjg@chromium.org; jagan@amarulasolutions.com; > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export clk_fixed_rate > > On Wed, 8 May 2019 06:51:46 +0000 > Peng Fan <peng.fan@nxp.com> wrote: > > > > -----Original Message----- > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > Sent: 2019年5月8日 14:46 > > > To: Peng Fan <peng.fan@nxp.com> > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > <uboot-imx@nxp.com>; sjg@chromium.org; > jagan@amarulasolutions.com; > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > clk_fixed_rate > > > > > > On Tue, 7 May 2019 13:27:45 +0000 > > > Peng Fan <peng.fan@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@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? > > > > The A53 clk could not change on the fly. There is a mux here, one is > > PLL, one is OSC, And there are others. If we want to change the pll > > clock which is currently being used by A53, we need first switch the > > A53 clk to source from OSC, then change pll clock, then switch A53 clk > > back to PLL. > > The above description looks like a "standard" procedure for bypassing PLL > when it is going to be locked. > > The same is also performed on IMX6Q: > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx6q.c#L88 > > But I've not ported that part from the original Linux source code. i.MX8MM is different. This is how Linux change cpu's clock. https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx8mm.c#L655 It has a new cpu clk driver, I do not want to introduce that complexity in U-Boot. This patch is just a simplified step of the upper Linux code. The reason to configure the clk here is that ROM not configure the clock following datasheet even it not hurt, but better to configure it right in SPL. Thanks, Peng. > > > > > > > > > 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? > > > > You mean get freq from cpu opp and set cpu freq? I do not have good > > idea how to set in DTS. > > I'm thinking about following description of the "osc" fixed clock: > https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/imx6qdl.dtsi# > L64 > > and > > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl. > dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 > > The "fixed-clock" code has been adjusted to comply with the above DTS > (which is provided in pre-reloc SPL - even before console and ddr setup). > > (I had to use debug uart for debugging). > > > > > Thanks, > > Peng. > > > > > > > > > + 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@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@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@denx.de
On Wed, 8 May 2019 07:45:39 +0000 Peng Fan <peng.fan@nxp.com> wrote: > > -----Original Message----- > > From: Lukasz Majewski [mailto:lukma@denx.de] > > Sent: 2019年5月8日 15:40 > > To: Peng Fan <peng.fan@nxp.com> > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > <uboot-imx@nxp.com>; sjg@chromium.org; jagan@amarulasolutions.com; > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > clk_fixed_rate > > > > On Wed, 8 May 2019 06:51:46 +0000 > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > -----Original Message----- > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > Sent: 2019年5月8日 14:46 > > > > To: Peng Fan <peng.fan@nxp.com> > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > jagan@amarulasolutions.com; > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > clk_fixed_rate > > > > > > > > On Tue, 7 May 2019 13:27:45 +0000 > > > > Peng Fan <peng.fan@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@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? > > > > > > The A53 clk could not change on the fly. There is a mux here, one > > > is PLL, one is OSC, And there are others. If we want to change > > > the pll clock which is currently being used by A53, we need first > > > switch the A53 clk to source from OSC, then change pll clock, > > > then switch A53 clk back to PLL. > > > > The above description looks like a "standard" procedure for > > bypassing PLL when it is going to be locked. > > > > The same is also performed on IMX6Q: > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx6q.c#L88 > > > > But I've not ported that part from the original Linux source code. > > i.MX8MM is different. > > This is how Linux change cpu's clock. > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx8mm.c#L655 > > It has a new cpu clk driver, I do not want to introduce that > complexity in U-Boot. Ok. > > This patch is just a simplified step of the upper Linux code. My point is that this driver pollutes the fixed-clock code and makes it imx8 specific. > > The reason to configure the clk here is that ROM not configure the > clock following datasheet even it not hurt, but better to configure > it right in SPL. So this code is for fixing bugs (incomplete initialisation) in SoC's ROM? > > Thanks, > Peng. > > > > > > > > > > > > > > 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? > > > > > > You mean get freq from cpu opp and set cpu freq? I do not have > > > good idea how to set in DTS. > > > > I'm thinking about following description of the "osc" fixed clock: > > https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/imx6qdl.dtsi# > > L64 > > > > and > > > > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl. > > dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 > > > > The "fixed-clock" code has been adjusted to comply with the above > > DTS (which is provided in pre-reloc SPL - even before console and > > ddr setup). > > > > (I had to use debug uart for debugging). > > > > > > > > Thanks, > > > Peng. > > > > > > > > > > > > + 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@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@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@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@denx.de
> Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export clk_fixed_rate > > On Wed, 8 May 2019 07:45:39 +0000 > Peng Fan <peng.fan@nxp.com> wrote: > > > > -----Original Message----- > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > Sent: 2019年5月8日 15:40 > > > To: Peng Fan <peng.fan@nxp.com> > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > <uboot-imx@nxp.com>; sjg@chromium.org; > jagan@amarulasolutions.com; > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > clk_fixed_rate > > > > > > On Wed, 8 May 2019 06:51:46 +0000 > > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > > > -----Original Message----- > > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > > Sent: 2019年5月8日 14:46 > > > > > To: Peng Fan <peng.fan@nxp.com> > > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > > jagan@amarulasolutions.com; > > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > > clk_fixed_rate > > > > > > > > > > On Tue, 7 May 2019 13:27:45 +0000 Peng Fan <peng.fan@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@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? > > > > > > > > The A53 clk could not change on the fly. There is a mux here, one > > > > is PLL, one is OSC, And there are others. If we want to change the > > > > pll clock which is currently being used by A53, we need first > > > > switch the A53 clk to source from OSC, then change pll clock, then > > > > switch A53 clk back to PLL. > > > > > > The above description looks like a "standard" procedure for > > > bypassing PLL when it is going to be locked. > > > > > > The same is also performed on IMX6Q: > > > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx > > > 6q.c#L88 > > > > > > But I've not ported that part from the original Linux source code. > > > > i.MX8MM is different. > > > > This is how Linux change cpu's clock. > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx8m > > m.c#L655 > > > > It has a new cpu clk driver, I do not want to introduce that > > complexity in U-Boot. > > Ok. > > > > > This patch is just a simplified step of the upper Linux code. > > My point is that this driver pollutes the fixed-clock code and makes it > imx8 specific. If i.MX7 switch to CCF, it also has such requirement. I am not sure others, Linux exports grep "to_clk_fix" ./include/linux/clk-provider.h -rn 313:#define to_clk_fixed_rate(_hw) container_of(_hw, struct clk_fixed_rate, hw) 575:#define to_clk_fixed_factor(_hw) container_of(_hw, struct clk_fixed_factor, hw) I think it does not hurt for uboot also export it. > > > > > The reason to configure the clk here is that ROM not configure the > > clock following datasheet even it not hurt, but better to configure it > > right in SPL. > > So this code is for fixing bugs (incomplete initialisation) in SoC's ROM? Strictly speaking it is not a bug. The datasheet opp has 1.2GHz, but not 1GHz configured by ROM. But actually when power up, the initial freq is 24MHz. Just configure in uboot to follow datasheet and might make Linux kernel happy. Thanks, Peng. > > > > > Thanks, > > Peng. > > > > > > > > > > > > > > > > > > > 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? > > > > > > > > You mean get freq from cpu opp and set cpu freq? I do not have > > > > good idea how to set in DTS. > > > > > > I'm thinking about following description of the "osc" fixed clock: > > > https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/imx6q > > > dl.dtsi# > > > L64 > > > > > > and > > > > > > > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl. > > > dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 > > > > > > The "fixed-clock" code has been adjusted to comply with the above > > > DTS (which is provided in pre-reloc SPL - even before console and > > > ddr setup). > > > > > > (I had to use debug uart for debugging). > > > > > > > > > > > Thanks, > > > > Peng. > > > > > > > > > > > > > > > + 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@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@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@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@denx.de
On Thu, 9 May 2019 01:13:15 +0000 Peng Fan <peng.fan@nxp.com> wrote: > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > clk_fixed_rate > > > > On Wed, 8 May 2019 07:45:39 +0000 > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > -----Original Message----- > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > Sent: 2019年5月8日 15:40 > > > > To: Peng Fan <peng.fan@nxp.com> > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > jagan@amarulasolutions.com; > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > clk_fixed_rate > > > > > > > > On Wed, 8 May 2019 06:51:46 +0000 > > > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > > > > > -----Original Message----- > > > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > > > Sent: 2019年5月8日 14:46 > > > > > > To: Peng Fan <peng.fan@nxp.com> > > > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > > > jagan@amarulasolutions.com; > > > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > > > clk_fixed_rate > > > > > > > > > > > > On Tue, 7 May 2019 13:27:45 +0000 Peng Fan > > > > > > <peng.fan@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@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? > > > > > > > > > > The A53 clk could not change on the fly. There is a mux here, > > > > > one is PLL, one is OSC, And there are others. If we want to > > > > > change the pll clock which is currently being used by A53, we > > > > > need first switch the A53 clk to source from OSC, then change > > > > > pll clock, then switch A53 clk back to PLL. > > > > > > > > The above description looks like a "standard" procedure for > > > > bypassing PLL when it is going to be locked. > > > > > > > > The same is also performed on IMX6Q: > > > > > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx > > > > 6q.c#L88 > > > > > > > > But I've not ported that part from the original Linux source > > > > code. > > > > > > i.MX8MM is different. > > > > > > This is how Linux change cpu's clock. > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-imx8m > > > m.c#L655 > > > > > > It has a new cpu clk driver, I do not want to introduce that > > > complexity in U-Boot. > > > > Ok. > > > > > > > > This patch is just a simplified step of the upper Linux code. > > > > My point is that this driver pollutes the fixed-clock code and > > makes it imx8 specific. > > If i.MX7 switch to CCF, it also has such requirement. > > I am not sure others, Linux exports > grep "to_clk_fix" ./include/linux/clk-provider.h -rn > 313:#define to_clk_fixed_rate(_hw) container_of(_hw, struct > clk_fixed_rate, hw) 575:#define to_clk_fixed_factor(_hw) > container_of(_hw, struct clk_fixed_factor, hw) > > I think it does not hurt for uboot also export it. > > > > > > > > > The reason to configure the clk here is that ROM not configure the > > > clock following datasheet even it not hurt, but better to > > > configure it right in SPL. > > > > So this code is for fixing bugs (incomplete initialisation) in > > SoC's ROM? > > Strictly speaking it is not a bug. The datasheet opp has 1.2GHz, but > not 1GHz configured by ROM. But actually when power up, the initial > freq is 24MHz. Just configure in uboot to follow datasheet and might > make Linux kernel happy. For IMX6Q there is a table in the User Manual, which describes the state (i.e. the clock frequencies of the IP blocks) in which is the board when ROM handles the execution to SPL. I also assume that 24 MHz Core frequency is too low and will be fixed in next ROM (SoC) revisions ? > > Thanks, > Peng. > > > > > > > > > Thanks, > > > Peng. > > > > > > > > > > > > > > > > > > > > > > > > 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? > > > > > > > > > > You mean get freq from cpu opp and set cpu freq? I do not have > > > > > good idea how to set in DTS. > > > > > > > > I'm thinking about following description of the "osc" fixed > > > > clock: > > > > https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/imx6q > > > > dl.dtsi# L64 > > > > > > > > and > > > > > > > > > > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl. > > > > dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 > > > > > > > > The "fixed-clock" code has been adjusted to comply with the > > > > above DTS (which is provided in pre-reloc SPL - even before > > > > console and ddr setup). > > > > > > > > (I had to use debug uart for debugging). > > > > > > > > > > > > > > Thanks, > > > > > Peng. > > > > > > > > > > > > > > > > > > + 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@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@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@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@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@denx.de
> Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export clk_fixed_rate > > On Thu, 9 May 2019 01:13:15 +0000 > Peng Fan <peng.fan@nxp.com> wrote: > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > clk_fixed_rate > > > > > > On Wed, 8 May 2019 07:45:39 +0000 > > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > > > -----Original Message----- > > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > > Sent: 2019年5月8日 15:40 > > > > > To: Peng Fan <peng.fan@nxp.com> > > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > > jagan@amarulasolutions.com; > > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > > clk_fixed_rate > > > > > > > > > > On Wed, 8 May 2019 06:51:46 +0000 Peng Fan <peng.fan@nxp.com> > > > > > wrote: > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > > > > Sent: 2019年5月8日 14:46 > > > > > > > To: Peng Fan <peng.fan@nxp.com> > > > > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > > > > jagan@amarulasolutions.com; > > > > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > > > > clk_fixed_rate > > > > > > > > > > > > > > On Tue, 7 May 2019 13:27:45 +0000 Peng Fan > > > > > > > <peng.fan@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@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? > > > > > > > > > > > > The A53 clk could not change on the fly. There is a mux here, > > > > > > one is PLL, one is OSC, And there are others. If we want to > > > > > > change the pll clock which is currently being used by A53, we > > > > > > need first switch the A53 clk to source from OSC, then change > > > > > > pll clock, then switch A53 clk back to PLL. > > > > > > > > > > The above description looks like a "standard" procedure for > > > > > bypassing PLL when it is going to be locked. > > > > > > > > > > The same is also performed on IMX6Q: > > > > > > > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk > > > > > -imx > > > > > 6q.c#L88 > > > > > > > > > > But I've not ported that part from the original Linux source > > > > > code. > > > > > > > > i.MX8MM is different. > > > > > > > > This is how Linux change cpu's clock. > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-i > > > > mx8m > > > > m.c#L655 > > > > > > > > It has a new cpu clk driver, I do not want to introduce that > > > > complexity in U-Boot. > > > > > > Ok. > > > > > > > > > > > This patch is just a simplified step of the upper Linux code. > > > > > > My point is that this driver pollutes the fixed-clock code and makes > > > it imx8 specific. > > > > If i.MX7 switch to CCF, it also has such requirement. > > > > I am not sure others, Linux exports > > grep "to_clk_fix" ./include/linux/clk-provider.h -rn 313:#define > > to_clk_fixed_rate(_hw) container_of(_hw, struct clk_fixed_rate, hw) > > 575:#define to_clk_fixed_factor(_hw) container_of(_hw, struct > > clk_fixed_factor, hw) > > > > I think it does not hurt for uboot also export it. > > > > > > > > > > > > > The reason to configure the clk here is that ROM not configure the > > > > clock following datasheet even it not hurt, but better to > > > > configure it right in SPL. > > > > > > So this code is for fixing bugs (incomplete initialisation) in SoC's > > > ROM? > > > > Strictly speaking it is not a bug. The datasheet opp has 1.2GHz, but > > not 1GHz configured by ROM. But actually when power up, the initial > > freq is 24MHz. Just configure in uboot to follow datasheet and might > > make Linux kernel happy. > > For IMX6Q there is a table in the User Manual, which describes the state (i.e. > the clock frequencies of the IP blocks) in which is the board when ROM > handles the execution to SPL. > > I also assume that 24 MHz Core frequency is too low and will be fixed in next > ROM (SoC) revisions ? When power up, before ROM configure PLL, the core will runs at osc clock, After ROM configures ARM PLL, the core will runs at higher frequency. It is just ROM not configure ARM PLL following datasheet, but it is not actually an issue here. U-Boot just following spec to reconfigure the ARM PLL. Regards, Peng. > > > > > Thanks, > > Peng. > > > > > > > > > > > > > Thanks, > > > > Peng. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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? > > > > > > > > > > > > You mean get freq from cpu opp and set cpu freq? I do not have > > > > > > good idea how to set in DTS. > > > > > > > > > > I'm thinking about following description of the "osc" fixed > > > > > clock: > > > > > https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/i > > > > > mx6q > > > > > dl.dtsi# L64 > > > > > > > > > > and > > > > > > > > > > > > > > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl. > > > > > dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 > > > > > > > > > > The "fixed-clock" code has been adjusted to comply with the > > > > > above DTS (which is provided in pre-reloc SPL - even before > > > > > console and ddr setup). > > > > > > > > > > (I had to use debug uart for debugging). > > > > > > > > > > > > > > > > > Thanks, > > > > > > Peng. > > > > > > > > > > > > > > > > > > > > > + 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@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@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@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@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@denx.de
On Thu, 16 May 2019 09:58:17 +0000 Peng Fan <peng.fan@nxp.com> wrote: > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > clk_fixed_rate > > > > On Thu, 9 May 2019 01:13:15 +0000 > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > clk_fixed_rate > > > > > > > > On Wed, 8 May 2019 07:45:39 +0000 > > > > Peng Fan <peng.fan@nxp.com> wrote: > > > > > > > > > > -----Original Message----- > > > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > > > Sent: 2019年5月8日 15:40 > > > > > > To: Peng Fan <peng.fan@nxp.com> > > > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > > > jagan@amarulasolutions.com; > > > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > > > clk_fixed_rate > > > > > > > > > > > > On Wed, 8 May 2019 06:51:46 +0000 Peng Fan > > > > > > <peng.fan@nxp.com> wrote: > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Lukasz Majewski [mailto:lukma@denx.de] > > > > > > > > Sent: 2019年5月8日 14:46 > > > > > > > > To: Peng Fan <peng.fan@nxp.com> > > > > > > > > Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx > > > > > > > > <uboot-imx@nxp.com>; sjg@chromium.org; > > > > > > jagan@amarulasolutions.com; > > > > > > > > sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com > > > > > > > > Subject: Re: [i.MX8MM+CCF 11/41] clk: fixed_rate: export > > > > > > > > clk_fixed_rate > > > > > > > > > > > > > > > > On Tue, 7 May 2019 13:27:45 +0000 Peng Fan > > > > > > > > <peng.fan@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@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? > > > > > > > > > > > > > > The A53 clk could not change on the fly. There is a mux > > > > > > > here, one is PLL, one is OSC, And there are others. If we > > > > > > > want to change the pll clock which is currently being > > > > > > > used by A53, we need first switch the A53 clk to source > > > > > > > from OSC, then change pll clock, then switch A53 clk back > > > > > > > to PLL. > > > > > > > > > > > > The above description looks like a "standard" procedure for > > > > > > bypassing PLL when it is going to be locked. > > > > > > > > > > > > The same is also performed on IMX6Q: > > > > > > > > > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk > > > > > > -imx > > > > > > 6q.c#L88 > > > > > > > > > > > > But I've not ported that part from the original Linux source > > > > > > code. > > > > > > > > > > i.MX8MM is different. > > > > > > > > > > This is how Linux change cpu's clock. > > > > > https://elixir.bootlin.com/linux/v5.1/source/drivers/clk/imx/clk-i > > > > > mx8m > > > > > m.c#L655 > > > > > > > > > > It has a new cpu clk driver, I do not want to introduce that > > > > > complexity in U-Boot. > > > > > > > > Ok. > > > > > > > > > > > > > > This patch is just a simplified step of the upper Linux > > > > > code. > > > > > > > > My point is that this driver pollutes the fixed-clock code and > > > > makes it imx8 specific. > > > > > > If i.MX7 switch to CCF, it also has such requirement. > > > > > > I am not sure others, Linux exports > > > grep "to_clk_fix" ./include/linux/clk-provider.h -rn 313:#define > > > to_clk_fixed_rate(_hw) container_of(_hw, struct clk_fixed_rate, > > > hw) 575:#define to_clk_fixed_factor(_hw) container_of(_hw, struct > > > clk_fixed_factor, hw) > > > > > > I think it does not hurt for uboot also export it. > > > > > > > > > > > > > > > > > The reason to configure the clk here is that ROM not > > > > > configure the clock following datasheet even it not hurt, but > > > > > better to configure it right in SPL. > > > > > > > > So this code is for fixing bugs (incomplete initialisation) in > > > > SoC's ROM? > > > > > > Strictly speaking it is not a bug. The datasheet opp has 1.2GHz, > > > but not 1GHz configured by ROM. But actually when power up, the > > > initial freq is 24MHz. Just configure in uboot to follow > > > datasheet and might make Linux kernel happy. > > > > For IMX6Q there is a table in the User Manual, which describes the > > state (i.e. the clock frequencies of the IP blocks) in which is the > > board when ROM handles the execution to SPL. > > > > I also assume that 24 MHz Core frequency is too low and will be > > fixed in next ROM (SoC) revisions ? > > When power up, before ROM configure PLL, the core will runs at osc > clock, After ROM configures ARM PLL, the core will runs at higher > frequency. > > It is just ROM not configure ARM PLL following datasheet, but it is > not actually an issue here. U-Boot just following spec to reconfigure > the ARM PLL. Ach... OK. I must have misunderstood the last e-mail. The issue is with difference between 1.2 GHz and 1.0 GHz PLL configuration. > > Regards, > Peng. > > > > > > > > > Thanks, > > > Peng. > > > > > > > > > > > > > > > > > Thanks, > > > > > Peng. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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? > > > > > > > > > > > > > > You mean get freq from cpu opp and set cpu freq? I do not > > > > > > > have good idea how to set in DTS. > > > > > > > > > > > > I'm thinking about following description of the "osc" fixed > > > > > > clock: > > > > > > https://elixir.bootlin.com/linux/v5.1/source/arch/arm/boot/dts/i > > > > > > mx6q > > > > > > dl.dtsi# L64 > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/dts/imx6qdl. > > > > > > dtsi;h=c0a94780087382a6e2805b7d6572f3e5e294e302;hb=HEAD#l53 > > > > > > > > > > > > The "fixed-clock" code has been adjusted to comply with the > > > > > > above DTS (which is provided in pre-reloc SPL - even before > > > > > > console and ddr setup). > > > > > > > > > > > > (I had to use debug uart for debugging). > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > Peng. > > > > > > > > > > > > > > > > > > > > > > > > + 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@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@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@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@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@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@denx.de
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; +}; + +#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);
Export the structure for others to use. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- drivers/clk/clk_fixed_rate.c | 8 +------- include/linux/clk-provider.h | 7 +++++++ 2 files changed, 8 insertions(+), 7 deletions(-)