* [PATCH 01/13] Input: adp5589-keys: fix NULL pointer dereference
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 14:49 ` Dmitry Torokhov
2024-10-01 13:41 ` [PATCH 02/13] Input: adp5589-keys: fix adp5589_gpio_get_value() Nuno Sa
` (12 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
We register a devm action to call adp5589_clear_config() and then pass
the i2c client as argument so that we can call i2c_get_clientdata() in
order to get our device object. However, i2c_set_clientdata() is only
being set at the end of the probe function which means that we'll get a
NULL pointer dereference in case the probe function fails early.
Fixes: 30df385e35a4 ("Input: adp5589-keys - use devm_add_action_or_reset() for register clear")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 8996e00cd63a8203ec53d46ccb922c21ddb47355..68a29d67be57fc22088a912694e3e6e16e46d956 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -936,10 +936,9 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
static void adp5589_clear_config(void *data)
{
- struct i2c_client *client = data;
- struct adp5589_kpad *kpad = i2c_get_clientdata(client);
+ struct adp5589_kpad *kpad = data;
- adp5589_write(client, kpad->var->reg(ADP5589_GENERAL_CFG), 0);
+ adp5589_write(kpad->client, kpad->var->reg(ADP5589_GENERAL_CFG), 0);
}
static int adp5589_probe(struct i2c_client *client)
@@ -983,7 +982,7 @@ static int adp5589_probe(struct i2c_client *client)
}
error = devm_add_action_or_reset(&client->dev, adp5589_clear_config,
- client);
+ kpad);
if (error)
return error;
@@ -1010,8 +1009,6 @@ static int adp5589_probe(struct i2c_client *client)
if (error)
return error;
- i2c_set_clientdata(client, kpad);
-
dev_info(&client->dev, "Rev.%d keypad, irq %d\n", revid, client->irq);
return 0;
}
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 01/13] Input: adp5589-keys: fix NULL pointer dereference
2024-10-01 13:41 ` [PATCH 01/13] Input: adp5589-keys: fix NULL pointer dereference Nuno Sa
@ 2024-10-01 14:49 ` Dmitry Torokhov
0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2024-10-01 14:49 UTC (permalink / raw)
To: Nuno Sa
Cc: Mike Frysinger, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
On Tue, Oct 01, 2024 at 03:41:32PM +0200, Nuno Sa wrote:
> We register a devm action to call adp5589_clear_config() and then pass
> the i2c client as argument so that we can call i2c_get_clientdata() in
> order to get our device object. However, i2c_set_clientdata() is only
> being set at the end of the probe function which means that we'll get a
> NULL pointer dereference in case the probe function fails early.
>
> Fixes: 30df385e35a4 ("Input: adp5589-keys - use devm_add_action_or_reset() for register clear")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Applied and tagged for stable.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 02/13] Input: adp5589-keys: fix adp5589_gpio_get_value()
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
2024-10-01 13:41 ` [PATCH 01/13] Input: adp5589-keys: fix NULL pointer dereference Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 14:50 ` Dmitry Torokhov
2024-10-01 13:41 ` [PATCH 03/13] Input: adp5589-keys: add chip_info structure Nuno Sa
` (11 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
The adp5589 seems to have the same behavior as similar devices as
explained in
commit 910a9f5636f5 ("Input: adp5588-keys - get value from data out when
dir is out").
Basically, when the gpio is set as output we need to get the value from
ADP5589_GPO_DATA_OUT_A register instead of ADP5589_GPI_STATUS_A.
Fixes: 9d2e173644bb ("Input: ADP5589 - new driver for I2C Keypad Decoder and I/O Expander")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 68a29d67be57fc22088a912694e3e6e16e46d956..922d3ab998f3a5dfbaf277f10eb19e5cd1b35415 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -391,10 +391,17 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->var->bank(kpad->gpiomap[off]);
unsigned int bit = kpad->var->bit(kpad->gpiomap[off]);
+ int val;
- return !!(adp5589_read(kpad->client,
- kpad->var->reg(ADP5589_GPI_STATUS_A) + bank) &
- bit);
+ mutex_lock(&kpad->gpio_lock);
+ if (kpad->dir[bank] & bit)
+ val = kpad->dat_out[bank];
+ else
+ val = adp5589_read(kpad->client,
+ kpad->var->reg(ADP5589_GPI_STATUS_A) + bank);
+ mutex_unlock(&kpad->gpio_lock);
+
+ return !!(val & bit);
}
static void adp5589_gpio_set_value(struct gpio_chip *chip,
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 02/13] Input: adp5589-keys: fix adp5589_gpio_get_value()
2024-10-01 13:41 ` [PATCH 02/13] Input: adp5589-keys: fix adp5589_gpio_get_value() Nuno Sa
@ 2024-10-01 14:50 ` Dmitry Torokhov
0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2024-10-01 14:50 UTC (permalink / raw)
To: Nuno Sa
Cc: Mike Frysinger, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
On Tue, Oct 01, 2024 at 03:41:33PM +0200, Nuno Sa wrote:
> The adp5589 seems to have the same behavior as similar devices as
> explained in
>
> commit 910a9f5636f5 ("Input: adp5588-keys - get value from data out when
> dir is out").
>
> Basically, when the gpio is set as output we need to get the value from
> ADP5589_GPO_DATA_OUT_A register instead of ADP5589_GPI_STATUS_A.
>
> Fixes: 9d2e173644bb ("Input: ADP5589 - new driver for I2C Keypad Decoder and I/O Expander")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Applied and tagged for stable.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 03/13] Input: adp5589-keys: add chip_info structure
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
2024-10-01 13:41 ` [PATCH 01/13] Input: adp5589-keys: fix NULL pointer dereference Nuno Sa
2024-10-01 13:41 ` [PATCH 02/13] Input: adp5589-keys: fix adp5589_gpio_get_value() Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 14:55 ` Dmitry Torokhov
2024-10-01 13:41 ` [PATCH 04/13] Input: adp5589-keys: support gpi key events as 'gpio keys' Nuno Sa
` (10 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Add a more natural chip_info structure and add it to the i2c id table
driver data so that we do not need an enum a switch() to get the
specific bits of each device.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 181 ++++++++++++++++++----------------
1 file changed, 95 insertions(+), 86 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 922d3ab998f3a5dfbaf277f10eb19e5cd1b35415..eaa5440d4f9e14352409dd880cd254354612bf3e 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -228,16 +228,20 @@ struct adp_constants {
u8 (*reg) (u8 reg);
};
+struct adp5589_info {
+ const struct adp_constants *var;
+ bool support_row5;
+ bool is_adp5585;
+};
+
struct adp5589_kpad {
struct i2c_client *client;
struct input_dev *input;
- const struct adp_constants *var;
+ const struct adp5589_info *info;
unsigned short keycode[ADP5589_KEYMAPSIZE];
const struct adp5589_gpi_map *gpimap;
unsigned short gpimapsize;
unsigned extend_cfg;
- bool is_adp5585;
- bool support_row5;
#ifdef CONFIG_GPIOLIB
unsigned char gpiomap[ADP5589_MAXGPIO];
struct gpio_chip gc;
@@ -389,8 +393,8 @@ static int adp5589_write(struct i2c_client *client, u8 reg, u8 val)
static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
int val;
mutex_lock(&kpad->gpio_lock);
@@ -398,7 +402,7 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
val = kpad->dat_out[bank];
else
val = adp5589_read(kpad->client,
- kpad->var->reg(ADP5589_GPI_STATUS_A) + bank);
+ kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank);
mutex_unlock(&kpad->gpio_lock);
return !!(val & bit);
@@ -408,8 +412,8 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
unsigned off, int val)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
mutex_lock(&kpad->gpio_lock);
@@ -418,7 +422,7 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
else
kpad->dat_out[bank] &= ~bit;
- adp5589_write(kpad->client, kpad->var->reg(ADP5589_GPO_DATA_OUT_A) +
+ adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) +
bank, kpad->dat_out[bank]);
mutex_unlock(&kpad->gpio_lock);
@@ -427,15 +431,15 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
static int adp5589_gpio_direction_input(struct gpio_chip *chip, unsigned off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
int ret;
mutex_lock(&kpad->gpio_lock);
kpad->dir[bank] &= ~bit;
ret = adp5589_write(kpad->client,
- kpad->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
+ kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
kpad->dir[bank]);
mutex_unlock(&kpad->gpio_lock);
@@ -447,8 +451,8 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
unsigned off, int val)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
int ret;
mutex_lock(&kpad->gpio_lock);
@@ -460,10 +464,10 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
else
kpad->dat_out[bank] &= ~bit;
- ret = adp5589_write(kpad->client, kpad->var->reg(ADP5589_GPO_DATA_OUT_A)
+ ret = adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A)
+ bank, kpad->dat_out[bank]);
ret |= adp5589_write(kpad->client,
- kpad->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
+ kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
kpad->dir[bank]);
mutex_unlock(&kpad->gpio_lock);
@@ -480,23 +484,23 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad,
memset(pin_used, false, sizeof(pin_used));
- for (i = 0; i < kpad->var->maxgpio; i++)
+ for (i = 0; i < kpad->info->var->maxgpio; i++)
if (pdata->keypad_en_mask & BIT(i))
pin_used[i] = true;
for (i = 0; i < kpad->gpimapsize; i++)
- pin_used[kpad->gpimap[i].pin - kpad->var->gpi_pin_base] = true;
+ pin_used[kpad->gpimap[i].pin - kpad->info->var->gpi_pin_base] = true;
if (kpad->extend_cfg & R4_EXTEND_CFG)
pin_used[4] = true;
if (kpad->extend_cfg & C4_EXTEND_CFG)
- pin_used[kpad->var->c4_extend_cfg] = true;
+ pin_used[kpad->info->var->c4_extend_cfg] = true;
- if (!kpad->support_row5)
+ if (!kpad->info->support_row5)
pin_used[5] = true;
- for (i = 0; i < kpad->var->maxgpio; i++)
+ for (i = 0; i < kpad->info->var->maxgpio; i++)
if (!pin_used[i])
kpad->gpiomap[n_unused++] = i;
@@ -536,11 +540,11 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
if (error)
return error;
- for (i = 0; i <= kpad->var->bank(kpad->var->maxgpio); i++) {
- kpad->dat_out[i] = adp5589_read(kpad->client, kpad->var->reg(
- ADP5589_GPO_DATA_OUT_A) + i);
- kpad->dir[i] = adp5589_read(kpad->client, kpad->var->reg(
- ADP5589_GPIO_DIRECTION_A) + i);
+ for (i = 0; i <= kpad->info->var->bank(kpad->info->var->maxgpio); i++) {
+ kpad->dat_out[i] = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) + i);
+ kpad->dir[i] = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + i);
}
return 0;
@@ -575,8 +579,8 @@ static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
int key = adp5589_read(kpad->client, ADP5589_5_FIFO_1 + i);
int key_val = key & KEY_EV_MASK;
- if (key_val >= kpad->var->gpi_pin_base &&
- key_val <= kpad->var->gpi_pin_end) {
+ if (key_val >= kpad->info->var->gpi_pin_base &&
+ key_val <= kpad->info->var->gpi_pin_end) {
adp5589_report_switches(kpad, key, key_val);
} else {
input_report_key(kpad->input,
@@ -614,7 +618,7 @@ static int adp5589_get_evcode(struct adp5589_kpad *kpad, unsigned short key)
{
int i;
- for (i = 0; i < kpad->var->keymapsize; i++)
+ for (i = 0; i < kpad->info->var->keymapsize; i++)
if (key == kpad->keycode[i])
return (i + 1) | KEY_EV_PRESSED;
@@ -628,22 +632,22 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
struct i2c_client *client = kpad->client;
const struct adp5589_kpad_platform_data *pdata =
dev_get_platdata(&client->dev);
- u8 (*reg) (u8) = kpad->var->reg;
+ u8 (*reg)(u8) = kpad->info->var->reg;
unsigned char evt_mode1 = 0, evt_mode2 = 0, evt_mode3 = 0;
unsigned char pull_mask = 0;
int i, ret;
ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_A),
- pdata->keypad_en_mask & kpad->var->row_mask);
+ pdata->keypad_en_mask & kpad->info->var->row_mask);
ret |= adp5589_write(client, reg(ADP5589_PIN_CONFIG_B),
- (pdata->keypad_en_mask >> kpad->var->col_shift) &
- kpad->var->col_mask);
+ (pdata->keypad_en_mask >> kpad->info->var->col_shift) &
+ kpad->info->var->col_mask);
- if (!kpad->is_adp5585)
+ if (!kpad->info->is_adp5585)
ret |= adp5589_write(client, ADP5589_PIN_CONFIG_C,
(pdata->keypad_en_mask >> 16) & 0xFF);
- if (!kpad->is_adp5585 && pdata->en_keylock) {
+ if (!kpad->info->is_adp5585 && pdata->en_keylock) {
ret |= adp5589_write(client, ADP5589_UNLOCK1,
pdata->unlock_key1);
ret |= adp5589_write(client, ADP5589_UNLOCK2,
@@ -659,14 +663,14 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
for (i = 0; i < pdata->gpimapsize; i++) {
unsigned short pin = pdata->gpimap[i].pin;
- if (pin <= kpad->var->gpi_pin_row_end) {
- evt_mode1 |= BIT(pin - kpad->var->gpi_pin_row_base);
+ if (pin <= kpad->info->var->gpi_pin_row_end) {
+ evt_mode1 |= BIT(pin - kpad->info->var->gpi_pin_row_base);
} else {
evt_mode2 |=
- BIT(pin - kpad->var->gpi_pin_col_base) & 0xFF;
- if (!kpad->is_adp5585)
+ BIT(pin - kpad->info->var->gpi_pin_col_base) & 0xFF;
+ if (!kpad->info->is_adp5585)
evt_mode3 |=
- BIT(pin - kpad->var->gpi_pin_col_base) >> 8;
+ BIT(pin - kpad->info->var->gpi_pin_col_base) >> 8;
}
}
@@ -675,7 +679,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
evt_mode1);
ret |= adp5589_write(client, reg(ADP5589_GPI_EVENT_EN_B),
evt_mode2);
- if (!kpad->is_adp5585)
+ if (!kpad->info->is_adp5585)
ret |= adp5589_write(client,
reg(ADP5589_GPI_EVENT_EN_C),
evt_mode3);
@@ -685,7 +689,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
pdata->pullup_en_300k & pdata->pulldown_en_300k)
dev_warn(&client->dev, "Conflicting pull resistor config\n");
- for (i = 0; i <= kpad->var->max_row_num; i++) {
+ for (i = 0; i <= kpad->info->var->max_row_num; i++) {
unsigned int val = 0, bit = BIT(i);
if (pdata->pullup_en_300k & bit)
val = 0;
@@ -698,15 +702,15 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
pull_mask |= val << (2 * (i & 0x3));
- if (i % 4 == 3 || i == kpad->var->max_row_num) {
+ if (i % 4 == 3 || i == kpad->info->var->max_row_num) {
ret |= adp5589_write(client, reg(ADP5585_RPULL_CONFIG_A)
+ (i >> 2), pull_mask);
pull_mask = 0;
}
}
- for (i = 0; i <= kpad->var->max_col_num; i++) {
- unsigned int val = 0, bit = BIT(i + kpad->var->col_shift);
+ for (i = 0; i <= kpad->info->var->max_col_num; i++) {
+ unsigned int val = 0, bit = BIT(i + kpad->info->var->col_shift);
if (pdata->pullup_en_300k & bit)
val = 0;
else if (pdata->pulldown_en_300k & bit)
@@ -718,7 +722,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
pull_mask |= val << (2 * (i & 0x3));
- if (i % 4 == 3 || i == kpad->var->max_col_num) {
+ if (i % 4 == 3 || i == kpad->info->var->max_col_num) {
ret |= adp5589_write(client,
reg(ADP5585_RPULL_CONFIG_C) +
(i >> 2), pull_mask);
@@ -757,22 +761,22 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
}
ret |= adp5589_write(client, reg(ADP5589_DEBOUNCE_DIS_A),
- pdata->debounce_dis_mask & kpad->var->row_mask);
+ pdata->debounce_dis_mask & kpad->info->var->row_mask);
ret |= adp5589_write(client, reg(ADP5589_DEBOUNCE_DIS_B),
- (pdata->debounce_dis_mask >> kpad->var->col_shift)
- & kpad->var->col_mask);
+ (pdata->debounce_dis_mask >> kpad->info->var->col_shift)
+ & kpad->info->var->col_mask);
- if (!kpad->is_adp5585)
+ if (!kpad->info->is_adp5585)
ret |= adp5589_write(client, reg(ADP5589_DEBOUNCE_DIS_C),
(pdata->debounce_dis_mask >> 16) & 0xFF);
ret |= adp5589_write(client, reg(ADP5589_POLL_PTIME_CFG),
pdata->scan_cycle_time & PTIME_MASK);
ret |= adp5589_write(client, ADP5589_5_INT_STATUS,
- (kpad->is_adp5585 ? 0 : LOGIC2_INT) |
+ (kpad->info->is_adp5585 ? 0 : LOGIC2_INT) |
LOGIC1_INT | OVRFLOW_INT |
- (kpad->is_adp5585 ? 0 : LOCK_INT) |
+ (kpad->info->is_adp5585 ? 0 : LOCK_INT) |
GPI_INT | EVENT_INT); /* Status is W1C */
ret |= adp5589_write(client, reg(ADP5589_GENERAL_CFG),
@@ -793,24 +797,24 @@ static void adp5589_report_switch_state(struct adp5589_kpad *kpad)
int gpi_stat_tmp, pin_loc;
int i;
int gpi_stat1 = adp5589_read(kpad->client,
- kpad->var->reg(ADP5589_GPI_STATUS_A));
+ kpad->info->var->reg(ADP5589_GPI_STATUS_A));
int gpi_stat2 = adp5589_read(kpad->client,
- kpad->var->reg(ADP5589_GPI_STATUS_B));
- int gpi_stat3 = !kpad->is_adp5585 ?
+ kpad->info->var->reg(ADP5589_GPI_STATUS_B));
+ int gpi_stat3 = !kpad->info->is_adp5585 ?
adp5589_read(kpad->client, ADP5589_GPI_STATUS_C) : 0;
for (i = 0; i < kpad->gpimapsize; i++) {
unsigned short pin = kpad->gpimap[i].pin;
- if (pin <= kpad->var->gpi_pin_row_end) {
+ if (pin <= kpad->info->var->gpi_pin_row_end) {
gpi_stat_tmp = gpi_stat1;
- pin_loc = pin - kpad->var->gpi_pin_row_base;
- } else if ((pin - kpad->var->gpi_pin_col_base) < 8) {
+ pin_loc = pin - kpad->info->var->gpi_pin_row_base;
+ } else if ((pin - kpad->info->var->gpi_pin_col_base) < 8) {
gpi_stat_tmp = gpi_stat2;
- pin_loc = pin - kpad->var->gpi_pin_col_base;
+ pin_loc = pin - kpad->info->var->gpi_pin_col_base;
} else {
gpi_stat_tmp = gpi_stat3;
- pin_loc = pin - kpad->var->gpi_pin_col_base - 8;
+ pin_loc = pin - kpad->info->var->gpi_pin_col_base - 8;
}
if (gpi_stat_tmp < 0) {
@@ -837,14 +841,13 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
unsigned int i;
int error;
- if (!((pdata->keypad_en_mask & kpad->var->row_mask) &&
- (pdata->keypad_en_mask >> kpad->var->col_shift)) ||
- !pdata->keymap) {
+ if (!((pdata->keypad_en_mask & kpad->info->var->row_mask) &&
+ (pdata->keypad_en_mask >> kpad->info->var->col_shift)) || !pdata->keymap) {
dev_err(&client->dev, "no rows, cols or keymap from pdata\n");
return -EINVAL;
}
- if (pdata->keymapsize != kpad->var->keymapsize) {
+ if (pdata->keymapsize != kpad->info->var->keymapsize) {
dev_err(&client->dev, "invalid keymapsize\n");
return -EINVAL;
}
@@ -854,7 +857,7 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
return -EINVAL;
}
- if (pdata->gpimapsize > kpad->var->gpimapsize_max) {
+ if (pdata->gpimapsize > kpad->info->var->gpimapsize_max) {
dev_err(&client->dev, "invalid gpimapsize\n");
return -EINVAL;
}
@@ -862,13 +865,13 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
for (i = 0; i < pdata->gpimapsize; i++) {
unsigned short pin = pdata->gpimap[i].pin;
- if (pin < kpad->var->gpi_pin_base ||
- pin > kpad->var->gpi_pin_end) {
+ if (pin < kpad->info->var->gpi_pin_base ||
+ pin > kpad->info->var->gpi_pin_end) {
dev_err(&client->dev, "invalid gpi pin data\n");
return -EINVAL;
}
- if (BIT(pin - kpad->var->gpi_pin_row_base) &
+ if (BIT(pin - kpad->info->var->gpi_pin_row_base) &
pdata->keypad_en_mask) {
dev_err(&client->dev, "invalid gpi row/col data\n");
return -EINVAL;
@@ -945,12 +948,28 @@ static void adp5589_clear_config(void *data)
{
struct adp5589_kpad *kpad = data;
- adp5589_write(kpad->client, kpad->var->reg(ADP5589_GENERAL_CFG), 0);
+ adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GENERAL_CFG),
+ 0);
}
+static const struct adp5589_info adp5589_info = {
+ .var = &const_adp5589,
+ .support_row5 = true,
+};
+
+static const struct adp5589_info adp5585_info = {
+ .var = &const_adp5585,
+ .is_adp5585 = true,
+};
+
+static const struct adp5589_info adp5585_2_info = {
+ .var = &const_adp5585,
+ .is_adp5585 = true,
+ .support_row5 = true,
+};
+
static int adp5589_probe(struct i2c_client *client)
{
- const struct i2c_device_id *id = i2c_client_get_device_id(client);
struct adp5589_kpad *kpad;
const struct adp5589_kpad_platform_data *pdata =
dev_get_platdata(&client->dev);
@@ -974,19 +993,9 @@ static int adp5589_probe(struct i2c_client *client)
kpad->client = client;
- switch (id->driver_data) {
- case ADP5585_02:
- kpad->support_row5 = true;
- fallthrough;
- case ADP5585_01:
- kpad->is_adp5585 = true;
- kpad->var = &const_adp5585;
- break;
- case ADP5589:
- kpad->support_row5 = true;
- kpad->var = &const_adp5589;
- break;
- }
+ kpad->info = i2c_get_match_data(client);
+ if (!kpad->info)
+ return -ENODEV;
error = devm_add_action_or_reset(&client->dev, adp5589_clear_config,
kpad);
@@ -1045,9 +1054,9 @@ static int adp5589_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(adp5589_dev_pm_ops, adp5589_suspend, adp5589_resume);
static const struct i2c_device_id adp5589_id[] = {
- {"adp5589-keys", ADP5589},
- {"adp5585-keys", ADP5585_01},
- {"adp5585-02-keys", ADP5585_02}, /* Adds ROW5 to ADP5585 */
+ {"adp5589-keys", (kernel_ulong_t)&adp5589_info},
+ {"adp5585-keys", (kernel_ulong_t)&adp5585_info},
+ {"adp5585-02-keys", (kernel_ulong_t)&adp5585_2_info}, /* Adds ROW5 to ADP5585 */
{}
};
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 03/13] Input: adp5589-keys: add chip_info structure
2024-10-01 13:41 ` [PATCH 03/13] Input: adp5589-keys: add chip_info structure Nuno Sa
@ 2024-10-01 14:55 ` Dmitry Torokhov
2024-10-02 9:13 ` Nuno Sá
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2024-10-01 14:55 UTC (permalink / raw)
To: Nuno Sa
Cc: Mike Frysinger, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
On Tue, Oct 01, 2024 at 03:41:34PM +0200, Nuno Sa wrote:
> Add a more natural chip_info structure and add it to the i2c id table
> driver data so that we do not need an enum a switch() to get the
> specific bits of each device.
>
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
> drivers/input/keyboard/adp5589-keys.c | 181 ++++++++++++++++++----------------
> 1 file changed, 95 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
> index 922d3ab998f3a5dfbaf277f10eb19e5cd1b35415..eaa5440d4f9e14352409dd880cd254354612bf3e 100644
> --- a/drivers/input/keyboard/adp5589-keys.c
> +++ b/drivers/input/keyboard/adp5589-keys.c
> @@ -228,16 +228,20 @@ struct adp_constants {
> u8 (*reg) (u8 reg);
> };
>
> +struct adp5589_info {
> + const struct adp_constants *var;
> + bool support_row5;
Is it possible to derive "row5" data from keymap information to avoid
having this fake "adp5585-02-keys" device?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 03/13] Input: adp5589-keys: add chip_info structure
2024-10-01 14:55 ` Dmitry Torokhov
@ 2024-10-02 9:13 ` Nuno Sá
2024-10-02 9:47 ` Dmitry Torokhov
0 siblings, 1 reply; 28+ messages in thread
From: Nuno Sá @ 2024-10-02 9:13 UTC (permalink / raw)
To: Dmitry Torokhov, Nuno Sa
Cc: Mike Frysinger, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
On Tue, 2024-10-01 at 07:55 -0700, Dmitry Torokhov wrote:
> On Tue, Oct 01, 2024 at 03:41:34PM +0200, Nuno Sa wrote:
> > Add a more natural chip_info structure and add it to the i2c id table
> > driver data so that we do not need an enum a switch() to get the
> > specific bits of each device.
> >
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> > drivers/input/keyboard/adp5589-keys.c | 181 ++++++++++++++++++----------------
> > 1 file changed, 95 insertions(+), 86 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/adp5589-keys.c
> > b/drivers/input/keyboard/adp5589-keys.c
> > index
> > 922d3ab998f3a5dfbaf277f10eb19e5cd1b35415..eaa5440d4f9e14352409dd880cd254354612bf3
> > e 100644
> > --- a/drivers/input/keyboard/adp5589-keys.c
> > +++ b/drivers/input/keyboard/adp5589-keys.c
> > @@ -228,16 +228,20 @@ struct adp_constants {
> > u8 (*reg) (u8 reg);
> > };
> >
> > +struct adp5589_info {
> > + const struct adp_constants *var;
> > + bool support_row5;
>
> Is it possible to derive "row5" data from keymap information to avoid
> having this fake "adp5585-02-keys" device?
>
This is not a fake device. Looking at the adp5585 datasheet you can see there's
module with 25 keys (without GPIO5) and another with 11 GPIOS. From the datasheet:
"- 10 configurable I/Os allowing functions such as Key pad decoding for a matrix of
up to 5 × 5
- 11 GPIOs (5 × 6) with ADP5585ACxZ-01-R7 models"
Why its named adp5585-02 in the driver I'm not sure. I kept the same name as the i2c
id? Should I call it ADP5585-1 instead? Or even ADP5585-1-r7?
- Nuno Sá
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 03/13] Input: adp5589-keys: add chip_info structure
2024-10-02 9:13 ` Nuno Sá
@ 2024-10-02 9:47 ` Dmitry Torokhov
2024-10-02 10:57 ` Nuno Sá
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2024-10-02 9:47 UTC (permalink / raw)
To: Nuno Sá
Cc: Nuno Sa, Mike Frysinger, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree
On Wed, Oct 02, 2024 at 11:13:05AM +0200, Nuno Sá wrote:
> On Tue, 2024-10-01 at 07:55 -0700, Dmitry Torokhov wrote:
> > On Tue, Oct 01, 2024 at 03:41:34PM +0200, Nuno Sa wrote:
> > > Add a more natural chip_info structure and add it to the i2c id table
> > > driver data so that we do not need an enum a switch() to get the
> > > specific bits of each device.
> > >
> > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > > ---
> > > drivers/input/keyboard/adp5589-keys.c | 181 ++++++++++++++++++----------------
> > > 1 file changed, 95 insertions(+), 86 deletions(-)
> > >
> > > diff --git a/drivers/input/keyboard/adp5589-keys.c
> > > b/drivers/input/keyboard/adp5589-keys.c
> > > index
> > > 922d3ab998f3a5dfbaf277f10eb19e5cd1b35415..eaa5440d4f9e14352409dd880cd254354612bf3
> > > e 100644
> > > --- a/drivers/input/keyboard/adp5589-keys.c
> > > +++ b/drivers/input/keyboard/adp5589-keys.c
> > > @@ -228,16 +228,20 @@ struct adp_constants {
> > > u8 (*reg) (u8 reg);
> > > };
> > >
> > > +struct adp5589_info {
> > > + const struct adp_constants *var;
> > > + bool support_row5;
> >
> > Is it possible to derive "row5" data from keymap information to avoid
> > having this fake "adp5585-02-keys" device?
> >
>
> This is not a fake device. Looking at the adp5585 datasheet you can see there's
> module with 25 keys (without GPIO5) and another with 11 GPIOS. From the datasheet:
>
> "- 10 configurable I/Os allowing functions such as Key pad decoding for a matrix of
> up to 5 × 5
> - 11 GPIOs (5 × 6) with ADP5585ACxZ-01-R7 models"
Ah, I misunderstood. I thought it was a runtime configuration.
>
> Why its named adp5585-02 in the driver I'm not sure. I kept the same name as the i2c
> id? Should I call it ADP5585-1 instead? Or even ADP5585-1-r7?
I think this question is better answered by the DT folks.
BTW, in case of not using row5 we need to describe this pin as a reset
line for the chip, right?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 03/13] Input: adp5589-keys: add chip_info structure
2024-10-02 9:47 ` Dmitry Torokhov
@ 2024-10-02 10:57 ` Nuno Sá
0 siblings, 0 replies; 28+ messages in thread
From: Nuno Sá @ 2024-10-02 10:57 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Nuno Sa, Mike Frysinger, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-input, devicetree
On Wed, 2024-10-02 at 02:47 -0700, Dmitry Torokhov wrote:
> On Wed, Oct 02, 2024 at 11:13:05AM +0200, Nuno Sá wrote:
> > On Tue, 2024-10-01 at 07:55 -0700, Dmitry Torokhov wrote:
> > > On Tue, Oct 01, 2024 at 03:41:34PM +0200, Nuno Sa wrote:
> > > > Add a more natural chip_info structure and add it to the i2c id table
> > > > driver data so that we do not need an enum a switch() to get the
> > > > specific bits of each device.
> > > >
> > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > > > ---
> > > > drivers/input/keyboard/adp5589-keys.c | 181 ++++++++++++++++++--------------
> > > > --
> > > > 1 file changed, 95 insertions(+), 86 deletions(-)
> > > >
> > > > diff --git a/drivers/input/keyboard/adp5589-keys.c
> > > > b/drivers/input/keyboard/adp5589-keys.c
> > > > index
> > > > 922d3ab998f3a5dfbaf277f10eb19e5cd1b35415..eaa5440d4f9e14352409dd880cd25435461
> > > > 2bf3
> > > > e 100644
> > > > --- a/drivers/input/keyboard/adp5589-keys.c
> > > > +++ b/drivers/input/keyboard/adp5589-keys.c
> > > > @@ -228,16 +228,20 @@ struct adp_constants {
> > > > u8 (*reg) (u8 reg);
> > > > };
> > > >
> > > > +struct adp5589_info {
> > > > + const struct adp_constants *var;
> > > > + bool support_row5;
> > >
> > > Is it possible to derive "row5" data from keymap information to avoid
> > > having this fake "adp5585-02-keys" device?
> > >
> >
> > This is not a fake device. Looking at the adp5585 datasheet you can see there's
> > module with 25 keys (without GPIO5) and another with 11 GPIOS. From the
> > datasheet:
> >
> > "- 10 configurable I/Os allowing functions such as Key pad decoding for a matrix
> > of
> > up to 5 × 5
> > - 11 GPIOs (5 × 6) with ADP5585ACxZ-01-R7 models"
>
> Ah, I misunderstood. I thought it was a runtime configuration.
>
> >
> > Why its named adp5585-02 in the driver I'm not sure. I kept the same name as the
> > i2c
> > id? Should I call it ADP5585-1 instead? Or even ADP5585-1-r7?
>
> I think this question is better answered by the DT folks.
>
> BTW, in case of not using row5 we need to describe this pin as a reset
> line for the chip, right?
>
Oh yes. I can add a reset pin to the bindings. And make it false for the model where
R5 is in place.
- Nuno Sá
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 04/13] Input: adp5589-keys: support gpi key events as 'gpio keys'
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (2 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 03/13] Input: adp5589-keys: add chip_info structure Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 05/13] dt-bindings: input: Document adp5589 and similar devices Nuno Sa
` (9 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
This change replaces the support for GPIs as key event generators.
Instead of reporting the events directly, we add a gpio based irqchip
so that these events can be consumed by keys defined in the gpio-keys
driver (as it's goal is indeed for keys on GPIOs capable of generating
interrupts).
The basic idea is that all the pins that are not being used as part of
the keymap matrix (or by the reset block) can be possibly requested as
GPIOs by gpio-keys (it's also fine to use these pins as plain interrupts
though that's not really the point).
While at it, always select GPIOLIB so that we don't need to use #ifdef
guards.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/Kconfig | 2 +
drivers/input/keyboard/adp5589-keys.c | 260 +++++++++++++++++-----------------
include/linux/input/adp5589.h | 2 -
3 files changed, 133 insertions(+), 131 deletions(-)
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a5015d6f8bedd6a2ae6e42eb28d20a0a109b4ddf..33ab9a178983b836a2724f870ebb9718b44da18a 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -53,6 +53,8 @@ config KEYBOARD_ADP5588
config KEYBOARD_ADP5589
tristate "ADP5585/ADP5589 I2C QWERTY Keypad and IO Expander"
depends on I2C
+ select GPIOLIB
+ select GPIOLIB_IRQCHIP
help
Say Y here if you want to use a ADP5585/ADP5589 attached to your
system I2C bus.
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index eaa5440d4f9e14352409dd880cd254354612bf3e..f107ed08fc787d31c09a5177395085548a6fadd7 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -242,13 +242,13 @@ struct adp5589_kpad {
const struct adp5589_gpi_map *gpimap;
unsigned short gpimapsize;
unsigned extend_cfg;
-#ifdef CONFIG_GPIOLIB
unsigned char gpiomap[ADP5589_MAXGPIO];
struct gpio_chip gc;
struct mutex gpio_lock; /* Protect cached dir, dat_out */
u8 dat_out[3];
u8 dir[3];
-#endif
+ u8 int_en[3];
+ u8 irq_mask[3];
};
/*
@@ -389,7 +389,6 @@ static int adp5589_write(struct i2c_client *client, u8 reg, u8 val)
return i2c_smbus_write_byte_data(client, reg, val);
}
-#ifdef CONFIG_GPIOLIB
static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
@@ -488,9 +487,6 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad,
if (pdata->keypad_en_mask & BIT(i))
pin_used[i] = true;
- for (i = 0; i < kpad->gpimapsize; i++)
- pin_used[kpad->gpimap[i].pin - kpad->info->var->gpi_pin_base] = true;
-
if (kpad->extend_cfg & R4_EXTEND_CFG)
pin_used[4] = true;
@@ -507,11 +503,84 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad,
return n_unused;
}
+static void adp5589_irq_bus_lock(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct adp5589_kpad *kpad = gpiochip_get_data(gc);
+
+ mutex_lock(&kpad->gpio_lock);
+}
+
+static void adp5589_irq_bus_sync_unlock(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct adp5589_kpad *kpad = gpiochip_get_data(gc);
+ const struct adp_constants *var = kpad->info->var;
+ int i;
+
+ for (i = 0; i <= kpad->info->var->bank(kpad->info->var->maxgpio); i++) {
+ if (kpad->int_en[i] ^ kpad->irq_mask[i]) {
+ kpad->int_en[i] = kpad->irq_mask[i];
+ adp5589_write(kpad->client,
+ var->reg(ADP5589_GPI_EVENT_EN_A) + i,
+ kpad->int_en[i]);
+ }
+ }
+
+ mutex_unlock(&kpad->gpio_lock);
+}
+
+static void adp5589_irq_mask(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct adp5589_kpad *kpad = gpiochip_get_data(gc);
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
+ unsigned long real_irq = kpad->gpiomap[hwirq];
+ unsigned int bank = kpad->info->var->bank(real_irq);
+
+ kpad->irq_mask[bank] &= ~kpad->info->var->bit(real_irq);
+ gpiochip_disable_irq(gc, hwirq);
+}
+
+static void adp5589_irq_unmask(struct irq_data *d)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct adp5589_kpad *kpad = gpiochip_get_data(gc);
+ irq_hw_number_t hwirq = irqd_to_hwirq(d);
+ unsigned long real_irq = kpad->gpiomap[hwirq];
+ unsigned int bank = kpad->info->var->bank(real_irq);
+
+ gpiochip_enable_irq(gc, hwirq);
+ kpad->irq_mask[bank] |= kpad->info->var->bit(real_irq);
+}
+
+static int adp5589_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ if (!(type & IRQ_TYPE_EDGE_BOTH))
+ return -EINVAL;
+
+ irq_set_handler_locked(d, handle_edge_irq);
+
+ return 0;
+}
+
+static const struct irq_chip adp5589_irq_chip = {
+ .name = "adp5589",
+ .irq_mask = adp5589_irq_mask,
+ .irq_unmask = adp5589_irq_unmask,
+ .irq_bus_lock = adp5589_irq_bus_lock,
+ .irq_bus_sync_unlock = adp5589_irq_bus_sync_unlock,
+ .irq_set_type = adp5589_irq_set_type,
+ .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_IMMUTABLE,
+ GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
static int adp5589_gpio_add(struct adp5589_kpad *kpad)
{
struct device *dev = &kpad->client->dev;
const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
const struct adp5589_gpio_platform_data *gpio_data = pdata->gpio_data;
+ struct gpio_irq_chip *girq;
int i, error;
if (!gpio_data)
@@ -534,6 +603,11 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
kpad->gc.label = kpad->client->name;
kpad->gc.owner = THIS_MODULE;
+ girq = &kpad->gc.irq;
+ gpio_irq_chip_set_chip(girq, &adp5589_irq_chip);
+ girq->handler = handle_bad_irq;
+ girq->threaded = true;
+
mutex_init(&kpad->gpio_lock);
error = devm_gpiochip_add_data(dev, &kpad->gc, kpad);
@@ -549,26 +623,57 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
return 0;
}
-#else
-static inline int adp5589_gpio_add(struct adp5589_kpad *kpad)
+
+static unsigned long adp5589_gpiomap_get_hwirq(struct device *dev,
+ const u8 *map, unsigned int gpio,
+ unsigned int ngpios)
{
- return 0;
+ unsigned int hwirq;
+
+ for (hwirq = 0; hwirq < ngpios; hwirq++)
+ if (map[hwirq] == gpio)
+ return hwirq;
+
+ /* should never happen */
+ dev_warn_ratelimited(dev, "could not find the hwirq for gpio(%u)\n", gpio);
+
+ return INVALID_HWIRQ;
}
-#endif
-static void adp5589_report_switches(struct adp5589_kpad *kpad,
- int key, int key_val)
+static void adp5589_gpio_irq_handle(struct adp5589_kpad *kpad, int key_val,
+ int key_press)
{
- int i;
+ unsigned int irq, gpio = key_val - kpad->info->var->gpi_pin_base, irq_type;
+ struct i2c_client *client = kpad->client;
+ struct irq_data *irqd;
+ unsigned long hwirq;
- for (i = 0; i < kpad->gpimapsize; i++) {
- if (key_val == kpad->gpimap[i].pin) {
- input_report_switch(kpad->input,
- kpad->gpimap[i].sw_evt,
- key & KEY_EV_PRESSED);
- break;
- }
+ hwirq = adp5589_gpiomap_get_hwirq(&client->dev, kpad->gpiomap,
+ gpio, kpad->gc.ngpio);
+ if (hwirq == INVALID_HWIRQ) {
+ dev_err(&client->dev, "Could not get hwirq for key(%u)\n", key_val);
+ return;
}
+
+ irq = irq_find_mapping(kpad->gc.irq.domain, hwirq);
+ if (!irq)
+ return;
+
+ irqd = irq_get_irq_data(irq);
+ if (!irqd) {
+ dev_err(&client->dev, "Could not get irq(%u) data\n", irq);
+ return;
+ }
+
+ irq_type = irqd_get_trigger_type(irqd);
+
+ /*
+ * Default is active low which means key_press is asserted on
+ * the falling edge.
+ */
+ if ((irq_type & IRQ_TYPE_EDGE_RISING && !key_press) ||
+ (irq_type & IRQ_TYPE_EDGE_FALLING && key_press))
+ handle_nested_irq(irq);
}
static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
@@ -578,15 +683,15 @@ static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
for (i = 0; i < ev_cnt; i++) {
int key = adp5589_read(kpad->client, ADP5589_5_FIFO_1 + i);
int key_val = key & KEY_EV_MASK;
+ int key_press = key & KEY_EV_PRESSED;
if (key_val >= kpad->info->var->gpi_pin_base &&
- key_val <= kpad->info->var->gpi_pin_end) {
- adp5589_report_switches(kpad, key, key_val);
- } else {
+ key_val <= kpad->info->var->gpi_pin_end)
+ /* gpio line used as IRQ source */
+ adp5589_gpio_irq_handle(kpad, key_val, key_press);
+ else
input_report_key(kpad->input,
- kpad->keycode[key_val - 1],
- key & KEY_EV_PRESSED);
- }
+ kpad->keycode[key_val - 1], key_press);
}
}
@@ -633,7 +738,6 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
const struct adp5589_kpad_platform_data *pdata =
dev_get_platdata(&client->dev);
u8 (*reg)(u8) = kpad->info->var->reg;
- unsigned char evt_mode1 = 0, evt_mode2 = 0, evt_mode3 = 0;
unsigned char pull_mask = 0;
int i, ret;
@@ -660,31 +764,6 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
for (i = 0; i < KEYP_MAX_EVENT; i++)
ret |= adp5589_read(client, ADP5589_5_FIFO_1 + i);
- for (i = 0; i < pdata->gpimapsize; i++) {
- unsigned short pin = pdata->gpimap[i].pin;
-
- if (pin <= kpad->info->var->gpi_pin_row_end) {
- evt_mode1 |= BIT(pin - kpad->info->var->gpi_pin_row_base);
- } else {
- evt_mode2 |=
- BIT(pin - kpad->info->var->gpi_pin_col_base) & 0xFF;
- if (!kpad->info->is_adp5585)
- evt_mode3 |=
- BIT(pin - kpad->info->var->gpi_pin_col_base) >> 8;
- }
- }
-
- if (pdata->gpimapsize) {
- ret |= adp5589_write(client, reg(ADP5589_GPI_EVENT_EN_A),
- evt_mode1);
- ret |= adp5589_write(client, reg(ADP5589_GPI_EVENT_EN_B),
- evt_mode2);
- if (!kpad->info->is_adp5585)
- ret |= adp5589_write(client,
- reg(ADP5589_GPI_EVENT_EN_C),
- evt_mode3);
- }
-
if (pdata->pull_dis_mask & pdata->pullup_en_100k &
pdata->pullup_en_300k & pdata->pulldown_en_300k)
dev_warn(&client->dev, "Conflicting pull resistor config\n");
@@ -792,46 +871,6 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
return 0;
}
-static void adp5589_report_switch_state(struct adp5589_kpad *kpad)
-{
- int gpi_stat_tmp, pin_loc;
- int i;
- int gpi_stat1 = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPI_STATUS_A));
- int gpi_stat2 = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPI_STATUS_B));
- int gpi_stat3 = !kpad->info->is_adp5585 ?
- adp5589_read(kpad->client, ADP5589_GPI_STATUS_C) : 0;
-
- for (i = 0; i < kpad->gpimapsize; i++) {
- unsigned short pin = kpad->gpimap[i].pin;
-
- if (pin <= kpad->info->var->gpi_pin_row_end) {
- gpi_stat_tmp = gpi_stat1;
- pin_loc = pin - kpad->info->var->gpi_pin_row_base;
- } else if ((pin - kpad->info->var->gpi_pin_col_base) < 8) {
- gpi_stat_tmp = gpi_stat2;
- pin_loc = pin - kpad->info->var->gpi_pin_col_base;
- } else {
- gpi_stat_tmp = gpi_stat3;
- pin_loc = pin - kpad->info->var->gpi_pin_col_base - 8;
- }
-
- if (gpi_stat_tmp < 0) {
- dev_err(&kpad->client->dev,
- "Can't read GPIO_DAT_STAT switch %d, default to OFF\n",
- pin);
- gpi_stat_tmp = 0;
- }
-
- input_report_switch(kpad->input,
- kpad->gpimap[i].sw_evt,
- !(gpi_stat_tmp & BIT(pin_loc)));
- }
-
- input_sync(kpad->input);
-}
-
static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
{
struct i2c_client *client = kpad->client;
@@ -852,32 +891,6 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
return -EINVAL;
}
- if (!pdata->gpimap && pdata->gpimapsize) {
- dev_err(&client->dev, "invalid gpimap from pdata\n");
- return -EINVAL;
- }
-
- if (pdata->gpimapsize > kpad->info->var->gpimapsize_max) {
- dev_err(&client->dev, "invalid gpimapsize\n");
- return -EINVAL;
- }
-
- for (i = 0; i < pdata->gpimapsize; i++) {
- unsigned short pin = pdata->gpimap[i].pin;
-
- if (pin < kpad->info->var->gpi_pin_base ||
- pin > kpad->info->var->gpi_pin_end) {
- dev_err(&client->dev, "invalid gpi pin data\n");
- return -EINVAL;
- }
-
- if (BIT(pin - kpad->info->var->gpi_pin_row_base) &
- pdata->keypad_en_mask) {
- dev_err(&client->dev, "invalid gpi row/col data\n");
- return -EINVAL;
- }
- }
-
if (!client->irq) {
dev_err(&client->dev, "no IRQ?\n");
return -EINVAL;
@@ -907,9 +920,6 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
memcpy(kpad->keycode, pdata->keymap,
pdata->keymapsize * input->keycodesize);
- kpad->gpimap = pdata->gpimap;
- kpad->gpimapsize = pdata->gpimapsize;
-
/* setup input device */
__set_bit(EV_KEY, input->evbit);
@@ -921,11 +931,6 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
__set_bit(kpad->keycode[i], input->keybit);
__clear_bit(KEY_RESERVED, input->keybit);
- if (kpad->gpimapsize)
- __set_bit(EV_SW, input->evbit);
- for (i = 0; i < kpad->gpimapsize; i++)
- __set_bit(kpad->gpimap[i].sw_evt, input->swbit);
-
error = input_register_device(input);
if (error) {
dev_err(&client->dev, "unable to register input device\n");
@@ -1018,9 +1023,6 @@ static int adp5589_probe(struct i2c_client *client)
if (error)
return error;
- if (kpad->gpimapsize)
- adp5589_report_switch_state(kpad);
-
error = adp5589_gpio_add(kpad);
if (error)
return error;
diff --git a/include/linux/input/adp5589.h b/include/linux/input/adp5589.h
index 0e4742c8c81e3c2e1e78b3e3aeb6e6a07cc83215..6185fc1ba30869c014b2d3e49e21d46b464cba6a 100644
--- a/include/linux/input/adp5589.h
+++ b/include/linux/input/adp5589.h
@@ -166,8 +166,6 @@ struct adp5589_kpad_platform_data {
unsigned pullup_en_100k; /* Pull-Up 100k Enable Mask */
unsigned pullup_en_300k; /* Pull-Up 300k Enable Mask */
unsigned pulldown_en_300k; /* Pull-Down 300k Enable Mask */
- const struct adp5589_gpi_map *gpimap;
- unsigned short gpimapsize;
const struct adp5589_gpio_platform_data *gpio_data;
};
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 05/13] dt-bindings: input: Document adp5589 and similar devices
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (3 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 04/13] Input: adp5589-keys: support gpi key events as 'gpio keys' Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-02 20:58 ` Rob Herring
2024-10-01 13:41 ` [PATCH 06/13] Input: adp5589-keys: add support for fw properties Nuno Sa
` (8 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Add device tree bindings for the adp5589 keypad (and similar devices). The
ADP5585 family has small differences like the lack of the unlock
function and less pins (cols x rows) for the keymap.
As there's no MAINTAINERS entry for these devices, add one. Also to note
that these devices were removed from trivial-devices.yaml.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
.../devicetree/bindings/input/adi,adp5589.yaml | 310 +++++++++++++++++++++
.../devicetree/bindings/trivial-devices.yaml | 6 -
MAINTAINERS | 9 +
3 files changed, 319 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/adi,adp5589.yaml b/Documentation/devicetree/bindings/input/adi,adp5589.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..bdbc79758a0390952c8363ec28f48057dab929a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adi,adp5589.yaml
@@ -0,0 +1,310 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/adi,adp5589.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADP5589 and similar Keypad Controllers
+
+maintainers:
+ - Nuno Sa <nuno.sa@analog.com>
+ - Michael Hennerich <michael.hennerich@analog.com>
+
+description: |
+ Analog Devices Mobile I/O Expander and QWERTY Keypad Controllers
+ - https://www.analog.com/media/en/technical-documentation/data-sheets/ADP5589.pdf
+ - https://www.analog.com/media/en/technical-documentation/data-sheets/ADP5585.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,adp5589
+ - adi,adp5585
+ - adi,adp5585-2
+
+ reg:
+ maxItems: 1
+
+ vcc-supply: true
+
+ interrupts:
+ maxItems: 1
+
+ gpio-controller:
+ description:
+ This property applies if there are pins not used in the keypad.
+
+ '#gpio-cells':
+ const: 2
+
+ interrupt-controller:
+ description:
+ This property applies if there are pins not used in the keypad.
+
+ '#interrupt-cells':
+ const: 2
+
+ adi,cols-mask:
+ description:
+ Defines the number of pins (columns) being used ad part of the keymap. As
+ the device is fully configurable and we can have holes in the columns
+ being used, this is given as mask.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0x1
+ maximum: 0x3f
+
+ adi,rows-mask:
+ description:
+ Defines the number of pins (rows) being used ad part of the keymap. As
+ the device is fully configurable and we can have holes in the rows being
+ used, this is given as mask.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0x1
+ maximum: 0xff
+
+ adi,key-poll-ms:
+ description: Configure time between consecutive scan cycles.
+ enum: [10, 20, 30, 40]
+ default: 10
+
+ adi,unlock-keys:
+ description:
+ Specifies a maximum of 2 keys that can be used to unlock the keypad.
+ If this property is set, the keyboard will be locked and only unlocked
+ after these keys are pressed. The value 127 serves as a wildcard which
+ means any key can be used for unlocking.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 2
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 88
+ - minimum: 97
+ maximum: 115
+ - const: 127
+
+ adi,unlock-trigger-sec:
+ description:
+ Defines the time in which the second unlock event must occur after the
+ first unlock event has occurred.
+ maximum: 7
+ default: 0
+
+ adi,reset1-keys:
+ description:
+ Defines the trigger events (key presses) that can generate reset
+ conditions one the reset1 block.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 3
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 88
+ - minimum: 97
+ maximum: 115
+
+ adi,reset2-keys:
+ description:
+ Defines the trigger events (key presses) that can generate reset
+ conditions one the reset2 block.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 2
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 88
+ - minimum: 97
+ maximum: 115
+
+ adi,reset1-active-high:
+ description: Sets the reset1 signal as active high.
+ type: boolean
+
+ adi,reset2-active-high:
+ description: Sets the reset2 signal as active high.
+ type: boolean
+
+ adi,rst-passtrough-enable:
+ description: Allows the RST pin to override (OR with) the reset1 signal.
+ type: boolean
+
+ adi,reset-trigger-ms:
+ description:
+ Defines the length of time that the reset events must be active before a
+ reset signal is generated. All events must be active at the same time for
+ the same duration.
+ enum: [0, 1000, 1500, 2000, 2500, 3000, 3500, 4000]
+ default: 0
+
+ adi,reset-pulse-width-us:
+ description: Defines the pulse width of the reset signals.
+ enum: [500, 1000, 2000, 10000]
+ default: 500
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+patternProperties:
+ "^gpio@":
+ type: object
+ additionalProperties: false
+
+ properties:
+ reg:
+ description: The GPIO number being configured.
+ maximum: 18
+
+ adi,pull-up-ohms:
+ description: The pullup resistor to be used.
+ enum: [100000, 300000]
+ default: 300000
+
+ required:
+ - reg
+
+dependencies:
+ adi,rows-mask:
+ - linux,keymap
+ - adi,cols-mask
+ adi,cols-mask:
+ - linux,keymap
+ - adi,rows-mask
+ linux,keymap:
+ - adi,rows-mask
+ - adi,cols-mask
+ - interrupts
+ interrupt-controller:
+ - interrupts
+ adi,unlock-trigger-sec:
+ - adi,unlock-keys
+ adi,reset1-active-high:
+ - adi,reset1-keys
+ adi,rst-passtrough-enable:
+ - adi,reset1-keys
+ adi,reset2-active-high:
+ - adi,reset2-keys
+
+required:
+ - compatible
+ - reg
+ - vcc-supply
+
+allOf:
+ - $ref: matrix-keymap.yaml#
+ - $ref: input.yaml#
+ - if:
+ properties:
+ compatible:
+ enum:
+ - adi,adp5585-2
+ then:
+ properties:
+ adi,unlock-keys: false
+ adi,unlock-trigger-sec: false
+ adi,rows-mask:
+ maximum: 0x2f
+ adi,cols-mask:
+ maximum: 0x1f
+ adi,reset1-keys:
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 30
+ - minimum: 37
+ maximum: 47
+ adi,reset2-keys:
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 30
+ - minimum: 37
+ maximum: 47
+ patternProperties:
+ "^gpio@":
+ properties:
+ reg:
+ maximum: 10
+ - if:
+ properties:
+ compatible:
+ enum:
+ - adi,adp5585
+ then:
+ properties:
+ adi,unlock-keys: false
+ adi,unlock-trigger-sec: false
+ adi,rows-mask:
+ maximum: 0x1f
+ adi,cols-mask:
+ maximum: 0x1f
+ adi,reset1-keys:
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 25
+ - enum: [37, 38, 39, 40, 41, 43, 44, 45, 46, 47]
+ adi,reset2-keys:
+ items:
+ anyOf:
+ - minimum: 1
+ maximum: 25
+ - enum: [37, 38, 39, 40, 41, 43, 44, 45, 46, 47]
+ patternProperties:
+ "^gpio@":
+ properties:
+ reg:
+ enum: [0, 1, 2, 3, 4, 6, 7, 8, 9, 10]
+
+unevaluatedProperties: false
+
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/input/input.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ keys@34 {
+ compatible = "adi,adp5589";
+ reg = <0x34>;
+
+ vcc-supply = <&vcc>;
+ interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+
+ adi,rows-mask = <0x13>;
+ adi,cols-mask = <0xf>;
+
+ linux,keymap = <
+ MATRIX_KEY(0x00, 0x00, KEY_1)
+ MATRIX_KEY(0x00, 0x01, KEY_2)
+ MATRIX_KEY(0x00, 0x02, KEY_3)
+ MATRIX_KEY(0x00, 0x03, KEY_4)
+ MATRIX_KEY(0x01, 0x00, KEY_5)
+ MATRIX_KEY(0x01, 0x01, KEY_6)
+ MATRIX_KEY(0x01, 0x02, KEY_7)
+ MATRIX_KEY(0x01, 0x03, KEY_8)
+ MATRIX_KEY(0x04, 0x00, KEY_9)
+ MATRIX_KEY(0x04, 0x01, KEY_A)
+ MATRIX_KEY(0x04, 0x02, KEY_B)
+ MATRIX_KEY(0x04, 0x03, KEY_C)
+ >;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ gpio@10 {
+ reg = <10>;
+ adi,pull-up-ohms = <100000>;
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 7913ca9b6b54020c58e387b3618922386ce03763..35238b1d89e65bfed09e1a1a5b421a07555f6351 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -38,12 +38,6 @@ properties:
- ad,adm9240
# AD5110 - Nonvolatile Digital Potentiometer
- adi,ad5110
- # Analog Devices ADP5585 Keypad Decoder and I/O Expansion
- - adi,adp5585
- # Analog Devices ADP5585 Keypad Decoder and I/O Expansion with support for Row5
- - adi,adp5585-02
- # Analog Devices ADP5589 Keypad Decoder and I/O Expansion
- - adi,adp5589
# Analog Devices LT7182S Dual Channel 6A, 20V PolyPhase Step-Down Silent Switcher
- adi,lt7182s
# AMS iAQ-Core VOC Sensor
diff --git a/MAINTAINERS b/MAINTAINERS
index a533d882b0022fd7580b829b68d846d319a4a8a7..120a1867b8f716ae11bffedab94a938c25888457 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -545,6 +545,15 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/input/adi,adp5588.yaml
F: drivers/input/keyboard/adp5588-keys.c
+ADP5589 QWERTY KEYPAD AND IO EXPANDER DRIVER (ADP5589/ADP5585)
+M: Michael Hennerich <michael.hennerich@analog.com>
+M: Nuno Sa <nuno.sa@analog.com>
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/input/adi,adp5589.yaml
+F: drivers/input/keyboard/adp5589-keys.c
+F: include/linux/input/adp5589.h
+
ADP8860 BACKLIGHT DRIVER (ADP8860/ADP8861/ADP8863)
M: Michael Hennerich <michael.hennerich@analog.com>
S: Supported
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 05/13] dt-bindings: input: Document adp5589 and similar devices
2024-10-01 13:41 ` [PATCH 05/13] dt-bindings: input: Document adp5589 and similar devices Nuno Sa
@ 2024-10-02 20:58 ` Rob Herring
2024-10-08 7:01 ` Nuno Sá
0 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2024-10-02 20:58 UTC (permalink / raw)
To: Nuno Sa
Cc: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree
On Tue, Oct 01, 2024 at 03:41:36PM +0200, Nuno Sa wrote:
> Add device tree bindings for the adp5589 keypad (and similar devices). The
> ADP5585 family has small differences like the lack of the unlock
> function and less pins (cols x rows) for the keymap.
>
> As there's no MAINTAINERS entry for these devices, add one. Also to note
> that these devices were removed from trivial-devices.yaml.
>
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
> .../devicetree/bindings/input/adi,adp5589.yaml | 310 +++++++++++++++++++++
> .../devicetree/bindings/trivial-devices.yaml | 6 -
> MAINTAINERS | 9 +
> 3 files changed, 319 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/adi,adp5589.yaml b/Documentation/devicetree/bindings/input/adi,adp5589.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..bdbc79758a0390952c8363ec28f48057dab929a9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/adi,adp5589.yaml
> @@ -0,0 +1,310 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/adi,adp5589.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADP5589 and similar Keypad Controllers
> +
> +maintainers:
> + - Nuno Sa <nuno.sa@analog.com>
> + - Michael Hennerich <michael.hennerich@analog.com>
> +
> +description: |
> + Analog Devices Mobile I/O Expander and QWERTY Keypad Controllers
> + - https://www.analog.com/media/en/technical-documentation/data-sheets/ADP5589.pdf
> + - https://www.analog.com/media/en/technical-documentation/data-sheets/ADP5585.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adp5589
> + - adi,adp5585
> + - adi,adp5585-2
> +
> + reg:
> + maxItems: 1
> +
> + vcc-supply: true
> +
> + interrupts:
> + maxItems: 1
> +
> + gpio-controller:
> + description:
> + This property applies if there are pins not used in the keypad.
> +
> + '#gpio-cells':
> + const: 2
> +
> + interrupt-controller:
> + description:
> + This property applies if there are pins not used in the keypad.
> +
> + '#interrupt-cells':
> + const: 2
> +
> + adi,cols-mask:
> + description:
> + Defines the number of pins (columns) being used ad part of the keymap. As
> + the device is fully configurable and we can have holes in the columns
> + being used, this is given as mask.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x1
> + maximum: 0x3f
> +
> + adi,rows-mask:
> + description:
> + Defines the number of pins (rows) being used ad part of the keymap. As
> + the device is fully configurable and we can have holes in the rows being
> + used, this is given as mask.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0x1
> + maximum: 0xff
> +
> + adi,key-poll-ms:
> + description: Configure time between consecutive scan cycles.
> + enum: [10, 20, 30, 40]
> + default: 10
> +
> + adi,unlock-keys:
> + description:
> + Specifies a maximum of 2 keys that can be used to unlock the keypad.
> + If this property is set, the keyboard will be locked and only unlocked
> + after these keys are pressed. The value 127 serves as a wildcard which
> + means any key can be used for unlocking.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
You probably don't allow repeated key values? If so, then you can add
'uniqueItems: true' to enforce that.
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 88
> + - minimum: 97
> + maximum: 115
> + - const: 127
> +
> + adi,unlock-trigger-sec:
> + description:
> + Defines the time in which the second unlock event must occur after the
> + first unlock event has occurred.
> + maximum: 7
> + default: 0
> +
> + adi,reset1-keys:
> + description:
> + Defines the trigger events (key presses) that can generate reset
> + conditions one the reset1 block.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 3
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 88
> + - minimum: 97
> + maximum: 115
> +
> + adi,reset2-keys:
> + description:
> + Defines the trigger events (key presses) that can generate reset
> + conditions one the reset2 block.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 88
> + - minimum: 97
> + maximum: 115
> +
> + adi,reset1-active-high:
> + description: Sets the reset1 signal as active high.
> + type: boolean
> +
> + adi,reset2-active-high:
> + description: Sets the reset2 signal as active high.
> + type: boolean
> +
> + adi,rst-passtrough-enable:
passthrough
> + description: Allows the RST pin to override (OR with) the reset1 signal.
> + type: boolean
> +
> + adi,reset-trigger-ms:
> + description:
> + Defines the length of time that the reset events must be active before a
> + reset signal is generated. All events must be active at the same time for
> + the same duration.
> + enum: [0, 1000, 1500, 2000, 2500, 3000, 3500, 4000]
> + default: 0
> +
> + adi,reset-pulse-width-us:
> + description: Defines the pulse width of the reset signals.
> + enum: [500, 1000, 2000, 10000]
> + default: 500
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> +patternProperties:
> + "^gpio@":
'gpio' for node name is for gpio-controllers which this is not.
> + type: object
> + additionalProperties: false
> +
> + properties:
> + reg:
> + description: The GPIO number being configured.
> + maximum: 18
> +
> + adi,pull-up-ohms:
> + description: The pullup resistor to be used.
> + enum: [100000, 300000]
> + default: 300000
Key mode doesn't have a pull-up?
Kind of overkill to have a node for 1 property. I'd probably just make
this an array of 18 entries for all pins using 0 or something if you
don't want to configure pins.
> +
> + required:
> + - reg
> +
> +dependencies:
> + adi,rows-mask:
> + - linux,keymap
> + - adi,cols-mask
> + adi,cols-mask:
> + - linux,keymap
> + - adi,rows-mask
> + linux,keymap:
> + - adi,rows-mask
> + - adi,cols-mask
> + - interrupts
> + interrupt-controller:
> + - interrupts
> + adi,unlock-trigger-sec:
> + - adi,unlock-keys
> + adi,reset1-active-high:
> + - adi,reset1-keys
> + adi,rst-passtrough-enable:
> + - adi,reset1-keys
> + adi,reset2-active-high:
> + - adi,reset2-keys
> +
> +required:
> + - compatible
> + - reg
> + - vcc-supply
> +
> +allOf:
> + - $ref: matrix-keymap.yaml#
> + - $ref: input.yaml#
> + - if:
> + properties:
> + compatible:
> + enum:
> + - adi,adp5585-2
> + then:
> + properties:
> + adi,unlock-keys: false
> + adi,unlock-trigger-sec: false
> + adi,rows-mask:
> + maximum: 0x2f
> + adi,cols-mask:
> + maximum: 0x1f
> + adi,reset1-keys:
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 30
> + - minimum: 37
> + maximum: 47
> + adi,reset2-keys:
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 30
> + - minimum: 37
> + maximum: 47
> + patternProperties:
> + "^gpio@":
> + properties:
> + reg:
> + maximum: 10
> + - if:
> + properties:
> + compatible:
> + enum:
> + - adi,adp5585
> + then:
> + properties:
> + adi,unlock-keys: false
> + adi,unlock-trigger-sec: false
> + adi,rows-mask:
> + maximum: 0x1f
> + adi,cols-mask:
> + maximum: 0x1f
> + adi,reset1-keys:
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 25
> + - enum: [37, 38, 39, 40, 41, 43, 44, 45, 46, 47]
> + adi,reset2-keys:
> + items:
> + anyOf:
> + - minimum: 1
> + maximum: 25
> + - enum: [37, 38, 39, 40, 41, 43, 44, 45, 46, 47]
> + patternProperties:
> + "^gpio@":
> + properties:
> + reg:
> + enum: [0, 1, 2, 3, 4, 6, 7, 8, 9, 10]
> +
> +unevaluatedProperties: false
> +
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/input/input.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + keys@34 {
> + compatible = "adi,adp5589";
> + reg = <0x34>;
> +
> + vcc-supply = <&vcc>;
> + interrupts = <21 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-parent = <&gpio>;
> +
> + adi,rows-mask = <0x13>;
> + adi,cols-mask = <0xf>;
> +
> + linux,keymap = <
> + MATRIX_KEY(0x00, 0x00, KEY_1)
> + MATRIX_KEY(0x00, 0x01, KEY_2)
> + MATRIX_KEY(0x00, 0x02, KEY_3)
> + MATRIX_KEY(0x00, 0x03, KEY_4)
> + MATRIX_KEY(0x01, 0x00, KEY_5)
> + MATRIX_KEY(0x01, 0x01, KEY_6)
> + MATRIX_KEY(0x01, 0x02, KEY_7)
> + MATRIX_KEY(0x01, 0x03, KEY_8)
> + MATRIX_KEY(0x04, 0x00, KEY_9)
> + MATRIX_KEY(0x04, 0x01, KEY_A)
> + MATRIX_KEY(0x04, 0x02, KEY_B)
> + MATRIX_KEY(0x04, 0x03, KEY_C)
> + >;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio@10 {
> + reg = <10>;
> + adi,pull-up-ohms = <100000>;
> + };
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 7913ca9b6b54020c58e387b3618922386ce03763..35238b1d89e65bfed09e1a1a5b421a07555f6351 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -38,12 +38,6 @@ properties:
> - ad,adm9240
> # AD5110 - Nonvolatile Digital Potentiometer
> - adi,ad5110
> - # Analog Devices ADP5585 Keypad Decoder and I/O Expansion
> - - adi,adp5585
> - # Analog Devices ADP5585 Keypad Decoder and I/O Expansion with support for Row5
> - - adi,adp5585-02
> - # Analog Devices ADP5589 Keypad Decoder and I/O Expansion
> - - adi,adp5589
> # Analog Devices LT7182S Dual Channel 6A, 20V PolyPhase Step-Down Silent Switcher
> - adi,lt7182s
> # AMS iAQ-Core VOC Sensor
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a533d882b0022fd7580b829b68d846d319a4a8a7..120a1867b8f716ae11bffedab94a938c25888457 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -545,6 +545,15 @@ W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/input/adi,adp5588.yaml
> F: drivers/input/keyboard/adp5588-keys.c
>
> +ADP5589 QWERTY KEYPAD AND IO EXPANDER DRIVER (ADP5589/ADP5585)
> +M: Michael Hennerich <michael.hennerich@analog.com>
> +M: Nuno Sa <nuno.sa@analog.com>
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/devicetree/bindings/input/adi,adp5589.yaml
> +F: drivers/input/keyboard/adp5589-keys.c
> +F: include/linux/input/adp5589.h
> +
> ADP8860 BACKLIGHT DRIVER (ADP8860/ADP8861/ADP8863)
> M: Michael Hennerich <michael.hennerich@analog.com>
> S: Supported
>
> --
> 2.46.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 05/13] dt-bindings: input: Document adp5589 and similar devices
2024-10-02 20:58 ` Rob Herring
@ 2024-10-08 7:01 ` Nuno Sá
0 siblings, 0 replies; 28+ messages in thread
From: Nuno Sá @ 2024-10-08 7:01 UTC (permalink / raw)
To: Rob Herring, Nuno Sa
Cc: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree
On Wed, 2024-10-02 at 15:58 -0500, Rob Herring wrote:
> On Tue, Oct 01, 2024 at 03:41:36PM +0200, Nuno Sa wrote:
> > Add device tree bindings for the adp5589 keypad (and similar devices). The
> > ADP5585 family has small differences like the lack of the unlock
> > function and less pins (cols x rows) for the keymap.
> >
> > As there's no MAINTAINERS entry for these devices, add one. Also to note
> > that these devices were removed from trivial-devices.yaml.
> >
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> > .../devicetree/bindings/input/adi,adp5589.yaml | 310
> > +++++++++++++++++++++
> > .../devicetree/bindings/trivial-devices.yaml | 6 -
> > MAINTAINERS | 9 +
> > 3 files changed, 319 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/adi,adp5589.yaml
> > b/Documentation/devicetree/bindings/input/adi,adp5589.yaml
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..bdbc79758a0390952c8363ec28f48057da
> > b929a9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/adi,adp5589.yaml
> > @@ -0,0 +1,310 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/adi,adp5589.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADP5589 and similar Keypad Controllers
> > +
> > +maintainers:
> > + - Nuno Sa <nuno.sa@analog.com>
> > + - Michael Hennerich <michael.hennerich@analog.com>
> > +
> > +description: |
> > + Analog Devices Mobile I/O Expander and QWERTY Keypad Controllers
> > + -
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ADP5589.pdf
> > + -
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ADP5585.pdf
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,adp5589
> > + - adi,adp5585
> > + - adi,adp5585-2
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + vcc-supply: true
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + gpio-controller:
> > + description:
> > + This property applies if there are pins not used in the keypad.
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > + interrupt-controller:
> > + description:
> > + This property applies if there are pins not used in the keypad.
> > +
> > + '#interrupt-cells':
> > + const: 2
> > +
> > + adi,cols-mask:
> > + description:
> > + Defines the number of pins (columns) being used ad part of the
> > keymap. As
> > + the device is fully configurable and we can have holes in the columns
> > + being used, this is given as mask.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x1
> > + maximum: 0x3f
> > +
> > + adi,rows-mask:
> > + description:
> > + Defines the number of pins (rows) being used ad part of the keymap.
> > As
> > + the device is fully configurable and we can have holes in the rows
> > being
> > + used, this is given as mask.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0x1
> > + maximum: 0xff
> > +
> > + adi,key-poll-ms:
> > + description: Configure time between consecutive scan cycles.
> > + enum: [10, 20, 30, 40]
> > + default: 10
> > +
> > + adi,unlock-keys:
> > + description:
> > + Specifies a maximum of 2 keys that can be used to unlock the keypad.
> > + If this property is set, the keyboard will be locked and only
> > unlocked
> > + after these keys are pressed. The value 127 serves as a wildcard
> > which
> > + means any key can be used for unlocking.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + minItems: 1
> > + maxItems: 2
>
> You probably don't allow repeated key values? If so, then you can add
> 'uniqueItems: true' to enforce that.
>
Oh nice. Indeed repeated keys make no sense...
> > + items:
> > + anyOf:
> > + - minimum: 1
> > + maximum: 88
> > + - minimum: 97
> > + maximum: 115
> > + - const: 127
> > +
> > + adi,unlock-trigger-sec:
> > + description:
> > + Defines the time in which the second unlock event must occur after
> > the
> > + first unlock event has occurred.
> > + maximum: 7
> > + default: 0
> > +
> > + adi,reset1-keys:
> > + description:
> > + Defines the trigger events (key presses) that can generate reset
> > + conditions one the reset1 block.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + minItems: 1
> > + maxItems: 3
> > + items:
> > + anyOf:
> > + - minimum: 1
> > + maximum: 88
> > + - minimum: 97
> > + maximum: 115
> > +
> > + adi,reset2-keys:
> > + description:
> > + Defines the trigger events (key presses) that can generate reset
> > + conditions one the reset2 block.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + minItems: 1
> > + maxItems: 2
> > + items:
> > + anyOf:
> > + - minimum: 1
> > + maximum: 88
> > + - minimum: 97
> > + maximum: 115
> > +
> > + adi,reset1-active-high:
> > + description: Sets the reset1 signal as active high.
> > + type: boolean
> > +
> > + adi,reset2-active-high:
> > + description: Sets the reset2 signal as active high.
> > + type: boolean
> > +
> > + adi,rst-passtrough-enable:
>
> passthrough
>
> > + description: Allows the RST pin to override (OR with) the reset1
> > signal.
> > + type: boolean
> > +
> > + adi,reset-trigger-ms:
> > + description:
> > + Defines the length of time that the reset events must be active
> > before a
> > + reset signal is generated. All events must be active at the same time
> > for
> > + the same duration.
> > + enum: [0, 1000, 1500, 2000, 2500, 3000, 3500, 4000]
> > + default: 0
> > +
> > + adi,reset-pulse-width-us:
> > + description: Defines the pulse width of the reset signals.
> > + enum: [500, 1000, 2000, 10000]
> > + default: 500
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > +patternProperties:
> > + "^gpio@":
>
> 'gpio' for node name is for gpio-controllers which this is not.
Well this part is also a gpio-controllers but given your comment below, I'll
replace the child nodes for an array.
>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + reg:
> > + description: The GPIO number being configured.
> > + maximum: 18
> > +
> > + adi,pull-up-ohms:
> > + description: The pullup resistor to be used.
> > + enum: [100000, 300000]
> > + default: 300000
>
> Key mode doesn't have a pull-up?
>
Fair question... I'm taking the same approach as another similar part I
refactored a while ago. Which is, the pin bias is configured through the GPIO
subsystem when the pins are used as GPIOs. Seems to me the more sensible usecase
but truth to be said, there's nothing on the datasheet enforcing that...
Anyways, moving this into the array will "drop" this restriction at least in the
bindings.
- Nuno Sá
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 06/13] Input: adp5589-keys: add support for fw properties
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (4 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 05/13] dt-bindings: input: Document adp5589 and similar devices Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 14:59 ` Dmitry Torokhov
2024-10-01 13:41 ` [PATCH 07/13] Input: adp5589-keys: add guard() notation Nuno Sa
` (7 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Use firmware properties (eg: OF) to get the device specific
configuration. This change just replaces the platform data since there
was no platform using it and so, it makes no sense having both.
Special note to the GPIO bias and debounce settings that are now supported
as part of the gpio subsystem (using 'set_config()' callback).
While at it, also allow GPIO's to be usable even if no keymap is given.
Effectively this means that an interrupt is no longer mandatory. Only in
case:
1) Keymapd is being defined;
2) Or GPIs are being used as gpio-keys and the interrupt-controller is
present.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
MAINTAINERS | 1 -
drivers/input/keyboard/Kconfig | 1 +
drivers/input/keyboard/adp5589-keys.c | 858 ++++++++++++++++++++++++++--------
include/linux/input/adp5589.h | 178 -------
4 files changed, 656 insertions(+), 382 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 120a1867b8f716ae11bffedab94a938c25888457..70170272a342c4191756615e1062d1b6b39f9767 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -552,7 +552,6 @@ S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/input/adi,adp5589.yaml
F: drivers/input/keyboard/adp5589-keys.c
-F: include/linux/input/adp5589.h
ADP8860 BACKLIGHT DRIVER (ADP8860/ADP8861/ADP8863)
M: Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 33ab9a178983b836a2724f870ebb9718b44da18a..9d03118a933c8d88002d63c576427afa5cd499ca 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -55,6 +55,7 @@ config KEYBOARD_ADP5589
depends on I2C
select GPIOLIB
select GPIOLIB_IRQCHIP
+ select INPUT_MATRIXKMAP
help
Say Y here if you want to use a ADP5585/ADP5589 attached to your
system I2C bus.
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index f107ed08fc787d31c09a5177395085548a6fadd7..d2c99f5cfee6a6b8adf4e3841fc235588632017b 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -7,21 +7,42 @@
* Copyright (C) 2010-2011 Analog Devices Inc.
*/
+#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/minmax.h>
#include <linux/module.h>
+#include <linux/mod_devicetable.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/workqueue.h>
#include <linux/errno.h>
#include <linux/pm.h>
#include <linux/pm_wakeirq.h>
-#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
#include <linux/i2c.h>
#include <linux/gpio/driver.h>
#include <linux/slab.h>
+#include <linux/units.h>
-#include <linux/input/adp5589.h>
+/*
+ * ADP5589 specific GPI and Keymap defines
+ */
+#define ADP5589_KEYMAPSIZE 88
+
+#define ADP5589_COL_SHIFT 8
+#define ADP5589_MAX_ROW_NUM 7
+#define ADP5589_MAX_COL_NUM 10
+/*
+ * ADP5585 specific GPI and Keymap defines
+ */
+#define ADP5585_KEYMAPSIZE 30
+
+#define ADP5585_COL_SHIFT 6
+#define ADP5585_MAX_ROW_NUM 5
+#define ADP5585_MAX_COL_NUM 4
/* ADP5589/ADP5585 Common Registers */
#define ADP5589_5_ID 0x00
@@ -188,6 +209,12 @@
#define C4_EXTEND_CFG BIT(6) /* RESET2 */
#define R4_EXTEND_CFG BIT(5) /* RESET1 */
+#define RESET1_POL BIT(7)
+#define RESET2_POL BIT(6)
+#define RST_PASSTHRU_EN BIT(5)
+#define RESET_TRIGGER_TIME GENMASK(4, 2)
+#define RESET_PULSE_WIDTH GENMASK(1, 0)
+
/* LOCK_CFG */
#define LOCK_EN BIT(0)
@@ -202,25 +229,18 @@
#define ADP5589_MAXGPIO 19
#define ADP5585_MAXGPIO 11 /* 10 on the ADP5585-01, 11 on ADP5585-02 */
-enum {
- ADP5589,
- ADP5585_01,
- ADP5585_02
-};
+/* As needed for the matrix parsing code */
+#define ADP5589_MAX_KEYMAPSIZE 123
+
+#define ADP5589_MAX_UNLOCK_TIME_SEC 7
struct adp_constants {
u8 maxgpio;
u8 keymapsize;
- u8 gpi_pin_row_base;
- u8 gpi_pin_row_end;
- u8 gpi_pin_col_base;
u8 gpi_pin_base;
u8 gpi_pin_end;
- u8 gpimapsize_max;
u8 max_row_num;
u8 max_col_num;
- u8 row_mask;
- u8 col_mask;
u8 col_shift;
u8 c4_extend_cfg;
u8 (*bank) (u8 offset);
@@ -230,6 +250,7 @@ struct adp_constants {
struct adp5589_info {
const struct adp_constants *var;
+ u8 rpull_banks;
bool support_row5;
bool is_adp5585;
};
@@ -238,13 +259,27 @@ struct adp5589_kpad {
struct i2c_client *client;
struct input_dev *input;
const struct adp5589_info *info;
- unsigned short keycode[ADP5589_KEYMAPSIZE];
+ unsigned short keycode[ADP5589_MAX_KEYMAPSIZE];
const struct adp5589_gpi_map *gpimap;
+ unsigned long pull_up_100k_map;
+ u32 keypad_en_mask;
+ u32 key_poll_time;
+ u32 row_shift;
+ u32 unlock_time;
+ u32 unlock_keys[2];
+ u32 nkeys_unlock;
+ u32 reset1_keys[3];
+ u32 nkeys_reset1;
+ u32 reset2_keys[2];
+ u32 nkeys_reset2;
unsigned short gpimapsize;
unsigned extend_cfg;
unsigned char gpiomap[ADP5589_MAXGPIO];
struct gpio_chip gc;
struct mutex gpio_lock; /* Protect cached dir, dat_out */
+ u8 reset_cfg;
+ u8 rpull[5];
+ u8 debounce_dis[3];
u8 dat_out[3];
u8 dir[3];
u8 int_en[3];
@@ -276,17 +311,11 @@ static unsigned char adp5589_reg(unsigned char reg)
static const struct adp_constants const_adp5589 = {
.maxgpio = ADP5589_MAXGPIO,
.keymapsize = ADP5589_KEYMAPSIZE,
- .gpi_pin_row_base = ADP5589_GPI_PIN_ROW_BASE,
- .gpi_pin_row_end = ADP5589_GPI_PIN_ROW_END,
- .gpi_pin_col_base = ADP5589_GPI_PIN_COL_BASE,
- .gpi_pin_base = ADP5589_GPI_PIN_BASE,
- .gpi_pin_end = ADP5589_GPI_PIN_END,
- .gpimapsize_max = ADP5589_GPIMAPSIZE_MAX,
+ .gpi_pin_base = 97,
+ .gpi_pin_end = 115,
.c4_extend_cfg = 12,
.max_row_num = ADP5589_MAX_ROW_NUM,
.max_col_num = ADP5589_MAX_COL_NUM,
- .row_mask = ADP5589_ROW_MASK,
- .col_mask = ADP5589_COL_MASK,
.col_shift = ADP5589_COL_SHIFT,
.bank = adp5589_bank,
.bit = adp5589_bit,
@@ -357,17 +386,11 @@ static unsigned char adp5585_reg(unsigned char reg)
static const struct adp_constants const_adp5585 = {
.maxgpio = ADP5585_MAXGPIO,
.keymapsize = ADP5585_KEYMAPSIZE,
- .gpi_pin_row_base = ADP5585_GPI_PIN_ROW_BASE,
- .gpi_pin_row_end = ADP5585_GPI_PIN_ROW_END,
- .gpi_pin_col_base = ADP5585_GPI_PIN_COL_BASE,
- .gpi_pin_base = ADP5585_GPI_PIN_BASE,
- .gpi_pin_end = ADP5585_GPI_PIN_END,
- .gpimapsize_max = ADP5585_GPIMAPSIZE_MAX,
+ .gpi_pin_base = 37,
+ .gpi_pin_end = 47,
.c4_extend_cfg = 10,
.max_row_num = ADP5585_MAX_ROW_NUM,
.max_col_num = ADP5585_MAX_COL_NUM,
- .row_mask = ADP5585_ROW_MASK,
- .col_mask = ADP5585_COL_MASK,
.col_shift = ADP5585_COL_SHIFT,
.bank = adp5585_bank,
.bit = adp5585_bit,
@@ -474,8 +497,99 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
return ret;
}
-static int adp5589_build_gpiomap(struct adp5589_kpad *kpad,
- const struct adp5589_kpad_platform_data *pdata)
+static const u8 adp5589_rpull_masks[] = {
+ GENMASK(1, 0),
+ GENMASK(3, 2),
+ GENMASK(5, 4),
+ GENMASK(7, 6)
+};
+
+static int adp5589_gpio_set_bias(struct adp5589_kpad *kpad, unsigned int pin,
+ enum pin_config_param cfg)
+{
+ unsigned int bank, msk;
+ int error;
+ u8 val;
+
+ mutex_lock(&kpad->gpio_lock);
+
+ if (cfg == PIN_CONFIG_BIAS_PULL_UP)
+ val = test_bit(pin, &kpad->pull_up_100k_map) ? 2 : 0;
+ else if (cfg == PIN_CONFIG_BIAS_PULL_DOWN)
+ val = 1;
+ else
+ val = 3;
+
+ /*
+ * For the BIAS configs, we have 4 pins per register. We need to take
+ * care for the adp5585 variant where we have a hole in the registers
+ * between rows and columns. Hence, we detect when a pin is a column and
+ * apply the proper offset and pin normalization.
+ */
+ if (kpad->info->is_adp5585 && pin >= kpad->info->var->col_shift) {
+ bank = 2 + (pin - kpad->info->var->col_shift) / 4;
+ msk = adp5589_rpull_masks[(pin - kpad->info->var->col_shift) % 4];
+ } else {
+ bank = pin / 4;
+ msk = adp5589_rpull_masks[pin % 4];
+ }
+
+ val <<= __bf_shf(msk);
+ kpad->rpull[bank] = (kpad->rpull[bank] & ~msk) | (val & msk);
+
+ error = adp5589_write(kpad->client,
+ kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + bank,
+ kpad->rpull[bank]);
+
+ mutex_unlock(&kpad->gpio_lock);
+
+ return error;
+}
+
+static int adp5589_gpio_set_config(struct gpio_chip *chip, unsigned int off,
+ unsigned long config)
+{
+ struct adp5589_kpad *kpad = gpiochip_get_data(chip);
+ unsigned int bank, bit, reg, pin = kpad->gpiomap[off];
+ unsigned int cfg = pinconf_to_config_param(config);
+ unsigned int val;
+ int error;
+
+ switch (cfg) {
+ case PIN_CONFIG_BIAS_PULL_UP:
+ fallthrough;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ fallthrough;
+ case PIN_CONFIG_BIAS_DISABLE:
+ return adp5589_gpio_set_bias(kpad, pin, cfg);
+ case PIN_CONFIG_INPUT_DEBOUNCE:
+ bank = kpad->info->var->bank(pin);
+ bit = kpad->info->var->bit(pin);
+ reg = kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A) + bank;
+
+ val = pinconf_to_config_argument(config);
+ if (val && val != 50) {
+ dev_err(&kpad->client->dev, "Debounce needs to be 50us(%u)\n",
+ val);
+ return -EINVAL;
+ }
+
+ mutex_lock(&kpad->gpio_lock);
+ if (!val)
+ kpad->debounce_dis[bank] |= bit;
+ else
+ kpad->debounce_dis[bank] &= bit;
+
+ error = adp5589_write(kpad->client, reg,
+ kpad->debounce_dis[bank]);
+ mutex_unlock(&kpad->gpio_lock);
+ return error;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int adp5589_build_gpiomap(struct adp5589_kpad *kpad)
{
bool pin_used[ADP5589_MAXGPIO];
int n_unused = 0;
@@ -484,7 +598,7 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad,
memset(pin_used, false, sizeof(pin_used));
for (i = 0; i < kpad->info->var->maxgpio; i++)
- if (pdata->keypad_en_mask & BIT(i))
+ if (kpad->keypad_en_mask & BIT(i))
pin_used[i] = true;
if (kpad->extend_cfg & R4_EXTEND_CFG)
@@ -578,16 +692,11 @@ static const struct irq_chip adp5589_irq_chip = {
static int adp5589_gpio_add(struct adp5589_kpad *kpad)
{
struct device *dev = &kpad->client->dev;
- const struct adp5589_kpad_platform_data *pdata = dev_get_platdata(dev);
- const struct adp5589_gpio_platform_data *gpio_data = pdata->gpio_data;
struct gpio_irq_chip *girq;
int i, error;
- if (!gpio_data)
- return 0;
-
kpad->gc.parent = dev;
- kpad->gc.ngpio = adp5589_build_gpiomap(kpad, pdata);
+ kpad->gc.ngpio = adp5589_build_gpiomap(kpad);
if (kpad->gc.ngpio == 0) {
dev_info(dev, "No unused gpios left to export\n");
return 0;
@@ -597,16 +706,24 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
kpad->gc.direction_output = adp5589_gpio_direction_output;
kpad->gc.get = adp5589_gpio_get_value;
kpad->gc.set = adp5589_gpio_set_value;
+ kpad->gc.set_config = adp5589_gpio_set_config;
kpad->gc.can_sleep = 1;
- kpad->gc.base = gpio_data->gpio_start;
+ kpad->gc.base = -1;
kpad->gc.label = kpad->client->name;
kpad->gc.owner = THIS_MODULE;
- girq = &kpad->gc.irq;
- gpio_irq_chip_set_chip(girq, &adp5589_irq_chip);
- girq->handler = handle_bad_irq;
- girq->threaded = true;
+ if (device_property_present(dev, "interrupt-controller")) {
+ if (!kpad->client->irq) {
+ dev_err(dev, "Unable to serve as interrupt controller without IRQ\n");
+ return -EINVAL;
+ }
+
+ girq = &kpad->gc.irq;
+ gpio_irq_chip_set_chip(girq, &adp5589_irq_chip);
+ girq->handler = handle_bad_irq;
+ girq->threaded = true;
+ }
mutex_init(&kpad->gpio_lock);
@@ -619,8 +736,15 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) + i);
kpad->dir[i] = adp5589_read(kpad->client,
kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + i);
+ kpad->debounce_dis[i] = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A)
+ + i);
}
+ for (i = 0; i < kpad->info->rpull_banks; i++)
+ kpad->rpull[i] = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + i);
+
return 0;
}
@@ -686,12 +810,21 @@ static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
int key_press = key & KEY_EV_PRESSED;
if (key_val >= kpad->info->var->gpi_pin_base &&
- key_val <= kpad->info->var->gpi_pin_end)
+ key_val <= kpad->info->var->gpi_pin_end) {
/* gpio line used as IRQ source */
adp5589_gpio_irq_handle(kpad, key_val, key_press);
- else
+ } else {
+ int row = (key_val - 1) / (kpad->info->var->max_col_num + 1);
+ int col = (key_val - 1) % (kpad->info->var->max_col_num + 1);
+ int code = MATRIX_SCAN_CODE(row, col, kpad->row_shift);
+
+ dev_dbg_ratelimited(&kpad->client->dev,
+ "report key(%d) r(%d) c(%d) code(%d)\n",
+ key_val, row, col, kpad->keycode[code]);
+
input_report_key(kpad->input,
- kpad->keycode[key_val - 1], key_press);
+ kpad->keycode[code], key_press);
+ }
}
}
@@ -719,139 +852,72 @@ static irqreturn_t adp5589_irq(int irq, void *handle)
return IRQ_HANDLED;
}
-static int adp5589_get_evcode(struct adp5589_kpad *kpad, unsigned short key)
-{
- int i;
-
- for (i = 0; i < kpad->info->var->keymapsize; i++)
- if (key == kpad->keycode[i])
- return (i + 1) | KEY_EV_PRESSED;
-
- dev_err(&kpad->client->dev, "RESET/UNLOCK key not in keycode map\n");
-
- return -EINVAL;
-}
-
static int adp5589_setup(struct adp5589_kpad *kpad)
{
struct i2c_client *client = kpad->client;
- const struct adp5589_kpad_platform_data *pdata =
- dev_get_platdata(&client->dev);
u8 (*reg)(u8) = kpad->info->var->reg;
- unsigned char pull_mask = 0;
int i, ret;
ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_A),
- pdata->keypad_en_mask & kpad->info->var->row_mask);
+ kpad->keypad_en_mask);
ret |= adp5589_write(client, reg(ADP5589_PIN_CONFIG_B),
- (pdata->keypad_en_mask >> kpad->info->var->col_shift) &
- kpad->info->var->col_mask);
+ kpad->keypad_en_mask >> kpad->info->var->col_shift);
if (!kpad->info->is_adp5585)
ret |= adp5589_write(client, ADP5589_PIN_CONFIG_C,
- (pdata->keypad_en_mask >> 16) & 0xFF);
+ kpad->keypad_en_mask >> 16);
- if (!kpad->info->is_adp5585 && pdata->en_keylock) {
- ret |= adp5589_write(client, ADP5589_UNLOCK1,
- pdata->unlock_key1);
- ret |= adp5589_write(client, ADP5589_UNLOCK2,
- pdata->unlock_key2);
- ret |= adp5589_write(client, ADP5589_UNLOCK_TIMERS,
- pdata->unlock_timer & LTIME_MASK);
- ret |= adp5589_write(client, ADP5589_LOCK_CFG, LOCK_EN);
+ /* unlock keys */
+ for (i = 0; i < kpad->nkeys_unlock; i++) {
+ ret = adp5589_write(client, ADP5589_UNLOCK1 + i,
+ kpad->unlock_keys[i] | KEY_EV_PRESSED);
+ if (ret)
+ return ret;
+ }
+
+ if (kpad->nkeys_unlock) {
+ ret = adp5589_write(client, ADP5589_UNLOCK_TIMERS,
+ kpad->unlock_time);
+ if (ret)
+ return ret;
+
+ ret = adp5589_write(client, ADP5589_LOCK_CFG, LOCK_EN);
+ if (ret)
+ return ret;
}
for (i = 0; i < KEYP_MAX_EVENT; i++)
ret |= adp5589_read(client, ADP5589_5_FIFO_1 + i);
- if (pdata->pull_dis_mask & pdata->pullup_en_100k &
- pdata->pullup_en_300k & pdata->pulldown_en_300k)
- dev_warn(&client->dev, "Conflicting pull resistor config\n");
-
- for (i = 0; i <= kpad->info->var->max_row_num; i++) {
- unsigned int val = 0, bit = BIT(i);
- if (pdata->pullup_en_300k & bit)
- val = 0;
- else if (pdata->pulldown_en_300k & bit)
- val = 1;
- else if (pdata->pullup_en_100k & bit)
- val = 2;
- else if (pdata->pull_dis_mask & bit)
- val = 3;
-
- pull_mask |= val << (2 * (i & 0x3));
-
- if (i % 4 == 3 || i == kpad->info->var->max_row_num) {
- ret |= adp5589_write(client, reg(ADP5585_RPULL_CONFIG_A)
- + (i >> 2), pull_mask);
- pull_mask = 0;
- }
+ /* reset keys */
+ for (i = 0; i < kpad->nkeys_reset1; i++) {
+ ret = adp5589_write(client, reg(ADP5589_RESET1_EVENT_A + i),
+ kpad->reset1_keys[i] | KEY_EV_PRESSED);
+ if (ret)
+ return ret;
}
- for (i = 0; i <= kpad->info->var->max_col_num; i++) {
- unsigned int val = 0, bit = BIT(i + kpad->info->var->col_shift);
- if (pdata->pullup_en_300k & bit)
- val = 0;
- else if (pdata->pulldown_en_300k & bit)
- val = 1;
- else if (pdata->pullup_en_100k & bit)
- val = 2;
- else if (pdata->pull_dis_mask & bit)
- val = 3;
-
- pull_mask |= val << (2 * (i & 0x3));
-
- if (i % 4 == 3 || i == kpad->info->var->max_col_num) {
- ret |= adp5589_write(client,
- reg(ADP5585_RPULL_CONFIG_C) +
- (i >> 2), pull_mask);
- pull_mask = 0;
- }
- }
-
- if (pdata->reset1_key_1 && pdata->reset1_key_2 && pdata->reset1_key_3) {
- ret |= adp5589_write(client, reg(ADP5589_RESET1_EVENT_A),
- adp5589_get_evcode(kpad,
- pdata->reset1_key_1));
- ret |= adp5589_write(client, reg(ADP5589_RESET1_EVENT_B),
- adp5589_get_evcode(kpad,
- pdata->reset1_key_2));
- ret |= adp5589_write(client, reg(ADP5589_RESET1_EVENT_C),
- adp5589_get_evcode(kpad,
- pdata->reset1_key_3));
- kpad->extend_cfg |= R4_EXTEND_CFG;
- }
-
- if (pdata->reset2_key_1 && pdata->reset2_key_2) {
- ret |= adp5589_write(client, reg(ADP5589_RESET2_EVENT_A),
- adp5589_get_evcode(kpad,
- pdata->reset2_key_1));
- ret |= adp5589_write(client, reg(ADP5589_RESET2_EVENT_B),
- adp5589_get_evcode(kpad,
- pdata->reset2_key_2));
- kpad->extend_cfg |= C4_EXTEND_CFG;
+ for (i = 0; i < kpad->nkeys_reset2; i++) {
+ ret = adp5589_write(client, reg(ADP5589_RESET2_EVENT_A + i),
+ kpad->reset2_keys[i] | KEY_EV_PRESSED);
+ if (ret)
+ return ret;
}
if (kpad->extend_cfg) {
- ret |= adp5589_write(client, reg(ADP5589_RESET_CFG),
- pdata->reset_cfg);
- ret |= adp5589_write(client, reg(ADP5589_PIN_CONFIG_D),
- kpad->extend_cfg);
+ ret = adp5589_write(client, reg(ADP5589_RESET_CFG),
+ kpad->reset_cfg);
+ if (ret)
+ return ret;
+
+ ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_D),
+ kpad->extend_cfg);
+ if (ret)
+ return ret;
}
- ret |= adp5589_write(client, reg(ADP5589_DEBOUNCE_DIS_A),
- pdata->debounce_dis_mask & kpad->info->var->row_mask);
-
- ret |= adp5589_write(client, reg(ADP5589_DEBOUNCE_DIS_B),
- (pdata->debounce_dis_mask >> kpad->info->var->col_shift)
- & kpad->info->var->col_mask);
-
- if (!kpad->info->is_adp5585)
- ret |= adp5589_write(client, reg(ADP5589_DEBOUNCE_DIS_C),
- (pdata->debounce_dis_mask >> 16) & 0xFF);
-
ret |= adp5589_write(client, reg(ADP5589_POLL_PTIME_CFG),
- pdata->scan_cycle_time & PTIME_MASK);
+ kpad->key_poll_time);
ret |= adp5589_write(client, ADP5589_5_INT_STATUS,
(kpad->info->is_adp5585 ? 0 : LOGIC2_INT) |
LOGIC1_INT | OVRFLOW_INT |
@@ -871,31 +937,425 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
return 0;
}
+static int adp5589_validate_key(struct adp5589_kpad *kpad, u32 key, bool is_gpi)
+{
+ struct i2c_client *client = kpad->client;
+ u32 row, col;
+
+ if (is_gpi) {
+ u32 gpi = key - kpad->info->var->gpi_pin_base;
+
+ if (gpi == 5 && !kpad->info->support_row5) {
+ dev_err(&client->dev,
+ "Invalid unlock/reset GPI(%u) not supported\n",
+ gpi);
+ return -EINVAL;
+ }
+
+ /* check if it's being used in the keypad */
+ if (BIT(gpi) & kpad->keypad_en_mask) {
+ dev_err(&client->dev,
+ "Invalid unlock/reset GPI(%u) being used in the keypad(%x)\n",
+ gpi, kpad->keypad_en_mask);
+ return -EINVAL;
+ }
+
+ return 0;
+ }
+
+ row = (key - 1) / (kpad->info->var->max_col_num + 1);
+ col = (key - 1) % (kpad->info->var->max_col_num + 1);
+
+ /* both the row and col must be part of the keypad */
+ if (BIT(row) & kpad->keypad_en_mask &&
+ BIT(col) << kpad->info->var->col_shift & kpad->keypad_en_mask)
+ return 0;
+
+ dev_err(&client->dev, "Invalid unlock/reset key(%u) not used in the keypad(%x)\n",
+ key, kpad->keypad_en_mask);
+
+ return -EINVAL;
+}
+
+static int adp5589_parse_key_array(struct adp5589_kpad *kpad, const char *prop,
+ u32 *keys, u32 *n_keys, u32 max_keys,
+ bool reset_key)
+{
+ struct i2c_client *client = kpad->client;
+ unsigned int key, max_keypad;
+ int ret;
+
+ ret = device_property_count_u32(&client->dev, prop);
+ if (ret < 0)
+ return 0;
+
+ *n_keys = ret;
+
+ if (kpad->info->is_adp5585 && !reset_key) {
+ dev_err(&client->dev, "Unlock keys not supported for adp5585\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (*n_keys > max_keys) {
+ dev_err(&client->dev, "Invalid number of keys(%u > %u) for %s\n",
+ *n_keys, max_keys, prop);
+ return -EINVAL;
+ }
+
+ ret = device_property_read_u32_array(&client->dev, prop, keys, *n_keys);
+ if (ret)
+ return ret;
+
+ max_keypad = (kpad->info->var->max_row_num + 1) * (kpad->info->var->max_col_num + 1);
+
+ for (key = 0; key < *n_keys; key++) {
+ /* part of the keypad... */
+ if (in_range(keys[key], 1, max_keypad)) {
+ /* is it part of the keypad?! */
+ ret = adp5589_validate_key(kpad, keys[key], false);
+ if (ret)
+ return ret;
+
+ continue;
+ }
+
+ /* part of gpio-keys... */
+ if (in_range(keys[key], kpad->info->var->gpi_pin_base,
+ kpad->info->var->maxgpio)) {
+ /* is the GPI being used as part of the keypad?! */
+ ret = adp5589_validate_key(kpad, keys[key], true);
+ if (ret)
+ return ret;
+
+ continue;
+ }
+
+ /* wildcard unlock event... */
+ if (!reset_key && keys[key] == 127)
+ continue;
+
+ dev_err(&client->dev, "Invalid key(%u) for %s\n", keys[key],
+ prop);
+
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int adp5589_unlock_parse(struct adp5589_kpad *kpad)
+{
+ struct i2c_client *client = kpad->client;
+ int error;
+
+ error = adp5589_parse_key_array(kpad, "adi,unlock-keys",
+ kpad->unlock_keys, &kpad->nkeys_unlock,
+ ARRAY_SIZE(kpad->unlock_keys), false);
+ if (error)
+ return error;
+ if (!kpad->nkeys_unlock)
+ /* no unlock keys */
+ return 0;
+
+ error = device_property_read_u32(&client->dev, "adi,unlock-trigger-sec",
+ &kpad->unlock_time);
+ if (!error) {
+ if (kpad->unlock_time > ADP5589_MAX_UNLOCK_TIME_SEC) {
+ dev_err(&client->dev, "Invalid unlock time(%u > %d)\n",
+ kpad->unlock_time, ADP5589_MAX_UNLOCK_TIME_SEC);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int adp5589_reset_parse(struct adp5589_kpad *kpad)
+{
+ struct i2c_client *client = kpad->client;
+ u32 prop_val;
+ int error;
+
+ error = adp5589_parse_key_array(kpad, "adi,reset1-keys",
+ kpad->reset1_keys, &kpad->nkeys_reset1,
+ ARRAY_SIZE(kpad->reset1_keys), true);
+ if (error)
+ return error;
+ if (kpad->nkeys_reset1 > 0) {
+ /*
+ * Then R4 is used as reset output. Make sure it's not being used
+ * in the keypad.
+ */
+ if (BIT(4) & kpad->keypad_en_mask) {
+ dev_err(&client->dev, "Row4 cannot be used if reset1 is used\n");
+ return -EINVAL;
+ }
+
+ kpad->extend_cfg = R4_EXTEND_CFG;
+ }
+
+ error = adp5589_parse_key_array(kpad, "adi,reset2-keys",
+ kpad->reset2_keys, &kpad->nkeys_reset2,
+ ARRAY_SIZE(kpad->reset2_keys), true);
+ if (error)
+ return error;
+ if (kpad->nkeys_reset2 > 0) {
+ /*
+ * Then C4 is used as reset output. Make sure it's not being used
+ * in the keypad.
+ */
+ if (BIT(kpad->info->var->c4_extend_cfg) & kpad->keypad_en_mask) {
+ dev_err(&client->dev, "Col4 cannot be used if reset2 is used\n");
+ return -EINVAL;
+ }
+
+ kpad->extend_cfg |= C4_EXTEND_CFG;
+ }
+
+ if (!kpad->nkeys_reset2 && !kpad->nkeys_reset1)
+ return 0;
+
+ if (device_property_read_bool(&client->dev, "adi,reset1-active-high"))
+ kpad->reset_cfg |= FIELD_PREP(RESET1_POL, 1);
+
+ if (device_property_read_bool(&client->dev, "adi,reset2-active-high"))
+ kpad->reset_cfg |= FIELD_PREP(RESET2_POL, 1);
+
+ if (device_property_read_bool(&client->dev, "adi,rst-passtrough-enable"))
+ kpad->reset_cfg |= FIELD_PREP(RST_PASSTHRU_EN, 1);
+
+ error = device_property_read_u32(&client->dev, "adi,reset-trigger-ms",
+ &prop_val);
+ if (!error) {
+ switch (prop_val) {
+ case 0:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 0);
+ break;
+ case 1000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 1);
+ break;
+ case 1500:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 2);
+ break;
+ case 2000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 3);
+ break;
+ case 2500:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 4);
+ break;
+ case 3000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 5);
+ break;
+ case 3500:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 6);
+ break;
+ case 4000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 7);
+ break;
+ default:
+ dev_err(&client->dev, "Invalid value(%u) for adi,reset-trigger-ms\n",
+ prop_val);
+ return -EINVAL;
+ }
+ }
+
+ error = device_property_read_u32(&client->dev,
+ "adi,reset-pulse-width-us", &prop_val);
+ if (!error) {
+ switch (prop_val) {
+ case 500:
+ kpad->reset_cfg |= FIELD_PREP(RESET_PULSE_WIDTH, 0);
+ break;
+ case 1000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_PULSE_WIDTH, 1);
+ break;
+ case 2000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_PULSE_WIDTH, 2);
+ break;
+ case 10000:
+ kpad->reset_cfg |= FIELD_PREP(RESET_PULSE_WIDTH, 3);
+ break;
+ default:
+ dev_err(&client->dev, "Invalid value(%u) for adi,reset-pulse-width-us\n",
+ prop_val);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int adp5589_gpio_parse(struct adp5589_kpad *kpad)
+{
+ struct i2c_client *client = kpad->client;
+ u32 reg, pullup;
+ int error;
+
+ device_for_each_child_node_scoped(&client->dev, child) {
+ error = fwnode_property_read_u32(child, "reg", ®);
+ if (error) {
+ dev_err(&client->dev, "Failed to get reg property\n");
+ return -EINVAL;
+ }
+
+ if (reg >= kpad->info->var->maxgpio) {
+ dev_err(&client->dev, "Invalid gpio(%u > %u)\n",
+ reg, kpad->info->var->maxgpio);
+ return -EINVAL;
+ }
+
+ if (BIT(reg) & kpad->keypad_en_mask) {
+ dev_err(&client->dev, "Invalid gpio(%u) used in keypad\n",
+ reg);
+ return -EINVAL;
+ }
+
+ if (reg == 5 && !kpad->info->support_row5) {
+ dev_err(&client->dev, "Invalid gpio(%u) not supported\n",
+ reg);
+ return -EINVAL;
+ }
+
+ /* Check if it's gpio4 and R4 is being used as reset */
+ if (kpad->extend_cfg & R4_EXTEND_CFG && reg == 4) {
+ dev_err(&client->dev, "Invalid gpio(%u) used as reset1\n",
+ reg);
+ return -EINVAL;
+ }
+
+ /* Check if the gpio is being used as reset2 */
+ if (kpad->extend_cfg & C4_EXTEND_CFG && reg == kpad->info->var->c4_extend_cfg) {
+ dev_err(&client->dev, "Invalid gpio(%u) used as reset2\n",
+ reg);
+ return -EINVAL;
+ }
+
+ error = fwnode_property_read_u32(child, "adi,pull-up-ohms",
+ &pullup);
+ if (!error) {
+ if (pullup != 100 * KILO && pullup != 300 * KILO) {
+ dev_err(&client->dev, "Invalid pullup resistor val(%u)",
+ pullup);
+ return -EINVAL;
+ }
+
+ if (pullup == 100 * KILO)
+ __set_bit(reg, &kpad->pull_up_100k_map);
+ }
+ }
+
+ return 0;
+}
+
+static int adp5589_parse_fw(struct adp5589_kpad *kpad)
+{
+ struct i2c_client *client = kpad->client;
+ u32 cols = 0, rows = 0, prop_val;
+ int error;
+
+ error = device_property_read_u32(&client->dev, "adi,cols-mask",
+ &prop_val);
+ if (!error) {
+ if (prop_val > GENMASK(kpad->info->var->max_col_num, 0)) {
+ dev_err(&client->dev, "Invalid column mask(%x)\n",
+ prop_val);
+ return -EINVAL;
+ }
+
+ kpad->keypad_en_mask = prop_val << kpad->info->var->col_shift;
+ /*
+ * Note that given that we get a mask (and the HW allows it), we
+ * can have holes in our keypad (eg: row0, row1 and row7 enabled).
+ * However, for the matrix parsing functions we need to pass the
+ * number of rows/cols as the maximum row/col used plus 1. This
+ * pretty much means we will also have holes in our SW keypad.
+ */
+ cols = fls(prop_val);
+ }
+
+ error = device_property_read_u32(&client->dev, "adi,rows-mask",
+ &prop_val);
+ if (!error) {
+ if (prop_val > GENMASK(kpad->info->var->max_row_num, 0)) {
+ dev_err(&client->dev, "Invalid row mask(%x)\n",
+ prop_val);
+ return -EINVAL;
+ }
+
+ if (prop_val & BIT(5) && !kpad->info->support_row5) {
+ dev_err(&client->dev, "Row5 not supported!\n");
+ return -EINVAL;
+ }
+
+ kpad->keypad_en_mask |= prop_val;
+ rows = fls(prop_val);
+ }
+
+ if (cols && !rows) {
+ dev_err(&client->dev, "Cannot have columns with no rows!\n");
+ return -EINVAL;
+ }
+
+ if (rows && !cols) {
+ dev_err(&client->dev, "Cannot have rows with no columns!\n");
+ return -EINVAL;
+ }
+
+ if (rows && cols) {
+ if (!client->irq) {
+ dev_err(&client->dev,
+ "Keymaps won't work without interrupts\n");
+ return -EINVAL;
+ }
+
+ error = matrix_keypad_build_keymap(NULL, NULL, rows, cols,
+ kpad->keycode, kpad->input);
+ if (error)
+ return error;
+
+ kpad->row_shift = get_count_order(cols);
+ }
+
+ if (device_property_present(&client->dev, "autorepeat"))
+ __set_bit(EV_REP, kpad->input->evbit);
+
+ error = device_property_read_u32(&client->dev, "adi,key-poll-ms",
+ &prop_val);
+ if (!error) {
+ switch (prop_val) {
+ case 10:
+ fallthrough;
+ case 20:
+ fallthrough;
+ case 30:
+ fallthrough;
+ case 40:
+ kpad->key_poll_time = prop_val / 10 - 1;
+ break;
+ default:
+ dev_err(&client->dev, "Invalid value(%u) for adi,key-poll-ms\n",
+ prop_val);
+ return -EINVAL;
+ }
+ }
+
+ error = adp5589_unlock_parse(kpad);
+ if (error)
+ return error;
+
+ error = adp5589_reset_parse(kpad);
+ if (error)
+ return error;
+
+ return adp5589_gpio_parse(kpad);
+}
+
static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
{
struct i2c_client *client = kpad->client;
- const struct adp5589_kpad_platform_data *pdata =
- dev_get_platdata(&client->dev);
struct input_dev *input;
- unsigned int i;
int error;
- if (!((pdata->keypad_en_mask & kpad->info->var->row_mask) &&
- (pdata->keypad_en_mask >> kpad->info->var->col_shift)) || !pdata->keymap) {
- dev_err(&client->dev, "no rows, cols or keymap from pdata\n");
- return -EINVAL;
- }
-
- if (pdata->keymapsize != kpad->info->var->keymapsize) {
- dev_err(&client->dev, "invalid keymapsize\n");
- return -EINVAL;
- }
-
- if (!client->irq) {
- dev_err(&client->dev, "no IRQ?\n");
- return -EINVAL;
- }
-
input = devm_input_allocate_device(&client->dev);
if (!input)
return -ENOMEM;
@@ -913,23 +1373,9 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
input->id.product = 0x0001;
input->id.version = revid;
- input->keycodesize = sizeof(kpad->keycode[0]);
- input->keycodemax = pdata->keymapsize;
- input->keycode = kpad->keycode;
-
- memcpy(kpad->keycode, pdata->keymap,
- pdata->keymapsize * input->keycodesize);
-
- /* setup input device */
- __set_bit(EV_KEY, input->evbit);
-
- if (pdata->repeat)
- __set_bit(EV_REP, input->evbit);
-
- for (i = 0; i < input->keycodemax; i++)
- if (kpad->keycode[i] <= KEY_MAX)
- __set_bit(kpad->keycode[i], input->keybit);
- __clear_bit(KEY_RESERVED, input->keybit);
+ error = adp5589_parse_fw(kpad);
+ if (error)
+ return error;
error = input_register_device(input);
if (error) {
@@ -937,6 +1383,9 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
return error;
}
+ if (!client->irq)
+ return 0;
+
error = devm_request_threaded_irq(&client->dev, client->irq,
NULL, adp5589_irq,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
@@ -960,24 +1409,25 @@ static void adp5589_clear_config(void *data)
static const struct adp5589_info adp5589_info = {
.var = &const_adp5589,
.support_row5 = true,
+ .rpull_banks = ADP5589_RPULL_CONFIG_E - ADP5589_RPULL_CONFIG_A + 1,
};
static const struct adp5589_info adp5585_info = {
.var = &const_adp5585,
.is_adp5585 = true,
+ .rpull_banks = ADP5585_RPULL_CONFIG_D - ADP5585_RPULL_CONFIG_A + 1,
};
static const struct adp5589_info adp5585_2_info = {
.var = &const_adp5585,
.is_adp5585 = true,
.support_row5 = true,
+ .rpull_banks = ADP5585_RPULL_CONFIG_D - ADP5585_RPULL_CONFIG_A + 1,
};
static int adp5589_probe(struct i2c_client *client)
{
struct adp5589_kpad *kpad;
- const struct adp5589_kpad_platform_data *pdata =
- dev_get_platdata(&client->dev);
unsigned int revid;
int error, ret;
@@ -987,11 +1437,6 @@ static int adp5589_probe(struct i2c_client *client)
return -EIO;
}
- if (!pdata) {
- dev_err(&client->dev, "no platform data?\n");
- return -EINVAL;
- }
-
kpad = devm_kzalloc(&client->dev, sizeof(*kpad), GFP_KERNEL);
if (!kpad)
return -ENOMEM;
@@ -1013,11 +1458,9 @@ static int adp5589_probe(struct i2c_client *client)
revid = (u8) ret & ADP5589_5_DEVICE_ID_MASK;
- if (pdata->keymapsize) {
- error = adp5589_keypad_add(kpad, revid);
- if (error)
- return error;
- }
+ error = adp5589_keypad_add(kpad, revid);
+ if (error)
+ return error;
error = adp5589_setup(kpad);
if (error)
@@ -1064,10 +1507,19 @@ static const struct i2c_device_id adp5589_id[] = {
MODULE_DEVICE_TABLE(i2c, adp5589_id);
+static const struct of_device_id adp5589_of_id[] = {
+ { .compatible = "adi,adp5589", .data = &adp5589_info},
+ { .compatible = "adi,adp5585", .data = &adp5585_info},
+ { .compatible = "adi,adp5585-2", .data = &adp5585_2_info},
+ {}
+};
+MODULE_DEVICE_TABLE(of, adp5589_of_id);
+
static struct i2c_driver adp5589_driver = {
.driver = {
.name = KBUILD_MODNAME,
.pm = pm_sleep_ptr(&adp5589_dev_pm_ops),
+ .of_match_table = adp5589_of_id,
},
.probe = adp5589_probe,
.id_table = adp5589_id,
diff --git a/include/linux/input/adp5589.h b/include/linux/input/adp5589.h
deleted file mode 100644
index 6185fc1ba30869c014b2d3e49e21d46b464cba6a..0000000000000000000000000000000000000000
--- a/include/linux/input/adp5589.h
+++ /dev/null
@@ -1,178 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Analog Devices ADP5589/ADP5585 I/O Expander and QWERTY Keypad Controller
- *
- * Copyright 2010-2011 Analog Devices Inc.
- */
-
-#ifndef _ADP5589_H
-#define _ADP5589_H
-
-/*
- * ADP5589 specific GPI and Keymap defines
- */
-
-#define ADP5589_KEYMAPSIZE 88
-
-#define ADP5589_GPI_PIN_ROW0 97
-#define ADP5589_GPI_PIN_ROW1 98
-#define ADP5589_GPI_PIN_ROW2 99
-#define ADP5589_GPI_PIN_ROW3 100
-#define ADP5589_GPI_PIN_ROW4 101
-#define ADP5589_GPI_PIN_ROW5 102
-#define ADP5589_GPI_PIN_ROW6 103
-#define ADP5589_GPI_PIN_ROW7 104
-#define ADP5589_GPI_PIN_COL0 105
-#define ADP5589_GPI_PIN_COL1 106
-#define ADP5589_GPI_PIN_COL2 107
-#define ADP5589_GPI_PIN_COL3 108
-#define ADP5589_GPI_PIN_COL4 109
-#define ADP5589_GPI_PIN_COL5 110
-#define ADP5589_GPI_PIN_COL6 111
-#define ADP5589_GPI_PIN_COL7 112
-#define ADP5589_GPI_PIN_COL8 113
-#define ADP5589_GPI_PIN_COL9 114
-#define ADP5589_GPI_PIN_COL10 115
-#define GPI_LOGIC1 116
-#define GPI_LOGIC2 117
-
-#define ADP5589_GPI_PIN_ROW_BASE ADP5589_GPI_PIN_ROW0
-#define ADP5589_GPI_PIN_ROW_END ADP5589_GPI_PIN_ROW7
-#define ADP5589_GPI_PIN_COL_BASE ADP5589_GPI_PIN_COL0
-#define ADP5589_GPI_PIN_COL_END ADP5589_GPI_PIN_COL10
-
-#define ADP5589_GPI_PIN_BASE ADP5589_GPI_PIN_ROW_BASE
-#define ADP5589_GPI_PIN_END ADP5589_GPI_PIN_COL_END
-
-#define ADP5589_GPIMAPSIZE_MAX (ADP5589_GPI_PIN_END - ADP5589_GPI_PIN_BASE + 1)
-
-/*
- * ADP5585 specific GPI and Keymap defines
- */
-
-#define ADP5585_KEYMAPSIZE 30
-
-#define ADP5585_GPI_PIN_ROW0 37
-#define ADP5585_GPI_PIN_ROW1 38
-#define ADP5585_GPI_PIN_ROW2 39
-#define ADP5585_GPI_PIN_ROW3 40
-#define ADP5585_GPI_PIN_ROW4 41
-#define ADP5585_GPI_PIN_ROW5 42
-#define ADP5585_GPI_PIN_COL0 43
-#define ADP5585_GPI_PIN_COL1 44
-#define ADP5585_GPI_PIN_COL2 45
-#define ADP5585_GPI_PIN_COL3 46
-#define ADP5585_GPI_PIN_COL4 47
-#define GPI_LOGIC 48
-
-#define ADP5585_GPI_PIN_ROW_BASE ADP5585_GPI_PIN_ROW0
-#define ADP5585_GPI_PIN_ROW_END ADP5585_GPI_PIN_ROW5
-#define ADP5585_GPI_PIN_COL_BASE ADP5585_GPI_PIN_COL0
-#define ADP5585_GPI_PIN_COL_END ADP5585_GPI_PIN_COL4
-
-#define ADP5585_GPI_PIN_BASE ADP5585_GPI_PIN_ROW_BASE
-#define ADP5585_GPI_PIN_END ADP5585_GPI_PIN_COL_END
-
-#define ADP5585_GPIMAPSIZE_MAX (ADP5585_GPI_PIN_END - ADP5585_GPI_PIN_BASE + 1)
-
-struct adp5589_gpi_map {
- unsigned short pin;
- unsigned short sw_evt;
-};
-
-/* scan_cycle_time */
-#define ADP5589_SCAN_CYCLE_10ms 0
-#define ADP5589_SCAN_CYCLE_20ms 1
-#define ADP5589_SCAN_CYCLE_30ms 2
-#define ADP5589_SCAN_CYCLE_40ms 3
-
-/* RESET_CFG */
-#define RESET_PULSE_WIDTH_500us 0
-#define RESET_PULSE_WIDTH_1ms 1
-#define RESET_PULSE_WIDTH_2ms 2
-#define RESET_PULSE_WIDTH_10ms 3
-
-#define RESET_TRIG_TIME_0ms (0 << 2)
-#define RESET_TRIG_TIME_1000ms (1 << 2)
-#define RESET_TRIG_TIME_1500ms (2 << 2)
-#define RESET_TRIG_TIME_2000ms (3 << 2)
-#define RESET_TRIG_TIME_2500ms (4 << 2)
-#define RESET_TRIG_TIME_3000ms (5 << 2)
-#define RESET_TRIG_TIME_3500ms (6 << 2)
-#define RESET_TRIG_TIME_4000ms (7 << 2)
-
-#define RESET_PASSTHRU_EN (1 << 5)
-#define RESET1_POL_HIGH (1 << 6)
-#define RESET1_POL_LOW (0 << 6)
-#define RESET2_POL_HIGH (1 << 7)
-#define RESET2_POL_LOW (0 << 7)
-
-/* ADP5589 Mask Bits:
- * C C C C C C C C C C C | R R R R R R R R
- * 1 9 8 7 6 5 4 3 2 1 0 | 7 6 5 4 3 2 1 0
- * 0
- * ---------------- BIT ------------------
- * 1 1 1 1 1 1 1 1 1 0 0 | 0 0 0 0 0 0 0 0
- * 8 7 6 5 4 3 2 1 0 9 8 | 7 6 5 4 3 2 1 0
- */
-
-#define ADP_ROW(x) (1 << (x))
-#define ADP_COL(x) (1 << (x + 8))
-#define ADP5589_ROW_MASK 0xFF
-#define ADP5589_COL_MASK 0xFF
-#define ADP5589_COL_SHIFT 8
-#define ADP5589_MAX_ROW_NUM 7
-#define ADP5589_MAX_COL_NUM 10
-
-/* ADP5585 Mask Bits:
- * C C C C C | R R R R R R
- * 4 3 2 1 0 | 5 4 3 2 1 0
- *
- * ---- BIT -- -----------
- * 1 0 0 0 0 | 0 0 0 0 0 0
- * 0 9 8 7 6 | 5 4 3 2 1 0
- */
-
-#define ADP5585_ROW_MASK 0x3F
-#define ADP5585_COL_MASK 0x1F
-#define ADP5585_ROW_SHIFT 0
-#define ADP5585_COL_SHIFT 6
-#define ADP5585_MAX_ROW_NUM 5
-#define ADP5585_MAX_COL_NUM 4
-
-#define ADP5585_ROW(x) (1 << ((x) & ADP5585_ROW_MASK))
-#define ADP5585_COL(x) (1 << (((x) & ADP5585_COL_MASK) + ADP5585_COL_SHIFT))
-
-/* Put one of these structures in i2c_board_info platform_data */
-
-struct adp5589_kpad_platform_data {
- unsigned keypad_en_mask; /* Keypad (Rows/Columns) enable mask */
- const unsigned short *keymap; /* Pointer to keymap */
- unsigned short keymapsize; /* Keymap size */
- bool repeat; /* Enable key repeat */
- bool en_keylock; /* Enable key lock feature (ADP5589 only)*/
- unsigned char unlock_key1; /* Unlock Key 1 (ADP5589 only) */
- unsigned char unlock_key2; /* Unlock Key 2 (ADP5589 only) */
- unsigned char unlock_timer; /* Time in seconds [0..7] between the two unlock keys 0=disable (ADP5589 only) */
- unsigned char scan_cycle_time; /* Time between consecutive scan cycles */
- unsigned char reset_cfg; /* Reset config */
- unsigned short reset1_key_1; /* Reset Key 1 */
- unsigned short reset1_key_2; /* Reset Key 2 */
- unsigned short reset1_key_3; /* Reset Key 3 */
- unsigned short reset2_key_1; /* Reset Key 1 */
- unsigned short reset2_key_2; /* Reset Key 2 */
- unsigned debounce_dis_mask; /* Disable debounce mask */
- unsigned pull_dis_mask; /* Disable all pull resistors mask */
- unsigned pullup_en_100k; /* Pull-Up 100k Enable Mask */
- unsigned pullup_en_300k; /* Pull-Up 300k Enable Mask */
- unsigned pulldown_en_300k; /* Pull-Down 300k Enable Mask */
- const struct adp5589_gpio_platform_data *gpio_data;
-};
-
-struct i2c_client; /* forward declaration */
-
-struct adp5589_gpio_platform_data {
- int gpio_start; /* GPIO Chip base # */
-};
-
-#endif
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 06/13] Input: adp5589-keys: add support for fw properties
2024-10-01 13:41 ` [PATCH 06/13] Input: adp5589-keys: add support for fw properties Nuno Sa
@ 2024-10-01 14:59 ` Dmitry Torokhov
2024-10-02 9:01 ` Nuno Sá
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2024-10-01 14:59 UTC (permalink / raw)
To: Nuno Sa
Cc: Mike Frysinger, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
On Tue, Oct 01, 2024 at 03:41:37PM +0200, Nuno Sa wrote:
> +
> + switch (cfg) {
> + case PIN_CONFIG_BIAS_PULL_UP:
> + fallthrough;
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + fallthrough;
> + case PIN_CONFIG_BIAS_DISABLE:
> + return adp5589_gpio_set_bias(kpad, pin, cfg);
I don't think you need to use "fallhrough" here, saying:
case PIN_CONFIG_BIAS_PULL_UP:
case PIN_CONFIG_BIAS_PULL_DOWN:
case PIN_CONFIG_BIAS_DISABLE:
return adp5589_gpio_set_bias(kpad, pin, cfg);
should not result in any warnings.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 06/13] Input: adp5589-keys: add support for fw properties
2024-10-01 14:59 ` Dmitry Torokhov
@ 2024-10-02 9:01 ` Nuno Sá
0 siblings, 0 replies; 28+ messages in thread
From: Nuno Sá @ 2024-10-02 9:01 UTC (permalink / raw)
To: Dmitry Torokhov, Nuno Sa
Cc: Mike Frysinger, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, devicetree
On Tue, 2024-10-01 at 07:59 -0700, Dmitry Torokhov wrote:
> On Tue, Oct 01, 2024 at 03:41:37PM +0200, Nuno Sa wrote:
> > +
> > + switch (cfg) {
> > + case PIN_CONFIG_BIAS_PULL_UP:
> > + fallthrough;
> > + case PIN_CONFIG_BIAS_PULL_DOWN:
> > + fallthrough;
> > + case PIN_CONFIG_BIAS_DISABLE:
> > + return adp5589_gpio_set_bias(kpad, pin, cfg);
>
> I don't think you need to use "fallhrough" here, saying:
>
> case PIN_CONFIG_BIAS_PULL_UP:
> case PIN_CONFIG_BIAS_PULL_DOWN:
> case PIN_CONFIG_BIAS_DISABLE:
> return adp5589_gpio_set_bias(kpad, pin, cfg);
>
> should not result in any warnings.
>
> Thanks.
>
Arghh sure... Will do that in v2.
- Nuno Sá
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 07/13] Input: adp5589-keys: add guard() notation
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (5 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 06/13] Input: adp5589-keys: add support for fw properties Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 08/13] Input: adp5589-keys: bail out on returned error Nuno Sa
` (6 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Using the cleanup.h APi makes the code path easier and less prone to
mistakes.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 54 +++++++++++++----------------------
1 file changed, 20 insertions(+), 34 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index d2c99f5cfee6a6b8adf4e3841fc235588632017b..19ebcb71fd5c7fcba6c7979ba9495179dd393808 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -8,6 +8,7 @@
*/
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/bitops.h>
#include <linux/bits.h>
#include <linux/minmax.h>
@@ -419,13 +420,12 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
int val;
- mutex_lock(&kpad->gpio_lock);
+ guard(mutex)(&kpad->gpio_lock);
if (kpad->dir[bank] & bit)
val = kpad->dat_out[bank];
else
val = adp5589_read(kpad->client,
kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank);
- mutex_unlock(&kpad->gpio_lock);
return !!(val & bit);
}
@@ -437,7 +437,7 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
- mutex_lock(&kpad->gpio_lock);
+ guard(mutex)(&kpad->gpio_lock);
if (val)
kpad->dat_out[bank] |= bit;
@@ -446,8 +446,6 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) +
bank, kpad->dat_out[bank]);
-
- mutex_unlock(&kpad->gpio_lock);
}
static int adp5589_gpio_direction_input(struct gpio_chip *chip, unsigned off)
@@ -455,18 +453,13 @@ static int adp5589_gpio_direction_input(struct gpio_chip *chip, unsigned off)
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
- int ret;
- mutex_lock(&kpad->gpio_lock);
+ guard(mutex)(&kpad->gpio_lock);
kpad->dir[bank] &= ~bit;
- ret = adp5589_write(kpad->client,
- kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
- kpad->dir[bank]);
-
- mutex_unlock(&kpad->gpio_lock);
-
- return ret;
+ return adp5589_write(kpad->client,
+ kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
+ kpad->dir[bank]);
}
static int adp5589_gpio_direction_output(struct gpio_chip *chip,
@@ -477,7 +470,7 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
int ret;
- mutex_lock(&kpad->gpio_lock);
+ guard(mutex)(&kpad->gpio_lock);
kpad->dir[bank] |= bit;
@@ -492,8 +485,6 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
kpad->dir[bank]);
- mutex_unlock(&kpad->gpio_lock);
-
return ret;
}
@@ -508,10 +499,9 @@ static int adp5589_gpio_set_bias(struct adp5589_kpad *kpad, unsigned int pin,
enum pin_config_param cfg)
{
unsigned int bank, msk;
- int error;
u8 val;
- mutex_lock(&kpad->gpio_lock);
+ guard(mutex)(&kpad->gpio_lock);
if (cfg == PIN_CONFIG_BIAS_PULL_UP)
val = test_bit(pin, &kpad->pull_up_100k_map) ? 2 : 0;
@@ -537,13 +527,9 @@ static int adp5589_gpio_set_bias(struct adp5589_kpad *kpad, unsigned int pin,
val <<= __bf_shf(msk);
kpad->rpull[bank] = (kpad->rpull[bank] & ~msk) | (val & msk);
- error = adp5589_write(kpad->client,
- kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + bank,
- kpad->rpull[bank]);
-
- mutex_unlock(&kpad->gpio_lock);
-
- return error;
+ return adp5589_write(kpad->client,
+ kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + bank,
+ kpad->rpull[bank]);
}
static int adp5589_gpio_set_config(struct gpio_chip *chip, unsigned int off,
@@ -574,15 +560,15 @@ static int adp5589_gpio_set_config(struct gpio_chip *chip, unsigned int off,
return -EINVAL;
}
- mutex_lock(&kpad->gpio_lock);
- if (!val)
- kpad->debounce_dis[bank] |= bit;
- else
- kpad->debounce_dis[bank] &= bit;
+ scoped_guard(mutex, &kpad->gpio_lock) {
+ if (!val)
+ kpad->debounce_dis[bank] |= bit;
+ else
+ kpad->debounce_dis[bank] &= bit;
- error = adp5589_write(kpad->client, reg,
- kpad->debounce_dis[bank]);
- mutex_unlock(&kpad->gpio_lock);
+ error = adp5589_write(kpad->client, reg,
+ kpad->debounce_dis[bank]);
+ }
return error;
default:
return -EOPNOTSUPP;
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 08/13] Input: adp5589-keys: bail out on returned error
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (6 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 07/13] Input: adp5589-keys: add guard() notation Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 09/13] Input: adp5589-keys: refactor adp5589_read() Nuno Sa
` (5 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Don't continue in code paths after some error is found. It makes no
sense to do any other device configuration if a previous one failed.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 109 +++++++++++++++++++++++-----------
1 file changed, 75 insertions(+), 34 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 19ebcb71fd5c7fcba6c7979ba9495179dd393808..622ff442b378768aca7c1da81f7cc38516c7806c 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -422,10 +422,12 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
guard(mutex)(&kpad->gpio_lock);
if (kpad->dir[bank] & bit)
- val = kpad->dat_out[bank];
- else
- val = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank);
+ return !!(kpad->dat_out[bank] & bit);
+
+ val = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank);
+ if (val < 0)
+ return val;
return !!(val & bit);
}
@@ -481,11 +483,12 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
ret = adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A)
+ bank, kpad->dat_out[bank]);
- ret |= adp5589_write(kpad->client,
+ if (ret)
+ return ret;
+
+ return adp5589_write(kpad->client,
kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
kpad->dir[bank]);
-
- return ret;
}
static const u8 adp5589_rpull_masks[] = {
@@ -720,16 +723,27 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
for (i = 0; i <= kpad->info->var->bank(kpad->info->var->maxgpio); i++) {
kpad->dat_out[i] = adp5589_read(kpad->client,
kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) + i);
+ if (kpad->dat_out[i] < 0)
+ return kpad->dat_out[i];
+
kpad->dir[i] = adp5589_read(kpad->client,
kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + i);
+ if (kpad->dir[i] < 0)
+ return kpad->dir[i];
+
kpad->debounce_dis[i] = adp5589_read(kpad->client,
kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A)
+ i);
+ if (kpad->debounce_dis[i] < 0)
+ return kpad->debounce_dis[i];
}
- for (i = 0; i < kpad->info->rpull_banks; i++)
+ for (i = 0; i < kpad->info->rpull_banks; i++) {
kpad->rpull[i] = adp5589_read(kpad->client,
kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + i);
+ if (kpad->rpull[i] < 0)
+ return kpad->rpull[i];
+ }
return 0;
}
@@ -791,9 +805,14 @@ static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
int i;
for (i = 0; i < ev_cnt; i++) {
- int key = adp5589_read(kpad->client, ADP5589_5_FIFO_1 + i);
- int key_val = key & KEY_EV_MASK;
- int key_press = key & KEY_EV_PRESSED;
+ int key, key_val, key_press;
+
+ key = adp5589_read(kpad->client, ADP5589_5_FIFO_1 + i);
+ if (key < 0)
+ return;
+
+ key_val = key & KEY_EV_MASK;
+ key_press = key & KEY_EV_PRESSED;
if (key_val >= kpad->info->var->gpi_pin_base &&
key_val <= kpad->info->var->gpi_pin_end) {
@@ -821,18 +840,22 @@ static irqreturn_t adp5589_irq(int irq, void *handle)
int status, ev_cnt;
status = adp5589_read(client, ADP5589_5_INT_STATUS);
+ if (status < 0)
+ return IRQ_HANDLED;
if (status & OVRFLOW_INT) /* Unlikely and should never happen */
dev_err(&client->dev, "Event Overflow Error\n");
if (status & EVENT_INT) {
ev_cnt = adp5589_read(client, ADP5589_5_STATUS) & KEC;
- if (ev_cnt) {
- adp5589_report_events(kpad, ev_cnt);
- input_sync(kpad->input);
- }
+ if (ev_cnt <= 0)
+ goto out_irq;
+
+ adp5589_report_events(kpad, ev_cnt);
+ input_sync(kpad->input);
}
+out_irq:
adp5589_write(client, ADP5589_5_INT_STATUS, status); /* Status is W1C */
return IRQ_HANDLED;
@@ -846,12 +869,20 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_A),
kpad->keypad_en_mask);
- ret |= adp5589_write(client, reg(ADP5589_PIN_CONFIG_B),
- kpad->keypad_en_mask >> kpad->info->var->col_shift);
+ if (ret)
+ return ret;
- if (!kpad->info->is_adp5585)
- ret |= adp5589_write(client, ADP5589_PIN_CONFIG_C,
- kpad->keypad_en_mask >> 16);
+ ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_B),
+ kpad->keypad_en_mask >> kpad->info->var->col_shift);
+ if (ret)
+ return ret;
+
+ if (!kpad->info->is_adp5585) {
+ ret = adp5589_write(client, ADP5589_PIN_CONFIG_C,
+ kpad->keypad_en_mask >> 16);
+ if (ret)
+ return ret;
+ }
/* unlock keys */
for (i = 0; i < kpad->nkeys_unlock; i++) {
@@ -872,8 +903,11 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
return ret;
}
- for (i = 0; i < KEYP_MAX_EVENT; i++)
- ret |= adp5589_read(client, ADP5589_5_FIFO_1 + i);
+ for (i = 0; i < KEYP_MAX_EVENT; i++) {
+ ret = adp5589_read(client, ADP5589_5_FIFO_1 + i);
+ if (ret < 0)
+ return ret;
+ }
/* reset keys */
for (i = 0; i < kpad->nkeys_reset1; i++) {
@@ -902,20 +936,27 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
return ret;
}
- ret |= adp5589_write(client, reg(ADP5589_POLL_PTIME_CFG),
- kpad->key_poll_time);
- ret |= adp5589_write(client, ADP5589_5_INT_STATUS,
- (kpad->info->is_adp5585 ? 0 : LOGIC2_INT) |
- LOGIC1_INT | OVRFLOW_INT |
- (kpad->info->is_adp5585 ? 0 : LOCK_INT) |
- GPI_INT | EVENT_INT); /* Status is W1C */
+ ret = adp5589_write(client, reg(ADP5589_POLL_PTIME_CFG),
+ kpad->key_poll_time);
+ if (ret)
+ return ret;
- ret |= adp5589_write(client, reg(ADP5589_GENERAL_CFG),
- INT_CFG | OSC_EN | CORE_CLK(3));
- ret |= adp5589_write(client, reg(ADP5589_INT_EN),
- OVRFLOW_IEN | GPI_IEN | EVENT_IEN);
+ ret = adp5589_write(client, ADP5589_5_INT_STATUS,
+ (kpad->info->is_adp5585 ? 0 : LOGIC2_INT) |
+ LOGIC1_INT | OVRFLOW_INT |
+ (kpad->info->is_adp5585 ? 0 : LOCK_INT) |
+ GPI_INT | EVENT_INT); /* Status is W1C */
+ if (ret)
+ return ret;
- if (ret < 0) {
+ ret = adp5589_write(client, reg(ADP5589_GENERAL_CFG),
+ INT_CFG | OSC_EN | CORE_CLK(3));
+ if (ret)
+ return ret;
+
+ ret = adp5589_write(client, reg(ADP5589_INT_EN),
+ OVRFLOW_IEN | GPI_IEN | EVENT_IEN);
+ if (ret) {
dev_err(&client->dev, "Write Error\n");
return ret;
}
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 09/13] Input: adp5589-keys: refactor adp5589_read()
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (7 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 08/13] Input: adp5589-keys: bail out on returned error Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 10/13] Input: adp5589-keys: fix coding style Nuno Sa
` (4 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
adp5589_read() now either returns 0 or an error code. The value to read
is passed as an argument to the function. Therefore we don't mix
register values with error codes.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 94 ++++++++++++++++++++---------------
1 file changed, 53 insertions(+), 41 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 622ff442b378768aca7c1da81f7cc38516c7806c..103a290c5f9f5407253960bbc44566b115789dfc 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -398,14 +398,17 @@ static const struct adp_constants const_adp5585 = {
.reg = adp5585_reg,
};
-static int adp5589_read(struct i2c_client *client, u8 reg)
+static int adp5589_read(struct i2c_client *client, u8 reg, u8 *val)
{
int ret = i2c_smbus_read_byte_data(client, reg);
- if (ret < 0)
+ if (ret < 0) {
dev_err(&client->dev, "Read Error\n");
+ return ret;
+ }
- return ret;
+ *val = ret;
+ return 0;
}
static int adp5589_write(struct i2c_client *client, u8 reg, u8 val)
@@ -418,16 +421,18 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
- int val;
+ int error;
+ u8 val;
guard(mutex)(&kpad->gpio_lock);
if (kpad->dir[bank] & bit)
return !!(kpad->dat_out[bank] & bit);
- val = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank);
- if (val < 0)
- return val;
+ error = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank,
+ &val);
+ if (error)
+ return error;
return !!(val & bit);
}
@@ -721,28 +726,31 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
return error;
for (i = 0; i <= kpad->info->var->bank(kpad->info->var->maxgpio); i++) {
- kpad->dat_out[i] = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) + i);
- if (kpad->dat_out[i] < 0)
- return kpad->dat_out[i];
+ error = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) + i,
+ &kpad->dat_out[i]);
+ if (error)
+ return error;
- kpad->dir[i] = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + i);
- if (kpad->dir[i] < 0)
- return kpad->dir[i];
+ error = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + i,
+ &kpad->dir[i]);
+ if (error)
+ return error;
- kpad->debounce_dis[i] = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A)
- + i);
- if (kpad->debounce_dis[i] < 0)
- return kpad->debounce_dis[i];
+ error = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A) + i,
+ &kpad->debounce_dis[i]);
+ if (error)
+ return error;
}
for (i = 0; i < kpad->info->rpull_banks; i++) {
- kpad->rpull[i] = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + i);
- if (kpad->rpull[i] < 0)
- return kpad->rpull[i];
+ error = adp5589_read(kpad->client,
+ kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + i,
+ &kpad->rpull[i]);
+ if (error)
+ return error;
}
return 0;
@@ -805,10 +813,11 @@ static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
int i;
for (i = 0; i < ev_cnt; i++) {
- int key, key_val, key_press;
+ u8 key, key_val, key_press;
+ int error;
- key = adp5589_read(kpad->client, ADP5589_5_FIFO_1 + i);
- if (key < 0)
+ error = adp5589_read(kpad->client, ADP5589_5_FIFO_1 + i, &key);
+ if (error)
return;
key_val = key & KEY_EV_MASK;
@@ -837,21 +846,22 @@ static irqreturn_t adp5589_irq(int irq, void *handle)
{
struct adp5589_kpad *kpad = handle;
struct i2c_client *client = kpad->client;
- int status, ev_cnt;
+ u8 status, ev_cnt;
+ int error;
- status = adp5589_read(client, ADP5589_5_INT_STATUS);
- if (status < 0)
+ error = adp5589_read(client, ADP5589_5_INT_STATUS, &status);
+ if (error)
return IRQ_HANDLED;
if (status & OVRFLOW_INT) /* Unlikely and should never happen */
dev_err(&client->dev, "Event Overflow Error\n");
if (status & EVENT_INT) {
- ev_cnt = adp5589_read(client, ADP5589_5_STATUS) & KEC;
- if (ev_cnt <= 0)
+ error = adp5589_read(client, ADP5589_5_STATUS, &ev_cnt);
+ if (error || !(ev_cnt & KEC))
goto out_irq;
- adp5589_report_events(kpad, ev_cnt);
+ adp5589_report_events(kpad, ev_cnt & KEC);
input_sync(kpad->input);
}
@@ -866,6 +876,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
struct i2c_client *client = kpad->client;
u8 (*reg)(u8) = kpad->info->var->reg;
int i, ret;
+ u8 dummy;
ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_A),
kpad->keypad_en_mask);
@@ -904,8 +915,8 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
}
for (i = 0; i < KEYP_MAX_EVENT; i++) {
- ret = adp5589_read(client, ADP5589_5_FIFO_1 + i);
- if (ret < 0)
+ ret = adp5589_read(client, ADP5589_5_FIFO_1 + i, &dummy);
+ if (ret)
return ret;
}
@@ -1456,7 +1467,8 @@ static int adp5589_probe(struct i2c_client *client)
{
struct adp5589_kpad *kpad;
unsigned int revid;
- int error, ret;
+ int error;
+ u8 id;
if (!i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -1479,11 +1491,11 @@ static int adp5589_probe(struct i2c_client *client)
if (error)
return error;
- ret = adp5589_read(client, ADP5589_5_ID);
- if (ret < 0)
- return ret;
+ error = adp5589_read(client, ADP5589_5_ID, &id);
+ if (error)
+ return error;
- revid = (u8) ret & ADP5589_5_DEVICE_ID_MASK;
+ revid = id & ADP5589_5_DEVICE_ID_MASK;
error = adp5589_keypad_add(kpad, revid);
if (error)
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 10/13] Input: adp5589-keys: fix coding style
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (8 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 09/13] Input: adp5589-keys: refactor adp5589_read() Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 11/13] Input: adp5589-keys: unify adp_constants in info struct Nuno Sa
` (3 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Just some code cleanup regarding coding style.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 103a290c5f9f5407253960bbc44566b115789dfc..741353bf19ca8725d6697507c1d2183a019972a4 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -244,9 +244,9 @@ struct adp_constants {
u8 max_col_num;
u8 col_shift;
u8 c4_extend_cfg;
- u8 (*bank) (u8 offset);
- u8 (*bit) (u8 offset);
- u8 (*reg) (u8 reg);
+ u8 (*bank)(u8 offset);
+ u8 (*bit)(u8 offset);
+ u8 (*reg)(u8 reg);
};
struct adp5589_info {
@@ -274,7 +274,7 @@ struct adp5589_kpad {
u32 reset2_keys[2];
u32 nkeys_reset2;
unsigned short gpimapsize;
- unsigned extend_cfg;
+ unsigned int extend_cfg;
unsigned char gpiomap[ADP5589_MAXGPIO];
struct gpio_chip gc;
struct mutex gpio_lock; /* Protect cached dir, dat_out */
@@ -416,7 +416,7 @@ static int adp5589_write(struct i2c_client *client, u8 reg, u8 val)
return i2c_smbus_write_byte_data(client, reg, val);
}
-static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
+static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned int off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
@@ -438,7 +438,7 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned off)
}
static void adp5589_gpio_set_value(struct gpio_chip *chip,
- unsigned off, int val)
+ unsigned int off, int val)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
@@ -455,7 +455,8 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
bank, kpad->dat_out[bank]);
}
-static int adp5589_gpio_direction_input(struct gpio_chip *chip, unsigned off)
+static int adp5589_gpio_direction_input(struct gpio_chip *chip,
+ unsigned int off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
@@ -470,7 +471,7 @@ static int adp5589_gpio_direction_input(struct gpio_chip *chip, unsigned off)
}
static int adp5589_gpio_direction_output(struct gpio_chip *chip,
- unsigned off, int val)
+ unsigned int off, int val)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 11/13] Input: adp5589-keys: unify adp_constants in info struct
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (9 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 10/13] Input: adp5589-keys: fix coding style Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 12/13] Input: adp5589-keys: make use of dev_err_probe() Nuno Sa
` (2 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
It makes no sense to have both chip_info and struct adp_constants.
Hence, let's move it all to the more common chip_info structure.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 192 +++++++++++++++++-----------------
1 file changed, 94 insertions(+), 98 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 741353bf19ca8725d6697507c1d2183a019972a4..8cbc2c861ffc2854b5621e515712ae107f46a73f 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -235,7 +235,9 @@
#define ADP5589_MAX_UNLOCK_TIME_SEC 7
-struct adp_constants {
+struct adp5589_info {
+ u8 rpull_banks;
+ u8 c4_extend_cfg;
u8 maxgpio;
u8 keymapsize;
u8 gpi_pin_base;
@@ -243,19 +245,13 @@ struct adp_constants {
u8 max_row_num;
u8 max_col_num;
u8 col_shift;
- u8 c4_extend_cfg;
+ bool support_row5;
+ bool is_adp5585;
u8 (*bank)(u8 offset);
u8 (*bit)(u8 offset);
u8 (*reg)(u8 reg);
};
-struct adp5589_info {
- const struct adp_constants *var;
- u8 rpull_banks;
- bool support_row5;
- bool is_adp5585;
-};
-
struct adp5589_kpad {
struct i2c_client *client;
struct input_dev *input;
@@ -309,20 +305,6 @@ static unsigned char adp5589_reg(unsigned char reg)
return reg;
}
-static const struct adp_constants const_adp5589 = {
- .maxgpio = ADP5589_MAXGPIO,
- .keymapsize = ADP5589_KEYMAPSIZE,
- .gpi_pin_base = 97,
- .gpi_pin_end = 115,
- .c4_extend_cfg = 12,
- .max_row_num = ADP5589_MAX_ROW_NUM,
- .max_col_num = ADP5589_MAX_COL_NUM,
- .col_shift = ADP5589_COL_SHIFT,
- .bank = adp5589_bank,
- .bit = adp5589_bit,
- .reg = adp5589_reg,
-};
-
/* ADP5585 */
static unsigned char adp5585_bank(unsigned char offset)
@@ -384,20 +366,6 @@ static unsigned char adp5585_reg(unsigned char reg)
return adp5585_reg_lut[reg];
}
-static const struct adp_constants const_adp5585 = {
- .maxgpio = ADP5585_MAXGPIO,
- .keymapsize = ADP5585_KEYMAPSIZE,
- .gpi_pin_base = 37,
- .gpi_pin_end = 47,
- .c4_extend_cfg = 10,
- .max_row_num = ADP5585_MAX_ROW_NUM,
- .max_col_num = ADP5585_MAX_COL_NUM,
- .col_shift = ADP5585_COL_SHIFT,
- .bank = adp5585_bank,
- .bit = adp5585_bit,
- .reg = adp5585_reg,
-};
-
static int adp5589_read(struct i2c_client *client, u8 reg, u8 *val)
{
int ret = i2c_smbus_read_byte_data(client, reg);
@@ -419,8 +387,8 @@ static int adp5589_write(struct i2c_client *client, u8 reg, u8 val)
static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned int off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->bit(kpad->gpiomap[off]);
int error;
u8 val;
@@ -429,7 +397,7 @@ static int adp5589_gpio_get_value(struct gpio_chip *chip, unsigned int off)
return !!(kpad->dat_out[bank] & bit);
error = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPI_STATUS_A) + bank,
+ kpad->info->reg(ADP5589_GPI_STATUS_A) + bank,
&val);
if (error)
return error;
@@ -441,8 +409,8 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
unsigned int off, int val)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->bit(kpad->gpiomap[off]);
guard(mutex)(&kpad->gpio_lock);
@@ -451,7 +419,7 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
else
kpad->dat_out[bank] &= ~bit;
- adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) +
+ adp5589_write(kpad->client, kpad->info->reg(ADP5589_GPO_DATA_OUT_A) +
bank, kpad->dat_out[bank]);
}
@@ -459,14 +427,14 @@ static int adp5589_gpio_direction_input(struct gpio_chip *chip,
unsigned int off)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->bit(kpad->gpiomap[off]);
guard(mutex)(&kpad->gpio_lock);
kpad->dir[bank] &= ~bit;
return adp5589_write(kpad->client,
- kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
+ kpad->info->reg(ADP5589_GPIO_DIRECTION_A) + bank,
kpad->dir[bank]);
}
@@ -474,8 +442,8 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
unsigned int off, int val)
{
struct adp5589_kpad *kpad = gpiochip_get_data(chip);
- unsigned int bank = kpad->info->var->bank(kpad->gpiomap[off]);
- unsigned int bit = kpad->info->var->bit(kpad->gpiomap[off]);
+ unsigned int bank = kpad->info->bank(kpad->gpiomap[off]);
+ unsigned int bit = kpad->info->bit(kpad->gpiomap[off]);
int ret;
guard(mutex)(&kpad->gpio_lock);
@@ -487,13 +455,13 @@ static int adp5589_gpio_direction_output(struct gpio_chip *chip,
else
kpad->dat_out[bank] &= ~bit;
- ret = adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A)
+ ret = adp5589_write(kpad->client, kpad->info->reg(ADP5589_GPO_DATA_OUT_A)
+ bank, kpad->dat_out[bank]);
if (ret)
return ret;
return adp5589_write(kpad->client,
- kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + bank,
+ kpad->info->reg(ADP5589_GPIO_DIRECTION_A) + bank,
kpad->dir[bank]);
}
@@ -525,9 +493,9 @@ static int adp5589_gpio_set_bias(struct adp5589_kpad *kpad, unsigned int pin,
* between rows and columns. Hence, we detect when a pin is a column and
* apply the proper offset and pin normalization.
*/
- if (kpad->info->is_adp5585 && pin >= kpad->info->var->col_shift) {
- bank = 2 + (pin - kpad->info->var->col_shift) / 4;
- msk = adp5589_rpull_masks[(pin - kpad->info->var->col_shift) % 4];
+ if (kpad->info->is_adp5585 && pin >= kpad->info->col_shift) {
+ bank = 2 + (pin - kpad->info->col_shift) / 4;
+ msk = adp5589_rpull_masks[(pin - kpad->info->col_shift) % 4];
} else {
bank = pin / 4;
msk = adp5589_rpull_masks[pin % 4];
@@ -537,7 +505,7 @@ static int adp5589_gpio_set_bias(struct adp5589_kpad *kpad, unsigned int pin,
kpad->rpull[bank] = (kpad->rpull[bank] & ~msk) | (val & msk);
return adp5589_write(kpad->client,
- kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + bank,
+ kpad->info->reg(ADP5589_RPULL_CONFIG_A) + bank,
kpad->rpull[bank]);
}
@@ -558,9 +526,9 @@ static int adp5589_gpio_set_config(struct gpio_chip *chip, unsigned int off,
case PIN_CONFIG_BIAS_DISABLE:
return adp5589_gpio_set_bias(kpad, pin, cfg);
case PIN_CONFIG_INPUT_DEBOUNCE:
- bank = kpad->info->var->bank(pin);
- bit = kpad->info->var->bit(pin);
- reg = kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A) + bank;
+ bank = kpad->info->bank(pin);
+ bit = kpad->info->bit(pin);
+ reg = kpad->info->reg(ADP5589_DEBOUNCE_DIS_A) + bank;
val = pinconf_to_config_argument(config);
if (val && val != 50) {
@@ -592,7 +560,7 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad)
memset(pin_used, false, sizeof(pin_used));
- for (i = 0; i < kpad->info->var->maxgpio; i++)
+ for (i = 0; i < kpad->info->maxgpio; i++)
if (kpad->keypad_en_mask & BIT(i))
pin_used[i] = true;
@@ -600,12 +568,12 @@ static int adp5589_build_gpiomap(struct adp5589_kpad *kpad)
pin_used[4] = true;
if (kpad->extend_cfg & C4_EXTEND_CFG)
- pin_used[kpad->info->var->c4_extend_cfg] = true;
+ pin_used[kpad->info->c4_extend_cfg] = true;
if (!kpad->info->support_row5)
pin_used[5] = true;
- for (i = 0; i < kpad->info->var->maxgpio; i++)
+ for (i = 0; i < kpad->info->maxgpio; i++)
if (!pin_used[i])
kpad->gpiomap[n_unused++] = i;
@@ -624,14 +592,13 @@ static void adp5589_irq_bus_sync_unlock(struct irq_data *d)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct adp5589_kpad *kpad = gpiochip_get_data(gc);
- const struct adp_constants *var = kpad->info->var;
int i;
- for (i = 0; i <= kpad->info->var->bank(kpad->info->var->maxgpio); i++) {
+ for (i = 0; i <= kpad->info->bank(kpad->info->maxgpio); i++) {
if (kpad->int_en[i] ^ kpad->irq_mask[i]) {
kpad->int_en[i] = kpad->irq_mask[i];
adp5589_write(kpad->client,
- var->reg(ADP5589_GPI_EVENT_EN_A) + i,
+ kpad->info->reg(ADP5589_GPI_EVENT_EN_A) + i,
kpad->int_en[i]);
}
}
@@ -645,9 +612,9 @@ static void adp5589_irq_mask(struct irq_data *d)
struct adp5589_kpad *kpad = gpiochip_get_data(gc);
irq_hw_number_t hwirq = irqd_to_hwirq(d);
unsigned long real_irq = kpad->gpiomap[hwirq];
- unsigned int bank = kpad->info->var->bank(real_irq);
+ unsigned int bank = kpad->info->bank(real_irq);
- kpad->irq_mask[bank] &= ~kpad->info->var->bit(real_irq);
+ kpad->irq_mask[bank] &= ~kpad->info->bit(real_irq);
gpiochip_disable_irq(gc, hwirq);
}
@@ -657,10 +624,10 @@ static void adp5589_irq_unmask(struct irq_data *d)
struct adp5589_kpad *kpad = gpiochip_get_data(gc);
irq_hw_number_t hwirq = irqd_to_hwirq(d);
unsigned long real_irq = kpad->gpiomap[hwirq];
- unsigned int bank = kpad->info->var->bank(real_irq);
+ unsigned int bank = kpad->info->bank(real_irq);
gpiochip_enable_irq(gc, hwirq);
- kpad->irq_mask[bank] |= kpad->info->var->bit(real_irq);
+ kpad->irq_mask[bank] |= kpad->info->bit(real_irq);
}
static int adp5589_irq_set_type(struct irq_data *d, unsigned int type)
@@ -726,21 +693,21 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
if (error)
return error;
- for (i = 0; i <= kpad->info->var->bank(kpad->info->var->maxgpio); i++) {
+ for (i = 0; i <= kpad->info->bank(kpad->info->maxgpio); i++) {
error = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPO_DATA_OUT_A) + i,
+ kpad->info->reg(ADP5589_GPO_DATA_OUT_A) + i,
&kpad->dat_out[i]);
if (error)
return error;
error = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_GPIO_DIRECTION_A) + i,
+ kpad->info->reg(ADP5589_GPIO_DIRECTION_A) + i,
&kpad->dir[i]);
if (error)
return error;
error = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_DEBOUNCE_DIS_A) + i,
+ kpad->info->reg(ADP5589_DEBOUNCE_DIS_A) + i,
&kpad->debounce_dis[i]);
if (error)
return error;
@@ -748,7 +715,7 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
for (i = 0; i < kpad->info->rpull_banks; i++) {
error = adp5589_read(kpad->client,
- kpad->info->var->reg(ADP5589_RPULL_CONFIG_A) + i,
+ kpad->info->reg(ADP5589_RPULL_CONFIG_A) + i,
&kpad->rpull[i]);
if (error)
return error;
@@ -776,7 +743,7 @@ static unsigned long adp5589_gpiomap_get_hwirq(struct device *dev,
static void adp5589_gpio_irq_handle(struct adp5589_kpad *kpad, int key_val,
int key_press)
{
- unsigned int irq, gpio = key_val - kpad->info->var->gpi_pin_base, irq_type;
+ unsigned int irq, gpio = key_val - kpad->info->gpi_pin_base, irq_type;
struct i2c_client *client = kpad->client;
struct irq_data *irqd;
unsigned long hwirq;
@@ -824,13 +791,13 @@ static void adp5589_report_events(struct adp5589_kpad *kpad, int ev_cnt)
key_val = key & KEY_EV_MASK;
key_press = key & KEY_EV_PRESSED;
- if (key_val >= kpad->info->var->gpi_pin_base &&
- key_val <= kpad->info->var->gpi_pin_end) {
+ if (key_val >= kpad->info->gpi_pin_base &&
+ key_val <= kpad->info->gpi_pin_end) {
/* gpio line used as IRQ source */
adp5589_gpio_irq_handle(kpad, key_val, key_press);
} else {
- int row = (key_val - 1) / (kpad->info->var->max_col_num + 1);
- int col = (key_val - 1) % (kpad->info->var->max_col_num + 1);
+ int row = (key_val - 1) / (kpad->info->max_col_num + 1);
+ int col = (key_val - 1) % (kpad->info->max_col_num + 1);
int code = MATRIX_SCAN_CODE(row, col, kpad->row_shift);
dev_dbg_ratelimited(&kpad->client->dev,
@@ -875,7 +842,7 @@ static irqreturn_t adp5589_irq(int irq, void *handle)
static int adp5589_setup(struct adp5589_kpad *kpad)
{
struct i2c_client *client = kpad->client;
- u8 (*reg)(u8) = kpad->info->var->reg;
+ u8 (*reg)(u8) = kpad->info->reg;
int i, ret;
u8 dummy;
@@ -885,7 +852,7 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
return ret;
ret = adp5589_write(client, reg(ADP5589_PIN_CONFIG_B),
- kpad->keypad_en_mask >> kpad->info->var->col_shift);
+ kpad->keypad_en_mask >> kpad->info->col_shift);
if (ret)
return ret;
@@ -982,7 +949,7 @@ static int adp5589_validate_key(struct adp5589_kpad *kpad, u32 key, bool is_gpi)
u32 row, col;
if (is_gpi) {
- u32 gpi = key - kpad->info->var->gpi_pin_base;
+ u32 gpi = key - kpad->info->gpi_pin_base;
if (gpi == 5 && !kpad->info->support_row5) {
dev_err(&client->dev,
@@ -1002,12 +969,12 @@ static int adp5589_validate_key(struct adp5589_kpad *kpad, u32 key, bool is_gpi)
return 0;
}
- row = (key - 1) / (kpad->info->var->max_col_num + 1);
- col = (key - 1) % (kpad->info->var->max_col_num + 1);
+ row = (key - 1) / (kpad->info->max_col_num + 1);
+ col = (key - 1) % (kpad->info->max_col_num + 1);
/* both the row and col must be part of the keypad */
if (BIT(row) & kpad->keypad_en_mask &&
- BIT(col) << kpad->info->var->col_shift & kpad->keypad_en_mask)
+ BIT(col) << kpad->info->col_shift & kpad->keypad_en_mask)
return 0;
dev_err(&client->dev, "Invalid unlock/reset key(%u) not used in the keypad(%x)\n",
@@ -1045,7 +1012,7 @@ static int adp5589_parse_key_array(struct adp5589_kpad *kpad, const char *prop,
if (ret)
return ret;
- max_keypad = (kpad->info->var->max_row_num + 1) * (kpad->info->var->max_col_num + 1);
+ max_keypad = (kpad->info->max_row_num + 1) * (kpad->info->max_col_num + 1);
for (key = 0; key < *n_keys; key++) {
/* part of the keypad... */
@@ -1059,8 +1026,8 @@ static int adp5589_parse_key_array(struct adp5589_kpad *kpad, const char *prop,
}
/* part of gpio-keys... */
- if (in_range(keys[key], kpad->info->var->gpi_pin_base,
- kpad->info->var->maxgpio)) {
+ if (in_range(keys[key], kpad->info->gpi_pin_base,
+ kpad->info->maxgpio)) {
/* is the GPI being used as part of the keypad?! */
ret = adp5589_validate_key(kpad, keys[key], true);
if (ret)
@@ -1143,7 +1110,7 @@ static int adp5589_reset_parse(struct adp5589_kpad *kpad)
* Then C4 is used as reset output. Make sure it's not being used
* in the keypad.
*/
- if (BIT(kpad->info->var->c4_extend_cfg) & kpad->keypad_en_mask) {
+ if (BIT(kpad->info->c4_extend_cfg) & kpad->keypad_en_mask) {
dev_err(&client->dev, "Col4 cannot be used if reset2 is used\n");
return -EINVAL;
}
@@ -1237,9 +1204,9 @@ static int adp5589_gpio_parse(struct adp5589_kpad *kpad)
return -EINVAL;
}
- if (reg >= kpad->info->var->maxgpio) {
+ if (reg >= kpad->info->maxgpio) {
dev_err(&client->dev, "Invalid gpio(%u > %u)\n",
- reg, kpad->info->var->maxgpio);
+ reg, kpad->info->maxgpio);
return -EINVAL;
}
@@ -1263,7 +1230,7 @@ static int adp5589_gpio_parse(struct adp5589_kpad *kpad)
}
/* Check if the gpio is being used as reset2 */
- if (kpad->extend_cfg & C4_EXTEND_CFG && reg == kpad->info->var->c4_extend_cfg) {
+ if (kpad->extend_cfg & C4_EXTEND_CFG && reg == kpad->info->c4_extend_cfg) {
dev_err(&client->dev, "Invalid gpio(%u) used as reset2\n",
reg);
return -EINVAL;
@@ -1295,13 +1262,13 @@ static int adp5589_parse_fw(struct adp5589_kpad *kpad)
error = device_property_read_u32(&client->dev, "adi,cols-mask",
&prop_val);
if (!error) {
- if (prop_val > GENMASK(kpad->info->var->max_col_num, 0)) {
+ if (prop_val > GENMASK(kpad->info->max_col_num, 0)) {
dev_err(&client->dev, "Invalid column mask(%x)\n",
prop_val);
return -EINVAL;
}
- kpad->keypad_en_mask = prop_val << kpad->info->var->col_shift;
+ kpad->keypad_en_mask = prop_val << kpad->info->col_shift;
/*
* Note that given that we get a mask (and the HW allows it), we
* can have holes in our keypad (eg: row0, row1 and row7 enabled).
@@ -1315,7 +1282,7 @@ static int adp5589_parse_fw(struct adp5589_kpad *kpad)
error = device_property_read_u32(&client->dev, "adi,rows-mask",
&prop_val);
if (!error) {
- if (prop_val > GENMASK(kpad->info->var->max_row_num, 0)) {
+ if (prop_val > GENMASK(kpad->info->max_row_num, 0)) {
dev_err(&client->dev, "Invalid row mask(%x)\n",
prop_val);
return -EINVAL;
@@ -1441,27 +1408,56 @@ static void adp5589_clear_config(void *data)
{
struct adp5589_kpad *kpad = data;
- adp5589_write(kpad->client, kpad->info->var->reg(ADP5589_GENERAL_CFG),
- 0);
+ adp5589_write(kpad->client, kpad->info->reg(ADP5589_GENERAL_CFG), 0);
}
static const struct adp5589_info adp5589_info = {
- .var = &const_adp5589,
.support_row5 = true,
.rpull_banks = ADP5589_RPULL_CONFIG_E - ADP5589_RPULL_CONFIG_A + 1,
+ .c4_extend_cfg = 12,
+ .maxgpio = ADP5589_MAXGPIO,
+ .keymapsize = ADP5589_KEYMAPSIZE,
+ .gpi_pin_base = 97,
+ .gpi_pin_end = 115,
+ .max_row_num = ADP5589_MAX_ROW_NUM,
+ .max_col_num = ADP5589_MAX_COL_NUM,
+ .col_shift = ADP5589_COL_SHIFT,
+ .bank = adp5589_bank,
+ .bit = adp5589_bit,
+ .reg = adp5589_reg,
};
static const struct adp5589_info adp5585_info = {
- .var = &const_adp5585,
.is_adp5585 = true,
.rpull_banks = ADP5585_RPULL_CONFIG_D - ADP5585_RPULL_CONFIG_A + 1,
+ .c4_extend_cfg = 10,
+ .maxgpio = ADP5585_MAXGPIO,
+ .keymapsize = ADP5585_KEYMAPSIZE,
+ .gpi_pin_base = 37,
+ .gpi_pin_end = 47,
+ .max_row_num = ADP5585_MAX_ROW_NUM,
+ .max_col_num = ADP5585_MAX_COL_NUM,
+ .col_shift = ADP5585_COL_SHIFT,
+ .bank = adp5585_bank,
+ .bit = adp5585_bit,
+ .reg = adp5585_reg,
};
static const struct adp5589_info adp5585_2_info = {
- .var = &const_adp5585,
.is_adp5585 = true,
.support_row5 = true,
.rpull_banks = ADP5585_RPULL_CONFIG_D - ADP5585_RPULL_CONFIG_A + 1,
+ .c4_extend_cfg = 10,
+ .maxgpio = ADP5585_MAXGPIO,
+ .keymapsize = ADP5585_KEYMAPSIZE,
+ .gpi_pin_base = 37,
+ .gpi_pin_end = 47,
+ .max_row_num = ADP5585_MAX_ROW_NUM,
+ .max_col_num = ADP5585_MAX_COL_NUM,
+ .col_shift = ADP5585_COL_SHIFT,
+ .bank = adp5585_bank,
+ .bit = adp5585_bit,
+ .reg = adp5585_reg,
};
static int adp5589_probe(struct i2c_client *client)
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 12/13] Input: adp5589-keys: make use of dev_err_probe()
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (10 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 11/13] Input: adp5589-keys: unify adp_constants in info struct Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-01 13:41 ` [PATCH 13/13] Input: adp5589-keys: add regulator support Nuno Sa
2024-10-16 13:36 ` [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sá
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Simplify the probe error path by using dev_err_probe().
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 226 ++++++++++++++--------------------
1 file changed, 94 insertions(+), 132 deletions(-)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 8cbc2c861ffc2854b5621e515712ae107f46a73f..a3d51e36132b73ef07715f256b82e428c81bd6f6 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -676,10 +676,9 @@ static int adp5589_gpio_add(struct adp5589_kpad *kpad)
kpad->gc.owner = THIS_MODULE;
if (device_property_present(dev, "interrupt-controller")) {
- if (!kpad->client->irq) {
- dev_err(dev, "Unable to serve as interrupt controller without IRQ\n");
- return -EINVAL;
- }
+ if (!kpad->client->irq)
+ return dev_err_probe(dev, -EINVAL,
+ "Unable to serve as interrupt controller without IRQ\n");
girq = &kpad->gc.irq;
gpio_irq_chip_set_chip(girq, &adp5589_irq_chip);
@@ -935,10 +934,8 @@ static int adp5589_setup(struct adp5589_kpad *kpad)
ret = adp5589_write(client, reg(ADP5589_INT_EN),
OVRFLOW_IEN | GPI_IEN | EVENT_IEN);
- if (ret) {
- dev_err(&client->dev, "Write Error\n");
- return ret;
- }
+ if (ret)
+ return dev_err_probe(&client->dev, ret, "Write Error\n");
return 0;
}
@@ -951,20 +948,15 @@ static int adp5589_validate_key(struct adp5589_kpad *kpad, u32 key, bool is_gpi)
if (is_gpi) {
u32 gpi = key - kpad->info->gpi_pin_base;
- if (gpi == 5 && !kpad->info->support_row5) {
- dev_err(&client->dev,
- "Invalid unlock/reset GPI(%u) not supported\n",
- gpi);
- return -EINVAL;
- }
+ if (gpi == 5 && !kpad->info->support_row5)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid unlock/reset GPI(%u) not supported\n", gpi);
/* check if it's being used in the keypad */
- if (BIT(gpi) & kpad->keypad_en_mask) {
- dev_err(&client->dev,
- "Invalid unlock/reset GPI(%u) being used in the keypad(%x)\n",
- gpi, kpad->keypad_en_mask);
- return -EINVAL;
- }
+ if (BIT(gpi) & kpad->keypad_en_mask)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid unlock/reset GPI(%u) being used in the keypad(%x)\n",
+ gpi, kpad->keypad_en_mask);
return 0;
}
@@ -977,10 +969,9 @@ static int adp5589_validate_key(struct adp5589_kpad *kpad, u32 key, bool is_gpi)
BIT(col) << kpad->info->col_shift & kpad->keypad_en_mask)
return 0;
- dev_err(&client->dev, "Invalid unlock/reset key(%u) not used in the keypad(%x)\n",
- key, kpad->keypad_en_mask);
-
- return -EINVAL;
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid unlock/reset key(%u) not used in the keypad(%x)\n",
+ key, kpad->keypad_en_mask);
}
static int adp5589_parse_key_array(struct adp5589_kpad *kpad, const char *prop,
@@ -997,16 +988,14 @@ static int adp5589_parse_key_array(struct adp5589_kpad *kpad, const char *prop,
*n_keys = ret;
- if (kpad->info->is_adp5585 && !reset_key) {
- dev_err(&client->dev, "Unlock keys not supported for adp5585\n");
- return -EOPNOTSUPP;
- }
+ if (kpad->info->is_adp5585 && !reset_key)
+ return dev_err_probe(&client->dev, -EOPNOTSUPP,
+ "Unlock keys not supported for adp5585\n");
- if (*n_keys > max_keys) {
- dev_err(&client->dev, "Invalid number of keys(%u > %u) for %s\n",
- *n_keys, max_keys, prop);
- return -EINVAL;
- }
+ if (*n_keys > max_keys)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid number of keys(%u > %u) for %s\n",
+ *n_keys, max_keys, prop);
ret = device_property_read_u32_array(&client->dev, prop, keys, *n_keys);
if (ret)
@@ -1040,10 +1029,8 @@ static int adp5589_parse_key_array(struct adp5589_kpad *kpad, const char *prop,
if (!reset_key && keys[key] == 127)
continue;
- dev_err(&client->dev, "Invalid key(%u) for %s\n", keys[key],
- prop);
-
- return -EINVAL;
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid key(%u) for %s\n", keys[key], prop);
}
return 0;
@@ -1066,11 +1053,10 @@ static int adp5589_unlock_parse(struct adp5589_kpad *kpad)
error = device_property_read_u32(&client->dev, "adi,unlock-trigger-sec",
&kpad->unlock_time);
if (!error) {
- if (kpad->unlock_time > ADP5589_MAX_UNLOCK_TIME_SEC) {
- dev_err(&client->dev, "Invalid unlock time(%u > %d)\n",
- kpad->unlock_time, ADP5589_MAX_UNLOCK_TIME_SEC);
- return -EINVAL;
- }
+ if (kpad->unlock_time > ADP5589_MAX_UNLOCK_TIME_SEC)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid unlock time(%u > %d)\n",
+ kpad->unlock_time, ADP5589_MAX_UNLOCK_TIME_SEC);
}
return 0;
@@ -1092,10 +1078,9 @@ static int adp5589_reset_parse(struct adp5589_kpad *kpad)
* Then R4 is used as reset output. Make sure it's not being used
* in the keypad.
*/
- if (BIT(4) & kpad->keypad_en_mask) {
- dev_err(&client->dev, "Row4 cannot be used if reset1 is used\n");
- return -EINVAL;
- }
+ if (BIT(4) & kpad->keypad_en_mask)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Row4 cannot be used if reset1 is used\n");
kpad->extend_cfg = R4_EXTEND_CFG;
}
@@ -1110,10 +1095,9 @@ static int adp5589_reset_parse(struct adp5589_kpad *kpad)
* Then C4 is used as reset output. Make sure it's not being used
* in the keypad.
*/
- if (BIT(kpad->info->c4_extend_cfg) & kpad->keypad_en_mask) {
- dev_err(&client->dev, "Col4 cannot be used if reset2 is used\n");
- return -EINVAL;
- }
+ if (BIT(kpad->info->c4_extend_cfg) & kpad->keypad_en_mask)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Col4 cannot be used if reset2 is used\n");
kpad->extend_cfg |= C4_EXTEND_CFG;
}
@@ -1159,9 +1143,9 @@ static int adp5589_reset_parse(struct adp5589_kpad *kpad)
kpad->reset_cfg |= FIELD_PREP(RESET_TRIGGER_TIME, 7);
break;
default:
- dev_err(&client->dev, "Invalid value(%u) for adi,reset-trigger-ms\n",
- prop_val);
- return -EINVAL;
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid value(%u) for adi,reset-trigger-ms\n",
+ prop_val);
}
}
@@ -1182,9 +1166,9 @@ static int adp5589_reset_parse(struct adp5589_kpad *kpad)
kpad->reset_cfg |= FIELD_PREP(RESET_PULSE_WIDTH, 3);
break;
default:
- dev_err(&client->dev, "Invalid value(%u) for adi,reset-pulse-width-us\n",
- prop_val);
- return -EINVAL;
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid value(%u) for adi,reset-pulse-width-us\n",
+ prop_val);
}
}
@@ -1199,51 +1183,41 @@ static int adp5589_gpio_parse(struct adp5589_kpad *kpad)
device_for_each_child_node_scoped(&client->dev, child) {
error = fwnode_property_read_u32(child, "reg", ®);
- if (error) {
- dev_err(&client->dev, "Failed to get reg property\n");
- return -EINVAL;
- }
+ if (error)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Failed to get reg property\n");
- if (reg >= kpad->info->maxgpio) {
- dev_err(&client->dev, "Invalid gpio(%u > %u)\n",
- reg, kpad->info->maxgpio);
- return -EINVAL;
- }
+ if (reg >= kpad->info->maxgpio)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid gpio(%u > %u)\n",
+ reg, kpad->info->maxgpio);
- if (BIT(reg) & kpad->keypad_en_mask) {
- dev_err(&client->dev, "Invalid gpio(%u) used in keypad\n",
- reg);
- return -EINVAL;
- }
+ if (BIT(reg) & kpad->keypad_en_mask)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid gpio(%u) used in keypad\n", reg);
- if (reg == 5 && !kpad->info->support_row5) {
- dev_err(&client->dev, "Invalid gpio(%u) not supported\n",
- reg);
- return -EINVAL;
- }
+ if (reg == 5 && !kpad->info->support_row5)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid gpio(%u) not supported\n",
+ reg);
/* Check if it's gpio4 and R4 is being used as reset */
- if (kpad->extend_cfg & R4_EXTEND_CFG && reg == 4) {
- dev_err(&client->dev, "Invalid gpio(%u) used as reset1\n",
- reg);
- return -EINVAL;
- }
+ if (kpad->extend_cfg & R4_EXTEND_CFG && reg == 4)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid gpio(%u) used as reset1\n", reg);
/* Check if the gpio is being used as reset2 */
- if (kpad->extend_cfg & C4_EXTEND_CFG && reg == kpad->info->c4_extend_cfg) {
- dev_err(&client->dev, "Invalid gpio(%u) used as reset2\n",
- reg);
- return -EINVAL;
- }
+ if (kpad->extend_cfg & C4_EXTEND_CFG && reg == kpad->info->c4_extend_cfg)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid gpio(%u) used as reset2\n", reg);
error = fwnode_property_read_u32(child, "adi,pull-up-ohms",
&pullup);
if (!error) {
- if (pullup != 100 * KILO && pullup != 300 * KILO) {
- dev_err(&client->dev, "Invalid pullup resistor val(%u)",
- pullup);
- return -EINVAL;
- }
+ if (pullup != 100 * KILO && pullup != 300 * KILO)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid pullup resistor val(%u)",
+ pullup);
if (pullup == 100 * KILO)
__set_bit(reg, &kpad->pull_up_100k_map);
@@ -1262,11 +1236,9 @@ static int adp5589_parse_fw(struct adp5589_kpad *kpad)
error = device_property_read_u32(&client->dev, "adi,cols-mask",
&prop_val);
if (!error) {
- if (prop_val > GENMASK(kpad->info->max_col_num, 0)) {
- dev_err(&client->dev, "Invalid column mask(%x)\n",
- prop_val);
- return -EINVAL;
- }
+ if (prop_val > GENMASK(kpad->info->max_col_num, 0))
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid column mask(%x)\n", prop_val);
kpad->keypad_en_mask = prop_val << kpad->info->col_shift;
/*
@@ -1282,37 +1254,30 @@ static int adp5589_parse_fw(struct adp5589_kpad *kpad)
error = device_property_read_u32(&client->dev, "adi,rows-mask",
&prop_val);
if (!error) {
- if (prop_val > GENMASK(kpad->info->max_row_num, 0)) {
- dev_err(&client->dev, "Invalid row mask(%x)\n",
- prop_val);
- return -EINVAL;
- }
+ if (prop_val > GENMASK(kpad->info->max_row_num, 0))
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid row mask(%x)\n", prop_val);
- if (prop_val & BIT(5) && !kpad->info->support_row5) {
- dev_err(&client->dev, "Row5 not supported!\n");
- return -EINVAL;
- }
+ if (prop_val & BIT(5) && !kpad->info->support_row5)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Row5 not supported!\n");
kpad->keypad_en_mask |= prop_val;
rows = fls(prop_val);
}
- if (cols && !rows) {
- dev_err(&client->dev, "Cannot have columns with no rows!\n");
- return -EINVAL;
- }
+ if (cols && !rows)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Cannot have columns with no rows!\n");
- if (rows && !cols) {
- dev_err(&client->dev, "Cannot have rows with no columns!\n");
- return -EINVAL;
- }
+ if (rows && !cols)
+ dev_err_probe(&client->dev, -EINVAL,
+ "Cannot have rows with no columns!\n");
if (rows && cols) {
- if (!client->irq) {
- dev_err(&client->dev,
- "Keymaps won't work without interrupts\n");
- return -EINVAL;
- }
+ if (!client->irq)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Keymaps won't work without interrupts\n");
error = matrix_keypad_build_keymap(NULL, NULL, rows, cols,
kpad->keycode, kpad->input);
@@ -1339,9 +1304,9 @@ static int adp5589_parse_fw(struct adp5589_kpad *kpad)
kpad->key_poll_time = prop_val / 10 - 1;
break;
default:
- dev_err(&client->dev, "Invalid value(%u) for adi,key-poll-ms\n",
- prop_val);
- return -EINVAL;
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid value(%u) for adi,key-poll-ms\n",
+ prop_val);
}
}
@@ -1384,10 +1349,9 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
return error;
error = input_register_device(input);
- if (error) {
- dev_err(&client->dev, "unable to register input device\n");
- return error;
- }
+ if (error)
+ return dev_err_probe(&client->dev, error,
+ "unable to register input device\n");
if (!client->irq)
return 0;
@@ -1396,10 +1360,9 @@ static int adp5589_keypad_add(struct adp5589_kpad *kpad, unsigned int revid)
NULL, adp5589_irq,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
client->dev.driver->name, kpad);
- if (error) {
- dev_err(&client->dev, "unable to request irq %d\n", client->irq);
- return error;
- }
+ if (error)
+ return dev_err_probe(&client->dev, error,
+ "unable to request irq %d\n", client->irq);
return 0;
}
@@ -1468,10 +1431,9 @@ static int adp5589_probe(struct i2c_client *client)
u8 id;
if (!i2c_check_functionality(client->adapter,
- I2C_FUNC_SMBUS_BYTE_DATA)) {
- dev_err(&client->dev, "SMBUS Byte Data not Supported\n");
- return -EIO;
- }
+ I2C_FUNC_SMBUS_BYTE_DATA))
+ return dev_err_probe(&client->dev, -EIO,
+ "SMBUS Byte Data not Supported\n");
kpad = devm_kzalloc(&client->dev, sizeof(*kpad), GFP_KERNEL);
if (!kpad)
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 13/13] Input: adp5589-keys: add regulator support
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (11 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 12/13] Input: adp5589-keys: make use of dev_err_probe() Nuno Sa
@ 2024-10-01 13:41 ` Nuno Sa
2024-10-16 13:36 ` [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sá
13 siblings, 0 replies; 28+ messages in thread
From: Nuno Sa @ 2024-10-01 13:41 UTC (permalink / raw)
To: Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, devicetree
Support feeding VCC through a regulator.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/input/keyboard/adp5589-keys.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index a3d51e36132b73ef07715f256b82e428c81bd6f6..f3a0ea1bec4a7a8ed0a5a96211decc5d86728b71 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -1445,6 +1445,10 @@ static int adp5589_probe(struct i2c_client *client)
if (!kpad->info)
return -ENODEV;
+ error = devm_regulator_get_enable(&client->dev, "vcc");
+ if (error)
+ return error;
+
error = devm_add_action_or_reset(&client->dev, adp5589_clear_config,
kpad);
if (error)
--
2.46.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 00/13] Input: adp5589: refactor and platform_data removal
2024-10-01 13:41 [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sa
` (12 preceding siblings ...)
2024-10-01 13:41 ` [PATCH 13/13] Input: adp5589-keys: add regulator support Nuno Sa
@ 2024-10-16 13:36 ` Nuno Sá
2024-10-18 21:30 ` Dmitry Torokhov
13 siblings, 1 reply; 28+ messages in thread
From: Nuno Sá @ 2024-10-16 13:36 UTC (permalink / raw)
To: Nuno Sa, Dmitry Torokhov, Mike Frysinger, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bartosz Golaszewski, Lee Jones
Cc: linux-input, devicetree
On Tue, 2024-10-01 at 15:41 +0200, Nuno Sa wrote:
> This series aims to remove platform data dependency from the adp5589
> driver (as no platform is really using it) and instead add support for
> FW properties. Note that rows and columns for the keypad are being given
> as masks and that was briefly discussed with Dmitry. For context
> on why this is being done as mask [1].
>
> The first couple of patches are fixes that we may want to backport...
>
> [1]:
> https://lore.kernel.org/linux-input/9db96c99c805e615ba40ca7fd3632174d1e8d11f.camel@gmail.com/
>
> ---
> Nuno Sa (13):
> Input: adp5589-keys: fix NULL pointer dereference
> Input: adp5589-keys: fix adp5589_gpio_get_value()
> Input: adp5589-keys: add chip_info structure
> Input: adp5589-keys: support gpi key events as 'gpio keys'
> dt-bindings: input: Document adp5589 and similar devices
> Input: adp5589-keys: add support for fw properties
> Input: adp5589-keys: add guard() notation
> Input: adp5589-keys: bail out on returned error
> Input: adp5589-keys: refactor adp5589_read()
> Input: adp5589-keys: fix coding style
> Input: adp5589-keys: unify adp_constants in info struct
> Input: adp5589-keys: make use of dev_err_probe()
> Input: adp5589-keys: add regulator support
>
> .../devicetree/bindings/input/adi,adp5589.yaml | 310 +++++
> .../devicetree/bindings/trivial-devices.yaml | 6 -
> MAINTAINERS | 8 +
> drivers/input/keyboard/Kconfig | 3 +
> drivers/input/keyboard/adp5589-keys.c | 1397 +++++++++++++-------
> include/linux/input/adp5589.h | 180 ---
> 6 files changed, 1254 insertions(+), 650 deletions(-)
> ---
> base-commit: c7bf046925dc5885d9c4d8fbcbb7e4e73665bfcf
> change-id: 20240930-b4-dev-adp5589-fw-conversion-955b2f42da70
> --
>
> Thanks!
> - Nuno Sá
>
Hi Dmitry,
Something really caught my attention now while checking 6.12 merge window. It seems
we have a new MFD device for adp5585 [1] which adds duplicated functionality (that
was already present in adp5589-keys.c). So, having this as MFD might makes sense
(even though it makes it harder to validate the keys and to make use of gpio-keys)
but we are now duplicating GPIO support. Bottom line, not sure what we should do next
and should I proceed for v2?
Also ccing Lee and Bartosz...
[1]: https://lore.kernel.org/lkml/20240816152738.GB5853@google.com/
- Nuno Sá
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 00/13] Input: adp5589: refactor and platform_data removal
2024-10-16 13:36 ` [PATCH 00/13] Input: adp5589: refactor and platform_data removal Nuno Sá
@ 2024-10-18 21:30 ` Dmitry Torokhov
2024-10-19 17:18 ` Laurent Pinchart
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2024-10-18 21:30 UTC (permalink / raw)
To: Nuno Sá
Cc: Nuno Sa, Mike Frysinger, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bartosz Golaszewski, Lee Jones, linux-input,
devicetree, Laurent Pinchart, Krzysztof Kozlowski
On Wed, Oct 16, 2024 at 03:36:03PM +0200, Nuno Sá wrote:
> On Tue, 2024-10-01 at 15:41 +0200, Nuno Sa wrote:
> > This series aims to remove platform data dependency from the adp5589
> > driver (as no platform is really using it) and instead add support for
> > FW properties. Note that rows and columns for the keypad are being given
> > as masks and that was briefly discussed with Dmitry. For context
> > on why this is being done as mask [1].
> >
> > The first couple of patches are fixes that we may want to backport...
> >
> > [1]:
> > https://lore.kernel.org/linux-input/9db96c99c805e615ba40ca7fd3632174d1e8d11f.camel@gmail.com/
> >
> > ---
> > Nuno Sa (13):
> > Input: adp5589-keys: fix NULL pointer dereference
> > Input: adp5589-keys: fix adp5589_gpio_get_value()
> > Input: adp5589-keys: add chip_info structure
> > Input: adp5589-keys: support gpi key events as 'gpio keys'
> > dt-bindings: input: Document adp5589 and similar devices
> > Input: adp5589-keys: add support for fw properties
> > Input: adp5589-keys: add guard() notation
> > Input: adp5589-keys: bail out on returned error
> > Input: adp5589-keys: refactor adp5589_read()
> > Input: adp5589-keys: fix coding style
> > Input: adp5589-keys: unify adp_constants in info struct
> > Input: adp5589-keys: make use of dev_err_probe()
> > Input: adp5589-keys: add regulator support
> >
> > .../devicetree/bindings/input/adi,adp5589.yaml | 310 +++++
> > .../devicetree/bindings/trivial-devices.yaml | 6 -
> > MAINTAINERS | 8 +
> > drivers/input/keyboard/Kconfig | 3 +
> > drivers/input/keyboard/adp5589-keys.c | 1397 +++++++++++++-------
> > include/linux/input/adp5589.h | 180 ---
> > 6 files changed, 1254 insertions(+), 650 deletions(-)
> > ---
> > base-commit: c7bf046925dc5885d9c4d8fbcbb7e4e73665bfcf
> > change-id: 20240930-b4-dev-adp5589-fw-conversion-955b2f42da70
> > --
> >
> > Thanks!
> > - Nuno Sá
> >
>
> Hi Dmitry,
>
> Something really caught my attention now while checking 6.12 merge window. It seems
> we have a new MFD device for adp5585 [1] which adds duplicated functionality (that
> was already present in adp5589-keys.c). So, having this as MFD might makes sense
> (even though it makes it harder to validate the keys and to make use of gpio-keys)
> but we are now duplicating GPIO support. Bottom line, not sure what we should do next
> and should I proceed for v2?
>
> Also ccing Lee and Bartosz...
Let's add Laurent and Krzysztof too please.
I am surprised we do not see warnings for various bots because
"adi,adp5585" compatible is present in trivial devices.
I think moving it all to MFD makes sense (I think original drivers were
added well before we had MFD infrastructure), but we need to make sure
the device tree binding is complete and allows describing keypad (and if
not maybe we can pull it from the release and work on it so that it
describes the hardware fully).
Hopefully next time folks try to add drivers to Analog devices they will
remember that Analog is pretty active upstream and they will reach out
to you guys so that work can be coordinated better.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/13] Input: adp5589: refactor and platform_data removal
2024-10-18 21:30 ` Dmitry Torokhov
@ 2024-10-19 17:18 ` Laurent Pinchart
2024-10-21 11:26 ` Nuno Sá
0 siblings, 1 reply; 28+ messages in thread
From: Laurent Pinchart @ 2024-10-19 17:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Nuno Sá, Nuno Sa, Mike Frysinger, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bartosz Golaszewski, Lee Jones,
linux-input, devicetree, Krzysztof Kozlowski
On Fri, Oct 18, 2024 at 02:30:20PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 16, 2024 at 03:36:03PM +0200, Nuno Sá wrote:
> > On Tue, 2024-10-01 at 15:41 +0200, Nuno Sa wrote:
> > > This series aims to remove platform data dependency from the adp5589
> > > driver (as no platform is really using it) and instead add support for
> > > FW properties. Note that rows and columns for the keypad are being given
> > > as masks and that was briefly discussed with Dmitry. For context
> > > on why this is being done as mask [1].
> > >
> > > The first couple of patches are fixes that we may want to backport...
> > >
> > > [1]: https://lore.kernel.org/linux-input/9db96c99c805e615ba40ca7fd3632174d1e8d11f.camel@gmail.com/
> > >
> > > ---
> > > Nuno Sa (13):
> > > Input: adp5589-keys: fix NULL pointer dereference
> > > Input: adp5589-keys: fix adp5589_gpio_get_value()
> > > Input: adp5589-keys: add chip_info structure
> > > Input: adp5589-keys: support gpi key events as 'gpio keys'
> > > dt-bindings: input: Document adp5589 and similar devices
> > > Input: adp5589-keys: add support for fw properties
> > > Input: adp5589-keys: add guard() notation
> > > Input: adp5589-keys: bail out on returned error
> > > Input: adp5589-keys: refactor adp5589_read()
> > > Input: adp5589-keys: fix coding style
> > > Input: adp5589-keys: unify adp_constants in info struct
> > > Input: adp5589-keys: make use of dev_err_probe()
> > > Input: adp5589-keys: add regulator support
> > >
> > > .../devicetree/bindings/input/adi,adp5589.yaml | 310 +++++
> > > .../devicetree/bindings/trivial-devices.yaml | 6 -
> > > MAINTAINERS | 8 +
> > > drivers/input/keyboard/Kconfig | 3 +
> > > drivers/input/keyboard/adp5589-keys.c | 1397 +++++++++++++-------
> > > include/linux/input/adp5589.h | 180 ---
> > > 6 files changed, 1254 insertions(+), 650 deletions(-)
> > > ---
> > > base-commit: c7bf046925dc5885d9c4d8fbcbb7e4e73665bfcf
> > > change-id: 20240930-b4-dev-adp5589-fw-conversion-955b2f42da70
> > > --
> > >
> > > Thanks!
> > > - Nuno Sá
> >
> > Hi Dmitry,
> >
> > Something really caught my attention now while checking 6.12 merge window. It seems
> > we have a new MFD device for adp5585 [1] which adds duplicated functionality (that
> > was already present in adp5589-keys.c). So, having this as MFD might makes sense
> > (even though it makes it harder to validate the keys and to make use of gpio-keys)
> > but we are now duplicating GPIO support. Bottom line, not sure what we should do next
> > and should I proceed for v2?
> >
> > Also ccing Lee and Bartosz...
>
> Let's add Laurent and Krzysztof too please.
>
> I am surprised we do not see warnings for various bots because
> "adi,adp5585" compatible is present in trivial devices.
>
> I think moving it all to MFD makes sense (I think original drivers were
> added well before we had MFD infrastructure), but we need to make sure
> the device tree binding is complete and allows describing keypad (and if
> not maybe we can pull it from the release and work on it so that it
> describes the hardware fully).
Keypad support is nice. I didn't include it in my adp5585 driver
submission because I had no way to test it. Would it be more difficult
to add it to the MFD driver, compared to what is done in this series ?
> Hopefully next time folks try to add drivers to Analog devices they will
> remember that Analog is pretty active upstream and they will reach out
> to you guys so that work can be coordinated better.
I had no idea, and neither did get_maintainer.pl. I suppose I could have
explored git log deeper, and I'll try to remember for next time.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/13] Input: adp5589: refactor and platform_data removal
2024-10-19 17:18 ` Laurent Pinchart
@ 2024-10-21 11:26 ` Nuno Sá
0 siblings, 0 replies; 28+ messages in thread
From: Nuno Sá @ 2024-10-21 11:26 UTC (permalink / raw)
To: Laurent Pinchart, Dmitry Torokhov
Cc: Nuno Sa, Mike Frysinger, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bartosz Golaszewski, Lee Jones, linux-input,
devicetree, Krzysztof Kozlowski
On Sat, 2024-10-19 at 20:18 +0300, Laurent Pinchart wrote:
> On Fri, Oct 18, 2024 at 02:30:20PM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 16, 2024 at 03:36:03PM +0200, Nuno Sá wrote:
> > > On Tue, 2024-10-01 at 15:41 +0200, Nuno Sa wrote:
> > > > This series aims to remove platform data dependency from the adp5589
> > > > driver (as no platform is really using it) and instead add support for
> > > > FW properties. Note that rows and columns for the keypad are being given
> > > > as masks and that was briefly discussed with Dmitry. For context
> > > > on why this is being done as mask [1].
> > > >
> > > > The first couple of patches are fixes that we may want to backport...
> > > >
> > > > [1]:
> > > > https://lore.kernel.org/linux-input/9db96c99c805e615ba40ca7fd3632174d1e8d11f.camel@gmail.com/
> > > >
> > > > ---
> > > > Nuno Sa (13):
> > > > Input: adp5589-keys: fix NULL pointer dereference
> > > > Input: adp5589-keys: fix adp5589_gpio_get_value()
> > > > Input: adp5589-keys: add chip_info structure
> > > > Input: adp5589-keys: support gpi key events as 'gpio keys'
> > > > dt-bindings: input: Document adp5589 and similar devices
> > > > Input: adp5589-keys: add support for fw properties
> > > > Input: adp5589-keys: add guard() notation
> > > > Input: adp5589-keys: bail out on returned error
> > > > Input: adp5589-keys: refactor adp5589_read()
> > > > Input: adp5589-keys: fix coding style
> > > > Input: adp5589-keys: unify adp_constants in info struct
> > > > Input: adp5589-keys: make use of dev_err_probe()
> > > > Input: adp5589-keys: add regulator support
> > > >
> > > > .../devicetree/bindings/input/adi,adp5589.yaml | 310 +++++
> > > > .../devicetree/bindings/trivial-devices.yaml | 6 -
> > > > MAINTAINERS | 8 +
> > > > drivers/input/keyboard/Kconfig | 3 +
> > > > drivers/input/keyboard/adp5589-keys.c | 1397
> > > > +++++++++++++-------
> > > > include/linux/input/adp5589.h | 180 ---
> > > > 6 files changed, 1254 insertions(+), 650 deletions(-)
> > > > ---
> > > > base-commit: c7bf046925dc5885d9c4d8fbcbb7e4e73665bfcf
> > > > change-id: 20240930-b4-dev-adp5589-fw-conversion-955b2f42da70
> > > > --
> > > >
> > > > Thanks!
> > > > - Nuno Sá
> > >
> > > Hi Dmitry,
> > >
> > > Something really caught my attention now while checking 6.12 merge window.
> > > It seems
> > > we have a new MFD device for adp5585 [1] which adds duplicated
> > > functionality (that
> > > was already present in adp5589-keys.c). So, having this as MFD might makes
> > > sense
> > > (even though it makes it harder to validate the keys and to make use of
> > > gpio-keys)
> > > but we are now duplicating GPIO support. Bottom line, not sure what we
> > > should do next
> > > and should I proceed for v2?
> > >
> > > Also ccing Lee and Bartosz...
> >
> > Let's add Laurent and Krzysztof too please.
> >
> > I am surprised we do not see warnings for various bots because
> > "adi,adp5585" compatible is present in trivial devices.
> >
> > I think moving it all to MFD makes sense (I think original drivers were
> > added well before we had MFD infrastructure), but we need to make sure
> > the device tree binding is complete and allows describing keypad (and if
> > not maybe we can pull it from the release and work on it so that it
> > describes the hardware fully).
>
> Keypad support is nice. I didn't include it in my adp5585 driver
> submission because I had no way to test it. Would it be more difficult
> to add it to the MFD driver, compared to what is done in this series ?
Well, I still wonder about the GPIO part of it... It's duplicating something
that existed before even if we all agree that MFD makes more sense.
Anyways, there's no point in over discussing this... I'll see how it works
sending my v2 on top of the MFD implementations. This means that adp5589 also
needs to be added.
- Nuno Sá
^ permalink raw reply [flat|nested] 28+ messages in thread