2022-11-27 23:14:40

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 1/2] nvmem: core: refactor .cell_post_process() CB arguments

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

Pass whole NVMEM cell struct and length pointer as arguments to callback
functions.

This allows:

1. Cells content to be modified based on more info
Some cells (identified by their names) contain specific data that
needs further processing. This can be e.g. MAC address stored in an
ASCII format. NVMEM consumers expect MAC to be read in a binary form.
More complex cells may be additionally described in DT. This change
allows also accessing relevant DT nodes and reading extra info.

2. Adjusting data length
If cell processing results in reformatting it, it's required to
adjust length. This again applies e.g. to the MAC format change from
ASCII to the byte-based.

Later on we may consider more cleanups & features like:
1. Dropping "const char *id" and just using NVMEM cell name
2. Adding extra argument for cells providing multiple values

Signed-off-by: Rafał Miłecki <[email protected]>
---
This solution conflicts with 1 part of Michael's work:
[PATCH v2 00/20] nvmem: core: introduce NVMEM layouts
https://lore.kernel.org/linux-arm-kernel/[email protected]/

Instead of:
1. Adding NVMEM cell-level post_process callback
2. Adding callback (.fixup_cell_info()) for setting callbacks
3. Dropping NVMEM device-level post_process callback
I decided to refactor existing callback.

Michael's work on adding #nvmem-cell-cells should be possible to easily
rebase on top of those changes.

This doen't add support for 1 cell providing multiple values. That needs
to be added later once we sort out #nvmem-cell-cells bindings. This
fixes the basic case with reformatting cells data.
---
drivers/nvmem/core.c | 19 +++----------------
drivers/nvmem/imx-ocotp.c | 8 ++++----
include/linux/nvmem-provider.h | 17 ++++++++++++++---
3 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 321d7d63e068..0bc3e26e4ca8 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -46,16 +46,6 @@ struct nvmem_device {
#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)

#define FLAG_COMPAT BIT(0)
-struct nvmem_cell_entry {
- const char *name;
- int offset;
- int bytes;
- int bit_offset;
- int nbits;
- struct device_node *np;
- struct nvmem_device *nvmem;
- struct list_head node;
-};

struct nvmem_cell {
struct nvmem_cell_entry *entry;
@@ -1416,24 +1406,21 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
int rc;

rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->bytes);
-
if (rc)
return rc;
+ if (len)
+ *len = cell->bytes;

/* shift bits in-place */
if (cell->bit_offset || cell->nbits)
nvmem_shift_read_buffer_in_place(cell, buf);

if (nvmem->cell_post_process) {
- rc = nvmem->cell_post_process(nvmem->priv, id,
- cell->offset, buf, cell->bytes);
+ rc = nvmem->cell_post_process(nvmem->priv, cell, id, buf, len);
if (rc)
return rc;
}

- if (len)
- *len = cell->bytes;
-
return 0;
}

diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 14284e866f26..d383989d48bf 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -222,8 +222,8 @@ static int imx_ocotp_read(void *context, unsigned int offset,
return ret;
}

-static int imx_ocotp_cell_pp(void *context, const char *id, unsigned int offset,
- void *data, size_t bytes)
+static int imx_ocotp_cell_pp(void *context, struct nvmem_cell_entry *cell,
+ const char *id, void *data, size_t *len)
{
struct ocotp_priv *priv = context;

@@ -233,8 +233,8 @@ static int imx_ocotp_cell_pp(void *context, const char *id, unsigned int offset,
u8 *buf = data;
int i;

- for (i = 0; i < bytes/2; i++)
- swap(buf[i], buf[bytes - i - 1]);
+ for (i = 0; i < cell->bytes / 2; i++)
+ swap(buf[i], buf[cell->bytes - i - 1]);
}
}

diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 50caa117cb62..b0d2b6af9f37 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -14,14 +14,25 @@
#include <linux/gpio/consumer.h>

struct nvmem_device;
-struct nvmem_cell_info;
+
+struct nvmem_cell_entry {
+ const char *name;
+ int offset;
+ int bytes;
+ int bit_offset;
+ int nbits;
+ struct device_node *np;
+ struct nvmem_device *nvmem;
+ struct list_head node;
+};
+
typedef int (*nvmem_reg_read_t)(void *priv, unsigned int offset,
void *val, size_t bytes);
typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset,
void *val, size_t bytes);
/* used for vendor specific post processing of cell data */
-typedef int (*nvmem_cell_post_process_t)(void *priv, const char *id, unsigned int offset,
- void *buf, size_t bytes);
+typedef int (*nvmem_cell_post_process_t)(void *priv, struct nvmem_cell_entry *cell, const char *id,
+ void *buf, size_t *len);

enum nvmem_type {
NVMEM_TYPE_UNKNOWN = 0,
--
2.34.1


2022-11-27 23:45:45

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/2] nvmem: u-boot-env: reformat MAC in "ethaddr" cell when reading

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

NVMEM consumers expect MAC in a byte-based format (see e.g.
nvmem_get_mac_address()). U-Boot environment data stores all values in
ASCII form.

