diff mbox series

[RESEND,v8,06/10] serial: sc16is7xx: fix regression with GPIO configuration

Message ID 20230721161840.1393996-7-hugo@hugovil.com
State Superseded
Headers show
Series [RESEND,v8,01/10] serial: sc16is7xx: fix broken port 0 uart init | expand

Commit Message

Hugo Villeneuve July 21, 2023, 4:18 p.m. UTC 
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
changed the function of the GPIOs pins to act as modem control
lines without any possibility of selecting GPIO function.

As a consequence, applications that depends on GPIO lines configured
by default as GPIO pins no longer work as expected.

Also, the change to select modem control lines function was done only
for channel A of dual UART variants (752/762). This was not documented
in the log message.

Allow to specify GPIO or modem control line function in the device
tree, and for each of the ports (A or B).

Do so by using the new device-tree property named
"modem-control-line-ports" (property added in separate patch).

When registering GPIO chip controller, mask-out GPIO pins declared as
modem control lines according to this new "modem-control-line-ports"
DT property.

Boards that need to have GPIOS configured as modem control lines
should add that property to their device tree. Here is a list of
boards using the sc16is7xx driver in their device tree and that may
need to be modified:
    arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
    mips/boot/dts/ingenic/cu1830-neo.dts
    mips/boot/dts/ingenic/cu1000-neo.dts

Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines")
Cc: <stable@vger.kernel.org> # 6.1.x: 95982fad dt-bindings: sc16is7xx: Add property to change GPIO function
Cc: <stable@vger.kernel.org> # 6.1.x: 1584d572 serial: sc16is7xx: refactor GPIO controller registration
Cc: <stable@vger.kernel.org> # 6.1.x: ac2caa5a serial: sc16is7xx: remove obsolete out_thread label
Cc: <stable@vger.kernel.org> # 6.1.x: d90961ad serial: sc16is7xx: mark IOCONTROL register as volatile
Cc: <stable@vger.kernel.org> # 6.1.x: 6dae3bad serial: sc16is7xx: fix broken port 0 uart init
Cc: <stable@vger.kernel.org> # 6.1.x
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Lech Perczak <lech.perczak@camlingroup.com>
Tested-by: Lech Perczak <lech.perczak@camlingroup.com>
---
 drivers/tty/serial/sc16is7xx.c | 104 +++++++++++++++++++++++++++------
 1 file changed, 85 insertions(+), 19 deletions(-)

Comments

Hugo Villeneuve July 22, 2023, 2:47 p.m. UTC | #1 
On Fri, 21 Jul 2023 13:24:19 -0600
Rob Herring <robh+dt@kernel.org> wrote:

> On Fri, Jul 21, 2023 at 10:19 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> >
> > Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > changed the function of the GPIOs pins to act as modem control
> > lines without any possibility of selecting GPIO function.
> 
> Requiring a new DT property is not fixing a kernel regression. You
> should be returning the kernel to original behavior and then have a
> new DT property for new behavior.

Hi Rob,
please read the entire patch history starting from V1
 and you will understand why this course of action was
 not selected.

 
> > As a consequence, applications that depends on GPIO lines configured
> > by default as GPIO pins no longer work as expected.
> >
> > Also, the change to select modem control lines function was done only
> > for channel A of dual UART variants (752/762). This was not documented
> > in the log message.
> >
> > Allow to specify GPIO or modem control line function in the device
> > tree, and for each of the ports (A or B).
> >
> > Do so by using the new device-tree property named
> > "modem-control-line-ports" (property added in separate patch).
> 
> That's not the name in the patch.

We added a "nxp," prefix at some point.

Do you want me to send a V9 only with this change?

Hugo.


