[RFC,0/4] block: partition table OF support

Hi,
this is an initial proposal to complete support for manually defining
partition table.

Some background on this. Many OEM on embedded device (modem, router...)
are starting to migrate from NOR/NAND flash to eMMC. The reason for this
is that OEM are starting to require more and more space for the firmware
and price difference is becoming so little that using eMMC is only benefits
and no cons.

Given these reason, OEM are also using very custom way to provide a
partition table and doesn't relay on common method like writing a table
on the eMMC.

One way that is commonly used is to hardcode the partition table and
pass it to the system via various way (cmdline, special glue driver,
block2mtd...)
This way is also used on Android where the partition table
is passed from the bootloader via cmdline.

One reason to use this method is to save space on the device and to
permit more flexibility on partition handling.

What this series does is complete support for this feature.
It's possible to use the cmdline to define a partition table similar
to how it's done for MTD but this is problematic for a number of device
where tweaking the cmdline is not possible. This series adds OF support
to make it possible to define a partition table in the Device Tree.

We implement a similar schema to the MTD fixed-partition, where we define
a "label" and a "reg" with "offset" and "size".

A new block partition parser is introduced that check if the block device
have an OF node attached and check if a fixed-partition table is defined.

If a correct node is found, then partition table is filled. cmdline will
still have priority to this new parser.

Some block device also implement boot1 and boot2 additional disk. Similar
to the cmdline parser, these disk can have OF support using the
"partitions-boot0" and "partitions-boot1" additional node.

It's also completed support for declaring partition as read-only as this
feature was introduced but never finished in the cmdline parser.

Posting as RFC for any comments or additional checks on OF parser code.

I hope this solution is better accepted as downstream this is becoming
a real problem with a growing number of strange solution for the simple
task of providing a fixed partition table.

Christian Marangi (4):
  block: add support for defining read-only partitions
  docs: block: Document support for read-only partition in cmdline part
  block: add support for partition table defined in OF
  dt-bindings: mmc: Document support for partition table in mmc-card

 Documentation/block/cmdline-partition.rst     |  5 +-
 .../devicetree/bindings/mmc/mmc-card.yaml     | 75 ++++++++++++++++
 block/blk.h                                   |  1 +
 block/partitions/Kconfig                      |  8 ++
 block/partitions/Makefile                     |  1 +
 block/partitions/cmdline.c                    |  3 +
 block/partitions/core.c                       |  3 +
 block/partitions/of.c                         | 85 +++++++++++++++++++
 8 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 block/partitions/of.c

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

On Mon, Sep 23, 2024 at 11:34:53PM -0700, Christoph Hellwig wrote:
> On Mon, Sep 23, 2024 at 12:59:32PM +0200, Christian Marangi wrote:
> > +#define BOOT0_STR	"boot0"
> > +#define BOOT1_STR	"boot1"
> > +
> 
> This boot0/1 stuff looks like black magic, so it should probably be
> documented at very least.
>

It is but from what I have read in the spec for flash in general (this
is not limited to eMMC but also apply to UFS) these are hardware
partition. If the version is high enough these are always present and
have boot0 and boot1 name hardcoded by the driver.

> > +	partitions_np = get_partitions_node(disk_np,
> > +					    state->disk->disk_name);
> 
> disk->disk_name is not a stable identifier and can change from boot to
> boot due to async probing.  You'll need to check a uuid or label instead.

This is really for the 2 special partition up to check the suffix, we
don't really care about the name. I guess it's acceptable to use
unstable identifier?

Thanks a lot for the review!

On Tue, Sep 24, 2024 at 12:17:36PM +0200, Christian Marangi wrote:
> On Mon, Sep 23, 2024 at 11:34:53PM -0700, Christoph Hellwig wrote:
> > On Mon, Sep 23, 2024 at 12:59:32PM +0200, Christian Marangi wrote:
> > > +#define BOOT0_STR	"boot0"
> > > +#define BOOT1_STR	"boot1"
> > > +
> > 
> > This boot0/1 stuff looks like black magic, so it should probably be
> > documented at very least.
> >
> 
> It is but from what I have read in the spec for flash in general (this
> is not limited to eMMC but also apply to UFS) these are hardware
> partition. If the version is high enough these are always present and
> have boot0 and boot1 name hardcoded by the driver.

