[PATCH v2 0/5] Implement SoC bus driver for Vybrid

[PATCH v2 0/5] Implement SoC bus driver for Vybrid

[Sanchayan Maity]

Hello,

This second patch series is rebased on top of shawn's for-next branch
and tested on Colibri Vybrid VF50 and VF61 modules.

This patchset implements SoC bus support for Freescale Vybrid platform,
implementing the following
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc

This a reworked version of an older patchset series posted in June 2015
which was at v5 then [1]. Since the NVMEM framework was then getting
introduced, we decided that first a NVMEM driver for OCOTP peripheral
being in place would be better.

Compared to the older revisions, this driver now relies on NVMEM
consumer API using the NVMEM based vf610_ocotp driver which has
already been in mainline for a while now. Also now a new syscon
abstraction "syscon_regmap_read_from_offset" is implemented and
exported from syscon allowing accessing a register from a syscon
reference like this

ocotp-cfg1 = <&ocotp 0x20>;

avoiding code repetition in the driver.

One point on which we were not sure here is whether we really should
introduce a new Kconfig symbol as being introduced here. While we
could just enable it when SOC_VF610 is selected, this however would
introduce circular dependencies.

Feedback is most welcome.

Changes since v1:
Add device tree binding documentation.

v1 new patchset
https://lkml.org/lkml/2016/3/11/132

[1] Older v5:
http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html
Even earlier versions:
Version 4 of the patchset can be found here
https://lkml.org/lkml/2015/5/26/199
Version 3 of the patchset can be found here
http://www.spinics.net/lists/arm-kernel/msg420847.html
Version 2 of the patchset can be found here
http://www.spinics.net/lists/devicetree/msg80654.html
Version 1 of the patchset can be found here
http://www.spinics.net/lists/devicetree/msg80257.html
The RFC version can be found here
https://lkml.org/lkml/2015/5/11/13

Regards,
Sanchayan.

Sanchayan Maity (5):
  mfd: syscon: Introduce syscon_regmap_read_from_offset
  ARM: dts: vfxxx: Add device tree node for OCOTP
  ARM: dts: vfxxx: Add OCROM and phandle entries for Vybrid SoC bus driver
  soc: Add SoC bus driver for Freescale Vybrid Platform
  vf610-soc: Add Vybrid SoC device tree binding documentation

 .../bindings/arm/freescale/fsl,vf610-soc.txt       |  35 +++++
 arch/arm/boot/dts/vfxxx.dtsi                       |  28 +++-
 drivers/mfd/syscon.c                               |  30 ++++
 drivers/soc/Kconfig                                |   1 +
 drivers/soc/fsl/Kconfig                            |  10 ++
 drivers/soc/fsl/Makefile                           |   1 +
 drivers/soc/fsl/soc-vf610.c                        | 160 +++++++++++++++++++++
 include/linux/mfd/syscon.h                         |  10 ++
 8 files changed, 274 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
 create mode 100644 drivers/soc/fsl/Kconfig
 create mode 100644 drivers/soc/fsl/soc-vf610.c

-- 
2.8.2

[PATCH v2 1/5] mfd: syscon: Introduce syscon_regmap_read_from_offset

[Sanchayan Maity]

Currently syscon does not provide an abstraction to access a
register from syscon reference like below

ocotp-cfg1 = <&ocotp 0x20>

