diff mbox series

[v2,1/3] reset: hisilicon: correct vendor prefix

Message ID 20201204014236.1158-2-thunder.leizhen@huawei.com
State New
Headers show
Series dt-bindings: reset: convert Hisilicon reset controller bindings to json-schema | expand

Commit Message

Zhen Lei Dec. 4, 2020, 1:42 a.m. UTC 
The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly
stated in "vendor-prefixes.yaml".

Fixes: 1527058736fa ("reset: hisilicon: add reset-hi3660")
Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")
Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Chen Feng <puck.chen@hisilicon.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 4 ++--
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 2 +-
 drivers/reset/hisilicon/reset-hi3660.c    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
1.8.3

Comments

Rob Herring (Arm) Dec. 7, 2020, 11:08 p.m. UTC | #1 
On Fri, Dec 04, 2020 at 09:42:34AM +0800, Zhen Lei wrote:
> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly

> stated in "vendor-prefixes.yaml".


Yes, but you can't fix this as changing it breaks compability between 
DTBs and kernels.

hisi has to be documented and marked 'deprecated'.

> 

> Fixes: 1527058736fa ("reset: hisilicon: add reset-hi3660")

> Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")

> Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")

> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>

> Cc: Chen Feng <puck.chen@hisilicon.com>

> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---

>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 4 ++--

>  arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 2 +-

>  drivers/reset/hisilicon/reset-hi3660.c    | 2 +-

>  3 files changed, 4 insertions(+), 4 deletions(-)

> 

> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

> index 49c19c6879f95ce..bfb1375426d2b58 100644

> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

> @@ -345,7 +345,7 @@

>  		crg_rst: crg_rst_controller {

>  			compatible = "hisilicon,hi3660-reset";

>  			#reset-cells = <2>;

> -			hisi,rst-syscon = <&crg_ctrl>;

> +			hisilicon,rst-syscon = <&crg_ctrl>;

>  		};

>  

>  

> @@ -376,7 +376,7 @@

>  

>  		iomcu_rst: reset {

>  			compatible = "hisilicon,hi3660-reset";

> -			hisi,rst-syscon = <&iomcu>;

> +			hisilicon,rst-syscon = <&iomcu>;

>  			#reset-cells = <2>;

>  		};

>  

> diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

> index 85b0dfb35d6d396..5c5a5dc964ea848 100644

> --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

> +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

> @@ -155,7 +155,7 @@

>  			compatible = "hisilicon,hi3670-reset",

>  				     "hisilicon,hi3660-reset";

>  			#reset-cells = <2>;

> -			hisi,rst-syscon = <&crg_ctrl>;

> +			hisilicon,rst-syscon = <&crg_ctrl>;

>  		};

>  

