| Message ID | 20230807080726.7190-1-Wenhua.Lin@unisoc.com |
|---|---|
| State | New |
| Headers | show |
| Series | serial: sprd: Support 12 uart ports | expand |
The change of the max ports is to adapt to an new SPRD serial hardware,also is backward-compatibility with old hardware. Baolin Wang <baolin.wang@linux.alibaba.com> 于2023年8月7日周一 16:41写道： > > > > On 8/7/2023 4:07 PM, Wenhua Lin wrote: > > From: Wenhua Lin <wenhua.lin@unisoc.com> > > > > Support 12 uart ports, which can solve the problem > > of insufficient uart ports. > > This sounds incorrect to me. > If this is a software bug, please add a Fixes tag to backport for stable > kernels. If the change of the max ports is only avaliable on the new > SPRD serial hardware, you should ensure the change is > backward-compatibility with old hardware. > > > Signed-off-by: Wenhua Lin <wenhua.lin@unisoc.com> > > --- > > drivers/tty/serial/sprd_serial.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c > > index b58f51296ace..2774df490899 100644 > > --- a/drivers/tty/serial/sprd_serial.c > > +++ b/drivers/tty/serial/sprd_serial.c > > @@ -22,7 +22,7 @@ > > #include <linux/tty_flip.h> > > > > /* device name */ > > -#define UART_NR_MAX 8 > > +#define UART_NR_MAX 12 > > #define SPRD_TTY_NAME "ttyS" > > #define SPRD_FIFO_SIZE 128 > > #define SPRD_DEF_RATE 26000000
On Wed, 9 Aug 2023 at 13:52, wenhua lin <wenhua.lin1994@gmail.com> wrote: > > The change of the max ports is to adapt to an new > SPRD serial hardware,also is backward-compatibility with old hardware. This change seems not backward-compatible if the serial devices on some old Unisoc SoCs can support 8 ports only. BTW, please do not top-post your reply. Thanks, Chunyan > > Baolin Wang <baolin.wang@linux.alibaba.com> 于2023年8月7日周一 16:41写道： > > > > > > > > On 8/7/2023 4:07 PM, Wenhua Lin wrote: > > > From: Wenhua Lin <wenhua.lin@unisoc.com> > > > > > > Support 12 uart ports, which can solve the problem > > > of insufficient uart ports. > > > > This sounds incorrect to me. > > If this is a software bug, please add a Fixes tag to backport for stable > > kernels. If the change of the max ports is only avaliable on the new > > SPRD serial hardware, you should ensure the change is > > backward-compatibility with old hardware. > > > > > Signed-off-by: Wenhua Lin <wenhua.lin@unisoc.com> > > > --- > > > drivers/tty/serial/sprd_serial.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c > > > index b58f51296ace..2774df490899 100644 > > > --- a/drivers/tty/serial/sprd_serial.c > > > +++ b/drivers/tty/serial/sprd_serial.c > > > @@ -22,7 +22,7 @@ > > > #include <linux/tty_flip.h> > > > > > > /* device name */ > > > -#define UART_NR_MAX 8 > > > +#define UART_NR_MAX 12 > > > #define SPRD_TTY_NAME "ttyS" > > > #define SPRD_FIFO_SIZE 128 > > > #define SPRD_DEF_RATE 26000000
On 8/9/2023 1:52 PM, wenhua lin wrote: > The change of the max ports is to adapt to an new > SPRD serial hardware,also is backward-compatibility with old hardware. Again, DO NOT top-posting! Thanks. >> >> On 8/7/2023 4:07 PM, Wenhua Lin wrote: >>> From: Wenhua Lin <wenhua.lin@unisoc.com> >>> >>> Support 12 uart ports, which can solve the problem >>> of insufficient uart ports. >> >> This sounds incorrect to me. >> If this is a software bug, please add a Fixes tag to backport for stable >> kernels. If the change of the max ports is only avaliable on the new >> SPRD serial hardware, you should ensure the change is >> backward-compatibility with old hardware. This is not backward-compatibility. Please see 'sprd_uart_driver' structure as below: static struct uart_driver sprd_uart_driver = { .owner = THIS_MODULE, .driver_name = "sprd_serial", .dev_name = SPRD_TTY_NAME, .major = 0, .minor = 0, .nr = UART_NR_MAX, .cons = SPRD_CONSOLE, }; That means you will still register 12 UART ports in the tty for old serial hardwares which actually only support 8 UART ports, and if users want to use another 4 ports which are not supported in the hardware, then system crashes. So I think you should add a SoC data in 'serial_ids' to change the number of UART port according to different SoC. >>> Signed-off-by: Wenhua Lin <wenhua.lin@unisoc.com> >>> --- >>> drivers/tty/serial/sprd_serial.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c >>> index b58f51296ace..2774df490899 100644 >>> --- a/drivers/tty/serial/sprd_serial.c >>> +++ b/drivers/tty/serial/sprd_serial.c >>> @@ -22,7 +22,7 @@ >>> #include <linux/tty_flip.h> >>> >>> /* device name */ >>> -#define UART_NR_MAX 8 >>> +#define UART_NR_MAX 12 >>> #define SPRD_TTY_NAME "ttyS" >>> #define SPRD_FIFO_SIZE 128 >>> #define SPRD_DEF_RATE 26000000
On Wed, Aug 9, 2023 at 2:27 PM Baolin Wang <baolin.wang@linux.alibaba.com> wrote: > > > > On 8/9/2023 1:52 PM, wenhua lin wrote: > > The change of the max ports is to adapt to an new > > SPRD serial hardware,also is backward-compatibility with old hardware. > > Again, DO NOT top-posting! Thanks. Thank you for your correction. > > >> > >> On 8/7/2023 4:07 PM, Wenhua Lin wrote: > >>> From: Wenhua Lin <wenhua.lin@unisoc.com> > >>> > >>> Support 12 uart ports, which can solve the problem > >>> of insufficient uart ports. > >> > >> This sounds incorrect to me. > >> If this is a software bug, please add a Fixes tag to backport for stable > >> kernels. If the change of the max ports is only avaliable on the new > >> SPRD serial hardware, you should ensure the change is > >> backward-compatibility with old hardware. > > This is not backward-compatibility. Please see 'sprd_uart_driver' > structure as below: > > static struct uart_driver sprd_uart_driver = { > .owner = THIS_MODULE, > .driver_name = "sprd_serial", > .dev_name = SPRD_TTY_NAME, > .major = 0, > .minor = 0, > .nr = UART_NR_MAX, > .cons = SPRD_CONSOLE, > }; > > That means you will still register 12 UART ports in the tty for old > serial hardwares which actually only support 8 UART ports, and if users > want to use another 4 ports which are not supported in the hardware, > then system crashes. > > So I think you should add a SoC data in 'serial_ids' to change the > number of UART port according to different SoC. > Thank you for your valuable comments, we should probably consider how to make changes based on your suggestions. > >>> Signed-off-by: Wenhua Lin <wenhua.lin@unisoc.com> > >>> --- > >>> drivers/tty/serial/sprd_serial.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c > >>> index b58f51296ace..2774df490899 100644 > >>> --- a/drivers/tty/serial/sprd_serial.c > >>> +++ b/drivers/tty/serial/sprd_serial.c > >>> @@ -22,7 +22,7 @@ > >>> #include <linux/tty_flip.h> > >>> > >>> /* device name */ > >>> -#define UART_NR_MAX 8 > >>> +#define UART_NR_MAX 12 > >>> #define SPRD_TTY_NAME "ttyS" > >>> #define SPRD_FIFO_SIZE 128 > >>> #define SPRD_DEF_RATE 26000000
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index b58f51296ace..2774df490899 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -22,7 +22,7 @@ #include <linux/tty_flip.h> /* device name */ -#define UART_NR_MAX 8 +#define UART_NR_MAX 12 #define SPRD_TTY_NAME "ttyS" #define SPRD_FIFO_SIZE 128 #define SPRD_DEF_RATE 26000000