| Message ID | 20230117105504.18296-4-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] dt-bindings: arm: rockchip: Add meaningful comments for Radxa CM3 | expand |
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@jmu.edu.cn> > Reported-by: FUKAUMI Naoki <naoki@radxa.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > 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. > > 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... Best regards, Krzysztof
On Tue, Jan 17, 2023 at 4:37 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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@jmu.edu.cn> > > Reported-by: FUKAUMI Naoki <naoki@radxa.com> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > 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 > > > > > 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. Jagan.
On 17/01/2023 13:52, Jagan Teki wrote: > On Tue, Jan 17, 2023 at 4:37 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> 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@jmu.edu.cn> >>> Reported-by: FUKAUMI Naoki <naoki@radxa.com> >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >>> --- >>> 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. > >> >>> >>> 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. Best regards, Krzysztof
On Tue, Jan 17, 2023 at 6:28 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/01/2023 13:52, Jagan Teki wrote: > > On Tue, Jan 17, 2023 at 4:37 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> 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@jmu.edu.cn> > >>> Reported-by: FUKAUMI Naoki <naoki@radxa.com> > >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > >>> --- > >>> 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.
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"; 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"; aliases { mmc0 = &sdmmc0;
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@jmu.edu.cn> Reported-by: FUKAUMI Naoki <naoki@radxa.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- 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(-)