[U-Boot] [PATCH v1 06/15] dm: clk: imx: Add support for controlling imx6q clocks via Driver Model
Lukasz Majewski
lukma at denx.de
Thu Jan 31 12:59:26 UTC 2019
Hi Jagan,
> On Tue, Jan 29, 2019 at 1:35 PM Lukasz Majewski <lukma at denx.de> wrote:
> >
> > Hi Jagan,
> >
> > > On Tue, Jan 29, 2019 at 12:46 PM Lukasz Majewski <lukma at denx.de>
> > > wrote:
> > > >
> > > > Hi Stefano, Fabio,
> > > >
> > > > > Hi Lukasz,
> > > > >
> > > > > On 21/01/19 15:19, Lukasz Majewski wrote:
> > > > > > Hi Fabio,
> > > > > >
> > > > > >> Hi Lukasz,
> > > > > >>
> > > > > >> On Sat, Jan 19, 2019 at 7:15 AM Lukasz Majewski
> > > > > >> <lukma at denx.de> wrote:
> > > > > >>> +static ulong imx6q_clk_get_rate(struct clk *clk)
> > > > > >>> +{
> > > > > >>> + ulong rate = 0;
> > > > > >>> +
> > > > > >>> + debug("%s(#%lu)\n", __func__, clk->id);
> > > > > >>> +
> > > > > >>> + switch (clk->id) {
> > > > > >>> + case IMX6QDL_CLK_ECSPI1:
> > > > > >>> + case IMX6QDL_CLK_ECSPI2:
> > > > > >>> + case IMX6QDL_CLK_ECSPI3:
> > > > > >>> + case IMX6QDL_CLK_ECSPI4:
> > > > > >>> + return imx6_get_cspi_clk();
> > > > > >>> +
> > > > > >>> + case IMX6QDL_CLK_USDHC1:
> > > > > >>> + case IMX6QDL_CLK_USDHC2:
> > > > > >>> + case IMX6QDL_CLK_USDHC3:
> > > > > >>> + case IMX6QDL_CLK_USDHC4:
> > > > > >>> + return imx6_get_usdhc_clk(clk->id -
> > > > > >>> IMX6QDL_CLK_USDHC1);
> > > > > >>
> > > > > >> I don't think this scales well as this needs to grow for
> > > > > >> all other peripherals and for each port instance.
> > > > > >
> > > > >
> > > > > I am hiiting the same doubts as Fabio when I reviewed Peng's
> > > > > patches with clocks for i.MX8M. The current approach was
> > > > > introduced some years ago with i.MX5, but it does not fit well
> > > > > now and it does not scale.
> > > > >
> > > > >
> > > > > > The rationale regarding this approach:
> > > > > >
> > > > > > 1. Reuse the clock.c code for iMX6Q as much as possible.
> > > > > >
> > > > > > 2, This code is based on the clk-imx8q.c file - hence the
> > > > > > question why the Linux clock API was not ported for this new
> > > > > > SoC?.
> > > > >
> > > > > Many reasons, first there was already a set of functions to
> > > > > get / set clocks coming from previos i.MX platforms, and this
> > > > > API was simply reused. And nobody was in charge to port the
> > > > > clock API.
> > > > > >
> > > > > >>
> > > > > >> If we are adding a clock driver for mx6, why don't we add
> > > > > >> it just like the kernel one?
> > > > > >
> > > > > > I can try to port the Linux code, but IMHO it would be
> > > > > > feasible to port only relevant (ECSPI, USDHC) parts of it
> > > > > > (not all as I cannot test it all properly).
> > > > >
> > > > > Fully agree. Further parts will be added on demand. Less is
> > > > > better. I disagree to add not tested code.
> > > >
> > > > The work is in progress - I will add the same directory
> > > > structure as Linux's Common Clock Framework [CCF]. I shall
> > > > finish in 2-3 days.
> > > >
> > > > There are a few problems to tackle (when porting code from
> > > > Linux):
> > >
> > > Just to add my experience with previous CLK support series[1]. Do
> > > we really need to port it from Linux, because I'm thinking we can
> > > manage it as simple as other SoC support in U-Boot.(I have few
> > > clocks addition on this series other than ENET)
> > >
> > > What do you think?
> >
> > This patch series seems to me like the one which reuses the "large"
> > switch/case approach with "fsl,imx6q-ccm" driver [*].
> >
> > The ENET shall also be handled by the CCF from Linux.
> >
> > Another problem - the U-boot's clock DM API is not supporting
> > passing parent clock rate, which is important in the hierarchical
> > clock tree. The Linux CCF is a hierarchical one built in the
> > driver's [*] probe.
>
> Understand, we need to taken care the parent clock rates as well for
> set and get rates. In fact we tried similar thing with sunxi, patch[2]
> implements get_rate by taking parent clock rates in to account by
> using switch statements. and this change[3] do add same but with
> recursive calls. Since U-Boot need minimal clocks and most of them run
> with default parents it better to implement as smooth as possible
> otherwise it would ended-up size and complex issues, IMHO.
>
> I think other platforms like mediatek clocks are also following same.
I've posted some patches today:
http://patchwork.ozlabs.org/cover/1034127/
>
> >
> >
> >
> > Do we need to port the (trimmed) Common Clock Framework [CCF] from
> > Linux?
> >
> > With the first version of this patch I also wanted to re-use the
> > code from clock.c imx6 file. This would be good enough for now.
> >
> > However, in the long term (and taking into consideration the fact
> > that imx6 needs to be converted to DM anyway) it may be beneficial
> > to re-use CCF.
> >
> > The _real_ problem is to fit it and reuse in SPL (to avoid footprint
> > size regression). The CCF port will work correctly in u-boot proper,
> > but for SPL we would need some hacks (as e.g. by default we now
> > strip clock properties from DTS when appending to SPL).
> >
> > Now SPLs for IMX6 use solid code without any unneeded stuff (and
> > hence we didn't need TPL...) .
>
> Yes, all these size issues might occur even in future if we port large
> things from Linux.
>
> [2] https://patchwork.ozlabs.org/patch/1019673/
> [3]
> https://gist.github.com/apritzel/db93dd06b4defb46504bccbfe4fc2c20#file-sunxi_clk-c-L86-L112
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190131/6c1ddcee/attachment.sig>
More information about the U-Boot
mailing list