Hello
This is my first mail to this list... sorry for all my mistakes
We use a 8-port RS-232 MIC-3620 from advantech (see
http://www.advantech.com/products/8-port-RS-232-Communication-CPCI-Card/mod_1-2MLG80.aspx)
and need linux support for this card.
This patch add is:
-------------------------------------
--- a/drivers/serial/8250_pci.c
+++ b/drivers/serial/8250_pci.c
@@ -768,6 +769,8 @@
#define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
#define PCI_SUBDEVICE_ID_POCTAL232 0x0308
#define PCI_SUBDEVICE_ID_POCTAL422 0x0408
+#define PCI_VENDOR_ID_ADVANTECH 0x13fe
+#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
/*
* Master list of serial port init/setup/exit quirks.
@@ -789,6 +792,16 @@
.setup = addidata_apci7800_setup,
},
/*
+ * ADVANTECH
+ */
+ {
+ .vendor = PCI_VENDOR_ID_ADVANTECH,
+ .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ .subdevice = PCI_ANY_ID,
+ .setup = pci_default_setup,
+ },
+ /*
* AFAVLAB cards - these may be called via parport_serial
* It is not clear whether this applies to all products.
*/
@@ -2041,6 +2054,9 @@
#endif
static struct pci_device_id serial_pci_tbl[] = {
+ { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ PCI_DEVICE_ID_ADVANTECH_PCI3620, PCI_ANY_ID, 0, 0,
+ pbn_b2_8_921600 },
{ PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
PCI_SUBVENDOR_ID_CONNECT_TECH,
PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
-------------------------------------
can you add this in the next linux kernel?
Thanks for linux!
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
> + .vendor = PCI_VENDOR_ID_ADVANTECH,
> + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + .subdevice = PCI_ANY_ID,
This looks odd - the subvendor ought to be a vendor id, and there should
be a device id as well.
> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + PCI_DEVICE_ID_ADVANTECH_PCI3620, PCI_ANY_ID, 0, 0,
> + pbn_b2_8_921600 },
> { PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
> PCI_SUBVENDOR_ID_CONNECT_TECH,
> PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
> -------------------------------------
>
> can you add this in the next linux kernel?
Can you firstly send me an lspci -vvxxx of that device. Also see
Documentation/SubmittingPatches about Signed-off-by lines and we'll sort
out merging it.
Alan
On Sun, Jan 18, 2009 at 02:14:09PM +0000, Alan Cox wrote:
> > + .vendor = PCI_VENDOR_ID_ADVANTECH,
> > + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > + .subdevice = PCI_ANY_ID,
>
> This looks odd - the subvendor ought to be a vendor id, and there should
> be a device id as well.
the number ist the same... i use now numbers in the patch...
> > can you add this in the next linux kernel?
>
> Can you firstly send me an lspci -vvxxx of that device. Also see
> Documentation/SubmittingPatches about Signed-off-by lines and we'll sort
> out merging it.
done:
eps:~# lspci -vvxxx -d 13fe:3620
04:0d.0 Serial controller: Advantech Co. Ltd Device 3620 (rev 01) (prog-if 00 [8250])
Subsystem: Device 3620:0001
Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Interrupt: pin A routed to IRQ 11
Region 0: Memory at fddff000 (32-bit, non-prefetchable) [size=128]
Region 2: I/O ports at df00 [size=128]
Capabilities: [40] Power Management version 1
Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-)
Status: D0 PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [48] CompactPCI hot-swap <?>
Capabilities: [4c] Vital Product Data <?>
Kernel modules: 8250_pci-, 8250_pci
00: fe 13 20 36 03 00 90 02 01 00 00 07 10 00 00 00
10: 00 f0 df fd 00 00 00 00 01 df 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 20 36 01 00
30: 00 00 00 00 40 00 00 00 00 00 00 00 0b 01 00 00
40: 01 48 01 48 00 00 00 00 06 4c 00 00 03 00 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
and now the patch:
--------------
This Patch add the device information for the
MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
Signed-off-by: Michael Bramer <[email protected]>
--- a/drivers/serial/8250_pci.c
+++ b/drivers/serial/8250_pci.c
@@ -31,6 +31,7 @@
#include "8250.h"
#undef SERIAL_DEBUG_PCI
+#define SERIAL_DEBUG_PCI
/*
* init function returns:
@@ -768,6 +769,8 @@
#define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
#define PCI_SUBDEVICE_ID_POCTAL232 0x0308
#define PCI_SUBDEVICE_ID_POCTAL422 0x0408
+#define PCI_VENDOR_ID_ADVANTECH 0x13fe
+#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
/*
* Master list of serial port init/setup/exit quirks.
@@ -789,6 +792,16 @@
.setup = addidata_apci7800_setup,
},
/*
+ * ADVANTECH
+ */
+ {
+ .vendor = PCI_VENDOR_ID_ADVANTECH,
+ .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ .subvendor = 0x3620,
+ .subdevice = PCI_ANY_ID,
+ .setup = pci_default_setup,
+ },
+ /*
* AFAVLAB cards - these may be called via parport_serial
* It is not clear whether this applies to all products.
*/
@@ -2041,6 +2054,9 @@
#endif
static struct pci_device_id serial_pci_tbl[] = {
+ { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ PCI_DEVICE_ID_ADVANTECH_PCI3620, PCI_ANY_ID, 0, 0,
+ pbn_b2_8_921600 },
{ PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
PCI_SUBVENDOR_ID_CONNECT_TECH,
PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
--------------
Thanks for your help
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
On 018, 01 18, 2009 at 01:53:59PM +0000, Michael Bramer wrote:
> Hello
>
> This is my first mail to this list... sorry for all my mistakes
>
> We use a 8-port RS-232 MIC-3620 from advantech (see
> http://www.advantech.com/products/8-port-RS-232-Communication-CPCI-Card/mod_1-2MLG80.aspx)
> and need linux support for this card.
>
> This patch add is:
> -------------------------------------
> --- a/drivers/serial/8250_pci.c
> +++ b/drivers/serial/8250_pci.c
> @@ -768,6 +769,8 @@
> #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
> #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
> #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
> +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
> +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
>
> /*
> * Master list of serial port init/setup/exit quirks.
> @@ -789,6 +792,16 @@
> .setup = addidata_apci7800_setup,
> },
> /*
> + * ADVANTECH
> + */
> + {
> + .vendor = PCI_VENDOR_ID_ADVANTECH,
> + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + .subdevice = PCI_ANY_ID,
> + .setup = pci_default_setup,
> + },
> + /*
> * AFAVLAB cards - these may be called via parport_serial
> * It is not clear whether this applies to all products.
> */
This part is not needed at all, pci_default_setup() will be used by default.
> @@ -2041,6 +2054,9 @@
> #endif
>
> static struct pci_device_id serial_pci_tbl[] = {
> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + PCI_DEVICE_ID_ADVANTECH_PCI3620, PCI_ANY_ID, 0, 0,
> + pbn_b2_8_921600 },
> { PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
> PCI_SUBVENDOR_ID_CONNECT_TECH,
> PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
> -------------------------------------
>
> can you add this in the next linux kernel?
>
> Thanks for linux!
>
> Gruss
> Grisu
> --
> Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
> PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
> "Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
> --- Albert Einstein
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Mon, Jan 19, 2009 at 05:51:01PM +0300, Andrey Panin wrote:
> On 018, 01 18, 2009 at 01:53:59PM +0000, Michael Bramer wrote:
> > Hello
> >
> > This is my first mail to this list... sorry for all my mistakes
> >
> > We use a 8-port RS-232 MIC-3620 from advantech (see
> > http://www.advantech.com/products/8-port-RS-232-Communication-CPCI-Card/mod_1-2MLG80.aspx)
> > and need linux support for this card.
> >
> > This patch add is:
> > -------------------------------------
> > --- a/drivers/serial/8250_pci.c
> > +++ b/drivers/serial/8250_pci.c
> > @@ -768,6 +769,8 @@
> > #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
> > #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
> > #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
> > +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
> > +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
> >
> > /*
> > * Master list of serial port init/setup/exit quirks.
> > @@ -789,6 +792,16 @@
> > .setup = addidata_apci7800_setup,
> > },
> > /*
> > + * ADVANTECH
> > + */
> > + {
> > + .vendor = PCI_VENDOR_ID_ADVANTECH,
> > + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > + .subdevice = PCI_ANY_ID,
> > + .setup = pci_default_setup,
> > + },
> > + /*
> > * AFAVLAB cards - these may be called via parport_serial
> > * It is not clear whether this applies to all products.
> > */
>
> This part is not needed at all, pci_default_setup() will be used by default.
ok. I don't test this. But you are right. The last entry in
pci_serial_quirks should catch it.
This make the patch to a 'add only 5 lines for support of 8
Ports'-Patch. :-)
Should I post a new patch, without this 10 lines?
BTW: there are one other card's with a part like this:
- PCI_DEVICE_ID_PLX_9030
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
On 019, 01 19, 2009 at 11:51:22PM +0000, Michael Bramer wrote:
> On Mon, Jan 19, 2009 at 05:51:01PM +0300, Andrey Panin wrote:
> > On 018, 01 18, 2009 at 01:53:59PM +0000, Michael Bramer wrote:
> > > Hello
> > >
> > > This is my first mail to this list... sorry for all my mistakes
> > >
> > > We use a 8-port RS-232 MIC-3620 from advantech (see
> > > http://www.advantech.com/products/8-port-RS-232-Communication-CPCI-Card/mod_1-2MLG80.aspx)
> > > and need linux support for this card.
> > >
> > > This patch add is:
> > > -------------------------------------
> > > --- a/drivers/serial/8250_pci.c
> > > +++ b/drivers/serial/8250_pci.c
> > > @@ -768,6 +769,8 @@
> > > #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
> > > #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
> > > #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
> > > +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
> > > +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
> > >
> > > /*
> > > * Master list of serial port init/setup/exit quirks.
> > > @@ -789,6 +792,16 @@
> > > .setup = addidata_apci7800_setup,
> > > },
> > > /*
> > > + * ADVANTECH
> > > + */
> > > + {
> > > + .vendor = PCI_VENDOR_ID_ADVANTECH,
> > > + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > > + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > > + .subdevice = PCI_ANY_ID,
> > > + .setup = pci_default_setup,
> > > + },
> > > + /*
> > > * AFAVLAB cards - these may be called via parport_serial
> > > * It is not clear whether this applies to all products.
> > > */
> >
> > This part is not needed at all, pci_default_setup() will be used by default.
>
> ok. I don't test this. But you are right. The last entry in
> pci_serial_quirks should catch it.
>
> This make the patch to a 'add only 5 lines for support of 8
> Ports'-Patch. :-)
>
> Should I post a new patch, without this 10 lines?
Yes and add Signed-off-by: line please.
> BTW: there are one other card's with a part like this:
> - PCI_DEVICE_ID_PLX_9030
We can safely remove it.
> Gruss
> Grisu
> --
> Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
> PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
> "Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
> --- Albert Einstein
On Tue, Jan 20, 2009 at 06:12:16PM +0300, Andrey Panin wrote:
> On 019, 01 19, 2009 at 11:51:22PM +0000, Michael Bramer wrote:
> > On Mon, Jan 19, 2009 at 05:51:01PM +0300, Andrey Panin wrote:
> > > On 018, 01 18, 2009 at 01:53:59PM +0000, Michael Bramer wrote:
> > > > + * ADVANTECH
> > > > + */
> > > > + {
> > > > + .vendor = PCI_VENDOR_ID_ADVANTECH,
> > > > + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > > > + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > > > + .subdevice = PCI_ANY_ID,
> > > > + .setup = pci_default_setup,
> > > > + },
> > > > + /*
> > > > * AFAVLAB cards - these may be called via parport_serial
> > > > * It is not clear whether this applies to all products.
> > > > */
> > >
> > > This part is not needed at all, pci_default_setup() will be used by default.
> >
> > ok. I don't test this. But you are right. The last entry in
> > pci_serial_quirks should catch it.
> >
> > Should I post a new patch, without this 10 lines?
>
> Yes and add Signed-off-by: line please.
the new patch:
--------------
This Patch add the device information for the
MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
Signed-off-by: Michael Bramer <[email protected]>
--- a/drivers/serial/8250_pci.c
+++ b/drivers/serial/8250_pci.c
@@ -31,6 +31,7 @@
#include "8250.h"
#undef SERIAL_DEBUG_PCI
+#define SERIAL_DEBUG_PCI
/*
* init function returns:
@@ -768,6 +769,8 @@
#define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
#define PCI_SUBDEVICE_ID_POCTAL232 0x0308
#define PCI_SUBDEVICE_ID_POCTAL422 0x0408
+#define PCI_VENDOR_ID_ADVANTECH 0x13fe
+#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
/*
* Master list of serial port init/setup/exit quirks.
@@ -2041,6 +2054,9 @@
#endif
static struct pci_device_id serial_pci_tbl[] = {
+ { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ 0x3620, PCI_ANY_ID, 0, 0,
+ pbn_b2_8_921600 },
{ PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
PCI_SUBVENDOR_ID_CONNECT_TECH,
PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
--------------
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
Michael Bramer wrote:
> On Tue, Jan 20, 2009 at 06:12:16PM +0300, Andrey Panin wrote:
>> On 019, 01 19, 2009 at 11:51:22PM +0000, Michael Bramer wrote:
>>> On Mon, Jan 19, 2009 at 05:51:01PM +0300, Andrey Panin wrote:
>>>> On 018, 01 18, 2009 at 01:53:59PM +0000, Michael Bramer wrote:
>>>>> + * ADVANTECH
>>>>> + */
>>>>> + {
>>>>> + .vendor = PCI_VENDOR_ID_ADVANTECH,
>>>>> + .device = PCI_DEVICE_ID_ADVANTECH_PCI3620,
>>>>> + .subvendor = PCI_DEVICE_ID_ADVANTECH_PCI3620,
>>>>> + .subdevice = PCI_ANY_ID,
>>>>> + .setup = pci_default_setup,
>>>>> + },
>>>>> + /*
>>>>> * AFAVLAB cards - these may be called via parport_serial
>>>>> * It is not clear whether this applies to all products.
>>>>> */
>>>> This part is not needed at all, pci_default_setup() will be used by default.
>>> ok. I don't test this. But you are right. The last entry in
>>> pci_serial_quirks should catch it.
>>>
>>> Should I post a new patch, without this 10 lines?
>> Yes and add Signed-off-by: line please.
>
>
> the new patch:
> --------------
> This Patch add the device information for the
> MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
>
> Signed-off-by: Michael Bramer <[email protected]>
> --- a/drivers/serial/8250_pci.c
> +++ b/drivers/serial/8250_pci.c
> @@ -31,6 +31,7 @@
> #include "8250.h"
>
> #undef SERIAL_DEBUG_PCI
> +#define SERIAL_DEBUG_PCI
NAK.
> /*
> * init function returns:
> @@ -768,6 +769,8 @@
> #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
> #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
> #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
> +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
> +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
>
> /*
> * Master list of serial port init/setup/exit quirks.
> @@ -2041,6 +2054,9 @@
> #endif
>
> static struct pci_device_id serial_pci_tbl[] = {
> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + 0x3620, PCI_ANY_ID, 0, 0,
> + pbn_b2_8_921600 },
> { PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
> PCI_SUBVENDOR_ID_CONNECT_TECH,
> PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
> --------------
>
> Gruss
> Grisu
Paulius Zaleckas wrote:
> Michael Bramer wrote:
...
>> This Patch add the device information for the
>> MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
>>
>> Signed-off-by: Michael Bramer <[email protected]>
>> --- a/drivers/serial/8250_pci.c
>> +++ b/drivers/serial/8250_pci.c
>> @@ -31,6 +31,7 @@
>> #include "8250.h"
>>
>> #undef SERIAL_DEBUG_PCI
>> +#define SERIAL_DEBUG_PCI
>
> NAK.
>
>> /*
>> * init function returns:
>> @@ -768,6 +769,8 @@
>> #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
>> #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
>> #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
>> +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
>> +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
Should this not be in <include/linux/pci_ids.h>?
>> /*
>> * Master list of serial port init/setup/exit quirks.
>> @@ -2041,6 +2054,9 @@
>> #endif
>>
>> static struct pci_device_id serial_pci_tbl[] = {
>> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
>> + 0x3620, PCI_ANY_ID, 0, 0,
Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
Cheers,
Niels
> >> static struct pci_device_id serial_pci_tbl[] = {
> >> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
> >> + 0x3620, PCI_ANY_ID, 0, 0,
>
> Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
The Advantech vendor id is not 0x3620. This confused me as well which is
why I asked for an lspci. Advantech has stuck the device id in the
subvendor bits and '1' in the subdevice (so it should be 1 not
PCI_ANY_ID).
Alan
On Wed, Jan 21, 2009 at 10:18:05AM +0200, Paulius Zaleckas wrote:
> Michael Bramer wrote:
> > the new patch:
> > --------------
> > This Patch add the device information for the
> > MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
> >
> > Signed-off-by: Michael Bramer <[email protected]>
> > --- a/drivers/serial/8250_pci.c
> > +++ b/drivers/serial/8250_pci.c
> > @@ -31,6 +31,7 @@
> > #include "8250.h"
> >
> > #undef SERIAL_DEBUG_PCI
> > +#define SERIAL_DEBUG_PCI
>
> NAK.
sorry, this was a bug in the patch.
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
On Wed, Jan 21, 2009 at 12:32:15PM +0000, Alan Cox wrote:
> > >> static struct pci_device_id serial_pci_tbl[] = {
> > >> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > >> + 0x3620, PCI_ANY_ID, 0, 0,
> >
> > Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
>
> The Advantech vendor id is not 0x3620. This confused me as well which is
> why I asked for an lspci. Advantech has stuck the device id in the
> subvendor bits and '1' in the subdevice (so it should be 1 not
> PCI_ANY_ID).
is this better?
+ { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
> [mailto:[email protected]]On Behalf Of Michael Bramer
>
> On Wed, Jan 21, 2009 at 12:32:15PM +0000, Alan Cox wrote:
> > > >> static struct pci_device_id serial_pci_tbl[] = {
> > > >> + { PCI_VENDOR_ID_ADVANTECH,
> PCI_DEVICE_ID_ADVANTECH_PCI3620,
> > > >> + 0x3620, PCI_ANY_ID, 0, 0,
> > >
> > > Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
> >
> > The Advantech vendor id is not 0x3620. This confused me as
> well which is
> > why I asked for an lspci. Advantech has stuck the device id in the
> > subvendor bits and '1' in the subdevice (so it should be 1 not
> > PCI_ANY_ID).
>
> is this better?
>
> + { PCI_VENDOR_ID_ADVANTECH,
> PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
Since the name describes a device id where it should be a (sub)vendor id,
I would suggest that you add a line of comment to explain the case.
So that no one will be tempted to change it back to PCI_VENDOR_ID_ADVANTECH
in the future.
Regards
Tosoni wrote:
>> On Wed, Jan 21, 2009 at 12:32:15PM +0000, Alan Cox wrote:
>>>>>> static struct pci_device_id serial_pci_tbl[] = {
>>>>>> + { PCI_VENDOR_ID_ADVANTECH,
>> PCI_DEVICE_ID_ADVANTECH_PCI3620,
>>>>>> + 0x3620, PCI_ANY_ID, 0, 0,
>>>> Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
>>> The Advantech vendor id is not 0x3620. This confused me as
>> well which is
>>> why I asked for an lspci. Advantech has stuck the device id in the
>>> subvendor bits and '1' in the subdevice (so it should be 1 not
>>> PCI_ANY_ID).
>> is this better?
>>
>> + { PCI_VENDOR_ID_ADVANTECH,
>> PCI_DEVICE_ID_ADVANTECH_PCI3620,
>> + PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
>
> Since the name describes a device id where it should be a (sub)vendor id,
> I would suggest that you add a line of comment to explain the case.
> So that no one will be tempted to change it back to PCI_VENDOR_ID_ADVANTECH
> in the future.
Definitely!
Niels
On Thu, Jan 22, 2009 at 10:03:53AM +0100, Niels de Vos wrote:
> Tosoni wrote:
> >> On Wed, Jan 21, 2009 at 12:32:15PM +0000, Alan Cox wrote:
> >>>>>> static struct pci_device_id serial_pci_tbl[] = {
> >>>>>> + { PCI_VENDOR_ID_ADVANTECH,
> >> PCI_DEVICE_ID_ADVANTECH_PCI3620,
> >>>>>> + 0x3620, PCI_ANY_ID, 0, 0,
> >>>> Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
> >>> The Advantech vendor id is not 0x3620. This confused me as
> >> well which is
> >>> why I asked for an lspci. Advantech has stuck the device id in the
> >>> subvendor bits and '1' in the subdevice (so it should be 1 not
> >>> PCI_ANY_ID).
> >> is this better?
> >>
> >> + { PCI_VENDOR_ID_ADVANTECH,
> >> PCI_DEVICE_ID_ADVANTECH_PCI3620,
> >> + PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
> >
> > Since the name describes a device id where it should be a (sub)vendor id,
> > I would suggest that you add a line of comment to explain the case.
> > So that no one will be tempted to change it back to PCI_VENDOR_ID_ADVANTECH
> > in the future.
>
> Definitely!
Is the patch now ok?
----
This Patch add the device information for the
MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
Signed-off-by: Michael Bramer <[email protected]>
--- a/drivers/serial/8250_pci.c 2008-07-13 23:51:29.000000000 +0200
+++ b/drivers/serial/8250_pci.c 2009-01-17 21:37:37.000000000 +0100
@@ -769,6 +768,8 @@
#define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
#define PCI_SUBDEVICE_ID_POCTAL232 0x0308
#define PCI_SUBDEVICE_ID_POCTAL422 0x0408
+#define PCI_VENDOR_ID_ADVANTECH 0x13fe
+#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
/*
* Master list of serial port init/setup/exit quirks.
@@ -2054,6 +2041,10 @@
#endif
static struct pci_device_id serial_pci_tbl[] = {
+ // Advantech use PCI_DEVICE_ID_ADVANTECH_PCI3620 (0x3620) as 'PCI_SUBVENDOR_ID'
+ { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
+ pbn_b2_8_921600 },
{ PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
PCI_SUBVENDOR_ID_CONNECT_TECH,
PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
----
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein
Michael Bramer wrote:
> On Thu, Jan 22, 2009 at 10:03:53AM +0100, Niels de Vos wrote:
>> Tosoni wrote:
>>>> On Wed, Jan 21, 2009 at 12:32:15PM +0000, Alan Cox wrote:
>>>>>>>> static struct pci_device_id serial_pci_tbl[] = {
>>>>>>>> + { PCI_VENDOR_ID_ADVANTECH,
>>>> PCI_DEVICE_ID_ADVANTECH_PCI3620,
>>>>>>>> + 0x3620, PCI_ANY_ID, 0, 0,
>>>>>> Why not use PCI_VENDOR_ID_ADVANTECH as PCI_SUBVENDOR_ID too?
>>>>> The Advantech vendor id is not 0x3620. This confused me as
>>>> well which is
>>>>> why I asked for an lspci. Advantech has stuck the device id in the
>>>>> subvendor bits and '1' in the subdevice (so it should be 1 not
>>>>> PCI_ANY_ID).
>>>> is this better?
>>>>
>>>> + { PCI_VENDOR_ID_ADVANTECH,
>>>> PCI_DEVICE_ID_ADVANTECH_PCI3620,
>>>> + PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
>>> Since the name describes a device id where it should be a (sub)vendor id,
>>> I would suggest that you add a line of comment to explain the case.
>>> So that no one will be tempted to change it back to PCI_VENDOR_ID_ADVANTECH
>>> in the future.
>> Definitely!
>
> Is the patch now ok?
No :)
> ----
> This Patch add the device information for the
> MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
>
> Signed-off-by: Michael Bramer <[email protected]>
> --- a/drivers/serial/8250_pci.c 2008-07-13 23:51:29.000000000 +0200
> +++ b/drivers/serial/8250_pci.c 2009-01-17 21:37:37.000000000 +0100
> @@ -769,6 +768,8 @@
> #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
> #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
> #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
> +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
> +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
Use TABs for indentation here.
> /*
> * Master list of serial port init/setup/exit quirks.
> @@ -2054,6 +2041,10 @@
> #endif
>
> static struct pci_device_id serial_pci_tbl[] = {
> + // Advantech use PCI_DEVICE_ID_ADVANTECH_PCI3620 (0x3620) as 'PCI_SUBVENDOR_ID'
> + { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
> + PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
> + pbn_b2_8_921600 },
> { PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
> PCI_SUBVENDOR_ID_CONNECT_TECH,
> PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
> ----
>
>
> Gruss
> Grisu
On Thu, Jan 22, 2009 at 01:47:35PM +0200, Paulius Zaleckas wrote:
> > Is the patch now ok?
>
> No :)
>
> > ----
> > This Patch add the device information for the
> > MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
> >
> > Signed-off-by: Michael Bramer <[email protected]>
> > --- a/drivers/serial/8250_pci.c 2008-07-13 23:51:29.000000000 +0200
> > +++ b/drivers/serial/8250_pci.c 2009-01-17 21:37:37.000000000 +0100
> > @@ -769,6 +768,8 @@
> > #define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
> > #define PCI_SUBDEVICE_ID_POCTAL232 0x0308
> > #define PCI_SUBDEVICE_ID_POCTAL422 0x0408
> > +#define PCI_VENDOR_ID_ADVANTECH 0x13fe
> > +#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
>
> Use TABs for indentation here.
ok:
-----
This Patch add the device information for the
MIC-3620 8-port RS-232 cPCI card from Advantech Co. Ltd.
Signed-off-by: Michael Bramer <[email protected]>
--- a/drivers/serial/8250_pci.c 2008-07-13 23:51:29.000000000 +0200
+++ b/drivers/serial/8250_pci.c 2009-01-17 21:37:37.000000000 +0100
@@ -769,6 +768,8 @@
#define PCI_SUBDEVICE_ID_OCTPRO422 0x0208
#define PCI_SUBDEVICE_ID_POCTAL232 0x0308
#define PCI_SUBDEVICE_ID_POCTAL422 0x0408
+#define PCI_VENDOR_ID_ADVANTECH 0x13fe
+#define PCI_DEVICE_ID_ADVANTECH_PCI3620 0x3620
/*
* Master list of serial port init/setup/exit quirks.
@@ -2054,6 +2041,10 @@
#endif
static struct pci_device_id serial_pci_tbl[] = {
+ // Advantech use PCI_DEVICE_ID_ADVANTECH_PCI3620 (0x3620) as 'PCI_SUBVENDOR_ID'
+ { PCI_VENDOR_ID_ADVANTECH, PCI_DEVICE_ID_ADVANTECH_PCI3620,
+ PCI_DEVICE_ID_ADVANTECH_PCI3620, 1, 0, 0,
+ pbn_b2_8_921600 },
{ PCI_VENDOR_ID_V3, PCI_DEVICE_ID_V3_V960,
PCI_SUBVENDOR_ID_CONNECT_TECH,
PCI_SUBDEVICE_ID_CONNECT_TECH_BH8_232, 0, 0,
-----
Gruss
Grisu
--
Michael Bramer -- http://www.feuerwehr.kreuzau.de/wiki/
PGP: finger [email protected] -- Linux Sysadmin -- Use Debian Linux
"Wenn ich die Folgen geahnt h?tte, w?re ich Uhrmacher geworden!"
--- Albert Einstein