[PATCH v4 0/5] Implement SoC driver for Vybrid

[PATCH v4 0/5] Implement SoC driver for Vybrid

[Sanchayan Maity]

Hello,

This fourth 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? The only change
to the device tree is now for the compatible property.

@Srinivas
Is this new NVMEM consumer API acceptable? Would you recommend a
different approach?

Changes since v3:
1. Use just a compatible node at the SoC node and do not
use a separate node for the binding
2. Use syscon regmap lookup for getting information from
MSCM and OCROM nodes.
3. Introduce a NVMEM consumer API for getting a NVMEM cell
given a device node containing that cell.
4. Do not introduce any node at the SoC level.

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: v3 patchset
https://lkml.org/lkml/2016/5/20/200

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
  soc: Add SoC driver for Freescale Vybrid platform
  ARM: dts: vfxxx: Add a compatible binding for Vybrid SoC bus driver

Stefan Agner (1):
  nvmem: core: Add consumer API to get nvmem cell from node

 arch/arm/boot/dts/vfxxx.dtsi   |  23 ++++-
 drivers/nvmem/core.c           |  44 ++++++---
 drivers/soc/Kconfig            |   1 +
 drivers/soc/fsl/Kconfig        |  10 ++
 drivers/soc/fsl/Makefile       |   1 +
 drivers/soc/fsl/soc-vf610.c    | 212 +++++++++++++++++++++++++++++++++++++++++
 include/linux/nvmem-consumer.h |   1 +
 7 files changed, 278 insertions(+), 14 deletions(-)
 create mode 100644 drivers/soc/fsl/Kconfig
 create mode 100644 drivers/soc/fsl/soc-vf610.c

-- 
2.9.0

[PATCH v4 0/5] Implement SoC driver for Vybrid

[Sanchayan Maity]

Hello,

This fourth 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? The only change
to the device tree is now for the compatible property.

@Srinivas
Is this new NVMEM consumer API acceptable? Would you recommend a
different approach?

Changes since v3:
1. Use just a compatible node at the SoC node and do not
use a separate node for the binding
2. Use syscon regmap lookup for getting information from
MSCM and OCROM nodes.
3. Introduce a NVMEM consumer API for getting a NVMEM cell
given a device node containing that cell.
4. Do not introduce any node at the SoC level.

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: v3 patchset
https://lkml.org/lkml/2016/5/20/200

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
  soc: Add SoC driver for Freescale Vybrid platform
  ARM: dts: vfxxx: Add a compatible binding for Vybrid SoC bus driver

Stefan Agner (1):
  nvmem: core: Add consumer API to get nvmem cell from node

 arch/arm/boot/dts/vfxxx.dtsi   |  23 ++++-
 drivers/nvmem/core.c           |  44 ++++++---
 drivers/soc/Kconfig            |   1 +
 drivers/soc/fsl/Kconfig        |  10 ++
 drivers/soc/fsl/Makefile       |   1 +
 drivers/soc/fsl/soc-vf610.c    | 212 +++++++++++++++++++++++++++++++++++++++++
 include/linux/nvmem-consumer.h |   1 +
 7 files changed, 278 insertions(+), 14 deletions(-)
 create mode 100644 drivers/soc/fsl/Kconfig
 create mode 100644 drivers/soc/fsl/soc-vf610.c

-- 
2.9.0

[PATCH v4 1/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.9.0

[PATCH v4 1/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 at 400a5000 {
+				compatible = "fsl,vf610-ocotp";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x400a5000 0xCF0>;
+				clocks = <&clks VF610_CLK_OCOTP>;
+
+				ocotp_cfg0: cfg0 at 410 {
+					reg = <0x410 0x4>;
+				};
+
+				ocotp_cfg1: cfg1 at 420 {
+					reg = <0x420 0x4>;
+				};
+			};
+
 			snvs0: snvs at 400a7000 {
 			    compatible = "fsl,sec-v4.0-mon", "syscon", "simple-mfd";
 				reg = <0x400a7000 0x2000>;
-- 
2.9.0

[PATCH v4 2/5] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid

[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.9.0

[PATCH v4 2/5] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid

[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 at 00000000 {
+			compatible = "fsl,vf610-ocrom", "syscon";
+			reg = <0x00000000 0x18000>;
+		};
+
 		aips0: aips-bus at 40000000 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			#address-cells = <1>;
-- 
2.9.0

[PATCH v4 2/5] ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid

[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.9.0

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Sanchayan Maity]

From: Stefan Agner <stefan@agner.ch>

The existing NVMEM consumer API's do not allow to get a
NVMEM cell directly given a device tree node. This patch
adds a function to provide this functionality.

Assuming the nvmem cell id name is known, this can be used
as follows

struct device_node *cell_np;
struct nvmem_cell *foo_cell;

cell_np = of_find_node_by_name(parent, "foo");
foo_cell = of_nvmem_cell_get_direct(cell_np);

Parent node can also be the of_node of the main SoC device
node.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
 include/linux/nvmem-consumer.h |  1 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 965911d..470abee 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
 /**
- * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
+ * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
  *
- * @dev node: Device tree node that uses the nvmem cell
- * @id: nvmem cell name from nvmem-cell-names property.
+ * @dev node: Device tree node that uses nvmem cell
  *
  * Return: Will be an ERR_PTR() on error or a valid pointer
  * to a struct nvmem_cell.  The nvmem_cell will be freed by the
  * nvmem_cell_put().
  */
-struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
-					    const char *name)
+struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
 {
-	struct device_node *cell_np, *nvmem_np;
+	struct device_node *nvmem_np;
 	struct nvmem_cell *cell;
 	struct nvmem_device *nvmem;
 	const __be32 *addr;
-	int rval, len, index;
-
-	index = of_property_match_string(np, "nvmem-cell-names", name);
-
-	cell_np = of_parse_phandle(np, "nvmem-cells", index);
-	if (!cell_np)
-		return ERR_PTR(-EINVAL);
+	int rval, len;
 
 	nvmem_np = of_get_next_parent(cell_np);
 	if (!nvmem_np)
@@ -824,6 +816,32 @@ err_mem:
 
 	return ERR_PTR(rval);
 }
+EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
+
+/**
+ * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
+ *
+ * @dev node: Device tree node that uses the nvmem cell
+ * @id: nvmem cell name from nvmem-cell-names property.
+ *
+ * Return: Will be an ERR_PTR() on error or a valid pointer
+ * to a struct nvmem_cell.  The nvmem_cell will be freed by the
+ * nvmem_cell_put().
+ */
+struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
+					    const char *name)
+{
+	struct device_node *cell_np;
+	int index;
+
+	index = of_property_match_string(np, "nvmem-cell-names", name);
+
+	cell_np = of_parse_phandle(np, "nvmem-cells", index);
+	if (!cell_np)
+		return ERR_PTR(-EINVAL);
+
+	return of_nvmem_cell_get_direct(cell_np);
+}
 EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
 #endif
 
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 9bb77d3..bf879fc 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
 #endif /* CONFIG_NVMEM */
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
+struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
 struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
 				     const char *name);
 struct nvmem_device *of_nvmem_device_get(struct device_node *np,
-- 
2.9.0

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Sanchayan Maity]

From: Stefan Agner <stefan@agner.ch>

The existing NVMEM consumer API's do not allow to get a
NVMEM cell directly given a device tree node. This patch
adds a function to provide this functionality.

Assuming the nvmem cell id name is known, this can be used
as follows

struct device_node *cell_np;
struct nvmem_cell *foo_cell;

cell_np = of_find_node_by_name(parent, "foo");
foo_cell = of_nvmem_cell_get_direct(cell_np);

Parent node can also be the of_node of the main SoC device
node.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
 include/linux/nvmem-consumer.h |  1 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 965911d..470abee 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
 /**
- * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
+ * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
  *
- * @dev node: Device tree node that uses the nvmem cell
- * @id: nvmem cell name from nvmem-cell-names property.
+ * @dev node: Device tree node that uses nvmem cell
  *
  * Return: Will be an ERR_PTR() on error or a valid pointer
  * to a struct nvmem_cell.  The nvmem_cell will be freed by the
  * nvmem_cell_put().
  */
-struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
-					    const char *name)
+struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
 {
-	struct device_node *cell_np, *nvmem_np;
+	struct device_node *nvmem_np;
 	struct nvmem_cell *cell;
 	struct nvmem_device *nvmem;
 	const __be32 *addr;
-	int rval, len, index;
-
-	index = of_property_match_string(np, "nvmem-cell-names", name);
-
-	cell_np = of_parse_phandle(np, "nvmem-cells", index);
-	if (!cell_np)
-		return ERR_PTR(-EINVAL);
+	int rval, len;
 
 	nvmem_np = of_get_next_parent(cell_np);
 	if (!nvmem_np)
@@ -824,6 +816,32 @@ err_mem:
 
 	return ERR_PTR(rval);
 }
+EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
+
+/**
+ * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
+ *
+ * @dev node: Device tree node that uses the nvmem cell
+ * @id: nvmem cell name from nvmem-cell-names property.
+ *
+ * Return: Will be an ERR_PTR() on error or a valid pointer
+ * to a struct nvmem_cell.  The nvmem_cell will be freed by the
+ * nvmem_cell_put().
+ */
+struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
+					    const char *name)
+{
+	struct device_node *cell_np;
+	int index;
+
+	index = of_property_match_string(np, "nvmem-cell-names", name);
+
+	cell_np = of_parse_phandle(np, "nvmem-cells", index);
+	if (!cell_np)
+		return ERR_PTR(-EINVAL);
+
+	return of_nvmem_cell_get_direct(cell_np);
+}
 EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
 #endif
 
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 9bb77d3..bf879fc 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
 #endif /* CONFIG_NVMEM */
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
+struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
 struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
 				     const char *name);
 struct nvmem_device *of_nvmem_device_get(struct device_node *np,
-- 
2.9.0

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Sanchayan Maity]

From: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>

The existing NVMEM consumer API's do not allow to get a
NVMEM cell directly given a device tree node. This patch
adds a function to provide this functionality.

Assuming the nvmem cell id name is known, this can be used
as follows

struct device_node *cell_np;
struct nvmem_cell *foo_cell;

cell_np = of_find_node_by_name(parent, "foo");
foo_cell = of_nvmem_cell_get_direct(cell_np);

Parent node can also be the of_node of the main SoC device
node.

Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
 include/linux/nvmem-consumer.h |  1 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 965911d..470abee 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
 /**
- * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
+ * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
  *
- * @dev node: Device tree node that uses the nvmem cell
- * @id: nvmem cell name from nvmem-cell-names property.
+ * @dev node: Device tree node that uses nvmem cell
  *
  * Return: Will be an ERR_PTR() on error or a valid pointer
  * to a struct nvmem_cell.  The nvmem_cell will be freed by the
  * nvmem_cell_put().
  */
-struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
-					    const char *name)
+struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
 {
-	struct device_node *cell_np, *nvmem_np;
+	struct device_node *nvmem_np;
 	struct nvmem_cell *cell;
 	struct nvmem_device *nvmem;
 	const __be32 *addr;
-	int rval, len, index;
-
-	index = of_property_match_string(np, "nvmem-cell-names", name);
-
-	cell_np = of_parse_phandle(np, "nvmem-cells", index);
-	if (!cell_np)
-		return ERR_PTR(-EINVAL);
+	int rval, len;
 
 	nvmem_np = of_get_next_parent(cell_np);
 	if (!nvmem_np)
@@ -824,6 +816,32 @@ err_mem:
 
 	return ERR_PTR(rval);
 }
+EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
+
+/**
+ * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
+ *
+ * @dev node: Device tree node that uses the nvmem cell
+ * @id: nvmem cell name from nvmem-cell-names property.
+ *
+ * Return: Will be an ERR_PTR() on error or a valid pointer
+ * to a struct nvmem_cell.  The nvmem_cell will be freed by the
+ * nvmem_cell_put().
+ */
+struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
+					    const char *name)
+{
+	struct device_node *cell_np;
+	int index;
+
+	index = of_property_match_string(np, "nvmem-cell-names", name);
+
+	cell_np = of_parse_phandle(np, "nvmem-cells", index);
+	if (!cell_np)
+		return ERR_PTR(-EINVAL);
+
+	return of_nvmem_cell_get_direct(cell_np);
+}
 EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
 #endif
 
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 9bb77d3..bf879fc 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
 #endif /* CONFIG_NVMEM */
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
+struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
 struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
 				     const char *name);
 struct nvmem_device *of_nvmem_device_get(struct device_node *np,
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 07:39, Sanchayan Maity wrote:
> From: Stefan Agner <stefan@agner.ch>
>
> The existing NVMEM consumer API's do not allow to get a
> NVMEM cell directly given a device tree node. This patch
> adds a function to provide this functionality.
>
> Assuming the nvmem cell id name is known, this can be used
> as follows
>
> struct device_node *cell_np;
> struct nvmem_cell *foo_cell;
>
> cell_np = of_find_node_by_name(parent, "foo");
> foo_cell = of_nvmem_cell_get_direct(cell_np);

I don't see a real gain in adding this new api,
This will encourage people to use non-standard nvmem bindings.

why not just use standard nvmem bindings.. and use

of_nvmem_cell_get(np, "foo");

which should work in your case.

thanks,
srini
>
> Parent node can also be the of_node of the main SoC device
> node.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>   drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>   include/linux/nvmem-consumer.h |  1 +
>   2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 965911d..470abee 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>
>   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>   /**
> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>    *
> - * @dev node: Device tree node that uses the nvmem cell
> - * @id: nvmem cell name from nvmem-cell-names property.
> + * @dev node: Device tree node that uses nvmem cell
>    *
>    * Return: Will be an ERR_PTR() on error or a valid pointer
>    * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>    * nvmem_cell_put().
>    */
> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> -					    const char *name)
> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>   {
> -	struct device_node *cell_np, *nvmem_np;
> +	struct device_node *nvmem_np;
>   	struct nvmem_cell *cell;
>   	struct nvmem_device *nvmem;
>   	const __be32 *addr;
> -	int rval, len, index;
> -
> -	index = of_property_match_string(np, "nvmem-cell-names", name);
> -
> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> -	if (!cell_np)
> -		return ERR_PTR(-EINVAL);
> +	int rval, len;
>
>   	nvmem_np = of_get_next_parent(cell_np);
>   	if (!nvmem_np)
> @@ -824,6 +816,32 @@ err_mem:
>
>   	return ERR_PTR(rval);
>   }
> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> +
> +/**
> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + *
> + * @dev node: Device tree node that uses the nvmem cell
> + * @id: nvmem cell name from nvmem-cell-names property.
> + *
> + * Return: Will be an ERR_PTR() on error or a valid pointer
> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> + * nvmem_cell_put().
> + */
> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> +					    const char *name)
> +{
> +	struct device_node *cell_np;
> +	int index;
> +
> +	index = of_property_match_string(np, "nvmem-cell-names", name);
> +
> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> +	if (!cell_np)
> +		return ERR_PTR(-EINVAL);
> +
> +	return of_nvmem_cell_get_direct(cell_np);
> +}
>   EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>   #endif
>
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index 9bb77d3..bf879fc 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>   #endif /* CONFIG_NVMEM */
>
>   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>   struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>   				     const char *name);
>   struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 07:39, Sanchayan Maity wrote:
> From: Stefan Agner <stefan@agner.ch>
>
> The existing NVMEM consumer API's do not allow to get a
> NVMEM cell directly given a device tree node. This patch
> adds a function to provide this functionality.
>
> Assuming the nvmem cell id name is known, this can be used
> as follows
>
> struct device_node *cell_np;
> struct nvmem_cell *foo_cell;
>
> cell_np = of_find_node_by_name(parent, "foo");
> foo_cell = of_nvmem_cell_get_direct(cell_np);

I don't see a real gain in adding this new api,
This will encourage people to use non-standard nvmem bindings.

why not just use standard nvmem bindings.. and use

of_nvmem_cell_get(np, "foo");

which should work in your case.

thanks,
srini
>
> Parent node can also be the of_node of the main SoC device
> node.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>   drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>   include/linux/nvmem-consumer.h |  1 +
>   2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 965911d..470abee 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>
>   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>   /**
> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>    *
> - * @dev node: Device tree node that uses the nvmem cell
> - * @id: nvmem cell name from nvmem-cell-names property.
> + * @dev node: Device tree node that uses nvmem cell
>    *
>    * Return: Will be an ERR_PTR() on error or a valid pointer
>    * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>    * nvmem_cell_put().
>    */
> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> -					    const char *name)
> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>   {
> -	struct device_node *cell_np, *nvmem_np;
> +	struct device_node *nvmem_np;
>   	struct nvmem_cell *cell;
>   	struct nvmem_device *nvmem;
>   	const __be32 *addr;
> -	int rval, len, index;
> -
> -	index = of_property_match_string(np, "nvmem-cell-names", name);
> -
> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> -	if (!cell_np)
> -		return ERR_PTR(-EINVAL);
> +	int rval, len;
>
>   	nvmem_np = of_get_next_parent(cell_np);
>   	if (!nvmem_np)
> @@ -824,6 +816,32 @@ err_mem:
>
>   	return ERR_PTR(rval);
>   }
> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> +
> +/**
> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + *
> + * @dev node: Device tree node that uses the nvmem cell
> + * @id: nvmem cell name from nvmem-cell-names property.
> + *
> + * Return: Will be an ERR_PTR() on error or a valid pointer
> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> + * nvmem_cell_put().
> + */
> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> +					    const char *name)
> +{
> +	struct device_node *cell_np;
> +	int index;
> +
> +	index = of_property_match_string(np, "nvmem-cell-names", name);
> +
> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> +	if (!cell_np)
> +		return ERR_PTR(-EINVAL);
> +
> +	return of_nvmem_cell_get_direct(cell_np);
> +}
>   EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>   #endif
>
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index 9bb77d3..bf879fc 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>   #endif /* CONFIG_NVMEM */
>
>   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>   struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>   				     const char *name);
>   struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 07:39, Sanchayan Maity wrote:
> From: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
>
> The existing NVMEM consumer API's do not allow to get a
> NVMEM cell directly given a device tree node. This patch
> adds a function to provide this functionality.
>
> Assuming the nvmem cell id name is known, this can be used
> as follows
>
> struct device_node *cell_np;
> struct nvmem_cell *foo_cell;
>
> cell_np = of_find_node_by_name(parent, "foo");
> foo_cell = of_nvmem_cell_get_direct(cell_np);

I don't see a real gain in adding this new api,
This will encourage people to use non-standard nvmem bindings.

why not just use standard nvmem bindings.. and use

of_nvmem_cell_get(np, "foo");

which should work in your case.

thanks,
srini
>
> Parent node can also be the of_node of the main SoC device
> node.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>   include/linux/nvmem-consumer.h |  1 +
>   2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 965911d..470abee 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>
>   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>   /**
> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>    *
> - * @dev node: Device tree node that uses the nvmem cell
> - * @id: nvmem cell name from nvmem-cell-names property.
> + * @dev node: Device tree node that uses nvmem cell
>    *
>    * Return: Will be an ERR_PTR() on error or a valid pointer
>    * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>    * nvmem_cell_put().
>    */
> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> -					    const char *name)
> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>   {
> -	struct device_node *cell_np, *nvmem_np;
> +	struct device_node *nvmem_np;
>   	struct nvmem_cell *cell;
>   	struct nvmem_device *nvmem;
>   	const __be32 *addr;
> -	int rval, len, index;
> -
> -	index = of_property_match_string(np, "nvmem-cell-names", name);
> -
> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> -	if (!cell_np)
> -		return ERR_PTR(-EINVAL);
> +	int rval, len;
>
>   	nvmem_np = of_get_next_parent(cell_np);
>   	if (!nvmem_np)
> @@ -824,6 +816,32 @@ err_mem:
>
>   	return ERR_PTR(rval);
>   }
> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> +
> +/**
> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + *
> + * @dev node: Device tree node that uses the nvmem cell
> + * @id: nvmem cell name from nvmem-cell-names property.
> + *
> + * Return: Will be an ERR_PTR() on error or a valid pointer
> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> + * nvmem_cell_put().
> + */
> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> +					    const char *name)
> +{
> +	struct device_node *cell_np;
> +	int index;
> +
> +	index = of_property_match_string(np, "nvmem-cell-names", name);
> +
> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> +	if (!cell_np)
> +		return ERR_PTR(-EINVAL);
> +
> +	return of_nvmem_cell_get_direct(cell_np);
> +}
>   EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>   #endif
>
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index 9bb77d3..bf879fc 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>   #endif /* CONFIG_NVMEM */
>
>   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>   struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>   				     const char *name);
>   struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan]

Hello Srinivas,

On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
> 
> 
> On 07/07/16 07:39, Sanchayan Maity wrote:
> > From: Stefan Agner <stefan@agner.ch>
> > 
> > The existing NVMEM consumer API's do not allow to get a
> > NVMEM cell directly given a device tree node. This patch
> > adds a function to provide this functionality.
> > 
> > Assuming the nvmem cell id name is known, this can be used
> > as follows
> > 
> > struct device_node *cell_np;
> > struct nvmem_cell *foo_cell;
> > 
> > cell_np = of_find_node_by_name(parent, "foo");
> > foo_cell = of_nvmem_cell_get_direct(cell_np);
> 
> I don't see a real gain in adding this new api,
> This will encourage people to use non-standard nvmem bindings.
> 
> why not just use standard nvmem bindings.. and use
> 
> of_nvmem_cell_get(np, "foo");
> 
> which should work in your case.

It will not work in our case. I believe what you are referring to will
work if I were to pass the device node pointer which was a NVMEM consumer
containing the nvmem-cells property. In our case, we pass the device node
pointer pointing to /soc which is not a nvmem consumer. In this case it
will not have nvmem-cells property causing of_nvmem_cell_get to return
EINVAL when it calls of_parse_phandle with "nvmem-cells".

Our requirement is to be able to pass the soc node pointer and then
be able to get a nvmem cell by specifying it's name. So for our case
ocotp node has cfg0 and cfg1 which we want but we cannot use existing
nvmem consumer API since that requires having the nvmem consumer properties
in the node we are binding to viz. is a direct nvmem consumer.

Regards,
Sanchayan.

> 
> thanks,
> srini
> > 
> > Parent node can also be the of_node of the main SoC device
> > node.
> > 
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> >   drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> >   include/linux/nvmem-consumer.h |  1 +
> >   2 files changed, 32 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 965911d..470abee 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > 
> >   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> >   /**
> > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> >    *
> > - * @dev node: Device tree node that uses the nvmem cell
> > - * @id: nvmem cell name from nvmem-cell-names property.
> > + * @dev node: Device tree node that uses nvmem cell
> >    *
> >    * Return: Will be an ERR_PTR() on error or a valid pointer
> >    * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> >    * nvmem_cell_put().
> >    */
> > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > -					    const char *name)
> > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> >   {
> > -	struct device_node *cell_np, *nvmem_np;
> > +	struct device_node *nvmem_np;
> >   	struct nvmem_cell *cell;
> >   	struct nvmem_device *nvmem;
> >   	const __be32 *addr;
> > -	int rval, len, index;
> > -
> > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > -
> > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > -	if (!cell_np)
> > -		return ERR_PTR(-EINVAL);
> > +	int rval, len;
> > 
> >   	nvmem_np = of_get_next_parent(cell_np);
> >   	if (!nvmem_np)
> > @@ -824,6 +816,32 @@ err_mem:
> > 
> >   	return ERR_PTR(rval);
> >   }
> > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > +
> > +/**
> > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > + *
> > + * @dev node: Device tree node that uses the nvmem cell
> > + * @id: nvmem cell name from nvmem-cell-names property.
> > + *
> > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > + * nvmem_cell_put().
> > + */
> > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > +					    const char *name)
> > +{
> > +	struct device_node *cell_np;
> > +	int index;
> > +
> > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > +
> > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > +	if (!cell_np)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	return of_nvmem_cell_get_direct(cell_np);
> > +}
> >   EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> >   #endif
> > 
> > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > index 9bb77d3..bf879fc 100644
> > --- a/include/linux/nvmem-consumer.h
> > +++ b/include/linux/nvmem-consumer.h
> > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> >   #endif /* CONFIG_NVMEM */
> > 
> >   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> >   struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> >   				     const char *name);
> >   struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > 

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan at gmail.com]

Hello Srinivas,

On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
> 
> 
> On 07/07/16 07:39, Sanchayan Maity wrote:
> > From: Stefan Agner <stefan@agner.ch>
> > 
> > The existing NVMEM consumer API's do not allow to get a
> > NVMEM cell directly given a device tree node. This patch
> > adds a function to provide this functionality.
> > 
> > Assuming the nvmem cell id name is known, this can be used
> > as follows
> > 
> > struct device_node *cell_np;
> > struct nvmem_cell *foo_cell;
> > 
> > cell_np = of_find_node_by_name(parent, "foo");
> > foo_cell = of_nvmem_cell_get_direct(cell_np);
> 
> I don't see a real gain in adding this new api,
> This will encourage people to use non-standard nvmem bindings.
> 
> why not just use standard nvmem bindings.. and use
> 
> of_nvmem_cell_get(np, "foo");
> 
> which should work in your case.

It will not work in our case. I believe what you are referring to will
work if I were to pass the device node pointer which was a NVMEM consumer
containing the nvmem-cells property. In our case, we pass the device node
pointer pointing to /soc which is not a nvmem consumer. In this case it
will not have nvmem-cells property causing of_nvmem_cell_get to return
EINVAL when it calls of_parse_phandle with "nvmem-cells".

Our requirement is to be able to pass the soc node pointer and then
be able to get a nvmem cell by specifying it's name. So for our case
ocotp node has cfg0 and cfg1 which we want but we cannot use existing
nvmem consumer API since that requires having the nvmem consumer properties
in the node we are binding to viz. is a direct nvmem consumer.

Regards,
Sanchayan.

> 
> thanks,
> srini
> > 
> > Parent node can also be the of_node of the main SoC device
> > node.
> > 
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> >   drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> >   include/linux/nvmem-consumer.h |  1 +
> >   2 files changed, 32 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 965911d..470abee 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > 
> >   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> >   /**
> > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> >    *
> > - * @dev node: Device tree node that uses the nvmem cell
> > - * @id: nvmem cell name from nvmem-cell-names property.
> > + * @dev node: Device tree node that uses nvmem cell
> >    *
> >    * Return: Will be an ERR_PTR() on error or a valid pointer
> >    * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> >    * nvmem_cell_put().
> >    */
> > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > -					    const char *name)
> > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> >   {
> > -	struct device_node *cell_np, *nvmem_np;
> > +	struct device_node *nvmem_np;
> >   	struct nvmem_cell *cell;
> >   	struct nvmem_device *nvmem;
> >   	const __be32 *addr;
> > -	int rval, len, index;
> > -
> > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > -
> > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > -	if (!cell_np)
> > -		return ERR_PTR(-EINVAL);
> > +	int rval, len;
> > 
> >   	nvmem_np = of_get_next_parent(cell_np);
> >   	if (!nvmem_np)
> > @@ -824,6 +816,32 @@ err_mem:
> > 
> >   	return ERR_PTR(rval);
> >   }
> > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > +
> > +/**
> > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > + *
> > + * @dev node: Device tree node that uses the nvmem cell
> > + * @id: nvmem cell name from nvmem-cell-names property.
> > + *
> > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > + * nvmem_cell_put().
> > + */
> > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > +					    const char *name)
> > +{
> > +	struct device_node *cell_np;
> > +	int index;
> > +
> > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > +
> > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > +	if (!cell_np)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	return of_nvmem_cell_get_direct(cell_np);
> > +}
> >   EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> >   #endif
> > 
> > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > index 9bb77d3..bf879fc 100644
> > --- a/include/linux/nvmem-consumer.h
> > +++ b/include/linux/nvmem-consumer.h
> > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> >   #endif /* CONFIG_NVMEM */
> > 
> >   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> >   struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> >   				     const char *name);
> >   struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > 

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w]

Hello Srinivas,

On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
> 
> 
> On 07/07/16 07:39, Sanchayan Maity wrote:
> > From: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
> > 
> > The existing NVMEM consumer API's do not allow to get a
> > NVMEM cell directly given a device tree node. This patch
> > adds a function to provide this functionality.
> > 
> > Assuming the nvmem cell id name is known, this can be used
> > as follows
> > 
> > struct device_node *cell_np;
> > struct nvmem_cell *foo_cell;
> > 
> > cell_np = of_find_node_by_name(parent, "foo");
> > foo_cell = of_nvmem_cell_get_direct(cell_np);
> 
> I don't see a real gain in adding this new api,
> This will encourage people to use non-standard nvmem bindings.
> 
> why not just use standard nvmem bindings.. and use
> 
> of_nvmem_cell_get(np, "foo");
> 
> which should work in your case.

It will not work in our case. I believe what you are referring to will
work if I were to pass the device node pointer which was a NVMEM consumer
containing the nvmem-cells property. In our case, we pass the device node
pointer pointing to /soc which is not a nvmem consumer. In this case it
will not have nvmem-cells property causing of_nvmem_cell_get to return
EINVAL when it calls of_parse_phandle with "nvmem-cells".

Our requirement is to be able to pass the soc node pointer and then
be able to get a nvmem cell by specifying it's name. So for our case
ocotp node has cfg0 and cfg1 which we want but we cannot use existing
nvmem consumer API since that requires having the nvmem consumer properties
in the node we are binding to viz. is a direct nvmem consumer.

Regards,
Sanchayan.

> 
> thanks,
> srini
> > 
> > Parent node can also be the of_node of the main SoC device
> > node.
> > 
> > Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >   drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> >   include/linux/nvmem-consumer.h |  1 +
> >   2 files changed, 32 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 965911d..470abee 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > 
> >   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> >   /**
> > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> >    *
> > - * @dev node: Device tree node that uses the nvmem cell
> > - * @id: nvmem cell name from nvmem-cell-names property.
> > + * @dev node: Device tree node that uses nvmem cell
> >    *
> >    * Return: Will be an ERR_PTR() on error or a valid pointer
> >    * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> >    * nvmem_cell_put().
> >    */
> > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > -					    const char *name)
> > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> >   {
> > -	struct device_node *cell_np, *nvmem_np;
> > +	struct device_node *nvmem_np;
> >   	struct nvmem_cell *cell;
> >   	struct nvmem_device *nvmem;
> >   	const __be32 *addr;
> > -	int rval, len, index;
> > -
> > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > -
> > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > -	if (!cell_np)
> > -		return ERR_PTR(-EINVAL);
> > +	int rval, len;
> > 
> >   	nvmem_np = of_get_next_parent(cell_np);
> >   	if (!nvmem_np)
> > @@ -824,6 +816,32 @@ err_mem:
> > 
> >   	return ERR_PTR(rval);
> >   }
> > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > +
> > +/**
> > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > + *
> > + * @dev node: Device tree node that uses the nvmem cell
> > + * @id: nvmem cell name from nvmem-cell-names property.
> > + *
> > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > + * nvmem_cell_put().
> > + */
> > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > +					    const char *name)
> > +{
> > +	struct device_node *cell_np;
> > +	int index;
> > +
> > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > +
> > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > +	if (!cell_np)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	return of_nvmem_cell_get_direct(cell_np);
> > +}
> >   EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> >   #endif
> > 
> > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > index 9bb77d3..bf879fc 100644
> > --- a/include/linux/nvmem-consumer.h
> > +++ b/include/linux/nvmem-consumer.h
> > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> >   #endif /* CONFIG_NVMEM */
> > 
> >   #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> >   struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> >   				     const char *name);
> >   struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 13:33, maitysanchayan@gmail.com wrote:
> Hello Srinivas,
>
> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>
>>
>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>> From: Stefan Agner <stefan@agner.ch>
>>>
>>> The existing NVMEM consumer API's do not allow to get a
>>> NVMEM cell directly given a device tree node. This patch
>>> adds a function to provide this functionality.
>>>
>>> Assuming the nvmem cell id name is known, this can be used
>>> as follows
>>>
>>> struct device_node *cell_np;
>>> struct nvmem_cell *foo_cell;
>>>
>>> cell_np = of_find_node_by_name(parent, "foo");
>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>
>> I don't see a real gain in adding this new api,
>> This will encourage people to use non-standard nvmem bindings.
>>
>> why not just use standard nvmem bindings.. and use
>>
>> of_nvmem_cell_get(np, "foo");
>>
>> which should work in your case.
>
> It will not work in our case. I believe what you are referring to will
> work if I were to pass the device node pointer which was a NVMEM consumer
> containing the nvmem-cells property. In our case, we pass the device node
> pointer pointing to /soc which is not a nvmem consumer. In this case it
> will not have nvmem-cells property causing of_nvmem_cell_get to return
> EINVAL when it calls of_parse_phandle with "nvmem-cells".

I could not see any bindings/ dt patches or dt examples for this this 
series.. so Am guessing your node would look like:

soc {
	cfg0: cfg0 {
		...
	};
	cfg1: cfg1 {
		...
	};
};

If this is not how it looks, can you share the details.

What Am saying is that why not have:

soc {
	nvmem-cells = <&cfg0>, <&cfg1>;
	nvmem-cell-names = "cfg0", "cfg1";

	cfg0: cfg0 {
		...
	};

	cfg1: cfg1 {
		...
	};
};

>
> Our requirement is to be able to pass the soc node pointer and then
> be able to get a nvmem cell by specifying it's name. So for our case

Why?

> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> nvmem consumer API since that requires having the nvmem consumer properties
> in the node we are binding to viz. is a direct nvmem consumer.
>
> Regards,
> Sanchayan.
>
>>
>> thanks,
>> srini
>>>
>>> Parent node can also be the of_node of the main SoC device
>>> node.
>>>
>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>> ---
>>>    drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>    include/linux/nvmem-consumer.h |  1 +
>>>    2 files changed, 32 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>> index 965911d..470abee 100644
>>> --- a/drivers/nvmem/core.c
>>> +++ b/drivers/nvmem/core.c
>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>
>>>    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>    /**
>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>     *
>>> - * @dev node: Device tree node that uses the nvmem cell
>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>> + * @dev node: Device tree node that uses nvmem cell
>>>     *
>>>     * Return: Will be an ERR_PTR() on error or a valid pointer
>>>     * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>     * nvmem_cell_put().
>>>     */
>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>> -					    const char *name)
>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>    {
>>> -	struct device_node *cell_np, *nvmem_np;
>>> +	struct device_node *nvmem_np;
>>>    	struct nvmem_cell *cell;
>>>    	struct nvmem_device *nvmem;
>>>    	const __be32 *addr;
>>> -	int rval, len, index;
>>> -
>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>> -
>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>> -	if (!cell_np)
>>> -		return ERR_PTR(-EINVAL);
>>> +	int rval, len;
>>>
>>>    	nvmem_np = of_get_next_parent(cell_np);
>>>    	if (!nvmem_np)
>>> @@ -824,6 +816,32 @@ err_mem:
>>>
>>>    	return ERR_PTR(rval);
>>>    }
>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>> +
>>> +/**
>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>> + *
>>> + * @dev node: Device tree node that uses the nvmem cell
>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>> + *
>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>> + * nvmem_cell_put().
>>> + */
>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>> +					    const char *name)
>>> +{
>>> +	struct device_node *cell_np;
>>> +	int index;
>>> +
>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>> +
>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>> +	if (!cell_np)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	return of_nvmem_cell_get_direct(cell_np);
>>> +}
>>>    EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>    #endif
>>>
>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>> index 9bb77d3..bf879fc 100644
>>> --- a/include/linux/nvmem-consumer.h
>>> +++ b/include/linux/nvmem-consumer.h
>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>    #endif /* CONFIG_NVMEM */
>>>
>>>    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>    struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>    				     const char *name);
>>>    struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 13:33, maitysanchayan at gmail.com wrote:
> Hello Srinivas,
>
> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>
>>
>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>> From: Stefan Agner <stefan@agner.ch>
>>>
>>> The existing NVMEM consumer API's do not allow to get a
>>> NVMEM cell directly given a device tree node. This patch
>>> adds a function to provide this functionality.
>>>
>>> Assuming the nvmem cell id name is known, this can be used
>>> as follows
>>>
>>> struct device_node *cell_np;
>>> struct nvmem_cell *foo_cell;
>>>
>>> cell_np = of_find_node_by_name(parent, "foo");
>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>
>> I don't see a real gain in adding this new api,
>> This will encourage people to use non-standard nvmem bindings.
>>
>> why not just use standard nvmem bindings.. and use
>>
>> of_nvmem_cell_get(np, "foo");
>>
>> which should work in your case.
>
> It will not work in our case. I believe what you are referring to will
> work if I were to pass the device node pointer which was a NVMEM consumer
> containing the nvmem-cells property. In our case, we pass the device node
> pointer pointing to /soc which is not a nvmem consumer. In this case it
> will not have nvmem-cells property causing of_nvmem_cell_get to return
> EINVAL when it calls of_parse_phandle with "nvmem-cells".

I could not see any bindings/ dt patches or dt examples for this this 
series.. so Am guessing your node would look like:

soc {
	cfg0: cfg0 {
		...
	};
	cfg1: cfg1 {
		...
	};
};

If this is not how it looks, can you share the details.

What Am saying is that why not have:

soc {
	nvmem-cells = <&cfg0>, <&cfg1>;
	nvmem-cell-names = "cfg0", "cfg1";

	cfg0: cfg0 {
		...
	};

	cfg1: cfg1 {
		...
	};
};

>
> Our requirement is to be able to pass the soc node pointer and then
> be able to get a nvmem cell by specifying it's name. So for our case

Why?

> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> nvmem consumer API since that requires having the nvmem consumer properties
> in the node we are binding to viz. is a direct nvmem consumer.
>
> Regards,
> Sanchayan.
>
>>
>> thanks,
>> srini
>>>
>>> Parent node can also be the of_node of the main SoC device
>>> node.
>>>
>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>> ---
>>>    drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>    include/linux/nvmem-consumer.h |  1 +
>>>    2 files changed, 32 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>> index 965911d..470abee 100644
>>> --- a/drivers/nvmem/core.c
>>> +++ b/drivers/nvmem/core.c
>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>
>>>    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>    /**
>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>     *
>>> - * @dev node: Device tree node that uses the nvmem cell
>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>> + * @dev node: Device tree node that uses nvmem cell
>>>     *
>>>     * Return: Will be an ERR_PTR() on error or a valid pointer
>>>     * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>     * nvmem_cell_put().
>>>     */
>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>> -					    const char *name)
>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>    {
>>> -	struct device_node *cell_np, *nvmem_np;
>>> +	struct device_node *nvmem_np;
>>>    	struct nvmem_cell *cell;
>>>    	struct nvmem_device *nvmem;
>>>    	const __be32 *addr;
>>> -	int rval, len, index;
>>> -
>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>> -
>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>> -	if (!cell_np)
>>> -		return ERR_PTR(-EINVAL);
>>> +	int rval, len;
>>>
>>>    	nvmem_np = of_get_next_parent(cell_np);
>>>    	if (!nvmem_np)
>>> @@ -824,6 +816,32 @@ err_mem:
>>>
>>>    	return ERR_PTR(rval);
>>>    }
>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>> +
>>> +/**
>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>> + *
>>> + * @dev node: Device tree node that uses the nvmem cell
>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>> + *
>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>> + * nvmem_cell_put().
>>> + */
>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>> +					    const char *name)
>>> +{
>>> +	struct device_node *cell_np;
>>> +	int index;
>>> +
>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>> +
>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>> +	if (!cell_np)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	return of_nvmem_cell_get_direct(cell_np);
>>> +}
>>>    EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>    #endif
>>>
>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>> index 9bb77d3..bf879fc 100644
>>> --- a/include/linux/nvmem-consumer.h
>>> +++ b/include/linux/nvmem-consumer.h
>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>    #endif /* CONFIG_NVMEM */
>>>
>>>    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>    struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>    				     const char *name);
>>>    struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan]

Hello Srinivas,

On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
> 
> 
> On 07/07/16 13:33, maitysanchayan@gmail.com wrote:
> > Hello Srinivas,
> > 
> > On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
> > > 
> > > 
> > > On 07/07/16 07:39, Sanchayan Maity wrote:
> > > > From: Stefan Agner <stefan@agner.ch>
> > > > 
> > > > The existing NVMEM consumer API's do not allow to get a
> > > > NVMEM cell directly given a device tree node. This patch
> > > > adds a function to provide this functionality.
> > > > 
> > > > Assuming the nvmem cell id name is known, this can be used
> > > > as follows
> > > > 
> > > > struct device_node *cell_np;
> > > > struct nvmem_cell *foo_cell;
> > > > 
> > > > cell_np = of_find_node_by_name(parent, "foo");
> > > > foo_cell = of_nvmem_cell_get_direct(cell_np);
> > > 
> > > I don't see a real gain in adding this new api,
> > > This will encourage people to use non-standard nvmem bindings.
> > > 
> > > why not just use standard nvmem bindings.. and use
> > > 
> > > of_nvmem_cell_get(np, "foo");
> > > 
> > > which should work in your case.
> > 
> > It will not work in our case. I believe what you are referring to will
> > work if I were to pass the device node pointer which was a NVMEM consumer
> > containing the nvmem-cells property. In our case, we pass the device node
> > pointer pointing to /soc which is not a nvmem consumer. In this case it
> > will not have nvmem-cells property causing of_nvmem_cell_get to return
> > EINVAL when it calls of_parse_phandle with "nvmem-cells".
> 
> I could not see any bindings/ dt patches or dt examples for this this
> series.. so Am guessing your node would look like:
> 
> soc {
> 	cfg0: cfg0 {
> 		...
> 	};
> 	cfg1: cfg1 {
> 		...
> 	};
> };
> 
> If this is not how it looks, can you share the details.
> 
> What Am saying is that why not have:
> 
> soc {
> 	nvmem-cells = <&cfg0>, <&cfg1>;
> 	nvmem-cell-names = "cfg0", "cfg1";
> 
> 	cfg0: cfg0 {
> 		...
> 	};
> 
> 	cfg1: cfg1 {
> 		...
> 	};
> };
> 
> > 
> > Our requirement is to be able to pass the soc node pointer and then
> > be able to get a nvmem cell by specifying it's name. So for our case
> 
> Why?

Sorry for not providing the background directly. The patches before this
series used that approach. In the previous discussions it has been pointed
out that it is not acceptable to have additional device tree bindings for
providing data that the driver wants at the SoC node level or to have bindings
just for the SoC bus driver alone since we aren't really describing the
hardware.

For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71

Regards,
Sanchayan.


> 
> > ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> > nvmem consumer API since that requires having the nvmem consumer properties
> > in the node we are binding to viz. is a direct nvmem consumer.
> > 
> > Regards,
> > Sanchayan.
> > 
> > > 
> > > thanks,
> > > srini
> > > > 
> > > > Parent node can also be the of_node of the main SoC device
> > > > node.
> > > > 
> > > > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > > > ---
> > > >    drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> > > >    include/linux/nvmem-consumer.h |  1 +
> > > >    2 files changed, 32 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > > > index 965911d..470abee 100644
> > > > --- a/drivers/nvmem/core.c
> > > > +++ b/drivers/nvmem/core.c
> > > > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > > > 
> > > >    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > >    /**
> > > > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> > > >     *
> > > > - * @dev node: Device tree node that uses the nvmem cell
> > > > - * @id: nvmem cell name from nvmem-cell-names property.
> > > > + * @dev node: Device tree node that uses nvmem cell
> > > >     *
> > > >     * Return: Will be an ERR_PTR() on error or a valid pointer
> > > >     * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > >     * nvmem_cell_put().
> > > >     */
> > > > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > -					    const char *name)
> > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> > > >    {
> > > > -	struct device_node *cell_np, *nvmem_np;
> > > > +	struct device_node *nvmem_np;
> > > >    	struct nvmem_cell *cell;
> > > >    	struct nvmem_device *nvmem;
> > > >    	const __be32 *addr;
> > > > -	int rval, len, index;
> > > > -
> > > > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > -
> > > > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > -	if (!cell_np)
> > > > -		return ERR_PTR(-EINVAL);
> > > > +	int rval, len;
> > > > 
> > > >    	nvmem_np = of_get_next_parent(cell_np);
> > > >    	if (!nvmem_np)
> > > > @@ -824,6 +816,32 @@ err_mem:
> > > > 
> > > >    	return ERR_PTR(rval);
> > > >    }
> > > > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > > > +
> > > > +/**
> > > > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > + *
> > > > + * @dev node: Device tree node that uses the nvmem cell
> > > > + * @id: nvmem cell name from nvmem-cell-names property.
> > > > + *
> > > > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > + * nvmem_cell_put().
> > > > + */
> > > > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > +					    const char *name)
> > > > +{
> > > > +	struct device_node *cell_np;
> > > > +	int index;
> > > > +
> > > > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > +
> > > > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > +	if (!cell_np)
> > > > +		return ERR_PTR(-EINVAL);
> > > > +
> > > > +	return of_nvmem_cell_get_direct(cell_np);
> > > > +}
> > > >    EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> > > >    #endif
> > > > 
> > > > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > > > index 9bb77d3..bf879fc 100644
> > > > --- a/include/linux/nvmem-consumer.h
> > > > +++ b/include/linux/nvmem-consumer.h
> > > > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> > > >    #endif /* CONFIG_NVMEM */
> > > > 
> > > >    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> > > >    struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > >    				     const char *name);
> > > >    struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > > > 

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan at gmail.com]

Hello Srinivas,

On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
> 
> 
> On 07/07/16 13:33, maitysanchayan at gmail.com wrote:
> > Hello Srinivas,
> > 
> > On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
> > > 
> > > 
> > > On 07/07/16 07:39, Sanchayan Maity wrote:
> > > > From: Stefan Agner <stefan@agner.ch>
> > > > 
> > > > The existing NVMEM consumer API's do not allow to get a
> > > > NVMEM cell directly given a device tree node. This patch
> > > > adds a function to provide this functionality.
> > > > 
> > > > Assuming the nvmem cell id name is known, this can be used
> > > > as follows
> > > > 
> > > > struct device_node *cell_np;
> > > > struct nvmem_cell *foo_cell;
> > > > 
> > > > cell_np = of_find_node_by_name(parent, "foo");
> > > > foo_cell = of_nvmem_cell_get_direct(cell_np);
> > > 
> > > I don't see a real gain in adding this new api,
> > > This will encourage people to use non-standard nvmem bindings.
> > > 
> > > why not just use standard nvmem bindings.. and use
> > > 
> > > of_nvmem_cell_get(np, "foo");
> > > 
> > > which should work in your case.
> > 
> > It will not work in our case. I believe what you are referring to will
> > work if I were to pass the device node pointer which was a NVMEM consumer
> > containing the nvmem-cells property. In our case, we pass the device node
> > pointer pointing to /soc which is not a nvmem consumer. In this case it
> > will not have nvmem-cells property causing of_nvmem_cell_get to return
> > EINVAL when it calls of_parse_phandle with "nvmem-cells".
> 
> I could not see any bindings/ dt patches or dt examples for this this
> series.. so Am guessing your node would look like:
> 
> soc {
> 	cfg0: cfg0 {
> 		...
> 	};
> 	cfg1: cfg1 {
> 		...
> 	};
> };
> 
> If this is not how it looks, can you share the details.
> 
> What Am saying is that why not have:
> 
> soc {
> 	nvmem-cells = <&cfg0>, <&cfg1>;
> 	nvmem-cell-names = "cfg0", "cfg1";
> 
> 	cfg0: cfg0 {
> 		...
> 	};
> 
> 	cfg1: cfg1 {
> 		...
> 	};
> };
> 
> > 
> > Our requirement is to be able to pass the soc node pointer and then
> > be able to get a nvmem cell by specifying it's name. So for our case
> 
> Why?

Sorry for not providing the background directly. The patches before this
series used that approach. In the previous discussions it has been pointed
out that it is not acceptable to have additional device tree bindings for
providing data that the driver wants at the SoC node level or to have bindings
just for the SoC bus driver alone since we aren't really describing the
hardware.

For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71

Regards,
Sanchayan.


> 
> > ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> > nvmem consumer API since that requires having the nvmem consumer properties
> > in the node we are binding to viz. is a direct nvmem consumer.
> > 
> > Regards,
> > Sanchayan.
> > 
> > > 
> > > thanks,
> > > srini
> > > > 
> > > > Parent node can also be the of_node of the main SoC device
> > > > node.
> > > > 
> > > > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > > > ---
> > > >    drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> > > >    include/linux/nvmem-consumer.h |  1 +
> > > >    2 files changed, 32 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > > > index 965911d..470abee 100644
> > > > --- a/drivers/nvmem/core.c
> > > > +++ b/drivers/nvmem/core.c
> > > > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > > > 
> > > >    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > >    /**
> > > > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> > > >     *
> > > > - * @dev node: Device tree node that uses the nvmem cell
> > > > - * @id: nvmem cell name from nvmem-cell-names property.
> > > > + * @dev node: Device tree node that uses nvmem cell
> > > >     *
> > > >     * Return: Will be an ERR_PTR() on error or a valid pointer
> > > >     * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > >     * nvmem_cell_put().
> > > >     */
> > > > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > -					    const char *name)
> > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> > > >    {
> > > > -	struct device_node *cell_np, *nvmem_np;
> > > > +	struct device_node *nvmem_np;
> > > >    	struct nvmem_cell *cell;
> > > >    	struct nvmem_device *nvmem;
> > > >    	const __be32 *addr;
> > > > -	int rval, len, index;
> > > > -
> > > > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > -
> > > > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > -	if (!cell_np)
> > > > -		return ERR_PTR(-EINVAL);
> > > > +	int rval, len;
> > > > 
> > > >    	nvmem_np = of_get_next_parent(cell_np);
> > > >    	if (!nvmem_np)
> > > > @@ -824,6 +816,32 @@ err_mem:
> > > > 
> > > >    	return ERR_PTR(rval);
> > > >    }
> > > > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > > > +
> > > > +/**
> > > > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > + *
> > > > + * @dev node: Device tree node that uses the nvmem cell
> > > > + * @id: nvmem cell name from nvmem-cell-names property.
> > > > + *
> > > > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > + * nvmem_cell_put().
> > > > + */
> > > > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > +					    const char *name)
> > > > +{
> > > > +	struct device_node *cell_np;
> > > > +	int index;
> > > > +
> > > > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > +
> > > > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > +	if (!cell_np)
> > > > +		return ERR_PTR(-EINVAL);
> > > > +
> > > > +	return of_nvmem_cell_get_direct(cell_np);
> > > > +}
> > > >    EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> > > >    #endif
> > > > 
> > > > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > > > index 9bb77d3..bf879fc 100644
> > > > --- a/include/linux/nvmem-consumer.h
> > > > +++ b/include/linux/nvmem-consumer.h
> > > > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> > > >    #endif /* CONFIG_NVMEM */
> > > > 
> > > >    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> > > >    struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > >    				     const char *name);
> > > >    struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > > > 

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w]

Hello Srinivas,

On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
> 
> 
> On 07/07/16 13:33, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> > Hello Srinivas,
> > 
> > On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
> > > 
> > > 
> > > On 07/07/16 07:39, Sanchayan Maity wrote:
> > > > From: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
> > > > 
> > > > The existing NVMEM consumer API's do not allow to get a
> > > > NVMEM cell directly given a device tree node. This patch
> > > > adds a function to provide this functionality.
> > > > 
> > > > Assuming the nvmem cell id name is known, this can be used
> > > > as follows
> > > > 
> > > > struct device_node *cell_np;
> > > > struct nvmem_cell *foo_cell;
> > > > 
> > > > cell_np = of_find_node_by_name(parent, "foo");
> > > > foo_cell = of_nvmem_cell_get_direct(cell_np);
> > > 
> > > I don't see a real gain in adding this new api,
> > > This will encourage people to use non-standard nvmem bindings.
> > > 
> > > why not just use standard nvmem bindings.. and use
> > > 
> > > of_nvmem_cell_get(np, "foo");
> > > 
> > > which should work in your case.
> > 
> > It will not work in our case. I believe what you are referring to will
> > work if I were to pass the device node pointer which was a NVMEM consumer
> > containing the nvmem-cells property. In our case, we pass the device node
> > pointer pointing to /soc which is not a nvmem consumer. In this case it
> > will not have nvmem-cells property causing of_nvmem_cell_get to return
> > EINVAL when it calls of_parse_phandle with "nvmem-cells".
> 
> I could not see any bindings/ dt patches or dt examples for this this
> series.. so Am guessing your node would look like:
> 
> soc {
> 	cfg0: cfg0 {
> 		...
> 	};
> 	cfg1: cfg1 {
> 		...
> 	};
> };
> 
> If this is not how it looks, can you share the details.
> 
> What Am saying is that why not have:
> 
> soc {
> 	nvmem-cells = <&cfg0>, <&cfg1>;
> 	nvmem-cell-names = "cfg0", "cfg1";
> 
> 	cfg0: cfg0 {
> 		...
> 	};
> 
> 	cfg1: cfg1 {
> 		...
> 	};
> };
> 
> > 
> > Our requirement is to be able to pass the soc node pointer and then
> > be able to get a nvmem cell by specifying it's name. So for our case
> 
> Why?

Sorry for not providing the background directly. The patches before this
series used that approach. In the previous discussions it has been pointed
out that it is not acceptable to have additional device tree bindings for
providing data that the driver wants at the SoC node level or to have bindings
just for the SoC bus driver alone since we aren't really describing the
hardware.

For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71

Regards,
Sanchayan.


> 
> > ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> > nvmem consumer API since that requires having the nvmem consumer properties
> > in the node we are binding to viz. is a direct nvmem consumer.
> > 
> > Regards,
> > Sanchayan.
> > 
> > > 
> > > thanks,
> > > srini
> > > > 
> > > > Parent node can also be the of_node of the main SoC device
> > > > node.
> > > > 
> > > > Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > >    drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> > > >    include/linux/nvmem-consumer.h |  1 +
> > > >    2 files changed, 32 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > > > index 965911d..470abee 100644
> > > > --- a/drivers/nvmem/core.c
> > > > +++ b/drivers/nvmem/core.c
> > > > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > > > 
> > > >    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > >    /**
> > > > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> > > >     *
> > > > - * @dev node: Device tree node that uses the nvmem cell
> > > > - * @id: nvmem cell name from nvmem-cell-names property.
> > > > + * @dev node: Device tree node that uses nvmem cell
> > > >     *
> > > >     * Return: Will be an ERR_PTR() on error or a valid pointer
> > > >     * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > >     * nvmem_cell_put().
> > > >     */
> > > > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > -					    const char *name)
> > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> > > >    {
> > > > -	struct device_node *cell_np, *nvmem_np;
> > > > +	struct device_node *nvmem_np;
> > > >    	struct nvmem_cell *cell;
> > > >    	struct nvmem_device *nvmem;
> > > >    	const __be32 *addr;
> > > > -	int rval, len, index;
> > > > -
> > > > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > -
> > > > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > -	if (!cell_np)
> > > > -		return ERR_PTR(-EINVAL);
> > > > +	int rval, len;
> > > > 
> > > >    	nvmem_np = of_get_next_parent(cell_np);
> > > >    	if (!nvmem_np)
> > > > @@ -824,6 +816,32 @@ err_mem:
> > > > 
> > > >    	return ERR_PTR(rval);
> > > >    }
> > > > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > > > +
> > > > +/**
> > > > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > + *
> > > > + * @dev node: Device tree node that uses the nvmem cell
> > > > + * @id: nvmem cell name from nvmem-cell-names property.
> > > > + *
> > > > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > + * nvmem_cell_put().
> > > > + */
> > > > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > +					    const char *name)
> > > > +{
> > > > +	struct device_node *cell_np;
> > > > +	int index;
> > > > +
> > > > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > +
> > > > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > +	if (!cell_np)
> > > > +		return ERR_PTR(-EINVAL);
> > > > +
> > > > +	return of_nvmem_cell_get_direct(cell_np);
> > > > +}
> > > >    EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> > > >    #endif
> > > > 
> > > > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > > > index 9bb77d3..bf879fc 100644
> > > > --- a/include/linux/nvmem-consumer.h
> > > > +++ b/include/linux/nvmem-consumer.h
> > > > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> > > >    #endif /* CONFIG_NVMEM */
> > > > 
> > > >    #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> > > >    struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > >    				     const char *name);
> > > >    struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > > > 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 14:48, maitysanchayan@gmail.com wrote:
> Hello Srinivas,
>
> On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
>>
>>
>> On 07/07/16 13:33, maitysanchayan@gmail.com wrote:
>>> Hello Srinivas,
>>>
>>> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>>>> From: Stefan Agner <stefan@agner.ch>
>>>>>
>>>>> The existing NVMEM consumer API's do not allow to get a
>>>>> NVMEM cell directly given a device tree node. This patch
>>>>> adds a function to provide this functionality.
>>>>>
>>>>> Assuming the nvmem cell id name is known, this can be used
>>>>> as follows
>>>>>
>>>>> struct device_node *cell_np;
>>>>> struct nvmem_cell *foo_cell;
>>>>>
>>>>> cell_np = of_find_node_by_name(parent, "foo");
>>>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>>>
>>>> I don't see a real gain in adding this new api,
>>>> This will encourage people to use non-standard nvmem bindings.
>>>>
>>>> why not just use standard nvmem bindings.. and use
>>>>
>>>> of_nvmem_cell_get(np, "foo");
>>>>
>>>> which should work in your case.
>>>
>>> It will not work in our case. I believe what you are referring to will
>>> work if I were to pass the device node pointer which was a NVMEM consumer
>>> containing the nvmem-cells property. In our case, we pass the device node
>>> pointer pointing to /soc which is not a nvmem consumer. In this case it
>>> will not have nvmem-cells property causing of_nvmem_cell_get to return
>>> EINVAL when it calls of_parse_phandle with "nvmem-cells".
>>
>> I could not see any bindings/ dt patches or dt examples for this this
>> series.. so Am guessing your node would look like:
>>
>> soc {
>> 	cfg0: cfg0 {
>> 		...
>> 	};
>> 	cfg1: cfg1 {
>> 		...
>> 	};
>> };
>>
>> If this is not how it looks, can you share the details.
>>
>> What Am saying is that why not have:
>>
>> soc {
>> 	nvmem-cells = <&cfg0>, <&cfg1>;
>> 	nvmem-cell-names = "cfg0", "cfg1";
>>
>> 	cfg0: cfg0 {
>> 		...
>> 	};
>>
>> 	cfg1: cfg1 {
>> 		...
>> 	};
>> };
>>
>>>
>>> Our requirement is to be able to pass the soc node pointer and then
>>> be able to get a nvmem cell by specifying it's name. So for our case
>>
>> Why?
>
> Sorry for not providing the background directly. The patches before this
> series used that approach. In the previous discussions it has been pointed
> out that it is not acceptable to have additional device tree bindings for
> providing data that the driver wants at the SoC node level or to have bindings
> just for the SoC bus driver alone since we aren't really describing the
> hardware.
>
SOC driver seems to search for an arbitrary node by its name, which is 
not a binding and can break anytime in cases If the scope of nvmem 
provider is out of soc node or if the nvmem cells are not named as 
expected. That looks very fragile.

