| Message ID | 20240414083347.131724-3-ryan@testtoast.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: dts: allwinner: Add Anbernic RG35XX (Plus/H/2024) support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 14/04/2024 10:33, Ryan Walklin wrote: > From: Ryan Walklin <ryan@testtoast.com> > > RG35XX 2024: Base version with Allwinner H700 > RG35XX Plus: Adds Wifi/BT > RG35XX H: Adds second USB port and analog sticks to -Plus in horizontal > altered form factor, > > Signed-off-by: Ryan Walklin <ryan@testtoast.com> > --- > Documentation/devicetree/bindings/arm/sunxi.yaml | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml > index 09d835db6db5..fc10f54561c9 100644 > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml > @@ -56,6 +56,21 @@ properties: > - const: anbernic,rg-nano > - const: allwinner,sun8i-v3s > > + - description: Anbernic RG35XX (2024) > + - items: > + - const: anbernic,rg35xx-2024 > + - const: allwinner,sun50i-h700 > + > + - description: Anbernic RG35XX Plus > + - items: > + - const: anbernic,rg35xx-plus > + - const: allwinner,sun50i-h700 > + > + - description: Anbernic RG35XX H > + - items: > + - const: anbernic,rg35xx-h Any reason these are not just one enum with three entires? Best regards, Krzysztof
Thanks for the review and feedback. > Any reason these are not just one enum with three entires? No, this is just to match the existing devices, are you able to point to an example elsewhere? Ryan On Sun, 14 Apr 2024, at 9:07 PM, Krzysztof Kozlowski wrote: > On 14/04/2024 10:33, Ryan Walklin wrote: >> From: Ryan Walklin <ryan@testtoast.com> >> >> RG35XX 2024: Base version with Allwinner H700 >> RG35XX Plus: Adds Wifi/BT >> RG35XX H: Adds second USB port and analog sticks to -Plus in horizontal >> altered form factor, >> >> Signed-off-by: Ryan Walklin <ryan@testtoast.com> >> --- >> Documentation/devicetree/bindings/arm/sunxi.yaml | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml >> index 09d835db6db5..fc10f54561c9 100644 >> --- a/Documentation/devicetree/bindings/arm/sunxi.yaml >> +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml >> @@ -56,6 +56,21 @@ properties: >> - const: anbernic,rg-nano >> - const: allwinner,sun8i-v3s >> >> + - description: Anbernic RG35XX (2024) >> + - items: >> + - const: anbernic,rg35xx-2024 >> + - const: allwinner,sun50i-h700 >> + >> + - description: Anbernic RG35XX Plus >> + - items: >> + - const: anbernic,rg35xx-plus >> + - const: allwinner,sun50i-h700 >> + >> + - description: Anbernic RG35XX H >> + - items: >> + - const: anbernic,rg35xx-h > > Any reason these are not just one enum with three entires? > > Best regards, > Krzysztof
On Sun, 14 Apr 2024 11:07:19 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: Hi Krzysztof, > On 14/04/2024 10:33, Ryan Walklin wrote: > > From: Ryan Walklin <ryan@testtoast.com> > > > > RG35XX 2024: Base version with Allwinner H700 > > RG35XX Plus: Adds Wifi/BT > > RG35XX H: Adds second USB port and analog sticks to -Plus in horizontal > > altered form factor, > > > > Signed-off-by: Ryan Walklin <ryan@testtoast.com> > > --- > > Documentation/devicetree/bindings/arm/sunxi.yaml | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml > > index 09d835db6db5..fc10f54561c9 100644 > > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml > > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml > > @@ -56,6 +56,21 @@ properties: > > - const: anbernic,rg-nano > > - const: allwinner,sun8i-v3s > > > > + - description: Anbernic RG35XX (2024) > > + - items: > > + - const: anbernic,rg35xx-2024 > > + - const: allwinner,sun50i-h700 > > + > > + - description: Anbernic RG35XX Plus > > + - items: > > + - const: anbernic,rg35xx-plus > > + - const: allwinner,sun50i-h700 > > + > > + - description: Anbernic RG35XX H > > + - items: > > + - const: anbernic,rg35xx-h > > Any reason these are not just one enum with three entires? I think the policy in *this* file is one entry for each device, so that we can attribute a device name ("Anbernic RG35XX H") to the compatible string. I mean otherwise we could group all H616 boards for instance into one entry. Don't know if this is what we want? Cheers, Andre
On 17/04/2024 11:05, Ryan Walklin wrote: > Thanks for the review and feedback. > >> Any reason these are not just one enum with three entires? > > No, this is just to match the existing devices, are you able to point to an example elsewhere? > Even for variants of same boards? The examples are everywhere, e.g. Qualcomm or NXP. Best regards, Krzysztof
On Wed, 17 Apr 2024 15:33:13 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: Hi, > On 17/04/2024 11:05, Ryan Walklin wrote: > > Thanks for the review and feedback. > > > >> Any reason these are not just one enum with three entires? > > > > No, this is just to match the existing devices, are you able to point to an example elsewhere? > > > > Even for variants of same boards? > > The examples are everywhere, e.g. Qualcomm or NXP. We have separate entries for closely related boards (Pine64 H64 model A and B), and also indeed for updated variants (the various PinePhone revisions). That doesn't need the stay this way, of course. We would lose a quite natural way of putting a descriptive name to each compatible string (cf. "Pine64 PinePhone Developer Batch (1.0)"), but if the main purpose of this file is to *reserve* the compatible strings, it would indeed be shorter to use enums for related boards. Don't know if this would a real advantage, though. Cheers, Andre
On 17/04/2024 15:58, Andre Przywara wrote: > On Wed, 17 Apr 2024 15:33:13 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > Hi, > >> On 17/04/2024 11:05, Ryan Walklin wrote: >>> Thanks for the review and feedback. >>> >>>> Any reason these are not just one enum with three entires? >>> >>> No, this is just to match the existing devices, are you able to point to an example elsewhere? >>> >> >> Even for variants of same boards? >> >> The examples are everywhere, e.g. Qualcomm or NXP. > > We have separate entries for closely related boards (Pine64 H64 model A > and B), and also indeed for updated variants (the various PinePhone > revisions). > That doesn't need the stay this way, of course. We would lose a quite > natural way of putting a descriptive name to each compatible string (cf. > "Pine64 PinePhone Developer Batch (1.0)"), but if the main purpose of this > file is to *reserve* the compatible strings, it would indeed be shorter to > use enums for related boards. > Don't know if this would a real advantage, though. > If this matches your existing practice, then it is perfectly fine for me. I will probably still be bringing up this question from time to time, because for me it blows the binding unnecessarily making it harder to maintain/read, but that's only matter of taste. Best regards, Krzysztof
On 14/04/2024 10:33, Ryan Walklin wrote: > From: Ryan Walklin <ryan@testtoast.com> > > RG35XX 2024: Base version with Allwinner H700 > RG35XX Plus: Adds Wifi/BT > RG35XX H: Adds second USB port and analog sticks to -Plus in horizontal > altered form factor, > > Signed-off-by: Ryan Walklin <ryan@testtoast.com> > --- With the assumption this matches existing coding style for sunxi: Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml index 09d835db6db5..fc10f54561c9 100644 --- a/Documentation/devicetree/bindings/arm/sunxi.yaml +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml @@ -56,6 +56,21 @@ properties: - const: anbernic,rg-nano - const: allwinner,sun8i-v3s + - description: Anbernic RG35XX (2024) + - items: + - const: anbernic,rg35xx-2024 + - const: allwinner,sun50i-h700 + + - description: Anbernic RG35XX Plus + - items: + - const: anbernic,rg35xx-plus + - const: allwinner,sun50i-h700 + + - description: Anbernic RG35XX H + - items: + - const: anbernic,rg35xx-h + - const: allwinner,sun50i-h700 + - description: Amarula A64 Relic items: - const: amarula,a64-relic