* [PATCH v6 0/5] watchdog: orion_wdt & at91sam9_wdt: improve dt support @ 2012-10-01 12:24 Fabio Porcedda [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:24 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, Jason Cooper, Andrew Lunn Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Hi all, This set of patches is for adding device tree support to the at91sam9_wdt driver and to add the new timeout property to the watchdog core. I've tested the at91sam9_wdt modifications on an at91sam9260 board (evk-pro3). Changes: v6: - add watchdog core proprty timeout binding - add as example to the orion_wdt driver the new timeout binding v5: - split dts commit in two, one for socs and one for boards. v4: - use "atmel,at91sam9260-wdt" as compatible v3: - add heartbeat option - default disabled for all the soc, enable only for evk-pro3 board - add at91sam9263 and at91sam9g45 soc v2: - add missing to and cc addresses Fabio Porcedda (5): watchdog: core: dt: add support for the timeout device tree property watchdog: orion_wdt: dt: add the timeout property binding watchdog: at91sam9_wdt: add device tree support ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45 ARM: at91/dts: evk-pro3: enable watchdog .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ .../devicetree/bindings/watchdog/marvel.txt | 5 +++++ Documentation/watchdog/watchdog-kernel-api.txt | 3 +++ arch/arm/boot/dts/at91sam9260.dtsi | 6 ++++++ arch/arm/boot/dts/at91sam9263.dtsi | 6 ++++++ arch/arm/boot/dts/at91sam9g45.dtsi | 6 ++++++ arch/arm/boot/dts/evk-pro3.dts | 4 ++++ drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ drivers/watchdog/orion_wdt.c | 2 +- include/linux/watchdog.h | 11 +++++++++++ 10 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt -- 1.7.11.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2012-10-01 12:24 ` Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding Fabio Porcedda ` (3 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:24 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, Jason Cooper, Andrew Lunn Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- Documentation/watchdog/watchdog-kernel-api.txt | 3 +++ include/linux/watchdog.h | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt index 086638f..decd981 100644 --- a/Documentation/watchdog/watchdog-kernel-api.txt +++ b/Documentation/watchdog/watchdog-kernel-api.txt @@ -212,3 +212,6 @@ driver specific data to and a pointer to the data itself. The watchdog_get_drvdata function allows you to retrieve driver specific data. The argument of this function is the watchdog device where you want to retrieve data from. The function returns the pointer to the driver specific data. + +The watchdog_probe_dt function allows you to retrieve the timeout property +from the device tree. diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index da70f0f..06f9384 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -11,6 +11,7 @@ #include <linux/ioctl.h> #include <linux/types.h> +#include <linux/of.h> #define WATCHDOG_IOCTL_BASE 'W' @@ -174,6 +175,16 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd) return wdd->driver_data; } +/* Use the following function to retrieve the timeout property from dt */ +static inline void watchdog_probe_dt(struct watchdog_device *wdd, + struct device_node *node) +{ + if (!node) + return; + + of_property_read_u32(node, "timeout", &wdd->timeout); +} + /* drivers/watchdog/core/watchdog_core.c */ extern int watchdog_register_device(struct watchdog_device *); extern void watchdog_unregister_device(struct watchdog_device *); -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2012-10-01 12:24 ` [PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property Fabio Porcedda @ 2012-10-01 12:24 ` Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support Fabio Porcedda ` (2 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:24 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, Jason Cooper, Andrew Lunn Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ The binding is provided by the watchdog core. Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++ drivers/watchdog/orion_wdt.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt index 0b2503a..66c2b8c 100644 --- a/Documentation/devicetree/bindings/watchdog/marvel.txt +++ b/Documentation/devicetree/bindings/watchdog/marvel.txt @@ -5,10 +5,15 @@ Required Properties: - Compatibility : "marvell,orion-wdt" - reg : Address of the timer registers +Optional properties: + +- timeout : Contains the watchdog timeout in seconds + Example: wdt@20300 { compatible = "marvell,orion-wdt"; reg = <0x20300 0x28>; + timeout = <10>; status = "okay"; }; diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c index c20f96b..3dc7481 100644 --- a/drivers/watchdog/orion_wdt.c +++ b/drivers/watchdog/orion_wdt.c @@ -24,7 +24,6 @@ #include <linux/spinlock.h> #include <linux/clk.h> #include <linux/err.h> -#include <linux/of.h> #include <mach/bridge-regs.h> /* @@ -168,6 +167,7 @@ static int __devinit orion_wdt_probe(struct platform_device *pdev) orion_wdt.timeout = heartbeat; orion_wdt.min_timeout = 1; orion_wdt.max_timeout = wdt_max_duration; + watchdog_probe_dt(&orion_wdt, pdev->dev.of_node); watchdog_set_nowayout(&orion_wdt, nowayout); ret = watchdog_register_device(&orion_wdt); -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2012-10-01 12:24 ` [PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding Fabio Porcedda @ 2012-10-01 12:24 ` Fabio Porcedda 2012-10-01 12:45 ` Andrew Lunn 2012-10-01 12:24 ` [PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45 Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog Fabio Porcedda 4 siblings, 1 reply; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:24 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, Jason Cooper, Andrew Lunn Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Tested on an at91sam9260 board (evk-pro3) Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt new file mode 100644 index 0000000..65c1755 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt @@ -0,0 +1,19 @@ +* Atmel Watchdog Timers + +** at91sam9-wdt + +Required properties: +- compatible: must be "atmel,at91sam9260-wdt". +- reg: physical base address of the controller and length of memory mapped + region. + +Optional properties: +- timeout: contains the watchdog timeout in seconds. + +Example: + + watchdog@fffffd40 { + compatible = "atmel,at91sam9260-wdt"; + reg = <0xfffffd40 0x10>; + timeout = <10>; + }; diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c index 05e1be8..c9e6bfa 100644 --- a/drivers/watchdog/at91sam9_wdt.c +++ b/drivers/watchdog/at91sam9_wdt.c @@ -32,6 +32,7 @@ #include <linux/timer.h> #include <linux/bitops.h> #include <linux/uaccess.h> +#include <linux/of.h> #include "at91sam9_wdt.h" @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { .fops = &at91wdt_fops, }; +static inline void __init at91wdt_probe_dt(struct device_node *node) +{ + if (!node) + return; + + of_property_read_u32(node, "timeout", &heartbeat); +} + static int __init at91wdt_probe(struct platform_device *pdev) { struct resource *r; @@ -272,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev) return -ENOMEM; } + at91wdt_probe_dt(pdev->dev.of_node); + /* Set watchdog */ res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000)); if (res) @@ -302,11 +313,21 @@ static int __exit at91wdt_remove(struct platform_device *pdev) return res; } +#if defined(CONFIG_OF) +static const struct of_device_id at91_wdt_dt_ids[] = { + { .compatible = "atmel,at91sam9260-wdt" }, + { /* sentinel */ } +}; + +MODULE_DEVICE_TABLE(of, at91_wdt_dt_ids); +#endif + static struct platform_driver at91wdt_driver = { .remove = __exit_p(at91wdt_remove), .driver = { .name = "at91_wdt", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(at91_wdt_dt_ids), }, }; -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support 2012-10-01 12:24 ` [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support Fabio Porcedda @ 2012-10-01 12:45 ` Andrew Lunn [not found] ` <20121001124546.GE11837-g2DYL2Zd6BY@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Lunn @ 2012-10-01 12:45 UTC (permalink / raw) To: Fabio Porcedda Cc: Andrew Lunn, linux-watchdog, devicetree-discuss, Nicolas Ferre, Wim Van Sebroeck, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, linux-arm-kernel, Jason Cooper 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 <fabio.porcedda@gmail.com> > --- > .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ > drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ > 2 files changed, 40 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > > diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > new file mode 100644 > index 0000000..65c1755 > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > @@ -0,0 +1,19 @@ > +* Atmel Watchdog Timers > + > +** at91sam9-wdt > + > +Required properties: > +- compatible: must be "atmel,at91sam9260-wdt". > +- reg: physical base address of the controller and length of memory mapped > + region. > + > +Optional properties: > +- timeout: contains the watchdog timeout in seconds. > + > +Example: > + > + watchdog@fffffd40 { > + compatible = "atmel,at91sam9260-wdt"; > + reg = <0xfffffd40 0x10>; > + timeout = <10>; > + }; > diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c > index 05e1be8..c9e6bfa 100644 > --- a/drivers/watchdog/at91sam9_wdt.c > +++ b/drivers/watchdog/at91sam9_wdt.c > @@ -32,6 +32,7 @@ > #include <linux/timer.h> > #include <linux/bitops.h> > #include <linux/uaccess.h> > +#include <linux/of.h> > > #include "at91sam9_wdt.h" > > @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { > .fops = &at91wdt_fops, > }; > > +static inline void __init at91wdt_probe_dt(struct device_node *node) > +{ > + if (!node) > + return; > + > + of_property_read_u32(node, "timeout", &heartbeat); > +} > + 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? Thanks Andrew ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20121001124546.GE11837-g2DYL2Zd6BY@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <20121001124546.GE11837-g2DYL2Zd6BY@public.gmane.org> @ 2012-10-01 12:48 ` Fabio Porcedda [not found] ` <CAHkwnC_u+bMX7REU5G3u=cm3VGmOUn+AShruw+ERkPJWtZpxUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:48 UTC (permalink / raw) To: Andrew Lunn Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> --- >> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ >> drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ >> 2 files changed, 40 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >> >> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >> new file mode 100644 >> index 0000000..65c1755 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >> @@ -0,0 +1,19 @@ >> +* Atmel Watchdog Timers >> + >> +** at91sam9-wdt >> + >> +Required properties: >> +- compatible: must be "atmel,at91sam9260-wdt". >> +- reg: physical base address of the controller and length of memory mapped >> + region. >> + >> +Optional properties: >> +- timeout: contains the watchdog timeout in seconds. >> + >> +Example: >> + >> + watchdog@fffffd40 { >> + compatible = "atmel,at91sam9260-wdt"; >> + reg = <0xfffffd40 0x10>; >> + timeout = <10>; >> + }; >> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c >> index 05e1be8..c9e6bfa 100644 >> --- a/drivers/watchdog/at91sam9_wdt.c >> +++ b/drivers/watchdog/at91sam9_wdt.c >> @@ -32,6 +32,7 @@ >> #include <linux/timer.h> >> #include <linux/bitops.h> >> #include <linux/uaccess.h> >> +#include <linux/of.h> >> >> #include "at91sam9_wdt.h" >> >> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { >> .fops = &at91wdt_fops, >> }; >> >> +static inline void __init at91wdt_probe_dt(struct device_node *node) >> +{ >> + if (!node) >> + return; >> + >> + of_property_read_u32(node, "timeout", &heartbeat); >> +} >> + > > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAHkwnC_u+bMX7REU5G3u=cm3VGmOUn+AShruw+ERkPJWtZpxUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <CAHkwnC_u+bMX7REU5G3u=cm3VGmOUn+AShruw+ERkPJWtZpxUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2012-10-01 12:54 ` Fabio Porcedda [not found] ` <CAHkwnC_5wRgf-DGnUfzkrreT_E1v8i=G8U0qo_5t1DY6+L7c+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-10-01 12:56 ` Andrew Lunn 1 sibling, 1 reply; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:54 UTC (permalink / raw) To: Andrew Lunn Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >>> --- >>> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ >>> drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ >>> 2 files changed, 40 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >>> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >>> new file mode 100644 >>> index 0000000..65c1755 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >>> @@ -0,0 +1,19 @@ >>> +* Atmel Watchdog Timers >>> + >>> +** at91sam9-wdt >>> + >>> +Required properties: >>> +- compatible: must be "atmel,at91sam9260-wdt". >>> +- reg: physical base address of the controller and length of memory mapped >>> + region. >>> + >>> +Optional properties: >>> +- timeout: contains the watchdog timeout in seconds. >>> + >>> +Example: >>> + >>> + watchdog@fffffd40 { >>> + compatible = "atmel,at91sam9260-wdt"; >>> + reg = <0xfffffd40 0x10>; >>> + timeout = <10>; >>> + }; >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c >>> index 05e1be8..c9e6bfa 100644 >>> --- a/drivers/watchdog/at91sam9_wdt.c >>> +++ b/drivers/watchdog/at91sam9_wdt.c >>> @@ -32,6 +32,7 @@ >>> #include <linux/timer.h> >>> #include <linux/bitops.h> >>> #include <linux/uaccess.h> >>> +#include <linux/of.h> >>> >>> #include "at91sam9_wdt.h" >>> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { >>> .fops = &at91wdt_fops, >>> }; >>> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node) >>> +{ >>> + if (!node) >>> + return; >>> + >>> + of_property_read_u32(node, "timeout", &heartbeat); >>> +} >>> + >> >> 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. When the at91sam9_wdt is converted to the watchdog core framework, the driver can use finally the new function. Best regards -- Fabio Porcedda ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAHkwnC_5wRgf-DGnUfzkrreT_E1v8i=G8U0qo_5t1DY6+L7c+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <CAHkwnC_5wRgf-DGnUfzkrreT_E1v8i=G8U0qo_5t1DY6+L7c+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2012-10-01 13:06 ` Andrew Lunn [not found] ` <20121001130658.GG11837-g2DYL2Zd6BY@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Lunn @ 2012-10-01 13:06 UTC (permalink / raw) To: Fabio Porcedda Cc: Andrew Lunn, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote: > On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > >>> --- > >>> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ > >>> drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ > >>> 2 files changed, 40 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >>> new file mode 100644 > >>> index 0000000..65c1755 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >>> @@ -0,0 +1,19 @@ > >>> +* Atmel Watchdog Timers > >>> + > >>> +** at91sam9-wdt > >>> + > >>> +Required properties: > >>> +- compatible: must be "atmel,at91sam9260-wdt". > >>> +- reg: physical base address of the controller and length of memory mapped > >>> + region. > >>> + > >>> +Optional properties: > >>> +- timeout: contains the watchdog timeout in seconds. > >>> + > >>> +Example: > >>> + > >>> + watchdog@fffffd40 { > >>> + compatible = "atmel,at91sam9260-wdt"; > >>> + reg = <0xfffffd40 0x10>; > >>> + timeout = <10>; > >>> + }; > >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c > >>> index 05e1be8..c9e6bfa 100644 > >>> --- a/drivers/watchdog/at91sam9_wdt.c > >>> +++ b/drivers/watchdog/at91sam9_wdt.c > >>> @@ -32,6 +32,7 @@ > >>> #include <linux/timer.h> > >>> #include <linux/bitops.h> > >>> #include <linux/uaccess.h> > >>> +#include <linux/of.h> > >>> > >>> #include "at91sam9_wdt.h" > >>> > >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { > >>> .fops = &at91wdt_fops, > >>> }; > >>> > >>> +static inline void __init at91wdt_probe_dt(struct device_node *node) > >>> +{ > >>> + if (!node) > >>> + return; > >>> + > >>> + of_property_read_u32(node, "timeout", &heartbeat); > >>> +} > >>> + > >> > >> 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. Andrew ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20121001130658.GG11837-g2DYL2Zd6BY@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <20121001130658.GG11837-g2DYL2Zd6BY@public.gmane.org> @ 2012-10-02 15:04 ` Fabio Porcedda 2012-10-02 19:00 ` Jean-Christophe PLAGNIOL-VILLARD [not found] ` <CAHkwnC83DcKXZOtLse_xnYe7zOZ5qwGJVimLkBzDwSJ7Y5vL_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 2 replies; 17+ messages in thread From: Fabio Porcedda @ 2012-10-02 15:04 UTC (permalink / raw) To: Andrew Lunn Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> >>> --- >> >>> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ >> >>> drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ >> >>> 2 files changed, 40 insertions(+) >> >>> create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >> >>> >> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >> >>> new file mode 100644 >> >>> index 0000000..65c1755 >> >>> --- /dev/null >> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt >> >>> @@ -0,0 +1,19 @@ >> >>> +* Atmel Watchdog Timers >> >>> + >> >>> +** at91sam9-wdt >> >>> + >> >>> +Required properties: >> >>> +- compatible: must be "atmel,at91sam9260-wdt". >> >>> +- reg: physical base address of the controller and length of memory mapped >> >>> + region. >> >>> + >> >>> +Optional properties: >> >>> +- timeout: contains the watchdog timeout in seconds. >> >>> + >> >>> +Example: >> >>> + >> >>> + watchdog@fffffd40 { >> >>> + compatible = "atmel,at91sam9260-wdt"; >> >>> + reg = <0xfffffd40 0x10>; >> >>> + timeout = <10>; >> >>> + }; >> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c >> >>> index 05e1be8..c9e6bfa 100644 >> >>> --- a/drivers/watchdog/at91sam9_wdt.c >> >>> +++ b/drivers/watchdog/at91sam9_wdt.c >> >>> @@ -32,6 +32,7 @@ >> >>> #include <linux/timer.h> >> >>> #include <linux/bitops.h> >> >>> #include <linux/uaccess.h> >> >>> +#include <linux/of.h> >> >>> >> >>> #include "at91sam9_wdt.h" >> >>> >> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { >> >>> .fops = &at91wdt_fops, >> >>> }; >> >>> >> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node) >> >>> +{ >> >>> + if (!node) >> >>> + return; >> >>> + >> >>> + of_property_read_u32(node, "timeout", &heartbeat); >> >>> +} >> >>> + >> >> >> >> 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? Best regards -- Fabio Porcedda ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support 2012-10-02 15:04 ` Fabio Porcedda @ 2012-10-02 19:00 ` Jean-Christophe PLAGNIOL-VILLARD [not found] ` <20121002190020.GD5117-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org> [not found] ` <CAHkwnC83DcKXZOtLse_xnYe7zOZ5qwGJVimLkBzDwSJ7Y5vL_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-02 19:00 UTC (permalink / raw) To: Fabio Porcedda Cc: Andrew Lunn, linux-watchdog, devicetree-discuss, Nicolas Ferre, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel, Jason Cooper On 17:04 Tue 02 Oct , Fabio Porcedda wrote: > On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> 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 <fabio.porcedda@gmail.com> wrote: > >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> 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 <fabio.porcedda@gmail.com> > >> >>> --- > >> >>> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ > >> >>> drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ > >> >>> 2 files changed, 40 insertions(+) > >> >>> create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >> >>> > >> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >> >>> new file mode 100644 > >> >>> index 0000000..65c1755 > >> >>> --- /dev/null > >> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >> >>> @@ -0,0 +1,19 @@ > >> >>> +* Atmel Watchdog Timers > >> >>> + > >> >>> +** at91sam9-wdt > >> >>> + > >> >>> +Required properties: > >> >>> +- compatible: must be "atmel,at91sam9260-wdt". > >> >>> +- reg: physical base address of the controller and length of memory mapped > >> >>> + region. > >> >>> + > >> >>> +Optional properties: > >> >>> +- timeout: contains the watchdog timeout in seconds. > >> >>> + > >> >>> +Example: > >> >>> + > >> >>> + watchdog@fffffd40 { > >> >>> + compatible = "atmel,at91sam9260-wdt"; > >> >>> + reg = <0xfffffd40 0x10>; > >> >>> + timeout = <10>; > >> >>> + }; > >> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c > >> >>> index 05e1be8..c9e6bfa 100644 > >> >>> --- a/drivers/watchdog/at91sam9_wdt.c > >> >>> +++ b/drivers/watchdog/at91sam9_wdt.c > >> >>> @@ -32,6 +32,7 @@ > >> >>> #include <linux/timer.h> > >> >>> #include <linux/bitops.h> > >> >>> #include <linux/uaccess.h> > >> >>> +#include <linux/of.h> > >> >>> > >> >>> #include "at91sam9_wdt.h" > >> >>> > >> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { > >> >>> .fops = &at91wdt_fops, > >> >>> }; > >> >>> > >> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node) > >> >>> +{ > >> >>> + if (!node) > >> >>> + return; > >> >>> + > >> >>> + of_property_read_u32(node, "timeout", &heartbeat); > >> >>> +} > >> >>> + > >> >> > >> >> 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 Best Regards, J. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20121002190020.GD5117-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <20121002190020.GD5117-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org> @ 2012-10-03 16:32 ` Fabio Porcedda 2012-10-03 17:47 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 17+ messages in thread From: Fabio Porcedda @ 2012-10-03 16:32 UTC (permalink / raw) To: Jean-Christophe PLAGNIOL-VILLARD Cc: Andrew Lunn, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org> wrote: > On 17:04 Tue 02 Oct , Fabio Porcedda wrote: >> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> >> >>> --- >> >> >>> .../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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support 2012-10-03 16:32 ` Fabio Porcedda @ 2012-10-03 17:47 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 0 replies; 17+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-10-03 17:47 UTC (permalink / raw) To: Fabio Porcedda Cc: Andrew Lunn, linux-watchdog, devicetree-discuss, Nicolas Ferre, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel, Jason Cooper On 18:32 Wed 03 Oct , Fabio Porcedda wrote: > On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD > <plagnioj@jcrosoft.com> wrote: > > On 17:04 Tue 02 Oct , Fabio Porcedda wrote: > >> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> 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 <fabio.porcedda@gmail.com> wrote: > >> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> 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 <fabio.porcedda@gmail.com> > >> >> >>> --- > >> >> >>> .../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? no a global helper not watchdog specific for timeout binding that why I said of.h not of_watchdog.h Best Regards, J. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAHkwnC83DcKXZOtLse_xnYe7zOZ5qwGJVimLkBzDwSJ7Y5vL_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <CAHkwnC83DcKXZOtLse_xnYe7zOZ5qwGJVimLkBzDwSJ7Y5vL_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2012-10-02 19:59 ` Andrew Lunn [not found] ` <20121002195914.GG21046-g2DYL2Zd6BY@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Andrew Lunn @ 2012-10-02 19:59 UTC (permalink / raw) To: Fabio Porcedda Cc: Andrew Lunn, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper > 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); > } You forgot to change the function signature. Also, if you are adding a generic function, it should be a generic function for the framework. All drivers should be slowly moving towards the framework, so adding functions which help you not move towards the framework are wrong. Andrew ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <20121002195914.GG21046-g2DYL2Zd6BY@public.gmane.org>]
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <20121002195914.GG21046-g2DYL2Zd6BY@public.gmane.org> @ 2012-10-03 16:32 ` Fabio Porcedda 0 siblings, 0 replies; 17+ messages in thread From: Fabio Porcedda @ 2012-10-03 16:32 UTC (permalink / raw) To: Andrew Lunn Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Tue, Oct 2, 2012 at 9:59 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote: >> 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); >> } > > You forgot to change the function signature. > > Also, if you are adding a generic function, it should be a generic > function for the framework. All drivers should be slowly moving > towards the framework, so adding functions which help you not move > towards the framework are wrong. Maybe i can use a framework specific function and use a dummy watchdog_device for the non framerwork drivers. If it's ok i will send an updated patch. Best regards and thanks for the review. -- Fabio Porcedda ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support [not found] ` <CAHkwnC_u+bMX7REU5G3u=cm3VGmOUn+AShruw+ERkPJWtZpxUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-10-01 12:54 ` Fabio Porcedda @ 2012-10-01 12:56 ` Andrew Lunn 1 sibling, 0 replies; 17+ messages in thread From: Andrew Lunn @ 2012-10-01 12:56 UTC (permalink / raw) To: Fabio Porcedda Cc: Andrew Lunn, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Wim Van Sebroeck, Andrew Victor, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper On Mon, Oct 01, 2012 at 02:48:01PM +0200, Fabio Porcedda wrote: > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> 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 <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > >> --- > >> .../devicetree/bindings/watchdog/atmel-wdt.txt | 19 +++++++++++++++++++ > >> drivers/watchdog/at91sam9_wdt.c | 21 +++++++++++++++++++++ > >> 2 files changed, 40 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >> > >> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >> new file mode 100644 > >> index 0000000..65c1755 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt > >> @@ -0,0 +1,19 @@ > >> +* Atmel Watchdog Timers > >> + > >> +** at91sam9-wdt > >> + > >> +Required properties: > >> +- compatible: must be "atmel,at91sam9260-wdt". > >> +- reg: physical base address of the controller and length of memory mapped > >> + region. > >> + > >> +Optional properties: > >> +- timeout: contains the watchdog timeout in seconds. > >> + > >> +Example: > >> + > >> + watchdog@fffffd40 { > >> + compatible = "atmel,at91sam9260-wdt"; > >> + reg = <0xfffffd40 0x10>; > >> + timeout = <10>; > >> + }; > >> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c > >> index 05e1be8..c9e6bfa 100644 > >> --- a/drivers/watchdog/at91sam9_wdt.c > >> +++ b/drivers/watchdog/at91sam9_wdt.c > >> @@ -32,6 +32,7 @@ > >> #include <linux/timer.h> > >> #include <linux/bitops.h> > >> #include <linux/uaccess.h> > >> +#include <linux/of.h> > >> > >> #include "at91sam9_wdt.h" > >> > >> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = { > >> .fops = &at91wdt_fops, > >> }; > >> > >> +static inline void __init at91wdt_probe_dt(struct device_node *node) > >> +{ > >> + if (!node) > >> + return; > >> + > >> + of_property_read_u32(node, "timeout", &heartbeat); > >> +} > >> + > > > > 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? It makes little sense doing this here, because you are going to have to clean this up sometime very soon and use the helper, unless there is a very good reason not to use the helper. Andrew ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45 [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> ` (2 preceding siblings ...) 2012-10-01 12:24 ` [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support Fabio Porcedda @ 2012-10-01 12:24 ` Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog Fabio Porcedda 4 siblings, 0 replies; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:24 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, Jason Cooper, Andrew Lunn Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Tested on an at91sam9260 board (evk-pro3). Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- arch/arm/boot/dts/at91sam9260.dtsi | 6 ++++++ arch/arm/boot/dts/at91sam9263.dtsi | 6 ++++++ arch/arm/boot/dts/at91sam9g45.dtsi | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi index 7c95f76..7612193 100644 --- a/arch/arm/boot/dts/at91sam9260.dtsi +++ b/arch/arm/boot/dts/at91sam9260.dtsi @@ -236,6 +236,12 @@ trigger-external; }; }; + + watchdog@fffffd40 { + compatible = "atmel,at91sam9260-wdt"; + reg = <0xfffffd40 0x10>; + status = "disabled"; + }; }; nand0: nand@40000000 { diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi index 195019b..d822240 100644 --- a/arch/arm/boot/dts/at91sam9263.dtsi +++ b/arch/arm/boot/dts/at91sam9263.dtsi @@ -185,6 +185,12 @@ interrupts = <24 4 2>; status = "disabled"; }; + + watchdog@fffffd40 { + compatible = "atmel,at91sam9260-wdt"; + reg = <0xfffffd40 0x10>; + status = "disabled"; + }; }; nand0: nand@40000000 { diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi index 63751b1..df998ab 100644 --- a/arch/arm/boot/dts/at91sam9g45.dtsi +++ b/arch/arm/boot/dts/at91sam9g45.dtsi @@ -242,6 +242,12 @@ trigger-value = <0x6>; }; }; + + watchdog@fffffd40 { + compatible = "atmel,at91sam9260-wdt"; + reg = <0xfffffd40 0x10>; + status = "disabled"; + }; }; nand0: nand@40000000 { -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> ` (3 preceding siblings ...) 2012-10-01 12:24 ` [PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45 Fabio Porcedda @ 2012-10-01 12:24 ` Fabio Porcedda 4 siblings, 0 replies; 17+ messages in thread From: Fabio Porcedda @ 2012-10-01 12:24 UTC (permalink / raw) To: Wim Van Sebroeck, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Nicolas Ferre, Jean-Christophe PLAGNIOL-VILLARD, Andrew Victor, Jason Cooper, Andrew Lunn Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Tested on evk-pro3. Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- arch/arm/boot/dts/evk-pro3.dts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/evk-pro3.dts b/arch/arm/boot/dts/evk-pro3.dts index b7354e6..ce959ee 100644 --- a/arch/arm/boot/dts/evk-pro3.dts +++ b/arch/arm/boot/dts/evk-pro3.dts @@ -26,6 +26,10 @@ atmel,vbus-gpio = <&pioC 5 0>; status = "okay"; }; + + watchdog@fffffd40 { + status = "okay"; + }; }; usb0: ohci@00500000 { -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-10-03 17:47 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-01 12:24 [PATCH v6 0/5] watchdog: orion_wdt & at91sam9_wdt: improve dt support Fabio Porcedda [not found] ` <1349094281-28889-1-git-send-email-fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2012-10-01 12:24 ` [PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support Fabio Porcedda 2012-10-01 12:45 ` Andrew Lunn [not found] ` <20121001124546.GE11837-g2DYL2Zd6BY@public.gmane.org> 2012-10-01 12:48 ` Fabio Porcedda [not found] ` <CAHkwnC_u+bMX7REU5G3u=cm3VGmOUn+AShruw+ERkPJWtZpxUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-10-01 12:54 ` Fabio Porcedda [not found] ` <CAHkwnC_5wRgf-DGnUfzkrreT_E1v8i=G8U0qo_5t1DY6+L7c+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-10-01 13:06 ` Andrew Lunn [not found] ` <20121001130658.GG11837-g2DYL2Zd6BY@public.gmane.org> 2012-10-02 15:04 ` Fabio Porcedda 2012-10-02 19:00 ` Jean-Christophe PLAGNIOL-VILLARD [not found] ` <20121002190020.GD5117-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org> 2012-10-03 16:32 ` Fabio Porcedda 2012-10-03 17:47 ` Jean-Christophe PLAGNIOL-VILLARD [not found] ` <CAHkwnC83DcKXZOtLse_xnYe7zOZ5qwGJVimLkBzDwSJ7Y5vL_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-10-02 19:59 ` Andrew Lunn [not found] ` <20121002195914.GG21046-g2DYL2Zd6BY@public.gmane.org> 2012-10-03 16:32 ` Fabio Porcedda 2012-10-01 12:56 ` Andrew Lunn 2012-10-01 12:24 ` [PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45 Fabio Porcedda 2012-10-01 12:24 ` [PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog Fabio Porcedda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).