diff mbox series

[v3,1/2] dt-bindings: timer: Add Ingenic X1000 OST bindings.

Message ID 20200630171553.70670-2-zhouyanjie@wanyeetech.com
State Superseded
Headers show
Series Add support for the OST in Ingenic X1000. | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch success

Commit Message

Zhou Yanjie June 30, 2020, 5:15 p.m. UTC
Add the OST bindings for the X10000 SoC from Ingenic.

Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    No change.
    
    v2->v3:
    Fix wrong parameters in "clocks".

 .../devicetree/bindings/timer/ingenic,ost.yaml     | 61 ++++++++++++++++++++++
 include/dt-bindings/clock/ingenic,ost.h            | 12 +++++
 2 files changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/ingenic,ost.yaml
 create mode 100644 include/dt-bindings/clock/ingenic,ost.h

Comments

Paul Cercueil June 30, 2020, 7:15 p.m. UTC | #1
Hi Zhou,

Le mer. 1 juil. 2020 à 1:15, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add the OST bindings for the X10000 SoC from Ingenic.
> 
> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
> 
> Notes:
>     v1->v2:
>     No change.
> 
>     v2->v3:
>     Fix wrong parameters in "clocks".
> 
>  .../devicetree/bindings/timer/ingenic,ost.yaml     | 61 
> ++++++++++++++++++++++
>  include/dt-bindings/clock/ingenic,ost.h            | 12 +++++

Please name them ingenic,sysost.yaml and ingenic,sysost.h, to 
differenciate with the OST that is in the JZ SoCs.

>  2 files changed, 73 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/timer/ingenic,ost.yaml
>  create mode 100644 include/dt-bindings/clock/ingenic,ost.h
> 
> diff --git a/Documentation/devicetree/bindings/timer/ingenic,ost.yaml 
> b/Documentation/devicetree/bindings/timer/ingenic,ost.yaml
> new file mode 100644
> index 000000000000..459dd3d161c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ingenic,ost.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/ingenic,ost.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for SYSOST in Ingenic XBurst family SoCs
> +
> +maintainers:
> +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> +
> +description:
> +  The SYSOST in an Ingenic SoC provides one 64bit timer for 
> clocksource
> +  and one or more than one 32bit timers for clockevent.

"and one or more than one" -> "and one or more"

> +
> +properties:
> +  compatible:
> +    oneOf:
> +
> +      - description: SYSOST in Ingenic XBurst family SoCs

XBurst is the name of the CPU, not a SoC family, so you would just 
write 'Ingenic SoCs'. But just drop the description alltogether, it 
does not add anything valuable.

> +        enum:
> +          - ingenic,x1000-ost
> +          - ingenic,x2000-ost

You have "ingenic,x2000-ost" as a compatible string, but your driver 
(in patch [2/2]) only handles the first compatible string. Either they 
are different, in this case the driver should handle both, or they work 
the same, and in the case the "ingenic,x2000-ost" string should use 
"ingenic,x1000-ost" as a fallback string.

Cheers,
-Paul

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: ost
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - "#clock-cells"
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/x1000-cgu.h>
> +
> +    ost: timer@12000000 {
> +    		compatible = "ingenic,x1000-ost";
> +    		reg = <0x12000000 0x3c>;
> +
> +    		#clock-cells = <1>;
> +
> +    		clocks = <&cgu X1000_CLK_OST>;
> +    		clock-names = "ost";
> +
> +    		interrupt-parent = <&cpuintc>;
> +    		interrupts = <3>;
> +    	};
> +...
> diff --git a/include/dt-bindings/clock/ingenic,ost.h 
> b/include/dt-bindings/clock/ingenic,ost.h
> new file mode 100644
> index 000000000000..9ac88e90babf
> --- /dev/null
> +++ b/include/dt-bindings/clock/ingenic,ost.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides clock numbers for the ingenic,tcu DT binding.
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_INGENIC_OST_H__
> +#define __DT_BINDINGS_CLOCK_INGENIC_OST_H__
> +
> +#define OST_CLK_PERCPU_TIMER	0
> +#define OST_CLK_GLOBAL_TIMER	1
> +
> +#endif /* __DT_BINDINGS_CLOCK_INGENIC_OST_H__ */
> --
> 2.11.0
>
Zhou Yanjie July 1, 2020, 4:53 p.m. UTC | #2
Hi Paul,

在 2020/7/1 上午3:15, Paul Cercueil 写道:
> Hi Zhou,
>
> Le mer. 1 juil. 2020 à 1:15, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add the OST bindings for the X10000 SoC from Ingenic.
>>
>> Tested-by: 周正 (Zhou Zheng) <sernia.zhou@foxmail.com>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>
>> Notes:
>>     v1->v2:
>>     No change.
>>
>>     v2->v3:
>>     Fix wrong parameters in "clocks".
>>
>>  .../devicetree/bindings/timer/ingenic,ost.yaml     | 61 
>> ++++++++++++++++++++++
>>  include/dt-bindings/clock/ingenic,ost.h            | 12 +++++
>
> Please name them ingenic,sysost.yaml and ingenic,sysost.h, to 
> differenciate with the OST that is in the JZ SoCs.
>