syscon_regmap_read_from_offset provides a generic abstraction to
access a register from syscon reference as above. It allows to
specify the node and node name of phandle reference, reading the
offset from the node entry and providing the value from the offset
in the register map.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/mfd/syscon.c       | 30 ++++++++++++++++++++++++++++++
 include/linux/mfd/syscon.h | 10 ++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index 2f2225e..74724c3 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -136,6 +136,36 @@ struct regmap *syscon_node_to_regmap(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(syscon_node_to_regmap);
 
+int syscon_regmap_read_from_offset(struct device_node *np,
+				const char *s, unsigned int *val)
+{
+	struct of_phandle_args pargs;
+	struct regmap *regmap;
+	int offset;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	ret = of_parse_phandle_with_fixed_args(np, s, 1, 0, &pargs);
+	if (ret)
+		return ret;
+
+	regmap = syscon_node_to_regmap(pargs.np);
+	if (IS_ERR(regmap)) {
+		of_node_put(pargs.np);
+		return PTR_ERR(regmap);
+	}
+
+	offset = pargs.args[0];
+	of_node_put(pargs.np);
+
+	ret = regmap_read(regmap, offset, val);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(syscon_regmap_read_from_offset);
+
 struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
 {
 	struct device_node *syscon_np;
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 1088149..42b0759 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -26,6 +26,9 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
 extern struct regmap *syscon_regmap_lookup_by_phandle(
 					struct device_node *np,
 					const char *property);
+extern int syscon_regmap_read_from_offset(struct device_node *np,
+					  const char *s,
+					  unsigned int *val);
 #else
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
@@ -48,6 +51,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle(
 {
 	return ERR_PTR(-ENOTSUPP);
 }
+
+static inline int syscon_regmap_read_from_offset(struct device_node *np,
+						 const char *s,
+						 unsigned int *val)
+{
+	return ERR_PTR(-ENOSYS);
+}
 #endif
 
 #endif /* __LINUX_MFD_SYSCON_H__ */
-- 
2.8.2

[PATCH v2 2/5] ARM: dts: vfxxx: Add device tree node for OCOTP

[Sanchayan Maity]

Add device tree node for the OCOTP peripheral on Vybrid.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vfxxx.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 2c13ec6..0e34d44 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -520,6 +520,22 @@
 				status = "disabled";
 			};
 
+			ocotp@400a5000 {
+				compatible = "fsl,vf610-ocotp";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x400a5000 0xCF0>;
+				clocks = <&clks VF610_CLK_OCOTP>;
+
+				ocotp_cfg0: cfg0@410 {
+					reg = <0x410 0x4>;
+				};
+
+				ocotp_cfg1: cfg1@420 {
+					reg = <0x420 0x4>;
+				};
+			};
+
 			snvs0: snvs@400a7000 {
 			    compatible = "fsl,sec-v4.0-mon", "syscon", "simple-mfd";
 				reg = <0x400a7000 0x2000>;
-- 
2.8.2

[PATCH v2 3/5] ARM: dts: vfxxx: Add OCROM and phandle entries for Vybrid SoC bus driver

[Sanchayan Maity]

Add OCROM node and introduce phandles to OCROM, MSCM and NVMEM
OCOTP for use by the Vybrid SoC bus driver.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 arch/arm/boot/dts/vfxxx.dtsi | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 0e34d44..e58f4c6 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -87,9 +87,19 @@
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-		compatible = "simple-bus";
+		compatible = "fsl,vf610-soc-bus", "simple-bus";
 		interrupt-parent = <&mscm_ir>;
 		ranges;
+		fsl,rom-revision = <&ocrom 0x80>;
+		fsl,cpu-count = <&mscm_cpucfg 0x2C>;
+		fsl,l2-size = <&mscm_cpucfg 0x14>;
+		nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
+		nvmem-cell-names = "cfg0", "cfg1";
+
+		ocrom: ocrom@00000000 {
+			compatible = "fsl,vf610-ocrom", "syscon";
+			reg = <0x00000000 0x18000>;
+		};
 
 		aips0: aips-bus@40000000 {
 			compatible = "fsl,aips-bus", "simple-bus";
-- 
2.8.2

[PATCH v2 4/5] soc: Add SoC bus driver for Freescale Vybrid Platform

[Sanchayan Maity]

This adds a SoC driver to be used by Freescale Vybrid SoC's.
Driver utilises syscon and nvmem consumer API's to get the
various register values needed and sysfs exposes the SoC specific
properties.

A sample output from Colibri Vybrid VF61 is below:

root@colibri-vf:~# cd /sys/bus/soc/devices/soc0
root@colibri-vf:/sys/bus/soc/devices/soc0# ls
family     machine    power      revision   soc_id     subsystem  uevent
root@colibri-vf:/sys/bus/soc/devices/soc0# cat family
Freescale Vybrid VF610
root@colibri-vf:/sys/bus/soc/devices/soc0# cat machine
Freescale Vybrid
root@colibri-vf:/sys/bus/soc/devices/soc0# cat revision
00000013
root@colibri-vf:/sys/bus/soc/devices/soc0# cat soc_id
df6472a6130f29d4

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/soc/Kconfig         |   1 +
 drivers/soc/fsl/Kconfig     |  10 +++
 drivers/soc/fsl/Makefile    |   1 +
 drivers/soc/fsl/soc-vf610.c | 160 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 172 insertions(+)
 create mode 100644 drivers/soc/fsl/Kconfig
 create mode 100644 drivers/soc/fsl/soc-vf610.c

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index cb58ef0..4410eb7 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -2,6 +2,7 @@ menu "SOC (System On Chip) specific Drivers"
 
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/brcmstb/Kconfig"
+source "drivers/soc/fsl/Kconfig"
 source "drivers/soc/fsl/qe/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
 source "drivers/soc/qcom/Kconfig"
diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
new file mode 100644
index 0000000..029ea17
--- /dev/null
+++ b/drivers/soc/fsl/Kconfig
@@ -0,0 +1,10 @@
+#
+# Freescale SoC drivers
+
+config SOC_BUS_VF610
+	bool "SoC bus device for the Freescale Vybrid platform"
+	depends on NVMEM && NVMEM_VF610_OCOTP
+	select SOC_BUS
+	help
+	 Include support for the SoC bus on the Freescale Vybrid platform
+	 providing sysfs information about the module variant.
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 203307f..afaf092 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -2,5 +2,6 @@
 # Makefile for the Linux Kernel SOC fsl specific device drivers
 #
 
+obj-$(CONFIG_SOC_VF610)			+= soc-vf610.o
 obj-$(CONFIG_QUICC_ENGINE)		+= qe/
 obj-$(CONFIG_CPM)			+= qe/
diff --git a/drivers/soc/fsl/soc-vf610.c b/drivers/soc/fsl/soc-vf610.c
new file mode 100644
index 0000000..99bf641
--- /dev/null
+++ b/drivers/soc/fsl/soc-vf610.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright (C) 2016 Toradex AG.
+ *
+ * Author: Sanchayan Maity <sanchayan.maity@toradex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/random.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+
+struct vf610_soc {
+	struct device *dev;
+	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device *soc_dev;
+	struct nvmem_cell *ocotp_cfg0;
+	struct nvmem_cell *ocotp_cfg1;
+};
+
+static int vf610_soc_probe(struct platform_device *pdev)
+{
+	struct vf610_soc *info;
+	struct device *dev = &pdev->dev;
+	struct device_node *soc_node;
+	char soc_type[] = "xx0";
+	size_t id1_len;
+	size_t id2_len;
+	u32 cpucount;
+	u32 l2size;
+	u32 rom_rev;
+	u8 *socid1;
+	u8 *socid2;
+	int ret;
+
+	info = devm_kzalloc(dev, sizeof(struct vf610_soc), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->dev = dev;
+
+	info->ocotp_cfg0 = devm_nvmem_cell_get(dev, "cfg0");
+	if (IS_ERR(info->ocotp_cfg0))
+		return -EPROBE_DEFER;
+
+	info->ocotp_cfg1 = devm_nvmem_cell_get(dev, "cfg1");
+	if (IS_ERR(info->ocotp_cfg1))
+		return -EPROBE_DEFER;
+
+	socid1 = nvmem_cell_read(info->ocotp_cfg0, &id1_len);
+	if (IS_ERR(socid1)) {
+		dev_err(dev, "Could not read nvmem cell %ld\n",
+			PTR_ERR(socid1));
+		return PTR_ERR(socid1);
+	}
+
+	socid2 = nvmem_cell_read(info->ocotp_cfg1, &id2_len);
+	if (IS_ERR(socid2)) {
+		dev_err(dev, "Could not read nvmem cell %ld\n",
+			PTR_ERR(socid2));
+		return PTR_ERR(socid2);
+	}
+	add_device_randomness(socid1, id1_len);
+	add_device_randomness(socid2, id2_len);
+
+	soc_node = of_find_node_by_path("/soc");
+	if (soc_node == NULL)
+		return -ENODEV;
+
+	ret = syscon_regmap_read_from_offset(soc_node,
+					"fsl,rom-revision", &rom_rev);
+	if (ret) {
+		of_node_put(soc_node);
+		return ret;
+	}
+
+	ret = syscon_regmap_read_from_offset(soc_node,
+					"fsl,cpu-count", &cpucount);
+	if (ret) {
+		of_node_put(soc_node);
+		return ret;
+	}
+
+	ret = syscon_regmap_read_from_offset(soc_node,
+					"fsl,l2-size", &l2size);
+	if (ret) {
+		of_node_put(soc_node);
+		return ret;
+	}
+
+	of_node_put(soc_node);
+
+	soc_type[0] = cpucount ? '6' : '5'; /* Dual Core => VF6x0 */
+	soc_type[1] = l2size ? '1' : '0'; /* L2 Cache => VFx10 */
+
+	info->soc_dev_attr = devm_kzalloc(dev,
+				sizeof(info->soc_dev_attr), GFP_KERNEL);
+	if (!info->soc_dev_attr)
+		return -ENOMEM;
+
+	info->soc_dev_attr->machine = devm_kasprintf(dev,
+					GFP_KERNEL, "Freescale Vybrid");
+	info->soc_dev_attr->soc_id = devm_kasprintf(dev,
+					GFP_KERNEL,
+					"%02x%02x%02x%02x%02x%02x%02x%02x",
+					socid1[3], socid1[2], socid1[1],
+					socid1[0], socid2[3], socid2[2],
+					socid2[1], socid2[0]);
+	info->soc_dev_attr->family = devm_kasprintf(&pdev->dev,
+					GFP_KERNEL, "Freescale Vybrid VF%s",
+					soc_type);
+	info->soc_dev_attr->revision = devm_kasprintf(dev,
+					GFP_KERNEL, "%08x", rom_rev);
+
+	platform_set_drvdata(pdev, info);
+
+	info->soc_dev = soc_device_register(info->soc_dev_attr);
+	if (IS_ERR(info->soc_dev))
+		return -ENODEV;
+
+	return 0;
+}
+
+static int vf610_soc_remove(struct platform_device *pdev)
+{
+	struct vf610_soc *info = platform_get_drvdata(pdev);
+
+	if (info->soc_dev)
+		soc_device_unregister(info->soc_dev);
+
+	return 0;
+}
+
+static const struct of_device_id vf610_soc_bus_match[] = {
+	{ .compatible = "fsl,vf610-soc-bus", },
+	{ /* */ }
+};
+
+static struct platform_driver vf610_soc_driver = {
+	.probe		= vf610_soc_probe,
+	.remove		= vf610_soc_remove,
+	.driver		= {
+		.name = "vf610-soc-bus",
+		.of_match_table = vf610_soc_bus_match,
+	},
+};
+builtin_platform_driver(vf610_soc_driver);
-- 
2.8.2

[PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Sanchayan Maity]

Add device tree binding documentation for Vybrid SoC.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt

diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
new file mode 100644
index 0000000..bdd95e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
@@ -0,0 +1,35 @@
+Vybrid System-on-Chip
+---------------------
+
+Required properties:
+
+- #address-cells: must be 1
+- #size-cells: must be 1
+- compatible: "fsl,vf610-soc-bus", "simple-bus"
+- interrupt-parent: phandle to the MSCM interrupt router node
+- ranges
+- fsl,rom-revision: phandle to the on-chip ROM node and address of rom
+  revision register
+- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of
+  CPU count register
+- fsl,l2-size: phandle to the MSCM CPU configuration node and address of
+  L2 cache size register
+- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
+- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
+
+Example:
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "fsl,vf610-soc-bus", "simple-bus";
+		interrupt-parent = <&mscm_ir>;
+		ranges;
+		fsl,rom-revision = <&ocrom 0x80>;
+		fsl,cpu-count = <&mscm_cpucfg 0x2C>;
+		fsl,l2-size = <&mscm_cpucfg 0x14>;
+		nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
+		nvmem-cell-names = "cfg0", "cfg1";
+
+		...
+	};
-- 
2.8.2

Re: [PATCH v2 2/5] ARM: dts: vfxxx: Add device tree node for OCOTP

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

On Monday 02 May 2016 12:35:01 Sanchayan Maity wrote:
> +                       ocotp@400a5000 {
> +                               compatible = "fsl,vf610-ocotp";
> +                               #address-cells = <1>;
> +                               #size-cells = <1>;
> +                               reg = <0x400a5000 0xCF0>;
> +                               clocks = <&clks VF610_CLK_OCOTP>;
> +
> +                               ocotp_cfg0: cfg0@410 {
> +                                       reg = <0x410 0x4>;
> +                               };
> +
> +                               ocotp_cfg1: cfg1@420 {
> +                                       reg = <0x420 0x4>;
> +                               };
> +                       };

How do the registers of the child nodes relate to the registers of the
parent? If there are in the same address space, it might be good to
add a "ranges" property to describe it.

	Arnd

[-- Attachment #2: Gavin Shan <gwshan@linux.vnet.ibm.com>: Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node() --]
[-- Type: message/rfc822, Size: 8776 bytes --]

From: Gavin Shan <gwshan@linux.vnet.ibm.com>
To: Rob Herring <robherring2@gmail.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, aik@ozlabs.ru, Gavin Shan <gwshan@linux.vnet.ibm.com>, Grant Likely <grant.likely@linaro.org>, "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>, Bjorn Helgaas <bhelgaas@google.com>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, dja@axtens.net
Subject: Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node()
Date: Mon, 02 May 2016 12:02:21 +1000
Message-ID: <20160502020221.GA16612@gwshan>

On Wed, Feb 17, 2016 at 08:30:42AM -0600, Rob Herring wrote:
>On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> The function unflatten_dt_node() is called recursively to unflatten
>> device nodes and properties in the FDT blob. It looks complicated
>> and hard to be understood.
>>
>> This splits the function into 3 functions: populate_properties(),
>> populate_node() and unflatten_dt_node(). populate_properties(),
>> which is called by populate_node(), creates properties for the
>> indicated device node. The later one creates the device nodes
>> from FDT blob. populate_node() gets the offset in FDT blob for
>> next device nodes and then calls populate_node(). No logical
>> changes introduced.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  drivers/of/fdt.c | 249 ++++++++++++++++++++++++++++++++-----------------------
>>  1 file changed, 147 insertions(+), 102 deletions(-)
>
>One nit, otherwise:
>
>Acked-by: Rob Herring <robh@kernel.org>
>
>[...]
>
>> +               /* And we process the "ibm,phandle" property
>> +                * used in pSeries dynamic device tree
>> +                * stuff
>> +                */
>> +               if (!strcmp(pname, "ibm,phandle"))
>> +                       np->phandle = be32_to_cpup(val);
>> +
>> +               pp->name   = (char *)pname;
>> +               pp->length = sz;
>> +               pp->value  = (__be32 *)val;
>
>This cast should not be needed.
>

It's needed. Otherwise, we will have warning. So I will keep it. I just
went through this one for next revision and sorry for late response.

drivers/of/fdt.c:225:14: warning: assignment discards ‘const’ qualifier from pointer target type
   pp->value  = val;
              ^

Thanks,
Gavin

>> +               *pprev     = pp;
>> +               pprev      = &pp->next;
>> +       }
>

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 2/5] ARM: dts: vfxxx: Add device tree node for OCOTP

[maitysanchayan]

Hello Arnd,

On 16-05-02 09:31:12, Arnd Bergmann wrote:
> On Monday 02 May 2016 12:35:01 Sanchayan Maity wrote:
> > +                       ocotp@400a5000 {
> > +                               compatible = "fsl,vf610-ocotp";
> > +                               #address-cells = <1>;
> > +                               #size-cells = <1>;
> > +                               reg = <0x400a5000 0xCF0>;
> > +                               clocks = <&clks VF610_CLK_OCOTP>;
> > +
> > +                               ocotp_cfg0: cfg0@410 {
> > +                                       reg = <0x410 0x4>;
> > +                               };
> > +
> > +                               ocotp_cfg1: cfg1@420 {
> > +                                       reg = <0x420 0x4>;
> > +                               };
> > +                       };
> 
> How do the registers of the child nodes relate to the registers of the
> parent? If there are in the same address space, it might be good to
> add a "ranges" property to describe it.

cfg0 and cfg1 are in the same address space viz. 0x400a5410 and 0x400a5420
respectively. These nodes are primarily for use by the NVMEM consumer API in
the SoC bus driver to retrieve the values from these registers leveraging
the NVMEM vf610 ocotp driver.

Based on the NVMEM bindings here
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/nvmem/nvmem.txt#L33

Thanks.

Regards,
Sanchayan.

> 
> 	Arnd

> Date: Mon, 02 May 2016 12:02:21 +1000
> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
> To: Rob Herring <robherring2@gmail.com>
> Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
>  aik@ozlabs.ru, Gavin Shan <gwshan@linux.vnet.ibm.com>, Grant Likely
>  <grant.likely@linaro.org>, "linux-pci@vger.kernel.org"
>  <linux-pci@vger.kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
>  linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, dja@axtens.net
> Subject: Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node()
> 
> On Wed, Feb 17, 2016 at 08:30:42AM -0600, Rob Herring wrote:
> >On Tue, Feb 16, 2016 at 9:44 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> >> The function unflatten_dt_node() is called recursively to unflatten
> >> device nodes and properties in the FDT blob. It looks complicated
> >> and hard to be understood.
> >>
> >> This splits the function into 3 functions: populate_properties(),
> >> populate_node() and unflatten_dt_node(). populate_properties(),
> >> which is called by populate_node(), creates properties for the
> >> indicated device node. The later one creates the device nodes
> >> from FDT blob. populate_node() gets the offset in FDT blob for
> >> next device nodes and then calls populate_node(). No logical
> >> changes introduced.
> >>
> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >> ---
> >>  drivers/of/fdt.c | 249 ++++++++++++++++++++++++++++++++-----------------------
> >>  1 file changed, 147 insertions(+), 102 deletions(-)
> >
> >One nit, otherwise:
> >
> >Acked-by: Rob Herring <robh@kernel.org>
> >
> >[...]
> >
> >> +               /* And we process the "ibm,phandle" property
> >> +                * used in pSeries dynamic device tree
> >> +                * stuff
> >> +                */
> >> +               if (!strcmp(pname, "ibm,phandle"))
> >> +                       np->phandle = be32_to_cpup(val);
> >> +
> >> +               pp->name   = (char *)pname;
> >> +               pp->length = sz;
> >> +               pp->value  = (__be32 *)val;
> >
> >This cast should not be needed.
> >
> 
> It's needed. Otherwise, we will have warning. So I will keep it. I just
> went through this one for next revision and sorry for late response.
> 
> drivers/of/fdt.c:225:14: warning: assignment discards ‘const’ qualifier from pointer target type
>    pp->value  = val;
>               ^
> 
> Thanks,
> Gavin
> 
> >> +               *pprev     = pp;
> >> +               pprev      = &pp->next;
> >> +       }
> >
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Rob Herring]

On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
> Add device tree binding documentation for Vybrid SoC.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> new file mode 100644
> index 0000000..bdd95e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> @@ -0,0 +1,35 @@
> +Vybrid System-on-Chip
> +---------------------
> +
> +Required properties:
> +
> +- #address-cells: must be 1
> +- #size-cells: must be 1
> +- compatible: "fsl,vf610-soc-bus", "simple-bus"

If this is a bus, put the file in bindings/bus/...

> +- interrupt-parent: phandle to the MSCM interrupt router node
> +- ranges
> +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom
> +  revision register

Why is this needed here? Can't you search the tree for the ROM node and 
get this info.

> +- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of
> +  CPU count register
> +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of
> +  L2 cache size register
> +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"

How are all these properties used?

> +
> +Example:
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "fsl,vf610-soc-bus", "simple-bus";
> +		interrupt-parent = <&mscm_ir>;
> +		ranges;
> +		fsl,rom-revision = <&ocrom 0x80>;
> +		fsl,cpu-count = <&mscm_cpucfg 0x2C>;
> +		fsl,l2-size = <&mscm_cpucfg 0x14>;
> +		nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
> +		nvmem-cell-names = "cfg0", "cfg1";
> +
> +		...
> +	};
> -- 
> 2.8.2
> 

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[maitysanchayan]

Hello Rob,

On 16-05-03 21:30:26, Rob Herring wrote:
> On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
> > Add device tree binding documentation for Vybrid SoC.
> > 
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> > new file mode 100644
> > index 0000000..bdd95e8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> > @@ -0,0 +1,35 @@
> > +Vybrid System-on-Chip
> > +---------------------
> > +
> > +Required properties:
> > +
> > +- #address-cells: must be 1
> > +- #size-cells: must be 1
> > +- compatible: "fsl,vf610-soc-bus", "simple-bus"
> 
> If this is a bus, put the file in bindings/bus/...

The fsl,vf610-soc-bus binding is used to bind the driver in question with
an appropriate compatible node.

Basically being a standalone platform driver, there was need of a compatible
property to bind on. Introducing a separate device tree node for it's sake
didn't seem appropriate so the alteration to SoC node's compatible.

> 
> > +- interrupt-parent: phandle to the MSCM interrupt router node
> > +- ranges
> > +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom
> > +  revision register
> 
> Why is this needed here? Can't you search the tree for the ROM node and 
> get this info.

Strictly per say this and next two can be specified in their respective nodes
of ocrom and mscm cpucfg, however they would then require the use of syscon_
regmap_lookup_by_compatible and this seems clean along with the introduction
of new syscon_regmap_read_from_offset function used with SoC node.

> 
> > +- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of
> > +  CPU count register
> > +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of
> > +  L2 cache size register
> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> How are all these properties used?

All the above five are used to get the relevant values from the registers and
expose the information for SoC sysfs device.
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc

nvmem are consumer nodes which are accessed using the NVMEM consumer API's
leveraging the NVMEM framework and NVMEM vf610 ocotp driver.

Regards,
Sanchayan.

> 
> > +
> > +Example:
> > +
> > +	soc {
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		compatible = "fsl,vf610-soc-bus", "simple-bus";
> > +		interrupt-parent = <&mscm_ir>;
> > +		ranges;
> > +		fsl,rom-revision = <&ocrom 0x80>;
> > +		fsl,cpu-count = <&mscm_cpucfg 0x2C>;
> > +		fsl,l2-size = <&mscm_cpucfg 0x14>;
> > +		nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
> > +		nvmem-cell-names = "cfg0", "cfg1";
> > +
> > +		...
> > +	};
> > -- 
> > 2.8.2
> > 

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Rob Herring]

