Implement SoC driver for Vybrid
Subsystems (3)
v4 cover Implement SoC driver for Vybrid
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
Signature · 1 line
2.9.0
PATCH v4 1/5 ARM: dts: vfxxx: Add device tree node for OCOTP
Add device tree node for the OCOTP peripheral on Vybrid.
--- arch/arm/boot/dts/vfxxx.dtsi | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
arch/arm/boot/dts/vfxxx.dtsi +16 -0
@@ -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>;
Signature · 1 line
2.9.0
PATCH v4 2/5 ARM: dts: vfxxx: Add On-Chip ROM node for Vybrid
In reply to Sanchayan MaityAdd a device tree node for the On-Chip ROM on Vybrid.
--- arch/arm/boot/dts/vfxxx.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm/boot/dts/vfxxx.dtsi +5 -0
@@ -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>;
Signature · 1 line
2.9.0
PATCH v4 3/5 nvmem: core: Add consumer API to get nvmem cell from node
In reply to Sanchayan MaityFrom: Stefan Agner <[email protected]>
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.
--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)
drivers/nvmem/core.c +31 -13
@@ -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
include/linux/nvmem-consumer.h +1 -0
@@ -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,
Signature · 1 line
2.9.0
Re: [PATCH v4 3/5] nvmem: core: Add consumer API to get nvmem cell from node
Quoted 6 lines · Sanchayan Maity
From: Stefan Agner <[email protected]>
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.
Quoted 8 lines
Assuming the nvmem cell id name is known, this can be used as follows
struct
device_node*cell_np;
structnvmem_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
Quoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Hello Srinivas,
Quoted 18 lines · Srinivas Kandagatla
Quoted 6 lines · Sanchayan Maity
From: Stefan Agner <[email protected]>
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.
Quoted 8 lines
Assuming the nvmem cell id name is known, this can be used as follows
struct
device_node*cell_np;
structnvmem_cell*foo_cell;
cell_np=of_find_node_by_name(parent, "foo");foo_cell=of_nvmem_cell_get_direct(cell_np);
Quoted 8 lines
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.
Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Quoted 28 lines · maitysanchayan at gmail.com
Hello Srinivas,
Quoted 25 lines · Srinivas Kandagatla
Quoted 6 lines · Sanchayan Maity
From: Stefan Agner <[email protected]>
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.
Quoted 8 lines
Assuming the nvmem cell id name is known, this can be used as follows
struct
device_node*cell_np;
structnvmem_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");
Quoted 8 lines
Quoted 1 line
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_getto return EINVAL when it callsof_parse_phandlewith "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 {
...
};
};Quoted 3 lines
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?
Quoted 105 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Hello Srinivas,
Quoted 70 lines · Srinivas Kandagatla
Quoted 28 lines · maitysanchayan at gmail.com
Hello Srinivas,
Quoted 25 lines · Srinivas Kandagatla
Quoted 6 lines · Sanchayan Maity
From: Stefan Agner <[email protected]>
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.
Quoted 8 lines
Assuming the nvmem cell id name is known, this can be used as follows
struct
device_node*cell_np;
structnvmem_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");Quoted 8 lines
Quoted 1 line
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_getto return EINVAL when it callsof_parse_phandlewith "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 { ... }; };Quoted 1 line
Quoted 4 lines
Quoted 2 lines
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.
Quoted 106 lines
Quoted 106 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Quoted 76 lines · maitysanchayan at gmail.com
Hello Srinivas,
Quoted 73 lines · Srinivas Kandagatla
Quoted 28 lines · maitysanchayan at gmail.com
Hello Srinivas,
Quoted 25 lines · Srinivas Kandagatla
Quoted 6 lines · Sanchayan Maity
From: Stefan Agner <[email protected]>
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.
Quoted 8 lines
Assuming the nvmem cell id name is known, this can be used as follows
struct
device_node*cell_np;
structnvmem_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");Quoted 8 lines
Quoted 1 line
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_getto return EINVAL when it callsof_parse_phandlewith "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 { ... }; };Quoted 3 lines
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
Quoted 9 lines
Quoted 1 line
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
Quoted 114 lines
For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71Regards,
Sanchayan.Quoted 106 lines
Quoted 106 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Quoted 86 lines · Srinivas Kandagatla
Quoted 76 lines · maitysanchayan at gmail.com
Hello Srinivas,
Quoted 73 lines · Srinivas Kandagatla
Quoted 28 lines · maitysanchayan at gmail.com
Hello Srinivas,
Quoted 25 lines · Srinivas Kandagatla
Quoted 6 lines · Sanchayan Maity
From: Stefan Agner <[email protected]>
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.
Quoted 8 lines
Assuming the nvmem cell id name is known, this can be used as follows
struct
device_node*cell_np;
structnvmem_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");Quoted 8 lines
Quoted 1 line
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_getto return EINVAL when it callsof_parse_phandlewith "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 { ... }; };Quoted 3 lines
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
Quoted 9 lines
Quoted 1 line
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.
Quoted 4 lines
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.
Quoted 3 lines
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
Quoted 3 lines
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
Quoted 118 lines
thanks,
sriniQuoted 114 lines
For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71Regards,
Sanchayan.Quoted 106 lines
Quoted 106 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Quoted 5 lines · Stefan Agner
Quoted 4 lines · Srinivas Kandagatla
Quoted 3 lines · maitysanchayan at gmail.com
Hello Srinivas,
On 16-07-07 1
...
Quoted 23 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
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
Quoted 9 lines
Quoted 1 line
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.
Quoted 4 lines
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.
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
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?
Quoted 11 lines
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.
Quoted 3 lines
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
Quoted 136 lines
Quoted 4 lines
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.
--
StefanQuoted 118 lines
thanks,
sriniQuoted 114 lines
For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71Regards,
Sanchayan.Quoted 106 lines
Quoted 106 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
Quoted 43 lines · Srinivas Kandagatla
Quoted 5 lines · Stefan Agner
Quoted 4 lines · Srinivas Kandagatla
Quoted 3 lines · maitysanchayan at gmail.com
Hello Srinivas,
On 16-07-07 1
...
Quoted 23 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
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
Quoted 9 lines
Quoted 1 line
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.
Quoted 4 lines
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.
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
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?
Quoted 7 lines
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
Quoted 163 lines
Quoted 8 lines
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...
Quoted 5 lines
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.
Quoted 3 lines
Looking-up the nodes direcly is what Rob suggested here:
https://lkml.org/lkml/2016/5/23/573I did read this, I was not convinced that we should do a direct lookup for nvmem cells.
thanks,
sriniQuoted 136 lines
Quoted 4 lines
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.
--
StefanQuoted 118 lines
thanks,
sriniQuoted 114 lines
For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71Regards,
Sanchayan.Quoted 106 lines
Quoted 106 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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
In reply to Srinivas KandagatlaQuoted 45 lines · Srinivas Kandagatla
Quoted 5 lines · Stefan Agner
Quoted 4 lines · Srinivas Kandagatla
Quoted 3 lines · maitysanchayan at gmail.com
Hello Srinivas,
On 16-07-07 1
...
Quoted 23 lines
Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
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
Quoted 9 lines
Quoted 1 line
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.
Quoted 4 lines
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.
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
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?
Quoted 7 lines
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.
Quoted 163 lines
Quoted 8 lines
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...
Quoted 5 lines
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.
Quoted 3 lines
Looking-up the nodes direcly is what Rob suggested here:
https://lkml.org/lkml/2016/5/23/573I did read this, I was not convinced that we should do a direct lookup for nvmem cells.
thanks,
sriniQuoted 136 lines
Quoted 4 lines
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.
--
StefanQuoted 118 lines
thanks,
sriniQuoted 114 lines
For the discussion,
https://lkml.org/lkml/2016/5/23/573
https://lkml.org/lkml/2016/5/2/71Regards,
Sanchayan.Quoted 106 lines
Quoted 106 lines
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.Quoted 98 lines
thanks,
sriniQuoted 95 lines
Parent node can also be the
of_nodeof the main SoC device node.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/nvmem/core.c | 44 +++++++++++++++++++++++++++++------------- include/linux/nvmem-consumer.h | 1 + 2 files changed, 32 insertions(+), 13 deletions(-)drivers/nvmem/core.c +31
-13@@ -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 include/linux/nvmem-consumer.h +1
-0@@ -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 4/5 soc: Add SoC driver for Freescale Vybrid platform
In reply to Sanchayan MaityThis 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
--- 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
drivers/soc/Kconfig +1 -0
@@ -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"
drivers/soc/fsl/Kconfig new file +10 -0
@@ -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.
drivers/soc/fsl/Makefile +1 -0
@@ -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/
drivers/soc/fsl/soc-vf610.c new file +212 -0
212 lines not on this page · raw message
Signature · 1 line
2.9.0
Re: [PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform
Quoted 5 lines · CEST Sanchayan Maity
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
kbuild test robot
build botRe: [PATCH v4 4/5] soc: Add SoC driver for Freescale Vybrid platform
In reply to Sanchayan Maityreproduce: 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…
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>
PATCH v4 5/5 ARM: dts: vfxxx: Add a compatible binding for Vybrid SoC bus driver
In reply to Sanchayan MaityAdd a compatible binding to the main soc node required by the Vybrid SoC bus driver to bind on.
--- arch/arm/boot/dts/vfxxx.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/vfxxx.dtsi +1 -1
@@ -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;
Signature · 1 line
2.9.0