[PATCH 03/18] clk: imx: clk-imxrt1050: setup PLL5 for video in non-SPL

[Giulio Benetti]

Hi Fabio,

On 2/26/20 6:37 PM, Fabio Estevam wrote:
> Hi Giulio,
> 
> On Wed, Feb 26, 2020 at 2:16 PM Giulio Benetti
> <giulio.benetti at benettiengineering.com> wrote:
>>
>> mxsfb needs PLL5 as source, so let's setup it and set it as source for
>> mxsfb(lcdif).
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti at benettiengineering.com>
>> ---
>>   drivers/clk/imx/clk-imxrt1050.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/imx/clk-imxrt1050.c b/drivers/clk/imx/clk-imxrt1050.c
>> index e33d426363..2819ffb9ac 100644
>> --- a/drivers/clk/imx/clk-imxrt1050.c
>> +++ b/drivers/clk/imx/clk-imxrt1050.c
>> @@ -238,9 +238,9 @@ static int imxrt1050_clk_probe(struct udevice *dev)
>>          clk_dm(IMXRT1050_CLK_LCDIF,
>>                 imx_clk_gate2("lcdif", "lcdif_podf", base + 0x70, 28));
>>
>> -#ifdef CONFIG_SPL_BUILD
>>          struct clk *clk, *clk1;
>>
>> +#ifdef CONFIG_SPL_BUILD
>>          /* bypass pll1 before setting its rate */
>>          clk_get_by_id(IMXRT1050_CLK_PLL1_REF_SEL, &clk);
>>          clk_get_by_id(IMXRT1050_CLK_PLL1_BYPASS, &clk1);
>> @@ -271,7 +271,18 @@ static int imxrt1050_clk_probe(struct udevice *dev)
>>
>>          clk_get_by_id(IMXRT1050_CLK_PLL3_BYPASS, &clk1);
>>          clk_set_parent(clk1, clk);
>> +#else
>> +       /* Set PLL5 for LCDIF to its default 650Mhz */
>> +       clk_get_by_id(IMXRT1050_CLK_PLL5_VIDEO, &clk);
>> +       clk_enable(clk);
>> +       clk_set_rate(clk, 650000000UL);
>> +
>> +       clk_get_by_id(IMXRT1050_CLK_PLL5_BYPASS, &clk1);
>> +       clk_set_parent(clk1, clk);
>>
>> +       /* Configure PLL5 as LCDIF source */
>> +       clk_get_by_id(IMXRT1050_CLK_LCDIF_SEL, &clk1);
>> +       clk_set_parent(clk1, clk);
> 
> This is more like a board design decision and IMHO should not be
> hardcoded as part of the clock driver.
> 
> Other users may want to use a different clock source for the eLCDIF driver.
> 
> Setting the clock parent in board device tree makes more sense.

Yes, it's a good idea. Doing this I've taken this[1] as example.
So I don't know where in u-boot PLLs are initialized according to a dts 
file, can you please provide me an example? I will be happy to modify 
this according to that!

Thank you

[1]: 
https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/clk/imx/clk-imx8mm.c#L398-409

Best regards
-- 
Giulio Benetti
Benetti Engineering sas