[PATCH 1/3] watchdog: xilinx: Fix driver header

[PATCH 1/3] watchdog: xilinx: Fix driver header

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]

- Remove reference for IP version
- Fix header coding style
- Remove notes which are visible from the code
- Fix driver license according to header

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 2761ddb..d4a35ab 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -1,23 +1,13 @@
 /*
-*   of_xilinx_wdt.c  1.01  A Watchdog Device Driver for Xilinx xps_timebase_wdt
-*
-*   (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
-*
-*       -----------------------
-*
-*   This program is free software; you can redistribute it and/or
-*   modify it under the terms of the GNU General Public License
-*   as published by the Free Software Foundation; either version
-*   2 of the License, or (at your option) any later version.
-*
-*       -----------------------
-*	30-May-2011 Alejandro Cabrera <aldaya@gmail.com>
-*		- If "xlnx,wdt-enable-once" wasn't found on device tree the
-*		  module will use CONFIG_WATCHDOG_NOWAYOUT
-*		- If the device tree parameters ("clock-frequency" and
-*		  "xlnx,wdt-interval") wasn't found the driver won't
-*		  know the wdt reset interval
-*/
+ * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
+ *
+ * (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

@@ -413,5 +403,5 @@ module_platform_driver(xwdt_driver);

 MODULE_AUTHOR("Alejandro Cabrera <aldaya@gmail.com>");
 MODULE_DESCRIPTION("Xilinx Watchdog driver");
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
 MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 2/3] watchdog: xilinx: Setup the origin compatible string

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

Watchdog 1.01.a is also compatible with 1.00.a.
Setup the origin version to compatible list.
If you want to use newer watchdog version, please
extend your compatible list.

For example:
compatible = "xlnx,xps-timebase-wdt-1.02.a", "xlnx,xps-timebase-wdt-1.00.a";

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index d4a35ab..79f358c 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -384,7 +384,7 @@ static int xwdt_remove(struct platform_device *dev)

 /* Match table for of_platform binding */
 static struct of_device_id xwdt_of_match[] = {
-	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
+	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, xwdt_of_match);
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

Standard watchdog programs try to setup timeout
via ioctl and this functionality should be implemented.
Timeout value is hardcoded in the hardware but
based on Documentation/watchdog/watchdog-api.txt
can return the real timeout used in the same variable.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 79f358c..a3bbe72 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		xwdt_keepalive();
 		return 0;

+	case WDIOC_SETTIMEOUT:
 	case WDIOC_GETTIMEOUT:
 		if (no_timeout)
 			return -ENOTTY;
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

RE: [PATCH 1/3] watchdog: xilinx: Fix driver header

[Venu Byravarasu]

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Michal Simek
> Sent: Thursday, May 30, 2013 5:56 PM
> To: linux-kernel@vger.kernel.org
> Cc: Michal Simek; Michal Simek; Wim Van Sebroeck; linux-
> watchdog@vger.kernel.org
> Subject: [PATCH 1/3] watchdog: xilinx: Fix driver header
> 
> * PGP Signed by an unknown key
> 
> - Remove reference for IP version
> - Fix header coding style
> - Remove notes which are visible from the code
> - Fix driver license according to header
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  drivers/watchdog/of_xilinx_wdt.c | 30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c
> b/drivers/watchdog/of_xilinx_wdt.c
> index 2761ddb..d4a35ab 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -1,23 +1,13 @@
>  /*
> -*   of_xilinx_wdt.c  1.01  A Watchdog Device Driver for Xilinx
> xps_timebase_wdt
> -*
> -*   (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
> -*
> -*       -----------------------
> -*/
> + * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
> + *
> + * (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)

Should year not be updated? 

> + *
> + * This program is free software; you can redistribute it and/or

Re: [PATCH 1/3] watchdog: xilinx: Fix driver header

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

On 05/30/2013 02:30 PM, Venu Byravarasu wrote:
>> -----Original Message-----
>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
>> owner@vger.kernel.org] On Behalf Of Michal Simek
>> Sent: Thursday, May 30, 2013 5:56 PM
>> To: linux-kernel@vger.kernel.org
>> Cc: Michal Simek; Michal Simek; Wim Van Sebroeck; linux-
>> watchdog@vger.kernel.org
>> Subject: [PATCH 1/3] watchdog: xilinx: Fix driver header
>>
>> * PGP Signed by an unknown key
>>
>> - Remove reference for IP version
>> - Fix header coding style
>> - Remove notes which are visible from the code
>> - Fix driver license according to header
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>  drivers/watchdog/of_xilinx_wdt.c | 30 ++++++++++--------------------
>>  1 file changed, 10 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/watchdog/of_xilinx_wdt.c
>> b/drivers/watchdog/of_xilinx_wdt.c
>> index 2761ddb..d4a35ab 100644
>> --- a/drivers/watchdog/of_xilinx_wdt.c
>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>> @@ -1,23 +1,13 @@
>>  /*
>> -*   of_xilinx_wdt.c  1.01  A Watchdog Device Driver for Xilinx
>> xps_timebase_wdt
>> -*
>> -*   (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
>> -*
>> -*       -----------------------
>> -*/
>> + * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
>> + *
>> + * (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
> 
> Should year not be updated? 

This is just header fixup.
I think he is not working/maintaining this driver for a long time.
I would write there Xilinx copyright but I really don't care.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 2/3] watchdog: xilinx: Setup the origin compatible string

[Guenter Roeck]

On Thu, May 30, 2013 at 02:26:03PM +0200, Michal Simek wrote:
> Watchdog 1.01.a is also compatible with 1.00.a.
> Setup the origin version to compatible list.
> If you want to use newer watchdog version, please
> extend your compatible list.
> 
> For example:
> compatible = "xlnx,xps-timebase-wdt-1.02.a", "xlnx,xps-timebase-wdt-1.00.a";
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  drivers/watchdog/of_xilinx_wdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index d4a35ab..79f358c 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -384,7 +384,7 @@ static int xwdt_remove(struct platform_device *dev)
> 
>  /* Match table for of_platform binding */
>  static struct of_device_id xwdt_of_match[] = {
> -	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
> +	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a", },

Is this really a good idea ? It means every existing device tree binding which
specifies 1.01a will now fail. If the code is compatible to 1.00a, I think it
would make more sense to add that to the driver as additional entry instead of
deleting the existing entry for 1.01a.

Thanks,
Guenter

Re: [PATCH 2/3] watchdog: xilinx: Setup the origin compatible string

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]

On 05/30/2013 04:00 PM, Guenter Roeck wrote:
> On Thu, May 30, 2013 at 02:26:03PM +0200, Michal Simek wrote:
>> Watchdog 1.01.a is also compatible with 1.00.a.
>> Setup the origin version to compatible list.
>> If you want to use newer watchdog version, please
>> extend your compatible list.
>>
>> For example:
>> compatible = "xlnx,xps-timebase-wdt-1.02.a", "xlnx,xps-timebase-wdt-1.00.a";
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>  drivers/watchdog/of_xilinx_wdt.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
>> index d4a35ab..79f358c 100644
>> --- a/drivers/watchdog/of_xilinx_wdt.c
>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>> @@ -384,7 +384,7 @@ static int xwdt_remove(struct platform_device *dev)
>>
>>  /* Match table for of_platform binding */
>>  static struct of_device_id xwdt_of_match[] = {
>> -	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
>> +	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a", },
> 
> Is this really a good idea ? It means every existing device tree binding which
> specifies 1.01a will now fail. If the code is compatible to 1.00a, I think it
> would make more sense to add that to the driver as additional entry instead of
> deleting the existing entry for 1.01a.

The most of users/I believe all of them are using device-tree generator
which generate DTS directly from Xilinx design tools because it is almost
impossible to write DTS for any xilinx fpga plaform and
1.00.a is setup as backward compatible property.
But if you think that it is worth to keep there 1.01.a I have no problem
with that I will keep there 1.01.a and add 1.00.a.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Guenter Roeck]

On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
> Standard watchdog programs try to setup timeout
> via ioctl and this functionality should be implemented.
> Timeout value is hardcoded in the hardware but
> based on Documentation/watchdog/watchdog-api.txt
> can return the real timeout used in the same variable.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  drivers/watchdog/of_xilinx_wdt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index 79f358c..a3bbe72 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		xwdt_keepalive();
>  		return 0;
> 
> +	case WDIOC_SETTIMEOUT:
>  	case WDIOC_GETTIMEOUT:
>  		if (no_timeout)
>  			return -ENOTTY;

Watchdog programs should check ident.options before trying to set the timeout.
If they don't, there is an application bug. I don't think it is a good idea
to start hacking the kernel to work around application bugs.

Guenter

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2537 bytes --]

On 05/30/2013 04:07 PM, Guenter Roeck wrote:
> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
>> Standard watchdog programs try to setup timeout
>> via ioctl and this functionality should be implemented.
>> Timeout value is hardcoded in the hardware but
>> based on Documentation/watchdog/watchdog-api.txt
>> can return the real timeout used in the same variable.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
>> index 79f358c..a3bbe72 100644
>> --- a/drivers/watchdog/of_xilinx_wdt.c
>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>  		xwdt_keepalive();
>>  		return 0;
>>
>> +	case WDIOC_SETTIMEOUT:
>>  	case WDIOC_GETTIMEOUT:
>>  		if (no_timeout)
>>  			return -ENOTTY;
> 
> Watchdog programs should check ident.options before trying to set the timeout.
> If they don't, there is an application bug. I don't think it is a good idea
> to start hacking the kernel to work around application bugs.

Based on Documentation/watchdog/watchdog-api.txt

"For some drivers it is possible to modify the watchdog timeout on the
fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
flag set in their option field.  The argument is an integer
representing the timeout in seconds.  The driver returns the real
timeout used in the same variable, and this timeout might differ from
the requested one due to limitation of the hardware.

    int timeout = 45;
    ioctl(fd, WDIOC_SETTIMEOUT, &timeout);
    printf("The timeout was set to %d seconds\n", timeout);

This example might actually print "The timeout was set to 60 seconds"
if the device has a granularity of minutes for its timeout."

should be completely fine that user application is trying to setup timeout
and driver should return value based on it.

And yes, user application should check return value from ioctl call
but still based on documentation driver can properly support it too.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Guenter Roeck]

On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
> > On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
> >> Standard watchdog programs try to setup timeout
> >> via ioctl and this functionality should be implemented.
> >> Timeout value is hardcoded in the hardware but
> >> based on Documentation/watchdog/watchdog-api.txt
> >> can return the real timeout used in the same variable.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>  drivers/watchdog/of_xilinx_wdt.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> >> index 79f358c..a3bbe72 100644
> >> --- a/drivers/watchdog/of_xilinx_wdt.c
> >> +++ b/drivers/watchdog/of_xilinx_wdt.c
> >> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >>  		xwdt_keepalive();
> >>  		return 0;
> >>
> >> +	case WDIOC_SETTIMEOUT:
> >>  	case WDIOC_GETTIMEOUT:
> >>  		if (no_timeout)
> >>  			return -ENOTTY;
> > 
> > Watchdog programs should check ident.options before trying to set the timeout.
> > If they don't, there is an application bug. I don't think it is a good idea
> > to start hacking the kernel to work around application bugs.
> 
> Based on Documentation/watchdog/watchdog-api.txt
> 
> "For some drivers it is possible to modify the watchdog timeout on the
> fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
> flag set in their option field.  The argument is an integer

Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.

Guenter

> representing the timeout in seconds.  The driver returns the real
> timeout used in the same variable, and this timeout might differ from
> the requested one due to limitation of the hardware.
> 
>     int timeout = 45;
>     ioctl(fd, WDIOC_SETTIMEOUT, &timeout);
>     printf("The timeout was set to %d seconds\n", timeout);
> 
> This example might actually print "The timeout was set to 60 seconds"
> if the device has a granularity of minutes for its timeout."
> 
> should be completely fine that user application is trying to setup timeout
> and driver should return value based on it.
> 
> And yes, user application should check return value from ioctl call
> but still based on documentation driver can properly support it too.
> 
> Thanks,
> Michal
> 
> -- 
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
> Maintainer of Linux kernel - Xilinx Zynq ARM architecture
> Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
> 
> 

Re: [PATCH 1/3] watchdog: xilinx: Fix driver header

[Alejandro Cabrera]

On 30/5/2013 7:32 AM, Michal Simek wrote:
> On 05/30/2013 02:30 PM, Venu Byravarasu wrote:
>>> -----Original Message-----
>>> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
>>> owner@vger.kernel.org] On Behalf Of Michal Simek
>>> Sent: Thursday, May 30, 2013 5:56 PM
>>> To: linux-kernel@vger.kernel.org
>>> Cc: Michal Simek; Michal Simek; Wim Van Sebroeck; linux-
>>> watchdog@vger.kernel.org
>>> Subject: [PATCH 1/3] watchdog: xilinx: Fix driver header
>>>
>>> * PGP Signed by an unknown key
>>>
>>> - Remove reference for IP version
>>> - Fix header coding style
>>> - Remove notes which are visible from the code
>>> - Fix driver license according to header
>>>
>>> Signed-off-by: Michal Simek<michal.simek@xilinx.com>
>>> ---
>>>   drivers/watchdog/of_xilinx_wdt.c | 30 ++++++++++--------------------
>>>   1 file changed, 10 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c
>>> b/drivers/watchdog/of_xilinx_wdt.c
>>> index 2761ddb..d4a35ab 100644
>>> --- a/drivers/watchdog/of_xilinx_wdt.c
>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>>> @@ -1,23 +1,13 @@
>>>   /*
>>> -*   of_xilinx_wdt.c  1.01  A Watchdog Device Driver for Xilinx
>>> xps_timebase_wdt
>>> -*
>>> -*   (C) Copyright 2011 (Alejandro Cabrera<aldaya@gmail.com>)
>>> -*
>>> -*       -----------------------
>>> -*/
>>> + * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
>>> + *
>>> + * (C) Copyright 2011 (Alejandro Cabrera<aldaya@gmail.com>)
>> Should year not be updated?
> This is just header fixup.
> I think he is not working/maintaining this driver for a long time.
Indeed :(.
Since this driver was upstream the new watchdog subsystem was under 
development and I want to port it to the new core, but unfortunately I 
haven't enough time :(.
> I would write there Xilinx copyright but I really don't care.
>
> Thanks,
> Michal
Thanks, you,
Alejandro




48 Aniversario de la Cujae, Una obra de la Revolucion Cubana | 2 de diciembre de 1964 | http://cujae.edu.cu

Consulte la enciclopedia colaborativa cubana. http://www.ecured.cu

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2122 bytes --]

On 05/30/2013 04:21 PM, Guenter Roeck wrote:
> On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
>> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
>>> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
>>>> Standard watchdog programs try to setup timeout
>>>> via ioctl and this functionality should be implemented.
>>>> Timeout value is hardcoded in the hardware but
>>>> based on Documentation/watchdog/watchdog-api.txt
>>>> can return the real timeout used in the same variable.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
>>>> index 79f358c..a3bbe72 100644
>>>> --- a/drivers/watchdog/of_xilinx_wdt.c
>>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>>>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>>>  		xwdt_keepalive();
>>>>  		return 0;
>>>>
>>>> +	case WDIOC_SETTIMEOUT:
>>>>  	case WDIOC_GETTIMEOUT:
>>>>  		if (no_timeout)
>>>>  			return -ENOTTY;
>>>
>>> Watchdog programs should check ident.options before trying to set the timeout.
>>> If they don't, there is an application bug. I don't think it is a good idea
>>> to start hacking the kernel to work around application bugs.
>>
>> Based on Documentation/watchdog/watchdog-api.txt
>>
>> "For some drivers it is possible to modify the watchdog timeout on the
>> fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
>> flag set in their option field.  The argument is an integer
> 
> Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.

ok. It means I should probably enable it.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Guenter Roeck]

On Thu, May 30, 2013 at 04:34:02PM +0200, Michal Simek wrote:
> On 05/30/2013 04:21 PM, Guenter Roeck wrote:
> > On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
> >> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
> >>> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
> >>>> Standard watchdog programs try to setup timeout
> >>>> via ioctl and this functionality should be implemented.
> >>>> Timeout value is hardcoded in the hardware but
> >>>> based on Documentation/watchdog/watchdog-api.txt
> >>>> can return the real timeout used in the same variable.
> >>>>
> >>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >>>> ---
> >>>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> >>>> index 79f358c..a3bbe72 100644
> >>>> --- a/drivers/watchdog/of_xilinx_wdt.c
> >>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
> >>>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >>>>  		xwdt_keepalive();
> >>>>  		return 0;
> >>>>
> >>>> +	case WDIOC_SETTIMEOUT:
> >>>>  	case WDIOC_GETTIMEOUT:
> >>>>  		if (no_timeout)
> >>>>  			return -ENOTTY;
> >>>
> >>> Watchdog programs should check ident.options before trying to set the timeout.
> >>> If they don't, there is an application bug. I don't think it is a good idea
> >>> to start hacking the kernel to work around application bugs.
> >>
> >> Based on Documentation/watchdog/watchdog-api.txt
> >>
> >> "For some drivers it is possible to modify the watchdog timeout on the
> >> fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
> >> flag set in their option field.  The argument is an integer
> > 
> > Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.
> 
> ok. It means I should probably enable it.
> 
I am missing your point. Applications should not try to write the timeout
since WDIOF_SETTIMEOUT is not set. Any application doing it anyway is buggy
and should be fixed.

Guenter

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

On 05/30/2013 05:03 PM, Guenter Roeck wrote:
> On Thu, May 30, 2013 at 04:34:02PM +0200, Michal Simek wrote:
>> On 05/30/2013 04:21 PM, Guenter Roeck wrote:
>>> On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
>>>> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
>>>>> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
>>>>>> Standard watchdog programs try to setup timeout
>>>>>> via ioctl and this functionality should be implemented.
>>>>>> Timeout value is hardcoded in the hardware but
>>>>>> based on Documentation/watchdog/watchdog-api.txt
>>>>>> can return the real timeout used in the same variable.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>> ---
>>>>>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
>>>>>> index 79f358c..a3bbe72 100644
>>>>>> --- a/drivers/watchdog/of_xilinx_wdt.c
>>>>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>>>>>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>>>>>  		xwdt_keepalive();
>>>>>>  		return 0;
>>>>>>
>>>>>> +	case WDIOC_SETTIMEOUT:
>>>>>>  	case WDIOC_GETTIMEOUT:
>>>>>>  		if (no_timeout)
>>>>>>  			return -ENOTTY;
>>>>>
>>>>> Watchdog programs should check ident.options before trying to set the timeout.
>>>>> If they don't, there is an application bug. I don't think it is a good idea
>>>>> to start hacking the kernel to work around application bugs.
>>>>
>>>> Based on Documentation/watchdog/watchdog-api.txt
>>>>
>>>> "For some drivers it is possible to modify the watchdog timeout on the
>>>> fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
>>>> flag set in their option field.  The argument is an integer
>>>
>>> Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.
>>
>> ok. It means I should probably enable it.
>>
> I am missing your point. Applications should not try to write the timeout
> since WDIOF_SETTIMEOUT is not set. Any application doing it anyway is buggy
> and should be fixed.

I fully understand your points and 100% agree with you
1. the application is broken and should be fixed
2. also the kernel shouldn't fix any problem with stupid application

But based on documentation the driver can support setup timeout
and based on description "the driver returns the real timeout used
in the same variable and this timeout might differ from the requested one
due to limitation of the hardware"
Based on this I still think that the driver can support set timeout
feature and if the driver supports this option then WDIOF_SETTIMEOUT
should be set in driver's option field. And I would add this to v2.

Can you see my point now?
Or my point of view is completely incorrect that this driver can't
support set timeout option.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Guenter Roeck]

On Thu, May 30, 2013 at 05:12:24PM +0200, Michal Simek wrote:
> On 05/30/2013 05:03 PM, Guenter Roeck wrote:
> > On Thu, May 30, 2013 at 04:34:02PM +0200, Michal Simek wrote:
> >> On 05/30/2013 04:21 PM, Guenter Roeck wrote:
> >>> On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
> >>>> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
> >>>>> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
> >>>>>> Standard watchdog programs try to setup timeout
> >>>>>> via ioctl and this functionality should be implemented.
> >>>>>> Timeout value is hardcoded in the hardware but
> >>>>>> based on Documentation/watchdog/watchdog-api.txt
> >>>>>> can return the real timeout used in the same variable.
> >>>>>>
> >>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >>>>>> ---
> >>>>>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
> >>>>>>  1 file changed, 1 insertion(+)
> >>>>>>
> >>>>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> >>>>>> index 79f358c..a3bbe72 100644
> >>>>>> --- a/drivers/watchdog/of_xilinx_wdt.c
> >>>>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
> >>>>>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >>>>>>  		xwdt_keepalive();
> >>>>>>  		return 0;
> >>>>>>
> >>>>>> +	case WDIOC_SETTIMEOUT:
> >>>>>>  	case WDIOC_GETTIMEOUT:
> >>>>>>  		if (no_timeout)
> >>>>>>  			return -ENOTTY;
> >>>>>
> >>>>> Watchdog programs should check ident.options before trying to set the timeout.
> >>>>> If they don't, there is an application bug. I don't think it is a good idea
> >>>>> to start hacking the kernel to work around application bugs.
> >>>>
> >>>> Based on Documentation/watchdog/watchdog-api.txt
> >>>>
> >>>> "For some drivers it is possible to modify the watchdog timeout on the
> >>>> fly with the SETTIMEOUT ioctl, those drivers have the WDIOF_SETTIMEOUT
> >>>> flag set in their option field.  The argument is an integer
> >>>
> >>> Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.
> >>
> >> ok. It means I should probably enable it.
> >>
> > I am missing your point. Applications should not try to write the timeout
> > since WDIOF_SETTIMEOUT is not set. Any application doing it anyway is buggy
> > and should be fixed.
> 
> I fully understand your points and 100% agree with you
> 1. the application is broken and should be fixed
> 2. also the kernel shouldn't fix any problem with stupid application
> 
> But based on documentation the driver can support setup timeout
> and based on description "the driver returns the real timeout used
> in the same variable and this timeout might differ from the requested one
> due to limitation of the hardware"
> Based on this I still think that the driver can support set timeout
> feature and if the driver supports this option then WDIOF_SETTIMEOUT
> should be set in driver's option field. And I would add this to v2.
> 
> Can you see my point now?

No. The driver doesn't support setting the timeout. You just want it
to falsely claim that it does to work around an application problem.
With your logic, _every_ watchdog driver would "support" setting
the timeout.

I'll defer to a higher authority, ie Wim.

Guenter

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Wim Van Sebroeck]

Hi All,

> > On Thu, May 30, 2013 at 05:12:24PM +0200, Michal Simek wrote:
> > > On 05/30/2013 05:03 PM, Guenter Roeck wrote:
> > > > On Thu, May 30, 2013 at 04:34:02PM +0200, Michal Simek wrote:
> > > >> On 05/30/2013 04:21 PM, Guenter Roeck wrote:
> > > >>> On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
> > > >>>> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
> > > >>>>> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
> > > >>>>>> Standard watchdog programs try to setup timeout
> > > >>>>>> via ioctl and this functionality should be implemented.
> > > >>>>>> Timeout value is hardcoded in the hardware but
> > > >>>>>> based on Documentation/watchdog/watchdog-api.txt
> > > >>>>>> can return the real timeout used in the same variable.
> > > >>>>>>
> > > >>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > > >>>>>> ---
> > > >>>>>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
> > > >>>>>>  1 file changed, 1 insertion(+)
> > > >>>>>>
> > > >>>>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c
> > b/drivers/watchdog/of_xilinx_wdt.c
> > > >>>>>> index 79f358c..a3bbe72 100644
> > > >>>>>> --- a/drivers/watchdog/of_xilinx_wdt.c
> > > >>>>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
> > > >>>>>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file,
> > unsigned int cmd, unsigned long arg)
> > > >>>>>>                xwdt_keepalive();
> > > >>>>>>                return 0;
> > > >>>>>>
> > > >>>>>> +      case WDIOC_SETTIMEOUT:
> > > >>>>>>        case WDIOC_GETTIMEOUT:
> > > >>>>>>                if (no_timeout)
> > > >>>>>>                        return -ENOTTY;
> > > >>>>>
> > > >>>>> Watchdog programs should check ident.options before trying to set
> > the timeout.
> > > >>>>> If they don't, there is an application bug. I don't think it is a
> > good idea
> > > >>>>> to start hacking the kernel to work around application bugs.
> > > >>>>
> > > >>>> Based on Documentation/watchdog/watchdog-api.txt
> > > >>>>
> > > >>>> "For some drivers it is possible to modify the watchdog timeout on
> > the
> > > >>>> fly with the SETTIMEOUT ioctl, those drivers have the
> > WDIOF_SETTIMEOUT
> > > >>>> flag set in their option field.  The argument is an integer
> > > >>>
> > > >>> Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.
> > > >>
> > > >> ok. It means I should probably enable it.
> > > >>
> > > > I am missing your point. Applications should not try to write the
> > timeout
> > > > since WDIOF_SETTIMEOUT is not set. Any application doing it anyway is
> > buggy
> > > > and should be fixed.
> > >
> > > I fully understand your points and 100% agree with you
> > > 1. the application is broken and should be fixed
> > > 2. also the kernel shouldn't fix any problem with stupid application
> > >
> > > But based on documentation the driver can support setup timeout
> > > and based on description "the driver returns the real timeout used
> > > in the same variable and this timeout might differ from the requested one
> > > due to limitation of the hardware"
> > > Based on this I still think that the driver can support set timeout
> > > feature and if the driver supports this option then WDIOF_SETTIMEOUT
> > > should be set in driver's option field. And I would add this to v2.
> > >
> > > Can you see my point now?
> >
> > No. The driver doesn't support setting the timeout. You just want it
> > to falsely claim that it does to work around an application problem.
> > With your logic, _every_ watchdog driver would "support" setting
> > the timeout.
> >
> 
> I don't want to falsely claim anything.
> I am just saying this is written in the documentation and
> it is my understanding that this can be implemented it this way
> for this xilinx device and behaviour of the driver will be correct
> according to documentation which I copied to this thread.
> 
> If Wim says that if device doesn't support setting timeout in HW
> then this ioctl can't be implemented in this way I am definitely OK with
> it.
> And I will remove this patch and will also remove this change
> from our xilinx repo.
> The main purpose for me is to cleanup our repo and push all changes
> to the mainline. Of course if this change goes against watchdog
> logic and it is broken I will the first who will revert it in our repo
> and will have proper description based on our discussion.
> 
> I really appreciate your inputs for this discussion and both
> resolution ACK/NACK are OK because I will know what's the correct
> way and I can fix it in mainline or our repo and both will be in sync.

Logic is: if device supports setting timeout values, then and only then
you indicate this by setting the WDIOF_SETTIMEOUT flag and adding the code
for it. And then you can do minor adjustments if needed and report this back.
But that's only to make sure that some roundings (like a timer in minutes 
gives back 60 or 120 seconds if you would set a new timeout of 70 seconds).

So in this case: the HW doesn't support setting timeout values,
so we don't add the WDIOF_SETTIMEOUT flag and thus we don't add the ioctl
call neither. So NACK on this patch.

Kind regards,
Wim.

Re: [PATCH 3/3] watchdog: xilinx: Add WDIOC_SETTIMEOUT ioctl function

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 5613 bytes --]

On 05/31/2013 12:08 AM, Wim Van Sebroeck wrote:
> Hi All,
> 
>>> On Thu, May 30, 2013 at 05:12:24PM +0200, Michal Simek wrote:
>>>> On 05/30/2013 05:03 PM, Guenter Roeck wrote:
>>>>> On Thu, May 30, 2013 at 04:34:02PM +0200, Michal Simek wrote:
>>>>>> On 05/30/2013 04:21 PM, Guenter Roeck wrote:
>>>>>>> On Thu, May 30, 2013 at 04:15:45PM +0200, Michal Simek wrote:
>>>>>>>> On 05/30/2013 04:07 PM, Guenter Roeck wrote:
>>>>>>>>> On Thu, May 30, 2013 at 02:26:04PM +0200, Michal Simek wrote:
>>>>>>>>>> Standard watchdog programs try to setup timeout
>>>>>>>>>> via ioctl and this functionality should be implemented.
>>>>>>>>>> Timeout value is hardcoded in the hardware but
>>>>>>>>>> based on Documentation/watchdog/watchdog-api.txt
>>>>>>>>>> can return the real timeout used in the same variable.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>>>> ---
>>>>>>>>>>  drivers/watchdog/of_xilinx_wdt.c | 1 +
>>>>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/watchdog/of_xilinx_wdt.c
>>> b/drivers/watchdog/of_xilinx_wdt.c
>>>>>>>>>> index 79f358c..a3bbe72 100644
>>>>>>>>>> --- a/drivers/watchdog/of_xilinx_wdt.c
>>>>>>>>>> +++ b/drivers/watchdog/of_xilinx_wdt.c
>>>>>>>>>> @@ -253,6 +253,7 @@ static long xwdt_ioctl(struct file *file,
>>> unsigned int cmd, unsigned long arg)
>>>>>>>>>>                xwdt_keepalive();
>>>>>>>>>>                return 0;
>>>>>>>>>>
>>>>>>>>>> +      case WDIOC_SETTIMEOUT:
>>>>>>>>>>        case WDIOC_GETTIMEOUT:
>>>>>>>>>>                if (no_timeout)
>>>>>>>>>>                        return -ENOTTY;
>>>>>>>>>
>>>>>>>>> Watchdog programs should check ident.options before trying to set
>>> the timeout.
>>>>>>>>> If they don't, there is an application bug. I don't think it is a
>>> good idea
>>>>>>>>> to start hacking the kernel to work around application bugs.
>>>>>>>>
>>>>>>>> Based on Documentation/watchdog/watchdog-api.txt
>>>>>>>>
>>>>>>>> "For some drivers it is possible to modify the watchdog timeout on
>>> the
>>>>>>>> fly with the SETTIMEOUT ioctl, those drivers have the
>>> WDIOF_SETTIMEOUT
>>>>>>>> flag set in their option field.  The argument is an integer
>>>>>>>
>>>>>>> Yes, but WDIOF_SETTIMEOUT is not set in the driver's option field.
>>>>>>
>>>>>> ok. It means I should probably enable it.
>>>>>>
>>>>> I am missing your point. Applications should not try to write the
>>> timeout
>>>>> since WDIOF_SETTIMEOUT is not set. Any application doing it anyway is
>>> buggy
>>>>> and should be fixed.
>>>>
>>>> I fully understand your points and 100% agree with you
>>>> 1. the application is broken and should be fixed
>>>> 2. also the kernel shouldn't fix any problem with stupid application
>>>>
>>>> But based on documentation the driver can support setup timeout
>>>> and based on description "the driver returns the real timeout used
>>>> in the same variable and this timeout might differ from the requested one
>>>> due to limitation of the hardware"
>>>> Based on this I still think that the driver can support set timeout
>>>> feature and if the driver supports this option then WDIOF_SETTIMEOUT
>>>> should be set in driver's option field. And I would add this to v2.
>>>>
>>>> Can you see my point now?
>>>
>>> No. The driver doesn't support setting the timeout. You just want it
>>> to falsely claim that it does to work around an application problem.
>>> With your logic, _every_ watchdog driver would "support" setting
>>> the timeout.
>>>
>>
>> I don't want to falsely claim anything.
>> I am just saying this is written in the documentation and
>> it is my understanding that this can be implemented it this way
>> for this xilinx device and behaviour of the driver will be correct
>> according to documentation which I copied to this thread.
>>
>> If Wim says that if device doesn't support setting timeout in HW
>> then this ioctl can't be implemented in this way I am definitely OK with
>> it.
>> And I will remove this patch and will also remove this change
>> from our xilinx repo.
>> The main purpose for me is to cleanup our repo and push all changes
>> to the mainline. Of course if this change goes against watchdog
>> logic and it is broken I will the first who will revert it in our repo
>> and will have proper description based on our discussion.
>>
>> I really appreciate your inputs for this discussion and both
>> resolution ACK/NACK are OK because I will know what's the correct
>> way and I can fix it in mainline or our repo and both will be in sync.
> 
> Logic is: if device supports setting timeout values, then and only then
> you indicate this by setting the WDIOF_SETTIMEOUT flag and adding the code
> for it. And then you can do minor adjustments if needed and report this back.
> But that's only to make sure that some roundings (like a timer in minutes 
> gives back 60 or 120 seconds if you would set a new timeout of 70 seconds).
> 
> So in this case: the HW doesn't support setting timeout values,
> so we don't add the WDIOF_SETTIMEOUT flag and thus we don't add the ioctl
> call neither. So NACK on this patch.

ok. Good. I will revert this change in our tree and will send v2 without
it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]