[PATCH v2 1/4] extcon: adc-jack: Fix wakeup source leaks on device unbind

Krzysztof Kozlowski posted 4 patches 9 months, 2 weeks ago
[PATCH v2 1/4] extcon: adc-jack: Fix wakeup source leaks on device unbind
Posted by Krzysztof Kozlowski 9 months, 2 weeks ago
Device can be unbound, so driver must also release memory for the wakeup
source.  Do not use devm interface, because it would change the order of
cleanup.

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/extcon/extcon-adc-jack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index 46c40d85c2ac89599ffbe7b6d11b161b295d5564..557930394abd25771799733a22121d1f8e254918 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -164,6 +164,7 @@ static void adc_jack_remove(struct platform_device *pdev)
 {
 	struct adc_jack_data *data = platform_get_drvdata(pdev);
 
+	device_init_wakeup(&pdev->dev, false);
 	free_irq(data->irq, data);
 	cancel_work_sync(&data->handler.work);
 }

-- 
2.45.2
Re: [PATCH v2 1/4] extcon: adc-jack: Fix wakeup source leaks on device unbind
Posted by Christophe JAILLET 9 months ago
Le 01/05/2025 à 16:33, Krzysztof Kozlowski a écrit :
> Device can be unbound, so driver must also release memory for the wakeup
> source.  Do not use devm interface, because it would change the order of
> cleanup.
> 
> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/extcon/extcon-adc-jack.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
> index 46c40d85c2ac89599ffbe7b6d11b161b295d5564..557930394abd25771799733a22121d1f8e254918 100644
> --- a/drivers/extcon/extcon-adc-jack.c
> +++ b/drivers/extcon/extcon-adc-jack.c
> @@ -164,6 +164,7 @@ static void adc_jack_remove(struct platform_device *pdev)
>   {
>   	struct adc_jack_data *data = platform_get_drvdata(pdev);
>   
> +	device_init_wakeup(&pdev->dev, false);

Hi,

Shouldn't this be:

	if (data->wakeup_source)
		device_init_wakeup(&pdev->dev, false);

to match how things are done in the probe?

CJ

>   	free_irq(data->irq, data);
>   	cancel_work_sync(&data->handler.work);
>   }
> 

Re: [PATCH v2 1/4] extcon: adc-jack: Fix wakeup source leaks on device unbind
Posted by Krzysztof Kozlowski 9 months ago
On 09/05/2025 08:20, Christophe JAILLET wrote:
>>   drivers/extcon/extcon-adc-jack.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
>> index 46c40d85c2ac89599ffbe7b6d11b161b295d5564..557930394abd25771799733a22121d1f8e254918 100644
>> --- a/drivers/extcon/extcon-adc-jack.c
>> +++ b/drivers/extcon/extcon-adc-jack.c
>> @@ -164,6 +164,7 @@ static void adc_jack_remove(struct platform_device *pdev)
>>   {
>>   	struct adc_jack_data *data = platform_get_drvdata(pdev);
>>   
>> +	device_init_wakeup(&pdev->dev, false);
> 
> Hi,
> 
> Shouldn't this be:
> 
> 	if (data->wakeup_source)
> 		device_init_wakeup(&pdev->dev, false);
> 
> to match how things are done in the probe?

Yes, it should. I'll fix it.

Best regards,
Krzysztof
Re: [PATCH v2 1/4] extcon: adc-jack: Fix wakeup source leaks on device unbind
Posted by Christophe JAILLET 9 months ago
Le 09/05/2025 à 09:12, Krzysztof Kozlowski a écrit :
> On 09/05/2025 08:20, Christophe JAILLET wrote:
>>>    drivers/extcon/extcon-adc-jack.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
>>> index 46c40d85c2ac89599ffbe7b6d11b161b295d5564..557930394abd25771799733a22121d1f8e254918 100644
>>> --- a/drivers/extcon/extcon-adc-jack.c
>>> +++ b/drivers/extcon/extcon-adc-jack.c
>>> @@ -164,6 +164,7 @@ static void adc_jack_remove(struct platform_device *pdev)
>>>    {
>>>    	struct adc_jack_data *data = platform_get_drvdata(pdev);
>>>    
>>> +	device_init_wakeup(&pdev->dev, false);
>>
>> Hi,
>>
>> Shouldn't this be:
>>
>> 	if (data->wakeup_source)
>> 		device_init_wakeup(&pdev->dev, false);
>>
>> to match how things are done in the probe?
> 
> Yes, it should. I'll fix it.
> 
> Best regards,
> Krzysztof
> 
> 

While at it, maybe also s/1/true/ in device_init_wakeup() in the probe?

CJ