[PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()

Andy Shevchenko posted 13 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Andy Shevchenko 1 year, 7 months ago
The CTI_PCI_DEVICE() duplicates EXAR_DEVICE(). Kill the former.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_exar.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 150c4abd92fc..ab0abc14ecf8 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -1737,7 +1737,6 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 	.exit		= pci_xr17v35x_exit,
 };
 
-// For Connect Tech cards with Exar vendor/device PCI IDs
 #define CTI_EXAR_DEVICE(devid, bd) {                    \
 	PCI_DEVICE_SUB(                                 \
 		PCI_VENDOR_ID_EXAR,                     \
@@ -1747,16 +1746,6 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 		(kernel_ulong_t)&bd                     \
 	}
 
-// For Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based)
-#define CTI_PCI_DEVICE(devid, bd) {                     \
-	PCI_DEVICE_SUB(                                 \
-		PCI_VENDOR_ID_CONNECT_TECH,             \
-		PCI_DEVICE_ID_CONNECT_TECH_PCI_##devid, \
-		PCI_ANY_ID,                             \
-		PCI_ANY_ID), 0, 0,                      \
-		(kernel_ulong_t)&bd                     \
-	}
-
 #define EXAR_DEVICE(vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }
 
 #define IBM_DEVICE(devid, sdevid, bd) {			\
@@ -1786,6 +1775,7 @@ static const struct pci_device_id exar_pci_tbl[] = {
 	EXAR_DEVICE(ACCESSIO, COM_4SM, pbn_exar_XR17C15x),
 	EXAR_DEVICE(ACCESSIO, COM_8SM, pbn_exar_XR17C15x),
 
+	/* Connect Tech cards with Exar vendor/device PCI IDs */
 	CTI_EXAR_DEVICE(XR17C152,       pbn_cti_xr17c15x),
 	CTI_EXAR_DEVICE(XR17C154,       pbn_cti_xr17c15x),
 	CTI_EXAR_DEVICE(XR17C158,       pbn_cti_xr17c15x),
@@ -1798,9 +1788,10 @@ static const struct pci_device_id exar_pci_tbl[] = {
 	CTI_EXAR_DEVICE(XR17V354,       pbn_cti_xr17v35x),
 	CTI_EXAR_DEVICE(XR17V358,       pbn_cti_xr17v35x),
 
-	CTI_PCI_DEVICE(XR79X_12_XIG00X, pbn_cti_fpga),
-	CTI_PCI_DEVICE(XR79X_12_XIG01X, pbn_cti_fpga),
-	CTI_PCI_DEVICE(XR79X_16,        pbn_cti_fpga),
+	/* Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based) */
+	EXAR_DEVICE(CONNECT_TECH, PCI_XR79X_12_XIG00X, pbn_cti_fpga),
+	EXAR_DEVICE(CONNECT_TECH, PCI_XR79X_12_XIG01X, pbn_cti_fpga),
+	EXAR_DEVICE(CONNECT_TECH, PCI_XR79X_16,        pbn_cti_fpga),
 
 	IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),
 
-- 
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Parker Newman 1 year, 7 months ago
On Thu,  2 May 2024 17:43:57 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The CTI_PCI_DEVICE() duplicates EXAR_DEVICE(). Kill the former.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/tty/serial/8250/8250_exar.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 150c4abd92fc..ab0abc14ecf8 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -1737,7 +1737,6 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
>  	.exit		= pci_xr17v35x_exit,
>  };
>
> -// For Connect Tech cards with Exar vendor/device PCI IDs
>  #define CTI_EXAR_DEVICE(devid, bd) {                    \
>  	PCI_DEVICE_SUB(                                 \
>  		PCI_VENDOR_ID_EXAR,                     \
> @@ -1747,16 +1746,6 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
>  		(kernel_ulong_t)&bd                     \
>  	}
>
> -// For Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based)
> -#define CTI_PCI_DEVICE(devid, bd) {                     \
> -	PCI_DEVICE_SUB(                                 \
> -		PCI_VENDOR_ID_CONNECT_TECH,             \
> -		PCI_DEVICE_ID_CONNECT_TECH_PCI_##devid, \
> -		PCI_ANY_ID,                             \
> -		PCI_ANY_ID), 0, 0,                      \
> -		(kernel_ulong_t)&bd                     \
> -	}
> -
>  #define EXAR_DEVICE(vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }
>
This is not correct. The CTI_PCI_DEVICE() macro is for cards that have the
Connect Tech PCI Vendor ID (not Sub-Vendor ID). EXAR_DEVICE() is for cards with
Exar PCI Vendor ID.

