| Message ID | 89abd9ab93783da0e8934ebc03d66559f78f6060.1711048433.git.daniel@makrotopia.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: implement NVMEM provider | expand |
On 3/21/24 12:33, Daniel Golle wrote: > Add new flag to destinguish block devices which may act as an NVMEM > provider. > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > --- > include/linux/blkdev.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index c3e8f7cf96be9..f2c4f280d7619 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -81,11 +81,13 @@ struct partition_meta_info { > * ``GENHD_FL_NO_PART``: partition support is disabled. The kernel will not > * scan for partitions from add_disk, and users can't add partitions manually. > * > + * ``GENHD_FL_NVMEM``: the block device should be considered as NVMEM provider. > */ > enum { > GENHD_FL_REMOVABLE = 1 << 0, > GENHD_FL_HIDDEN = 1 << 1, > GENHD_FL_NO_PART = 1 << 2, > + GENHD_FL_NVMEM = 1 << 3, > }; What would break if this flag wouldn't exist? Thanks, Bart.
On Fri, Mar 22, 2024 at 10:49:48AM -0700, Bart Van Assche wrote: > On 3/21/24 12:33, Daniel Golle wrote: > > Add new flag to destinguish block devices which may act as an NVMEM > > provider. > > > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > --- > > include/linux/blkdev.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > > index c3e8f7cf96be9..f2c4f280d7619 100644 > > --- a/include/linux/blkdev.h > > +++ b/include/linux/blkdev.h > > @@ -81,11 +81,13 @@ struct partition_meta_info { > > * ``GENHD_FL_NO_PART``: partition support is disabled. The kernel will not > > * scan for partitions from add_disk, and users can't add partitions manually. > > * > > + * ``GENHD_FL_NVMEM``: the block device should be considered as NVMEM provider. > > */ > > enum { > > GENHD_FL_REMOVABLE = 1 << 0, > > GENHD_FL_HIDDEN = 1 << 1, > > GENHD_FL_NO_PART = 1 << 2, > > + GENHD_FL_NVMEM = 1 << 3, > > }; > > What would break if this flag wouldn't exist? As both, MTD and UBI already act as NVMEM providers themselves, once the user creates a ubiblock device or got CONFIG_MTD_BLOCK=y set in their kernel configuration, we would run into problems because both, the block layer as well as MTD or UBI would try to be an NVMEM provider for the same device tree node. I intially suggested the invert of this flag, GENHD_FL_NO_NVMEM which would be set only for mtdblock and ubiblock devices to opt-out of acting as NVMEM proviers. However, in a previous comment [1] on the RFC it was requested to make this opt-in instead. [1]: https://patchwork.kernel.org/comment/25432948/
On 3/22/24 11:07, Daniel Golle wrote: > On Fri, Mar 22, 2024 at 10:49:48AM -0700, Bart Van Assche wrote: >> On 3/21/24 12:33, Daniel Golle wrote: >>> enum { >>> GENHD_FL_REMOVABLE = 1 << 0, >>> GENHD_FL_HIDDEN = 1 << 1, >>> GENHD_FL_NO_PART = 1 << 2, >>> + GENHD_FL_NVMEM = 1 << 3, >>> }; >> >> What would break if this flag wouldn't exist? > > As both, MTD and UBI already act as NVMEM providers themselves, once > the user creates a ubiblock device or got CONFIG_MTD_BLOCK=y set in their > kernel configuration, we would run into problems because both, the block > layer as well as MTD or UBI would try to be an NVMEM provider for the same > device tree node. Why would both MTD and UBI try to be an NVMEM provider for the same device tree node? Why can't this patch series be implemented such that a partition UUID occurs in the device tree and such that other code scans for that partition UUID? Thanks, Bart.
On Fri, Mar 22, 2024 at 12:22:32PM -0700, Bart Van Assche wrote: > On 3/22/24 11:07, Daniel Golle wrote: > > On Fri, Mar 22, 2024 at 10:49:48AM -0700, Bart Van Assche wrote: > > > On 3/21/24 12:33, Daniel Golle wrote: > > > > enum { > > > > GENHD_FL_REMOVABLE = 1 << 0, > > > > GENHD_FL_HIDDEN = 1 << 1, > > > > GENHD_FL_NO_PART = 1 << 2, > > > > + GENHD_FL_NVMEM = 1 << 3, > > > > }; > > > > > > What would break if this flag wouldn't exist? > > > > As both, MTD and UBI already act as NVMEM providers themselves, once > > the user creates a ubiblock device or got CONFIG_MTD_BLOCK=y set in their > > kernel configuration, we would run into problems because both, the block > > layer as well as MTD or UBI would try to be an NVMEM provider for the same > > device tree node. > > Why would both MTD and UBI try to be an NVMEM provider for the same > device tree node? I didn't mean that both MTD and UBI would **simultanously** try to act as NVMEM providers for the same device tree node. What I meant was that either of them can act as an NVMEM provider while at the same time also providing an emulated block device (mtdblock xor ubiblock). Hence those emulated block devices will have to be excluded from acting as NVMEM providers. In this patch I suggest to do this by opt-in of block drivers which should potentially provide NVMEM (typically mmcblk). I apologize for the confusion and assume that wasn't clear from the wording I've used. I hope it's more clear now. Alternatively it could also be solved via opt-out of ubiblock and mtdblock devices using the inverted flag (GENHD_FL_NO_NVMEM) -- however, this has previously been criticized and I was asked to rather make it opt-in.[1] > Why can't this patch series be implemented such that > a partition UUID occurs in the device tree and such that other code > scans for that partition UUID? This is actually one way this very series allows one to handle this: by identifying a partition using its partuuid. However, it's also quite common that the MMC boot **hardware** partitions are used to store MAC addresses and/or Wi-Fi calibration data. In this case there is no partition table and the NVMEM provider has to act directly on the whole disk device (which is only a few megabytes in size in case of those mmcblkXbootY devices and never has a partition table). [1]: https://patchwork.kernel.org/comment/25432948/
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index c3e8f7cf96be9..f2c4f280d7619 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -81,11 +81,13 @@ struct partition_meta_info { * ``GENHD_FL_NO_PART``: partition support is disabled. The kernel will not * scan for partitions from add_disk, and users can't add partitions manually. * + * ``GENHD_FL_NVMEM``: the block device should be considered as NVMEM provider. */ enum { GENHD_FL_REMOVABLE = 1 << 0, GENHD_FL_HIDDEN = 1 << 1, GENHD_FL_NO_PART = 1 << 2, + GENHD_FL_NVMEM = 1 << 3, }; enum {
Add new flag to destinguish block devices which may act as an NVMEM provider. Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- include/linux/blkdev.h | 2 ++ 1 file changed, 2 insertions(+)