Add post processing callback detecting "ethaddr" reads and reformat data
as expected. This fixes Ethernet drivers reading MAC from NVMEM devices.

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

diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c
index 2a87dda45188..d103a52e0008 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,24 @@ static int u_boot_env_read(void *context, unsigned int offset, void *val,
return 0;
}

+static int u_boot_env_cell_post_process(void *context, struct nvmem_cell_entry *cell,
+ const char *id, void *buf, size_t *len)
+{
+ struct u_boot_env *priv = context;
+
+ if (!strcmp(cell->name, "ethaddr")) {
+ u8 mac[ETH_ALEN];
+
+ if (mac_pton(buf, mac)) {
+ ether_addr_copy(buf, mac);
+ if (len)
+ *len = 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)
{
@@ -179,6 +199,7 @@ static int u_boot_env_probe(struct platform_device *pdev)
struct nvmem_config config = {
.name = "u-boot-env",
.reg_read = u_boot_env_read,
+ .cell_post_process = u_boot_env_cell_post_process,
};
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
--
2.34.1

2022-11-28 03:03:22

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] nvmem: u-boot-env: reformat MAC in "ethaddr" cell when reading

Hi Rafał,

I love your patch! Yet something to improve:

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on soc/for-next linus/master v6.1-rc7 next-20221125]
[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-refactor-cell_post_process-CB-arguments/20221128-071155
base: https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
patch link: https://lore.kernel.org/r/20221127231035.17547-2-zajec5%40gmail.com
patch subject: [PATCH 2/2] nvmem: u-boot-env: reformat MAC in "ethaddr" cell when reading
config: riscv-randconfig-r042-20221127
compiler: riscv64-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/7ed383c8df3e3a2435255360e03171ebdc4437b4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/nvmem-core-refactor-cell_post_process-CB-arguments/20221128-071155
git checkout 7ed383c8df3e3a2435255360e03171ebdc4437b4
# 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=riscv 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 >>):

riscv64-linux-ld: arch/riscv/kvm/vcpu.o: in function `.L0 ':
arch/riscv/kvm/vcpu.c:271: undefined reference to `riscv_cbom_block_size'
riscv64-linux-ld: arch/riscv/kvm/vcpu.c:271: undefined reference to `riscv_cbom_block_size'
riscv64-linux-ld: drivers/nvmem/u-boot-env.o: in function `.L0 ':
>> drivers/nvmem/u-boot-env.c:75: undefined reference to `mac_pton'


vim +75 drivers/nvmem/u-boot-env.c

66
67 static int u_boot_env_cell_post_process(void *context, struct nvmem_cell_entry *cell,
68 const char *id, void *buf, size_t *len)
69 {
70 struct u_boot_env *priv = context;
71
72 if (!strcmp(cell->name, "ethaddr")) {
73 u8 mac[ETH_ALEN];
74
> 75 if (mac_pton(buf, mac)) {
76 ether_addr_copy(buf, mac);
77 if (len)
78 *len = ETH_ALEN;
79 }
80 }
81
82 return 0;
83 }
84

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.68 kB)
config (179.46 kB)
Download all attachments

2022-11-28 04:02:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] nvmem: u-boot-env: reformat MAC in "ethaddr" cell when reading

Hi Rafał,

I love your patch! Perhaps something to improve:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on soc/for-next linus/master v6.1-rc7 next-20221125]
[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-refactor-cell_post_process-CB-arguments/20221128-071155
base: https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
patch link: https://lore.kernel.org/r/20221127231035.17547-2-zajec5%40gmail.com
patch subject: [PATCH 2/2] nvmem: u-boot-env: reformat MAC in "ethaddr" cell when reading
config: x86_64-allyesconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/7ed383c8df3e3a2435255360e03171ebdc4437b4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rafa-Mi-ecki/nvmem-core-refactor-cell_post_process-CB-arguments/20221128-071155
git checkout 7ed383c8df3e3a2435255360e03171ebdc4437b4
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

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

All warnings (new ones prefixed by >>):

drivers/nvmem/u-boot-env.c: In function 'u_boot_env_cell_post_process':
>> drivers/nvmem/u-boot-env.c:70:28: warning: unused variable 'priv' [-Wunused-variable]
70 | struct u_boot_env *priv = context;
| ^~~~


vim +/priv +70 drivers/nvmem/u-boot-env.c

66
67 static int u_boot_env_cell_post_process(void *context, struct nvmem_cell_entry *cell,
68 const char *id, void *buf, size_t *len)
69 {
> 70 struct u_boot_env *priv = context;
71
72 if (!strcmp(cell->name, "ethaddr")) {
73 u8 mac[ETH_ALEN];
74
75 if (mac_pton(buf, mac)) {
76 ether_addr_copy(buf, mac);
77 if (len)
78 *len = ETH_ALEN;
79 }
80 }
81
82 return 0;
83 }
84

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.39 kB)
config (297.60 kB)
Download all attachments