linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] MTD concat
@ 2019-11-13 17:15 Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol Miquel Raynal
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Miquel Raynal @ 2019-11-13 17:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus
  Cc: Mark Rutland, devicetree, Miquel Raynal, linux-kernel,
	Rob Herring, Paul Kocialkowski, Mark Brown, linux-mtd,
	Thomas Petazzoni, Boris Brezillon

Hello,

A year ago Bernhard Frauendienst started an effort to bring MTD
devices concatenation generic [1]. Today I also need this
concatenation to be possible in order to support configurations where
two MTD devices are treated like one bigger in order to be able to
define partitions across chip boundaries, hence reviving this
patchset.

After having talked with Mark Brown and Boris Brezillon this approach
seems to be the cleanest and easiest one. If discussions need to
happen, it is probably on the dt-bindings file where I tried to
summarize the issue and the possible solutions in the commit log.

I changed a bit the code logic and style but not so much, all the
changes with the 2018 version are in [ ] in the commit logs.

I would like to add another way to concatenate devices: with module
parameters/arguments on the cmdline. I will extend this work once the
bindings will have been discussed and accepted.

Thanks,
Miquèl

[1] https://lwn.net/ml/linux-kernel/20180907173515.19990-1-kernel@nospam.obeliks.de/


Bernhard Frauendienst (3):
  mtd: Add get_mtd_device_by_node() helper
  dt-bindings: mtd: Describe mtd-concat devices
  mtd: Add driver for concatenating devices

Miquel Raynal (1):
  mtd: concat: Fix a comment referring to an unknown symbol

 .../devicetree/bindings/mtd/mtd-concat.yaml   |  56 ++++++++
 drivers/mtd/Kconfig                           |   8 ++
 drivers/mtd/Makefile                          |   1 +
 drivers/mtd/mtd_virt_concat.c                 | 132 ++++++++++++++++++
 drivers/mtd/mtdconcat.c                       |   5 +-
 drivers/mtd/mtdcore.c                         |  38 +++++
 include/linux/mtd/mtd.h                       |   2 +
 7 files changed, 238 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtd-concat.yaml
 create mode 100644 drivers/mtd/mtd_virt_concat.c

-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol
  2019-11-13 17:15 [PATCH v4 0/4] MTD concat Miquel Raynal
@ 2019-11-13 17:15 ` Miquel Raynal
  2020-01-14 17:09   ` Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 2/4] mtd: Add get_mtd_device_by_node() helper Miquel Raynal
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2019-11-13 17:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus
  Cc: Mark Rutland, devicetree, Miquel Raynal, linux-kernel,
	Rob Herring, Paul Kocialkowski, Mark Brown, linux-mtd,
	Thomas Petazzoni, Boris Brezillon

Fix the comment describing what the mtd_concat_destroy() function
does. It referrers to the concat_mtd_devs symbol which has never
existed (at least not since the beginning of the Git era).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/mtdconcat.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index cbc5925e6440..1b6428d6e13d 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -852,10 +852,7 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[],	/* subdevices to c
 	return &concat->mtd;
 }
 
-/*
- * This function destroys an MTD object obtained from concat_mtd_devs()
- */
-
+/* Cleans the context obtained from mtd_concat_create() */
 void mtd_concat_destroy(struct mtd_info *mtd)
 {
 	struct mtd_concat *concat = CONCAT(mtd);
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v4 2/4] mtd: Add get_mtd_device_by_node() helper
  2019-11-13 17:15 [PATCH v4 0/4] MTD concat Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol Miquel Raynal
@ 2019-11-13 17:15 ` Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 4/4] mtd: Add driver for concatenating devices Miquel Raynal
  3 siblings, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2019-11-13 17:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus
  Cc: Mark Rutland, devicetree, Bernhard Frauendienst, Miquel Raynal,
	linux-kernel, Rob Herring, Paul Kocialkowski, Mark Brown,
	linux-mtd, Thomas Petazzoni, Boris Brezillon

From: Bernhard Frauendienst <kernel@nospam.obeliks.de>

Add an helper to retrieve a MTD device by its OF node. Since drivers can
assign arbitrary names to MTD devices in the absence of a 'label' DT
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>
[<miquel.raynal@bootlin.com>: light internals rework]
Signed-off-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 76b4264936ff..5a94a2c0a6de 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -985,6 +985,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 an MTD device structure in case of success,
+ *	an error code otherwise.
+ */
+struct mtd_info *get_mtd_device_by_node(const struct device_node *of_node)
+{
+	struct mtd_info *mtd;
+	bool found = false;
+	int ret;
+
+	mutex_lock(&mtd_table_mutex);
+
+	mtd_for_each_device(mtd) {
+		if (of_node == mtd->dev.of_node) {
+			found = true;
+			break;
+		}
+	}
+
+	if (found)
+		ret = __get_mtd_device(mtd);
+
+	mutex_unlock(&mtd_table_mutex);
+
+	if (!found)
+		return ERR_PTR(-ENODEV);
+
+	if (ret)
+		return ERR_PTR(ret);
+
+	return mtd;
+}
+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 677768b21a1d..0f25c476a1a3 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -573,6 +573,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.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices
  2019-11-13 17:15 [PATCH v4 0/4] MTD concat Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol Miquel Raynal
  2019-11-13 17:15 ` [PATCH v4 2/4] mtd: Add get_mtd_device_by_node() helper Miquel Raynal
@ 2019-11-13 17:15 ` Miquel Raynal
  2019-11-18 22:13   ` Rob Herring
  2019-11-13 17:15 ` [PATCH v4 4/4] mtd: Add driver for concatenating devices Miquel Raynal
  3 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2019-11-13 17:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus
  Cc: Mark Rutland, devicetree, Bernhard Frauendienst, Miquel Raynal,
	linux-kernel, Rob Herring, Paul Kocialkowski, Mark Brown,
	linux-mtd, Thomas Petazzoni, Boris Brezillon

From: Bernhard Frauendienst <kernel@nospam.obeliks.de>

The main use case to concatenate MTD devices is probably SPI-NOR
flashes where the number of address bits is limited to 24, which can
access a range of 16MiB. Board manufacturers might want to double the
SPI storage size by adding a second flash asserted thanks to a second
chip selects which enhances the addressing capabilities to 25 bits,
32MiB. Having two devices for twice the size is great but without more
glue, we cannot define partition boundaries spread across the two
devices. This is the gap mtd-concat intends to address.

There are two options to describe concatenated devices:
1/ One flash chip is described in the DT with two CS;
2/ Two flash chips are described in the DT with one CS each, a virtual
device is also created to describe the concatenation.

Solution 1/ presents at least 3 issues:
* The hardware description is abused;
* The concatenation only works for SPI devices (while it could be
  helpful for any MTD);
* It would require a lot of rework in the SPI core as most of the
  logic assumes there is and there always will be only one CS per
  chip.

Solution 2/ also has caveats:
* The virtual device has no hardware reality;
* Possible optimizations at the hardware level will be hard to enable
  efficiently (ie. a common direct mapping abstracted by a SPI
  memories oriented controller).

There is no easy and perfect answer to this need but it feels more
reasonable to address the problem with solution 2, with the
information/needs we have today.

Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
[<miquel.raynal@bootlin.com>:
Wrote a commit message explaining what mtd-concat is.
Explained the implementation details.
Switched to yaml schema.]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/mtd/mtd-concat.yaml   | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtd-concat.yaml

diff --git a/Documentation/devicetree/bindings/mtd/mtd-concat.yaml b/Documentation/devicetree/bindings/mtd/mtd-concat.yaml
new file mode 100644
index 000000000000..7341198575cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtd-concat.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mtd-concat.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual MTD concatenation device bindings
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    const: "mtd-concat"
+
+  devices:
+    minItems: 2
+    description: |
+      List of phandles to MTD nodes that should be concatenated (in
+      order).
+
+required:
+  - compatible
+  - devices
+
+examples:
+  - |
+    &spi {
+            flash0: flash@0 {
+	            reg = <0>;
+            };
+            flash1: flash@1 {
+	            reg = <1>;
+            };
+    };
+
+    flash {
+            compatible = "mtd-concat";
+            devices = <&flash0 &flash1>;
+
+            partitions {
+                    compatible = "fixed-partitions";
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+
+                    partition@0 {
+                            label = "boot";
+                            reg = <0x0000000 0x0040000>;
+                            read-only;
+                    };
+                    partition@40000 {
+                            label = "firmware";
+                            reg = <0x0040000 0x1fc0000>;
+                    };
+            };
+    };
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v4 4/4] mtd: Add driver for concatenating devices
  2019-11-13 17:15 [PATCH v4 0/4] MTD concat Miquel Raynal
                   ` (2 preceding siblings ...)
  2019-11-13 17:15 ` [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices Miquel Raynal
@ 2019-11-13 17:15 ` Miquel Raynal
  3 siblings, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2019-11-13 17:15 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus
  Cc: Mark Rutland, devicetree, Bernhard Frauendienst, Miquel Raynal,
	linux-kernel, Rob Herring, Paul Kocialkowski, Mark Brown,
	linux-mtd, Thomas Petazzoni, Boris Brezillon

From: Bernhard Frauendienst <kernel@nospam.obeliks.de>

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

This is useful for boards where memory range has been extended with
the use of multiple flash chips as memory banks of a single MTD
device, with partitions spanning chip borders.

Add a driver for creating virtual mtd-concat devices. They must have
the "mtd-concat" compatible, and define a list of 'devices' to
concatenate, ie:

        flash {
                compatible = "mtd-concat";
                devices = <&flash0 &flash1>;

                partitions {
                        ...
                };
        };

Signed-off-by: Bernhard Frauendienst <kernel@nospam.obeliks.de>
[<miquel.raynal@bootlin.com>:
Reword commit message a bit.
Use the word 'virtual' instead of 'composite'.
Do not probe the virtual device last: SPI is after MTD anyway.
Change the driver's location.
Update the driver logic and coding style.]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/Kconfig           |   8 +++
 drivers/mtd/Makefile          |   1 +
 drivers/mtd/mtd_virt_concat.c | 132 ++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 drivers/mtd/mtd_virt_concat.c

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 79a8ff542883..3e1e55e7158f 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -276,6 +276,14 @@ config MTD_PARTITIONED_MASTER
 	  the parent of the partition device be the master device, rather than
 	  what lies behind the master.
 
+config MTD_VIRT_CONCAT
+	tristate "Virtual concatenated MTD devices"
+	help
+	  This driver allows creation of a virtual MTD device, which
+	  concatenates multiple physical MTD devices into a single one.
+	  This is useful to create partitions bigger than the underlying
+	  physical chips by allowing cross-chip boundaries.
+
 source "drivers/mtd/chips/Kconfig"
 
 source "drivers/mtd/maps/Kconfig"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 58fc327a5276..c7ee13368a66 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_SSFDC)		+= ssfdc.o
 obj-$(CONFIG_SM_FTL)		+= sm_ftl.o
 obj-$(CONFIG_MTD_OOPS)		+= mtdoops.o
 obj-$(CONFIG_MTD_SWAP)		+= mtdswap.o
+obj-$(CONFIG_MTD_VIRT_CONCAT)	+= mtd_virt_concat.o
 
 nftl-objs		:= nftlcore.o nftlmount.o
 inftl-objs		:= inftlcore.o inftlmount.o
