nvmem: add block device NVMEM provider
Subsystems (4)
cover nvmem: add block device NVMEM provider
On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.
Implement an NVMEM provider backed by block devices as typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...
In order to be able to reference hardware partitions on an eMMC, add code to bind each hardware partition to a specific firmware subnode.
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
Daniel Golle (6):
mmc: core: set card fwnode_handle
mmc: block: set fwnode of disk devices
block: add new genhd flag GENHD_FL_NO_NVMEM
mtd: blkdevs: set GENHD_FL_NO_NVMEM
mtd: ubi: block: set GENHD_FL_NO_NVMEM
block: implement NVMEM provider
block/Kconfig | 8 ++ block/Makefile | 1 + block/blk-nvmem.c | 187 ++++++++++++++++++++++++++++++++++++++ block/blk.h | 13 +++ block/genhd.c | 2 + block/partitions/core.c | 2 + drivers/mmc/core/block.c | 8 ++ drivers/mmc/core/bus.c | 2 + drivers/mtd/mtd_blkdevs.c | 1 + drivers/mtd/ubi/block.c | 2 +- include/linux/blkdev.h | 3 + 11 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 block/blk-nvmem.c
Signature · 1 line
2.41.0
RFC PATCH 1/6 mmc: core: set card fwnode_handle
Instead of just populating dev.of_node, also set fwnode in case it isn't set yet and of_node is present.
--- drivers/mmc/core/bus.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/mmc/core/bus.c +2 -0
@@ -361,6 +361,8 @@ int mmc_add_card(struct mmc_card *card)mmc_add_card_debugfs (card); card->dev.of_node =mmc_of_find_child_device (card->host,0 ); +if (card->dev.of_node && !card->dev.fwnode) + card->dev.fwnode = &card->dev.of_node->fwnode;device_enable_async_suspend (&card->dev);
Signature · 1 line
2.41.0
RFC PATCH 2/6 mmc: block: set fwnode of disk devices
In reply to Daniel GolleSet fwnode of disk devices to 'block', 'boot0' and 'boot1' subnodes of the mmc-card. This is done in preparation for having the eMMC act as NVMEM provider.
--- drivers/mmc/core/block.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/mmc/core/block.c +8 -0
@@ -2413,6 +2413,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,int area_type, unsigned int part_type) { + struct fwnode_handle *fwnode; +struct device *ddev;struct mmc_blk_data *md;int devidx, ret;char cap_str[10 ];
@@ -2509,6 +2511,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,blk_queue_write_cache (md->queue.queue, cache_enabled, fua_enabled); + ddev =disk_to_dev (md->disk); + fwnode =device_get_named_child_node (subname ? md->parent->parent : + md->parent, + subname ? subname :"block" ); + ddev->fwnode = fwnode; +string_get_size ((u64 )size,512 ,STRING_UNITS_2 , cap_str,sizeof (cap_str)); pr_info("%s: %s %s %s%s\n",
Signature · 5 lines
2.41.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 2/6] mmc: block: set fwnode of disk devices
Quoted 4 lines · Daniel Golle
Set fwnode of disk devices to 'block', 'boot0' and 'boot1' subnodes of the mmc-card. This is done in preparation for having the eMMC act as NVMEM provider.
Sorry, but I don't quite understand what you are trying to do here. Maybe you should re-order the patches in the series so it becomes clear why this is needed?
Moreover, I don't see any DT docs being updated as a part of the series, which looks like it is needed too. That would also help to understand what you are proposing, I think.
Quoted 22 lines
Signed-off-by Daniel Golle <[email protected]>--- drivers/mmc/core/block.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/mmc/core/block.c +2
-0@@ -2413,6 +2413,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,int area_type, unsigned int part_type) { + struct fwnode_handle *fwnode; +struct device *ddev;struct mmc_blk_data *md;int devidx, ret;char cap_str[10 ];@@ -2509,6 +2511,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,blk_queue_write_cache (md->queue.queue, cache_enabled, fua_enabled);
drivers/mmc/core/block.c static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, · line 2522
Quoted 10 lines
drivers/mmc/core/block.c +6
-0+ ddev =disk_to_dev (md->disk); + fwnode =device_get_named_child_node (subname ? md->parent->parent : + md->parent, + subname ? subname :"block" ); + ddev->fwnode = fwnode; +string_get_size ((u64 )size,512 ,STRING_UNITS_2 , cap_str,sizeof (cap_str)); pr_info("%s: %s %s %s%s\n",
Kind regards
Uffe
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 2/6] mmc: block: set fwnode of disk devices
Hi Ulf,
thank you for reviewing and suggesting ways to improve this series!
Quoted 6 lines · Ulf Hansson
Quoted 4 lines · Daniel Golle
Set fwnode of disk devices to 'block', 'boot0' and 'boot1' subnodes of the mmc-card. This is done in preparation for having the eMMC act as NVMEM provider.
Quoted 7 lines
Sorry, but I don't quite understand what you are trying to do here. Maybe you should re-order the patches in the series so it becomes clear why this is needed?
Moreover, I don't see any DT docs being updated as a part of the series, which looks like it is needed too. That would also help to understand what you are proposing, I think.
I've prepared a tree on Github which now also includes commits adding dt-bindings for block devices and partitions, so they can be referenced as nvmem-cells provider.
The dt-schema addition supposedly explaining this specific patch:
https://github.com/dangowrt/linux/commit/b399a758f0e1c444ae9443dc80902a30de54af09
The whole tree:
https://github.com/dangowrt/linux/commits/for-nvmem-next
Most comments have been addressed, however, I still depend on using either a class_interface *or* adding calls to add/remove the NVMEM representation of a block device to block/genhd.c as well as block/partitions/core.c, simply because afaik there isn't any better way for in-kernel users of block devices to be notified about the creation or removal of a block device.
Cheers
Daniel
Quoted 36 lines
Quoted 23 lines
Signed-off-by Daniel Golle <[email protected]>--- drivers/mmc/core/block.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/mmc/core/block.c +2
-0@@ -2413,6 +2413,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,int area_type, unsigned int part_type) { + struct fwnode_handle *fwnode; +struct device *ddev;struct mmc_blk_data *md;int devidx, ret;char cap_str[10 ];@@ -2509,6 +2511,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,blk_queue_write_cache (md->queue.queue, cache_enabled, fua_enabled);drivers/mmc/core/block.c static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card, · line 2522
Quoted 10 lines
drivers/mmc/core/block.c +6
-0+ ddev =disk_to_dev (md->disk); + fwnode =device_get_named_child_node (subname ? md->parent->parent : + md->parent, + subname ? subname :"block" ); + ddev->fwnode = fwnode; +string_get_size ((u64 )size,512 ,STRING_UNITS_2 , cap_str,sizeof (cap_str)); pr_info("%s: %s %s %s%s\n",Kind regards
Uffe
Re: [RFC PATCH 2/6] mmc: block: set fwnode of disk devices
Quoted 24 lines · Daniel Golle
Hi Ulf,
thank you for reviewing and suggesting ways to improve this series!
Quoted 6 lines · Ulf Hansson
Quoted 4 lines · Daniel Golle
Set fwnode of disk devices to 'block', 'boot0' and 'boot1' subnodes of the mmc-card. This is done in preparation for having the eMMC act as NVMEM provider.
Quoted 7 lines
Sorry, but I don't quite understand what you are trying to do here. Maybe you should re-order the patches in the series so it becomes clear why this is needed?
Moreover, I don't see any DT docs being updated as a part of the series, which looks like it is needed too. That would also help to understand what you are proposing, I think.
I've prepared a tree on Github which now also includes commits adding dt-bindings for block devices and partitions, so they can be referenced as nvmem-cells provider.
Quoted 7 lines
The dt-schema addition supposedly explaining this specific patch:
https://github.com/dangowrt/linux/commit/b399a758f0e1c444ae9443dc80902a30de54af09
The whole tree:
Thanks for sharing. However, allow people to review, I suggest you post a new version with the updated DT bindings included.
The point is, we really need confirmation from the DT maintainers - otherwise this is simply a no go.
Quoted 7 lines
Most comments have been addressed, however, I still depend on using either a
class_interface*or* adding calls to add/remove the NVMEM representation of a block device toblock/genhd.cas well asblock/partitions/core.c, simply because afaik there isn't any better way for in-kernel users of block devices to be notified about the creation or removal of a block device.
Okay, so that needs further discussions then. I will try to chim in.
[...]
Kind regards
Uffe
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
RFC PATCH 3/6 block: add new genhd flag GENHD_FL_NO_NVMEM
In reply to Daniel GolleAdd new flag to destinguish block devices which should not act as an NVMEM provider, such as for example an emulated block device on top of an MTD partition which already acts as an NVMEM provider itself.
--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)
include/linux/blkdev.h +3 -0
@@ -80,11 +80,14 @@ 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_NO_NVMEM``: NVMEM emulation is disabled. The kernel will not +* emulate an NVMEM device on top of this disk. */ enum {GENHD_FL_REMOVABLE =1 <<0 ,GENHD_FL_HIDDEN =1 <<1 ,GENHD_FL_NO_PART =1 <<2 , +GENHD_FL_NO_NVMEM =1 <<3 , };enum {
Signature · 5 lines
2.41.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 3/6] block: add new genhd flag GENHD_FL_NO_NVMEM
Quoted 18 lines · Daniel Golle
Add new flag to destinguish block devices which should not act as an NVMEM provider, such as for example an emulated block device on top of an MTD partition which already acts as an NVMEM provider itself.
Signed-off-by Daniel Golle <[email protected]>--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)include/linux/blkdev.h +1
-0@@ -80,11 +80,14 @@ 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_NO_NVMEM``: NVMEM emulation is disabled. The kernel will not
include/linux/blkdev.h struct partition_meta_info { · line 93
Quoted 10 lines
include/linux/blkdev.h +2
-0+* emulate an NVMEM device on top of this disk. */ enum {GENHD_FL_REMOVABLE =1 <<0 ,GENHD_FL_HIDDEN =1 <<1 ,GENHD_FL_NO_PART =1 <<2 , +GENHD_FL_NO_NVMEM =1 <<3 , };enum {
Please reverse this flag. Most of the devices will not have an NVMEM partition, and we shouldn't require each and every driver to tag their devices. So please use GENHD_FL_NVMEM and only set this flag on devices which really have an NVMEM partition.
Cheers,
Hannes
Re: [RFC PATCH 3/6] block: add new genhd flag GENHD_FL_NO_NVMEM
Quoted 25 lines · Hannes Reinecke
Quoted 24 lines · Daniel Golle
Add new flag to destinguish block devices which should not act as an NVMEM provider, such as for example an emulated block device on top of an MTD partition which already acts as an NVMEM provider itself.
Signed-off-by Daniel Golle <[email protected]>--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)include/linux/blkdev.h +2
-0@@ -80,11 +80,14 @@ 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_NO_NVMEM``: NVMEM emulation is disabled. The kernel will not +* emulate an NVMEM device on top of this disk. */ enum {GENHD_FL_REMOVABLE =1 <<0 ,GENHD_FL_HIDDEN =1 <<1 ,GENHD_FL_NO_PART =1 <<2 ,
Quoted 8 lines
include/linux/blkdev.h struct partition_meta_info { · line 93
Quoted 3 lines
include/linux/blkdev.h +1
-0+ GENHD_FL_NO_NVMEM = 1 << 3, };enum {Please reverse this flag. Most of the devices will not have an NVMEM partition, and we shouldn't require each and every driver to tag their devices.
So please useGENHD_FL_NVMEMand only set this flag on devices which really have an NVMEM partition.
The idea here was to exclude all those devices which already implement an NVMEM provider on a lower layer themselves, such as MTD. In this cases it would be ambigous if the OF node represents the NVMEM device registered by the MTD framework or if blk-nvmem should be used.
In all other cases device tree can unambigously indicate whether a block device should serve as NVMEM provider (and right, most of them never will).
However, reversing the logic seems fine just as well.
Re: [RFC PATCH 3/6] block: add new genhd flag GENHD_FL_NO_NVMEM
Quoted 35 lines · Daniel Golle
Quoted 25 lines · Hannes Reinecke
Quoted 24 lines · Daniel Golle
Add new flag to destinguish block devices which should not act as an NVMEM provider, such as for example an emulated block device on top of an MTD partition which already acts as an NVMEM provider itself.
Signed-off-by Daniel Golle <[email protected]>--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)include/linux/blkdev.h +2
-0@@ -80,11 +80,14 @@ 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_NO_NVMEM``: NVMEM emulation is disabled. The kernel will not +* emulate an NVMEM device on top of this disk. */ enum {GENHD_FL_REMOVABLE =1 <<0 ,GENHD_FL_HIDDEN =1 <<1 ,GENHD_FL_NO_PART =1 <<2 ,Quoted 8 lines
include/linux/blkdev.h struct partition_meta_info { · line 93
Quoted 3 lines
include/linux/blkdev.h +1
-0+ GENHD_FL_NO_NVMEM = 1 << 3, };enum {Please reverse this flag. Most of the devices will not have an NVMEM partition, and we shouldn't require each and every driver to tag their devices.
So please useGENHD_FL_NVMEMand only set this flag on devices which really have an NVMEM partition.
Quoted 6 lines
The idea here was to exclude all those devices which already implement an NVMEM provider on a lower layer themselves, such as MTD. In this cases it would be ambigous if the OF node represents the NVMEM device registered by the MTD framework or if blk-nvmem should be used.
Hmm; not sure if I follow.
In the end, it doesn't really matter whether you check for GENHD_FL_NO_NVMEM or !GENHD_FL_NVMEM.
With the difference being that in the former case you have to tag 99% of all existing block devices, and in the latter you have to tag 1%.
Quoted 5 lines
In all other cases device tree can unambigously indicate whether a block device should serve as NVMEM provider (and right, most of them never will).
However, reversing the logic seems fine just as well.
Thanks. Please do.
Cheers,
Hannes
Re: [RFC PATCH 3/6] block: add new genhd flag GENHD_FL_NO_NVMEM
Quoted 42 lines · Hannes Reinecke
Quoted 35 lines · Daniel Golle
Quoted 25 lines · Hannes Reinecke
Quoted 24 lines · Daniel Golle
Add new flag to destinguish block devices which should not act as an NVMEM provider, such as for example an emulated block device on top of an MTD partition which already acts as an NVMEM provider itself.
Signed-off-by Daniel Golle <[email protected]>--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)include/linux/blkdev.h +2
-0@@ -80,11 +80,14 @@ 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_NO_NVMEM``: NVMEM emulation is disabled. The kernel will not +* emulate an NVMEM device on top of this disk. */ enum {GENHD_FL_REMOVABLE =1 <<0 ,GENHD_FL_HIDDEN =1 <<1 ,GENHD_FL_NO_PART =1 <<2 ,Quoted 8 lines
include/linux/blkdev.h struct partition_meta_info { · line 93
Quoted 3 lines
include/linux/blkdev.h +1
-0+ GENHD_FL_NO_NVMEM = 1 << 3, };enum {Please reverse this flag. Most of the devices will not have an NVMEM partition, and we shouldn't require each and every driver to tag their devices.
So please useGENHD_FL_NVMEMand only set this flag on devices which really have an NVMEM partition.Quoted 6 lines
The idea here was to exclude all those devices which already implement an NVMEM provider on a lower layer themselves, such as MTD. In this cases it would be ambigous if the OF node represents the NVMEM device registered by the MTD framework or if blk-nvmem should be used.
Quoted 6 lines
Hmm; not sure if I follow.
In the end, it doesn't really matter whether you check forGENHD_FL_NO_NVMEMor !GENHD_FL_NVMEM.
With the difference being that in the former case you have to tag 99% of all existing block devices, and in the latter you have to tag 1%.
That's not exactly true. In the current case I only have to flag MTD (and UBI in future, I'm working on a UBI NVMEM provider as well) with GENHD_FL_NO_NVMEM, so a 'compatible = "nvmem-cells"' in the corresponding device tree node should not result in blk-nvmem creating an NVMEM device based on the (mtd/ubi)block device, simply because the MTD framework (and UBI in future) will already have created their own NVMEM device attached to the very same device tree node.
In all other cases of block devices, the compatible string can be used to unambigously decide whether an NVMEM device should be created or not. blk-nvmem is opt-in, so unless the device is flagged by 'compatible = "nvmem-cells"' it will not do anything.
For all devices which anyway do not have any device tree representation it won't do anything (think: loop, nbd, ...), we would not need to opt them out using GENHD_FL_NO_NVMEM. Also all other drivers which do not already bring their own NVMEM implementation won't need GENHD_FL_NO_NVMEM, the absence of 'compatible = "nvmem-cells"' is enough to indicate that they should not be considered as NVMEM providers.
The way you are suggesting will require that, in addition to selecting the targetted block device in device tree, the block driver will also have to set GENHD_FL_NVMEM. Hence we will need changes in MMC, NVMe and potentially also SATA disk drivers setting GENHD_FL_NVMEM when registering the disk.
Quoted 8 lines
Quoted 6 lines
In all other cases device tree can unambigously indicate whether a block device should serve as NVMEM provider (and right, most of them never will).
However, reversing the logic seems fine just as well.
Thanks. Please do.
Either way is fine with me, just 99% vs. 1% is not the right argument in this case.
Re: [RFC PATCH 3/6] block: add new genhd flag GENHD_FL_NO_NVMEM
Quoted 70 lines · Daniel Golle
Quoted 42 lines · Hannes Reinecke
Quoted 35 lines · Daniel Golle
Quoted 25 lines · Hannes Reinecke
Quoted 24 lines · Daniel Golle
Add new flag to destinguish block devices which should not act as an NVMEM provider, such as for example an emulated block device on top of an MTD partition which already acts as an NVMEM provider itself.
Signed-off-by Daniel Golle <[email protected]>--- include/linux/blkdev.h | 3 +++ 1 file changed, 3 insertions(+)include/linux/blkdev.h +2
-0@@ -80,11 +80,14 @@ 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_NO_NVMEM``: NVMEM emulation is disabled. The kernel will not +* emulate an NVMEM device on top of this disk. */ enum {GENHD_FL_REMOVABLE =1 <<0 ,GENHD_FL_HIDDEN =1 <<1 ,GENHD_FL_NO_PART =1 <<2 ,Quoted 8 lines
include/linux/blkdev.h struct partition_meta_info { · line 93
Quoted 3 lines
include/linux/blkdev.h +1
-0+ GENHD_FL_NO_NVMEM = 1 << 3, };enum {Please reverse this flag. Most of the devices will not have an NVMEM partition, and we shouldn't require each and every driver to tag their devices.
So please useGENHD_FL_NVMEMand only set this flag on devices which really have an NVMEM partition.Quoted 6 lines
The idea here was to exclude all those devices which already implement an NVMEM provider on a lower layer themselves, such as MTD. In this cases it would be ambigous if the OF node represents the NVMEM device registered by the MTD framework or if blk-nvmem should be used.
Quoted 6 lines
Hmm; not sure if I follow.
In the end, it doesn't really matter whether you check forGENHD_FL_NO_NVMEMor !GENHD_FL_NVMEM.
With the difference being that in the former case you have to tag 99% of all existing block devices, and in the latter you have to tag 1%.That's not exactly true. In the current case I only have to flag MTD (and UBI in future, I'm working on a UBI NVMEM provider as well) with
GENHD_FL_NO_NVMEM, so a 'compatible = "nvmem-cells"' in the corresponding device tree node should not result in blk-nvmem creating an NVMEM device based on the (mtd/ubi)block device, simply because the MTD framework (and UBI in future) will already have created their own NVMEM device attached to the very same device tree node.In all other cases of block devices, the compatible string can be used to unambigously decide whether an NVMEM device should be created or not. blk-nvmem is opt-in, so unless the device is flagged by 'compatible = "nvmem-cells"' it will not do anything.
For all devices which anyway do not have any device tree representation it won't do anything (think: loop, nbd, ...), we would not need to opt them out using
GENHD_FL_NO_NVMEM. Also all other drivers which do not already bring their own NVMEM implementation won't needGENHD_FL_NO_NVMEM, the absence of 'compatible = "nvmem-cells"' is enough to indicate that they should not be considered as NVMEM providers.
Quoted 6 lines
The way you are suggesting will require that, in addition to selecting the targetted block device in device tree, the block driver will also have to set
GENHD_FL_NVMEM. Hence we will need changes in MMC, NVMe and potentially also SATA disk drivers settingGENHD_FL_NVMEMwhen registering the disk.
That is absolutely correct, and was my intention all along.
Drivers which can (and do) supply an NVMEM partition should be required to set this flag, yes.
Cheers,
Hannes
RFC PATCH 4/6 mtd: blkdevs: set GENHD_FL_NO_NVMEM
In reply to Daniel GolleAs the MTD subsystem already acts as an NVMEM provider, emulated mtdblock devices should not be considered NVMEM providers.
--- drivers/mtd/mtd_blkdevs.c | 1 + 1 file changed, 1 insertion(+)
drivers/mtd/mtd_blkdevs.c +1 -0
@@ -362,6 +362,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) gd->flags |=GENHD_FL_NO_PART ; } + gd->flags |=GENHD_FL_NO_NVMEM ;set_capacity (gd, ((u64 )new->size * tr->blksize) >>9 );/* Create the request queue */
Signature · 1 line
2.41.0
Re: [RFC PATCH 4/6] mtd: blkdevs: set GENHD_FL_NO_NVMEM
Hi Daniel,
[email protected] wrote on Wed, 19 Jul 2023 23:03:24 +0100:
Quoted 13 lines
As the MTD subsystem already acts as an NVMEM provider, emulated mtdblock devices should not be considered NVMEM providers.
Signed-off-by Daniel Golle <[email protected]>--- drivers/mtd/mtd_blkdevs.c | 1 + 1 file changed, 1 insertion(+)drivers/mtd/mtd_blkdevs.c +0
-0
drivers/mtd/mtd_blkdevs.c int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) · line 368
Quoted 7 lines
drivers/mtd/mtd_blkdevs.c +1
-0@@ -362,6 +362,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new) gd->flags |=GENHD_FL_NO_PART ; } + gd->flags |=GENHD_FL_NO_NVMEM ;set_capacity (gd, ((u64 )new->size * tr->blksize) >>9 );/* Create the request queue */
Thanks,
Miquèl
RFC PATCH 5/6 mtd: ubi: block: set GENHD_FL_NO_NVMEM
In reply to Daniel GolleAs the UBI volumes may already act as a NVMEM providers, emulated ubiblock devices should not be considered NVMEM providers.
--- drivers/mtd/ubi/block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/ubi/block.c +1 -1
@@ -410,7 +410,7 @@ int ubiblock_create(struct ubi_volume_info *vi) ret = -ENODEV ; goto out_cleanup_disk; } - gd->flags |=GENHD_FL_NO_PART ; + gd->flags |=GENHD_FL_NO_PART |GENHD_FL_NO_NVMEM ; gd->private_data = dev;sprintf (gd->disk_name,"ubiblock%d_%d" , dev->ubi_num, dev->vol_id);set_capacity (gd, disk_capacity);
Signature · 5 lines
2.41.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
RFC PATCH 6/6 block: implement NVMEM provider
In reply to Daniel GolleOn embedded devices using an eMMC it is common that one or more partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data. Allow referencing the partition in device tree for the kernel and Wi-Fi drivers accessing it via the NVMEM layer.
--- block/Kconfig | 8 ++ block/Makefile | 1 + block/blk-nvmem.c | 187 ++++++++++++++++++++++++++++++++++++++++ block/blk.h | 13 +++ block/genhd.c | 2 + block/partitions/core.c | 2 + 6 files changed, 213 insertions(+) create mode 100644 block/blk-nvmem.c
block/Kconfig +8 -0
@@ -218,6 +218,14 @@ config BLK_MQ_VIRTIO config BLK_PM def_bool PM +config BLK_NVMEM + bool "Block device NVMEM provider" + depends on OF + help + Allow block devices (or partitions) to act as NVMEM prodivers, + typically using if an eMMC is used to store MAC addresses or Wi-Fi + calibration data on embedded devices. + # do not use in new code config BLOCK_HOLDER_DEPRECATED bool
block/Makefile +1 -0
@@ -34,6 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED) += blk-zoned.o obj-$(CONFIG_BLK_WBT) += blk-wbt.o obj-$(CONFIG_BLK_DEBUG_FS) += blk-mq-debugfs.o obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o +obj-$(CONFIG_BLK_NVMEM) += blk-nvmem.o obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o obj-$(CONFIG_BLK_PM) += blk-pm.o obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o blk-crypto-profile.o \
block/blk-nvmem.c new file +187 -0
187 lines not on this page · raw message
block/blk.h +13 -0
@@ -515,4 +515,17 @@ static inline int req_ref_read(struct request *req) return atomic_read(&req->ref); } +#ifdef CONFIG_BLK_NVMEM +void blk_register_nvmem (struct block_device *bdev); +void blk_unregister_nvmem (struct block_device *bdev); +#else +static inline void blk_register_nvmem (struct block_device *bdev) +{ +} + +static inline void blk_unregister_nvmem (struct block_device *bdev) +{ +} +#endif +#endif /* BLK_INTERNAL_H */
block/genhd.c +2 -0
@@ -527,6 +527,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,disk_update_readahead (disk);disk_add_events (disk);set_bit (GD_ADDED , &disk->state); +blk_register_nvmem (disk->part0);return 0 ; out_unregister_bdi:
@@ -569,6 +570,7 @@ static void blk_report_disk_dead(struct gendisk *disk)if (bdev->bd_holder_ops && bdev->bd_holder_ops->mark_dead) bdev->bd_holder_ops->mark_dead (bdev);mutex_unlock (&bdev->bd_holder_lock); +blk_unregister_nvmem (bdev);put_device (&bdev->bd_device);rcu_read_lock ();
block/partitions/core.c +2 -0
@@ -404,6 +404,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,/* suppress uevent if the disk suppresses it */ if (!dev_get_uevent_suppress (ddev))kobject_uevent (&pdev->kobj,KOBJ_ADD ); + +blk_register_nvmem (bdev);return bdev; out_del:
Signature · 5 lines
2.41.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 21 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data. Allow referencing the partition in device tree for the kernel and Wi-Fi drivers accessing it via the NVMEM layer.
Signed-off-by Daniel Golle <[email protected]>--- block/Kconfig | 8 ++ block/Makefile | 1 + block/blk-nvmem.c | 187 ++++++++++++++++++++++++++++++++++++++++ block/blk.h | 13 +++ block/genhd.c | 2 + block/partitions/core.c | 2 + 6 files changed, 213 insertions(+) create mode 100644 block/blk-nvmem.cblock/Kconfig +0
-0
block/Kconfig config BLK_MQ_VIRTIO · line 226
Quoted 9 lines
block/Kconfig +6
-0@@ -218,6 +218,14 @@ config BLK_MQ_VIRTIO config BLK_PM def_bool PM +config BLK_NVMEM + bool "Block device NVMEM provider" + depends on OF + help + Allow block devices (or partitions) to act as NVMEM prodivers, + typically using if an eMMC is used to store MAC addresses or Wi-Fi
Odd grammar... May be "typically used with eMMC to store ..."
Quoted 45 lines
block/Kconfig +2
-0+ calibration data on embedded devices. + # do not use in new code config BLOCK_HOLDER_DEPRECATED boolblock/Makefile +1
-0@@ -34,6 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED) += blk-zoned.o obj-$(CONFIG_BLK_WBT) += blk-wbt.o obj-$(CONFIG_BLK_DEBUG_FS) += blk-mq-debugfs.o obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o +obj-$(CONFIG_BLK_NVMEM) += blk-nvmem.o obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o obj-$(CONFIG_BLK_PM) += blk-pm.o obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o blk-crypto-profile.o \block/blk-nvmem.c new file +22
-0@@ -0,0 +1,187 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* block device NVMEM provider +* +* Copyright (c) 2023 Daniel Golle <[email protected]> +* +* Useful on devices using a partition on an eMMC for MAC addresses or +* Wi-Fi calibration EEPROM data. +*/ + +#include "blk.h" +#include <linux/nvmem-provider.h> +#include <linux/of.h> +#include <linux/pagemap.h> +#include <linux/property.h> + +/* List of all NVMEM devices */ +static LIST_HEAD (nvmem_devices); +static DEFINE_MUTEX (devices_mutex); + +struct blk_nvmem { +struct nvmem_device *nvmem;
block/blk-nvmem.c line 32
Quoted 10 lines
block/blk-nvmem.c new file +10
-0+struct block_device *bdev; +struct list_head list; +}; + +static int blk_nvmem_reg_read (void *priv,unsigned int from, +void *val,size_t bytes) +{ +pgoff_t f_index = from >>PAGE_SHIFT ; +struct address_space *mapping; +struct blk_nvmem *bnv = priv;
Why not have bnv passed as argument directly ?
block/blk-nvmem.c line 41
Quoted 9 lines
block/blk-nvmem.c new file +9
-0+size_t bytes_left = bytes; +struct folio *folio; +unsigned long offs, to_read; +void *p; + +if (!bnv->bdev) +return -ENODEV ; + + offs = from & ((1 <<PAGE_SHIFT ) -1 );
offs = from & PAGE_MASK;
from being an int is really odd though.
Quoted 7 lines
block/blk-nvmem.c new file +7
-0+ mapping = bnv->bdev->bd_inode->i_mapping; + +while (bytes_left) { + folio =read_mapping_folio (mapping, f_index++,NULL ); +if (IS_ERR (folio)) +return PTR_ERR (folio); +
block/blk-nvmem.c line 58
Quoted 10 lines
block/blk-nvmem.c new file +10
-0+ to_read = min_t(unsigned long, bytes_left, PAGE_SIZE - offs); + p =folio_address (folio) +offset_in_folio (folio, offs); +memcpy (val, p, to_read); + offs =0 ; + bytes_left -= to_read; + val += to_read; +folio_put (folio); + } + +return bytes_left ==0 ?0 : -EIO ;
How can bytes_left be 0 here given the above loop with no break ?
Quoted 13 lines
block/blk-nvmem.c new file +13
-0+} + +void blk_register_nvmem (struct block_device *bdev) +{ +struct fwnode_handle *fw_parts =NULL , *fw_part_c, *fw_part =NULL ; +struct nvmem_config config = {}; +const char *partname, *uuid; +struct device *dev, *p0dev; +struct blk_nvmem *bnv; +u32 reg; + +/* +* skip devices which set GENHD_FL_NO_NVMEM
block/blk-nvmem.c line 81
Quoted 10 lines
block/blk-nvmem.c new file +10
-0+* +* This flag is used for mtdblock and ubiblock devices because +* both, MTD and UBI already implement their own NVMEM provider. +* To avoid registering multiple NVMEM providers for the same +* device node, skip the block NVMEM provider. +*/ +if (bdev->bd_disk->flags &GENHD_FL_NO_NVMEM ) +return ; + +/* skip too large devices */
Why ? Is that defined in some standards somewhere ?
Quoted 62 lines
block/blk-nvmem.c new file +62
-0+if (bdev_nr_bytes (bdev) >INT_MAX ) +return ; + + dev = &bdev->bd_device; +if (!bdev_is_partition (bdev)) { + fw_part = dev->fwnode; + +if (!fw_part && dev->parent) + fw_part = dev->parent->fwnode; + + goto no_parts; + } + + p0dev = &bdev->bd_disk->part0->bd_device; + fw_parts =device_get_named_child_node (p0dev,"partitions" ); +if (!fw_parts) + fw_parts =device_get_named_child_node (p0dev->parent,"partitions" ); + +if (!fw_parts) +return ; + +fwnode_for_each_child_node (fw_parts, fw_part_c) { +if (!fwnode_property_read_string (fw_part_c,"uuid" , &uuid) && + (!bdev->bd_meta_info ||strncmp (uuid, + bdev->bd_meta_info->uuid, +PARTITION_META_INFO_UUIDLTH ))) +continue ; + +if (!fwnode_property_read_string (fw_part_c,"partname" , &partname) && + (!bdev->bd_meta_info ||strncmp (partname, + bdev->bd_meta_info->volname, +PARTITION_META_INFO_VOLNAMELTH ))) +continue ; + +/* +* partition addresses (reg) in device tree greater than +* DISK_MAX_PARTS can be used to match uuid or partname only +*/ +if (!fwnode_property_read_u32 (fw_part_c,"reg" , ®) && + reg <DISK_MAX_PARTS && bdev->bd_partno != reg) +continue ; + + fw_part = fw_part_c; +break ; + } + +no_parts: +if (!fwnode_device_is_compatible (fw_part,"nvmem-cells" )) +return ; + + bnv =kzalloc (sizeof (struct blk_nvmem ),GFP_KERNEL ); +if (!bnv) +return ; + + config.id =NVMEM_DEVID_NONE ; + config.dev = &bdev->bd_device; + config.name =dev_name (&bdev->bd_device); + config.owner =THIS_MODULE ; + config.priv = bnv; + config.reg_read = blk_nvmem_reg_read; + config.size =bdev_nr_bytes (bdev); + config.word_size =1 ;
block/blk-nvmem.c line 153
Quoted 10 lines
block/blk-nvmem.c new file +10
-0+ config.stride =1 ; + config.read_only = true; + config.root_only = true; + config.ignore_wp = true; + config.of_node =to_of_node (fw_part); + + bnv->bdev = bdev; + bnv->nvmem =nvmem_register (&config); +if (IS_ERR (bnv->nvmem)) { +/* Just ignore if there is no NVMEM support in the kernel */
If there is not, why would this function even be called ?
Quoted 13 lines
block/blk-nvmem.c new file +13
-0+if (PTR_ERR (bnv->nvmem) != -EOPNOTSUPP ) +dev_err_probe (&bdev->bd_device,PTR_ERR (bnv->nvmem), +"Failed to register NVMEM device\n" ); + +kfree (bnv); +return ; + } + +mutex_lock (&devices_mutex); +list_add_tail (&bnv->list, &nvmem_devices); +mutex_unlock (&devices_mutex); +} +
block/blk-nvmem.c line 176
Quoted 10 lines
block/blk-nvmem.c new file +10
-0+void blk_unregister_nvmem (struct block_device *bdev) +{ +struct blk_nvmem *bnv_c, *bnv =NULL ; + +mutex_lock (&devices_mutex); +list_for_each_entry (bnv_c, &nvmem_devices, list) +if (bnv_c->bdev == bdev) { + bnv = bnv_c; +break ; + }
Curly brackets for list_for_each_entry() {} would be nice, even though they are not strictly necessary in this case.
Quoted 16 lines
block/blk-nvmem.c new file +11
-0+ +if (!bnv) { +mutex_unlock (&devices_mutex); +return ; + } + +list_del (&bnv->list); +mutex_unlock (&devices_mutex); +nvmem_unregister (bnv->nvmem); +kfree (bnv); +}block/blk.h +0
-0
block/blk.h static inline int req_ref_read(struct request *req) · line 524
Quoted 10 lines
block/blk.h +7
-0@@ -515,4 +515,17 @@ static inline int req_ref_read(struct request *req)return atomic_read (&req->ref); } +#ifdef CONFIG_BLK_NVMEM +void blk_register_nvmem (struct block_device *bdev); +void blk_unregister_nvmem (struct block_device *bdev); +#else +static inline void blk_register_nvmem (struct block_device *bdev) +{ +}
These could go at the end of the static inline line.
Quoted 40 lines
block/blk.h +6
-0+ +static inline void blk_unregister_nvmem (struct block_device *bdev) +{ +} +#endif +#endif /* BLK_INTERNAL_H */ block/genhd.c +2
-0@@ -527,6 +527,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,disk_update_readahead (disk);disk_add_events (disk);set_bit (GD_ADDED , &disk->state); +blk_register_nvmem (disk->part0);return 0 ; out_unregister_bdi:@@ -569,6 +570,7 @@ static void blk_report_disk_dead(struct gendisk *disk)if (bdev->bd_holder_ops && bdev->bd_holder_ops->mark_dead) bdev->bd_holder_ops->mark_dead (bdev);mutex_unlock (&bdev->bd_holder_lock); +blk_unregister_nvmem (bdev);put_device (&bdev->bd_device);rcu_read_lock ();block/partitions/core.c +2
-0@@ -404,6 +404,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,/* suppress uevent if the disk suppresses it */ if (!dev_get_uevent_suppress (ddev))kobject_uevent (&pdev->kobj,KOBJ_ADD ); + +blk_register_nvmem (bdev);return bdev; out_del:
Signature · 2 lines
Damien Le Moal Western Digital Research
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 25 lines · Damien Le Moal
Quoted 24 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data. Allow referencing the partition in device tree for the kernel and Wi-Fi drivers accessing it via the NVMEM layer.
Signed-off-by Daniel Golle <[email protected]>--- block/Kconfig | 8 ++ block/Makefile | 1 + block/blk-nvmem.c | 187 ++++++++++++++++++++++++++++++++++++++++ block/blk.h | 13 +++ block/genhd.c | 2 + block/partitions/core.c | 2 + 6 files changed, 213 insertions(+) create mode 100644 block/blk-nvmem.cblock/Kconfig +0
-0@@ -218,6 +218,14 @@ config BLK_MQ_VIRTIO config BLK_PM def_bool PM
Quoted 8 lines
block/Kconfig config BLK_MQ_VIRTIO · line 226
Quoted 6 lines
block/Kconfig +6
-0+config BLK_NVMEM + bool "Block device NVMEM provider" + depends on OF + help + Allow block devices (or partitions) to act as NVMEM prodivers, + typically using if an eMMC is used to store MAC addresses or Wi-FiOdd grammar... May be "typically used with eMMC to store ..."
Thank you, I'll use your suggestion.
Quoted 50 lines
Quoted 50 lines
block/Kconfig +2
-0+ calibration data on embedded devices. + # do not use in new code config BLOCK_HOLDER_DEPRECATED boolblock/Makefile +1
-0@@ -34,6 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED) += blk-zoned.o obj-$(CONFIG_BLK_WBT) += blk-wbt.o obj-$(CONFIG_BLK_DEBUG_FS) += blk-mq-debugfs.o obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o +obj-$(CONFIG_BLK_NVMEM) += blk-nvmem.o obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o obj-$(CONFIG_BLK_PM) += blk-pm.o obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o blk-crypto-profile.o \block/blk-nvmem.c new file +26
-0@@ -0,0 +1,187 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* +* block device NVMEM provider +* +* Copyright (c) 2023 Daniel Golle <[email protected]> +* +* Useful on devices using a partition on an eMMC for MAC addresses or +* Wi-Fi calibration EEPROM data. +*/ + +#include "blk.h" +#include <linux/nvmem-provider.h> +#include <linux/of.h> +#include <linux/pagemap.h> +#include <linux/property.h> + +/* List of all NVMEM devices */ +static LIST_HEAD (nvmem_devices); +static DEFINE_MUTEX (devices_mutex); + +struct blk_nvmem { +struct nvmem_device *nvmem; +struct block_device *bdev; +struct list_head list; +}; +
Quoted 8 lines
block/blk-nvmem.c line 32
Quoted 6 lines
block/blk-nvmem.c new file +6
-0+static int blk_nvmem_reg_read (void *priv,unsigned int from, +void *val,size_t bytes) +{ +pgoff_t f_index = from >>PAGE_SHIFT ; +struct address_space *mapping; +struct blk_nvmem *bnv = priv;Why not have bnv passed as argument directly ?
Because blk_nvmem_reg_read is used as reg_read function, ie. nvmem_reg_read_t {aka 'int (*)(void *, unsigned int, void *, long unsigned int)'}
Hence 'void *' has to be implicitely type-casted into 'struct blk_nvmem *'.
Quoted 12 lines
block/blk-nvmem.c line 41
Quoted 10 lines
block/blk-nvmem.c new file +9
-0+size_t bytes_left = bytes; +struct folio *folio; +unsigned long offs, to_read; +void *p; + +if (!bnv->bdev) +return -ENODEV ; + + offs = from & ((1 <<PAGE_SHIFT ) -1 );offs = from &
PAGE_MASK;
Right, that makes it easier...
Quoted 2 lines
from being an int is really odd though.
Yep, but that's how NVMEM framework defines it.
Quoted 12 lines
Quoted 12 lines
block/blk-nvmem.c new file +11
-0+ mapping = bnv->bdev->bd_inode->i_mapping; + +while (bytes_left) { + folio =read_mapping_folio (mapping, f_index++,NULL ); +if (IS_ERR (folio)) +return PTR_ERR (folio); + + to_read = min_t(unsigned long, bytes_left, PAGE_SIZE - offs); + p =folio_address (folio) +offset_in_folio (folio, offs); +memcpy (val, p, to_read); + offs =0 ;
Quoted 8 lines
block/blk-nvmem.c line 58
Quoted 6 lines
block/blk-nvmem.c new file +6
-0+ bytes_left -= to_read; + val += to_read; +folio_put (folio); + } + +return bytes_left ==0 ?0 : -EIO ;How can
bytes_leftbe 0 here given the above loop with no break ?
Well, right... Can just be 'return 0;' at this point obviously.
Quoted 18 lines
Quoted 18 lines
block/blk-nvmem.c new file +17
-0+} + +void blk_register_nvmem (struct block_device *bdev) +{ +struct fwnode_handle *fw_parts =NULL , *fw_part_c, *fw_part =NULL ; +struct nvmem_config config = {}; +const char *partname, *uuid; +struct device *dev, *p0dev; +struct blk_nvmem *bnv; +u32 reg; + +/* +* skip devices which set GENHD_FL_NO_NVMEM +* +* This flag is used for mtdblock and ubiblock devices because +* both, MTD and UBI already implement their own NVMEM provider. +* To avoid registering multiple NVMEM providers for the same
Quoted 8 lines
block/blk-nvmem.c line 81
Quoted 6 lines
block/blk-nvmem.c new file +6
-0+* device node, skip the block NVMEM provider. +*/ +if (bdev->bd_disk->flags &GENHD_FL_NO_NVMEM ) +return ; + +/* skip too large devices */ Why ? Is that defined in some standards somewhere ?
NVMEM framework uses 'int' to address byte offsets inside NVMEM devices. Hence devices larger than INT_MAX cannot be addressed, I will also state that in that comment.
Quoted 67 lines
Quoted 67 lines
block/blk-nvmem.c new file +66
-0+if (bdev_nr_bytes (bdev) >INT_MAX ) +return ; + + dev = &bdev->bd_device; +if (!bdev_is_partition (bdev)) { + fw_part = dev->fwnode; + +if (!fw_part && dev->parent) + fw_part = dev->parent->fwnode; + + goto no_parts; + } + + p0dev = &bdev->bd_disk->part0->bd_device; + fw_parts =device_get_named_child_node (p0dev,"partitions" ); +if (!fw_parts) + fw_parts =device_get_named_child_node (p0dev->parent,"partitions" ); + +if (!fw_parts) +return ; + +fwnode_for_each_child_node (fw_parts, fw_part_c) { +if (!fwnode_property_read_string (fw_part_c,"uuid" , &uuid) && + (!bdev->bd_meta_info ||strncmp (uuid, + bdev->bd_meta_info->uuid, +PARTITION_META_INFO_UUIDLTH ))) +continue ; + +if (!fwnode_property_read_string (fw_part_c,"partname" , &partname) && + (!bdev->bd_meta_info ||strncmp (partname, + bdev->bd_meta_info->volname, +PARTITION_META_INFO_VOLNAMELTH ))) +continue ; + +/* +* partition addresses (reg) in device tree greater than +* DISK_MAX_PARTS can be used to match uuid or partname only +*/ +if (!fwnode_property_read_u32 (fw_part_c,"reg" , ®) && + reg <DISK_MAX_PARTS && bdev->bd_partno != reg) +continue ; + + fw_part = fw_part_c; +break ; + } + +no_parts: +if (!fwnode_device_is_compatible (fw_part,"nvmem-cells" )) +return ; + + bnv =kzalloc (sizeof (struct blk_nvmem ),GFP_KERNEL ); +if (!bnv) +return ; + + config.id =NVMEM_DEVID_NONE ; + config.dev = &bdev->bd_device; + config.name =dev_name (&bdev->bd_device); + config.owner =THIS_MODULE ; + config.priv = bnv; + config.reg_read = blk_nvmem_reg_read; + config.size =bdev_nr_bytes (bdev); + config.word_size =1 ; + config.stride =1 ; + config.read_only = true; + config.root_only = true; + config.ignore_wp = true;
Quoted 8 lines
block/blk-nvmem.c line 153
Quoted 6 lines
block/blk-nvmem.c new file +6
-0+ config.of_node =to_of_node (fw_part); + + bnv->bdev = bdev; + bnv->nvmem =nvmem_register (&config); +if (IS_ERR (bnv->nvmem)) { +/* Just ignore if there is no NVMEM support in the kernel */ If there is not, why would this function even be called ?
True. Kconfig depends should make sure blk-nvmem is only built if nvmem is supported at all.
Quoted 19 lines
Quoted 19 lines
block/blk-nvmem.c new file +18
-0+if (PTR_ERR (bnv->nvmem) != -EOPNOTSUPP ) +dev_err_probe (&bdev->bd_device,PTR_ERR (bnv->nvmem), +"Failed to register NVMEM device\n" ); + +kfree (bnv); +return ; + } + +mutex_lock (&devices_mutex); +list_add_tail (&bnv->list, &nvmem_devices); +mutex_unlock (&devices_mutex); +} + +void blk_unregister_nvmem (struct block_device *bdev) +{ +struct blk_nvmem *bnv_c, *bnv =NULL ; + +mutex_lock (&devices_mutex);
Quoted 8 lines
block/blk-nvmem.c line 176
Quoted 5 lines
block/blk-nvmem.c new file +5
-0+list_for_each_entry (bnv_c, &nvmem_devices, list) +if (bnv_c->bdev == bdev) { + bnv = bnv_c; +break ; + }Curly brackets for
list_for_each_entry(){} would be nice, even though they are not strictly necessary in this case.
Ack, will add them.
Quoted 20 lines
Quoted 20 lines
block/blk-nvmem.c new file +11
-0+ +if (!bnv) { +mutex_unlock (&devices_mutex); +return ; + } + +list_del (&bnv->list); +mutex_unlock (&devices_mutex); +nvmem_unregister (bnv->nvmem); +kfree (bnv); +}block/blk.h +0
-0@@ -515,4 +515,17 @@ static inline int req_ref_read(struct request *req)return atomic_read (&req->ref); }
Quoted 9 lines
block/blk.h static inline int req_ref_read(struct request *req) · line 524
Quoted 7 lines
block/blk.h +7
-0+#ifdef CONFIG_BLK_NVMEM +void blk_register_nvmem (struct block_device *bdev); +void blk_unregister_nvmem (struct block_device *bdev); +#else +static inline void blk_register_nvmem (struct block_device *bdev) +{ +}These could go at the end of the static inline line.
I tried keeping the existing style in that header file. See a couple of lines above:
static inline void bio_integrity_free(struct bio *bio) {
}
Quoted 46 lines
Quoted 41 lines
block/blk.h +6
-0+ +static inline void blk_unregister_nvmem (struct block_device *bdev) +{ +} +#endif +#endif /* BLK_INTERNAL_H */ block/genhd.c +2
-0@@ -527,6 +527,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,disk_update_readahead (disk);disk_add_events (disk);set_bit (GD_ADDED , &disk->state); +blk_register_nvmem (disk->part0);return 0 ; out_unregister_bdi:@@ -569,6 +570,7 @@ static void blk_report_disk_dead(struct gendisk *disk)if (bdev->bd_holder_ops && bdev->bd_holder_ops->mark_dead) bdev->bd_holder_ops->mark_dead (bdev);mutex_unlock (&bdev->bd_holder_lock); +blk_unregister_nvmem (bdev);put_device (&bdev->bd_device);rcu_read_lock ();block/partitions/core.c +2
-0@@ -404,6 +404,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,/* suppress uevent if the disk suppresses it */ if (!dev_get_uevent_suppress (ddev))kobject_uevent (&pdev->kobj,KOBJ_ADD ); + +blk_register_nvmem (bdev);return bdev; out_del:Signature · 2 lines
Damien Le Moal Western Digital Research
Re: [RFC PATCH 6/6] block: implement NVMEM provider
In reply to Daniel GolleThe layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Ok. I will use a class_interface instead.
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 9 lines · Daniel Golle
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Ok. I will use a
class_interfaceinstead.
I'm not sure a class_interface makes much sense here. Why does the block layer even need to know about you using a device a nvmem provider? As far as I can tell your provider should layer entirely above the block layer and not have to be integrated with it.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Daniel Golle
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Quoted 4 lines
Quoted 1 line
Ok. I will use a
class_interfaceinstead.I'm not sure a
class_interfacemakes much sense here. Why does the block layer even need to know about you using a device a nvmem provider?
It doesn't. But it has to notify the nvmem providing driver about the addition of new block devices. This is what I'm using class_interface for, simply to hook into .add_dev of the block_class.
Quoted 2 lines
As far as I can tell your provider should layer entirely above the block layer and not have to be integrated with it.
My approach using class_interface doesn't require any changes to be made to existing block code. However, it does use block_class. If you see any other good option to implement matching off and usage of block devices by in-kernel users, please let me know.
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 10 lines · Daniel Golle
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Daniel Golle
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Ok. I will use a
class_interfaceinstead.I'm not sure a
class_interfacemakes much sense here. Why does the block layer even need to know about you using a device a nvmem provider?It doesn't. But it has to notify the nvmem providing driver about the addition of new block devices. This is what I'm using
class_interfacefor, simply to hook into .add_dev of theblock_class.
Why is this single type of block device special to require this, yet all others do not? Encoding this into the block layer feels like a huge layering violation to me, why not do it how all other block drivers do it instead?
Quoted 7 lines
Quoted 2 lines
As far as I can tell your provider should layer entirely above the block layer and not have to be integrated with it.
My approach using
class_interfacedoesn't require any changes to be made to existing block code. However, it does useblock_class. If you see any other good option to implement matching off and usage of block devices by in-kernel users, please let me know.
Do not use block_class, again, that should only be for the block core to touch. Individual block drivers should never be poking around in it.
thanks,
greg k-h
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 16 lines · Greg Kroah-Hartman
Quoted 15 lines · Daniel Golle
Quoted 13 lines · Christoph Hellwig
Quoted 9 lines · Daniel Golle
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Ok. I will use a
class_interfaceinstead.I'm not sure a
class_interfacemakes much sense here. Why does the block layer even need to know about you using a device a nvmem provider?
Quoted 8 lines
Quoted 3 lines
It doesn't. But it has to notify the nvmem providing driver about the addition of new block devices. This is what I'm using
class_interfacefor, simply to hook into .add_dev of theblock_class.Why is this single type of block device special to require this, yet all others do not? Encoding this into the block layer feels like a huge layering violation to me, why not do it how all other block drivers do it instead?
I was thinkng of this as a generic solution in no way tied to one specific type of block device. *Any* internal block device which can be used to boot from should also be usable as NVMEM provider imho.
Just like all MTD devices also act as NVMEM providers (just in case of block devices I'd make that opt-in via device tree).
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
As far as I can tell your provider should layer entirely above the block layer and not have to be integrated with it.
My approach using
class_interfacedoesn't require any changes to be made to existing block code. However, it does useblock_class. If you see any other good option to implement matching off and usage of block devices by in-kernel users, please let me know.Do not use
block_class, again, that should only be for the block core to touch. Individual block drivers should never be poking around in it.
Do I have any other options to coldplug and be notified about newly added block devices, so the block-device-consuming driver can know about them?
This is not a rhetoric question, I've been looking for other ways and haven't found anything better than class_find_device or class_interface. Using those also prevents blk-nvmem to be built as a module, so I'd really like to find alternatives.
E.g. for MTD we got struct mtd_notifier and register_mtd_user().
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 21 lines · Daniel Golle
Quoted 20 lines · Greg Kroah-Hartman
Quoted 18 lines · Daniel Golle
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Daniel Golle
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Quoted 4 lines
Quoted 1 line
Ok. I will use a
class_interfaceinstead.I'm not sure a
class_interfacemakes much sense here. Why does the block layer even need to know about you using a device a nvmem provider?It doesn't. But it has to notify the nvmem providing driver about the addition of new block devices. This is what I'm using
class_interfacefor, simply to hook into .add_dev of theblock_class.
Quoted 8 lines
Quoted 4 lines
Why is this single type of block device special to require this, yet all others do not? Encoding this into the block layer feels like a huge layering violation to me, why not do it how all other block drivers do it instead?
I was thinkng of this as a generic solution in no way tied to one specific type of block device. *Any* internal block device which can be used to boot from should also be usable as NVMEM provider imho.
Define "internal" :)
And that's all up to the boot process in userspace, the kernel doesn't care about this.
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
As far as I can tell your provider should layer entirely above the block layer and not have to be integrated with it.
My approach using
class_interfacedoesn't require any changes to be made to existing block code. However, it does useblock_class. If you see any other good option to implement matching off and usage of block devices by in-kernel users, please let me know.
Quoted 6 lines
Quoted 2 lines
Do not use
block_class, again, that should only be for the block core to touch. Individual block drivers should never be poking around in it.Do I have any other options to coldplug and be notified about newly added block devices, so the block-device-consuming driver can know about them?
What other options do you need?
Quoted 3 lines
This is not a rhetoric question, I've been looking for other ways and haven't found anything better than
class_find_deviceorclass_interface.
Never use that, sorry, that's not for a driver to touch.
Quoted 3 lines
Using those also prevents blk-nvmem to be built as
a module, so I'd really like to find alternatives. E.g. for MTD we got structmtd_notifierandregister_mtd_user().
Your storage/hardware driver should be the thing that "finds block devices" and registers them with the block class core, right? After that, what matters?
confused,
greg k-h
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 27 lines · Greg Kroah-Hartman
Quoted 26 lines · Daniel Golle
Quoted 24 lines · Greg Kroah-Hartman
Quoted 10 lines · Daniel Golle
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Daniel Golle
Quoted 6 lines · Christoph Hellwig
The layering here is exactly the wrong way around. This block device as nvmem provide has not business sitting in the block layer and being keyed ff the gendisk registration. Instead you should create a new nvmem backed that opens the block device as needed if it fits your OF description without any changes to the core block layer.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Ok. I will use a
class_interfaceinstead.I'm not sure a
class_interfacemakes much sense here. Why does the block layer even need to know about you using a device a nvmem provider?It doesn't. But it has to notify the nvmem providing driver about the addition of new block devices. This is what I'm using
class_interfacefor, simply to hook into .add_dev of theblock_class.Why is this single type of block device special to require this, yet all others do not? Encoding this into the block layer feels like a huge layering violation to me, why not do it how all other block drivers do it instead?
Quoted 5 lines
Quoted 3 lines
I was thinkng of this as a generic solution in no way tied to one specific type of block device. *Any* internal block device which can be used to boot from should also be usable as NVMEM provider imho.
Define "internal" :)
Exactly, it could be anything, MMC, SATA, even USB. As long as there is a device tree node associated with the device, it could be referenced as an NVMEM provider. And that's what this series tries to implement.
Quoted 3 lines
And that's all up to the boot process in userspace, the kernel doesn't care about this.
So lets look at any random embedded Linux router or Wi-Fi AP:
Typically we got some kind of NAND or NOR flash memory with a hard-coded partitioning dedicating different areas in the flash to be used for the bootloader, bootloader environment, factory data (such as MAC addresses and Wi-Fi EEPROMs), a linux kernel, a root filesystem and some way to store user data.
And things are not that different when using an eMMC instead of NOR flash, from the perspective of the OS the main difference is just that the eMMC is larger and represented as a block device and typically MBR or GPT partitioning is used.
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
As far as I can tell your provider should layer entirely above the block layer and not have to be integrated with it.
My approach using
class_interfacedoesn't require any changes to be made to existing block code. However, it does useblock_class. If you see any other good option to implement matching off and usage of block devices by in-kernel users, please let me know.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Do not use
block_class, again, that should only be for the block core to touch. Individual block drivers should never be poking around in it.Do I have any other options to coldplug and be notified about newly added block devices, so the block-device-consuming driver can know about them?
What other options do you need?
Either calls for adding/removing the NVMEM devices need to be added to block/genhd.c and block/partitions/core.c, and if that's not acceptable (see below), then we'd need something like register_mtd_user() would be for MTD:
A way for a driver to cold-plug existing block devices and be notified about newly added ones.
Quoted 7 lines
Quoted 4 lines
This is not a rhetoric question, I've been looking for other ways and haven't found anything better than
class_find_deviceorclass_interface.Never use that, sorry, that's not for a driver to touch.
Quoted 7 lines
Quoted 3 lines
Using those also prevents blk-nvmem to be built as
a module, so I'd really like to find alternatives. E.g. for MTD we got structmtd_notifierandregister_mtd_user().Your storage/hardware driver should be the thing that "finds block devices" and registers them with the block class core, right? After that, what matters?
Well, I'd say: Make the device available as NVMEM provider, as it could be needed for e.g. the Ethernet driver to know the MAC address to be used.
This is how it is done for MTD devices here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mtd/mtdcore.c#n723
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mtd/mtdcore.c#n804
... and then used for example here:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/arch/arm64/boot/dts/marvell/armada-3720-gl-mv1000.dts#n227
The idea to use a class_interface at all is a result of the criticism by Christoph Hellwig that the series requires changes to be made to the block subsystem, ie. calls to register and unregister the NVMEM provider.
Yet another way would obviously be to let the block-device-providing driver (eg. drivers/mmc/core/block.c) register the NVMEM provider, but that would then have to be re-implemented for every relevant block driver which also seems wrong.
Re: [RFC PATCH 6/6] block: implement NVMEM provider
In reply to Daniel Golleblock/blk-nvmem.c line 36
Quoted 10 lines
block/blk-nvmem.c new file +10
-0+static int blk_nvmem_reg_read (void *priv,unsigned int from, +void *val,size_t bytes) +{ +pgoff_t f_index = from >>PAGE_SHIFT ; +struct address_space *mapping; +struct blk_nvmem *bnv = priv; +size_t bytes_left = bytes; +struct folio *folio; +unsigned long offs, to_read; +void *p;
Btw, this function really should be using kern_read on a file using filp_open instead of poking into block device internals. That way you can even have a generic file provider that works on anything that can be read from.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 6/6] block: implement NVMEM provider
Quoted 5 lines · Christoph Hellwig
Quoted 5 lines
block/blk-nvmem.c new file +5
-0+static int blk_nvmem_reg_read (void *priv,unsigned int from, +void *val,size_t bytes) +{ +pgoff_t f_index = from >>PAGE_SHIFT ; +struct address_space *mapping;
Quoted 8 lines
block/blk-nvmem.c line 36
Quoted 5 lines
block/blk-nvmem.c new file +5
-0+struct blk_nvmem *bnv = priv; +size_t bytes_left = bytes; +struct folio *folio; +unsigned long offs, to_read; +void *p;Btw, this function really should be using
kern_readon a file usingfilp_openinstead of poking into block device internals.
Unfortunately filp_open requires a device inode on a filesystem to be able to open a block device. What if the root filesystem has not yet been mounted, or even requires NVMEM (let's say for the Ethernet MAC address to be known for a nfsroot, nbd or iSCSI to work) to be available first?
Can you imagine we could implement something like filp_open_bdev which takes a (struct block_device *) as parameter instead of (const char *)?
That would allow the driver to open and read from the block device before any filesystems (incl. /dev) become ready.
Quoted 3 lines
That way you can even have a generic file provider that works on anything that can be read from.
While this could also be useful, it doesn't fulfil the requirement of making NVMEM available as early as possible for other drivers to use, e.g. for the Ethernet MAC address.
And it would also heavily deviate from how things work with other types of flash storage typically used in embedded Linux devices such as SPI NOR or NAND flash which is represented as MTD device.
Sidenote: block2mtd is also not an option because one cannot reference block2mtd devices in device tree, hence they cannot serve as NVMEM providers.
Quoted 4 lines · Daniel Golle
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
If only NVMe did something sane there. NVMe copied the completely idiotic idea of boot partitions from eMMC/SD, and made it even worse by first requiring bit banged register access for them, and now adding a weird admin command to read them, which is not bound to a block device. If we want to support that in NVMe we'll need code in the nvme core, but I hope we can avoid it and no one sane (that is everyone but the completely clueless BIOS people at Intel) will never use this completely fucked up boot partition concept.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Quoted 8 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.
Implement an NVMEM provider backed by block devices as typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...
Quoted 7 lines
In order to be able to reference hardware partitions on an eMMC, add code to bind each hardware partition to a specific firmware subnode.
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
Is my understanding correct that these devices boot from eMMC and not over Wi-Fi? If so, why does this calibration data have to be stored on a raw block device? Why can't this information be loaded from a file on a filesystem?
Thanks,
Bart.
Quoted 12 lines · Bart Van Assche
Quoted 11 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.
Implement an NVMEM provider backed by block devices as typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...
In order to be able to reference hardware partitions on an eMMC, add code to bind each hardware partition to a specific firmware subnode.
Quoted 7 lines
Quoted 4 lines
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
Is my understanding correct that these devices boot from eMMC and not over Wi-Fi?
Yes, that's right.
Quoted 2 lines
If so, why does this calibration data have to be stored on a raw block device?
It's just how many vendors decided to implement it in their firmware.
I reckon this happened mostly out of habbit and in order to avoid any potential complexities in their manufacturing and QA processes -- most of those processes have been created in a time when NOR flash was the most common way to store firmware on embedded devices with Wi-Fi.
The occurance of raw NAND and SPI-NAND didn't change much about that, most vendors still just use a raw MTD partition to store all that per-device 'factory' data. Very few have have started to use raw data inside UBI volumes (and I've been working on a UBI NVMEM driver as well).
Also when it comes to eMMC they just keep doing things how they were always doing them, just that instead of a raw NOR flash, some devices nowadays come with an eMMC. Some vendors use the 'boot1' hardware partition to store things like MAC addresses at raw offsets there (GL.iNet for example [1]), others use small GPT or MBR partitions on the main hardware partition of the eMMC to store MAC addresses and calibration blobs (for example Adtran [2]).
Quoted 2 lines
Why can't this information be loaded from a file on a filesystem?
Having this information stored in a file on a filesystem is also an option, of course, but out in the wild you will hardly find it being done like that, for the reason stated above and also because having the OS pull the Ethernet MAC addresses somewhere from a filesystem while in early user-space will require the OS to even know about the individual hardware and how this should be done, so it's not exactly beautiful, may need fsck, mounting, ... all before the machine can become available with it's assigned MAC address on Ethernet.
The RaspberryPi and other brcmfmac-based systems are the exception to the rule, here a simple text file stored on the root filesystem serves the Wi-Fi calibration. This file hence needs to be shipped with the OS instead of being stored on the device, and e.g. Raspbian does so. I suppose this is mostly because there just isn't any permanent on-board storage large enough and being a low-barrier DYI system they wanted to make it easy for users designing systems besed on that (as in: not having to mess with raw offsets or partitions, but rather just use tools like Etcher on Windows and edit CR-LF terminated text files on a FAT filesystem inside notepad...).
However, practically all Qualcomm and MediaTek Wi-Fi AP/Routers which come with an eMMC come with their MAC addresses and Wi-Fi EEPROM data stored as raw data inside a hardware or software partition.
[1]: https://github.com/dangowrt/linux/commit/8a24f03efb6b1dd7d8fdf68558146bacc71c2c1b
[2]: https://github.com/dangowrt/linux/commit/b14c0215641047d0447956b40bd344049a11becb
Quoted 8 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.
Implement an NVMEM provider backed by block devices as typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...
Quoted 7 lines
In order to be able to reference hardware partitions on an eMMC, add code to bind each hardware partition to a specific firmware subnode.
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
Just wondering why you don't use request_firmware() in drivers which consume the data, then the logic can be moved out of kernel, and you needn't to deal with device tree & block device.
Or Android doesn't support udev and initrd?
Thanks,
Ming
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Quoted 12 lines · Ming Lei
Quoted 11 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.
Implement an NVMEM provider backed by block devices as typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...
In order to be able to reference hardware partitions on an eMMC, add code to bind each hardware partition to a specific firmware subnode.
Quoted 8 lines
Quoted 4 lines
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
Just wondering why you don't use
request_firmware()in drivers which consume the data, then the logic can be moved out of kernel, and you needn't to deal with device tree & block device.
The thing is: Why should the OS need to know about how to read calibration to be fed into a wireless driver on a specific hardware/firmware?
Or even worse: The MAC address of the main Ethernet interface? What if for one of many possible reasons the extraction script in userland gets broken and you would need to recover the system via SSH (because it's a router or switch and doesn't even have any local console)?
Having information about the location of firmware artifacts be supplied by the firmware (via device tree) seems to be the natural choice, and also how it is done for devices booting off NOR or NAND flash by using the NVMEM framework. Why should eMMC be the exception?
Quoted 2 lines
Or Android doesn't support udev and initrd?
Yes, sure, but then the OS ROM needs to know about and handle a lot of very device-specific details in userland and if (unlike Android) the same OS ROM should run on many devices that would mean it will have to ship all of them in a huge initrd or root filesystem.
Also using userspace mechanics to acquire things as basic as a MAC address seems overly complex and much more fragile than having the firmware instruct the kernel via device tree to do so.
Hence, within the OpenWrt project, we've been working for years now to push for Device Tree and lately for use of the NVMEM framework, exactly to **reduce** the amount of hardware-specific knowledge in userland and get rid of shell scripts acting as hotplug handlers for firmware requests and such.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Quoted 17 lines · Ming Lei
Quoted 8 lines · Daniel Golle
On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.
Implement an NVMEM provider backed by block devices as typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...
Quoted 7 lines
In order to be able to reference hardware partitions on an eMMC, add code to bind each hardware partition to a specific firmware subnode.
This series is meant to open the discussion on how exactly the device tree schema for block devices and partitions may look like, and even if using the block layer to back the NVMEM device is at all the way to go -- to me it seemed to be a good solution because it will be reuable e.g. for NVMe.
Quoted 5 lines
Just wondering why you don't use
request_firmware()in drivers which consume the data, then the logic can be moved out of kernel, and you needn't to deal with device tree & block device.Or Android doesn't support udev and initrd?
It does support initrd, but not really udev last I looked.
But it does allow request_firmware() to be called at boot time, so yes, finding out why that isn't used here would be good.
thanks,
greg k-h
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/