| Message ID | 20210805045716.46141-1-samuel@sholland.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2,1/3] dt-bindings: watchdog: sunxi: Add compatible for R329 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | fail | build log |
On Wed, Aug 04, 2021 at 11:57:14PM -0500, Samuel Holland wrote: > On existing SoCs, the watchdog has a single clock input: HOSC (OSC24M) > divided by 750. However, starting with R329, LOSC (OSC32k) is added as > an alternative clock source, with a bit to switch between them. > > Since 24 MHz / 750 == 32 kHz, not 32.768 kHz, the hardware adjusts the > cycle counts to keep the timeouts independent of the clock source. This > keeps the programming interface backward-compatible. > > R329 also adds two new registers, to allow software to immediately drive > the SoC reset signal. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > Changes v1 to v2: > - Switch from enum to const > - Add descriptions to "clocks" items > > .../watchdog/allwinner,sun4i-a10-wdt.yaml | 32 ++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml > index 9aa3c313c49f..8fa5b0d32381 100644 > --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml > @@ -24,6 +24,7 @@ properties: > - allwinner,sun50i-a100-wdt > - allwinner,sun50i-h6-wdt > - allwinner,sun50i-h616-wdt > + - allwinner,sun50i-r329-wdt > - const: allwinner,sun6i-a31-wdt > - items: > - const: allwinner,suniv-f1c100s-wdt > @@ -33,7 +34,18 @@ properties: > maxItems: 1 > > clocks: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 > + items: > + - description: High-frequency oscillator input, divided internally > + - description: Low-frequency oscillator input, only found on some variants > + > + clock-names: > + minItems: 1 > + maxItems: 2 > + items: > + - const: hosc > + - const: losc > > interrupts: > maxItems: 1 > @@ -44,6 +56,24 @@ required: > - clocks > - interrupts > > +if: > + properties: > + compatible: > + contains: > + enum: > + - allwinner,sun50i-r329-wdt > + > +then: > + properties: > + clocks: > + minItems: 2 > + > + clock-names: > + minItems: 2 > + > + required: > + - clock-names Needs an else clause with 'maxItems: 1' or is the 2nd clock now valid for all? > + > unevaluatedProperties: false > > examples: > -- > 2.31.1 > >
Hi, On Wed, Aug 04, 2021 at 11:57:14PM -0500, Samuel Holland wrote: > On existing SoCs, the watchdog has a single clock input: HOSC (OSC24M) > divided by 750. However, starting with R329, LOSC (OSC32k) is added as > an alternative clock source, with a bit to switch between them. > > Since 24 MHz / 750 == 32 kHz, not 32.768 kHz, the hardware adjusts the > cycle counts to keep the timeouts independent of the clock source. This > keeps the programming interface backward-compatible. > > R329 also adds two new registers, to allow software to immediately drive > the SoC reset signal. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > Changes v1 to v2: > - Switch from enum to const > - Add descriptions to "clocks" items > > .../watchdog/allwinner,sun4i-a10-wdt.yaml | 32 ++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml > index 9aa3c313c49f..8fa5b0d32381 100644 > --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml > +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml > @@ -24,6 +24,7 @@ properties: > - allwinner,sun50i-a100-wdt > - allwinner,sun50i-h6-wdt > - allwinner,sun50i-h616-wdt > + - allwinner,sun50i-r329-wdt > - const: allwinner,sun6i-a31-wdt > - items: > - const: allwinner,suniv-f1c100s-wdt > @@ -33,7 +34,18 @@ properties: > maxItems: 1 > > clocks: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 > + items: > + - description: High-frequency oscillator input, divided internally > + - description: Low-frequency oscillator input, only found on some variants > + > + clock-names: > + minItems: 1 > + maxItems: 2 > + items: > + - const: hosc > + - const: losc > > interrupts: > maxItems: 1 > @@ -44,6 +56,24 @@ required: > - clocks > - interrupts > > +if: > + properties: > + compatible: > + contains: > + enum: > + - allwinner,sun50i-r329-wdt > + > +then: > + properties: > + clocks: > + minItems: 2 > + > + clock-names: > + minItems: 2 > + > + required: > + - clock-names > + We should also make sure through an else clause that all the other SoCs have a length of one for both clocks and clock-names Also, it looks like you forgot the patch to enable the compatible in the driver? Maxime
On Wed, Aug 04, 2021 at 11:57:16PM -0500, Samuel Holland wrote: > D1 adds a key field to the "CFG" and "MODE" registers, that must be set > to change the other bits. Add logic to set the key when updating those > registers. > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Samuel Holland <samuel@sholland.org> Acked-by: Maxime Ripard <maxime@cerno.tech> Maxime
On 8/18/21 4:07 AM, Maxime Ripard wrote: > Hi, > > On Wed, Aug 04, 2021 at 11:57:14PM -0500, Samuel Holland wrote: >> On existing SoCs, the watchdog has a single clock input: HOSC (OSC24M) >> divided by 750. However, starting with R329, LOSC (OSC32k) is added as >> an alternative clock source, with a bit to switch between them. >> >> Since 24 MHz / 750 == 32 kHz, not 32.768 kHz, the hardware adjusts the >> cycle counts to keep the timeouts independent of the clock source. This >> keeps the programming interface backward-compatible. >> >> R329 also adds two new registers, to allow software to immediately drive >> the SoC reset signal. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> Changes v1 to v2: >> - Switch from enum to const >> - Add descriptions to "clocks" items >> >> .../watchdog/allwinner,sun4i-a10-wdt.yaml | 32 ++++++++++++++++++- >> 1 file changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml >> index 9aa3c313c49f..8fa5b0d32381 100644 >> --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml >> +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml >> @@ -24,6 +24,7 @@ properties: >> - allwinner,sun50i-a100-wdt >> - allwinner,sun50i-h6-wdt >> - allwinner,sun50i-h616-wdt >> + - allwinner,sun50i-r329-wdt >> - const: allwinner,sun6i-a31-wdt >> - items: >> - const: allwinner,suniv-f1c100s-wdt >> @@ -33,7 +34,18 @@ properties: >> maxItems: 1 >> >> clocks: >> - maxItems: 1 >> + minItems: 1 >> + maxItems: 2 >> + items: >> + - description: High-frequency oscillator input, divided internally >> + - description: Low-frequency oscillator input, only found on some variants >> + >> + clock-names: >> + minItems: 1 >> + maxItems: 2 >> + items: >> + - const: hosc >> + - const: losc >> >> interrupts: >> maxItems: 1 >> @@ -44,6 +56,24 @@ required: >> - clocks >> - interrupts >> >> +if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - allwinner,sun50i-r329-wdt >> + >> +then: >> + properties: >> + clocks: >> + minItems: 2 >> + >> + clock-names: >> + minItems: 2 >> + >> + required: >> + - clock-names >> + > > We should also make sure through an else clause that all the other SoCs > have a length of one for both clocks and clock-names Yes, I'll do that for v3. > Also, it looks like you forgot the patch to enable the compatible in the > driver? R329 has a fallback to A31, so it doesn't need any changes to the driver. Regards, Samuel
diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml index 9aa3c313c49f..8fa5b0d32381 100644 --- a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml +++ b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml @@ -24,6 +24,7 @@ properties: - allwinner,sun50i-a100-wdt - allwinner,sun50i-h6-wdt - allwinner,sun50i-h616-wdt + - allwinner,sun50i-r329-wdt - const: allwinner,sun6i-a31-wdt - items: - const: allwinner,suniv-f1c100s-wdt @@ -33,7 +34,18 @@ properties: maxItems: 1 clocks: - maxItems: 1 + minItems: 1 + maxItems: 2 + items: + - description: High-frequency oscillator input, divided internally + - description: Low-frequency oscillator input, only found on some variants + + clock-names: + minItems: 1 + maxItems: 2 + items: + - const: hosc + - const: losc interrupts: maxItems: 1 @@ -44,6 +56,24 @@ required: - clocks - interrupts +if: + properties: + compatible: + contains: + enum: + - allwinner,sun50i-r329-wdt + +then: + properties: + clocks: + minItems: 2 + + clock-names: + minItems: 2 + + required: + - clock-names + unevaluatedProperties: false examples:
On existing SoCs, the watchdog has a single clock input: HOSC (OSC24M) divided by 750. However, starting with R329, LOSC (OSC32k) is added as an alternative clock source, with a bit to switch between them. Since 24 MHz / 750 == 32 kHz, not 32.768 kHz, the hardware adjusts the cycle counts to keep the timeouts independent of the clock source. This keeps the programming interface backward-compatible. R329 also adds two new registers, to allow software to immediately drive the SoC reset signal. Signed-off-by: Samuel Holland <samuel@sholland.org> --- Changes v1 to v2: - Switch from enum to const - Add descriptions to "clocks" items .../watchdog/allwinner,sun4i-a10-wdt.yaml | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)