Re: [PATCH] mtd: spinand: Add support for GigaDevice GD5F1GQ4UC

Stefan Roese
2019-01-24T08:52:43Z

On 24.01.19 09:14, Boris Brezillon wrote:
> On Thu, 24 Jan 2019 09:00:43 +0100
> Stefan Roese <sr@denx.de> wrote:
>
>> On 24.01.19 08:50, Boris Brezillon wrote:
>>> On Thu, 24 Jan 2019 08:35:32 +0100
>>> Stefan Roese <sr@denx.de> wrote:
>>>
>>>> On 23.01.19 13:57, Boris Brezillon wrote:
>>>>> On Wed, 23 Jan 2019 13:40:50 +0100
>>>>> Boris Brezillon <bbrezillon@kernel.org> wrote:
>>>>>
>>>>>>> This definitely does look better. I assume that we are we on the
>>>>>>> right track now?
>>>>>>
>>>>>> Yep, and it confirms the ECC caps => 8bits/512bytes. Will send a proper
>>>>>> commit for the fix I did and Cc you so you can add your
>>>>>> Tested-by/Reviewed-by.
>>>>>
>>>>> Oh, looks like a side-effect of migrating to the dirmap approach
>>>>> (merged in nand/next [1]) is that this bug does not exist. Can you test
>>>>> the nand/next branch and let me know if it still works?
>>>>>
>>>>> [1]http://git.infradead.org/linux-mtd.git/shortlog/refs/heads/nand/next
>>>>
>>>> Unfortunately this does not seem to work. I was unable to boot my
>>>> platform from this branch directly so I rebased all MTD/NAND related
>>>> patches on top of the latest kernel.org tree for this.
>>>
>>> You mean linux-next?
>>
>> No. I can try linux-next as well if necessary.
>
> So which branch/tag is it based on?

Linus's tree "master" (5.0.0-rc3) with some mostly platform
patches applied on top.

From your other mail:

> Can you find out which layer (spinand, spi-mem or the spi driver) is
> returning this -EIO?

Sure. With this small debug patch applied:

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 52f17fc42daa..80fa234ecbdd 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -298,9 +298,11 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
while (nbytes) {
ret = spi_mem_dirmap_read(wdesc, column, nbytes,
spinand->databuf + column);
+ printk("%s (%d): ret=%d nbytes=%d\n", __func__, __LINE__, ret, nbytes); // test-only
if (!ret || ret > nbytes)
ret = -EIO;

+ printk("%s (%d): ret=%d nbytes=%d\n", __func__, __LINE__, ret, nbytes); // test-only
if (ret < 0)
return ret;

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 5217a5628be2..964ba3dc4e64 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -573,8 +573,10 @@ ssize_t spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
struct spi_controller *ctlr = desc->mem->spi->controller;
ssize_t ret;

+ printk("%s (%d)\n", __func__, __LINE__); // test-only
if (desc->info.op_tmpl.data.dir != SPI_MEM_DATA_IN)
return -EINVAL;
+ printk("%s (%d)\n", __func__, __LINE__); // test-only

if (!len)
return 0;

I get this output:

root@mt7688:~# ./nandbiterrs /dev/mtd5 -i
incremental bite[ 66.598843] spi_mem_dirmap_read (576)
rrors test
[ 66.603779] spinand_write_to_cache_op (301): ret=-22 nbytes=2176
[ 66.610912] spinand_write_to_cache_op (305): ret=-5 nbytes=2176
libmtd: error!: cannot write 2048 bytes to mtd5 (eraseblock 0, offset 0)
error 5 (Input/output error)
Failed to write page 0 in block 0
ERROR: 1 | root@mt7688:~# dmesg -c
[ 66.598843] spi_mem_dirmap_read (576)
[ 66.603779] spinand_write_to_cache_op (301): ret=-22 nbytes=2176
[ 66.610912] spinand_write_to_cache_op (305): ret=-5 nbytes=2176

So spi_mem_dirmap_read() returns -EINVAL to spinand_write_to_cache_op()
which then returns -EIO.

Thanks,
Stefan