linux-kernel - Re: [PATCH v1 4/4] serial: exar: change port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240419081727.435437d0@SWDEV2.connecttech.local>
Date: Fri, 19 Apr 2024 08:17:27 -0400
From: Parker Newman <parker@...est.io>
To: Jiri Slaby <jirislaby@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org, Parker Newman
 <pnewman@...necttech.com>, Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v1 4/4] serial: exar: change port_type ternary line
 wrapping

On Fri, 19 Apr 2024 08:07:50 +0200
Jiri Slaby <jirislaby@...nel.org> wrote:

> On 18. 04. 24, 17:36, Parker Newman wrote:
> > From: Parker Newman <pnewman@...necttech.com>
> >
> > Change line wrapping of ternary operators in
> > cti_get_port_type_xr17c15x_xr17v25x() for better readability.
> >
> > Old example:
> >
> > 	port_type = port_num == 0 ?
> > 			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> >
> > New:
> > 	port_type = port_num == 0 ? CTI_PORT_TYPE_RS232 :
> > 				CTI_PORT_TYPE_RS422_485;
>
> This is worse IMO. Ilpo suggested a bit different alignment. But still...
>
> > Based on feedback from:
> > Link: https://lore.kernel.org/linux-serial/f2353b8c-2079-b895-2707-f6be83161288@linux.intel.com
>
> You should have CCed the author.
>
> > Signed-off-by: Parker Newman <pnewman@...necttech.com>
> > ---
> >   drivers/tty/serial/8250/8250_exar.c | 20 ++++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> > index 10725ad0f3ef..a76b4e5bab4e 100644
> > --- a/drivers/tty/serial/8250/8250_exar.c
> > +++ b/drivers/tty/serial/8250/8250_exar.c
> > @@ -741,19 +741,19 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
> >   		break;
> >   	// 1x RS232, 1x RS422/RS485
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_1_1:
> > -		port_type = port_num == 0 ?
> > -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> > +		port_type = port_num == 0 ? CTI_PORT_TYPE_RS232 :
> > +					CTI_PORT_TYPE_RS422_485;
>
>
> Well, could you initialize port_type = CTI_PORT_TYPE_RS232? And here do
> only:
> if (port_num > 0)
>      return CTI_PORT_TYPE_RS422_485;
> ?
>

I like this idea I will move to that. Thanks.

>
> >   		break;
> >   	// 2x RS232, 2x RS422/RS485
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_2:
> > -		port_type = port_num < 2 ?
> > -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> > +		port_type = port_num < 2 ? CTI_PORT_TYPE_RS232 :
> > +					CTI_PORT_TYPE_RS422_485;
>
> And so on.
>
> >   		break;
> >   	// 4x RS232, 4x RS422/RS485
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4:
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP:
> > -		port_type = port_num < 4 ?
> > -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> > +		port_type = port_num < 4 ? CTI_PORT_TYPE_RS232 :
> > +					CTI_PORT_TYPE_RS422_485;
> >   		break;
> >   	// RS232/RS422/RS485 HW (jumper) selectable
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2:
> > @@ -789,13 +789,13 @@ static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x(struct exar8250 *p
> >   		break;
> >   	// 6x RS232, 2x RS422/RS485
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP:
> > -		port_type = port_num < 6 ?
> > -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> > +		port_type = port_num < 6 ? CTI_PORT_TYPE_RS232 :
> > +					CTI_PORT_TYPE_RS422_485;
> >   		break;
> >   	// 2x RS232, 6x RS422/RS485
> >   	case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP:
> > -		port_type = port_num < 2 ?
> > -			CTI_PORT_TYPE_RS232 : CTI_PORT_TYPE_RS422_485;
> > +		port_type = port_num < 2 ? CTI_PORT_TYPE_RS232 :
> > +					CTI_PORT_TYPE_RS422_485;
> >   		break;
> >   	default:
> >   		dev_err(&pcidev->dev, "unknown/unsupported device\n");
> > --
> > 2.43.2
> >
>