2023-01-05 17:37:58

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: nvmem: u-boot,env: add MAC's #nvmem-cell-cells

From: Rafał Miłecki <[email protected]>

U-Boot's "ethaddr" environment variable is very often used to store
*base* MAC address. It's used as a base for calculating addresses for
multiple interfaces. It's done by adding proper values. Actual offsets
are picked by manufacturers and vary across devices.

Signed-off-by: Rafał Miłecki <[email protected]>
---
Documentation/devicetree/bindings/nvmem/u-boot,env.yaml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
index cbc5c69fd405..1c139bd689ea 100644
--- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
+++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
@@ -50,7 +50,11 @@ properties:

ethaddr:
type: object
- description: Ethernet interface's MAC address
+ description:
+ Ethernet interfaces base MAC address. The first argument is an offset.
+ properties:
+ "#nvmem-cell-cells":
+ const: 1

additionalProperties: false

@@ -72,6 +76,7 @@ examples:
reg = <0x40000 0x10000>;

mac: ethaddr {
+ #nvmem-cell-cells = <1>;
};
};
};
--
2.34.1


2023-01-05 17:38:55

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 3/3] nvmem: u-boot-env: post process "ethaddr" env variable

From: Rafał Miłecki <[email protected]>

U-Boot environment variables are stored in ASCII format so "ethaddr"
requires parsing into binary to make it work with Ethernet interfaces.

This includes support for indexes to support #nvmem-cell-cells = <1>.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/nvmem/u-boot-env.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c
index 2a87dda45188..54283f8061b0 100644
--- a/drivers/nvmem/u-boot-env.c
+++ b/drivers/nvmem/u-boot-env.c
@@ -4,6 +4,8 @@
*/

#include <linux/crc32.h>
+#include <linux/etherdevice.h>
+#include <linux/if_ether.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/mtd/mtd.h>
@@ -70,6 +72,26 @@ static int u_boot_env_read(void *context, unsigned int offset, void *val,
return 0;
}

+static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index,
+ unsigned int offset, void *data, size_t *bytes)
+{
+ u8 mac[ETH_ALEN];
+
+ if (*bytes != 3 * ETH_ALEN - 1)
+ return -EINVAL;
+
+ if (!mac_pton(data, mac))
+ return -EINVAL;
+
+ if (index)
+ eth_addr_add(mac, index);
+
+ ether_addr_copy(data, mac);
+ *bytes = ETH_ALEN;
+
+ return 0;
+}
+
static int u_boot_env_add_cells(struct u_boot_env *priv, uint8_t *buf,
size_t data_offset, size_t data_len)
{
@@ -101,6 +123,8 @@ static int u_boot_env_add_cells(struct u_boot_env *priv, uint8_t *buf,
priv->cells[idx].offset = data_offset + value - data;
priv->cells[idx].bytes = strlen(value);
priv->cells[idx].np = of_get_child_by_name(dev->of_node, priv->cells[idx].name);
+ if (!strcmp(var, "ethaddr"))
+ priv->cells[idx].read_post_process = u_boot_env_read_post_process_ethaddr;
}

if (WARN_ON(idx != priv->ncells))
--
2.34.1

2023-01-05 18:19:11

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/3] nvmem: core: allow .read_post_process() callbacks to adjust data length

From: Rafał Miłecki <[email protected]>

Sometimes reading NVMEM cell value involves some data reformatting. It
requires passing updated size value to the caller. Support that.

It's required e.g. to provide properly formatted MAC address in case
it's stored in a non-binary format (e.g. using ASCII).

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/nvmem/core.c | 5 +++--
drivers/nvmem/imx-ocotp.c | 6 +++---
drivers/nvmem/layouts/onie-tlv.c | 2 +-
drivers/nvmem/layouts/sl28vpd.c | 4 ++--
include/linux/nvmem-provider.h | 2 +-
5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 1b61c8bf0de4..1daf5a1d3ec7 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1537,6 +1537,7 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
struct nvmem_cell_entry *cell,
void *buf, size_t *len, const char *id, int index)
{
+ size_t bytes = cell->bytes;
int rc;

rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->bytes);
@@ -1550,13 +1551,13 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,

if (cell->read_post_process) {
rc = cell->read_post_process(cell->priv, id, index,
- cell->offset, buf, cell->bytes);
+ cell->offset, buf, &bytes);
if (rc)
return rc;
}

if (len)
- *len = cell->bytes;
+ *len = bytes;

return 0;
}
diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index ac0edb6398f1..ebd0e9e0314e 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -223,15 +223,15 @@ static int imx_ocotp_read(void *context, unsigned int offset,
}

