Re: [PATCH v3 2/3] ASoC: codecs: wsa883x: Add devm action to safely disable regulator on device removal
From: Mohammad Rafi Shaik <hidden>
Date: 2025-08-06 14:35:49
Also in:
linux-arm-msm, linux-gpio, linux-sound, lkml
On 7/28/2025 6:32 PM, Krzysztof Kozlowski wrote:
On 28/07/2025 14:36, Mohammad Rafi Shaik wrote:quoted
On 7/27/2025 3:00 PM, Krzysztof Kozlowski wrote:quoted
On 27/07/2025 10:31, Mohammad Rafi Shaik wrote:quoted
To prevent potential warnings from _regulator_put() during deviceWarning is either there or not. Either you fix real, specific issue or not. The code looks correct at first glance, so please describe exactly how these warnings happen or how what is the bug being fixed.The current wsa883x codec driver manually enables and disables regulators during probe and remove. In patch v3-0003, reset functionality was added using devm_reset_control_get_optional_shared_deasserted() for shared gpios.There is no such code at this point. Each patch is a separate commit and must stand on its own. With its own explanation. You cannot say that you add bugs later, so you need to fix something now. Describe actual problem here. If there is no problem here, describe why you are doing this.
Identified the actual root cause of the issue observed in the reset changes. The failure condition was not properly handled in the reset patch. I will update the patch to include error handling for failure scenarios and ensure regulators are disabled appropriately. will Drop this patch for next version, only will keep the reset changes. Thanks & Regards, Rafi.
quoted
However, during cleanup, this led to a warning: "WARNING: CPU: 2 PID: 195 at drivers/regulator/core.c:2450 _regulator_put+0x50/0x58" This occurs because the regulator is still enabled/released when the devm-managed cleanup path attempts to release it.So that patch was broken? You just did not properly clean up there?quoted
To resolve this, remove the manual regulator disable logic and instead register a devm-managed cleanup action using devm_add_action_or_reset(). This ensures proper cleanup and avoids regulator misuse warnings. For reference, the wsa884x codec driver already follows this approach by using devm actions for regulator management.quoted
quoted
removal, register a devm-managed cleanup action using devm_add_action_or_reset() to safely disable the regulator associated with the WSA883x codec, ensuring that the regulator is properly disabled when the device is removed, even if theDevice cannot be removed/unloaded, AFAIK, because of suppressed bind. Regulator is already disabled during error paths, so that part of above sentences is just misleading. How can one trigger the warnings?The warning in _regulator_put() can be triggered by applying patch v3-0003, which introduces reset functionality using devm_reset_control_get_optional_shared_deasserted().There is no such code now. You say "potential warnings" are here.quoted
Since the existing driver handles regulator enable/disable manually, the devm-managed reset cleanup path may attempt to release regulators that are still enabled, leading to the warning. This issue highlights the need to replace manual regulator handling with devm_add_action_or_reset() to ensure proper cleanup and avoid such warnings.quoted
quoted
probe fails or the driver is unloaded unexpectedly.How driver can be unloaded unexpectedly?"Unloaded" might not be the most accurate term here. What I meant is that the driver’s probe can fail due to an error—such as missing resources or improper regulator handling.Use standard Linux terms, e.g. probe failure, probe deferral etc.
Ack, will ensure all upcoming changes are managed effectively. Thanks & Regards, Rafi.
Best regards, Krzysztof