From: Dan Carpenter <dan.carpenter@oracle.com>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Jheng-Jhong Wu <goodwater.wu@gmail.com>,
Palle Christensen <palle.christensen@microsemi.com>,
devel@driverdev.osuosl.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org,
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.
Date: Mon, 6 Aug 2018 15:01:37 +0300 [thread overview]
Message-ID: <20180806120137.s2lenmmhynw3zndz@mwanda> (raw)
In-Reply-To: <20180806134648.2976de9e@bbrezillon>
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@oracle.com> 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@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:
>
> 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
next prev parent reply other threads:[~2018-08-06 12:02 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-01 3:24 Jheng-Jhong Wu
2018-08-01 6:01 ` Boris Brezillon
2018-08-01 12:05 ` Dan Carpenter
2018-08-01 13:55 ` Miquel Raynal
2018-08-01 14:04 ` Dan Carpenter
2018-08-06 1:49 ` Jheng-Jhong Wu
2018-08-06 8:59 ` Dan Carpenter
2018-08-06 11:46 ` Boris Brezillon
2018-08-06 12:01 ` Dan Carpenter [this message]
2018-08-06 12:18 ` Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180806120137.s2lenmmhynw3zndz@mwanda \
--to=dan.carpenter@oracle.com \
--cc=arunrasppi@gmail.com \
--cc=boris.brezillon@bootlin.com \
--cc=devel@driverdev.osuosl.org \
--cc=goodwater.wu@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=palle.christensen@microsemi.com \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®