This patch is for handling a return error.
Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 1d18b32..19ad3ec 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1237,9 +1237,11 @@ static int tpacpi_rfk_hook_set_block(void *data, bool blocked)
/* try to set radio state */
res = (tp_rfk->ops->set_status)(blocked ?
TPACPI_RFK_RADIO_OFF : TPACPI_RFK_RADIO_ON);
+ if (res < 0)
+ return res;
/* and update the rfkill core with whatever the FW really did */
- tpacpi_rfk_update_swstate(tp_rfk);
+ res = tpacpi_rfk_update_swstate(tp_rfk);
return (res < 0) ? res : 0;
}
@@ -1371,7 +1373,10 @@ static ssize_t tpacpi_rfk_sysfs_enable_store(const enum tpacpi_rfk_id id,
res = tpacpi_rfkill_switches[id]->ops->set_status((!!t) ?
TPACPI_RFK_RADIO_ON : TPACPI_RFK_RADIO_OFF);
- tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[id]);
+ if (res < 0)
+ return res;
+
+ res = tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[id]);
return (res < 0) ? res : count;
}
@@ -1427,7 +1432,9 @@ static int tpacpi_rfk_procfs_write(const enum tpacpi_rfk_id id, char *buf)
"enable" : "disable",
tpacpi_rfkill_names[id]);
res = (tpacpi_rfkill_switches[id]->ops->set_status)(status);
- tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[id]);
+ if (res < 0)
+ return res;
+ res = tpacpi_rfk_update_swstate(tpacpi_rfkill_switches[id]);
}
return res;
@@ -3517,6 +3524,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
if (tp_features.hotkey_wlsw)
res = add_to_attr_set(hotkey_dev_attributes,
&dev_attr_hotkey_radio_sw.attr);
+ if (res)
+ goto err_exit;
res = hotkey_init_tablet_mode();
if (res < 0)
--
1.9.1
On Mon, Mar 6, 2017 at 9:43 AM, Arvind Yadav <[email protected]> wrote:
> This patch is for handling a return error.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 1d18b32..19ad3ec 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -1237,9 +1237,11 @@ static int tpacpi_rfk_hook_set_block(void *data, bool blocked)
> /* try to set radio state */
> res = (tp_rfk->ops->set_status)(blocked ?
> TPACPI_RFK_RADIO_OFF : TPACPI_RFK_RADIO_ON);
Just in case: first parens are redundant here.
> + if (res < 0)
> + return res;
So, this changes behaviour. Before we call _rfk_update_swstate()
independently on error code.
Care to explain in commit message how this change does / does not
affect overall user experience?
> /* and update the rfkill core with whatever the FW really did */
> - tpacpi_rfk_update_swstate(tp_rfk);
> + res = tpacpi_rfk_update_swstate(tp_rfk);
>
> return (res < 0) ? res : 0;
> }
> @@ -3517,6 +3524,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
> if (tp_features.hotkey_wlsw)
> res = add_to_attr_set(hotkey_dev_attributes,
> &dev_attr_hotkey_radio_sw.attr);
> + if (res)
> + goto err_exit;
>
> res = hotkey_init_tablet_mode();
> if (res < 0)
Ditto for hotkey_init_tablet_mode().
--
With Best Regards,
Andy Shevchenko
On Mon, Mar 06, 2017 at 10:45:50AM +0200, Andy Shevchenko wrote:
> On Mon, Mar 6, 2017 at 9:43 AM, Arvind Yadav <[email protected]> wrote:
> > This patch is for handling a return error.
> >
> > Signed-off-by: Arvind Yadav <[email protected]>
> > ---
> > drivers/platform/x86/thinkpad_acpi.c | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index 1d18b32..19ad3ec 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -1237,9 +1237,11 @@ static int tpacpi_rfk_hook_set_block(void *data, bool blocked)
> > /* try to set radio state */
>
> > res = (tp_rfk->ops->set_status)(blocked ?
> > TPACPI_RFK_RADIO_OFF : TPACPI_RFK_RADIO_ON);
>
> Just in case: first parens are redundant here.
>
> > + if (res < 0)
> > + return res;
>
> So, this changes behaviour. Before we call _rfk_update_swstate()
> independently on error code.
>
> Care to explain in commit message how this change does / does not
> affect overall user experience?
Arvind has made a number of similar such changes throughout the tree, I presume
with some automation. In this case, since there is no indication of a particular
manifestation of this bug on hardware, I will presume the same approach was
taken here. I'm dropping this patch as "changes requested" because at the very
least this needs a commit log which documents the problem manifested and why the
behavioral change is appropriate.
--
Darren Hart
VMware Open Source Technology Center