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

[Takahiro Kuwano <tkuw584924@gmail.com>]

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.

So, I think we need to differentiate between address length for
read/program/erase and flash's default address mode.

Obviously we are using nor->addr_nbytes as address length for read/
program/erase and should keep this usage.

For flash's default address mode, my preference is to use
params->addr_nbytes, but I should rename it to something like
params->def_addr_nbytes and rework spi_nor_set_addr_nbytes().

 static int spi_nor_set_addr_nbytes(struct spi_nor *nor)
 {
	if (nor->flags & SNOR_F_HAS_4BAIT) {
		nor->addr_nbytes = 4;
	} else if (nor->params->def_addr_nbytes) {
 		nor->addr_nbytes = nor->params->def_addr_nbytes;

> At some point, the mode is switched and nor->addr_nbytes becomes
> params->addr_nbytes. It seems in your case nor->addr_nbytes should
> be 4 right from the beginning. Which also means nor->addr_nbytes
> should be 3 for the other cases (and probably not 0).
> 
With param->def_addr_nbytes, I think we can keep nor->addr_nbytes = 0
during SFDP parse.

Thanks,
Takahiro


 




______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/