Re: [PATCH V2 mips/linux.git] firmware: bcm47xx_nvram: refactor finding & reading NVRAM

From: Philippe Mathieu-Daudé
Date: Fri Mar 05 2021 - 04:59:38 EST


Hi Rafał,

On Fri, Mar 5, 2021 at 6:55 AM Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
>
> From: Rafał Miłecki <rafal@xxxxxxxxxx>
>
> 1. Use meaningful variable names (e.g. "flash_start", "res_size" instead
> of e.g. "iobase", "end")
> 2. Always operate on "offset" instead of mix of start, end, size, etc.

"instead of a mix"

> 3. Add helper checking for NVRAM to avoid duplicating code
> 4. Use "found" variable instead of goto
> 5. Use simpler checking of offsets and sizes (2 nested loops with
> trivial check instead of extra function)

This could be a series of trivial patches, why did you choose to make a mixed
bag harder to review?

>
> This change has been tested on BCM4706. Updated code checks the same
> offsets as before. Driver still finds & copies NVRAM content.
>
> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
> ---
> V2: Fix comment to match actual function name
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> ---
> drivers/firmware/broadcom/bcm47xx_nvram.c | 111 ++++++++++++----------
> 1 file changed, 63 insertions(+), 48 deletions(-)