* [PATCH 0/3] watchdog: omap: several cleanups
@ 2015-04-24 9:48 Uwe Kleine-König
2015-04-24 9:48 ` [PATCH 1/3] watchdog: omap: use watchdog_init_timeout Uwe Kleine-König
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Uwe Kleine-König @ 2015-04-24 9:48 UTC (permalink / raw)
To: linux-watchdog, Wim Van Sebroeck; +Cc: Lokesh Vutla, Felipe Balbi, kernel
Hello,
this is the current state of my cleanups for the omap watchdog driver.
As overtaking a running watchdog isn't that trivial as expected I'm
sending this out that it doesn't bitrot.
Best regards
Uwe Kleine-König
Uwe Kleine-König (3):
watchdog: omap: use watchdog_init_timeout
watchdog: omap: put struct watchdog_device into driver data
watchdog: omap: simplify assignment of bootstatus
.../devicetree/bindings/watchdog/omap-wdt.txt | 9 +--
drivers/watchdog/omap_wdt.c | 66 +++++++++-------------
2 files changed, 32 insertions(+), 43 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 1/3] watchdog: omap: use watchdog_init_timeout 2015-04-24 9:48 [PATCH 0/3] watchdog: omap: several cleanups Uwe Kleine-König @ 2015-04-24 9:48 ` Uwe Kleine-König 2015-04-24 14:42 ` Felipe Balbi 2015-04-24 9:48 ` [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data Uwe Kleine-König 2015-04-24 9:48 ` [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus Uwe Kleine-König 2 siblings, 1 reply; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 9:48 UTC (permalink / raw) To: linux-watchdog, Wim Van Sebroeck; +Cc: Lokesh Vutla, Felipe Balbi, kernel Instead of (partly) open coding it use the core function. As a side effect the "timeout-sec" devicetree property is used now. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 9 +++++---- drivers/watchdog/omap_wdt.c | 5 +---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt index c227970671ea..4f18ec38be2f 100644 --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt @@ -1,10 +1,11 @@ TI Watchdog Timer (WDT) Controller for OMAP Required properties: -compatible: -- "ti,omap3-wdt" for OMAP3 -- "ti,omap4-wdt" for OMAP4 -- ti,hwmods: Name of the hwmod associated to the WDT +- compatible : "ti,omap3-wdt" (for OMAP3) or "ti,omap4-wdt" (for OMAP4) +- ti,hwmods : Name of the hwmod associated to the WDT + +Optional properties: +- timeout-sec : default watchdog timeout in seconds Examples: diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c index 39a6cfcba016..0eb9ca04e672 100644 --- a/drivers/watchdog/omap_wdt.c +++ b/drivers/watchdog/omap_wdt.c @@ -234,10 +234,7 @@ static int omap_wdt_probe(struct platform_device *pdev) omap_wdt->min_timeout = TIMER_MARGIN_MIN; omap_wdt->max_timeout = TIMER_MARGIN_MAX; - if (timer_margin >= TIMER_MARGIN_MIN && - timer_margin <= TIMER_MARGIN_MAX) - omap_wdt->timeout = timer_margin; - else + if (watchdog_init_timeout(omap_wdt, timer_margin, &pdev->dev) < 0) omap_wdt->timeout = TIMER_MARGIN_DEFAULT; watchdog_set_drvdata(omap_wdt, wdev); -- 2.1.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] watchdog: omap: use watchdog_init_timeout @ 2015-04-24 14:42 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-24 14:42 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel, Linux OMAP Mailing List [-- Attachment #1: Type: text/plain, Size: 2139 bytes --] Hi, On Fri, Apr 24, 2015 at 11:48:31AM +0200, Uwe Kleine-König wrote: > Instead of (partly) open coding it use the core function. As a side > effect the "timeout-sec" devicetree property is used now. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 9 +++++---- > drivers/watchdog/omap_wdt.c | 5 +---- > 2 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > index c227970671ea..4f18ec38be2f 100644 > --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > @@ -1,10 +1,11 @@ > TI Watchdog Timer (WDT) Controller for OMAP > > Required properties: > -compatible: > -- "ti,omap3-wdt" for OMAP3 > -- "ti,omap4-wdt" for OMAP4 > -- ti,hwmods: Name of the hwmod associated to the WDT > +- compatible : "ti,omap3-wdt" (for OMAP3) or "ti,omap4-wdt" (for OMAP4) > +- ti,hwmods : Name of the hwmod associated to the WDT > + > +Optional properties: > +- timeout-sec : default watchdog timeout in seconds adding a new property here ? Why ? In fact, none of these DT binding doc changes fit in $subject. Sure, watchdog_init_timeout() needs timeout-sec, but no OMAP DT today passes it. > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 39a6cfcba016..0eb9ca04e672 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -234,10 +234,7 @@ static int omap_wdt_probe(struct platform_device *pdev) > omap_wdt->min_timeout = TIMER_MARGIN_MIN; > omap_wdt->max_timeout = TIMER_MARGIN_MAX; > > - if (timer_margin >= TIMER_MARGIN_MIN && > - timer_margin <= TIMER_MARGIN_MAX) > - omap_wdt->timeout = timer_margin; > - else > + if (watchdog_init_timeout(omap_wdt, timer_margin, &pdev->dev) < 0) > omap_wdt->timeout = TIMER_MARGIN_DEFAULT; > > watchdog_set_drvdata(omap_wdt, wdev); > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] watchdog: omap: use watchdog_init_timeout @ 2015-04-24 14:42 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-24 14:42 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Linux OMAP Mailing List [-- Attachment #1: Type: text/plain, Size: 2164 bytes --] Hi, On Fri, Apr 24, 2015 at 11:48:31AM +0200, Uwe Kleine-König wrote: > Instead of (partly) open coding it use the core function. As a side > effect the "timeout-sec" devicetree property is used now. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > --- > Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 9 +++++---- > drivers/watchdog/omap_wdt.c | 5 +---- > 2 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > index c227970671ea..4f18ec38be2f 100644 > --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > @@ -1,10 +1,11 @@ > TI Watchdog Timer (WDT) Controller for OMAP > > Required properties: > -compatible: > -- "ti,omap3-wdt" for OMAP3 > -- "ti,omap4-wdt" for OMAP4 > -- ti,hwmods: Name of the hwmod associated to the WDT > +- compatible : "ti,omap3-wdt" (for OMAP3) or "ti,omap4-wdt" (for OMAP4) > +- ti,hwmods : Name of the hwmod associated to the WDT > + > +Optional properties: > +- timeout-sec : default watchdog timeout in seconds adding a new property here ? Why ? In fact, none of these DT binding doc changes fit in $subject. Sure, watchdog_init_timeout() needs timeout-sec, but no OMAP DT today passes it. > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 39a6cfcba016..0eb9ca04e672 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -234,10 +234,7 @@ static int omap_wdt_probe(struct platform_device *pdev) > omap_wdt->min_timeout = TIMER_MARGIN_MIN; > omap_wdt->max_timeout = TIMER_MARGIN_MAX; > > - if (timer_margin >= TIMER_MARGIN_MIN && > - timer_margin <= TIMER_MARGIN_MAX) > - omap_wdt->timeout = timer_margin; > - else > + if (watchdog_init_timeout(omap_wdt, timer_margin, &pdev->dev) < 0) > omap_wdt->timeout = TIMER_MARGIN_DEFAULT; > > watchdog_set_drvdata(omap_wdt, wdev); > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] watchdog: omap: use watchdog_init_timeout 2015-04-24 14:42 ` Felipe Balbi @ 2015-04-24 19:02 ` Uwe Kleine-König -1 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 19:02 UTC (permalink / raw) To: Felipe Balbi Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, kernel, Linux OMAP Mailing List On Fri, Apr 24, 2015 at 09:42:59AM -0500, Felipe Balbi wrote: > Hi, > > On Fri, Apr 24, 2015 at 11:48:31AM +0200, Uwe Kleine-König wrote: > > Instead of (partly) open coding it use the core function. As a side > > effect the "timeout-sec" devicetree property is used now. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 9 +++++---- > > drivers/watchdog/omap_wdt.c | 5 +---- > > 2 files changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > > index c227970671ea..4f18ec38be2f 100644 > > --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > > +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > > @@ -1,10 +1,11 @@ > > TI Watchdog Timer (WDT) Controller for OMAP > > > > Required properties: > > -compatible: > > -- "ti,omap3-wdt" for OMAP3 > > -- "ti,omap4-wdt" for OMAP4 > > -- ti,hwmods: Name of the hwmod associated to the WDT > > +- compatible : "ti,omap3-wdt" (for OMAP3) or "ti,omap4-wdt" (for OMAP4) > > +- ti,hwmods : Name of the hwmod associated to the WDT > > + > > +Optional properties: > > +- timeout-sec : default watchdog timeout in seconds > > adding a new property here ? Why ? In fact, none of these DT binding doc > changes fit in $subject. Sure, watchdog_init_timeout() needs > timeout-sec, but no OMAP DT today passes it. watchdog_init_timeout doesn't *need* the new property. After all it's optional. The other changes are just reformatting to make it match the usual style and make it understandable. Could have mentioned it in the commitlog, that's true. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/3] watchdog: omap: use watchdog_init_timeout @ 2015-04-24 19:02 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 19:02 UTC (permalink / raw) To: Felipe Balbi Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, kernel, Linux OMAP Mailing List On Fri, Apr 24, 2015 at 09:42:59AM -0500, Felipe Balbi wrote: > Hi, > > On Fri, Apr 24, 2015 at 11:48:31AM +0200, Uwe Kleine-König wrote: > > Instead of (partly) open coding it use the core function. As a side > > effect the "timeout-sec" devicetree property is used now. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 9 +++++---- > > drivers/watchdog/omap_wdt.c | 5 +---- > > 2 files changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > > index c227970671ea..4f18ec38be2f 100644 > > --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > > +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > > @@ -1,10 +1,11 @@ > > TI Watchdog Timer (WDT) Controller for OMAP > > > > Required properties: > > -compatible: > > -- "ti,omap3-wdt" for OMAP3 > > -- "ti,omap4-wdt" for OMAP4 > > -- ti,hwmods: Name of the hwmod associated to the WDT > > +- compatible : "ti,omap3-wdt" (for OMAP3) or "ti,omap4-wdt" (for OMAP4) > > +- ti,hwmods : Name of the hwmod associated to the WDT > > + > > +Optional properties: > > +- timeout-sec : default watchdog timeout in seconds > > adding a new property here ? Why ? In fact, none of these DT binding doc > changes fit in $subject. Sure, watchdog_init_timeout() needs > timeout-sec, but no OMAP DT today passes it. watchdog_init_timeout doesn't *need* the new property. After all it's optional. The other changes are just reformatting to make it match the usual style and make it understandable. Could have mentioned it in the commitlog, that's true. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-24 20:20 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 20:20 UTC (permalink / raw) To: Felipe Balbi Cc: Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog, kernel ti,hwmods doesn't belong into the compatible section but is a property on it's own. Also reformat the section of required properties to match the usual style of dt binding documents. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello Felipe, what about this patch before the previously sent patch 1? Best regards Uwe Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt index c227970671ea..597e19d18dca 100644 --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt @@ -1,10 +1,8 @@ TI Watchdog Timer (WDT) Controller for OMAP Required properties: -compatible: -- "ti,omap3-wdt" for OMAP3 -- "ti,omap4-wdt" for OMAP4 -- ti,hwmods: Name of the hwmod associated to the WDT +- compatible : "ti,omap3-wdt" for OMAP3 or "ti,omap4-wdt" for OMAP4 +- ti,hwmods : Name of the hwmod associated to the WDT Examples: -- 2.1.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-24 20:20 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 20:20 UTC (permalink / raw) To: Felipe Balbi Cc: Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ ti,hwmods doesn't belong into the compatible section but is a property on it's own. Also reformat the section of required properties to match the usual style of dt binding documents. Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> --- Hello Felipe, what about this patch before the previously sent patch 1? Best regards Uwe Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt index c227970671ea..597e19d18dca 100644 --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt @@ -1,10 +1,8 @@ TI Watchdog Timer (WDT) Controller for OMAP Required properties: -compatible: -- "ti,omap3-wdt" for OMAP3 -- "ti,omap4-wdt" for OMAP4 -- ti,hwmods: Name of the hwmod associated to the WDT +- compatible : "ti,omap3-wdt" for OMAP3 or "ti,omap4-wdt" for OMAP4 +- ti,hwmods : Name of the hwmod associated to the WDT Examples: -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-25 2:10 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-25 2:10 UTC (permalink / raw) To: Uwe Kleine-König Cc: Felipe Balbi, Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog, kernel [-- Attachment #1: Type: text/plain, Size: 1416 bytes --] On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: > ti,hwmods doesn't belong into the compatible section but is a property > on it's own. Also reformat the section of required properties to match the > usual style of dt binding documents. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> looks great to me, thanks Reviewed-by: Felipe Balbi <balbi@ti.com> Acked-by: Felipe Balbi <balbi@ti.com> > --- > Hello Felipe, > > what about this patch before the previously sent patch 1? > > Best regards > Uwe > > Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > index c227970671ea..597e19d18dca 100644 > --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > @@ -1,10 +1,8 @@ > TI Watchdog Timer (WDT) Controller for OMAP > > Required properties: > -compatible: > -- "ti,omap3-wdt" for OMAP3 > -- "ti,omap4-wdt" for OMAP4 > -- ti,hwmods: Name of the hwmod associated to the WDT > +- compatible : "ti,omap3-wdt" for OMAP3 or "ti,omap4-wdt" for OMAP4 > +- ti,hwmods : Name of the hwmod associated to the WDT > > Examples: > > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-25 2:10 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-25 2:10 UTC (permalink / raw) To: Uwe Kleine-König Cc: Felipe Balbi, Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ [-- Attachment #1: Type: text/plain, Size: 1485 bytes --] On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: > ti,hwmods doesn't belong into the compatible section but is a property > on it's own. Also reformat the section of required properties to match the > usual style of dt binding documents. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> looks great to me, thanks Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> > --- > Hello Felipe, > > what about this patch before the previously sent patch 1? > > Best regards > Uwe > > Documentation/devicetree/bindings/watchdog/omap-wdt.txt | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > index c227970671ea..597e19d18dca 100644 > --- a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt > @@ -1,10 +1,8 @@ > TI Watchdog Timer (WDT) Controller for OMAP > > Required properties: > -compatible: > -- "ti,omap3-wdt" for OMAP3 > -- "ti,omap4-wdt" for OMAP4 > -- ti,hwmods: Name of the hwmod associated to the WDT > +- compatible : "ti,omap3-wdt" for OMAP3 or "ti,omap4-wdt" for OMAP4 > +- ti,hwmods : Name of the hwmod associated to the WDT > > Examples: > > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-26 15:29 ` Guenter Roeck 0 siblings, 0 replies; 27+ messages in thread From: Guenter Roeck @ 2015-04-26 15:29 UTC (permalink / raw) To: balbi, Uwe Kleine-König Cc: Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog, kernel On 04/24/2015 07:10 PM, Felipe Balbi wrote: > On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: >> ti,hwmods doesn't belong into the compatible section but is a property >> on it's own. Also reformat the section of required properties to match the >> usual style of dt binding documents. >> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > looks great to me, thanks > > Reviewed-by: Felipe Balbi <balbi@ti.com> > Acked-by: Felipe Balbi <balbi@ti.com> > Reviewed-by: and Acked-by: are mutually exclusive. Guenter ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-26 15:29 ` Guenter Roeck 0 siblings, 0 replies; 27+ messages in thread From: Guenter Roeck @ 2015-04-26 15:29 UTC (permalink / raw) To: balbi-l0cyMroinI0, Uwe Kleine-König Cc: Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ On 04/24/2015 07:10 PM, Felipe Balbi wrote: > On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: >> ti,hwmods doesn't belong into the compatible section but is a property >> on it's own. Also reformat the section of required properties to match the >> usual style of dt binding documents. >> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > > looks great to me, thanks > > Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> > Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> > Reviewed-by: and Acked-by: are mutually exclusive. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-26 19:12 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-26 19:12 UTC (permalink / raw) To: Guenter Roeck Cc: balbi, Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog, kernel On Sun, Apr 26, 2015 at 08:29:20AM -0700, Guenter Roeck wrote: > On 04/24/2015 07:10 PM, Felipe Balbi wrote: > >On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: > >>ti,hwmods doesn't belong into the compatible section but is a property > >>on it's own. Also reformat the section of required properties to match the > >>usual style of dt binding documents. > >> > >>Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > >looks great to me, thanks > > > >Reviewed-by: Felipe Balbi <balbi@ti.com> > >Acked-by: Felipe Balbi <balbi@ti.com> > > > > Reviewed-by: and Acked-by: are mutually exclusive. Really? Acked means you like it and Reviewed by means you looked deeper into it. Maybe reviewing without liking isn't likely, *shrug*. The command $ git rev-list linus/master | while read cmmt; do git cat-file commit $cmmt | awk -v cmmt=$cmmt '$1 ~ /(Acked|Reviewed)-by:/ { $1 = ""; print cmmt " " $0 }' | sort; done | uniq -d should catch all commits that have duplicate tags. There are a few, *shrugagain*. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-26 19:12 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-26 19:12 UTC (permalink / raw) To: Guenter Roeck Cc: balbi-l0cyMroinI0, Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ On Sun, Apr 26, 2015 at 08:29:20AM -0700, Guenter Roeck wrote: > On 04/24/2015 07:10 PM, Felipe Balbi wrote: > >On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: > >>ti,hwmods doesn't belong into the compatible section but is a property > >>on it's own. Also reformat the section of required properties to match the > >>usual style of dt binding documents. > >> > >>Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > > > >looks great to me, thanks > > > >Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> > >Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> > > > > Reviewed-by: and Acked-by: are mutually exclusive. Really? Acked means you like it and Reviewed by means you looked deeper into it. Maybe reviewing without liking isn't likely, *shrug*. The command $ git rev-list linus/master | while read cmmt; do git cat-file commit $cmmt | awk -v cmmt=$cmmt '$1 ~ /(Acked|Reviewed)-by:/ { $1 = ""; print cmmt " " $0 }' | sort; done | uniq -d should catch all commits that have duplicate tags. There are a few, *shrugagain*. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation 2015-04-26 19:12 ` Uwe Kleine-König @ 2015-04-26 19:46 ` Guenter Roeck -1 siblings, 0 replies; 27+ messages in thread From: Guenter Roeck @ 2015-04-26 19:46 UTC (permalink / raw) To: Uwe Kleine-König Cc: balbi, Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog, kernel On 04/26/2015 12:12 PM, Uwe Kleine-König wrote: > On Sun, Apr 26, 2015 at 08:29:20AM -0700, Guenter Roeck wrote: >> On 04/24/2015 07:10 PM, Felipe Balbi wrote: >>> On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: >>>> ti,hwmods doesn't belong into the compatible section but is a property >>>> on it's own. Also reformat the section of required properties to match the >>>> usual style of dt binding documents. >>>> >>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >>> >>> looks great to me, thanks >>> >>> Reviewed-by: Felipe Balbi <balbi@ti.com> >>> Acked-by: Felipe Balbi <balbi@ti.com> >>> >> >> Reviewed-by: and Acked-by: are mutually exclusive. > Really? Acked means you like it and Reviewed by means you looked deeper > into it. Maybe reviewing without liking isn't likely, *shrug*. > SubmittingPatches says about Acked-by: " ... to signify and record their approval ... has at least reviewed the patch and has indicated acceptance ...". and about Reviewed-by: " ... indicates that the patch has been reviewed and found acceptable ..." Maybe I should have used the term "redundant". *shrugtoo*. Guenter ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation @ 2015-04-26 19:46 ` Guenter Roeck 0 siblings, 0 replies; 27+ messages in thread From: Guenter Roeck @ 2015-04-26 19:46 UTC (permalink / raw) To: Uwe Kleine-König Cc: balbi, Lokesh Vutla, Wim Van Sebroeck, Linux OMAP Mailing List, linux-watchdog, kernel On 04/26/2015 12:12 PM, Uwe Kleine-König wrote: > On Sun, Apr 26, 2015 at 08:29:20AM -0700, Guenter Roeck wrote: >> On 04/24/2015 07:10 PM, Felipe Balbi wrote: >>> On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: >>>> ti,hwmods doesn't belong into the compatible section but is a property >>>> on it's own. Also reformat the section of required properties to match the >>>> usual style of dt binding documents. >>>> >>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >>> >>> looks great to me, thanks >>> >>> Reviewed-by: Felipe Balbi <balbi@ti.com> >>> Acked-by: Felipe Balbi <balbi@ti.com> >>> >> >> Reviewed-by: and Acked-by: are mutually exclusive. > Really? Acked means you like it and Reviewed by means you looked deeper > into it. Maybe reviewing without liking isn't likely, *shrug*. > SubmittingPatches says about Acked-by: " ... to signify and record their approval ... has at least reviewed the patch and has indicated acceptance ...". and about Reviewed-by: " ... indicates that the patch has been reviewed and found acceptable ..." Maybe I should have used the term "redundant". *shrugtoo*. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data 2015-04-24 9:48 [PATCH 0/3] watchdog: omap: several cleanups Uwe Kleine-König 2015-04-24 9:48 ` [PATCH 1/3] watchdog: omap: use watchdog_init_timeout Uwe Kleine-König @ 2015-04-24 9:48 ` Uwe Kleine-König 2015-04-24 14:44 ` Felipe Balbi 2015-04-24 9:48 ` [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus Uwe Kleine-König 2 siblings, 1 reply; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 9:48 UTC (permalink / raw) To: linux-watchdog, Wim Van Sebroeck; +Cc: Lokesh Vutla, Felipe Balbi, kernel This way only a single allocation is needed (per device). Also this stops making use of watchdog_{set,get}_drvdata. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/watchdog/omap_wdt.c | 55 ++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c index 0eb9ca04e672..479e7c8e44f5 100644 --- a/drivers/watchdog/omap_wdt.c +++ b/drivers/watchdog/omap_wdt.c @@ -53,7 +53,10 @@ static unsigned timer_margin; module_param(timer_margin, uint, 0); MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)"); +#define to_omap_wdt_dev(_wdog) container_of(_wdog, struct omap_wdt_dev, wdog) + struct omap_wdt_dev { + struct watchdog_device wdog; void __iomem *base; /* physical */ struct device *dev; bool omap_wdt_users; @@ -123,7 +126,7 @@ static void omap_wdt_set_timer(struct omap_wdt_dev *wdev, static int omap_wdt_start(struct watchdog_device *wdog) { - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); void __iomem *base = wdev->base; mutex_lock(&wdev->lock); @@ -151,7 +154,7 @@ static int omap_wdt_start(struct watchdog_device *wdog) static int omap_wdt_stop(struct watchdog_device *wdog) { - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); mutex_lock(&wdev->lock); omap_wdt_disable(wdev); @@ -163,7 +166,7 @@ static int omap_wdt_stop(struct watchdog_device *wdog) static int omap_wdt_ping(struct watchdog_device *wdog) { - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); mutex_lock(&wdev->lock); omap_wdt_reload(wdev); @@ -175,7 +178,7 @@ static int omap_wdt_ping(struct watchdog_device *wdog) static int omap_wdt_set_timeout(struct watchdog_device *wdog, unsigned int timeout) { - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); mutex_lock(&wdev->lock); omap_wdt_disable(wdev); @@ -204,16 +207,11 @@ static const struct watchdog_ops omap_wdt_ops = { static int omap_wdt_probe(struct platform_device *pdev) { struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); - struct watchdog_device *omap_wdt; struct resource *res; struct omap_wdt_dev *wdev; u32 rs; int ret; - omap_wdt = devm_kzalloc(&pdev->dev, sizeof(*omap_wdt), GFP_KERNEL); - if (!omap_wdt) - return -ENOMEM; - wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); if (!wdev) return -ENOMEM; @@ -229,18 +227,17 @@ static int omap_wdt_probe(struct platform_device *pdev) if (IS_ERR(wdev->base)) return PTR_ERR(wdev->base); - omap_wdt->info = &omap_wdt_info; - omap_wdt->ops = &omap_wdt_ops; - omap_wdt->min_timeout = TIMER_MARGIN_MIN; - omap_wdt->max_timeout = TIMER_MARGIN_MAX; + wdev->wdog.info = &omap_wdt_info; + wdev->wdog.ops = &omap_wdt_ops; + wdev->wdog.min_timeout = TIMER_MARGIN_MIN; + wdev->wdog.max_timeout = TIMER_MARGIN_MAX; - if (watchdog_init_timeout(omap_wdt, timer_margin, &pdev->dev) < 0) - omap_wdt->timeout = TIMER_MARGIN_DEFAULT; + if (watchdog_init_timeout(&wdev->wdog, timer_margin, &pdev->dev) < 0) + wdev->wdog.timeout = TIMER_MARGIN_DEFAULT; - watchdog_set_drvdata(omap_wdt, wdev); - watchdog_set_nowayout(omap_wdt, nowayout); + watchdog_set_nowayout(&wdev->wdog, nowayout); - platform_set_drvdata(pdev, omap_wdt); + platform_set_drvdata(pdev, wdev); pm_runtime_enable(wdev->dev); pm_runtime_get_sync(wdev->dev); @@ -249,12 +246,12 @@ static int omap_wdt_probe(struct platform_device *pdev) rs = pdata->read_reset_sources(); else rs = 0; - omap_wdt->bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? - WDIOF_CARDRESET : 0; + wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? + WDIOF_CARDRESET : 0; omap_wdt_disable(wdev); - ret = watchdog_register_device(omap_wdt); + ret = watchdog_register_device(&wdev->wdog); if (ret) { pm_runtime_disable(wdev->dev); return ret; @@ -262,7 +259,7 @@ static int omap_wdt_probe(struct platform_device *pdev) pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n", readl_relaxed(wdev->base + OMAP_WATCHDOG_REV) & 0xFF, - omap_wdt->timeout); + wdev->wdog.timeout); pm_runtime_put_sync(wdev->dev); @@ -271,8 +268,7 @@ static int omap_wdt_probe(struct platform_device *pdev) static void omap_wdt_shutdown(struct platform_device *pdev) { - struct watchdog_device *wdog = platform_get_drvdata(pdev); - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); mutex_lock(&wdev->lock); if (wdev->omap_wdt_users) { @@ -284,11 +280,10 @@ static void omap_wdt_shutdown(struct platform_device *pdev) static int omap_wdt_remove(struct platform_device *pdev) { - struct watchdog_device *wdog = platform_get_drvdata(pdev); - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); pm_runtime_disable(wdev->dev); - watchdog_unregister_device(wdog); + watchdog_unregister_device(&wdev->wdog); return 0; } @@ -303,8 +298,7 @@ static int omap_wdt_remove(struct platform_device *pdev) static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state) { - struct watchdog_device *wdog = platform_get_drvdata(pdev); - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); mutex_lock(&wdev->lock); if (wdev->omap_wdt_users) { @@ -318,8 +312,7 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state) static int omap_wdt_resume(struct platform_device *pdev) { - struct watchdog_device *wdog = platform_get_drvdata(pdev); - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); mutex_lock(&wdev->lock); if (wdev->omap_wdt_users) { -- 2.1.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data @ 2015-04-24 14:44 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-24 14:44 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel, Linux OMAP Mailing List [-- Attachment #1: Type: text/plain, Size: 6806 bytes --] On Fri, Apr 24, 2015 at 11:48:32AM +0200, Uwe Kleine-König wrote: > This way only a single allocation is needed (per device). Also this > stops making use of watchdog_{set,get}_drvdata. And this is better because ... ? Nothing against it, just feels like this commit log needs a little more verbosity > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/watchdog/omap_wdt.c | 55 ++++++++++++++++++++------------------------- > 1 file changed, 24 insertions(+), 31 deletions(-) > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 0eb9ca04e672..479e7c8e44f5 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -53,7 +53,10 @@ static unsigned timer_margin; > module_param(timer_margin, uint, 0); > MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)"); > > +#define to_omap_wdt_dev(_wdog) container_of(_wdog, struct omap_wdt_dev, wdog) > + > struct omap_wdt_dev { > + struct watchdog_device wdog; > void __iomem *base; /* physical */ > struct device *dev; > bool omap_wdt_users; > @@ -123,7 +126,7 @@ static void omap_wdt_set_timer(struct omap_wdt_dev *wdev, > > static int omap_wdt_start(struct watchdog_device *wdog) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > void __iomem *base = wdev->base; > > mutex_lock(&wdev->lock); > @@ -151,7 +154,7 @@ static int omap_wdt_start(struct watchdog_device *wdog) > > static int omap_wdt_stop(struct watchdog_device *wdog) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > > mutex_lock(&wdev->lock); > omap_wdt_disable(wdev); > @@ -163,7 +166,7 @@ static int omap_wdt_stop(struct watchdog_device *wdog) > > static int omap_wdt_ping(struct watchdog_device *wdog) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > > mutex_lock(&wdev->lock); > omap_wdt_reload(wdev); > @@ -175,7 +178,7 @@ static int omap_wdt_ping(struct watchdog_device *wdog) > static int omap_wdt_set_timeout(struct watchdog_device *wdog, > unsigned int timeout) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > > mutex_lock(&wdev->lock); > omap_wdt_disable(wdev); > @@ -204,16 +207,11 @@ static const struct watchdog_ops omap_wdt_ops = { > static int omap_wdt_probe(struct platform_device *pdev) > { > struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); > - struct watchdog_device *omap_wdt; > struct resource *res; > struct omap_wdt_dev *wdev; > u32 rs; > int ret; > > - omap_wdt = devm_kzalloc(&pdev->dev, sizeof(*omap_wdt), GFP_KERNEL); > - if (!omap_wdt) > - return -ENOMEM; > - > wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); > if (!wdev) > return -ENOMEM; > @@ -229,18 +227,17 @@ static int omap_wdt_probe(struct platform_device *pdev) > if (IS_ERR(wdev->base)) > return PTR_ERR(wdev->base); > > - omap_wdt->info = &omap_wdt_info; > - omap_wdt->ops = &omap_wdt_ops; > - omap_wdt->min_timeout = TIMER_MARGIN_MIN; > - omap_wdt->max_timeout = TIMER_MARGIN_MAX; > + wdev->wdog.info = &omap_wdt_info; > + wdev->wdog.ops = &omap_wdt_ops; > + wdev->wdog.min_timeout = TIMER_MARGIN_MIN; > + wdev->wdog.max_timeout = TIMER_MARGIN_MAX; > > - if (watchdog_init_timeout(omap_wdt, timer_margin, &pdev->dev) < 0) > - omap_wdt->timeout = TIMER_MARGIN_DEFAULT; > + if (watchdog_init_timeout(&wdev->wdog, timer_margin, &pdev->dev) < 0) > + wdev->wdog.timeout = TIMER_MARGIN_DEFAULT; > > - watchdog_set_drvdata(omap_wdt, wdev); > - watchdog_set_nowayout(omap_wdt, nowayout); > + watchdog_set_nowayout(&wdev->wdog, nowayout); > > - platform_set_drvdata(pdev, omap_wdt); > + platform_set_drvdata(pdev, wdev); > > pm_runtime_enable(wdev->dev); > pm_runtime_get_sync(wdev->dev); > @@ -249,12 +246,12 @@ static int omap_wdt_probe(struct platform_device *pdev) > rs = pdata->read_reset_sources(); > else > rs = 0; > - omap_wdt->bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > - WDIOF_CARDRESET : 0; > + wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > + WDIOF_CARDRESET : 0; > > omap_wdt_disable(wdev); > > - ret = watchdog_register_device(omap_wdt); > + ret = watchdog_register_device(&wdev->wdog); > if (ret) { > pm_runtime_disable(wdev->dev); > return ret; > @@ -262,7 +259,7 @@ static int omap_wdt_probe(struct platform_device *pdev) > > pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n", > readl_relaxed(wdev->base + OMAP_WATCHDOG_REV) & 0xFF, > - omap_wdt->timeout); > + wdev->wdog.timeout); > > pm_runtime_put_sync(wdev->dev); > > @@ -271,8 +268,7 @@ static int omap_wdt_probe(struct platform_device *pdev) > > static void omap_wdt_shutdown(struct platform_device *pdev) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > mutex_lock(&wdev->lock); > if (wdev->omap_wdt_users) { > @@ -284,11 +280,10 @@ static void omap_wdt_shutdown(struct platform_device *pdev) > > static int omap_wdt_remove(struct platform_device *pdev) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > pm_runtime_disable(wdev->dev); > - watchdog_unregister_device(wdog); > + watchdog_unregister_device(&wdev->wdog); > > return 0; > } > @@ -303,8 +298,7 @@ static int omap_wdt_remove(struct platform_device *pdev) > > static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > mutex_lock(&wdev->lock); > if (wdev->omap_wdt_users) { > @@ -318,8 +312,7 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state) > > static int omap_wdt_resume(struct platform_device *pdev) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > mutex_lock(&wdev->lock); > if (wdev->omap_wdt_users) { > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data @ 2015-04-24 14:44 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-24 14:44 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Linux OMAP Mailing List [-- Attachment #1: Type: text/plain, Size: 6831 bytes --] On Fri, Apr 24, 2015 at 11:48:32AM +0200, Uwe Kleine-König wrote: > This way only a single allocation is needed (per device). Also this > stops making use of watchdog_{set,get}_drvdata. And this is better because ... ? Nothing against it, just feels like this commit log needs a little more verbosity > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > --- > drivers/watchdog/omap_wdt.c | 55 ++++++++++++++++++++------------------------- > 1 file changed, 24 insertions(+), 31 deletions(-) > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 0eb9ca04e672..479e7c8e44f5 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -53,7 +53,10 @@ static unsigned timer_margin; > module_param(timer_margin, uint, 0); > MODULE_PARM_DESC(timer_margin, "initial watchdog timeout (in seconds)"); > > +#define to_omap_wdt_dev(_wdog) container_of(_wdog, struct omap_wdt_dev, wdog) > + > struct omap_wdt_dev { > + struct watchdog_device wdog; > void __iomem *base; /* physical */ > struct device *dev; > bool omap_wdt_users; > @@ -123,7 +126,7 @@ static void omap_wdt_set_timer(struct omap_wdt_dev *wdev, > > static int omap_wdt_start(struct watchdog_device *wdog) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > void __iomem *base = wdev->base; > > mutex_lock(&wdev->lock); > @@ -151,7 +154,7 @@ static int omap_wdt_start(struct watchdog_device *wdog) > > static int omap_wdt_stop(struct watchdog_device *wdog) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > > mutex_lock(&wdev->lock); > omap_wdt_disable(wdev); > @@ -163,7 +166,7 @@ static int omap_wdt_stop(struct watchdog_device *wdog) > > static int omap_wdt_ping(struct watchdog_device *wdog) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > > mutex_lock(&wdev->lock); > omap_wdt_reload(wdev); > @@ -175,7 +178,7 @@ static int omap_wdt_ping(struct watchdog_device *wdog) > static int omap_wdt_set_timeout(struct watchdog_device *wdog, > unsigned int timeout) > { > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); > > mutex_lock(&wdev->lock); > omap_wdt_disable(wdev); > @@ -204,16 +207,11 @@ static const struct watchdog_ops omap_wdt_ops = { > static int omap_wdt_probe(struct platform_device *pdev) > { > struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); > - struct watchdog_device *omap_wdt; > struct resource *res; > struct omap_wdt_dev *wdev; > u32 rs; > int ret; > > - omap_wdt = devm_kzalloc(&pdev->dev, sizeof(*omap_wdt), GFP_KERNEL); > - if (!omap_wdt) > - return -ENOMEM; > - > wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); > if (!wdev) > return -ENOMEM; > @@ -229,18 +227,17 @@ static int omap_wdt_probe(struct platform_device *pdev) > if (IS_ERR(wdev->base)) > return PTR_ERR(wdev->base); > > - omap_wdt->info = &omap_wdt_info; > - omap_wdt->ops = &omap_wdt_ops; > - omap_wdt->min_timeout = TIMER_MARGIN_MIN; > - omap_wdt->max_timeout = TIMER_MARGIN_MAX; > + wdev->wdog.info = &omap_wdt_info; > + wdev->wdog.ops = &omap_wdt_ops; > + wdev->wdog.min_timeout = TIMER_MARGIN_MIN; > + wdev->wdog.max_timeout = TIMER_MARGIN_MAX; > > - if (watchdog_init_timeout(omap_wdt, timer_margin, &pdev->dev) < 0) > - omap_wdt->timeout = TIMER_MARGIN_DEFAULT; > + if (watchdog_init_timeout(&wdev->wdog, timer_margin, &pdev->dev) < 0) > + wdev->wdog.timeout = TIMER_MARGIN_DEFAULT; > > - watchdog_set_drvdata(omap_wdt, wdev); > - watchdog_set_nowayout(omap_wdt, nowayout); > + watchdog_set_nowayout(&wdev->wdog, nowayout); > > - platform_set_drvdata(pdev, omap_wdt); > + platform_set_drvdata(pdev, wdev); > > pm_runtime_enable(wdev->dev); > pm_runtime_get_sync(wdev->dev); > @@ -249,12 +246,12 @@ static int omap_wdt_probe(struct platform_device *pdev) > rs = pdata->read_reset_sources(); > else > rs = 0; > - omap_wdt->bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > - WDIOF_CARDRESET : 0; > + wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > + WDIOF_CARDRESET : 0; > > omap_wdt_disable(wdev); > > - ret = watchdog_register_device(omap_wdt); > + ret = watchdog_register_device(&wdev->wdog); > if (ret) { > pm_runtime_disable(wdev->dev); > return ret; > @@ -262,7 +259,7 @@ static int omap_wdt_probe(struct platform_device *pdev) > > pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n", > readl_relaxed(wdev->base + OMAP_WATCHDOG_REV) & 0xFF, > - omap_wdt->timeout); > + wdev->wdog.timeout); > > pm_runtime_put_sync(wdev->dev); > > @@ -271,8 +268,7 @@ static int omap_wdt_probe(struct platform_device *pdev) > > static void omap_wdt_shutdown(struct platform_device *pdev) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > mutex_lock(&wdev->lock); > if (wdev->omap_wdt_users) { > @@ -284,11 +280,10 @@ static void omap_wdt_shutdown(struct platform_device *pdev) > > static int omap_wdt_remove(struct platform_device *pdev) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > pm_runtime_disable(wdev->dev); > - watchdog_unregister_device(wdog); > + watchdog_unregister_device(&wdev->wdog); > > return 0; > } > @@ -303,8 +298,7 @@ static int omap_wdt_remove(struct platform_device *pdev) > > static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > mutex_lock(&wdev->lock); > if (wdev->omap_wdt_users) { > @@ -318,8 +312,7 @@ static int omap_wdt_suspend(struct platform_device *pdev, pm_message_t state) > > static int omap_wdt_resume(struct platform_device *pdev) > { > - struct watchdog_device *wdog = platform_get_drvdata(pdev); > - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); > + struct omap_wdt_dev *wdev = platform_get_drvdata(pdev); > > mutex_lock(&wdev->lock); > if (wdev->omap_wdt_users) { > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data 2015-04-24 14:44 ` Felipe Balbi @ 2015-04-24 19:10 ` Uwe Kleine-König -1 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 19:10 UTC (permalink / raw) To: Felipe Balbi Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, kernel, Linux OMAP Mailing List Hello Felipe, On Fri, Apr 24, 2015 at 09:44:48AM -0500, Felipe Balbi wrote: > On Fri, Apr 24, 2015 at 11:48:32AM +0200, Uwe Kleine-König wrote: > > This way only a single allocation is needed (per device). Also this > > stops making use of watchdog_{set,get}_drvdata. > > And this is better because ... ? a single allocation is better because AFAIK it takes less memory to allocate a + b bytes in a single piece than in two. watchdog_{set,get}_drvdata seems ugly to me. Judging from the other frameworks I know better than wdog I'd say there is no need for these functions. I didn't know if I should be more explicit about my view in the commitlog and if so how. Also I didn't check other wdog drivers, but using this style of embedding the "framework struct" into driver data is a common idiom for uart and gpio drivers for example. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data @ 2015-04-24 19:10 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 19:10 UTC (permalink / raw) To: Felipe Balbi Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, kernel, Linux OMAP Mailing List Hello Felipe, On Fri, Apr 24, 2015 at 09:44:48AM -0500, Felipe Balbi wrote: > On Fri, Apr 24, 2015 at 11:48:32AM +0200, Uwe Kleine-König wrote: > > This way only a single allocation is needed (per device). Also this > > stops making use of watchdog_{set,get}_drvdata. > > And this is better because ... ? a single allocation is better because AFAIK it takes less memory to allocate a + b bytes in a single piece than in two. watchdog_{set,get}_drvdata seems ugly to me. Judging from the other frameworks I know better than wdog I'd say there is no need for these functions. I didn't know if I should be more explicit about my view in the commitlog and if so how. Also I didn't check other wdog drivers, but using this style of embedding the "framework struct" into driver data is a common idiom for uart and gpio drivers for example. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus 2015-04-24 9:48 [PATCH 0/3] watchdog: omap: several cleanups Uwe Kleine-König 2015-04-24 9:48 ` [PATCH 1/3] watchdog: omap: use watchdog_init_timeout Uwe Kleine-König 2015-04-24 9:48 ` [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data Uwe Kleine-König @ 2015-04-24 9:48 ` Uwe Kleine-König 2015-04-24 14:45 ` Felipe Balbi 2015-04-24 19:03 ` Uwe Kleine-König 2 siblings, 2 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 9:48 UTC (permalink / raw) To: linux-watchdog, Wim Van Sebroeck; +Cc: Lokesh Vutla, Felipe Balbi, kernel Instead of using an over-long expression involving the ?: operator use an if and intead of an else branch rely on the fact that the data structure was allocated using devm_kzalloc. This also allows to put the used helper variable into a more local scope. There is no functional change. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/watchdog/omap_wdt.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c index 479e7c8e44f5..0421c06a6cf0 100644 --- a/drivers/watchdog/omap_wdt.c +++ b/drivers/watchdog/omap_wdt.c @@ -209,7 +209,6 @@ static int omap_wdt_probe(struct platform_device *pdev) struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); struct resource *res; struct omap_wdt_dev *wdev; - u32 rs; int ret; wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); @@ -242,12 +241,11 @@ static int omap_wdt_probe(struct platform_device *pdev) pm_runtime_enable(wdev->dev); pm_runtime_get_sync(wdev->dev); - if (pdata && pdata->read_reset_sources) - rs = pdata->read_reset_sources(); - else - rs = 0; - wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? - WDIOF_CARDRESET : 0; + if (pdata && pdata->read_reset_sources) { + u32 rs = pdata->read_reset_sources(); + if (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) + wdev->wdog.bootstatus = WDIOF_CARDRESET; + } omap_wdt_disable(wdev); -- 2.1.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus @ 2015-04-24 14:45 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-24 14:45 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel, Linux OMAP Mailing List [-- Attachment #1: Type: text/plain, Size: 1767 bytes --] On Fri, Apr 24, 2015 at 11:48:33AM +0200, Uwe Kleine-König wrote: > Instead of using an over-long expression involving the ?: operator use > an if and intead of an else branch rely on the fact that the data > structure was allocated using devm_kzalloc. This also allows to put the > used helper variable into a more local scope. > > There is no functional change. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> looks good to me Reviewed-by: Felipe Balbi <balbi@ti.com> > --- > drivers/watchdog/omap_wdt.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 479e7c8e44f5..0421c06a6cf0 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -209,7 +209,6 @@ static int omap_wdt_probe(struct platform_device *pdev) > struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); > struct resource *res; > struct omap_wdt_dev *wdev; > - u32 rs; > int ret; > > wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); > @@ -242,12 +241,11 @@ static int omap_wdt_probe(struct platform_device *pdev) > pm_runtime_enable(wdev->dev); > pm_runtime_get_sync(wdev->dev); > > - if (pdata && pdata->read_reset_sources) > - rs = pdata->read_reset_sources(); > - else > - rs = 0; > - wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > - WDIOF_CARDRESET : 0; > + if (pdata && pdata->read_reset_sources) { > + u32 rs = pdata->read_reset_sources(); > + if (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) > + wdev->wdog.bootstatus = WDIOF_CARDRESET; > + } > > omap_wdt_disable(wdev); > > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus @ 2015-04-24 14:45 ` Felipe Balbi 0 siblings, 0 replies; 27+ messages in thread From: Felipe Balbi @ 2015-04-24 14:45 UTC (permalink / raw) To: Uwe Kleine-König Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Linux OMAP Mailing List [-- Attachment #1: Type: text/plain, Size: 1814 bytes --] On Fri, Apr 24, 2015 at 11:48:33AM +0200, Uwe Kleine-König wrote: > Instead of using an over-long expression involving the ?: operator use > an if and intead of an else branch rely on the fact that the data > structure was allocated using devm_kzalloc. This also allows to put the > used helper variable into a more local scope. > > There is no functional change. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> looks good to me Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> > --- > drivers/watchdog/omap_wdt.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 479e7c8e44f5..0421c06a6cf0 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -209,7 +209,6 @@ static int omap_wdt_probe(struct platform_device *pdev) > struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); > struct resource *res; > struct omap_wdt_dev *wdev; > - u32 rs; > int ret; > > wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); > @@ -242,12 +241,11 @@ static int omap_wdt_probe(struct platform_device *pdev) > pm_runtime_enable(wdev->dev); > pm_runtime_get_sync(wdev->dev); > > - if (pdata && pdata->read_reset_sources) > - rs = pdata->read_reset_sources(); > - else > - rs = 0; > - wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > - WDIOF_CARDRESET : 0; > + if (pdata && pdata->read_reset_sources) { > + u32 rs = pdata->read_reset_sources(); > + if (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) > + wdev->wdog.bootstatus = WDIOF_CARDRESET; > + } > > omap_wdt_disable(wdev); > > -- > 2.1.4 > -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus 2015-04-24 9:48 ` [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus Uwe Kleine-König 2015-04-24 14:45 ` Felipe Balbi @ 2015-04-24 19:03 ` Uwe Kleine-König 2015-04-26 15:28 ` Guenter Roeck 1 sibling, 1 reply; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-24 19:03 UTC (permalink / raw) To: linux-watchdog, Wim Van Sebroeck; +Cc: Lokesh Vutla, Felipe Balbi, kernel On Fri, Apr 24, 2015 at 11:48:33AM +0200, Uwe Kleine-König wrote: > Instead of using an over-long expression involving the ?: operator use > an if and intead of an else branch rely on the fact that the data just noticed ^^ there is an s missing ... Wim, if you apply this version, can you fix that up? Best regards Uwe > structure was allocated using devm_kzalloc. This also allows to put the > used helper variable into a more local scope. > > There is no functional change. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/watchdog/omap_wdt.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index 479e7c8e44f5..0421c06a6cf0 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -209,7 +209,6 @@ static int omap_wdt_probe(struct platform_device *pdev) > struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev); > struct resource *res; > struct omap_wdt_dev *wdev; > - u32 rs; > int ret; > > wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL); > @@ -242,12 +241,11 @@ static int omap_wdt_probe(struct platform_device *pdev) > pm_runtime_enable(wdev->dev); > pm_runtime_get_sync(wdev->dev); > > - if (pdata && pdata->read_reset_sources) > - rs = pdata->read_reset_sources(); > - else > - rs = 0; > - wdev->wdog.bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? > - WDIOF_CARDRESET : 0; > + if (pdata && pdata->read_reset_sources) { > + u32 rs = pdata->read_reset_sources(); > + if (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) > + wdev->wdog.bootstatus = WDIOF_CARDRESET; > + } > > omap_wdt_disable(wdev); > > -- > 2.1.4 > > -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus 2015-04-24 19:03 ` Uwe Kleine-König @ 2015-04-26 15:28 ` Guenter Roeck 2015-04-26 19:14 ` Uwe Kleine-König 0 siblings, 1 reply; 27+ messages in thread From: Guenter Roeck @ 2015-04-26 15:28 UTC (permalink / raw) To: Uwe Kleine-König, linux-watchdog, Wim Van Sebroeck Cc: Lokesh Vutla, Felipe Balbi, kernel On 04/24/2015 12:03 PM, Uwe Kleine-König wrote: > On Fri, Apr 24, 2015 at 11:48:33AM +0200, Uwe Kleine-König wrote: >> Instead of using an over-long expression involving the ?: operator use >> an if and intead of an else branch rely on the fact that the data > just noticed ^^ there is an s missing ... > > Wim, if you apply this version, can you fix that up? > Hi Uwe, if you don't mind, can you sesend the entire series ? I don't think patch 1 applies after 1a, and it might make sense to add some more explanation to patch 2. FWIW, I would say that using to_omap_wdt_dev() instead of watchdog_get_drvdata() simplifies the code, or simply that the functions are no longer needed in the driver (ugliness may be seen as personal opinion). When you do so, please feel free to add Reviewed-by: Guenter Roeck <linux@roeck-us.net> to all four patches. Thanks, Guenter ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus 2015-04-26 15:28 ` Guenter Roeck @ 2015-04-26 19:14 ` Uwe Kleine-König 0 siblings, 0 replies; 27+ messages in thread From: Uwe Kleine-König @ 2015-04-26 19:14 UTC (permalink / raw) To: Guenter Roeck Cc: linux-watchdog, Wim Van Sebroeck, Lokesh Vutla, Felipe Balbi, kernel Hello Guenter, On Sun, Apr 26, 2015 at 08:28:16AM -0700, Guenter Roeck wrote: > if you don't mind, can you sesend the entire series ? yeah, that's my plan. I already fixed it for Felipe's comments and intend to resend tomorrow. > I don't think patch 1 applies after 1a, and it might make sense > to add some more explanation to patch 2. FWIW, I would say that using > to_omap_wdt_dev() instead of watchdog_get_drvdata() simplifies the code, > or simply that the functions are no longer needed in the driver (ugliness > may be seen as personal opinion). > > When you do so, please feel free to add > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> to all four patches. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2015-04-26 19:46 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-24 9:48 [PATCH 0/3] watchdog: omap: several cleanups Uwe Kleine-König 2015-04-24 9:48 ` [PATCH 1/3] watchdog: omap: use watchdog_init_timeout Uwe Kleine-König 2015-04-24 14:42 ` Felipe Balbi 2015-04-24 14:42 ` Felipe Balbi 2015-04-24 19:02 ` Uwe Kleine-König 2015-04-24 19:02 ` Uwe Kleine-König 2015-04-24 20:20 ` [PATCH 1a/3] watchdog: omap: clearify device tree documentation Uwe Kleine-König 2015-04-24 20:20 ` Uwe Kleine-König 2015-04-25 2:10 ` Felipe Balbi 2015-04-25 2:10 ` Felipe Balbi 2015-04-26 15:29 ` Guenter Roeck 2015-04-26 15:29 ` Guenter Roeck 2015-04-26 19:12 ` Uwe Kleine-König 2015-04-26 19:12 ` Uwe Kleine-König 2015-04-26 19:46 ` Guenter Roeck 2015-04-26 19:46 ` Guenter Roeck 2015-04-24 9:48 ` [PATCH 2/3] watchdog: omap: put struct watchdog_device into driver data Uwe Kleine-König 2015-04-24 14:44 ` Felipe Balbi 2015-04-24 14:44 ` Felipe Balbi 2015-04-24 19:10 ` Uwe Kleine-König 2015-04-24 19:10 ` Uwe Kleine-König 2015-04-24 9:48 ` [PATCH 3/3] watchdog: omap: simplify assignment of bootstatus Uwe Kleine-König 2015-04-24 14:45 ` Felipe Balbi 2015-04-24 14:45 ` Felipe Balbi 2015-04-24 19:03 ` Uwe Kleine-König 2015-04-26 15:28 ` Guenter Roeck 2015-04-26 19:14 ` Uwe Kleine-König
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.