| Message ID | 1466277476-14853-1-git-send-email-hauke@hauke-m.de |
|---|---|
| State | Rejected |
| Headers | show |
Am 18.06.2016 um 21:17 schrieb Hauke Mehrtens: > This makes it possible to open a ubi layer in device tree, this is > helpful when the rootfs is on a ubi layer. It loops though all mtd > partitions and mounts the partition which is compatible with > "ubi,volume". The same was only possible with kernel command line > arguments before. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ > drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ > 2 files changed, 96 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt Some time ago I thought about an UBI DT binding too, but I have been told that device tree is only for describing the hardware and nothing else. So I fear this will be rejected by DT folks. Thanks, //richard
On 06/18/2016 09:30 PM, Richard Weinberger wrote: > Am 18.06.2016 um 21:17 schrieb Hauke Mehrtens: >> This makes it possible to open a ubi layer in device tree, this is >> helpful when the rootfs is on a ubi layer. It loops though all mtd >> partitions and mounts the partition which is compatible with >> "ubi,volume". The same was only possible with kernel command line >> arguments before. >> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> --- >> Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ >> drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ >> 2 files changed, 96 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt > > Some time ago I thought about an UBI DT binding too, but > I have been told that device tree is only for describing the hardware > and nothing else. > So I fear this will be rejected by DT folks. There are some devices on the market that are storing the root file system in an ubi layer. To make this work there are currently two options. 1. patch the kernel to specify which partitions to open in the code. 2. provide some kernel command line parameter which specifies what to open. I would like to specify this in device tree. Hauke
Am 18.06.2016 um 21:35 schrieb Hauke Mehrtens: > There are some devices on the market that are storing the root file > system in an ubi layer. > To make this work there are currently two options. > 1. patch the kernel to specify which partitions to open in the code. > 2. provide some kernel command line parameter which specifies what to open. > > I would like to specify this in device tree. What is wrong with command line parameters? If there is some parameter the UBI module misses I'll happily add it. You can also put your kernel command line into device tree. Thanks, //richard
On 06/18/2016 09:46 PM, Richard Weinberger wrote: > Am 18.06.2016 um 21:35 schrieb Hauke Mehrtens: >> There are some devices on the market that are storing the root file >> system in an ubi layer. >> To make this work there are currently two options. >> 1. patch the kernel to specify which partitions to open in the code. >> 2. provide some kernel command line parameter which specifies what to open. >> >> I would like to specify this in device tree. > > What is wrong with command line parameters? If there is some parameter > the UBI module misses I'll happily add it. > You can also put your kernel command line into device tree. I would like to configure this in device tree. When I install a third party firmware on a router normally the kernel command line is taken from the boot loader and the device tree is appended to the kernel. Hauke
Hi! On Sat, Jun 18, 2016 at 09:35:42PM +0200, Hauke Mehrtens wrote: > On 06/18/2016 09:30 PM, Richard Weinberger wrote: > > Am 18.06.2016 um 21:17 schrieb Hauke Mehrtens: > >> This makes it possible to open a ubi layer in device tree, this is > >> helpful when the rootfs is on a ubi layer. It loops though all mtd > >> partitions and mounts the partition which is compatible with > >> "ubi,volume". The same was only possible with kernel command line > >> arguments before. > >> > >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > >> --- > >> Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ > >> drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ > >> 2 files changed, 96 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt > > > > Some time ago I thought about an UBI DT binding too, but > > I have been told that device tree is only for describing the hardware > > and nothing else. > > So I fear this will be rejected by DT folks. > > There are some devices on the market that are storing the root file > system in an ubi layer. > To make this work there are currently two options. > 1. patch the kernel to specify which partitions to open in the code. > 2. provide some kernel command line parameter which specifies what to open. > > I would like to specify this in device tree. In MBR there used to be an 'active' flag stored for each partition. Maybe it'd be nice to introduce something similar to mark UBI volumes to be the default rootfs. Currently we solve this issue by convention: If a volume is named 'rootfs' it is automatically mounted during boot. Depending on the filesystem in use a ubiblock device has to be created as well. This is mostly just the continuation of the existing naming convention of mtd partitions, a patch OpenWrt is carrying around for a long while already. To support the same on UBI, another set of patches was made. I agree that there should be a way to pass this through the of_node of the mtd partition which is defined in the device tree. Selecting to-be-ubi-attached mtd partitions in device-tree would replace patch: https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/490-ubi-auto-attach-mtd-device-named-ubi-or-data-on-boot.patch To auto-select the rootfs, we currently carry another bunch of patches https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch This is more or less filesystem-agnostic and works fine as long as there is only one volume called 'rootfs' and this volume is always used as rootfs. Dual-boot setups will need some way to pass the active rootfs volume to the kernel. While I agree that this is possible by appending or prepending to the cmdline string passed to the kernel, this either limits the users' ability to manually specify the rootfs using the cmdline or becomes a more complex task to only append/prepend the cmdline in case the user-defined string doesn't already contain relevant parameters... Thus it'd be nicer to flag the default rootfs volume via the device- tree. However, I realize that other than mtd partitions, ubi volumes are obviously dynamic and thus don't occur in the device-tree... Modifying the volume on every boot to select whether it is active also doesn't seem to be the right way. A non-standard cmdline parameter is the most common solution implemented by many vendors, that's also far from beautiful. Just my 2 cents... Cheers Daniel > > Hauke > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi! I got some remarks here: On Sat, Jun 18, 2016 at 09:17:55PM +0200, Hauke Mehrtens wrote: > This makes it possible to open a ubi layer in device tree, this is > helpful when the rootfs is on a ubi layer. It loops though all mtd > partitions and mounts the partition which is compatible with > "ubi,volume". The same was only possible with kernel command line > arguments before. Strictly speaking this doesn't describe what this change does. Rather than mounting anything you are creating ubiblock devices... More comments in-line. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ > drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ > 2 files changed, 96 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt > > diff --git a/Documentation/devicetree/bindings/mtd/ubi.txt b/Documentation/devicetree/bindings/mtd/ubi.txt > new file mode 100644 > index 0000000..5fcd47e > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/ubi.txt > @@ -0,0 +1,33 @@ > +UBI - Unsorted block images > + > +Describe of a UBI layer in device tree. > + > + - compatible: "ubi,device", This marks a partition that contains > + a ubi layer. > + - vid_hdr_offs: Optional parameter specifies UBI VID header position > + to be used by UBI. (default value if 0) > + - max_beb_per1024: Optional parameter specifies the maximum expected bad > + eraseblock per 1024 eraseblocks. > + (default value CONFIG_MTD_UBI_BEB_LIMIT) > + -ubi_num: Optional parameter specifies UBI device number > + which have to be assigned to the newly created UBI > + device (assigned automatically by default) > + > +Example: > + > +partitions { > + compatible = "fixed-partitions"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + partition@0 { > + label = "uboot"; > + reg = <0x00000 0x100000>; > + }; > + > + partition@1c0000 { > + label = "system_sw"; > + reg = <0x1c0000 0xc800000>; > + compatible = "ubi,device"; > + }; > +}; Similar to the other patch, the example in the documentation is swapped and doesn't match with the code added by the patch. > diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c > index ebf46ad..5ed390d 100644 > --- a/drivers/mtd/ubi/block.c > +++ b/drivers/mtd/ubi/block.c Wait a moment: What about ubifs being the root filesystem? That doesn't need a ubiblock device to be created... > @@ -1,6 +1,7 @@ > /* > * Copyright (c) 2014 Ezequiel Garcia > * Copyright (c) 2011 Free Electrons > + * Copyright (c) 2016 Hauke Mehrtens <hauke@hauke-m.de> > * > * Driver parameter handling strongly based on drivers/mtd/ubi/build.c > * Copyright (c) International Business Machines Corp., 2006 > @@ -41,6 +42,7 @@ > #include <linux/kernel.h> > #include <linux/list.h> > #include <linux/mutex.h> > +#include <linux/of.h> > #include <linux/slab.h> > #include <linux/mtd/ubi.h> > #include <linux/workqueue.h> > @@ -628,6 +630,64 @@ static void __init ubiblock_create_from_param(void) > } > } > > +static void __init ubiblock_create_from_device_tree(void) > +{ > + int ubi_num; > + const char *name; > + u32 mode; > + struct ubi_device *ubi; > + struct ubi_volume_desc *desc; > + struct ubi_volume_info vi; > + struct mtd_info *mtd; > + struct device_node *volume; > + int ret; > + > + for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++) { > + ubi = ubi_get_device(ubi_num); > + if (!ubi) > + continue; > + mtd = ubi->mtd; > + if (!mtd || !of_device_is_compatible(mtd->dev.of_node, > + "ubi,device")) { > + ubi_put_device(ubi); > + continue; > + } > + > + for_each_child_of_node(mtd->dev.of_node, volume) { > + if (!of_device_is_compatible(volume, "ubi,volume")) > + continue; > + > + ret = of_property_read_string(volume, "name", &name); > + if (ret) > + continue; > + > + ret = of_property_read_u32(volume, "ubi-mode", &mode); > + if (ret) > + continue; > + > + desc = ubi_open_volume_nm(ubi_num, name, mode); > + if (IS_ERR(desc)) { > + pr_err( > + "UBI: block: can't open volume %s on ubi%d, err=%ld", > + name, ubi_num, 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, err=%d", > + vi.name, ubi_num, ret); > + continue; > + } > + } > + ubi_put_device(ubi); > + } > +} > + > static void ubiblock_remove_all(void) > { > struct ubiblock *next; > @@ -658,6 +718,9 @@ int __init ubiblock_init(void) > */ > ubiblock_create_from_param(); > > + /* Attach block devices from device tree */ > + ubiblock_create_from_device_tree(); > + Probably you also want to set ROOT_DEV similar to what we currently do in https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > /* > * Block devices are only created upon user requests, so we ignore > * existing volumes. > -- > 2.8.1 > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Am 19.06.2016 um 00:54 schrieb Hauke Mehrtens: > On 06/18/2016 09:46 PM, Richard Weinberger wrote: >> Am 18.06.2016 um 21:35 schrieb Hauke Mehrtens: >>> There are some devices on the market that are storing the root file >>> system in an ubi layer. >>> To make this work there are currently two options. >>> 1. patch the kernel to specify which partitions to open in the code. >>> 2. provide some kernel command line parameter which specifies what to open. >>> >>> I would like to specify this in device tree. >> >> What is wrong with command line parameters? If there is some parameter >> the UBI module misses I'll happily add it. >> You can also put your kernel command line into device tree. > > I would like to configure this in device tree. When I install a third > party firmware on a router normally the kernel command line is taken > from the boot loader and the device tree is appended to the kernel. Let me get this straight, your use case is booting Linux in an environment where you can't control the bootloader? You can still set the command line by building it into the kernel. And, as noted before, via device tree. Thanks, //richard
Am 19.06.2016 um 01:20 schrieb Daniel Golle: > In MBR there used to be an 'active' flag stored for each partition. > Maybe it'd be nice to introduce something similar to mark UBI volumes > to be the default rootfs. > Currently we solve this issue by convention: If a volume is named > 'rootfs' it is automatically mounted during boot. Depending on the > filesystem in use a ubiblock device has to be created as well. > This is mostly just the continuation of the existing naming convention > of mtd partitions, a patch OpenWrt is carrying around for a long > while already. > To support the same on UBI, another set of patches was made. Sorry, I still have troubles to understand your use case. Both of you seem to hate the kernel command line for reasons I don't fully understand so far. > I agree that there should be a way to pass this through the of_node > of the mtd partition which is defined in the device tree. > Selecting to-be-ubi-attached mtd partitions in device-tree would > replace patch: > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/490-ubi-auto-attach-mtd-device-named-ubi-or-data-on-boot.patch What is the need of this? Use use the kernel command line to tell UBI from which MTD to attach. > To auto-select the rootfs, we currently carry another bunch of patches > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch Same question here. > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch Ditto. > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch Ditto. > This is more or less filesystem-agnostic and works fine as long as > there is only one volume called 'rootfs' and this volume is always > used as rootfs. > > Dual-boot setups will need some way to pass the active rootfs volume to > the kernel. While I agree that this is possible by appending or > prepending to the cmdline string passed to the kernel, this either > limits the users' ability to manually specify the rootfs using the > cmdline or becomes a more complex task to only append/prepend the > cmdline in case the user-defined string doesn't already contain > relevant parameters... Sorry, but this is just a tooling problem and not to be addressed in the kernel. There is also the possibility to use an initramfs (either as file or embedded in the kernel) if the mount/attach logic becomes *really* complicated... > Thus it'd be nicer to flag the default rootfs volume via the device- > tree. As I said, as far I'm informed device tree is for configuring Linux, it describes the hardware. We also don't have LVM, DM or iSCSI bindings in DT. ;) Maybe device tree folks will tell more... Thanks, //richard
Am 19.06.2016 um 10:53 schrieb Richard Weinberger: >> Thus it'd be nicer to flag the default rootfs volume via the device- >> tree. > > As I said, as far I'm informed device tree is for configuring Linux, it describes > the hardware. We also don't have LVM, DM or iSCSI bindings in DT. ;) > Maybe device tree folks will tell more... Should be read "... is *not* for configuring Linux ...". Thanks, //richard
Hi Richard, On Sun, Jun 19, 2016 at 10:53:42AM +0200, Richard Weinberger wrote: > Am 19.06.2016 um 01:20 schrieb Daniel Golle: > > In MBR there used to be an 'active' flag stored for each partition. > > Maybe it'd be nice to introduce something similar to mark UBI volumes > > to be the default rootfs. > > Currently we solve this issue by convention: If a volume is named > > 'rootfs' it is automatically mounted during boot. Depending on the > > filesystem in use a ubiblock device has to be created as well. > > This is mostly just the continuation of the existing naming convention > > of mtd partitions, a patch OpenWrt is carrying around for a long > > while already. > > To support the same on UBI, another set of patches was made. > > Sorry, I still have troubles to understand your use case. > Both of you seem to hate the kernel command line for reasons > I don't fully understand so far. I like the kernel command line a lot :) And *being the end-user* I like to be the one defining and modifying it according to my current needs. For that reason I do *not* like firmware bootloaders to prepend/append or even overwrite the cmdline for things like selecting the rootfs, because usually that restricts the use of the relevant parameters by device owners. Imagine: The device runs a bootloader which already sets rootfs= or overwrites the cmdline of the stock firmware. How will an end-user who cannot change the bootloader use an alternative OS which uses e.g. a USB pendrive as it's rootfs? This is why we end up with things like renaming kernel cmdline parameters in alternative firmware projects, e.g. rootfs2=...., so end-users can re-gain access to cmdline parameters augmented by the bootloader. For that reason I believe that using the cmdline to pass *non-user-defined* details from the bootloader to the kernel is just not such a nice thing to do. > > > I agree that there should be a way to pass this through the of_node > > of the mtd partition which is defined in the device tree. > > Selecting to-be-ubi-attached mtd partitions in device-tree would > > replace patch: > > > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/490-ubi-auto-attach-mtd-device-named-ubi-or-data-on-boot.patch > > What is the need of this? Use use the kernel command line to tell UBI from which MTD to attach. The same kernel gets used on many devices having different $vendor mtd-partition layouts. A way other than the kernel cmdline allows to specify the default behaviour without restricting the user to manually use those cmdline options. > > > To auto-select the rootfs, we currently carry another bunch of patches > > > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch > > Same question here. > > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch > > Ditto. > > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > > Ditto. > Same arguments as above. In addition, we do not want to hard-code the filesystem type used for the rootfs volume, as it can either be UBIFS or a read-only filesystem needing a ubiblock device. Thus we would need the bootloader to know which filesystem *type* is being used and then decide wether to pass 'rootfs=ubiX:Y' or 'ubiblock=... rootfs=/dev/ubiblock0'. > > This is more or less filesystem-agnostic and works fine as long as > > there is only one volume called 'rootfs' and this volume is always > > used as rootfs. > > > > Dual-boot setups will need some way to pass the active rootfs volume to > > the kernel. While I agree that this is possible by appending or > > prepending to the cmdline string passed to the kernel, this either > > limits the users' ability to manually specify the rootfs using the > > cmdline or becomes a more complex task to only append/prepend the > > cmdline in case the user-defined string doesn't already contain > > relevant parameters... > > Sorry, but this is just a tooling problem and not to be addressed in the kernel. > There is also the possibility to use an initramfs (either as file or embedded in the kernel) > if the mount/attach logic becomes *really* complicated... > > > Thus it'd be nicer to flag the default rootfs volume via the device- > > tree. > > As I said, as far I'm informed device tree is for configuring Linux, it describes > the hardware. We also don't have LVM, DM or iSCSI bindings in DT. ;) > Maybe device tree folks will tell more... But we do have MTD and MTD partitions in DT. To me it'd feel more consistent if MTD devices, partitioning and which MTD partition(s) to use as UBI would be defined in the same place. Cheers Daniel
Daniel, Am 19.06.2016 um 13:25 schrieb Daniel Golle: > Hi Richard, > > On Sun, Jun 19, 2016 at 10:53:42AM +0200, Richard Weinberger wrote: >> Am 19.06.2016 um 01:20 schrieb Daniel Golle: >>> In MBR there used to be an 'active' flag stored for each partition. >>> Maybe it'd be nice to introduce something similar to mark UBI volumes >>> to be the default rootfs. >>> Currently we solve this issue by convention: If a volume is named >>> 'rootfs' it is automatically mounted during boot. Depending on the >>> filesystem in use a ubiblock device has to be created as well. >>> This is mostly just the continuation of the existing naming convention >>> of mtd partitions, a patch OpenWrt is carrying around for a long >>> while already. >>> To support the same on UBI, another set of patches was made. >> >> Sorry, I still have troubles to understand your use case. >> Both of you seem to hate the kernel command line for reasons >> I don't fully understand so far. > > I like the kernel command line a lot :) > And *being the end-user* I like to be the one defining and modifying it > according to my current needs. For that reason I do *not* like firmware > bootloaders to prepend/append or even overwrite the cmdline for things > like selecting the rootfs, because usually that restricts the use of > the relevant parameters by device owners. > Imagine: The device runs a bootloader which already sets rootfs= or > overwrites the cmdline of the stock firmware. How will an end-user > who cannot change the bootloader use an alternative OS which uses e.g. > a USB pendrive as it's rootfs? This is why we end up with things like > renaming kernel cmdline parameters in alternative firmware projects, > e.g. rootfs2=...., so end-users can re-gain access to cmdline > parameters augmented by the bootloader. So, the use case is being able to use a custom OS on a "locked" device where the bootloader hates you? But you still can override the cmdline supplied by the bootloader by adding your cmdline into the device tree, right? > For that reason I believe that using the cmdline to pass > *non-user-defined* details from the bootloader to the kernel is just > not such a nice thing to do. Well, this is not how the cmdline works. We don't have such an separation. Some distros use hacks such as "showopts" to have such a split. >> >>> I agree that there should be a way to pass this through the of_node >>> of the mtd partition which is defined in the device tree. >>> Selecting to-be-ubi-attached mtd partitions in device-tree would >>> replace patch: >>> >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/490-ubi-auto-attach-mtd-device-named-ubi-or-data-on-boot.patch >> >> What is the need of this? Use use the kernel command line to tell UBI from which MTD to attach. > > The same kernel gets used on many devices having different $vendor > mtd-partition layouts. A way other than the kernel cmdline allows > to specify the default behaviour without restricting the user to > manually use those cmdline options. You can put the cmdline into the per-device device tree. This is the concept of a multi-device kernel. One kernel and many device trees. >> >>> To auto-select the rootfs, we currently carry another bunch of patches >>> >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch >> >> Same question here. >> >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch >> >> Ditto. >> >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch >> >> Ditto. >> > > Same arguments as above. In addition, we do not want to hard-code the > filesystem type used for the rootfs volume, as it can either be UBIFS > or a read-only filesystem needing a ubiblock device. Thus we would > need the bootloader to know which filesystem *type* is being used and > then decide wether to pass 'rootfs=ubiX:Y' or > 'ubiblock=... rootfs=/dev/ubiblock0'. What is wrong with having a very minimal initramfs to do such an auto discovery logic? > >>> This is more or less filesystem-agnostic and works fine as long as >>> there is only one volume called 'rootfs' and this volume is always >>> used as rootfs. >>> >>> Dual-boot setups will need some way to pass the active rootfs volume to >>> the kernel. While I agree that this is possible by appending or >>> prepending to the cmdline string passed to the kernel, this either >>> limits the users' ability to manually specify the rootfs using the >>> cmdline or becomes a more complex task to only append/prepend the >>> cmdline in case the user-defined string doesn't already contain >>> relevant parameters... >> >> Sorry, but this is just a tooling problem and not to be addressed in the kernel. >> There is also the possibility to use an initramfs (either as file or embedded in the kernel) >> if the mount/attach logic becomes *really* complicated... >> >>> Thus it'd be nicer to flag the default rootfs volume via the device- >>> tree. >> >> As I said, as far I'm informed device tree is for configuring Linux, it describes >> the hardware. We also don't have LVM, DM or iSCSI bindings in DT. ;) >> Maybe device tree folks will tell more... > > But we do have MTD and MTD partitions in DT. To me it'd feel more > consistent if MTD devices, partitioning and which MTD partition(s) to > use as UBI would be defined in the same place. This is something you need to discuss wit DT folks. I'm not per se against UBI DT bindings it just does not feel right and contradicts my understanding of it. Thanks, //richard
Hi Richard, On Sun, Jun 19, 2016 at 02:02:20PM +0200, Richard Weinberger wrote: > Daniel, > > Am 19.06.2016 um 13:25 schrieb Daniel Golle: > > Hi Richard, > > > > On Sun, Jun 19, 2016 at 10:53:42AM +0200, Richard Weinberger wrote: > >> Am 19.06.2016 um 01:20 schrieb Daniel Golle: > >>> In MBR there used to be an 'active' flag stored for each partition. > >>> Maybe it'd be nice to introduce something similar to mark UBI volumes > >>> to be the default rootfs. > >>> Currently we solve this issue by convention: If a volume is named > >>> 'rootfs' it is automatically mounted during boot. Depending on the > >>> filesystem in use a ubiblock device has to be created as well. > >>> This is mostly just the continuation of the existing naming convention > >>> of mtd partitions, a patch OpenWrt is carrying around for a long > >>> while already. > >>> To support the same on UBI, another set of patches was made. > >> > >> Sorry, I still have troubles to understand your use case. > >> Both of you seem to hate the kernel command line for reasons > >> I don't fully understand so far. > > > > I like the kernel command line a lot :) > > And *being the end-user* I like to be the one defining and modifying it > > according to my current needs. For that reason I do *not* like firmware > > bootloaders to prepend/append or even overwrite the cmdline for things > > like selecting the rootfs, because usually that restricts the use of > > the relevant parameters by device owners. > > Imagine: The device runs a bootloader which already sets rootfs= or > > overwrites the cmdline of the stock firmware. How will an end-user > > who cannot change the bootloader use an alternative OS which uses e.g. > > a USB pendrive as it's rootfs? This is why we end up with things like > > renaming kernel cmdline parameters in alternative firmware projects, > > e.g. rootfs2=...., so end-users can re-gain access to cmdline > > parameters augmented by the bootloader. > > So, the use case is being able to use a custom OS on a "locked" > device where the bootloader hates you? At least that's a valid use-case which should be considered, but not the only. Some people also just don't want to modify the bootloader (especially on headless systems) so they don't risk to brick it. Many devices offer some rescue firmware flashing methods through TFTP -- however, you cannot change bootloader environment variables like the cmdline in that way and even if you could, the loader might still patch the root=... parameter in the way needed for their stock firmware. > > But you still can override the cmdline supplied by the bootloader > by adding your cmdline into the device tree, right? > > > For that reason I believe that using the cmdline to pass > > *non-user-defined* details from the bootloader to the kernel is just > > not such a nice thing to do. > > Well, this is not how the cmdline works. We don't have such > an separation. > Some distros use hacks such as "showopts" to have such a split. /me researching 'showopts' which I had never heard of before. Ok, this is for grub running interactively on x86, probably not the best reference if it comes to small headless embedded systems... There is no such a thing as a proprietary board-specific version of grub, so on x86 the whole problem just doesn't exist and showopts is really just cosmetics. However, on most ARM, MIPS or PPC based boards users may not have the option or may not want to change the bootloader. As the whole reason why some distro-kernels then decide to override the bootloader cmdline is *because* $vendor decided to make her bootloader pass the root=... and similar basic options and that screws up the rootfs of the 3rd-party Linux. Thus I believe we shouldn't encourage the use of the kernel cmdline in places the user cannot influence it (like in most $vendor variants of U-Boot where the cmdline is patched somewhere behind the scenes rather than just using the environment variable). > > > >> > >>> I agree that there should be a way to pass this through the of_node > >>> of the mtd partition which is defined in the device tree. > >>> Selecting to-be-ubi-attached mtd partitions in device-tree would > >>> replace patch: > >>> > >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/490-ubi-auto-attach-mtd-device-named-ubi-or-data-on-boot.patch > >> > >> What is the need of this? Use use the kernel command line to tell UBI from which MTD to attach. > > > > The same kernel gets used on many devices having different $vendor > > mtd-partition layouts. A way other than the kernel cmdline allows > > to specify the default behaviour without restricting the user to > > manually use those cmdline options. > > You can put the cmdline into the per-device device tree. > This is the concept of a multi-device kernel. One kernel and > many device trees. Exactly true. However, as the mtd partitions are defined in the tree itself rather than using 'mtdparts=...' in the cmdline built-into the device-tree, it'd be nice to also select that a specific partition should be ubiattached. > > >> > >>> To auto-select the rootfs, we currently carry another bunch of patches > >>> > >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch > >> > >> Same question here. > >> > >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch > >> > >> Ditto. > >> > >>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > >> > >> Ditto. > >> > > > > Same arguments as above. In addition, we do not want to hard-code the > > filesystem type used for the rootfs volume, as it can either be UBIFS > > or a read-only filesystem needing a ubiblock device. Thus we would > > need the bootloader to know which filesystem *type* is being used and > > then decide wether to pass 'rootfs=ubiX:Y' or > > 'ubiblock=... rootfs=/dev/ubiblock0'. > > What is wrong with having a very minimal initramfs to do such an > auto discovery logic? Sorry, but we are not talking about traditional desktop or server systems. OpenWrt/LEDE runs on devices with as little as 32 MB of RAM and 4 MB of Flash. Devices with UBI will typically have at least 32 MB of NAND Flash, but still not necessarily a lot of RAM. Have a look at https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/lantiq/dts/BTHOMEHUBV2B.dts for a good example of the type of hardware we are talking about. Apart from that we already automatically generate initramfs-based images for devices to be used in situations where you cannot or don't want to touch the devices' flash. As there wasn't a need to embed a initramfs for anything as simple as mounting the rootfs, I reckon it would be hard to argue that we should have an initramfs for all systems potentially using UBI just so we won't need those ~ 100 lines of C code added to the kernel to auto-select the root device *and* filesystem for both MTD and UBI. > > > > >>> This is more or less filesystem-agnostic and works fine as long as > >>> there is only one volume called 'rootfs' and this volume is always > >>> used as rootfs. > >>> > >>> Dual-boot setups will need some way to pass the active rootfs volume to > >>> the kernel. While I agree that this is possible by appending or > >>> prepending to the cmdline string passed to the kernel, this either > >>> limits the users' ability to manually specify the rootfs using the > >>> cmdline or becomes a more complex task to only append/prepend the > >>> cmdline in case the user-defined string doesn't already contain > >>> relevant parameters... > >> > >> Sorry, but this is just a tooling problem and not to be addressed in the kernel. > >> There is also the possibility to use an initramfs (either as file or embedded in the kernel) > >> if the mount/attach logic becomes *really* complicated... > >> > >>> Thus it'd be nicer to flag the default rootfs volume via the device- > >>> tree. > >> > >> As I said, as far I'm informed device tree is for configuring Linux, it describes > >> the hardware. We also don't have LVM, DM or iSCSI bindings in DT. ;) > >> Maybe device tree folks will tell more... > > > > But we do have MTD and MTD partitions in DT. To me it'd feel more > > consistent if MTD devices, partitioning and which MTD partition(s) to > > use as UBI would be defined in the same place. > > This is something you need to discuss wit DT folks. > I'm not per se against UBI DT bindings it just does not feel right > and contradicts my understanding of it. I agree with your view when it comes to describing UBI volumes in the device tree. That's wrong just as it would be wrong to add DT bindings for LVM2. As said previously, the situation is different for flash chips having partitions defined for use with UBI on some boards. Cheers Daniel
Daniel, Am 19.06.2016 um 15:05 schrieb Daniel Golle: >>> The same kernel gets used on many devices having different $vendor >>> mtd-partition layouts. A way other than the kernel cmdline allows >>> to specify the default behaviour without restricting the user to >>> manually use those cmdline options. >> >> You can put the cmdline into the per-device device tree. >> This is the concept of a multi-device kernel. One kernel and >> many device trees. > > Exactly true. However, as the mtd partitions are defined in the tree > itself rather than using 'mtdparts=...' in the cmdline built-into the > device-tree, it'd be nice to also select that a specific partition > should be ubiattached. DT already offer all you need and nothing hinders you from using mtdparts=. >> >>>> >>>>> To auto-select the rootfs, we currently carry another bunch of patches >>>>> >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch >>>> >>>> Same question here. >>>> >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch >>>> >>>> Ditto. >>>> >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch >>>> >>>> Ditto. >>>> >>> >>> Same arguments as above. In addition, we do not want to hard-code the >>> filesystem type used for the rootfs volume, as it can either be UBIFS >>> or a read-only filesystem needing a ubiblock device. Thus we would >>> need the bootloader to know which filesystem *type* is being used and >>> then decide wether to pass 'rootfs=ubiX:Y' or >>> 'ubiblock=... rootfs=/dev/ubiblock0'. >> >> What is wrong with having a very minimal initramfs to do such an >> auto discovery logic? > > Sorry, but we are not talking about traditional desktop or server > systems. OpenWrt/LEDE runs on devices with as little as 32 MB of RAM > and 4 MB of Flash. Devices with UBI will typically have at least > 32 MB of NAND Flash, but still not necessarily a lot of RAM. > Have a look at > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/lantiq/dts/BTHOMEHUBV2B.dts > for a good example of the type of hardware we are talking about. I didn't recommend adding glibc+systemd in the initramfs. It can be very, very small and trivial. At least it would be much more clean than your pile of not so fancy MTD/UBI patches. > Apart from that we already automatically generate initramfs-based > images for devices to be used in situations where you cannot or don't > want to touch the devices' flash. The initramfs can be part of the kernel image. No additional file needed. >> This is something you need to discuss wit DT folks. >> I'm not per se against UBI DT bindings it just does not feel right >> and contradicts my understanding of it. > > I agree with your view when it comes to describing UBI volumes > in the device tree. That's wrong just as it would be wrong to add > DT bindings for LVM2. > As said previously, the situation is different for flash chips having > partitions defined for use with UBI on some boards. Everything can be done with the existing framework. DT binding maybe be nice for your use case but I fear it is not applicable. Thanks, //richard
Hi Richard, On Sun, Jun 19, 2016 at 03:19:38PM +0200, Richard Weinberger wrote: > Daniel, > > Am 19.06.2016 um 15:05 schrieb Daniel Golle: > >>> The same kernel gets used on many devices having different $vendor > >>> mtd-partition layouts. A way other than the kernel cmdline allows > >>> to specify the default behaviour without restricting the user to > >>> manually use those cmdline options. > >> > >> You can put the cmdline into the per-device device tree. > >> This is the concept of a multi-device kernel. One kernel and > >> many device trees. > > > > Exactly true. However, as the mtd partitions are defined in the tree > > itself rather than using 'mtdparts=...' in the cmdline built-into the > > device-tree, it'd be nice to also select that a specific partition > > should be ubiattached. > > DT already offer all you need and nothing hinders you from > using mtdparts=. I know. However, it's much nicer to define partitions for a specific flash chip inside the device tree (I've sent a link to the DTS of the BTHOMEHUBV2B to illustrate that, please have a look at it and tell me if you'd really like that partitioning currently stored as device-tree nodes to be migrated back to 'mtdparts=...'.) > > >> > >>>> > >>>>> To auto-select the rootfs, we currently carry another bunch of patches > >>>>> > >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch > >>>> > >>>> Same question here. > >>>> > >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch > >>>> > >>>> Ditto. > >>>> > >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > >>>> > >>>> Ditto. > >>>> > >>> > >>> Same arguments as above. In addition, we do not want to hard-code the > >>> filesystem type used for the rootfs volume, as it can either be UBIFS > >>> or a read-only filesystem needing a ubiblock device. Thus we would > >>> need the bootloader to know which filesystem *type* is being used and > >>> then decide wether to pass 'rootfs=ubiX:Y' or > >>> 'ubiblock=... rootfs=/dev/ubiblock0'. > >> > >> What is wrong with having a very minimal initramfs to do such an > >> auto discovery logic? > > > > Sorry, but we are not talking about traditional desktop or server > > systems. OpenWrt/LEDE runs on devices with as little as 32 MB of RAM > > and 4 MB of Flash. Devices with UBI will typically have at least > > 32 MB of NAND Flash, but still not necessarily a lot of RAM. > > Have a look at > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/lantiq/dts/BTHOMEHUBV2B.dts > > for a good example of the type of hardware we are talking about. > > I didn't recommend adding glibc+systemd in the initramfs. It can be very, very small > and trivial. Again, please understand that the whole production firmware can be as small as 3.5 megs for kernel and rootfs. Of course we never use glibc or systemd anywhere. What you imagine to be the ways to make a initramfs really small is what we already do for *all* our system, ie. we use busybox on top of a very tiny libc. > At least it would be much more clean than your pile of not so fancy MTD/UBI patches. > In the end, do_mounts.c is already running in user-space, just the source-code happens to be part of the kernel tree. Sure, that could become a seperate executable binary which mounts the rootfs and then passes control to /sbin/init. On the other hand that would add quite some redundant infrastructure, because we already got ROOT_DEV and lots of automagic mounting going on in the kernel. > > Apart from that we already automatically generate initramfs-based > > images for devices to be used in situations where you cannot or don't > > want to touch the devices' flash. > > The initramfs can be part of the kernel image. No additional file needed. That's excatctly what we've already been doing for initramfs-only images, see e.g. http://downloads.lede-project.org/snapshots/targets/lantiq/xway/lede-lantiq-xway-BTHOMEHUBV2B-uImage-initramfs So if we need an initramfs to mount the on-flash rootfs and *another* initfamfs containing the whole system for other cases, things become even more fishy on our side. But yes, I'm aware that this would be possible. > > >> This is something you need to discuss wit DT folks. > >> I'm not per se against UBI DT bindings it just does not feel right > >> and contradicts my understanding of it. > > > > I agree with your view when it comes to describing UBI volumes > > in the device tree. That's wrong just as it would be wrong to add > > DT bindings for LVM2. > > As said previously, the situation is different for flash chips having > > partitions defined for use with UBI on some boards. > > Everything can be done with the existing framework. DT binding maybe be nice > for your use case but I fear it is not applicable. Yes, it can be done. But you would have to define your flash chips in device-tree, you may define the mtd partitions either in DT or by using mtdparts=... in the cmdline. Now to define which MTD device to ubiattach you will have to use ubi.mtd=X on the cmdline. If you defined your partitions inside the of_node of the flash chip, changes there (which do happen) may change the index of the partition you want to ubi-attach. C'mon. This *is* messy. Just having a compatible-flag to attach the partition would already greatly improve things. Hauke: Maybe we should discuss the two patches individually, as I feel that ubiattaching specific mtd partitions flagged in DT is quite straight forward, while specifying volumes inside the ubi device might be out-of-scope. Cheers Daniel > > Thanks, > //richard > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Daniel, Am 19.06.2016 um 16:09 schrieb Daniel Golle: >> DT already offer all you need and nothing hinders you from >> using mtdparts=. > > I know. However, it's much nicer to define partitions for a specific > flash chip inside the device tree (I've sent a link to the DTS of the > BTHOMEHUBV2B to illustrate that, please have a look at it and tell > me if you'd really like that partitioning currently stored as > device-tree nodes to be migrated back to 'mtdparts=...'.) I'd be fine with having the partitions in DT and the UBI kernel parameters as kernel cmdline also in DT. >> I didn't recommend adding glibc+systemd in the initramfs. It can be very, very small >> and trivial. > > Again, please understand that the whole production firmware can be as > small as 3.5 megs for kernel and rootfs. Of course we never use glibc > or systemd anywhere. I know these use cases, I design and maintain such stuff for customers. > Yes, it can be done. But you would have to define your flash chips in > device-tree, you may define the mtd partitions either in DT or by using > mtdparts=... in the cmdline. Now to define which MTD device to > ubiattach you will have to use ubi.mtd=X on the cmdline. If you defined > your partitions inside the of_node of the flash chip, changes there > (which do happen) may change the index of the partition you want to > ubi-attach. C'mon. This *is* messy. Just having a compatible-flag to > attach the partition would already greatly improve things. You mean marking a MTD partition in DT and UBI will attach from it? That makes sense. To sum up, I asked a lot of questions to understand your use case(s). Everything you described can be done with existing facilities. But I agree that at least some UBI DT machinery would be nice to have although we need to check with DT folks first. At least marking an MTD partition should be fine, hopefully. Brian, Boris, what do you think? Thanks, //richard
Hi Richard, On Sun, Jun 19, 2016 at 04:35:05PM +0200, Richard Weinberger wrote: > Daniel, > > Am 19.06.2016 um 16:09 schrieb Daniel Golle: > >> DT already offer all you need and nothing hinders you from > >> using mtdparts=. > > > > I know. However, it's much nicer to define partitions for a specific > > flash chip inside the device tree (I've sent a link to the DTS of the > > BTHOMEHUBV2B to illustrate that, please have a look at it and tell > > me if you'd really like that partitioning currently stored as > > device-tree nodes to be migrated back to 'mtdparts=...'.) > > I'd be fine with having the partitions in DT and the UBI kernel parameters > as kernel cmdline also in DT. That's the current state of affairs. I'd just want to have it in one place, so if the MTD partitioning gets updated one would see easily how that affects UBI. For that, I don't think it's good enough to have 'ubi.mtd=X' in the cmdline (in DT or wherever, it's a single string in a different place than the MTD partitioning if people use DT for MTD partitioning). > > >> I didn't recommend adding glibc+systemd in the initramfs. It can be very, very small > >> and trivial. > > > > Again, please understand that the whole production firmware can be as > > small as 3.5 megs for kernel and rootfs. Of course we never use glibc > > or systemd anywhere. > > I know these use cases, I design and maintain such stuff for customers. > > > Yes, it can be done. But you would have to define your flash chips in > > device-tree, you may define the mtd partitions either in DT or by using > > mtdparts=... in the cmdline. Now to define which MTD device to > > ubiattach you will have to use ubi.mtd=X on the cmdline. If you defined > > your partitions inside the of_node of the flash chip, changes there > > (which do happen) may change the index of the partition you want to > > ubi-attach. C'mon. This *is* messy. Just having a compatible-flag to > > attach the partition would already greatly improve things. > > You mean marking a MTD partition in DT and UBI will attach from it? > That makes sense. Yes. Currently we just use a naming convention (the first MTD partition named 'ubi' will be auto-attached), that's obviously not very clean... > > To sum up, I asked a lot of questions to understand your use case(s). > Everything you described can be done with existing facilities. > But I agree that at least some UBI DT machinery would be nice to have > although we need to check with DT folks first. > At least marking an MTD partition should be fine, hopefully. Great. That'd already greatly improve things. I can see that sooner or later we will need some way to reference UBI volumes in DT, independently of the whole rootfs selection. Vendors do use UBI volumes also for things like WiFi EEPROM data. Currentlhy, we extract that in user-space and save a copy in /lib/firmware/... for things more crazy than plain offsets inside MTD partitions. Cheers Daniel > > Brian, Boris, what do you think? > > Thanks, > //richard > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Am 19.06.2016 um 17:24 schrieb Daniel Golle: >> You mean marking a MTD partition in DT and UBI will attach from it? >> That makes sense. > > Yes. Currently we just use a naming convention (the first MTD partition > named 'ubi' will be auto-attached), that's obviously not very clean... I was about to reply to my own mail that you can still attach by name. Boris reminded me of that, I forgot that feature.^^ Why is it not clean? >> >> To sum up, I asked a lot of questions to understand your use case(s). >> Everything you described can be done with existing facilities. >> But I agree that at least some UBI DT machinery would be nice to have >> although we need to check with DT folks first. >> At least marking an MTD partition should be fine, hopefully. > > Great. That'd already greatly improve things. How is that better than attach by name? You mark the to be attached MTD by its name... > I can see that sooner or later we will need some way to reference UBI > volumes in DT, independently of the whole rootfs selection. Vendors do > use UBI volumes also for things like WiFi EEPROM data. Currentlhy, we > extract that in user-space and save a copy in /lib/firmware/... for > things more crazy than plain offsets inside MTD partitions. Really, please append a decent root= kernel parameter. Thanks, //richard
Hi Daniel, On Sun, 19 Jun 2016 17:24:46 +0200 Daniel Golle <daniel@makrotopia.org> wrote: > Hi Richard, > > On Sun, Jun 19, 2016 at 04:35:05PM +0200, Richard Weinberger wrote: > > Daniel, > > > > Am 19.06.2016 um 16:09 schrieb Daniel Golle: > > >> DT already offer all you need and nothing hinders you from > > >> using mtdparts=. > > > > > > I know. However, it's much nicer to define partitions for a specific > > > flash chip inside the device tree (I've sent a link to the DTS of the > > > BTHOMEHUBV2B to illustrate that, please have a look at it and tell > > > me if you'd really like that partitioning currently stored as > > > device-tree nodes to be migrated back to 'mtdparts=...'.) > > > > I'd be fine with having the partitions in DT and the UBI kernel parameters > > as kernel cmdline also in DT. > > That's the current state of affairs. I'd just want to have it in one > place, so if the MTD partitioning gets updated one would see easily how > that affects UBI. For that, I don't think it's good enough to have > 'ubi.mtd=X' in the cmdline (in DT or wherever, it's a single string in > a different place than the MTD partitioning if people use DT for MTD > partitioning). > > > > > >> I didn't recommend adding glibc+systemd in the initramfs. It can be very, very small > > >> and trivial. > > > > > > Again, please understand that the whole production firmware can be as > > > small as 3.5 megs for kernel and rootfs. Of course we never use glibc > > > or systemd anywhere. > > > > I know these use cases, I design and maintain such stuff for customers. > > > > > Yes, it can be done. But you would have to define your flash chips in > > > device-tree, you may define the mtd partitions either in DT or by using > > > mtdparts=... in the cmdline. Now to define which MTD device to > > > ubiattach you will have to use ubi.mtd=X on the cmdline. If you defined > > > your partitions inside the of_node of the flash chip, changes there > > > (which do happen) may change the index of the partition you want to > > > ubi-attach. C'mon. This *is* messy. Just having a compatible-flag to > > > attach the partition would already greatly improve things. > > > > You mean marking a MTD partition in DT and UBI will attach from it? > > That makes sense. > > Yes. Currently we just use a naming convention (the first MTD partition > named 'ubi' will be auto-attached), that's obviously not very clean... > > > > > To sum up, I asked a lot of questions to understand your use case(s). > > Everything you described can be done with existing facilities. > > But I agree that at least some UBI DT machinery would be nice to have > > although we need to check with DT folks first. > > At least marking an MTD partition should be fine, hopefully. > > Great. That'd already greatly improve things. > > I can see that sooner or later we will need some way to reference UBI > volumes in DT, independently of the whole rootfs selection. Vendors do > use UBI volumes also for things like WiFi EEPROM data. Currentlhy, we > extract that in user-space and save a copy in /lib/firmware/... for > things more crazy than plain offsets inside MTD partitions. Hm, you're clearly abusing the DT role here (and this comment comes from someone who tend to think that some of the config information should be described in the DT ;)). You want to link your partition (which is already a specific usage of your flash device, and as such might not even be described in the DT) to a Linux specific software layer (UBI). Remember that the DT is supposed to be OS agnostic, you clearly don't need to describe that in the DT, since everything you need is already there (a way to attach the UBI layer to an MTD device by its name). Now, if that's something you want to make generic for all the boards supporting OpenWrt, you just have to enforce the partition name, and make sure the ubi.mtd=<default-ubi-name> parameter is passed on the cmdline. IIRC, there's a way to define a default cmdline in case you don't have access to the bootloaders. This sounds like something that could be done at the build-system level. Am I missing something? For the EEPROM emulation using UBI volumes part, I'm not sure what you want to do exactly? Are you planning to use the nvmem abstraction and provide an nvmem wrapper around UBI volumes? Regards, Boris
Hi Richard, On Sun, Jun 19, 2016 at 05:31:04PM +0200, Richard Weinberger wrote: > Am 19.06.2016 um 17:24 schrieb Daniel Golle: > >> You mean marking a MTD partition in DT and UBI will attach from it? > >> That makes sense. > > > > Yes. Currently we just use a naming convention (the first MTD partition > > named 'ubi' will be auto-attached), that's obviously not very clean... > > I was about to reply to my own mail that you can still attach by name. > Boris reminded me of that, I forgot that feature.^^ > Why is it not clean? That's nice and I also didn't know that. It's still not perfect because I got to add it to the cmdline (in DT), and should have it only on devices where a partition with that given name actually exists. A flag for the mtd partition would still be nicer. > > >> > >> To sum up, I asked a lot of questions to understand your use case(s). > >> Everything you described can be done with existing facilities. > >> But I agree that at least some UBI DT machinery would be nice to have > >> although we need to check with DT folks first. > >> At least marking an MTD partition should be fine, hopefully. > > > > Great. That'd already greatly improve things. > > How is that better than attach by name? You mark the to be attached > MTD by its name... See above. We'd then still need to have that ubi.mtd=name in the cmdline for NAND devices using UBI and *not* have it for other devices within the same family which may use SPI or NOR flash without UBI. I'd prefer to have one place inside the device-tree for everything flash-storage related, eg. &nand { status = "okay"; partition@0 { label = "boot"; reg = <0x00000000 0x00e00000>; read-only; }; partition@e00000 { label = "data"; compatible = "ubi,device"; reg = <0x00e00000 0x07200000>; }; }; > > > I can see that sooner or later we will need some way to reference UBI > > volumes in DT, independently of the whole rootfs selection. Vendors do > > use UBI volumes also for things like WiFi EEPROM data. Currentlhy, we > > extract that in user-space and save a copy in /lib/firmware/... for > > things more crazy than plain offsets inside MTD partitions. > > Really, please append a decent root= kernel parameter. Maybe we just focus to solve one problem at a time :) Anyway, it'd be great if 'root=' was possible and flexible enough. However, as we support several filesystem types (ubifs vs. squashfs) this is not straight forward. ubi.mtd=mtdname root=ubiX:volname rootfstype=ubifs vs. ubi.mtd=mtdname ubiblock=... root=/dev/ubiblockX_Y And we wouldn't want two seperate DT files for those cases nor can we rely in bootloader support or any other means to differentiate them. That was part of the initial motivation for having those not-so-fancy patches in first place... (And yes, others have initramfs pre-init systems, things like real_root=... in their cmdline and other also not-that-fancy solutions to the problem) Cheers Daniel > > Thanks, > //richard
Hi Boris, On Sun, Jun 19, 2016 at 05:43:51PM +0200, Boris Brezillon wrote: > Hi Daniel, > > On Sun, 19 Jun 2016 17:24:46 +0200 > Daniel Golle <daniel@makrotopia.org> wrote: > > > Hi Richard, > > > > On Sun, Jun 19, 2016 at 04:35:05PM +0200, Richard Weinberger wrote: > > > Daniel, > > > > > > Am 19.06.2016 um 16:09 schrieb Daniel Golle: > > > >> DT already offer all you need and nothing hinders you from > > > >> using mtdparts=. > > > > > > > > I know. However, it's much nicer to define partitions for a specific > > > > flash chip inside the device tree (I've sent a link to the DTS of the > > > > BTHOMEHUBV2B to illustrate that, please have a look at it and tell > > > > me if you'd really like that partitioning currently stored as > > > > device-tree nodes to be migrated back to 'mtdparts=...'.) > > > > > > I'd be fine with having the partitions in DT and the UBI kernel parameters > > > as kernel cmdline also in DT. > > > > That's the current state of affairs. I'd just want to have it in one > > place, so if the MTD partitioning gets updated one would see easily how > > that affects UBI. For that, I don't think it's good enough to have > > 'ubi.mtd=X' in the cmdline (in DT or wherever, it's a single string in > > a different place than the MTD partitioning if people use DT for MTD > > partitioning). > > > > > > > > >> I didn't recommend adding glibc+systemd in the initramfs. It can be very, very small > > > >> and trivial. > > > > > > > > Again, please understand that the whole production firmware can be as > > > > small as 3.5 megs for kernel and rootfs. Of course we never use glibc > > > > or systemd anywhere. > > > > > > I know these use cases, I design and maintain such stuff for customers. > > > > > > > Yes, it can be done. But you would have to define your flash chips in > > > > device-tree, you may define the mtd partitions either in DT or by using > > > > mtdparts=... in the cmdline. Now to define which MTD device to > > > > ubiattach you will have to use ubi.mtd=X on the cmdline. If you defined > > > > your partitions inside the of_node of the flash chip, changes there > > > > (which do happen) may change the index of the partition you want to > > > > ubi-attach. C'mon. This *is* messy. Just having a compatible-flag to > > > > attach the partition would already greatly improve things. > > > > > > You mean marking a MTD partition in DT and UBI will attach from it? > > > That makes sense. > > > > Yes. Currently we just use a naming convention (the first MTD partition > > named 'ubi' will be auto-attached), that's obviously not very clean... > > > > > > > > To sum up, I asked a lot of questions to understand your use case(s). > > > Everything you described can be done with existing facilities. > > > But I agree that at least some UBI DT machinery would be nice to have > > > although we need to check with DT folks first. > > > At least marking an MTD partition should be fine, hopefully. > > > > Great. That'd already greatly improve things. > > > > I can see that sooner or later we will need some way to reference UBI > > volumes in DT, independently of the whole rootfs selection. Vendors do > > use UBI volumes also for things like WiFi EEPROM data. Currentlhy, we > > extract that in user-space and save a copy in /lib/firmware/... for > > things more crazy than plain offsets inside MTD partitions. > > Hm, you're clearly abusing the DT role here (and this comment comes from > someone who tend to think that some of the config information should be > described in the DT ;)). > > You want to link your partition (which is already a specific usage of > your flash device, and as such might not even be described in the DT) > to a Linux specific software layer (UBI). > Remember that the DT is supposed to be OS agnostic, you clearly don't > need to describe that in the DT, since everything you need is already > there (a way to attach the UBI layer to an MTD device by its name). UBI is supported by U-Boot as well and U-Boot supports storing its environment inside an UBI volume, loading things from volumes or even mounting UBIFS, so information about which partition to attach might already be useful outside of Linux as well. Apart from that, DT already stores a whole lot of Linux-specific properties, such as linux,keycodes or for gpio-keys-polled or linux,default-trigger for LEDs. Not having that information in the devicetree would definitely make things worse imho. > > Now, if that's something you want to make generic for all the boards > supporting OpenWrt, you just have to enforce the partition name, and > make sure the ubi.mtd=<default-ubi-name> parameter is passed on the > cmdline. IIRC, there's a way to define a default cmdline in case you > don't have access to the bootloaders. > > This sounds like something that could be done at the build-system level. True, but devices which do not have UBI will throw some errors to the log about not being able to attach the device. > > Am I missing something? > > For the EEPROM emulation using UBI volumes part, I'm not sure what you > want to do exactly? Are you planning to use the nvmem abstraction and > provide an nvmem wrapper around UBI volumes? Definitely not something I'd hope to get into at all, however, I can smell it coming ;) Cheers Daniel > > Regards, > > Boris
On Sun, 19 Jun 2016 18:13:36 +0200 Daniel Golle <daniel@makrotopia.org> wrote: > Hi Richard, > > On Sun, Jun 19, 2016 at 05:31:04PM +0200, Richard Weinberger wrote: > > Am 19.06.2016 um 17:24 schrieb Daniel Golle: > > >> You mean marking a MTD partition in DT and UBI will attach from it? > > >> That makes sense. > > > > > > Yes. Currently we just use a naming convention (the first MTD partition > > > named 'ubi' will be auto-attached), that's obviously not very clean... > > > > I was about to reply to my own mail that you can still attach by name. > > Boris reminded me of that, I forgot that feature.^^ > > Why is it not clean? > > That's nice and I also didn't know that. > It's still not perfect because I got to add it to the cmdline (in DT), > and should have it only on devices where a partition with that given > name actually exists. A flag for the mtd partition would still be > nicer. > > > > > >> > > >> To sum up, I asked a lot of questions to understand your use case(s). > > >> Everything you described can be done with existing facilities. > > >> But I agree that at least some UBI DT machinery would be nice to have > > >> although we need to check with DT folks first. > > >> At least marking an MTD partition should be fine, hopefully. > > > > > > Great. That'd already greatly improve things. > > > > How is that better than attach by name? You mark the to be attached > > MTD by its name... > > See above. We'd then still need to have that ubi.mtd=name in the > cmdline for NAND devices using UBI and *not* have it for other devices > within the same family which may use SPI or NOR flash without UBI. > I'd prefer to have one place inside the device-tree for everything > flash-storage related, eg. > &nand { > status = "okay"; > partition@0 { > label = "boot"; > reg = <0x00000000 0x00e00000>; > read-only; > }; > > partition@e00000 { > label = "data"; > compatible = "ubi,device"; So, if we follow your logic we should also have compatible = "jffs2,file-system"; Because JFFS2 is an MTD user, just as UBI is. Let's see what Rob and other DT maintainers think about that. Still, it seems to me that you're trying to solve a problem in the kernel when it should actually be solved in an upper layer. Another option would be to try attaching UBI (along with all possible MTD users) to all the the MTD partitions. That's what's done for block filesystems when rootfstype is not specified. > reg = <0x00e00000 0x07200000>; > }; > };
Hi Boris, On Sun, Jun 19, 2016 at 06:53:43PM +0200, Boris Brezillon wrote: > On Sun, 19 Jun 2016 18:13:36 +0200 > Daniel Golle <daniel@makrotopia.org> wrote: > > > Hi Richard, > > > > On Sun, Jun 19, 2016 at 05:31:04PM +0200, Richard Weinberger wrote: > > > Am 19.06.2016 um 17:24 schrieb Daniel Golle: > > > >> You mean marking a MTD partition in DT and UBI will attach from it? > > > >> That makes sense. > > > > > > > > Yes. Currently we just use a naming convention (the first MTD partition > > > > named 'ubi' will be auto-attached), that's obviously not very clean... > > > > > > I was about to reply to my own mail that you can still attach by name. > > > Boris reminded me of that, I forgot that feature.^^ > > > Why is it not clean? > > > > That's nice and I also didn't know that. > > It's still not perfect because I got to add it to the cmdline (in DT), > > and should have it only on devices where a partition with that given > > name actually exists. A flag for the mtd partition would still be > > nicer. > > > > > > > > >> > > > >> To sum up, I asked a lot of questions to understand your use case(s). > > > >> Everything you described can be done with existing facilities. > > > >> But I agree that at least some UBI DT machinery would be nice to have > > > >> although we need to check with DT folks first. > > > >> At least marking an MTD partition should be fine, hopefully. > > > > > > > > Great. That'd already greatly improve things. > > > > > > How is that better than attach by name? You mark the to be attached > > > MTD by its name... > > > > See above. We'd then still need to have that ubi.mtd=name in the > > cmdline for NAND devices using UBI and *not* have it for other devices > > within the same family which may use SPI or NOR flash without UBI. > > I'd prefer to have one place inside the device-tree for everything > > flash-storage related, eg. > > &nand { > > status = "okay"; > > partition@0 { > > label = "boot"; > > reg = <0x00000000 0x00e00000>; > > read-only; > > }; > > > > partition@e00000 { > > label = "data"; > > compatible = "ubi,device"; > > So, if we follow your logic we should also have > > compatible = "jffs2,file-system"; > > Because JFFS2 is an MTD user, just as UBI is. Well, if there was any use for that, then yes, we should have that. However, if root=/dev/mtdblockX is set, the filesystem type simply doesn't matter as it is being probed and thus there simply is no real need for that. In some rare cases the bootloader may make use of JFFS2 and a certain (/boot) partition is thus required to be JFFS2 no-matter-what. In that case I'd agree, there should be such a thing as "jffs2,file-system" set in DT (but that's a very rare corner case and abuse would probably prevail over ligitimate use-cases). In the end, the classication as "is an MTD user" isn't relevant for this debate imho. > > Let's see what Rob and other DT maintainers think about that. > > Still, it seems to me that you're trying to solve a problem in the > kernel when it should actually be solved in an upper layer. It's definitely inside the grey-zone of early-userland... Doing things which need to be done to mount the root filesystem are traditionally the exceptions of things which are user-land and yet happen inside the kernel before executing init. > > Another option would be to try attaching UBI (along with all possible > MTD users) to all the the MTD partitions. That's what's done for block > filesystems when rootfstype is not specified. Correct, but in case of filesystems, it is only done for the specified partition/device. Probing all partitions sounds nice, however, please keep in mind that MTD partitions may overlap and there may be left-overs from previous UBI devices. Simply trying to ubiattach *all* available MTD devices will need quite some extra work to handle all potential outcomes of trying to attach non-UBI or broken/corrupted/wrongly-sized UBI partitions. Generally, I believe that's actually the only good alternative, but I doubt it can easily made as robust and reliable compared to hinting which mtd device to ubiattach via the devicetree. Cheers Daniel > > > > reg = <0x00e00000 0x07200000>; > > }; > > }; > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Sun, 19 Jun 2016 21:42:39 +0200 Daniel Golle <daniel@makrotopia.org> wrote: > Hi Boris, > > On Sun, Jun 19, 2016 at 06:53:43PM +0200, Boris Brezillon wrote: > > On Sun, 19 Jun 2016 18:13:36 +0200 > > Daniel Golle <daniel@makrotopia.org> wrote: > > > > > Hi Richard, > > > > > > On Sun, Jun 19, 2016 at 05:31:04PM +0200, Richard Weinberger wrote: > > > > Am 19.06.2016 um 17:24 schrieb Daniel Golle: > > > > >> You mean marking a MTD partition in DT and UBI will attach from it? > > > > >> That makes sense. > > > > > > > > > > Yes. Currently we just use a naming convention (the first MTD partition > > > > > named 'ubi' will be auto-attached), that's obviously not very clean... > > > > > > > > I was about to reply to my own mail that you can still attach by name. > > > > Boris reminded me of that, I forgot that feature.^^ > > > > Why is it not clean? > > > > > > That's nice and I also didn't know that. > > > It's still not perfect because I got to add it to the cmdline (in DT), > > > and should have it only on devices where a partition with that given > > > name actually exists. A flag for the mtd partition would still be > > > nicer. > > > > > > > > > > > >> > > > > >> To sum up, I asked a lot of questions to understand your use case(s). > > > > >> Everything you described can be done with existing facilities. > > > > >> But I agree that at least some UBI DT machinery would be nice to have > > > > >> although we need to check with DT folks first. > > > > >> At least marking an MTD partition should be fine, hopefully. > > > > > > > > > > Great. That'd already greatly improve things. > > > > > > > > How is that better than attach by name? You mark the to be attached > > > > MTD by its name... > > > > > > See above. We'd then still need to have that ubi.mtd=name in the > > > cmdline for NAND devices using UBI and *not* have it for other devices > > > within the same family which may use SPI or NOR flash without UBI. > > > I'd prefer to have one place inside the device-tree for everything > > > flash-storage related, eg. > > > &nand { > > > status = "okay"; > > > partition@0 { > > > label = "boot"; > > > reg = <0x00000000 0x00e00000>; > > > read-only; > > > }; > > > > > > partition@e00000 { > > > label = "data"; > > > compatible = "ubi,device"; > > > > So, if we follow your logic we should also have > > > > compatible = "jffs2,file-system"; > > > > Because JFFS2 is an MTD user, just as UBI is. > > Well, if there was any use for that, then yes, we should have that. > However, if root=/dev/mtdblockX is set, the filesystem type simply > doesn't matter as it is being probed and thus there simply is no > real need for that. > > In some rare cases the bootloader may make use of JFFS2 and a certain > (/boot) partition is thus required to be JFFS2 no-matter-what. In that > case I'd agree, there should be such a thing as "jffs2,file-system" set > in DT (but that's a very rare corner case and abuse would probably > prevail over ligitimate use-cases). > > In the end, the classication as "is an MTD user" isn't relevant for > this debate imho. It is, because supporting the UBI specific case is just an open door to supporting any specific cases people might have, and how should we justify that UBI is more legitimate than others. > > > > > Let's see what Rob and other DT maintainers think about that. > > > > Still, it seems to me that you're trying to solve a problem in the > > kernel when it should actually be solved in an upper layer. > > It's definitely inside the grey-zone of early-userland... Doing things > which need to be done to mount the root filesystem are traditionally > the exceptions of things which are user-land and yet happen inside > the kernel before executing init. But I've never seen any DT properties specifying which FS should be used on a block device... I know, partitions on block devices are stored in a dedicated partition table, and MTD partitions are not (or let's say that most of the time they are not). But still, I think the software stack you put on top of your storage device should not appear in the DT. Now, if all other people think this is sane to do it, I won't block it. > > > > > Another option would be to try attaching UBI (along with all possible > > MTD users) to all the the MTD partitions. That's what's done for block > > filesystems when rootfstype is not specified. > > Correct, but in case of filesystems, it is only done for the specified > partition/device. Probing all partitions sounds nice, however, please > keep in mind that MTD partitions may overlap and there may be > left-overs from previous UBI devices. Overlapping partitions are unsafe in general, no matter what you want to do with it. The only valid case would be 1 RW partition and several RO partitions overlapping with the RW one. The other accepted use case is exposing the master device (the real MTD device under MTD partitions). But in any case, using 2 overlapping partitions in // and both in RW mode is unsafe and should be avoided. > Simply trying to ubiattach *all* > available MTD devices will need quite some extra work to handle all > potential outcomes of trying to attach non-UBI or > broken/corrupted/wrongly-sized UBI partitions. Attaching broken/corrupted/wrongly-sized partitions will still fail, except they might be detected as valid UBI partition by the auto-detection code, which would trigger a full attach step. The only drawback is that you might try something that has been corrupted, but this is already the case with the current approach. BTW, I don't expect to see a lot of systems defining several UBI devices attached to a single MTD device. So this only leaves the multi-devices case. And of course this auto-detection logic would have to be explicitly enabled, so the penalty would only impact those enabling this feature. > Generally, I believe that's actually the only good alternative, but I > doubt it can easily made as robust and reliable compared to hinting > which mtd device to ubiattach via the devicetree. As I said, I'm not in favor of putting this kind of information in the DT, so I'm trying to find other approaches to solve your problem ;).
On 06/19/2016 01:56 AM, Daniel Golle wrote: > Hi! > > I got some remarks here: > > On Sat, Jun 18, 2016 at 09:17:55PM +0200, Hauke Mehrtens wrote: >> This makes it possible to open a ubi layer in device tree, this is >> helpful when the rootfs is on a ubi layer. It loops though all mtd >> partitions and mounts the partition which is compatible with >> "ubi,volume". The same was only possible with kernel command line >> arguments before. > > Strictly speaking this doesn't describe what this change does. > Rather than mounting anything you are creating ubiblock devices... > > More comments in-line. > >> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> --- >> Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ >> drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ >> 2 files changed, 96 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt >> >> diff --git a/Documentation/devicetree/bindings/mtd/ubi.txt b/Documentation/devicetree/bindings/mtd/ubi.txt >> new file mode 100644 >> index 0000000..5fcd47e >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mtd/ubi.txt >> @@ -0,0 +1,33 @@ >> +UBI - Unsorted block images >> + >> +Describe of a UBI layer in device tree. >> + >> + - compatible: "ubi,device", This marks a partition that contains >> + a ubi layer. >> + - vid_hdr_offs: Optional parameter specifies UBI VID header position >> + to be used by UBI. (default value if 0) >> + - max_beb_per1024: Optional parameter specifies the maximum expected bad >> + eraseblock per 1024 eraseblocks. >> + (default value CONFIG_MTD_UBI_BEB_LIMIT) >> + -ubi_num: Optional parameter specifies UBI device number >> + which have to be assigned to the newly created UBI >> + device (assigned automatically by default) >> + >> +Example: >> + >> +partitions { >> + compatible = "fixed-partitions"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + >> + partition@0 { >> + label = "uboot"; >> + reg = <0x00000 0x100000>; >> + }; >> + >> + partition@1c0000 { >> + label = "system_sw"; >> + reg = <0x1c0000 0xc800000>; >> + compatible = "ubi,device"; >> + }; >> +}; > > Similar to the other patch, the example in the documentation is > swapped and doesn't match with the code added by the patch. > > > >> diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c >> index ebf46ad..5ed390d 100644 >> --- a/drivers/mtd/ubi/block.c >> +++ b/drivers/mtd/ubi/block.c > > Wait a moment: What about ubifs being the root filesystem? That > doesn't need a ubiblock device to be created... Should I write an error message and do nothing when this is a ubifs partition? >> @@ -1,6 +1,7 @@ >> /* >> * Copyright (c) 2014 Ezequiel Garcia >> * Copyright (c) 2011 Free Electrons >> + * Copyright (c) 2016 Hauke Mehrtens <hauke@hauke-m.de> >> * >> * Driver parameter handling strongly based on drivers/mtd/ubi/build.c >> * Copyright (c) International Business Machines Corp., 2006 >> @@ -41,6 +42,7 @@ >> #include <linux/kernel.h> >> #include <linux/list.h> >> #include <linux/mutex.h> >> +#include <linux/of.h> >> #include <linux/slab.h> >> #include <linux/mtd/ubi.h> >> #include <linux/workqueue.h> >> @@ -628,6 +630,64 @@ static void __init ubiblock_create_from_param(void) >> } >> } >> >> +static void __init ubiblock_create_from_device_tree(void) >> +{ >> + int ubi_num; >> + const char *name; >> + u32 mode; >> + struct ubi_device *ubi; >> + struct ubi_volume_desc *desc; >> + struct ubi_volume_info vi; >> + struct mtd_info *mtd; >> + struct device_node *volume; >> + int ret; >> + >> + for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++) { >> + ubi = ubi_get_device(ubi_num); >> + if (!ubi) >> + continue; >> + mtd = ubi->mtd; >> + if (!mtd || !of_device_is_compatible(mtd->dev.of_node, >> + "ubi,device")) { >> + ubi_put_device(ubi); >> + continue; >> + } >> + >> + for_each_child_of_node(mtd->dev.of_node, volume) { >> + if (!of_device_is_compatible(volume, "ubi,volume")) >> + continue; >> + >> + ret = of_property_read_string(volume, "name", &name); >> + if (ret) >> + continue; >> + >> + ret = of_property_read_u32(volume, "ubi-mode", &mode); >> + if (ret) >> + continue; >> + >> + desc = ubi_open_volume_nm(ubi_num, name, mode); >> + if (IS_ERR(desc)) { >> + pr_err( >> + "UBI: block: can't open volume %s on ubi%d, err=%ld", >> + name, ubi_num, 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, err=%d", >> + vi.name, ubi_num, ret); >> + continue; >> + } >> + } >> + ubi_put_device(ubi); >> + } >> +} >> + >> static void ubiblock_remove_all(void) >> { >> struct ubiblock *next; >> @@ -658,6 +718,9 @@ int __init ubiblock_init(void) >> */ >> ubiblock_create_from_param(); >> >> + /* Attach block devices from device tree */ >> + ubiblock_create_from_device_tree(); >> + > > Probably you also want to set ROOT_DEV similar to what we currently > do in > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > >> /* >> * Block devices are only created upon user requests, so we ignore >> * existing volumes. >> -- >> 2.8.1 >> >> >> ______________________________________________________ >> Linux MTD discussion mailing list >> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Boris, On Sun, Jun 19, 2016 at 10:14:34PM +0200, Boris Brezillon wrote: > On Sun, 19 Jun 2016 21:42:39 +0200 > Daniel Golle <daniel@makrotopia.org> wrote: > > > Hi Boris, > > > > On Sun, Jun 19, 2016 at 06:53:43PM +0200, Boris Brezillon wrote: > > > On Sun, 19 Jun 2016 18:13:36 +0200 > > > Daniel Golle <daniel@makrotopia.org> wrote: > > > > > > > Hi Richard, > > > > > > > > On Sun, Jun 19, 2016 at 05:31:04PM +0200, Richard Weinberger wrote: > > > > > Am 19.06.2016 um 17:24 schrieb Daniel Golle: > > > > > >> You mean marking a MTD partition in DT and UBI will attach from it? > > > > > >> That makes sense. > > > > > > > > > > > > Yes. Currently we just use a naming convention (the first MTD partition > > > > > > named 'ubi' will be auto-attached), that's obviously not very clean... > > > > > > > > > > I was about to reply to my own mail that you can still attach by name. > > > > > Boris reminded me of that, I forgot that feature.^^ > > > > > Why is it not clean? > > > > > > > > That's nice and I also didn't know that. > > > > It's still not perfect because I got to add it to the cmdline (in DT), > > > > and should have it only on devices where a partition with that given > > > > name actually exists. A flag for the mtd partition would still be > > > > nicer. > > > > > > > > > > > > > > >> > > > > > >> To sum up, I asked a lot of questions to understand your use case(s). > > > > > >> Everything you described can be done with existing facilities. > > > > > >> But I agree that at least some UBI DT machinery would be nice to have > > > > > >> although we need to check with DT folks first. > > > > > >> At least marking an MTD partition should be fine, hopefully. > > > > > > > > > > > > Great. That'd already greatly improve things. > > > > > > > > > > How is that better than attach by name? You mark the to be attached > > > > > MTD by its name... > > > > > > > > See above. We'd then still need to have that ubi.mtd=name in the > > > > cmdline for NAND devices using UBI and *not* have it for other devices > > > > within the same family which may use SPI or NOR flash without UBI. > > > > I'd prefer to have one place inside the device-tree for everything > > > > flash-storage related, eg. > > > > &nand { > > > > status = "okay"; > > > > partition@0 { > > > > label = "boot"; > > > > reg = <0x00000000 0x00e00000>; > > > > read-only; > > > > }; > > > > > > > > partition@e00000 { > > > > label = "data"; > > > > compatible = "ubi,device"; > > > > > > So, if we follow your logic we should also have > > > > > > compatible = "jffs2,file-system"; > > > > > > Because JFFS2 is an MTD user, just as UBI is. > > > > Well, if there was any use for that, then yes, we should have that. > > However, if root=/dev/mtdblockX is set, the filesystem type simply > > doesn't matter as it is being probed and thus there simply is no > > real need for that. > > > > In some rare cases the bootloader may make use of JFFS2 and a certain > > (/boot) partition is thus required to be JFFS2 no-matter-what. In that > > case I'd agree, there should be such a thing as "jffs2,file-system" set > > in DT (but that's a very rare corner case and abuse would probably > > prevail over ligitimate use-cases). > > > > In the end, the classication as "is an MTD user" isn't relevant for > > this debate imho. > > It is, because supporting the UBI specific case is just an open door to > supporting any specific cases people might have, and how should we > justify that UBI is more legitimate than others. That could be true, if there actually are other things which require device-specific information about MTD partitions. In a way something similar already exists in a reverse-fashion for in-flash WiFi EEPROMs, eg. fpi@10000000 { ... ath9k_eep { compatible = "ath9k,eeprom"; ath,eep-flash = <&ath9k_cal 0x985>; ath,eep-endian; ath,eep-swap; }; ... }; &spi { pinctrl-names = "default"; pinctrl-0 = <&pins_spi_default>; status = "ok"; m25p80@4 { #address-cells = <1>; #size-cells = <1>; compatible = "jedec,spi-nor"; reg = <4 0>; spi-max-frequency = <1000000>; ath9k_cal: partition@0 { reg = <0x0 0x20000>; label = "urlader"; read-only; }; ... }; (excerpt from FRITZ3370.dts) > > > > > > > > > Let's see what Rob and other DT maintainers think about that. > > > > > > Still, it seems to me that you're trying to solve a problem in the > > > kernel when it should actually be solved in an upper layer. > > > > It's definitely inside the grey-zone of early-userland... Doing things > > which need to be done to mount the root filesystem are traditionally > > the exceptions of things which are user-land and yet happen inside > > the kernel before executing init. > > But I've never seen any DT properties specifying which FS should be > used on a block device... No, because that's not needed for filesystems. UBI sitting on top of a specific MTD partition is kinda part of the general way a device is supposed to be used, bootloaders may assume UBI to be present in a certain area and if used by the stock firmware, the partition used for UBI by a third party firmware should be within the same boundaries. To me it looks like a device-specific convention, similar to flash areas used for WiFi EEPROMs or bootloader environments. (to detect the bootloader environment in user-space we are currently appending the device-tree by having per-board defaults. That's also not that nice, but it's not as problematic because usually uneeded during normal runtime, see https://git.lede-project.org/?p=source.git;a=blob;f=package/boot/uboot-envtools/files/lantiq ) > I know, partitions on block devices are stored in a dedicated partition > table, and MTD partitions are not (or let's say that most of the time > they are not). > > But still, I think the software stack you put on top of your storage > device should not appear in the DT. I agree, with the exception of whatever is the minimum needed for the device to boot. LVM2, cryptsetup or FUSE are optional luxury and I agree there shouldn't be any volume-manager or filesystem specifics in the device tree. However, while I believe that this includes > > Now, if all other people think this is sane to do it, I won't block it. > > > > > > > > > Another option would be to try attaching UBI (along with all possible > > > MTD users) to all the the MTD partitions. That's what's done for block > > > filesystems when rootfstype is not specified. > > > > Correct, but in case of filesystems, it is only done for the specified > > partition/device. Probing all partitions sounds nice, however, please > > keep in mind that MTD partitions may overlap and there may be > > left-overs from previous UBI devices. > > Overlapping partitions are unsafe in general, no matter what you want to > do with it. The only valid case would be 1 RW partition and several RO > partitions overlapping with the RW one. > The other accepted use case is exposing the master device (the real MTD > device under MTD partitions). But in any case, using 2 overlapping > partitions in // and both in RW mode is unsafe and should be avoided. That second case is quite common, and also having a MTD partition defined for the whole firmware-region (ie. usually starting at an offset above the bootloader and it's environment and ending just before the WiFi EEPROM which is commonly stored at the high end of flash chips). Accidentally ubi-attaching that firmware partition may end up quite unlucky if it starts with a UBI device... > > > Simply trying to ubiattach *all* > > available MTD devices will need quite some extra work to handle all > > potential outcomes of trying to attach non-UBI or > > broken/corrupted/wrongly-sized UBI partitions. > > Attaching broken/corrupted/wrongly-sized partitions will still fail, > except they might be detected as valid UBI partition by the > auto-detection code, which would trigger a full attach step. > The only drawback is that you might try something that has been > corrupted, but this is already the case with the current approach. Not quite, because you are only probing the partition selected rather than all of them. > > BTW, I don't expect to see a lot of systems defining several UBI > devices attached to a single MTD device. So this only leaves the > multi-devices case. And of course this auto-detection logic would have > to be explicitly enabled, so the penalty would only impact those > enabling this feature. > > > Generally, I believe that's actually the only good alternative, but I > > doubt it can easily made as robust and reliable compared to hinting > > which mtd device to ubiattach via the devicetree. > > As I said, I'm not in favor of putting this kind of information in the > DT, so I'm trying to find other approaches to solve your problem ;). Making it safe to auto-probe all MTD devices is indeed a sound option, I just expected it to be more effort and more pitfalls compared to an optional marker in the device-tree which is just a convenience replacement for the cmdline syntax. Cheers Daniel
On Sun, Jun 19, 2016 at 11:36:19PM +0200, Hauke Mehrtens wrote: > On 06/19/2016 01:56 AM, Daniel Golle wrote: > > Hi! > > > > I got some remarks here: > > > > On Sat, Jun 18, 2016 at 09:17:55PM +0200, Hauke Mehrtens wrote: > >> This makes it possible to open a ubi layer in device tree, this is > >> helpful when the rootfs is on a ubi layer. It loops though all mtd > >> partitions and mounts the partition which is compatible with > >> "ubi,volume". The same was only possible with kernel command line > >> arguments before. > > > > Strictly speaking this doesn't describe what this change does. > > Rather than mounting anything you are creating ubiblock devices... > > > > More comments in-line. > > > >> > >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > >> --- > >> Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ > >> drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ > >> 2 files changed, 96 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt > >> > >> diff --git a/Documentation/devicetree/bindings/mtd/ubi.txt b/Documentation/devicetree/bindings/mtd/ubi.txt > >> new file mode 100644 > >> index 0000000..5fcd47e > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/mtd/ubi.txt > >> @@ -0,0 +1,33 @@ > >> +UBI - Unsorted block images > >> + > >> +Describe of a UBI layer in device tree. > >> + > >> + - compatible: "ubi,device", This marks a partition that contains > >> + a ubi layer. > >> + - vid_hdr_offs: Optional parameter specifies UBI VID header position > >> + to be used by UBI. (default value if 0) > >> + - max_beb_per1024: Optional parameter specifies the maximum expected bad > >> + eraseblock per 1024 eraseblocks. > >> + (default value CONFIG_MTD_UBI_BEB_LIMIT) > >> + -ubi_num: Optional parameter specifies UBI device number > >> + which have to be assigned to the newly created UBI > >> + device (assigned automatically by default) > >> + > >> +Example: > >> + > >> +partitions { > >> + compatible = "fixed-partitions"; > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + > >> + partition@0 { > >> + label = "uboot"; > >> + reg = <0x00000 0x100000>; > >> + }; > >> + > >> + partition@1c0000 { > >> + label = "system_sw"; > >> + reg = <0x1c0000 0xc800000>; > >> + compatible = "ubi,device"; > >> + }; > >> +}; > > > > Similar to the other patch, the example in the documentation is > > swapped and doesn't match with the code added by the patch. > > > > > > > >> diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c > >> index ebf46ad..5ed390d 100644 > >> --- a/drivers/mtd/ubi/block.c > >> +++ b/drivers/mtd/ubi/block.c > > > > Wait a moment: What about ubifs being the root filesystem? That > > doesn't need a ubiblock device to be created... > > Should I write an error message and do nothing when this is a ubifs > partition? Just do nothing if you find a UBIFS magic. I reckon there shouldn't be any error message because it's not an error but rather just another valid case. Maybe an info message could be useful for starters... > > >> @@ -1,6 +1,7 @@ > >> /* > >> * Copyright (c) 2014 Ezequiel Garcia > >> * Copyright (c) 2011 Free Electrons > >> + * Copyright (c) 2016 Hauke Mehrtens <hauke@hauke-m.de> > >> * > >> * Driver parameter handling strongly based on drivers/mtd/ubi/build.c > >> * Copyright (c) International Business Machines Corp., 2006 > >> @@ -41,6 +42,7 @@ > >> #include <linux/kernel.h> > >> #include <linux/list.h> > >> #include <linux/mutex.h> > >> +#include <linux/of.h> > >> #include <linux/slab.h> > >> #include <linux/mtd/ubi.h> > >> #include <linux/workqueue.h> > >> @@ -628,6 +630,64 @@ static void __init ubiblock_create_from_param(void) > >> } > >> } > >> > >> +static void __init ubiblock_create_from_device_tree(void) > >> +{ > >> + int ubi_num; > >> + const char *name; > >> + u32 mode; > >> + struct ubi_device *ubi; > >> + struct ubi_volume_desc *desc; > >> + struct ubi_volume_info vi; > >> + struct mtd_info *mtd; > >> + struct device_node *volume; > >> + int ret; > >> + > >> + for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++) { > >> + ubi = ubi_get_device(ubi_num); > >> + if (!ubi) > >> + continue; > >> + mtd = ubi->mtd; > >> + if (!mtd || !of_device_is_compatible(mtd->dev.of_node, > >> + "ubi,device")) { > >> + ubi_put_device(ubi); > >> + continue; > >> + } > >> + > >> + for_each_child_of_node(mtd->dev.of_node, volume) { > >> + if (!of_device_is_compatible(volume, "ubi,volume")) > >> + continue; > >> + > >> + ret = of_property_read_string(volume, "name", &name); > >> + if (ret) > >> + continue; > >> + > >> + ret = of_property_read_u32(volume, "ubi-mode", &mode); > >> + if (ret) > >> + continue; > >> + > >> + desc = ubi_open_volume_nm(ubi_num, name, mode); > >> + if (IS_ERR(desc)) { > >> + pr_err( > >> + "UBI: block: can't open volume %s on ubi%d, err=%ld", > >> + name, ubi_num, 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, err=%d", > >> + vi.name, ubi_num, ret); > >> + continue; > >> + } > >> + } > >> + ubi_put_device(ubi); > >> + } > >> +} > >> + > >> static void ubiblock_remove_all(void) > >> { > >> struct ubiblock *next; > >> @@ -658,6 +718,9 @@ int __init ubiblock_init(void) > >> */ > >> ubiblock_create_from_param(); > >> > >> + /* Attach block devices from device tree */ > >> + ubiblock_create_from_device_tree(); > >> + > > > > Probably you also want to set ROOT_DEV similar to what we currently > > do in > > > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > > > >> /* > >> * Block devices are only created upon user requests, so we ignore > >> * existing volumes. > >> -- > >> 2.8.1 > >> > >> > >> ______________________________________________________ > >> Linux MTD discussion mailing list > >> http://lists.infradead.org/mailman/listinfo/linux-mtd/ > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 06/19/2016 11:48 PM, Daniel Golle wrote: > Hi Boris, > > On Sun, Jun 19, 2016 at 10:14:34PM +0200, Boris Brezillon wrote: >> On Sun, 19 Jun 2016 21:42:39 +0200 >> Daniel Golle <daniel@makrotopia.org> wrote: >> >>> Hi Boris, >>> >>> On Sun, Jun 19, 2016 at 06:53:43PM +0200, Boris Brezillon wrote: >>>> On Sun, 19 Jun 2016 18:13:36 +0200 >>>> Daniel Golle <daniel@makrotopia.org> wrote: >>>> >>>>> Hi Richard, >>>>> >>>>> On Sun, Jun 19, 2016 at 05:31:04PM +0200, Richard Weinberger wrote: >>>>>> Am 19.06.2016 um 17:24 schrieb Daniel Golle: >>>>>>>> You mean marking a MTD partition in DT and UBI will attach from it? >>>>>>>> That makes sense. >>>>>>> >>>>>>> Yes. Currently we just use a naming convention (the first MTD partition >>>>>>> named 'ubi' will be auto-attached), that's obviously not very clean... >>>>>> >>>>>> I was about to reply to my own mail that you can still attach by name. >>>>>> Boris reminded me of that, I forgot that feature.^^ >>>>>> Why is it not clean? >>>>> >>>>> That's nice and I also didn't know that. >>>>> It's still not perfect because I got to add it to the cmdline (in DT), >>>>> and should have it only on devices where a partition with that given >>>>> name actually exists. A flag for the mtd partition would still be >>>>> nicer. Is it possible to addend the kernel command line with device tree? Some vendor boot loaders are adding the mac address to the kernel command line and I would like to get it. Like Daniel said replacing the vendor boot loader is dangerous because sometimes recovery means to desolder the flash chip. >>>>> >>>>>> >>>>>>>> >>>>>>>> To sum up, I asked a lot of questions to understand your use case(s). >>>>>>>> Everything you described can be done with existing facilities. >>>>>>>> But I agree that at least some UBI DT machinery would be nice to have >>>>>>>> although we need to check with DT folks first. >>>>>>>> At least marking an MTD partition should be fine, hopefully. >>>>>>> >>>>>>> Great. That'd already greatly improve things. >>>>>> >>>>>> How is that better than attach by name? You mark the to be attached >>>>>> MTD by its name... >>>>> >>>>> See above. We'd then still need to have that ubi.mtd=name in the >>>>> cmdline for NAND devices using UBI and *not* have it for other devices >>>>> within the same family which may use SPI or NOR flash without UBI. >>>>> I'd prefer to have one place inside the device-tree for everything >>>>> flash-storage related, eg. >>>>> &nand { >>>>> status = "okay"; >>>>> partition@0 { >>>>> label = "boot"; >>>>> reg = <0x00000000 0x00e00000>; >>>>> read-only; >>>>> }; >>>>> >>>>> partition@e00000 { >>>>> label = "data"; >>>>> compatible = "ubi,device"; >>>> >>>> So, if we follow your logic we should also have >>>> >>>> compatible = "jffs2,file-system"; >>>> >>>> Because JFFS2 is an MTD user, just as UBI is. >>> >>> Well, if there was any use for that, then yes, we should have that. >>> However, if root=/dev/mtdblockX is set, the filesystem type simply >>> doesn't matter as it is being probed and thus there simply is no >>> real need for that. >>> >>> In some rare cases the bootloader may make use of JFFS2 and a certain >>> (/boot) partition is thus required to be JFFS2 no-matter-what. In that >>> case I'd agree, there should be such a thing as "jffs2,file-system" set >>> in DT (but that's a very rare corner case and abuse would probably >>> prevail over ligitimate use-cases). >>> >>> In the end, the classication as "is an MTD user" isn't relevant for >>> this debate imho. >> >> It is, because supporting the UBI specific case is just an open door to >> supporting any specific cases people might have, and how should we >> justify that UBI is more legitimate than others. > > That could be true, if there actually are other things which require > device-specific information about MTD partitions. In a way something > similar already exists in a reverse-fashion for in-flash WiFi EEPROMs, > eg. > fpi@10000000 { > ... > ath9k_eep { > compatible = "ath9k,eeprom"; > ath,eep-flash = <&ath9k_cal 0x985>; > ath,eep-endian; > ath,eep-swap; > }; > ... > }; > > &spi { > pinctrl-names = "default"; > pinctrl-0 = <&pins_spi_default>; > > status = "ok"; > > m25p80@4 { > #address-cells = <1>; > #size-cells = <1>; > compatible = "jedec,spi-nor"; > reg = <4 0>; > spi-max-frequency = <1000000>; > > ath9k_cal: partition@0 { > reg = <0x0 0x20000>; > label = "urlader"; > read-only; > }; > ... > }; > > (excerpt from FRITZ3370.dts) > > >> >>> >>>> >>>> Let's see what Rob and other DT maintainers think about that. >>>> >>>> Still, it seems to me that you're trying to solve a problem in the >>>> kernel when it should actually be solved in an upper layer. >>> >>> It's definitely inside the grey-zone of early-userland... Doing things >>> which need to be done to mount the root filesystem are traditionally >>> the exceptions of things which are user-land and yet happen inside >>> the kernel before executing init. >> >> But I've never seen any DT properties specifying which FS should be >> used on a block device... > > No, because that's not needed for filesystems. UBI sitting on top of > a specific MTD partition is kinda part of the general way a device is > supposed to be used, bootloaders may assume UBI to be present in a > certain area and if used by the stock firmware, the partition used > for UBI by a third party firmware should be within the same > boundaries. To me it looks like a device-specific convention, similar > to flash areas used for WiFi EEPROMs or bootloader environments. > (to detect the bootloader environment in user-space we are currently > appending the device-tree by having per-board defaults. That's also > not that nice, but it's not as problematic because usually uneeded > during normal runtime, see > https://git.lede-project.org/?p=source.git;a=blob;f=package/boot/uboot-envtools/files/lantiq > ) In the end I would like to be able to specify from where to mount the root filesystem in device tree. The partition layout directly on the flash chip is already defined in device tree. The U-Boot from the Intel / Lantiq SDK currently loads the kernel from an UBI partition and then the kernel should also load the rootfs from an UBI partition. You can also flash the images from the boot loader the boot loader takes care of placing your image into the correct UBI partitions. In this case the boot loader is aware of the partition schema and also the UBI schema. >> I know, partitions on block devices are stored in a dedicated partition >> table, and MTD partitions are not (or let's say that most of the time >> they are not). >> >> But still, I think the software stack you put on top of your storage >> device should not appear in the DT. > > I agree, with the exception of whatever is the minimum needed for the > device to boot. LVM2, cryptsetup or FUSE are optional luxury and I > agree there shouldn't be any volume-manager or filesystem specifics in > the device tree. However, while I believe that this includes > >> >> Now, if all other people think this is sane to do it, I won't block it. >> >>> >>>> >>>> Another option would be to try attaching UBI (along with all possible >>>> MTD users) to all the the MTD partitions. That's what's done for block >>>> filesystems when rootfstype is not specified. >>> >>> Correct, but in case of filesystems, it is only done for the specified >>> partition/device. Probing all partitions sounds nice, however, please >>> keep in mind that MTD partitions may overlap and there may be >>> left-overs from previous UBI devices. >> >> Overlapping partitions are unsafe in general, no matter what you want to >> do with it. The only valid case would be 1 RW partition and several RO >> partitions overlapping with the RW one. >> The other accepted use case is exposing the master device (the real MTD >> device under MTD partitions). But in any case, using 2 overlapping >> partitions in // and both in RW mode is unsafe and should be avoided. > > That second case is quite common, and also having a MTD partition > defined for the whole firmware-region (ie. usually starting at an > offset above the bootloader and it's environment and ending just > before the WiFi EEPROM which is commonly stored at the high end of > flash chips). Accidentally ubi-attaching that firmware partition may > end up quite unlucky if it starts with a UBI device... > >> >>> Simply trying to ubiattach *all* >>> available MTD devices will need quite some extra work to handle all >>> potential outcomes of trying to attach non-UBI or >>> broken/corrupted/wrongly-sized UBI partitions. >> >> Attaching broken/corrupted/wrongly-sized partitions will still fail, >> except they might be detected as valid UBI partition by the >> auto-detection code, which would trigger a full attach step. >> The only drawback is that you might try something that has been >> corrupted, but this is already the case with the current approach. > > Not quite, because you are only probing the partition selected rather > than all of them. > >> >> BTW, I don't expect to see a lot of systems defining several UBI >> devices attached to a single MTD device. So this only leaves the >> multi-devices case. And of course this auto-detection logic would have >> to be explicitly enabled, so the penalty would only impact those >> enabling this feature. >> >>> Generally, I believe that's actually the only good alternative, but I >>> doubt it can easily made as robust and reliable compared to hinting >>> which mtd device to ubiattach via the devicetree. >> >> As I said, I'm not in favor of putting this kind of information in the >> DT, so I'm trying to find other approaches to solve your problem ;). > > Making it safe to auto-probe all MTD devices is indeed a sound option, > I just expected it to be more effort and more pitfalls compared to > an optional marker in the device-tree which is just a convenience > replacement for the cmdline syntax. > > > Cheers > > > Daniel >
On Monday, June 20, 2016 12:21:07 AM CEST Hauke Mehrtens wrote: > On 06/19/2016 11:48 PM, Daniel Golle wrote: > > Hi Boris, > > > > On Sun, Jun 19, 2016 at 10:14:34PM +0200, Boris Brezillon wrote: > >> On Sun, 19 Jun 2016 21:42:39 +0200 > >> Daniel Golle <daniel@makrotopia.org> wrote: > >>>>> That's nice and I also didn't know that. > >>>>> It's still not perfect because I got to add it to the cmdline (in DT), > >>>>> and should have it only on devices where a partition with that given > >>>>> name actually exists. A flag for the mtd partition would still be > >>>>> nicer. > > Is it possible to addend the kernel command line with device tree? Some > vendor boot loaders are adding the mac address to the kernel command > line and I would like to get it. Like Daniel said replacing the vendor > boot loader is dangerous because sometimes recovery means to desolder > the flash chip. The bootloader can easily modify the command line in the dt, but things like device mac address should be modified in the ethernet device node if possible. > > > > That could be true, if there actually are other things which require > > device-specific information about MTD partitions. In a way something > > similar already exists in a reverse-fashion for in-flash WiFi EEPROMs, > > eg. > > fpi@10000000 { > > ... > > ath9k_eep { > > compatible = "ath9k,eeprom"; > > ath,eep-flash = <&ath9k_cal 0x985>; > > ath,eep-endian; > > ath,eep-swap; > > }; > > ... > > }; This is a bit of an ad-hoc binding, I wouldn't use that as an example for how to do things elsewhere. > >>>> Let's see what Rob and other DT maintainers think about that. > >>>> > >>>> Still, it seems to me that you're trying to solve a problem in the > >>>> kernel when it should actually be solved in an upper layer. > >>> > >>> It's definitely inside the grey-zone of early-userland... Doing things > >>> which need to be done to mount the root filesystem are traditionally > >>> the exceptions of things which are user-land and yet happen inside > >>> the kernel before executing init. > >> > >> But I've never seen any DT properties specifying which FS should be > >> used on a block device... Right, that goes a bit too far. File system type etc should clearly be detectable from either the partition table or the partition itself. > > No, because that's not needed for filesystems. UBI sitting on top of > > a specific MTD partition is kinda part of the general way a device is > > supposed to be used, bootloaders may assume UBI to be present in a > > certain area and if used by the stock firmware, the partition used > > for UBI by a third party firmware should be within the same > > boundaries. To me it looks like a device-specific convention, similar > > to flash areas used for WiFi EEPROMs or bootloader environments. > > (to detect the bootloader environment in user-space we are currently > > appending the device-tree by having per-board defaults. That's also > > not that nice, but it's not as problematic because usually uneeded > > during normal runtime, see > > https://git.lede-project.org/?p=source.git;a=blob;f=package/boot/uboot-envtools/files/lantiq > > ) > > In the end I would like to be able to specify from where to mount the > root filesystem in device tree. The partition layout directly on the > flash chip is already defined in device tree. > > The U-Boot from the Intel / Lantiq SDK currently loads the kernel from > an UBI partition and then the kernel should also load the rootfs from an > UBI partition. You can also flash the images from the boot loader the > boot loader takes care of placing your image into the correct UBI > partitions. In this case the boot loader is aware of the partition > schema and also the UBI schema. A few more thoughts on various things that came up in the discussion so far (so I don't have to reply to each of them separately) - The /chosen and /aliases nodes do configure the OS to some degree, and historically we have used them to specify the device names that show up in the kernel (e.g. on MacOS and AIX) as well as the rootfs itself - The device tree should give us enough information to identify the root partition by a GUID from the kernel. This works on all block devices (at least with MBR and EFI partition tables, probably many of the lesser known ones as well) and it should really also work on UBI. - MTD partitioning unfortunately does not seem to have a widely used method for identifying partitions from what's stored on the flash, unlike block devices, so whatever we normally have in the partition table has to be stored in DT. This really sucks, but I don't know what else to do about it. - The UBI design was originally meant to cover the entire flash device and provide the partitioning information inside. I have not looked at UBI in a long time (actually not since before embedded PPC moved to DT), but I would hope that this case works without any DT magic. Arnd
Am 20.06.2016 um 10:09 schrieb Arnd Bergmann: > A few more thoughts on various things that came up in the discussion > so far (so I don't have to reply to each of them separately) > > - The /chosen and /aliases nodes do configure the OS to some degree, > and historically we have used them to specify the device names that > show up in the kernel (e.g. on MacOS and AIX) as well as the rootfs > itself > > - The device tree should give us enough information to identify the > root partition by a GUID from the kernel. This works on all block > devices (at least with MBR and EFI partition tables, probably many > of the lesser known ones as well) and it should really also work > on UBI. It does. > - MTD partitioning unfortunately does not seem to have a widely used > method for identifying partitions from what's stored on the flash, > unlike block devices, so whatever we normally have in the partition > table has to be stored in DT. This really sucks, but I don't know > what else to do about it. Since blocks on a MTD can render bad you'd lose the table sooner or later. That's why we cannot store it on the MTD directly. Defining the table in DT is at least less ugly than using the mtdparts= kernel parameter. > - The UBI design was originally meant to cover the entire flash > device and provide the partitioning information inside. I have not > looked at UBI in a long time (actually not since before embedded > PPC moved to DT), but I would hope that this case works without > any DT magic. Of course it works without DT magic. So, I don't see a urge to have UBI DT bindings. First I thought having an attach flag would be handy but it turned out that this behavior can easily achieved by naming. i.e. you a kernel command line such as "ubi.mtd=UBI" and name the to be attached MTD "UBI" in your DT. Thanks, //richard
On Monday, June 20, 2016 10:26:11 AM CEST Richard Weinberger wrote: > Am 20.06.2016 um 10:09 schrieb Arnd Bergmann: > > A few more thoughts on various things that came up in the discussion > > so far (so I don't have to reply to each of them separately) > > > > - The /chosen and /aliases nodes do configure the OS to some degree, > > and historically we have used them to specify the device names that > > show up in the kernel (e.g. on MacOS and AIX) as well as the rootfs > > itself > > > > - The device tree should give us enough information to identify the > > root partition by a GUID from the kernel. This works on all block > > devices (at least with MBR and EFI partition tables, probably many > > of the lesser known ones as well) and it should really also work > > on UBI. > > It does. > > > - MTD partitioning unfortunately does not seem to have a widely used > > method for identifying partitions from what's stored on the flash, > > unlike block devices, so whatever we normally have in the partition > > table has to be stored in DT. This really sucks, but I don't know > > what else to do about it. > > Since blocks on a MTD can render bad you'd lose the table sooner or later. > That's why we cannot store it on the MTD directly. > Defining the table in DT is at least less ugly than using the mtdparts= > kernel parameter. Right, there would be no benefit in using the kernel command line, it just moves the information to another place inside of the same DT (the /chosen property). I think you can normally rely on the first block being readable on flash, in particular if you write it very rarely (when updating the partition information), so it would be technically possible to have a partition table in there, but in practice that's not how things are done, so the argument is useless. > > - The UBI design was originally meant to cover the entire flash > > device and provide the partitioning information inside. I have not > > looked at UBI in a long time (actually not since before embedded > > PPC moved to DT), but I would hope that this case works without > > any DT magic. > > Of course it works without DT magic. > > So, I don't see a urge to have UBI DT bindings. > First I thought having an attach flag would be handy but it turned out > that this behavior can easily achieved by naming. i.e. you a kernel > command line such as "ubi.mtd=UBI" and name the to be attached MTD "UBI" > in your DT. Right. I think either way can work, and I see advantages in both. The downside of relying just on the name is that mounting the rootfs by GUID doesn't work until you specify another command line, which is a bit annoying but not a huge problem. Arnd
Hi, On Sun, Jun 19, 2016 at 04:09:53PM +0200, Daniel Golle wrote: > On Sun, Jun 19, 2016 at 03:19:38PM +0200, Richard Weinberger wrote: > > Am 19.06.2016 um 15:05 schrieb Daniel Golle: > > >>> The same kernel gets used on many devices having different $vendor > > >>> mtd-partition layouts. A way other than the kernel cmdline allows > > >>> to specify the default behaviour without restricting the user to > > >>> manually use those cmdline options. > > >> > > >> You can put the cmdline into the per-device device tree. > > >> This is the concept of a multi-device kernel. One kernel and > > >> many device trees. > > > > > > Exactly true. However, as the mtd partitions are defined in the tree > > > itself rather than using 'mtdparts=...' in the cmdline built-into the > > > device-tree, it'd be nice to also select that a specific partition > > > should be ubiattached. > > > > DT already offer all you need and nothing hinders you from > > using mtdparts=. > > I know. However, it's much nicer to define partitions for a specific > flash chip inside the device tree (I've sent a link to the DTS of the > BTHOMEHUBV2B to illustrate that, please have a look at it and tell > me if you'd really like that partitioning currently stored as > device-tree nodes to be migrated back to 'mtdparts=...'.) Just because it's currently nicer to put partitions in device tree doesn't really mean it's good to extend those bindings to do even more. I believe there are alternatives that could work just fine. See below. > > >>>> > > >>>>> To auto-select the rootfs, we currently carry another bunch of patches > > >>>>> > > >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/491-ubi-auto-create-ubiblock-device-for-rootfs.patch > > >>>> > > >>>> Same question here. > > >>>> > > >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/492-try-auto-mounting-ubi0-rootfs-in-init-do_mounts.c.patch > > >>>> > > >>>> Ditto. > > >>>> > > >>>>> https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/generic/patches-4.4/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch > > >>>> > > >>>> Ditto. > > >>>> > > >>> > > >>> Same arguments as above. In addition, we do not want to hard-code the > > >>> filesystem type used for the rootfs volume, as it can either be UBIFS > > >>> or a read-only filesystem needing a ubiblock device. Thus we would > > >>> need the bootloader to know which filesystem *type* is being used and > > >>> then decide wether to pass 'rootfs=ubiX:Y' or > > >>> 'ubiblock=... rootfs=/dev/ubiblock0'. > > >> > > >> What is wrong with having a very minimal initramfs to do such an > > >> auto discovery logic? > > > > > > Sorry, but we are not talking about traditional desktop or server > > > systems. OpenWrt/LEDE runs on devices with as little as 32 MB of RAM > > > and 4 MB of Flash. Devices with UBI will typically have at least > > > 32 MB of NAND Flash, but still not necessarily a lot of RAM. > > > Have a look at > > > https://git.lede-project.org/?p=source.git;a=blob;f=target/linux/lantiq/dts/BTHOMEHUBV2B.dts > > > for a good example of the type of hardware we are talking about. [...] > > > Apart from that we already automatically generate initramfs-based > > > images for devices to be used in situations where you cannot or don't > > > want to touch the devices' flash. [...] > > >> This is something you need to discuss wit DT folks. > > >> I'm not per se against UBI DT bindings it just does not feel right > > >> and contradicts my understanding of it. I haven't followed the entire thread, though I've skimmed most of it, and I agree I don't see a good reason for UBI DT bindings. > > > I agree with your view when it comes to describing UBI volumes > > > in the device tree. That's wrong just as it would be wrong to add > > > DT bindings for LVM2. > > > As said previously, the situation is different for flash chips having > > > partitions defined for use with UBI on some boards. I disagree. How is the situation different? > > Everything can be done with the existing framework. DT binding maybe be nice > > for your use case but I fear it is not applicable. > > Yes, it can be done. But you would have to define your flash chips in > device-tree, you may define the mtd partitions either in DT or by using > mtdparts=... in the cmdline. Now to define which MTD device to > ubiattach you will have to use ubi.mtd=X on the cmdline. If you defined > your partitions inside the of_node of the flash chip, changes there > (which do happen) may change the index of the partition you want to > ubi-attach. C'mon. This *is* messy. Just having a compatible-flag to > attach the partition would already greatly improve things. To be clear, I'd really like to avoid relying on and extending fixed partitioning like defined in Documentation/.../mtd/partitions.txt. So anything we can do to help you plan for not relying on that case would be awesome. For instance, with this series, there will hopefully be more cases where you don't need your whole partition layout in DT: http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html So what will you do then? Also, why are you relying on the index form of ubi.mtd=X (e.g., ubi.mtd=0)? Why can't you use ubi.mtd=rootfs (and define 'label = "rootfs"' with the existing fixed-partition DT bindings)? http://linux-mtd.infradead.org/faq/ubi.html#L_attachmtd That seems less likely to break, and it could probably be adapted/reused on systems where you might not put the entire partition layout in device tree. (i.e., you don't have to reinvent your wheel if you start using non-"fixed-partitions" device trees.) > Hauke: Maybe we should discuss the two patches individually, as I feel > that ubiattaching specific mtd partitions flagged in DT is quite > straight forward, while specifying volumes inside the ubi device might > be out-of-scope. I agree that those two concepts have a quite different likelihood of being accepted (the former is possible; the latter seems highly suspect). Brian
Hi, I'll take a moment to hijack this and solicit comments on this old thread: On Mon, Jun 20, 2016 at 10:09:50AM +0200, Arnd Bergmann wrote: > - MTD partitioning unfortunately does not seem to have a widely used > method for identifying partitions from what's stored on the flash, > unlike block devices, so whatever we normally have in the partition > table has to be stored in DT. This really sucks, but I don't know > what else to do about it. I got sidetracked, but I have attempted previously to at least diminish the amount of MTD partitioning info that is stuck into DT: http://lists.infradead.org/pipermail/linux-mtd/2015-December/064076.html That series effectively accepts the fact that there isn't a single standard or widely used method for identifying partitions, but it does allow for a description that's at least more flexible than a static list of partitions. I'd appreciate any thoughts on that. The main factors that stalled that were that (a) there was some vague discomfort over the use of the "compatible" property (I could probably have argued my case better there) and (b) I didn't have more time to pursue that. I may pick this up again when I get some time, especially if there are constructive suggestions. Brian
On Mon, Jun 20, 2016 at 05:08:51PM +0200, Arnd Bergmann wrote: > On Monday, June 20, 2016 10:26:11 AM CEST Richard Weinberger wrote: > > Am 20.06.2016 um 10:09 schrieb Arnd Bergmann: > > > - MTD partitioning unfortunately does not seem to have a widely used > > > method for identifying partitions from what's stored on the flash, > > > unlike block devices, so whatever we normally have in the partition > > > table has to be stored in DT. This really sucks, but I don't know > > > what else to do about it. > > > > Since blocks on a MTD can render bad you'd lose the table sooner or later. > > That's why we cannot store it on the MTD directly. That's assuming you're talking about NAND, and that no one has devised any clever partition scheme that is robust against bad blocks. I've seen attempts at the latter, but nothing widely used. And eventually, you're just reimplementing UBI anyway. About the former: some systems will still use a dual-flash system, where they get reliable partition information from a NOR flash to bootstrap getting the NAND up and running. So it's possible to avoid sticking all partitions in DT. > > Defining the table in DT is at least less ugly than using the mtdparts= > > kernel parameter. > > Right, there would be no benefit in using the kernel command line, > it just moves the information to another place inside of the same DT > (the /chosen property). > > I think you can normally rely on the first block being readable > on flash, in particular if you write it very rarely (when updating > the partition information), so it would be technically possible to > have a partition table in there, but in practice that's not how > things are done, so the argument is useless. It's not completely useless, but it's difficult for real technical reasons: many SoCs boot straight off address 0 of a flash (hopefully something reliable like NOR flash, but I've even seen that for NAND...), so you can't possibly put a partition table there. That doesn't preclude coming up with a decent non-0-address-based scheme, but nobody's really done that generically, although there are various vendor-specific attempts to just progressively scan all blocks on the flash -- see a few drivers/mtd/*part.c. Brian
Am 20.06.2016 um 17:08 schrieb Arnd Bergmann: >> Since blocks on a MTD can render bad you'd lose the table sooner or later. >> That's why we cannot store it on the MTD directly. >> Defining the table in DT is at least less ugly than using the mtdparts= >> kernel parameter. > > Right, there would be no benefit in using the kernel command line, > it just moves the information to another place inside of the same DT > (the /chosen property). > > I think you can normally rely on the first block being readable > on flash, in particular if you write it very rarely (when updating > the partition information), so it would be technically possible to > have a partition table in there, but in practice that's not how > things are done, so the argument is useless. Speaking of NAND, only SLC (and also here not all) chips guarantee that the first block is better than the other ones. Thanks, //richard
On 06/20/2016 09:57 PM, Richard Weinberger wrote: > Am 20.06.2016 um 17:08 schrieb Arnd Bergmann: >>> Since blocks on a MTD can render bad you'd lose the table sooner or later. >>> That's why we cannot store it on the MTD directly. >>> Defining the table in DT is at least less ugly than using the mtdparts= >>> kernel parameter. >> >> Right, there would be no benefit in using the kernel command line, >> it just moves the information to another place inside of the same DT >> (the /chosen property). >> >> I think you can normally rely on the first block being readable >> on flash, in particular if you write it very rarely (when updating >> the partition information), so it would be technically possible to >> have a partition table in there, but in practice that's not how >> things are done, so the argument is useless. > > Speaking of NAND, only SLC (and also here not all) chips guarantee that the first > block is better than the other ones. Being able to define a rootfs the kernel should load with a /chosen property would also be nice. https://www.kernel.org/doc/Documentation/devicetree/bindings/chosen.txt These UBI patches were just one step to make it possible to define the root partition in device tree. It would probably be better to first look on how to do that with normal mtd partitions and then see how to extend that to UBI. Currently I have to extend the boot args with these parameters to boot a unmodified mainline kernel: "ubi.mtd=ubi ubi.block=0,rootfsA root=/dev/ubiblock0_1" This is a SquashFS in a UBI layer on a mtd partition. I know that I can put this into the bootargs in a device tree file. Hauke
On 18 June 2016 at 16:35, Hauke Mehrtens <hauke@hauke-m.de> wrote: > On 06/18/2016 09:30 PM, Richard Weinberger wrote: >> Am 18.06.2016 um 21:17 schrieb Hauke Mehrtens: >>> This makes it possible to open a ubi layer in device tree, this is >>> helpful when the rootfs is on a ubi layer. It loops though all mtd >>> partitions and mounts the partition which is compatible with >>> "ubi,volume". The same was only possible with kernel command line >>> arguments before. >>> >>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >>> --- >>> Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ >>> drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ >>> 2 files changed, 96 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt >> >> Some time ago I thought about an UBI DT binding too, but >> I have been told that device tree is only for describing the hardware >> and nothing else. >> So I fear this will be rejected by DT folks. > > There are some devices on the market that are storing the root file > system in an ubi layer. > To make this work there are currently two options. > 1. patch the kernel to specify which partitions to open in the code. I wouldn't go as far as to claim this an option. > 2. provide some kernel command line parameter which specifies what to open. > There's another option: use an initramfs, where you can do whatever you want to find your UBI and mount it. Frankly, I don't see a real need for this in the devicetree, other than solving someone's particular workflow/use case.
On 06/24/2016 08:28 PM, Ezequiel Garcia wrote: > On 18 June 2016 at 16:35, Hauke Mehrtens <hauke@hauke-m.de> wrote: >> On 06/18/2016 09:30 PM, Richard Weinberger wrote: >>> Am 18.06.2016 um 21:17 schrieb Hauke Mehrtens: >>>> This makes it possible to open a ubi layer in device tree, this is >>>> helpful when the rootfs is on a ubi layer. It loops though all mtd >>>> partitions and mounts the partition which is compatible with >>>> "ubi,volume". The same was only possible with kernel command line >>>> arguments before. >>>> >>>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >>>> --- >>>> Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ >>>> drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ >>>> 2 files changed, 96 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt >>> >>> Some time ago I thought about an UBI DT binding too, but >>> I have been told that device tree is only for describing the hardware >>> and nothing else. >>> So I fear this will be rejected by DT folks. >> >> There are some devices on the market that are storing the root file >> system in an ubi layer. >> To make this work there are currently two options. >> 1. patch the kernel to specify which partitions to open in the code. > > I wouldn't go as far as to claim this an option. I would also not do this, but when you look into the Linux patches of "professional" embedded devices you see many strange things. > >> 2. provide some kernel command line parameter which specifies what to open. >> > > There's another option: use an initramfs, where you can do whatever you want > to find your UBI and mount it. An initramfs probably needs some 10 to 100 KBytes of additional memory on the flash chip and we only have 4 MBytes on some devices for the bootloader, kernel and rootfs, adding 100 KBytes there is a significant amount of memory. Instead of adding a initramfs I would like to be able to define the rootfs somewhere in device tree and not by adding a bootargs line into device tree. These particular patches are probably the wrong way, but nobody told me how to do this in a better way. > Frankly, I don't see a real need for this in the devicetree, other than solving > someone's particular workflow/use case.
Am 25.06.2016 um 22:20 schrieb Hauke Mehrtens: >> There's another option: use an initramfs, where you can do whatever you want >> to find your UBI and mount it. > > An initramfs probably needs some 10 to 100 KBytes of additional memory > on the flash chip and we only have 4 MBytes on some devices for the > bootloader, kernel and rootfs, adding 100 KBytes there is a significant > amount of memory. Instead of adding a initramfs I would like to be able > to define the rootfs somewhere in device tree and not by adding a > bootargs line into device tree. These particular patches are probably > the wrong way, but nobody told me how to do this in a better way. I think it is time to bring this discussion to a end. It has been stated multiple times why DT is not the right thing for the job and how all you need can be solved without an UBI DT binding. Consider both patches as rejected. Thanks, //richard
diff --git a/Documentation/devicetree/bindings/mtd/ubi.txt b/Documentation/devicetree/bindings/mtd/ubi.txt new file mode 100644 index 0000000..5fcd47e --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/ubi.txt @@ -0,0 +1,33 @@ +UBI - Unsorted block images + +Describe of a UBI layer in device tree. + + - compatible: "ubi,device", This marks a partition that contains + a ubi layer. + - vid_hdr_offs: Optional parameter specifies UBI VID header position + to be used by UBI. (default value if 0) + - max_beb_per1024: Optional parameter specifies the maximum expected bad + eraseblock per 1024 eraseblocks. + (default value CONFIG_MTD_UBI_BEB_LIMIT) + -ubi_num: Optional parameter specifies UBI device number + which have to be assigned to the newly created UBI + device (assigned automatically by default) + +Example: + +partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "uboot"; + reg = <0x00000 0x100000>; + }; + + partition@1c0000 { + label = "system_sw"; + reg = <0x1c0000 0xc800000>; + compatible = "ubi,device"; + }; +}; diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c index ebf46ad..5ed390d 100644 --- a/drivers/mtd/ubi/block.c +++ b/drivers/mtd/ubi/block.c @@ -1,6 +1,7 @@ /* * Copyright (c) 2014 Ezequiel Garcia * Copyright (c) 2011 Free Electrons + * Copyright (c) 2016 Hauke Mehrtens <hauke@hauke-m.de> * * Driver parameter handling strongly based on drivers/mtd/ubi/build.c * Copyright (c) International Business Machines Corp., 2006 @@ -41,6 +42,7 @@ #include <linux/kernel.h> #include <linux/list.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/mtd/ubi.h> #include <linux/workqueue.h> @@ -628,6 +630,64 @@ static void __init ubiblock_create_from_param(void) } } +static void __init ubiblock_create_from_device_tree(void) +{ + int ubi_num; + const char *name; + u32 mode; + struct ubi_device *ubi; + struct ubi_volume_desc *desc; + struct ubi_volume_info vi; + struct mtd_info *mtd; + struct device_node *volume; + int ret; + + for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++) { + ubi = ubi_get_device(ubi_num); + if (!ubi) + continue; + mtd = ubi->mtd; + if (!mtd || !of_device_is_compatible(mtd->dev.of_node, + "ubi,device")) { + ubi_put_device(ubi); + continue; + } + + for_each_child_of_node(mtd->dev.of_node, volume) { + if (!of_device_is_compatible(volume, "ubi,volume")) + continue; + + ret = of_property_read_string(volume, "name", &name); + if (ret) + continue; + + ret = of_property_read_u32(volume, "ubi-mode", &mode); + if (ret) + continue; + + desc = ubi_open_volume_nm(ubi_num, name, mode); + if (IS_ERR(desc)) { + pr_err( + "UBI: block: can't open volume %s on ubi%d, err=%ld", + name, ubi_num, 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, err=%d", + vi.name, ubi_num, ret); + continue; + } + } + ubi_put_device(ubi); + } +} + static void ubiblock_remove_all(void) { struct ubiblock *next; @@ -658,6 +718,9 @@ int __init ubiblock_init(void) */ ubiblock_create_from_param(); + /* Attach block devices from device tree */ + ubiblock_create_from_device_tree(); + /* * Block devices are only created upon user requests, so we ignore * existing volumes.
This makes it possible to open a ubi layer in device tree, this is helpful when the rootfs is on a ubi layer. It loops though all mtd partitions and mounts the partition which is compatible with "ubi,volume". The same was only possible with kernel command line arguments before. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- Documentation/devicetree/bindings/mtd/ubi.txt | 33 ++++++++++++++ drivers/mtd/ubi/block.c | 63 +++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/ubi.txt