linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Implement SoC driver for Vybrid
@ 2016-05-20 10:02 Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 1/4] ARM: dts: vfxxx: Add device tree node for OCOTP Sanchayan Maity
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Sanchayan Maity @ 2016-05-20 10:02 UTC (permalink / raw)
  To: arnd, shawnguo
  Cc: stefan, robh+dt, linux-arm-kernel, devicetree, linux-kernel,
	Sanchayan Maity

Hello,

This third 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.

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.

@Rob Herring
Does this patchset address the concerns you had?

Changes since v2:
1. Remove syscon_regmap_read_from_offset function and use the
available syscon functions
2. Remove fsl,vf610-soc-bus and related bindings at SoC node
level and introduce a fsl,vf610-soc node which is used by the
driver to bind and has all the required phandles plus the NVMEM
consumer handles.
3. Fix memory leak. of_node_put was not called for returned node
of of_parse_phandle and memory allocated by nvmem_cell_read was
not freed explicitly in return error paths.

Changes since v1:
Add device tree binding documentation.

2016: v2 patchset
https://lkml.org/lkml/2016/5/2/69

2016: v1 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 (4):
  ARM: dts: vfxxx: Add device tree node for OCOTP
  ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid
  ARM: dts: vfxxx: Add device tree node required by Vybrid SoC driver
  soc: Add SoC driver for Freescale Vybrid platform

 .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
 arch/arm/boot/dts/vfxxx.dtsi                       |  29 +++
 drivers/soc/Kconfig                                |   1 +
 drivers/soc/fsl/Kconfig                            |  10 ++
 drivers/soc/fsl/Makefile                           |   1 +
 drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
 6 files changed, 259 insertions(+)
 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

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

* [PATCH v3 1/4] ARM: dts: vfxxx: Add device tree node for OCOTP
  2016-05-20 10:02 [PATCH v3 0/4] Implement SoC driver for Vybrid Sanchayan Maity
@ 2016-05-20 10:02 ` Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 2/4] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid Sanchayan Maity
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Sanchayan Maity @ 2016-05-20 10:02 UTC (permalink / raw)
  To: arnd, shawnguo
  Cc: stefan, robh+dt, linux-arm-kernel, devicetree, linux-kernel,
	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

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

* [PATCH v3 2/4] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid
  2016-05-20 10:02 [PATCH v3 0/4] Implement SoC driver for Vybrid Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 1/4] ARM: dts: vfxxx: Add device tree node for OCOTP Sanchayan Maity
@ 2016-05-20 10:02 ` Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 3/4] ARM: dts: vfxxx: Add device tree node required by Vybrid SoC driver Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform Sanchayan Maity
  3 siblings, 0 replies; 15+ messages in thread
From: Sanchayan Maity @ 2016-05-20 10:02 UTC (permalink / raw)
  To: arnd, shawnguo
  Cc: stefan, robh+dt, linux-arm-kernel, devicetree, linux-kernel,
	Sanchayan Maity

Add a device tree node for the On-Chip ROM on Vybrid.

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

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 0e34d44..6c5222e 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -91,6 +91,11 @@
 		interrupt-parent = <&mscm_ir>;
 		ranges;
 
+		ocrom: ocrom@00000000 {
+			compatible = "fsl,vf610-ocrom", "syscon";
+			reg = <0x00000000 0x18000>;
+		};
+
 		aips0: aips-bus@40000000 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			#address-cells = <1>;
-- 
2.8.2

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

* [PATCH v3 3/4] ARM: dts: vfxxx: Add device tree node required by Vybrid SoC driver
  2016-05-20 10:02 [PATCH v3 0/4] Implement SoC driver for Vybrid Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 1/4] ARM: dts: vfxxx: Add device tree node for OCOTP Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 2/4] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid Sanchayan Maity
@ 2016-05-20 10:02 ` Sanchayan Maity
  2016-05-20 10:02 ` [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform Sanchayan Maity
  3 siblings, 0 replies; 15+ messages in thread
From: Sanchayan Maity @ 2016-05-20 10:02 UTC (permalink / raw)
  To: arnd, shawnguo
  Cc: stefan, robh+dt, linux-arm-kernel, devicetree, linux-kernel,
	Sanchayan Maity

Add a device tree node which will be used to bind the Vybrid
SoC driver and provide information adhering to the following:
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc

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

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 6c5222e..24979d6 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -770,5 +770,13 @@
 			compatible = "iio-hwmon";
 			io-channels = <&adc0 16>, <&adc1 16>;
 		};
