Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()

[Andy Shevchenko]

On Fri, Nov 3, 2017 at 3:23 PM, SF Markus Elfring
 wrote:
>> I have applied first two,
>
> Thanks for another change acceptance.
>
>
>> the last one is subject to discuss a necessity of it.
>
> I can offer another bit of information for this software development 
> discussion.
>
> The following build settings were active in my “Makefile” for this Linux test 
> case.
>
> …
> HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O0 
> -fomit-frame-pointer -std=gnu89
> …
>
>
> The affected source file can be compiled for the processor architecture 
> “x86_64”
> by a tool like “GCC 6.4.1+r251631-1.3” from the software distribution
> “openSUSE Tumbleweed” with the following command example.
>
> my_cc=/usr/bin/gcc-6 \
> && my_module=drivers/platform/x86/sony-laptop.ko \
> && git checkout ':/^Sony-laptop: Delete an unnecessary variable 
> initialisation in sony_nc_setup_rfkill' \
> && make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
> && size "${my_module}" \
> && git checkout ':/^Sony-laptop: Use common error handling code in 
> sony_nc_setup_rfkill' \
> && make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
> && size "${my_module}"
>
>
> Do you find the following details useful for further clarification?
>
> text: -32
> data: 0
> bss:  0

...but kernel is compiled with -O2 which, I suppose, will eliminate
these repeats.

So, the main question is "WHY" you are doing this change.

I didn't find any convinced explanation (yet?).

As an example, I would understand it if the consequent patch will
bring locking to the function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()

[Andy Shevchenko]

On Fri, Nov 3, 2017 at 3:23 PM, SF Markus Elfring
 wrote:
>> I have applied first two,
>
> Thanks for another change acceptance.
>
>
>> the last one is subject to discuss a necessity of it.
>
> I can offer another bit of information for this software development 
> discussion.
>
> The following build settings were active in my “Makefile” for this Linux test 
> case.
>
> …
> HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O0 
> -fomit-frame-pointer -std=gnu89
> …
>
>
> The affected source file can be compiled for the processor architecture 
> “x86_64”
> by a tool like “GCC 6.4.1+r251631-1.3” from the software distribution
> “openSUSE Tumbleweed” with the following command example.
>
> my_cc=/usr/bin/gcc-6 \
> && my_module=drivers/platform/x86/sony-laptop.ko \
> && git checkout ':/^Sony-laptop: Delete an unnecessary variable 
> initialisation in sony_nc_setup_rfkill' \
> && make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
> && size "${my_module}" \
> && git checkout ':/^Sony-laptop: Use common error handling code in 
> sony_nc_setup_rfkill' \
> && make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
> && size "${my_module}"
>
>
> Do you find the following details useful for further clarification?
>
> text: -32
> data: 0
> bss:  0

...but kernel is compiled with -O2 which, I suppose, will eliminate
these repeats.

So, the main question is "WHY" you are doing this change.

I didn't find any convinced explanation (yet?).

As an example, I would understand it if the consequent patch will
bring locking to the function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()

[SF Markus Elfring]

> I have applied first two,

Thanks for another change acceptance.


> the last one is subject to discuss a necessity of it.

I can offer another bit of information for this software development discussion.

The following build settings were active in my “Makefile” for this Linux test 
case.

…
HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O0 
-fomit-frame-pointer -std=gnu89
…


The affected source file can be compiled for the processor architecture “x86_64”
by a tool like “GCC 6.4.1+r251631-1.3” from the software distribution
“openSUSE Tumbleweed” with the following command example.

my_cc=/usr/bin/gcc-6 \
&& my_module=drivers/platform/x86/sony-laptop.ko \
&& git checkout ':/^Sony-laptop: Delete an unnecessary variable initialisation 
in sony_nc_setup_rfkill' \
&& make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
&& size "${my_module}" \
&& git checkout ':/^Sony-laptop: Use common error handling code in 
sony_nc_setup_rfkill' \
&& make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
&& size "${my_module}"


Do you find the following details useful for further clarification?

text: -32
data: 0
bss:  0

Regards,
Markus

Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()

[SF Markus Elfring]

> I have applied first two,

Thanks for another change acceptance.


> the last one is subject to discuss a necessity of it.

I can offer another bit of information for this software development discussion.

The following build settings were active in my “Makefile” for this Linux test 
case.

…
HOSTCFLAGS   = -Wall -Wmissing-prototypes -Wstrict-prototypes -O0 
-fomit-frame-pointer -std=gnu89
…


The affected source file can be compiled for the processor architecture “x86_64”
by a tool like “GCC 6.4.1+r251631-1.3” from the software distribution
“openSUSE Tumbleweed” with the following command example.

my_cc=/usr/bin/gcc-6 \
&& my_module=drivers/platform/x86/sony-laptop.ko \
&& git checkout ':/^Sony-laptop: Delete an unnecessary variable initialisation 
in sony_nc_setup_rfkill' \
&& make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
&& size "${my_module}" \
&& git checkout ':/^Sony-laptop: Use common error handling code in 
sony_nc_setup_rfkill' \
&& make -j4 CC="${my_cc}" HOSTCC="${my_cc}" allmodconfig "${my_module}" \
&& size "${my_module}"


Do you find the following details useful for further clarification?

text: -32
data: 0
bss:  0

Regards,
Markus

Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()

[Andy Shevchenko]

On Wed, Nov 1, 2017 at 8:45 PM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Wed, 1 Nov 2017 19:34:56 +0100
>
> Three update suggestions were taken into account
> from static source code analysis.

I have applied first two, the last one is subject to discuss a necessity of it.

So, if Darren on my side than we won't apply it, if he opposes, I
would hear an argument why we might need it.

>
> Markus Elfring (3):
>   Fix exception handling
>   Delete an unnecessary variable initialisation
>   Use common error handling code
> ---
>
> v2:
> Two additional suggestions were taken into account from a corresponding
> source code review.
>
>  drivers/platform/x86/sony-laptop.c | 33 ++---
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] Sony-laptop: Adjustments for sony_nc_setup_rfkill()

[Andy Shevchenko]

On Wed, Nov 1, 2017 at 8:45 PM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Wed, 1 Nov 2017 19:34:56 +0100
>
> Three update suggestions were taken into account
> from static source code analysis.

I have applied first two, the last one is subject to discuss a necessity of it.

So, if Darren on my side than we won't apply it, if he opposes, I
would hear an argument why we might need it.

>
> Markus Elfring (3):
>   Fix exception handling
>   Delete an unnecessary variable initialisation
>   Use common error handling code
> ---
>
> v2:
> Two additional suggestions were taken into account from a corresponding
> source code review.
>
>  drivers/platform/x86/sony-laptop.c | 33 ++---
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko