[RESEND v2 1/4] libnvdimm: Add of_node to region and bus descriptors

[RESEND v2 1/4] libnvdimm: Add of_node to region and bus descriptors

[Oliver O'Halloran]

We want to be able to cross reference the region and bus devices
with the device tree node that they were spawned from. libNVDIMM
handles creating the actual devices for these internally, so we
need to pass in a pointer to the relevant node in the descriptor.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/bus.c         | 1 +
 drivers/nvdimm/region_devs.c | 1 +
 include/linux/libnvdimm.h    | 3 +++
 3 files changed, 5 insertions(+)

diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 78eabc3a1ab1..c6106914f396 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -358,6 +358,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
 	nvdimm_bus->dev.release = nvdimm_bus_release;
 	nvdimm_bus->dev.groups = nd_desc->attr_groups;
 	nvdimm_bus->dev.bus = &nvdimm_bus_type;
+	nvdimm_bus->dev.of_node = nd_desc->of_node;
 	dev_set_name(&nvdimm_bus->dev, "ndbus%d", nvdimm_bus->id);
 	rc = device_register(&nvdimm_bus->dev);
 	if (rc) {
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index e6d01911e092..2f1d5771100e 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1005,6 +1005,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
 	dev->parent = &nvdimm_bus->dev;
 	dev->type = dev_type;
 	dev->groups = ndr_desc->attr_groups;
+	dev->of_node = ndr_desc->of_node;
 	nd_region->ndr_size = resource_size(ndr_desc->res);
 	nd_region->ndr_start = ndr_desc->res->start;
 	nd_device_register(dev);
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index ff855ed965fb..f61cb5050297 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -76,12 +76,14 @@ typedef int (*ndctl_fn)(struct nvdimm_bus_descriptor *nd_desc,
 		struct nvdimm *nvdimm, unsigned int cmd, void *buf,
 		unsigned int buf_len, int *cmd_rc);
 
+struct device_node;
 struct nvdimm_bus_descriptor {
 	const struct attribute_group **attr_groups;
 	unsigned long bus_dsm_mask;
 	unsigned long cmd_mask;
 	struct module *module;
 	char *provider_name;
+	struct device_node *of_node;
 	ndctl_fn ndctl;
 	int (*flush_probe)(struct nvdimm_bus_descriptor *nd_desc);
 	int (*clear_to_send)(struct nvdimm_bus_descriptor *nd_desc,
@@ -123,6 +125,7 @@ struct nd_region_desc {
 	int num_lanes;
 	int numa_node;
 	unsigned long flags;
+	struct device_node *of_node;
 };
 
 struct device;
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[RESEND v2 2/4] libnvdimm: Add device-tree based driver

[Oliver O'Halloran]

This patch adds peliminary device-tree bindings for persistent memory
regions. The driver registers a libnvdimm bus for each pmem-region
node and each address range under the node is converted to a region
within that bus.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Made each bus have a separate node rather having a shared bus.
    Renamed to of_pmem rather than of_nvdimm.
    Changed log level of happy-path messages to debug.
---
 MAINTAINERS              |   7 +++
 drivers/nvdimm/Kconfig   |  10 ++++
 drivers/nvdimm/Makefile  |   1 +
 drivers/nvdimm/of_pmem.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 137 insertions(+)
 create mode 100644 drivers/nvdimm/of_pmem.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4e62756936fa..6ef38be700e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8035,6 +8035,13 @@ Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
 F:	drivers/nvdimm/pmem*
 
+LIBNVDIMM: DEVICETREE BINDINGS
+M:	Oliver O'Halloran <oohall@gmail.com>
+L:	linux-nvdimm@lists.01.org
+Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
+S:	Supported
+F:	drivers/nvdimm/of_pmem.c
+
 LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
 M:	Dan Williams <dan.j.williams@intel.com>
 L:	linux-nvdimm@lists.01.org
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index a65f2e1d9f53..2d6862bf7436 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -102,4 +102,14 @@ config NVDIMM_DAX
 
 	  Select Y if unsure
 
+config OF_PMEM
+	tristate "Device-tree support for persistent memory regions"
+	depends on OF
+	default LIBNVDIMM
+	help
+	  Allows regions of persistent memory to be described in the
+	  device-tree.
+
+	  Select Y if unsure.
+
 endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index 70d5f3ad9909..e8847045dac0 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o
 obj-$(CONFIG_ND_BTT) += nd_btt.o
 obj-$(CONFIG_ND_BLK) += nd_blk.o
 obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
+obj-$(CONFIG_OF_PMEM) += of_pmem.o
 
 nd_pmem-y := pmem.o
 
diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
new file mode 100644
index 000000000000..374c796ea1de
--- /dev/null
+++ b/drivers/nvdimm/of_pmem.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define pr_fmt(fmt) "of_pmem: " fmt
+
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/libnvdimm.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+
+static const struct attribute_group *region_attr_groups[] = {
+	&nd_region_attribute_group,
+	&nd_device_attribute_group,
+	NULL,
+};
+
+static const struct attribute_group *bus_attr_groups[] = {
+	&nvdimm_bus_attribute_group,
+	NULL,
+};
+
+struct of_nd_private {
+	struct nvdimm_bus_descriptor bus_desc;
+	struct nvdimm_bus *bus;
+};
+
+static int of_nd_region_probe(struct platform_device *pdev)
+{
+	struct of_nd_private *priv;
+	struct device_node *np;
+	struct nvdimm_bus *bus;
+	bool is_volatile;
+	int i;
+
+	np = dev_of_node(&pdev->dev);
+	if (!np)
+		return -ENXIO;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->bus_desc.attr_groups = bus_attr_groups;
+	priv->bus_desc.provider_name = "of_pmem";
+	priv->bus_desc.module = THIS_MODULE;
+	priv->bus_desc.of_node = np;
+
+	priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc);
+	if (!bus) {
+		kfree(priv);
+		return -ENODEV;
+	}
+	platform_set_drvdata(pdev, priv);
+
+	is_volatile = !!of_find_property(np, "volatile", NULL);
+	dev_dbg(&pdev->dev, "Registering %s regions from %pOF\n",
+			is_volatile ? "volatile" : "non-volatile",  np);
+
+	for (i = 0; i < pdev->num_resources; i++) {
+		struct nd_region_desc ndr_desc;
+		struct nd_region *region;
+
+		/*
+		 * NB: libnvdimm copies the data from ndr_desc into it's own
+		 * structures so passing a stack pointer is fine.
+		 */
+		memset(&ndr_desc, 0, sizeof(ndr_desc));
+		ndr_desc.attr_groups = region_attr_groups;
+		ndr_desc.numa_node = of_node_to_nid(np);
+		ndr_desc.res = &pdev->resource[i];
+		ndr_desc.of_node = np;
+		set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+
+		if (is_volatile)
+			region = nvdimm_volatile_region_create(bus, &ndr_desc);
+		else
+			region = nvdimm_pmem_region_create(bus, &ndr_desc);
+
+		if (!region)
+			dev_warn(&pdev->dev, "Unable to register region %pR from %pOF\n",
+					ndr_desc.res, np);
+		else
+			dev_dbg(&pdev->dev, "Registered region %pR from %pOF\n",
+					ndr_desc.res, np);
+	}
+
+	return 0;
+}
+
+static int of_nd_region_remove(struct platform_device *pdev)
+{
+	struct of_nd_private *priv = platform_get_drvdata(pdev);
+
+	nvdimm_bus_unregister(priv->bus);
+	kfree(priv);
+
+	return 0;
+}
+
+static const struct of_device_id of_nd_region_match[] = {
+	{ .compatible = "pmem-region" },
+	{ },
+};
+
+static struct platform_driver of_nd_region_driver = {
+	.probe = of_nd_region_probe,
+	.remove = of_nd_region_remove,
+	.driver = {
+		.name = "of_pmem",
+		.owner = THIS_MODULE,
+		.of_match_table = of_nd_region_match,
+	},
+};
+
+module_platform_driver(of_nd_region_driver);
+MODULE_DEVICE_TABLE(of, of_nd_region_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("IBM Corporation");
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Oliver O'Halloran]

Add device-tree binding documentation for the nvdimm region driver.

Cc: devicetree@vger.kernel.org
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Changed name from nvdimm-region to pmem-region.
    Cleaned up the example binding and fixed the overlapping regions.
    Added support for multiple regions in a single reg.
---
 .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt

diff --git a/Documentation/devicetree/bindings/pmem/pmem-region.txt b/Documentation/devicetree/bindings/pmem/pmem-region.txt
new file mode 100644
index 000000000000..de48dc8cd562
--- /dev/null
+++ b/Documentation/devicetree/bindings/pmem/pmem-region.txt
@@ -0,0 +1,80 @@
+Device-tree bindings for persistent memory regions
+-----------------------------------------------------
+
+Persistent memory refers to a class of memory devices that are:
+
+	a) Usable as main system memory (i.e. cacheable), and
+	b) Retain their contents across power failure.
+
+Given b) it is best to think of persistent memory as a kind of memory mapped
+storage device. To ensure data integrity the operating system needs to manage
+persistent regions separately to the normal memory pool. To aid with that this
+binding provides a standardised interface for discovering where persistent
+memory regions exist inside the physical address space.
+
+Bindings for the region nodes:
+-----------------------------
+
+Required properties:
+	- compatible = "pmem-region"
+
+	- reg = <base, size>;
+		The system physical address range of this nvdimm region.
+
+		If the reg property contains multiple address ranges
+		each address range will be treated as though it was specified
+		in a separate device node. Having multiple address ranges in a
+		node implies no special relationship between the two ranges.
+
+Optional properties:
+	- Any relevant NUMA assocativity properties for the target platform.
+
+	- A "volatile" property indicating that this region is actually in
+	  normal DRAM and does not require cache flushes after each write.
+
+	  If this property is absent then the OS must assume that the region
+	  is backed by non-volatile memory.
+
+A complete example:
+--------------------
+
+Here we define three 4KB regions:
+
+	a) A volatile region at 0x5000 on numa node 0,
+	b) A non-volatile region at 0x6000, and
+	c) A non-volatile region at 0x8000.
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	platform {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		/*
+		 * This node specifies one non-volatile region spanning from
+		 * 0x5000 to 0x5fff.
+		 */
+		pmem@5000 {
+			compatible = "pmem-region";
+			reg = <0x00005000 0x00001000>;
+			numa-node-id = <0>;
+			volatile;
+		};
+
+		/*
+		 * This node specifies two 4KB regions that are backed by
+		 * volatile (normal) memory.
+		 */
+		pmem@6000 {
+			compatible = "pmem-region";
+			reg = <0x00006000 0x00001000 0x00008000 0x00001000>;
+		};
+	};
+};
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 6ef38be700e8..fa3c9211d6ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8041,6 +8041,7 @@ L:	linux-nvdimm@lists.01.org
 Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
 F:	drivers/nvdimm/of_pmem.c
