* [PATCH] i2c: i2c-tiny-usb: fix buffer not being DMA capable
@ 2017-05-04 12:56 Sebastian Reichel
2017-05-04 15:27 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Reichel @ 2017-05-04 12:56 UTC (permalink / raw)
To: Sebastian Reichel, Till Harbaum, Wolfram Sang
Cc: linux-i2c, linux-kernel, Sebastian Reichel, # Kernel 4 . 9+
The i2c-tiny-usb driver generates the following trace and fails to
read or write data on a v4.9 or newer kernels. This patch avoids the
problem by allocating a DMA capable buffer for data transfer.
[ 17.504959] ------------[ cut here ]------------
[ 17.505488] WARNING: CPU: 0 PID: 93 at drivers/usb/core/hcd.c:1587 usb_hcd_map_urb_for_dma+0x37c/0x570
[ 17.506545] transfer buffer not dma capable
[ 17.507022] Modules linked in:
[ 17.507370] CPU: 0 PID: 93 Comm: i2cdetect Not tainted 4.11.0-rc8+ #10
[ 17.508103] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 17.509039] Call Trace:
[ 17.509320] ? dump_stack+0x5c/0x78
[ 17.509714] ? __warn+0xbe/0xe0
[ 17.510073] ? warn_slowpath_fmt+0x5a/0x80
[ 17.510532] ? nommu_map_sg+0xb0/0xb0
[ 17.510949] ? usb_hcd_map_urb_for_dma+0x37c/0x570
[ 17.511482] ? usb_hcd_submit_urb+0x336/0xab0
[ 17.511976] ? wait_for_completion_timeout+0x12f/0x1a0
[ 17.512549] ? wait_for_completion_timeout+0x65/0x1a0
[ 17.513125] ? usb_start_wait_urb+0x65/0x160
[ 17.513604] ? usb_control_msg+0xdc/0x130
[ 17.514061] ? usb_xfer+0xa4/0x2a0
[ 17.514445] ? __i2c_transfer+0x108/0x3c0
[ 17.514899] ? i2c_transfer+0x57/0xb0
[ 17.515310] ? i2c_smbus_xfer_emulated+0x12f/0x590
[ 17.515851] ? _raw_spin_unlock_irqrestore+0x11/0x20
[ 17.516408] ? i2c_smbus_xfer+0x125/0x330
[ 17.516876] ? i2c_smbus_xfer+0x125/0x330
[ 17.517329] ? i2cdev_ioctl_smbus+0x1c1/0x2b0
[ 17.517824] ? i2cdev_ioctl+0x75/0x1c0
[ 17.518248] ? do_vfs_ioctl+0x9f/0x600
[ 17.518671] ? vfs_write+0x144/0x190
[ 17.519078] ? SyS_ioctl+0x74/0x80
[ 17.519463] ? entry_SYSCALL_64_fastpath+0x1e/0xad
[ 17.519959] ---[ end trace d047c04982f5ac50 ]---
Cc: <stable@vger.kernel.org> # Kernel 4.9+
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
drivers/i2c/busses/i2c-tiny-usb.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tiny-usb.c b/drivers/i2c/busses/i2c-tiny-usb.c
index 0ed77eeff31e..ca30a1a93d29 100644
--- a/drivers/i2c/busses/i2c-tiny-usb.c
+++ b/drivers/i2c/busses/i2c-tiny-usb.c
@@ -178,22 +178,33 @@ static int usb_read(struct i2c_adapter *adapter, int cmd,
int value, int index, void *data, int len)
{
struct i2c_tiny_usb *dev = (struct i2c_tiny_usb *)adapter->algo_data;
+ void *dmadata = kmalloc(len, GFP_KERNEL);
+ int ret;
/* do control transfer */
- return usb_control_msg(dev->usb_dev, usb_rcvctrlpipe(dev->usb_dev, 0),
+ ret = usb_control_msg(dev->usb_dev, usb_rcvctrlpipe(dev->usb_dev, 0),
cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
- USB_DIR_IN, value, index, data, len, 2000);
+ USB_DIR_IN, value, index, dmadata, len, 2000);
+
+ memcpy(data, dmadata, len);
+ kfree(dmadata);
+ return ret;
}
static int usb_write(struct i2c_adapter *adapter, int cmd,
int value, int index, void *data, int len)
{
struct i2c_tiny_usb *dev = (struct i2c_tiny_usb *)adapter->algo_data;
+ void *dmadata = kmemdup(data, len, GFP_KERNEL);
+ int ret;
/* do control transfer */
- return usb_control_msg(dev->usb_dev, usb_sndctrlpipe(dev->usb_dev, 0),
+ ret = usb_control_msg(dev->usb_dev, usb_sndctrlpipe(dev->usb_dev, 0),
cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
- value, index, data, len, 2000);
+ value, index, dmadata, len, 2000);
+
+ kfree(dmadata);
+ return ret;
}
static void i2c_tiny_usb_free(struct i2c_tiny_usb *dev)
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] i2c: i2c-tiny-usb: fix buffer not being DMA capable
2017-05-04 12:56 [PATCH] i2c: i2c-tiny-usb: fix buffer not being DMA capable Sebastian Reichel
@ 2017-05-04 15:27 ` Greg KH
2017-05-04 15:50 ` Sebastian Reichel
0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2017-05-04 15:27 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sebastian Reichel, Till Harbaum, Wolfram Sang, linux-i2c,
linux-kernel, # Kernel 4 . 9+
On Thu, May 04, 2017 at 02:56:32PM +0200, Sebastian Reichel wrote:
> The i2c-tiny-usb driver generates the following trace and fails to
> read or write data on a v4.9 or newer kernels. This patch avoids the
> problem by allocating a DMA capable buffer for data transfer.
>
> [ 17.504959] ------------[ cut here ]------------
> [ 17.505488] WARNING: CPU: 0 PID: 93 at drivers/usb/core/hcd.c:1587 usb_hcd_map_urb_for_dma+0x37c/0x570
> [ 17.506545] transfer buffer not dma capable
> [ 17.507022] Modules linked in:
> [ 17.507370] CPU: 0 PID: 93 Comm: i2cdetect Not tainted 4.11.0-rc8+ #10
> [ 17.508103] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [ 17.509039] Call Trace:
> [ 17.509320] ? dump_stack+0x5c/0x78
> [ 17.509714] ? __warn+0xbe/0xe0
> [ 17.510073] ? warn_slowpath_fmt+0x5a/0x80
> [ 17.510532] ? nommu_map_sg+0xb0/0xb0
> [ 17.510949] ? usb_hcd_map_urb_for_dma+0x37c/0x570
> [ 17.511482] ? usb_hcd_submit_urb+0x336/0xab0
> [ 17.511976] ? wait_for_completion_timeout+0x12f/0x1a0
> [ 17.512549] ? wait_for_completion_timeout+0x65/0x1a0
> [ 17.513125] ? usb_start_wait_urb+0x65/0x160
> [ 17.513604] ? usb_control_msg+0xdc/0x130
> [ 17.514061] ? usb_xfer+0xa4/0x2a0
> [ 17.514445] ? __i2c_transfer+0x108/0x3c0
> [ 17.514899] ? i2c_transfer+0x57/0xb0
> [ 17.515310] ? i2c_smbus_xfer_emulated+0x12f/0x590
> [ 17.515851] ? _raw_spin_unlock_irqrestore+0x11/0x20
> [ 17.516408] ? i2c_smbus_xfer+0x125/0x330
> [ 17.516876] ? i2c_smbus_xfer+0x125/0x330
> [ 17.517329] ? i2cdev_ioctl_smbus+0x1c1/0x2b0
> [ 17.517824] ? i2cdev_ioctl+0x75/0x1c0
> [ 17.518248] ? do_vfs_ioctl+0x9f/0x600
> [ 17.518671] ? vfs_write+0x144/0x190
> [ 17.519078] ? SyS_ioctl+0x74/0x80
> [ 17.519463] ? entry_SYSCALL_64_fastpath+0x1e/0xad
> [ 17.519959] ---[ end trace d047c04982f5ac50 ]---
>
> Cc: <stable@vger.kernel.org> # Kernel 4.9+
This has been a USB core requirement since the 2.2 days, it's only since
4.9 that most people have started to notice this. So backporting it to
anything that it can go to is best to have happen :)
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
> drivers/i2c/busses/i2c-tiny-usb.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-tiny-usb.c b/drivers/i2c/busses/i2c-tiny-usb.c
> index 0ed77eeff31e..ca30a1a93d29 100644
> --- a/drivers/i2c/busses/i2c-tiny-usb.c
> +++ b/drivers/i2c/busses/i2c-tiny-usb.c
> @@ -178,22 +178,33 @@ static int usb_read(struct i2c_adapter *adapter, int cmd,
> int value, int index, void *data, int len)
> {
> struct i2c_tiny_usb *dev = (struct i2c_tiny_usb *)adapter->algo_data;
> + void *dmadata = kmalloc(len, GFP_KERNEL);
No error check to see if this worked or not?
> + int ret;
>
> /* do control transfer */
> - return usb_control_msg(dev->usb_dev, usb_rcvctrlpipe(dev->usb_dev, 0),
> + ret = usb_control_msg(dev->usb_dev, usb_rcvctrlpipe(dev->usb_dev, 0),
> cmd, USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
> - USB_DIR_IN, value, index, data, len, 2000);
> + USB_DIR_IN, value, index, dmadata, len, 2000);
> +
> + memcpy(data, dmadata, len);
> + kfree(dmadata);
> + return ret;
> }
>
> static int usb_write(struct i2c_adapter *adapter, int cmd,
> int value, int index, void *data, int len)
> {
> struct i2c_tiny_usb *dev = (struct i2c_tiny_usb *)adapter->algo_data;
> + void *dmadata = kmemdup(data, len, GFP_KERNEL);
No error checking?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] i2c: i2c-tiny-usb: fix buffer not being DMA capable
2017-05-04 15:27 ` Greg KH
@ 2017-05-04 15:50 ` Sebastian Reichel
0 siblings, 0 replies; 3+ messages in thread
From: Sebastian Reichel @ 2017-05-04 15:50 UTC (permalink / raw)
To: Greg KH
Cc: Till Harbaum, Wolfram Sang, linux-i2c, linux-kernel, # Kernel 4 . 9+
[-- Attachment #1: Type: text/plain, Size: 2917 bytes --]
Hi Greg,
On Thu, May 04, 2017 at 08:27:09AM -0700, Greg KH wrote:
> On Thu, May 04, 2017 at 02:56:32PM +0200, Sebastian Reichel wrote:
> > The i2c-tiny-usb driver generates the following trace and fails to
> > read or write data on a v4.9 or newer kernels. This patch avoids the
> > problem by allocating a DMA capable buffer for data transfer.
> >
> > [ 17.504959] ------------[ cut here ]------------
> > [ 17.505488] WARNING: CPU: 0 PID: 93 at drivers/usb/core/hcd.c:1587 usb_hcd_map_urb_for_dma+0x37c/0x570
> > [ 17.506545] transfer buffer not dma capable
> > [ 17.507022] Modules linked in:
> > [ 17.507370] CPU: 0 PID: 93 Comm: i2cdetect Not tainted 4.11.0-rc8+ #10
> > [ 17.508103] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> > [ 17.509039] Call Trace:
> > [ 17.509320] ? dump_stack+0x5c/0x78
> > [ 17.509714] ? __warn+0xbe/0xe0
> > [ 17.510073] ? warn_slowpath_fmt+0x5a/0x80
> > [ 17.510532] ? nommu_map_sg+0xb0/0xb0
> > [ 17.510949] ? usb_hcd_map_urb_for_dma+0x37c/0x570
> > [ 17.511482] ? usb_hcd_submit_urb+0x336/0xab0
> > [ 17.511976] ? wait_for_completion_timeout+0x12f/0x1a0
> > [ 17.512549] ? wait_for_completion_timeout+0x65/0x1a0
> > [ 17.513125] ? usb_start_wait_urb+0x65/0x160
> > [ 17.513604] ? usb_control_msg+0xdc/0x130
> > [ 17.514061] ? usb_xfer+0xa4/0x2a0
> > [ 17.514445] ? __i2c_transfer+0x108/0x3c0
> > [ 17.514899] ? i2c_transfer+0x57/0xb0
> > [ 17.515310] ? i2c_smbus_xfer_emulated+0x12f/0x590
> > [ 17.515851] ? _raw_spin_unlock_irqrestore+0x11/0x20
> > [ 17.516408] ? i2c_smbus_xfer+0x125/0x330
> > [ 17.516876] ? i2c_smbus_xfer+0x125/0x330
> > [ 17.517329] ? i2cdev_ioctl_smbus+0x1c1/0x2b0
> > [ 17.517824] ? i2cdev_ioctl+0x75/0x1c0
> > [ 17.518248] ? do_vfs_ioctl+0x9f/0x600
> > [ 17.518671] ? vfs_write+0x144/0x190
> > [ 17.519078] ? SyS_ioctl+0x74/0x80
> > [ 17.519463] ? entry_SYSCALL_64_fastpath+0x1e/0xad
> > [ 17.519959] ---[ end trace d047c04982f5ac50 ]---
> >
> > Cc: <stable@vger.kernel.org> # Kernel 4.9+
>
> This has been a USB core requirement since the 2.2 days, it's only since
> 4.9 that most people have started to notice this. So backporting it to
> anything that it can go to is best to have happen :)
I intentionally did not add a "Fixes:" for the commit in 4.9, since
I assumed that to be the case (did not bisect to the commit either).
OTOH the driver worked before v4.9, so I thought I would reduce your
(and the other backporters) workload - users won't care.
Looks like it added more work now, so I will drop the comment in
PATCHv2 and leave it to stable porters figuring out if they want
to backport it.
> > + void *dmadata = kmalloc(len, GFP_KERNEL);
> > + void *dmadata = kmemdup(data, len, GFP_KERNEL);
>
> No error checking?
Coming in PATCHv2.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-04 15:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 12:56 [PATCH] i2c: i2c-tiny-usb: fix buffer not being DMA capable Sebastian Reichel
2017-05-04 15:27 ` Greg KH
2017-05-04 15:50 ` Sebastian Reichel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).