diff mbox series

ramips: set Netgear R6220 MAC NVMEM cell directly in the part node

Message ID 20230509132948.17808-1-zajec5@gmail.com
State Accepted
Delegated to: Rafał Miłecki
Headers show
Series ramips: set Netgear R6220 MAC NVMEM cell directly in the part node | expand

Commit Message

Rafał Miłecki May 9, 2023, 1:29 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

There is no need to use reference if original node it specified in
exactly the same file. This is a minor cleanup simplifying DTS code.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../linux/ramips/dts/mt7621_netgear_r6220.dts  | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Comments

Felix Baumann May 9, 2023, 4:21 p.m. UTC | #1
Am 9. Mai 2023 15:29:48 MESZ schrieb "Rafał Miłecki" <zajec5@gmail.com>:
>From: Rafał Miłecki <rafal@milecki.pl>
>
>There is no need to use reference if original node it specified in
>exactly the same file. This is a minor cleanup simplifying DTS code.
>
>Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>---
> .../linux/ramips/dts/mt7621_netgear_r6220.dts  | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
>diff --git a/target/linux/ramips/dts/mt7621_netgear_r6220.dts b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>index 0f476ef060..7bb49c15b4 100644
>--- a/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>+++ b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>@@ -42,9 +42,17 @@
> 		};
> 
> 		factory: partition@2e00000 {

The "factory" reference could be probably be dropped as well. Proposed changes look good to me.

>+			compatible = "nvmem-cells";
> 			label = "factory";
> 			reg = <0x2e00000 0x100000>;
> 			read-only;
>+
>+			#address-cells = <1>;
>+			#size-cells = <1>;
>+
>+			macaddr_factory_4: macaddr@4 {
>+				reg = <0x4 0x6>;
>+			};
> 		};
> 
> 		partition@4200000 {
>@@ -65,13 +73,3 @@
> 	nvmem-cell-names = "mac-address";
> 	mac-address-increment = <1>;
> };
>-
>-&factory {
>-	compatible = "nvmem-cells";
>-	#address-cells = <1>;
>-	#size-cells = <1>;
>-
>-	macaddr_factory_4: macaddr@4 {
>-		reg = <0x4 0x6>;
>-	};
>-};
>-- 
>2.35.3
>
>
>_______________________________________________
>openwrt-devel mailing list
>openwrt-devel@lists.openwrt.org
>https://lists.openwrt.org/mailman/listinfo/openwrt-devel




