From patchwork Mon Aug 6 21:50:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 143526 Delivered-To: patch@linaro.org Received: by 2002:a2e:9754:0:0:0:0:0 with SMTP id f20-v6csp3758543ljj; Mon, 6 Aug 2018 14:50:45 -0700 (PDT) X-Google-Smtp-Source: AAOMgpeIZ6/DzjYNlMSAJ4I0mJ/72CxDNLJ8vk75ij+46mQH/+yKVtcRV+GIwLaZJckPYmMrOZQN X-Received: by 2002:a65:52cc:: with SMTP id z12-v6mr16051249pgp.69.1533592245446; Mon, 06 Aug 2018 14:50:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1533592245; cv=none; d=google.com; s=arc-20160816; b=zzT+2+qAgiCBIXwF8ojAcuGF29EpzTrs8v/PQ8mq38rPi8uMABZgo0iWGfOCpknW/i wGcn5LJPFujP4B9tAb+84IeyNv9v/GdXW4EaXGujngp2dyC4F0z61ie/u1miLRCrftoR Twyd0jAe2TsFyAP+CqH3FJZQg8g37xKTdhgrgtRnxd6Pv7IO7domcQrsKB4SUQkFNIUs D8RIR9cYX+YWaetS2yIZ8off2iykd/Ch2L6oqGgIZ6hnerMltUFcgHdNngWr2CaCJ/Z0 ifOvb+Ae0xiMsbN2EhfzSoTCwnCa/qEZIPzWf6Itd5TNh8O7kfYckgYeQ1cwUbdWqFy3 ko7Q== 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=Y4OdLlzJPmUCGtE890MDKTok1e+CCQCwDPXQeEhIpQs=; b=Ozyb23sswgaue+nlMz2+E7mTb0WbmOD/ofucd7KMP4dzoFCe80TpucCrqW7ruDMKZU SBWduRK5lDMJv4PJTwif2ebQ35OkD1rqppguO4ExiJC7RewVGIkjFkVilyBW4AGRBgLH UBvrqZIDXJ3DYIRqWDITKkN60u2M37g7EUMgIc9pWy8Yrs5KYLgEDLFaWD9+WPawyR2R +iZrAFQ13/7FktULuGm33JWd3OBGNu1wBjoo1Nj9e4NAjbGiB/nIlG0qASfEqJYc5rAt X9gSv+3K8kiTcKX4g/Qnz7mddvuuUKnf4OqwuClCgRAjywyttKIddJtGuW3vaBgL+4iq tJww== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=Csie8kRU; 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 p15-v6si13401308pgh.281.2018.08.06.14.50.44; Mon, 06 Aug 2018 14:50:45 -0700 (PDT) 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=Csie8kRU; 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 S1733051AbeHGABr (ORCPT + 5 others); Mon, 6 Aug 2018 20:01:47 -0400 Received: from mail-wr1-f65.google.com ([209.85.221.65]:39680 "EHLO mail-wr1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732698AbeHGABr (ORCPT ); Mon, 6 Aug 2018 20:01:47 -0400 Received: by mail-wr1-f65.google.com with SMTP id h10-v6so13735001wre.6 for ; Mon, 06 Aug 2018 14:50:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=LAnr5YT7Klgn1E1OwM8lZ/dtx56iuzWG42dDjrmNPqs=; b=Csie8kRU66IExcpcyQ+AYM2h4T8rIOjL6+UNmzmgJY8PRu7y0WLx1v09Vkl+4M5TqR yNeYp/fDyDNW0Q1akzphDfxPJr5xNx7s2TE1t3SvUSafCwjbXIXzD3PEO6aRDJt0KLvN eASA4mVfJGSw1m+w2nsfTyWrdbLL8BkOoJ2W4= 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=LAnr5YT7Klgn1E1OwM8lZ/dtx56iuzWG42dDjrmNPqs=; b=LX81J+MLs7/iwIAzIPOvMa3c2IkJcEsBZv3SGivvfM+rFbL9AxIVTiOY+q2WnnsO6Y g03g21Q5PTF7jszVh4pTY44tooEhhqdImsiFdRel1zJUOAnF2bFual/7dLyf5Ykui54e hsn2HkloLIR1iQFcmYhMmtWSqoKpnjHZ8uC8FCsKlo9jgYzHNX4WnAKACqerR/DeziVF aVc53XXWeOqHJMUWjf+swUPrNYptGbNYEjSBncfeO7o4OCS8/UCcFZxjytjCl7fRQLag WKmqpI66+gA3Uc9fDJqFLVVDz2Rrj2g6yrRWzR5+ea/wcJXVDEiuXBpCUaBQopffgffC 4L0w== X-Gm-Message-State: AOUpUlG5Ijak+jWnyQT5r8k09CGM66R6gPlQiNybETTkUPk1HeKgZywk sGMFS9uPvfN3byFkxRvOcXsPyqpbuB8= X-Received: by 2002:adf:c684:: with SMTP id j4-v6mr11131956wrg.243.1533592240953; Mon, 06 Aug 2018 14:50:40 -0700 (PDT) Received: from localhost.localdomain (catv-89-135-96-219.catv.broadband.hu. [89.135.96.219]) by smtp.gmail.com with ESMTPSA id j131-v6sm8450268wmb.35.2018.08.06.14.50.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 06 Aug 2018 14:50:39 -0700 (PDT) From: Linus Walleij To: linux-gpio@vger.kernel.org, Tomer Maimon Cc: Linus Walleij Subject: [PATCH v2] gpio: mmio: Fix up inverted direction registers Date: Mon, 6 Aug 2018 23:50:36 +0200 Message-Id: <20180806215036.30864-1-linus.walleij@linaro.org> X-Mailer: git-send-email 2.17.0 Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org The bgpio_init() takes one of two arguments to specify a register to set the direction of the GPIO line: either dirout that indicates that a 1 in the bit in that register sets the corresponding line to output, or dirin which indicates that a 1 in the bit in that register sets the corresponding line to input. Conversely setting the bit to 0 on these will turn the line into input and output respectively. One of these can be defined but not both. This means that a platform that sets a bit to 1 for output only defines dirout and a platform that sets a bit to 0 for output only defines dirin. In short this defines the polarity of the direction register. Both can also be left as NULL meaning the GPIO chip is either input only or output only. Tomer Maimon discovered that for get/set chips (those where the get and set registers are defined but no separate clear register, and specifying BGPIOF_READ_OUTPUT_REG_SET so that we say we want to read the output value from the SET register) we are unconditionally reading the value from the SET register when the direction bit is 1 and from the DAT register when the direction bit is 0, not taking the direction bit polarity into account. It would be expected that when the direction bit is inverted (dirin is defined but not dirout) we read the current value from the DAT register when the bit is 1 and from the SET register when the bit is 0. Currently only some versions of ATH79, brcmstb, some versions of CLP711x, GE, IOP and Loongson use the dirin mode (a 1 in the register means input). They are unaffected because BGPIOF_READ_OUTPUT_REG_SET is not set on any of them. (They do not read back the SET register to figure out the output value.) So this is no regression with current drivers. However the behaviour is wrong and does not work with Tomer's new driver where he needs to use the BGIOF_READ_OUTPUT_REG_SET. This fixes the above issue by: - Instead of defining separate functions for the inverted case, set up a flag in the gpio_chip that indicates that the direction is inverted. - Remove the special inverted functions for setting input/output and getting the direction, rely on the flag instead. - Respect this flag in bgpio_get_set() and bgpio_get_set_multiple() Reported-by: Tomer Maimon Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Fix up a bug in getset as suggested by Tomer. Kind of. --- drivers/gpio/gpio-mmio.c | 108 +++++++++++++++++++++--------------- include/linux/gpio/driver.h | 3 + 2 files changed, 66 insertions(+), 45 deletions(-) -- 2.17.0 -- 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 diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index 7b14d6280e44..935292a30c99 100644 --- a/drivers/gpio/gpio-mmio.c +++ b/drivers/gpio/gpio-mmio.c @@ -136,8 +136,20 @@ static unsigned long bgpio_line2mask(struct gpio_chip *gc, unsigned int line) static int bgpio_get_set(struct gpio_chip *gc, unsigned int gpio) { unsigned long pinmask = bgpio_line2mask(gc, gpio); + bool dir = !!(gc->bgpio_dir & pinmask); - if (gc->bgpio_dir & pinmask) + /* + * If the direction is OUT we read the value from the SET + * register, and if the direction is IN we read the value + * from the DAT register. + * + * If the direction bits are inverted, naturally this gets + * inverted too. + */ + if (gc->bgpio_dir_inverted) + dir = !dir; + + if (dir) return !!(gc->read_reg(gc->reg_set) & pinmask); else return !!(gc->read_reg(gc->reg_dat) & pinmask); @@ -157,8 +169,13 @@ static int bgpio_get_set_multiple(struct gpio_chip *gc, unsigned long *mask, *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 (gc->bgpio_dir_inverted) { + set_mask = *mask & ~gc->bgpio_dir; + get_mask = *mask & gc->bgpio_dir; + } else { + set_mask = *mask & gc->bgpio_dir; + get_mask = *mask & ~gc->bgpio_dir; + } if (set_mask) *bits |= gc->read_reg(gc->reg_set) & set_mask; @@ -359,7 +376,10 @@ static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio) spin_lock_irqsave(&gc->bgpio_lock, flags); - gc->bgpio_dir &= ~bgpio_line2mask(gc, gpio); + if (gc->bgpio_dir_inverted) + gc->bgpio_dir |= bgpio_line2mask(gc, gpio); + else + gc->bgpio_dir &= ~bgpio_line2mask(gc, gpio); gc->write_reg(gc->reg_dir, gc->bgpio_dir); spin_unlock_irqrestore(&gc->bgpio_lock, flags); @@ -370,7 +390,10 @@ static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio) static int bgpio_get_dir(struct gpio_chip *gc, unsigned int gpio) { /* Return 0 if output, 1 of input */ - return !(gc->read_reg(gc->reg_dir) & bgpio_line2mask(gc, gpio)); + if (gc->bgpio_dir_inverted) + return !!(gc->read_reg(gc->reg_dir) & bgpio_line2mask(gc, gpio)); + else + return !(gc->read_reg(gc->reg_dir) & bgpio_line2mask(gc, gpio)); } static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) @@ -381,37 +404,10 @@ static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) spin_lock_irqsave(&gc->bgpio_lock, flags); - gc->bgpio_dir |= bgpio_line2mask(gc, gpio); - gc->write_reg(gc->reg_dir, gc->bgpio_dir); - - spin_unlock_irqrestore(&gc->bgpio_lock, flags); - - return 0; -} - -static int bgpio_dir_in_inv(struct gpio_chip *gc, unsigned int gpio) -{ - unsigned long flags; - - spin_lock_irqsave(&gc->bgpio_lock, flags); - - gc->bgpio_dir |= bgpio_line2mask(gc, gpio); - gc->write_reg(gc->reg_dir, gc->bgpio_dir); - - spin_unlock_irqrestore(&gc->bgpio_lock, flags); - - return 0; -} - -static int bgpio_dir_out_inv(struct gpio_chip *gc, unsigned int gpio, int val) -{ - unsigned long flags; - - gc->set(gc, gpio, val); - - spin_lock_irqsave(&gc->bgpio_lock, flags); - - gc->bgpio_dir &= ~bgpio_line2mask(gc, gpio); + if (gc->bgpio_dir_inverted) + gc->bgpio_dir &= ~bgpio_line2mask(gc, gpio); + else + gc->bgpio_dir |= bgpio_line2mask(gc, gpio); gc->write_reg(gc->reg_dir, gc->bgpio_dir); spin_unlock_irqrestore(&gc->bgpio_lock, flags); @@ -419,12 +415,6 @@ static int bgpio_dir_out_inv(struct gpio_chip *gc, unsigned int gpio, int val) return 0; } -static int bgpio_get_dir_inv(struct gpio_chip *gc, unsigned int gpio) -{ - /* Return 0 if output, 1 if input */ - return !!(gc->read_reg(gc->reg_dir) & bgpio_line2mask(gc, gpio)); -} - static int bgpio_setup_accessors(struct device *dev, struct gpio_chip *gc, bool byte_be) @@ -560,9 +550,10 @@ static int bgpio_setup_direction(struct gpio_chip *gc, gc->get_direction = bgpio_get_dir; } else if (dirin) { gc->reg_dir = dirin; - gc->direction_output = bgpio_dir_out_inv; - gc->direction_input = bgpio_dir_in_inv; - gc->get_direction = bgpio_get_dir_inv; + gc->direction_output = bgpio_dir_out; + gc->direction_input = bgpio_dir_in; + gc->get_direction = bgpio_get_dir; + gc->bgpio_dir_inverted = true; } else { if (flags & BGPIOF_NO_OUTPUT) gc->direction_output = bgpio_dir_out_err; @@ -582,6 +573,33 @@ static int bgpio_request(struct gpio_chip *chip, unsigned gpio_pin) return -EINVAL; } +/** + * bgpio_init() - Initialize generic GPIO accessor functions + * @gc: the GPIO chip to set up + * @dev: the parent device of the new GPIO chip (compulsory) + * @sz: the size (width) of the MMIO registers in bytes, typically 1, 2 or 4 + * @dat: MMIO address for the register to READ the value of the GPIO lines, it + * is expected that a 1 in the corresponding bit in this register means the + * line is asserted + * @set: MMIO address for the register to SET the value of the GPIO lines, it is + * expected that we write the line with 1 in this register to drive the GPIO line + * high. + * @clr: MMIO address for the register to CLEAR the value of the GPIO lines, it is + * expected that we write the line with 1 in this register to drive the GPIO line + * low. It is allowed to leave this address as NULL, in that case the SET register + * will be assumed to also clear the GPIO lines, by actively writing the line + * with 0. + * @dirout: MMIO address for the register to set the line as OUTPUT. It is assumed + * that setting a line to 1 in this register will turn that line into an + * output line. Conversely, setting the line to 0 will turn that line into + * an input. Either this or @dirin can be defined, but never both. + * @dirin: MMIO address for the register to set this line as INPUT. It is assumed + * that setting a line to 1 in this register will turn that line into an + * input line. Conversely, setting the line to 0 will turn that line into + * an output. Either this or @dirout can be defined, but never both. + * @flags: Different flags that will affect the behaviour of the device, such as + * endianness etc. + */ int bgpio_init(struct gpio_chip *gc, struct device *dev, unsigned long sz, void __iomem *dat, void __iomem *set, void __iomem *clr, void __iomem *dirout, void __iomem *dirin, diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 5382b5183b7e..0ea328e71ec9 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -201,6 +201,8 @@ static inline struct gpio_irq_chip *to_gpio_irq_chip(struct irq_chip *chip) * @reg_set: output set register (out=high) for generic GPIO * @reg_clr: output clear register (out=low) for generic GPIO * @reg_dir: direction setting register for generic GPIO + * @bgpio_dir_inverted: indicates that the direction register is inverted + * (gpiolib private state variable) * @bgpio_bits: number of register bits used for a generic GPIO i.e. * * 8 * @bgpio_lock: used to lock chip->bgpio_data. Also, this is needed to keep @@ -267,6 +269,7 @@ struct gpio_chip { void __iomem *reg_set; void __iomem *reg_clr; void __iomem *reg_dir; + bool bgpio_dir_inverted; int bgpio_bits; spinlock_t bgpio_lock; unsigned long bgpio_data;