Re: [PATCH v9 6/6] mtd: spi-nor: add support for Macronix Octal flash

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Alvin,

tudor.ambarus@linaro.org wrote on Tue, 24 Sep 2024 08:17:26 +0100:

> On 9/24/24 7:36 AM, Tudor Ambarus wrote:
> > 
> > 
> > On 9/24/24 7:26 AM, Tudor Ambarus wrote:  
> >>
> >>
> >> On 9/24/24 4:25 AM, Alvin Zhou wrote:  
> >>> Hi Tudor,
> >>>
> >>> Tudor Ambarus <tudor.ambarus@linaro.org> 於 2024年9月23日 週一 下午2:54寫道：  
> >>>>
> >>>> Hi, Alvin,
> >>>>
> >>>> I quickly skimmed over the previous 5 patches and they are looking fine.
> >>>>
> >>>> I don't get this patch however.
> >>>>
> >>>> On 7/18/24 4:46 AM, AlvinZhou wrote:  
> >>>>> From: AlvinZhou <alvinzhou@mxic.com.tw>
> >>>>>
> >>>>> Adding Manufacture ID 0xC2 in last of ID table because of
> >>>>> Octal Flash need manufacturer fixup for enabling/disabling
> >>>>> Octal DTR mode.
> >>>>>
> >>>>> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> >>>>> Signed-off-by: AlvinZhou <alvinzhou@mxic.com.tw>
> >>>>> ---
> >>>>>  drivers/mtd/spi-nor/macronix.c | 4 +++-
> >>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >>>>> index f039819a5252..1a8ccebdfe0e 100644
> >>>>> --- a/drivers/mtd/spi-nor/macronix.c
> >>>>> +++ b/drivers/mtd/spi-nor/macronix.c
> >>>>> @@ -200,7 +200,9 @@ static const struct flash_info macronix_nor_parts[] = {
> >>>>>               .name = "mx25l3255e",
> >>>>>               .size = SZ_4M,
> >>>>>               .no_sfdp_flags = SECT_4K,
> >>>>> -     }
> >>>>> +     },
> >>>>> +     /* Need the manufacturer fixups, Keep this last */
> >>>>> +     { .id = SNOR_ID(0xc2) }
> >>>>>  };
> >>>>>  
> >>>>
> >>>> Could you please elaborate why you need just the manufacturer id here? I
> >>>> would have expected to see a specific flash entry instead.  
> >>>
> >>> Grateful to Michael for the valuable suggestion. This addition of the
> >>> Macronix manufacturer ID enables the fixup functions such as
> >>> macronix_nor_set_octal_dtr to be executed without the need to
> >>> create separate ID entries for each Octal DTR NOR Flash in the
> >>> flash_info.
> >>>  
> >>
> >> Ah, nice. Okay then. I'm going to review the rest of the patches. They
> >> look promising ;).  
> > 
> > ah, but then you'll always have a matched ID, so you break the generic
> > flash probing for macronix. Is that correct?  
> 
> Answering myself: it's fine. Generic flash probe just fills a name,
> which we don't really care about.

I was also a bit surprised by the diff, would you mind filling a more
complete with details on the actual goal of this entry (having all
Macronix flashes to get the fixups, without creating separate ID
entries for each of the flashes)

Thanks,
Miquèl