Re: [PATCH v4 1/2] dt-bindings: arm: rockchip: add support for Radxa ROCK Pi E v3.0

["Heiko Stübner" <heiko@sntech.de>]

Am Mittwoch, 14. August 2024, 22:04:51 CEST schrieb FUKAUMI Naoki:
> Hi,
> 
> thanks for the review!
> 
> On 8/14/24 20:30, Heiko Stübner wrote:
> > Am Mittwoch, 14. August 2024, 11:57:26 CEST schrieb FUKAUMI Naoki:
> >> Radxa ROCK Pi E v3.0 is a compact networking SBC[1] using the Rockchip
> >> RK3328 chip.
> >>
> >> [1] https://radxa.com/products/rockpi/pie
> >>
> >> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> >> ---
> >> Changes in v4:
> >> - update compatible string for OpenWrt
> >> - drop A-b tag
> >> Changes in v3:
> >> - collect A-b tag
> >> Changes in v2:
> >> - fix typo in commit message
> >> - add missing --- in commit message
> >> - add new section instead of new item in rockchip.yaml
> >> ---
> >>   Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> >> index 2ad835f4068e..1d426d4e1928 100644
> >> --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> >> @@ -790,6 +790,11 @@ properties:
> >>             - const: radxa,rockpi-e
> >>             - const: rockchip,rk3328
> >>   
> >> +      - description: Radxa ROCK Pi E v3.0
> >> +        items:
> >> +          - const: radxa,rock-pi-e-v3
> >> +          - const: rockchip,rk3328
> >> +
> > 
> > Please don't add a separate entry for the simple updated version.
> > Instead group it with the original Rock Pi e above.
> > 
> > For how to do it, look at the Radxa Zero 3w/3e .
> 
> https://patchwork.kernel.org/project/linux-rockchip/patch/20240711210526.40448-1-naoki@radxa.com/
> 
> is this right way?

Nope. You need an enum. That's the reason I pointed to the Radxa Zero as
an example [0]

So your entry should look something like:

       - description: Radxa ROCK Pi E
        items:
          - enum:
              - radxa,rockpi-e
              - radxa,rockpi-e-v3
          - const: rockchip,rk3328


that way the binding matches against both:
	compatible = "radxa,rockpi-e", "rockchip,rk3328"
as well as
	compatible = "radxa,rockpi-e-v3", "rockchip,rk3328"


Hope that helps a bit
Heiko


[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/rockchip.yaml#n844