Re: [PATCH v2 07/10] ARM: OMAP1: ams-delta FIQ: Keep serio input GPIOs requested

From: Dmitry Torokhov
Date: Thu Jun 21 2018 - 20:10:12 EST


On Fri, Jun 22, 2018 at 12:41:25AM +0200, Janusz Krzysztofik wrote:
> From the very beginning, input GPIO pins of ams-delta serio port have
> been used by FIQ handler, not serio driver.
>
> Don't request those pins from the ams-delta-serio driver any longer,
> instead keep them requested and initialized by the FIQ initialization
> routine which already requests them and releases while identifying GPIO
> IRQs.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx>

Input bits look good.

Acked-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts
>
> arch/arm/mach-omap1/ams-delta-fiq.c | 42 ++++++++++++++++++++++++++++++-----
> drivers/input/serio/ams_delta_serio.c | 30 ++-----------------------
> 2 files changed, 39 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
> index 1d54a6177f14..5a6c59ac9b5f 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.c
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.c
> @@ -45,6 +45,11 @@ static struct irq_chip *irq_chip;
> static struct irq_data *irq_data[16];
> static unsigned int irq_counter[16];
>
> +static const char *pin_name[16] __initconst = {
> + [AMS_DELTA_GPIO_PIN_KEYBRD_DATA] = "keybrd_data",
> + [AMS_DELTA_GPIO_PIN_KEYBRD_CLK] = "keybrd_clk",
> +};
> +
> static irqreturn_t deferred_fiq(int irq, void *dev_id)
> {
> struct irq_data *d;
> @@ -80,7 +85,7 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
>
> void __init ams_delta_init_fiq(struct gpio_chip *chip)
> {
> - struct gpio_desc *gpiod;
> + struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
> void *fiqhandler_start;
> unsigned int fiqhandler_length;
> struct pt_regs FIQ_regs;
> @@ -96,7 +101,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> }
>
> for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
> - gpiod = gpiochip_request_own_desc(chip, i, NULL);
> + gpiod = gpiochip_request_own_desc(chip, i, pin_name[i]);
> if (IS_ERR(gpiod)) {
> pr_err("%s: failed to get GPIO pin %d (%ld)\n",
> __func__, i, PTR_ERR(gpiod));
> @@ -105,8 +110,27 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> /* Store irq_data location for IRQ handler use */
> irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
>
> - gpiochip_free_own_desc(gpiod);
> + /*
> + * FIQ handler takes full control over serio data and clk GPIO
> + * pins. Initiaize them and keep requested so nobody can
> + * interfere. Fail if any of those two couldn't be requested.
> + */
> + switch (i) {
> + case AMS_DELTA_GPIO_PIN_KEYBRD_DATA:
> + data = gpiod;
> + gpiod_direction_input(data);
> + break;
> + case AMS_DELTA_GPIO_PIN_KEYBRD_CLK:
> + clk = gpiod;
> + gpiod_direction_input(clk);
> + break;
> + default:
> + gpiochip_free_own_desc(gpiod);
> + break;
> + }
> }
> + if (!data || !clk)
> + goto out_gpio;
>
> fiqhandler_start = &qwerty_fiqin_start;
> fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
> @@ -117,7 +141,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> if (retval) {
> pr_err("ams_delta_init_fiq(): couldn't claim FIQ, ret=%d\n",
> retval);
> - return;
> + goto out_gpio;
> }
>
> retval = request_irq(INT_DEFERRED_FIQ, deferred_fiq,
> @@ -125,7 +149,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> if (retval < 0) {
> pr_err("Failed to get deferred_fiq IRQ, ret=%d\n", retval);
> release_fiq(&fh);
> - return;
> + goto out_gpio;
> }
> /*
> * Since no set_type() method is provided by OMAP irq chip,
> @@ -175,4 +199,12 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> offset = IRQ_ILR0_REG_OFFSET + (INT_GPIO_BANK1 - NR_IRQS_LEGACY) * 0x4;
> val = omap_readl(OMAP_IH1_BASE + offset) | 1;
> omap_writel(val, OMAP_IH1_BASE + offset);
> +
> + return;
> +
> +out_gpio:
> + if (data)
> + gpiochip_free_own_desc(data);
> + if (clk)
> + gpiochip_free_own_desc(clk);
> }
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index b955c6a72e99..7952a29f9540 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -110,19 +110,6 @@ static void ams_delta_serio_close(struct serio *serio)
> regulator_disable(priv->vcc);
> }
>
> -static const struct gpio ams_delta_gpios[] __initconst_or_module = {
> - {
> - .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
> - .flags = GPIOF_DIR_IN,
> - .label = "serio-data",
> - },
> - {
> - .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
> - .flags = GPIOF_DIR_IN,
> - .label = "serio-clock",
> - },
> -};
> -
> static int ams_delta_serio_init(struct platform_device *pdev)
> {
> struct ams_delta_serio *priv;
> @@ -133,13 +120,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> - err = gpio_request_array(ams_delta_gpios,
> - ARRAY_SIZE(ams_delta_gpios));
> - if (err) {
> - dev_err(&pdev->dev, "Couldn't request gpio pins\n");
> - goto serio;
> - }
> -
> priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> if (IS_ERR(priv->vcc)) {
> err = PTR_ERR(priv->vcc);
> @@ -157,7 +137,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> */
> if (err == -ENODEV)
> err = -EPROBE_DEFER;
> - goto gpio;
> + return err;
> }
>
> err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> @@ -165,7 +145,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> DRIVER_NAME, priv);
> if (err < 0) {
> dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
> - goto gpio;
> + return err;
> }
> /*
> * Since GPIO register handling for keyboard clock pin is performed
> @@ -201,10 +181,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>
> irq:
> free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
> -gpio:
> - gpio_free_array(ams_delta_gpios,
> - ARRAY_SIZE(ams_delta_gpios));
> -serio:
> return err;
> }
>
> @@ -214,8 +190,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
>
> serio_unregister_port(priv->serio);
> free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
> - gpio_free_array(ams_delta_gpios,
> - ARRAY_SIZE(ams_delta_gpios));
>
> return 0;
> }
> --
> 2.16.4
>

--
Dmitry