| Message ID | 1545197799-9082-1-git-send-email-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Commit | 0ce7b4a7741218465e492d30e4cf90a835fb7dfa |
| Headers | show |
| Series | ARM: dts: imx6sl: correct PWM ipg clock source | expand |
[Cc: += linux-pwm] Hello, On Wed, Dec 19, 2018 at 05:41:31AM +0000, Anson Huang wrote: > From i.MX6SL Reference Manual, the PWMx's ipg clock > for registers access is from perclk, correct them. I assume this is related to the patch "pwm: imx: add ipg clock operation"? This patch doesn't fix a real problem because in practise perclk is already enabled, right? (I don't question the patch, just wonder how urgent it is.) > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > arch/arm/boot/dts/imx6sl.dtsi | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi > index e7524e7..4b4813f 100644 > --- a/arch/arm/boot/dts/imx6sl.dtsi > +++ b/arch/arm/boot/dts/imx6sl.dtsi > @@ -338,7 +338,7 @@ > compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; > reg = <0x02080000 0x4000>; > interrupts = <0 83 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&clks IMX6SL_CLK_PWM1>, > + clocks = <&clks IMX6SL_CLK_PERCLK>, > <&clks IMX6SL_CLK_PWM1>; > clock-names = "ipg", "per"; It's a bit irritating that IMX6SL_CLK_PERCLK is used for the "ipg" clk, not the "per" clk. Is this correct? Best regards Uwe
Best Regards! Anson Huang > -----Original Message----- > From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de] > Sent: 2018年12月19日 16:37 > To: Anson Huang <anson.huang@nxp.com> > Cc: shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; > Fabio Estevam <fabio.estevam@nxp.com>; robh+dt@kernel.org; > mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com>; linux-pwm@vger.kernel.org > Subject: Re: [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source > > [Cc: += linux-pwm] > > Hello, > > On Wed, Dec 19, 2018 at 05:41:31AM +0000, Anson Huang wrote: > > From i.MX6SL Reference Manual, the PWMx's ipg clock for registers > > access is from perclk, correct them. > > I assume this is related to the patch "pwm: imx: add ipg clock operation"? This > patch doesn't fix a real problem because in practise perclk is already enabled, > right? (I don't question the patch, just wonder how urgent it is.) Yes, I met the kernel boot up hang on i.MX7D and look into the PWM module and found that the ipg_clk_s is for register access, but different i.MX SoC could have different ipg_clk_s clock source, such as on i.MX6QDL, it is from ipg clock, the rest are from perclk, they are always ON, so no issue, but on i.MX7D, the ipg_clk_s of PWM module is controllable in CCM CCGR, so we have to add the ipg clock operation as well as perclk. i.MX6SL current setting has no issue, because the ipg_clk_s is from perclk which is always ON, but it does NOT match the clock info in reference manual, while other i.MX6 SoCs are having correct clock settings for PWM IPG clock. It is just an improvement, NOT fix a real problem. > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > --- > > arch/arm/boot/dts/imx6sl.dtsi | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx6sl.dtsi > > b/arch/arm/boot/dts/imx6sl.dtsi index e7524e7..4b4813f 100644 > > --- a/arch/arm/boot/dts/imx6sl.dtsi > > +++ b/arch/arm/boot/dts/imx6sl.dtsi > > @@ -338,7 +338,7 @@ > > compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; > > reg = <0x02080000 0x4000>; > > interrupts = <0 83 IRQ_TYPE_LEVEL_HIGH>; > > - clocks = <&clks IMX6SL_CLK_PWM1>, > > + clocks = <&clks IMX6SL_CLK_PERCLK>, > > <&clks IMX6SL_CLK_PWM1>; > > clock-names = "ipg", "per"; > > It's a bit irritating that IMX6SL_CLK_PERCLK is used for the "ipg" clk, not the > "per" clk. Is this correct? Yes, you can check the i.MX6SL CCM chapter for PWM clocks, the ipg_clk_s is from perclk and its enabled control is VCC which is always ON. The "per" clock is for PWM function, NOT register access, I tried disabling the PWM per clock, PWM registers still can be accessed. Anson. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. > pengutronix.de%2F&data=02%7C01%7Canson.huang%40nxp.com%7C7bf > fc7bad61545fb595508d6658d1d0f%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C0%7C0%7C636808054141733417&sdata=dDv2spIwbQmkpu7mhEm > 8bRHyKKviSO%2BQ33ZU7nD1bJQ%3D&reserved=0 |
Hello, On Wed, Dec 19, 2018 at 08:44:13AM +0000, Anson Huang wrote: > i.MX6SL current setting has no issue, because the ipg_clk_s is from perclk which is always > ON, but it does NOT match the clock info in reference manual, while other i.MX6 SoCs > are having correct clock settings for PWM IPG clock. > > It is just an improvement, NOT fix a real problem. OK, so it's merge window material as I expected. > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > > --- > > > arch/arm/boot/dts/imx6sl.dtsi | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/arm/boot/dts/imx6sl.dtsi > > > b/arch/arm/boot/dts/imx6sl.dtsi index e7524e7..4b4813f 100644 > > > --- a/arch/arm/boot/dts/imx6sl.dtsi > > > +++ b/arch/arm/boot/dts/imx6sl.dtsi > > > @@ -338,7 +338,7 @@ > > > compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; > > > reg = <0x02080000 0x4000>; > > > interrupts = <0 83 IRQ_TYPE_LEVEL_HIGH>; > > > - clocks = <&clks IMX6SL_CLK_PWM1>, > > > + clocks = <&clks IMX6SL_CLK_PERCLK>, > > > <&clks IMX6SL_CLK_PWM1>; > > > clock-names = "ipg", "per"; > > > > It's a bit irritating that IMX6SL_CLK_PERCLK is used for the "ipg" clk, not the > > "per" clk. Is this correct? > > Yes, you can check the i.MX6SL CCM chapter for PWM clocks, the ipg_clk_s > is from perclk and its enabled control is VCC which is always ON. The "per" > clock is for PWM function, NOT register access, I tried disabling the PWM per clock, > PWM registers still can be accessed. OK, then Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Industrial Linux Solutions | > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. > > pengutronix.de%2F&data=02%7C01%7Canson.huang%40nxp.com%7C7bf > > fc7bad61545fb595508d6658d1d0f%7C686ea1d3bc2b4c6fa92cd99c5c301635 > > %7C0%7C0%7C636808054141733417&sdata=dDv2spIwbQmkpu7mhEm > > 8bRHyKKviSO%2BQ33ZU7nD1bJQ%3D&reserved=0 | haha, something between you and me mangles links. I bet it's on your side. Best regards Uwe
On Wed, Dec 19, 2018 at 05:41:31AM +0000, Anson Huang wrote: > From i.MX6SL Reference Manual, the PWMx's ipg clock > for registers access is from perclk, correct them. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Applied, thanks.
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi index e7524e7..4b4813f 100644 --- a/arch/arm/boot/dts/imx6sl.dtsi +++ b/arch/arm/boot/dts/imx6sl.dtsi @@ -338,7 +338,7 @@ compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; reg = <0x02080000 0x4000>; interrupts = <0 83 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clks IMX6SL_CLK_PWM1>, + clocks = <&clks IMX6SL_CLK_PERCLK>, <&clks IMX6SL_CLK_PWM1>; clock-names = "ipg", "per"; }; @@ -348,7 +348,7 @@ compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; reg = <0x02084000 0x4000>; interrupts = <0 84 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clks IMX6SL_CLK_PWM2>, + clocks = <&clks IMX6SL_CLK_PERCLK>, <&clks IMX6SL_CLK_PWM2>; clock-names = "ipg", "per"; }; @@ -358,7 +358,7 @@ compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; reg = <0x02088000 0x4000>; interrupts = <0 85 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clks IMX6SL_CLK_PWM3>, + clocks = <&clks IMX6SL_CLK_PERCLK>, <&clks IMX6SL_CLK_PWM3>; clock-names = "ipg", "per"; }; @@ -368,7 +368,7 @@ compatible = "fsl,imx6sl-pwm", "fsl,imx27-pwm"; reg = <0x0208c000 0x4000>; interrupts = <0 86 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clks IMX6SL_CLK_PWM4>, + clocks = <&clks IMX6SL_CLK_PERCLK>, <&clks IMX6SL_CLK_PWM4>; clock-names = "ipg", "per"; };
From i.MX6SL Reference Manual, the PWMx's ipg clock for registers access is from perclk, correct them. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- arch/arm/boot/dts/imx6sl.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)