i2c cleanup stuff etc, still untested

host/modules/dmj-char.c

@@ -280,7 +280,7 @@ static int __init dmj_char_init(void)
 	if (IS_ERR(class)) {
 		ret = PTR_ERR(class);
 		printk(KERN_ERR " failed to create class: %d\n", ret);
-		unregister_chrdev(major, DEVICE_NAME); /* TODO: unregister_chrdev_rage */
+		unregister_chrdev(major, DEVICE_NAME);
 		return ret;
 	}
 	printk(KERN_DEBUG DEVICE_NAME " created class\n");
@@ -301,7 +301,7 @@ static void __exit dmj_char_exit(void)
 	spin_unlock(&ndevs_lock);
 
 	class_destroy(dmj_char_class);
-	unregister_chrdev(MKDEV(dmj_char_major, 0), CLASS_NAME); /* TODO: unregister_chrdev_rage */
+	unregister_chrdev(dmj_char_major, CLASS_NAME);
 
 	dmj_char_major = -1;
 	dmj_char_class = NULL;

host/modules/dmj.c

@@ -286,24 +286,8 @@ static int dmj_check_hw(struct dmj_dev *dmj)
 
 	ret = dmj_xfer_internal(dmj, DMJ_CMD_CFG_GET_VERSION,
 			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &protover, &len);
-
+	ret = dmj_check_retval(ret, len, dev, "version check", true, sizeof(protover), sizeof(protover));
-	if (ret < 0) {
+	if (ret < 0) return ret;
-		dev_err(dev, "USB fail: %d\n", ret);
-		return ret;
-	}
-	if (ret) {
-		dev_err(dev, "USB protocol fail: %s (%d)\n", dmj_get_protoerr(ret), ret);
-		return -EIO;
-	}
-	if (len < sizeof(protover)) {
-		dev_err(dev, "USB fail remoteio: %d\n", len);
-		return -EREMOTEIO;
-	}
-	if (len > sizeof(protover)) {
-		dev_err(dev, "USB fail msgsize: %d\n", len);
-		return -EMSGSIZE;
-	}
-
 	if (le16_to_cpu(protover) != DMJ_USB_CFG_PROTO_VER) {
 		dev_err(&dmj->interface->dev, HARDWARE_NAME " config protocol version 0x%04x too %s\n",
 				le16_to_cpu(protover), (le16_to_cpu(protover) > DMJ_USB_CFG_PROTO_VER) ? "new" : "old");
@@ -325,12 +309,8 @@ static int dmj_print_info(struct dmj_dev *dmj)
 	len = sizeof(strinfo)-1;
 	ret = dmj_xfer_internal(dmj, DMJ_CMD_CFG_GET_INFOSTR,
 			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, strinfo, &len);
+	ret = dmj_check_retval(ret, len, dev, "get info", true, -1, sizeof(strinfo));
 	if (ret < 0) return ret;
-	if (ret) {
-		dev_err(dev, "USB protocol fail: %s (%d)\n", dmj_get_protoerr(ret), ret);
-		return -EIO;
-	}
-	if (len >= sizeof(strinfo)) return -EMSGSIZE;
 	strinfo[len] = 0; /*strinfo[64] = 0;*/
 	dev_info(dev, HARDWARE_NAME " '%s'\n", strinfo);
 
@@ -338,18 +318,16 @@ static int dmj_print_info(struct dmj_dev *dmj)
 	len = sizeof(curmode);
 	ret = dmj_xfer_internal(dmj, DMJ_CMD_CFG_GET_CUR_MODE,
 			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &curmode, &len);
+	ret = dmj_check_retval(ret, len, dev, "get info", true, sizeof(curmode), sizeof(curmode));
 	if (ret < 0) return ret;
-	if (len < sizeof(curmode)) return -EREMOTEIO;
-	if (len > sizeof(curmode)) return -EMSGSIZE;
 	dmj->dmj_mode = curmode;
 
 	/* map of available modes */
 	len = sizeof(modes);
 	ret = dmj_xfer_internal(dmj, DMJ_CMD_CFG_GET_MODES,
 			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &modes, &len);
+	ret = dmj_check_retval(ret, len, dev, "get info", true, sizeof(modes), sizeof(modes));
 	if (ret < 0) return ret;
-	if (len < sizeof(modes)) return -EREMOTEIO;
-	if (len > sizeof(modes)) return -EMSGSIZE;
 
 	for (i = 1; i < 16; ++i) { /* build the string, uglily */
 		if (le16_to_cpu(modes) & (1<<i)) {
@@ -368,6 +346,7 @@ static int dmj_print_info(struct dmj_dev *dmj)
 		len = sizeof(strinfo)-1;
 		ret = dmj_xfer_internal(dmj, (i<<4) | DMJ_CMD_MODE_GET_NAME,
 				DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, strinfo, &len);
+		ret = dmj_check_retval(ret, len, dev, "get info", true, -1, sizeof(strinfo));
 		if (ret < 0) return ret;
 		if (len >= sizeof(strinfo)) return -EMSGSIZE;
 		strinfo[len] = 0; /*strinfo[64] = 0;*/
@@ -376,17 +355,15 @@ static int dmj_print_info(struct dmj_dev *dmj)
 		len = sizeof(mversion);
 		ret = dmj_xfer_internal(dmj, (i<<4) | DMJ_CMD_MODE_GET_VERSION,
 				DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &mversion, &len);
+		ret = dmj_check_retval(ret, len, dev, "get info", true, sizeof(mversion), sizeof(mversion));
 		if (ret < 0) return ret;
-		if (len < sizeof(mversion)) return -EREMOTEIO;
-		if (len > sizeof(mversion)) return -EMSGSIZE;
 
 		/* features */
 		len = sizeof(features);
 		ret = dmj_xfer_internal(dmj, (i<<4) | DMJ_CMD_MODE_GET_FEATURES,
 				DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &features, &len);
+		ret = dmj_check_retval(ret, len, dev, "get info", true, sizeof(features), sizeof(features));
 		if (ret < 0) return ret;
-		if (len < sizeof(features)) return -EREMOTEIO;
-		if (len > sizeof(features)) return -EMSGSIZE;
 
 		if (i == 1) dmj->dmj_m1feature = features;
 

host/modules/dmj.h

@@ -48,6 +48,28 @@ inline static const char *dmj_get_protoerr(int err)
 	default: return "???";
 	}
 }
+inline static int dmj_check_retval(int ret, int len, struct device *dev,
+		const char *pfix, bool check_pos_val, int lmin, int lmax)
+{
+	if (ret < 0) {
+		dev_err(dev, "%s: USB fail: %d\n", pfix, ret);
+		return ret;
+	}
+	if (ret && check_pos_val) {
+		dev_err(dev, "%s: USB protocol fail: %s (%d)\n", pfix, dmj_get_protoerr(ret), ret);
+		return -EIO;
+	}
+	if (len < lmin && lmin >= 0) {
+		dev_err(dev, "%s: USB reply too short: %d\n", pfix, len);
+		return -EREMOTEIO;
+	}
+	if (len > lmax && lmax >= 0) {
+		dev_err(dev, "%s: USB reply too long: %d\n", pfix, len);
+		return -EMSGSIZE;
+	}
+
+	return 0;
+}
 
 int dmj_transfer(struct platform_device *pdev, int cmd, int recvflags,
 		const void *wbuf, int wbufsize, void *rbuf, int *rbufsize);

host/modules/i2c-dmj.c

@@ -42,7 +42,7 @@
 #define DMJ_I2C_STAT_ACK  1
 #define DMJ_I2C_STAT_NAK  2
 
-static unsigned short delay = 10;
+static uint16_t delay = 10;
 module_param(delay, ushort, 0);
 MODULE_PARM_DESC(delay, "bit delay in microseconds (default is 10us for 100kHz)");
 
@@ -72,14 +72,8 @@ static int dmj_i2c_read(struct dmj_i2c *dmji, struct i2c_msg *msg, int cmd)
 
 	ret = dmj_transfer(dmji->pdev, DMJ_CMD_MODE1_I2C, DMJ_XFER_FLAGS_PARSE_RESP,
 			cmdbuf, sizeof(cmdbuf), respbuf, &len);
-	if (ret < 0) {
+	ret = dmj_check_retval(ret, len, dev, "i2c read", true, -1, -1);
-		dev_err(dev, "read: USB comms error: %d\n", ret);
+	if (ret < 0) goto err_free;
-		goto err_free;
-	} else if (ret) {
-		dev_err(dev, "read: protocol error: %s (%d)\n", dmj_get_protoerr(ret), ret);
-		ret = -EIO;
-		goto err_free;
-	}
 
 	memcpy(msg->buf, respbuf, msg->len);
 	kfree(respbuf);
@@ -109,14 +103,8 @@ static int dmj_i2c_write(struct dmj_i2c *dmji, struct i2c_msg *msg, int cmd)
 	memcpy(&cmdbuf[7], msg->buf, msg->len);
 
 	ret = dmj_write(dmji->pdev, DMJ_CMD_MODE1_I2C, cmdbuf, len);
-	if (ret < 0) {
+	ret = dmj_check_retval(ret, len, dev, "i2c write", true, -1, -1);
-		dev_err(dev, "write: USB comms error: %d\n", ret);
+	if (ret < 0) goto err_free;
-		goto err_free;
-	} else if (ret) {
-		dev_err(dev, "write: protocol error: %s (%d)\n", dmj_get_protoerr(ret), ret);
-		ret = -EIO;
-		goto err_free;
-	}
 
 	kfree(cmdbuf);
 	return msg->len;
@@ -141,7 +129,7 @@ static int dmj_i2c_xfer(struct i2c_adapter *a, struct i2c_msg *msgs, int nmsg)
 
 		pmsg = &msgs[i];
 
-		dev_dbg(&a->dev,
+		dev_warn(&a->dev,
 			"  %d: %s (flags %04x) %d bytes to 0x%02x\n",
 			i, pmsg->flags & I2C_M_RD ? "read" : "write",
 			pmsg->flags, pmsg->len, pmsg->addr);
@@ -169,20 +157,10 @@ static int dmj_i2c_xfer(struct i2c_adapter *a, struct i2c_msg *msgs, int nmsg)
 		stlen = sizeof(status);
 		ret = dmj_transfer(dmji->pdev, DMJ_CMD_MODE1_I2C, DMJ_XFER_FLAGS_PARSE_RESP,
 				&i2ccmd, sizeof(i2ccmd), &status, &stlen);
-		if (ret < 0) {
+		ret = dmj_check_retval(ret, stlen, dev, "i2c stat", true, sizeof(status), sizeof(status));
-			dev_err(dev, "xfer get stat: USB comms error: %d\n", ret);
+		if (ret < 0) goto err_ret;
-			goto err_ret;
-		} else if (ret) {
-			dev_err(dev, "xfer get stat: protocol error: %s (%d)\n", dmj_get_protoerr(ret), ret);
-			ret = -EIO;
-			goto err_ret;
-		} else if (stlen != sizeof(status)) {
-			dev_err(dev, "xfer get stat: unexpected return length: want %zu, got %d\n", sizeof(status), stlen);
-			ret = -EMSGSIZE;
-			goto err_ret;
-		}
 
-		dev_dbg(dev, "  status = %d\n", status);
+		dev_warn(dev, "  status = %d\n", status);
 		if (status == DMJ_I2C_STAT_NAK) {
 			ret = -ENXIO;
 			goto err_ret;
@@ -205,16 +183,8 @@ static uint32_t dmj_i2c_func(struct i2c_adapter *a)
 
 	ret = dmj_transfer(dmji->pdev, DMJ_CMD_MODE1_I2C, DMJ_XFER_FLAGS_PARSE_RESP,
 			&i2ccmd, sizeof(i2ccmd), &func, &len);
-	if (ret < 0) {
+	ret = dmj_check_retval(ret, len, dev, "i2c get_func", true, sizeof(func), sizeof(func));
-		dev_err(dev, "func: USB comms error: %d\n", ret);
+	if (ret < 0) return 0;
-		return 0;
-	} else if (ret) {
-		dev_err(dev, "func: protocol error: %s (%d)\n", dmj_get_protoerr(ret), ret);
-		return 0;
-	} else if (len != sizeof(func)) {
-		dev_err(dev, "func: unexpected return length: want %zu, got %d\n", sizeof(func), len);
-		return 0;
-	}
 
 	dev_warn(dev, "I2C functionality: 0x%08x\n", le32_to_cpu(func));
 
@@ -230,15 +200,100 @@ static const struct i2c_adapter_quirks dmj_i2c_quirks = {
 	.max_write_len = DMJ_I2C_MAX_XSFER_SIZE,
 };
 
+static int dmj_i2c_check_hw(struct platform_device *pdev)
+{
+	/*
+	 * 1. check if mode 1 is available
+	 * 2. check mode 1 version
+	 * 3. check if mode 1 has the I2C feature
+	 * 4. test the echo I2C command
+	 */
+
+	struct device *dev = &pdev->dev;
+	__le16 m1ver;
+	uint8_t curmode, m1feat, echoval;
+	const int ver_min = 0x0010, ver_max = 0x0010;
+	uint8_t i2ccmd[2];
+	int ret = 0, len;
+
+	len = sizeof(curmode);
+	ret = dmj_transfer(pdev, DMJ_CMD_CFG_GET_CUR_MODE,
+			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &curmode, &len);
+	ret = dmj_check_retval(ret, len, dev, "i2c test", true, sizeof(curmode), sizeof(curmode));
+	if (ret < 0) return ret;
+	if (curmode != 0x1) {
+		dev_err(dev, "device must be in mode 1 for ICD to work, but it is in mode %d\n", curmode);
+		return -EIO;
+	}
+
+	len = sizeof(m1ver);
+	ret = dmj_transfer(pdev, (1<<4) | DMJ_CMD_MODE_GET_VERSION,
+			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &m1ver, &len);
+	ret = dmj_check_retval(ret, len, dev, "i2c test", true, sizeof(m1ver), sizeof(m1ver));
+	if (ret < 0) return ret;
+	if (le16_to_cpu(m1ver) > ver_max || le16_to_cpu(m1ver) < ver_min) {
+		dev_err(dev, "bad mode 1 version %04x on device, must be between %04x and %04x\n",
+				le16_to_cpu(m1ver), ver_min, ver_max);
+		return -EIO;
+	}
+
+	len = sizeof(m1feat);
+	ret = dmj_transfer(pdev, (1<<4) | DMJ_CMD_MODE_GET_FEATURES,
+			DMJ_XFER_FLAGS_PARSE_RESP, NULL, 0, &m1feat, &len);
+	ret = dmj_check_retval(ret, len, dev, "i2c test", true, sizeof(m1feat), sizeof(m1feat));
+	if (ret < 0) return ret;
+	if (!(m1feat & DMJ_FEATURE_MODE1_I2C)) {
+		dev_err(dev, "device's mode 1 does not support I2C\n");
+		return -EIO;
+	}
+
+	echoval = 0x42;
+	i2ccmd[0] = DMJ_I2C_CMD_ECHO;
+	i2ccmd[1] = ~echoval;
+	len = sizeof(echoval);
+	ret = dmj_transfer(pdev, (1<<4) | DMJ_CMD_MODE1_I2C,
+			DMJ_XFER_FLAGS_PARSE_RESP, i2ccmd, sizeof(i2ccmd), &m1feat, &len);
+	ret = dmj_check_retval(ret, len, dev, "i2c test", true, sizeof(m1feat), sizeof(m1feat));
+	if (ret < 0) return ret;
+	if (echoval != i2ccmd[1]) {
+		dev_err(dev, "I2C echo test command not functional\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int dmj_i2c_set_delay(struct platform_device *pdev, uint16_t us)
+{
+	struct device *dev = &pdev->dev;
+	uint8_t i2ccmd[3];
+	int ret = 0;
+
+	i2ccmd[0] = DMJ_I2C_CMD_SET_DELAY;
+	i2ccmd[1] = (us >> 0) & 0xff;
+	i2ccmd[2] = (us >> 8) & 0xff;
+
+	ret = dmj_transfer(pdev, (1<<4) | DMJ_CMD_MODE1_I2C, DMJ_XFER_FLAGS_PARSE_RESP,
+			i2ccmd, sizeof(i2ccmd), NULL, NULL);
+	ret = dmj_check_retval(ret, -1, dev, "i2c set delay", true, -1, -1);
+
+	return ret;
+}
+
 static int dmj_i2c_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct dmj_i2c *dmji;
 	struct device *dev = &pdev->dev;
 
-	// TODO: check if mode 1 and I2C available?
+	ret = dmj_i2c_check_hw(pdev);
+	if (ret) return -ENODEV;
 
-	// TODO: test ECHO cmd
+	ret = dmj_i2c_set_delay(pdev, delay);
+	if (ret) {
+		dev_err(dev, "failed to set I2C speed: %d\n", ret);
+		return ret;
+	}
 
 	dmji = devm_kzalloc(dev, sizeof(*dmji), GFP_KERNEL);
 	if (!dmji) return -ENOMEM;
@@ -252,8 +307,6 @@ static int dmj_i2c_probe(struct platform_device *pdev)
 	dmji->adapter.dev.of_node = dev->of_node;
 	i2c_set_adapdata(&dmji->adapter, dmji);
 
-	// TODO: set delay from module param
-
 	snprintf(dmji->adapter.name, sizeof(dmji->adapter.name), "%s-%s",
 		"dln2-i2c", dev_name(pdev->dev.parent));