Patchwork i2c-tiny-usb: do not use stack as URB transfer_buffer

login
register
mail settings
Submitter Jussi Kivilinna
Date Aug. 6, 2013, 11:17 a.m.
Message ID <20130806111742.25265.94046.stgit@localhost6.localdomain6>
Download mbox | patch
Permalink /patch/265022/
State Accepted
Headers show

Comments

Jussi Kivilinna - Aug. 6, 2013, 11:17 a.m.
Patch fixes i2c-tiny-usb not to use stack as URB transfer_buffer. URB buffers
need to be DMA-able, which stack is not.

Patch is only compile tested.

Cc: stable@vger.kernel.org
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
 drivers/i2c/busses/i2c-tiny-usb.c |   49 ++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 15 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - Aug. 15, 2013, 5:55 p.m.
On Tue, Aug 06, 2013 at 02:17:42PM +0300, Jussi Kivilinna wrote:
> Patch fixes i2c-tiny-usb not to use stack as URB transfer_buffer. URB buffers
> need to be DMA-able, which stack is not.
> 
> Patch is only compile tested.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>

Looks OK to me. Till, are you there?
Jussi Kivilinna - Aug. 15, 2013, 7:46 p.m.
On 15.08.2013 20:55, Wolfram Sang wrote:
> On Tue, Aug 06, 2013 at 02:17:42PM +0300, Jussi Kivilinna wrote:
>> Patch fixes i2c-tiny-usb not to use stack as URB transfer_buffer. URB buffers
>> need to be DMA-able, which stack is not.
>>
>> Patch is only compile tested.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> 
> Looks OK to me. Till, are you there?

That cc to stable should probably be removed if patch is not tested on hw.

-Jussi

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - Aug. 19, 2013, 5:47 p.m.
On Thu, Aug 15, 2013 at 10:46:32PM +0300, Jussi Kivilinna wrote:
> On 15.08.2013 20:55, Wolfram Sang wrote:
> > On Tue, Aug 06, 2013 at 02:17:42PM +0300, Jussi Kivilinna wrote:
> >> Patch fixes i2c-tiny-usb not to use stack as URB transfer_buffer. URB buffers
> >> need to be DMA-able, which stack is not.
> >>
> >> Patch is only compile tested.
> >>
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> > 
> > Looks OK to me. Till, are you there?
> 
> That cc to stable should probably be removed if patch is not tested on hw.

Applied to for-next, thanks! And removed stable for now...

Patch

diff --git a/drivers/i2c/busses/i2c-tiny-usb.c b/drivers/i2c/busses/i2c-tiny-usb.c
index 0510636..e7d3b75 100644
--- a/drivers/i2c/busses/i2c-tiny-usb.c
+++ b/drivers/i2c/busses/i2c-tiny-usb.c
@@ -54,12 +54,16 @@  static int usb_write(struct i2c_adapter *adapter, int cmd,
 
 static int usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
 {
-	unsigned char status;
+	unsigned char *pstatus;
 	struct i2c_msg *pmsg;
-	int i;
+	int i, ret;
 
 	dev_dbg(&adapter->dev, "master xfer %d messages:\n", num);
 
+	pstatus = kmalloc(sizeof(*pstatus), GFP_KERNEL);
+	if (!pstatus)
+		return -ENOMEM;
+
 	for (i = 0 ; i < num ; i++) {
 		int cmd = CMD_I2C_IO;
 
@@ -84,7 +88,8 @@  static int usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
 				     pmsg->buf, pmsg->len) != pmsg->len) {
 				dev_err(&adapter->dev,
 					"failure reading data\n");
-				return -EREMOTEIO;
+				ret = -EREMOTEIO;
+				goto out;
 			}
 		} else {
 			/* write data */
@@ -93,36 +98,50 @@  static int usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
 				      pmsg->buf, pmsg->len) != pmsg->len) {
 				dev_err(&adapter->dev,
 					"failure writing data\n");
-				return -EREMOTEIO;
+				ret = -EREMOTEIO;
+				goto out;
 			}
 		}
 
 		/* read status */
-		if (usb_read(adapter, CMD_GET_STATUS, 0, 0, &status, 1) != 1) {
+		if (usb_read(adapter, CMD_GET_STATUS, 0, 0, pstatus, 1) != 1) {
 			dev_err(&adapter->dev, "failure reading status\n");
-			return -EREMOTEIO;
+			ret = -EREMOTEIO;
+			goto out;
 		}
 
-		dev_dbg(&adapter->dev, "  status = %d\n", status);
-		if (status == STATUS_ADDRESS_NAK)
-			return -EREMOTEIO;
+		dev_dbg(&adapter->dev, "  status = %d\n", *pstatus);
+		if (*pstatus == STATUS_ADDRESS_NAK) {
+			ret = -EREMOTEIO;
+			goto out;
+		}
 	}
 
-	return i;
+	ret = i;
+out:
+	kfree(pstatus);
+	return ret;
 }
 
 static u32 usb_func(struct i2c_adapter *adapter)
 {
-	__le32 func;
+	__le32 *pfunc;
+	u32 ret;
+
+	pfunc = kmalloc(sizeof(*pfunc), GFP_KERNEL);
 
 	/* get functionality from adapter */
-	if (usb_read(adapter, CMD_GET_FUNC, 0, 0, &func, sizeof(func)) !=
-	    sizeof(func)) {
+	if (!pfunc || usb_read(adapter, CMD_GET_FUNC, 0, 0, pfunc,
+			       sizeof(*pfunc)) != sizeof(*pfunc)) {
 		dev_err(&adapter->dev, "failure reading functionality\n");
-		return 0;
+		ret = 0;
+		goto out;
 	}
 
-	return le32_to_cpu(func);
+	ret = le32_to_cpup(pfunc);
+out:
+	kfree(pfunc);
+	return ret;
 }
 
 /* This is the actual algorithm we define */