[PATCH v4 6/6] mtd: spi-nor: spansion: Add s25hl-t/s25hs-t IDs and fixups

[Takahiro Kuwano]

On 4/8/2021 7:03 PM, Tudor.Ambarus at microchip.com wrote:
> On 4/8/21 11:21 AM, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi Tudor,
>>
>> On 4/8/2021 2:06 PM, Tudor.Ambarus at microchip.com wrote:
>>> Hi,
>>>
>>> On 3/19/21 8:58 AM, tkuw584924 at gmail.com wrote:
>>>> +static int
>>>> +s25hx_t_post_bfpt_fixups(struct spi_nor *nor,
>>>> +                        const struct sfdp_parameter_header *bfpt_header,
>>>> +                        const struct sfdp_bfpt *bfpt,
>>>> +                        struct spi_nor_flash_parameter *params)
>>>> +{
>>>> +       int ret;
>>>> +       u32 addr;
>>>> +       u8 cfr3v;
>>>> +
>>>> +       ret = spi_nor_set_4byte_addr_mode(nor, true);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +       nor->addr_width = 4;
>>>
>>> Takahiro, you are bypassing the core functions. spansion_set_4byte_addr_mode()
>>> will be called at set_4byte_addr_mode() time, resulting in an illegal op?
>>>
>> Since the spansion_post_sfdp_fixups() adds the SNOR_F_4B_OPCODES flag,
>> spansion_set_4byte_addr_mode() is not called actually.
> 
> right. Do we have to undo this somewhere, i.e, call
> spi_nor_set_4byte_addr_mode(nor, false); ?
> 
No. The Read/Write Any Register commands take 3 or 4 byte address depending
on the Flash's current addr mode. Since the spansion_read/write_any_reg()
use nor->addr_width which is always 4, the Flash's addr mode must always be
in 4 byte mode.

>>
>>> Are we safe to modify the core and do the spi_nor_set_addr_width() and
>>> nor->params->set_4byte_addr_mode() before parsing SFDP?
>>>
>> It sounds not safe to me as there are some discussions about addr_width.
> 
> oh, yes, of course, because addr width and 4byte addr mode opcodes are
> discovered when parsing SFDP. I need to drink my coffee before writing
> emails :D.
> 
:)