Message ID | 20230515160206.2801991-1-hugo@hugovil.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp7083144vqo; Mon, 15 May 2023 10:33:54 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ71UJ1Lzk+BhfiHufBqp+lrvFkMQryYDcfIVb5jBtEJqhjRoOAcqUGpMGaV7Zwfwmwc2OYD X-Received: by 2002:a17:902:b907:b0:1ac:84dd:6d1f with SMTP id bf7-20020a170902b90700b001ac84dd6d1fmr25051631plb.1.1684172034508; Mon, 15 May 2023 10:33:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684172034; cv=none; d=google.com; s=arc-20160816; b=YrMCyCzMpvnUASGdmk0du2kFjegcTruqbcZsAOZSP+FW2w8ilpZR4QuEajnMpsGKya 1XmEmXZZYBEneNvpRFS67bdJWq0Q3RPoplv1NrJ91+6LQFXXg44kgwDX6vlLgxzs4b81 LcHiy/tsgoN+ssA9RiFiVD7tP3cKgjEER8CefLgpDkgIelaFuSqRIdzJCAICKNn9e0Av KQi2Fdf14DhjdPeBrv4bqbM9T+xI12A85iE9aTuLQQdrJBLcULrDXAJOC2gZQPkrrJLf ePqvPDtbrRg6nIKaG7i52s5HoFh29YyugAXA0DxTNUk7pdwH+SaI1Ch3Hys0AX7PSRBv YLQw== 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 :message-id:date:cc:to:from:dkim-signature; bh=XUIEpNRUXpJvYmFAKwBCCJfgCkzLAHM5z0DC/nHI13A=; b=0ERde7St3NXdt842AyYdEeSh24skL1lTy6QAZl3LSJ6Nl0wNnzWEov8chzP9D5nbpg DzSSHfGzvsflLRXtaSBbaFitBrkFyovqD9bvURBwAKRh9vKPmIZH1Ih8Qajabdu418NT 5IugqBMW599RHsTNKctn0cBJtZRwRzx/P5FZ4sD02WqaYetHfya9pGmU9VTa6PbxAVjc Eh24XIznOEmKYbwPi8sKQ7kLdaQ+vYZaC/on9yQwtEV/eGTo6rOtv6Lr2bz4VFV2puO7 sXagk4I/DBs8vFp0ti8UqZ19aibZGG5spoESKn6uvliebuARiWCncUM503gqoZafhr5K 2Z8g== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@hugovil.com header.s=x header.b=COQJj6sq; 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 nk9-20020a17090b194900b0025281755f55si84797pjb.43.2023.05.15.10.33.41; Mon, 15 May 2023 10:33:54 -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=COQJj6sq; 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 S242972AbjEOQqm (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Mon, 15 May 2023 12:46:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50788 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242990AbjEOQqg (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 15 May 2023 12:46:36 -0400 X-Greylist: delayed 2622 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Mon, 15 May 2023 09:46:30 PDT Received: from mail.hugovil.com (mail.hugovil.com [162.243.120.170]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4C8D25259; Mon, 15 May 2023 09:46:29 -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: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:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=XUIEpNRUXpJvYmFAKwBCCJfgCkzLAHM5z0DC/nHI13A=; b=C OQJj6sqPxX1Q7FxQhjaUUWeXuw3XXan7JNUCzhueLTvMhc5UVRePxj/9FrhWq8xoK/PYs8Qz6OmsS db1EVVcO8JeQ/mCL9KxlCEZB2ew42iYk5ZwNzStoIz1YTCn7Ah3Cchp3zITdcEILi/J9FYpyyEJXx +bbtXKn9wrC1d8zM=; Received: from modemcable168.174-80-70.mc.videotron.ca ([70.80.174.168]:41230 helo=pettiford.lan) by mail.hugovil.com with esmtpa (Exim 4.92) (envelope-from <hugo@hugovil.com>) id 1pyaeU-0003GD-Fw; Mon, 15 May 2023 12:02:23 -0400 From: Hugo Villeneuve <hugo@hugovil.com> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jirislaby@kernel.org>, Lech Perczak <l.perczak@camlintechnologies.com>, =?utf-8?q?Tomasz_Mo=C5=84?= <tomasz.mon@camlingroup.com> Cc: hugo@hugovil.com, Hugo Villeneuve <hvilleneuve@dimonoff.com>, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org Date: Mon, 15 May 2023 12:02:07 -0400 Message-Id: <20230515160206.2801991-1-hugo@hugovil.com> X-Mailer: git-send-email 2.30.2 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 autolearn=ham autolearn_force=no version=3.4.6 Subject: [RFC PATCH] Revert "sc16is7xx: Separate GPIOs from modem control lines" 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?1765978303051592660?= X-GMAIL-MSGID: =?utf-8?q?1765982374894368092?= |
Series |
[RFC] Revert "sc16is7xx: Separate GPIOs from modem control lines"
|
|
Commit Message
Hugo Villeneuve
May 15, 2023, 4:02 p.m. UTC
From: Hugo Villeneuve <hvilleneuve@dimonoff.com> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61. Because of this commit, it is no longer possible to use the 16 GPIO lines as dedicated GPIOs on the SC16IS752. Reverting it makes it work again. The log message of the original commit states: "Export only the GPIOs that are not shared with hardware modem control lines" But there is no explanation as to why this decision was taken to permanently set the function of the GPIO lines as modem control lines. AFAIK, there is no problem with using these lines as GPIO or modem control lines. Maybe after reverting this commit, we could define a new device-tree property named, for example, "use-modem-control-lines", so that both options can be supported. Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines") Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> --- drivers/tty/serial/sc16is7xx.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
Comments
Hi Hugo, Please see my answers inline. W dniu 15.05.2023 o 18:51, Hugo Villeneuve pisze: > Hi Greg, > > On Mon, 15 May 2023 18:20:02 +0200 > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > >> On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote: >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com> >>> >>> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61. >>> >>> Because of this commit, it is no longer possible to use the 16 GPIO >>> lines as dedicated GPIOs on the SC16IS752. >>> >>> Reverting it makes it work again. >>> >>> The log message of the original commit states: >>> "Export only the GPIOs that are not shared with hardware modem >>> control lines" >>> >>> But there is no explanation as to why this decision was taken to >>> permanently set the function of the GPIO lines as modem control >>> lines. AFAIK, there is no problem with using these lines as GPIO or modem >>> control lines. >>> >>> Maybe after reverting this commit, we could define a new >>> device-tree property named, for example, >>> "use-modem-control-lines", so that both options can be supported. >>> >>> Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control >>> lines") >> Please do not line-wrap these lines. > Ok. > >> Nor is a blank line needed here. > Ok. > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> >>> --- >>> drivers/tty/serial/sc16is7xx.c | 14 ++++---------- >>> 1 file changed, 4 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c >>> index 5bd98e4316f5..25f1b2f6ec51 100644 >>> --- a/drivers/tty/serial/sc16is7xx.c >>> +++ b/drivers/tty/serial/sc16is7xx.c >>> @@ -306,7 +306,6 @@ struct sc16is7xx_devtype { >>> char name[10]; >>> int nr_gpio; >>> int nr_uart; >>> - int has_mctrl; >>> }; >>> >>> #define SC16IS7XX_RECONF_MD (1 << 0) >>> @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = { >>> .name = "SC16IS74X", >>> .nr_gpio = 0, >>> .nr_uart = 1, >>> - .has_mctrl = 0, >>> }; >>> >>> static const struct sc16is7xx_devtype sc16is750_devtype = { >>> .name = "SC16IS750", >>> - .nr_gpio = 4, >>> + .nr_gpio = 8, >> I think this one line change is all you really need here, right? the >> otner changes do nothing in this patch, so you should just create a new >> one changing this value. Oh, and this one: >> >>> .nr_uart = 1, >>> - .has_mctrl = 1, >>> }; >>> >>> static const struct sc16is7xx_devtype sc16is752_devtype = { >>> .name = "SC16IS752", >>> - .nr_gpio = 0, >>> + .nr_gpio = 8, >> right? >> >> Don't mess with the has_mctrl stuff, that's not relevant here. > Sorry, I just noticed that simply reverting commit 679875d1d880 is not sufficient (and will not compile). We must also revert part of commit: > 21144bab4f11 ("sc16is7xx: Handle modem status lines"). > > The problem is that the commit 679875d1d880 was incomplete, and it was (unfortunately) completed by integrating it in commit 21144bab4f11 ("sc16is7xx: Handle modem status lines"). The relevant change was only these 5 new lines, burried deeply into the second commit: Just as you noticed, this was required to support full set of flow control lines on this device. The commit you're trying to revert was a preparation for it. Disabling has_mctrl will break it. I kindly suggest to suggest a fix, instead of hurrying a revert, and waiting for a proper fix later. > > @@ -1353,9 +1452,17 @@ static int sc16is7xx_probe(struct device *dev, > sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_EFCR_REG, > SC16IS7XX_EFCR_RXDISABLE_BIT | > SC16IS7XX_EFCR_TXDISABLE_BIT); > + > + /* Use GPIO lines as modem status registers */ > + if (devtype->has_mctrl) > + sc16is7xx_port_write(&s->p[i].port, > + SC16IS7XX_IOCONTROL_REG, > + SC16IS7XX_IOCONTROL_MODEM_BIT); > + > > Therefore, I should also remove these lines if we go forward with a revert of the patch (should I add another tag "Fixes..." in that case?). > > And what do you think of my proposal to maybe replace has_mctrl with a device tree property so that both modes can be fully supported? I think the proper solution here, is not to invent a new device tree property for every single use case. I would start by looking for other drivers, if, and how they handle similar cases. For example, imx-serial driver respects "uart-has-rtscts" property, as do a lot of other controllers built into SoC-s. On the other hand, other devices which can also provide GPIOs, respect "gpio-controller" property. According to SC16IS752 datasheet [1], respecting one of those should be enough, as GPIOs can be enabled in groups of four pins even for dual UART version. Every group matches a single port, so probably this can be probably selected per UART even on dual-port versions. I'll be more than happy to assist with that. > > Thank you, > Hugo. > [1] https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf
On Tue, 16 May 2023 10:50:11 +0200 Lech Perczak <lech.perczak@camlingroup.com> wrote: > Hi Hugo, > > Please see my answers inline. > > W dniu 15.05.2023 o 18:51, Hugo Villeneuve pisze: > > Hi Greg, > > > > On Mon, 15 May 2023 18:20:02 +0200 > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > > >> On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote: > >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > >>> > >>> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61. > >>> > >>> Because of this commit, it is no longer possible to use the 16 GPIO > >>> lines as dedicated GPIOs on the SC16IS752. > >>> > >>> Reverting it makes it work again. > >>> > >>> The log message of the original commit states: > >>> "Export only the GPIOs that are not shared with hardware modem > >>> control lines" > >>> > >>> But there is no explanation as to why this decision was taken to > >>> permanently set the function of the GPIO lines as modem control > >>> lines. AFAIK, there is no problem with using these lines as GPIO or modem > >>> control lines. > >>> > >>> Maybe after reverting this commit, we could define a new > >>> device-tree property named, for example, > >>> "use-modem-control-lines", so that both options can be supported. > >>> > >>> Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control > >>> lines") > >> Please do not line-wrap these lines. > > Ok. > > > >> Nor is a blank line needed here. > > Ok. > > > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > >>> --- > >>> drivers/tty/serial/sc16is7xx.c | 14 ++++---------- > >>> 1 file changed, 4 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > >>> index 5bd98e4316f5..25f1b2f6ec51 100644 > >>> --- a/drivers/tty/serial/sc16is7xx.c > >>> +++ b/drivers/tty/serial/sc16is7xx.c > >>> @@ -306,7 +306,6 @@ struct sc16is7xx_devtype { > >>> char name[10]; > >>> int nr_gpio; > >>> int nr_uart; > >>> - int has_mctrl; > >>> }; > >>> > >>> #define SC16IS7XX_RECONF_MD (1 << 0) > >>> @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = { > >>> .name = "SC16IS74X", > >>> .nr_gpio = 0, > >>> .nr_uart = 1, > >>> - .has_mctrl = 0, > >>> }; > >>> > >>> static const struct sc16is7xx_devtype sc16is750_devtype = { > >>> .name = "SC16IS750", > >>> - .nr_gpio = 4, > >>> + .nr_gpio = 8, > >> I think this one line change is all you really need here, right? the > >> otner changes do nothing in this patch, so you should just create a new > >> one changing this value. Oh, and this one: > >> > >>> .nr_uart = 1, > >>> - .has_mctrl = 1, > >>> }; > >>> > >>> static const struct sc16is7xx_devtype sc16is752_devtype = { > >>> .name = "SC16IS752", > >>> - .nr_gpio = 0, > >>> + .nr_gpio = 8, > >> right? > >> > >> Don't mess with the has_mctrl stuff, that's not relevant here. > > Sorry, I just noticed that simply reverting commit 679875d1d880 is not sufficient (and will not compile). We must also revert part of commit: > > 21144bab4f11 ("sc16is7xx: Handle modem status lines"). > > > > The problem is that the commit 679875d1d880 was incomplete, and it was (unfortunately) completed by integrating it in commit 21144bab4f11 ("sc16is7xx: Handle modem status lines"). The relevant change was only these 5 new lines, burried deeply into the second commit: > Just as you noticed, this was required to support full set of flow control lines on this device. > The commit you're trying to revert was a preparation for it. Disabling has_mctrl will break it. > I kindly suggest to suggest a fix, instead of hurrying a revert, and waiting for a proper fix later. Hi Lech, the [RFC] in the subject was there to discuss about a possible revert, and/or maybe a possible fix that would allow both modes to be supported. I am not hurrying anything and I am certainly not waiting for a later fix, as I very much want to help and maybe submit such a fix myself. But the reality is that commits 679875d1d880/21144bab4f11 broke userspace by forcing GPIOs as modem control lines. I understand that reverting these patches could also potentially break things for applications depending on these patches. I am simply wondering what is the proper course of action here: revert patches and work on a fix to support both modes, or skip revert and work on a fix (my preference)? > > @@ -1353,9 +1452,17 @@ static int sc16is7xx_probe(struct device *dev, > > sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_EFCR_REG, > > SC16IS7XX_EFCR_RXDISABLE_BIT | > > SC16IS7XX_EFCR_TXDISABLE_BIT); > > + > > + /* Use GPIO lines as modem status registers */ > > + if (devtype->has_mctrl) > > + sc16is7xx_port_write(&s->p[i].port, > > + SC16IS7XX_IOCONTROL_REG, > > + SC16IS7XX_IOCONTROL_MODEM_BIT); > > + > > > > Therefore, I should also remove these lines if we go forward with a revert of the patch (should I add another tag "Fixes..." in that case?). > > > > And what do you think of my proposal to maybe replace has_mctrl with a device tree property so that both modes can be fully supported? > I think the proper solution here, is not to invent a new device tree property for every single use case. > I would start by looking for other drivers, if, and how they handle similar cases. > For example, imx-serial driver respects "uart-has-rtscts" property, as do a lot of other controllers built into SoC-s. > On the other hand, other devices which can also provide GPIOs, respect "gpio-controller" property. I think that testing the presence of the "uart-has-rtscts" to force GPIOs as modem control lines would make a lot of sense. > According to SC16IS752 datasheet [1], respecting one of those should be enough, > as GPIOs can be enabled in groups of four pins even for dual UART version. > Every group matches a single port, so probably this can be probably selected per UART even on dual-port versions. I am trying to see how we could set "uart-has-rtscts" for only UART channel A or B in the device tree, but cannot find any example or documentation about that. How do you propose to do it? From what I understand, the property "uart-has-rtscts" can be set only for the whole chip (channels A and B)... Hugo. > I'll be more than happy to assist with that. > > > > > Thank you, > > Hugo. > > > [1] https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf > > -- > Pozdrawiam/With kind regards, > Lech Perczak > > Sr. Software Engineer > Camlin Technologies Poland Limited Sp. z o.o. > Strzegomska 54, > 53-611 Wroclaw > Tel: (+48) 71 75 000 16 > Email: lech.perczak@camlingroup.com > Website: http://www.camlingroup.com > >
On Tue, 16 May 2023 11:59:06 -0400 Hugo Villeneuve <hugo@hugovil.com> wrote: > On Tue, 16 May 2023 10:50:11 +0200 > Lech Perczak <lech.perczak@camlingroup.com> wrote: > > > Hi Hugo, > > > > Please see my answers inline. > > > > W dniu 15.05.2023 o 18:51, Hugo Villeneuve pisze: > > > Hi Greg, > > > > > > On Mon, 15 May 2023 18:20:02 +0200 > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > > > > >> On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote: > > >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > >>> > > >>> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61. > > >>> > > >>> Because of this commit, it is no longer possible to use the 16 GPIO > > >>> lines as dedicated GPIOs on the SC16IS752. > > >>> > > >>> Reverting it makes it work again. > > >>> > > >>> The log message of the original commit states: > > >>> "Export only the GPIOs that are not shared with hardware modem > > >>> control lines" > > >>> > > >>> But there is no explanation as to why this decision was taken to > > >>> permanently set the function of the GPIO lines as modem control > > >>> lines. AFAIK, there is no problem with using these lines as GPIO or modem > > >>> control lines. > > >>> > > >>> Maybe after reverting this commit, we could define a new > > >>> device-tree property named, for example, > > >>> "use-modem-control-lines", so that both options can be supported. > > >>> > > >>> Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control > > >>> lines") > > >> Please do not line-wrap these lines. > > > Ok. > > > > > >> Nor is a blank line needed here. > > > Ok. > > > > > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > >>> --- > > >>> drivers/tty/serial/sc16is7xx.c | 14 ++++---------- > > >>> 1 file changed, 4 insertions(+), 10 deletions(-) > > >>> > > >>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > > >>> index 5bd98e4316f5..25f1b2f6ec51 100644 > > >>> --- a/drivers/tty/serial/sc16is7xx.c > > >>> +++ b/drivers/tty/serial/sc16is7xx.c > > >>> @@ -306,7 +306,6 @@ struct sc16is7xx_devtype { > > >>> char name[10]; > > >>> int nr_gpio; > > >>> int nr_uart; > > >>> - int has_mctrl; > > >>> }; > > >>> > > >>> #define SC16IS7XX_RECONF_MD (1 << 0) > > >>> @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = { > > >>> .name = "SC16IS74X", > > >>> .nr_gpio = 0, > > >>> .nr_uart = 1, > > >>> - .has_mctrl = 0, > > >>> }; > > >>> > > >>> static const struct sc16is7xx_devtype sc16is750_devtype = { > > >>> .name = "SC16IS750", > > >>> - .nr_gpio = 4, > > >>> + .nr_gpio = 8, > > >> I think this one line change is all you really need here, right? the > > >> otner changes do nothing in this patch, so you should just create a new > > >> one changing this value. Oh, and this one: > > >> > > >>> .nr_uart = 1, > > >>> - .has_mctrl = 1, > > >>> }; > > >>> > > >>> static const struct sc16is7xx_devtype sc16is752_devtype = { > > >>> .name = "SC16IS752", > > >>> - .nr_gpio = 0, > > >>> + .nr_gpio = 8, > > >> right? > > >> > > >> Don't mess with the has_mctrl stuff, that's not relevant here. > > > Sorry, I just noticed that simply reverting commit 679875d1d880 is not sufficient (and will not compile). We must also revert part of commit: > > > 21144bab4f11 ("sc16is7xx: Handle modem status lines"). > > > > > > The problem is that the commit 679875d1d880 was incomplete, and it was (unfortunately) completed by integrating it in commit 21144bab4f11 ("sc16is7xx: Handle modem status lines"). The relevant change was only these 5 new lines, burried deeply into the second commit: > > Just as you noticed, this was required to support full set of flow control lines on this device. > > The commit you're trying to revert was a preparation for it. Disabling has_mctrl will break it. > > I kindly suggest to suggest a fix, instead of hurrying a revert, and waiting for a proper fix later. > > Hi Lech, > the [RFC] in the subject was there to discuss about a possible revert, and/or maybe a possible fix that would allow both modes to be supported. I am not hurrying anything and I am certainly not waiting for a later fix, as I very much want to help and maybe submit such a fix myself. > > But the reality is that commits 679875d1d880/21144bab4f11 broke userspace by forcing GPIOs as modem control lines. I understand that reverting these patches could also potentially break things for applications depending on these patches. I am simply wondering what is the proper course of action here: revert patches and work on a fix to support both modes, or skip revert and work on a fix (my preference)? > > > > @@ -1353,9 +1452,17 @@ static int sc16is7xx_probe(struct device *dev, > > > sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_EFCR_REG, > > > SC16IS7XX_EFCR_RXDISABLE_BIT | > > > SC16IS7XX_EFCR_TXDISABLE_BIT); > > > + > > > + /* Use GPIO lines as modem status registers */ > > > + if (devtype->has_mctrl) > > > + sc16is7xx_port_write(&s->p[i].port, > > > + SC16IS7XX_IOCONTROL_REG, > > > + SC16IS7XX_IOCONTROL_MODEM_BIT); > > > + > > > > > > Therefore, I should also remove these lines if we go forward with a revert of the patch (should I add another tag "Fixes..." in that case?). > > > > > > And what do you think of my proposal to maybe replace has_mctrl with a device tree property so that both modes can be fully supported? > > I think the proper solution here, is not to invent a new device tree property for every single use case. > > I would start by looking for other drivers, if, and how they handle similar cases. > > For example, imx-serial driver respects "uart-has-rtscts" property, as do a lot of other controllers built into SoC-s. > > On the other hand, other devices which can also provide GPIOs, respect "gpio-controller" property. > > I think that testing the presence of the "uart-has-rtscts" to force GPIOs as modem control lines would make a lot of sense. > > > According to SC16IS752 datasheet [1], respecting one of those should be enough, > > as GPIOs can be enabled in groups of four pins even for dual UART version. > > Every group matches a single port, so probably this can be probably selected per UART even on dual-port versions. > > I am trying to see how we could set "uart-has-rtscts" for only UART channel A or B in the device tree, but cannot find any example or documentation about that. How do you propose to do it? > > From what I understand, the property "uart-has-rtscts" can be set only for the whole chip (channels A and B)... After some analysis, I don't think that we should be using the property "uart-has-rtscts". For our chip, this doesn't make sense because RTS/CTS are dedicated pins. also, like I said, this property applies to the whole chip/device, not to indivual A or B channels (like sc16is752). The way to go would be to define a new DT property similar to "irda-mode-ports" (for the same sc16is7xx driver). Defining a new property named "modem-control-line-ports" would allow us to specify an array that lists the indices of the port that should have shared GPIO lines configured as modem control lines. I already implemented that as a proof of concept and it works great. Hugo. > > I'll be more than happy to assist with that. > > > > > > > > Thank you, > > > Hugo. > > > > > [1] https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf > > > > -- > > Pozdrawiam/With kind regards, > > Lech Perczak > > > > Sr. Software Engineer > > Camlin Technologies Poland Limited Sp. z o.o. > > Strzegomska 54, > > 53-611 Wroclaw > > Tel: (+48) 71 75 000 16 > > Email: lech.perczak@camlingroup.com > > Website: http://www.camlingroup.com > > > > > > > -- > Hugo Villeneuve >
W dniu 17.05.2023 o 00:09, Hugo Villeneuve pisze: > On Tue, 16 May 2023 11:59:06 -0400 > Hugo Villeneuve <hugo@hugovil.com> wrote: > > > On Tue, 16 May 2023 10:50:11 +0200 > > Lech Perczak <lech.perczak@camlingroup.com> wrote: > > > > > Hi Hugo, > > > > > > Please see my answers inline. > > > > > > W dniu 15.05.2023 o 18:51, Hugo Villeneuve pisze: > > > > Hi Greg, > > > > > > > > On Mon, 15 May 2023 18:20:02 +0200 > > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > > > > > > >> On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote: > > > >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > >>> > > > >>> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61. > > > >>> > > > >>> Because of this commit, it is no longer possible to use the 16 GPIO > > > >>> lines as dedicated GPIOs on the SC16IS752. > > > >>> > > > >>> Reverting it makes it work again. > > > >>> > > > >>> The log message of the original commit states: > > > >>> "Export only the GPIOs that are not shared with hardware modem > > > >>> control lines" > > > >>> > > > >>> But there is no explanation as to why this decision was taken to > > > >>> permanently set the function of the GPIO lines as modem control > > > >>> lines. AFAIK, there is no problem with using these lines as GPIO or modem > > > >>> control lines. > > > >>> > > > >>> Maybe after reverting this commit, we could define a new > > > >>> device-tree property named, for example, > > > >>> "use-modem-control-lines", so that both options can be supported. > > > >>> > > > >>> Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control > > > >>> lines") > > > >> Please do not line-wrap these lines. > > > > Ok. > > > > > > > >> Nor is a blank line needed here. > > > > Ok. > > > > > > > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > >>> --- > > > >>> drivers/tty/serial/sc16is7xx.c | 14 ++++---------- > > > >>> 1 file changed, 4 insertions(+), 10 deletions(-) > > > >>> > > > >>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > > > >>> index 5bd98e4316f5..25f1b2f6ec51 100644 > > > >>> --- a/drivers/tty/serial/sc16is7xx.c > > > >>> +++ b/drivers/tty/serial/sc16is7xx.c > > > >>> @@ -306,7 +306,6 @@ struct sc16is7xx_devtype { > > > >>> char name[10]; > > > >>> int nr_gpio; > > > >>> int nr_uart; > > > >>> - int has_mctrl; > > > >>> }; > > > >>> > > > >>> #define SC16IS7XX_RECONF_MD (1 << 0) > > > >>> @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = { > > > >>> .name = "SC16IS74X", > > > >>> .nr_gpio = 0, > > > >>> .nr_uart = 1, > > > >>> - .has_mctrl = 0, > > > >>> }; > > > >>> > > > >>> static const struct sc16is7xx_devtype sc16is750_devtype = { > > > >>> .name = "SC16IS750", > > > >>> - .nr_gpio = 4, > > > >>> + .nr_gpio = 8, > > > >> I think this one line change is all you really need here, right? the > > > >> otner changes do nothing in this patch, so you should just create a new > > > >> one changing this value. Oh, and this one: > > > >> > > > >>> .nr_uart = 1, > > > >>> - .has_mctrl = 1, > > > >>> }; > > > >>> > > > >>> static const struct sc16is7xx_devtype sc16is752_devtype = { > > > >>> .name = "SC16IS752", > > > >>> - .nr_gpio = 0, > > > >>> + .nr_gpio = 8, > > > >> right? > > > >> > > > >> Don't mess with the has_mctrl stuff, that's not relevant here. > > > > Sorry, I just noticed that simply reverting commit 679875d1d880 is not sufficient (and will not compile). We must also revert part of commit: > > > > 21144bab4f11 ("sc16is7xx: Handle modem status lines"). > > > > > > > > The problem is that the commit 679875d1d880 was incomplete, and it was (unfortunately) completed by integrating it in commit 21144bab4f11 ("sc16is7xx: Handle modem status lines"). The relevant change was only these 5 new lines, burried deeply into the second commit: > > > Just as you noticed, this was required to support full set of flow control lines on this device. > > > The commit you're trying to revert was a preparation for it. Disabling has_mctrl will break it. > > > I kindly suggest to suggest a fix, instead of hurrying a revert, and waiting for a proper fix later. > > > > Hi Lech, > > the [RFC] in the subject was there to discuss about a possible revert, and/or maybe a possible fix that would allow both modes to be supported. I am not hurrying anything and I am certainly not waiting for a later fix, as I very much want to help and maybe submit such a fix myself. > > > > But the reality is that commits 679875d1d880/21144bab4f11 broke userspace by forcing GPIOs as modem control lines. I understand that reverting these patches could also potentially break things for applications depending on these patches. I am simply wondering what is the proper course of action here: revert patches and work on a fix to support both modes, or skip revert and work on a fix (my preference)? > > > > > > @@ -1353,9 +1452,17 @@ static int sc16is7xx_probe(struct device *dev, > > > > sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_EFCR_REG, > > > > SC16IS7XX_EFCR_RXDISABLE_BIT | > > > > SC16IS7XX_EFCR_TXDISABLE_BIT); > > > > + > > > > + /* Use GPIO lines as modem status registers */ > > > > + if (devtype->has_mctrl) > > > > + sc16is7xx_port_write(&s->p[i].port, > > > > + SC16IS7XX_IOCONTROL_REG, > > > > + SC16IS7XX_IOCONTROL_MODEM_BIT); > > > > + > > > > > > > > Therefore, I should also remove these lines if we go forward with a revert of the patch (should I add another tag "Fixes..." in that case?). > > > > > > > > And what do you think of my proposal to maybe replace has_mctrl with a device tree property so that both modes can be fully supported? > > > I think the proper solution here, is not to invent a new device tree property for every single use case. > > > I would start by looking for other drivers, if, and how they handle similar cases. > > > For example, imx-serial driver respects "uart-has-rtscts" property, as do a lot of other controllers built into SoC-s. > > > On the other hand, other devices which can also provide GPIOs, respect "gpio-controller" property. > > > > I think that testing the presence of the "uart-has-rtscts" to force GPIOs as modem control lines would make a lot of sense. > > > > > According to SC16IS752 datasheet [1], respecting one of those should be enough, > > > as GPIOs can be enabled in groups of four pins even for dual UART version. > > > Every group matches a single port, so probably this can be probably selected per UART even on dual-port versions. > > > > I am trying to see how we could set "uart-has-rtscts" for only UART channel A or B in the device tree, but cannot find any example or documentation about that. How do you propose to do it? > > > > From what I understand, the property "uart-has-rtscts" can be set only for the whole chip (channels A and B)... > > After some analysis, I don't think that we should be using the property "uart-has-rtscts". For our chip, this doesn't make sense because RTS/CTS are dedicated pins. also, like I said, this property applies to the whole chip/device, not to indivual A or B channels (like sc16is752). Hi Hugo, That's correct, my idea was to analyze what is available and pick the best one, if at all possible. "gpio-controller" could also be used - in theory - but it isn't a great choice either, because it doesn't allow us to specify, which pin groups should be used as GPIOs. > > The way to go would be to define a new DT property similar to "irda-mode-ports" (for the same sc16is7xx driver). Defining a new property named "modem-control-line-ports" would allow us to specify an array that lists the indices of the port that should have shared GPIO lines configured as modem control lines. > > I already implemented that as a proof of concept and it works great. There is nothing wrong per se in adding new device tree property, I'd just like to avoid jumping the gun in inventing one. After quickly reviewing documentation of available bindings I see that it's most likely unavoidable. The "modem-control-line-ports" proposal with a mask of ports sounds very reasonable - please share your PoC, it will be easier to discuss having a concrete example. The general approach I noticed in other places (for example, the WF8960 audio codec) is setting a register value directly. This would allow us to control the IOLATCH bit in IOControl register, to make inputs register behave as interrupt flag register, but I think that if we ever need it, it would be cleaner to set it with a separate boolean property - I'm in favor of modem-control-line-ports. I think it would be a good idea to include DT and GPIO maintainers and mailing lists as well. Especially the DT maintainers - they would like to see this property documented. They however, are not concerned with the code changes themselves. BTW, the commit split between adding has_mctrl property and the rest of implementation warrants some explanation - this was based on my previous patches, which Tomasz reworked and submitted. The split was kept to split up the changes to minimal, logical chunks, and to maintain correct authorship of the changes. With kind regards, Lech > > Hugo. > > > > > I'll be more than happy to assist with that. > > > > > > > > > > > Thank you, > > > > Hugo. > > > > > > > [1] https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf <https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf> > > > > > > -- > > > Pozdrawiam/With kind regards, > > > Lech Perczak > > > > > > Sr. Software Engineer > > > Camlin Technologies Poland Limited Sp. z o.o. > > > Strzegomska 54, > > > 53-611 Wroclaw > > > Tel: (+48) 71 75 000 16 > > > Email: lech.perczak@camlingroup.com > > > Website: http://www.camlingroup.com <http://www.camlingroup.com> > > > > > > > > > > > > -- > > Hugo Villeneuve > > > > > -- > Hugo Villeneuve
On Wed, 17 May 2023 11:18:57 +0200 Lech Perczak <lech.perczak@camlingroup.com> wrote: > > W dniu 17.05.2023 o 00:09, Hugo Villeneuve pisze: > > On Tue, 16 May 2023 11:59:06 -0400 > > Hugo Villeneuve <hugo@hugovil.com> wrote: > > > > > On Tue, 16 May 2023 10:50:11 +0200 > > > Lech Perczak <lech.perczak@camlingroup.com> wrote: > > > > > > > Hi Hugo, > > > > > > > > Please see my answers inline. > > > > > > > > W dniu 15.05.2023 o 18:51, Hugo Villeneuve pisze: > > > > > Hi Greg, > > > > > > > > > > On Mon, 15 May 2023 18:20:02 +0200 > > > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > > > > > > > > >> On Mon, May 15, 2023 at 12:02:07PM -0400, Hugo Villeneuve wrote: > > > > >>> From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > >>> > > > > >>> This reverts commit 679875d1d8802669590ef4d69b0e7d13207ebd61. > > > > >>> > > > > >>> Because of this commit, it is no longer possible to use the 16 GPIO > > > > >>> lines as dedicated GPIOs on the SC16IS752. > > > > >>> > > > > >>> Reverting it makes it work again. > > > > >>> > > > > >>> The log message of the original commit states: > > > > >>> "Export only the GPIOs that are not shared with hardware modem > > > > >>> control lines" > > > > >>> > > > > >>> But there is no explanation as to why this decision was taken to > > > > >>> permanently set the function of the GPIO lines as modem control > > > > >>> lines. AFAIK, there is no problem with using these lines as GPIO or modem > > > > >>> control lines. > > > > >>> > > > > >>> Maybe after reverting this commit, we could define a new > > > > >>> device-tree property named, for example, > > > > >>> "use-modem-control-lines", so that both options can be supported. > > > > >>> > > > > >>> Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control > > > > >>> lines") > > > > >> Please do not line-wrap these lines. > > > > > Ok. > > > > > > > > > >> Nor is a blank line needed here. > > > > > Ok. > > > > > > > > > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > >>> --- > > > > >>> drivers/tty/serial/sc16is7xx.c | 14 ++++---------- > > > > >>> 1 file changed, 4 insertions(+), 10 deletions(-) > > > > >>> > > > > >>> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > > > > >>> index 5bd98e4316f5..25f1b2f6ec51 100644 > > > > >>> --- a/drivers/tty/serial/sc16is7xx.c > > > > >>> +++ b/drivers/tty/serial/sc16is7xx.c > > > > >>> @@ -306,7 +306,6 @@ struct sc16is7xx_devtype { > > > > >>> char name[10]; > > > > >>> int nr_gpio; > > > > >>> int nr_uart; > > > > >>> - int has_mctrl; > > > > >>> }; > > > > >>> > > > > >>> #define SC16IS7XX_RECONF_MD (1 << 0) > > > > >>> @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = { > > > > >>> .name = "SC16IS74X", > > > > >>> .nr_gpio = 0, > > > > >>> .nr_uart = 1, > > > > >>> - .has_mctrl = 0, > > > > >>> }; > > > > >>> > > > > >>> static const struct sc16is7xx_devtype sc16is750_devtype = { > > > > >>> .name = "SC16IS750", > > > > >>> - .nr_gpio = 4, > > > > >>> + .nr_gpio = 8, > > > > >> I think this one line change is all you really need here, right? the > > > > >> otner changes do nothing in this patch, so you should just create a new > > > > >> one changing this value. Oh, and this one: > > > > >> > > > > >>> .nr_uart = 1, > > > > >>> - .has_mctrl = 1, > > > > >>> }; > > > > >>> > > > > >>> static const struct sc16is7xx_devtype sc16is752_devtype = { > > > > >>> .name = "SC16IS752", > > > > >>> - .nr_gpio = 0, > > > > >>> + .nr_gpio = 8, > > > > >> right? > > > > >> > > > > >> Don't mess with the has_mctrl stuff, that's not relevant here. > > > > > Sorry, I just noticed that simply reverting commit 679875d1d880 is not sufficient (and will not compile). We must also revert part of commit: > > > > > 21144bab4f11 ("sc16is7xx: Handle modem status lines"). > > > > > > > > > > The problem is that the commit 679875d1d880 was incomplete, and it was (unfortunately) completed by integrating it in commit 21144bab4f11 ("sc16is7xx: Handle modem status lines"). The relevant change was only these 5 new lines, burried deeply into the second commit: > > > > Just as you noticed, this was required to support full set of flow control lines on this device. > > > > The commit you're trying to revert was a preparation for it. Disabling has_mctrl will break it. > > > > I kindly suggest to suggest a fix, instead of hurrying a revert, and waiting for a proper fix later. > > > > > > Hi Lech, > > > the [RFC] in the subject was there to discuss about a possible revert, and/or maybe a possible fix that would allow both modes to be supported. I am not hurrying anything and I am certainly not waiting for a later fix, as I very much want to help and maybe submit such a fix myself. > > > > > > But the reality is that commits 679875d1d880/21144bab4f11 broke userspace by forcing GPIOs as modem control lines. I understand that reverting these patches could also potentially break things for applications depending on these patches. I am simply wondering what is the proper course of action here: revert patches and work on a fix to support both modes, or skip revert and work on a fix (my preference)? > > > > > > > > @@ -1353,9 +1452,17 @@ static int sc16is7xx_probe(struct device *dev, > > > > > sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_EFCR_REG, > > > > > SC16IS7XX_EFCR_RXDISABLE_BIT | > > > > > SC16IS7XX_EFCR_TXDISABLE_BIT); > > > > > + > > > > > + /* Use GPIO lines as modem status registers */ > > > > > + if (devtype->has_mctrl) > > > > > + sc16is7xx_port_write(&s->p[i].port, > > > > > + SC16IS7XX_IOCONTROL_REG, > > > > > + SC16IS7XX_IOCONTROL_MODEM_BIT); > > > > > + > > > > > > > > > > Therefore, I should also remove these lines if we go forward with a revert of the patch (should I add another tag "Fixes..." in that case?). > > > > > > > > > > And what do you think of my proposal to maybe replace has_mctrl with a device tree property so that both modes can be fully supported? > > > > I think the proper solution here, is not to invent a new device tree property for every single use case. > > > > I would start by looking for other drivers, if, and how they handle similar cases. > > > > For example, imx-serial driver respects "uart-has-rtscts" property, as do a lot of other controllers built into SoC-s. > > > > On the other hand, other devices which can also provide GPIOs, respect "gpio-controller" property. > > > > > > I think that testing the presence of the "uart-has-rtscts" to force GPIOs as modem control lines would make a lot of sense. > > > > > > > According to SC16IS752 datasheet [1], respecting one of those should be enough, > > > > as GPIOs can be enabled in groups of four pins even for dual UART version. > > > > Every group matches a single port, so probably this can be probably selected per UART even on dual-port versions. > > > > > > I am trying to see how we could set "uart-has-rtscts" for only UART channel A or B in the device tree, but cannot find any example or documentation about that. How do you propose to do it? > > > > > > From what I understand, the property "uart-has-rtscts" can be set only for the whole chip (channels A and B)... > > > > After some analysis, I don't think that we should be using the property "uart-has-rtscts". For our chip, this doesn't make sense because RTS/CTS are dedicated pins. also, like I said, this property applies to the whole chip/device, not to indivual A or B channels (like sc16is752). > > Hi Hugo, > > That's correct, my idea was to analyze what is available and pick the best one, if at all possible. > "gpio-controller" could also be used - in theory - but it isn't a great choice either, because it doesn't allow us to specify, which pin groups should be used as GPIOs. > > > > The way to go would be to define a new DT property similar to "irda-mode-ports" (for the same sc16is7xx driver). Defining a new property named "modem-control-line-ports" would allow us to specify an array that lists the indices of the port that should have shared GPIO lines configured as modem control lines. > > > > I already implemented that as a proof of concept and it works great. > > There is nothing wrong per se in adding new device tree property, I'd just like to avoid jumping the gun in inventing one. > After quickly reviewing documentation of available bindings I see that it's most likely unavoidable. > The "modem-control-line-ports" proposal with a mask of ports sounds very reasonable - please share your PoC, > it will be easier to discuss having a concrete example. > > The general approach I noticed in other places (for example, the WF8960 audio codec) is setting a register value directly. > This would allow us to control the IOLATCH bit in IOControl register, to make inputs register behave as interrupt flag register, > but I think that if we ever need it, it would be cleaner to set it with a separate boolean property - I'm in favor of modem-control-line-ports. Hi Lech, I think that control of the IOLATCH bit should be part of a separate discussion (patch) if we want to change it. For the moment, I just want to restore the GPIO function as it was before your patches. > I think it would be a good idea to include DT and GPIO maintainers and mailing lists as well. > Especially the DT maintainers - they would like to see this property documented. They however, are not concerned with the code changes themselves. Ok, I will submit my PoC soon. Thank you, Hugo. > BTW, the commit split between adding has_mctrl property and the rest of implementation warrants some explanation - this was based on my previous patches, > which Tomasz reworked and submitted. The split was kept to split up the changes to minimal, logical chunks, and to maintain correct authorship of the changes. > > With kind regards, > Lech > > > > Hugo. > > > > > > > > I'll be more than happy to assist with that. > > > > > > > > > > > > > > Thank you, > > > > > Hugo. > > > > > > > > > [1] https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf <https://www.nxp.com/docs/en/data-sheet/SC16IS752_SC16IS762.pdf> > > > > > > > > -- > > > > Pozdrawiam/With kind regards, > > > > Lech Perczak > > > > > > > > Sr. Software Engineer > > > > Camlin Technologies Poland Limited Sp. z o.o. > > > > Strzegomska 54, > > > > 53-611 Wroclaw > > > > Tel: (+48) 71 75 000 16 > > > > Email: lech.perczak@camlingroup.com > > > > Website: http://www.camlingroup.com <http://www.camlingroup.com> > > > > > > > > > > > > > > > > > -- > > > Hugo Villeneuve > > > > > > > > > -- > > Hugo Villeneuve > >
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 5bd98e4316f5..25f1b2f6ec51 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -306,7 +306,6 @@ struct sc16is7xx_devtype { char name[10]; int nr_gpio; int nr_uart; - int has_mctrl; }; #define SC16IS7XX_RECONF_MD (1 << 0) @@ -447,35 +446,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = { .name = "SC16IS74X", .nr_gpio = 0, .nr_uart = 1, - .has_mctrl = 0, }; static const struct sc16is7xx_devtype sc16is750_devtype = { .name = "SC16IS750", - .nr_gpio = 4, + .nr_gpio = 8, .nr_uart = 1, - .has_mctrl = 1, }; static const struct sc16is7xx_devtype sc16is752_devtype = { .name = "SC16IS752", - .nr_gpio = 0, + .nr_gpio = 8, .nr_uart = 2, - .has_mctrl = 1, }; static const struct sc16is7xx_devtype sc16is760_devtype = { .name = "SC16IS760", - .nr_gpio = 4, + .nr_gpio = 8, .nr_uart = 1, - .has_mctrl = 1, }; static const struct sc16is7xx_devtype sc16is762_devtype = { .name = "SC16IS762", - .nr_gpio = 0, + .nr_gpio = 8, .nr_uart = 2, - .has_mctrl = 1, }; static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)