[02/11] dt-bindings: serial: snps-dw-apb-uart: Relax dma-names order constraint

  Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
dma-names properties") documented dma-names property to handle Allwiner
D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
the reverse of what a different board expects:

  rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected

A quick and incomplete check shows the inconsistency is present in many
other DT files:

$ git grep -A10 snps,dw-apb-uart | grep dma-names | sort -u
arch/arm64/boot/dts/rockchip/px30.dtsi-         dma-names = "tx", "rx";
arch/arm64/boot/dts/rockchip/rk3328.dtsi-       dma-names = "tx", "rx";
arch/arm64/boot/dts/rockchip/rk3588s.dtsi-      dma-names = "tx", "rx";
arch/arm/boot/dts/rk3066a.dtsi-                 dma-names = "tx", "rx";
arch/arm/boot/dts/rk3128.dtsi-                  dma-names = "tx", "rx";
arch/arm/boot/dts/rk3288.dtsi-                  dma-names = "tx", "rx";
arch/arm/boot/dts/rv1126.dtsi-                  dma-names = "tx", "rx";
arch/arm/boot/dts/socfpga.dtsi-                 dma-names = "tx", "rx";
arch/arm/boot/dts/sun6i-a31.dtsi-               dma-names = "rx", "tx";
arch/arm/boot/dts/sun8i-a23-a33.dtsi-           dma-names = "rx", "tx";
arch/arm/boot/dts/sun8i-v3s.dtsi-               dma-names = "rx", "tx";
arch/arm/boot/dts/sunxi-h3-h5.dtsi-             dma-names = "rx", "tx";
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi-  dma-names = "rx", "tx";

Do not enforce the order of the dma-names items.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../devicetree/bindings/serial/snps-dw-apb-uart.yaml   | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
  

On 15/03/2023 12:47, Cristian Ciocaltea wrote:
> Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
> dma-names properties") documented dma-names property to handle Allwiner
> D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
> the reverse of what a different board expects:
> 
>   rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
> 
> A quick and incomplete check shows the inconsistency is present in many
> other DT files:

Why not fixing the DTS? The properties should have fixed order.

Best regards,
Krzysztof
  

On 3/17/23 10:31, Krzysztof Kozlowski wrote:
> On 15/03/2023 12:47, Cristian Ciocaltea wrote:
>> Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
>> dma-names properties") documented dma-names property to handle Allwiner
>> D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
>> the reverse of what a different board expects:
>>
>>    rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
>>
>> A quick and incomplete check shows the inconsistency is present in many
>> other DT files:
> 
> Why not fixing the DTS? The properties should have fixed order.

I was initially concerned about the risk of a potential ABI breakage, 
but I think that's not really a problem since dma-names is not directly 
accessed in the driver and DT Kernel API doesn't rely on a particular order.

If there are no objections, I would switch the order in the binding to 
tx->rx, since that's what most of the DTS use, and fix the remaining ones.

> Best regards,
> Krzysztof

Thanks,
Cristian
  

On 17/03/2023 11:21, Cristian Ciocaltea wrote:
> On 3/17/23 10:31, Krzysztof Kozlowski wrote:
>> On 15/03/2023 12:47, Cristian Ciocaltea wrote:
>>> Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
>>> dma-names properties") documented dma-names property to handle Allwiner
>>> D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
>>> the reverse of what a different board expects:
>>>
>>>    rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
>>>
>>> A quick and incomplete check shows the inconsistency is present in many
>>> other DT files:
>>
>> Why not fixing the DTS? The properties should have fixed order.
> 
> I was initially concerned about the risk of a potential ABI breakage, 
> but I think that's not really a problem since dma-names is not directly 
> accessed in the driver and DT Kernel API doesn't rely on a particular order.
> 
> If there are no objections, I would switch the order in the binding to 
> tx->rx, since that's what most of the DTS use, and fix the remaining ones.

Since we added the order recently, I rather assume it is the correct or
preferred one.

Best regards,
Krzysztof
  

On Fri, Mar 17, 2023 at 04:54:47PM +0100, Krzysztof Kozlowski wrote:
> On 17/03/2023 11:21, Cristian Ciocaltea wrote:
> > On 3/17/23 10:31, Krzysztof Kozlowski wrote:
> >> On 15/03/2023 12:47, Cristian Ciocaltea wrote:
> >>> Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
> >>> dma-names properties") documented dma-names property to handle Allwiner
> >>> D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
> >>> the reverse of what a different board expects:
> >>>
> >>>    rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
> >>>
> >>> A quick and incomplete check shows the inconsistency is present in many
> >>> other DT files:
> >>
> >> Why not fixing the DTS? The properties should have fixed order.
> > 
> > I was initially concerned about the risk of a potential ABI breakage, 
> > but I think that's not really a problem since dma-names is not directly 
> > accessed in the driver and DT Kernel API doesn't rely on a particular order.
> > 
> > If there are no objections, I would switch the order in the binding to 
> > tx->rx, since that's what most of the DTS use, and fix the remaining ones.
> 
> Since we added the order recently, I rather assume it is the correct or
> preferred one.

IIRC I checked around the other serial bindings & there was not a
consistent order that all serial bindings used, so I picked the order that
was used across the various allwinner boards that do use dma-names.

Before changing dts files, it's probably a good idea to make sure that
the dma-names are not used somewhere outside of Linux.

Cheers,
Conor.
  

On 3/17/23 18:26, Conor Dooley wrote:
> On Fri, Mar 17, 2023 at 04:54:47PM +0100, Krzysztof Kozlowski wrote:
>> On 17/03/2023 11:21, Cristian Ciocaltea wrote:
>>> On 3/17/23 10:31, Krzysztof Kozlowski wrote:
>>>> On 15/03/2023 12:47, Cristian Ciocaltea wrote:
>>>>> Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
>>>>> dma-names properties") documented dma-names property to handle Allwiner
>>>>> D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
>>>>> the reverse of what a different board expects:
>>>>>
>>>>>     rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
>>>>>
>>>>> A quick and incomplete check shows the inconsistency is present in many
>>>>> other DT files:
>>>>
>>>> Why not fixing the DTS? The properties should have fixed order.
>>>
>>> I was initially concerned about the risk of a potential ABI breakage,
>>> but I think that's not really a problem since dma-names is not directly
>>> accessed in the driver and DT Kernel API doesn't rely on a particular order.
>>>
>>> If there are no objections, I would switch the order in the binding to
>>> tx->rx, since that's what most of the DTS use, and fix the remaining ones.
>>
>> Since we added the order recently, I rather assume it is the correct or
>> preferred one.
> 
> IIRC I checked around the other serial bindings & there was not a
> consistent order that all serial bindings used, so I picked the order that
> was used across the various allwinner boards that do use dma-names.

Thanks for clarifying this, Conor! Would it be fine to switch to tx->rx 
order as it requires less changes to fix the inconsistencies?

> Before changing dts files, it's probably a good idea to make sure that
> the dma-names are not used somewhere outside of Linux.

Right, that means we cannot exclude the ABI breakage concern. Not sure 
how easy would be to actually verify this. Hence I wonder if there is 
really no chance to allow the flexible order in the binding..

> Cheers,
> Conor
  

On Fri, Mar 17, 2023 at 12:21:41PM +0200, Cristian Ciocaltea via Alsa-devel wrote:
> Date: Fri, 17 Mar 2023 12:21:41 +0200
> From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Sudeep Holla
>  <sudeep.holla@arm.com>, Cristian Marussi <cristian.marussi@arm.com>, Rob
>  Herring <robh+dt@kernel.org>, Krzysztof Kozlowski
>  <krzysztof.kozlowski+dt@linaro.org>, Greg Kroah-Hartman
>  <gregkh@linuxfoundation.org>, Liam Girdwood <lgirdwood@gmail.com>, Mark
>  Brown <broonie@kernel.org>, Nicolas Frattaroli
>  <frattaroli.nicolas@gmail.com>, Heiko Stuebner <heiko@sntech.de>, Jaroslav
>  Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, Paul Walmsley
>  <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou
>  <aou@eecs.berkeley.edu>, Daniel Drake <drake@endlessm.com>, Katsuhiro
>  Suzuki <katsuhiro@katsuster.net>
> CC: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
>  linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
>  alsa-devel@alsa-project.org, linux-rockchip@lists.infradead.org,
>  linux-riscv@lists.infradead.org, kernel@collabora.com
> Subject: Re: [PATCH 02/11] dt-bindings: serial: snps-dw-apb-uart: Relax
>  dma-names order constraint
> Message-ID: <8ae57fe3-56aa-7e50-3eaa-a12a40657baf@collabora.com>
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
>  Thunderbird/102.7.2

There is something strange going on with your mails as there are 2 
copies in the archives with the 2nd one getting the header twice. It's 
coming from the alsa-devel list.

> 
> On 3/17/23 10:31, Krzysztof Kozlowski wrote:
> > On 15/03/2023 12:47, Cristian Ciocaltea wrote:
> > > Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
> > > dma-names properties") documented dma-names property to handle Allwiner
> > > D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
> > > the reverse of what a different board expects:
> > > 
> > >    rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
> > > 
> > > A quick and incomplete check shows the inconsistency is present in many
> > > other DT files:
> > 
> > Why not fixing the DTS? The properties should have fixed order.
> 
> I was initially concerned about the risk of a potential ABI breakage, but I
> think that's not really a problem since dma-names is not directly accessed
> in the driver and DT Kernel API doesn't rely on a particular order.
> 
> If there are no objections, I would switch the order in the binding to
> tx->rx, since that's what most of the DTS use, and fix the remaining ones.
> 
> > Best regards,
> > Krzysztof
> 
> Thanks,
> Cristian
  

On Fri, Mar 17, 2023 at 07:43:53PM +0200, Cristian Ciocaltea wrote:
> On 3/17/23 18:26, Conor Dooley wrote:
> > On Fri, Mar 17, 2023 at 04:54:47PM +0100, Krzysztof Kozlowski wrote:
> > > On 17/03/2023 11:21, Cristian Ciocaltea wrote:
> > > > On 3/17/23 10:31, Krzysztof Kozlowski wrote:
> > > > > On 15/03/2023 12:47, Cristian Ciocaltea wrote:
> > > > > > Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
> > > > > > dma-names properties") documented dma-names property to handle Allwiner
> > > > > > D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
> > > > > > the reverse of what a different board expects:
> > > > > > 
> > > > > >     rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
> > > > > > 
> > > > > > A quick and incomplete check shows the inconsistency is present in many
> > > > > > other DT files:
> > > > > 
> > > > > Why not fixing the DTS? The properties should have fixed order.
> > > > 
> > > > I was initially concerned about the risk of a potential ABI breakage,
> > > > but I think that's not really a problem since dma-names is not directly
> > > > accessed in the driver and DT Kernel API doesn't rely on a particular order.
> > > > 
> > > > If there are no objections, I would switch the order in the binding to
> > > > tx->rx, since that's what most of the DTS use, and fix the remaining ones.
> > > 
> > > Since we added the order recently, I rather assume it is the correct or
> > > preferred one.
> > 
> > IIRC I checked around the other serial bindings & there was not a
> > consistent order that all serial bindings used, so I picked the order that
> > was used across the various allwinner boards that do use dma-names.
> 
> Thanks for clarifying this, Conor! Would it be fine to switch to tx->rx
> order as it requires less changes to fix the inconsistencies?
> 
> > Before changing dts files, it's probably a good idea to make sure that
> > the dma-names are not used somewhere outside of Linux.
> 
> Right, that means we cannot exclude the ABI breakage concern. Not sure how
> easy would be to actually verify this. Hence I wonder if there is really no
> chance to allow the flexible order in the binding..

If it changes and someone complains, then yes we'll allow flexible 
order.

Rob
  

On Mon, Mar 20, 2023 at 10:58:12AM -0500, Rob Herring wrote:
> On Fri, Mar 17, 2023 at 12:21:41PM +0200, Cristian Ciocaltea via Alsa-devel wrote:

> >  dma-names order constraint
> > Message-ID: <8ae57fe3-56aa-7e50-3eaa-a12a40657baf@collabora.com>
> > User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
> >  Thunderbird/102.7.2

> There is something strange going on with your mails as there are 2 
> copies in the archives with the 2nd one getting the header twice. It's 
> coming from the alsa-devel list.

This is probably caused by alsa-devel, it'll be mailman rewriting bits
of the message.  There's stuff coming up with other people's mails too.
  

