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-next>] [day] [month] [year] [list]
Message-ID: <20240828160728.GR1368797@kernel.org>
Date: Wed, 28 Aug 2024 17:07:28 +0100
From: Simon Horman <horms@...nel.org>
To: 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 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.

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.

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.

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) {

-- 
pw-bot: cr



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