Re: [PATCH] mtd: spi-nor: Add support for w25qNNjwim

[Tudor.Ambarus]

On Monday, January 13, 2020 3:15:15 PM EET Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Hi Tudor,

Hi, Michael,
> 
> Am 2020-01-13 11:07, schrieb Michael Walle:
> >>> Btw. is renaming the flashes also considered a backwards incomaptible
> >>> change?
> >> 
> >> No, we can fix the names.
> >> 
> >>> And can there be two flashes with the same name? Because IMHO it
> >>> would
> >>> be
> >> 
> >> I would prefer that we don't. Why would you have two different
> >> jedec-ids with
> >> the same name?
> > 
> > Because as pointed out in the Winbond example you cannot distiguish
> > between
> > W25Q32DW and W25Q32JWIQ; and in the Macronix example between MX25L8005
> > and
> > MX25L8006E. Thus my reasoning was to show only the common part, ie
> > W25Q32
> > or MX25L80 which should be the same for this particular ID. Like I
> > said, I'd
> > prefer showing an ambiguous name instead of a wrong one. But then you
> > may
> > have different IDs with the same ambiguous name.
> 
> Another solution would be to have the device tree provide a hint for the
> actual flash chip. There would be multiple entries in the spi_nor_ids
> with the
> same flash id. By default the first one is used (keeping the current
> behaviour). If there is for example
> 
>    compatible = "jedec,spi-nor", "w25q32jwq";
> 
> the flash_info for the w25q32jwq will be chosen.

This won't work for plug-able flashes. You will influence the name in dt to be 
chosen as w25q32jwq, and if you change w25q32jwq with w25q32dw you will end up 
with a wrong name for w25q32dw, thus the same problem.

If the flashes are identical but differ just in terms of name, we can rename 
the flash to "w25q32jwq (w25q32dw)". I haven't studied the differences between 
these flashes; if you want to fix them, send a patch and I'll try to help.

Cheers,
ta

> 
> I know this will conflict with the new rule that there should only be
> 
>    compatible = "jedec,spi-nor";
> 
> without the actual flash chip. But it seems that it is not always
> possible
> to just use the jedec id to match the correct chip.
> 
> Also see for example mx25l25635_post_bfpt_fixups() which tries to figure
> out different behaviour by looking at "some" SFDP data. In this case we
> might have been lucky, but I fear that this won't work in all cases and
> for older flashes it won't work at all.
> 
> BTW I do not suggest to add the strings to the the spi_nor_dev_ids[].
> 
> I guess that would be a less invasive way to fix different flashes with
> same jedec ids.
> 
> -michael