[PATCH 0/2] arm: lpc32xx: restart: add support for soft reset

[PATCH 0/2] arm: lpc32xx: restart: add support for soft reset

[slemieux.tyco at gmail.com]

From: Sylvain Lemieux <slemieux@tycoint.com>

The LPC32xx provide the capability to perform a soft or hard reset;
a soft reset will no reset the external peripheral
(i.e. RESOUT_N output not asserted).

The default behavior (i.e. hard reset) remain unchanged.

Sylvain Lemieux (2):
  arm: lpc32xx: restart: add support for soft reset
  arm: lpc32xx: restart: support "cmd" from userspace

 arch/arm/mach-lpc32xx/common.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] arm: lpc32xx: restart: add support for soft reset

[slemieux.tyco at gmail.com]

From: Sylvain Lemieux <slemieux@tycoint.com>

Add support for explicit soft reset using the reboot mode.

The default reboot mode behavior is unchanged;
you can specified the default reboot type in the board specific file
"DT_MACHINE_START" definition using the "reboot_mode" parameter.

Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
---
 arch/arm/mach-lpc32xx/common.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
index 716e83e..9eb9f22 100644
--- a/arch/arm/mach-lpc32xx/common.c
+++ b/arch/arm/mach-lpc32xx/common.c
@@ -200,9 +200,16 @@ void lpc23xx_restart(enum reboot_mode mode, const char *cmd)
 	__raw_writel(LPC32XX_CLKPWR_PWMCLK_WDOG_EN,
 		LPC32XX_CLKPWR_TIMER_CLK_CTRL);
 
-	/* Instant assert of RESETOUT_N with pulse length 1mS */
-	__raw_writel(13000, io_p2v(LPC32XX_WDTIM_BASE + 0x18));
-	__raw_writel(0x70, io_p2v(LPC32XX_WDTIM_BASE + 0xC));
+	if (mode == REBOOT_SOFT) {
+		/* Force match output active */
+		__raw_writel(0x01, io_p2v(LPC32XX_WDTIM_BASE + 0x14));
+		/* Enable internal reset on match output (RESOUT_N not asserted) */
+		__raw_writel(0x08, io_p2v(LPC32XX_WDTIM_BASE + 0xC));
+	} else {
+		/* Instant assert of RESETOUT_N with pulse length 1mS */
+		__raw_writel(13000, io_p2v(LPC32XX_WDTIM_BASE + 0x18));
+		__raw_writel(0x70, io_p2v(LPC32XX_WDTIM_BASE + 0xC));
+	}
 
 	/* Wait for watchdog to reset system */
 	while (1)
-- 
1.8.3.1

[PATCH 2/2] arm: lpc32xx: restart: support "cmd" from userspace

[slemieux.tyco at gmail.com]

From: Sylvain Lemieux <slemieux@tycoint.com>

Add the support to verify if a "cmd" is passed from the userspace program rebooting the system;
- if a valid "cmd" is available, it will be handle;
- If the received "cmd" is not supported, the default reboot mode is used.

Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
---
 arch/arm/mach-lpc32xx/common.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
index 9eb9f22..ba61839 100644
--- a/arch/arm/mach-lpc32xx/common.c
+++ b/arch/arm/mach-lpc32xx/common.c
@@ -196,6 +196,19 @@ void __init lpc32xx_map_io(void)
 
 void lpc23xx_restart(enum reboot_mode mode, const char *cmd)
 {
+	/*
+	 * Verify if a "cmd" passed from the userspace program rebooting
+	 * the system; if available, and handle it.
+	 * - For details, see the 'reboot' syscall in kernel/reboot.c
+	 * - If the received "cmd" is not supported, use the default mode.
+	 */
+	if (cmd) {
+		if (cmd[0] == 'h')
+			mode = REBOOT_HARD;
+		else if (cmd[0] == 's')
+			mode = REBOOT_SOFT;
+	}
+
 	/* Make sure WDT clocks are enabled */
 	__raw_writel(LPC32XX_CLKPWR_PWMCLK_WDOG_EN,
 		LPC32XX_CLKPWR_TIMER_CLK_CTRL);
-- 
1.8.3.1

[PATCH 0/2] arm: lpc32xx: restart: add support for soft reset

[Joachim Eastwood]

Hi Sylvain,

On 18 February 2016 at 22:35,  <slemieux.tyco@gmail.com> wrote:
> From: Sylvain Lemieux <slemieux@tycoint.com>
>
> The LPC32xx provide the capability to perform a soft or hard reset;
> a soft reset will no reset the external peripheral
> (i.e. RESOUT_N output not asserted).
>
> The default behavior (i.e. hard reset) remain unchanged.
>
> Sylvain Lemieux (2):
>   arm: lpc32xx: restart: add support for soft reset
>   arm: lpc32xx: restart: support "cmd" from userspace
>
>  arch/arm/mach-lpc32xx/common.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)

Instead of adding more code to mach-lpc32xx could this code be moved
to the watchdog driver (pnx4008_wdt.c) ?
See register_restart_handler() and how it's used in other watchdog drivers.

Then maybe the restart hook in phy3250.c could be removed all together.


regards,
Joachim Eastwood

[PATCH 0/2] arm: lpc32xx: restart: add support for soft reset

[Joachim Eastwood]

On 19 February 2016 at 22:24, Sylvain Lemieux <slemieux.tyco@gmail.com> wrote:
> Hi Joachim,
>
>> From: Joachim Eastwood [mailto:manabian at gmail.com]
>> Sent: 18-Feb-16 5:08 PM
>> To: slemieux.tyco at gmail.com
>>
>> Hi Sylvain,
>>
>> On 18 February 2016 at 22:35,  <slemieux.tyco@gmail.com> wrote:
>> > From: Sylvain Lemieux <slemieux@tycoint.com>
>> >
>> > The LPC32xx provide the capability to perform a soft or hard reset;
>> > a soft reset will no reset the external peripheral
>> > (i.e. RESOUT_N output not asserted).
>> >
>> > The default behavior (i.e. hard reset) remain unchanged.
>> >
>> > Sylvain Lemieux (2):
>> >   arm: lpc32xx: restart: add support for soft reset
>> >   arm: lpc32xx: restart: support "cmd" from userspace
>> >
>> >  arch/arm/mach-lpc32xx/common.c | 26 +++++++++++++++++++++++---
>> >  1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> Instead of adding more code to mach-lpc32xx could this code be moved
>> to the watchdog driver (pnx4008_wdt.c) ?
>> See register_restart_handler() and how it's used in other watchdog drivers.
>>
>> Then maybe the restart hook in phy3250.c could be removed all together.
>
> Thanks for the feedback;
>
> It is possible to remove the "lpc23xx_restart" function from "mach-lpc32xx"
> and add the functionality to the pnx4008 watchdog driver.
> The restart hook in phy3250.c can be remove.
>
> I will submit a new revision of the patch on Monday.
>
>>
> I need to add a new functionality to the pnx4008 driver to
> keep the driver disable and only provide the restart hook.

Why do you need to disable the driver?
The watchdog is not enabled unless you don't explicitly do so, ie
opening it and starting it.


> I am planning to add the functionality using a module parameter;
> this is the way the pnx833x & s3c2410 driver are doing it.

You should normally try to avoid module parameters if you can.


> Do you have any feedback before a submit a patch for this change.

Take a look at:
2165bf524da5f5e496d - "watchdog: core: add restart handler support"

Note that this will not give you the reboot_mode parameter. But maybe
that could be added to the restart-callback function.


regards,
Joachim Eastwood

[PATCH 0/2] arm: lpc32xx: restart: add support for soft reset

[Sylvain Lemieux]

On Sat, 2016-02-20 at 23:19 +0100, Joachim Eastwood wrote:
> On 19 February 2016 at 22:24, Sylvain Lemieux <slemieux.tyco@gmail.com> wrote:
> > Hi Joachim,
> >
> >> From: Joachim Eastwood [mailto:manabian at gmail.com]
> >> Sent: 18-Feb-16 5:08 PM
> >> To: slemieux.tyco at gmail.com
> >>
> >> Hi Sylvain,
> >>
> >> On 18 February 2016 at 22:35,  <slemieux.tyco@gmail.com> wrote:
> >> > From: Sylvain Lemieux <slemieux@tycoint.com>
> >> >
> >> > The LPC32xx provide the capability to perform a soft or hard reset;
> >> > a soft reset will no reset the external peripheral
> >> > (i.e. RESOUT_N output not asserted).
> >> >
> >> > The default behavior (i.e. hard reset) remain unchanged.
> >> >
> >> > Sylvain Lemieux (2):
> >> >   arm: lpc32xx: restart: add support for soft reset
> >> >   arm: lpc32xx: restart: support "cmd" from userspace
> >> >
> >> >  arch/arm/mach-lpc32xx/common.c | 26 +++++++++++++++++++++++---
> >> >  1 file changed, 23 insertions(+), 3 deletions(-)
> >>
> >> Instead of adding more code to mach-lpc32xx could this code be moved
> >> to the watchdog driver (pnx4008_wdt.c) ?
> >> See register_restart_handler() and how it's used in other watchdog drivers.
> >>
> >> Then maybe the restart hook in phy3250.c could be removed all together.
> >
> > Thanks for the feedback;
> >
> > It is possible to remove the "lpc23xx_restart" function from "mach-lpc32xx"
> > and add the functionality to the pnx4008 watchdog driver.
> > The restart hook in phy3250.c can be remove.
> >
> > I will submit a new revision of the patch on Monday.
> >
> >>
> > I need to add a new functionality to the pnx4008 driver to
> > keep the driver disable and only provide the restart hook.
> 
> Why do you need to disable the driver?
> The watchdog is not enabled unless you don't explicitly do so, ie
> opening it and starting it.
> 
On our custom lpc32xx board, we are using an external gpio watchdog.
Prior to this change the pnx4008 driver was disable in the
device tree for our board.

I was trying to only have the restart functionality selected in
the pnx4008 driver and not disable the register operation; this will
provide a single watchdog under "/dev", in our case, the gpio watchdog.

I will work with the "linux-watchdog" maintainer to find a standard
way to provide an option for using a watchdog driver for the
restart capability only (i.e. do not register the driver).

> > I am planning to add the functionality using a module parameter;
> > this is the way the pnx833x & s3c2410 driver are doing it.
> 
> You should normally try to avoid module parameters if you can.
> 
> 
> > Do you have any feedback before a submit a patch for this change.
> 
> Take a look at:
> 2165bf524da5f5e496d - "watchdog: core: add restart handler support"

Thanks for pointing this.

I will submit a new revision of the patch later today.
> 
> Note that this will not give you the reboot_mode parameter. But maybe
> that could be added to the restart-callback function.
> 
> 
> regards,
> Joachim Eastwood