[PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

There is one weird data in rxfifo after one full rx/tx transfer
done sometimes. It looks a design issue and hard to workaround
totally, so disable dma functhion here. And will re-enable it
once the root cause found.

Signed-off-by: Robin Gong <b38343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6q.dtsi   | 20 ++++++++++++++++++++
 arch/arm/boot/dts/imx6qdl.dtsi |  8 --------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9f3646..8d5d33b 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -291,6 +291,26 @@
 	};
 };
 
+&ecspi1 {
+	dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
+	dma-names = "rx", "tx";
+};
+
+&ecspi2 {
+	dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
+	dma-names = "rx", "tx";
+};
+
+&ecspi3 {
+	dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
+	dma-names = "rx", "tx";
+};
+
+&ecspi4 {
+	dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
+	dma-names = "rx", "tx";
+};
+
 &mipi_dsi {
 	port@2 {
 		reg = <2>;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 70d7207..f696546 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -210,8 +210,6 @@
 					clocks = <&clks IMX6QDL_CLK_ECSPI1>,
 						 <&clks IMX6QDL_CLK_ECSPI1>;
 					clock-names = "ipg", "per";
-					dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
-					dma-names = "rx", "tx";
 					status = "disabled";
 				};
 
@@ -224,8 +222,6 @@
 					clocks = <&clks IMX6QDL_CLK_ECSPI2>,
 						 <&clks IMX6QDL_CLK_ECSPI2>;
 					clock-names = "ipg", "per";
-					dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
-					dma-names = "rx", "tx";
 					status = "disabled";
 				};
 
@@ -238,8 +234,6 @@
 					clocks = <&clks IMX6QDL_CLK_ECSPI3>,
 						 <&clks IMX6QDL_CLK_ECSPI3>;
 					clock-names = "ipg", "per";
-					dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
-					dma-names = "rx", "tx";
 					status = "disabled";
 				};
 
@@ -252,8 +246,6 @@
 					clocks = <&clks IMX6QDL_CLK_ECSPI4>,
 						 <&clks IMX6QDL_CLK_ECSPI4>;
 					clock-names = "ipg", "per";
-					dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
-					dma-names = "rx", "tx";
 					status = "disabled";
 				};
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Shawn Guo]

On Wed, Sep 10, 2014 at 01:30:12PM +0800, Robin Gong wrote:
> There is one weird data in rxfifo after one full rx/tx transfer
> done sometimes. It looks a design issue and hard to workaround
> totally, so disable dma functhion here. And will re-enable it
> once the root cause found.
> 
> Signed-off-by: Robin Gong <b38343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Lucas Stach]

Am Mittwoch, den 10.09.2014, 13:30 +0800 schrieb Robin Gong:
> There is one weird data in rxfifo after one full rx/tx transfer
> done sometimes. It looks a design issue and hard to workaround
> totally, so disable dma functhion here. And will re-enable it
> once the root cause found.
> 
Sorry, I'm late to this as Shawn seems to already have picked up this
patch, but this isn't the right way to fix the problem.

We made it clear at kernel summit last year that we try to not break
existing DTs as booting a new kernel with an old DT is a valid use case.
While you don't strictly violate this rule what you do here is only
fixing systems booting with a new DT while leaving others broken.

If you are working around a hardware problem please disable DMA support
in the driver. This will also allow you to enable it again, if you find
another workaround without touching the DT again.

So this patch gets a NAK from me.

