Skip to contentSkip to conversation

mpc83xx_wdt: add support for MPC86xx CPUs

PATCH In review Anton Vorontsov 6 participants 15 messages May 12, 2008 – May 13, 2008 Watchdog device drivers

lore
Share & tools
lore

Anton Vorontsov

author
#0 lore raw

PATCH 1/3 mpc83xx_wdt: add support for MPC86xx CPUs

On MPC86xx the watchdog could be enabled only at power-on-reset, and could not be disabled afterwards. We must ping the watchdog from the kernel until the userspace handles it.

MPC83xx CPUs are only differ in a way that watchdog could be disabled once, but after it was enabled via software it becomes just the same as MPC86xx.

Thus, to support MPC86xx I added the kernel timer which pings the watchdog until the userspace opens it.

Since we implemented the timer, now we're able to implement proper handling for the CONFIG_WATCHDOG_NOWAYOUT case, for MPC83xx and MPC86xx.

Also move the probe code into subsys_initcall, because we want start pinging the watchdog ASAP, and misc devices are available in subsys_initcall.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/watchdog/Kconfig       |    4 +-
 drivers/watchdog/mpc83xx_wdt.c |   63 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 59 insertions(+), 8 deletions(-)
drivers/watchdog/Kconfig +2 -2
index 254d115..2929055 100644
@@ -683,8 +683,8 @@ config 8xx_WDT
 	depends on 8xx
 
 config 83xx_WDT
-	tristate "MPC83xx Watchdog Timer"
-	depends on PPC_83xx
+	tristate "MPC83xx/MPC86xx Watchdog Timer"
+	depends on PPC_83xx || PPC_86xx
 
 config MV64X60_WDT
 	tristate "MV64X60 (Marvell Discovery) Watchdog Timer"
drivers/watchdog/mpc83xx_wdt.c +57 -6

137 lines not on this page · raw message

Signature · 1 line
1.5.5.1
#1 lore raw

drivers/watchdog/mpc83xx_wdt.c static int __devinit mpc83xx_wdt_probe(struct platform_device *dev) · line 208

Quoted 8 lines · Anton Vorontsov
drivers/watchdog/mpc83xx_wdt.c +8 -0
+	enabled = in_be32(&wd_base->swcrr) & SWCRR_SWEN;
+#ifdef CONFIG_PPC_86xx
+	if (!enabled) {
+		dev_info(&dev->dev, "could not be enabled by software\n");
+		ret = -ENOSYS;
+		goto err_unmap;
+	}
+#endif

What happens on an 83xx+86xx multiplatform kernel?

-Scott

#2 lore raw
In reply to Anton Vorontsov
Quoted 18 lines · Anton Vorontsov

On MPC86xx the watchdog could be enabled only at power-on-reset, and could not be disabled afterwards. We must ping the watchdog from the kernel until the userspace handles it.

MPC83xx CPUs are only differ in a way that watchdog could be disabled once, but after it was enabled via software it becomes just the same as MPC86xx.

Thus, to support MPC86xx I added the kernel timer which pings the watchdog until the userspace opens it.

Since we implemented the timer, now we're able to implement proper handling for the CONFIG_WATCHDOG_NOWAYOUT case, for MPC83xx and MPC86xx.

Also move the probe code into subsys_initcall, because we want start pinging the watchdog ASAP, and misc devices are available in subsys_initcall.

Quoted 5 lines
Signed-off-by Anton Vorontsov <[email protected]>

---
drivers/watchdog/Kconfig | 4 +-
drivers/watchdog/mpc83xx_wdt.c | 63 +++++++++++++++++++++++++++++++
+++++----

we should rename it to mpc8xxx_wdt.c

- k

#3 lore raw

Hi Kumar,

Quoted 11 lines
Quoted 9 lines

Also move the probe code into subsys_initcall, because we want start pinging the watchdog ASAP, and misc devices are available in subsys_initcall.

Signed-off-by Anton Vorontsov <[email protected]>

---
drivers/watchdog/Kconfig | 4 +-
drivers/watchdog/mpc83xx_wdt.c | 63 +++++++++++++++++++++++++++++++
+++++----

we should rename it to mpc8xxx_wdt.c

IIRC, even MPC8xx also has the same style of wdt device, just with a different prescale factor.

Thanks,
Jochen

#4 lore raw
In reply to Kumar Gala
Quoted 22 lines · Kumar Gala
Quoted 19 lines · Anton Vorontsov

On MPC86xx the watchdog could be enabled only at power-on-reset, and could not be disabled afterwards. We must ping the watchdog from the kernel until the userspace handles it.

MPC83xx CPUs are only differ in a way that watchdog could be disabled once, but after it was enabled via software it becomes just the same as MPC86xx.

Thus, to support MPC86xx I added the kernel timer which pings the watchdog until the userspace opens it.

Since we implemented the timer, now we're able to implement proper handling for the CONFIG_WATCHDOG_NOWAYOUT case, for MPC83xx and MPC86xx.

Also move the probe code into subsys_initcall, because we want start pinging the watchdog ASAP, and misc devices are available in subsys_initcall.

Quoted 7 lines
Quoted 5 lines
Signed-off-by Anton Vorontsov <[email protected]>

---
drivers/watchdog/Kconfig | 4 +-
drivers/watchdog/mpc83xx_wdt.c | 63 +++++++++++++++++++++++++++++++
+++++----

we should rename it to mpc8xxx_wdt.c

I don't see much sense in renaming the files just because the driver now supports another line of processors... Do you really want the rename? Please repeat if so.

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#5 lore raw
Quoted 7 lines · Anton Vorontsov
Quoted 1 line · Kumar Gala

we should rename it to mpc8xxx_wdt.c

I don't see much sense in renaming the files just because the driver now supports another line of processors... Do you really want the rename? Please repeat if so.

It avoids confusion. I vote for renaming.

-Scott

#6 lore raw
Quoted 1 line · Scott Wood

It avoids confusion. I vote for renaming.

Me too. How about fsl_wdt.c?

Or we could come up with a name for that particular WDT device, like we did with the 85xx DMA controller.

#7 lore raw
Quoted 5 lines · Timur Tabi
Quoted 1 line · Scott Wood

It avoids confusion. I vote for renaming.

Me too. How about fsl_wdt.c?

fsl_wdt sounds too generic, I think it would conflict with at least booke_wdt.c.. no?

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#8 lore raw
Quoted 8 lines · Anton Vorontsov
Quoted 4 lines · Timur Tabi
Quoted 1 line · Scott Wood

It avoids confusion. I vote for renaming.

Me too. How about fsl_wdt.c?

fsl_wdt sounds too generic, I think it would conflict with at least booke_wdt.c.. no?

Yeah, that makes sense. What's the difference between booke_wdt.c and mpc83xx_wdt.c?

#9 lore raw
Quoted 4 lines · Timur Tabi
Quoted 3 lines · Anton Vorontsov
Quoted 2 lines · Timur Tabi

Scott Wood wrote:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Quoted 1 line

It avoids confusion. I vote for renaming.

Me too. How about fsl_wdt.c?

fsl_wdt sounds too generic, I think it would conflict with at least booke_wdt.c.. no?

Yeah, that makes sense. What's the difference between booke_wdt.c and mpc83xx_wdt.c?

They're completely different watchdogs...

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#10 lore raw
Quoted 1 line · Anton Vorontsov

They're completely different watchdogs...

But the drivers both generally do the same thing, just with different timers?

I think the best option is to just pick a name for the IP block that mpc83xx_wdt.c supports and use that name for the driver as well.

#11 lore raw
Quoted 6 lines · Timur Tabi
Quoted 1 line · Anton Vorontsov

They're completely different watchdogs...

But the drivers both generally do the same thing, just with different timers?

All the watchdogs do the same thing, generally...

Quoted 2 lines

I think the best option is to just pick a name for the IP block that mpc83xx_wdt.c supports and use that name for the driver as well.

I don't know the IP block name... what is it for the 83xx/86xx watchdogs?

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#12 lore raw
Quoted 2 lines · Anton Vorontsov

I don't know the IP block name... what is it for the 83xx/86xx watchdogs?

My point was that we developers should just pick a name and run with it.
That's where the name "Elo" came from. None of official Freescale
documentation calls the DMA controller on 85xx parts "Elo", but the kernel code does.

#13 lore raw
Quoted 9 lines · Timur Tabi
Quoted 2 lines · Anton Vorontsov

I don't know the IP block name... what is it for the 83xx/86xx watchdogs?

My point was that we developers should just pick a name and run with it.
That's where the name "Elo" came from. None of official Freescale
documentation calls the DMA controller on 85xx parts "Elo", but the kernel code does.

Ahh, I see. Ok, my purpose: name the driver for mpc83xx-like watchdogs as mpc83xx_wdt.c? :-)

It's less confusion. For example DMA. I really don't know what the "Elo" is. Nobody does actually, except the few people who discussed Linux Freescale DMA drivers...

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#14 lore raw
Quoted 11 lines · Anton Vorontsov
Quoted 9 lines · Timur Tabi
Quoted 2 lines · Anton Vorontsov

I don't know the IP block name... what is it for the 83xx/86xx watchdogs?

My point was that we developers should just pick a name and run with it. That's where the name "Elo" came from. None of official Freescale documentation calls the DMA controller on 85xx parts "Elo", but the kernel code does.

Quoted 6 lines

Ahh, I see. Ok, my purpose: name the driver for mpc83xx-like watchdogs as mpc83xx_wdt.c? :-)

It's less confusion. For example DMA. I really don't know what the "Elo" is. Nobody does actually, except the few people who discussed Linux Freescale DMA drivers...

I repeat, name it mpc8xxx_wdt.c or fsl_ppc_soc_wdt.c.

- k