[PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Sherry Sun]

Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need to
increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
initialization failures.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 57b0632a3db6..7cb1e36fdaab 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -245,7 +245,7 @@
 
 #define DRIVER_NAME	"fsl-lpuart"
 #define DEV_NAME	"ttyLP"
-#define UART_NR		8
+#define UART_NR		12
 
 /* IMX lpuart has four extra unused regs located at the beginning */
 #define IMX_REG_OFF	0x10
-- 
2.34.1

Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Greg KH]

On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need to
> increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
> initialization failures.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 57b0632a3db6..7cb1e36fdaab 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -245,7 +245,7 @@
>  
>  #define DRIVER_NAME	"fsl-lpuart"
>  #define DEV_NAME	"ttyLP"
> -#define UART_NR		8
> +#define UART_NR		12

Why not fix this properly and make this dynamic and get rid of the
static array causing all of this problem?  That way when you get a
system with 13 uarts, you will be ok :)

thanks,

greg k-h

RE: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Sherry Sun]

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, January 3, 2025 5:08 PM
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev
> Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12
> 
> On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> > Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need to
> > increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
> > initialization failures.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >  drivers/tty/serial/fsl_lpuart.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > b/drivers/tty/serial/fsl_lpuart.c index 57b0632a3db6..7cb1e36fdaab
> > 100644
> > --- a/drivers/tty/serial/fsl_lpuart.c
> > +++ b/drivers/tty/serial/fsl_lpuart.c
> > @@ -245,7 +245,7 @@
> >
> >  #define DRIVER_NAME	"fsl-lpuart"
> >  #define DEV_NAME	"ttyLP"
> > -#define UART_NR		8
> > +#define UART_NR		12
> 
> Why not fix this properly and make this dynamic and get rid of the static array
> causing all of this problem?  That way when you get a system with 13 uarts,
> you will be ok :)
> 

Hi Greg,

Thanks for your comment.
But I checked all the uart drivers under drivers/tty/serial/, UART_NR is widely used, currently almost every uart driver that supports multiple uart ports defines this macro, this value is needed for the nr parameter of struct uart_driver, also for console index checking and setup.
This patch just refers to many other uart driver patches to extend maximum uart number, such as https://lore.kernel.org/all/20240112095300.2004878-3-valentin.caron@foss.st.com/.
Agree that it will be nice to dynamically allocate everything, but for now I prefer to simply change this value as there doesn't seem to be a good uart implementation at the moment, not sure what you prefer?  :)

Best Regards
Sherry

Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Greg KH]

On Mon, Jan 06, 2025 at 10:24:52AM +0000, Sherry Sun wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Friday, January 3, 2025 5:08 PM
> > To: Sherry Sun <sherry.sun@nxp.com>
> > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> > kernel@vger.kernel.org; imx@lists.linux.dev
> > Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12
> > 
> > On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> > > Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need to
> > > increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
> > > initialization failures.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > ---
> > >  drivers/tty/serial/fsl_lpuart.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > b/drivers/tty/serial/fsl_lpuart.c index 57b0632a3db6..7cb1e36fdaab
> > > 100644
> > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > @@ -245,7 +245,7 @@
> > >
> > >  #define DRIVER_NAME	"fsl-lpuart"
> > >  #define DEV_NAME	"ttyLP"
> > > -#define UART_NR		8
> > > +#define UART_NR		12
> > 
> > Why not fix this properly and make this dynamic and get rid of the static array
> > causing all of this problem?  That way when you get a system with 13 uarts,
> > you will be ok :)
> > 
> 
> Hi Greg,
> 
> Thanks for your comment.
> But I checked all the uart drivers under drivers/tty/serial/, UART_NR
> is widely used, currently almost every uart driver that supports
> multiple uart ports defines this macro, this value is needed for the
> nr parameter of struct uart_driver, also for console index checking
> and setup.

Yeah, it's messy, but it can be done (for example see all of the
usb-serial devices, we don't limit the number of those ports in the
system except to 256 I think.)

> This patch just refers to many other uart driver patches to extend
> maximum uart number, such as
> https://lore.kernel.org/all/20240112095300.2004878-3-valentin.caron@foss.st.com/.
> Agree that it will be nice to dynamically allocate everything, but for
> now I prefer to simply change this value as there doesn't seem to be a
> good uart implementation at the moment, not sure what you prefer?  :)

I'd prefer not hard-coding stuff like this but if it's going to be too
much of a pain I guess I'll take this for now...

thanks,

greg k-h

RE: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Sherry Sun]

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Monday, January 6, 2025 9:45 PM
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev
> Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12
>
> On Mon, Jan 06, 2025 at 10:24:52AM +0000, Sherry Sun wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH <gregkh@linuxfoundation.org>
> > > Sent: Friday, January 3, 2025 5:08 PM
> > > To: Sherry Sun <sherry.sun@nxp.com>
> > > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; imx@lists.linux.dev
> > > Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum
> > > uart_nr to 12
> > >
> > > On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> > > > Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need
> > > > to increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
> > > > initialization failures.
> > > >
> > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > ---
> > > >  drivers/tty/serial/fsl_lpuart.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > > b/drivers/tty/serial/fsl_lpuart.c index 57b0632a3db6..7cb1e36fdaab
> > > > 100644
> > > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > > @@ -245,7 +245,7 @@
> > > >
> > > >  #define DRIVER_NAME    "fsl-lpuart"
> > > >  #define DEV_NAME       "ttyLP"
> > > > -#define UART_NR                8
> > > > +#define UART_NR                12
> > >
> > > Why not fix this properly and make this dynamic and get rid of the
> > > static array causing all of this problem?  That way when you get a
> > > system with 13 uarts, you will be ok :)
> > >
> >
> > Hi Greg,
> >
> > Thanks for your comment.
> > But I checked all the uart drivers under drivers/tty/serial/, UART_NR
> > is widely used, currently almost every uart driver that supports
> > multiple uart ports defines this macro, this value is needed for the
> > nr parameter of struct uart_driver, also for console index checking
> > and setup.
>
> Yeah, it's messy, but it can be done (for example see all of the usb-serial
> devices, we don't limit the number of those ports in the system except to 256
> I think.)
>
> > This patch just refers to many other uart driver patches to extend
> > maximum uart number, such as
> >
> https://lore.ke/
> rnel.org%2Fall%2F20240112095300.2004878-3-
> valentin.caron%40foss.st.com%2F&data=05%7C02%7Csherry.sun%40nxp.co
> m%7C4c12dccfce2c44e0a21408dd2e584fba%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C638717679009907376%7CUnknown%7CTWFpbGZsb3
> d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkF
> OIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=uq3dYJI3va5uRu
> LcPkF8DxP0mMPcYBiflG%2Bc18TiC3U%3D&reserved=0.
> > Agree that it will be nice to dynamically allocate everything, but for
> > now I prefer to simply change this value as there doesn't seem to be a
> > good uart implementation at the moment, not sure what you prefer?  :)
>
> I'd prefer not hard-coding stuff like this but if it's going to be too much of a
> pain I guess I'll take this for now...
>

Yes, agree, at least for now change the old hard-coding stuff which are widely
used in uart seems painful, maybe we can try to improve this after further
research and discussion later.

Best Regards
Sherry

RE: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Sherry Sun]

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Monday, January 6, 2025 9:45 PM
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> kernel@vger.kernel.org; imx@lists.linux.dev
> Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12
> 
> On Mon, Jan 06, 2025 at 10:24:52AM +0000, Sherry Sun wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH <gregkh@linuxfoundation.org>
> > > Sent: Friday, January 3, 2025 5:08 PM
> > > To: Sherry Sun <sherry.sun@nxp.com>
> > > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; imx@lists.linux.dev
> > > Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum
> > > uart_nr to 12
> > >
> > > On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> > > > Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need
> > > > to increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
> > > > initialization failures.
> > > >
> > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > ---
> > > >  drivers/tty/serial/fsl_lpuart.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > > b/drivers/tty/serial/fsl_lpuart.c index 57b0632a3db6..7cb1e36fdaab
> > > > 100644
> > > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > > @@ -245,7 +245,7 @@
> > > >
> > > >  #define DRIVER_NAME	"fsl-lpuart"
> > > >  #define DEV_NAME	"ttyLP"
> > > > -#define UART_NR		8
> > > > +#define UART_NR		12
> > >
> > > Why not fix this properly and make this dynamic and get rid of the
> > > static array causing all of this problem?  That way when you get a
> > > system with 13 uarts, you will be ok :)
> > >
> >
> > Hi Greg,
> >
> > Thanks for your comment.
> > But I checked all the uart drivers under drivers/tty/serial/, UART_NR
> > is widely used, currently almost every uart driver that supports
> > multiple uart ports defines this macro, this value is needed for the
> > nr parameter of struct uart_driver, also for console index checking
> > and setup.
> 
> Yeah, it's messy, but it can be done (for example see all of the usb-serial
> devices, we don't limit the number of those ports in the system except to 256
> I think.)

