Re: [PATCH] usb: typec: ucsi: acpi: Remove notifier before destroying handler

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Tue, Mar 21, 2023 at 03:01:36PM -0400, Mark Pearson wrote:
> Was debugging another issue (since fixed) and noticed that the acpi
> notify_handler should be removed before the ucsi object is destroyed.
> 
> This isn't fixing any issues that I'm aware of - but I assume could
> potentially lead to a race condition if you were really unlucky?
> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> ---
>  drivers/usb/typec/ucsi/ucsi_acpi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> index ce0c8ef80c04..be3bf4f996d3 100644
> --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> @@ -176,12 +176,12 @@ static int ucsi_acpi_remove(struct platform_device *pdev)
>  {
>  	struct ucsi_acpi *ua = platform_get_drvdata(pdev);
>  
> -	ucsi_unregister(ua->ucsi);
> -	ucsi_destroy(ua->ucsi);
> -
>  	acpi_remove_notify_handler(ACPI_HANDLE(&pdev->dev), ACPI_DEVICE_NOTIFY,
>  				   ucsi_acpi_notify);
>  
> +	ucsi_unregister(ua->ucsi);
> +	ucsi_destroy(ua->ucsi);
> +
>  	return 0;
>  }

Calling ucsi_desctroy() after removing the notifier makes sense to me,
but do you also need to unregister the instance after that?

You may still be in the middle of init or resume, so I think we need
to accept notifications until we are sure those have finished, i.e.
ucsi_unregister() has finished.

thanks,

-- 
heikki