+
+		vf610-soc {
+			compatible = "fsl,vf610-soc";
+			rom-revision = <&ocrom>;
+			mscm = <&mscm_cpucfg>;
+			nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
+			nvmem-cell-names = "cfg0", "cfg1";
+		};
 	};
 };
-- 
2.8.2

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

* [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-20 10:02 [PATCH v3 0/4] Implement SoC driver for Vybrid Sanchayan Maity
                   ` (2 preceding siblings ...)
  2016-05-20 10:02 ` [PATCH v3 3/4] ARM: dts: vfxxx: Add device tree node required by Vybrid SoC driver Sanchayan Maity
@ 2016-05-20 10:02 ` Sanchayan Maity
  2016-05-23 21:18   ` Rob Herring
  3 siblings, 1 reply; 15+ messages in thread
From: Sanchayan Maity @ 2016-05-20 10:02 UTC (permalink / raw)
  To: arnd, shawnguo
  Cc: stefan, robh+dt, linux-arm-kernel, devicetree, linux-kernel,
	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 expose the SoC specific
properties via sysfs.

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>
---
 .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
 drivers/soc/Kconfig                                |   1 +
 drivers/soc/fsl/Kconfig                            |  10 ++
 drivers/soc/fsl/Makefile                           |   1 +
 drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
 5 files changed, 230 insertions(+)
 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

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..338905d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
@@ -0,0 +1,20 @@
+Vybrid System-on-Chip
+---------------------
+
+Required properties:
+
+- compatible: "fsl,vf610-soc"
+- rom-revision: phandle to the on-chip ROM node
+- mscm: phandle to the MSCM CPU configuration node
+- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
+- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
+
+Example:
+
+	vf610-soc {
+		compatible = "fsl,vf610-soc";
+		rom-revision = <&ocrom>;
+		mscm = <&mscm_cpucfg>;
+		nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
+		nvmem-cell-names = "cfg0", "cfg1";
+	};
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..571ba69
--- /dev/null
+++ b/drivers/soc/fsl/soc-vf610.c
@@ -0,0 +1,198 @@
+/*
+ * 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>
+
+#define MSCM_CPxCOUNT_OFFSET	0x2C
+#define MSCM_CPxCFG1_OFFSET	0x14
+#define ROM_REVISION_OFFSET	0x80
+
+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 *rom_node, *mscm_node;
+	struct regmap *rom_regmap, *mscm_regmap;
+	char soc_type[] = "xx0";
+	size_t id1_len, id2_len;
+	u32 cpucount, l2size, rom_rev;
+	u8 *socid1, *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));
+		ret = PTR_ERR(socid2);
+		goto out_socid2;
+	}
+	add_device_randomness(socid1, id1_len);
+	add_device_randomness(socid2, id2_len);
+
+	rom_node = of_parse_phandle(pdev->dev.of_node, "rom-revision", 0);
+	if (!rom_node) {
+		dev_err(dev, "Lookup failed for rom-revision node\n");
+		ret = -ENODEV;
+		goto out_rom_node;
+	}
+
+	mscm_node = of_parse_phandle(pdev->dev.of_node, "mscm", 0);
+	if (!mscm_node) {
+		dev_err(dev, "lookup failed for mscm node\n");
+		ret = -ENODEV;
+		goto out_mscm_node;
+	}
+
+	rom_regmap = syscon_node_to_regmap(rom_node);
+	if (IS_ERR(rom_regmap)) {
+		dev_err(dev, "regmap lookup for ocrom failed %ld\n",
+			PTR_ERR(rom_regmap));
+		ret = PTR_ERR(rom_regmap);
+		goto out;
+	}
+
+	mscm_regmap = syscon_node_to_regmap(mscm_node);
+	if (IS_ERR(mscm_regmap)) {
+		dev_err(dev, "regmap lookup for mscm failed %ld\n",
+			PTR_ERR(mscm_regmap));
+		ret = PTR_ERR(mscm_regmap);
+		goto out;
+	}
+
+	ret = regmap_read(rom_regmap, ROM_REVISION_OFFSET, &rom_rev);
+	if (ret) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = regmap_read(mscm_regmap, MSCM_CPxCOUNT_OFFSET, &cpucount);
+	if (ret) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = regmap_read(mscm_regmap, MSCM_CPxCFG1_OFFSET, &l2size);
+	if (ret) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	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) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	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)) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	of_node_put(mscm_node);
+out_mscm_node:
+	of_node_put(rom_node);
+out_rom_node:
+	kfree(socid2);
+out_socid2:
+	kfree(socid1);
+
+	return ret;
+}
+
+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_match[] = {
+	{ .compatible = "fsl,vf610-soc", },
+	{ /* */ }
+};
+
+static struct platform_driver vf610_soc_driver = {
+	.probe		= vf610_soc_probe,
+	.remove		= vf610_soc_remove,
+	.driver		= {
+		.name = "vf610-soc",
+		.of_match_table = vf610_soc_match,
+	},
+};
+builtin_platform_driver(vf610_soc_driver);
-- 
2.8.2

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-20 10:02 ` [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform Sanchayan Maity
@ 2016-05-23 21:18   ` Rob Herring
  2016-05-24  4:14     ` maitysanchayan
  0 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2016-05-23 21:18 UTC (permalink / raw)
  To: Sanchayan Maity
  Cc: arnd, shawnguo, stefan, linux-arm-kernel, devicetree, linux-kernel

On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote:
> 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 expose the SoC specific
> properties via sysfs.
> 
> 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>
> ---
>  .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
>  drivers/soc/Kconfig                                |   1 +
>  drivers/soc/fsl/Kconfig                            |  10 ++
>  drivers/soc/fsl/Makefile                           |   1 +
>  drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
>  5 files changed, 230 insertions(+)
>  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
> 
> 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..338905d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> @@ -0,0 +1,20 @@
> +Vybrid System-on-Chip
> +---------------------
> +
> +Required properties:
> +
> +- compatible: "fsl,vf610-soc"
> +- rom-revision: phandle to the on-chip ROM node
> +- mscm: phandle to the MSCM CPU configuration node
> +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"

I still have similar concerns as the discussion on the last version. 
This version only proves that you aren't describing h/w, but rather just 
a collection of data that some driver wants.

A driver can just as easily look-up all the nodes directly that these 
phandles point to.

And as long as we have inconsistent use of soc_device, I don't want to 
see any compatible strings related to it.

Rob

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-23 21:18   ` Rob Herring
@ 2016-05-24  4:14     ` maitysanchayan
  2016-05-24 17:09       ` Rob Herring
  0 siblings, 1 reply; 15+ messages in thread
