Message ID | 20230525040324.3773741-10-hugo@hugovil.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp112665vqr; Wed, 24 May 2023 21:51:30 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6vFsLRISbrxjSDN3S3Qvkye5mxUhsmWYBP1AuWDP7NvtgPZ6M7H1en91F1p1m7VhDt0mSq X-Received: by 2002:a17:90a:bc97:b0:255:e301:7b01 with SMTP id x23-20020a17090abc9700b00255e3017b01mr277194pjr.35.1684990290207; Wed, 24 May 2023 21:51:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684990290; cv=none; d=google.com; s=arc-20160816; b=O/mjVn/OEJvFJNm7cbJ3+XLU0RgGUKy/TBTIFUGg5M5LD1OZvsmaOQB8gCreBj5KIh LFHdwtmFRFYKDalSOzer7iKorX87xOlcbV80iB3Tj4cSrYDX4eDlqr+vStZRiCoWsB8B 1cPDgWBIXnp22uaazjTJfvApI3oOqDzAAQzjifWEsj33/wsApy5ao8/PYSGHKIFrtMi1 bnX4az9WwgTovkVEof8mblF40wmEMQy49LlwRvOg4h9e1jaengcmjbZnP1+Lgwv602Uh py0KpHUl2RVOHHIZyfwuonLq3Xe5XTa4hesetARGtwuQG4K/ZMIS7/lGbHJCNoDZ3weU mJcg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:subject:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:cc:to:from:dkim-signature; bh=9uFf80nuPq7IpOBCTtsrmsGYtNovsW/4uJuo42PakOo=; b=d4938meVO7oNA5OvgmVjhEfPVgp+6e7oW60cRwSyrILLxds8C1WsyR0CvmEhf+Iq3x 1KXeitw5tLKVR6COtP3PWxsk5y/HVdy3MBF6sZspYq1G0kS4Ll38pa6BIZLQRKQae7+e ydyGrz1AgWfGcFXsq5bRtK0KkXauH+Cs2WCNwBnVm5hoT6uRSLaqPT07WzSxBgj1qRda yF9r+TYv+x7m6l1MKzSB4+MvBg29l0oVp73IDJev3vNiT+exEgZC2p0OxgGZts5ewLCn GR/okenYI0IMTfVXyJfKfcJINHIZFmE240AfIbdYEqS2eiFZsYG2qYiiNY7mG9q5eJgV xwTA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@hugovil.com header.s=x header.b=hZS8RhBG; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id v1-20020a654601000000b00528d30e6990si252517pgq.509.2023.05.24.21.51.15; Wed, 24 May 2023 21:51:30 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@hugovil.com header.s=x header.b=hZS8RhBG; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238910AbjEYEGo (ORCPT <rfc822;ahmedalshaiji.dev@gmail.com> + 99 others); Thu, 25 May 2023 00:06:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39794 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231764AbjEYEEv (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 25 May 2023 00:04:51 -0400 Received: from mail.hugovil.com (mail.hugovil.com [162.243.120.170]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5AD57E62; Wed, 24 May 2023 21:04:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=hugovil.com ; s=x; h=Subject:Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=9uFf80nuPq7IpOBCTtsrmsGYtNovsW/4uJuo42PakOo=; b=hZS8RhBGHMEWLID3br+gSAdS39 WNwEED5LXP+qFQrO0SMH4y60c3mBJasKvbCKoLvBfl7dcE1drHXktQSModQe0c9gXeMrMzRV8IVgR XTy5IQ0QncyR5VRTIM4p2KUUwtvNMtP35sRxeB6GinMfzzB5l+wz4CcO7mFLpQV2zIwY=; Received: from modemcable168.174-80-70.mc.videotron.ca ([70.80.174.168]:52970 helo=pettiford.lan) by mail.hugovil.com with esmtpa (Exim 4.92) (envelope-from <hugo@hugovil.com>) id 1q22DJ-0001dB-5g; Thu, 25 May 2023 00:04:34 -0400 From: Hugo Villeneuve <hugo@hugovil.com> To: gregkh@linuxfoundation.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, jirislaby@kernel.org, jringle@gridpoint.com, tomasz.mon@camlingroup.com, l.perczak@camlintechnologies.com Cc: linux-serial@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, hugo@hugovil.com, linux-gpio@vger.kernel.org, Hugo Villeneuve <hvilleneuve@dimonoff.com> Date: Thu, 25 May 2023 00:03:23 -0400 Message-Id: <20230525040324.3773741-10-hugo@hugovil.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230525040324.3773741-1-hugo@hugovil.com> References: <20230525040324.3773741-1-hugo@hugovil.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SA-Exim-Connect-IP: 70.80.174.168 X-SA-Exim-Mail-From: hugo@hugovil.com X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Subject: [PATCH v3 09/11] serial: sc16is7xx: add I/O register translation offset X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on mail.hugovil.com) Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1766840378625611528?= X-GMAIL-MSGID: =?utf-8?q?1766840378625611528?= |
Series |
serial: sc16is7xx: fix GPIO regression and rs485 improvements
|
|
Commit Message
Hugo Villeneuve
May 25, 2023, 4:03 a.m. UTC
From: Hugo Villeneuve <hvilleneuve@dimonoff.com> If the shared GPIO pins on a dual port/channel variant like the SC16IS752 are configured as GPIOs for port A, and modem control lines on port A, we need to translate the Linux GPIO offset to an offset that is compatible with the I/O registers of the SC16IS7XX (IOState, IODir and IOIntEna). Add a new variable to store that offset and set it when we detect that special case. Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> --- drivers/tty/serial/sc16is7xx.c | 54 +++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-)
Comments
Thu, May 25, 2023 at 12:03:23AM -0400, Hugo Villeneuve kirjoitti: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > If the shared GPIO pins on a dual port/channel variant like the > SC16IS752 are configured as GPIOs for port A, and modem control lines > on port A, we need to translate the Linux GPIO offset to an offset > that is compatible with the I/O registers of the SC16IS7XX (IOState, > IODir and IOIntEna). > > Add a new variable to store that offset and set it when we detect that > special case. ... > +/* > + * We may need to translate the Linux GPIO offset to a SC16IS7XX offset. > + * This is needed only for the case where a dual port variant is configured to > + * have only port B as modem status lines. > + * > + * Example for SC16IS752/762 with upper bank (port A) set as GPIOs, and > + * lower bank (port B) set as modem status lines (special case described above): > + * > + * Pin GPIO pin Linux GPIO SC16IS7XX > + * name function offset offset > + * -------------------------------------------------- > + * GPIO7/RIA GPIO7 3 7 > + * GPIO6/CDA GPIO6 2 6 > + * GPIO5/DTRA GPIO5 1 5 > + * GPIO4/DSRA GPIO4 0 4 > + * GPIO3/RIB RIB N/A N/A > + * GPIO2/CDB CDB N/A N/A > + * GPIO1/DTRB DTRB N/A N/A > + * GPIO0/DSRB DSRB N/A N/A > + * > + * Example for SC16IS750/760 with upper bank (7..4) set as modem status lines, Single space is enough. > + * and lower bank (3..0) as GPIOs: > + * > + * Pin GPIO pin Linux GPIO SC16IS7XX > + * name function offset offset > + * -------------------------------------------------- > + * GPIO7/RI RI N/A N/A > + * GPIO6/CD CD N/A N/A > + * GPIO5/DTR DTR N/A N/A > + * GPIO4/DSR DSR N/A N/A > + * GPIO3 GPIO3 3 3 > + * GPIO2 GPIO2 2 2 > + * GPIO1 GPIO1 1 1 > + * GPIO0 GPIO0 0 0 > + */ Wondering if you can always register 8 pins and use valid mask to define which one are in use?
On Thu, 25 May 2023, Hugo Villeneuve wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > If the shared GPIO pins on a dual port/channel variant like the > SC16IS752 are configured as GPIOs for port A, and modem control lines > on port A, we need to translate the Linux GPIO offset to an offset > that is compatible with the I/O registers of the SC16IS7XX (IOState, > IODir and IOIntEna). > > Add a new variable to store that offset and set it when we detect that > special case. > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- > drivers/tty/serial/sc16is7xx.c | 54 +++++++++++++++++++++++++++++++++- > 1 file changed, 53 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > index 97ec532a0a19..c2cfd057ed9a 100644 > --- a/drivers/tty/serial/sc16is7xx.c > +++ b/drivers/tty/serial/sc16is7xx.c > @@ -338,6 +338,7 @@ struct sc16is7xx_port { > #ifdef CONFIG_GPIOLIB > struct gpio_chip gpio; > int gpio_configured; > + int gpio_offset; > #endif > unsigned char buf[SC16IS7XX_FIFO_SIZE]; > struct kthread_worker kworker; > @@ -1298,12 +1299,50 @@ static const struct uart_ops sc16is7xx_ops = { > }; > > #ifdef CONFIG_GPIOLIB > + > +/* > + * We may need to translate the Linux GPIO offset to a SC16IS7XX offset. > + * This is needed only for the case where a dual port variant is configured to > + * have only port B as modem status lines. > + * > + * Example for SC16IS752/762 with upper bank (port A) set as GPIOs, and > + * lower bank (port B) set as modem status lines (special case described above): > + * > + * Pin GPIO pin Linux GPIO SC16IS7XX > + * name function offset offset > + * -------------------------------------------------- > + * GPIO7/RIA GPIO7 3 7 > + * GPIO6/CDA GPIO6 2 6 > + * GPIO5/DTRA GPIO5 1 5 > + * GPIO4/DSRA GPIO4 0 4 > + * GPIO3/RIB RIB N/A N/A > + * GPIO2/CDB CDB N/A N/A > + * GPIO1/DTRB DTRB N/A N/A > + * GPIO0/DSRB DSRB N/A N/A > + * > + * Example for SC16IS750/760 with upper bank (7..4) set as modem status lines, > + * and lower bank (3..0) as GPIOs: > + * > + * Pin GPIO pin Linux GPIO SC16IS7XX > + * name function offset offset > + * -------------------------------------------------- > + * GPIO7/RI RI N/A N/A > + * GPIO6/CD CD N/A N/A > + * GPIO5/DTR DTR N/A N/A > + * GPIO4/DSR DSR N/A N/A > + * GPIO3 GPIO3 3 3 > + * GPIO2 GPIO2 2 2 > + * GPIO1 GPIO1 1 1 > + * GPIO0 GPIO0 0 0 > + */ > + > static int sc16is7xx_gpio_get(struct gpio_chip *chip, unsigned offset) > { > unsigned int val; > struct sc16is7xx_port *s = gpiochip_get_data(chip); > struct uart_port *port = &s->p[0].port; > > + offset += s->gpio_offset; > val = sc16is7xx_port_read(port, SC16IS7XX_IOSTATE_REG); > > return !!(val & BIT(offset)); > @@ -1314,6 +1353,7 @@ static void sc16is7xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val) > struct sc16is7xx_port *s = gpiochip_get_data(chip); > struct uart_port *port = &s->p[0].port; > > + offset += s->gpio_offset; > sc16is7xx_port_update(port, SC16IS7XX_IOSTATE_REG, BIT(offset), > val ? BIT(offset) : 0); > } > @@ -1324,6 +1364,7 @@ static int sc16is7xx_gpio_direction_input(struct gpio_chip *chip, > struct sc16is7xx_port *s = gpiochip_get_data(chip); > struct uart_port *port = &s->p[0].port; > > + offset += s->gpio_offset; > sc16is7xx_port_update(port, SC16IS7XX_IODIR_REG, BIT(offset), 0); > > return 0; > @@ -1336,6 +1377,8 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip, > struct uart_port *port = &s->p[0].port; > u8 state = sc16is7xx_port_read(port, SC16IS7XX_IOSTATE_REG); > > + offset += s->gpio_offset; > + > if (val) > state |= BIT(offset); > else > @@ -1395,6 +1438,7 @@ static int sc16is7xx_probe(struct device *dev, > > #ifdef CONFIG_GPIOLIB > s->gpio_configured = devtype->nr_gpio; > + s->gpio_offset = 0; > #endif /* CONFIG_GPIOLIB */ > > /* Always ask for fixed clock rate from a property. */ > @@ -1529,16 +1573,24 @@ static int sc16is7xx_probe(struct device *dev, > #endif /* CONFIG_GPIOLIB */ > } > > - if (val) > + if (val) { > +#ifdef CONFIG_GPIOLIB > + /* Additional I/O regs offset. */ > + if (val == SC16IS7XX_IOCONTROL_MODEM_B_BIT) > + s->gpio_offset = SC16IS7XX_GPIOS_PER_BANK; > +#endif /* CONFIG_GPIOLIB */ > + > regmap_update_bits( > s->regmap, > SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT, > SC16IS7XX_IOCONTROL_MODEM_A_BIT | > SC16IS7XX_IOCONTROL_MODEM_B_BIT, val); > + } > } > > #ifdef CONFIG_GPIOLIB > dev_dbg(dev, "GPIOs to configure: %d\n", s->gpio_configured); > + dev_dbg(dev, "GPIOs offset: %d\n", s->gpio_offset); > > if (s->gpio_configured) { > /* Setup GPIO controller */ > Is the order of this and 8/11 patch correct or should this precede that other patch? That is, is 8/11 alone useful enough or would this also be wanted? (I'm aware that 8/11 has a Fixes tag).
On Thu, 25 May 2023 15:10:00 +0300 (EEST) Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > On Thu, 25 May 2023, Hugo Villeneuve wrote: > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > If the shared GPIO pins on a dual port/channel variant like the > > SC16IS752 are configured as GPIOs for port A, and modem control lines > > on port A, we need to translate the Linux GPIO offset to an offset > > that is compatible with the I/O registers of the SC16IS7XX (IOState, > > IODir and IOIntEna). > > > > Add a new variable to store that offset and set it when we detect that > > special case. > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > drivers/tty/serial/sc16is7xx.c | 54 +++++++++++++++++++++++++++++++++- > > 1 file changed, 53 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > > index 97ec532a0a19..c2cfd057ed9a 100644 > > --- a/drivers/tty/serial/sc16is7xx.c > > +++ b/drivers/tty/serial/sc16is7xx.c > > @@ -338,6 +338,7 @@ struct sc16is7xx_port { > > #ifdef CONFIG_GPIOLIB > > struct gpio_chip gpio; > > int gpio_configured; > > + int gpio_offset; > > #endif > > unsigned char buf[SC16IS7XX_FIFO_SIZE]; > > struct kthread_worker kworker; > > @@ -1298,12 +1299,50 @@ static const struct uart_ops sc16is7xx_ops = { > > }; > > > > #ifdef CONFIG_GPIOLIB > > + > > +/* > > + * We may need to translate the Linux GPIO offset to a SC16IS7XX offset. > > + * This is needed only for the case where a dual port variant is configured to > > + * have only port B as modem status lines. > > + * > > + * Example for SC16IS752/762 with upper bank (port A) set as GPIOs, and > > + * lower bank (port B) set as modem status lines (special case described above): > > + * > > + * Pin GPIO pin Linux GPIO SC16IS7XX > > + * name function offset offset > > + * -------------------------------------------------- > > + * GPIO7/RIA GPIO7 3 7 > > + * GPIO6/CDA GPIO6 2 6 > > + * GPIO5/DTRA GPIO5 1 5 > > + * GPIO4/DSRA GPIO4 0 4 > > + * GPIO3/RIB RIB N/A N/A > > + * GPIO2/CDB CDB N/A N/A > > + * GPIO1/DTRB DTRB N/A N/A > > + * GPIO0/DSRB DSRB N/A N/A > > + * > > + * Example for SC16IS750/760 with upper bank (7..4) set as modem status lines, > > + * and lower bank (3..0) as GPIOs: > > + * > > + * Pin GPIO pin Linux GPIO SC16IS7XX > > + * name function offset offset > > + * -------------------------------------------------- > > + * GPIO7/RI RI N/A N/A > > + * GPIO6/CD CD N/A N/A > > + * GPIO5/DTR DTR N/A N/A > > + * GPIO4/DSR DSR N/A N/A > > + * GPIO3 GPIO3 3 3 > > + * GPIO2 GPIO2 2 2 > > + * GPIO1 GPIO1 1 1 > > + * GPIO0 GPIO0 0 0 > > + */ > > + > > static int sc16is7xx_gpio_get(struct gpio_chip *chip, unsigned offset) > > { > > unsigned int val; > > struct sc16is7xx_port *s = gpiochip_get_data(chip); > > struct uart_port *port = &s->p[0].port; > > > > + offset += s->gpio_offset; > > val = sc16is7xx_port_read(port, SC16IS7XX_IOSTATE_REG); > > > > return !!(val & BIT(offset)); > > @@ -1314,6 +1353,7 @@ static void sc16is7xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val) > > struct sc16is7xx_port *s = gpiochip_get_data(chip); > > struct uart_port *port = &s->p[0].port; > > > > + offset += s->gpio_offset; > > sc16is7xx_port_update(port, SC16IS7XX_IOSTATE_REG, BIT(offset), > > val ? BIT(offset) : 0); > > } > > @@ -1324,6 +1364,7 @@ static int sc16is7xx_gpio_direction_input(struct gpio_chip *chip, > > struct sc16is7xx_port *s = gpiochip_get_data(chip); > > struct uart_port *port = &s->p[0].port; > > > > + offset += s->gpio_offset; > > sc16is7xx_port_update(port, SC16IS7XX_IODIR_REG, BIT(offset), 0); > > > > return 0; > > @@ -1336,6 +1377,8 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip, > > struct uart_port *port = &s->p[0].port; > > u8 state = sc16is7xx_port_read(port, SC16IS7XX_IOSTATE_REG); > > > > + offset += s->gpio_offset; > > + > > if (val) > > state |= BIT(offset); > > else > > @@ -1395,6 +1438,7 @@ static int sc16is7xx_probe(struct device *dev, > > > > #ifdef CONFIG_GPIOLIB > > s->gpio_configured = devtype->nr_gpio; > > + s->gpio_offset = 0; > > #endif /* CONFIG_GPIOLIB */ > > > > /* Always ask for fixed clock rate from a property. */ > > @@ -1529,16 +1573,24 @@ static int sc16is7xx_probe(struct device *dev, > > #endif /* CONFIG_GPIOLIB */ > > } > > > > - if (val) > > + if (val) { > > +#ifdef CONFIG_GPIOLIB > > + /* Additional I/O regs offset. */ > > + if (val == SC16IS7XX_IOCONTROL_MODEM_B_BIT) > > + s->gpio_offset = SC16IS7XX_GPIOS_PER_BANK; > > +#endif /* CONFIG_GPIOLIB */ > > + > > regmap_update_bits( > > s->regmap, > > SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT, > > SC16IS7XX_IOCONTROL_MODEM_A_BIT | > > SC16IS7XX_IOCONTROL_MODEM_B_BIT, val); > > + } > > } > > > > #ifdef CONFIG_GPIOLIB > > dev_dbg(dev, "GPIOs to configure: %d\n", s->gpio_configured); > > + dev_dbg(dev, "GPIOs offset: %d\n", s->gpio_offset); > > > > if (s->gpio_configured) { > > /* Setup GPIO controller */ > > > > Is the order of this and 8/11 patch correct or should this precede that > other patch? That is, is 8/11 alone useful enough or would this also be > wanted? (I'm aware that 8/11 has a Fixes tag). In fact, this patch (9/11) could be considered to be part of 8/11. I decided to separate it in order to facilitate the review process. Maybe I should merge them... Hugo.
On Thu, 25 May 2023 14:22:46 +0300 andy.shevchenko@gmail.com wrote: > Thu, May 25, 2023 at 12:03:23AM -0400, Hugo Villeneuve kirjoitti: > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > If the shared GPIO pins on a dual port/channel variant like the > > SC16IS752 are configured as GPIOs for port A, and modem control lines > > on port A, we need to translate the Linux GPIO offset to an offset > > that is compatible with the I/O registers of the SC16IS7XX (IOState, > > IODir and IOIntEna). > > > > Add a new variable to store that offset and set it when we detect that > > special case. > > ... > > > +/* > > + * We may need to translate the Linux GPIO offset to a SC16IS7XX offset. > > + * This is needed only for the case where a dual port variant is configured to > > + * have only port B as modem status lines. > > + * > > + * Example for SC16IS752/762 with upper bank (port A) set as GPIOs, and > > + * lower bank (port B) set as modem status lines (special case described above): > > + * > > + * Pin GPIO pin Linux GPIO SC16IS7XX > > + * name function offset offset > > + * -------------------------------------------------- > > + * GPIO7/RIA GPIO7 3 7 > > + * GPIO6/CDA GPIO6 2 6 > > + * GPIO5/DTRA GPIO5 1 5 > > + * GPIO4/DSRA GPIO4 0 4 > > + * GPIO3/RIB RIB N/A N/A > > + * GPIO2/CDB CDB N/A N/A > > + * GPIO1/DTRB DTRB N/A N/A > > + * GPIO0/DSRB DSRB N/A N/A > > + * > > + * Example for SC16IS750/760 with upper bank (7..4) set as modem status lines, > > Single space is enough. Fixed. > > + * and lower bank (3..0) as GPIOs: > > + * > > + * Pin GPIO pin Linux GPIO SC16IS7XX > > + * name function offset offset > > + * -------------------------------------------------- > > + * GPIO7/RI RI N/A N/A > > + * GPIO6/CD CD N/A N/A > > + * GPIO5/DTR DTR N/A N/A > > + * GPIO4/DSR DSR N/A N/A > > + * GPIO3 GPIO3 3 3 > > + * GPIO2 GPIO2 2 2 > > + * GPIO1 GPIO1 1 1 > > + * GPIO0 GPIO0 0 0 > > + */ > > Wondering if you can always register 8 pins and use valid mask to define which > one are in use? I will look into it, I think it may be a good idea and could help to simplify things a bit. Hugo.
On Thu, 25 May 2023 11:31:45 -0400 Hugo Villeneuve <hugo@hugovil.com> wrote: > On Thu, 25 May 2023 14:22:46 +0300 > andy.shevchenko@gmail.com wrote: > > > Thu, May 25, 2023 at 12:03:23AM -0400, Hugo Villeneuve kirjoitti: > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > > > If the shared GPIO pins on a dual port/channel variant like the > > > SC16IS752 are configured as GPIOs for port A, and modem control lines > > > on port A, we need to translate the Linux GPIO offset to an offset > > > that is compatible with the I/O registers of the SC16IS7XX (IOState, > > > IODir and IOIntEna). > > > > > > Add a new variable to store that offset and set it when we detect that > > > special case. > > > > ... > > > > > +/* > > > + * We may need to translate the Linux GPIO offset to a SC16IS7XX offset. > > > + * This is needed only for the case where a dual port variant is configured to > > > + * have only port B as modem status lines. > > > + * > > > + * Example for SC16IS752/762 with upper bank (port A) set as GPIOs, and > > > + * lower bank (port B) set as modem status lines (special case described above): > > > + * > > > + * Pin GPIO pin Linux GPIO SC16IS7XX > > > + * name function offset offset > > > + * -------------------------------------------------- > > > + * GPIO7/RIA GPIO7 3 7 > > > + * GPIO6/CDA GPIO6 2 6 > > > + * GPIO5/DTRA GPIO5 1 5 > > > + * GPIO4/DSRA GPIO4 0 4 > > > + * GPIO3/RIB RIB N/A N/A > > > + * GPIO2/CDB CDB N/A N/A > > > + * GPIO1/DTRB DTRB N/A N/A > > > + * GPIO0/DSRB DSRB N/A N/A > > > + * > > > + * Example for SC16IS750/760 with upper bank (7..4) set as modem status lines, > > > > Single space is enough. > > Fixed. > > > > > + * and lower bank (3..0) as GPIOs: > > > + * > > > + * Pin GPIO pin Linux GPIO SC16IS7XX > > > + * name function offset offset > > > + * -------------------------------------------------- > > > + * GPIO7/RI RI N/A N/A > > > + * GPIO6/CD CD N/A N/A > > > + * GPIO5/DTR DTR N/A N/A > > > + * GPIO4/DSR DSR N/A N/A > > > + * GPIO3 GPIO3 3 3 > > > + * GPIO2 GPIO2 2 2 > > > + * GPIO1 GPIO1 1 1 > > > + * GPIO0 GPIO0 0 0 > > > + */ > > > > Wondering if you can always register 8 pins and use valid mask to define which > > one are in use? > > I will look into it, I think it may be a good idea and could help to simplify things a bit. Hi, finally, this was the way to go. The resulting code/patch is much simpler and elegant this way. Thank you for the suggestion. I will submit a V4 with all the changes. Hugo.
Thu, May 25, 2023 at 01:20:46PM -0400, Hugo Villeneuve kirjoitti: > On Thu, 25 May 2023 11:31:45 -0400 > Hugo Villeneuve <hugo@hugovil.com> wrote: > > On Thu, 25 May 2023 14:22:46 +0300 > > andy.shevchenko@gmail.com wrote: > > > Thu, May 25, 2023 at 12:03:23AM -0400, Hugo Villeneuve kirjoitti: ... > > > Wondering if you can always register 8 pins and use valid mask to define which > > > one are in use? > > > > I will look into it, I think it may be a good idea and could help to > > simplify things a bit. > > finally, this was the way to go. The resulting code/patch is much simpler and > elegant this way. Thank you for the suggestion. > > I will submit a V4 with all the changes. Thank you for trying!
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 97ec532a0a19..c2cfd057ed9a 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -338,6 +338,7 @@ struct sc16is7xx_port { #ifdef CONFIG_GPIOLIB struct gpio_chip gpio; int gpio_configured; + int gpio_offset; #endif unsigned char buf[SC16IS7XX_FIFO_SIZE]; struct kthread_worker kworker; @@ -1298,12 +1299,50 @@ static const struct uart_ops sc16is7xx_ops = { }; #ifdef CONFIG_GPIOLIB + +/* + * We may need to translate the Linux GPIO offset to a SC16IS7XX offset. + * This is needed only for the case where a dual port variant is configured to + * have only port B as modem status lines. + * + * Example for SC16IS752/762 with upper bank (port A) set as GPIOs, and + * lower bank (port B) set as modem status lines (special case described above): + * + * Pin GPIO pin Linux GPIO SC16IS7XX + * name function offset offset + * -------------------------------------------------- + * GPIO7/RIA GPIO7 3 7 + * GPIO6/CDA GPIO6 2 6 + * GPIO5/DTRA GPIO5 1 5 + * GPIO4/DSRA GPIO4 0 4 + * GPIO3/RIB RIB N/A N/A + * GPIO2/CDB CDB N/A N/A + * GPIO1/DTRB DTRB N/A N/A + * GPIO0/DSRB DSRB N/A N/A + * + * Example for SC16IS750/760 with upper bank (7..4) set as modem status lines, + * and lower bank (3..0) as GPIOs: + * + * Pin GPIO pin Linux GPIO SC16IS7XX + * name function offset offset + * -------------------------------------------------- + * GPIO7/RI RI N/A N/A + * GPIO6/CD CD N/A N/A + * GPIO5/DTR DTR N/A N/A + * GPIO4/DSR DSR N/A N/A + * GPIO3 GPIO3 3 3 + * GPIO2 GPIO2 2 2 + * GPIO1 GPIO1 1 1 + * GPIO0 GPIO0 0 0 + */ + static int sc16is7xx_gpio_get(struct gpio_chip *chip, unsigned offset) { unsigned int val; struct sc16is7xx_port *s = gpiochip_get_data(chip); struct uart_port *port = &s->p[0].port; + offset += s->gpio_offset; val = sc16is7xx_port_read(port, SC16IS7XX_IOSTATE_REG); return !!(val & BIT(offset)); @@ -1314,6 +1353,7 @@ static void sc16is7xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val) struct sc16is7xx_port *s = gpiochip_get_data(chip); struct uart_port *port = &s->p[0].port; + offset += s->gpio_offset; sc16is7xx_port_update(port, SC16IS7XX_IOSTATE_REG, BIT(offset), val ? BIT(offset) : 0); } @@ -1324,6 +1364,7 @@ static int sc16is7xx_gpio_direction_input(struct gpio_chip *chip, struct sc16is7xx_port *s = gpiochip_get_data(chip); struct uart_port *port = &s->p[0].port; + offset += s->gpio_offset; sc16is7xx_port_update(port, SC16IS7XX_IODIR_REG, BIT(offset), 0); return 0; @@ -1336,6 +1377,8 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip, struct uart_port *port = &s->p[0].port; u8 state = sc16is7xx_port_read(port, SC16IS7XX_IOSTATE_REG); + offset += s->gpio_offset; + if (val) state |= BIT(offset); else @@ -1395,6 +1438,7 @@ static int sc16is7xx_probe(struct device *dev, #ifdef CONFIG_GPIOLIB s->gpio_configured = devtype->nr_gpio; + s->gpio_offset = 0; #endif /* CONFIG_GPIOLIB */ /* Always ask for fixed clock rate from a property. */ @@ -1529,16 +1573,24 @@ static int sc16is7xx_probe(struct device *dev, #endif /* CONFIG_GPIOLIB */ } - if (val) + if (val) { +#ifdef CONFIG_GPIOLIB + /* Additional I/O regs offset. */ + if (val == SC16IS7XX_IOCONTROL_MODEM_B_BIT) + s->gpio_offset = SC16IS7XX_GPIOS_PER_BANK; +#endif /* CONFIG_GPIOLIB */ + regmap_update_bits( s->regmap, SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT, SC16IS7XX_IOCONTROL_MODEM_A_BIT | SC16IS7XX_IOCONTROL_MODEM_B_BIT, val); + } } #ifdef CONFIG_GPIOLIB dev_dbg(dev, "GPIOs to configure: %d\n", s->gpio_configured); + dev_dbg(dev, "GPIOs offset: %d\n", s->gpio_offset); if (s->gpio_configured) { /* Setup GPIO controller */