On Thu, May 5, 2016 at 3:27 AM,  <maitysanchayan@gmail.com> wrote:
> Hello Rob,
>
> On 16-05-03 21:30:26, Rob Herring wrote:
>> On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
>> > Add device tree binding documentation for Vybrid SoC.
>> >
>> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>> > ---
>> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
>> >  1 file changed, 35 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>> > new file mode 100644
>> > index 0000000..bdd95e8
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>> > @@ -0,0 +1,35 @@
>> > +Vybrid System-on-Chip
>> > +---------------------
>> > +
>> > +Required properties:
>> > +
>> > +- #address-cells: must be 1
>> > +- #size-cells: must be 1
>> > +- compatible: "fsl,vf610-soc-bus", "simple-bus"
>>
>> If this is a bus, put the file in bindings/bus/...
>
> The fsl,vf610-soc-bus binding is used to bind the driver in question with
> an appropriate compatible node.
>
> Basically being a standalone platform driver, there was need of a compatible
> property to bind on. Introducing a separate device tree node for it's sake
> didn't seem appropriate so the alteration to SoC node's compatible.

Ah, so you are designing a node around the needs of a Linux specific
driver. Don't do that. DT describes the h/w and this node is not a h/w
block.

Create a platform device based on a matching SOC compatible string
instead and make your driver find the information it needs directly
from the relevant nodes like the ROM node.

>> > +- interrupt-parent: phandle to the MSCM interrupt router node
>> > +- ranges
>> > +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom
>> > +  revision register
>>
>> Why is this needed here? Can't you search the tree for the ROM node and
>> get this info.
>
> Strictly per say this and next two can be specified in their respective nodes
> of ocrom and mscm cpucfg, however they would then require the use of syscon_
> regmap_lookup_by_compatible and this seems clean along with the introduction
> of new syscon_regmap_read_from_offset function used with SoC node.

That does not make sense. You can find the ROM node by compatible
string, get the address, get an revision offset property, and read the
address. There's no need for syscon nor regmap here.

>> > +- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of
>> > +  CPU count register
>> > +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of
>> > +  L2 cache size register
>> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
>> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
>>
>> How are all these properties used?
>
> All the above five are used to get the relevant values from the registers and
> expose the information for SoC sysfs device.
> https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc
>
> nvmem are consumer nodes which are accessed using the NVMEM consumer API's
> leveraging the NVMEM framework and NVMEM vf610 ocotp driver.

So, please describe in the binding doc what the values contain. cfg0
and cfg1 is meaningless. But based on the above, this whole binding
should go.

Rob

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Stefan Agner]

On 2016-05-09 10:14, Rob Herring wrote:
> On Thu, May 5, 2016 at 3:27 AM,  <maitysanchayan@gmail.com> wrote:
>> Hello Rob,
>>
>> On 16-05-03 21:30:26, Rob Herring wrote:
>>> On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
>>> > Add device tree binding documentation for Vybrid SoC.
>>> >
>>> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>> > ---
>>> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
>>> >  1 file changed, 35 insertions(+)
>>> >  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>> > new file mode 100644
>>> > index 0000000..bdd95e8
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>> > @@ -0,0 +1,35 @@
>>> > +Vybrid System-on-Chip
>>> > +---------------------
>>> > +
>>> > +Required properties:
>>> > +
>>> > +- #address-cells: must be 1
>>> > +- #size-cells: must be 1
>>> > +- compatible: "fsl,vf610-soc-bus", "simple-bus"
>>>
>>> If this is a bus, put the file in bindings/bus/...
>>
>> The fsl,vf610-soc-bus binding is used to bind the driver in question with
>> an appropriate compatible node.
>>
>> Basically being a standalone platform driver, there was need of a compatible
>> property to bind on. Introducing a separate device tree node for it's sake
>> didn't seem appropriate so the alteration to SoC node's compatible.
> 
> Ah, so you are designing a node around the needs of a Linux specific
> driver. Don't do that. DT describes the h/w and this node is not a h/w
> block.
> 
> Create a platform device based on a matching SOC compatible string
> instead and make your driver find the information it needs directly
> from the relevant nodes like the ROM node.

