linux-kernel - Re: [PATCH v1] cirrus: Simplify with dev_err

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]

Message-ID: <82d147d8-8b97-48b1-8e2a-9479bff14f3c@opensource.cirrus.com>
Date: Wed, 28 Aug 2024 13:08:55 +0100
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: Yan Zhen <yanzhen@...o.com>, <ckeepax@...nsource.cirrus.com>,
        <tglx@...utronix.de>, <linus.walleij@...aro.org>
CC: <linux-sound@...r.kernel.org>, <patches@...nsource.cirrus.com>,
        <linux-kernel@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
        <opensource.kernel@...o.com>
Subject: Re: [PATCH v1] cirrus: Simplify with dev_err_probe()

On 28/08/2024 13:01, Yan Zhen wrote:
> Switch to use dev_err_probe() to simplify the error path and
> unify a message template.
> 
> Using this helper is totally fine even if err is known to never
> be -EPROBE_DEFER.
> 
> The benefit compared to a normal dev_err() is the standardized format
> of the error code, it being emitted symbolically and the fact that
> the error code is returned which allows more compact error paths.
> 
> Signed-off-by: Yan Zhen <yanzhen@...o.com>
> ---
>   drivers/irqchip/irq-madera.c                 | 5 ++---
>   drivers/pinctrl/cirrus/pinctrl-madera-core.c | 7 +++----

Split into separate patches for irqchip and pinctrl. There's no
dependency between these two changes so they don't need to be
applied in the same patch.

Also use the correct commit subject prefix for the subsystem and
driver that is being patched. Not "cirrus:".

>   2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-madera.c b/drivers/irqchip/irq-madera.c
> index acceb6e7fa95..68ebe09baaf6 100644
> --- a/drivers/irqchip/irq-madera.c
> +++ b/drivers/irqchip/irq-madera.c
> @@ -199,9 +199,8 @@ static int madera_irq_probe(struct platform_device *pdev)
>   		ret = regmap_update_bits(madera->regmap, MADERA_IRQ1_CTRL,
>   					 MADERA_IRQ_POL_MASK, 0);
>   		if (ret) {
> -			dev_err(&pdev->dev,
> -				"Failed to set IRQ polarity: %d\n", ret);
> -			return ret;
> +			return dev_err_probe(&pdev->dev, ret,
> +						"Failed to set IRQ polarity: %d\n", ret);

dev_err_probe already logs 'ret' so you don't need to log it again in
the message.

>   		}
>   	}
>   
> diff --git a/drivers/pinctrl/cirrus/pinctrl-madera-core.c b/drivers/pinctrl/cirrus/pinctrl-madera-core.c
> index 898b197c3738..532fa8ac9a94 100644
> --- a/drivers/pinctrl/cirrus/pinctrl-madera-core.c
> +++ b/drivers/pinctrl/cirrus/pinctrl-madera-core.c
> @@ -1064,10 +1064,9 @@ static int madera_pin_probe(struct platform_device *pdev)
>   		ret = pinctrl_register_mappings(pdata->gpio_configs,
>   						pdata->n_gpio_configs);
>   		if (ret) {
> -			dev_err(priv->dev,
> -				"Failed to register pdata mappings (%d)\n",
> -				ret);
> -			return ret;
> +			return dev_err_probe(priv->dev, ret,
> +						"Failed to register pdata mappings (%d)\n",
> +						ret);

Same here. No need to log 'ret' twice.

>   		}
>   	}
>