Re: [PATCH] USB: gadgetfs: Fix crash caused by inadequate synchronization
On Thu, Sep 21, 2017 at 01:23:58PM -0400, Alan Stern wrote: > The gadgetfs driver (drivers/usb/gadget/legacy/inode.c) was written > before the UDC and composite frameworks were adopted; it is a legacy > driver. As such, it expects that once bound to a UDC controller, it > will not be unbound until it unregisters itself. > > However, the UDC framework does unbind function drivers while they are > still registered. When this happens, it can cause the gadgetfs driver > to misbehave or crash. For example, userspace can cause a crash by > opening the device file and doing an ioctl call before setting up a > configuration (found by Andrey Konovalov using the syzkaller fuzzer). > > This patch adds checks and synchronization to prevent these bad > behaviors. It adds a udc_usage counter that the driver increments at > times when it is using a gadget interface without holding the private > spinlock. The unbind routine waits for this counter to go to 0 before > returning, thereby ensuring that the UDC is no longer in use. > > The patch also adds a check in the dev_ioctl() routine to make sure > the driver is bound to a UDC before dereferencing the gadget pointer, > and it makes destroy_ep_files() synchronize with the endpoint I/O > routines, to prevent the user from accessing an endpoint data > structure after it has been removed. > > Signed-off-by: Alan Stern > Reported-by: Andrey Konovalov > Tested-by: Andrey Konovalov > CC: Felipe, any objection for me taking this, and the other gadget driver fixes that Alan just sent out, directly in my tree? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/storage/uas: slab-out-of-bounds in uas_probe
On Thu, Sep 21, 2017 at 03:04:05PM -0400, Alan Stern wrote: > On Thu, 21 Sep 2017, Andrey Konovalov wrote: > > > On Thu, Sep 21, 2017 at 6:10 PM, Greg Kroah-Hartman > > wrote: > > > On Thu, Sep 21, 2017 at 05:39:05PM +0200, Andrey Konovalov wrote: > > >> Hi! > > >> > > >> I've got the following report while fuzzing the kernel with syzkaller. > > >> > > >> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). > > >> > > >> The issue occurs when we iterate over interface altsettings, but I > > >> don't see the driver doing anything wrong. I might be missing > > >> something, or this might be an issue in USB core altsettings parsing. > > > > > > > > > Any chance you happen to have the descriptors that you were feeding into > > > the kernel at this crash? That might help in figuring out what "went > > > wrong". > > > > Here's the data that I feed into dummy_udc: > > > > 00 00 00 00 09 02 12 00 01 34 05 80 07 09 04 6e > > 09 00 08 06 62 00 12 01 05 00 cb f7 71 83 04 00 > > 05 00 ab f6 07 81 08 01 > > > > Also attaching a C reproducer (requires dummy_hcd and gadgetfs) and my > > .config. > > Why do your reproducers use an mmap'ed array for their data instead of > a plain old statically allocated array? > > Anyway, this turns out to be a genuine (and subtle!) bug in the uas > driver. The uas_find_uas_alt_setting() routine in uas-detect.h returns > a bAlternateSetting value, but then the uas_use_uas_driver() routine > uses this value as an index to the altsetting array -- which it isn't. > > Normally this doesn't cause any problems because the the entries in the > array have bAlternateSetting values 0, 1, etc., so the value is equal > to the index. But in your fuzzed case, that wasn't true. > > The patch below fixes this bug, by returning a pointer to the > alt-setting entry instead of either the value or the index. Pointers > are less subject to misinterpretation. Ugh, messy, nice catch and fix, I'll go queue it up now, thanks for resolving this. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/storage/uas: slab-out-of-bounds in uas_probe
On Fri, Sep 22, 2017 at 09:58:15AM +0200, Greg Kroah-Hartman wrote: > On Thu, Sep 21, 2017 at 03:04:05PM -0400, Alan Stern wrote: > > On Thu, 21 Sep 2017, Andrey Konovalov wrote: > > > > > On Thu, Sep 21, 2017 at 6:10 PM, Greg Kroah-Hartman > > > wrote: > > > > On Thu, Sep 21, 2017 at 05:39:05PM +0200, Andrey Konovalov wrote: > > > >> Hi! > > > >> > > > >> I've got the following report while fuzzing the kernel with syzkaller. > > > >> > > > >> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). > > > >> > > > >> The issue occurs when we iterate over interface altsettings, but I > > > >> don't see the driver doing anything wrong. I might be missing > > > >> something, or this might be an issue in USB core altsettings parsing. > > > > > > > > > > > > Any chance you happen to have the descriptors that you were feeding into > > > > the kernel at this crash? That might help in figuring out what "went > > > > wrong". > > > > > > Here's the data that I feed into dummy_udc: > > > > > > 00 00 00 00 09 02 12 00 01 34 05 80 07 09 04 6e > > > 09 00 08 06 62 00 12 01 05 00 cb f7 71 83 04 00 > > > 05 00 ab f6 07 81 08 01 > > > > > > Also attaching a C reproducer (requires dummy_hcd and gadgetfs) and my > > > .config. > > > > Why do your reproducers use an mmap'ed array for their data instead of > > a plain old statically allocated array? > > > > Anyway, this turns out to be a genuine (and subtle!) bug in the uas > > driver. The uas_find_uas_alt_setting() routine in uas-detect.h returns > > a bAlternateSetting value, but then the uas_use_uas_driver() routine > > uses this value as an index to the altsetting array -- which it isn't. > > > > Normally this doesn't cause any problems because the the entries in the > > array have bAlternateSetting values 0, 1, etc., so the value is equal > > to the index. But in your fuzzed case, that wasn't true. > > > > The patch below fixes this bug, by returning a pointer to the > > alt-setting entry instead of either the value or the index. Pointers > > are less subject to misinterpretation. > > Ugh, messy, nice catch and fix, I'll go queue it up now, thanks for > resolving this. Oops, no, you didn't send this as a "real" patch yet, I'll wait :) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: exynos: Add dwc3 SUSPHY quirk
Hi, W dniu 19.09.2017 o 20:10, Robin Murphy pisze: On 19/09/17 18:40, Krzysztof Kozlowski wrote: On Mon, Sep 18, 2017 at 12:02:13PM +0200, Andrzej Pietrasiewicz wrote: Odroid XU4 board does not enumerate SuperSpeed devices. This patch makes exynos5 series chips use USB SUSPHY quirk, which solves the problem. Signed-off-by: Andrzej Pietrasiewicz --- arch/arm/boot/dts/exynos54xx.dtsi | 2 ++ 1 file changed, 2 insertions(+) Makes sense to me... was it tested also on XU3 and XU? Well, it at least doesn't make USB3 on my XU any more or less broken than it was before ;) (both ports still report an over-current condition even with nothing plugged in - I suspect there's probably still some pinctrl/regulator stuff missing) Robin. Similar with XU3: nothing is any more or less broken with this patch compared to the situation without the patch. Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9] use setup_timer() helper function.
This series uses setup_timer() helper function. The series addresses the files under drivers/usb/*. Allen Pais (9): drivers: usb: hcd: use setup_timer() helper. drivers: usb: phy: omap: use setup_timer() helper. usb: gadget: udc: m66592: use setup_timer() helper. usb: gadget: udc: pxa25x_udc: use setup_timer() helper. drivers: usb: atm: cxacru: use setup_timer() helper. usb: gadget: udc: r8a66597: use setup_timer() helper. drivers: usb: speedtch: use setup_timer() helper. usb: gadget: udc: dummy_hcd: use setup_timer() helper. usb: gadget: udc: snps_udc_core: use setup_timer() helper. drivers/usb/atm/cxacru.c | 4 +--- drivers/usb/atm/speedtch.c | 11 --- drivers/usb/core/hcd.c | 4 +--- drivers/usb/gadget/udc/dummy_hcd.c | 8 ++-- drivers/usb/gadget/udc/m66592-udc.c| 4 +--- drivers/usb/gadget/udc/pxa25x_udc.c| 4 +--- drivers/usb/gadget/udc/r8a66597-udc.c | 4 +--- drivers/usb/gadget/udc/snps_udc_core.c | 8 ++-- drivers/usb/phy/phy-isp1301-omap.c | 4 +--- 9 files changed, 14 insertions(+), 37 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] drivers: usb: hcd: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the function and data fields. Signed-off-by: Allen Pais --- drivers/usb/core/hcd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 75ad671..67aa3d0 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2558,9 +2558,7 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver, hcd->self.bus_name = bus_name; hcd->self.uses_dma = (sysdev->dma_mask != NULL); - init_timer(&hcd->rh_timer); - hcd->rh_timer.function = rh_timer_func; - hcd->rh_timer.data = (unsigned long) hcd; + setup_timer(&hcd->rh_timer, rh_timer_func, (unsigned long)hcd); #ifdef CONFIG_PM INIT_WORK(&hcd->wakeup_work, hcd_resume_work); #endif -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] drivers: usb: phy: omap: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the function and data fields. Signed-off-by: Allen Pais --- drivers/usb/phy/phy-isp1301-omap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/phy/phy-isp1301-omap.c b/drivers/usb/phy/phy-isp1301-omap.c index c6052c8..5630840 100644 --- a/drivers/usb/phy/phy-isp1301-omap.c +++ b/drivers/usb/phy/phy-isp1301-omap.c @@ -1507,9 +1507,7 @@ isp1301_probe(struct i2c_client *i2c, const struct i2c_device_id *id) } INIT_WORK(&isp->work, isp1301_work); - init_timer(&isp->timer); - isp->timer.function = isp1301_timer; - isp->timer.data = (unsigned long) isp; + setup_timer(&isp->timer, isp1301_timer, (unsigned long)isp); i2c_set_clientdata(i2c, isp); isp->client = i2c; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] drivers: usb: atm: cxacru: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais
---
drivers/usb/atm/cxacru.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index 5160a4a..600a670 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -570,10 +570,8 @@ static int cxacru_start_wait_urb(struct urb *urb, struct
completion *done,
{
struct timer_list timer;
- init_timer(&timer);
+ setup_timer(&timer, cxacru_timeout_kill, (unsigned long)urb);
timer.expires = jiffies + msecs_to_jiffies(CMD_TIMEOUT);
- timer.data = (unsigned long) urb;
- timer.function = cxacru_timeout_kill;
add_timer(&timer);
wait_for_completion(done);
del_timer_sync(&timer);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] usb: gadget: udc: pxa25x_udc: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the function and data fields. Signed-off-by: Allen Pais --- drivers/usb/gadget/udc/pxa25x_udc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c index a238da9..974b778 100644 --- a/drivers/usb/gadget/udc/pxa25x_udc.c +++ b/drivers/usb/gadget/udc/pxa25x_udc.c @@ -2417,9 +2417,7 @@ static int pxa25x_udc_probe(struct platform_device *pdev) gpio_direction_output(dev->mach->gpio_pullup, 0); } - init_timer(&dev->timer); - dev->timer.function = udc_watchdog; - dev->timer.data = (unsigned long) dev; + setup_timer(&dev->timer, udc_watchdog, (unsigned long)dev); the_controller = dev; platform_set_drvdata(pdev, dev); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] usb: gadget: udc: r8a66597: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais
---
drivers/usb/gadget/udc/r8a66597-udc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c
b/drivers/usb/gadget/udc/r8a66597-udc.c
index 118ad70..bb844b9 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -1877,9 +1877,7 @@ static int r8a66597_probe(struct platform_device *pdev)
r8a66597->gadget.max_speed = USB_SPEED_HIGH;
r8a66597->gadget.name = udc_name;
- init_timer(&r8a66597->timer);
- r8a66597->timer.function = r8a66597_timer;
- r8a66597->timer.data = (unsigned long)r8a66597;
+ setup_timer(&r8a66597->timer, r8a66597_timer, (unsigned long)r8a66597);
r8a66597->reg = reg;
if (r8a66597->pdata->on_chip) {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] usb: gadget: udc: dummy_hcd: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais
---
drivers/usb/gadget/udc/dummy_hcd.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c
b/drivers/usb/gadget/udc/dummy_hcd.c
index a030d79..c9ac44d 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2389,9 +2389,7 @@ static DEVICE_ATTR_RO(urbs);
static int dummy_start_ss(struct dummy_hcd *dum_hcd)
{
- init_timer(&dum_hcd->timer);
- dum_hcd->timer.function = dummy_timer;
- dum_hcd->timer.data = (unsigned long)dum_hcd;
+ setup_timer(&dum_hcd->timer, dummy_timer, (unsigned long)dum_hcd);
dum_hcd->rh_state = DUMMY_RH_RUNNING;
dum_hcd->stream_en_ep = 0;
INIT_LIST_HEAD(&dum_hcd->urbp_list);
@@ -2420,9 +2418,7 @@ static int dummy_start(struct usb_hcd *hcd)
return dummy_start_ss(dum_hcd);
spin_lock_init(&dum_hcd->dum->lock);
- init_timer(&dum_hcd->timer);
- dum_hcd->timer.function = dummy_timer;
- dum_hcd->timer.data = (unsigned long)dum_hcd;
+ setup_timer(&dum_hcd->timer, dummy_timer, (unsigned long)dum_hcd);
dum_hcd->rh_state = DUMMY_RH_RUNNING;
INIT_LIST_HEAD(&dum_hcd->urbp_list);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] usb: gadget: udc: snps_udc_core: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the function and data fields. Signed-off-by: Allen Pais --- drivers/usb/gadget/udc/snps_udc_core.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/udc/snps_udc_core.c b/drivers/usb/gadget/udc/snps_udc_core.c index 38a165d..47df99d 100644 --- a/drivers/usb/gadget/udc/snps_udc_core.c +++ b/drivers/usb/gadget/udc/snps_udc_core.c @@ -3207,13 +3207,9 @@ int udc_probe(struct udc *dev) goto finished; /* timer init */ - init_timer(&udc_timer); - udc_timer.function = udc_timer_function; - udc_timer.data = 1; + setup_timer(&udc_timer, udc_timer_function, 1); /* timer pollstall init */ - init_timer(&udc_pollstall_timer); - udc_pollstall_timer.function = udc_pollstall_timer_function; - udc_pollstall_timer.data = 1; + setup_timer(&udc_pollstall_timer, udc_pollstall_timer_function, 1); /* set SD */ reg = readl(&dev->regs->ctl); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] drivers: usb: speedtch: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the function and data fields. Signed-off-by: Allen Pais --- drivers/usb/atm/speedtch.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c index 3676adb..091db9b 100644 --- a/drivers/usb/atm/speedtch.c +++ b/drivers/usb/atm/speedtch.c @@ -874,16 +874,13 @@ static int speedtch_bind(struct usbatm_data *usbatm, usbatm->flags |= (use_isoc ? UDSL_USE_ISOC : 0); INIT_WORK(&instance->status_check_work, speedtch_check_status); - init_timer(&instance->status_check_timer); - - instance->status_check_timer.function = speedtch_status_poll; - instance->status_check_timer.data = (unsigned long)instance; + setup_timer(&instance->status_check_timer, speedtch_status_poll, + (unsigned long)instance); instance->last_status = 0xff; instance->poll_delay = MIN_POLL_DELAY; - init_timer(&instance->resubmit_timer); - instance->resubmit_timer.function = speedtch_resubmit_int; - instance->resubmit_timer.data = (unsigned long)instance; + setup_timer(&instance->resubmit_timer, speedtch_resubmit_int, + (unsigned long)instance); instance->int_urb = usb_alloc_urb(0, GFP_KERNEL); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] usb: gadget: udc: m66592: use setup_timer() helper.
Use setup_timer function instead of initializing timer with the function and data fields. Signed-off-by: Allen Pais --- drivers/usb/gadget/udc/m66592-udc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c index 46ce7bc..3b8dbed 100644 --- a/drivers/usb/gadget/udc/m66592-udc.c +++ b/drivers/usb/gadget/udc/m66592-udc.c @@ -1592,9 +1592,7 @@ static int m66592_probe(struct platform_device *pdev) m66592->gadget.max_speed = USB_SPEED_HIGH; m66592->gadget.name = udc_name; - init_timer(&m66592->timer); - m66592->timer.function = m66592_timer; - m66592->timer.data = (unsigned long)m66592; + setup_timer(&m66592->timer, m66592_timer, (unsigned long)m66592); m66592->reg = reg; ret = request_irq(ires->start, m66592_irq, IRQF_SHARED, -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: gadgetfs: Fix crash caused by inadequate synchronization
Hi, Greg KH writes: > On Thu, Sep 21, 2017 at 01:23:58PM -0400, Alan Stern wrote: >> The gadgetfs driver (drivers/usb/gadget/legacy/inode.c) was written >> before the UDC and composite frameworks were adopted; it is a legacy >> driver. As such, it expects that once bound to a UDC controller, it >> will not be unbound until it unregisters itself. >> >> However, the UDC framework does unbind function drivers while they are >> still registered. When this happens, it can cause the gadgetfs driver >> to misbehave or crash. For example, userspace can cause a crash by >> opening the device file and doing an ioctl call before setting up a >> configuration (found by Andrey Konovalov using the syzkaller fuzzer). >> >> This patch adds checks and synchronization to prevent these bad >> behaviors. It adds a udc_usage counter that the driver increments at >> times when it is using a gadget interface without holding the private >> spinlock. The unbind routine waits for this counter to go to 0 before >> returning, thereby ensuring that the UDC is no longer in use. >> >> The patch also adds a check in the dev_ioctl() routine to make sure >> the driver is bound to a UDC before dereferencing the gadget pointer, >> and it makes destroy_ep_files() synchronize with the endpoint I/O >> routines, to prevent the user from accessing an endpoint data >> structure after it has been removed. >> >> Signed-off-by: Alan Stern >> Reported-by: Andrey Konovalov >> Tested-by: Andrey Konovalov >> CC: > > Felipe, any objection for me taking this, and the other gadget driver > fixes that Alan just sent out, directly in my tree? none whatsoever, for all of them: Acked-by: Felipe Balbi I'll rebase my testing/fixes on top of your greg/usb-linus for the remaining of the -rc cycle ;-) -- balbi signature.asc Description: PGP signature
Re: [PATCH] USB: g_mass_storage: Fix deadlock when driver is unbound
Alan Stern writes: > As a holdover from the old g_file_storage gadget, the g_mass_storage > legacy gadget driver attempts to unregister itself when its main > operating thread terminates (if it hasn't been unregistered already). > This is not strictly necessary; it was never more than an attempt to > have the gadget fail cleanly if something went wrong and the main > thread was killed. > > However, now that the UDC core manages gadget drivers independently of > UDC drivers, this scheme doesn't work any more. A simple test: > > modprobe dummy-hcd > modprobe g-mass-storage file=... > rmmod dummy-hcd > > ends up in a deadlock with the following backtrace: > > sysrq: SysRq : Show Blocked State >taskPC stack pid father > file-storageD0 1130 2 0x > Call Trace: > __schedule+0x53e/0x58c > schedule+0x6e/0x77 > schedule_preempt_disabled+0xd/0xf > __mutex_lock.isra.1+0x129/0x224 > ? _raw_spin_unlock_irqrestore+0x12/0x14 > __mutex_lock_slowpath+0x12/0x14 > mutex_lock+0x28/0x2b > usb_gadget_unregister_driver+0x29/0x9b [udc_core] > usb_composite_unregister+0x10/0x12 [libcomposite] > msg_cleanup+0x1d/0x20 [g_mass_storage] > msg_thread_exits+0xd/0xdd7 [g_mass_storage] > fsg_main_thread+0x1395/0x13d6 [usb_f_mass_storage] > ? __schedule+0x573/0x58c > kthread+0xd9/0xdb > ? do_set_interface+0x25c/0x25c [usb_f_mass_storage] > ? init_completion+0x1e/0x1e > ret_from_fork+0x19/0x24 > rmmod D0 1155683 0x > Call Trace: > __schedule+0x53e/0x58c > schedule+0x6e/0x77 > schedule_timeout+0x26/0xbc > ? __schedule+0x573/0x58c > do_wait_for_common+0xb3/0x128 > ? usleep_range+0x81/0x81 > ? wake_up_q+0x3f/0x3f > wait_for_common+0x2e/0x45 > wait_for_completion+0x17/0x19 > fsg_common_put+0x34/0x81 [usb_f_mass_storage] > fsg_free_inst+0x13/0x1e [usb_f_mass_storage] > usb_put_function_instance+0x1a/0x25 [libcomposite] > msg_unbind+0x2a/0x42 [g_mass_storage] > __composite_unbind+0x4a/0x6f [libcomposite] > composite_unbind+0x12/0x14 [libcomposite] > usb_gadget_remove_driver+0x4f/0x77 [udc_core] > usb_del_gadget_udc+0x52/0xcc [udc_core] > dummy_udc_remove+0x27/0x2c [dummy_hcd] > platform_drv_remove+0x1d/0x31 > device_release_driver_internal+0xe9/0x16d > device_release_driver+0x11/0x13 > bus_remove_device+0xd2/0xe2 > device_del+0x19f/0x221 > ? selinux_capable+0x22/0x27 > platform_device_del+0x21/0x63 > platform_device_unregister+0x10/0x1a > cleanup+0x20/0x817 [dummy_hcd] > SyS_delete_module+0x10c/0x197 > ? fput+0xd/0xf > ? task_work_run+0x55/0x62 > ? prepare_exit_to_usermode+0x65/0x75 > do_fast_syscall_32+0x86/0xc3 > entry_SYSENTER_32+0x4e/0x7c > > What happens is that removing the dummy-hcd driver causes the UDC core > to unbind the gadget driver, which it does while holding the udc_lock > mutex. The unbind routine in g_mass_storage tells the main thread to > exit and waits for it to terminate. > > But as mentioned above, when the main thread exits it tries to > unregister the mass-storage function driver. Via the composite > framework this ends up calling usb_gadget_unregister_driver(), which > tries to acquire the udc_lock mutex. The result is deadlock. > > The simplest way to fix the problem is not to be so clever: The main > thread doesn't have to unregister the function driver. The side > effects won't be so terrible; if the gadget is still attached to a USB > host when the main thread is killed, it will appear to the host as > though the gadget's firmware has crashed -- a reasonably accurate > interpretation, and an all-too-common occurrence for USB mass-storage > devices. > > In fact, the code to unregister the driver when the main thread exits > is specific to g-mass-storage; it is not used when f-mass-storage is > included as a function in a larger composite device. Therefore the > entire mechanism responsible for this (the fsg_operations structure > with its ->thread_exits method, the fsg_common_set_ops() routine, and > the msg_thread_exits() callback routine) can all be eliminated. Even > the msg_registered bitflag can be removed, because now the driver is > unregistered in only one place rather than in two places. > > Signed-off-by: Alan Stern > CC: Michal Nazarewicz > CC: Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH] USB: gadgetfs: fix copy_to_user while holding spinlock
Alan Stern writes: > The gadgetfs driver as a long-outstanding FIXME, regarding a call of > copy_to_user() made while holding a spinlock. This patch fixes the > issue by dropping the spinlock and using the dev->udc_usage mechanism > introduced by another recent patch to guard against status changes > while the lock isn't held. > > Signed-off-by: Alan Stern > Reported-by: Andrey Konovalov > CC: Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH v2 3/9] clk: at91: pmc: Support backup for programmable clocks
On 15/09/2017 at 16:04, Romain Izard wrote:
> From: Romain Izard
>
> When an AT91 programmable clock is declared in the device tree, register
> it into the Power Management Controller driver. On entering suspend mode,
> the driver saves and restores the Programmable Clock registers to support
> the backup mode for these clocks.
>
> Signed-off-by: Romain Izard
Romain,
Some nitpicking and one comment. But on the overall patch, here is my:
Acked-by: Nicolas Ferre
See below:
> ---
> Changes in v2:
> * register PCKs on clock startup
>
> drivers/clk/at91/clk-programmable.c | 2 ++
> drivers/clk/at91/pmc.c | 27 +++
> drivers/clk/at91/pmc.h | 2 ++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/clk/at91/clk-programmable.c
> b/drivers/clk/at91/clk-programmable.c
> index 85a449cf61e3..0e6aab1252fc 100644
> --- a/drivers/clk/at91/clk-programmable.c
> +++ b/drivers/clk/at91/clk-programmable.c
> @@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
> if (ret) {
> kfree(prog);
> hw = ERR_PTR(ret);
Nit: "else" not needed.
> + } else {
> + pmc_register_pck(id);
> }
>
> return hw;
> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
> index 07dc2861ad3f..3910b7537152 100644
> --- a/drivers/clk/at91/pmc.c
> +++ b/drivers/clk/at91/pmc.c
> @@ -22,6 +22,7 @@
> #include "pmc.h"
>
> #define PMC_MAX_IDS 128
> +#define PMC_MAX_PCKS 8
>
> int of_at91_get_clk_range(struct device_node *np, const char *propname,
> struct clk_range *range)
> @@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
> static struct regmap *pmcreg;
>
> static u8 registered_ids[PMC_MAX_IDS];
> +static u8 registered_pcks[PMC_MAX_PCKS];
>
> static struct
> {
> @@ -66,8 +68,10 @@ static struct
> u32 pcr[PMC_MAX_IDS];
> u32 audio_pll0;
> u32 audio_pll1;
> + u32 pckr[PMC_MAX_PCKS];
> } pmc_cache;
>
> +/* Clock ID 0 is invalid */
(read: so we can use the 0 value as an indicator that this place in the
table hasn't been filled, so unused)
> void pmc_register_id(u8 id)
> {
> int i;
> @@ -82,6 +86,21 @@ void pmc_register_id(u8 id)
> }
> }
>
> +/* Programmable Clock 0 is valid */
I understand the rationale behind these ^^ two comments, but I would
like that it's more explicit. Saying that you will store the pck id as
(id + 1) and that you would have to invert this operation while using
the stored id.
Maybe add a comment about this transformation to the struct definition
as well...
> +void pmc_register_pck(u8 pck)
> +{
> + int i;
> +
> + for (i = 0; i < PMC_MAX_PCKS; i++) {
> + if (registered_pcks[i] == 0) {
> + registered_pcks[i] = pck + 1;
> + break;
> + }
> + if (registered_pcks[i] == (pck + 1))
> + break;
> + }
> +}
> +
> static int pmc_suspend(void)
> {
> int i;
> @@ -103,6 +122,10 @@ static int pmc_suspend(void)
> regmap_read(pmcreg, AT91_PMC_PCR,
> &pmc_cache.pcr[registered_ids[i]]);
> }
> + for (i = 0; registered_pcks[i]; i++) {
> + u8 num = registered_pcks[i] - 1;
Nit: declaration are better made at the beginning of the function. This
lead to a checkpatch warning:
"WARNING: Missing a blank line after declarations"
> + regmap_read(pmcreg, AT91_PMC_PCKR(num), &pmc_cache.pckr[num]);
> + }
>
> return 0;
> }
> @@ -143,6 +166,10 @@ static void pmc_resume(void)
>pmc_cache.pcr[registered_ids[i]] |
>AT91_PMC_PCR_CMD);
> }
> + for (i = 0; registered_pcks[i]; i++) {
> + u8 num = registered_pcks[i] - 1;
Ditto
> + regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
> + }
>
> if (pmc_cache.uckr & AT91_PMC_UPLLEN)
> mask |= AT91_PMC_LOCKU;
> diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
> index 858e8ef7e8db..d22b1fa9ecdc 100644
> --- a/drivers/clk/at91/pmc.h
> +++ b/drivers/clk/at91/pmc.h
> @@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const
> char *propname,
>
> #ifdef CONFIG_PM
> void pmc_register_id(u8 id);
> +void pmc_register_pck(u8 pck);
> #else
> static inline void pmc_register_id(u8 id) {}
> +static inline void pmc_register_pck(u8 pck) {}
> #endif
>
> #endif /* __PMC_H_ */
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/storage/uas: slab-out-of-bounds in uas_probe
On Thu, Sep 21, 2017 at 9:04 PM, Alan Stern wrote:
> On Thu, 21 Sep 2017, Andrey Konovalov wrote:
>
>> On Thu, Sep 21, 2017 at 6:10 PM, Greg Kroah-Hartman
>> wrote:
>> > On Thu, Sep 21, 2017 at 05:39:05PM +0200, Andrey Konovalov wrote:
>> >> Hi!
>> >>
>> >> I've got the following report while fuzzing the kernel with syzkaller.
>> >>
>> >> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>> >>
>> >> The issue occurs when we iterate over interface altsettings, but I
>> >> don't see the driver doing anything wrong. I might be missing
>> >> something, or this might be an issue in USB core altsettings parsing.
>> >
>> >
>> > Any chance you happen to have the descriptors that you were feeding into
>> > the kernel at this crash? That might help in figuring out what "went
>> > wrong".
>>
>> Here's the data that I feed into dummy_udc:
>>
>> 00 00 00 00 09 02 12 00 01 34 05 80 07 09 04 6e
>> 09 00 08 06 62 00 12 01 05 00 cb f7 71 83 04 00
>> 05 00 ab f6 07 81 08 01
>>
>> Also attaching a C reproducer (requires dummy_hcd and gadgetfs) and my
>> .config.
>
> Why do your reproducers use an mmap'ed array for their data instead of
> a plain old statically allocated array?
That's just the way syzkaller generates reproducers, it uses mmap() to
allocate memory to feed into syscalls.
>
> Anyway, this turns out to be a genuine (and subtle!) bug in the uas
> driver. The uas_find_uas_alt_setting() routine in uas-detect.h returns
> a bAlternateSetting value, but then the uas_use_uas_driver() routine
> uses this value as an index to the altsetting array -- which it isn't.
>
> Normally this doesn't cause any problems because the the entries in the
> array have bAlternateSetting values 0, 1, etc., so the value is equal
> to the index. But in your fuzzed case, that wasn't true.
>
> The patch below fixes this bug, by returning a pointer to the
> alt-setting entry instead of either the value or the index. Pointers
> are less subject to misinterpretation.
The patch helps, thanks!
Tested-by: Andrey Konovalov
>
> Alan Stern
>
>
>
> Index: usb-4.x/drivers/usb/storage/uas-detect.h
> ===
> --- usb-4.x.orig/drivers/usb/storage/uas-detect.h
> +++ usb-4.x/drivers/usb/storage/uas-detect.h
> @@ -9,7 +9,8 @@ static int uas_is_interface(struct usb_h
> intf->desc.bInterfaceProtocol == USB_PR_UAS);
> }
>
> -static int uas_find_uas_alt_setting(struct usb_interface *intf)
> +static struct usb_host_interface *uas_find_uas_alt_setting(
> + struct usb_interface *intf)
> {
> int i;
>
> @@ -17,10 +18,10 @@ static int uas_find_uas_alt_setting(stru
> struct usb_host_interface *alt = &intf->altsetting[i];
>
> if (uas_is_interface(alt))
> - return alt->desc.bAlternateSetting;
> + return alt;
> }
>
> - return -ENODEV;
> + return NULL;
> }
>
> static int uas_find_endpoints(struct usb_host_interface *alt,
> @@ -58,14 +59,14 @@ static int uas_use_uas_driver(struct usb
> struct usb_device *udev = interface_to_usbdev(intf);
> struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> unsigned long flags = id->driver_info;
> - int r, alt;
> -
> + struct usb_host_interface *alt;
> + int r;
>
> alt = uas_find_uas_alt_setting(intf);
> - if (alt < 0)
> + if (!alt)
> return 0;
>
> - r = uas_find_endpoints(&intf->altsetting[alt], eps);
> + r = uas_find_endpoints(alt, eps);
> if (r < 0)
> return 0;
>
> Index: usb-4.x/drivers/usb/storage/uas.c
> ===
> --- usb-4.x.orig/drivers/usb/storage/uas.c
> +++ usb-4.x/drivers/usb/storage/uas.c
> @@ -873,14 +873,14 @@ MODULE_DEVICE_TABLE(usb, uas_usb_ids);
> static int uas_switch_interface(struct usb_device *udev,
> struct usb_interface *intf)
> {
> - int alt;
> + struct usb_host_interface *alt;
>
> alt = uas_find_uas_alt_setting(intf);
> - if (alt < 0)
> - return alt;
> + if (!alt)
> + return -ENODEV;
>
> - return usb_set_interface(udev,
> - intf->altsetting[0].desc.bInterfaceNumber, alt);
> + return usb_set_interface(udev, alt->desc.bInterfaceNumber,
> + alt->desc.bAlternateSetting);
> }
>
> static int uas_configure_endpoints(struct uas_dev_info *devinfo)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] clk: at91: pmc: Wait for clocks when resuming
On Fri, Sep 15, 2017 at 04:04:03PM +0200, Romain Izard wrote:
> Wait for the syncronization of all clocks when resuming, not only the
> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
> when interrupts are masked, which is the case in here.
>
> Signed-off-by: Romain Izard
Acked-by: Ludovic Desroches
I faced the same issue because of the use of regmap_read_poll_timeout
when timekeeping is not ready.
Ludovic
> ---
> drivers/clk/at91/pmc.c | 24
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
> index 775af473fe11..5c2b26de303e 100644
> --- a/drivers/clk/at91/pmc.c
> +++ b/drivers/clk/at91/pmc.c
> @@ -107,10 +107,20 @@ static int pmc_suspend(void)
> return 0;
> }
>
> +static bool pmc_ready(unsigned int mask)
> +{
> + unsigned int status;
> +
> + regmap_read(pmcreg, AT91_PMC_SR, &status);
> +
> + return ((status & mask) == mask) ? 1 : 0;
> +}
> +
> static void pmc_resume(void)
> {
> - int i, ret = 0;
> + int i;
> u32 tmp;
> + u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
>
> regmap_read(pmcreg, AT91_PMC_MCKR, &tmp);
> if (pmc_cache.mckr != tmp)
> @@ -134,13 +144,11 @@ static void pmc_resume(void)
>AT91_PMC_PCR_CMD);
> }
>
> - if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
> - ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
> -!(tmp & AT91_PMC_LOCKU),
> -10, 5000);
> - if (ret)
> - pr_crit("USB PLL didn't lock when resuming\n");
> - }
> + if (pmc_cache.uckr & AT91_PMC_UPLLEN)
> + mask |= AT91_PMC_LOCKU;
> +
> + while (!pmc_ready(mask))
> + cpu_relax();
> }
>
> static struct syscore_ops pmc_syscore_ops = {
> --
> 2.11.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming
On 14/09/2017 at 18:15, Romain Izard wrote:
> 2017-09-13 14:15 GMT+02:00 Nicolas Ferre :
>> On 08/09/2017 at 17:35, Romain Izard wrote:
>>> Wait for the syncronization of all clocks when resuming, not only the
>>> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
>>> when interrupts are masked, which is the case in here.
>>>
>>> Signed-off-by: Romain Izard
>>> ---
>>> drivers/clk/at91/pmc.c | 24
>>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
>>> index 775af473fe11..5c2b26de303e 100644
>>> --- a/drivers/clk/at91/pmc.c
>>> +++ b/drivers/clk/at91/pmc.c
>>> @@ -107,10 +107,20 @@ static int pmc_suspend(void)
>>> return 0;
>>> }
>>>
>>> +static bool pmc_ready(unsigned int mask)
>>> +{
>>> + unsigned int status;
>>> +
>>> + regmap_read(pmcreg, AT91_PMC_SR, &status);
>>> +
>>> + return ((status & mask) == mask) ? 1 : 0;
>>> +}
>>> +
>>> static void pmc_resume(void)
>>> {
>>> - int i, ret = 0;
>>> + int i;
>>> u32 tmp;
>>> + u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
>>>
>>> regmap_read(pmcreg, AT91_PMC_MCKR, &tmp);
>>> if (pmc_cache.mckr != tmp)
>>> @@ -134,13 +144,11 @@ static void pmc_resume(void)
>>>AT91_PMC_PCR_CMD);
>>> }
>>>
>>> - if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
>>> - ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
>>> -!(tmp & AT91_PMC_LOCKU),
>>> -10, 5000);
>>> - if (ret)
>>> - pr_crit("USB PLL didn't lock when resuming\n");
>>> - }
>>> + if (pmc_cache.uckr & AT91_PMC_UPLLEN)
>>> + mask |= AT91_PMC_LOCKU;
>>> +
>>> + while (!pmc_ready(mask))
>>> + cpu_relax();
>>
>> Okay, but I would prefer to keep the timeout property in it. So we may
>> need to re-implement a timeout way-out here.
>>
>
> We need to have a reference clock to measure the timeout delay. If we use
> the kernel's timekeeping, it relies on the clocks that we are configuring in
> this code. Moreover, my experience with the mainline code is that when
> something goes wrong, nothing will work. No oops or panic will be reported,
> the device will just stop working.
>
> In my case, I had obvious failures (it just stopped working unless I removed
> USB wakeup or activated the console during suspend) but also very rare
> failures, that occurred in the bootloader. Those issues were detected when
> testing repeated suspend cycles for a night: the memory controller would
> never enter the self-refresh mode during the resume sequence.
>
> This led me to question the bootloader's code first, and I set up 4 boards
> with the backup prototype code on v4.9 to verify that it was stable on
> suspend. I've reached 1.5 million sleep cycles over 3 weeks without
> failure, so this hinted towards the difference between the prototype and the
> backup code provided for v4.12 (which contained the patch that got in
> v4.13). Once I integrated this patch, I've run the v4.12 code for 2 weeks
> without issue as well.
>
> In the end, I don't want to touch this code if I do not have to, as checking
> that it does not regress is really cumbersome.
The timeout was more for PLL like the one use for USB. I didn't want to
block only for USB PLL failure (which is kind of hypothetical, I admit).
Anyway, I understand your arguments and taking into account the
extensive tests that you've run, I agree with your approach. I'm adding
my Ack to the v2.
Thanks for having take the time to describe your debugging session: it's
valuable information for everybody.
Best regards,
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] clk: at91: pmc: Wait for clocks when resuming
On 15/09/2017 at 16:04, Romain Izard wrote:
> Wait for the syncronization of all clocks when resuming, not only the
> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
> when interrupts are masked, which is the case in here.
>
> Signed-off-by: Romain Izard
And here is my:
Acked-by: Nicolas Ferre
> ---
> drivers/clk/at91/pmc.c | 24
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
> index 775af473fe11..5c2b26de303e 100644
> --- a/drivers/clk/at91/pmc.c
> +++ b/drivers/clk/at91/pmc.c
> @@ -107,10 +107,20 @@ static int pmc_suspend(void)
> return 0;
> }
>
> +static bool pmc_ready(unsigned int mask)
> +{
> + unsigned int status;
> +
> + regmap_read(pmcreg, AT91_PMC_SR, &status);
> +
> + return ((status & mask) == mask) ? 1 : 0;
> +}
> +
> static void pmc_resume(void)
> {
> - int i, ret = 0;
> + int i;
> u32 tmp;
> + u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
>
> regmap_read(pmcreg, AT91_PMC_MCKR, &tmp);
> if (pmc_cache.mckr != tmp)
> @@ -134,13 +144,11 @@ static void pmc_resume(void)
>AT91_PMC_PCR_CMD);
> }
>
> - if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
> - ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
> -!(tmp & AT91_PMC_LOCKU),
> -10, 5000);
> - if (ret)
> - pr_crit("USB PLL didn't lock when resuming\n");
> - }
> + if (pmc_cache.uckr & AT91_PMC_UPLLEN)
> + mask |= AT91_PMC_LOCKU;
> +
> + while (!pmc_ready(mask))
> + cpu_relax();
> }
>
> static struct syscore_ops pmc_syscore_ops = {
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/9] ehci-atmel: Power down during suspend is normal
On 15/09/2017 at 16:04, Romain Izard wrote: > When an Atmel SoC is suspended with the backup mode, the USB bus will be > powered down. As this is expected, do not return an error to the driver > core when ehci_resume detects it. > > Signed-off-by: Romain Izard > --- > drivers/usb/host/ehci-atmel.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c > index 7440722bfbf0..2a8b9bdc0e57 100644 > --- a/drivers/usb/host/ehci-atmel.c > +++ b/drivers/usb/host/ehci-atmel.c > @@ -205,7 +205,8 @@ static int __maybe_unused ehci_atmel_drv_resume(struct > device *dev) > struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd); > > atmel_start_clock(atmel_ehci); > - return ehci_resume(hcd, false); > + ehci_resume(hcd, false); > + return 0; Ok, I agree with that as the underlying function takes care about the controller, in any case (even for !B+S-R case). So we don't have any added value to propagate this information. Acked-by: Nicolas Ferre > } > > #ifdef CONFIG_OF > -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/9] pwm: atmel-tcb: Support backup mode
On 15/09/2017 at 16:04, Romain Izard wrote:
> Save and restore registers for the PWM on suspend and resume, which
> makes hibernation and backup modes possible.
>
> Signed-off-by: Romain Izard
Seems good to me:
Acked-by: Nicolas Ferre
> ---
> drivers/pwm/pwm-atmel-tcb.c | 63
> +++--
> 1 file changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
> index 75db585a2a94..acd3ce8ecf3f 100644
> --- a/drivers/pwm/pwm-atmel-tcb.c
> +++ b/drivers/pwm/pwm-atmel-tcb.c
> @@ -37,11 +37,20 @@ struct atmel_tcb_pwm_device {
> unsigned period;/* PWM period expressed in clk cycles */
> };
>
> +struct atmel_tcb_channel {
> + u32 enabled;
> + u32 cmr;
> + u32 ra;
> + u32 rb;
> + u32 rc;
> +};
> +
> struct atmel_tcb_pwm_chip {
> struct pwm_chip chip;
> spinlock_t lock;
> struct atmel_tc *tc;
> struct atmel_tcb_pwm_device *pwms[NPWM];
> + struct atmel_tcb_channel bkup[NPWM / 2];
> };
>
> static inline struct atmel_tcb_pwm_chip *to_tcb_chip(struct pwm_chip *chip)
> @@ -175,12 +184,15 @@ static void atmel_tcb_pwm_disable(struct pwm_chip
> *chip, struct pwm_device *pwm)
>* Use software trigger to apply the new setting.
>* If both PWM devices in this group are disabled we stop the clock.
>*/
> - if (!(cmr & (ATMEL_TC_ACPC | ATMEL_TC_BCPC)))
> + if (!(cmr & (ATMEL_TC_ACPC | ATMEL_TC_BCPC))) {
> __raw_writel(ATMEL_TC_SWTRG | ATMEL_TC_CLKDIS,
>regs + ATMEL_TC_REG(group, CCR));
> - else
> + tcbpwmc->bkup[group].enabled = 1;
> + } else {
> __raw_writel(ATMEL_TC_SWTRG, regs +
>ATMEL_TC_REG(group, CCR));
> + tcbpwmc->bkup[group].enabled = 0;
> + }
>
> spin_unlock(&tcbpwmc->lock);
> }
> @@ -263,6 +275,7 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip,
> struct pwm_device *pwm)
> /* Use software trigger to apply the new setting */
> __raw_writel(ATMEL_TC_CLKEN | ATMEL_TC_SWTRG,
>regs + ATMEL_TC_REG(group, CCR));
> + tcbpwmc->bkup[group].enabled = 1;
> spin_unlock(&tcbpwmc->lock);
> return 0;
> }
> @@ -445,10 +458,56 @@ static const struct of_device_id atmel_tcb_pwm_dt_ids[]
> = {
> };
> MODULE_DEVICE_TABLE(of, atmel_tcb_pwm_dt_ids);
>
> +#ifdef CONFIG_PM_SLEEP
> +static int atmel_tcb_pwm_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
> + void __iomem *base = tcbpwm->tc->regs;
> + int i;
> +
> + for (i = 0; i < (NPWM / 2); i++) {
> + struct atmel_tcb_channel *chan = &tcbpwm->bkup[i];
> +
> + chan->cmr = readl(base + ATMEL_TC_REG(i, CMR));
> + chan->ra = readl(base + ATMEL_TC_REG(i, RA));
> + chan->rb = readl(base + ATMEL_TC_REG(i, RB));
> + chan->rc = readl(base + ATMEL_TC_REG(i, RC));
> + }
> + return 0;
> +}
> +
> +static int atmel_tcb_pwm_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
> + void __iomem *base = tcbpwm->tc->regs;
> + int i;
> +
> + for (i = 0; i < (NPWM / 2); i++) {
> + struct atmel_tcb_channel *chan = &tcbpwm->bkup[i];
> +
> + writel(chan->cmr, base + ATMEL_TC_REG(i, CMR));
> + writel(chan->ra, base + ATMEL_TC_REG(i, RA));
> + writel(chan->rb, base + ATMEL_TC_REG(i, RB));
> + writel(chan->rc, base + ATMEL_TC_REG(i, RC));
> + if (chan->enabled) {
> + writel(ATMEL_TC_CLKEN | ATMEL_TC_SWTRG,
> + base + ATMEL_TC_REG(i, CCR));
> + }
> + }
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(atmel_tcb_pwm_pm_ops, atmel_tcb_pwm_suspend,
> + atmel_tcb_pwm_resume);
> +
> static struct platform_driver atmel_tcb_pwm_driver = {
> .driver = {
> .name = "atmel-tcb-pwm",
> .of_match_table = atmel_tcb_pwm_dt_ids,
> + .pm = &atmel_tcb_pwm_pm_ops,
> },
> .probe = atmel_tcb_pwm_probe,
> .remove = atmel_tcb_pwm_remove,
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
usb/media/zr364xx: GPF in zr364xx_vidioc_querycap/strlcpy
Hi! I've got the following report while fuzzing the kernel with syzkaller. On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). usb 1-1: new full-speed USB device number 2 using dummy_hcd gadgetfs: connected gadgetfs: disconnected gadgetfs: connected usb 1-1: config 225 has an invalid interface number: 1 but max is 0 usb 1-1: config 225 has no interface number 0 usb 1-1: config 225 interface 1 altsetting 0 endpoint 0x5 has invalid maxpacket 2047, setting to 64 usb 1-1: config 225 interface 1 altsetting 0 has an invalid endpoint with address 0xF5, skipping usb 1-1: config 225 interface 1 altsetting 0 endpoint 0x8A has invalid maxpacket 2047, setting to 64 usb 1-1: config 225 interface 1 altsetting 0 endpoint 0x81 has an invalid bInterval 0, changing to 10 usb 1-1: config 225 interface 1 altsetting 0 endpoint 0x81 has invalid maxpacket 1025, setting to 64 usb 1-1: config 225 interface 1 altsetting 0 has an invalid endpoint with address 0xF7, skipping usb 1-1: config 225 interface 1 altsetting 0 has an invalid endpoint with address 0xB8, skipping usb 1-1: New USB device found, idVendor=041e, idProduct=4024 usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=1 usb 1-1: SerialNumber: a gadgetfs: configuration #225 zr364xx 1-1:225.1: Zoran 364xx compatible webcam plugged zr364xx 1-1:225.1: model 041e:4024 detected usb 1-1: 320x240 mode selected usb 1-1: Zoran 364xx controlling device video0 kasan: CONFIG_KASAN_INLINE enabled kasan: GPF could be caused by NULL-ptr deref or user memory access general protection fault: [#1] PREEMPT SMP KASAN Modules linked in: CPU: 0 PID: 4306 Comm: v4l_id Not tainted 4.14.0-rc1-42261-ga67ef73a6f27 #225 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 task: 88006a27e300 task.stack: 880067f7 RIP: 0010:strlcpy+0x21/0x120 lib/string.c:140 RSP: 0018:880067f777a0 EFLAGS: 00010286 RAX: dc00 RBX: 880067f77c00 RCX: RDX: 0020 RSI: RDI: 880067f77c10 RBP: 880067f777c8 R08: ed000cfeef82 R09: ed000cfeef82 R10: 0002 R11: ed000cfeef81 R12: 880067f77c10 R13: 880063034400 R14: 8000 R15: 880063193180 FS: 7f5561fe8700() GS:88006c80() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f5561b16110 CR3: 6b00c000 CR4: 06f0 Call Trace: zr364xx_vidioc_querycap+0xb8/0x220 drivers/media/usb/zr364xx/zr364xx.c:709 v4l_querycap+0x134/0x370 drivers/media/v4l2-core/v4l2-ioctl.c:1008 __video_do_ioctl+0x9c6/0xa80 drivers/media/v4l2-core/v4l2-ioctl.c:2750 video_usercopy+0x4ea/0x1580 drivers/media/v4l2-core/v4l2-ioctl.c:2926 video_ioctl2+0x31/0x40 drivers/media/v4l2-core/v4l2-ioctl.c:2968 v4l2_ioctl+0x1c5/0x310 drivers/media/v4l2-core/v4l2-dev.c:360 vfs_ioctl fs/ioctl.c:45 do_vfs_ioctl+0x1c4/0x15c0 fs/ioctl.c:685 SYSC_ioctl fs/ioctl.c:700 SyS_ioctl+0x94/0xc0 fs/ioctl.c:691 entry_SYSCALL_64_fastpath+0x23/0xc2 arch/x86/entry/entry_64.S:202 RIP: 0033:0x7f5561b1b347 RSP: 002b:7ffd403d19a8 EFLAGS: 0202 ORIG_RAX: 0010 RAX: ffda RBX: 7ffd403d1b00 RCX: 7f5561b1b347 RDX: 7ffd403d19b0 RSI: 80685600 RDI: 0003 RBP: 00400884 R08: R09: R10: R11: 0202 R12: 0003 R13: 7ffd403d1b00 R14: R15: Code: 8b 45 f0 e9 64 ff ff ff 66 90 48 b8 00 00 00 00 00 fc ff df 55 48 89 f1 48 89 e5 48 c1 e9 03 41 55 41 54 49 89 fc 53 48 83 ec 10 <0f> b6 04 01 48 89 f1 83 e1 07 38 c8 7f 08 84 c0 0f 85 9d 00 00 RIP: strlcpy+0x21/0x120 RSP: 880067f777a0 ---[ end trace 23c9876972269088 ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode when configured as such
Hi,
On 09/11/2017 12:56 AM, Guenter Roeck wrote:
On 09/05/2017 09:42 AM, Hans de Goede wrote:
Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
data-role is device is not correct. Plenty of devices support operating
as USB device through a (separate) USB device controller.
So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
accordingly.
Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
this is for configuring the Type-C port as a Display Port source, not a
sink.
Last this commit makes tcpm_reset_port() to set the mux to
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT so that it does not and up
staying in host (and with this commit also device) mode after a detach.
This sentence is hard to understand.
Ok, changed to:
"Last this commit makes tcpm_reset_port() call tcpm_mux_set(port,
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT) so that the mux does _not_
stay in its last mode after a detach."
For v3 of this patchset.
Signed-off-by: Hans de Goede
Otherwise
Reviewed-by: Guenter Roeck
And added your Reviewed-by.
Thanks & Regards,
Hans
---
drivers/staging/typec/tcpm.c | 7 ---
drivers/staging/typec/tcpm.h | 22 ++
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..ffe7e26d4ed3 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -752,11 +752,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool
attached,
int ret;
if (data == TYPEC_HOST)
- ret = tcpm_mux_set(port, TYPEC_MUX_USB,
+ ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
TCPC_USB_SWITCH_CONNECT);
else
- ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
- TCPC_USB_SWITCH_DISCONNECT);
+ ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
+ TCPC_USB_SWITCH_CONNECT);
if (ret < 0)
return ret;
@@ -2025,6 +2025,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
tcpm_init_vconn(port);
tcpm_set_current_limit(port, 0, 0);
tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
+ tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
tcpm_set_attached_state(port, false);
port->try_src_count = 0;
port->try_snk_count = 0;
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..f662eed48c86 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -83,17 +83,23 @@ enum tcpc_usb_switch {
};
/* Mux state attributes */
-#define TCPC_MUX_USB_ENABLED BIT(0) /* USB enabled */
-#define TCPC_MUX_DP_ENABLED BIT(1) /* DP enabled */
-#define TCPC_MUX_POLARITY_INVERTED BIT(2) /* Polarity inverted */
+#define TCPC_MUX_USB_DEVICE_ENABLED BIT(0) /* USB device enabled */
+#define TCPC_MUX_USB_HOST_ENABLED BIT(1) /* USB host enabled */
+#define TCPC_MUX_DP_SRC_ENABLED BIT(2) /* DP enabled */
+#define TCPC_MUX_POLARITY_INVERTED BIT(3) /* Polarity inverted */
/* Mux modes, decoded to attributes */
enum tcpc_mux_mode {
- TYPEC_MUX_NONE = 0, /* Open switch */
- TYPEC_MUX_USB = TCPC_MUX_USB_ENABLED, /* USB only */
- TYPEC_MUX_DP = TCPC_MUX_DP_ENABLED, /* DP only */
- TYPEC_MUX_DOCK = TCPC_MUX_USB_ENABLED | /* Both USB and DP */
- TCPC_MUX_DP_ENABLED,
+ /* Open switch */
+ TYPEC_MUX_NONE = 0,
+ /* USB device only */
+ TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
+ /* USB host only */
+ TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
+ /* DP source only */
+ TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
+ /* Both USB host and DP source */
+ TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
};
struct tcpc_mux_dev {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/storage/uas: slab-out-of-bounds in uas_probe
On Fri, 22 Sep 2017, Greg Kroah-Hartman wrote: > On Fri, Sep 22, 2017 at 09:58:15AM +0200, Greg Kroah-Hartman wrote: > > On Thu, Sep 21, 2017 at 03:04:05PM -0400, Alan Stern wrote: > > > On Thu, 21 Sep 2017, Andrey Konovalov wrote: > > > > > > > On Thu, Sep 21, 2017 at 6:10 PM, Greg Kroah-Hartman > > > > wrote: > > > > > On Thu, Sep 21, 2017 at 05:39:05PM +0200, Andrey Konovalov wrote: > > > > >> Hi! > > > > >> > > > > >> I've got the following report while fuzzing the kernel with > > > > >> syzkaller. > > > > >> > > > > >> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). > > > > >> > > > > >> The issue occurs when we iterate over interface altsettings, but I > > > > >> don't see the driver doing anything wrong. I might be missing > > > > >> something, or this might be an issue in USB core altsettings parsing. > > > > > > > > > > > > > > > Any chance you happen to have the descriptors that you were feeding > > > > > into > > > > > the kernel at this crash? That might help in figuring out what "went > > > > > wrong". > > > > > > > > Here's the data that I feed into dummy_udc: > > > > > > > > 00 00 00 00 09 02 12 00 01 34 05 80 07 09 04 6e > > > > 09 00 08 06 62 00 12 01 05 00 cb f7 71 83 04 00 > > > > 05 00 ab f6 07 81 08 01 > > > > > > > > Also attaching a C reproducer (requires dummy_hcd and gadgetfs) and my > > > > .config. > > > > > > Why do your reproducers use an mmap'ed array for their data instead of > > > a plain old statically allocated array? > > > > > > Anyway, this turns out to be a genuine (and subtle!) bug in the uas > > > driver. The uas_find_uas_alt_setting() routine in uas-detect.h returns > > > a bAlternateSetting value, but then the uas_use_uas_driver() routine > > > uses this value as an index to the altsetting array -- which it isn't. > > > > > > Normally this doesn't cause any problems because the the entries in the > > > array have bAlternateSetting values 0, 1, etc., so the value is equal > > > to the index. But in your fuzzed case, that wasn't true. > > > > > > The patch below fixes this bug, by returning a pointer to the > > > alt-setting entry instead of either the value or the index. Pointers > > > are less subject to misinterpretation. > > > > Ugh, messy, nice catch and fix, I'll go queue it up now, thanks for > > resolving this. > > Oops, no, you didn't send this as a "real" patch yet, I'll wait :) I was waiting for Andrey to verify that the problem was really gone. I'll submit it officially later today. On Fri, 22 Sep 2017, Felipe Balbi wrote: > > Felipe, any objection for me taking this, and the other gadget driver > > fixes that Alan just sent out, directly in my tree? > > none whatsoever, for all of them: There's still more to come. I've got four patches for dummy-hcd just about ready to send. :-) Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ASM1153 detected as ASM1051 and breaking UAS
On Tue, 2017-09-19 at 10:00 +0200, Oliver Neukum wrote: > > "In order to use UAS that must be enabled in the firmware as well. On that > > device it is not enabled and the official product id wouldn't help there > > either." > > > > What does that mean? Did I purchase a UAS capable hardware with the wrong > > Yes. That is what they told you. With a bit of sugar coating. > > If they provide a tool for updating the firmware and updating the firmware > is supported. Which may be the case or not. Only the vendor can tell you. Heureka! Before buying a new case with a UAS capable bridge I tried flashing some unknown firmware. I searched for "ASM1153 firmware" and found http://www.station-drivers.com/index.php?option=com_remository&Itemid=353&func=fileinfo&id=2842&lang=en I fired up some VirtualBox with extensions to be able to flash that USB3 device. Flashing worked, however ReLink failed. Now plugging to Linux I finally see holy land: [kernel] scsi host9: uas [kernel] usb 4-1: Product: USB3-SATA-UASP1 Opening with cryptsetup showed [kernel] device-mapper: table: 254:4: adding target device sdf caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=2097152 But reading works fine. However the new descriptor of my device now looks like: Bus 004 Device 007: ID 174c:55aa ASMedia Technology Inc. ASM1051E SATA 6Gb/s bridge, ASM1053E SATA 6Gb/s bridge, ASM1153 SATA 3Gb/s bridge Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x174c ASMedia Technology Inc. idProduct 0x55aa ASM1051E SATA 6Gb/s bridge, ASM1053E SATA 6Gb/s bridge, ASM1153 SATA 3Gb/s bridge bcdDevice1.00 iManufacturer 2 Plugable iProduct3 USB3-SATA-UASP1 iSerial 1 123456789012 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 121 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only I wondered the InterfaceClass is still "Mass Storage" but now the uas module has taken over. Best regards, Massimo signature.asc Description: This is a digitally signed message part
Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface
On Thu, Sep 21, 2017 at 05:07:14PM +0200, Greg KH wrote: > On Thu, Sep 21, 2017 at 05:51:29PM +0300, Serge Semin wrote: > > On Thu, Sep 21, 2017 at 10:23:38AM +0200, Greg KH > > wrote: > > > On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote: > > > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > > > > index 71994b883..c2dd9742f 100644 > > > > --- a/drivers/usb/misc/usb251xb.c > > > > +++ b/drivers/usb/misc/usb251xb.c > > > > @@ -3,6 +3,7 @@ > > > > * Configuration via SMBus. > > > > * > > > > * Copyright (c) 2017 SKIDATA AG > > > > + * Copyright (c) 2017 T-platforms > > > > > > Again, no, please consult with your corporate lawyers why this isn't ok. > > > > > > greg k-h > > > > I still can't see why this isn't right. We submitted the patchset. It is not > > that big and still it isn't just two lines. As I've seen all over the > > kernel, It is > > a common practice to have multiple copyrights in kernel files. We are not > > claiming > > the copyright to the whole file, but to the contribution only. I got a > > consent to > > contribute when I was employed by the company. What's wrong with that? > > Shall I > > send the patchset from my corporate e-mail then? > > Well, yes, I need some way to properly identify that this corporation > did do the changes. I said that before, I don't know why you ignored > that. > > And yes, multiple copyrights are just fine, but again, please talk to > your corporate lawyer about why these changes don't seem to warrant that > "mark". If they do think that they do warrant that, great, I will be > glad to discuss that with them, off-list if needed. > > For even more fun, try discussing with your lawyers about why copyright > marks like this don't even mean anything anymore, and haven't for 20+ > years now (can't remember the actual date...) But that's a different > topic, and one not really relevant here. > > thanks, > > greg k-h Alright then. We'll remove the Copyright mark and I'll resend the patchset from my corporate e-mail. Hope it will solve this issue. Could you review the rest of the patchset? Regards, -Sergey -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: uas: fix bug in handling of alternate settings
The uas driver has a subtle bug in the way it handles alternate
settings. The uas_find_uas_alt_setting() routine returns an
altsetting value (the bAlternateSetting number in the descriptor), but
uas_use_uas_driver() then treats that value as an index to the
intf->altsetting array, which it isn't.
Normally this doesn't cause any problems because the various
alternate settings have bAlternateSetting values 0, 1, 2, ..., so the
value is equal to the index in the array. But this is not guaranteed,
and Andrey Konovalov used the syzkaller fuzzer with KASAN to get a
slab-out-of-bounds error by violating this assumption.
This patch fixes the bug by making uas_find_uas_alt_setting() return a
pointer to the altsetting entry rather than either the value or the
index. Pointers are less subject to misinterpretation.
Signed-off-by: Alan Stern
Reported-by: Andrey Konovalov
Tested-by: Andrey Konovalov
CC: Oliver Neukum
CC:
---
[as1845]
drivers/usb/storage/uas-detect.h | 15 ---
drivers/usb/storage/uas.c| 10 +-
2 files changed, 13 insertions(+), 12 deletions(-)
Index: usb-4.x/drivers/usb/storage/uas-detect.h
===
--- usb-4.x.orig/drivers/usb/storage/uas-detect.h
+++ usb-4.x/drivers/usb/storage/uas-detect.h
@@ -9,7 +9,8 @@ static int uas_is_interface(struct usb_h
intf->desc.bInterfaceProtocol == USB_PR_UAS);
}
-static int uas_find_uas_alt_setting(struct usb_interface *intf)
+static struct usb_host_interface *uas_find_uas_alt_setting(
+ struct usb_interface *intf)
{
int i;
@@ -17,10 +18,10 @@ static int uas_find_uas_alt_setting(stru
struct usb_host_interface *alt = &intf->altsetting[i];
if (uas_is_interface(alt))
- return alt->desc.bAlternateSetting;
+ return alt;
}
- return -ENODEV;
+ return NULL;
}
static int uas_find_endpoints(struct usb_host_interface *alt,
@@ -58,14 +59,14 @@ static int uas_use_uas_driver(struct usb
struct usb_device *udev = interface_to_usbdev(intf);
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
unsigned long flags = id->driver_info;
- int r, alt;
-
+ struct usb_host_interface *alt;
+ int r;
alt = uas_find_uas_alt_setting(intf);
- if (alt < 0)
+ if (!alt)
return 0;
- r = uas_find_endpoints(&intf->altsetting[alt], eps);
+ r = uas_find_endpoints(alt, eps);
if (r < 0)
return 0;
Index: usb-4.x/drivers/usb/storage/uas.c
===
--- usb-4.x.orig/drivers/usb/storage/uas.c
+++ usb-4.x/drivers/usb/storage/uas.c
@@ -873,14 +873,14 @@ MODULE_DEVICE_TABLE(usb, uas_usb_ids);
static int uas_switch_interface(struct usb_device *udev,
struct usb_interface *intf)
{
- int alt;
+ struct usb_host_interface *alt;
alt = uas_find_uas_alt_setting(intf);
- if (alt < 0)
- return alt;
+ if (!alt)
+ return -ENODEV;
- return usb_set_interface(udev,
- intf->altsetting[0].desc.bInterfaceNumber, alt);
+ return usb_set_interface(udev, alt->desc.bInterfaceNumber,
+ alt->desc.bAlternateSetting);
}
static int uas_configure_endpoints(struct uas_dev_info *devinfo)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface
On Fri, Sep 22, 2017 at 06:26:54PM +0300, Serge Semin wrote: > On Thu, Sep 21, 2017 at 05:07:14PM +0200, Greg KH > wrote: > > On Thu, Sep 21, 2017 at 05:51:29PM +0300, Serge Semin wrote: > > > On Thu, Sep 21, 2017 at 10:23:38AM +0200, Greg KH > > > wrote: > > > > On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote: > > > > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > > > > > index 71994b883..c2dd9742f 100644 > > > > > --- a/drivers/usb/misc/usb251xb.c > > > > > +++ b/drivers/usb/misc/usb251xb.c > > > > > @@ -3,6 +3,7 @@ > > > > > * Configuration via SMBus. > > > > > * > > > > > * Copyright (c) 2017 SKIDATA AG > > > > > + * Copyright (c) 2017 T-platforms > > > > > > > > Again, no, please consult with your corporate lawyers why this isn't ok. > > > > > > > > greg k-h > > > > > > I still can't see why this isn't right. We submitted the patchset. It is > > > not > > > that big and still it isn't just two lines. As I've seen all over the > > > kernel, It is > > > a common practice to have multiple copyrights in kernel files. We are not > > > claiming > > > the copyright to the whole file, but to the contribution only. I got a > > > consent to > > > contribute when I was employed by the company. What's wrong with that? > > > Shall I > > > send the patchset from my corporate e-mail then? > > > > Well, yes, I need some way to properly identify that this corporation > > did do the changes. I said that before, I don't know why you ignored > > that. > > > > And yes, multiple copyrights are just fine, but again, please talk to > > your corporate lawyer about why these changes don't seem to warrant that > > "mark". If they do think that they do warrant that, great, I will be > > glad to discuss that with them, off-list if needed. > > > > For even more fun, try discussing with your lawyers about why copyright > > marks like this don't even mean anything anymore, and haven't for 20+ > > years now (can't remember the actual date...) But that's a different > > topic, and one not really relevant here. > > > > thanks, > > > > greg k-h > > Alright then. We'll remove the Copyright mark and I'll resend the patchset > from my corporate e-mail. Hope it will solve this issue. > Could you review the rest of the patchset? I'll wait for the resend, as Rob already pointed out issues, so it is long-gone from my patchqueue. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fibocom L831-EAU and cdc_mbim
On 19/09/17 17:38, Bjørn Mork wrote: > Andreas Böhler writes: > >> On 19/09/17 14:25, Bjørn Mork wrote: >>> Bjørn Mork writes: >>> >>> Maybe we should simply ignore stalls completely? They should not happen >>> with any real WDM device anyway. >>> >>> What happens if you e.g. change the if-test here: >>> >>> /* >>> * only set a new error if there is no previous error. >>> * Errors are only cleared during read/open >>> */ >>> if (desc->rerr == 0) >>> desc->rerr = status; >>> >>> >>> to >>> >>> if (desc->rerr == 0 && status != -EPIPE) >> >> Well, this looks very promising: Although the error messages persist >> every now and then, the device now works! >> >> I'll make the new module the default and I'll test it more thouroughly >> during the next few days. As promised I made the new module the default and tested it for a few days. Although the error messages persist, I did not notice any negative side effects. In fact, it also fixes a problem where mbim-proxy was at 100% load. Two other users also reported success [1], so I'm looking forward to having it fixed in the default kernel tree. In the meantime, I'll keep the module patched using DKMS. Thanks again for your help, Andreas [1] https://bugs.freedesktop.org/show_bug.cgi?id=100938 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue
inc_deq() currently bails earlier for EVENT rings than the common return
point of the function, due to the fact that EVENT rings do not have
link TRBs. The unfortunate side effect of this is that the very useful
trace_xhci_inc_deq() function is not called/usable for EVENT ring
debug.
This patch provides a refactor by removing the multiple return exit
points into a single return which additionally allows for all rings to
use the trace function.
Signed-off-by: Adam Wallis
---
drivers/usb/host/xhci-ring.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a944365..10afd50 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -171,25 +171,24 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (ring->type == TYPE_EVENT) {
if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
ring->dequeue++;
- return;
+ } else {
+ if (last_trb_on_ring(ring, ring->deq_seg,
+ ring->dequeue))
+ ring->cycle_state ^= 1;
+ ring->deq_seg = ring->deq_seg->next;
+ ring->dequeue = ring->deq_seg->trbs;
+ }
+ } else {
+ /* All other rings have link trbs */
+ if (!trb_is_link(ring->dequeue)) {
+ ring->dequeue++;
+ ring->num_trbs_free++;
+ }
+ while (trb_is_link(ring->dequeue)) {
+ ring->deq_seg = ring->deq_seg->next;
+ ring->dequeue = ring->deq_seg->trbs;
}
- if (last_trb_on_ring(ring, ring->deq_seg, ring->dequeue))
- ring->cycle_state ^= 1;
- ring->deq_seg = ring->deq_seg->next;
- ring->dequeue = ring->deq_seg->trbs;
- return;
- }
-
- /* All other rings have link trbs */
- if (!trb_is_link(ring->dequeue)) {
- ring->dequeue++;
- ring->num_trbs_free++;
- }
- while (trb_is_link(ring->dequeue)) {
- ring->deq_seg = ring->deq_seg->next;
- ring->dequeue = ring->deq_seg->trbs;
}
-
trace_xhci_inc_deq(ring);
return;
--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fibocom L831-EAU and cdc_mbim
Andreas Böhler writes: > As promised I made the new module the default and tested it for a few > days. Although the error messages persist, I did not notice any negative > side effects. In fact, it also fixes a problem where mbim-proxy was at > 100% load. > > Two other users also reported success [1], so I'm looking forward to > having it fixed in the default kernel tree. In the meantime, I'll keep > the module patched using DKMS. Sounds good. Ideally we should also get it tested with lots of other devices. But the only way I know of to make that happen is by pushing the patch into mainline and stable kernels. I'll do some minimal testing myself for a while and then submit. Thanks for your patience. I know this issue has been around for way too long already. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 14/14] platform/x86: intel_cht_int33fe: Add mux mappings for the Type-C port
We need to add mappings for the mux subsys to be able to find the
muxes for the fusb302 driver to be able to control the PI3USB30532
Type-C mux and the device/host mux integrated in the CHT SoC.
Signed-off-by: Hans de Goede
Acked-by: Andy Shevchenko
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_cht_int33fe.c | 23 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 312d2472b8b3..830ff8d0a1eb 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -794,6 +794,7 @@ config ACPI_CMPC
config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
+ select MULTIPLEXER
---help---
This driver add support for the INT33FE ACPI device found on
some Intel Cherry Trail devices.
diff --git a/drivers/platform/x86/intel_cht_int33fe.c
b/drivers/platform/x86/intel_cht_int33fe.c
index b2925d996613..89ba510dac78 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -35,6 +36,19 @@ struct cht_int33fe_data {
struct i2c_client *pi3usb30532;
};
+static struct mux_lookup cht_int33fe_mux_lookup[] = {
+ {
+ .provider = "i2c-pi3usb30532",
+ .dev_id = "i2c-fusb302",
+ .mux_name = "type-c-mode-mux",
+ },
+ {
+ .provider = "intel_cht_usb_mux",
+ .dev_id = "i2c-fusb302",
+ .mux_name = "usb-role-mux",
+ },
+};
+
/*
* Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates
* the max17047 both through the INT33FE ACPI device (it is right there
@@ -177,6 +191,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
board_info.properties = fusb302_props;
board_info.irq = fusb302_irq;
+ mux_add_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
if (!data->fusb302)
goto out_unregister_max17047;
@@ -200,6 +217,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
+ mux_remove_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
}
@@ -212,6 +232,9 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
+ mux_remove_table(cht_int33fe_mux_lookup,
+ ARRAY_SIZE(cht_int33fe_mux_lookup));
+
return 0;
}
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 13/14] staging: typec: fusb302: Hook up mux support using tcpc_gen_mux support
Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()
to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.
Also document the mux-names used by the generic tcpc_mux_dev code in
our devicetree bindings.
Cc: Rob Herring
Cc: Mark Rutland
Cc: devicet...@vger.kernel.org
Signed-off-by: Hans de Goede
---
Changes in v3:
-Drop devicetree bindings documentation, since this is only used with
device-properties set by platform code on X86/ACPI now, we don't need
bindings yet
---
drivers/usb/typec/fusb302/fusb302.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/fusb302/fusb302.c
b/drivers/usb/typec/fusb302/fusb302.c
index e790b67d4953..10415d6b2292 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1259,7 +1259,6 @@ static void init_tcpc_dev(struct tcpc_dev
*fusb302_tcpc_dev)
fusb302_tcpc_dev->set_roles = tcpm_set_roles;
fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
- fusb302_tcpc_dev->mux = NULL;
}
static const char * const cc_polarity_name[] = {
@@ -1817,6 +1816,10 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
+ chip->tcpc_dev.mux = devm_tcpc_gen_mux_create(dev);
+ if (IS_ERR(chip->tcpc_dev.mux))
+ return PTR_ERR(chip->tcpc_dev.mux);
+
cfg.drv_data = chip;
chip->psy = devm_power_supply_register(dev, &fusb302_psy_desc, &cfg);
if (IS_ERR(chip->psy)) {
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 12/14] staging: typec: Add Generic TCPC mux driver using the mux subsys
So far the mux functionality of the tcpm code has not been hooked up
in any tcpc drivers. This commit adds a generic TCPC mux driver using
the mux subsys, which tcpc drivers can use to provide mux functionality
in cases where an external my is used.
Signed-off-by: Hans de Goede
---
Changes in v3:
-Use new devm_mux_control_get_optional()
-Simplify tcpc_gen_mux_set as we now no longer need to worry about the muxes
being present
-Adjust for updated MUX_TYPEC_* defines
---
drivers/usb/typec/Makefile | 2 +-
drivers/usb/typec/tcpc_gen_mux.c | 122 +++
include/linux/usb/tcpm.h | 2 +
3 files changed, 125 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/typec/tcpc_gen_mux.c
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index b77688ce1f16..95a7d9c4527b 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_TYPEC)+= typec.o
-obj-$(CONFIG_TYPEC_TCPM) += tcpm.o
+obj-$(CONFIG_TYPEC_TCPM) += tcpm.o tcpc_gen_mux.o
obj-y += fusb302/
obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o
obj-$(CONFIG_TYPEC_UCSI) += ucsi/
diff --git a/drivers/usb/typec/tcpc_gen_mux.c b/drivers/usb/typec/tcpc_gen_mux.c
new file mode 100644
index ..ea40b52ef6a6
--- /dev/null
+++ b/drivers/usb/typec/tcpc_gen_mux.c
@@ -0,0 +1,122 @@
+/*
+ * Generic TCPC mux driver using the mux subsys
+ *
+ * Copyright (c) 2017 Hans de Goede
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation, or (at your option)
+ * any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct tcpc_gen_mux_data {
+ struct tcpc_mux_dev mux;
+ struct device *dev;
+ struct mux_control *type_c_mode_mux; /* Type-C cross switch / mux */
+ struct mux_control *usb_role_mux;/* USB Device / Host mode mux */
+ bool muxes_set;
+};
+
+static int tcpc_gen_mux_set(struct tcpc_mux_dev *mux_dev,
+ enum tcpc_mux_mode mux_mode,
+ enum tcpc_usb_switch usb_config,
+ enum typec_cc_polarity polarity)
+{
+ struct tcpc_gen_mux_data *data =
+ container_of(mux_dev, struct tcpc_gen_mux_data, mux);
+ unsigned int typec_state = MUX_TYPEC_USB;
+ unsigned int usb_state = MUX_USB_DEVICE;
+ int ret;
+
+ /* Put the muxes back in their open (idle) state */
+ if (data->muxes_set) {
+ mux_control_deselect(data->type_c_mode_mux);
+ mux_control_deselect(data->usb_role_mux);
+ data->muxes_set = false;
+ }
+
+ switch (mux_mode) {
+ case TYPEC_MUX_NONE:
+ /* Muxes are in their open state, done. */
+ return 0;
+ case TYPEC_MUX_USB_DEVICE:
+ typec_state = MUX_TYPEC_USB;
+ usb_state = MUX_USB_DEVICE;
+ break;
+ case TYPEC_MUX_USB_HOST:
+ typec_state = MUX_TYPEC_USB;
+ usb_state = MUX_USB_HOST;
+ break;
+ case TYPEC_MUX_DP:
+ typec_state = MUX_TYPEC_DP;
+ break;
+ case TYPEC_MUX_DOCK:
+ typec_state = MUX_TYPEC_USB_AND_DP;
+ usb_state = MUX_USB_HOST;
+ break;
+ }
+
+ if (polarity)
+ typec_state |= MUX_TYPEC_POLARITY_INV;
+
+ ret = mux_control_select(data->type_c_mode_mux, typec_state);
+ if (ret) {
+ dev_err(data->dev, "Error setting Type-C mode mux: %d\n", ret);
+ return ret;
+ }
+
+ ret = mux_control_select(data->usb_role_mux, usb_state);
+ if (ret) {
+ dev_err(data->dev, "Error setting USB role mux: %d\n", ret);
+ mux_control_deselect(data->type_c_mode_mux);
+ return ret;
+ }
+
+ data->muxes_set = true;
+ return 0;
+}
+
+struct tcpc_mux_dev *devm_tcpc_gen_mux_create(struct device *dev)
+{
+ struct tcpc_gen_mux_data *data;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return ERR_PTR(-ENOMEM);
+
+ /* The use of either mux is optional */
+ data->type_c_mode_mux =
+ devm_mux_control_get_optional(dev, "type-c-mode-mux");
+ if (IS_ERR(data->type_c_mode_mux)) {
+ ret = PTR_ERR(data->type_c_mode_mux);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Error getting Type-C mux: %d\n", ret);
+ return ERR_PTR(-ret);
+ }
+
+ data->usb_role_mux = devm_mux_control_get_optional(dev, "usb-role-mux");
+ if (IS_ERR(data->usb_role_mux)) {
+ ret = PTR_ERR(data->usb_role_mux);
+ if (ret != -EPR
[PATCH v3 11/14] staging: typec: tcpm: Set mux to device mode when configured as such
Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
data-role is device is not correct. Plenty of devices support operating
as USB device through a (separate) USB device controller.
So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
accordingly.
Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
this is for configuring the Type-C port as a Display Port source, not a
sink.
Last this commit makes tcpm_reset_port() call tcpm_mux_set(port,
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT) so that the mux does _not_
stay in its last mode after a detach.
Signed-off-by: Hans de Goede
Reviewed-by: Guenter Roeck
---
Changes in v3:
-Improve / reword last paragraph of the commit message
-Add Guenter's Reviewed-by
---
drivers/usb/typec/tcpm.c | 7 ---
include/linux/usb/tcpm.h | 22 ++
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index f557c479fdc2..e104c218cb4a 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -751,11 +751,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool
attached,
int ret;
if (data == TYPEC_HOST)
- ret = tcpm_mux_set(port, TYPEC_MUX_USB,
+ ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
TCPC_USB_SWITCH_CONNECT);
else
- ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
- TCPC_USB_SWITCH_DISCONNECT);
+ ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
+ TCPC_USB_SWITCH_CONNECT);
if (ret < 0)
return ret;
@@ -1995,6 +1995,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
tcpm_init_vconn(port);
tcpm_set_current_limit(port, 0, 0);
tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
+ tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
tcpm_set_attached_state(port, false);
port->try_src_count = 0;
port->try_snk_count = 0;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 073197f0d2bb..bc76389ee257 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -103,17 +103,23 @@ enum tcpc_usb_switch {
};
/* Mux state attributes */
-#define TCPC_MUX_USB_ENABLED BIT(0) /* USB enabled */
-#define TCPC_MUX_DP_ENABLEDBIT(1) /* DP enabled */
-#define TCPC_MUX_POLARITY_INVERTED BIT(2) /* Polarity inverted */
+#define TCPC_MUX_USB_DEVICE_ENABLEDBIT(0) /* USB device enabled */
+#define TCPC_MUX_USB_HOST_ENABLED BIT(1) /* USB host enabled */
+#define TCPC_MUX_DP_SRC_ENABLEDBIT(2) /* DP enabled */
+#define TCPC_MUX_POLARITY_INVERTED BIT(3) /* Polarity inverted */
/* Mux modes, decoded to attributes */
enum tcpc_mux_mode {
- TYPEC_MUX_NONE = 0,/* Open switch */
- TYPEC_MUX_USB = TCPC_MUX_USB_ENABLED, /* USB only */
- TYPEC_MUX_DP= TCPC_MUX_DP_ENABLED, /* DP only */
- TYPEC_MUX_DOCK = TCPC_MUX_USB_ENABLED |/* Both USB and DP */
- TCPC_MUX_DP_ENABLED,
+ /* Open switch */
+ TYPEC_MUX_NONE = 0,
+ /* USB device only */
+ TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
+ /* USB host only */
+ TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
+ /* DP source only */
+ TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
+ /* Both USB host and DP source */
+ TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
};
struct tcpc_mux_dev {
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 10/14] extcon: intel-int3496: Add support for controlling the USB-role mux
Cherry Trail SoCs have a built-in USB-role mux for switching between
the host and device controllers, rather then using an external mux
controller by a GPIO.
There is a driver using the mux-subsys to control this mux, this
commit adds support to the intel-int3496 driver to get a mux_controller
handle for the mux and set the mux through the mux-subsys rather then
through a GPIO.
Signed-off-by: Hans de Goede
---
Changes in v2:
-Drop || COMPILE_TEST from Kconfig depends on, as we will now fail to
compile on !X86
-Minor code style tweaks
---
drivers/extcon/Kconfig| 3 +-
drivers/extcon/extcon-intel-int3496.c | 59 +++
2 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index a7bca4207f44..168f9d710ea0 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -44,7 +44,8 @@ config EXTCON_GPIO
config EXTCON_INTEL_INT3496
tristate "Intel INT3496 ACPI device extcon driver"
- depends on GPIOLIB && ACPI && (X86 || COMPILE_TEST)
+ depends on GPIOLIB && ACPI && X86
+ select MULTIPLEXER
help
Say Y here to enable extcon support for USB OTG ports controlled by
an Intel INT3496 ACPI device.
diff --git a/drivers/extcon/extcon-intel-int3496.c
b/drivers/extcon/extcon-intel-int3496.c
index 1a45e745717d..3c8e17449c12 100644
--- a/drivers/extcon/extcon-intel-int3496.c
+++ b/drivers/extcon/extcon-intel-int3496.c
@@ -23,8 +23,13 @@
#include
#include
#include
+#include
+#include
#include
+#include
+#include
+
#define INT3496_GPIO_USB_ID0
#define INT3496_GPIO_VBUS_EN 1
#define INT3496_GPIO_USB_MUX 2
@@ -37,6 +42,8 @@ struct int3496_data {
struct gpio_desc *gpio_usb_id;
struct gpio_desc *gpio_vbus_en;
struct gpio_desc *gpio_usb_mux;
+ struct mux_control *usb_mux;
+ bool usb_mux_set;
int usb_id_irq;
};
@@ -56,11 +63,32 @@ static const struct acpi_gpio_mapping
acpi_int3496_default_gpios[] = {
{ },
};
+static struct mux_lookup acpi_int3496_cht_mux_lookup[] = {
+ {
+ .provider = "intel_cht_usb_mux",
+ .dev_id = "INT3496:00",
+ .mux_name = "usb-role-mux",
+ },
+};
+
+#define ICPU(model){ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id cht_cpu_ids[] = {
+ ICPU(INTEL_FAM6_ATOM_AIRMONT), /* Braswell, Cherry Trail */
+ {}
+};
+
+static bool int3496_soc_has_mux(void)
+{
+ return x86_match_cpu(cht_cpu_ids);
+}
+
static void int3496_do_usb_id(struct work_struct *work)
{
struct int3496_data *data =
container_of(work, struct int3496_data, work.work);
int id = gpiod_get_value_cansleep(data->gpio_usb_id);
+ int ret;
/* id == 1: PERIPHERAL, id == 0: HOST */
dev_dbg(data->dev, "Connected %s cable\n", id ? "PERIPHERAL" : "HOST");
@@ -72,6 +100,22 @@ static void int3496_do_usb_id(struct work_struct *work)
if (!IS_ERR(data->gpio_usb_mux))
gpiod_direction_output(data->gpio_usb_mux, id);
+ if (data->usb_mux) {
+ /*
+* The mux framework expects multiple competing users, we must
+* release our previous setting before applying the new one.
+*/
+ if (data->usb_mux_set)
+ mux_control_deselect(data->usb_mux);
+
+ ret = mux_control_select(data->usb_mux,
+id ? MUX_USB_DEVICE : MUX_USB_HOST);
+ if (ret)
+ dev_err(data->dev, "Error setting mux: %d\n", ret);
+
+ data->usb_mux_set = ret == 0;
+ }
+
if (!IS_ERR(data->gpio_vbus_en))
gpiod_direction_output(data->gpio_vbus_en, !id);
@@ -107,6 +151,21 @@ static int int3496_probe(struct platform_device *pdev)
data->dev = dev;
INIT_DELAYED_WORK(&data->work, int3496_do_usb_id);
+ if (int3496_soc_has_mux()) {
+ mux_add_table(acpi_int3496_cht_mux_lookup,
+ ARRAY_SIZE(acpi_int3496_cht_mux_lookup));
+ data->usb_mux = devm_mux_control_get(dev, "usb-role-mux");
+ /* Doing this here keeps our error handling clean. */
+ mux_remove_table(acpi_int3496_cht_mux_lookup,
+ARRAY_SIZE(acpi_int3496_cht_mux_lookup));
+ if (IS_ERR(data->usb_mux)) {
+ ret = PTR_ERR(data->usb_mux);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "can't get mux: %d\n", ret);
+ return ret;
+ }
+ }
+
data->gpio_usb_id = devm_gpiod_get(dev, "id", GPIOD_IN);
if (IS_ERR(data->gpio_usb_id)) {
ret = PTR_ERR(data->gpio_usb_id);
--
2.14.1
--
To unsubscribe from this list: send the li
[PATCH v3 09/14] mux: Add Pericom PI3USB30532 Type-C mux driver
Add a driver for the Pericom PI3USB30532 Type-C cross switch /
mux chip found on some devices with a Type-C port.
Signed-off-by: Hans de Goede
---
Changes in v2:
-Adjust for new MUX_TYPEC_foo state defines
-Add MAINTAINERS entry
-Various code-style fixes
Changes in v3:
-Use new init_as_is mux flag
-Adjust for updated MUX_TYPEC_* defines
---
MAINTAINERS | 5 +++
drivers/mux/Kconfig | 10 ++
drivers/mux/Makefile | 2 ++
drivers/mux/pi3usb30532.c | 92 +++
4 files changed, 109 insertions(+)
create mode 100644 drivers/mux/pi3usb30532.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6cfa0e5f2d2b..5c844fd3a23c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9181,6 +9181,11 @@ M: Hans de Goede
S: Maintained
F: drivers/mux/intel-cht-usb-mux.c
+MULTIPLEXER SUBSYSTEM PI3USB30532 DRIVER
+M: Hans de Goede
+S: Maintained
+F: drivers/mux/pi3usb30532.c
+
MULTISOUND SOUND DRIVER
M: Andrew Veliath
S: Maintained
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index ebc04ae7ff30..f94660229d20 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -58,4 +58,14 @@ config MUX_MMIO
To compile the driver as a module, choose M here: the module will
be called mux-mmio.
+config MUX_PI3USB30532
+ tristate "Pericom PI3USB30532 Type-C cross switch driver"
+ depends on I2C
+ help
+ This driver adds support for the Pericom PI3USB30532 Type-C cross
+ switch / mux chip found on some devices with a Type-C port.
+
+ To compile the driver as a module, choose M here: the module will
+ be called mux-pi3usb30532.
+
endmenu
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index 6cf41be2754f..df381c219708 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -7,9 +7,11 @@ mux-adg792a-objs := adg792a.o
mux-gpio-objs := gpio.o
mux-intel-cht-usb-mux-objs := intel-cht-usb-mux.o
mux-mmio-objs := mmio.o
+mux-pi3usb30532-objs := pi3usb30532.o
obj-$(CONFIG_MULTIPLEXER) += mux-core.o
obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
obj-$(CONFIG_MUX_INTEL_CHT_USB_MUX)+= mux-intel-cht-usb-mux.o
obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
+obj-$(CONFIG_MUX_PI3USB30532) += mux-pi3usb30532.o
diff --git a/drivers/mux/pi3usb30532.c b/drivers/mux/pi3usb30532.c
new file mode 100644
index ..13c7c4ec0047
--- /dev/null
+++ b/drivers/mux/pi3usb30532.c
@@ -0,0 +1,92 @@
+/*
+ * Pericom PI3USB30532 Type-C cross switch / mux driver
+ *
+ * Copyright (c) 2017 Hans de Goede
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation, or (at your option)
+ * any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define PI3USB30532_CONF 0x00
+
+#define PI3USB30532_CONF_OPEN 0x00
+#define PI3USB30532_CONF_SWAP 0x01
+#define PI3USB30532_CONF_4LANE_DP 0x02
+#define PI3USB30532_CONF_USB3 0x04
+#define PI3USB30532_CONF_USB3_AND_2LANE_DP 0x06
+
+static int pi3usb30532_set_mux(struct mux_control *mux, int state)
+{
+ struct i2c_client *i2c = to_i2c_client(mux->chip->dev.parent);
+ u8 conf = PI3USB30532_CONF_OPEN;
+
+ if (state == MUX_IDLE_DISCONNECT)
+ return i2c_smbus_write_byte_data(i2c, PI3USB30532_CONF, conf);
+
+ switch (state & ~MUX_TYPEC_POLARITY_INV) {
+ case MUX_TYPEC_USB:
+ conf = PI3USB30532_CONF_USB3;
+ break;
+ case MUX_TYPEC_USB_AND_DP:
+ conf = PI3USB30532_CONF_USB3_AND_2LANE_DP;
+ break;
+ case MUX_TYPEC_DP:
+ conf = PI3USB30532_CONF_4LANE_DP;
+ break;
+ }
+
+ if (state & MUX_TYPEC_POLARITY_INV)
+ conf |= PI3USB30532_CONF_SWAP;
+
+ return i2c_smbus_write_byte_data(i2c, PI3USB30532_CONF, conf);
+}
+
+static const struct mux_control_ops pi3usb30532_ops = {
+ .set = pi3usb30532_set_mux,
+};
+
+static int pi3usb30532_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct mux_chip *mux_chip;
+
+ mux_chip = devm_mux_chip_alloc(dev, 1, 0);
+ if (IS_ERR(mux_chip))
+ return PTR_ERR(mux_chip);
+
+ mux_chip->ops = &pi3usb30532_ops;
+ mux_chip->mux[0].idle_state = MUX_IDLE_DISCONNECT;
+ /* Keep initial state as is, for e.g. booting from an USB disk */
+ mux_chip->mux[0].init_as_is = true;
+ mux_chip->mux[0].states = MUX_TYPEC_STATES;
+
+ return devm_mux_chip_register(dev, mux_chip);
+}
+
+static const struct i2c_device_id pi3usb30532_table[] = {
+ { "pi
[PATCH v3 08/14] mux: Add Intel Cherrytrail USB mux driver
Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port USB data lines between the xHCI host controller and the dwc3 gadget controller. On some Cherrytrail systems this mux is controlled through AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through an _AIE ACPI method) so things just work. But on other Cherrytrail systems we need to control the mux ourselves this driver exports the mux through the mux subsys, so that other drivers can control it if necessary. This driver also updates the vbus-valid reporting to the dwc3 gadget controller, as this uses the same registers as the mux. This is needed for gadget/device mode to work properly (even on systems which control the mux from their AML code). Note this depends on the xhci driver registering a platform device named "intel_cht_usb_mux", which has an IOMEM resource 0 which points to the Intel Vendor Defined XHCI extended capabilities region. Signed-off-by: Hans de Goede --- Changes in v2: -Complete rewrite as a stand-alone platform-driver rather then as a phy driver, since this is just a mux, not a phy Changes in v3: -Make this a mux subsys driver instead of listening to USB_HOST extcon cable events and responding to those Changes in v4 (of patch, v2 of new mux based series): -Rename C-file to use - in name -Add MAINTAINERS entry -Various code-style fixes Changes in v3 of new mux based series: -Various code-style fixes -Use new init_as_is mux flag --- MAINTAINERS | 5 + drivers/mux/Kconfig | 11 ++ drivers/mux/Makefile| 10 +- drivers/mux/intel-cht-usb-mux.c | 251 4 files changed, 273 insertions(+), 4 deletions(-) create mode 100644 drivers/mux/intel-cht-usb-mux.c diff --git a/MAINTAINERS b/MAINTAINERS index 04e7a5b9d6bd..6cfa0e5f2d2b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9176,6 +9176,11 @@ F: include/linux/dt-bindings/mux/ F: include/linux/mux/ F: drivers/mux/ +MULTIPLEXER SUBSYSTEM INTEL CHT USB MUX DRIVER +M: Hans de Goede +S: Maintained +F: drivers/mux/intel-cht-usb-mux.c + MULTISOUND SOUND DRIVER M: Andrew Veliath S: Maintained diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index 19e4e904c9bf..ebc04ae7ff30 100644 --- a/drivers/mux/Kconfig +++ b/drivers/mux/Kconfig @@ -34,6 +34,17 @@ config MUX_GPIO To compile the driver as a module, choose M here: the module will be called mux-gpio. +config MUX_INTEL_CHT_USB_MUX + tristate "Intel Cherrytrail USB Multiplexer" + depends on ACPI && X86 && EXTCON + help + This driver adds support for the internal USB mux for muxing the OTG + USB data lines between the xHCI host controller and the dwc3 gadget + controller found on Intel Cherrytrail SoCs. + + To compile the driver as a module, choose M here: the module will + be called mux-intel-cht-usb-mux. + config MUX_MMIO tristate "MMIO register bitfield-controlled Multiplexer" depends on (OF && MFD_SYSCON) || COMPILE_TEST diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile index 0e1e59760e3f..6cf41be2754f 100644 --- a/drivers/mux/Makefile +++ b/drivers/mux/Makefile @@ -5,9 +5,11 @@ mux-core-objs := core.o mux-adg792a-objs := adg792a.o mux-gpio-objs := gpio.o +mux-intel-cht-usb-mux-objs := intel-cht-usb-mux.o mux-mmio-objs := mmio.o -obj-$(CONFIG_MULTIPLEXER) += mux-core.o -obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o -obj-$(CONFIG_MUX_GPIO) += mux-gpio.o -obj-$(CONFIG_MUX_MMIO) += mux-mmio.o +obj-$(CONFIG_MULTIPLEXER) += mux-core.o +obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o +obj-$(CONFIG_MUX_GPIO) += mux-gpio.o +obj-$(CONFIG_MUX_INTEL_CHT_USB_MUX)+= mux-intel-cht-usb-mux.o +obj-$(CONFIG_MUX_MMIO) += mux-mmio.o diff --git a/drivers/mux/intel-cht-usb-mux.c b/drivers/mux/intel-cht-usb-mux.c new file mode 100644 index ..726facefd479 --- /dev/null +++ b/drivers/mux/intel-cht-usb-mux.c @@ -0,0 +1,251 @@ +/* + * Intel Cherrytrail USB OTG MUX driver + * + * Copyright (c) 2016-2017 Hans de Goede + * + * Loosely based on android x86 kernel code which is: + * + * Copyright (C) 2014 Intel Corp. + * + * Author: Wu, Hao + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation, or (at your option) + * any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* register definition */ +#define DUAL_ROLE_CFG0 0x68 +#define SW_VBUS_VALID (1 << 24) +#define SW_IDPIN_EN(1 << 21) +#define SW_IDPIN (1 << 20) + +#define DUAL_ROL
[PATCH v3 07/14] xhci: Add Intel cherrytrail extended cap / otg phy mux handling
The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.
Note this commit adds a call to the new xhci_ext_cap_init() function
to xhci_pci_probe(), it is added here because xhci_ext_cap_init() must
be called only once. If in the future we also want to handle ext-caps
on non pci XHCI HCDs from xhci_ext_cap_init() a call to it should also
be added to other bus probe paths.
Signed-off-by: Hans de Goede
---
Changes in v2:
-Check xHCI controller PCI device-id instead of only checking for the
Intel Extended capability ID, as the Extended capability ID is used on
other model Intel xHCI controllers too
Changes in v3:
-Add a new generic xhci_ext_cap_init() function and handle the new
XHCI_INTEL_CHT_USB_MUX quirk there.
---
drivers/usb/host/Makefile| 2 +-
drivers/usb/host/xhci-ext-caps.c | 88
drivers/usb/host/xhci-ext-caps.h | 2 +
drivers/usb/host/xhci-pci.c | 5 +++
drivers/usb/host/xhci.h | 2 +
5 files changed, 98 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/host/xhci-ext-caps.c
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..59329a9cf392 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -10,7 +10,7 @@ fhci-y += fhci-mem.o fhci-tds.o fhci-sched.o
fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
-xhci-hcd-y := xhci.o xhci-mem.o
+xhci-hcd-y := xhci.o xhci-mem.o xhci-ext-caps.o
xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
xhci-hcd-y += xhci-trace.o
ifneq ($(CONFIG_USB_XHCI_MTK), )
diff --git a/drivers/usb/host/xhci-ext-caps.c b/drivers/usb/host/xhci-ext-caps.c
new file mode 100644
index ..8bc4787afdd0
--- /dev/null
+++ b/drivers/usb/host/xhci-ext-caps.c
@@ -0,0 +1,88 @@
+/*
+ * XHCI extended capability handling
+ *
+ * Copyright (c) 2017 Hans de Goede
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include "xhci.h"
+
+static void xhci_intel_unregister_pdev(void *arg)
+{
+ platform_device_unregister(arg);
+}
+
+static int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci, u32
cap_offset)
+{
+ struct usb_hcd *hcd = xhci_to_hcd(xhci);
+ struct device *dev = hcd->self.controller;
+ struct platform_device *pdev;
+ struct resource res = { 0, };
+ int ret;
+
+ pdev = platform_device_alloc("intel_cht_usb_mux", PLATFORM_DEVID_NONE);
+ if (!pdev) {
+ xhci_err(xhci, "couldn't allocate intel_cht_usb_mux pdev\n");
+ return -ENOMEM;
+ }
+
+ res.start = hcd->rsrc_start + cap_offset;
+ res.end = res.start + 0x3ff;
+ res.name = "intel_cht_usb_mux";
+ res.flags = IORESOURCE_MEM;
+
+ ret = platform_device_add_resources(pdev, &res, 1);
+ if (ret) {
+ dev_err(dev, "couldn't add resources to intel_cht_usb_mux
pdev\n");
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ pdev->dev.parent = dev;
+
+ ret = platform_device_add(pdev);
+ if (ret) {
+ dev_err(dev, "couldn't register intel_cht_usb_mux pdev\n");
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
+ if (ret) {
+ dev_err(dev, "couldn't add unregister action for
intel_cht_usb_mux pdev\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+int xhci_ext_cap_init(struct xhci_hcd *xhci)
+{
+ void __iomem *base = &xhci->cap_regs->hc_capbase;
+ u32 cap_offset, val;
+ int ret;
+
+ cap_offset = xhci_find_next_ext_cap(base, 0, 0);
+
+ while (cap_offset) {
+ val = readl(base + cap_offset);
+
+ switch (XHCI_EXT_CAPS_ID(val)) {
+ case XHCI_EXT_CAPS_VENDOR_INTEL:
+ if (xhci->quirks & XHCI_INTEL_CHT_USB_MUX) {
+ ret = xhci_create_intel_cht_mux_pdev(
+ xhci, cap_offset);
+ if (ret)
+ return ret;
+ }
+ break;
+ }
+ cap_offset = xhci_find_next_ext_cap(base, cap_offset, 0);
+ }
+
+ return 0;
+}
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/
[PATCH v3 05/14] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants
Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by USB device/host, resp. Type-C polarity/role/altmode mux drivers and consumers to ensure that they agree on the meaning of the mux_control_select() state argument. Signed-off-by: Hans de Goede --- Changes in v2: -Start numbering of defines at 0 not 1 -Use a new usb.h header, rather then adding these to consumer.h -Add separate MUX_USB_* and MUX_TYPEC_* defines Changes in v3: -Simplify MUX_TYPEC_* states, drop having separate USB HOST / DEVICE states --- include/linux/mux/usb.h | 31 +++ 1 file changed, 31 insertions(+) create mode 100644 include/linux/mux/usb.h diff --git a/include/linux/mux/usb.h b/include/linux/mux/usb.h new file mode 100644 index ..2fec06846e14 --- /dev/null +++ b/include/linux/mux/usb.h @@ -0,0 +1,31 @@ +/* + * mux/usb.h - definitions for USB multiplexers + * + * Copyright (C) 2017 Hans de Goede + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#ifndef _LINUX_MUX_USB_H +#define _LINUX_MUX_USB_H + +/* Mux state values for USB device/host role muxes */ +#define MUX_USB_DEVICE (0) /* USB device mode */ +#define MUX_USB_HOST (1) /* USB host mode */ +#define MUX_USB_STATES (2) + +/* + * Mux state values for Type-C polarity/role/altmode muxes. + * + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as + * inverted-polarity (Type-C plugged in upside down) can happen with any + * other mux-state. + */ +#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ +#define MUX_TYPEC_USB (0 << 1) /* USB only mode */ +#define MUX_TYPEC_USB_AND_DP (1 << 1) /* USB host + 2 lanes DP */ +#define MUX_TYPEC_DP (2 << 1) /* 4 lanes Display Port */ +#define MUX_TYPEC_STATES (3 << 1) + +#endif /* _LINUX_MUX_TYPEC_H */ -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 04/14] mux: core: Add support for getting a mux controller on a non DT platform
On non DT platforms we cannot get the mux_chip by pnode. Other subsystems
(regulator, clock, pwm) have the same problem and solve this by allowing
platform / board-setup code to add entries to a lookup table and then use
this table to look things up.
This commit adds support for getting a mux controller on a non DT platform
following this pattern. It is based on a simplified version of the pwm
subsys lookup code, the dev_id and mux_name parts of a lookup table entry
are mandatory in the mux-core implementation.
Signed-off-by: Hans de Goede
---
Changes in v2:
-Fixup some kerneldoc comments, add kerneldoc comment to structs
-Minor code-style tweaks
---
drivers/mux/core.c | 93 +++-
include/linux/mux/consumer.h | 19 +
2 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 8c0a4c83cdc5..c9afc79196f0 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -24,6 +24,9 @@
#include
#include
+static DEFINE_MUTEX(mux_lookup_lock);
+static LIST_HEAD(mux_lookup_list);
+
/*
* The idle-as-is "state" is not an actual state that may be selected, it
* only implies that the state should not be changed. So, use that state
@@ -423,6 +426,23 @@ int mux_control_deselect(struct mux_control *mux)
}
EXPORT_SYMBOL_GPL(mux_control_deselect);
+static int parent_name_match(struct device *dev, const void *data)
+{
+ const char *parent_name = dev_name(dev->parent);
+ const char *name = data;
+
+ return strcmp(parent_name, name) == 0;
+}
+
+static struct mux_chip *mux_chip_get_by_name(const char *name)
+{
+ struct device *dev;
+
+ dev = class_find_device(&mux_class, NULL, name, parent_name_match);
+
+ return dev ? to_mux_chip(dev) : NULL;
+}
+
static int of_dev_node_match(struct device *dev, const void *data)
{
return dev->of_node == data;
@@ -503,12 +523,83 @@ of_mux_control_get(struct device *dev, const char
*mux_name, bool optional)
static struct mux_control *
__mux_control_get(struct device *dev, const char *mux_name, bool optional)
{
+ struct mux_lookup *m, *chosen = NULL;
+ const char *dev_id = dev_name(dev);
+ struct mux_chip *mux_chip;
+
/* look up via DT first */
if (IS_ENABLED(CONFIG_OF) && dev->of_node)
return of_mux_control_get(dev, mux_name, optional);
- return optional ? NULL : ERR_PTR(-ENODEV);
+ /*
+* For non DT we look up the provider in the static table typically
+* provided by board setup code.
+*
+* If a match is found, the provider mux chip is looked up by name
+* and a mux-control is requested using the table provided index.
+*/
+ mutex_lock(&mux_lookup_lock);
+ list_for_each_entry(m, &mux_lookup_list, list) {
+ if (WARN_ON(!m->dev_id || !m->mux_name || !m->provider))
+ continue;
+
+ if (!strcmp(m->dev_id, dev_id) &&
+ !strcmp(m->mux_name, mux_name))
+ {
+ chosen = m;
+ break;
+ }
+ }
+ mutex_unlock(&mux_lookup_lock);
+
+ if (!chosen)
+ return optional ? NULL : ERR_PTR(-ENODEV);
+
+ mux_chip = mux_chip_get_by_name(chosen->provider);
+ if (!mux_chip)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ if (chosen->index >= mux_chip->controllers) {
+ dev_err(dev, "Mux lookup table index out of bounds %u >= %u\n",
+ chosen->index, mux_chip->controllers);
+ put_device(&mux_chip->dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return &mux_chip->mux[chosen->index];
+}
+
+/**
+ * mux_add_table() - Register consumer to mux-controller mappings
+ * @table: array of mappings to register
+ * @num: number of mappings in table
+ */
+void mux_add_table(struct mux_lookup *table, size_t num)
+{
+ mutex_lock(&mux_lookup_lock);
+
+ for (; num--; table++)
+ list_add_tail(&table->list, &mux_lookup_list);
+
+ mutex_unlock(&mux_lookup_lock);
+}
+EXPORT_SYMBOL_GPL(mux_add_table);
+
+/**
+ * mux_remove_table() - Unregister consumer to mux-controller mappings
+ * @table: array of mappings to unregister
+ * @num: number of mappings in table
+ */
+void mux_remove_table(struct mux_lookup *table, size_t num)
+{
+ mutex_lock(&mux_lookup_lock);
+
+ for (; num--; table++)
+ list_del(&table->list);
+
+ mutex_unlock(&mux_lookup_lock);
}
+EXPORT_SYMBOL_GPL(mux_remove_table);
/**
* mux_control_get() - Get the mux-control for a device.
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index fc98547bf494..eaa6d239c4f5 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -18,6 +18,25 @@
struct device;
struct mux_control;
+/**
+ * struct mux_lookup - Mux consumer to mux-control
[PATCH v3 03/14] mux: core: Add of_mux_control_get helper function
Currently the mux_control_get implementation only deals with getting
mux controllers on DT platforms. This commit renames the current
implementation to of_mux_control_get to reflect this and makes
mux_control_get a wrapper around of_mux_control_get.
This is a preparation patch for adding support for getting muxes on
non DT platforms.
Signed-off-by: Hans de Goede
---
drivers/mux/core.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index d0ad56abca2a..8c0a4c83cdc5 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -439,7 +439,7 @@ static struct mux_chip *of_find_mux_chip_by_node(struct
device_node *np)
}
static struct mux_control *
-__mux_control_get(struct device *dev, const char *mux_name, bool optional)
+of_mux_control_get(struct device *dev, const char *mux_name, bool optional)
{
struct device_node *np = dev->of_node;
struct of_phandle_args args;
@@ -500,6 +500,16 @@ __mux_control_get(struct device *dev, const char
*mux_name, bool optional)
return &mux_chip->mux[controller];
}
+static struct mux_control *
+__mux_control_get(struct device *dev, const char *mux_name, bool optional)
+{
+ /* look up via DT first */
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ return of_mux_control_get(dev, mux_name, optional);
+
+ return optional ? NULL : ERR_PTR(-ENODEV);
+}
+
/**
* mux_control_get() - Get the mux-control for a device.
* @dev: The device that needs a mux-control.
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/14] mux: core: Add mux_control_get_optional() API
From: Stephen Boyd
Sometimes drivers only use muxes under certain scenarios. For
example, the chipidea usb controller may be connected to a usb
switch on some platforms, and connected directly to a usb port on
others. The driver won't know one way or the other though, so add
a mux_control_get_optional() API that allows the driver to
differentiate errors getting the mux from there not being a mux
for the driver to use at all.
Signed-off-by: Stephen Boyd
Signed-off-by: Hans de Goede
---
Documentation/driver-model/devres.txt | 1 +
drivers/mux/core.c| 104 +++---
include/linux/mux/consumer.h | 4 ++
3 files changed, 89 insertions(+), 20 deletions(-)
diff --git a/Documentation/driver-model/devres.txt
b/Documentation/driver-model/devres.txt
index 69f08c0f23a8..5e41f3ac8a05 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -343,6 +343,7 @@ MUX
devm_mux_chip_alloc()
devm_mux_chip_register()
devm_mux_control_get()
+ devm_mux_control_get_optional()
PER-CPU MEM
devm_alloc_percpu()
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 6e5cf9d9cd99..244bceb17877 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -289,6 +289,9 @@ EXPORT_SYMBOL_GPL(devm_mux_chip_register);
*/
unsigned int mux_control_states(struct mux_control *mux)
{
+ if (!mux)
+ return 0;
+
return mux->states;
}
EXPORT_SYMBOL_GPL(mux_control_states);
@@ -338,6 +341,9 @@ int mux_control_select(struct mux_control *mux, unsigned
int state)
{
int ret;
+ if (!mux)
+ return 0;
+
ret = down_killable(&mux->lock);
if (ret < 0)
return ret;
@@ -370,6 +376,9 @@ int mux_control_try_select(struct mux_control *mux,
unsigned int state)
{
int ret;
+ if (!mux)
+ return 0;
+
if (down_trylock(&mux->lock))
return -EBUSY;
@@ -398,6 +407,9 @@ int mux_control_deselect(struct mux_control *mux)
{
int ret = 0;
+ if (!mux)
+ return 0;
+
if (mux->idle_state != MUX_IDLE_AS_IS &&
mux->idle_state != mux->cached_state)
ret = mux_control_set(mux, mux->idle_state);
@@ -423,14 +435,8 @@ static struct mux_chip *of_find_mux_chip_by_node(struct
device_node *np)
return dev ? to_mux_chip(dev) : NULL;
}
-/**
- * mux_control_get() - Get the mux-control for a device.
- * @dev: The device that needs a mux-control.
- * @mux_name: The name identifying the mux-control.
- *
- * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
- */
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+static struct mux_control *
+__mux_control_get(struct device *dev, const char *mux_name, bool optional)
{
struct device_node *np = dev->of_node;
struct of_phandle_args args;
@@ -442,16 +448,22 @@ struct mux_control *mux_control_get(struct device *dev,
const char *mux_name)
if (mux_name) {
index = of_property_match_string(np, "mux-control-names",
mux_name);
+ if ((index == -EINVAL || index == -ENODATA) && optional)
+ return NULL;
if (index < 0) {
dev_err(dev, "mux controller '%s' not found\n",
mux_name);
return ERR_PTR(index);
}
+ /* OF does point to a mux, so it's no longer optional */
+ optional = false;
}
ret = of_parse_phandle_with_args(np,
"mux-controls", "#mux-control-cells",
index, &args);
+ if (ret == -ENOENT && optional)
+ return NULL;
if (ret) {
dev_err(dev, "%pOF: failed to get mux-control %s(%i)\n",
np, mux_name ?: "", index);
@@ -484,8 +496,35 @@ struct mux_control *mux_control_get(struct device *dev,
const char *mux_name)
return &mux_chip->mux[controller];
}
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{
+ return __mux_control_get(dev, mux_name, false);
+}
EXPORT_SYMBOL_GPL(mux_control_get);
+/**
+ * mux_control_get_optional() - Get the optional mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: NULL if no mux with the provided name is found, a pointer to
+ * the named mux-control or an ERR_PTR with a negative errno.
+ */
+struct mux_control *
+mux_control_get_op
[PATCH v3 02/14] mux: core: Add explicit hook to leave the mux as-is on init/registration
From: Peter Rosin
A board may need a mux controller to stay as-is for a while longer, e.g.
if setting the normally preferred idle state destroys booting.
The mechanism provided here is not perfect in two ways.
1. As soon as the mux controller is registered, some mux consumer can
access it and set a state that destroys booting all the same.
2. The mux controller might linger in a state that is not the
preferred idle state indefinitely, if no mux consumer ever selects
and then deselects the mux.
Signed-off-by: Hans de Goede
---
drivers/mux/core.c | 3 +++
include/linux/mux/driver.h | 4
2 files changed, 7 insertions(+)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 244bceb17877..d0ad56abca2a 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -155,6 +155,9 @@ int mux_chip_register(struct mux_chip *mux_chip)
for (i = 0; i < mux_chip->controllers; ++i) {
struct mux_control *mux = &mux_chip->mux[i];
+ if (mux->init_as_is)
+ continue;
+
if (mux->idle_state == mux->cached_state)
continue;
diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
index 35c3579c3304..21cf6041a962 100644
--- a/include/linux/mux/driver.h
+++ b/include/linux/mux/driver.h
@@ -36,6 +36,9 @@ struct mux_control_ops {
* @states:The number of mux controller states.
* @idle_state:The mux controller state to use when inactive,
or one
* of MUX_IDLE_AS_IS and MUX_IDLE_DISCONNECT.
+ * @init_as_is:Set to true to have the core leave the mux
controller
+ * state as-is until first selection. If @idle_state is
+ * MUX_IDLE_AS_IS, @init_as_is is irrelevant.
*
* Mux drivers may only change @states and @idle_state, and may only do so
* between allocation and registration of the mux controller. Specifically,
@@ -50,6 +53,7 @@ struct mux_control {
unsigned int states;
int idle_state;
+ bool init_as_is;
};
/**
--
2.14.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/14] xhci: Add option to get next extended capability in list by passing id = 0
From: Mathias Nyman Modify xhci_find_next_ext_cap(base, offset, id) to return the next capability offset if 0 is passed for id. Otherwise it will behave as previously and return the offset of the next capability with matching id capability id 0 is not used by xhci (reserved) This is useful when we want to loop through all capabilities. Signed-off-by: Mathias Nyman Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-ext-caps.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h index 28deea584884..c1b404224839 100644 --- a/drivers/usb/host/xhci-ext-caps.h +++ b/drivers/usb/host/xhci-ext-caps.h @@ -96,7 +96,8 @@ * @base PCI MMIO registers base address. * @start address at which to start looking, (0 or HCC_PARAMS to start at * beginning of list) - * @id Extended capability ID to search for. + * @id Extended capability ID to search for, or 0 for the next + * capability * * Returns the offset of the next matching extended capability structure. * Some capabilities can occur several times, e.g., the XHCI_EXT_CAPS_PROTOCOL, @@ -122,7 +123,7 @@ static inline int xhci_find_next_ext_cap(void __iomem *base, u32 start, int id) val = readl(base + offset); if (val == ~0) return 0; - if (XHCI_EXT_CAPS_ID(val) == id && offset != start) + if (offset != start && (id == 0 || XHCI_EXT_CAPS_ID(val) == id)) return offset; next = XHCI_EXT_CAPS_NEXT(val); -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] r8152: add Linksys USB3GIGV1 id
This Linksys dongle by default comes up in cdc_ether mode.
This patch allows r8152 to claim the device:
Bus 002 Device 002: ID 13b1:0041 Linksys
Signed-off-by: Grant Grundler
---
drivers/net/usb/r8152.c | 2 ++
1 file changed, 2 insertions(+)
This was tested on chromeos-3.14, chromeos-3.18, and chromeos-4.4 kernels
with a mix of ARM/x86-64 systems.
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ceb78e2ea4f0..941ece08ba78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -613,6 +613,7 @@ enum rtl8152_flags {
#define VENDOR_ID_MICROSOFT0x045e
#define VENDOR_ID_SAMSUNG 0x04e8
#define VENDOR_ID_LENOVO 0x17ef
+#define VENDOR_ID_LINKSYS 0x13b1
#define VENDOR_ID_NVIDIA 0x0955
#define MCU_TYPE_PLA 0x0100
@@ -5316,6 +5317,7 @@ static const struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214)},
+ {REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA, 0x09ff)},
{}
};
--
2.14.1.821.g8fa685d3b7-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue
inc_deq() currently bails earlier for EVENT rings than the common return
point of the function, due to the fact that EVENT rings do not have
link TRBs. The unfortunate side effect of this is that the very useful
trace_xhci_inc_deq() function is not called/usable for EVENT ring
debug.
This patch provides a refactor by removing the multiple return exit
points into a single return which additionally allows for all rings to
use the trace function.
Signed-off-by: Adam Wallis
---
Changes in v2: undo accidental line removal at end of patch
drivers/usb/host/xhci-ring.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a944365..3960ba9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -171,23 +171,23 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (ring->type == TYPE_EVENT) {
if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
ring->dequeue++;
- return;
+ } else {
+ if (last_trb_on_ring(ring, ring->deq_seg,
+ ring->dequeue))
+ ring->cycle_state ^= 1;
+ ring->deq_seg = ring->deq_seg->next;
+ ring->dequeue = ring->deq_seg->trbs;
+ }
+ } else {
+ /* All other rings have link trbs */
+ if (!trb_is_link(ring->dequeue)) {
+ ring->dequeue++;
+ ring->num_trbs_free++;
+ }
+ while (trb_is_link(ring->dequeue)) {
+ ring->deq_seg = ring->deq_seg->next;
+ ring->dequeue = ring->deq_seg->trbs;
}
- if (last_trb_on_ring(ring, ring->deq_seg, ring->dequeue))
- ring->cycle_state ^= 1;
- ring->deq_seg = ring->deq_seg->next;
- ring->dequeue = ring->deq_seg->trbs;
- return;
- }
-
- /* All other rings have link trbs */
- if (!trb_is_link(ring->dequeue)) {
- ring->dequeue++;
- ring->num_trbs_free++;
- }
- while (trb_is_link(ring->dequeue)) {
- ring->deq_seg = ring->deq_seg->next;
- ring->dequeue = ring->deq_seg->trbs;
}
trace_xhci_inc_deq(ring);
--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: cdc-wdm: ignore -EPIPE from GetEncapsulatedResponse
The driver will forward errors to userspace after turning most of them
into -EIO. But all status codes are not equal. The -EPIPE (stall) in
particular can be seen more as a result of normal USB signaling than
an actual error. The state is automatically cleared by the USB core
without intervention from either driver or userspace.
And most devices and firmwares will never trigger a stall as a result
of GetEncapsulatedResponse. This is in fact a requirement for CDC WDM
devices. Quoting from section 7.1 of the CDC WMC spec revision 1.1:
The function shall not return STALL in response to
GetEncapsulatedResponse.
But this driver is also handling GetEncapsulatedResponse on behalf of
the qmi_wwan and cdc_mbim drivers. Unfortunately the relevant specs
are not as clear wrt stall. So some QMI and MBIM devices *will*
occasionally stall, causing the GetEncapsulatedResponse to return an
-EPIPE status. Translating this into -EIO for userspace has proven to
be harmful. Treating it as an empty read is safer, making the driver
behave as if the device was conforming to the CDC WDM spec.
There have been numerous reports of issues related to -EPIPE errors
from some newer CDC MBIM devices in particular, like for example the
Fibocom L831-EAU. Testing on this device has shown that the issues
go away if we simply ignore the -EPIPE status. Similar handling of
-EPIPE is already known from e.g. usb_get_string()
The -EPIPE log message is still kept to let us track devices with this
unexpected behaviour, hoping that it attracts attention from firmware
developers.
Cc:
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100938
Reported-and-tested-by: Christian Ehrig
Reported-and-tested-by: Patrick Chilton
Reported-and-tested-by: Andreas Böhler
Signed-off-by: Bjørn Mork
---
The fallout of this problem has been reported by a number of people
for a long time. Many more than those being listed above. Apologies
to all who didn't get the appropriate credit. Sorry, I am lousy with
paper work and lost track of you all.
Hoping the fix will make up for it...
Bjørn
drivers/usb/class/cdc-wdm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 0b845e550fbd..9f001659807a 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -194,8 +194,10 @@ static void wdm_in_callback(struct urb *urb)
/*
* only set a new error if there is no previous error.
* Errors are only cleared during read/open
+* Avoid propagating -EPIPE (stall) to userspace since it is
+* better handled as an empty read
*/
- if (desc->rerr == 0)
+ if (desc->rerr == 0 && status != -EPIPE)
desc->rerr = status;
if (length + desc->length > desc->wMaxCommand) {
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] USB: devio: Don't corrupt user memory
The user buffer has "uurb->buffer_length" bytes. If the kernel has more information than that, we should truncate it instead of writing past the end of the user's buffer. I added a WARN_ONCE() to help the user debug the issue. Reported-by: Alan Stern Signed-off-by: Dan Carpenter diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 318bb3b96687..4664e543cf2f 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1571,7 +1576,11 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb totlen += isopkt[u].length; } u *= sizeof(struct usb_iso_packet_descriptor); - uurb->buffer_length = totlen; + if (totlen <= uurb->buffer_length) + uurb->buffer_length = totlen; + else + WARN_ONCE(1, "uurb->buffer_length is too short %d vs %d", + totlen, uurb->buffer_length); break; default: -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] USB: devio: Prevent integer overflow in proc_do_submiturb()
There used to be an integer overflow check in proc_do_submiturb() but
we removed it. It turns out that it's still required. The
uurb->buffer_length variable is a signed integer and it's controlled by
the user. It can lead to an integer overflow when we do:
num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);
If we strip away the macro then that line looks like this:
num_sgs = (uurb->buffer_length + USB_SG_SIZE - 1) / USB_SG_SIZE;
^
It's the first addition which can overflow.
Fixes: 1129d270cbfb ("USB: Increase usbfs transfer limit")
Signed-off-by: Dan Carpenter
---
v2: Cast the ->buffer_length to unsigned int which is more readable than
relying on type promotion.
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 318bb3b96687..4664e543cf2f 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -140,6 +140,9 @@ module_param(usbfs_memory_mb, uint, 0644);
MODULE_PARM_DESC(usbfs_memory_mb,
"maximum MB allowed for usbfs buffers (0 = no limit)");
+/* Hard limit, necessary to avoid arithmetic overflow */
+#define USBFS_XFER_MAX (UINT_MAX / 2 - 100)
+
static atomic64_t usbfs_memory_usage; /* Total memory currently allocated */
/* Check whether it's okay to allocate more memory for a transfer */
@@ -1460,6 +1463,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
struct usbdevfs_urb *uurb
USBDEVFS_URB_ZERO_PACKET |
USBDEVFS_URB_NO_INTERRUPT))
return -EINVAL;
+ if ((unsigned int)uurb->buffer_length >= USBFS_XFER_MAX)
+ return -EINVAL;
if (uurb->buffer_length > 0 && !uurb->buffer)
return -EINVAL;
if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL &&
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] USB: devio: Prevent integer overflow in proc_do_submiturb()
On Fri, 22 Sep 2017, Dan Carpenter wrote:
> There used to be an integer overflow check in proc_do_submiturb() but
> we removed it. It turns out that it's still required. The
> uurb->buffer_length variable is a signed integer and it's controlled by
> the user. It can lead to an integer overflow when we do:
>
> num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);
>
> If we strip away the macro then that line looks like this:
>
> num_sgs = (uurb->buffer_length + USB_SG_SIZE - 1) / USB_SG_SIZE;
>^
> It's the first addition which can overflow.
>
> Fixes: 1129d270cbfb ("USB: Increase usbfs transfer limit")
> Signed-off-by: Dan Carpenter
> ---
> v2: Cast the ->buffer_length to unsigned int which is more readable than
> relying on type promotion.
For both 1/2 and 2/2:
Acked-by: Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
