[PATCH 1/2] serial: 8250_exar: Use PCI_DEVICE_DATA macro directly

[PATCH 1/2] serial: 8250_exar: Use PCI_DEVICE_DATA macro directly

[Andrew Davis]

The EXAR_DEVICE macro was converted to use PCI_DEVICE_DATA, having
this macro at doesn't add much, remove it.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/tty/serial/8250/8250_exar.c | 65 +++++++++++++++--------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 64770c62bbec5..878d87f4202bd 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -818,8 +818,6 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 		(kernel_ulong_t)&bd					\
 	}
 
-#define EXAR_DEVICE(vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }
-
 #define IBM_DEVICE(devid, sdevid, bd) {			\
 	PCI_DEVICE_SUB(					\
 		PCI_VENDOR_ID_EXAR,			\
@@ -830,13 +828,14 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 	}
 
 static const struct pci_device_id exar_pci_tbl[] = {
-	EXAR_DEVICE(ACCESSIO, COM_2S, pbn_exar_XR17C15x),
-	EXAR_DEVICE(ACCESSIO, COM_4S, pbn_exar_XR17C15x),
-	EXAR_DEVICE(ACCESSIO, COM_8S, pbn_exar_XR17C15x),
-	EXAR_DEVICE(ACCESSIO, COM232_8, pbn_exar_XR17C15x),
-	EXAR_DEVICE(ACCESSIO, COM_2SM, pbn_exar_XR17C15x),
-	EXAR_DEVICE(ACCESSIO, COM_4SM, pbn_exar_XR17C15x),
-	EXAR_DEVICE(ACCESSIO, COM_8SM, pbn_exar_XR17C15x),
+	/* ACCES I/O Products */
+	{ PCI_DEVICE_DATA(ACCESSIO, COM_2S, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(ACCESSIO, COM_4S, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(ACCESSIO, COM_8S, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(ACCESSIO, COM232_8, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(ACCESSIO, COM_2SM, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(ACCESSIO, COM_4SM, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(ACCESSIO, COM_8SM, &pbn_exar_XR17C15x) },
 
 	CONNECT_DEVICE(XR17C152, UART_2_232, pbn_connect),
 	CONNECT_DEVICE(XR17C154, UART_4_232, pbn_connect),
@@ -854,30 +853,34 @@ static const struct pci_device_id exar_pci_tbl[] = {
 	IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),
 
 	/* Exar Corp. XR17C15[248] Dual/Quad/Octal UART */
-	EXAR_DEVICE(EXAR, XR17C152, pbn_exar_XR17C15x),
-	EXAR_DEVICE(EXAR, XR17C154, pbn_exar_XR17C15x),
-	EXAR_DEVICE(EXAR, XR17C158, pbn_exar_XR17C15x),
+	{ PCI_DEVICE_DATA(EXAR, XR17C152, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(EXAR, XR17C154, &pbn_exar_XR17C15x) },
+	{ PCI_DEVICE_DATA(EXAR, XR17C158, &pbn_exar_XR17C15x) },
 
 	/* Exar Corp. XR17V[48]35[248] Dual/Quad/Octal/Hexa PCIe UARTs */
-	EXAR_DEVICE(EXAR, XR17V352, pbn_exar_XR17V35x),
-	EXAR_DEVICE(EXAR, XR17V354, pbn_exar_XR17V35x),
-	EXAR_DEVICE(EXAR, XR17V358, pbn_exar_XR17V35x),
-	EXAR_DEVICE(EXAR, XR17V4358, pbn_exar_XR17V4358),
-	EXAR_DEVICE(EXAR, XR17V8358, pbn_exar_XR17V8358),
-	EXAR_DEVICE(COMMTECH, 4222PCIE, pbn_fastcom35x_2),
-	EXAR_DEVICE(COMMTECH, 4224PCIE, pbn_fastcom35x_4),
-	EXAR_DEVICE(COMMTECH, 4228PCIE, pbn_fastcom35x_8),
-
-	EXAR_DEVICE(COMMTECH, 4222PCI335, pbn_fastcom335_2),
-	EXAR_DEVICE(COMMTECH, 4224PCI335, pbn_fastcom335_4),
-	EXAR_DEVICE(COMMTECH, 2324PCI335, pbn_fastcom335_4),
-	EXAR_DEVICE(COMMTECH, 2328PCI335, pbn_fastcom335_8),
-
-	EXAR_DEVICE(SEALEVEL, 710xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 720xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 740xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 780xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 716xC, pbn_exar_XR17V35x),
+	{ PCI_DEVICE_DATA(EXAR, XR17V352, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(EXAR, XR17V354, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(EXAR, XR17V358, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(EXAR, XR17V4358, &pbn_exar_XR17V4358) },
+	{ PCI_DEVICE_DATA(EXAR, XR17V8358, &pbn_exar_XR17V8358) },
+
+	/* Commtech PCIe cards */
+	{ PCI_DEVICE_DATA(COMMTECH, 4222PCIE, &pbn_fastcom35x_2) },
+	{ PCI_DEVICE_DATA(COMMTECH, 4224PCIE, &pbn_fastcom35x_4) },
+	{ PCI_DEVICE_DATA(COMMTECH, 4228PCIE, &pbn_fastcom35x_8) },
+
+	/* Commtech PCI cards */
+	{ PCI_DEVICE_DATA(COMMTECH, 4222PCI335, &pbn_fastcom335_2) },
+	{ PCI_DEVICE_DATA(COMMTECH, 4224PCI335, &pbn_fastcom335_4) },
+	{ PCI_DEVICE_DATA(COMMTECH, 2324PCI335, &pbn_fastcom335_4) },
+	{ PCI_DEVICE_DATA(COMMTECH, 2328PCI335, &pbn_fastcom335_8) },
+
+	/* Sealevel 7xxxC serial cards */
+	{ PCI_DEVICE_DATA(SEALEVEL, 710xC, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(SEALEVEL, 720xC, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(SEALEVEL, 740xC, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(SEALEVEL, 780xC, &pbn_exar_XR17V35x) },
+	{ PCI_DEVICE_DATA(SEALEVEL, 716xC, &pbn_exar_XR17V35x) },
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, exar_pci_tbl);
-- 
2.39.2

[PATCH 2/2] serial: 8250_exar: Add support for USR298x PCI Modems

[Andrew Davis]

Possibly the last PCI controller-based (i.e. not a soft/winmodem)
dial-up modem one can still buy.

Looks to have a stock XR17C154 PCI UART chip for communication, but for
some reason when provisioning the PCI IDs they swapped the vendor and
subvendor IDs. Otherwise this card would have worked out of the box.

Searching online, some folks seem to not have this issue and others do,
so it is possible only some batches of cards have this error.

Create a new macro to handle the switched IDs and add support here.

Signed-off-by: Andrew Davis <afd@ti.com>
---
 drivers/tty/serial/8250/8250_exar.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 878d87f4202bd..9792db550f8cb 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -40,9 +40,13 @@
 #define PCI_DEVICE_ID_COMMTECH_4224PCIE		0x0020
 #define PCI_DEVICE_ID_COMMTECH_4228PCIE		0x0021
 #define PCI_DEVICE_ID_COMMTECH_4222PCIE		0x0022
+
 #define PCI_DEVICE_ID_EXAR_XR17V4358		0x4358
 #define PCI_DEVICE_ID_EXAR_XR17V8358		0x8358
 
+#define PCI_SUBDEVICE_ID_USR_2980		0x0128
+#define PCI_SUBDEVICE_ID_USR_2981		0x0129
+
 #define PCI_DEVICE_ID_SEALEVEL_710xC		0x1001
 #define PCI_DEVICE_ID_SEALEVEL_720xC		0x1002
 #define PCI_DEVICE_ID_SEALEVEL_740xC		0x1004
@@ -827,6 +831,15 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 		(kernel_ulong_t)&bd			\
 	}
 
+#define USR_DEVICE(devid, sdevid, bd) {			\
+	PCI_DEVICE_SUB(					\
+		PCI_VENDOR_ID_USR,			\
+		PCI_DEVICE_ID_EXAR_##devid,		\
+		PCI_VENDOR_ID_EXAR,			\
+		PCI_SUBDEVICE_ID_USR_##sdevid), 0, 0,	\
+		(kernel_ulong_t)&bd			\
+	}
+
 static const struct pci_device_id exar_pci_tbl[] = {
 	/* ACCES I/O Products */
 	{ PCI_DEVICE_DATA(ACCESSIO, COM_2S, &pbn_exar_XR17C15x) },
@@ -852,6 +865,10 @@ static const struct pci_device_id exar_pci_tbl[] = {
 
 	IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),
 
+	/* USRobotics USR298x-OEM PCI Modems */
+	USR_DEVICE(XR17C152, 2980, pbn_exar_XR17C15x),
+	USR_DEVICE(XR17C152, 2981, pbn_exar_XR17C15x),
+
 	/* Exar Corp. XR17C15[248] Dual/Quad/Octal UART */
 	{ PCI_DEVICE_DATA(EXAR, XR17C152, &pbn_exar_XR17C15x) },
 	{ PCI_DEVICE_DATA(EXAR, XR17C154, &pbn_exar_XR17C15x) },
-- 
2.39.2

Re: [PATCH 1/2] serial: 8250_exar: Use PCI_DEVICE_DATA macro directly

[Andy Shevchenko]

On Thu, Apr 13, 2023 at 04:44:20PM -0500, Andrew Davis wrote:
> The EXAR_DEVICE macro was converted to use PCI_DEVICE_DATA, having
> this macro at doesn't add much, remove it.

I'm not against this, but I have to point out that this patch brings
inconsistency into the table. Either convert all, or none, I think.

That's why the patch that converts EXAR_DEVICE() to use PCI_DEVICE_DATA()
had a little intrusion.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] serial: 8250_exar: Add support for USR298x PCI Modems

[Andy Shevchenko]

On Thu, Apr 13, 2023 at 04:44:21PM -0500, Andrew Davis wrote:
> Possibly the last PCI controller-based (i.e. not a soft/winmodem)
> dial-up modem one can still buy.
> 
> Looks to have a stock XR17C154 PCI UART chip for communication, but for
> some reason when provisioning the PCI IDs they swapped the vendor and
> subvendor IDs. Otherwise this card would have worked out of the box.
> 
> Searching online, some folks seem to not have this issue and others do,
> so it is possible only some batches of cards have this error.
> 
> Create a new macro to handle the switched IDs and add support here.

Yeah, and here you just going to support the schema before your patch 1.

For this patch
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] serial: 8250_exar: Use PCI_DEVICE_DATA macro directly

[Andrew Davis]

On 4/14/23 12:07 PM, Andy Shevchenko wrote:
> On Thu, Apr 13, 2023 at 04:44:20PM -0500, Andrew Davis wrote:
>> The EXAR_DEVICE macro was converted to use PCI_DEVICE_DATA, having
>> this macro at doesn't add much, remove it.
> 
> I'm not against this, but I have to point out that this patch brings
> inconsistency into the table. Either convert all, or none, I think.
> 

I did notice that, and was not sure how I feel about it either. The
issue is the others in the table have SUBDEVICE_IDs but we have
no simple macro for that.

Maybe what we need is a PCI_DEVICE_SUB_DATA() macro in pci.h, basically
it would be to PCI_DEVICE_SUB() what PCI_DEVICE_DATA() is to PCI_DEVICE().

Then I could re-consistify the table later with that. Thoughts?

Andrew

> That's why the patch that converts EXAR_DEVICE() to use PCI_DEVICE_DATA()
> had a little intrusion.
> 
> 

Re: [PATCH 2/2] serial: 8250_exar: Add support for USR298x PCI Modems

[Greg Kroah-Hartman]

On Thu, Apr 13, 2023 at 04:44:21PM -0500, Andrew Davis wrote:
> Possibly the last PCI controller-based (i.e. not a soft/winmodem)
> dial-up modem one can still buy.
> 
> Looks to have a stock XR17C154 PCI UART chip for communication, but for
> some reason when provisioning the PCI IDs they swapped the vendor and
> subvendor IDs. Otherwise this card would have worked out of the box.
> 
> Searching online, some folks seem to not have this issue and others do,
> so it is possible only some batches of cards have this error.
> 
> Create a new macro to handle the switched IDs and add support here.
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/tty/serial/8250/8250_exar.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Please redo this without patch 1/2 as that would not make sense to
backport anywhere, but adding new device ids are allowed in stable
kernels.  Also, as others pointed out, either convert them all or none
:)

thanks,

greg k-h

Re: [PATCH 2/2] serial: 8250_exar: Add support for USR298x PCI Modems

[Andrew Davis]

On 4/20/23 6:38 AM, Greg Kroah-Hartman wrote:
> On Thu, Apr 13, 2023 at 04:44:21PM -0500, Andrew Davis wrote:
>> Possibly the last PCI controller-based (i.e. not a soft/winmodem)
>> dial-up modem one can still buy.
>>
>> Looks to have a stock XR17C154 PCI UART chip for communication, but for
>> some reason when provisioning the PCI IDs they swapped the vendor and
>> subvendor IDs. Otherwise this card would have worked out of the box.
>>
>> Searching online, some folks seem to not have this issue and others do,
>> so it is possible only some batches of cards have this error.
>>
>> Create a new macro to handle the switched IDs and add support here.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   drivers/tty/serial/8250/8250_exar.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
> 
> Please redo this without patch 1/2 as that would not make sense to
> backport anywhere, but adding new device ids are allowed in stable
> kernels.  Also, as others pointed out, either convert them all or none
> :)
> 

Fair enough, posting v2 with only the second patch now.

Thanks,
Andrew