From: maitysanchayan @ 2016-05-24  4:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: arnd, shawnguo, stefan, linux-arm-kernel, devicetree, linux-kernel

Hello Rob,

On 16-05-23 16:18:13, Rob Herring wrote:
> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote:
> > 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 expose the SoC specific
> > properties via sysfs.
> > 
> > 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>
> > ---
> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
> >  drivers/soc/Kconfig                                |   1 +
> >  drivers/soc/fsl/Kconfig                            |  10 ++
> >  drivers/soc/fsl/Makefile                           |   1 +
> >  drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
> >  5 files changed, 230 insertions(+)
> >  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
> > 
> > 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..338905d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> > @@ -0,0 +1,20 @@
> > +Vybrid System-on-Chip
> > +---------------------
> > +
> > +Required properties:
> > +
> > +- compatible: "fsl,vf610-soc"
> > +- rom-revision: phandle to the on-chip ROM node
> > +- mscm: phandle to the MSCM CPU configuration node
> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> I still have similar concerns as the discussion on the last version. 
> This version only proves that you aren't describing h/w, but rather just 
> a collection of data that some driver wants.
> 
> A driver can just as easily look-up all the nodes directly that these 
> phandles point to.

Agreed, that we can look up all the nodes directly that these phandles
refer to but I would still need a DT entry to bind to. While I could
bind to existing nodes like mscm cpucfg but that doesn't seem right.

The very first approach that we had taken was to integrate this functionality
in mach-vf610.c code under mach-imx
http://www.spinics.net/lists/devicetree/msg80654.html

and then it was recommended to migrate this to drivers/soc where we did use
phandles or direct look up via compatible strings

http://www.spinics.net/lists/arm-kernel/msg420847.html

and

http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html

There hasn't been a consensus since v1.

> 
> And as long as we have inconsistent use of soc_device, I don't want to 
> see any compatible strings related to it.
> 

Regards,
Sanchayan.

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-24  4:14     ` maitysanchayan
@ 2016-05-24 17:09       ` Rob Herring
  2016-05-25 15:18         ` Arnd Bergmann
  2016-05-27  6:33         ` maitysanchayan
  0 siblings, 2 replies; 15+ messages in thread
From: Rob Herring @ 2016-05-24 17:09 UTC (permalink / raw)
  To: Sanchayan Maity
  Cc: Arnd Bergmann, Shawn Guo, Stefan Agner, linux-arm-kernel,
	devicetree, linux-kernel

On Mon, May 23, 2016 at 11:14 PM,  <maitysanchayan@gmail.com> wrote:
> Hello Rob,
>
> On 16-05-23 16:18:13, Rob Herring wrote:
>> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote:
>> > 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 expose the SoC specific
>> > properties via sysfs.
>> >
>> > 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>
>> > ---
>> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
>> >  drivers/soc/Kconfig                                |   1 +
>> >  drivers/soc/fsl/Kconfig                            |  10 ++
>> >  drivers/soc/fsl/Makefile                           |   1 +
>> >  drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
>> >  5 files changed, 230 insertions(+)
>> >  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
>> >
>> > 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..338905d
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
>> > @@ -0,0 +1,20 @@
>> > +Vybrid System-on-Chip
>> > +---------------------
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: "fsl,vf610-soc"
>> > +- rom-revision: phandle to the on-chip ROM node
>> > +- mscm: phandle to the MSCM CPU configuration node
>> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
>> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
>>
>> I still have similar concerns as the discussion on the last version.
>> This version only proves that you aren't describing h/w, but rather just
>> a collection of data that some driver wants.
>>
>> A driver can just as easily look-up all the nodes directly that these
>> phandles point to.
>
> Agreed, that we can look up all the nodes directly that these phandles
> refer to but I would still need a DT entry to bind to. While I could
> bind to existing nodes like mscm cpucfg but that doesn't seem right.
>
> The very first approach that we had taken was to integrate this functionality
> in mach-vf610.c code under mach-imx
> http://www.spinics.net/lists/devicetree/msg80654.html

Yes, everyone wants to move all platform devices in the kernel to a
corresponding DT node. The result is often making up nodes to do this.
It's the same thing with cpufreq.

> and then it was recommended to migrate this to drivers/soc where we did use
> phandles or direct look up via compatible strings

The location in the tree is an orthogonal issue. You could move it and
use of_machine_is_compatible without any DT change.

The primary issue I have here is how do we bind soc_bus to DT in a
consistent way. Sorry, but vybrid specific patches alone are never
going to solve that issue.

> http://www.spinics.net/lists/arm-kernel/msg420847.html
>
> and
>
> http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html
>
> There hasn't been a consensus since v1.

I actually prefer your previous version binding soc_bus to the root
bus node to this version. I think that is closer to the right
direction. After all, pretty much everything is an SOC and every SOC
has an SOC bus. Pretty much every SOC and board have revisions and may
need to expose that revision info as well. We have to do this
consistently which means having a default implementation for
simple-bus that is not opt-in.

Alternatively, we should just deprecate soc_bus and come up a
different solution. Either way, I think we have a half implemented
solution currently.

Rob

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-24 17:09       ` Rob Herring
@ 2016-05-25 15:18         ` Arnd Bergmann
  2016-05-27  6:33         ` maitysanchayan
  1 sibling, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2016-05-25 15:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sanchayan Maity, Shawn Guo, Stefan Agner, linux-arm-kernel,
	devicetree, linux-kernel

On Tuesday, May 24, 2016 12:09:41 PM CEST Rob Herring wrote:
> On Mon, May 23, 2016 at 11:14 PM,  <maitysanchayan@gmail.com> wrote:
> > Hello Rob,
> >
> > On 16-05-23 16:18:13, Rob Herring wrote:
> >> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote:
> >> > 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 expose the SoC specific
> >> > properties via sysfs.
> >> >
> >> > 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>
> >> > ---
> >> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
> >> >  drivers/soc/Kconfig                                |   1 +
> >> >  drivers/soc/fsl/Kconfig                            |  10 ++
> >> >  drivers/soc/fsl/Makefile                           |   1 +
> >> >  drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
> >> >  5 files changed, 230 insertions(+)
> >> >  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
> >> >
> >> > 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..338905d
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> >> > @@ -0,0 +1,20 @@
> >> > +Vybrid System-on-Chip
> >> > +---------------------
> >> > +
> >> > +Required properties:
> >> > +
> >> > +- compatible: "fsl,vf610-soc"
> >> > +- rom-revision: phandle to the on-chip ROM node
> >> > +- mscm: phandle to the MSCM CPU configuration node
> >> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> >> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> >>
> >> I still have similar concerns as the discussion on the last version.
> >> This version only proves that you aren't describing h/w, but rather just
> >> a collection of data that some driver wants.
> >>
> >> A driver can just as easily look-up all the nodes directly that these
> >> phandles point to.
> >
> > Agreed, that we can look up all the nodes directly that these phandles
> > refer to but I would still need a DT entry to bind to. While I could
> > bind to existing nodes like mscm cpucfg but that doesn't seem right.
> >
> > The very first approach that we had taken was to integrate this functionality
> > in mach-vf610.c code under mach-imx
> > http://www.spinics.net/lists/devicetree/msg80654.html
> 
> Yes, everyone wants to move all platform devices in the kernel to a
> corresponding DT node. The result is often making up nodes to do this.
> It's the same thing with cpufreq.
> 
> > and then it was recommended to migrate this to drivers/soc where we did use
> > phandles or direct look up via compatible strings
> 
> The location in the tree is an orthogonal issue. You could move it and
> use of_machine_is_compatible without any DT change.
> 
> The primary issue I have here is how do we bind soc_bus to DT in a
> consistent way. Sorry, but vybrid specific patches alone are never
> going to solve that issue.

On a lot of chips, there is actually one device that holds the soc
identification registers, and that can easily be used for this purpose.

Originally, the soc_device was meant to be the parent device for all
other on-chip devices and be bound to the DT node that holds that
bus (unlike things like external clocks that are not part of the SoC).

However, that model has never really caught on, and we have stopped
doing this for new chips. Instead, the soc_device now somewhat
stands for itself.

> > http://www.spinics.net/lists/arm-kernel/msg420847.html
> >
> > and
> >
> > http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html
> >
> > There hasn't been a consensus since v1.
> 
> I actually prefer your previous version binding soc_bus to the root
> bus node to this version. I think that is closer to the right
> direction. After all, pretty much everything is an SOC and every SOC
> has an SOC bus. Pretty much every SOC and board have revisions and may
> need to expose that revision info as well. We have to do this
> consistently which means having a default implementation for
> simple-bus that is not opt-in.
> 
> Alternatively, we should just deprecate soc_bus and come up a
> different solution. Either way, I think we have a half implemented
> solution currently.

We have had several drivers in the past that needed to figure out
whether they were running on a particular version of a SoC, and that
could not get this information from the DT. My suggested solution
for this is to provide an API from soc_bus that compares a
glob string to the available soc_device instance(s) in order to
figure out whether we are running on a particular kind of system.

	Arnd

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-24 17:09       ` Rob Herring
  2016-05-25 15:18         ` Arnd Bergmann
@ 2016-05-27  6:33         ` maitysanchayan
  2016-05-27  8:31           ` Arnd Bergmann
  1 sibling, 1 reply; 15+ messages in thread
From: maitysanchayan @ 2016-05-27  6:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: Arnd Bergmann, Shawn Guo, Stefan Agner, linux-arm-kernel,
	devicetree, linux-kernel

Hello Rob,

On 16-05-24 12:09:41, Rob Herring wrote:
> On Mon, May 23, 2016 at 11:14 PM,  <maitysanchayan@gmail.com> wrote:
> > Hello Rob,
> >
> > On 16-05-23 16:18:13, Rob Herring wrote:
> >> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote:
> >> > 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 expose the SoC specific
> >> > properties via sysfs.
> >> >
> >> > 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>
> >> > ---
> >> >  .../bindings/arm/freescale/fsl,vf610-soc.txt       |  20 +++
> >> >  drivers/soc/Kconfig                                |   1 +
> >> >  drivers/soc/fsl/Kconfig                            |  10 ++
> >> >  drivers/soc/fsl/Makefile                           |   1 +
> >> >  drivers/soc/fsl/soc-vf610.c                        | 198 +++++++++++++++++++++
> >> >  5 files changed, 230 insertions(+)
> >> >  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
> >> >
> >> > 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..338905d
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
> >> > @@ -0,0 +1,20 @@
> >> > +Vybrid System-on-Chip
> >> > +---------------------
> >> > +
> >> > +Required properties:
> >> > +
> >> > +- compatible: "fsl,vf610-soc"
> >> > +- rom-revision: phandle to the on-chip ROM node
> >> > +- mscm: phandle to the MSCM CPU configuration node
> >> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> >> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> >>
> >> I still have similar concerns as the discussion on the last version.
> >> This version only proves that you aren't describing h/w, but rather just
> >> a collection of data that some driver wants.
> >>
> >> A driver can just as easily look-up all the nodes directly that these
> >> phandles point to.
> >
> > Agreed, that we can look up all the nodes directly that these phandles
> > refer to but I would still need a DT entry to bind to. While I could
> > bind to existing nodes like mscm cpucfg but that doesn't seem right.
> >
> > The very first approach that we had taken was to integrate this functionality
> > in mach-vf610.c code under mach-imx
> > http://www.spinics.net/lists/devicetree/msg80654.html
> 
> Yes, everyone wants to move all platform devices in the kernel to a
> corresponding DT node. The result is often making up nodes to do this.
> It's the same thing with cpufreq.
> 
> > and then it was recommended to migrate this to drivers/soc where we did use
> > phandles or direct look up via compatible strings
> 
> The location in the tree is an orthogonal issue. You could move it and
> use of_machine_is_compatible without any DT change.
> 
> The primary issue I have here is how do we bind soc_bus to DT in a
> consistent way. Sorry, but vybrid specific patches alone are never
> going to solve that issue.
> 
> > http://www.spinics.net/lists/arm-kernel/msg420847.html
> >
> > and
> >
> > http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html
> >
> > There hasn't been a consensus since v1.
> 
> I actually prefer your previous version binding soc_bus to the root
> bus node to this version. I think that is closer to the right
> direction. 

So if I understand correctly, the binding at the SoC level is fine.
Keeping that but removing the additional made-up properties, viz. below

rom-revision: phandle to the on-chip ROM node
mscm: phandle to the MSCM CPU configuration node
nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
nvmem-cell-names: should contain string names "cfg0" and "cfg1"

would be fine?

We would have something similar to here
http://www.spinics.net/lists/devicetree/msg80655.html

but now with the DT binding under SoC bus.

Regards,
Sanchayan.

> After all, pretty much everything is an SOC and every SOC
> has an SOC bus. Pretty much every SOC and board have revisions and may
> need to expose that revision info as well. We have to do this
> consistently which means having a default implementation for
> simple-bus that is not opt-in.
> 
> Alternatively, we should just deprecate soc_bus and come up a
> different solution. Either way, I think we have a half implemented
> solution currently.
> 
> Rob

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-27  6:33         ` maitysanchayan
@ 2016-05-27  8:31           ` Arnd Bergmann
  2016-05-27 10:08             ` maitysanchayan
  0 siblings, 1 reply; 15+ messages in thread
From: Arnd Bergmann @ 2016-05-27  8:31 UTC (permalink / raw)
  To: maitysanchayan
  Cc: Rob Herring, Shawn Guo, Stefan Agner, linux-arm-kernel,
	devicetree, linux-kernel

On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> 
> So if I understand correctly, the binding at the SoC level is fine.
> Keeping that but removing the additional made-up properties, viz. below
> 
> rom-revision: phandle to the on-chip ROM node
> mscm: phandle to the MSCM CPU configuration node
> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> would be fine?
> 
> We would have something similar to here
> http://www.spinics.net/lists/devicetree/msg80655.html
> 
> but now with the DT binding under SoC bus.
> 


You look up the OTP device as a syscon here, which seems odd since there
is already an nvmem driver for it. Shouldn't you use the nvmem API for
that?

	Arnd

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-27  8:31           ` Arnd Bergmann
@ 2016-05-27 10:08             ` maitysanchayan
  2016-05-27 17:28               ` Stefan Agner
  2016-06-09 10:38               ` maitysanchayan
  0 siblings, 2 replies; 15+ messages in thread
From: maitysanchayan @ 2016-05-27 10:08 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rob Herring, Shawn Guo, Stefan Agner, linux-arm-kernel,
	devicetree, linux-kernel

On 16-05-27 10:31:55, Arnd Bergmann wrote:
> On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> > 
> > So if I understand correctly, the binding at the SoC level is fine.
> > Keeping that but removing the additional made-up properties, viz. below
> > 
> > rom-revision: phandle to the on-chip ROM node
> > mscm: phandle to the MSCM CPU configuration node
> > nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> > nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> > 
> > would be fine?
> > 
> > We would have something similar to here
> > http://www.spinics.net/lists/devicetree/msg80655.html
> > 
> > but now with the DT binding under SoC bus.
> > 
> 
> 
> You look up the OTP device as a syscon here, which seems odd since there
> is already an nvmem driver for it. Shouldn't you use the nvmem API for
> that?
> 
> 	Arnd

I need the following 

nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
nvmem-cell-names: should contain string names "cfg0" and "cfg1"

to be able to use the NVMEM consumer API.

If I can put them at SoC node level then I certainly can use the NVMEM API.

Regards,
Sanchayan.

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-27 10:08             ` maitysanchayan
@ 2016-05-27 17:28               ` Stefan Agner
  2016-05-27 17:56                 ` maitysanchayan
  2016-06-09 10:38               ` maitysanchayan
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Agner @ 2016-05-27 17:28 UTC (permalink / raw)
  To: maitysanchayan
  Cc: Arnd Bergmann, Rob Herring, Shawn Guo, linux-arm-kernel,
	devicetree, linux-kernel

On 2016-05-27 03:08, maitysanchayan@gmail.com wrote:
> On 16-05-27 10:31:55, Arnd Bergmann wrote:
>> On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
>> >
>> > So if I understand correctly, the binding at the SoC level is fine.
>> > Keeping that but removing the additional made-up properties, viz. below
>> >
>> > rom-revision: phandle to the on-chip ROM node
>> > mscm: phandle to the MSCM CPU configuration node
>> > nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
>> > nvmem-cell-names: should contain string names "cfg0" and "cfg1"
>> >
>> > would be fine?
>> >
>> > We would have something similar to here
>> > http://www.spinics.net/lists/devicetree/msg80655.html
>> >
>> > but now with the DT binding under SoC bus.
>> >
>>
>>
>> You look up the OTP device as a syscon here, which seems odd since there
>> is already an nvmem driver for it. Shouldn't you use the nvmem API for
>> that?
>>
>> 	Arnd
> 
> I need the following 
> 
> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> to be able to use the NVMEM consumer API.

I did not tested it, but it seems the NVMEM consumer API has some kind
of non-DT fallback:
http://lxr.free-electrons.com/source/drivers/nvmem/core.c#L827

Right now, it seems to me that it does not handle the case where we have
a of_node but don't want to use it... You might need to add some extra
handling if there is a of_node without nvmem-cell-names/nvmem-cells, and
fall back to nvmem_cell_get_from_list.

--
Stefan

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-27 17:28               ` Stefan Agner
@ 2016-05-27 17:56                 ` maitysanchayan
  0 siblings, 0 replies; 15+ messages in thread
From: maitysanchayan @ 2016-05-27 17:56 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Arnd Bergmann, Rob Herring, Shawn Guo, linux-arm-kernel,
	devicetree, linux-kernel

Hello Stefan,

On 16-05-27 10:28:45, Stefan Agner wrote:
> On 2016-05-27 03:08, maitysanchayan@gmail.com wrote:
> > On 16-05-27 10:31:55, Arnd Bergmann wrote:
> >> On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> >> >
> >> > So if I understand correctly, the binding at the SoC level is fine.
> >> > Keeping that but removing the additional made-up properties, viz. below
> >> >
> >> > rom-revision: phandle to the on-chip ROM node
> >> > mscm: phandle to the MSCM CPU configuration node
> >> > nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> >> > nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> >> >
> >> > would be fine?
> >> >
> >> > We would have something similar to here
> >> > http://www.spinics.net/lists/devicetree/msg80655.html
> >> >
> >> > but now with the DT binding under SoC bus.
> >> >
> >>
> >>
> >> You look up the OTP device as a syscon here, which seems odd since there
> >> is already an nvmem driver for it. Shouldn't you use the nvmem API for
> >> that?
> >>
> >> 	Arnd
> > 
> > I need the following 
> > 
> > nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> > nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> > 
> > to be able to use the NVMEM consumer API.
> 
> I did not tested it, but it seems the NVMEM consumer API has some kind
> of non-DT fallback:
> http://lxr.free-electrons.com/source/drivers/nvmem/core.c#L827
> 
> Right now, it seems to me that it does not handle the case where we have
> a of_node but don't want to use it... You might need to add some extra
> handling if there is a of_node without nvmem-cell-names/nvmem-cells, and
> fall back to nvmem_cell_get_from_list.
> 

I have already looked at the core nvmem code for options but wanted to avoid
any additons if possible.

However if adding the nvmem properties at the SoC node level is frowned upon
I will make the necessary changes required.

Regards,
Sanchayan.

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

* Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform
  2016-05-27 10:08             ` maitysanchayan
  2016-05-27 17:28               ` Stefan Agner
@ 2016-06-09 10:38               ` maitysanchayan
  1 sibling, 0 replies; 15+ messages in thread
From: maitysanchayan @ 2016-06-09 10:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: arnd, Shawn Guo, Stefan Agner, linux-arm-kernel, devicetree,
	linux-kernel

Hello Rob,

On 16-05-27 15:38:17, maitysanchayan@gmail.com wrote:
> On 16-05-27 10:31:55, Arnd Bergmann wrote:
> > On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> > > 
> > > So if I understand correctly, the binding at the SoC level is fine.
> > > Keeping that but removing the additional made-up properties, viz. below
> > > 
> > > rom-revision: phandle to the on-chip ROM node
> > > mscm: phandle to the MSCM CPU configuration node
> > > nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> > > nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> > > 
> > > would be fine?
> > > 
> > > We would have something similar to here
> > > http://www.spinics.net/lists/devicetree/msg80655.html
> > > 
> > > but now with the DT binding under SoC bus.
> > > 
> > 
> > 
> > You look up the OTP device as a syscon here, which seems odd since there
> > is already an nvmem driver for it. Shouldn't you use the nvmem API for
> > that?
> > 
> > 	Arnd
> 
> I need the following 
> 
> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> to be able to use the NVMEM consumer API.
> 
> If I can put them at SoC node level then I certainly can use the NVMEM API.
> 

Would the following be acceptable at the SoC node level

> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"

Regards,
Sanchayan.
 

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

end of thread, other threads:[~2016-06-09 10:45 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-20 10:02 [PATCH v3 0/4] Implement SoC driver for Vybrid Sanchayan Maity
2016-05-20 10:02 ` [PATCH v3 1/4] ARM: dts: vfxxx: Add device tree node for OCOTP Sanchayan Maity
2016-05-20 10:02 ` [PATCH v3 2/4] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid Sanchayan Maity
2016-05-20 10:02 ` [PATCH v3 3/4] ARM: dts: vfxxx: Add device tree node required by Vybrid SoC driver Sanchayan Maity
2016-05-20 10:02 ` [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform Sanchayan Maity
2016-05-23 21:18   ` Rob Herring
2016-05-24  4:14     ` maitysanchayan
2016-05-24 17:09       ` Rob Herring
2016-05-25 15:18         ` Arnd Bergmann
2016-05-27  6:33         ` maitysanchayan
2016-05-27  8:31           ` Arnd Bergmann
2016-05-27 10:08             ` maitysanchayan
2016-05-27 17:28               ` Stefan Agner
2016-05-27 17:56                 ` maitysanchayan
2016-06-09 10:38               ` maitysanchayan

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