mtd: ubi: allow UBI volumes to provide NVMEM
v4 cover mtd: ubi: allow UBI volumes to provide NVMEM
The series is a follow-up and contains all patches of the previous series "mtd: ubi: behave like a good MTD citizen"[1] which was meant in preparation for implementing the NVMEM provider.
The goal is to support embedded Linux devices which got NVMEM bits stored inside a UBI volume. Representing the UBI volume in the Device Tree, adding a phandle to be referenced by NVMEM consumers allows such devices to come up with their correct MAC addresses and device-specific Wi-Fi calibration data loaded.
In order to be available for other drivers, attaching UBI devices has to be moved from late_initcall (which is too late for other drivers) to happen earlier. As an alternative to the existing kernel cmdline parameter the Device Tree property 'compatible = "linux,ubi";' inside an MTD partition can be used to have that MTD device attached as UBI device. MTD partitions which serve as UBI devices may have a "volumes" firmware subnode with volumes which may be compatible with "nvmem-cells".
In this way, other drivers (think: Ethernet, Wi-Fi) can resolve and acquire NVMEM bits using the usual device tree phandle, just this time the NVMEM content is read from a UBI volume.
[1]: https://patchwork.ozlabs.org/project/linux-mtd/list/?series=353177&state=%2A&archive=both
Changes since v3:
* dt-bindings fixes as requested
Changes since v2:
* include dt-bindings additions
Changes since v1:
* include patch to fix exiting Kconfig formatting issues
* fix typo and indentation in Kconfig
Daniel Golle (8):
dt-bindings: mtd: add basic bindings for UBI
dt-bindings: mtd: nvmem-cells: add support for UBI volumes
mtd: ubi: block: don't return on error when removing
mtd: ubi: block: use notifier to create ubiblock
mtd: ubi: attach MTD partition from device-tree
mtd: ubi: introduce pre-removal notification for UBI volumes
mtd: ubi: populate ubi volume fwnode
mtd: ubi: provide NVMEM layer over UBI volumes
.../bindings/mtd/partitions/linux,ubi.yaml | 66 ++++++ .../bindings/mtd/partitions/nvmem-cells.yaml | 5 +- .../bindings/mtd/partitions/ubi-volume.yaml | 36 ++++ drivers/mtd/ubi/Kconfig | 12 ++ drivers/mtd/ubi/Makefile | 1 + drivers/mtd/ubi/block.c | 186 ++++++++++------- drivers/mtd/ubi/build.c | 160 +++++++++++---- drivers/mtd/ubi/cdev.c | 4 +- drivers/mtd/ubi/nvmem.c | 189 ++++++++++++++++++ drivers/mtd/ubi/ubi.h | 6 +- drivers/mtd/ubi/vmt.c | 32 +++ include/linux/mtd/ubi.h | 2 + 12 files changed, 579 insertions(+), 120 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/linux,ubi.yaml create mode 100644 Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml create mode 100644 drivers/mtd/ubi/nvmem.c
Signature · 1 line
2.41.0
PATCH v4 1/8 dt-bindings: mtd: add basic bindings for UBI
Add basic bindings for UBI devices and volumes.
--- .../bindings/mtd/partitions/linux,ubi.yaml | 65 +++++++++++++++++++ .../bindings/mtd/partitions/ubi-volume.yaml | 36 ++++++++++ 2 files changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/linux,ubi.yaml create mode 100644 Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml
Documentation/devicetree/bindings/mtd/partitions/linux,ubi.yaml new file +65 -0
@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/linux,ubi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Unsorted Block Images + +description: | + UBI ("Unsorted Block Images") is a volume management system for raw + flash devices which manages multiple logical volumes on a single + physical flash device and spreads the I/O load (i.e, wear-leveling) + across whole flash chip. + +maintainers: + - Daniel Golle <[email protected]> + +allOf: + - $ref: partition.yaml# + +properties: + compatible: + const: linux,ubi + + volumes: + type: object + description: UBI Volumes + + patternProperties: + "^ubi-volume-.*$": + $ref: /schemas/mtd/partitions/ubi-volume.yaml + + unevaluatedProperties: false + +required: + - compatible + +unevaluatedProperties: false + +examples: + - | + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + reg = <0x0 0x100000>; + label = "bootloader"; + read-only; + }; + + partition@100000 { + reg = <0x100000 0x1ff00000>; + label = "ubi"; + compatible = "linux,ubi"; + + volumes { + ubi-volume-caldata { + volume-id = <2>; + volume-name = "rf"; + }; + }; + }; + };
Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml new file +36 -0
@@ -0,0 +1,36 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/ubi-volume.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: UBI volume + +description: | + This binding describes a single UBI volume. Volumes can be matches either + by their ID or their name, or both. + +maintainers: + - Daniel Golle <[email protected]> + +properties: + compatible: true + + volume-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Match UBI volume ID + + volume-name: + $ref: /schemas/types.yaml#/definitions/string + description: + Match UBI volume name + +anyOf: + - required: + - volume-id + + - required: + - volume-name + +unevaluatedProperties: false
Signature · 1 line
2.41.0
PATCH v4 2/8 dt-bindings: mtd: nvmem-cells: add support for UBI volumes
In reply to Daniel GolleUBI volumes may be used to contain NVMEM bits, typically device MAC addresses or wireless radio calibration data.
--- .../devicetree/bindings/mtd/partitions/linux,ubi.yaml | 3 ++- .../devicetree/bindings/mtd/partitions/nvmem-cells.yaml | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/mtd/partitions/linux,ubi.yaml +2 -1
@@ -56,9 +56,10 @@ examples: compatible = "linux,ubi"; volumes { - ubi-volume-caldata { + wifi_caldata: ubi-volume-caldata { volume-id = <2>; volume-name = "rf"; + compatible = "nvmem-cells"; }; }; };
Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml +4 -1
@@ -17,9 +17,12 @@ maintainers: - Ansuel Smith <[email protected]> allOf: - - $ref: /schemas/mtd/partitions/partition.yaml# - $ref: /schemas/nvmem/nvmem.yaml# +oneOf: + - $ref: /schemas/mtd/partitions/partition.yaml# + - $ref: /schemas/mtd/partitions/ubi-volume.yaml# + properties: compatible: const: nvmem-cells
Signature · 1 line
2.41.0
PATCH v4 3/8 mtd: ubi: block: don't return on error when removing
In reply to Daniel GolleThere is no point on returning the error from ubiblock_remove in case it is being called due to a volume removal event -- the volume is gone, we should destroy and remove the ubiblock device no matter what.
Introduce new boolean parameter 'force' to tell ubiblock_remove to go on even in case the ubiblock device is still busy. Use that new option when calling ubiblock_remove due to a UBI_VOLUME_REMOVED event.
--- drivers/mtd/ubi/block.c | 6 +++--- drivers/mtd/ubi/cdev.c | 2 +- drivers/mtd/ubi/ubi.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)
drivers/mtd/ubi/block.c +3 -3
@@ -456,7 +456,7 @@ static void ubiblock_cleanup(struct ubiblock *dev)idr_remove (&ubiblock_minor_idr, dev->gd->first_minor); } -int ubiblock_remove (struct ubi_volume_info *vi) +int ubiblock_remove (struct ubi_volume_info *vi,bool force) {struct ubiblock *dev;int ret;
@@ -470,7 +470,7 @@ int ubiblock_remove(struct ubi_volume_info *vi)/* Found a device, let's lock it so we can check if it's busy */ mutex_lock (&dev->dev_mutex); -if (dev->refcnt >0 ) { +if (dev->refcnt >0 && !force) { ret = -EBUSY ; goto out_unlock_dev; }
@@ -545,7 +545,7 @@ static int ubiblock_notify(struct notifier_block *nb,*/ break ;case UBI_VOLUME_REMOVED : -ubiblock_remove (&nt->vi); +ubiblock_remove (&nt->vi, true);break ;case UBI_VOLUME_RESIZED :ubiblock_resize (&nt->vi);
drivers/mtd/ubi/cdev.c +1 -1
@@ -572,7 +572,7 @@ static long vol_cdev_ioctl(struct file *file, unsigned int cmd,struct ubi_volume_info vi;ubi_get_volume_info (desc, &vi); - err =ubiblock_remove (&vi); + err =ubiblock_remove (&vi, false);break ; }
drivers/mtd/ubi/ubi.h +2 -2
@@ -979,7 +979,7 @@ static inline void ubi_fastmap_destroy_checkmap(struct ubi_volume *vol) {}int ubiblock_init (void );void ubiblock_exit (void );int ubiblock_create (struct ubi_volume_info *vi); -int ubiblock_remove (struct ubi_volume_info *vi); +int ubiblock_remove (struct ubi_volume_info *vi,bool force);#else static inline int ubiblock_init (void ) {return 0 ; }static inline void ubiblock_exit (void ) {}
@@ -987,7 +987,7 @@ static inline int ubiblock_create(struct ubi_volume_info *vi) {return -ENOSYS ; } -static inline int ubiblock_remove (struct ubi_volume_info *vi) +static inline int ubiblock_remove (struct ubi_volume_info *vi,bool force) {return -ENOSYS ; }
Signature · 1 line
2.41.0
PATCH v4 4/8 mtd: ubi: block: use notifier to create ubiblock
In reply to Daniel GolleUse UBI_VOLUME_ADDED notification to create ubiblock device specified on kernel cmdline or module parameter.
This makes thing more simple and has the advantage that ubiblock devices on volumes which are not present at the time the ubi module is probed will still be created.
--- drivers/mtd/ubi/block.c | 152 ++++++++++++++++++++++------------------ 1 file changed, 84 insertions(+), 68 deletions(-)
drivers/mtd/ubi/block.c +84 -68
@@ -33,6 +33,7 @@#include <linux/kernel.h> #include <linux/list.h> #include <linux/mutex.h> +#include <linux/namei.h> #include <linux/slab.h> #include <linux/mtd/ubi.h> #include <linux/blkdev.h>
@@ -65,10 +66,10 @@ struct ubiblock_pdu { };/* Numbers of elements set in the @ubiblock_param array */ -static int ubiblock_devs__initdata ; +static int ubiblock_devs;/* MTD devices specification parameters */ -static struct ubiblock_param ubiblock_param[UBIBLOCK_MAX_DEVICES ]__initdata ; +static struct ubiblock_param ubiblock_param[UBIBLOCK_MAX_DEVICES ];struct ubiblock {struct ubi_volume_desc *desc;
@@ -532,6 +533,85 @@ static int ubiblock_resize(struct ubi_volume_info *vi)return 0 ; } +static bool +match_volume_desc (struct ubi_volume_info *vi,const char *name,int ubi_num,int vol_id) +{ +int err, len; +struct path path; +struct kstat stat; + +if (ubi_num ==-1 ) { +/* No ubi num, name must be a vol device path */ + err =kern_path (name,LOOKUP_FOLLOW , &path); +if (err) +return false; + + err =vfs_getattr (&path, &stat,STATX_TYPE ,AT_STATX_SYNC_AS_STAT ); +path_put (&path); +if (err) +return false; + +if (!S_ISCHR (stat.mode)) +return false; + +if (vi->ubi_num !=ubi_major2num (MAJOR (stat.rdev))) +return false; + +if (vi->vol_id !=MINOR (stat.rdev) -1 ) +return false; + +return true; + } + +if (vol_id ==-1 ) { +if (vi->ubi_num != ubi_num) +return false; + + len =strnlen (name,UBI_VOL_NAME_MAX +1 ); +if (len <1 || vi->name_len != len) +return false; + +if (strcmp (name, vi->name)) +return false; + +return true; + } + +if (vi->ubi_num != ubi_num) +return false; + +if (vi->vol_id != vol_id) +return false; + +return true; +} + +static void +ubiblock_create_from_param (struct ubi_volume_info *vi) +{ +int i, ret =0 ; +struct ubiblock_param *p; + +/* +* Iterate over ubiblock cmdline parameters. If a parameter matches the +* newly added volume create the ubiblock device for it. +*/ +for (i =0 ; i < ubiblock_devs; i++) { + p = &ubiblock_param[i]; + +if (!match_volume_desc (vi, p->name, p->ubi_num, p->vol_id)) +continue ; + + ret =ubiblock_create (vi); +if (ret) { +pr_err ( +"UBI: block: can't add '%s' volume on ubi%d_%d, err=%d\n" , + vi->name, p->ubi_num, p->vol_id, ret); + } +break ; + } +} +static int ubiblock_notify (struct notifier_block *nb,unsigned long notification_type,void *ns_ptr) {
@@ -539,10 +619,7 @@ static int ubiblock_notify(struct notifier_block *nb,switch (notification_type) {case UBI_VOLUME_ADDED : -/* -* We want to enforce explicit block device creation for -* volumes, so when a volume is added we do nothing. -*/ +ubiblock_create_from_param (&nt->vi);break ;case UBI_VOLUME_REMOVED :ubiblock_remove (&nt->vi, true);
@@ -568,56 +645,6 @@ static struct notifier_block ubiblock_notifier = { .notifier_call = ubiblock_notify, }; -static struct ubi_volume_desc *__init -open_volume_desc (const char *name,int ubi_num,int vol_id) -{ -if (ubi_num ==-1 ) -/* No ubi num, name must be a vol device path */ -return ubi_open_volume_path (name,UBI_READONLY ); -else if (vol_id ==-1 ) -/* No vol_id, must be vol_name */ -return ubi_open_volume_nm (ubi_num, name,UBI_READONLY ); -else -return ubi_open_volume (ubi_num, vol_id,UBI_READONLY ); -} - -static void __init ubiblock_create_from_param (void ) -{ -int i, ret =0 ; -struct ubiblock_param *p; -struct ubi_volume_desc *desc; -struct ubi_volume_info vi; - -/* -* If there is an error creating one of the ubiblocks, continue on to -* create the following ubiblocks. This helps in a circumstance where -* the kernel command-line specifies multiple block devices and some -* may be broken, but we still want the working ones to come up. -*/ -for (i =0 ; i < ubiblock_devs; i++) { - p = &ubiblock_param[i]; - - desc =open_volume_desc (p->name, p->ubi_num, p->vol_id); -if (IS_ERR (desc)) { -pr_err ( -"UBI: block: can't open volume on ubi%d_%d, err=%ld\n" , - p->ubi_num, p->vol_id,PTR_ERR (desc)); -continue ; - } - -ubi_get_volume_info (desc, &vi); -ubi_close_volume (desc); - - ret =ubiblock_create (&vi); -if (ret) { -pr_err ( -"UBI: block: can't add '%s' volume on ubi%d_%d, err=%d\n" , - vi.name, p->ubi_num, p->vol_id, ret); -continue ; - } - } -} -static void ubiblock_remove_all (void ) {struct ubiblock *next;
@@ -643,18 +670,7 @@ int __init ubiblock_init(void)if (ubiblock_major <0 )return ubiblock_major; -/* -* Attach block devices from 'block=' module param. -* Even if one block device in the param list fails to come up, -* still allow the module to load and leave any others up. -*/ -ubiblock_create_from_param (); - -/* -* Block devices are only created upon user requests, so we ignore -* existing volumes. -*/ - ret =ubi_register_volume_notifier (&ubiblock_notifier,1 ); + ret =ubi_register_volume_notifier (&ubiblock_notifier,0 );if (ret) goto err_unreg;return 0 ;
Signature · 1 line
2.41.0
PATCH v4 5/8 mtd: ubi: attach MTD partition from device-tree
In reply to Daniel GolleSplit ubi_init() function into early function to be called by device_initcall() and keep cmdline attachment in late_initcall(). (when building ubi as module, both is still done in a single module_init() call)
Register MTD notifier and attach MTD devices which are marked as compatible with 'linux,ubi' in OF device-tree when being added, detach UBI device from MTD device when it is being removed.
For existing users this should not change anything besides automatic removal of (dead) UBI devices when their underlying MTD devices are already gone, e.g. in case of MTD driver module or (SPI) bus driver module being removed.
For new users this opens up the option to attach UBI using device-tree which then happens early and in parallel with other drivers being probed which slightly reduces the total boot time.
Attachment no longer happening late is also a requirement for other drivers to make use of UBI, e.g. drivers/nvmem/u-boot-env.c can now be extended to support U-Boot environment stored in UBI volumes.
--- drivers/mtd/ubi/block.c | 2 +- drivers/mtd/ubi/build.c | 153 +++++++++++++++++++++++++++++----------- drivers/mtd/ubi/cdev.c | 2 +- drivers/mtd/ubi/ubi.h | 2 +- 4 files changed, 115 insertions(+), 44 deletions(-)
drivers/mtd/ubi/block.c +1 -1
@@ -470,7 +470,7 @@ int ubiblock_remove(struct ubi_volume_info *vi, bool force) }/* Found a device, let's lock it so we can check if it's busy */ -mutex_lock (&dev->dev_mutex); +mutex_lock_nested (&dev->dev_mutex,SINGLE_DEPTH_NESTING );if (dev->refcnt >0 && !force) { ret = -EBUSY ; goto out_unlock_dev;
drivers/mtd/ubi/build.c +112 -41
230 lines not on this page · raw message
drivers/mtd/ubi/cdev.c +1 -1
@@ -1065,7 +1065,7 @@ static long ctrl_cdev_ioctl(struct file *file, unsigned int cmd, }mutex_lock (&ubi_devices_mutex); - err =ubi_detach_mtd_dev (ubi_num,0 ); + err =ubi_detach_mtd_dev (ubi_num,0 , false);mutex_unlock (&ubi_devices_mutex);break ; }
drivers/mtd/ubi/ubi.h +1 -1
@@ -939,7 +939,7 @@ int ubi_io_write_vid_hdr(struct ubi_device *ubi, int pnum,int ubi_attach_mtd_dev (struct mtd_info *mtd,int ubi_num,int vid_hdr_offset,int max_beb_per1024,bool disable_fm); -int ubi_detach_mtd_dev (int ubi_num,int anyway); +int ubi_detach_mtd_dev (int ubi_num,int anyway,bool have_lock);struct ubi_device *ubi_get_device (int ubi_num);void ubi_put_device (struct ubi_device *ubi);struct ubi_device *ubi_get_by_major (int major);
Signature · 1 line
2.41.0
PATCH v4 6/8 mtd: ubi: introduce pre-removal notification for UBI volumes
In reply to Daniel GolleIntroduce a new notification type UBI_VOLUME_SHUTDOWN to inform users that a volume is just about to be removed.
This is needed because users (such as the NVMEM subsystem) expect that at the time their removal function is called, the parenting device is still available (for removal of sysfs nodes, for example, in case of NVMEM which otherwise WARNs on volume removal).
--- drivers/mtd/ubi/block.c | 26 ++++++++++++++++++++++++++ drivers/mtd/ubi/build.c | 7 ++++++- drivers/mtd/ubi/vmt.c | 5 +++++ include/linux/mtd/ubi.h | 2 ++ 4 files changed, 39 insertions(+), 1 deletion(-)
drivers/mtd/ubi/block.c +26 -0
@@ -533,6 +533,29 @@ static int ubiblock_resize(struct ubi_volume_info *vi)return 0 ; } +static int ubiblock_shutdown (struct ubi_volume_info *vi) +{ +struct ubiblock *dev; +struct gendisk *disk; +int ret =0 ; + +mutex_lock (&devices_mutex); + dev =find_dev_nolock (vi->ubi_num, vi->vol_id); +if (!dev) { + ret = -ENODEV ; + goto out_unlock; + } + disk = dev->gd; + +out_unlock: +mutex_unlock (&devices_mutex); + +if (!ret) +blk_mark_disk_dead (disk); + +return ret; +}; +static bool match_volume_desc (struct ubi_volume_info *vi,const char *name,int ubi_num,int vol_id) {
@@ -624,6 +647,9 @@ static int ubiblock_notify(struct notifier_block *nb,case UBI_VOLUME_REMOVED :ubiblock_remove (&nt->vi, true);break ; +case UBI_VOLUME_SHUTDOWN : +ubiblock_shutdown (&nt->vi); +break ;case UBI_VOLUME_RESIZED :ubiblock_resize (&nt->vi);break ;
drivers/mtd/ubi/build.c +6 -1
@@ -1088,7 +1088,6 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway, bool have_lock)return -EINVAL ;spin_lock (&ubi_devices_lock); -put_device (&ubi->dev); ubi->ref_count -=1 ;if (ubi->ref_count) {if (!anyway) {
@@ -1099,6 +1098,12 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway, bool have_lock)ubi_err (ubi,"%s reference count %d, destroy anyway" , ubi->ubi_name, ubi->ref_count); } +spin_unlock (&ubi_devices_lock); + +ubi_notify_all (ubi,UBI_VOLUME_SHUTDOWN ,NULL ); + +spin_lock (&ubi_devices_lock); +put_device (&ubi->dev); ubi_devices[ubi_num] =NULL ;spin_unlock (&ubi_devices_lock);
drivers/mtd/ubi/vmt.c +5 -0
@@ -352,6 +352,11 @@ int ubi_remove_volume(struct ubi_volume_desc *desc, int no_vtbl) err = -EBUSY ; goto out_unlock; } +spin_unlock (&ubi->volumes_lock); + +ubi_volume_notify (ubi, vol,UBI_VOLUME_SHUTDOWN ); + +spin_lock (&ubi->volumes_lock); ubi->volumes[vol_id] =NULL ;spin_unlock (&ubi->volumes_lock);
include/linux/mtd/ubi.h +2 -0
@@ -192,6 +192,7 @@ struct ubi_device_info {* or a volume was removed) * @UBI_VOLUME_RESIZED: a volume has been re-sized * @UBI_VOLUME_RENAMED: a volume has been re-named +* @UBI_VOLUME_SHUTDOWN: a volume is going to removed, shutdown users * @UBI_VOLUME_UPDATED: data has been written to a volume * * These constants define which type of event has happened when a volume
@@ -202,6 +203,7 @@ enum {UBI_VOLUME_REMOVED ,UBI_VOLUME_RESIZED ,UBI_VOLUME_RENAMED , +UBI_VOLUME_SHUTDOWN , UBI_VOLUME_UPDATED, };
Signature · 1 line
2.41.0
PATCH v4 7/8 mtd: ubi: populate ubi volume fwnode
In reply to Daniel GolleLook for the 'volumes' subnode of an MTD partition attached to a UBI device and attach matching child nodes to UBI volumes. This allows UBI volumes to be referenced in device tree, e.g. for use as NVMEM providers.
--- drivers/mtd/ubi/vmt.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
drivers/mtd/ubi/vmt.c +27 -0
@@ -124,6 +124,31 @@ static void vol_release(struct device *dev)kfree (vol); } +static struct fwnode_handle *find_volume_fwnode (struct ubi_volume *vol) +{ +struct fwnode_handle *fw_vols, *fw_vol; +const char *volname; +u32 volid; + + fw_vols =device_get_named_child_node (vol->dev.parent->parent,"volumes" ); +if (!fw_vols) +return NULL ; + +fwnode_for_each_child_node (fw_vols, fw_vol) { +if (!fwnode_property_read_string (fw_vol,"volume-name" , &volname) && +strncmp (volname, vol->name, vol->name_len)) +continue ; + +if (!fwnode_property_read_u32 (fw_vol,"volume-id" , &volid) && + vol->vol_id != volid) +continue ; + +return fw_vol; + } + +return NULL ; +} +/** * ubi_create_volume - create volume. * @ubi: UBI device description object
@@ -223,6 +248,7 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req) vol->name_len = req->name_len;memcpy (vol->name, req->name, vol->name_len); vol->ubi = ubi; +device_set_node (&vol->dev,find_volume_fwnode (vol));/* * Finish all pending erases because there may be some LEBs belonging
@@ -597,6 +623,7 @@ int ubi_add_volume(struct ubi_device *ubi, struct ubi_volume *vol) vol->dev.class = &ubi_class; vol->dev.groups = volume_dev_groups;dev_set_name (&vol->dev,"%s_%d" , ubi->ubi_name, vol->vol_id); +device_set_node (&vol->dev,find_volume_fwnode (vol)); err =device_register (&vol->dev);if (err) {cdev_del (&vol->cdev);
Signature · 1 line
2.41.0
PATCH v4 8/8 mtd: ubi: provide NVMEM layer over UBI volumes
In reply to Daniel GolleIn an ideal world we would like UBI to be used where ever possible on a NAND chip. And with UBI support in ARM Trusted Firmware and U-Boot it is possible to achieve an (almost-)all-UBI flash layout. Hence the need for a way to also use UBI volumes to store board-level constants, such as MAC addresses and calibration data of wireless interfaces.
Add UBI volume NVMEM driver module exposing UBI volumes as NVMEM providers. Allow UBI devices to have a "volumes" firmware subnode with volumes which may be compatible with "nvmem-cells".
Access to UBI volumes via the NVMEM interface at this point is read-only, and it is slow, opening and closing the UBI volume for each access due to limitations of the NVMEM provider API.
--- drivers/mtd/ubi/Kconfig | 12 +++ drivers/mtd/ubi/Makefile | 1 + drivers/mtd/ubi/nvmem.c | 189 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+) create mode 100644 drivers/mtd/ubi/nvmem.c
drivers/mtd/ubi/Kconfig +12 -0
@@ -104,4 +104,16 @@ config MTD_UBI_BLOCK If in doubt, say "N". +config MTD_UBI_NVMEM + tristate "UBI virtual NVMEM" + default n + depends on NVMEM + help + This option enabled an additional driver exposing UBI volumes as NVMEM + providers, intended for platforms where UBI is part of the firmware + specification and used to store also e.g. MAC addresses or board- + specific Wi-Fi calibration data. + + If in doubt, say "N". + endif # MTD_UBI
drivers/mtd/ubi/Makefile +1 -0
@@ -7,3 +7,4 @@ ubi-$(CONFIG_MTD_UBI_FASTMAP) += fastmap.o ubi-$(CONFIG_MTD_UBI_BLOCK) += block.o obj-$(CONFIG_MTD_UBI_GLUEBI) += gluebi.o +obj-$(CONFIG_MTD_UBI_NVMEM) += nvmem.o
drivers/mtd/ubi/nvmem.c new file +189 -0
189 lines not on this page · raw message
Signature · 5 lines
2.41.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [PATCH v4 1/8] dt-bindings: mtd: add basic bindings for UBI
In reply to Daniel GolleQuoted 10 lines · Daniel Golle
Add basic bindings for UBI devices and volumes.
Signed-off-by Daniel Golle <[email protected]>--- .../bindings/mtd/partitions/linux,ubi.yaml | 65 +++++++++++++++++++ .../bindings/mtd/partitions/ubi-volume.yaml | 36 ++++++++++ 2 files changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/linux,ubi.yaml create mode 100644 Documentation/devicetree/bindings/mtd/partitions/ubi-volume.yaml
Re: [PATCH v4 2/8] dt-bindings: mtd: nvmem-cells: add support for UBI volumes
In reply to Daniel GolleQuoted 9 lines · Daniel Golle
UBI volumes may be used to contain NVMEM bits, typically device MAC addresses or wireless radio calibration data.
Signed-off-by Daniel Golle <[email protected]>--- .../devicetree/bindings/mtd/partitions/linux,ubi.yaml | 3 ++- .../devicetree/bindings/mtd/partitions/nvmem-cells.yaml | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-)
Re: [PATCH v4 1/8] dt-bindings: mtd: add basic bindings for UBI
In reply to Rob HerringQuoted 5 lines · Rob Herring
Quoted 2 lines · Daniel Golle
Add basic bindings for UBI devices and volumes.
Reviewed-by Rob Herring <[email protected]>
Patch 3 - 7 have previously been discussed on the mailing list, patch 1 and 2 have been reviewed by Rob, patch 8 ("mtd: ubi: provide NVMEM layer over UBI volumes") still needs review.
Is there anything I can do from my end to have this series moving forward?
Patchwork: https://patchwork.ozlabs.org/project/linux-mtd/list/?series=368347
Thank you for your advise!
Daniel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Richard Weinberger
maintainerRe: [PATCH v4 1/8] dt-bindings: mtd: add basic bindings for UBI
----- Ursprüngliche Mail -----
Quoted 16 lines
Von: "Daniel Golle" <[email protected]>
An: "linux-mtd" <[email protected]>
CC: "chengzhihao1" <[email protected]>, "robh" <[email protected]>, "Vignesh Raghavendra" <[email protected]>, "Randy Dunlap" <[email protected]>, "Miquel Raynal" <[email protected]>, "richard" <[email protected]>, "devicetree" <[email protected]>, "Krzysztof Kozlowski" <[email protected]>, "Conor Dooley" <[email protected]>, "Rob Herring" <[email protected]>, "linux-kernel" <[email protected]>
Gesendet: Montag, 11. September 2023 21:01:57
Betreff: Re: [PATCH v4 1/8] dt-bindings: mtd: add basic bindings for UBIQuoted 5 lines · Rob Herring
Quoted 2 lines · Daniel Golle
Add basic bindings for UBI devices and volumes.
Reviewed-by Rob Herring <[email protected]>
Quoted 8 lines
Patch 3 - 7 have previously been discussed on the mailing list, patch 1 and 2 have been reviewed by Rob, patch 8 ("mtd: ubi: provide NVMEM layer over UBI volumes") still needs review.
Is there anything I can do from my end to have this series moving forward?
Patchwork: https://patchwork.ozlabs.org/project/linux-mtd/list/?series=368347
It is on my review TODO.
Thanks for your patience,
//richard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [PATCH v4 1/8] dt-bindings: mtd: add basic bindings for UBI
In reply to Daniel GolleHi!
Quoted 1 line · Daniel Golle
Is there anything I can do from my end to have this series moving forward?
You could review other UBI submissions to reduce the review load on Richard. I guess the queue of UBI pending patches is at:
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=&submitter=&state=&q=ubi&archive=&delegate=
Cheers,
ta
Richard Weinberger
maintainerRe: [PATCH v4 3/8] mtd: ubi: block: don't return on error when removing
In reply to Daniel Golle----- Ursprüngliche Mail -----
Quoted 38 lines
Von: "Daniel Golle" <[email protected]>
An: "Randy Dunlap" <[email protected]>, "Miquel Raynal" <[email protected]>, "richard" <[email protected]>, "Vignesh Raghavendra" <[email protected]>, "Rob Herring" <[email protected]>, "Krzysztof Kozlowski" <[email protected]>, "Conor Dooley" <[email protected]>, "Daniel Golle" <[email protected]>, "linux-mtd" <[email protected]>, "devicetree" <[email protected]>, "linux-kernel" <[email protected]>
Gesendet: Freitag, 11. August 2023 03:37:12
Betreff: [PATCH v4 3/8] mtd: ubi: block: don't return on error when removingThere is no point on returning the error from
ubiblock_removein case it is being called due to a volume removal event -- the volume is gone, we should destroy and remove the ubiblock device no matter what.Introduce new boolean parameter 'force' to tell
ubiblock_removeto go on even in case the ubiblock device is still busy. Use that new option when callingubiblock_removedue to aUBI_VOLUME_REMOVEDevent.Signed-off-by Daniel Golle <[email protected]>---
drivers/mtd/ubi/block.c | 6 +++--- drivers/mtd/ubi/cdev.c | 2 +- drivers/mtd/ubi/ubi.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)drivers/mtd/ubi/block.c +1
-1@@ -456,7 +456,7 @@ static void ubiblock_cleanup(struct ubiblock *dev)idr_remove (&ubiblock_minor_idr, dev->gd->first_minor); } -int ubiblock_remove (struct ubi_volume_info *vi) +int ubiblock_remove (struct ubi_volume_info *vi,bool force) {struct ubiblock *dev;int ret;
drivers/mtd/ubi/block.c int ubiblock_remove(struct ubi_volume_info *vi) · line 475
Quoted 7 lines
drivers/mtd/ubi/block.c +1
-1@@ -470,7 +470,7 @@ int ubiblock_remove(struct ubi_volume_info *vi)/* Found a device, let's lock it so we can check if it's busy */ mutex_lock (&dev->dev_mutex); -if (dev->refcnt >0 ) { +if (dev->refcnt >0 && !force) { ret = -EBUSY ; goto out_unlock_dev;
Is it really safe to destroy the blk queue (via ubiblock_cleanup()) if refcnt is > 0?
Thanks,
//richard
Richard Weinberger
maintainerRe: [PATCH v4 4/8] mtd: ubi: block: use notifier to create ubiblock
In reply to Daniel Golle----- Ursprüngliche Mail -----
Quoted 9 lines
Von: "Daniel Golle" <[email protected]>
An: "Randy Dunlap" <[email protected]>, "Miquel Raynal" <[email protected]>, "richard" <[email protected]>, "Vignesh Raghavendra" <[email protected]>, "Rob Herring" <[email protected]>, "Krzysztof Kozlowski" <[email protected]>, "Conor Dooley" <[email protected]>, "Daniel Golle" <[email protected]>, "linux-mtd" <[email protected]>, "devicetree" <[email protected]>, "linux-kernel" <[email protected]>
Gesendet: Freitag, 11. August 2023 03:37:31
Betreff: [PATCH v4 4/8] mtd: ubi: block: use notifier to create ubiblock
Quoted 3 lines
Use
UBI_VOLUME_ADDEDnotification to create ubiblock device specified on kernel cmdline or module parameter.
This makes thing more simple and has the advantage that ubiblock devices
*things
Quoted 56 lines
on volumes which are not present at the time the ubi module is probed will still be created.
Suggested-by Zhihao Cheng <[email protected]>Signed-off-by Daniel Golle <[email protected]>---
drivers/mtd/ubi/block.c| 152 ++++++++++++++++++++++------------------1 file changed, 84 insertions(+), 68 deletions(-)drivers/mtd/ubi/block.c +20
-2@@ -33,6 +33,7 @@#include <linux/kernel.h> #include <linux/list.h> #include <linux/mutex.h> +#include <linux/namei.h> #include <linux/slab.h> #include <linux/mtd/ubi.h> #include <linux/blkdev.h> @@ -65,10 +66,10 @@ struct ubiblock_pdu { };/* Numbers of elements set in the @ubiblock_param array */ -static int ubiblock_devs__initdata ; +static int ubiblock_devs;/* MTD devices specification parameters */ -static struct ubiblock_param ubiblock_param[UBIBLOCK_MAX_DEVICES ]__initdata ; +static struct ubiblock_param ubiblock_param[UBIBLOCK_MAX_DEVICES ];struct ubiblock {struct ubi_volume_desc *desc;@@ -532,6 +533,85 @@ static int ubiblock_resize(struct ubi_volume_info *vi)return 0 ; } +static bool +match_volume_desc (struct ubi_volume_info *vi,const char *name,int ubi_num,int vol_id) +{ +int err, len; +struct path path; +struct kstat stat; + +if (ubi_num ==-1 ) { +/* No ubi num, name must be a vol device path */ + err =kern_path (name,LOOKUP_FOLLOW , &path); +if (err) +return false; + + err =vfs_getattr (&path, &stat,STATX_TYPE ,AT_STATX_SYNC_AS_STAT ); +path_put (&path); +if (err) +return false;
drivers/mtd/ubi/block.c static int ubiblock_resize(struct ubi_volume_info *vi) · line 563
Quoted 10 lines
drivers/mtd/ubi/block.c +10
-0+ +if (!S_ISCHR (stat.mode)) +return false; + +if (vi->ubi_num !=ubi_major2num (MAJOR (stat.rdev))) +return false; + +if (vi->vol_id !=MINOR (stat.rdev) -1 ) +return false; +
This is more or less an open coded ubi_open_volume_path(). Please either split or adopt ubi_open_volume_path() to fit your use case.
Quoted 6 lines
drivers/mtd/ubi/block.c +6
-0+return true; + } + +if (vol_id ==-1 ) { +if (vi->ubi_num != ubi_num) +return false;
drivers/mtd/ubi/block.c static int ubiblock_resize(struct ubi_volume_info *vi) · line 578
Quoted 10 lines
drivers/mtd/ubi/block.c +10
-0+ + len =strnlen (name,UBI_VOL_NAME_MAX +1 ); +if (len <1 || vi->name_len != len) +return false; + +if (strcmp (name, vi->name)) +return false; + +return true; + }
Same for ubi_open_volume_nm().
Thanks,
//richard
Richard Weinberger
maintainerRe: [PATCH v4 5/8] mtd: ubi: attach MTD partition from device-tree
In reply to Daniel Golle----- Ursprüngliche Mail -----
drivers/mtd/ubi/block.c int ubiblock_remove(struct ubi_volume_info *vi, bool force) · line 473
Quoted 10 lines
drivers/mtd/ubi/block.c +1
-1@@ -470,7 +470,7 @@ int ubiblock_remove(struct ubi_volume_info *vi, bool force) }/* Found a device, let's lock it so we can check if it's busy */ -mutex_lock (&dev->dev_mutex); +mutex_lock_nested (&dev->dev_mutex,SINGLE_DEPTH_NESTING );
The usage of mutex_lock_nested() in this patch looks fishy.
Can you please elaborate a bit more why all these mutexes can be taken twice? (Any why not more often).
Thanks,
//richard
Richard Weinberger
maintainerRe: [PATCH v4 5/8] mtd: ubi: attach MTD partition from device-tree
----- Ursprüngliche Mail -----
Quoted 9 lines
Von: "richard" <[email protected]>
----- Ursprüngliche Mail -----Quoted 7 lines
drivers/mtd/ubi/block.c +0
-0@@ -470,7 +470,7 @@ int ubiblock_remove(struct ubi_volume_info *vi, bool force) }
Quoted 7 lines
drivers/mtd/ubi/block.c int ubiblock_remove(struct ubi_volume_info *vi, bool force) · line 473
Quoted 3 lines
drivers/mtd/ubi/block.c +1
-1/* Found a device, let's lock it so we can check if it's busy */ -mutex_lock (&dev->dev_mutex); +mutex_lock_nested (&dev->dev_mutex,SINGLE_DEPTH_NESTING );The usage of
mutex_lock_nested()in this patch looks fishy.
Can you please elaborate a bit more why all these mutexes can be taken twice? (Any why not more often).
I think I figured myself.ubiblock_ops->open() and ->release() are both called with disk->open_mutex held. ubiblock_open() and ubiblock_release() take dev->dev_mutex.
So, the locking order is open_mutex, followed by dev_mutex.
On the other hand, ubiblock_remove() is called via UBI notify.
It takes first dev_mutex and then calls del_gendisk() which will trigger ubiblock_ops->release() under disk->open_mutex but takes dev_mutex again.
So, we this not only takes a lock twice but also in reverse order. mutex_lock_nested() might silence lockdep but I'm not sure whether this is safe at all.
Thanks,
//richard
Re: [PATCH v4 5/8] mtd: ubi: attach MTD partition from device-tree
Hi Richard,
thank you for the review and sorry for my late reply to it.
Quoted 23 lines · Richard Weinberger
----- Ursprüngliche Mail -----
Quoted 9 lines
Von: "richard" <[email protected]>
----- Ursprüngliche Mail -----Quoted 7 lines
drivers/mtd/ubi/block.c +0
-0@@ -470,7 +470,7 @@ int ubiblock_remove(struct ubi_volume_info *vi, bool force) }Quoted 7 lines
drivers/mtd/ubi/block.c int ubiblock_remove(struct ubi_volume_info *vi, bool force) · line 473
Quoted 3 lines
drivers/mtd/ubi/block.c +1
-1/* Found a device, let's lock it so we can check if it's busy */ -mutex_lock (&dev->dev_mutex); +mutex_lock_nested (&dev->dev_mutex,SINGLE_DEPTH_NESTING );The usage of
mutex_lock_nested()in this patch looks fishy.
Can you please elaborate a bit more why all these mutexes can be taken twice? (Any why not more often).I think I figured myself.
ubiblock_ops->open()and ->release() are both called withdisk->open_mutexheld.ubiblock_open()andubiblock_release()takedev->dev_mutex.
So, the locking order isopen_mutex, followed bydev_mutex.
Quoted 5 lines
On the other hand,
ubiblock_remove()is called via UBI notify.
It takes firstdev_mutexand then callsdel_gendisk()which will triggerubiblock_ops->release()underdisk->open_mutexbut takesdev_mutexagain.
So, we this not only takes a lock twice but also in reverse order.mutex_lock_nested()might silence lockdep but I'm not sure whether this is safe at all.
I agree that the locking here is not trivial.
Taking the same lock twice is that SINGLE_DEPTH_NESTING is intended for and resolves running into a deadlock otherwise.
I'm not sure if the changed order of acquiring dev_mutex and open_mutex present a problem.
The detach-by-notification case is anyway quite exotic and I only see quite synthetic paths to actually get there (such as hotplug removal of the SPI controller providing access to SPI-NAND flash, which is how I was testing this...). To make things easier and more aligned with the original goal (being to attach a UBI device from DT, early enough to allow its use as NVMEM provider by other drivers) I suggest to for now only handle attachment by MTD notification (if linux,ubi compatible is present in DT) and keep handling detachment the way it is now.
This will (just like it is already now) result in "zombie" UBI devices being present after their MTD device has already been removed. Accessing them creates loud kernel warnings at best.
Let me know if keeing detachment the way it is and only implementing attaching from MTD notification and device tree compatible would be ok with you.
Thank you!
Best regards
Daniel