Sure.


>>  2 files changed, 73 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/timer/ingenic,ost.yaml
>>  create mode 100644 include/dt-bindings/clock/ingenic,ost.h
>>
>> diff --git a/Documentation/devicetree/bindings/timer/ingenic,ost.yaml 
>> b/Documentation/devicetree/bindings/timer/ingenic,ost.yaml
>> new file mode 100644
>> index 000000000000..459dd3d161c2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/ingenic,ost.yaml
>> @@ -0,0 +1,61 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/timer/ingenic,ost.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bindings for SYSOST in Ingenic XBurst family SoCs
>> +
>> +maintainers:
>> +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> +
>> +description:
>> +  The SYSOST in an Ingenic SoC provides one 64bit timer for clocksource
>> +  and one or more than one 32bit timers for clockevent.
>
> "and one or more than one" -> "and one or more"
>

OK, I'll change it in the next version.


>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +
>> +      - description: SYSOST in Ingenic XBurst family SoCs
>
> XBurst is the name of the CPU, not a SoC family, so you would just 
> write 'Ingenic SoCs'. But just drop the description alltogether, it 
> does not add anything valuable.
>

Sure.


>> +        enum:
>> +          - ingenic,x1000-ost
>> +          - ingenic,x2000-ost
>
> You have "ingenic,x2000-ost" as a compatible string, but your driver 
> (in patch [2/2]) only handles the first compatible string. Either they 
> are different, in this case the driver should handle both, or they 
> work the same, and in the case the "ingenic,x2000-ost" string should 
> use "ingenic,x1000-ost" as a fallback string.
>

If SMT is not turned on, X2000 can be compatible with X1000, but if SMT 
is turned on, some additional processing is required, and now this 
compatible string is reserved for this.

Thanks and best regards!


> Cheers,
> -Paul
>
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    const: ost
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +required:
>> +  - "#clock-cells"
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - interrupts
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/x1000-cgu.h>
>> +
>> +    ost: timer@12000000 {
>> +            compatible = "ingenic,x1000-ost";
>> +            reg = <0x12000000 0x3c>;
>> +
>> +            #clock-cells = <1>;
>> +
>> +            clocks = <&cgu X1000_CLK_OST>;
>> +            clock-names = "ost";
>> +
>> +            interrupt-parent = <&cpuintc>;
>> +            interrupts = <3>;
>> +        };
>> +...
>> diff --git a/include/dt-bindings/clock/ingenic,ost.h 
>> b/include/dt-bindings/clock/ingenic,ost.h
>> new file mode 100644
>> index 000000000000..9ac88e90babf
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/ingenic,ost.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * This header provides clock numbers for the ingenic,tcu DT binding.
>> + */
>> +
>> +#ifndef __DT_BINDINGS_CLOCK_INGENIC_OST_H__
>> +#define __DT_BINDINGS_CLOCK_INGENIC_OST_H__
>> +
>> +#define OST_CLK_PERCPU_TIMER    0
>> +#define OST_CLK_GLOBAL_TIMER    1
>> +
>> +#endif /* __DT_BINDINGS_CLOCK_INGENIC_OST_H__ */
>> -- 
>> 2.11.0
>>
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/timer/ingenic,ost.yaml b/Documentation/devicetree/bindings/timer/ingenic,ost.yaml
new file mode 100644
index 000000000000..459dd3d161c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ingenic,ost.yaml
@@ -0,0 +1,61 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/ingenic,ost.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for SYSOST in Ingenic XBurst family SoCs
+
+maintainers:
+  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+
+description:
+  The SYSOST in an Ingenic SoC provides one 64bit timer for clocksource
+  and one or more than one 32bit timers for clockevent.
+
+properties:
+  compatible:
+    oneOf:
+
+      - description: SYSOST in Ingenic XBurst family SoCs
+        enum:
+          - ingenic,x1000-ost
+          - ingenic,x2000-ost
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: ost
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - "#clock-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/clock/x1000-cgu.h>
+
+    ost: timer@12000000 {
+    		compatible = "ingenic,x1000-ost";
+    		reg = <0x12000000 0x3c>;
+
+    		#clock-cells = <1>;
+
+    		clocks = <&cgu X1000_CLK_OST>;
+    		clock-names = "ost";
+
+    		interrupt-parent = <&cpuintc>;
+    		interrupts = <3>;
+    	};
+...
diff --git a/include/dt-bindings/clock/ingenic,ost.h b/include/dt-bindings/clock/ingenic,ost.h
new file mode 100644
index 000000000000..9ac88e90babf
--- /dev/null
+++ b/include/dt-bindings/clock/ingenic,ost.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides clock numbers for the ingenic,tcu DT binding.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_INGENIC_OST_H__
+#define __DT_BINDINGS_CLOCK_INGENIC_OST_H__
+
+#define OST_CLK_PERCPU_TIMER	0
+#define OST_CLK_GLOBAL_TIMER	1
+
+#endif /* __DT_BINDINGS_CLOCK_INGENIC_OST_H__ */