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] [day] [month] [year] [list]
Message-ID: <20250204144457.00000248@huawei.com>
Date: Tue, 4 Feb 2025 14:44:57 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Claudiu Beznea <claudiu.beznea@...on.dev>
CC: <prabhakar.mahadev-lad.rj@...renesas.com>, <jic23@...nel.org>,
	<lars@...afoo.de>, <ulf.hansson@...aro.org>, <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()

On Tue, 4 Feb 2025 14:25:38 +0200
Claudiu Beznea <claudiu.beznea@...on.dev> wrote:

> 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?
> 

Quickest might be to propose a patch similar to the one for i2c that thread
references. Post that as an RFC and see if Greg KH or anyone else shoots it
down? Also verify it fixes what you see here of course!

It was on my list to do, but quite a few other things on that list so
if you have time that would be great.

Thanks,

Jonathan

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