* [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region @ 2025-05-21 7:03 Manikandan Muralidharan 2025-05-21 7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Manikandan Muralidharan @ 2025-05-21 7:03 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, tudor.ambarus, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Cc: Manikandan Muralidharan This patch series adds support to parse the SFDP SST vendor map, read and store the EUI-48 and EUI-64 Address (if its programmed) using the resource-managed devm_kcalloc which will be freed on driver detach. Register EUI addresses into NVMEM framework for the net drivers to access them using nvmem properties. This change ensures consistent and reliable MAC address retrieval from QSPI benefiting boards like the sama5d27_wlsom1, sama5d29 curiosity and sam9x75 curiosity. -------- changes in v3: - 2/3 - add support to update the QSPI partition into 'fixed-partition' binding in sama5d27_wlsom1 - 3/3 - add nvmem-layout in qspi node for EUI48 MAC Address and nvmem cell properties for macb node in sama5d27_wlsom1 changes in v2: - 1/3 - parse the SST vendor table, read and store the addresses into a resource - managed space. Register the addresses into NVMEM framework - 2/3 - add support to update the QSPI partition into 'fixed-partition' binding -------- Manikandan Muralidharan (3): mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address .../dts/microchip/at91-sama5d27_wlsom1.dtsi | 65 ++++--- drivers/mtd/spi-nor/sfdp.c | 161 ++++++++++++++++++ include/linux/mtd/spi-nor.h | 7 + 3 files changed, 209 insertions(+), 24 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework 2025-05-21 7:03 [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Manikandan Muralidharan @ 2025-05-21 7:03 ` Manikandan Muralidharan 2025-06-07 11:01 ` Claudiu Beznea 2025-06-09 8:04 ` Tudor Ambarus 2025-05-21 7:03 ` [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding Manikandan Muralidharan ` (2 subsequent siblings) 3 siblings, 2 replies; 15+ messages in thread From: Manikandan Muralidharan @ 2025-05-21 7:03 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, tudor.ambarus, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Cc: Manikandan Muralidharan Some SST flash like SST26VF064BEUI serial quad flash memory is programmed at the factory with a globally unique EUI-48 and EUI-64 identifiers stored in the SFDP vendor parameter table and it is permanently write-protected. Add SST Vendor table SFDP parser to read the EUI-48 and EUI-64 Mac Addresses and allocate them using resource-managed devm_kcalloc which will be freed on driver detach. Regitser the Addresses into NVMEM framework and parse them when requested using the nvmem properties in the DT by the net drivers. In kernel the Ethernet MAC address relied on U-Boot env variables or generated a random address, which posed challenges for boards without on-board EEPROMs or with multiple Ethernet ports. This change ensures consistent and reliable MAC address retrieval from QSPI benefiting boards like the sama5d27-wlsom1-ek, sama5d29 curiosity and sam9x75 curiosity. Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> --- drivers/mtd/spi-nor/sfdp.c | 161 ++++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 7 ++ 2 files changed, 168 insertions(+) diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c index 21727f9a4ac6..920708ae928a 100644 --- a/drivers/mtd/spi-nor/sfdp.c +++ b/drivers/mtd/spi-nor/sfdp.c @@ -31,6 +31,7 @@ * Register Map Offsets for Multi-Chip * SPI Memory Devices. */ +#define SFDP_MCHP_SST_ID 0x01bf #define SFDP_SIGNATURE 0x50444653U @@ -1344,6 +1345,163 @@ static int spi_nor_parse_sccr_mc(struct spi_nor *nor, return ret; } +#define SFDP_MCHP_PARAM_TABLE_LEN 28 +#define SFDP_SST26VF064BEUI_ID 0xFF4326BFU + +#define SFDP_MCHP_EUI48 0x30 +#define SFDP_MCHP_EUI48_MASK GENMASK(7, 0) +#define SFDP_MCHP_EUI48_MAC_LEN 6 + +#define SFDP_MCHP_EUI64 0x40 +#define SFDP_MCHP_EUI64_MASK GENMASK(31, 24) +#define SFDP_MCHP_EUI64_MAC_LEN 8 + +/** + * spi_nor_mchp_sfdp_read_addr()- read callback to copy the EUI-48 or EUI-68 + * Addresses for device that request via NVMEM + * + * @priv: User context passed to read callbacks. + * @offset: Offset within the NVMEM device. + * @val: pointer where to fill the ethernet address + * @bytes: Length of the NVMEM cell + * + * Return: 0 on success, -EINVAL otherwise. + */ +static int spi_nor_mchp_sfdp_read_addr(void *priv, unsigned int off, + void *val, size_t bytes) +{ + struct spi_nor *nor = priv; + + if (SFDP_MCHP_PARAM_TABLE_LEN == nor->mchp_eui->vendor_param_length) { + switch (bytes) { + case SFDP_MCHP_EUI48_MAC_LEN: + memcpy(val, nor->mchp_eui->ethaddr_eui48, SFDP_MCHP_EUI48_MAC_LEN); + break; + case SFDP_MCHP_EUI64_MAC_LEN: + memcpy(val, nor->mchp_eui->ethaddr_eui64, SFDP_MCHP_EUI64_MAC_LEN); + break; + default: + return -EINVAL; + } + } + + return 0; +} + +/** + * spi_nor_parse_mchp_sfdp() - Parse the Microchip vendor specific parameter table + * Read and store the EUI-48 and EUI-64 address to + * struct spi_nor_sst_mchp_eui_info if the addresses are + * programmed in the SST26VF064BEUI sst flag + * + * @nor: pointer to a 'struct spi_nor' + * @sccr_header: pointer to the 'struct sfdp_parameter_header' describing + * the Microchip vendor parameter header length and version. + * + * Return: 0 on success of if addresses are not programmed, -errno otherwise. + */ +static int spi_nor_parse_mchp_sfdp(struct spi_nor *nor, + const struct sfdp_parameter_header *mchp_header) +{ + struct nvmem_device *nvmem; + struct nvmem_config nvmem_config = { }; + struct spi_nor_sst_mchp_eui_info *mchp_eui; + u32 *dwords, addr, sst_flash_id; + size_t len; + int ret = 0, size = 0; + + if (SFDP_MCHP_PARAM_TABLE_LEN != mchp_header->length) + return -EINVAL; + + addr = SFDP_PARAM_HEADER_PTP(mchp_header); + /* Get the SST SPI NOR FLASH ID */ + ret = spi_nor_read_sfdp_dma_unsafe(nor, addr, sizeof(sst_flash_id), + &sst_flash_id); + if (ret < 0) + return ret; + + /* Check the SPI NOR FLASH ID */ + if (le32_to_cpu(sst_flash_id) != SFDP_SST26VF064BEUI_ID) + return -EINVAL; + + len = mchp_header->length * sizeof(*dwords); + dwords = kmalloc(len, GFP_KERNEL); + if (!dwords) + return -ENOMEM; + + ret = spi_nor_read_sfdp(nor, addr, len, dwords); + if (ret) + goto out; + + le32_to_cpu_array(dwords, mchp_header->length); + + mchp_eui = devm_kzalloc(nor->dev, sizeof(*mchp_eui), GFP_KERNEL); + if (!mchp_eui) { + ret = -ENOMEM; + goto out; + } + + if (SFDP_MCHP_EUI48 == FIELD_GET(SFDP_MCHP_EUI48_MASK, + dwords[SFDP_DWORD(25)])) { + mchp_eui->ethaddr_eui48 = devm_kcalloc(nor->dev, + SFDP_MCHP_EUI48_MAC_LEN, + sizeof(u8), GFP_KERNEL); + if (!mchp_eui->ethaddr_eui48) { + ret = -ENOMEM; + devm_kfree(nor->dev, mchp_eui); + goto out; + } + memcpy(mchp_eui->ethaddr_eui48, (u8 *)&dwords[SFDP_DWORD(25)] + 1, + SFDP_MCHP_EUI48_MAC_LEN); + size = SFDP_MCHP_EUI48_MAC_LEN; + } + + if (SFDP_MCHP_EUI64 == FIELD_GET(SFDP_MCHP_EUI64_MASK, + dwords[SFDP_DWORD(26)])) { + mchp_eui->ethaddr_eui64 = devm_kcalloc(nor->dev, + SFDP_MCHP_EUI64_MAC_LEN, + sizeof(u8), GFP_KERNEL); + if (!mchp_eui->ethaddr_eui64) { + ret = -ENOMEM; + devm_kfree(nor->dev, mchp_eui->ethaddr_eui48); + devm_kfree(nor->dev, mchp_eui); + goto out; + } + memcpy(mchp_eui->ethaddr_eui64, (u8 *)&dwords[SFDP_DWORD(27)], + SFDP_MCHP_EUI64_MAC_LEN); + size += SFDP_MCHP_EUI64_MAC_LEN; + } + + /* + * Return if SST26VF064BEUI sst flash is not programmed + * with EUI-48 or EUI-64 information + */ + if (!size) { + devm_kfree(nor->dev, mchp_eui); + goto out; + } + + mchp_eui->vendor_param_length = mchp_header->length; + nor->mchp_eui = mchp_eui; + nvmem_config.word_size = 1; + nvmem_config.stride = 1; + nvmem_config.dev = nor->dev; + nvmem_config.size = size; + nvmem_config.priv = nor; + nvmem_config.reg_read = spi_nor_mchp_sfdp_read_addr; + + nvmem = devm_nvmem_register(nor->dev, &nvmem_config); + if (IS_ERR(nvmem)) { + dev_err(nor->dev, "failed to register NVMEM device: %ld\n", + PTR_ERR(nvmem)); + ret = PTR_ERR(nvmem); + } + +out: + kfree(dwords); + return ret; +} + /** * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings * after SFDP has been parsed. Called only for flashes that define JESD216 SFDP @@ -1564,6 +1722,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor) err = spi_nor_parse_sccr_mc(nor, param_header); break; + case SFDP_MCHP_SST_ID: + err = spi_nor_parse_mchp_sfdp(nor, param_header); + break; default: break; } diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index cdcfe0fd2e7d..051078d23ea1 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -339,6 +339,12 @@ struct flash_info; struct spi_nor_manufacturer; struct spi_nor_flash_parameter; +struct spi_nor_sst_mchp_eui_info { + u8 vendor_param_length; + u8 *ethaddr_eui48; + u8 *ethaddr_eui64; +}; + /** * struct spi_nor - Structure for defining the SPI NOR layer * @mtd: an mtd_info structure @@ -408,6 +414,7 @@ struct spi_nor { u32 flags; enum spi_nor_cmd_ext cmd_ext_type; struct sfdp *sfdp; + struct spi_nor_sst_mchp_eui_info *mchp_eui; struct dentry *debugfs_root; const struct spi_nor_controller_ops *controller_ops; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework 2025-05-21 7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan @ 2025-06-07 11:01 ` Claudiu Beznea 2025-06-09 8:04 ` Tudor Ambarus 1 sibling, 0 replies; 15+ messages in thread From: Claudiu Beznea @ 2025-06-07 11:01 UTC (permalink / raw) To: Manikandan Muralidharan, robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, tudor.ambarus, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Hi, Manikandan, On 21.05.2025 10:03, Manikandan Muralidharan wrote: > Some SST flash like SST26VF064BEUI serial quad flash memory is programmed > at the factory with a globally unique EUI-48 and EUI-64 identifiers stored > in the SFDP vendor parameter table and it is permanently write-protected. > > Add SST Vendor table SFDP parser to read the EUI-48 and EUI-64 > Mac Addresses and allocate them using resource-managed devm_kcalloc > which will be freed on driver detach. > > Regitser the Addresses into NVMEM framework and parse them when > requested using the nvmem properties in the DT by the net drivers. > In kernel the Ethernet MAC address relied on U-Boot env variables or > generated a random address, which posed challenges for boards without > on-board EEPROMs or with multiple Ethernet ports. > This change ensures consistent and reliable MAC address retrieval > from QSPI benefiting boards like the sama5d27-wlsom1-ek, sama5d29 curiosity > and sam9x75 curiosity. > > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> > --- > drivers/mtd/spi-nor/sfdp.c | 161 ++++++++++++++++++++++++++++++++++++ > include/linux/mtd/spi-nor.h | 7 ++ > 2 files changed, 168 insertions(+) > > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index 21727f9a4ac6..920708ae928a 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -31,6 +31,7 @@ > * Register Map Offsets for Multi-Chip > * SPI Memory Devices. > */ > +#define SFDP_MCHP_SST_ID 0x01bf > > #define SFDP_SIGNATURE 0x50444653U > > @@ -1344,6 +1345,163 @@ static int spi_nor_parse_sccr_mc(struct spi_nor *nor, > return ret; > } > > +#define SFDP_MCHP_PARAM_TABLE_LEN 28 > +#define SFDP_SST26VF064BEUI_ID 0xFF4326BFU > + > +#define SFDP_MCHP_EUI48 0x30 > +#define SFDP_MCHP_EUI48_MASK GENMASK(7, 0) > +#define SFDP_MCHP_EUI48_MAC_LEN 6 > + > +#define SFDP_MCHP_EUI64 0x40 > +#define SFDP_MCHP_EUI64_MASK GENMASK(31, 24) > +#define SFDP_MCHP_EUI64_MAC_LEN 8 > + > +/** > + * spi_nor_mchp_sfdp_read_addr()- read callback to copy the EUI-48 or EUI-68 > + * Addresses for device that request via NVMEM > + * > + * @priv: User context passed to read callbacks. > + * @offset: Offset within the NVMEM device. > + * @val: pointer where to fill the ethernet address > + * @bytes: Length of the NVMEM cell > + * > + * Return: 0 on success, -EINVAL otherwise. > + */ > +static int spi_nor_mchp_sfdp_read_addr(void *priv, unsigned int off, > + void *val, size_t bytes) > +{ > + struct spi_nor *nor = priv; > + > + if (SFDP_MCHP_PARAM_TABLE_LEN == nor->mchp_eui->vendor_param_length) { From checkpatch.pl: [Checkpatch] WARNING: Comparisons should place the constant on the right side of the test #71: FILE: drivers/mtd/spi-nor/sfdp.c:1375: + if (SFDP_MCHP_PARAM_TABLE_LEN == nor->mchp_eui->vendor_param_length) { Also, to avoid indenting the above block you can reverse the check here, and return, e.g.: if (nor->mchp_eui->vendor_param_length != SFDP_MCHP_PARAM_TABLE_LEN) return 0; > + switch (bytes) { > + case SFDP_MCHP_EUI48_MAC_LEN: > + memcpy(val, nor->mchp_eui->ethaddr_eui48, SFDP_MCHP_EUI48_MAC_LEN); > + break; > + case SFDP_MCHP_EUI64_MAC_LEN: > + memcpy(val, nor->mchp_eui->ethaddr_eui64, SFDP_MCHP_EUI64_MAC_LEN); > + break; > + default: > + return -EINVAL; > + } > + } > + > + return 0; > +} > + > +/** > + * spi_nor_parse_mchp_sfdp() - Parse the Microchip vendor specific parameter table > + * Read and store the EUI-48 and EUI-64 address to > + * struct spi_nor_sst_mchp_eui_info if the addresses are > + * programmed in the SST26VF064BEUI sst flag > + * > + * @nor: pointer to a 'struct spi_nor' > + * @sccr_header: pointer to the 'struct sfdp_parameter_header' describing > + * the Microchip vendor parameter header length and version. > + * > + * Return: 0 on success of if addresses are not programmed, -errno otherwise. > + */ > +static int spi_nor_parse_mchp_sfdp(struct spi_nor *nor, > + const struct sfdp_parameter_header *mchp_header) > +{ > + struct nvmem_device *nvmem; > + struct nvmem_config nvmem_config = { }; > + struct spi_nor_sst_mchp_eui_info *mchp_eui; > + u32 *dwords, addr, sst_flash_id; > + size_t len; > + int ret = 0, size = 0; > + > + if (SFDP_MCHP_PARAM_TABLE_LEN != mchp_header->length) From checkpatch.pl: WARNING: Comparisons should place the constant on the right side of the test #109: FILE: drivers/mtd/spi-nor/sfdp.c:1413: + if (SFDP_MCHP_PARAM_TABLE_LEN != mchp_header->length) > + return -EINVAL; > + > + addr = SFDP_PARAM_HEADER_PTP(mchp_header); > + /* Get the SST SPI NOR FLASH ID */ > + ret = spi_nor_read_sfdp_dma_unsafe(nor, addr, sizeof(sst_flash_id), > + &sst_flash_id); > + if (ret < 0) > + return ret; > + > + /* Check the SPI NOR FLASH ID */ > + if (le32_to_cpu(sst_flash_id) != SFDP_SST26VF064BEUI_ID) > + return -EINVAL; > + > + len = mchp_header->length * sizeof(*dwords); > + dwords = kmalloc(len, GFP_KERNEL); I think this can be replaced by: u32 *dwords __free(kfree) = kmalloc(...); > + if (!dwords) > + return -ENOMEM; > + > + ret = spi_nor_read_sfdp(nor, addr, len, dwords); > + if (ret) > + goto out; > + > + le32_to_cpu_array(dwords, mchp_header->length); > + > + mchp_eui = devm_kzalloc(nor->dev, sizeof(*mchp_eui), GFP_KERNEL); > + if (!mchp_eui) { > + ret = -ENOMEM; > + goto out; > + } > + > + if (SFDP_MCHP_EUI48 == FIELD_GET(SFDP_MCHP_EUI48_MASK, > + dwords[SFDP_DWORD(25)])) { > + mchp_eui->ethaddr_eui48 = devm_kcalloc(nor->dev, > + SFDP_MCHP_EUI48_MAC_LEN, > + sizeof(u8), GFP_KERNEL); > + if (!mchp_eui->ethaddr_eui48) { > + ret = -ENOMEM; > + devm_kfree(nor->dev, mchp_eui); > + goto out; > + } > + memcpy(mchp_eui->ethaddr_eui48, (u8 *)&dwords[SFDP_DWORD(25)] + 1, > + SFDP_MCHP_EUI48_MAC_LEN); > + size = SFDP_MCHP_EUI48_MAC_LEN; > + } > + > + if (SFDP_MCHP_EUI64 == FIELD_GET(SFDP_MCHP_EUI64_MASK, > + dwords[SFDP_DWORD(26)])) { > + mchp_eui->ethaddr_eui64 = devm_kcalloc(nor->dev, > + SFDP_MCHP_EUI64_MAC_LEN, > + sizeof(u8), GFP_KERNEL); > + if (!mchp_eui->ethaddr_eui64) { > + ret = -ENOMEM; > + devm_kfree(nor->dev, mchp_eui->ethaddr_eui48); > + devm_kfree(nor->dev, mchp_eui); > + goto out; > + } > + memcpy(mchp_eui->ethaddr_eui64, (u8 *)&dwords[SFDP_DWORD(27)], > + SFDP_MCHP_EUI64_MAC_LEN); > + size += SFDP_MCHP_EUI64_MAC_LEN; > + } > + > + /* > + * Return if SST26VF064BEUI sst flash is not programmed > + * with EUI-48 or EUI-64 information > + */ > + if (!size) { > + devm_kfree(nor->dev, mchp_eui); > + goto out; > + } > + > + mchp_eui->vendor_param_length = mchp_header->length; > + nor->mchp_eui = mchp_eui; > + nvmem_config.word_size = 1; > + nvmem_config.stride = 1; > + nvmem_config.dev = nor->dev; > + nvmem_config.size = size; > + nvmem_config.priv = nor; > + nvmem_config.reg_read = spi_nor_mchp_sfdp_read_addr; > + > + nvmem = devm_nvmem_register(nor->dev, &nvmem_config); > + if (IS_ERR(nvmem)) { > + dev_err(nor->dev, "failed to register NVMEM device: %ld\n", > + PTR_ERR(nvmem)); > + ret = PTR_ERR(nvmem); > + } > + > +out: > + kfree(dwords); > + return ret; > +} > + > /** > * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings > * after SFDP has been parsed. Called only for flashes that define JESD216 SFDP > @@ -1564,6 +1722,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor) > err = spi_nor_parse_sccr_mc(nor, param_header); > break; > > + case SFDP_MCHP_SST_ID: > + err = spi_nor_parse_mchp_sfdp(nor, param_header); > + break; > default: > break; > } > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h > index cdcfe0fd2e7d..051078d23ea1 100644 > --- a/include/linux/mtd/spi-nor.h > +++ b/include/linux/mtd/spi-nor.h > @@ -339,6 +339,12 @@ struct flash_info; > struct spi_nor_manufacturer; > struct spi_nor_flash_parameter; > > +struct spi_nor_sst_mchp_eui_info { > + u8 vendor_param_length; > + u8 *ethaddr_eui48; > + u8 *ethaddr_eui64; you may want to keep pointer first to avoid padding, if any. Thank you, Claudiu > +}; > + > /** > * struct spi_nor - Structure for defining the SPI NOR layer > * @mtd: an mtd_info structure > @@ -408,6 +414,7 @@ struct spi_nor { > u32 flags; > enum spi_nor_cmd_ext cmd_ext_type; > struct sfdp *sfdp; > + struct spi_nor_sst_mchp_eui_info *mchp_eui; > struct dentry *debugfs_root; > > const struct spi_nor_controller_ops *controller_ops; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework 2025-05-21 7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan 2025-06-07 11:01 ` Claudiu Beznea @ 2025-06-09 8:04 ` Tudor Ambarus 2025-06-11 6:49 ` Michael Walle 1 sibling, 1 reply; 15+ messages in thread From: Tudor Ambarus @ 2025-06-09 8:04 UTC (permalink / raw) To: Manikandan Muralidharan, robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Hi, On 5/21/25 8:03 AM, Manikandan Muralidharan wrote: > drivers/mtd/spi-nor/sfdp.c | 161 ++++++++++++++++++++++++++++++++++++ > include/linux/mtd/spi-nor.h | 7 ++ > 2 files changed, 168 insertions(+) Please find a way to move the vendor specific handling to a vendor file, don't pollute the core drivers. In what concerns the nvmem idea, I find it fine. Michael may comment more on it as I think he dealt with a similar use case in the past. Cheers, ta ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework 2025-06-09 8:04 ` Tudor Ambarus @ 2025-06-11 6:49 ` Michael Walle 0 siblings, 0 replies; 15+ messages in thread From: Michael Walle @ 2025-06-11 6:49 UTC (permalink / raw) To: Tudor Ambarus, Manikandan Muralidharan, robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, pratyush, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 797 bytes --] On Mon Jun 9, 2025 at 10:04 AM CEST, Tudor Ambarus wrote: > Hi, > > On 5/21/25 8:03 AM, Manikandan Muralidharan wrote: > > drivers/mtd/spi-nor/sfdp.c | 161 ++++++++++++++++++++++++++++++++++++ > > include/linux/mtd/spi-nor.h | 7 ++ > > 2 files changed, 168 insertions(+) > > Please find a way to move the vendor specific handling to a vendor file, > don't pollute the core drivers. IIRC I've suggested to move the handling into the core, but it should be more generic, not handling any MAC address related things, but more like a framework/helper functions to expose anything related to SFDP. -michael > In what concerns the nvmem idea, I find > it fine. Michael may comment more on it as I think he dealt with a > similar use case in the past. > > Cheers, > ta [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 297 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding 2025-05-21 7:03 [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Manikandan Muralidharan 2025-05-21 7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan @ 2025-05-21 7:03 ` Manikandan Muralidharan 2025-06-07 11:02 ` Claudiu Beznea 2025-05-21 7:03 ` [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address Manikandan Muralidharan 2025-05-21 12:56 ` [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Rob Herring (Arm) 3 siblings, 1 reply; 15+ messages in thread From: Manikandan Muralidharan @ 2025-05-21 7:03 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, tudor.ambarus, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Cc: Manikandan Muralidharan update the QSPI partitions using "fixed-partition" binding Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> --- .../dts/microchip/at91-sama5d27_wlsom1.dtsi | 54 ++++++++++--------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi index 9543214adc9f..b34c5072425a 100644 --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi @@ -229,8 +229,6 @@ &qspi1 { status = "disabled"; qspi1_flash: flash@0 { - #address-cells = <1>; - #size-cells = <1>; compatible = "jedec,spi-nor"; reg = <0>; spi-max-frequency = <104000000>; @@ -240,34 +238,40 @@ qspi1_flash: flash@0 { m25p,fast-read; status = "disabled"; - at91bootstrap@0 { - label = "at91bootstrap"; - reg = <0x0 0x40000>; - }; + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; - bootloader@40000 { - label = "bootloader"; - reg = <0x40000 0xc0000>; - }; + at91bootstrap@0 { + label = "at91bootstrap"; + reg = <0x0 0x40000>; + }; - bootloaderenvred@100000 { - label = "bootloader env redundant"; - reg = <0x100000 0x40000>; - }; + bootloader@40000 { + label = "bootloader"; + reg = <0x40000 0xc0000>; + }; - bootloaderenv@140000 { - label = "bootloader env"; - reg = <0x140000 0x40000>; - }; + bootloaderenvred@100000 { + label = "bootloader env redundant"; + reg = <0x100000 0x40000>; + }; - dtb@180000 { - label = "device tree"; - reg = <0x180000 0x80000>; - }; + bootloaderenv@140000 { + label = "bootloader env"; + reg = <0x140000 0x40000>; + }; - kernel@200000 { - label = "kernel"; - reg = <0x200000 0x600000>; + dtb@180000 { + label = "device tree"; + reg = <0x180000 0x80000>; + }; + + kernel@200000 { + label = "kernel"; + reg = <0x200000 0x600000>; + }; }; }; }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding 2025-05-21 7:03 ` [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding Manikandan Muralidharan @ 2025-06-07 11:02 ` Claudiu Beznea 0 siblings, 0 replies; 15+ messages in thread From: Claudiu Beznea @ 2025-06-07 11:02 UTC (permalink / raw) To: Manikandan Muralidharan, robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, tudor.ambarus, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Hi, Manikandan, On 21.05.2025 10:03, Manikandan Muralidharan wrote: > update the QSPI partitions using "fixed-partition" binding Please use capital letter at the beginning of the sentence and dot at the end of it. Can you please also explain here why you did this update? Thank you, Claudiu > > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> > --- > .../dts/microchip/at91-sama5d27_wlsom1.dtsi | 54 ++++++++++--------- > 1 file changed, 29 insertions(+), 25 deletions(-) > > diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > index 9543214adc9f..b34c5072425a 100644 > --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > @@ -229,8 +229,6 @@ &qspi1 { > status = "disabled"; > > qspi1_flash: flash@0 { > - #address-cells = <1>; > - #size-cells = <1>; > compatible = "jedec,spi-nor"; > reg = <0>; > spi-max-frequency = <104000000>; > @@ -240,34 +238,40 @@ qspi1_flash: flash@0 { > m25p,fast-read; > status = "disabled"; > > - at91bootstrap@0 { > - label = "at91bootstrap"; > - reg = <0x0 0x40000>; > - }; > + partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > > - bootloader@40000 { > - label = "bootloader"; > - reg = <0x40000 0xc0000>; > - }; > + at91bootstrap@0 { > + label = "at91bootstrap"; > + reg = <0x0 0x40000>; > + }; > > - bootloaderenvred@100000 { > - label = "bootloader env redundant"; > - reg = <0x100000 0x40000>; > - }; > + bootloader@40000 { > + label = "bootloader"; > + reg = <0x40000 0xc0000>; > + }; > > - bootloaderenv@140000 { > - label = "bootloader env"; > - reg = <0x140000 0x40000>; > - }; > + bootloaderenvred@100000 { > + label = "bootloader env redundant"; > + reg = <0x100000 0x40000>; > + }; > > - dtb@180000 { > - label = "device tree"; > - reg = <0x180000 0x80000>; > - }; > + bootloaderenv@140000 { > + label = "bootloader env"; > + reg = <0x140000 0x40000>; > + }; > > - kernel@200000 { > - label = "kernel"; > - reg = <0x200000 0x600000>; > + dtb@180000 { > + label = "device tree"; > + reg = <0x180000 0x80000>; > + }; > + > + kernel@200000 { > + label = "kernel"; > + reg = <0x200000 0x600000>; > + }; > }; > }; > }; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-05-21 7:03 [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Manikandan Muralidharan 2025-05-21 7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan 2025-05-21 7:03 ` [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding Manikandan Muralidharan @ 2025-05-21 7:03 ` Manikandan Muralidharan 2025-06-09 8:17 ` Tudor Ambarus 2025-05-21 12:56 ` [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Rob Herring (Arm) 3 siblings, 1 reply; 15+ messages in thread From: Manikandan Muralidharan @ 2025-05-21 7:03 UTC (permalink / raw) To: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, tudor.ambarus, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Cc: Manikandan Muralidharan Add nvmem-layout in QSPI to read the EUI48 Mac address by the net drivers using the nvmem property.The offset is set to 0x0 since the factory programmed address is available in the resource managed space and the size determine if the requested address is of EUI48 (0x6) or EUI-64 (0x8) type. This is useful for cases where U-Boot is skipped and the Ethernet MAC address is needed to be configured by the kernel Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> --- .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi index b34c5072425a..be06df1b7d66 100644 --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi @@ -210,6 +210,9 @@ &macb0 { #size-cells = <0>; phy-mode = "rmii"; + nvmem-cells = <&mac_address_eui48>; + nvmem-cell-names = "mac-address"; + ethernet-phy@0 { reg = <0x0>; interrupt-parent = <&pioA>; @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { m25p,fast-read; status = "disabled"; + nvmem-layout { + compatible = "fixed-layout"; + #address-cells = <1>; + #size-cells = <1>; + + mac_address_eui48: mac-address@0 { + reg = <0x0 0x6>; + }; + }; + partitions { compatible = "fixed-partitions"; #address-cells = <1>; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-05-21 7:03 ` [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address Manikandan Muralidharan @ 2025-06-09 8:17 ` Tudor Ambarus 2025-06-09 9:27 ` Manikandan.M 0 siblings, 1 reply; 15+ messages in thread From: Tudor Ambarus @ 2025-06-09 8:17 UTC (permalink / raw) To: Manikandan Muralidharan, robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni, claudiu.beznea, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd On 5/21/25 8:03 AM, Manikandan Muralidharan wrote: > Add nvmem-layout in QSPI to read the EUI48 Mac address by the > net drivers using the nvmem property.The offset is set to 0x0 > since the factory programmed address is available in the > resource managed space and the size determine if the requested > address is of EUI48 (0x6) or EUI-64 (0x8) type. > This is useful for cases where U-Boot is skipped and the Ethernet > MAC address is needed to be configured by the kernel > > Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> > --- > .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > index b34c5072425a..be06df1b7d66 100644 > --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > @@ -210,6 +210,9 @@ &macb0 { > #size-cells = <0>; > phy-mode = "rmii"; > > + nvmem-cells = <&mac_address_eui48>; > + nvmem-cell-names = "mac-address"; > + > ethernet-phy@0 { > reg = <0x0>; > interrupt-parent = <&pioA>; > @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { > m25p,fast-read; > status = "disabled"; > > + nvmem-layout { > + compatible = "fixed-layout"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + mac_address_eui48: mac-address@0 { > + reg = <0x0 0x6>; > + }; How would this work if in the future the mchp vendor table adds some other info that needs to be referenced as nvmem? How do you distinguish the info from the table? Would it be possible to have some kind of address and size to reference the SFDP? > + }; > + > partitions { > compatible = "fixed-partitions"; > #address-cells = <1>; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-06-09 8:17 ` Tudor Ambarus @ 2025-06-09 9:27 ` Manikandan.M 2025-06-11 7:31 ` Michael Walle 0 siblings, 1 reply; 15+ messages in thread From: Manikandan.M @ 2025-06-09 9:27 UTC (permalink / raw) To: tudor.ambarus, robh, krzk+dt, conor+dt, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, pratyush, mwalle, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Hi Tudor, On 09/06/25 1:47 pm, Tudor Ambarus wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 5/21/25 8:03 AM, Manikandan Muralidharan wrote: >> Add nvmem-layout in QSPI to read the EUI48 Mac address by the >> net drivers using the nvmem property.The offset is set to 0x0 >> since the factory programmed address is available in the >> resource managed space and the size determine if the requested >> address is of EUI48 (0x6) or EUI-64 (0x8) type. >> This is useful for cases where U-Boot is skipped and the Ethernet >> MAC address is needed to be configured by the kernel >> >> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> >> --- >> .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >> index b34c5072425a..be06df1b7d66 100644 >> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >> @@ -210,6 +210,9 @@ &macb0 { >> #size-cells = <0>; >> phy-mode = "rmii"; >> >> + nvmem-cells = <&mac_address_eui48>; >> + nvmem-cell-names = "mac-address"; >> + >> ethernet-phy@0 { >> reg = <0x0>; >> interrupt-parent = <&pioA>; >> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { >> m25p,fast-read; >> status = "disabled"; >> >> + nvmem-layout { >> + compatible = "fixed-layout"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + mac_address_eui48: mac-address@0 { >> + reg = <0x0 0x6>; >> + }; > > How would this work if in the future the mchp vendor table adds some > other info that needs to be referenced as nvmem? How do you distinguish > the info from the table? > Would it be possible to have some kind of address and size to reference > the SFDP? I was previously advised not to hardcode the offset in the Device Tree [1]. In the current implementation (patch 1/3), the read callback for the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC addresses based on the nvmem size, which corresponds to the size of the respective MAC address. [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/ > >> + }; >> + >> partitions { >> compatible = "fixed-partitions"; >> #address-cells = <1>; > -- Thanks and Regards, Manikandan M. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-06-09 9:27 ` Manikandan.M @ 2025-06-11 7:31 ` Michael Walle 2025-06-12 3:33 ` Manikandan.M 0 siblings, 1 reply; 15+ messages in thread From: Michael Walle @ 2025-06-11 7:31 UTC (permalink / raw) To: Manikandan.M, tudor.ambarus, robh, krzk+dt, conor+dt, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, pratyush, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 3690 bytes --] Hi, On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote: > >> Add nvmem-layout in QSPI to read the EUI48 Mac address by the > >> net drivers using the nvmem property.The offset is set to 0x0 > >> since the factory programmed address is available in the > >> resource managed space and the size determine if the requested > >> address is of EUI48 (0x6) or EUI-64 (0x8) type. > >> This is useful for cases where U-Boot is skipped and the Ethernet > >> MAC address is needed to be configured by the kernel > >> > >> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> > >> --- > >> .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ > >> 1 file changed, 13 insertions(+) > >> > >> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > >> index b34c5072425a..be06df1b7d66 100644 > >> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > >> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > >> @@ -210,6 +210,9 @@ &macb0 { > >> #size-cells = <0>; > >> phy-mode = "rmii"; > >> > >> + nvmem-cells = <&mac_address_eui48>; > >> + nvmem-cell-names = "mac-address"; > >> + > >> ethernet-phy@0 { > >> reg = <0x0>; > >> interrupt-parent = <&pioA>; > >> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { > >> m25p,fast-read; > >> status = "disabled"; > >> > >> + nvmem-layout { IMHO this should be "sfdp {". > >> + compatible = "fixed-layout"; Please read my feedback on the first version again.. For the DT maintainers. The SFDP is a small table based content that provide basic information about the flash. There are standard tables which are specified by the JEDEC standard and there are vendor tables, most of the time without proper documentation (or none at all). Somehow we need to specify at what table we want to look at. I'd like to see a binding which can potentially expose anything inside the SFDP. So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN" where NNNN is the table parameter id. Additionally, the standard ids could have names like "jedec,sfdp-bfpt" (basic flash parameter table). So in your case that would be: flash { sfdp { mac_address: table-1 { compatible = "jedec,sfdp-idNNNN"; }; }; }; I don't know what NNNN is. Could you please provide a dump of the sfdp of your flash. -michael > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + > >> + mac_address_eui48: mac-address@0 { > >> + reg = <0x0 0x6>; > >> + }; > > > > How would this work if in the future the mchp vendor table adds some > > other info that needs to be referenced as nvmem? How do you distinguish > > the info from the table? > > Would it be possible to have some kind of address and size to reference > > the SFDP? > > I was previously advised not to hardcode the offset in the Device Tree > [1]. In the current implementation (patch 1/3), the read callback for > the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC > addresses based on the nvmem size, which corresponds to the size of the > respective MAC address. > > [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/ > > > > >> + }; > >> + > >> partitions { > >> compatible = "fixed-partitions"; > >> #address-cells = <1>; > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 297 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-06-11 7:31 ` Michael Walle @ 2025-06-12 3:33 ` Manikandan.M 2025-06-12 6:17 ` Michael Walle 0 siblings, 1 reply; 15+ messages in thread From: Manikandan.M @ 2025-06-12 3:33 UTC (permalink / raw) To: mwalle, tudor.ambarus, robh, krzk+dt, conor+dt, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, pratyush, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Hi Michael, On 11/06/25 1:01 pm, Michael Walle wrote: > Hi, > > On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote: >>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the >>>> net drivers using the nvmem property.The offset is set to 0x0 >>>> since the factory programmed address is available in the >>>> resource managed space and the size determine if the requested >>>> address is of EUI48 (0x6) or EUI-64 (0x8) type. >>>> This is useful for cases where U-Boot is skipped and the Ethernet >>>> MAC address is needed to be configured by the kernel >>>> >>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> >>>> --- >>>> .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >>>> index b34c5072425a..be06df1b7d66 100644 >>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >>>> @@ -210,6 +210,9 @@ &macb0 { >>>> #size-cells = <0>; >>>> phy-mode = "rmii"; >>>> >>>> + nvmem-cells = <&mac_address_eui48>; >>>> + nvmem-cell-names = "mac-address"; >>>> + >>>> ethernet-phy@0 { >>>> reg = <0x0>; >>>> interrupt-parent = <&pioA>; >>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { >>>> m25p,fast-read; >>>> status = "disabled"; >>>> >>>> + nvmem-layout { > > IMHO this should be "sfdp {". > >>>> + compatible = "fixed-layout"; > > Please read my feedback on the first version again.. > > For the DT maintainers. The SFDP is a small table based content that > provide basic information about the flash. There are standard tables > which are specified by the JEDEC standard and there are vendor > tables, most of the time without proper documentation (or none at > all). > > Somehow we need to specify at what table we want to look at. I'd > like to see a binding which can potentially expose anything inside > the SFDP. > > So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN" > where NNNN is the table parameter id. Additionally, the standard ids > could have names like "jedec,sfdp-bfpt" (basic flash parameter table). > > So in your case that would be: > > flash { > sfdp { > mac_address: table-1 { > compatible = "jedec,sfdp-idNNNN"; > }; > }; Should the nvmem-layout be included as a child node under sfdp {}, or should it be implemented as a separate vendor-specific driver to handle the changes introduced in patch 1/3 ? > }; > > I don't know what NNNN is. Could you please provide a dump of the > sfdp of your flash. Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1 of 11.0 APPENDIX https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf The vendor parameter ID is 'BF' if I am not wrong. > > -michael > >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + >>>> + mac_address_eui48: mac-address@0 { >>>> + reg = <0x0 0x6>; >>>> + }; >>> >>> How would this work if in the future the mchp vendor table adds some >>> other info that needs to be referenced as nvmem? How do you distinguish >>> the info from the table? >>> Would it be possible to have some kind of address and size to reference >>> the SFDP? >> >> I was previously advised not to hardcode the offset in the Device Tree >> [1]. In the current implementation (patch 1/3), the read callback for >> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC >> addresses based on the nvmem size, which corresponds to the size of the >> respective MAC address. >> >> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/ >> >>> >>>> + }; >>>> + >>>> partitions { >>>> compatible = "fixed-partitions"; >>>> #address-cells = <1>; >>> > -- Thanks and Regards, Manikandan M. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-06-12 3:33 ` Manikandan.M @ 2025-06-12 6:17 ` Michael Walle 2025-06-16 6:43 ` Manikandan.M 0 siblings, 1 reply; 15+ messages in thread From: Michael Walle @ 2025-06-12 6:17 UTC (permalink / raw) To: Manikandan.M, tudor.ambarus, robh, krzk+dt, conor+dt, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, pratyush, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 5043 bytes --] Hi, > >>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the > >>>> net drivers using the nvmem property.The offset is set to 0x0 > >>>> since the factory programmed address is available in the > >>>> resource managed space and the size determine if the requested > >>>> address is of EUI48 (0x6) or EUI-64 (0x8) type. > >>>> This is useful for cases where U-Boot is skipped and the Ethernet > >>>> MAC address is needed to be configured by the kernel > >>>> > >>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> > >>>> --- > >>>> .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ > >>>> 1 file changed, 13 insertions(+) > >>>> > >>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > >>>> index b34c5072425a..be06df1b7d66 100644 > >>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > >>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi > >>>> @@ -210,6 +210,9 @@ &macb0 { > >>>> #size-cells = <0>; > >>>> phy-mode = "rmii"; > >>>> > >>>> + nvmem-cells = <&mac_address_eui48>; > >>>> + nvmem-cell-names = "mac-address"; > >>>> + > >>>> ethernet-phy@0 { > >>>> reg = <0x0>; > >>>> interrupt-parent = <&pioA>; > >>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { > >>>> m25p,fast-read; > >>>> status = "disabled"; > >>>> > >>>> + nvmem-layout { > > > > IMHO this should be "sfdp {". > > > >>>> + compatible = "fixed-layout"; > > > > Please read my feedback on the first version again.. > > > > For the DT maintainers. The SFDP is a small table based content that > > provide basic information about the flash. There are standard tables > > which are specified by the JEDEC standard and there are vendor > > tables, most of the time without proper documentation (or none at > > all). > > > > Somehow we need to specify at what table we want to look at. I'd > > like to see a binding which can potentially expose anything inside > > the SFDP. > > > > So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN" > > where NNNN is the table parameter id. Additionally, the standard ids > > could have names like "jedec,sfdp-bfpt" (basic flash parameter table). > > > > So in your case that would be: > > > > flash { > > sfdp { > > mac_address: table-1 { > > compatible = "jedec,sfdp-idNNNN"; > > }; > > }; > > Should the nvmem-layout be included as a child node under sfdp {}, or > should it be implemented as a separate vendor-specific driver to handle > the changes introduced in patch 1/3 ? There is no nvmem-layout involved here. But another possibility is to make it one. Then you have to (1) expose the *entire* sfpd as a nvmem device (2a) write an nvmem-layouts driver (in drivers/nvmem/layouts/) (2b) come up with a DT binding that is generic enough to expose various parameters of the SFDP, not just a one-off for the MAC address use case. Maybe that is even a better fit. > > }; > > > > I don't know what NNNN is. Could you please provide a dump of the > > sfdp of your flash. > > Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1 > of 11.0 APPENDIX Please dump it according to [1], so we have it in a machine readable format. -michael [1] https://docs.kernel.org/driver-api/mtd/spi-nor.html > > On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote: > > https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf > > > The vendor parameter ID is 'BF' if I am not wrong. > > > > -michael > > > >>>> + #address-cells = <1>; > >>>> + #size-cells = <1>; > >>>> + > >>>> + mac_address_eui48: mac-address@0 { > >>>> + reg = <0x0 0x6>; > >>>> + }; > >>> > >>> How would this work if in the future the mchp vendor table adds some > >>> other info that needs to be referenced as nvmem? How do you distinguish > >>> the info from the table? > >>> Would it be possible to have some kind of address and size to reference > >>> the SFDP? > >> > >> I was previously advised not to hardcode the offset in the Device Tree > >> [1]. In the current implementation (patch 1/3), the read callback for > >> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC > >> addresses based on the nvmem size, which corresponds to the size of the > >> respective MAC address. > >> > >> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/ > >> > >>> > >>>> + }; > >>>> + > >>>> partitions { > >>>> compatible = "fixed-partitions"; > >>>> #address-cells = <1>; > >>> > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 297 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address 2025-06-12 6:17 ` Michael Walle @ 2025-06-16 6:43 ` Manikandan.M 0 siblings, 0 replies; 15+ messages in thread From: Manikandan.M @ 2025-06-16 6:43 UTC (permalink / raw) To: mwalle, tudor.ambarus, robh, krzk+dt, conor+dt, Nicolas.Ferre, alexandre.belloni, claudiu.beznea, pratyush, miquel.raynal, richard, vigneshr, devicetree, linux-arm-kernel, linux-kernel, linux-mtd Hi Michael, On 12/06/25 11:47 am, Michael Walle wrote: > Hi, > >>>>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the >>>>>> net drivers using the nvmem property.The offset is set to 0x0 >>>>>> since the factory programmed address is available in the >>>>>> resource managed space and the size determine if the requested >>>>>> address is of EUI48 (0x6) or EUI-64 (0x8) type. >>>>>> This is useful for cases where U-Boot is skipped and the Ethernet >>>>>> MAC address is needed to be configured by the kernel >>>>>> >>>>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com> >>>>>> --- >>>>>> .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi | 13 +++++++++++++ >>>>>> 1 file changed, 13 insertions(+) >>>>>> >>>>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >>>>>> index b34c5072425a..be06df1b7d66 100644 >>>>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >>>>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi >>>>>> @@ -210,6 +210,9 @@ &macb0 { >>>>>> #size-cells = <0>; >>>>>> phy-mode = "rmii"; >>>>>> >>>>>> + nvmem-cells = <&mac_address_eui48>; >>>>>> + nvmem-cell-names = "mac-address"; >>>>>> + >>>>>> ethernet-phy@0 { >>>>>> reg = <0x0>; >>>>>> interrupt-parent = <&pioA>; >>>>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 { >>>>>> m25p,fast-read; >>>>>> status = "disabled"; >>>>>> >>>>>> + nvmem-layout { >>> >>> IMHO this should be "sfdp {". >>> >>>>>> + compatible = "fixed-layout"; >>> >>> Please read my feedback on the first version again.. >>> >>> For the DT maintainers. The SFDP is a small table based content that >>> provide basic information about the flash. There are standard tables >>> which are specified by the JEDEC standard and there are vendor >>> tables, most of the time without proper documentation (or none at >>> all). >>> >>> Somehow we need to specify at what table we want to look at. I'd >>> like to see a binding which can potentially expose anything inside >>> the SFDP. >>> >>> So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN" >>> where NNNN is the table parameter id. Additionally, the standard ids >>> could have names like "jedec,sfdp-bfpt" (basic flash parameter table). >>> >>> So in your case that would be: >>> >>> flash { >>> sfdp { >>> mac_address: table-1 { >>> compatible = "jedec,sfdp-idNNNN"; >>> }; >>> }; >> >> Should the nvmem-layout be included as a child node under sfdp {}, or >> should it be implemented as a separate vendor-specific driver to handle >> the changes introduced in patch 1/3 ? > > There is no nvmem-layout involved here. > > But another possibility is to make it one. Then you have to > (1) expose the *entire* sfpd as a nvmem device > (2a) write an nvmem-layouts driver (in drivers/nvmem/layouts/) > (2b) come up with a DT binding that is generic enough to expose > various parameters of the SFDP, not just a one-off for the > MAC address use case. > > Maybe that is even a better fit. > >>> }; >>> >>> I don't know what NNNN is. Could you please provide a dump of the >>> sfdp of your flash. >> >> Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1 >> of 11.0 APPENDIX > > Please dump it according to [1], so we have it in a machine readable > format. Here is the dump of sfdp as per [1], [root@sama5 ~]$ xxd -p /sys/bus/spi/devices/spi2.0/spi-nor/sfdp 53464450060102ff00060110300000ff81000106000100ffbf00021c0002 0001fffffffffffffffffffffffffffffffffd20f1ffffffff0344eb086b 083b80bbfeffffffffff00ffffff440b0c200dd80fd810d820914824806f 1d81ed0f773830b030b0f7ffffff29c25cfff030c080ffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffff0004fff37f0000f57f0000f9ff 7d00f57f0000f37f0000ffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffbf2643ffb95ffdff30f260f332ff0a122346ff0f19320f1919ffffff ffffffff00669938ff05013506040232b03072428de89888a585c09faf5a ffff06ec060c0003080bffffffffff07ffff0202ff060300fdfd040700fc 0300fefe0202070e3004916245a09240001ec0000005a092 > > -michael > > [1] https://docs.kernel.org/driver-api/mtd/spi-nor.html > > >>> On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote: > >> >> https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf >> >> >> The vendor parameter ID is 'BF' if I am not wrong. >>> >>> -michael >>> >>>>>> + #address-cells = <1>; >>>>>> + #size-cells = <1>; >>>>>> + >>>>>> + mac_address_eui48: mac-address@0 { >>>>>> + reg = <0x0 0x6>; >>>>>> + }; >>>>> >>>>> How would this work if in the future the mchp vendor table adds some >>>>> other info that needs to be referenced as nvmem? How do you distinguish >>>>> the info from the table? >>>>> Would it be possible to have some kind of address and size to reference >>>>> the SFDP? >>>> >>>> I was previously advised not to hardcode the offset in the Device Tree >>>> [1]. In the current implementation (patch 1/3), the read callback for >>>> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC >>>> addresses based on the nvmem size, which corresponds to the size of the >>>> respective MAC address. >>>> >>>> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/ >>>> >>>>> >>>>>> + }; >>>>>> + >>>>>> partitions { >>>>>> compatible = "fixed-partitions"; >>>>>> #address-cells = <1>; >>>>> >>> > -- Thanks and Regards, Manikandan M. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region 2025-05-21 7:03 [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Manikandan Muralidharan ` (2 preceding siblings ...) 2025-05-21 7:03 ` [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address Manikandan Muralidharan @ 2025-05-21 12:56 ` Rob Herring (Arm) 3 siblings, 0 replies; 15+ messages in thread From: Rob Herring (Arm) @ 2025-05-21 12:56 UTC (permalink / raw) To: Manikandan Muralidharan Cc: linux-mtd, pratyush, conor+dt, tudor.ambarus, krzk+dt, miquel.raynal, mwalle, linux-kernel, claudiu.beznea, vigneshr, linux-arm-kernel, alexandre.belloni, nicolas.ferre, devicetree, richard On Wed, 21 May 2025 12:33:33 +0530, Manikandan Muralidharan wrote: > This patch series adds support to parse the SFDP SST vendor map, read and > store the EUI-48 and EUI-64 Address (if its programmed) using the > resource-managed devm_kcalloc which will be freed on driver detach. > Register EUI addresses into NVMEM framework for the net drivers to access > them using nvmem properties. > This change ensures consistent and reliable MAC address retrieval > from QSPI benefiting boards like the sama5d27_wlsom1, sama5d29 curiosity > and sam9x75 curiosity. > > -------- > changes in v3: > - 2/3 - add support to update the QSPI partition into 'fixed-partition' > binding in sama5d27_wlsom1 > - 3/3 - add nvmem-layout in qspi node for EUI48 MAC Address and nvmem cell > properties for macb node in sama5d27_wlsom1 > > changes in v2: > - 1/3 - parse the SST vendor table, read and store the addresses > into a resource - managed space. Register the addresses > into NVMEM framework > - 2/3 - add support to update the QSPI partition into 'fixed-partition' > binding > -------- > > Manikandan Muralidharan (3): > mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI > addresses into NVMEM framework > ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using > "fixed-partition" binding > ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for > EUI48 MAC Address > > .../dts/microchip/at91-sama5d27_wlsom1.dtsi | 65 ++++--- > drivers/mtd/spi-nor/sfdp.c | 161 ++++++++++++++++++ > include/linux/mtd/spi-nor.h | 7 + > 3 files changed, 209 insertions(+), 24 deletions(-) > > -- > 2.25.1 > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade This patch series was applied (using b4) to base: Base: attempting to guess base-commit... Base: tags/next-20250516 (best guess, 2/3 blobs matched) If this is not the correct base, please add 'base-commit' tag (or use b4 which does this automatically) New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/microchip/' for 20250521070336.402202-1-manikandan.m@microchip.com: arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1_ek.dtb: flash@0 (jedec,spi-nor): Unevaluated properties are not allowed ('nvmem-layout', 'spi-cs-setup-ns' were unexpected) from schema $id: http://devicetree.org/schemas/mtd/jedec,spi-nor.yaml# ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-06-16 6:44 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-21 7:03 [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Manikandan Muralidharan 2025-05-21 7:03 ` [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework Manikandan Muralidharan 2025-06-07 11:01 ` Claudiu Beznea 2025-06-09 8:04 ` Tudor Ambarus 2025-06-11 6:49 ` Michael Walle 2025-05-21 7:03 ` [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding Manikandan Muralidharan 2025-06-07 11:02 ` Claudiu Beznea 2025-05-21 7:03 ` [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address Manikandan Muralidharan 2025-06-09 8:17 ` Tudor Ambarus 2025-06-09 9:27 ` Manikandan.M 2025-06-11 7:31 ` Michael Walle 2025-06-12 3:33 ` Manikandan.M 2025-06-12 6:17 ` Michael Walle 2025-06-16 6:43 ` Manikandan.M 2025-05-21 12:56 ` [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region Rob Herring (Arm)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).