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

[Stefano Babic]

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.

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

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================