Re: [PATCH v4 3/4] mfd: aat2870: Convert to use OF bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mfd: aat2870: missing linux/gpio/consumer.h include breaks !CONFIG_GPIOLIB build
--

commit 7efa32d759fbc7ee0e1963e704db3b5316d972db
Author: Svyatoslav Ryhel <clamor95@gmail.com>

mfd: aat2870: Convert to use OF bindings

This commit converts the AAT2870 core, regulator, and backlight drivers from
platform data to Device Tree bindings and modern GPIO consumer helpers.

> diff --git a/drivers/mfd/aat2870-core.c b/drivers/mfd/aat2870-core.c
> index 0d56cd6fbc6a5..fe43ef675f257 100644
> --- a/drivers/mfd/aat2870-core.c
> +++ b/drivers/mfd/aat2870-core.c
> @@ -16,8 +16,15 @@
>  #include <linux/gpio/legacy.h>
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/aat2870.h>
> +#include <linux/of_platform.h>
>  #include <linux/regulator/machine.h>

[ ... ]

> @@ -348,60 +309,29 @@ static int aat2870_i2c_probe(struct i2c_client *client)
>  
>  	aat2870->reg_cache = aat2870_regs;
>  
> -	if (pdata->en_pin < 0)
> -		aat2870->en_pin = -1;
> -	else
> -		aat2870->en_pin = pdata->en_pin;
> +	aat2870->en_pin = devm_gpiod_get_optional(&client->dev, "enable",
> +						  GPIOD_OUT_HIGH);
> +	if (IS_ERR(aat2870->en_pin))
> +		return dev_err_probe(&client->dev, PTR_ERR(aat2870->en_pin),
> +				     "Failed to get EN GPIO\n");
>  
> -	aat2870->init = pdata->init;
> -	aat2870->uninit = pdata->uninit;
>  	aat2870->read = aat2870_read;
>  	aat2870->write = aat2870_write;
>  	aat2870->update = aat2870_update;
>  
>  	mutex_init(&aat2870->io_lock);
>  
> -	if (aat2870->init)
> -		aat2870->init(aat2870);
> -
> -	if (aat2870->en_pin >= 0) {
> -		ret = devm_gpio_request_one(&client->dev, aat2870->en_pin,
> -					GPIOF_OUT_INIT_HIGH, "aat2870-en");
> -		if (ret < 0) {
> -			dev_err(&client->dev,
> -				"Failed to request GPIO %d\n", aat2870->en_pin);
> -			return ret;
> -		}
> -	}
> -
> -	aat2870_enable(aat2870);
> -
> -	for (i = 0; i < pdata->num_subdevs; i++) {
> -		for (j = 0; j < ARRAY_SIZE(aat2870_devs); j++) {
> -			if ((pdata->subdevs[i].id == aat2870_devs[j].id) &&
> -					!strcmp(pdata->subdevs[i].name,
> -						aat2870_devs[j].name)) {
> -				aat2870_devs[j].platform_data =
> -					pdata->subdevs[i].platform_data;
> -				break;
> -			}
> -		}
> -	}
> +	gpiod_set_value_cansleep(aat2870->en_pin, 1);

[Severity: Low]
Should drivers/mfd/aat2870-core.c replace #include <linux/gpio/legacy.h>
with #include <linux/gpio/consumer.h>?

In drivers/mfd/aat2870-core.c, aat2870_i2c_probe(), aat2870_i2c_suspend(),
and aat2870_i2c_resume() now use devm_gpiod_get_optional(), GPIOD_OUT_HIGH,
and gpiod_set_value_cansleep(), which are declared in
<linux/gpio/consumer.h>.

However, <linux/gpio/legacy.h> only pulls in <linux/gpio/consumer.h> when
CONFIG_GPIOLIB is enabled:

include/linux/gpio/legacy.h:
    #ifdef CONFIG_GPIOLIB

    #include <linux/gpio/consumer.h>

Because MFD_AAT2870_CORE in drivers/mfd/Kconfig has:

    depends on GPIOLIB || COMPILE_TEST

building with CONFIG_MFD_AAT2870_CORE=y, CONFIG_COMPILE_TEST=y, and
CONFIG_GPIOLIB=n will fail to compile due to missing declarations for
GPIOD_OUT_HIGH, devm_gpiod_get_optional(), and gpiod_set_value_cansleep().

[ ... ]

>
> base-commit: 8c3ce6554a5b995599b84388347b301553e9efd5
> --

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019100930.110954-1-clamor95@gmail.com?part=3