linux-kernel - Re: [PATCH v2 1/2] iio: adc: rzg2l_adc: Drop devm_pm_runtime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <427572c8-72b7-41fa-b065-fcb398a30779@tuxon.dev>
Date: Tue, 4 Feb 2025 14:25:38 +0200
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: prabhakar.mahadev-lad.rj@...renesas.com, jic23@...nel.org,
 lars@...afoo.de, ulf.hansson@...aro.org
Cc: linux-iio@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v2 1/2] iio: adc: rzg2l_adc: Drop devm_pm_runtime_enable()

Hi, Jonathan,

On 17.01.2025 13:45, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> 
> On all systems where the rzg2l_adc driver is used, the ADC clocks are part
> of a PM domain. The code that implements the PM domains support is in
> drivers/clk/renesas/rzg2l-cpg.c, the functions of interest for this commit
> being rzg2l_cpg_attach_dev() and rzg2l_cpg_deattach_dev(). The PM
> domains support is registered with GENPD_FLAG_PM_CLK which, according to
> the documentation, instructs genpd to use the PM clk framework while
> powering on/off attached devices.
> 
> During probe, the ADC device is attached to the PM domain
> controlling the ADC clocks. Similarly, during removal, the ADC device is
> detached from the PM domain.
> 
> The detachment call stack is as follows:
> 
> device_driver_detach() ->
>   device_release_driver_internal() ->
>     __device_release_driver() ->
>       device_remove() ->
>         platform_remove() ->
>           dev_pm_domain_detach()
> 
> During driver unbind, after the ADC device is detached from its PM domain,
> the device_unbind_cleanup() function is called, which subsequently invokes
> devres_release_all(). This function handles devres resource cleanup.
> 
> If runtime PM is enabled via devm_pm_runtime_enable(), the cleanup process
> triggers the action or reset function for disabling runtime PM. This
> function is pm_runtime_disable_action(), which leads to the following call
> stack of interest when called:
> 
> pm_runtime_disable_action() ->
>   pm_runtime_dont_use_autosuspend() ->
>     __pm_runtime_use_autosuspend() ->
>       update_autosuspend() ->
>         rpm_idle()
> 
> The rpm_idle() function attempts to runtime resume the ADC device. However,
> at the point it is called, the ADC device is no longer part of the PM
> domain (which manages the ADC clocks). Since the rzg2l_adc runtime PM
> APIs directly modifies hardware registers, the
> rzg2l_adc_pm_runtime_resume() function is invoked without the ADC clocks
> being enabled. This is because the PM domain no longer resumes along with
> the ADC device. As a result, this leads to system aborts.
> 
> Drop the devres API for runtime PM enable along with the other devres APIs
> after it (devm_request_irq(), devm_register_iio_device()).
> 
> Fixes: 89ee8174e8c8 ("iio: adc: rzg2l_adc: Simplify the runtime PM code")
> Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> ---

As of my understanding, currently there is is no conclusion from the
discussion at [1]. If it's not too early in the discussion, can you please
let me know how would you prefer to go forward for fixing this driver?

Thank you,
Claudiu

[1]
https://lore.kernel.org/all/20250103140042.1619703-2-claudiu.beznea.uj@bp.renesas.com/