| Message ID | 20250328151516.2219971-1-daniel.lezcano@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: watchdog: Add NXP Software Watchdog Timer | expand |
On Fri, Mar 28, 2025 at 08:37:03PM -0500, Rob Herring (Arm) wrote: > > On Fri, 28 Mar 2025 16:15:13 +0100, Daniel Lezcano wrote: > > Describe the Software Watchdog Timer available on the S32G platforms. > > > > Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > > Cc: Thomas Fossati <thomas.fossati@linaro.org> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > --- > > .../bindings/watchdog/nxp,s32g-wdt.yaml | 46 +++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.example.dts:18.29-23.11: Warning (unit_address_format): /example-0/watchdog@0x40100000: unit name should not have leading "0x" > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.example.dtb: watchdog@0x40100000: 'timeout-sec' does not match any of the regexes: 'pinctrl-[0-9]+' > from schema $id: http://devicetree.org/schemas/watchdog/nxp,s32g-wdt.yaml# You need unevaluatedProperties instead of additionalProperties. Rob
On 29/03/2025 06:04, Krzysztof Kozlowski wrote: > On 28/03/2025 16:15, Daniel Lezcano wrote: >> +description: >> + The System Timer Module supports commonly required system and >> + application software timing functions. STM includes a 32-bit >> + count-up timer and four 32-bit compare channels with a separate >> + interrupt source for each channel. The timer is driven by the STM >> + >> +allOf: >> + - $ref: watchdog.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - nxp,s32g-wdt > This wasn't tested, so limited review - this also has wrong compatible, > There is no such soc as s32g in the kernel. If that's a new soc, come > with proper top-level bindings and some explanation, because I am sure > previously we talked with NXP that this is not s32g but something else. If I refer to Documentation/devicetree/bindings/arm/fsl.yaml We found the entries: - description: S32G2 based Boards items: - enum: - nxp,s32g274a-evb - nxp,s32g274a-rdb2 - const: nxp,s32g2 - description: S32G3 based Boards items: - enum: - nxp,s32g399a-rdb3 - const: nxp,s32g3 I guess it should nxp,s32g2-wdt and nxp,s32g3-wdt Right ?
On 29/03/2025 18:12, Rob Herring wrote: > On Fri, Mar 28, 2025 at 08:37:03PM -0500, Rob Herring (Arm) wrote: >> >> On Fri, 28 Mar 2025 16:15:13 +0100, Daniel Lezcano wrote: >>> Describe the Software Watchdog Timer available on the S32G platforms. >>> >>> Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> >>> Cc: Thomas Fossati <thomas.fossati@linaro.org> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >>> --- >>> .../bindings/watchdog/nxp,s32g-wdt.yaml | 46 +++++++++++++++++++ >>> 1 file changed, 46 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml >>> >> >> My bot found errors running 'make dt_binding_check' on your patch: >> >> yamllint warnings/errors: >> >> dtschema/dtc warnings/errors: >> Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.example.dts:18.29-23.11: Warning (unit_address_format): /example-0/watchdog@0x40100000: unit name should not have leading "0x" >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.example.dtb: watchdog@0x40100000: 'timeout-sec' does not match any of the regexes: 'pinctrl-[0-9]+' >> from schema $id: http://devicetree.org/schemas/watchdog/nxp,s32g-wdt.yaml# > > You need unevaluatedProperties instead of additionalProperties. Thanks!
On 3/28/2025 5:15 PM, Daniel Lezcano wrote: > Describe the Software Watchdog Timer available on the S32G platforms. > > Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > Cc: Thomas Fossati <thomas.fossati@linaro.org> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > .../bindings/watchdog/nxp,s32g-wdt.yaml | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml > > diff --git a/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml b/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml > new file mode 100644 > index 000000000000..06ead743d5c1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml > @@ -0,0 +1,46 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/watchdog/nxp,s32g-wdt.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP Software Watchdog Timer > + > +maintainers: > + - Daniel Lezcano <daniel.lezcano@kernel.org> > + > +description: > + The System Timer Module supports commonly required system and > + application software timing functions. STM includes a 32-bit > + count-up timer and four 32-bit compare channels with a separate > + interrupt source for each channel. The timer is driven by the STM > + Please update the description, as this one refers to STM instead of SWT.
On 31/03/2025 09:57, Daniel Lezcano wrote: > On 29/03/2025 06:04, Krzysztof Kozlowski wrote: >> On 28/03/2025 16:15, Daniel Lezcano wrote: >>> +description: >>> + The System Timer Module supports commonly required system and >>> + application software timing functions. STM includes a 32-bit >>> + count-up timer and four 32-bit compare channels with a separate >>> + interrupt source for each channel. The timer is driven by the STM >>> + >>> +allOf: >>> + - $ref: watchdog.yaml# >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - nxp,s32g-wdt >> This wasn't tested, so limited review - this also has wrong compatible, >> There is no such soc as s32g in the kernel. If that's a new soc, come >> with proper top-level bindings and some explanation, because I am sure >> previously we talked with NXP that this is not s32g but something else. > > If I refer to Documentation/devicetree/bindings/arm/fsl.yaml > > We found the entries: > > - description: S32G2 based Boards > items: > - enum: > - nxp,s32g274a-evb > - nxp,s32g274a-rdb2 > - const: nxp,s32g2 > > - description: S32G3 based Boards > items: > - enum: > - nxp,s32g399a-rdb3 > - const: nxp,s32g3 > > I guess it should nxp,s32g2-wdt and nxp,s32g3-wdt > Yes, with one being the fallback. Best regards, Krzysztof
On 31/03/2025 13:42, Krzysztof Kozlowski wrote: > On 31/03/2025 09:57, Daniel Lezcano wrote: >> On 29/03/2025 06:04, Krzysztof Kozlowski wrote: >>> On 28/03/2025 16:15, Daniel Lezcano wrote: >>>> +description: >>>> + The System Timer Module supports commonly required system and >>>> + application software timing functions. STM includes a 32-bit >>>> + count-up timer and four 32-bit compare channels with a separate >>>> + interrupt source for each channel. The timer is driven by the STM >>>> + >>>> +allOf: >>>> + - $ref: watchdog.yaml# >>>> + >>>> +properties: >>>> + compatible: >>>> + enum: >>>> + - nxp,s32g-wdt >>> This wasn't tested, so limited review - this also has wrong compatible, >>> There is no such soc as s32g in the kernel. If that's a new soc, come >>> with proper top-level bindings and some explanation, because I am sure >>> previously we talked with NXP that this is not s32g but something else. >> >> If I refer to Documentation/devicetree/bindings/arm/fsl.yaml >> >> We found the entries: >> >> - description: S32G2 based Boards >> items: >> - enum: >> - nxp,s32g274a-evb >> - nxp,s32g274a-rdb2 >> - const: nxp,s32g2 >> >> - description: S32G3 based Boards >> items: >> - enum: >> - nxp,s32g399a-rdb3 >> - const: nxp,s32g3 >> >> I guess it should nxp,s32g2-wdt and nxp,s32g3-wdt >> > Yes, with one being the fallback. Like that ? properties: compatible: oneOf: - const: nxp,s32g2-wdt - items: - const: nxp,s32g3-wdt - const: nxp,s32g2-wdt Why a fallback is needed for this case ? It is exactly the same hardware for s32g2 and s32g3. Could it be: properties: compatible: oneOf: - const: nxp,s32g2-wdt - const: nxp,s32g3-wdt ?
On 01/04/2025 10:46, Daniel Lezcano wrote: >> Yes, with one being the fallback. > > Like that ? > > properties: > compatible: > oneOf: > - const: nxp,s32g2-wdt > - items: > - const: nxp,s32g3-wdt > - const: nxp,s32g2-wdt Yes > > Why a fallback is needed for this case ? It is exactly the same hardware > for s32g2 and s32g3. Could it be: Fallback is needed because it is exactly the same hardware. > > properties: > compatible: > oneOf: > - const: nxp,s32g2-wdt > - const: nxp,s32g3-wdt This tells hardware is different. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml b/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml new file mode 100644 index 000000000000..06ead743d5c1 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml @@ -0,0 +1,46 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/watchdog/nxp,s32g-wdt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP Software Watchdog Timer + +maintainers: + - Daniel Lezcano <daniel.lezcano@kernel.org> + +description: + The System Timer Module supports commonly required system and + application software timing functions. STM includes a 32-bit + count-up timer and four 32-bit compare channels with a separate + interrupt source for each channel. The timer is driven by the STM + +allOf: + - $ref: watchdog.yaml# + +properties: + compatible: + enum: + - nxp,s32g-wdt + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + watchdog@0x40100000 { + compatible = "nxp,s32g-wdt"; + reg = <0x40100000 0x1000>; + clocks = <&clks 0x3a>; + timeout-sec = <10>; + };
Describe the Software Watchdog Timer available on the S32G platforms. Cc: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> Cc: Thomas Fossati <thomas.fossati@linaro.org> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- .../bindings/watchdog/nxp,s32g-wdt.yaml | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/nxp,s32g-wdt.yaml