Re: [PATCH v3 1/2] mtd: fsl-quadspi: Call fsl_qspi_set_base_addr after nor_size is set

[Brian Norris <computersforpeace@gmail.com>]

On Wed, Jan 07, 2015 at 08:32:06AM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> fsl_qspi_set_base_addr() uses nor_size information, but it is called prior to
> the initialization of nor_size.
> 
> Fix it by calling fsl_qspi_set_base_addr() after nor_size is configured.

This patch doesn't look correct either. But then, the original driver
seems confusing too.

Huang, is this driver assuming that all flashes on this controller will
be the same size? Or maybe I'm not understanding your MMIO layout. But I
notice that 'nor_size' is shared between all NOR instances on this
controller. And for that matter, I don't see any locking. Are you sure
this driver is safe for multiple flash instances?

> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v2:
> - Newly introduced in this version
> 
>  drivers/mtd/spi-nor/fsl-quadspi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 39763b9..20cffd2 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -897,9 +897,6 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto map_failed;
>  
> -		/* set the chip address for READID */
> -		fsl_qspi_set_base_addr(q, nor);
> -
>  		ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
>  		if (ret)
>  			goto map_failed;
> @@ -917,6 +914,9 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>  			fsl_qspi_set_map_addr(q);
>  		}
>  
> +		/* set the chip address for READID */

OK, so this is to get READID correct... but you're doing this after
READID. So is this for configuring the *next* flash? I'm confused.

> +		fsl_qspi_set_base_addr(q, nor);
> +
>  		/*
>  		 * The TX FIFO is 64 bytes in the Vybrid, but the Page Program
>  		 * may writes 265 bytes per time. The write is working in the

I don't think I want to take any of these patches until I get a clearer
picture of who/what/how you're testing these, and I get an ack from
Huang (or someone else who is going to be the de factor / official
maintianer of this driver, since I note that Huang is no longer with
Freescale).

BTW, do we want a MAINTAINERS entry for this driver?

Brian