static int imx_ocotp_cell_pp(void *context, const char *id, int index,
- unsigned int offset, void *data, size_t bytes)
+ unsigned int offset, void *data, size_t *bytes)
{
u8 *buf = data;
int i;

/* Deal with some post processing of nvmem cell data */
if (id && !strcmp(id, "mac-address"))
- for (i = 0; i < bytes / 2; i++)
- swap(buf[i], buf[bytes - i - 1]);
+ for (i = 0; i < *bytes / 2; i++)
+ swap(buf[i], buf[*bytes - i - 1]);

return 0;
}
diff --git a/drivers/nvmem/layouts/onie-tlv.c b/drivers/nvmem/layouts/onie-tlv.c
index 074c7c700845..2cb7112229ba 100644
--- a/drivers/nvmem/layouts/onie-tlv.c
+++ b/drivers/nvmem/layouts/onie-tlv.c
@@ -76,7 +76,7 @@ static const char *onie_tlv_cell_name(u8 type)

static int onie_tlv_mac_read_cb(void *priv, const char *id, int index,
unsigned int offset, void *buf,
- size_t bytes)
+ size_t *bytes)
{
eth_addr_add(buf, index);

diff --git a/drivers/nvmem/layouts/sl28vpd.c b/drivers/nvmem/layouts/sl28vpd.c
index a36800f201a3..63c0da58ad60 100644
--- a/drivers/nvmem/layouts/sl28vpd.c
+++ b/drivers/nvmem/layouts/sl28vpd.c
@@ -23,9 +23,9 @@ struct sl28vpd_v1 {

static int sl28vpd_mac_address_pp(void *priv, const char *id, int index,
unsigned int offset, void *buf,
- size_t bytes)
+ size_t *bytes)
{
- if (bytes != ETH_ALEN)
+ if (*bytes != ETH_ALEN)
return -EINVAL;

if (index < 0)
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 0cf9f9490514..5d896eec2f1c 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -21,7 +21,7 @@ typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
/* used for vendor specific post processing of cell data */
typedef int (*nvmem_cell_post_process_t)(void *priv, const char *id, int index,
unsigned int offset, void *buf,
- size_t bytes);
+ size_t *bytes);

enum nvmem_type {
NVMEM_TYPE_UNKNOWN = 0,
--
2.34.1

2023-01-05 21:32:52

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH 2/3] nvmem: core: allow .read_post_process() callbacks to adjust data length

Hi,

Am 2023-01-05 18:10, schrieb Rafał Miłecki:
> From: Rafał Miłecki <[email protected]>
>
> Sometimes reading NVMEM cell value involves some data reformatting. It
> requires passing updated size value to the caller. Support that.

Wouldn't it make more sense to convert that driver to
proper nvmem layouts, where
(1) you get that for free,
(2) support others storages than just mtd
(3) don't duplicate the mtd read code

-michael

2023-01-05 21:44:56

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 3/3] nvmem: u-boot-env: post process "ethaddr" env variable

Hi Rafał,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20230105]
[cannot apply to robh/for-next shawnguo/for-next krzk-dt/for-next linus/master v6.2-rc2 v6.2-rc1 v6.1 v6.2-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Rafa-Mi-ecki/nvmem-core-allow-read_post_process-callbacks-to-adjust-data-length/20230106-012141
patch link: https://lore.kernel.org/r/20230105171038.13649-3-zajec5%40gmail.com
patch subject: [PATCH 3/3] nvmem: u-boot-env: post process "ethaddr" env variable
config: nios2-buildonly-randconfig-r005-20230105
compiler: nios2-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c87cc9dc60051170044407765ab613e77c55b348
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/nvmem-core-allow-read_post_process-callbacks-to-adjust-data-length/20230106-012141
git checkout c87cc9dc60051170044407765ab613e77c55b348
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=nios2 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

nios2-linux-ld: drivers/nvmem/u-boot-env.o: in function `u_boot_env_read_post_process_ethaddr':
>> u-boot-env.c:(.text+0x2ec): undefined reference to `mac_pton'
u-boot-env.c:(.text+0x2ec): relocation truncated to fit: R_NIOS2_CALL26 against `mac_pton'

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


Attachments:
(No filename) (2.15 kB)
config (154.03 kB)
Download all attachments

2023-01-08 22:30:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: nvmem: u-boot,env: add MAC's #nvmem-cell-cells

On Thu, Jan 05, 2023 at 06:10:36PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> U-Boot's "ethaddr" environment variable is very often used to store
> *base* MAC address. It's used as a base for calculating addresses for
> multiple interfaces. It's done by adding proper values. Actual offsets
> are picked by manufacturers and vary across devices.
>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> Documentation/devicetree/bindings/nvmem/u-boot,env.yaml | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> index cbc5c69fd405..1c139bd689ea 100644
> --- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> @@ -50,7 +50,11 @@ properties:
>
> ethaddr:
> type: object
> - description: Ethernet interface's MAC address
> + description:
> + Ethernet interfaces base MAC address. The first argument is an offset.

The 2nd sentence belongs in the '#nvmem-cell-cells' description.

> + properties:
> + "#nvmem-cell-cells":
> + const: 1
>
> additionalProperties: false
>
> @@ -72,6 +76,7 @@ examples:
> reg = <0x40000 0x10000>;
>
> mac: ethaddr {
> + #nvmem-cell-cells = <1>;
> };
> };
> };
> --
> 2.34.1
>
>