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

[Takahiro Kuwano]

Hello,

Any comments on how to handle this?

On 5/14/2022 12:51 PM, Takahiro Kuwano wrote:
> 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.
> 
> 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
> 
> 
>  
> 
> 
>