+F:	Documentation/devicetree/bindings/pmem/pmem-region.txt
 
 LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
 M:	Dan Williams <dan.j.williams@intel.com>
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[RESEND v2 4/4] powerpc/powernv: Create platform devs for nvdimm buses

[Oliver O'Halloran]

Scan the devicetree for an nvdimm-bus compatible and create
a platform device for them.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/opal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index c15182765ff5..c37485a3c5c9 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -821,6 +821,9 @@ static int __init opal_init(void)
 	/* Create i2c platform devices */
 	opal_pdev_init("ibm,opal-i2c");
 
+	/* Handle non-volatile memory devices */
+	opal_pdev_init("pmem-region");
+
 	/* Setup a heatbeat thread if requested by OPAL */
 	opal_init_heartbeat();
 
-- 
2.9.5

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 2/4] libnvdimm: Add device-tree based driver

[Dan Williams]

On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
> This patch adds peliminary device-tree bindings for persistent memory
> regions. The driver registers a libnvdimm bus for each pmem-region
> node and each address range under the node is converted to a region
> within that bus.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> v2: Made each bus have a separate node rather having a shared bus.
>     Renamed to of_pmem rather than of_nvdimm.
>     Changed log level of happy-path messages to debug.
> ---
[..]
> +static struct platform_driver of_nd_region_driver = {
> +       .probe = of_nd_region_probe,
> +       .remove = of_nd_region_remove,
> +       .driver = {
> +               .name = "of_pmem",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_nd_region_match,
> +       },
> +};

This and the other patches look good to me. Just a nit on the
naming... since you name the regions pmem-regions in the device-tree
description shouldn't this be the "of_pmem_region" or "of_pmem_range"
driver? Otherwise, it is confusing to me that anything named
*nd_region would be creating an nvdimm_bus. In general An nd_region is
always a child of a bus.

That said, with an ack/reviewed-by on the device-tree bindings I can
take these through the nvdimm tree. I'll reply to patch 4 with that
request for ack.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Dan Williams]

On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
> Add device-tree binding documentation for the nvdimm region driver.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> v2: Changed name from nvdimm-region to pmem-region.
>     Cleaned up the example binding and fixed the overlapping regions.
>     Added support for multiple regions in a single reg.
> ---
>  .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt

Device-tree folks, does this look, ok?

Oliver, is there any concept of a management interface to the
device(s) backing these regions? libnvdimm calls these "nmem" devices
and support operations like health status and namespace label
management.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 1/4] libnvdimm: Add of_node to region and bus descriptors

[Balbir Singh]

On Wed,  4 Apr 2018 00:24:12 +1000
Oliver O'Halloran <oohall@gmail.com> wrote:

> We want to be able to cross reference the region and bus devices
> with the device tree node that they were spawned from. libNVDIMM
> handles creating the actual devices for these internally, so we
> need to pass in a pointer to the relevant node in the descriptor.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> ---
Acked-by: Balbir Singh <bsingharora@gmail.com>

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Balbir Singh]

On Tue, 3 Apr 2018 10:37:51 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
> > Add device-tree binding documentation for the nvdimm region driver.
> >
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > ---
> > v2: Changed name from nvdimm-region to pmem-region.
> >     Cleaned up the example binding and fixed the overlapping regions.
> >     Added support for multiple regions in a single reg.
> > ---
> >  .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 81 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt  
> 
> Device-tree folks, does this look, ok?
> 
> Oliver, is there any concept of a management interface to the
> device(s) backing these regions? libnvdimm calls these "nmem" devices
> and support operations like health status and namespace label
> management.

We would need a way to have nmem and pmem-regions find each other. Since we
don't have the ACPI abstractions, the nmem region would need to add the
ability for a driver to have a phandle to the interleaving and nmem properties.

I guess that would be a separate driver, that would manage the nmem devices
and there would be a way to relate the pmem and nmems. Oliver?

Balbir Singh.

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 4/4] powerpc/powernv: Create platform devs for nvdimm buses

[Balbir Singh]

On Wed,  4 Apr 2018 00:24:15 +1000
Oliver O'Halloran <oohall@gmail.com> wrote:

> Scan the devicetree for an nvdimm-bus compatible and create
> a platform device for them.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---

Acked-by: Balbir Singh <bsingharora@gmail.com>

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Oliver]

On Wed, Apr 4, 2018 at 10:07 PM, Balbir Singh <bsingharora@gmail.com> wrote:
> On Tue, 3 Apr 2018 10:37:51 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
>> > Add device-tree binding documentation for the nvdimm region driver.
>> >
>> > Cc: devicetree@vger.kernel.org
>> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>> > ---
>> > v2: Changed name from nvdimm-region to pmem-region.
>> >     Cleaned up the example binding and fixed the overlapping regions.
>> >     Added support for multiple regions in a single reg.
>> > ---
>> >  .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
>> >  MAINTAINERS                                        |  1 +
>> >  2 files changed, 81 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt
>>
>> Device-tree folks, does this look, ok?
>>
>> Oliver, is there any concept of a management interface to the
>> device(s) backing these regions? libnvdimm calls these "nmem" devices
>> and support operations like health status and namespace label
>> management.

It's something I'm planning on implementing as soon as someone gives
me some hardware that isn't hacked up lab crap. I'm posting this
version with just regions since people have been asking for something
in upstream even if it's not fully featured.

Grumbling aside, the plan is to have separate drivers for the DIMM
type. Discovering DIMM devices happens via the normal discovery
mechanisms (e.g. an NVDIMM supporting the JEDEC interface is an I2C
device) and when binding to a specific DIMM device it registers a DIMM
descriptor structure and a ndctl implementation for that DIMM type
with of_pmem. When of_pmem binds to a region it can plug everything
into the region specific bus. There's a few details to work out, but I
think it's a reasonable approach.

> We would need a way to have nmem and pmem-regions find each other. Since we
> don't have the ACPI abstractions, the nmem region would need to add the
> ability for a driver to have a phandle to the interleaving and nmem properties.
>
> I guess that would be a separate driver, that would manage the nmem devices
> and there would be a way to relate the pmem and nmems. Oliver?

Yes, that's the plan.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Dan Williams]

On Wed, Apr 4, 2018 at 7:04 AM, Oliver <oohall@gmail.com> wrote:
> On Wed, Apr 4, 2018 at 10:07 PM, Balbir Singh <bsingharora@gmail.com> wrote:
>> On Tue, 3 Apr 2018 10:37:51 -0700
>> Dan Williams <dan.j.williams@intel.com> wrote:
>>
>>> On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
>>> > Add device-tree binding documentation for the nvdimm region driver.
>>> >
>>> > Cc: devicetree@vger.kernel.org
>>> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>>> > ---
>>> > v2: Changed name from nvdimm-region to pmem-region.
>>> >     Cleaned up the example binding and fixed the overlapping regions.
>>> >     Added support for multiple regions in a single reg.
>>> > ---
>>> >  .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
>>> >  MAINTAINERS                                        |  1 +
>>> >  2 files changed, 81 insertions(+)
>>> >  create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt
>>>
>>> Device-tree folks, does this look, ok?
>>>
>>> Oliver, is there any concept of a management interface to the
>>> device(s) backing these regions? libnvdimm calls these "nmem" devices
>>> and support operations like health status and namespace label
>>> management.
>
> It's something I'm planning on implementing as soon as someone gives
> me some hardware that isn't hacked up lab crap. I'm posting this
> version with just regions since people have been asking for something
> in upstream even if it's not fully featured.
>
> Grumbling aside, the plan is to have separate drivers for the DIMM
> type. Discovering DIMM devices happens via the normal discovery
> mechanisms (e.g. an NVDIMM supporting the JEDEC interface is an I2C
> device) and when binding to a specific DIMM device it registers a DIMM
> descriptor structure and a ndctl implementation for that DIMM type
> with of_pmem. When of_pmem binds to a region it can plug everything
> into the region specific bus. There's a few details to work out, but I
> think it's a reasonable approach.

Yeah, that sounds reasonable. It would mean that your management
interface would need to understand that nmems on different buses could
potentially move to another bus after a reconfiguration, but that's
not too much different than the ACPI case where nmems can join and
leave regions after a reset / reconfig.

>> We would need a way to have nmem and pmem-regions find each other. Since we
>> don't have the ACPI abstractions, the nmem region would need to add the
>> ability for a driver to have a phandle to the interleaving and nmem properties.
>>
>> I guess that would be a separate driver, that would manage the nmem devices
>> and there would be a way to relate the pmem and nmems. Oliver?
>
> Yes, that's the plan.

So Balbir, is that enough for an Acked-by for this device-tree proposal?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Balbir Singh]

On Wed, 4 Apr 2018 07:21:32 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> On Wed, Apr 4, 2018 at 7:04 AM, Oliver <oohall@gmail.com> wrote:
> > On Wed, Apr 4, 2018 at 10:07 PM, Balbir Singh <bsingharora@gmail.com> wrote:  
> >> On Tue, 3 Apr 2018 10:37:51 -0700
> >> Dan Williams <dan.j.williams@intel.com> wrote:
> >>  
> >>> On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:  
> >>> > Add device-tree binding documentation for the nvdimm region driver.
> >>> >
> >>> > Cc: devicetree@vger.kernel.org
> >>> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> >>> > ---
> >>> > v2: Changed name from nvdimm-region to pmem-region.
> >>> >     Cleaned up the example binding and fixed the overlapping regions.
> >>> >     Added support for multiple regions in a single reg.
> >>> > ---
> >>> >  .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
> >>> >  MAINTAINERS                                        |  1 +
> >>> >  2 files changed, 81 insertions(+)
> >>> >  create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt  
> >>>
> >>> Device-tree folks, does this look, ok?
> >>>
> >>> Oliver, is there any concept of a management interface to the
> >>> device(s) backing these regions? libnvdimm calls these "nmem" devices
> >>> and support operations like health status and namespace label
> >>> management.  
> >
> > It's something I'm planning on implementing as soon as someone gives
> > me some hardware that isn't hacked up lab crap. I'm posting this
> > version with just regions since people have been asking for something
> > in upstream even if it's not fully featured.
> >
> > Grumbling aside, the plan is to have separate drivers for the DIMM
> > type. Discovering DIMM devices happens via the normal discovery
> > mechanisms (e.g. an NVDIMM supporting the JEDEC interface is an I2C
> > device) and when binding to a specific DIMM device it registers a DIMM
> > descriptor structure and a ndctl implementation for that DIMM type
> > with of_pmem. When of_pmem binds to a region it can plug everything
> > into the region specific bus. There's a few details to work out, but I
> > think it's a reasonable approach.  
> 
> Yeah, that sounds reasonable. It would mean that your management
> interface would need to understand that nmems on different buses could
> potentially move to another bus after a reconfiguration, but that's
> not too much different than the ACPI case where nmems can join and
> leave regions after a reset / reconfig.
> 
> >> We would need a way to have nmem and pmem-regions find each other. Since we
> >> don't have the ACPI abstractions, the nmem region would need to add the
> >> ability for a driver to have a phandle to the interleaving and nmem properties.
> >>
> >> I guess that would be a separate driver, that would manage the nmem devices
> >> and there would be a way to relate the pmem and nmems. Oliver?  
> >
> > Yes, that's the plan.  
> 
> So Balbir, is that enough for an Acked-by for this device-tree proposal?

I don't see any major problems with the binding, but I think Oliver wanted
to send in a few clarifications based on a private discussion.

Balbir Singh.


_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Oliver]

On Thu, Apr 5, 2018 at 12:21 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Wed, Apr 4, 2018 at 7:04 AM, Oliver <oohall@gmail.com> wrote:
>> On Wed, Apr 4, 2018 at 10:07 PM, Balbir Singh <bsingharora@gmail.com> wrote:
>>> On Tue, 3 Apr 2018 10:37:51 -0700
>>> Dan Williams <dan.j.williams@intel.com> wrote:
>>>
>>>> On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
>>>> > Add device-tree binding documentation for the nvdimm region driver.
>>>> >
>>>> > Cc: devicetree@vger.kernel.org
>>>> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>>>> > ---
>>>> > v2: Changed name from nvdimm-region to pmem-region.
>>>> >     Cleaned up the example binding and fixed the overlapping regions.
>>>> >     Added support for multiple regions in a single reg.
>>>> > ---
>>>> >  .../devicetree/bindings/pmem/pmem-region.txt       | 80 ++++++++++++++++++++++
>>>> >  MAINTAINERS                                        |  1 +
>>>> >  2 files changed, 81 insertions(+)
>>>> >  create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt
>>>>
>>>> Device-tree folks, does this look, ok?
>>>>
>>>> Oliver, is there any concept of a management interface to the
>>>> device(s) backing these regions? libnvdimm calls these "nmem" devices
>>>> and support operations like health status and namespace label
>>>> management.
>>
>> It's something I'm planning on implementing as soon as someone gives
>> me some hardware that isn't hacked up lab crap. I'm posting this
>> version with just regions since people have been asking for something
>> in upstream even if it's not fully featured.
>>
>> Grumbling aside, the plan is to have separate drivers for the DIMM
>> type. Discovering DIMM devices happens via the normal discovery
>> mechanisms (e.g. an NVDIMM supporting the JEDEC interface is an I2C
>> device) and when binding to a specific DIMM device it registers a DIMM
>> descriptor structure and a ndctl implementation for that DIMM type
>> with of_pmem. When of_pmem binds to a region it can plug everything
>> into the region specific bus. There's a few details to work out, but I
>> think it's a reasonable approach.
>
> Yeah, that sounds reasonable. It would mean that your management
> interface would need to understand that nmems on different buses could
> potentially move to another bus after a reconfiguration, but that's
> not too much different than the ACPI case where nmems can join and
> leave regions after a reset / reconfig.
>
>>> We would need a way to have nmem and pmem-regions find each other. Since we
>>> don't have the ACPI abstractions, the nmem region would need to add the
>>> ability for a driver to have a phandle to the interleaving and nmem properties.
>>>
>>> I guess that would be a separate driver, that would manage the nmem devices
>>> and there would be a way to relate the pmem and nmems. Oliver?
>>
>> Yes, that's the plan.
>
> So Balbir, is that enough for an Acked-by for this device-tree proposal?

For context Balbir is working with me on some of the pmem stuff. You
probably want an Ack from Rob rather than one of us.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Michael Ellerman]

Oliver <oohall@gmail.com> writes:
...
>
> For context Balbir is working with me on some of the pmem stuff. You
> probably want an Ack from Rob rather than one of us.

