mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] watchdog: aspeed: Add AST2700 support
@ 2025-10-07  8:36 Chin-Ting Kuo
  2025-10-07  8:36 ` [PATCH 1/3] dt-bindings: watchdog: aspeed,ast2400-wdt: Add support for AST2700 Chin-Ting Kuo
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Chin-Ting Kuo @ 2025-10-07  8:36 UTC (permalink / raw)
  To: wim, linux, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

This patch series improves the ASPEED watchdog driver to support newer
AST2700 SoC. It enhances reset mask handling to accommodate platforms
with multiple registers, adds AST2700-specific configuration, and
updates the device tree bindings accordingly.

Chin-Ting Kuo (3):
  dt-bindings: watchdog: aspeed,ast2400-wdt: Add support for AST2700
  watchdog: aspeed: Support variable number of reset mask registers
  watchdog: aspeed: Add support for AST2700 platform

 .../bindings/watchdog/aspeed,ast2400-wdt.yaml |   8 +-
 drivers/watchdog/aspeed_wdt.c                 |  31 +++-
 include/dt-bindings/watchdog/aspeed-wdt.h     | 138 ++++++++++++++++++
 3 files changed, 169 insertions(+), 8 deletions(-)

-- 
2.34.1


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

* [PATCH 1/3] dt-bindings: watchdog: aspeed,ast2400-wdt: Add support for AST2700
  2025-10-07  8:36 [PATCH 0/3] watchdog: aspeed: Add AST2700 support Chin-Ting Kuo
@ 2025-10-07  8:36 ` Chin-Ting Kuo
  2025-10-07  8:36 ` [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers Chin-Ting Kuo
  2025-10-07  8:36 ` [PATCH 3/3] watchdog: aspeed: Add support for AST2700 platform Chin-Ting Kuo
  2 siblings, 0 replies; 8+ messages in thread
From: Chin-Ting Kuo @ 2025-10-07  8:36 UTC (permalink / raw)
  To: wim, linux, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

Add support for the AST2700 SoC in the ASPEED watchdog device tree
bindings. This includes:

- Adding "aspeed,ast2700-wdt" to the compatible string list.
- Extending the "aspeed,reset-mask" property description for AST2700.
- Defining AST2700-specific reset mask bits in aspeed-wdt.h,
  covering RESET1 to RESET5.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 .../bindings/watchdog/aspeed,ast2400-wdt.yaml |   8 +-
 include/dt-bindings/watchdog/aspeed-wdt.h     | 138 ++++++++++++++++++
 2 files changed, 144 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
index be78a9865584..9322cb5b462a 100644
--- a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
@@ -15,6 +15,7 @@ properties:
       - aspeed,ast2400-wdt
       - aspeed,ast2500-wdt
       - aspeed,ast2600-wdt
+      - aspeed,ast2700-wdt
 
   reg:
     maxItems: 1
@@ -87,13 +88,15 @@ properties:
   aspeed,reset-mask:
     $ref: /schemas/types.yaml#/definitions/uint32-array
     minItems: 1
-    maxItems: 2
+    maxItems: 5
     description: >
       A bitmask indicating which peripherals will be reset if the watchdog
       timer expires. On AST2500 SoCs this should be a single word defined using
       the AST2500_WDT_RESET_* macros; on AST2600 SoCs this should be a two-word
       array with the first word defined using the AST2600_WDT_RESET1_* macros,
-      and the second word defined using the AST2600_WDT_RESET2_* macros.
+      and the second word defined using the AST2600_WDT_RESET2_* macros; on
+      AST2700 SoCs, this should be five-word array from AST2700_WDT_RESET1_*
+      macros to AST2700_WDT_RESET5_* macros.
 
 required:
   - compatible
@@ -114,6 +117,7 @@ allOf:
           enum:
             - aspeed,ast2500-wdt
             - aspeed,ast2600-wdt
+            - aspeed,ast2700-wdt
   - if:
       required:
         - aspeed,ext-active-high
diff --git a/include/dt-bindings/watchdog/aspeed-wdt.h b/include/dt-bindings/watchdog/aspeed-wdt.h
index 7ae6d84b2bd9..89fa31ffce2d 100644
--- a/include/dt-bindings/watchdog/aspeed-wdt.h
+++ b/include/dt-bindings/watchdog/aspeed-wdt.h
@@ -89,4 +89,142 @@
 
 #define AST2600_WDT_RESET2_DEFAULT 0x03fffff1
 
+#define AST2700_WDT_RESET1_CPU		(1 << 0)
+#define AST2700_WDT_RESET1_DRAM		(1 << 1)
+#define AST2700_WDT_RESET1_SLI0		(1 << 2)
+#define AST2700_WDT_RESET1_EHCI		(1 << 3)
+#define AST2700_WDT_RESET1_HACE		(1 << 4)
+#define AST2700_WDT_RESET1_SOC_MISC0	(1 << 5)
+#define AST2700_WDT_RESET1_VIDEO	(1 << 6)
+#define AST2700_WDT_RESET1_2D_GRAPHIC	(1 << 7)
+#define AST2700_WDT_RESET1_RAVS0	(1 << 8)
+#define AST2700_WDT_RESET1_RAVS1	(1 << 9)
+#define AST2700_WDT_RESET1_GPIO0	(1 << 10)
+#define AST2700_WDT_RESET1_SSP		(1 << 11)
+#define AST2700_WDT_RESET1_TSP		(1 << 12)
+#define AST2700_WDT_RESET1_CRT		(1 << 13)
+#define AST2700_WDT_RESET1_USB20_HOST	(1 << 14)
+#define AST2700_WDT_RESET1_USB11_HOST	(1 << 15)
+#define AST2700_WDT_RESET1_UFS		(1 << 16)
+#define AST2700_WDT_RESET1_EMMC		(1 << 17)
+#define AST2700_WDT_RESET1_AHB_TO_PCIE1	(1 << 18)
+#define AST2700_WDT_RESET1_XDMA0	(1 << 22)
+#define AST2700_WDT_RESET1_MCTP1	(1 << 23)
+#define AST2700_WDT_RESET1_MCTP0	(1 << 24)
+#define AST2700_WDT_RESET1_JTAG0	(1 << 25)
+#define AST2700_WDT_RESET1_ECC		(1 << 26)
+#define AST2700_WDT_RESET1_XDMA1	(1 << 27)
+#define AST2700_WDT_RESET1_DP		(1 << 28)
+#define AST2700_WDT_RESET1_DP_MCU	(1 << 29)
+#define AST2700_WDT_RESET1_AHB_TO_PCIE0	(1 << 31)
+
+#define AST2700_WDT_RESET1_DEFAULT 0x8207ff71
+
+#define AST2700_WDT_RESET2_USB3_A_HOST	(1 << 0)
+#define AST2700_WDT_RESET2_USB3_A_VHUB3	(1 << 1)
+#define AST2700_WDT_RESET2_USB3_A_VHUB2	(1 << 2)
+#define AST2700_WDT_RESET2_USB3_B_HOST	(1 << 3)
+#define AST2700_WDT_RESET2_USB3_B_VHUB3	(1 << 4)
+#define AST2700_WDT_RESET2_USB3_B_VHUB2	(1 << 5)
+#define AST2700_WDT_RESET2_SM3		(1 << 6)
+#define AST2700_WDT_RESET2_SM4		(1 << 7)
+#define AST2700_WDT_RESET2_SHA3		(1 << 8)
+#define AST2700_WDT_RESET2_RSA		(1 << 9)
+
+#define AST2700_WDT_RESET2_DEFAULT 0x000003f6
+
+#define AST2700_WDT_RESET3_LPC0		(1 << 0)
+#define AST2700_WDT_RESET3_LPC1		(1 << 1)
+#define AST2700_WDT_RESET3_MDIO		(1 << 2)
+#define AST2700_WDT_RESET3_PECI		(1 << 3)
+#define AST2700_WDT_RESET3_PWM		(1 << 4)
+#define AST2700_WDT_RESET3_MAC0		(1 << 5)
+#define AST2700_WDT_RESET3_MAC1		(1 << 6)
+#define AST2700_WDT_RESET3_MAC2		(1 << 7)
+#define AST2700_WDT_RESET3_ADC		(1 << 8)
+#define AST2700_WDT_RESET3_SDC		(1 << 9)
+#define AST2700_WDT_RESET3_ESPI0	(1 << 10)
+#define AST2700_WDT_RESET3_ESPI1	(1 << 11)
+#define AST2700_WDT_RESET3_JTAG1	(1 << 12)
+#define AST2700_WDT_RESET3_SPI0		(1 << 13)
+#define AST2700_WDT_RESET3_SPI1		(1 << 14)
+#define AST2700_WDT_RESET3_SPI2		(1 << 15)
+#define AST2700_WDT_RESET3_I3C0		(1 << 16)
+#define AST2700_WDT_RESET3_I3C1		(1 << 17)
+#define AST2700_WDT_RESET3_I3C2		(1 << 18)
+#define AST2700_WDT_RESET3_I3C3		(1 << 19)
+#define AST2700_WDT_RESET3_I3C4		(1 << 20)
+#define AST2700_WDT_RESET3_I3C5		(1 << 21)
+#define AST2700_WDT_RESET3_I3C6		(1 << 22)
+#define AST2700_WDT_RESET3_I3C7		(1 << 23)
+#define AST2700_WDT_RESET3_I3C8		(1 << 24)
+#define AST2700_WDT_RESET3_I3C9		(1 << 25)
+#define AST2700_WDT_RESET3_I3C10	(1 << 26)
+#define AST2700_WDT_RESET3_I3C11	(1 << 27)
+#define AST2700_WDT_RESET3_I3C12	(1 << 28)
+#define AST2700_WDT_RESET3_I3C13	(1 << 29)
+#define AST2700_WDT_RESET3_I3C14	(1 << 30)
+#define AST2700_WDT_RESET3_I3C15	(1 << 31)
+
+#define AST2700_WDT_RESET3_DEFAULT 0x000093ec
+
+#define AST2700_WDT_RESET4_FMC		(1 << 0)
+#define AST2700_WDT_RESET4_SOC_MISC1	(1 << 1)
+#define AST2700_WDT_RESET4_AHB		(1 << 2)
+#define AST2700_WDT_RESET4_SLI1		(1 << 3)
+#define AST2700_WDT_RESET4_UART0	(1 << 4)
+#define AST2700_WDT_RESET4_UART1	(1 << 5)
+#define AST2700_WDT_RESET4_UART2	(1 << 6)
+#define AST2700_WDT_RESET4_UART3	(1 << 7)
+#define AST2700_WDT_RESET4_I2C_MONITOR	(1 << 8)
+#define AST2700_WDT_RESET4_HOST_TO_SPI1	(1 << 9)
+#define AST2700_WDT_RESET4_HOST_TO_SPI2	(1 << 10)
+#define AST2700_WDT_RESET4_GPIO1	(1 << 11)
+#define AST2700_WDT_RESET4_FSI		(1 << 12)
+#define AST2700_WDT_RESET4_CANBUS	(1 << 13)
+#define AST2700_WDT_RESET4_MCTP		(1 << 14)
+#define AST2700_WDT_RESET4_XDMA		(1 << 15)
+#define AST2700_WDT_RESET4_UART5	(1 << 16)
+#define AST2700_WDT_RESET4_UART6	(1 << 17)
+#define AST2700_WDT_RESET4_UART7	(1 << 18)
+#define AST2700_WDT_RESET4_UART8	(1 << 19)
+#define AST2700_WDT_RESET4_BOOT_MCU	(1 << 20)
+#define AST2700_WDT_RESET4_IO_MCU	(1 << 21)
+#define AST2700_WDT_RESET4_LTPI0	(1 << 22)
+#define AST2700_WDT_RESET4_VGA_LINK	(1 << 23)
+#define AST2700_WDT_RESET4_LTPI1	(1 << 24)
+#define AST2700_WDT_RESET4_LTPI_PHY	(1 << 25)
+#define AST2700_WDT_RESET4_ACE		(1 << 26)
+#define AST2700_WDT_RESET4_LTPI_GPIO0	(1 << 28)
+#define AST2700_WDT_RESET4_LTPI_GPIO1	(1 << 29)
+#define AST2700_WDT_RESET4_AHB_TO_PCIE1	(1 << 30)
+#define AST2700_WDT_RESET4_I3C_DMA	(1 << 31)
+
+#define AST2700_WDT_RESET4_DEFAULT 0x40303803
+
+#define AST2700_WDT_RESET5_I2C_GLOBAL	(1 << 0)
+#define AST2700_WDT_RESET5_I2C0		(1 << 1)
+#define AST2700_WDT_RESET5_I2C1		(1 << 2)
+#define AST2700_WDT_RESET5_I2C2		(1 << 3)
+#define AST2700_WDT_RESET5_I2C3		(1 << 4)
+#define AST2700_WDT_RESET5_I2C4		(1 << 5)
+#define AST2700_WDT_RESET5_I2C5		(1 << 6)
+#define AST2700_WDT_RESET5_I2C6		(1 << 7)
+#define AST2700_WDT_RESET5_I2C7		(1 << 8)
+#define AST2700_WDT_RESET5_I2C8		(1 << 9)
+#define AST2700_WDT_RESET5_I2C9		(1 << 10)
+#define AST2700_WDT_RESET5_I2C10	(1 << 11)
+#define AST2700_WDT_RESET5_I2C11	(1 << 12)
+#define AST2700_WDT_RESET5_I2C12	(1 << 13)
+#define AST2700_WDT_RESET5_I2C13	(1 << 14)
+#define AST2700_WDT_RESET5_I2C14	(1 << 15)
+#define AST2700_WDT_RESET5_I2C15	(1 << 16)
+#define AST2700_WDT_RESET5_UHCI		(1 << 17)
+#define AST2700_WDT_RESET5_USB2_C_UART	(1 << 18)
+#define AST2700_WDT_RESET5_USB2_C	(1 << 19)
+#define AST2700_WDT_RESET5_USB2_D_UART	(1 << 20)
+#define AST2700_WDT_RESET5_USB2_D	(1 << 21)
+
+#define AST2700_WDT_RESET5_DEFAULT 0x00320000
+
 #endif
-- 
2.34.1


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

* [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers
  2025-10-07  8:36 [PATCH 0/3] watchdog: aspeed: Add AST2700 support Chin-Ting Kuo
  2025-10-07  8:36 ` [PATCH 1/3] dt-bindings: watchdog: aspeed,ast2400-wdt: Add support for AST2700 Chin-Ting Kuo
@ 2025-10-07  8:36 ` Chin-Ting Kuo
  2025-10-07 14:46   ` Guenter Roeck
  2025-10-07 14:55   ` Guenter Roeck
  2025-10-07  8:36 ` [PATCH 3/3] watchdog: aspeed: Add support for AST2700 platform Chin-Ting Kuo
  2 siblings, 2 replies; 8+ messages in thread
From: Chin-Ting Kuo @ 2025-10-07  8:36 UTC (permalink / raw)
  To: wim, linux, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

Starting from the AST2600 platform, the SoC design has become more
complex, with an increased number of reset mask registers.
To support this, introduce a new field 'num_reset_masks' in the
'aspeed_wdt_config' structure to specify the number of reset mask
registers per platform. This change removes the need for hardcoded
platform-specific logic and improves scalability for future SoCs.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/watchdog/aspeed_wdt.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index 837e15701c0e..e15f70c5e416 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -35,6 +35,7 @@ struct aspeed_wdt_config {
 	u32 irq_shift;
 	u32 irq_mask;
 	struct aspeed_wdt_scu scu;
+	u32 num_reset_masks;
 };
 
 struct aspeed_wdt {
@@ -54,6 +55,7 @@ static const struct aspeed_wdt_config ast2400_config = {
 		.wdt_reset_mask = 0x1,
 		.wdt_reset_mask_shift = 1,
 	},
+	.num_reset_masks = 1,
 };
 
 static const struct aspeed_wdt_config ast2500_config = {
@@ -66,6 +68,7 @@ static const struct aspeed_wdt_config ast2500_config = {
 		.wdt_reset_mask = 0x1,
 		.wdt_reset_mask_shift = 2,
 	},
+	.num_reset_masks = 1,
 };
 
 static const struct aspeed_wdt_config ast2600_config = {
@@ -78,6 +81,7 @@ static const struct aspeed_wdt_config ast2600_config = {
 		.wdt_reset_mask = 0xf,
 		.wdt_reset_mask_shift = 16,
 	},
+	.num_reset_masks = 2,
 };
 
 static const struct of_device_id aspeed_wdt_of_table[] = {
@@ -482,8 +486,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
 		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
 		u32 reset_mask[2];
-		size_t nrstmask = of_device_is_compatible(np, "aspeed,ast2600-wdt") ? 2 : 1;
+		size_t nrstmask = wdt->cfg->num_reset_masks;
 		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
+		int i;
 
 		reg &= wdt->cfg->ext_pulse_width_mask;
 		if (of_property_read_bool(np, "aspeed,ext-active-high"))
@@ -503,9 +508,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 
 		ret = of_property_read_u32_array(np, "aspeed,reset-mask", reset_mask, nrstmask);
 		if (!ret) {
-			writel(reset_mask[0], wdt->base + WDT_RESET_MASK1);
-			if (nrstmask > 1)
-				writel(reset_mask[1], wdt->base + WDT_RESET_MASK2);
+			for (i = 0; i < nrstmask; i++)
+				writel(reset_mask[i], wdt->base + WDT_RESET_MASK1 + i * 4);
 		}
 	}
 
-- 
2.34.1


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

* [PATCH 3/3] watchdog: aspeed: Add support for AST2700 platform
  2025-10-07  8:36 [PATCH 0/3] watchdog: aspeed: Add AST2700 support Chin-Ting Kuo
  2025-10-07  8:36 ` [PATCH 1/3] dt-bindings: watchdog: aspeed,ast2400-wdt: Add support for AST2700 Chin-Ting Kuo
  2025-10-07  8:36 ` [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers Chin-Ting Kuo
@ 2025-10-07  8:36 ` Chin-Ting Kuo
  2 siblings, 0 replies; 8+ messages in thread
From: Chin-Ting Kuo @ 2025-10-07  8:36 UTC (permalink / raw)
  To: wim, linux, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

Add AST2700 platform support to the ASPEED watchdog driver. This includes
a new per-platform configuration with SCU reset status register at
SCU1_070 and support for 5 reset mask registers.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
---
 drivers/watchdog/aspeed_wdt.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index e15f70c5e416..f2245a28ac10 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -84,10 +84,24 @@ static const struct aspeed_wdt_config ast2600_config = {
 	.num_reset_masks = 2,
 };
 
+static const struct aspeed_wdt_config ast2700_config = {
+	.ext_pulse_width_mask = 0xfffff,
+	.irq_shift = 0,
+	.irq_mask = GENMASK(31, 10),
+	.scu = {
+		.compatible = "aspeed,ast2700-scu0",
+		.reset_status_reg = 0x70,
+		.wdt_reset_mask = 0xf,
+		.wdt_reset_mask_shift = 0,
+	},
+	.num_reset_masks = 5,
+};
+
 static const struct of_device_id aspeed_wdt_of_table[] = {
 	{ .compatible = "aspeed,ast2400-wdt", .data = &ast2400_config },
 	{ .compatible = "aspeed,ast2500-wdt", .data = &ast2500_config },
 	{ .compatible = "aspeed,ast2600-wdt", .data = &ast2600_config },
+	{ .compatible = "aspeed,ast2700-wdt", .data = &ast2700_config },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
@@ -484,8 +498,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
 	}
 
 	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
-		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
-		u32 reset_mask[2];
+	    (of_device_is_compatible(np, "aspeed,ast2600-wdt")) ||
+	    (of_device_is_compatible(np, "aspeed,ast2700-wdt"))) {
+		u32 reset_mask[5];
 		size_t nrstmask = wdt->cfg->num_reset_masks;
 		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
 		int i;
-- 
2.34.1


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

* Re: [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers
  2025-10-07  8:36 ` [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers Chin-Ting Kuo
@ 2025-10-07 14:46   ` Guenter Roeck
  2025-10-07 14:55   ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2025-10-07 14:46 UTC (permalink / raw)
  To: Chin-Ting Kuo, wim, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On 10/7/25 01:36, Chin-Ting Kuo wrote:
> Starting from the AST2600 platform, the SoC design has become more
> complex, with an increased number of reset mask registers.
> To support this, introduce a new field 'num_reset_masks' in the
> 'aspeed_wdt_config' structure to specify the number of reset mask
> registers per platform. This change removes the need for hardcoded
> platform-specific logic and improves scalability for future SoCs.
> 
> 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, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 837e15701c0e..e15f70c5e416 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -35,6 +35,7 @@ struct aspeed_wdt_config {
>   	u32 irq_shift;
>   	u32 irq_mask;
>   	struct aspeed_wdt_scu scu;
> +	u32 num_reset_masks;
>   };
>   
>   struct aspeed_wdt {
> @@ -54,6 +55,7 @@ static const struct aspeed_wdt_config ast2400_config = {
>   		.wdt_reset_mask = 0x1,
>   		.wdt_reset_mask_shift = 1,
>   	},
> +	.num_reset_masks = 1,
>   };
>   
>   static const struct aspeed_wdt_config ast2500_config = {
> @@ -66,6 +68,7 @@ static const struct aspeed_wdt_config ast2500_config = {
>   		.wdt_reset_mask = 0x1,
>   		.wdt_reset_mask_shift = 2,
>   	},
> +	.num_reset_masks = 1,
>   };
>   
>   static const struct aspeed_wdt_config ast2600_config = {
> @@ -78,6 +81,7 @@ static const struct aspeed_wdt_config ast2600_config = {
>   		.wdt_reset_mask = 0xf,
>   		.wdt_reset_mask_shift = 16,
>   	},
> +	.num_reset_masks = 2,
>   };
>   
>   static const struct of_device_id aspeed_wdt_of_table[] = {
> @@ -482,8 +486,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>   	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
>   		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
>   		u32 reset_mask[2];
> -		size_t nrstmask = of_device_is_compatible(np, "aspeed,ast2600-wdt") ? 2 : 1;
> +		size_t nrstmask = wdt->cfg->num_reset_masks;
>   		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
> +		int i;
>   
>   		reg &= wdt->cfg->ext_pulse_width_mask;
>   		if (of_property_read_bool(np, "aspeed,ext-active-high"))
> @@ -503,9 +508,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>   
>   		ret = of_property_read_u32_array(np, "aspeed,reset-mask", reset_mask, nrstmask);
>   		if (!ret) {
> -			writel(reset_mask[0], wdt->base + WDT_RESET_MASK1);
> -			if (nrstmask > 1)
> -				writel(reset_mask[1], wdt->base + WDT_RESET_MASK2);
> +			for (i = 0; i < nrstmask; i++)
> +				writel(reset_mask[i], wdt->base + WDT_RESET_MASK1 + i * 4);
>   		}
>   	}
>   


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

* Re: [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers
  2025-10-07  8:36 ` [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers Chin-Ting Kuo
  2025-10-07 14:46   ` Guenter Roeck
@ 2025-10-07 14:55   ` Guenter Roeck
  2025-10-08  3:29     ` Chin-Ting Kuo
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2025-10-07 14:55 UTC (permalink / raw)
  To: Chin-Ting Kuo, wim, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On 10/7/25 01:36, Chin-Ting Kuo wrote:
> Starting from the AST2600 platform, the SoC design has become more
> complex, with an increased number of reset mask registers.
> To support this, introduce a new field 'num_reset_masks' in the
> 'aspeed_wdt_config' structure to specify the number of reset mask
> registers per platform. This change removes the need for hardcoded
> platform-specific logic and improves scalability for future SoCs.
> 
> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> ---
>   drivers/watchdog/aspeed_wdt.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 837e15701c0e..e15f70c5e416 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -35,6 +35,7 @@ struct aspeed_wdt_config {
>   	u32 irq_shift;
>   	u32 irq_mask;
>   	struct aspeed_wdt_scu scu;
> +	u32 num_reset_masks;
>   };
>   
>   struct aspeed_wdt {
> @@ -54,6 +55,7 @@ static const struct aspeed_wdt_config ast2400_config = {
>   		.wdt_reset_mask = 0x1,
>   		.wdt_reset_mask_shift = 1,
>   	},
> +	.num_reset_masks = 1,

Looking at this again: Why set it on ast2400 ?

>   };
>   
>   static const struct aspeed_wdt_config ast2500_config = {
> @@ -66,6 +68,7 @@ static const struct aspeed_wdt_config ast2500_config = {
>   		.wdt_reset_mask = 0x1,
>   		.wdt_reset_mask_shift = 2,
>   	},
> +	.num_reset_masks = 1,
>   };
>   
>   static const struct aspeed_wdt_config ast2600_config = {
> @@ -78,6 +81,7 @@ static const struct aspeed_wdt_config ast2600_config = {
>   		.wdt_reset_mask = 0xf,
>   		.wdt_reset_mask_shift = 16,
>   	},
> +	.num_reset_masks = 2,
>   };
>   
>   static const struct of_device_id aspeed_wdt_of_table[] = {
> @@ -482,8 +486,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>   	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
>   		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {

... because the code here only evaluates it if this is an ast2500 or ast2600.

If num_reset_masks would be set to 0 for ast2400, the value could be used here.

	if (wdt->cfg->num_reset_masks) {
		...
	}

and it would not be necessary to add of_device_is_compatible() for new chips.

Guenter

>   		u32 reset_mask[2];
> -		size_t nrstmask = of_device_is_compatible(np, "aspeed,ast2600-wdt") ? 2 : 1;
> +		size_t nrstmask = wdt->cfg->num_reset_masks;
>   		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
> +		int i;
>   
>   		reg &= wdt->cfg->ext_pulse_width_mask;
>   		if (of_property_read_bool(np, "aspeed,ext-active-high"))
> @@ -503,9 +508,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>   
>   		ret = of_property_read_u32_array(np, "aspeed,reset-mask", reset_mask, nrstmask);
>   		if (!ret) {
> -			writel(reset_mask[0], wdt->base + WDT_RESET_MASK1);
> -			if (nrstmask > 1)
> -				writel(reset_mask[1], wdt->base + WDT_RESET_MASK2);
> +			for (i = 0; i < nrstmask; i++)
> +				writel(reset_mask[i], wdt->base + WDT_RESET_MASK1 + i * 4);
>   		}
>   	}
>   


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

* RE: [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers
  2025-10-07 14:55   ` Guenter Roeck
@ 2025-10-08  3:29     ` Chin-Ting Kuo
  2025-10-08  4:21       ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Chin-Ting Kuo @ 2025-10-08  3:29 UTC (permalink / raw)
  To: Guenter Roeck, wim, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

Hi Guenter,

Thanks for the quick review.

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Tuesday, October 7, 2025 10:55 PM
> Subject: Re: [PATCH 2/3] watchdog: aspeed: Support variable number of reset
> mask registers
> 
> On 10/7/25 01:36, Chin-Ting Kuo wrote:
> > Starting from the AST2600 platform, the SoC design has become more
> > complex, with an increased number of reset mask registers.
> > To support this, introduce a new field 'num_reset_masks' in the
> > 'aspeed_wdt_config' structure to specify the number of reset mask
> > registers per platform. This change removes the need for hardcoded
> > platform-specific logic and improves scalability for future SoCs.
> >
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
> > ---
> >   drivers/watchdog/aspeed_wdt.c | 12 ++++++++----
> >   1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/watchdog/aspeed_wdt.c
> > b/drivers/watchdog/aspeed_wdt.c index 837e15701c0e..e15f70c5e416
> > 100644
> > --- a/drivers/watchdog/aspeed_wdt.c
> > +++ b/drivers/watchdog/aspeed_wdt.c
> > @@ -35,6 +35,7 @@ struct aspeed_wdt_config {
> >   	u32 irq_shift;
> >   	u32 irq_mask;
> >   	struct aspeed_wdt_scu scu;
> > +	u32 num_reset_masks;
> >   };
> >
> >   struct aspeed_wdt {
> > @@ -54,6 +55,7 @@ static const struct aspeed_wdt_config ast2400_config =
> {
> >   		.wdt_reset_mask = 0x1,
> >   		.wdt_reset_mask_shift = 1,
> >   	},
> > +	.num_reset_masks = 1,
> 
> Looking at this again: Why set it on ast2400 ?
> 

This should be removed for AST2400 platform. I will update it in the next patch version.

> >   };
> >
> >   static const struct aspeed_wdt_config ast2500_config = { @@ -66,6
> > +68,7 @@ static const struct aspeed_wdt_config ast2500_config = {
> >   		.wdt_reset_mask = 0x1,
> >   		.wdt_reset_mask_shift = 2,
> >   	},
> > +	.num_reset_masks = 1,
> >   };
> >
> >   static const struct aspeed_wdt_config ast2600_config = { @@ -78,6
> > +81,7 @@ static const struct aspeed_wdt_config ast2600_config = {
> >   		.wdt_reset_mask = 0xf,
> >   		.wdt_reset_mask_shift = 16,
> >   	},
> > +	.num_reset_masks = 2,
> >   };
> >
> >   static const struct of_device_id aspeed_wdt_of_table[] = { @@ -482,8
> > +486,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> >   	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
> >   		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
> 
> ... because the code here only evaluates it if this is an ast2500 or ast2600.
> 
> If num_reset_masks would be set to 0 for ast2400, the value could be used
> here.
> 
> 	if (wdt->cfg->num_reset_masks) {
> 		...
> 	}
> 
> and it would not be necessary to add of_device_is_compatible() for new chips.
> 

This "if" conditional statement includes not only reset mask configuration but also pulse polarity and driving type of reset output signal.
How about changing this "if" statement to the below one?
	if (!of_device_is_compatible(np, "aspeed,ast2400-wdt")) {
		...
	}
It will also not need to add of_device_is_compatible() for new chips.

> Guenter
> 
> >   		u32 reset_mask[2];
> > -		size_t nrstmask = of_device_is_compatible(np,
> "aspeed,ast2600-wdt") ? 2 : 1;
> > +		size_t nrstmask = wdt->cfg->num_reset_masks;
> >   		u32 reg = readl(wdt->base + WDT_RESET_WIDTH);
> > +		int i;
> >
> >   		reg &= wdt->cfg->ext_pulse_width_mask;
> >   		if (of_property_read_bool(np, "aspeed,ext-active-high")) @@
> -503,9
> > +508,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> >
> >   		ret = of_property_read_u32_array(np, "aspeed,reset-mask",
> reset_mask, nrstmask);
> >   		if (!ret) {
> > -			writel(reset_mask[0], wdt->base + WDT_RESET_MASK1);
> > -			if (nrstmask > 1)
> > -				writel(reset_mask[1], wdt->base + WDT_RESET_MASK2);
> > +			for (i = 0; i < nrstmask; i++)
> > +				writel(reset_mask[i], wdt->base + WDT_RESET_MASK1 + i
> * 4);
> >   		}
> >   	}
> >

Chin-Ting

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

* Re: [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers
  2025-10-08  3:29     ` Chin-Ting Kuo
@ 2025-10-08  4:21       ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2025-10-08  4:21 UTC (permalink / raw)
  To: Chin-Ting Kuo, wim, robh, krzk+dt, conor+dt, joel, andrew,
	linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On 10/7/25 20:29, Chin-Ting Kuo wrote:
> Hi Guenter,
> 
> Thanks for the quick review.
> 
>> -----Original Message-----
>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>> Sent: Tuesday, October 7, 2025 10:55 PM
>> Subject: Re: [PATCH 2/3] watchdog: aspeed: Support variable number of reset
>> mask registers
>>
>> On 10/7/25 01:36, Chin-Ting Kuo wrote:
>>> Starting from the AST2600 platform, the SoC design has become more
>>> complex, with an increased number of reset mask registers.
>>> To support this, introduce a new field 'num_reset_masks' in the
>>> 'aspeed_wdt_config' structure to specify the number of reset mask
>>> registers per platform. This change removes the need for hardcoded
>>> platform-specific logic and improves scalability for future SoCs.
>>>
>>> Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
>>> ---
>>>    drivers/watchdog/aspeed_wdt.c | 12 ++++++++----
>>>    1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/aspeed_wdt.c
>>> b/drivers/watchdog/aspeed_wdt.c index 837e15701c0e..e15f70c5e416
>>> 100644
>>> --- a/drivers/watchdog/aspeed_wdt.c
>>> +++ b/drivers/watchdog/aspeed_wdt.c
>>> @@ -35,6 +35,7 @@ struct aspeed_wdt_config {
>>>    	u32 irq_shift;
>>>    	u32 irq_mask;
>>>    	struct aspeed_wdt_scu scu;
>>> +	u32 num_reset_masks;
>>>    };
>>>
>>>    struct aspeed_wdt {
>>> @@ -54,6 +55,7 @@ static const struct aspeed_wdt_config ast2400_config =
>> {
>>>    		.wdt_reset_mask = 0x1,
>>>    		.wdt_reset_mask_shift = 1,
>>>    	},
>>> +	.num_reset_masks = 1,
>>
>> Looking at this again: Why set it on ast2400 ?
>>
> 
> This should be removed for AST2400 platform. I will update it in the next patch version.
> 
>>>    };
>>>
>>>    static const struct aspeed_wdt_config ast2500_config = { @@ -66,6
>>> +68,7 @@ static const struct aspeed_wdt_config ast2500_config = {
>>>    		.wdt_reset_mask = 0x1,
>>>    		.wdt_reset_mask_shift = 2,
>>>    	},
>>> +	.num_reset_masks = 1,
>>>    };
>>>
>>>    static const struct aspeed_wdt_config ast2600_config = { @@ -78,6
>>> +81,7 @@ static const struct aspeed_wdt_config ast2600_config = {
>>>    		.wdt_reset_mask = 0xf,
>>>    		.wdt_reset_mask_shift = 16,
>>>    	},
>>> +	.num_reset_masks = 2,
>>>    };
>>>
>>>    static const struct of_device_id aspeed_wdt_of_table[] = { @@ -482,8
>>> +486,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>>>    	if ((of_device_is_compatible(np, "aspeed,ast2500-wdt")) ||
>>>    		(of_device_is_compatible(np, "aspeed,ast2600-wdt"))) {
>>
>> ... because the code here only evaluates it if this is an ast2500 or ast2600.
>>
>> If num_reset_masks would be set to 0 for ast2400, the value could be used
>> here.
>>
>> 	if (wdt->cfg->num_reset_masks) {
>> 		...
>> 	}
>>
>> and it would not be necessary to add of_device_is_compatible() for new chips.
>>
> 
> This "if" conditional statement includes not only reset mask configuration but also pulse polarity and driving type of reset output signal.
> How about changing this "if" statement to the below one?
> 	if (!of_device_is_compatible(np, "aspeed,ast2400-wdt")) {
> 		...
> 	}
> It will also not need to add of_device_is_compatible() for new chips.
> 

Ok..

Thanks,
Guenter


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

end of thread, other threads:[~2025-10-08  4:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-07  8:36 [PATCH 0/3] watchdog: aspeed: Add AST2700 support Chin-Ting Kuo
2025-10-07  8:36 ` [PATCH 1/3] dt-bindings: watchdog: aspeed,ast2400-wdt: Add support for AST2700 Chin-Ting Kuo
2025-10-07  8:36 ` [PATCH 2/3] watchdog: aspeed: Support variable number of reset mask registers Chin-Ting Kuo
2025-10-07 14:46   ` Guenter Roeck
2025-10-07 14:55   ` Guenter Roeck
2025-10-08  3:29     ` Chin-Ting Kuo
2025-10-08  4:21       ` Guenter Roeck
2025-10-07  8:36 ` [PATCH 3/3] watchdog: aspeed: Add support for AST2700 platform 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

all inboxes | Powered by JetHome®