| Message ID | 20210607081727.4723-1-cnsztl@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: rockchip: add EEPROM node for NanoPi R4S | expand |
Hi, On Mon, Jun 7, 2021 at 4:19 PM Tianling Shen <cnsztl@gmail.com> wrote: > > NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > stores the MAC address. > > Signed-off-by: Tianling Shen <cnsztl@gmail.com> > --- > arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > index cef4d18b599d..4a82f50a07c5 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > @@ -68,6 +68,15 @@ > status = "disabled"; > }; > > +&i2c2 { > + eeprom@51 { > + compatible = "microchip,24c02", "atmel,24c02"; The actual part number is Microchip 24AA025E48, but I don't think the way this device splits its storage area is easy to support without additional modifications to the driver. > + reg = <0x51>; > + pagesize = <16>; > + read-only; /* This holds our MAC */ Please also add the size, as well as a cell defining the offset and length to the MAC address. The gmac node can then reference this with: nvmem-cells = <&mac_address>; nvmem-cells-names = "mac-address"; which will actually pick up the MAC address. Regards ChenYu > + }; > +}; > + > &i2c4 { > status = "disabled"; > }; > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Tianling, On 6/7/21 10:17 AM, Tianling Shen wrote: > NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > stores the MAC address. > > Signed-off-by: Tianling Shen <cnsztl@gmail.com> > --- > arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > index cef4d18b599d..4a82f50a07c5 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > @@ -68,6 +68,15 @@ > status = "disabled"; > }; > > +&i2c2 { > + eeprom@51 { > + compatible = "microchip,24c02", "atmel,24c02"; > + reg = <0x51>; > + pagesize = <16>; > + read-only; /* This holds our MAC */ The mainline dts files should be generic I think. Any comment about "use", partitions or write ability should be avoided. It's up the user. Johan > + }; > +}; > + > &i2c4 { > status = "disabled"; > }; >
On Mon, Jun 7, 2021 at 5:31 PM Johan Jonker <jbx6244@gmail.com> wrote: > > Hi Tianling, > > On 6/7/21 10:17 AM, Tianling Shen wrote: > > NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > > stores the MAC address. > > > > Signed-off-by: Tianling Shen <cnsztl@gmail.com> > > --- > > arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > > index cef4d18b599d..4a82f50a07c5 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > > @@ -68,6 +68,15 @@ > > status = "disabled"; > > }; > > > > +&i2c2 { > > + eeprom@51 { > > + compatible = "microchip,24c02", "atmel,24c02"; > > + reg = <0x51>; > > + pagesize = <16>; > > > + read-only; /* This holds our MAC */ > > The mainline dts files should be generic I think. > Any comment about "use", partitions or write ability should be avoided. > It's up the user. Per the datasheet for this specific EEPROM, the latter half (128 bytes) is read-only in hardware by design though. ChenYu
Hi Chen-Yu, On 6/7/21 11:40 AM, Chen-Yu Tsai wrote: > On Mon, Jun 7, 2021 at 5:31 PM Johan Jonker <jbx6244@gmail.com> wrote: >> >> Hi Tianling, >> >> On 6/7/21 10:17 AM, Tianling Shen wrote: >>> NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which >>> stores the MAC address. >>> >>> Signed-off-by: Tianling Shen <cnsztl@gmail.com> >>> --- >>> arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts >>> index cef4d18b599d..4a82f50a07c5 100644 >>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts >>> @@ -68,6 +68,15 @@ >>> status = "disabled"; >>> }; >>> >>> +&i2c2 { >>> + eeprom@51 { >>> + compatible = "microchip,24c02", "atmel,24c02"; >>> + reg = <0x51>; >>> + pagesize = <16>; >> >>> + read-only; /* This holds our MAC */ >> >> The mainline dts files should be generic I think. >> Any comment about "use", partitions or write ability should be avoided. >> It's up the user. > > Per the datasheet for this specific EEPROM, the latter half (128 bytes) > is read-only in hardware by design though. The 24AA02XEXX is programmed at the factory with a globally unique node address stored in the upper half of the array and permanently write-protected. The remaining 1,024 bits are available for application use. Just a question... nvmem-cells = <&mac_address>; nvmem-cells-names = "mac-address"; Which part does this point to? Can we use the lower part to store/rewrite this too? === From at24.yaml: items: - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$" - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$" How does Microchip 24AA025E48 fit the regex? What compatible would you advise? === Johan > > ChenYu >
Hi, On Mon, Jun 7, 2021 at 6:26 PM Johan Jonker <jbx6244@gmail.com> wrote: > > Hi Chen-Yu, > > On 6/7/21 11:40 AM, Chen-Yu Tsai wrote: > > On Mon, Jun 7, 2021 at 5:31 PM Johan Jonker <jbx6244@gmail.com> wrote: > >> > >> Hi Tianling, > >> > >> On 6/7/21 10:17 AM, Tianling Shen wrote: > >>> NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > >>> stores the MAC address. > >>> > >>> Signed-off-by: Tianling Shen <cnsztl@gmail.com> > >>> --- > >>> arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > >>> 1 file changed, 9 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>> index cef4d18b599d..4a82f50a07c5 100644 > >>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>> @@ -68,6 +68,15 @@ > >>> status = "disabled"; > >>> }; > >>> > >>> +&i2c2 { > >>> + eeprom@51 { > >>> + compatible = "microchip,24c02", "atmel,24c02"; > >>> + reg = <0x51>; > >>> + pagesize = <16>; > >> > >>> + read-only; /* This holds our MAC */ > >> > >> The mainline dts files should be generic I think. > >> Any comment about "use", partitions or write ability should be avoided. > >> It's up the user. > > > > > Per the datasheet for this specific EEPROM, the latter half (128 bytes) > > is read-only in hardware by design though. > > The 24AA02XEXX is programmed at the factory with a > globally unique node address stored in the upper half > of the array and permanently write-protected. The > remaining 1,024 bits are available for application use. > > Just a question... > > nvmem-cells = <&mac_address>; > nvmem-cells-names = "mac-address"; > > Which part does this point to? In the eeprom node: mac_address: mac-address@fa { reg = <0xfa 0x6>; }; > Can we use the lower part to store/rewrite this too? If you don't set it to read only, then you could rewrite the lower portion. Writes to the upper portion would be ignored. > === > > From at24.yaml: > > items: > - pattern: > "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$" > - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$" > > How does Microchip 24AA025E48 fit the regex? > What compatible would you advise? New regex: "^microchip,24aa025?e(48|64)$" All that was what I had around in my tree. ChenYu
Hi Chen-Yu, On Mon, Jun 7, 2021 at 4:36 PM Chen-Yu Tsai <wens@kernel.org> wrote: > > Hi, > > On Mon, Jun 7, 2021 at 4:19 PM Tianling Shen <cnsztl@gmail.com> wrote: > > > > NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > > stores the MAC address. > > > > Signed-off-by: Tianling Shen <cnsztl@gmail.com> > > --- > > arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > > index cef4d18b599d..4a82f50a07c5 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > > @@ -68,6 +68,15 @@ > > status = "disabled"; > > }; > > > > +&i2c2 { > > + eeprom@51 { > > + compatible = "microchip,24c02", "atmel,24c02"; > > The actual part number is Microchip 24AA025E48, but I don't think > the way this device splits its storage area is easy to support > without additional modifications to the driver. > On the face of it, it seems to work fine, and I can read the data correctly. (built with at24 eeprom driver) Thanks, Tianling. > > + reg = <0x51>; > > + pagesize = <16>; > > + read-only; /* This holds our MAC */ > > Please also add the size, as well as a cell defining the offset > and length to the MAC address. The gmac node can then reference > this with: > > nvmem-cells = <&mac_address>; > nvmem-cells-names = "mac-address"; > > which will actually pick up the MAC address. > > > Regards > ChenYu > > > + }; > > +}; > > + > > &i2c4 { > > status = "disabled"; > > }; > > -- > > 2.17.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Johan, On Mon, Jun 7, 2021 at 6:26 PM Johan Jonker <jbx6244@gmail.com> wrote: > > Hi Chen-Yu, > > On 6/7/21 11:40 AM, Chen-Yu Tsai wrote: > > On Mon, Jun 7, 2021 at 5:31 PM Johan Jonker <jbx6244@gmail.com> wrote: > >> > >> Hi Tianling, > >> > >> On 6/7/21 10:17 AM, Tianling Shen wrote: > >>> NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > >>> stores the MAC address. > >>> > >>> Signed-off-by: Tianling Shen <cnsztl@gmail.com> > >>> --- > >>> arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > >>> 1 file changed, 9 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>> index cef4d18b599d..4a82f50a07c5 100644 > >>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>> @@ -68,6 +68,15 @@ > >>> status = "disabled"; > >>> }; > >>> > >>> +&i2c2 { > >>> + eeprom@51 { > >>> + compatible = "microchip,24c02", "atmel,24c02"; > >>> + reg = <0x51>; > >>> + pagesize = <16>; > >> > >>> + read-only; /* This holds our MAC */ > >> > >> The mainline dts files should be generic I think. > >> Any comment about "use", partitions or write ability should be avoided. > >> It's up the user. > > > > > Per the datasheet for this specific EEPROM, the latter half (128 bytes) > > is read-only in hardware by design though. > > The 24AA02XEXX is programmed at the factory with a > globally unique node address stored in the upper half > of the array and permanently write-protected. The > remaining 1,024 bits are available for application use. > In my opinion, as this contains data programmed by the factory, would it be okay to keep it read-only? > Just a question... > > nvmem-cells = <&mac_address>; > nvmem-cells-names = "mac-address"; > > Which part does this point to? > > Can we use the lower part to store/rewrite this too? > > === > > From at24.yaml: > > items: > - pattern: > "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$" > - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$" > > How does Microchip 24AA025E48 fit the regex? > What compatible would you advise? It seems that 24AA025E48 is a variant of 24MAC402 [1], and `atmel,24c02` will be okay in this case. 1. https://lkml.org/lkml/2018/1/24/494 Thanks, Tianling. > > === > > Johan > > > > > ChenYu > >
On 6/7/21 6:01 PM, Tianling Shen wrote: > Hi Johan, > > On Mon, Jun 7, 2021 at 6:26 PM Johan Jonker <jbx6244@gmail.com> wrote: >> >> Hi Chen-Yu, >> >> On 6/7/21 11:40 AM, Chen-Yu Tsai wrote: >>> On Mon, Jun 7, 2021 at 5:31 PM Johan Jonker <jbx6244@gmail.com> wrote: >>>> >>>> Hi Tianling, >>>> >>>> On 6/7/21 10:17 AM, Tianling Shen wrote: >>>>> NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which >>>>> stores the MAC address. >>>>> >>>>> Signed-off-by: Tianling Shen <cnsztl@gmail.com> >>>>> --- >>>>> arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ >>>>> 1 file changed, 9 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts >>>>> index cef4d18b599d..4a82f50a07c5 100644 >>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts >>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts >>>>> @@ -68,6 +68,15 @@ >>>>> status = "disabled"; >>>>> }; >>>>> >>>>> +&i2c2 { >>>>> + eeprom@51 { >>>>> + compatible = "microchip,24c02", "atmel,24c02"; >>>>> + reg = <0x51>; >>>>> + pagesize = <16>; >>>> >>>>> + read-only; /* This holds our MAC */ >>>> >>>> The mainline dts files should be generic I think. >>>> Any comment about "use", partitions or write ability should be avoided. >>>> It's up the user. >>> >> >>> Per the datasheet for this specific EEPROM, the latter half (128 bytes) >>> is read-only in hardware by design though. >> >> The 24AA02XEXX is programmed at the factory with a >> globally unique node address stored in the upper half >> of the array and permanently write-protected. The >> remaining 1,024 bits are available for application use. >> > > In my opinion, as this contains data programmed by the factory, would > it be okay to keep it read-only? This chip is not completely read-only. There might be users that like to try some other mac_address or store something else in that lower part. Is this then still possible? Generic DT describes hardware independent from what Linux drivers or other OS are capable off. This factory mac_addres is permanently write-protected, so no need to keep the rest read-only. nvmem-cells = <&new_mac_address_in_lower_part>; nvmem-cells-names = "mac-address"; > >> Just a question... >> >> nvmem-cells = <&mac_address>; >> nvmem-cells-names = "mac-address"; >> >> Which part does this point to? >> >> Can we use the lower part to store/rewrite this too? >> >> === >> >> From at24.yaml: >> >> items: >> - pattern: >> "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$" >> - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$" >> >> How does Microchip 24AA025E48 fit the regex? >> What compatible would you advise? > > It seems that 24AA025E48 is a variant of 24MAC402 [1], and > `atmel,24c02` will be okay in this case. > 1. https://lkml.org/lkml/2018/1/24/494 Ask Heiko. ;) As long as it does not generate more notifications then we already have. > > Thanks, > Tianling. > >> >> === >> >> Johan >> >>> >>> ChenYu >>>
On Tue, Jun 8, 2021 at 1:04 AM Johan Jonker <jbx6244@gmail.com> wrote: > > > > On 6/7/21 6:01 PM, Tianling Shen wrote: > > Hi Johan, > > > > On Mon, Jun 7, 2021 at 6:26 PM Johan Jonker <jbx6244@gmail.com> wrote: > >> > >> Hi Chen-Yu, > >> > >> On 6/7/21 11:40 AM, Chen-Yu Tsai wrote: > >>> On Mon, Jun 7, 2021 at 5:31 PM Johan Jonker <jbx6244@gmail.com> wrote: > >>>> > >>>> Hi Tianling, > >>>> > >>>> On 6/7/21 10:17 AM, Tianling Shen wrote: > >>>>> NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which > >>>>> stores the MAC address. > >>>>> > >>>>> Signed-off-by: Tianling Shen <cnsztl@gmail.com> > >>>>> --- > >>>>> arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ > >>>>> 1 file changed, 9 insertions(+) > >>>>> > >>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>>>> index cef4d18b599d..4a82f50a07c5 100644 > >>>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts > >>>>> @@ -68,6 +68,15 @@ > >>>>> status = "disabled"; > >>>>> }; > >>>>> > >>>>> +&i2c2 { > >>>>> + eeprom@51 { > >>>>> + compatible = "microchip,24c02", "atmel,24c02"; > >>>>> + reg = <0x51>; > >>>>> + pagesize = <16>; > >>>> > >>>>> + read-only; /* This holds our MAC */ > >>>> > >>>> The mainline dts files should be generic I think. > >>>> Any comment about "use", partitions or write ability should be avoided. > >>>> It's up the user. > >>> > >> > >>> Per the datasheet for this specific EEPROM, the latter half (128 bytes) > >>> is read-only in hardware by design though. > >> > >> The 24AA02XEXX is programmed at the factory with a > >> globally unique node address stored in the upper half > >> of the array and permanently write-protected. The > >> remaining 1,024 bits are available for application use. > >> > > > > > In my opinion, as this contains data programmed by the factory, would > > it be okay to keep it read-only? > > This chip is not completely read-only. > There might be users that like to try some other mac_address or store > something else in that lower part. Is this then still possible? > Generic DT describes hardware independent from what Linux drivers or > other OS are capable off. > This factory mac_addres is permanently write-protected, so no need to > keep the rest read-only. > > nvmem-cells = <&new_mac_address_in_lower_part>; > nvmem-cells-names = "mac-address"; > > > > >> Just a question... > >> > >> nvmem-cells = <&mac_address>; > >> nvmem-cells-names = "mac-address"; > >> > >> Which part does this point to? > >> > >> Can we use the lower part to store/rewrite this too? > >> > >> === > >> > >> From at24.yaml: > >> > >> items: > >> - pattern: > >> "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$" > >> - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$" > >> > >> How does Microchip 24AA025E48 fit the regex? > >> What compatible would you advise? > > > > It seems that 24AA025E48 is a variant of 24MAC402 [1], and > > `atmel,24c02` will be okay in this case. > > 1. https://lkml.org/lkml/2018/1/24/494 > > Ask Heiko. ;) > > As long as it does not generate more notifications then we already have. I think having a part specific compatible would be better. That way if someone wanted to implement read-only "feedback" to users for the second half they could. ChenYu > > > > Thanks, > > Tianling. > > > >> > >> === > >> > >> Johan > >> > >>> > >>> ChenYu > >>>
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts index cef4d18b599d..4a82f50a07c5 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts @@ -68,6 +68,15 @@ status = "disabled"; }; +&i2c2 { + eeprom@51 { + compatible = "microchip,24c02", "atmel,24c02"; + reg = <0x51>; + pagesize = <16>; + read-only; /* This holds our MAC */ + }; +}; + &i2c4 { status = "disabled"; };
NanoPi R4S has a EEPROM attached to the 2nd I2C bus (U92), which stores the MAC address. Signed-off-by: Tianling Shen <cnsztl@gmail.com> --- arch/arm64/boot/dts/rockchip/rk3399-nanopi-r4s.dts | 9 +++++++++ 1 file changed, 9 insertions(+)