Hi Andrew, > > Andrew, Wim: Alessandro was not around in the last weeks, so I'd suggest this > > patch goes via the watchdog-tree together with the rest of this series if it > > passes the review of the watchdog-list? All changes here are in preparation for > > the watchdog driver and do not affect the general RTC. Is that okay? > > OK by me. Be careful about the Kconfig setup when doing > cross-subsystem dependencies like this. Otherwise you get sad emails > from Randy when his build breaks. Yes, the watchdog driver depends on the RTC. > > + .name = "stmp3xxx_rtc_wdt", > > + .id = -1, > > +}; > > + > > +void stmp3xxx_wdt_setup(struct device *dev, u32 timeout) > > I'd suggest documenting `timeout' at least. Why would one want to set > it to zero and what effect does that have? What are its units? See below. > The patch adds a global symbol but fails to declare that symbol in a > header file. This is always wrong. I admit the aproach was quite wrong. I exported the function but wanted the watchdog driver to be the only user (and so the parameters of the function were quite specific to the watchdog driver). Thus, I kind of tried to hide its interface for other users. I will try to come up with something better, probably passing the function via the platform_device created at runtime. I also played with sharing resources, yet that would require a top-level driver managing the resources and two sub-level drivers (RTC and watchdog) which feels unneccessary complex to me. Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature