[RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[dinguyen-EIB2kfCEclfQT0dZR+AlfA]

From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>

Add device tree support to the DW watchdog timer.

Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
v2:
- Use of_match_ptr() for of_match_table
---
 .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
 drivers/watchdog/dw_wdt.c                          |    8 ++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
new file mode 100644
index 0000000..29e150b
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
@@ -0,0 +1,16 @@
+Synopsys Designware Watchdog Timer
+
+Required Properties:
+
+- Compatiblity	: "snps,dw-wdt"
+- reg		: Base address of the watchdog timer register.
+
+Example:
+
+	watchdog0: wd@ffd02000 {
+		compatible = "snps,dw-wdt";
+		reg = <0xffd02000 0x1000>;
+		interrupts = <0 171 4>;
+		clocks = <&per_base_clk>;
+		status = "okay";
+	};
diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 2037669..a720f9b 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -29,6 +29,7 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/pm.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
@@ -343,12 +344,19 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id dw_wdt_of_match[] = {
+	{ .compatible = "snps,dw-wdt", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dw_wdt_of_match);
+
 static struct platform_driver dw_wdt_driver = {
 	.probe		= dw_wdt_drv_probe,
 	.remove		= dw_wdt_drv_remove,
 	.driver		= {
 		.name	= "dw_wdt",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(dw_wdt_of_match),
 #ifdef CONFIG_PM
 		.pm	= &dw_wdt_pm_ops,
 #endif /* CONFIG_PM */
-- 
1.7.9.5


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Guenter Roeck]

On Wed, Oct 02, 2013 at 01:44:32PM -0500, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> 
> Add device tree support to the DW watchdog timer.
> 
> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> v2:
> - Use of_match_ptr() for of_match_table
> ---
>  .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
>  drivers/watchdog/dw_wdt.c                          |    8 ++++++++
>  2 files changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> new file mode 100644
> index 0000000..29e150b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> @@ -0,0 +1,16 @@
> +Synopsys Designware Watchdog Timer
> +
> +Required Properties:
> +
> +- Compatiblity	: "snps,dw-wdt"
> +- reg		: Base address of the watchdog timer register.
> +
> +Example:
> +
> +	watchdog0: wd@ffd02000 {
> +		compatible = "snps,dw-wdt";
> +		reg = <0xffd02000 0x1000>;
> +		interrupts = <0 171 4>;
> +		clocks = <&per_base_clk>;
> +		status = "okay";
> +	};
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 2037669..a720f9b 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -29,6 +29,7 @@
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/of.h>
>  #include <linux/pm.h>
>  #include <linux/platform_device.h>
>  #include <linux/spinlock.h>
> @@ -343,12 +344,19 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id dw_wdt_of_match[] = {
> +	{ .compatible = "snps,dw-wdt", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, dw_wdt_of_match);
> +
>  static struct platform_driver dw_wdt_driver = {
>  	.probe		= dw_wdt_drv_probe,
>  	.remove		= dw_wdt_drv_remove,
>  	.driver		= {
>  		.name	= "dw_wdt",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(dw_wdt_of_match),

There is a separate set of patches from Sachin, removing unnecessary
uses of of_match_ptr() from various watchdog drivers. Sachin's argument
is that it does not make sense to use of_match_ptr if the function
it points to (dw_wdt_of_match in this case) is always compiled
unconditionally anyway.

Your comments suggest that you actually _added_ of_match_ptr() in v2 of this
patch, even though dw_wdt_of_match always exists and of_match_ptr is thus
technically unnecessary.

Not that I really care one way or another, but can we get an authoritative
answer what subsystem maintainers are supposed to accept ?

Thanks,
Guenter

>  #ifdef CONFIG_PM
>  		.pm	= &dw_wdt_pm_ops,
>  #endif /* CONFIG_PM */
> -- 
> 1.7.9.5
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Mark Rutland]

On Wed, Oct 02, 2013 at 07:44:32PM +0100, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> 
> Add device tree support to the DW watchdog timer.
> 
> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> v2:
> - Use of_match_ptr() for of_match_table
> ---
>  .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
>  drivers/watchdog/dw_wdt.c                          |    8 ++++++++
>  2 files changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> new file mode 100644
> index 0000000..29e150b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> @@ -0,0 +1,16 @@
> +Synopsys Designware Watchdog Timer
> +
> +Required Properties:
> +
> +- Compatiblity	: "snps,dw-wdt"

This should presumably be:

- compatbile: should contain "snps,dw-wdt"

> +- reg		: Base address of the watchdog timer register.

And the size...

> +
> +Example:
> +
> +	watchdog0: wd@ffd02000 {
> +		compatible = "snps,dw-wdt";
> +		reg = <0xffd02000 0x1000>;
> +		interrupts = <0 171 4>;

This wasn't mentioned.

Is it necessary?

Is it the only interrupt?

> +		clocks = <&per_base_clk>;

Similarly, is this the only clock?

Is it necessary?

> +		status = "okay";

This is unnecessary.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Guenter Roeck]

On 10/21/2013 04:05 AM, Mark Rutland wrote:
> On Wed, Oct 02, 2013 at 07:44:32PM +0100, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
>> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>>
>> Add device tree support to the DW watchdog timer.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>> Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>> Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
>> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
>> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>> Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
>> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
>> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>> Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> ---
>> v2:
>> - Use of_match_ptr() for of_match_table
>> ---
>>   .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
>>   drivers/watchdog/dw_wdt.c                          |    8 ++++++++
>>   2 files changed, 24 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>> new file mode 100644
>> index 0000000..29e150b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>> @@ -0,0 +1,16 @@
>> +Synopsys Designware Watchdog Timer
>> +
>> +Required Properties:
>> +
>> +- Compatiblity	: "snps,dw-wdt"
>
> This should presumably be:
>
> - compatbile: should contain "snps,dw-wdt"
>

Hi Mark,

s/compatbile/compatible/ :-)

"must be" or "should contain" ? I see both in various bindings.
Is there a preference ?

>> +- reg		: Base address of the watchdog timer register.
>
> And the size...
>
>> +
>> +Example:
>> +
>> +	watchdog0: wd@ffd02000 {
>> +		compatible = "snps,dw-wdt";
>> +		reg = <0xffd02000 0x1000>;
>> +		interrupts = <0 171 4>;
>
> This wasn't mentioned.
>
> Is it necessary?
>

 From looking into the code ...

The driver doesn't use interrupts, so I guess the answer is no. Cut-and-paste error, maybe ?

> Is it the only interrupt?
>
>> +		clocks = <&per_base_clk>;
>
> Similarly, is this the only clock?
>
The driver uses one clock, and it is mandatory.

Guenter

> Is it necessary?
>
>> +		status = "okay";
>
> This is unnecessary.
>
> Thanks,
> Mark.
>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Mark Rutland]

On Tue, Oct 22, 2013 at 01:41:20AM +0100, Guenter Roeck wrote:
> On 10/21/2013 04:05 AM, Mark Rutland wrote:
> > On Wed, Oct 02, 2013 at 07:44:32PM +0100, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> >> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> >>
> >> Add device tree support to the DW watchdog timer.
> >>
> >> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> >> Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> >> Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> >> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> >> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> >> Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
> >> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> >> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> >> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> >> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> >> Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> >> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> ---
> >> v2:
> >> - Use of_match_ptr() for of_match_table
> >> ---
> >>   .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
> >>   drivers/watchdog/dw_wdt.c                          |    8 ++++++++
> >>   2 files changed, 24 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> >> new file mode 100644
> >> index 0000000..29e150b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> >> @@ -0,0 +1,16 @@
> >> +Synopsys Designware Watchdog Timer
> >> +
> >> +Required Properties:
> >> +
> >> +- Compatiblity	: "snps,dw-wdt"
> >
> > This should presumably be:
> >
> > - compatbile: should contain "snps,dw-wdt"
> >
> 
> Hi Mark,
> 
> s/compatbile/compatible/ :-)

Indeed :)

> 
> "must be" or "should contain" ? I see both in various bindings.
> Is there a preference ?

I would prefer "should contain" -- it doesn't preclude future compatible
variants.

> 
> >> +- reg		: Base address of the watchdog timer register.
> >
> > And the size...
> >
> >> +
> >> +Example:
> >> +
> >> +	watchdog0: wd@ffd02000 {
> >> +		compatible = "snps,dw-wdt";
> >> +		reg = <0xffd02000 0x1000>;
> >> +		interrupts = <0 171 4>;
> >
> > This wasn't mentioned.
> >
> > Is it necessary?
> >
> 
>  From looking into the code ...
> 
> The driver doesn't use interrupts, so I guess the answer is no. Cut-and-paste error, maybe ?

Does the device have any interrupts, even if they're unused?

> 
> > Is it the only interrupt?
> >
> >> +		clocks = <&per_base_clk>;
> >
> > Similarly, is this the only clock?
> >
> The driver uses one clock, and it is mandatory.

Is this the only clock into the unit?

Does it have a name?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Dinh Nguyen]

Hi Mark,

On 10/22/13 5:39 AM, Mark Rutland wrote:
> On Tue, Oct 22, 2013 at 01:41:20AM +0100, Guenter Roeck wrote:
>> On 10/21/2013 04:05 AM, Mark Rutland wrote:
>>> On Wed, Oct 02, 2013 at 07:44:32PM +0100, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
>>>> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> Add device tree support to the DW watchdog timer.
>>>>
>>>> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
>>>> Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>>>> Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
>>>> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
>>>> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>>>> Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
>>>> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
>>>> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
>>>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>>>> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>>>> Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> ---
>>>> v2:
>>>> - Use of_match_ptr() for of_match_table
>>>> ---
>>>>    .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
>>>>    drivers/watchdog/dw_wdt.c                          |    8 ++++++++
>>>>    2 files changed, 24 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>>>> new file mode 100644
>>>> index 0000000..29e150b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>>>> @@ -0,0 +1,16 @@
>>>> +Synopsys Designware Watchdog Timer
>>>> +
>>>> +Required Properties:
>>>> +
>>>> +- Compatiblity	: "snps,dw-wdt"
>>> This should presumably be:
>>>
>>> - compatbile: should contain "snps,dw-wdt"
>>>
>> Hi Mark,
>>
>> s/compatbile/compatible/ :-)
> Indeed :)
Changed for v3.
>
>> "must be" or "should contain" ? I see both in various bindings.
>> Is there a preference ?
> I would prefer "should contain" -- it doesn't preclude future compatible
> variants.
Oops..v3 has "should be"...will change to "should contain" in v4.
>
>>>> +- reg		: Base address of the watchdog timer register.
>>> And the size...
>>>
>>>> +
>>>> +Example:
>>>> +
>>>> +	watchdog0: wd@ffd02000 {
>>>> +		compatible = "snps,dw-wdt";
>>>> +		reg = <0xffd02000 0x1000>;
>>>> +		interrupts = <0 171 4>;
>>> This wasn't mentioned.
>>>
>>> Is it necessary?
>>>
>>   From looking into the code ...
>>
>> The driver doesn't use interrupts, so I guess the answer is no. Cut-and-paste error, maybe ?
> Does the device have any interrupts, even if they're unused?
Yes, the device does have an interrupt. Should I add it as an optional 
property?
>
>>> Is it the only interrupt?
>>>
>>>> +		clocks = <&per_base_clk>;
>>> Similarly, is this the only clock?
>>>
>> The driver uses one clock, and it is mandatory.
> Is this the only clock into the unit?
>
> Does it have a name?
Yes, this is is the only clock into the unit. No, it does not have name.

Thanks,
Dinh
>
> Thanks,
> Mark.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Mark Rutland]

On Tue, Oct 22, 2013 at 11:57:34AM +0100, Dinh Nguyen wrote:
> Hi Mark,
> 
> On 10/22/13 5:39 AM, Mark Rutland wrote:
> > On Tue, Oct 22, 2013 at 01:41:20AM +0100, Guenter Roeck wrote:
> >> On 10/21/2013 04:05 AM, Mark Rutland wrote:
> >>> On Wed, Oct 02, 2013 at 07:44:32PM +0100, dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org wrote:
> >>>> From: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> >>>>
> >>>> Add device tree support to the DW watchdog timer.
> >>>>
> >>>> Signed-off-by: Dinh Nguyen <dinguyen-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
> >>>> Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> >>>> Reviewed-by: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> >>>> Cc: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> >>>> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> >>>> Cc: Viresh Kumar <viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>> Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
> >>>> Cc: Pavel Machek <pavel-ynQEQJNshbs@public.gmane.org>
> >>>> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> >>>> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> >>>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> >>>> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> >>>> Cc: Ian Campbell <ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> >>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>>> Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>>> ---
> >>>> v2:
> >>>> - Use of_match_ptr() for of_match_table
> >>>> ---
> >>>>    .../devicetree/bindings/watchdog/dw_wdt.txt        |   16 ++++++++++++++++
> >>>>    drivers/watchdog/dw_wdt.c                          |    8 ++++++++
> >>>>    2 files changed, 24 insertions(+)
> >>>>    create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> >>>> new file mode 100644
> >>>> index 0000000..29e150b
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> >>>> @@ -0,0 +1,16 @@
> >>>> +Synopsys Designware Watchdog Timer
> >>>> +
> >>>> +Required Properties:
> >>>> +
> >>>> +- Compatiblity	: "snps,dw-wdt"
> >>> This should presumably be:
> >>>
> >>> - compatbile: should contain "snps,dw-wdt"
> >>>
> >> Hi Mark,
> >>
> >> s/compatbile/compatible/ :-)
> > Indeed :)
> Changed for v3.
> >
> >> "must be" or "should contain" ? I see both in various bindings.
> >> Is there a preference ?
> > I would prefer "should contain" -- it doesn't preclude future compatible
> > variants.
> Oops..v3 has "should be"...will change to "should contain" in v4.

Cheers.

> >
> >>>> +- reg		: Base address of the watchdog timer register.
> >>> And the size...
> >>>
> >>>> +
> >>>> +Example:
> >>>> +
> >>>> +	watchdog0: wd@ffd02000 {
> >>>> +		compatible = "snps,dw-wdt";
> >>>> +		reg = <0xffd02000 0x1000>;
> >>>> +		interrupts = <0 171 4>;
> >>> This wasn't mentioned.
> >>>
> >>> Is it necessary?
> >>>
> >>   From looking into the code ...
> >>
> >> The driver doesn't use interrupts, so I guess the answer is no. Cut-and-paste error, maybe ?
> > Does the device have any interrupts, even if they're unused?
> Yes, the device does have an interrupt. Should I add it as an optional 
> property?

Yes please, given it's easy to add. We might want to use it in future so it
would be nice to have now.

> >
> >>> Is it the only interrupt?
> >>>
> >>>> +		clocks = <&per_base_clk>;
> >>> Similarly, is this the only clock?
> >>>
> >> The driver uses one clock, and it is mandatory.
> > Is this the only clock into the unit?
> >
> > Does it have a name?
> Yes, this is is the only clock into the unit. No, it does not have name.

Ok.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Jamie Iles]

On Tue, Oct 22, 2013 at 05:57:34AM -0500, Dinh Nguyen wrote:
> On 10/22/13 5:39 AM, Mark Rutland wrote:
> >Does the device have any interrupts, even if they're unused?
> Yes, the device does have an interrupt. Should I add it as an
> optional property?

iirc the interrupt feature is part of the IP configuration so not all 
instantiations will have the interrupt, so this should be an optional 
property.

Jamie
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html