> Signed-off-by: Robin Gong <b38343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6q.dtsi   | 20 ++++++++++++++++++++
>  arch/arm/boot/dts/imx6qdl.dtsi |  8 --------
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index e9f3646..8d5d33b 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -291,6 +291,26 @@
>  	};
>  };
>  
> +&ecspi1 {
> +	dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
> +	dma-names = "rx", "tx";
> +};
> +
> +&ecspi2 {
> +	dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
> +	dma-names = "rx", "tx";
> +};
> +
> +&ecspi3 {
> +	dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
> +	dma-names = "rx", "tx";
> +};
> +
> +&ecspi4 {
> +	dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
> +	dma-names = "rx", "tx";
> +};
> +
>  &mipi_dsi {
>  	port@2 {
>  		reg = <2>;
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 70d7207..f696546 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -210,8 +210,6 @@
>  					clocks = <&clks IMX6QDL_CLK_ECSPI1>,
>  						 <&clks IMX6QDL_CLK_ECSPI1>;
>  					clock-names = "ipg", "per";
> -					dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
> -					dma-names = "rx", "tx";
>  					status = "disabled";
>  				};
>  
> @@ -224,8 +222,6 @@
>  					clocks = <&clks IMX6QDL_CLK_ECSPI2>,
>  						 <&clks IMX6QDL_CLK_ECSPI2>;
>  					clock-names = "ipg", "per";
> -					dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
> -					dma-names = "rx", "tx";
>  					status = "disabled";
>  				};
>  
> @@ -238,8 +234,6 @@
>  					clocks = <&clks IMX6QDL_CLK_ECSPI3>,
>  						 <&clks IMX6QDL_CLK_ECSPI3>;
>  					clock-names = "ipg", "per";
> -					dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
> -					dma-names = "rx", "tx";
>  					status = "disabled";
>  				};
>  
> @@ -252,8 +246,6 @@
>  					clocks = <&clks IMX6QDL_CLK_ECSPI4>,
>  						 <&clks IMX6QDL_CLK_ECSPI4>;
>  					clock-names = "ipg", "per";
> -					dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
> -					dma-names = "rx", "tx";
>  					status = "disabled";
>  				};
>  

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Shawn Guo]

On Mon, Sep 15, 2014 at 11:41:13AM +0200, Lucas Stach wrote:
> Am Mittwoch, den 10.09.2014, 13:30 +0800 schrieb Robin Gong:
> > There is one weird data in rxfifo after one full rx/tx transfer
> > done sometimes. It looks a design issue and hard to workaround
> > totally, so disable dma functhion here. And will re-enable it
> > once the root cause found.
> > 
> Sorry, I'm late to this as Shawn seems to already have picked up this
> patch, but this isn't the right way to fix the problem.
> 
> We made it clear at kernel summit last year that we try to not break
> existing DTs as booting a new kernel with an old DT is a valid use case.
> While you don't strictly violate this rule what you do here is only
> fixing systems booting with a new DT while leaving others broken.
> 
> If you are working around a hardware problem please disable DMA support
> in the driver. This will also allow you to enable it again, if you find
> another workaround without touching the DT again.

Okay, it's a valid point.  Patch dropped.

Shawn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

Hi Lucas,
  I understood your concern,but looks we have to break old DT. Our old DT
support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
for SPI driver is still in reviewing(v6). So the violation you mentioned comes
if someone use the DT during the time(from v2 to spi driver patch upstreamed).
  The different behaviors on different chips(only i.mxdl can't pass strength
test) are just found from last month, this is why I sent the patch for disable
SPI DMA support on i.mx6dl during I sent v6 patch for spi driver. I think that's
make sense, because DTs are also in development, new difference between different
chips maybe found in the future although they share the same IP....
  Yes, I can check cpu type by looking at DT in spi driver, but it's not nice.
So I'm afraid that we have to break the old DTs in the gap between the two
levels patch accepted cycle.

On Mon, Sep 15, 2014 at 11:41:13AM +0200, Lucas Stach wrote:
> Am Mittwoch, den 10.09.2014, 13:30 +0800 schrieb Robin Gong:
> > There is one weird data in rxfifo after one full rx/tx transfer
> > done sometimes. It looks a design issue and hard to workaround
> > totally, so disable dma functhion here. And will re-enable it
> > once the root cause found.
> > 
> Sorry, I'm late to this as Shawn seems to already have picked up this
> patch, but this isn't the right way to fix the problem.
> 
> We made it clear at kernel summit last year that we try to not break
> existing DTs as booting a new kernel with an old DT is a valid use case.
> While you don't strictly violate this rule what you do here is only
> fixing systems booting with a new DT while leaving others broken.
> 
> If you are working around a hardware problem please disable DMA support
> in the driver. This will also allow you to enable it again, if you find
> another workaround without touching the DT again.
> 
> So this patch gets a NAK from me.
> 
> > Signed-off-by: Robin Gong <b38343-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx6q.dtsi   | 20 ++++++++++++++++++++
> >  arch/arm/boot/dts/imx6qdl.dtsi |  8 --------
> >  2 files changed, 20 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index e9f3646..8d5d33b 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -291,6 +291,26 @@
> >  	};
> >  };
> >  
> > +&ecspi1 {
> > +	dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
> > +	dma-names = "rx", "tx";
> > +};
> > +
> > +&ecspi2 {
> > +	dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
> > +	dma-names = "rx", "tx";
> > +};
> > +
> > +&ecspi3 {
> > +	dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
> > +	dma-names = "rx", "tx";
> > +};
> > +
> > +&ecspi4 {
> > +	dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
> > +	dma-names = "rx", "tx";
> > +};
> > +
> >  &mipi_dsi {
> >  	port@2 {
> >  		reg = <2>;
> > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> > index 70d7207..f696546 100644
> > --- a/arch/arm/boot/dts/imx6qdl.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> > @@ -210,8 +210,6 @@
> >  					clocks = <&clks IMX6QDL_CLK_ECSPI1>,
> >  						 <&clks IMX6QDL_CLK_ECSPI1>;
> >  					clock-names = "ipg", "per";
> > -					dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
> > -					dma-names = "rx", "tx";
> >  					status = "disabled";
> >  				};
> >  
> > @@ -224,8 +222,6 @@
> >  					clocks = <&clks IMX6QDL_CLK_ECSPI2>,
> >  						 <&clks IMX6QDL_CLK_ECSPI2>;
> >  					clock-names = "ipg", "per";
> > -					dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
> > -					dma-names = "rx", "tx";
> >  					status = "disabled";
> >  				};
> >  
> > @@ -238,8 +234,6 @@
> >  					clocks = <&clks IMX6QDL_CLK_ECSPI3>,
> >  						 <&clks IMX6QDL_CLK_ECSPI3>;
> >  					clock-names = "ipg", "per";
> > -					dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
> > -					dma-names = "rx", "tx";
> >  					status = "disabled";
> >  				};
> >  
> > @@ -252,8 +246,6 @@
> >  					clocks = <&clks IMX6QDL_CLK_ECSPI4>,
> >  						 <&clks IMX6QDL_CLK_ECSPI4>;
> >  					clock-names = "ipg", "per";
> > -					dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
> > -					dma-names = "rx", "tx";
> >  					status = "disabled";
> >  				};
> >  
> 
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Lucas Stach]

Hi Robin,

Am Dienstag, den 16.09.2014, 11:41 +0800 schrieb Robin Gong:
> Hi Lucas,
>   I understood your concern,but looks we have to break old DT.

Sorry, but this isn't going to happen. And honestly I don't even see the
need to do so.

> Our old DT
> support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
> for SPI driver is still in reviewing(v6).

So this means now is the time to fix this driver patch to not enable DMA
on imx6dl. Nobody will experience any breakage in this case.

> So the violation you mentioned comes
> if someone use the DT during the time(from v2 to spi driver patch upstreamed).
>   The different behaviors on different chips(only i.mxdl can't pass strength
> test) are just found from last month, this is why I sent the patch for disable
> SPI DMA support on i.mx6dl during I sent v6 patch for spi driver. I think that's
> make sense, because DTs are also in development, new difference between different
> chips maybe found in the future although they share the same IP....
>   Yes, I can check cpu type by looking at DT in spi driver, but it's not nice.

Yeah it would have been nicer if we had an explicit DT compatible for
the imx6dl ecspi version, but it's not there, so we have to cope with
this.

> So I'm afraid that we have to break the old DTs in the gap between the two
> levels patch accepted cycle.
> 
> On Mon, Sep 15, 2014 at 11:41:13AM +0200, Lucas Stach wrote:
> > Am Mittwoch, den 10.09.2014, 13:30 +0800 schrieb Robin Gong:
> > > There is one weird data in rxfifo after one full rx/tx transfer
> > > done sometimes. It looks a design issue and hard to workaround
> > > totally, so disable dma functhion here. And will re-enable it
> > > once the root cause found.
> > > 
> > Sorry, I'm late to this as Shawn seems to already have picked up this
> > patch, but this isn't the right way to fix the problem.
> > 
> > We made it clear at kernel summit last year that we try to not break
> > existing DTs as booting a new kernel with an old DT is a valid use case.
> > While you don't strictly violate this rule what you do here is only
> > fixing systems booting with a new DT while leaving others broken.
> > 
> > If you are working around a hardware problem please disable DMA support
> > in the driver. This will also allow you to enable it again, if you find
> > another workaround without touching the DT again.
> > 
> > So this patch gets a NAK from me.
> > 

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

On Tue, Sep 16, 2014 at 11:50:06AM +0200, Lucas Stach wrote:
> Hi Robin,
> 
> Am Dienstag, den 16.09.2014, 11:41 +0800 schrieb Robin Gong:
> > Hi Lucas,
> >   I understood your concern,but looks we have to break old DT.
> 
> Sorry, but this isn't going to happen. And honestly I don't even see the
> need to do so.
> 
> > Our old DT
> > support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
> > for SPI driver is still in reviewing(v6).
> 
> So this means now is the time to fix this driver patch to not enable DMA
> on imx6dl. Nobody will experience any breakage in this case.
>
Sorry, i.mx6q and i.mx6dl are totally same, not only IP but also clock. So we
can't distinguish them except for different compatible name....
> > So the violation you mentioned comes
> > if someone use the DT during the time(from v2 to spi driver patch upstreamed).
> >   The different behaviors on different chips(only i.mxdl can't pass strength
> > test) are just found from last month, this is why I sent the patch for disable
> > SPI DMA support on i.mx6dl during I sent v6 patch for spi driver. I think that's
> > make sense, because DTs are also in development, new difference between different
> > chips maybe found in the future although they share the same IP....
> >   Yes, I can check cpu type by looking at DT in spi driver, but it's not nice.
> 
> Yeah it would have been nicer if we had an explicit DT compatible for
> the imx6dl ecspi version, but it's not there, so we have to cope with
> this.
> 
> > So I'm afraid that we have to break the old DTs in the gap between the two
> > levels patch accepted cycle.
> > 
> > On Mon, Sep 15, 2014 at 11:41:13AM +0200, Lucas Stach wrote:
> > > Am Mittwoch, den 10.09.2014, 13:30 +0800 schrieb Robin Gong:
> > > > There is one weird data in rxfifo after one full rx/tx transfer
> > > > done sometimes. It looks a design issue and hard to workaround
> > > > totally, so disable dma functhion here. And will re-enable it
> > > > once the root cause found.
> > > > 
> > > Sorry, I'm late to this as Shawn seems to already have picked up this
> > > patch, but this isn't the right way to fix the problem.
> > > 
> > > We made it clear at kernel summit last year that we try to not break
> > > existing DTs as booting a new kernel with an old DT is a valid use case.
> > > While you don't strictly violate this rule what you do here is only
> > > fixing systems booting with a new DT while leaving others broken.
> > > 
> > > If you are working around a hardware problem please disable DMA support
> > > in the driver. This will also allow you to enable it again, if you find
> > > another workaround without touching the DT again.
> > > 
> > > So this patch gets a NAK from me.
> > > 
> 
> Regards,
> Lucas
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Lucas Stach]

Am Mittwoch, den 17.09.2014, 16:41 +0800 schrieb Robin Gong:
> On Tue, Sep 16, 2014 at 11:50:06AM +0200, Lucas Stach wrote:
> > Hi Robin,
> > 
> > Am Dienstag, den 16.09.2014, 11:41 +0800 schrieb Robin Gong:
> > > Hi Lucas,
> > >   I understood your concern,but looks we have to break old DT.
> > 
> > Sorry, but this isn't going to happen. And honestly I don't even see the
> > need to do so.
> > 
> > > Our old DT
> > > support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
> > > for SPI driver is still in reviewing(v6).
> > 
> > So this means now is the time to fix this driver patch to not enable DMA
> > on imx6dl. Nobody will experience any breakage in this case.
> >
> Sorry, i.mx6q and i.mx6dl are totally same, not only IP but also clock. So we
> can't distinguish them except for different compatible name....

I know that, but I still don't see the problem.

If this issue is specific to the ECSPI IP integrated into a imx6dl SoC
you need to check for the machine compatible in the driver and disable
DMA support based on this. We generally avoid checking for the machine
compatible in drivers and prefer to have a specific IP block compatible
instead, but in that case it's the only reasonable thing to do.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

