Hi Peng, > > -----Original Message----- > > From: Lukasz Majewski [mailto:lu...@denx.de] > > Sent: 2019年4月23日 16:46 > > To: Peng Fan <peng....@nxp.com> > > Cc: Jagan Teki <ja...@amarulasolutions.com>; Stefano Babic > > <sba...@denx.de>; Fabio Estevam <fabio.este...@nxp.com>; Simon Glass > > <s...@chromium.org>; Tom Rini <tr...@konsulko.com>; Marek Vasut > > <marek.vasut+rene...@gmail.com>; Neil Armstrong > > <narmstr...@baylibre.com>; Philipp Tomsich > > <philipp.toms...@theobroma-systems.com>; Maxime Ripard > > <maxime.rip...@bootlin.com>; Michael Trimarchi > > <mich...@amarulasolutions.com>; Andre Przywara > > <andre.przyw...@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>; > > linux-amar...@amarulasolutions.com; dl-uboot-imx <uboot-...@nxp.com> > > Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK > > support > > > > On Tue, 23 Apr 2019 07:47:38 +0000 > > Peng Fan <peng....@nxp.com> wrote: > > > > > Hi Lukasz, > > > > > > > -----Original Message----- > > > > From: Lukasz Majewski [mailto:lu...@denx.de] > > > > Sent: 2019年4月20日 6:18 > > > > To: Peng Fan <peng....@nxp.com> > > > > Cc: Jagan Teki <ja...@amarulasolutions.com>; Stefano Babic > > > > <sba...@denx.de>; Fabio Estevam <fabio.este...@nxp.com>; Simon > > Glass > > > > <s...@chromium.org>; Tom Rini <tr...@konsulko.com>; Marek Vasut > > > > <marek.vasut+rene...@gmail.com>; Neil Armstrong > > > > <narmstr...@baylibre.com>; Philipp Tomsich > > > > <philipp.toms...@theobroma-systems.com>; Maxime Ripard > > > > <maxime.rip...@bootlin.com>; Michael Trimarchi > > > > <mich...@amarulasolutions.com>; Andre Przywara > > > > <andre.przyw...@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>; > > > > linux-amar...@amarulasolutions.com; dl-uboot-imx > > <uboot-...@nxp.com> > > > > Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK > > > > support > > > > > > > > On Fri, 19 Apr 2019 08:52:28 +0000 > > > > Peng Fan <peng....@nxp.com> wrote: > > > > > > > > > > On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki > > > > > > <ja...@amarulasolutions.com> wrote: > > > > > > > > > > > > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski > > > > > > > <lu...@denx.de> wrote: > > > > > > > > > > > > > > > > Hi Jagan, > > > > > > > > > > > > > > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski > > > > > > > > > <lu...@denx.de> wrote: > > > > > > > > > > > > > > > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki > > > > > > > > > > <ja...@amarulasolutions.com> wrote: > > > > > > > > > > > > > > > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski > > > > > > > > > > > <lu...@denx.de> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > > > > > > > > > > > <ja...@amarulasolutions.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > This is revised version of previous i.MX6 > > > > > > > > > > > > > clock management [1]. > > > > > > > > > > > > > > > > > > > > > > > > > > The main difference between previous version > > > > > > > > > > > > > is > > > > > > > > > > > > > - Group the i.MX6 ccm clocks into gates and > > > > > > > > > > > > > tree instead of handling the clocks in simple > > > > > > > > > > > > > way using case statement. > > > > > > > > > > > > > - use gate clocks for enable/disable > > > > > > > > > > > > > management. > > > > > > > > > > > > > - use tree clocks for get/set rate or parent > > > > > > > > > > > > > traverse management. > > > > > > > > > > > > > - parent clock handling via clock type. > > > > > > > > > > > > > - traverse the parent clock using recursive > > > > > > > > > > > > > functionlaity. > > > > > > > > > > > > > > > > > > > > > > > > > > The main motive behind this tree framework is > > > > > > > > > > > > > to make the clock tree management simple and > > > > > > > > > > > > > useful for U-Boot requirements instead of > > > > > > > > > > > > > garbing Linux clock management code. > > > > > > > > > > > > > > > > > > > > > > > > > > We are trying to manage the Allwinner clocks > > > > > > > > > > > > > with similar kind, so having this would > > > > > > > > > > > > > really help i.MX6 as well. > > > > > > > > > > > > > > > > > > > > > > > > > > Added simple names for clock macros, but will > > > > > > > > > > > > > update it in future version. > > > > > > > > > > > > > > > > > > > > > > > > > > I have skipped ENET clocks from previous > > > > > > > > > > > > > series, will add it in future patches. > > > > > > > > > > > > > > > > > > > > > > > > > > Changes for v2: > > > > > > > > > > > > > - changed framework patches. > > > > > > > > > > > > > - add support for imx6qdl and imx6ul boards > > > > > > > > > > > > > - add clock gates, tree. > > > > > > > > > > > > > > > > > > > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/ > > > > > > > > > > > > > > > > > > > > > > > > > > Any inputs? > > > > > > > > > > > > > > > > > > > > > > > > Hmm.... It looks like we are doing some > > > > > > > > > > > > development in parallel. > > > > > > > > > > > > > > > > > > > > > > > > Please look into following commit [1]: > > > > > > > > > > > > https://patchwork.ozlabs.org/patch/1034051/ > > > > > > > > > > > > > > > > > > > > > > > > It ports from Linux 5.0 the CCF framework for > > > > > > > > > > > > iMX6Q, which IMHO in the long term is a better > > > > > > > > > > > > approach. The code is kept simple and resembles > > > > > > > > > > > > the code from Barebox. > > > > > > > > > > > > > > > > > > > > > > > > Please correct me if I'm wrong, but the code > > > > > > > > > > > > from your work is not modeling muxes, gates and > > > > > > > > > > > > other components from Linux CCF. > > > > > > > > > > > > > > > > > > > > > > The U-Boot implementation of CLK would require as > > > > > > > > > > > minimal and simple as possible due to requirement > > > > > > > > > > > of U-Boot itself. Hope you agree this point? > > > > > > > > > > > > > > > > > > > > Now i.MX6 is using clock.c CLK implementation. If we > > > > > > > > > > decide to replace it - we shall do it in a way, > > > > > > > > > > which would allow us to follow Linux kernel. (the > > > > > > > > > > barebox implementation is a stripped CCF from > > > > > > > > > > Linux, the same is in patch [1]). > > > > > > > > > > > if yes having CCF stack code to handle all clock > > > > > > > > > > > with respective separate drivers management is > > > > > > > > > > > may not require as of now, IMHO. > > > > > > > > > > > > > > > > > > > > I do have a gut feeling, that we will end up with > > > > > > > > > > the need to have the CCF framework ported anyway. > > > > > > > > > > As for example imx7/8 can re-use muxes, gates > > > > > > > > > > code. > > > > > > > > > > > > > > > > > > As per my experience the main the over-ahead to handle > > > > > > > > > clocks in U-Boot if we go with separate clock drivers > > > > > > > > > is for Video and Ethernet peripherals. these are key > > > > > > > > > IP's which use more clocks from U-Boot point-of-view, > > > > > > > > > others can be handle pretty straight-forward unless > > > > > > > > > if they don't have too much tree chain. > > > > > > > > > > > > > > > > > > On this series, the tree management is already > > > > > > > > > supported ENET in i.MX6, and Allwinner platforms. > > > > > > > > > > > > > > > > > > As of now, I'm thinking I can handle reset of the > > > > > > > > > clocks with similar way. > > > > > > > > > > > > > > > > But this code also supports ENET and ESDHCI clocks on > > > > > > > > i.MX6Q (as supporting those was the motivator for this > > > > > > > > work). > > > > > > > > > > > > > > > > One important thing to be aware of - the problem with > > > > > > > > SPL's footprint. The implementation with clock.c is > > > > > > > > small and simple, but doesn't scale well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > However, those are only my "feelings" after a > > > > > > > > > > glimpse look > > > > > > > > > > - I will look into your code more thoroughly and > > > > > > > > > > provide feedback. > > > > > > > > > > > > > > > > > > Please have a look, if possible check even the code > > > > > > > > > size by adding USDHC clocks. > > > > > > > > > > > > > > > > Yes, code size (especially in SPL) is an _important_ > > > > > > > > factor here. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This series is using recursive calls for handling > > > > > > > > > > > parenting stuff to handle get or set rates, which > > > > > > > > > > > is fine for handling clock tree management as far > > > > > > > > > > > as U-Boot point-of-view. We have faced similar > > > > > > > > > > > situation as I explained in commit message about > > > > > > > > > > > Allwinner clocks [2] and we ended up going this > > > > > > > > > > > way. > > > > > > > > > > > > > > > > > > > > I'm not Allwinner expert - but if I may ask - how > > > > > > > > > > far away is this implementation from mainline Linux > > > > > > > > > > kernel? > > > > > > > > > > > > > > > > > > > > How difficult is it to port the new code (or update > > > > > > > > > > it)? > > > > > > > > > > > > > > > > > > Allwinner clocks also has similar gates, muxs, and > > > > > > > > > with other platform stuff which has too much scope in > > > > > > > > > Linux to use CCM. > > > > > > > > > > > > > > > > For example the barebox managed to get subset of Linux > > > > > > > > CCF ported, without loosing the CCF similarity. > > > > > > > > > > > > > > > > > > > > > > > > Important factors/requirements for the i.MX clock code: > > > > > > > > > > > > > > > > 1. Easy maintenance in long-term > > > > > > > > > > > > > > > > 2. Reusing the code in SPL (with a very important > > > > > > > > factor of _code_size_). > > > > > > > > > > > > > > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8) > > > > > > > > > > > > > > > > 4. Effort needed to use DM with this code > > > > > > > > > > > > > > I understand your points, I was managed this series based > > > > > > > on these requirements as well. > > > > > > > > > > > > Ok. > > > > > > > > > > > > Could you share the delta of footprint size (u-boot.img/SPL) > > > > > > with and without your patch (on imx6q) ? > > > > > > > > > > > > In my case the CCF caused increase of u-boot.img proper (as > > > > > > it was not yet adapted to SPL): > > > > > > > > > > > > 415KiB -> 421KiB = 6KiB increase of size (< 2%). > > > > > > > > > > > > (This can be further reduced by using OF_PLATDATA). > > > > > > > > > > > > This CCF code hasn't been ported to SPL (yet) > > > > > > > > > > > > > We even consider the foot-print, atleast for recursive > > > > > > > calls of handling parenting scale well. > > > > > > > > > > > > With CCF porting v3 I'm going to provide some caching, so > > > > > > the descending would be done at most once. > > > > > > > > > > > > > May be we can > > > > > > > consider to design based on this as per U-Boot. > > > > > > > > > > > > > > > > > > > Please look into point 1. Having code ported from Linux is > > > > > > IMHO better in the long term. > > > > > > > > > > Agree. > > > > > > > > > > > > > > > > > > Let me come-back with another series or do you have any > > > > > > > inputs or questions, please post it. > > > > > > > > > > > > I will post CCF port for imx6q v3 in a few days. > > > > > > > > > > Looking forward your new patchset. > > > > > Working on enabling i.MX8MM CCF support. > > > > > > > > Output of 'dm tree' on imx6q: > > > > > > > > clk 1 [ ] fixed_rate_clock |-- ckil > > > > clk 2 [ ] fixed_rate_clock |-- ckih1 > > > > clk 3 [ + ] fixed_rate_clock `-- osc > > > > clk 4 [ + ] imx_clk_pllv3 |-- pll2_bus > > > > clk 7 [ ] imx_clk_pfd | |-- > > > > pll2_pfd0_352m > > > > clk 8 [ + ] imx_clk_pfd | `-- > > > > pll2_pfd2_396m > > > > clk 9 [ + ] imx_clk_mux | |-- > > > > usdhc1_sel > > > > clk 13 [ + ] imx_clk_divider | | > > `-- > > > > usdhc1_podf > > > > clk 22 [ ] imx_clk_gate2 | | > > > > `-- usdhc1 > > > > clk 10 [ + ] imx_clk_mux | |-- > > > > usdhc2_sel > > > > clk 14 [ + ] imx_clk_divider | | > > `-- > > > > usdhc2_podf > > > > clk 23 [ ] imx_clk_gate2 | | > > > > `-- usdhc2 > > > > clk 11 [ + ] imx_clk_mux | |-- > > > > usdhc3_sel > > > > clk 15 [ + ] imx_clk_divider | | > > `-- > > > > usdhc3_podf > > > > clk 24 [ ] imx_clk_gate2 | | > > > > `-- usdhc3 > > > > clk 12 [ + ] imx_clk_mux | `-- > > > > usdhc4_sel > > > > clk 16 [ + ] imx_clk_divider | > > `-- > > > > usdhc4_podf > > > > clk 25 [ ] imx_clk_gate2 | > > > > `-- usdhc4 > > > > clk 5 [ + ] imx_clk_pllv3 `-- > > > > pll3_usb_otg clk 6 [ + ] > > > > imx_clk_fixed_factor `-- pll3_60m clk 17 [ + > > > > ] imx_clk_divider `-- ecspi_root > > > > clk 18 [ ] imx_clk_gate2 > > |-- > > > > ecspi1 > > > > clk 19 [ ] imx_clk_gate2 > > |-- > > > > ecspi2 > > > > clk 20 [ ] imx_clk_gate2 > > |-- > > > > ecspi3 > > > > clk 21 [ ] imx_clk_gate2 > > `-- > > > > ecspi4 > > > > > > > > > > Do you have a public tree/branch for CCF? > > > > Please find the CCF devel work: > > https://github.com/lmajewski/u-boot-dfu/commits/CCF-devel > > > > The CCF starts from: c792297e1a47ead02ff2baa4f162de8782b29910 > > Thanks. > > > > > (below you will find imx6q DM/DTS conversion code). > > > > What is added when compared to the original one: > > > > - SPL support > > > > - Some fixes for v2019.04+ > > > > What is on the TO DO list: > > > > - OF_PLATDATA for SPL (as I did not used any optimisations yet). > > > > > > >I am adding imx8mm clk and > > > would like to base on your tree. I think need to extend clk_ops to > > >support mux/divider, but not just get rate. > > > > Some mux/divider is provided (clk-mux.c / clk-divider.c) > > After reading into details, there is no clk core logic. > mux not support reparenting. > Divider not support rate setting. > no composite clk support.
I've stripped out the code, which I couldn't test/needed on imx6q. This version only has the "must have" for imx6q (including SPL) with some generic code for clock framework in u-boot. > We could not directly reuse Linux vendor clk driver, > need adapt to > U-Boot, Yes, you would need to adapt the code (as it was done in barebox and this patch set). > should not be hard from your code. I wanted to keep the structure of the code untouched from Linux. However, for example, we can't afford in imx6q the clk[IMX6QDL_<clock_name>] table - it is simply too big for SPL. > > When multiple devices sources from one PLL, one device might would > like to set pll value that break other devices, there is no logic to > protect if support freq changing. Ok, so in your use-case you need the frequency re-calc code? (Which changes - updates - the clocks up in the tree when root PLL is changed) ? No, it was not added. > > Do you have plan work on the upper items? For the imx6q use case - in which I need some muxing and gating only - I do not need to recalc/reparent the frequency. The CCF porting patch only brought: - the smallest subset of CCF functions for my use case - directory (and source files) structure from Linux - some clock uclass enhancement for u-boot - the clock generic (register) code (clk/clk.c) The plan was to allow others to port their subset of code when needed. In that way we only get the code which is really used and tested. > i.MX8MM use composite heavily in Linux, so I have started. Ok. I see. Your patches/review is more than welcome. > > Thanks, > Peng. > > > > > > To avoid conflict with > > > you work, if you have a public tree, that could be good. > > > > No problem. Thanks for the interest. > > > > > > > > Thanks, > > > Peng. > > > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > Peng. > > > > > > > > > > > > > > > > > > > > > > > > > Jagan. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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: lu...@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: lu...@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: > > lu...@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: lu...@denx.de
pgpxoHK9HZhEn.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot