From: Vignesh Raghavendra <vigneshr@ti.com>
To: u-boot@lists.denx.de
Subject: [PATCH 5/5] mtd: spi: Switch to MTD uclass (absolute UCLASS_SPI_FLASH)
Date: Mon, 13 Jul 2020 14:30:04 +0530 [thread overview]
Message-ID: <74ed16b0-7ac6-7f2b-89b3-a0e7ac8827bf@ti.com> (raw)
In-Reply-To: <20200709111709.68904-6-jagan@amarulasolutions.com>
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
prev parent reply other threads:[~2020-07-13 9:00 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-09 11:17 [PATCH 0/5] mtd: Implement MTD UCLASS (use SPINOR) Jagan Teki
2020-07-09 11:17 ` [PATCH 1/5] mtd: spi: Drop redundent SPI flash driver Jagan Teki
2020-07-13 8:32 ` Vignesh Raghavendra
2020-07-09 11:17 ` [PATCH 2/5] mtd: Add dm-mtd core ops Jagan Teki
2020-07-09 11:17 ` [PATCH 3/5] mtd: Add SPL_DM_MTD option Jagan Teki
2020-07-09 11:17 ` [PATCH 4/5] mtd: Build mtd-uclass as obj Jagan Teki
2020-07-09 11:17 ` [PATCH 5/5] mtd: spi: Switch to MTD uclass (absolute UCLASS_SPI_FLASH) Jagan Teki
2020-07-13 9:00 ` Vignesh Raghavendra [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74ed16b0-7ac6-7f2b-89b3-a0e7ac8827bf@ti.com \
--to=vigneshr@ti.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.