> > When registering GPIO chip controller, mask-out GPIO pins declared as
> > modem control lines according to this new "modem-control-line-ports"
> > DT property.
> >
> > Boards that need to have GPIOS configured as modem control lines
> > should add that property to their device tree. Here is a list of
> > boards using the sc16is7xx driver in their device tree and that may
> > need to be modified:
> >     arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> >     mips/boot/dts/ingenic/cu1830-neo.dts
> >     mips/boot/dts/ingenic/cu1000-neo.dts
> 
> Then again, if no one cares about those boards needing a change then
> it can be okay.
> 
> 
> Rob
>
Hugo Villeneuve July 24, 2023, 3:54 p.m. UTC | #2 
On Sat, 22 Jul 2023 17:15:26 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Sat, Jul 22, 2023 at 10:47:24AM -0400, Hugo Villeneuve wrote:
> > On Fri, 21 Jul 2023 13:24:19 -0600
> > Rob Herring <robh+dt@kernel.org> wrote:
> > 
> > > On Fri, Jul 21, 2023 at 10:19 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > >
> > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > >
> > > > Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > > > and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > > > changed the function of the GPIOs pins to act as modem control
> > > > lines without any possibility of selecting GPIO function.
> > > 
> > > Requiring a new DT property is not fixing a kernel regression. You
> > > should be returning the kernel to original behavior and then have a
> > > new DT property for new behavior.
> > 
> > Hi Rob,
> > please read the entire patch history starting from V1
> >  and you will understand why this course of action was
> >  not selected.
> 
> That's not going to happen, sorry, you need to explain it here, in this
> patch series, why a specific action is being taken over another one, as
> no one has time to go dig through past history, sorry.

Hi Rob,
I initially submitted a patch to revert the kernel to original
behavior, but it created more problems because the patch was
unfortunately split in two separate patches, and mixed with other non
closely-related changes. It was also noted to me that reverting to the
old behavior would break things for some users.

It was suggested to me by a more experienced kernel developer to
"suggest a fix, instead of hurrying a revert":

    https://lkml.org/lkml/2023/5/17/758

That is what we decided to do in the end, and it worked quite well.

Hugo.
Hugo Villeneuve Aug. 3, 2023, 5:54 p.m. UTC | #3 
On Mon, 31 Jul 2023 14:41:15 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Mon, 31 Jul 2023 12:04:45 -0600
> Rob Herring <robh+dt@kernel.org> wrote:
> 
> > On Mon, Jul 31, 2023 at 10:46 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > On Mon, 31 Jul 2023 09:31:53 -0600
> > > Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > > On Mon, Jul 24, 2023 at 9:54 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > >
> > > > > On Sat, 22 Jul 2023 17:15:26 +0200
> > > > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > > On Sat, Jul 22, 2023 at 10:47:24AM -0400, Hugo Villeneuve wrote:
> > > > > > > On Fri, 21 Jul 2023 13:24:19 -0600
> > > > > > > Rob Herring <robh+dt@kernel.org> wrote:
> > > > > > >
> > > > > > > > On Fri, Jul 21, 2023 at 10:19 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > > >
> > > > > > > > > Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > > > > > > > > and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > > > > > > > > changed the function of the GPIOs pins to act as modem control
> > > > > > > > > lines without any possibility of selecting GPIO function.
> > > > > > > >
> > > > > > > > Requiring a new DT property is not fixing a kernel regression. You
> > > > > > > > should be returning the kernel to original behavior and then have a
> > > > > > > > new DT property for new behavior.
> > > > > > >
> > > > > > > Hi Rob,
> > > > > > > please read the entire patch history starting from V1
> > > > > > >  and you will understand why this course of action was
> > > > > > >  not selected.
> > > > > >
> > > > > > That's not going to happen, sorry, you need to explain it here, in this
> > > > > > patch series, why a specific action is being taken over another one, as
> > > > > > no one has time to go dig through past history, sorry.
> > > > >
> > > > > Hi Rob,
> > > > > I initially submitted a patch to revert the kernel to original
> > > > > behavior, but it created more problems because the patch was
> > > > > unfortunately split in two separate patches, and mixed with other non
> > > > > closely-related changes. It was also noted to me that reverting to the
> > > > > old behavior would break things for some users.
> > > > >
> > > > > It was suggested to me by a more experienced kernel developer to
> > > > > "suggest a fix, instead of hurrying a revert":
> > > > >
> > > > >     https://lkml.org/lkml/2023/5/17/758
> > > >
> > > > Do I have to go read this to decipher the justification and reasoning?
> > > > When Greg says "in this patch series", he means in the commit messages
> > > > of the patches. You send v9 already and it doesn't have that. The
> > > > patchset needs to stand on its own summarizing any relevant prior
> > > > discussions.
> > > >
> > > > I never suggested doing a revert.
> > >
> > > Hi Rob,
> > > I am sorry, but this is exactly what I "deciphered" from your
> > > original email.
> > >
> > > I am trying very hard to understand exactly what you mean, but it is
> > > not that obvious for me. If something is not clear in my commit message,
> > > I will try to improve it. But before, let's try to focus on making sure
> > > I understand more clearly what you want exactly.
> > >
> > > > Obviously, someone still wants the
> > > > new feature.
> > >
> > > I assume that you refer to the "new feature" as what was added in
> > > the commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control
> > > lines")?
> > 
> > Shrug. It's one of the 2 commits mentioned, I don't know which one
> > exactly. Whichever one changed default behavior from use GPIOs to use
> > modem ctrl lines.
> > 
> > Reading it again, I *think* this patch is correct. Default behavior is
> > restored to use GPIOs. The DT property is needed to enable modem ctrl
> > lines.
> 
> Hi,
> this is correct.
> 
> 
> > What's not okay is just saying, these platforms may or may not need an update:
> > 
> >     arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> >     mips/boot/dts/ingenic/cu1830-neo.dts
> >     mips/boot/dts/ingenic/cu1000-neo.dts
> 
> Yes, my bad. I initially mentioned them and hoped to get some
> feedback, which I never got, and I kind of forgot about it.
> 
> > You need to figure that out. Have you checked with maintainers of
> > these boards? When were they added and by who? At the same time or by
> > the same person would be a good indication the platform uses modem
> > ctrl lines. Or were these platforms in use before adding modem ctrl
> > support? Then they probably use GPIOs or nothing.
> > 
> > If there are platforms which would regress if the modem ctrl feature
> > was just reverted, which ones are those?
> 
> Ok, let me do some checks and get back to you on this.

Hi Rob,
for this board:
    arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts

it uses a SC16IS740, which doesn't have any GPIOs nor modem
control lines, so no DT changes required.

For these two Ingenic boards:
    mips/boot/dts/ingenic/cu1830-neo.dts
    mips/boot/dts/ingenic/cu1000-neo.dts

They use a SC16IS752, which has shared modem control lines and GPIOs.
Unfortunately, the maintainers have not (yet) responded to my
inquiries. Also, I tried to search for schematics or block diagrams on
the net but couldn't find anything.

These platforms were in use before the patch to add the modem control
lines was added. Then like you said they probably use these shared
lines as GPIOs or nothing, so no DT changes would be required.

Hugo.
Hugo Villeneuve Aug. 7, 2023, 2:55 p.m. UTC | #4 
On Thu, 3 Aug 2023 17:38:54 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Thu, 3 Aug 2023 14:10:30 -0600
> Rob Herring <robh+dt@kernel.org> wrote:
> 
> > On Thu, Aug 3, 2023 at 11:54 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > On Mon, 31 Jul 2023 14:41:15 -0400
> > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > >
> > > > On Mon, 31 Jul 2023 12:04:45 -0600
> > > > Rob Herring <robh+dt@kernel.org> wrote:
> > > >
> > > > > On Mon, Jul 31, 2023 at 10:46 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > >
> > > > > > On Mon, 31 Jul 2023 09:31:53 -0600
> > > > > > Rob Herring <robh+dt@kernel.org> wrote:
> > > > > >
> > > > > > > On Mon, Jul 24, 2023 at 9:54 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > >
> > > > > > > > On Sat, 22 Jul 2023 17:15:26 +0200
> > > > > > > > Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > > > >
> > > > > > > > > On Sat, Jul 22, 2023 at 10:47:24AM -0400, Hugo Villeneuve wrote:
> > > > > > > > > > On Fri, 21 Jul 2023 13:24:19 -0600
> > > > > > > > > > Rob Herring <robh+dt@kernel.org> wrote:
> > > > > > > > > >
> > > > > > > > > > > On Fri, Jul 21, 2023 at 10:19 AM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > > > > > >
> > > > > > > > > > > > Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > > > > > > > > > > > and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > > > > > > > > > > > changed the function of the GPIOs pins to act as modem control
> > > > > > > > > > > > lines without any possibility of selecting GPIO function.
> > > > > > > > > > >
> > > > > > > > > > > Requiring a new DT property is not fixing a kernel regression. You
> > > > > > > > > > > should be returning the kernel to original behavior and then have a
> > > > > > > > > > > new DT property for new behavior.
> > > > > > > > > >
> > > > > > > > > > Hi Rob,
> > > > > > > > > > please read the entire patch history starting from V1
> > > > > > > > > >  and you will understand why this course of action was
> > > > > > > > > >  not selected.
> > > > > > > > >
> > > > > > > > > That's not going to happen, sorry, you need to explain it here, in this
> > > > > > > > > patch series, why a specific action is being taken over another one, as
> > > > > > > > > no one has time to go dig through past history, sorry.
> > > > > > > >
> > > > > > > > Hi Rob,
> > > > > > > > I initially submitted a patch to revert the kernel to original
> > > > > > > > behavior, but it created more problems because the patch was
> > > > > > > > unfortunately split in two separate patches, and mixed with other non
> > > > > > > > closely-related changes. It was also noted to me that reverting to the
> > > > > > > > old behavior would break things for some users.
> > > > > > > >
> > > > > > > > It was suggested to me by a more experienced kernel developer to
> > > > > > > > "suggest a fix, instead of hurrying a revert":
> > > > > > > >
> > > > > > > >     https://lkml.org/lkml/2023/5/17/758
> > > > > > >
> > > > > > > Do I have to go read this to decipher the justification and reasoning?
> > > > > > > When Greg says "in this patch series", he means in the commit messages
> > > > > > > of the patches. You send v9 already and it doesn't have that. The
> > > > > > > patchset needs to stand on its own summarizing any relevant prior
> > > > > > > discussions.
> > > > > > >
> > > > > > > I never suggested doing a revert.
> > > > > >
> > > > > > Hi Rob,
> > > > > > I am sorry, but this is exactly what I "deciphered" from your
> > > > > > original email.
> > > > > >
> > > > > > I am trying very hard to understand exactly what you mean, but it is
> > > > > > not that obvious for me. If something is not clear in my commit message,
> > > > > > I will try to improve it. But before, let's try to focus on making sure
> > > > > > I understand more clearly what you want exactly.
> > > > > >
> > > > > > > Obviously, someone still wants the
> > > > > > > new feature.
> > > > > >
> > > > > > I assume that you refer to the "new feature" as what was added in
> > > > > > the commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control
> > > > > > lines")?
> > > > >
> > > > > Shrug. It's one of the 2 commits mentioned, I don't know which one
> > > > > exactly. Whichever one changed default behavior from use GPIOs to use
> > > > > modem ctrl lines.
> > > > >
> > > > > Reading it again, I *think* this patch is correct. Default behavior is
> > > > > restored to use GPIOs. The DT property is needed to enable modem ctrl
> > > > > lines.
> > > >
> > > > Hi,
> > > > this is correct.
> > > >
> > > >
> > > > > What's not okay is just saying, these platforms may or may not need an update:
> > > > >
> > > > >     arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> > > > >     mips/boot/dts/ingenic/cu1830-neo.dts
> > > > >     mips/boot/dts/ingenic/cu1000-neo.dts
> > > >
> > > > Yes, my bad. I initially mentioned them and hoped to get some
> > > > feedback, which I never got, and I kind of forgot about it.
> > > >
> > > > > You need to figure that out. Have you checked with maintainers of
> > > > > these boards? When were they added and by who? At the same time or by
> > > > > the same person would be a good indication the platform uses modem
> > > > > ctrl lines. Or were these platforms in use before adding modem ctrl
> > > > > support? Then they probably use GPIOs or nothing.
> > > > >
> > > > > If there are platforms which would regress if the modem ctrl feature
> > > > > was just reverted, which ones are those?
> > > >
> > > > Ok, let me do some checks and get back to you on this.
> > >
> > > Hi Rob,
> > > for this board:
> > >     arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> > >
> > > it uses a SC16IS740, which doesn't have any GPIOs nor modem
> > > control lines, so no DT changes required.
> > >
> > > For these two Ingenic boards:
> > >     mips/boot/dts/ingenic/cu1830-neo.dts
> > >     mips/boot/dts/ingenic/cu1000-neo.dts
> > >
> > > They use a SC16IS752, which has shared modem control lines and GPIOs.
> > > Unfortunately, the maintainers have not (yet) responded to my
> > > inquiries. Also, I tried to search for schematics or block diagrams on
> > > the net but couldn't find anything.
> > >
> > > These platforms were in use before the patch to add the modem control
> > > lines was added. Then like you said they probably use these shared
> > > lines as GPIOs or nothing, so no DT changes would be required.
> > 
> > Okay, that's useful (please add to the commit msg).
> 
> I added the information in the cover letter, but I can add it to the
> actual patch commit message if you prefer.
> 
> > Still, what platform(s) need the modem control feature? Presumably
> > that's whatever platform Lech and Tomasz work on. I guess given the
> > Reviewed-by they are fine with needing a DT change.
> 
> Ok. I have previously also emailed Lech about that, but he has not
> responded yet.

Hi Rob,
Lech just confirmed that he made the DT changes
to a board that was used only internally, and that no other DT changes
should be necessary for in-tree DTS.

Hugo.
diff mbox series

Patch

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 5b0aeef9d534..bc0a288f258d 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -236,7 +236,8 @@ 
 
 /* IOControl register bits (Only 750/760) */
 #define SC16IS7XX_IOCONTROL_LATCH_BIT	(1 << 0) /* Enable input latching */
-#define SC16IS7XX_IOCONTROL_MODEM_BIT	(1 << 1) /* Enable GPIO[7:4] as modem pins */
+#define SC16IS7XX_IOCONTROL_MODEM_A_BIT	(1 << 1) /* Enable GPIO[7:4] as modem A pins */
+#define SC16IS7XX_IOCONTROL_MODEM_B_BIT	(1 << 2) /* Enable GPIO[3:0] as modem B pins */
 #define SC16IS7XX_IOCONTROL_SRESET_BIT	(1 << 3) /* Software Reset */
 
 /* EFCR register bits */
@@ -301,12 +302,12 @@ 
 /* Misc definitions */
 #define SC16IS7XX_FIFO_SIZE		(64)
 #define SC16IS7XX_REG_SHIFT		2
+#define SC16IS7XX_GPIOS_PER_BANK	4
 
 struct sc16is7xx_devtype {
 	char	name[10];
 	int	nr_gpio;
 	int	nr_uart;
-	int	has_mctrl;
 };
 
 #define SC16IS7XX_RECONF_MD		(1 << 0)
