From 2b0378cf9f3163bac29fa9946b3aa1607fc03802 Mon Sep 17 00:00:00 2001 From: Christian Lamparter Date: Thu, 5 Aug 2021 15:29:25 +0200 Subject: [PATCH] gpio-button-hotplug: convert to gpio descriptor (gpiod_) API OpenWrt's special gpio-button-hotplug driver is still using exclusively the legacy GPIO Subsystem gpio_ API. While it still does work fine for most devices, upstream linux is starting to convert platform support like that of the APU2/3/4 to the new GPIOD LOOKUP tables that are not supported by it. Hence, this patch replaces the gpio_ calls present in gpio-button-hotplug with gpiod_ equivalent wherever it's possible. This allows the driver to use the gpiod lookup tables and still have a fallback for legacy platform data code that just sets button->gpio set to the real button/switch GPIO. As a bonus: the active_low logic is now being handled by the linux's gpio subsystem too. Another issue that was address is the of_handle leak in the dt parser error path. Tested with legacy platform data: x86_64: APU2, MX-100 Tested on OF: ATH79; MR18, APM821xx: Netgear WNDR4700, RAMIPS: WL-330N3G LANTIQ: AVM FritzBox 7360v1 Reported-by: Chris Blake Tested-by: Chris Blake Reviewed-by: Linus Walleij Signed-off-by: Christian Lamparter --- .../src/gpio-button-hotplug.c | 142 ++++++++---------- 1 file changed, 63 insertions(+), 79 deletions(-) diff --git a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c index 9575c6245b7..fcaf7f59de5 100644 --- a/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c +++ b/package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c @@ -242,11 +242,11 @@ static int gpio_button_get_value(struct gpio_keys_button_data *bdata) int val; if (bdata->can_sleep) - val = !!gpio_get_value_cansleep(bdata->b->gpio); + val = !!gpiod_get_value_cansleep(bdata->gpiod); else - val = !!gpio_get_value(bdata->b->gpio); + val = !!gpiod_get_value(bdata->gpiod); - return val ^ bdata->b->active_low; + return val; } static void gpio_keys_handle_button(struct gpio_keys_button_data *bdata) @@ -365,7 +365,6 @@ gpio_keys_get_devtree_pdata(struct device *dev) struct device_node *node, *pp; struct gpio_keys_platform_data *pdata; struct gpio_keys_button *button; - int error; int nbuttons; int i = 0; @@ -375,14 +374,12 @@ gpio_keys_get_devtree_pdata(struct device *dev) nbuttons = of_get_child_count(node); if (nbuttons == 0) - return NULL; + return ERR_PTR(-EINVAL); pdata = devm_kzalloc(dev, sizeof(*pdata) + nbuttons * (sizeof *button), GFP_KERNEL); - if (!pdata) { - error = -ENOMEM; - goto err_out; - } + if (!pdata) + return ERR_PTR(-ENOMEM); pdata->buttons = (struct gpio_keys_button *)(pdata + 1); pdata->nbuttons = nbuttons; @@ -391,37 +388,13 @@ gpio_keys_get_devtree_pdata(struct device *dev) of_property_read_u32(node, "poll-interval", &pdata->poll_interval); for_each_child_of_node(node, pp) { - enum of_gpio_flags flags; - - if (!of_find_property(pp, "gpios", NULL)) { - pdata->nbuttons--; - dev_warn(dev, "Found button without gpios\n"); - continue; - } - button = (struct gpio_keys_button *)(&pdata->buttons[i++]); - button->irq = irq_of_parse_and_map(pp, 0); - - button->gpio = of_get_gpio_flags(pp, 0, &flags); - if (button->gpio < 0) { - error = button->gpio; - if (error != -ENOENT) { - if (error != -EPROBE_DEFER) - dev_err(dev, - "Failed to get gpio flags, error: %d\n", - error); - return ERR_PTR(error); - } - } else { - button->active_low = !!(flags & OF_GPIO_ACTIVE_LOW); - } - if (of_property_read_u32(pp, "linux,code", &button->code)) { - dev_err(dev, "Button without keycode: 0x%x\n", - button->gpio); - error = -EINVAL; - goto err_out; + dev_err(dev, "Button node '%s' without keycode\n", + pp->full_name); + of_node_put(pp); + return ERR_PTR(-EINVAL); } button->desc = of_get_property(pp, "label", NULL); @@ -434,17 +407,12 @@ gpio_keys_get_devtree_pdata(struct device *dev) if (of_property_read_u32(pp, "debounce-interval", &button->debounce_interval)) button->debounce_interval = 5; - } - if (pdata->nbuttons == 0) { - error = -EINVAL; - goto err_out; + button->irq = irq_of_parse_and_map(pp, 0); + button->gpio = -ENOENT; /* mark this as device-tree */ } return pdata; - -err_out: - return ERR_PTR(error); } static struct of_device_id gpio_keys_of_match[] = { @@ -471,11 +439,12 @@ gpio_keys_get_devtree_pdata(struct device *dev) static int gpio_keys_button_probe(struct platform_device *pdev, struct gpio_keys_button_dev **_bdev, int polled) { - struct gpio_keys_platform_data *pdata = pdev->dev.platform_data; struct device *dev = &pdev->dev; + struct gpio_keys_platform_data *pdata = dev_get_platdata(dev); struct gpio_keys_button_dev *bdev; struct gpio_keys_button *buttons; - int error; + struct device_node *prev = NULL; + int error = 0; int i; if (!pdata) { @@ -514,46 +483,67 @@ static int gpio_keys_button_probe(struct platform_device *pdev, for (i = 0; i < pdata->nbuttons; i++) { struct gpio_keys_button *button = &buttons[i]; struct gpio_keys_button_data *bdata = &bdev->data[i]; - unsigned int gpio = button->gpio; + const char *desc = button->desc ? button->desc : DRV_NAME; if (button->wakeup) { dev_err(dev, "does not support wakeup\n"); - return -EINVAL; + error = -EINVAL; + goto out; } bdata->map_entry = button_get_index(button->code); if (bdata->map_entry < 0) { - dev_warn(dev, "does not support key code:%u\n", + dev_err(dev, "does not support key code:%u\n", button->code); - continue; + error = -EINVAL; + goto out; } if (!(button->type == 0 || button->type == EV_KEY || button->type == EV_SW)) { - dev_warn(dev, "only supports buttons or switches\n"); - continue; + dev_err(dev, "only supports buttons or switches\n"); + error = -EINVAL; + goto out; } - error = devm_gpio_request(dev, gpio, - button->desc ? button->desc : DRV_NAME); - if (error) { - dev_err(dev, "unable to claim gpio %u, err=%d\n", - gpio, error); - return error; + if (gpio_is_valid(button->gpio)) { + /* legacy platform data... but is it the lookup table? */ + bdata->gpiod = devm_gpiod_get_index(dev, desc, i, + GPIOD_IN); + if (IS_ERR(bdata->gpiod)) { + /* or the legacy (button->gpio is good) way? */ + error = devm_gpio_request_one(dev, + button->gpio, GPIOF_IN | ( + button->active_low ? GPIOF_ACTIVE_LOW : + 0), desc); + if (error) { + if (error != -EPROBE_DEFER) { + dev_err(dev, "unable to claim gpio %d, err=%d\n", + button->gpio, error); + } + goto out; + } + + bdata->gpiod = gpio_to_desc(button->gpio); + } + } else { + /* Device-tree */ + struct device_node *child = + of_get_next_child(dev->of_node, prev); + + bdata->gpiod = devm_gpiod_get_from_of_node(dev, + child, "gpios", 0, GPIOD_IN, desc); + + prev = child; } - bdata->gpiod = gpio_to_desc(gpio); - if (!bdata->gpiod) - return -EINVAL; - error = gpio_direction_input(gpio); - if (error) { - dev_err(dev, - "unable to set direction on gpio %u, err=%d\n", - gpio, error); - return error; + if (IS_ERR_OR_NULL(bdata->gpiod)) { + error = IS_ERR(bdata->gpiod) ? PTR_ERR(bdata->gpiod) : + -EINVAL; + goto out; } - bdata->can_sleep = gpio_cansleep(gpio); + bdata->can_sleep = gpiod_cansleep(bdata->gpiod); bdata->last_state = -1; /* Unknown state on boot */ if (bdev->polled) { @@ -584,8 +574,11 @@ static int gpio_keys_button_probe(struct platform_device *pdev, platform_set_drvdata(pdev, bdev); *_bdev = bdev; + error = 0; - return 0; +out: + of_node_put(prev); + return error; } static int gpio_keys_probe(struct platform_device *pdev) @@ -594,9 +587,7 @@ static int gpio_keys_probe(struct platform_device *pdev) struct gpio_keys_button_dev *bdev; int ret, i; - ret = gpio_keys_button_probe(pdev, &bdev, 0); - if (ret) return ret; @@ -608,12 +599,8 @@ static int gpio_keys_probe(struct platform_device *pdev) INIT_DELAYED_WORK(&bdata->work, gpio_keys_irq_work_func); - if (!bdata->gpiod) - continue; - if (!button->irq) { - bdata->irq = gpio_to_irq(button->gpio); - + bdata->irq = gpiod_to_irq(bdata->gpiod); if (bdata->irq < 0) { dev_err(&pdev->dev, "failed to get irq for gpio:%d\n", button->gpio); @@ -631,7 +618,6 @@ static int gpio_keys_probe(struct platform_device *pdev) ret = devm_request_threaded_irq(&pdev->dev, bdata->irq, NULL, button_handle_irq, irqflags, dev_name(&pdev->dev), bdata); - if (ret < 0) { bdata->irq = 0; dev_err(&pdev->dev, "failed to request irq:%d for gpio:%d\n", @@ -653,14 +639,12 @@ static int gpio_keys_polled_probe(struct platform_device *pdev) int ret; ret = gpio_keys_button_probe(pdev, &bdev, 1); - if (ret) return ret; INIT_DELAYED_WORK(&bdev->work, gpio_keys_polled_poll); pdata = bdev->pdata; - if (pdata->enable) pdata->enable(bdev->dev); -- 2.30.2