| Message ID | 20230711091713.1113010-4-huaqian.li@siemens.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for WDIOF_CARDRESET on TI AM65x | expand |
On 7/11/23 02:17, huaqian.li@siemens.com wrote: > From: Li Hua Qian <huaqian.li@siemens.com> > > This patch adds the WDIOF_CARDRESET support for the platform watchdog > whose hardware does not support this feature, to know if the board > reboot is due to a watchdog reset. > > This is done via reserved memory(RAM), which indicates if specific > info saved, triggering the watchdog reset in last boot. > > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com> > --- > drivers/watchdog/rti_wdt.c | 48 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c > index ce8f18e93aa9..77fd6b54137c 100644 > --- a/drivers/watchdog/rti_wdt.c > +++ b/drivers/watchdog/rti_wdt.c > @@ -18,6 +18,7 @@ > #include <linux/pm_runtime.h> > #include <linux/types.h> > #include <linux/watchdog.h> > +#include <linux/of.h> > > #define DEFAULT_HEARTBEAT 60 > > @@ -52,6 +53,11 @@ > > #define DWDST BIT(1) > > +#define PON_REASON_SOF_NUM 0xBBBBCCCC > +#define PON_REASON_MAGIC_NUM 0xDDDDDDDD > +#define PON_REASON_EOF_NUM 0xCCCCBBBB > +#define PON_REASON_ITEM_BITS 0xFFFFFFFF > + > static int heartbeat = DEFAULT_HEARTBEAT; > > /* > @@ -198,6 +204,11 @@ static int rti_wdt_probe(struct platform_device *pdev) > struct rti_wdt_device *wdt; > struct clk *clk; > u32 last_ping = 0; > + u32 reserved_mem_size; > + unsigned long *vaddr; > + unsigned long paddr; > + u32 data[3]; > + u32 reg[8]; > > wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); > if (!wdt) > @@ -284,6 +295,43 @@ static int rti_wdt_probe(struct platform_device *pdev) > } > } > > + ret = of_property_read_variable_u32_array(pdev->dev.of_node, "reg", reg, > + 0, ARRAY_SIZE(reg)); > + if (ret < 0) { > + dev_err(dev, "cannot read the reg info.\n"); > + goto err_iomap; > + } This aborts if the property does not exist, which is unacceptable. Any such addition must be optional. > + > + /* > + * If reserved memory is defined for watchdog reset cause. > + * Readout the Power-on(PON) reason and pass to bootstatus. > + */ > + if (ret == 8) { > + paddr = reg[5]; > + reserved_mem_size = reg[7]; It seems odd that reserved_mem_size is not checked, and that it is even provided given that it needs to be (at least) 24 bytes, and any other value does not really make sense. > + vaddr = memremap(paddr, reserved_mem_size, MEMREMAP_WB); > + if (vaddr == NULL) { > + dev_err(dev, "Failed to map memory-region.\n"); > + goto err_iomap; This returns 8, which would be an odd error return. > + } > + > + data[0] = *vaddr & PON_REASON_ITEM_BITS; > + data[1] = *(vaddr + 1) & PON_REASON_ITEM_BITS; > + data[2] = *(vaddr + 2) & PON_REASON_ITEM_BITS; > + The & seems pointless / wasteful. Why ignore the upper 32 bits of each location ? Either make it u32 or make it u64 and use the entire 64 bit. Besides, vaddr[0..2] would make the code much easier to read. > + dev_dbg(dev, "Watchdog: sof = %lX, data = %lX, eof = %lX\n", > + data[0], data[1], data[2]); > + > + if ((data[0] == PON_REASON_SOF_NUM) > + && (data[1] == PON_REASON_MAGIC_NUM) > + && (data[1] == PON_REASON_MAGIC_NUM)) { Unnecessary inner (), and I don't see the point of checking data[1] twice. > + dev_info(dev, "Watchdog reset cause detected.\n"); Unnecessary noise. > + wdd->bootstatus |= WDIOF_CARDRESET; > + } > + memset(vaddr, 0, reserved_mem_size); > + memunmap(vaddr); > + } And some random data in the property is acceptable ? That is odd, especially after mandating the property itself. > + > watchdog_init_timeout(wdd, heartbeat, dev); > > ret = watchdog_register_device(wdd);
On Tue, 2023-07-11 at 19:32 -0700, Guenter Roeck wrote: > On 7/11/23 02:17, huaqian.li@siemens.com wrote: > > From: Li Hua Qian <huaqian.li@siemens.com> > > > > This patch adds the WDIOF_CARDRESET support for the platform > > watchdog > > whose hardware does not support this feature, to know if the board > > reboot is due to a watchdog reset. > > > > This is done via reserved memory(RAM), which indicates if specific > > info saved, triggering the watchdog reset in last boot. > > > > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com> > > --- > > drivers/watchdog/rti_wdt.c | 48 > > ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 48 insertions(+) > > > > diff --git a/drivers/watchdog/rti_wdt.c > > b/drivers/watchdog/rti_wdt.c > > index ce8f18e93aa9..77fd6b54137c 100644 > > --- a/drivers/watchdog/rti_wdt.c > > +++ b/drivers/watchdog/rti_wdt.c > > @@ -18,6 +18,7 @@ > > #include <linux/pm_runtime.h> > > #include <linux/types.h> > > #include <linux/watchdog.h> > > +#include <linux/of.h> > > > > #define DEFAULT_HEARTBEAT 60 > > > > @@ -52,6 +53,11 @@ > > > > #define DWDST BIT(1) > > > > +#define PON_REASON_SOF_NUM 0xBBBBCCCC > > +#define PON_REASON_MAGIC_NUM 0xDDDDDDDD > > +#define PON_REASON_EOF_NUM 0xCCCCBBBB > > +#define PON_REASON_ITEM_BITS 0xFFFFFFFF > > + > > static int heartbeat = DEFAULT_HEARTBEAT; > > > > /* > > @@ -198,6 +204,11 @@ static int rti_wdt_probe(struct > > platform_device *pdev) > > struct rti_wdt_device *wdt; > > struct clk *clk; > > u32 last_ping = 0; > > + u32 reserved_mem_size; > > + unsigned long *vaddr; > > + unsigned long paddr; > > + u32 data[3]; > > + u32 reg[8]; > > > > wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); > > if (!wdt) > > @@ -284,6 +295,43 @@ static int rti_wdt_probe(struct > > platform_device *pdev) > > } > > } > > > > + ret = of_property_read_variable_u32_array(pdev- > > >dev.of_node, "reg", reg, > > + 0, ARRAY_SIZE(reg)); > > + if (ret < 0) { > > + dev_err(dev, "cannot read the reg info.\n"); > > + goto err_iomap; > > + } > > This aborts if the property does not exist, which is unacceptable. > Any such addition must be optional. Agree, refactor it. > > > + > > + /* > > + * If reserved memory is defined for watchdog reset cause. > > + * Readout the Power-on(PON) reason and pass to bootstatus. > > + */ > > + if (ret == 8) { > > + paddr = reg[5]; > > + reserved_mem_size = reg[7]; > > It seems odd that reserved_mem_size is not checked, ACK > and that it is even provided > given that it needs to be (at least) 24 bytes, and any other value > does not really > make sense. > I was thinkg to add the reliability, but it seems to be unnecessary and pointless. Were you suggesting that 8 bytes are enough? > > + vaddr = memremap(paddr, reserved_mem_size, > > MEMREMAP_WB); > > + if (vaddr == NULL) { > > + dev_err(dev, "Failed to map memory- > > region.\n"); > > + goto err_iomap; > > This returns 8, which would be an odd error return. > ACK,refactor it. > > + } > > + > > + data[0] = *vaddr & PON_REASON_ITEM_BITS; > > + data[1] = *(vaddr + 1) & PON_REASON_ITEM_BITS; > > + data[2] = *(vaddr + 2) & PON_REASON_ITEM_BITS; > > + > > The & seems pointless / wasteful. Why ignore the upper 32 bits of > each location ? > Either make it u32 or make it u64 and use the entire 64 bit. Besides, > vaddr[0..2] would make the code much easier to read. > ACK, refactor it. > > + dev_dbg(dev, "Watchdog: sof = %lX, data = %lX, eof > > = %lX\n", > > + data[0], data[1], data[2]); > > + > > + if ((data[0] == PON_REASON_SOF_NUM) > > + && (data[1] == PON_REASON_MAGIC_NUM) > > + && (data[1] == PON_REASON_MAGIC_NUM)) { > > Unnecessary inner (), and I don't see the point of checking data[1] > twice. Yeah, a typo happened. > > > + dev_info(dev, "Watchdog reset cause > > detected.\n"); > > Unnecessary noise. ACK,rename dev_info to dev_dbg. > > > + wdd->bootstatus |= WDIOF_CARDRESET; > > + } > > + memset(vaddr, 0, reserved_mem_size); > > + memunmap(vaddr); > > + } > > And some random data in the property is acceptable ? That is odd, > especially > after mandating the property itself. > Yeah, do you have any suggestions about how to store the watchdog reset cause? Best regards, Li Hua Qian > > + > > watchdog_init_timeout(wdd, heartbeat, dev); > > > > ret = watchdog_register_device(wdd); >
On 7/11/23 21:03, Li, Hua Qian wrote: > On Tue, 2023-07-11 at 19:32 -0700, Guenter Roeck wrote: >> On 7/11/23 02:17, huaqian.li@siemens.com wrote: >>> From: Li Hua Qian <huaqian.li@siemens.com> >>> >>> This patch adds the WDIOF_CARDRESET support for the platform >>> watchdog >>> whose hardware does not support this feature, to know if the board >>> reboot is due to a watchdog reset. >>> >>> This is done via reserved memory(RAM), which indicates if specific >>> info saved, triggering the watchdog reset in last boot. >>> >>> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com> >>> --- >>> drivers/watchdog/rti_wdt.c | 48 >>> ++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 48 insertions(+) >>> >>> diff --git a/drivers/watchdog/rti_wdt.c >>> b/drivers/watchdog/rti_wdt.c >>> index ce8f18e93aa9..77fd6b54137c 100644 >>> --- a/drivers/watchdog/rti_wdt.c >>> +++ b/drivers/watchdog/rti_wdt.c >>> @@ -18,6 +18,7 @@ >>> #include <linux/pm_runtime.h> >>> #include <linux/types.h> >>> #include <linux/watchdog.h> >>> +#include <linux/of.h> >>> >>> #define DEFAULT_HEARTBEAT 60 >>> >>> @@ -52,6 +53,11 @@ >>> >>> #define DWDST BIT(1) >>> >>> +#define PON_REASON_SOF_NUM 0xBBBBCCCC >>> +#define PON_REASON_MAGIC_NUM 0xDDDDDDDD >>> +#define PON_REASON_EOF_NUM 0xCCCCBBBB >>> +#define PON_REASON_ITEM_BITS 0xFFFFFFFF >>> + >>> static int heartbeat = DEFAULT_HEARTBEAT; >>> >>> /* >>> @@ -198,6 +204,11 @@ static int rti_wdt_probe(struct >>> platform_device *pdev) >>> struct rti_wdt_device *wdt; >>> struct clk *clk; >>> u32 last_ping = 0; >>> + u32 reserved_mem_size; >>> + unsigned long *vaddr; >>> + unsigned long paddr; >>> + u32 data[3]; >>> + u32 reg[8]; >>> >>> wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); >>> if (!wdt) >>> @@ -284,6 +295,43 @@ static int rti_wdt_probe(struct >>> platform_device *pdev) >>> } >>> } >>> >>> + ret = of_property_read_variable_u32_array(pdev- >>>> dev.of_node, "reg", reg, >>> + 0, ARRAY_SIZE(reg)); >>> + if (ret < 0) { >>> + dev_err(dev, "cannot read the reg info.\n"); >>> + goto err_iomap; >>> + } >> >> This aborts if the property does not exist, which is unacceptable. >> Any such addition must be optional. > Agree, refactor it. >> >>> + >>> + /* >>> + * If reserved memory is defined for watchdog reset cause. >>> + * Readout the Power-on(PON) reason and pass to bootstatus. >>> + */ >>> + if (ret == 8) { >>> + paddr = reg[5]; >>> + reserved_mem_size = reg[7]; >> >> It seems odd that reserved_mem_size is not checked, > ACK >> and that it is even provided >> given that it needs to be (at least) 24 bytes, and any other value >> does not really >> make sense. >> > I was thinkg to add the reliability, but it seems to be unnecessary and > pointless. Were you suggesting that 8 bytes are enough? > No. >>> MEMREMAP_WB); >>> + if (vaddr == NULL) { >>> + dev_err(dev, "Failed to map memory- >>> region.\n"); >>> + goto err_iomap; >> >> This returns 8, which would be an odd error return. >> > ACK,refactor it. >>> + } >>> + >>> + data[0] = *vaddr & PON_REASON_ITEM_BITS; >>> + data[1] = *(vaddr + 1) & PON_REASON_ITEM_BITS; >>> + data[2] = *(vaddr + 2) & PON_REASON_ITEM_BITS; >>> + >> >> The & seems pointless / wasteful. Why ignore the upper 32 bits of >> each location ? >> Either make it u32 or make it u64 and use the entire 64 bit. Besides, >> vaddr[0..2] would make the code much easier to read. >> > ACK, refactor it. >>> + dev_dbg(dev, "Watchdog: sof = %lX, data = %lX, eof >>> = %lX\n", >>> + data[0], data[1], data[2]); >>> + >>> + if ((data[0] == PON_REASON_SOF_NUM) >>> + && (data[1] == PON_REASON_MAGIC_NUM) >>> + && (data[1] == PON_REASON_MAGIC_NUM)) { >> >> Unnecessary inner (), and I don't see the point of checking data[1] >> twice. > Yeah, a typo happened. >> >>> + dev_info(dev, "Watchdog reset cause >>> detected.\n"); >> >> Unnecessary noise. > ACK,rename dev_info to dev_dbg. >> >>> + wdd->bootstatus |= WDIOF_CARDRESET; >>> + } >>> + memset(vaddr, 0, reserved_mem_size); >>> + memunmap(vaddr); >>> + } >> >> And some random data in the property is acceptable ? That is odd, >> especially >> after mandating the property itself. >> > Yeah, do you have any suggestions about how to store the watchdog > reset cause? > No, and that is not the point I was trying to make. Your code actively aborts probe if the "reg" property does not exist at all, but then it silently ignores if it contains a random number of elements (other than 8). For example, something like reg = <0x1234 0x5678> is silently ignored. If anything, _that_ should return -EINVAL because it is obviously wrong. On a higher level, the entire code puzzles me. Obviously there must be a means for the BIOS or ROMMON to write PON_REASON_SOF_NUM into some memory area. That means the BIOS/ROMMON must be able to detect that situation. Why not use the same code to detect this in the driver without the complexity of passing it from BIOS/ROMMON to driver in some random memory area ? > Best regards, > Li Hua Qian >>> + >>> watchdog_init_timeout(wdd, heartbeat, dev); >>> >>> ret = watchdog_register_device(wdd); >> >
On Tue, 2023-07-11 at 22:01 -0700, Guenter Roeck wrote: > On 7/11/23 21:03, Li, Hua Qian wrote: > > On Tue, 2023-07-11 at 19:32 -0700, Guenter Roeck wrote: > > > On 7/11/23 02:17, huaqian.li@siemens.com wrote: > > > > From: Li Hua Qian <huaqian.li@siemens.com> > > > > > > > > This patch adds the WDIOF_CARDRESET support for the platform > > > > watchdog > > > > whose hardware does not support this feature, to know if the > > > > board > > > > reboot is due to a watchdog reset. > > > > > > > > This is done via reserved memory(RAM), which indicates if > > > > specific > > > > info saved, triggering the watchdog reset in last boot. > > > > > > > > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com> > > > > --- > > > > drivers/watchdog/rti_wdt.c | 48 > > > > ++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 48 insertions(+) > > > > > > > > diff --git a/drivers/watchdog/rti_wdt.c > > > > b/drivers/watchdog/rti_wdt.c > > > > index ce8f18e93aa9..77fd6b54137c 100644 > > > > --- a/drivers/watchdog/rti_wdt.c > > > > +++ b/drivers/watchdog/rti_wdt.c > > > > @@ -18,6 +18,7 @@ > > > > #include <linux/pm_runtime.h> > > > > #include <linux/types.h> > > > > #include <linux/watchdog.h> > > > > +#include <linux/of.h> > > > > > > > > #define DEFAULT_HEARTBEAT 60 > > > > > > > > @@ -52,6 +53,11 @@ > > > > > > > > #define DWDST BIT(1) > > > > > > > > +#define PON_REASON_SOF_NUM 0xBBBBCCCC > > > > +#define PON_REASON_MAGIC_NUM 0xDDDDDDDD > > > > +#define PON_REASON_EOF_NUM 0xCCCCBBBB > > > > +#define PON_REASON_ITEM_BITS 0xFFFFFFFF > > > > + > > > > static int heartbeat = DEFAULT_HEARTBEAT; > > > > > > > > /* > > > > @@ -198,6 +204,11 @@ static int rti_wdt_probe(struct > > > > platform_device *pdev) > > > > struct rti_wdt_device *wdt; > > > > struct clk *clk; > > > > u32 last_ping = 0; > > > > + u32 reserved_mem_size; > > > > + unsigned long *vaddr; > > > > + unsigned long paddr; > > > > + u32 data[3]; > > > > + u32 reg[8]; > > > > > > > > wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); > > > > if (!wdt) > > > > @@ -284,6 +295,43 @@ static int rti_wdt_probe(struct > > > > platform_device *pdev) > > > > } > > > > } > > > > > > > > + ret = of_property_read_variable_u32_array(pdev- > > > > > dev.of_node, "reg", reg, > > > > + 0, ARRAY_SIZE(reg)); > > > > + if (ret < 0) { > > > > + dev_err(dev, "cannot read the reg info.\n"); > > > > + goto err_iomap; > > > > + } > > > > > > This aborts if the property does not exist, which is > > > unacceptable. > > > Any such addition must be optional. > > Agree, refactor it. > > > > > > > + > > > > + /* > > > > + * If reserved memory is defined for watchdog reset > > > > cause. > > > > + * Readout the Power-on(PON) reason and pass to > > > > bootstatus. > > > > + */ > > > > + if (ret == 8) { > > > > + paddr = reg[5]; > > > > + reserved_mem_size = reg[7]; > > > > > > It seems odd that reserved_mem_size is not checked, > > ACK > > > and that it is even provided > > > given that it needs to be (at least) 24 bytes, and any other > > > value > > > does not really > > > make sense. > > > > > I was thinkg to add the reliability, but it seems to be unnecessary > > and > > pointless. Were you suggesting that 8 bytes are enough? > > > > No. I guess I misunderstood you. Here you was suggesting that reserved_mem_size should be checked and make sure the value to be at least 24 bytes. Am I understanding you correctly? > > > > MEMREMAP_WB); > > > > + if (vaddr == NULL) { > > > > + dev_err(dev, "Failed to map memory- > > > > region.\n"); > > > > + goto err_iomap; > > > > > > This returns 8, which would be an odd error return. > > > > > ACK,refactor it. > > > > + } > > > > + > > > > + data[0] = *vaddr & PON_REASON_ITEM_BITS; > > > > + data[1] = *(vaddr + 1) & PON_REASON_ITEM_BITS; > > > > + data[2] = *(vaddr + 2) & PON_REASON_ITEM_BITS; > > > > + > > > > > > The & seems pointless / wasteful. Why ignore the upper 32 bits of > > > each location ? > > > Either make it u32 or make it u64 and use the entire 64 bit. > > > Besides, > > > vaddr[0..2] would make the code much easier to read. > > > > > ACK, refactor it. > > > > + dev_dbg(dev, "Watchdog: sof = %lX, data = %lX, > > > > eof > > > > = %lX\n", > > > > + data[0], data[1], data[2]); > > > > + > > > > + if ((data[0] == PON_REASON_SOF_NUM) > > > > + && (data[1] == PON_REASON_MAGIC_NUM) > > > > + && (data[1] == PON_REASON_MAGIC_NUM)) { > > > > > > Unnecessary inner (), and I don't see the point of checking > > > data[1] > > > twice. > > Yeah, a typo happened. > > > > > > > + dev_info(dev, "Watchdog reset cause > > > > detected.\n"); > > > > > > Unnecessary noise. > > ACK,rename dev_info to dev_dbg. > > > > > > > + wdd->bootstatus |= WDIOF_CARDRESET; > > > > + } > > > > + memset(vaddr, 0, reserved_mem_size); > > > > + memunmap(vaddr); > > > > + } > > > > > > And some random data in the property is acceptable ? That is odd, > > > especially > > > after mandating the property itself. > > > > > Yeah, do you have any suggestions about how to store the watchdog > > reset cause? > > > > No, and that is not the point I was trying to make. Your code > actively > aborts probe if the "reg" property does not exist at all, but then it > silently ignores if it contains a random number of elements (other > than 8). For example, something like > reg = <0x1234 0x5678> > is silently ignored. If anything, _that_ should return -EINVAL > because it is obviously wrong. Now I get it, many thanks for pointing out. > > On a higher level, the entire code puzzles me. Obviously there > must be a means for the BIOS or ROMMON to write PON_REASON_SOF_NUM > into some memory area. That means the BIOS/ROMMON must be able > to detect that situation. Why not use the same code to detect this > in the driver without the complexity of passing it from BIOS/ROMMON > to driver in some random memory area ? > In TI AM65x cases, the hardware is not capable of issuing a reset on its own, and is not possible to record or detect the watchdog reset situation. So `k3-rit-wdt` firmware which runs in a mcu core was used to detect the watchdog interrupt and reset the Soc. Here I am trying to write the reason in this firmware when watchdog interrupt happens, and read it out in kernel. > > Best regards, > > Li Hua Qian > > > > + > > > > watchdog_init_timeout(wdd, heartbeat, dev); > > > > > > > > ret = watchdog_register_device(wdd); > > > > > >
On 12.07.23 08:05, Li, Hua Qian (DI FA CTR IPC CN PRC4) wrote: > On Tue, 2023-07-11 at 22:01 -0700, Guenter Roeck wrote: >> On a higher level, the entire code puzzles me. Obviously there >> must be a means for the BIOS or ROMMON to write PON_REASON_SOF_NUM >> into some memory area. That means the BIOS/ROMMON must be able >> to detect that situation. Why not use the same code to detect this >> in the driver without the complexity of passing it from BIOS/ROMMON >> to driver in some random memory area ? >> > In TI AM65x cases, the hardware is not capable of issuing a reset on > its own, and is not possible to record or detect the watchdog reset > situation. So `k3-rit-wdt` firmware which runs in a mcu core was used > to detect the watchdog interrupt and reset the Soc. Here I am trying to > write the reason in this firmware when watchdog interrupt happens, and > read it out in kernel. See https://github.com/siemens/k3-rti-wdt/issues/1 for where this started and where the firmware part comes into play. I still wonder why we couldn't have had a nicer hardware watchdog instead but this can't be changed anymore. Jan
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c index ce8f18e93aa9..77fd6b54137c 100644 --- a/drivers/watchdog/rti_wdt.c +++ b/drivers/watchdog/rti_wdt.c @@ -18,6 +18,7 @@ #include <linux/pm_runtime.h> #include <linux/types.h> #include <linux/watchdog.h> +#include <linux/of.h> #define DEFAULT_HEARTBEAT 60 @@ -52,6 +53,11 @@ #define DWDST BIT(1) +#define PON_REASON_SOF_NUM 0xBBBBCCCC +#define PON_REASON_MAGIC_NUM 0xDDDDDDDD +#define PON_REASON_EOF_NUM 0xCCCCBBBB +#define PON_REASON_ITEM_BITS 0xFFFFFFFF + static int heartbeat = DEFAULT_HEARTBEAT; /* @@ -198,6 +204,11 @@ static int rti_wdt_probe(struct platform_device *pdev) struct rti_wdt_device *wdt; struct clk *clk; u32 last_ping = 0; + u32 reserved_mem_size; + unsigned long *vaddr; + unsigned long paddr; + u32 data[3]; + u32 reg[8]; wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL); if (!wdt) @@ -284,6 +295,43 @@ static int rti_wdt_probe(struct platform_device *pdev) } } + ret = of_property_read_variable_u32_array(pdev->dev.of_node, "reg", reg, + 0, ARRAY_SIZE(reg)); + if (ret < 0) { + dev_err(dev, "cannot read the reg info.\n"); + goto err_iomap; + } + + /* + * If reserved memory is defined for watchdog reset cause. + * Readout the Power-on(PON) reason and pass to bootstatus. + */ + if (ret == 8) { + paddr = reg[5]; + reserved_mem_size = reg[7]; + vaddr = memremap(paddr, reserved_mem_size, MEMREMAP_WB); + if (vaddr == NULL) { + dev_err(dev, "Failed to map memory-region.\n"); + goto err_iomap; + } + + data[0] = *vaddr & PON_REASON_ITEM_BITS; + data[1] = *(vaddr + 1) & PON_REASON_ITEM_BITS; + data[2] = *(vaddr + 2) & PON_REASON_ITEM_BITS; + + dev_dbg(dev, "Watchdog: sof = %lX, data = %lX, eof = %lX\n", + data[0], data[1], data[2]); + + if ((data[0] == PON_REASON_SOF_NUM) + && (data[1] == PON_REASON_MAGIC_NUM) + && (data[1] == PON_REASON_MAGIC_NUM)) { + dev_info(dev, "Watchdog reset cause detected.\n"); + wdd->bootstatus |= WDIOF_CARDRESET; + } + memset(vaddr, 0, reserved_mem_size); + memunmap(vaddr); + } + watchdog_init_timeout(wdd, heartbeat, dev); ret = watchdog_register_device(wdd);