diff mbox series

[1/6] arm64: xilinx: Do not use '_' in DT node names

Message ID 5137958580c85a35cf6aadd1c33a2f6bcf81a9e5.1695040866.git.michal.simek@amd.com
State New
Headers
Series arm64: xilinx: Tune DTSes to remove warnings from make W=1 dtbs |

Commit Message

Michal Simek Sept. 18, 2023, 12:41 p.m. UTC 
  Character '_' not recommended in node name. Use '-' instead.
Pretty much run seds below for node names.
s/zynqmp_ipi/zynqmp-ipi/
s/nvmem_firmware/nvmem-firmware/
s/soc_revision/soc-revision/
s/si5335_/si5335-/

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

 arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts | 4 ++--
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi            | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart Sept. 18, 2023, 2:56 p.m. UTC | #1 
Hi Michal,

Thank you for the patch.

On Mon, Sep 18, 2023 at 02:41:12PM +0200, Michal Simek wrote:
> Character '_' not recommended in node name. Use '-' instead.
> Pretty much run seds below for node names.
> s/zynqmp_ipi/zynqmp-ipi/
> s/nvmem_firmware/nvmem-firmware/
> s/soc_revision/soc-revision/
> s/si5335_/si5335-/
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>

The si5335 nodes may be better named after the clock name instead of the
component type, but that's nitpicking.

