| Message ID | 20210726035143.53132-1-samuel@sholland.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [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 Sun, Jul 25, 2021 at 10:51:43PM -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. > > Signed-off-by: Samuel Holland <samuel@sholland.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/sunxi_wdt.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c > index b50757882a98..6cf82922d3fb 100644 > --- a/drivers/watchdog/sunxi_wdt.c > +++ b/drivers/watchdog/sunxi_wdt.c > @@ -48,6 +48,7 @@ struct sunxi_wdt_reg { > u8 wdt_timeout_shift; > u8 wdt_reset_mask; > u8 wdt_reset_val; > + u32 wdt_key_val; > }; > > struct sunxi_wdt_dev { > @@ -91,12 +92,14 @@ static int sunxi_wdt_restart(struct watchdog_device *wdt_dev, > val = readl(wdt_base + regs->wdt_cfg); > val &= ~(regs->wdt_reset_mask); > val |= regs->wdt_reset_val; > + val |= regs->wdt_key_val; > writel(val, wdt_base + regs->wdt_cfg); > > /* Set lowest timeout and enable watchdog */ > val = readl(wdt_base + regs->wdt_mode); > val &= ~(WDT_TIMEOUT_MASK << regs->wdt_timeout_shift); > val |= WDT_MODE_EN; > + val |= regs->wdt_key_val; > writel(val, wdt_base + regs->wdt_mode); > > /* > @@ -109,6 +112,7 @@ static int sunxi_wdt_restart(struct watchdog_device *wdt_dev, > mdelay(5); > val = readl(wdt_base + regs->wdt_mode); > val |= WDT_MODE_EN; > + val |= regs->wdt_key_val; > writel(val, wdt_base + regs->wdt_mode); > } > return 0; > @@ -141,6 +145,7 @@ static int sunxi_wdt_set_timeout(struct watchdog_device *wdt_dev, > reg = readl(wdt_base + regs->wdt_mode); > reg &= ~(WDT_TIMEOUT_MASK << regs->wdt_timeout_shift); > reg |= wdt_timeout_map[timeout] << regs->wdt_timeout_shift; > + reg |= regs->wdt_key_val; > writel(reg, wdt_base + regs->wdt_mode); > > sunxi_wdt_ping(wdt_dev); > @@ -154,7 +159,7 @@ static int sunxi_wdt_stop(struct watchdog_device *wdt_dev) > void __iomem *wdt_base = sunxi_wdt->wdt_base; > const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs; > > - writel(0, wdt_base + regs->wdt_mode); > + writel(regs->wdt_key_val, wdt_base + regs->wdt_mode); > > return 0; > } > @@ -176,11 +181,13 @@ static int sunxi_wdt_start(struct watchdog_device *wdt_dev) > reg = readl(wdt_base + regs->wdt_cfg); > reg &= ~(regs->wdt_reset_mask); > reg |= regs->wdt_reset_val; > + reg |= regs->wdt_key_val; > writel(reg, wdt_base + regs->wdt_cfg); > > /* Enable watchdog */ > reg = readl(wdt_base + regs->wdt_mode); > reg |= WDT_MODE_EN; > + reg |= regs->wdt_key_val; > writel(reg, wdt_base + regs->wdt_mode); > > return 0; > @@ -220,9 +227,20 @@ static const struct sunxi_wdt_reg sun6i_wdt_reg = { > .wdt_reset_val = 0x01, > }; > > +static const struct sunxi_wdt_reg sun20i_wdt_reg = { > + .wdt_ctrl = 0x10, > + .wdt_cfg = 0x14, > + .wdt_mode = 0x18, > + .wdt_timeout_shift = 4, > + .wdt_reset_mask = 0x03, > + .wdt_reset_val = 0x01, > + .wdt_key_val = 0x16aa0000, > +}; > + > static const struct of_device_id sunxi_wdt_dt_ids[] = { > { .compatible = "allwinner,sun4i-a10-wdt", .data = &sun4i_wdt_reg }, > { .compatible = "allwinner,sun6i-a31-wdt", .data = &sun6i_wdt_reg }, > + { .compatible = "allwinner,sun20i-d1-wdt", .data = &sun20i_wdt_reg }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, sunxi_wdt_dt_ids);
Hi, On Sun, Jul 25, 2021 at 10:51:41PM -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> > --- > .../watchdog/allwinner,sun4i-a10-wdt.yaml | 30 ++++++++++++++++++- > 1 file changed, 29 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..853ceb1b7c0f 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,16 @@ properties: > maxItems: 1 > > clocks: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 > + > + clock-names: > + minItems: 1 > + maxItems: 2 > + items: > + enum: > + - hosc > + - losc So I assume that the expectations are that hosc is first (or the only valid choice with older SoCs), and losc second? If so, we should make it clear using two const instead of an enum. Here you state that on older SoCs we could use either the hosc or losc clocks as source, which is incorrect. I guess adding descriptions for clocks would be great too. Maxime
diff --git a/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml b/Documentation/devicetree/bindings/watchdog/allwinner,sun4i-a10-wdt.yaml index 9aa3c313c49f..853ceb1b7c0f 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,16 @@ properties: maxItems: 1 clocks: - maxItems: 1 + minItems: 1 + maxItems: 2 + + clock-names: + minItems: 1 + maxItems: 2 + items: + enum: + - hosc + - losc interrupts: maxItems: 1 @@ -44,6 +54,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> --- .../watchdog/allwinner,sun4i-a10-wdt.yaml | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)