[00/11] mtd: rawnand: denali: exec_op(), controller/chip separation, and cleanups

Message ID 1549613335-30319-1-git-send-email-yamada.masahiro@socionext.com
Headers show
Series
  • mtd: rawnand: denali: exec_op(), controller/chip separation, and cleanups
Related show

Message

Masahiro Yamada Feb. 8, 2019, 8:08 a.m.
I took time for the Denali driver to catch up with the latest framework.

 - switch over to exec_op() and remove legacy hooks

 - separate controller/chips

 - various cleanups



Masahiro Yamada (11):
  mtd: rawnand: denali: use nand_chip pointer more for internal
    functions
  mtd: rawnand: denali: refactor syndrome layout handling for raw access
  mtd: rawnand: denali: remove unneeded casts in denali_{read,write}_pio
  mtd: rawnand: denali: switch over to ->exec_op() from legacy hooks
  mtd: rawnand: denali: rename irq_status to irq_stat
  mtd: rawnand: denali: use more precise timeout for
    NAND_OP_WAITRDT_INSTR
  mtd: rawnand: denali: use bool type instead of int where appropriate
  mtd: rawnand: denali_pci: rename goto labels
  mtd: rawnand: denali: decouple controller and NAND chips
  mtd: rawnand: denali: remove DENALI_NR_BANKS macro
  mtd: rawnand: denali: clean up coding style

 .../devicetree/bindings/mtd/denali-nand.txt        |   39 +-
 drivers/mtd/nand/raw/denali.c                      | 1182 ++++++++++----------
 drivers/mtd/nand/raw/denali.h                      |  119 +-
 drivers/mtd/nand/raw/denali_dt.c                   |   98 +-
 drivers/mtd/nand/raw/denali_pci.c                  |   38 +-
 5 files changed, 867 insertions(+), 609 deletions(-)

-- 
2.7.4

Comments

Masahiro Yamada Feb. 8, 2019, 9:49 a.m. | #1
On Fri, Feb 8, 2019 at 5:09 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>

> Implement ->exec_op().

>

> Remove the deprecated select_chip(), read_byte(), write_byte(),

> cmd_ctrl(), and waitfunc().


I also deleted read_buf(), write_buf().

Will fix the log.




-- 
Best Regards
Masahiro Yamada
Miquel Raynal Feb. 8, 2019, 10:05 p.m. | #2
Hi Masahiro,

Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Fri,  8 Feb
2019 17:08:50 +0900:

> Currently, wait_for_completion_timeout() is always passed in the

> hard-coded msec_to_jiffies(1000). There is no specific reason for

> 1000 msec, but I just chose it long enough.

> 

> With the exec_op() conversion, NAND_OP_WAITRDY_INSTR provides more

> precise timeout value, depending on the preceding command. Let's use

> it to bail out earlier in error case.


I'm not sure using 10ms instead of 1000ms is relevant in the below
cases, 10ms is rather short for an IRQ, if your system is under load
you might end up with a timeout, not because the right IRQ did not
fire, but because the handler was not executed yet (it happened to me
in the marvell_nand.c driver recently).

Also, would you mind using a define instead of hardcoding '1000'?
 
> 

> I am still keeping the hard-coded values for other higher level hooks

> such as page_read, page_write, etc. We know the value of tR, tPROG, but

> we have unknowledge about the data transfer speed of the DMA engine.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>



Thanks,
Miquèl
Masahiro Yamada Feb. 11, 2019, 1:26 a.m. | #3
Hi Miquel,

On Sat, Feb 9, 2019 at 7:05 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>

> Hi Masahiro,

>

> Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Fri,  8 Feb

> 2019 17:08:50 +0900:

>

> > Currently, wait_for_completion_timeout() is always passed in the

> > hard-coded msec_to_jiffies(1000). There is no specific reason for

> > 1000 msec, but I just chose it long enough.

> >

> > With the exec_op() conversion, NAND_OP_WAITRDY_INSTR provides more

> > precise timeout value, depending on the preceding command. Let's use

> > it to bail out earlier in error case.

>

> I'm not sure using 10ms instead of 1000ms is relevant in the below

> cases, 10ms is rather short for an IRQ, if your system is under load

> you might end up with a timeout, not because the right IRQ did not

> fire, but because the handler was not executed yet (it happened to me

> in the marvell_nand.c driver recently).



Good point.
Since Linux is not RT-OS, there is no defined worst-case time
until the handler is invoked.


I will add the following to denali_wait_for_irq().

        /* Prolong the IRQ wait time in case the system is under heavy load. */
        timeout_ms += 100;





> Also, would you mind using a define instead of hardcoding '1000'?



I do not think this is worth doing.




> >

> > I am still keeping the hard-coded values for other higher level hooks

> > such as page_read, page_write, etc. We know the value of tR, tPROG, but

> > we have unknowledge about the data transfer speed of the DMA engine.

> >

> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>

>

> Thanks,

> Miquèl




-- 
Best Regards
Masahiro Yamada