BTW, it seems that usb-serial devices also have the max ports limit, you can
check the MAX_NUM_PORTS macro in include/linux/usb/serial.h, it was extended from 8 to 16 now.

/* The maximum number of ports one device can grab at once */
#define MAX_NUM_PORTS       16

Best Regards
Sherry

Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Greg KH]

On Tue, Jan 07, 2025 at 08:29:12AM +0000, Sherry Sun wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Monday, January 6, 2025 9:45 PM
> > To: Sherry Sun <sherry.sun@nxp.com>
> > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> > kernel@vger.kernel.org; imx@lists.linux.dev
> > Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12
> > 
> > On Mon, Jan 06, 2025 at 10:24:52AM +0000, Sherry Sun wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH <gregkh@linuxfoundation.org>
> > > > Sent: Friday, January 3, 2025 5:08 PM
> > > > To: Sherry Sun <sherry.sun@nxp.com>
> > > > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; imx@lists.linux.dev
> > > > Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum
> > > > uart_nr to 12
> > > >
> > > > On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> > > > > Some SoCs like the i.MX943 have aliases for up to 12 UARTs, need
> > > > > to increase UART_NR from 8 to 12 to support lpuart9-12 to avoid
> > > > > initialization failures.
> > > > >
> > > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > > ---
> > > > >  drivers/tty/serial/fsl_lpuart.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > > > b/drivers/tty/serial/fsl_lpuart.c index 57b0632a3db6..7cb1e36fdaab
> > > > > 100644
> > > > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > > > @@ -245,7 +245,7 @@
> > > > >
> > > > >  #define DRIVER_NAME	"fsl-lpuart"
> > > > >  #define DEV_NAME	"ttyLP"
> > > > > -#define UART_NR		8
> > > > > +#define UART_NR		12
> > > >
> > > > Why not fix this properly and make this dynamic and get rid of the
> > > > static array causing all of this problem?  That way when you get a
> > > > system with 13 uarts, you will be ok :)
> > > >
> > >
> > > Hi Greg,
> > >
> > > Thanks for your comment.
> > > But I checked all the uart drivers under drivers/tty/serial/, UART_NR
> > > is widely used, currently almost every uart driver that supports
> > > multiple uart ports defines this macro, this value is needed for the
> > > nr parameter of struct uart_driver, also for console index checking
> > > and setup.
> > 
> > Yeah, it's messy, but it can be done (for example see all of the usb-serial
> > devices, we don't limit the number of those ports in the system except to 256
> > I think.)
> 
> BTW, it seems that usb-serial devices also have the max ports limit, you can
> check the MAX_NUM_PORTS macro in include/linux/usb/serial.h, it was extended from 8 to 16 now.
> 
> /* The maximum number of ports one device can grab at once */
> #define MAX_NUM_PORTS       16

That's different, that is "max devices a single device can use".  We
don't know of any single-usb-device that has more than 16 ports on it,
do you?  I've seen big ones, but internally they are all split up into
smaller USB devices in order to handle the bandwidth properly.  And I
think even the 16 port devices are almost always really just 2 8-port
devices, or 4 4-port ones.

Now you can have a lot of 16-port devices in the system at the same
time, but I think we max out at 256.

Oops, nope, we now support 512 usb-serial ports in the system at one
time:
	#define USB_SERIAL_TTY_MINORS   512     /* should be enough for a while */

Well "now" is relative, that change was made in 2013 :)

The commit that bumped the number also gives a hint on how to make this
more dynamic, if you want to read the changelog text for commit
455b4f7e18e7 ("USB: serial: increase the number of devices we support")
for more details.

thanks,

greg k-h

RE: [PATCH] tty: serial: fsl_lpuart: increase maximum uart_nr to 12

[Sherry Sun]

> > > > > -----Original Message-----
> > > > > From: Greg KH <gregkh@linuxfoundation.org>
> > > > > Sent: Friday, January 3, 2025 5:08 PM
> > > > > To: Sherry Sun <sherry.sun@nxp.com>
> > > > > Cc: jirislaby@kernel.org; linux-serial@vger.kernel.org; linux-
> > > > > kernel@vger.kernel.org; imx@lists.linux.dev
> > > > > Subject: Re: [PATCH] tty: serial: fsl_lpuart: increase maximum
> > > > > uart_nr to 12
> > > > >
> > > > > On Fri, Jan 03, 2025 at 03:11:54PM +0800, Sherry Sun wrote:
> > > > > > Some SoCs like the i.MX943 have aliases for up to 12 UARTs,
> > > > > > need to increase UART_NR from 8 to 12 to support lpuart9-12 to
> > > > > > avoid initialization failures.
> > > > > >
> > > > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > > > > ---
> > > > > >  drivers/tty/serial/fsl_lpuart.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > > > > > b/drivers/tty/serial/fsl_lpuart.c index
> > > > > > 57b0632a3db6..7cb1e36fdaab
> > > > > > 100644
> > > > > > --- a/drivers/tty/serial/fsl_lpuart.c
> > > > > > +++ b/drivers/tty/serial/fsl_lpuart.c
> > > > > > @@ -245,7 +245,7 @@
> > > > > >
> > > > > >  #define DRIVER_NAME	"fsl-lpuart"
> > > > > >  #define DEV_NAME	"ttyLP"
> > > > > > -#define UART_NR		8
> > > > > > +#define UART_NR		12
> > > > >
> > > > > Why not fix this properly and make this dynamic and get rid of
> > > > > the static array causing all of this problem?  That way when you
> > > > > get a system with 13 uarts, you will be ok :)
> > > > >
> > > >
> > > > Hi Greg,
> > > >
> > > > Thanks for your comment.
> > > > But I checked all the uart drivers under drivers/tty/serial/,
> > > > UART_NR is widely used, currently almost every uart driver that
> > > > supports multiple uart ports defines this macro, this value is
> > > > needed for the nr parameter of struct uart_driver, also for
> > > > console index checking and setup.
> > >
> > > Yeah, it's messy, but it can be done (for example see all of the
> > > usb-serial devices, we don't limit the number of those ports in the
> > > system except to 256 I think.)
> >
> > BTW, it seems that usb-serial devices also have the max ports limit,
> > you can check the MAX_NUM_PORTS macro in include/linux/usb/serial.h, it
> was extended from 8 to 16 now.
> >
> > /* The maximum number of ports one device can grab at once */
> > #define MAX_NUM_PORTS       16
> 
> That's different, that is "max devices a single device can use".  We don't know
> of any single-usb-device that has more than 16 ports on it, do you?  I've seen
> big ones, but internally they are all split up into smaller USB devices in order
> to handle the bandwidth properly.  And I think even the 16 port devices are
> almost always really just 2 8-port devices, or 4 4-port ones.
> 
> Now you can have a lot of 16-port devices in the system at the same time, but
> I think we max out at 256.
> 
> Oops, nope, we now support 512 usb-serial ports in the system at one
> time:
> 	#define USB_SERIAL_TTY_MINORS   512     /* should be enough for a
> while */
> 
> Well "now" is relative, that change was made in 2013 :)
> 
> The commit that bumped the number also gives a hint on how to make this
> more dynamic, if you want to read the changelog text for commit
> 455b4f7e18e7 ("USB: serial: increase the number of devices we support") for
> more details.
> 

Hi Greg,
Thanks for the detailed explanation, yes it is clear to me now, anyway the next
step may needs to remove the limitation of both normal uart and usb serial,
dynamic configuration is our goal.

Best Regards
Sherry