diff mbox

platform: x86: dell-smo8800: remove redundant assignments to byte_data

Message ID 20171031110318.20985-1-colin.king@canonical.com (mailing list archive)
State Accepted, archived
Delegated to: Andy Shevchenko
Headers show

Commit Message

Colin King Oct. 31, 2017, 11:03 a.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

Variable byte_data is being initialized and re-assigned with values that
are never read. Remove these as these redundant assignments. Cleans up
clang warning:

drivers/platform/x86/dell-smo8800.c:106:2: warning: Value stored to 'byte_data'
is never read

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/platform/x86/dell-smo8800.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Andy Shevchenko Oct. 31, 2017, 1:47 p.m. UTC | #1 
On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Variable byte_data is being initialized and re-assigned with values that
> are never read. Remove these as these redundant assignments. Cleans up
> clang warning:
>
> drivers/platform/x86/dell-smo8800.c:106:2: warning: Value stored to 'byte_data'
> is never read

Thanks for the patch.

As I read a code it was from the beginning like this. I dunno what was
a motivation of this pieces, thus I would like to hear Pali.

Per se patch looks pretty much good to me.
Pali Rohár Oct. 31, 2017, 1:55 p.m. UTC | #2 
On Tuesday 31 October 2017 15:47:58 Andy Shevchenko wrote:
> On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > Variable byte_data is being initialized and re-assigned with values that
> > are never read. Remove these as these redundant assignments. Cleans up
> > clang warning:
> >
> > drivers/platform/x86/dell-smo8800.c:106:2: warning: Value stored to 'byte_data'
> > is never read
> 
> Thanks for the patch.
> 
> As I read a code it was from the beginning like this. I dunno what was
> a motivation of this pieces, thus I would like to hear Pali.

IIRC this part of code comes from the lis3lv* driver which was used for
HP EliteBook machines. As both drivers provides similar interface for
userspace.

> Per se patch looks pretty much good to me.
>
Andy Shevchenko Oct. 31, 2017, 2:07 p.m. UTC | #3 
On Tue, Oct 31, 2017 at 3:55 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Tuesday 31 October 2017 15:47:58 Andy Shevchenko wrote:
>> On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:
>> > From: Colin Ian King <colin.king@canonical.com>
>> >
>> > Variable byte_data is being initialized and re-assigned with values that
>> > are never read. Remove these as these redundant assignments. Cleans up
>> > clang warning:
>> >
>> > drivers/platform/x86/dell-smo8800.c:106:2: warning: Value stored to 'byte_data'
>> > is never read
>>
>> Thanks for the patch.
>>
>> As I read a code it was from the beginning like this. I dunno what was
>> a motivation of this pieces, thus I would like to hear Pali.
>
> IIRC this part of code comes from the lis3lv* driver which was used for
> HP EliteBook machines. As both drivers provides similar interface for
> userspace.

OK, though it doesn't clarify the intention of the byte_data
(useless?) assignments.
Pali Rohár Oct. 31, 2017, 2:13 p.m. UTC | #4 
On Tuesday 31 October 2017 16:07:25 Andy Shevchenko wrote:
> On Tue, Oct 31, 2017 at 3:55 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > On Tuesday 31 October 2017 15:47:58 Andy Shevchenko wrote:
> >> On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:
> >> > From: Colin Ian King <colin.king@canonical.com>
> >> >
> >> > Variable byte_data is being initialized and re-assigned with values that
> >> > are never read. Remove these as these redundant assignments. Cleans up
> >> > clang warning:
> >> >
> >> > drivers/platform/x86/dell-smo8800.c:106:2: warning: Value stored to 'byte_data'
> >> > is never read
> >>
> >> Thanks for the patch.
> >>
> >> As I read a code it was from the beginning like this. I dunno what was
> >> a motivation of this pieces, thus I would like to hear Pali.
> >
> > IIRC this part of code comes from the lis3lv* driver which was used for
> > HP EliteBook machines. As both drivers provides similar interface for
> > userspace.
> 
> OK, though it doesn't clarify the intention of the byte_data
> (useless?) assignments.

Probably similar code pattern exists in that lis3lv* driver...
Andy Shevchenko Oct. 31, 2017, 6:08 p.m. UTC | #5 
On Tue, Oct 31, 2017 at 4:13 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Tuesday 31 October 2017 16:07:25 Andy Shevchenko wrote:
>> On Tue, Oct 31, 2017 at 3:55 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> > On Tuesday 31 October 2017 15:47:58 Andy Shevchenko wrote:
>> >> On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:

>> OK, though it doesn't clarify the intention of the byte_data
>> (useless?) assignments.
>
> Probably similar code pattern exists in that lis3lv* driver...

So, it seems to me OK to apply the patch. No objections?
Pali Rohár Oct. 31, 2017, 6:41 p.m. UTC | #6 
On Tuesday 31 October 2017 20:08:45 Andy Shevchenko wrote:
> On Tue, Oct 31, 2017 at 4:13 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > On Tuesday 31 October 2017 16:07:25 Andy Shevchenko wrote:
> >> On Tue, Oct 31, 2017 at 3:55 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> >> > On Tuesday 31 October 2017 15:47:58 Andy Shevchenko wrote:
> >> >> On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:
> 
> >> OK, though it doesn't clarify the intention of the byte_data
> >> (useless?) assignments.
> >
> > Probably similar code pattern exists in that lis3lv* driver...
> 
> So, it seems to me OK to apply the patch. No objections?

No objections, you can add my:

Acked-by: Pali Rohár <pali.rohar@gmail.com>
Andy Shevchenko Nov. 3, 2017, 12:37 p.m. UTC | #7 
On Tue, Oct 31, 2017 at 8:41 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Tuesday 31 October 2017 20:08:45 Andy Shevchenko wrote:
>> On Tue, Oct 31, 2017 at 4:13 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> > On Tuesday 31 October 2017 16:07:25 Andy Shevchenko wrote:
>> >> On Tue, Oct 31, 2017 at 3:55 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> >> > On Tuesday 31 October 2017 15:47:58 Andy Shevchenko wrote:
>> >> >> On Tue, Oct 31, 2017 at 1:03 PM, Colin King <colin.king@canonical.com> wrote:
>>
>> >> OK, though it doesn't clarify the intention of the byte_data
>> >> (useless?) assignments.
>> >
>> > Probably similar code pattern exists in that lis3lv* driver...
>>
>> So, it seems to me OK to apply the patch. No objections?
>
> No objections, you can add my:
>
> Acked-by: Pali Rohár <pali.rohar@gmail.com>

Applied to my review and testing queue, thanks!

>
> --
> Pali Rohár
> pali.rohar@gmail.com
diff mbox

Patch

diff --git a/drivers/platform/x86/dell-smo8800.c b/drivers/platform/x86/dell-smo8800.c
index 37e646034ef8..1d87237bc731 100644
--- a/drivers/platform/x86/dell-smo8800.c
+++ b/drivers/platform/x86/dell-smo8800.c
@@ -90,7 +90,7 @@  static ssize_t smo8800_misc_read(struct file *file, char __user *buf,
 					 struct smo8800_device, miscdev);
 
 	u32 data = 0;
-	unsigned char byte_data = 0;
+	unsigned char byte_data;
 	ssize_t retval = 1;
 
 	if (count < 1)
@@ -103,7 +103,6 @@  static ssize_t smo8800_misc_read(struct file *file, char __user *buf,
 	if (retval)
 		return retval;
 
-	byte_data = 1;
 	retval = 1;
 
 	if (data < 255)