Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

Ezequiel Garcia
2015-02-18T14:08:51Z

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 02/18/2015 11:01 AM, Maxime Ripard wrote:
> On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
>> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
>>> The NDDB register holds the data that are needed by the read
>>> and write commands.
>>>
>>> However, during a read PIO access, the datasheet specifies that
>>> after each 32 bytes read in that register, when BCH is enabled,
>>> we have to make sure that the RDDREQ bit is set in the NDSR
>>> register.
>>>
>>> This fixes an issue that was seen on the Armada 385, and
>>> presumably other mvebu SoCs, when a read on a newly erased page
>>> would end up in the driver reporting a timeout from the NAND.
>>>
>>> Cc: <stable@vger.kernel.org> # v3.14 Signed-off-by: Maxime
>>> Ripard <maxime.ripard@free-electrons.com> ---
>>> drivers/mtd/nand/pxa3xx_nand.c | 48
>>> ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42
>>> insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>>> b/drivers/mtd/nand/pxa3xx_nand.c index
>>> 96b0b1d27df1..bc677362bc73 100644 ---
>>> a/drivers/mtd/nand/pxa3xx_nand.c +++
>>> b/drivers/mtd/nand/pxa3xx_nand.c @@ -480,6 +480,42 @@ static
>>> void disable_int(struct pxa3xx_nand_info *info, uint32_t
>>> int_mask) nand_writel(info, NDCR, ndcr | int_mask); }
>>>
>>> +static void drain_fifo(struct pxa3xx_nand_info *info, void
>>> *data, int len) +{ + if (info->ecc_bch) { + int timeout; + +
>>> /* + * According to the datasheet, when reading from NDDB +
>>> * with BCH enabled, after each 32 bytes reads, we + * have to
>>> make sure that the NDSR.RDDREQ bit is set. + * + * Drain
>>> the FIFO 8 32 bits reads at a time, and skip + * the polling
>>> on the last read. + */ + while (len > 8) { +
>>> __raw_readsl(info->mmio_base + NDDB, data, 8); + + for
>>> (timeout = 0; + !(nand_readl(info, NDSR) &
>>> NDSR_RDDREQ); + timeout++) { + if (timeout >= 5) { +
>>> dev_err(&info->pdev->dev, + "Timeout on RDDREQ while
>>> draining the FIFO\n"); + return; + } + + mdelay(1);
>>
>> This is probably a stupid nit.. but here it goes is it any
>> difference if udelay is used here?
>>
>> Does this makes anything better/worse?
>
> It doesn't make any difference. On the board I've been using, we
> never hit the delay.
>
> So I really don't care about the number of retries and the sleep
> behind them. I made these numbers up, feel free to come up with
> others if it makes you more comfortable, but could we settle this?
>

OK, let's stop the bikeshedding. For both patches:

Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
- --
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJU5JxaAAoJEIOKbhOEIHKi9HYP/jxU75zI6QjNk1yGL7Y3qwEy
M2UYrepXTwgRVRxAIN3nhGqERuBVOJCIVKlb3CUSiq9auNAO8rsRs6JTAossV781
LTUniAA0nIBFbTn/k2Wc6yGQizSy7iJRXu73MrLJrcSFO8JxFFqu04V1EYuZbh5s
fuVpeLJEX8Gfy6gj85ybFs7+wkD/XnENKlnDzD6y/n4ewBC3yDPLNh436hZpEVDO
ky8lYjGPHMQs0yEDcp1rfFejLAmxJ4SY6hVSKAxq3/Bn58S4y3Pgkm4cJtP8nHYe
UN4q9UfOBIHWrQIVbBlViK//n3zyEtaPojDSKUiSvI2+Hmz9+eortlYEvpN1HCP3
Xqy1gzFto9FpP4Wp3KpyF609JNVUeQxAsUQZMXM6yaH2oz35szZhBq2xlIpsyo3C
BDbjaYKFk3hVg+jAE0iitZW8BiZS+WZ/pzX4A8rwQBSTMcbrLTuRV611R4E/nEBL
sfCwDWc1gxDDiM8pMJBGC97gwtHEibJqxES9y+T3LrhxtqPl1kUczHFDPgd3uoVw
fT71PsZBtGUeOu3ysymNPc+mF9b9G9KRHYhSiOjnEIle9yvXh6kaGWv93ZM3RCUG
JASv01+gqb+Bz5ZU3MMU+xjHxeoWBq7KfSWcshEHpD8QCuiZzNZ3yt0dZaYXao+M
YsLlm5s62fDVILb2Q3+d
=MA8V
-----END PGP SIGNATURE-----