On Wed, Sep 17, 2014 at 10:55:56AM +0200, Lucas Stach wrote:
> Am Mittwoch, den 17.09.2014, 16:41 +0800 schrieb Robin Gong:
> > On Tue, Sep 16, 2014 at 11:50:06AM +0200, Lucas Stach wrote:
> > > Hi Robin,
> > > 
> > > Am Dienstag, den 16.09.2014, 11:41 +0800 schrieb Robin Gong:
> > > > Hi Lucas,
> > > >   I understood your concern,but looks we have to break old DT.
> > > 
> > > Sorry, but this isn't going to happen. And honestly I don't even see the
> > > need to do so.
> > > 
> > > > Our old DT
> > > > support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
> > > > for SPI driver is still in reviewing(v6).
> > > 
> > > So this means now is the time to fix this driver patch to not enable DMA
> > > on imx6dl. Nobody will experience any breakage in this case.
> > >
> > Sorry, i.mx6q and i.mx6dl are totally same, not only IP but also clock. So we
> > can't distinguish them except for different compatible name....
> 
> I know that, but I still don't see the problem.
> 
> If this issue is specific to the ECSPI IP integrated into a imx6dl SoC
> you need to check for the machine compatible in the driver and disable
> DMA support based on this. We generally avoid checking for the machine
> compatible in drivers and prefer to have a specific IP block compatible
> instead, but in that case it's the only reasonable thing to do.
> 
> Regards,
> Lucas
Sounds we make a agreement on use different compatible name, right? I will
make patch for reviewing, thanks.
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Lucas Stach]

Am Mittwoch, den 17.09.2014, 17:19 +0800 schrieb Robin Gong:
> On Wed, Sep 17, 2014 at 10:55:56AM +0200, Lucas Stach wrote:
> > Am Mittwoch, den 17.09.2014, 16:41 +0800 schrieb Robin Gong:
> > > On Tue, Sep 16, 2014 at 11:50:06AM +0200, Lucas Stach wrote:
> > > > Hi Robin,
> > > > 
> > > > Am Dienstag, den 16.09.2014, 11:41 +0800 schrieb Robin Gong:
> > > > > Hi Lucas,
> > > > >   I understood your concern,but looks we have to break old DT.
> > > > 
> > > > Sorry, but this isn't going to happen. And honestly I don't even see the
> > > > need to do so.
> > > > 
> > > > > Our old DT
> > > > > support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
> > > > > for SPI driver is still in reviewing(v6).
> > > > 
> > > > So this means now is the time to fix this driver patch to not enable DMA
> > > > on imx6dl. Nobody will experience any breakage in this case.
> > > >
> > > Sorry, i.mx6q and i.mx6dl are totally same, not only IP but also clock. So we
> > > can't distinguish them except for different compatible name....
> > 
> > I know that, but I still don't see the problem.
> > 
> > If this issue is specific to the ECSPI IP integrated into a imx6dl SoC
> > you need to check for the machine compatible in the driver and disable
> > DMA support based on this. We generally avoid checking for the machine
> > compatible in drivers and prefer to have a specific IP block compatible
> > instead, but in that case it's the only reasonable thing to do.
> > 
> > Regards,
> > Lucas
> Sounds we make a agreement on use different compatible name, right? I will
> make patch for reviewing, thanks.

No you can't use a new compatible name for the IP block, as this again
would only fix things for users with new devicetrees. Also this has
nothing to do with the IP block, but is an integration issue between
ECPSI and imx6dl.

So what you need to do is something like this in the ecspi kernel
driver:

