* [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword()
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
@ 2019-02-04 18:19 ` Tokunori Ikegami
2019-02-04 18:24 ` Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 02/11] mtd: cfi_cmdset_0002: Remove chip_ready() from do_write_buffer() Tokunori Ikegami
` (9 subsequent siblings)
10 siblings, 1 reply; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: stable, Joakim Tjernlund, Tokunori Ikegami, linux-mtd,
Chris Packham, Felix Fietkau
As reported by the OpenWRT team, write requests sometimes fail on some
platforms.
Currently to check the state chip_ready() is used correctly as described by
the flash memory S29GL256P11TFI01 datasheet.
Also chip_good() is used to check if the write is succeeded and it was
implemented by the commit fb4a90bfcd6d8 ("[MTD] CFI-0002 - Improve error
checking").
But actually the write failure is caused on some platforms and also it can
be fixed by using chip_good() to check the state and retry instead.
Also it seems that it is caused after repeated about 1,000 times to retry
the write one word with the reset command.
By using chip_good() to check the state to be done it can be reduced the
retry with reset.
It is depended on the actual flash chip behavior so the root cause is
unknown.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Co-Developed-by: Hauke Mehrtens <hauke@hauke-m.de>
Co-Developed-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
Reported-by: Fabio Bettoni <fbettoni@gmail.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
Cc: stable@vger.kernel.org
---
Changes since v3:
- Update the commit message for the comments.
- Drop the addition of blanks lines around xip_enable().
- Delete unnecessary setting the ret variable to -EIO.
- Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- Just update the commit message for the comment.
Changes since v1:
- Just update the commit message.
Background:
This is required for OpenWrt Project to result the flash write issue as
below patche.
<https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=ddc11c3932c7b7b7df7d5fbd48f207e77619eaa7>
Also the original patch in OpenWRT is below.
<https://github.com/openwrt/openwrt/blob/v18.06.0/target/linux/ar71xx/patches-4.9/403-mtd_fix_cfi_cmdset_0002_status_check.patch>
The reason to use chip_good() is that just actually fix the issue.
And also in the past I had fixed the erase function also as same way by the
patch below.
<https://patchwork.ozlabs.org/patch/922656/>
Note: The reason for the patch for erase is same.
In my understanding the chip_ready() is just checked the value twice from
flash.
So I think that sometimes incorrect value is read twice and it is depended
on the flash device behavior but not sure..
So change to use chip_good() instead of chip_ready().
drivers/mtd/chips/cfi_cmdset_0002.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
mode change 100644 => 100755 drivers/mtd/chips/cfi_cmdset_0002.c
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
old mode 100644
new mode 100755
index 72428b6..91a491b
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1627,29 +1627,31 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
continue;
}
- if (time_after(jiffies, timeo) && !chip_ready(map, adr)){
+ if (chip_good(map, adr, datum))
+ break;
+
+ if (time_after(jiffies, timeo)){
xip_enable(map, chip, adr);
printk(KERN_WARNING "MTD %s(): software timeout\n", __func__);
xip_disable(map, chip, adr);
+ ret = -EIO;
break;
}
- if (chip_ready(map, adr))
- break;
-
/* Latency issues. Drop the lock, wait a while and retry */
UDELAY(map, chip, adr, 1);
}
+
/* Did we succeed? */
- if (!chip_good(map, adr, datum)) {
+ if (ret) {
/* reset on all failures. */
map_write(map, CMD(0xF0), chip->start);
/* FIXME - should have reset delay before continuing */
- if (++retry_cnt <= MAX_RETRIES)
+ if (++retry_cnt <= MAX_RETRIES) {
+ ret = 0;
goto retry;
-
- ret = -EIO;
+ }
}
xip_enable(map, chip, adr);
op_done:
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* RE: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword()
2019-02-04 18:19 ` [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword() Tokunori Ikegami
@ 2019-02-04 18:24 ` Tokunori Ikegami
2019-02-05 11:57 ` Vignesh R
0 siblings, 1 reply; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:24 UTC (permalink / raw)
To: 'Boris Brezillon'
Cc: 'Joakim Tjernlund', 'Chris Packham',
linux-mtd, stable, 'Felix Fietkau'
Hi,
Very sorry for many times resending the patch mails.
Since I could not send the mails correctly with the incorrect mail address.
Still I can not send correctly the mail to stable@vger.kernel.org so I will
investigate this later.
Regards,
Ikegami
> -----Original Message-----
> From: Tokunori Ikegami [mailto:ikegami_to@yahoo.co.jp]
> Sent: Tuesday, February 5, 2019 3:20 AM
> To: Boris Brezillon
> Cc: Tokunori Ikegami; Felix Fietkau; Chris Packham; Joakim Tjernlund;
> linux-mtd@lists.infradead.org; stable@vger.kernel.org
> Subject: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry
> in do_write_oneword()
>
> As reported by the OpenWRT team, write requests sometimes fail on some
> platforms.
> Currently to check the state chip_ready() is used correctly as described
> by
> the flash memory S29GL256P11TFI01 datasheet.
> Also chip_good() is used to check if the write is succeeded and it was
> implemented by the commit fb4a90bfcd6d8 ("[MTD] CFI-0002 - Improve error
> checking").
> But actually the write failure is caused on some platforms and also it can
> be fixed by using chip_good() to check the state and retry instead.
> Also it seems that it is caused after repeated about 1,000 times to retry
> the write one word with the reset command.
> By using chip_good() to check the state to be done it can be reduced the
> retry with reset.
> It is depended on the actual flash chip behavior so the root cause is
> unknown.
>
> Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> Co-Developed-by: Hauke Mehrtens <hauke@hauke-m.de>
> Co-Developed-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> Reported-by: Fabio Bettoni <fbettoni@gmail.com>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> Cc: linux-mtd@lists.infradead.org
> Cc: stable@vger.kernel.org
> ---
> Changes since v3:
> - Update the commit message for the comments.
> - Drop the addition of blanks lines around xip_enable().
> - Delete unnecessary setting the ret variable to -EIO.
> - Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
>
> Changes since v2:
> - Just update the commit message for the comment.
>
> Changes since v1:
> - Just update the commit message.
>
> Background:
> This is required for OpenWrt Project to result the flash write issue as
> below patche.
> <https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=ddc11c3
> 932c7b7b7df7d5fbd48f207e77619eaa7>
>
> Also the original patch in OpenWRT is below.
> <https://github.com/openwrt/openwrt/blob/v18.06.0/target/linux/ar71xx/
> patches-4.9/403-mtd_fix_cfi_cmdset_0002_status_check.patch>
>
> The reason to use chip_good() is that just actually fix the issue.
> And also in the past I had fixed the erase function also as same way by
> the
> patch below.
> <https://patchwork.ozlabs.org/patch/922656/>
> Note: The reason for the patch for erase is same.
>
> In my understanding the chip_ready() is just checked the value twice from
> flash.
> So I think that sometimes incorrect value is read twice and it is depended
> on the flash device behavior but not sure..
>
> So change to use chip_good() instead of chip_ready().
>
> drivers/mtd/chips/cfi_cmdset_0002.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
> mode change 100644 => 100755 drivers/mtd/chips/cfi_cmdset_0002.c
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> b/drivers/mtd/chips/cfi_cmdset_0002.c
> old mode 100644
> new mode 100755
> index 72428b6..91a491b
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -1627,29 +1627,31 @@ static int __xipram do_write_oneword(struct
> map_info *map, struct flchip *chip,
> continue;
> }
>
> - if (time_after(jiffies, timeo) && !chip_ready(map, adr)){
> + if (chip_good(map, adr, datum))
> + break;
> +
> + if (time_after(jiffies, timeo)){
> xip_enable(map, chip, adr);
> printk(KERN_WARNING "MTD %s(): software
> timeout\n", __func__);
> xip_disable(map, chip, adr);
> + ret = -EIO;
> break;
> }
>
> - if (chip_ready(map, adr))
> - break;
> -
> /* Latency issues. Drop the lock, wait a while and retry
> */
> UDELAY(map, chip, adr, 1);
> }
> +
> /* Did we succeed? */
> - if (!chip_good(map, adr, datum)) {
> + if (ret) {
> /* reset on all failures. */
> map_write(map, CMD(0xF0), chip->start);
> /* FIXME - should have reset delay before continuing */
>
> - if (++retry_cnt <= MAX_RETRIES)
> + if (++retry_cnt <= MAX_RETRIES) {
> + ret = 0;
> goto retry;
> -
> - ret = -EIO;
> + }
> }
> xip_enable(map, chip, adr);
> op_done:
> --
> 2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword()
2019-02-04 18:24 ` Tokunori Ikegami
@ 2019-02-05 11:57 ` Vignesh R
2019-02-05 12:33 ` Tokunori Ikegami
0 siblings, 1 reply; 16+ messages in thread
From: Vignesh R @ 2019-02-05 11:57 UTC (permalink / raw)
To: Tokunori Ikegami, 'Boris Brezillon'
Cc: 'Joakim Tjernlund', 'Chris Packham',
linux-mtd, 'Felix Fietkau'
Hi,
On 04/02/19 11:54 PM, Tokunori Ikegami wrote:
> Hi,
>
> Very sorry for many times resending the patch mails.
> Since I could not send the mails correctly with the incorrect mail address.
> Still I can not send correctly the mail to stable@vger.kernel.org so I will
> investigate this later.
>
You don't have to explicitly send mail/Cc stable@vger.kernel.org. Its just a tag.
From Documentation/process/stable-kernel-rules.rst[1]:
Option 1
********
To have the patch automatically included in the stable tree, add the tag
.. code-block:: none
Cc: stable@vger.kernel.org
in the sign-off area. Once the patch is merged it will be applied to
the stable tree without anything else needing to be done by the author
or subsystem maintainer.
Along with Fixes tag[2]:
A Fixes: tag indicates that the patch fixes an issue in a previous commit. It
is used to make it easy to determine where a bug originated, which can help
review a bug fix. This tag also assists the stable kernel team in determining
which stable kernel versions should receive your fix.
[1]https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#for-all-other-submissions-choose-one-of-the-following-procedures
[2]https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
Regards
Vignesh
> Regards,
> Ikegami
>
>> -----Original Message-----
>> From: Tokunori Ikegami [mailto:ikegami_to@yahoo.co.jp]
>> Sent: Tuesday, February 5, 2019 3:20 AM
>> To: Boris Brezillon
>> Cc: Tokunori Ikegami; Felix Fietkau; Chris Packham; Joakim Tjernlund;
>> linux-mtd@lists.infradead.org; stable@vger.kernel.org
>> Subject: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry
>> in do_write_oneword()
>>
>> As reported by the OpenWRT team, write requests sometimes fail on some
>> platforms.
>> Currently to check the state chip_ready() is used correctly as described
>> by
>> the flash memory S29GL256P11TFI01 datasheet.
>> Also chip_good() is used to check if the write is succeeded and it was
>> implemented by the commit fb4a90bfcd6d8 ("[MTD] CFI-0002 - Improve error
>> checking").
>> But actually the write failure is caused on some platforms and also it can
>> be fixed by using chip_good() to check the state and retry instead.
>> Also it seems that it is caused after repeated about 1,000 times to retry
>> the write one word with the reset command.
>> By using chip_good() to check the state to be done it can be reduced the
>> retry with reset.
>> It is depended on the actual flash chip behavior so the root cause is
>> unknown.
>>
>> Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> Co-Developed-by: Hauke Mehrtens <hauke@hauke-m.de>
>> Co-Developed-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
>> Reported-by: Fabio Bettoni <fbettoni@gmail.com>
>> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
>> Cc: linux-mtd@lists.infradead.org
>> Cc: stable@vger.kernel.org
>> ---
>> Changes since v3:
>> - Update the commit message for the comments.
>> - Drop the addition of blanks lines around xip_enable().
>> - Delete unnecessary setting the ret variable to -EIO.
>> - Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
>>
>> Changes since v2:
>> - Just update the commit message for the comment.
>>
>> Changes since v1:
>> - Just update the commit message.
>>
>> Background:
>> This is required for OpenWrt Project to result the flash write issue as
>> below patche.
>> <https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=ddc11c3
>> 932c7b7b7df7d5fbd48f207e77619eaa7>
>>
>> Also the original patch in OpenWRT is below.
>> <https://github.com/openwrt/openwrt/blob/v18.06.0/target/linux/ar71xx/
>> patches-4.9/403-mtd_fix_cfi_cmdset_0002_status_check.patch>
>>
>> The reason to use chip_good() is that just actually fix the issue.
>> And also in the past I had fixed the erase function also as same way by
>> the
>> patch below.
>> <https://patchwork.ozlabs.org/patch/922656/>
>> Note: The reason for the patch for erase is same.
>>
>> In my understanding the chip_ready() is just checked the value twice from
>> flash.
>> So I think that sometimes incorrect value is read twice and it is depended
>> on the flash device behavior but not sure..
>>
>> So change to use chip_good() instead of chip_ready().
>>
>> drivers/mtd/chips/cfi_cmdset_0002.c | 18 ++++++++++--------
>> 1 file changed, 10 insertions(+), 8 deletions(-)
>> mode change 100644 => 100755 drivers/mtd/chips/cfi_cmdset_0002.c
>>
>> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
>> b/drivers/mtd/chips/cfi_cmdset_0002.c
>> old mode 100644
>> new mode 100755
>> index 72428b6..91a491b
>> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
>> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
>> @@ -1627,29 +1627,31 @@ static int __xipram do_write_oneword(struct
>> map_info *map, struct flchip *chip,
>> continue;
>> }
>>
>> - if (time_after(jiffies, timeo) && !chip_ready(map, adr)){
>> + if (chip_good(map, adr, datum))
>> + break;
>> +
>> + if (time_after(jiffies, timeo)){
>> xip_enable(map, chip, adr);
>> printk(KERN_WARNING "MTD %s(): software
>> timeout\n", __func__);
>> xip_disable(map, chip, adr);
>> + ret = -EIO;
>> break;
>> }
>>
>> - if (chip_ready(map, adr))
>> - break;
>> -
>> /* Latency issues. Drop the lock, wait a while and retry
>> */
>> UDELAY(map, chip, adr, 1);
>> }
>> +
>> /* Did we succeed? */
>> - if (!chip_good(map, adr, datum)) {
>> + if (ret) {
>> /* reset on all failures. */
>> map_write(map, CMD(0xF0), chip->start);
>> /* FIXME - should have reset delay before continuing */
>>
>> - if (++retry_cnt <= MAX_RETRIES)
>> + if (++retry_cnt <= MAX_RETRIES) {
>> + ret = 0;
>> goto retry;
>> -
>> - ret = -EIO;
>> + }
>> }
>> xip_enable(map, chip, adr);
>> op_done:
>> --
>> 2.11.0
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
--
Regards
Vignesh
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword()
2019-02-05 11:57 ` Vignesh R
@ 2019-02-05 12:33 ` Tokunori Ikegami
0 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-05 12:33 UTC (permalink / raw)
To: 'Vignesh R', 'Boris Brezillon'
Cc: 'Joakim Tjernlund', 'Chris Packham',
linux-mtd, 'Felix Fietkau'
Hi Vignesh-san,
Thanks for your advice.
Noted it.
Regards,
Ikegami
> -----Original Message-----
> From: Vignesh R [mailto:vigneshr@ti.com]
> Sent: Tuesday, February 5, 2019 8:58 PM
> To: Tokunori Ikegami; 'Boris Brezillon'
> Cc: 'Joakim Tjernlund'; 'Chris Packham'; linux-mtd@lists.infradead.org;
> 'Felix Fietkau'
> Subject: Re: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to
> retry in do_write_oneword()
>
> Hi,
>
> On 04/02/19 11:54 PM, Tokunori Ikegami wrote:
> > Hi,
> >
> > Very sorry for many times resending the patch mails.
> > Since I could not send the mails correctly with the incorrect mail address.
> > Still I can not send correctly the mail to stable@vger.kernel.org so I
> will
> > investigate this later.
> >
>
> You don't have to explicitly send mail/Cc stable@vger.kernel.org. Its just
> a tag.
>
> From Documentation/process/stable-kernel-rules.rst[1]:
> Option 1
> ********
>
> To have the patch automatically included in the stable tree, add the tag
>
> . code-block:: none
>
> Cc: stable@vger.kernel.org
>
> in the sign-off area. Once the patch is merged it will be applied to
> the stable tree without anything else needing to be done by the author
> or subsystem maintainer.
>
> Along with Fixes tag[2]:
> A Fixes: tag indicates that the patch fixes an issue in a previous commit.
> It
> is used to make it easy to determine where a bug originated, which can help
> review a bug fix. This tag also assists the stable kernel team in determining
> which stable kernel versions should receive your fix.
>
>
> [1]https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.
> html#for-all-other-submissions-choose-one-of-the-following-procedures
> [2]https://www.kernel.org/doc/html/latest/process/submitting-patches.h
> tml#describe-your-changes
>
> Regards
> Vignesh
>
> > Regards,
> > Ikegami
> >
> >> -----Original Message-----
> >> From: Tokunori Ikegami [mailto:ikegami_to@yahoo.co.jp]
> >> Sent: Tuesday, February 5, 2019 3:20 AM
> >> To: Boris Brezillon
> >> Cc: Tokunori Ikegami; Felix Fietkau; Chris Packham; Joakim Tjernlund;
> >> linux-mtd@lists.infradead.org; stable@vger.kernel.org
> >> Subject: [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry
> >> in do_write_oneword()
> >>
> >> As reported by the OpenWRT team, write requests sometimes fail on some
> >> platforms.
> >> Currently to check the state chip_ready() is used correctly as described
> >> by
> >> the flash memory S29GL256P11TFI01 datasheet.
> >> Also chip_good() is used to check if the write is succeeded and it was
> >> implemented by the commit fb4a90bfcd6d8 ("[MTD] CFI-0002 - Improve error
> >> checking").
> >> But actually the write failure is caused on some platforms and also it
> can
> >> be fixed by using chip_good() to check the state and retry instead.
> >> Also it seems that it is caused after repeated about 1,000 times to retry
> >> the write one word with the reset command.
> >> By using chip_good() to check the state to be done it can be reduced
> the
> >> retry with reset.
> >> It is depended on the actual flash chip behavior so the root cause is
> >> unknown.
> >>
> >> Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
> >> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> >> Co-Developed-by: Hauke Mehrtens <hauke@hauke-m.de>
> >> Co-Developed-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
> >> Reported-by: Fabio Bettoni <fbettoni@gmail.com>
> >> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> >> Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> >> Cc: linux-mtd@lists.infradead.org
> >> Cc: stable@vger.kernel.org
> >> ---
> >> Changes since v3:
> >> - Update the commit message for the comments.
> >> - Drop the addition of blanks lines around xip_enable().
> >> - Delete unnecessary setting the ret variable to -EIO.
> >> - Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
> >>
> >> Changes since v2:
> >> - Just update the commit message for the comment.
> >>
> >> Changes since v1:
> >> - Just update the commit message.
> >>
> >> Background:
> >> This is required for OpenWrt Project to result the flash write issue
> as
> >> below patche.
> >>
> <https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=ddc11c3
> >> 932c7b7b7df7d5fbd48f207e77619eaa7>
> >>
> >> Also the original patch in OpenWRT is below.
> >>
> <https://github.com/openwrt/openwrt/blob/v18.06.0/target/linux/ar71xx/
> >> patches-4.9/403-mtd_fix_cfi_cmdset_0002_status_check.patch>
> >>
> >> The reason to use chip_good() is that just actually fix the issue.
> >> And also in the past I had fixed the erase function also as same way
> by
> >> the
> >> patch below.
> >> <https://patchwork.ozlabs.org/patch/922656/>
> >> Note: The reason for the patch for erase is same.
> >>
> >> In my understanding the chip_ready() is just checked the value twice
> from
> >> flash.
> >> So I think that sometimes incorrect value is read twice and it is depended
> >> on the flash device behavior but not sure..
> >>
> >> So change to use chip_good() instead of chip_ready().
> >>
> >> drivers/mtd/chips/cfi_cmdset_0002.c | 18 ++++++++++--------
> >> 1 file changed, 10 insertions(+), 8 deletions(-)
> >> mode change 100644 => 100755 drivers/mtd/chips/cfi_cmdset_0002.c
> >>
> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> >> b/drivers/mtd/chips/cfi_cmdset_0002.c
> >> old mode 100644
> >> new mode 100755
> >> index 72428b6..91a491b
> >> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> >> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> >> @@ -1627,29 +1627,31 @@ static int __xipram do_write_oneword(struct
> >> map_info *map, struct flchip *chip,
> >> continue;
> >> }
> >>
> >> - if (time_after(jiffies, timeo) && !chip_ready(map, adr)){
> >> + if (chip_good(map, adr, datum))
> >> + break;
> >> +
> >> + if (time_after(jiffies, timeo)){
> >> xip_enable(map, chip, adr);
> >> printk(KERN_WARNING "MTD %s(): software
> >> timeout\n", __func__);
> >> xip_disable(map, chip, adr);
> >> + ret = -EIO;
> >> break;
> >> }
> >>
> >> - if (chip_ready(map, adr))
> >> - break;
> >> -
> >> /* Latency issues. Drop the lock, wait a while and retry
> >> */
> >> UDELAY(map, chip, adr, 1);
> >> }
> >> +
> >> /* Did we succeed? */
> >> - if (!chip_good(map, adr, datum)) {
> >> + if (ret) {
> >> /* reset on all failures. */
> >> map_write(map, CMD(0xF0), chip->start);
> >> /* FIXME - should have reset delay before continuing */
> >>
> >> - if (++retry_cnt <= MAX_RETRIES)
> >> + if (++retry_cnt <= MAX_RETRIES) {
> >> + ret = 0;
> >> goto retry;
> >> -
> >> - ret = -EIO;
> >> + }
> >> }
> >> xip_enable(map, chip, adr);
> >> op_done:
> >> --
> >> 2.11.0
> >
> >
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> >
>
> --
> Regards
> Vignesh
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RESEND v4 02/11] mtd: cfi_cmdset_0002: Remove chip_ready() from do_write_buffer()
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword() Tokunori Ikegami
@ 2019-02-04 18:19 ` Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 03/11] mtd: cfi_cmdset_0002: Remove goto statement " Tokunori Ikegami
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
It is enough to use chip_good() only so chip_ready() is not necessary.
Since chip_good() is doing the same check chip_ready() is doing plus an extra
check to make sure we end up with the data we wrote.
For this change the order to check timeout also will be changed at same time.
The order is not a hard requirement, it's just better to avoid the case where
the data update happens just after the timeout has expired.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Update the commit message for the comment.
- Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- None.
Changes since v1:
- None.
drivers/mtd/chips/cfi_cmdset_0002.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 91a491b..ca9d5fa 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1878,14 +1878,14 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
continue;
}
- if (time_after(jiffies, timeo) && !chip_ready(map, adr))
- break;
-
if (chip_good(map, adr, datum)) {
xip_enable(map, chip, adr);
goto op_done;
}
+ if (time_after(jiffies, timeo))
+ break;
+
/* Latency issues. Drop the lock, wait a while and retry */
UDELAY(map, chip, adr, 1);
}
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 03/11] mtd: cfi_cmdset_0002: Remove goto statement from do_write_buffer()
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 01/11] mtd: cfi_cmdset_0002: Use chip_good() to retry in do_write_oneword() Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 02/11] mtd: cfi_cmdset_0002: Remove chip_ready() from do_write_buffer() Tokunori Ikegami
@ 2019-02-04 18:19 ` Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 04/11] mtd: cfi_cmdset_0002: Call xip_enable() once only in do_write_buffer() Tokunori Ikegami
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
For a maintainability by reducing the goto statement remove it from
do_write_buffer().
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Just change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- None.
Changes since v1:
- Split the patch v1 3/3.
drivers/mtd/chips/cfi_cmdset_0002.c | 46 +++++++++++++++++++------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index ca9d5fa..1aacf22 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1880,38 +1880,40 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
if (chip_good(map, adr, datum)) {
xip_enable(map, chip, adr);
- goto op_done;
+ break;
}
- if (time_after(jiffies, timeo))
+ if (time_after(jiffies, timeo)) {
+ ret = -EIO;
break;
+ }
/* Latency issues. Drop the lock, wait a while and retry */
UDELAY(map, chip, adr, 1);
}
- /*
- * Recovery from write-buffer programming failures requires
- * the write-to-buffer-reset sequence. Since the last part
- * of the sequence also works as a normal reset, we can run
- * the same commands regardless of why we are here.
- * See e.g.
- * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
- */
- cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
- cfi->device_type, NULL);
- cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
- cfi->device_type, NULL);
- cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
- cfi->device_type, NULL);
- xip_enable(map, chip, adr);
- /* FIXME - should have reset delay before continuing */
+ if (ret) {
+ /*
+ * Recovery from write-buffer programming failures requires
+ * the write-to-buffer-reset sequence. Since the last part
+ * of the sequence also works as a normal reset, we can run
+ * the same commands regardless of why we are here.
+ * See e.g.
+ * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
+ */
+ cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+ cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+ cfi->device_type, NULL);
+ cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
+ cfi->device_type, NULL);
+ xip_enable(map, chip, adr);
+ /* FIXME - should have reset delay before continuing */
- printk(KERN_WARNING "MTD %s(): software timeout, address:0x%.8lx.\n",
- __func__, adr);
+ printk(KERN_WARNING "MTD %s(): software timeout, address:0x%.8lx.\n",
+ __func__, adr);
+ }
- ret = -EIO;
- op_done:
chip->state = FL_READY;
DISABLE_VPP(map);
put_chip(map, chip, adr);
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 04/11] mtd: cfi_cmdset_0002: Call xip_enable() once only in do_write_buffer().
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (2 preceding siblings ...)
2019-02-04 18:19 ` [RESEND v4 03/11] mtd: cfi_cmdset_0002: Remove goto statement " Tokunori Ikegami
@ 2019-02-04 18:19 ` Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 05/11] mtd: cfi_cmdset_0002: Split do_write_oneword() to reduce function size Tokunori Ikegami
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
By the removed goto statement it can be called xip_enable() once.
Also for a maintainability refactor it to call the function only once.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Just change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- None.
Changes since v1:
- Split from the patch v1 3/3.
drivers/mtd/chips/cfi_cmdset_0002.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 1aacf22..92d3e82 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1878,10 +1878,8 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
continue;
}
- if (chip_good(map, adr, datum)) {
- xip_enable(map, chip, adr);
+ if (chip_good(map, adr, datum))
break;
- }
if (time_after(jiffies, timeo)) {
ret = -EIO;
@@ -1907,13 +1905,14 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
cfi->device_type, NULL);
cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
cfi->device_type, NULL);
- xip_enable(map, chip, adr);
/* FIXME - should have reset delay before continuing */
printk(KERN_WARNING "MTD %s(): software timeout, address:0x%.8lx.\n",
__func__, adr);
}
+ xip_enable(map, chip, adr);
+
chip->state = FL_READY;
DISABLE_VPP(map);
put_chip(map, chip, adr);
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 05/11] mtd: cfi_cmdset_0002: Split do_write_oneword() to reduce function size
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (3 preceding siblings ...)
2019-02-04 18:19 ` [RESEND v4 04/11] mtd: cfi_cmdset_0002: Call xip_enable() once only in do_write_buffer() Tokunori Ikegami
@ 2019-02-04 18:19 ` Tokunori Ikegami
2019-02-04 18:19 ` [RESEND v4 06/11] mtd: cfi_cmdset_0002: Split do_write_oneword() op_done goto statement Tokunori Ikegami
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, Tokunori Ikegami, linux-mtd, Fabio Bettoni
Reduce the size of do_write_oneword() by extracting a helper function
for the hardware access.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Drop the unnecessary comment and blank line.
- Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- Just update the commit message for the comment
- Add Reviewed-by tag.
Changes since v1:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 89 ++++++++++++++++++++-----------------
1 file changed, 49 insertions(+), 40 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 92d3e82..6393af4 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1547,11 +1547,10 @@ static int cfi_amdstd_lock_user_prot_reg(struct mtd_info *mtd, loff_t from,
do_otp_lock, 1);
}
-static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
- unsigned long adr, map_word datum,
- int mode)
+static int __xipram do_write_oneword_once(struct map_info *map, struct flchip *chip,
+ unsigned long adr, map_word datum,
+ int mode, struct cfi_private *cfi)
{
- struct cfi_private *cfi = map->fldrv_priv;
unsigned long timeo = jiffies + HZ;
/*
* We use a 1ms + 1 jiffies generic timeout for writes (most devices
@@ -1564,42 +1563,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
*/
unsigned long uWriteTimeout = (HZ / 1000) + 1;
int ret = 0;
- map_word oldd;
- int retry_cnt = 0;
-
- adr += chip->start;
-
- mutex_lock(&chip->mutex);
- ret = get_chip(map, chip, adr, mode);
- if (ret) {
- mutex_unlock(&chip->mutex);
- return ret;
- }
- pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n",
- __func__, adr, datum.x[0]);
-
- if (mode == FL_OTP_WRITE)
- otp_enter(map, chip, adr, map_bankwidth(map));
-
- /*
- * Check for a NOP for the case when the datum to write is already
- * present - it saves time and works around buggy chips that corrupt
- * data at other locations when 0xff is written to a location that
- * already contains 0xff.
- */
- oldd = map_read(map, adr);
- if (map_word_equal(map, oldd, datum)) {
- pr_debug("MTD %s(): NOP\n",
- __func__);
- goto op_done;
- }
-
- XIP_INVAL_CACHED_RANGE(map, adr, map_bankwidth(map));
- ENABLE_VPP(map);
- xip_disable(map, chip, adr);
-
- retry:
cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
@@ -1642,7 +1606,52 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
UDELAY(map, chip, adr, 1);
}
- /* Did we succeed? */
+ return ret;
+}
+
+static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
+ unsigned long adr, map_word datum,
+ int mode)
+{
+ struct cfi_private *cfi = map->fldrv_priv;
+ int ret = 0;
+ map_word oldd;
+ int retry_cnt = 0;
+
+ adr += chip->start;
+
+ mutex_lock(&chip->mutex);
+ ret = get_chip(map, chip, adr, mode);
+ if (ret) {
+ mutex_unlock(&chip->mutex);
+ return ret;
+ }
+
+ pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n",
+ __func__, adr, datum.x[0]);
+
+ if (mode == FL_OTP_WRITE)
+ otp_enter(map, chip, adr, map_bankwidth(map));
+
+ /*
+ * Check for a NOP for the case when the datum to write is already
+ * present - it saves time and works around buggy chips that corrupt
+ * data at other locations when 0xff is written to a location that
+ * already contains 0xff.
+ */
+ oldd = map_read(map, adr);
+ if (map_word_equal(map, oldd, datum)) {
+ pr_debug("MTD %s(): NOP\n",
+ __func__);
+ goto op_done;
+ }
+
+ XIP_INVAL_CACHED_RANGE(map, adr, map_bankwidth(map));
+ ENABLE_VPP(map);
+ xip_disable(map, chip, adr);
+
+ retry:
+ ret = do_write_oneword_once(map, chip, adr, datum, mode, cfi);
if (ret) {
/* reset on all failures. */
map_write(map, CMD(0xF0), chip->start);
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 06/11] mtd: cfi_cmdset_0002: Split do_write_oneword() op_done goto statement
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (4 preceding siblings ...)
2019-02-04 18:19 ` [RESEND v4 05/11] mtd: cfi_cmdset_0002: Split do_write_oneword() to reduce function size Tokunori Ikegami
@ 2019-02-04 18:19 ` Tokunori Ikegami
2019-02-04 18:20 ` [RESEND v4 07/11] mtd: cfi_cmdset_0002: Remove op_done goto statement from do_write_oneword() Tokunori Ikegami
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
To reduce function size and remove the goto statement split the op_done goto
statement part into do_write_oneword_done() created a function.
Also split the start part into do_write_oneword_start() to find easier pairs.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Rebased on the patch v4 01/11.
- Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- Change to split the start part of do_write_oneword() additionally.
- Fix indentation to call pr_debug().
Changes since v1:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 57 ++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 6393af4..a11dc40 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1609,6 +1609,40 @@ static int __xipram do_write_oneword_once(struct map_info *map, struct flchip *c
return ret;
}
+static int __xipram do_write_oneword_start(struct map_info *map,
+ struct flchip *chip,
+ unsigned long adr, int mode)
+{
+ int ret = 0;
+
+ mutex_lock(&chip->mutex);
+
+ ret = get_chip(map, chip, adr, mode);
+ if (ret) {
+ mutex_unlock(&chip->mutex);
+ return ret;
+ }
+
+ if (mode == FL_OTP_WRITE)
+ otp_enter(map, chip, adr, map_bankwidth(map));
+
+ return ret;
+}
+
+static void __xipram do_write_oneword_done(struct map_info *map,
+ struct flchip *chip,
+ unsigned long adr, int mode)
+{
+ if (mode == FL_OTP_WRITE)
+ otp_exit(map, chip, adr, map_bankwidth(map));
+
+ chip->state = FL_READY;
+ DISABLE_VPP(map);
+ put_chip(map, chip, adr);
+
+ mutex_unlock(&chip->mutex);
+}
+
static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
unsigned long adr, map_word datum,
int mode)
@@ -1620,19 +1654,14 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
adr += chip->start;
- mutex_lock(&chip->mutex);
- ret = get_chip(map, chip, adr, mode);
+ pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n", __func__, adr,
+ datum.x[0]);
+
+ ret = do_write_oneword_start(map, chip, adr, mode);
if (ret) {
- mutex_unlock(&chip->mutex);
return ret;
}
- pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n",
- __func__, adr, datum.x[0]);
-
- if (mode == FL_OTP_WRITE)
- otp_enter(map, chip, adr, map_bankwidth(map));
-
/*
* Check for a NOP for the case when the datum to write is already
* present - it saves time and works around buggy chips that corrupt
@@ -1641,8 +1670,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
*/
oldd = map_read(map, adr);
if (map_word_equal(map, oldd, datum)) {
- pr_debug("MTD %s(): NOP\n",
- __func__);
+ pr_debug("MTD %s(): NOP\n", __func__);
goto op_done;
}
@@ -1664,12 +1692,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
}
xip_enable(map, chip, adr);
op_done:
- if (mode == FL_OTP_WRITE)
- otp_exit(map, chip, adr, map_bankwidth(map));
- chip->state = FL_READY;
- DISABLE_VPP(map);
- put_chip(map, chip, adr);
- mutex_unlock(&chip->mutex);
+ do_write_oneword_done(map, chip, adr, mode);
return ret;
}
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 07/11] mtd: cfi_cmdset_0002: Remove op_done goto statement from do_write_oneword()
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (5 preceding siblings ...)
2019-02-04 18:19 ` [RESEND v4 06/11] mtd: cfi_cmdset_0002: Split do_write_oneword() op_done goto statement Tokunori Ikegami
@ 2019-02-04 18:20 ` Tokunori Ikegami
2019-02-04 18:20 ` [RESEND v4 08/11] mtd: cfi_cmdset_0002: Remove retry " Tokunori Ikegami
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
This is just to refactor the function by removing the goto statement.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Rebased on the patch v4 01/11.
- Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- Just rebased.
Changes since v1:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a11dc40..a5513bd 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1671,7 +1671,8 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
oldd = map_read(map, adr);
if (map_word_equal(map, oldd, datum)) {
pr_debug("MTD %s(): NOP\n", __func__);
- goto op_done;
+ do_write_oneword_done(map, chip, adr, mode);
+ return ret;
}
XIP_INVAL_CACHED_RANGE(map, adr, map_bankwidth(map));
@@ -1691,7 +1692,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
}
}
xip_enable(map, chip, adr);
- op_done:
+
do_write_oneword_done(map, chip, adr, mode);
return ret;
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 08/11] mtd: cfi_cmdset_0002: Remove retry goto statement from do_write_oneword()
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (6 preceding siblings ...)
2019-02-04 18:20 ` [RESEND v4 07/11] mtd: cfi_cmdset_0002: Remove op_done goto statement from do_write_oneword() Tokunori Ikegami
@ 2019-02-04 18:20 ` Tokunori Ikegami
2019-02-04 18:20 ` [RESEND v4 09/11] mtd: cfi_cmdset_0002: Split write-to-buffer-reset sequence Tokunori Ikegami
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
This is just to refactor the function by removing the goto statement.
Change to use the for loop instead of the goto statement.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Rebased.
- Change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- None.
Changes since v1:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a5513bd..81f5e5e 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1679,17 +1679,15 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
ENABLE_VPP(map);
xip_disable(map, chip, adr);
- retry:
- ret = do_write_oneword_once(map, chip, adr, datum, mode, cfi);
- if (ret) {
+ for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
+ ret = do_write_oneword_once(map, chip, adr, datum, mode, cfi);
+ if (!ret)
+ break;
+
/* reset on all failures. */
map_write(map, CMD(0xF0), chip->start);
- /* FIXME - should have reset delay before continuing */
- if (++retry_cnt <= MAX_RETRIES) {
- ret = 0;
- goto retry;
- }
+ /* FIXME - should have reset delay before continuing */
}
xip_enable(map, chip, adr);
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 09/11] mtd: cfi_cmdset_0002: Split write-to-buffer-reset sequence
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (7 preceding siblings ...)
2019-02-04 18:20 ` [RESEND v4 08/11] mtd: cfi_cmdset_0002: Remove retry " Tokunori Ikegami
@ 2019-02-04 18:20 ` Tokunori Ikegami
2019-02-04 18:20 ` [RESEND v4 10/11] mtd: cfi_cmdset_0002: Split to wait write buffer to check if completed Tokunori Ikegami
2019-02-04 18:20 ` [RESEND v4 11/11] mtd: cfi_cmdset_0002: Split do_write_oneword() to reduce exit paths Tokunori Ikegami
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
Just refactor to split the sequence from do_write_buffer().
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Just change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- None.
Changes since v1:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 38 +++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 81f5e5e..d5954bf 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1820,6 +1820,27 @@ static int cfi_amdstd_write_words(struct mtd_info *mtd, loff_t to, size_t len,
return 0;
}
+static void __xipram do_write_buffer_reset(struct map_info *map,
+ struct flchip *chip,
+ struct cfi_private *cfi)
+{
+ /*
+ * Recovery from write-buffer programming failures requires
+ * the write-to-buffer-reset sequence. Since the last part
+ * of the sequence also works as a normal reset, we can run
+ * the same commands regardless of why we are here.
+ * See e.g.
+ * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
+ */
+ cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+ cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+ cfi->device_type, NULL);
+ cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
+ cfi->device_type, NULL);
+
+ /* FIXME - should have reset delay before continuing */
+}
/*
* FIXME: interleaved mode not tested, and probably not supported!
@@ -1922,22 +1943,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
}
if (ret) {
- /*
- * Recovery from write-buffer programming failures requires
- * the write-to-buffer-reset sequence. Since the last part
- * of the sequence also works as a normal reset, we can run
- * the same commands regardless of why we are here.
- * See e.g.
- * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
- */
- cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
- cfi->device_type, NULL);
- cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
- cfi->device_type, NULL);
- cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
- cfi->device_type, NULL);
- /* FIXME - should have reset delay before continuing */
-
+ do_write_buffer_reset(map, chip, cfi);
printk(KERN_WARNING "MTD %s(): software timeout, address:0x%.8lx.\n",
__func__, adr);
}
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 10/11] mtd: cfi_cmdset_0002: Split to wait write buffer to check if completed
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (8 preceding siblings ...)
2019-02-04 18:20 ` [RESEND v4 09/11] mtd: cfi_cmdset_0002: Split write-to-buffer-reset sequence Tokunori Ikegami
@ 2019-02-04 18:20 ` Tokunori Ikegami
2019-02-04 18:20 ` [RESEND v4 11/11] mtd: cfi_cmdset_0002: Split do_write_oneword() to reduce exit paths Tokunori Ikegami
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
Just refactor to split the wait from do_write_buffer().
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Just change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- None.
Changes since v1:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 81 +++++++++++++++++++++----------------
1 file changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index d5954bf..da48b4c 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1820,6 +1820,51 @@ static int cfi_amdstd_write_words(struct mtd_info *mtd, loff_t to, size_t len,
return 0;
}
+static int __xipram do_write_buffer_wait(struct map_info *map,
+ struct flchip *chip, unsigned long adr,
+ map_word datum)
+{
+ unsigned long timeo;
+ unsigned long uWriteTimeout;
+ int ret = 0;
+
+ /*
+ * Timeout is calculated according to CFI data, if available.
+ * See more comments in cfi_cmdset_0002().
+ */
+ uWriteTimeout = usecs_to_jiffies(chip->buffer_write_time_max);
+ timeo = jiffies + uWriteTimeout;
+
+ for (;;) {
+ if (chip->state != FL_WRITING) {
+ /* Someone's suspended the write. Sleep */
+ DECLARE_WAITQUEUE(wait, current);
+
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ add_wait_queue(&chip->wq, &wait);
+ mutex_unlock(&chip->mutex);
+ schedule();
+ remove_wait_queue(&chip->wq, &wait);
+ timeo = jiffies + (HZ / 2); /* FIXME */
+ mutex_lock(&chip->mutex);
+ continue;
+ }
+
+ if (chip_good(map, adr, datum))
+ break;
+
+ if (time_after(jiffies, timeo)) {
+ ret = -EIO;
+ break;
+ }
+
+ /* Latency issues. Drop the lock, wait a while and retry */
+ UDELAY(map, chip, adr, 1);
+ }
+
+ return ret;
+}
+
static void __xipram do_write_buffer_reset(struct map_info *map,
struct flchip *chip,
struct cfi_private *cfi)
@@ -1850,13 +1895,6 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
int len)
{
struct cfi_private *cfi = map->fldrv_priv;
- unsigned long timeo = jiffies + HZ;
- /*
- * Timeout is calculated according to CFI data, if available.
- * See more comments in cfi_cmdset_0002().
- */
- unsigned long uWriteTimeout =
- usecs_to_jiffies(chip->buffer_write_time_max);
int ret = -EIO;
unsigned long cmd_adr;
int z, words;
@@ -1913,34 +1951,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
adr, map_bankwidth(map),
chip->word_write_time);
- timeo = jiffies + uWriteTimeout;
-
- for (;;) {
- if (chip->state != FL_WRITING) {
- /* Someone's suspended the write. Sleep */
- DECLARE_WAITQUEUE(wait, current);
-
- set_current_state(TASK_UNINTERRUPTIBLE);
- add_wait_queue(&chip->wq, &wait);
- mutex_unlock(&chip->mutex);
- schedule();
- remove_wait_queue(&chip->wq, &wait);
- timeo = jiffies + (HZ / 2); /* FIXME */
- mutex_lock(&chip->mutex);
- continue;
- }
-
- if (chip_good(map, adr, datum))
- break;
-
- if (time_after(jiffies, timeo)) {
- ret = -EIO;
- break;
- }
-
- /* Latency issues. Drop the lock, wait a while and retry */
- UDELAY(map, chip, adr, 1);
- }
+ ret = do_write_buffer_wait(map, chip, adr, datum);
if (ret) {
do_write_buffer_reset(map, chip, cfi);
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RESEND v4 11/11] mtd: cfi_cmdset_0002: Split do_write_oneword() to reduce exit paths
2019-02-04 18:19 [RESEND v4 00/11] mtd: cfi_cmdset_0002: Fix flash write issue for OpenWrt Project Tokunori Ikegami
` (9 preceding siblings ...)
2019-02-04 18:20 ` [RESEND v4 10/11] mtd: cfi_cmdset_0002: Split to wait write buffer to check if completed Tokunori Ikegami
@ 2019-02-04 18:20 ` Tokunori Ikegami
10 siblings, 0 replies; 16+ messages in thread
From: Tokunori Ikegami @ 2019-02-04 18:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Joakim Tjernlund, linux-mtd, Tokunori Ikegami, Chris Packham,
Fabio Bettoni
The do_write_oneword_done() is called twice at the exit paths.
By splitting the retry functionality it can be reduced to call once.
Signed-off-by: Tokunori Ikegami <ikegami_to@yahoo.co.jp>
Cc: Fabio Bettoni <fbettoni@gmail.com>
Co: Hauke Mehrtens <hauke@hauke-m.de>
Co: Koen Vandeputte <koen.vandeputte@ncentric.com>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: linux-mtd@lists.infradead.org
---
Changes since v3:
- Just change the email address of Tokunori Ikegami to ikegami_to@yahoo.co.jp.
Changes since v2:
- Add the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 39 ++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index da48b4c..21ba6d6 100755
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1643,25 +1643,16 @@ static void __xipram do_write_oneword_done(struct map_info *map,
mutex_unlock(&chip->mutex);
}
-static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
- unsigned long adr, map_word datum,
- int mode)
+static int __xipram do_write_oneword_retry(struct map_info *map,
+ struct flchip *chip,
+ unsigned long adr, map_word datum,
+ int mode)
{
struct cfi_private *cfi = map->fldrv_priv;
int ret = 0;
map_word oldd;
int retry_cnt = 0;
- adr += chip->start;
-
- pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n", __func__, adr,
- datum.x[0]);
-
- ret = do_write_oneword_start(map, chip, adr, mode);
- if (ret) {
- return ret;
- }
-
/*
* Check for a NOP for the case when the datum to write is already
* present - it saves time and works around buggy chips that corrupt
@@ -1671,7 +1662,6 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
oldd = map_read(map, adr);
if (map_word_equal(map, oldd, datum)) {
pr_debug("MTD %s(): NOP\n", __func__);
- do_write_oneword_done(map, chip, adr, mode);
return ret;
}
@@ -1691,6 +1681,27 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
}
xip_enable(map, chip, adr);
+ return ret;
+}
+
+static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
+ unsigned long adr, map_word datum,
+ int mode)
+{
+ int ret = 0;
+
+ adr += chip->start;
+
+ pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n", __func__, adr,
+ datum.x[0]);
+
+ ret = do_write_oneword_start(map, chip, adr, mode);
+ if (ret) {
+ return ret;
+ }
+
+ ret = do_write_oneword_retry(map, chip, adr, datum, mode);
+
do_write_oneword_done(map, chip, adr, mode);
return ret;
--
2.11.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread