Re: [PATCH v2 2/2] spi: spi-geni-qcom: Fix missing undo runtime PM changes at driver exit time

[Jinjie Ruan <ruanjinjie@xxxxxxxxxx>]

On 2024/9/6 11:36, Dmitry Baryshkov wrote:
> On Fri, 6 Sept 2024 at 06:31, Jinjie Ruan <ruanjinjie@xxxxxxxxxx> wrote:
>>
>>
>>
>> On 2024/9/6 11:15, Dmitry Baryshkov wrote:
>>> On Fri, Sep 06, 2024 at 11:13:45AM GMT, Jinjie Ruan wrote:
>>>> It's important to undo pm_runtime_use_autosuspend() with
>>>> pm_runtime_dont_use_autosuspend() at driver exit time unless driver
>>>> initially enabled pm_runtime with devm_pm_runtime_enable()
>>>> (which handles it for you).
>>>>
>>>> Hence, call pm_runtime_dont_use_autosuspend() at driver exit time
>>>> to fix it.
>>>>
>>>> Fixes: cfdab2cd85ec ("spi: spi-geni-qcom: Set an autosuspend delay of 250 ms")
>>>> ---
>>>> v2:
>>>> - Fix it directly instead of use devm_pm_runtime_enable().
>>>
>>> Why?
>>
>> The devm* sequence will have some problem, which will not consistent
>> with the former.
>>
>> Link:
>> https://lore.kernel.org/all/CAD=FV=VyDk-e2KNiuiBcACFAdrQmihOH6X6BSpGB+T1MsgsiKw@xxxxxxxxxxxxxx/
> 
> That comment was for devm_request_irq(), not devm_pm_runtime_enable().


In the very least, ** parch #2 needs to come before this one and that
would help, but it won't fix everything **. Specifically in order to
keep the order proper you'll need to use devm_add_action_or_reset() to
"devm-ize" the freeing of the DMA channels.


> 
>>
>>>
>>>> ---
>>>>  drivers/spi/spi-geni-qcom.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
>>>> index fc2819effe2d..38857edbc785 100644
>>>> --- a/drivers/spi/spi-geni-qcom.c
>>>> +++ b/drivers/spi/spi-geni-qcom.c
>>>> @@ -1158,6 +1158,7 @@ static int spi_geni_probe(struct platform_device *pdev)
>>>>  spi_geni_release_dma:
>>>>      spi_geni_release_dma_chan(mas);
>>>>  spi_geni_probe_runtime_disable:
>>>> +    pm_runtime_dont_use_autosuspend(dev);
>>>>      pm_runtime_disable(dev);
>>>>      return ret;
>>>>  }
>>>> @@ -1174,6 +1175,7 @@ static void spi_geni_remove(struct platform_device *pdev)
>>>>
>>>>      spi_geni_release_dma_chan(mas);
>>>>
>>>> +    pm_runtime_dont_use_autosuspend(&pdev->dev);
>>>>      pm_runtime_disable(&pdev->dev);
>>>>  }
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>>
> 
> 
>