Re: [PATCH] platform/x86: x86-android-tablets: fix codec teardown if poweroff handler setup fails

[Hans de Goede]

Hi,

On 3-Nov-26 11:55 AM, Wentao Liang wrote:
> lenovo_yoga_tab2_830_1050_init_codec() stores the codec device reference,
> the pinctrl handle and the pin mappings in globals on success; they are
> only released by lenovo_yoga_tab2_830_1050_exit(). If the
> register_sys_off_handler() call which follows it fails, the init callback
> returns an error before the exit callback has been registered with the
> framework, so the codec device reference and the other resources are
> never released.
> 
> Factor the codec teardown out into a helper and call it from the error
> path of both init callbacks.
> 
> Fixes: 4ce8f4c2027d ("platform/x86: x86-android-tablets: Fix Lenovo Yoga Tablet 2 830/1050 poweroff again")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>
> ---
>  .../platform/x86/x86-android-tablets/lenovo.c | 27 +++++++++++++------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/x86-android-tablets/lenovo.c b/drivers/platform/x86/x86-android-tablets/lenovo.c
> index 52d96ae89078..294986f84ce8 100644
> --- a/drivers/platform/x86/x86-android-tablets/lenovo.c
> +++ b/drivers/platform/x86/x86-android-tablets/lenovo.c
> @@ -548,6 +548,14 @@ static int __init lenovo_yoga_tab2_830_1050_init_codec(void)
>  	return ret;
>  }
>  
> +static void lenovo_yoga_tab2_830_1050_exit_codec(void)
> +{
> +	device_remove_software_node(lenovo_yoga_tab2_830_1050_codec_dev);
> +	pinctrl_put(lenovo_yoga_tab2_830_1050_codec_pinctrl);
> +	pinctrl_unregister_mappings(&lenovo_yoga_tab2_830_1050_codec_pinctrl_map);
> +	put_device(lenovo_yoga_tab2_830_1050_codec_dev);
> +}
> +
>  /*
>   * These tablet's DSDT does not set acpi_gbl_reduced_hardware, so acpi_power_off()
>   * gets used as pm_power_off handler. This causes "poweroff" on these tablets
> @@ -578,8 +586,11 @@ static int __init lenovo_yoga_tab2_830_1050_init(struct device *dev)
>  	lenovo_yoga_tab2_830_1050_sys_off_handler =
>  		register_sys_off_handler(SYS_OFF_MODE_POWER_OFF, SYS_OFF_PRIO_FIRMWARE + 1,
>  					 lenovo_yoga_tab2_830_1050_power_off, NULL);
> -	if (IS_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler))
> -		return PTR_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler);
> +	if (IS_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler)) {
> +		ret = PTR_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler);
> +		lenovo_yoga_tab2_830_1050_exit_codec();
> +		return ret;
> +	}

lenovo_yoga_tab2_830_1050_exit_codec() does not touch lenovo_yoga_tab2_830_1050_sys_off_handler,
so there is no reason to store the error-code in ret before calling it.

Please just keep this line:

		return PTR_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler);

as-is.


>  
>  	return 0;
>  }
> @@ -588,10 +599,7 @@ static void lenovo_yoga_tab2_830_1050_exit(void)
>  {
>  	unregister_sys_off_handler(lenovo_yoga_tab2_830_1050_sys_off_handler);
>  
> -	device_remove_software_node(lenovo_yoga_tab2_830_1050_codec_dev);
> -	pinctrl_put(lenovo_yoga_tab2_830_1050_codec_pinctrl);
> -	pinctrl_unregister_mappings(&lenovo_yoga_tab2_830_1050_codec_pinctrl_map);
> -	put_device(lenovo_yoga_tab2_830_1050_codec_dev);
> +	lenovo_yoga_tab2_830_1050_exit_codec();

Please also remove the empty line which was there to group all the codec
cleanup calls together which now no longer is necessary.

>  }
>  
>  /*
> @@ -776,8 +784,11 @@ static int __init lenovo_yoga_tab2_1380_init(struct device *dev)
>  	lenovo_yoga_tab2_830_1050_sys_off_handler =
>  		register_sys_off_handler(SYS_OFF_MODE_POWER_OFF, SYS_OFF_PRIO_FIRMWARE + 1,
>  					 lenovo_yoga_tab2_830_1050_power_off, NULL);
> -	if (IS_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler))
> -		return PTR_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler);
> +	if (IS_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler)) {
> +		ret = PTR_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler);
> +		lenovo_yoga_tab2_830_1050_exit_codec();
> +		return ret;
> +	}

Same as above, please keep this line:

		return PTR_ERR(lenovo_yoga_tab2_830_1050_sys_off_handler);

as-is.

Regards,

Hans