diff --git a/drivers/mtd/mtd_virt_concat.c b/drivers/mtd/mtd_virt_concat.c
new file mode 100644
index 000000000000..d184c58f7e09
--- /dev/null
+++ b/drivers/mtd/mtd_virt_concat.c
@@ -0,0 +1,132 @@
+// 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 mtd_virt_concat - platform device driver data.
+ * @vmtd: Virtual mtd_concat device
+ * @count: Number of physical underlaying devices in @devices
+ * @devices: Array of the physical devices used
+ */
+struct mtd_virt_concat {
+	struct mtd_info	*vmtd;
+	unsigned int count;
+	struct mtd_info	**devices;
+};
+
+static void mtd_virt_concat_put_devices(struct mtd_virt_concat *concat)
+{
+	int i;
+
+	for (i = 0; i < concat->count; i++)
+		put_mtd_device(concat->devices[i]);
+}
+
+static int mtd_virt_concat_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct mtd_virt_concat *concat;
+	struct of_phandle_iterator it;
+	struct mtd_info *mtd;
+	int ret, count;
+
+	count = of_count_phandle_with_args(node, "devices", NULL);
+	if (count < 2) {
+		dev_err(dev, "minimum 2 devices, given: %d\n", count);
+		return -EINVAL;
+	}
+
+	concat = devm_kzalloc(dev, sizeof(*concat), GFP_KERNEL);
+	if (!concat)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, concat);
+
+	concat->devices = devm_kcalloc(dev, count, sizeof(*concat->devices),
+				       GFP_KERNEL);
+	if (!concat->devices)
+		return -ENOMEM;
+
+	/* Aggregate the physical devices */
+	of_for_each_phandle(&it, ret, node, "devices", NULL, 0) {
+		mtd = get_mtd_device_by_node(it.node);
+		if (IS_ERR(mtd)) {
+			ret = -EPROBE_DEFER;
+			goto put_mtd_devices;
+		}
+
+		concat->devices[concat->count++] = mtd;
+	}
+
+	/* Create the virtual device */
+	concat->vmtd = mtd_concat_create(concat->devices, concat->count,
+					 dev_name(dev));
+	if (!concat->vmtd) {
+		ret = -ENXIO;
+		goto put_mtd_devices;
+	}
+
+	concat->vmtd->dev.parent = dev;
+	mtd_set_of_node(concat->vmtd, node);
+
+	/* Register the platform device */
+	ret = mtd_device_register(concat->vmtd, NULL, 0);
+	if (ret)
+		goto destroy_concat;
+
+	return 0;
+
+destroy_concat:
+	mtd_concat_destroy(concat->vmtd);
+put_mtd_devices:
+	mtd_virt_concat_put_devices(concat);
+
+	return ret;
+}
+
+static int mtd_virt_concat_remove(struct platform_device *pdev)
+{
+	struct mtd_virt_concat *concat = platform_get_drvdata(pdev);
+
+	mtd_device_unregister(concat->vmtd);
+	mtd_concat_destroy(concat->vmtd);
+	mtd_virt_concat_put_devices(concat);
+
+	return 0;
+}
+
+static const struct of_device_id mtd_virt_concat_of_match[] = {
+	{
+		.compatible = "mtd-concat",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtd_virt_concat_of_match);
+
+static struct platform_driver mtd_virt_concat_driver = {
+	.probe = mtd_virt_concat_probe,
+	.remove = mtd_virt_concat_remove,
+	.driver	 = {
+		.name   = "mtd-virt-concat",
+		.of_match_table = mtd_virt_concat_of_match,
+	},
+};
+module_platform_driver(mtd_virt_concat_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Bernhard Frauendienst <kernel@nospam.obeliks.de>");
+MODULE_DESCRIPTION("Virtual concat MTD device driver");
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices
  2019-11-13 17:15 ` [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices Miquel Raynal
@ 2019-11-18 22:13   ` Rob Herring
  2019-11-25 14:15     ` Miquel Raynal
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2019-11-18 22:13 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-kernel, Paul Kocialkowski, Mark Brown,
	linux-mtd, Bernhard Frauendienst, Thomas Petazzoni,
	Boris Brezillon

On Wed, Nov 13, 2019 at 06:15:04PM +0100, Miquel Raynal wrote:
> From: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> 
> The main use case to concatenate MTD devices is probably SPI-NOR
> flashes where the number of address bits is limited to 24, which can
> access a range of 16MiB. Board manufacturers might want to double the
> SPI storage size by adding a second flash asserted thanks to a second
> chip selects which enhances the addressing capabilities to 25 bits,
> 32MiB. Having two devices for twice the size is great but without more
> glue, we cannot define partition boundaries spread across the two
> devices. This is the gap mtd-concat intends to address.
> 
> There are two options to describe concatenated devices:
> 1/ One flash chip is described in the DT with two CS;
> 2/ Two flash chips are described in the DT with one CS each, a virtual
> device is also created to describe the concatenation.
> 
> Solution 1/ presents at least 3 issues:
> * The hardware description is abused;
> * The concatenation only works for SPI devices (while it could be
>   helpful for any MTD);
> * It would require a lot of rework in the SPI core as most of the
>   logic assumes there is and there always will be only one CS per
>   chip.

This seems ok if all the devices are identical.

> Solution 2/ also has caveats:
> * The virtual device has no hardware reality;
> * Possible optimizations at the hardware level will be hard to enable
>   efficiently (ie. a common direct mapping abstracted by a SPI
>   memories oriented controller).

Something like this may be necessary if data is interleaved rather than 
concatinated.


Solution 3
Describe each device and partition separately and add link(s) from one 
partition to the next 

flash0 {
  partitions {
    compatible = "fixed-partitions";
    concat-partition = <&flash1_partitions>;
    ...
  };
};

flash1 {
  flash1_partition: partitions {
    compatible = "fixed-partitions";
    ...
  };
};

Maybe a link back to the previous paritions too or a boolean to mark as 
a continuation.

No idea how well this works or not for the kernel, but that really 
shouldn't matter for the binding design.

Rob

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices
  2019-11-18 22:13   ` Rob Herring
@ 2019-11-25 14:15     ` Miquel Raynal
  2019-12-02 16:26       ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2019-11-25 14:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-kernel, Paul Kocialkowski, Mark Brown,
	linux-mtd, Bernhard Frauendienst, Thomas Petazzoni,
	Boris Brezillon

Hi Rob,

Rob Herring <robh@kernel.org> wrote on Mon, 18 Nov 2019 16:13:41 -0600:

> On Wed, Nov 13, 2019 at 06:15:04PM +0100, Miquel Raynal wrote:
> > From: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> > 
> > The main use case to concatenate MTD devices is probably SPI-NOR
> > flashes where the number of address bits is limited to 24, which can
> > access a range of 16MiB. Board manufacturers might want to double the
> > SPI storage size by adding a second flash asserted thanks to a second
> > chip selects which enhances the addressing capabilities to 25 bits,
> > 32MiB. Having two devices for twice the size is great but without more
> > glue, we cannot define partition boundaries spread across the two
> > devices. This is the gap mtd-concat intends to address.
> > 
> > There are two options to describe concatenated devices:
> > 1/ One flash chip is described in the DT with two CS;
> > 2/ Two flash chips are described in the DT with one CS each, a virtual
> > device is also created to describe the concatenation.
> > 
> > Solution 1/ presents at least 3 issues:
> > * The hardware description is abused;
> > * The concatenation only works for SPI devices (while it could be
> >   helpful for any MTD);
> > * It would require a lot of rework in the SPI core as most of the
> >   logic assumes there is and there always will be only one CS per
> >   chip.  
> 
> This seems ok if all the devices are identical.

This is not an option for Mark and I agree with him as we are faking
the reality: the two devices we want to virtually concatenate may be
two physically different devices. Binding them as one is lying.

> > Solution 2/ also has caveats:
> > * The virtual device has no hardware reality;
> > * Possible optimizations at the hardware level will be hard to enable
> >   efficiently (ie. a common direct mapping abstracted by a SPI
> >   memories oriented controller).  
> 
> Something like this may be necessary if data is interleaved rather than 
> concatinated.

This is something that is gonna happen too, it is called "dual
parallel".

> Solution 3
> Describe each device and partition separately and add link(s) from one 
> partition to the next 
> 
> flash0 {
>   partitions {
>     compatible = "fixed-partitions";
>     concat-partition = <&flash1_partitions>;
>     ...
>   };
> };
> 
> flash1 {
>   flash1_partition: partitions {
>     compatible = "fixed-partitions";
>     ...
>   };
> };

I honestly don't see how this is different as solution 2/? In one case
we describe the partition concatenation in one subnode as a "link", in
the other we create a separate node to describe the link. Are you
strongly opposed as solution 2/? From a pure conceptual point of view,
is it really different than 3/?
 

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices
  2019-11-25 14:15     ` Miquel Raynal
@ 2019-12-02 16:26       ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2019-12-02 16:26 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mark Rutland, devicetree, Vignesh Raghavendra, Tudor Ambarus,
	Richard Weinberger, linux-kernel, Paul Kocialkowski, Mark Brown,
	MTD Maling List, Bernhard Frauendienst, Thomas Petazzoni,
	Boris Brezillon

On Mon, Nov 25, 2019 at 8:15 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Rob,
>
> Rob Herring <robh@kernel.org> wrote on Mon, 18 Nov 2019 16:13:41 -0600:
>
> > On Wed, Nov 13, 2019 at 06:15:04PM +0100, Miquel Raynal wrote:
> > > From: Bernhard Frauendienst <kernel@nospam.obeliks.de>
> > >
> > > The main use case to concatenate MTD devices is probably SPI-NOR
> > > flashes where the number of address bits is limited to 24, which can
> > > access a range of 16MiB. Board manufacturers might want to double the
> > > SPI storage size by adding a second flash asserted thanks to a second
> > > chip selects which enhances the addressing capabilities to 25 bits,
> > > 32MiB. Having two devices for twice the size is great but without more
> > > glue, we cannot define partition boundaries spread across the two
> > > devices. This is the gap mtd-concat intends to address.
> > >
> > > There are two options to describe concatenated devices:
> > > 1/ One flash chip is described in the DT with two CS;
> > > 2/ Two flash chips are described in the DT with one CS each, a virtual
> > > device is also created to describe the concatenation.
> > >
> > > Solution 1/ presents at least 3 issues:
> > > * The hardware description is abused;
> > > * The concatenation only works for SPI devices (while it could be
> > >   helpful for any MTD);
> > > * It would require a lot of rework in the SPI core as most of the
> > >   logic assumes there is and there always will be only one CS per
> > >   chip.
> >
> > This seems ok if all the devices are identical.
>
> This is not an option for Mark and I agree with him as we are faking
> the reality: the two devices we want to virtually concatenate may be
> two physically different devices. Binding them as one is lying.
>
> > > Solution 2/ also has caveats:
> > > * The virtual device has no hardware reality;
> > > * Possible optimizations at the hardware level will be hard to enable
> > >   efficiently (ie. a common direct mapping abstracted by a SPI
> > >   memories oriented controller).
> >
> > Something like this may be necessary if data is interleaved rather than
> > concatinated.
>
> This is something that is gonna happen too, it is called "dual
> parallel".

Then it would be good to think about how that should look. Maybe
there's overlap or maybe not.

> > Solution 3
> > Describe each device and partition separately and add link(s) from one
> > partition to the next
> >
> > flash0 {
> >   partitions {
> >     compatible = "fixed-partitions";
> >     concat-partition = <&flash1_partitions>;
> >     ...
> >   };
> > };
> >
> > flash1 {
> >   flash1_partition: partitions {
> >     compatible = "fixed-partitions";
> >     ...
> >   };
> > };
>
> I honestly don't see how this is different as solution 2/?

It's a single new property rather than a whole binding for a virtual
device. It's a minimal change to the DT. It would work with existing
bootloaders (and other OSs and older kernels) without change except
for the one concatenated partition.

> In one case
> we describe the partition concatenation in one subnode as a "link", in
> the other we create a separate node to describe the link. Are you
> strongly opposed as solution 2/?

I'd prefer to not have virtual devices without good reason.

> From a pure conceptual point of view,
> is it really different than 3/?
>
>
> Thanks,
> Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol
  2019-11-13 17:15 ` [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol Miquel Raynal
@ 2020-01-14 17:09   ` Miquel Raynal
  0 siblings, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2020-01-14 17:09 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus
  Cc: Mark Rutland, devicetree, linux-kernel, Rob Herring,
	Paul Kocialkowski, Mark Brown, linux-mtd, Thomas Petazzoni,
	Boris Brezillon

On Wed, 2019-11-13 at 17:15:02 UTC, Miquel Raynal wrote:
> Fix the comment describing what the mtd_concat_destroy() function
> does. It referrers to the concat_mtd_devs symbol which has never
> existed (at least not since the beginning of the Git era).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2020-01-14 17:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-13 17:15 [PATCH v4 0/4] MTD concat Miquel Raynal
2019-11-13 17:15 ` [PATCH v4 1/4] mtd: concat: Fix a comment referring to an unknown symbol Miquel Raynal
2020-01-14 17:09   ` Miquel Raynal
2019-11-13 17:15 ` [PATCH v4 2/4] mtd: Add get_mtd_device_by_node() helper Miquel Raynal
2019-11-13 17:15 ` [PATCH v4 3/4] dt-bindings: mtd: Describe mtd-concat devices Miquel Raynal
2019-11-18 22:13   ` Rob Herring
2019-11-25 14:15     ` Miquel Raynal
2019-12-02 16:26       ` Rob Herring
2019-11-13 17:15 ` [PATCH v4 4/4] mtd: Add driver for concatenating devices Miquel Raynal

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).