linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers
@ 2016-07-26 19:50 Jan Kiszka
  2016-07-26 20:14 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2016-07-26 19:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linux-serial; +Cc: Linux Kernel Mailing List

Less magics that only require comments.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/tty/serial/8250/8250_pci.c | 42 +++++++++++++++++++-------------------
 include/uapi/linux/serial_reg.h    | 13 ++++++++++++
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index c1d4a8f..7f77060 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1796,18 +1796,18 @@ pci_xr17v35x_setup(struct serial_private *priv,
 	 * Setup Multipurpose Input/Output pins.
 	 */
 	if (idx == 0) {
-		writeb(0x00, p + 0x8f); /*MPIOINT[7:0]*/
-		writeb(0x00, p + 0x90); /*MPIOLVL[7:0]*/
-		writeb(0x00, p + 0x91); /*MPIO3T[7:0]*/
-		writeb(0x00, p + 0x92); /*MPIOINV[7:0]*/
-		writeb(0x00, p + 0x93); /*MPIOSEL[7:0]*/
-		writeb(0x00, p + 0x94); /*MPIOOD[7:0]*/
-		writeb(0x00, p + 0x95); /*MPIOINT[15:8]*/
-		writeb(0x00, p + 0x96); /*MPIOLVL[15:8]*/
-		writeb(0x00, p + 0x97); /*MPIO3T[15:8]*/
-		writeb(0x00, p + 0x98); /*MPIOINV[15:8]*/
-		writeb(0x00, p + 0x99); /*MPIOSEL[15:8]*/
-		writeb(0x00, p + 0x9a); /*MPIOOD[15:8]*/
+		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
+		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
+		writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
 	}
 	writeb(0x00, p + UART_EXAR_8XMODE);
 	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
@@ -1843,20 +1843,20 @@ pci_fastcom335_setup(struct serial_private *priv,
 		switch (priv->dev->device) {
 		case PCI_DEVICE_ID_COMMTECH_4222PCI335:
 		case PCI_DEVICE_ID_COMMTECH_4224PCI335:
-			writeb(0x78, p + 0x90); /* MPIOLVL[7:0] */
-			writeb(0x00, p + 0x92); /* MPIOINV[7:0] */
-			writeb(0x00, p + 0x93); /* MPIOSEL[7:0] */
+			writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
+			writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
+			writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
 			break;
 		case PCI_DEVICE_ID_COMMTECH_2324PCI335:
 		case PCI_DEVICE_ID_COMMTECH_2328PCI335:
-			writeb(0x00, p + 0x90); /* MPIOLVL[7:0] */
-			writeb(0xc0, p + 0x92); /* MPIOINV[7:0] */
-			writeb(0xc0, p + 0x93); /* MPIOSEL[7:0] */
+			writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
+			writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
+			writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
 			break;
 		}
-		writeb(0x00, p + 0x8f); /* MPIOINT[7:0] */
-		writeb(0x00, p + 0x91); /* MPIO3T[7:0] */
-		writeb(0x00, p + 0x94); /* MPIOOD[7:0] */
+		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
+		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
 	}
 	writeb(0x00, p + UART_EXAR_8XMODE);
 	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 1e5ac4e7..d176448 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -376,5 +376,18 @@
 #define UART_EXAR_TXTRG		0x0a	/* Tx FIFO trigger level write-only */
 #define UART_EXAR_RXTRG		0x0b	/* Rx FIFO trigger level write-only */
 
+#define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
+#define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
+#define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
+#define UART_EXAR_MPIOINV_7_0	0x92	/* MPIOINV[7:0] */
+#define UART_EXAR_MPIOSEL_7_0	0x93	/* MPIOSEL[7:0] */
+#define UART_EXAR_MPIOOD_7_0	0x94	/* MPIOOD[7:0] */
+#define UART_EXAR_MPIOINT_15_8	0x95	/* MPIOINT[15:8] */
+#define UART_EXAR_MPIOLVL_15_8	0x96	/* MPIOLVL[15:8] */
+#define UART_EXAR_MPIO3T_15_8	0x97	/* MPIO3T[15:8] */
+#define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
+#define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
+#define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
+
 #endif /* _LINUX_SERIAL_REG_H */
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers
  2016-07-26 19:50 [PATCH] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers Jan Kiszka
@ 2016-07-26 20:14 ` Greg Kroah-Hartman
  2016-07-26 20:44   ` Jan Kiszka
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2016-07-26 20:14 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: linux-serial, Linux Kernel Mailing List

On Tue, Jul 26, 2016 at 09:50:37PM +0200, Jan Kiszka wrote:
> Less magics that only require comments.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/tty/serial/8250/8250_pci.c | 42 +++++++++++++++++++-------------------
>  include/uapi/linux/serial_reg.h    | 13 ++++++++++++
>  2 files changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index c1d4a8f..7f77060 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1796,18 +1796,18 @@ pci_xr17v35x_setup(struct serial_private *priv,
>  	 * Setup Multipurpose Input/Output pins.
>  	 */
>  	if (idx == 0) {
> -		writeb(0x00, p + 0x8f); /*MPIOINT[7:0]*/
> -		writeb(0x00, p + 0x90); /*MPIOLVL[7:0]*/
> -		writeb(0x00, p + 0x91); /*MPIO3T[7:0]*/
> -		writeb(0x00, p + 0x92); /*MPIOINV[7:0]*/
> -		writeb(0x00, p + 0x93); /*MPIOSEL[7:0]*/
> -		writeb(0x00, p + 0x94); /*MPIOOD[7:0]*/
> -		writeb(0x00, p + 0x95); /*MPIOINT[15:8]*/
> -		writeb(0x00, p + 0x96); /*MPIOLVL[15:8]*/
> -		writeb(0x00, p + 0x97); /*MPIO3T[15:8]*/
> -		writeb(0x00, p + 0x98); /*MPIOINV[15:8]*/
> -		writeb(0x00, p + 0x99); /*MPIOSEL[15:8]*/
> -		writeb(0x00, p + 0x9a); /*MPIOOD[15:8]*/
> +		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
> +		writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
> +		writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
> +		writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
> +		writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
> +		writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
>  	}
>  	writeb(0x00, p + UART_EXAR_8XMODE);
>  	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
> @@ -1843,20 +1843,20 @@ pci_fastcom335_setup(struct serial_private *priv,
>  		switch (priv->dev->device) {
>  		case PCI_DEVICE_ID_COMMTECH_4222PCI335:
>  		case PCI_DEVICE_ID_COMMTECH_4224PCI335:
> -			writeb(0x78, p + 0x90); /* MPIOLVL[7:0] */
> -			writeb(0x00, p + 0x92); /* MPIOINV[7:0] */
> -			writeb(0x00, p + 0x93); /* MPIOSEL[7:0] */
> +			writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
> +			writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
> +			writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
>  			break;
>  		case PCI_DEVICE_ID_COMMTECH_2324PCI335:
>  		case PCI_DEVICE_ID_COMMTECH_2328PCI335:
> -			writeb(0x00, p + 0x90); /* MPIOLVL[7:0] */
> -			writeb(0xc0, p + 0x92); /* MPIOINV[7:0] */
> -			writeb(0xc0, p + 0x93); /* MPIOSEL[7:0] */
> +			writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
> +			writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
> +			writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
>  			break;
>  		}
> -		writeb(0x00, p + 0x8f); /* MPIOINT[7:0] */
> -		writeb(0x00, p + 0x91); /* MPIO3T[7:0] */
> -		writeb(0x00, p + 0x94); /* MPIOOD[7:0] */
> +		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
> +		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
>  	}
>  	writeb(0x00, p + UART_EXAR_8XMODE);
>  	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
> diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
> index 1e5ac4e7..d176448 100644
> --- a/include/uapi/linux/serial_reg.h
> +++ b/include/uapi/linux/serial_reg.h
> @@ -376,5 +376,18 @@
>  #define UART_EXAR_TXTRG		0x0a	/* Tx FIFO trigger level write-only */
>  #define UART_EXAR_RXTRG		0x0b	/* Rx FIFO trigger level write-only */
>  
> +#define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
> +#define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
> +#define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
> +#define UART_EXAR_MPIOINV_7_0	0x92	/* MPIOINV[7:0] */
> +#define UART_EXAR_MPIOSEL_7_0	0x93	/* MPIOSEL[7:0] */
> +#define UART_EXAR_MPIOOD_7_0	0x94	/* MPIOOD[7:0] */
> +#define UART_EXAR_MPIOINT_15_8	0x95	/* MPIOINT[15:8] */
> +#define UART_EXAR_MPIOLVL_15_8	0x96	/* MPIOLVL[15:8] */
> +#define UART_EXAR_MPIO3T_15_8	0x97	/* MPIO3T[15:8] */
> +#define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
> +#define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
> +#define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
> +

Why are you putting these in a user api file?  I know it matches the
existing ones, but is this something that userspace really cares about?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers
  2016-07-26 20:14 ` Greg Kroah-Hartman
@ 2016-07-26 20:44   ` Jan Kiszka
  2016-08-31 13:58     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2016-07-26 20:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-serial, Linux Kernel Mailing List

On 2016-07-26 22:14, Greg Kroah-Hartman wrote:
> On Tue, Jul 26, 2016 at 09:50:37PM +0200, Jan Kiszka wrote:
>> Less magics that only require comments.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  drivers/tty/serial/8250/8250_pci.c | 42 +++++++++++++++++++-------------------
>>  include/uapi/linux/serial_reg.h    | 13 ++++++++++++
>>  2 files changed, 34 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
>> index c1d4a8f..7f77060 100644
>> --- a/drivers/tty/serial/8250/8250_pci.c
>> +++ b/drivers/tty/serial/8250/8250_pci.c
>> @@ -1796,18 +1796,18 @@ pci_xr17v35x_setup(struct serial_private *priv,
>>  	 * Setup Multipurpose Input/Output pins.
>>  	 */
>>  	if (idx == 0) {
>> -		writeb(0x00, p + 0x8f); /*MPIOINT[7:0]*/
>> -		writeb(0x00, p + 0x90); /*MPIOLVL[7:0]*/
>> -		writeb(0x00, p + 0x91); /*MPIO3T[7:0]*/
>> -		writeb(0x00, p + 0x92); /*MPIOINV[7:0]*/
>> -		writeb(0x00, p + 0x93); /*MPIOSEL[7:0]*/
>> -		writeb(0x00, p + 0x94); /*MPIOOD[7:0]*/
>> -		writeb(0x00, p + 0x95); /*MPIOINT[15:8]*/
>> -		writeb(0x00, p + 0x96); /*MPIOLVL[15:8]*/
>> -		writeb(0x00, p + 0x97); /*MPIO3T[15:8]*/
>> -		writeb(0x00, p + 0x98); /*MPIOINV[15:8]*/
>> -		writeb(0x00, p + 0x99); /*MPIOSEL[15:8]*/
>> -		writeb(0x00, p + 0x9a); /*MPIOOD[15:8]*/
>> +		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
>> +		writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
>> +		writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
>> +		writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
>> +		writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
>> +		writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
>>  	}
>>  	writeb(0x00, p + UART_EXAR_8XMODE);
>>  	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
>> @@ -1843,20 +1843,20 @@ pci_fastcom335_setup(struct serial_private *priv,
>>  		switch (priv->dev->device) {
>>  		case PCI_DEVICE_ID_COMMTECH_4222PCI335:
>>  		case PCI_DEVICE_ID_COMMTECH_4224PCI335:
>> -			writeb(0x78, p + 0x90); /* MPIOLVL[7:0] */
>> -			writeb(0x00, p + 0x92); /* MPIOINV[7:0] */
>> -			writeb(0x00, p + 0x93); /* MPIOSEL[7:0] */
>> +			writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
>> +			writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
>> +			writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
>>  			break;
>>  		case PCI_DEVICE_ID_COMMTECH_2324PCI335:
>>  		case PCI_DEVICE_ID_COMMTECH_2328PCI335:
>> -			writeb(0x00, p + 0x90); /* MPIOLVL[7:0] */
>> -			writeb(0xc0, p + 0x92); /* MPIOINV[7:0] */
>> -			writeb(0xc0, p + 0x93); /* MPIOSEL[7:0] */
>> +			writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
>> +			writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
>> +			writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
>>  			break;
>>  		}
>> -		writeb(0x00, p + 0x8f); /* MPIOINT[7:0] */
>> -		writeb(0x00, p + 0x91); /* MPIO3T[7:0] */
>> -		writeb(0x00, p + 0x94); /* MPIOOD[7:0] */
>> +		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
>> +		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
>>  	}
>>  	writeb(0x00, p + UART_EXAR_8XMODE);
>>  	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
>> diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
>> index 1e5ac4e7..d176448 100644
>> --- a/include/uapi/linux/serial_reg.h
>> +++ b/include/uapi/linux/serial_reg.h
>> @@ -376,5 +376,18 @@
>>  #define UART_EXAR_TXTRG		0x0a	/* Tx FIFO trigger level write-only */
>>  #define UART_EXAR_RXTRG		0x0b	/* Rx FIFO trigger level write-only */
>>  
>> +#define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
>> +#define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
>> +#define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
>> +#define UART_EXAR_MPIOINV_7_0	0x92	/* MPIOINV[7:0] */
>> +#define UART_EXAR_MPIOSEL_7_0	0x93	/* MPIOSEL[7:0] */
>> +#define UART_EXAR_MPIOOD_7_0	0x94	/* MPIOOD[7:0] */
>> +#define UART_EXAR_MPIOINT_15_8	0x95	/* MPIOINT[15:8] */
>> +#define UART_EXAR_MPIOLVL_15_8	0x96	/* MPIOLVL[15:8] */
>> +#define UART_EXAR_MPIO3T_15_8	0x97	/* MPIO3T[15:8] */
>> +#define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
>> +#define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
>> +#define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
>> +
> 
> Why are you putting these in a user api file?  I know it matches the
> existing ones, but is this something that userspace really cares about?

Very valid question, and I was wondering the same. Maybe 95% of this
header is just lazy leftover from 607ca46e97.

I'm fine with starting off some include/linux/serial_reg.h, provided
someone gives me a hint what should be moved - I'm not very familiar
with the serial kernel ABI. Or I just start it off with those defines
above. You choose.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers
  2016-07-26 20:44   ` Jan Kiszka
@ 2016-08-31 13:58     ` Greg Kroah-Hartman
  2016-09-19  4:56       ` [PATCH v2] " Jan Kiszka
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2016-08-31 13:58 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: linux-serial, Linux Kernel Mailing List

On Tue, Jul 26, 2016 at 10:44:56PM +0200, Jan Kiszka wrote:
> On 2016-07-26 22:14, Greg Kroah-Hartman wrote:
> > On Tue, Jul 26, 2016 at 09:50:37PM +0200, Jan Kiszka wrote:
> >> Less magics that only require comments.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  drivers/tty/serial/8250/8250_pci.c | 42 +++++++++++++++++++-------------------
> >>  include/uapi/linux/serial_reg.h    | 13 ++++++++++++
> >>  2 files changed, 34 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> >> index c1d4a8f..7f77060 100644
> >> --- a/drivers/tty/serial/8250/8250_pci.c
> >> +++ b/drivers/tty/serial/8250/8250_pci.c
> >> @@ -1796,18 +1796,18 @@ pci_xr17v35x_setup(struct serial_private *priv,
> >>  	 * Setup Multipurpose Input/Output pins.
> >>  	 */
> >>  	if (idx == 0) {
> >> -		writeb(0x00, p + 0x8f); /*MPIOINT[7:0]*/
> >> -		writeb(0x00, p + 0x90); /*MPIOLVL[7:0]*/
> >> -		writeb(0x00, p + 0x91); /*MPIO3T[7:0]*/
> >> -		writeb(0x00, p + 0x92); /*MPIOINV[7:0]*/
> >> -		writeb(0x00, p + 0x93); /*MPIOSEL[7:0]*/
> >> -		writeb(0x00, p + 0x94); /*MPIOOD[7:0]*/
> >> -		writeb(0x00, p + 0x95); /*MPIOINT[15:8]*/
> >> -		writeb(0x00, p + 0x96); /*MPIOLVL[15:8]*/
> >> -		writeb(0x00, p + 0x97); /*MPIO3T[15:8]*/
> >> -		writeb(0x00, p + 0x98); /*MPIOINV[15:8]*/
> >> -		writeb(0x00, p + 0x99); /*MPIOSEL[15:8]*/
> >> -		writeb(0x00, p + 0x9a); /*MPIOOD[15:8]*/
> >> +		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
> >> +		writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
> >> +		writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
> >> +		writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
> >> +		writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
> >> +		writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
> >>  	}
> >>  	writeb(0x00, p + UART_EXAR_8XMODE);
> >>  	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
> >> @@ -1843,20 +1843,20 @@ pci_fastcom335_setup(struct serial_private *priv,
> >>  		switch (priv->dev->device) {
> >>  		case PCI_DEVICE_ID_COMMTECH_4222PCI335:
> >>  		case PCI_DEVICE_ID_COMMTECH_4224PCI335:
> >> -			writeb(0x78, p + 0x90); /* MPIOLVL[7:0] */
> >> -			writeb(0x00, p + 0x92); /* MPIOINV[7:0] */
> >> -			writeb(0x00, p + 0x93); /* MPIOSEL[7:0] */
> >> +			writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
> >> +			writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
> >> +			writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
> >>  			break;
> >>  		case PCI_DEVICE_ID_COMMTECH_2324PCI335:
> >>  		case PCI_DEVICE_ID_COMMTECH_2328PCI335:
> >> -			writeb(0x00, p + 0x90); /* MPIOLVL[7:0] */
> >> -			writeb(0xc0, p + 0x92); /* MPIOINV[7:0] */
> >> -			writeb(0xc0, p + 0x93); /* MPIOSEL[7:0] */
> >> +			writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
> >> +			writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
> >> +			writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
> >>  			break;
> >>  		}
> >> -		writeb(0x00, p + 0x8f); /* MPIOINT[7:0] */
> >> -		writeb(0x00, p + 0x91); /* MPIO3T[7:0] */
> >> -		writeb(0x00, p + 0x94); /* MPIOOD[7:0] */
> >> +		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
> >> +		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
> >>  	}
> >>  	writeb(0x00, p + UART_EXAR_8XMODE);
> >>  	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
> >> diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
> >> index 1e5ac4e7..d176448 100644
> >> --- a/include/uapi/linux/serial_reg.h
> >> +++ b/include/uapi/linux/serial_reg.h
> >> @@ -376,5 +376,18 @@
> >>  #define UART_EXAR_TXTRG		0x0a	/* Tx FIFO trigger level write-only */
> >>  #define UART_EXAR_RXTRG		0x0b	/* Rx FIFO trigger level write-only */
> >>  
> >> +#define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
> >> +#define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
> >> +#define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
> >> +#define UART_EXAR_MPIOINV_7_0	0x92	/* MPIOINV[7:0] */
> >> +#define UART_EXAR_MPIOSEL_7_0	0x93	/* MPIOSEL[7:0] */
> >> +#define UART_EXAR_MPIOOD_7_0	0x94	/* MPIOOD[7:0] */
> >> +#define UART_EXAR_MPIOINT_15_8	0x95	/* MPIOINT[15:8] */
> >> +#define UART_EXAR_MPIOLVL_15_8	0x96	/* MPIOLVL[15:8] */
> >> +#define UART_EXAR_MPIO3T_15_8	0x97	/* MPIO3T[15:8] */
> >> +#define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
> >> +#define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
> >> +#define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
> >> +
> > 
> > Why are you putting these in a user api file?  I know it matches the
> > existing ones, but is this something that userspace really cares about?
> 
> Very valid question, and I was wondering the same. Maybe 95% of this
> header is just lazy leftover from 607ca46e97.
> 
> I'm fine with starting off some include/linux/serial_reg.h, provided
> someone gives me a hint what should be moved - I'm not very familiar
> with the serial kernel ABI. Or I just start it off with those defines
> above. You choose.

Just put them in the driver itself for now, no need to add them when no
one is asking for them :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH v2] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers
  2016-08-31 13:58     ` Greg Kroah-Hartman
@ 2016-09-19  4:56       ` Jan Kiszka
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2016-09-19  4:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-serial, Linux Kernel Mailing List

Less magic that only requires comments.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - move new constants from uapi header into driver

 drivers/tty/serial/8250/8250_pci.c | 55 +++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index c1d4a8f..eff6c2f 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1754,6 +1754,19 @@ static int pci_eg20t_init(struct pci_dev *dev)
 #define PCI_DEVICE_ID_EXAR_XR17V4358	0x4358
 #define PCI_DEVICE_ID_EXAR_XR17V8358	0x8358
 
+#define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
+#define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
+#define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
+#define UART_EXAR_MPIOINV_7_0	0x92	/* MPIOINV[7:0] */
+#define UART_EXAR_MPIOSEL_7_0	0x93	/* MPIOSEL[7:0] */
+#define UART_EXAR_MPIOOD_7_0	0x94	/* MPIOOD[7:0] */
+#define UART_EXAR_MPIOINT_15_8	0x95	/* MPIOINT[15:8] */
+#define UART_EXAR_MPIOLVL_15_8	0x96	/* MPIOLVL[15:8] */
+#define UART_EXAR_MPIO3T_15_8	0x97	/* MPIO3T[15:8] */
+#define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
+#define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
+#define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
+
 static int
 pci_xr17c154_setup(struct serial_private *priv,
 		  const struct pciserial_board *board,
@@ -1796,18 +1809,18 @@ pci_xr17v35x_setup(struct serial_private *priv,
 	 * Setup Multipurpose Input/Output pins.
 	 */
 	if (idx == 0) {
-		writeb(0x00, p + 0x8f); /*MPIOINT[7:0]*/
-		writeb(0x00, p + 0x90); /*MPIOLVL[7:0]*/
-		writeb(0x00, p + 0x91); /*MPIO3T[7:0]*/
-		writeb(0x00, p + 0x92); /*MPIOINV[7:0]*/
-		writeb(0x00, p + 0x93); /*MPIOSEL[7:0]*/
-		writeb(0x00, p + 0x94); /*MPIOOD[7:0]*/
-		writeb(0x00, p + 0x95); /*MPIOINT[15:8]*/
-		writeb(0x00, p + 0x96); /*MPIOLVL[15:8]*/
-		writeb(0x00, p + 0x97); /*MPIO3T[15:8]*/
-		writeb(0x00, p + 0x98); /*MPIOINV[15:8]*/
-		writeb(0x00, p + 0x99); /*MPIOSEL[15:8]*/
-		writeb(0x00, p + 0x9a); /*MPIOOD[15:8]*/
+		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
+		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
+		writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
+		writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
 	}
 	writeb(0x00, p + UART_EXAR_8XMODE);
 	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
@@ -1843,20 +1856,20 @@ pci_fastcom335_setup(struct serial_private *priv,
 		switch (priv->dev->device) {
 		case PCI_DEVICE_ID_COMMTECH_4222PCI335:
 		case PCI_DEVICE_ID_COMMTECH_4224PCI335:
-			writeb(0x78, p + 0x90); /* MPIOLVL[7:0] */
-			writeb(0x00, p + 0x92); /* MPIOINV[7:0] */
-			writeb(0x00, p + 0x93); /* MPIOSEL[7:0] */
+			writeb(0x78, p + UART_EXAR_MPIOLVL_7_0);
+			writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
+			writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
 			break;
 		case PCI_DEVICE_ID_COMMTECH_2324PCI335:
 		case PCI_DEVICE_ID_COMMTECH_2328PCI335:
-			writeb(0x00, p + 0x90); /* MPIOLVL[7:0] */
-			writeb(0xc0, p + 0x92); /* MPIOINV[7:0] */
-			writeb(0xc0, p + 0x93); /* MPIOSEL[7:0] */
+			writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
+			writeb(0xc0, p + UART_EXAR_MPIOINV_7_0);
+			writeb(0xc0, p + UART_EXAR_MPIOSEL_7_0);
 			break;
 		}
-		writeb(0x00, p + 0x8f); /* MPIOINT[7:0] */
-		writeb(0x00, p + 0x91); /* MPIO3T[7:0] */
-		writeb(0x00, p + 0x94); /* MPIOOD[7:0] */
+		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
+		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
+		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
 	}
 	writeb(0x00, p + UART_EXAR_8XMODE);
 	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-19  4:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-26 19:50 [PATCH] serial: 8250_pci: Use symbolic constants for EXAR's MPIO registers Jan Kiszka
2016-07-26 20:14 ` Greg Kroah-Hartman
2016-07-26 20:44   ` Jan Kiszka
2016-08-31 13:58     ` Greg Kroah-Hartman
2016-09-19  4:56       ` [PATCH v2] " Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).