Re: [PATCH] MIPS: BCM47XX: Move NVRAM driver to the drivers/misc/

[Hauke Mehrtens <hauke@hauke-m.de>]

On 11/11/2014 08:45 PM, Rafał Miłecki wrote:
> After Broadcom switched from MIPS to ARM for their home routers we need
> to have NVRAM driver in some common place (not arch/mips/).
> We were thinking about putting it in bus directory, however there are
> two possible buses for MIPS: drivers/ssb/ and drivers/bcma/. So this
> won't fit there neither.
> This is why I would like to move this driver to the drivers/misc/

I will do a more detailed review when you send a patch with -M

> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  arch/mips/Kconfig                                  |   1 +
>  arch/mips/bcm47xx/Makefile                         |   2 +-
>  arch/mips/bcm47xx/board.c                          |   2 +-
>  arch/mips/bcm47xx/nvram.c                          | 228 --------------------
>  arch/mips/bcm47xx/setup.c                          |   1 -
>  arch/mips/bcm47xx/sprom.c                          |   1 -
>  arch/mips/bcm47xx/time.c                           |   1 -
>  arch/mips/include/asm/mach-bcm47xx/bcm47xx.h       |   1 +
>  arch/mips/include/asm/mach-bcm47xx/bcm47xx_nvram.h |  21 --
>  drivers/bcma/driver_mips.c                         |   2 +-
>  drivers/misc/Kconfig                               |   9 +
>  drivers/misc/Makefile                              |   1 +
>  drivers/misc/bcm47xx_nvram.c                       | 230 +++++++++++++++++++++
>  drivers/net/ethernet/broadcom/b44.c                |   2 +-
>  drivers/net/ethernet/broadcom/bgmac.c              |   2 +-
>  drivers/ssb/driver_chipcommon_pmu.c                |   2 +-
>  drivers/ssb/driver_mipscore.c                      |   2 +-
>  include/linux/bcm47xx_nvram.h                      |  18 ++
>  18 files changed, 267 insertions(+), 259 deletions(-)
>  delete mode 100644 arch/mips/bcm47xx/nvram.c
>  delete mode 100644 arch/mips/include/asm/mach-bcm47xx/bcm47xx_nvram.h
>  create mode 100644 drivers/misc/bcm47xx_nvram.c
>  create mode 100644 include/linux/bcm47xx_nvram.h

....

> diff --git a/include/linux/bcm47xx_nvram.h b/include/linux/bcm47xx_nvram.h
> new file mode 100644
> index 0000000..5ed6917
> --- /dev/null
> +++ b/include/linux/bcm47xx_nvram.h
> @@ -0,0 +1,18 @@
> +/*
> + *  This program is free software; you can redistribute  it and/or modify it
> + *  under  the terms of  the GNU General  Public License as published by the
> + *  Free Software Foundation;  either version 2 of the  License, or (at your
> + *  option) any later version.
> + */
> +
> +#ifndef __BCM47XX_NVRAM_H
> +#define __BCM47XX_NVRAM_H
> +
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +
> +int bcm47xx_nvram_init_from_mem(u32 base, u32 lim);
> +int bcm47xx_nvram_getenv(const char *name, char *val, size_t val_len);
> +int bcm47xx_nvram_gpio_pin(const char *name);

Could you change this to something like this:

#ifdef CONFIG_BCM47XX_NVRAM
int bcm47xx_nvram_init_from_mem(u32 base, u32 lim);
int bcm47xx_nvram_getenv(const char *name, char *val, size_t val_len);
int bcm47xx_nvram_gpio_pin(const char *name);
#else
static inline int bcm47xx_nvram_init_from_mem(u32 base, u32 lim) {return
-1;};
static inline int bcm47xx_nvram_getenv(const char *name, char *val,
size_t val_len) {return -1;};
static inline int bcm47xx_nvram_gpio_pin(const char *name) {return -1;};
#endif

and use something better than -1.

This way we can get rid of these  all other the code.
#ifdef CONFIG_BCM47XX
..
#endif

If this is planed in the next patch I would be fine with that.

> +
> +#endif /* __BCM47XX_NVRAM_H */
>