[U-Boot] mtd partitions go away

Boris Brezillon boris.brezillon at bootlin.com
Thu Nov 15 15:15:51 UTC 2018


Hi Heiko, 

On Thu, 15 Nov 2018 15:21:31 +0100
Boris Brezillon <boris.brezillon at bootlin.com> wrote:

> 
> Looks like spi_flash_mtd_register() is doing something really bad here
> [1]: memsetting the global sf_mtd_info object without testing if
> this object has already been registered, and then registering it again
> to the MTD layer. So any partition that had been attached to the mtd
> object through mtd_probe_devices() are lost, and mtdparts/mtdids are not
> parsed again because they haven't changed since the last time
> mtd_probe_devices() was called.
> 
> [1]https://elixir.bootlin.com/u-boot/v2018.11/source/drivers/mtd/spi/sf_mtd.c#L76

Can you try with the following diff applied?

--->8---
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 26f5c7c995e5..799056d97d0a 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -223,7 +223,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 
 #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
 
 /**
diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 58d7e4439903..c4fda82b4a36 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -9,8 +9,7 @@
 #include <linux/mtd/mtd.h>
 #include <spi_flash.h>
 
-static struct mtd_info sf_mtd_info;
-static char sf_mtd_name[8];
+#define SF_MTD_MAX_NAME_SIZE 8
 
 static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
@@ -71,33 +70,81 @@ static int spi_flash_mtd_number(void)
 #endif
 }
 
-int spi_flash_mtd_register(struct spi_flash *flash)
+static int spi_flash_mtd_init_and_add(struct spi_flash *flash,
+                                     struct mtd_info *mtd)
 {
-       memset(&sf_mtd_info, 0, sizeof(sf_mtd_info));
-       sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number());
+       snprintf(mtd->name, SF_MTD_MAX_NAME_SIZE, "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;
+       mtd->type = MTD_NORFLASH;
+       mtd->flags = MTD_CAP_NORFLASH;
+       mtd->writesize = 1;
+       mtd->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;
+       mtd->_erase = spi_flash_mtd_erase;
+       mtd->_read = spi_flash_mtd_read;
+       mtd->_write = spi_flash_mtd_write;
+       mtd->_sync = spi_flash_mtd_sync;
 
-       sf_mtd_info.size = flash->size;
-       sf_mtd_info.priv = flash;
+       mtd->size = flash->size;
+       mtd->priv = flash;
 
        /* Only uniform flash devices for now */
-       sf_mtd_info.numeraseregions = 0;
-       sf_mtd_info.erasesize = flash->sector_size;
+       mtd->numeraseregions = 0;
+       mtd->erasesize = flash->sector_size;
+
+       return add_mtd_device(mtd);
+}
+
+#ifdef CONFIG_DM_SPI_FLASH
+int spi_flash_mtd_register(struct spi_flash *flash)
+{
+       struct mtd_info *mtd = dev_get_uclass_priv(flash->dev);
+       int ret;
+
+       /* MTD object already registered, return directly. */
+       if (mtd->name)
+               return 0;
+
+       mtd->name = malloc(SF_MTD_MAX_NAME_SIZE);
+       if (!mtd->name)
+               return -ENOMEM;
+
+       mtd->dev = flash->dev;
+       ret = spi_flash_mtd_init_and_add(flash, mtd);
+       if (ret) {
+               free(mtd->name);
+               mtd->name = NULL;
+       }
+
+       return ret;
+}
+
+void spi_flash_mtd_unregister(struct spi_flash *flash)
+{
+       struct mtd_info *mtd = dev_get_uclass_priv(flash->dev);
+
+       if (!mtd->name)
+               return;
+
+       del_mtd_device(mtd);
+       free(mtd->name);
+       mtd->name = NULL;
+}
+#else
+static struct mtd_info sf_mtd_info;
+static char sf_mtd_name[SF_MTD_MAX_NAME_SIZE];
+
+int spi_flash_mtd_register(struct spi_flash *flash)
+{
+       memset(&sf_mtd_info, 0, sizeof(sf_mtd_info));
+       sf_mtd_info.name = sf_mtd_name;
 
-       return add_mtd_device(&sf_mtd_info);
+       return spi_flash_mtd_init_and_add(flash, &sf_mtd_info);
 }
 
-void spi_flash_mtd_unregister(void)
+void spi_flash_mtd_unregister(struct spi_flash *flash)
 {
        del_mtd_device(&sf_mtd_info);
 }
+#endif /* CONFIG_DM_SPI_FLASH */
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 94fde2ae7a36..b8ee6da8ae9f 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);


More information about the U-Boot mailing list