Hi
On Mon, Mar 3, 2025 at 1:54 PM Adam Ford <aford...@gmail.com> wrote:
> On Mon, Mar 3, 2025 at 6:34 AM Michael Nazzareno Trimarchi
> <mich...@amarulasolutions.com> wrote:
> >
> > Hi Adam
> >
> > On Sun, Mar 2, 2025 at 5:53 PM Adam Ford <aford...@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 <aford...@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.
>
>
It's ok with other patches, I have seen the rest. I can test on some imx6
Michael
> 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 <mich...@amarulasolutions.com>
> >
> >>
> >> --
> >> 2.45.2
> >>
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > mich...@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > i...@amarulasolutions.com
> > www.amarulasolutions.com
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