>  		pctrl: pctrl@e8a09000 {

> diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c

> index a7d4445924e558c..8f1953159a65b31 100644

> --- a/drivers/reset/hisilicon/reset-hi3660.c

> +++ b/drivers/reset/hisilicon/reset-hi3660.c

> @@ -83,7 +83,7 @@ static int hi3660_reset_probe(struct platform_device *pdev)

>  	if (!rc)

>  		return -ENOMEM;

>  

> -	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");

> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");

>  	if (IS_ERR(rc->map)) {

>  		dev_err(dev, "failed to get hi3660,rst-syscon\n");

>  		return PTR_ERR(rc->map);

> -- 

> 1.8.3

> 

>
Zhen Lei Dec. 8, 2020, 2:16 a.m. UTC | #2 
On 2020/12/8 7:08, Rob Herring wrote:
> On Fri, Dec 04, 2020 at 09:42:34AM +0800, Zhen Lei wrote:

>> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly

>> stated in "vendor-prefixes.yaml".

> 

> Yes, but you can't fix this as changing it breaks compability between 

> DTBs and kernels.

> 

> hisi has to be documented and marked 'deprecated'.


I searched, and this is the only place that uses the hisi prefix. Currently,
YAML check will report warnings due to mismatch any of the regexes defined
in Documentation/devicetree/bindings/vendor-prefixes.yaml

Is it not good to add hisi prefix to clear warnings?
"^hisi,.*":
  description: Hisilicon Limited, deprecated.


By the way, Hi Rob:
The license information in some YAML files is incorrect. Can you correct it in v5.11?
For example:
WARNING: DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause)
#26: FILE: Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml:1:
+# SPDX-License-Identifier: GPL-2.0

> 

>>

>> Fixes: 1527058736fa ("reset: hisilicon: add reset-hi3660")

>> Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")

>> Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")

>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

>> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>

>> Cc: Chen Feng <puck.chen@hisilicon.com>

>> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

>> ---

>>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 4 ++--

>>  arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 2 +-

>>  drivers/reset/hisilicon/reset-hi3660.c    | 2 +-

>>  3 files changed, 4 insertions(+), 4 deletions(-)

>>

>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

>> index 49c19c6879f95ce..bfb1375426d2b58 100644

>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

>> @@ -345,7 +345,7 @@

>>  		crg_rst: crg_rst_controller {

>>  			compatible = "hisilicon,hi3660-reset";

>>  			#reset-cells = <2>;

>> -			hisi,rst-syscon = <&crg_ctrl>;

>> +			hisilicon,rst-syscon = <&crg_ctrl>;

>>  		};

>>  

>>  

>> @@ -376,7 +376,7 @@

>>  

>>  		iomcu_rst: reset {

>>  			compatible = "hisilicon,hi3660-reset";

>> -			hisi,rst-syscon = <&iomcu>;

>> +			hisilicon,rst-syscon = <&iomcu>;

>>  			#reset-cells = <2>;

>>  		};

>>  

>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

>> index 85b0dfb35d6d396..5c5a5dc964ea848 100644

>> --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

>> +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

>> @@ -155,7 +155,7 @@

>>  			compatible = "hisilicon,hi3670-reset",

>>  				     "hisilicon,hi3660-reset";

>>  			#reset-cells = <2>;

>> -			hisi,rst-syscon = <&crg_ctrl>;

>> +			hisilicon,rst-syscon = <&crg_ctrl>;

>>  		};

>>  

>>  		pctrl: pctrl@e8a09000 {

>> diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c

>> index a7d4445924e558c..8f1953159a65b31 100644

>> --- a/drivers/reset/hisilicon/reset-hi3660.c

>> +++ b/drivers/reset/hisilicon/reset-hi3660.c

>> @@ -83,7 +83,7 @@ static int hi3660_reset_probe(struct platform_device *pdev)

>>  	if (!rc)

>>  		return -ENOMEM;

>>  

>> -	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");

>> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");

>>  	if (IS_ERR(rc->map)) {

>>  		dev_err(dev, "failed to get hi3660,rst-syscon\n");

>>  		return PTR_ERR(rc->map);

>> -- 

>> 1.8.3

>>

>>

> 

> .

>
Philipp Zabel Dec. 8, 2020, 9:25 a.m. UTC | #3 
Hi Zhen,

On Fri, 2020-12-04 at 09:42 +0800, Zhen Lei wrote:
> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly

> stated in "vendor-prefixes.yaml".

> 

> Fixes: 1527058736fa ("reset: hisilicon: add reset-hi3660")

> Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")

> Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")

> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>

> Cc: Chen Feng <puck.chen@hisilicon.com>

> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---

>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 4 ++--

>  arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 2 +-

>  drivers/reset/hisilicon/reset-hi3660.c    | 2 +-


Please keep device tree patches and reset driver patch separate, as they
were in v1.

>  3 files changed, 4 insertions(+), 4 deletions(-)

> 

> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

> index 49c19c6879f95ce..bfb1375426d2b58 100644

> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

> @@ -345,7 +345,7 @@

>  		crg_rst: crg_rst_controller {

>  			compatible = "hisilicon,hi3660-reset";

>  			#reset-cells = <2>;

> -			hisi,rst-syscon = <&crg_ctrl>;

> +			hisilicon,rst-syscon = <&crg_ctrl>;

>  		};

>  

>  

> @@ -376,7 +376,7 @@

>  

>  		iomcu_rst: reset {

>  			compatible = "hisilicon,hi3660-reset";

> -			hisi,rst-syscon = <&iomcu>;

> +			hisilicon,rst-syscon = <&iomcu>;

>  			#reset-cells = <2>;

>  		};

>  

> diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

> index 85b0dfb35d6d396..5c5a5dc964ea848 100644

> --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

> +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

> @@ -155,7 +155,7 @@

>  			compatible = "hisilicon,hi3670-reset",

>  				     "hisilicon,hi3660-reset";

>  			#reset-cells = <2>;

> -			hisi,rst-syscon = <&crg_ctrl>;

> +			hisilicon,rst-syscon = <&crg_ctrl>;

>  		};

>  

>  		pctrl: pctrl@e8a09000 {

> diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c

> index a7d4445924e558c..8f1953159a65b31 100644

> --- a/drivers/reset/hisilicon/reset-hi3660.c

> +++ b/drivers/reset/hisilicon/reset-hi3660.c

> @@ -83,7 +83,7 @@ static int hi3660_reset_probe(struct platform_device *pdev)

>  	if (!rc)

>  		return -ENOMEM;

>  

> -	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");

> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");


This should fall back to the deprecated compatible, for example:

	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");
	if (PTR_ERR(rc->map) == -ENODEV)
		rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-
syscon");

>  	if (IS_ERR(rc->map)) {

>  		dev_err(dev, "failed to get hi3660,rst-syscon\n");

>  		return PTR_ERR(rc->map);


regards
Philipp
Zhen Lei Dec. 8, 2020, 11:29 a.m. UTC | #4 
On 2020/12/8 17:25, Philipp Zabel wrote:
> Hi Zhen,

> 

> On Fri, 2020-12-04 at 09:42 +0800, Zhen Lei wrote:

>> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly

>> stated in "vendor-prefixes.yaml".

>>

>> Fixes: 1527058736fa ("reset: hisilicon: add reset-hi3660")

>> Fixes: 35ca8168133c ("arm64: dts: Add dts files for Hisilicon Hi3660 SoC")

>> Fixes: dd8c7b78c11b ("arm64: dts: Add devicetree for Hisilicon Hi3670 SoC")

>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

>> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>

>> Cc: Chen Feng <puck.chen@hisilicon.com>

>> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

>> ---

>>  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 4 ++--

>>  arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 2 +-

>>  drivers/reset/hisilicon/reset-hi3660.c    | 2 +-

> 

> Please keep device tree patches and reset driver patch separate, as they

> were in v1.


OK

> 

>>  3 files changed, 4 insertions(+), 4 deletions(-)

>>

>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

>> index 49c19c6879f95ce..bfb1375426d2b58 100644

>> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

>> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi

>> @@ -345,7 +345,7 @@

>>  		crg_rst: crg_rst_controller {

>>  			compatible = "hisilicon,hi3660-reset";

>>  			#reset-cells = <2>;

>> -			hisi,rst-syscon = <&crg_ctrl>;

>> +			hisilicon,rst-syscon = <&crg_ctrl>;

>>  		};

>>  

>>  

>> @@ -376,7 +376,7 @@

>>  

>>  		iomcu_rst: reset {

>>  			compatible = "hisilicon,hi3660-reset";

>> -			hisi,rst-syscon = <&iomcu>;

>> +			hisilicon,rst-syscon = <&iomcu>;

>>  			#reset-cells = <2>;

>>  		};

>>  

>> diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

>> index 85b0dfb35d6d396..5c5a5dc964ea848 100644

>> --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

>> +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi

>> @@ -155,7 +155,7 @@

>>  			compatible = "hisilicon,hi3670-reset",

>>  				     "hisilicon,hi3660-reset";

>>  			#reset-cells = <2>;

>> -			hisi,rst-syscon = <&crg_ctrl>;

>> +			hisilicon,rst-syscon = <&crg_ctrl>;

>>  		};

>>  

>>  		pctrl: pctrl@e8a09000 {

>> diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c

>> index a7d4445924e558c..8f1953159a65b31 100644

>> --- a/drivers/reset/hisilicon/reset-hi3660.c

>> +++ b/drivers/reset/hisilicon/reset-hi3660.c

>> @@ -83,7 +83,7 @@ static int hi3660_reset_probe(struct platform_device *pdev)

>>  	if (!rc)

>>  		return -ENOMEM;

>>  

>> -	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");

>> +	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");

> 

> This should fall back to the deprecated compatible, for example:

> 

> 	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");

> 	if (PTR_ERR(rc->map) == -ENODEV)

> 		rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-

> syscon");


Oh, thanks. I misunderstood your suggestion the other day. I'll fix it right away.

> 

>>  	if (IS_ERR(rc->map)) {

>>  		dev_err(dev, "failed to get hi3660,rst-syscon\n");

>>  		return PTR_ERR(rc->map);

> 

> regards

> Philipp

> 

> .

>
Rob Herring (Arm) Dec. 8, 2020, 7:30 p.m. UTC | #5 
On Mon, Dec 7, 2020 at 8:16 PM Leizhen (ThunderTown)
<thunder.leizhen@huawei.com> wrote:
>

>

>

> On 2020/12/8 7:08, Rob Herring wrote:

> > On Fri, Dec 04, 2020 at 09:42:34AM +0800, Zhen Lei wrote:

> >> The vendor prefix of "Hisilicon Limited" is "hisilicon", it is clearly

> >> stated in "vendor-prefixes.yaml".

> >

> > Yes, but you can't fix this as changing it breaks compability between

> > DTBs and kernels.

> >

> > hisi has to be documented and marked 'deprecated'.

>

> I searched, and this is the only place that uses the hisi prefix. Currently,

> YAML check will report warnings due to mismatch any of the regexes defined

> in Documentation/devicetree/bindings/vendor-prefixes.yaml

>

> Is it not good to add hisi prefix to clear warnings?

> "^hisi,.*":

>   description: Hisilicon Limited, deprecated.

>

>

> By the way, Hi Rob:

> The license information in some YAML files is incorrect. Can you correct it in v5.11?

> For example:

> WARNING: DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause)

> #26: FILE: Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml:1:

> +# SPDX-License-Identifier: GPL-2.0


No, I can't change them. I'm not the copyright holder. The ones that
are not dual licensed are most likely converted from the old text
bindings which were default GPL2.

Rob
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 49c19c6879f95ce..bfb1375426d2b58 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -345,7 +345,7 @@ 
 		crg_rst: crg_rst_controller {
 			compatible = "hisilicon,hi3660-reset";
 			#reset-cells = <2>;
-			hisi,rst-syscon = <&crg_ctrl>;
+			hisilicon,rst-syscon = <&crg_ctrl>;
 		};
 
 
@@ -376,7 +376,7 @@ 
 
 		iomcu_rst: reset {
 			compatible = "hisilicon,hi3660-reset";
-			hisi,rst-syscon = <&iomcu>;
+			hisilicon,rst-syscon = <&iomcu>;
 			#reset-cells = <2>;
 		};
 
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index 85b0dfb35d6d396..5c5a5dc964ea848 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -155,7 +155,7 @@ 
 			compatible = "hisilicon,hi3670-reset",
 				     "hisilicon,hi3660-reset";
 			#reset-cells = <2>;
-			hisi,rst-syscon = <&crg_ctrl>;
+			hisilicon,rst-syscon = <&crg_ctrl>;
 		};
 
 		pctrl: pctrl@e8a09000 {
diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c
index a7d4445924e558c..8f1953159a65b31 100644
--- a/drivers/reset/hisilicon/reset-hi3660.c
+++ b/drivers/reset/hisilicon/reset-hi3660.c
@@ -83,7 +83,7 @@  static int hi3660_reset_probe(struct platform_device *pdev)
 	if (!rc)
 		return -ENOMEM;
 
-	rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
+	rc->map = syscon_regmap_lookup_by_phandle(np, "hisilicon,rst-syscon");
 	if (IS_ERR(rc->map)) {
 		dev_err(dev, "failed to get hi3660,rst-syscon\n");
 		return PTR_ERR(rc->map);