Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 1 Aug 2018 15:05:51 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Jheng-Jhong Wu <goodwater.wu@gmail.com>,
        Palle Christensen <palle.christensen@microsemi.com>
Cc:     devel@driverdev.osuosl.org,
        Boris Brezillon <boris.brezillon@bootlin.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org,
        Palle Christensen <palle.christensen@microsemi.com>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        Arun Nagendran <arunrasppi@gmail.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit
 arguments and variables used for addressing.
Message-ID: <20180801120551.ju6zhxhoef6w5zim@mwanda>
References: <1533093861-9761-1-git-send-email-goodwater.wu@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1533093861-9761-1-git-send-email-goodwater.wu@gmail.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Aug 01, 2018 at 11:24:19AM +0800, Jheng-Jhong Wu wrote:
> For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits
> are necessary to address the correct page. The driver sets the address for
> more than 16 bits, but it uses 16-bit arguments and variables (these are
> page_id, block_id, row) to do address operations. Obviously, these
> arguments and variables cannot deal with more than 16-bit address.
> 
> Signed-off-by: Jheng-Jhong Wu <goodwater.wu@gmail.com>

This seems reasonable...  It would be needed to make commit 6efb21d6d0e7
("staging:mt29f_spinand: MT29F2G failing as only 16 bits used for
addressing.") work.  It also fixes a static checker warning.

My only concern is that the mtd/nand code seems to use -1 as a magical
page_id.  For example:


  2069  /**
  2070   * nand_exit_status_op - Exit a STATUS operation
  2071   * @chip: The NAND chip
  2072   *
  2073   * This function sends a READ0 command to cancel the effect of the STATUS
  2074   * command to avoid reading only the status until a new read command is sent.
  2075   *
  2076   * This function does not select/unselect the CS line.
  2077   *
  2078   * Returns 0 on success, a negative error code otherwise.
  2079   */
  2080  int nand_exit_status_op(struct nand_chip *chip)
  2081  {
  2082          struct mtd_info *mtd = nand_to_mtd(chip);
  2083  
  2084          if (chip->exec_op) {
  2085                  struct nand_op_instr instrs[] = {
  2086                          NAND_OP_CMD(NAND_CMD_READ0, 0),
  2087                  };
  2088                  struct nand_operation op = NAND_OPERATION(instrs);
  2089  
  2090                  return nand_exec_op(chip, &op);
  2091          }
  2092  
  2093          chip->cmdfunc(mtd, NAND_CMD_READ0, -1, -1);
                                   ^^^^^^^^^^^^^^      ^^
  2094  
  2095          return 0;
  2096  }
  2097  EXPORT_SYMBOL_GPL(nand_exit_status_op);

I'm not sure if this affect spinand_read_page() etc.

regards,
dan carpenter