Thanks,
Parker

>  #define IBM_DEVICE(devid, sdevid, bd) {			\
> @@ -1786,6 +1775,7 @@ static const struct pci_device_id exar_pci_tbl[] = {
>  	EXAR_DEVICE(ACCESSIO, COM_4SM, pbn_exar_XR17C15x),
>  	EXAR_DEVICE(ACCESSIO, COM_8SM, pbn_exar_XR17C15x),
>
> +	/* Connect Tech cards with Exar vendor/device PCI IDs */
>  	CTI_EXAR_DEVICE(XR17C152,       pbn_cti_xr17c15x),
>  	CTI_EXAR_DEVICE(XR17C154,       pbn_cti_xr17c15x),
>  	CTI_EXAR_DEVICE(XR17C158,       pbn_cti_xr17c15x),
> @@ -1798,9 +1788,10 @@ static const struct pci_device_id exar_pci_tbl[] = {
>  	CTI_EXAR_DEVICE(XR17V354,       pbn_cti_xr17v35x),
>  	CTI_EXAR_DEVICE(XR17V358,       pbn_cti_xr17v35x),
>
> -	CTI_PCI_DEVICE(XR79X_12_XIG00X, pbn_cti_fpga),
> -	CTI_PCI_DEVICE(XR79X_12_XIG01X, pbn_cti_fpga),
> -	CTI_PCI_DEVICE(XR79X_16,        pbn_cti_fpga),
> +	/* Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based) */
> +	EXAR_DEVICE(CONNECT_TECH, PCI_XR79X_12_XIG00X, pbn_cti_fpga),
> +	EXAR_DEVICE(CONNECT_TECH, PCI_XR79X_12_XIG01X, pbn_cti_fpga),
> +	EXAR_DEVICE(CONNECT_TECH, PCI_XR79X_16,        pbn_cti_fpga),
>
>  	IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),
>
Re: [PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Andy Shevchenko 1 year, 7 months ago
On Thu, May 02, 2024 at 11:13:14AM -0400, Parker Newman wrote:
> On Thu,  2 May 2024 17:43:57 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> > The CTI_PCI_DEVICE() duplicates EXAR_DEVICE(). Kill the former.

...

> > -// For Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based)
> > -#define CTI_PCI_DEVICE(devid, bd) {                     \
> > -	PCI_DEVICE_SUB(                                 \
> > -		PCI_VENDOR_ID_CONNECT_TECH,             \
> > -		PCI_DEVICE_ID_CONNECT_TECH_PCI_##devid, \


#define PCI_DEVICE_SUB(vend, dev, subvend, subdev) \
        .vendor = (vend), .device = (dev), \
        .subvendor = (subvend), .subdevice = (subdev)

#define PCI_DEVICE_DATA(vend, dev, data) \
        .vendor = PCI_VENDOR_ID_##vend, .device = PCI_DEVICE_ID_##vend##_##dev, \
        .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0, \
        .driver_data = (kernel_ulong_t)(data)


> > -		PCI_ANY_ID,                             \
> > -		PCI_ANY_ID), 0, 0,                      \
> > -		(kernel_ulong_t)&bd                     \
> > -	}
> > -
> >  #define EXAR_DEVICE(vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }

> This is not correct. The CTI_PCI_DEVICE() macro is for cards that have the
> Connect Tech PCI Vendor ID (not Sub-Vendor ID). EXAR_DEVICE() is for cards with
> Exar PCI Vendor ID.

Above I added current code of these macros, can you elaborate how it's incorrect?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Parker Newman 1 year, 7 months ago
On Thu, 2 May 2024 18:29:05 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, May 02, 2024 at 11:13:14AM -0400, Parker Newman wrote:
> > On Thu,  2 May 2024 17:43:57 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > > The CTI_PCI_DEVICE() duplicates EXAR_DEVICE(). Kill the former.
>
> ...
>
> > > -// For Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based)
> > > -#define CTI_PCI_DEVICE(devid, bd) {                     \
> > > -	PCI_DEVICE_SUB(                                 \
> > > -		PCI_VENDOR_ID_CONNECT_TECH,             \
> > > -		PCI_DEVICE_ID_CONNECT_TECH_PCI_##devid, \
>
>
> #define PCI_DEVICE_SUB(vend, dev, subvend, subdev) \
>         .vendor = (vend), .device = (dev), \
>         .subvendor = (subvend), .subdevice = (subdev)
>
> #define PCI_DEVICE_DATA(vend, dev, data) \
>         .vendor = PCI_VENDOR_ID_##vend, .device = PCI_DEVICE_ID_##vend##_##dev, \
>         .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID, 0, 0, \
>         .driver_data = (kernel_ulong_t)(data)
>
>
> > > -		PCI_ANY_ID,                             \
> > > -		PCI_ANY_ID), 0, 0,                      \
> > > -		(kernel_ulong_t)&bd                     \
> > > -	}
> > > -
> > >  #define EXAR_DEVICE(vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }
>
> > This is not correct. The CTI_PCI_DEVICE() macro is for cards that have the
> > Connect Tech PCI Vendor ID (not Sub-Vendor ID). EXAR_DEVICE() is for cards with
> > Exar PCI Vendor ID.
>
> Above I added current code of these macros, can you elaborate how it's incorrect?
>