How does this belong into generic block layer code?

> > > +	partitions_np = get_partitions_node(disk_np,
> > > +					    state->disk->disk_name);
> > 
> > disk->disk_name is not a stable identifier and can change from boot to
> > boot due to async probing.  You'll need to check a uuid or label instead.
> 
> This is really for the 2 special partition up to check the suffix, we
> don't really care about the name. I guess it's acceptable to use
> unstable identifier?

No.  ->disk_name is in no way reliable, we can't hardcode that into
a partition parser.

On Tue, Oct 01, 2024 at 01:37:05AM -0700, Christoph Hellwig wrote:
> On Tue, Sep 24, 2024 at 12:17:36PM +0200, Christian Marangi wrote:
> > On Mon, Sep 23, 2024 at 11:34:53PM -0700, Christoph Hellwig wrote:
> > > On Mon, Sep 23, 2024 at 12:59:32PM +0200, Christian Marangi wrote:
> > > > +#define BOOT0_STR	"boot0"
> > > > +#define BOOT1_STR	"boot1"
> > > > +
> > > 
> > > This boot0/1 stuff looks like black magic, so it should probably be
> > > documented at very least.
> > >
> > 
> > It is but from what I have read in the spec for flash in general (this
> > is not limited to eMMC but also apply to UFS) these are hardware
> > partition. If the version is high enough these are always present and
> > have boot0 and boot1 name hardcoded by the driver.
> 
> How does this belong into generic block layer code?
>

(just as an info, we are at v4 where I added more info about this)

The cmdline partition parser supports this already, just not clearly
stated in the code but described in the Documentation example and info.

> > > > +	partitions_np = get_partitions_node(disk_np,
> > > > +					    state->disk->disk_name);
> > > 
> > > disk->disk_name is not a stable identifier and can change from boot to
> > > boot due to async probing.  You'll need to check a uuid or label instead.
> > 
> > This is really for the 2 special partition up to check the suffix, we
> > don't really care about the name. I guess it's acceptable to use
> > unstable identifier?
> 
> No.  ->disk_name is in no way reliable, we can't hardcode that into
> a partition parser.
> 

Then any hint on this or alternative way?
Again this is how it's done with cmdline partition so I'm just following
how it's already done.

Also I feel it's not clear enough that we really don't care about the
identifier, eMMC driver hardcode and always append to disk_name boot0, boot1,
the fact that one disk or another might have a different identifier and
they change on different boot is not important for the task needed here.

I can drop this thing entirely and make the implementation very simple
but there are already request and happy dev that would benefits for the
additional hardware partition supported by this.

On Tue, Oct 01, 2024 at 11:26:22AM +0200, Christian Marangi wrote:
> > No.  ->disk_name is in no way reliable, we can't hardcode that into
> > a partition parser.
> > 
> 
> Then any hint on this or alternative way?

The normal way would be to use eui/ngui/uuid provided by the storage
device.  We have a interface for that in the block layer support by
scsi and nvme, but I don't know how to wire that up for eMMC which
I suspect is what you care about.

On Wed, Oct 02, 2024 at 12:45:37AM -0700, Christoph Hellwig wrote:
> On Tue, Oct 01, 2024 at 11:26:22AM +0200, Christian Marangi wrote:
> > > No.  ->disk_name is in no way reliable, we can't hardcode that into
> > > a partition parser.
> > > 
> > 
> > Then any hint on this or alternative way?
> 
> The normal way would be to use eui/ngui/uuid provided by the storage
> device.  We have a interface for that in the block layer support by
> scsi and nvme, but I don't know how to wire that up for eMMC which
> I suspect is what you care about.
> 

I think I have found a better way to handle it, please check v5, did you
receive the series?

The new series just make the driver pass the partition node and the OF
code just take it and use it.

This way we don't have to parse the disk name at all and it's driver
specific work to select the "partitions" node if multiple are present.

I feel your hint produced a much better implementation without having to
pollute the block code with specific case.