[PATCH v1 2/2] serial: 8250_pci: Share WCH IDs with parport_serial driver

Andy Shevchenko posted 2 patches 1 year ago
[PATCH v1 2/2] serial: 8250_pci: Share WCH IDs with parport_serial driver
Posted by Andy Shevchenko 1 year ago
parport_serial driver uses subset of WCH IDs that are present in 8250_pci.
Share them via pci_ids.h and switch parport_serial to use defined constants.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/parport/parport_serial.c   | 12 ++++++++----
 drivers/tty/serial/8250/8250_pci.c | 10 ++--------
 include/linux/pci_ids.h            | 11 +++++++++++
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
index 3644997a8342..24d4f3a3ec3d 100644
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -266,10 +266,14 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
 	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
 
 	/* WCH CARDS */
-	{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
-	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
-	{ 0x1c00, 0x3050, 0x1c00, 0x3050, 0, 0, wch_ch382_0s1p},
-	{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
+	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_1S1P,
+	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p },
+	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_2S1P,
+	  0x4348, 0x3253, 0, 0, wch_ch353_2s1p },
+	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_0S1P,
+	  0x1c00, 0x3050, 0, 0, wch_ch382_0s1p },
+	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_2S1P,
+	  0x1c00, 0x3250, 0, 0, wch_ch382_2s1p },
 
 	/* BrainBoxes PX272/PX306 MIO card */
 	{ PCI_VENDOR_ID_INTASHIELD, 0x4100,
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index dfac79744d37..df4d0d832e54 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -64,23 +64,17 @@
 #define PCIE_DEVICE_ID_NEO_2_OX_IBM	0x00F6
 #define PCI_DEVICE_ID_PLX_CRONYX_OMEGA	0xc001
 #define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
-#define PCI_VENDOR_ID_WCHCN		0x4348
+
 #define PCI_DEVICE_ID_WCHCN_CH352_2S	0x3253
-#define PCI_DEVICE_ID_WCHCN_CH353_4S	0x3453
-#define PCI_DEVICE_ID_WCHCN_CH353_2S1PF	0x5046
-#define PCI_DEVICE_ID_WCHCN_CH353_1S1P	0x5053
-#define PCI_DEVICE_ID_WCHCN_CH353_2S1P	0x7053
 #define PCI_DEVICE_ID_WCHCN_CH355_4S	0x7173
+
 #define PCI_VENDOR_ID_AGESTAR		0x5372
 #define PCI_DEVICE_ID_AGESTAR_9375	0x6872
 #define PCI_DEVICE_ID_BROADCOM_TRUMANAGE 0x160a
 #define PCI_DEVICE_ID_AMCC_ADDIDATA_APCI7800 0x818e
 
-#define PCI_VENDOR_ID_WCHIC		0x1c00
-#define PCI_DEVICE_ID_WCHIC_CH382_2S1P	0x3250
 #define PCI_DEVICE_ID_WCHIC_CH384_4S	0x3470
 #define PCI_DEVICE_ID_WCHIC_CH384_8S	0x3853
-#define PCI_DEVICE_ID_WCHIC_CH382_2S	0x3253
 
 #define PCI_DEVICE_ID_MOXA_CP102E	0x1024
 #define PCI_DEVICE_ID_MOXA_CP102EL	0x1025
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index d2402bf4aea2..de5deb1a0118 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2593,6 +2593,11 @@
 
 #define PCI_VENDOR_ID_REDHAT		0x1b36
 
+#define PCI_VENDOR_ID_WCHIC		0x1c00
+#define PCI_DEVICE_ID_WCHIC_CH382_0S1P	0x3050
+#define PCI_DEVICE_ID_WCHIC_CH382_2S1P	0x3250
+#define PCI_DEVICE_ID_WCHIC_CH382_2S	0x3253
+
 #define PCI_VENDOR_ID_SILICOM_DENMARK	0x1c2c
 
 #define PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS	0x1c36
@@ -2647,6 +2652,12 @@
 #define PCI_VENDOR_ID_AKS		0x416c
 #define PCI_DEVICE_ID_AKS_ALADDINCARD	0x0100
 
+#define PCI_VENDOR_ID_WCHCN		0x4348
+#define PCI_DEVICE_ID_WCHCN_CH353_4S	0x3453
+#define PCI_DEVICE_ID_WCHCN_CH353_2S1PF	0x5046
+#define PCI_DEVICE_ID_WCHCN_CH353_1S1P	0x5053
+#define PCI_DEVICE_ID_WCHCN_CH353_2S1P	0x7053
+
 #define PCI_VENDOR_ID_ACCESSIO		0x494f
 #define PCI_DEVICE_ID_ACCESSIO_WDG_CSM	0x22c0
 
-- 
2.43.0.rc1.1336.g36b5255a03ac
Re: [PATCH v1 2/2] serial: 8250_pci: Share WCH IDs with parport_serial driver
Posted by Bjorn Helgaas 1 year ago
On Wed, Dec 04, 2024 at 05:09:22AM +0200, Andy Shevchenko wrote:
> parport_serial driver uses subset of WCH IDs that are present in 8250_pci.
> Share them via pci_ids.h and switch parport_serial to use defined constants.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/parport/parport_serial.c   | 12 ++++++++----
>  drivers/tty/serial/8250/8250_pci.c | 10 ++--------
>  include/linux/pci_ids.h            | 11 +++++++++++

Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# pci-ids.h

>  3 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
> index 3644997a8342..24d4f3a3ec3d 100644
> --- a/drivers/parport/parport_serial.c
> +++ b/drivers/parport/parport_serial.c
> @@ -266,10 +266,14 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
>  	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
>  
>  	/* WCH CARDS */
> -	{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
> -	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
> -	{ 0x1c00, 0x3050, 0x1c00, 0x3050, 0, 0, wch_ch382_0s1p},
> -	{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
> +	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_1S1P,
> +	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p },
> +	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_2S1P,
> +	  0x4348, 0x3253, 0, 0, wch_ch353_2s1p },
> +	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_0S1P,
> +	  0x1c00, 0x3050, 0, 0, wch_ch382_0s1p },
> +	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_2S1P,
> +	  0x1c00, 0x3250, 0, 0, wch_ch382_2s1p },
>  
>  	/* BrainBoxes PX272/PX306 MIO card */
>  	{ PCI_VENDOR_ID_INTASHIELD, 0x4100,
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index dfac79744d37..df4d0d832e54 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -64,23 +64,17 @@
>  #define PCIE_DEVICE_ID_NEO_2_OX_IBM	0x00F6
>  #define PCI_DEVICE_ID_PLX_CRONYX_OMEGA	0xc001
>  #define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
> -#define PCI_VENDOR_ID_WCHCN		0x4348
> +
>  #define PCI_DEVICE_ID_WCHCN_CH352_2S	0x3253
> -#define PCI_DEVICE_ID_WCHCN_CH353_4S	0x3453
> -#define PCI_DEVICE_ID_WCHCN_CH353_2S1PF	0x5046
> -#define PCI_DEVICE_ID_WCHCN_CH353_1S1P	0x5053
> -#define PCI_DEVICE_ID_WCHCN_CH353_2S1P	0x7053
>  #define PCI_DEVICE_ID_WCHCN_CH355_4S	0x7173
> +
>  #define PCI_VENDOR_ID_AGESTAR		0x5372
>  #define PCI_DEVICE_ID_AGESTAR_9375	0x6872
>  #define PCI_DEVICE_ID_BROADCOM_TRUMANAGE 0x160a
>  #define PCI_DEVICE_ID_AMCC_ADDIDATA_APCI7800 0x818e
>  
> -#define PCI_VENDOR_ID_WCHIC		0x1c00
> -#define PCI_DEVICE_ID_WCHIC_CH382_2S1P	0x3250
>  #define PCI_DEVICE_ID_WCHIC_CH384_4S	0x3470
>  #define PCI_DEVICE_ID_WCHIC_CH384_8S	0x3853
> -#define PCI_DEVICE_ID_WCHIC_CH382_2S	0x3253
>  
>  #define PCI_DEVICE_ID_MOXA_CP102E	0x1024
>  #define PCI_DEVICE_ID_MOXA_CP102EL	0x1025
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index d2402bf4aea2..de5deb1a0118 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2593,6 +2593,11 @@
>  
>  #define PCI_VENDOR_ID_REDHAT		0x1b36
>  
> +#define PCI_VENDOR_ID_WCHIC		0x1c00
> +#define PCI_DEVICE_ID_WCHIC_CH382_0S1P	0x3050
> +#define PCI_DEVICE_ID_WCHIC_CH382_2S1P	0x3250
> +#define PCI_DEVICE_ID_WCHIC_CH382_2S	0x3253
> +
>  #define PCI_VENDOR_ID_SILICOM_DENMARK	0x1c2c
>  
>  #define PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS	0x1c36
> @@ -2647,6 +2652,12 @@
>  #define PCI_VENDOR_ID_AKS		0x416c
>  #define PCI_DEVICE_ID_AKS_ALADDINCARD	0x0100
>  
> +#define PCI_VENDOR_ID_WCHCN		0x4348
> +#define PCI_DEVICE_ID_WCHCN_CH353_4S	0x3453
> +#define PCI_DEVICE_ID_WCHCN_CH353_2S1PF	0x5046
> +#define PCI_DEVICE_ID_WCHCN_CH353_1S1P	0x5053
> +#define PCI_DEVICE_ID_WCHCN_CH353_2S1P	0x7053
> +
>  #define PCI_VENDOR_ID_ACCESSIO		0x494f
>  #define PCI_DEVICE_ID_ACCESSIO_WDG_CSM	0x22c0
>  
> -- 
> 2.43.0.rc1.1336.g36b5255a03ac
>
Re: [PATCH v1 2/2] serial: 8250_pci: Share WCH IDs with parport_serial driver
Posted by Jiri Slaby 1 year ago
On 04. 12. 24, 4:09, Andy Shevchenko wrote:
> parport_serial driver uses subset of WCH IDs that are present in 8250_pci.
> Share them via pci_ids.h and switch parport_serial to use defined constants.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/parport/parport_serial.c   | 12 ++++++++----
>   drivers/tty/serial/8250/8250_pci.c | 10 ++--------
>   include/linux/pci_ids.h            | 11 +++++++++++
>   3 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
> index 3644997a8342..24d4f3a3ec3d 100644
> --- a/drivers/parport/parport_serial.c
> +++ b/drivers/parport/parport_serial.c
> @@ -266,10 +266,14 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
>   	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
>   
>   	/* WCH CARDS */
> -	{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
> -	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
> -	{ 0x1c00, 0x3050, 0x1c00, 0x3050, 0, 0, wch_ch382_0s1p},
> -	{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
> +	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_1S1P,
> +	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p },
> +	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_2S1P,
> +	  0x4348, 0x3253, 0, 0, wch_ch353_2s1p },
> +	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_0S1P,
> +	  0x1c00, 0x3050, 0, 0, wch_ch382_0s1p },
> +	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_2S1P,
> +	  0x1c00, 0x3250, 0, 0, wch_ch382_2s1p },