I'll ack it if you make all the niggly nit picky trivial annoying
changes I asked for :D

cheers
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Oliver]

On Thu, Apr 5, 2018 at 10:11 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Oliver <oohall@gmail.com> writes:
> ...
>>
>> For context Balbir is working with me on some of the pmem stuff. You
>> probably want an Ack from Rob rather than one of us.
>
> I'll ack it if you make all the niggly nit picky trivial annoying
> changes I asked for :D

*groan*

Fine, I'll respin it tomorrow. If anyone else has comments now would
be the time to make them.

>
> cheers
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Dan Williams]

On Thu, Apr 5, 2018 at 5:43 AM, Oliver <oohall@gmail.com> wrote:
> On Thu, Apr 5, 2018 at 10:11 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Oliver <oohall@gmail.com> writes:
>> ...
>>>
>>> For context Balbir is working with me on some of the pmem stuff. You
>>> probably want an Ack from Rob rather than one of us.
>>
>> I'll ack it if you make all the niggly nit picky trivial annoying
>> changes I asked for :D
>
> *groan*
>
> Fine, I'll respin it tomorrow. If anyone else has comments now would
> be the time to make them.

Please also include my niggly nit picky trivial annoying bike shed
color for the driver name to *not* use the "nd_region" suffix for a
driver registering "nvdimm_bus" objects. "of_pmem_range" or
"of_pmem_bus" or almost anything else would be fine.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Oliver]

On Fri, Apr 6, 2018 at 12:43 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Thu, Apr 5, 2018 at 5:43 AM, Oliver <oohall@gmail.com> wrote:
>> On Thu, Apr 5, 2018 at 10:11 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> Oliver <oohall@gmail.com> writes:
>>> ...
>>>>
>>>> For context Balbir is working with me on some of the pmem stuff. You
>>>> probably want an Ack from Rob rather than one of us.
>>>
>>> I'll ack it if you make all the niggly nit picky trivial annoying
>>> changes I asked for :D
>>
>> *groan*
>>
>> Fine, I'll respin it tomorrow. If anyone else has comments now would
>> be the time to make them.
>
> Please also include my niggly nit picky trivial annoying bike shed
> color for the driver name to *not* use the "nd_region" suffix for a
> driver registering "nvdimm_bus" objects. "of_pmem_range" or
> "of_pmem_bus" or almost anything else would be fine.

Oh sure, would using of_pmem_region to match the compatible be ok?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Dan Williams]

On Thu, Apr 5, 2018 at 7:14 PM, Oliver <oohall@gmail.com> wrote:
>
> On Fri, Apr 6, 2018 at 12:43 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> > On Thu, Apr 5, 2018 at 5:43 AM, Oliver <oohall@gmail.com> wrote:
> >> On Thu, Apr 5, 2018 at 10:11 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> >>> Oliver <oohall@gmail.com> writes:
> >>> ...
> >>>>
> >>>> For context Balbir is working with me on some of the pmem stuff. You
> >>>> probably want an Ack from Rob rather than one of us.
> >>>
> >>> I'll ack it if you make all the niggly nit picky trivial annoying
> >>> changes I asked for :D
> >>
> >> *groan*
> >>
> >> Fine, I'll respin it tomorrow. If anyone else has comments now would
> >> be the time to make them.
> >
> > Please also include my niggly nit picky trivial annoying bike shed
> > color for the driver name to *not* use the "nd_region" suffix for a
> > driver registering "nvdimm_bus" objects. "of_pmem_range" or
> > "of_pmem_bus" or almost anything else would be fine.
>
> Oh sure, would using of_pmem_region to match the compatible be ok?

That works for me.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings

[Benjamin Herrenschmidt]

On Thu, 2018-04-05 at 19:25 -0700, Dan Williams wrote:
> > > Please also include my niggly nit picky trivial annoying bike shed
> > > color for the driver name to *not* use the "nd_region" suffix for a
> > > driver registering "nvdimm_bus" objects. "of_pmem_range" or
> > > "of_pmem_bus" or almost anything else would be fine.
> > 
> > Oh sure, would using of_pmem_region to match the compatible be ok?
> 
> That works for me.

The prefix "of" is not generally used in matching properties,...

my own pot of paint :)

Cheers,
Ben.

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm