On Wed, 8 May 2019 06:51:46 +0000 Peng Fan <[email protected]> wrote: > > -----Original Message----- > > From: Lukasz Majewski [mailto:[email protected]] > > Sent: 2019年5月8日 14:46 > > To: Peng Fan <[email protected]> > > Cc: [email protected]; [email protected]; dl-uboot-imx > > <[email protected]>; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected] > > 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 <[email protected]> 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 <[email protected]> > > > > > --- > > > > > 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: [email protected] > > > > > > > > > > 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: > > [email protected] 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: [email protected]
pgpQ7B0b3Kpv9.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
