* [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
@ 2022-03-06 15:32 ` Tokunori Ikegami
0 siblings, 0 replies; 7+ messages in thread
From: Tokunori Ikegami @ 2022-03-06 15:32 UTC (permalink / raw)
To: vigneshr
Cc: linux-mtd, Tokunori Ikegami, Miquel Raynal, Richard Weinberger, stable
The regression issue has been caused on S29GL064N and reported it.
Also the change mentioned is to use chip_good() for buffered write.
So disable the change on S29GL064N and use chip_ready() as before.
Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
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
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
---
drivers/mtd/chips/cfi_cmdset_0002.c | 89 +++++++++++++++--------------
1 file changed, 47 insertions(+), 42 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a761134fd3be..5e14b60e8638 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 },
@@ -801,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);
@@ -826,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.
*
@@ -855,28 +864,24 @@ 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);
+ return chip_check(map, chip, addr, &expected);
+}
- /*
- * 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);
+static bool cfi_use_chip_ready_for_write(struct map_info *map)
+{
+ struct cfi_private *cfi = map->fldrv_priv;
- return map_word_andequal(map, curd, ready, ready);
- }
+ return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
+}
- oldd = map_read(map, addr);
- curd = map_read(map, addr);
+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 map_word_equal(map, oldd, curd) &&
- map_word_equal(map, curd, expected);
+ return chip_good(map, chip, addr, expected);
}
static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr, int mode)
@@ -1699,7 +1704,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);
@@ -1707,7 +1712,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;
@@ -1979,14 +1984,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
@ 2022-03-06 15:32 ` Tokunori Ikegami
0 siblings, 0 replies; 7+ messages in thread
From: Tokunori Ikegami @ 2022-03-06 15:32 UTC (permalink / raw)
To: vigneshr
Cc: linux-mtd, Tokunori Ikegami, Miquel Raynal, Richard Weinberger, stable
The regression issue has been caused on S29GL064N and reported it.
Also the change mentioned is to use chip_good() for buffered write.
So disable the change on S29GL064N and use chip_ready() as before.
Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
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
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
---
drivers/mtd/chips/cfi_cmdset_0002.c | 89 +++++++++++++++--------------
1 file changed, 47 insertions(+), 42 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a761134fd3be..5e14b60e8638 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 },
@@ -801,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);
@@ -826,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.
*
@@ -855,28 +864,24 @@ 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);
+ return chip_check(map, chip, addr, &expected);
+}
- /*
- * 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);
+static bool cfi_use_chip_ready_for_write(struct map_info *map)
+{
+ struct cfi_private *cfi = map->fldrv_priv;
- return map_word_andequal(map, curd, ready, ready);
- }
+ return cfi->mfr == CFI_MFR_AMD && cfi->id == S29GL064N_MN12;
+}
- oldd = map_read(map, addr);
- curd = map_read(map, addr);
+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 map_word_equal(map, oldd, curd) &&
- map_word_equal(map, curd, expected);
+ return chip_good(map, chip, addr, expected);
}
static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr, int mode)
@@ -1699,7 +1704,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);
@@ -1707,7 +1712,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;
@@ -1979,14 +1984,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/
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
2022-03-06 15:32 ` Tokunori Ikegami
(?)
@ 2022-03-09 8:17 ` Vignesh Raghavendra
2022-03-09 10:29 ` Ahmad Fatoum
-1 siblings, 1 reply; 7+ messages in thread
From: Vignesh Raghavendra @ 2022-03-09 8:17 UTC (permalink / raw)
To: Tokunori Ikegami, Ahmad Fatoum
Cc: linux-mtd, Miquel Raynal, Richard Weinberger
+ Ahmad (who reported the bug)
Hi Ahmad,
On 06/03/22 9:02 pm, Tokunori Ikegami wrote:
> The regression issue has been caused on S29GL064N and reported it.
> Also the change mentioned is to use chip_good() for buffered write.
> So disable the change on S29GL064N and use chip_ready() as before.
>
> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
> 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
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
Could you please test this exact patch and provide Tested-by so that it
gets registered by patch tracking systems?
[...]
Regards
Vignesh
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
2022-03-09 8:17 ` Vignesh Raghavendra
@ 2022-03-09 10:29 ` Ahmad Fatoum
0 siblings, 0 replies; 7+ messages in thread
From: Ahmad Fatoum @ 2022-03-09 10:29 UTC (permalink / raw)
To: Vignesh Raghavendra, Tokunori Ikegami
Cc: linux-mtd, Miquel Raynal, Richard Weinberger, Pengutronix Kernel Team
Hello Vignesh,
On 09.03.22 09:17, Vignesh Raghavendra wrote:
> + Ahmad (who reported the bug)
Thanks for Cc'ing.
> Hi Ahmad,
>
> On 06/03/22 9:02 pm, Tokunori Ikegami wrote:
>> The regression issue has been caused on S29GL064N and reported it.
>> Also the change mentioned is to use chip_good() for buffered write.
>> So disable the change on S29GL064N and use chip_ready() as before.
>>
>> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>> 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
>> Cc: stable@vger.kernel.org
>> Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
>
> Could you please test this exact patch and provide Tested-by so that it
> gets registered by patch tracking systems?
Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Thanks,
Ahmad
>
> [...]
>
> Regards
> Vignesh
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
2022-03-06 15:32 ` Tokunori Ikegami
(?)
(?)
@ 2022-03-15 6:11 ` Vignesh Raghavendra
2022-03-15 9:50 ` Miquel Raynal
-1 siblings, 1 reply; 7+ messages in thread
From: Vignesh Raghavendra @ 2022-03-15 6:11 UTC (permalink / raw)
To: Tokunori Ikegami; +Cc: linux-mtd, Miquel Raynal, Richard Weinberger
Hi Miquel,
On 06/03/22 9:02 pm, Tokunori Ikegami wrote:
> The regression issue has been caused on S29GL064N and reported it.
> Also the change mentioned is to use chip_good() for buffered write.
> So disable the change on S29GL064N and use chip_ready() as before.
>
> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
> 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
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
Could you please pull this in for v5.18-rc1 as this is a regression
being tracked for a while?
Acked-by: Vignesh Raghavendra <vigneshr@ti.com>
[...]
Regards
Vignesh
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
2022-03-15 6:11 ` Vignesh Raghavendra
@ 2022-03-15 9:50 ` Miquel Raynal
2022-03-15 17:08 ` Tokunori Ikegami
0 siblings, 1 reply; 7+ messages in thread
From: Miquel Raynal @ 2022-03-15 9:50 UTC (permalink / raw)
To: Vignesh Raghavendra; +Cc: Tokunori Ikegami, linux-mtd, Richard Weinberger
Hello,
vigneshr@ti.com wrote on Tue, 15 Mar 2022 11:41:52 +0530:
> Hi Miquel,
>
> On 06/03/22 9:02 pm, Tokunori Ikegami wrote:
> > The regression issue has been caused on S29GL064N and reported it.
> > Also the change mentioned is to use chip_good() for buffered write.
> > So disable the change on S29GL064N and use chip_ready() as before.
> >
> > Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
> > 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
> > Cc: stable@vger.kernel.org
> > Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
>
> Could you please pull this in for v5.18-rc1 as this is a regression
> being tracked for a while?
I was about to apply the patch but reviewing it quickly shown a number
of issues. I can still pull the patch for -rc1 if all this is
corrected rather quickly but otherwise it will go through Fixes later
on.
- I just don't understand the commit log, please rephrase it entirely.
You can include the link to the bug report as well inside the commit
text, not as part of a Link tag, because the Link tag is here to
refer to the mailing list discussion and is added automatically.
Something like:
As pointed out by this bug report [1], <what> is now broken on
S29GL064N. The reason is that <explain the problem>. One way to
solve the issue is to <explain why I should choose this
solution>.
[1] <the link here>
- The patch itself should be split into at least three parts:
* The ID definition (should not be part of the fix).
* The chip_check/chip_ready/chip_good_for_write code moves/renaming
independently of any functinal change as a preparation patch.
* The actual fix within one of the above functions.
Aside from these, I don't quite get what the __xipram keyword is
supposed to carry. We return a boolean expression stored in an int
provided by the map_word_equal() macro which does return a simple int
declared on the stack. It's not stored anywhere else than regular RAM,
or am I missing something?
> Acked-by: Vignesh Raghavendra <vigneshr@ti.com>
>
> [...]
>
> Regards
> Vignesh
>
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N
2022-03-15 9:50 ` Miquel Raynal
@ 2022-03-15 17:08 ` Tokunori Ikegami
0 siblings, 0 replies; 7+ messages in thread
From: Tokunori Ikegami @ 2022-03-15 17:08 UTC (permalink / raw)
To: Miquel Raynal, Vignesh Raghavendra; +Cc: linux-mtd, Richard Weinberger
Hi,
On 2022/03/15 18:50, Miquel Raynal wrote:
> Hello,
>
> vigneshr@ti.com wrote on Tue, 15 Mar 2022 11:41:52 +0530:
>
>> Hi Miquel,
>>
>> On 06/03/22 9:02 pm, Tokunori Ikegami wrote:
>>> The regression issue has been caused on S29GL064N and reported it.
>>> Also the change mentioned is to use chip_good() for buffered write.
>>> So disable the change on S29GL064N and use chip_ready() as before.
>>>
>>> Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>>> 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
>>> Cc: stable@vger.kernel.org
>>> Link: https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
>> Could you please pull this in for v5.18-rc1 as this is a regression
>> being tracked for a while?
> I was about to apply the patch but reviewing it quickly shown a number
> of issues. I can still pull the patch for -rc1 if all this is
> corrected rather quickly but otherwise it will go through Fixes later
> on.
>
> - I just don't understand the commit log, please rephrase it entirely.
> You can include the link to the bug report as well inside the commit
> text, not as part of a Link tag, because the Link tag is here to
> refer to the mailing list discussion and is added automatically.
> Something like:
>
> As pointed out by this bug report [1], <what> is now broken on
> S29GL064N. The reason is that <explain the problem>. One way to
> solve the issue is to <explain why I should choose this
> solution>.
>
> [1] <the link here>
Thanks. Just fixed by the version 3 patches and sent them.
> - The patch itself should be split into at least three parts:
> * The ID definition (should not be part of the fix).
> * The chip_check/chip_ready/chip_good_for_write code moves/renaming
> independently of any functinal change as a preparation patch.
> * The actual fix within one of the above functions.
This is also fixed by the version patches.
>
> Aside from these, I don't quite get what the __xipram keyword is
> supposed to carry. We return a boolean expression stored in an int
> provided by the map_word_equal() macro which does return a simple int
> declared on the stack. It's not stored anywhere else than regular RAM,
> or am I missing something?
The __xipram keyword was implemented by the commit 02b15e343aee for the
chip_ready and chip_good, etc.
Also the definision is implemneted in include/linux/mtd/xip.h as below.
/*
* Function that are modifying the flash state away from array mode must
* obviously not be running from flash. The __xipram is therefore marking
* those functions so they get relocated to ram.
*/
#ifdef CONFIG_XIP_KERNEL
#define __xipram noinline __section(".xiptext")
#endif
The changes for the fix just follows the implementation as same I think.
By the way it was added to use the __xipram keyword for
cfi_use_chip_ready_for_write() also.
Regards,
Ikegami
>
>> Acked-by: Vignesh Raghavendra <vigneshr@ti.com>
>>
>> [...]
>>
>> Regards
>> Vignesh
>>
> Thanks,
> Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-15 17:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-06 15:32 [PATCH v2] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N Tokunori Ikegami
2022-03-06 15:32 ` Tokunori Ikegami
2022-03-09 8:17 ` Vignesh Raghavendra
2022-03-09 10:29 ` Ahmad Fatoum
2022-03-15 6:11 ` Vignesh Raghavendra
2022-03-15 9:50 ` Miquel Raynal
2022-03-15 17:08 ` Tokunori Ikegami
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.