That reads like my words a year ago:
https://lkml.org/lkml/2015/5/22/408

Initially pretty much everything was hard-coded in the driver.

Arnd then pushed to use more descriptive in the device tree.

Of course, we should not end up making up relations which are not there
in hardware. We need to find the right balance.

Here is my suggestion:
1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it
to bind the "soc bus driver" as a platform driver located in driver/soc/
2. Add ROM as syscon device (it is not erasable ROM memory, hence eeprom
seems not to be appropriate)
3. In the new soc bus driver, search for the relevant nodes using
hardcoded strings:
   - "ocrom" to get the syscon device, read the ROM revision with the
hardcoded offset
   - "ocotp" to get the NVMEM device or "cfg0"/"cfg1" to the cells
directly, read the values using the defined cells.
   - "mscm_cpucfg" is already there as a syscon device

Arnd, Rob, does that sound reasonable?

--
Stefan

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Rob Herring]

On Mon, May 9, 2016 at 1:25 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-05-09 10:14, Rob Herring wrote:
>> On Thu, May 5, 2016 at 3:27 AM,  <maitysanchayan@gmail.com> wrote:
>>> Hello Rob,
>>>
>>> On 16-05-03 21:30:26, Rob Herring wrote:
>>>> On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
>>>> > Add device tree binding documentation for Vybrid SoC.
>>>> >
>>>> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>> > ---
>>>> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
>>>> >  1 file changed, 35 insertions(+)
>>>> >  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>>> >
>>>> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>>> > new file mode 100644
>>>> > index 0000000..bdd95e8
>>>> > --- /dev/null
>>>> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>>> > @@ -0,0 +1,35 @@
>>>> > +Vybrid System-on-Chip
>>>> > +---------------------
>>>> > +
>>>> > +Required properties:
>>>> > +
>>>> > +- #address-cells: must be 1
>>>> > +- #size-cells: must be 1
>>>> > +- compatible: "fsl,vf610-soc-bus", "simple-bus"
>>>>
>>>> If this is a bus, put the file in bindings/bus/...
>>>
>>> The fsl,vf610-soc-bus binding is used to bind the driver in question with
>>> an appropriate compatible node.
>>>
>>> Basically being a standalone platform driver, there was need of a compatible
>>> property to bind on. Introducing a separate device tree node for it's sake
>>> didn't seem appropriate so the alteration to SoC node's compatible.
>>
>> Ah, so you are designing a node around the needs of a Linux specific
>> driver. Don't do that. DT describes the h/w and this node is not a h/w
>> block.
>>
>> Create a platform device based on a matching SOC compatible string
>> instead and make your driver find the information it needs directly
>> from the relevant nodes like the ROM node.
>
> That reads like my words a year ago:
> https://lkml.org/lkml/2015/5/22/408
>
> Initially pretty much everything was hard-coded in the driver.
>
> Arnd then pushed to use more descriptive in the device tree.
>
> Of course, we should not end up making up relations which are not there
> in hardware. We need to find the right balance.
>
> Here is my suggestion:
> 1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it
> to bind the "soc bus driver" as a platform driver located in driver/soc/

