From: <Tudor.Ambarus@microchip.com>
To: <tkuw584924@gmail.com>, <michael@walle.cc>
Cc: <linux-mtd@lists.infradead.org>, <miquel.raynal@bootlin.com>,
<richard@nod.at>, <vigneshr@ti.com>, <p.yadav@ti.com>,
<Bacem.Daassi@infineon.com>, <Takahiro.Kuwano@infineon.com>
Subject: Re: [PATCH v15 6/8] mtd: spi-nor: Retain nor->addr_width at 4BAIT parse
Date: Fri, 22 Jul 2022 07:45:21 +0000 [thread overview]
Message-ID: <70ff017e-cff3-0dc2-6624-5dcc27fc67de@microchip.com> (raw)
In-Reply-To: <070bfe6a-00e8-1c59-c9db-52d249dfbcfe@microchip.com>
On 7/22/22 10:38, Tudor Ambarus - M18064 wrote:
> On 7/22/22 09:08, Tudor.Ambarus@microchip.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 7/22/22 08:11, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 7/22/2022 2:06 PM, Tudor.Ambarus@microchip.com wrote:
>>>> On 7/22/22 07:46, Takahiro Kuwano wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 7/22/2022 1:20 PM, Tudor.Ambarus@microchip.com wrote:
>>>>>> On 7/22/22 07:00, Takahiro Kuwano wrote:
>>>>>>
>>>>>> Good morning, Takahiro!
>>>>>>
>>>>> Good morning, Tudor!
>>>>>
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>
>>>>>>> On 7/22/2022 1:06 AM, Tudor.Ambarus@microchip.com wrote:
>>>>>>>> On 5/23/22 10:49, Michael Walle wrote:
>>>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>>>
>>>>>>>>> Am 2022-05-14 05:51, schrieb Takahiro Kuwano:
>>>>>>>>>> On 5/13/2022 6:40 PM, Michael Walle wrote:
>>>>>>>>>>> [btw the subject still has the old name of the addr_width]
>>>>>>>>>>>
>>>>>>>>>> Yes, it must be fixed in next rev.
>>>>>>>>>>
>>>>>>>>>>> Am 2022-05-13 03:26, schrieb Takahiro Kuwano:
>>>>>>>>>>>> On 5/13/2022 7:14 AM, Michael Walle wrote:
>>>>>>>>>>>>> Am 2022-05-10 00:10, schrieb tkuw584924@gmail.com:
>>>>>>>>>>>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In 4BAIT parse, keep nor->params->addr_width because it may be used
>>>>>>>>>>>>>> as
>>>>>>>>>>>>>> current address mode in SMPT parse later on.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Mh I'm not sure this is needed at all.
>>>>>>>>>>>>>
>>>>>>>>>>>>> SFDP spec says
>>>>>>>>>>>>> Variable address length (the current setting of the address
>>>>>>>>>>>>> length mode defines the address length)
>>>>>>>>>>>>>
>>>>>>>>>>>>> and
>>>>>>>>>>>>> When the length is defined as variable, the software or hardware
>>>>>>>>>>>>> controlling the memory is aware of the address length mode last
>>>>>>>>>>>>> set in the memory device and this same length of address.
>>>>>>>>>>>>>
>>>>>>>>>>>>> We don't set any address mode until all the SFDP parsing is
>>>>>>>>>>>>> over. Therefore we should always be in 3 byte mode, no?
>>>>>>>>>>>>>
>>>>>>>>>>>> Actually there are some devices that have variable address length but
>>>>>>>>>>>> 4 byte mode by default (I will work on those devices after this
>>>>>>>>>>>> series
>>>>>>>>>>>> is settled). To support such case, I prefer to use
>>>>>>>>>>>> params->addr_nbytes
>>>>>>>>>>>> as current address mode so that I can fix it in post_bfpt_fixup()
>>>>>>>>>>>> hook.
>>>>>>>>>>>
>>>>>>>>>>> Are there public datasheets available? So these devices have a 3 byte
>>>>>>>>>> I will send datasheets to you in another email. At this point, only
>>>>>>>>>> summary datasheet is available in website.
>>>>>>>>>>
>>>>>>>>>>> and a 4 byte mode, but after reset, they are in the 4 byte mode? Looks
>>>>>>>>>> Yes.
>>>>>>>>>>
>>>>>>>>>>> like it should be fixed in a different way. I'm not sure the "current
>>>>>>>>>>> mode" handling is correct.
>>>>>>>>>>>
>>>>>>>>>> Yes, we may want to introduce a new flag like SPI_NOR_4BAM_DEFAULT and
>>>>>>>>>> check
>>>>>>>>>> the flag in BFPT parse. Once I send another series, please review.
>>>>>>>>>>
>>>>>>>>>>> We need to differentiate between the mode the flash currently is using
>>>>>>>>>>> (nor->addr_nbytes) and the mode parsed by SFDP (params->addr_nbytes).
>>>>>>>>>>>
>>>>>>>>>> The flash's address mode affects the address length of Non-4B opcodes,
>>>>>>>>>> including read/write any register ops used in SMPT parse and Infineon
>>>>>>>>>> (spansion) specific hooks.
>>>>>>>>>>
>>>>>>>>>> The 4B opcodes always take address length of 4 regardless of flash's
>>>>>>>>>> address mode. In these Infineon chips, 4B opcodes for read/program/
>>>>>>>>>> erase are available and 4BAIT advertises them. We don't have to enter
>>>>>>>>>> 4 byte address mode for read/program/erase.
>>>>>>>>>
>>>>>>>>> btw. this is a pity. you are using the stateless 4b opcodes but
>>>>>>>>> then you don't provide stateless opcodes for the read any register
>>>>>>>>> op :/
>>>>>>>>>
>>>>>>>>>> So, I think we need to differentiate between address length for
>>>>>>>>>> read/program/erase and flash's default address mode.
>>>>>>>>>
>>>>>>>>> Or we keep them in sync. E.g. switch to 4bytes mode if we are
>>>>>>>>> using the 4 byte. Granted, that sounds like a hack :)
>>>>>>>>>
>>>>>>>>>> Obviously we are using nor->addr_nbytes as address length for read/
>>>>>>>>>> program/erase and should keep this usage.
>>>>>>>>>
>>>>>>>>> Yes, I wasn't aware that we actually two different runtime
>>>>>>>>> parameters:
>>>>>>>>> - the read/program/erase address width, also used with the
>>>>>>>>> 4b opcodes
>>>>>>>>> - internal mode 3b/4b. Up until now, this wasn't an issue
>>>>>>>>> because either the mode was switched or the 4b opcodes
>>>>>>>>> were used. So this was mutually exclusive. Now we have
>>>>>>>>> flashes which uses 4b opcodes _and_ we need the state
>>>>>>>>> of the internal mode.
>>>>>>>>>
>>>>>>>>> I can't think of a good solution for now. Need to think
>>>>>>>>> more about this, but I'm pretty busy at the moment.
>>>>>>>>> What I think is clear is that we need two different modes
>>>>>>>>> here in the spi_nor struct. nor->addr_nbytes for the
>>>>>>>>> read/program/erase opcodes and nor->address_mode or similar
>>>>>>>>> which tracks the SPI flash's internal address mode.
>>>>>>>>
>>>>>>>> Hi, Takahiro,
>>>>>>>>
>>>>>>>> Can we determine the flash's internal address mode by querying
>>>>>>>> the flash at run-time? Is this possible on Semper flashes?
>>>>>>>>
>>>>>>> CFR2V[7] has current address mode, but to read that, we need to
>>>>>>> issue Read Any Register which address length relies on current
>>>>>>> address mode. Chicken-and-egg...
>>>>>>>
>>>>>>
>>>>>> I see. What happens if we issue the Read Any Register command with
>>>>>> the wrong address internal mode? Will I read just 0xff?
>>>>>> For example try reading CFR2V[7] using addr_nbytes of value 3, and
>>>>>> if it fails, to read it again but this time using addr_nbytes of
>>>>>> value 4.
>>>>>>
>>>>> It's undetermined. In case we issue Read Any Register with 3B address,
>>>>> the host controller outputs 4 bytes (opcode + address) then inputs
>>>>> 1 byte. If the device is in 4B address mode at this time, the host
>>>>> controller inputs the 1 byte while device is still waits for LSB of
>>>>> address so IO is not driven by device.
>>>>>
>>>>
>>>> So if the IO lines are floating, we'll "read" whatever is indicated
>>>> by the IOs at that specific moment of time, right?
>>>>
>>> Yes, right.
>>>
>>>> If so, we're forced to statically specify the default internal address
>>>> mode.
>>>>
>>> Yes.
>> OK, let me scratch something.
>>
>
> Would you please send me the datasheet for Semper flashes?
> I noticed that the addr mode is controlled via a volatile conf register
> in your case, but there are flashes like w25q256jv that control the
> addr mode via a non-volatile register, so only a default value will
> not empower flashes with NV register conf to actually toggle that bit.
> We may introduce a dt prop for the NV case when needed, the disadvantage
> being that the generic jedec,spi-nor will not be so generic, as it will
> have a fixed default NV addr mode specified in dt.
>
Oh, but winbond uses a dedicated RDSR3 opcode and doesn't need addr_nbytes,
so it's safe. It looks like we can use an addr_mode_nbytes after all.
--
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2022-07-22 7:45 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-09 22:10 [PATCH v15 0/8] mtd: spi-nor: Add support for Infineon s25hl-t/s25hs-t tkuw584924
2022-05-09 22:10 ` [PATCH v15 1/8] mtd: spi-nor: s/addr_width/addr_nbytes tkuw584924
2022-05-12 21:01 ` Michael Walle
2022-05-31 11:13 ` Pratyush Yadav
2022-05-09 22:10 ` [PATCH v15 2/8] mtd: spi-nor: core: Shrink the storage size of the flash_info's addr_nbytes tkuw584924
2022-05-12 21:22 ` Michael Walle
2022-05-31 11:14 ` Pratyush Yadav
2022-05-09 22:10 ` [PATCH v15 3/8] mtd: spi-nor: sfdp: Clarify that nor->read_{opcode, dummy} are uninitialized tkuw584924
2022-05-12 21:33 ` Michael Walle
2022-05-12 21:38 ` Michael Walle
2022-05-31 11:18 ` Pratyush Yadav
2022-05-09 22:10 ` [PATCH v15 4/8] mtd: spi-nor: Do not change nor->addr_nbytes at SFDP parsing time tkuw584924
2022-05-12 21:45 ` Michael Walle
2022-05-31 11:30 ` Pratyush Yadav
2022-07-21 14:32 ` Tudor.Ambarus
2022-07-21 15:02 ` Tudor.Ambarus
2022-05-09 22:10 ` [PATCH v15 5/8] mtd: spi-nor: core: Couple the number of address bytes with the address mode tkuw584924
2022-05-12 22:07 ` Michael Walle
2022-05-09 22:10 ` [PATCH v15 6/8] mtd: spi-nor: Retain nor->addr_width at 4BAIT parse tkuw584924
2022-05-12 22:14 ` Michael Walle
2022-05-13 1:26 ` Takahiro Kuwano
2022-05-13 9:40 ` Michael Walle
2022-05-14 3:51 ` Takahiro Kuwano
2022-05-18 6:04 ` Takahiro Kuwano
2022-05-18 8:35 ` Michael Walle
2022-05-18 9:12 ` Takahiro Kuwano
2022-05-23 7:49 ` Michael Walle
2022-05-23 9:56 ` Takahiro Kuwano
2022-06-03 9:33 ` Takahiro Kuwano
2022-07-21 16:06 ` Tudor.Ambarus
2022-07-22 4:00 ` Takahiro Kuwano
2022-07-22 4:20 ` Tudor.Ambarus
2022-07-22 4:31 ` Vanessa Page
2022-07-22 4:46 ` Takahiro Kuwano
2022-07-22 5:06 ` Tudor.Ambarus
2022-07-22 5:11 ` Takahiro Kuwano
2022-07-22 6:08 ` Tudor.Ambarus
2022-07-22 7:38 ` Tudor.Ambarus
2022-07-22 7:45 ` Tudor.Ambarus [this message]
2022-06-08 11:39 ` Tudor.Ambarus
2022-05-09 22:10 ` [PATCH v15 7/8] mtd: spi-nor: spansion: Add local function to discover page size tkuw584924
2022-05-09 22:10 ` [PATCH v15 8/8] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups tkuw584924
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=70ff017e-cff3-0dc2-6624-5dcc27fc67de@microchip.com \
--to=tudor.ambarus@microchip.com \
--cc=Bacem.Daassi@infineon.com \
--cc=Takahiro.Kuwano@infineon.com \
--cc=linux-mtd@lists.infradead.org \
--cc=michael@walle.cc \
--cc=miquel.raynal@bootlin.com \
--cc=p.yadav@ti.com \
--cc=richard@nod.at \
--cc=tkuw584924@gmail.com \
--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).