If the soc node is actual consumer of nvmem cells, I see no reason why 
we should not use proper nvmem bindings?

Given the fact that the patch is potentially bypassing the nvmem 
bindings, am not happy to take it!

thanks,
srini

> For the discussion,
> https://lkml.org/lkml/2016/5/23/573
> https://lkml.org/lkml/2016/5/2/71
>
> Regards,
> Sanchayan.
>
>
>>
>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>> nvmem consumer API since that requires having the nvmem consumer properties
>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>
>>> Regards,
>>> Sanchayan.
>>>
>>>>
>>>> thanks,
>>>> srini
>>>>>
>>>>> Parent node can also be the of_node of the main SoC device
>>>>> node.
>>>>>
>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>> ---
>>>>>     drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>     include/linux/nvmem-consumer.h |  1 +
>>>>>     2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>> index 965911d..470abee 100644
>>>>> --- a/drivers/nvmem/core.c
>>>>> +++ b/drivers/nvmem/core.c
>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>
>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>     /**
>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>      *
>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>      *
>>>>>      * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>      * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>      * nvmem_cell_put().
>>>>>      */
>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>> -					    const char *name)
>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>     {
>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>> +	struct device_node *nvmem_np;
>>>>>     	struct nvmem_cell *cell;
>>>>>     	struct nvmem_device *nvmem;
>>>>>     	const __be32 *addr;
>>>>> -	int rval, len, index;
>>>>> -
>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>> -
>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>> -	if (!cell_np)
>>>>> -		return ERR_PTR(-EINVAL);
>>>>> +	int rval, len;
>>>>>
>>>>>     	nvmem_np = of_get_next_parent(cell_np);
>>>>>     	if (!nvmem_np)
>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>
>>>>>     	return ERR_PTR(rval);
>>>>>     }
>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>> +
>>>>> +/**
>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>> + *
>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>> + *
>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>> + * nvmem_cell_put().
>>>>> + */
>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>> +					    const char *name)
>>>>> +{
>>>>> +	struct device_node *cell_np;
>>>>> +	int index;
>>>>> +
>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>> +
>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>> +	if (!cell_np)
>>>>> +		return ERR_PTR(-EINVAL);
>>>>> +
>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>> +}
>>>>>     EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>     #endif
>>>>>
>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>> index 9bb77d3..bf879fc 100644
>>>>> --- a/include/linux/nvmem-consumer.h
>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>     #endif /* CONFIG_NVMEM */
>>>>>
>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>     struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>     				     const char *name);
>>>>>     struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 14:48, maitysanchayan at gmail.com wrote:
> Hello Srinivas,
>
> On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
>>
>>
>> On 07/07/16 13:33, maitysanchayan at gmail.com wrote:
>>> Hello Srinivas,
>>>
>>> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>>>> From: Stefan Agner <stefan@agner.ch>
>>>>>
>>>>> The existing NVMEM consumer API's do not allow to get a
>>>>> NVMEM cell directly given a device tree node. This patch
>>>>> adds a function to provide this functionality.
>>>>>
>>>>> Assuming the nvmem cell id name is known, this can be used
>>>>> as follows
>>>>>
>>>>> struct device_node *cell_np;
>>>>> struct nvmem_cell *foo_cell;
>>>>>
>>>>> cell_np = of_find_node_by_name(parent, "foo");
>>>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>>>
>>>> I don't see a real gain in adding this new api,
>>>> This will encourage people to use non-standard nvmem bindings.
>>>>
>>>> why not just use standard nvmem bindings.. and use
>>>>
>>>> of_nvmem_cell_get(np, "foo");
>>>>
>>>> which should work in your case.
>>>
>>> It will not work in our case. I believe what you are referring to will
>>> work if I were to pass the device node pointer which was a NVMEM consumer
>>> containing the nvmem-cells property. In our case, we pass the device node
>>> pointer pointing to /soc which is not a nvmem consumer. In this case it
>>> will not have nvmem-cells property causing of_nvmem_cell_get to return
>>> EINVAL when it calls of_parse_phandle with "nvmem-cells".
>>
>> I could not see any bindings/ dt patches or dt examples for this this
>> series.. so Am guessing your node would look like:
>>
>> soc {
>> 	cfg0: cfg0 {
>> 		...
>> 	};
>> 	cfg1: cfg1 {
>> 		...
>> 	};
>> };
>>
>> If this is not how it looks, can you share the details.
>>
>> What Am saying is that why not have:
>>
>> soc {
>> 	nvmem-cells = <&cfg0>, <&cfg1>;
>> 	nvmem-cell-names = "cfg0", "cfg1";
>>
>> 	cfg0: cfg0 {
>> 		...
>> 	};
>>
>> 	cfg1: cfg1 {
>> 		...
>> 	};
>> };
>>
>>>
>>> Our requirement is to be able to pass the soc node pointer and then
>>> be able to get a nvmem cell by specifying it's name. So for our case
>>
>> Why?
>
> Sorry for not providing the background directly. The patches before this
> series used that approach. In the previous discussions it has been pointed
> out that it is not acceptable to have additional device tree bindings for
> providing data that the driver wants at the SoC node level or to have bindings
> just for the SoC bus driver alone since we aren't really describing the
> hardware.
>
SOC driver seems to search for an arbitrary node by its name, which is 
not a binding and can break anytime in cases If the scope of nvmem 
provider is out of soc node or if the nvmem cells are not named as 
expected. That looks very fragile.

If the soc node is actual consumer of nvmem cells, I see no reason why 
we should not use proper nvmem bindings?

Given the fact that the patch is potentially bypassing the nvmem 
bindings, am not happy to take it!

thanks,
srini

> For the discussion,
> https://lkml.org/lkml/2016/5/23/573
> https://lkml.org/lkml/2016/5/2/71
>
> Regards,
> Sanchayan.
>
>
>>
>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>> nvmem consumer API since that requires having the nvmem consumer properties
>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>
>>> Regards,
>>> Sanchayan.
>>>
>>>>
>>>> thanks,
>>>> srini
>>>>>
>>>>> Parent node can also be the of_node of the main SoC device
>>>>> node.
>>>>>
>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>> ---
>>>>>     drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>     include/linux/nvmem-consumer.h |  1 +
>>>>>     2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>> index 965911d..470abee 100644
>>>>> --- a/drivers/nvmem/core.c
>>>>> +++ b/drivers/nvmem/core.c
>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>
>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>     /**
>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>      *
>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>      *
>>>>>      * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>      * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>      * nvmem_cell_put().
>>>>>      */
>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>> -					    const char *name)
>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>     {
>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>> +	struct device_node *nvmem_np;
>>>>>     	struct nvmem_cell *cell;
>>>>>     	struct nvmem_device *nvmem;
>>>>>     	const __be32 *addr;
>>>>> -	int rval, len, index;
>>>>> -
>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>> -
>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>> -	if (!cell_np)
>>>>> -		return ERR_PTR(-EINVAL);
>>>>> +	int rval, len;
>>>>>
>>>>>     	nvmem_np = of_get_next_parent(cell_np);
>>>>>     	if (!nvmem_np)
>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>
>>>>>     	return ERR_PTR(rval);
>>>>>     }
>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>> +
>>>>> +/**
>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>> + *
>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>> + *
>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>> + * nvmem_cell_put().
>>>>> + */
>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>> +					    const char *name)
>>>>> +{
>>>>> +	struct device_node *cell_np;
>>>>> +	int index;
>>>>> +
>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>> +
>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>> +	if (!cell_np)
>>>>> +		return ERR_PTR(-EINVAL);
>>>>> +
>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>> +}
>>>>>     EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>     #endif
>>>>>
>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>> index 9bb77d3..bf879fc 100644
>>>>> --- a/include/linux/nvmem-consumer.h
>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>     #endif /* CONFIG_NVMEM */
>>>>>
>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>     struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>     				     const char *name);
>>>>>     struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 07/07/16 14:48, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> Hello Srinivas,
>
> On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
>>
>>
>> On 07/07/16 13:33, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>> Hello Srinivas,
>>>
>>> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>>>> From: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
>>>>>
>>>>> The existing NVMEM consumer API's do not allow to get a
>>>>> NVMEM cell directly given a device tree node. This patch
>>>>> adds a function to provide this functionality.
>>>>>
>>>>> Assuming the nvmem cell id name is known, this can be used
>>>>> as follows
>>>>>
>>>>> struct device_node *cell_np;
>>>>> struct nvmem_cell *foo_cell;
>>>>>
>>>>> cell_np = of_find_node_by_name(parent, "foo");
>>>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>>>
>>>> I don't see a real gain in adding this new api,
>>>> This will encourage people to use non-standard nvmem bindings.
>>>>
>>>> why not just use standard nvmem bindings.. and use
>>>>
>>>> of_nvmem_cell_get(np, "foo");
>>>>
>>>> which should work in your case.
>>>
>>> It will not work in our case. I believe what you are referring to will
>>> work if I were to pass the device node pointer which was a NVMEM consumer
>>> containing the nvmem-cells property. In our case, we pass the device node
>>> pointer pointing to /soc which is not a nvmem consumer. In this case it
>>> will not have nvmem-cells property causing of_nvmem_cell_get to return
>>> EINVAL when it calls of_parse_phandle with "nvmem-cells".
>>
>> I could not see any bindings/ dt patches or dt examples for this this
>> series.. so Am guessing your node would look like:
>>
>> soc {
>> 	cfg0: cfg0 {
>> 		...
>> 	};
>> 	cfg1: cfg1 {
>> 		...
>> 	};
>> };
>>
>> If this is not how it looks, can you share the details.
>>
>> What Am saying is that why not have:
>>
>> soc {
>> 	nvmem-cells = <&cfg0>, <&cfg1>;
>> 	nvmem-cell-names = "cfg0", "cfg1";
>>
>> 	cfg0: cfg0 {
>> 		...
>> 	};
>>
>> 	cfg1: cfg1 {
>> 		...
>> 	};
>> };
>>
>>>
>>> Our requirement is to be able to pass the soc node pointer and then
>>> be able to get a nvmem cell by specifying it's name. So for our case
>>
>> Why?
>
> Sorry for not providing the background directly. The patches before this
> series used that approach. In the previous discussions it has been pointed
> out that it is not acceptable to have additional device tree bindings for
> providing data that the driver wants at the SoC node level or to have bindings
> just for the SoC bus driver alone since we aren't really describing the
> hardware.
>
SOC driver seems to search for an arbitrary node by its name, which is 
not a binding and can break anytime in cases If the scope of nvmem 
provider is out of soc node or if the nvmem cells are not named as 
expected. That looks very fragile.

If the soc node is actual consumer of nvmem cells, I see no reason why 
we should not use proper nvmem bindings?

Given the fact that the patch is potentially bypassing the nvmem 
bindings, am not happy to take it!

thanks,
srini

> For the discussion,
> https://lkml.org/lkml/2016/5/23/573
> https://lkml.org/lkml/2016/5/2/71
>
> Regards,
> Sanchayan.
>
>
>>
>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>> nvmem consumer API since that requires having the nvmem consumer properties
>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>
>>> Regards,
>>> Sanchayan.
>>>
>>>>
>>>> thanks,
>>>> srini
>>>>>
>>>>> Parent node can also be the of_node of the main SoC device
>>>>> node.
>>>>>
>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>     drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>     include/linux/nvmem-consumer.h |  1 +
>>>>>     2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>> index 965911d..470abee 100644
>>>>> --- a/drivers/nvmem/core.c
>>>>> +++ b/drivers/nvmem/core.c
>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>
>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>     /**
>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>      *
>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>      *
>>>>>      * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>      * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>      * nvmem_cell_put().
>>>>>      */
>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>> -					    const char *name)
>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>     {
>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>> +	struct device_node *nvmem_np;
>>>>>     	struct nvmem_cell *cell;
>>>>>     	struct nvmem_device *nvmem;
>>>>>     	const __be32 *addr;
>>>>> -	int rval, len, index;
>>>>> -
>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>> -
>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>> -	if (!cell_np)
>>>>> -		return ERR_PTR(-EINVAL);
>>>>> +	int rval, len;
>>>>>
>>>>>     	nvmem_np = of_get_next_parent(cell_np);
>>>>>     	if (!nvmem_np)
>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>
>>>>>     	return ERR_PTR(rval);
>>>>>     }
>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>> +
>>>>> +/**
>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>> + *
>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>> + *
>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>> + * nvmem_cell_put().
>>>>> + */
>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>> +					    const char *name)
>>>>> +{
>>>>> +	struct device_node *cell_np;
>>>>> +	int index;
>>>>> +
>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>> +
>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>> +	if (!cell_np)
>>>>> +		return ERR_PTR(-EINVAL);
>>>>> +
>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>> +}
>>>>>     EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>     #endif
>>>>>
>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>> index 9bb77d3..bf879fc 100644
>>>>> --- a/include/linux/nvmem-consumer.h
>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>     #endif /* CONFIG_NVMEM */
>>>>>
>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>     struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>     				     const char *name);
>>>>>     struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Stefan Agner]

On 2016-07-08 08:41, Srinivas Kandagatla wrote:
> On 07/07/16 14:48, maitysanchayan@gmail.com wrote:
>> Hello Srinivas,
>>
>> On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 07/07/16 13:33, maitysanchayan@gmail.com wrote:
>>>> Hello Srinivas,
>>>>
>>>> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>>>>> From: Stefan Agner <stefan@agner.ch>
>>>>>>
>>>>>> The existing NVMEM consumer API's do not allow to get a
>>>>>> NVMEM cell directly given a device tree node. This patch
>>>>>> adds a function to provide this functionality.
>>>>>>
>>>>>> Assuming the nvmem cell id name is known, this can be used
>>>>>> as follows
>>>>>>
>>>>>> struct device_node *cell_np;
>>>>>> struct nvmem_cell *foo_cell;
>>>>>>
>>>>>> cell_np = of_find_node_by_name(parent, "foo");
>>>>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>>>>
>>>>> I don't see a real gain in adding this new api,
>>>>> This will encourage people to use non-standard nvmem bindings.
>>>>>
>>>>> why not just use standard nvmem bindings.. and use
>>>>>
>>>>> of_nvmem_cell_get(np, "foo");
>>>>>
>>>>> which should work in your case.
>>>>
>>>> It will not work in our case. I believe what you are referring to will
>>>> work if I were to pass the device node pointer which was a NVMEM consumer
>>>> containing the nvmem-cells property. In our case, we pass the device node
>>>> pointer pointing to /soc which is not a nvmem consumer. In this case it
>>>> will not have nvmem-cells property causing of_nvmem_cell_get to return
>>>> EINVAL when it calls of_parse_phandle with "nvmem-cells".
>>>
>>> I could not see any bindings/ dt patches or dt examples for this this
>>> series.. so Am guessing your node would look like:
>>>
>>> soc {
>>> 	cfg0: cfg0 {
>>> 		...
>>> 	};
>>> 	cfg1: cfg1 {
>>> 		...
>>> 	};
>>> };
>>>
>>> If this is not how it looks, can you share the details.
>>>
>>> What Am saying is that why not have:
>>>
>>> soc {
>>> 	nvmem-cells = <&cfg0>, <&cfg1>;
>>> 	nvmem-cell-names = "cfg0", "cfg1";
>>>
>>> 	cfg0: cfg0 {
>>> 		...
>>> 	};
>>>
>>> 	cfg1: cfg1 {
>>> 		...
>>> 	};
>>> };
>>>
>>>>
>>>> Our requirement is to be able to pass the soc node pointer and then
>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>
>>> Why?
>>
>> Sorry for not providing the background directly. The patches before this
>> series used that approach. In the previous discussions it has been pointed
>> out that it is not acceptable to have additional device tree bindings for
>> providing data that the driver wants at the SoC node level or to have bindings
>> just for the SoC bus driver alone since we aren't really describing the
>> hardware.
>>
> SOC driver seems to search for an arbitrary node by its name, which is
> not a binding and can break anytime in cases If the scope of nvmem
> provider is out of soc node or if the nvmem cells are not named as
> expected. That looks very fragile.

In that case, that just "won't happen" because the soc driver is a very
soc specific driver only used for this device tree. We it will always
bind to that high level soc node.

> 
> If the soc node is actual consumer of nvmem cells, I see no reason why
> we should not use proper nvmem bindings?

There is a reason: We don't describe the hardware with it...

The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
driver are just two register with a unique ID of the SoC. In whatever
driver throughout the system we use that ID (e.g. in a random generator
for initialization) we never describe an actual hardware relation... Its
just software and how we use that unique ID. The device tree is ment to
describe hardware. Hence the NVMEM consumer binding is not suited for
such NVMEM cells...

By describing the NVMEM cells location in device tree (producer API, the
NVMEM cells are in hardware at that location, so using the device tree
for that part is fine) and hard coding the NVMEM cell we need in the
driver code we don't violate the device tree matra "describe the
hardware"...

Looking-up the nodes direcly is what Rob suggested here:
https://lkml.org/lkml/2016/5/23/573

> 
> Given the fact that the patch is potentially bypassing the nvmem
> bindings, am not happy to take it!

If you can provide a solution acceptable by the device tree folks and
works without this patch, I am happy to do it...

Btw, I am not entirely happy with the API name, but did not had a better
idea... And we we should probably add a note that the device tree
consumer binding is the preferred way to do it.

--
Stefan


> 
> thanks,
> srini
> 
>> For the discussion,
>> https://lkml.org/lkml/2016/5/23/573
>> https://lkml.org/lkml/2016/5/2/71
>>
>> Regards,
>> Sanchayan.
>>
>>
>>>
>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>
>>>> Regards,
>>>> Sanchayan.
>>>>
>>>>>
>>>>> thanks,
>>>>> srini
>>>>>>
>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>> node.
>>>>>>
>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>> ---
>>>>>>     drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>     include/linux/nvmem-consumer.h |  1 +
>>>>>>     2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>> index 965911d..470abee 100644
>>>>>> --- a/drivers/nvmem/core.c
>>>>>> +++ b/drivers/nvmem/core.c
>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>
>>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>     /**
>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>      *
>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>      *
>>>>>>      * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>      * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>      * nvmem_cell_put().
>>>>>>      */
>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>> -					    const char *name)
>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>     {
>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>> +	struct device_node *nvmem_np;
>>>>>>     	struct nvmem_cell *cell;
>>>>>>     	struct nvmem_device *nvmem;
>>>>>>     	const __be32 *addr;
>>>>>> -	int rval, len, index;
>>>>>> -
>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>> -
>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>> -	if (!cell_np)
>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>> +	int rval, len;
>>>>>>
>>>>>>     	nvmem_np = of_get_next_parent(cell_np);
>>>>>>     	if (!nvmem_np)
>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>
>>>>>>     	return ERR_PTR(rval);
>>>>>>     }
>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>> +
>>>>>> +/**
>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>> + *
>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>> + *
>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>> + * nvmem_cell_put().
>>>>>> + */
>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>> +					    const char *name)
>>>>>> +{
>>>>>> +	struct device_node *cell_np;
>>>>>> +	int index;
>>>>>> +
>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>> +
>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>> +	if (!cell_np)
>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>> +
>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>> +}
>>>>>>     EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>     #endif
>>>>>>
>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>> index 9bb77d3..bf879fc 100644
>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>     #endif /* CONFIG_NVMEM */
>>>>>>
>>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>     struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>     				     const char *name);
>>>>>>     struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Stefan Agner]

On 2016-07-08 08:41, Srinivas Kandagatla wrote:
> On 07/07/16 14:48, maitysanchayan at gmail.com wrote:
>> Hello Srinivas,
>>
>> On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 07/07/16 13:33, maitysanchayan at gmail.com wrote:
>>>> Hello Srinivas,
>>>>
>>>> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>>>>> From: Stefan Agner <stefan@agner.ch>
>>>>>>
>>>>>> The existing NVMEM consumer API's do not allow to get a
>>>>>> NVMEM cell directly given a device tree node. This patch
>>>>>> adds a function to provide this functionality.
>>>>>>
>>>>>> Assuming the nvmem cell id name is known, this can be used
>>>>>> as follows
>>>>>>
>>>>>> struct device_node *cell_np;
>>>>>> struct nvmem_cell *foo_cell;
>>>>>>
>>>>>> cell_np = of_find_node_by_name(parent, "foo");
>>>>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>>>>
>>>>> I don't see a real gain in adding this new api,
>>>>> This will encourage people to use non-standard nvmem bindings.
>>>>>
>>>>> why not just use standard nvmem bindings.. and use
>>>>>
>>>>> of_nvmem_cell_get(np, "foo");
>>>>>
>>>>> which should work in your case.
>>>>
>>>> It will not work in our case. I believe what you are referring to will
>>>> work if I were to pass the device node pointer which was a NVMEM consumer
>>>> containing the nvmem-cells property. In our case, we pass the device node
>>>> pointer pointing to /soc which is not a nvmem consumer. In this case it
>>>> will not have nvmem-cells property causing of_nvmem_cell_get to return
>>>> EINVAL when it calls of_parse_phandle with "nvmem-cells".
>>>
>>> I could not see any bindings/ dt patches or dt examples for this this
>>> series.. so Am guessing your node would look like:
>>>
>>> soc {
>>> 	cfg0: cfg0 {
>>> 		...
>>> 	};
>>> 	cfg1: cfg1 {
>>> 		...
>>> 	};
>>> };
>>>
>>> If this is not how it looks, can you share the details.
>>>
>>> What Am saying is that why not have:
>>>
>>> soc {
>>> 	nvmem-cells = <&cfg0>, <&cfg1>;
>>> 	nvmem-cell-names = "cfg0", "cfg1";
>>>
>>> 	cfg0: cfg0 {
>>> 		...
>>> 	};
>>>
>>> 	cfg1: cfg1 {
>>> 		...
>>> 	};
>>> };
>>>
>>>>
>>>> Our requirement is to be able to pass the soc node pointer and then
>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>
>>> Why?
>>
>> Sorry for not providing the background directly. The patches before this
>> series used that approach. In the previous discussions it has been pointed
>> out that it is not acceptable to have additional device tree bindings for
>> providing data that the driver wants at the SoC node level or to have bindings
>> just for the SoC bus driver alone since we aren't really describing the
>> hardware.
>>
> SOC driver seems to search for an arbitrary node by its name, which is
> not a binding and can break anytime in cases If the scope of nvmem
> provider is out of soc node or if the nvmem cells are not named as
> expected. That looks very fragile.

In that case, that just "won't happen" because the soc driver is a very
soc specific driver only used for this device tree. We it will always
bind to that high level soc node.

> 
> If the soc node is actual consumer of nvmem cells, I see no reason why
> we should not use proper nvmem bindings?

There is a reason: We don't describe the hardware with it...

The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
driver are just two register with a unique ID of the SoC. In whatever
driver throughout the system we use that ID (e.g. in a random generator
for initialization) we never describe an actual hardware relation... Its
just software and how we use that unique ID. The device tree is ment to
describe hardware. Hence the NVMEM consumer binding is not suited for
such NVMEM cells...

By describing the NVMEM cells location in device tree (producer API, the
NVMEM cells are in hardware at that location, so using the device tree
for that part is fine) and hard coding the NVMEM cell we need in the
driver code we don't violate the device tree matra "describe the
hardware"...

Looking-up the nodes direcly is what Rob suggested here:
https://lkml.org/lkml/2016/5/23/573

> 
> Given the fact that the patch is potentially bypassing the nvmem
> bindings, am not happy to take it!

If you can provide a solution acceptable by the device tree folks and
works without this patch, I am happy to do it...

Btw, I am not entirely happy with the API name, but did not had a better
idea... And we we should probably add a note that the device tree
consumer binding is the preferred way to do it.

--
Stefan


> 
> thanks,
> srini
> 
>> For the discussion,
>> https://lkml.org/lkml/2016/5/23/573
>> https://lkml.org/lkml/2016/5/2/71
>>
>> Regards,
>> Sanchayan.
>>
>>
>>>
>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>
>>>> Regards,
>>>> Sanchayan.
>>>>
>>>>>
>>>>> thanks,
>>>>> srini
>>>>>>
>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>> node.
>>>>>>
>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>> ---
>>>>>>     drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>     include/linux/nvmem-consumer.h |  1 +
>>>>>>     2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>> index 965911d..470abee 100644
>>>>>> --- a/drivers/nvmem/core.c
>>>>>> +++ b/drivers/nvmem/core.c
>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>
>>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>     /**
>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>      *
>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>      *
>>>>>>      * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>      * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>      * nvmem_cell_put().
>>>>>>      */
>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>> -					    const char *name)
>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>     {
>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>> +	struct device_node *nvmem_np;
>>>>>>     	struct nvmem_cell *cell;
>>>>>>     	struct nvmem_device *nvmem;
>>>>>>     	const __be32 *addr;
>>>>>> -	int rval, len, index;
>>>>>> -
>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>> -
>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>> -	if (!cell_np)
>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>> +	int rval, len;
>>>>>>
>>>>>>     	nvmem_np = of_get_next_parent(cell_np);
>>>>>>     	if (!nvmem_np)
>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>
>>>>>>     	return ERR_PTR(rval);
>>>>>>     }
>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>> +
>>>>>> +/**
>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>> + *
>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>> + *
>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>> + * nvmem_cell_put().
>>>>>> + */
>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>> +					    const char *name)
>>>>>> +{
>>>>>> +	struct device_node *cell_np;
>>>>>> +	int index;
>>>>>> +
>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>> +
>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>> +	if (!cell_np)
>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>> +
>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>> +}
>>>>>>     EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>     #endif
>>>>>>
>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>> index 9bb77d3..bf879fc 100644
>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>     #endif /* CONFIG_NVMEM */
>>>>>>
>>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>     struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>     				     const char *name);
>>>>>>     struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Stefan Agner]

On 2016-07-08 08:41, Srinivas Kandagatla wrote:
> On 07/07/16 14:48, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> Hello Srinivas,
>>
>> On 16-07-07 14:16:36, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 07/07/16 13:33, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>> Hello Srinivas,
>>>>
>>>> On 16-07-07 10:18:49, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 07/07/16 07:39, Sanchayan Maity wrote:
>>>>>> From: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
>>>>>>
>>>>>> The existing NVMEM consumer API's do not allow to get a
>>>>>> NVMEM cell directly given a device tree node. This patch
>>>>>> adds a function to provide this functionality.
>>>>>>
>>>>>> Assuming the nvmem cell id name is known, this can be used
>>>>>> as follows
>>>>>>
>>>>>> struct device_node *cell_np;
>>>>>> struct nvmem_cell *foo_cell;
>>>>>>
>>>>>> cell_np = of_find_node_by_name(parent, "foo");
>>>>>> foo_cell = of_nvmem_cell_get_direct(cell_np);
>>>>>
>>>>> I don't see a real gain in adding this new api,
>>>>> This will encourage people to use non-standard nvmem bindings.
>>>>>
>>>>> why not just use standard nvmem bindings.. and use
>>>>>
>>>>> of_nvmem_cell_get(np, "foo");
>>>>>
>>>>> which should work in your case.
>>>>
>>>> It will not work in our case. I believe what you are referring to will
>>>> work if I were to pass the device node pointer which was a NVMEM consumer
>>>> containing the nvmem-cells property. In our case, we pass the device node
>>>> pointer pointing to /soc which is not a nvmem consumer. In this case it
>>>> will not have nvmem-cells property causing of_nvmem_cell_get to return
>>>> EINVAL when it calls of_parse_phandle with "nvmem-cells".
>>>
>>> I could not see any bindings/ dt patches or dt examples for this this
>>> series.. so Am guessing your node would look like:
>>>
>>> soc {
>>> 	cfg0: cfg0 {
>>> 		...
>>> 	};
>>> 	cfg1: cfg1 {
>>> 		...
>>> 	};
>>> };
>>>
>>> If this is not how it looks, can you share the details.
>>>
>>> What Am saying is that why not have:
>>>
>>> soc {
>>> 	nvmem-cells = <&cfg0>, <&cfg1>;
>>> 	nvmem-cell-names = "cfg0", "cfg1";
>>>
>>> 	cfg0: cfg0 {
>>> 		...
>>> 	};
>>>
>>> 	cfg1: cfg1 {
>>> 		...
>>> 	};
>>> };
>>>
>>>>
>>>> Our requirement is to be able to pass the soc node pointer and then
>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>
>>> Why?
>>
>> Sorry for not providing the background directly. The patches before this
>> series used that approach. In the previous discussions it has been pointed
>> out that it is not acceptable to have additional device tree bindings for
>> providing data that the driver wants at the SoC node level or to have bindings
>> just for the SoC bus driver alone since we aren't really describing the
>> hardware.
>>
> SOC driver seems to search for an arbitrary node by its name, which is
> not a binding and can break anytime in cases If the scope of nvmem
> provider is out of soc node or if the nvmem cells are not named as
> expected. That looks very fragile.

In that case, that just "won't happen" because the soc driver is a very
soc specific driver only used for this device tree. We it will always
bind to that high level soc node.

> 
> If the soc node is actual consumer of nvmem cells, I see no reason why
> we should not use proper nvmem bindings?

There is a reason: We don't describe the hardware with it...

The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
driver are just two register with a unique ID of the SoC. In whatever
driver throughout the system we use that ID (e.g. in a random generator
for initialization) we never describe an actual hardware relation... Its
just software and how we use that unique ID. The device tree is ment to
describe hardware. Hence the NVMEM consumer binding is not suited for
such NVMEM cells...

By describing the NVMEM cells location in device tree (producer API, the
NVMEM cells are in hardware at that location, so using the device tree
for that part is fine) and hard coding the NVMEM cell we need in the
driver code we don't violate the device tree matra "describe the
hardware"...

Looking-up the nodes direcly is what Rob suggested here:
https://lkml.org/lkml/2016/5/23/573

> 
> Given the fact that the patch is potentially bypassing the nvmem
> bindings, am not happy to take it!

If you can provide a solution acceptable by the device tree folks and
works without this patch, I am happy to do it...

Btw, I am not entirely happy with the API name, but did not had a better
idea... And we we should probably add a note that the device tree
consumer binding is the preferred way to do it.

--
Stefan


> 
> thanks,
> srini
> 
>> For the discussion,
>> https://lkml.org/lkml/2016/5/23/573
>> https://lkml.org/lkml/2016/5/2/71
>>
>> Regards,
>> Sanchayan.
>>
>>
>>>
>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>
>>>> Regards,
>>>> Sanchayan.
>>>>
>>>>>
>>>>> thanks,
>>>>> srini
>>>>>>
>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>> node.
>>>>>>
>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>>     drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>     include/linux/nvmem-consumer.h |  1 +
>>>>>>     2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>> index 965911d..470abee 100644
>>>>>> --- a/drivers/nvmem/core.c
>>>>>> +++ b/drivers/nvmem/core.c
>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>
>>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>     /**
>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>      *
>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>      *
>>>>>>      * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>      * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>      * nvmem_cell_put().
>>>>>>      */
>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>> -					    const char *name)
>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>     {
>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>> +	struct device_node *nvmem_np;
>>>>>>     	struct nvmem_cell *cell;
>>>>>>     	struct nvmem_device *nvmem;
>>>>>>     	const __be32 *addr;
>>>>>> -	int rval, len, index;
>>>>>> -
>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>> -
>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>> -	if (!cell_np)
>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>> +	int rval, len;
>>>>>>
>>>>>>     	nvmem_np = of_get_next_parent(cell_np);
>>>>>>     	if (!nvmem_np)
>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>
>>>>>>     	return ERR_PTR(rval);
>>>>>>     }
>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>> +
>>>>>> +/**
>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>> + *
>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>> + *
>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>> + * nvmem_cell_put().
>>>>>> + */
>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>> +					    const char *name)
>>>>>> +{
>>>>>> +	struct device_node *cell_np;
>>>>>> +	int index;
>>>>>> +
>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>> +
>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>> +	if (!cell_np)
>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>> +
>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>> +}
>>>>>>     EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>     #endif
>>>>>>
>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>> index 9bb77d3..bf879fc 100644
>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>     #endif /* CONFIG_NVMEM */
>>>>>>
>>>>>>     #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>     struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>     				     const char *name);
>>>>>>     struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 08/07/16 17:42, Stefan Agner wrote:
> On 2016-07-08 08:41, Srinivas Kandagatla wrote:
>> On 07/07/16 14:48, maitysanchayan@gmail.com wrote:
>>> Hello Srinivas,
>>>
>>> On 16-07-07 1

...

>>>>>
>>>>> Our requirement is to be able to pass the soc node pointer and then
>>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>>
>>>> Why?
>>>
>>> Sorry for not providing the background directly. The patches before this
>>> series used that approach. In the previous discussions it has been pointed
>>> out that it is not acceptable to have additional device tree bindings for
>>> providing data that the driver wants at the SoC node level or to have bindings
>>> just for the SoC bus driver alone since we aren't really describing the
>>> hardware.
>>>
>> SOC driver seems to search for an arbitrary node by its name, which is
>> not a binding and can break anytime in cases If the scope of nvmem
>> provider is out of soc node or if the nvmem cells are not named as
>> expected. That looks very fragile.
>
> In that case, that just "won't happen" because the soc driver is a very
> soc specific driver only used for this device tree. We it will always
> bind to that high level soc node.
>
>>
>> If the soc node is actual consumer of nvmem cells, I see no reason why
>> we should not use proper nvmem bindings?
>
> There is a reason: We don't describe the hardware with it...
>
> The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
> driver are just two register with a unique ID of the SoC. In whatever

"Unique ID of the SOC" doesn't this mean that its a part of soc hw 
description/configuration/setup?

Am still not clear why this setup any different to other use cases like 
mac address/calibration data?

I still feel that this should be described in the DT.

Rob,
  What do you think?


> driver throughout the system we use that ID (e.g. in a random generator
> for initialization) we never describe an actual hardware relation... Its
> just software and how we use that unique ID. The device tree is ment to
> describe hardware. Hence the NVMEM consumer binding is not suited for
> such NVMEM cells...
>
> By describing the NVMEM cells location in device tree (producer API, the
> NVMEM cells are in hardware at that location, so using the device tree
> for that part is fine) and hard coding the NVMEM cell we need in the
> driver code we don't violate the device tree matra "describe the
> hardware"...

IMHO, We should indeed describe the SOC hardware and its relationship 
w.r.t to nvmem provider in device tree. Reasoning being these both are 
some form of IP blocks/hw which depend on each other.

>
> Looking-up the nodes direcly is what Rob suggested here:
> https://lkml.org/lkml/2016/5/23/573

I did read this, I was not convinced that we should do a direct lookup 
for nvmem cells.

thanks,
srini
>
>>
>> Given the fact that the patch is potentially bypassing the nvmem
>> bindings, am not happy to take it!
>
> If you can provide a solution acceptable by the device tree folks and
> works without this patch, I am happy to do it...


>
> Btw, I am not entirely happy with the API name, but did not had a better
> idea... And we we should probably add a note that the device tree
> consumer binding is the preferred way to do it.
>
> --
> Stefan
>
>
>>
>> thanks,
>> srini
>>
>>> For the discussion,
>>> https://lkml.org/lkml/2016/5/23/573
>>> https://lkml.org/lkml/2016/5/2/71
>>>
>>> Regards,
>>> Sanchayan.
>>>
>>>
>>>>
>>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>>
>>>>> Regards,
>>>>> Sanchayan.
>>>>>
>>>>>>
>>>>>> thanks,
>>>>>> srini
>>>>>>>
>>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>>> node.
>>>>>>>
>>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>>> ---
>>>>>>>      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>>      include/linux/nvmem-consumer.h |  1 +
>>>>>>>      2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>>> index 965911d..470abee 100644
>>>>>>> --- a/drivers/nvmem/core.c
>>>>>>> +++ b/drivers/nvmem/core.c
>>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>>
>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>      /**
>>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>>       *
>>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>>       *
>>>>>>>       * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>       * nvmem_cell_put().
>>>>>>>       */
>>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>> -					    const char *name)
>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>>      {
>>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>>> +	struct device_node *nvmem_np;
>>>>>>>      	struct nvmem_cell *cell;
>>>>>>>      	struct nvmem_device *nvmem;
>>>>>>>      	const __be32 *addr;
>>>>>>> -	int rval, len, index;
>>>>>>> -
>>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>> -
>>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>> -	if (!cell_np)
>>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>>> +	int rval, len;
>>>>>>>
>>>>>>>      	nvmem_np = of_get_next_parent(cell_np);
>>>>>>>      	if (!nvmem_np)
>>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>>
>>>>>>>      	return ERR_PTR(rval);
>>>>>>>      }
>>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>> + *
>>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>> + *
>>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>> + * nvmem_cell_put().
>>>>>>> + */
>>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>> +					    const char *name)
>>>>>>> +{
>>>>>>> +	struct device_node *cell_np;
>>>>>>> +	int index;
>>>>>>> +
>>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>> +
>>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>> +	if (!cell_np)
>>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>>> +
>>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>>> +}
>>>>>>>      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>>      #endif
>>>>>>>
>>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>>> index 9bb77d3..bf879fc 100644
>>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>>      #endif /* CONFIG_NVMEM */
>>>>>>>
>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>>      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>      				     const char *name);
>>>>>>>      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>>

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Srinivas Kandagatla]

On 08/07/16 17:42, Stefan Agner wrote:
> On 2016-07-08 08:41, Srinivas Kandagatla wrote:
>> On 07/07/16 14:48, maitysanchayan at gmail.com wrote:
>>> Hello Srinivas,
>>>
>>> On 16-07-07 1

...

