2018-05-10 18:49:49

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 1/8] watchdog: JZ4740: Disable clock after stopping counter

Previously, the clock was disabled first, which makes the watchdog
component insensitive to register writes.

Signed-off-by: Paul Cercueil <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/jz4740_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

v2: No change
v3: No change

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index aafbeb96561b..55c9a1f26498 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -124,8 +124,8 @@ static int jz4740_wdt_stop(struct watchdog_device *wdt_dev)
{
struct jz4740_wdt_drvdata *drvdata = watchdog_get_drvdata(wdt_dev);

- jz4740_timer_disable_watchdog();
writeb(0x0, drvdata->base + JZ_REG_WDT_COUNTER_ENABLE);
+ jz4740_timer_disable_watchdog();

return 0;
}
--
2.11.0



2018-05-10 18:49:00

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 3/8] watchdog: JZ4740: Register a restart handler

The watchdog driver can restart the system by simply configuring the
hardware for a timeout of 0 seconds.

Signed-off-by: Paul Cercueil <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/jz4740_wdt.c | 9 +++++++++
1 file changed, 9 insertions(+)

v2: No change
v3: No change

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index 22136e3522b9..b8b015a7d045 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -130,6 +130,14 @@ static int jz4740_wdt_stop(struct watchdog_device *wdt_dev)
return 0;
}

+static int jz4740_wdt_restart(struct watchdog_device *wdt_dev,
+ unsigned long action, void *data)
+{
+ wdt_dev->timeout = 0;
+ jz4740_wdt_start(wdt_dev);
+ return 0;
+}
+
static const struct watchdog_info jz4740_wdt_info = {
.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
.identity = "jz4740 Watchdog",
@@ -141,6 +149,7 @@ static const struct watchdog_ops jz4740_wdt_ops = {
.stop = jz4740_wdt_stop,
.ping = jz4740_wdt_ping,
.set_timeout = jz4740_wdt_set_timeout,
+ .restart = jz4740_wdt_restart,
};

#ifdef CONFIG_OF
--
2.11.0


2018-05-10 18:49:17

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 6/8] MIPS: jz4780: dts: Fix watchdog node

- The previous node requested a memory area of 0x100 bytes, while the
driver only manipulates four registers present in the first 0x10 bytes.

- The driver requests for the "rtc" clock, but the previous node did not
provide any.

Signed-off-by: Paul Cercueil <[email protected]>
Reviewed-by: Mathieu Malaterre <[email protected]>
Acked-by: James Hogan <[email protected]>
---
Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt | 7 ++++++-
arch/mips/boot/dts/ingenic/jz4780.dtsi | 5 ++++-
2 files changed, 10 insertions(+), 2 deletions(-)

v2: No change
v3: Also fix documentation

diff --git a/Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt b/Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt
index cb44918f01a8..ce1cb72d5345 100644
--- a/Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt
@@ -3,10 +3,15 @@ Ingenic Watchdog Timer (WDT) Controller for JZ4740 & JZ4780
Required properties:
compatible: "ingenic,jz4740-watchdog" or "ingenic,jz4780-watchdog"
reg: Register address and length for watchdog registers
+clocks: phandle to the RTC clock
+clock-names: should be "rtc"

Example:

watchdog: jz4740-watchdog@10002000 {
compatible = "ingenic,jz4740-watchdog";
- reg = <0x10002000 0x100>;
+ reg = <0x10002000 0x10>;
+
+ clocks = <&cgu JZ4740_CLK_RTC>;
+ clock-names = "rtc";
};
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 9b5794667aee..a52f59bf58c7 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -221,7 +221,10 @@

watchdog: watchdog@10002000 {
compatible = "ingenic,jz4780-watchdog";
- reg = <0x10002000 0x100>;
+ reg = <0x10002000 0x10>;
+
+ clocks = <&cgu JZ4780_CLK_RTCLK>;
+ clock-names = "rtc";
};

nemc: nemc@13410000 {
--
2.11.0


2018-05-10 18:49:19

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 8/8] MIPS: jz4740: Drop old platform reset code

This work is now performed by the watchdog driver directly.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: James Hogan <[email protected]>
---
arch/mips/jz4740/reset.c | 31 -------------------------------
1 file changed, 31 deletions(-)

v2: No change
v3: No change

