Re: [PATCH v5 25/28] mtd: rawnand: hynix: Implement ->choose_data_interface() for H27UCG8T2ATR-BC

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rickard Andersson <rickaran@axis.com>,
	Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Subject: Re: [PATCH v5 25/28] mtd: rawnand: hynix: Implement ->choose_data_interface() for H27UCG8T2ATR-BC
Date: Wed, 27 May 2020 00:06:06 +0200	[thread overview]
Message-ID: <20200527000606.2c26f977@collabora.com> (raw)
In-Reply-To: <20200526191725.7591-26-miquel.raynal@bootlin.com>

On Tue, 26 May 2020 21:17:22 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> This chip supports ONFI SDR timing mode 4, implement the new hook to
> advertize it.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_hynix.c | 16 ++++++++++++++++
>  drivers/mtd/nand/raw/nand_ids.c   |  2 +-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
> index 7d1be53f27f3..d61a55121962 100644
> --- a/drivers/mtd/nand/raw/nand_hynix.c
> +++ b/drivers/mtd/nand/raw/nand_hynix.c
> @@ -673,6 +673,19 @@ static void hynix_nand_cleanup(struct nand_chip *chip)
>  	nand_set_manufacturer_data(chip, NULL);
>  }
>  
> +static int
> +h27ucg8t2atrbc_choose_data_interface(struct nand_chip *chip,
> +				     struct nand_data_interface *iface)
> +{
> +	int ret;
> +
> +	ret = onfi_fill_data_interface(chip, iface, NAND_SDR_IFACE, 4);
> +	if (ret)
> +		return ret;
> +
> +	return nand_choose_best_sdr_timings(chip, iface, NULL);
> +}
> +
>  static int hynix_nand_init(struct nand_chip *chip)
>  {
>  	struct hynix_nand *hynix;
> @@ -689,6 +702,9 @@ static int hynix_nand_init(struct nand_chip *chip)
>  
>  	nand_set_manufacturer_data(chip, hynix);
>  
> +	if (!strcmp("H27UCG8T2ATR-BC", chip->parameters.model))
> +		chip->ops.choose_data_interface = h27ucg8t2atrbc_choose_data_interface;

Same here. You either have to compare against "H27UCG8T2ATR-BC 64G 3.3V
8-bit" or use strncmp. This being said, I wonder if comparing the ID is
not simpler.

> +
>  	ret = hynix_nand_rr_init(chip);
>  	if (ret)
>  		hynix_nand_cleanup(chip);
> diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
> index c729a8bc895d..3b890d55703d 100644
> --- a/drivers/mtd/nand/raw/nand_ids.c
> +++ b/drivers/mtd/nand/raw/nand_ids.c
> @@ -50,7 +50,7 @@ struct nand_flash_dev nand_flash_ids[] = {
>  	{"H27UCG8T2ATR-BC 64G 3.3V 8-bit",
>  		{ .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
>  		  SZ_8K, SZ_8K, SZ_2M, NAND_NEED_SCRAMBLING, 6, 640,
> -		  NAND_ECC_INFO(40, SZ_1K), 4 },
> +		  NAND_ECC_INFO(40, SZ_1K) },
>  

Ultimately we should have those vendor specific entries defined in each
vendor driver rather than declared here (not asking that for this
series though).

>  	LEGACY_ID_NAND("NAND 4MiB 5V 8-bit",   0x6B, 4, SZ_8K, SP_OPTIONS),
>  	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/