* [PATCH linux dev-4.10 v2 0/4] Witherspoon WDTRST1 enablement
@ 2017-07-26 16:08 Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 1/4] ARM: aspeed: Request WDTRST1 pinctrl function Andrew Jeffery
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 16:08 UTC (permalink / raw)
To: joel; +Cc: Andrew Jeffery, mspinler, msbarth, openbmc
Hello,
Earlier I sent a patch fixing the wdt1 pinctrl bindings so that we request the
WDTRST1 function:
https://lists.ozlabs.org/pipermail/openbmc/2017-July/008373.html
This series builds on top of that patch (and contains it, hence v2), adding
new bindings to further configure the pin's behaviour through the watchdog
controller itself. This is necessary for correct behaviour of the latch
asserting the MAX31785 FAULT pin across a BMC reset.
Cheers,
Andrew
Andrew Jeffery (4):
ARM: aspeed: Request WDTRST1 pinctrl function
dt-bindings: watchdog: aspeed: External reset signal properties
watchdog: aspeed: Support external signal drive and polarity bindings
ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull
.../devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 4 ++++
drivers/watchdog/aspeed_wdt.c | 21 +++++++++++++++++++++
3 files changed, 32 insertions(+)
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH linux dev-4.10 v2 1/4] ARM: aspeed: Request WDTRST1 pinctrl function
2017-07-26 16:08 [PATCH linux dev-4.10 v2 0/4] Witherspoon WDTRST1 enablement Andrew Jeffery
@ 2017-07-26 16:08 ` Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 2/4] dt-bindings: watchdog: aspeed: External reset signal properties Andrew Jeffery
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 16:08 UTC (permalink / raw)
To: joel; +Cc: Andrew Jeffery, mspinler, msbarth, openbmc
On Witherspoon, the Aspeed SoC's WDTRST1 pin is wired to the FAULT pin
on the MAX31785 fan controller, which drives the fans to 100% PWM duty
when asserted. The pulse generated by the watchdog is latched to ensure
the fans stay at full speed across the BMC reboot. The latch is reset
when the BMC transitions through the chassis-poweron systemd target.
The SoC's WDTRST1 pinctrl function needs to be requested for
aspeed,external-signal to be effective, otherwise the pin is not
associated with the watchdog controller.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Cc: Matt Spinler <mspinler@linux.vnet.ibm.com>
---
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index d9649013ee78..c28222c17d03 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -488,6 +488,9 @@
&wdt1 {
aspeed,reset-type = "none";
aspeed,external-signal;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_wdtrst1_default>;
};
&wdt2 {
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH linux dev-4.10 v2 2/4] dt-bindings: watchdog: aspeed: External reset signal properties
2017-07-26 16:08 [PATCH linux dev-4.10 v2 0/4] Witherspoon WDTRST1 enablement Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 1/4] ARM: aspeed: Request WDTRST1 pinctrl function Andrew Jeffery
@ 2017-07-26 16:08 ` Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull Andrew Jeffery
3 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 16:08 UTC (permalink / raw)
To: joel; +Cc: Andrew Jeffery, mspinler, msbarth, openbmc
For the AST2500 and compatible watchdog controllers the external reset
signal can be configured for push-pull or open-drain drive types, and in
the case of open-drain driving, active low or high.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
index f526b003b2e0..fa661319a080 100644
--- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
@@ -41,6 +41,13 @@ Optional properties:
specified no external signal is sent.
- aspeed,alt-boot: If property is present then boot from alternate block.
+Optional properties for AST2500-compatible watchdogs:
+ - aspeed,ext-push-pull: If aspeed,external-signal is present, set the pin's
+ drive type to push-pull. The default is open-drain.
+ - aspeed,ext-active-high: If aspeed,external-signal is present and and the pin
+ is configured as open-drain, then set the pulse
+ polarity to active-high. The default is active-low.
+
Example:
wdt1: watchdog@1e785000 {
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings
2017-07-26 16:08 [PATCH linux dev-4.10 v2 0/4] Witherspoon WDTRST1 enablement Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 1/4] ARM: aspeed: Request WDTRST1 pinctrl function Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 2/4] dt-bindings: watchdog: aspeed: External reset signal properties Andrew Jeffery
@ 2017-07-26 16:08 ` Andrew Jeffery
2017-07-26 17:52 ` Matt Spinler
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull Andrew Jeffery
3 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 16:08 UTC (permalink / raw)
To: joel; +Cc: Andrew Jeffery, mspinler, msbarth, openbmc
These bindings are optional, and only relevant on the AST2500 and
compatible watchdog controllers.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/watchdog/aspeed_wdt.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index d29f75e90bb6..9a07ef176054 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -42,6 +42,9 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
#define WDT_CTRL_WDT_INTR BIT(2)
#define WDT_CTRL_RESET_SYSTEM BIT(1)
#define WDT_CTRL_ENABLE BIT(0)
+#define WDT_RESET_WIDTH 0x18
+#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31)
+#define WDT_RESET_WIDTH_PUSHPULL BIT(30)
#define WDT_RESTART_MAGIC 0x4755
@@ -204,6 +207,24 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
}
+ if (of_device_is_compatible(np, "aspeed,ast2500-wdt")) {
+ u32 from = readl(wdt->base + WDT_RESET_WIDTH);
+ u32 to = from;
+
+ if (of_property_read_bool(np, "aspeed,ext-push-pull"))
+ to |= WDT_RESET_WIDTH_PUSHPULL;
+ else
+ to &= ~WDT_RESET_WIDTH_PUSHPULL;
+
+ if (of_property_read_bool(np, "aspeed,ext-active-high"))
+ to |= WDT_RESET_WIDTH_ACTIVE_HIGH;
+ else
+ to &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;
+
+ if (from != to)
+ writel(to, wdt->base + WDT_RESET_WIDTH);
+ }
+
ret = watchdog_register_device(&wdt->wdd);
if (ret) {
dev_err(&pdev->dev, "failed to register\n");
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull
2017-07-26 16:08 [PATCH linux dev-4.10 v2 0/4] Witherspoon WDTRST1 enablement Andrew Jeffery
` (2 preceding siblings ...)
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings Andrew Jeffery
@ 2017-07-26 16:08 ` Andrew Jeffery
2017-07-26 17:54 ` Matt Spinler
3 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 16:08 UTC (permalink / raw)
To: joel; +Cc: Andrew Jeffery, mspinler, msbarth, openbmc
Correctly drive the latch asserting the FAULT pin on the MAX31785 when
the watchdog bites. This ensures the fan outputs are ramped to 100% PWM
duty.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index c28222c17d03..df36b87ba105 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -488,6 +488,7 @@
&wdt1 {
aspeed,reset-type = "none";
aspeed,external-signal;
+ aspeed,ext-push-pull;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_wdtrst1_default>;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings Andrew Jeffery
@ 2017-07-26 17:52 ` Matt Spinler
2017-07-26 20:51 ` Andrew Jeffery
0 siblings, 1 reply; 9+ messages in thread
From: Matt Spinler @ 2017-07-26 17:52 UTC (permalink / raw)
To: Andrew Jeffery, joel; +Cc: msbarth, openbmc
On 7/26/2017 11:08 AM, Andrew Jeffery wrote:
> These bindings are optional, and only relevant on the AST2500 and
> compatible watchdog controllers.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/watchdog/aspeed_wdt.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index d29f75e90bb6..9a07ef176054 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -42,6 +42,9 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
> #define WDT_CTRL_WDT_INTR BIT(2)
> #define WDT_CTRL_RESET_SYSTEM BIT(1)
> #define WDT_CTRL_ENABLE BIT(0)
> +#define WDT_RESET_WIDTH 0x18
> +#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31)
> +#define WDT_RESET_WIDTH_PUSHPULL BIT(30)
>
> #define WDT_RESTART_MAGIC 0x4755
>
> @@ -204,6 +207,24 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> }
>
> + if (of_device_is_compatible(np, "aspeed,ast2500-wdt")) {
> + u32 from = readl(wdt->base + WDT_RESET_WIDTH);
> + u32 to = from;
> +
> + if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> + to |= WDT_RESET_WIDTH_PUSHPULL;
> + else
> + to &= ~WDT_RESET_WIDTH_PUSHPULL;
From the spec:
To set this bit to value ’1’, write bit[31:24] = 0xA8.
To set this bit to value ’0’, write bit[31:24] = 0x8A.
> +
> + if (of_property_read_bool(np, "aspeed,ext-active-high"))
> + to |= WDT_RESET_WIDTH_ACTIVE_HIGH;
> + else
> + to &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;
> +
And again here:
To set this bit to value ’1’, write bit[31:24] = 0xA5.
To set this bit to value ’0’, write bit[31:24] = 0x5A.
For others value, this bit will keep old value without change.
> + if (from != to)
> + writel(to, wdt->base + WDT_RESET_WIDTH);
> + }
> +
> ret = watchdog_register_device(&wdt->wdd);
> if (ret) {
> dev_err(&pdev->dev, "failed to register\n");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull Andrew Jeffery
@ 2017-07-26 17:54 ` Matt Spinler
2017-07-26 20:56 ` Andrew Jeffery
0 siblings, 1 reply; 9+ messages in thread
From: Matt Spinler @ 2017-07-26 17:54 UTC (permalink / raw)
To: Andrew Jeffery, joel; +Cc: msbarth, openbmc
On 7/26/2017 11:08 AM, Andrew Jeffery wrote:
> Correctly drive the latch asserting the FAULT pin on the MAX31785 when
> the watchdog bites. This ensures the fan outputs are ramped to 100% PWM
> duty.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index c28222c17d03..df36b87ba105 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -488,6 +488,7 @@
> &wdt1 {
> aspeed,reset-type = "none";
> aspeed,external-signal;
> + aspeed,ext-push-pull;
We need active-high for sure. It still works without push-pull, but not
sure if that is totally electronically ideal or not as the HW designer
requested for this to be push-pull.
>
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_wdtrst1_default>;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings
2017-07-26 17:52 ` Matt Spinler
@ 2017-07-26 20:51 ` Andrew Jeffery
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 20:51 UTC (permalink / raw)
To: Matt Spinler, joel; +Cc: msbarth, openbmc
On Thu, Jul 27, 2017, at 03:22, Matt Spinler wrote:
>
>
> On 7/26/2017 11:08 AM, Andrew Jeffery wrote:
> > These bindings are optional, and only relevant on the AST2500 and
> > compatible watchdog controllers.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > drivers/watchdog/aspeed_wdt.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> > index d29f75e90bb6..9a07ef176054 100644
> > --- a/drivers/watchdog/aspeed_wdt.c
> > +++ b/drivers/watchdog/aspeed_wdt.c
> > @@ -42,6 +42,9 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
> > #define WDT_CTRL_WDT_INTR BIT(2)
> > #define WDT_CTRL_RESET_SYSTEM BIT(1)
> > #define WDT_CTRL_ENABLE BIT(0)
> > +#define WDT_RESET_WIDTH 0x18
> > +#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31)
> > +#define WDT_RESET_WIDTH_PUSHPULL BIT(30)
> >
> > #define WDT_RESTART_MAGIC 0x4755
> >
> > @@ -204,6 +207,24 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> > set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> > }
> >
> > + if (of_device_is_compatible(np, "aspeed,ast2500-wdt")) {
> > + u32 from = readl(wdt->base + WDT_RESET_WIDTH);
> > + u32 to = from;
> > +
> > + if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> > + to |= WDT_RESET_WIDTH_PUSHPULL;
> > + else
> > + to &= ~WDT_RESET_WIDTH_PUSHPULL;
> From the spec:
>
> To set this bit to value ’1’, write bit[31:24] = 0xA8.
> To set this bit to value ’0’, write bit[31:24] = 0x8A.
Oof. Yeah, clearly I should have gone to bed earlier. I tested the patch
under qemu but forgot to model that behaviour.
>
> > +
> > + if (of_property_read_bool(np, "aspeed,ext-active-high"))
> > + to |= WDT_RESET_WIDTH_ACTIVE_HIGH;
> > + else
> > + to &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;
> > +
> And again here:
> To set this bit to value ’1’, write bit[31:24] = 0xA5.
> To set this bit to value ’0’, write bit[31:24] = 0x5A.
> For others value, this bit will keep old value without change.
As above.
Sorry about that.
Andrew
> > + if (from != to)
> > + writel(to, wdt->base + WDT_RESET_WIDTH);
> > + }
> > +
> > ret = watchdog_register_device(&wdt->wdd);
> > if (ret) {
> > dev_err(&pdev->dev, "failed to register\n");
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull
2017-07-26 17:54 ` Matt Spinler
@ 2017-07-26 20:56 ` Andrew Jeffery
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-07-26 20:56 UTC (permalink / raw)
To: Matt Spinler, joel; +Cc: msbarth, openbmc
On Thu, Jul 27, 2017, at 03:24, Matt Spinler wrote:
>
>
> On 7/26/2017 11:08 AM, Andrew Jeffery wrote:
> > Correctly drive the latch asserting the FAULT pin on the MAX31785 when
> > the watchdog bites. This ensures the fan outputs are ramped to 100% PWM
> > duty.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > index c28222c17d03..df36b87ba105 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > @@ -488,6 +488,7 @@
> > &wdt1 {
> > aspeed,reset-type = "none";
> > aspeed,external-signal;
> > + aspeed,ext-push-pull;
>
> We need active-high for sure. It still works without push-pull, but not
> sure if that is totally electronically ideal or not as the HW designer
> requested for this to be push-pull.
Right; given Jordan's statement, the wording of the documentation and
your testing the behaviour doesn't entirely make sense to me.
I might try to get it clarified with Aspeed.
Andrew
>
> >
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_wdtrst1_default>;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-26 20:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 16:08 [PATCH linux dev-4.10 v2 0/4] Witherspoon WDTRST1 enablement Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 1/4] ARM: aspeed: Request WDTRST1 pinctrl function Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 2/4] dt-bindings: watchdog: aspeed: External reset signal properties Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 3/4] watchdog: aspeed: Support external signal drive and polarity bindings Andrew Jeffery
2017-07-26 17:52 ` Matt Spinler
2017-07-26 20:51 ` Andrew Jeffery
2017-07-26 16:08 ` [PATCH linux dev-4.10 v2 4/4] ARM: dts: aspeed: Witherspoon WDT1 external signal is push-pull Andrew Jeffery
2017-07-26 17:54 ` Matt Spinler
2017-07-26 20:56 ` Andrew Jeffery
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.