@@ -337,7 +338,9 @@  struct sc16is7xx_port {
 	struct clk			*clk;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip		gpio;
+	unsigned long			gpio_valid_mask;
 #endif
+	u8				mctrl_mask;
 	unsigned char			buf[SC16IS7XX_FIFO_SIZE];
 	struct kthread_worker		kworker;
 	struct task_struct		*kworker_task;
@@ -448,35 +451,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)
@@ -1351,14 +1349,43 @@  static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
 	return 0;
 }
 
+static int sc16is7xx_gpio_init_valid_mask(struct gpio_chip *chip,
+					  unsigned long *valid_mask,
+					  unsigned int ngpios)
+{
+	struct sc16is7xx_port *s = gpiochip_get_data(chip);
+
+	*valid_mask = s->gpio_valid_mask;
+
+	return 0;
+}
+
 static int sc16is7xx_setup_gpio_chip(struct sc16is7xx_port *s)
 {
 	if (!s->devtype->nr_gpio)
 		return 0;
 
+	switch (s->mctrl_mask) {
+	case 0:
+		s->gpio_valid_mask = GENMASK(7, 0);
+		break;
+	case SC16IS7XX_IOCONTROL_MODEM_A_BIT:
+		s->gpio_valid_mask = GENMASK(3, 0);
+		break;
+	case SC16IS7XX_IOCONTROL_MODEM_B_BIT:
+		s->gpio_valid_mask = GENMASK(7, 4);
+		break;
+	default:
+		break;
+	}
+
+	if (s->gpio_valid_mask == 0)
+		return 0;
+
 	s->gpio.owner		 = THIS_MODULE;
 	s->gpio.parent		 = s->dev;
 	s->gpio.label		 = dev_name(s->dev);
+	s->gpio.init_valid_mask	 = sc16is7xx_gpio_init_valid_mask;
 	s->gpio.direction_input	 = sc16is7xx_gpio_direction_input;
 	s->gpio.get		 = sc16is7xx_gpio_get;
 	s->gpio.direction_output = sc16is7xx_gpio_direction_output;
@@ -1371,6 +1398,47 @@  static int sc16is7xx_setup_gpio_chip(struct sc16is7xx_port *s)
 }
 #endif
 
