Skip to contentSkip to conversation

plx_pci: add Advantech PCI-1680 & UNO-2052 support

RFC PATCH In review Oliver Hartkopp 3 participants 3 messages Nov 21, 2011 – Nov 24, 2011 CAN network drivers

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 plx_pci: add Advantech PCI-1680 & UNO-2052 support

Add Advantech PCI-1680 & UNO-2052 support for the SJA1000 plx_pci driver.

---

Hello Frank,

i found your patch to support the Advantech PCI-1680 & UNO-2052 cards here:

http://www.fccps.cz/download/adv/frr/can-notes.html

Are you fine with posting the changes on the netdev mailing list to become part of the mainline Linux kernel?

PLEASE REVIEW: You added an mdelay(100) in plx_pci_check_sja1000() ...

Are you sure this long delay is needed in plx_pci_check_sja1000() or should we better add a shorter mdelay(10) at the end of plx_pci_reset_common() ??

Are you sure your setup is not working without the added mdelay(100) ?

Thanks for your patch,
Oliver

drivers/net/can/sja1000/Kconfig +1 -0
index fe9e64d..70e416c 100644
@@ -72,6 +72,7 @@ config CAN_PLX_PCI
 	   - esd CAN-PCIe/2000
 	   - Marathon CAN-bus-PCI card (http://www.marathon.ru/)
 	   - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
+	   - Advantech PCI-1680 and UNO-2052 (http://www.advantech.com)
 
 config CAN_TSCAN1
 	tristate "TS-CAN1 PC104 boards"
drivers/net/can/sja1000/plx_pci.c +39 -1
index c7f3d4e..6ea73ef 100644
@@ -43,7 +43,8 @@ MODULE_SUPPORTED_DEVICE("Adlink PCI-7841/cPCI-7841, "
 			"TEWS TECHNOLOGIES TPMC810, "
 			"esd CAN-PCI/CPCI/PCI104/200, "
 			"esd CAN-PCI/PMC/266, "
-			"esd CAN-PCIe/2000")
+			"esd CAN-PCIe/2000, "
+			"Advantech PCI-1680 & UNO-2052");
 MODULE_LICENSE("GPL v2");
 
 #define PLX_PCI_MAX_CHAN 2
@@ -126,6 +127,10 @@ struct plx_pci_card {
 #define TEWS_PCI_VENDOR_ID		0x1498
 #define TEWS_PCI_DEVICE_ID_TMPC810	0x032A
 
+#define ADVANTECH_PCI_VENDOR_ID		0x13FE
+#define ADVANTECH_PCI1680_DEVICE_ID	0x1680
+#define ADVANTECH_UNO2052_DEVICE_ID	0x2052
+
 static void plx_pci_reset_common(struct pci_dev *pdev);
 static void plx_pci_reset_marathon(struct pci_dev *pdev);
 static void plx9056_pci_reset_common(struct pci_dev *pdev);
@@ -209,6 +214,22 @@ static struct plx_pci_card_info plx_pci_card_info_tews __devinitdata = {
 	/* based on PLX9030 */
 };
 
+static struct plx_pci_card_info plx_pci_card_info_pci1680 __devinitdata = {
+	"Advantech PCI-1680", 2,
+	PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
+	{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {3, 0x00, 0x80} },
+	&plx_pci_reset_common
+	/* based on PLX PCI9030 */
+};
+
+static struct plx_pci_card_info plx_pci_card_info_uno2052 __devinitdata = {
+	"Advantech UNO-2052", 2,
+	PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
+	{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {3, 0x00, 0x80} },
+	&plx_pci_reset_common
+	/* based on PLX PCI9052 */
+};
+
 static DEFINE_PCI_DEVICE_TABLE(plx_pci_tbl) = {
 	{
 		/* Adlink PCI-7841/cPCI-7841 */
@@ -280,6 +301,20 @@ static DEFINE_PCI_DEVICE_TABLE(plx_pci_tbl) = {
 		0, 0,
 		(kernel_ulong_t)&plx_pci_card_info_tews
 	},
+	{
+		/* Advantech PCI-1680 */
+		ADVANTECH_PCI_VENDOR_ID, ADVANTECH_PCI1680_DEVICE_ID,
+		ADVANTECH_PCI_VENDOR_ID, PCI_ANY_ID,
+		0, 0,
+		(kernel_ulong_t)&plx_pci_card_info_pci1680
+	},
+	{
+		/* Advantech UNO-2052 */
+		ADVANTECH_PCI_VENDOR_ID, ADVANTECH_UNO2052_DEVICE_ID,
+		PCI_ANY_ID, PCI_ANY_ID,
+		0, 0,
+		(kernel_ulong_t)&plx_pci_card_info_uno2052
+	},
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, plx_pci_tbl);
@@ -303,6 +338,9 @@ static inline int plx_pci_check_sja1000(const struct sja1000_priv *priv)
 {
 	int flag = 0;
 
+	/* settle the SJA1000 from PLX chip reset */
+	mdelay(100);
+
 	/*
 	 * Check registers after hardware reset (the Basic mode)
 	 * See states on p. 10 of the Datasheet.
#1 lore raw
Quoted 3 lines · Oliver Hartkopp

Add Advantech PCI-1680 & UNO-2052 support for the SJA1000 plx_pci driver.

Nitpick: When posting this patch to netdev don't forget to add your S-o-b line.

Quoted 5 lines

---

Hello Frank,

Quoted 8 lines

i found your patch to support the Advantech PCI-1680 & UNO-2052 cards here:

http://www.fccps.cz/download/adv/frr/can-notes.html

Are you fine with posting the changes on the netdev mailing list to become part of the mainline Linux kernel?

PLEASE REVIEW: You added an mdelay(100) in plx_pci_check_sja1000() ...

+1 - Such a long delay should not end up in the common code.

Quoted 4 lines

Are you sure this long delay is needed in plx_pci_check_sja1000() or should we better add a shorter mdelay(10) at the end of plx_pci_reset_common() ??

Are you sure your setup is not working without the added mdelay(100) ?

Marc

Signature · 4 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
#2 lore raw
In reply to Oliver Hartkopp

Dne 21.11.2011 21:34, Oliver Hartkopp napsal(a):

Quoted 6 lines

Add Advantech PCI-1680 & UNO-2052 support for the SJA1000 plx_pci driver.

---

Quoted 9 lines

Hello Frank,

i found your patch to support the Advantech PCI-1680 & UNO-2052 cards here:

http://www.fccps.cz/download/adv/frr/can-notes.html

Are you fine with posting the changes on the netdev mailing list to become part of the mainline Linux kernel?

definitely, that would be excellent :-)
Actually I've tried to submit the patch to linux-netdev, but it didn't have the GIT-wise bells and whistles and I wasn't surprised that there was no response...

Quoted 7 lines

PLEASE REVIEW: You added an mdelay(100) in plx_pci_check_sja1000() ...

Are you sure this long delay is needed in plx_pci_check_sja1000() or should we better add a shorter mdelay(10) at the end of plx_pci_reset_common() ??

Are you sure your setup is not working without the added mdelay(100) ?

That's a good question.
I believe I got the time delay of 100 ms from some macro in a "character device driver" maintained by the people at Peak Systemtechnik - the macro specifies a "maximum time limit for the SJA1000 to accomplish a switch into the RESET MODE".
The algorithm used by Peak was more complex than my hardwired delay:
they would wait for the flag (sign of success) in a loop, calling just schedule() during every fruitless iteration.
And, the wait for the chip to "switch into RESET mode" is not exactly the same as waiting for the chip to come back after a cold reset.

I added the delay because without it, my two SJA1000 channels would remain undetected. It worked for me, and it didn't occur to me that 100ms at boot would be a problem - so I didn't bother to try a shorter delay. As far as I can tell from the datasheet, the SJA1000 does not seem to contain an MCU core running some "microcode", so I see no reason why it shouldn't come up within a few clock cycles, rather than a 100 ms. I'm gonna have to take a look if we happen to have some of those boards in stock (they keep coming and going), stick one piece into a suitable PC at my workplace and try to "bisect" an appropriate interval for that delay...
Or can you suggest some "sign of life" to wait for in a semi-tight loop?

The problem is that at the moment I'm pretty swamped by other intervening activities, both at work (hunting down other people's hardware bugs) and in my private life (small children) - I hope to be able to get this check done in a few days. I'll keep you posted.

Quoted 3 lines

Thanks for your patch,
Oliver

Thanks a lot for *your* time and attention :-)

Frank Rysanek