arm: lpc32xx: restart: add support for soft reset

PATCH New Sylvain Lemieux 2 participants 3 messages Feb 19, 2016 – Feb 22, 2016 Watchdog device drivers

lore

Share & tools

lore

Sylvain Lemieux

author

2016-02-19 21:24 UTC #0 lore raw

Hi Joachim,

Quoted 19 lines

From: Joachim Eastwood [mailto:[email protected]]
Sent: 18-Feb-16 5:08 PM
To: [email protected]
Hi Sylvain,
Quoted 12 lines · On 18 February 2016 at 22:35, <[email protected]> wrote:
From: Sylvain Lemieux <[email protected]>
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

Quoted 8 lines

Quoted 2 lines
 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.

Quoted 1 line

I need to add a new functionality to the pnx4008 driver to keep the driver disable and only provide the restart hook.

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

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

Sylvain Lemieux

Quoted 3 lines

regards,
Joachim Eastwood

Joachim Eastwood

2016-02-20 22:19 UTC #1 lore raw

cover arm: lpc32xx: restart: add support for soft reset

Quoted 36 lines · Sylvain Lemieux

Hi Joachim,
Quoted 19 lines
From: Joachim Eastwood [mailto:manabian at gmail.com]
Sent: 18-Feb-16 5:08 PM
To: slemieux.tyco at gmail.com
Hi Sylvain,
Quoted 12 lines · On 18 February 2016 at 22:35, <[email protected]> wrote:
From: Sylvain Lemieux <[email protected]>
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
Quoted 8 lines
Quoted 2 lines
 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.

Quoted 5 lines

I will submit a new revision of the patch on Monday.
Quoted 1 line
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.

Quoted 2 lines

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.

Quoted 1 line

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

Sylvain Lemieux

author

2016-02-22 14:49 UTC #2 lore raw

cover arm: lpc32xx: restart: add support for soft reset

Quoted 40 lines · Joachim Eastwood

Quoted 39 lines · Sylvain Lemieux
Hi Joachim,
Quoted 19 lines
From: Joachim Eastwood [mailto:manabian at gmail.com]
Sent: 18-Feb-16 5:08 PM
To: slemieux.tyco at gmail.com
Hi Sylvain,
Quoted 12 lines · On 18 February 2016 at 22:35, <[email protected]> wrote:
From: Sylvain Lemieux <[email protected]>
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
Quoted 8 lines
Quoted 2 lines
 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.
Quoted 1 line

Quoted 7 lines

Quoted 2 lines
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).

Quoted 10 lines

Quoted 2 lines
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.
Quoted 1 line
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.

Quoted 7 lines

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