diff --git a/arch/mips/jz4740/reset.c b/arch/mips/jz4740/reset.c
index 67780c4b6573..5bf0cf44b55f 100644
--- a/arch/mips/jz4740/reset.c
+++ b/arch/mips/jz4740/reset.c
@@ -12,18 +12,9 @@
*
*/

-#include <linux/clk.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/pm.h>
-
#include <asm/reboot.h>

-#include <asm/mach-jz4740/base.h>
-#include <asm/mach-jz4740/timer.h>
-
#include "reset.h"
-#include "clock.h"

static void jz4740_halt(void)
{
@@ -36,29 +27,7 @@ static void jz4740_halt(void)
}
}

-#define JZ_REG_WDT_DATA 0x00
-#define JZ_REG_WDT_COUNTER_ENABLE 0x04
-#define JZ_REG_WDT_COUNTER 0x08
-#define JZ_REG_WDT_CTRL 0x0c
-
-static void jz4740_restart(char *command)
-{
- void __iomem *wdt_base = ioremap(JZ4740_WDT_BASE_ADDR, 0x0f);
-
- jz4740_timer_enable_watchdog();
-
- writeb(0, wdt_base + JZ_REG_WDT_COUNTER_ENABLE);
-
- writew(0, wdt_base + JZ_REG_WDT_COUNTER);
- writew(0, wdt_base + JZ_REG_WDT_DATA);
- writew(BIT(2), wdt_base + JZ_REG_WDT_CTRL);
-
- writeb(1, wdt_base + JZ_REG_WDT_COUNTER_ENABLE);
- jz4740_halt();
-}
-
void jz4740_reset_init(void)
{
- _machine_restart = jz4740_restart;
_machine_halt = jz4740_halt;
}
--
2.11.0


2018-05-10 18:49:36

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 7/8] MIPS: qi_lb60: Enable the jz4740-wdt driver

The watchdog is an useful piece of hardware, so there's no reason not to
enable it.

Besides, this is important for restart to work after the change in the
next commit.

This commit enables the Kconfig option in the qi_lb60 defconfig.

Signed-off-by: Paul Cercueil <[email protected]>
Acked-by: James Hogan <[email protected]>
---
arch/mips/configs/qi_lb60_defconfig | 2 ++
1 file changed, 2 insertions(+)

v2: No change
v3: No change

diff --git a/arch/mips/configs/qi_lb60_defconfig b/arch/mips/configs/qi_lb60_defconfig
index 3b02ff9a7c64..d8b7211a7b0f 100644
--- a/arch/mips/configs/qi_lb60_defconfig
+++ b/arch/mips/configs/qi_lb60_defconfig
@@ -72,6 +72,8 @@ CONFIG_POWER_SUPPLY=y
CONFIG_BATTERY_JZ4740=y
CONFIG_CHARGER_GPIO=y
# CONFIG_HWMON is not set
+CONFIG_WATCHDOG=y
+CONFIG_JZ4740_WDT=y
CONFIG_MFD_JZ4740_ADC=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
--
2.11.0


2018-05-10 18:50:24

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver

Also remove the watchdog platform_device from platform.c, since it
wasn't used anywhere anyway.

Signed-off-by: Paul Cercueil <[email protected]>
---
arch/mips/boot/dts/ingenic/jz4740.dtsi | 8 ++++++++
arch/mips/jz4740/platform.c | 16 ----------------
2 files changed, 8 insertions(+), 16 deletions(-)

v2: No change
v3: No change

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index cd5185bb90ae..26c6b561d6f7 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -45,6 +45,14 @@
#clock-cells = <1>;
};

+ watchdog: watchdog@10002000 {
+ compatible = "ingenic,jz4740-watchdog";
+ reg = <0x10002000 0x10>;
+
+ clocks = <&cgu JZ4740_CLK_RTC>;
+ clock-names = "rtc";
+ };
+
rtc_dev: rtc@10003000 {
compatible = "ingenic,jz4740-rtc";
reg = <0x10003000 0x40>;
diff --git a/arch/mips/jz4740/platform.c b/arch/mips/jz4740/platform.c
index 5b7cdd67a9d9..cbc5f8e87230 100644
--- a/arch/mips/jz4740/platform.c
+++ b/arch/mips/jz4740/platform.c
@@ -233,22 +233,6 @@ struct platform_device jz4740_adc_device = {
.resource = jz4740_adc_resources,
};

-/* Watchdog */
-static struct resource jz4740_wdt_resources[] = {
- {
- .start = JZ4740_WDT_BASE_ADDR,
- .end = JZ4740_WDT_BASE_ADDR + 0x10 - 1,
- .flags = IORESOURCE_MEM,
- },
-};
-
-struct platform_device jz4740_wdt_device = {
- .name = "jz4740-wdt",
- .id = -1,
- .num_resources = ARRAY_SIZE(jz4740_wdt_resources),
- .resource = jz4740_wdt_resources,
-};
-
/* PWM */
struct platform_device jz4740_pwm_device = {
.name = "jz4740-pwm",
--
2.11.0


2018-05-10 18:50:26

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 2/8] watchdog: jz4740: Use devm_* functions

- Use devm_clk_get instead of clk_get
- Use devm_watchdog_register_device instead of watchdog_register_device

Signed-off-by: Paul Cercueil <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/jz4740_wdt.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)

v2: No change
v3: No change

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index 55c9a1f26498..22136e3522b9 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -179,40 +179,29 @@ static int jz4740_wdt_probe(struct platform_device *pdev)

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
drvdata->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(drvdata->base)) {
- ret = PTR_ERR(drvdata->base);
- goto err_out;
- }
+ if (IS_ERR(drvdata->base))
+ return PTR_ERR(drvdata->base);

- drvdata->rtc_clk = clk_get(&pdev->dev, "rtc");
+ drvdata->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
if (IS_ERR(drvdata->rtc_clk)) {
dev_err(&pdev->dev, "cannot find RTC clock\n");
- ret = PTR_ERR(drvdata->rtc_clk);
- goto err_out;
+ return PTR_ERR(drvdata->rtc_clk);
}

- ret = watchdog_register_device(&drvdata->wdt);
+ ret = devm_watchdog_register_device(&pdev->dev, &drvdata->wdt);
if (ret < 0)
- goto err_disable_clk;
+ return ret;

platform_set_drvdata(pdev, drvdata);
- return 0;

-err_disable_clk:
- clk_put(drvdata->rtc_clk);
-err_out:
- return ret;
+ return 0;
}

static int jz4740_wdt_remove(struct platform_device *pdev)
{
struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev);

- jz4740_wdt_stop(&drvdata->wdt);
- watchdog_unregister_device(&drvdata->wdt);
- clk_put(drvdata->rtc_clk);
-
- return 0;
+ return jz4740_wdt_stop(&drvdata->wdt);
}

static struct platform_driver jz4740_wdt_driver = {
--
2.11.0


2018-05-10 18:50:26

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v3 4/8] watchdog: JZ4740: Drop module remove function

When the watchdog was configured for nowayout, and after the
userspace watchdog daemon closed the dev node without sending the
magic character, unloading this module stopped the watchdog
hardware, which was clearly a problem.

Besides, unloading the module is not possible when the userspace
watchdog daemon is running, so it's safe to assume that we don't
need to stop the watchdog hardware in the jz4740_wdt_remove()
function.

For this reason, the jz4740_wdt_remove() function can then be
dropped alltogether.

Signed-off-by: Paul Cercueil <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---
drivers/watchdog/jz4740_wdt.c | 8 --------
1 file changed, 8 deletions(-)

v2: New patch in this series
v3: No change

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index b8b015a7d045..ec4d99a830ba 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -206,16 +206,8 @@ static int jz4740_wdt_probe(struct platform_device *pdev)
return 0;
}

-static int jz4740_wdt_remove(struct platform_device *pdev)
-{
- struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev);
-
- return jz4740_wdt_stop(&drvdata->wdt);
-}
-
static struct platform_driver jz4740_wdt_driver = {
.probe = jz4740_wdt_probe,
- .remove = jz4740_wdt_remove,
.driver = {
.name = "jz4740-wdt",
.of_match_table = of_match_ptr(jz4740_wdt_of_matches),
--
2.11.0


2018-05-11 14:53:54

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver

On Thu, May 10, 2018 at 08:47:48PM +0200, Paul Cercueil wrote:
> Also remove the watchdog platform_device from platform.c, since it
> wasn't used anywhere anyway.

Nit: it'd be slightly nicer IMO if the patch body was a superset of the
subject line. It's fine to repeat what the subject says since thats
meant to summarise the body.

> -struct platform_device jz4740_wdt_device = {

There's an extern in arch/mips/include/asm/mach-jz4740/platform.h that
should perhaps be removed also?

Otherwise
Acked-by: James Hogan <[email protected]>

I'm happy to apply for 4.18 with that change if you want it to go
through the MIPS tree.

Cheers
James


Attachments:
(No filename) (664.00 B)
signature.asc (235.00 B)
Download all attachments

2018-05-11 20:55:30

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver

On Fri, May 11, 2018 at 01:17:04PM -0300, Paul Cercueil wrote:
> Le 11 mai 2018 11:52, James Hogan <[email protected]> a écrit :
> > Otherwise
> > Acked-by: James Hogan <[email protected]>
> >
> > I'm happy to apply for 4.18 with that change if you want it to go
> > through the MIPS tree.
>
> Yes please!

Done

Thanks
James


Attachments:
(No filename) (346.00 B)
signature.asc (235.00 B)
Download all attachments

2018-05-11 21:14:50

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver

On Fri, May 11, 2018 at 09:54:14PM +0100, James Hogan wrote:
> On Fri, May 11, 2018 at 01:17:04PM -0300, Paul Cercueil wrote:
> > Le 11 mai 2018 11:52, James Hogan <[email protected]> a ?crit :
> > > Otherwise
> > > Acked-by: James Hogan <[email protected]>
> > >
> > > I'm happy to apply for 4.18 with that change if you want it to go
> > > through the MIPS tree.
> >
> > Yes please!
>
> Done
>
Does that include the watchdog changes ? No problem with it, just asking to make
sure that those don't get lost.

Thanks,
Guenter

2018-05-11 21:17:40

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver

On Fri, May 11, 2018 at 02:14:16PM -0700, Guenter Roeck wrote:
> On Fri, May 11, 2018 at 09:54:14PM +0100, James Hogan wrote:
> > On Fri, May 11, 2018 at 01:17:04PM -0300, Paul Cercueil wrote:
> > > Le 11 mai 2018 11:52, James Hogan <[email protected]> a écrit :
> > > > Otherwise
> > > > Acked-by: James Hogan <[email protected]>
> > > >
> > > > I'm happy to apply for 4.18 with that change if you want it to go
> > > > through the MIPS tree.
> > >
> > > Yes please!
> >
> > Done
> >
> Does that include the watchdog changes ? No problem with it, just asking to make
> sure that those don't get lost.

Yes, I suppose I was taking your reviewed-by as an ack.

Cheers
James


Attachments:
(No filename) (702.00 B)
signature.asc (235.00 B)
Download all attachments

2018-05-11 21:31:10

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver

On Fri, May 11, 2018 at 10:15:55PM +0100, James Hogan wrote:
> On Fri, May 11, 2018 at 02:14:16PM -0700, Guenter Roeck wrote:
> > On Fri, May 11, 2018 at 09:54:14PM +0100, James Hogan wrote:
> > > On Fri, May 11, 2018 at 01:17:04PM -0300, Paul Cercueil wrote:
> > > > Le 11 mai 2018 11:52, James Hogan <[email protected]> a ?crit :
> > > > > Otherwise
> > > > > Acked-by: James Hogan <[email protected]>
> > > > >
> > > > > I'm happy to apply for 4.18 with that change if you want it to go
> > > > > through the MIPS tree.
> > > >
> > > > Yes please!
> > >
> > > Done
> > >
> > Does that include the watchdog changes ? No problem with it, just asking to make
> > sure that those don't get lost.
>
> Yes, I suppose I was taking your reviewed-by as an ack.
>
Ok.

Thanks,
Guenter

2018-05-18 21:33:05

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] MIPS: jz4780: dts: Fix watchdog node

On Thu, May 10, 2018 at 08:47:49PM +0200, Paul Cercueil wrote:
> - The previous node requested a memory area of 0x100 bytes, while the
> driver only manipulates four registers present in the first 0x10 bytes.
>
> - The driver requests for the "rtc" clock, but the previous node did not
> provide any.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> Reviewed-by: Mathieu Malaterre <[email protected]>
> Acked-by: James Hogan <[email protected]>
> ---
> Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt | 7 ++++++-
> arch/mips/boot/dts/ingenic/jz4780.dtsi | 5 ++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> v2: No change
> v3: Also fix documentation

Reviewed-by: Rob Herring <[email protected]>