From patchwork Thu Aug 19 19:19:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Lamparter X-Patchwork-Id: 1518812 X-Patchwork-Delegate: chunkeey@googlemail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=rQKbkGWd; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=CQdypmCa; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4GrF5k08Bjz9sCD for ; Fri, 20 Aug 2021 05:22:33 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=9Qg/2CrkJdFt8+E8l74M/HY9SFE9/5i18jG/AtsmRH0=; b=rQKbkGWdPriCBp /ChiHZipZ9TK6u0f58iIsMAWNnXYyxqP8XF4p5WaoTbF60T9UXvNiIR8VgtAYREKa+F9C/rNFt6nT Xg0wL1apt1OD2hjUUj4Dvk+t+3D30vKzCvssH2ofOu0nr9Eon0IPxoIZahqS9cu+LRvU+7MjI9331 cpZghZu8Ksjy/APE/Di/n68xKa/pmdZxh3UupvzER3zZL15FCIZAllEBRUrV16AZkWbmAoK+gIA6a hLy9FohssiE2tb4i1vzpAiIZRF/gqttjRWrvaLbf872ap/9sS4tvMGRe4H9y/QYxfaagWgYfu/zAH BHZ9xv8hxLi91b2CjIrQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mGnZl-009KzR-Hd; Thu, 19 Aug 2021 19:19:41 +0000 Received: from mail-wm1-x32d.google.com ([2a00:1450:4864:20::32d]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mGnZh-009KyU-Id for openwrt-devel@lists.openwrt.org; Thu, 19 Aug 2021 19:19:39 +0000 Received: by mail-wm1-x32d.google.com with SMTP id w21-20020a7bc1150000b02902e69ba66ce6so4636324wmi.1 for ; Thu, 19 Aug 2021 12:19:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=plxDDawuHbD4sfdUP1vI7JVLN1+Z87IlrfkTwlpTZDc=; b=CQdypmCaDSXxj6HeQvh01Mg5flzehSXpIbMSG5IRdAcc1OVY1Jo3N4D3NFMBvLDWv0 5U0Bdwt0V/ULb6W1/MZhScIdESIjtgky78T5ud9va8QLVW53h4PaPUqjPxcd7xiLgniV yv+3PtCvLXuCpof4qfcNoP9eH7Bll9Trh+2eKFRVyGhci1lbJAltyT+vxojOAlDfIzeY 2gvCYX/0+K9uUZdUgJ7G72JdZB1Rqx5SYvS9N2NSOmjbKAfHpfw/BaMXH5BS1dsKIfaK 7uOeglb512tclfMUjEd7ADFJsLcsBy4JFobWnSNyab4mWenAfLsmPc3hq33llHt7cwPU kyfQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=plxDDawuHbD4sfdUP1vI7JVLN1+Z87IlrfkTwlpTZDc=; b=iD6GVHfk2NBe7wMy9BFtsMEz34cCZrnmxhVGDBZwJ0+gldVU0WzhMhLzlUiNgkmbq+ QUgKn2PaJHTesj/6gTnms3Ud2Lb4DXrG4Rl5yO1EnipfBtugn+WYepGrZ8o13OZh0m0s Mhd7ozWqDkdGb6f7iVoCg0jiOX/NC6Gm+ssXZWs36mM95uaoqroMnROLJNj7nIJMgGwI 1V+5m2ZjLWlgsVrFGVMlT4ZrextFPumVJGtXCcjcQtCFxp1bwRaJ7U1Tq5pZToDu9QHF k+nSwspMYy/52NftoZQg86JN0XEaeaYMRZTyTe20v1ZGSR0yEFWxRwaen2qh3mNuSFz+ otnQ== X-Gm-Message-State: AOAM5337pO4yuuXDEXzGraP0LZdC6nKPFvNyorina/dQnJy8QO4GgD6N +gzPCxwPVKxD2m9DjvxDQ36+U9Uzu84= X-Google-Smtp-Source: ABdhPJxrrc92b+gGiR5neI077D9OIci6xeX6dgsy6d4tMOPcZ7jNyGD0rawFctQ+x5qIOLD1GwLWpQ== X-Received: by 2002:a05:600c:2283:: with SMTP id 3mr200360wmf.149.1629400774958; Thu, 19 Aug 2021 12:19:34 -0700 (PDT) Received: from debian64.daheim (pd9e298aa.dip0.t-ipconnect.de. [217.226.152.170]) by smtp.gmail.com with ESMTPSA id e3sm3792790wrv.65.2021.08.19.12.19.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Aug 2021 12:19:34 -0700 (PDT) Received: from chuck by debian64.daheim with local (Exim 4.94.2) (envelope-from ) id 1mGnZd-0002fN-Ia; Thu, 19 Aug 2021 21:19:33 +0200 From: Christian Lamparter To: openwrt-devel@lists.openwrt.org Cc: Chris Blake , Linus Walleij Subject: [RFC PATCH v2] gpio-button-hotplug: convert to gpio descriptor (gpiod_) API Date: Thu, 19 Aug 2021 21:19:33 +0200 Message-Id: <20210819191933.10241-1-chunkeey@gmail.com> X-Mailer: git-send-email 2.33.0 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210819_121937_664237_E21B1440 X-CRM114-Status: GOOD ( 25.61 ) X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: 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. Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2a00:1450:4864:20:0:0:0:32d listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider [chunkeey[at]gmail.com] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: OpenWrt Development List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org 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 9575c6245b..fcaf7f59de 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);