[PATCH 01/10] clk: imx6q: Properly handle imx6qp ESPI clk_sels

[Adam Ford]

On Mon, Mar 3, 2025 at 6:34 AM Michael Nazzareno Trimarchi
<michael at amarulasolutions.com> wrote:
>
> Hi Adam
>
> On Sun, Mar 2, 2025 at 5:53 PM Adam Ford <aford173 at gmail.com> wrote:
>>
>> The ECSPI clock has the ability to select between pll3_60m and
>> osc on the imx6qp, where it's fixed on other variants.  Fix this
>> by adding using a helper function to determine SoC variant and
>> register the clock accordingly.
>>
>> Signed-off-by: Adam Ford <aford173 at gmail.com>
>> ---
>>  drivers/clk/imx/clk-imx6q.c | 22 +++++++++++++++++++---
>>  1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
>> index df9f0285e1e..de5e5d8132b 100644
>> --- a/drivers/clk/imx/clk-imx6q.c
>> +++ b/drivers/clk/imx/clk-imx6q.c
>> @@ -13,6 +13,11 @@
>>
>>  #include "clk.h"
>>
>> +static inline int clk_on_imx6qp(void)
>> +{
>> +       return of_machine_is_compatible("fsl,imx6qp");
>> +}
>> +
>
>
> It's static, why just not use direct compatible?

I looked at how it was done in Linux, and it seemed easier to read.
I can look at alternatives too.  There might already be a helper
function.

adam
>
>>
>>  static int imx6q_clk_request(struct clk *clk)
>>  {
>>         if (clk->id < IMX6QDL_CLK_DUMMY || clk->id >= IMX6QDL_CLK_END) {
>> @@ -35,6 +40,7 @@ static const char *const usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", };
>>  static const char *const periph_sels[] = { "periph_pre", "periph_clk2", };
>>  static const char *const periph_pre_sels[] = { "pll2_bus", "pll2_pfd2_396m",
>>                                                "pll2_pfd0_352m", "pll2_198m", };
>> +static const char *const ecspi_sels[] = { "pll3_60m", "osc", };
>>
>>  static int imx6q_clk_probe(struct udevice *dev)
>>  {
>> @@ -78,6 +84,12 @@ static int imx6q_clk_probe(struct udevice *dev)
>>                imx_clk_mux("usdhc4_sel", base + 0x1c, 19, 1,
>>                            usdhc_sels, ARRAY_SIZE(usdhc_sels)));
>>
>> +       if (clk_on_imx6qp()) {
>> +               clk_dm(IMX6QDL_CLK_ECSPI_SEL,
>> +                      imx_clk_mux("ecspi_sel", base + 0x38, 18, 1, ecspi_sels,
>> +                                  ARRAY_SIZE(ecspi_sels)));
>> +       }
>> +
>>         clk_dm(IMX6QDL_CLK_USDHC1_PODF,
>>                imx_clk_divider("usdhc1_podf", "usdhc1_sel",
>>                                base + 0x24, 11, 3));
>> @@ -91,9 +103,13 @@ static int imx6q_clk_probe(struct udevice *dev)
>>                imx_clk_divider("usdhc4_podf", "usdhc4_sel",
>>                                base + 0x24, 22, 3));
>>
>> -       clk_dm(IMX6QDL_CLK_ECSPI_ROOT,
>> -              imx_clk_divider("ecspi_root", "pll3_60m", base + 0x38, 19, 6));
>> -
>> +       if (clk_on_imx6qp()) {
>> +               clk_dm(IMX6QDL_CLK_ECSPI_ROOT,
>> +                      imx_clk_divider("ecspi_root", "ecspi_sel", base + 0x38, 19, 6));
>> +       } else {
>> +               clk_dm(IMX6QDL_CLK_ECSPI_ROOT,
>> +                      imx_clk_divider("ecspi_root", "pll3_60m", base + 0x38, 19, 6));
>> +       }
>>         clk_dm(IMX6QDL_CLK_ECSPI1,
>>                imx_clk_gate2("ecspi1", "ecspi_root", base + 0x6c, 0));
>>         clk_dm(IMX6QDL_CLK_ECSPI2,
>
>
> Apart of that I'm almost fine with it.
>
> Reviewed-By: Michael Trimarchi <michael at amarulasolutions.com>
>
>>
>> --
>> 2.45.2
>>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael at amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info at amarulasolutions.com
> www.amarulasolutions.com