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

[FUKAUMI Naoki <naoki@xxxxxxxxx>]

hi,

On 8/15/24 06:53, Heiko Stübner wrote:
> 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@xxxxxxxxx>
> > > > ---
> > > > 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@xxxxxxxxx/
> >
> > 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

I see. I'll do it in v5.

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

> 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