LinuxLists.cc - [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

2022-08-19 07:19:36

Subject: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

From: Gireesh Hiremath <[email protected]>

Switch from gpio API to gpiod API

Signed-off-by: Gireesh Hiremath <[email protected]>

Gbp-Pq: Topic apertis/guardian
Gbp-Pq: Name driver-input-matric-keypad-switch-gpio-to-gpiod.patch
---
drivers/input/keyboard/matrix_keypad.c | 84 ++++++++++----------------
include/linux/input/matrix_keypad.h | 4 +-
2 files changed, 33 insertions(+), 55 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 30924b57058f..b99574edad9c 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -15,11 +15,10 @@
#include <linux/interrupt.h>
#include <linux/jiffies.h>
#include <linux/module.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/input/matrix_keypad.h>
#include <linux/slab.h>
#include <linux/of.h>
-#include <linux/of_gpio.h>
#include <linux/of_platform.h>

struct matrix_keypad {
@@ -49,11 +48,11 @@ static void __activate_col(const struct matrix_keypad_platform_data *pdata,
bool level_on = !pdata->active_low;

if (on) {
- gpio_direction_output(pdata->col_gpios[col], level_on);
+ gpiod_direction_output(pdata->col_gpios[col], level_on);
} else {
- gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
+ gpiod_set_value_cansleep(pdata->col_gpios[col], !level_on);
if (!pdata->drive_inactive_cols)
- gpio_direction_input(pdata->col_gpios[col]);
+ gpiod_direction_input(pdata->col_gpios[col]);
}
}

@@ -78,7 +77,7 @@ static void activate_all_cols(const struct matrix_keypad_platform_data *pdata,
static bool row_asserted(const struct matrix_keypad_platform_data *pdata,
int row)
{
- return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
+ return gpiod_get_value_cansleep(pdata->row_gpios[row]) ?
!pdata->active_low : pdata->active_low;
}

@@ -91,7 +90,7 @@ static void enable_row_irqs(struct matrix_keypad *keypad)
enable_irq(pdata->clustered_irq);
else {
for (i = 0; i < pdata->num_row_gpios; i++)
- enable_irq(gpio_to_irq(pdata->row_gpios[i]));
+ enable_irq(gpiod_to_irq(pdata->row_gpios[i]));
}
}

@@ -104,7 +103,7 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
disable_irq_nosync(pdata->clustered_irq);
else {
for (i = 0; i < pdata->num_row_gpios; i++)
- disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
+ disable_irq_nosync(gpiod_to_irq(pdata->row_gpios[i]));
}
}

@@ -230,7 +229,7 @@ static void matrix_keypad_stop(struct input_dev *dev)
static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
{
const struct matrix_keypad_platform_data *pdata = keypad->pdata;
- unsigned int gpio;
+ struct gpio_desc *gpio;
int i;

if (pdata->clustered_irq > 0) {
@@ -242,7 +241,7 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
if (!test_bit(i, keypad->disabled_gpios)) {
gpio = pdata->row_gpios[i];

- if (enable_irq_wake(gpio_to_irq(gpio)) == 0)
+ if (enable_irq_wake(gpiod_to_irq(gpio)) == 0)
__set_bit(i, keypad->disabled_gpios);
}
}
@@ -252,7 +251,7 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
static void matrix_keypad_disable_wakeup(struct matrix_keypad *keypad)
{
const struct matrix_keypad_platform_data *pdata = keypad->pdata;
- unsigned int gpio;
+ struct gpio_desc *gpio;
int i;

if (pdata->clustered_irq > 0) {
@@ -264,7 +263,7 @@ static void matrix_keypad_disable_wakeup(struct matrix_keypad *keypad)
for (i = 0; i < pdata->num_row_gpios; i++) {
if (test_and_clear_bit(i, keypad->disabled_gpios)) {
gpio = pdata->row_gpios[i];
- disable_irq_wake(gpio_to_irq(gpio));
+ disable_irq_wake(gpiod_to_irq(gpio));
}
}
}
@@ -308,27 +307,11 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,

/* initialized strobe lines as outputs, activated */
for (i = 0; i < pdata->num_col_gpios; i++) {
- err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
- if (err) {
- dev_err(&pdev->dev,
- "failed to request GPIO%d for COL%d\n",
- pdata->col_gpios[i], i);
- goto err_free_cols;
- }
-
- gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
+ gpiod_direction_output(pdata->col_gpios[i], !pdata->active_low);
}

for (i = 0; i < pdata->num_row_gpios; i++) {
- err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
- if (err) {
- dev_err(&pdev->dev,
- "failed to request GPIO%d for ROW%d\n",
- pdata->row_gpios[i], i);
- goto err_free_rows;
- }
-
- gpio_direction_input(pdata->row_gpios[i]);
+ gpiod_direction_input(pdata->row_gpios[i]);
}

if (pdata->clustered_irq > 0) {
@@ -344,7 +327,7 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
} else {
for (i = 0; i < pdata->num_row_gpios; i++) {
err = request_any_context_irq(
- gpio_to_irq(pdata->row_gpios[i]),
+ gpiod_to_irq(pdata->row_gpios[i]),
matrix_keypad_interrupt,
IRQF_TRIGGER_RISING |
IRQF_TRIGGER_FALLING,
@@ -352,7 +335,7 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
if (err < 0) {
dev_err(&pdev->dev,
"Unable to acquire interrupt for GPIO line %i\n",
- pdata->row_gpios[i]);
+ i);
goto err_free_irqs;
}
}
@@ -364,15 +347,12 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,

err_free_irqs:
while (--i >= 0)
- free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+ free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
i = pdata->num_row_gpios;
err_free_rows:
while (--i >= 0)
- gpio_free(pdata->row_gpios[i]);
+ gpiod_put(pdata->row_gpios[i]);
i = pdata->num_col_gpios;
-err_free_cols:
- while (--i >= 0)
- gpio_free(pdata->col_gpios[i]);

return err;
}
@@ -386,14 +366,14 @@ static void matrix_keypad_free_gpio(struct matrix_keypad *keypad)
free_irq(pdata->clustered_irq, keypad);
} else {
for (i = 0; i < pdata->num_row_gpios; i++)
- free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
+ free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
}

for (i = 0; i < pdata->num_row_gpios; i++)
- gpio_free(pdata->row_gpios[i]);
+ gpiod_put(pdata->row_gpios[i]);

for (i = 0; i < pdata->num_col_gpios; i++)
- gpio_free(pdata->col_gpios[i]);
+ gpiod_put(pdata->col_gpios[i]);
}

#ifdef CONFIG_OF
@@ -402,7 +382,8 @@ matrix_keypad_parse_dt(struct device *dev)
{
struct matrix_keypad_platform_data *pdata;
struct device_node *np = dev->of_node;
- unsigned int *gpios;
+ struct gpio_desc **gpios;
+ struct gpio_desc *desc;
int ret, i, nrow, ncol;

if (!np) {
@@ -416,8 +397,8 @@ matrix_keypad_parse_dt(struct device *dev)
return ERR_PTR(-ENOMEM);
}

- pdata->num_row_gpios = nrow = of_gpio_named_count(np, "row-gpios");
- pdata->num_col_gpios = ncol = of_gpio_named_count(np, "col-gpios");
+ pdata->num_row_gpios = nrow = gpiod_count(dev, "row");
+ pdata->num_col_gpios = ncol = gpiod_count(dev, "col");
if (nrow <= 0 || ncol <= 0) {
dev_err(dev, "number of keypad rows/columns not specified\n");
return ERR_PTR(-EINVAL);
@@ -429,9 +410,6 @@ matrix_keypad_parse_dt(struct device *dev)
pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
of_property_read_bool(np, "linux,wakeup"); /* legacy */

- if (of_get_property(np, "gpio-activelow", NULL))
- pdata->active_low = true;
-
pdata->drive_inactive_cols =
of_property_read_bool(np, "drive-inactive-cols");

@@ -441,7 +419,7 @@ matrix_keypad_parse_dt(struct device *dev)

gpios = devm_kcalloc(dev,
pdata->num_row_gpios + pdata->num_col_gpios,
- sizeof(unsigned int),
+ sizeof(*gpios),
GFP_KERNEL);
if (!gpios) {
dev_err(dev, "could not allocate memory for gpios\n");
@@ -449,17 +427,17 @@ matrix_keypad_parse_dt(struct device *dev)
}

for (i = 0; i < nrow; i++) {
- ret = of_get_named_gpio(np, "row-gpios", i);
- if (ret < 0)
+ desc = devm_gpiod_get_index(dev, "row", i, GPIOD_IN);
+ if (IS_ERR(desc))
return ERR_PTR(ret);
- gpios[i] = ret;
+ gpios[i] = desc;
}

for (i = 0; i < ncol; i++) {
- ret = of_get_named_gpio(np, "col-gpios", i);
- if (ret < 0)
+ desc = devm_gpiod_get_index(dev, "col", i, GPIOD_IN);
+ if (IS_ERR(desc))
return ERR_PTR(ret);
- gpios[nrow + i] = ret;
+ gpios[nrow + i] = desc;
}

pdata->row_gpios = gpios;
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
index 9476768c3b90..8ad7d4626e62 100644
--- a/include/linux/input/matrix_keypad.h
+++ b/include/linux/input/matrix_keypad.h
@@ -59,8 +59,8 @@ struct matrix_keymap_data {
struct matrix_keypad_platform_data {
const struct matrix_keymap_data *keymap_data;

- const unsigned int *row_gpios;
- const unsigned int *col_gpios;
+ struct gpio_desc **row_gpios;
+ struct gpio_desc **col_gpios;

unsigned int num_row_gpios;
unsigned int num_col_gpios;
--
2.20.1

2022-08-19 07:20:38

by Gireesh.Hiremath

[permalink] [raw]

Subject: [PATCH v3 2/3] driver: input: matric-keypad: add reduced matrix support

From: Gireesh Hiremath <[email protected]>

Add support to reduced matrix keypad driver.
Tested on Bosch Guardian Dtect board(TI-am335x).

Signed-off-by: Gireesh Hiremath <[email protected]>
---
drivers/input/keyboard/matrix_keypad.c | 559 +++++++++++++++++++++----
include/linux/input/matrix_keypad.h | 68 +--
2 files changed, 520 insertions(+), 107 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index b99574edad9c..72c0663b6ed3 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -21,9 +21,30 @@
#include <linux/of.h>
#include <linux/of_platform.h>

+struct button_states {
+ uint8_t previous_state : 1;
+ uint8_t current_state : 1;
+};
+
+union button_event {
+ uint8_t clear_event;
+ struct {
+ uint8_t global_changed : 1;
+ uint8_t pressed : 1;
+ uint8_t released : 1;
+ } status;
+};
+
+struct button {
+ uint8_t key;
+ union button_event event;
+ struct button_states state;
+};
+
struct matrix_keypad {
const struct matrix_keypad_platform_data *pdata;
struct input_dev *input_dev;
+ struct button *button_array;
unsigned int row_shift;

DECLARE_BITMAP(disabled_gpios, MATRIX_MAX_ROWS);
@@ -31,11 +52,34 @@ struct matrix_keypad {
uint32_t last_key_state[MATRIX_MAX_COLS];
struct delayed_work work;
spinlock_t lock;
+ int8_t current_line_gpio;
bool scan_pending;
bool stopped;
bool gpio_all_disabled;
};

+struct keypad_info {
+ unsigned char mode;
+};
+
+static const struct keypad_info keypad_infos[] = {
+ {
+ .mode = GENERIC,
+ },
+ {
+ .mode = REDUCED,
+ },
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id matrix_keypad_dt_match[] = {
+ { .compatible = "gpio-matrix-keypad", .data = &keypad_infos[0] },
+ { .compatible = "gpio-matrix-keypad-reduced",
+ .data = &keypad_infos[1] },
+ {}
+};
+MODULE_DEVICE_TABLE(of, matrix_keypad_dt_match);
+#endif
/*
* NOTE: If drive_inactive_cols is false, then the GPIO has to be put into
* HiZ when de-activated to cause minmal side effect when scanning other
@@ -78,7 +122,8 @@ static bool row_asserted(const struct matrix_keypad_platform_data *pdata,
int row)
{
return gpiod_get_value_cansleep(pdata->row_gpios[row]) ?
- !pdata->active_low : pdata->active_low;
+ !pdata->active_low :
+ pdata->active_low;
}

static void enable_row_irqs(struct matrix_keypad *keypad)
@@ -107,6 +152,247 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
}
}

+static void
+__activate_line_driving(const struct matrix_keypad_platform_data *pdata,
+ int line, bool on)
+{
+ bool level_on = pdata->active_low;
+
+ if (on)
+ gpiod_direction_output(pdata->desc_gpios[line], level_on);
+ else
+ gpiod_direction_input(pdata->desc_gpios[line]);
+}
+
+static void
+activate_line_driving(const struct matrix_keypad_platform_data *pdata, int line,
+ bool on)
+{
+ __activate_line_driving(pdata, line, on);
+
+ if (on && pdata->col_scan_delay_us)
+ udelay(pdata->col_scan_delay_us);
+}
+
+static void button_init(struct button *btn, bool btn_hw_state, int key)
+{
+ btn->state.previous_state = btn_hw_state;
+ btn->state.current_state = btn_hw_state;
+ btn->event.clear_event = 0;
+ btn->key = key;
+}
+
+static bool check_button_changes(struct button *btn)
+{
+ /* Check if Button is pressed */
+ if ((!btn->state.previous_state) && btn->state.current_state)
+ btn->event.status.pressed = true;
+
+ /* Check if Button is released */
+ else if (btn->state.previous_state && (!btn->state.current_state))
+ btn->event.status.released = true;
+
+ if (btn->event.clear_event != 0)
+ btn->event.status.global_changed = true;
+
+ btn->state.previous_state = btn->state.current_state;
+
+ return btn->event.status.global_changed;
+}
+
+static union button_event get_and_clear_events(struct button *btn)
+{
+ union button_event temp = btn->event;
+
+ btn->event.clear_event = 0;
+
+ return temp;
+}
+
+static union button_event get_and_clear_btn_events(struct matrix_keypad *keypad,
+ int btn_index)
+{
+ if (btn_index < keypad->pdata->num_of_buttons)
+ return get_and_clear_events(&keypad->button_array[btn_index]);
+ else
+ return get_and_clear_events(&keypad->button_array[0]);
+}
+
+static void button_hdl_init(struct matrix_keypad *keypad)
+{
+ const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+ int row, col, index;
+ int i;
+
+ /* Init Button Objects, will be reinited once states are captured */
+ i = 0;
+ for (row = 1; row < pdata->num_line_gpios; row++)
+ for (col = 0; col < row; col++) {
+ index = (row * pdata->num_line_gpios) + col;
+ if (pdata->keycode[index] != pdata->keycode[0]) {
+ if (i < pdata->num_of_buttons) {
+ button_init(&keypad->button_array[i],
+ false,
+ pdata->keycode[index]);
+ i++;
+ }
+ }
+ }
+
+ pr_debug("[matrix-keypad]: %s Done\n", __func__);
+}
+
+static void on_button_event(struct matrix_keypad *keypad, int btn_index,
+ union button_event btn_event,
+ struct input_dev *input_dev)
+{
+ unsigned int key_code = 0;
+ int key_value = 0;
+
+ key_code = keypad->button_array[btn_index].key;
+
+ if (btn_event.status.pressed) {
+ key_value = 1;
+ pr_debug("[matrix-keypad]:%d Pressed\n", key_code);
+ }
+
+ if (btn_event.status.released) {
+ key_value = 0;
+ pr_debug("[matrix-keypad]:%d Released\n", key_code);
+ }
+
+ input_report_key(input_dev, key_code, key_value);
+ input_sync(input_dev);
+}
+
+static void process_button_events(struct matrix_keypad *keypad,
+ struct input_dev *input_dev)
+{
+ int btn_index;
+
+ for (btn_index = 0; btn_index < keypad->pdata->num_of_buttons;
+ btn_index++) {
+ const union button_event beEvent =
+ get_and_clear_btn_events(keypad, (int)btn_index);
+
+ if (beEvent.status.global_changed) {
+ on_button_event(keypad, (int)btn_index, beEvent,
+ input_dev);
+ }
+ }
+}
+
+static bool get_gpio_line_value(const struct matrix_keypad_platform_data *pdata,
+ int row)
+{
+ return gpiod_get_value(pdata->desc_gpios[row]) ? pdata->active_low :
+ !pdata->active_low;
+}
+
+static uint8_t get_btn_index(struct matrix_keypad *keypad, int btn_key)
+{
+ uint8_t i;
+
+ for (i = 0; i < keypad->pdata->num_of_buttons; i++) {
+ if (keypad->button_array[i].key == btn_key)
+ break;
+ }
+ return i;
+}
+
+static void set_btn_state_by_hw(struct button *btn, bool buttonstate)
+{
+ btn->state.current_state = buttonstate;
+}
+
+static void poll_prepare(struct matrix_keypad *keypad)
+{
+ const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+
+ keypad->current_line_gpio = 0;
+ activate_line_driving(pdata, (int)keypad->current_line_gpio, true);
+}
+
+static void poll_process(struct matrix_keypad *keypad,
+ struct input_dev *input_dev)
+{
+ const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+ bool btn_changes_occured = false;
+ int btn_index;
+
+ for (btn_index = 0; btn_index < pdata->num_of_buttons; btn_index++) {
+ btn_changes_occured |=
+ check_button_changes(&keypad->button_array[btn_index]);
+ }
+
+ if (btn_changes_occured)
+ process_button_events(keypad, input_dev);
+
+ keypad->current_line_gpio = 0;
+}
+
+static void poll_update(struct matrix_keypad *keypad,
+ struct input_dev *input_dev)
+{
+ const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+ uint8_t *btn_keylines;
+ uint8_t number_of_buttons_pressed = 0;
+ uint8_t btn_index;
+ uint8_t btn_key;
+ uint16_t index;
+ int i;
+
+ btn_keylines =
+ kcalloc(pdata->num_line_gpios, sizeof(uint8_t), GFP_KERNEL);
+ for (i = 0; i < pdata->num_line_gpios; i++) {
+ index = (keypad->current_line_gpio * pdata->num_line_gpios) + i;
+ btn_key = pdata->keycode[index];
+ btn_keylines[i] = false;
+
+ if ((btn_key != pdata->keycode[0]) &&
+ (get_gpio_line_value(pdata, (int)i) != false)) {
+ btn_keylines[i] = true;
+ number_of_buttons_pressed++;
+ }
+ }
+ if (number_of_buttons_pressed < 2) {
+ for (i = 0; i < pdata->num_line_gpios; i++) {
+ index = (keypad->current_line_gpio *
+ pdata->num_line_gpios) +
+ i;
+ btn_key = pdata->keycode[index];
+ if (btn_key != pdata->keycode[0]) {
+ btn_index = get_btn_index(keypad, btn_key);
+ set_btn_state_by_hw(
+ &keypad->button_array[btn_index],
+ btn_keylines[i]);
+ }
+ }
+ }
+
+ kfree(btn_keylines);
+ activate_line_driving(pdata, (int)keypad->current_line_gpio, false);
+ keypad->current_line_gpio++;
+ activate_line_driving(
+ pdata, (int)(keypad->current_line_gpio % pdata->num_line_gpios),
+ true);
+}
+
+static void matrix_keypad_poll(struct input_dev *input)
+{
+ struct matrix_keypad *keypad = input_get_drvdata(input);
+ struct input_dev *input_dev = keypad->input_dev;
+
+ if (keypad->stopped == false) {
+ if (keypad->current_line_gpio ==
+ keypad->pdata->num_line_gpios) {
+ poll_process(keypad, input_dev);
+ } else {
+ poll_update(keypad, input_dev);
+ }
+ }
+}
+
/*
* This gets the keys from keyboard and reports it to input subsystem
*/
@@ -127,7 +413,6 @@ static void matrix_keypad_scan(struct work_struct *work)

/* assert each column and read the row status out */
for (col = 0; col < pdata->num_col_gpios; col++) {
-
activate_col(pdata, col, true);

for (row = 0; row < pdata->num_row_gpios; row++)
@@ -150,8 +435,7 @@ static void matrix_keypad_scan(struct work_struct *work)

code = MATRIX_SCAN_CODE(row, col, keypad->row_shift);
input_event(input_dev, EV_MSC, MSC_SCAN, code);
- input_report_key(input_dev,
- keycodes[code],
+ input_report_key(input_dev, keycodes[code],
new_state[col] & (1 << row));
}
}
@@ -186,7 +470,7 @@ static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
disable_row_irqs(keypad);
keypad->scan_pending = true;
schedule_delayed_work(&keypad->work,
- msecs_to_jiffies(keypad->pdata->debounce_ms));
+ msecs_to_jiffies(keypad->pdata->debounce_ms));

out:
spin_unlock_irqrestore(&keypad->lock, flags);
@@ -204,7 +488,8 @@ static int matrix_keypad_start(struct input_dev *dev)
* Schedule an immediate key scan to capture current key state;
* columns will be activated and IRQs be enabled after the scan.
*/
- schedule_delayed_work(&keypad->work, 0);
+ if (keypad->pdata->mode == GENERIC)
+ schedule_delayed_work(&keypad->work, 0);

return 0;
}
@@ -217,7 +502,9 @@ static void matrix_keypad_stop(struct input_dev *dev)
keypad->stopped = true;
spin_unlock_irq(&keypad->lock);

- flush_delayed_work(&keypad->work);
+ if (keypad->pdata->mode == GENERIC)
+ flush_delayed_work(&keypad->work);
+
/*
* matrix_keypad_scan() will leave IRQs enabled;
* we should disable them now.
@@ -236,7 +523,6 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
if (enable_irq_wake(pdata->clustered_irq) == 0)
keypad->gpio_all_disabled = true;
} else {
-
for (i = 0; i < pdata->num_row_gpios; i++) {
if (!test_bit(i, keypad->disabled_gpios)) {
gpio = pdata->row_gpios[i];
@@ -296,8 +582,8 @@ static int matrix_keypad_resume(struct device *dev)
}
#endif

-static SIMPLE_DEV_PM_OPS(matrix_keypad_pm_ops,
- matrix_keypad_suspend, matrix_keypad_resume);
+static SIMPLE_DEV_PM_OPS(matrix_keypad_pm_ops, matrix_keypad_suspend,
+ matrix_keypad_resume);

static int matrix_keypad_init_gpio(struct platform_device *pdev,
struct matrix_keypad *keypad)
@@ -316,9 +602,9 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,

if (pdata->clustered_irq > 0) {
err = request_any_context_irq(pdata->clustered_irq,
- matrix_keypad_interrupt,
- pdata->clustered_irq_flags,
- "matrix-keypad", keypad);
+ matrix_keypad_interrupt,
+ pdata->clustered_irq_flags,
+ "matrix-keypad", keypad);
if (err < 0) {
dev_err(&pdev->dev,
"Unable to acquire clustered interrupt\n");
@@ -327,11 +613,10 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
} else {
for (i = 0; i < pdata->num_row_gpios; i++) {
err = request_any_context_irq(
- gpiod_to_irq(pdata->row_gpios[i]),
- matrix_keypad_interrupt,
- IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING,
- "matrix-keypad", keypad);
+ gpiod_to_irq(pdata->row_gpios[i]),
+ matrix_keypad_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+ "matrix-keypad", keypad);
if (err < 0) {
dev_err(&pdev->dev,
"Unable to acquire interrupt for GPIO line %i\n",
@@ -361,30 +646,42 @@ static void matrix_keypad_free_gpio(struct matrix_keypad *keypad)
{
const struct matrix_keypad_platform_data *pdata = keypad->pdata;
int i;
-
- if (pdata->clustered_irq > 0) {
- free_irq(pdata->clustered_irq, keypad);
+ if (pdata->mode == REDUCED) {
+ for (i = 0; i < pdata->num_line_gpios; i++)
+ gpiod_put(pdata->desc_gpios[i]);
} else {
- for (i = 0; i < pdata->num_row_gpios; i++)
- free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
- }
+ if (pdata->clustered_irq > 0) {
+ free_irq(pdata->clustered_irq, keypad);
+ } else {
+ for (i = 0; i < pdata->num_row_gpios; i++)
+ free_irq(gpiod_to_irq(pdata->row_gpios[i]),
+ keypad);
+ }

- for (i = 0; i < pdata->num_row_gpios; i++)
- gpiod_put(pdata->row_gpios[i]);
+ for (i = 0; i < pdata->num_row_gpios; i++)
+ gpiod_put(pdata->row_gpios[i]);

- for (i = 0; i < pdata->num_col_gpios; i++)
- gpiod_put(pdata->col_gpios[i]);
+ for (i = 0; i < pdata->num_col_gpios; i++)
+ gpiod_put(pdata->col_gpios[i]);
+ }
}

#ifdef CONFIG_OF
static struct matrix_keypad_platform_data *
matrix_keypad_parse_dt(struct device *dev)
{
+ const struct of_device_id *of_id =
+ of_match_device(matrix_keypad_dt_match, dev);
struct matrix_keypad_platform_data *pdata;
struct device_node *np = dev->of_node;
+ const struct keypad_info *info = of_id->data;
struct gpio_desc **gpios;
struct gpio_desc *desc;
int ret, i, nrow, ncol;
+ int8_t *keycode;
+ uint32_t *ptr;
+ int num_gpio;
+ int keymap;

if (!np) {
dev_err(dev, "device lacks DT data\n");
@@ -397,11 +694,23 @@ matrix_keypad_parse_dt(struct device *dev)
return ERR_PTR(-ENOMEM);
}

- pdata->num_row_gpios = nrow = gpiod_count(dev, "row");
- pdata->num_col_gpios = ncol = gpiod_count(dev, "col");
- if (nrow <= 0 || ncol <= 0) {
- dev_err(dev, "number of keypad rows/columns not specified\n");
- return ERR_PTR(-EINVAL);
+ pdata->mode = info->mode;
+
+ if (pdata->mode == REDUCED) {
+ num_gpio = gpiod_count(dev, "line");
+ if (num_gpio <= 0) {
+ dev_err(dev, "number of gpio line not specified\n");
+ return ERR_PTR(-EINVAL);
+ }
+ } else {
+ pdata->num_row_gpios = nrow = gpiod_count(dev, "row");
+ pdata->num_col_gpios = ncol = gpiod_count(dev, "col");
+
+ if (nrow <= 0 || ncol <= 0) {
+ dev_err(dev,
+ "number of keypad rows/columns not specified\n");
+ return ERR_PTR(-EINVAL);
+ }
}

if (of_get_property(np, "linux,no-autorepeat", NULL))
@@ -415,34 +724,90 @@ matrix_keypad_parse_dt(struct device *dev)

of_property_read_u32(np, "debounce-delay-ms", &pdata->debounce_ms);
of_property_read_u32(np, "col-scan-delay-us",
- &pdata->col_scan_delay_us);
-
- gpios = devm_kcalloc(dev,
- pdata->num_row_gpios + pdata->num_col_gpios,
- sizeof(*gpios),
- GFP_KERNEL);
- if (!gpios) {
- dev_err(dev, "could not allocate memory for gpios\n");
- return ERR_PTR(-ENOMEM);
- }
+ &pdata->col_scan_delay_us);
+ if (pdata->mode == REDUCED) {
+ gpios = devm_kcalloc(dev, num_gpio, sizeof(*gpios), GFP_KERNEL);
+ if (!gpios)
+ return ERR_PTR(-ENOMEM);
+
+ for (i = 0; i < num_gpio; i++) {
+ desc = devm_gpiod_get_index(dev, "line", i, GPIOD_ASIS);
+ if (IS_ERR(desc))
+ return ERR_PTR(-EPROBE_DEFER);
+ gpios[i] = desc;
+ }

- for (i = 0; i < nrow; i++) {
- desc = devm_gpiod_get_index(dev, "row", i, GPIOD_IN);
- if (IS_ERR(desc))
- return ERR_PTR(ret);
- gpios[i] = desc;
- }
+ pdata->desc_gpios = gpios;
+ pdata->num_line_gpios = num_gpio;
+
+ if (!gpiod_is_active_low(pdata->desc_gpios[0]))
+ pdata->active_low = true;
+ } else {
+ gpios = devm_kcalloc(
+ dev, pdata->num_row_gpios + pdata->num_col_gpios,
+ sizeof(*gpios), GFP_KERNEL);
+ if (!gpios) {
+ dev_err(dev, "could not allocate memory for gpios\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ for (i = 0; i < nrow; i++) {
+ desc = devm_gpiod_get_index(dev, "row", i, GPIOD_IN);
+ if (IS_ERR(desc))
+ return ERR_PTR(ret);
+ gpios[i] = desc;
+ }
+
+ for (i = 0; i < ncol; i++) {
+ desc = devm_gpiod_get_index(dev, "col", i, GPIOD_IN);
+ if (IS_ERR(desc))
+ return ERR_PTR(ret);
+ gpios[nrow + i] = desc;
+ }

- for (i = 0; i < ncol; i++) {
- desc = devm_gpiod_get_index(dev, "col", i, GPIOD_IN);
- if (IS_ERR(desc))
- return ERR_PTR(ret);
- gpios[nrow + i] = desc;
+ pdata->row_gpios = gpios;
+ pdata->col_gpios = &gpios[pdata->num_row_gpios];
}

- pdata->row_gpios = gpios;
- pdata->col_gpios = &gpios[pdata->num_row_gpios];
+ if (pdata->mode == REDUCED) {
+ if (of_property_read_u32(np, "poll-interval-ms",
+ &pdata->poll_interval_ms))
+ pdata->poll_interval_ms = 10;

+ of_property_read_u32(np, "number-of-buttons",
+ &pdata->num_of_buttons);
+ if (pdata->num_of_buttons <= 0) {
+ dev_err(dev, "number of button not specified\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ keymap = device_property_count_u32(dev, "linux,keymap");
+ if (keymap <= 0 ||
+ keymap > (pdata->num_line_gpios * pdata->num_line_gpios)) {
+ dev_err(dev, "linux,keymap property count is invalid");
+ return ERR_PTR(-ENXIO);
+ }
+
+ ptr = devm_kzalloc(dev, (keymap * sizeof(uint32_t)),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ if (device_property_read_u32_array(dev, "linux,keymap", ptr,
+ keymap)) {
+ dev_err(dev, "problem parsing keymap property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ keycode = devm_kzalloc(dev, (keymap * sizeof(int8_t)),
+ GFP_KERNEL);
+ if (!keycode)
+ return ERR_PTR(-ENOMEM);
+
+ pdata->keycode = keycode;
+ for (i = 0; i < keymap; i++)
+ pdata->keycode[i] = KEYCODE(ptr[i]);
+ }
return pdata;
}
#else
@@ -483,22 +848,58 @@ static int matrix_keypad_probe(struct platform_device *pdev)
keypad->pdata = pdata;
keypad->row_shift = get_count_order(pdata->num_col_gpios);
keypad->stopped = true;
- INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+
+ if (pdata->mode == REDUCED) {
+ keypad->button_array = devm_kzalloc(
+ &pdev->dev,
+ sizeof(struct button) * (pdata->num_of_buttons),
+ GFP_KERNEL);
+ if (!keypad->button_array) {
+ dev_err(&pdev->dev,
+ "could not allocate memory for button array\n");
+ goto err_free_mem;
+ ;
+ }
+
+ poll_prepare(keypad);
+
+ err = input_setup_polling(input_dev, matrix_keypad_poll);
+ if (err) {
+ dev_err(&pdev->dev,
+ "unable to set up polling, err=%d\n", err);
+ return err;
+ }
+
+ input_set_poll_interval(input_dev, pdata->poll_interval_ms);
+ } else {
+ INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
+ }
spin_lock_init(&keypad->lock);

- input_dev->name = pdev->name;
- input_dev->id.bustype = BUS_HOST;
- input_dev->dev.parent = &pdev->dev;
- input_dev->open = matrix_keypad_start;
- input_dev->close = matrix_keypad_stop;
-
- err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
- pdata->num_row_gpios,
- pdata->num_col_gpios,
- NULL, input_dev);
- if (err) {
- dev_err(&pdev->dev, "failed to build keymap\n");
- goto err_free_mem;
+ input_dev->name = pdev->name;
+ input_dev->id.bustype = BUS_HOST;
+ input_dev->dev.parent = &pdev->dev;
+ input_dev->open = matrix_keypad_start;
+ input_dev->close = matrix_keypad_stop;
+
+ if (pdata->mode == REDUCED) {
+ err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
+ pdata->num_line_gpios,
+ pdata->num_line_gpios, NULL,
+ input_dev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to build keymap for reduced mode\n");
+ goto err_free_mem;
+ }
+ } else {
+ err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
+ pdata->num_row_gpios,
+ pdata->num_col_gpios, NULL,
+ input_dev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to build keymap for generic mode\n");
+ goto err_free_mem;
+ }
}

if (!pdata->no_autorepeat)
@@ -506,9 +907,13 @@ static int matrix_keypad_probe(struct platform_device *pdev)
input_set_capability(input_dev, EV_MSC, MSC_SCAN);
input_set_drvdata(input_dev, keypad);

- err = matrix_keypad_init_gpio(pdev, keypad);
- if (err)
- goto err_free_mem;
+ if (pdata->mode == REDUCED) {
+ button_hdl_init(keypad);
+ } else {
+ err = matrix_keypad_init_gpio(pdev, keypad);
+ if (err)
+ goto err_free_mem;
+ }

err = input_register_device(keypad->input_dev);
if (err)
@@ -538,14 +943,6 @@ static int matrix_keypad_remove(struct platform_device *pdev)
return 0;
}

-#ifdef CONFIG_OF
-static const struct of_device_id matrix_keypad_dt_match[] = {
- { .compatible = "gpio-matrix-keypad" },
- { }
-};
-MODULE_DEVICE_TABLE(of, matrix_keypad_dt_match);
-#endif
-
static struct platform_driver matrix_keypad_driver = {
.probe = matrix_keypad_probe,
.remove = matrix_keypad_remove,
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
index 8ad7d4626e62..d5ba9ec21752 100644
--- a/include/linux/input/matrix_keypad.h
+++ b/include/linux/input/matrix_keypad.h
@@ -6,18 +6,22 @@
#include <linux/input.h>
#include <linux/of.h>

-#define MATRIX_MAX_ROWS 32
-#define MATRIX_MAX_COLS 32
+#define MATRIX_MAX_ROWS 32
+#define MATRIX_MAX_COLS 32

-#define KEY(row, col, val) ((((row) & (MATRIX_MAX_ROWS - 1)) << 24) |\
- (((col) & (MATRIX_MAX_COLS - 1)) << 16) |\
- ((val) & 0xffff))
+#define KEY(row, col, val) \
+ ((((row) & (MATRIX_MAX_ROWS - 1)) << 24) | \
+ (((col) & (MATRIX_MAX_COLS - 1)) << 16) | ((val)&0xffff))

-#define KEY_ROW(k) (((k) >> 24) & 0xff)
-#define KEY_COL(k) (((k) >> 16) & 0xff)
-#define KEY_VAL(k) ((k) & 0xffff)
+#define KEY_ROW(k) (((k) >> 24) & 0xff)
+#define KEY_COL(k) (((k) >> 16) & 0xff)
+#define KEY_VAL(k) ((k)&0xffff)

-#define MATRIX_SCAN_CODE(row, col, row_shift) (((row) << (row_shift)) + (col))
+#define MATRIX_SCAN_CODE(row, col, row_shift) (((row) << (row_shift)) + (col))
+#define KEYCODE(keymap) (keymap & 0xFFFF)
+
+#define GENERIC (1) /* keypad mode generic */
+#define REDUCED (2) /* keypad mode reduced */

/**
* struct matrix_keymap_data - keymap for matrix keyboards
@@ -30,7 +34,7 @@
*/
struct matrix_keymap_data {
const uint32_t *keymap;
- unsigned int keymap_size;
+ unsigned int keymap_size;
};

/**
@@ -38,6 +42,7 @@ struct matrix_keymap_data {
* @keymap_data: pointer to &matrix_keymap_data
* @row_gpios: pointer to array of gpio numbers representing rows
* @col_gpios: pointer to array of gpio numbers reporesenting colums
+ * @desc_gpios: actual number of gpio used devide in reduced mode
* @num_row_gpios: actual number of row gpios used by device
* @num_col_gpios: actual number of col gpios used by device
* @col_scan_delay_us: delay, measured in microseconds, that is
@@ -46,6 +51,12 @@ struct matrix_keymap_data {
* @clustered_irq: may be specified if interrupts of all row/column GPIOs
* are bundled to one single irq
* @clustered_irq_flags: flags that are needed for the clustered irq
+ * @num_line_gpios: actual number of gpio line in reduced mode
+ * @num_of_buttons: number of physical buttons on keypad
+ * in reduced mode
+ * @poll_interval_ms: interval to poll gpio in reduced mode
+ * @keycode: hold the keycode value
+ * @mode: flag to set mode
* @active_low: gpio polarity
* @wakeup: controls whether the device should be set up as wakeup
* source
@@ -61,32 +72,37 @@ struct matrix_keypad_platform_data {

struct gpio_desc **row_gpios;
struct gpio_desc **col_gpios;
+ struct gpio_desc **desc_gpios;

- unsigned int num_row_gpios;
- unsigned int num_col_gpios;
+ unsigned int num_row_gpios;
+ unsigned int num_col_gpios;

- unsigned int col_scan_delay_us;
+ unsigned int col_scan_delay_us;

/* key debounce interval in milli-second */
- unsigned int debounce_ms;
+ unsigned int debounce_ms;
+
+ unsigned int clustered_irq;
+ unsigned int clustered_irq_flags;
+ unsigned int num_line_gpios;
+ unsigned int num_of_buttons;
+ unsigned int poll_interval_ms;

- unsigned int clustered_irq;
- unsigned int clustered_irq_flags;
+ int8_t *keycode;
+ int8_t mode;

- bool active_low;
- bool wakeup;
- bool no_autorepeat;
- bool drive_inactive_cols;
+ bool active_low;
+ bool wakeup;
+ bool no_autorepeat;
+ bool drive_inactive_cols;
};

int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
- const char *keymap_name,
- unsigned int rows, unsigned int cols,
- unsigned short *keymap,
+ const char *keymap_name, unsigned int rows,
+ unsigned int cols, unsigned short *keymap,
struct input_dev *input_dev);
-int matrix_keypad_parse_properties(struct device *dev,
- unsigned int *rows, unsigned int *cols);
-
+int matrix_keypad_parse_properties(struct device *dev, unsigned int *rows,
+ unsigned int *cols);
#define matrix_keypad_parse_of_params matrix_keypad_parse_properties

#endif /* _MATRIX_KEYPAD_H */
--
2.20.1

2022-08-19 10:46:07

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v3 2/3] driver: input: matric-keypad: add reduced matrix support

2022-08-19 12:17:27

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH v3 2/3] driver: input: matric-keypad: add reduced matrix support

