| Message ID | 20181204122659.14720-10-vigneshr@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | SF: Migrate to Linux SPI NOR framework | expand |
On Tue, 4 Dec 2018 17:56:57 +0530 Vignesh R <vigneshr@ti.com> wrote: > Now that there is new SPI NOR framework, simplify mtd device > registration and read/write/erase operations. > > Signed-off-by: Vignesh R <vigneshr@ti.com> > --- > drivers/mtd/spi/sf_internal.h | 2 +- > drivers/mtd/spi/sf_mtd.c | 39 ++++++++++++----------------------- > drivers/mtd/spi/sf_probe.c | 2 +- > 3 files changed, 15 insertions(+), 28 deletions(-) > > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > index 7e7d400cdbdf..8b445bb0b506 100644 > --- a/drivers/mtd/spi/sf_internal.h > +++ b/drivers/mtd/spi/sf_internal.h > @@ -99,6 +99,6 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash); > > #ifdef CONFIG_SPI_FLASH_MTD > int spi_flash_mtd_register(struct spi_flash *flash); > -void spi_flash_mtd_unregister(void); > +void spi_flash_mtd_unregister(struct spi_flash *flash); > #endif > #endif /* _SF_INTERNAL_H_ */ > diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c > index 58d7e4439903..9c07ba3cadf7 100644 > --- a/drivers/mtd/spi/sf_mtd.c > +++ b/drivers/mtd/spi/sf_mtd.c > @@ -9,17 +9,15 @@ > #include <linux/mtd/mtd.h> > #include <spi_flash.h> > > -static struct mtd_info sf_mtd_info; > static char sf_mtd_name[8]; > > static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) > { > - struct spi_flash *flash = mtd->priv; > int err; > > instr->state = MTD_ERASING; > > - err = spi_flash_erase(flash, instr->addr, instr->len); > + err = mtd->_erase(mtd, instr); Please don't dereference mtd hooks directly. I'm pretty sure the mtd_erase() overhead is negligible, and if it's not, you can implement dummy wrappers for the SPL case to reduce it to zero.
Hi Boris, On 04/12/18 6:19 PM, Boris Brezillon wrote: > On Tue, 4 Dec 2018 17:56:57 +0530 > Vignesh R <vigneshr@ti.com> wrote: > >> Now that there is new SPI NOR framework, simplify mtd device >> registration and read/write/erase operations. >> >> Signed-off-by: Vignesh R <vigneshr@ti.com> >> --- >> drivers/mtd/spi/sf_internal.h | 2 +- >> drivers/mtd/spi/sf_mtd.c | 39 ++++++++++++----------------------- >> drivers/mtd/spi/sf_probe.c | 2 +- >> 3 files changed, 15 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h >> index 7e7d400cdbdf..8b445bb0b506 100644 >> --- a/drivers/mtd/spi/sf_internal.h >> +++ b/drivers/mtd/spi/sf_internal.h >> @@ -99,6 +99,6 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash); >> >> #ifdef CONFIG_SPI_FLASH_MTD >> int spi_flash_mtd_register(struct spi_flash *flash); >> -void spi_flash_mtd_unregister(void); >> +void spi_flash_mtd_unregister(struct spi_flash *flash); >> #endif >> #endif /* _SF_INTERNAL_H_ */ >> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c >> index 58d7e4439903..9c07ba3cadf7 100644 >> --- a/drivers/mtd/spi/sf_mtd.c >> +++ b/drivers/mtd/spi/sf_mtd.c >> @@ -9,17 +9,15 @@ >> #include <linux/mtd/mtd.h> >> #include <spi_flash.h> >> >> -static struct mtd_info sf_mtd_info; >> static char sf_mtd_name[8]; >> >> static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) >> { >> - struct spi_flash *flash = mtd->priv; >> int err; >> >> instr->state = MTD_ERASING; >> >> - err = spi_flash_erase(flash, instr->addr, instr->len); >> + err = mtd->_erase(mtd, instr); > > Please don't dereference mtd hooks directly. I'm pretty sure the > mtd_erase() overhead is negligible, and if it's not, you can > implement dummy wrappers for the SPL case to reduce it to zero. > I did start with mtd_* APIs here. But, ended up with bunch of patches just to get MTD configs in shape to satisfy compile time dependencies. I will send a patch to use mtd_* APIs here once Miquel's MTD config cleanups are in. Thanks for the review!
Hi Vignesh, Vignesh R <vigneshr@ti.com> wrote on Wed, 5 Dec 2018 13:30:27 +0530: > Hi Boris, > > On 04/12/18 6:19 PM, Boris Brezillon wrote: > > On Tue, 4 Dec 2018 17:56:57 +0530 > > Vignesh R <vigneshr@ti.com> wrote: > > > >> Now that there is new SPI NOR framework, simplify mtd device > >> registration and read/write/erase operations. > >> > >> Signed-off-by: Vignesh R <vigneshr@ti.com> > >> --- > >> drivers/mtd/spi/sf_internal.h | 2 +- > >> drivers/mtd/spi/sf_mtd.c | 39 ++++++++++++----------------------- > >> drivers/mtd/spi/sf_probe.c | 2 +- > >> 3 files changed, 15 insertions(+), 28 deletions(-) > >> > >> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > >> index 7e7d400cdbdf..8b445bb0b506 100644 > >> --- a/drivers/mtd/spi/sf_internal.h > >> +++ b/drivers/mtd/spi/sf_internal.h > >> @@ -99,6 +99,6 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash); > >> > >> #ifdef CONFIG_SPI_FLASH_MTD > >> int spi_flash_mtd_register(struct spi_flash *flash); > >> -void spi_flash_mtd_unregister(void); > >> +void spi_flash_mtd_unregister(struct spi_flash *flash); > >> #endif > >> #endif /* _SF_INTERNAL_H_ */ > >> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c > >> index 58d7e4439903..9c07ba3cadf7 100644 > >> --- a/drivers/mtd/spi/sf_mtd.c > >> +++ b/drivers/mtd/spi/sf_mtd.c > >> @@ -9,17 +9,15 @@ > >> #include <linux/mtd/mtd.h> > >> #include <spi_flash.h> > >> > >> -static struct mtd_info sf_mtd_info; > >> static char sf_mtd_name[8]; > >> > >> static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) > >> { > >> - struct spi_flash *flash = mtd->priv; > >> int err; > >> > >> instr->state = MTD_ERASING; > >> > >> - err = spi_flash_erase(flash, instr->addr, instr->len); > >> + err = mtd->_erase(mtd, instr); > > > > Please don't dereference mtd hooks directly. I'm pretty sure the > > mtd_erase() overhead is negligible, and if it's not, you can > > implement dummy wrappers for the SPL case to reduce it to zero. > > > > I did start with mtd_* APIs here. But, ended up with bunch of patches > just to get MTD configs in shape to satisfy compile time dependencies. > I will send a patch to use mtd_* APIs here once Miquel's MTD config > cleanups are in. Thanks for the review! > As an FYI, it took me more than 30 Travis builds to get the MTD Kconfigs/Makefiles (+ defconfigs) in shape :) To find faulty configurations (for example, configurations using CONFIG_SPI_FLASH_MTD but without enabling CONFIG_MTD) I wrote a small script just to open these files automatically. As a reference if someone wants to use/improve it, it is there [1]. [1] http://code.bulix.org/4ep8u5-518535 Thanks, Miquèl
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 7e7d400cdbdf..8b445bb0b506 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -99,6 +99,6 @@ int spi_flash_cmd_get_sw_write_prot(struct spi_flash *flash); #ifdef CONFIG_SPI_FLASH_MTD int spi_flash_mtd_register(struct spi_flash *flash); -void spi_flash_mtd_unregister(void); +void spi_flash_mtd_unregister(struct spi_flash *flash); #endif #endif /* _SF_INTERNAL_H_ */ diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c index 58d7e4439903..9c07ba3cadf7 100644 --- a/drivers/mtd/spi/sf_mtd.c +++ b/drivers/mtd/spi/sf_mtd.c @@ -9,17 +9,15 @@ #include <linux/mtd/mtd.h> #include <spi_flash.h> -static struct mtd_info sf_mtd_info; static char sf_mtd_name[8]; static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) { - struct spi_flash *flash = mtd->priv; int err; instr->state = MTD_ERASING; - err = spi_flash_erase(flash, instr->addr, instr->len); + err = mtd->_erase(mtd, instr); if (err) { instr->state = MTD_ERASE_FAILED; instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN; @@ -35,10 +33,9 @@ static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf) { - struct spi_flash *flash = mtd->priv; int err; - err = spi_flash_read(flash, from, len, buf); + err = mtd->_read(mtd, from, len, retlen, buf); if (!err) *retlen = len; @@ -48,10 +45,9 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t len, static int spi_flash_mtd_write(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, const u_char *buf) { - struct spi_flash *flash = mtd->priv; int err; - err = spi_flash_write(flash, to, len, buf); + err = mtd->_write(mtd, to, len, retlen, buf); if (!err) *retlen = len; @@ -73,31 +69,22 @@ static int spi_flash_mtd_number(void) int spi_flash_mtd_register(struct spi_flash *flash) { - memset(&sf_mtd_info, 0, sizeof(sf_mtd_info)); + struct mtd_info *mtd = &flash->mtd; sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number()); - sf_mtd_info.name = sf_mtd_name; - sf_mtd_info.type = MTD_NORFLASH; - sf_mtd_info.flags = MTD_CAP_NORFLASH; - sf_mtd_info.writesize = 1; - sf_mtd_info.writebufsize = flash->page_size; - - sf_mtd_info._erase = spi_flash_mtd_erase; - sf_mtd_info._read = spi_flash_mtd_read; - sf_mtd_info._write = spi_flash_mtd_write; - sf_mtd_info._sync = spi_flash_mtd_sync; - - sf_mtd_info.size = flash->size; - sf_mtd_info.priv = flash; + mtd->name = sf_mtd_name; + mtd->_erase = spi_flash_mtd_erase; + mtd->_read = spi_flash_mtd_read; + mtd->_write = spi_flash_mtd_write; + mtd->_sync = spi_flash_mtd_sync; /* Only uniform flash devices for now */ - sf_mtd_info.numeraseregions = 0; - sf_mtd_info.erasesize = flash->sector_size; + mtd->numeraseregions = 0; - return add_mtd_device(&sf_mtd_info); + return add_mtd_device(mtd); } -void spi_flash_mtd_unregister(void) +void spi_flash_mtd_unregister(struct spi_flash *flash) { - del_mtd_device(&sf_mtd_info); + del_mtd_device(&flash->mtd); } diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index c4c25a7e8403..81c9de54b6a8 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -84,7 +84,7 @@ struct spi_flash *spi_flash_probe(unsigned int busnum, unsigned int cs, void spi_flash_free(struct spi_flash *flash) { #ifdef CONFIG_SPI_FLASH_MTD - spi_flash_mtd_unregister(); + spi_flash_mtd_unregister(flash); #endif spi_free_slave(flash->spi); free(flash);
Now that there is new SPI NOR framework, simplify mtd device registration and read/write/erase operations. Signed-off-by: Vignesh R <vigneshr@ti.com> --- drivers/mtd/spi/sf_internal.h | 2 +- drivers/mtd/spi/sf_mtd.c | 39 ++++++++++++----------------------- drivers/mtd/spi/sf_probe.c | 2 +- 3 files changed, 15 insertions(+), 28 deletions(-)