search

openbouffalo / linux

Linux kernel source tree
Other
6 stars 4 forks source link

Bl808/watchdog #2

Open grant-olson opened 1 year ago

grant-olson commented 1 year ago

This adds support for the BL808 watchdog. The implementation has a resolution of 1 second and the timeout range can be set from 1 - 2^14 seconds. To test with busybox's watchdog:

grant-olson commented 1 year ago

https://github.com/arm000/linux-bl808/pull/8

There were two issues remaining that I didn't take care of.

  1. I take over the whole set of timer registers. smauel pointed out it's not ideal that this register controls other timers and we do nothing with them. I looked to see if there were other drivers I could use to implement the low level timers, but nothing obvious stood out. So there's not really an agreed upon way to make this better now. If someone points me to the right driver type I can probably implement these as well now that I'm familiar with the code.

  2. smauel also commented that there were a few areas where I wrote 16 bit values instead of 32 bit register values. These were part of a knocking sequence used to make sure we don't enable the watchdog accidentally. I tried changing to 32 bit values, and then things didn't work, so I believe the 16 bit writes are critical to the knocking sequence and should not be changed.

alexhorner commented 1 year ago

arm000#8

There were two issues remaining that I didn't take care of.

1. I take over the whole set of timer registers. smauel pointed out it's not ideal that this register controls other timers and we do nothing with them. I looked to see if there were other drivers I could use to implement the low level timers, but nothing obvious stood out. So there's not really an agreed upon way to make this better now. If someone points me to the right driver type I can probably implement these as well now that I'm familiar with the code.

2. smauel also commented that there were a few areas where I wrote 16 bit values instead of 32 bit register values. These were part of a **knocking** sequence used to make sure we don't enable the watchdog accidentally. I tried changing to 32 bit values, and then things didn't work, so I believe the 16 bit writes are critical to the **knocking** sequence and should not be changed.

Hey!

I don't think 2 is too much if at all an issue. If it works and it does not cause any issues or have any caveats, thats fine IMO.

With regards to 1, I do think this is an issue worth further discussion before we complete this PR. All the different bits I have looked at appear to point "timer" as under the category of "clocksource" and commonly associated with watchdogs too.

I think it might be worth taking a look around the clocksource API to see if the timer setup can be implemented there or not (I have not checked at all) and if it does appear possible, it might be worth looking at supporting as a dependency to this prior to completing this driver.

Not sure if @smaeul wants to chime in with anything else regarding clocksource considering this is not the only clock thing that needs to be considered right now for the BL808?

Fishwaldo commented 1 year ago

I think we need to first decide which timer should be "supported" on linux. There are two timers, (timer0, and timer1) both wired upto M0/LP... (and thus IRQ forwarding is also required one way or another). Timers are also a pretty standard requirement for a lot of RTOS/Bare Metal programs so having linux take over both is not optimal.

But I was also thinking of writing a different WDG driver here. The actual WDG runs on M0, but a new Linux WDG pings it via the Mailbox Services. If we don't get a WDG ping from linux (and don't get a ping from anything running on D0) then we let the WDG expire and execute a reset. This handles the case of M0 or D0 crashing, where as right now if M0 crashes (and stops forwarding interupts) then linux just sits there throwing errors and the WDG doesn't trigger.

(nothing stopping us from having both @grant-olson driver and a 2nd driver, and in fact we might be able to merge them via some Device Tree Magic)

alexhorner commented 1 year ago

Hey all, just wanted to spark conversation on this one again. Looking to merge it when/if appropriate

We no longer have an IRQ forwarding dependency on M0 too, so that probably changes things a tad