On Tue, Jan 17, 2023 at 6:28 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 17/01/2023 13:52, Jagan Teki wrote:
> > On Tue, Jan 17, 2023 at 4:37 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>
> >> On 17/01/2023 11:55, Jagan Teki wrote:
> >>> Fix the model name and compatible strings for Radxa CM3i SoM
> >>> and Carrier board based on Radxa recommendation.
> >>>
> >>> Fixes: 2bf2f4d9f673 ("arm64: dts: rockchip: Add Radxa CM3I E25")
> >>> Cc: Chukun Pan <amadeus@xxxxxxxxxx>
> >>> Reported-by: FUKAUMI Naoki <naoki@xxxxxxxxx>
> >>> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> >>> ---
> >>> arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi | 2 +-
> >>> arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts | 4 ++--
> >>> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> >>> index 225dbbe4955d..0c9a918b809a 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-cm3i.dtsi
> >>> @@ -7,7 +7,7 @@
> >>>
> >>> / {
> >>> model = "Radxa CM3 Industrial Board";
> >>> - compatible = "radxa,cm3i", "rockchip,rk3568";
> >>> + compatible = "radxa,radxa-cm3i", "rockchip,rk3568";
> >>
> >> Nope, second radxa is redundant.
> >
> > It is not redundant, Product name and vendor name is same. Vendor name
> > is Radxa and product name is Radxa CM3i
>
> If this was true, then on their website the product would be called
> "Radxa Radxa CM3i", but it is not. It is called Radxa CM3i, to denote
> the company name with the model name. Anyway, in the name of model you
> can call it. In the context of compatible it is redundant.
Is it mandatory to consider the manufacturer as well in order to
denote the model name of the product?
This I what I thought, correct me if I'm wrong. The recommended
compatible format is "manufacturer,model" so we can consider only the
model in compatible string in order to denote the model name, not the
manufacturer. So with "radxa,radxa-cm3i" compatible the desired model
would be "Radxa CM3i"
>
> >
> >>
> >>>
> >>> aliases {
> >>> mmc0 = &sdhci;
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
> >>> index a4c33310ca42..8ca15b723799 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-radxa-e25.dts
> >>> @@ -4,8 +4,8 @@
> >>> #include "rk3568-radxa-cm3i.dtsi"
> >>>
> >>> / {
> >>> - model = "Radxa E25";
> >>> - compatible = "radxa,e25", "radxa,cm3i", "rockchip,rk3568";
> >>> + model = "Radxa E25 Carrier Board";
> >>> + compatible = "radxa,radxa-e25", "radxa,radxa-cm3i", "rockchip,rk3568";
> >>
> >> NAK. You just changed it in patch 2. Don't add broken compatibles in one
> >> patch (cm3i) and fix them next one. It's like adding intentional bugs
> >> and fixing them immediately...
> >
> > I did squash this with previous patch but the check patch showing
> > warning to separate the dt-bindings patch from actual devicetree file
> > change.
>
> Patch 2 was not a bindings patch, so you squashed wrong files.
I mean 3/7 patch - "[PATCH 3/7] dt-bindings: arm: rockchip: Fix
description and compatible for Radxa CM3i"
Jagan.