Sorry, you are correct. I was mixed up.
Thanks,
Parker
Re: [PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Andy Shevchenko 1 year, 7 months ago
On Thu, May 02, 2024 at 11:36:10AM -0400, Parker Newman wrote:
> On Thu, 2 May 2024 18:29:05 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 02, 2024 at 11:13:14AM -0400, Parker Newman wrote:

...

> > Above I added current code of these macros, can you elaborate how it's incorrect?
> 
> Sorry, you are correct. I was mixed up.

No problem. And since you are here, can you tell me what type of EEPROM is
connected to the chip in your case?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Parker Newman 1 year, 7 months ago
On Thu, 2 May 2024 18:43:59 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, May 02, 2024 at 11:36:10AM -0400, Parker Newman wrote:
> > On Thu, 2 May 2024 18:29:05 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > On Thu, May 02, 2024 at 11:13:14AM -0400, Parker Newman wrote:
>
> ...
>
> > > Above I added current code of these macros, can you elaborate how it's incorrect?
> >
> > Sorry, you are correct. I was mixed up.
>
> No problem. And since you are here, can you tell me what type of EEPROM is
> connected to the chip in your case?
>

Microchip AT93C46D and various other vendors equivalent parts.
Let me know if you have any other questions.
Parker
Re: [PATCH v1 03/13] serial: 8250_exar: Kill CTI_PCI_DEVICE()
Posted by Andy Shevchenko 1 year, 7 months ago
On Thu, May 02, 2024 at 11:54:39AM -0400, Parker Newman wrote:
> On Thu, 2 May 2024 18:43:59 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 02, 2024 at 11:36:10AM -0400, Parker Newman wrote:
> > > On Thu, 2 May 2024 18:29:05 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Thu, May 02, 2024 at 11:13:14AM -0400, Parker Newman wrote:

...

> > > > Above I added current code of these macros, can you elaborate how it's incorrect?
> > >
> > > Sorry, you are correct. I was mixed up.
> >
> > No problem. And since you are here, can you tell me what type of EEPROM is
> > connected to the chip in your case?
> 
> Microchip AT93C46D and various other vendors equivalent parts.

Thank you!

> Let me know if you have any other questions.

Nope, just asking you to test and review the series when you have time.

-- 
With Best Regards,
Andy Shevchenko