diff mbox series

watchdog: aspeed: Add sysfs attributes for reset mask bits

Message ID 20230922013542.29136-2-zev@bewilderbeest.net
State New
Headers show
Series watchdog: aspeed: Add sysfs attributes for reset mask bits | expand

Commit Message

Zev Weiss Sept. 22, 2023, 1:35 a.m. UTC 
The AST2500 and AST2600 watchdog timers provide the ability to control
which devices are reset by the watchdog timer via a reset mask
resgister.  Previously the driver ignored that register, leaving
whatever configuration it found at boot and offering no way of
altering its settings.  Add a 'reset_ctrl' sysfs subdirectory with a
file per bit so that userspace can determine which devices the reset
is applied to.

Note that not all bits in the hardware register are exposed -- in
particular, the ARM CPU and SOC/misc reset bits are left hidden since
clearing them can render the system unable to reboot.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---

I'm porting OpenBMC to a platform that requires that the LPC controller remain
un-reset by a BMC reboot.  With this patch userspace can control the reset
mask of the Aspeed watchdog timer, with a few bits remaining unexposed so as
to prevent some almost-certainly undesirable situations.  If there are other
bits that people feel shouldn't be exposed (or conversely if someone feels
strongly that the "dangerous" bits _should_ be exposed) I can adjust
accordingly.

Also, I was a little unsure about appropriately-concise names for some of the
bits, and am not hugely attached to the ones currently in this patch, so
suggestions on better alternatives there would also be welcome.

I've tested this on ast2500 hardware and a qemu ast2600 model (since I don't
have any ast2600 hardware) and it appears to be working as intended, but if
anyone can verify on actual ast2600 hardware that would of course be nice to
confirm.

 .../ABI/testing/sysfs-class-watchdog          |  10 +
 drivers/watchdog/aspeed_wdt.c                 | 290 ++++++++++++++++--
 2 files changed, 272 insertions(+), 28 deletions(-)

Comments

Andrew Jeffery Sept. 22, 2023, 4:16 a.m. UTC | #1 
On Thu, 2023-09-21 at 18:35 -0700, Zev Weiss wrote:
> The AST2500 and AST2600 watchdog timers provide the ability to control
> which devices are reset by the watchdog timer via a reset mask
> resgister.  Previously the driver ignored that register, leaving
> whatever configuration it found at boot and offering no way of
> altering its settings.  Add a 'reset_ctrl' sysfs subdirectory with a
> file per bit so that userspace can determine which devices the reset
> is applied to.
> 
> Note that not all bits in the hardware register are exposed -- in
> particular, the ARM CPU and SOC/misc reset bits are left hidden since
> clearing them can render the system unable to reboot.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> 
> I'm porting OpenBMC to a platform that requires that the LPC controller remain
> un-reset by a BMC reboot.  With this patch userspace can control the reset
> mask of the Aspeed watchdog timer, with a few bits remaining unexposed so as
> to prevent some almost-certainly undesirable situations.  If there are other
> bits that people feel shouldn't be exposed (or conversely if someone feels
> strongly that the "dangerous" bits _should_ be exposed) I can adjust
> accordingly.


Is there a reason this has to be managed by userspace? It sounds a lot
like a property of platform design, in which case exposing this feature
in the devicetree might be a better approach.

Andrew
Guenter Roeck Sept. 22, 2023, 4:24 a.m. UTC | #2 
On 9/21/23 21:16, Andrew Jeffery wrote:
> On Thu, 2023-09-21 at 18:35 -0700, Zev Weiss wrote:
>> The AST2500 and AST2600 watchdog timers provide the ability to control
>> which devices are reset by the watchdog timer via a reset mask
>> resgister.  Previously the driver ignored that register, leaving
>> whatever configuration it found at boot and offering no way of
>> altering its settings.  Add a 'reset_ctrl' sysfs subdirectory with a
>> file per bit so that userspace can determine which devices the reset
>> is applied to.
>>
>> Note that not all bits in the hardware register are exposed -- in
>> particular, the ARM CPU and SOC/misc reset bits are left hidden since
>> clearing them can render the system unable to reboot.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>>
>> I'm porting OpenBMC to a platform that requires that the LPC controller remain
>> un-reset by a BMC reboot.  With this patch userspace can control the reset
>> mask of the Aspeed watchdog timer, with a few bits remaining unexposed so as
>> to prevent some almost-certainly undesirable situations.  If there are other
>> bits that people feel shouldn't be exposed (or conversely if someone feels
>> strongly that the "dangerous" bits _should_ be exposed) I can adjust
>> accordingly.
> 
> 
> Is there a reason this has to be managed by userspace? It sounds a lot
> like a property of platform design, in which case exposing this feature
> in the devicetree might be a better approach.
> 

Same sentiment here.

Guenter
Zev Weiss Sept. 22, 2023, 4:33 a.m. UTC | #3 
On Thu, Sep 21, 2023 at 09:24:40PM PDT, Guenter Roeck wrote:
>On 9/21/23 21:16, Andrew Jeffery wrote:
>>On Thu, 2023-09-21 at 18:35 -0700, Zev Weiss wrote:
>>>The AST2500 and AST2600 watchdog timers provide the ability to control
>>>which devices are reset by the watchdog timer via a reset mask
>>>resgister.  Previously the driver ignored that register, leaving
>>>whatever configuration it found at boot and offering no way of
>>>altering its settings.  Add a 'reset_ctrl' sysfs subdirectory with a
>>>file per bit so that userspace can determine which devices the reset
>>>is applied to.
>>>
>>>Note that not all bits in the hardware register are exposed -- in
>>>particular, the ARM CPU and SOC/misc reset bits are left hidden since
>>>clearing them can render the system unable to reboot.
>>>
>>>Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>>>---
>>>
>>>I'm porting OpenBMC to a platform that requires that the LPC controller remain
>>>un-reset by a BMC reboot.  With this patch userspace can control the reset
>>>mask of the Aspeed watchdog timer, with a few bits remaining unexposed so as
>>>to prevent some almost-certainly undesirable situations.  If there are other
>>>bits that people feel shouldn't be exposed (or conversely if someone feels
>>>strongly that the "dangerous" bits _should_ be exposed) I can adjust
>>>accordingly.
>>
>>
>>Is there a reason this has to be managed by userspace? It sounds a lot
>>like a property of platform design, in which case exposing this feature
>>in the devicetree might be a better approach.
>>
>
>Same sentiment here.
>
>Guenter
>
>

Yes, and indeed the same thing occurred to me, too, though unfortunately 
not until just *after* I sent the patch...I'll rework it as a DT thing 
instead.

Thanks,
Zev
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-class-watchdog b/Documentation/ABI/testing/sysfs-class-watchdog
index 94fb74615951..a2f4bb6a4263 100644
--- a/Documentation/ABI/testing/sysfs-class-watchdog
+++ b/Documentation/ABI/testing/sysfs-class-watchdog
@@ -127,3 +127,13 @@  Description:
 		shown. When written with any non-zero value, it clears
 		the boot code selection and the timeout counter, which results
 		in chipselect reset for AST2400/AST2500.
+
+What:		/sys/class/watchdog/watchdogn/reset_ctrl/*
+Date:		September 2023
+Contact:	Zev Weiss <zev@bewilderbeest.net>
+Description:
+		The read/write files in this subdirectory (present on Aspeed
+		AST2500 and AST2600 only) control which devices and SoC
+		components are reset when the watchdog timer expires.  When set
+		to '1', the device indicated by the name of the file will be
+		reset; when set to '0' it will not.
diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index b72a858bbac7..a05dcf1b5d34 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -26,6 +26,7 @@  struct aspeed_wdt_config {
 	u32 ext_pulse_width_mask;
 	u32 irq_shift;
 	u32 irq_mask;
+	const struct attribute_group *reset_ctrl_group;
 };
 
 struct aspeed_wdt {
@@ -33,34 +34,10 @@  struct aspeed_wdt {
 	void __iomem		*base;
 	u32			ctrl;
 	const struct aspeed_wdt_config *cfg;
+	const struct attribute_group *groups[3]; /* bswitch_group, reset ctrl, NULL terminator */
+	spinlock_t		lock;
 };
 
-static const struct aspeed_wdt_config ast2400_config = {
-	.ext_pulse_width_mask = 0xff,
-	.irq_shift = 0,
-	.irq_mask = 0,
-};
-
-static const struct aspeed_wdt_config ast2500_config = {
-	.ext_pulse_width_mask = 0xfffff,
-	.irq_shift = 12,
-	.irq_mask = GENMASK(31, 12),
-};
-
-static const struct aspeed_wdt_config ast2600_config = {
-	.ext_pulse_width_mask = 0xfffff,
-	.irq_shift = 0,
-	.irq_mask = GENMASK(31, 10),
-};
-
-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 },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
-
 #define WDT_STATUS		0x00
 #define WDT_RELOAD_VALUE	0x04
 #define WDT_RESTART		0x08
@@ -79,6 +56,8 @@  MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
 #define   WDT_TIMEOUT_STATUS_BOOT_SECONDARY	BIT(1)
 #define WDT_CLEAR_TIMEOUT_STATUS	0x14
 #define   WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION	BIT(0)
+#define WDT_RESET_MASK1		0x1c
+#define WDT_RESET_MASK2		0x20
 
 /*
  * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
@@ -263,7 +242,227 @@  static struct attribute *bswitch_attrs[] = {
 	&dev_attr_access_cs0.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(bswitch);
+
+static const struct attribute_group bswitch_group = {
+	.attrs = bswitch_attrs,
+};
+
+struct aspeed_wdt_rstctrl_bit {
+	struct device_attribute dev_attr;
+	u8 reg, bit;
+};
+
+static ssize_t aspeed_wdt_reset_ctrl_show(struct device *dev, struct device_attribute *attr,
+					  char *buf)
+{
+	struct aspeed_wdt *wdt = dev_get_drvdata(dev);
+	struct aspeed_wdt_rstctrl_bit *bit = container_of(attr, struct aspeed_wdt_rstctrl_bit,
+							  dev_attr);
+	u32 mask = readl(wdt->base + bit->reg);
+
+	return sysfs_emit(buf, "%u\n", !!(mask & BIT(bit->bit)));
+}
+
+static ssize_t aspeed_wdt_reset_ctrl_store(struct device *dev, struct device_attribute *attr,
+					   const char *buf, size_t size)
+{
+	struct aspeed_wdt *wdt = dev_get_drvdata(dev);
+	struct aspeed_wdt_rstctrl_bit *bit = container_of(attr, struct aspeed_wdt_rstctrl_bit,
+							  dev_attr);
+	u32 mask;
+	bool val;
+
+	if (kstrtobool(buf, &val))
+		return -EINVAL;
+
+	spin_lock(&wdt->lock);
+	mask = readl(wdt->base + bit->reg);
+	if (val)
+		mask |= BIT(bit->bit);
+	else
+		mask &= ~BIT(bit->bit);
+	writel(mask, wdt->base + bit->reg);
+	spin_unlock(&wdt->lock);
+
+	return size;
+}
+
+#define ASPEED_WDT_RSTCTRL_BIT(chip, name, regnum, bitnum)			\
+	static struct aspeed_wdt_rstctrl_bit chip##_##name##_reset_ctrl = {	\
+		.dev_attr = __ATTR(name, 0644, aspeed_wdt_reset_ctrl_show,	\
+				   aspeed_wdt_reset_ctrl_store),		\
+		.reg = regnum,							\
+		.bit = bitnum,							\
+	}
+
+#define AST2500_WDT_RESET_CTRL(name, bit) \
+	ASPEED_WDT_RSTCTRL_BIT(ast2500, name, WDT_RESET_MASK1, bit)
+
+AST2500_WDT_RESET_CTRL(spi, 24);
+AST2500_WDT_RESET_CTRL(xdma, 23);
+AST2500_WDT_RESET_CTRL(mctp, 22);
+AST2500_WDT_RESET_CTRL(gpio, 21);
+AST2500_WDT_RESET_CTRL(adc, 20);
+AST2500_WDT_RESET_CTRL(jtag, 19);
+AST2500_WDT_RESET_CTRL(peci, 18);
+AST2500_WDT_RESET_CTRL(pwm, 17);
+AST2500_WDT_RESET_CTRL(crt, 16);
+AST2500_WDT_RESET_CTRL(mic, 15);
+AST2500_WDT_RESET_CTRL(sdio, 14);
+AST2500_WDT_RESET_CTRL(lpc, 13);
+AST2500_WDT_RESET_CTRL(hac, 12);
+AST2500_WDT_RESET_CTRL(video, 11);
+AST2500_WDT_RESET_CTRL(hid_ehci, 10);
+AST2500_WDT_RESET_CTRL(usb_host, 9);
+AST2500_WDT_RESET_CTRL(usb2_host_hub, 8);
+AST2500_WDT_RESET_CTRL(graphics, 7);
+AST2500_WDT_RESET_CTRL(mac1, 6);
+AST2500_WDT_RESET_CTRL(mac0, 5);
+AST2500_WDT_RESET_CTRL(i2c, 4);
+AST2500_WDT_RESET_CTRL(ahb, 3);
+AST2500_WDT_RESET_CTRL(sdram, 2);
+AST2500_WDT_RESET_CTRL(coproc, 1);
+
+static struct attribute *ast2500_reset_ctrl_attrs[] = {
+	&ast2500_spi_reset_ctrl.dev_attr.attr,
+	&ast2500_xdma_reset_ctrl.dev_attr.attr,
+	&ast2500_mctp_reset_ctrl.dev_attr.attr,
+	&ast2500_gpio_reset_ctrl.dev_attr.attr,
+	&ast2500_adc_reset_ctrl.dev_attr.attr,
+	&ast2500_jtag_reset_ctrl.dev_attr.attr,
+	&ast2500_peci_reset_ctrl.dev_attr.attr,
+	&ast2500_pwm_reset_ctrl.dev_attr.attr,
+	&ast2500_crt_reset_ctrl.dev_attr.attr,
+	&ast2500_mic_reset_ctrl.dev_attr.attr,
+	&ast2500_sdio_reset_ctrl.dev_attr.attr,
+	&ast2500_lpc_reset_ctrl.dev_attr.attr,
+	&ast2500_hac_reset_ctrl.dev_attr.attr,
+	&ast2500_video_reset_ctrl.dev_attr.attr,
+	&ast2500_hid_ehci_reset_ctrl.dev_attr.attr,
+	&ast2500_usb_host_reset_ctrl.dev_attr.attr,
+	&ast2500_usb2_host_hub_reset_ctrl.dev_attr.attr,
+	&ast2500_graphics_reset_ctrl.dev_attr.attr,
+	&ast2500_mac1_reset_ctrl.dev_attr.attr,
+	&ast2500_mac0_reset_ctrl.dev_attr.attr,
+	&ast2500_i2c_reset_ctrl.dev_attr.attr,
+	&ast2500_ahb_reset_ctrl.dev_attr.attr,
+	&ast2500_sdram_reset_ctrl.dev_attr.attr,
+	&ast2500_coproc_reset_ctrl.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ast2500_reset_ctrl_group = {
+	.name = "reset_ctrl",
+	.attrs = ast2500_reset_ctrl_attrs,
+};
+
+#define AST2600_WDT_RESET_CTRL(name, reg, bit) \
+	ASPEED_WDT_RSTCTRL_BIT(ast2600, name, reg, bit)
+
+AST2600_WDT_RESET_CTRL(rvas, WDT_RESET_MASK1, 25);
+AST2600_WDT_RESET_CTRL(gpio0, WDT_RESET_MASK1, 24);
+AST2600_WDT_RESET_CTRL(xdma1, WDT_RESET_MASK1, 23);
+AST2600_WDT_RESET_CTRL(xdma0, WDT_RESET_MASK1, 22);
+AST2600_WDT_RESET_CTRL(mctp1, WDT_RESET_MASK1, 21);
+AST2600_WDT_RESET_CTRL(mctp0, WDT_RESET_MASK1, 20);
+AST2600_WDT_RESET_CTRL(jtag0, WDT_RESET_MASK1, 19);
+AST2600_WDT_RESET_CTRL(sdio0, WDT_RESET_MASK1, 18);
+AST2600_WDT_RESET_CTRL(mac1, WDT_RESET_MASK1, 17);
+AST2600_WDT_RESET_CTRL(mac0, WDT_RESET_MASK1, 16);
+AST2600_WDT_RESET_CTRL(gp_mcu, WDT_RESET_MASK1, 15);
+AST2600_WDT_RESET_CTRL(dp_mcu, WDT_RESET_MASK1, 14);
+AST2600_WDT_RESET_CTRL(dp, WDT_RESET_MASK1, 13);
+AST2600_WDT_RESET_CTRL(hac, WDT_RESET_MASK1, 12);
+AST2600_WDT_RESET_CTRL(video, WDT_RESET_MASK1, 11);
+AST2600_WDT_RESET_CTRL(crt, WDT_RESET_MASK1, 10);
+AST2600_WDT_RESET_CTRL(graphics, WDT_RESET_MASK1, 9);
+AST2600_WDT_RESET_CTRL(uhci, WDT_RESET_MASK1, 8);
+AST2600_WDT_RESET_CTRL(usb_b, WDT_RESET_MASK1, 7);
+AST2600_WDT_RESET_CTRL(usb_a, WDT_RESET_MASK1, 6);
+AST2600_WDT_RESET_CTRL(coproc, WDT_RESET_MASK1, 5);
+AST2600_WDT_RESET_CTRL(sli, WDT_RESET_MASK1, 3);
+AST2600_WDT_RESET_CTRL(ahb, WDT_RESET_MASK1, 2);
+AST2600_WDT_RESET_CTRL(sdram, WDT_RESET_MASK1, 1);
+
+AST2600_WDT_RESET_CTRL(espi, WDT_RESET_MASK2, 26);
+AST2600_WDT_RESET_CTRL(i3c5, WDT_RESET_MASK2, 23);
+AST2600_WDT_RESET_CTRL(i3c4, WDT_RESET_MASK2, 22);
+AST2600_WDT_RESET_CTRL(i3c3, WDT_RESET_MASK2, 21);
+AST2600_WDT_RESET_CTRL(i3c2, WDT_RESET_MASK2, 20);
+AST2600_WDT_RESET_CTRL(i3c1, WDT_RESET_MASK2, 19);
+AST2600_WDT_RESET_CTRL(i3c0, WDT_RESET_MASK2, 18);
+AST2600_WDT_RESET_CTRL(i3c_global, WDT_RESET_MASK2, 17);
+AST2600_WDT_RESET_CTRL(i2c, WDT_RESET_MASK2, 16);
+AST2600_WDT_RESET_CTRL(fsi, WDT_RESET_MASK2, 15);
+AST2600_WDT_RESET_CTRL(adc, WDT_RESET_MASK2, 14);
+AST2600_WDT_RESET_CTRL(pwm, WDT_RESET_MASK2, 13);
+AST2600_WDT_RESET_CTRL(peci, WDT_RESET_MASK2, 12);
+AST2600_WDT_RESET_CTRL(lpc, WDT_RESET_MASK2, 11);
+AST2600_WDT_RESET_CTRL(mdio, WDT_RESET_MASK2, 10);
+AST2600_WDT_RESET_CTRL(gpio1, WDT_RESET_MASK2, 9);
+AST2600_WDT_RESET_CTRL(jtag1, WDT_RESET_MASK2, 8);
+AST2600_WDT_RESET_CTRL(sdio1, WDT_RESET_MASK2, 7);
+AST2600_WDT_RESET_CTRL(mac3, WDT_RESET_MASK2, 6);
+AST2600_WDT_RESET_CTRL(mac2, WDT_RESET_MASK2, 5);
+AST2600_WDT_RESET_CTRL(sli2, WDT_RESET_MASK2, 3);
+AST2600_WDT_RESET_CTRL(ahb2, WDT_RESET_MASK2, 2);
+AST2600_WDT_RESET_CTRL(spi, WDT_RESET_MASK2, 1);
+
+static struct attribute *ast2600_reset_ctrl_attrs[] = {
+	&ast2600_rvas_reset_ctrl.dev_attr.attr,
+	&ast2600_gpio0_reset_ctrl.dev_attr.attr,
+	&ast2600_xdma1_reset_ctrl.dev_attr.attr,
+	&ast2600_xdma0_reset_ctrl.dev_attr.attr,
+	&ast2600_mctp1_reset_ctrl.dev_attr.attr,
+	&ast2600_mctp0_reset_ctrl.dev_attr.attr,
+	&ast2600_jtag0_reset_ctrl.dev_attr.attr,
+	&ast2600_sdio0_reset_ctrl.dev_attr.attr,
+	&ast2600_mac1_reset_ctrl.dev_attr.attr,
+	&ast2600_mac0_reset_ctrl.dev_attr.attr,
+	&ast2600_gp_mcu_reset_ctrl.dev_attr.attr,
+	&ast2600_dp_mcu_reset_ctrl.dev_attr.attr,
+	&ast2600_dp_reset_ctrl.dev_attr.attr,
+	&ast2600_hac_reset_ctrl.dev_attr.attr,
+	&ast2600_video_reset_ctrl.dev_attr.attr,
+	&ast2600_crt_reset_ctrl.dev_attr.attr,
+	&ast2600_graphics_reset_ctrl.dev_attr.attr,
+	&ast2600_uhci_reset_ctrl.dev_attr.attr,
+	&ast2600_usb_b_reset_ctrl.dev_attr.attr,
+	&ast2600_usb_a_reset_ctrl.dev_attr.attr,
+	&ast2600_coproc_reset_ctrl.dev_attr.attr,
+	&ast2600_sli_reset_ctrl.dev_attr.attr,
+	&ast2600_ahb_reset_ctrl.dev_attr.attr,
+	&ast2600_sdram_reset_ctrl.dev_attr.attr,
+	&ast2600_espi_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c5_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c4_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c3_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c2_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c1_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c0_reset_ctrl.dev_attr.attr,
+	&ast2600_i3c_global_reset_ctrl.dev_attr.attr,
+	&ast2600_i2c_reset_ctrl.dev_attr.attr,
+	&ast2600_fsi_reset_ctrl.dev_attr.attr,
+	&ast2600_adc_reset_ctrl.dev_attr.attr,
+	&ast2600_pwm_reset_ctrl.dev_attr.attr,
+	&ast2600_peci_reset_ctrl.dev_attr.attr,
+	&ast2600_lpc_reset_ctrl.dev_attr.attr,
+	&ast2600_mdio_reset_ctrl.dev_attr.attr,
+	&ast2600_gpio1_reset_ctrl.dev_attr.attr,
+	&ast2600_jtag1_reset_ctrl.dev_attr.attr,
+	&ast2600_sdio1_reset_ctrl.dev_attr.attr,
+	&ast2600_mac3_reset_ctrl.dev_attr.attr,
+	&ast2600_mac2_reset_ctrl.dev_attr.attr,
+	&ast2600_sli2_reset_ctrl.dev_attr.attr,
+	&ast2600_ahb2_reset_ctrl.dev_attr.attr,
+	&ast2600_spi_reset_ctrl.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ast2600_reset_ctrl_group = {
+	.name = "reset_ctrl",
+	.attrs = ast2600_reset_ctrl_attrs,
+};
 
 static const struct watchdog_ops aspeed_wdt_ops = {
 	.start		= aspeed_wdt_start,
@@ -302,6 +501,34 @@  static irqreturn_t aspeed_wdt_irq(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static const struct aspeed_wdt_config ast2400_config = {
+	.ext_pulse_width_mask = 0xff,
+	.irq_shift = 0,
+	.irq_mask = 0,
+};
+
+static const struct aspeed_wdt_config ast2500_config = {
+	.ext_pulse_width_mask = 0xfffff,
+	.irq_shift = 12,
+	.irq_mask = GENMASK(31, 12),
+	.reset_ctrl_group = &ast2500_reset_ctrl_group,
+};
+
+static const struct aspeed_wdt_config ast2600_config = {
+	.ext_pulse_width_mask = 0xfffff,
+	.irq_shift = 0,
+	.irq_mask = GENMASK(31, 10),
+	.reset_ctrl_group = &ast2600_reset_ctrl_group,
+};
+
+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 },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
+
 static int aspeed_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -312,6 +539,7 @@  static int aspeed_wdt_probe(struct platform_device *pdev)
 	u32 duration;
 	u32 status;
 	int ret;
+	int ngroups = 0;
 
 	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
 	if (!wdt)
@@ -328,6 +556,8 @@  static int aspeed_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(wdt->base))
 		return PTR_ERR(wdt->base);
 
+	spin_lock_init(&wdt->lock);
+
 	wdt->wdd.info = &aspeed_wdt_info;
 
 	if (wdt->cfg->irq_mask) {
@@ -347,6 +577,7 @@  static int aspeed_wdt_probe(struct platform_device *pdev)
 	wdt->wdd.ops = &aspeed_wdt_ops;
 	wdt->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS;
 	wdt->wdd.parent = dev;
+	wdt->wdd.groups = wdt->groups;
 
 	wdt->wdd.timeout = WDT_DEFAULT_TIMEOUT;
 	watchdog_init_timeout(&wdt->wdd, 0, dev);
@@ -453,9 +684,12 @@  static int aspeed_wdt_probe(struct platform_device *pdev)
 
 		if (of_device_is_compatible(np, "aspeed,ast2400-wdt") ||
 		    of_device_is_compatible(np, "aspeed,ast2500-wdt"))
-			wdt->wdd.groups = bswitch_groups;
+			wdt->groups[ngroups++] = &bswitch_group;
 	}
 
+	if (wdt->cfg->reset_ctrl_group)
+		wdt->groups[ngroups++] = wdt->cfg->reset_ctrl_group;
+
 	dev_set_drvdata(dev, wdt);
 
 	return devm_watchdog_register_device(dev, &wdt->wdd);