subject:"Re\: \[U\-Boot\] \[PATCH 3\/4\] mtd\: Delete partitions attached to the device when a device is deleted"

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

2018-11-19 Thread Heiko Schocher


Hello Boris,

Am 19.11.2018 um 12:57 schrieb Boris Brezillon:

On Mon, 19 Nov 2018 07:16:53 +0100
Heiko Schocher  wrote:


Hello Boris,

Am 17.11.2018 um 10:19 schrieb Boris Brezillon:

On Fri, 16 Nov 2018 15:40:25 +0100
Boris Brezillon  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 
---
   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(_table_mutex);
   
   	if (idr_find(_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: h...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

2018-11-19 Thread Boris Brezillon

On Mon, 19 Nov 2018 07:16:53 +0100
Heiko Schocher  wrote:

> Hello Boris,
> 
> Am 17.11.2018 um 10:19 schrieb Boris Brezillon:
> > On Fri, 16 Nov 2018 15:40:25 +0100
> > Boris Brezillon  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 
> >> ---
> >>   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(_table_mutex);
> >>   
> >>if (idr_find(_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.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

2018-11-19 Thread Miquel Raynal

Hi Heiko,

Heiko Schocher  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  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 
> >> ---
> >>   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(_table_mutex);  
> >>   >>   if (idr_find(_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

---

From 55421b43e624e3db51c1d80350be2cf576cf6356 Mon Sep 17 00:00:00 2001
From: Miquel Raynal 
Date: Mon, 19 Nov 2018 10:22:02 +0100
Subject: [PATCH] FIX: mtd: avoid erroring-out once mtd_dev_list_updated() has
 been called

Signed-off-by: Miquel Raynal 
---
 drivers/mtd/mtd_uboot.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 6a3e64395d..89bf856bcc 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -161,6 +161,15 @@ int mtd_probe_devices(void)
 
mtd_probe_uclass_mtd_devs();
 
+   /* If at least one partition is still in use, do not delete anything */
+   mtd_for_each_device(mtd) {
+   if (mtd->usecount) {
+   printf("Partition \"%s\" already in use, aborting\n",
+  mtd->name);
+   return -EACCES;
+   }
+   }
+
/*
 * Check if mtdparts/mtdids changed or if the MTD dev list was updated
 * since last call, otherwise: exit
@@ -178,15 +187,6 @@ int mtd_probe_devices(void)
old_mtdparts = strdup(mtdparts);
old_mtdids = strdup(mtdids);
 
-   /* If at least one partition is still in use, do not delete anything */
-   mtd_for_each_device(mtd) {
-   if (mtd->usecount) {
-   printf("Partition \"%s\" already in use, aborting\n",
-  mtd->name);
-   return -EACCES;
-   }
-   }
-
/*
 * Everything looks clear, remove all partitions. It is not safe to
 * remove entries from the mtd_for_each_device loop as it uses idr
-- 
2.17.1


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

2018-11-18 Thread Heiko Schocher


Hello Boris,

Am 17.11.2018 um 10:19 schrieb Boris Brezillon:

On Fri, 16 Nov 2018 15:40:25 +0100
Boris Brezillon  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 
---
  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(_table_mutex);
  
  	if (idr_find(_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: 0x2 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
  - 0x-0x0800 : "nand0"
  - 0x-0x0800 : "ubi"
* nor0
  - type: NOR flash
  - block size: 0x1 bytes
  - min I/O: 0x1 bytes
  - 0x-0x0080 : "nor0"
  - 0x-0x0001 : "spl"
  - 0x0001-0x000d : "u-boot"
  - 0x000d-0x000e : "env"
  - 0x000e-0x000f : "env-red"
  - 0x000f-0x0080 : "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: 0x2 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
  - 0x-0x0800 : "nand0"
  - 0x-0x0800 : "ubi"
* nor0
  - type: NOR flash
  - block size: 0x1 bytes
  - min I/O: 0x1 bytes
  - 0x-0x0080 : "nor0"
  - 0x-0x0001 : "spl"
  - 0x0001-0x000d : "u-boot"
  - 0x000d-0x000e : "env"
  - 0x000e-0x000f : "env-red"
  - 0x000f-0x0080 : "rescue"
=> sf probe
SF: Detected s25f064l with page size 256 Bytes, erase size 64 KiB, total 8 MiB
=> mtd list
Partition "ubi"

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

2018-11-17 Thread Boris Brezillon

On Fri, 16 Nov 2018 15:40:25 +0100
Boris Brezillon  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 
> ---
>  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(_table_mutex);
>  
>   if (idr_find(_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) \

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

Re: [U-Boot] [PATCH 3/4] mtd: Delete partitions attached to the device when a device is deleted

5 matches

Site Navigation

Mail list logo

Footer information