Re: [PATCH 3/7] dt-bindings: arm: rockchip: Fix description and compatible for Radxa CM3i

[Rob Herring <robh@kernel.org>]

On Tue, Jan 17, 2023 at 06:25:44PM +0530, Jagan Teki wrote:
> On Tue, Jan 17, 2023 at 4:36 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 17/01/2023 11:55, Jagan Teki wrote:
> > > Fix the dt-bindings for Radxa CM3i SoM based boards with proper
> > > description and compatible strings recommended by Radxa.
> > >
> > > Fixes: ae9fbe0b1f96 ("dt-bindings: arm: rockchip: add Radxa CM3I E25")
> > > Cc: Chukun Pan <amadeus@jmu.edu.cn>
> > > Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > index 3d3c59624483..6f276d787e50 100644
> > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > @@ -628,11 +628,11 @@ properties:
> > >            - const: radxa,radxa-cm3   # Radxa Compute Module 3(CM3)
> > >            - const: rockchip,rk3566
> > >
> > > -      - description: Radxa CM3 Industrial
> > > +      - description: Radxa Compute Module 3 Industrial(CM3i) SoM based boards
> > >          items:
> > >            - enum:
> > > -              - radxa,e25
> > > -          - const: radxa,cm3i
> > > +              - radxa,radxa-e25      # Radxa E25 Carrier Board
> > > +          - const: radxa,radxa-cm3i  # Radxa Compute Module 3 Industrial(CM3i)
> >
> > No, it does not match your patch 2/7. It also does not make sense to
> > duplicate vendor prefix.
> 
> 2/7 follows the existing bindings and that one is incorrect as the
> actual product name is radxa-cm3i not cm3i. This patch is trying to
> fix that.

Compatibles are unique (hopefully) identifiers, not 'product names'. 
Granted, 'e25' is a bit terse, but does Radxa have multiple things which 
are called 'e25'? If not, then you don't need this change. If the name 
is problematic in some other way, then state that reason for the patch.

Rob