[U-Boot] [PATCH v1 06/15] dm: clk: imx: Add support for controlling imx6q clocks via Driver Model

[Lukasz Majewski <lukma@denx.de>]

Hi Jagan,

> On Tue, Jan 29, 2019 at 12:46 PM Lukasz Majewski <lukma@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@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.



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...) .

> 
> [1] https://patchwork.ozlabs.org/cover/950964/


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/20190129/649a9023/attachment.sig>