linux-kernel - Re: [PATCH v1] net: ipa: make use of dev_err_cast

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: <1bf791fc-0990-4f7a-9879-f0677bc1628f@huawei.com>
Date: Thu, 29 Aug 2024 09:47:55 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Alex Elder <elder@...e.org>, Simon Horman <horms@...nel.org>, Yuesong Li
	<liyuesong@...o.com>
CC: <elder@...nel.org>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <opensource.kernel@...o.com>
Subject: Re: [PATCH v1] net: ipa: make use of dev_err_cast_probe()



On 2024/8/29 9:27, Alex Elder wrote:
> On 8/28/24 11:07 AM, Simon Horman wrote:
>> On Wed, Aug 28, 2024 at 04:41:15PM +0800, Yuesong Li wrote:
>>> Using dev_err_cast_probe() to simplify the code.
>>>
>>> Signed-off-by: Yuesong Li <liyuesong@...o.com>
>>> ---
>>>   drivers/net/ipa/ipa_power.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c
>>> index 65fd14da0f86..248bcc0b661e 100644
>>> --- a/drivers/net/ipa/ipa_power.c
>>> +++ b/drivers/net/ipa/ipa_power.c
>>> @@ -243,9 +243,8 @@ ipa_power_init(struct device *dev, const struct 
>>> ipa_power_data *data)
>>>       clk = clk_get(dev, "core");
>>>       if (IS_ERR(clk)) {
>>> -        dev_err_probe(dev, PTR_ERR(clk), "error getting core clock\n");
>>> -
>>> -        return ERR_CAST(clk);
>>> +        return dev_err_cast_probe(dev, clk,
>>> +                "error getting core clock\n");
>>>       }
>>>       ret = clk_set_rate(clk, data->core_clock_rate);
>>
>> Hi,
>>
>> There are lot of clean-up patches floating around at this time.
>> And I'm unsure if you are both on the same team or not, but in
>> any case it would be nice if there was some co-ordination.
>> Because here we have two different versions of the same patch.
>> Which, from a maintainer and reviewer pov is awkward.
I apologize for causing confusion, and I will strive to submit patches 
that are truly valuable in the future.

Thanks,
Hongbo

> 
> I just noticed this (looking at the patch from Hongbo Li).
> 
>> In principle the change(s) look(s) fine to me. But there are some minor
>> problems.
>>
>> 1. For the patch above, it should be explicitly targeted at net-next.
>>     (Or net if it was a bug fix, which it is not.)
>>
>>     Not a huge problem, as this is the default. But please keep this 
>> in mind
>>     for future posts.
>>
>>     Subject: [PATCH vX net-next]: ...
>>
>> 2. For the patch above, the {} should be dropped, as in the patch below.
> 
> Agreed.
> 
>> 3. For both patches. The dev_err_cast_probe should be line wrapped,
>>     and the indentation should align with the opening (.
>>
>>         return dev_err_cast_probe(dev, clk,
>>                       "error getting core clock\n");
>>
>> I'd like to ask you to please negotiate amongst yourselves and
>> post _just one_ v2 which addresses the minor problems highlighted above.
> 
> Thank you Simon, you are correct and I appreciate your proposed
> solution.  I'll await a followup patch (perhaps jointly signed
> off?)
> 
>                      -Alex
> 
>> Thanks!
>>
>> On Wed, Aug 28, 2024 at 08:15:51PM +0800, Hongbo Li wrote:
>>> Using dev_err_cast_probe() to simplify the code.
>>>
>>> Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
>>> ---
>>>   drivers/net/ipa/ipa_power.c | 7 ++-----
>>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c
>>> index 65fd14da0f86..c572da9e9bc4 100644
>>> --- a/drivers/net/ipa/ipa_power.c
>>> +++ b/drivers/net/ipa/ipa_power.c
>>> @@ -242,11 +242,8 @@ ipa_power_init(struct device *dev, const struct 
>>> ipa_power_data *data)
>>>       int ret;
>>>       clk = clk_get(dev, "core");
>>> -    if (IS_ERR(clk)) {
>>> -        dev_err_probe(dev, PTR_ERR(clk), "error getting core clock\n");
>>> -
>>> -        return ERR_CAST(clk);
>>> -    }
>>> +    if (IS_ERR(clk))
>>> +        return dev_err_cast_probe(dev, clk, "error getting core 
>>> clock\n");
>>>       ret = clk_set_rate(clk, data->core_clock_rate);
>>>       if (ret) {
>>
> 
>