On 12 November 2014 at 22:16, Hauke Mehrtens <hauke@xxxxxxxxxx> wrote:
> 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@xxxxxxxxx>
>> ---
>> 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
How many drivers using
#ifdef CONFIG_BCM47XX
bcm47xx_nvram_foo(...)
#endif
do we have?
I think right now it is done in
drivers/bcma/driver_mips.c
but should be dropped anyway. We should make BCMA_DRIVER_MIPS depend on BCM47XX.
And second usage is in:
drivers/net/ethernet/broadcom/b44.c
Do you think it's worth doing just for b44.c?
--
Rafał
