Implement SoC bus driver for Vybrid
Subsystems (4)
v2 cover Implement SoC bus driver for Vybrid
Hello,
This second patch series is rebased on top of shawn's for-next branch and tested on Colibri Vybrid VF50 and VF61 modules.
This patchset implements SoC bus support for Freescale Vybrid platform, implementing the following
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc
This a reworked version of an older patchset series posted in June 2015 which was at v5 then [1]. Since the NVMEM framework was then getting introduced, we decided that first a NVMEM driver for OCOTP peripheral being in place would be better.
Compared to the older revisions, this driver now relies on NVMEM consumer API using the NVMEM based vf610_ocotp driver which has already been in mainline for a while now. Also now a new syscon abstraction "syscon_regmap_read_from_offset" is implemented and exported from syscon allowing accessing a register from a syscon reference like this
ocotp-cfg1 = <&ocotp 0x20>;
avoiding code repetition in the driver.
One point on which we were not sure here is whether we really should introduce a new Kconfig symbol as being introduced here. While we could just enable it when SOC_VF610 is selected, this however would introduce circular dependencies.
Feedback is most welcome.
Changes since v1:
Add device tree binding documentation.
v1 new patchset
https://lkml.org/lkml/2016/3/11/132
[1] Older v5:
http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html
Even earlier versions:
Version 4 of the patchset can be found here
https://lkml.org/lkml/2015/5/26/199
Version 3 of the patchset can be found here
http://www.spinics.net/lists/arm-kernel/msg420847.html
Version 2 of the patchset can be found here
http://www.spinics.net/lists/devicetree/msg80654.html
Version 1 of the patchset can be found here
http://www.spinics.net/lists/devicetree/msg80257.html
The RFC version can be found here
https://lkml.org/lkml/2015/5/11/13
Regards,
Sanchayan.
Sanchayan Maity (5):
mfd: syscon: Introduce syscon_regmap_read_from_offset
ARM: dts: vfxxx: Add device tree node for OCOTP
ARM: dts: vfxxx: Add OCROM and phandle entries for Vybrid SoC bus driver
soc: Add SoC bus driver for Freescale Vybrid Platform
vf610-soc: Add Vybrid SoC device tree binding documentation
.../bindings/arm/freescale/fsl,vf610-soc.txt | 35 +++++ arch/arm/boot/dts/vfxxx.dtsi | 28 +++- drivers/mfd/syscon.c | 30 ++++ drivers/soc/Kconfig | 1 + drivers/soc/fsl/Kconfig | 10 ++ drivers/soc/fsl/Makefile | 1 + drivers/soc/fsl/soc-vf610.c | 160 +++++++++++++++++++++ include/linux/mfd/syscon.h | 10 ++ 8 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt create mode 100644 drivers/soc/fsl/Kconfig create mode 100644 drivers/soc/fsl/soc-vf610.c
Signature · 1 line
2.8.2
PATCH v2 1/5 mfd: syscon: Introduce syscon_regmap_read_from_offset
Currently syscon does not provide an abstraction to access a register from syscon reference like below
ocotp-cfg1 = <&ocotp 0x20>
syscon_regmap_read_from_offset provides a generic abstraction to access a register from syscon reference as above. It allows to specify the node and node name of phandle reference, reading the offset from the node entry and providing the value from the offset in the register map.
--- drivers/mfd/syscon.c | 30 ++++++++++++++++++++++++++++++ include/linux/mfd/syscon.h | 10 ++++++++++ 2 files changed, 40 insertions(+)
drivers/mfd/syscon.c +30 -0
@@ -136,6 +136,36 @@ struct regmap *syscon_node_to_regmap(struct device_node *np) }EXPORT_SYMBOL_GPL (syscon_node_to_regmap); +int syscon_regmap_read_from_offset (struct device_node *np, +const char *s,unsigned int *val) +{ +struct of_phandle_args pargs; +struct regmap *regmap; +int offset; +int ret; + +if (!np) +return -ENODEV ; + + ret =of_parse_phandle_with_fixed_args (np, s,1 ,0 , &pargs); +if (ret) +return ret; + + regmap =syscon_node_to_regmap (pargs.np); +if (IS_ERR (regmap)) { +of_node_put (pargs.np); +return PTR_ERR (regmap); + } + + offset = pargs.args[0 ]; +of_node_put (pargs.np); + + ret =regmap_read (regmap, offset, val); + +return ret; +} +EXPORT_SYMBOL_GPL (syscon_regmap_read_from_offset); +struct regmap *syscon_regmap_lookup_by_compatible (const char *s) {struct device_node *syscon_np;
include/linux/mfd/syscon.h +10 -0
@@ -26,6 +26,9 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);extern struct regmap *syscon_regmap_lookup_by_phandle (struct device_node *np,const char *property); +extern int syscon_regmap_read_from_offset (struct device_node *np, +const char *s, +unsigned int *val);#else static inline struct regmap *syscon_node_to_regmap (struct device_node *np) {
@@ -48,6 +51,13 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle( {return ERR_PTR (-ENOTSUPP ); } + +static inline int syscon_regmap_read_from_offset (struct device_node *np, +const char *s, +unsigned int *val) +{ +return ERR_PTR (-ENOSYS ); +}#endif #endif /* __LINUX_MFD_SYSCON_H__ */
Signature · 1 line
2.8.2
PATCH v2 2/5 ARM: dts: vfxxx: Add device tree node for OCOTP
In reply to Sanchayan MaityAdd 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.8.2
Arnd Bergmann
maintainerRe: [PATCH v2 2/5] ARM: dts: vfxxx: Add device tree node for OCOTP
Quoted 5 lines · On Monday 02 May 2016 12:35:01 Sanchayan Maity wrote:
arch/arm/boot/dts/vfxxx.dtsi +5
-0+ ocotp at 400a5000 { + compatible = "fsl,vf610-ocotp"; + #address-cells = <1>; + #size-cells = <1>; + reg = <0x400a5000 0xCF0>;
arch/arm/boot/dts/vfxxx.dtsi line 537
Quoted 10 lines
arch/arm/boot/dts/vfxxx.dtsi +10
-0+ clocks = <&clks VF610_CLK_OCOTP>; + + ocotp_cfg0: cfg0 at 410 { + reg = <0x410 0x4>; + }; + + ocotp_cfg1: cfg1 at 420 { + reg = <0x420 0x4>; + }; + };
How do the registers of the child nodes relate to the registers of the parent? If there are in the same address space, it might be good to add a "ranges" property to describe it.
Arnd
-------------- next part --------------
An embedded message was scrubbed...
From: Gavin Shan <[email protected]>
Subject: Re: [PATCH v8 40/45] drivers/of: Split unflatten_dt_node()
Date: Mon, 02 May 2016 12:02:21 +1000
Size: 8869
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160502/8b012f87/attachment-0001.mht>
Re: [PATCH v2 2/5] ARM: dts: vfxxx: Add device tree node for OCOTP
Hello Arnd,
Quoted 12 lines · Arnd Bergmann
Quoted 11 lines · On Monday 02 May 2016 12:35:01 Sanchayan Maity wrote:
arch/arm/boot/dts/vfxxx.dtsi +11
-0+ 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>; + }; +
Quoted 8 lines
arch/arm/boot/dts/vfxxx.dtsi line 537
Quoted 4 lines
arch/arm/boot/dts/vfxxx.dtsi +4
-0+ ocotp_cfg1: cfg1 at 420 { + reg = <0x420 0x4>; + }; + };How do the registers of the child nodes relate to the registers of the parent? If there are in the same address space, it might be good to add a "ranges" property to describe it.
cfg0 and cfg1 are in the same address space viz. 0x400a5410 and 0x400a5420 respectively. These nodes are primarily for use by the NVMEM consumer API in the SoC bus driver to retrieve the values from these registers leveraging the NVMEM vf610 ocotp driver.
Based on the NVMEM bindings here
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/nvmem/nvmem.txt#L33
Thanks.
Regards,
Sanchayan.
Quoted 71 lines
Arnd
Date: Mon, 02 May 2016 12:02:21 +1000
From: Gavin Shan <[email protected]>
To: Rob Herring <[email protected]>
Cc: "devicetree at vger.kernel.org" <[email protected]>,
aik at ozlabs.ru, Gavin Shan <[email protected]>, Grant Likely <[email protected]>, "linux-pci at vger.kernel.org" <[email protected]>, Bjorn Helgaas <[email protected]>, linuxppc-dev <[email protected]>, dja at axtens.net
Subject: Re: [PATCH v8 40/45]drivers/of: Splitunflatten_dt_node()Quoted 35 lines · Rob Herring
Quoted 12 lines · Gavin Shan
The function
unflatten_dt_node()is called recursively to unflatten device nodes and properties in the FDT blob. It looks complicated and hard to be understood.This splits the function into 3 functions:
populate_properties(),populate_node()andunflatten_dt_node().populate_properties(), which is called bypopulate_node(), creates properties for the indicated device node. The later one creates the device nodes from FDT blob.populate_node()gets the offset in FDT blob for next device nodes and then callspopulate_node(). No logical changes introduced.Quoted 4 lines
Signed-off-by Gavin Shan <[email protected]>--- drivers/of/fdt.c | 249 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 147 insertions(+), 102 deletions(-)One nit, otherwise:
Acked-by Rob Herring <[email protected]>[...]
Review
Quoted 10 lines
(quoted diff) +10
-0+ /* And we process the "ibm,phandle" property + * used in pSeries dynamic device tree + * stuff + */ + if (!strcmp(pname, "ibm,phandle")) + np->phandle = be32_to_cpup(val); + + pp->name = (char *)pname; + pp->length = sz; + pp->value = (__be32 *)val;Quoted 2 lines
This cast should not be needed.
It's needed. Otherwise, we will have warning. So I will keep it. I just went through this one for next revision and sorry for late response.
drivers/of/fdt.c:225:14: warning: assignment discards ?const? qualifier from pointer target typepp->value = val; ^Thanks,
GavinQuoted 4 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ *pprev = pp; + pprev = &pp->next; + }_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
PATCH v2 3/5 ARM: dts: vfxxx: Add OCROM and phandle entries for Vybrid SoC bus driver
In reply to Sanchayan MaityAdd OCROM node and introduce phandles to OCROM, MSCM and NVMEM OCOTP for use by the Vybrid SoC bus driver.
--- arch/arm/boot/dts/vfxxx.dtsi | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm/boot/dts/vfxxx.dtsi +11 -1
@@ -87,9 +87,19 @@ soc { #address-cells = <1>; #size-cells = <1>; - compatible = "simple-bus"; + compatible = "fsl,vf610-soc-bus", "simple-bus"; interrupt-parent = <&mscm_ir>; ranges; + fsl,rom-revision = <&ocrom 0x80>; + fsl,cpu-count = <&mscm_cpucfg 0x2C>; + fsl,l2-size = <&mscm_cpucfg 0x14>; + nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>; + nvmem-cell-names = "cfg0", "cfg1"; + + ocrom: ocrom at 00000000 { + compatible = "fsl,vf610-ocrom", "syscon"; + reg = <0x00000000 0x18000>; + }; aips0: aips-bus at 40000000 { compatible = "fsl,aips-bus", "simple-bus";
Signature · 1 line
2.8.2
PATCH v2 4/5 soc: Add SoC bus 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 sysfs exposes the SoC specific properties.
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 | 160 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 172 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 + 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 +160 -0
160 lines not on this page · raw message
Signature · 1 line
2.8.2
PATCH v2 5/5 vf610-soc: Add Vybrid SoC device tree binding documentation
In reply to Sanchayan MaityAdd device tree binding documentation for Vybrid SoC.
--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +35 -0
@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus" +- interrupt-parent: phandle to the MSCM interrupt router node +- ranges +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom + revision register +- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of + CPU count register +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of + L2 cache size register +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 +- nvmem-cell-names: should contain string names "cfg0" and "cfg1" + +Example: + + soc { + #address-cells = <1>; + #size-cells = <1>; + compatible = "fsl,vf610-soc-bus", "simple-bus"; + interrupt-parent = <&mscm_ir>; + ranges; + fsl,rom-revision = <&ocrom 0x80>; + fsl,cpu-count = <&mscm_cpucfg 0x2C>; + fsl,l2-size = <&mscm_cpucfg 0x14>; + nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>; + nvmem-cell-names = "cfg0", "cfg1"; + + ... + };
Signature · 1 line
2.8.2
Rob Herring
maintainerRe: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Quoted 14 lines · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
Signed-off-by Sanchayan Maity <[email protected]>--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txtDocumentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +0
-0
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +8
-0@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus"
If this is a bus, put the file in bindings/bus/...
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 12
Quoted 4 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +4
-0+- interrupt-parent: phandle to the MSCM interrupt router node +- ranges +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom + revision register
Why is this needed here? Can't you search the tree for the ROM node and get this info.
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 18
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +6
-0+- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of + CPU count register +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of + L2 cache size register +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
How are all these properties used?
Quoted 20 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +17
-0+ +Example: + + soc { + #address-cells = <1>; + #size-cells = <1>; + compatible = "fsl,vf610-soc-bus", "simple-bus"; + interrupt-parent = <&mscm_ir>; + ranges; + fsl,rom-revision = <&ocrom 0x80>; + fsl,cpu-count = <&mscm_cpucfg 0x2C>; + fsl,l2-size = <&mscm_cpucfg 0x14>; + nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>; + nvmem-cell-names = "cfg0", "cfg1"; + + ... + };Signature · 1 line
2.8.2
Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Hello Rob,
Quoted 9 lines · Rob Herring
Quoted 8 lines · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
Signed-off-by Sanchayan Maity <[email protected]>--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt
Quoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +0
-0Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +8
-0@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus"If this is a bus, put the file in bindings/bus/...
The fsl,vf610-soc-bus binding is used to bind the driver in question with an appropriate compatible node.
Basically being a standalone platform driver, there was need of a compatible property to bind on. Introducing a separate device tree node for it's sake didn't seem appropriate so the alteration to SoC node's compatible.
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 12
Quoted 5 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +4
-0+- interrupt-parent: phandle to the MSCM interrupt router node +- ranges +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom + revision registerWhy is this needed here? Can't you search the tree for the ROM node and get this info.
Strictly per say this and next two can be specified in their respective nodes of ocrom and mscm cpucfg, however they would then require the use of syscon_ regmap_lookup_by_compatible and this seems clean along with the introduction of new syscon_regmap_read_from_offset function used with SoC node.
Quoted 9 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 18
Quoted 7 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +6
-0+- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of + CPU count register +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of + L2 cache size register +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"How are all these properties used?
All the above five are used to get the relevant values from the registers and expose the information for SoC sysfs device.
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-soc
nvmem are consumer nodes which are accessed using the NVMEM consumer API's leveraging the NVMEM framework and NVMEM vf610 ocotp driver.
Regards,
Sanchayan.
Quoted 21 lines
Quoted 21 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +17
-0+ +Example: + + soc { + #address-cells = <1>; + #size-cells = <1>; + compatible = "fsl,vf610-soc-bus", "simple-bus"; + interrupt-parent = <&mscm_ir>; + ranges; + fsl,rom-revision = <&ocrom 0x80>; + fsl,cpu-count = <&mscm_cpucfg 0x2C>; + fsl,l2-size = <&mscm_cpucfg 0x14>; + nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>; + nvmem-cell-names = "cfg0", "cfg1"; + + ... + };Signature · 1 line
2.8.2
Rob Herring
maintainerRe: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Quoted 29 lines · On Thu, May 5, 2016 at 3:27 AM, <[email protected]> wrote:
Hello Rob,
Quoted 9 lines · Rob Herring
Quoted 8 lines · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
Signed-off-by Sanchayan Maity <[email protected]>--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txtQuoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +0
-0Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +8
-0@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus"If this is a bus, put the file in bindings/bus/...
Quoted 6 lines
The fsl,vf610-soc-bus binding is used to bind the driver in question with an appropriate compatible node.
Basically being a standalone platform driver, there was need of a compatible property to bind on. Introducing a separate device tree node for it's sake didn't seem appropriate so the alteration to SoC node's compatible.
Ah, so you are designing a node around the needs of a Linux specific driver. Don't do that. DT describes the h/w and this node is not a h/w block.
Create a platform device based on a matching SOC compatible string instead and make your driver find the information it needs directly from the relevant nodes like the ROM node.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +4
-0+- interrupt-parent: phandle to the MSCM interrupt router node +- ranges +- fsl,rom-revision: phandle to the on-chip ROM node and address of rom + revision register
Quoted 7 lines
Quoted 2 lines
Why is this needed here? Can't you search the tree for the ROM node and get this info.
Strictly per say this and next two can be specified in their respective nodes of ocrom and mscm cpucfg, however they would then require the use of
syscon_regmap_lookup_by_compatibleand this seems clean along with the introduction of newsyscon_regmap_read_from_offsetfunction used with SoC node.
That does not make sense. You can find the ROM node by compatible string, get the address, get an revision offset property, and read the address. There's no need for syscon nor regmap here.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +6
-0+- fsl,cpu-count: phandle to the MSCM CPU configuration node and address of + CPU count register +- fsl,l2-size: phandle to the MSCM CPU configuration node and address of + L2 cache size register +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 +- nvmem-cell-names: should contain string names "cfg0" and "cfg1"
Quoted 8 lines
Quoted 1 line
How are all these properties used?
All the above five are used to get the relevant values from the registers and expose the information for SoC sysfs device.
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-socnvmem are consumer nodes which are accessed using the NVMEM consumer API's leveraging the NVMEM framework and NVMEM vf610 ocotp driver.
So, please describe in the binding doc what the values contain. cfg0 and cfg1 is meaningless. But based on the above, this whole binding should go.
Rob
Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Quoted 37 lines · Rob Herring
Quoted 29 lines · On Thu, May 5, 2016 at 3:27 AM, <[email protected]> wrote:
Hello Rob,
Quoted 9 lines · Rob Herring
Quoted 8 lines · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
Signed-off-by Sanchayan Maity <[email protected]>--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txtQuoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +0
-0Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +8
-0@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus"If this is a bus, put the file in bindings/bus/...
Quoted 6 lines
The fsl,vf610-soc-bus binding is used to bind the driver in question with an appropriate compatible node.
Basically being a standalone platform driver, there was need of a compatible property to bind on. Introducing a separate device tree node for it's sake didn't seem appropriate so the alteration to SoC node's compatible.
Quoted 7 lines
Ah, so you are designing a node around the needs of a Linux specific driver. Don't do that. DT describes the h/w and this node is not a h/w block.
Create a platform device based on a matching SOC compatible string instead and make your driver find the information it needs directly from the relevant nodes like the ROM node.
That reads like my words a year ago:
https://lkml.org/lkml/2015/5/22/408
Initially pretty much everything was hard-coded in the driver.
Arnd then pushed to use more descriptive in the device tree.
Of course, we should not end up making up relations which are not there in hardware. We need to find the right balance.
Here is my suggestion:
1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it to bind the "soc bus driver" as a platform driver located in driver/soc/
2. Add ROM as syscon device (it is not erasable ROM memory, hence eeprom seems not to be appropriate)
3. In the new soc bus driver, search for the relevant nodes using hardcoded strings:
- "ocrom" to get the syscon device, read the ROM revision with the
hardcoded offset
- "ocotp" to get the NVMEM device or "cfg0"/"cfg1" to the cells
directly, read the values using the defined cells.
- "mscm_cpucfg" is already there as a syscon device
Arnd, Rob, does that sound reasonable?
--
Stefan
Rob Herring
maintainerRe: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Quoted 51 lines · Stefan Agner
Quoted 37 lines · Rob Herring
Quoted 29 lines · On Thu, May 5, 2016 at 3:27 AM, <[email protected]> wrote:
Hello Rob,
Quoted 9 lines · Rob Herring
Quoted 8 lines · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
Signed-off-by Sanchayan Maity <[email protected]>--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txtQuoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +0
-0Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +8
-0@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus"If this is a bus, put the file in bindings/bus/...
Quoted 6 lines
The fsl,vf610-soc-bus binding is used to bind the driver in question with an appropriate compatible node.
Basically being a standalone platform driver, there was need of a compatible property to bind on. Introducing a separate device tree node for it's sake didn't seem appropriate so the alteration to SoC node's compatible.
Quoted 7 lines
Ah, so you are designing a node around the needs of a Linux specific driver. Don't do that. DT describes the h/w and this node is not a h/w block.
Create a platform device based on a matching SOC compatible string instead and make your driver find the information it needs directly from the relevant nodes like the ROM node.
That reads like my words a year ago:
https://lkml.org/lkml/2015/5/22/408Initially pretty much everything was hard-coded in the driver.
Quoted 8 lines
Arnd then pushed to use more descriptive in the device tree.
Of course, we should not end up making up relations which are not there in hardware. We need to find the right balance.
Here is my suggestion:
1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it to bind the "soc bus driver" as a platform driver located in driver/soc/
I'm not convinced this is a h/w block. This keeps coming up and I think this is a kernel problem, not a DT problem. Let's face it that there are drivers at the SOC level which don't fit into a DT node. They may be the exception, but they are a common exception. My proposal for how to deal with these cases is here[1].
I also think drivers/soc is a mess because it is randomly used or not. IMO, using it should not be at the whim of whomever does SOC support.
Quoted 2 lines
2. Add ROM as syscon device (it is not erasable ROM memory, hence eeprom seems not to be appropriate)
It is not a syscon. It is just memory. nvmem covers read-only memory, so I have no issue using that.
Rob
Re: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Quoted 61 lines · Rob Herring
Quoted 51 lines · Stefan Agner
Quoted 37 lines · Rob Herring
Quoted 29 lines · On Thu, May 5, 2016 at 3:27 AM, <[email protected]> wrote:
Hello Rob,
Quoted 9 lines · Rob Herring
Quoted 8 lines · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
Signed-off-by Sanchayan Maity <[email protected]>--- .../bindings/arm/freescale/fsl,vf610-soc.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txtQuoted 16 lines
Quoted 6 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +0
-0Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt line 8
Quoted 8 lines
Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt new file +8
-0@@ -0,0 +1,35 @@ +Vybrid System-on-Chip +--------------------- + +Required properties: + +- #address-cells: must be 1 +- #size-cells: must be 1 +- compatible: "fsl,vf610-soc-bus", "simple-bus"If this is a bus, put the file in bindings/bus/...
Quoted 6 lines
The fsl,vf610-soc-bus binding is used to bind the driver in question with an appropriate compatible node.
Basically being a standalone platform driver, there was need of a compatible property to bind on. Introducing a separate device tree node for it's sake didn't seem appropriate so the alteration to SoC node's compatible.
Quoted 7 lines
Ah, so you are designing a node around the needs of a Linux specific driver. Don't do that. DT describes the h/w and this node is not a h/w block.
Create a platform device based on a matching SOC compatible string instead and make your driver find the information it needs directly from the relevant nodes like the ROM node.
That reads like my words a year ago:
https://lkml.org/lkml/2015/5/22/408Initially pretty much everything was hard-coded in the driver.
Quoted 8 lines
Arnd then pushed to use more descriptive in the device tree.
Of course, we should not end up making up relations which are not there in hardware. We need to find the right balance.
Here is my suggestion:
1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it to bind the "soc bus driver" as a platform driver located in driver/soc/
Quoted 5 lines
I'm not convinced this is a h/w block. This keeps coming up and I think this is a kernel problem, not a DT problem. Let's face it that there are drivers at the SOC level which don't fit into a DT node. They may be the exception, but they are a common exception. My proposal for how to deal with these cases is here[1].
Probably "fsl,vf610-soc" would be better than "fsl,vf610-soc-bus".
But the SoC is definitely a h/w block! Despite all the individual silicon in there, I can even touch the SoC ;-)
But yeah, I understand what you are saying... We model it as a container, and do not attribute any specific thing directly to it. But aren't there other containers where we bind to? Is it wrong to bind a driver to a container, if that driver implements issues concerning that level..?
The two drivers under drivers/soc/ which implement the SoC bus API and both use a compatible string on a container level (e.g. arm,realview-pb11mp-soc in arch/arm/boot/dts/arm-realview-pb11mp.dts).
Quoted 3 lines
I also think
drivers/socis a mess because it is randomly used or not. IMO, using it should not be at the whim of whomever does SOC support.
Are you talking about random drivers under drivers/soc/ or the ones which implement the SoC bus API? I think that is not the same...
If SoC bus, agreed, it is a mess. Not only its adoption is sparse, the specification is also interpreted differently across different SoC's, which I brought up in the past:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333765.html
But it exports really useful information to user space, and that has real value in practice.
--
Stefan
Rob Herring
maintainerRe: [PATCH v2 5/5] vf610-soc: Add Vybrid SoC device tree binding documentation
Quoted 9 lines · Stefan Agner
Quoted 8 lines · Rob Herring
Quoted 7 lines · Stefan Agner
Quoted 6 lines · Rob Herring
Quoted 5 lines · On Thu, May 5, 2016 at 3:27 AM, <[email protected]> wrote:
Hello Rob,
Quoted 2 lines · Rob Herring
Quoted 1 line · Sanchayan Maity
Add device tree binding documentation for Vybrid SoC.
[...]
Quoted 26 lines
Quoted 26 lines
Quoted 25 lines
Quoted 11 lines
Quoted 3 lines
Basically being a standalone platform driver, there was need of a compatible property to bind on. Introducing a separate device tree node for it's sake didn't seem appropriate so the alteration to SoC node's compatible.
Ah, so you are designing a node around the needs of a Linux specific driver. Don't do that. DT describes the h/w and this node is not a h/w block.
Create a platform device based on a matching SOC compatible string instead and make your driver find the information it needs directly from the relevant nodes like the ROM node.
That reads like my words a year ago:
https://lkml.org/lkml/2015/5/22/408Initially pretty much everything was hard-coded in the driver.
Arnd then pushed to use more descriptive in the device tree.
Of course, we should not end up making up relations which are not there in hardware. We need to find the right balance.
Here is my suggestion:
1. Add "fsl,vf610-soc-bus" as compatible string to the soc node, use it to bind the "soc bus driver" as a platform driver located in driver/soc/
Quoted 7 lines
Quoted 5 lines
I'm not convinced this is a h/w block. This keeps coming up and I think this is a kernel problem, not a DT problem. Let's face it that there are drivers at the SOC level which don't fit into a DT node. They may be the exception, but they are a common exception. My proposal for how to deal with these cases is here[1].
Probably "fsl,vf610-soc" would be better than "fsl,vf610-soc-bus".
But the node in question is just the bus part of the SoC. Things like cpus are not included.
Quoted 2 lines
But the SoC is definitely a h/w block! Despite all the individual silicon in there, I can even touch the SoC ;-)
Can't argue with that...
Quoted 5 lines
But yeah, I understand what you are saying... We model it as a container, and do not attribute any specific thing directly to it. But aren't there other containers where we bind to? Is it wrong to bind a driver to a container, if that driver implements issues concerning that level..?
It is supposed to be a bus, not just a container. However, we probably don't generally fully and accurately model the buses in SoCs so DT ends up being just a container frequently. If the bus is more than simple-bus, then it should have some resource that needs to be controlled to enable the bus.
Quoted 4 lines
The two drivers under
drivers/soc/which implement the SoC bus API and both use a compatible string on a container level (e.g. arm,realview-pb11mp-soc inarch/arm/boot/dts/arm-realview-pb11mp.dts).
I'm not sure Realview and Integrator are models to follow. They are a bit unique in their h/w design.
Quoted 7 lines
Quoted 4 lines
I also think
drivers/socis a mess because it is randomly used or not. IMO, using it should not be at the whim of whomever does SOC support.Are you talking about random drivers under
drivers/soc/or the ones which implement the SoC bus API? I think that is not the same...
The SoC bus API.
Quoted 7 lines
If SoC bus, agreed, it is a mess. Not only its adoption is sparse, the specification is also interpreted differently across different SoC's, which I brought up in the past:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/333765.htmlBut it exports really useful information to user space, and that has real value in practice.
Yes, but the information exported has nothing really to do with a bus. The "bus" part of it is more sub-classing platform_bus_type for all platform devices on an SOC. That container may or may not directly correspond to the bus structure of the SOC as defined in DT. I guess it is the mixing of really 2 independent things that I don't like and has made its use sparse. We should perhaps mandate using the bus (for what I described) and make the information part separate and be able to populate it from any driver (e.g. an SoC syscon driver which reads the SOC revision).
Rob