[PATCH v2 0/3] Read MAC address through NVMEM for sama7g5ek

[PATCH v2 0/3] Read MAC address through NVMEM for sama7g5ek

[Andrei Simion]

This series proposes to add EEPROM support and reading MAC addresses through
NVMEM (via Devicetree) for sama7g5ek:
- Add in DT bindings document the EEPROM compatibles : "at24,mac02e4" and
"at24,mac02e6"
- Update to the driver to support "at24,mac02e4" and
"at24,mac02e6" and adjusting offset for those 24AA025E{48, 64}.
- Added the nodes in devicetree for eeproms where are stored eui48 MAC,
and update gmac nodes to read the MAC via devicetree through NVMEM.

------------------------------------------------------------------
v1 -> v2:
* dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6
  - change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler

* eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
  - no change

* ARM: dts: at91: at91-sama7g5ek: add EEPROMs
  - remove unnecessary #address-cells #size-cells

------------------------------------------------------------------
Andrei Simion (1):
  dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

Claudiu Beznea (2):
  eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}
  ARM: dts: at91: at91-sama7g5ek: add EEPROMs

 .../devicetree/bindings/eeprom/at24.yaml      | 10 ++-
 .../arm/boot/dts/microchip/at91-sama7g5ek.dts | 40 ++++++++++
 drivers/misc/eeprom/at24.c                    | 73 +++++++++++--------
 3 files changed, 89 insertions(+), 34 deletions(-)


base-commit: b992b79ca8bc336fa8e2c80990b5af80ed8f36fd
-- 
2.34.1

[PATCH v2 1/3] eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}

[Andrei Simion]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

The EEPROMs could be used only for MAC storage. In this case the
EEPROM areas where MACs resides could be modeled as NVMEM cells
(directly via DT bindings) such that the already available networking
infrastructure to read properly the MAC addresses (via
of_get_mac_address()). The previously available compatibles needs the
offset adjustment probably for compatibility w/ old DT bindings.
Added "atmel,24mac02e4", "atmel,24mac02e6" compatible for the usage w/
24AA025E{48, 64} type of EEPROMs.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Co-developed-by: Andrei Simion <andrei.simion@microchip.com>
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
v1 -> v2:
- no change
---
 drivers/misc/eeprom/at24.c | 73 ++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 4bd4f32bcdab..8699a6c585c4 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -121,17 +121,19 @@ struct at24_chip_data {
 	u32 byte_len;
 	u8 flags;
 	u8 bank_addr_shift;
+	u8 adjoff;
 	void (*read_post)(unsigned int off, char *buf, size_t count);
 };
 
-#define AT24_CHIP_DATA(_name, _len, _flags)				\
+#define AT24_CHIP_DATA(_name, _len, _flags, _adjoff)			\
 	static const struct at24_chip_data _name = {			\
-		.byte_len = _len, .flags = _flags,			\
+		.byte_len = _len, .flags = _flags, .adjoff = _adjoff, \
 	}
 
-#define AT24_CHIP_DATA_CB(_name, _len, _flags, _read_post)		\
+#define AT24_CHIP_DATA_CB(_name, _len, _flags, _adjoff, _read_post)	\
 	static const struct at24_chip_data _name = {			\
 		.byte_len = _len, .flags = _flags,			\
+		.adjoff = _adjoff,					\
 		.read_post = _read_post,				\
 	}
 
@@ -162,53 +164,57 @@ static void at24_read_post_vaio(unsigned int off, char *buf, size_t count)
 }
 
 /* needs 8 addresses as A0-A2 are ignored */
-AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR);
+AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR, 0);
 /* old variants can't be handled with this generic entry! */
-AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0);
+AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0, 0);
 AT24_CHIP_DATA(at24_data_24cs01, 16,
-	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
-AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0);
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
+AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0, 0);
 AT24_CHIP_DATA(at24_data_24cs02, 16,
-	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
 AT24_CHIP_DATA(at24_data_24mac402, 48 / 8,
-	AT24_FLAG_MAC | AT24_FLAG_READONLY);
+	AT24_FLAG_MAC | AT24_FLAG_READONLY, 1);
 AT24_CHIP_DATA(at24_data_24mac602, 64 / 8,
-	AT24_FLAG_MAC | AT24_FLAG_READONLY);
+	AT24_FLAG_MAC | AT24_FLAG_READONLY, 1);
+AT24_CHIP_DATA(at24_data_24mac02e4, 48 / 8,
+	AT24_FLAG_MAC | AT24_FLAG_READONLY, 0);
+AT24_CHIP_DATA(at24_data_24mac02e6, 64 / 8,
+	AT24_FLAG_MAC | AT24_FLAG_READONLY, 0);
 /* spd is a 24c02 in memory DIMMs */
 AT24_CHIP_DATA(at24_data_spd, 2048 / 8,
-	AT24_FLAG_READONLY | AT24_FLAG_IRUGO);
+	AT24_FLAG_READONLY | AT24_FLAG_IRUGO, 0);
 /* 24c02_vaio is a 24c02 on some Sony laptops */
 AT24_CHIP_DATA_CB(at24_data_24c02_vaio, 2048 / 8,
-	AT24_FLAG_READONLY | AT24_FLAG_IRUGO,
+	AT24_FLAG_READONLY | AT24_FLAG_IRUGO, 0,
 	at24_read_post_vaio);
-AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0);
+AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0, 0);
 AT24_CHIP_DATA(at24_data_24cs04, 16,
-	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
 /* 24rf08 quirk is handled at i2c-core */
-AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0);
+AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0, 0);
 AT24_CHIP_DATA(at24_data_24cs08, 16,
-	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
-AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0);
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
+AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0, 0);
 AT24_CHIP_DATA(at24_data_24cs16, 16,
-	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
-AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16);
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
+AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16, 0);
 /* M24C32-D Additional Write lockable page (M24C32-D order codes) */
-AT24_CHIP_DATA(at24_data_24c32d_wlp, 32, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24c32d_wlp, 32, AT24_FLAG_ADDR16, 0);
 AT24_CHIP_DATA(at24_data_24cs32, 16,
-	AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
-AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16);
+	AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
+AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16, 0);
 /* M24C64-D Additional Write lockable page (M24C64-D order codes) */
-AT24_CHIP_DATA(at24_data_24c64d_wlp, 32, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24c64d_wlp, 32, AT24_FLAG_ADDR16, 0);
 AT24_CHIP_DATA(at24_data_24cs64, 16,
-	AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
-AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16);
-AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16);
-AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16);
-AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16);
+	AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
+AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16, 0);
+AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16, 0);
+AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16, 0);
+AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16, 0);
+AT24_CHIP_DATA(at24_data_24c2048, 2097152 / 8, AT24_FLAG_ADDR16, 0);
 AT24_CHIP_DATA_BS(at24_data_24c1025, 1048576 / 8, AT24_FLAG_ADDR16, 2);
-AT24_CHIP_DATA(at24_data_24c2048, 2097152 / 8, AT24_FLAG_ADDR16);
 /* identical to 24c08 ? */
-AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0);
+AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0, 0);
 
 static const struct i2c_device_id at24_ids[] = {
 	{ "24c00",	(kernel_ulong_t)&at24_data_24c00 },
@@ -217,7 +223,9 @@ static const struct i2c_device_id at24_ids[] = {
 	{ "24c02",	(kernel_ulong_t)&at24_data_24c02 },
 	{ "24cs02",	(kernel_ulong_t)&at24_data_24cs02 },
 	{ "24mac402",	(kernel_ulong_t)&at24_data_24mac402 },
+	{ "24mac02e4",	(kernel_ulong_t)&at24_data_24mac02e4 },
 	{ "24mac602",	(kernel_ulong_t)&at24_data_24mac602 },
+	{ "24mac02e6",	(kernel_ulong_t)&at24_data_24mac02e6 },
 	{ "spd",	(kernel_ulong_t)&at24_data_spd },
 	{ "24c02-vaio",	(kernel_ulong_t)&at24_data_24c02_vaio },
 	{ "24c04",	(kernel_ulong_t)&at24_data_24c04 },
@@ -250,7 +258,9 @@ static const struct of_device_id __maybe_unused at24_of_match[] = {
 	{ .compatible = "atmel,24c02",		.data = &at24_data_24c02 },
 	{ .compatible = "atmel,24cs02",		.data = &at24_data_24cs02 },
 	{ .compatible = "atmel,24mac402",	.data = &at24_data_24mac402 },
+	{ .compatible = "atmel,24mac02e4",	.data = &at24_data_24mac02e4 },
 	{ .compatible = "atmel,24mac602",	.data = &at24_data_24mac602 },
+	{ .compatible = "atmel,24mac02e6",	.data = &at24_data_24mac02e6 },
 	{ .compatible = "atmel,spd",		.data = &at24_data_spd },
 	{ .compatible = "atmel,24c04",		.data = &at24_data_24c04 },
 	{ .compatible = "atmel,24cs04",		.data = &at24_data_24cs04 },
@@ -690,7 +700,8 @@ static int at24_probe(struct i2c_client *client)
 	at24->read_post = cdata->read_post;
 	at24->bank_addr_shift = cdata->bank_addr_shift;
 	at24->num_addresses = num_addresses;
-	at24->offset_adj = at24_get_offset_adj(flags, byte_len);
+	at24->offset_adj = cdata->adjoff ?
+				at24_get_offset_adj(flags, byte_len) : 0;
 	at24->client_regmaps[0] = regmap;
 
 	at24->vcc_reg = devm_regulator_get(dev, "vcc");
-- 
2.34.1

[PATCH v2 2/3] ARM: dts: at91: at91-sama7g5ek: add EEPROMs

[Andrei Simion]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add EEPROMs and nvmem-layout to describe eui48 mac address region.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Co-developed-by: Andrei Simion <andrei.simion@microchip.com>
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
v1 -> v2:
- remove unnecessary #address-cells #size-cells
---
 .../arm/boot/dts/microchip/at91-sama7g5ek.dts | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts b/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts
index 20b2497657ae..266ca71f48c3 100644
--- a/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts
+++ b/arch/arm/boot/dts/microchip/at91-sama7g5ek.dts
@@ -403,6 +403,42 @@ i2c8: i2c@600 {
 		i2c-digital-filter;
 		i2c-digital-filter-width-ns = <35>;
 		status = "okay";
+
+		eeprom0: eeprom@52 {
+			compatible = "atmel,24mac02e4";
+			reg = <0x52>;
+			size = <256>;
+			pagesize = <16>;
+			vcc-supply = <&vdd_3v3>;
+
+			nvmem-layout {
+				compatible = "fixed-layout";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				eeprom0_eui48: eui48@fa {
+					reg = <0xfa 0x6>;
+				};
+			};
+		};
+
+		eeprom1: eeprom@53 {
+			compatible = "atmel,24mac02e4";
+			reg = <0x53>;
+			size = <256>;
+			pagesize = <16>;
+			vcc-supply = <&vdd_3v3>;
+
+			nvmem-layout {
+				compatible = "fixed-layout";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				eeprom1_eui48: eui48@fa {
+					reg = <0xfa 0x6>;
+				};
+			};
+		};
 	};
 };
 
@@ -440,6 +476,8 @@ &pinctrl_gmac0_mdio_default
 		     &pinctrl_gmac0_txck_default
 		     &pinctrl_gmac0_phy_irq>;
 	phy-mode = "rgmii-id";
+	nvmem-cells = <&eeprom0_eui48>;
+	nvmem-cell-names = "mac-address";
 	status = "okay";
 
 	ethernet-phy@7 {
@@ -457,6 +495,8 @@ &gmac1 {
 		     &pinctrl_gmac1_mdio_default
 		     &pinctrl_gmac1_phy_irq>;
 	phy-mode = "rmii";
+	nvmem-cells = <&eeprom1_eui48>;
+	nvmem-cell-names = "mac-address";
 	status = "okay"; /* Conflict with pdmc0. */
 
 	ethernet-phy@0 {
-- 
2.34.1

[PATCH v2 3/3] dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

[Andrei Simion]

Update regex check and add pattern to match both EEPROMs.

Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
v1 -> v2:
- change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
---
 Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index 3c36cd0510de..f914ca37ceea 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -18,7 +18,7 @@ select:
   properties:
     compatible:
       contains:
-        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
+        pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
   required:
     - compatible
 
@@ -37,8 +37,8 @@ properties:
       - allOf:
           - minItems: 1
             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)$"
+              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[a-z0-9]+|spd)$"
+              - pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
           - oneOf:
               - items:
                   pattern: c00$
@@ -54,6 +54,10 @@ properties:
                   pattern: mac402$
               - items:
                   pattern: mac602$
+              - items:
+                  pattern: mac02e4$
+              - items:
+                  pattern: mac02e6$
               - items:
                   pattern: c04$
               - items:
-- 
2.34.1

Re: [PATCH v2 3/3] dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]

On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote:
> Update regex check and add pattern to match both EEPROMs.
> 
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

[Rob Herring]

On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote:
> Update regex check and add pattern to match both EEPROMs.

The subject is wrong as 'at24' is not the vendor.

> 
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> ---
> v1 -> v2:
> - change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
> ---
>  Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> index 3c36cd0510de..f914ca37ceea 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -18,7 +18,7 @@ select:
>    properties:
>      compatible:
>        contains:
> -        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
> +        pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
>    required:
>      - compatible
>  
> @@ -37,8 +37,8 @@ properties:
>        - allOf:
>            - minItems: 1
>              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)$"
> +              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[a-z0-9]+|spd)$"
> +              - pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"