> ---
> 
>  arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts | 4 ++--
>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi            | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> index d0091d3cb764..52f998c22538 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> @@ -123,13 +123,13 @@ ina226 {
>  		io-channels = <&u35 0>, <&u35 1>, <&u35 2>, <&u35 3>;
>  	};
>  
> -	si5335_0: si5335_0 { /* clk0_usb - u23 */
> +	si5335_0: si5335-0 { /* clk0_usb - u23 */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <26000000>;
>  	};
>  
> -	si5335_1: si5335_1 { /* clk1_dp - u23 */
> +	si5335_1: si5335-1 { /* clk1_dp - u23 */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <27000000>;
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index b61fc99cd911..e50e95cbe817 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -129,7 +129,7 @@ rproc_1_fw_image: memory@3ef00000 {
>  		};
>  	};
>  
> -	zynqmp_ipi: zynqmp_ipi {
> +	zynqmp_ipi: zynqmp-ipi {
>  		bootph-all;
>  		compatible = "xlnx,zynqmp-ipi-mailbox";
>  		interrupt-parent = <&gic>;
> @@ -194,12 +194,12 @@ zynqmp_power: zynqmp-power {
>  				mbox-names = "tx", "rx";
>  			};
>  
> -			nvmem_firmware {
> +			nvmem-firmware {
>  				compatible = "xlnx,zynqmp-nvmem-fw";
>  				#address-cells = <1>;
>  				#size-cells = <1>;
>  
> -				soc_revision: soc_revision@0 {
> +				soc_revision: soc-revision@0 {

Unless I'm mistaken, this will change the userspace API, as it changes
the nvmem cell name. Is it an issue ?

>  					reg = <0x0 0x4>;
>  				};
>  			};
Michal Simek Sept. 19, 2023, 7:47 a.m. UTC | #2 
On 9/18/23 16:56, Laurent Pinchart wrote:
> Hi Michal,
> 
> Thank you for the patch.
> 
> On Mon, Sep 18, 2023 at 02:41:12PM +0200, Michal Simek wrote:
>> Character '_' not recommended in node name. Use '-' instead.
>> Pretty much run seds below for node names.
>> s/zynqmp_ipi/zynqmp-ipi/
>> s/nvmem_firmware/nvmem-firmware/
>> s/soc_revision/soc-revision/
>> s/si5335_/si5335-/
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
> 
> The si5335 nodes may be better named after the clock name instead of the
> component type, but that's nitpicking.

I don't know what's the guidance on this. fixed-clock.yaml is using generic 
"clock" name. I have no problem to do it if this is recommended way to go.


>> ---
>>
>>   arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts | 4 ++--
>>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi            | 6 +++---
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
>> index d0091d3cb764..52f998c22538 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
>> @@ -123,13 +123,13 @@ ina226 {
>>   		io-channels = <&u35 0>, <&u35 1>, <&u35 2>, <&u35 3>;
>>   	};
>>   
>> -	si5335_0: si5335_0 { /* clk0_usb - u23 */
>> +	si5335_0: si5335-0 { /* clk0_usb - u23 */
>>   		compatible = "fixed-clock";
>>   		#clock-cells = <0>;
>>   		clock-frequency = <26000000>;
>>   	};
>>   
>> -	si5335_1: si5335_1 { /* clk1_dp - u23 */
>> +	si5335_1: si5335-1 { /* clk1_dp - u23 */
>>   		compatible = "fixed-clock";
>>   		#clock-cells = <0>;
>>   		clock-frequency = <27000000>;
>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> index b61fc99cd911..e50e95cbe817 100644
>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
>> @@ -129,7 +129,7 @@ rproc_1_fw_image: memory@3ef00000 {
>>   		};
>>   	};
>>   
>> -	zynqmp_ipi: zynqmp_ipi {
>> +	zynqmp_ipi: zynqmp-ipi {
>>   		bootph-all;
>>   		compatible = "xlnx,zynqmp-ipi-mailbox";
>>   		interrupt-parent = <&gic>;
>> @@ -194,12 +194,12 @@ zynqmp_power: zynqmp-power {
>>   				mbox-names = "tx", "rx";
>>   			};
>>   
>> -			nvmem_firmware {
>> +			nvmem-firmware {
>>   				compatible = "xlnx,zynqmp-nvmem-fw";
>>   				#address-cells = <1>;
>>   				#size-cells = <1>;
>>   
>> -				soc_revision: soc_revision@0 {
>> +				soc_revision: soc-revision@0 {
> 
> Unless I'm mistaken, this will change the userspace API, as it changes
> the nvmem cell name. Is it an issue ?

Based on
https://docs.kernel.org/driver-api/nvmem.html#userspace-binary-interface

The only interface to user space is via nvmem file which has all of them 
together. And reference to this node is the same if used inside kernel itself.
That's why I think there is no change in connection to user space API from nvmem 
side. Of course entry is listed differently if you parse DT names.

Thanks,
Michal
Laurent Pinchart Sept. 19, 2023, 7:56 a.m. UTC | #3 
On Tue, Sep 19, 2023 at 09:47:52AM +0200, Michal Simek wrote:
> 
> 
> On 9/18/23 16:56, Laurent Pinchart wrote:
> > Hi Michal,
> > 
> > Thank you for the patch.
> > 
> > On Mon, Sep 18, 2023 at 02:41:12PM +0200, Michal Simek wrote:
> >> Character '_' not recommended in node name. Use '-' instead.
> >> Pretty much run seds below for node names.
> >> s/zynqmp_ipi/zynqmp-ipi/
> >> s/nvmem_firmware/nvmem-firmware/
> >> s/soc_revision/soc-revision/
> >> s/si5335_/si5335-/
> >>
> >> Signed-off-by: Michal Simek <michal.simek@amd.com>
> > 
> > The si5335 nodes may be better named after the clock name instead of the
> > component type, but that's nitpicking.
> 
> I don't know what's the guidance on this. fixed-clock.yaml is using generic 
> "clock" name. I have no problem to do it if this is recommended way to go.
> 
> 
> >> ---
> >>
> >>   arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts | 4 ++--
> >>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi            | 6 +++---
> >>   2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> >> index d0091d3cb764..52f998c22538 100644
> >> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> >> @@ -123,13 +123,13 @@ ina226 {
> >>   		io-channels = <&u35 0>, <&u35 1>, <&u35 2>, <&u35 3>;
> >>   	};
> >>   
> >> -	si5335_0: si5335_0 { /* clk0_usb - u23 */
> >> +	si5335_0: si5335-0 { /* clk0_usb - u23 */
> >>   		compatible = "fixed-clock";
> >>   		#clock-cells = <0>;
> >>   		clock-frequency = <26000000>;
> >>   	};
> >>   
> >> -	si5335_1: si5335_1 { /* clk1_dp - u23 */
> >> +	si5335_1: si5335-1 { /* clk1_dp - u23 */
> >>   		compatible = "fixed-clock";
> >>   		#clock-cells = <0>;
> >>   		clock-frequency = <27000000>;
> >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >> index b61fc99cd911..e50e95cbe817 100644
> >> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >> @@ -129,7 +129,7 @@ rproc_1_fw_image: memory@3ef00000 {
> >>   		};
> >>   	};
> >>   
> >> -	zynqmp_ipi: zynqmp_ipi {
> >> +	zynqmp_ipi: zynqmp-ipi {
> >>   		bootph-all;
> >>   		compatible = "xlnx,zynqmp-ipi-mailbox";
> >>   		interrupt-parent = <&gic>;
> >> @@ -194,12 +194,12 @@ zynqmp_power: zynqmp-power {
> >>   				mbox-names = "tx", "rx";
> >>   			};
> >>   
> >> -			nvmem_firmware {
> >> +			nvmem-firmware {
> >>   				compatible = "xlnx,zynqmp-nvmem-fw";
> >>   				#address-cells = <1>;
> >>   				#size-cells = <1>;
> >>   
> >> -				soc_revision: soc_revision@0 {
> >> +				soc_revision: soc-revision@0 {
> > 
> > Unless I'm mistaken, this will change the userspace API, as it changes
> > the nvmem cell name. Is it an issue ?
> 
> Based on
> https://docs.kernel.org/driver-api/nvmem.html#userspace-binary-interface
> 
> The only interface to user space is via nvmem file which has all of them 
> together. And reference to this node is the same if used inside kernel itself.
> That's why I think there is no change in connection to user space API from nvmem 
> side. Of course entry is listed differently if you parse DT names.

Ah my bad. It should be fine then, as long as nobody in the kernel calls
nvmem_cell_get("soc_revision"), which I didn't find any occurrence of.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
index d0091d3cb764..52f998c22538 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
@@ -123,13 +123,13 @@  ina226 {
 		io-channels = <&u35 0>, <&u35 1>, <&u35 2>, <&u35 3>;
 	};
 
-	si5335_0: si5335_0 { /* clk0_usb - u23 */
+	si5335_0: si5335-0 { /* clk0_usb - u23 */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <26000000>;
 	};
 
-	si5335_1: si5335_1 { /* clk1_dp - u23 */
+	si5335_1: si5335-1 { /* clk1_dp - u23 */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <27000000>;
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index b61fc99cd911..e50e95cbe817 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -129,7 +129,7 @@  rproc_1_fw_image: memory@3ef00000 {
 		};
 	};
 
-	zynqmp_ipi: zynqmp_ipi {
+	zynqmp_ipi: zynqmp-ipi {
 		bootph-all;
 		compatible = "xlnx,zynqmp-ipi-mailbox";
 		interrupt-parent = <&gic>;
@@ -194,12 +194,12 @@  zynqmp_power: zynqmp-power {
 				mbox-names = "tx", "rx";
 			};
 
-			nvmem_firmware {
+			nvmem-firmware {
 				compatible = "xlnx,zynqmp-nvmem-fw";
 				#address-cells = <1>;
 				#size-cells = <1>;
 
-				soc_revision: soc_revision@0 {
+				soc_revision: soc-revision@0 {
 					reg = <0x0 0x4>;
 				};
 			};