[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5622e611-ce5d-4d0b-852f-759616f9452c@ieee.org>
Date: Wed, 28 Aug 2024 20:27:52 -0500
From: Alex Elder <elder@...e.org>
To: Simon Horman <horms@...nel.org>, Yuesong Li <liyuesong@...o.com>,
Hongbo Li <lihongbo22@...wei.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 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 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) {
>
Powered by blists - more mailing lists