Are these devices available from multiple vendors? If not, I think I'd 
add specific compatible strings with the right vendor rather than adding 
to this pattern. It's rather loosely defined because that's what was in 
use already.

Rob

Re: [PATCH v2 3/3] dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

[Andrei.Simion]

On 24.06.2024 22:49, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote:
>> Update regex check and add pattern to match both EEPROMs.
> 
> The subject is wrong as 'at24' is not the vendor.
>

My mistake. It needs to be atmel,24mac02e4 and atmel,24mac02e6.

>>
>> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
>> ---
>> v1 -> v2:
>> - change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
>> ---
>>  Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
>> index 3c36cd0510de..f914ca37ceea 100644
>> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
>> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
>> @@ -18,7 +18,7 @@ select:
>>    properties:
>>      compatible:
>>        contains:
>> -        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
>> +        pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
>>    required:
>>      - compatible
>>
>> @@ -37,8 +37,8 @@ properties:
>>        - allOf:
>>            - minItems: 1
>>              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)$"
>> +              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[a-z0-9]+|spd)$"
>> +              - pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
> 
> Are these devices available from multiple vendors? If not, I think I'd
> add specific compatible strings with the right vendor rather than adding
> to this pattern. It's rather loosely defined because that's what was in
> use already.
>

So, would you like me to keep how it was before:  "^atmel,(24(c|cs|mac)[0-9]+|spd)$" and  "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$"
and to add only: 
- items:
 pattern: mac02e4$
- items:
 pattern: mac02e6$
?

Or would you like me to add to "the special cases that don't conform to the above pattern. Each requires a standard at24 model as fallback."  area?
 
> Rob

Best Regards, 
Andrei Simion

Re: [PATCH v2 3/3] dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2749 bytes --]

On Tue, Jun 25, 2024 at 07:33:18AM +0000, Andrei.Simion@microchip.com wrote:
> On 24.06.2024 22:49, Rob Herring wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote:
> >> Update regex check and add pattern to match both EEPROMs.
> > 
> > The subject is wrong as 'at24' is not the vendor.
> >
> 
> My mistake. It needs to be atmel,24mac02e4 and atmel,24mac02e6.
> 
> >>
> >> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> >> ---
> >> v1 -> v2:
> >> - change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
> >> ---
> >>  Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
> >>  1 file changed, 7 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> index 3c36cd0510de..f914ca37ceea 100644
> >> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> @@ -18,7 +18,7 @@ select:
> >>    properties:
> >>      compatible:
> >>        contains:
> >> -        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
> >> +        pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
> >>    required:
> >>      - compatible
> >>
> >> @@ -37,8 +37,8 @@ properties:
> >>        - allOf:
> >>            - minItems: 1
> >>              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)$"
> >> +              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[a-z0-9]+|spd)$"
> >> +              - pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
> > 
> > Are these devices available from multiple vendors? If not, I think I'd
> > add specific compatible strings with the right vendor rather than adding
> > to this pattern. It's rather loosely defined because that's what was in
> > use already.
> >
> 
> So, would you like me to keep how it was before:  "^atmel,(24(c|cs|mac)[0-9]+|spd)$" and  "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$"
> and to add only: 
> - items:
>  pattern: mac02e4$
> - items:
>  pattern: mac02e6$
> ?
> 
> Or would you like me to add to "the special cases that don't conform to the above pattern. Each requires a standard at24 model as fallback."  area?

I think the suggestion is to explicitly add these two devices down at the
bottom instead of adding to the regex. The first hunk I think in this
patch needs to remain a regex.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] eeprom: at24: avoid adjusting offset for 24AA025E{48, 64}

