RE: [PATCH] ARM: dts: imx6sl: correct PWM ipg clock source
From: Anson Huang
Date: Wed Dec 19 2018 - 03:44:25 EST
Best Regards!
Anson Huang
> -----Original Message-----
> From: Uwe Kleine-KÃnig [mailto:u.kleine-koenig@xxxxxxxxxxxxxx]
> Sent: 2018å12æ19æ 16:37
> To: Anson Huang <anson.huang@xxxxxxx>
> Cc: shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> Fabio Estevam <fabio.estevam@xxxxxxx>; robh+dt@xxxxxxxxxx;
> mark.rutland@xxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>; linux-pwm@xxxxxxxxxxxxxxx
> 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@xxxxxxx>
> > ---
> > 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 |