[PATCH v3 0/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[PATCH v3 0/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Tokunori Ikegami]

As pointed out by this bug report [1], the buffered write is now broken on
S29GL064N. The reason is that changed the buffered write to use chip_good
instead of chip_ready. One way to solve the issue is to revert the change
partially to use chip_ready for S29GL064N since the way of least surprise.

[1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/

Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
Cc: stable@vger.kernel.org

Tokunori Ikegami (3):
  mtd: cfi_cmdset_0002: Add S29GL064N ID definition
  mtd: cfi_cmdset_0002: Move and rename
    chip_check/chip_ready/chip_good_for_write
  mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

 drivers/mtd/chips/cfi_cmdset_0002.c | 89 +++++++++++++++--------------
 1 file changed, 47 insertions(+), 42 deletions(-)

-- 
2.32.0

[PATCH v3 0/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Tokunori Ikegami]

As pointed out by this bug report [1], the buffered write is now broken on
S29GL064N. The reason is that changed the buffered write to use chip_good
instead of chip_ready. One way to solve the issue is to revert the change
partially to use chip_ready for S29GL064N since the way of least surprise.

[1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/

Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
Cc: stable@vger.kernel.org

Tokunori Ikegami (3):
  mtd: cfi_cmdset_0002: Add S29GL064N ID definition
  mtd: cfi_cmdset_0002: Move and rename
    chip_check/chip_ready/chip_good_for_write
  mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

 drivers/mtd/chips/cfi_cmdset_0002.c | 89 +++++++++++++++--------------
 1 file changed, 47 insertions(+), 42 deletions(-)

-- 
2.32.0


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

[PATCH v3 1/3] mtd: cfi_cmdset_0002: Add S29GL064N ID definition

[Tokunori Ikegami]

It is for the model number 01, 02, V1 and V2.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a761134fd3be..0125658a1d30 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -48,6 +48,7 @@
 #define SST49LF040B		0x0050
 #define SST49LF008A		0x005a
 #define AT49BV6416		0x00d6
+#define S29GL064N_MN12		0x0c01
 
 /*
  * Status Register bit description. Used by flash devices that don't
@@ -462,7 +463,7 @@ static struct cfi_fixup cfi_fixup_table[] = {
 	{ CFI_MFR_AMD, 0x0056, fixup_use_secsi },
 	{ CFI_MFR_AMD, 0x005C, fixup_use_secsi },
 	{ CFI_MFR_AMD, 0x005F, fixup_use_secsi },
-	{ CFI_MFR_AMD, 0x0c01, fixup_s29gl064n_sectors },
+	{ CFI_MFR_AMD, S29GL064N_MN12, fixup_s29gl064n_sectors },
 	{ CFI_MFR_AMD, 0x1301, fixup_s29gl064n_sectors },
 	{ CFI_MFR_AMD, 0x1a00, fixup_s29gl032n_sectors },
 	{ CFI_MFR_AMD, 0x1a01, fixup_s29gl032n_sectors },
-- 
2.32.0


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

Re: [PATCH v3 1/3] mtd: cfi_cmdset_0002: Add S29GL064N ID definition

[Miquel Raynal]

Hi Tokunori,

ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:05 +0900:

> It is for the model number 01, 02, V1 and V2.

Please move this change to the end of the series. We do not need to
backport this improvement.

Also use the hexadecimal value in your fix (patch 3/3) so that we can
apply it easily to older kernels.

> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: linux-mtd@lists.infradead.org
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index a761134fd3be..0125658a1d30 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -48,6 +48,7 @@
>  #define SST49LF040B		0x0050
>  #define SST49LF008A		0x005a
>  #define AT49BV6416		0x00d6
> +#define S29GL064N_MN12		0x0c01
>  
>  /*
>   * Status Register bit description. Used by flash devices that don't
> @@ -462,7 +463,7 @@ static struct cfi_fixup cfi_fixup_table[] = {
>  	{ CFI_MFR_AMD, 0x0056, fixup_use_secsi },
>  	{ CFI_MFR_AMD, 0x005C, fixup_use_secsi },
>  	{ CFI_MFR_AMD, 0x005F, fixup_use_secsi },
> -	{ CFI_MFR_AMD, 0x0c01, fixup_s29gl064n_sectors },
> +	{ CFI_MFR_AMD, S29GL064N_MN12, fixup_s29gl064n_sectors },
>  	{ CFI_MFR_AMD, 0x1301, fixup_s29gl064n_sectors },
>  	{ CFI_MFR_AMD, 0x1a00, fixup_s29gl032n_sectors },
>  	{ CFI_MFR_AMD, 0x1a01, fixup_s29gl032n_sectors },


Thanks,
Miquèl

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

Re: [PATCH v3 1/3] mtd: cfi_cmdset_0002: Add S29GL064N ID definition

[Tokunori Ikegami]

Hi,

On 2022/03/16 3:37, Miquel Raynal wrote:
> Hi Tokunori,
>
> ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:05 +0900:
>
>> It is for the model number 01, 02, V1 and V2.
> Please move this change to the end of the series. We do not need to
> backport this improvement.
>
> Also use the hexadecimal value in your fix (patch 3/3) so that we can
> apply it easily to older kernels.

Fixed by the version 4 patches.

Regards,
Ikegami

>
>> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>> Cc: linux-mtd@lists.infradead.org
>> ---
>>   drivers/mtd/chips/cfi_cmdset_0002.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
>> index a761134fd3be..0125658a1d30 100644
>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>> @@ -48,6 +48,7 @@
>>   #define SST49LF040B		0x0050
>>   #define SST49LF008A		0x005a
>>   #define AT49BV6416		0x00d6
>> +#define S29GL064N_MN12		0x0c01
>>   
>>   /*
>>    * Status Register bit description. Used by flash devices that don't
>> @@ -462,7 +463,7 @@ static struct cfi_fixup cfi_fixup_table[] = {
>>   	{ CFI_MFR_AMD, 0x0056, fixup_use_secsi },
>>   	{ CFI_MFR_AMD, 0x005C, fixup_use_secsi },
>>   	{ CFI_MFR_AMD, 0x005F, fixup_use_secsi },
>> -	{ CFI_MFR_AMD, 0x0c01, fixup_s29gl064n_sectors },
>> +	{ CFI_MFR_AMD, S29GL064N_MN12, fixup_s29gl064n_sectors },
>>   	{ CFI_MFR_AMD, 0x1301, fixup_s29gl064n_sectors },
>>   	{ CFI_MFR_AMD, 0x1a00, fixup_s29gl032n_sectors },
>>   	{ CFI_MFR_AMD, 0x1a01, fixup_s29gl032n_sectors },
>
> Thanks,
> Miquèl

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

[PATCH v3 2/3] mtd: cfi_cmdset_0002: Move and rename chip_check/chip_ready/chip_good_for_write

[Tokunori Ikegami]

This is a preparation patch for the functinal change to fix the issue.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 82 +++++++++++++----------------
 1 file changed, 38 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 0125658a1d30..8f3f0309dc03 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -802,22 +802,12 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
 	return NULL;
 }
 
-/*
- * Return true if the chip is ready.
- *
- * Ready is one of: read mode, query mode, erase-suspend-read mode (in any
- * non-suspended sector) and is indicated by no toggle bits toggling.
- *
- * Note that anything more complicated than checking if no bits are toggling
- * (including checking DQ5 for an error status) is tricky to get working
- * correctly and is therefore not done	(particularly with interleaved chips
- * as each chip must be checked independently of the others).
- */
-static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
-			       unsigned long addr)
+static int __xipram chip_check(struct map_info *map, struct flchip *chip,
+			       unsigned long addr, map_word *expected)
 {
 	struct cfi_private *cfi = map->fldrv_priv;
-	map_word d, t;
+	map_word oldd, curd;
+	int ret;
 
 	if (cfi_use_status_reg(cfi)) {
 		map_word ready = CMD(CFI_SR_DRB);
@@ -827,17 +817,35 @@ static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
 		 */
 		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
 				 cfi->device_type, NULL);
-		d = map_read(map, addr);
+		curd = map_read(map, addr);
 
-		return map_word_andequal(map, d, ready, ready);
+		return map_word_andequal(map, curd, ready, ready);
 	}
 
-	d = map_read(map, addr);
-	t = map_read(map, addr);
+	oldd = map_read(map, addr);
+	curd = map_read(map, addr);
+
+	ret = map_word_equal(map, oldd, curd);
 
-	return map_word_equal(map, d, t);
+	if (!ret || !expected)
+		return ret;
+
+	return map_word_equal(map, curd, *expected);
 }
 
+/*
+ * Return true if the chip is ready.
+ *
+ * Ready is one of: read mode, query mode, erase-suspend-read mode (in any
+ * non-suspended sector) and is indicated by no toggle bits toggling.
+ *
+ * Note that anything more complicated than checking if no bits are toggling
+ * (including checking DQ5 for an error status) is tricky to get working
+ * correctly and is therefore not done	(particularly with interleaved chips
+ * as each chip must be checked independently of the others).
+ */
+#define chip_ready(map, chip, addr) chip_check(map, chip, addr, NULL)
+
 /*
  * Return true if the chip is ready and has the correct value.
  *
@@ -856,28 +864,14 @@ static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
 static int __xipram chip_good(struct map_info *map, struct flchip *chip,
 			      unsigned long addr, map_word expected)
 {
-	struct cfi_private *cfi = map->fldrv_priv;
-	map_word oldd, curd;
-
-	if (cfi_use_status_reg(cfi)) {
-		map_word ready = CMD(CFI_SR_DRB);
-
-		/*
-		 * For chips that support status register, check device
-		 * ready bit
-		 */
-		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
-				 cfi->device_type, NULL);
-		curd = map_read(map, addr);
-
-		return map_word_andequal(map, curd, ready, ready);
-	}
-
-	oldd = map_read(map, addr);
-	curd = map_read(map, addr);
+	return chip_check(map, chip, addr, &expected);
+}
 