>>>>>
>>>>> Our requirement is to be able to pass the soc node pointer and then
>>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>>
>>>> Why?
>>>
>>> Sorry for not providing the background directly. The patches before this
>>> series used that approach. In the previous discussions it has been pointed
>>> out that it is not acceptable to have additional device tree bindings for
>>> providing data that the driver wants at the SoC node level or to have bindings
>>> just for the SoC bus driver alone since we aren't really describing the
>>> hardware.
>>>
>> SOC driver seems to search for an arbitrary node by its name, which is
>> not a binding and can break anytime in cases If the scope of nvmem
>> provider is out of soc node or if the nvmem cells are not named as
>> expected. That looks very fragile.
>
> In that case, that just "won't happen" because the soc driver is a very
> soc specific driver only used for this device tree. We it will always
> bind to that high level soc node.
>
>>
>> If the soc node is actual consumer of nvmem cells, I see no reason why
>> we should not use proper nvmem bindings?
>
> There is a reason: We don't describe the hardware with it...
>
> The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
> driver are just two register with a unique ID of the SoC. In whatever

"Unique ID of the SOC" doesn't this mean that its a part of soc hw 
description/configuration/setup?

Am still not clear why this setup any different to other use cases like 
mac address/calibration data?

I still feel that this should be described in the DT.

Rob,
  What do you think?


> driver throughout the system we use that ID (e.g. in a random generator
> for initialization) we never describe an actual hardware relation... Its
> just software and how we use that unique ID. The device tree is ment to
> describe hardware. Hence the NVMEM consumer binding is not suited for
> such NVMEM cells...
>
> By describing the NVMEM cells location in device tree (producer API, the
> NVMEM cells are in hardware at that location, so using the device tree
> for that part is fine) and hard coding the NVMEM cell we need in the
> driver code we don't violate the device tree matra "describe the
> hardware"...

IMHO, We should indeed describe the SOC hardware and its relationship 
w.r.t to nvmem provider in device tree. Reasoning being these both are 
some form of IP blocks/hw which depend on each other.

>
> Looking-up the nodes direcly is what Rob suggested here:
> https://lkml.org/lkml/2016/5/23/573

I did read this, I was not convinced that we should do a direct lookup 
for nvmem cells.

thanks,
srini
>
>>
>> Given the fact that the patch is potentially bypassing the nvmem
>> bindings, am not happy to take it!
>
> If you can provide a solution acceptable by the device tree folks and
> works without this patch, I am happy to do it...


>
> Btw, I am not entirely happy with the API name, but did not had a better
> idea... And we we should probably add a note that the device tree
> consumer binding is the preferred way to do it.
>
> --
> Stefan
>
>
>>
>> thanks,
>> srini
>>
>>> For the discussion,
>>> https://lkml.org/lkml/2016/5/23/573
>>> https://lkml.org/lkml/2016/5/2/71
>>>
>>> Regards,
>>> Sanchayan.
>>>
>>>
>>>>
>>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>>
>>>>> Regards,
>>>>> Sanchayan.
>>>>>
>>>>>>
>>>>>> thanks,
>>>>>> srini
>>>>>>>
>>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>>> node.
>>>>>>>
>>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>>> ---
>>>>>>>      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>>      include/linux/nvmem-consumer.h |  1 +
>>>>>>>      2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>>> index 965911d..470abee 100644
>>>>>>> --- a/drivers/nvmem/core.c
>>>>>>> +++ b/drivers/nvmem/core.c
>>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>>
>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>      /**
>>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>>       *
>>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>>       *
>>>>>>>       * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>       * nvmem_cell_put().
>>>>>>>       */
>>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>> -					    const char *name)
>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>>      {
>>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>>> +	struct device_node *nvmem_np;
>>>>>>>      	struct nvmem_cell *cell;
>>>>>>>      	struct nvmem_device *nvmem;
>>>>>>>      	const __be32 *addr;
>>>>>>> -	int rval, len, index;
>>>>>>> -
>>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>> -
>>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>> -	if (!cell_np)
>>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>>> +	int rval, len;
>>>>>>>
>>>>>>>      	nvmem_np = of_get_next_parent(cell_np);
>>>>>>>      	if (!nvmem_np)
>>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>>
>>>>>>>      	return ERR_PTR(rval);
>>>>>>>      }
>>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>> + *
>>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>> + *
>>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>> + * nvmem_cell_put().
>>>>>>> + */
>>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>> +					    const char *name)
>>>>>>> +{
>>>>>>> +	struct device_node *cell_np;
>>>>>>> +	int index;
>>>>>>> +
>>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>> +
>>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>> +	if (!cell_np)
>>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>>> +
>>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>>> +}
>>>>>>>      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>>      #endif
>>>>>>>
>>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>>> index 9bb77d3..bf879fc 100644
>>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>>      #endif /* CONFIG_NVMEM */
>>>>>>>
>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>>      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>      				     const char *name);
>>>>>>>      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Stefan Agner]

On 2016-07-08 10:23, Srinivas Kandagatla wrote:
> On 08/07/16 17:42, Stefan Agner wrote:
>> On 2016-07-08 08:41, Srinivas Kandagatla wrote:
>>> On 07/07/16 14:48, maitysanchayan@gmail.com wrote:
>>>> Hello Srinivas,
>>>>
>>>> On 16-07-07 1
> 
> ...
> 
>>>>>>
>>>>>> Our requirement is to be able to pass the soc node pointer and then
>>>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>>>
>>>>> Why?
>>>>
>>>> Sorry for not providing the background directly. The patches before this
>>>> series used that approach. In the previous discussions it has been pointed
>>>> out that it is not acceptable to have additional device tree bindings for
>>>> providing data that the driver wants at the SoC node level or to have bindings
>>>> just for the SoC bus driver alone since we aren't really describing the
>>>> hardware.
>>>>
>>> SOC driver seems to search for an arbitrary node by its name, which is
>>> not a binding and can break anytime in cases If the scope of nvmem
>>> provider is out of soc node or if the nvmem cells are not named as
>>> expected. That looks very fragile.
>>
>> In that case, that just "won't happen" because the soc driver is a very
>> soc specific driver only used for this device tree. We it will always
>> bind to that high level soc node.
>>
>>>
>>> If the soc node is actual consumer of nvmem cells, I see no reason why
>>> we should not use proper nvmem bindings?
>>
>> There is a reason: We don't describe the hardware with it...
>>
>> The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
>> driver are just two register with a unique ID of the SoC. In whatever
> 
> "Unique ID of the SOC" doesn't this mean that its a part of soc hw
> description/configuration/setup?
> 
> Am still not clear why this setup any different to other use cases
> like mac address/calibration data?
> 
> I still feel that this should be described in the DT.
> 
> Rob,
>  What do you think?

[added Rob to "To"]

Rob, can you comment?

--
Stefan

> 
> 
>> driver throughout the system we use that ID (e.g. in a random generator
>> for initialization) we never describe an actual hardware relation... Its
>> just software and how we use that unique ID. The device tree is ment to
>> describe hardware. Hence the NVMEM consumer binding is not suited for
>> such NVMEM cells...
>>
>> By describing the NVMEM cells location in device tree (producer API, the
>> NVMEM cells are in hardware at that location, so using the device tree
>> for that part is fine) and hard coding the NVMEM cell we need in the
>> driver code we don't violate the device tree matra "describe the
>> hardware"...
> 
> IMHO, We should indeed describe the SOC hardware and its relationship
> w.r.t to nvmem provider in device tree. Reasoning being these both are
> some form of IP blocks/hw which depend on each other.
> 
>>
>> Looking-up the nodes direcly is what Rob suggested here:
>> https://lkml.org/lkml/2016/5/23/573
> 
> I did read this, I was not convinced that we should do a direct lookup
> for nvmem cells.
> 
> thanks,
> srini
>>
>>>
>>> Given the fact that the patch is potentially bypassing the nvmem
>>> bindings, am not happy to take it!
>>
>> If you can provide a solution acceptable by the device tree folks and
>> works without this patch, I am happy to do it...
> 
> 
>>
>> Btw, I am not entirely happy with the API name, but did not had a better
>> idea... And we we should probably add a note that the device tree
>> consumer binding is the preferred way to do it.
>>
>> --
>> Stefan
>>
>>
>>>
>>> thanks,
>>> srini
>>>
>>>> For the discussion,
>>>> https://lkml.org/lkml/2016/5/23/573
>>>> https://lkml.org/lkml/2016/5/2/71
>>>>
>>>> Regards,
>>>> Sanchayan.
>>>>
>>>>
>>>>>
>>>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>>>
>>>>>> Regards,
>>>>>> Sanchayan.
>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>> srini
>>>>>>>>
>>>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>>>> node.
>>>>>>>>
>>>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>>>> ---
>>>>>>>>      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>>>      include/linux/nvmem-consumer.h |  1 +
>>>>>>>>      2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>>>> index 965911d..470abee 100644
>>>>>>>> --- a/drivers/nvmem/core.c
>>>>>>>> +++ b/drivers/nvmem/core.c
>>>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>>>
>>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>>      /**
>>>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>>>       *
>>>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>>>       *
>>>>>>>>       * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>>       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>>       * nvmem_cell_put().
>>>>>>>>       */
>>>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>> -					    const char *name)
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>>>      {
>>>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>>>> +	struct device_node *nvmem_np;
>>>>>>>>      	struct nvmem_cell *cell;
>>>>>>>>      	struct nvmem_device *nvmem;
>>>>>>>>      	const __be32 *addr;
>>>>>>>> -	int rval, len, index;
>>>>>>>> -
>>>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>>> -
>>>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>>> -	if (!cell_np)
>>>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>>>> +	int rval, len;
>>>>>>>>
>>>>>>>>      	nvmem_np = of_get_next_parent(cell_np);
>>>>>>>>      	if (!nvmem_np)
>>>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>>>
>>>>>>>>      	return ERR_PTR(rval);
>>>>>>>>      }
>>>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>>> + *
>>>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>>> + *
>>>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>> + * nvmem_cell_put().
>>>>>>>> + */
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>> +					    const char *name)
>>>>>>>> +{
>>>>>>>> +	struct device_node *cell_np;
>>>>>>>> +	int index;
>>>>>>>> +
>>>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>>> +
>>>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>>> +	if (!cell_np)
>>>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>>>> +
>>>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>>>> +}
>>>>>>>>      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>>>      #endif
>>>>>>>>
>>>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>>>> index 9bb77d3..bf879fc 100644
>>>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>>>      #endif /* CONFIG_NVMEM */
>>>>>>>>
>>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>>>      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>>      				     const char *name);
>>>>>>>>      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>>>

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Stefan Agner]

On 2016-07-08 10:23, Srinivas Kandagatla wrote:
> On 08/07/16 17:42, Stefan Agner wrote:
>> On 2016-07-08 08:41, Srinivas Kandagatla wrote:
>>> On 07/07/16 14:48, maitysanchayan at gmail.com wrote:
>>>> Hello Srinivas,
>>>>
>>>> On 16-07-07 1
> 
> ...
> 
>>>>>>
>>>>>> Our requirement is to be able to pass the soc node pointer and then
>>>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>>>
>>>>> Why?
>>>>
>>>> Sorry for not providing the background directly. The patches before this
>>>> series used that approach. In the previous discussions it has been pointed
>>>> out that it is not acceptable to have additional device tree bindings for
>>>> providing data that the driver wants at the SoC node level or to have bindings
>>>> just for the SoC bus driver alone since we aren't really describing the
>>>> hardware.
>>>>
>>> SOC driver seems to search for an arbitrary node by its name, which is
>>> not a binding and can break anytime in cases If the scope of nvmem
>>> provider is out of soc node or if the nvmem cells are not named as
>>> expected. That looks very fragile.
>>
>> In that case, that just "won't happen" because the soc driver is a very
>> soc specific driver only used for this device tree. We it will always
>> bind to that high level soc node.
>>
>>>
>>> If the soc node is actual consumer of nvmem cells, I see no reason why
>>> we should not use proper nvmem bindings?
>>
>> There is a reason: We don't describe the hardware with it...
>>
>> The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
>> driver are just two register with a unique ID of the SoC. In whatever
> 
> "Unique ID of the SOC" doesn't this mean that its a part of soc hw
> description/configuration/setup?
> 
> Am still not clear why this setup any different to other use cases
> like mac address/calibration data?
> 
> I still feel that this should be described in the DT.
> 
> Rob,
>  What do you think?

[added Rob to "To"]

Rob, can you comment?

--
Stefan

> 
> 
>> driver throughout the system we use that ID (e.g. in a random generator
>> for initialization) we never describe an actual hardware relation... Its
>> just software and how we use that unique ID. The device tree is ment to
>> describe hardware. Hence the NVMEM consumer binding is not suited for
>> such NVMEM cells...
>>
>> By describing the NVMEM cells location in device tree (producer API, the
>> NVMEM cells are in hardware at that location, so using the device tree
>> for that part is fine) and hard coding the NVMEM cell we need in the
>> driver code we don't violate the device tree matra "describe the
>> hardware"...
> 
> IMHO, We should indeed describe the SOC hardware and its relationship
> w.r.t to nvmem provider in device tree. Reasoning being these both are
> some form of IP blocks/hw which depend on each other.
> 
>>
>> Looking-up the nodes direcly is what Rob suggested here:
>> https://lkml.org/lkml/2016/5/23/573
> 
> I did read this, I was not convinced that we should do a direct lookup
> for nvmem cells.
> 
> thanks,
> srini
>>
>>>
>>> Given the fact that the patch is potentially bypassing the nvmem
>>> bindings, am not happy to take it!
>>
>> If you can provide a solution acceptable by the device tree folks and
>> works without this patch, I am happy to do it...
> 
> 
>>
>> Btw, I am not entirely happy with the API name, but did not had a better
>> idea... And we we should probably add a note that the device tree
>> consumer binding is the preferred way to do it.
>>
>> --
>> Stefan
>>
>>
>>>
>>> thanks,
>>> srini
>>>
>>>> For the discussion,
>>>> https://lkml.org/lkml/2016/5/23/573
>>>> https://lkml.org/lkml/2016/5/2/71
>>>>
>>>> Regards,
>>>> Sanchayan.
>>>>
>>>>
>>>>>
>>>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>>>
>>>>>> Regards,
>>>>>> Sanchayan.
>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>> srini
>>>>>>>>
>>>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>>>> node.
>>>>>>>>
>>>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>>>> ---
>>>>>>>>      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>>>      include/linux/nvmem-consumer.h |  1 +
>>>>>>>>      2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>>>> index 965911d..470abee 100644
>>>>>>>> --- a/drivers/nvmem/core.c
>>>>>>>> +++ b/drivers/nvmem/core.c
>>>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>>>
>>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>>      /**
>>>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>>>       *
>>>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>>>       *
>>>>>>>>       * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>>       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>>       * nvmem_cell_put().
>>>>>>>>       */
>>>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>> -					    const char *name)
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>>>      {
>>>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>>>> +	struct device_node *nvmem_np;
>>>>>>>>      	struct nvmem_cell *cell;
>>>>>>>>      	struct nvmem_device *nvmem;
>>>>>>>>      	const __be32 *addr;
>>>>>>>> -	int rval, len, index;
>>>>>>>> -
>>>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>>> -
>>>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>>> -	if (!cell_np)
>>>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>>>> +	int rval, len;
>>>>>>>>
>>>>>>>>      	nvmem_np = of_get_next_parent(cell_np);
>>>>>>>>      	if (!nvmem_np)
>>>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>>>
>>>>>>>>      	return ERR_PTR(rval);
>>>>>>>>      }
>>>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>>> + *
>>>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>>> + *
>>>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>> + * nvmem_cell_put().
>>>>>>>> + */
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>> +					    const char *name)
>>>>>>>> +{
>>>>>>>> +	struct device_node *cell_np;
>>>>>>>> +	int index;
>>>>>>>> +
>>>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>>> +
>>>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>>> +	if (!cell_np)
>>>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>>>> +
>>>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>>>> +}
>>>>>>>>      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>>>      #endif
>>>>>>>>
>>>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>>>> index 9bb77d3..bf879fc 100644
>>>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>>>      #endif /* CONFIG_NVMEM */
>>>>>>>>
>>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>>>      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>>      				     const char *name);
>>>>>>>>      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>>>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[Stefan Agner]

On 2016-07-08 10:23, Srinivas Kandagatla wrote:
> On 08/07/16 17:42, Stefan Agner wrote:
>> On 2016-07-08 08:41, Srinivas Kandagatla wrote:
>>> On 07/07/16 14:48, maitysanchayan@gmail.com wrote:
>>>> Hello Srinivas,
>>>>
>>>> On 16-07-07 1
> 
> ...
> 
>>>>>>
>>>>>> Our requirement is to be able to pass the soc node pointer and then
>>>>>> be able to get a nvmem cell by specifying it's name. So for our case
>>>>>
>>>>> Why?
>>>>
>>>> Sorry for not providing the background directly. The patches before this
>>>> series used that approach. In the previous discussions it has been pointed
>>>> out that it is not acceptable to have additional device tree bindings for
>>>> providing data that the driver wants at the SoC node level or to have bindings
>>>> just for the SoC bus driver alone since we aren't really describing the
>>>> hardware.
>>>>
>>> SOC driver seems to search for an arbitrary node by its name, which is
>>> not a binding and can break anytime in cases If the scope of nvmem
>>> provider is out of soc node or if the nvmem cells are not named as
>>> expected. That looks very fragile.
>>
>> In that case, that just "won't happen" because the soc driver is a very
>> soc specific driver only used for this device tree. We it will always
>> bind to that high level soc node.
>>
>>>
>>> If the soc node is actual consumer of nvmem cells, I see no reason why
>>> we should not use proper nvmem bindings?
>>
>> There is a reason: We don't describe the hardware with it...
>>
>> The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
>> driver are just two register with a unique ID of the SoC. In whatever
> 
> "Unique ID of the SOC" doesn't this mean that its a part of soc hw
> description/configuration/setup?
> 
> Am still not clear why this setup any different to other use cases
> like mac address/calibration data?
> 
> I still feel that this should be described in the DT.
> 
> Rob,
>  What do you think?

[added Rob to "To"]

Rob, can you comment?

--
Stefan

