[U-Boot] [PATCH 09/16] sf_mtd: Simply mtd operations

[Daniel Schwierzeck <daniel.schwierzeck@gmail.com>]

Am 12.12.18 um 18:32 schrieb Vignesh R:
> 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      | 52 ++++++++++++++---------------------
>  drivers/mtd/spi/sf_probe.c    |  5 ++--
>  3 files changed, 24 insertions(+), 35 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 68c36002bee2..65185b7c57dc 100644
> --- a/drivers/mtd/spi/sf_mtd.c
> +++ b/drivers/mtd/spi/sf_mtd.c
> @@ -9,21 +9,19 @@
>  #include <linux/mtd/mtd.h>
>  #include <spi_flash.h>
>  
> -static struct mtd_info sf_mtd_info;
>  static bool sf_mtd_registered;
>  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;
>  
> -	if (!flash)
> +	if (!mtd || !mtd->priv)
>  		return -ENODEV;
>  
>  	instr->state = MTD_ERASING;
>  
> -	err = spi_flash_erase(flash, instr->addr, instr->len);
> +	err = mtd->_erase(mtd, instr);

this looks strange. Now you're delegating from the MTD instance created
in this driver to the one created by spi-nor. This driver was only meant
as an adapter between MTD and legacy SPI flash API. After the switch to
spi-nor this driver is obsolete and should be removed in patch 6/16. Or
is there any reason why users like cmd_mtd can't directly use the
spi-nor MTD instance?

>  	if (err) {
>  		instr->state = MTD_ERASE_FAILED;
>  		instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> @@ -39,13 +37,12 @@ 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;
>  
> -	if (!flash)
> +	if (!mtd || !mtd->priv)
>  		return -ENODEV;
>  
> -	err = spi_flash_read(flash, from, len, buf);
> +	err = mtd->_read(mtd, from, len, retlen, buf);
>  	if (!err)
>  		*retlen = len;
>  
> @@ -55,13 +52,12 @@ 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;
>  
> -	if (!flash)
> +	if (!mtd || !mtd->priv)
>  		return -ENODEV;
>  
> -	err = spi_flash_write(flash, to, len, buf);
> +	err = mtd->_write(mtd, to, len, retlen, buf);
>  	if (!err)
>  		*retlen = len;
>  
> @@ -83,10 +79,11 @@ static int spi_flash_mtd_number(void)
>  
>  int spi_flash_mtd_register(struct spi_flash *flash)
>  {
> +	struct mtd_info *mtd = &flash->mtd;
>  	int ret;
>  
>  	if (sf_mtd_registered) {
> -		ret = del_mtd_device(&sf_mtd_info);
> +		ret = del_mtd_device(mtd);
>  		if (ret)
>  			return ret;
>  
> @@ -94,42 +91,33 @@ int spi_flash_mtd_register(struct spi_flash *flash)
>  	}
>  
>  	sf_mtd_registered = false;
> -	memset(&sf_mtd_info, 0, sizeof(sf_mtd_info));
>  	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;
>  
> -	ret = add_mtd_device(&sf_mtd_info);
> +	ret = add_mtd_device(mtd);
>  	if (!ret)
>  		sf_mtd_registered = true;
>  
>  	return ret;
>  }
>  
> -void spi_flash_mtd_unregister(void)
> +void spi_flash_mtd_unregister(struct spi_flash *flash)
>  {
> +	struct mtd_info *mtd = &flash->mtd;
>  	int ret;
>  
>  	if (!sf_mtd_registered)
>  		return;
>  
> -	ret = del_mtd_device(&sf_mtd_info);
> +	ret = del_mtd_device(mtd);
>  	if (!ret) {
>  		sf_mtd_registered = false;
>  		return;
> @@ -141,7 +129,7 @@ void spi_flash_mtd_unregister(void)
>  	 * use-after-free bug. Still, things should be fixed to prevent the
>  	 * spi_flash object from being destroyed when del_mtd_device() fails.
>  	 */
> -	sf_mtd_info.priv = NULL;
> +	mtd->priv = NULL;
>  	printf("Failed to unregister MTD %s and the spi_flash object is going away: you're in deep trouble!",
> -	       sf_mtd_info.name);
> +	       sf_mtd_name);
>  }
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 7f1378f4946d..05a38dcb91c8 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);
> @@ -153,7 +153,8 @@ static int spi_flash_std_probe(struct udevice *dev)
>  static int spi_flash_std_remove(struct udevice *dev)
>  {
>  #ifdef CONFIG_SPI_FLASH_MTD
> -	spi_flash_mtd_unregister();
> +	struct spi_flash *flash = dev_get_uclass_priv(dev);
> +	spi_flash_mtd_unregister(flash);
>  #endif
>  	return 0;
>  }
> 

-- 
- Daniel