I'm not convinced this is a h/w block. This keeps coming up and I
think this is a kernel problem, not a DT problem. Let's face it that
there are drivers at the SOC level which don't fit into a DT node.
They may be the exception, but they are a common exception. My
proposal for how to deal with these cases is here[1].

I also think drivers/soc is a mess because it is randomly used or not.
IMO, using it should not be at the whim of whomever does SOC support.

> 2. Add ROM as syscon device (it is not erasable ROM memory, hence eeprom
> seems not to be appropriate)

It is not a syscon. It is just memory. nvmem covers read-only memory,
so I have no issue using that.

Rob

[1] https://lkml.org/lkml/2013/10/30/27

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Stefan Agner]

On 2016-05-09 15:58, Rob Herring wrote:
> On Mon, May 9, 2016 at 1:25 PM, Stefan Agner <stefan@agner.ch> wrote:
>> On 2016-05-09 10:14, Rob Herring wrote:
>>> On Thu, May 5, 2016 at 3:27 AM,  <maitysanchayan@gmail.com> wrote:
>>>> Hello Rob,
>>>>
>>>> On 16-05-03 21:30:26, Rob Herring wrote:
>>>>> On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
>>>>> > Add device tree binding documentation for Vybrid SoC.
>>>>> >
>>>>> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>> > ---
>>>>> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       | 35 ++++++++++++++++++++++
>>>>> >  1 file changed, 35 insertions(+)
>>>>> >  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>>>> >
>>>>> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>>>> > new file mode 100644
>>>>> > index 0000000..bdd95e8
>>>>> > --- /dev/null
>>>>> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>>>>> > @@ -0,0 +1,35 @@
>>>>> > +Vybrid System-on-Chip
>>>>> > +---------------------
>>>>> > +
>>>>> > +Required properties:
>>>>> > +
>>>>> > +- #address-cells: must be 1
>>>>> > +- #size-cells: must be 1
>>>>> > +- compatible: "fsl,vf610-soc-bus", "simple-bus"
>>>>>
>>>>> If this is a bus, put the file in bindings/bus/...
>>>>
>>>> The fsl,vf610-soc-bus binding is used to bind the driver in question with
>>>> an appropriate compatible node.
>>>>
>>>> Basically being a standalone platform driver, there was need of a compatible
>>>> property to bind on. Introducing a separate device tree node for it's sake
>>>> didn't seem appropriate so the alteration to SoC node's compatible.
>>>
>>> Ah, so you are designing a node around the needs of a Linux specific
>>> driver. Don't do that. DT describes the h/w and this node is not a h/w
>>> block.
>>>
>>> Create a platform device based on a matching SOC compatible string
>>> instead and make your driver find the information it needs directly
>>> from the relevant nodes like the ROM node.
>>
>> That reads like my words a year ago:
>> https://lkml.org/lkml/2015/5/22/408
>>
>> Initially pretty much everything was hard-coded in the driver.
>>
>> Arnd then pushed to use more descriptive in the device tree.
>>
>> Of course, we should not end up making up relations which are not there
>> in hardware. We need to find the right balance.
>>
>> Here is my suggestion:
>> 1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it
>> to bind the "soc bus driver" as a platform driver located in driver/soc/
> 
> I'm not convinced this is a h/w block. This keeps coming up and I
> think this is a kernel problem, not a DT problem. Let's face it that
> there are drivers at the SOC level which don't fit into a DT node.
> They may be the exception, but they are a common exception. My
> proposal for how to deal with these cases is here[1].

