| Message ID | 20210727045222.905056-15-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Handle ID collisions and clean params init | expand |
On 27/07/21 07:52AM, Tudor Ambarus wrote: > spansion_post_sfdp_fixups() was called regardless if the flash defined > SFDP tables or not. A better place for this kind of parameters init is > in manufacturer's late_init() hook. post_sfdp() should be called only > when SFDP is defined. No functional change in this patch. > > Instead of doing the 4b opcodes settings at manufacturer level, thus > also for every flash that will be introduced, this should be done > just where it is needed, per flash. I'll let this for other patch. Yes, agreed. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Reviewed-by: Pratyush Yadav <p.yadav@ti.com> > --- > drivers/mtd/spi-nor/spansion.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c > index ee82dcd75310..aad7170768b4 100644 > --- a/drivers/mtd/spi-nor/spansion.c > +++ b/drivers/mtd/spi-nor/spansion.c > @@ -276,7 +276,7 @@ static const struct flash_info spansion_parts[] = { > }, > }; > > -static void spansion_post_sfdp_fixups(struct spi_nor *nor) > +static void spansion_late_init(struct spi_nor *nor) > { > if (nor->params->size <= SZ_16M) > return; > @@ -287,13 +287,9 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor) > nor->mtd.erasesize = nor->info->sector_size; > } > > -static const struct spi_nor_fixups spansion_fixups = { > - .post_sfdp = spansion_post_sfdp_fixups, > -}; > - > const struct spi_nor_manufacturer spi_nor_spansion = { > .name = "spansion", > .parts = spansion_parts, > .nparts = ARRAY_SIZE(spansion_parts), > - .fixups = &spansion_fixups, > + .late_init = spansion_late_init, > }; > -- > 2.25.1 >
Am 2021-07-27 06:52, schrieb Tudor Ambarus: > spansion_post_sfdp_fixups() was called regardless if the flash defined > SFDP tables or not. A better place for this kind of parameters init is > in manufacturer's late_init() hook. post_sfdp() should be called only > when SFDP is defined. No functional change in this patch. > > Instead of doing the 4b opcodes settings at manufacturer level, thus > also for every flash that will be introduced, this should be done > just where it is needed, per flash. I'll let this for other patch. Mh, then why does some flashes define SPI_NOR_4B_OPCODES. It's unclear to me when you would use SPI_NOR_4B_OPCODES and when this individual fixup. Also right now, these SPI_NOR_4B_OPCODES flags for the spansion flashes are superfluous, no? -michael
On 9/10/21 1:02 AM, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2021-07-27 06:52, schrieb Tudor Ambarus: >> spansion_post_sfdp_fixups() was called regardless if the flash defined >> SFDP tables or not. A better place for this kind of parameters init is >> in manufacturer's late_init() hook. post_sfdp() should be called only >> when SFDP is defined. No functional change in this patch. >> >> Instead of doing the 4b opcodes settings at manufacturer level, thus >> also for every flash that will be introduced, this should be done >> just where it is needed, per flash. I'll let this for other patch. > > Mh, then why does some flashes define SPI_NOR_4B_OPCODES. It's > unclear to me when you would use SPI_NOR_4B_OPCODES and when check patch 18/35, I dropped the SPI_NOR_4B_OPCODES flash_info entry flag. 4b-opcodes support is SFDP discoverable. Where flashes define the 4bait table, they will fill the support by setting SPI NOR SNOR_F_4B_OPCODES flag. Where the 4bait table is not defined, SNOR_F_4B_OPCODES should be set in a late_init() hook. > this individual fixup. Also right now, these SPI_NOR_4B_OPCODES > flags for the spansion flashes are superfluous, no? > there's no spansion flash_info entry that explicitly declare SPI_NOR_4B_OPCODES, if that's what you meant. Cheers, ta
Am 2021-10-01 14:14, schrieb Tudor.Ambarus@microchip.com: > On 9/10/21 1:02 AM, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2021-07-27 06:52, schrieb Tudor Ambarus: >>> spansion_post_sfdp_fixups() was called regardless if the flash >>> defined >>> SFDP tables or not. A better place for this kind of parameters init >>> is >>> in manufacturer's late_init() hook. post_sfdp() should be called only >>> when SFDP is defined. No functional change in this patch. >>> >>> Instead of doing the 4b opcodes settings at manufacturer level, thus >>> also for every flash that will be introduced, this should be done >>> just where it is needed, per flash. I'll let this for other patch. >> >> Mh, then why does some flashes define SPI_NOR_4B_OPCODES. It's >> unclear to me when you would use SPI_NOR_4B_OPCODES and when > > check patch 18/35, I dropped the SPI_NOR_4B_OPCODES flash_info entry > flag. I wasn't that far down the patch series when I reviewed this patch. Ok. > 4b-opcodes support is SFDP discoverable. Where flashes define the 4bait > table, > they will fill the support by setting SPI NOR SNOR_F_4B_OPCODES flag. > Where the > 4bait table is not defined, SNOR_F_4B_OPCODES should be set in a > late_init() hook. > >> this individual fixup. Also right now, these SPI_NOR_4B_OPCODES >> flags for the spansion flashes are superfluous, no? >> > > there's no spansion flash_info entry that explicitly declare > SPI_NOR_4B_OPCODES, > if that's what you meant. after patch 18/35, yes ;) Reviewed-by: Michael Walle <michael@walle.cc>
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c index ee82dcd75310..aad7170768b4 100644 --- a/drivers/mtd/spi-nor/spansion.c +++ b/drivers/mtd/spi-nor/spansion.c @@ -276,7 +276,7 @@ static const struct flash_info spansion_parts[] = { }, }; -static void spansion_post_sfdp_fixups(struct spi_nor *nor) +static void spansion_late_init(struct spi_nor *nor) { if (nor->params->size <= SZ_16M) return; @@ -287,13 +287,9 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor) nor->mtd.erasesize = nor->info->sector_size; } -static const struct spi_nor_fixups spansion_fixups = { - .post_sfdp = spansion_post_sfdp_fixups, -}; - const struct spi_nor_manufacturer spi_nor_spansion = { .name = "spansion", .parts = spansion_parts, .nparts = ARRAY_SIZE(spansion_parts), - .fixups = &spansion_fixups, + .late_init = spansion_late_init, };
spansion_post_sfdp_fixups() was called regardless if the flash defined SFDP tables or not. A better place for this kind of parameters init is in manufacturer's late_init() hook. post_sfdp() should be called only when SFDP is defined. No functional change in this patch. Instead of doing the 4b opcodes settings at manufacturer level, thus also for every flash that will be introduced, this should be done just where it is needed, per flash. I'll let this for other patch. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/spi-nor/spansion.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)