Hi,
On 8/29/24 6:50 PM, Andy Shevchenko wrote:
> First of all, it's a bit counterintuitive to have something like
>
> int err;
> ...
> scoped_guard(...)
> err = foo(...);
> if (err)
> return err;
>
> Second, with a particular kernel configuration and compiler version in
> one of such cases the objtool is not happy:
>
> ideapad-laptop.o: warning: objtool: .text.fan_mode_show: unexpected end of section
>
> I'm not an expert on all this, but the theory is that compiler and
> linker in this case can't understand that 'result' variable will be
> always initialized as long as no error has been returned. Assigning
> 'result' to a dummy value helps with this. Note, that fixing the
> scoped_guard() scope (as per above) does not make issue gone.
>
> That said, assign dummy value and make the scope_guard() clear of its scope.
> For the sake of consistency do it in the entire file.
>
> Fixes: 7cc06e729460 ("platform/x86: ideapad-laptop: add a mutex to synchronize VPC commands")
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Closes: https://lore.kernel.org/oe-kbuild-all/202408290219.BrPO8twi-lkp@xxxxxxxxx/
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
>
> This has been Cc'ed to objtool and clang maintainers to have a look and
> tell if this can be addressed in a better way.
>
> drivers/platform/x86/ideapad-laptop.c | 48 +++++++++++++++------------
> 1 file changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 35c75bcff195..c64dfc56651d 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -554,13 +554,14 @@ static ssize_t camera_power_show(struct device *dev,
> char *buf)
> {
> struct ideapad_private *priv = dev_get_drvdata(dev);
> - unsigned long result;
> + unsigned long result = 0;
> int err;
>
> - scoped_guard(mutex, &priv->vpc_mutex)
> + scoped_guard(mutex, &priv->vpc_mutex) {
> err = read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &result);
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> return sysfs_emit(buf, "%d\n", !!result);
> }
> @@ -577,10 +578,11 @@ static ssize_t camera_power_store(struct device *dev,
> if (err)
> return err;
>
> - scoped_guard(mutex, &priv->vpc_mutex)
> + scoped_guard(mutex, &priv->vpc_mutex) {
> err = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> return count;
> }
> @@ -628,13 +630,14 @@ static ssize_t fan_mode_show(struct device *dev,
> char *buf)
> {
> struct ideapad_private *priv = dev_get_drvdata(dev);
> - unsigned long result;
> + unsigned long result = 0;
> int err;
>
> - scoped_guard(mutex, &priv->vpc_mutex)
> + scoped_guard(mutex, &priv->vpc_mutex) {
> err = read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result);
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> return sysfs_emit(buf, "%lu\n", result);
> }
> @@ -654,10 +657,11 @@ static ssize_t fan_mode_store(struct device *dev,
> if (state > 4 || state == 3)
> return -EINVAL;
>
> - scoped_guard(mutex, &priv->vpc_mutex)
> + scoped_guard(mutex, &priv->vpc_mutex) {
> err = write_ec_cmd(priv->adev->handle, VPCCMD_W_FAN, state);
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> return count;
> }
> @@ -737,13 +741,14 @@ static ssize_t touchpad_show(struct device *dev,
> char *buf)
> {
> struct ideapad_private *priv = dev_get_drvdata(dev);
> - unsigned long result;
> + unsigned long result = 0;
> int err;
>
> - scoped_guard(mutex, &priv->vpc_mutex)
> + scoped_guard(mutex, &priv->vpc_mutex) {
> err = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result);
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> priv->r_touchpad_val = result;
>
> @@ -762,10 +767,11 @@ static ssize_t touchpad_store(struct device *dev,
> if (err)
> return err;
>
> - scoped_guard(mutex, &priv->vpc_mutex)
> + scoped_guard(mutex, &priv->vpc_mutex) {
> err = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
> - if (err)
> - return err;
> + if (err)
> + return err;
> + }
>
> priv->r_touchpad_val = state;
>
