* [PATCH 0/4] HID: fix few non-DMA capable HID transfers
@ 2016-11-21 10:48 Benjamin Tissoires
2016-11-21 10:48 ` [PATCH 1/4] HID: cp2112: make transfer buffers DMA capable Benjamin Tissoires
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Benjamin Tissoires @ 2016-11-21 10:48 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
Hi Jiri,
While playing a little bit with the CP2112 (for hid-rmi I must confess), I
realized that kernel v4.9 now enforces DMA capable buffers when calling
hid_hw_raw_request(). Kernel v4.8 works fine (but gives a stacktrace the first
time), but v4.9 doesn't.
So I gave a check of all the other drivers in the HID tree, and it looks like
only 4 drivers are not properly allocating their buffers.
I'd say this is v4.9 material but I was not able to test magicmouse and lg.
If this doesn't qualifies for 4.9-rc7, I think we should add the stable@ stamp,
given that the chance this will break hid-rmi is huge (cp2112 is not so much an
issue, given it's a devel board).
Cheers,
Benjamin
Benjamin Tissoires (4):
HID: cp2112: make transfer buffers DMA capable
HID: lg: make transfer buffers DMA capable
HID: magicmouse: make transfer buffers DMA capable
HID: rmi: make transfer buffers DMA capable
drivers/hid/hid-cp2112.c | 115 +++++++++++++++++++++++++++++--------------
drivers/hid/hid-lg.c | 12 +++--
drivers/hid/hid-magicmouse.c | 12 ++++-
drivers/hid/hid-rmi.c | 10 +++-
4 files changed, 106 insertions(+), 43 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/4] HID: cp2112: make transfer buffers DMA capable 2016-11-21 10:48 [PATCH 0/4] HID: fix few non-DMA capable HID transfers Benjamin Tissoires @ 2016-11-21 10:48 ` Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 2/4] HID: lg: " Benjamin Tissoires ` (3 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Benjamin Tissoires @ 2016-11-21 10:48 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, linux-kernel Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Use a spinlock to prevent concurrent accesses to the buffer. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-cp2112.c | 115 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 79 insertions(+), 36 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 086d8a5..60d3020 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -32,6 +32,11 @@ #include <linux/usb/ch9.h> #include "hid-ids.h" +#define CP2112_REPORT_MAX_LENGTH 64 +#define CP2112_GPIO_CONFIG_LENGTH 5 +#define CP2112_GPIO_GET_LENGTH 2 +#define CP2112_GPIO_SET_LENGTH 3 + enum { CP2112_GPIO_CONFIG = 0x02, CP2112_GPIO_GET = 0x03, @@ -161,6 +166,8 @@ struct cp2112_device { atomic_t read_avail; atomic_t xfer_avail; struct gpio_chip gc; + u8 *in_out_buffer; + spinlock_t lock; }; static int gpio_push_pull = 0xFF; @@ -171,62 +178,86 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[5]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; + spin_lock_irqsave(&dev->lock, flags); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, - sizeof(buf), HID_FEATURE_REPORT, - HID_REQ_GET_REPORT); - if (ret != sizeof(buf)) { + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_GET_REPORT); + if (ret != CP2112_GPIO_CONFIG_LENGTH) { hid_err(hdev, "error requesting GPIO config: %d\n", ret); - return ret; + goto exit; } buf[1] &= ~(1 << offset); buf[2] = gpio_push_pull; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); if (ret < 0) { hid_err(hdev, "error setting GPIO config: %d\n", ret); - return ret; + goto exit; } - return 0; + ret = 0; + +exit: + spin_unlock_irqrestore(&dev->lock, flags); + return ret <= 0 ? ret : -EIO; } static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value) { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[3]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; + spin_lock_irqsave(&dev->lock, flags); + buf[0] = CP2112_GPIO_SET; buf[1] = value ? 0xff : 0; buf[2] = 1 << offset; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_SET, buf, + CP2112_GPIO_SET_LENGTH, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); if (ret < 0) hid_err(hdev, "error setting GPIO values: %d\n", ret); + + spin_unlock_irqrestore(&dev->lock, flags); } static int cp2112_gpio_get(struct gpio_chip *chip, unsigned offset) { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[2]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_GET, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_GET_REPORT); - if (ret != sizeof(buf)) { + spin_lock_irqsave(&dev->lock, flags); + + ret = hid_hw_raw_request(hdev, CP2112_GPIO_GET, buf, + CP2112_GPIO_GET_LENGTH, HID_FEATURE_REPORT, + HID_REQ_GET_REPORT); + if (ret != CP2112_GPIO_GET_LENGTH) { hid_err(hdev, "error requesting GPIO values: %d\n", ret); - return ret; + ret = ret < 0 ? ret : -EIO; + goto exit; } - return (buf[1] >> offset) & 1; + ret = (buf[1] >> offset) & 1; + +exit: + spin_unlock_irqrestore(&dev->lock, flags); + + return ret; } static int cp2112_gpio_direction_output(struct gpio_chip *chip, @@ -234,27 +265,33 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, { struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; - u8 buf[5]; + u8 *buf = dev->in_out_buffer; + unsigned long flags; int ret; + spin_lock_irqsave(&dev->lock, flags); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, - sizeof(buf), HID_FEATURE_REPORT, - HID_REQ_GET_REPORT); - if (ret != sizeof(buf)) { + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_GET_REPORT); + if (ret != CP2112_GPIO_CONFIG_LENGTH) { hid_err(hdev, "error requesting GPIO config: %d\n", ret); - return ret; + goto fail; } buf[1] |= 1 << offset; buf[2] = gpio_push_pull; - ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, + CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); if (ret < 0) { hid_err(hdev, "error setting GPIO config: %d\n", ret); - return ret; + goto fail; } + spin_unlock_irqrestore(&dev->lock, flags); + /* * Set gpio value when output direction is already set, * as specified in AN495, Rev. 0.2, cpt. 4.4 @@ -262,6 +299,10 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, cp2112_gpio_set(chip, offset, value); return 0; + +fail: + spin_unlock_irqrestore(&dev->lock, flags); + return ret < 0 ? ret : -EIO; } static int cp2112_hid_get(struct hid_device *hdev, unsigned char report_number, @@ -1007,6 +1048,17 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) struct cp2112_smbus_config_report config; int ret; + dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL); + if (!dev) + return -ENOMEM; + + dev->in_out_buffer = devm_kzalloc(&hdev->dev, CP2112_REPORT_MAX_LENGTH, + GFP_KERNEL); + if (!dev->in_out_buffer) + return -ENOMEM; + + spin_lock_init(&dev->lock); + ret = hid_parse(hdev); if (ret) { hid_err(hdev, "parse failed\n"); @@ -1063,12 +1115,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) goto err_power_normal; } - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) { - ret = -ENOMEM; - goto err_power_normal; - } - hid_set_drvdata(hdev, (void *)dev); dev->hdev = hdev; dev->adap.owner = THIS_MODULE; @@ -1087,7 +1133,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) if (ret) { hid_err(hdev, "error registering i2c adapter\n"); - goto err_free_dev; + goto err_power_normal; } hid_dbg(hdev, "adapter registered\n"); @@ -1123,8 +1169,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) gpiochip_remove(&dev->gc); err_free_i2c: i2c_del_adapter(&dev->adap); -err_free_dev: - kfree(dev); err_power_normal: hid_hw_power(hdev, PM_HINT_NORMAL); err_hid_close: @@ -1149,7 +1193,6 @@ static void cp2112_remove(struct hid_device *hdev) */ hid_hw_close(hdev); hid_hw_stop(hdev); - kfree(dev); } static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report, -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] HID: lg: make transfer buffers DMA capable 2016-11-21 10:48 [PATCH 0/4] HID: fix few non-DMA capable HID transfers Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 1/4] HID: cp2112: make transfer buffers DMA capable Benjamin Tissoires @ 2016-11-21 10:48 ` Benjamin Tissoires 2016-11-21 12:05 ` [PATCH] HID: lg: fix noderef.cocci warnings kbuild test robot 2016-11-21 12:05 ` [PATCH 2/4] HID: lg: make transfer buffers DMA capable kbuild test robot 2016-11-21 10:48 ` [PATCH 3/4] HID: magicmouse: " Benjamin Tissoires ` (2 subsequent siblings) 4 siblings, 2 replies; 12+ messages in thread From: Benjamin Tissoires @ 2016-11-21 10:48 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, linux-kernel Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-lg.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c index 76f644d..d4c72a5 100644 --- a/drivers/hid/hid-lg.c +++ b/drivers/hid/hid-lg.c @@ -756,11 +756,16 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id) /* Setup wireless link with Logitech Wii wheel */ if (hdev->product == USB_DEVICE_ID_LOGITECH_WII_WHEEL) { - unsigned char buf[] = { 0x00, 0xAF, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; + const unsigned char cbuf[] = { 0x00, 0xAF, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; + u8 *buf = kmemdup(cbuf, sizeof(cbuf), GFP_KERNEL); - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + if (!buf) { + ret = -ENOMEM; + goto err_free; + } + ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(cbuf), + HID_FEATURE_REPORT, HID_REQ_SET_REPORT); if (ret >= 0) { /* insert a little delay of 10 jiffies ~ 40ms */ wait_queue_head_t wait; @@ -775,6 +780,7 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id) ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); } + kfree(buf); } if (drv_data->quirks & LG_FF) -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] HID: lg: fix noderef.cocci warnings 2016-11-21 10:48 ` [PATCH 2/4] HID: lg: " Benjamin Tissoires @ 2016-11-21 12:05 ` kbuild test robot 2016-11-21 14:17 ` Benjamin Tissoires 2016-11-21 12:05 ` [PATCH 2/4] HID: lg: make transfer buffers DMA capable kbuild test robot 1 sibling, 1 reply; 12+ messages in thread From: kbuild test robot @ 2016-11-21 12:05 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: kbuild-all, Jiri Kosina, linux-input, linux-kernel drivers/hid/hid-lg.c:780:47-53: ERROR: application of sizeof to pointer sizeof when applied to a pointer typed expression gives the size of the pointer Generated by: scripts/coccinelle/misc/noderef.cocci CC: Benjamin Tissoires <benjamin.tissoires@redhat.com> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- hid-lg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) --- a/drivers/hid/hid-lg.c +++ b/drivers/hid/hid-lg.c @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h buf[1] = 0xB2; get_random_bytes(&buf[2], 2); - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + ret = hid_hw_raw_request(hdev, buf[0], buf, + sizeof(*buf), + HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); } kfree(buf); } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] HID: lg: fix noderef.cocci warnings 2016-11-21 12:05 ` [PATCH] HID: lg: fix noderef.cocci warnings kbuild test robot @ 2016-11-21 14:17 ` Benjamin Tissoires 2016-11-22 10:44 ` Jiri Kosina 0 siblings, 1 reply; 12+ messages in thread From: Benjamin Tissoires @ 2016-11-21 14:17 UTC (permalink / raw) To: kbuild test robot; +Cc: kbuild-all, Jiri Kosina, linux-input, linux-kernel On Nov 21 2016 or thereabouts, kbuild test robot wrote: > drivers/hid/hid-lg.c:780:47-53: ERROR: application of sizeof to pointer > > sizeof when applied to a pointer typed expression gives the size of > the pointer > > Generated by: scripts/coccinelle/misc/noderef.cocci > > CC: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > --- > > hid-lg.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > --- a/drivers/hid/hid-lg.c > +++ b/drivers/hid/hid-lg.c > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h > buf[1] = 0xB2; > get_random_bytes(&buf[2], 2); > > - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), > - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); > + ret = hid_hw_raw_request(hdev, buf[0], buf, > + sizeof(*buf), This is wrong. I messed up and should have used "sizeof(cbuf)", but the coccinelle script failed at detecting the correct solution (I guess it couldn't). Jiri, do you want me to send a v2 of the series or will you just amend the patch while applying? Cheers, Benjamin > + HID_FEATURE_REPORT, > + HID_REQ_SET_REPORT); > } > kfree(buf); > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] HID: lg: fix noderef.cocci warnings 2016-11-21 14:17 ` Benjamin Tissoires @ 2016-11-22 10:44 ` Jiri Kosina 2016-11-23 2:22 ` Fengguang Wu 0 siblings, 1 reply; 12+ messages in thread From: Jiri Kosina @ 2016-11-22 10:44 UTC (permalink / raw) To: Benjamin Tissoires Cc: kbuild test robot, kbuild-all, linux-input, linux-kernel On Mon, 21 Nov 2016, Benjamin Tissoires wrote: > > Generated by: scripts/coccinelle/misc/noderef.cocci > > > > CC: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > > --- > > > > hid-lg.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > --- a/drivers/hid/hid-lg.c > > +++ b/drivers/hid/hid-lg.c > > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h > > buf[1] = 0xB2; > > get_random_bytes(&buf[2], 2); > > > > - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), > > - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); > > + ret = hid_hw_raw_request(hdev, buf[0], buf, > > + sizeof(*buf), > > This is wrong. I messed up and should have used "sizeof(cbuf)", but the > coccinelle script failed at detecting the correct solution (I guess it > couldn't). Fengguang, is there anything that could be done to improve this? > Jiri, do you want me to send a v2 of the series or will you just amend > the patch while applying? I'll fix that up, no worries. Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] HID: lg: fix noderef.cocci warnings 2016-11-22 10:44 ` Jiri Kosina @ 2016-11-23 2:22 ` Fengguang Wu 2016-11-23 7:03 ` Julia Lawall 0 siblings, 1 reply; 12+ messages in thread From: Fengguang Wu @ 2016-11-23 2:22 UTC (permalink / raw) To: Jiri Kosina Cc: Benjamin Tissoires, kbuild-all, linux-input, linux-kernel, Julia Lawall, Gilles Muller On Tue, Nov 22, 2016 at 11:44:34AM +0100, Jiri Kosina wrote: >On Mon, 21 Nov 2016, Benjamin Tissoires wrote: > >> > Generated by: scripts/coccinelle/misc/noderef.cocci >> > >> > CC: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> >> > --- >> > >> > hid-lg.c | 6 ++++-- >> > 1 file changed, 4 insertions(+), 2 deletions(-) >> > >> > --- a/drivers/hid/hid-lg.c >> > +++ b/drivers/hid/hid-lg.c >> > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h >> > buf[1] = 0xB2; >> > get_random_bytes(&buf[2], 2); >> > >> > - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf), >> > - HID_FEATURE_REPORT, HID_REQ_SET_REPORT); >> > + ret = hid_hw_raw_request(hdev, buf[0], buf, >> > + sizeof(*buf), >> >> This is wrong. I messed up and should have used "sizeof(cbuf)", but the >> coccinelle script failed at detecting the correct solution (I guess it >> couldn't). > >Fengguang, is there anything that could be done to improve this? CC Julie and Gilles. I'm not sure if the coccinelle script could be made that smart. :) >> Jiri, do you want me to send a v2 of the series or will you just amend >> the patch while applying? > >I'll fix that up, no worries. Thanks, > >-- >Jiri Kosina >SUSE Labs ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] HID: lg: fix noderef.cocci warnings 2016-11-23 2:22 ` Fengguang Wu @ 2016-11-23 7:03 ` Julia Lawall 0 siblings, 0 replies; 12+ messages in thread From: Julia Lawall @ 2016-11-23 7:03 UTC (permalink / raw) To: Fengguang Wu Cc: Jiri Kosina, Benjamin Tissoires, kbuild-all, linux-input, linux-kernel, Gilles Muller On Wed, 23 Nov 2016, Fengguang Wu wrote: > On Tue, Nov 22, 2016 at 11:44:34AM +0100, Jiri Kosina wrote: > > On Mon, 21 Nov 2016, Benjamin Tissoires wrote: > > > > > > Generated by: scripts/coccinelle/misc/noderef.cocci > > > > > > > > CC: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > > > > --- > > > > > > > > hid-lg.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > --- a/drivers/hid/hid-lg.c > > > > +++ b/drivers/hid/hid-lg.c > > > > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h > > > > buf[1] = 0xB2; > > > > get_random_bytes(&buf[2], 2); > > > > > > > > - ret = hid_hw_raw_request(hdev, buf[0], buf, > > > sizeof(buf), > > > > - HID_FEATURE_REPORT, > > > HID_REQ_SET_REPORT); > > > > + ret = hid_hw_raw_request(hdev, buf[0], buf, > > > > + sizeof(*buf), > > > > > > This is wrong. I messed up and should have used "sizeof(cbuf)", but the > > > coccinelle script failed at detecting the correct solution (I guess it > > > couldn't). > > > > Fengguang, is there anything that could be done to improve this? > > CC Julie and Gilles. I'm not sure if the coccinelle script could be > made that smart. :) Thanks for forwarding. From looking at the code snippet, I have the impression that if it were possible, it would require an interprocedural analysis, and the cost would outweigh the benefit. Basically, I don't see any cbuf nearby. julia > > > Jiri, do you want me to send a v2 of the series or will you just amend > > > the patch while applying? > > > > I'll fix that up, no worries. Thanks, > > > > -- > > Jiri Kosina > > SUSE Labs > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] HID: lg: make transfer buffers DMA capable 2016-11-21 10:48 ` [PATCH 2/4] HID: lg: " Benjamin Tissoires 2016-11-21 12:05 ` [PATCH] HID: lg: fix noderef.cocci warnings kbuild test robot @ 2016-11-21 12:05 ` kbuild test robot 1 sibling, 0 replies; 12+ messages in thread From: kbuild test robot @ 2016-11-21 12:05 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: kbuild-all, Jiri Kosina, linux-input, linux-kernel Hi Benjamin, [auto build test WARNING on hid/for-next] [also build test WARNING on v4.9-rc6 next-20161117] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/HID-fix-few-non-DMA-capable-HID-transfers/20161121-185330 base: https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next coccinelle warnings: (new ones prefixed by >>) >> drivers/hid/hid-lg.c:780:47-53: ERROR: application of sizeof to pointer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] HID: magicmouse: make transfer buffers DMA capable 2016-11-21 10:48 [PATCH 0/4] HID: fix few non-DMA capable HID transfers Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 1/4] HID: cp2112: make transfer buffers DMA capable Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 2/4] HID: lg: " Benjamin Tissoires @ 2016-11-21 10:48 ` Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 4/4] HID: rmi: " Benjamin Tissoires 2016-11-23 16:44 ` [PATCH 0/4] HID: fix few non-DMA capable HID transfers Jiri Kosina 4 siblings, 0 replies; 12+ messages in thread From: Benjamin Tissoires @ 2016-11-21 10:48 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, linux-kernel Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-magicmouse.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index d6fa496..20b40ad 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -493,7 +493,8 @@ static int magicmouse_input_configured(struct hid_device *hdev, static int magicmouse_probe(struct hid_device *hdev, const struct hid_device_id *id) { - __u8 feature[] = { 0xd7, 0x01 }; + const u8 feature[] = { 0xd7, 0x01 }; + u8 *buf; struct magicmouse_sc *msc; struct hid_report *report; int ret; @@ -544,6 +545,12 @@ static int magicmouse_probe(struct hid_device *hdev, } report->size = 6; + buf = kmemdup(feature, sizeof(feature), GFP_KERNEL); + if (!buf) { + ret = -ENOMEM; + goto err_stop_hw; + } + /* * Some devices repond with 'invalid report id' when feature * report switching it into multitouch mode is sent to it. @@ -552,8 +559,9 @@ static int magicmouse_probe(struct hid_device *hdev, * but there seems to be no other way of switching the mode. * Thus the super-ugly hacky success check below. */ - ret = hid_hw_raw_request(hdev, feature[0], feature, sizeof(feature), + ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(feature), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + kfree(buf); if (ret != -EIO && ret != sizeof(feature)) { hid_err(hdev, "unable to request touch data (%d)\n", ret); goto err_stop_hw; -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] HID: rmi: make transfer buffers DMA capable 2016-11-21 10:48 [PATCH 0/4] HID: fix few non-DMA capable HID transfers Benjamin Tissoires ` (2 preceding siblings ...) 2016-11-21 10:48 ` [PATCH 3/4] HID: magicmouse: " Benjamin Tissoires @ 2016-11-21 10:48 ` Benjamin Tissoires 2016-11-23 16:44 ` [PATCH 0/4] HID: fix few non-DMA capable HID transfers Jiri Kosina 4 siblings, 0 replies; 12+ messages in thread From: Benjamin Tissoires @ 2016-11-21 10:48 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input, linux-kernel Kernel v4.9 strictly enforces DMA capable buffers, so we need to remove buffers allocated on the stack. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-rmi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c index 9cd2ca3..be89bcb 100644 --- a/drivers/hid/hid-rmi.c +++ b/drivers/hid/hid-rmi.c @@ -188,10 +188,16 @@ static int rmi_set_page(struct hid_device *hdev, u8 page) static int rmi_set_mode(struct hid_device *hdev, u8 mode) { int ret; - u8 txbuf[2] = {RMI_SET_RMI_MODE_REPORT_ID, mode}; + const u8 txbuf[2] = {RMI_SET_RMI_MODE_REPORT_ID, mode}; + u8 *buf; - ret = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, txbuf, + buf = kmemdup(txbuf, sizeof(txbuf), GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, buf, sizeof(txbuf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + kfree(buf); if (ret < 0) { dev_err(&hdev->dev, "unable to set rmi mode to %d (%d)\n", mode, ret); -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] HID: fix few non-DMA capable HID transfers 2016-11-21 10:48 [PATCH 0/4] HID: fix few non-DMA capable HID transfers Benjamin Tissoires ` (3 preceding siblings ...) 2016-11-21 10:48 ` [PATCH 4/4] HID: rmi: " Benjamin Tissoires @ 2016-11-23 16:44 ` Jiri Kosina 4 siblings, 0 replies; 12+ messages in thread From: Jiri Kosina @ 2016-11-23 16:44 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: linux-input, linux-kernel On Mon, 21 Nov 2016, Benjamin Tissoires wrote: > While playing a little bit with the CP2112 (for hid-rmi I must confess), I > realized that kernel v4.9 now enforces DMA capable buffers when calling > hid_hw_raw_request(). Kernel v4.8 works fine (but gives a stacktrace the first > time), but v4.9 doesn't. > > So I gave a check of all the other drivers in the HID tree, and it looks like > only 4 drivers are not properly allocating their buffers. > > I'd say this is v4.9 material but I was not able to test magicmouse and lg. > If this doesn't qualifies for 4.9-rc7, I think we should add the stable@ stamp, > given that the chance this will break hid-rmi is huge (cp2112 is not so much an > issue, given it's a devel board). Thanks for fixing this up. Applied to for-4.9/upstream-fixes. -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-11-23 16:44 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-21 10:48 [PATCH 0/4] HID: fix few non-DMA capable HID transfers Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 1/4] HID: cp2112: make transfer buffers DMA capable Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 2/4] HID: lg: " Benjamin Tissoires 2016-11-21 12:05 ` [PATCH] HID: lg: fix noderef.cocci warnings kbuild test robot 2016-11-21 14:17 ` Benjamin Tissoires 2016-11-22 10:44 ` Jiri Kosina 2016-11-23 2:22 ` Fengguang Wu 2016-11-23 7:03 ` Julia Lawall 2016-11-21 12:05 ` [PATCH 2/4] HID: lg: make transfer buffers DMA capable kbuild test robot 2016-11-21 10:48 ` [PATCH 3/4] HID: magicmouse: " Benjamin Tissoires 2016-11-21 10:48 ` [PATCH 4/4] HID: rmi: " Benjamin Tissoires 2016-11-23 16:44 ` [PATCH 0/4] HID: fix few non-DMA capable HID transfers Jiri Kosina
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).