Le mar. 19 mai 2020 à 19:28, Boris Brezillon
<boris.brezillon@xxxxxxxxxxxxx> a écrit :
On Tue, 19 May 2020 17:04:37 +0200
Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
Le lun. 18 mai 2020 à 21:35, Boris Brezillon
<boris.brezillon@xxxxxxxxxxxxx> a écrit :
> On Mon, 18 May 2020 21:24:22 +0200
> Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote:
>
>> 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?
>
> I checked the DT, and the GPIO is indeed declared GPIO_ACTIVE_LOW,
> which explain why the test is inverted. Because of DT ABI
stability it
> might not be an option to change that, but the signal should
actually
> be
> declared GPIO_ACTIVE_HIGH.
It depends what you consider what is the active state, is it when
"busy" or "ready"? ;)
True, this should really be documented in the generic binding part. As
you probably guessed from this discussion, all other drivers (and the
framework) is assuming "ready" is the state we're monitoring, so it's
effectively active high.
I can fix it in the devicetree, and the driver would return
(gpiod_get_value_cansleep(gpiod) ^ gpiod_is_active_low(gpiod)) for
compatibility with the old devicetree.
Or you could read the raw value (gpiod_get_raw_value_cansleep()),
but that still means you can't move away from the old semantics
without
breaking the existing DT with the erroneous active-low. I mean,
active-low is still valid if someone has the R/B signal inverted,
but you can't discriminate when it's valid and when it's not.
I guess having a custom helper, and documenting that the active state
for ingenic is BUSY would be okay. Unless you'd be willing to break
the backward compat for the only board that has a rb-gpios property
defined.
What I suggest, in the probe:
if (of_machine_is_compatible("qi,lb60") &&
gpiod_is_active_low(nand->busy_gpio)) {
gpiod_toggle_active_low(nand->busy_gpio);
}
Then it's backward-compatible, would allow me to fix the rb-gpios in
devicetree, and wouldn't require a custom nand_gpio_waitrdy() function.
Cheers,
-Paul
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
