| Message ID | 20220408081258.57213-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: timer: Update TI timer to yaml and add compatible for am6 | expand |
On 11:12-20220408, Tony Lindgren wrote: [...] > + ti,hwmods: > + description: > + Name of the HWMOD associated with timer. This is for legacy > + omap2/3 platforms only. > + $ref: /schemas/types.yaml#/definitions/string > + deprecated: true > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + Should we strengthen the check like in 8250_omap.yaml - something to the effect of: if: properties: compatible: contains: enum: - ti,omap2420-timer - ti,omap3430-timer - ti,omap4430-timer then: properties: ti,hwmods: items: - pattern: "^timer([1-9]+)$" else: properties: ti,hwmods: false
On 08/04/2022 10:12, Tony Lindgren wrote: > Let's update the TI timer binding to use yaml. And add compatible for > ti,am654-timer for TI am64, am65 and j72 SoCs. As the timer hardware is > the same between am64, am65 and j72 we use the compatible name for the > earliest SoC with this timer. > > As this binding is specific to the TI dual-mode timers also known > as dm-timers, let's use ti,timer-dm.yaml naming for the new file. Thank you for your patch. There is something to discuss/improve. > --- > .../bindings/timer/ti,timer-dm.yaml | 105 ++++++++++++++++++ > .../devicetree/bindings/timer/ti,timer.txt | 44 -------- pwm-omap-dmtimer.txt references old path. > 2 files changed, 105 insertions(+), 44 deletions(-) > create mode 100644 Documentation/devicetree/bindings/timer/ti,timer-dm.yaml > delete mode 100644 Documentation/devicetree/bindings/timer/ti,timer.txt > > diff --git a/Documentation/devicetree/bindings/timer/ti,timer-dm.yaml b/Documentation/devicetree/bindings/timer/ti,timer-dm.yaml > new file mode 100644 > --- /dev/null > +++ b/Documentation/devicetree/bindings/timer/ti,timer-dm.yaml > @@ -0,0 +1,105 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/timer/ti,timer-dm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Binding for TI dual-mode timer "TI dual-mode timer" > + > +maintainers: > + - Tony Lindgren <tony@atomide.com> > + > +description: | > + The TI dual-mode timer is a general purpose timer with PWM capabilities. > + > +properties: > + compatible: > + enum: > + - ti,omap2420-timer > + - ti,omap3430-timer > + - ti,omap4430-timer > + - ti,omap5430-timer > + - ti,am335x-timer > + - ti,am335x-timer-1ms > + - ti,am654-timer How about ordering the entries by name (so amxxx before omapxxx)? > + > + reg: > + minItems: 1 > + maxItems: 2 > + description: Timer IO register range This was maxItems:1 in old binding, so please mention briefly in commit msg why the change is needed. If only some versions need it, then add allOf:if:then: constraints. > + > + '#address-cells': > + enum: [ 1, 2 ] > + > + '#size-cells': > + enum: [ 1, 2 ] The same. Are these changes to the binding an effect of new compatible? If yes, better to split it into two patches. One for old binding (passing dtbs_check on old compatibles) and one for new compatible with new properties. > + > + clocks: > + description: > + The functional clock for the timer. Some SoCs like omap24xx also have a > + separate interface clock, and some clocks may be only defined for the > + interconnect target module parent. > + minItems: 1 > + maxItems: 2 The same - not mentioned in commit msg. > + > + clock-names: > + description: > + Timer clock names like "fck", "timer_sys_ck". > + oneOf: > + - enum: [ ick, fck ] > + - items: > + - const: fck > + - enum: [ ick, timer_sys_ck ] Are the combinations depending on compatible? If so, you need allOf:if:then: > + > + interrupts: > + description: > + Interrupt if available. The timer PWM features may be usable > + in a limited way even without interrupts. > + maxItems: 1 > + > + ti,timer-alwon: > + description: > + Timer is always enabled when the SoC is powered. Note that some SoCs like > + am335x can suspend to PM coprocessor RTC only mode and in that case the > + SoC power is cut including timers. > + type: boolean > + > + ti,timer-dsp: > + description: > + Timer is routable to the DSP in addition to the operating system. > + type: boolean > + > + ti,timer-pwm: > + description: > + Timer has been wired for PWM capability. > + type: boolean > + > + ti,timer-secure: > + description: > + Timer access has been limited to secure mode only. > + type: boolean > + > + ti,hwmods: > + description: > + Name of the HWMOD associated with timer. This is for legacy > + omap2/3 platforms only. > + $ref: /schemas/types.yaml#/definitions/string > + deprecated: true > + > +required: > + - compatible > + - reg Missing interrupts - they were required. Aren't anymore? > + > +additionalProperties: false > + Best regards, Krzysztof
* Nishanth Menon <nm@ti.com> [220408 08:25]: > On 11:12-20220408, Tony Lindgren wrote: > [...] > > > + ti,hwmods: > > + description: > > + Name of the HWMOD associated with timer. This is for legacy > > + omap2/3 platforms only. > > + $ref: /schemas/types.yaml#/definitions/string > > + deprecated: true > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > + > > Should we strengthen the check like in 8250_omap.yaml - something to the > effect of: > > if: > properties: > compatible: > contains: > enum: > - ti,omap2420-timer > - ti,omap3430-timer > - ti,omap4430-timer > > then: > properties: > ti,hwmods: > items: > - pattern: "^timer([1-9]+)$" > > else: > properties: > ti,hwmods: false Sure, sounds good to me. I need to check again what all SoCs are already using ti-sysc for all the timers. I believe system timers only got updated for some SoCs. Regards, Tony
On 08/04/2022 10:27, Nishanth Menon wrote: > On 11:12-20220408, Tony Lindgren wrote: > [...] > >> + ti,hwmods: >> + description: >> + Name of the HWMOD associated with timer. This is for legacy >> + omap2/3 platforms only. >> + $ref: /schemas/types.yaml#/definitions/string >> + deprecated: true >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + > > Should we strengthen the check like in 8250_omap.yaml - something to the > effect of: > > if: Yes, please. Within allOf because other properties also might need such constraints. Best regards, Krzysztof
* Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> [220408 08:30]: > On 08/04/2022 10:12, Tony Lindgren wrote: > > +required: > > + - compatible > > + - reg > > Missing interrupts - they were required. Aren't anymore? There are cases where the interrupts are not routable to Linux yet the timer registers can be accessible. These timers can still be usable in a limited way for some PWM cases though. Thanks for the good comments on the binding, will update accordingly. Yeah adding a second patch for the new features makes sense as few things have changed with the new SoCs. Regards, Tony
On Fri, 08 Apr 2022 11:12:58 +0300, Tony Lindgren wrote: > Let's update the TI timer binding to use yaml. And add compatible for > ti,am654-timer for TI am64, am65 and j72 SoCs. As the timer hardware is > the same between am64, am65 and j72 we use the compatible name for the > earliest SoC with this timer. > > As this binding is specific to the TI dual-mode timers also known > as dm-timers, let's use ti,timer-dm.yaml naming for the new file. > > Cc: Daniel Lezcano <daniel.lezcano@linaro.org> > Cc: Keerthy <j-keerthy@ti.com> > Cc: Nishanth Menon <nm@ti.com> > Cc: Vignesh Raghavendra <vigneshr@ti.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > .../bindings/timer/ti,timer-dm.yaml | 105 ++++++++++++++++++ > .../devicetree/bindings/timer/ti,timer.txt | 44 -------- > 2 files changed, 105 insertions(+), 44 deletions(-) > create mode 100644 Documentation/devicetree/bindings/timer/ti,timer-dm.yaml > delete mode 100644 Documentation/devicetree/bindings/timer/ti,timer.txt > Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings. Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future. Full log is available here: https://patchwork.ozlabs.org/patch/ timer@0: compatible:0: 'ti,am4372-timer-1ms' is not one of ['ti,omap2420-timer', 'ti,omap3430-timer', 'ti,omap4430-timer', 'ti,omap5430-timer', 'ti,am335x-timer', 'ti,am335x-timer-1ms', 'ti,am654-timer'] arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb timer@0: compatible:0: 'ti,am4372-timer' is not one of ['ti,omap2420-timer', 'ti,omap3430-timer', 'ti,omap4430-timer', 'ti,omap5430-timer', 'ti,am335x-timer', 'ti,am335x-timer-1ms', 'ti,am654-timer'] arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb timer@0: compatible: ['ti,am4372-timer-1ms', 'ti,am335x-timer-1ms'] is too long arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb timer@0: compatible: ['ti,am4372-timer', 'ti,am335x-timer'] is too long arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-cm-t43.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-gp-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-idk-evm.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sbc-t43.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am437x-sk-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb arch/arm/boot/dts/am43x-epos-evm.dtb
* Rob Herring <robh@kernel.org> [220408 15:39]: > On Fri, 08 Apr 2022 11:12:58 +0300, Tony Lindgren wrote: > > Let's update the TI timer binding to use yaml. And add compatible for > > ti,am654-timer for TI am64, am65 and j72 SoCs. As the timer hardware is > > the same between am64, am65 and j72 we use the compatible name for the > > earliest SoC with this timer. > > > > As this binding is specific to the TI dual-mode timers also known > > as dm-timers, let's use ti,timer-dm.yaml naming for the new file. > > > > Cc: Daniel Lezcano <daniel.lezcano@linaro.org> > > Cc: Keerthy <j-keerthy@ti.com> > > Cc: Nishanth Menon <nm@ti.com> > > Cc: Vignesh Raghavendra <vigneshr@ti.com> > > Signed-off-by: Tony Lindgren <tony@atomide.com> > > --- > > .../bindings/timer/ti,timer-dm.yaml | 105 ++++++++++++++++++ > > .../devicetree/bindings/timer/ti,timer.txt | 44 -------- > > 2 files changed, 105 insertions(+), 44 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/timer/ti,timer-dm.yaml > > delete mode 100644 Documentation/devicetree/bindings/timer/ti,timer.txt > > > > Running 'make dtbs_check' with the schema in this patch gives the > following warnings. Consider if they are expected or the schema is > incorrect. These may not be new warnings. Well I was planning to drop the unused ti,am4372-timer related properties, but instead will document those as discussed in the related dts thread. I'll be sending an updated version a bit later on. Regards, Tony
diff --git a/Documentation/devicetree/bindings/timer/ti,timer-dm.yaml b/Documentation/devicetree/bindings/timer/ti,timer-dm.yaml new file mode 100644 --- /dev/null +++ b/Documentation/devicetree/bindings/timer/ti,timer-dm.yaml @@ -0,0 +1,105 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/timer/ti,timer-dm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Binding for TI dual-mode timer + +maintainers: + - Tony Lindgren <tony@atomide.com> + +description: | + The TI dual-mode timer is a general purpose timer with PWM capabilities. + +properties: + compatible: + enum: + - ti,omap2420-timer + - ti,omap3430-timer + - ti,omap4430-timer + - ti,omap5430-timer + - ti,am335x-timer + - ti,am335x-timer-1ms + - ti,am654-timer + + reg: + minItems: 1 + maxItems: 2 + description: Timer IO register range + + '#address-cells': + enum: [ 1, 2 ] + + '#size-cells': + enum: [ 1, 2 ] + + clocks: + description: + The functional clock for the timer. Some SoCs like omap24xx also have a + separate interface clock, and some clocks may be only defined for the + interconnect target module parent. + minItems: 1 + maxItems: 2 + + clock-names: + description: + Timer clock names like "fck", "timer_sys_ck". + oneOf: + - enum: [ ick, fck ] + - items: + - const: fck + - enum: [ ick, timer_sys_ck ] + + interrupts: + description: + Interrupt if available. The timer PWM features may be usable + in a limited way even without interrupts. + maxItems: 1 + + ti,timer-alwon: + description: + Timer is always enabled when the SoC is powered. Note that some SoCs like + am335x can suspend to PM coprocessor RTC only mode and in that case the + SoC power is cut including timers. + type: boolean + + ti,timer-dsp: + description: + Timer is routable to the DSP in addition to the operating system. + type: boolean + + ti,timer-pwm: + description: + Timer has been wired for PWM capability. + type: boolean + + ti,timer-secure: + description: + Timer access has been limited to secure mode only. + type: boolean + + ti,hwmods: + description: + Name of the HWMOD associated with timer. This is for legacy + omap2/3 platforms only. + $ref: /schemas/types.yaml#/definitions/string + deprecated: true + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + timer1: timer@0 { + compatible = "ti,am335x-timer-1ms"; + reg = <0x0 0x400>; + interrupts = <67>; + ti,timer-alwon; + clocks = <&timer1_fck>; + clock-names = "fck"; + }; +... diff --git a/Documentation/devicetree/bindings/timer/ti,timer.txt b/Documentation/devicetree/bindings/timer/ti,timer.txt deleted file mode 100644 --- a/Documentation/devicetree/bindings/timer/ti,timer.txt +++ /dev/null @@ -1,44 +0,0 @@ -OMAP Timer bindings - -Required properties: -- compatible: Should be set to one of the below. Please note that - OMAP44xx devices have timer instances that are 100% - register compatible with OMAP3xxx devices as well as - newer timers that are not 100% register compatible. - So for OMAP44xx devices timer instances may use - different compatible strings. - - ti,omap2420-timer (applicable to OMAP24xx devices) - ti,omap3430-timer (applicable to OMAP3xxx/44xx devices) - ti,omap4430-timer (applicable to OMAP44xx devices) - ti,omap5430-timer (applicable to OMAP543x devices) - ti,am335x-timer (applicable to AM335x devices) - ti,am335x-timer-1ms (applicable to AM335x devices) - -- reg: Contains timer register address range (base address and - length). -- interrupts: Contains the interrupt information for the timer. The - format is being dependent on which interrupt controller - the OMAP device uses. -- ti,hwmods: Name of the hwmod associated to the timer, "timer<X>", - where <X> is the instance number of the timer from the - HW spec. - -Optional properties: -- ti,timer-alwon: Indicates the timer is in an alway-on power domain. -- ti,timer-dsp: Indicates the timer can interrupt the on-chip DSP in - addition to the ARM CPU. -- ti,timer-pwm: Indicates the timer can generate a PWM output. -- ti,timer-secure: Indicates the timer is reserved on a secure OMAP device - and therefore cannot be used by the kernel. - -Example: - -timer12: timer@48304000 { - compatible = "ti,omap3430-timer"; - reg = <0x48304000 0x400>; - interrupts = <95>; - ti,hwmods = "timer12" - ti,timer-alwon; - ti,timer-secure; -};
Let's update the TI timer binding to use yaml. And add compatible for ti,am654-timer for TI am64, am65 and j72 SoCs. As the timer hardware is the same between am64, am65 and j72 we use the compatible name for the earliest SoC with this timer. As this binding is specific to the TI dual-mode timers also known as dm-timers, let's use ti,timer-dm.yaml naming for the new file. Cc: Daniel Lezcano <daniel.lezcano@linaro.org> Cc: Keerthy <j-keerthy@ti.com> Cc: Nishanth Menon <nm@ti.com> Cc: Vignesh Raghavendra <vigneshr@ti.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- .../bindings/timer/ti,timer-dm.yaml | 105 ++++++++++++++++++ .../devicetree/bindings/timer/ti,timer.txt | 44 -------- 2 files changed, 105 insertions(+), 44 deletions(-) create mode 100644 Documentation/devicetree/bindings/timer/ti,timer-dm.yaml delete mode 100644 Documentation/devicetree/bindings/timer/ti,timer.txt