1. 20 Jun, 2017 8 commits
    • Masahiro Yamada's avatar
      mtd: nand: denali: propagate page to helpers via function argument · 2291cb89
      Masahiro Yamada authored
      This driver stores the currently addressed page into denali->page,
      which is later read out by helper functions.  While I am tackling on
      this driver, I often missed to insert "denali->page = page;" where
      needed.  This makes page_read/write callbacks to get access to a
      wrong page, which is a bug hard to figure out.
      
      Instead, I'd rather pass the page via function argument because the
      compiler's prototype checks will help to detect bugs.
      
      For the same reason, propagate dma_addr to the DMA helpers instead
      of denali->buf.dma_buf .
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      2291cb89
    • Masahiro Yamada's avatar
      mtd: nand: denali: use interrupt instead of polling for bank reset · d49f5790
      Masahiro Yamada authored
      The current bank reset implementation polls the INTR_STATUS register
      until interested bits are set.  This is not good because:
      
      - polling simply wastes time-slice of the thread
      
      - The while() loop may continue eternally if no bit is set, for
        example, due to the controller problem.  The denali_wait_for_irq()
        uses wait_for_completion_timeout(), which is safer.
      
      We can use interrupt by moving the denali_reset_bank() call below
      the interrupt setup.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      d49f5790
    • Masahiro Yamada's avatar
      mtd: nand: denali: fix bank reset function to detect the number of chips · f486287d
      Masahiro Yamada authored
      The nand_scan_ident() iterates over maxchips, and calls nand_reset()
      for each.  This driver currently passes the maximum number of banks
      (=chip selects) supported by the controller as maxchips.  So, maxchips
      is typically 4 or 8.  Usually, less number of NAND chips are connected
      to the controller.
      
      This can be a problem for ONFi devices.  Now, this driver implements
      ->setup_data_interface() hook, so nand_setup_data_interface() issues
      Set Features (0xEF) command, which waits until the chip returns R/B#
      response.  If no chip there, we know it never happens, but the driver
      still ends up with waiting for a long time.  It will finally bail-out
      with timeout error and the driver will work with existing chips, but
      unnecessary wait will give a bad user experience.
      
      The denali_nand_reset() polls the INTR__RST_COMP and INTR__TIME_OUT
      bits, but they are always set even if not NAND chip is connected to
      that bank.  To know the chip existence, INTR__INT_ACT bit must be
      checked; this flag is set only when R/B# is toggled.  Since the Reset
      (0xFF) command toggles the R/B# pin, this can be used to know the
      actual number of chips, and update denali->max_banks.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      f486287d
    • Masahiro Yamada's avatar
      mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc · fa6134e5
      Masahiro Yamada authored
      The NAND_CMD_SET_FEATURES support is missing from denali_cmdfunc().
      We also see /* TODO: Read OOB data */ comment.
      
      It would be possible to add more commands along with the current
      implementation, but having ->cmd_ctrl() seems a better approach from
      the discussion with Boris [1].
      
      Rely on the default ->cmdfunc() from the framework and implement the
      driver's own ->cmd_ctrl().
      
      This transition also fixes NAND_CMD_STATUS and NAND_CMD_PARAM handling.
      NAND_CMD_STATUS was just faked by the register read, so the only valid
      bit was the WP bit.  NAND_CMD_PARAM was completely broken; not only the
      command sent on the bus was NAND_CMD_STATUS instead of NAND_CMD_PARAM,
      but also the driver was only reading 8 bytes, while the parameter page
      contains several hundreds of bytes.
      
      Also add ->write_byte(), which is needed for write direction commands,
      ->read/write_buf(16), which will be used some commits later.
      ->read_word() is not used for now, but the core may call it in the
      future.
      
      Now, this driver can drop nand_onfi_get_set_features_notsupp().
      
      [1] https://lkml.org/lkml/2017/3/15/97Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      fa6134e5
    • Masahiro Yamada's avatar
      mtd: nand: denali: rework interrupt handling · c19e31d0
      Masahiro Yamada authored
      Simplify the interrupt handling and fix issues:
      
      - The register field view of INTR_EN / INTR_STATUS is different
        among IP versions.  The global macro DENALI_IRQ_ALL is hard-coded
        for Intel platforms.  The interrupt mask should be determined at
        run-time depending on the running platform.
      
      - wait_for_irq() loops do {} while() until interested flags are
        asserted.  The logic can be simplified.
      
      - The spin_lock() guard seems too complex (and suspicious in a race
        condition if wait_for_completion_timeout() bails out by timeout).
      
      - denali->complete is reused again and again, but reinit_completion()
        is missing.  Add it.
      
      Re-work the code to make it more robust and easier to handle.
      
      While we are here, also rename the jump label "failed_req_irq" to
      more appropriate "disable_irq".
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      c19e31d0
    • Masahiro Yamada's avatar
      mtd: nand: denali: handle timing parameters by setup_data_interface() · 1bb88666
      Masahiro Yamada authored
      Handling timing parameters in a driver's own way should be avoided
      because it duplicates efforts of drivers/mtd/nand/nand_timings.c
      Besides, this driver hard-codes Intel specific parameters such as
      CLK_X=5, CLK_MULTI=4.  Taking a certain device (Samsung K9WAG08U1A)
      into account by get_samsung_nand_para() is weird as well.
      
      Now, the core framework provides .setup_data_interface() hook, which
      handles timing parameters in a generic manner.
      
      While I am working on this, I found even more issues in the current
      code, so fixed the following as well:
      
      - In recent IP versions, WE_2_RE and TWHR2 share the same register.
        Likewise for ADDR_2_DATA and TCWAW, CS_SETUP_CNT and TWB.  When
        updating one, the other must be masked.  Otherwise, the other will
        be set to 0, then timing settings will be broken.
      
      - The recent IP release expanded the ADDR_2_DATA to 7-bit wide.
        This register is related to tADL.  As commit 74a332e7 ("mtd:
        nand: timings: Fix tADL_min for ONFI 4.0 chips") addressed, the
        ONFi 4.0 increased the minimum of tADL to 400 nsec.  This may not
        fit in the 6-bit ADDR_2_DATA in older versions.  Check the IP
        revision and handle this correctly, otherwise the register value
        would wrap around.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      1bb88666
    • Masahiro Yamada's avatar
      mtd: nand: denali: remove unneeded find_valid_banks() · 959e9f2a
      Masahiro Yamada authored
      The function find_valid_banks() issues the Read ID (0x90) command,
      then compares the first byte (Manufacturer ID) of each bank with
      the one of bank0.
      
      This is equivalent to what nand_scan_ident() does.  The number of
      chips is detected there, so this is unneeded.
      
      What is worse for find_valid_banks() is that, if multiple chips are
      connected to INTEL_CE4100 platform, it crashes the kernel by BUG().
      This is what we should avoid.  This function is just harmful and
      unneeded.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      959e9f2a
    • Masahiro Yamada's avatar
      mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS · b21ff825
      Masahiro Yamada authored
      The denali_cmdfunc() actually does nothing valuable for
      NAND_CMD_{PAGEPROG,READ0,SEQIN}.
      
      For NAND_CMD_{READ0,SEQIN}, it copies "page" to "denali->page", then
      denali_read_page(_raw) compares them just for the sanity check.
      (Inconsistently, this check is missing from denali_write_page(_raw).)
      
      The Denali controller is equipped with high level read/write interface,
      so let's skip unneeded call of cmdfunc().
      
      If NAND_ECC_CUSTOM_PAGE_ACCESS is set, nand_write_page() will not
      call ->waitfunc hook.  So, ->write_page(_raw) hooks should directly
      return -EIO on failure.  The error handling of page writes will be
      much simpler.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Signed-off-by: default avatarBoris Brezillon <boris.brezillon@free-electrons.com>
      b21ff825
  2. 13 Jun, 2017 1 commit
  3. 10 Jun, 2017 11 commits
  4. 01 Jun, 2017 20 commits