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]
Date: Thu, 4 Apr 2024 15:54:08 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Anand Moon <linux.amoon@...il.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Alan Stern <stern@...land.harvard.edu>,
 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 1/6] usb: ehci-exynos: Use devm_clk_get_enabled()
 helpers

On 04/04/2024 15:52, Anand Moon wrote:
> Hi Greg,
> 
> On Thu, 4 Apr 2024 at 18:30, Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
>>
>> On Thu, Apr 04, 2024 at 12:43:17PM +0530, Anand Moon wrote:
>>> The devm_clk_get_enabled() helpers:
>>>     - call devm_clk_get()
>>>     - call clk_prepare_enable() and register what is needed in order to
>>>      call clk_disable_unprepare() when needed, as a managed resource.
>>>
>>> This simplifies the code and avoids the calls to clk_disable_unprepare().
>>>
>>> While at it, use dev_err_probe consistently, and use its return value
>>> to return the error code.
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@...il.com>
>>> ---
>>> V2: drop the clk_disable_unprepare in suspend/resume functions
>>>     fix the usb_put_hcd return before the devm_clk_get_enabled
>>> ---
>>>  drivers/usb/host/ehci-exynos.c | 19 +++++--------------
>>>  1 file changed, 5 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
>>> index f644b131cc0b..f00bfd0b13dc 100644
>>> --- a/drivers/usb/host/ehci-exynos.c
>>> +++ b/drivers/usb/host/ehci-exynos.c
>>> @@ -159,20 +159,15 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>>>
>>>       err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
>>>       if (err)
>>> -             goto fail_clk;
>>> -
>>> -     exynos_ehci->clk = devm_clk_get(&pdev->dev, "usbhost");
>>> +             goto fail_io;
>>>
>>> +     exynos_ehci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost");
>>>       if (IS_ERR(exynos_ehci->clk)) {
>>> -             dev_err(&pdev->dev, "Failed to get usbhost clock\n");
>>> -             err = PTR_ERR(exynos_ehci->clk);
>>> -             goto fail_clk;
>>> +             usb_put_hcd(hcd);
>>> +             return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ehci->clk),
>>> +                               "Failed to get usbhost clock\n");
>>
>> Why is this logic changed?
>>
>> If you want to call dev_err_probe(), that's great, but do NOT mix it up
>> with a commit that does something totally different.
>>
>> When you say something like "while at it" in a changelog text, that is a
>> HUGE hint that it needs to be a separate commit.  Because of that reason
>> alone, I can't take these, you know better :(
>>
>> thanks,
>>
> 
> Ok, I will improve the commit message relevant to the code changes.

Please read Greg's message one more time. He did not propose to fix
commit msg, right?

Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