| Message ID | 20210727045222.905056-22-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 7/27/21 7:52 AM, Tudor Ambarus wrote: > Used to init all the mtd_info fields. Move the mtd_info init > the last thing in the spi_nor_scan(), so that we avoid superfluous > initialization of the mtd_info fields in case of errors. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/spi-nor/core.c | 55 +++++++++++++++++++++----------------- > 1 file changed, 31 insertions(+), 24 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 5c8cffb5e6f2..26acfc9901db 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -3124,6 +3124,36 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor, > return info; > } > > +static void spi_nor_set_mtd_info(struct spi_nor *nor) > +{ > + struct mtd_info *mtd = &nor->mtd; > + struct device *dev = nor->dev; > + > + spi_nor_register_locking_ops(nor); > + > + /* Configure OTP parameters and ops */ > + spi_nor_otp_init(nor); > + > + mtd->dev.parent = dev; > + if (!mtd->name) > + mtd->name = dev_name(dev); > + mtd->priv = nor; Rasmus sent me a heads up that I have a leftover here, probably from a rebase. The priv assignment is no longer needed as per previous patch. Will update in v3.
On 27/07/21 07:52AM, Tudor Ambarus wrote: > Used to init all the mtd_info fields. Move the mtd_info init > the last thing in the spi_nor_scan(), so that we avoid superfluous > initialization of the mtd_info fields in case of errors. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/spi-nor/core.c | 55 +++++++++++++++++++++----------------- > 1 file changed, 31 insertions(+), 24 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 5c8cffb5e6f2..26acfc9901db 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -3124,6 +3124,36 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor, > return info; > } > > +static void spi_nor_set_mtd_info(struct spi_nor *nor) > +{ > + struct mtd_info *mtd = &nor->mtd; > + struct device *dev = nor->dev; > + > + spi_nor_register_locking_ops(nor); > + > + /* Configure OTP parameters and ops */ > + spi_nor_otp_init(nor); > + > + mtd->dev.parent = dev; > + if (!mtd->name) > + mtd->name = dev_name(dev); > + mtd->priv = nor; You remove the lines setting mtd->priv in the previous patch. Why add it back? Other than this, Reviewed-by: Pratyush Yadav <p.yadav@ti.com> > + mtd->type = MTD_NORFLASH; > + mtd->flags = MTD_CAP_NORFLASH; > + if (nor->info->flags & SPI_NOR_NO_ERASE) > + mtd->flags |= MTD_NO_ERASE; > + mtd->writesize = nor->params->writesize; > + mtd->writebufsize = nor->page_size; > + mtd->size = nor->params->size; > + mtd->_erase = spi_nor_erase; > + mtd->_read = spi_nor_read; > + mtd->_write = spi_nor_write; > + mtd->_suspend = spi_nor_suspend; > + mtd->_resume = spi_nor_resume; > + mtd->_get_device = spi_nor_get_device; > + mtd->_put_device = spi_nor_put_device; > +} > + > int spi_nor_scan(struct spi_nor *nor, const char *name, > const struct spi_nor_hwcaps *hwcaps) > { > @@ -3166,32 +3196,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, > > mutex_init(&nor->lock); > > - mtd->_write = spi_nor_write; > - > /* Init flash parameters based on flash_info struct and SFDP */ > ret = spi_nor_init_params(nor); > if (ret) > return ret; > > - if (!mtd->name) > - mtd->name = dev_name(dev); > - mtd->type = MTD_NORFLASH; > - mtd->writesize = nor->params->writesize; > - mtd->flags = MTD_CAP_NORFLASH; > - mtd->size = nor->params->size; > - mtd->_erase = spi_nor_erase; > - mtd->_read = spi_nor_read; > - mtd->_suspend = spi_nor_suspend; > - mtd->_resume = spi_nor_resume; > - mtd->_get_device = spi_nor_get_device; > - mtd->_put_device = spi_nor_put_device; > - > - if (info->flags & SPI_NOR_NO_ERASE) > - mtd->flags |= MTD_NO_ERASE; > - > - mtd->dev.parent = dev; > nor->page_size = nor->params->page_size; > - mtd->writebufsize = nor->page_size; > > /* > * Configure the SPI memory: > @@ -3207,15 +3217,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, > if (ret) > return ret; > > - spi_nor_register_locking_ops(nor); > - > /* Send all the required SPI flash commands to initialize device */ > ret = spi_nor_init(nor); > if (ret) > return ret; > > - /* Configure OTP parameters and ops */ > - spi_nor_otp_init(nor); > + spi_nor_set_mtd_info(nor); > > if (!nor->name) > nor->name = info->name; > -- > 2.25.1 >
Am 2021-07-27 06:52, schrieb Tudor Ambarus: > Used to init all the mtd_info fields. Move the mtd_info init > the last thing in the spi_nor_scan(), so that we avoid superfluous > initialization of the mtd_info fields in case of errors. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/spi-nor/core.c | 55 +++++++++++++++++++++----------------- > 1 file changed, 31 insertions(+), 24 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 5c8cffb5e6f2..26acfc9901db 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -3124,6 +3124,36 @@ static const struct flash_info > *spi_nor_get_flash_info(struct spi_nor *nor, > return info; > } > > +static void spi_nor_set_mtd_info(struct spi_nor *nor) > +{ > + struct mtd_info *mtd = &nor->mtd; > + struct device *dev = nor->dev; > + > + spi_nor_register_locking_ops(nor); This .. > + > + /* Configure OTP parameters and ops */ > + spi_nor_otp_init(nor); .. this looks odd here now. The first could be renamed to express that it only registers mtd ops. spi_nor_otp_init() does more - strictly speaking. That is, it does also check that the otp region length is a power of 2. At least it should also be renamed. But then I wonder, if in the future there is more to do in spi_nor_otp_init(), where do we put that then? So mhh. I don't know. spi_nor_otp_init() is a more fitting name, but calling it in spi_nor_set_mtd_info() is odd. -michael
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 5c8cffb5e6f2..26acfc9901db 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -3124,6 +3124,36 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor, return info; } +static void spi_nor_set_mtd_info(struct spi_nor *nor) +{ + struct mtd_info *mtd = &nor->mtd; + struct device *dev = nor->dev; + + spi_nor_register_locking_ops(nor); + + /* Configure OTP parameters and ops */ + spi_nor_otp_init(nor); + + mtd->dev.parent = dev; + if (!mtd->name) + mtd->name = dev_name(dev); + mtd->priv = nor; + mtd->type = MTD_NORFLASH; + mtd->flags = MTD_CAP_NORFLASH; + if (nor->info->flags & SPI_NOR_NO_ERASE) + mtd->flags |= MTD_NO_ERASE; + mtd->writesize = nor->params->writesize; + mtd->writebufsize = nor->page_size; + mtd->size = nor->params->size; + mtd->_erase = spi_nor_erase; + mtd->_read = spi_nor_read; + mtd->_write = spi_nor_write; + mtd->_suspend = spi_nor_suspend; + mtd->_resume = spi_nor_resume; + mtd->_get_device = spi_nor_get_device; + mtd->_put_device = spi_nor_put_device; +} + int spi_nor_scan(struct spi_nor *nor, const char *name, const struct spi_nor_hwcaps *hwcaps) { @@ -3166,32 +3196,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, mutex_init(&nor->lock); - mtd->_write = spi_nor_write; - /* Init flash parameters based on flash_info struct and SFDP */ ret = spi_nor_init_params(nor); if (ret) return ret; - if (!mtd->name) - mtd->name = dev_name(dev); - mtd->type = MTD_NORFLASH; - mtd->writesize = nor->params->writesize; - mtd->flags = MTD_CAP_NORFLASH; - mtd->size = nor->params->size; - mtd->_erase = spi_nor_erase; - mtd->_read = spi_nor_read; - mtd->_suspend = spi_nor_suspend; - mtd->_resume = spi_nor_resume; - mtd->_get_device = spi_nor_get_device; - mtd->_put_device = spi_nor_put_device; - - if (info->flags & SPI_NOR_NO_ERASE) - mtd->flags |= MTD_NO_ERASE; - - mtd->dev.parent = dev; nor->page_size = nor->params->page_size; - mtd->writebufsize = nor->page_size; /* * Configure the SPI memory: @@ -3207,15 +3217,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, if (ret) return ret; - spi_nor_register_locking_ops(nor); - /* Send all the required SPI flash commands to initialize device */ ret = spi_nor_init(nor); if (ret) return ret; - /* Configure OTP parameters and ops */ - spi_nor_otp_init(nor); + spi_nor_set_mtd_info(nor); if (!nor->name) nor->name = info->name;
Used to init all the mtd_info fields. Move the mtd_info init the last thing in the spi_nor_scan(), so that we avoid superfluous initialization of the mtd_info fields in case of errors. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/spi-nor/core.c | 55 +++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 24 deletions(-)