[Bartosz Golaszewski]

On Fri, Jun 21, 2024 at 2:16 PM Andrei Simion
<andrei.simion@microchip.com> wrote:
>
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>
> The EEPROMs could be used only for MAC storage. In this case the
> EEPROM areas where MACs resides could be modeled as NVMEM cells
> (directly via DT bindings) such that the already available networking
> infrastructure to read properly the MAC addresses (via
> of_get_mac_address()). The previously available compatibles needs the
> offset adjustment probably for compatibility w/ old DT bindings.
> Added "atmel,24mac02e4", "atmel,24mac02e6" compatible for the usage w/

Use imperative mode: "Add ...".

What does e4 and e6 stand for? It's not explained in the commit message.

> 24AA025E{48, 64} type of EEPROMs.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> Co-developed-by: Andrei Simion <andrei.simion@microchip.com>
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> ---
> v1 -> v2:
> - no change
> ---
>  drivers/misc/eeprom/at24.c | 73 ++++++++++++++++++++++----------------
>  1 file changed, 42 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 4bd4f32bcdab..8699a6c585c4 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -121,17 +121,19 @@ struct at24_chip_data {
>         u32 byte_len;
>         u8 flags;
>         u8 bank_addr_shift;
> +       u8 adjoff;
>         void (*read_post)(unsigned int off, char *buf, size_t count);
>  };
>
> -#define AT24_CHIP_DATA(_name, _len, _flags)                            \
> +#define AT24_CHIP_DATA(_name, _len, _flags, _adjoff)                   \
>         static const struct at24_chip_data _name = {                    \
> -               .byte_len = _len, .flags = _flags,                      \
> +               .byte_len = _len, .flags = _flags, .adjoff = _adjoff, \
>         }
>

This is a lot of churn for no reason, please keep this macro, add a
new one extended with adjoff and just pass 0 to it by default from the
existing one.

