[PATCH v15 6/8] mtd: spi-nor: Retain nor->addr_width at 4BAIT parse

[Takahiro Kuwano]

On 7/22/2022 2:06 PM, Tudor.Ambarus at 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 at 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 at 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 at gmail.com:
>>>>>>>>>>> From: Takahiro Kuwano <Takahiro.Kuwano at 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.