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

[Paul Cercueil <paul@crapouillou.net>]

Hi Boris,

Le lun. 18 mai 2020 à 21:24, Boris Brezillon 
<boris.brezillon@collabora.com> a écrit :
> 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?

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.

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.

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