linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
@ 2015-11-18 12:55 LABBE Corentin
  2015-11-18 12:55 ` [PATCH v3 1/1] " LABBE Corentin
  0 siblings, 1 reply; 5+ messages in thread
From: LABBE Corentin @ 2015-11-18 12:55 UTC (permalink / raw)
  To: jdelvare, wsa; +Cc: LABBE Corentin, linux-i2c, linux-kernel

Hello

Changes since v2
- removed err variable
- fix a spelling issue

Changes since v1
- drop the return code of kstrtou8 and return -EPROTO
  as suggested by Jean Delvare
- Added a comment on the return code drop

Regards

LABBE Corentin (1):
  i2c: taos-evm: replace simple_strtoul by kstrtou8

 drivers/i2c/busses/i2c-taos-evm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 
2.4.10

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
  2015-11-18 12:55 [PATCH v3 0/1] i2c: taos-evm: replace simple_strtoul by kstrtou8 LABBE Corentin
@ 2015-11-18 12:55 ` LABBE Corentin
  2015-11-18 16:26   ` Andy Shevchenko
  2015-11-21 20:11   ` Jean Delvare
  0 siblings, 2 replies; 5+ messages in thread
From: LABBE Corentin @ 2015-11-18 12:55 UTC (permalink / raw)
  To: jdelvare, wsa; +Cc: LABBE Corentin, linux-i2c, linux-kernel

The simple_strtoul function is marked as obsolete.
This patch replace it by kstrtou8.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/i2c/busses/i2c-taos-evm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-taos-evm.c b/drivers/i2c/busses/i2c-taos-evm.c
index 4c7fc2d..f673f5d 100644
--- a/drivers/i2c/busses/i2c-taos-evm.c
+++ b/drivers/i2c/busses/i2c-taos-evm.c
@@ -130,7 +130,13 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 			return 0;
 	} else {
 		if (p[0] == 'x') {
-			data->byte = simple_strtol(p + 1, NULL, 16);
+			/*
+			 * voluntarily dropping error code of kstrtou8 since all
+			 * error code that it could return are invalid according
+			 * to Documentation/i2c/fault-codes
+			 */
+			if (kstrtou8(p + 1, 16, &data->byte))
+				return -EPROTO;
 			return 0;
 		}
 	}
-- 
2.4.10

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
  2015-11-18 12:55 ` [PATCH v3 1/1] " LABBE Corentin
@ 2015-11-18 16:26   ` Andy Shevchenko
  2015-11-21 20:10     ` Jean Delvare
  2015-11-21 20:11   ` Jean Delvare
  1 sibling, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2015-11-18 16:26 UTC (permalink / raw)
  To: LABBE Corentin
  Cc: jdelvare, Wolfram Sang, linux-i2c, linux-kernel@vger.kernel.org

On Wed, Nov 18, 2015 at 2:55 PM, LABBE Corentin
<clabbe.montjoie@gmail.com> wrote:
> The simple_strtoul function is marked as obsolete.
> This patch replace it by kstrtou8.
>

Only one concern. simple_strto* goes through the string until it has
an invalid character or \0. In your case kstrtou8 will fail the
transfer. So, is there possible cases when HW returns such data?

And just a style nitpicks below.

>                 if (p[0] == 'x') {
> -                       data->byte = simple_strtol(p + 1, NULL, 16);
> +                       /*
> +                        * voluntarily dropping error code of kstrtou8 since all

-> Voluntarily…

> +                        * error code that it could return are invalid according
> +                        * to Documentation/i2c/fault-codes

-> …codes.

> +                        */
> +                       if (kstrtou8(p + 1, 16, &data->byte))
> +                               return -EPROTO;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
  2015-11-18 16:26   ` Andy Shevchenko
@ 2015-11-21 20:10     ` Jean Delvare
  0 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2015-11-21 20:10 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: LABBE Corentin, Wolfram Sang, linux-i2c, linux-kernel

Hi Andy,

On Wed, 18 Nov 2015 18:26:25 +0200, Andy Shevchenko wrote:
> On Wed, Nov 18, 2015 at 2:55 PM, LABBE Corentin
> <clabbe.montjoie@gmail.com> wrote:
> > The simple_strtoul function is marked as obsolete.
> > This patch replace it by kstrtou8.
> >
> 
> Only one concern. simple_strto* goes through the string until it has
> an invalid character or \0. In your case kstrtou8 will fail the
> transfer. So, is there possible cases when HW returns such data?

It's not supposed to happen.

> And just a style nitpicks below.
> 
> >                 if (p[0] == 'x') {
> > -                       data->byte = simple_strtol(p + 1, NULL, 16);
> > +                       /*
> > +                        * voluntarily dropping error code of kstrtou8 since all
> 
> -> Voluntarily…
> 
> > +                        * error code that it could return are invalid according
> > +                        * to Documentation/i2c/fault-codes
> 
> -> …codes.
> 
> > +                        */
> > +                       if (kstrtou8(p + 1, 16, &data->byte))
> > +                               return -EPROTO;
> 


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] i2c: taos-evm: replace simple_strtoul by kstrtou8
  2015-11-18 12:55 ` [PATCH v3 1/1] " LABBE Corentin
  2015-11-18 16:26   ` Andy Shevchenko
@ 2015-11-21 20:11   ` Jean Delvare
  1 sibling, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2015-11-21 20:11 UTC (permalink / raw)
  To: LABBE Corentin; +Cc: wsa, linux-i2c, linux-kernel

Hi Corentin,

On Wed, 18 Nov 2015 13:55:56 +0100, LABBE Corentin wrote:
> The simple_strtoul function is marked as obsolete.
> This patch replace it by kstrtou8.
> 
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Tested-by: Jean Delvare <jdelvare@suse.de>

Note: when there's a single patch you don't have to send an email with
PATCH 0/1.

> ---
>  drivers/i2c/busses/i2c-taos-evm.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-taos-evm.c b/drivers/i2c/busses/i2c-taos-evm.c
> index 4c7fc2d..f673f5d 100644
> --- a/drivers/i2c/busses/i2c-taos-evm.c
> +++ b/drivers/i2c/busses/i2c-taos-evm.c
> @@ -130,7 +130,13 @@ static int taos_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
>  			return 0;
>  	} else {
>  		if (p[0] == 'x') {
> -			data->byte = simple_strtol(p + 1, NULL, 16);
> +			/*
> +			 * voluntarily dropping error code of kstrtou8 since all
> +			 * error code that it could return are invalid according
> +			 * to Documentation/i2c/fault-codes
> +			 */
> +			if (kstrtou8(p + 1, 16, &data->byte))
> +				return -EPROTO;
>  			return 0;
>  		}
>  	}


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-11-21 20:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-18 12:55 [PATCH v3 0/1] i2c: taos-evm: replace simple_strtoul by kstrtou8 LABBE Corentin
2015-11-18 12:55 ` [PATCH v3 1/1] " LABBE Corentin
2015-11-18 16:26   ` Andy Shevchenko
2015-11-21 20:10     ` Jean Delvare
2015-11-21 20:11   ` Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).