All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5
@ 2026-03-02 11:33 Balakrishnan Sambath
  2026-03-02 11:33 ` [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on " Balakrishnan Sambath
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Balakrishnan Sambath @ 2026-03-02 11:33 UTC (permalink / raw)
  To: linux-watchdog
  Cc: wim, linux, alexandre.belloni, andrei.simion, linux-kernel,
	linux-arm-kernel

The sama5d4_wdt driver hardcodes AT91_WDT_WDDIS (bit 15) for WDDIS
detection, which is incorrect for SAM9X60 and SAMA7G5 that use bit 12
(AT91_SAM9X60_WDDIS). This series fixes the detection by introducing
a per-device wddis_mask and documents the bit position difference in
the header.

Changes in v2:
- Reorder patches: fix first, documentation second
- Drop patch 3/3, not needed with wddis_mask approach
- Keep AT91_SAM9X60_* register names, drop _LEGACY/_MODERN naming
- Limit header changes to WDDIS comments and datasheet references

Balakrishnan Sambath (2):
  watchdog: sama5d4_wdt: Fix WDDIS detection on SAM9X60 and SAMA7G5
  watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family

 drivers/watchdog/at91sam9_wdt.h |  6 +++--
 drivers/watchdog/sama5d4_wdt.c  | 48 +++++++++++++++------------------
 2 files changed, 25 insertions(+), 29 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on SAM9X60 and SAMA7G5
  2026-03-02 11:33 [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan Sambath
@ 2026-03-02 11:33 ` Balakrishnan Sambath
  2026-03-02 14:32   ` Alexandre Belloni
  2026-03-02 11:33 ` [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family Balakrishnan Sambath
  2026-05-27 13:04 ` [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan.S
  2 siblings, 1 reply; 7+ messages in thread
From: Balakrishnan Sambath @ 2026-03-02 11:33 UTC (permalink / raw)
  To: linux-watchdog
  Cc: wim, linux, alexandre.belloni, andrei.simion, linux-kernel,
	linux-arm-kernel, Nicolas Ferre, Claudiu Beznea, Mathieu Othacehe

The driver hardcoded AT91_WDT_WDDIS (bit 15) in wdt_enabled and the
probe initial state readout. SAM9X60 and SAMA7G5 use bit 12
(AT91_SAM9X60_WDDIS), causing incorrect WDDIS detection.

Introduce a per-device wddis_mask field to select the correct WDDIS
bit based on the compatible string.

Fixes: 266da53c35fc ("watchdog: sama5d4: readout initial state")
Co-developed-by: Andrei Simion <andrei.simion@microchip.com>
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/watchdog/sama5d4_wdt.c | 48 +++++++++++++++-------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 13e72918338a..704b786cc2ec 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -24,37 +24,41 @@
 #define WDT_DEFAULT_TIMEOUT	MAX_WDT_TIMEOUT
 
 #define WDT_SEC2TICKS(s)	((s) ? (((s) << 8) - 1) : 0)
 
 struct sama5d4_wdt {
 	struct watchdog_device	wdd;
 	void __iomem		*reg_base;
 	u32			mr;
 	u32			ir;
+	u32			wddis_mask;
 	unsigned long		last_ping;
 	bool			need_irq;
 	bool			sam9x60_support;
 };
 
 static int wdt_timeout;
 static bool nowayout = WATCHDOG_NOWAYOUT;
 
 module_param(wdt_timeout, int, 0);
 MODULE_PARM_DESC(wdt_timeout,
 	"Watchdog timeout in seconds. (default = "
 	__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
 
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout,
 	"Watchdog cannot be stopped once started (default="
 	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-#define wdt_enabled (!(wdt->mr & AT91_WDT_WDDIS))
+static inline bool wdt_enabled(struct sama5d4_wdt *wdt)
+{
+	return !(wdt->mr & wdt->wddis_mask);
+}
 
 #define wdt_read(wdt, field) \
 	readl_relaxed((wdt)->reg_base + (field))
 
 /* 4 slow clock periods is 4/32768 = 122.07µs*/
 #define WDT_DELAY	usecs_to_jiffies(123)
 
 static void wdt_write(struct sama5d4_wdt *wdt, u32 field, u32 val)
 {
@@ -75,55 +79,49 @@ static void wdt_write_nosleep(struct sama5d4_wdt *wdt, u32 field, u32 val)
 		udelay(123);
 	writel_relaxed(val, wdt->reg_base + field);
 	wdt->last_ping = jiffies;
 }
 
 static int sama5d4_wdt_start(struct watchdog_device *wdd)
 {
 	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
 
-	if (wdt->sam9x60_support) {
+	if (wdt->sam9x60_support)
 		writel_relaxed(wdt->ir, wdt->reg_base + AT91_SAM9X60_IER);
-		wdt->mr &= ~AT91_SAM9X60_WDDIS;
-	} else {
-		wdt->mr &= ~AT91_WDT_WDDIS;
-	}
+	wdt->mr &= ~wdt->wddis_mask;
 	wdt_write(wdt, AT91_WDT_MR, wdt->mr);
 
 	return 0;
 }
 
 static int sama5d4_wdt_stop(struct watchdog_device *wdd)
 {
 	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
 
-	if (wdt->sam9x60_support) {
+	if (wdt->sam9x60_support)
 		writel_relaxed(wdt->ir, wdt->reg_base + AT91_SAM9X60_IDR);
-		wdt->mr |= AT91_SAM9X60_WDDIS;
-	} else {
-		wdt->mr |= AT91_WDT_WDDIS;
-	}
+	wdt->mr |= wdt->wddis_mask;
 	wdt_write(wdt, AT91_WDT_MR, wdt->mr);
 
 	return 0;
 }
 
 static int sama5d4_wdt_ping(struct watchdog_device *wdd)
 {
 	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
 
 	wdt_write(wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT);
 
 	return 0;
 }
 
 static int sama5d4_wdt_set_timeout(struct watchdog_device *wdd,
-				 unsigned int timeout)
+				    unsigned int timeout)
 {
 	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
 	u32 value = WDT_SEC2TICKS(timeout);
 
 	if (wdt->sam9x60_support) {
 		wdt_write(wdt, AT91_SAM9X60_WLR,
 			  AT91_SAM9X60_SET_COUNTER(value));
 
 		wdd->timeout = timeout;
@@ -134,20 +132,20 @@ static int sama5d4_wdt_set_timeout(struct watchdog_device *wdd,
 	wdt->mr |= AT91_WDT_SET_WDV(value);
 
 	/*
 	 * WDDIS has to be 0 when updating WDD/WDV. The datasheet states: When
 	 * setting the WDDIS bit, and while it is set, the fields WDV and WDD
 	 * must not be modified.
 	 * If the watchdog is enabled, then the timeout can be updated. Else,
 	 * wait that the user enables it.
 	 */
-	if (wdt_enabled)
-		wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~AT91_WDT_WDDIS);
+	if (wdt_enabled(wdt))
+		wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~wdt->wddis_mask);
 
 	wdd->timeout = timeout;
 
 	return 0;
 }
 
 static const struct watchdog_info sama5d4_wdt_info = {
 	.options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
 	.identity = "Atmel SAMA5D4 Watchdog",
@@ -178,22 +176,19 @@ static irqreturn_t sama5d4_wdt_irq_handler(int irq, void *dev_id)
 	}
 
 	return IRQ_HANDLED;
 }
 
 static int of_sama5d4_wdt_init(struct device_node *np, struct sama5d4_wdt *wdt)
 {
 	const char *tmp;
 
-	if (wdt->sam9x60_support)
-		wdt->mr = AT91_SAM9X60_WDDIS;
-	else
-		wdt->mr = AT91_WDT_WDDIS;
+	wdt->mr = wdt->wddis_mask;
 
 	if (!of_property_read_string(np, "atmel,watchdog-type", &tmp) &&
 	    !strcmp(tmp, "software"))
 		wdt->need_irq = true;
 
 	if (of_property_read_bool(np, "atmel,idle-halt"))
 		wdt->mr |= AT91_WDT_WDIDLEHLT;
 
 	if (of_property_read_bool(np, "atmel,dbg-halt"))
@@ -207,27 +202,23 @@ static int sama5d4_wdt_init(struct sama5d4_wdt *wdt)
 	u32 reg, val;
 
 	val = WDT_SEC2TICKS(WDT_DEFAULT_TIMEOUT);
 	/*
 	 * When booting and resuming, the bootloader may have changed the
 	 * watchdog configuration.
 	 * If the watchdog is already running, we can safely update it.
 	 * Else, we have to disable it properly.
 	 */
-	if (!wdt_enabled) {
+	if (!wdt_enabled(wdt)) {
 		reg = wdt_read(wdt, AT91_WDT_MR);
-		if (wdt->sam9x60_support && (!(reg & AT91_SAM9X60_WDDIS)))
-			wdt_write_nosleep(wdt, AT91_WDT_MR,
-					  reg | AT91_SAM9X60_WDDIS);
-		else if (!wdt->sam9x60_support &&
-			 (!(reg & AT91_WDT_WDDIS)))
+		if (!(reg & wdt->wddis_mask))
 			wdt_write_nosleep(wdt, AT91_WDT_MR,
-					  reg | AT91_WDT_WDDIS);
+					  reg | wdt->wddis_mask);
 	}
 
 	if (wdt->sam9x60_support) {
 		if (wdt->need_irq)
 			wdt->ir = AT91_SAM9X60_PERINT;
 		else
 			wdt->mr |= AT91_SAM9X60_PERIODRST;
 
 		wdt_write(wdt, AT91_SAM9X60_IER, wdt->ir);
@@ -267,18 +258,21 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 	wdd->ops = &sama5d4_wdt_ops;
 	wdd->min_timeout = MIN_WDT_TIMEOUT;
 	wdd->max_timeout = MAX_WDT_TIMEOUT;
 	wdt->last_ping = jiffies;
 
 	if (of_device_is_compatible(dev->of_node, "microchip,sam9x60-wdt") ||
 	    of_device_is_compatible(dev->of_node, "microchip,sama7g5-wdt"))
 		wdt->sam9x60_support = true;
 
+	wdt->wddis_mask = wdt->sam9x60_support ? AT91_SAM9X60_WDDIS
+						: AT91_WDT_WDDIS;
+
 	watchdog_set_drvdata(wdd, wdt);
 
 	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
 	wdt->reg_base = regs;
 
 	ret = of_sama5d4_wdt_init(dev->of_node, wdt);
@@ -300,20 +294,20 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(dev, "cannot register interrupt handler\n");
 			return ret;
 		}
 	}
 
 	watchdog_init_timeout(wdd, wdt_timeout, dev);
 
 	reg = wdt_read(wdt, AT91_WDT_MR);
-	if (!(reg & AT91_WDT_WDDIS)) {
-		wdt->mr &= ~AT91_WDT_WDDIS;
+	if (!(reg & wdt->wddis_mask)) {
+		wdt->mr &= ~wdt->wddis_mask;
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
 	}
 
 	ret = sama5d4_wdt_init(wdt);
 	if (ret)
 		return ret;
 
 	watchdog_set_nowayout(wdd, nowayout);
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family
  2026-03-02 11:33 [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan Sambath
  2026-03-02 11:33 ` [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on " Balakrishnan Sambath
@ 2026-03-02 11:33 ` Balakrishnan Sambath
  2026-03-02 14:32   ` Alexandre Belloni
  2026-05-03 21:03   ` Guenter Roeck
  2026-05-27 13:04 ` [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan.S
  2 siblings, 2 replies; 7+ messages in thread
From: Balakrishnan Sambath @ 2026-03-02 11:33 UTC (permalink / raw)
  To: linux-watchdog
  Cc: wim, linux, alexandre.belloni, andrei.simion, linux-kernel,
	linux-arm-kernel, Nicolas Ferre, Claudiu Beznea

AT91_WDT_WDDIS (bit 15) applies to SAMA5/AT91SAM9261 and
AT91_SAM9X60_WDDIS (bit 12) to SAM9X60/SAMA7G5/SAM9X75. Update
comments to reflect this and add SAMA7G5 and SAM9X75 datasheet
references to the file header.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/watchdog/at91sam9_wdt.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/at91sam9_wdt.h b/drivers/watchdog/at91sam9_wdt.h
index 298d545df1a1..2020694f8f6f 100644
--- a/drivers/watchdog/at91sam9_wdt.h
+++ b/drivers/watchdog/at91sam9_wdt.h
@@ -3,40 +3,42 @@
  * drivers/watchdog/at91sam9_wdt.h
  *
  * Copyright (C) 2007 Andrew Victor
  * Copyright (C) 2007 Atmel Corporation.
  * Copyright (C) 2019 Microchip Technology Inc. and its subsidiaries
  *
  * Watchdog Timer (WDT) - System peripherals regsters.
  * Based on AT91SAM9261 datasheet revision D.
  * Based on SAM9X60 datasheet.
+ * Based on SAMA7G5 datasheet.
+ * Based on SAM9X75 datasheet.
  *
  */
 
 #ifndef AT91_WDT_H
 #define AT91_WDT_H
 
 #include <linux/bits.h>
 
 #define AT91_WDT_CR		0x00			/* Watchdog Control Register */
 #define  AT91_WDT_WDRSTT	BIT(0)			/* Restart */
 #define  AT91_WDT_KEY		(0xa5UL << 24)		/* KEY Password */
 
 #define AT91_WDT_MR		0x04			/* Watchdog Mode Register */
 #define  AT91_WDT_WDV		(0xfffUL << 0)		/* Counter Value */
 #define  AT91_WDT_SET_WDV(x)	((x) & AT91_WDT_WDV)
 #define  AT91_SAM9X60_PERIODRST	BIT(4)		/* Period Reset */
 #define  AT91_SAM9X60_RPTHRST	BIT(5)		/* Minimum Restart Period */
 #define  AT91_WDT_WDFIEN	BIT(12)		/* Fault Interrupt Enable */
-#define  AT91_SAM9X60_WDDIS	BIT(12)		/* Watchdog Disable */
+#define  AT91_SAM9X60_WDDIS	BIT(12)		/* Watchdog Disable (SAM9X60, SAMA7G5, SAM9X75) */
 #define  AT91_WDT_WDRSTEN	BIT(13)		/* Reset Processor */
 #define  AT91_WDT_WDRPROC	BIT(14)		/* Timer Restart */
-#define  AT91_WDT_WDDIS		BIT(15)		/* Watchdog Disable */
+#define  AT91_WDT_WDDIS		BIT(15)		/* Watchdog Disable (SAMA5, AT91SAM9261) */
 #define  AT91_WDT_WDD		(0xfffUL << 16)		/* Delta Value */
 #define  AT91_WDT_SET_WDD(x)	(((x) << 16) & AT91_WDT_WDD)
 #define  AT91_WDT_WDDBGHLT	BIT(28)		/* Debug Halt */
 #define  AT91_WDT_WDIDLEHLT	BIT(29)		/* Idle Halt */
 
 #define AT91_WDT_SR		0x08		/* Watchdog Status Register */
 #define  AT91_WDT_WDUNF		BIT(0)		/* Watchdog Underflow */
 #define  AT91_WDT_WDERR		BIT(1)		/* Watchdog Error */
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on SAM9X60 and SAMA7G5
  2026-03-02 11:33 ` [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on " Balakrishnan Sambath
@ 2026-03-02 14:32   ` Alexandre Belloni
  0 siblings, 0 replies; 7+ messages in thread
From: Alexandre Belloni @ 2026-03-02 14:32 UTC (permalink / raw)
  To: Balakrishnan Sambath
  Cc: linux-watchdog, wim, linux, andrei.simion, linux-kernel,
	linux-arm-kernel, Nicolas Ferre, Claudiu Beznea, Mathieu Othacehe

On 02/03/2026 17:03:09+0530, Balakrishnan Sambath wrote:
> The driver hardcoded AT91_WDT_WDDIS (bit 15) in wdt_enabled and the
> probe initial state readout. SAM9X60 and SAMA7G5 use bit 12
> (AT91_SAM9X60_WDDIS), causing incorrect WDDIS detection.
> 
> Introduce a per-device wddis_mask field to select the correct WDDIS
> bit based on the compatible string.
> 
> Fixes: 266da53c35fc ("watchdog: sama5d4: readout initial state")
> Co-developed-by: Andrei Simion <andrei.simion@microchip.com>
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/watchdog/sama5d4_wdt.c | 48 +++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 13e72918338a..704b786cc2ec 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c
> @@ -24,37 +24,41 @@
>  #define WDT_DEFAULT_TIMEOUT	MAX_WDT_TIMEOUT
>  
>  #define WDT_SEC2TICKS(s)	((s) ? (((s) << 8) - 1) : 0)
>  
>  struct sama5d4_wdt {
>  	struct watchdog_device	wdd;
>  	void __iomem		*reg_base;
>  	u32			mr;
>  	u32			ir;
> +	u32			wddis_mask;
>  	unsigned long		last_ping;
>  	bool			need_irq;
>  	bool			sam9x60_support;
>  };
>  
>  static int wdt_timeout;
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  
>  module_param(wdt_timeout, int, 0);
>  MODULE_PARM_DESC(wdt_timeout,
>  	"Watchdog timeout in seconds. (default = "
>  	__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
>  
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout,
>  	"Watchdog cannot be stopped once started (default="
>  	__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> -#define wdt_enabled (!(wdt->mr & AT91_WDT_WDDIS))
> +static inline bool wdt_enabled(struct sama5d4_wdt *wdt)
> +{
> +	return !(wdt->mr & wdt->wddis_mask);
> +}
>  
>  #define wdt_read(wdt, field) \
>  	readl_relaxed((wdt)->reg_base + (field))
>  
>  /* 4 slow clock periods is 4/32768 = 122.07µs*/
>  #define WDT_DELAY	usecs_to_jiffies(123)
>  
>  static void wdt_write(struct sama5d4_wdt *wdt, u32 field, u32 val)
>  {
> @@ -75,55 +79,49 @@ static void wdt_write_nosleep(struct sama5d4_wdt *wdt, u32 field, u32 val)
>  		udelay(123);
>  	writel_relaxed(val, wdt->reg_base + field);
>  	wdt->last_ping = jiffies;
>  }
>  
>  static int sama5d4_wdt_start(struct watchdog_device *wdd)
>  {
>  	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -	if (wdt->sam9x60_support) {
> +	if (wdt->sam9x60_support)
>  		writel_relaxed(wdt->ir, wdt->reg_base + AT91_SAM9X60_IER);
> -		wdt->mr &= ~AT91_SAM9X60_WDDIS;
> -	} else {
> -		wdt->mr &= ~AT91_WDT_WDDIS;
> -	}
> +	wdt->mr &= ~wdt->wddis_mask;
>  	wdt_write(wdt, AT91_WDT_MR, wdt->mr);
>  
>  	return 0;
>  }
>  
>  static int sama5d4_wdt_stop(struct watchdog_device *wdd)
>  {
>  	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -	if (wdt->sam9x60_support) {
> +	if (wdt->sam9x60_support)
>  		writel_relaxed(wdt->ir, wdt->reg_base + AT91_SAM9X60_IDR);
> -		wdt->mr |= AT91_SAM9X60_WDDIS;
> -	} else {
> -		wdt->mr |= AT91_WDT_WDDIS;
> -	}
> +	wdt->mr |= wdt->wddis_mask;
>  	wdt_write(wdt, AT91_WDT_MR, wdt->mr);
>  
>  	return 0;
>  }
>  
>  static int sama5d4_wdt_ping(struct watchdog_device *wdd)
>  {
>  	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
>  
>  	wdt_write(wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT);
>  
>  	return 0;
>  }
>  
>  static int sama5d4_wdt_set_timeout(struct watchdog_device *wdd,
> -				 unsigned int timeout)
> +				    unsigned int timeout)
>  {
>  	struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
>  	u32 value = WDT_SEC2TICKS(timeout);
>  
>  	if (wdt->sam9x60_support) {
>  		wdt_write(wdt, AT91_SAM9X60_WLR,
>  			  AT91_SAM9X60_SET_COUNTER(value));
>  
>  		wdd->timeout = timeout;
> @@ -134,20 +132,20 @@ static int sama5d4_wdt_set_timeout(struct watchdog_device *wdd,
>  	wdt->mr |= AT91_WDT_SET_WDV(value);
>  
>  	/*
>  	 * WDDIS has to be 0 when updating WDD/WDV. The datasheet states: When
>  	 * setting the WDDIS bit, and while it is set, the fields WDV and WDD
>  	 * must not be modified.
>  	 * If the watchdog is enabled, then the timeout can be updated. Else,
>  	 * wait that the user enables it.
>  	 */
> -	if (wdt_enabled)
> -		wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~AT91_WDT_WDDIS);
> +	if (wdt_enabled(wdt))
> +		wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~wdt->wddis_mask);
>  
>  	wdd->timeout = timeout;
>  
>  	return 0;
>  }
>  
>  static const struct watchdog_info sama5d4_wdt_info = {
>  	.options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
>  	.identity = "Atmel SAMA5D4 Watchdog",
> @@ -178,22 +176,19 @@ static irqreturn_t sama5d4_wdt_irq_handler(int irq, void *dev_id)
>  	}
>  
>  	return IRQ_HANDLED;
>  }
>  
>  static int of_sama5d4_wdt_init(struct device_node *np, struct sama5d4_wdt *wdt)
>  {
>  	const char *tmp;
>  
> -	if (wdt->sam9x60_support)
> -		wdt->mr = AT91_SAM9X60_WDDIS;
> -	else
> -		wdt->mr = AT91_WDT_WDDIS;
> +	wdt->mr = wdt->wddis_mask;
>  
>  	if (!of_property_read_string(np, "atmel,watchdog-type", &tmp) &&
>  	    !strcmp(tmp, "software"))
>  		wdt->need_irq = true;
>  
>  	if (of_property_read_bool(np, "atmel,idle-halt"))
>  		wdt->mr |= AT91_WDT_WDIDLEHLT;
>  
>  	if (of_property_read_bool(np, "atmel,dbg-halt"))
> @@ -207,27 +202,23 @@ static int sama5d4_wdt_init(struct sama5d4_wdt *wdt)
>  	u32 reg, val;
>  
>  	val = WDT_SEC2TICKS(WDT_DEFAULT_TIMEOUT);
>  	/*
>  	 * When booting and resuming, the bootloader may have changed the
>  	 * watchdog configuration.
>  	 * If the watchdog is already running, we can safely update it.
>  	 * Else, we have to disable it properly.
>  	 */
> -	if (!wdt_enabled) {
> +	if (!wdt_enabled(wdt)) {
>  		reg = wdt_read(wdt, AT91_WDT_MR);
> -		if (wdt->sam9x60_support && (!(reg & AT91_SAM9X60_WDDIS)))
> -			wdt_write_nosleep(wdt, AT91_WDT_MR,
> -					  reg | AT91_SAM9X60_WDDIS);
> -		else if (!wdt->sam9x60_support &&
> -			 (!(reg & AT91_WDT_WDDIS)))
> +		if (!(reg & wdt->wddis_mask))
>  			wdt_write_nosleep(wdt, AT91_WDT_MR,
> -					  reg | AT91_WDT_WDDIS);
> +					  reg | wdt->wddis_mask);
>  	}
>  
>  	if (wdt->sam9x60_support) {
>  		if (wdt->need_irq)
>  			wdt->ir = AT91_SAM9X60_PERINT;
>  		else
>  			wdt->mr |= AT91_SAM9X60_PERIODRST;
>  
>  		wdt_write(wdt, AT91_SAM9X60_IER, wdt->ir);
> @@ -267,18 +258,21 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
>  	wdd->ops = &sama5d4_wdt_ops;
>  	wdd->min_timeout = MIN_WDT_TIMEOUT;
>  	wdd->max_timeout = MAX_WDT_TIMEOUT;
>  	wdt->last_ping = jiffies;
>  
>  	if (of_device_is_compatible(dev->of_node, "microchip,sam9x60-wdt") ||
>  	    of_device_is_compatible(dev->of_node, "microchip,sama7g5-wdt"))
>  		wdt->sam9x60_support = true;
>  
> +	wdt->wddis_mask = wdt->sam9x60_support ? AT91_SAM9X60_WDDIS
> +						: AT91_WDT_WDDIS;
> +
>  	watchdog_set_drvdata(wdd, wdt);
>  
>  	regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(regs))
>  		return PTR_ERR(regs);
>  
>  	wdt->reg_base = regs;
>  
>  	ret = of_sama5d4_wdt_init(dev->of_node, wdt);
> @@ -300,20 +294,20 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
>  		if (ret) {
>  			dev_err(dev, "cannot register interrupt handler\n");
>  			return ret;
>  		}
>  	}
>  
>  	watchdog_init_timeout(wdd, wdt_timeout, dev);
>  
>  	reg = wdt_read(wdt, AT91_WDT_MR);
> -	if (!(reg & AT91_WDT_WDDIS)) {
> -		wdt->mr &= ~AT91_WDT_WDDIS;
> +	if (!(reg & wdt->wddis_mask)) {
> +		wdt->mr &= ~wdt->wddis_mask;
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  	}
>  
>  	ret = sama5d4_wdt_init(wdt);
>  	if (ret)
>  		return ret;
>  
>  	watchdog_set_nowayout(wdd, nowayout);
>  
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family
  2026-03-02 11:33 ` [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family Balakrishnan Sambath
@ 2026-03-02 14:32   ` Alexandre Belloni
  2026-05-03 21:03   ` Guenter Roeck
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Belloni @ 2026-03-02 14:32 UTC (permalink / raw)
  To: Balakrishnan Sambath
  Cc: linux-watchdog, wim, linux, andrei.simion, linux-kernel,
	linux-arm-kernel, Nicolas Ferre, Claudiu Beznea

On 02/03/2026 17:03:10+0530, Balakrishnan Sambath wrote:
> AT91_WDT_WDDIS (bit 15) applies to SAMA5/AT91SAM9261 and
> AT91_SAM9X60_WDDIS (bit 12) to SAM9X60/SAMA7G5/SAM9X75. Update
> comments to reflect this and add SAMA7G5 and SAM9X75 datasheet
> references to the file header.
> 
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/watchdog/at91sam9_wdt.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/at91sam9_wdt.h b/drivers/watchdog/at91sam9_wdt.h
> index 298d545df1a1..2020694f8f6f 100644
> --- a/drivers/watchdog/at91sam9_wdt.h
> +++ b/drivers/watchdog/at91sam9_wdt.h
> @@ -3,40 +3,42 @@
>   * drivers/watchdog/at91sam9_wdt.h
>   *
>   * Copyright (C) 2007 Andrew Victor
>   * Copyright (C) 2007 Atmel Corporation.
>   * Copyright (C) 2019 Microchip Technology Inc. and its subsidiaries
>   *
>   * Watchdog Timer (WDT) - System peripherals regsters.
>   * Based on AT91SAM9261 datasheet revision D.
>   * Based on SAM9X60 datasheet.
> + * Based on SAMA7G5 datasheet.
> + * Based on SAM9X75 datasheet.
>   *
>   */
>  
>  #ifndef AT91_WDT_H
>  #define AT91_WDT_H
>  
>  #include <linux/bits.h>
>  
>  #define AT91_WDT_CR		0x00			/* Watchdog Control Register */
>  #define  AT91_WDT_WDRSTT	BIT(0)			/* Restart */
>  #define  AT91_WDT_KEY		(0xa5UL << 24)		/* KEY Password */
>  
>  #define AT91_WDT_MR		0x04			/* Watchdog Mode Register */
>  #define  AT91_WDT_WDV		(0xfffUL << 0)		/* Counter Value */
>  #define  AT91_WDT_SET_WDV(x)	((x) & AT91_WDT_WDV)
>  #define  AT91_SAM9X60_PERIODRST	BIT(4)		/* Period Reset */
>  #define  AT91_SAM9X60_RPTHRST	BIT(5)		/* Minimum Restart Period */
>  #define  AT91_WDT_WDFIEN	BIT(12)		/* Fault Interrupt Enable */
> -#define  AT91_SAM9X60_WDDIS	BIT(12)		/* Watchdog Disable */
> +#define  AT91_SAM9X60_WDDIS	BIT(12)		/* Watchdog Disable (SAM9X60, SAMA7G5, SAM9X75) */
>  #define  AT91_WDT_WDRSTEN	BIT(13)		/* Reset Processor */
>  #define  AT91_WDT_WDRPROC	BIT(14)		/* Timer Restart */
> -#define  AT91_WDT_WDDIS		BIT(15)		/* Watchdog Disable */
> +#define  AT91_WDT_WDDIS		BIT(15)		/* Watchdog Disable (SAMA5, AT91SAM9261) */
>  #define  AT91_WDT_WDD		(0xfffUL << 16)		/* Delta Value */
>  #define  AT91_WDT_SET_WDD(x)	(((x) << 16) & AT91_WDT_WDD)
>  #define  AT91_WDT_WDDBGHLT	BIT(28)		/* Debug Halt */
>  #define  AT91_WDT_WDIDLEHLT	BIT(29)		/* Idle Halt */
>  
>  #define AT91_WDT_SR		0x08		/* Watchdog Status Register */
>  #define  AT91_WDT_WDUNF		BIT(0)		/* Watchdog Underflow */
>  #define  AT91_WDT_WDERR		BIT(1)		/* Watchdog Error */
>  
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family
  2026-03-02 11:33 ` [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family Balakrishnan Sambath
  2026-03-02 14:32   ` Alexandre Belloni
@ 2026-05-03 21:03   ` Guenter Roeck
  1 sibling, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-05-03 21:03 UTC (permalink / raw)
  To: Balakrishnan Sambath
  Cc: linux-watchdog, wim, alexandre.belloni, andrei.simion,
	linux-kernel, linux-arm-kernel, Nicolas Ferre, Claudiu Beznea

On Mon, Mar 02, 2026 at 05:03:10PM +0530, Balakrishnan Sambath wrote:
> AT91_WDT_WDDIS (bit 15) applies to SAMA5/AT91SAM9261 and
> AT91_SAM9X60_WDDIS (bit 12) to SAM9X60/SAMA7G5/SAM9X75. Update
> comments to reflect this and add SAMA7G5 and SAM9X75 datasheet
> references to the file header.
> 
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

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

> ---
>  drivers/watchdog/at91sam9_wdt.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/at91sam9_wdt.h b/drivers/watchdog/at91sam9_wdt.h
> index 298d545df1a1..2020694f8f6f 100644
> --- a/drivers/watchdog/at91sam9_wdt.h
> +++ b/drivers/watchdog/at91sam9_wdt.h
> @@ -3,40 +3,42 @@
>   * drivers/watchdog/at91sam9_wdt.h
>   *
>   * Copyright (C) 2007 Andrew Victor
>   * Copyright (C) 2007 Atmel Corporation.
>   * Copyright (C) 2019 Microchip Technology Inc. and its subsidiaries
>   *
>   * Watchdog Timer (WDT) - System peripherals regsters.
>   * Based on AT91SAM9261 datasheet revision D.
>   * Based on SAM9X60 datasheet.
> + * Based on SAMA7G5 datasheet.
> + * Based on SAM9X75 datasheet.
>   *
>   */
>  
>  #ifndef AT91_WDT_H
>  #define AT91_WDT_H
>  
>  #include <linux/bits.h>
>  
>  #define AT91_WDT_CR		0x00			/* Watchdog Control Register */
>  #define  AT91_WDT_WDRSTT	BIT(0)			/* Restart */
>  #define  AT91_WDT_KEY		(0xa5UL << 24)		/* KEY Password */
>  
>  #define AT91_WDT_MR		0x04			/* Watchdog Mode Register */
>  #define  AT91_WDT_WDV		(0xfffUL << 0)		/* Counter Value */
>  #define  AT91_WDT_SET_WDV(x)	((x) & AT91_WDT_WDV)
>  #define  AT91_SAM9X60_PERIODRST	BIT(4)		/* Period Reset */
>  #define  AT91_SAM9X60_RPTHRST	BIT(5)		/* Minimum Restart Period */
>  #define  AT91_WDT_WDFIEN	BIT(12)		/* Fault Interrupt Enable */
> -#define  AT91_SAM9X60_WDDIS	BIT(12)		/* Watchdog Disable */
> +#define  AT91_SAM9X60_WDDIS	BIT(12)		/* Watchdog Disable (SAM9X60, SAMA7G5, SAM9X75) */
>  #define  AT91_WDT_WDRSTEN	BIT(13)		/* Reset Processor */
>  #define  AT91_WDT_WDRPROC	BIT(14)		/* Timer Restart */
> -#define  AT91_WDT_WDDIS		BIT(15)		/* Watchdog Disable */
> +#define  AT91_WDT_WDDIS		BIT(15)		/* Watchdog Disable (SAMA5, AT91SAM9261) */
>  #define  AT91_WDT_WDD		(0xfffUL << 16)		/* Delta Value */
>  #define  AT91_WDT_SET_WDD(x)	(((x) << 16) & AT91_WDT_WDD)
>  #define  AT91_WDT_WDDBGHLT	BIT(28)		/* Debug Halt */
>  #define  AT91_WDT_WDIDLEHLT	BIT(29)		/* Idle Halt */
>  
>  #define AT91_WDT_SR		0x08		/* Watchdog Status Register */
>  #define  AT91_WDT_WDUNF		BIT(0)		/* Watchdog Underflow */
>  #define  AT91_WDT_WDERR		BIT(1)		/* Watchdog Error */
>  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5
  2026-03-02 11:33 [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan Sambath
  2026-03-02 11:33 ` [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on " Balakrishnan Sambath
  2026-03-02 11:33 ` [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family Balakrishnan Sambath
@ 2026-05-27 13:04 ` Balakrishnan.S
  2 siblings, 0 replies; 7+ messages in thread
From: Balakrishnan.S @ 2026-05-27 13:04 UTC (permalink / raw)
  To: linux-watchdog
  Cc: wim, linux, alexandre.belloni, andrei.simion, linux-kernel,
	linux-arm-kernel

Hi Wim,

Gentle ping — both patches have been reviewed. Could this be queued for 
the next cycle?

Thanks,
Balakrishnan

On 02/03/26 5:03 pm, Balakrishnan Sambath wrote:
> The sama5d4_wdt driver hardcodes AT91_WDT_WDDIS (bit 15) for WDDIS
> detection, which is incorrect for SAM9X60 and SAMA7G5 that use bit 12
> (AT91_SAM9X60_WDDIS). This series fixes the detection by introducing
> a per-device wddis_mask and documents the bit position difference in
> the header.
> 
> Changes in v2:
> - Reorder patches: fix first, documentation second
> - Drop patch 3/3, not needed with wddis_mask approach
> - Keep AT91_SAM9X60_* register names, drop _LEGACY/_MODERN naming
> - Limit header changes to WDDIS comments and datasheet references
> 
> Balakrishnan Sambath (2):
>    watchdog: sama5d4_wdt: Fix WDDIS detection on SAM9X60 and SAMA7G5
>    watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family
> 
>   drivers/watchdog/at91sam9_wdt.h |  6 +++--
>   drivers/watchdog/sama5d4_wdt.c  | 48 +++++++++++++++------------------
>   2 files changed, 25 insertions(+), 29 deletions(-)
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-27 13:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 11:33 [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan Sambath
2026-03-02 11:33 ` [PATCH 1/2] watchdog: sama5d4_wdt: Fix WDDIS detection on " Balakrishnan Sambath
2026-03-02 14:32   ` Alexandre Belloni
2026-03-02 11:33 ` [PATCH 2/2] watchdog: at91sam9_wdt.h: Document WDDIS bit position per SoC family Balakrishnan Sambath
2026-03-02 14:32   ` Alexandre Belloni
2026-05-03 21:03   ` Guenter Roeck
2026-05-27 13:04 ` [PATCH v2 0/2] watchdog: sama5d4_wdt: Fix WDDIS handling for SAM9X60 and SAMA7G5 Balakrishnan.S

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.