From: Hauke Mehrtens <hauke@hauke-m.de>
To: Daniel Golle <daniel@makrotopia.org>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
dedekind1@gmail.com, richard@nod.at, robh+dt@kernel.org,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
dwmw2@infradead.org
Subject: Re: [PATCH 1/2] ubi: mount partitions specified in device tree
Date: Sun, 19 Jun 2016 23:36:19 +0200 [thread overview]
Message-ID: <ecb7950d-9e07-5c5d-6668-1c67ba170ddf@hauke-m.de> (raw)
In-Reply-To: <20160618235634.GD29476@makrotopia.org>
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/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2016-06-19 21:36 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-18 19:17 [PATCH 1/2] ubi: mount partitions specified in device tree Hauke Mehrtens
2016-06-18 19:30 ` Richard Weinberger
[not found] ` <5765A14F.6020201-/L3Ra7n9ekc@public.gmane.org>
2016-06-18 19:35 ` Hauke Mehrtens
2016-06-18 23:20 ` Daniel Golle
[not found] ` <20160618232054.GB29476-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 8:53 ` Richard Weinberger
[not found] ` <57665D96.1070804-/L3Ra7n9ekc@public.gmane.org>
2016-06-19 9:16 ` Richard Weinberger
2016-06-19 11:25 ` Daniel Golle
[not found] ` <20160619112510.GA820-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 12:02 ` Richard Weinberger
[not found] ` <576689CC.3030809-/L3Ra7n9ekc@public.gmane.org>
2016-06-19 13:05 ` Daniel Golle
2016-06-19 13:19 ` Richard Weinberger
[not found] ` <57669BEA.5030306-/L3Ra7n9ekc@public.gmane.org>
2016-06-19 14:09 ` Daniel Golle
[not found] ` <20160619140952.GC820-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 14:35 ` Richard Weinberger
[not found] ` <5766AD99.1040809-/L3Ra7n9ekc@public.gmane.org>
2016-06-19 15:24 ` Daniel Golle
2016-06-19 15:31 ` Richard Weinberger
[not found] ` <5766BAB8.9090707-/L3Ra7n9ekc@public.gmane.org>
2016-06-19 16:13 ` Daniel Golle
2016-06-19 16:53 ` Boris Brezillon
2016-06-19 19:42 ` Daniel Golle
[not found] ` <20160619194236.GA1222-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 20:14 ` Boris Brezillon
2016-06-19 21:48 ` Daniel Golle
[not found] ` <20160619214820.GB1222-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 22:21 ` Hauke Mehrtens
2016-06-20 8:09 ` Arnd Bergmann
2016-06-20 8:26 ` Richard Weinberger
2016-06-20 15:08 ` Arnd Bergmann
2016-06-20 17:24 ` Brian Norris
2016-06-20 19:57 ` Richard Weinberger
[not found] ` <57684AB9.5040607-/L3Ra7n9ekc@public.gmane.org>
2016-06-20 21:18 ` Hauke Mehrtens
2016-06-20 17:05 ` Brian Norris
[not found] ` <20160619152445.GG820-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 15:43 ` Boris Brezillon
2016-06-19 16:23 ` Daniel Golle
2016-06-20 17:03 ` Brian Norris
[not found] ` <48242f26-7812-6957-6bf5-c12989b875b4-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2016-06-18 19:46 ` Richard Weinberger
[not found] ` <5765A4F9.3050107-/L3Ra7n9ekc@public.gmane.org>
2016-06-18 22:54 ` Hauke Mehrtens
[not found] ` <8697ff77-218b-2bc6-5d36-bfb02360d223-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2016-06-19 8:41 ` Richard Weinberger
2016-06-24 18:28 ` Ezequiel Garcia
[not found] ` <CAAEAJfC8mYnohRxKg0b-onbGadk0vPDty=7Bh8emP92mazB=aA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-25 20:20 ` Hauke Mehrtens
[not found] ` <aedd029f-858b-97f8-7fc7-f224999ed24c-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2016-06-25 20:33 ` Richard Weinberger
[not found] ` <1466277476-14853-1-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2016-06-18 19:17 ` [PATCH 2/2] ubi: open volumes define " Hauke Mehrtens
[not found] ` <1466277476-14853-2-git-send-email-hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2016-06-18 23:36 ` Daniel Golle
[not found] ` <20160618233654.GC29476-g5gK2j5usbvCyp4qypjU+w@public.gmane.org>
2016-06-19 21:21 ` Hauke Mehrtens
2016-06-24 18:45 ` Ezequiel Garcia
[not found] ` <CAAEAJfATWJL0fGEcBup+cWzhHUe_4CF4XHWuvp4NYOesmaTyZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-25 20:24 ` Hauke Mehrtens
2016-06-18 23:56 ` [PATCH 1/2] ubi: mount partitions specified " Daniel Golle
2016-06-19 21:36 ` Hauke Mehrtens [this message]
[not found] ` <ecb7950d-9e07-5c5d-6668-1c67ba170ddf-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
2016-06-19 21:52 ` Daniel Golle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ecb7950d-9e07-5c5d-6668-1c67ba170ddf@hauke-m.de \
--to=hauke@hauke-m.de \
--cc=computersforpeace@gmail.com \
--cc=daniel@makrotopia.org \
--cc=dedekind1@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=dwmw2@infradead.org \
--cc=linux-mtd@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=richard@nod.at \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).