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

[Lukasz Majewski]

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

1. As it is now - for IMX6Q we need a static table to store clock
pointers. It is around 1KiB of SRAM/SDRAM. This is _a_lot_ for SPL.
For u-boot proper this is not a huge problem.

2. The SPL shall use OF_PLATDATA, as we do need clock management mostly
in SPL. Moreover, I do think that SPL will be bigger (this is a _real_
problem, IMHO).

3. The CCF design needs to be fitted into U-Boot's Driver Model - this
is achievable. The most annoying problem is the lack of .get_rate()
method in the CCF's API.

4. The CCF driver for imx6q uses 

clks: ccm at 020c4000 {

}

Node as a "manager". In its probe we create other (proper) clocks
(hierarchy). Also, this driver is a starting point for getting access
to other clocks.

As said above - I will first post the CCF port very similar to
Barebox/Linux. Afterwards, we can decide if and how we optimise it. 

> 
> >   
> >>
> >> Barebox imports the clock driver from the kernel and it is much
> >> cleaner:
> >> https://git.pengutronix.de/cgit/barebox/tree/drivers/clk/imx/clk-imx6.c  
> > 
> > Yes, it has been trimmed (...a bit...) when compared to original
> > v4.20 :-) .
> >   
> Best regards,
> Stefano
> 




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/e3c34ce3/attachment.sig>