[RFC PATCH 4/5] mtd_blkdevs: scan partitions on mtdblock if FIT_PARTITION is set

[RFC PATCH 4/5] mtd_blkdevs: scan partitions on mtdblock if FIT_PARTITION is set

[Daniel Golle]

Enable partition parsers on plain mtdblock devices in case of
CONFIG_FIT_PARTITION being selected.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/mtd/mtd_blkdevs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index f7317211146550..e9759c4182f8d5 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -359,7 +359,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 	} else {
 		snprintf(gd->disk_name, sizeof(gd->disk_name),
 			 "%s%d", tr->name, new->devnum);
+#ifndef CONFIG_FIT_PARTITION
 		gd->flags |= GENHD_FL_NO_PART;
+#endif
 	}
 
 	set_capacity(gd, ((u64)new->size * tr->blksize) >> 9);
-- 
2.36.0

Re: [RFC PATCH 4/5] mtd_blkdevs: scan partitions on mtdblock if FIT_PARTITION is set

[Miquel Raynal]

Hi Daniel,

daniel@makrotopia.org wrote on Mon, 25 Apr 2022 16:00:02 +0100:

> Enable partition parsers on plain mtdblock devices in case of
> CONFIG_FIT_PARTITION being selected.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  drivers/mtd/mtd_blkdevs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index f7317211146550..e9759c4182f8d5 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -359,7 +359,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
>  	} else {
>  		snprintf(gd->disk_name, sizeof(gd->disk_name),
>  			 "%s%d", tr->name, new->devnum);
> +#ifndef CONFIG_FIT_PARTITION

Can we use a regular 'if' here?

>  		gd->flags |= GENHD_FL_NO_PART;
> +#endif
>  	}
>  
>  	set_capacity(gd, ((u64)new->size * tr->blksize) >> 9);

Thanks,
Miquèl

Re: [RFC PATCH 4/5] mtd_blkdevs: scan partitions on mtdblock if FIT_PARTITION is set

[Daniel Golle]

On Tue, Apr 26, 2022 at 09:37:10AM +0200, Miquel Raynal wrote:
> Hi Daniel,
> 
> daniel@makrotopia.org wrote on Mon, 25 Apr 2022 16:00:02 +0100:
> 
> > Enable partition parsers on plain mtdblock devices in case of
> > CONFIG_FIT_PARTITION being selected.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  drivers/mtd/mtd_blkdevs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> > index f7317211146550..e9759c4182f8d5 100644
> > --- a/drivers/mtd/mtd_blkdevs.c
> > +++ b/drivers/mtd/mtd_blkdevs.c
> > @@ -359,7 +359,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
> >  	} else {
> >  		snprintf(gd->disk_name, sizeof(gd->disk_name),
> >  			 "%s%d", tr->name, new->devnum);
> > +#ifndef CONFIG_FIT_PARTITION
> 
> Can we use a regular 'if' here?

I'll introduce additional Kconfig options to enable scanning for
(sub-)partitions on MTD block and UBI block devices as Christoph Hellwig
correctly critizied that re-using this unrelated Kconfig option is
misleading.

I'll also use
if (IS_ENABLED(...))
instead of
#ifdef ...
for better coverage testing.

Now going to wait a few more days for more comments and then re-send.

Thank you for the review!

> 
> >  		gd->flags |= GENHD_FL_NO_PART;
> > +#endif
> >  	}
> >  
> >  	set_capacity(gd, ((u64)new->size * tr->blksize) >> 9);
> 
> Thanks,
> Miquèl
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [RFC PATCH 4/5] mtd_blkdevs: scan partitions on mtdblock if FIT_PARTITION is set

[Christoph Hellwig]

On Mon, Apr 25, 2022 at 04:00:02PM +0100, Daniel Golle wrote:
> Enable partition parsers on plain mtdblock devices in case of
> CONFIG_FIT_PARTITION being selected.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  drivers/mtd/mtd_blkdevs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index f7317211146550..e9759c4182f8d5 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -359,7 +359,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
>  	} else {
>  		snprintf(gd->disk_name, sizeof(gd->disk_name),
>  			 "%s%d", tr->name, new->devnum);
> +#ifndef CONFIG_FIT_PARTITION
>  		gd->flags |= GENHD_FL_NO_PART;
> +#endif

This will just recreate the fixed regression, just with the extra
twist of needіng a completely unrelted config option to trigger it.

Re: [RFC PATCH 4/5] mtd_blkdevs: scan partitions on mtdblock if FIT_PARTITION is set

[Daniel Golle]

On Tue, Apr 26, 2022 at 06:52:42AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 25, 2022 at 04:00:02PM +0100, Daniel Golle wrote:
> > Enable partition parsers on plain mtdblock devices in case of
> > CONFIG_FIT_PARTITION being selected.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  drivers/mtd/mtd_blkdevs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> > index f7317211146550..e9759c4182f8d5 100644
> > --- a/drivers/mtd/mtd_blkdevs.c
> > +++ b/drivers/mtd/mtd_blkdevs.c
> > @@ -359,7 +359,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
> >  	} else {
> >  		snprintf(gd->disk_name, sizeof(gd->disk_name),
> >  			 "%s%d", tr->name, new->devnum);
> > +#ifndef CONFIG_FIT_PARTITION
> >  		gd->flags |= GENHD_FL_NO_PART;
> > +#endif
> 
> This will just recreate the fixed regression, just with the extra
> twist of needіng a completely unrelted config option to trigger it.

Do have any alternative suggestion of how partition scanning could
be (ideally selectively) enabled for mtdblock (and ubiblock) devices?
And why it should be disabled in first place?