| Message ID | c26dfdf9ce56e92d23530a09db386b283e62845d.1638289204.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCH/RFC,-nxt] mtd_blkdevs: Set GENHD_FL_NO_PART | expand |
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?
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
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)
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
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)
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(+)