I know this is the current pattern in the file. But what about using 
PCI_DEVICE_DATA() for the first and PCI_DEVICE_SUB() + .driver_data for 
the rest? Otherwise it occurs as a load of incomprehensible constants.

thanks,
-- 
js
suse labs
Re: [PATCH v1 2/2] serial: 8250_pci: Share WCH IDs with parport_serial driver
Posted by Andy Shevchenko 1 year ago
On Wed, Dec 04, 2024 at 08:14:42AM +0100, Jiri Slaby wrote:
> On 04. 12. 24, 4:09, Andy Shevchenko wrote:
> > parport_serial driver uses subset of WCH IDs that are present in 8250_pci.
> > Share them via pci_ids.h and switch parport_serial to use defined constants.

> >   	{ 0x1409, 0x7168, 0x1409, 0xd079, 0, 0, timedia_9079c },
> >   	/* WCH CARDS */
> > -	{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
> > -	{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
> > -	{ 0x1c00, 0x3050, 0x1c00, 0x3050, 0, 0, wch_ch382_0s1p},
> > -	{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
> > +	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_1S1P,
> > +	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p },
> > +	{ PCI_VENDOR_ID_WCHCN, PCI_DEVICE_ID_WCHCN_CH353_2S1P,
> > +	  0x4348, 0x3253, 0, 0, wch_ch353_2s1p },
> > +	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_0S1P,
> > +	  0x1c00, 0x3050, 0, 0, wch_ch382_0s1p },
> > +	{ PCI_VENDOR_ID_WCHIC, PCI_DEVICE_ID_WCHIC_CH382_2S1P,
> > +	  0x1c00, 0x3250, 0, 0, wch_ch382_2s1p },
> 
> I know this is the current pattern in the file. But what about using
> PCI_DEVICE_DATA() for the first and PCI_DEVICE_SUB() + .driver_data for the
> rest? Otherwise it occurs as a load of incomprehensible constants.

I can issue an additional patch, I was really thinking about this, but decided
to go the above way as it seems logical and moving to mentioned macros sounds
like unrelated to the point of the change.

-- 
With Best Regards,
Andy Shevchenko