From: <Tudor.Ambarus@microchip.com>
To: <michael@walle.cc>
Cc: richard@nod.at, miquel.raynal@bootlin.com,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
vigneshr@ti.com
Subject: Re: [PATCH] mtd: spi-nor: Add support for w25qNNjwim
Date: Mon, 13 Jan 2020 09:06:46 +0000 [thread overview]
Message-ID: <4050087.dyKUiXJtgz@localhost.localdomain> (raw)
In-Reply-To: <9d39be0f45f4c8e087b269f0c802ed6b@walle.cc>
Hi, Michael,
On Sunday, January 12, 2020 1:16:12 AM EET Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Hi Tudor,
>
> Am 2020-01-11 15:19, schrieb Tudor.Ambarus@microchip.com:
> > Hi, Michael,
> >
> > On Saturday, January 4, 2020 12:34:23 AM EET Michael Walle wrote:
> >> Add support for the Winbond W25QnnJW-IM flashes. These have a
> >> programmable QE bit. There are also the W25QnnJW-IQ variant which
> >> shares
> >> the ID with the W25QnnFW parts. These have the QE bit hard strapped to
> >> 1, thus don't support hardware write protection.
> >
> > There are few flavors of hw write protection supported by this flash,
> > the Q
> > version does not disable them all. How about saying just that the /HOLD
> > function is disabled?
>
> I don't get your point here ;) My understanding is that HOLD# and WP#
> will
> be disabled. Thus there is no "hardware write protection". What other hw
> write protection do you have in mind?
Time delay write disable after Power-up for example.
>
> > When we receive new flash id patches, we ask the contributors to
> > specify if
> > they test the flash, in which modes (single, quad), and with which
> > controller.
> > Ideally all the flash's flags should be tested, but there are cases in
> > which
> > the controllers do not support quad read for example, and we accept the
> > patches even if tested in single read mode. SPI_NOR_HAS_LOCK and
> > SPI_NOR_HAS_TB must be tested as well.
> >
> > Even if the patches are rather simple, we ask for this to be sure that
> > we
> > don't add a flash that is broken from day one. So, would you please
> > tell us
> > what flashes did you test, what flags, and with which controller?
>
> Ok will add that to the commit message. Just to make sure. I've only
> tested the
> 32mbit part. So is it still ok to include all other flashes of this
> family?
No, just the ones that you can test please.
>
> For now. tested with the NXP FlexSPI, single and dual (no quad since we
> are
> using the write protection feature and IO2 and IO3 are not connected to
> the
> CPU). So write protection is also tested. I will retest the TB bit.
Great, thanks.
>
> >> Signed-off-by: Michael Walle <michael@walle.cc>
> >> ---
> >>
> >> drivers/mtd/spi-nor/spi-nor.c | 22 ++++++++++++++++++++++
> >> 1 file changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c
> >> b/drivers/mtd/spi-nor/spi-nor.c
> >> index addb6319fcbb..3fa8a81bdab0 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -2627,6 +2627,11 @@ static const struct flash_info spi_nor_ids[] =
> >> {
> >>
> >> SECT_4K | SPI_NOR_DUAL_READ |
> >
> > SPI_NOR_QUAD_READ |
> >
> >> SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> >>
> >> },
> >>
> >> + {
> >> + "w25q16jwim", INFO(0xef8015, 0, 64 * 1024, 32,
> >
> > "i" is for the temperature range, which is not a fixed characteristic.
> > Usually
> > there are flashes with the same jedec-id, but with different
> > temperature
> > ranges. Let's drop the "i" and rename it to "w25q16jwm"
>
> Only that there is no flash with that part name :( according to the
> datasheet
The datasheet describes the W25Q32JW flash (check the first page of the
datasheet). There are two flavors of this flash, each with its own jedec-id: Q
version uses 156016h, M 158016h. We should name this flashes as "w25q32jwq"
and "w25q32jwm". Please notice that I skipped intentionally the "i" that
stands for temperature range. Manufacturers can provide better temperature
ranges for the same flash without changing the jedec-id. See this datasheet:
https://ro.mouser.com/datasheet/2/949/w25q128jv_revf_03272018_plus-1489608.pdf
> there is only this one temp range available. From what I've seen for now
> (esp
> looking at the macronix parts) it seems to first come first serve ;)
> That being said, I don't insist on keeping that name, I'm fine with any
> name,
you should be fine just with the name that best describes the flash :)
> since I've learned you cannot rely on it in any way. Eg. the w25q32jwiq
> will
> be discovered as w25q32dw. Or some Macronix flashes will be discovered
> as
> ancient ones.
Would you please study what's wrong with these names and provide a patch to
fix them?
>
> Btw. is renaming the flashes also considered a backwards incomaptible
> change?
No, we can fix the names.
> And can there be two flashes with the same name? Because IMHO it would
> be
I would prefer that we don't. Why would you have two different jedec-ids with
the same name?
Cheers,
ta
> better to just have the name "w25q16" regardless whether its an FW/JW/JV
> etc.
> It's better to show an ambiguous name than a wrong name.
>
> -michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2020-01-13 9:07 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-03 22:34 [PATCH] mtd: spi-nor: Add support for w25qNNjwim Michael Walle
2020-01-11 14:19 ` Tudor.Ambarus
2020-01-11 23:16 ` Michael Walle
2020-01-13 9:06 ` Tudor.Ambarus [this message]
2020-01-13 10:07 ` Michael Walle
2020-01-13 13:15 ` Michael Walle
2020-01-19 7:13 ` Tudor.Ambarus
2020-01-19 22:24 ` Michael Walle
2020-01-20 11:03 ` Tudor.Ambarus
2020-01-20 15:55 ` Michael Walle
2020-01-21 18:40 ` Tudor.Ambarus
2020-01-21 23:28 ` Michael Walle
2020-01-22 6:48 ` Tudor.Ambarus
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=4050087.dyKUiXJtgz@localhost.localdomain \
--to=tudor.ambarus@microchip.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=michael@walle.cc \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=vigneshr@ti.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;
as well as URLs for NNTP newsgroup(s).