Re: [PATCH] mtd: rawnand: ingenic: Convert the driver to exec_op()

[Paul Cercueil <paul@crapouillou.net>]

Le lun. 18 mai 2020 à 21:35, Boris Brezillon 
<boris.brezillon@collabora.com> a écrit :
> On Mon, 18 May 2020 21:24:22 +0200
> Boris Brezillon <boris.brezillon@collabora.com> wrote:
> 
>>  On Mon, 18 May 2020 19:50:04 +0200
>>  Paul Cercueil <paul@crapouillou.net> wrote:
>> 
>>  > Hi Boris,
>>  >
>>  > Le lun. 18 mai 2020 à 18:56, Boris Brezillon
>>  > <boris.brezillon@collabora.com> 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"? ;)

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.

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..9206792629a8 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)
>>  @@ -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/