[PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Maxin B. John]

  CC [M]  drivers/staging/iio/light/tsl2563.o
drivers/staging/iio/light/tsl2563.c: In function 'tsl2563_probe':
drivers/staging/iio/light/tsl2563.c:736: warning: 'id' may be used 
uninitialized in this function

Also fixes the tsl2563_read_id().

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
---
diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
index f25243b..f8f787d 100644
--- a/drivers/staging/iio/light/tsl2563.c
+++ b/drivers/staging/iio/light/tsl2563.c
@@ -225,9 +225,9 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
 
 	ret = i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL2563_REG_ID);
 	if (ret < 0)
-		return ret;
+		return -EIO;
 
-	return 0;
+	return ret;
 }
 
 /*
@@ -697,7 +697,7 @@ static int __devinit tsl2563_probe(struct i2c_client *client,
 	struct tsl2563_platform_data *pdata = client->dev.platform_data;
 	int err = 0;
 	int ret;
-	u8 id;
+	u8 id = 0;
 
 	indio_dev = iio_allocate_device(sizeof(*chip));
 	if (!indio_dev)
@@ -709,15 +709,17 @@ static int __devinit tsl2563_probe(struct i2c_client *client,
 	chip->client = client;
 
 	err = tsl2563_detect(chip);
-	if (err) {
+	if (err < 0) {
 		dev_err(&client->dev, "device not found, error %d\n", -err);
 		goto fail1;
 	}
 
 	err = tsl2563_read_id(chip, &id);
-	if (err)
+	if (err < 0) {
+		dev_err(&client->dev, "id reading failed, error %d\n", -err);
 		goto fail1;
-
+	}
+	id = err;
 	mutex_init(&chip->lock);
 
 	/* Default values used until userspace says otherwise */

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Dan Carpenter]

On Fri, Sep 02, 2011 at 12:24:13AM +0300, Maxin B. John wrote:
> --- a/drivers/staging/iio/light/tsl2563.c
> +++ b/drivers/staging/iio/light/tsl2563.c
> @@ -225,9 +225,9 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
>  
>  	ret = i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL2563_REG_ID);
>  	if (ret < 0)
> -		return ret;
> +		return -EIO;

Don't overwrite the error code.  For example, the lower layers can
return -EAGAIN and that's more useful than just returning -EIO every
time.

Your fix works, but it's not very clean.  Just add a "*id = ret;"
line before the "return 0;" and that's it.  (It doesn't make sense
to pass a pointer to "id" and not use it).

(In other words, don't make any changes to the tsl2563_probe()
function)

>  
> -	return 0;
> +	return ret;
>  }
>  

regards,
dan carpenter

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 09/01/11 22:55, Dan Carpenter wrote:
> On Fri, Sep 02, 2011 at 12:24:13AM +0300, Maxin B. John wrote:
>> --- a/drivers/staging/iio/light/tsl2563.c
>> +++ b/drivers/staging/iio/light/tsl2563.c
>> @@ -225,9 +225,9 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
>>  
>>  	ret = i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL2563_REG_ID);
>>  	if (ret < 0)
>> -		return ret;
>> +		return -EIO;
> 
> Don't overwrite the error code.  For example, the lower layers can
> return -EAGAIN and that's more useful than just returning -EIO every
> time.
> 
> Your fix works, but it's not very clean.  Just add a "*id = ret;"
> line before the "return 0;" and that's it.  (It doesn't make sense
> to pass a pointer to "id" and not use it).
> 
> (In other words, don't make any changes to the tsl2563_probe()
> function)
> 
>>  
>> -	return 0;
>> +	return ret;
>>  }
>> 
Yikes - I wonder why my various compilers don't throw that up.
Ah well Dan's fix is indeed the right one and what was intended.

Thanks,

Jonathan

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 10/24/11 10:03, Maxin B John wrote:
> Hi,
> 
> On Mon, Oct 24, 2011 at 11:51 AM, Jonathan Cameron <jic23@cam.ac.uk> wrote:
>>
>>
>> On 10/22/11 06:15, Maxin B John wrote:
>>> Hi,
>>>
>>> On Fri, Sep 2, 2011 at 1:55 PM, Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org> wrote:
>>>> On 09/02/11 10:41, Maxin B. John wrote:
>>>>>> Don't overwrite the error code.  For example, the lower layers can
>>>>>> return -EAGAIN and that's more useful than just returning -EIO every
>>>>>> time.
>>>>> Ahh.. Thanks a lot for explaining this.
>>>>>
>>>>>> Your fix works, but it's not very clean.  Just add a "*id = ret;"
>>>>>> line before the "return 0;" and that's it.  (It doesn't make sense
>>>>>> to pass a pointer to "id" and not use it).
>>>>> Dan, yes, I agree with you. This fix is much much better than what I
>>>>> had in my mind.
>>>>>
>>>>>> Yikes - I wonder why my various compilers don't throw that up.
>>>>> I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.
>>>> That'd do it. oops.
>>>>
>>>> Ideally keep the white space but doesn't really matter. Either send on
>>>> to Greg directly or I'll add it to iio-blue and send on with the next fixes
>>>> series - probably this afternoon.
>>>>>
>>>>> Signed-off-by: Maxin B. John <maxin.john-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>>>
>>>
>>> Sorry for the delay in replying to this mail. I am curious about the
>>> status of this patch. I couldn't locate this patch at  iio-blue.git as
>>> it is not present in kernel.org now
>>> (http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
>>> Please ignore this mail if you have already added this patch to your tree.
>> Ah, sorry I slipped up here and should have noticed it didn't go directly to
>> Greg. Just to check, with everyone I have author as Dan, reporter as Maxin
>> and have signed off myself (it's going through my tree anyway now).
>>
>> I think from looking back it was Dan's suggestion in response to Maxin's
>> patch.  Hence convention would be a sign off from Dan and perhaps an
>> Ack from Maxin?
> 
> Sounds good to me.
> 
> Acked-by: Maxin B. John <maxin.john@gmail.com>
> 
As Dan reckon's you are the author I'll switch that to a signed-off by.

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 10/24/11 10:03, Maxin B John wrote:
> Hi,
> 
> On Mon, Oct 24, 2011 at 11:51 AM, Jonathan Cameron <jic23@cam.ac.uk> wrote:
>>
>>
>> On 10/22/11 06:15, Maxin B John wrote:
>>> Hi,
>>>
>>> On Fri, Sep 2, 2011 at 1:55 PM, Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org> wrote:
>>>> On 09/02/11 10:41, Maxin B. John wrote:
>>>>>> Don't overwrite the error code.  For example, the lower layers can
>>>>>> return -EAGAIN and that's more useful than just returning -EIO every
>>>>>> time.
>>>>> Ahh.. Thanks a lot for explaining this.
>>>>>
>>>>>> Your fix works, but it's not very clean.  Just add a "*id = ret;"
>>>>>> line before the "return 0;" and that's it.  (It doesn't make sense
>>>>>> to pass a pointer to "id" and not use it).
>>>>> Dan, yes, I agree with you. This fix is much much better than what I
>>>>> had in my mind.
>>>>>
>>>>>> Yikes - I wonder why my various compilers don't throw that up.
>>>>> I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.
>>>> That'd do it. oops.
>>>>
>>>> Ideally keep the white space but doesn't really matter. Either send on
>>>> to Greg directly or I'll add it to iio-blue and send on with the next fixes
>>>> series - probably this afternoon.
>>>>>
>>>>> Signed-off-by: Maxin B. John <maxin.john-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>>>
>>>
>>> Sorry for the delay in replying to this mail. I am curious about the
>>> status of this patch. I couldn't locate this patch at  iio-blue.git as
>>> it is not present in kernel.org now
>>> (http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
>>> Please ignore this mail if you have already added this patch to your tree.
>> Ah, sorry I slipped up here and should have noticed it didn't go directly to
>> Greg. Just to check, with everyone I have author as Dan, reporter as Maxin
>> and have signed off myself (it's going through my tree anyway now).
>>
>> I think from looking back it was Dan's suggestion in response to Maxin's
>> patch.  Hence convention would be a sign off from Dan and perhaps an
>> Ack from Maxin?
> 
> Sounds good to me.
> 
> Acked-by: Maxin B. John <maxin.john@gmail.com>
> 
As Dan reckon's you are the author I'll switch that to a signed-off by.

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 10/24/11 10:01, Dan Carpenter wrote:
> On Mon, Oct 24, 2011 at 09:51:28AM +0100, Jonathan Cameron wrote:
>>>>> Signed-off-by: Maxin B. John <maxin.john-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>>>
>>>
>>> Sorry for the delay in replying to this mail. I am curious about the
>>> status of this patch. I couldn't locate this patch at  iio-blue.git as
>>> it is not present in kernel.org now
>>> (http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
>>> Please ignore this mail if you have already added this patch to your tree.
>> Ah, sorry I slipped up here and should have noticed it didn't go directly to
>> Greg. Just to check, with everyone I have author as Dan, reporter as Maxin
>> and have signed off myself (it's going through my tree anyway now).
>>
>> I think from looking back it was Dan's suggestion in response to Maxin's
>> patch.  Hence convention would be a sign off from Dan and perhaps an
>> Ack from Maxin?
>>
> 
> Maxim is the author.  He didn't resend this in the proper way so it
> was confusing...  Can you give me a Reviewed-by tag?
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> 

Sure.

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 10/24/11 10:01, Dan Carpenter wrote:
> On Mon, Oct 24, 2011 at 09:51:28AM +0100, Jonathan Cameron wrote:
>>>>> Signed-off-by: Maxin B. John <maxin.john-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>>>
>>>
>>> Sorry for the delay in replying to this mail. I am curious about the
>>> status of this patch. I couldn't locate this patch at  iio-blue.git as
>>> it is not present in kernel.org now
>>> (http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
>>> Please ignore this mail if you have already added this patch to your tree.
>> Ah, sorry I slipped up here and should have noticed it didn't go directly to
>> Greg. Just to check, with everyone I have author as Dan, reporter as Maxin
>> and have signed off myself (it's going through my tree anyway now).
>>
>> I think from looking back it was Dan's suggestion in response to Maxin's
>> patch.  Hence convention would be a sign off from Dan and perhaps an
>> Ack from Maxin?
>>
> 
> Maxim is the author.  He didn't resend this in the proper way so it
> was confusing...  Can you give me a Reviewed-by tag?
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> 

Sure.

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 10/22/11 06:15, Maxin B John wrote:
> Hi,
> 
> On Fri, Sep 2, 2011 at 1:55 PM, Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org> wrote:
>> On 09/02/11 10:41, Maxin B. John wrote:
>>>> Don't overwrite the error code.  For example, the lower layers can
>>>> return -EAGAIN and that's more useful than just returning -EIO every
>>>> time.
>>> Ahh.. Thanks a lot for explaining this.
>>>
>>>> Your fix works, but it's not very clean.  Just add a "*id = ret;"
>>>> line before the "return 0;" and that's it.  (It doesn't make sense
>>>> to pass a pointer to "id" and not use it).
>>> Dan, yes, I agree with you. This fix is much much better than what I
>>> had in my mind.
>>>
>>>> Yikes - I wonder why my various compilers don't throw that up.
>>> I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.
>> That'd do it. oops.
>>
>> Ideally keep the white space but doesn't really matter. Either send on
>> to Greg directly or I'll add it to iio-blue and send on with the next fixes
>> series - probably this afternoon.
>>>
>>> Signed-off-by: Maxin B. John <maxin.john-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>
> 
> Sorry for the delay in replying to this mail. I am curious about the
> status of this patch. I couldn't locate this patch at  iio-blue.git as
> it is not present in kernel.org now
> (http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
> Please ignore this mail if you have already added this patch to your tree.
Ah, sorry I slipped up here and should have noticed it didn't go directly to
Greg. Just to check, with everyone I have author as Dan, reporter as Maxin
and have signed off myself (it's going through my tree anyway now).

I think from looking back it was Dan's suggestion in response to Maxin's
patch.  Hence convention would be a sign off from Dan and perhaps an
Ack from Maxin?

Jonathan

>From c33a01010decf686f5f2984d2ed04c1a70e18bc3 Mon Sep 17 00:00:00 2001
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 24 Oct 2011 09:46:32 +0100
Subject: [PATCH] staging:iio:light:tsl2563 missing setting of id in get id
 function.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
Reported-by: Maxin B. John <maxin.john@gmail.com>
---
 drivers/staging/iio/light/tsl2563.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
index 5cee5ca..45d04a1 100644
--- a/drivers/staging/iio/light/tsl2563.c
+++ b/drivers/staging/iio/light/tsl2563.c
@@ -227,6 +227,8 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
 	if (ret < 0)
 		return ret;
 
+	*id = ret;
+
 	return 0;
 }
 
-- 
1.7.7

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 10/22/11 06:15, Maxin B John wrote:
> Hi,
> 
> On Fri, Sep 2, 2011 at 1:55 PM, Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org> wrote:
>> On 09/02/11 10:41, Maxin B. John wrote:
>>>> Don't overwrite the error code.  For example, the lower layers can
>>>> return -EAGAIN and that's more useful than just returning -EIO every
>>>> time.
>>> Ahh.. Thanks a lot for explaining this.
>>>
>>>> Your fix works, but it's not very clean.  Just add a "*id = ret;"
>>>> line before the "return 0;" and that's it.  (It doesn't make sense
>>>> to pass a pointer to "id" and not use it).
>>> Dan, yes, I agree with you. This fix is much much better than what I
>>> had in my mind.
>>>
>>>> Yikes - I wonder why my various compilers don't throw that up.
>>> I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.
>> That'd do it. oops.
>>
>> Ideally keep the white space but doesn't really matter. Either send on
>> to Greg directly or I'll add it to iio-blue and send on with the next fixes
>> series - probably this afternoon.
>>>
>>> Signed-off-by: Maxin B. John <maxin.john-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>
> 
> Sorry for the delay in replying to this mail. I am curious about the
> status of this patch. I couldn't locate this patch at  iio-blue.git as
> it is not present in kernel.org now
> (http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
> Please ignore this mail if you have already added this patch to your tree.
Ah, sorry I slipped up here and should have noticed it didn't go directly to
Greg. Just to check, with everyone I have author as Dan, reporter as Maxin
and have signed off myself (it's going through my tree anyway now).

I think from looking back it was Dan's suggestion in response to Maxin's
patch.  Hence convention would be a sign off from Dan and perhaps an
Ack from Maxin?

Jonathan

>>From c33a01010decf686f5f2984d2ed04c1a70e18bc3 Mon Sep 17 00:00:00 2001
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 24 Oct 2011 09:46:32 +0100
Subject: [PATCH] staging:iio:light:tsl2563 missing setting of id in get id
 function.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
Reported-by: Maxin B. John <maxin.john@gmail.com>
---
 drivers/staging/iio/light/tsl2563.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
index 5cee5ca..45d04a1 100644
--- a/drivers/staging/iio/light/tsl2563.c
+++ b/drivers/staging/iio/light/tsl2563.c
@@ -227,6 +227,8 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
 	if (ret < 0)
 		return ret;
 
+	*id = ret;
+
 	return 0;
 }
 
-- 
1.7.7

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Maxin B John]

Hi,

On Fri, Sep 2, 2011 at 1:55 PM, Jonathan Cameron <jic23@cam.ac.uk> wrote:
> On 09/02/11 10:41, Maxin B. John wrote:
>>> Don't overwrite the error code.  For example, the lower layers can
>>> return -EAGAIN and that's more useful than just returning -EIO every
>>> time.
>> Ahh.. Thanks a lot for explaining this.
>>
>>> Your fix works, but it's not very clean.  Just add a "*id = ret;"
>>> line before the "return 0;" and that's it.  (It doesn't make sense
>>> to pass a pointer to "id" and not use it).
>> Dan, yes, I agree with you. This fix is much much better than what I
>> had in my mind.
>>
>>> Yikes - I wonder why my various compilers don't throw that up.
>> I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.
> That'd do it. oops.
>
> Ideally keep the white space but doesn't really matter. Either send on
> to Greg directly or I'll add it to iio-blue and send on with the next fixes
> series - probably this afternoon.
>>
>> Signed-off-by: Maxin B. John <maxin.john@gmail.com>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>

Sorry for the delay in replying to this mail. I am curious about the
status of this patch. I couldn't locate this patch at  iio-blue.git as
it is not present in kernel.org now
(http://git.kernel.org/?p=linux/kernel/git/jic23/iio-blue.git;a=summary)
Please ignore this mail if you have already added this patch to your tree.

>> ---
>> diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
>> index f25243b..55012ff 100644
>> --- a/drivers/staging/iio/light/tsl2563.c
>> +++ b/drivers/staging/iio/light/tsl2563.c
>> @@ -226,7 +226,7 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
>>       ret = i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL2563_REG_ID);
>>       if (ret < 0)
>>               return ret;
>> -
>> +     *id = ret;
>>       return 0;
>>  }
>>
>>
>
>

Warm Regards,
Maxin B. John

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Maxin B John]

Hi,

On Fri, Sep 2, 2011 at 1:55 PM, Jonathan Cameron <jic23@cam.ac.uk> wrote:
> On 09/02/11 10:41, Maxin B. John wrote:
>>> Don't overwrite the error code. =A0For example, the lower layers can
>>> return -EAGAIN and that's more useful than just returning -EIO every
>>> time.
>> Ahh.. Thanks a lot for explaining this.
>>
>>> Your fix works, but it's not very clean. =A0Just add a "*id =3D ret;"
>>> line before the "return 0;" and that's it. =A0(It doesn't make sense
>>> to pass a pointer to "id" and not use it).
>> Dan, yes, I agree with you. This fix is much much better than what I
>> had in my mind.
>>
>>> Yikes - I wonder why my various compilers don't throw that up.
>> I guess, in "iio-blue.git" tree, the 'id =3D 0' suppresses this warning.
> That'd do it. oops.
>
> Ideally keep the white space but doesn't really matter. Either send on
> to Greg directly or I'll add it to iio-blue and send on with the next fix=
es
> series - probably this afternoon.
>>
>> Signed-off-by: Maxin B. John <maxin.john@gmail.com>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>

Sorry for the delay in replying to this mail. I am curious about the
status of this patch. I couldn't locate this patch at  iio-blue.git as
it is not present in kernel.org now
(http://git.kernel.org/?p=3Dlinux/kernel/git/jic23/iio-blue.git;a=3Dsummary=
)
Please ignore this mail if you have already added this patch to your tree.

>> ---
>> diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/l=
ight/tsl2563.c
>> index f25243b..55012ff 100644
>> --- a/drivers/staging/iio/light/tsl2563.c
>> +++ b/drivers/staging/iio/light/tsl2563.c
>> @@ -226,7 +226,7 @@ static int tsl2563_read_id(struct tsl2563_chip *chip=
, u8 *id)
>> =A0 =A0 =A0 ret =3D i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL25=
63_REG_ID);
>> =A0 =A0 =A0 if (ret < 0)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return ret;
>> -
>> + =A0 =A0 *id =3D ret;
>> =A0 =A0 =A0 return 0;
>> =A0}
>>
>>
>
>

Warm Regards,
Maxin B. John

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Jonathan Cameron]

On 09/02/11 10:41, Maxin B. John wrote:
>> Don't overwrite the error code.  For example, the lower layers can
>> return -EAGAIN and that's more useful than just returning -EIO every
>> time.
> Ahh.. Thanks a lot for explaining this.
> 
>> Your fix works, but it's not very clean.  Just add a "*id = ret;"
>> line before the "return 0;" and that's it.  (It doesn't make sense
>> to pass a pointer to "id" and not use it).
> Dan, yes, I agree with you. This fix is much much better than what I 
> had in my mind.
> 
>> Yikes - I wonder why my various compilers don't throw that up.
> I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.
That'd do it. oops.

Ideally keep the white space but doesn't really matter. Either send on
to Greg directly or I'll add it to iio-blue and send on with the next fixes
series - probably this afternoon.
> 
> Signed-off-by: Maxin B. John <maxin.john@gmail.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>

> ---
> diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
> index f25243b..55012ff 100644
> --- a/drivers/staging/iio/light/tsl2563.c
> +++ b/drivers/staging/iio/light/tsl2563.c
> @@ -226,7 +226,7 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
>  	ret = i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL2563_REG_ID);
>  	if (ret < 0)
>  		return ret;
> -
> +	*id = ret;
>  	return 0;
>  }
>  
> 

Re: [PATCH] staging: iio: light: Fix compiler warning in tsl2563.c

[Maxin B. John]

> Don't overwrite the error code.  For example, the lower layers can
> return -EAGAIN and that's more useful than just returning -EIO every
> time.
Ahh.. Thanks a lot for explaining this.

> Your fix works, but it's not very clean.  Just add a "*id = ret;"
> line before the "return 0;" and that's it.  (It doesn't make sense
> to pass a pointer to "id" and not use it).
Dan, yes, I agree with you. This fix is much much better than what I 
had in my mind.

> Yikes - I wonder why my various compilers don't throw that up.
I guess, in "iio-blue.git" tree, the 'id = 0' suppresses this warning.

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
---
diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
index f25243b..55012ff 100644
--- a/drivers/staging/iio/light/tsl2563.c
+++ b/drivers/staging/iio/light/tsl2563.c
@@ -226,7 +226,7 @@ static int tsl2563_read_id(struct tsl2563_chip *chip, u8 *id)
 	ret = i2c_smbus_read_byte_data(client, TSL2563_CMD | TSL2563_REG_ID);
 	if (ret < 0)
 		return ret;
-
+	*id = ret;
 	return 0;
 }