* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-06 21:58 ` Martin Kaiser 0 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-06 21:58 UTC (permalink / raw) To: linux-arm-kernel According to the i.MX25 reference manuals, each SSI has four FIFOs. All of those FIFOs can store up to 15 entries. The fsl_ssi driver's internal default for the FIFO depth in 8. Set our non-default FIFO depth explicitly in the Device Tree. Signed-off-by: Martin Kaiser <martin@kaiser.cx> --- arch/arm/boot/dts/imx25.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi index 9725705..cf70df2 100644 --- a/arch/arm/boot/dts/imx25.dtsi +++ b/arch/arm/boot/dts/imx25.dtsi @@ -269,6 +269,7 @@ dmas = <&sdma 24 1 0>, <&sdma 25 1 0>; dma-names = "rx", "tx"; + fsl,fifo-depth = <15>; status = "disabled"; }; @@ -329,6 +330,7 @@ dmas = <&sdma 28 1 0>, <&sdma 29 1 0>; dma-names = "rx", "tx"; + fsl,fifo-depth = <15>; status = "disabled"; }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-06 21:58 ` Martin Kaiser 0 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-06 21:58 UTC (permalink / raw) To: Shawn Guo, Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, linux-arm-kernel, devicetree, linux-kernel Cc: Martin Kaiser According to the i.MX25 reference manuals, each SSI has four FIFOs. All of those FIFOs can store up to 15 entries. The fsl_ssi driver's internal default for the FIFO depth in 8. Set our non-default FIFO depth explicitly in the Device Tree. Signed-off-by: Martin Kaiser <martin@kaiser.cx> --- arch/arm/boot/dts/imx25.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi index 9725705..cf70df2 100644 --- a/arch/arm/boot/dts/imx25.dtsi +++ b/arch/arm/boot/dts/imx25.dtsi @@ -269,6 +269,7 @@ dmas = <&sdma 24 1 0>, <&sdma 25 1 0>; dma-names = "rx", "tx"; + fsl,fifo-depth = <15>; status = "disabled"; }; @@ -329,6 +330,7 @@ dmas = <&sdma 28 1 0>, <&sdma 29 1 0>; dma-names = "rx", "tx"; + fsl,fifo-depth = <15>; status = "disabled"; }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-06 21:58 ` Martin Kaiser @ 2018-03-06 22:04 ` Fabio Estevam -1 siblings, 0 replies; 20+ messages in thread From: Fabio Estevam @ 2018-03-06 22:04 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 6, 2018 at 6:58 PM, Martin Kaiser <martin@kaiser.cx> wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-06 22:04 ` Fabio Estevam 0 siblings, 0 replies; 20+ messages in thread From: Fabio Estevam @ 2018-03-06 22:04 UTC (permalink / raw) To: Martin Kaiser Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Tue, Mar 6, 2018 at 6:58 PM, Martin Kaiser <martin@kaiser.cx> wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-06 21:58 ` Martin Kaiser (?) @ 2018-03-08 15:11 ` Lothar Waßmann -1 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-08 15:11 UTC (permalink / raw) To: linux-arm-kernel Hi, On Tue, 6 Mar 2018 22:58:19 +0100 Martin Kaiser wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> > --- > arch/arm/boot/dts/imx25.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > index 9725705..cf70df2 100644 > --- a/arch/arm/boot/dts/imx25.dtsi > +++ b/arch/arm/boot/dts/imx25.dtsi > @@ -269,6 +269,7 @@ > dmas = <&sdma 24 1 0>, > <&sdma 25 1 0>; > dma-names = "rx", "tx"; > + fsl,fifo-depth = <15>; > status = "disabled"; > }; > > @@ -329,6 +330,7 @@ > dmas = <&sdma 28 1 0>, > <&sdma 29 1 0>; > dma-names = "rx", "tx"; > + fsl,fifo-depth = <15>; > status = "disabled"; > }; > You are changing the global .dtsi file. Did you test this change with all devices that are affected by it? Lothar Wa?mann ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-08 15:11 ` Lothar Waßmann 0 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-08 15:11 UTC (permalink / raw) To: Martin Kaiser Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, linux-arm-kernel, devicetree, linux-kernel Hi, On Tue, 6 Mar 2018 22:58:19 +0100 Martin Kaiser wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> > --- > arch/arm/boot/dts/imx25.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > index 9725705..cf70df2 100644 > --- a/arch/arm/boot/dts/imx25.dtsi > +++ b/arch/arm/boot/dts/imx25.dtsi > @@ -269,6 +269,7 @@ > dmas = <&sdma 24 1 0>, > <&sdma 25 1 0>; > dma-names = "rx", "tx"; > + fsl,fifo-depth = <15>; > status = "disabled"; > }; > > @@ -329,6 +330,7 @@ > dmas = <&sdma 28 1 0>, > <&sdma 29 1 0>; > dma-names = "rx", "tx"; > + fsl,fifo-depth = <15>; > status = "disabled"; > }; > You are changing the global .dtsi file. Did you test this change with all devices that are affected by it? Lothar Waßmann ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-08 15:11 ` Lothar Waßmann 0 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-08 15:11 UTC (permalink / raw) To: Martin Kaiser Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring, Sascha Hauer, Fabio Estevam, Shawn Guo, linux-arm-kernel Hi, On Tue, 6 Mar 2018 22:58:19 +0100 Martin Kaiser wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> > --- > arch/arm/boot/dts/imx25.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > index 9725705..cf70df2 100644 > --- a/arch/arm/boot/dts/imx25.dtsi > +++ b/arch/arm/boot/dts/imx25.dtsi > @@ -269,6 +269,7 @@ > dmas = <&sdma 24 1 0>, > <&sdma 25 1 0>; > dma-names = "rx", "tx"; > + fsl,fifo-depth = <15>; > status = "disabled"; > }; > > @@ -329,6 +330,7 @@ > dmas = <&sdma 28 1 0>, > <&sdma 29 1 0>; > dma-names = "rx", "tx"; > + fsl,fifo-depth = <15>; > status = "disabled"; > }; > You are changing the global .dtsi file. Did you test this change with all devices that are affected by it? Lothar Waßmann _______________________________________________ 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] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-08 15:11 ` Lothar Waßmann (?) @ 2018-03-08 15:38 ` Martin Kaiser -1 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-08 15:38 UTC (permalink / raw) To: linux-arm-kernel Hi Lothar, Thus wrote Lothar Wa?mann (LW at KARO-electronics.de): > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > index 9725705..cf70df2 100644 > > --- a/arch/arm/boot/dts/imx25.dtsi > > +++ b/arch/arm/boot/dts/imx25.dtsi > > @@ -269,6 +269,7 @@ > > dmas = <&sdma 24 1 0>, > > <&sdma 25 1 0>; > > dma-names = "rx", "tx"; > > + fsl,fifo-depth = <15>; > > status = "disabled"; > > }; > > @@ -329,6 +330,7 @@ > > dmas = <&sdma 28 1 0>, > > <&sdma 29 1 0>; > > dma-names = "rx", "tx"; > > + fsl,fifo-depth = <15>; > > status = "disabled"; > > }; > You are changing the global .dtsi file. Did you test this change with > all devices that are affected by it? I changed the hardware description of the imx25 SSI to match the reference manual. I did test this change on an imx25 board with audio playback. This uses the SSI description I modified. I verified that the driver is actually taking the modified setting into account and that this causes no problems. As of today, this setting is used by the fsl_ssi driver to set the fifo water level for dma requests. Of course, I don't have access to the enitre range of supported imx25 boards and I don't think this is required for submitting patches. Do you have any indication why this patch should not be merged? Best regards, Martin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-08 15:38 ` Martin Kaiser 0 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-08 15:38 UTC (permalink / raw) To: Lothar Waßmann Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, linux-arm-kernel, devicetree, linux-kernel Hi Lothar, Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > index 9725705..cf70df2 100644 > > --- a/arch/arm/boot/dts/imx25.dtsi > > +++ b/arch/arm/boot/dts/imx25.dtsi > > @@ -269,6 +269,7 @@ > > dmas = <&sdma 24 1 0>, > > <&sdma 25 1 0>; > > dma-names = "rx", "tx"; > > + fsl,fifo-depth = <15>; > > status = "disabled"; > > }; > > @@ -329,6 +330,7 @@ > > dmas = <&sdma 28 1 0>, > > <&sdma 29 1 0>; > > dma-names = "rx", "tx"; > > + fsl,fifo-depth = <15>; > > status = "disabled"; > > }; > You are changing the global .dtsi file. Did you test this change with > all devices that are affected by it? I changed the hardware description of the imx25 SSI to match the reference manual. I did test this change on an imx25 board with audio playback. This uses the SSI description I modified. I verified that the driver is actually taking the modified setting into account and that this causes no problems. As of today, this setting is used by the fsl_ssi driver to set the fifo water level for dma requests. Of course, I don't have access to the enitre range of supported imx25 boards and I don't think this is required for submitting patches. Do you have any indication why this patch should not be merged? Best regards, Martin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-08 15:38 ` Martin Kaiser 0 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-08 15:38 UTC (permalink / raw) To: Lothar Waßmann Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring, Sascha Hauer, Fabio Estevam, Shawn Guo, linux-arm-kernel Hi Lothar, Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > index 9725705..cf70df2 100644 > > --- a/arch/arm/boot/dts/imx25.dtsi > > +++ b/arch/arm/boot/dts/imx25.dtsi > > @@ -269,6 +269,7 @@ > > dmas = <&sdma 24 1 0>, > > <&sdma 25 1 0>; > > dma-names = "rx", "tx"; > > + fsl,fifo-depth = <15>; > > status = "disabled"; > > }; > > @@ -329,6 +330,7 @@ > > dmas = <&sdma 28 1 0>, > > <&sdma 29 1 0>; > > dma-names = "rx", "tx"; > > + fsl,fifo-depth = <15>; > > status = "disabled"; > > }; > You are changing the global .dtsi file. Did you test this change with > all devices that are affected by it? I changed the hardware description of the imx25 SSI to match the reference manual. I did test this change on an imx25 board with audio playback. This uses the SSI description I modified. I verified that the driver is actually taking the modified setting into account and that this causes no problems. As of today, this setting is used by the fsl_ssi driver to set the fifo water level for dma requests. Of course, I don't have access to the enitre range of supported imx25 boards and I don't think this is required for submitting patches. Do you have any indication why this patch should not be merged? Best regards, Martin ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-08 15:38 ` Martin Kaiser @ 2018-03-09 8:37 ` Lothar Waßmann -1 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-09 8:37 UTC (permalink / raw) To: linux-arm-kernel Hi, On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > Hi Lothar, > > Thus wrote Lothar Wa?mann (LW at KARO-electronics.de): > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > index 9725705..cf70df2 100644 > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > @@ -269,6 +269,7 @@ > > > dmas = <&sdma 24 1 0>, > > > <&sdma 25 1 0>; > > > dma-names = "rx", "tx"; > > > + fsl,fifo-depth = <15>; > > > status = "disabled"; > > > }; > > > > @@ -329,6 +330,7 @@ > > > dmas = <&sdma 28 1 0>, > > > <&sdma 29 1 0>; > > > dma-names = "rx", "tx"; > > > + fsl,fifo-depth = <15>; > > > status = "disabled"; > > > }; > > > You are changing the global .dtsi file. Did you test this change with > > all devices that are affected by it? > > I changed the hardware description of the imx25 SSI to match the > reference manual. > > I did test this change on an imx25 board with audio playback. This uses > the SSI description I modified. I verified that the driver is actually > taking the modified setting into account and that this causes no > problems. > > As of today, this setting is used by the fsl_ssi driver to set the fifo > water level for dma requests. > > Of course, I don't have access to the enitre range of supported imx25 > boards and I don't think this is required for submitting patches. > > Do you have any indication why this patch should not be merged? > Usually patches should not willy-nilly change the behaviour of existing configurations unless they fix any real life bugs. Lothar Wa?mann ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-09 8:37 ` Lothar Waßmann 0 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-09 8:37 UTC (permalink / raw) To: Martin Kaiser Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, linux-arm-kernel, devicetree, linux-kernel Hi, On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > Hi Lothar, > > Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > index 9725705..cf70df2 100644 > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > @@ -269,6 +269,7 @@ > > > dmas = <&sdma 24 1 0>, > > > <&sdma 25 1 0>; > > > dma-names = "rx", "tx"; > > > + fsl,fifo-depth = <15>; > > > status = "disabled"; > > > }; > > > > @@ -329,6 +330,7 @@ > > > dmas = <&sdma 28 1 0>, > > > <&sdma 29 1 0>; > > > dma-names = "rx", "tx"; > > > + fsl,fifo-depth = <15>; > > > status = "disabled"; > > > }; > > > You are changing the global .dtsi file. Did you test this change with > > all devices that are affected by it? > > I changed the hardware description of the imx25 SSI to match the > reference manual. > > I did test this change on an imx25 board with audio playback. This uses > the SSI description I modified. I verified that the driver is actually > taking the modified setting into account and that this causes no > problems. > > As of today, this setting is used by the fsl_ssi driver to set the fifo > water level for dma requests. > > Of course, I don't have access to the enitre range of supported imx25 > boards and I don't think this is required for submitting patches. > > Do you have any indication why this patch should not be merged? > Usually patches should not willy-nilly change the behaviour of existing configurations unless they fix any real life bugs. Lothar Waßmann ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-09 8:37 ` Lothar Waßmann @ 2018-03-09 9:25 ` Martin Kaiser -1 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-09 9:25 UTC (permalink / raw) To: linux-arm-kernel Hi Lothar, Thus wrote Lothar Wa?mann (LW at KARO-electronics.de): > On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > > Hi Lothar, > > Thus wrote Lothar Wa?mann (LW at KARO-electronics.de): > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > index 9725705..cf70df2 100644 > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > @@ -269,6 +269,7 @@ > > > > dmas = <&sdma 24 1 0>, > > > > <&sdma 25 1 0>; > > > > dma-names = "rx", "tx"; > > > > + fsl,fifo-depth = <15>; > > > > status = "disabled"; > > > > }; > > > > @@ -329,6 +330,7 @@ > > > > dmas = <&sdma 28 1 0>, > > > > <&sdma 29 1 0>; > > > > dma-names = "rx", "tx"; > > > > + fsl,fifo-depth = <15>; > > > > status = "disabled"; > > > > }; > > > You are changing the global .dtsi file. Did you test this change with > > > all devices that are affected by it? > > I changed the hardware description of the imx25 SSI to match the > > reference manual. > > I did test this change on an imx25 board with audio playback. This uses > > the SSI description I modified. I verified that the driver is actually > > taking the modified setting into account and that this causes no > > problems. > > As of today, this setting is used by the fsl_ssi driver to set the fifo > > water level for dma requests. > > Of course, I don't have access to the enitre range of supported imx25 > > boards and I don't think this is required for submitting patches. > > Do you have any indication why this patch should not be merged? > Usually patches should not willy-nilly change the behaviour of existing > configurations unless they fix any real life bugs. We both made our points, let the maintainer decide what to do with the patch. Thanks & Best regards, Martin ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-09 9:25 ` Martin Kaiser 0 siblings, 0 replies; 20+ messages in thread From: Martin Kaiser @ 2018-03-09 9:25 UTC (permalink / raw) To: Lothar Waßmann Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, linux-arm-kernel, devicetree, linux-kernel Hi Lothar, Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > > Hi Lothar, > > Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > index 9725705..cf70df2 100644 > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > @@ -269,6 +269,7 @@ > > > > dmas = <&sdma 24 1 0>, > > > > <&sdma 25 1 0>; > > > > dma-names = "rx", "tx"; > > > > + fsl,fifo-depth = <15>; > > > > status = "disabled"; > > > > }; > > > > @@ -329,6 +330,7 @@ > > > > dmas = <&sdma 28 1 0>, > > > > <&sdma 29 1 0>; > > > > dma-names = "rx", "tx"; > > > > + fsl,fifo-depth = <15>; > > > > status = "disabled"; > > > > }; > > > You are changing the global .dtsi file. Did you test this change with > > > all devices that are affected by it? > > I changed the hardware description of the imx25 SSI to match the > > reference manual. > > I did test this change on an imx25 board with audio playback. This uses > > the SSI description I modified. I verified that the driver is actually > > taking the modified setting into account and that this causes no > > problems. > > As of today, this setting is used by the fsl_ssi driver to set the fifo > > water level for dma requests. > > Of course, I don't have access to the enitre range of supported imx25 > > boards and I don't think this is required for submitting patches. > > Do you have any indication why this patch should not be merged? > Usually patches should not willy-nilly change the behaviour of existing > configurations unless they fix any real life bugs. We both made our points, let the maintainer decide what to do with the patch. Thanks & Best regards, Martin ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-09 8:37 ` Lothar Waßmann @ 2018-03-09 9:27 ` Lucas Stach -1 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2018-03-09 9:27 UTC (permalink / raw) To: linux-arm-kernel Hi Lothar, Am Freitag, den 09.03.2018, 09:37 +0100 schrieb Lothar Wa?mann: > Hi, > > On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > > Hi Lothar, > > > > > > Thus wrote Lothar Wa?mann (LW at KARO-electronics.de): > > > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > index 9725705..cf70df2 100644 > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > @@ -269,6 +269,7 @@ > > > > > > > > ? dmas = <&sdma 24 1 0>, > > > > > > > > ? ???????<&sdma 25 1 0>; > > > > > > > > ? dma-names = "rx", "tx"; > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > ? status = "disabled"; > > > > > > > > ? }; > > > > @@ -329,6 +330,7 @@ > > > > > > > > ? dmas = <&sdma 28 1 0>, > > > > > > > > ? ???????<&sdma 29 1 0>; > > > > > > > > ? dma-names = "rx", "tx"; > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > ? status = "disabled"; > > > > ? }; > > > You are changing the global .dtsi file. Did you test this change with > > > all devices that are affected by it? > > > > I changed the hardware description of the imx25 SSI to match the > > reference manual. > > > > I did test this change on an imx25 board with audio playback. This uses > > the SSI description I modified. I verified that the driver is actually > > taking the modified setting into account and that this causes no > > problems. > > > > As of today, this setting is used by the fsl_ssi driver to set the fifo > > water level for dma requests. > > > > Of course, I don't have access to the enitre range of supported imx25 > > boards and I don't think this is required for submitting patches. > > > > Do you have any indication why this patch should not be merged? > > > > Usually patches should not willy-nilly change the behaviour of existing > configurations unless they fix any real life bugs. With this logic we wouldn't be able to get most driver changes applied. If it is matching the reference manual and has been tested on the affected hardware it should be good to go. If you know about any specific corner cases that might break with this change, please speak up now. Don't reject patches based on the overarching "it might break something" argument. Regards, Lucas ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-09 9:27 ` Lucas Stach 0 siblings, 0 replies; 20+ messages in thread From: Lucas Stach @ 2018-03-09 9:27 UTC (permalink / raw) To: Lothar Waßmann, Martin Kaiser Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring, Sascha Hauer, Fabio Estevam, Shawn Guo, linux-arm-kernel Hi Lothar, Am Freitag, den 09.03.2018, 09:37 +0100 schrieb Lothar Waßmann: > Hi, > > On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > > Hi Lothar, > > > > > > Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > > > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > index 9725705..cf70df2 100644 > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > @@ -269,6 +269,7 @@ > > > > > > > > dmas = <&sdma 24 1 0>, > > > > > > > > <&sdma 25 1 0>; > > > > > > > > dma-names = "rx", "tx"; > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > status = "disabled"; > > > > > > > > }; > > > > @@ -329,6 +330,7 @@ > > > > > > > > dmas = <&sdma 28 1 0>, > > > > > > > > <&sdma 29 1 0>; > > > > > > > > dma-names = "rx", "tx"; > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > status = "disabled"; > > > > }; > > > You are changing the global .dtsi file. Did you test this change with > > > all devices that are affected by it? > > > > I changed the hardware description of the imx25 SSI to match the > > reference manual. > > > > I did test this change on an imx25 board with audio playback. This uses > > the SSI description I modified. I verified that the driver is actually > > taking the modified setting into account and that this causes no > > problems. > > > > As of today, this setting is used by the fsl_ssi driver to set the fifo > > water level for dma requests. > > > > Of course, I don't have access to the enitre range of supported imx25 > > boards and I don't think this is required for submitting patches. > > > > Do you have any indication why this patch should not be merged? > > > > Usually patches should not willy-nilly change the behaviour of existing > configurations unless they fix any real life bugs. With this logic we wouldn't be able to get most driver changes applied. If it is matching the reference manual and has been tested on the affected hardware it should be good to go. If you know about any specific corner cases that might break with this change, please speak up now. Don't reject patches based on the overarching "it might break something" argument. Regards, Lucas ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-09 9:27 ` Lucas Stach @ 2018-03-09 15:40 ` Lothar Waßmann -1 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-09 15:40 UTC (permalink / raw) To: linux-arm-kernel Hi, On Fri, 09 Mar 2018 10:27:12 +0100 Lucas Stach wrote: > Hi Lothar, > > Am Freitag, den 09.03.2018, 09:37 +0100 schrieb Lothar Wa?mann: > > Hi, > > > > On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > > > Hi Lothar, > > > > > > > > Thus wrote Lothar Wa?mann (LW at KARO-electronics.de): > > > > > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > > index 9725705..cf70df2 100644 > > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > > @@ -269,6 +269,7 @@ > > > > > > > > > ? dmas = <&sdma 24 1 0>, > > > > > > > > > ? ???????<&sdma 25 1 0>; > > > > > > > > > ? dma-names = "rx", "tx"; > > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > > ? status = "disabled"; > > > > > > > > > ? }; > > > > > @@ -329,6 +330,7 @@ > > > > > > > > > ? dmas = <&sdma 28 1 0>, > > > > > > > > > ? ???????<&sdma 29 1 0>; > > > > > > > > > ? dma-names = "rx", "tx"; > > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > > ? status = "disabled"; > > > > > ? }; > > > > You are changing the global .dtsi file. Did you test this change with > > > > all devices that are affected by it? > > > > > > I changed the hardware description of the imx25 SSI to match the > > > reference manual. > > > > > > I did test this change on an imx25 board with audio playback. This uses > > > the SSI description I modified. I verified that the driver is actually > > > taking the modified setting into account and that this causes no > > > problems. > > > > > > As of today, this setting is used by the fsl_ssi driver to set the fifo > > > water level for dma requests. > > > > > > Of course, I don't have access to the enitre range of supported imx25 > > > boards and I don't think this is required for submitting patches. > > > > > > Do you have any indication why this patch should not be merged? > > > > > > > Usually patches should not willy-nilly change the behaviour of existing > > configurations unless they fix any real life bugs. > > With this logic we wouldn't be able to get most driver changes applied. > If it is matching the reference manual and has been tested on the > affected hardware it should be good to go. > > If you know about any specific corner cases that might break with this > change, please speak up now. Don't reject patches based on the > overarching "it might break something" argument. > I didn't reject anything, I just wanted to make sure, that the implications of this patch were sufficiently considered. Lothar Wa?mann ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-09 15:40 ` Lothar Waßmann 0 siblings, 0 replies; 20+ messages in thread From: Lothar Waßmann @ 2018-03-09 15:40 UTC (permalink / raw) To: Lucas Stach Cc: Martin Kaiser, Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring, Sascha Hauer, Fabio Estevam, Shawn Guo, linux-arm-kernel Hi, On Fri, 09 Mar 2018 10:27:12 +0100 Lucas Stach wrote: > Hi Lothar, > > Am Freitag, den 09.03.2018, 09:37 +0100 schrieb Lothar Waßmann: > > Hi, > > > > On Thu, 8 Mar 2018 16:38:32 +0100 Martin Kaiser wrote: > > > Hi Lothar, > > > > > > > > Thus wrote Lothar Waßmann (LW@KARO-electronics.de): > > > > > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > > index 9725705..cf70df2 100644 > > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > > @@ -269,6 +269,7 @@ > > > > > > > > > dmas = <&sdma 24 1 0>, > > > > > > > > > <&sdma 25 1 0>; > > > > > > > > > dma-names = "rx", "tx"; > > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > > status = "disabled"; > > > > > > > > > }; > > > > > @@ -329,6 +330,7 @@ > > > > > > > > > dmas = <&sdma 28 1 0>, > > > > > > > > > <&sdma 29 1 0>; > > > > > > > > > dma-names = "rx", "tx"; > > > > > > > > > + fsl,fifo-depth = <15>; > > > > > > > > > status = "disabled"; > > > > > }; > > > > You are changing the global .dtsi file. Did you test this change with > > > > all devices that are affected by it? > > > > > > I changed the hardware description of the imx25 SSI to match the > > > reference manual. > > > > > > I did test this change on an imx25 board with audio playback. This uses > > > the SSI description I modified. I verified that the driver is actually > > > taking the modified setting into account and that this causes no > > > problems. > > > > > > As of today, this setting is used by the fsl_ssi driver to set the fifo > > > water level for dma requests. > > > > > > Of course, I don't have access to the enitre range of supported imx25 > > > boards and I don't think this is required for submitting patches. > > > > > > Do you have any indication why this patch should not be merged? > > > > > > > Usually patches should not willy-nilly change the behaviour of existing > > configurations unless they fix any real life bugs. > > With this logic we wouldn't be able to get most driver changes applied. > If it is matching the reference manual and has been tested on the > affected hardware it should be good to go. > > If you know about any specific corner cases that might break with this > change, please speak up now. Don't reject patches based on the > overarching "it might break something" argument. > I didn't reject anything, I just wanted to make sure, that the implications of this patch were sufficiently considered. Lothar Waßmann ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: i.MX25: define SSI FIFO depth 2018-03-06 21:58 ` Martin Kaiser @ 2018-03-12 7:24 ` Shawn Guo -1 siblings, 0 replies; 20+ messages in thread From: Shawn Guo @ 2018-03-12 7:24 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 06, 2018 at 10:58:19PM +0100, Martin Kaiser wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> Applied, thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: i.MX25: define SSI FIFO depth @ 2018-03-12 7:24 ` Shawn Guo 0 siblings, 0 replies; 20+ messages in thread From: Shawn Guo @ 2018-03-12 7:24 UTC (permalink / raw) To: Martin Kaiser Cc: Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland, Russell King, linux-arm-kernel, devicetree, linux-kernel On Tue, Mar 06, 2018 at 10:58:19PM +0100, Martin Kaiser wrote: > According to the i.MX25 reference manuals, each SSI has four FIFOs. All > of those FIFOs can store up to 15 entries. > > The fsl_ssi driver's internal default for the FIFO depth in 8. Set our > non-default FIFO depth explicitly in the Device Tree. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> Applied, thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-03-12 7:24 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-06 21:58 [PATCH] ARM: dts: i.MX25: define SSI FIFO depth Martin Kaiser 2018-03-06 21:58 ` Martin Kaiser 2018-03-06 22:04 ` Fabio Estevam 2018-03-06 22:04 ` Fabio Estevam 2018-03-08 15:11 ` Lothar Waßmann 2018-03-08 15:11 ` Lothar Waßmann 2018-03-08 15:11 ` Lothar Waßmann 2018-03-08 15:38 ` Martin Kaiser 2018-03-08 15:38 ` Martin Kaiser 2018-03-08 15:38 ` Martin Kaiser 2018-03-09 8:37 ` Lothar Waßmann 2018-03-09 8:37 ` Lothar Waßmann 2018-03-09 9:25 ` Martin Kaiser 2018-03-09 9:25 ` Martin Kaiser 2018-03-09 9:27 ` Lucas Stach 2018-03-09 9:27 ` Lucas Stach 2018-03-09 15:40 ` Lothar Waßmann 2018-03-09 15:40 ` Lothar Waßmann 2018-03-12 7:24 ` Shawn Guo 2018-03-12 7:24 ` Shawn Guo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.