> -#define AT24_CHIP_DATA_CB(_name, _len, _flags, _read_post)             \
> +#define AT24_CHIP_DATA_CB(_name, _len, _flags, _adjoff, _read_post)    \
>         static const struct at24_chip_data _name = {                    \
>                 .byte_len = _len, .flags = _flags,                      \
> +               .adjoff = _adjoff,                                      \
>                 .read_post = _read_post,                                \
>         }
>
> @@ -162,53 +164,57 @@ static void at24_read_post_vaio(unsigned int off, char *buf, size_t count)
>  }
>
>  /* needs 8 addresses as A0-A2 are ignored */
> -AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR);
> +AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR, 0);
>  /* old variants can't be handled with this generic entry! */
> -AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0, 0);
>  AT24_CHIP_DATA(at24_data_24cs01, 16,
> -       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> -AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0);
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0, 0);
>  AT24_CHIP_DATA(at24_data_24cs02, 16,
> -       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
>  AT24_CHIP_DATA(at24_data_24mac402, 48 / 8,
> -       AT24_FLAG_MAC | AT24_FLAG_READONLY);
> +       AT24_FLAG_MAC | AT24_FLAG_READONLY, 1);
>  AT24_CHIP_DATA(at24_data_24mac602, 64 / 8,
> -       AT24_FLAG_MAC | AT24_FLAG_READONLY);
> +       AT24_FLAG_MAC | AT24_FLAG_READONLY, 1);
> +AT24_CHIP_DATA(at24_data_24mac02e4, 48 / 8,
> +       AT24_FLAG_MAC | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24mac02e6, 64 / 8,
> +       AT24_FLAG_MAC | AT24_FLAG_READONLY, 0);
>  /* spd is a 24c02 in memory DIMMs */
>  AT24_CHIP_DATA(at24_data_spd, 2048 / 8,
> -       AT24_FLAG_READONLY | AT24_FLAG_IRUGO);
> +       AT24_FLAG_READONLY | AT24_FLAG_IRUGO, 0);
>  /* 24c02_vaio is a 24c02 on some Sony laptops */
>  AT24_CHIP_DATA_CB(at24_data_24c02_vaio, 2048 / 8,
> -       AT24_FLAG_READONLY | AT24_FLAG_IRUGO,
> +       AT24_FLAG_READONLY | AT24_FLAG_IRUGO, 0,
>         at24_read_post_vaio);
> -AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0, 0);
>  AT24_CHIP_DATA(at24_data_24cs04, 16,
> -       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
>  /* 24rf08 quirk is handled at i2c-core */
> -AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0, 0);
>  AT24_CHIP_DATA(at24_data_24cs08, 16,
> -       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> -AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0);
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0, 0);
>  AT24_CHIP_DATA(at24_data_24cs16, 16,
> -       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> -AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16);
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16, 0);
>  /* M24C32-D Additional Write lockable page (M24C32-D order codes) */
> -AT24_CHIP_DATA(at24_data_24c32d_wlp, 32, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24c32d_wlp, 32, AT24_FLAG_ADDR16, 0);
>  AT24_CHIP_DATA(at24_data_24cs32, 16,
> -       AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> -AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16);
> +       AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16, 0);
>  /* M24C64-D Additional Write lockable page (M24C64-D order codes) */
> -AT24_CHIP_DATA(at24_data_24c64d_wlp, 32, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24c64d_wlp, 32, AT24_FLAG_ADDR16, 0);
>  AT24_CHIP_DATA(at24_data_24cs64, 16,
> -       AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> -AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16);
> -AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16);
> -AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16);
> -AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16);
> +       AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY, 0);
> +AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16, 0);
> +AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16, 0);
> +AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16, 0);
> +AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16, 0);
> +AT24_CHIP_DATA(at24_data_24c2048, 2097152 / 8, AT24_FLAG_ADDR16, 0);
>  AT24_CHIP_DATA_BS(at24_data_24c1025, 1048576 / 8, AT24_FLAG_ADDR16, 2);
> -AT24_CHIP_DATA(at24_data_24c2048, 2097152 / 8, AT24_FLAG_ADDR16);
>  /* identical to 24c08 ? */
> -AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0);
> +AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0, 0);
>
>  static const struct i2c_device_id at24_ids[] = {
>         { "24c00",      (kernel_ulong_t)&at24_data_24c00 },
> @@ -217,7 +223,9 @@ static const struct i2c_device_id at24_ids[] = {
>         { "24c02",      (kernel_ulong_t)&at24_data_24c02 },
>         { "24cs02",     (kernel_ulong_t)&at24_data_24cs02 },
>         { "24mac402",   (kernel_ulong_t)&at24_data_24mac402 },
> +       { "24mac02e4",  (kernel_ulong_t)&at24_data_24mac02e4 },
>         { "24mac602",   (kernel_ulong_t)&at24_data_24mac602 },
> +       { "24mac02e6",  (kernel_ulong_t)&at24_data_24mac02e6 },
>         { "spd",        (kernel_ulong_t)&at24_data_spd },
>         { "24c02-vaio", (kernel_ulong_t)&at24_data_24c02_vaio },
>         { "24c04",      (kernel_ulong_t)&at24_data_24c04 },
> @@ -250,7 +258,9 @@ static const struct of_device_id __maybe_unused at24_of_match[] = {
>         { .compatible = "atmel,24c02",          .data = &at24_data_24c02 },
>         { .compatible = "atmel,24cs02",         .data = &at24_data_24cs02 },
>         { .compatible = "atmel,24mac402",       .data = &at24_data_24mac402 },
> +       { .compatible = "atmel,24mac02e4",      .data = &at24_data_24mac02e4 },
>         { .compatible = "atmel,24mac602",       .data = &at24_data_24mac602 },
> +       { .compatible = "atmel,24mac02e6",      .data = &at24_data_24mac02e6 },
>         { .compatible = "atmel,spd",            .data = &at24_data_spd },
>         { .compatible = "atmel,24c04",          .data = &at24_data_24c04 },
>         { .compatible = "atmel,24cs04",         .data = &at24_data_24cs04 },
> @@ -690,7 +700,8 @@ static int at24_probe(struct i2c_client *client)
>         at24->read_post = cdata->read_post;
>         at24->bank_addr_shift = cdata->bank_addr_shift;
>         at24->num_addresses = num_addresses;
> -       at24->offset_adj = at24_get_offset_adj(flags, byte_len);
> +       at24->offset_adj = cdata->adjoff ?
> +                               at24_get_offset_adj(flags, byte_len) : 0;
>         at24->client_regmaps[0] = regmap;
>
>         at24->vcc_reg = devm_regulator_get(dev, "vcc");
> --
> 2.34.1
>

Bart