Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RESEND PATCH v3 06/20] mtd: spi-nor: Add a default_init() fixup
 hook for gd25q256
To:     <Tudor.Ambarus@microchip.com>, <boris.brezillon@collabora.com>,
        <marek.vasut@gmail.com>, <miquel.raynal@bootlin.com>,
        <richard@nod.at>, <linux-mtd@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>
CC:     <boris.brezillon@bootlin.com>
References: <20190826120821.16351-1-tudor.ambarus@microchip.com>
 <20190826120821.16351-7-tudor.ambarus@microchip.com>
From:   Vignesh Raghavendra <vigneshr@ti.com>
Message-ID: <74bfb73e-e1c4-ce78-f1cb-6ee0050eb275@ti.com>
Date:   Tue, 27 Aug 2019 11:18:57 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190826120821.16351-7-tudor.ambarus@microchip.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 26/08/19 5:38 PM, Tudor.Ambarus@microchip.com wrote:
> From: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> gd25q256 needs to tweak the ->quad_enable() implementation and the
> ->default_init() fixup hook is the perfect place to do that. This way,
> if we ever need to tweak more things for this flash, we won't have to
> add new fields in flash_info.
> 
> We can get rid of the flash_info->quad_enable field as gd25q256 was
> the only user.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> [tudor.ambarus@microchip.com: use ->default_init() hook instead of
> ->post_sfdp()]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---


Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> v3: no changes
> >  drivers/mtd/spi-nor/spi-nor.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 8fd60e1eebd2..3dbbfe34d1d2 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -222,8 +222,6 @@ struct flash_info {
>  
>  	/* Part specific fixup hooks. */
>  	const struct spi_nor_fixups *fixups;
> -
> -	int	(*quad_enable)(struct spi_nor *nor);
>  };
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
> @@ -2126,6 +2124,21 @@ static struct spi_nor_fixups mx25l25635_fixups = {
>  	.post_bfpt = mx25l25635_post_bfpt_fixups,
>  };
>  
> +static void gd25q256_default_init(struct spi_nor *nor)
> +{
> +	/*
> +	 * Some manufacturer like GigaDevice may use different
> +	 * bit to set QE on different memories, so the MFR can't
> +	 * indicate the quad_enable method for this case, we need
> +	 * to set it in the default_init fixup hook.
> +	 */
> +	nor->params.quad_enable = macronix_quad_enable;
> +}
> +
> +static struct spi_nor_fixups gd25q256_fixups = {
> +	.default_init = gd25q256_default_init,
> +};
> +
>  /* NOTE: double check command sets and memory organization when you add
>   * more nor chips.  This current list focusses on newer chips, which
>   * have been converging on command sets which including JEDEC ID.
> @@ -2218,7 +2231,7 @@ static const struct flash_info spi_nor_ids[] = {
>  		"gd25q256", INFO(0xc84019, 0, 64 * 1024, 512,
>  			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>  			SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> -			.quad_enable = macronix_quad_enable,
> +			.fixups = &gd25q256_fixups,
>  	},
>  
>  	/* Intel/Numonyx -- xxxs33b */
> @@ -4237,15 +4250,6 @@ static int spi_nor_init_params(struct spi_nor *nor)
>  			params->quad_enable = spansion_quad_enable;
>  			break;
>  		}
> -
> -		/*
> -		 * Some manufacturer like GigaDevice may use different
> -		 * bit to set QE on different memories, so the MFR can't
> -		 * indicate the quad_enable method for this case, we need
> -		 * set it in flash info list.
> -		 */
> -		if (info->quad_enable)
> -			params->quad_enable = info->quad_enable;
>  	}
>  
>  	spi_nor_manufacturer_init_params(nor);
> 

-- 
Regards
Vignesh