+/*
+ * Configure ports designated to operate as modem control lines.
+ */
+static int sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s)
+{
+	int i;
+	int ret;
+	int count;
+	u32 mctrl_port[2];
+
+	count = device_property_count_u32(s->dev,
+					  "nxp,modem-control-line-ports");
+	if (count < 0 || count > ARRAY_SIZE(mctrl_port))
+		return 0;
+
+	ret = device_property_read_u32_array(s->dev,
+					     "nxp,modem-control-line-ports",
+					     mctrl_port, count);
+	if (ret)
+		return ret;
+
+	s->mctrl_mask = 0;
+
+	for (i = 0; i < count; i++) {
+		/* Use GPIO lines as modem control lines */
+		if (mctrl_port[i] == 0)
+			s->mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_A_BIT;
+		else if (mctrl_port[i] == 1)
+			s->mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_B_BIT;
+	}
+
+	if (s->mctrl_mask)
+		regmap_update_bits(
+			s->regmap,
+			SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
+			SC16IS7XX_IOCONTROL_MODEM_A_BIT |
+			SC16IS7XX_IOCONTROL_MODEM_B_BIT, s->mctrl_mask);
+
+	return 0;
+}
+
 static const struct serial_rs485 sc16is7xx_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND,
 	.delay_rts_before_send = 1,
@@ -1479,12 +1547,6 @@  static int sc16is7xx_probe(struct device *dev,
 				     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);
-
 		/* Initialize kthread work structs */
 		kthread_init_work(&s->p[i].tx_work, sc16is7xx_tx_proc);
 		kthread_init_work(&s->p[i].reg_work, sc16is7xx_reg_proc);
@@ -1522,6 +1584,10 @@  static int sc16is7xx_probe(struct device *dev,
 				s->p[u].irda_mode = true;
 	}
 
+	ret = sc16is7xx_setup_mctrl_ports(s);
+	if (ret)
+		goto out_ports;
+
 #ifdef CONFIG_GPIOLIB
 	ret = sc16is7xx_setup_gpio_chip(s);
 	if (ret)
@@ -1548,7 +1614,7 @@  static int sc16is7xx_probe(struct device *dev,
 		return 0;
 
 #ifdef CONFIG_GPIOLIB
-	if (devtype->nr_gpio)
+	if (s->gpio_valid_mask)
 		gpiochip_remove(&s->gpio);
 #endif
 
@@ -1572,7 +1638,7 @@  static void sc16is7xx_remove(struct device *dev)
 	int i;
 
 #ifdef CONFIG_GPIOLIB
-	if (s->devtype->nr_gpio)
+	if (s->gpio_valid_mask)
 		gpiochip_remove(&s->gpio);
 #endif