| Message ID | 20240919122759.10456-1-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: watchdog: airoha: document watchdog for Airoha EN7581 | expand |
On 19/09/2024 14:26, Christian Marangi wrote: > Document watchdog for Airoha EN7581. This SoC implement a simple > watchdog that supports a max timeout of 28 seconds. > > The watchdog ticks on half the BUS clock and require the BUS frequency > to be provided. Clock provider should implement clk_get_rate()... > ... > +maintainers: > + - Christian Marangi <ansuelsmth@gmail.com> > + > +allOf: > + - $ref: watchdog.yaml# > + > +properties: > + compatible: > + const: airoha,en7581-wdt > + > + reg: > + maxItems: 1 > + > + clock-frequency: > + description: BUS frequency in Hz (timer ticks at half the BUS freq) > + const: 300000000 Which bus frequency? Aren't you missing here clock input? Best regards, Krzysztof
On Thu, Sep 19, 2024 at 02:35:02PM +0200, Krzysztof Kozlowski wrote: > On 19/09/2024 14:26, Christian Marangi wrote: > > Document watchdog for Airoha EN7581. This SoC implement a simple > > watchdog that supports a max timeout of 28 seconds. > > > > The watchdog ticks on half the BUS clock and require the BUS frequency > > to be provided. > > Clock provider should implement clk_get_rate()... > The BUS clock is internal and not exposed to the system hence clk_get_rate is not possible saddly. > > > > ... > > > +maintainers: > > + - Christian Marangi <ansuelsmth@gmail.com> > > + > > +allOf: > > + - $ref: watchdog.yaml# > > + > > +properties: > > + compatible: > > + const: airoha,en7581-wdt > > + > > + reg: > > + maxItems: 1 > > + > > + clock-frequency: > > + description: BUS frequency in Hz (timer ticks at half the BUS freq) > > + const: 300000000 > > Which bus frequency? Aren't you missing here clock input? I'm putting here property to describe the internal clock to what the watchdog is attached. Should I drop this and just hardcode it internally to the driver or maybe declare the clock to be 150000000 directly? Tick frequency is already not well defined so I tought it was a good idea to describe it in DT.
On 19/09/2024 14:39, Christian Marangi wrote: > On Thu, Sep 19, 2024 at 02:35:02PM +0200, Krzysztof Kozlowski wrote: >> On 19/09/2024 14:26, Christian Marangi wrote: >>> Document watchdog for Airoha EN7581. This SoC implement a simple >>> watchdog that supports a max timeout of 28 seconds. >>> >>> The watchdog ticks on half the BUS clock and require the BUS frequency >>> to be provided. >> >> Clock provider should implement clk_get_rate()... >> > > The BUS clock is internal and not exposed to the system hence > clk_get_rate is not possible saddly. > >>> >> >> ... >> >>> +maintainers: >>> + - Christian Marangi <ansuelsmth@gmail.com> >>> + >>> +allOf: >>> + - $ref: watchdog.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: airoha,en7581-wdt >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clock-frequency: >>> + description: BUS frequency in Hz (timer ticks at half the BUS freq) >>> + const: 300000000 >> >> Which bus frequency? Aren't you missing here clock input? > > I'm putting here property to describe the internal clock to what the > watchdog is attached. Should I drop this and just hardcode it > internally to the driver or maybe declare the clock to be 150000000 > directly? If this stays, then please mention "internal watchdog bus frequency". If this is internal and it is part of an SoC (so not board!) why would we need it in DT? I would imagine this is fixed per SoC, thus deduced from the compatible. clock-frequency property is legacy and in general discouraged. This might be an exception, but for that I would like to see more of explanations. > > Tick frequency is already not well defined so I tought it was a good > idea to describe it in DT. > Best regards, Krzysztof
On 9/19/24 05:39, Christian Marangi wrote: > On Thu, Sep 19, 2024 at 02:35:02PM +0200, Krzysztof Kozlowski wrote: >> On 19/09/2024 14:26, Christian Marangi wrote: >>> Document watchdog for Airoha EN7581. This SoC implement a simple >>> watchdog that supports a max timeout of 28 seconds. >>> >>> The watchdog ticks on half the BUS clock and require the BUS frequency >>> to be provided. >> >> Clock provider should implement clk_get_rate()... >> > > The BUS clock is internal and not exposed to the system hence > clk_get_rate is not possible saddly. > >>> >> >> ... >> >>> +maintainers: >>> + - Christian Marangi <ansuelsmth@gmail.com> >>> + >>> +allOf: >>> + - $ref: watchdog.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: airoha,en7581-wdt >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clock-frequency: >>> + description: BUS frequency in Hz (timer ticks at half the BUS freq) >>> + const: 300000000 >> >> Which bus frequency? Aren't you missing here clock input? > > I'm putting here property to describe the internal clock to what the > watchdog is attached. Should I drop this and just hardcode it > internally to the driver or maybe declare the clock to be 150000000 > directly? > > Tick frequency is already not well defined so I tought it was a good > idea to describe it in DT. > If the value is a constant it should not be in devicetree. Guenter
On Thu, Sep 19, 2024 at 02:42:33PM +0200, Krzysztof Kozlowski wrote: > On 19/09/2024 14:39, Christian Marangi wrote: > > On Thu, Sep 19, 2024 at 02:35:02PM +0200, Krzysztof Kozlowski wrote: > >> On 19/09/2024 14:26, Christian Marangi wrote: > >>> Document watchdog for Airoha EN7581. This SoC implement a simple > >>> watchdog that supports a max timeout of 28 seconds. > >>> > >>> The watchdog ticks on half the BUS clock and require the BUS frequency > >>> to be provided. > >> > >> Clock provider should implement clk_get_rate()... > >> > > > > The BUS clock is internal and not exposed to the system hence > > clk_get_rate is not possible saddly. > > > >>> > >> > >> ... > >> > >>> +maintainers: > >>> + - Christian Marangi <ansuelsmth@gmail.com> > >>> + > >>> +allOf: > >>> + - $ref: watchdog.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + const: airoha,en7581-wdt > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + clock-frequency: > >>> + description: BUS frequency in Hz (timer ticks at half the BUS freq) > >>> + const: 300000000 > >> > >> Which bus frequency? Aren't you missing here clock input? > > > > I'm putting here property to describe the internal clock to what the > > watchdog is attached. Should I drop this and just hardcode it > > internally to the driver or maybe declare the clock to be 150000000 > > directly? > > If this stays, then please mention "internal watchdog bus frequency". > > If this is internal and it is part of an SoC (so not board!) why would > we need it in DT? I would imagine this is fixed per SoC, thus deduced > from the compatible. > > clock-frequency property is legacy and in general discouraged. This > might be an exception, but for that I would like to see more of > explanations. > Ok it took a while but finally I got my answer. The Documentation had a mistake and conflicting info. (one bus was said running at 250Mhz instead of 300Mhz) With this error fixed I can correctly attach a clock and drop this stupiud thing. Win-Win for everyone! > > > > Tick frequency is already not well defined so I tought it was a good > > idea to describe it in DT. > > >
diff --git a/Documentation/devicetree/bindings/watchdog/airoha,en7581-wdt.yaml b/Documentation/devicetree/bindings/watchdog/airoha,en7581-wdt.yaml new file mode 100644 index 000000000000..47210a5990ee --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/airoha,en7581-wdt.yaml @@ -0,0 +1,39 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/watchdog/airoha,en7581-wdt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Airoha EN7581 Watchdog Timer + +maintainers: + - Christian Marangi <ansuelsmth@gmail.com> + +allOf: + - $ref: watchdog.yaml# + +properties: + compatible: + const: airoha,en7581-wdt + + reg: + maxItems: 1 + + clock-frequency: + description: BUS frequency in Hz (timer ticks at half the BUS freq) + const: 300000000 + +required: + - compatible + - reg + - clock-frequency + +unevaluatedProperties: false + +examples: + - | + watchdog@1fbf0100 { + compatible = "airoha,en7581-wdt"; + reg = <0x1fbf0100 0x3c>; + clock-frequency = <300000000>; + };
Document watchdog for Airoha EN7581. This SoC implement a simple watchdog that supports a max timeout of 28 seconds. The watchdog ticks on half the BUS clock and require the BUS frequency to be provided. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- .../bindings/watchdog/airoha,en7581-wdt.yaml | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/airoha,en7581-wdt.yaml