LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH v3 0/3] mtd concat device driver
@ 2018-09-08 13:13 Bernhard Frauendienst
  2018-09-08 13:13 ` [PATCH v3 1/3] mtd: core: add get_mtd_device_by_node Bernhard Frauendienst
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-08 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland
  Cc: Bernhard Frauendienst, Miquel Raynal, linux-mtd, devicetree,
	linux-kernel

Hi everybody,

my router firmware concatenates two identical flash chips into a single
mtd using mtd_concat_create(), and sets up partitions in a way where one
of them crosses the chip boundary. When porting OpenWRT support for the
board from a mach-file based setup to a device-tree based one, I found
that there is no generic way to create a mtd_concat device from within
the dts. The following patches attempt to provide that possibility.

This is a third roll of that patch series, the first one can be seen at
[1]. The second one [2] was a blunder of my own making. Apologies for
not being able to address the correct recipients two times in a row.

In the first discussion, concerns were raised that a driver for a
"virtual" device like this might have no place in the device tree
system. However, I would argue that specifying a composite device is
very similar to specifying the partitions of a mtd, which can also done
in the device tree. In fact, I believe this is the only way to be able
to specify the partitions of such a concat device in the dts file (but
I'm happy to be corrected if I'm mistaken).
I have made the example in the dt-binding documentation a bit more
expressive in this detail.

In the second roll I have also addressed all issues that reviewers have
brought up so far, hopefully to their satisfaction. These were mainly
whitespace fixes and improved comments.

Best regards,
Bernhard

[1] http://lists.infradead.org/pipermail/linux-mtd/2018-September/083832.html
[2] https://lkml.org/lkml/2018/9/7/1015

Bernhard Frauendienst (3):
  mtd: core: add get_mtd_device_by_node
  dt-bindings: add bindings for mtd-concat devices
  mtd: mtdconcat: add dt driver for concat devices

 .../devicetree/bindings/mtd/mtd-concat.txt    |  36 +++++
 drivers/mtd/Kconfig                           |   2 +
 drivers/mtd/Makefile                          |   3 +
 drivers/mtd/composite/Kconfig                 |  12 ++
 drivers/mtd/composite/Makefile                |   6 +
 drivers/mtd/composite/virt_concat.c           | 128 ++++++++++++++++++
 drivers/mtd/mtdcore.c                         |  38 ++++++
 include/linux/mtd/mtd.h                       |   2 +
 8 files changed, 228 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtd-concat.txt
 create mode 100644 drivers/mtd/composite/Kconfig
 create mode 100644 drivers/mtd/composite/Makefile
 create mode 100644 drivers/mtd/composite/virt_concat.c

-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/3] mtd: core: add get_mtd_device_by_node
  2018-09-08 13:13 [PATCH v3 0/3] mtd concat device driver Bernhard Frauendienst
@ 2018-09-08 13:13 ` Bernhard Frauendienst
  2018-09-08 13:13 ` [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices Bernhard Frauendienst
  2018-09-08 13:13 ` [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices Bernhard Frauendienst
  2 siblings, 0 replies; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-08 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland
  Cc: Bernhard Frauendienst, Miquel Raynal, linux-mtd, devicetree,
	linux-kernel

Add function to retrieve a mtd device by its OF node. Since drivers can
assign arbitrary names to mtd devices in the absence of a label
property, there is no other reliable way to retrieve a mtd device for a
given OF node.

Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/mtdcore.c   | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |  2 ++
 2 files changed, 40 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 97ac219c082e..87dd63926bc9 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -926,6 +926,44 @@ struct mtd_info *get_mtd_device_nm(const char *name)
 }
 EXPORT_SYMBOL_GPL(get_mtd_device_nm);
 