> 
> 
>> driver throughout the system we use that ID (e.g. in a random generator
>> for initialization) we never describe an actual hardware relation... Its
>> just software and how we use that unique ID. The device tree is ment to
>> describe hardware. Hence the NVMEM consumer binding is not suited for
>> such NVMEM cells...
>>
>> By describing the NVMEM cells location in device tree (producer API, the
>> NVMEM cells are in hardware at that location, so using the device tree
>> for that part is fine) and hard coding the NVMEM cell we need in the
>> driver code we don't violate the device tree matra "describe the
>> hardware"...
> 
> IMHO, We should indeed describe the SOC hardware and its relationship
> w.r.t to nvmem provider in device tree. Reasoning being these both are
> some form of IP blocks/hw which depend on each other.
> 
>>
>> Looking-up the nodes direcly is what Rob suggested here:
>> https://lkml.org/lkml/2016/5/23/573
> 
> I did read this, I was not convinced that we should do a direct lookup
> for nvmem cells.
> 
> thanks,
> srini
>>
>>>
>>> Given the fact that the patch is potentially bypassing the nvmem
>>> bindings, am not happy to take it!
>>
>> If you can provide a solution acceptable by the device tree folks and
>> works without this patch, I am happy to do it...
> 
> 
>>
>> Btw, I am not entirely happy with the API name, but did not had a better
>> idea... And we we should probably add a note that the device tree
>> consumer binding is the preferred way to do it.
>>
>> --
>> Stefan
>>
>>
>>>
>>> thanks,
>>> srini
>>>
>>>> For the discussion,
>>>> https://lkml.org/lkml/2016/5/23/573
>>>> https://lkml.org/lkml/2016/5/2/71
>>>>
>>>> Regards,
>>>> Sanchayan.
>>>>
>>>>
>>>>>
>>>>>> ocotp node has cfg0 and cfg1 which we want but we cannot use existing
>>>>>> nvmem consumer API since that requires having the nvmem consumer properties
>>>>>> in the node we are binding to viz. is a direct nvmem consumer.
>>>>>>
>>>>>> Regards,
>>>>>> Sanchayan.
>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>> srini
>>>>>>>>
>>>>>>>> Parent node can also be the of_node of the main SoC device
>>>>>>>> node.
>>>>>>>>
>>>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>>>>> ---
>>>>>>>>      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
>>>>>>>>      include/linux/nvmem-consumer.h |  1 +
>>>>>>>>      2 files changed, 32 insertions(+), 13 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>>>>>> index 965911d..470abee 100644
>>>>>>>> --- a/drivers/nvmem/core.c
>>>>>>>> +++ b/drivers/nvmem/core.c
>>>>>>>> @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>>>>>>>>
>>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>>      /**
>>>>>>>> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>>> + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
>>>>>>>>       *
>>>>>>>> - * @dev node: Device tree node that uses the nvmem cell
>>>>>>>> - * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>>> + * @dev node: Device tree node that uses nvmem cell
>>>>>>>>       *
>>>>>>>>       * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>>       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>>       * nvmem_cell_put().
>>>>>>>>       */
>>>>>>>> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>> -					    const char *name)
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
>>>>>>>>      {
>>>>>>>> -	struct device_node *cell_np, *nvmem_np;
>>>>>>>> +	struct device_node *nvmem_np;
>>>>>>>>      	struct nvmem_cell *cell;
>>>>>>>>      	struct nvmem_device *nvmem;
>>>>>>>>      	const __be32 *addr;
>>>>>>>> -	int rval, len, index;
>>>>>>>> -
>>>>>>>> -	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>>> -
>>>>>>>> -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>>> -	if (!cell_np)
>>>>>>>> -		return ERR_PTR(-EINVAL);
>>>>>>>> +	int rval, len;
>>>>>>>>
>>>>>>>>      	nvmem_np = of_get_next_parent(cell_np);
>>>>>>>>      	if (!nvmem_np)
>>>>>>>> @@ -824,6 +816,32 @@ err_mem:
>>>>>>>>
>>>>>>>>      	return ERR_PTR(rval);
>>>>>>>>      }
>>>>>>>> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
>>>>>>>> + *
>>>>>>>> + * @dev node: Device tree node that uses the nvmem cell
>>>>>>>> + * @id: nvmem cell name from nvmem-cell-names property.
>>>>>>>> + *
>>>>>>>> + * Return: Will be an ERR_PTR() on error or a valid pointer
>>>>>>>> + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
>>>>>>>> + * nvmem_cell_put().
>>>>>>>> + */
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>> +					    const char *name)
>>>>>>>> +{
>>>>>>>> +	struct device_node *cell_np;
>>>>>>>> +	int index;
>>>>>>>> +
>>>>>>>> +	index = of_property_match_string(np, "nvmem-cell-names", name);
>>>>>>>> +
>>>>>>>> +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
>>>>>>>> +	if (!cell_np)
>>>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>>>> +
>>>>>>>> +	return of_nvmem_cell_get_direct(cell_np);
>>>>>>>> +}
>>>>>>>>      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
>>>>>>>>      #endif
>>>>>>>>
>>>>>>>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>>>>>>>> index 9bb77d3..bf879fc 100644
>>>>>>>> --- a/include/linux/nvmem-consumer.h
>>>>>>>> +++ b/include/linux/nvmem-consumer.h
>>>>>>>> @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
>>>>>>>>      #endif /* CONFIG_NVMEM */
>>>>>>>>
>>>>>>>>      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>>>>>>>> +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
>>>>>>>>      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>>>>>>>>      				     const char *name);
>>>>>>>>      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
>>>>>>>>

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan]

On 16-07-08 18:23:42, Srinivas Kandagatla wrote:
> 
> 
> On 08/07/16 17:42, Stefan Agner wrote:
> > On 2016-07-08 08:41, Srinivas Kandagatla wrote:
> > > On 07/07/16 14:48, maitysanchayan@gmail.com wrote:
> > > > Hello Srinivas,
> > > > 
> > > > On 16-07-07 1
> 
> ...
> 
> > > > > > 
> > > > > > Our requirement is to be able to pass the soc node pointer and then
> > > > > > be able to get a nvmem cell by specifying it's name. So for our case
> > > > > 
> > > > > Why?
> > > > 
> > > > Sorry for not providing the background directly. The patches before this
> > > > series used that approach. In the previous discussions it has been pointed
> > > > out that it is not acceptable to have additional device tree bindings for
> > > > providing data that the driver wants at the SoC node level or to have bindings
> > > > just for the SoC bus driver alone since we aren't really describing the
> > > > hardware.
> > > > 
> > > SOC driver seems to search for an arbitrary node by its name, which is
> > > not a binding and can break anytime in cases If the scope of nvmem
> > > provider is out of soc node or if the nvmem cells are not named as
> > > expected. That looks very fragile.
> > 
> > In that case, that just "won't happen" because the soc driver is a very
> > soc specific driver only used for this device tree. We it will always
> > bind to that high level soc node.
> > 
> > > 
> > > If the soc node is actual consumer of nvmem cells, I see no reason why
> > > we should not use proper nvmem bindings?
> > 
> > There is a reason: We don't describe the hardware with it...
> > 
> > The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
> > driver are just two register with a unique ID of the SoC. In whatever
> 
> "Unique ID of the SOC" doesn't this mean that its a part of soc hw
> description/configuration/setup?
> 
> Am still not clear why this setup any different to other use cases like mac
> address/calibration data?
> 
> I still feel that this should be described in the DT.
> 
> Rob,
>  What do you think?

Hello Rob,

Can you please comment?

Regards,
Sanchayan.

> 
> 
> > driver throughout the system we use that ID (e.g. in a random generator
> > for initialization) we never describe an actual hardware relation... Its
> > just software and how we use that unique ID. The device tree is ment to
> > describe hardware. Hence the NVMEM consumer binding is not suited for
> > such NVMEM cells...
> > 
> > By describing the NVMEM cells location in device tree (producer API, the
> > NVMEM cells are in hardware at that location, so using the device tree
> > for that part is fine) and hard coding the NVMEM cell we need in the
> > driver code we don't violate the device tree matra "describe the
> > hardware"...
> 
> IMHO, We should indeed describe the SOC hardware and its relationship w.r.t
> to nvmem provider in device tree. Reasoning being these both are some form
> of IP blocks/hw which depend on each other.
> 
> > 
> > Looking-up the nodes direcly is what Rob suggested here:
> > https://lkml.org/lkml/2016/5/23/573
> 
> I did read this, I was not convinced that we should do a direct lookup for
> nvmem cells.
> 
> thanks,
> srini
> > 
> > > 
> > > Given the fact that the patch is potentially bypassing the nvmem
> > > bindings, am not happy to take it!
> > 
> > If you can provide a solution acceptable by the device tree folks and
> > works without this patch, I am happy to do it...
> 
> 
> > 
> > Btw, I am not entirely happy with the API name, but did not had a better
> > idea... And we we should probably add a note that the device tree
> > consumer binding is the preferred way to do it.
> > 
> > --
> > Stefan
> > 
> > 
> > > 
> > > thanks,
> > > srini
> > > 
> > > > For the discussion,
> > > > https://lkml.org/lkml/2016/5/23/573
> > > > https://lkml.org/lkml/2016/5/2/71
> > > > 
> > > > Regards,
> > > > Sanchayan.
> > > > 
> > > > 
> > > > > 
> > > > > > ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> > > > > > nvmem consumer API since that requires having the nvmem consumer properties
> > > > > > in the node we are binding to viz. is a direct nvmem consumer.
> > > > > > 
> > > > > > Regards,
> > > > > > Sanchayan.
> > > > > > 
> > > > > > > 
> > > > > > > thanks,
> > > > > > > srini
> > > > > > > > 
> > > > > > > > Parent node can also be the of_node of the main SoC device
> > > > > > > > node.
> > > > > > > > 
> > > > > > > > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > > > > > > > ---
> > > > > > > >      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> > > > > > > >      include/linux/nvmem-consumer.h |  1 +
> > > > > > > >      2 files changed, 32 insertions(+), 13 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > > > > > > > index 965911d..470abee 100644
> > > > > > > > --- a/drivers/nvmem/core.c
> > > > > > > > +++ b/drivers/nvmem/core.c
> > > > > > > > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > > > > > > > 
> > > > > > > >      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > > > > >      /**
> > > > > > > > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > > > > > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> > > > > > > >       *
> > > > > > > > - * @dev node: Device tree node that uses the nvmem cell
> > > > > > > > - * @id: nvmem cell name from nvmem-cell-names property.
> > > > > > > > + * @dev node: Device tree node that uses nvmem cell
> > > > > > > >       *
> > > > > > > >       * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > > > > >       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > > > > >       * nvmem_cell_put().
> > > > > > > >       */
> > > > > > > > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > > -					    const char *name)
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> > > > > > > >      {
> > > > > > > > -	struct device_node *cell_np, *nvmem_np;
> > > > > > > > +	struct device_node *nvmem_np;
> > > > > > > >      	struct nvmem_cell *cell;
> > > > > > > >      	struct nvmem_device *nvmem;
> > > > > > > >      	const __be32 *addr;
> > > > > > > > -	int rval, len, index;
> > > > > > > > -
> > > > > > > > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > > > > > -
> > > > > > > > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > > > > > -	if (!cell_np)
> > > > > > > > -		return ERR_PTR(-EINVAL);
> > > > > > > > +	int rval, len;
> > > > > > > > 
> > > > > > > >      	nvmem_np = of_get_next_parent(cell_np);
> > > > > > > >      	if (!nvmem_np)
> > > > > > > > @@ -824,6 +816,32 @@ err_mem:
> > > > > > > > 
> > > > > > > >      	return ERR_PTR(rval);
> > > > > > > >      }
> > > > > > > > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > > > > > > > +
> > > > > > > > +/**
> > > > > > > > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > > > > > + *
> > > > > > > > + * @dev node: Device tree node that uses the nvmem cell
> > > > > > > > + * @id: nvmem cell name from nvmem-cell-names property.
> > > > > > > > + *
> > > > > > > > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > > > > > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > > > > > + * nvmem_cell_put().
> > > > > > > > + */
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > > +					    const char *name)
> > > > > > > > +{
> > > > > > > > +	struct device_node *cell_np;
> > > > > > > > +	int index;
> > > > > > > > +
> > > > > > > > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > > > > > +
> > > > > > > > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > > > > > +	if (!cell_np)
> > > > > > > > +		return ERR_PTR(-EINVAL);
> > > > > > > > +
> > > > > > > > +	return of_nvmem_cell_get_direct(cell_np);
> > > > > > > > +}
> > > > > > > >      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> > > > > > > >      #endif
> > > > > > > > 
> > > > > > > > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > > > > > > > index 9bb77d3..bf879fc 100644
> > > > > > > > --- a/include/linux/nvmem-consumer.h
> > > > > > > > +++ b/include/linux/nvmem-consumer.h
> > > > > > > > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> > > > > > > >      #endif /* CONFIG_NVMEM */
> > > > > > > > 
> > > > > > > >      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> > > > > > > >      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > >      				     const char *name);
> > > > > > > >      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > > > > > > > 

[PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan at gmail.com]

On 16-07-08 18:23:42, Srinivas Kandagatla wrote:
> 
> 
> On 08/07/16 17:42, Stefan Agner wrote:
> > On 2016-07-08 08:41, Srinivas Kandagatla wrote:
> > > On 07/07/16 14:48, maitysanchayan at gmail.com wrote:
> > > > Hello Srinivas,
> > > > 
> > > > On 16-07-07 1
> 
> ...
> 
> > > > > > 
> > > > > > Our requirement is to be able to pass the soc node pointer and then
> > > > > > be able to get a nvmem cell by specifying it's name. So for our case
> > > > > 
> > > > > Why?
> > > > 
> > > > Sorry for not providing the background directly. The patches before this
> > > > series used that approach. In the previous discussions it has been pointed
> > > > out that it is not acceptable to have additional device tree bindings for
> > > > providing data that the driver wants at the SoC node level or to have bindings
> > > > just for the SoC bus driver alone since we aren't really describing the
> > > > hardware.
> > > > 
> > > SOC driver seems to search for an arbitrary node by its name, which is
> > > not a binding and can break anytime in cases If the scope of nvmem
> > > provider is out of soc node or if the nvmem cells are not named as
> > > expected. That looks very fragile.
> > 
> > In that case, that just "won't happen" because the soc driver is a very
> > soc specific driver only used for this device tree. We it will always
> > bind to that high level soc node.
> > 
> > > 
> > > If the soc node is actual consumer of nvmem cells, I see no reason why
> > > we should not use proper nvmem bindings?
> > 
> > There is a reason: We don't describe the hardware with it...
> > 
> > The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
> > driver are just two register with a unique ID of the SoC. In whatever
> 
> "Unique ID of the SOC" doesn't this mean that its a part of soc hw
> description/configuration/setup?
> 
> Am still not clear why this setup any different to other use cases like mac
> address/calibration data?
> 
> I still feel that this should be described in the DT.
> 
> Rob,
>  What do you think?

Hello Rob,

Can you please comment?

Regards,
Sanchayan.

> 
> 
> > driver throughout the system we use that ID (e.g. in a random generator
> > for initialization) we never describe an actual hardware relation... Its
> > just software and how we use that unique ID. The device tree is ment to
> > describe hardware. Hence the NVMEM consumer binding is not suited for
> > such NVMEM cells...
> > 
> > By describing the NVMEM cells location in device tree (producer API, the
> > NVMEM cells are in hardware at that location, so using the device tree
> > for that part is fine) and hard coding the NVMEM cell we need in the
> > driver code we don't violate the device tree matra "describe the
> > hardware"...
> 
> IMHO, We should indeed describe the SOC hardware and its relationship w.r.t
> to nvmem provider in device tree. Reasoning being these both are some form
> of IP blocks/hw which depend on each other.
> 
> > 
> > Looking-up the nodes direcly is what Rob suggested here:
> > https://lkml.org/lkml/2016/5/23/573
> 
> I did read this, I was not convinced that we should do a direct lookup for
> nvmem cells.
> 
> thanks,
> srini
> > 
> > > 
> > > Given the fact that the patch is potentially bypassing the nvmem
> > > bindings, am not happy to take it!
> > 
> > If you can provide a solution acceptable by the device tree folks and
> > works without this patch, I am happy to do it...
> 
> 
> > 
> > Btw, I am not entirely happy with the API name, but did not had a better
> > idea... And we we should probably add a note that the device tree
> > consumer binding is the preferred way to do it.
> > 
> > --
> > Stefan
> > 
> > 
> > > 
> > > thanks,
> > > srini
> > > 
> > > > For the discussion,
> > > > https://lkml.org/lkml/2016/5/23/573
> > > > https://lkml.org/lkml/2016/5/2/71
> > > > 
> > > > Regards,
> > > > Sanchayan.
> > > > 
> > > > 
> > > > > 
> > > > > > ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> > > > > > nvmem consumer API since that requires having the nvmem consumer properties
> > > > > > in the node we are binding to viz. is a direct nvmem consumer.
> > > > > > 
> > > > > > Regards,
> > > > > > Sanchayan.
> > > > > > 
> > > > > > > 
> > > > > > > thanks,
> > > > > > > srini
> > > > > > > > 
> > > > > > > > Parent node can also be the of_node of the main SoC device
> > > > > > > > node.
> > > > > > > > 
> > > > > > > > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > > > > > > > ---
> > > > > > > >      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> > > > > > > >      include/linux/nvmem-consumer.h |  1 +
> > > > > > > >      2 files changed, 32 insertions(+), 13 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > > > > > > > index 965911d..470abee 100644
> > > > > > > > --- a/drivers/nvmem/core.c
> > > > > > > > +++ b/drivers/nvmem/core.c
> > > > > > > > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > > > > > > > 
> > > > > > > >      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > > > > >      /**
> > > > > > > > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > > > > > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> > > > > > > >       *
> > > > > > > > - * @dev node: Device tree node that uses the nvmem cell
> > > > > > > > - * @id: nvmem cell name from nvmem-cell-names property.
> > > > > > > > + * @dev node: Device tree node that uses nvmem cell
> > > > > > > >       *
> > > > > > > >       * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > > > > >       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > > > > >       * nvmem_cell_put().
> > > > > > > >       */
> > > > > > > > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > > -					    const char *name)
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> > > > > > > >      {
> > > > > > > > -	struct device_node *cell_np, *nvmem_np;
> > > > > > > > +	struct device_node *nvmem_np;
> > > > > > > >      	struct nvmem_cell *cell;
> > > > > > > >      	struct nvmem_device *nvmem;
> > > > > > > >      	const __be32 *addr;
> > > > > > > > -	int rval, len, index;
> > > > > > > > -
> > > > > > > > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > > > > > -
> > > > > > > > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > > > > > -	if (!cell_np)
> > > > > > > > -		return ERR_PTR(-EINVAL);
> > > > > > > > +	int rval, len;
> > > > > > > > 
> > > > > > > >      	nvmem_np = of_get_next_parent(cell_np);
> > > > > > > >      	if (!nvmem_np)
> > > > > > > > @@ -824,6 +816,32 @@ err_mem:
> > > > > > > > 
> > > > > > > >      	return ERR_PTR(rval);
> > > > > > > >      }
> > > > > > > > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > > > > > > > +
> > > > > > > > +/**
> > > > > > > > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > > > > > + *
> > > > > > > > + * @dev node: Device tree node that uses the nvmem cell
> > > > > > > > + * @id: nvmem cell name from nvmem-cell-names property.
> > > > > > > > + *
> > > > > > > > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > > > > > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > > > > > + * nvmem_cell_put().
> > > > > > > > + */
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > > +					    const char *name)
> > > > > > > > +{
> > > > > > > > +	struct device_node *cell_np;
> > > > > > > > +	int index;
> > > > > > > > +
> > > > > > > > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > > > > > +
> > > > > > > > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > > > > > +	if (!cell_np)
> > > > > > > > +		return ERR_PTR(-EINVAL);
> > > > > > > > +
> > > > > > > > +	return of_nvmem_cell_get_direct(cell_np);
> > > > > > > > +}
> > > > > > > >      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> > > > > > > >      #endif
> > > > > > > > 
> > > > > > > > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > > > > > > > index 9bb77d3..bf879fc 100644
> > > > > > > > --- a/include/linux/nvmem-consumer.h
> > > > > > > > +++ b/include/linux/nvmem-consumer.h
> > > > > > > > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> > > > > > > >      #endif /* CONFIG_NVMEM */
> > > > > > > > 
> > > > > > > >      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> > > > > > > >      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > >      				     const char *name);
> > > > > > > >      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > > > > > > > 

Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node

[maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w]

