Hi Boris,
Le mar. 19 mai 2020 à 19:35, Boris Brezillon
<boris.brezillon@xxxxxxxxxxxxx> a écrit :
On Tue, 19 May 2020 17:10:12 +0200
Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
Le mar. 19 mai 2020 à 17:01, Boris Brezillon
<boris.brezillon@xxxxxxxxxxxxx> a écrit :
> On Tue, 19 May 2020 16:52:27 +0200
> Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
>
>> Hi Boris,
>>
>> Le lun. 18 mai 2020 à 21:24, Boris Brezillon
>> <boris.brezillon@xxxxxxxxxxxxx> a écrit :
>> > On Mon, 18 May 2020 19:50:04 +0200
>> > Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
>> >
>> >> Hi Boris,
>> >>
>> >> Le lun. 18 mai 2020 à 18:56, Boris Brezillon
>> >> <boris.brezillon@xxxxxxxxxxxxx> a écrit :
>> >> > Let's convert the driver to exec_op() to have one less
driver
>> >> relying
>> >> > on the legacy interface.
>> >>
>> >> Great work, thanks for that.
>> >>
>> >> However it does not work :( nand_scan() returns error -145.
>> >
>> > Looks like the R/B signal is inverted. Can you try with the
>> > following diff applied?
>>
>> Still doesn't work properly. I get -ENODEV in nand_detect(), at
the
>> "second ID read did not match..." pr_info().
>>
>> The R/B signal doesn't seem to be the primary cause, if I use
>> nand_soft_waitrdy() it doesn't work any better.
>
> Well, it does solve the ETIMEDOUT issue, so we're one step further
> ;-).
> Can you print the returned ID?
It reads 00/00, so it doesn't seem to be able to read any data.
>>
>> One thing I noticed that jz4780_nemc_assert() is called with
>> assert=true unconditionally, while before it was called with
(ctrl &
>> NAND_NCE), whatever that is. Whether or not that's a problem, I
>> have no
>> idea.
>
> Yes, we really want to assert the CE signal unconditionally here,
but
> maybe we should add a delay after asserting it/before
de-asserting it.
I added some udelay() here and there, unfortunately it didn't change
anything.
Can you try with this diff? It's basically the same as before except
there's an additional fix (s/cmd_offset/addr_offset/ in the address
emission path).
Oops, I'm sorry I didn't even spot that.
With the diff below, it works, so:
Tested-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
Thank you for the patch.
Cheers,
-Paul
--->8---
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
index dcecd54af20b..2668135f2d26 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
@@ -249,6 +249,26 @@ static int ingenic_nand_attach_chip(struct
nand_chip *chip)
return 0;
}
+static int ingenic_nand_gpio_waitrdy(struct gpio_desc *gpiod,
+ unsigned long timeout_ms)
+{
+ /*
+ * Wait until R/B pin indicates chip is ready or timeout
occurs.
+ * +1 below is necessary because if we are now in the last
fraction
+ * of jiffy and msecs_to_jiffies is 1 then we will wait only
that
+ * small jiffy fraction - possibly leading to false timeout.
+ */
+ timeout_ms = jiffies + msecs_to_jiffies(timeout_ms) + 1;
+ do {
+ if (!gpiod_get_value_cansleep(gpiod))
+ return 0;
+
+ cond_resched();
+ } while (time_before(jiffies, timeout_ms));
+
+ return !gpiod_get_value_cansleep(gpiod) ? 0 : -ETIMEDOUT;
+};
+
static int ingenic_nand_exec_instr(struct nand_chip *chip,
struct ingenic_nand_cs *cs,
const struct nand_op_instr *instr)
@@ -265,7 +285,7 @@ static int ingenic_nand_exec_instr(struct
nand_chip *chip,
case NAND_OP_ADDR_INSTR:
for (i = 0; i < instr->ctx.addr.naddrs; i++)
writeb(instr->ctx.addr.addrs[i],
- cs->base + nfc->soc_info->cmd_offset);
+ cs->base + nfc->soc_info->addr_offset);
return 0;
case NAND_OP_DATA_IN_INSTR:
if (instr->ctx.data.force_8bit ||
@@ -294,8 +314,8 @@ static int ingenic_nand_exec_instr(struct
nand_chip *chip,
return nand_soft_waitrdy(chip,
instr->ctx.waitrdy.timeout_ms);
- return nand_gpio_waitrdy(chip, nand->busy_gpio,
-
instr->ctx.waitrdy.timeout_ms);
+ return ingenic_nand_gpio_waitrdy(nand->busy_gpio,
+
instr->ctx.waitrdy.timeout_ms);
default:
break;
}
@@ -322,6 +342,9 @@ static int ingenic_nand_exec_op(struct nand_chip
*chip,
ret = ingenic_nand_exec_instr(chip, cs,
&op->instrs[i]);
if (ret)
break;
+
+ if (op->instrs[i].delay_ns)
+ ndelay(op->instrs[i].delay_ns);
}
jz4780_nemc_assert(nfc->dev, cs->bank, false);
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
