Re: [PATCH] can: janz-ican3: add support for CAL/CANopen firmware

["Andreas Gröger" <andreas24groeger@gmail.com>]

Hi Marc, Hi Ira,

thank you very much for the positive and constructive responses, seems to be the right way. 

String comparison for firmware identification: there is another way to retrieve the id and version (IDVERS-request), but it is more complicated. You have to await the response and the result is a longer string about 200 bytes. Janz recommends this firmware stamp for module identification.

Bittiming in ican3_set_bus_state: this affects the driver with ICANOS-firmware. So I asked Janz for the ICANOS-firmware to be able to test. I tested both firmware-variants with different baudrates: went as expected. Hope I met your needs.

Kind regards,
Andreas

Signed-off-by: Andreas Gröger <andreas24groeger@google.com>
---
 Documentation/ABI/testing/sysfs-bus-pci-drivers-janz-cmodio |    8 
 Documentation/ABI/testing/sysfs-class-net-janz-ican3        |   21 ++
 drivers/mfd/janz-cmodio.c                                   |    4 
 drivers/net/can/janz-ican3.c                                |  125 ++++++++----
 4 files changed, 121 insertions(+), 37 deletions(-)

diff -uNr -X linux-3.19.me/Documentation/dontdiff linux-3.19.6/Documentation/ABI/testing/sysfs-bus-pci-drivers-janz-cmodio linux-3.19.me/Documentation/ABI/testing/sysfs-bus-pci-drivers-janz-cmodio
--- linux-3.19.6/Documentation/ABI/testing/sysfs-bus-pci-drivers-janz-cmodio	1970-01-01 01:00:00.000000000 +0100
+++ linux-3.19.me/Documentation/ABI/testing/sysfs-bus-pci-drivers-janz-cmodio	2015-05-04 15:44:33.056595249 +0200
@@ -0,0 +1,8 @@
+What:		/sys/bus/pci/drivers/janz-cmodio/.../modulbus_number
+Date:		May 2010
+KernelVersion:	2.6.35
+Contact:	Ira W. Snyder <ira.snyder@gmail.com>
+Description:
+		Value representing the HEX switch S2 of the janz carrier board CMOD-IO or CAN-PCI2
+
+		Read-only: value of the configuration switch (0..15)
diff -uNr -X linux-3.19.me/Documentation/dontdiff linux-3.19.6/Documentation/ABI/testing/sysfs-class-net-janz-ican3 linux-3.19.me/Documentation/ABI/testing/sysfs-class-net-janz-ican3
--- linux-3.19.6/Documentation/ABI/testing/sysfs-class-net-janz-ican3	1970-01-01 01:00:00.000000000 +0100
+++ linux-3.19.me/Documentation/ABI/testing/sysfs-class-net-janz-ican3	2015-05-04 17:38:38.527615571 +0200
@@ -0,0 +1,21 @@
+What:		/sys/class/net/<iface>/termination
+Date:		May 2010
+KernelVersion:	2.6.35
+Contact:	Ira W. Snyder <ira.snyder@gmail.com>
+Description:
+		Value representing the can bus termination
+
+		Default: 1 (termination active)
+		Reading: get actual termination state
+		Writing: set actual termination state (0=no termination, 1=termination active)
+
+What:		/sys/class/net/<iface>/fwinfo
+Date:		May 2015
+KernelVersion:	3.19
+Contact:	Andreas Gröger <andreas24groeger@gmail.com>
+Description:
+		Firmware stamp of ican3 module
+		Read-only: 32 byte string identification of the ICAN3 module
+		(known values: "JANZ-ICAN3 ICANOS 1.xx", "JANZ-ICAN3 CAL/CANopen 1.xx")
+
+
diff -uNr -X linux-3.19.me/Documentation/dontdiff linux-3.19.6/drivers/mfd/janz-cmodio.c linux-3.19.me/drivers/mfd/janz-cmodio.c
--- linux-3.19.6/drivers/mfd/janz-cmodio.c	2015-04-29 10:30:26.000000000 +0200
+++ linux-3.19.me/drivers/mfd/janz-cmodio.c	2015-05-04 17:34:00.044635384 +0200
@@ -267,6 +267,10 @@
 static const struct pci_device_id cmodio_pci_ids[] = {
 	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9030, PCI_VENDOR_ID_JANZ, 0x0101 },
 	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_JANZ, 0x0100 },
+	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9030, PCI_VENDOR_ID_JANZ, 0x0201 },
+	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9030, PCI_VENDOR_ID_JANZ, 0x0202 },
+	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_JANZ, 0x0201 },
+	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_JANZ, 0x0202 },
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, cmodio_pci_ids);
diff -uNr -X linux-3.19.me/Documentation/dontdiff linux-3.19.6/drivers/net/can/janz-ican3.c linux-3.19.me/drivers/net/can/janz-ican3.c
--- linux-3.19.6/drivers/net/can/janz-ican3.c	2015-04-29 10:30:26.000000000 +0200
+++ linux-3.19.me/drivers/net/can/janz-ican3.c	2015-05-04 17:34:00.044635384 +0200
@@ -40,6 +40,7 @@
 #define MSYNC_PEER		0x00		/* ICAN only */
 #define MSYNC_LOCL		0x01		/* host only */
 #define TARGET_RUNNING		0x02
+#define FIRMWARE_STAMP		0x60		/* big endian firmware stamp */
 
 #define MSYNC_RB0		0x01
 #define MSYNC_RB1		0x02
@@ -83,6 +84,7 @@
 #define MSG_COFFREQ		0x42
 #define MSG_CONREQ		0x43
 #define MSG_CCONFREQ		0x47
+#define MSG_LMTS		0xb4
 
 /*
  * Janz ICAN3 CAN Inquiry Message Types
@@ -165,6 +167,12 @@
 /* SJA1000 Clock Input */
 #define ICAN3_CAN_CLOCK		8000000
 
+/* Janz ICAN3 firmware types */
+enum ican3_fwtype {
+	ICAN3_FWTYPE_ICANOS,
+	ICAN3_FWTYPE_CAL_CANOPEN
+};
+
 /* Driver Name */
 #define DRV_NAME "janz-ican3"
 
@@ -215,6 +223,10 @@
 	struct completion buserror_comp;
 	struct can_berr_counter bec;
 
+	/* firmware type */
+	enum ican3_fwtype fwtype;
+	char fwinfo[32];
+
 	/* old and new style host interface */
 	unsigned int iftype;
 
@@ -750,13 +762,61 @@
  */
 static int ican3_set_bus_state(struct ican3_dev *mod, bool on)
 {
+	struct can_bittiming *bt = &mod->can.bittiming;
 	struct ican3_msg msg;
+	u8 btr0, btr1;
+	int res;
 
-	memset(&msg, 0, sizeof(msg));
-	msg.spec = on ? MSG_CONREQ : MSG_COFFREQ;
-	msg.len = cpu_to_le16(0);
+	/* This algorithm was stolen from drivers/net/can/sja1000/sja1000.c      */
+	/* The bittiming register command for the ICAN3 just sets the bit timing */
+	/* registers on the SJA1000 chip directly                                */
+	btr0 = ((bt->brp - 1) & 0x3f) | (((bt->sjw - 1) & 0x3) << 6);
+	btr1 = ((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) |
+		(((bt->phase_seg2 - 1) & 0x7) << 4);
+	if (mod->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		btr1 |= 0x80;
 
-	return ican3_send_msg(mod, &msg);
+	if (mod->fwtype == ICAN3_FWTYPE_ICANOS) {
+		if (on) {
+			/* set bittiming */
+			memset(&msg, 0, sizeof(msg));
+			msg.spec = MSG_CBTRREQ;
+			msg.len = cpu_to_le16(4);
+			msg.data[0] = 0x00;
+			msg.data[1] = 0x00;
+			msg.data[2] = btr0;
+			msg.data[3] = btr1;
+
+			res = ican3_send_msg(mod, &msg);
+			if (res)
+				return res;
+		}
+
+		/* can-on/off request */
+		memset(&msg, 0, sizeof(msg));
+		msg.spec = on ? MSG_CONREQ : MSG_COFFREQ;
+		msg.len = cpu_to_le16(0);
+
+		return ican3_send_msg(mod, &msg);
+
+	} else if (mod->fwtype == ICAN3_FWTYPE_CAL_CANOPEN) {
+		memset(&msg, 0, sizeof(msg));
+		msg.spec = MSG_LMTS;
+		if (on) {
+			msg.len = cpu_to_le16(4);
+			msg.data[0] = 0;
+			msg.data[1] = 0;
+			msg.data[2] = btr0;
+			msg.data[3] = btr1;
+		} else {
+			msg.len = cpu_to_le16(2);
+			msg.data[0] = 1;
+			msg.data[1] = 0;
+		}
+
+		return ican3_send_msg(mod, &msg);
+	}
+	return -ENOTSUPP;
 }
 
 static int ican3_set_termination(struct ican3_dev *mod, bool on)
@@ -1401,7 +1461,7 @@
 			return 0;
 
 		msleep(10);
-	} while (time_before(jiffies, start + HZ / 4));
+	} while (time_before(jiffies, start + HZ / 2));
 
 	netdev_err(mod->ndev, "failed to reset CAN module\n");
 	return -ETIMEDOUT;
