Re: [PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit arguments and variables used for addressing.

[Dan Carpenter]

On Mon, Aug 06, 2018 at 01:46:48PM +0200, Boris Brezillon wrote:
> Hi Dan,
> 
> On Wed, 1 Aug 2018 15:05:51 +0300
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> 
> > 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@xxxxxxxxx>  
> > 
> > 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:
> 
> Yes, -1 means "don't issue the row/page address cycles", though I
> don't think page can be -1 for NAND_CMD_READ{1,0} commands.
> 

It sure looks like it can be in nand_exit_status_op()...

> Anyway, if you want this patch merged to fix a static checker warning,
> I'm fine with that. In any case, I still plan to send a patch removing
> this driver for v4.20, so, anyone using this driver should start
> testing the new SPI NAND driver (drivers/mtd/nand/spi) and tweak/fix
> the new implementation if needed.

I don't think we should make the code more complicated than necessary
just to make static checkers happy.  When you say "this driver", you
mean the staging driver?  In that case, there is no need to revert
commit 6efb21d6d0e7 ("staging:mt29f_spinand: MT29F2G failing as only 16
bits used for addressing.").

regards,
dan carpenter