Viele Grüße
Felix von Freifunk
Rafał Miłecki July 14, 2023, 8:26 a.m. UTC | #2
On 9.05.2023 18:21, Felix Baumann wrote:
> Am 9. Mai 2023 15:29:48 MESZ schrieb "Rafał Miłecki" <zajec5@gmail.com>:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> There is no need to use reference if original node it specified in
>> exactly the same file. This is a minor cleanup simplifying DTS code.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> .../linux/ramips/dts/mt7621_netgear_r6220.dts  | 18 ++++++++----------
>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/linux/ramips/dts/mt7621_netgear_r6220.dts b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>> index 0f476ef060..7bb49c15b4 100644
>> --- a/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>> +++ b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>> @@ -42,9 +42,17 @@
>> 		};
>>
>> 		factory: partition@2e00000 {
> 
> The "factory" reference could be probably be dropped as well. Proposed changes look good to me.

Actually it can't be dropped. While it's a bit counter-intuitive, that
label is used in a *parent* DTS file mt7621_netgear_sercomm_ayx.dtsi .


>> +			compatible = "nvmem-cells";
>> 			label = "factory";
>> 			reg = <0x2e00000 0x100000>;
>> 			read-only;
>> +
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			macaddr_factory_4: macaddr@4 {
>> +				reg = <0x4 0x6>;
>> +			};
>> 		};
>>
>> 		partition@4200000 {
>> @@ -65,13 +73,3 @@
>> 	nvmem-cell-names = "mac-address";
>> 	mac-address-increment = <1>;
>> };
>> -
>> -&factory {
>> -	compatible = "nvmem-cells";
>> -	#address-cells = <1>;
>> -	#size-cells = <1>;
>> -
>> -	macaddr_factory_4: macaddr@4 {
>> -		reg = <0x4 0x6>;
>> -	};
>> -};
>> -- 
>> 2.35.3
Felix Baumann July 14, 2023, 9:57 a.m. UTC | #3
Am 14. Juli 2023 10:26:33 MESZ schrieb "Rafał Miłecki" <zajec5@gmail.com>:
>On 9.05.2023 18:21, Felix Baumann wrote:
>> Am 9. Mai 2023 15:29:48 MESZ schrieb "Rafał Miłecki" <zajec5@gmail.com>:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>> 
>>> There is no need to use reference if original node it specified in
>>> exactly the same file. This is a minor cleanup simplifying DTS code.
>>> 
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> .../linux/ramips/dts/mt7621_netgear_r6220.dts  | 18 ++++++++----------
>>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/target/linux/ramips/dts/mt7621_netgear_r6220.dts b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>>> index 0f476ef060..7bb49c15b4 100644
>>> --- a/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>>> +++ b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>>> @@ -42,9 +42,17 @@
>>> 		};
>>> 
>>> 		factory: partition@2e00000 {
>> 
>> The "factory" reference could be probably be dropped as well. Proposed changes look good to me.
>
>Actually it can't be dropped. While it's a bit counter-intuitive, that
>label is used in a *parent* DTS file mt7621_netgear_sercomm_ayx.dtsi .

Hello Rafał,

I was under the strong impression that dts(i) files could only get reference from the same file or parents but not childs. I remember reading a few exchanges by Ansuel, I think, that mentioned this.
I might be mixing two topics here, but was this (intended to be) fixed by dynamic partitions or something like that?

Are you certain the eeprom is loaded successfully?

Aren't dts(i) files read from top to bottom?


Regards
Felix Baumann
Rafał Miłecki July 14, 2023, 4:42 p.m. UTC | #4
On 2023-07-14 11:57, Felix Baumann wrote:
> Am 14. Juli 2023 10:26:33 MESZ schrieb "Rafał Miłecki" 
> <zajec5@gmail.com>:
>> On 9.05.2023 18:21, Felix Baumann wrote:
>>> Am 9. Mai 2023 15:29:48 MESZ schrieb "Rafał Miłecki" 
>>> <zajec5@gmail.com>:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>> 
>>>> There is no need to use reference if original node it specified in
>>>> exactly the same file. This is a minor cleanup simplifying DTS code.
>>>> 
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>> .../linux/ramips/dts/mt7621_netgear_r6220.dts  | 18 
>>>> ++++++++----------
>>>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>>> 
>>>> diff --git a/target/linux/ramips/dts/mt7621_netgear_r6220.dts 
>>>> b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>>>> index 0f476ef060..7bb49c15b4 100644
>>>> --- a/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>>>> +++ b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
>>>> @@ -42,9 +42,17 @@
>>>> 		};
>>>> 
>>>> 		factory: partition@2e00000 {
>>> 
>>> The "factory" reference could be probably be dropped as well. 
>>> Proposed changes look good to me.
>> 
>> Actually it can't be dropped. While it's a bit counter-intuitive, that
>> label is used in a *parent* DTS file mt7621_netgear_sercomm_ayx.dtsi .
> 
> Hello Rafał,
> 
> I was under the strong impression that dts(i) files could only get
> reference from the same file or parents but not childs. I remember
> reading a few exchanges by Ansuel, I think, that mentioned this.
> I might be mixing two topics here, but was this (intended to be) fixed
> by dynamic partitions or something like that?
> 
> Are you certain the eeprom is loaded successfully?
> 
> Aren't dts(i) files read from top to bottom?

I just verified that and it works.

First I decompiled .dtb from the latest "master" branch with:
dtc -I dtb -O dts -o r6220.dts 
build_dir/target-mipsel_24kc_musl/linux-ramips_mt7621/image-mt7621_netgear_r6220.dtb

Then I opened r6220.dts and analyzed it.

My MAC cell looks like this:

macaddr@4 {
	compatible = "mac-base";
	reg = <0x04 0x06>;
	#nvmem-cell-cells = <0x01>;
	phandle = <0x13>;
};


Then my Ethernet controller nodes look like that:

mac@0 {
	compatible = "mediatek,eth-mac";
	reg = <0x00>;
	nvmem-cells = <0x13 0x00>;
	nvmem-cell-names = "mac-address";
	(...)
};

mac@1 {
	compatible = "mediatek,eth-mac";
	reg = <0x01>;
	status = "okay";
	nvmem-cells = <0x13 0x01>;
	nvmem-cell-names = "mac-address";
	(...)
};

As you can see both reference phandle 0x13 so that looks all good.
diff mbox series

Patch

diff --git a/target/linux/ramips/dts/mt7621_netgear_r6220.dts b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
index 0f476ef060..7bb49c15b4 100644
--- a/target/linux/ramips/dts/mt7621_netgear_r6220.dts
+++ b/target/linux/ramips/dts/mt7621_netgear_r6220.dts
@@ -42,9 +42,17 @@ 
 		};
 
 		factory: partition@2e00000 {
+			compatible = "nvmem-cells";
 			label = "factory";
 			reg = <0x2e00000 0x100000>;
 			read-only;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			macaddr_factory_4: macaddr@4 {
+				reg = <0x4 0x6>;
+			};
 		};
 
 		partition@4200000 {
@@ -65,13 +73,3 @@ 
 	nvmem-cell-names = "mac-address";
 	mac-address-increment = <1>;
 };
-
-&factory {
-	compatible = "nvmem-cells";
-	#address-cells = <1>;
-	#size-cells = <1>;
-
-	macaddr_factory_4: macaddr@4 {
-		reg = <0x4 0x6>;
-	};
-};