gpio-button-hotplug: convert to gpio descriptor (gpiod_) API
authorChristian Lamparter <chunkeey@gmail.com>
Thu, 5 Aug 2021 13:29:25 +0000 (15:29 +0200)
committerChristian Lamparter <chunkeey@gmail.com>
Thu, 26 Aug 2021 19:00:26 +0000 (21:00 +0200)
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 <chrisrblake93@gmail.com>
Tested-by: Chris Blake <chrisrblake93@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
package/kernel/gpio-button-hotplug/src/gpio-button-hotplug.c

index 9575c6245b7e69201eb700261deb4c08b036acab..fcaf7f59de5c0e855ea8567867d8b520560a2ba7 100644 (file)
@@ -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);