* [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART
@ 2021-11-30 16:23 Geert Uytterhoeven
2021-12-01 7:23 ` Christoph Hellwig
2021-12-03 6:57 ` Christoph Hellwig
0 siblings, 2 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2021-11-30 16:23 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Tudor Ambarus
Cc: Michael Walle, Pratyush Yadav, linux-mtd, linux-block,
linux-renesas-soc, linux-kernel, Geert Uytterhoeven
When DT declares the partitions of an spi-nor device using
"fixed-partitions", the individual mtdblockN partitions are now scanned
for partitition tables, which should not happen.
Fix this by setting the GENHD_FL_NO_PART flag in the MTD block layer
interface.
Fixes: 1ebe2e5f9d68e94c ("block: remove GENHD_FL_EXT_DEVT")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Seen with e.g. arch/arm/boot/dts/r8a7791-koelsch.dts.
I only noticed because I have debug code to measure QSPI performance,
which informed me about 8 x 512 bytes being read from each partition
detected.
RFC as I'm not sure this is correct in all cases.
I did verify that in the absence of "fixed-partitions", the spi-nor
device is not scanned for partitions before and after commit
1ebe2e5f9d68e94c.
---
drivers/mtd/mtd_blkdevs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 113f86df76038575..95c84faa794d22c6 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -345,6 +345,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
gd->first_minor = (new->devnum) << tr->part_bits;
gd->minors = 1 << tr->part_bits;
gd->fops = &mtd_block_ops;
+ gd->flags |= GENHD_FL_NO_PART;
if (tr->part_bits)
if (new->devnum < 26)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART
2021-11-30 16:23 [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART Geert Uytterhoeven
@ 2021-12-01 7:23 ` Christoph Hellwig
2021-12-01 10:08 ` Geert Uytterhoeven
2021-12-03 6:57 ` Christoph Hellwig
1 sibling, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2021-12-01 7:23 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Christoph Hellwig, Jens Axboe, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Tudor Ambarus, Michael Walle,
Pratyush Yadav, linux-mtd, linux-block, linux-renesas-soc,
linux-kernel
On Tue, Nov 30, 2021 at 05:23:46PM +0100, Geert Uytterhoeven wrote:
> When DT declares the partitions of an spi-nor device using
> "fixed-partitions", the individual mtdblockN partitions are now scanned
> for partitition tables, which should not happen.
>
> Fix this by setting the GENHD_FL_NO_PART flag in the MTD block layer
> interface.
>
> Fixes: 1ebe2e5f9d68e94c ("block: remove GENHD_FL_EXT_DEVT")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Seen with e.g. arch/arm/boot/dts/r8a7791-koelsch.dts.
> I only noticed because I have debug code to measure QSPI performance,
> which informed me about 8 x 512 bytes being read from each partition
> detected.
>
> RFC as I'm not sure this is correct in all cases.
> I did verify that in the absence of "fixed-partitions", the spi-nor
> device is not scanned for partitions before and after commit
> 1ebe2e5f9d68e94c.
As far as I can tell mtd fixed partitions have nothing to do with
the block layer concept of partitions. What kind of behavior change
did you see?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART
2021-12-01 7:23 ` Christoph Hellwig
@ 2021-12-01 10:08 ` Geert Uytterhoeven
0 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2021-12-01 10:08 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Tudor Ambarus, Michael Walle,
Pratyush Yadav, MTD Maling List, linux-block, Linux-Renesas,
Linux Kernel Mailing List
Hi Christoph,
On Wed, Dec 1, 2021 at 8:23 AM Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Nov 30, 2021 at 05:23:46PM +0100, Geert Uytterhoeven wrote:
> > When DT declares the partitions of an spi-nor device using
> > "fixed-partitions", the individual mtdblockN partitions are now scanned
> > for partitition tables, which should not happen.
> >
> > Fix this by setting the GENHD_FL_NO_PART flag in the MTD block layer
> > interface.
> >
> > Fixes: 1ebe2e5f9d68e94c ("block: remove GENHD_FL_EXT_DEVT")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Seen with e.g. arch/arm/boot/dts/r8a7791-koelsch.dts.
> > I only noticed because I have debug code to measure QSPI performance,
> > which informed me about 8 x 512 bytes being read from each partition
> > detected.
> >
> > RFC as I'm not sure this is correct in all cases.
> > I did verify that in the absence of "fixed-partitions", the spi-nor
> > device is not scanned for partitions before and after commit
> > 1ebe2e5f9d68e94c.
>
> As far as I can tell mtd fixed partitions have nothing to do with
> the block layer concept of partitions. What kind of behavior change
> did you see?
After the aforementioned commit, 8 x 512 bytes are being read from
the start of each partition described by "fixed-partitions".
Dmesg difference with debug print added to spi_nor_spimem_read_data():
renesas_spi e6b10000.spi: registered master spi0
spi spi0.0: setup mode 3, 8 bits/w, 30000000 Hz max --> 0
spi-nor spi0.0: s25fl512s (65536 Kbytes)
3 fixed-partitions partitions found on MTD device spi0.0
Creating 3 MTD partitions on "spi0.0":
0x000000000000-0x000000080000 : "loader"
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 0 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 512 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 1024 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 1536 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 2048 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 2560 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 3072 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 3584 len 512
0x000000080000-0x000000600000 : "user"
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 524288 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 524800 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 525312 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 525824 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 526336 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 526848 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 527360 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 527872 len 512
0x000000600000-0x000004000000 : "flash"
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6291456 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6291968 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6292480 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6292992 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6293504 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6294016 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6294528 len 512
+ spi-nor spi0.0: spi_nor_spimem_read_data: from 6295040 len 512
renesas_spi e6b10000.spi: registered child spi0.0
renesas_spi e6b10000.spi: probed
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART
2021-11-30 16:23 [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART Geert Uytterhoeven
2021-12-01 7:23 ` Christoph Hellwig
@ 2021-12-03 6:57 ` Christoph Hellwig
2021-12-03 8:25 ` Geert Uytterhoeven
1 sibling, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2021-12-03 6:57 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Christoph Hellwig, Jens Axboe, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Tudor Ambarus, Michael Walle,
Pratyush Yadav, linux-mtd, linux-block, linux-renesas-soc,
linux-kernel
I think we need the patch below to restore the old behavior where a
partitions scan happens only for those sub-drivers that do report a
partition shift.
MTD maintainers: is this intentional that raw mtdblock does not support
partitions, but the various "FTL" modules do?
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 113f86df76038..57a22d2ebaeca 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -345,6 +345,8 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
gd->first_minor = (new->devnum) << tr->part_bits;
gd->minors = 1 << tr->part_bits;
gd->fops = &mtd_block_ops;
+ if (!tr->part_bits)
+ gd->flags |= GENHD_FL_NO_PART;
if (tr->part_bits)
if (new->devnum < 26)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART
2021-12-03 6:57 ` Christoph Hellwig
@ 2021-12-03 8:25 ` Geert Uytterhoeven
0 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2021-12-03 8:25 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Tudor Ambarus, Michael Walle,
Pratyush Yadav, MTD Maling List, linux-block, Linux-Renesas,
Linux Kernel Mailing List
Hi Christoph,
On Fri, Dec 3, 2021 at 7:57 AM Christoph Hellwig <hch@lst.de> wrote:
> I think we need the patch below to restore the old behavior where a
> partitions scan happens only for those sub-drivers that do report a
> partition shift.
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> MTD maintainers: is this intentional that raw mtdblock does not support
> partitions, but the various "FTL" modules do?
>
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index 113f86df76038..57a22d2ebaeca 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -345,6 +345,8 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
> gd->first_minor = (new->devnum) << tr->part_bits;
> gd->minors = 1 << tr->part_bits;
> gd->fops = &mtd_block_ops;
> + if (!tr->part_bits)
> + gd->flags |= GENHD_FL_NO_PART;
Move this into the "else" branch of the test below?
> if (tr->part_bits)
> if (new->devnum < 26)
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-03 8:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 16:23 [PATCH/RFC -nxt] mtd_blkdevs: Set GENHD_FL_NO_PART Geert Uytterhoeven
2021-12-01 7:23 ` Christoph Hellwig
2021-12-01 10:08 ` Geert Uytterhoeven
2021-12-03 6:57 ` Christoph Hellwig
2021-12-03 8:25 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).