On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD <[email protected]> wrote: > On 17:04 Tue 02 Oct , Fabio Porcedda wrote: >> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <[email protected]> wrote: >> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote: >> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <[email protected]> >> >> wrote: >> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <[email protected]> wrote: >> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote: >> >> >>> Tested on an at91sam9260 board (evk-pro3) >> >> >>> >> >> >>> Signed-off-by: Fabio Porcedda <[email protected]> >> >> >>> --- >> >> >>> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 >> >> >>> +++++++++++++++++++ >> >> >>> drivers/watchdog/at91sam9_wdt.c | 21 >> >> >>> +++++++++++++++++++++ >> >> >>> 2 files changed, 40 insertions(+) >> >> >>> ... >> >> >> >> >> >> In patch #1 you add a function to do this, and then you don't make use >> >> >> of it here ? >> >> >> >> >> >> Or am i missing something? >> >> > >> >> > I'm using it on the patch #2 for the orion_wdt driver. >> >> > Do you think it's better to join the #1 and the #2 patch? >> >> > >> >> > Best regards >> >> > -- >> >> > Fabio Porcedda >> >> >> >> I'm sorry, only now i understand your question. >> >> The at91sam9_wdt driver don't use the watchdog core framework si i >> >> can't use that function cleanly. >> > >> >> The patch #1 and #2 are for introducing the same property as the >> >> at91sam9_wdt driver. >> > >> > So maybe split this up into two different patchsets. One patchset to >> > add the helper function, and the use of this helper to all watchdog >> > divers that can use it. I think the following drivers should be >> > modified: >> > >> > orion_wdt.c >> > pnx4008_wdt.c >> > s3c2410_wdt.c >> > >> > In a second patchset, convert the AT91SAM9 driver over to the watchdog >> > core framework, and then use the helper function. >> >> I was thinking to add a more generic helper function like this: >> >> static inline void watchdog_get_dttimeout(struct device_node *node, >> u32 *timeout) >> { >> if (node) >> of_property_read_u32(node, "timeout", &wdd->timeout); >> } >> >> This way i can use this helper function in the at91sam9_wdt driver too. >> What do you think? > timeout_sec and this can be move at of.h level > > as this is not watchdog framework secific
I can not find any property with the "_sec" suffix, you think it's still fine to use that suffix? You are speaking about a of_watchdog.h header with a of_watchdog_get_timeout function? Best regards and thanks for the review. -- Fabio Porcedda _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
