Re: [PATCH 09/46] watchdog: sa1100: use platform device registration
On 10/22/19 2:44 AM, Arnd Bergmann wrote:
On Sat, Oct 19, 2019 at 4:07 PM Guenter Roeck wrote:
@@ -319,10 +316,13 @@ static struct platform_device *sa11x0_devices[]
__initdata = {
static int __init sa1100_init(void)
{
+ struct resource wdt_res = DEFINE_RES_MEM(0x9000, 0x20);
pm_power_off = sa1100_power_off;
regulator_has_full_constraints();
+ platform_device_register_simple("sa1100_wdt", -1, _res, 1);
+
return platform_add_devices(sa11x0_devices, ARRAY_SIZE(sa11x0_devices));
Wouldn't it be better to add the watchdog device to sa11x0_devices ?
Generally speaking, platform_device_register_simple() is better than
platform_add_devices(), it does the same thing with fewer source lines
and smaller object code, and it doesn't have the problem of lifetime rules
for statically allocated reference-counted devices.
One day we may want to replace all static platform_device instances with
platform_device_info instead, but right now there are too many of those.
I can change this one to a platform_device for consistency though if you
think it's worth it.
No, I was just wondering. Thanks for the explanation.
Guenter
Re: [PATCH 09/46] watchdog: sa1100: use platform device registration
On Sat, Oct 19, 2019 at 4:07 PM Guenter Roeck wrote:
> > @@ -319,10 +316,13 @@ static struct platform_device *sa11x0_devices[]
> > __initdata = {
> >
> > static int __init sa1100_init(void)
> > {
> > + struct resource wdt_res = DEFINE_RES_MEM(0x9000, 0x20);
> > pm_power_off = sa1100_power_off;
> >
> > regulator_has_full_constraints();
> >
> > + platform_device_register_simple("sa1100_wdt", -1, _res, 1);
> > +
> > return platform_add_devices(sa11x0_devices,
> > ARRAY_SIZE(sa11x0_devices));
>
> Wouldn't it be better to add the watchdog device to sa11x0_devices ?
Generally speaking, platform_device_register_simple() is better than
platform_add_devices(), it does the same thing with fewer source lines
and smaller object code, and it doesn't have the problem of lifetime rules
for statically allocated reference-counted devices.
One day we may want to replace all static platform_device instances with
platform_device_info instead, but right now there are too many of those.
I can change this one to a platform_device for consistency though if you
think it's worth it.
Arnd
Re: [PATCH 09/46] watchdog: sa1100: use platform device registration
On 10/18/19 8:41 AM, Arnd Bergmann wrote:
Rather than relying on machine specific headers to
pass down the reboot status and the register locations,
use resources and platform_data.
Aside from this, keep the changes to a minimum.
Cc: Wim Van Sebroeck
Cc: Guenter Roeck
Cc: linux-watch...@vger.kernel.org
Signed-off-by: Arnd Bergmann
---
arch/arm/mach-pxa/devices.c | 11 +++
arch/arm/mach-pxa/include/mach/regs-ost.h | 2 +
arch/arm/mach-pxa/include/mach/reset.h| 2 +-
arch/arm/mach-pxa/pxa25x.c| 2 +-
arch/arm/mach-pxa/pxa27x.c| 2 +-
arch/arm/mach-pxa/pxa3xx.c| 2 +-
arch/arm/mach-pxa/reset.c | 3 -
arch/arm/mach-sa1100/generic.c| 6 +-
arch/arm/mach-sa1100/include/mach/reset.h | 1 -
drivers/watchdog/sa1100_wdt.c | 87 ---
10 files changed, 83 insertions(+), 35 deletions(-)
diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 233035e6a2ff..fb9b4f6d32de 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -23,6 +23,8 @@
#include
#include
+#include
+#include
#include "devices.h"
#include "generic.h"
@@ -1110,3 +1112,12 @@ void __init pxa2xx_set_dmac_info(struct mmp_dma_platdata *dma_pdata)
{
pxa_register_device(_pxa_dma, dma_pdata);
}
+
+void __init pxa_register_wdt(unsigned int reset_status)
+{
+ struct resource res = DEFINE_RES_MEM(OST_PHYS, OST_LEN);
+
+ reset_status &= RESET_STATUS_WATCHDOG;
+ platform_device_register_resndata(NULL, "sa1100_wdt", -1, , 1,
+ _status, sizeof(reset_status));
+}
diff --git a/arch/arm/mach-pxa/include/mach/regs-ost.h
b/arch/arm/mach-pxa/include/mach/regs-ost.h
index 109d0ed264df..c8001cfc8d6b 100644
--- a/arch/arm/mach-pxa/include/mach/regs-ost.h
+++ b/arch/arm/mach-pxa/include/mach/regs-ost.h
@@ -7,6 +7,8 @@
/*
* OS Timer & Match Registers
*/
+#define OST_PHYS 0x40A0
+#define OST_LEN0x0020
#define OSMR0 io_p2v(0x40A0) /* */
#define OSMR1 io_p2v(0x40A4) /* */
diff --git a/arch/arm/mach-pxa/include/mach/reset.h
b/arch/arm/mach-pxa/include/mach/reset.h
index e1c4d100fd45..963dd190bc13 100644
--- a/arch/arm/mach-pxa/include/mach/reset.h
+++ b/arch/arm/mach-pxa/include/mach/reset.h
@@ -8,8 +8,8 @@
#define RESET_STATUS_GPIO (1 << 3) /* GPIO Reset */
#define RESET_STATUS_ALL (0xf)
-extern unsigned int reset_status;
extern void clear_reset_status(unsigned int mask);
+extern void pxa_register_wdt(unsigned int reset_status);
/**
* init_gpio_reset() - register GPIO as reset generator
diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
index 305047ebd2f1..dfc90b41fba3 100644
--- a/arch/arm/mach-pxa/pxa25x.c
+++ b/arch/arm/mach-pxa/pxa25x.c
@@ -240,7 +240,7 @@ static int __init pxa25x_init(void)
if (cpu_is_pxa25x()) {
- reset_status = RCSR;
+ pxa_register_wdt(RCSR);
pxa25x_init_pm();
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index a81ac88ecbfd..38fdd22c4dc5 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -337,7 +337,7 @@ static int __init pxa27x_init(void)
if (cpu_is_pxa27x()) {
- reset_status = RCSR;
+ pxa_register_wdt(RCSR);
pxa27x_init_pm();
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index fc84aed99481..7c569fa2a6da 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -463,7 +463,7 @@ static int __init pxa3xx_init(void)
if (cpu_is_pxa3xx()) {
- reset_status = ARSR;
+ pxa_register_wdt(ARSR);
/*
* clear RDH bit every time after reset
diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
index af78405aa4e9..fcb791c5ae3e 100644
--- a/arch/arm/mach-pxa/reset.c
+++ b/arch/arm/mach-pxa/reset.c
@@ -11,9 +11,6 @@
#include
#include
-unsigned int reset_status;
-EXPORT_SYMBOL(reset_status);
-
static void do_hw_reset(void);
static int reset_gpio = -1;
diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
index 4dfb7554649d..6c21f214cd60 100644
--- a/arch/arm/mach-sa1100/generic.c
+++ b/arch/arm/mach-sa1100/generic.c
@@ -39,9 +39,6 @@
#include "generic.h"
#include
-unsigned int reset_status;
-EXPORT_SYMBOL(reset_status);
-
#define NR_FREQS 16
/*
@@ -319,10 +316,13 @@ static struct platform_device *sa11x0_devices[]
__initdata = {
static int __init sa1100_init(void)
{
+ struct resource wdt_res = DEFINE_RES_MEM(0x9000, 0x20);
pm_power_off = sa1100_power_off;
regulator_has_full_constraints();
+ platform_device_register_simple("sa1100_wdt", -1, _res, 1);
+
return platform_add_devices(sa11x0_devices, ARRAY_SIZE(sa11x0_devices));
Wouldn't it be better to add the
[PATCH 09/46] watchdog: sa1100: use platform device registration
Rather than relying on machine specific headers to
pass down the reboot status and the register locations,
use resources and platform_data.
Aside from this, keep the changes to a minimum.
Cc: Wim Van Sebroeck
Cc: Guenter Roeck
Cc: linux-watch...@vger.kernel.org
Signed-off-by: Arnd Bergmann
---
arch/arm/mach-pxa/devices.c | 11 +++
arch/arm/mach-pxa/include/mach/regs-ost.h | 2 +
arch/arm/mach-pxa/include/mach/reset.h| 2 +-
arch/arm/mach-pxa/pxa25x.c| 2 +-
arch/arm/mach-pxa/pxa27x.c| 2 +-
arch/arm/mach-pxa/pxa3xx.c| 2 +-
arch/arm/mach-pxa/reset.c | 3 -
arch/arm/mach-sa1100/generic.c| 6 +-
arch/arm/mach-sa1100/include/mach/reset.h | 1 -
drivers/watchdog/sa1100_wdt.c | 87 ---
10 files changed, 83 insertions(+), 35 deletions(-)
diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 233035e6a2ff..fb9b4f6d32de 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -23,6 +23,8 @@
#include
#include
+#include
+#include
#include "devices.h"
#include "generic.h"
@@ -1110,3 +1112,12 @@ void __init pxa2xx_set_dmac_info(struct mmp_dma_platdata
*dma_pdata)
{
pxa_register_device(_pxa_dma, dma_pdata);
}
+
+void __init pxa_register_wdt(unsigned int reset_status)
+{
+ struct resource res = DEFINE_RES_MEM(OST_PHYS, OST_LEN);
+
+ reset_status &= RESET_STATUS_WATCHDOG;
+ platform_device_register_resndata(NULL, "sa1100_wdt", -1, , 1,
+ _status, sizeof(reset_status));
+}
diff --git a/arch/arm/mach-pxa/include/mach/regs-ost.h
b/arch/arm/mach-pxa/include/mach/regs-ost.h
index 109d0ed264df..c8001cfc8d6b 100644
--- a/arch/arm/mach-pxa/include/mach/regs-ost.h
+++ b/arch/arm/mach-pxa/include/mach/regs-ost.h
@@ -7,6 +7,8 @@
/*
* OS Timer & Match Registers
*/
+#define OST_PHYS 0x40A0
+#define OST_LEN0x0020
#define OSMR0 io_p2v(0x40A0) /* */
#define OSMR1 io_p2v(0x40A4) /* */
diff --git a/arch/arm/mach-pxa/include/mach/reset.h
b/arch/arm/mach-pxa/include/mach/reset.h
index e1c4d100fd45..963dd190bc13 100644
--- a/arch/arm/mach-pxa/include/mach/reset.h
+++ b/arch/arm/mach-pxa/include/mach/reset.h
@@ -8,8 +8,8 @@
#define RESET_STATUS_GPIO (1 << 3)/* GPIO Reset */
#define RESET_STATUS_ALL (0xf)
-extern unsigned int reset_status;
extern void clear_reset_status(unsigned int mask);
+extern void pxa_register_wdt(unsigned int reset_status);
/**
* init_gpio_reset() - register GPIO as reset generator
diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
index 305047ebd2f1..dfc90b41fba3 100644
--- a/arch/arm/mach-pxa/pxa25x.c
+++ b/arch/arm/mach-pxa/pxa25x.c
@@ -240,7 +240,7 @@ static int __init pxa25x_init(void)
if (cpu_is_pxa25x()) {
- reset_status = RCSR;
+ pxa_register_wdt(RCSR);
pxa25x_init_pm();
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index a81ac88ecbfd..38fdd22c4dc5 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -337,7 +337,7 @@ static int __init pxa27x_init(void)
if (cpu_is_pxa27x()) {
- reset_status = RCSR;
+ pxa_register_wdt(RCSR);
pxa27x_init_pm();
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index fc84aed99481..7c569fa2a6da 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -463,7 +463,7 @@ static int __init pxa3xx_init(void)
if (cpu_is_pxa3xx()) {
- reset_status = ARSR;
+ pxa_register_wdt(ARSR);
/*
* clear RDH bit every time after reset
diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
index af78405aa4e9..fcb791c5ae3e 100644
--- a/arch/arm/mach-pxa/reset.c
+++ b/arch/arm/mach-pxa/reset.c
@@ -11,9 +11,6 @@
#include
#include
-unsigned int reset_status;
-EXPORT_SYMBOL(reset_status);
-
static void do_hw_reset(void);
static int reset_gpio = -1;
diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
index 4dfb7554649d..6c21f214cd60 100644
--- a/arch/arm/mach-sa1100/generic.c
+++ b/arch/arm/mach-sa1100/generic.c
@@ -39,9 +39,6 @@
#include "generic.h"
#include
-unsigned int reset_status;
-EXPORT_SYMBOL(reset_status);
-
#define NR_FREQS 16
/*
@@ -319,10 +316,13 @@ static struct platform_device *sa11x0_devices[]
__initdata = {
static int __init sa1100_init(void)
{
+ struct resource wdt_res = DEFINE_RES_MEM(0x9000, 0x20);
pm_power_off = sa1100_power_off;
regulator_has_full_constraints();
+ platform_device_register_simple("sa1100_wdt", -1, _res, 1);
+
return platform_add_devices(sa11x0_devices, ARRAY_SIZE(sa11x0_devices));
}
diff --git
