| Message ID | 20240718034614.484018-7-alvinzhou.tw@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add octal DTR support for Macronix flash | expand |
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. Thanks, ta
Hi, > > --- 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. I can't find the thread anymore as this is going on for forever, but that was my suggestion. That's a catch all to have the manufacturer fixup applied for any macronix flashes instead of having to list each flash id here. -michael
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. > > Thanks, > ta Thanks, Alvin
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 ;).
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?
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.
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
Hi Miquel, Miquel Raynal <miquel.raynal@bootlin.com> 於 2024年9月25日 週三 下午5:57寫道： > > 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) Thank you for the reminder, I will incorporate more detailed explanations. > > Thanks, > Miquèl Thanks, Alvin
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) } }; static int macronix_nor_octal_dtr_en(struct spi_nor *nor)