[PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[Andrey Smirnov]

Since 25aaa75df1e6 SDMA driver uses clock rates of "ipg" and "ahb"
clock to determine if it needs to configure the IP block as operating
at 1:1 or 1:2 clock ratio (ACR bit in SDMAARM_CONFIG). Specifying both
clocks as IMX6QDL_CLK_SDMA results in driver incorrectly thinking that
ratio is 1:1 which results in broken SDMA functionality(this at least
breaks RAVE SP serdev driver on RDU2). Fix the code to specify
IMX6QDL_CLK_IPG as "ipg" clock for SDMA, to the problem.

Fixes: 25aaa75df1e6 ("dmaengine: imx-sdma: add clock ratio 1:1 check")
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Angus Ainslie (Purism) <angus@akkea.ca>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---

This probably has been fixed already, so please ignore this patch if
that is the case.

Thanks,
Andrey Smirnov

 arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 9f9aa6e7ed0e..354feba077b2 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -949,7 +949,7 @@
 				compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
 				reg = <0x020ec000 0x4000>;
 				interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>;
-				clocks = <&clks IMX6QDL_CLK_SDMA>,
+				clocks = <&clks IMX6QDL_CLK_IPG>,
 					 <&clks IMX6QDL_CLK_SDMA>;
 				clock-names = "ipg", "ahb";
 				#dma-cells = <3>;
-- 
2.20.1

Re: [PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[Lucas Stach]

Am Sonntag, den 24.03.2019, 22:01 -0700 schrieb Andrey Smirnov:
> Since 25aaa75df1e6 SDMA driver uses clock rates of "ipg" and "ahb"
> clock to determine if it needs to configure the IP block as operating
> at 1:1 or 1:2 clock ratio (ACR bit in SDMAARM_CONFIG). Specifying both
> clocks as IMX6QDL_CLK_SDMA results in driver incorrectly thinking that
> ratio is 1:1 which results in broken SDMA functionality(this at least
> breaks RAVE SP serdev driver on RDU2). Fix the code to specify
> IMX6QDL_CLK_IPG as "ipg" clock for SDMA, to the problem.

As this is breaking DT backward compatibility, I think we need a
blacklist in the driver for SOCs where the DT is known to be broken.

But fixing the broken DT is still a good idea, so:

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> Fixes: 25aaa75df1e6 ("dmaengine: imx-sdma: add clock ratio 1:1 check")
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Angus Ainslie (Purism) <angus@akkea.ca>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> 
> This probably has been fixed already, so please ignore this patch if
> that is the case.
> 
> Thanks,
> Andrey Smirnov
> 
>  arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 9f9aa6e7ed0e..354feba077b2 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -949,7 +949,7 @@
> >  				compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
> >  				reg = <0x020ec000 0x4000>;
> >  				interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>;
> > -				clocks = <&clks IMX6QDL_CLK_SDMA>,
> > +				clocks = <&clks IMX6QDL_CLK_IPG>,
> >  					 <&clks IMX6QDL_CLK_SDMA>;
> >  				clock-names = "ipg", "ahb";
> >  				#dma-cells = <3>;

Re: [PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[Fabio Estevam]

Hi Andrey,

On Mon, Mar 25, 2019 at 2:01 AM Andrey Smirnov <andrew.smirnov@gmail.com> wrote:

> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 9f9aa6e7ed0e..354feba077b2 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -949,7 +949,7 @@
>                                 compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
>                                 reg = <0x020ec000 0x4000>;
>                                 interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>;
> -                               clocks = <&clks IMX6QDL_CLK_SDMA>,
> +                               clocks = <&clks IMX6QDL_CLK_IPG>,

This problem affects other i.MX SoCs, such as i,MX6SX as originally
reported here:
https://patchwork.kernel.org/patch/10864867/

So I think we need either a  dtsi fix for the other i.MX SoCs or maybe
a fix in the SDMA driver?

Re: [PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[Andrey Smirnov]

On Mon, Mar 25, 2019 at 3:49 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Andrey,
>
> On Mon, Mar 25, 2019 at 2:01 AM Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
>
> > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> > index 9f9aa6e7ed0e..354feba077b2 100644
> > --- a/arch/arm/boot/dts/imx6qdl.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> > @@ -949,7 +949,7 @@
> >                                 compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
> >                                 reg = <0x020ec000 0x4000>;
> >                                 interrupts = <0 2 IRQ_TYPE_LEVEL_HIGH>;
> > -                               clocks = <&clks IMX6QDL_CLK_SDMA>,
> > +                               clocks = <&clks IMX6QDL_CLK_IPG>,
>
> This problem affects other i.MX SoCs, such as i,MX6SX as originally
> reported here:
> https://patchwork.kernel.org/patch/10864867/
>
> So I think we need either a  dtsi fix for the other i.MX SoCs or maybe
> a fix in the SDMA driver?

I think it is worth fixing in DT, regardless if we do anything about
SDMA driver or not. I'll add fixes for 6SX and 7D in v2.

Thanks,
Andrey Smirnov

Re: [PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[Fabio Estevam]

Hi Andrey,

On Tue, Mar 26, 2019 at 11:52 PM Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:

> I think it is worth fixing in DT, regardless if we do anything about
> SDMA driver or not. I'll add fixes for 6SX and 7D in v2.

What about imx25, imx31, imx50, imx51, imx53, imx6sl, imx6ul, imx6sll,
imx6ull, etc?

If we plan to fix the DT files, I think we should cover all i.MX SoCs then.

Thanks

Re: [PATCH] ARM: dts: imx6qdl: Specify IMX6QDL_CLK_IPG as "ipg" clock for SDMA

[Andrey Smirnov]

On Wed, Mar 27, 2019 at 4:17 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Andrey,
>
> On Tue, Mar 26, 2019 at 11:52 PM Andrey Smirnov
> <andrew.smirnov@gmail.com> wrote:
>
> > I think it is worth fixing in DT, regardless if we do anything about
> > SDMA driver or not. I'll add fixes for 6SX and 7D in v2.
>
> What about imx25, imx31, imx50, imx51, imx53, imx6sl, imx6ul, imx6sll,
> imx6ull, etc?
>

I believe imx25, imx31, imx35 already have proper clocks configured.
I'll add the rest in v3.

Thanks,
Andrey Smirnov