All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/4] mtd: Add a function to report when the MTD dev list has been updated
@ 2018-11-16 14:40 Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 2/4] mtd: Parse mtdparts/mtdids again when the MTD " Boris Brezillon
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Boris Brezillon @ 2018-11-16 14:40 UTC (permalink / raw)
  To: u-boot

We need to parse mtdparts/mtids again everytime a device has been
added/removed from the MTD list, but there's currently no way to know
when such an update has been done.

Add an ->updated field to the idr struct that we set to true every time
a device is added/removed and expose a function returning the value
of this field and resetting it to false.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/mtdcore.c   | 16 +++++++++++++++-
 include/linux/mtd/mtd.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index fb6c779abbfe..7a15ded8c883 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -87,14 +87,17 @@ struct idr_layer {
 
 struct idr {
 	struct idr_layer id[MAX_IDR_ID];
+	bool updated;
 };
 
 #define DEFINE_IDR(name)	struct idr name;
 
 void idr_remove(struct idr *idp, int id)
 {
-	if (idp->id[id].used)
+	if (idp->id[id].used) {
 		idp->id[id].used = 0;
+		idp->updated = true;
+	}
 
 	return;
 }
@@ -134,6 +137,7 @@ int idr_alloc(struct idr *idp, void *ptr, int start, int end, gfp_t gfp_mask)
 		if (idl->used == 0) {
 			idl->used = 1;
 			idl->ptr = ptr;
+			idp->updated = true;
 			return i;
 		}
 		i++;
@@ -155,6 +159,16 @@ struct mtd_info *__mtd_next_device(int i)
 }
 EXPORT_SYMBOL_GPL(__mtd_next_device);
 
+bool mtd_dev_list_updated(void)
+{
+	if (mtd_idr.updated) {
+		mtd_idr.updated = false;
+		return true;
+	}
+
+	return false;
+}
+
 #ifndef __UBOOT__
 static LIST_HEAD(mtd_notifiers);
 
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 68e591532492..d20ebd820289 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -581,6 +581,7 @@ int mtd_arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
 void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
 			  const uint64_t length, uint64_t *len_incl_bad,
 			  int *truncated);
+bool mtd_dev_list_updated(void);
 
 /* drivers/mtd/mtd_uboot.c */
 int mtd_search_alternate_name(const char *mtdname, char *altname,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/4] mtd: Parse mtdparts/mtdids again when the MTD list has been updated
  2018-11-16 14:40 [U-Boot] [PATCH 1/4] mtd: Add a function to report when the MTD dev list has been updated Boris Brezillon
