| Message ID | 1410327012-31185-1-git-send-email-b38343@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@freescale.com> Applied, thanks.
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@freescale.com> > --- > 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"; > }; >
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
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
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@freescale.com> > > --- > > 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/ | >
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.
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
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
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/ | >
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.
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
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/ | >
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
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/ | >
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
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"; };
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@freescale.com> --- arch/arm/boot/dts/imx6q.dtsi | 20 ++++++++++++++++++++ arch/arm/boot/dts/imx6qdl.dtsi | 8 -------- 2 files changed, 20 insertions(+), 8 deletions(-)