Re: [PATCH v2 4/5] pinctrl: at91: Use dev_err_probe() instead of custom messaging
From: Claudiu.Beznea
Date: Tue Feb 14 2023 - 06:10:27 EST
On 13.02.2023 17:45, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The custom message has no value except printing the error code,
> the same does dev_err_probe(). Let's use the latter for the sake
> of unification.
>
> Note that some APIs already have messaging in them and some simply
> do not require the current noise.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Reviewed-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
Tested-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>
> ---
> drivers/pinctrl/pinctrl-at91.c | 64 +++++++++++-----------------------
> 1 file changed, 21 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 055a88b2dacc..08f88403affb 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1294,10 +1294,11 @@ static const struct of_device_id at91_pinctrl_of_match[] = {
> static int at91_pinctrl_probe_dt(struct platform_device *pdev,
> struct at91_pinctrl *info)
> {
> + struct device *dev = &pdev->dev;
> int ret = 0;
> int i, j, ngpio_chips_enabled = 0;
> uint32_t *tmp;
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = dev->of_node;
> struct device_node *child;
>
> if (!np)
> @@ -1360,9 +1361,8 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
> continue;
> ret = at91_pinctrl_parse_functions(child, info, i++);
> if (ret) {
> - dev_err(&pdev->dev, "failed to parse function\n");
> of_node_put(child);
> - return ret;
> + return dev_err_probe(dev, ret, "failed to parse function\n");
> }
> }
>
> @@ -1415,11 +1415,8 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, info);
> info->pctl = devm_pinctrl_register(&pdev->dev, &at91_pinctrl_desc,
> info);
> -
> - if (IS_ERR(info->pctl)) {
> - dev_err(&pdev->dev, "could not register AT91 pinctrl driver\n");
> - return PTR_ERR(info->pctl);
> - }
> + if (IS_ERR(info->pctl))
> + return dev_err_probe(dev, PTR_ERR(info->pctl), "could not register AT91 pinctrl driver\n");
>
> /* We will handle a range of GPIO pins */
> for (i = 0; i < gpio_banks; i++)
> @@ -1820,39 +1817,28 @@ static int at91_gpio_probe(struct platform_device *pdev)
> char **names;
>
> BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chips));
> - if (gpio_chips[alias_idx]) {
> - ret = -EBUSY;
> - goto err;
> - }
> + if (gpio_chips[alias_idx])
> + return dev_err_probe(dev, -EBUSY, "%d slot is occupied.\n", alias_idx);
>
> irq = platform_get_irq(pdev, 0);
> - if (irq < 0) {
> - ret = irq;
> - goto err;
> - }
> + if (irq < 0)
> + return irq;
>
> at91_chip = devm_kzalloc(&pdev->dev, sizeof(*at91_chip), GFP_KERNEL);
> - if (!at91_chip) {
> - ret = -ENOMEM;
> - goto err;
> - }
> + if (!at91_chip)
> + return -ENOMEM;
>
> at91_chip->regbase = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(at91_chip->regbase)) {
> - ret = PTR_ERR(at91_chip->regbase);
> - goto err;
> - }
> + if (IS_ERR(at91_chip->regbase))
> + return PTR_ERR(at91_chip->regbase);
>
> at91_chip->ops = of_device_get_match_data(dev);
> at91_chip->pioc_virq = irq;
> at91_chip->pioc_idx = alias_idx;
>
> at91_chip->clock = devm_clk_get_enabled(&pdev->dev, NULL);
> - if (IS_ERR(at91_chip->clock)) {
> - dev_err(&pdev->dev, "failed to get clock, ignoring.\n");
> - ret = PTR_ERR(at91_chip->clock);
> - goto err;
> - }
> + if (IS_ERR(at91_chip->clock))
> + return dev_err_probe(dev, PTR_ERR(at91_chip->clock), "failed to get clock, ignoring.\n");
>
> at91_chip->chip = at91_gpio_template;
> at91_chip->id = alias_idx;
> @@ -1865,17 +1851,15 @@ static int at91_gpio_probe(struct platform_device *pdev)
>
> if (!of_property_read_u32(np, "#gpio-lines", &ngpio)) {
> if (ngpio >= MAX_NB_GPIO_PER_BANK)
> - pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
> - alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
> + dev_err(dev, "at91_gpio.%d, gpio-nb >= %d failback to %d\n",
> + alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
> else
> chip->ngpio = ngpio;
> }
>
> names = devm_kasprintf_strarray(dev, "pio", chip->ngpio);
> - if (!names) {
> - ret = -ENOMEM;
> - goto err;
> - }
> + if (!names)
> + return -ENOMEM;
>
> for (i = 0; i < chip->ngpio; i++)
> strreplace(names[i], '-', alias_idx + 'A');
> @@ -1892,11 +1876,11 @@ static int at91_gpio_probe(struct platform_device *pdev)
>
> ret = at91_gpio_of_irq_setup(pdev, at91_chip);
> if (ret)
> - goto gpiochip_add_err;
> + return ret;
>
> ret = gpiochip_add_data(chip, at91_chip);
> if (ret)
> - goto gpiochip_add_err;
> + return ret;
>
> gpio_chips[alias_idx] = at91_chip;
> platform_set_drvdata(pdev, at91_chip);
> @@ -1905,12 +1889,6 @@ static int at91_gpio_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
>
> return 0;
> -
> -gpiochip_add_err:
> -err:
> - dev_err(&pdev->dev, "Failure %i for GPIO %i\n", ret, alias_idx);
> -
> - return ret;
> }
>
> static const struct dev_pm_ops at91_gpio_pm_ops = {
> --
> 2.39.1
>