Re: Support of Mark-10 Force Gauge USB
On Thu, Aug 20, 2015 at 06:06:22PM +0200, Sepp Käsbauer wrote:
> Hi Johan,
>
> if you want to take the following patch then the Mark-10 Force Gauge Series 5
> is supported by the cp210x driver. We have tested only with kernel > 4.1.5
> It works perfect.
>
> Thanks, Sepp Käsbauer
>
> see: http://www.mark-10.com/instruments/force/series5.html
>
> diff -c cp210x.c.orig cp210x.c
> *** cp210x.c.orig 2015-08-20 10:09:31.811953677 +0200
> --- cp210x.c2015-08-20 10:11:26.203796713 +0200
> ***
> *** 111,116
> --- 111,117
> { USB_DEVICE(0x10C4, 0x8341) }, /* Siemens MC35PU GPRS Modem */
> { USB_DEVICE(0x10C4, 0x8382) }, /* Cygnal Integrated Products, Inc. */
> { USB_DEVICE(0x10C4, 0x83A8) }, /* Amber Wireless AMB2560 */
> + { USB_DEVICE(0x10C4, 0x83AA) }, /* Mark-10 Force Gauge M5-05 */
> { USB_DEVICE(0x10C4, 0x83D8) }, /* DekTec DTA Plus VHF/UHF
> Booster/Attenuator */
> { USB_DEVICE(0x10C4, 0x8411) }, /* Kyocera GPS Module */
> { USB_DEVICE(0x10C4, 0x8418) }, /* IRZ Automation Teleport SG-10
> GSM/GPRS Modem */
Thanks for the patch. Care to submit it on a format that can be applied?
Take a look at Documentation/SubmittingPatches and this example of what
such a patch could look like:
https://lkml.kernel.org/r/1439683988-13123-1-git-send-email-david.w...@ll.mit.edu
Try sending it to yourself first and run scripts/checkpatch.pl on the
result.
Thanks,
Johan
--
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 0/7] Exynos4412-based Trats2 USB gadget (DWC2) fixes
On 21.08.2015 21:38, Marek Szyprowski wrote:
> Dear All,
>
> Since v3.19 s3c-hsotg (DWC2) USB controller stopped working on
> Exynos4412-based Trats2 platform. However on Odroid-U3 (which is also
> Exynos4412-based) it worked fine all the time. After long investigation
> it turned out that this was caused by 2 independent issues.
>
> First issue was caused by patch 7eec1266751bd3a25e35ce88686634c768fedc24
> ("ARM: dts: Add Maxim 77693 PMIC to exynos4412-trats2") added support
> for Maxim 77693 regulators, but without defining consumers for them.
> This causes regulator core to disable them. However SAFEOUT1 regulator
> provides power supply to VBUS signal, which also power some USB phy
> related parts of Exynos SoC core. This has been fixed by patches 1-3,
> which adds support for VBUS regulator, defines them in DTS for all
> Exynos platforms and changes probe sequence of the drivers to ensure no
> deferred probe occurs (USB gadget subsystem doesn't support deferred
> probe yet).
>
> Second issue is related to DWC2 driver rewrite and host/gadget/dual-role
> integration code. DWC2 module on some platforms needs three additional
> hardware resources: phy, clock and power supply. All of them must be
> enabled/activated to properly initialize and operate. This was initially
> handled in s3c-hsotg driver, which has been converted to 'gadget' part
> of dwc2 driver. Unfortunately, not all of this code got moved to common
> platform code, what resulted in accessing DWC2 registers without
> enabling power supply regulators and clock. This caused initialization
> failure on Exynos4412-based Trats2. Odroid U3 board worked fine, because
> power supplies used by DWC2 module are marked there as 'always-on',
> because they are also used by other modules (USB hub) and clock was
> shared with USB2 PHY, so it was already enabled. This initialization
> issue has been fixed by the last patch. While preparing it I've also
> fixed a few minor issues found in nearby code (patches 4-6).
>
> I would like to get those patches merged separately to respective
> subsystem trees:
> patch 1: phy subsystem tree
> patch 2: Samsung Exynos tree
> patch 3: regulator subsystem tree
> patch 4-7: USB gadget subsystem tree
>
> Patches have been prepared on top of linux-next from 20150821.
Entire patchset tested on Trats2 board:
Tested-by: Krzysztof Kozlowski
Best regards,
Krzysztof
--
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 v3 1/1] USB:option:add ZTE PIDs
On Wed, Aug 19, 2015 at 08:51:17AM -0700, Liu.Zhao wrote:
>
> This is intended to add ZTE device PIDs on kernel.
>
>
>
> Signed-off-by: Liu.Zhao
> ---
> drivers/usb/serial/option.c | 36
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 876423b..e26db28 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -278,13 +278,17 @@ static void option_instat_callback(struct urb *urb);
> #define ZTE_PRODUCT_MF6220x0001
> #define ZTE_PRODUCT_MF6280x0015
> #define ZTE_PRODUCT_MF6260x0031
> -#define ZTE_PRODUCT_AC2726 0xfff1
> -#define ZTE_PRODUCT_MG8800xfffd
> -#define ZTE_PRODUCT_CDMA_TECH0xfffe
> -#define ZTE_PRODUCT_AC8710T 0x
> +#define ZTE_PRODUCT_ZM8620_X 0x0396
> +#define ZTE_PRODUCT_ME3620_MBIM 0x0426
> +#define ZTE_PRODUCT_ME3620_X 0x1432
> +#define ZTE_PRODUCT_ME3620_L 0x1433
> #define ZTE_PRODUCT_MC2718 0xffe8
> #define ZTE_PRODUCT_AD3812 0xffeb
> #define ZTE_PRODUCT_MC2716 0xffed
> +#define ZTE_PRODUCT_AC2726 0xfff1
> +#define ZTE_PRODUCT_MG8800xfffd
> +#define ZTE_PRODUCT_CDMA_TECH0xfffe
> +#define ZTE_PRODUCT_AC8710T 0x
Reordering the current entries is not needed, but if you do it then you
must mention it in the commit message above.
Also make sure to not change any formatting; checkpatch is now
complaining about whitespace issues (always run checkpatch before
submitting).
> #define BENQ_VENDOR_ID 0x04a5
> #define BENQ_PRODUCT_H10 0x4068
> @@ -544,6 +548,14 @@ static const struct option_blacklist_info
> zte_mc2716_z_blacklist = {
> .sendsetup = BIT(1) | BIT(2) | BIT(3),
> };
>
> +static const struct option_blacklist_info zte_me3620andzm8620_xl_blacklist =
> {
> + .reserved = BIT(3) | BIT(4) | BIT(5),
> +};
Use two structs for this: zte_me3620_blacklist and zm8620_xl_blacklist
even if they reserve the same ports.
> +
> +static const struct option_blacklist_info zte_me3620_mbim_blacklist = {
> + .reserved = BIT(2) | BIT(3) | BIT(4),
> +};
> +
> static const struct option_blacklist_info huawei_cdc12_blacklist = {
> .reserved = BIT(1) | BIT(2),
> };
> @@ -1581,16 +1593,24 @@ static const struct usb_device_id option_ids[] = {
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xfff9, 0xff, 0xff,
> 0xff) },
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xfffb, 0xff, 0xff,
> 0xff) },
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xfffc, 0xff, 0xff,
> 0xff) },
> - { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MG880, 0xff,
> 0xff, 0xff) },
> - { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_CDMA_TECH,
> 0xff, 0xff, 0xff) },
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC2726,
> 0xff, 0xff, 0xff) },
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AC8710T,
> 0xff, 0xff, 0xff) },
> - { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MC2718,
> 0xff, 0xff, 0xff),
> - .driver_info = (kernel_ulong_t)&zte_mc2718_z_blacklist },
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_AD3812,
> 0xff, 0xff, 0xff),
>.driver_info = (kernel_ulong_t)&zte_ad3812_z_blacklist },
> + { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_CDMA_TECH,
> 0xff, 0xff, 0xff) },
> { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MC2716,
> 0xff, 0xff, 0xff),
>.driver_info = (kernel_ulong_t)&zte_mc2716_z_blacklist },
> + { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MC2718,
> 0xff, 0xff, 0xff),
> + .driver_info = (kernel_ulong_t)&zte_mc2718_z_blacklist },
> + { USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MG880, 0xff,
> 0xff, 0xff) },
> + { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_L),
> + .driver_info = (kernel_ulong_t)&zte_me3620andzm8620_xl_blacklist },
> + { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_X),
> + .driver_info = (kernel_ulong_t)&zte_me3620andzm8620_xl_blacklist },
> + { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ZM8620_X),
> + .driver_info = (kernel_ulong_t)&zte_me3620andzm8620_xl_blacklist },
> + { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_MBIM),
> + .driver_info = (kernel_ulong_t)&zte_me3620_mbim_blacklist },
If really want to reorder the current entries, then at least make sure
the end result is indeed sorted.
> { USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x01) },
> { USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x05) },
> { USB_VENDOR_AND_IN
Re: [PATCH v3 1/1] USB:option:add ZTE PIDs
Johan Hovold writes:
> On Wed, Aug 19, 2015 at 08:51:17AM -0700, Liu.Zhao wrote:
>>
>> #define BENQ_VENDOR_ID 0x04a5
>> #define BENQ_PRODUCT_H100x4068
>> @@ -544,6 +548,14 @@ static const struct option_blacklist_info
>> zte_mc2716_z_blacklist = {
>> .sendsetup = BIT(1) | BIT(2) | BIT(3),
>> };
>>
>> +static const struct option_blacklist_info zte_me3620andzm8620_xl_blacklist
>> = {
>> +.reserved = BIT(3) | BIT(4) | BIT(5),
>> +};
>
> Use two structs for this: zte_me3620_blacklist and zm8620_xl_blacklist
> even if they reserve the same ports.
Why? Wouldn't it be better to merge all identical lists and give them
structured names describing their contents instead? E.g.
static const struct option_blacklist_info bi_s0001_r = {
.sendsetup = BIT(0) | BIT(1),
};
static const struct option_blacklist_info bi_s0001_r04 = {
.sendsetup = BIT(0) | BIT(1),
.reserved = BIT(4),
};
static const struct option_blacklist_info bi_s_r030405 = {
.reserved = BIT(3) | BIT(4) | BIT(5),
};
etc. Or some other naming scheme.
I don't see the point of having lots of identical structs just to be
able to name them after some rarely meaningful marketing name. Many
vendors recycle their pids, making this completely futile.
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 1/1] usb: chipidea: udc: using the correct stall implementation
According to spec, there are functional and protocol stalls.
For functional stall, it is for bulk and interrupt endpoints,
below are cases for it:
- Host sends SET_FEATURE request for Set-Halt, the udc driver
needs to set stall, and return true unconditionally.
- The gadget driver may call usb_ep_set_halt to stall certain
endpoints, if there is a transfer in pending, the udc driver
should not set stall, and return -EAGAIN accordingly.
These two kinds of stall need to be cleared by host using CLEAR_FEATURE
request (Clear-Halt).
For protocol stall, it is for control endpoint, this stall will
be set if the control request has failed. This stall will be
cleared by next setup request (hardware will do it).
It fixed usbtest (drivers/usb/misc/usbtest.c) Test 13 "set/clear halt"
test failure, meanwhile, this change has been verified by
USB2 CV Compliance Test and MSC Tests.
Cc: #3.10+
Cc: Alan Stern
Cc: Felipe Balbi
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/udc.c | 84 --
1 file changed, 44 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index a637da2..8223fe7 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -656,6 +656,44 @@ __acquires(hwep->lock)
return 0;
}
+static int _ep_set_halt(struct usb_ep *ep, int value, bool check_transfer)
+{
+ struct ci_hw_ep *hwep = container_of(ep, struct ci_hw_ep, ep);
+ int direction, retval = 0;
+ unsigned long flags;
+
+ if (ep == NULL || hwep->ep.desc == NULL)
+ return -EINVAL;
+
+ if (usb_endpoint_xfer_isoc(hwep->ep.desc))
+ return -EOPNOTSUPP;
+
+ spin_lock_irqsave(hwep->lock, flags);
+
+ if (value && hwep->dir == TX && check_transfer &&
+ !list_empty(&hwep->qh.queue) &&
+ !usb_endpoint_xfer_control(hwep->ep.desc)) {
+ spin_unlock_irqrestore(hwep->lock, flags);
+ return -EAGAIN;
+ }
+
+ direction = hwep->dir;
+ do {
+ retval |= hw_ep_set_halt(hwep->ci, hwep->num, hwep->dir, value);
+
+ if (!value)
+ hwep->wedge = 0;
+
+ if (hwep->type == USB_ENDPOINT_XFER_CONTROL)
+ hwep->dir = (hwep->dir == TX) ? RX : TX;
+
+ } while (hwep->dir != direction);
+
+ spin_unlock_irqrestore(hwep->lock, flags);
+ return retval;
+}
+
+
/**
* _gadget_stop_activity: stops all USB activity, flushes & disables all endpts
* @gadget: gadget
@@ -1051,7 +1089,7 @@ __acquires(ci->lock)
num += ci->hw_ep_max / 2;
spin_unlock(&ci->lock);
- err = usb_ep_set_halt(&ci->ci_hw_ep[num].ep);
+ err = _ep_set_halt(&ci->ci_hw_ep[num].ep, 1, false);
spin_lock(&ci->lock);
if (!err)
isr_setup_status_phase(ci);
@@ -1117,8 +1155,8 @@ delegate:
if (err < 0) {
spin_unlock(&ci->lock);
- if (usb_ep_set_halt(&hwep->ep))
- dev_err(ci->dev, "error: ep_set_halt\n");
+ if (_ep_set_halt(&hwep->ep, 1, false))
+ dev_err(ci->dev, "error: _ep_set_halt\n");
spin_lock(&ci->lock);
}
}
@@ -1149,9 +1187,9 @@ __acquires(ci->lock)
err = isr_setup_status_phase(ci);
if (err < 0) {
spin_unlock(&ci->lock);
- if (usb_ep_set_halt(&hwep->ep))
+ if (_ep_set_halt(&hwep->ep, 1, false))
dev_err(ci->dev,
- "error: ep_set_halt\n");
+ "error: _ep_set_halt\n");
spin_lock(&ci->lock);
}
}
@@ -1397,41 +1435,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
*/
static int ep_set_halt(struct usb_ep *ep, int value)
{
- struct ci_hw_ep *hwep = container_of(ep, struct ci_hw_ep, ep);
- int direction, retval = 0;
- unsigned long flags;
-
- if (ep == NULL || hwep->ep.desc == NULL)
- return -EINVAL;
-
- if (usb_endpoint_xfer_isoc(hwep->ep.desc))
- return -EOPNOTSUPP;
-
- spin_lock_irqsave(hwep->lock, flags);
-
-#ifndef STALL_IN
- /* g_file_storage MS compliant but g_zero fails chapter 9 compliance */
- if (value && hwep->type == USB_ENDPOINT_XFER_BULK && hwep->dir == TX &&
- !list_empty(&hwep->qh.queue)) {
- spin_unlock_irqrestore(hwep->lock, flags);
- return -EAGAIN;
- }
-#endif
-
- direction =
RE: [PATCH] usb: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16
From: Vaishali Thakkar [mailto:vthakkar1...@gmail.com]
> Sent: 22 August 2015 02:57
...
> >> - .bcdADC = __constant_cpu_to_le16(0x0100),
> >> - .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH),
> >> + .bcdADC = cpu_to_le16(0x0100),
> >> + .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH),
> >
> > Have you test compiled this on a big-endian system?
> > My gut feeling is that is fails.
>
> No. I have tested it on little-endian system only. But I'll
> be really surprised if this will fail. Can you please tell me
> if I am missing something in this particular case or same
> applies for other cases because most of the cases like
> __constant_ are already converted to ?
>
> As far as I know, if the argument is a constant the
> conversion happens at compile time. And unfolding both
> definitions returns to same expression. Still I am trying if
> someone can test it for me on big endian system.
Flip one to cpu_to_be16() and see if it still compiles.
Static initialisers and case labels can be expressions, but the
expression itself must only contain constants.
So it needs to be constant regardless of the value of any constants.
If it contains 'a ? t : f' then both 't' and 'f' must be constant.
In code, if 'a' is constant the optimiser discards one of 't' or 'f'.
I'm not sure what happens for non-static initialisers (they generate
odd code at the best of times).
David
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&�� �G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: option driver crashes on modem removal
Hi Johan,
On Mon, Aug 17, 2015 at 5:29 PM, Johan Hovold wrote:
> On Tue, Aug 11, 2015 at 12:31:47PM +0200, Yegor Yefremov wrote:
>> On Tue, Aug 11, 2015 at 11:58 AM, Bjørn Mork wrote:
>> > [replaced 'netdev' with 'linux-usb' as this concerns a USB serial driver
>> > only]
>> >
>> > Yegor Yefremov writes:
>> >
>> >> I have following problem. When removing USB dongle 07d1:3e01 or
>> >> SierraWireless MC7304 I get following messages:
>> >>
>> >> option1 ttyUSB10: option_instat_callback: error -71
>> >> option1 ttyUSB9: option_instat_callback: error -71
>> >> option1 ttyUSB10: option_instat_callback: error -71
>> >> option1 ttyUSB9: option_instat_callback: error -71
>> >> option1 ttyUSB10: option_instat_callback: error -71
>> >> option1 ttyUSB9: option_instat_callback: error -71
>> >> INFO: rcu_sched detected stalls on CPUs/tasks: {} (detected by 0,
>> >> t=2102 jiffies, g=694, c=693, q=24)
>> >> INFO: Stall ended before state dump start
>> >> option1 ttyUSB10: option_instat_callback: error -71
>> >>
>> >> drivers/usb/serial/option.c seems to make nothing with such a status
>> >> and just prints error. How one would handle this properly and just
>> >> unregister device? Do you need more info?
>> >>
>> >> Tested kernels: 3.18.20 and 4.2.0-rc5 (this kernel shows only RCU stall
>> >> crash)
>> >> Hardware: TI am335x
>> >
>> >
>> > Isn't the device unregistered? What else can be done here?
>>
>> The problem is, that the system is dead (stall). It only prints
>> "option1 ttyUSB10: option_instat_callback: error -71" endlessly
>> (kernel 3.18.20) and console shows no reaction for input. And when you
>> start watchdog from userspace the systems reboots after specified
>> timeout (watchdog -t 5 -T 10 /dev/watchdog).
>
> Related issues have been reported with musb (BeagleBone Black) and dwc
> controllers (RPi) when using an external hub.
>
> The exact reasons have not yet been fully determined, including whether
> it is the hub driver that is prevented from processing the event or if
> the disconnect event is never received at all. See for example:
>
> https://lkml.kernel.org/r/20150409171314.GA30002@localhost
>
> In the cases I've seen, changing the timing slightly was enough to make
> the problem go away. Specifically, avoiding a large, unnecessary memcpy
> or silencing an error message was sufficient.
>
> This obviously does not solve the underlying issue, but could you try
> the below patch nonetheless? We don't want these error messages for the
> "valid" use case of removing an in-use device anyway.
>
> Thanks,
> Johan
>
>
> From 81913b6a91bd53b4f00455618c141c35148acbfc Mon Sep 17 00:00:00 2001
> From: Johan Hovold
> Date: Mon, 17 Aug 2015 16:48:47 +0200
> Subject: [PATCH] USB: option: silence interrupt errors
>
> Avoid spamming the logs (e.g. with -EPROTO errors) when attempting to
> resubmit the interrupt urb while a disconnect of an in-use device is
> being processed.
>
> Signed-off-by: Johan Hovold
> ---
> drivers/usb/serial/option.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index 983c911b3986..3992f7709b70 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -1942,7 +1942,7 @@ static void option_instat_callback(struct urb *urb)
> } else if (status == -ENOENT || status == -ESHUTDOWN) {
> dev_dbg(dev, "%s: urb stopped: %d\n", __func__, status);
> } else
> - dev_err(dev, "%s: error %d\n", __func__, status);
> + dev_dbg(dev, "%s: error %d\n", __func__, status);
>
> /* Resubmit urb so we continue receiving IRQ data */
> if (status != -ESHUTDOWN && status != -ENOENT) {
> --
> 2.4.6
sorry for delay. Had no access to the hardware for some time. First of
all error messages with timing info:
[ 37.106216] option1 ttyUSB10: option_instat_callback: error -71
[ 37.112555] option1 ttyUSB9: option_instat_callback: error -71
[ 37.118843] option1 ttyUSB10: option_instat_callback: error -71
[ 37.125182] option1 ttyUSB9: option_instat_callback: error -71
[ 37.131507] option1 ttyUSB10: option_instat_callback: error -71
[ 37.137893] option1 ttyUSB9: option_instat_callback: error -71
[ 37.144141] option1 ttyUSB10: option_instat_callback: error -71
[ 37.150521] option1 ttyUSB9: option_instat_callback: error -71
[ 37.156810] option1 ttyUSB10: option_instat_callback: error -71
Applying your patch doesn't help, i.e. error messages disappear, but I
get following crash anyway (kernel 3.18.20):
[ 71.855731] INFO: rcu_sched self-detected stall on CPU { 0}
(t=2100 jiffies g=951 c=950 q=7)
[ 71.864822] Task dump for CPU 0:
[ 71.868225] onrisctool R running 0 876837 0x0002
[ 71.874981] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 71.883138] [] (show_stack) from []
(rcu_dump_cpu_stacks+0x84/0xc8)
[ 71.891563] [] (rcu_dump_cpu_stacks) from []
(rcu_check_callbacks+0x424/0
Re: option driver crashes on modem removal
On Mon, 2015-08-24 at 11:14 +0200, Yegor Yefremov wrote: > Applying your patch doesn't help, i.e. error messages disappear, but I > get following crash anyway (kernel 3.18.20): Johan, I think you need to limit the number of retries and schedule a reset when the limit is hit. Regards Oliver -- 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: v4.1.4 xhci: 50%~66% chance of "HC died; cleaning up" on boot
Hi On 23.08.2015 09:37, Vincent Pelletier wrote: Hello, I have one machine which fails to get a working USB bus on at least every other boot. This machine having no PS2 ports, bus failure means no keyboard, and a forced power cycle (sadly, there is no accessible reset button) to try again. Do the usb devices work normally after a successful boot? (no suspicious xhci entries in dmesg) This is with vanilla kernel as of v4.1.4 tag, but I do not know if this is a regression (new machine). Visible USB-related boot messages (boot with "quiet" kernel parameter to avoid scrolling, messages typed from a screenshot): [ 7.25] xhci_hcd :00:14.0: Command completion event does not match command This is not very common, when we queue a command for xhci we write the command to to the command ring, and also add it to a linked list. We get an event for each completed command. the event should match the command in the list. What can I do to debug this issue further ? Add xhci boot debugging to the kernel cmdline: dyndbg='module xhci_hcd +p' or, if as a separate module: xhci_hcd.dyndbg=+p Would there be a way to tell the driver to retry when HC dies so I can at least get the full logs written to disk ? (at a glance, I do not see one in the source) Not what I can remember. If I make some extra xhci debugging patch on top of v4.1.4 can you try it out? (maybe dumping the command ring and the command list) -Mathias -- 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: Some restrictions when using usbtest and g_zero
On Thu, Aug 20, 2015 at 05:03:46PM -0400, Alan Stern wrote:
> On Thu, 20 Aug 2015, Felipe Balbi wrote:
>
> > > > > - When using pattern = 1 as module parameters to compare the data, the
> > > > > packet size must be same between host and device's.
> > > >
> > > > why ?
> > >
> > > The gadget stores the pattern data starting from 0 for each packet it
> > > sends. But the host tests the pattern data starting from 0 only at the
> > > beginning of the transfer; the host expects the pattern to continue
> > > without resetting at packet boundaries (if the transfer length is
> > > larger than the maxpacket size).
> >
> > then that's another bug which needs to be fixed :-)
>
> Here's my attempt at a fix. Completely untested, but it does compile
> without errors. Peter, do you mind trying this out?
After adding related changes at gadget side, it works.
In fact, the gadget stores the pattern data starting from 0 to
transfer length (not the packet length).
Besides, you may need to consider high bandwidth ISO transfer,
you can send the formal patch and I will change gadget side
and have a test.
Besides, if I use vary < length, the test 4/7/8 have failed
(still not check why)
Besides, considering let vary equals to length for control
transfer? In that case, we can use one line command for all tests.
diff --git a/drivers/usb/gadget/function/f_sourcesink.c
b/drivers/usb/gadget/function/f_sourcesink.c
index cbfaf86..414046b 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -510,7 +510,8 @@ static int check_read_data(struct f_sourcesink *ss, struct
usb_request *req)
* stutter for any reason, including buffer duplication...)
*/
case 1:
- if (*buf == (u8)(i % 63))
+
+ if (*buf == (u8)((i % ss->out_ep->desc->wMaxPacketSize)
% 63))
continue;
break;
}
@@ -525,14 +526,14 @@ static void reinit_write_data(struct usb_ep *ep, struct
usb_request *req)
{
unsignedi;
u8 *buf = req->buf;
-
+
switch (pattern) {
case 0:
memset(req->buf, 0, req->length);
break;
case 1:
for (i = 0; i < req->length; i++)
- *buf++ = (u8) (i % 63);
+ *buf++ = (u8) ((i % ep->desc->wMaxPacketSize) % 63);
break;
case 2:
break;
>
> Alan Stern
>
>
>
> Index: usb-4.2/drivers/usb/misc/usbtest.c
> ===
> --- usb-4.2.orig/drivers/usb/misc/usbtest.c
> +++ usb-4.2/drivers/usb/misc/usbtest.c
> @@ -303,11 +303,20 @@ static unsigned mod_pattern;
> module_param_named(pattern, mod_pattern, uint, S_IRUGO | S_IWUSR);
> MODULE_PARM_DESC(mod_pattern, "i/o pattern (0 == zeroes)");
>
> -static inline void simple_fill_buf(struct urb *urb)
> +static unsigned get_maxpacket(struct usb_device *udev, int pipe)
> +{
> + struct usb_host_endpoint*ep;
> +
> + ep = usb_pipe_endpoint(udev, pipe);
> + return le16_to_cpup(&ep->desc.wMaxPacketSize);
> +}
> +
> +static void simple_fill_buf(struct urb *urb)
> {
> unsignedi;
> u8 *buf = urb->transfer_buffer;
> unsignedlen = urb->transfer_buffer_length;
> + unsignedmaxpacket;
>
> switch (pattern) {
> default:
> @@ -316,8 +325,9 @@ static inline void simple_fill_buf(struc
> memset(buf, 0, len);
> break;
> case 1: /* mod63 */
> + maxpacket = get_maxpacket(urb->dev, urb->pipe);
> for (i = 0; i < len; i++)
> - *buf++ = (u8) (i % 63);
> + *buf++ = (u8) ((i % maxpacket) % 63);
> break;
> }
> }
> @@ -349,6 +359,7 @@ static int simple_check_buf(struct usbte
> u8 expected;
> u8 *buf = urb->transfer_buffer;
> unsignedlen = urb->actual_length;
> + unsignedmaxpacket = get_maxpacket(urb->dev, urb->pipe);
>
> int ret = check_guard_bytes(tdev, urb);
> if (ret)
> @@ -366,7 +377,7 @@ static int simple_check_buf(struct usbte
>* with set_interface or set_config.
>*/
> case 1: /* mod63 */
> - expected = i % 63;
> + expected = (i % maxpacket) % 63;
> break;
> /* always fail unsupported patterns */
> default:
> @@ -478,11 +489,12 @@ static void free_sglist(struct scatterli
> }
>
> static struct scatterlist *
> -alloc_sglist(int nents, int max, int vary)
> +alloc_sglist(int nents, int max, int vary, struct usbtest_dev *dev, int pipe)
> {
> struct scatterlist *sg;
> unsignedi;
>
RE: v4.1.4 xhci: 50%~66% chance of "HC died; cleaning up" on boot
From: Mathias Nyman > Sent: 24 August 2015 10:48 > On 23.08.2015 09:37, Vincent Pelletier wrote: > > > > I have one machine which fails to get a working USB bus on at least > > every other boot. This machine having no PS2 ports, bus failure means > > no keyboard, and a forced power cycle (sadly, there is no accessible > > reset button) to try again. > > > > Do the usb devices work normally after a successful boot? > (no suspicious xhci entries in dmesg) > > > This is with vanilla kernel as of v4.1.4 tag, but I do not know if this > > is a regression (new machine). > > > > Visible USB-related boot messages (boot with "quiet" kernel parameter > > to avoid scrolling, messages typed from a screenshot): > > > > [ 7.25] xhci_hcd :00:14.0: Command completion event does not match > > command > > This is not very common, when we queue a command for xhci we write the > command to > to the command ring, and also add it to a linked list. We get an event for > each completed > command. the event should match the command in the list. Probably worth determining which xhci controller hardware is being used. I've seen issues with (I think) the asmedia one where it would only process one command per ring of the doorbell. This generated timeouts when multiple requests got queued (doesn't happen often, but the Ge driver didn't wait for all responses). After that each later command actioned one of the queued ones - giving confusing results. I didn't try patching that one, I've still still got the host, but not the ethernet card. (Easy enough to ring the doorbell again just in case.) David -- 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: gadget: f_uac1: Convert use of __constant_cpu_to_le16 to cpu_to_le16
On Mon, Aug 24, 2015 at 2:29 PM, David Laight wrote: > From: Vaishali Thakkar [mailto:vthakkar1...@gmail.com] >> Sent: 22 August 2015 02:57 > ... >> >> - .bcdADC = __constant_cpu_to_le16(0x0100), >> >> - .wTotalLength = __constant_cpu_to_le16(UAC_DT_TOTAL_LENGTH), >> >> + .bcdADC = cpu_to_le16(0x0100), >> >> + .wTotalLength = cpu_to_le16(UAC_DT_TOTAL_LENGTH), >> > >> > Have you test compiled this on a big-endian system? >> > My gut feeling is that is fails. >> >> No. I have tested it on little-endian system only. But I'll >> be really surprised if this will fail. Can you please tell me >> if I am missing something in this particular case or same >> applies for other cases because most of the cases like >> __constant_ are already converted to ? >> >> As far as I know, if the argument is a constant the >> conversion happens at compile time. And unfolding both >> definitions returns to same expression. Still I am trying if >> someone can test it for me on big endian system. > > Flip one to cpu_to_be16() and see if it still compiles. Yes. It still compiles. > Static initialisers and case labels can be expressions, but the > expression itself must only contain constants. > So it needs to be constant regardless of the value of any constants. > If it contains 'a ? t : f' then both 't' and 'f' must be constant. > > In code, if 'a' is constant the optimiser discards one of 't' or 'f'. > I'm not sure what happens for non-static initialisers (they generate > odd code at the best of times). > > David > -- Vaishali -- 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] usbnet: Fix two races between usbnet_stop() and the BH
19.08.2015 15:31, Bjørn Mork пишет: Eugene Shatokhin writes: The problem is not in the reordering but rather in the fact that "dev->flags = 0" is not necessarily atomic w.r.t. "clear_bit(EVENT_RX_KILL, &dev->flags)", and vice versa. So the following might be possible, although unlikely: CPU0 CPU1 clear_bit: read dev->flags clear_bit: clear EVENT_RX_KILL in the read value dev->flags=0; clear_bit: write updated dev->flags As a result, dev->flags may become non-zero again. Ah, right. Thanks for explaining. I cannot prove yet that this is an impossible situation. If anyone can, please explain. If so, this part of the patch will not be needed. I wonder if we could simply move the dev->flags = 0 down a few lines to fix both issues? It doesn't seem to do anything useful except for resetting the flags to a sane initial state after the device is down. Stopping the tasklet rescheduling etc depends only on netif_running(), which will be false when usbnet_stop is called. There is no need to touch dev->flags for this to happen. That was one of the first ideas we discussed here. Unfortunately, it is probably not so simple. Setting dev->flags to 0 makes some delayed operations do nothing and, among other things, not to reschedule usbnet_bh(). As you can see in drivers/net/usb/usbnet.c, usbnet_bh() can be called as a tasklet function and as a timer function in a number of situations (look for the usage of dev->bh and dev->delay there). netif_running() is indeed false when usbnet_stop() runs, usbnet_stop() also disables Tx. This seems to be enough for many cases where usbnet_bh() is scheduled, but I am not so sure about the remaining ones, namely: 1. A work function, usbnet_deferred_kevent(), may reschedule usbnet_bh(). Looks like the workqueue is only stopped in usbnet_disconnect(), so a work item might be processed while usbnet_stop() works. Setting dev->flags to 0 makes the work function do nothing, by the way. See also the comment in usbnet_stop() about this. A work item may be placed to this workqueue in a number of ways, by both usbnet module and the mini-drivers. It is not too easy to track all these situations. 2. rx_complete() and tx_complete() may schedule execution of usbnet_bh() as a tasklet or a timer function. These two are URB completion callbacks. It seems, new Rx and Tx URBs cannot be submitted when usbnet_stop() clears dev->flags, indeed. But it does not prevent the completion handlers for the previously submitted URBs from running concurrently with usbnet_stop(). The latter waits for them to complete (via usbnet_terminate_urbs(dev)) but only if FLAG_AVOID_UNLINK_URBS is not set in info->flags. rndis_wlan, however, sets this flag for a few hardware models. So - no guarantees here as well. If someone could list the particular bits of dev->flags that should be cleared to make sure no deferred call could reschedule usbnet_bh(), etc... Well, it would be enough to clear these first and use dev->flags = 0 later, after tasklet_kill() and del_timer_sync(). I cannot point out these particular bits now. Besides, it is possible, although unlikely, that new event bits will be added to dev->flags in the future. And one will need to keep track of these to see if they should be cleared as well. I'd prever to play safer for now and clear them all. The EVENT_NO_RUNTIME_PM bug should definitely be fixed. Please split that out as a separate fix. It's a separate issue, and should be backported to all maintained stable releases it applies to (anything from v3.8 and newer) Yes, that makes sense. However, this fix was originally provided by Oliver Neukum rather than me, so I would like to hear his opinion as well first. If what I write above is correct (please help me verify...), then maybe it does make sense to do these together anyway. I think, your suggestion to make it a separate patch is right. Will do it in the next version of the patchset, hopefully soon. Regards, Eugene -- 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 v3 1/1] USB:option:add ZTE PIDs
On Mon, Aug 24, 2015 at 09:51:33AM +0200, Bjørn Mork wrote:
> Johan Hovold writes:
> > On Wed, Aug 19, 2015 at 08:51:17AM -0700, Liu.Zhao wrote:
> >>
> >> #define BENQ_VENDOR_ID0x04a5
> >> #define BENQ_PRODUCT_H10 0x4068
> >> @@ -544,6 +548,14 @@ static const struct option_blacklist_info
> >> zte_mc2716_z_blacklist = {
> >>.sendsetup = BIT(1) | BIT(2) | BIT(3),
> >> };
> >>
> >> +static const struct option_blacklist_info
> >> zte_me3620andzm8620_xl_blacklist = {
> >> + .reserved = BIT(3) | BIT(4) | BIT(5),
> >> +};
> >
> > Use two structs for this: zte_me3620_blacklist and zm8620_xl_blacklist
> > even if they reserve the same ports.
>
> Why?
To avoid including every device family in the symbol name (and we
already have duplicate blacklist definitions).
> Wouldn't it be better to merge all identical lists and give them
> structured names describing their contents instead?
It certainly would.
> E.g.
>
> static const struct option_blacklist_info bi_s0001_r = {
> .sendsetup = BIT(0) | BIT(1),
> };
>
> static const struct option_blacklist_info bi_s0001_r04 = {
> .sendsetup = BIT(0) | BIT(1),
> .reserved = BIT(4),
> };
>
> static const struct option_blacklist_info bi_s_r030405 = {
> .reserved = BIT(3) | BIT(4) | BIT(5),
> };
>
>
> etc. Or some other naming scheme.
Perhaps bi_s_r (e.g. bi_s3_r0, bi_s3_r10, and
bi_s0_r38 for the above) would be too compact?
> I don't see the point of having lots of identical structs just to be
> able to name them after some rarely meaningful marketing name. Many
> vendors recycle their pids, making this completely futile.
I agree. Let's just decide on a naming scheme first.
Johan
--
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: option driver crashes on modem removal
On Mon, Aug 24, 2015 at 11:14:13AM +0200, Yegor Yefremov wrote: > On Mon, Aug 17, 2015 at 5:29 PM, Johan Hovold wrote: > > This obviously does not solve the underlying issue, but could you try > > the below patch nonetheless? We don't want these error messages for the > > "valid" use case of removing an in-use device anyway. > > From 81913b6a91bd53b4f00455618c141c35148acbfc Mon Sep 17 00:00:00 2001 > > From: Johan Hovold > > Date: Mon, 17 Aug 2015 16:48:47 +0200 > > Subject: [PATCH] USB: option: silence interrupt errors > > > > Avoid spamming the logs (e.g. with -EPROTO errors) when attempting to > > resubmit the interrupt urb while a disconnect of an in-use device is > > being processed. > > > > Signed-off-by: Johan Hovold > Applying your patch doesn't help, i.e. error messages disappear, but I > get following crash anyway (kernel 3.18.20): Ok, thanks for testing anyway. Johan -- 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: option driver crashes on modem removal
On Mon, Aug 24, 2015 at 11:32:37AM +0200, Oliver Neukum wrote: > On Mon, 2015-08-24 at 11:14 +0200, Yegor Yefremov wrote: > > Applying your patch doesn't help, i.e. error messages disappear, but I > > get following crash anyway (kernel 3.18.20): > > Johan, I think you need to limit the number of retries > and schedule a reset when the limit is hit. Yeah, it looks like that. But this means we'd end up having to modify basically every USB driver (with IN endpoints) because of these host controllers. Seems like something that should be handled by USB core if possible. Deferred URB give back on (consecutive) errors? Johan -- 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 v4 13/13] usb: otg: Add dual-role device (DRD) support
DRD mode is a reduced functionality OTG mode. In this mode
we don't support SRP, HNP and dynamic role-swap.
In DRD operation, the controller mode (Host or Peripheral)
is decided based on the ID pin status. Once a cable plug (Type-A
or Type-B) is attached the controller selects the state
and doesn't change till the cable in unplugged and a different
cable type is inserted.
As we don't need most of the complex OTG states and OTG timers
we implement a lean DRD state machine in usb-otg.c.
The DRD state machine is only interested in 2 hardware inputs
'id' and 'b_sess_vld'.
Signed-off-by: Roger Quadros
---
drivers/usb/common/usb-otg.c | 178 +--
include/linux/usb/otg-fsm.h | 5 ++
include/linux/usb/otg.h | 2 +
3 files changed, 177 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
index 930c2fe..44b5577 100644
--- a/drivers/usb/common/usb-otg.c
+++ b/drivers/usb/common/usb-otg.c
@@ -519,14 +519,165 @@ int usb_otg_start_gadget(struct otg_fsm *fsm, int on)
}
EXPORT_SYMBOL_GPL(usb_otg_start_gadget);
+/* Change USB protocol when there is a protocol change */
+static int drd_set_protocol(struct otg_fsm *fsm, int protocol)
+{
+ struct usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
+ int ret = 0;
+
+ if (fsm->protocol != protocol) {
+ dev_dbg(otgd->dev, "otg: changing role fsm->protocol= %d; new
protocol= %d\n",
+ fsm->protocol, protocol);
+ /* stop old protocol */
+ if (fsm->protocol == PROTO_HOST)
+ ret = otg_start_host(fsm, 0);
+ else if (fsm->protocol == PROTO_GADGET)
+ ret = otg_start_gadget(fsm, 0);
+ if (ret)
+ return ret;
+
+ /* start new protocol */
+ if (protocol == PROTO_HOST)
+ ret = otg_start_host(fsm, 1);
+ else if (protocol == PROTO_GADGET)
+ ret = otg_start_gadget(fsm, 1);
+ if (ret)
+ return ret;
+
+ fsm->protocol = protocol;
+ return 0;
+ }
+
+ return 0;
+}
+
+/* Called when entering a DRD state */
+static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
+{
+ struct usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
+
+ if (fsm->otg->state == new_state)
+ return;
+
+ fsm->state_changed = 1;
+ dev_dbg(otgd->dev, "otg: set state: %s\n",
+ usb_otg_state_string(new_state));
+ switch (new_state) {
+ case OTG_STATE_B_IDLE:
+ drd_set_protocol(fsm, PROTO_UNDEF);
+ break;
+ case OTG_STATE_B_PERIPHERAL:
+ drd_set_protocol(fsm, PROTO_GADGET);
+ break;
+ case OTG_STATE_A_HOST:
+ drd_set_protocol(fsm, PROTO_HOST);
+ break;
+ case OTG_STATE_UNDEFINED:
+ case OTG_STATE_B_SRP_INIT:
+ case OTG_STATE_B_WAIT_ACON:
+ case OTG_STATE_B_HOST:
+ case OTG_STATE_A_IDLE:
+ case OTG_STATE_A_WAIT_VRISE:
+ case OTG_STATE_A_WAIT_BCON:
+ case OTG_STATE_A_SUSPEND:
+ case OTG_STATE_A_PERIPHERAL:
+ case OTG_STATE_A_WAIT_VFALL:
+ case OTG_STATE_A_VBUS_ERR:
+ default:
+ dev_warn(otgd->dev, "%s: otg: invalid state: %s\n",
+__func__, usb_otg_state_string(new_state));
+ break;
+ }
+
+ fsm->otg->state = new_state;
+}
+
/**
- * OTG FSM work function
+ * DRD state change judgement
+ *
+ * For DRD we're only interested in some of the OTG states
+ * i.e. OTG_STATE_B_IDLE: both peripheral and host are stopped
+ * OTG_STATE_B_PERIPHERAL: peripheral active
+ * OTG_STATE_A_HOST: host active
+ * we're only interested in the following inputs
+ * fsm->id, fsm->b_sess_vld
+ */
+static int drd_statemachine(struct otg_fsm *fsm)
+{
+ struct usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
+ enum usb_otg_state state;
+
+ mutex_lock(&fsm->lock);
+
+ state = fsm->otg->state;
+
+ switch (state) {
+ case OTG_STATE_UNDEFINED:
+ if (!fsm->id)
+ drd_set_state(fsm, OTG_STATE_A_HOST);
+ else if (fsm->id && fsm->b_sess_vld)
+ drd_set_state(fsm, OTG_STATE_B_PERIPHERAL);
+ else
+ drd_set_state(fsm, OTG_STATE_B_IDLE);
+ break;
+ case OTG_STATE_B_IDLE:
+ if (!fsm->id)
+ drd_set_state(fsm, OTG_STATE_A_HOST);
+ else if (fsm->b_sess_vld)
+ drd_set_state(fsm, OTG_STATE_B_PERIPHERAL);
+ break;
+ case OTG_STATE_B_PERIPHERAL:
+ if (!fsm->id)
+ drd_set_state(fsm, OTG_STATE_A_HOST);
+ else if
[PATCH v4 11/13] usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable
This is the a_set_b_hnp_enable flag in the OTG state machine
diagram and must be set when the A-Host has successfully set
the b_hnp_enable feature of the OTG-B-Peripheral attached to it.
When this bit changes we kick our OTG FSM to make note of the
change and act accordingly.
Signed-off-by: Roger Quadros
---
drivers/usb/core/hub.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 431839b..3993168 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2271,6 +2271,9 @@ static int usb_enumerate_device_otg(struct usb_device
*udev)
"can't set HNP mode: %d\n",
err);
bus->b_hnp_enable = 0;
+ } else {
+ /* notify OTG fsm about
a_set_b_hnp_enable */
+ usb_otg_kick_fsm(udev->bus->controller);
}
}
}
@@ -4238,8 +4241,13 @@ hub_port_init (struct usb_hub *hub, struct usb_device
*udev, int port1,
*/
if (!hdev->parent) {
delay = HUB_ROOT_RESET_TIME;
- if (port1 == hdev->bus->otg_port)
+ if (port1 == hdev->bus->otg_port) {
hdev->bus->b_hnp_enable = 0;
+#ifdef CONFIG_USB_OTG
+ /* notify OTG fsm about a_set_b_hnp_enable change */
+ usb_otg_kick_fsm(hdev->bus->controller);
+#endif
+ }
}
/* Some low speed devices have problems with the quick delay, so */
--
2.1.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 v4 10/13] usb: hcd: Adapt to OTG core
The existing usb_add/remove_hcd() functionality
remains unchanged for non-OTG devices. For OTG
devices they only register the HCD with the OTG core.
Introduce usb_otg_add/remove_hcd() for use by OTG core.
These functions actually add/remove the HCD.
Signed-off-by: Roger Quadros
---
drivers/usb/core/hcd.c | 55 +-
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index c0fd1f6..4851682 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -46,6 +46,7 @@
#include
#include
#include
+#include
#include "usb.h"
@@ -2625,8 +2626,8 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd)
* buffers of consistent memory, register the bus, request the IRQ line,
* and call the driver's reset() and start() routines.
*/
-int usb_add_hcd(struct usb_hcd *hcd,
- unsigned int irqnum, unsigned long irqflags)
+static int usb_otg_add_hcd(struct usb_hcd *hcd,
+ unsigned int irqnum, unsigned long irqflags)
{
int retval;
struct usb_device *rhdev;
@@ -2839,17 +2840,16 @@ err_phy:
}
return retval;
}
-EXPORT_SYMBOL_GPL(usb_add_hcd);
/**
- * usb_remove_hcd - shutdown processing for generic HCDs
+ * usb_otg_remove_hcd - shutdown processing for generic HCDs
* @hcd: the usb_hcd structure to remove
* Context: !in_interrupt()
*
* Disconnects the root hub, then reverses the effects of usb_add_hcd(),
* invoking the HCD's stop() method.
*/
-void usb_remove_hcd(struct usb_hcd *hcd)
+static void usb_otg_remove_hcd(struct usb_hcd *hcd)
{
struct usb_device *rhdev = hcd->self.root_hub;
@@ -2923,6 +2923,51 @@ void usb_remove_hcd(struct usb_hcd *hcd)
usb_put_invalidate_rhdev(hcd);
}
+
+static struct otg_hcd_ops otg_hcd_intf = {
+ .add = usb_otg_add_hcd,
+ .remove = usb_otg_remove_hcd,
+};
+
+/**
+ * usb_add_hcd - finish generic HCD structure initialization and register
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ *
+ * Finish the remaining parts of generic HCD initialization: allocate the
+ * buffers of consistent memory, register the bus, request the IRQ line,
+ * and call the driver's reset() and start() routines.
+ * If it is an OTG device then it only registers the HCD with OTG core.
+ *
+ */
+int usb_add_hcd(struct usb_hcd *hcd,
+ unsigned int irqnum, unsigned long irqflags)
+{
+ /* If OTG device, OTG core takes care of adding HCD */
+ if (usb_otg_register_hcd(hcd, irqnum, irqflags, &otg_hcd_intf))
+ return usb_otg_add_hcd(hcd, irqnum, irqflags);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(usb_add_hcd);
+
+/**
+ * usb_remove_hcd - shutdown processing for generic HCDs
+ * @hcd: the usb_hcd structure to remove
+ * Context: !in_interrupt()
+ *
+ * Disconnects the root hub, then reverses the effects of usb_add_hcd(),
+ * invoking the HCD's stop() method.
+ * If it is an OTG device then it unregisters the HCD from OTG core
+ * as well.
+ */
+void usb_remove_hcd(struct usb_hcd *hcd)
+{
+ /* If OTG device, OTG core takes care of stopping HCD */
+ if (usb_otg_unregister_hcd(hcd))
+ usb_otg_remove_hcd(hcd);
+}
EXPORT_SYMBOL_GPL(usb_remove_hcd);
void
--
2.1.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 v4 08/13] usb: doc: dt-binding: Add otg-controller property
The otg-controller property is used to link the host/gadget controllers to the otg controller. otg-controller property must contain the phandle of the otg controller. The otg core uses this property to tie the host/gadget controllres to the correct otg controller. Signed-off-by: Roger Quadros --- Documentation/devicetree/bindings/usb/generic.txt | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt index bba8257..5a457bc 100644 --- a/Documentation/devicetree/bindings/usb/generic.txt +++ b/Documentation/devicetree/bindings/usb/generic.txt @@ -24,6 +24,11 @@ Optional properties: optional for OTG device. - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is optional for OTG device. + - otg-controller: phandle to otg controller. Host or gadget controllers can + contain this property to link it to a particular otg + controller. If this property is not present then otg + core assumes that host and gadget controllers are + children of the otg controller. This is an attribute to a USB controller such as: -- 2.1.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 v4 12/13] usb: gadget: udc: adapt to OTG core
The OTG state machine needs a mechanism to start and
stop the gadget controller. Add usb_gadget_start()
and usb_gadget_stop().
Register with OTG core when gadget function driver
is available and unregister when function driver is unbound.
We need to unlock the usb_lock mutexbefore calling
usb_otg_register_gadget() in udc_bind_to_driver() and
usb_gadget_remove_driver() else it will cause a circular
locking dependency.
Ignore softconnect sysfs control when we're in OTG
mode as OTG FSM takes care of gadget softconnect using
the b_bus_req mechanism.
Signed-off-by: Roger Quadros
---
drivers/usb/gadget/udc/udc-core.c | 124 +++---
1 file changed, 114 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index f660afb..e961668 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
/**
* struct usb_udc - describes one usb device controller
@@ -37,6 +38,7 @@
* @list - for use by the udc class driver
* @vbus - for udcs who care about vbus status, this value is real vbus status;
* for udcs who do not care about vbus status, this value is always true
+ * @is_otg - we're registered with OTG core and it takes care of UDC start/stop
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -47,6 +49,7 @@ struct usb_udc {
struct device dev;
struct list_headlist;
boolvbus;
+ boolis_otg;
};
static struct class *udc_class;
@@ -300,6 +303,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
*/
static inline int usb_gadget_udc_start(struct usb_udc *udc)
{
+ dev_dbg(&udc->dev, "%s\n", __func__);
return udc->gadget->ops->udc_start(udc->gadget, udc->driver);
}
@@ -317,10 +321,81 @@ static inline int usb_gadget_udc_start(struct usb_udc
*udc)
*/
static inline void usb_gadget_udc_stop(struct usb_udc *udc)
{
+ dev_dbg(&udc->dev, "%s\n", __func__);
udc->gadget->ops->udc_stop(udc->gadget);
}
/**
+ * usb_gadget_start - start the usb gadget controller and connect to bus
+ * @gadget: the gadget device to start
+ *
+ * This is external API for use by OTG core.
+ *
+ * Start the usb device controller and connect to bus (enable pull).
+ */
+static int usb_gadget_start(struct usb_gadget *gadget)
+{
+ int ret;
+ struct usb_udc *udc = NULL;
+
+ dev_dbg(&gadget->dev, "%s\n", __func__);
+ mutex_lock(&udc_lock);
+ list_for_each_entry(udc, &udc_list, list)
+ if (udc->gadget == gadget)
+ goto found;
+
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+
+found:
+ ret = usb_gadget_udc_start(udc);
+ if (ret)
+ dev_err(&udc->dev, "USB Device Controller didn't start: %d\n",
+ ret);
+ else
+ usb_udc_connect_control(udc);
+
+ mutex_unlock(&udc_lock);
+
+ return ret;
+}
+
+/**
+ * usb_gadget_stop - disconnect from bus and stop the usb gadget
+ * @gadget: The gadget device we want to stop
+ *
+ * This is external API for use by OTG core.
+ *
+ * Disconnect from the bus (disable pull) and stop the
+ * gadget controller.
+ */
+static int usb_gadget_stop(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc = NULL;
+
+ dev_dbg(&gadget->dev, "%s\n", __func__);
+ mutex_lock(&udc_lock);
+ list_for_each_entry(udc, &udc_list, list)
+ if (udc->gadget == gadget)
+ goto found;
+
+ dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+ __func__);
+ mutex_unlock(&udc_lock);
+ return -EINVAL;
+
+found:
+ usb_gadget_disconnect(udc->gadget);
+ udc->driver->disconnect(udc->gadget);
+ usb_gadget_udc_stop(udc);
+ mutex_unlock(&udc_lock);
+
+ return 0;
+}
+
+/**
* usb_udc_release - release the usb_udc struct
* @dev: the dev member within usb_udc
*
@@ -438,6 +513,7 @@ int usb_add_gadget_udc(struct device *parent, struct
usb_gadget *gadget)
}
EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
+/* udc_lock must be held */
static void usb_gadget_remove_driver(struct usb_udc *udc)
{
dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n",
@@ -445,10 +521,18 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
- usb_gadget_disconnect(udc->gadget);
- udc->driver->disconnect(udc->gadget);
+ /* If OTG, the otg core ensures UDC is stopped on unregister */
+ if (udc->is_otg) {
+ mutex_unlock(&udc_lock);
+ usb_otg_unregister_gadget(udc->gadget);
+
[PATCH v4 09/13] usb: chipidea: move from CONFIG_USB_OTG_FSM to CONFIG_USB_OTG
CONFIG_USB_OTG_FSM no longer exists and the OTG FSM
is available if CONFIG_USB_OTG is defined so move
to using CONFIG_USB_OTG.
Signed-off-by: Roger Quadros
---
Documentation/usb/chipidea.txt | 2 +-
drivers/usb/chipidea/Makefile | 2 +-
drivers/usb/chipidea/ci.h | 2 +-
drivers/usb/chipidea/otg_fsm.h | 2 +-
drivers/usb/phy/Kconfig| 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index 3f848c1..e7f97da 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -5,7 +5,7 @@ with 2 Freescale i.MX6Q sabre SD boards.
1.1 How to enable OTG FSM in menuconfig
---
-Select CONFIG_USB_OTG_FSM, rebuild kernel Image and modules.
+Select CONFIG_USB_OTG, rebuild kernel Image and modules.
If you want to check some internal variables for otg fsm,
select CONFIG_USB_CHIPIDEA_DEBUG, there are 2 files which
can show otg fsm variables and some controller registers value:
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 4decb12..63f2394 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -6,7 +6,7 @@ ci_hdrc-y := core.o otg.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)+= host.o
ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG) += debug.o
-ci_hdrc-$(CONFIG_USB_OTG_FSM) += otg_fsm.o
+ci_hdrc-$(CONFIG_USB_OTG) += otg_fsm.o
# Glue/Bridge layers go here
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 41d7cf6..438818c 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -407,7 +407,7 @@ static inline u32 hw_test_and_write(struct ci_hdrc *ci,
enum ci_hw_regs reg,
*/
static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
{
-#ifdef CONFIG_USB_OTG_FSM
+#ifdef CONFIG_USB_OTG
struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
index 2689375..fb66695 100644
--- a/drivers/usb/chipidea/otg_fsm.h
+++ b/drivers/usb/chipidea/otg_fsm.h
@@ -62,7 +62,7 @@
/* SSEND time before SRP */
#define TB_SSEND_SRP (1500)/* minimum 1.5 sec, section:5.1.2 */
-#ifdef CONFIG_USB_OTG_FSM
+#ifdef CONFIG_USB_OTG
int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
int ci_otg_fsm_work(struct ci_hdrc *ci);
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 7d3beee..664cacb 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -20,7 +20,7 @@ config AB8500_USB
config FSL_USB2_OTG
bool "Freescale USB OTG Transceiver Driver"
- depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM
+ depends on USB_EHCI_FSL && USB_FSL_USB2 && PM
select USB_OTG
select USB_PHY
help
--
2.1.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 v4 06/13] usb: gadget.h: Add OTG to gadget interface
The OTG core will use struct otg_gadget_ops to
start/stop the gadget controller.
The main purpose of this interface is to avoid directly
calling usb_gadget_start/stop() from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB_GADGET is m.
Signed-off-by: Roger Quadros
Reviewed-by: Peter Chen
---
include/linux/usb/gadget.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c14a69b..c019242 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1052,6 +1052,20 @@ struct usb_gadget_driver {
};
+/*-*/
+
+/**
+ * struct otg_gadget_ops - Interface between OTG core and gadget
+ *
+ * Provided by the gadget core to allow the OTG core to start/stop the gadget
+ *
+ * @start: function to start the gadget
+ * @stop: function to stop the gadget
+ */
+struct otg_gadget_ops {
+ int (*start)(struct usb_gadget *gadget);
+ int (*stop)(struct usb_gadget *gadget);
+};
/*-*/
--
2.1.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 v4 07/13] usb: otg: add OTG core
The OTG core instantiates the OTG Finite State Machine
per OTG controller and manages starting/stopping the
host and gadget controllers based on the bus state.
It provides APIs for the following tasks
- Registering an OTG capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine
Signed-off-by: Roger Quadros
---
MAINTAINERS |4 +-
drivers/usb/Kconfig |2 +-
drivers/usb/Makefile |1 +
drivers/usb/common/Makefile |3 +-
drivers/usb/common/usb-otg.c | 1061 ++
drivers/usb/common/usb-otg.h | 71 +++
drivers/usb/core/Kconfig | 11 +-
include/linux/usb/otg.h | 189 +++-
8 files changed, 1321 insertions(+), 21 deletions(-)
create mode 100644 drivers/usb/common/usb-otg.c
create mode 100644 drivers/usb/common/usb-otg.h
diff --git a/MAINTAINERS b/MAINTAINERS
index a9ae6c1..4bc1279 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10667,12 +10667,14 @@ S:Maintained
F: Documentation/usb/ohci.txt
F: drivers/usb/host/ohci*
-USB OTG FSM (Finite State Machine)
+USB OTG/DRD core and FSM (Finite State Machine)
M: Peter Chen
+M: Roger Quadros
T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
L: linux-usb@vger.kernel.org
S: Maintained
F: drivers/usb/common/usb-otg-fsm.c
+F: drivers/usb/common/usb-otg.c
USB OVER IP DRIVER
M: Valentina Manea
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 8ed451d..5b625e2 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -32,7 +32,7 @@ if USB_SUPPORT
config USB_COMMON
tristate
default y
- depends on USB || USB_GADGET
+ depends on USB || USB_GADGET || USB_OTG
config USB_ARCH_HAS_HCD
def_bool y
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index d8926c6..769d13b 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/
obj-$(CONFIG_USB_GADGET) += gadget/
obj-$(CONFIG_USB_COMMON) += common/
+obj-$(CONFIG_USB_OTG) += common/
obj-$(CONFIG_USBIP_CORE) += usbip/
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..730d928 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
usb-common-$(CONFIG_USB_LED_TRIG) += led.o
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
+usbotg-y := usb-otg.o usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG) += usbotg.o
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
new file mode 100644
index 000..930c2fe
--- /dev/null
+++ b/drivers/usb/common/usb-otg.c
@@ -0,0 +1,1061 @@
+/**
+ * drivers/usb/common/usb-otg.c - USB OTG core
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Roger Quadros
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include /* enum usb_otg_state */
+#include
+#include
+
+#include "usb-otg.h"
+
+struct otg_gcd {
+ struct usb_gadget *gadget;
+ struct otg_gadget_ops *ops;
+};
+
+/* OTG device list */
+LIST_HEAD(otg_list);
+static DEFINE_MUTEX(otg_list_mutex);
+
+/* Hosts and Gadgets waiting for OTG controller */
+struct otg_wait_data {
+ struct device *dev; /* OTG controller device */
+
+ struct otg_hcd primary_hcd;
+ struct otg_hcd shared_hcd;
+ struct otg_gcd gcd;
+ struct list_head list;
+};
+
+LIST_HEAD(wait_list);
+static DEFINE_MUTEX(wait_list_mutex);
+
+static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
+{
+ if (!hcd->primary_hcd)
+ return 1;
+ return hcd == hcd->primary_hcd;
+}
+
+/**
+ * Check if the OTG device is in our wait list and return
+ * otg_wait_data, else NULL.
+ *
+ * wait_list_mutex must be held.
+ */
+static struct otg_wait_data *usb_otg_get_wait(struct device *otg_dev)
+{
+ struct otg_wait_data *wait;
+
+ if (!otg_dev)
+ return NULL;
+
+ /* is there an entry for this otg_dev ?*/
+ list_for_each_entry(wait, &wait_list, list) {
+ if (wait->dev == otg_dev)
+ return wait;
+ }
+
+ return NULL;
+}
+
+/**
+ * Add the hcd to our wait list
+ */
+static int usb_otg_hcd_wait
[PATCH v4 01/13] usb: otg-fsm: Add documentation for struct otg_fsm
struct otg_fsm is the interface to the OTG state machine.
Document the input, output and internal state variables.
Definations are taken from Table 7-2 and Table 7-4 of
the USB OTG & EH Specification Rev.2.0
Re-arrange some of the members as per use case for more
clarity.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
include/linux/usb/otg-fsm.h | 90 +
1 file changed, 83 insertions(+), 7 deletions(-)
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index f728f18..fc5b4d9 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -59,37 +59,113 @@ enum otg_fsm_timer {
NUM_OTG_FSM_TIMERS,
};
-/* OTG state machine according to the OTG spec */
+/**
+ * struct otg_fsm - OTG state machine according to the OTG spec
+ *
+ * OTG hardware Inputs
+ *
+ * Common inputs for A and B device
+ * @id:TRUE for B-device, FALSE for A-device.
+ * @adp_change: TRUE when current ADP measurement (n) value, compared to the
+ * ADP measurement taken at n-2, differs by more than CADP_THR
+ * @power_up: TRUE when the OTG device first powers up its USB system and
+ * ADP measurement taken if ADP capable
+ *
+ * A-Device state inputs
+ * @a_srp_det: TRUE if the A-device detects SRP
+ * @a_vbus_vld:TRUE when VBUS voltage is in regulation
+ * @b_conn:TRUE if the A-device detects connection from the B-device
+ * @a_bus_resume: TRUE when the B-device detects that the A-device is signaling
+ * a resume (K state)
+ * B-Device state inputs
+ * @a_bus_suspend: TRUE when the B-device detects that the A-device has put the
+ * bus into suspend
+ * @a_conn:TRUE if the B-device detects a connection from the A-device
+ * @b_se0_srp: TRUE when the line has been at SE0 for more than the minimum
+ * time before generating SRP
+ * @b_ssend_srp: TRUE when the VBUS has been below VOTG_SESS_VLD for more than
+ * the minimum time before generating SRP
+ * @b_sess_vld:TRUE when the B-device detects that the voltage on VBUS
is
+ * above VOTG_SESS_VLD
+ * @test_device: TRUE when the B-device switches to B-Host and detects an OTG
+ * test device. This must be set by host/hub driver
+ *
+ * Application inputs (A-Device)
+ * @a_bus_drop:TRUE when A-device application needs to power down the
bus
+ * @a_bus_req: TRUE when A-device application wants to use the bus.
+ * FALSE to suspend the bus
+ *
+ * Application inputs (B-Device)
+ * @b_bus_req: TRUE during the time that the Application running on the
+ * B-device wants to use the bus
+ *
+ * Auxilary inputs (OTG v1.3 only. Obsolete now.)
+ * @a_sess_vld:TRUE if the A-device detects that VBUS is above
VA_SESS_VLD
+ * @b_bus_suspend: TRUE when the A-device detects that the B-device has put
+ * the bus into suspend
+ * @b_bus_resume: TRUE when the A-device detects that the B-device is signaling
+ * resume on the bus
+ *
+ * OTG Output status. Read only for users. Updated by OTG FSM helpers defined
+ * in this file
+ *
+ * Outputs for Both A and B device
+ * @drv_vbus: TRUE when A-device is driving VBUS
+ * @loc_conn: TRUE when the local device has signaled that it is connected
+ * to the bus
+ * @loc_sof: TRUE when the local device is generating activity on the bus
+ * @adp_prb: TRUE when the local device is in the process of doing
+ * ADP probing
+ *
+ * Outputs for B-device state
+ * @adp_sns: TRUE when the B-device is in the process of carrying out
+ * ADP sensing
+ * @data_pulse: TRUE when the B-device is performing data line pulsing
+ *
+ * Internal Variables
+ *
+ * a_set_b_hnp_en: TRUE when the A-device has successfully set the
+ * b_hnp_enable bit in the B-device.
+ *Unused as OTG fsm uses otg->host->b_hnp_enable instead
+ * b_srp_done: TRUE when the B-device has completed initiating SRP
+ * b_hnp_enable: TRUE when the B-device has accepted the
+ * SetFeature(b_hnp_enable) B-device.
+ * Unused as OTG fsm uses otg->gadget->b_hnp_enable instead
+ * a_clr_err: Asserted (by application ?) to clear a_vbus_err due to an
+ * overcurrent condition and causes the A-device to transition
+ * to a_wait_vfall
+ */
struct otg_fsm {
/* Input */
int id;
int adp_change;
int power_up;
- int test_device;
- int a_bus_drop;
- int a_bus_req;
int a_srp_det;
int a_vbus_vld;
int b_conn;
int a_bus_resume;
int a_bus_suspend;
int a_conn;
- int b_bus_req;
int b_se0_srp;
int b_ssend_srp;
int b_sess_vld;
+ int test_device;
+ int a_bus_drop;
+ int a_bus_req;
+ int b_bus_req;
+
/* Auxilary inputs */
int a_sess_vl
[PATCH v4 00/13] USB: OTG/DRD Core functionality
Hi, This series centralizes OTG/Dual-role functionality in the kernel. As of now I've got Dual-role functionality working pretty reliably on dra7-evm and am437x-gp-evm. DWC3 controller and platform related patches will be sent separately. Series is based on Greg's usb-next tree. Changelog: - v4: - Added DT support for tying otg-controller to host and gadget controllers. For DT we no longer have the constraint that OTG controller needs to be parent of host and gadget. They can be tied together using the "otg-controller" property. - Relax the requirement for DT case that otg controller must register before host/gadget. We maintain a wait list of host/gadget devices waiting on the otg controller. - Use a single struct usb_otg for otg data. - Don't override host/gadget start/stop APIs. Let the controller drivers do what they want as they know best. Helper API is provided for controller start/stop that controller driver can use. - Introduce struct usb_otg_config to pass the otg capabilities, otg ops and otg timer timeouts during otg controller registration. - rebased on Greg's usb.git/usb-next v3: - all otg related definations now in otg.h - single kernel config USB_OTG to enable OTG core and FSM. - resolved symbol dependency issues. - use dev_vdbg instead of VDBG() in usb-otg-fsm.c - rebased on v4.2-rc1 v2: - Use add/remove_hcd() instead of start/stop_hcd() to enable/disable the host controller - added dual-role-device (DRD) state machine which is a much simpler mode of operation when compared to OTG. Here we don't support fancy OTG features like HNP, SRP, on the fly role-swap. The mode of operation is determined based on ID pin (cable type) and the role doesn't change till the cable type changes. Why?: Most of the OTG drivers have been dealing with the OTG state machine themselves and there is a scope for code re-use. This has been partly addressed by the usb/common/usb-otg-fsm.c but it still leaves the instantiation of the state machine and OTG timers to the controller drivers. We re-use usb-otg-fsm.c but go one step further by instantiating the state machine and timers thus making it easier for drivers to implement OTG functionality. Newer OTG cores support standard host interface (e.g. xHCI) so host and gadget functionality are no longer closely knit like older cores. There needs to be a way to co-ordinate the operation of the host and gadget in OTG mode. i.e. to stop and start them from a central location. This central location should be the USB OTG core. Host and gadget controllers might be sharing resources and can't be always running. One has to be stopped for the other to run. This can't be done as of now and can be done from the OTG core. What?: - The OTG core instantiates the OTG/DRD Finite State Machine per OTG controller and manages starting/stopping the host and gadget controllers based on the bus state. It provides APIs for the following - Registering an OTG capable controller struct otg_fsm *usb_otg_register(struct device *dev, struct usb_otg_config *config); int usb_otg_unregister(struct device *dev); - Registering Host controllers to OTG core (used by hcd-core) int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags, struct otg_hcd_ops *ops); int usb_otg_unregister_hcd(struct usb_hcd *hcd); - Registering Gadget controllers to OTG core (used by udc-core) int usb_otg_register_gadget(struct usb_gadget *gadget, struct otg_gadget_ops *ops); int usb_otg_unregister_gadget(struct usb_gadget *gadget); - Providing inputs to and kicking the OTG state machine void usb_otg_sync_inputs(struct otg_fsm *fsm); int usb_otg_kick_fsm(struct device *hcd_gcd_device); - Getting controller device structure from OTG state machine instance struct device *usb_otg_fsm_to_dev(struct otg_fsm *fsm); 'struct otg_fsm' is the interface to the OTG state machine. It contains inputs to the fsm, status of the fsm and operations for the OTG controller driver. - Helper APIs for starting/stopping host/gadget controllers int usb_otg_start_host(struct otg_fsm *fsm, int on); int usb_otg_start_gadget(struct otg_fsm *fsm, int on); Usage model: --- - The OTG core needs to know what host and gadget controllers are linked to the OTG controller. For DT boots we can provide that information by adding "otg-controller" property to the host and gadget controller nodes that points to the right otg controller. For legacy boot we assume that OTG controller is the parent of the host and gadget controllers. For DT if "otg-controller" property is not present then parent child relationship constraint applies. - The OTG controller driver must call usb_otg_register() to register itself with the OTG core. It must also provide the required OTG configuration, fsm operations and timer timeouts (optional) via struct usb_otg_config. The fsm operations will be called depen
[PATCH v4 04/13] otg-fsm: move usb_bus_start_enum into otg-fsm->ops
This is to prevent missing symbol build error if OTG is
enabled (built-in) and HCD core (CONFIG_USB) is module.
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/common/usb-otg-fsm.c | 6 --
drivers/usb/phy/phy-fsl-usb.c| 2 ++
include/linux/usb/otg-fsm.h | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index a46f29a..6e56c8c 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -165,8 +165,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
otg_loc_conn(fsm, 0);
otg_loc_sof(fsm, 1);
otg_set_protocol(fsm, PROTO_HOST);
- usb_bus_start_enum(fsm->otg->host,
- fsm->otg->host->otg_port);
+ if (fsm->ops->start_enum) {
+ fsm->ops->start_enum(fsm->otg->host,
+fsm->otg->host->otg_port);
+ }
break;
case OTG_STATE_A_IDLE:
otg_drv_vbus(fsm, 0);
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index ee3f2c2..19541ed 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -783,6 +783,8 @@ static struct otg_fsm_ops fsl_otg_ops = {
.start_host = fsl_otg_start_host,
.start_gadget = fsl_otg_start_gadget,
+
+ .start_enum = usb_bus_start_enum,
};
/* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 672551c..75e82cc 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -199,6 +199,7 @@ struct otg_fsm_ops {
void(*del_timer)(struct otg_fsm *fsm, enum otg_fsm_timer timer);
int (*start_host)(struct otg_fsm *fsm, int on);
int (*start_gadget)(struct otg_fsm *fsm, int on);
+ int (*start_enum)(struct usb_bus *bus, unsigned port_num);
};
--
2.1.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 v4 02/13] usb: otg-fsm: support multiple instances
Move the state_changed variable into struct otg_fsm
so that we can support multiple instances.
Signed-off-by: Roger Quadros
---
drivers/usb/common/usb-otg-fsm.c | 10 --
include/linux/usb/otg-fsm.h | 1 +
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 61d538a..32862a4 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -61,8 +61,6 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
return 0;
}
-static int state_changed;
-
/* Called when leaving a state. Do state clean up jobs here */
static void otg_leave_state(struct otg_fsm *fsm, enum usb_otg_state old_state)
{
@@ -123,7 +121,6 @@ static void otg_leave_state(struct otg_fsm *fsm, enum
usb_otg_state old_state)
/* Called when entering a state */
static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
{
- state_changed = 1;
if (fsm->otg->state == new_state)
return 0;
VDBG("Set state: %s\n", usb_otg_state_string(new_state));
@@ -237,6 +234,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
}
fsm->otg->state = new_state;
+ fsm->state_changed = 1;
return 0;
}
@@ -248,7 +246,7 @@ int otg_statemachine(struct otg_fsm *fsm)
mutex_lock(&fsm->lock);
state = fsm->otg->state;
- state_changed = 0;
+ fsm->state_changed = 0;
/* State machine state change judgement */
switch (state) {
@@ -361,7 +359,7 @@ int otg_statemachine(struct otg_fsm *fsm)
}
mutex_unlock(&fsm->lock);
- VDBG("quit statemachine, changed = %d\n", state_changed);
- return state_changed;
+ VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+ return fsm->state_changed;
}
EXPORT_SYMBOL_GPL(otg_statemachine);
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index fc5b4d9..20c8219 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -195,6 +195,7 @@ struct otg_fsm {
/* Current usb protocol used: 0:undefine; 1:host; 2:client */
int protocol;
struct mutex lock;
+ bool state_changed;
};
struct otg_fsm_ops {
--
2.1.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 v4 03/13] usb: otg-fsm: Prevent build warning "VDBG" redefined
If usb/otg-fsm.h and usb/composite.h are included together
then it results in the build warning [1].
Prevent that by using dev_vdbg() instead.
Also get rid of MPC_LOC which doesn't seem to be used
by anyone.
[1] - warning fixed by this patch:
In file included from drivers/usb/dwc3/core.h:33,
from drivers/usb/dwc3/ep0.c:33:
include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
In file included from drivers/usb/dwc3/ep0.c:31:
include/linux/usb/composite.h:615:1: warning: this is the location
of the previous definition
Signed-off-by: Roger Quadros
Acked-by: Peter Chen
---
drivers/usb/chipidea/otg_fsm.c | 1 +
drivers/usb/common/usb-otg-fsm.c | 12 +++-
drivers/usb/phy/phy-fsl-usb.c| 1 +
include/linux/usb/otg-fsm.h | 19 ---
4 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 00ab59d..f644752 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -776,6 +776,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
ci->fsm.otg->state = OTG_STATE_UNDEFINED;
ci->fsm.ops = &ci_otg_ops;
+ ci->fsm.dev = ci->dev;
mutex_init(&ci->fsm.lock);
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 32862a4..a46f29a 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -36,8 +36,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
int ret = 0;
if (fsm->protocol != protocol) {
- VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
- fsm->protocol, protocol);
+ dev_vdbg(fsm->dev,
+"Changing role fsm->protocol= %d; new protocol= %d\n",
+fsm->protocol, protocol);
/* stop old protocol */
if (fsm->protocol == PROTO_HOST)
ret = otg_start_host(fsm, 0);
@@ -123,7 +124,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum
usb_otg_state new_state)
{
if (fsm->otg->state == new_state)
return 0;
- VDBG("Set state: %s\n", usb_otg_state_string(new_state));
+ dev_vdbg(fsm->dev, "Set state: %s\n", usb_otg_state_string(new_state));
otg_leave_state(fsm, fsm->otg->state);
switch (new_state) {
case OTG_STATE_B_IDLE:
@@ -251,7 +252,7 @@ int otg_statemachine(struct otg_fsm *fsm)
switch (state) {
case OTG_STATE_UNDEFINED:
- VDBG("fsm->id = %d\n", fsm->id);
+ dev_vdbg(fsm->dev, "fsm->id = %d\n", fsm->id);
if (fsm->id)
otg_set_state(fsm, OTG_STATE_B_IDLE);
else
@@ -359,7 +360,8 @@ int otg_statemachine(struct otg_fsm *fsm)
}
mutex_unlock(&fsm->lock);
- VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+ dev_vdbg(fsm->dev, "quit statemachine, changed = %d\n",
+fsm->state_changed);
return fsm->state_changed;
}
EXPORT_SYMBOL_GPL(otg_statemachine);
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..ee3f2c2 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -817,6 +817,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
/* Set OTG state machine operations */
fsl_otg_tc->fsm.ops = &fsl_otg_ops;
+ fsl_otg_tc->fsm.dev = &pdev->dev;
/* initialize the otg structure */
fsl_otg_tc->phy.label = DRIVER_DESC;
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 20c8219..672551c 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -18,24 +18,10 @@
#ifndef __LINUX_USB_OTG_FSM_H
#define __LINUX_USB_OTG_FSM_H
+#include
#include
#include
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s] " fmt , \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
#define PROTO_UNDEF(0)
#define PROTO_HOST (1)
#define PROTO_GADGET (2)
@@ -196,6 +182,9 @@ struct otg_fsm {
int protocol;
struct mutex lock;
bool state_changed;
+
+ /* for debug prints */
+ struct device *dev;
};
struct otg_fsm_ops {
--
2.1.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 v4 05/13] usb: hcd.h: Add OTG to HCD interface
The OTG core will use struct otg_hcd_ops to
add/remove the HCD controller.
The main purpose of this interface is to avoid directly
calling usb_add/remove_hcd() from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB is m.
Signed-off-by: Roger Quadros
Reviewed-by: Peter Chen
---
include/linux/usb/hcd.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index d2784c1..27d78b1 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -386,6 +386,20 @@ struct hc_driver {
};
+/**
+ * struct otg_hcd_ops - Interface between OTG core and HCD
+ *
+ * Provided by the HCD core to allow the OTG core to start/stop the HCD
+ *
+ * @add: function to add the HCD
+ * @remove: function to remove the HCD
+ */
+struct otg_hcd_ops {
+ int (*add)(struct usb_hcd *hcd,
+ unsigned int irqnum, unsigned long irqflags);
+ void (*remove)(struct usb_hcd *hcd);
+};
+
static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
{
return hcd->driver->flags & HCD_BH;
--
2.1.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] usbnet: Fix two races between usbnet_stop() and the BH
Eugene Shatokhin writes: > 19.08.2015 15:31, Bjørn Mork пишет: >> Eugene Shatokhin writes: >> >>> The problem is not in the reordering but rather in the fact that >>> "dev->flags = 0" is not necessarily atomic >>> w.r.t. "clear_bit(EVENT_RX_KILL, &dev->flags)", and vice versa. >>> >>> So the following might be possible, although unlikely: >>> >>> CPU0 CPU1 >>> clear_bit: read dev->flags >>> clear_bit: clear EVENT_RX_KILL in the read value >>> >>> dev->flags=0; >>> >>> clear_bit: write updated dev->flags >>> >>> As a result, dev->flags may become non-zero again. >> >> Ah, right. Thanks for explaining. >> >>> I cannot prove yet that this is an impossible situation. If anyone >>> can, please explain. If so, this part of the patch will not be needed. >> >> I wonder if we could simply move the dev->flags = 0 down a few lines to >> fix both issues? It doesn't seem to do anything useful except for >> resetting the flags to a sane initial state after the device is down. >> >> Stopping the tasklet rescheduling etc depends only on netif_running(), >> which will be false when usbnet_stop is called. There is no need to >> touch dev->flags for this to happen. > > That was one of the first ideas we discussed here. Unfortunately, it > is probably not so simple. > > Setting dev->flags to 0 makes some delayed operations do nothing and, > among other things, not to reschedule usbnet_bh(). Yes, but I believe that is merely a side effect. You should never need to clear multiple flags to get the desired behaviour. > As you can see in drivers/net/usb/usbnet.c, usbnet_bh() can be called > as a tasklet function and as a timer function in a number of > situations (look for the usage of dev->bh and dev->delay there). > > netif_running() is indeed false when usbnet_stop() runs, usbnet_stop() > also disables Tx. This seems to be enough for many cases where > usbnet_bh() is scheduled, but I am not so sure about the remaining > ones, namely: > > 1. A work function, usbnet_deferred_kevent(), may reschedule > usbnet_bh(). Looks like the workqueue is only stopped in > usbnet_disconnect(), so a work item might be processed while > usbnet_stop() works. Setting dev->flags to 0 makes the work function > do nothing, by the way. See also the comment in usbnet_stop() about > this. > > A work item may be placed to this workqueue in a number of ways, by > both usbnet module and the mini-drivers. It is not too easy to track > all these situations. That's an understatement :) > 2. rx_complete() and tx_complete() may schedule execution of > usbnet_bh() as a tasklet or a timer function. These two are URB > completion callbacks. > > It seems, new Rx and Tx URBs cannot be submitted when usbnet_stop() > clears dev->flags, indeed. But it does not prevent the completion > handlers for the previously submitted URBs from running concurrently > with usbnet_stop(). The latter waits for them to complete (via > usbnet_terminate_urbs(dev)) but only if FLAG_AVOID_UNLINK_URBS is not > set in info->flags. rndis_wlan, however, sets this flag for a few > hardware models. So - no guarantees here as well. FLAG_AVOID_UNLINK_URBS looks like it should be replaced by the newer ability to keep the status urb active. I believe that must have been the real reason for adding it, based on the commit message and the effect the flag will have: commit 1487cd5e76337555737cbc55d7d83f41460d198f Author: Jussi Kivilinna Date: Thu Jul 30 19:41:20 2009 +0300 usbnet: allow "minidriver" to prevent urb unlinking on usbnet_stop rndis_wlan devices freeze after running usbnet_stop several times. It appears that firmware freezes in state where it does not respond to any RNDIS commands and device have to be physically unplugged/replugged. This patch lets minidrivers to disable unlink_urbs on usbnet_stop through new info flag. Signed-off-by: Jussi Kivilinna Cc: David Brownell Signed-off-by: John W. Linville The rx urbs will not be resubmitted in any case, and there are of course no tx urbs being submitted. So the only effect of this flag is on the status/interrupt urb, which I can imagine some RNDIS devices wants active all the time. So FLAG_AVOID_UNLINK_URBS should probably be removed and replaced calls to usbnet_status_start() and usbnet_status_stop(). This will require testing on some of the devices with the original firmware problem however. In any case: I do not think this flag should be considered when trying to make usbnet_stop behaviour saner. It's only purpose is to deliberately break usbnet_stop by not actually stopping. > If someone could list the particular bits of dev->flags that should be > cleared to make sure no deferred call could reschedule usbnet_bh(), > etc... Well, it would be enough to clear these first and use > dev->flags = 0 later, after tasklet_kill() and del_timer_sync(). I > cannot point out these particular bits now. I don'
RE: [PATCH 3/8][v3]usb:fsl:otg: Add support to add/remove usb host driver
On Mon, 24 Aug 2015, Ramneek Mehresh wrote:
> > On Thu, 20 Aug 2015, Ramneek Mehresh wrote:
> >
> > > > > --- a/drivers/usb/host/ehci-fsl.h
> > > > > +++ b/drivers/usb/host/ehci-fsl.h
> > > > > @@ -63,4 +63,22 @@
> > > > > #define UTMI_PHY_EN (1<<9)
> > > > > #define ULPI_PHY_CLK_SEL(1<<10)
> > > > > #define PHY_CLK_VALID(1<<17)
> > > > > +
> > > > > +struct ehci_fsl {
> > > > > +#ifdef CONFIG_PM
> > > > > + /* Saved USB PHY settings, need to restore after deep sleep. */
> > > > > + u32 usb_ctrl;
> > > > > +#endif
> > > >
> > > > Do you need this #ifdef?
> > > >
> > > Yes, this is required for deep-sleep support...we need to save/restore
> > controller
> > > registers during deep-sleep when usb controller power is shut-off. Don't
> > need this
> > > during normal usb operation...saving/restoring usb controller registers in
> > non deep-sleep
> > > scenario will add unnecessary delays
> >
> > What I meant was, can you keep the "u32 usb_ctrl;" line but get rid of
> > the "#ifdef CONFIG_PM" and "#endif" lines?
> >
> > Alan Stern
> I do understand that.
It doesn't sound like you do.
> However, USB suspend/resume functionality work in context of
> PM. Only in this context, we need to save/restore usb controller register
> for deep-sleep functionality. If you see usage of this in ehci-fsl.c file,
> it's used in
> ehci_fsl_drv_suspend() under CONFIG_PM to save USB CNTL register.
Yes, I know.
> If I remove CONFIG_PM from struct ehci_fsl{}, I'll need to change the entire
> driver
> also to make suspend and resume functionalities compile by default.
Why? Suppose you remove the "#ifdef CONFIG_PM" and "#endif" lines but
leave everything else the same. Won't the driver still work?
If CONFIG_PM is enabled then everything will be exactly the same as it
is now. If CONFIG_PM isn't enabled then you will have an extra
usb_ctrl field in the ehci_fsl structure. It will never get used for
anything but the driver should still work. Right?
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: Some restrictions when using usbtest and g_zero
On Mon, 24 Aug 2015, Peter Chen wrote:
> Thanks, that's much clear.
>
> At udc driver:
>
> __set_halt(struct usb_ep *ep, int value, bool may_fail)
> {
> if (may_fail && ep queue is not empty) {
> return false
> } else {
> do stall;
> return true;
> }
> }
>
> gadget_ops:
> .set_halt = ep_set_halt,
>
> ep_set_halt(struct usb_ep *ep, int value)
> {
> __set_halt(ep, value, true);
> }
>
> And call __set_halt(ep, value, false) at below conditions:
> - SET(CLEAR)_FEATURE for Set(Clear)-Halt
> - If ep0 request has failed
Yes, that should work. In fact, when a control request fails, you
could even call ep_set_halt instead of __set_halt, because the ep0
queue will certainly be empty.
> Do we need to update kernel doc for usb_ep_set_halt that
> say it should only for bulk and interrupt endpoints?
I don't think we need to change it. The USB spec says that isochronous
endpoints never stall; section 5.6.5 says:
An endpoint for isochronous transfers never halts because there
is no handshake to report a halt condition.
Also, Figure 8-39 does not include any STALL packets.
Lastly, usb_ep_set_halt should be called for ep0 when the gadget is
unable to respond to a request.
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
[PATCH v3 1/1] USB:option:add ZTE PIDs
This is intended to add ZTE device PIDs on kernel.
Signed-off-by: Liu.Zhao
---
drivers/usb/serial/option.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 876423b..6b4a766 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -285,6 +285,10 @@ static void option_instat_callback(struct urb *urb);
#define ZTE_PRODUCT_MC2718 0xffe8
#define ZTE_PRODUCT_AD3812 0xffeb
#define ZTE_PRODUCT_MC2716 0xffed
+#define ZTE_PRODUCT_ZM8620_X 0x0396
+#define ZTE_PRODUCT_ME3620_MBIM0x0426
+#define ZTE_PRODUCT_ME3620_X 0x1432
+#define ZTE_PRODUCT_ME3620_L 0x1433
#define BENQ_VENDOR_ID 0x04a5
#define BENQ_PRODUCT_H10 0x4068
@@ -544,6 +548,18 @@ static const struct option_blacklist_info
zte_mc2716_z_blacklist = {
.sendsetup = BIT(1) | BIT(2) | BIT(3),
};
+static const struct option_blacklist_info zte_zm8620_x_blacklist = {
+ .reserved = BIT(3) | BIT(4) | BIT(5),
+};
+
+static const struct option_blacklist_info zte_me3620_xl_blacklist = {
+ .reserved = BIT(3) | BIT(4) | BIT(5),
+};
+
+static const struct option_blacklist_info zte_me3620_mbim_blacklist = {
+ .reserved = BIT(2) | BIT(3) | BIT(4),
+};
+
static const struct option_blacklist_info huawei_cdc12_blacklist = {
.reserved = BIT(1) | BIT(2),
};
@@ -1591,6 +1607,14 @@ static const struct usb_device_id option_ids[] = {
.driver_info = (kernel_ulong_t)&zte_ad3812_z_blacklist },
{ USB_DEVICE_AND_INTERFACE_INFO(ZTE_VENDOR_ID, ZTE_PRODUCT_MC2716,
0xff, 0xff, 0xff),
.driver_info = (kernel_ulong_t)&zte_mc2716_z_blacklist },
+ { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_L),
+.driver_info = (kernel_ulong_t)&zte_me3620_xl_blacklist },
+ { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_X),
+.driver_info = (kernel_ulong_t)&zte_me3620_xl_blacklist },
+ { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ZM8620_X),
+.driver_info = (kernel_ulong_t)&zte_zm8620_x_blacklist },
+ { USB_DEVICE(ZTE_VENDOR_ID, ZTE_PRODUCT_ME3620_MBIM),
+.driver_info = (kernel_ulong_t)&zte_me3620_mbim_blacklist },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x01) },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x02, 0x05) },
{ USB_VENDOR_AND_INTERFACE_INFO(ZTE_VENDOR_ID, 0xff, 0x86, 0x10) },
--
1.9.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
Re: Some restrictions when using usbtest and g_zero
On Mon, 24 Aug 2015, Peter Chen wrote: > On Thu, Aug 20, 2015 at 05:03:46PM -0400, Alan Stern wrote: > > On Thu, 20 Aug 2015, Felipe Balbi wrote: > > > > > > > > - When using pattern = 1 as module parameters to compare the data, > > > > > > the > > > > > > packet size must be same between host and device's. > > > > > > > > > > why ? > > > > > > > > The gadget stores the pattern data starting from 0 for each packet it > > > > sends. But the host tests the pattern data starting from 0 only at the > > > > beginning of the transfer; the host expects the pattern to continue > > > > without resetting at packet boundaries (if the transfer length is > > > > larger than the maxpacket size). > > > > > > then that's another bug which needs to be fixed :-) > > > > Here's my attempt at a fix. Completely untested, but it does compile > > without errors. Peter, do you mind trying this out? > > After adding related changes at gadget side, it works. > > In fact, the gadget stores the pattern data starting from 0 to > transfer length (not the packet length). This is the problem you were originally complaining about -- the pattern test fails if the transfer length (not the packet length) is different on the host and the gadget sides. I think it may make sense to require the transfer length to be the same on both sides. That will mean the pattern gives a more stringent test. With the change we have been discussing, all the packets will contain exactly the same pattern -- that means the test is weaker. Maybe we should add a vendor-specific control request to gadget Zero so that the host can tell the gadget what the transfer size will be. > Besides, you may need to consider high bandwidth ISO transfer, > you can send the formal patch and I will change gadget side > and have a test. I'm not so sure we ought to change this. > Besides, if I use vary < length, the test 4/7/8 have failed > (still not check why) Test 7 should be okay, since it involves OUT transfers. Tests 4 and 8 involve IN transfers, so they will run into trouble unless vary is a multiple of the maxpacket size. Otherwise the host will expect a short packet at the end of some of the transfers, but the gadget will not send any short packets. > Besides, considering let vary equals to length for control > transfer? In that case, we can use one line command for all tests. I don't know. For some tests, it may make sense to require the parameters to have special values, so that the host side will agree with the gadget side. 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: Some restrictions when using usbtest and g_zero
On Mon, Aug 24, 2015 at 10:51:04AM -0400, Alan Stern wrote:
> On Mon, 24 Aug 2015, Peter Chen wrote:
>
> > Thanks, that's much clear.
> >
> > At udc driver:
> >
> > __set_halt(struct usb_ep *ep, int value, bool may_fail)
> > {
> > if (may_fail && ep queue is not empty) {
> > return false
> > } else {
> > do stall;
> > return true;
> > }
> > }
> >
> > gadget_ops:
> > .set_halt = ep_set_halt,
> >
> > ep_set_halt(struct usb_ep *ep, int value)
> > {
> > __set_halt(ep, value, true);
> > }
> >
> > And call __set_halt(ep, value, false) at below conditions:
> > - SET(CLEAR)_FEATURE for Set(Clear)-Halt
> > - If ep0 request has failed
>
> Yes, that should work. In fact, when a control request fails, you
> could even call ep_set_halt instead of __set_halt, because the ep0
> queue will certainly be empty.
but you'll already hold controller's lock. ->set_halt() expects lock to
be held. Since you only know of a failing request from within EP0 IRQ
handler (well, you could defer to a tasklet or workqueue, or whatever),
you'll already hold controller's lock and you'd end up with recursive
locking errors.
--
balbi
signature.asc
Description: Digital signature
Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M
On Sun, 23 Aug 2015, Roland Weber wrote: > Hello Alan, > > bisecting turned out to be much simpler than I was afraid of. > Here's the result: > > 638139eb95d2d241781330a321e88c8dafe46078 is the first bad commit > commit 638139eb95d2d241781330a321e88c8dafe46078 > Author: Petr Mladek > Date: Fri Sep 19 17:32:24 2014 +0200 > > usb: hub: allow to process more usb hub events in parallel > > It seems that only choose_devnum() was not ready to process more hub > events at the same time. > > All should be fine if we take bus->usb_address0_mutex there. It will > make sure that more devnums will not be chosen for the given bus and > the related devices at the same time. > > Signed-off-by: Petr Mladek > Acked-by: Alan Stern > Signed-off-by: Greg Kroah-Hartman > > :04 04 6c3b464b8db2bec572cf7904c0aac317b41c1eec > da3ee40957d0637f17895ae05aad4a6646377b2a Mdrivers Hmmm. I can't see how that commit would cause such a drastic hang. Are you certain you found the right one? That is, if you check out that commit and build a kernel it hangs, but if you check out the parent commit (37ebb54915dc), it doesn't hang? Furthermore, the code changed by that commit doesn't run when you do "lsusb -v". It runs only when the USB stack first starts up or when a new host controller is registered. I just tested lsusb -v for several root hubs on my computer, which is currently running a 4.1.4 kernel. It worked okay. Can you check more carefully? For example, see if reverting that commit makes a difference? 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: Some restrictions when using usbtest and g_zero
On Mon, Aug 24, 2015 at 11:43:21AM -0400, Alan Stern wrote: > On Mon, 24 Aug 2015, Peter Chen wrote: > > > On Thu, Aug 20, 2015 at 05:03:46PM -0400, Alan Stern wrote: > > > On Thu, 20 Aug 2015, Felipe Balbi wrote: > > > > > > > > > > - When using pattern = 1 as module parameters to compare the > > > > > > > data, the > > > > > > > packet size must be same between host and device's. > > > > > > > > > > > > why ? > > > > > > > > > > The gadget stores the pattern data starting from 0 for each packet it > > > > > sends. But the host tests the pattern data starting from 0 only at > > > > > the > > > > > beginning of the transfer; the host expects the pattern to continue > > > > > without resetting at packet boundaries (if the transfer length is > > > > > larger than the maxpacket size). > > > > > > > > then that's another bug which needs to be fixed :-) > > > > > > Here's my attempt at a fix. Completely untested, but it does compile > > > without errors. Peter, do you mind trying this out? > > > > After adding related changes at gadget side, it works. > > > > In fact, the gadget stores the pattern data starting from 0 to > > transfer length (not the packet length). > > This is the problem you were originally complaining about -- the > pattern test fails if the transfer length (not the packet length) is > different on the host and the gadget sides. > > I think it may make sense to require the transfer length to be the same > on both sides. That will mean the pattern gives a more stringent test. > With the change we have been discussing, all the packets will contain > exactly the same pattern -- that means the test is weaker. > > Maybe we should add a vendor-specific control request to gadget Zero so > that the host can tell the gadget what the transfer size will be. I proposed that many months ago and you were against it, remember ? You claimed that there should be no issues with gadget and host not agreeing on transfer size, which actually makes sense, considering we have no idea what the host will send us until it does (except for things like BOT). If we add this vendor control request, we are likely to leave some issues in UDCs making assumptions as to what they should receive next. > > Besides, if I use vary < length, the test 4/7/8 have failed > > (still not check why) > > Test 7 should be okay, since it involves OUT transfers. Tests 4 and 8 > involve IN transfers, so they will run into trouble unless vary is a > multiple of the maxpacket size. Otherwise the host will expect a short > packet at the end of some of the transfers, but the gadget will not > send any short packets. we could add some argument sanity checks to usbtest.ko to make sure arguments make sense. > > Besides, considering let vary equals to length for control > > transfer? In that case, we can use one line command for all tests. > > I don't know. For some tests, it may make sense to require the > parameters to have special values, so that the host side will agree > with the gadget side. but why does the host need to agree with the gadget side ? Let's consider a bogus host, for example, talking to a BOT device. Say CBW states it will read 8192 bytes from device, but host ends up trying to read a different amount of bytes. The BOT device is not expected to simply die, it will either refuse to send more data with a NAK or wait for a further request to move the remaining amount. -- balbi signature.asc Description: Digital signature
USB suspend resume issue on Vybrid (Chipidea IP/MXS PHY)
Hello, I am working on Freescale Vybrid which is a Cortex A5 based SoC, with a Chipidea based USB controller and a Sigmatel Phy or some- thing if my memory serves me right. We are currently in the process of implementing suspend resume and fixing related issues. After resume the USB PHY does not come up and the USB subsytem prints [129.570097] usb 1-1: USB disconnect, device number 2 which comes from the core code in hub.c. I am using the 4.1.5 kernel with some of our own patches especially with regards to suspend resume being present only in our own tree. >From what I can see, the USB USBPHYx_PWDn register which has bits related to power down, all stay in their default "1" state which denotes power down, after resume. Now this in spite of the fact that the code seems [2] to write 0 to the register on resume. However, doing a quick check with devmem2 shows the register to have the default values of "1" denoting power down. So write seems to have no effect at all. Instead of the code at lines 392[1] and 394[2] if I do return mxs_phy_hw_init(mxs_phy); on resume, the USBPHYx_PWDn seems to have the correct value of all bits as zero. However of course, the USB PHY does not come up. The stmp block reset in mxs_phy_hw_init seems to make the write work. There is an errata for Vybrid at [3] in VYBRID_2N02G going as e4535: USB: USB suspend and resume flow clarifications. Not sure if I understand the explanation, however the following workaround which the errata mentions: To place the USB PHY in low power suspend mode, the following sequence should be performed in an atomic operation. (interrupts should be disabled during these three steps) 1. Set the PORTSC1.PHCD bit 2. Set all bits in USBPHY_PWD register 3. Set the USBPHY_CTRL.CLKGATE bit These sequence of steps seem to be correctly followed in the suspend code [4] of Chipidea IP AFAICT. I am not that well versed with USB subsystem code having only worked on it once before for fixing non working USB client on Vybrid [5]. Have tried messing with different register bits in the USB PHY and USB miscellaneous register but with no results. Peter, Felipe do you have any ideas perhaps? From what I can see this seems to be USB PHY issue. Also Peter I wanted to ask you, the following bits MXS_PHY_ABNORMAL_IN_SUSPEND and MXS_PHY_SENDING_SOF_TOO_FAST where are they being used? I can see the MXS_PHY_NEED_IP_FIX being used but not the others. Perhaps I am missing something? It is also surprising that disconnect line and ip fix are needed while the TRM states that writing to bits 17 and 18, which is BM_USBPHY_IP_FIX, will not gaurantee chip functionality if set. However, I also checked this is required else one gets enumeration errors as stated by Stefan in his patch which introduced it. Any inputs are welcomed. Thanks & Regards, Sanchayan. [1]. http://lxr.free-electrons.com/source/drivers/usb/phy/phy-mxs-usb.c#L392 [2]. http://lxr.free-electrons.com/source/drivers/usb/phy/phy-mxs-usb.c#L394 [3]. http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=VF6xx&fpsp=1&tab=Documentation_Tab [4]. http://lxr.free-electrons.com/source/drivers/usb/chipidea/core.c#L891 [5]. https://lkml.org/lkml/2014/12/19/110 -- 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 suspend resume issue on Vybrid (Chipidea IP/MXS PHY)
Hi, On Mon, Aug 24, 2015 at 09:40:04PM +0530, Sanchayan Maity wrote: > Hello, > > I am working on Freescale Vybrid which is a Cortex A5 based SoC, > with a Chipidea based USB controller and a Sigmatel Phy or some- > thing if my memory serves me right. We are currently in the process > of implementing suspend resume and fixing related issues. After > resume the USB PHY does not come up and the USB subsytem prints > > [129.570097] usb 1-1: USB disconnect, device number 2 > > which comes from the core code in hub.c. > > I am using the 4.1.5 kernel with some of our own patches especially > with regards to suspend resume being present only in our own tree. I think you need to send that code if you want to get any help, but in any case, check if by the time you call usb_phy_suspend(phy, 0) (from CI's resume), mxs ->resume() method has already been called. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH
24.08.2015 16:29, Bjørn Mork пишет: Eugene Shatokhin writes: 19.08.2015 15:31, Bjørn Mork пишет: Eugene Shatokhin writes: The problem is not in the reordering but rather in the fact that "dev->flags = 0" is not necessarily atomic w.r.t. "clear_bit(EVENT_RX_KILL, &dev->flags)", and vice versa. So the following might be possible, although unlikely: CPU0 CPU1 clear_bit: read dev->flags clear_bit: clear EVENT_RX_KILL in the read value dev->flags=0; clear_bit: write updated dev->flags As a result, dev->flags may become non-zero again. Ah, right. Thanks for explaining. I cannot prove yet that this is an impossible situation. If anyone can, please explain. If so, this part of the patch will not be needed. I wonder if we could simply move the dev->flags = 0 down a few lines to fix both issues? It doesn't seem to do anything useful except for resetting the flags to a sane initial state after the device is down. Stopping the tasklet rescheduling etc depends only on netif_running(), which will be false when usbnet_stop is called. There is no need to touch dev->flags for this to happen. That was one of the first ideas we discussed here. Unfortunately, it is probably not so simple. Setting dev->flags to 0 makes some delayed operations do nothing and, among other things, not to reschedule usbnet_bh(). Yes, but I believe that is merely a side effect. You should never need to clear multiple flags to get the desired behaviour. As you can see in drivers/net/usb/usbnet.c, usbnet_bh() can be called as a tasklet function and as a timer function in a number of situations (look for the usage of dev->bh and dev->delay there). netif_running() is indeed false when usbnet_stop() runs, usbnet_stop() also disables Tx. This seems to be enough for many cases where usbnet_bh() is scheduled, but I am not so sure about the remaining ones, namely: 1. A work function, usbnet_deferred_kevent(), may reschedule usbnet_bh(). Looks like the workqueue is only stopped in usbnet_disconnect(), so a work item might be processed while usbnet_stop() works. Setting dev->flags to 0 makes the work function do nothing, by the way. See also the comment in usbnet_stop() about this. A work item may be placed to this workqueue in a number of ways, by both usbnet module and the mini-drivers. It is not too easy to track all these situations. That's an understatement :) 2. rx_complete() and tx_complete() may schedule execution of usbnet_bh() as a tasklet or a timer function. These two are URB completion callbacks. It seems, new Rx and Tx URBs cannot be submitted when usbnet_stop() clears dev->flags, indeed. But it does not prevent the completion handlers for the previously submitted URBs from running concurrently with usbnet_stop(). The latter waits for them to complete (via usbnet_terminate_urbs(dev)) but only if FLAG_AVOID_UNLINK_URBS is not set in info->flags. rndis_wlan, however, sets this flag for a few hardware models. So - no guarantees here as well. FLAG_AVOID_UNLINK_URBS looks like it should be replaced by the newer ability to keep the status urb active. I believe that must have been the real reason for adding it, based on the commit message and the effect the flag will have: commit 1487cd5e76337555737cbc55d7d83f41460d198f Author: Jussi Kivilinna Date: Thu Jul 30 19:41:20 2009 +0300 usbnet: allow "minidriver" to prevent urb unlinking on usbnet_stop rndis_wlan devices freeze after running usbnet_stop several times. It appears that firmware freezes in state where it does not respond to any RNDIS commands and device have to be physically unplugged/replugged. This patch lets minidrivers to disable unlink_urbs on usbnet_stop through new info flag. Signed-off-by: Jussi Kivilinna Cc: David Brownell Signed-off-by: John W. Linville The rx urbs will not be resubmitted in any case, and there are of course no tx urbs being submitted. So the only effect of this flag is on the status/interrupt urb, which I can imagine some RNDIS devices wants active all the time. So FLAG_AVOID_UNLINK_URBS should probably be removed and replaced calls to usbnet_status_start() and usbnet_status_stop(). This will require testing on some of the devices with the original firmware problem however. In any case: I do not think this flag should be considered when trying to make usbnet_stop behaviour saner. It's only purpose is to deliberately break usbnet_stop by not actually stopping. If someone could list the particular bits of dev->flags that should be cleared to make sure no deferred call could reschedule usbnet_bh(), etc... Well, it would be enough to clear these first and use dev->flags = 0 later, after tasklet_kill() and del_timer_sync(). I cannot point out these particular bits now. I don't think any of the flags must be cleared. The sequence dev_close(dev->net); usbnet_terminate_urbs(d
Re: [PATCH] usb: phy: msm: Unregister driver interest for VBUS and ID events
On 08/18/2015 12:56 AM, Ivan T. Ivanov wrote:
> Right now even if driver failed to probe extcon framework will
> still deliver its VBUS and ID events, which will lead to random
> exception codes.
>
> Fix this by removing driver interest for VBUS and ID events when
> probe fail.
>
> Fixes: 591fc116f330 ("usb: phy: msm: Use extcon framework for VBUS and ID
> detection")
>
> Reported-by: Tim Bird
> Signed-off-by: Ivan T. Ivanov
> ---
> drivers/usb/phy/phy-msm-usb.c | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> index 00c49bb1bd29..a9082567f114 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -1581,6 +1581,8 @@ static int msm_otg_read_dt(struct platform_device
> *pdev, struct msm_otg *motg)
> ret = extcon_register_interest(&motg->id.conn, ext_id->name,
> "USB-HOST", &motg->id.nb);
> if (ret < 0) {
> + if (!IS_ERR(ext_vbus))
> + extcon_unregister_interest(&motg->vbus.conn);
> dev_err(&pdev->dev, "register ID notifier failed\n");
> return ret;
> }
...
This patch is obsoleted by commit 83b7b67c7, which changes the extcon API
a bit (from register_interest to register_notifier, among other things).
But, in general, I would expect this approach to work.
Do you want me to re-spin this with the new API?
-- Tim
--
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] usbnet: Fix two races between usbnet_stop() and the BH
From: Eugene Shatokhin Date: Wed, 19 Aug 2015 14:59:01 +0300 > So the following might be possible, although unlikely: > > CPU0 CPU1 > clear_bit: read dev->flags > clear_bit: clear EVENT_RX_KILL in the read value > > dev->flags=0; > > clear_bit: write updated dev->flags > > As a result, dev->flags may become non-zero again. Is this really possible? Stores really are "atomic" in the sense that the do their update in one indivisible operation. Atomic operations like clear_bit also will behave that way. If a clear_bit is in progress, the "dev->flags=0" store will not be able to grab the cache line exclusively until the clear_bit is done. So I think the above sequent of events is completely impossible. Once a clear_bit starts, a write by another foreign agent on the bus is absolutely impossible to legally occur until the clear_bit completes. I think this is a non-issue. -- 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] usbnet: Fix two races between usbnet_stop() and the BH
On Mon, 24 Aug 2015, David Miller wrote: > From: Eugene Shatokhin > Date: Wed, 19 Aug 2015 14:59:01 +0300 > > > So the following might be possible, although unlikely: > > > > CPU0 CPU1 > > clear_bit: read dev->flags > > clear_bit: clear EVENT_RX_KILL in the read value > > > > dev->flags=0; > > > > clear_bit: write updated dev->flags > > > > As a result, dev->flags may become non-zero again. > > Is this really possible? > > Stores really are "atomic" in the sense that the do their update > in one indivisible operation. Provided you use ACCESS_ONCE or WRITE_ONCE or whatever people like to call it now. > Atomic operations like clear_bit also will behave that way. Are you certain about that? I couldn't find any mention of it in Documentation/atomic_ops.txt. In theory, an architecture could implement atomic bit operations using a spinlock to insure atomicity. I don't know if any architectures do this, but if they do then the scenario above could arise. 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: Some restrictions when using usbtest and g_zero
On Mon, 24 Aug 2015, Felipe Balbi wrote: > > Maybe we should add a vendor-specific control request to gadget Zero so > > that the host can tell the gadget what the transfer size will be. > > I proposed that many months ago and you were against it, remember ? I believe you -- it sounds like the sort of thing I might say -- but I don't remember it. :-) > You claimed that there should be no issues with gadget and host not > agreeing on transfer size, which actually makes sense, considering we > have no idea what the host will send us until it does (except for things > like BOT). > > If we add this vendor control request, we are likely to leave some > issues in UDCs making assumptions as to what they should receive next. These are good arguments. The only place an issue arises, as far as I can see, is in the pattern=2 data. Where should the pattern reset back to the start? At the beginning of each packet? At the beginning of each transfer? At the beginning of each test? The difficulty is that the host and gadget don't have any common notions about transfers or tests. That leaves only packets (or maybe bursts/multiplets for isochronous). > > > Besides, if I use vary < length, the test 4/7/8 have failed > > > (still not check why) > > > > Test 7 should be okay, since it involves OUT transfers. Tests 4 and 8 > > involve IN transfers, so they will run into trouble unless vary is a > > multiple of the maxpacket size. Otherwise the host will expect a short > > packet at the end of some of the transfers, but the gadget will not > > send any short packets. > > we could add some argument sanity checks to usbtest.ko to make sure > arguments make sense. It might help a little. You'd still get a failure, but the error code would indicate that it was your mistake and not a driver problem. > > > Besides, considering let vary equals to length for control > > > transfer? In that case, we can use one line command for all tests. > > > > I don't know. For some tests, it may make sense to require the > > parameters to have special values, so that the host side will agree > > with the gadget side. > > but why does the host need to agree with the gadget side ? Let's > consider a bogus host, for example, talking to a BOT device. Say CBW > states it will read 8192 bytes from device, but host ends up trying to > read a different amount of bytes. The BOT device is not expected to > simply die, it will either refuse to send more data with a NAK or wait > for a further request to move the remaining amount. Yes, they don't need to agree for a MSC test. But they do need to agree for simple read/write tests with pattern=2. Otherwise the test will fail, because the sender will generate a pattern that the receiver doesn't expect. 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: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH
24.08.2015 20:43, David Miller пишет: From: Eugene Shatokhin Date: Wed, 19 Aug 2015 14:59:01 +0300 So the following might be possible, although unlikely: CPU0 CPU1 clear_bit: read dev->flags clear_bit: clear EVENT_RX_KILL in the read value dev->flags=0; clear_bit: write updated dev->flags As a result, dev->flags may become non-zero again. Is this really possible? On x86, it is not possible, so this is not a problem. Perhaps, for ARM too. As for the other architectures supported by the kernel - not sure, no common guarantees, it seems. Anyway, this is not a critical issue, I agree. OK, let us leave things as they are for this one and fix the rest. Stores really are "atomic" in the sense that the do their update in one indivisible operation. Atomic operations like clear_bit also will behave that way. If a clear_bit is in progress, the "dev->flags=0" store will not be able to grab the cache line exclusively until the clear_bit is done. So I think the above sequent of events is completely impossible. Once a clear_bit starts, a write by another foreign agent on the bus is absolutely impossible to legally occur until the clear_bit completes. I think this is a non-issue. Regards, Eugene -- 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] usbnet: Fix two races between usbnet_stop() and the BH
On Mon, 24 Aug 2015, Alan Stern wrote: > On Mon, 24 Aug 2015, David Miller wrote: > > > From: Eugene Shatokhin > > Date: Wed, 19 Aug 2015 14:59:01 +0300 > > > > > So the following might be possible, although unlikely: > > > > > > CPU0 CPU1 > > > clear_bit: read dev->flags > > > clear_bit: clear EVENT_RX_KILL in the read value > > > > > > dev->flags=0; > > > > > > clear_bit: write updated dev->flags > > > > > > As a result, dev->flags may become non-zero again. > > > > Is this really possible? > > > > Stores really are "atomic" in the sense that the do their update > > in one indivisible operation. > > Provided you use ACCESS_ONCE or WRITE_ONCE or whatever people like to > call it now. > > > Atomic operations like clear_bit also will behave that way. > > Are you certain about that? I couldn't find any mention of it in > Documentation/atomic_ops.txt. > > In theory, an architecture could implement atomic bit operations using > a spinlock to insure atomicity. I don't know if any architectures do > this, but if they do then the scenario above could arise. Now that I see this in writing, I realize it's not possible after all. clear_bit() et al. will work with a single unsigned long, which doesn't leave any place for spinlocks or other mechanisms. I was thinking of atomic_t. So never mind... 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: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH
From: Alan Stern Date: Mon, 24 Aug 2015 14:06:15 -0400 (EDT) > On Mon, 24 Aug 2015, David Miller wrote: >> Atomic operations like clear_bit also will behave that way. > > Are you certain about that? I couldn't find any mention of it in > Documentation/atomic_ops.txt. > > In theory, an architecture could implement atomic bit operations using > a spinlock to insure atomicity. I don't know if any architectures do > this, but if they do then the scenario above could arise. Indeed, we do have platforms like 32-bit sparc and parisc that do this. So, taking that into consideration, this is a bit unfortunate and on such platforms we do have this problem. -- 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: v4.1.4 xhci: 50%~66% chance of "HC died; cleaning up" on boot
On Mon, 24 Aug 2015 10:15:52 +, David Laight
wrote:
> Probably worth determining which xhci controller hardware is being used.
I believe it is part of the CPU:
http://ark.intel.com/products/78866/Intel-Celeron-Processor-J1800-1M-Cache-up-to-2_58-GHz
(from the datasheet link, pardon the "(c)" avalanche)
Intel® Pentium® processor N3500-series, J2850, J2900, and Intel®
Celeron® processor N2900-series, N2800-series, J1800-series, J1750
are Intel Architecture (IA) processors that integrate with the next
generation Intel® processor core, Graphics, Memory Controller, and
I/O interfaces into a single system-on-chip solution.
On page 57 (figure 6, Bus 0 PCI devices and functions), the xHCI
address matches the one in lspci & error messages (I'm discovering a
lot of stuff as I read that doc).
In any case, verbose ls{pci,usb} output:
$ sudo lspci -vvvs 00:14.0
00:14.0 USB controller: Intel Corporation Atom Processor Z36xxx/Z37xxx Series
USB xHCI (rev 0e) (prog-if 30 [XHCI])
Subsystem: Intel Corporation Atom Processor Z36xxx/Z37xxx Series USB
xHCI
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
SERR- http://vger.kernel.org/majordomo-info.html
Re: v4.1.4 xhci: 50%~66% chance of "HC died; cleaning up" on boot
On Mon, 24 Aug 2015 12:48:13 +0300, Mathias Nyman
wrote:
> Do the usb devices work normally after a successful boot?
> (no suspicious xhci entries in dmesg)
There are indeed some suspicious messages in my current dmesg:
$ dmesg | grep "[xeou]hci"
[2.115002] xhci_hcd :00:14.0: xHCI Host Controller
[2.115018] xhci_hcd :00:14.0: new USB bus registered, assigned bus
number 1
[2.115157] xhci_hcd :00:14.0: hcc params 0x200077c1 hci version 0x100
quirks 0x9810
[2.115165] xhci_hcd :00:14.0: cache line size of 64 is not supported
[2.115477] usb usb1: Manufacturer: Linux 4.1.4+ xhci-hcd
[2.116734] xhci_hcd :00:14.0: xHCI Host Controller
[2.116744] xhci_hcd :00:14.0: new USB bus registered, assigned bus
number 2
[2.116826] usb usb2: Manufacturer: Linux 4.1.4+ xhci-hcd
[2.426642] usb 1-1: new high-speed USB device number 2 using xhci_hcd
[7.337807] xhci_hcd :00:14.0: Command completion event does not match
command
[7.337808] xhci_hcd :00:14.0: Timeout while waiting for setup device
command
[ 12.349069] xhci_hcd :00:14.0: Timeout while waiting for setup device
command
[ 17.360340] xhci_hcd :00:14.0: Timeout while waiting for setup device
command
[ 17.472401] usb 1-1: new high-speed USB device number 3 using xhci_hcd
[ 17.772584] usb 1-4: new high-speed USB device number 4 using xhci_hcd
[ 17.856780] usb 2-1: new SuperSpeed USB device number 4 using xhci_hcd
(completion mismatch & timeout)
"HC died" is not present.
> Add xhci boot debugging to the kernel cmdline:
>
> dyndbg='module xhci_hcd +p'
>
> or, if as a separate module:
>
> xhci_hcd.dyndbg=+p
Thanks, added.
In an attempt to get the failed log still stored to disk, I've modified
my fstab to not mount /boot until accessed: on the mass storage device
there is only /boot, the rest is on a sata hdd[1]. This lowered the bug
probability (maybe by reducing USB load early on ?). I also worked on
getting rid of an apparently unrelated boot annoyance (it turned out
uswsusp locking hard during initrd and requiring me to pass "resume="
parameter on each boot).
After about 10 reboots/cold boots, re-enabling /boot "auto", I
triggered the error again, with given switch. AFAIKS there is a single
extra line (still using "quiet" to avoid loosing output, and typing
from screenshot):
[ 7.34] xhci_hcd :00:14.0: Command completion event does not match command
[12.36] usb 1-1: hub failed to enable device, error -62
[24.99] xhci_hcd :00:14.0: Stopped the command ring railed, maybe the host
is dead
[24.99] xhci_hcd :00:14.0: Abort command ring failed
[24.99] xhci_hcd :00:14.0: HC died; cleaning up
[25.00] usb usb1-port1: couldn't allocate usb_device
[25.20] usb 2-1: device not accepting address 2, error -62
[25.20] usb usb2-port1: couldn't allocate usb_device
[25.20] pci_pm_runtime_suspend(): hcd_pci_runtime_suspend+0x0/0x60 [usbcore]
returns -16
I do have custom udev rules for power management (topmost rules):
$ cat /etc/udev/rules.d/local.rules
# From https://wiki.archlinux.org/index.php/Power_management
KERNELS==":03:00.0", GOTO="nopowercontrol"
ACTION=="add|change", SUBSYSTEM=="pci", ATTR{power/control}="auto"
LABEL="nopowercontrol"
ACTION=="add|change", SUBSYSTEM=="usb", TEST=="power/control",
ATTR{power/control}="auto"
ACTION=="add|change", SUBSYSTEM=="scsi_host", KERNEL=="host*",
ATTR{link_power_management_policy}="min_power"
# Built-in USB drive is not rotational, but kernel thinks it is.
ACTION=="add", SUBSYSTEM=="block", KERNEL=="sd?", ENV{ID_MODEL_ID}=="b155",
ENV{ID_VENDOR_ID}=="1005", ATTR{queue/rotational}="0"
# Slot-stable symlinks
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="4:0:0:0",
SYMLINK+="qnap/hdd1"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="3:0:0:0",
SYMLINK+="qnap/hdd2"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="2:0:0:0",
SYMLINK+="qnap/hdd3"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="8:0:0:0",
SYMLINK+="qnap/hdd4"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="7:0:0:0",
SYMLINK+="qnap/hdd5"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="6:0:0:0",
SYMLINK+="qnap/hdd6"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="1-4",
SYMLINK+="qnap/builtin_usb"
ACTION=="add|change", SUBSYSTEM=="block", KERNEL=="sd?", KERNELS=="2-1.3",
SYMLINK+="qnap/front_usb"
# Fan speed
ACTION=="add|change", KERNEL=="f71882fg.656",
ATTR{pwm1_auto_point4_pwm}="100",ATTR{pwm1_auto_point4_temp}="35000",
ATTR{pwm1_auto_point5_pwm}="30", ATTR{pwm1_auto_channels_temp}="2",
ATTR{pwm2_auto_point4_pwm}="100", ATTR{pwm2_auto_point4_temp}="35000",
ATTR{pwm2_auto_point5_pwm}="30", ATTR{pwm2_auto_channels_temp}="1",
ATTR{pwm3_enable}="1", ATTR{pwm3}="0"
FWIW, I disabled power sving on :03:00.0 which contains the two Gb
ethernet adapters because it caused issues - issues which are about
next in m
[PATCH 2/2] usbnet: Fix a race between usbnet_stop() and the BH
The race may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.
Hardware breakpoints and Kprobes with delays were used to confirm that
the race does actually happen.
The race is on skb_queue ('next' pointer) between usbnet_stop()
and rx_complete(), which, in turn, calls usbnet_bh().
Here is a part of the call stack with the code where the changes to the
queue happen. The line numbers are for the kernel 4.1.0:
*0 __skb_unlink (skbuff.h:1517)
prev->next = next;
*1 defer_bh (usbnet.c:430)
spin_lock_irqsave(&list->lock, flags);
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
spin_unlock(&list->lock);
spin_lock(&dev->done.lock);
__skb_queue_tail(&dev->done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
spin_unlock_irqrestore(&dev->done.lock, flags);
*2 rx_complete (usbnet.c:640)
state = defer_bh(dev, skb, &dev->rxq, state);
At the same time, the following code repeatedly checks if the queue is
empty and reads these values concurrently with the above changes:
*0 usbnet_terminate_urbs (usbnet.c:765)
/* maybe wait for deletions to finish. */
while (!skb_queue_empty(&dev->rxq)
&& !skb_queue_empty(&dev->txq)
&& !skb_queue_empty(&dev->done)) {
schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
set_current_state(TASK_UNINTERRUPTIBLE);
netif_dbg(dev, ifdown, dev->net,
"waited for %d urb completions\n", temp);
}
*1 usbnet_stop (usbnet.c:806)
if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
usbnet_terminate_urbs(dev);
As a result, it is possible, for example, that the skb is removed from
dev->rxq by __skb_unlink() before the check
"!skb_queue_empty(&dev->rxq)" in usbnet_terminate_urbs() is made. It is
also possible in this case that the skb is added to dev->done queue
after "!skb_queue_empty(&dev->done)" is checked. So
usbnet_terminate_urbs() may stop waiting and return while dev->done
queue still has an item.
Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
this race.
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb/usbnet.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e049857..b4cf107 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -428,12 +428,18 @@ static enum skb_state defer_bh(struct usbnet *dev, struct
sk_buff *skb,
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
- spin_unlock(&list->lock);
- spin_lock(&dev->done.lock);
+
+ /* defer_bh() is never called with list == &dev->done.
+* spin_lock_nested() tells lockdep that it is OK to take
+* dev->done.lock here with list->lock held.
+*/
+ spin_lock_nested(&dev->done.lock, SINGLE_DEPTH_NESTING);
+
__skb_queue_tail(&dev->done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
- spin_unlock_irqrestore(&dev->done.lock, flags);
+ spin_unlock(&dev->done.lock);
+ spin_unlock_irqrestore(&list->lock, flags);
return old_state;
}
@@ -749,6 +755,20 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
/*-*/
+static void wait_skb_queue_empty(struct sk_buff_head *q)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&q->lock, flags);
+ while (!skb_queue_empty(q)) {
+ spin_unlock_irqrestore(&q->lock, flags);
+ schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ spin_lock_irqsave(&q->lock, flags);
+ }
+ spin_unlock_irqrestore(&q->lock, flags);
+}
+
// precondition: never called in_interrupt
static void usbnet_terminate_urbs(struct usbnet *dev)
{
@@ -762,14 +782,11 @@ static void usbnet_terminate_urbs(struct usbnet *dev)
unlink_urbs(dev, &dev->rxq);
/* maybe wait for deletions to finish. */
- while (!skb_queue_empty(&dev->rxq)
- && !skb_queue_empty(&dev->txq)
- && !skb_queue_empty(&dev->done)) {
- schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
- set_current_state(TASK_UNINTERRUPTIBLE);
- netif_dbg(dev, ifdown, dev->net,
- "waited for %d urb completions\n", temp);
- }
+ wait_skb_queue_empty(&dev->rxq);
+ wait_skb_queue_empty(&dev->txq);
+ wait_skb_queue_empty(&dev->done);
+ netif_dbg(dev, ifdown, dev->net,
+ "waited for %d urb completions\n", temp);
set_current_state(TASK_RUNNING);
remove_wait_queue(&dev->wait, &wait);
}
--
2.3.2
--
To unsubscribe from this list: sen
[PATCH 1/2] usbnet: Get EVENT_NO_RUNTIME_PM bit before it is cleared
It is needed to check EVENT_NO_RUNTIME_PM bit of dev->flags in
usbnet_stop(), but its value should be read before it is cleared
when dev->flags is set to 0.
The problem was spotted and the fix was provided by
Oliver Neukum .
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb/usbnet.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 3c86b10..e049857 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -778,7 +778,7 @@ int usbnet_stop (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
struct driver_info *info = dev->driver_info;
- int retval, pm;
+ int retval, pm, mpn;
clear_bit(EVENT_DEV_OPEN, &dev->flags);
netif_stop_queue (net);
@@ -809,6 +809,8 @@ int usbnet_stop (struct net_device *net)
usbnet_purge_paused_rxq(dev);
+ mpn = !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags);
+
/* deferred work (task, timer, softirq) must also stop.
* can't flush_scheduled_work() until we drop rtnl (later),
* else workers could deadlock; so make workers a NOP.
@@ -819,8 +821,7 @@ int usbnet_stop (struct net_device *net)
if (!pm)
usb_autopm_put_interface(dev->intf);
- if (info->manage_power &&
- !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags))
+ if (info->manage_power && mpn)
info->manage_power(dev, 0);
else
usb_autopm_put_interface(dev->intf);
--
2.3.2
--
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/2] usbnet: Fix 2 problems in usbnet_stop()
The following problems found when investigating races in usbnet module
are fixed here:
1. EVENT_NO_RUNTIME_PM bit of dev->flags should be read before it is
cleared by "dev->flags = 0". Thanks to Oliver Neukum for spotting this
problem and providing a fix.
2. A race on on skb_queue between usbnet_stop() and usbnet_bh().
Compared to the combined patch I sent earlier
("[PATCH] usbnet: Fix two races between usbnet_stop() and the BH"), this
patch set has the following changes:
* The fix for handling of EVENT_NO_RUNTIME_PM is now in a separate patch.
* The fix for the race on dev->flags has been removed because the race is
not considered harmful.
Regards,
Eugene
--
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: musb: fix cppi channel teardown for isoch transfer
After a few iterations of start/stop UVC camera streaming, the streaming
stops.
This patch adds 250us delay in the cppi channel abort path to let cppi
drain properly.
Using 50us delay seems to be too aggressive, some webcams are still
broken. 250us is the original value used in TI 3.2 kernel.
Signed-off-by: Bin Liu
---
drivers/usb/musb/musb_cppi41.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 8bd8c5e..e50ce02 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -551,6 +551,9 @@ static int cppi41_dma_channel_abort(struct dma_channel
*channel)
} else {
cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
+ /* delay to drain to cppi dma pipeline for isoch */
+ udelay(250);
+
csr = musb_readw(epio, MUSB_RXCSR);
csr &= ~(MUSB_RXCSR_H_REQPKT | MUSB_RXCSR_DMAENAB);
musb_writew(epio, MUSB_RXCSR, csr);
--
1.8.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 2/2] usbnet: Fix a race between usbnet_stop() and the BH
Eugene Shatokhin writes:
> The race may happen when a device (e.g. YOTA 4G LTE Modem) is
> unplugged while the system is downloading a large file from the Net.
>
> Hardware breakpoints and Kprobes with delays were used to confirm that
> the race does actually happen.
>
> The race is on skb_queue ('next' pointer) between usbnet_stop()
> and rx_complete(), which, in turn, calls usbnet_bh().
>
> Here is a part of the call stack with the code where the changes to the
> queue happen. The line numbers are for the kernel 4.1.0:
>
> *0 __skb_unlink (skbuff.h:1517)
> prev->next = next;
> *1 defer_bh (usbnet.c:430)
> spin_lock_irqsave(&list->lock, flags);
> old_state = entry->state;
> entry->state = state;
> __skb_unlink(skb, list);
> spin_unlock(&list->lock);
> spin_lock(&dev->done.lock);
> __skb_queue_tail(&dev->done, skb);
> if (dev->done.qlen == 1)
> tasklet_schedule(&dev->bh);
> spin_unlock_irqrestore(&dev->done.lock, flags);
> *2 rx_complete (usbnet.c:640)
> state = defer_bh(dev, skb, &dev->rxq, state);
>
> At the same time, the following code repeatedly checks if the queue is
> empty and reads these values concurrently with the above changes:
>
> *0 usbnet_terminate_urbs (usbnet.c:765)
> /* maybe wait for deletions to finish. */
> while (!skb_queue_empty(&dev->rxq)
> && !skb_queue_empty(&dev->txq)
> && !skb_queue_empty(&dev->done)) {
> schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
> set_current_state(TASK_UNINTERRUPTIBLE);
> netif_dbg(dev, ifdown, dev->net,
> "waited for %d urb completions\n", temp);
> }
> *1 usbnet_stop (usbnet.c:806)
> if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
> usbnet_terminate_urbs(dev);
>
> As a result, it is possible, for example, that the skb is removed from
> dev->rxq by __skb_unlink() before the check
> "!skb_queue_empty(&dev->rxq)" in usbnet_terminate_urbs() is made. It is
> also possible in this case that the skb is added to dev->done queue
> after "!skb_queue_empty(&dev->done)" is checked. So
> usbnet_terminate_urbs() may stop waiting and return while dev->done
> queue still has an item.
Exactly what problem will that result in? The tasklet_kill() will wait
for the processing of the single element done queue, and everything will
be fine. Or?
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
Re: [PATCH v9 1/7] usb: interface authorization: Declare authorized attribute
Am Montag, den 17.08.2015, 09:34 -0700 schrieb Greg KH: > On Sat, Aug 08, 2015 at 11:32:50AM +0200, Stefan Koch wrote: > > The attribute authorized shows the authorization state for an interface. > > > > Signed-off-by: Stefan Koch > > This email bounces, so I have to rip this series out of the tree :( > > Please resend this with a working email for a signed-off-by line. > > greg k-h OK, I will resend the patch and include the doc corrections from Sergei. Thanks for the help Stefan -- 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 v10 0/7] usb: interface authorization
This patch introduces an interface authorization for USB devices. The kernel supports a device authorization because of wireless USB. But the new interface authorization allows to authorize or deauthorize individual interfaces instead authorization or deauthorize a whole device. Therefore the authorized attribute is introduced for each interface. Each patch depends on all patches with a lesser number. v5 was acked-by Alan Stern: http://comments.gmane.org/gmane.linux.usb.general/127144 http://permalink.gmane.org/gmane.linux.usb.general/127151 Changes since v9: - Implemented suggestions from Greg K-H and Sergei Shtylyov Changes since v8: - Implemented suggestions from Greg K-H (number and doc issue) Changes since v7: - Implemented suggestions from Alan Stern and Sergei Shtylyov Changes since v6: - Implemented suggestions from Alan Stern and Sergei Shtylyov Changes since v5: - Implemented suggestions from Greg K-H - Changed device authorization to save the default bit in the same flag as the interface authorization does this (recommended by Alan Stern http://permalink.gmane.org/gmane.linux.usb.general/127086) Stefan Koch (7): usb: interface authorization: Declare authorized attribute usb: interface authorization: Introduces the default interface authorization usb: interface authorization: Control interface probing and claiming usb: interface authorization: Introduces the USB interface authorization usb: interface authorization: SysFS part of USB interface authorization usb: interface authorization: Documentation part usb: interface authorization: Use a flag for the default device authorization Documentation/ABI/testing/sysfs-bus-usb | 20 + Documentation/usb/authorization.txt | 31 + drivers/usb/core/driver.c | 8 drivers/usb/core/hcd.c | 78 - drivers/usb/core/message.c | 39 + drivers/usb/core/sysfs.c| 36 +++ drivers/usb/core/usb.c | 2 +- drivers/usb/core/usb.h | 2 + include/linux/usb.h | 1 + include/linux/usb/hcd.h | 25 --- 10 files changed, 224 insertions(+), 18 deletions(-) -- 2.1.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 v10 5/7] usb: interface authorization: SysFS part of USB interface authorization
This introduces an attribute for each interface to
authorize (1) or deauthorize (0) it:
/sys/bus/usb/devices/INTERFACE/authorized
Signed-off-by: Stefan Koch
---
drivers/usb/core/sysfs.c | 36
1 file changed, 36 insertions(+)
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d269738..3ddaada 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -926,6 +926,41 @@ static ssize_t supports_autosuspend_show(struct device
*dev,
}
static DEVICE_ATTR_RO(supports_autosuspend);
+/*
+ * interface_authorized_show - show authorization status of an USB interface
+ * 1 is authorized, 0 is deauthorized
+ */
+static ssize_t interface_authorized_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct usb_interface *intf = to_usb_interface(dev);
+
+ return sprintf(buf, "%u\n", intf->authorized);
+}
+
+/*
+ * interface_authorized_store - authorize or deauthorize an USB interface
+ */
+static ssize_t interface_authorized_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_interface *intf = to_usb_interface(dev);
+ bool val;
+
+ if (strtobool(buf, &val) != 0)
+ return -EINVAL;
+
+ if (val)
+ usb_authorize_interface(intf);
+ else
+ usb_deauthorize_interface(intf);
+
+ return count;
+}
+static struct device_attribute dev_attr_interface_authorized =
+ __ATTR(authorized, S_IRUGO | S_IWUSR,
+ interface_authorized_show, interface_authorized_store);
+
static struct attribute *intf_attrs[] = {
&dev_attr_bInterfaceNumber.attr,
&dev_attr_bAlternateSetting.attr,
@@ -935,6 +970,7 @@ static struct attribute *intf_attrs[] = {
&dev_attr_bInterfaceProtocol.attr,
&dev_attr_modalias.attr,
&dev_attr_supports_autosuspend.attr,
+ &dev_attr_interface_authorized.attr,
NULL,
};
static struct attribute_group intf_attr_grp = {
--
2.1.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 v10 6/7] usb: interface authorization: Documentation part
This part adds the documentation for the interface authorization. Signed-off-by: Stefan Koch --- Documentation/ABI/testing/sysfs-bus-usb | 20 Documentation/usb/authorization.txt | 31 +++ 2 files changed, 51 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index e5cc763..b5690d3 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -1,3 +1,23 @@ +What: /sys/bus/usb/devices/INTERFACE/authorized +Date: August 2015 +Description: + This allows to authorize (1) or deauthorize (0) + individual interfaces instead a whole device + in contrast to the device authorization. + If a deauthorized interface will be authorized + so the driver probing must be triggered manually + by writing INTERFACE to /sys/bus/usb/drivers_probe + This allows to avoid side-effects with drivers + that need multiple interfaces. + A deauthorized interface cannot be probed or claimed. + +What: /sys/bus/usb/devices/usbX/interface_authorized_default +Date: August 2015 +Description: + This is used as value that determines if interfaces + would be authorized by default. + The value can be 1 or 0. It's by default 1. + What: /sys/bus/usb/device/.../authorized Date: July 2008 KernelVersion: 2.6.26 diff --git a/Documentation/usb/authorization.txt b/Documentation/usb/authorization.txt index c069b68..c7e985f 100644 --- a/Documentation/usb/authorization.txt +++ b/Documentation/usb/authorization.txt @@ -90,3 +90,34 @@ etc, but you get the idea. Anybody with access to a device gadget kit can fake descriptors and device info. Don't trust that. You are welcome. + +Interface authorization +--- +There is a similar approach to allow or deny specific USB interfaces. +That allows to block only a subset of an USB device. + +Authorize an interface: +$ echo 1 > /sys/bus/usb/devices/INTERFACE/authorized + +Deauthorize an interface: +$ echo 0 > /sys/bus/usb/devices/INTERFACE/authorized + +The default value for new interfaces +on a particular USB bus can be changed, too. + +Allow interfaces per default: +$ echo 1 > /sys/bus/usb/devices/usbX/interface_authorized_default + +Deny interfaces per default: +$ echo 0 > /sys/bus/usb/devices/usbX/interface_authorized_default + +Per default the interface_authorized_default bit is 1. +So all interfaces would authorized per default. + +Note: +If a deauthorized interface will be authorized so the driver probing must +be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe + +For drivers that need multiple interfaces all needed interfaces should be +authroized first. After that the drivers should be probed. +This avoids side effects. -- 2.1.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 v10 1/7] usb: interface authorization: Declare authorized attribute
The attribute authorized shows the authorization state for an interface.
Signed-off-by: Stefan Koch
---
include/linux/usb.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447fe29..3deccab 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -178,6 +178,7 @@ struct usb_interface {
unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */
unsigned needs_binding:1; /* needs delayed unbind/rebind */
unsigned resetting_device:1;/* true: bandwidth alloc after reset */
+ unsigned authorized:1; /* used for interface authorization */
struct device dev; /* interface specific device info */
struct device *usb_dev;
--
2.1.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 v10 7/7] usb: interface authorization: Use a flag for the default device authorization
With this patch a flag instead of a variable
is used for the default device authorization.
Signed-off-by: Stefan Koch
---
drivers/usb/core/hcd.c | 31 +--
drivers/usb/core/usb.c | 2 +-
include/linux/usb/hcd.h | 16 +---
3 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 84b5923..a567647 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -854,10 +854,10 @@ static ssize_t authorized_default_show(struct device *dev,
{
struct usb_device *rh_usb_dev = to_usb_device(dev);
struct usb_bus *usb_bus = rh_usb_dev->bus;
- struct usb_hcd *usb_hcd;
+ struct usb_hcd *hcd;
- usb_hcd = bus_to_hcd(usb_bus);
- return snprintf(buf, PAGE_SIZE, "%u\n", usb_hcd->authorized_default);
+ hcd = bus_to_hcd(usb_bus);
+ return snprintf(buf, PAGE_SIZE, "%u\n", !!HCD_DEV_AUTHORIZED(hcd));
}
static ssize_t authorized_default_store(struct device *dev,
@@ -868,12 +868,16 @@ static ssize_t authorized_default_store(struct device
*dev,
unsigned val;
struct usb_device *rh_usb_dev = to_usb_device(dev);
struct usb_bus *usb_bus = rh_usb_dev->bus;
- struct usb_hcd *usb_hcd;
+ struct usb_hcd *hcd;
- usb_hcd = bus_to_hcd(usb_bus);
+ hcd = bus_to_hcd(usb_bus);
result = sscanf(buf, "%u\n", &val);
if (result == 1) {
- usb_hcd->authorized_default = val ? 1 : 0;
+ if (val)
+ set_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
+ else
+ clear_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
+
result = size;
} else {
result = -EINVAL;
@@ -2720,10 +2724,17 @@ int usb_add_hcd(struct usb_hcd *hcd,
dev_info(hcd->self.controller, "%s\n", hcd->product_desc);
/* Keep old behaviour if authorized_default is not in [0, 1]. */
- if (authorized_default < 0 || authorized_default > 1)
- hcd->authorized_default = hcd->wireless ? 0 : 1;
- else
- hcd->authorized_default = authorized_default;
+ if (authorized_default < 0 || authorized_default > 1) {
+ if (hcd->wireless)
+ clear_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
+ else
+ set_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
+ } else {
+ if (authorized_default)
+ set_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
+ else
+ clear_bit(HCD_FLAG_DEV_AUTHORIZED, &hcd->flags);
+ }
set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
/* per default all interfaces are authorized */
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8d5b2f4..f8bbd0b 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -510,7 +510,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
if (root_hub) /* Root hub always ok [and always wired] */
dev->authorized = 1;
else {
- dev->authorized = usb_hcd->authorized_default;
+ dev->authorized = !!HCD_DEV_AUTHORIZED(usb_hcd);
dev->wusb = usb_bus_is_wusb(bus) ? 1 : 0;
}
return dev;
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index e56c6b2..09a51a4 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -58,12 +58,6 @@
*
* Since "struct usb_bus" is so thin, you can't share much code in it.
* This framework is a layer over that, and should be more sharable.
- *
- * @authorized_default: Specifies if new devices are authorized to
- * connect by default or they require explicit
- * user space authorization; this bit is settable
- * through /sys/class/usb_host/X/authorized_default.
- * For the rest is RO, so we don't lock to r/w it.
*/
/*-*/
@@ -121,6 +115,7 @@ struct usb_hcd {
#define HCD_FLAG_RH_RUNNING5 /* root hub is running? */
#define HCD_FLAG_DEAD 6 /* controller has died? */
#define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */
+#define HCD_FLAG_DEV_AUTHORIZED8 /* authorize devices? */
/* The flags can be tested using these macros; they are likely to
* be slightly faster than test_bit().
@@ -140,6 +135,14 @@ struct usb_hcd {
#define HCD_INTF_AUTHORIZED(hcd) \
((hcd)->flags & (1U << HCD_FLAG_INTF_AUTHORIZED))
+ /*
+* Specifies if devices are authorized by default
+* or they require explicit user space authorization; this bit is
+* settable through /sys/class/usb_host/X/authorized_default
+*/
+#define HCD_DEV_AUTHORIZED(hcd) \
+ ((hcd)->f
[PATCH v10 2/7] usb: interface authorization: Introduces the default interface authorization
Interfaces are allowed per default.
This can disabled or enabled (again) by writing 0 or 1 to
/sys/bus/usb/devices/usbX/interface_authorized_default
Signed-off-by: Stefan Koch
---
drivers/usb/core/hcd.c | 47 ++
drivers/usb/core/message.c | 1 +
include/linux/usb/hcd.h| 9 +
3 files changed, 57 insertions(+)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index cbcd092..84b5923 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -882,9 +882,53 @@ static ssize_t authorized_default_store(struct device *dev,
}
static DEVICE_ATTR_RW(authorized_default);
+/*
+ * interface_authorized_default_show - show default authorization status
+ * for USB interfaces
+ *
+ * note: interface_authorized_default is the default value
+ * for initializing the authorized attribute of interfaces
+ */
+static ssize_t interface_authorized_default_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct usb_device *usb_dev = to_usb_device(dev);
+ struct usb_hcd *hcd = bus_to_hcd(usb_dev->bus);
+
+ return sprintf(buf, "%u\n", !!HCD_INTF_AUTHORIZED(hcd));
+}
+
+/*
+ * interface_authorized_default_store - store default authorization status
+ * for USB interfaces
+ *
+ * note: interface_authorized_default is the default value
+ * for initializing the authorized attribute of interfaces
+ */
+static ssize_t interface_authorized_default_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usb_dev = to_usb_device(dev);
+ struct usb_hcd *hcd = bus_to_hcd(usb_dev->bus);
+ int rc = count;
+ bool val;
+
+ if (strtobool(buf, &val) != 0)
+ return -EINVAL;
+
+ if (val)
+ set_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
+ else
+ clear_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
+
+ return rc;
+}
+static DEVICE_ATTR_RW(interface_authorized_default);
+
/* Group all the USB bus attributes */
static struct attribute *usb_bus_attrs[] = {
&dev_attr_authorized_default.attr,
+ &dev_attr_interface_authorized_default.attr,
NULL,
};
@@ -2682,6 +2726,9 @@ int usb_add_hcd(struct usb_hcd *hcd,
hcd->authorized_default = authorized_default;
set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+ /* per default all interfaces are authorized */
+ set_bit(HCD_FLAG_INTF_AUTHORIZED, &hcd->flags);
+
/* HC is in reset state, but accessible. Now do the one-time init,
* bottom up so that hcds can customize the root hubs before hub_wq
* starts talking to them. (Note, bus id is assigned early too.)
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f368d20..3d25d89 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1807,6 +1807,7 @@ free_interfaces:
intfc = cp->intf_cache[i];
intf->altsetting = intfc->altsetting;
intf->num_altsetting = intfc->num_altsetting;
+ intf->authorized = !!HCD_INTF_AUTHORIZED(hcd);
kref_get(&intfc->ref);
alt = usb_altnum_to_altsetting(intf, 0);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..e56c6b2 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -120,6 +120,7 @@ struct usb_hcd {
#define HCD_FLAG_WAKEUP_PENDING4 /* root hub is
resuming? */
#define HCD_FLAG_RH_RUNNING5 /* root hub is running? */
#define HCD_FLAG_DEAD 6 /* controller has died? */
+#define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */
/* The flags can be tested using these macros; they are likely to
* be slightly faster than test_bit().
@@ -131,6 +132,14 @@ struct usb_hcd {
#define HCD_RH_RUNNING(hcd)((hcd)->flags & (1U << HCD_FLAG_RH_RUNNING))
#define HCD_DEAD(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEAD))
+ /*
+* Specifies if interfaces are authorized by default
+* or they require explicit user space authorization; this bit is
+* settable through /sys/class/usb_host/X/interface_authorized_default
+*/
+#define HCD_INTF_AUTHORIZED(hcd) \
+ ((hcd)->flags & (1U << HCD_FLAG_INTF_AUTHORIZED))
+
/* Flags that get set only during HCD registration or removal. */
unsignedrh_registered:1;/* is root hub registered? */
unsignedrh_pollable:1; /* may we poll the root hub? */
--
2.1.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 v10 4/7] usb: interface authorization: Introduces the USB interface authorization
The kernel supports the device authorization because of wireless USB.
These is usable for wired USB devices, too.
These new interface authorization allows to enable or disable
individual interfaces instead a whole device.
If a deauthorized interface will be authorized so the driver probing must
be triggered manually by writing INTERFACE to /sys/bus/usb/drivers_probe
Signed-off-by: Stefan Koch
---
drivers/usb/core/message.c | 38 ++
drivers/usb/core/usb.h | 2 ++
2 files changed, 40 insertions(+)
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 3d25d89..c090f50 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1555,6 +1555,44 @@ static void usb_release_interface(struct device *dev)
kfree(intf);
}
+/*
+ * usb_deauthorize_interface - deauthorize an USB interface
+ *
+ * @intf: USB interface structure
+ */
+void usb_deauthorize_interface(struct usb_interface *intf)
+{
+ struct device *dev = &intf->dev;
+
+ device_lock(dev->parent);
+
+ if (intf->authorized) {
+ device_lock(dev);
+ intf->authorized = 0;
+ device_unlock(dev);
+
+ usb_forced_unbind_intf(intf);
+ }
+
+ device_unlock(dev->parent);
+}
+
+/*
+ * usb_authorize_interface - authorize an USB interface
+ *
+ * @intf: USB interface structure
+ */
+void usb_authorize_interface(struct usb_interface *intf)
+{
+ struct device *dev = &intf->dev;
+
+ if (!intf->authorized) {
+ device_lock(dev);
+ intf->authorized = 1; /* authorize interface */
+ device_unlock(dev);
+ }
+}
+
static int usb_if_uevent(struct device *dev, struct kobj_uevent_env *env)
{
struct usb_device *usb_dev;
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 457255a..05b5e17 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -27,6 +27,8 @@ extern void usb_release_interface_cache(struct kref *ref);
extern void usb_disable_device(struct usb_device *dev, int skip_ep0);
extern int usb_deauthorize_device(struct usb_device *);
extern int usb_authorize_device(struct usb_device *);
+extern void usb_deauthorize_interface(struct usb_interface *);
+extern void usb_authorize_interface(struct usb_interface *);
extern void usb_detect_quirks(struct usb_device *udev);
extern void usb_detect_interface_quirks(struct usb_device *udev);
extern int usb_remove_device(struct usb_device *udev);
--
2.1.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 v10 3/7] usb: interface authorization: Control interface probing and claiming
Driver probings and interface claims get rejected
if an interface is not authorized.
Signed-off-by: Stefan Koch
---
drivers/usb/core/driver.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 818369a..d542d43 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -295,6 +295,10 @@ static int usb_probe_interface(struct device *dev)
if (udev->authorized == 0) {
dev_err(&intf->dev, "Device is not authorized for usage\n");
return error;
+ } else if (intf->authorized == 0) {
+ dev_err(&intf->dev, "Interface %d is not authorized for
usage\n",
+ intf->altsetting->desc.bInterfaceNumber);
+ return error;
}
id = usb_match_dynamic_id(intf, driver);
@@ -507,6 +511,10 @@ int usb_driver_claim_interface(struct usb_driver *driver,
if (dev->driver)
return -EBUSY;
+ /* reject claim if not iterface is not authorized */
+ if (!iface->authorized)
+ return -ENODEV;
+
udev = interface_to_usbdev(iface);
dev->driver = &driver->drvwrap.driver;
--
2.1.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: 4.2 kernel trace when hot unplug a mounted USB/SATA/MMC devices with ext2/ext3/ext4 file system
On Sat, Aug 15, 2015 at 1:58 AM, Christoph Hellwig wrote: > On Fri, Aug 14, 2015 at 05:52:44PM -0700, Duc Dang wrote: >> The commit 08439fec26 "ext4: remove block_device_ejected" only causes >> issue with ext4 and >> trying reverting it helps our test passes with ext4. >> >> But how about the same issue with ext2/ext3? > > I tried to fix the underlying issue, but I either failed to fully > fixed it or someone regressed it. > > Can you if rev 08439fec26 on it's own works to see if I missed > a case or it was regressed later? For more information. We tested kernel at commit 5f80f62ada "ext4: remove useless condition in if statement." (right before your commit) and still saw the issue. df3305156f989339529b3d6744b898d498fb1f7b [media] v4l: xilinx: Add Xilinx Video IP core 08439fec266c3cc5702953b4f54bdf5649357de0 ext4: remove block_device_ejected 5f80f62adae2a2920781a847805d34b36b323f7d ext4: remove useless condition in if statement. c9bca8b33118573da9b7ac2ea21947a8e4d287dd [media] v4l: of: Add v4l2_of_parse_link() function Further more, the issue does not happen with 3.19-rc7 but happens with 4.00-rc1 -- Regards, Duc Dang. -- 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 v3] usb: dwc2: add support for big-endian Lantiq SoCs
Hello, Just in case it faded from memory, here is why try2 did not get included: On Tue, 17 Mar 2015 19:33:21 -0500, Felipe Balbi wrote: > On Tue, Mar 17, 2015 at 12:23:03PM -0700, John Youn wrote: > > Can you hold off on merging this series until we get Yousaf's > > latest changes reviewed and merged? I think it would be easier to > > redo this series than the other way around. > > sure thing, dropping from my queue for now. Thanks Antti for refreshing this patch ! Regards, -- Vincent Pelletier -- 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: dwc2: rename all s3c_* to dwc2_*
On 8/18/2015 11:54 AM, Felipe Balbi wrote: > this driver has long ago became dwc2.ko with > both peripheral and host roles, there's no point > in keeping the old function names. > > Signed-off-by: Felipe Balbi > --- > arch/arm/mach-s3c64xx/mach-crag6410.c | 4 +- > arch/arm/mach-s3c64xx/mach-smartq.c | 4 +- > arch/arm/mach-s3c64xx/mach-smdk6410.c | 4 +- > arch/arm/plat-samsung/devs.c| 6 +- > drivers/usb/dwc2/core.h | 52 +-- > drivers/usb/dwc2/core_intr.c| 4 +- > drivers/usb/dwc2/debugfs.c | 20 +- > drivers/usb/dwc2/gadget.c | 696 > > drivers/usb/dwc2/hcd.c | 4 +- > drivers/usb/dwc2/platform.c | 8 +- > include/linux/platform_data/s3c-hsotg.h | 10 +- Hi Felipe, For dwc2/*: Acked-by: John Youn Tested-by: John Youn Also, can any s3c64xx maintainers comment on this platform code? Has the dwc2 driver been used and tested on these platforms since becoming dwc2? I'm not sure how the driver matching is done since I don't see anything in the device tree and the device name seems to be "s3c-hsotg". Also, are there plans to remove the usage of the platform data? Thanks, John -- 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: dwc2: rename all s3c_* to dwc2_*
On 25.08.2015 08:30, John Youn wrote:
> On 8/18/2015 11:54 AM, Felipe Balbi wrote:
>> this driver has long ago became dwc2.ko with
>> both peripheral and host roles, there's no point
>> in keeping the old function names.
>>
>> Signed-off-by: Felipe Balbi
>> ---
>> arch/arm/mach-s3c64xx/mach-crag6410.c | 4 +-
>> arch/arm/mach-s3c64xx/mach-smartq.c | 4 +-
>> arch/arm/mach-s3c64xx/mach-smdk6410.c | 4 +-
>> arch/arm/plat-samsung/devs.c| 6 +-
>> drivers/usb/dwc2/core.h | 52 +--
>> drivers/usb/dwc2/core_intr.c| 4 +-
>> drivers/usb/dwc2/debugfs.c | 20 +-
>> drivers/usb/dwc2/gadget.c | 696
>>
>> drivers/usb/dwc2/hcd.c | 4 +-
>> drivers/usb/dwc2/platform.c | 8 +-
>> include/linux/platform_data/s3c-hsotg.h | 10 +-
>
> Hi Felipe,
>
> For dwc2/*:
>
> Acked-by: John Youn
> Tested-by: John Youn
>
>
> Also, can any s3c64xx maintainers comment on this platform code?
It's the first time I see this topic. I did not receive the patch so
it's hard to comment on something I could not read. :)
> Has the dwc2 driver been used and tested on these platforms since
> becoming dwc2? I'm not sure how the driver matching is done since
> I don't see anything in the device tree and the device name seems
> to be "s3c-hsotg".
>
> Also, are there plans to remove the usage of the platform data?
You mean since 11b2c3b ("usb: dwc2: Move gadget probe function")
which effectively removed matching driver by device name? Probably no
one tested it. There aren't many users of s3c6410...
Best regards,
Krzysztof
--
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: message: remove redundant declaration
Fix the Sparse warning:
message.c:1390:21: warning: symbol 'i' shadows an earlier one
message.c:1294:13: originally declared here
Signed-off-by: Kris Borer
---
drivers/usb/core/message.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f368d20..e83648d 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1387,7 +1387,6 @@ int usb_set_interface(struct usb_device *dev, int
interface, int alternate)
* new altsetting.
*/
if (manual) {
- int i;
for (i = 0; i < alt->desc.bNumEndpoints; i++) {
epaddr = alt->endpoint[i].desc.bEndpointAddress;
--
1.9.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
Re: Some restrictions when using usbtest and g_zero
On Mon, Aug 24, 2015 at 10:53:50AM -0500, Felipe Balbi wrote:
> On Mon, Aug 24, 2015 at 10:51:04AM -0400, Alan Stern wrote:
> > On Mon, 24 Aug 2015, Peter Chen wrote:
> >
> > > Thanks, that's much clear.
> > >
> > > At udc driver:
> > >
> > > __set_halt(struct usb_ep *ep, int value, bool may_fail)
> > > {
> > > if (may_fail && ep queue is not empty) {
> > > return false
> > > } else {
> > > do stall;
> > > return true;
> > > }
> > > }
> > >
> > > gadget_ops:
> > > .set_halt = ep_set_halt,
> > >
> > > ep_set_halt(struct usb_ep *ep, int value)
> > > {
> > > __set_halt(ep, value, true);
> > > }
> > >
> > > And call __set_halt(ep, value, false) at below conditions:
> > > - SET(CLEAR)_FEATURE for Set(Clear)-Halt
> > > - If ep0 request has failed
> >
> > Yes, that should work. In fact, when a control request fails, you
> > could even call ep_set_halt instead of __set_halt, because the ep0
> > queue will certainly be empty.
>
> but you'll already hold controller's lock. ->set_halt() expects lock to
> be held. Since you only know of a failing request from within EP0 IRQ
> handler (well, you could defer to a tasklet or workqueue, or whatever),
> you'll already hold controller's lock and you'd end up with recursive
> locking errors.
>
Other EPs may call ->set_halt() at its completion, it is at interrupt
handler too, we may unlock first for both cases.
--
Best Regards,
Peter Chen
--
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: Some restrictions when using usbtest and g_zero
On Mon, Aug 24, 2015 at 02:17:08PM -0400, Alan Stern wrote: > On Mon, 24 Aug 2015, Felipe Balbi wrote: > > > > Maybe we should add a vendor-specific control request to gadget Zero so > > > that the host can tell the gadget what the transfer size will be. > > > > I proposed that many months ago and you were against it, remember ? > > I believe you -- it sounds like the sort of thing I might say -- but I > don't remember it. :-) > > > You claimed that there should be no issues with gadget and host not > > agreeing on transfer size, which actually makes sense, considering we > > have no idea what the host will send us until it does (except for things > > like BOT). > > > > If we add this vendor control request, we are likely to leave some > > issues in UDCs making assumptions as to what they should receive next. > > These are good arguments. The only place an issue arises, as far as I > can see, is in the pattern=2 data. Where should the pattern reset back > to the start? At the beginning of each packet? At the beginning of > each transfer? At the beginning of each test? > > The difficulty is that the host and gadget don't have any common > notions about transfers or tests. That leaves only packets (or maybe > bursts/multiplets for isochronous). Before each test, there is a set_alt, we can have a vendor request before that, the content of this request can be transfer length, pattern , and so on which the host and device needs to be aligned during the test. I send this email is just want to see if usbtest/g_zero can be easier to use, and test more things without user interfere. -- Best Regards, Peter Chen -- 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 suspend resume issue on Vybrid (Chipidea IP/MXS PHY)
On Mon, Aug 24, 2015 at 09:40:04PM +0530, Sanchayan Maity wrote: > Hello, > > I am working on Freescale Vybrid which is a Cortex A5 based SoC, > with a Chipidea based USB controller and a Sigmatel Phy or some- > thing if my memory serves me right. We are currently in the process > of implementing suspend resume and fixing related issues. After > resume the USB PHY does not come up and the USB subsytem prints > > [129.570097] usb 1-1: USB disconnect, device number 2 > > which comes from the core code in hub.c. > > I am using the 4.1.5 kernel with some of our own patches especially > with regards to suspend resume being present only in our own tree. > > From what I can see, the USB USBPHYx_PWDn register which has bits > related to power down, all stay in their default "1" state which > denotes power down, after resume. Now this in spite of the fact that > the code seems [2] to write 0 to the register on resume. However, > doing a quick check with devmem2 shows the register to have the > default values of "1" denoting power down. So write seems to have > no effect at all. > The PHY's clock may not be resumed correct, without normal PHY's clock, the writing to PHY's register has no effect. Check related PLL is on/enable, and output to USB PHY correct. > Instead of the code at lines 392[1] and 394[2] if I do > > return mxs_phy_hw_init(mxs_phy); > > on resume, the USBPHYx_PWDn seems to have the correct value of all > bits as zero. However of course, the USB PHY does not come up. The > stmp block reset in mxs_phy_hw_init seems to make the write work. > It seems the reset can work, and resume can not reset for PHY. > There is an errata for Vybrid at [3] in VYBRID_2N02G going as > e4535: USB: USB suspend and resume flow clarifications. Not sure > if I understand the explanation, however the following workaround > which the errata mentions: > > To place the USB PHY in low power suspend mode, the following sequence > should be performed in an atomic operation. (interrupts should be disabled > during these three steps) > > 1. Set the PORTSC1.PHCD bit > 2. Set all bits in USBPHY_PWD register > 3. Set the USBPHY_CTRL.CLKGATE bit > > These sequence of steps seem to be correctly followed in the suspend > code [4] of Chipidea IP AFAICT. > > I am not that well versed with USB subsystem code having only worked > on it once before for fixing non working USB client on Vybrid [5]. > Have tried messing with different register bits in the USB PHY and > USB miscellaneous register but with no results. > > Peter, Felipe do you have any ideas perhaps? From what I can see this > seems to be USB PHY issue. > Yes, it is PHY's suspend/resume issue. The current code follows the errata you refer. > Also Peter I wanted to ask you, the following bits > > MXS_PHY_ABNORMAL_IN_SUSPEND and MXS_PHY_SENDING_SOF_TOO_FAST where > are they being used? I can see the MXS_PHY_NEED_IP_FIX being used > but not the others. Perhaps I am missing something? There are un-upstream patch which add .notify_suspend/.nofify_resume after bus suspend/resume, in these two PHY's APIs, it will use these two flags. You can find these two APIs at freescale's released bsp. -- Best Regards, Peter Chen -- 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: [PATCHv2] Input: xpad - Fix double URB submission races
On 08/21/2015 09:50 AM, Dmitry Torokhov wrote: Hi Laura, On Mon, Aug 10, 2015 at 05:26:12PM -0700, Laura Abbott wrote: v2: Created a proper queue for events instead of just dropping them How long does it take for the queue to exhaust your memory if you keep bombarding the driver with requests? My script which changes the LEDs as fast as possible ran for 7+ hours on my machine with 16GB of RAM without exhausting all of it. This is also a very extreme case as almost any kind of delay between sending commands will drain the queue. I do not think you need a queue. I believe the nature of LEDs and rumble force feedback effect is such that you can discard all requests but the latest that arrived between the moment you submitted a request to the device and the moment you are ready submit a new one. So your suggestion is to only keep a single item in the queue? Thanks. Thanks, Laura -- 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: Re: [PATCH v5]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()
Hi Greg
>
>This still doesn't apply at all, please work on sending the patch to
>yourself and seeing if you can add it to my usb-next git branch.
>
Sorry for the inconvenience to you
I think its problem with my email client for sending patch.
Now, i will make new patch against latest linux-next tree and share with you
using git-send-email.
Thanks & Regards,
Aman Deep
--- Original Message ---
Sender : Greg KH
Date : Aug 18, 2015 23:04 (GMT+06:00)
Title : Re: [PATCH v5]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle
race condition between usb_kill_urb() and finish_unlinks()
On Tue, Aug 18, 2015 at 12:25:05PM +, AMAN DEEP wrote:
> There is a race condition between
> finish_unlinks->finish_urb() function and
> usb_kill_urb() in ohci controller case. The finish_urb
> calls spin_unlock(&ohci->lock) before
> usb_hcd_giveback_urb() function call, then if during
> this time, usb_kill_urb is called for another endpoint,
> then new ed will be added to ed_rm_list at beginning
> for unlink. and ed_rm_list will point to newly added
> ed.
>
> When finish_urb() is completed in finish_unlinks() and
> ed->td_list becomes empty as in below code (in finish_unlinks() function)
> if (list_empty(&ed->td_list)) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> }
> *last = ed->ed_next will make ed_rm_list to point to ed->ed_next and
> previously added ed by usb_kill_urb will be left unreferenced by
> ed_rm_list. This causes usb_kill_urb() hang forever waiting for
> finish_unlink to remove added ed from ed_rm_list.
>
> The main reason for hang in this race condtion is addition and removal
> of ed from ed_rm_list in the beginning during usb_kill_urb and later last*
> is modified in finish_unlinks().
> As suggested by Alan Stern, the solution for proper handling of
> ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
> any URBs. Then at the end, we can add ed back to the list if necessary.
> This properly handle the updated ohci->ed_rm_list in
> usb_kill_urb().
>
> Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a
> controller dies")
>
> Acked-by: Alan Stern
>
> Signed-off-by: Aman Deep
> CC:
> ---
> drivers/usb/host/ohci-q.c | 17 ++---
> 1 file changed, 10 insertions(+), 7 deletions(-)
This still doesn't apply at all, please work on sending the patch to
yourself and seeing if you can add it to my usb-next git branch.
greg k-h
Thanks & Regards,
Aman
DeepN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ºÆâžØ^n‡r¡ö¦zË�ëh™¨èÚ&¢ø ®G«�éh®(階ŠÝ¢j"�ú¶m§ÿï�êäz¹Þ–Šàþf£¢·hšˆ§~ˆmš
Re: PROBLEM: USB devices not appearing after upgrading 4.1.4 => 4.1.5
On 21 August 2015 at 17:48, Daurnimator wrote: > On 21 August 2015 at 17:39, Mathias Nyman wrote: >> I can't directly see what would cause it. Only theory for now is if one >> those changes left >> some port change event uncleared, blocking port status change events and >> thus maybe also >> command completion event, and thus causing timeout. >> >> If possible, could you try to bisect (or just cherry pick) the changes and >> try to find >> the one causing issues? > > It's probably best I do this at my office (which I'll be back at on Monday) Excuse the noob-ness, but how do I bisect these commits? I use archlinux, so I downloaded the PKGBUILD for the linux package. But the git clone of git+https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git doesn't seem to contain the commits you mentioned. There are only tags for the minor version number, not the patch (i.e. there's a v4.1, but no v4.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
Re: [PATCH 1/7] phy: exynos-usb2: add vbus regulator support
Hello, On 2015-08-21 14:44, Kishon Vijay Abraham I wrote: On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote: Exynos USB2 PHY has separate power supply, which is usually provided by VBUS regulator. This patch adds support for it. VBUS regulator is optional, to keep compatibility with boards, which have VBUS provided from some always-on power source. Signed-off-by: Marek Szyprowski --- .../devicetree/bindings/phy/samsung-phy.txt| 3 +++ drivers/phy/phy-samsung-usb2.c | 25 -- drivers/phy/phy-samsung-usb2.h | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 60c6f2a633e0..0289d3b07853 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -44,6 +44,9 @@ Required properties: - the "ref" clock is used to get the rate of the clock provided to the PHY module +Optional properties: +- vbus-supply: power-supply phandle for vbus power source how about using phy-supply? I wanted to make it a bit more descriptive (vbus-supply is rather self explaining name) and keep it in line with Exynos usb3-drd phy, which already supports vbus-supply. If you think that this is a bad idea, a will use phy-supply then. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- 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/7] phy: exynos-usb2: add vbus regulator support
On 25.08.2015 14:47, Marek Szyprowski wrote: > Hello, > > On 2015-08-21 14:44, Kishon Vijay Abraham I wrote: >> On Friday 21 August 2015 06:08 PM, Marek Szyprowski wrote: >>> Exynos USB2 PHY has separate power supply, which is usually provided by >>> VBUS regulator. This patch adds support for it. VBUS regulator is >>> optional, to keep compatibility with boards, which have VBUS provided >>> from some always-on power source. >>> >>> Signed-off-by: Marek Szyprowski >>> --- >>> .../devicetree/bindings/phy/samsung-phy.txt| 3 +++ >>> drivers/phy/phy-samsung-usb2.c | 25 >>> -- >>> drivers/phy/phy-samsung-usb2.h | 2 ++ >>> 3 files changed, 28 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> index 60c6f2a633e0..0289d3b07853 100644 >>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> @@ -44,6 +44,9 @@ Required properties: >>> - the "ref" clock is used to get the rate of the clock provided >>> to the >>> PHY module >>> +Optional properties: >>> +- vbus-supply: power-supply phandle for vbus power source >> how about using phy-supply? > > I wanted to make it a bit more descriptive (vbus-supply is rather self > explaining name) > and keep it in line with Exynos usb3-drd phy, which already supports > vbus-supply. > If you think that this is a bad idea, a will use phy-supply then. This is actually supply for VBUS, not for the phy. Using phy-supply would work fine and reduce the size of code... but would be rather a hacky-use of phy and it could be misleading. I don't have strong feeling about this, both ideas have its advantages. If I had to choose than I would like to use vbus-supply because of its correctness with real-world (this is a VBUS after all). Best regards, Krzysztof -- 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
[RESEND PATCH 3/7] usb: phy: isp1301: Export I2C module alias information
The I2C core always reports the MODALIAS uevent as "i2c:
---
drivers/usb/phy/phy-isp1301.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index 8a55b37d1a02..db68156568e6 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
{ "isp1301", 0 },
{ }
};
+MODULE_DEVICE_TABLE(i2c, isp1301_id);
static struct i2c_client *isp1301_i2c_client;
--
2.4.3
--
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
[RESEND PATCH 0/7] Export I2C and OF module aliases in missing drivers
Hello, This is a resend of the patches that were not picked from the series "[PATCH 00/27] Export I2C and OF module aliases in missing drivers" [0] posted about a month ago. The patches have no dependencies and can be picked individually by the relevant maintainer. I preferred to resend instead of sending a naked ping for each patch that I got no answer. Best regards, Javier Javier Martinez Canillas (7): i2c: core: Export I2C module alias information in dummy driver backlight: tosa: Export I2C module alias information usb: phy: isp1301: Export I2C module alias information ALSA: ppc: keywest: Export I2C module alias information extcon: Export OF module alias information in missing drivers leds: Export OF module alias information in missing drivers regulator: isl9305: Export OF module alias information drivers/extcon/extcon-rt8973a.c | 1 + drivers/extcon/extcon-sm5502.c| 1 + drivers/i2c/i2c-core.c| 1 + drivers/leds/leds-pca963x.c | 1 + drivers/leds/leds-tca6507.c | 1 + drivers/regulator/isl9305.c | 1 + drivers/usb/phy/phy-isp1301.c | 1 + drivers/video/backlight/tosa_bl.c | 1 + sound/ppc/keywest.c | 1 + 9 files changed, 9 insertions(+) -- 2.4.3 -- 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
