| Message ID | 20240704-adp5588_gpio_support-v6-2-cb65514d714b@analog.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | adp5588-keys: Support for dedicated gpio operation | expand |
On Thu, 2024-07-04 at 11:45 +0100, Utsav Agarwal via B4 Relay wrote: > From: Utsav Agarwal <utsav.agarwal@analog.com> > > Keypad specific setup is relaxed if no keypad rows/columns are specified, > enabling a purely gpio operation. > > Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com> > --- > drivers/input/keyboard/adp5588-keys.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/input/keyboard/adp5588-keys.c > b/drivers/input/keyboard/adp5588-keys.c > index 09bcfc6b9408..572335c8ef0f 100644 > --- a/drivers/input/keyboard/adp5588-keys.c > +++ b/drivers/input/keyboard/adp5588-keys.c > @@ -188,6 +188,7 @@ struct adp5588_kpad { > u32 cols; > u32 unlock_keys[2]; > int nkeys_unlock; > + bool gpio_only; > unsigned short keycode[ADP5588_KEYMAPSIZE]; > unsigned char gpiomap[ADP5588_MAXGPIO]; > struct gpio_chip gc; > @@ -632,6 +633,21 @@ static int adp5588_fw_parse(struct adp5588_kpad *kpad) > struct i2c_client *client = kpad->client; > int ret, i; > > + /* > + * Check if the device is to be operated purely in GPIO mode. To do > + * so, check that no keypad rows or columns have been specified, > + * since all GPINS should be configured as GPIO. > + */ > + ret = device_property_present(&client->dev, > + "keypad,num-rows"); > + ret |= device_property_present(&client->dev, > + "keypad,num-columns"); > + /* If purely GPIO, skip keypad setup */ > + if (!ret) { > + kpad->gpio_only = true; > + return 0; > + } > + > ret = matrix_keypad_parse_properties(&client->dev, &kpad->rows, > &kpad->cols); > if (ret) > @@ -775,6 +791,11 @@ static int adp5588_probe(struct i2c_client *client) > if (error) > return error; > > + if (kpad->gpio_only && !client->irq) { > + dev_info(&client->dev, "Rev.%d, started as GPIO only\n", > revid); > + return 0; > + } You're still missing something regarding interrupts. If there's no client->irq then you should not setup the gpio_irq_chip. Hence, in adp5588_gpio_add(), you should have: if (kpad->client->irq) { girq = &kpad->gc.irq; gpio_irq_chip_set_chip(girq, &adp5588_irq_chip); girq->handler = handle_bad_irq; girq->threaded = true; } - Nuno Sá
Hi Nuno, > -----Original Message----- > From: Nuno Sá <noname.nuno@gmail.com> > Sent: Thursday, July 4, 2024 12:10 PM > To: Agarwal, Utsav <Utsav.Agarwal@analog.com>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Dmitry Torokhov > <dmitry.torokhov@gmail.com>; Rob Herring <robh@kernel.org>; Krzysztof > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Sa, > Nuno <Nuno.Sa@analog.com> > Cc: linux-input@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Artamonovs, Arturs > <Arturs.Artamonovs@analog.com>; Bimpikas, Vasileios > <Vasileios.Bimpikas@analog.com>; Gaskell, Oliver > <Oliver.Gaskell@analog.com> > Subject: Re: [PATCH v6 2/3] Input: adp5588-keys - add support for pure gpio > > [External] > > On Thu, 2024-07-04 at 11:45 +0100, Utsav Agarwal via B4 Relay wrote: > > From: Utsav Agarwal <utsav.agarwal@analog.com> > > > > Keypad specific setup is relaxed if no keypad rows/columns are specified, > > enabling a purely gpio operation. > > > > Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com> > > --- > > drivers/input/keyboard/adp5588-keys.c | 27 > +++++++++++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > > > diff --git a/drivers/input/keyboard/adp5588-keys.c > > b/drivers/input/keyboard/adp5588-keys.c > > index 09bcfc6b9408..572335c8ef0f 100644 > > --- a/drivers/input/keyboard/adp5588-keys.c > > +++ b/drivers/input/keyboard/adp5588-keys.c > > @@ -188,6 +188,7 @@ struct adp5588_kpad { > > u32 cols; > > u32 unlock_keys[2]; > > int nkeys_unlock; > > + bool gpio_only; > > unsigned short keycode[ADP5588_KEYMAPSIZE]; > > unsigned char gpiomap[ADP5588_MAXGPIO]; > > struct gpio_chip gc; > > @@ -632,6 +633,21 @@ static int adp5588_fw_parse(struct adp5588_kpad > *kpad) > > struct i2c_client *client = kpad->client; > > int ret, i; > > > > + /* > > + * Check if the device is to be operated purely in GPIO mode. To do > > + * so, check that no keypad rows or columns have been specified, > > + * since all GPINS should be configured as GPIO. > > + */ > > + ret = device_property_present(&client->dev, > > + "keypad,num-rows"); > > + ret |= device_property_present(&client->dev, > > + "keypad,num-columns"); > > + /* If purely GPIO, skip keypad setup */ > > + if (!ret) { > > + kpad->gpio_only = true; > > + return 0; > > + } > > + > > ret = matrix_keypad_parse_properties(&client->dev, &kpad->rows, > > &kpad->cols); > > if (ret) > > @@ -775,6 +791,11 @@ static int adp5588_probe(struct i2c_client *client) > > if (error) > > return error; > > > > + if (kpad->gpio_only && !client->irq) { > > + dev_info(&client->dev, "Rev.%d, started as GPIO only\n", > > revid); > > + return 0; > > + } > > You're still missing something regarding interrupts. If there's no client->irq > then you should not setup the gpio_irq_chip. Hence, in adp5588_gpio_add(), > you > should have: > > if (kpad->client->irq) { > girq = &kpad->gc.irq; > gpio_irq_chip_set_chip(girq, &adp5588_irq_chip); > girq->handler = handle_bad_irq; > girq->threaded = true; > } > > - Nuno Sá Thank you for pointing this out, I will add the same. Thanks, Utsav
diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c index 09bcfc6b9408..572335c8ef0f 100644 --- a/drivers/input/keyboard/adp5588-keys.c +++ b/drivers/input/keyboard/adp5588-keys.c @@ -188,6 +188,7 @@ struct adp5588_kpad { u32 cols; u32 unlock_keys[2]; int nkeys_unlock; + bool gpio_only; unsigned short keycode[ADP5588_KEYMAPSIZE]; unsigned char gpiomap[ADP5588_MAXGPIO]; struct gpio_chip gc; @@ -632,6 +633,21 @@ static int adp5588_fw_parse(struct adp5588_kpad *kpad) struct i2c_client *client = kpad->client; int ret, i; + /* + * Check if the device is to be operated purely in GPIO mode. To do + * so, check that no keypad rows or columns have been specified, + * since all GPINS should be configured as GPIO. + */ + ret = device_property_present(&client->dev, + "keypad,num-rows"); + ret |= device_property_present(&client->dev, + "keypad,num-columns"); + /* If purely GPIO, skip keypad setup */ + if (!ret) { + kpad->gpio_only = true; + return 0; + } + ret = matrix_keypad_parse_properties(&client->dev, &kpad->rows, &kpad->cols); if (ret) @@ -775,6 +791,11 @@ static int adp5588_probe(struct i2c_client *client) if (error) return error; + if (kpad->gpio_only && !client->irq) { + dev_info(&client->dev, "Rev.%d, started as GPIO only\n", revid); + return 0; + } + error = devm_request_threaded_irq(&client->dev, client->irq, adp5588_hard_irq, adp5588_thread_irq, IRQF_TRIGGER_FALLING | IRQF_ONESHOT, @@ -785,6 +806,12 @@ static int adp5588_probe(struct i2c_client *client) return error; } + if (kpad->gpio_only) { + dev_info(&client->dev, "Rev.%d GPIO only, irq %d\n", + revid, client->irq); + return 0; + } + dev_info(&client->dev, "Rev.%d keypad, irq %d\n", revid, client->irq); return 0; }