On 09/07/20 4:47 pm, Jagan Teki wrote:
[...]
On 09/07/20 4:47 pm, Jagan Teki wrote:
> diff --git a/arch/x86/cpu/apollolake/spl.c b/arch/x86/cpu/apollolake/spl.c
> index e1ee1e0624..9c80440bbb 100644
> --- a/arch/x86/cpu/apollolake/spl.c
> +++ b/arch/x86/cpu/apollolake/spl.c
> @@ -10,6 +10,7 @@
> #include <image.h>
> #include <log.h>
> #include <malloc.h>
> +#include <mtd.h>
> #include <spi.h>
> #include <spl.h>
> #include <spi_flash.h>
> @@ -41,7 +42,7 @@ static int rom_load_image(struct spl_image_info
*spl_image,
> debug("Reading from mapped SPI %lx, size %lx", spl_pos, spl_size);
>
> if (CONFIG_IS_ENABLED(SPI_FLASH_SUPPORT)) {
> - ret = uclass_find_first_device(UCLASS_SPI_FLASH, &dev);
> + ret = uclass_find_first_device(UCLASS_MTD, &dev);
You have to be careful here (and rest of places in this series where
uclass_find_first_device() is used) as the first MTD device need not be
first SPI FLASH device. RAW Nand and parallel flash drivers also
register MTD devices. Eg.: Above change will be wrong, if apollolake has
both SPI FLASH and NAND/Parallel NORs and Parallel NOR binds as first
UCLASS_MTD driver.
> if (ret)
> return log_msg_ret("spi_flash", ret);
> if (!dev)
> @@ -77,7 +78,7 @@ static int spl_fast_spi_load_image(struct
spl_image_info *spl_image,
> struct udevice *dev;
> int ret;
>
> - ret = uclass_first_device_err(UCLASS_SPI_FLASH, &dev);
> + ret = uclass_first_device_err(UCLASS_MTD, &dev);
> if (ret)
> return ret;
[...]
> diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
> index 44cdb3151d..9cab08f49b 100644
> --- a/drivers/mtd/spi/sf-uclass.c
> +++ b/drivers/mtd/spi/sf-uclass.c
> @@ -7,6 +7,7 @@
> #include <dm.h>
> #include <log.h>
> #include <malloc.h>
> +#include <mtd.h>
> #include <spi.h>
> #include <spi_flash.h>
> #include <dm/device-internal.h>
> @@ -14,22 +15,6 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> -int spi_flash_read_dm(struct udevice *dev, u32 offset, size_t len, void *buf)
> -{
> - return log_ret(sf_get_ops(dev)->read(dev, offset, len, buf));
> -}
> -
> -int spi_flash_write_dm(struct udevice *dev, u32 offset, size_t len,
> - const void *buf)
> -{
> - return log_ret(sf_get_ops(dev)->write(dev, offset, len, buf));
> -}
> -
> -int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len)
> -{
> - return log_ret(sf_get_ops(dev)->erase(dev, offset, len));
> -}
> -
> void spi_flash_free(struct spi_flash *flash)
> {
> device_remove(flash->spi->dev, DM_REMOVE_NORMAL);
> @@ -57,108 +42,78 @@ struct spi_flash *spi_flash_probe(unsigned int bus,
> unsigned int cs,
> str = strdup(name);
> #endif
> ret = spi_get_bus_and_cs(bus, cs, max_hz, spi_mode,
> - "spi_flash_std", str, &bus_dev, &slave);
> + "spi_flash_std", str, &bus_dev, &slave);
Unintended whitespace change?
> if (ret)
> return NULL;
>
> - return dev_get_uclass_priv(slave->dev);
> + return dev_get_priv(slave->dev);
> }
>
> -static int spi_flash_post_bind(struct udevice *dev)
> +static int spi_flash_std_read(struct udevice *dev, loff_t from, size_t len,
> + u_char *buf)
> {
> -#if defined(CONFIG_NEEDS_MANUAL_RELOC)
> - struct dm_spi_flash_ops *ops = sf_get_ops(dev);
> - static int reloc_done;
> -
> - if (!reloc_done) {
> - if (ops->read)
> - ops->read += gd->reloc_off;
> - if (ops->write)
> - ops->write += gd->reloc_off;
> - if (ops->erase)
> - ops->erase += gd->reloc_off;
> -
> - reloc_done++;
> - }
> -#endif
> - return 0;
> -}
> -
> -static int spi_flash_std_read(struct udevice *dev, u32 offset, size_t len,
> - void *buf)
> -{
> - struct spi_flash *flash = dev_get_uclass_priv(dev);
> + struct spi_flash *flash = dev_get_priv(dev);
> struct mtd_info *mtd = &flash->mtd;
> size_t retlen;
>
> - return log_ret(mtd->_read(mtd, offset, len, &retlen, buf));
> + return log_ret(mtd->_read(mtd, from, len, &retlen, buf));
> }
>
> -static int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
> - const void *buf)
> +static int spi_flash_std_write(struct udevice *dev, loff_t to, size_t len,
> + const u_char *buf)
> {
> - struct spi_flash *flash = dev_get_uclass_priv(dev);
> + struct spi_flash *flash = dev_get_priv(dev);
> struct mtd_info *mtd = &flash->mtd;
> size_t retlen;
>
> - return mtd->_write(mtd, offset, len, &retlen, buf);
> + return mtd->_write(mtd, to, len, &retlen, buf);
> }
>
> -static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
> +static int spi_flash_std_erase(struct udevice *dev, struct erase_info *instr)
> {
> - struct spi_flash *flash = dev_get_uclass_priv(dev);
> + struct spi_flash *flash = dev_get_priv(dev);
> struct mtd_info *mtd = &flash->mtd;
> - struct erase_info instr;
>
> - if (offset % mtd->erasesize || len % mtd->erasesize) {
> - printf("SF: Erase offset/length not multiple of erase size\n");
> + if (instr->addr % mtd->erasesize || instr->len % mtd->erasesize) {
> + dev_err(dev, "Erase offset/length not multiple of erasesize\n");
> return -EINVAL;
> }
Should this check be moved to mtd_derase() wrapper?
>
> - memset(&instr, 0, sizeof(instr));
> - instr.addr = offset;
> - instr.len = len;
> -
> - return mtd->_erase(mtd, &instr);
> + return mtd->_erase(mtd, instr);
> }
>
> static int spi_flash_std_probe(struct udevice *dev)
> {
> + struct spi_flash *flash = dev_get_priv(dev);
> struct spi_slave *slave = dev_get_parent_priv(dev);
> - struct spi_flash *flash;
> int ret;
>
> - if (!slave) {
> - printf("SF: Failed to set up slave\n");
> - return -ENODEV;
> - }
> -
> - flash = dev_get_uclass_priv(dev);
> flash->dev = dev;
> flash->spi = slave;
>
> - /* Claim spi bus */
> ret = spi_claim_bus(slave);
> if (ret) {
> - debug("SF: Failed to claim SPI bus: %d\n", ret);
> + dev_err(dev, "failed to claim bus (ret=%d)\n", ret);
> return ret;
> }
>
> ret = spi_nor_scan(flash);
> - if (ret)
> - goto err_read_id;
> + if (ret) {
> + dev_err(dev, "failed to scan spinor (ret=%d)\n", ret);
> + goto err_claim_bus;
> + }
>
> - if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
> + if (IS_ENABLED(CONFIG_SPI_FLASH_MTD))
> ret = spi_flash_mtd_register(flash);
>
> -err_read_id:
> +err_claim_bus:
> spi_release_bus(slave);
> return ret;
> }
>
> static int spi_flash_std_remove(struct udevice *dev)
> {
> - if (CONFIG_IS_ENABLED(SPI_FLASH_MTD))
> + if (IS_ENABLED(CONFIG_SPI_FLASH_MTD))
> spi_flash_mtd_unregister();
>
> return 0;
> @@ -190,7 +145,7 @@ static int spi_flash_std_bind(struct udevice *dev)
> return 0;
> }
>
> -static const struct dm_spi_flash_ops spi_flash_std_ops = {
> +static const struct mtd_ops spi_flash_std_ops = {
> .read = spi_flash_std_read,
> .write = spi_flash_std_write,
> .erase = spi_flash_std_erase,
> @@ -203,7 +158,7 @@ static const struct udevice_id spi_flash_std_ids[] = {
>
> U_BOOT_DRIVER(spi_flash_std) = {
> .name = "spi_flash_std",
> - .id = UCLASS_SPI_FLASH,
> + .id = UCLASS_MTD,
> .of_match = spi_flash_std_ids,
> .probe = spi_flash_std_probe,
> .bind = spi_flash_std_bind,
> @@ -211,10 +166,3 @@ U_BOOT_DRIVER(spi_flash_std) = {
> .priv_auto_alloc_size = sizeof(struct spi_flash),
> .ops = &spi_flash_std_ops,
> };
> -
> -UCLASS_DRIVER(spi_flash) = {
> - .id = UCLASS_SPI_FLASH,
> - .name = "spi_flash",
> - .post_bind = spi_flash_post_bind,
> - .per_device_auto_alloc_size = sizeof(struct spi_flash),
> -};
> diff --git a/drivers/mtd/spi/sf_dataflash.c b/drivers/mtd/spi/sf_dataflash.c
> index 27d847d421..5c73b54cbc 100644
> --- a/drivers/mtd/spi/sf_dataflash.c
> +++ b/drivers/mtd/spi/sf_dataflash.c
> @@ -12,6 +12,7 @@
> #include <fdtdec.h>
> #include <flash.h>
> #include <log.h>
> +#include <mtd.h>
> #include <spi.h>
> #include <spi_flash.h>
> #include <div64.h>
> @@ -117,7 +118,7 @@ static int dataflash_waitready(struct spi_slave *spi)
> }
>
> /* Erase pages of flash */
> -static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
> +static int spi_dataflash_erase(struct udevice *dev, struct erase_info *instr)
> {
> struct dataflash *dataflash;
> struct spi_flash *spi_flash;
> @@ -125,6 +126,8 @@ static int spi_dataflash_erase(struct udevice *dev, u32
> offset, size_t len)
> unsigned blocksize;
> uint8_t *command;
> uint32_t rem;
> + loff_t offset = instr->addr;
> + size_t len = instr->len;
> int status;
>
> dataflash = dev_get_priv(dev);
> @@ -136,7 +139,7 @@ static int spi_dataflash_erase(struct udevice *dev, u32
> offset, size_t len)
> memset(dataflash->command, 0 , sizeof(dataflash->command));
> command = dataflash->command;
>
> - debug("%s: erase addr=0x%x len 0x%x\n", dev->name, offset, len);
> + debug("%s: erase addr=0x%llx len 0x%x\n", dev->name, offset, len);
>
> div_u64_rem(len, spi_flash->page_size, &rem);
> if (rem) {
> @@ -146,7 +149,7 @@ static int spi_dataflash_erase(struct udevice *dev, u32
> offset, size_t len)
> }
> div_u64_rem(offset, spi_flash->page_size, &rem);
> if (rem) {
> - printf("%s: offset(0x%x) isn't the multiple of page
> size(0x%x)\n",
> + printf("%s: offset(0x%llx) isn't the multiple of page
> size(0x%x)\n",
> dev->name, offset, spi_flash->page_size);
> return -EINVAL;
> }
> @@ -210,8 +213,8 @@ static int spi_dataflash_erase(struct udevice *dev, u32
> offset, size_t len)
> * len : Amount to read
> * buf : Buffer containing the data
> */
> -static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
> - void *buf)
> +static int spi_dataflash_read(struct udevice *dev, loff_t offset, size_t len,
> + u_char *buf)
> {
> struct dataflash *dataflash;
> struct spi_flash *spi_flash;
> @@ -227,7 +230,7 @@ static int spi_dataflash_read(struct udevice *dev, u32
> offset, size_t len,
> memset(dataflash->command, 0 , sizeof(dataflash->command));
> command = dataflash->command;
>
> - debug("%s: erase addr=0x%x len 0x%x\n", dev->name, offset, len);
> + debug("%s: erase addr=0x%llx len 0x%x\n", dev->name, offset, len);
> debug("READ: (%x) %x %x %x\n",
> command[0], command[1], command[2], command[3]);
>
> @@ -266,8 +269,8 @@ static int spi_dataflash_read(struct udevice *dev, u32
> offset, size_t len,
> * len : Amount to write
> * buf : Buffer containing the data
> */
> -int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
> - const void *buf)
> +int spi_dataflash_write(struct udevice *dev, loff_t offset, size_t len,
> + const u_char *buf)
> {
> struct dataflash *dataflash;
> struct spi_flash *spi_flash;
> @@ -285,7 +288,7 @@ int spi_dataflash_write(struct udevice *dev, u32 offset,
> size_t len,
> memset(dataflash->command, 0 , sizeof(dataflash->command));
> command = dataflash->command;
>
> - debug("%s: write 0x%x..0x%x\n", dev->name, offset, (offset + len));
> + debug("%s: write 0x%llx..0x%llx\n", dev->name, offset, (offset + len));
>
> pageaddr = ((unsigned)offset / spi_flash->page_size);
> to = ((unsigned)offset % spi_flash->page_size);
> @@ -676,7 +679,7 @@ err_jedec_probe:
> return status;
> }
>
> -static const struct dm_spi_flash_ops spi_dataflash_ops = {
> +static const struct mtd_ops spi_dataflash_ops = {
> .read = spi_dataflash_read,
> .write = spi_dataflash_write,
> .erase = spi_dataflash_erase,
> @@ -690,7 +693,7 @@ static const struct udevice_id spi_dataflash_ids[] = {
>
> U_BOOT_DRIVER(spi_dataflash) = {
> .name = "spi_dataflash",
> - .id = UCLASS_SPI_FLASH,
> + .id = UCLASS_MTD,
> .of_match = spi_dataflash_ids,
> .probe = spi_dataflash_probe,
> .priv_auto_alloc_size = sizeof(struct dataflash),
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 7837d459f1..1f56f7f48a 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -99,7 +99,6 @@ enum uclass_id {
> UCLASS_SMEM, /* Shared memory interface */
> UCLASS_SOUND, /* Playing simple sounds */
> UCLASS_SPI, /* SPI bus */
> - UCLASS_SPI_FLASH, /* SPI flash */
> UCLASS_SPI_GENERIC, /* Generic SPI flash target */
> UCLASS_SPMI, /* System Power Management Interface bus */
> UCLASS_SYSCON, /* System configuration device */
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 24759944eb..f4852fc5c1 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -10,6 +10,7 @@
> #define _SPI_FLASH_H_
>
> #include <dm.h> /* Because we dereference struct udevice here */
> +#include <mtd.h>
> #include <linux/types.h>
> #include <linux/mtd/spi-nor.h>
>
> @@ -27,70 +28,24 @@
> # define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE
> #endif
>
> -struct spi_slave;
> -
> -struct dm_spi_flash_ops {
> - int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> - int (*write)(struct udevice *dev, u32 offset, size_t len,
> - const void *buf);
> - int (*erase)(struct udevice *dev, u32 offset, size_t len);
> -};
> -
> -/* Access the serial operations for a device */
> -#define sf_get_ops(dev) ((struct dm_spi_flash_ops *)(dev)->driver->ops)
> -
> #if CONFIG_IS_ENABLED(DM_SPI_FLASH)
> -/**
> - * spi_flash_read_dm() - Read data from SPI flash
> - *
> - * @dev: SPI flash device
> - * @offset: Offset into device in bytes to read from
> - * @len: Number of bytes to read
> - * @buf: Buffer to put the data that is read
> - * @return 0 if OK, -ve on error
> - */
> -int spi_flash_read_dm(struct udevice *dev, u32 offset, size_t len, void
> *buf);
> -
> -/**
> - * spi_flash_write_dm() - Write data to SPI flash
> - *
> - * @dev: SPI flash device
> - * @offset: Offset into device in bytes to write to
> - * @len: Number of bytes to write
> - * @buf: Buffer containing bytes to write
> - * @return 0 if OK, -ve on error
> - */
> -int spi_flash_write_dm(struct udevice *dev, u32 offset, size_t len,
> - const void *buf);
> -
> -/**
> - * spi_flash_erase_dm() - Erase blocks of the SPI flash
> - *
> - * Note that @len must be a muiltiple of the flash sector size.
> - *
> - * @dev: SPI flash device
> - * @offset: Offset into device in bytes to start erasing
> - * @len: Number of bytes to erase
> - * @return 0 if OK, -ve on error
> - */
> -int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len);
>
> static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
> size_t len, void *buf)
> {
> - return spi_flash_read_dm(flash->dev, offset, len, buf);
> + return mtd_dread(flash->dev, offset, len, buf);
> }
>
> static inline int spi_flash_write(struct spi_flash *flash, u32 offset,
> size_t len, const void *buf)
> {
> - return spi_flash_write_dm(flash->dev, offset, len, buf);
> + return mtd_dwrite(flash->dev, offset, len, buf);
> }
>
> static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
> size_t len)
> {
> - return spi_flash_erase_dm(flash->dev, offset, len);
> + return mtd_derase(flash->dev, offset, len);
> }
>
> struct sandbox_state;
> diff --git a/test/dm/sf.c b/test/dm/sf.c
> index 9e7dead684..a675923dc5 100644
> --- a/test/dm/sf.c
> +++ b/test/dm/sf.c
> @@ -31,23 +31,23 @@ static int dm_test_spi_flash(struct unit_test_state *uts)
>
> src = map_sysmem(0x20000, full_size);
> ut_assertok(os_write_file("spi.bin", src, full_size));
> - ut_assertok(uclass_first_device_err(UCLASS_SPI_FLASH, &dev));
> + ut_assertok(uclass_first_device_err(UCLASS_MTD, &dev));
>
> dst = map_sysmem(0x20000 + full_size, full_size);
> - ut_assertok(spi_flash_read_dm(dev, 0, size, dst));
> + ut_assertok(mtd_dread(dev, 0, size, dst));
> ut_asserteq_mem(src, dst, size);
>
> /* Erase */
> - ut_assertok(spi_flash_erase_dm(dev, 0, size));
> - ut_assertok(spi_flash_read_dm(dev, 0, size, dst));
> + ut_assertok(mtd_derase(dev, 0, size));
> + ut_assertok(mtd_dread(dev, 0, size, dst));
> for (i = 0; i < size; i++)
> ut_asserteq(dst[i], 0xff);
>
> /* Write some new data */
> for (i = 0; i < size; i++)
> src[i] = i;
> - ut_assertok(spi_flash_write_dm(dev, 0, size, src));
> - ut_assertok(spi_flash_read_dm(dev, 0, size, dst));
> + ut_assertok(mtd_dwrite(dev, 0, size, src));
> + ut_assertok(mtd_dread(dev, 0, size, dst));
> ut_asserteq_mem(src, dst, size);
>
> /* Check mapping */
> -- 2.25.1
