[PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data

arinc9.unal@gmail.com posted 2 patches 2 years, 7 months ago
[PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data
Posted by arinc9.unal@gmail.com 2 years, 7 months ago
From: Willem-Jan de Hoog <wdehoog@exalondelft.nl>

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.

[ arinc.unal@arinc9.com: Improved patch subject and log ]

Signed-off-by: Willem-Jan de Hoog <wdehoog@exalondelft.nl>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 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

Re: [PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data
Posted by kernel test robot 2 years, 7 months ago
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/202302070008.J1xnBnSn-lkp@intel.com/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 <lkp@intel.com>

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
Re: [PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data
Posted by Srinivas Kandagatla 2 years, 7 months ago

On 06/02/2023 10:16, arinc9.unal@gmail.com wrote:
> From: Willem-Jan de Hoog <wdehoog@exalondelft.nl>
> 
> 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.
> 
> [ arinc.unal@arinc9.com: Improved patch subject and log ]
> 
> Signed-off-by: Willem-Jan de Hoog <wdehoog@exalondelft.nl>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>   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,