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

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

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.

> 
> --->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/