| Message ID | 20220819065946.9572-1-Gireesh.Hiremath@in.bosch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] driver: input: matric-keypad: switch to gpiod | expand |
On 19/08/2022 09:59, Gireesh.Hiremath@in.bosch.com wrote: > From: Gireesh Hiremath <Gireesh.Hiremath@in.bosch.com> > > Switch from gpio API to gpiod API > > Signed-off-by: Gireesh Hiremath <Gireesh.Hiremath@in.bosch.com> > > 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
Hi Gireesh, On 22-08-19, Gireesh.Hiremath@in.bosch.com wrote: > From: Gireesh Hiremath <Gireesh.Hiremath@in.bosch.com> > > 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 <Gireesh.Hiremath@in.bosch.com> > > 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 > >
On Fri, Aug 19, 2022 at 03:12:31PM +0200, Marco Felsch wrote: > Hi Gireesh, > > On 22-08-19, Gireesh.Hiremath@in.bosch.com wrote: > > From: Gireesh Hiremath <Gireesh.Hiremath@in.bosch.com> > > > > 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 <Gireesh.Hiremath@in.bosch.com> > > > > 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.
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, Gireesh.Hiremath@in.bosch.com wrote: > > > From: Gireesh Hiremath <Gireesh.Hiremath@in.bosch.com> > > > > > > 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 <Gireesh.Hiremath@in.bosch.com> > > > > > > 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
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, Gireesh.Hiremath@in.bosch.com wrote: > > > > From: Gireesh Hiremath <Gireesh.Hiremath@in.bosch.com> > > > > > > > > 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 <Gireesh.Hiremath@in.bosch.com> > > > > > > > > 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.
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: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220819/202208192340.m1XMlTj5-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/input/keyboard/matrix_keypad.c:432 matrix_keypad_parse_dt() error: uninitialized symbol 'ret'.
Old smatch warnings:
drivers/input/keyboard/matrix_keypad.c:439 matrix_keypad_parse_dt() error: uninitialized symbol 'ret'.
vim +/ret +432 drivers/input/keyboard/matrix_keypad.c
5298cc4cc753bb Bill Pemberton 2012-11-23 380 static struct matrix_keypad_platform_data *
4a83eecff65bd3 AnilKumar Ch 2012-11-20 381 matrix_keypad_parse_dt(struct device *dev)
4a83eecff65bd3 AnilKumar Ch 2012-11-20 382 {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 383 struct matrix_keypad_platform_data *pdata;
4a83eecff65bd3 AnilKumar Ch 2012-11-20 384 struct device_node *np = dev->of_node;
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 385 struct gpio_desc **gpios;
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 386 struct gpio_desc *desc;
d55bda1b3e7c5a Christian Hoff 2018-11-12 387 int ret, i, nrow, ncol;
4a83eecff65bd3 AnilKumar Ch 2012-11-20 388
4a83eecff65bd3 AnilKumar Ch 2012-11-20 389 if (!np) {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 390 dev_err(dev, "device lacks DT data\n");
4a83eecff65bd3 AnilKumar Ch 2012-11-20 391 return ERR_PTR(-ENODEV);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 392 }
4a83eecff65bd3 AnilKumar Ch 2012-11-20 393
4a83eecff65bd3 AnilKumar Ch 2012-11-20 394 pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 395 if (!pdata) {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 396 dev_err(dev, "could not allocate memory for platform data\n");
4a83eecff65bd3 AnilKumar Ch 2012-11-20 397 return ERR_PTR(-ENOMEM);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 398 }
4a83eecff65bd3 AnilKumar Ch 2012-11-20 399
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 400 pdata->num_row_gpios = nrow = gpiod_count(dev, "row");
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 401 pdata->num_col_gpios = ncol = gpiod_count(dev, "col");
e80beb27d2f81a Grant Likely 2013-02-12 402 if (nrow <= 0 || ncol <= 0) {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 403 dev_err(dev, "number of keypad rows/columns not specified\n");
4a83eecff65bd3 AnilKumar Ch 2012-11-20 404 return ERR_PTR(-EINVAL);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 405 }
4a83eecff65bd3 AnilKumar Ch 2012-11-20 406
4a83eecff65bd3 AnilKumar Ch 2012-11-20 407 if (of_get_property(np, "linux,no-autorepeat", NULL))
4a83eecff65bd3 AnilKumar Ch 2012-11-20 408 pdata->no_autorepeat = true;
aeda5003d0b987 Dmitry Torokhov 2015-07-16 409
aeda5003d0b987 Dmitry Torokhov 2015-07-16 410 pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
aeda5003d0b987 Dmitry Torokhov 2015-07-16 411 of_property_read_bool(np, "linux,wakeup"); /* legacy */
aeda5003d0b987 Dmitry Torokhov 2015-07-16 412
aa0e26bb786b00 David Rivshin 2017-03-29 413 pdata->drive_inactive_cols =
aa0e26bb786b00 David Rivshin 2017-03-29 414 of_property_read_bool(np, "drive-inactive-cols");
aa0e26bb786b00 David Rivshin 2017-03-29 415
4a83eecff65bd3 AnilKumar Ch 2012-11-20 416 of_property_read_u32(np, "debounce-delay-ms", &pdata->debounce_ms);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 417 of_property_read_u32(np, "col-scan-delay-us",
4a83eecff65bd3 AnilKumar Ch 2012-11-20 418 &pdata->col_scan_delay_us);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 419
a86854d0c599b3 Kees Cook 2018-06-12 420 gpios = devm_kcalloc(dev,
a86854d0c599b3 Kees Cook 2018-06-12 421 pdata->num_row_gpios + pdata->num_col_gpios,
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 422 sizeof(*gpios),
4a83eecff65bd3 AnilKumar Ch 2012-11-20 423 GFP_KERNEL);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 424 if (!gpios) {
4a83eecff65bd3 AnilKumar Ch 2012-11-20 425 dev_err(dev, "could not allocate memory for gpios\n");
4a83eecff65bd3 AnilKumar Ch 2012-11-20 426 return ERR_PTR(-ENOMEM);
4a83eecff65bd3 AnilKumar Ch 2012-11-20 427 }
4a83eecff65bd3 AnilKumar Ch 2012-11-20 428
d55bda1b3e7c5a Christian Hoff 2018-11-12 429 for (i = 0; i < nrow; i++) {
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 430 desc = devm_gpiod_get_index(dev, "row", i, GPIOD_IN);
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 431 if (IS_ERR(desc))
d55bda1b3e7c5a Christian Hoff 2018-11-12 @432 return ERR_PTR(ret);
s/ret/desc/
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 433 gpios[i] = desc;
d55bda1b3e7c5a Christian Hoff 2018-11-12 434 }
4a83eecff65bd3 AnilKumar Ch 2012-11-20 435
d55bda1b3e7c5a Christian Hoff 2018-11-12 436 for (i = 0; i < ncol; i++) {
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 437 desc = devm_gpiod_get_index(dev, "col", i, GPIOD_IN);
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 438 if (IS_ERR(desc))
d55bda1b3e7c5a Christian Hoff 2018-11-12 439 return ERR_PTR(ret);
90bb4ee8dc3c27 Gireesh Hiremath 2022-08-19 440 gpios[nrow + i] = desc;
d55bda1b3e7c5a Christian Hoff 2018-11-12 441 }
4a83eecff65bd3 AnilKumar Ch 2012-11-20 442
4a83eecff65bd3 AnilKumar Ch 2012-11-20 443 pdata->row_gpios = gpios;
4a83eecff65bd3 AnilKumar Ch 2012-11-20 444 pdata->col_gpios = &gpios[pdata->num_row_gpios];
4a83eecff65bd3 AnilKumar Ch 2012-11-20 445
4a83eecff65bd3 AnilKumar Ch 2012-11-20 446 return pdata;
4a83eecff65bd3 AnilKumar Ch 2012-11-20 447 }
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;