diff mbox series

[v2] watchdog: ixp4xx: Make sure restart always works

Message ID 20230926-ixp4xx-wdt-restart-v2-1-15cf4639b423@linaro.org (mailing list archive)
State New
Headers show
Series [v2] watchdog: ixp4xx: Make sure restart always works | expand

Commit Message

Linus Walleij Sept. 26, 2023, 9:13 a.m. UTC
The IXP4xx watchdog in early "A0" silicon is unreliable and
cannot be registered, however for some systems such as the
USRobotics USR8200 the watchdog is the only restart option,
so implement a "dummy" watchdog that can only support restart
in this case.

Fixes: 1aea522809e6 ("watchdog: ixp4xx: Implement restart")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v2:
- Switch debugprint about only supporting restart from dev_err()
  to dev_info().
- Link to v1: https://lore.kernel.org/r/20230925-ixp4xx-wdt-restart-v1-1-fe2b9b131dbf@linaro.org
---
 drivers/watchdog/ixp4xx_wdt.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)


---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230924-ixp4xx-wdt-restart-d6bf28493173

Best regards,

Comments

Guenter Roeck Sept. 26, 2023, 12:41 p.m. UTC | #1
On Tue, Sep 26, 2023 at 11:13:44AM +0200, Linus Walleij wrote:
> The IXP4xx watchdog in early "A0" silicon is unreliable and
> cannot be registered, however for some systems such as the
> USRobotics USR8200 the watchdog is the only restart option,
> so implement a "dummy" watchdog that can only support restart
> in this case.
> 
> Fixes: 1aea522809e6 ("watchdog: ixp4xx: Implement restart")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v2:
> - Switch debugprint about only supporting restart from dev_err()
>   to dev_info().
> - Link to v1: https://lore.kernel.org/r/20230925-ixp4xx-wdt-restart-v1-1-fe2b9b131dbf@linaro.org
> ---
>  drivers/watchdog/ixp4xx_wdt.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/ixp4xx_wdt.c b/drivers/watchdog/ixp4xx_wdt.c
> index 607ce4b8df57..ec0c08652ec2 100644
> --- a/drivers/watchdog/ixp4xx_wdt.c
> +++ b/drivers/watchdog/ixp4xx_wdt.c
> @@ -105,6 +105,25 @@ static const struct watchdog_ops ixp4xx_wdt_ops = {
>  	.owner = THIS_MODULE,
>  };
>  
> +/*
> + * The A0 version of the IXP422 had a bug in the watchdog making
> + * is useless, but we still need to use it to restart the system
> + * as it is the only way, so in this special case we register a
> + * "dummy" watchdog that doesn't really work, but will support
> + * the restart operation.
> + */
> +static int ixp4xx_wdt_dummy(struct watchdog_device *wdd)
> +{
> +	return 0;
> +}
> +
> +static const struct watchdog_ops ixp4xx_wdt_restart_only_ops = {
> +	.start = ixp4xx_wdt_dummy,
> +	.stop = ixp4xx_wdt_dummy,
> +	.restart = ixp4xx_wdt_restart,
> +	.owner = THIS_MODULE,
> +};
> +
>  static const struct watchdog_info ixp4xx_wdt_info = {
>  	.options = WDIOF_KEEPALIVEPING
>  		| WDIOF_MAGICCLOSE
> @@ -114,14 +133,17 @@ static const struct watchdog_info ixp4xx_wdt_info = {
>  
>  static int ixp4xx_wdt_probe(struct platform_device *pdev)
>  {
> +	static const struct watchdog_ops *iwdt_ops;
>  	struct device *dev = &pdev->dev;
>  	struct ixp4xx_wdt *iwdt;
>  	struct clk *clk;
>  	int ret;
>  
>  	if (!(read_cpuid_id() & 0xf) && !cpu_is_ixp46x()) {
> -		dev_err(dev, "Rev. A0 IXP42x CPU detected - watchdog disabled\n");
> -		return -ENODEV;
> +		dev_info(dev, "Rev. A0 IXP42x CPU detected - only restart supported\n");
> +		iwdt_ops = &ixp4xx_wdt_restart_only_ops;
> +	} else {
> +		iwdt_ops = &ixp4xx_wdt_ops;
>  	}
>  
>  	iwdt = devm_kzalloc(dev, sizeof(*iwdt), GFP_KERNEL);
> @@ -141,7 +163,7 @@ static int ixp4xx_wdt_probe(struct platform_device *pdev)
>  		iwdt->rate = IXP4XX_TIMER_FREQ;
>  
>  	iwdt->wdd.info = &ixp4xx_wdt_info;
> -	iwdt->wdd.ops = &ixp4xx_wdt_ops;
> +	iwdt->wdd.ops = iwdt_ops;
>  	iwdt->wdd.min_timeout = 1;
>  	iwdt->wdd.max_timeout = U32_MAX / iwdt->rate;
>  	iwdt->wdd.parent = dev;
> 
> ---
> base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
> change-id: 20230924-ixp4xx-wdt-restart-d6bf28493173
> 
> Best regards,
> -- 
> Linus Walleij <linus.walleij@linaro.org>
>
diff mbox series

Patch

diff --git a/drivers/watchdog/ixp4xx_wdt.c b/drivers/watchdog/ixp4xx_wdt.c
index 607ce4b8df57..ec0c08652ec2 100644
--- a/drivers/watchdog/ixp4xx_wdt.c
+++ b/drivers/watchdog/ixp4xx_wdt.c
@@ -105,6 +105,25 @@  static const struct watchdog_ops ixp4xx_wdt_ops = {
 	.owner = THIS_MODULE,
 };
 
+/*
+ * The A0 version of the IXP422 had a bug in the watchdog making
+ * is useless, but we still need to use it to restart the system
+ * as it is the only way, so in this special case we register a
+ * "dummy" watchdog that doesn't really work, but will support
+ * the restart operation.
+ */
+static int ixp4xx_wdt_dummy(struct watchdog_device *wdd)
+{
+	return 0;
+}
+
+static const struct watchdog_ops ixp4xx_wdt_restart_only_ops = {
+	.start = ixp4xx_wdt_dummy,
+	.stop = ixp4xx_wdt_dummy,
+	.restart = ixp4xx_wdt_restart,
+	.owner = THIS_MODULE,
+};
+
 static const struct watchdog_info ixp4xx_wdt_info = {
 	.options = WDIOF_KEEPALIVEPING
 		| WDIOF_MAGICCLOSE
@@ -114,14 +133,17 @@  static const struct watchdog_info ixp4xx_wdt_info = {
 
 static int ixp4xx_wdt_probe(struct platform_device *pdev)
 {
+	static const struct watchdog_ops *iwdt_ops;
 	struct device *dev = &pdev->dev;
 	struct ixp4xx_wdt *iwdt;
 	struct clk *clk;
 	int ret;
 
 	if (!(read_cpuid_id() & 0xf) && !cpu_is_ixp46x()) {
-		dev_err(dev, "Rev. A0 IXP42x CPU detected - watchdog disabled\n");
-		return -ENODEV;
+		dev_info(dev, "Rev. A0 IXP42x CPU detected - only restart supported\n");
+		iwdt_ops = &ixp4xx_wdt_restart_only_ops;
+	} else {
+		iwdt_ops = &ixp4xx_wdt_ops;
 	}
 
 	iwdt = devm_kzalloc(dev, sizeof(*iwdt), GFP_KERNEL);
@@ -141,7 +163,7 @@  static int ixp4xx_wdt_probe(struct platform_device *pdev)
 		iwdt->rate = IXP4XX_TIMER_FREQ;
 
 	iwdt->wdd.info = &ixp4xx_wdt_info;
-	iwdt->wdd.ops = &ixp4xx_wdt_ops;
+	iwdt->wdd.ops = iwdt_ops;
 	iwdt->wdd.min_timeout = 1;
 	iwdt->wdd.max_timeout = U32_MAX / iwdt->rate;
 	iwdt->wdd.parent = dev;