On 16-07-08 18:23:42, Srinivas Kandagatla wrote:
> 
> 
> On 08/07/16 17:42, Stefan Agner wrote:
> > On 2016-07-08 08:41, Srinivas Kandagatla wrote:
> > > On 07/07/16 14:48, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> > > > Hello Srinivas,
> > > > 
> > > > On 16-07-07 1
> 
> ...
> 
> > > > > > 
> > > > > > Our requirement is to be able to pass the soc node pointer and then
> > > > > > be able to get a nvmem cell by specifying it's name. So for our case
> > > > > 
> > > > > Why?
> > > > 
> > > > Sorry for not providing the background directly. The patches before this
> > > > series used that approach. In the previous discussions it has been pointed
> > > > out that it is not acceptable to have additional device tree bindings for
> > > > providing data that the driver wants at the SoC node level or to have bindings
> > > > just for the SoC bus driver alone since we aren't really describing the
> > > > hardware.
> > > > 
> > > SOC driver seems to search for an arbitrary node by its name, which is
> > > not a binding and can break anytime in cases If the scope of nvmem
> > > provider is out of soc node or if the nvmem cells are not named as
> > > expected. That looks very fragile.
> > 
> > In that case, that just "won't happen" because the soc driver is a very
> > soc specific driver only used for this device tree. We it will always
> > bind to that high level soc node.
> > 
> > > 
> > > If the soc node is actual consumer of nvmem cells, I see no reason why
> > > we should not use proper nvmem bindings?
> > 
> > There is a reason: We don't describe the hardware with it...
> > 
> > The cfg0/cfg1 register which Sanchayan needs to read in the soc bus
> > driver are just two register with a unique ID of the SoC. In whatever
> 
> "Unique ID of the SOC" doesn't this mean that its a part of soc hw
> description/configuration/setup?
> 
> Am still not clear why this setup any different to other use cases like mac
> address/calibration data?
> 
> I still feel that this should be described in the DT.
> 
> Rob,
>  What do you think?

Hello Rob,

Can you please comment?

Regards,
Sanchayan.

> 
> 
> > driver throughout the system we use that ID (e.g. in a random generator
> > for initialization) we never describe an actual hardware relation... Its
> > just software and how we use that unique ID. The device tree is ment to
> > describe hardware. Hence the NVMEM consumer binding is not suited for
> > such NVMEM cells...
> > 
> > By describing the NVMEM cells location in device tree (producer API, the
> > NVMEM cells are in hardware at that location, so using the device tree
> > for that part is fine) and hard coding the NVMEM cell we need in the
> > driver code we don't violate the device tree matra "describe the
> > hardware"...
> 
> IMHO, We should indeed describe the SOC hardware and its relationship w.r.t
> to nvmem provider in device tree. Reasoning being these both are some form
> of IP blocks/hw which depend on each other.
> 
> > 
> > Looking-up the nodes direcly is what Rob suggested here:
> > https://lkml.org/lkml/2016/5/23/573
> 
> I did read this, I was not convinced that we should do a direct lookup for
> nvmem cells.
> 
> thanks,
> srini
> > 
> > > 
> > > Given the fact that the patch is potentially bypassing the nvmem
> > > bindings, am not happy to take it!
> > 
> > If you can provide a solution acceptable by the device tree folks and
> > works without this patch, I am happy to do it...
> 
> 
> > 
> > Btw, I am not entirely happy with the API name, but did not had a better
> > idea... And we we should probably add a note that the device tree
> > consumer binding is the preferred way to do it.
> > 
> > --
> > Stefan
> > 
> > 
> > > 
> > > thanks,
> > > srini
> > > 
> > > > For the discussion,
> > > > https://lkml.org/lkml/2016/5/23/573
> > > > https://lkml.org/lkml/2016/5/2/71
> > > > 
> > > > Regards,
> > > > Sanchayan.
> > > > 
> > > > 
> > > > > 
> > > > > > ocotp node has cfg0 and cfg1 which we want but we cannot use existing
> > > > > > nvmem consumer API since that requires having the nvmem consumer properties
> > > > > > in the node we are binding to viz. is a direct nvmem consumer.
> > > > > > 
> > > > > > Regards,
> > > > > > Sanchayan.
> > > > > > 
> > > > > > > 
> > > > > > > thanks,
> > > > > > > srini
> > > > > > > > 
> > > > > > > > Parent node can also be the of_node of the main SoC device
> > > > > > > > node.
> > > > > > > > 
> > > > > > > > Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > > > > > ---
> > > > > > > >      drivers/nvmem/core.c           | 44 +++++++++++++++++++++++++++++-------------
> > > > > > > >      include/linux/nvmem-consumer.h |  1 +
> > > > > > > >      2 files changed, 32 insertions(+), 13 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > > > > > > > index 965911d..470abee 100644
> > > > > > > > --- a/drivers/nvmem/core.c
> > > > > > > > +++ b/drivers/nvmem/core.c
> > > > > > > > @@ -743,29 +743,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> > > > > > > > 
> > > > > > > >      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > > > > >      /**
> > > > > > > > - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > > > > > + * of_nvmem_cell_get_direct() - Get a nvmem cell from given device node
> > > > > > > >       *
> > > > > > > > - * @dev node: Device tree node that uses the nvmem cell
> > > > > > > > - * @id: nvmem cell name from nvmem-cell-names property.
> > > > > > > > + * @dev node: Device tree node that uses nvmem cell
> > > > > > > >       *
> > > > > > > >       * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > > > > >       * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > > > > >       * nvmem_cell_put().
> > > > > > > >       */
> > > > > > > > -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > > -					    const char *name)
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np)
> > > > > > > >      {
> > > > > > > > -	struct device_node *cell_np, *nvmem_np;
> > > > > > > > +	struct device_node *nvmem_np;
> > > > > > > >      	struct nvmem_cell *cell;
> > > > > > > >      	struct nvmem_device *nvmem;
> > > > > > > >      	const __be32 *addr;
> > > > > > > > -	int rval, len, index;
> > > > > > > > -
> > > > > > > > -	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > > > > > -
> > > > > > > > -	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > > > > > -	if (!cell_np)
> > > > > > > > -		return ERR_PTR(-EINVAL);
> > > > > > > > +	int rval, len;
> > > > > > > > 
> > > > > > > >      	nvmem_np = of_get_next_parent(cell_np);
> > > > > > > >      	if (!nvmem_np)
> > > > > > > > @@ -824,6 +816,32 @@ err_mem:
> > > > > > > > 
> > > > > > > >      	return ERR_PTR(rval);
> > > > > > > >      }
> > > > > > > > +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_direct);
> > > > > > > > +
> > > > > > > > +/**
> > > > > > > > + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> > > > > > > > + *
> > > > > > > > + * @dev node: Device tree node that uses the nvmem cell
> > > > > > > > + * @id: nvmem cell name from nvmem-cell-names property.
> > > > > > > > + *
> > > > > > > > + * Return: Will be an ERR_PTR() on error or a valid pointer
> > > > > > > > + * to a struct nvmem_cell.  The nvmem_cell will be freed by the
> > > > > > > > + * nvmem_cell_put().
> > > > > > > > + */
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > > +					    const char *name)
> > > > > > > > +{
> > > > > > > > +	struct device_node *cell_np;
> > > > > > > > +	int index;
> > > > > > > > +
> > > > > > > > +	index = of_property_match_string(np, "nvmem-cell-names", name);
> > > > > > > > +
> > > > > > > > +	cell_np = of_parse_phandle(np, "nvmem-cells", index);
> > > > > > > > +	if (!cell_np)
> > > > > > > > +		return ERR_PTR(-EINVAL);
> > > > > > > > +
> > > > > > > > +	return of_nvmem_cell_get_direct(cell_np);
> > > > > > > > +}
> > > > > > > >      EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> > > > > > > >      #endif
> > > > > > > > 
> > > > > > > > diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> > > > > > > > index 9bb77d3..bf879fc 100644
> > > > > > > > --- a/include/linux/nvmem-consumer.h
> > > > > > > > +++ b/include/linux/nvmem-consumer.h
> > > > > > > > @@ -136,6 +136,7 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> > > > > > > >      #endif /* CONFIG_NVMEM */
> > > > > > > > 
> > > > > > > >      #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
> > > > > > > > +struct nvmem_cell *of_nvmem_cell_get_direct(struct device_node *cell_np);
> > > > > > > >      struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> > > > > > > >      				     const char *name);
> > > > > > > >      struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> > > > > > > > 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 4/5] soc: Add SoC 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 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>
---
 drivers/soc/Kconfig         |   1 +
 drivers/soc/fsl/Kconfig     |  10 +++
 drivers/soc/fsl/Makefile    |   1 +
 drivers/soc/fsl/soc-vf610.c | 212 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 224 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..746b5e3
--- /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 && OF
+	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..23900ea
--- /dev/null
+++ b/drivers/soc/fsl/soc-vf610.c
@@ -0,0 +1,212 @@
+/*
+ * 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 *soc_node;
+	struct device_node *cfg0_node, *cfg1_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;
+
+	soc_node = of_find_node_by_path("/soc");
+	if (!soc_node)
+		return -ENODEV;
+
+	cfg0_node = of_find_node_by_name(soc_node, "cfg0");
+	if (!cfg0_node) {
+		ret = -ENODEV;
+		goto out_cfg0_node;
+	}
+
+	cfg1_node = of_find_node_by_name(soc_node, "cfg1");
+	if (!cfg1_node) {
+		ret = -ENODEV;
+		goto out_cfg1_node;
+	}
+
+	info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
+	if (IS_ERR(info->ocotp_cfg0)) {
+		ret = PTR_ERR(info->ocotp_cfg0);
+		goto out_ocotp_cfg0;
+	}
+
+	info->ocotp_cfg1 = of_nvmem_cell_get_direct(cfg1_node);
+	if (IS_ERR(info->ocotp_cfg1)) {
+		ret = PTR_ERR(info->ocotp_cfg1);
+		goto out_ocotp_cfg1;
+	}
+
+	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));
+		ret = PTR_ERR(socid1);
+		goto out_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_regmap = syscon_regmap_lookup_by_compatible("fsl,vf610-ocrom");
+	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_regmap_lookup_by_compatible("fsl,vf610-mscm-cpucfg");
+	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:
+	kfree(socid2);
+out_socid2:
+	kfree(socid1);
+out_socid1:
+	nvmem_cell_put(info->ocotp_cfg1);
+out_ocotp_cfg1:
+	nvmem_cell_put(info->ocotp_cfg0);
+out_ocotp_cfg0:
+	of_node_put(cfg1_node);
+out_cfg1_node:
+	of_node_put(cfg0_node);
+out_cfg0_node:
+	of_node_put(soc_node);
+
+	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.9.0

[PATCH v4 4/5] soc: Add SoC 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 expose the SoC specific
properties via sysfs.

A sample output from Colibri Vybrid VF61 is below:

root at colibri-vf:~# cd /sys/bus/soc/devices/soc0
root at colibri-vf:/sys/bus/soc/devices/soc0# ls
family     machine    power      revision   soc_id     subsystem  uevent
root at colibri-vf:/sys/bus/soc/devices/soc0# cat family
Freescale Vybrid VF610
root at colibri-vf:/sys/bus/soc/devices/soc0# cat machine
Freescale Vybrid
root at colibri-vf:/sys/bus/soc/devices/soc0# cat revision
00000013
root at 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 | 212 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 224 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..746b5e3
--- /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 && OF
+	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..23900ea
--- /dev/null
+++ b/drivers/soc/fsl/soc-vf610.c
@@ -0,0 +1,212 @@
+/*
+ * 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 *soc_node;
+	struct device_node *cfg0_node, *cfg1_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;
+
+	soc_node = of_find_node_by_path("/soc");
+	if (!soc_node)
+		return -ENODEV;
+
+	cfg0_node = of_find_node_by_name(soc_node, "cfg0");
+	if (!cfg0_node) {
+		ret = -ENODEV;
+		goto out_cfg0_node;
+	}
+
+	cfg1_node = of_find_node_by_name(soc_node, "cfg1");
+	if (!cfg1_node) {
+		ret = -ENODEV;
+		goto out_cfg1_node;
+	}
+
+	info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
+	if (IS_ERR(info->ocotp_cfg0)) {
+		ret = PTR_ERR(info->ocotp_cfg0);
+		goto out_ocotp_cfg0;
+	}
+
+	info->ocotp_cfg1 = of_nvmem_cell_get_direct(cfg1_node);
+	if (IS_ERR(info->ocotp_cfg1)) {
+		ret = PTR_ERR(info->ocotp_cfg1);
+		goto out_ocotp_cfg1;
+	}
+
+	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));
+		ret = PTR_ERR(socid1);
+		goto out_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_regmap = syscon_regmap_lookup_by_compatible("fsl,vf610-ocrom");
+	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_regmap_lookup_by_compatible("fsl,vf610-mscm-cpucfg");
+	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:
+	kfree(socid2);
+out_socid2:
+	kfree(socid1);
+out_socid1:
+	nvmem_cell_put(info->ocotp_cfg1);
+out_ocotp_cfg1:
+	nvmem_cell_put(info->ocotp_cfg0);
+out_ocotp_cfg0:
+	of_node_put(cfg1_node);
+out_cfg1_node:
+	of_node_put(cfg0_node);
+out_cfg0_node:
+	of_node_put(soc_node);
+
+	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.9.0

Re: [PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform

[Arnd Bergmann]

On Thursday, July 7, 2016 12:09:29 PM CEST Sanchayan Maity wrote:
> 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
> 

I only have one very high-level comment: I think that "family"
is generally supposed to be the more generic one than "machine",
so maybe swap the contents of the two?

Can you have a look at the other users of soc_device and see how
they handle it? I realize that we are already inconsistent about
how we handle it, just try to make it do what the majority of the
others have.

Aside from that, it all looks reasonable.

	Arnd

[PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform

[Arnd Bergmann]

On Thursday, July 7, 2016 12:09:29 PM CEST Sanchayan Maity wrote:
> root at colibri-vf:/sys/bus/soc/devices/soc0# cat family
> Freescale Vybrid VF610
> root at colibri-vf:/sys/bus/soc/devices/soc0# cat machine
> Freescale Vybrid
> 

I only have one very high-level comment: I think that "family"
is generally supposed to be the more generic one than "machine",
so maybe swap the contents of the two?

Can you have a look at the other users of soc_device and see how
they handle it? I realize that we are already inconsistent about
how we handle it, just try to make it do what the majority of the
others have.

Aside from that, it all looks reasonable.

	Arnd

Re: [PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform

[Arnd Bergmann]

On Thursday, July 7, 2016 12:09:29 PM CEST Sanchayan Maity wrote:
> 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
> 

I only have one very high-level comment: I think that "family"
is generally supposed to be the more generic one than "machine",
so maybe swap the contents of the two?

Can you have a look at the other users of soc_device and see how
they handle it? I realize that we are already inconsistent about
how we handle it, just try to make it do what the majority of the
others have.

Aside from that, it all looks reasonable.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform

[kbuild test robot]

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

Hi,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on v4.7-rc6]
[cannot apply to next-20160707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sanchayan-Maity/Implement-SoC-driver-for-Vybrid/20160707-145122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/soc/fsl/soc-vf610.c: In function 'vf610_soc_probe':
>> drivers/soc/fsl/soc-vf610.c:73:21: error: implicit declaration of function 'of_nvmem_cell_get_direct' [-Werror=implicit-function-declaration]
     info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
                        ^
>> drivers/soc/fsl/soc-vf610.c:73:19: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
                      ^
   drivers/soc/fsl/soc-vf610.c:79:19: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info->ocotp_cfg1 = of_nvmem_cell_get_direct(cfg1_node);
                      ^
   cc1: some warnings being treated as errors

vim +/of_nvmem_cell_get_direct +73 drivers/soc/fsl/soc-vf610.c

    67		cfg1_node = of_find_node_by_name(soc_node, "cfg1");
    68		if (!cfg1_node) {
    69			ret = -ENODEV;
    70			goto out_cfg1_node;
    71		}
    72	
  > 73		info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
    74		if (IS_ERR(info->ocotp_cfg0)) {
    75			ret = PTR_ERR(info->ocotp_cfg0);
    76			goto out_ocotp_cfg0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29428 bytes --]

[PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform

[kbuild test robot]

Hi,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on v4.7-rc6]
[cannot apply to next-20160707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sanchayan-Maity/Implement-SoC-driver-for-Vybrid/20160707-145122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/soc/fsl/soc-vf610.c: In function 'vf610_soc_probe':
>> drivers/soc/fsl/soc-vf610.c:73:21: error: implicit declaration of function 'of_nvmem_cell_get_direct' [-Werror=implicit-function-declaration]
     info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
                        ^
>> drivers/soc/fsl/soc-vf610.c:73:19: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
                      ^
   drivers/soc/fsl/soc-vf610.c:79:19: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info->ocotp_cfg1 = of_nvmem_cell_get_direct(cfg1_node);
                      ^
   cc1: some warnings being treated as errors

vim +/of_nvmem_cell_get_direct +73 drivers/soc/fsl/soc-vf610.c

    67		cfg1_node = of_find_node_by_name(soc_node, "cfg1");
    68		if (!cfg1_node) {
    69			ret = -ENODEV;
    70			goto out_cfg1_node;
    71		}
    72	
  > 73		info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
    74		if (IS_ERR(info->ocotp_cfg0)) {
    75			ret = PTR_ERR(info->ocotp_cfg0);
    76			goto out_ocotp_cfg0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 29428 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160708/c4426e6f/attachment-0001.obj>

Re: [PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform

[kbuild test robot]

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

Hi,

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on v4.7-rc6]
[cannot apply to next-20160707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sanchayan-Maity/Implement-SoC-driver-for-Vybrid/20160707-145122
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/soc/fsl/soc-vf610.c: In function 'vf610_soc_probe':
>> drivers/soc/fsl/soc-vf610.c:73:21: error: implicit declaration of function 'of_nvmem_cell_get_direct' [-Werror=implicit-function-declaration]
     info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
                        ^
>> drivers/soc/fsl/soc-vf610.c:73:19: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
                      ^
   drivers/soc/fsl/soc-vf610.c:79:19: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info->ocotp_cfg1 = of_nvmem_cell_get_direct(cfg1_node);
                      ^
   cc1: some warnings being treated as errors

vim +/of_nvmem_cell_get_direct +73 drivers/soc/fsl/soc-vf610.c

    67		cfg1_node = of_find_node_by_name(soc_node, "cfg1");
    68		if (!cfg1_node) {
    69			ret = -ENODEV;
    70			goto out_cfg1_node;
    71		}
    72	
  > 73		info->ocotp_cfg0 = of_nvmem_cell_get_direct(cfg0_node);
    74		if (IS_ERR(info->ocotp_cfg0)) {
    75			ret = PTR_ERR(info->ocotp_cfg0);
    76			goto out_ocotp_cfg0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29428 bytes --]

[PATCH v4 5/5] ARM: dts: vfxxx: Add a compatible binding for Vybrid SoC bus driver

[Sanchayan Maity]

Add a compatible binding to the main soc node required by the
Vybrid SoC bus driver to bind on.

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

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 6c5222e..c68bc72 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -87,7 +87,7 @@
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-		compatible = "simple-bus";
+		compatible = "fsl,vf610-soc", "simple-bus";
 		interrupt-parent = <&mscm_ir>;
 		ranges;
 
-- 
2.9.0

[PATCH v4 5/5] ARM: dts: vfxxx: Add a compatible binding for Vybrid SoC bus driver

[Sanchayan Maity]

Add a compatible binding to the main soc node required by the
Vybrid SoC bus driver to bind on.

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

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 6c5222e..c68bc72 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -87,7 +87,7 @@
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-		compatible = "simple-bus";
+		compatible = "fsl,vf610-soc", "simple-bus";
 		interrupt-parent = <&mscm_ir>;
 		ranges;
 
-- 
2.9.0