On 3/20/23 18:01, Rob Herring wrote:
> On Fri, Mar 17, 2023 at 07:43:53PM +0200, Cristian Ciocaltea wrote:
>> On 3/17/23 18:26, Conor Dooley wrote:
>>> On Fri, Mar 17, 2023 at 04:54:47PM +0100, Krzysztof Kozlowski wrote:
>>>> On 17/03/2023 11:21, Cristian Ciocaltea wrote:
>>>>> On 3/17/23 10:31, Krzysztof Kozlowski wrote:
>>>>>> On 15/03/2023 12:47, Cristian Ciocaltea wrote:
>>>>>>> Commit 370f696e4474 ("dt-bindings: serial: snps-dw-apb-uart: add dma &
>>>>>>> dma-names properties") documented dma-names property to handle Allwiner
>>>>>>> D1 dtbs_check warnings, but relies on a strict rx->tx ordering, which is
>>>>>>> the reverse of what a different board expects:
>>>>>>>
>>>>>>>      rk3326-odroid-go2.dtb: serial@ff030000: dma-names:0: 'rx' was expected
>>>>>>>
>>>>>>> A quick and incomplete check shows the inconsistency is present in many
>>>>>>> other DT files:
>>>>>>
>>>>>> Why not fixing the DTS? The properties should have fixed order.
>>>>>
>>>>> I was initially concerned about the risk of a potential ABI breakage,
>>>>> but I think that's not really a problem since dma-names is not directly
>>>>> accessed in the driver and DT Kernel API doesn't rely on a particular order.
>>>>>
>>>>> If there are no objections, I would switch the order in the binding to
>>>>> tx->rx, since that's what most of the DTS use, and fix the remaining ones.
>>>>
>>>> Since we added the order recently, I rather assume it is the correct or
>>>> preferred one.
>>>
>>> IIRC I checked around the other serial bindings & there was not a
>>> consistent order that all serial bindings used, so I picked the order that
>>> was used across the various allwinner boards that do use dma-names.
>>
>> Thanks for clarifying this, Conor! Would it be fine to switch to tx->rx
>> order as it requires less changes to fix the inconsistencies?
>>
>>> Before changing dts files, it's probably a good idea to make sure that
>>> the dma-names are not used somewhere outside of Linux.
>>
>> Right, that means we cannot exclude the ABI breakage concern. Not sure how
>> easy would be to actually verify this. Hence I wonder if there is really no
>> chance to allow the flexible order in the binding..
> 
> If it changes and someone complains, then yes we'll allow flexible
> order.

I looked a bit further and it seems the allwiner boards are not really 
affected as all of them are using the same DMA channel for both rx and 
tx. So we should be fine by switching to tx->rx order.

$ git grep -A10 snps,dw-apb-uart | grep 'sun.*dmas' | sort -u

arch/arm/boot/dts/sun6i-a31.dtsi-	dmas = <&dma 10>, <&dma 10>;
arch/arm/boot/dts/sun6i-a31.dtsi-	dmas = <&dma 22>, <&dma 22>;
arch/arm/boot/dts/sun6i-a31.dtsi-	dmas = <&dma 6>, <&dma 6>;
arch/arm/boot/dts/sun6i-a31.dtsi-	dmas = <&dma 7>, <&dma 7>;
arch/arm/boot/dts/sun6i-a31.dtsi-	dmas = <&dma 8>, <&dma 8>;
arch/arm/boot/dts/sun6i-a31.dtsi-	dmas = <&dma 9>, <&dma 9>;
arch/arm/boot/dts/sun8i-a23-a33.dtsi-	dmas = <&dma 10>, <&dma 10>;
arch/arm/boot/dts/sun8i-a23-a33.dtsi-	dmas = <&dma 6>, <&dma 6>;
arch/arm/boot/dts/sun8i-a23-a33.dtsi-	dmas = <&dma 7>, <&dma 7>;
arch/arm/boot/dts/sun8i-a23-a33.dtsi-	dmas = <&dma 8>, <&dma 8>;
arch/arm/boot/dts/sun8i-a23-a33.dtsi-	dmas = <&dma 9>, <&dma 9>;
arch/arm/boot/dts/sun8i-v3s.dtsi-	dmas = <&dma 6>, <&dma 6>;
arch/arm/boot/dts/sun8i-v3s.dtsi-	dmas = <&dma 7>, <&dma 7>;
arch/arm/boot/dts/sun8i-v3s.dtsi-	dmas = <&dma 8>, <&dma 8>;
arch/arm/boot/dts/sunxi-h3-h5.dtsi-	dmas = <&dma 6>, <&dma 6>;
arch/arm/boot/dts/sunxi-h3-h5.dtsi-	dmas = <&dma 7>, <&dma 7>;
arch/arm/boot/dts/sunxi-h3-h5.dtsi-	dmas = <&dma 8>, <&dma 8>;
arch/arm/boot/dts/sunxi-h3-h5.dtsi-	dmas = <&dma 9>, <&dma 9>;
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi- dmas = <&dma 14>, 
<&dma 14>;
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi- dmas = <&dma 15>, 
<&dma 15>;
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi- dmas = <&dma 16>, 
<&dma 16>;
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi- dmas = <&dma 17>, 
<&dma 17>;
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi- dmas = <&dma 18>, 
<&dma 18>;
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi- dmas = <&dma 19>, 
<&dma 19>;

Thanks,
Cristian