+/**
+ *	get_mtd_device_by_node - obtain a validated handle for an MTD device
+ *	by of_node
+ *	@of_node: OF node of MTD device to open
+ *
+ *	This function returns MTD device description structure in case of
+ *	success and an error code in case of failure.
+ */
+struct mtd_info *get_mtd_device_by_node(const struct device_node *of_node)
+{
+	int err = -ENODEV;
+	struct mtd_info *mtd = NULL, *other;
+
+	mutex_lock(&mtd_table_mutex);
+
+	mtd_for_each_device(other) {
+		if (of_node == other->dev.of_node) {
+			mtd = other;
+			break;
+		}
+	}
+
+	if (!mtd)
+		goto out_unlock;
+
+	err = __get_mtd_device(mtd);
+	if (err)
+		goto out_unlock;
+
+	mutex_unlock(&mtd_table_mutex);
+	return mtd;
+
+out_unlock:
+	mutex_unlock(&mtd_table_mutex);
+	return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(get_mtd_device_by_node);
+
 void put_mtd_device(struct mtd_info *mtd)
 {
 	mutex_lock(&mtd_table_mutex);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index cd0be91bdefa..fe71358f8eaa 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -570,6 +570,8 @@ extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
 extern int __get_mtd_device(struct mtd_info *mtd);
 extern void __put_mtd_device(struct mtd_info *mtd);
 extern struct mtd_info *get_mtd_device_nm(const char *name);
+extern struct mtd_info *get_mtd_device_by_node(
+		const struct device_node *of_node);
 extern void put_mtd_device(struct mtd_info *mtd);
 
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices
  2018-09-08 13:13 [PATCH v3 0/3] mtd concat device driver Bernhard Frauendienst
  2018-09-08 13:13 ` [PATCH v3 1/3] mtd: core: add get_mtd_device_by_node Bernhard Frauendienst
@ 2018-09-08 13:13 ` Bernhard Frauendienst
  2018-09-10 10:51   ` Rafał Miłecki
  2018-09-08 13:13 ` [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices Bernhard Frauendienst
  2 siblings, 1 reply; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-08 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland
  Cc: Bernhard Frauendienst, Miquel Raynal, linux-mtd, devicetree,
	linux-kernel

Document virtual mtd-concat device bindings.

Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
---
 .../devicetree/bindings/mtd/mtd-concat.txt    | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtd-concat.txt

diff --git a/Documentation/devicetree/bindings/mtd/mtd-concat.txt b/Documentation/devicetree/bindings/mtd/mtd-concat.txt
new file mode 100644
index 000000000000..2daf3157b163
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtd-concat.txt
@@ -0,0 +1,36 @@
+Virtual MTD concat device
+
+Requires properties:
+- devices: list of phandles to mtd nodes that should be concatenated
+
+Example:
+
+&spi {
+	flash0: flash@0 {
+		...
+	};
+	flash1: flash@1 {
+		...
+	};
+};
+
+flash {
+	compatible = "mtd-concat";
+
+	devices = <&flash0 &flash1>;
+
+	partitions {
+		compatible = "fixed-partitions";
+
+		partition@0 {
+			label = "boot";
+			reg = <0x0000000 0x0040000>;
+			read-only;
+		};
+
+		partition@40000 {
+			label = "firmware";
+			reg = <0x0040000 0x1fc0000>;
+		};
+	}
+}
-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices
  2018-09-08 13:13 [PATCH v3 0/3] mtd concat device driver Bernhard Frauendienst
  2018-09-08 13:13 ` [PATCH v3 1/3] mtd: core: add get_mtd_device_by_node Bernhard Frauendienst
  2018-09-08 13:13 ` [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices Bernhard Frauendienst
@ 2018-09-08 13:13 ` Bernhard Frauendienst
  2018-09-10 21:47   ` Rob Herring
  2 siblings, 1 reply; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-08 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland
  Cc: Bernhard Frauendienst, Miquel Raynal, linux-mtd, devicetree,
	linux-kernel

Some mtd drivers like physmap variants have support for concatenating
multiple mtd devices, but there is no generic way to define such a
concat device from within the device tree.

This is useful for some SoC boards that use multiple flash chips as
memory banks of a single mtd device, with partitions spanning chip
borders.

This commit adds a driver for creating virtual mtd-concat devices. They
must have a compatible = "mtd-concat" line, and define a list of devices
to concat in the 'devices' property, for example:

flash {
  compatible = "mtd-concat";

  devices = <&flash0 &flash1>;

  partitions {
    ...
  };
};

The driver is added to the very end of the mtd Makefile to increase the
likelyhood of all child devices already being loaded at the time of
probing, preventing unnecessary deferred probes.

Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
---
 drivers/mtd/Kconfig                 |   2 +
 drivers/mtd/Makefile                |   3 +
 drivers/mtd/composite/Kconfig       |  12 +++
 drivers/mtd/composite/Makefile      |   7 ++
 drivers/mtd/composite/virt_concat.c | 128 ++++++++++++++++++++++++++++
 5 files changed, 152 insertions(+)
 create mode 100644 drivers/mtd/composite/Kconfig
 create mode 100644 drivers/mtd/composite/Makefile
 create mode 100644 drivers/mtd/composite/virt_concat.c

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index c77f537323ec..6345d886d458 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -339,4 +339,6 @@ source "drivers/mtd/spi-nor/Kconfig"
 
 source "drivers/mtd/ubi/Kconfig"
 
+source "drivers/mtd/composite/Kconfig"
+
 endif # MTD
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 93473d215a38..57af7190b063 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -36,3 +36,6 @@ obj-y		+= chips/ lpddr/ maps/ devices/ nand/ tests/
 
 obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor/
 obj-$(CONFIG_MTD_UBI)		+= ubi/
+
+# Composite drivers must be loaded last
+obj-y		+= composite/
diff --git a/drivers/mtd/composite/Kconfig b/drivers/mtd/composite/Kconfig
new file mode 100644
index 000000000000..0490fc0284bb
--- /dev/null
+++ b/drivers/mtd/composite/Kconfig
@@ -0,0 +1,12 @@
+menu "Composite MTD device drivers"
+	depends on MTD!=n
+
+config MTD_VIRT_CONCAT
+	tristate "Virtual concat MTD device"
+	help
+	  This driver allows creation of a virtual MTD concat device, which
+	  concatenates multiple underlying MTD devices to a single device.
+	  This is required by some SoC boards where multiple memory banks are
+	  used as one device with partitions spanning across device boundaries.
+
+endmenu
diff --git a/drivers/mtd/composite/Makefile b/drivers/mtd/composite/Makefile
new file mode 100644
index 000000000000..8421a0a30606
--- /dev/null
+++ b/drivers/mtd/composite/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# linux/drivers/mtd/composite/Makefile
+#
+
+obj-$(CONFIG_MTD_VIRT_CONCAT)   += virt_concat.o
diff --git a/drivers/mtd/composite/virt_concat.c b/drivers/mtd/composite/virt_concat.c
new file mode 100644
index 000000000000..76918d4ef07f
--- /dev/null
+++ b/drivers/mtd/composite/virt_concat.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Virtual concat MTD device driver
+ *
+ * Copyright (C) 2018 Bernhard Frauendienst
+ * Author: Bernhard Frauendienst, kernel@nospam.obeliks.de
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/mtd/concat.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+/*
+ * struct of_virt_concat - platform device driver data.
+ * @cmtd the final mtd_concat device
+ * @num_devices the number of devices in @devices
+ * @devices points to an array of devices already loaded
+ */
+struct of_virt_concat {
+	struct mtd_info	*cmtd;
+	int num_devices;
+	struct mtd_info	**devices;
+};
+
+static int virt_concat_remove(struct platform_device *pdev)
+{
+	struct of_virt_concat *info;
+	int i;
+
+	info = platform_get_drvdata(pdev);
+	if (!info)
+		return 0;
+
+	// unset data for when this is called after a probe error
+	platform_set_drvdata(pdev, NULL);
+
+	if (info->cmtd) {
+		mtd_device_unregister(info->cmtd);
+		mtd_concat_destroy(info->cmtd);
+	}
+
+	if (info->devices) {
+		for (i = 0; i < info->num_devices; i++)
+			put_mtd_device(info->devices[i]);
+	}
+
+	return 0;
+}
+
+static int virt_concat_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct of_phandle_iterator it;
+	struct of_virt_concat *info;
+	struct mtd_info *mtd;
+	int err = 0, count;
+
+	count = of_count_phandle_with_args(node, "devices", NULL);
+	if (count <= 0)
+		return -EINVAL;
+
+	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->devices = devm_kcalloc(&pdev->dev, count,
+				     sizeof(*(info->devices)), GFP_KERNEL);
+	if (!info->devices) {
+		err = -ENOMEM;
+		goto err_remove;
+	}
+
+	platform_set_drvdata(pdev, info);
+
+	of_for_each_phandle(&it, err, node, "devices", NULL, 0) {
+		mtd = get_mtd_device_by_node(it.node);
+		if (IS_ERR(mtd)) {
+			of_node_put(it.node);
+			err = -EPROBE_DEFER;
+			goto err_remove;
+		}
+
+		info->devices[info->num_devices++] = mtd;
+	}
+
+	info->cmtd = mtd_concat_create(info->devices, info->num_devices,
+				       dev_name(&pdev->dev));
+	if (!info->cmtd) {
+		err = -ENXIO;
+		goto err_remove;
+	}
+
+	info->cmtd->dev.parent = &pdev->dev;
+	mtd_set_of_node(info->cmtd, node);
+	mtd_device_register(info->cmtd, NULL, 0);
+
+	return 0;
+
+err_remove:
+	virt_concat_remove(pdev);
+
+	return err;
+}
+
+static const struct of_device_id virt_concat_of_match[] = {
+	{ .compatible = "mtd-concat", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, virt_concat_of_match);
+
+static struct platform_driver virt_concat_driver = {
+	.probe = virt_concat_probe,
+	.remove = virt_concat_remove,
+	.driver	 = {
+		.name   = "virt-mtdconcat",
+		.of_match_table = virt_concat_of_match,
+	},
+};
+
+module_platform_driver(virt_concat_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Bernhard Frauendienst <kernel@nospam.obeliks.de>");
+MODULE_DESCRIPTION("Virtual concat MTD device driver");
-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices
  2018-09-08 13:13 ` [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices Bernhard Frauendienst
@ 2018-09-10 10:51   ` Rafał Miłecki
  2018-09-10 12:13     ` Bernhard Frauendienst
  0 siblings, 1 reply; 9+ messages in thread
From: Rafał Miłecki @ 2018-09-10 10:51 UTC (permalink / raw)
  To: kernel
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland, devicetree,
	linux-mtd, Linux Kernel Mailing List, Miquel Raynal

On Sat, 8 Sep 2018 at 15:14, Bernhard Frauendienst
<kernel@nospam.obeliks.de> wrote:
>
> Document virtual mtd-concat device bindings.
>
> Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> ---
>  .../devicetree/bindings/mtd/mtd-concat.txt    | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/mtd-concat.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtd-concat.txt b/Documentation/devicetree/bindings/mtd/mtd-concat.txt
> new file mode 100644
> index 000000000000..2daf3157b163
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtd-concat.txt
> @@ -0,0 +1,36 @@
> +Virtual MTD concat device

I think you forgot to include the documentation. Please ***describe***
that binding.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices
  2018-09-10 10:51   ` Rafał Miłecki
@ 2018-09-10 12:13     ` Bernhard Frauendienst
  2018-09-10 12:17       ` Bernhard Frauendienst
  0 siblings, 1 reply; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-10 12:13 UTC (permalink / raw)
  To: Rafał Miłecki, kernel
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland, devicetree,
	linux-mtd, Linux Kernel Mailing List, Miquel Raynal

Am 10.09.2018 um 12:51 schrieb Rafał Miłecki:
> On Sat, 8 Sep 2018 at 15:14, Bernhard Frauendienst  > <kernel@nospam.obeliks.de> wrote: >> >> Document virtual mtd-concat 
device bindings. >> >> Signed-off-by: Bernhard Frauendienst 
<kernel@nospam.obeliks.de> >> --- >> 
.../devicetree/bindings/mtd/mtd-concat.txt | 36 +++++++++++++++++++ >> 1 
file changed, 36 insertions(+) >> create mode 100644 
Documentation/devicetree/bindings/mtd/mtd-concat.txt >> >> diff --git 
a/Documentation/devicetree/bindings/mtd/mtd-concat.txt 
b/Documentation/devicetree/bindings/mtd/mtd-concat.txt >> new file mode 
100644 >> index 000000000000..2daf3157b163 >> --- /dev/null >> +++ 
b/Documentation/devicetree/bindings/mtd/mtd-concat.txt >> @@ -0,0 +1,36 
@@ >> +Virtual MTD concat device > > I think you forgot to include the 
documentation. Please ***describe*** > that binding.
You are right, I'm afraid I was mislead by some of the rather sparse
documentation files in the mtd directory.
Would something along these lines be sufficient?

--snip--
Virtual MTD concat device

A virtual composite device that concatenates multiple child devices into a
single mtd device. The child mtd nodes must also be defined in the tree and
are referenced using phandles.

Required properties:
- compatible : must be "mtd-concat"
- devices : list of phandles to mtd nodes in the order they should be
         concatenated

The device tree may optionally contain sub-nodes describing partitions 
of the
address space. See partition.txt for more detail.


Example:
--snip--

Regards,
Bernhard


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices
  2018-09-10 12:13     ` Bernhard Frauendienst
@ 2018-09-10 12:17       ` Bernhard Frauendienst
  0 siblings, 0 replies; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-10 12:17 UTC (permalink / raw)
  To: Bernhard Frauendienst, Rafał Miłecki
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland, devicetree,
	linux-mtd, Linux Kernel Mailing List, Miquel Raynal

Am 10.09.2018 um 14:13 schrieb Bernhard Frauendienst:
> Am 10.09.2018 um 12:51 schrieb Rafał Miłecki:  >> I think you forgot to include the documentation. Please 
***describe*** >> that binding. > You are right, I'm afraid I was 
mislead by some of the rather sparse > documentation files in the mtd 
directory. > Would something along these lines be sufficient?
I'm sorry, I have no idea why my Thunderbird+External editor insists on
breaking quotes like this. I will switch to a different mailer for the 
mailing
list.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices
  2018-09-08 13:13 ` [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices Bernhard Frauendienst
@ 2018-09-10 21:47   ` Rob Herring
  2018-09-14 13:42     ` Bernhard Frauendienst
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2018-09-10 21:47 UTC (permalink / raw)
  To: kernel
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vašut,
	Richard Weinberger, Mark Rutland, Miquèl Raynal,
	MTD Maling List, devicetree, linux-kernel

On Sat, Sep 8, 2018 at 8:20 AM Bernhard Frauendienst
<kernel@nospam.obeliks.de> wrote:
>
> Some mtd drivers like physmap variants have support for concatenating
> multiple mtd devices, but there is no generic way to define such a
> concat device from within the device tree.

That is the generic way, but I suppose that only works for parallel
memory mapped devices. So is it just spi-nor that you need to support?
Can we just make reg take multiple chip selects just like the existing
support takes multiple reg entries?

> This is useful for some SoC boards that use multiple flash chips as
> memory banks of a single mtd device, with partitions spanning chip
> borders.
>
> This commit adds a driver for creating virtual mtd-concat devices. They
> must have a compatible = "mtd-concat" line, and define a list of devices
> to concat in the 'devices' property, for example:
>
> flash {
>   compatible = "mtd-concat";
>
>   devices = <&flash0 &flash1>;
>
>   partitions {
>     ...
>   };
> };
>
> The driver is added to the very end of the mtd Makefile to increase the
> likelyhood of all child devices already being loaded at the time of
> probing, preventing unnecessary deferred probes.
>
> Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> ---
>  drivers/mtd/Kconfig                 |   2 +
>  drivers/mtd/Makefile                |   3 +
>  drivers/mtd/composite/Kconfig       |  12 +++
>  drivers/mtd/composite/Makefile      |   7 ++
>  drivers/mtd/composite/virt_concat.c | 128 ++++++++++++++++++++++++++++
>  5 files changed, 152 insertions(+)
>  create mode 100644 drivers/mtd/composite/Kconfig
>  create mode 100644 drivers/mtd/composite/Makefile
>  create mode 100644 drivers/mtd/composite/virt_concat.c
>
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index c77f537323ec..6345d886d458 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -339,4 +339,6 @@ source "drivers/mtd/spi-nor/Kconfig"
>
>  source "drivers/mtd/ubi/Kconfig"
>
> +source "drivers/mtd/composite/Kconfig"
> +
>  endif # MTD
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 93473d215a38..57af7190b063 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -36,3 +36,6 @@ obj-y         += chips/ lpddr/ maps/ devices/ nand/ tests/
>
>  obj-$(CONFIG_MTD_SPI_NOR)      += spi-nor/
>  obj-$(CONFIG_MTD_UBI)          += ubi/
> +
> +# Composite drivers must be loaded last
> +obj-y          += composite/
> diff --git a/drivers/mtd/composite/Kconfig b/drivers/mtd/composite/Kconfig
> new file mode 100644
> index 000000000000..0490fc0284bb
> --- /dev/null
> +++ b/drivers/mtd/composite/Kconfig
> @@ -0,0 +1,12 @@
> +menu "Composite MTD device drivers"
> +       depends on MTD!=n
> +
> +config MTD_VIRT_CONCAT
> +       tristate "Virtual concat MTD device"
> +       help
> +         This driver allows creation of a virtual MTD concat device, which
> +         concatenates multiple underlying MTD devices to a single device.
> +         This is required by some SoC boards where multiple memory banks are
> +         used as one device with partitions spanning across device boundaries.
> +
> +endmenu
> diff --git a/drivers/mtd/composite/Makefile b/drivers/mtd/composite/Makefile
> new file mode 100644
> index 000000000000..8421a0a30606
> --- /dev/null
> +++ b/drivers/mtd/composite/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# linux/drivers/mtd/composite/Makefile
> +#
> +
> +obj-$(CONFIG_MTD_VIRT_CONCAT)   += virt_concat.o
> diff --git a/drivers/mtd/composite/virt_concat.c b/drivers/mtd/composite/virt_concat.c
> new file mode 100644
> index 000000000000..76918d4ef07f
> --- /dev/null
> +++ b/drivers/mtd/composite/virt_concat.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Virtual concat MTD device driver
> + *
> + * Copyright (C) 2018 Bernhard Frauendienst
> + * Author: Bernhard Frauendienst, kernel@nospam.obeliks.de
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/mtd/concat.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/slab.h>
> +
> +/*
> + * struct of_virt_concat - platform device driver data.
> + * @cmtd the final mtd_concat device
> + * @num_devices the number of devices in @devices
> + * @devices points to an array of devices already loaded
> + */
> +struct of_virt_concat {
> +       struct mtd_info *cmtd;
> +       int num_devices;
> +       struct mtd_info **devices;
> +};
> +
> +static int virt_concat_remove(struct platform_device *pdev)
> +{
> +       struct of_virt_concat *info;
> +       int i;
> +
> +       info = platform_get_drvdata(pdev);
> +       if (!info)
> +               return 0;
> +
> +       // unset data for when this is called after a probe error
> +       platform_set_drvdata(pdev, NULL);
> +
> +       if (info->cmtd) {
> +               mtd_device_unregister(info->cmtd);
> +               mtd_concat_destroy(info->cmtd);
> +       }
> +
> +       if (info->devices) {
> +               for (i = 0; i < info->num_devices; i++)
> +                       put_mtd_device(info->devices[i]);
> +       }
> +
> +       return 0;
> +}
> +
> +static int virt_concat_probe(struct platform_device *pdev)
> +{
> +       struct device_node *node = pdev->dev.of_node;
> +       struct of_phandle_iterator it;
> +       struct of_virt_concat *info;
> +       struct mtd_info *mtd;
> +       int err = 0, count;
> +
> +       count = of_count_phandle_with_args(node, "devices", NULL);
> +       if (count <= 0)
> +               return -EINVAL;
> +
> +       info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +       if (!info)
> +               return -ENOMEM;
> +       info->devices = devm_kcalloc(&pdev->dev, count,
> +                                    sizeof(*(info->devices)), GFP_KERNEL);
> +       if (!info->devices) {
> +               err = -ENOMEM;
> +               goto err_remove;
> +       }
> +
> +       platform_set_drvdata(pdev, info);
> +
> +       of_for_each_phandle(&it, err, node, "devices", NULL, 0) {
> +               mtd = get_mtd_device_by_node(it.node);
> +               if (IS_ERR(mtd)) {
> +                       of_node_put(it.node);
> +                       err = -EPROBE_DEFER;
> +                       goto err_remove;
> +               }
> +
> +               info->devices[info->num_devices++] = mtd;
> +       }
> +
> +       info->cmtd = mtd_concat_create(info->devices, info->num_devices,
> +                                      dev_name(&pdev->dev));
> +       if (!info->cmtd) {
> +               err = -ENXIO;
> +               goto err_remove;
> +       }
> +
> +       info->cmtd->dev.parent = &pdev->dev;
> +       mtd_set_of_node(info->cmtd, node);
> +       mtd_device_register(info->cmtd, NULL, 0);
> +
> +       return 0;
> +
> +err_remove:
> +       virt_concat_remove(pdev);
> +
> +       return err;
> +}
> +
> +static const struct of_device_id virt_concat_of_match[] = {
> +       { .compatible = "mtd-concat", },
> +       { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, virt_concat_of_match);
> +
> +static struct platform_driver virt_concat_driver = {
> +       .probe = virt_concat_probe,
> +       .remove = virt_concat_remove,
> +       .driver  = {
> +               .name   = "virt-mtdconcat",
> +               .of_match_table = virt_concat_of_match,
> +       },
> +};
> +
> +module_platform_driver(virt_concat_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Bernhard Frauendienst <kernel@nospam.obeliks.de>");
> +MODULE_DESCRIPTION("Virtual concat MTD device driver");
> --
> 2.18.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices
  2018-09-10 21:47   ` Rob Herring
@ 2018-09-14 13:42     ` Bernhard Frauendienst
  0 siblings, 0 replies; 9+ messages in thread
From: Bernhard Frauendienst @ 2018-09-14 13:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vašut,
	Richard Weinberger, Mark Rutland, Miquèl Raynal,
	MTD Maling List, devicetree, linux-kernel

Hi Rob,

sorry it took me so long to reply, I had to set up mutt first ;)

On Mon, Sep 10, 2018 at 04:47:22PM -0500, Rob Herring wrote:
> On Sat, Sep 8, 2018 at 8:20 AM Bernhard Frauendienst
> <kernel@nospam.obeliks.de> wrote:
> >
> > Some mtd drivers like physmap variants have support for concatenating
> > multiple mtd devices, but there is no generic way to define such a
> > concat device from within the device tree.
> 
> That is the generic way, but I suppose that only works for parallel
> memory mapped devices. So is it just spi-nor that you need to support?

I'm no expert in this domain, or regarding OpenWRT, but as far as I can
tell this only concerns spi-nor devices. It definitely does in my
specific use-case.

> Can we just make reg take multiple chip selects just like the existing
> support takes multiple reg entries?

That sounds like an interesting idea, but I'm afraid it's not a trivial
change, as far as I can tell from looking at spi.c. 

The 'reg' property is currently read in of_spi_parse_dt which gets
passed the spi device created in of_register_spi_device. The latter
would have to create multiple devices from a single OF node, and pass
the chipselect value to of_spi_parse_dt, which would have to parse the
same node multiple times, which feels a bit strange. To make things
worse, of_register_spi_device returns the created device, although that
value seems unused in the current tree (except for error checks).

Am I correct to assume that this change in method signature could be
accteptable since both functions are not exported?

Regards,
Bernhard

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-08 13:13 [PATCH v3 0/3] mtd concat device driver Bernhard Frauendienst
2018-09-08 13:13 ` [PATCH v3 1/3] mtd: core: add get_mtd_device_by_node Bernhard Frauendienst
2018-09-08 13:13 ` [PATCH v3 2/3] dt-bindings: add bindings for mtd-concat devices Bernhard Frauendienst
2018-09-10 10:51   ` Rafał Miłecki
2018-09-10 12:13     ` Bernhard Frauendienst
2018-09-10 12:17       ` Bernhard Frauendienst
2018-09-08 13:13 ` [PATCH v3 3/3] mtd: mtdconcat: add dt driver for concat devices Bernhard Frauendienst
2018-09-10 21:47   ` Rob Herring
2018-09-14 13:42     ` Bernhard Frauendienst

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox