From: Rafał Miłecki <[email protected]>
This change is required for NVMEM consumers to get expected MAC address
from U-Boot env data variable "ethaddr".
To address some previous concerns:
1. Yes, U-Boot env binding & driver should be converted to NVMEM layout
2. My priority is to get working MAC rather than fancy DT syntax
3. I AM going to convert U-Boot env into layout later
4. This work DOESN'T conflict with layout migration
5. This code WILL stay during / after layout migration
My brain limitations don't allow me to handle everything at once, please
let's review what I already have here.
Rafał Miłecki (3):
dt-bindings: nvmem: u-boot,env: add MAC's #nvmem-cell-cells
nvmem: core: support specifying both: cell raw data & post read
lengths
nvmem: u-boot-env: post-process "ethaddr" env variable
.../devicetree/bindings/nvmem/u-boot,env.yaml | 7 ++++-
drivers/nvmem/Kconfig | 1 +
drivers/nvmem/core.c | 11 +++++---
drivers/nvmem/u-boot-env.c | 26 +++++++++++++++++++
include/linux/nvmem-provider.h | 2 ++
5 files changed, 42 insertions(+), 5 deletions(-)
--
2.34.1
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]>
Reviewed-by: Rob Herring <[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..36d97fb87865 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.
+ properties:
+ "#nvmem-cell-cells":
+ description: The first argument is a MAC address offset.
+ const: 1
additionalProperties: false
@@ -72,6 +76,7 @@ examples:
reg = <0x40000 0x10000>;
mac: ethaddr {
+ #nvmem-cell-cells = <1>;
};
};
};
--
2.34.1
From: Rafał Miłecki <[email protected]>
Callback .read_post_process() is designed to modify raw cell content
before providing it to the consumer. So far we were dealing with
modifications that didn't affect cell size (length). In some cases
however cell content needs to be reformatted and resized.
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).
There were few discussions how to optimally handle that. Following
possible solutions were considered:
1. Allow .read_post_process() to realloc (resize) content buffer
2. Allow .read_post_process() to adjust (decrease) just buffer length
3. Register NVMEM cells using post-read sizes
The preferred solution was the last one. The problem is that simply
adjusting "bytes" in NVMEM providers would result in core code NOT
passing whole raw data to .read_post_process() callbacks. It means
callback functions couldn't do their job without somehow manually
reading original cell content on their own.
This patch deals with that by registering NVMEM cells with both lengths:
raw content one and post read one. It allows:
1. Core code to read whole raw cell content
2. Callbacks to return content they want
Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/nvmem/core.c | 11 +++++++----
include/linux/nvmem-provider.h | 2 ++
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 212c5ba5789f..a62973d010ff 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -50,6 +50,7 @@ struct nvmem_device {
struct nvmem_cell_entry {
const char *name;
int offset;
+ size_t raw_len;
int bytes;
int bit_offset;
int nbits;
@@ -469,6 +470,7 @@ static int nvmem_cell_info_to_nvmem_cell_entry_nodup(struct nvmem_device *nvmem,
{
cell->nvmem = nvmem;
cell->offset = info->offset;
+ cell->raw_len = info->raw_len ?: info->bytes;
cell->bytes = info->bytes;
cell->name = info->name;
cell->read_post_process = info->read_post_process;
@@ -1560,7 +1562,7 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
{
int rc;
- rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->bytes);
+ rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->raw_len);
if (rc)
return rc;
@@ -1571,7 +1573,7 @@ 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, cell->raw_len);
if (rc)
return rc;
}
@@ -1594,14 +1596,15 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
*/
void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len)
{
- struct nvmem_device *nvmem = cell->entry->nvmem;
+ struct nvmem_cell_entry *entry = cell->entry;
+ struct nvmem_device *nvmem = entry->nvmem;
u8 *buf;
int rc;
if (!nvmem)
return ERR_PTR(-EINVAL);
- buf = kzalloc(cell->entry->bytes, GFP_KERNEL);
+ buf = kzalloc(max_t(size_t, entry->raw_len, entry->bytes), GFP_KERNEL);
if (!buf)
return ERR_PTR(-ENOMEM);
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 0cf9f9490514..8ffb42ba0f62 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -51,6 +51,7 @@ struct nvmem_keepout {
* struct nvmem_cell_info - NVMEM cell description
* @name: Name.
* @offset: Offset within the NVMEM device.
+ * @raw_len: Length of raw data (without post processing).
* @bytes: Length of the cell.
* @bit_offset: Bit offset if cell is smaller than a byte.
* @nbits: Number of bits.
@@ -62,6 +63,7 @@ struct nvmem_keepout {
struct nvmem_cell_info {
const char *name;
unsigned int offset;
+ size_t raw_len;
unsigned int bytes;
unsigned int bit_offset;
unsigned int nbits;
--
2.34.1
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/Kconfig | 1 +
drivers/nvmem/u-boot-env.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index a2afba11c890..b291b27048c7 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -340,6 +340,7 @@ config NVMEM_U_BOOT_ENV
tristate "U-Boot environment variables support"
depends on OF && MTD
select CRC32
+ select GENERIC_NET_UTILS
help
U-Boot stores its setup as environment variables. This driver adds
support for verifying & exporting such data. It also exposes variables
diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c
index 29b1d87a3c51..ee9fd9989b6e 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,25 @@ 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 *buf, size_t bytes)
+{
+ u8 mac[ETH_ALEN];
+
+ if (bytes != 3 * ETH_ALEN - 1)
+ return -EINVAL;
+
+ if (!mac_pton(buf, mac))
+ return -EINVAL;
+
+ if (index)
+ eth_addr_add(mac, index);
+
+ ether_addr_copy(buf, mac);
+
+ 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 +122,11 @@ 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].raw_len = strlen(value);
+ priv->cells[idx].bytes = ETH_ALEN;
+ priv->cells[idx].read_post_process = u_boot_env_read_post_process_ethaddr;
+ }
}
if (WARN_ON(idx != priv->ncells))
--
2.34.1
On 18/03/2023 17:13, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> This change is required for NVMEM consumers to get expected MAC address
> from U-Boot env data variable "ethaddr".
>
> To address some previous concerns:
> 1. Yes, U-Boot env binding & driver should be converted to NVMEM layout
> 2. My priority is to get working MAC rather than fancy DT syntax
> 3. I AM going to convert U-Boot env into layout later
> 4. This work DOESN'T conflict with layout migration
> 5. This code WILL stay during / after layout migration
>
> My brain limitations don't allow me to handle everything at once, please
> let's review what I already have here.
Thanks for your patience.
Applied now,
--srini
>
> Rafał Miłecki (3):
> dt-bindings: nvmem: u-boot,env: add MAC's #nvmem-cell-cells
> nvmem: core: support specifying both: cell raw data & post read
> lengths
> nvmem: u-boot-env: post-process "ethaddr" env variable
>
> .../devicetree/bindings/nvmem/u-boot,env.yaml | 7 ++++-
> drivers/nvmem/Kconfig | 1 +
> drivers/nvmem/core.c | 11 +++++---
> drivers/nvmem/u-boot-env.c | 26 +++++++++++++++++++
> include/linux/nvmem-provider.h | 2 ++
> 5 files changed, 42 insertions(+), 5 deletions(-)
>