| Message ID | 20230424123522.18302-10-nikita.shubin@maquefel.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ep93xx device tree conversion | expand |
On Mon, Apr 24, 2023 at 03:34:25PM +0300, Nikita Shubin wrote: > This adds device tree bindings for the Cirrus Logic EP93xx > watchdog block used in these SoCs. > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > --- > .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 +++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > diff --git a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > new file mode 100644 > index 000000000000..f39d6b14062d > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > @@ -0,0 +1,38 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Cirrus Logic EP93xx Watchdog Timer > + > +maintainers: > + - Wim Van Sebroeck <wim@linux-watchdog.org> > + > +description: > + Watchdog driver for Cirrus Logic EP93xx family of devices. > + > +allOf: > + - $ref: "watchdog.yaml#" > + > +properties: > + compatible: > + enum: > + - cirrus,ep9301-wdt > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false The driver does support reading the timeout from devicetree. It might make sense to mention that here. > + > +examples: > + - | > + wdt0: watchdog@80940000 { > + compatible = "cirrus,ep9301-wdt"; > + reg = <0x80940000 0x08>; > + }; > + > -- > 2.39.2 >
On Mon, Apr 24, 2023 at 07:16:16AM -0700, Guenter Roeck wrote: > On Mon, Apr 24, 2023 at 03:34:25PM +0300, Nikita Shubin wrote: > > This adds device tree bindings for the Cirrus Logic EP93xx > > watchdog block used in these SoCs. > > > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > > --- > > .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 +++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > > > diff --git a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > new file mode 100644 > > index 000000000000..f39d6b14062d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > @@ -0,0 +1,38 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Cirrus Logic EP93xx Watchdog Timer > > + > > +maintainers: > > + - Wim Van Sebroeck <wim@linux-watchdog.org> > > + > > +description: > > + Watchdog driver for Cirrus Logic EP93xx family of devices. > > + > > +allOf: > > + - $ref: "watchdog.yaml#" > > + > > +properties: > > + compatible: > > + enum: > > + - cirrus,ep9301-wdt > > + > > + reg: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > The driver does support reading the timeout from devicetree. > It might make sense to mention that here. > Never mind - I guess that is includeds in watchdog.yaml. Sorry for the noise. > > + > > +examples: > > + - | > > + wdt0: watchdog@80940000 { > > + compatible = "cirrus,ep9301-wdt"; > > + reg = <0x80940000 0x08>; > > + }; > > + > > -- > > 2.39.2 > >
On Mon, Apr 24, 2023 at 07:18:06AM -0700, Guenter Roeck wrote: > On Mon, Apr 24, 2023 at 07:16:16AM -0700, Guenter Roeck wrote: > > On Mon, Apr 24, 2023 at 03:34:25PM +0300, Nikita Shubin wrote: > > > This adds device tree bindings for the Cirrus Logic EP93xx > > > watchdog block used in these SoCs. > > > > > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > > > --- > > > .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 +++++++++++++++++++ > > > 1 file changed, 38 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > > new file mode 100644 > > > index 000000000000..f39d6b14062d > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > > @@ -0,0 +1,38 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Cirrus Logic EP93xx Watchdog Timer > > > + > > > +maintainers: > > > + - Wim Van Sebroeck <wim@linux-watchdog.org> > > > + > > > +description: > > > + Watchdog driver for Cirrus Logic EP93xx family of devices. > > > + > > > +allOf: > > > + - $ref: "watchdog.yaml#" > > > + > > > +properties: > > > + compatible: > > > + enum: > > > + - cirrus,ep9301-wdt > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + > > > +additionalProperties: false > > > > The driver does support reading the timeout from devicetree. > > It might make sense to mention that here. > > > Never mind - I guess that is includeds in watchdog.yaml. > Sorry for the noise. Except that it needs to be 'unevaluatedProperties: false' instead if timeout property is supported. > > > > + > > > +examples: > > > + - | > > > + wdt0: watchdog@80940000 { > > > + compatible = "cirrus,ep9301-wdt"; > > > + reg = <0x80940000 0x08>; > > > + }; > > > + > > > -- > > > 2.39.2 > > >
On 24/04/2023 14:34, Nikita Shubin wrote: > This adds device tree bindings for the Cirrus Logic EP93xx > watchdog block used in these SoCs. > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > --- > .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 +++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > diff --git a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > new file mode 100644 > index 000000000000..f39d6b14062d > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > @@ -0,0 +1,38 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Cirrus Logic EP93xx Watchdog Timer EP93xx is no EP9301. This does not match your compatible list. You should probably list all of your devices. With or without compatibility between them (so with a generic fallback for example). > + > +maintainers: > + - Wim Van Sebroeck <wim@linux-watchdog.org> > + > +description: > + Watchdog driver for Cirrus Logic EP93xx family of devices. Drop "driver for" and instead describe the hardware. > + > +allOf: > + - $ref: "watchdog.yaml#" Drop quotes. Best regards, Krzysztof
On 28/04/2023 16:33, Nikita Shubin wrote: > Hello Krzysztof! > > On Tue, 2023-04-25 at 11:31 +0200, Krzysztof Kozlowski wrote: >> On 24/04/2023 14:34, Nikita Shubin wrote: >>> This adds device tree bindings for the Cirrus Logic EP93xx >>> watchdog block used in these SoCs. >>> >>> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> >>> --- >>> .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 >>> +++++++++++++++++++ >>> 1 file changed, 38 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml >>> b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml >>> new file mode 100644 >>> index 000000000000..f39d6b14062d >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- >>> wdt.yaml >>> @@ -0,0 +1,38 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: >>> http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Cirrus Logic EP93xx Watchdog Timer >> >> EP93xx is no EP9301. This does not match your compatible list. You >> should probably list all of your devices. With or without >> compatibility >> between them (so with a generic fallback for example). > > I will rename file to cirrus,ep9301-wdt.yaml, all ep93xx SoC family has > the same watchdog, so there is now reason for other compatible i think. You should always have dedicated compatibles, even if using one fallback. https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 Best regards, Krzysztof
Hello Krzysztof! On Tue, 2023-04-25 at 11:31 +0200, Krzysztof Kozlowski wrote: > On 24/04/2023 14:34, Nikita Shubin wrote: > > This adds device tree bindings for the Cirrus Logic EP93xx > > watchdog block used in these SoCs. > > > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > > --- > > .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 > > +++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml > > new file mode 100644 > > index 000000000000..f39d6b14062d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- > > wdt.yaml > > @@ -0,0 +1,38 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Cirrus Logic EP93xx Watchdog Timer > > EP93xx is no EP9301. This does not match your compatible list. You > should probably list all of your devices. With or without > compatibility > between them (so with a generic fallback for example). I will rename file to cirrus,ep9301-wdt.yaml, all ep93xx SoC family has the same watchdog, so there is now reason for other compatible i think. > > > + > > +maintainers: > > + - Wim Van Sebroeck <wim@linux-watchdog.org> > > + > > +description: > > + Watchdog driver for Cirrus Logic EP93xx family of devices. > > Drop "driver for" and instead describe the hardware. > > > + > > +allOf: > > + - $ref: "watchdog.yaml#" > > Drop quotes. > Best regards, > Krzysztof >
On Fri, 2023-04-28 at 14:20 +0200, Krzysztof Kozlowski wrote: > On 28/04/2023 16:33, Nikita Shubin wrote: > > Hello Krzysztof! > > > > On Tue, 2023-04-25 at 11:31 +0200, Krzysztof Kozlowski wrote: > > > On 24/04/2023 14:34, Nikita Shubin wrote: > > > > This adds device tree bindings for the Cirrus Logic EP93xx > > > > watchdog block used in these SoCs. > > > > > > > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > > > > --- > > > > .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 > > > > +++++++++++++++++++ > > > > 1 file changed, 38 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- > > > > wdt.yaml > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- > > > > wdt.yaml > > > > b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- > > > > wdt.yaml > > > > new file mode 100644 > > > > index 000000000000..f39d6b14062d > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- > > > > wdt.yaml > > > > @@ -0,0 +1,38 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: > > > > http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Cirrus Logic EP93xx Watchdog Timer > > > > > > EP93xx is no EP9301. This does not match your compatible list. > > > You > > > should probably list all of your devices. With or without > > > compatibility > > > between them (so with a generic fallback for example). > > > > I will rename file to cirrus,ep9301-wdt.yaml, all ep93xx SoC family > > has > > the same watchdog, so there is now reason for other compatible i > > think. > > You should always have dedicated compatibles, even if using one > fallback. > https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 Krzysztof, sorry to bother you - but i don't quite get, what we should have in compatibles ? Should i make an additional fallback compatible like "cirrus,ep-wdt" and then "compatible" will look like: properties: compatible: - items: - enum: - cirrus,ep9301-wdt - const: cirrus,ep-wdt Or should i describe every ep93xx SoC variant like: properties: compatible: - items: - enum: - cirrus,ep9302-wdt - cirrus,ep9307-wdt - cirrus,ep9312-wdt - cirrus,ep9315-wdt - const: cirrus,ep9301-wdt There are ep9301, ep9302, ep9307, ep9312 and ep9315 SoC variants - all have the same watchdog and rtc implementation without any difference at all. If on of this is true does the same applies to ep9301-rtc and any other variants where we do have a single compatible ? > > Best regards, > Krzysztof >
On 28/04/2023 19:42, Nikita Shubin wrote: > On Fri, 2023-04-28 at 14:20 +0200, Krzysztof Kozlowski wrote: >> On 28/04/2023 16:33, Nikita Shubin wrote: >>> Hello Krzysztof! >>> >>> On Tue, 2023-04-25 at 11:31 +0200, Krzysztof Kozlowski wrote: >>>> On 24/04/2023 14:34, Nikita Shubin wrote: >>>>> This adds device tree bindings for the Cirrus Logic EP93xx >>>>> watchdog block used in these SoCs. >>>>> >>>>> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> >>>>> --- >>>>> .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 >>>>> +++++++++++++++++++ >>>>> 1 file changed, 38 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- >>>>> wdt.yaml >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- >>>>> wdt.yaml >>>>> b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- >>>>> wdt.yaml >>>>> new file mode 100644 >>>>> index 000000000000..f39d6b14062d >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx- >>>>> wdt.yaml >>>>> @@ -0,0 +1,38 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: >>>>> http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>> + >>>>> +title: Cirrus Logic EP93xx Watchdog Timer >>>> >>>> EP93xx is no EP9301. This does not match your compatible list. >>>> You >>>> should probably list all of your devices. With or without >>>> compatibility >>>> between them (so with a generic fallback for example). >>> >>> I will rename file to cirrus,ep9301-wdt.yaml, all ep93xx SoC family >>> has >>> the same watchdog, so there is now reason for other compatible i >>> think. >> >> You should always have dedicated compatibles, even if using one >> fallback. >> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 > > Krzysztof, sorry to bother you - but i don't quite get, what we should > have in compatibles ? > > Should i make an additional fallback compatible like "cirrus,ep-wdt" > and then "compatible" will look like: > > properties: > compatible: > - items: > - enum: > - cirrus,ep9301-wdt > - const: cirrus,ep-wdt > > Or should i describe every ep93xx SoC variant like: > > properties: > compatible: > - items: > - enum: > - cirrus,ep9302-wdt > - cirrus,ep9307-wdt > - cirrus,ep9312-wdt > - cirrus,ep9315-wdt > - const: cirrus,ep9301-wdt This one is preferred. Just don't forget for an entry allowing 9301 alone (and everything within oneOf) Syntax looks like: https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31 > > There are ep9301, ep9302, ep9307, ep9312 and ep9315 SoC variants - all > have the same watchdog and rtc implementation without any difference at > all. We still prefer to have dedicated compatible, in case some bugs/differences are found. > > If on of this is true does the same applies to ep9301-rtc and any other > variants where we do have a single compatible ? Yes, please. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml new file mode 100644 index 000000000000..f39d6b14062d --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml @@ -0,0 +1,38 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/watchdog/cirrus,ep93xx-wdt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Cirrus Logic EP93xx Watchdog Timer + +maintainers: + - Wim Van Sebroeck <wim@linux-watchdog.org> + +description: + Watchdog driver for Cirrus Logic EP93xx family of devices. + +allOf: + - $ref: "watchdog.yaml#" + +properties: + compatible: + enum: + - cirrus,ep9301-wdt + + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + wdt0: watchdog@80940000 { + compatible = "cirrus,ep9301-wdt"; + reg = <0x80940000 0x08>; + }; +
This adds device tree bindings for the Cirrus Logic EP93xx watchdog block used in these SoCs. Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> --- .../bindings/watchdog/cirrus,ep93xx-wdt.yaml | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml