Linux-ARM-Kernel Archive on lore.kernel.org
 help / Atom feed
* [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source
@ 2018-12-19  5:41 Anson Huang
  2018-12-19  8:36 ` Uwe Kleine-König
  2019-01-11 13:07 ` Shawn Guo
  0 siblings, 2 replies; 5+ messages in thread
From: Anson Huang @ 2018-12-19  5:41 UTC (permalink / raw)
  To: shawnguo, s.hauer, kernel, Fabio Estevam, robh+dt, mark.rutland,
	linux-arm-kernel, devicetree, linux-kernel
  Cc: dl-linux-imx

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(-)

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";
 			};
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source
  2018-12-19  5:41 [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source Anson Huang
@ 2018-12-19  8:36 ` Uwe Kleine-König
  2018-12-19  8:44   ` Anson Huang
  2019-01-11 13:07 ` Shawn Guo
  1 sibling, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2018-12-19  8:36 UTC (permalink / raw)
  To: Anson Huang
  Cc: mark.rutland, devicetree, linux-pwm, s.hauer, linux-kernel,
	robh+dt, dl-linux-imx, kernel, Fabio Estevam, shawnguo,
	linux-arm-kernel

[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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source
  2018-12-19  8:36 ` Uwe Kleine-König
@ 2018-12-19  8:44   ` Anson Huang
  2018-12-19  8:51     ` Uwe Kleine-König
  0 siblings, 1 reply; 5+ messages in thread
From: Anson Huang @ 2018-12-19  8:44 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: mark.rutland, devicetree, linux-pwm, s.hauer, linux-kernel,
	robh+dt, dl-linux-imx, kernel, Fabio Estevam, shawnguo,
	linux-arm-kernel



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&amp;data=02%7C01%7Canson.huang%40nxp.com%7C7bf
> fc7bad61545fb595508d6658d1d0f%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C636808054141733417&amp;sdata=dDv2spIwbQmkpu7mhEm
> 8bRHyKKviSO%2BQ33ZU7nD1bJQ%3D&amp;reserved=0  |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source
  2018-12-19  8:44   ` Anson Huang
@ 2018-12-19  8:51     ` Uwe Kleine-König
  0 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2018-12-19  8:51 UTC (permalink / raw)
  To: Anson Huang
  Cc: mark.rutland, devicetree, linux-pwm, s.hauer, linux-kernel,
	robh+dt, dl-linux-imx, kernel, Fabio Estevam, shawnguo,
	linux-arm-kernel

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&amp;data=02%7C01%7Canson.huang%40nxp.com%7C7bf
> > fc7bad61545fb595508d6658d1d0f%7C686ea1d3bc2b4c6fa92cd99c5c301635
> > %7C0%7C0%7C636808054141733417&amp;sdata=dDv2spIwbQmkpu7mhEm
> > 8bRHyKKviSO%2BQ33ZU7nD1bJQ%3D&amp;reserved=0  |

haha, something between you and me mangles links. I bet it's on your
side.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source
  2018-12-19  5:41 [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source Anson Huang
  2018-12-19  8:36 ` Uwe Kleine-König
@ 2019-01-11 13:07 ` Shawn Guo
  1 sibling, 0 replies; 5+ messages in thread
From: Shawn Guo @ 2019-01-11 13:07 UTC (permalink / raw)
  To: Anson Huang
  Cc: mark.rutland, devicetree, s.hauer, linux-kernel, robh+dt,
	dl-linux-imx, kernel, Fabio Estevam, linux-arm-kernel

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-19  5:41 [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source Anson Huang
2018-12-19  8:36 ` Uwe Kleine-König
2018-12-19  8:44   ` Anson Huang
2018-12-19  8:51     ` Uwe Kleine-König
2019-01-11 13:07 ` Shawn Guo

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox