linux-kernel - Re: [PATCH v2 5/6] usb: dwc3: exynos: Use devm_regulator_bulk_get

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]

Date: Thu, 4 Apr 2024 10:04:49 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Anand Moon <linux.amoon@...il.com>
Cc: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Alim Akhtar <alim.akhtar@...sung.com>,
 Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 Johan Hovold <johan@...nel.org>, linux-usb@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/6] usb: dwc3: exynos: Use
 devm_regulator_bulk_get_enable() helper function

On 04/04/2024 09:38, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Thu, 4 Apr 2024 at 12:53, Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.org> wrote:
>>
>> On 04/04/2024 09:13, Anand Moon wrote:
>>> Use devm_regulator_bulk_get_enable() instead of open coded
>>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
>>
>> I fail to see how did you replace open-coded suspend/resume paths.
>>
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@...il.com>
>>> ---
>>> V2: no changes, did not find any regression in pm suspend/resume.
>>
>> No, that's not equivalent code. No explanation in commit msg.
>>
>> You already got comments on this and nothing improved. You just entirely
>> ignored received comments. That's not how it works.
>>
>> I don't think you understand the code and Linux driver model. This patch
>> repeats several previous attempts with similar issues: no logic behind a
>> change.
>>
>> NAK.
> 
> devm_regulator_get_enable and devm_regulator_bulk_get_enable
> both remove the dependency from the driver to handle the regulator_enabled
> and regulator_disabled. ie this removes the regulator from the driver structure.

Not true. Please do not paste some generic knowledge and assume reviewer
knows it. Instead provide proof.

> 
> Since these functions set devm_add_action to disable the regulator when the
> resource is not used.
> 
>      ret = devm_add_action(dev, devm_regulator_bulk_disable, devres);
>      if (!ret)
>                return 0;

Listen, you already got comments on this at v1. Address previous
comments instead of repeating something unrelated. We should not have
the same discussion twice.

Best regards,
Krzysztof