Hi Utsav,
On Fri, Jun 21, 2024 at 10:44:12AM +0000, Agarwal, Utsav wrote:
> From: UtsavAgarwalADI <utsav.agarwal@xxxxxxxxxx>
>
> We have a SoC which uses ADP5587 exclusively as an I2C GPIO expander.
> The current state of the driver for the ADP5588/87 only allows
> partial I/O to be used as GPIO. This support was present before as a
> separate gpio driver, which was dropped with the commit
> 5ddc896088b0 ("gpio: gpio-adp5588: drop the driver") since the
> functionality was deemed to have been merged with adp5588-keys.
>
> To restore this functionality, the "gpio-only" property allows
> indicating if the device is to be used for GPIO only.
> When specified, the driver skips relevant input device
> checks/parsing and allows all GPINS to be registered as GPIO.
>
> Signed-off-by: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx>
> ---
> drivers/input/keyboard/adp5588-keys.c | 30 ++++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
> index 1b0279393df4..78770b2dfe1b 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -719,7 +719,7 @@ static int adp5588_probe(struct i2c_client *client)
> struct input_dev *input;
> struct gpio_desc *gpio;
> unsigned int revid;
> - int ret;
> + int ret, gpio_mode_only;
> int error;
>
> if (!i2c_check_functionality(client->adapter,
> @@ -739,13 +739,17 @@ static int adp5588_probe(struct i2c_client *client)
> kpad->client = client;
> kpad->input = input;
>
> - error = adp5588_fw_parse(kpad);
> - if (error)
> - return error;
> + gpio_mode_only = device_property_present(&client->dev, "gpio-only");
Do we really need a new property? Can we simply allow omitting
keypad,num-rows/cols properties in case where we only want to have
GPIO functionality?
In any case this requires DT binding update.
> + if (!gpio_mode_only) {
> + error = adp5588_fw_parse(kpad);
> + if (error)
> + return error;
>
> - error = devm_regulator_get_enable(&client->dev, "vcc");
> - if (error)
> - return error;
> + error = devm_regulator_get_enable(&client->dev, "vcc");
> + if (error)
> + return error;
Why regulator is not needed for the pure GPIO mode? Please add a
comment.
> +
> + }
>
> gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
> if (IS_ERR(gpio))
> @@ -790,6 +794,11 @@ static int adp5588_probe(struct i2c_client *client)
> if (error)
> return error;
>
> + if (!client->irq && gpio_mode_only) {
> + dev_info(&client->dev, "Rev.%d, started as GPIO only\n", revid);
> + return 0;
> + }
> +
What is the reason for requesting interrupt in pure GPIO mode? Can we
program the controller to not raise attention in this case?
> error = devm_request_threaded_irq(&client->dev, client->irq,
> adp5588_hard_irq, adp5588_thread_irq,
> IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> @@ -800,6 +809,13 @@ static int adp5588_probe(struct i2c_client *client)
> return error;
> }
>
> +
> + if (gpio_mode_only) {
> + dev_info(&client->dev, "Rev.%d irq %d, started as GPIO only\n",
> + revid, client->irq);
> + return 0;
> + }
> +
> dev_info(&client->dev, "Rev.%d keypad, irq %d\n", revid, client->irq);
> return 0;
> }
> --
> 2.34.1
Thanks.
--
Dmitry
