[RFC PATCH] plx_pci: add Advantech PCI-1680 & UNO-2052 support

[RFC PATCH] plx_pci: add Advantech PCI-1680 & UNO-2052 support

[Oliver Hartkopp]

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

Signed-off-by: Frank Rysanek <rysanek@fccps.cz>

---

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

diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index fe9e64d..70e416c 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -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"
diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
index c7f3d4e..6ea73ef 100644
--- a/drivers/net/can/sja1000/plx_pci.c
+++ b/drivers/net/can/sja1000/plx_pci.c
@@ -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.

Re: [RFC PATCH] plx_pci: add Advantech PCI-1680 & UNO-2052 support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

On 11/21/2011 09:34 PM, Oliver Hartkopp wrote:
> Add Advantech PCI-1680 & UNO-2052 support for the SJA1000 plx_pci driver.
> 
> Signed-off-by: Frank Rysanek <rysanek@fccps.cz>

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

> 
> ---
> 
> 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() ...

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

> 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

-- 
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [RFC PATCH] plx_pci: add Advantech PCI-1680 & UNO-2052 support

[Frantisek Rysanek]

Dne 21.11.2011 21:34, Oliver Hartkopp napsal(a):
> Add Advantech PCI-1680 & UNO-2052 support for the SJA1000 plx_pci driver.
> 
> Signed-off-by: Frank Rysanek <rysanek@fccps.cz>
> 
> ---
> 
> 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...

> 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.

> Thanks for your patch,
> Oliver
>
Thanks a lot for *your* time and attention :-)

Frank Rysanek