diff mbox series

[1/6] serial: 8250: remove AR7 support

Message ID 20230920201035.3445-2-wsa+renesas@sang-engineering.com (mailing list archive)
State Superseded
Headers show
Series remove AR7 platform and associated drivers | expand

Commit Message

Wolfram Sang Sept. 20, 2023, 8:10 p.m. UTC
AR7 is going to be removed from the Kernel, so remove its type
definition from 8250 code. As with previous removals, I checked with
Debian Code Search that 'PORT_AR7' is not used in userspace.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/tty/serial/8250/8250_port.c | 7 -------
 include/uapi/linux/serial_core.h    | 1 -
 2 files changed, 8 deletions(-)

Comments

Florian Fainelli Sept. 20, 2023, 9:25 p.m. UTC | #1
On 9/20/23 13:10, Wolfram Sang wrote:
> AR7 is going to be removed from the Kernel, so remove its type
> definition from 8250 code. As with previous removals, I checked with
> Debian Code Search that 'PORT_AR7' is not used in userspace.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Greg KH Sept. 21, 2023, 7:31 a.m. UTC | #2
On Wed, Sep 20, 2023 at 10:10:27PM +0200, Wolfram Sang wrote:
> AR7 is going to be removed from the Kernel, so remove its type
> definition from 8250 code. As with previous removals, I checked with
> Debian Code Search that 'PORT_AR7' is not used in userspace.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Philippe Mathieu-Daudé Sept. 21, 2023, 10:36 a.m. UTC | #3
Hi Wolfram,

(Adding Andy for commit 54b45ee8bd42 "serial: core: Remove
unused PORT_* definitions").

On 20/9/23 22:10, Wolfram Sang wrote:
> AR7 is going to be removed from the Kernel, so remove its type
> definition from 8250 code. As with previous removals, I checked with
> Debian Code Search that 'PORT_AR7' is not used in userspace.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>   drivers/tty/serial/8250/8250_port.c | 7 -------
>   include/uapi/linux/serial_core.h    | 1 -
>   2 files changed, 8 deletions(-)


> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index add349889d0a..3b51901926f9 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -32,7 +32,6 @@
>   #define PORT_XSCALE	15
>   #define PORT_RM9000	16	/* PMC-Sierra RM9xxx internal UART */
>   #define PORT_OCTEON	17	/* Cavium OCTEON internal UART */
> -#define PORT_AR7	18	/* Texas Instruments AR7 internal UART */

I'm a bit surprised definitions are removed from the uAPI, isn't
it expected to be very stable? Shouldn't it be better to keep it
defined but modify the comment, mentioning "obsolete" or "deprecated"?

Regards,

Phil.

>   #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
>   #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
>   #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
Andy Shevchenko Sept. 21, 2023, 11:16 a.m. UTC | #4
On Thu, Sep 21, 2023 at 12:36:05PM +0200, Philippe Mathieu-Daudé wrote:
> On 20/9/23 22:10, Wolfram Sang wrote:

> > --- a/include/uapi/linux/serial_core.h
> > +++ b/include/uapi/linux/serial_core.h
> > @@ -32,7 +32,6 @@
> >   #define PORT_XSCALE	15
> >   #define PORT_RM9000	16	/* PMC-Sierra RM9xxx internal UART */
> >   #define PORT_OCTEON	17	/* Cavium OCTEON internal UART */
> > -#define PORT_AR7	18	/* Texas Instruments AR7 internal UART */
> 
> I'm a bit surprised definitions are removed from the uAPI, isn't
> it expected to be very stable? Shouldn't it be better to keep it
> defined but modify the comment, mentioning "obsolete" or "deprecated"?

The numbers up to 20 must stay, they are being used somewhere, setserial
implementation in busybox (IIRC).

NAK.
Andy Shevchenko Sept. 21, 2023, 11:18 a.m. UTC | #5
On Thu, Sep 21, 2023 at 12:36:05PM +0200, Philippe Mathieu-Daudé wrote:
> On 20/9/23 22:10, Wolfram Sang wrote:

> I'm a bit surprised definitions are removed from the uAPI, isn't
> it expected to be very stable?

They were added to uAPI by mistake to begin with. I don't know why people
still continue adding more there...

Basically we should stop to publish those numbers and move them into
internal header (except those 20 or so, that must stay).
Jiri Slaby Sept. 21, 2023, 11:21 a.m. UTC | #6
On 21. 09. 23, 13:16, Andy Shevchenko wrote:
> On Thu, Sep 21, 2023 at 12:36:05PM +0200, Philippe Mathieu-Daudé wrote:
>> On 20/9/23 22:10, Wolfram Sang wrote:
> 
>>> --- a/include/uapi/linux/serial_core.h
>>> +++ b/include/uapi/linux/serial_core.h
>>> @@ -32,7 +32,6 @@
>>>    #define PORT_XSCALE	15
>>>    #define PORT_RM9000	16	/* PMC-Sierra RM9xxx internal UART */
>>>    #define PORT_OCTEON	17	/* Cavium OCTEON internal UART */
>>> -#define PORT_AR7	18	/* Texas Instruments AR7 internal UART */
>>
>> I'm a bit surprised definitions are removed from the uAPI, isn't
>> it expected to be very stable? Shouldn't it be better to keep it
>> defined but modify the comment, mentioning "obsolete" or "deprecated"?
> 
> The numbers up to 20 must stay, they are being used somewhere, setserial
> implementation in busybox (IIRC).

But they define it if we don't:
#ifndef PORT_AR7
# define PORT_AR7               18
#endif

> NAK.
I don't mind either way. But likely we should reserve the field if we go 
and remove it (setserial has a number->string mapping in busybox). Hm, 
then reserving it or keep it? Perhaps keep it is better... So ack the 
NACK :).
Andy Shevchenko Sept. 21, 2023, 11:30 a.m. UTC | #7
On Thu, Sep 21, 2023 at 01:21:46PM +0200, Jiri Slaby wrote:
> On 21. 09. 23, 13:16, Andy Shevchenko wrote:
> > On Thu, Sep 21, 2023 at 12:36:05PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 20/9/23 22:10, Wolfram Sang wrote:

...

> > > > -#define PORT_AR7	18	/* Texas Instruments AR7 internal UART */
> > > 
> > > I'm a bit surprised definitions are removed from the uAPI, isn't
> > > it expected to be very stable? Shouldn't it be better to keep it
> > > defined but modify the comment, mentioning "obsolete" or "deprecated"?
> > 
> > The numbers up to 20 must stay, they are being used somewhere, setserial
> > implementation in busybox (IIRC).
> 
> But they define it if we don't:
> #ifndef PORT_AR7
> # define PORT_AR7               18
> #endif

Yep, but the problem is that we may not use that number anyway, because two
different versions of kernel can clash on the same version of tool that will
think about AR7 while it's something different. That's why instead of having
reserved space, better to leave with names assigned.

> > NAK.
> I don't mind either way. But likely we should reserve the field if we go and
> remove it (setserial has a number->string mapping in busybox). Hm, then
> reserving it or keep it? Perhaps keep it is better... So ack the NACK :).
Wolfram Sang Sept. 21, 2023, 12:04 p.m. UTC | #8
> I don't mind either way. But likely we should reserve the field if we go and
> remove it (setserial has a number->string mapping in busybox). Hm, then
> reserving it or keep it? Perhaps keep it is better... So ack the NACK :).

Okay. I will resend with this number kept.

Thanks everyone for the input!
diff mbox series

Patch

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index fb891b67968f..a4f5d792679a 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -170,13 +170,6 @@  static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO,
 	},
-	[PORT_AR7] = {
-		.name		= "AR7",
-		.fifo_size	= 16,
-		.tx_loadsz	= 16,
-		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_00,
-		.flags		= UART_CAP_FIFO /* | UART_CAP_AFE */,
-	},
 	[PORT_U6_16550A] = {
 		.name		= "U6_16550A",
 		.fifo_size	= 64,
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index add349889d0a..3b51901926f9 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -32,7 +32,6 @@ 
 #define PORT_XSCALE	15
 #define PORT_RM9000	16	/* PMC-Sierra RM9xxx internal UART */
 #define PORT_OCTEON	17	/* Cavium OCTEON internal UART */
-#define PORT_AR7	18	/* Texas Instruments AR7 internal UART */
 #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
 #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
 #define PORT_XR17D15X	21	/* Exar XR17D15x UART */