@ 2018-11-16 14:40 ` Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 4/4] mtd: sf: Make sure we don't register the same device twice Boris Brezillon
  2 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2018-11-16 14:40 UTC (permalink / raw)
  To: u-boot

Updates to the MTD device list should trigger a new parsing of the
mtdids/mtdparts vars even if those vars haven't changed.

Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command")
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/mtd_uboot.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 5ca560c96879..6a3e64395de2 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -161,9 +161,13 @@ int mtd_probe_devices(void)
 
 	mtd_probe_uclass_mtd_devs();
 
-	/* Check if mtdparts/mtdids changed since last call, otherwise: exit */
+	/*
+	 * Check if mtdparts/mtdids changed or if the MTD dev list was updated
+	 * since last call, otherwise: exit
+	 */
 	if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
 	    (mtdparts && old_mtdparts && mtdids && old_mtdids &&
+	     !mtd_dev_list_updated() &&
 	     !strcmp(mtdparts, old_mtdparts) &&
 	     !strcmp(mtdids, old_mtdids)))
 		return 0;
@@ -201,6 +205,12 @@ int mtd_probe_devices(void)
 		}
 	}
 
+	/*
+	 * Call mtd_dev_list_updated() to clear updates generated by our own
+	 * parts removal loop.
+	 */
+	mtd_dev_list_updated();
+
 	/* If either mtdparts or mtdids is empty, then exit */
 	if (!mtdparts || !mtdids)
 		return 0;
@@ -281,6 +291,12 @@ int mtd_probe_devices(void)
 		put_mtd_device(mtd);
 	}
 
+	/*
+	 * Call mtd_dev_list_updated() to clear updates generated by our own
+	 * parts registration loop.
+	 */
+	mtd_dev_list_updated();
+
 	return 0;
 }
 #else
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted
  2018-11-16 14:40 [U-Boot] [PATCH 1/4] mtd: Add a function to report when the MTD dev list has been updated Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 2/4] mtd: Parse mtdparts/mtdids again when the MTD " Boris Brezillon
@ 2018-11-16 14:40 ` Boris Brezillon
  2018-11-17  9:19   ` Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 4/4] mtd: sf: Make sure we don't register the same device twice Boris Brezillon
  2 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2018-11-16 14:40 UTC (permalink / raw)
  To: u-boot

If we don't do that, partitions might still be exposed while the
underlying device is gone.

Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/mtdcore.c   |  1 +
 include/linux/mtd/mtd.h | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 7a15ded8c883..46657fe7c949 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -528,6 +528,7 @@ int del_mtd_device(struct mtd_info *mtd)
 	struct mtd_notifier *not;
 #endif
 
+	del_mtd_partitions(mtd);
 	mutex_lock(&mtd_table_mutex);
 
 	if (idr_find(&mtd_idr, mtd->index) != mtd) {
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index d20ebd820289..c5b58dd3f0f7 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -562,8 +562,22 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
 /* drivers/mtd/mtdcore.h */
 int add_mtd_device(struct mtd_info *mtd);
 int del_mtd_device(struct mtd_info *mtd);
+
+#ifdef CONFIG_MTD_PARTITIONS
 int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
 int del_mtd_partitions(struct mtd_info *);
+#else
+static inline int add_mtd_partitions(struct mtd_info *,
+				     const struct mtd_partition *, int)
+{
+	return 0;
+}
+
+static int del_mtd_partitions(struct mtd_info *)
+{
+	return 0;
+}
+#endif
 
 struct mtd_info *__mtd_next_device(int i);
 #define mtd_for_each_device(mtd)			\
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 4/4] mtd: sf: Make sure we don't register the same device twice
  2018-11-16 14:40 [U-Boot] [PATCH 1/4] mtd: Add a function to report when the MTD dev list has been updated Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 2/4] mtd: Parse mtdparts/mtdids again when the MTD " Boris Brezillon
  2018-11-16 14:40 ` [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted Boris Brezillon
@ 2018-11-16 14:40 ` Boris Brezillon
  2 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2018-11-16 14:40 UTC (permalink / raw)
  To: u-boot

spi_flash_mtd_register() can be called several times and each time it
will register the same mtd_info instance like if it was a new one.
The MTD ID allocation gets crazy when that happens, so let's track the
status of the sf_mtd_info object to avoid that.

Fixes: 9fe6d8716e09 ("mtd, spi: Add MTD layer driver")
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/spi/sf_mtd.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 58d7e4439903..aabbc3589435 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -10,6 +10,7 @@
 #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)
@@ -73,6 +74,12 @@ static int spi_flash_mtd_number(void)
 
 int spi_flash_mtd_register(struct spi_flash *flash)
 {
+	int ret;
+
+	if (sf_mtd_registered)
+		del_mtd_device(&sf_mtd_info);
+
+	sf_mtd_registered = false;
 	memset(&sf_mtd_info, 0, sizeof(sf_mtd_info));
 	sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number());
 
@@ -94,7 +101,11 @@ int spi_flash_mtd_register(struct spi_flash *flash)
 	sf_mtd_info.numeraseregions = 0;
 	sf_mtd_info.erasesize = flash->sector_size;
 
-	return add_mtd_device(&sf_mtd_info);
+	ret = add_mtd_device(&sf_mtd_info);
+	if (!ret)
+		sf_mtd_registered = true;
+
+	return ret;
 }
 
 void spi_flash_mtd_unregister(void)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted
  2018-11-16 14:40 ` [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted Boris Brezillon
@ 2018-11-17  9:19   ` Boris Brezillon
  2018-11-19  6:16     ` Heiko Schocher
  0 siblings, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2018-11-17  9:19 UTC (permalink / raw)
  To: u-boot

On Fri, 16 Nov 2018 15:40:25 +0100
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> If we don't do that, partitions might still be exposed while the
> underlying device is gone.
> 
> Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/mtd/mtdcore.c   |  1 +
>  include/linux/mtd/mtd.h | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 7a15ded8c883..46657fe7c949 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -528,6 +528,7 @@ int del_mtd_device(struct mtd_info *mtd)
>  	struct mtd_notifier *not;
>  #endif
>  
> +	del_mtd_partitions(mtd);
>  	mutex_lock(&mtd_table_mutex);
>  
>  	if (idr_find(&mtd_idr, mtd->index) != mtd) {
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index d20ebd820289..c5b58dd3f0f7 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -562,8 +562,22 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
>  /* drivers/mtd/mtdcore.h */
>  int add_mtd_device(struct mtd_info *mtd);
>  int del_mtd_device(struct mtd_info *mtd);
> +
> +#ifdef CONFIG_MTD_PARTITIONS
>  int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
>  int del_mtd_partitions(struct mtd_info *);
> +#else
> +static inline int add_mtd_partitions(struct mtd_info *,
> +				     const struct mtd_partition *, int)

Args should have names.

> +{
> +	return 0;
> +}
> +
> +static int del_mtd_partitions(struct mtd_info *)

Missing inline here.

I'll send a v2 fixing those 2 bugs.

> +{
> +	return 0;
> +}
> +#endif
>  
>  struct mtd_info *__mtd_next_device(int i);
>  #define mtd_for_each_device(mtd)			\

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted
  2018-11-17  9:19   ` Boris Brezillon
@ 2018-11-19  6:16     ` Heiko Schocher
  2018-11-19  9:25       ` Miquel Raynal
  2018-11-19 11:57       ` Boris Brezillon
  0 siblings, 2 replies; 9+ messages in thread
From: Heiko Schocher @ 2018-11-19  6:16 UTC (permalink / raw)
  To: u-boot

Hello Boris,

Am 17.11.2018 um 10:19 schrieb Boris Brezillon:
> On Fri, 16 Nov 2018 15:40:25 +0100
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> 
>> If we don't do that, partitions might still be exposed while the
>> underlying device is gone.
>>
>> Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
>> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>> ---
>>   drivers/mtd/mtdcore.c   |  1 +
>>   include/linux/mtd/mtd.h | 14 ++++++++++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
>> index 7a15ded8c883..46657fe7c949 100644
>> --- a/drivers/mtd/mtdcore.c
>> +++ b/drivers/mtd/mtdcore.c
>> @@ -528,6 +528,7 @@ int del_mtd_device(struct mtd_info *mtd)
>>   	struct mtd_notifier *not;
>>   #endif
>>   
>> +	del_mtd_partitions(mtd);
>>   	mutex_lock(&mtd_table_mutex);
>>   
>>   	if (idr_find(&mtd_idr, mtd->index) != mtd) {
>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>> index d20ebd820289..c5b58dd3f0f7 100644
>> --- a/include/linux/mtd/mtd.h
>> +++ b/include/linux/mtd/mtd.h
>> @@ -562,8 +562,22 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
>>   /* drivers/mtd/mtdcore.h */
>>   int add_mtd_device(struct mtd_info *mtd);
>>   int del_mtd_device(struct mtd_info *mtd);
>> +
>> +#ifdef CONFIG_MTD_PARTITIONS
>>   int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
>>   int del_mtd_partitions(struct mtd_info *);
>> +#else
>> +static inline int add_mtd_partitions(struct mtd_info *,
>> +				     const struct mtd_partition *, int)
> 
> Args should have names.
> 
>> +{
>> +	return 0;
>> +}
>> +
>> +static int del_mtd_partitions(struct mtd_info *)
> 
> Missing inline here.
> 
> I'll send a v2 fixing those 2 bugs.

Thanks!

I tried your patchset, with them "ubi part ubi" does now work, also
after a "sf probe" ...

There is one problem, see log [1].

If you have an ubi partition attached (In my example on the NAND),
and issue "sf probe", the following "mtd list" shows not anymore
the SPI NOR MTD partitions. (It prints an error message
"Partition "ubi" already in use, aborting")

If I detach UBI from the NAND MTD partition, the MTD Partitions on
the SPI NOR are again found after a "sf probe" before "mtd list" [2]

bye,
Heiko

[1] U-Boot log
=> mtd list
List of MTD devices:
* nand0
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 64 bytes
   - OOB available: 0 bytes
   - ECC strength: 8 bits
   - ECC step size: 512 bytes
   - bitflip threshold: 6 bits
   - 0x000000000000-0x000008000000 : "nand0"
           - 0x000000000000-0x000008000000 : "ubi"
* nor0
   - type: NOR flash
   - block size: 0x10000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
           - 0x000000000000-0x000000010000 : "spl"
           - 0x000000010000-0x0000000d0000 : "u-boot"
           - 0x0000000d0000-0x0000000e0000 : "env"
           - 0x0000000e0000-0x0000000f0000 : "env-red"
           - 0x0000000f0000-0x000000800000 : "rescue"
=> ubi part ubi
ubi0: attaching mtd2
ubi0: scanning is finished
ubi0: attached mtd2 (name "ubi", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 4, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 52/46, WL threshold: 4096, image sequence number: 2030005227
ubi0: available PEBs: 8, total reserved PEBs: 1012, PEBs reserved for bad PEB handling: 16
=> ubi part ubi
ubi0: detaching mtd2
ubi0: mtd2 is detached
ubi0: attaching mtd2
ubi0: scanning is finished
ubi0: attached mtd2 (name "ubi", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 4, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 52/46, WL threshold: 4096, image sequence number: 2030005227
ubi0: available PEBs: 8, total reserved PEBs: 1012, PEBs reserved for bad PEB handling: 16
=> mtd list
List of MTD devices:
* nand0
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 64 bytes
   - OOB available: 0 bytes
   - ECC strength: 8 bits
   - ECC step size: 512 bytes
   - bitflip threshold: 6 bits
   - 0x000000000000-0x000008000000 : "nand0"
           - 0x000000000000-0x000008000000 : "ubi"
* nor0
   - type: NOR flash
   - block size: 0x10000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
           - 0x000000000000-0x000000010000 : "spl"
           - 0x000000010000-0x0000000d0000 : "u-boot"
           - 0x0000000d0000-0x0000000e0000 : "env"
           - 0x0000000e0000-0x0000000f0000 : "env-red"
           - 0x0000000f0000-0x000000800000 : "rescue"
=> sf probe
SF: Detected s25f064l with page size 256 Bytes, erase size 64 KiB, total 8 MiB
=> mtd list
Partition "ubi" already in use, aborting
List of MTD devices:
* nand0
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 64 bytes
   - OOB available: 0 bytes
   - ECC strength: 8 bits
   - ECC step size: 512 bytes
   - bitflip threshold: 6 bits
   - 0x000000000000-0x000008000000 : "nand0"
           - 0x000000000000-0x000008000000 : "ubi"
* nor0
   - type: NOR flash
   - block size: 0x10000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
=> ubi part ubi
ubi0: detaching mtd2
ubi0: mtd2 is detached
ubi0: attaching mtd2
ubi0: scanning is finished
ubi0: attached mtd2 (name "ubi", size 128 MiB)
ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
ubi0: good PEBs: 1020, bad PEBs: 4, corrupted PEBs: 0
ubi0: user volume: 4, internal volumes: 1, max. volumes count: 128
ubi0: max/mean erase counter: 52/46, WL threshold: 4096, image sequence number: 2030005227
ubi0: available PEBs: 8, total reserved PEBs: 1012, PEBs reserved for bad PEB handling: 16
=> mtd list
List of MTD devices:
* nand0
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 64 bytes
   - OOB available: 0 bytes
   - ECC strength: 8 bits
   - ECC step size: 512 bytes
   - bitflip threshold: 6 bits
   - 0x000000000000-0x000008000000 : "nand0"
           - 0x000000000000-0x000008000000 : "ubi"
* nor0
   - type: NOR flash
   - block size: 0x10000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
=>
=> mtd read spl 81000000
MTD device spl not found, ret -19
=>

[2] mtd list after ubi detach
=> ubi detach
ubi0: detaching mtd2
ubi0: mtd2 is detached
=> mtd list
List of MTD devices:
* nand0
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 64 bytes
   - OOB available: 0 bytes
   - ECC strength: 8 bits
   - ECC step size: 512 bytes
   - bitflip threshold: 6 bits
   - 0x000000000000-0x000008000000 : "nand0"
           - 0x000000000000-0x000008000000 : "ubi"
* nor0
   - type: NOR flash
   - block size: 0x10000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
=> sf probe
SF: Detected s25f064l with page size 256 Bytes, erase size 64 KiB, total 8 MiB
=> mtd list
List of MTD devices:
* nand0
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 64 bytes
   - OOB available: 0 bytes
   - ECC strength: 8 bits
   - ECC step size: 512 bytes
   - bitflip threshold: 6 bits
   - 0x000000000000-0x000008000000 : "nand0"
           - 0x000000000000-0x000008000000 : "ubi"
* nor0
   - type: NOR flash
   - block size: 0x10000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
           - 0x000000000000-0x000000010000 : "spl"
           - 0x000000010000-0x0000000d0000 : "u-boot"
           - 0x0000000d0000-0x0000000e0000 : "env"
           - 0x0000000e0000-0x0000000f0000 : "env-red"
           - 0x0000000f0000-0x000000800000 : "rescue"
=>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted
  2018-11-19  6:16     ` Heiko Schocher
@ 2018-11-19  9:25       ` Miquel Raynal
  2018-11-19 11:57       ` Boris Brezillon
  1 sibling, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2018-11-19  9:25 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

Heiko Schocher <hs@denx.de> wrote on Mon, 19 Nov 2018 07:16:53 +0100:

> Hello Boris,
> 
> Am 17.11.2018 um 10:19 schrieb Boris Brezillon:
> > On Fri, 16 Nov 2018 15:40:25 +0100
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote:  
> > >> If we don't do that, partitions might still be exposed while the  
> >> underlying device is gone.
> >>
> >> Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
> >> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> >> ---
> >>   drivers/mtd/mtdcore.c   |  1 +
> >>   include/linux/mtd/mtd.h | 14 ++++++++++++++
> >>   2 files changed, 15 insertions(+)
> >>
> >> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> >> index 7a15ded8c883..46657fe7c949 100644
> >> --- a/drivers/mtd/mtdcore.c
> >> +++ b/drivers/mtd/mtdcore.c
> >> @@ -528,6 +528,7 @@ int del_mtd_device(struct mtd_info *mtd)
> >>   	struct mtd_notifier *not;
> >>   #endif  
> >>   >> +	del_mtd_partitions(mtd);  
> >>   	mutex_lock(&mtd_table_mutex);  
> >>   >>   	if (idr_find(&mtd_idr, mtd->index) != mtd) {  
> >> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> >> index d20ebd820289..c5b58dd3f0f7 100644
> >> --- a/include/linux/mtd/mtd.h
> >> +++ b/include/linux/mtd/mtd.h
> >> @@ -562,8 +562,22 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
> >>   /* drivers/mtd/mtdcore.h */
> >>   int add_mtd_device(struct mtd_info *mtd);
> >>   int del_mtd_device(struct mtd_info *mtd);
> >> +
> >> +#ifdef CONFIG_MTD_PARTITIONS
> >>   int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
> >>   int del_mtd_partitions(struct mtd_info *);
> >> +#else
> >> +static inline int add_mtd_partitions(struct mtd_info *,
> >> +				     const struct mtd_partition *, int)
> > > Args should have names.  
> > >> +{  
> >> +	return 0;
> >> +}
> >> +
> >> +static int del_mtd_partitions(struct mtd_info *)
> > > Missing inline here.
> > > I'll send a v2 fixing those 2 bugs.  
> 
> Thanks!
> 
> I tried your patchset, with them "ubi part ubi" does now work, also
> after a "sf probe" ...
> 
> There is one problem, see log [1].
> 
> If you have an ubi partition attached (In my example on the NAND),
> and issue "sf probe", the following "mtd list" shows not anymore
> the SPI NOR MTD partitions. (It prints an error message
> "Partition "ubi" already in use, aborting")
> 
> If I detach UBI from the NAND MTD partition, the MTD Partitions on
> the SPI NOR are again found after a "sf probe" before "mtd list" [2]

I will have just a little time to look into this issue today, so
there is a quick fix, please tell me if it works for you, otherwise
I'll try to find more time during this week to look into it.


Thanks,
Miquèl

---

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted
  2018-11-19  6:16     ` Heiko Schocher
  2018-11-19  9:25       ` Miquel Raynal
@ 2018-11-19 11:57       ` Boris Brezillon
  2018-11-19 12:10         ` Heiko Schocher
  1 sibling, 1 reply; 9+ messages in thread
From: Boris Brezillon @ 2018-11-19 11:57 UTC (permalink / raw)
  To: u-boot

On Mon, 19 Nov 2018 07:16:53 +0100
Heiko Schocher <hs@denx.de> wrote:

> Hello Boris,
> 
> Am 17.11.2018 um 10:19 schrieb Boris Brezillon:
> > On Fri, 16 Nov 2018 15:40:25 +0100
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> >   
> >> If we don't do that, partitions might still be exposed while the
> >> underlying device is gone.
> >>
> >> Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
> >> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> >> ---
> >>   drivers/mtd/mtdcore.c   |  1 +
> >>   include/linux/mtd/mtd.h | 14 ++++++++++++++
> >>   2 files changed, 15 insertions(+)
> >>
> >> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> >> index 7a15ded8c883..46657fe7c949 100644
> >> --- a/drivers/mtd/mtdcore.c
> >> +++ b/drivers/mtd/mtdcore.c
> >> @@ -528,6 +528,7 @@ int del_mtd_device(struct mtd_info *mtd)
> >>   	struct mtd_notifier *not;
> >>   #endif
> >>   
> >> +	del_mtd_partitions(mtd);
> >>   	mutex_lock(&mtd_table_mutex);
> >>   
> >>   	if (idr_find(&mtd_idr, mtd->index) != mtd) {
> >> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> >> index d20ebd820289..c5b58dd3f0f7 100644
> >> --- a/include/linux/mtd/mtd.h
> >> +++ b/include/linux/mtd/mtd.h
> >> @@ -562,8 +562,22 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
> >>   /* drivers/mtd/mtdcore.h */
> >>   int add_mtd_device(struct mtd_info *mtd);
> >>   int del_mtd_device(struct mtd_info *mtd);
> >> +
> >> +#ifdef CONFIG_MTD_PARTITIONS
> >>   int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
> >>   int del_mtd_partitions(struct mtd_info *);
> >> +#else
> >> +static inline int add_mtd_partitions(struct mtd_info *,
> >> +				     const struct mtd_partition *, int)  
> > 
> > Args should have names.
> >   
> >> +{
> >> +	return 0;
> >> +}
> >> +
> >> +static int del_mtd_partitions(struct mtd_info *)  
> > 
> > Missing inline here.
> > 
> > I'll send a v2 fixing those 2 bugs.  
> 
> Thanks!
> 
> I tried your patchset, with them "ubi part ubi" does now work, also
> after a "sf probe" ...
> 
> There is one problem, see log [1].
> 
> If you have an ubi partition attached (In my example on the NAND),
> and issue "sf probe", the following "mtd list" shows not anymore
> the SPI NOR MTD partitions. (It prints an error message
> "Partition "ubi" already in use, aborting")
> 
> If I detach UBI from the NAND MTD partition, the MTD Partitions on
> the SPI NOR are again found after a "sf probe" before "mtd list" [2]

I sent you a new version of this patchset that should address your
issue. Note that updating parts on a device that is being used is still
forbidden, but at least it does not block updates on other devices.

Oh, and I keep thinking the spi-flash code is broken, and none of this
should happen if the spi-nor MTD devs were staying around instead of
being unregistered/registered every time sf probe is called. But I
don't intend to fix it now, as the proper solution is probably to port
the spi-nor layer we have in Linux to u-boot.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted
  2018-11-19 11:57       ` Boris Brezillon
@ 2018-11-19 12:10         ` Heiko Schocher
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Schocher @ 2018-11-19 12:10 UTC (permalink / raw)
  To: u-boot

Hello Boris,

Am 19.11.2018 um 12:57 schrieb Boris Brezillon:
> On Mon, 19 Nov 2018 07:16:53 +0100
> Heiko Schocher <hs@denx.de> wrote:
> 
>> Hello Boris,
>>
>> Am 17.11.2018 um 10:19 schrieb Boris Brezillon:
>>> On Fri, 16 Nov 2018 15:40:25 +0100
>>> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
>>>    
>>>> If we don't do that, partitions might still be exposed while the
>>>> underlying device is gone.
>>>>
>>>> Fixes: 2a74930da57f ("mtd: mtdpart: implement proper partition handling")
>>>> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>>>> ---
>>>>    drivers/mtd/mtdcore.c   |  1 +
>>>>    include/linux/mtd/mtd.h | 14 ++++++++++++++
>>>>    2 files changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
>>>> index 7a15ded8c883..46657fe7c949 100644
>>>> --- a/drivers/mtd/mtdcore.c
>>>> +++ b/drivers/mtd/mtdcore.c
>>>> @@ -528,6 +528,7 @@ int del_mtd_device(struct mtd_info *mtd)
>>>>    	struct mtd_notifier *not;
>>>>    #endif
>>>>    
>>>> +	del_mtd_partitions(mtd);
>>>>    	mutex_lock(&mtd_table_mutex);
>>>>    
>>>>    	if (idr_find(&mtd_idr, mtd->index) != mtd) {
>>>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>>>> index d20ebd820289..c5b58dd3f0f7 100644
>>>> --- a/include/linux/mtd/mtd.h
>>>> +++ b/include/linux/mtd/mtd.h
>>>> @@ -562,8 +562,22 @@ unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
>>>>    /* drivers/mtd/mtdcore.h */
>>>>    int add_mtd_device(struct mtd_info *mtd);
>>>>    int del_mtd_device(struct mtd_info *mtd);
>>>> +
>>>> +#ifdef CONFIG_MTD_PARTITIONS
>>>>    int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
>>>>    int del_mtd_partitions(struct mtd_info *);
>>>> +#else
>>>> +static inline int add_mtd_partitions(struct mtd_info *,
>>>> +				     const struct mtd_partition *, int)
>>>
>>> Args should have names.
>>>    
>>>> +{
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int del_mtd_partitions(struct mtd_info *)
>>>
>>> Missing inline here.
>>>
>>> I'll send a v2 fixing those 2 bugs.
>>
>> Thanks!
>>
>> I tried your patchset, with them "ubi part ubi" does now work, also
>> after a "sf probe" ...
>>
>> There is one problem, see log [1].
>>
>> If you have an ubi partition attached (In my example on the NAND),
>> and issue "sf probe", the following "mtd list" shows not anymore
>> the SPI NOR MTD partitions. (It prints an error message
>> "Partition "ubi" already in use, aborting")
>>
>> If I detach UBI from the NAND MTD partition, the MTD Partitions on
>> the SPI NOR are again found after a "sf probe" before "mtd list" [2]
> 
> I sent you a new version of this patchset that should address your
> issue. Note that updating parts on a device that is being used is still
> forbidden, but at least it does not block updates on other devices.

Yes.

> Oh, and I keep thinking the spi-flash code is broken, and none of this
> should happen if the spi-nor MTD devs were staying around instead of
> being unregistered/registered every time sf probe is called. But I
> don't intend to fix it now, as the proper solution is probably to port
> the spi-nor layer we have in Linux to u-boot.

I try your patches soon, but give me some time.

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-11-19 12:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16 14:40 [U-Boot] [PATCH 1/4] mtd: Add a function to report when the MTD dev list has been updated Boris Brezillon
2018-11-16 14:40 ` [U-Boot] [PATCH 2/4] mtd: Parse mtdparts/mtdids again when the MTD " Boris Brezillon
2018-11-16 14:40 ` [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted Boris Brezillon
2018-11-17  9:19   ` Boris Brezillon
2018-11-19  6:16     ` Heiko Schocher
2018-11-19  9:25       ` Miquel Raynal
2018-11-19 11:57       ` Boris Brezillon
2018-11-19 12:10         ` Heiko Schocher
2018-11-16 14:40 ` [U-Boot] [PATCH 4/4] mtd: sf: Make sure we don't register the same device twice Boris Brezillon

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.