From patchwork Tue Jan 16 11:00:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 124654 Delivered-To: patch@linaro.org Received: by 10.46.64.148 with SMTP id r20csp972484lje; Tue, 16 Jan 2018 03:02:42 -0800 (PST) X-Google-Smtp-Source: ACJfBovQt5OPWS5wbX+C2ruSzwZzA/iaTFiV3u05QfKOJpL5sEA1kzMIzghdILRJlcy6SUivZqf7 X-Received: by 10.101.77.144 with SMTP id p16mr5546552pgq.106.1516100562714; Tue, 16 Jan 2018 03:02:42 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1516100562; cv=none; d=google.com; s=arc-20160816; b=EGHu3zKqwL8qE0eksEthWo3lCC+s4AGrdL6NurFyU5ppagIjCPiSWmz81dSsltPOCr AL3MtbirUf1xk2KqVTF5JZxU8wpwVEp6PswqgMq0/FxcsddmMFYlI6c9yqHmz7JM0EKN Hrvi+q4AQvqFY5h2SOxv9IRooRvYjxTNlbxJN2PGXbNCZ1oOcNG7umNX1XkwA6bJKkZM YFADVyAIIZYgXjL3gONE9EvhndJcb4Y+yKmbKLLLc8N0vJZVw3KEhEVK6ug3ff6RjYlc awSAbwUJ/VsXBvQMYC+k09xgwmyHNQ7Ib2M9jJO9f1tbFuMB3cydEkdJ3v8dXE3D2h70 bZBw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=wEQs/PQWBxK3YBOi+ehJ9mvUi6KnclS3JoxNaNIvnGk=; b=ZjnksQ+rpdBtjnTOMmauixYB24qA9ssltNPpmn81Yg26u77O8Sina5eHz+ca9KzR0j bDYA963FlM/tmwaDVlbT3H/w29R9XqYd5bASJ4HpjRm2Crb3cujEwpu4ISzp41KnEsH3 rGOssFAGiOb5sEyTOzPda00R5SJHlcj8XOIkDfB1TB+MWjku23oYezPEIPx/nXFtrtAu Wy8KYBsUKFJkSxqyz6B5Pg9qx/YnY3opyjxy4QD+rARdUVssU7ekbMOHYWfqckJ8kLkf SB7GJuygtA9794xSSeFb7iYOuxJIxaBRmS9mgcHkcES3te+PjaUFa9jO1NYd3AzGXn+x HNTA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=hgMg4OMU; spf=pass (google.com: best guess record for domain of linux-gpio-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-gpio-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id y12si1664800plt.1.2018.01.16.03.02.42; Tue, 16 Jan 2018 03:02:42 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-gpio-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=hgMg4OMU; spf=pass (google.com: best guess record for domain of linux-gpio-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-gpio-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751988AbeAPLCl (ORCPT + 6 others); Tue, 16 Jan 2018 06:02:41 -0500 Received: from mail-lf0-f66.google.com ([209.85.215.66]:44850 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751969AbeAPLCk (ORCPT ); Tue, 16 Jan 2018 06:02:40 -0500 Received: by mail-lf0-f66.google.com with SMTP id w23so16861136lfd.11 for ; Tue, 16 Jan 2018 03:02:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=iGijv630+1QfBpie+4qxHTRU1Vo56ESxido2hH4HlLo=; b=hgMg4OMUVadsGE09DSotpvXH5IBcAB6xTZm9JW7Ncv5XocOhQcIJ5e/xKPMaCwnCud vODMvHoN7AxghxhHc4dx8zgsVyNzn83N0ZpKJ0kkufWexTcIlp8xGA9e8RamLtlOL11c zgGYKE9bZ7UUMl2hSWa1wUjLBQjn/l5MjvCyc= 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; bh=iGijv630+1QfBpie+4qxHTRU1Vo56ESxido2hH4HlLo=; b=aGBt3c3DaOZ+nTuqruAHtESIDgrwHKXjV0GfGwntObfBdWWHUipU23sjdLwTaDodXj Qkt+U8KJ46us3CEEZxf+FDgUUtxFQ/KOnqbD4Dq3Qm/tear2iAqA6Mv9YixrXA9Xe9Ac yXlyWH79XYxPFVQIumjf6qgcz6na+gYwVddHJpiEUGVHR85/HfXxbKBP5nOHWKKRpiyb kGRG6vF4dYK6JV/Cga1BqWzXd/LJF5ZwswFpccHx/hHymYSztKKd1OLALfqLagOG8r1i OP8nzKh8Mv3HzoNn6/aqVlMiz/sLuvsRZJF7A/AbgPzJQ9MkN1fQOmmyleIr+kITObfP nJLw== X-Gm-Message-State: AKGB3mK41o3ZapQufvYfh4oK5cN0Fpb4BSzWxNagujICWESNXseQ6kvs zuZ588GMhq4nQzKUnPfXiUCs9jW67rs= X-Received: by 10.46.9.70 with SMTP id 67mr21255848ljj.144.1516100558293; Tue, 16 Jan 2018 03:02:38 -0800 (PST) Received: from localhost.localdomain (c-cb7471d5.014-348-6c756e10.cust.bredbandsbolaget.se. [213.113.116.203]) by smtp.gmail.com with ESMTPSA id r20sm270313lje.93.2018.01.16.03.02.35 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Jan 2018 03:02:37 -0800 (PST) From: Linus Walleij To: linux-gpio@vger.kernel.org, Clemens Gruber , Lukas Wunner Cc: Linus Walleij , Bartosz Golaszewski Subject: [PATCH v2] gpio: mmio: Also read bits that are zero Date: Tue, 16 Jan 2018 12:00:31 +0100 Message-Id: <20180116110031.20533-1-linus.walleij@linaro.org> X-Mailer: git-send-email 2.14.3 Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org The code for .get_multiple() has bugs: 1. The simple .get_multiple() just reads a register, masks it and sets the return value. This is not correct: we only want to assign values (whether 0 or 1) to the bits that are set in the mask. Fix this by using &= ~mask to clear all bits in the mask and then |= val & mask to set the corresponding bits from the read. 2. The bgpio_get_multiple_be() call has a similar problem: it uses the |= operator to set the bits, so only the bits in the mask are affected, but it misses to clear all returned bits from the mask initially, so some bits will be returned erroneously set to 1. 3. The bgpio_get_set_multiple() again fails to clear the bits from the mask. 4. find_next_bit() wasn't handled correctly, use a totally different approach for one function and change the other function to follow the design pattern of assigning the first bit to -1, then use bit + 1 in the for loop and < num_iterations as break condition. Fixes: 80057cb417b2 ("gpio-mmio: Use the new .get_multiple() callback") Cc: Bartosz Golaszewski Reported-by: Clemens Gruber Reported-by: Lukas Wunner Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Fix the bug initializing the bit counter to -1 rather than 0 by rewriting to exploit cached direction bits for one function and following the kernel design pattern for the other. Clemens: it would be great if you could test this, I want to push the fix ASAP if it solves the problem. --- drivers/gpio/gpio-mmio.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Tested-by: Clemens Gruber diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index f9042bcc27a4..7b14d6280e44 100644 --- a/drivers/gpio/gpio-mmio.c +++ b/drivers/gpio/gpio-mmio.c @@ -152,14 +152,13 @@ static int bgpio_get_set_multiple(struct gpio_chip *gc, unsigned long *mask, { unsigned long get_mask = 0; unsigned long set_mask = 0; - int bit = 0; - while ((bit = find_next_bit(mask, gc->ngpio, bit)) != gc->ngpio) { - if (gc->bgpio_dir & BIT(bit)) - set_mask |= BIT(bit); - else - get_mask |= BIT(bit); - } + /* Make sure we first clear any bits that are zero when we read the register */ + *bits &= ~*mask; + + /* Exploit the fact that we know which directions are set */ + set_mask = *mask & gc->bgpio_dir; + get_mask = *mask & ~gc->bgpio_dir; if (set_mask) *bits |= gc->read_reg(gc->reg_set) & set_mask; @@ -176,13 +175,13 @@ static int bgpio_get(struct gpio_chip *gc, unsigned int gpio) /* * This only works if the bits in the GPIO register are in native endianness. - * It is dirt simple and fast in this case. (Also the most common case.) */ static int bgpio_get_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits) { - - *bits = gc->read_reg(gc->reg_dat) & *mask; + /* Make sure we first clear any bits that are zero when we read the register */ + *bits &= ~*mask; + *bits |= gc->read_reg(gc->reg_dat) & *mask; return 0; } @@ -196,9 +195,12 @@ static int bgpio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask, unsigned long val; int bit; + /* Make sure we first clear any bits that are zero when we read the register */ + *bits &= ~*mask; + /* Create a mirrored mask */ - bit = 0; - while ((bit = find_next_bit(mask, gc->ngpio, bit)) != gc->ngpio) + bit = -1; + while ((bit = find_next_bit(mask, gc->ngpio, bit + 1)) < gc->ngpio) readmask |= bgpio_line2mask(gc, bit); /* Read the register */ @@ -208,8 +210,8 @@ static int bgpio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask, * Mirror the result into the "bits" result, this will give line 0 * in bit 0 ... line 31 in bit 31 for a 32bit register. */ - bit = 0; - while ((bit = find_next_bit(&val, gc->ngpio, bit)) != gc->ngpio) + bit = -1; + while ((bit = find_next_bit(&val, gc->ngpio, bit + 1)) < gc->ngpio) *bits |= bgpio_line2mask(gc, bit); return 0;