On Wed Mar 5, 2025 at 11:01 AM CET, Manikandan Muralidharan wrote:
> From: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
>
> EUI identifier and the MAC Address of the Ethernet Interface is stored
> after the SFDP table of contents starting at address 0x260 in the
> QSPI memory.
> Register the entire SFDP region read by the spi-nor (nor->sfdp) into the
> NVMEM framework and read the MAC Address 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 sama5d29 curiosity and sam9x75 curiosity.
>
> Signed-off-by: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
> [manikandan.m@xxxxxxxxxxxxx: Integrate the nvmem->read callback framework]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@xxxxxxxxxxxxx>
> ---
> drivers/mtd/spi-nor/sst.c | 62 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
> index 175211fe6a5e..a0abf201ad41 100644
> --- a/drivers/mtd/spi-nor/sst.c
> +++ b/drivers/mtd/spi-nor/sst.c
> @@ -5,6 +5,7 @@
> */
>
> #include <linux/mtd/spi-nor.h>
> +#include <linux/nvmem-provider.h>
>
> #include "core.h"
>
> @@ -13,6 +14,8 @@
>
> #define SST26VF_CR_BPNV BIT(3)
>
> +#define SST26VF_SFDP_EUI48 0x30
> +
> static int sst26vf_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
> {
> return -EOPNOTSUPP;
> @@ -56,8 +59,67 @@ static int sst26vf_nor_late_init(struct spi_nor *nor)
> return 0;
> }
>
> +/**
> + * sst26vf_sfdp_mac_addr_read() - check if the EUI-48 MAC Address is programmed
> + * and read the data from the prestored SFDP data
> + *
> + * @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 sst26vf_sfdp_mac_addr_read(void *priv, unsigned int off,
> + void *val, size_t bytes)
> +{
> + struct spi_nor *nor = priv;
> + struct sfdp *sfdp = nor->sfdp;
> + loff_t offset = off;
> + size_t sfdp_size;
> +
> + /*
> + * Check if the EUI-48 MAC address is programmed in the next six address
> + * locations.
> + * @off is programmed in the DT and stores the start of MAC Address
> + * byte, (off - 1) stores the bit length of the Extended Unique
> + * Identifier
> + */
> + if (SST26VF_SFDP_EUI48 != *((u8 *)sfdp->dwords + (offset - 1)))
> + return -EINVAL;
What happens if you read at a different offset? You're exposing
the entire SFDP region. What happens if there is a 0x30 at a
different location?
> +
> + sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
> + memory_read_from_buffer(val, bytes, &offset, sfdp->dwords,
> + sfdp_size);
> + return 0;
> +}
> +
> +static struct nvmem_config sst26vf_sfdp_nvmem_config = {
> + .word_size = 1,
> + .stride = 1,
> +};
> +
> +static int sst26vf_nor_post_sfdp(struct spi_nor *nor)
> +{
> + struct nvmem_device *nvmem;
> +
> + sst26vf_sfdp_nvmem_config.dev = nor->dev;
> + sst26vf_sfdp_nvmem_config.size = nor->sfdp->num_dwords * sizeof(*nor->sfdp->dwords);
> + sst26vf_sfdp_nvmem_config.priv = nor;
> + sst26vf_sfdp_nvmem_config.reg_read = sst26vf_sfdp_mac_addr_read;
> +
> + nvmem = devm_nvmem_register(nor->dev, &sst26vf_sfdp_nvmem_config);
> + if (IS_ERR(nvmem)) {
> + dev_err(nor->dev, "failed to register NVMEM device: %ld\n", PTR_ERR(nvmem));
> + return PTR_ERR(nvmem);
I don't think it makes sense to have this one-off in a particular
driver. If at all, this should be handled in the core. Sorry, but
this really looks like an ugly hack.
-michael
> + }
> +
> + return 0;
> +}
> +
> static const struct spi_nor_fixups sst26vf_nor_fixups = {
> .late_init = sst26vf_nor_late_init,
> + .post_sfdp = sst26vf_nor_post_sfdp,
> };
>
> static const struct flash_info sst_nor_parts[] = {
