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

[Tudor.Ambarus at microchip.com]

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?

If so, we're forced to statically specify the default internal address
mode.

-- 
Cheers,
ta