Message-ID: <13261104-4544-b198-0d0c-0d39651f27c9@linaro.org>
Date:   Mon, 6 Feb 2023 10:45:03 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v2 2/2] nvmem: brcm_nvram: use bcm47xx buffered data
Content-Language: en-US
To:     arinc9.unal@gmail.com,
        =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Willem-Jan de Hoog <wdehoog@exalondelft.nl>,
        Florian Fainelli <f.fainelli@gmail.com>
Cc:     =?UTF-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>,
        =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>,
        linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
        erkin.bozoglu@xeront.com
References: <20230206101642.22720-1-wdehoog@exalondelft.nl>
 <20230206101642.22720-3-wdehoog@exalondelft.nl>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20230206101642.22720-3-wdehoog@exalondelft.nl>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


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,