* [PATCH v2 2/3] watchdog: mpc8xxx: provide boot status
2018-09-14 13:31 [PATCH v2 1/3] watchdog: mpc8xxx: use dev_xxxx() instead of pr_xxxx() Christophe Leroy
@ 2018-09-14 13:32 ` Christophe Leroy
2018-09-14 17:35 ` Guenter Roeck
2018-09-14 13:32 ` [PATCH v2 3/3] dt-bindings: watchdog: add mpc8xxx-wdt support Christophe Leroy
2018-09-14 17:25 ` [PATCH v2 1/3] watchdog: mpc8xxx: use dev_xxxx() instead of pr_xxxx() Guenter Roeck
2 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2018-09-14 13:32 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Radu Rendec, Rob Herring, Mark Rutland
Cc: linux-kernel, linuxppc-dev, linux-watchdog, devicetree
mpc8xxx watchdog driver supports the following platforms:
- mpc8xx
- mpc83xx
- mpc86xx
Those three platforms have a 32 bits register which provides the
reason of the last boot, including whether it was caused by the
watchdog.
mpc8xx: Register RSR, bit SWRS (bit 3)
mpc83xx: Register RSR, bit SWRS (bit 28)
mpc86xx: Register RSTRSCR, bit WDT_RR (bit 11)
This patch maps the register as defined in the device tree and updates
wdt.bootstatus based on the value of the watchdog related bit. Then
the information can be retrieved via the WDIOC_GETBOOTSTATUS ioctl.
Hereunder is an example of devicetree for mpc8xx,
the Reset Status Register being at offset 0x288:
WDT: watchdog@0 {
compatible = "fsl,mpc823-wdt";
reg = <0x0 0x10 0x288 0x4>;
};
On the mpc83xx, RSR is at offset 0x910
On the mpc86xx, RSTRSCR is at offset 0xe0094
Suggested-by: Radu Rendec <radu.rendec@gmail.com>
Tested-by: Christophe Leroy <christophe.leroy@c-s.fr> # On mpc885
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/watchdog/mpc8xxx_wdt.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/watchdog/mpc8xxx_wdt.c b/drivers/watchdog/mpc8xxx_wdt.c
index 1dcf5f10cdd9..4a4700458b17 100644
--- a/drivers/watchdog/mpc8xxx_wdt.c
+++ b/drivers/watchdog/mpc8xxx_wdt.c
@@ -47,6 +47,7 @@ struct mpc8xxx_wdt {
struct mpc8xxx_wdt_type {
int prescaler;
bool hw_enabled;
+ u32 rsr_mask;
};
struct mpc8xxx_wdt_ddata {
@@ -136,6 +137,7 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
u32 freq = fsl_get_sys_freq();
bool enabled;
struct device *dev = &ofdev->dev;
+ u32 __iomem *rsr = NULL;
wdt_type = of_device_get_match_data(dev);
if (!wdt_type)
@@ -159,6 +161,21 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
return -ENODEV;
}
+ res = platform_get_resource(ofdev, IORESOURCE_MEM, 1);
+ if (res)
+ rsr = ioremap(res->start, resource_size(res));
+ if (rsr) {
+ bool status = in_be32(rsr) & wdt_type->rsr_mask;
+
+ ddata->wdd.bootstatus = status ? WDIOF_CARDRESET : 0;
+ /* clear reset status bits related to watchdog timer */
+ out_be32(rsr, wdt_type->rsr_mask);
+ iounmap(rsr);
+
+ dev_info(dev, "Last boot was %scaused by watchdog\n",
+ status ? "" : "not ");
+ }
+
spin_lock_init(&ddata->lock);
ddata->wdd.info = &mpc8xxx_wdt_info,
@@ -216,6 +233,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
.compatible = "mpc83xx_wdt",
.data = &(struct mpc8xxx_wdt_type) {
.prescaler = 0x10000,
+ .rsr_mask = BIT(3), /* RSR Bit SWRS */
},
},
{
@@ -223,6 +241,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
.data = &(struct mpc8xxx_wdt_type) {
.prescaler = 0x10000,
.hw_enabled = true,
+ .rsr_mask = BIT(20), /* RSTRSCR Bit WDT_RR */
},
},
{
@@ -230,6 +249,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
.data = &(struct mpc8xxx_wdt_type) {
.prescaler = 0x800,
.hw_enabled = true,
+ .rsr_mask = BIT(28), /* RSR Bit SWRS */
},
},
{},
--
2.13.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/3] watchdog: mpc8xxx: provide boot status
2018-09-14 13:32 ` [PATCH v2 2/3] watchdog: mpc8xxx: provide boot status Christophe Leroy
@ 2018-09-14 17:35 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2018-09-14 17:35 UTC (permalink / raw)
To: Christophe Leroy
Cc: Wim Van Sebroeck, Radu Rendec, Rob Herring, Mark Rutland,
linux-kernel, linuxppc-dev, linux-watchdog, devicetree
On Fri, Sep 14, 2018 at 01:32:01PM +0000, Christophe Leroy wrote:
> mpc8xxx watchdog driver supports the following platforms:
> - mpc8xx
> - mpc83xx
> - mpc86xx
>
> Those three platforms have a 32 bits register which provides the
> reason of the last boot, including whether it was caused by the
> watchdog.
>
> mpc8xx: Register RSR, bit SWRS (bit 3)
> mpc83xx: Register RSR, bit SWRS (bit 28)
> mpc86xx: Register RSTRSCR, bit WDT_RR (bit 11)
>
> This patch maps the register as defined in the device tree and updates
> wdt.bootstatus based on the value of the watchdog related bit. Then
> the information can be retrieved via the WDIOC_GETBOOTSTATUS ioctl.
>
> Hereunder is an example of devicetree for mpc8xx,
> the Reset Status Register being at offset 0x288:
>
> WDT: watchdog@0 {
> compatible = "fsl,mpc823-wdt";
> reg = <0x0 0x10 0x288 0x4>;
> };
>
> On the mpc83xx, RSR is at offset 0x910
> On the mpc86xx, RSTRSCR is at offset 0xe0094
>
> Suggested-by: Radu Rendec <radu.rendec@gmail.com>
> Tested-by: Christophe Leroy <christophe.leroy@c-s.fr> # On mpc885
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> drivers/watchdog/mpc8xxx_wdt.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/watchdog/mpc8xxx_wdt.c b/drivers/watchdog/mpc8xxx_wdt.c
> index 1dcf5f10cdd9..4a4700458b17 100644
> --- a/drivers/watchdog/mpc8xxx_wdt.c
> +++ b/drivers/watchdog/mpc8xxx_wdt.c
> @@ -47,6 +47,7 @@ struct mpc8xxx_wdt {
> struct mpc8xxx_wdt_type {
> int prescaler;
> bool hw_enabled;
> + u32 rsr_mask;
> };
>
> struct mpc8xxx_wdt_ddata {
> @@ -136,6 +137,7 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
> u32 freq = fsl_get_sys_freq();
> bool enabled;
> struct device *dev = &ofdev->dev;
> + u32 __iomem *rsr = NULL;
>
> wdt_type = of_device_get_match_data(dev);
> if (!wdt_type)
> @@ -159,6 +161,21 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
> return -ENODEV;
> }
>
> + res = platform_get_resource(ofdev, IORESOURCE_MEM, 1);
> + if (res)
> + rsr = ioremap(res->start, resource_size(res));
> + if (rsr) {
This if() can be inside the first if(), and it should be something like
if (res) {
rsr = ioremap(res->start, resource_size(res));
if (!rsr) {
dev_err(...);
return -ENOMEM;
}
...
}
... because _if_ the resource is provided in dt it should be valid.
Thanks,
Guenter
> + bool status = in_be32(rsr) & wdt_type->rsr_mask;
> +
> + ddata->wdd.bootstatus = status ? WDIOF_CARDRESET : 0;
> + /* clear reset status bits related to watchdog timer */
> + out_be32(rsr, wdt_type->rsr_mask);
> + iounmap(rsr);
> +
> + dev_info(dev, "Last boot was %scaused by watchdog\n",
> + status ? "" : "not ");
> + }
> +
> spin_lock_init(&ddata->lock);
>
> ddata->wdd.info = &mpc8xxx_wdt_info,
> @@ -216,6 +233,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
> .compatible = "mpc83xx_wdt",
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x10000,
> + .rsr_mask = BIT(3), /* RSR Bit SWRS */
> },
> },
> {
> @@ -223,6 +241,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x10000,
> .hw_enabled = true,
> + .rsr_mask = BIT(20), /* RSTRSCR Bit WDT_RR */
> },
> },
> {
> @@ -230,6 +249,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x800,
> .hw_enabled = true,
> + .rsr_mask = BIT(28), /* RSR Bit SWRS */
> },
> },
> {},
> --
> 2.13.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] dt-bindings: watchdog: add mpc8xxx-wdt support
2018-09-14 13:31 [PATCH v2 1/3] watchdog: mpc8xxx: use dev_xxxx() instead of pr_xxxx() Christophe Leroy
2018-09-14 13:32 ` [PATCH v2 2/3] watchdog: mpc8xxx: provide boot status Christophe Leroy
@ 2018-09-14 13:32 ` Christophe Leroy
2018-09-14 17:29 ` Guenter Roeck
2018-09-14 17:25 ` [PATCH v2 1/3] watchdog: mpc8xxx: use dev_xxxx() instead of pr_xxxx() Guenter Roeck
2 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2018-09-14 13:32 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Radu Rendec, Rob Herring, Mark Rutland
Cc: linux-kernel, linuxppc-dev, linux-watchdog, devicetree
Add description of DT bindings for mpc8xxx-wdt driver which
handles the CPU watchdog timer on the mpc83xx, mpc86xx and mpc8xx.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
.../devicetree/bindings/watchdog/mpc8xxx-wdt.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt b/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
new file mode 100644
index 000000000000..1d99e1e4d306
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
@@ -0,0 +1,25 @@
+* Freescale mpc8xxx watchdog driver (For 83xx, 86xx and 8xx)
+
+Required properties:
+- compatible: Shall contain one of the following:
+ "mpc83xx_wdt" for an mpc83xx
+ "fsl,mpc8610-wdt" for an mpc86xx
+ "fsl,mpc823-wdt" for an mpc8xx
+- reg: base physical address and length of the area hosting the
+ watchdog registers.
+ On the 83xx, "Watchdog Timer Registers" area: <0x200 0x100>
+ On the 86xx, "Watchdog Timer Registers" area: <0xe4000 0x100>
+ On the 8xx, "General System Interface Unit" area: <0x0 0x10>
+
+Optional properties:
+- reg: additionnal physical address and length (4) of location of the
+ Reset Status Register (called RSTRSCR on the mpc86xx)
+ On the 83xx, it is located at offset 0x910
+ On the 86xx, it is located at offset 0xe0094
+ On the 8xx, it is located at offset 0x288
+
+Example:
+ WDT: watchdog@0 {
+ compatible = "fsl,mpc823-wdt";
+ reg = <0x0 0x10 0x288 0x4>;
+ };
--
2.13.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] dt-bindings: watchdog: add mpc8xxx-wdt support
2018-09-14 13:32 ` [PATCH v2 3/3] dt-bindings: watchdog: add mpc8xxx-wdt support Christophe Leroy
@ 2018-09-14 17:29 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2018-09-14 17:29 UTC (permalink / raw)
To: Christophe Leroy
Cc: Wim Van Sebroeck, Radu Rendec, Rob Herring, Mark Rutland,
linux-kernel, linuxppc-dev, linux-watchdog, devicetree
On Fri, Sep 14, 2018 at 01:32:03PM +0000, Christophe Leroy wrote:
> Add description of DT bindings for mpc8xxx-wdt driver which
> handles the CPU watchdog timer on the mpc83xx, mpc86xx and mpc8xx.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> .../devicetree/bindings/watchdog/mpc8xxx-wdt.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
>
> diff --git a/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt b/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
> new file mode 100644
> index 000000000000..1d99e1e4d306
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
> @@ -0,0 +1,25 @@
> +* Freescale mpc8xxx watchdog driver (For 83xx, 86xx and 8xx)
> +
> +Required properties:
> +- compatible: Shall contain one of the following:
> + "mpc83xx_wdt" for an mpc83xx
> + "fsl,mpc8610-wdt" for an mpc86xx
> + "fsl,mpc823-wdt" for an mpc8xx
> +- reg: base physical address and length of the area hosting the
> + watchdog registers.
> + On the 83xx, "Watchdog Timer Registers" area: <0x200 0x100>
> + On the 86xx, "Watchdog Timer Registers" area: <0xe4000 0x100>
> + On the 8xx, "General System Interface Unit" area: <0x0 0x10>
> +
Note for Rob: The above has been implemented for several years.
This is merely to document the current implementation.
Maybe "mpc83xx_wdt" should be deprecated and replaced, but I think
that should be a separate patch.
> +Optional properties:
> +- reg: additionnal physical address and length (4) of location of the
s/additionnal/additional/
> + Reset Status Register (called RSTRSCR on the mpc86xx)
> + On the 83xx, it is located at offset 0x910
> + On the 86xx, it is located at offset 0xe0094
> + On the 8xx, it is located at offset 0x288
> +
> +Example:
> + WDT: watchdog@0 {
> + compatible = "fsl,mpc823-wdt";
> + reg = <0x0 0x10 0x288 0x4>;
> + };
> --
> 2.13.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] watchdog: mpc8xxx: use dev_xxxx() instead of pr_xxxx()
2018-09-14 13:31 [PATCH v2 1/3] watchdog: mpc8xxx: use dev_xxxx() instead of pr_xxxx() Christophe Leroy
2018-09-14 13:32 ` [PATCH v2 2/3] watchdog: mpc8xxx: provide boot status Christophe Leroy
2018-09-14 13:32 ` [PATCH v2 3/3] dt-bindings: watchdog: add mpc8xxx-wdt support Christophe Leroy
@ 2018-09-14 17:25 ` Guenter Roeck
2 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2018-09-14 17:25 UTC (permalink / raw)
To: Christophe Leroy
Cc: Wim Van Sebroeck, Radu Rendec, Rob Herring, Mark Rutland,
linux-kernel, linuxppc-dev, linux-watchdog, devicetree
On Fri, Sep 14, 2018 at 01:31:59PM +0000, Christophe Leroy wrote:
> mpc8xxx watchdog driver is a platform device drivers, it is
> therefore possible to use dev_xxx() messaging rather than pr_xxx()
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/mpc8xxx_wdt.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/watchdog/mpc8xxx_wdt.c b/drivers/watchdog/mpc8xxx_wdt.c
> index aca2d6323f8a..1dcf5f10cdd9 100644
> --- a/drivers/watchdog/mpc8xxx_wdt.c
> +++ b/drivers/watchdog/mpc8xxx_wdt.c
> @@ -17,8 +17,6 @@
> * option) any later version.
> */
>
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> #include <linux/fs.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> @@ -137,26 +135,27 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
> struct mpc8xxx_wdt_ddata *ddata;
> u32 freq = fsl_get_sys_freq();
> bool enabled;
> + struct device *dev = &ofdev->dev;
>
> - wdt_type = of_device_get_match_data(&ofdev->dev);
> + wdt_type = of_device_get_match_data(dev);
> if (!wdt_type)
> return -EINVAL;
>
> if (!freq || freq == -1)
> return -EINVAL;
>
> - ddata = devm_kzalloc(&ofdev->dev, sizeof(*ddata), GFP_KERNEL);
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> if (!ddata)
> return -ENOMEM;
>
> res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> - ddata->base = devm_ioremap_resource(&ofdev->dev, res);
> + ddata->base = devm_ioremap_resource(dev, res);
> if (IS_ERR(ddata->base))
> return PTR_ERR(ddata->base);
>
> enabled = in_be32(&ddata->base->swcrr) & SWCRR_SWEN;
> if (!enabled && wdt_type->hw_enabled) {
> - pr_info("could not be enabled in software\n");
> + dev_info(dev, "could not be enabled in software\n");
> return -ENODEV;
> }
>
> @@ -166,7 +165,7 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
> ddata->wdd.ops = &mpc8xxx_wdt_ops,
>
> ddata->wdd.timeout = WATCHDOG_TIMEOUT;
> - watchdog_init_timeout(&ddata->wdd, timeout, &ofdev->dev);
> + watchdog_init_timeout(&ddata->wdd, timeout, dev);
>
> watchdog_set_nowayout(&ddata->wdd, nowayout);
>
> @@ -189,12 +188,13 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
>
> ret = watchdog_register_device(&ddata->wdd);
> if (ret) {
> - pr_err("cannot register watchdog device (err=%d)\n", ret);
> + dev_err(dev, "cannot register watchdog device (err=%d)\n", ret);
> return ret;
> }
>
> - pr_info("WDT driver for MPC8xxx initialized. mode:%s timeout=%d sec\n",
> - reset ? "reset" : "interrupt", ddata->wdd.timeout);
> + dev_info(dev,
> + "WDT driver for MPC8xxx initialized. mode:%s timeout=%d sec\n",
> + reset ? "reset" : "interrupt", ddata->wdd.timeout);
>
> platform_set_drvdata(ofdev, ddata);
> return 0;
> @@ -204,8 +204,8 @@ static int mpc8xxx_wdt_remove(struct platform_device *ofdev)
> {
> struct mpc8xxx_wdt_ddata *ddata = platform_get_drvdata(ofdev);
>
> - pr_crit("Watchdog removed, expect the %s soon!\n",
> - reset ? "reset" : "machine check exception");
> + dev_crit(&ofdev->dev, "Watchdog removed, expect the %s soon!\n",
> + reset ? "reset" : "machine check exception");
> watchdog_unregister_device(&ddata->wdd);
>
> return 0;
> --
> 2.13.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread