Words from Willem-Jan de Hoog:
On OpenWrt 22.03.3, Asus RT-AC88U does not boot anymore:
UBI: auto-attach mtd4
ubi0: attaching mtd4
ubi0 error: 0xc04f0b3c: PEB 0 contains corrupted VID header, and the
data does not contain all 0xFF
ubi0 error: 0xc04f0b4c: this may be a non-UBI PEB or a severe VID
header corruption which requires manual inspection
The problem seems to be that brcm_nvram_read accesses its (mapped) io
memory. When doing so the correct data is read but after that the
mtd/ubi process fails to work.
The bcm47xx_nvram.c code has buffered the nvram data so the cells value
can be read from there.
v2: Fix the wrong author information.
Willem-Jan de Hoog (2):
firmware: bcm47xx_nvram: allow to read from buffered nvram data
nvmem: brcm_nvram: use bcm47xx buffered data
drivers/firmware/broadcom/bcm47xx_nvram.c | 14 ++++++++++++++
drivers/nvmem/brcm_nvram.c | 8 ++++++++
include/linux/bcm47xx_nvram.h | 6 ++++++
3 files changed, 28 insertions(+)
From: Willem-Jan de Hoog <[email protected]>
The bcm47xx module has a copy of the NVRAM data in ram. When available, use
this one instead of reading from io memory since it causes mtd/ubi to fail.
[ [email protected]: Improved patch subject and log ]
Signed-off-by: Willem-Jan de Hoog <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/nvmem/brcm_nvram.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/nvmem/brcm_nvram.c b/drivers/nvmem/brcm_nvram.c
index 34130449f2d2..f74bcb1c948e 100644
--- a/drivers/nvmem/brcm_nvram.c
+++ b/drivers/nvmem/brcm_nvram.c
@@ -33,6 +33,12 @@ struct brcm_nvram_header {
static int brcm_nvram_read(void *context, unsigned int offset, void *val,
size_t bytes)
{
+#ifdef CONFIG_BCM47XX_NVRAM
+
+ return bcm47xx_nvram_read(offset, val, bytes);
+
+#else
+
struct brcm_nvram *priv = context;
u8 *dst = val;
@@ -40,6 +46,8 @@ static int brcm_nvram_read(void *context, unsigned int offset, void *val,
*dst++ = readb(priv->base + offset++);
return 0;
+
+#endif
}
static int brcm_nvram_add_cells(struct brcm_nvram *priv, uint8_t *data,
--
2.37.2
From: Willem-Jan de Hoog <[email protected]>
The bcm47xx code makes a copy of the NVRAM data in ram. Allow access to
this data so property values can be read using nvmem cell api.
[ [email protected]: Improved patch subject and log ]
Signed-off-by: Willem-Jan de Hoog <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/firmware/broadcom/bcm47xx_nvram.c | 14 ++++++++++++++
include/linux/bcm47xx_nvram.h | 6 ++++++
2 files changed, 20 insertions(+)
diff --git a/drivers/firmware/broadcom/bcm47xx_nvram.c b/drivers/firmware/broadcom/bcm47xx_nvram.c
index 5f47dbf4889a..7e5c62dc702f 100644
--- a/drivers/firmware/broadcom/bcm47xx_nvram.c
+++ b/drivers/firmware/broadcom/bcm47xx_nvram.c
@@ -182,6 +182,20 @@ static int nvram_init(void)
return -ENXIO;
}
+int bcm47xx_nvram_read(unsigned int offset, char *val, size_t val_len)
+{
+ if (!nvram_len)
+ return -ENXIO;
+
+ if ((offset+val_len) > nvram_len)
+ return -EINVAL;
+
+ while (val_len--)
+ *val++ = nvram_buf[offset++];
+
+ return 0;
+}
+
int bcm47xx_nvram_getenv(const char *name, char *val, size_t val_len)
{
char *var, *value, *end, *eq;
diff --git a/include/linux/bcm47xx_nvram.h b/include/linux/bcm47xx_nvram.h
index 7615f8d7b1ed..b265b8ce6434 100644
--- a/include/linux/bcm47xx_nvram.h
+++ b/include/linux/bcm47xx_nvram.h
@@ -20,6 +20,7 @@ static inline void bcm47xx_nvram_release_contents(char *nvram)
{
vfree(nvram);
};
+int bcm47xx_nvram_read(unsigned int offset, char *val, size_t val_len);
#else
static inline int bcm47xx_nvram_init_from_iomem(void __iomem *nvram_start,
size_t res_size)
@@ -48,6 +49,11 @@ static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
static inline void bcm47xx_nvram_release_contents(char *nvram)
{
};
+
+static inline int bcm47xx_nvram_read(unsigned int offset, char *val, size_t val_len)
+{
+ return -ENOTSUPP;
+}:
#endif
#endif /* __BCM47XX_NVRAM_H */
--
2.37.2
On 06/02/2023 10:16, [email protected] wrote:
> From: Willem-Jan de Hoog <[email protected]>
>
> The bcm47xx module has a copy of the NVRAM data in ram. When available, use
> this one instead of reading from io memory since it causes mtd/ubi to fail.
>
> [ [email protected]: Improved patch subject and log ]
>
> Signed-off-by: Willem-Jan de Hoog <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> drivers/nvmem/brcm_nvram.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/nvmem/brcm_nvram.c b/drivers/nvmem/brcm_nvram.c
> index 34130449f2d2..f74bcb1c948e 100644
> --- a/drivers/nvmem/brcm_nvram.c
> +++ b/drivers/nvmem/brcm_nvram.c
> @@ -33,6 +33,12 @@ struct brcm_nvram_header {
> static int brcm_nvram_read(void *context, unsigned int offset, void *val,
> size_t bytes)
> {
> +#ifdef CONFIG_BCM47XX_NVRAM
> +
Hmm this is not the right way to fix/add support for this, consider
adding SoC specific compatible and based on that you can take right
choice at runtime.
there are 2 issues with this patch which you should probably consider.
1. no dependency expressed in any way in Kconfig.
2. If we build these as modules everything will break as the symbol is
not exported.
--srini
> + return bcm47xx_nvram_read(offset, val, bytes);
> +
> +#else
> +
> struct brcm_nvram *priv = context;
> u8 *dst = val;
>
> @@ -40,6 +46,8 @@ static int brcm_nvram_read(void *context, unsigned int offset, void *val,
> *dst++ = readb(priv->base + offset++);
>
> return 0;
> +
> +#endif
> }
>
> static int brcm_nvram_add_cells(struct brcm_nvram *priv, uint8_t *data,
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.2-rc7 next-20230206]
[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/arinc9-unal-gmail-com/firmware-bcm47xx_nvram-allow-to-read-from-buffered-nvram-data/20230206-181817
patch link: https://lore.kernel.org/r/20230206101642.22720-3-wdehoog%40exalondelft.nl
patch subject: [PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20230207/[email protected]/config)
compiler: powerpc-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/33f35258e56cfe526d23f263596e9c76ea2d5925
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review arinc9-unal-gmail-com/firmware-bcm47xx_nvram-allow-to-read-from-buffered-nvram-data/20230206-181817
git checkout 33f35258e56cfe526d23f263596e9c76ea2d5925
# 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=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc 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 >>, old ones prefixed by <<):
>> ERROR: modpost: "bcm47xx_nvram_read" [drivers/nvmem/nvmem_brcm_nvram.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests