| Message ID | 20170209105024.1579-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Commit | 42e9401bd1467d22c4dc4d2c637347b874e6a80b |
| Headers | show |
+Moritz On Thu, 9 Feb 2017 11:50:24 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > The user visible change here is that mtd partitions get an of_node link > in sysfs. The same patch has already been posted last year [1]. Brian, can we take one of these? [1]https://patchwork.ozlabs.org/patch/625978/ > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/mtd/mtdpart.c | 1 + > drivers/mtd/ofpart.c | 1 + > include/linux/mtd/partitions.h | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index fccdd49bb964..d0b919ab85c0 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -424,6 +424,7 @@ static struct mtd_part *allocate_partition(struct mtd_info *master, > slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) ? > &master->dev : > master->dev.parent; > + slave->mtd.dev.of_node = part->of_node; > > slave->mtd._read = part_read; > slave->mtd._write = part_write; > diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c > index ede407d6e106..464470122493 100644 > --- a/drivers/mtd/ofpart.c > +++ b/drivers/mtd/ofpart.c > @@ -108,6 +108,7 @@ static int parse_ofpart_partitions(struct mtd_info *master, > > parts[i].offset = of_read_number(reg, a_cells); > parts[i].size = of_read_number(reg + a_cells, s_cells); > + parts[i].of_node = pp; > > partname = of_get_property(pp, "label", &len); > if (!partname) > diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h > index 70736e1e6c8f..06df1e06b6e0 100644 > --- a/include/linux/mtd/partitions.h > +++ b/include/linux/mtd/partitions.h > @@ -41,6 +41,7 @@ struct mtd_partition { > uint64_t size; /* partition size */ > uint64_t offset; /* offset within the master MTD space */ > uint32_t mask_flags; /* master MTD flags to mask out for this partition */ > + struct device_node *of_node; > }; > > #define MTDPART_OFS_RETAIN (-3)
On Thu, Feb 09, 2017 at 04:34:58PM +0100, Boris Brezillon wrote: > +Moritz > > On Thu, 9 Feb 2017 11:50:24 +0100 > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > The user visible change here is that mtd partitions get an of_node link > > in sysfs. > > The same patch has already been posted last year [1]. > Brian, can we take one of these? > > [1]https://patchwork.ozlabs.org/patch/625978/ Moritz' patch is more lame, it even updates the documentation (ok, one point for Moriz :-) Other than that the only difference is "node" vs. "of_node" (half a point for Sascha) and the position of the assignment in mtdpart.c has a different position (another half point for Sascha). If that would be liked to be seen I can volunteer to create a patch picking the best from both sources. Best regards Uwe
On Thu, 9 Feb 2017 20:39:40 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Thu, Feb 09, 2017 at 04:34:58PM +0100, Boris Brezillon wrote: > > +Moritz > > > > On Thu, 9 Feb 2017 11:50:24 +0100 > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > > The user visible change here is that mtd partitions get an of_node link > > > in sysfs. > > > > The same patch has already been posted last year [1]. > > Brian, can we take one of these? > > > > [1]https://patchwork.ozlabs.org/patch/625978/ > > Moritz' patch is more lame, it even updates the documentation (ok, one > point for Moriz :-) Other than that the only difference is "node" vs. > "of_node" (half a point for Sascha) and the position of the assignment > in mtdpart.c has a different position (another half point for Sascha). > > If that would be liked to be seen I can volunteer to create a patch > picking the best from both sources. Sure, you can also add my ack (which I already put on Moritz patch). BTW, is the of_node link in sysfs the only motivation for this change? I know Moritz had bigger plans (nvmem blocks on top of MTD devices), and I also considered advanced stuff (like per-partition ECC config) which required having a valid ->of_node on slave MTD devices. Thanks, Boris
Hello Boris, On Thu, Feb 09, 2017 at 08:59:22PM +0100, Boris Brezillon wrote: > On Thu, 9 Feb 2017 20:39:40 +0100 > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > On Thu, Feb 09, 2017 at 04:34:58PM +0100, Boris Brezillon wrote: > > > +Moritz > > > > > > On Thu, 9 Feb 2017 11:50:24 +0100 > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > > > > The user visible change here is that mtd partitions get an of_node link > > > > in sysfs. > > > > > > The same patch has already been posted last year [1]. > > > Brian, can we take one of these? > > > > > > [1]https://patchwork.ozlabs.org/patch/625978/ > > > > Moritz' patch is more lame, it even updates the documentation (ok, one > > point for Moriz :-) Other than that the only difference is "node" vs. > > "of_node" (half a point for Sascha) and the position of the assignment > > in mtdpart.c has a different position (another half point for Sascha). > > > > If that would be liked to be seen I can volunteer to create a patch > > picking the best from both sources. > > Sure, you can also add my ack (which I already put on Moritz patch). > BTW, is the of_node link in sysfs the only motivation for this change? > I know Moritz had bigger plans (nvmem blocks on top of MTD devices), and > I also considered advanced stuff (like per-partition ECC config) which > required having a valid ->of_node on slave MTD devices. The motivation for Sascha to create this patch and now me to mainline it, is that we specify some non-volatile state space in dts (to store for example hardware revision, serial number and mac addresses). See http://barebox.org/doc/latest/devicetree/bindings/barebox/barebox,state.html for some details. For the userspace part we read the dtb, something like state { compatible = "barebox,state"; backend = &mtdstatepartition; ... } , and with the symlink introduced by the patch under discussion it gets much simpler to find the device file (in /dev) that contains our state data. Best regards Uwe
On Thu, 9 Feb 2017 21:14:04 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Hello Boris, > > On Thu, Feb 09, 2017 at 08:59:22PM +0100, Boris Brezillon wrote: > > On Thu, 9 Feb 2017 20:39:40 +0100 > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > On Thu, Feb 09, 2017 at 04:34:58PM +0100, Boris Brezillon wrote: > > > > +Moritz > > > > > > > > On Thu, 9 Feb 2017 11:50:24 +0100 > > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > > > > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > > > > > > > The user visible change here is that mtd partitions get an of_node link > > > > > in sysfs. > > > > > > > > The same patch has already been posted last year [1]. > > > > Brian, can we take one of these? > > > > > > > > [1]https://patchwork.ozlabs.org/patch/625978/ > > > > > > Moritz' patch is more lame, it even updates the documentation (ok, one > > > point for Moriz :-) Other than that the only difference is "node" vs. > > > "of_node" (half a point for Sascha) and the position of the assignment > > > in mtdpart.c has a different position (another half point for Sascha). > > > > > > If that would be liked to be seen I can volunteer to create a patch > > > picking the best from both sources. > > > > Sure, you can also add my ack (which I already put on Moritz patch). > > BTW, is the of_node link in sysfs the only motivation for this change? > > I know Moritz had bigger plans (nvmem blocks on top of MTD devices), and > > I also considered advanced stuff (like per-partition ECC config) which > > required having a valid ->of_node on slave MTD devices. > > The motivation for Sascha to create this patch and now me to mainline > it, is that we specify some non-volatile state space in dts (to store > for example hardware revision, serial number and mac addresses). See > http://barebox.org/doc/latest/devicetree/bindings/barebox/barebox,state.html > for some details. > > For the userspace part we read the dtb, something like > > state { > compatible = "barebox,state"; > backend = &mtdstatepartition; > ... > } > > , and with the symlink introduced by the patch under discussion it gets > much simpler to find the device file (in /dev) that contains our state > data. Okay. Looks like an advanced nvmem [1] implementation. Anyway, having the of_node populated for MTD partition devs sounds reasonable, no matter the reason you need that for. [1]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/nvmem/nvmem.txt
On Thu, Feb 09, 2017 at 11:50:24AM +0100, Uwe Kleine-König wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > The user visible change here is that mtd partitions get an of_node link > in sysfs. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Appled to l2-mtd.git
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index fccdd49bb964..d0b919ab85c0 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -424,6 +424,7 @@ static struct mtd_part *allocate_partition(struct mtd_info *master, slave->mtd.dev.parent = IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) ? &master->dev : master->dev.parent; + slave->mtd.dev.of_node = part->of_node; slave->mtd._read = part_read; slave->mtd._write = part_write; diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c index ede407d6e106..464470122493 100644 --- a/drivers/mtd/ofpart.c +++ b/drivers/mtd/ofpart.c @@ -108,6 +108,7 @@ static int parse_ofpart_partitions(struct mtd_info *master, parts[i].offset = of_read_number(reg, a_cells); parts[i].size = of_read_number(reg + a_cells, s_cells); + parts[i].of_node = pp; partname = of_get_property(pp, "label", &len); if (!partname) diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h index 70736e1e6c8f..06df1e06b6e0 100644 --- a/include/linux/mtd/partitions.h +++ b/include/linux/mtd/partitions.h @@ -41,6 +41,7 @@ struct mtd_partition { uint64_t size; /* partition size */ uint64_t offset; /* offset within the master MTD space */ uint32_t mask_flags; /* master MTD flags to mask out for this partition */ + struct device_node *of_node; }; #define MTDPART_OFS_RETAIN (-3)