Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski lukma@denx.de wrote:
Hi Jagan,
On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski lukma@denx.de wrote:
On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski lukma@denx.de wrote: > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > jagan@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:
Easy maintenance in long-term
Reusing the code in SPL (with a very important factor of
_code_size_).
Reuse the code for other i.MX SoCs (imx7, imx8)
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.
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: lukma@denx.de
On Fri, 19 Apr 2019 08:52:28 +0000 Peng Fan peng.fan@nxp.com wrote:
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski lukma@denx.de wrote:
Hi Jagan,
On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski lukma@denx.de wrote:
On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
> On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski > lukma@denx.de wrote: > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > jagan@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:
Easy maintenance in long-term
Reusing the code in SPL (with a very important factor of
_code_size_).
Reuse the code for other i.MX SoCs (imx7, imx8)
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
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: 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
Hi Lukasz,
-----Original Message----- From: Lukasz Majewski [mailto:lukma@denx.de] Sent: 2019年4月20日 6:18 To: Peng Fan peng.fan@nxp.com Cc: Jagan Teki jagan@amarulasolutions.com; Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; Simon Glass sjg@chromium.org; Tom Rini trini@konsulko.com; Marek Vasut marek.vasut+renesas@gmail.com; Neil Armstrong narmstrong@baylibre.com; Philipp Tomsich philipp.tomsich@theobroma-systems.com; Maxime Ripard maxime.ripard@bootlin.com; Michael Trimarchi michael@amarulasolutions.com; Andre Przywara andre.przywara@arm.com; U-Boot-Denx u-boot@lists.denx.de; linux-amarula@amarulasolutions.com; dl-uboot-imx uboot-imx@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.fan@nxp.com wrote:
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski lukma@denx.de wrote:
Hi Jagan,
On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski lukma@denx.de wrote: > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki > jagan@amarulasolutions.com wrote: > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski > > lukma@denx.de wrote: > > > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > > jagan@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:
Easy maintenance in long-term
Reusing the code in SPL (with a very important factor of
_code_size_).
Reuse the code for other i.MX SoCs (imx7, imx8)
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? 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. To avoid conflict with you work, if you have a public tree, that could be good.
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: 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 Tue, 23 Apr 2019 07:47:38 +0000 Peng Fan peng.fan@nxp.com wrote:
Hi Lukasz,
-----Original Message----- From: Lukasz Majewski [mailto:lukma@denx.de] Sent: 2019年4月20日 6:18 To: Peng Fan peng.fan@nxp.com Cc: Jagan Teki jagan@amarulasolutions.com; Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; Simon Glass sjg@chromium.org; Tom Rini trini@konsulko.com; Marek Vasut marek.vasut+renesas@gmail.com; Neil Armstrong narmstrong@baylibre.com; Philipp Tomsich philipp.tomsich@theobroma-systems.com; Maxime Ripard maxime.ripard@bootlin.com; Michael Trimarchi michael@amarulasolutions.com; Andre Przywara andre.przywara@arm.com; U-Boot-Denx u-boot@lists.denx.de; linux-amarula@amarulasolutions.com; dl-uboot-imx uboot-imx@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.fan@nxp.com wrote:
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski lukma@denx.de wrote:
Hi Jagan,
> On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski > lukma@denx.de wrote: > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki > > jagan@amarulasolutions.com wrote: > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski > > > lukma@denx.de wrote: > > > > > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > > > jagan@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:
Easy maintenance in long-term
Reusing the code in SPL (with a very important factor of
_code_size_).
Reuse the code for other i.MX SoCs (imx7, imx8)
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
(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)
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: 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年4月23日 16:46 To: Peng Fan peng.fan@nxp.com Cc: Jagan Teki jagan@amarulasolutions.com; Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; Simon Glass sjg@chromium.org; Tom Rini trini@konsulko.com; Marek Vasut marek.vasut+renesas@gmail.com; Neil Armstrong narmstrong@baylibre.com; Philipp Tomsich philipp.tomsich@theobroma-systems.com; Maxime Ripard maxime.ripard@bootlin.com; Michael Trimarchi michael@amarulasolutions.com; Andre Przywara andre.przywara@arm.com; U-Boot-Denx u-boot@lists.denx.de; linux-amarula@amarulasolutions.com; dl-uboot-imx uboot-imx@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.fan@nxp.com wrote:
Hi Lukasz,
-----Original Message----- From: Lukasz Majewski [mailto:lukma@denx.de] Sent: 2019年4月20日 6:18 To: Peng Fan peng.fan@nxp.com Cc: Jagan Teki jagan@amarulasolutions.com; Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; Simon
Glass
sjg@chromium.org; Tom Rini trini@konsulko.com; Marek Vasut marek.vasut+renesas@gmail.com; Neil Armstrong narmstrong@baylibre.com; Philipp Tomsich philipp.tomsich@theobroma-systems.com; Maxime Ripard maxime.ripard@bootlin.com; Michael Trimarchi michael@amarulasolutions.com; Andre Przywara andre.przywara@arm.com; U-Boot-Denx u-boot@lists.denx.de; linux-amarula@amarulasolutions.com; dl-uboot-imx
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.fan@nxp.com wrote:
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski lukma@denx.de wrote: > > Hi Jagan, > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski > > lukma@denx.de wrote: > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki > > > jagan@amarulasolutions.com wrote: > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski > > > > lukma@denx.de wrote: > > > > > > > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > > > > jagan@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. We could not directly reuse Linux vendor clk driver, need adapt to U-Boot, should not be hard from your code.
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.
Do you have plan work on the upper items? i.MX8MM use composite heavily in Linux, so I have started.
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: 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
Hi Peng,
-----Original Message----- From: Lukasz Majewski [mailto:lukma@denx.de] Sent: 2019年4月23日 16:46 To: Peng Fan peng.fan@nxp.com Cc: Jagan Teki jagan@amarulasolutions.com; Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; Simon Glass sjg@chromium.org; Tom Rini trini@konsulko.com; Marek Vasut marek.vasut+renesas@gmail.com; Neil Armstrong narmstrong@baylibre.com; Philipp Tomsich philipp.tomsich@theobroma-systems.com; Maxime Ripard maxime.ripard@bootlin.com; Michael Trimarchi michael@amarulasolutions.com; Andre Przywara andre.przywara@arm.com; U-Boot-Denx u-boot@lists.denx.de; linux-amarula@amarulasolutions.com; dl-uboot-imx uboot-imx@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.fan@nxp.com wrote:
Hi Lukasz,
-----Original Message----- From: Lukasz Majewski [mailto:lukma@denx.de] Sent: 2019年4月20日 6:18 To: Peng Fan peng.fan@nxp.com Cc: Jagan Teki jagan@amarulasolutions.com; Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; Simon
Glass
sjg@chromium.org; Tom Rini trini@konsulko.com; Marek Vasut marek.vasut+renesas@gmail.com; Neil Armstrong narmstrong@baylibre.com; Philipp Tomsich philipp.tomsich@theobroma-systems.com; Maxime Ripard maxime.ripard@bootlin.com; Michael Trimarchi michael@amarulasolutions.com; Andre Przywara andre.przywara@arm.com; U-Boot-Denx u-boot@lists.denx.de; linux-amarula@amarulasolutions.com; dl-uboot-imx
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.fan@nxp.com wrote:
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
> On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski > lukma@denx.de wrote: > > > > Hi Jagan, > > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski > > > lukma@denx.de wrote: > > > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki > > > > jagan@amarulasolutions.com wrote: > > > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski > > > > > lukma@denx.de wrote: > > > > > > > > > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > > > > > jagan@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: 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 Fri, Apr 19, 2019 at 08:52:28AM +0000, Peng Fan wrote:
On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski lukma@denx.de wrote:
Hi Jagan,
On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski lukma@denx.de wrote:
On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki jagan@amarulasolutions.com wrote:
> On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski lukma@denx.de > wrote: > > > > On Tue, 2 Apr 2019 16:58:33 +0530 Jagan Teki > > jagan@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:
Easy maintenance in long-term
Reusing the code in SPL (with a very important factor of
_code_size_).
Reuse the code for other i.MX SoCs (imx7, imx8)
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.
I just want to re-iterate my support again here that we should be looking at adapting and stripping down frameworks from the kernel. They are: - Familiar to a large subset of our developers as most folks also work on the Linux kernel. - Already (generally) well designed to take into account the various designs of vastly different SoCs that we also want to support. - Likely to already be fed by device tree and we can just leverage what's already in the dts* files.
participants (3)
-
Lukasz Majewski -
Peng Fan -
Tom Rini