On 19/08/2022 09:59, [email protected] wrote:
>
> struct gpio_desc **row_gpios;
> struct gpio_desc **col_gpios;
> + struct gpio_desc **desc_gpios;
>
> - unsigned int num_row_gpios;
> - unsigned int num_col_gpios;
> + unsigned int num_row_gpios;
> + unsigned int num_col_gpios;

Do not mix up unrelated changes... One logical change, one commit.
Entire patch should be cleanup from this (it's not only here but in
several places). Except that you have build failures to fix...

Best regards,
Krzysztof

2022-08-19 12:18:39

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v3 2/3] driver: input: matric-keypad: add reduced matrix support

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on linus/master v6.0-rc1 next-20220819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Gireesh-Hiremath-in-bosch-com/driver-input-matric-keypad-switch-to-gpiod/20220819-151155
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: s390-randconfig-r044-20220819 (https://download.01.org/0day-ci/archive/20220819/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 0ac597f3cacf60479ffd36b03766fa7462dabd78)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/a0b420e08e3b8775a3dbc4857f6ef4831db1c2b3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Gireesh-Hiremath-in-bosch-com/driver-input-matric-keypad-switch-to-gpiod/20220819-151155
git checkout a0b420e08e3b8775a3dbc4857f6ef4831db1c2b3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/input/keyboard/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from drivers/input/keyboard/matrix_keypad.c:14:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/input/keyboard/matrix_keypad.c:14:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/input/keyboard/matrix_keypad.c:14:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/input/keyboard/matrix_keypad.c:857:3: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!keypad->button_array) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:56:28: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/input/keyboard/matrix_keypad.c:932:9: note: uninitialized use occurs here
return err;
^~~
drivers/input/keyboard/matrix_keypad.c:857:3: note: remove the 'if' if its condition is always false
if (!keypad->button_array) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:56:23: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^
drivers/input/keyboard/matrix_keypad.c:828:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
>> drivers/input/keyboard/matrix_keypad.c:65:33: warning: unused variable 'keypad_infos' [-Wunused-const-variable]
static const struct keypad_info keypad_infos[] = {
^
14 warnings generated.

vim +857 drivers/input/keyboard/matrix_keypad.c

822
823 static int matrix_keypad_probe(struct platform_device *pdev)
824 {
825 const struct matrix_keypad_platform_data *pdata;
826 struct matrix_keypad *keypad;
827 struct input_dev *input_dev;
> 828 int err;
829
830 pdata = dev_get_platdata(&pdev->dev);
831 if (!pdata) {
832 pdata = matrix_keypad_parse_dt(&pdev->dev);
833 if (IS_ERR(pdata))
834 return PTR_ERR(pdata);
835 } else if (!pdata->keymap_data) {
836 dev_err(&pdev->dev, "no keymap data defined\n");
837 return -EINVAL;
838 }
839
840 keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
841 input_dev = input_allocate_device();
842 if (!keypad || !input_dev) {
843 err = -ENOMEM;
844 goto err_free_mem;
845 }
846
847 keypad->input_dev = input_dev;
848 keypad->pdata = pdata;
849 keypad->row_shift = get_count_order(pdata->num_col_gpios);
850 keypad->stopped = true;
851
852 if (pdata->mode == REDUCED) {
853 keypad->button_array = devm_kzalloc(
854 &pdev->dev,
855 sizeof(struct button) * (pdata->num_of_buttons),
856 GFP_KERNEL);
> 857 if (!keypad->button_array) {
858 dev_err(&pdev->dev,
859 "could not allocate memory for button array\n");
860 goto err_free_mem;
861 ;
862 }
863
864 poll_prepare(keypad);
865
866 err = input_setup_polling(input_dev, matrix_keypad_poll);
867 if (err) {
868 dev_err(&pdev->dev,
869 "unable to set up polling, err=%d\n", err);
870 return err;
871 }
872
873 input_set_poll_interval(input_dev, pdata->poll_interval_ms);
874 } else {
875 INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
876 }
877 spin_lock_init(&keypad->lock);
878
879 input_dev->name = pdev->name;
880 input_dev->id.bustype = BUS_HOST;
881 input_dev->dev.parent = &pdev->dev;
882 input_dev->open = matrix_keypad_start;
883 input_dev->close = matrix_keypad_stop;
884
885 if (pdata->mode == REDUCED) {
886 err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
887 pdata->num_line_gpios,
888 pdata->num_line_gpios, NULL,
889 input_dev);
890 if (err) {
891 dev_err(&pdev->dev, "failed to build keymap for reduced mode\n");
892 goto err_free_mem;
893 }
894 } else {
895 err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
896 pdata->num_row_gpios,
897 pdata->num_col_gpios, NULL,
898 input_dev);
899 if (err) {
900 dev_err(&pdev->dev, "failed to build keymap for generic mode\n");
901 goto err_free_mem;
902 }
903 }
904
905 if (!pdata->no_autorepeat)
906 __set_bit(EV_REP, input_dev->evbit);
907 input_set_capability(input_dev, EV_MSC, MSC_SCAN);
908 input_set_drvdata(input_dev, keypad);
909
910 if (pdata->mode == REDUCED) {
911 button_hdl_init(keypad);
912 } else {
913 err = matrix_keypad_init_gpio(pdev, keypad);
914 if (err)
915 goto err_free_mem;
916 }
917
918 err = input_register_device(keypad->input_dev);
919 if (err)
920 goto err_free_gpio;
921
922 device_init_wakeup(&pdev->dev, pdata->wakeup);
923 platform_set_drvdata(pdev, keypad);
924
925 return 0;
926
927 err_free_gpio:
928 matrix_keypad_free_gpio(keypad);
929 err_free_mem:
930 input_free_device(input_dev);
931 kfree(keypad);
932 return err;
933 }
934

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-08-19 12:46:24

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

