* [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration
@ 2022-08-19 9:49 Chin-Ting Kuo
2022-08-19 11:44 ` Guenter Roeck
2022-08-24 0:46 ` Joel Stanley
0 siblings, 2 replies; 5+ messages in thread
From: Chin-Ting Kuo @ 2022-08-19 9:49 UTC (permalink / raw)
To: wim, linux, joel, andrew, BMC-SW, linux-watchdog, linux-aspeed,
linux-kernel, openbmc, Bonnie_Lo
If the output driving type is push-pull mode, the output
polarity should be selected in advance. Otherwise, an unexpected
value will be output at the moment of changing to push-pull mode.
Thus, output polarity, WDT18[31], must be configured before
changing driving type, WDT18[30].
Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
drivers/watchdog/aspeed_wdt.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index 436571b6fc79..a03e4ff812a2 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -325,18 +325,18 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
reg &= config->ext_pulse_width_mask;
- if (of_property_read_bool(np, "aspeed,ext-push-pull"))
- reg |= WDT_PUSH_PULL_MAGIC;
+ if (of_property_read_bool(np, "aspeed,ext-active-high"))
+ reg |= WDT_ACTIVE_HIGH_MAGIC;
else
- reg |= WDT_OPEN_DRAIN_MAGIC;
+ reg |= WDT_ACTIVE_LOW_MAGIC;
writel(reg, wdt->base + WDT_RESET_WIDTH);
reg &= config->ext_pulse_width_mask;
- if (of_property_read_bool(np, "aspeed,ext-active-high"))
- reg |= WDT_ACTIVE_HIGH_MAGIC;
+ if (of_property_read_bool(np, "aspeed,ext-push-pull"))
+ reg |= WDT_PUSH_PULL_MAGIC;
else
- reg |= WDT_ACTIVE_LOW_MAGIC;
+ reg |= WDT_OPEN_DRAIN_MAGIC;
writel(reg, wdt->base + WDT_RESET_WIDTH);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration
2022-08-19 9:49 [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration Chin-Ting Kuo
@ 2022-08-19 11:44 ` Guenter Roeck
2022-08-24 0:40 ` Bonnie Lo/WYHQ/Wiwynn
2022-08-24 0:46 ` Joel Stanley
1 sibling, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2022-08-19 11:44 UTC (permalink / raw)
To: Chin-Ting Kuo
Cc: wim, joel, andrew, BMC-SW, linux-watchdog, linux-aspeed,
linux-kernel, openbmc, Bonnie_Lo
On Fri, Aug 19, 2022 at 05:49:05PM +0800, Chin-Ting Kuo wrote:
> If the output driving type is push-pull mode, the output
> polarity should be selected in advance. Otherwise, an unexpected
> value will be output at the moment of changing to push-pull mode.
> Thus, output polarity, WDT18[31], must be configured before
> changing driving type, WDT18[30].
>
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/aspeed_wdt.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 436571b6fc79..a03e4ff812a2 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -325,18 +325,18 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
>
> reg &= config->ext_pulse_width_mask;
> - if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> - reg |= WDT_PUSH_PULL_MAGIC;
> + if (of_property_read_bool(np, "aspeed,ext-active-high"))
> + reg |= WDT_ACTIVE_HIGH_MAGIC;
> else
> - reg |= WDT_OPEN_DRAIN_MAGIC;
> + reg |= WDT_ACTIVE_LOW_MAGIC;
>
> writel(reg, wdt->base + WDT_RESET_WIDTH);
>
> reg &= config->ext_pulse_width_mask;
> - if (of_property_read_bool(np, "aspeed,ext-active-high"))
> - reg |= WDT_ACTIVE_HIGH_MAGIC;
> + if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> + reg |= WDT_PUSH_PULL_MAGIC;
> else
> - reg |= WDT_ACTIVE_LOW_MAGIC;
> + reg |= WDT_OPEN_DRAIN_MAGIC;
>
> writel(reg, wdt->base + WDT_RESET_WIDTH);
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration
2022-08-19 11:44 ` Guenter Roeck
@ 2022-08-24 0:40 ` Bonnie Lo/WYHQ/Wiwynn
0 siblings, 0 replies; 5+ messages in thread
From: Bonnie Lo/WYHQ/Wiwynn @ 2022-08-24 0:40 UTC (permalink / raw)
To: Guenter Roeck, Chin-Ting Kuo
Cc: wim, joel, andrew, BMC-SW, linux-watchdog, linux-aspeed,
linux-kernel, openbmc
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Friday, August 19, 2022 7:45 PM
> To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> Cc: wim@linux-watchdog.org; joel@jms.id.au; andrew@aj.id.au;
> BMC-SW@aspeedtech.com; linux-watchdog@vger.kernel.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> openbmc@lists.ozlabs.org; Bonnie Lo/WYHQ/Wiwynn
> <Bonnie_Lo@wiwynn.com>
> Subject: Re: [PATCH] watchdog: aspeed_wdt: Reorder output signal register
> configuration
>
> Security Reminder: Please be aware that this email was sent by an
> external sender.
>
> On Fri, Aug 19, 2022 at 05:49:05PM +0800, Chin-Ting Kuo wrote:
> > If the output driving type is push-pull mode, the output polarity
> > should be selected in advance. Otherwise, an unexpected value will be
> > output at the moment of changing to push-pull mode.
> > Thus, output polarity, WDT18[31], must be configured before changing
> > driving type, WDT18[30].
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
Tested-by: Bonnie Lo <Bonnie_Lo@wiwynn.com>
> > ---
> > drivers/watchdog/aspeed_wdt.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/watchdog/aspeed_wdt.c
> > b/drivers/watchdog/aspeed_wdt.c index 436571b6fc79..a03e4ff812a2
> > 100644
> > --- a/drivers/watchdog/aspeed_wdt.c
> > +++ b/drivers/watchdog/aspeed_wdt.c
> > @@ -325,18 +325,18 @@ static int aspeed_wdt_probe(struct
> platform_device *pdev)
> > u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
> >
> > reg &= config->ext_pulse_width_mask;
> > - if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> > - reg |= WDT_PUSH_PULL_MAGIC;
> > + if (of_property_read_bool(np, "aspeed,ext-active-high"))
> > + reg |= WDT_ACTIVE_HIGH_MAGIC;
> > else
> > - reg |= WDT_OPEN_DRAIN_MAGIC;
> > + reg |= WDT_ACTIVE_LOW_MAGIC;
> >
> > writel(reg, wdt->base + WDT_RESET_WIDTH);
> >
> > reg &= config->ext_pulse_width_mask;
> > - if (of_property_read_bool(np, "aspeed,ext-active-high"))
> > - reg |= WDT_ACTIVE_HIGH_MAGIC;
> > + if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> > + reg |= WDT_PUSH_PULL_MAGIC;
> > else
> > - reg |= WDT_ACTIVE_LOW_MAGIC;
> > + reg |= WDT_OPEN_DRAIN_MAGIC;
> >
> > writel(reg, wdt->base + WDT_RESET_WIDTH);
> > }
> > --
> > 2.25.1
> >
WIWYNN PROPRIETARY This email (and any attachments) contains proprietary or confidential information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. If you are not the intended recipient, please notify the sender and delete this email immediately.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration
2022-08-19 9:49 [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration Chin-Ting Kuo
2022-08-19 11:44 ` Guenter Roeck
@ 2022-08-24 0:46 ` Joel Stanley
2022-08-24 1:56 ` Chin-Ting Kuo
1 sibling, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2022-08-24 0:46 UTC (permalink / raw)
To: Chin-Ting Kuo
Cc: wim, linux, andrew, BMC-SW, linux-watchdog, linux-aspeed,
linux-kernel, openbmc, Bonnie_Lo
On Fri, 19 Aug 2022 at 09:49, Chin-Ting Kuo
<chin-ting_kuo@aspeedtech.com> wrote:
>
> If the output driving type is push-pull mode, the output
> polarity should be selected in advance. Otherwise, an unexpected
> value will be output at the moment of changing to push-pull mode.
> Thus, output polarity, WDT18[31], must be configured before
> changing driving type, WDT18[30].
>
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
Thanks Chin-Ting.
Fixes: 012c04601f9d ("watchdog: aspeed: Support configuration of
external signal properties")
Reviewed-by: Joel Stanley <joel@jms.id.au>
I wonder if any of these machines hit the bug:
$ git grep -l aspeed,ext-push-pull arch/arm/boot/dts/
arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
arch/arm/boot/dts/aspeed-bmc-inspur-fp5280g2.dts
arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
arch/arm/boot/dts/aspeed-bmc-opp-mowgli.dts
arch/arm/boot/dts/aspeed-bmc-opp-swift.dts
arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> ---
> drivers/watchdog/aspeed_wdt.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 436571b6fc79..a03e4ff812a2 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -325,18 +325,18 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
>
> reg &= config->ext_pulse_width_mask;
> - if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> - reg |= WDT_PUSH_PULL_MAGIC;
> + if (of_property_read_bool(np, "aspeed,ext-active-high"))
> + reg |= WDT_ACTIVE_HIGH_MAGIC;
> else
> - reg |= WDT_OPEN_DRAIN_MAGIC;
> + reg |= WDT_ACTIVE_LOW_MAGIC;
>
> writel(reg, wdt->base + WDT_RESET_WIDTH);
>
> reg &= config->ext_pulse_width_mask;
> - if (of_property_read_bool(np, "aspeed,ext-active-high"))
> - reg |= WDT_ACTIVE_HIGH_MAGIC;
> + if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> + reg |= WDT_PUSH_PULL_MAGIC;
> else
> - reg |= WDT_ACTIVE_LOW_MAGIC;
> + reg |= WDT_OPEN_DRAIN_MAGIC;
>
> writel(reg, wdt->base + WDT_RESET_WIDTH);
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration
2022-08-24 0:46 ` Joel Stanley
@ 2022-08-24 1:56 ` Chin-Ting Kuo
0 siblings, 0 replies; 5+ messages in thread
From: Chin-Ting Kuo @ 2022-08-24 1:56 UTC (permalink / raw)
To: Joel Stanley
Cc: wim, linux, andrew, BMC-SW, linux-watchdog, linux-aspeed,
linux-kernel, openbmc, Bonnie_Lo
Hi Joel,
Thanks for the review.
> -----Original Message-----
> From: Joel Stanley <joel@jms.id.au>
> Sent: Wednesday, August 24, 2022 8:46 AM
> To: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> Subject: Re: [PATCH] watchdog: aspeed_wdt: Reorder output signal register
> configuration
>
> On Fri, 19 Aug 2022 at 09:49, Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> wrote:
> >
> > If the output driving type is push-pull mode, the output polarity
> > should be selected in advance. Otherwise, an unexpected value will be
> > output at the moment of changing to push-pull mode.
> > Thus, output polarity, WDT18[31], must be configured before changing
> > driving type, WDT18[30].
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
>
> Thanks Chin-Ting.
>
> Fixes: 012c04601f9d ("watchdog: aspeed: Support configuration of external
> signal properties")
> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
> I wonder if any of these machines hit the bug:
>
Unexpected output pin value may not always appear at the moment of changing driving type.
It depends on the output status of that pin at that time.
But, with this patch, the output pin value can be confirmed.
Chin-Ting
> $ git grep -l aspeed,ext-push-pull arch/arm/boot/dts/
> arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> arch/arm/boot/dts/aspeed-bmc-inspur-fp5280g2.dts
> arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
> arch/arm/boot/dts/aspeed-bmc-opp-mowgli.dts
> arch/arm/boot/dts/aspeed-bmc-opp-swift.dts
> arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
> arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
>
>
> > ---
> > drivers/watchdog/aspeed_wdt.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/watchdog/aspeed_wdt.c
> > b/drivers/watchdog/aspeed_wdt.c index 436571b6fc79..a03e4ff812a2
> > 100644
> > --- a/drivers/watchdog/aspeed_wdt.c
> > +++ b/drivers/watchdog/aspeed_wdt.c
> > @@ -325,18 +325,18 @@ static int aspeed_wdt_probe(struct
> platform_device *pdev)
> > u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
> >
> > reg &= config->ext_pulse_width_mask;
> > - if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> > - reg |= WDT_PUSH_PULL_MAGIC;
> > + if (of_property_read_bool(np, "aspeed,ext-active-high"))
> > + reg |= WDT_ACTIVE_HIGH_MAGIC;
> > else
> > - reg |= WDT_OPEN_DRAIN_MAGIC;
> > + reg |= WDT_ACTIVE_LOW_MAGIC;
> >
> > writel(reg, wdt->base + WDT_RESET_WIDTH);
> >
> > reg &= config->ext_pulse_width_mask;
> > - if (of_property_read_bool(np, "aspeed,ext-active-high"))
> > - reg |= WDT_ACTIVE_HIGH_MAGIC;
> > + if (of_property_read_bool(np, "aspeed,ext-push-pull"))
> > + reg |= WDT_PUSH_PULL_MAGIC;
> > else
> > - reg |= WDT_ACTIVE_LOW_MAGIC;
> > + reg |= WDT_OPEN_DRAIN_MAGIC;
> >
> > writel(reg, wdt->base + WDT_RESET_WIDTH);
> > }
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-24 1:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-19 9:49 [PATCH] watchdog: aspeed_wdt: Reorder output signal register configuration Chin-Ting Kuo
2022-08-19 11:44 ` Guenter Roeck
2022-08-24 0:40 ` Bonnie Lo/WYHQ/Wiwynn
2022-08-24 0:46 ` Joel Stanley
2022-08-24 1:56 ` Chin-Ting Kuo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).