-	return	map_word_equal(map, oldd, curd) &&
-		map_word_equal(map, curd, expected);
+static int __xipram chip_good_for_write(struct map_info *map,
+					struct flchip *chip, unsigned long addr,
+					map_word expected)
+{
+	return chip_good(map, chip, addr, expected);
 }
 
 static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr, int mode)
@@ -1700,7 +1694,7 @@ static int __xipram do_write_oneword_once(struct map_info *map,
 		 * "chip_good" to avoid the failure due to scheduling.
 		 */
 		if (time_after(jiffies, timeo) &&
-		    !chip_good(map, chip, adr, datum)) {
+		    !chip_good_for_write(map, chip, adr, datum)) {
 			xip_enable(map, chip, adr);
 			printk(KERN_WARNING "MTD %s(): software timeout\n", __func__);
 			xip_disable(map, chip, adr);
@@ -1708,7 +1702,7 @@ static int __xipram do_write_oneword_once(struct map_info *map,
 			break;
 		}
 
-		if (chip_good(map, chip, adr, datum)) {
+		if (chip_good_for_write(map, chip, adr, datum)) {
 			if (cfi_check_err_status(map, chip, adr))
 				ret = -EIO;
 			break;
@@ -1980,14 +1974,14 @@ static int __xipram do_write_buffer_wait(struct map_info *map,
 		 * "chip_good" to avoid the failure due to scheduling.
 		 */
 		if (time_after(jiffies, timeo) &&
-		    !chip_good(map, chip, adr, datum)) {
+		    !chip_good_for_write(map, chip, adr, datum)) {
 			pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
 			       __func__, adr);
 			ret = -EIO;
 			break;
 		}
 
-		if (chip_good(map, chip, adr, datum)) {
+		if (chip_good_for_write(map, chip, adr, datum)) {
 			if (cfi_check_err_status(map, chip, adr))
 				ret = -EIO;
 			break;
-- 
2.32.0


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

Re: [PATCH v3 2/3] mtd: cfi_cmdset_0002: Move and rename chip_check/chip_ready/chip_good_for_write

[Miquel Raynal]

Hi Tokunori,

ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:06 +0900:

> This is a preparation patch for the functinal change to fix the issue.

				      functional

Commits are independent "to fix the issue" does not make any sense
here, please give more information like "in preparation to a change
expected to fix the buffered writes on S29GL..."

> 
> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: linux-mtd@lists.infradead.org
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 82 +++++++++++++----------------
>  1 file changed, 38 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 0125658a1d30..8f3f0309dc03 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -802,22 +802,12 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
>  	return NULL;
>  }
>  
> -/*
> - * Return true if the chip is ready.
> - *
> - * Ready is one of: read mode, query mode, erase-suspend-read mode (in any
> - * non-suspended sector) and is indicated by no toggle bits toggling.
> - *
> - * Note that anything more complicated than checking if no bits are toggling
> - * (including checking DQ5 for an error status) is tricky to get working
> - * correctly and is therefore not done	(particularly with interleaved chips
> - * as each chip must be checked independently of the others).
> - */
> -static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
> -			       unsigned long addr)
> +static int __xipram chip_check(struct map_info *map, struct flchip *chip,
> +			       unsigned long addr, map_word *expected)
>  {
>  	struct cfi_private *cfi = map->fldrv_priv;
> -	map_word d, t;
> +	map_word oldd, curd;
> +	int ret;
>  
>  	if (cfi_use_status_reg(cfi)) {
>  		map_word ready = CMD(CFI_SR_DRB);
> @@ -827,17 +817,35 @@ static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
>  		 */
>  		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
>  				 cfi->device_type, NULL);
> -		d = map_read(map, addr);
> +		curd = map_read(map, addr);
>  
> -		return map_word_andequal(map, d, ready, ready);
> +		return map_word_andequal(map, curd, ready, ready);
>  	}
>  
> -	d = map_read(map, addr);
> -	t = map_read(map, addr);
> +	oldd = map_read(map, addr);
> +	curd = map_read(map, addr);
> +
> +	ret = map_word_equal(map, oldd, curd);
>  
> -	return map_word_equal(map, d, t);
> +	if (!ret || !expected)
> +		return ret;
> +
> +	return map_word_equal(map, curd, *expected);
>  }
>  
> +/*
> + * Return true if the chip is ready.
> + *
> + * Ready is one of: read mode, query mode, erase-suspend-read mode (in any
> + * non-suspended sector) and is indicated by no toggle bits toggling.
> + *
> + * Note that anything more complicated than checking if no bits are toggling
> + * (including checking DQ5 for an error status) is tricky to get working
> + * correctly and is therefore not done	(particularly with interleaved chips
> + * as each chip must be checked independently of the others).
> + */
> +#define chip_ready(map, chip, addr) chip_check(map, chip, addr, NULL)
> +
>  /*
>   * Return true if the chip is ready and has the correct value.
>   *
> @@ -856,28 +864,14 @@ static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
>  static int __xipram chip_good(struct map_info *map, struct flchip *chip,
>  			      unsigned long addr, map_word expected)
>  {
> -	struct cfi_private *cfi = map->fldrv_priv;
> -	map_word oldd, curd;
> -
> -	if (cfi_use_status_reg(cfi)) {
> -		map_word ready = CMD(CFI_SR_DRB);
> -
> -		/*
> -		 * For chips that support status register, check device
> -		 * ready bit
> -		 */
> -		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
> -				 cfi->device_type, NULL);
> -		curd = map_read(map, addr);
> -
> -		return map_word_andequal(map, curd, ready, ready);
> -	}
> -
> -	oldd = map_read(map, addr);
> -	curd = map_read(map, addr);
> +	return chip_check(map, chip, addr, &expected);

I must admit that chip_check, chip_good(), chip_ready() are rather poor
names, at least they don't bring a lot of information. Is this part of a
revert? If yes maybe it would be better to actually revert the patch if
it not too complicated? Otherwise I don't really see the point of
having chip_good() calling just chip_check()? And creating a macro for
chip_ready()?

> +}
>  
> -	return	map_word_equal(map, oldd, curd) &&
> -		map_word_equal(map, curd, expected);
> +static int __xipram chip_good_for_write(struct map_info *map,
> +					struct flchip *chip, unsigned long addr,
> +					map_word expected)
> +{
> +	return chip_good(map, chip, addr, expected);
>  }
>  
>  static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr, int mode)
> @@ -1700,7 +1694,7 @@ static int __xipram do_write_oneword_once(struct map_info *map,
>  		 * "chip_good" to avoid the failure due to scheduling.
>  		 */
>  		if (time_after(jiffies, timeo) &&
> -		    !chip_good(map, chip, adr, datum)) {
> +		    !chip_good_for_write(map, chip, adr, datum)) {
>  			xip_enable(map, chip, adr);
>  			printk(KERN_WARNING "MTD %s(): software timeout\n", __func__);
>  			xip_disable(map, chip, adr);
> @@ -1708,7 +1702,7 @@ static int __xipram do_write_oneword_once(struct map_info *map,
>  			break;
>  		}
>  
> -		if (chip_good(map, chip, adr, datum)) {
> +		if (chip_good_for_write(map, chip, adr, datum)) {
>  			if (cfi_check_err_status(map, chip, adr))
>  				ret = -EIO;
>  			break;
> @@ -1980,14 +1974,14 @@ static int __xipram do_write_buffer_wait(struct map_info *map,
>  		 * "chip_good" to avoid the failure due to scheduling.
>  		 */
>  		if (time_after(jiffies, timeo) &&
> -		    !chip_good(map, chip, adr, datum)) {
> +		    !chip_good_for_write(map, chip, adr, datum)) {
>  			pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
>  			       __func__, adr);
>  			ret = -EIO;
>  			break;
>  		}
>  
> -		if (chip_good(map, chip, adr, datum)) {
> +		if (chip_good_for_write(map, chip, adr, datum)) {
>  			if (cfi_check_err_status(map, chip, adr))
>  				ret = -EIO;
>  			break;


Thanks,
Miquèl

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

Re: [PATCH v3 2/3] mtd: cfi_cmdset_0002: Move and rename chip_check/chip_ready/chip_good_for_write

[Tokunori Ikegami]

Hi,

On 2022/03/16 3:44, Miquel Raynal wrote:
> Hi Tokunori,
>
> ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:06 +0900:
>
>> This is a preparation patch for the functinal change to fix the issue.
> 				      functional
>
> Commits are independent "to fix the issue" does not make any sense
> here, please give more information like "in preparation to a change
> expected to fix the buffered writes on S29GL..."
Fixed by the version 4 patches.
>
>> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>> Cc: linux-mtd@lists.infradead.org
>> ---
>>   drivers/mtd/chips/cfi_cmdset_0002.c | 82 +++++++++++++----------------
>>   1 file changed, 38 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
>> index 0125658a1d30..8f3f0309dc03 100644
>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>> @@ -802,22 +802,12 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd)
>>   	return NULL;
>>   }
>>   
>> -/*
>> - * Return true if the chip is ready.
>> - *
>> - * Ready is one of: read mode, query mode, erase-suspend-read mode (in any
>> - * non-suspended sector) and is indicated by no toggle bits toggling.
>> - *
>> - * Note that anything more complicated than checking if no bits are toggling
>> - * (including checking DQ5 for an error status) is tricky to get working
>> - * correctly and is therefore not done	(particularly with interleaved chips
>> - * as each chip must be checked independently of the others).
>> - */
>> -static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
>> -			       unsigned long addr)
>> +static int __xipram chip_check(struct map_info *map, struct flchip *chip,
>> +			       unsigned long addr, map_word *expected)
>>   {
>>   	struct cfi_private *cfi = map->fldrv_priv;
>> -	map_word d, t;
>> +	map_word oldd, curd;
>> +	int ret;
>>   
>>   	if (cfi_use_status_reg(cfi)) {
>>   		map_word ready = CMD(CFI_SR_DRB);
>> @@ -827,17 +817,35 @@ static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
>>   		 */
>>   		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
>>   				 cfi->device_type, NULL);
>> -		d = map_read(map, addr);
>> +		curd = map_read(map, addr);
>>   
>> -		return map_word_andequal(map, d, ready, ready);
>> +		return map_word_andequal(map, curd, ready, ready);
>>   	}
>>   
>> -	d = map_read(map, addr);
>> -	t = map_read(map, addr);
>> +	oldd = map_read(map, addr);
>> +	curd = map_read(map, addr);
>> +
>> +	ret = map_word_equal(map, oldd, curd);
>>   
>> -	return map_word_equal(map, d, t);
>> +	if (!ret || !expected)
>> +		return ret;
>> +
>> +	return map_word_equal(map, curd, *expected);
>>   }
>>   
>> +/*
>> + * Return true if the chip is ready.
>> + *
>> + * Ready is one of: read mode, query mode, erase-suspend-read mode (in any
>> + * non-suspended sector) and is indicated by no toggle bits toggling.
>> + *
>> + * Note that anything more complicated than checking if no bits are toggling
>> + * (including checking DQ5 for an error status) is tricky to get working
>> + * correctly and is therefore not done	(particularly with interleaved chips
>> + * as each chip must be checked independently of the others).
>> + */
>> +#define chip_ready(map, chip, addr) chip_check(map, chip, addr, NULL)
>> +
>>   /*
>>    * Return true if the chip is ready and has the correct value.
>>    *
>> @@ -856,28 +864,14 @@ static int __xipram chip_ready(struct map_info *map, struct flchip *chip,
>>   static int __xipram chip_good(struct map_info *map, struct flchip *chip,
>>   			      unsigned long addr, map_word expected)
>>   {
>> -	struct cfi_private *cfi = map->fldrv_priv;
>> -	map_word oldd, curd;
>> -
>> -	if (cfi_use_status_reg(cfi)) {
>> -		map_word ready = CMD(CFI_SR_DRB);
>> -
>> -		/*
>> -		 * For chips that support status register, check device
>> -		 * ready bit
>> -		 */
>> -		cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
>> -				 cfi->device_type, NULL);
>> -		curd = map_read(map, addr);
>> -
>> -		return map_word_andequal(map, curd, ready, ready);
>> -	}
>> -
>> -	oldd = map_read(map, addr);
>> -	curd = map_read(map, addr);
>> +	return chip_check(map, chip, addr, &expected);
> I must admit that chip_check, chip_good(), chip_ready() are rather poor
> names, at least they don't bring a lot of information. Is this part of a
> revert? If yes maybe it would be better to actually revert the patch if
> it not too complicated? Otherwise I don't really see the point of
No this is not for the revert.
> having chip_good() calling just chip_check()? And creating a macro for
> chip_ready()?
Changed chip_good by the version 4 patches as a macro but not a function 
as same with the chip_ready.
>
>> +}
>>   
>> -	return	map_word_equal(map, oldd, curd) &&
>> -		map_word_equal(map, curd, expected);
>> +static int __xipram chip_good_for_write(struct map_info *map,
>> +					struct flchip *chip, unsigned long addr,
>> +					map_word expected)
>> +{
>> +	return chip_good(map, chip, addr, expected);
>>   }
>>   
>>   static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr, int mode)
>> @@ -1700,7 +1694,7 @@ static int __xipram do_write_oneword_once(struct map_info *map,
>>   		 * "chip_good" to avoid the failure due to scheduling.
>>   		 */
>>   		if (time_after(jiffies, timeo) &&
>> -		    !chip_good(map, chip, adr, datum)) {
>> +		    !chip_good_for_write(map, chip, adr, datum)) {
>>   			xip_enable(map, chip, adr);
>>   			printk(KERN_WARNING "MTD %s(): software timeout\n", __func__);
>>   			xip_disable(map, chip, adr);
>> @@ -1708,7 +1702,7 @@ static int __xipram do_write_oneword_once(struct map_info *map,
>>   			break;
>>   		}
>>   
>> -		if (chip_good(map, chip, adr, datum)) {
>> +		if (chip_good_for_write(map, chip, adr, datum)) {
>>   			if (cfi_check_err_status(map, chip, adr))
>>   				ret = -EIO;
>>   			break;
>> @@ -1980,14 +1974,14 @@ static int __xipram do_write_buffer_wait(struct map_info *map,
>>   		 * "chip_good" to avoid the failure due to scheduling.
>>   		 */
>>   		if (time_after(jiffies, timeo) &&
>> -		    !chip_good(map, chip, adr, datum)) {
>> +		    !chip_good_for_write(map, chip, adr, datum)) {
>>   			pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
>>   			       __func__, adr);
>>   			ret = -EIO;
>>   			break;
>>   		}
>>   
>> -		if (chip_good(map, chip, adr, datum)) {
>> +		if (chip_good_for_write(map, chip, adr, datum)) {
>>   			if (cfi_check_err_status(map, chip, adr))
>>   				ret = -EIO;
>>   			break;
>
> Thanks,
> Miquèl

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

[PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Tokunori Ikegami]

As pointed out by this bug report [1], the buffered write is now broken on
S29GL064N. The reason is that changed the buffered write to use chip_good
instead of chip_ready. One way to solve the issue is to revert the change
partially to use chip_ready for S29GL064N since the way of least surprise.

[1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/

Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
Cc: stable@vger.kernel.org
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 8f3f0309dc03..fa11db066c99 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
 	return chip_check(map, chip, addr, &expected);
 }
 
+static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map)
+{
+	struct cfi_private *cfi = map->fldrv_priv;
+
+	return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
+}
+
 static int __xipram chip_good_for_write(struct map_info *map,
 					struct flchip *chip, unsigned long addr,
 					map_word expected)
 {
+	if (cfi_use_chip_ready_for_write(map))
+		return chip_ready(map, chip, addr);
+
 	return chip_good(map, chip, addr, expected);
 }
 
-- 
2.32.0

[PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Tokunori Ikegami]

As pointed out by this bug report [1], the buffered write is now broken on
S29GL064N. The reason is that changed the buffered write to use chip_good
instead of chip_ready. One way to solve the issue is to revert the change
partially to use chip_ready for S29GL064N since the way of least surprise.

[1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/

Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
Cc: stable@vger.kernel.org
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 8f3f0309dc03..fa11db066c99 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
 	return chip_check(map, chip, addr, &expected);
 }
 
+static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map)
+{
+	struct cfi_private *cfi = map->fldrv_priv;
+
+	return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
+}
+
 static int __xipram chip_good_for_write(struct map_info *map,
 					struct flchip *chip, unsigned long addr,
 					map_word expected)
 {
+	if (cfi_use_chip_ready_for_write(map))
+		return chip_ready(map, chip, addr);
+
 	return chip_good(map, chip, addr, expected);
 }
 
-- 
2.32.0


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

Re: [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Miquel Raynal]

Hi Tokunori,

ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:07 +0900:

> As pointed out by this bug report [1], the buffered write is now broken on

                                       , buffered writes are now broken

> S29GL064N. The reason is that changed the buffered write to use chip_good
> instead of chip_ready.

"This issue comes from a rework which switched from using chip_good()
to chip_ready(), because <explain the difference here>."

[please note I am just trying to understand what the root cause is,
please rephrase if I'm wrong].

> One way to solve the issue is to revert the change
> partially to use chip_ready for S29GL064N since the way of least surprise.

s/since the way of least surprise//


> 
> [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
> 
> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: linux-mtd@lists.infradead.org

I think you can get rid of all the above Cc: tags and just copy all 3
of us + the mailing list when sending your v4.

> Cc: stable@vger.kernel.org
> ---

Please also include a Fixes/stable tag in the patch before (2/3) to explain
that both patches are required in order to fix the issue and the current patch alone won't apply.

You should mention that with a nice comment below the three dashes ("---") in patch 2/3 as well.

>  drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 8f3f0309dc03..fa11db066c99 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
>  	return chip_check(map, chip, addr, &expected);
>  }
>  
> +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map)
> +{
> +	struct cfi_private *cfi = map->fldrv_priv;
> +
> +	return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
> +}
> +
>  static int __xipram chip_good_for_write(struct map_info *map,
>  					struct flchip *chip, unsigned long addr,
>  					map_word expected)
>  {
> +	if (cfi_use_chip_ready_for_write(map))
> +		return chip_ready(map, chip, addr);
> +
>  	return chip_good(map, chip, addr, expected);
>  }
>  

This is much more understandable.

Vignesh, perhaps it would be better to provide a way for manufacturers
to overload certain callbacks instead of applying quirks like this in
the code. But that will come in a second time of course.


Thanks,
Miquèl

Re: [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Miquel Raynal]

Hi Tokunori,

ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:07 +0900:

> As pointed out by this bug report [1], the buffered write is now broken on

                                       , buffered writes are now broken

> S29GL064N. The reason is that changed the buffered write to use chip_good
> instead of chip_ready.

"This issue comes from a rework which switched from using chip_good()
to chip_ready(), because <explain the difference here>."

[please note I am just trying to understand what the root cause is,
please rephrase if I'm wrong].

> One way to solve the issue is to revert the change
> partially to use chip_ready for S29GL064N since the way of least surprise.

s/since the way of least surprise//


> 
> [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
> 
> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: linux-mtd@lists.infradead.org

I think you can get rid of all the above Cc: tags and just copy all 3
of us + the mailing list when sending your v4.

> Cc: stable@vger.kernel.org
> ---

Please also include a Fixes/stable tag in the patch before (2/3) to explain
that both patches are required in order to fix the issue and the current patch alone won't apply.

You should mention that with a nice comment below the three dashes ("---") in patch 2/3 as well.

>  drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 8f3f0309dc03..fa11db066c99 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
>  	return chip_check(map, chip, addr, &expected);
>  }
>  
> +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map)
> +{
> +	struct cfi_private *cfi = map->fldrv_priv;
> +
> +	return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
> +}
> +
>  static int __xipram chip_good_for_write(struct map_info *map,
>  					struct flchip *chip, unsigned long addr,
>  					map_word expected)
>  {
> +	if (cfi_use_chip_ready_for_write(map))
> +		return chip_ready(map, chip, addr);
> +
>  	return chip_good(map, chip, addr, expected);
>  }
>  

This is much more understandable.

Vignesh, perhaps it would be better to provide a way for manufacturers
to overload certain callbacks instead of applying quirks like this in
the code. But that will come in a second time of course.


Thanks,
Miquèl

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

Re: [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Tokunori Ikegami]

Hi,

On 2022/03/16 3:51, Miquel Raynal wrote:
> Hi Tokunori,
>
> ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:07 +0900:
>
>> As pointed out by this bug report [1], the buffered write is now broken on
>                                         , buffered writes are now broken
>
>> S29GL064N. The reason is that changed the buffered write to use chip_good
>> instead of chip_ready.
> "This issue comes from a rework which switched from using chip_good()
> to chip_ready(), because <explain the difference here>."
>
> [please note I am just trying to understand what the root cause is,
> please rephrase if I'm wrong].
Fixed by the version 4 patches.
>
>> One way to solve the issue is to revert the change
>> partially to use chip_ready for S29GL064N since the way of least surprise.
> s/since the way of least surprise//
Fixed by the version 4 patches.
>
>
>> [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
>>
>> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
>> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>> Cc: linux-mtd@lists.infradead.org
> I think you can get rid of all the above Cc: tags and just copy all 3
> of us + the mailing list when sending your v4.
Fixed by the version 4 patches.
>
>> Cc: stable@vger.kernel.org
>> ---
> Please also include a Fixes/stable tag in the patch before (2/3) to explain
> that both patches are required in order to fix the issue and the current patch alone won't apply.
>
> You should mention that with a nice comment below the three dashes ("---") in patch 2/3 as well.
>
>>   drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
>> index 8f3f0309dc03..fa11db066c99 100644
>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>> @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
>>   	return chip_check(map, chip, addr, &expected);
>>   }
>>   
>> +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map)
>> +{
>> +	struct cfi_private *cfi = map->fldrv_priv;
>> +
>> +	return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
>> +}
>> +
>>   static int __xipram chip_good_for_write(struct map_info *map,
>>   					struct flchip *chip, unsigned long addr,
>>   					map_word expected)
>>   {
>> +	if (cfi_use_chip_ready_for_write(map))
>> +		return chip_ready(map, chip, addr);
>> +
>>   	return chip_good(map, chip, addr, expected);
>>   }
>>   
> This is much more understandable.
>
> Vignesh, perhaps it would be better to provide a way for manufacturers
> to overload certain callbacks instead of applying quirks like this in
> the code. But that will come in a second time of course.
>
>
> Thanks,
> Miquèl

Re: [PATCH v3 3/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Tokunori Ikegami]

Hi,

On 2022/03/16 3:51, Miquel Raynal wrote:
> Hi Tokunori,
>
> ikegami.t@gmail.com wrote on Wed, 16 Mar 2022 01:56:07 +0900:
>
>> As pointed out by this bug report [1], the buffered write is now broken on
>                                         , buffered writes are now broken
>
>> S29GL064N. The reason is that changed the buffered write to use chip_good
>> instead of chip_ready.
> "This issue comes from a rework which switched from using chip_good()
> to chip_ready(), because <explain the difference here>."
>
> [please note I am just trying to understand what the root cause is,
> please rephrase if I'm wrong].
Fixed by the version 4 patches.
>
>> One way to solve the issue is to revert the change
>> partially to use chip_ready for S29GL064N since the way of least surprise.
> s/since the way of least surprise//
Fixed by the version 4 patches.
>
>
>> [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
>>
>> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
>> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>> Cc: linux-mtd@lists.infradead.org
> I think you can get rid of all the above Cc: tags and just copy all 3
> of us + the mailing list when sending your v4.
Fixed by the version 4 patches.
>
>> Cc: stable@vger.kernel.org
>> ---
> Please also include a Fixes/stable tag in the patch before (2/3) to explain
> that both patches are required in order to fix the issue and the current patch alone won't apply.
>
> You should mention that with a nice comment below the three dashes ("---") in patch 2/3 as well.
>
>>   drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
>> index 8f3f0309dc03..fa11db066c99 100644
>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>> @@ -867,10 +867,20 @@ static int __xipram chip_good(struct map_info *map, struct flchip *chip,
>>   	return chip_check(map, chip, addr, &expected);
>>   }
>>   
>> +static bool __xipram cfi_use_chip_ready_for_write(struct map_info *map)
>> +{
>> +	struct cfi_private *cfi = map->fldrv_priv;
>> +
>> +	return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
>> +}
>> +
>>   static int __xipram chip_good_for_write(struct map_info *map,
>>   					struct flchip *chip, unsigned long addr,
>>   					map_word expected)
>>   {
>> +	if (cfi_use_chip_ready_for_write(map))
>> +		return chip_ready(map, chip, addr);
>> +
>>   	return chip_good(map, chip, addr, expected);
>>   }
>>   
> This is much more understandable.
>
> Vignesh, perhaps it would be better to provide a way for manufacturers
> to overload certain callbacks instead of applying quirks like this in
> the code. But that will come in a second time of course.
>
>
> Thanks,
> Miquèl

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