| Message ID | 20211029172633.886453-11-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Clean params init | expand |
Am 2021-10-29 19:26, schrieb Tudor Ambarus: > Setting the correct nor->mtd._write in a fixup hook was misleading, > since this is not a fixup, just a specific setting for SST, that > differs > from the SPI NOR core default init. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Reviewed-by: Michael Walle <michael@walle.cc> Just a comment below. > --- > drivers/mtd/spi-nor/sst.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c > index 660aabde477a..3593aae0920f 100644 > --- a/drivers/mtd/spi-nor/sst.c > +++ b/drivers/mtd/spi-nor/sst.c > @@ -177,14 +177,14 @@ static int sst_write(struct mtd_info *mtd, > loff_t to, size_t len, > return ret; > } > > -static void sst_post_sfdp_fixups(struct spi_nor *nor) > +static void sst_late_init(struct spi_nor *nor) > { > if (nor->info->flags & SST_WRITE) SST_WRITE is only used during to set this. Shouldn't this be a fixup per flash so we don't need that flag. > nor->mtd._write = sst_write; > } > > static const struct spi_nor_fixups sst_fixups = { > - .post_sfdp = sst_post_sfdp_fixups, > + .late_init = sst_late_init, > }; > > const struct spi_nor_manufacturer spi_nor_sst = { -michael
On 11/9/21 11:47 AM, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2021-10-29 19:26, schrieb Tudor Ambarus: >> Setting the correct nor->mtd._write in a fixup hook was misleading, >> since this is not a fixup, just a specific setting for SST, that >> differs >> from the SPI NOR core default init. >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > > Reviewed-by: Michael Walle <michael@walle.cc> > > Just a comment below. > >> --- >> drivers/mtd/spi-nor/sst.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c >> index 660aabde477a..3593aae0920f 100644 >> --- a/drivers/mtd/spi-nor/sst.c >> +++ b/drivers/mtd/spi-nor/sst.c >> @@ -177,14 +177,14 @@ static int sst_write(struct mtd_info *mtd, >> loff_t to, size_t len, >> return ret; >> } >> >> -static void sst_post_sfdp_fixups(struct spi_nor *nor) >> +static void sst_late_init(struct spi_nor *nor) >> { >> if (nor->info->flags & SST_WRITE) > > SST_WRITE is only used during to set this. Shouldn't this be > a fixup per flash so we don't need that flag. We should get rid of the SST_WRITE indeed. Do you want me to handle this or do you want to take care of it yourself? As a side note, I find the manufacturer fixup hooks and generic settings not scalable. We may consider to remove the per manufacturer settings in the future. Cheers, ta > >> nor->mtd._write = sst_write; >> } >> >> static const struct spi_nor_fixups sst_fixups = { >> - .post_sfdp = sst_post_sfdp_fixups, >> + .late_init = sst_late_init, >> }; >> >> const struct spi_nor_manufacturer spi_nor_sst = { > > -michael
On 11/9/21 12:22 PM, Tudor Ambarus wrote: > On 11/9/21 11:47 AM, Michael Walle wrote: cut >>> -static void sst_post_sfdp_fixups(struct spi_nor *nor) >>> +static void sst_late_init(struct spi_nor *nor) >>> { >>> if (nor->info->flags & SST_WRITE) >> >> SST_WRITE is only used during to set this. Shouldn't this be >> a fixup per flash so we don't need that flag. > > We should get rid of the SST_WRITE indeed. Do you want me to handle this > or do you want to take care of it yourself? > oh, I forgot, I have already took care of it, check: [PATCH v3 13/25] mtd: spi-nor: sst: Get rid of SST_WRITE flash_info flag
>> SST_WRITE is only used during to set this. Shouldn't this be >> a fixup per flash so we don't need that flag. > > We should get rid of the SST_WRITE indeed. Do you want me to handle > this > or do you want to take care of it yourself? Well you already got a patch (in this series) which will do exactly this ;) But I didn't notice, because its later in this series. -michael
On 29/10/21 08:26PM, Tudor Ambarus wrote: > Setting the correct nor->mtd._write in a fixup hook was misleading, > since this is not a fixup, just a specific setting for SST, that differs > from the SPI NOR core default init. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c index 660aabde477a..3593aae0920f 100644 --- a/drivers/mtd/spi-nor/sst.c +++ b/drivers/mtd/spi-nor/sst.c @@ -177,14 +177,14 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len, return ret; } -static void sst_post_sfdp_fixups(struct spi_nor *nor) +static void sst_late_init(struct spi_nor *nor) { if (nor->info->flags & SST_WRITE) nor->mtd._write = sst_write; } static const struct spi_nor_fixups sst_fixups = { - .post_sfdp = sst_post_sfdp_fixups, + .late_init = sst_late_init, }; const struct spi_nor_manufacturer spi_nor_sst = {
Setting the correct nor->mtd._write in a fixup hook was misleading, since this is not a fixup, just a specific setting for SST, that differs from the SPI NOR core default init. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/spi-nor/sst.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)