Re: [PATCH V2] MIPS: BCM47XX: Make ssb init NVRAM instead of bcm47xx polling it

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

On 09/03/2014 01:34 PM, Rafał Miłecki wrote:
> This makes NVRAM code less bcm47xx/ssb specific allowing it to become a
> standalone driver in the future. A similar patch for bcma will follow
> when it's ready.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
> This patch depends on
> [PATCH] MIPS: BCM47XX: Get rid of calls to KSEG1ADDR in nvram
> 
> V2: Typo in commit message s/bcma/ssb/
> ---
>  arch/mips/bcm47xx/nvram.c     | 30 +++++++++---------------------
>  drivers/ssb/driver_mipscore.c | 18 +++++++++++++++++-
>  2 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/mips/bcm47xx/nvram.c b/arch/mips/bcm47xx/nvram.c
> index 2f0a646..8ea2116 100644
> --- a/arch/mips/bcm47xx/nvram.c
> +++ b/arch/mips/bcm47xx/nvram.c
> @@ -98,7 +98,14 @@ found:
>  	return 0;
>  }
>  
> -static int bcm47xx_nvram_init_from_mem(u32 base, u32 lim)
> +/*
> + * On bcm47xx we need access to the NVRAM very early, so we can't use mtd
> + * subsystem to access flash. We can't even use platform device / driver to
> + * store memory offset.
> + * To handle this we provide following symbol. It's supposed to be called as
> + * soon as we get info about flash device, before any NVRAM entry is needed.
> + */
> +int bcm47xx_nvram_init_from_mem(u32 base, u32 lim)
>  {
>  	void __iomem *iobase;
>  
> @@ -109,25 +116,6 @@ static int bcm47xx_nvram_init_from_mem(u32 base, u32 lim)
>  	return nvram_find_and_copy(iobase, lim);
>  }
>  
> -#ifdef CONFIG_BCM47XX_SSB
> -static int nvram_init_ssb(void)
> -{
> -	struct ssb_mipscore *mcore = &bcm47xx_bus.ssb.mipscore;
> -	u32 base;
> -	u32 lim;
> -
> -	if (mcore->pflash.present) {
> -		base = mcore->pflash.window;
> -		lim = mcore->pflash.window_size;
> -	} else {
> -		pr_err("Couldn't find supported flash memory\n");
> -		return -ENXIO;
> -	}
> -
> -	return bcm47xx_nvram_init_from_mem(base, lim);
> -}
> -#endif
> -
>  #ifdef CONFIG_BCM47XX_BCMA
>  static int nvram_init_bcma(void)
>  {
> @@ -163,7 +151,7 @@ static int nvram_init(void)
>  	switch (bcm47xx_bus_type) {
>  #ifdef CONFIG_BCM47XX_SSB
>  	case BCM47XX_BUS_TYPE_SSB:
> -		return nvram_init_ssb();
> +		break;
>  #endif
>  #ifdef CONFIG_BCM47XX_BCMA
>  	case BCM47XX_BUS_TYPE_BCMA:
> diff --git a/drivers/ssb/driver_mipscore.c b/drivers/ssb/driver_mipscore.c
> index 0907706..c51802f 100644
> --- a/drivers/ssb/driver_mipscore.c
> +++ b/drivers/ssb/driver_mipscore.c
> @@ -207,9 +207,17 @@ static void ssb_mips_serial_init(struct ssb_mipscore *mcore)
>  		mcore->nr_serial_ports = 0;
>  }
>  
> +/* bcm47xx_nvram isn't a separated driver yet and doesn't have its own header.
> + * Once we make it a standalone driver, remove following extern!
> + */
> +#ifdef CONFIG_BCM47XX
> +extern int bcm47xx_nvram_init_from_mem(u32 base, u32 lim);
> +#endif
> +

I do not like forward declarations, could you add this to the nvram
header file and make this file include it. When we move the nvram header
file to /include/linux/... we can provide an empty implementation and
get rid of all these ugly ifdef.

>  static void ssb_mips_flash_detect(struct ssb_mipscore *mcore)
>  {
>  	struct ssb_bus *bus = mcore->dev->bus;
> +	struct ssb_sflash *sflash = &mcore->sflash;
>  	struct ssb_pflash *pflash = &mcore->pflash;
>  
>  	/* When there is no chipcommon on the bus there is 4MB flash */
> @@ -242,7 +250,15 @@ static void ssb_mips_flash_detect(struct ssb_mipscore *mcore)
>  	}
>  
>  ssb_pflash:
> -	if (pflash->present) {
> +	if (sflash->present) {
> +#ifdef CONFIG_BCM47XX
> +		bcm47xx_nvram_init_from_mem(sflash->window, sflash->size);
> +#endif
> +	} else if (pflash->present) {
> +#ifdef CONFIG_BCM47XX
> +		bcm47xx_nvram_init_from_mem(pflash->window, pflash->window_size);
> +#endif
> +
>  		ssb_pflash_data.width = pflash->buswidth;
>  		ssb_pflash_resource.start = pflash->window;
>  		ssb_pflash_resource.end = pflash->window + pflash->window_size;
>