[PATCH 1/2] mtd: Fix the check on nvmem_register() ret code

[PATCH 1/2] mtd: Fix the check on nvmem_register() ret code

[Boris Brezillon]

Commit 20167b70c894 ("nvmem: use EOPNOTSUPP instead of ENOSYS") changed
the nvmem_register() ret code from ENOSYS to EOPNOTSUPP when
CONFIG_NVMEM is not enabled, but the check in mtd_nvmem_add() was not
adjusted accordingly.

Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Alban Bedel <albeu@free.fr>
Fixes: c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem API")
Reported-by: kernel test robot <rong.a.chen@intel.com>
Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
---
 drivers/mtd/mtdcore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 21e3cdc04036..999b705769a8 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -522,7 +522,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
 	mtd->nvmem = nvmem_register(&config);
 	if (IS_ERR(mtd->nvmem)) {
 		/* Just ignore if there is no NVMEM support in the kernel */
-		if (PTR_ERR(mtd->nvmem) == -ENOSYS) {
+		if (PTR_ERR(mtd->nvmem) == -EOPNOTSUPP) {
 			mtd->nvmem = NULL;
 		} else {
 			dev_err(&mtd->dev, "Failed to register NVMEM device\n");
-- 
2.17.1

[PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

add_mtd_device() can fail. We should always check its return value
and gracefully handle the failure case. Fix the call sites where this
not done (in mtdpart.c) and add a __must_check attribute to the
prototype to avoid this kind of mistakes.

Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
---
No Fixes or Cc-stable tag here, as this seems to have worked just fine
without checking add_mtd_device() ret code until we started to expose
MTD devices as NVMEM providers (queued for 4.21).
---
 drivers/mtd/mtdcore.h |  2 +-
 drivers/mtd/mtdpart.c | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/mtdcore.h b/drivers/mtd/mtdcore.h
index 9887bda317cd..b31c868019ad 100644
--- a/drivers/mtd/mtdcore.h
+++ b/drivers/mtd/mtdcore.h
@@ -7,7 +7,7 @@
 extern struct mutex mtd_table_mutex;
 
 struct mtd_info *__mtd_next_device(int i);
-int add_mtd_device(struct mtd_info *mtd);
+int __must_check add_mtd_device(struct mtd_info *mtd);
 int del_mtd_device(struct mtd_info *mtd);
 int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
 int del_mtd_partitions(struct mtd_info *);
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b6af41b04622..60104e1079c5 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -618,10 +618,22 @@ int mtd_add_partition(struct mtd_info *parent, const char *name,
 	list_add(&new->list, &mtd_partitions);
 	mutex_unlock(&mtd_partitions_mutex);
 
-	add_mtd_device(&new->mtd);
+	ret = add_mtd_device(&new->mtd);
+	if (ret)
+		goto err_remove_part;
 
 	mtd_add_partition_attrs(new);
 
+	return 0;
+
+err_remove_part:
+	mutex_lock(&mtd_partitions_mutex);
+	list_del(&new->list);
+	mutex_unlock(&mtd_partitions_mutex);
+
+	free_partition(new);
+	pr_info("%s:%i\n", __func__, __LINE__);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(mtd_add_partition);
@@ -712,22 +724,31 @@ int add_mtd_partitions(struct mtd_info *master,
 {
 	struct mtd_part *slave;
 	uint64_t cur_offset = 0;
-	int i;
+	int i, ret;
 
 	printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
 	for (i = 0; i < nbparts; i++) {
 		slave = allocate_partition(master, parts + i, i, cur_offset);
 		if (IS_ERR(slave)) {
-			del_mtd_partitions(master);
-			return PTR_ERR(slave);
+			ret = PTR_ERR(slave);
+			goto err_del_partitions;
 		}
 
 		mutex_lock(&mtd_partitions_mutex);
 		list_add(&slave->list, &mtd_partitions);
 		mutex_unlock(&mtd_partitions_mutex);
 
-		add_mtd_device(&slave->mtd);
+		ret = add_mtd_device(&slave->mtd);
+		if (ret) {
+			mutex_lock(&mtd_partitions_mutex);
+			list_del(&slave->list);
+			mutex_unlock(&mtd_partitions_mutex);
+
+			free_partition(slave);
+			goto err_del_partitions;
+		}
+
 		mtd_add_partition_attrs(slave);
 		/* Look for subpartitions */
 		parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
@@ -736,6 +757,11 @@ int add_mtd_partitions(struct mtd_info *master,
 	}
 
 	return 0;
+
+err_del_partitions:
+	del_mtd_partitions(master);
+
+	return ret;
 }
 
 static DEFINE_SPINLOCK(part_parser_lock);
-- 
2.17.1

Re: [PATCH 1/2] mtd: Fix the check on nvmem_register() ret code

[Bartosz Golaszewski]

śr., 2 sty 2019 o 15:37 Boris Brezillon <bbrezillon@kernel.org> napisał(a):
>
> Commit 20167b70c894 ("nvmem: use EOPNOTSUPP instead of ENOSYS") changed
> the nvmem_register() ret code from ENOSYS to EOPNOTSUPP when
> CONFIG_NVMEM is not enabled, but the check in mtd_nvmem_add() was not
> adjusted accordingly.
>
> Cc: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Alban Bedel <albeu@free.fr>
> Fixes: c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem API")
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> ---
>  drivers/mtd/mtdcore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 21e3cdc04036..999b705769a8 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -522,7 +522,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
>         mtd->nvmem = nvmem_register(&config);
>         if (IS_ERR(mtd->nvmem)) {
>                 /* Just ignore if there is no NVMEM support in the kernel */
> -               if (PTR_ERR(mtd->nvmem) == -ENOSYS) {
> +               if (PTR_ERR(mtd->nvmem) == -EOPNOTSUPP) {
>                         mtd->nvmem = NULL;
>                 } else {
>                         dev_err(&mtd->dev, "Failed to register NVMEM device\n");
> --
> 2.17.1
>

Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Re: [2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

From: Boris Brezillon <boris.brezillon@bootlin.com>

On Wed, 2019-01-02 at 14:36:54 UTC, Boris Brezillon wrote:
> add_mtd_device() can fail. We should always check its return value
> and gracefully handle the failure case. Fix the call sites where this
> not done (in mtdpart.c) and add a __must_check attribute to the
> prototype to avoid this kind of mistakes.
> 
> Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris

Re: [1/2] mtd: Fix the check on nvmem_register() ret code

[Boris Brezillon]

From: Boris Brezillon <boris.brezillon@bootlin.com>

On Wed, 2019-01-02 at 14:36:53 UTC, Boris Brezillon wrote:
> Commit 20167b70c894 ("nvmem: use EOPNOTSUPP instead of ENOSYS") changed
> the nvmem_register() ret code from ENOSYS to EOPNOTSUPP when
> CONFIG_NVMEM is not enabled, but the check in mtd_nvmem_add() was not
> adjusted accordingly.
> 
> Cc: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Alban Bedel <albeu@free.fr>
> Fixes: c4dfa25ab307 ("mtd: add support for reading MTD devices via the nvmem API")
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> add_mtd_device() can fail. We should always check its return value
> and gracefully handle the failure case. Fix the call sites where this
> not done (in mtdpart.c) and add a __must_check attribute to the
> prototype to avoid this kind of mistakes.
>
> Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> ---
> No Fixes or Cc-stable tag here, as this seems to have worked just fine
> without checking add_mtd_device() ret code until we started to expose
> MTD devices as NVMEM providers (queued for 4.21).

Oh yes ;-)

https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/

Your patch is very similar to mine, so the crash is gone.
However, the warning is still there:

    m25p80 spi0.0: s25sl032p (4096 Kbytes)
    3 fixed-partitions partitions found on MTD device spi0.0
    Creating 3 MTD partitions on "spi0.0":
    0x000000000000-0x000000080000 : "loader"
    0x000000080000-0x000000600000 : "user"
    mtd: partition "user" extends beyond the end of device "spi0.0" --
size truncated to 0x380000
    0x000000600000-0x000004000000 : "flash"
    mtd: partition "flash" is out of reach -- disabled
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
add_mtd_device+0x90/0x3b0

Interestingly, only one partition is created, covering the full size of the
device:

    # cat /proc/partitions
    major minor  #blocks  name

      31        0       4096 mtdblock0

While I would expect two partitions, "loader" and truncated "user":

      31        0        512 mtdblock0
      31        1       3584 mtdblock1

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

On Tue, 22 Jan 2019 12:21:11 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > add_mtd_device() can fail. We should always check its return value
> > and gracefully handle the failure case. Fix the call sites where this
> > not done (in mtdpart.c) and add a __must_check attribute to the
> > prototype to avoid this kind of mistakes.
> >
> > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > ---
> > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > without checking add_mtd_device() ret code until we started to expose
> > MTD devices as NVMEM providers (queued for 4.21).  
> 
> Oh yes ;-)
> 
> https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> 
> Your patch is very similar to mine, so the crash is gone.

Oops, sorry about that. I completely forgot about this patch. It seems
the discussion led to a different conclusion though (patch
allocate_partitions() to reject wrong parts early) and the v2 was never
sent (or I missed it). Anyway, I guess we should have done both (check
add_mtd_device() ret code everywhere and patch allocate_partitions() to
reject bad parts early).

> However, the warning is still there:
> 
>     m25p80 spi0.0: s25sl032p (4096 Kbytes)
>     3 fixed-partitions partitions found on MTD device spi0.0
>     Creating 3 MTD partitions on "spi0.0":
>     0x000000000000-0x000000080000 : "loader"
>     0x000000080000-0x000000600000 : "user"
>     mtd: partition "user" extends beyond the end of device "spi0.0" --
> size truncated to 0x380000
>     0x000000600000-0x000004000000 : "flash"
>     mtd: partition "flash" is out of reach -- disabled
>     ------------[ cut here ]------------
>     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> add_mtd_device+0x90/0x3b0
> 
> Interestingly, only one partition is created, covering the full size of the
> device:
> 
>     # cat /proc/partitions
>     major minor  #blocks  name
> 
>       31        0       4096 mtdblock0
> 
> While I would expect two partitions, "loader" and truncated "user":
> 
>       31        0        512 mtdblock0
>       31        1       3584 mtdblock1

Yes, makes sense, I guess your patch was better than mine :-/. Can you
try with the following diff applied and let me know if it solves the
problem?

--->8---
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 60104e1079c5..aefd3344991f 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -724,16 +724,14 @@ int add_mtd_partitions(struct mtd_info *master,
 {
        struct mtd_part *slave;
        uint64_t cur_offset = 0;
-       int i, ret;
+       int i, ret, actual_nbparts = 0;
 
        printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
        for (i = 0; i < nbparts; i++) {
                slave = allocate_partition(master, parts + i, i, cur_offset);
-               if (IS_ERR(slave)) {
-                       ret = PTR_ERR(slave);
-                       goto err_del_partitions;
-               }
+               if (IS_ERR(slave))
+                       continue;
 
                mutex_lock(&mtd_partitions_mutex);
                list_add(&slave->list, &mtd_partitions);
@@ -746,7 +744,7 @@ int add_mtd_partitions(struct mtd_info *master,
                        mutex_unlock(&mtd_partitions_mutex);
 
                        free_partition(slave);
-                       goto err_del_partitions;
+                       continue;
                }
 
                mtd_add_partition_attrs(slave);
@@ -754,14 +752,10 @@ int add_mtd_partitions(struct mtd_info *master,
                parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
 
                cur_offset = slave->offset + slave->mtd.size;
+               actual_nbparts++;
        }
 
-       return 0;
-
-err_del_partitions:
-       del_mtd_partitions(master);
-
-       return ret;
+       return actual_nbparts;
 }
 
 static DEFINE_SPINLOCK(part_parser_lock);
@@ -1003,10 +997,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
                }
                /* Found partitions! */
                if (ret > 0) {
-                       err = add_mtd_partitions(master, pparts.parts,
+                       ret = add_mtd_partitions(master, pparts.parts,
                                                 pparts.nr_parts);
                        mtd_part_parser_cleanup(&pparts);
-                       return err ? err : pparts.nr_parts;
+                       return ret;
                }
                /*
                 * Stash the first error we see; only report it if no parser

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

Hi Geert,

On Tue, 22 Jan 2019 13:31:59 +0100
Boris Brezillon <bbrezillon@kernel.org> wrote:

> On Tue, 22 Jan 2019 12:21:11 +0100
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> > Hi Boris,
> > 
> > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > add_mtd_device() can fail. We should always check its return value
> > > and gracefully handle the failure case. Fix the call sites where this
> > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > prototype to avoid this kind of mistakes.
> > >
> > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > ---
> > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > without checking add_mtd_device() ret code until we started to expose
> > > MTD devices as NVMEM providers (queued for 4.21).    
> > 
> > Oh yes ;-)
> > 
> > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > 
> > Your patch is very similar to mine, so the crash is gone.  
> 
> Oops, sorry about that. I completely forgot about this patch. It seems
> the discussion led to a different conclusion though (patch
> allocate_partitions() to reject wrong parts early) and the v2 was never
> sent (or I missed it). Anyway, I guess we should have done both (check
> add_mtd_device() ret code everywhere and patch allocate_partitions() to
> reject bad parts early).
> 
> > However, the warning is still there:
> > 
> >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> >     3 fixed-partitions partitions found on MTD device spi0.0
> >     Creating 3 MTD partitions on "spi0.0":
> >     0x000000000000-0x000000080000 : "loader"
> >     0x000000080000-0x000000600000 : "user"
> >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > size truncated to 0x380000
> >     0x000000600000-0x000004000000 : "flash"
> >     mtd: partition "flash" is out of reach -- disabled
> >     ------------[ cut here ]------------
> >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > add_mtd_device+0x90/0x3b0
> > 
> > Interestingly, only one partition is created, covering the full size of the
> > device:
> > 
> >     # cat /proc/partitions
> >     major minor  #blocks  name
> > 
> >       31        0       4096 mtdblock0
> > 
> > While I would expect two partitions, "loader" and truncated "user":
> > 
> >       31        0        512 mtdblock0
> >       31        1       3584 mtdblock1  
> 
> Yes, makes sense, I guess your patch was better than mine :-/. Can you
> try with the following diff applied and let me know if it solves the
> problem?

Gentle ping: is this diff fixing your problem, and do you want me to
send a proper patch for it or should I let you send one?

> 
> --->8---  
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 60104e1079c5..aefd3344991f 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -724,16 +724,14 @@ int add_mtd_partitions(struct mtd_info *master,
>  {
>         struct mtd_part *slave;
>         uint64_t cur_offset = 0;
> -       int i, ret;
> +       int i, ret, actual_nbparts = 0;
>  
>         printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
>  
>         for (i = 0; i < nbparts; i++) {
>                 slave = allocate_partition(master, parts + i, i, cur_offset);
> -               if (IS_ERR(slave)) {
> -                       ret = PTR_ERR(slave);
> -                       goto err_del_partitions;
> -               }
> +               if (IS_ERR(slave))
> +                       continue;
>  
>                 mutex_lock(&mtd_partitions_mutex);
>                 list_add(&slave->list, &mtd_partitions);
> @@ -746,7 +744,7 @@ int add_mtd_partitions(struct mtd_info *master,
>                         mutex_unlock(&mtd_partitions_mutex);
>  
>                         free_partition(slave);
> -                       goto err_del_partitions;
> +                       continue;
>                 }
>  
>                 mtd_add_partition_attrs(slave);
> @@ -754,14 +752,10 @@ int add_mtd_partitions(struct mtd_info *master,
>                 parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
>  
>                 cur_offset = slave->offset + slave->mtd.size;
> +               actual_nbparts++;
>         }
>  
> -       return 0;
> -
> -err_del_partitions:
> -       del_mtd_partitions(master);
> -
> -       return ret;
> +       return actual_nbparts;
>  }
>  
>  static DEFINE_SPINLOCK(part_parser_lock);
> @@ -1003,10 +997,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
>                 }
>                 /* Found partitions! */
>                 if (ret > 0) {
> -                       err = add_mtd_partitions(master, pparts.parts,
> +                       ret = add_mtd_partitions(master, pparts.parts,
>                                                  pparts.nr_parts);
>                         mtd_part_parser_cleanup(&pparts);
> -                       return err ? err : pparts.nr_parts;
> +                       return ret;
>                 }
>                 /*
>                  * Stash the first error we see; only report it if no parser


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> On Tue, 22 Jan 2019 13:31:59 +0100
> Boris Brezillon <bbrezillon@kernel.org> wrote:
> > On Tue, 22 Jan 2019 12:21:11 +0100
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > add_mtd_device() can fail. We should always check its return value
> > > > and gracefully handle the failure case. Fix the call sites where this
> > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > prototype to avoid this kind of mistakes.
> > > >
> > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > ---
> > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > without checking add_mtd_device() ret code until we started to expose
> > > > MTD devices as NVMEM providers (queued for 4.21).
> > >
> > > Oh yes ;-)
> > >
> > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > >
> > > Your patch is very similar to mine, so the crash is gone.
> >
> > Oops, sorry about that. I completely forgot about this patch. It seems
> > the discussion led to a different conclusion though (patch
> > allocate_partitions() to reject wrong parts early) and the v2 was never
> > sent (or I missed it). Anyway, I guess we should have done both (check
> > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > reject bad parts early).
> >
> > > However, the warning is still there:
> > >
> > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > >     3 fixed-partitions partitions found on MTD device spi0.0
> > >     Creating 3 MTD partitions on "spi0.0":
> > >     0x000000000000-0x000000080000 : "loader"
> > >     0x000000080000-0x000000600000 : "user"
> > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > size truncated to 0x380000
> > >     0x000000600000-0x000004000000 : "flash"
> > >     mtd: partition "flash" is out of reach -- disabled
> > >     ------------[ cut here ]------------
> > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > add_mtd_device+0x90/0x3b0
> > >
> > > Interestingly, only one partition is created, covering the full size of the
> > > device:
> > >
> > >     # cat /proc/partitions
> > >     major minor  #blocks  name
> > >
> > >       31        0       4096 mtdblock0
> > >
> > > While I would expect two partitions, "loader" and truncated "user":
> > >
> > >       31        0        512 mtdblock0
> > >       31        1       3584 mtdblock1
> >
> > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > try with the following diff applied and let me know if it solves the
> > problem?
>
> Gentle ping: is this diff fixing your problem, and do you want me to
> send a proper patch for it or should I let you send one?

Yes, the diff below fixes the partitions for me, so
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Note that the warning is still there, but that's probably OK.

> > --- a/drivers/mtd/mtdpart.c
> > +++ b/drivers/mtd/mtdpart.c
> > @@ -724,16 +724,14 @@ int add_mtd_partitions(struct mtd_info *master,
> >  {
> >         struct mtd_part *slave;
> >         uint64_t cur_offset = 0;
> > -       int i, ret;
> > +       int i, ret, actual_nbparts = 0;
> >
> >         printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
> >
> >         for (i = 0; i < nbparts; i++) {
> >                 slave = allocate_partition(master, parts + i, i, cur_offset);
> > -               if (IS_ERR(slave)) {
> > -                       ret = PTR_ERR(slave);
> > -                       goto err_del_partitions;
> > -               }
> > +               if (IS_ERR(slave))
> > +                       continue;
> >
> >                 mutex_lock(&mtd_partitions_mutex);
> >                 list_add(&slave->list, &mtd_partitions);
> > @@ -746,7 +744,7 @@ int add_mtd_partitions(struct mtd_info *master,
> >                         mutex_unlock(&mtd_partitions_mutex);
> >
> >                         free_partition(slave);
> > -                       goto err_del_partitions;
> > +                       continue;
> >                 }
> >
> >                 mtd_add_partition_attrs(slave);
> > @@ -754,14 +752,10 @@ int add_mtd_partitions(struct mtd_info *master,
> >                 parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
> >
> >                 cur_offset = slave->offset + slave->mtd.size;
> > +               actual_nbparts++;
> >         }
> >
> > -       return 0;
> > -
> > -err_del_partitions:
> > -       del_mtd_partitions(master);
> > -
> > -       return ret;
> > +       return actual_nbparts;
> >  }
> >
> >  static DEFINE_SPINLOCK(part_parser_lock);
> > @@ -1003,10 +997,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
> >                 }
> >                 /* Found partitions! */
> >                 if (ret > 0) {
> > -                       err = add_mtd_partitions(master, pparts.parts,
> > +                       ret = add_mtd_partitions(master, pparts.parts,
> >                                                  pparts.nr_parts);
> >                         mtd_part_parser_cleanup(&pparts);
> > -                       return err ? err : pparts.nr_parts;
> > +                       return ret;
> >                 }
> >                 /*
> >                  * Stash the first error we see; only report it if no parser

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

On Tue, 29 Jan 2019 16:29:55 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > On Tue, 22 Jan 2019 13:31:59 +0100
> > Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > add_mtd_device() can fail. We should always check its return value
> > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > prototype to avoid this kind of mistakes.
> > > > >
> > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > ---
> > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > MTD devices as NVMEM providers (queued for 4.21).  
> > > >
> > > > Oh yes ;-)
> > > >
> > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > >
> > > > Your patch is very similar to mine, so the crash is gone.  
> > >
> > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > the discussion led to a different conclusion though (patch
> > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > reject bad parts early).
> > >  
> > > > However, the warning is still there:
> > > >
> > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > >     Creating 3 MTD partitions on "spi0.0":
> > > >     0x000000000000-0x000000080000 : "loader"
> > > >     0x000000080000-0x000000600000 : "user"
> > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > size truncated to 0x380000
> > > >     0x000000600000-0x000004000000 : "flash"
> > > >     mtd: partition "flash" is out of reach -- disabled
> > > >     ------------[ cut here ]------------
> > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > add_mtd_device+0x90/0x3b0
> > > >
> > > > Interestingly, only one partition is created, covering the full size of the
> > > > device:
> > > >
> > > >     # cat /proc/partitions
> > > >     major minor  #blocks  name
> > > >
> > > >       31        0       4096 mtdblock0
> > > >
> > > > While I would expect two partitions, "loader" and truncated "user":
> > > >
> > > >       31        0        512 mtdblock0
> > > >       31        1       3584 mtdblock1  
> > >
> > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > try with the following diff applied and let me know if it solves the
> > > problem?  
> >
> > Gentle ping: is this diff fixing your problem, and do you want me to
> > send a proper patch for it or should I let you send one?  
> 
> Yes, the diff below fixes the partitions for me, so
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Note that the warning is still there, but that's probably OK.

You mean the pr_warn() or the WARN_ON() backtrace? The former is
expected not the latter.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

On Tue, 29 Jan 2019 16:29:55 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > On Tue, 22 Jan 2019 13:31:59 +0100
> > Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > add_mtd_device() can fail. We should always check its return value
> > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > prototype to avoid this kind of mistakes.
> > > > >
> > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > ---
> > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > MTD devices as NVMEM providers (queued for 4.21).  
> > > >
> > > > Oh yes ;-)
> > > >
> > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > >
> > > > Your patch is very similar to mine, so the crash is gone.  
> > >
> > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > the discussion led to a different conclusion though (patch
> > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > reject bad parts early).
> > >  
> > > > However, the warning is still there:
> > > >
> > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > >     Creating 3 MTD partitions on "spi0.0":
> > > >     0x000000000000-0x000000080000 : "loader"
> > > >     0x000000080000-0x000000600000 : "user"
> > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > size truncated to 0x380000
> > > >     0x000000600000-0x000004000000 : "flash"
> > > >     mtd: partition "flash" is out of reach -- disabled
> > > >     ------------[ cut here ]------------
> > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > add_mtd_device+0x90/0x3b0
> > > >
> > > > Interestingly, only one partition is created, covering the full size of the
> > > > device:
> > > >
> > > >     # cat /proc/partitions
> > > >     major minor  #blocks  name
> > > >
> > > >       31        0       4096 mtdblock0
> > > >
> > > > While I would expect two partitions, "loader" and truncated "user":
> > > >
> > > >       31        0        512 mtdblock0
> > > >       31        1       3584 mtdblock1  
> > >
> > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > try with the following diff applied and let me know if it solves the
> > > problem?  
> >
> > Gentle ping: is this diff fixing your problem, and do you want me to
> > send a proper patch for it or should I let you send one?  
> 
> Yes, the diff below fixes the partitions for me, so
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Note that the warning is still there, but that's probably OK.

Just out of curiosity, why do you need to define parts that do not fit
in the flash?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Wed, Jan 30, 2019 at 9:52 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> On Tue, 29 Jan 2019 16:29:55 +0100
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > On Tue, 22 Jan 2019 13:31:59 +0100
> > > Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > > > add_mtd_device() can fail. We should always check its return value
> > > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > > prototype to avoid this kind of mistakes.
> > > > > >
> > > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > > ---
> > > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > > MTD devices as NVMEM providers (queued for 4.21).
> > > > >
> > > > > Oh yes ;-)
> > > > >
> > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > > >
> > > > > Your patch is very similar to mine, so the crash is gone.
> > > >
> > > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > > the discussion led to a different conclusion though (patch
> > > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > > reject bad parts early).
> > > >
> > > > > However, the warning is still there:
> > > > >
> > > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > > >     Creating 3 MTD partitions on "spi0.0":
> > > > >     0x000000000000-0x000000080000 : "loader"
> > > > >     0x000000080000-0x000000600000 : "user"
> > > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > > size truncated to 0x380000
> > > > >     0x000000600000-0x000004000000 : "flash"
> > > > >     mtd: partition "flash" is out of reach -- disabled
> > > > >     ------------[ cut here ]------------
> > > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > > add_mtd_device+0x90/0x3b0
> > > > >
> > > > > Interestingly, only one partition is created, covering the full size of the
> > > > > device:
> > > > >
> > > > >     # cat /proc/partitions
> > > > >     major minor  #blocks  name
> > > > >
> > > > >       31        0       4096 mtdblock0
> > > > >
> > > > > While I would expect two partitions, "loader" and truncated "user":
> > > > >
> > > > >       31        0        512 mtdblock0
> > > > >       31        1       3584 mtdblock1
> > > >
> > > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > > try with the following diff applied and let me know if it solves the
> > > > problem?
> > >
> > > Gentle ping: is this diff fixing your problem, and do you want me to
> > > send a proper patch for it or should I let you send one?
> >
> > Yes, the diff below fixes the partitions for me, so
> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Note that the warning is still there, but that's probably OK.
>
> You mean the pr_warn() or the WARN_ON() backtrace? The former is
> expected not the latter.

The WARN_ON() backtrace is still there.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

On Wed, 30 Jan 2019 10:05:29 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Wed, Jan 30, 2019 at 9:52 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > On Tue, 29 Jan 2019 16:29:55 +0100
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > On Tue, 22 Jan 2019 13:31:59 +0100
> > > > Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > > > add_mtd_device() can fail. We should always check its return value
> > > > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > > > prototype to avoid this kind of mistakes.
> > > > > > >
> > > > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > > > ---
> > > > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > > > MTD devices as NVMEM providers (queued for 4.21).  
> > > > > >
> > > > > > Oh yes ;-)
> > > > > >
> > > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > > > >
> > > > > > Your patch is very similar to mine, so the crash is gone.  
> > > > >
> > > > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > > > the discussion led to a different conclusion though (patch
> > > > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > > > reject bad parts early).
> > > > >  
> > > > > > However, the warning is still there:
> > > > > >
> > > > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > > > >     Creating 3 MTD partitions on "spi0.0":
> > > > > >     0x000000000000-0x000000080000 : "loader"
> > > > > >     0x000000080000-0x000000600000 : "user"
> > > > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > > > size truncated to 0x380000
> > > > > >     0x000000600000-0x000004000000 : "flash"
> > > > > >     mtd: partition "flash" is out of reach -- disabled
> > > > > >     ------------[ cut here ]------------
> > > > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > > > add_mtd_device+0x90/0x3b0
> > > > > >
> > > > > > Interestingly, only one partition is created, covering the full size of the
> > > > > > device:
> > > > > >
> > > > > >     # cat /proc/partitions
> > > > > >     major minor  #blocks  name
> > > > > >
> > > > > >       31        0       4096 mtdblock0
> > > > > >
> > > > > > While I would expect two partitions, "loader" and truncated "user":
> > > > > >
> > > > > >       31        0        512 mtdblock0
> > > > > >       31        1       3584 mtdblock1  
> > > > >
> > > > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > > > try with the following diff applied and let me know if it solves the
> > > > > problem?  
> > > >
> > > > Gentle ping: is this diff fixing your problem, and do you want me to
> > > > send a proper patch for it or should I let you send one?  
> > >
> > > Yes, the diff below fixes the partitions for me, so
> > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > Note that the warning is still there, but that's probably OK.  
> >
> > You mean the pr_warn() or the WARN_ON() backtrace? The former is
> > expected not the latter.  
> 
> The WARN_ON() backtrace is still there.

I don't see a WARN_ON() at drivers/mtd/mtdcore.c:571. Which branch are
you using for your tests?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Wed, Jan 30, 2019 at 9:55 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> On Tue, 29 Jan 2019 16:29:55 +0100
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > On Tue, 22 Jan 2019 13:31:59 +0100
> > > Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > > > add_mtd_device() can fail. We should always check its return value
> > > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > > prototype to avoid this kind of mistakes.
> > > > > >
> > > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > > ---
> > > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > > MTD devices as NVMEM providers (queued for 4.21).
> > > > >
> > > > > Oh yes ;-)
> > > > >
> > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > > >
> > > > > Your patch is very similar to mine, so the crash is gone.
> > > >
> > > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > > the discussion led to a different conclusion though (patch
> > > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > > reject bad parts early).
> > > >
> > > > > However, the warning is still there:
> > > > >
> > > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > > >     Creating 3 MTD partitions on "spi0.0":
> > > > >     0x000000000000-0x000000080000 : "loader"
> > > > >     0x000000080000-0x000000600000 : "user"
> > > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > > size truncated to 0x380000
> > > > >     0x000000600000-0x000004000000 : "flash"
> > > > >     mtd: partition "flash" is out of reach -- disabled
> > > > >     ------------[ cut here ]------------
> > > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > > add_mtd_device+0x90/0x3b0
> > > > >
> > > > > Interestingly, only one partition is created, covering the full size of the
> > > > > device:
> > > > >
> > > > >     # cat /proc/partitions
> > > > >     major minor  #blocks  name
> > > > >
> > > > >       31        0       4096 mtdblock0
> > > > >
> > > > > While I would expect two partitions, "loader" and truncated "user":
> > > > >
> > > > >       31        0        512 mtdblock0
> > > > >       31        1       3584 mtdblock1
> > > >
> > > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > > try with the following diff applied and let me know if it solves the
> > > > problem?
> > >
> > > Gentle ping: is this diff fixing your problem, and do you want me to
> > > send a proper patch for it or should I let you send one?
> >
> > Yes, the diff below fixes the partitions for me, so
> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Note that the warning is still there, but that's probably OK.
>
> Just out of curiosity, why do you need to define parts that do not fit
> in the flash?

All of this started as an accident on my side ;-)
The Koelsch board has 2 QSPI FLASHes: the first is 64 MiB large, the
second is 4 MiB large.  Which FLASH is used is selected by a switch, but
the partitioning is specified in DT.
I wanted to use the smaller FLASH for some test, but forgot to update the
partitioning in DT, leading to:

    mtd: partition "user" extends beyond the end of device "spi0.0" --
size truncated to 0x380000

That kernel message indicated MTD tried to handle this gracefully, but
still, the kernel crashed later.

Anyway, it may be considered good practice to handle bad partition
tables, like is done for disk partitioning, so I think it's worthwhile to
have this fixed, which is what your patches achieve.
There's still a WARN_ON() drawing the user's attention, though.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Wed, Jan 30, 2019 at 10:10 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> On Wed, 30 Jan 2019 10:05:29 +0100
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Jan 30, 2019 at 9:52 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > On Tue, 29 Jan 2019 16:29:55 +0100
> > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > > On Tue, 22 Jan 2019 13:31:59 +0100
> > > > > Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > > > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > > > > > add_mtd_device() can fail. We should always check its return value
> > > > > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > > > > prototype to avoid this kind of mistakes.
> > > > > > > >
> > > > > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > > > > ---
> > > > > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > > > > MTD devices as NVMEM providers (queued for 4.21).
> > > > > > >
> > > > > > > Oh yes ;-)
> > > > > > >
> > > > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > > > > >
> > > > > > > Your patch is very similar to mine, so the crash is gone.
> > > > > >
> > > > > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > > > > the discussion led to a different conclusion though (patch
> > > > > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > > > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > > > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > > > > reject bad parts early).
> > > > > >
> > > > > > > However, the warning is still there:
> > > > > > >
> > > > > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > > > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > > > > >     Creating 3 MTD partitions on "spi0.0":
> > > > > > >     0x000000000000-0x000000080000 : "loader"
> > > > > > >     0x000000080000-0x000000600000 : "user"
> > > > > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > > > > size truncated to 0x380000
> > > > > > >     0x000000600000-0x000004000000 : "flash"
> > > > > > >     mtd: partition "flash" is out of reach -- disabled
> > > > > > >     ------------[ cut here ]------------
> > > > > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > > > > add_mtd_device+0x90/0x3b0
> > > > > > >
> > > > > > > Interestingly, only one partition is created, covering the full size of the
> > > > > > > device:
> > > > > > >
> > > > > > >     # cat /proc/partitions
> > > > > > >     major minor  #blocks  name
> > > > > > >
> > > > > > >       31        0       4096 mtdblock0
> > > > > > >
> > > > > > > While I would expect two partitions, "loader" and truncated "user":
> > > > > > >
> > > > > > >       31        0        512 mtdblock0
> > > > > > >       31        1       3584 mtdblock1
> > > > > >
> > > > > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > > > > try with the following diff applied and let me know if it solves the
> > > > > > problem?
> > > > >
> > > > > Gentle ping: is this diff fixing your problem, and do you want me to
> > > > > send a proper patch for it or should I let you send one?
> > > >
> > > > Yes, the diff below fixes the partitions for me, so
> > > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > >
> > > > Note that the warning is still there, but that's probably OK.
> > >
> > > You mean the pr_warn() or the WARN_ON() backtrace? The former is
> > > expected not the latter.
> >
> > The WARN_ON() backtrace is still there.
>
> I don't see a WARN_ON() at drivers/mtd/mtdcore.c:571. Which branch are
> you using for your tests?

A tree based on last week's l2-mtd/master, i.e. lacking commit f7fd818cca0cea3d
("mtd: Remove empty lines at end of sysfs related functions").

This is the one triggering:

        if (WARN_ON((!mtd->erasesize || !mtd->_erase) &&
                    !(mtd->flags & MTD_NO_ERASE)))
                return -EINVAL;

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

On Wed, 30 Jan 2019 10:05:29 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Wed, Jan 30, 2019 at 9:52 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > On Tue, 29 Jan 2019 16:29:55 +0100
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > On Tue, 22 Jan 2019 13:31:59 +0100
> > > > Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > > > add_mtd_device() can fail. We should always check its return value
> > > > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > > > prototype to avoid this kind of mistakes.
> > > > > > >
> > > > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > > > ---
> > > > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > > > MTD devices as NVMEM providers (queued for 4.21).  
> > > > > >
> > > > > > Oh yes ;-)
> > > > > >
> > > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > > > >
> > > > > > Your patch is very similar to mine, so the crash is gone.  
> > > > >
> > > > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > > > the discussion led to a different conclusion though (patch
> > > > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > > > reject bad parts early).
> > > > >  
> > > > > > However, the warning is still there:
> > > > > >
> > > > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > > > >     Creating 3 MTD partitions on "spi0.0":
> > > > > >     0x000000000000-0x000000080000 : "loader"
> > > > > >     0x000000080000-0x000000600000 : "user"
> > > > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > > > size truncated to 0x380000
> > > > > >     0x000000600000-0x000004000000 : "flash"
> > > > > >     mtd: partition "flash" is out of reach -- disabled
> > > > > >     ------------[ cut here ]------------
> > > > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > > > add_mtd_device+0x90/0x3b0
> > > > > >
> > > > > > Interestingly, only one partition is created, covering the full size of the
> > > > > > device:
> > > > > >
> > > > > >     # cat /proc/partitions
> > > > > >     major minor  #blocks  name
> > > > > >
> > > > > >       31        0       4096 mtdblock0
> > > > > >
> > > > > > While I would expect two partitions, "loader" and truncated "user":
> > > > > >
> > > > > >       31        0        512 mtdblock0
> > > > > >       31        1       3584 mtdblock1  
> > > > >
> > > > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > > > try with the following diff applied and let me know if it solves the
> > > > > problem?  
> > > >
> > > > Gentle ping: is this diff fixing your problem, and do you want me to
> > > > send a proper patch for it or should I let you send one?  
> > >
> > > Yes, the diff below fixes the partitions for me, so
> > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > Note that the warning is still there, but that's probably OK.  
> >
> > You mean the pr_warn() or the WARN_ON() backtrace? The former is
> > expected not the latter.  
> 
> The WARN_ON() backtrace is still there.

Can you try with this patch instead of the previous one?

--->8---
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index e6d9467f6be0..37f174ccbcec 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -480,6 +480,10 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent,
                /* let's register it anyway to preserve ordering */
                slave->offset = 0;
                slave->mtd.size = 0;
+
+               /* Initialize ->erasesize to make add_mtd_device() happy. */
+               slave->mtd.erasesize = parent->erasesize;
+
                printk(KERN_ERR"mtd: partition \"%s\" is out of reach -- disabled\n",
                        part->name);
                goto out_register;

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

Hi Geert,

On Wed, 30 Jan 2019 10:16:33 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> 
> A tree based on last week's l2-mtd/master, i.e. lacking commit f7fd818cca0cea3d
> ("mtd: Remove empty lines at end of sysfs related functions").
> 
> This is the one triggering:
> 
>         if (WARN_ON((!mtd->erasesize || !mtd->_erase) &&
>                     !(mtd->flags & MTD_NO_ERASE)))
>                 return -EINVAL;

Do you think you'll have time to test [1]? I'd like to make that patch
part of my fixes PR if possible. If you can't, that's fine, just let me
know.

Thanks,

Boris

[1]http://patchwork.ozlabs.org/patch/1033458/

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Fri, Feb 1, 2019 at 9:50 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
> On Wed, 30 Jan 2019 10:16:33 +0100
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > A tree based on last week's l2-mtd/master, i.e. lacking commit f7fd818cca0cea3d
> > ("mtd: Remove empty lines at end of sysfs related functions").
> >
> > This is the one triggering:
> >
> >         if (WARN_ON((!mtd->erasesize || !mtd->_erase) &&
> >                     !(mtd->flags & MTD_NO_ERASE)))
> >                 return -EINVAL;
>
> Do you think you'll have time to test [1]? I'd like to make that patch
> part of my fixes PR if possible. If you can't, that's fine, just let me
> know.

Sure, I will, after FOSDEM ;-)

> [1]http://patchwork.ozlabs.org/patch/1033458/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Geert Uytterhoeven]

Hi Boris,

On Tue, Jan 29, 2019 at 4:29 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > On Tue, 22 Jan 2019 13:31:59 +0100
> > Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:
> > > > > add_mtd_device() can fail. We should always check its return value
> > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > prototype to avoid this kind of mistakes.
> > > > >
> > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > ---
> > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > MTD devices as NVMEM providers (queued for 4.21).
> > > >
> > > > Oh yes ;-)
> > > >
> > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > >
> > > > Your patch is very similar to mine, so the crash is gone.
> > >
> > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > the discussion led to a different conclusion though (patch
> > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > reject bad parts early).
> > >
> > > > However, the warning is still there:
> > > >
> > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > >     Creating 3 MTD partitions on "spi0.0":
> > > >     0x000000000000-0x000000080000 : "loader"
> > > >     0x000000080000-0x000000600000 : "user"
> > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > size truncated to 0x380000
> > > >     0x000000600000-0x000004000000 : "flash"
> > > >     mtd: partition "flash" is out of reach -- disabled
> > > >     ------------[ cut here ]------------
> > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > add_mtd_device+0x90/0x3b0
> > > >
> > > > Interestingly, only one partition is created, covering the full size of the
> > > > device:
> > > >
> > > >     # cat /proc/partitions
> > > >     major minor  #blocks  name
> > > >
> > > >       31        0       4096 mtdblock0
> > > >
> > > > While I would expect two partitions, "loader" and truncated "user":
> > > >
> > > >       31        0        512 mtdblock0
> > > >       31        1       3584 mtdblock1
> > >
> > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > try with the following diff applied and let me know if it solves the
> > > problem?
> >
> > Gentle ping: is this diff fixing your problem, and do you want me to
> > send a proper patch for it or should I let you send one?
>
> Yes, the diff below fixes the partitions for me, so
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Note that the warning is still there, but that's probably OK.

Given the warning was fixed by commit ad4635153034c20c ("mtd: Make sure
mtd->erasesize is valid even if the partition is of size 0"), do you have any
plans to apply the below?

Thanks!

> > > --- a/drivers/mtd/mtdpart.c
> > > +++ b/drivers/mtd/mtdpart.c
> > > @@ -724,16 +724,14 @@ int add_mtd_partitions(struct mtd_info *master,
> > >  {
> > >         struct mtd_part *slave;
> > >         uint64_t cur_offset = 0;
> > > -       int i, ret;
> > > +       int i, ret, actual_nbparts = 0;
> > >
> > >         printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
> > >
> > >         for (i = 0; i < nbparts; i++) {
> > >                 slave = allocate_partition(master, parts + i, i, cur_offset);
> > > -               if (IS_ERR(slave)) {
> > > -                       ret = PTR_ERR(slave);
> > > -                       goto err_del_partitions;
> > > -               }
> > > +               if (IS_ERR(slave))
> > > +                       continue;
> > >
> > >                 mutex_lock(&mtd_partitions_mutex);
> > >                 list_add(&slave->list, &mtd_partitions);
> > > @@ -746,7 +744,7 @@ int add_mtd_partitions(struct mtd_info *master,
> > >                         mutex_unlock(&mtd_partitions_mutex);
> > >
> > >                         free_partition(slave);
> > > -                       goto err_del_partitions;
> > > +                       continue;
> > >                 }
> > >
> > >                 mtd_add_partition_attrs(slave);
> > > @@ -754,14 +752,10 @@ int add_mtd_partitions(struct mtd_info *master,
> > >                 parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
> > >
> > >                 cur_offset = slave->offset + slave->mtd.size;
> > > +               actual_nbparts++;
> > >         }
> > >
> > > -       return 0;
> > > -
> > > -err_del_partitions:
> > > -       del_mtd_partitions(master);
> > > -
> > > -       return ret;
> > > +       return actual_nbparts;
> > >  }
> > >
> > >  static DEFINE_SPINLOCK(part_parser_lock);
> > > @@ -1003,10 +997,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
> > >                 }
> > >                 /* Found partitions! */
> > >                 if (ret > 0) {
> > > -                       err = add_mtd_partitions(master, pparts.parts,
> > > +                       ret = add_mtd_partitions(master, pparts.parts,
> > >                                                  pparts.nr_parts);
> > >                         mtd_part_parser_cleanup(&pparts);
> > > -                       return err ? err : pparts.nr_parts;
> > > +                       return ret;
> > >                 }
> > >                 /*
> > >                  * Stash the first error we see; only report it if no parser

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: Check add_mtd_device() ret code

[Boris Brezillon]

On Mon, 1 Apr 2019 11:50:01 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Tue, Jan 29, 2019 at 4:29 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Jan 29, 2019 at 12:03 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > On Tue, 22 Jan 2019 13:31:59 +0100
> > > Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > On Tue, 22 Jan 2019 12:21:11 +0100
> > > > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@kernel.org> wrote:  
> > > > > > add_mtd_device() can fail. We should always check its return value
> > > > > > and gracefully handle the failure case. Fix the call sites where this
> > > > > > not done (in mtdpart.c) and add a __must_check attribute to the
> > > > > > prototype to avoid this kind of mistakes.
> > > > > >
> > > > > > Signed-off-by: Boris Brezillon <bbrezillon@kernel.org>
> > > > > > ---
> > > > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine
> > > > > > without checking add_mtd_device() ret code until we started to expose
> > > > > > MTD devices as NVMEM providers (queued for 4.21).  
> > > > >
> > > > > Oh yes ;-)
> > > > >
> > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@gmail.com/t/
> > > > >
> > > > > Your patch is very similar to mine, so the crash is gone.  
> > > >
> > > > Oops, sorry about that. I completely forgot about this patch. It seems
> > > > the discussion led to a different conclusion though (patch
> > > > allocate_partitions() to reject wrong parts early) and the v2 was never
> > > > sent (or I missed it). Anyway, I guess we should have done both (check
> > > > add_mtd_device() ret code everywhere and patch allocate_partitions() to
> > > > reject bad parts early).
> > > >  
> > > > > However, the warning is still there:
> > > > >
> > > > >     m25p80 spi0.0: s25sl032p (4096 Kbytes)
> > > > >     3 fixed-partitions partitions found on MTD device spi0.0
> > > > >     Creating 3 MTD partitions on "spi0.0":
> > > > >     0x000000000000-0x000000080000 : "loader"
> > > > >     0x000000080000-0x000000600000 : "user"
> > > > >     mtd: partition "user" extends beyond the end of device "spi0.0" --
> > > > > size truncated to 0x380000
> > > > >     0x000000600000-0x000004000000 : "flash"
> > > > >     mtd: partition "flash" is out of reach -- disabled
> > > > >     ------------[ cut here ]------------
> > > > >     WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571
> > > > > add_mtd_device+0x90/0x3b0
> > > > >
> > > > > Interestingly, only one partition is created, covering the full size of the
> > > > > device:
> > > > >
> > > > >     # cat /proc/partitions
> > > > >     major minor  #blocks  name
> > > > >
> > > > >       31        0       4096 mtdblock0
> > > > >
> > > > > While I would expect two partitions, "loader" and truncated "user":
> > > > >
> > > > >       31        0        512 mtdblock0
> > > > >       31        1       3584 mtdblock1  
> > > >
> > > > Yes, makes sense, I guess your patch was better than mine :-/. Can you
> > > > try with the following diff applied and let me know if it solves the
> > > > problem?  
> > >
> > > Gentle ping: is this diff fixing your problem, and do you want me to
> > > send a proper patch for it or should I let you send one?  
> >
> > Yes, the diff below fixes the partitions for me, so
> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Note that the warning is still there, but that's probably OK.  
> 
> Given the warning was fixed by commit ad4635153034c20c ("mtd: Make sure
> mtd->erasesize is valid even if the partition is of size 0"), do you have any
> plans to apply the below?

Is it still needed now that the only case we care about has been fixed?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/