| Message ID | 1526959560-6014-1-git-send-email-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 22, 2018 at 12:25 AM, Anson Huang <Anson.Huang@nxp.com> wrote: > i.MX6UL has GPIO clock gates in CCM CCGR, add > them into clock tree for clock management. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Hi Anson, > Anson Huang <Anson.Huang@nxp.com> hat am 22. Mai 2018 um 05:25 geschrieben: > > > i.MX6UL has GPIO clock gates in CCM CCGR, add > them into clock tree for clock management. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > drivers/clk/imx/clk-imx6ul.c | 5 +++++ > include/dt-bindings/clock/imx6ul-clock.h | 31 ++++++++++++++++++------------- > 2 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c > index ba563ba..3ea2d97 100644 > --- a/drivers/clk/imx/clk-imx6ul.c > +++ b/drivers/clk/imx/clk-imx6ul.c > @@ -360,6 +360,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) > clks[IMX6UL_CLK_UART2_SERIAL] = imx_clk_gate2("uart2_serial", "uart_podf", base + 0x68, 28); > if (clk_on_imx6ull()) > clks[IMX6UL_CLK_AIPSTZ3] = imx_clk_gate2("aips_tz3", "ahb", base + 0x80, 18); > + clks[IMX6UL_CLK_GPIO2] = imx_clk_gate2("gpio2", "ipg", base + 0x68, 30); > > /* CCGR1 */ > clks[IMX6UL_CLK_ECSPI1] = imx_clk_gate2("ecspi1", "ecspi_podf", base + 0x6c, 0); > @@ -376,6 +377,8 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) > clks[IMX6UL_CLK_GPT1_SERIAL] = imx_clk_gate2("gpt1_serial", "perclk", base + 0x6c, 22); > clks[IMX6UL_CLK_UART4_IPG] = imx_clk_gate2("uart4_ipg", "ipg", base + 0x6c, 24); > clks[IMX6UL_CLK_UART4_SERIAL] = imx_clk_gate2("uart4_serial", "uart_podf", base + 0x6c, 24); > + clks[IMX6UL_CLK_GPIO1] = imx_clk_gate2("gpio1", "ipg", base + 0x6c, 26); > + clks[IMX6UL_CLK_GPIO5] = imx_clk_gate2("gpio5", "ipg", base + 0x6c, 30); > > /* CCGR2 */ > if (clk_on_imx6ull()) { > @@ -389,6 +392,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) > clks[IMX6UL_CLK_I2C3] = imx_clk_gate2("i2c3", "perclk", base + 0x70, 10); > clks[IMX6UL_CLK_OCOTP] = imx_clk_gate2("ocotp", "ipg", base + 0x70, 12); > clks[IMX6UL_CLK_IOMUXC] = imx_clk_gate2("iomuxc", "lcdif_podf", base + 0x70, 14); > + clks[IMX6UL_CLK_GPIO3] = imx_clk_gate2("gpio3", "ipg", base + 0x70, 26); > clks[IMX6UL_CLK_LCDIF_APB] = imx_clk_gate2("lcdif_apb", "axi", base + 0x70, 28); > clks[IMX6UL_CLK_PXP] = imx_clk_gate2("pxp", "axi", base + 0x70, 30); > > @@ -405,6 +409,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) > clks[IMX6UL_CLK_UART6_IPG] = imx_clk_gate2("uart6_ipg", "ipg", base + 0x74, 6); > clks[IMX6UL_CLK_UART6_SERIAL] = imx_clk_gate2("uart6_serial", "uart_podf", base + 0x74, 6); > clks[IMX6UL_CLK_LCDIF_PIX] = imx_clk_gate2("lcdif_pix", "lcdif_podf", base + 0x74, 10); > + clks[IMX6UL_CLK_GPIO4] = imx_clk_gate2("gpio4", "ipg", base + 0x74, 12); > clks[IMX6UL_CLK_QSPI] = imx_clk_gate2("qspi1", "qspi1_podf", base + 0x74, 14); > clks[IMX6UL_CLK_WDOG1] = imx_clk_gate2("wdog1", "ipg", base + 0x74, 16); > clks[IMX6UL_CLK_MMDC_P0_FAST] = imx_clk_gate("mmdc_p0_fast", "mmdc_podf", base + 0x74, 20); > diff --git a/include/dt-bindings/clock/imx6ul-clock.h b/include/dt-bindings/clock/imx6ul-clock.h > index 9564597..1291328 100644 > --- a/include/dt-bindings/clock/imx6ul-clock.h > +++ b/include/dt-bindings/clock/imx6ul-clock.h > @@ -242,20 +242,25 @@ > #define IMX6UL_CLK_CKO2_PODF 229 > #define IMX6UL_CLK_CKO2 230 > #define IMX6UL_CLK_CKO 231 > +#define IMX6UL_CLK_GPIO1 232 > +#define IMX6UL_CLK_GPIO2 233 > +#define IMX6UL_CLK_GPIO3 234 > +#define IMX6UL_CLK_GPIO4 235 > +#define IMX6UL_CLK_GPIO5 236 this change looks like a breakage of devicetree ABI. You are changing the mean of the existing clock IDs on i.MX6ULL, which probably regress the combination of older DTBs with newer kernel. > > /* For i.MX6ULL */ > -#define IMX6ULL_CLK_ESAI_PRED 232 > -#define IMX6ULL_CLK_ESAI_PODF 233 > -#define IMX6ULL_CLK_ESAI_EXTAL 234 > -#define IMX6ULL_CLK_ESAI_MEM 235 > -#define IMX6ULL_CLK_ESAI_IPG 236 > -#define IMX6ULL_CLK_DCP_CLK 237 > -#define IMX6ULL_CLK_EPDC_PRE_SEL 238 > -#define IMX6ULL_CLK_EPDC_SEL 239 > -#define IMX6ULL_CLK_EPDC_PODF 240 > -#define IMX6ULL_CLK_EPDC_ACLK 241 > -#define IMX6ULL_CLK_EPDC_PIX 242 > -#define IMX6ULL_CLK_ESAI_SEL 243 > -#define IMX6UL_CLK_END 244 > +#define IMX6ULL_CLK_ESAI_PRED 237 > +#define IMX6ULL_CLK_ESAI_PODF 238 > +#define IMX6ULL_CLK_ESAI_EXTAL 239 > +#define IMX6ULL_CLK_ESAI_MEM 240 > +#define IMX6ULL_CLK_ESAI_IPG 241 > +#define IMX6ULL_CLK_DCP_CLK 242 > +#define IMX6ULL_CLK_EPDC_PRE_SEL 243 > +#define IMX6ULL_CLK_EPDC_SEL 244 > +#define IMX6ULL_CLK_EPDC_PODF 245 > +#define IMX6ULL_CLK_EPDC_ACLK 246 > +#define IMX6ULL_CLK_EPDC_PIX 247 > +#define IMX6ULL_CLK_ESAI_SEL 248 > +#define IMX6UL_CLK_END 249 > > #endif /* __DT_BINDINGS_CLOCK_IMX6UL_H */ > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoting Stefan Wahren (2018-05-22 05:25:35) > > +++ b/include/dt-bindings/clock/imx6ul-clock.h > > @@ -242,20 +242,25 @@ > > #define IMX6UL_CLK_CKO2_PODF 229 > > #define IMX6UL_CLK_CKO2 230 > > #define IMX6UL_CLK_CKO 231 > > +#define IMX6UL_CLK_GPIO1 232 > > +#define IMX6UL_CLK_GPIO2 233 > > +#define IMX6UL_CLK_GPIO3 234 > > +#define IMX6UL_CLK_GPIO4 235 > > +#define IMX6UL_CLK_GPIO5 236 > > this change looks like a breakage of devicetree ABI. You are changing the mean of the existing clock IDs on i.MX6ULL, which probably regress the combination of older DTBs with newer kernel. > Agreed. Why can't we just tack on more numbers at the end? > > > > /* For i.MX6ULL */ > > -#define IMX6ULL_CLK_ESAI_PRED 232
Hi Stefan, On Tue, May 22, 2018 at 9:25 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> --- a/include/dt-bindings/clock/imx6ul-clock.h >> +++ b/include/dt-bindings/clock/imx6ul-clock.h >> @@ -242,20 +242,25 @@ >> #define IMX6UL_CLK_CKO2_PODF 229 >> #define IMX6UL_CLK_CKO2 230 >> #define IMX6UL_CLK_CKO 231 >> +#define IMX6UL_CLK_GPIO1 232 >> +#define IMX6UL_CLK_GPIO2 233 >> +#define IMX6UL_CLK_GPIO3 234 >> +#define IMX6UL_CLK_GPIO4 235 >> +#define IMX6UL_CLK_GPIO5 236 > > this change looks like a breakage of devicetree ABI. You are changing the mean of the existing clock IDs on i.MX6ULL, which probably regress the combination of older DTBs with newer kernel. Good point! I will send a fix for f5a4670de96678 ("clk: imx: Add new clo01 and clo2 controlled by CCOSR") which did the same reordering. Thanks
Hi On Sat, Jun 2, 2018 at 3:48 PM, Fabio Estevam <festevam@gmail.com> wrote: > Hi Stefan, > > On Tue, May 22, 2018 at 9:25 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > >>> --- a/include/dt-bindings/clock/imx6ul-clock.h >>> +++ b/include/dt-bindings/clock/imx6ul-clock.h >>> @@ -242,20 +242,25 @@ >>> #define IMX6UL_CLK_CKO2_PODF 229 >>> #define IMX6UL_CLK_CKO2 230 >>> #define IMX6UL_CLK_CKO 231 >>> +#define IMX6UL_CLK_GPIO1 232 >>> +#define IMX6UL_CLK_GPIO2 233 >>> +#define IMX6UL_CLK_GPIO3 234 >>> +#define IMX6UL_CLK_GPIO4 235 >>> +#define IMX6UL_CLK_GPIO5 236 >> >> this change looks like a breakage of devicetree ABI. You are changing the mean of the existing clock IDs on i.MX6ULL, which probably regress the combination of older DTBs with newer kernel. > > Good point! I will send a fix for f5a4670de96678 ("clk: imx: Add new > clo01 and clo2 controlled > by CCOSR") which did the same reordering. > ull is a preatty new platform so one board was listed. Are you sure that we need? Michael > Thanks
Hi Michael, On Sat, Jun 2, 2018 at 11:04 AM, Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > ull is a preatty new platform so one board was listed. Are you sure > that we need? There are several imx6ul based dts in mainline and it is better if we can avoid dtb breakage when possible. In this case we can avoid the dtb breakage by adding the new clock definitions at the end of the file, just like we do for all the other imx devices.
Hi Fabio On Sat, Jun 2, 2018 at 4:07 PM, Fabio Estevam <festevam@gmail.com> wrote: > Hi Michael, > > On Sat, Jun 2, 2018 at 11:04 AM, Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: > >> ull is a preatty new platform so one board was listed. Are you sure >> that we need? > > There are several imx6ul based dts in mainline and it is better if we > can avoid dtb breakage when possible. > > In this case we can avoid the dtb breakage by adding the new clock > definitions at the end of the file, just like we do for all the other > imx devices. Yes, when I add new ul clock I move down ull (that is new), but agree that this is not possible in general. Michael
Hi, Stephen Anson Huang Best Regards! > -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] > Sent: Saturday, June 2, 2018 2:19 PM > To: Anson Huang <anson.huang@nxp.com>; Stefan Wahren > <stefan.wahren@i2se.com>; Fabio Estevam <fabio.estevam@nxp.com>; > kernel@pengutronix.de; mark.rutland@arm.com; matteo.lisi@engicam.com; > michael@amarulasolutions.com; mturquette@baylibre.com; > robh+dt@kernel.org; shawnguo@kernel.org > Cc: linux-clk@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 1/2] clk: imx6ul: add GPIO clock gates > > Quoting Stefan Wahren (2018-05-22 05:25:35) > > > +++ b/include/dt-bindings/clock/imx6ul-clock.h > > > @@ -242,20 +242,25 @@ > > > #define IMX6UL_CLK_CKO2_PODF 229 > > > #define IMX6UL_CLK_CKO2 230 > > > #define IMX6UL_CLK_CKO 231 > > > +#define IMX6UL_CLK_GPIO1 232 > > > +#define IMX6UL_CLK_GPIO2 233 > > > +#define IMX6UL_CLK_GPIO3 234 > > > +#define IMX6UL_CLK_GPIO4 235 > > > +#define IMX6UL_CLK_GPIO5 236 > > > > this change looks like a breakage of devicetree ABI. You are changing the > mean of the existing clock IDs on i.MX6ULL, which probably regress the > combination of older DTBs with newer kernel. > > > > Agreed. Why can't we just tack on more numbers at the end? Ah, yes, I saw 6ULL are at the end of 6UL, so added them in 6UL, but did NOT consider the old dtb support. Will send out a V2 patch to fix it, and I saw Fabio also sent a patch to fix the clko1/2 definition, I will do the V2 patch based on his patch. Anson. > > > > > > > /* For i.MX6ULL */ > > > -#define IMX6ULL_CLK_ESAI_PRED 232
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c index ba563ba..3ea2d97 100644 --- a/drivers/clk/imx/clk-imx6ul.c +++ b/drivers/clk/imx/clk-imx6ul.c @@ -360,6 +360,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) clks[IMX6UL_CLK_UART2_SERIAL] = imx_clk_gate2("uart2_serial", "uart_podf", base + 0x68, 28); if (clk_on_imx6ull()) clks[IMX6UL_CLK_AIPSTZ3] = imx_clk_gate2("aips_tz3", "ahb", base + 0x80, 18); + clks[IMX6UL_CLK_GPIO2] = imx_clk_gate2("gpio2", "ipg", base + 0x68, 30); /* CCGR1 */ clks[IMX6UL_CLK_ECSPI1] = imx_clk_gate2("ecspi1", "ecspi_podf", base + 0x6c, 0); @@ -376,6 +377,8 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) clks[IMX6UL_CLK_GPT1_SERIAL] = imx_clk_gate2("gpt1_serial", "perclk", base + 0x6c, 22); clks[IMX6UL_CLK_UART4_IPG] = imx_clk_gate2("uart4_ipg", "ipg", base + 0x6c, 24); clks[IMX6UL_CLK_UART4_SERIAL] = imx_clk_gate2("uart4_serial", "uart_podf", base + 0x6c, 24); + clks[IMX6UL_CLK_GPIO1] = imx_clk_gate2("gpio1", "ipg", base + 0x6c, 26); + clks[IMX6UL_CLK_GPIO5] = imx_clk_gate2("gpio5", "ipg", base + 0x6c, 30); /* CCGR2 */ if (clk_on_imx6ull()) { @@ -389,6 +392,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) clks[IMX6UL_CLK_I2C3] = imx_clk_gate2("i2c3", "perclk", base + 0x70, 10); clks[IMX6UL_CLK_OCOTP] = imx_clk_gate2("ocotp", "ipg", base + 0x70, 12); clks[IMX6UL_CLK_IOMUXC] = imx_clk_gate2("iomuxc", "lcdif_podf", base + 0x70, 14); + clks[IMX6UL_CLK_GPIO3] = imx_clk_gate2("gpio3", "ipg", base + 0x70, 26); clks[IMX6UL_CLK_LCDIF_APB] = imx_clk_gate2("lcdif_apb", "axi", base + 0x70, 28); clks[IMX6UL_CLK_PXP] = imx_clk_gate2("pxp", "axi", base + 0x70, 30); @@ -405,6 +409,7 @@ static void __init imx6ul_clocks_init(struct device_node *ccm_node) clks[IMX6UL_CLK_UART6_IPG] = imx_clk_gate2("uart6_ipg", "ipg", base + 0x74, 6); clks[IMX6UL_CLK_UART6_SERIAL] = imx_clk_gate2("uart6_serial", "uart_podf", base + 0x74, 6); clks[IMX6UL_CLK_LCDIF_PIX] = imx_clk_gate2("lcdif_pix", "lcdif_podf", base + 0x74, 10); + clks[IMX6UL_CLK_GPIO4] = imx_clk_gate2("gpio4", "ipg", base + 0x74, 12); clks[IMX6UL_CLK_QSPI] = imx_clk_gate2("qspi1", "qspi1_podf", base + 0x74, 14); clks[IMX6UL_CLK_WDOG1] = imx_clk_gate2("wdog1", "ipg", base + 0x74, 16); clks[IMX6UL_CLK_MMDC_P0_FAST] = imx_clk_gate("mmdc_p0_fast", "mmdc_podf", base + 0x74, 20); diff --git a/include/dt-bindings/clock/imx6ul-clock.h b/include/dt-bindings/clock/imx6ul-clock.h index 9564597..1291328 100644 --- a/include/dt-bindings/clock/imx6ul-clock.h +++ b/include/dt-bindings/clock/imx6ul-clock.h @@ -242,20 +242,25 @@ #define IMX6UL_CLK_CKO2_PODF 229 #define IMX6UL_CLK_CKO2 230 #define IMX6UL_CLK_CKO 231 +#define IMX6UL_CLK_GPIO1 232 +#define IMX6UL_CLK_GPIO2 233 +#define IMX6UL_CLK_GPIO3 234 +#define IMX6UL_CLK_GPIO4 235 +#define IMX6UL_CLK_GPIO5 236 /* For i.MX6ULL */ -#define IMX6ULL_CLK_ESAI_PRED 232 -#define IMX6ULL_CLK_ESAI_PODF 233 -#define IMX6ULL_CLK_ESAI_EXTAL 234 -#define IMX6ULL_CLK_ESAI_MEM 235 -#define IMX6ULL_CLK_ESAI_IPG 236 -#define IMX6ULL_CLK_DCP_CLK 237 -#define IMX6ULL_CLK_EPDC_PRE_SEL 238 -#define IMX6ULL_CLK_EPDC_SEL 239 -#define IMX6ULL_CLK_EPDC_PODF 240 -#define IMX6ULL_CLK_EPDC_ACLK 241 -#define IMX6ULL_CLK_EPDC_PIX 242 -#define IMX6ULL_CLK_ESAI_SEL 243 -#define IMX6UL_CLK_END 244 +#define IMX6ULL_CLK_ESAI_PRED 237 +#define IMX6ULL_CLK_ESAI_PODF 238 +#define IMX6ULL_CLK_ESAI_EXTAL 239 +#define IMX6ULL_CLK_ESAI_MEM 240 +#define IMX6ULL_CLK_ESAI_IPG 241 +#define IMX6ULL_CLK_DCP_CLK 242 +#define IMX6ULL_CLK_EPDC_PRE_SEL 243 +#define IMX6ULL_CLK_EPDC_SEL 244 +#define IMX6ULL_CLK_EPDC_PODF 245 +#define IMX6ULL_CLK_EPDC_ACLK 246 +#define IMX6ULL_CLK_EPDC_PIX 247 +#define IMX6ULL_CLK_ESAI_SEL 248 +#define IMX6UL_CLK_END 249 #endif /* __DT_BINDINGS_CLOCK_IMX6UL_H */
i.MX6UL has GPIO clock gates in CCM CCGR, add them into clock tree for clock management. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- drivers/clk/imx/clk-imx6ul.c | 5 +++++ include/dt-bindings/clock/imx6ul-clock.h | 31 ++++++++++++++++++------------- 2 files changed, 23 insertions(+), 13 deletions(-)