if (of_machine_is_compatible("fsl,imx6dl") {
    do_whatever_is_necessary_to_make_sure_DMA_is_not_enabled();
}

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

On Wed, Sep 17, 2014 at 11:19:34AM +0200, Lucas Stach wrote:
> Am Mittwoch, den 17.09.2014, 17:19 +0800 schrieb Robin Gong:
> > On Wed, Sep 17, 2014 at 10:55:56AM +0200, Lucas Stach wrote:
> > > Am Mittwoch, den 17.09.2014, 16:41 +0800 schrieb Robin Gong:
> > > > On Tue, Sep 16, 2014 at 11:50:06AM +0200, Lucas Stach wrote:
> > > > > Hi Robin,
> > > > > 
> > > > > Am Dienstag, den 16.09.2014, 11:41 +0800 schrieb Robin Gong:
> > > > > > Hi Lucas,
> > > > > >   I understood your concern,but looks we have to break old DT.
> > > > > 
> > > > > Sorry, but this isn't going to happen. And honestly I don't even see the
> > > > > need to do so.
> > > > > 
> > > > > > Our old DT
> > > > > > support SPI DMA on i.mx6q/dl(v2,b3810c3dc1bcbc6a), but the DMA support patch
> > > > > > for SPI driver is still in reviewing(v6).
> > > > > 
> > > > > So this means now is the time to fix this driver patch to not enable DMA
> > > > > on imx6dl. Nobody will experience any breakage in this case.
> > > > >
> > > > Sorry, i.mx6q and i.mx6dl are totally same, not only IP but also clock. So we
> > > > can't distinguish them except for different compatible name....
> > > 
> > > I know that, but I still don't see the problem.
> > > 
> > > If this issue is specific to the ECSPI IP integrated into a imx6dl SoC
> > > you need to check for the machine compatible in the driver and disable
> > > DMA support based on this. We generally avoid checking for the machine
> > > compatible in drivers and prefer to have a specific IP block compatible
> > > instead, but in that case it's the only reasonable thing to do.
> > > 
> > > Regards,
> > > Lucas
> > Sounds we make a agreement on use different compatible name, right? I will
> > make patch for reviewing, thanks.
> 
> No you can't use a new compatible name for the IP block, as this again
> would only fix things for users with new devicetrees. Also this has
> nothing to do with the IP block, but is an integration issue between
> ECPSI and imx6dl.
> 
> So what you need to do is something like this in the ecspi kernel
> driver:
> 
> if (of_machine_is_compatible("fsl,imx6dl") {
>     do_whatever_is_necessary_to_make_sure_DMA_is_not_enabled();
> }
> 
> Regards,
> Lucas
Sure, the ecspi kernel driver must check the new compatible name for i.mx6dl.
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Alexander Holler]

Am 10.09.2014 07:30, schrieb Robin Gong:
> There is one weird data in rxfifo after one full rx/tx transfer
> done sometimes. It looks a design issue and hard to workaround
> totally, so disable dma functhion here. And will re-enable it
> once the root cause found.

Hmm, I experience problems with DMA too but on uart3. I'm using the same
workaround for the uart (I've just commented out the dma entries in the DT).
The problem manifests itself here such, that brcm_patchram_plus hangs 
while uploading the firmware to a BCM4330 connected at uart3 (reproducible).

So maybe there is a bug in the DMA-engine which not only effects SPI. Or 
both drivers contain the same error in handling DMA (maybe through c&p).
But that's just specualtion from me, I haven't looked further into that 
problem.

Regards,

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

On Mon, Sep 15, 2014 at 01:50:02PM +0200, Alexander Holler wrote:
> Am 10.09.2014 07:30, schrieb Robin Gong:
> >There is one weird data in rxfifo after one full rx/tx transfer
> >done sometimes. It looks a design issue and hard to workaround
> >totally, so disable dma functhion here. And will re-enable it
> >once the root cause found.
> 
> Hmm, I experience problems with DMA too but on uart3. I'm using the same
> workaround for the uart (I've just commented out the dma entries in the DT).
> The problem manifests itself here such, that brcm_patchram_plus
> hangs while uploading the firmware to a BCM4330 connected at uart3
> (reproducible).
> 
> So maybe there is a bug in the DMA-engine which not only effects
> SPI. Or both drivers contain the same error in handling DMA (maybe
> through c&p).
> But that's just specualtion from me, I haven't looked further into
> that problem.
> 
> Regards,
> 
> Alexander Holler
Thanks for your information share. But my issue should be caused by hardware,
since everything is ok if it runs on other i.mx6 chip. Is your board also based
on i.mx6 chip? If yes, hope you can raise your issue in freescale community or
contact with Andy whose mail address added in CC list fugang.duan-KZfg59tc24yupXw9JCaiEA@public.gmane.org
We have fix some bugs in UART DMA case.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Alexander Holler]

Am 16.09.2014 05:52, schrieb Robin Gong:
> On Mon, Sep 15, 2014 at 01:50:02PM +0200, Alexander Holler wrote:
>> Am 10.09.2014 07:30, schrieb Robin Gong:
>>> There is one weird data in rxfifo after one full rx/tx transfer
>>> done sometimes. It looks a design issue and hard to workaround
>>> totally, so disable dma functhion here. And will re-enable it
>>> once the root cause found.
>>
>> Hmm, I experience problems with DMA too but on uart3. I'm using the same
>> workaround for the uart (I've just commented out the dma entries in the DT).
>> The problem manifests itself here such, that brcm_patchram_plus
>> hangs while uploading the firmware to a BCM4330 connected at uart3
>> (reproducible).
>>
>> So maybe there is a bug in the DMA-engine which not only effects
>> SPI. Or both drivers contain the same error in handling DMA (maybe
>> through c&p).
>> But that's just specualtion from me, I haven't looked further into
>> that problem.
>>
>> Regards,
>>
>> Alexander Holler
> Thanks for your information share. But my issue should be caused by hardware,
> since everything is ok if it runs on other i.mx6 chip. Is your board also based
> on i.mx6 chip? If yes, hope you can raise your issue in freescale community or
> contact with Andy whose mail address added in CC list fugang.duan-KZfg59tc24yupXw9JCaiEA@public.gmane.org
> We have fix some bugs in UART DMA case.

It's an i.mx6q (Wandboard quad c1) where I have this problem with 
mainline and much older (but heavily patched freescale 3.10.x based) 
kernels. A quick web-search suggests that this problem exists since a 
long time (noticed mainly by people which try to use BT as this seems to 
be the major use case for high speed serial communication).

Regards,

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Robin Gong]

On Tue, Sep 16, 2014 at 11:41:55AM +0200, Alexander Holler wrote:
> Am 16.09.2014 05:52, schrieb Robin Gong:
> >On Mon, Sep 15, 2014 at 01:50:02PM +0200, Alexander Holler wrote:
> >>Am 10.09.2014 07:30, schrieb Robin Gong:
> >>>There is one weird data in rxfifo after one full rx/tx transfer
> >>>done sometimes. It looks a design issue and hard to workaround
> >>>totally, so disable dma functhion here. And will re-enable it
> >>>once the root cause found.
> >>
> >>Hmm, I experience problems with DMA too but on uart3. I'm using the same
> >>workaround for the uart (I've just commented out the dma entries in the DT).
> >>The problem manifests itself here such, that brcm_patchram_plus
> >>hangs while uploading the firmware to a BCM4330 connected at uart3
> >>(reproducible).
> >>
> >>So maybe there is a bug in the DMA-engine which not only effects
> >>SPI. Or both drivers contain the same error in handling DMA (maybe
> >>through c&p).
> >>But that's just specualtion from me, I haven't looked further into
> >>that problem.
> >>
> >>Regards,
> >>
> >>Alexander Holler
> >Thanks for your information share. But my issue should be caused by hardware,
> >since everything is ok if it runs on other i.mx6 chip. Is your board also based
> >on i.mx6 chip? If yes, hope you can raise your issue in freescale community or
> >contact with Andy whose mail address added in CC list fugang.duan@freescale.cm.
> >We have fix some bugs in UART DMA case.
> 
> It's an i.mx6q (Wandboard quad c1) where I have this problem with
> mainline and much older (but heavily patched freescale 3.10.x based)
> kernels. A quick web-search suggests that this problem exists since
> a long time (noticed mainly by people which try to use BT as this
> seems to be the major use case for high speed serial communication).
> 
> Regards,
> 
> Alexander Holler
Yes, we just have fixed the BT issue over high speed UART last month. Suggest
you trying our patches based on v3.10.

Re: [PATCH v2] ARM: dts: imx6dl: disable dma support for spi on i.mx6dl

[Alexander Holler]

Am 17.09.2014 10:51, schrieb Robin Gong:
> On Tue, Sep 16, 2014 at 11:41:55AM +0200, Alexander Holler wrote:
>> Am 16.09.2014 05:52, schrieb Robin Gong:
>>> On Mon, Sep 15, 2014 at 01:50:02PM +0200, Alexander Holler wrote:
>>>> Am 10.09.2014 07:30, schrieb Robin Gong:
>>>>> There is one weird data in rxfifo after one full rx/tx transfer
>>>>> done sometimes. It looks a design issue and hard to workaround
>>>>> totally, so disable dma functhion here. And will re-enable it
>>>>> once the root cause found.
>>>>
>>>> Hmm, I experience problems with DMA too but on uart3. I'm using the same
>>>> workaround for the uart (I've just commented out the dma entries in the DT).
>>>> The problem manifests itself here such, that brcm_patchram_plus
>>>> hangs while uploading the firmware to a BCM4330 connected at uart3
>>>> (reproducible).

(...)

> Yes, we just have fixed the BT issue over high speed UART last month. Suggest
> you trying our patches based on v3.10.

Thanks for the pointer. Will look if I will find these patches.

Regards,

Alexander Holler

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html