On 19/08/2022 09:59, [email protected] wrote:
> From: Gireesh Hiremath <[email protected]>
>
> Switch from gpio API to gpiod API
>
> Signed-off-by: Gireesh Hiremath <[email protected]>
>
> Gbp-Pq: Topic apertis/guardian
> Gbp-Pq: Name driver-input-matric-keypad-switch-gpio-to-gpiod.patch

What's that? I don't recall such tags in Linux kernel process... Is it
documented anywhere?

Best regards,
Krzysztof

2022-08-19 13:37:24

by Marco Felsch

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

Hi Gireesh,

On 22-08-19, [email protected] wrote:
> From: Gireesh Hiremath <[email protected]>
>
> Switch from gpio API to gpiod API

Nice change.

Did you checked the users of this driver? This change changes the
behaviour for actice_low GPIOs. A quick check showed that at least on
upstream board: arch/arm/mach-pxa/palmtc.c uses active low GPIOs.

> Signed-off-by: Gireesh Hiremath <[email protected]>
>
> Gbp-Pq: Topic apertis/guardian
> Gbp-Pq: Name driver-input-matric-keypad-switch-gpio-to-gpiod.patch

Please drop this internal tags.

> ---
> drivers/input/keyboard/matrix_keypad.c | 84 ++++++++++----------------
> include/linux/input/matrix_keypad.h | 4 +-
> 2 files changed, 33 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> index 30924b57058f..b99574edad9c 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -15,11 +15,10 @@
> #include <linux/interrupt.h>
> #include <linux/jiffies.h>
> #include <linux/module.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/input/matrix_keypad.h>
> #include <linux/slab.h>
> #include <linux/of.h>
> -#include <linux/of_gpio.h>
> #include <linux/of_platform.h>
>
> struct matrix_keypad {
> @@ -49,11 +48,11 @@ static void __activate_col(const struct matrix_keypad_platform_data *pdata,
> bool level_on = !pdata->active_low;
>
> if (on) {
> - gpio_direction_output(pdata->col_gpios[col], level_on);
> + gpiod_direction_output(pdata->col_gpios[col], level_on);

Now strange things can happen, if active_low is set and you specified
GPIO_ACTIVE_LOW within the device-tree. We need a way to move to gpiod
and keep the current behaviour.

> } else {
> - gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
> + gpiod_set_value_cansleep(pdata->col_gpios[col], !level_on);
> if (!pdata->drive_inactive_cols)
> - gpio_direction_input(pdata->col_gpios[col]);
> + gpiod_direction_input(pdata->col_gpios[col]);
> }
> }
>
> @@ -78,7 +77,7 @@ static void activate_all_cols(const struct matrix_keypad_platform_data *pdata,
> static bool row_asserted(const struct matrix_keypad_platform_data *pdata,
> int row)
> {
> - return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
> + return gpiod_get_value_cansleep(pdata->row_gpios[row]) ?
> !pdata->active_low : pdata->active_low;
> }
>
> @@ -91,7 +90,7 @@ static void enable_row_irqs(struct matrix_keypad *keypad)
> enable_irq(pdata->clustered_irq);
> else {
> for (i = 0; i < pdata->num_row_gpios; i++)
> - enable_irq(gpio_to_irq(pdata->row_gpios[i]));
> + enable_irq(gpiod_to_irq(pdata->row_gpios[i]));
> }
> }
>
> @@ -104,7 +103,7 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
> disable_irq_nosync(pdata->clustered_irq);
> else {
> for (i = 0; i < pdata->num_row_gpios; i++)
> - disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
> + disable_irq_nosync(gpiod_to_irq(pdata->row_gpios[i]));
> }
> }
>
> @@ -230,7 +229,7 @@ static void matrix_keypad_stop(struct input_dev *dev)
> static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
> {
> const struct matrix_keypad_platform_data *pdata = keypad->pdata;
> - unsigned int gpio;
> + struct gpio_desc *gpio;
> int i;
>
> if (pdata->clustered_irq > 0) {
> @@ -242,7 +241,7 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
> if (!test_bit(i, keypad->disabled_gpios)) {
> gpio = pdata->row_gpios[i];
>
> - if (enable_irq_wake(gpio_to_irq(gpio)) == 0)
> + if (enable_irq_wake(gpiod_to_irq(gpio)) == 0)
> __set_bit(i, keypad->disabled_gpios);
> }
> }
> @@ -252,7 +251,7 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
> static void matrix_keypad_disable_wakeup(struct matrix_keypad *keypad)
> {
> const struct matrix_keypad_platform_data *pdata = keypad->pdata;
> - unsigned int gpio;
> + struct gpio_desc *gpio;
> int i;
>
> if (pdata->clustered_irq > 0) {
> @@ -264,7 +263,7 @@ static void matrix_keypad_disable_wakeup(struct matrix_keypad *keypad)
> for (i = 0; i < pdata->num_row_gpios; i++) {
> if (test_and_clear_bit(i, keypad->disabled_gpios)) {
> gpio = pdata->row_gpios[i];
> - disable_irq_wake(gpio_to_irq(gpio));
> + disable_irq_wake(gpiod_to_irq(gpio));
> }
> }
> }
> @@ -308,27 +307,11 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
>
> /* initialized strobe lines as outputs, activated */
> for (i = 0; i < pdata->num_col_gpios; i++) {
> - err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
> - if (err) {
> - dev_err(&pdev->dev,
> - "failed to request GPIO%d for COL%d\n",
> - pdata->col_gpios[i], i);
> - goto err_free_cols;
> - }
> -
> - gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
> + gpiod_direction_output(pdata->col_gpios[i], !pdata->active_low);
> }
>
> for (i = 0; i < pdata->num_row_gpios; i++) {
> - err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
> - if (err) {
> - dev_err(&pdev->dev,
> - "failed to request GPIO%d for ROW%d\n",
> - pdata->row_gpios[i], i);
> - goto err_free_rows;
> - }
> -
> - gpio_direction_input(pdata->row_gpios[i]);
> + gpiod_direction_input(pdata->row_gpios[i]);
> }
>
> if (pdata->clustered_irq > 0) {
> @@ -344,7 +327,7 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
> } else {
> for (i = 0; i < pdata->num_row_gpios; i++) {
> err = request_any_context_irq(
> - gpio_to_irq(pdata->row_gpios[i]),
> + gpiod_to_irq(pdata->row_gpios[i]),
> matrix_keypad_interrupt,
> IRQF_TRIGGER_RISING |
> IRQF_TRIGGER_FALLING,
> @@ -352,7 +335,7 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
> if (err < 0) {
> dev_err(&pdev->dev,
> "Unable to acquire interrupt for GPIO line %i\n",
> - pdata->row_gpios[i]);
> + i);
> goto err_free_irqs;
> }
> }
> @@ -364,15 +347,12 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
>
> err_free_irqs:
> while (--i >= 0)
> - free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
> + free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
> i = pdata->num_row_gpios;
> err_free_rows:
> while (--i >= 0)
> - gpio_free(pdata->row_gpios[i]);
> + gpiod_put(pdata->row_gpios[i]);
> i = pdata->num_col_gpios;
> -err_free_cols:
> - while (--i >= 0)
> - gpio_free(pdata->col_gpios[i]);
>
> return err;
> }
> @@ -386,14 +366,14 @@ static void matrix_keypad_free_gpio(struct matrix_keypad *keypad)
> free_irq(pdata->clustered_irq, keypad);
> } else {
> for (i = 0; i < pdata->num_row_gpios; i++)
> - free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
> + free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
> }
>
> for (i = 0; i < pdata->num_row_gpios; i++)
> - gpio_free(pdata->row_gpios[i]);
> + gpiod_put(pdata->row_gpios[i]);
>
> for (i = 0; i < pdata->num_col_gpios; i++)
> - gpio_free(pdata->col_gpios[i]);
> + gpiod_put(pdata->col_gpios[i]);
> }
>
> #ifdef CONFIG_OF
> @@ -402,7 +382,8 @@ matrix_keypad_parse_dt(struct device *dev)
> {
> struct matrix_keypad_platform_data *pdata;
> struct device_node *np = dev->of_node;
> - unsigned int *gpios;
> + struct gpio_desc **gpios;
> + struct gpio_desc *desc;
> int ret, i, nrow, ncol;
>
> if (!np) {
> @@ -416,8 +397,8 @@ matrix_keypad_parse_dt(struct device *dev)
> return ERR_PTR(-ENOMEM);
> }
>
> - pdata->num_row_gpios = nrow = of_gpio_named_count(np, "row-gpios");
> - pdata->num_col_gpios = ncol = of_gpio_named_count(np, "col-gpios");
> + pdata->num_row_gpios = nrow = gpiod_count(dev, "row");
> + pdata->num_col_gpios = ncol = gpiod_count(dev, "col");
> if (nrow <= 0 || ncol <= 0) {
> dev_err(dev, "number of keypad rows/columns not specified\n");
> return ERR_PTR(-EINVAL);
> @@ -429,9 +410,6 @@ matrix_keypad_parse_dt(struct device *dev)
> pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
> of_property_read_bool(np, "linux,wakeup"); /* legacy */
>
> - if (of_get_property(np, "gpio-activelow", NULL))
> - pdata->active_low = true;
> -

This removes backward compatibility, please don't do that.

Regards,
Marco

> pdata->drive_inactive_cols =
> of_property_read_bool(np, "drive-inactive-cols");
>
> @@ -441,7 +419,7 @@ matrix_keypad_parse_dt(struct device *dev)
>
> gpios = devm_kcalloc(dev,
> pdata->num_row_gpios + pdata->num_col_gpios,
> - sizeof(unsigned int),
> + sizeof(*gpios),
> GFP_KERNEL);
> if (!gpios) {
> dev_err(dev, "could not allocate memory for gpios\n");
> @@ -449,17 +427,17 @@ matrix_keypad_parse_dt(struct device *dev)
> }
>
> for (i = 0; i < nrow; i++) {
> - ret = of_get_named_gpio(np, "row-gpios", i);
> - if (ret < 0)
> + desc = devm_gpiod_get_index(dev, "row", i, GPIOD_IN);
> + if (IS_ERR(desc))
> return ERR_PTR(ret);
> - gpios[i] = ret;
> + gpios[i] = desc;
> }
>
> for (i = 0; i < ncol; i++) {
> - ret = of_get_named_gpio(np, "col-gpios", i);
> - if (ret < 0)
> + desc = devm_gpiod_get_index(dev, "col", i, GPIOD_IN);
> + if (IS_ERR(desc))
> return ERR_PTR(ret);
> - gpios[nrow + i] = ret;
> + gpios[nrow + i] = desc;
> }
>
> pdata->row_gpios = gpios;
> diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
> index 9476768c3b90..8ad7d4626e62 100644
> --- a/include/linux/input/matrix_keypad.h
> +++ b/include/linux/input/matrix_keypad.h
> @@ -59,8 +59,8 @@ struct matrix_keymap_data {
> struct matrix_keypad_platform_data {
> const struct matrix_keymap_data *keymap_data;
>
> - const unsigned int *row_gpios;
> - const unsigned int *col_gpios;
> + struct gpio_desc **row_gpios;
> + struct gpio_desc **col_gpios;
>
> unsigned int num_row_gpios;
> unsigned int num_col_gpios;
> --
> 2.20.1
>
>

2022-08-20 01:21:22

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

On Fri, Aug 19, 2022 at 03:12:31PM +0200, Marco Felsch wrote:
> Hi Gireesh,
>
> On 22-08-19, [email protected] wrote:
> > From: Gireesh Hiremath <[email protected]>
> >
> > Switch from gpio API to gpiod API
>
> Nice change.
>
> Did you checked the users of this driver? This change changes the
> behaviour for actice_low GPIOs. A quick check showed that at least on
> upstream board: arch/arm/mach-pxa/palmtc.c uses active low GPIOs.
>
> > Signed-off-by: Gireesh Hiremath <[email protected]>
> >
> > Gbp-Pq: Topic apertis/guardian
> > Gbp-Pq: Name driver-input-matric-keypad-switch-gpio-to-gpiod.patch
>
> Please drop this internal tags.
>
> > ---
> > drivers/input/keyboard/matrix_keypad.c | 84 ++++++++++----------------
> > include/linux/input/matrix_keypad.h | 4 +-
> > 2 files changed, 33 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> > index 30924b57058f..b99574edad9c 100644
> > --- a/drivers/input/keyboard/matrix_keypad.c
> > +++ b/drivers/input/keyboard/matrix_keypad.c
> > @@ -15,11 +15,10 @@
> > #include <linux/interrupt.h>
> > #include <linux/jiffies.h>
> > #include <linux/module.h>
> > -#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/input/matrix_keypad.h>
> > #include <linux/slab.h>
> > #include <linux/of.h>
> > -#include <linux/of_gpio.h>
> > #include <linux/of_platform.h>
> >
> > struct matrix_keypad {
> > @@ -49,11 +48,11 @@ static void __activate_col(const struct matrix_keypad_platform_data *pdata,
> > bool level_on = !pdata->active_low;
> >
> > if (on) {
> > - gpio_direction_output(pdata->col_gpios[col], level_on);
> > + gpiod_direction_output(pdata->col_gpios[col], level_on);
>
> Now strange things can happen, if active_low is set and you specified
> GPIO_ACTIVE_LOW within the device-tree. We need a way to move to gpiod
> and keep the current behaviour.
>
> > } else {
> > - gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
> > + gpiod_set_value_cansleep(pdata->col_gpios[col], !level_on);
> > if (!pdata->drive_inactive_cols)
> > - gpio_direction_input(pdata->col_gpios[col]);
> > + gpiod_direction_input(pdata->col_gpios[col]);
> > }
> > }
> >
> > @@ -78,7 +77,7 @@ static void activate_all_cols(const struct matrix_keypad_platform_data *pdata,
> > static bool row_asserted(const struct matrix_keypad_platform_data *pdata,
> > int row)
> > {
> > - return gpio_get_value_cansleep(pdata->row_gpios[row]) ?
> > + return gpiod_get_value_cansleep(pdata->row_gpios[row]) ?
> > !pdata->active_low : pdata->active_low;
> > }
> >
> > @@ -91,7 +90,7 @@ static void enable_row_irqs(struct matrix_keypad *keypad)
> > enable_irq(pdata->clustered_irq);
> > else {
> > for (i = 0; i < pdata->num_row_gpios; i++)
> > - enable_irq(gpio_to_irq(pdata->row_gpios[i]));
> > + enable_irq(gpiod_to_irq(pdata->row_gpios[i]));
> > }
> > }
> >
> > @@ -104,7 +103,7 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
> > disable_irq_nosync(pdata->clustered_irq);
> > else {
> > for (i = 0; i < pdata->num_row_gpios; i++)
> > - disable_irq_nosync(gpio_to_irq(pdata->row_gpios[i]));
> > + disable_irq_nosync(gpiod_to_irq(pdata->row_gpios[i]));
> > }
> > }
> >
> > @@ -230,7 +229,7 @@ static void matrix_keypad_stop(struct input_dev *dev)
> > static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
> > {
> > const struct matrix_keypad_platform_data *pdata = keypad->pdata;
> > - unsigned int gpio;
> > + struct gpio_desc *gpio;
> > int i;
> >
> > if (pdata->clustered_irq > 0) {
> > @@ -242,7 +241,7 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
> > if (!test_bit(i, keypad->disabled_gpios)) {
> > gpio = pdata->row_gpios[i];
> >
> > - if (enable_irq_wake(gpio_to_irq(gpio)) == 0)
> > + if (enable_irq_wake(gpiod_to_irq(gpio)) == 0)
> > __set_bit(i, keypad->disabled_gpios);
> > }
> > }
> > @@ -252,7 +251,7 @@ static void matrix_keypad_enable_wakeup(struct matrix_keypad *keypad)
> > static void matrix_keypad_disable_wakeup(struct matrix_keypad *keypad)
> > {
> > const struct matrix_keypad_platform_data *pdata = keypad->pdata;
> > - unsigned int gpio;
> > + struct gpio_desc *gpio;
> > int i;
> >
> > if (pdata->clustered_irq > 0) {
> > @@ -264,7 +263,7 @@ static void matrix_keypad_disable_wakeup(struct matrix_keypad *keypad)
> > for (i = 0; i < pdata->num_row_gpios; i++) {
> > if (test_and_clear_bit(i, keypad->disabled_gpios)) {
> > gpio = pdata->row_gpios[i];
> > - disable_irq_wake(gpio_to_irq(gpio));
> > + disable_irq_wake(gpiod_to_irq(gpio));
> > }
> > }
> > }
> > @@ -308,27 +307,11 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
> >
> > /* initialized strobe lines as outputs, activated */
> > for (i = 0; i < pdata->num_col_gpios; i++) {
> > - err = gpio_request(pdata->col_gpios[i], "matrix_kbd_col");
> > - if (err) {
> > - dev_err(&pdev->dev,
> > - "failed to request GPIO%d for COL%d\n",
> > - pdata->col_gpios[i], i);
> > - goto err_free_cols;
> > - }
> > -
> > - gpio_direction_output(pdata->col_gpios[i], !pdata->active_low);
> > + gpiod_direction_output(pdata->col_gpios[i], !pdata->active_low);
> > }
> >
> > for (i = 0; i < pdata->num_row_gpios; i++) {
> > - err = gpio_request(pdata->row_gpios[i], "matrix_kbd_row");
> > - if (err) {
> > - dev_err(&pdev->dev,
> > - "failed to request GPIO%d for ROW%d\n",
> > - pdata->row_gpios[i], i);
> > - goto err_free_rows;
> > - }
> > -
> > - gpio_direction_input(pdata->row_gpios[i]);
> > + gpiod_direction_input(pdata->row_gpios[i]);
> > }
> >
> > if (pdata->clustered_irq > 0) {
> > @@ -344,7 +327,7 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
> > } else {
> > for (i = 0; i < pdata->num_row_gpios; i++) {
> > err = request_any_context_irq(
> > - gpio_to_irq(pdata->row_gpios[i]),
> > + gpiod_to_irq(pdata->row_gpios[i]),
> > matrix_keypad_interrupt,
> > IRQF_TRIGGER_RISING |
> > IRQF_TRIGGER_FALLING,
> > @@ -352,7 +335,7 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
> > if (err < 0) {
> > dev_err(&pdev->dev,
> > "Unable to acquire interrupt for GPIO line %i\n",
> > - pdata->row_gpios[i]);
> > + i);
> > goto err_free_irqs;
> > }
> > }
> > @@ -364,15 +347,12 @@ static int matrix_keypad_init_gpio(struct platform_device *pdev,
> >
> > err_free_irqs:
> > while (--i >= 0)
> > - free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
> > + free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
> > i = pdata->num_row_gpios;
> > err_free_rows:
> > while (--i >= 0)
> > - gpio_free(pdata->row_gpios[i]);
> > + gpiod_put(pdata->row_gpios[i]);
> > i = pdata->num_col_gpios;
> > -err_free_cols:
> > - while (--i >= 0)
> > - gpio_free(pdata->col_gpios[i]);
> >
> > return err;
> > }
> > @@ -386,14 +366,14 @@ static void matrix_keypad_free_gpio(struct matrix_keypad *keypad)
> > free_irq(pdata->clustered_irq, keypad);
> > } else {
> > for (i = 0; i < pdata->num_row_gpios; i++)
> > - free_irq(gpio_to_irq(pdata->row_gpios[i]), keypad);
> > + free_irq(gpiod_to_irq(pdata->row_gpios[i]), keypad);
> > }
> >
> > for (i = 0; i < pdata->num_row_gpios; i++)
> > - gpio_free(pdata->row_gpios[i]);
> > + gpiod_put(pdata->row_gpios[i]);
> >
> > for (i = 0; i < pdata->num_col_gpios; i++)
> > - gpio_free(pdata->col_gpios[i]);
> > + gpiod_put(pdata->col_gpios[i]);
> > }
> >
> > #ifdef CONFIG_OF
> > @@ -402,7 +382,8 @@ matrix_keypad_parse_dt(struct device *dev)
> > {
> > struct matrix_keypad_platform_data *pdata;
> > struct device_node *np = dev->of_node;
> > - unsigned int *gpios;
> > + struct gpio_desc **gpios;
> > + struct gpio_desc *desc;
> > int ret, i, nrow, ncol;
> >
> > if (!np) {
> > @@ -416,8 +397,8 @@ matrix_keypad_parse_dt(struct device *dev)
> > return ERR_PTR(-ENOMEM);
> > }
> >
> > - pdata->num_row_gpios = nrow = of_gpio_named_count(np, "row-gpios");
> > - pdata->num_col_gpios = ncol = of_gpio_named_count(np, "col-gpios");
> > + pdata->num_row_gpios = nrow = gpiod_count(dev, "row");
> > + pdata->num_col_gpios = ncol = gpiod_count(dev, "col");
> > if (nrow <= 0 || ncol <= 0) {
> > dev_err(dev, "number of keypad rows/columns not specified\n");
> > return ERR_PTR(-EINVAL);
> > @@ -429,9 +410,6 @@ matrix_keypad_parse_dt(struct device *dev)
> > pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
> > of_property_read_bool(np, "linux,wakeup"); /* legacy */
> >
> > - if (of_get_property(np, "gpio-activelow", NULL))
> > - pdata->active_low = true;
> > -
>
> This removes backward compatibility, please don't do that.

I do not think there is a reasonable way of keeping the compatibility
while using gpiod API (sans abandoning polarity handling and using
*_raw() versions of API).

I would adjust the DTSes in the kernel and move on, especially given
that the DTSes in the kernel are inconsistent - they specify
gpio-activelow attribute while specifying "action high" in gpio
properties).

Thanks.

--
Dmitry

2022-08-20 19:38:44

by Marco Felsch

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

Hi Dmitry,

On 22-08-19, Dmitry Torokhov wrote:
> On Fri, Aug 19, 2022 at 03:12:31PM +0200, Marco Felsch wrote:
> > Hi Gireesh,
> >
> > On 22-08-19, [email protected] wrote:
> > > From: Gireesh Hiremath <[email protected]>
> > >
> > > Switch from gpio API to gpiod API
> >
> > Nice change.
> >
> > Did you checked the users of this driver? This change changes the
> > behaviour for actice_low GPIOs. A quick check showed that at least on
> > upstream board: arch/arm/mach-pxa/palmtc.c uses active low GPIOs.
> >
> > > Signed-off-by: Gireesh Hiremath <[email protected]>
> > >
> > > Gbp-Pq: Topic apertis/guardian
> > > Gbp-Pq: Name driver-input-matric-keypad-switch-gpio-to-gpiod.patch
> >
> > Please drop this internal tags.
> >
> > > ---
> > > drivers/input/keyboard/matrix_keypad.c | 84 ++++++++++----------------
> > > include/linux/input/matrix_keypad.h | 4 +-
> > > 2 files changed, 33 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> > > index 30924b57058f..b99574edad9c 100644
> > > --- a/drivers/input/keyboard/matrix_keypad.c
> > > +++ b/drivers/input/keyboard/matrix_keypad.c
> > > @@ -15,11 +15,10 @@
> > > #include <linux/interrupt.h>
> > > #include <linux/jiffies.h>
> > > #include <linux/module.h>
> > > -#include <linux/gpio.h>
> > > +#include <linux/gpio/consumer.h>
> > > #include <linux/input/matrix_keypad.h>
> > > #include <linux/slab.h>
> > > #include <linux/of.h>
> > > -#include <linux/of_gpio.h>
> > > #include <linux/of_platform.h>
> > >
> > > struct matrix_keypad {
> > > @@ -49,11 +48,11 @@ static void __activate_col(const struct matrix_keypad_platform_data *pdata,
> > > bool level_on = !pdata->active_low;
> > >
> > > if (on) {
> > > - gpio_direction_output(pdata->col_gpios[col], level_on);
> > > + gpiod_direction_output(pdata->col_gpios[col], level_on);
> >
> > Now strange things can happen, if active_low is set and you specified
> > GPIO_ACTIVE_LOW within the device-tree. We need a way to move to gpiod
> > and keep the current behaviour.
> >
> > > } else {
> > > - gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
> > > + gpiod_set_value_cansleep(pdata->col_gpios[col], !level_on);
> > > if (!pdata->drive_inactive_cols)
> > > - gpio_direction_input(pdata->col_gpios[col]);
> > > + gpiod_direction_input(pdata->col_gpios[col]);

...

> > > @@ -429,9 +410,6 @@ matrix_keypad_parse_dt(struct device *dev)
> > > pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
> > > of_property_read_bool(np, "linux,wakeup"); /* legacy */
> > >
> > > - if (of_get_property(np, "gpio-activelow", NULL))
> > > - pdata->active_low = true;
> > > -
> >
> > This removes backward compatibility, please don't do that.
>
> I do not think there is a reasonable way of keeping the compatibility
> while using gpiod API (sans abandoning polarity handling and using
> *_raw() versions of API).
>
> I would adjust the DTSes in the kernel and move on, especially given
> that the DTSes in the kernel are inconsistent - they specify
> gpio-activelow attribute while specifying "action high" in gpio
> properties).

Yes, because the driver didn't respect that by not using the gpiod API.
Got your point for the DTses but what about the boards based on the
platform-data? Those boards must be changed as well.

Also I thought DTB is firmware and we should never break it, now we
doing it by this commit. Just to get your opinion on this.

Regards,
Marco

2022-08-21 05:33:32

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

Hi Marco,

On Sat, Aug 20, 2022 at 09:36:23PM +0200, Marco Felsch wrote:
> Hi Dmitry,
>
> On 22-08-19, Dmitry Torokhov wrote:
> > On Fri, Aug 19, 2022 at 03:12:31PM +0200, Marco Felsch wrote:
> > > Hi Gireesh,
> > >
> > > On 22-08-19, [email protected] wrote:
> > > > From: Gireesh Hiremath <[email protected]>
> > > >
> > > > Switch from gpio API to gpiod API
> > >
> > > Nice change.
> > >
> > > Did you checked the users of this driver? This change changes the
> > > behaviour for actice_low GPIOs. A quick check showed that at least on
> > > upstream board: arch/arm/mach-pxa/palmtc.c uses active low GPIOs.
> > >
> > > > Signed-off-by: Gireesh Hiremath <[email protected]>
> > > >
> > > > Gbp-Pq: Topic apertis/guardian
> > > > Gbp-Pq: Name driver-input-matric-keypad-switch-gpio-to-gpiod.patch
> > >
> > > Please drop this internal tags.
> > >
> > > > ---
> > > > drivers/input/keyboard/matrix_keypad.c | 84 ++++++++++----------------
> > > > include/linux/input/matrix_keypad.h | 4 +-
> > > > 2 files changed, 33 insertions(+), 55 deletions(-)
> > > >
> > > > diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> > > > index 30924b57058f..b99574edad9c 100644
> > > > --- a/drivers/input/keyboard/matrix_keypad.c
> > > > +++ b/drivers/input/keyboard/matrix_keypad.c
> > > > @@ -15,11 +15,10 @@
> > > > #include <linux/interrupt.h>
> > > > #include <linux/jiffies.h>
> > > > #include <linux/module.h>
> > > > -#include <linux/gpio.h>
> > > > +#include <linux/gpio/consumer.h>
> > > > #include <linux/input/matrix_keypad.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/of.h>
> > > > -#include <linux/of_gpio.h>
> > > > #include <linux/of_platform.h>
> > > >
> > > > struct matrix_keypad {
> > > > @@ -49,11 +48,11 @@ static void __activate_col(const struct matrix_keypad_platform_data *pdata,
> > > > bool level_on = !pdata->active_low;
> > > >
> > > > if (on) {
> > > > - gpio_direction_output(pdata->col_gpios[col], level_on);
> > > > + gpiod_direction_output(pdata->col_gpios[col], level_on);
> > >
> > > Now strange things can happen, if active_low is set and you specified
> > > GPIO_ACTIVE_LOW within the device-tree. We need a way to move to gpiod
> > > and keep the current behaviour.
> > >
> > > > } else {
> > > > - gpio_set_value_cansleep(pdata->col_gpios[col], !level_on);
> > > > + gpiod_set_value_cansleep(pdata->col_gpios[col], !level_on);
> > > > if (!pdata->drive_inactive_cols)
> > > > - gpio_direction_input(pdata->col_gpios[col]);
> > > > + gpiod_direction_input(pdata->col_gpios[col]);
>
> ...
>
> > > > @@ -429,9 +410,6 @@ matrix_keypad_parse_dt(struct device *dev)
> > > > pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
> > > > of_property_read_bool(np, "linux,wakeup"); /* legacy */
> > > >
> > > > - if (of_get_property(np, "gpio-activelow", NULL))
> > > > - pdata->active_low = true;
> > > > -
> > >
> > > This removes backward compatibility, please don't do that.
> >
> > I do not think there is a reasonable way of keeping the compatibility
> > while using gpiod API (sans abandoning polarity handling and using
> > *_raw() versions of API).
> >
> > I would adjust the DTSes in the kernel and move on, especially given
> > that the DTSes in the kernel are inconsistent - they specify
> > gpio-activelow attribute while specifying "action high" in gpio
> > properties).
>
> Yes, because the driver didn't respect that by not using the gpiod API.
> Got your point for the DTses but what about the boards based on the
> platform-data? Those boards must be changed as well.

Yes, that is correct.

>
> Also I thought DTB is firmware and we should never break it, now we
> doing it by this commit. Just to get your opinion on this.

Well, this is true in theory, however from the practical POV the boards
that use this driver do not store DTB in firmware, but rather distribute
it with the kernel. So while we normally try to keep compatibility, in
cases like this I feel we can be practical and instead of trying to
handle a pure theoretical case simply fix up DTSes and move on with our
lives.

Thanks.

--
Dmitry

2022-08-22 07:44:26

by Dan Carpenter

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] driver: input: matric-keypad: switch to gpiod

2022-08-22 08:04:15

by Dan Carpenter

[permalink] [raw]

Subject: Re: [PATCH v3 2/3] driver: input: matric-keypad: add reduced matrix support

Hi,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Gireesh-Hiremath-in-bosch-com/driver-input-matric-keypad-switch-to-gpiod/20220819-151155
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: s390-randconfig-m041-20220819 (https://download.01.org/0day-ci/archive/20220820/[email protected]/config)
compiler: s390-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
drivers/input/keyboard/matrix_keypad.c:932 matrix_keypad_probe() error: uninitialized symbol 'err'.

vim +/err +932 drivers/input/keyboard/matrix_keypad.c

5298cc4cc753bb Bill Pemberton 2012-11-23 823 static int matrix_keypad_probe(struct platform_device *pdev)
bab7614d6d1b1f Eric Miao 2009-06-29 824 {
bab7614d6d1b1f Eric Miao 2009-06-29 825 const struct matrix_keypad_platform_data *pdata;
bab7614d6d1b1f Eric Miao 2009-06-29 826 struct matrix_keypad *keypad;
bab7614d6d1b1f Eric Miao 2009-06-29 827 struct input_dev *input_dev;
bab7614d6d1b1f Eric Miao 2009-06-29 828 int err;
bab7614d6d1b1f Eric Miao 2009-06-29 829
4a83eecff65bd3 AnilKumar Ch 2012-11-20 830 pdata = dev_get_platdata(&pdev->dev);
bab7614d6d1b1f Eric Miao 2009-06-29 831 if (!pdata) {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 832 pdata = matrix_keypad_parse_dt(&pdev->dev);
d55bda1b3e7c5a Christian Hoff 2018-11-12 833 if (IS_ERR(pdata))
4a83eecff65bd3 AnilKumar Ch 2012-11-20 834 return PTR_ERR(pdata);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 835 } else if (!pdata->keymap_data) {
bab7614d6d1b1f Eric Miao 2009-06-29 836 dev_err(&pdev->dev, "no keymap data defined\n");
bab7614d6d1b1f Eric Miao 2009-06-29 837 return -EINVAL;
bab7614d6d1b1f Eric Miao 2009-06-29 838 }
bab7614d6d1b1f Eric Miao 2009-06-29 839
4a83eecff65bd3 AnilKumar Ch 2012-11-20 840 keypad = kzalloc(sizeof(struct matrix_keypad), GFP_KERNEL);
bab7614d6d1b1f Eric Miao 2009-06-29 841 input_dev = input_allocate_device();
01111fcd42b050 Dmitry Torokhov 2012-04-20 842 if (!keypad || !input_dev) {
bab7614d6d1b1f Eric Miao 2009-06-29 843 err = -ENOMEM;
bab7614d6d1b1f Eric Miao 2009-06-29 844 goto err_free_mem;
bab7614d6d1b1f Eric Miao 2009-06-29 845 }
bab7614d6d1b1f Eric Miao 2009-06-29 846
bab7614d6d1b1f Eric Miao 2009-06-29 847 keypad->input_dev = input_dev;
bab7614d6d1b1f Eric Miao 2009-06-29 848 keypad->pdata = pdata;
4a83eecff65bd3 AnilKumar Ch 2012-11-20 849 keypad->row_shift = get_count_order(pdata->num_col_gpios);
bab7614d6d1b1f Eric Miao 2009-06-29 850 keypad->stopped = true;
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 851
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 852 if (pdata->mode == REDUCED) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 853 keypad->button_array = devm_kzalloc(
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 854 &pdev->dev,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 855 sizeof(struct button) * (pdata->num_of_buttons),
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 856 GFP_KERNEL);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 857 if (!keypad->button_array) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 858 dev_err(&pdev->dev,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 859 "could not allocate memory for button array\n");

err = -ENOMEM;

a0b420e08e3b87 Gireesh Hiremath 2022-08-19 860 goto err_free_mem;
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 861 ;

Why the extra ;?

a0b420e08e3b87 Gireesh Hiremath 2022-08-19 862 }
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 863
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 864 poll_prepare(keypad);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 865
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 866 err = input_setup_polling(input_dev, matrix_keypad_poll);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 867 if (err) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 868 dev_err(&pdev->dev,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 869 "unable to set up polling, err=%d\n", err);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 870 return err;

Memory leaks. Needs to goto err_free_mem;

a0b420e08e3b87 Gireesh Hiremath 2022-08-19 871 }
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 872
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 873 input_set_poll_interval(input_dev, pdata->poll_interval_ms);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 874 } else {
bab7614d6d1b1f Eric Miao 2009-06-29 875 INIT_DELAYED_WORK(&keypad->work, matrix_keypad_scan);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 876 }
bab7614d6d1b1f Eric Miao 2009-06-29 877 spin_lock_init(&keypad->lock);
bab7614d6d1b1f Eric Miao 2009-06-29 878
bab7614d6d1b1f Eric Miao 2009-06-29 879 input_dev->name = pdev->name;
bab7614d6d1b1f Eric Miao 2009-06-29 880 input_dev->id.bustype = BUS_HOST;
bab7614d6d1b1f Eric Miao 2009-06-29 881 input_dev->dev.parent = &pdev->dev;
bab7614d6d1b1f Eric Miao 2009-06-29 882 input_dev->open = matrix_keypad_start;
bab7614d6d1b1f Eric Miao 2009-06-29 883 input_dev->close = matrix_keypad_stop;
bab7614d6d1b1f Eric Miao 2009-06-29 884
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 885 if (pdata->mode == REDUCED) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 886 err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 887 pdata->num_line_gpios,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 888 pdata->num_line_gpios, NULL,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 889 input_dev);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 890 if (err) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 891 dev_err(&pdev->dev, "failed to build keymap for reduced mode\n");
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 892 goto err_free_mem;
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 893 }
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 894 } else {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 895 err = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
1932811f426fee Dmitry Torokhov 2012-05-10 896 pdata->num_row_gpios,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 897 pdata->num_col_gpios, NULL,
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 898 input_dev);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 899 if (err) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 900 dev_err(&pdev->dev, "failed to build keymap for generic mode\n");
1932811f426fee Dmitry Torokhov 2012-05-10 901 goto err_free_mem;
4a83eecff65bd3 AnilKumar Ch 2012-11-20 902 }
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 903 }
bab7614d6d1b1f Eric Miao 2009-06-29 904
1932811f426fee Dmitry Torokhov 2012-05-10 905 if (!pdata->no_autorepeat)
1932811f426fee Dmitry Torokhov 2012-05-10 906 __set_bit(EV_REP, input_dev->evbit);
bab7614d6d1b1f Eric Miao 2009-06-29 907 input_set_capability(input_dev, EV_MSC, MSC_SCAN);
bab7614d6d1b1f Eric Miao 2009-06-29 908 input_set_drvdata(input_dev, keypad);
bab7614d6d1b1f Eric Miao 2009-06-29 909
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 910 if (pdata->mode == REDUCED) {
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 911 button_hdl_init(keypad);
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 912 } else {
b83643ebf22423 Dmitry Torokhov 2012-04-20 913 err = matrix_keypad_init_gpio(pdev, keypad);
bab7614d6d1b1f Eric Miao 2009-06-29 914 if (err)
bab7614d6d1b1f Eric Miao 2009-06-29 915 goto err_free_mem;
a0b420e08e3b87 Gireesh Hiremath 2022-08-19 916 }
bab7614d6d1b1f Eric Miao 2009-06-29 917
bab7614d6d1b1f Eric Miao 2009-06-29 918 err = input_register_device(keypad->input_dev);
bab7614d6d1b1f Eric Miao 2009-06-29 919 if (err)
b83643ebf22423 Dmitry Torokhov 2012-04-20 920 goto err_free_gpio;
bab7614d6d1b1f Eric Miao 2009-06-29 921
bab7614d6d1b1f Eric Miao 2009-06-29 922 device_init_wakeup(&pdev->dev, pdata->wakeup);
bab7614d6d1b1f Eric Miao 2009-06-29 923 platform_set_drvdata(pdev, keypad);
bab7614d6d1b1f Eric Miao 2009-06-29 924
bab7614d6d1b1f Eric Miao 2009-06-29 925 return 0;
bab7614d6d1b1f Eric Miao 2009-06-29 926
b83643ebf22423 Dmitry Torokhov 2012-04-20 927 err_free_gpio:
b83643ebf22423 Dmitry Torokhov 2012-04-20 928 matrix_keypad_free_gpio(keypad);
bab7614d6d1b1f Eric Miao 2009-06-29 929 err_free_mem:
bab7614d6d1b1f Eric Miao 2009-06-29 930 input_free_device(input_dev);
bab7614d6d1b1f Eric Miao 2009-06-29 931 kfree(keypad);
bab7614d6d1b1f Eric Miao 2009-06-29 @932 return err;
bab7614d6d1b1f Eric Miao 2009-06-29 933 }

--
0-DAY CI Kernel Test Service
https://01.org/lkp