Probably "fsl,vf610-soc" would be better than "fsl,vf610-soc-bus".

But the SoC is definitely a h/w block! Despite all the individual
silicon in there, I can even touch the SoC ;-)

But yeah, I understand what you are saying... We model it as a
container, and do not attribute any specific thing directly to it. But
aren't there other containers where we bind to? Is it wrong to bind a
driver to a container, if that driver implements issues concerning that
level..?

The two drivers under drivers/soc/ which implement the SoC bus API and
both use a compatible string on a container level (e.g.
arm,realview-pb11mp-soc in arch/arm/boot/dts/arm-realview-pb11mp.dts).

> 
> I also think drivers/soc is a mess because it is randomly used or not.
> IMO, using it should not be at the whim of whomever does SOC support.

Are you talking about random drivers under drivers/soc/ or the ones
which implement the SoC bus API? I think that is not the same...

If SoC bus, agreed, it is a mess. Not only its adoption is sparse, the
specification is also interpreted differently across different SoC's,
which I brought up in the past:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333765.html

But it exports really useful information to user space, and that has
real value in practice.

--
Stefan

Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation

[Rob Herring]

On Mon, May 9, 2016 at 8:13 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-05-09 15:58, Rob Herring wrote:
>> On Mon, May 9, 2016 at 1:25 PM, Stefan Agner <stefan@agner.ch> wrote:
>>> On 2016-05-09 10:14, Rob Herring wrote:
>>>> On Thu, May 5, 2016 at 3:27 AM,  <maitysanchayan@gmail.com> wrote:
>>>>> Hello Rob,
>>>>>
>>>>> On 16-05-03 21:30:26, Rob Herring wrote:
>>>>>> On Mon, May 02, 2016 at 12:35:04PM +0530, Sanchayan Maity wrote:
>>>>>> > Add device tree binding documentation for Vybrid SoC.