@@ -1426,6 +1486,17 @@
 		return ret;
 	}
 
+	/* detect firmware */
+	memcpy_fromio(mod->fwinfo, mod->dpm + FIRMWARE_STAMP, sizeof(mod->fwinfo) - 1);
+	if (strncmp(mod->fwinfo, "JANZ-ICAN3", 10)) {
+		netdev_err(mod->ndev, "ICAN3 not detected (found %s)\n", mod->fwinfo);
+		return -ENODEV;
+	}
+	if (strstr(mod->fwinfo, "CAL/CANopen"))
+		mod->fwtype = ICAN3_FWTYPE_CAL_CANOPEN;
+	else
+		mod->fwtype = ICAN3_FWTYPE_ICANOS;
+
 	/* re-enable interrupts so we can send messages */
 	iowrite8(1 << mod->num, &mod->ctrl->int_enable);
 
@@ -1614,36 +1685,6 @@
 	.brp_inc = 1,
 };
 
-/*
- * This routine was stolen from drivers/net/can/sja1000/sja1000.c
- *
- * The bittiming register command for the ICAN3 just sets the bit timing
- * registers on the SJA1000 chip directly
- */
-static int ican3_set_bittiming(struct net_device *ndev)
-{
-	struct ican3_dev *mod = netdev_priv(ndev);
-	struct can_bittiming *bt = &mod->can.bittiming;
-	struct ican3_msg msg;
-	u8 btr0, btr1;
-
-	btr0 = ((bt->brp - 1) & 0x3f) | (((bt->sjw - 1) & 0x3) << 6);
-	btr1 = ((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) |
-		(((bt->phase_seg2 - 1) & 0x7) << 4);
-	if (mod->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
-		btr1 |= 0x80;
-
-	memset(&msg, 0, sizeof(msg));
-	msg.spec = MSG_CBTRREQ;
-	msg.len = cpu_to_le16(4);
-	msg.data[0] = 0x00;
-	msg.data[1] = 0x00;
-	msg.data[2] = btr0;
-	msg.data[3] = btr1;
-
-	return ican3_send_msg(mod, &msg);
-}
-
 static int ican3_set_mode(struct net_device *ndev, enum can_mode mode)
 {
 	struct ican3_dev *mod = netdev_priv(ndev);
@@ -1731,11 +1772,22 @@
 	return count;
 }
 
+static ssize_t ican3_sysfs_show_fwinfo(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	struct ican3_dev *mod = netdev_priv(to_net_dev(dev));
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", mod->fwinfo);
+}
+
 static DEVICE_ATTR(termination, S_IWUSR | S_IRUGO, ican3_sysfs_show_term,
 						   ican3_sysfs_set_term);
+static DEVICE_ATTR(fwinfo, S_IRUSR | S_IRUGO, ican3_sysfs_show_fwinfo, NULL);
 
 static struct attribute *ican3_sysfs_attrs[] = {
 	&dev_attr_termination.attr,
+	&dev_attr_fwinfo.attr,
 	NULL,
 };
 
@@ -1795,7 +1847,6 @@
 
 	mod->can.clock.freq = ICAN3_CAN_CLOCK;
 	mod->can.bittiming_const = &ican3_bittiming_const;
-	mod->can.do_set_bittiming = ican3_set_bittiming;
 	mod->can.do_set_mode = ican3_set_mode;
 	mod->can.do_get_berr_counter = ican3_get_berr_counter;
 	mod->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES
@@ -1867,7 +1918,7 @@
 		goto out_free_irq;
 	}
 
-	dev_info(dev, "module %d: registered CAN device\n", pdata->modno);
+	netdev_info(mod->ndev, "module %d: registered CAN device\n", pdata->modno);
 	return 0;
 
 out_free_irq: