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

[<Tudor.Ambarus@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/