[...]

>>>>> Basically being a standalone platform driver, there was need of a compatible
>>>>> property to bind on. Introducing a separate device tree node for it's sake
>>>>> didn't seem appropriate so the alteration to SoC node's compatible.
>>>>
>>>> Ah, so you are designing a node around the needs of a Linux specific
>>>> driver. Don't do that. DT describes the h/w and this node is not a h/w
>>>> block.
>>>>
>>>> Create a platform device based on a matching SOC compatible string
>>>> instead and make your driver find the information it needs directly
>>>> from the relevant nodes like the ROM node.
>>>
>>> That reads like my words a year ago:
>>> https://lkml.org/lkml/2015/5/22/408
>>>
>>> Initially pretty much everything was hard-coded in the driver.
>>>
>>> Arnd then pushed to use more descriptive in the device tree.
>>>
>>> Of course, we should not end up making up relations which are not there
>>> in hardware. We need to find the right balance.
>>>
>>> Here is my suggestion:
>>> 1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it
>>> to bind the "soc bus driver" as a platform driver located in driver/soc/
>>
>> I'm not convinced this is a h/w block. This keeps coming up and I
>> think this is a kernel problem, not a DT problem. Let's face it that
>> there are drivers at the SOC level which don't fit into a DT node.
>> They may be the exception, but they are a common exception. My
>> proposal for how to deal with these cases is here[1].
>
> Probably "fsl,vf610-soc" would be better than "fsl,vf610-soc-bus".

But the node in question is just the bus part of the SoC. Things like
cpus are not included.

> But the SoC is definitely a h/w block! Despite all the individual
> silicon in there, I can even touch the SoC ;-)

Can't argue with that...

> But yeah, I understand what you are saying... We model it as a
> container, and do not attribute any specific thing directly to it. But
> aren't there other containers where we bind to? Is it wrong to bind a
> driver to a container, if that driver implements issues concerning that
> level..?

It is supposed to be a bus, not just a container. However, we probably
don't generally fully and accurately model the buses in SoCs so DT
ends up being just a container frequently. If the bus is more than
simple-bus, then it should have some resource that needs to be
controlled to enable the bus.

>
> The two drivers under drivers/soc/ which implement the SoC bus API and
> both use a compatible string on a container level (e.g.
> arm,realview-pb11mp-soc in arch/arm/boot/dts/arm-realview-pb11mp.dts).

I'm not sure Realview and Integrator are models to follow. They are a
bit unique in their h/w design.

>
>>
>> I also think drivers/soc is a mess because it is randomly used or not.
>> IMO, using it should not be at the whim of whomever does SOC support.
>
> Are you talking about random drivers under drivers/soc/ or the ones
> which implement the SoC bus API? I think that is not the same...

The SoC bus API.

> If SoC bus, agreed, it is a mess. Not only its adoption is sparse, the
> specification is also interpreted differently across different SoC's,
> which I brought up in the past:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333765.html
>
> But it exports really useful information to user space, and that has
> real value in practice.

Yes, but the information exported has nothing really to do with a bus.
The "bus" part of it is more sub-classing platform_bus_type for all
platform devices on an SOC. That container may or may not directly
correspond to the bus structure of the SOC as defined in DT. I guess
it is the mixing of really 2 independent things that I don't like and
has made its use sparse. We should perhaps mandate using the bus (for
what I described) and make the information part separate and be able
to populate it from any driver (e.g. an SoC syscon driver which reads
the SOC revision).

Rob