Re: [PATCH v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups
On Wed, Mar 30, 2016 at 12:56:27PM +0300, Roger Quadros wrote: > Hi, > > Add documentation for struct otg_fsm with some trivial cleanups. > All patches have been Acked by otg-fsm maintainer (Peter Chen). > I will queue them, thanks. -- 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: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
On 31/03/16 01:16, Ruslan Bilovol wrote: > Hi, > > On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu wrote: >> Hi, >> >> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders wrote: >>> Bin Liu writes: >>> Dirk, On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders wrote: > Bin Liu writes: > >> All, >> >> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN >> wrote: >>> Dirk, All, >>> >>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly: If choices consist of choice_values that depend on symbols set to 'm', those choice_values are not set to 'n' if the choice is changed from 'm' to 'y' (in which case only one active choice_value is allowed). Those values are also written to the config file causing modules to be built when they should not. The following config can be used to reproduce and examine the problem; with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', then set "Tristate Choice" to 'y' and save the configuration: config modules boolean modules default y option modules config dependency tristate "Dependency" default m choice prompt "Tristate Choice" default choice0 config choice0 tristate "Choice 0" config choice1 tristate "Choice 1" depends on dependency endchoice This patch sets choice_values' visibility that depend on symbols set to 'm' to 'n' if the corresponding choice is set to 'y'. This makes them disappear from the choice list and will also cause the choice_values' value set to 'n' in sym_calc_value() and as a result they are written as "not set" to the resulting .config file. Reported-by: Sebastian Andrzej Siewior Signed-off-by: Dirk Gouders Tested-by: Sebastian Andrzej Siewior >>> >>> Acked-by: "Yann E. MORIN" >>> >>> It will be in my tree soon. Thanks! >> >> I don't see this patch in 3.16 but 3.16 does not have the issue any >> more. Anyone has an idea how the issue got fixed? I am trying to find >> the right patch to backport. > > With the above sample kconfig I still see the issue. How did you > notice the issue got fixed? I did not pay much attention on the above sample kconfig, but just focused on the USB gadget driver kconfig issue initially reported by Sebastian. I saw the issue exists in 3.14, but does not in 3.16, unless I messed up with my test. I will test 3.16 again some time next week. >>> >>> Hi Bin, >>> >>> I now also re-tested the initially reported steps to reproduce the >>> issue: >>> >>> in USB gadget menu (that is Device Drivers ---> USB support ---> USB Gadget Support ---> USB Gadget Drivers) I can create a configuration which is "lost". Here is how to reproduce it: - first config two gadgets as M: USB Gadget Drivers Audio Gadget Ethernet Gadget MIDI Gadget save config & leave - now start menu config again and go to the same menu, now select built-in: <*> USB Gadget Drivers (Ethernet Gadget the ethernet gadget is chosen automatically because we can have only one gadget selected. save config & leave - step three, go back to the menu and you will see that everything is as it was (the <*> is ignored). >>> >>> >>> Here, I still see the problem (I was wondering if the issue has been >>> solved/gone by a kconfig-file modification). >> >> This issue was gone since 3.16, but came back again due to commit >> 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules. >> > > I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet) > Issue is present on v4.6-rc1 as well and the $subject patch fixes the issue. cheers, -roger -- 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/2] usb:dwc3: Enable support for 64-bit system
Hi Balbi,
If CONFIG_DMA_CMA=y, dma mask is set properly. The issue just happen
when CONFIG_DMA_CMA is not set. In this case, dma mask is not set and
we need this code to check if dma mask should be manually set to 32 or
64.
Thang
On Wed, Mar 30, 2016 at 8:09 PM, Felipe Balbi
wrote:
>
> Hi,
>
> "Thang Q. Nguyen" writes:
>> From: "Thang Q. Nguyen"
>>
>> Add 64-bit DMA operation support to the USB DWC3 driver.
>> First attempt to set the coherent DMA mask for 64-bit DMA.
>> If that failed, attempt again with 32-bit DMA.
>>
>> Changes from v2:
>> - None.
>>
>> Changes from v1:
>> - Remove WARN_ON if dma_mask is NULL
>
> these changes lines should be between the tearline (---) and diffstat
> below.
>
>> Signed-off-by: Thang Q. Nguyen
>> ---
>> drivers/usb/dwc3/core.c | 15 +++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index de5e01f..2479c24 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>> dwc->mem = mem;
>> dwc->dev = dev;
>>
>> + /* Try to set 64-bit DMA first */
>> + if (!pdev->dev.dma_mask)
>> + /* Platform did not initialize dma_mask */
>> + ret = dma_coerce_mask_and_coherent(&pdev->dev,
>> +DMA_BIT_MASK(64));
>> + else
>> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>> +
>> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
>> + if (ret) {
>> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> + }
>
> Also, why is it so that you need this now ? glue layers are copying dma
> mask from parent device and that should be set properly. This really
> shouldn't be necessary in dwc3-core; it would mean that glue layer
> didn't set this device up properly.
>
> --
> balbi
--
Thang Q. Nguyen| Staff SW Eng.
C: +849.7684.7606 | O: +848.3770.0640
F: +848.3770.0641 | tqngu...@apm.com
--
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 2/2] usb:dwc3: pass arch data to xhci-hcd child
Thanks Grygorii for information. I checked but do not see dma_init_dev_from_parent is used in linux-next repository. Can you give me more information for what branch I can checkout to use it for USB DWC3? Thanks, Thang -- -- 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
need to resubmit serial fixes?
Hi, do I need to resubmit the serial fixes I submitted for the issues with the forged device descriptors leading to crashes? 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: [PATCH v7 1/4] gadget: Introduce the usb charger framework
On 31 March 2016 at 14:42, Felipe Balbi wrote:
+#define DEFAULT_CUR_PROTECT (50)
>>>
>>> Where is this coming from ? Also, () are not necessary.
>>
>> Just want to protect the default current limitation. If that does not
>> need, I'll remove it.
>
> It's your HW :-) You tell me if it's really necessary. But, hey, if you
> get enumerated @500mA, this is the host telling you it _CAN_ give you
> 500mA. In that case, why wouldn't you ?
Make sense. I'll remove the 'DEFAULT_CUR_PROTECT' macro.
>
+#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
>>>
>>> According to the spec we should always be talking about unit loads (1
>>> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
>>> work for SS capable ports and SS gadgets (we have quite a few of them,
>>> actually). You're missing the opportunity of charging at 900mA.
>>
>> I follow the DCP/SDP/CDP/ACA type's default current limitation and
>> user can set them what they want.
>
> no, the user CANNOT set it to what they want. If you get enumerated
> @100mA and the user just decides to set it to 2000mA, s/he could even
> melt the USB connector. The kernel _must_ prevent such cases.
>
> In any case, DEFAULT_SDP_CUR_LIMIT shouldn't be a constant, it should be
> variable because if you enumerate in SS, you _can_ get up to 900mA.
Make sense. But these are just default values. They can be changed
safely by power driver with 'usb_charger_set_cur_limit_by_type()'
function to set 900mA.
>
+#define DEFAULT_DCP_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_CDP_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_ACA_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
+#define UCHGER_STATE_LENGTH (50)
>>>
>>> what is this UCHGER_STATE_LENGTH ? And also, why don't you spell it out?
>>> Is this weird name coming from a spec ? Which spec ?
>>
>> It is used to indicate the array size to save the charger state to
>> report to userspace. I should move it to where it is used.
>
> and ARRAY_SIZE(arr) is not enough ?
OK.
>
>>> sure this fits as a bus_type. There's no "usb charger" bus. There's USB
>>> bus and its VBUS/GND lines. Why are you using a bus_type here ?
>>
>> I want to use bus structure to manage the charger device. Maybe choose
>> class to manage them?
>
> I guess a class would fit better in this case.
OK.
>
+{
+ return container_of(udev, struct usb_charger, dev);
+}
+
+static ssize_t sdp_limit_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
+}
+
+static ssize_t sdp_limit_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ unsigned int sdp_limit;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &sdp_limit);
+ if (ret < 0)
+ return ret;
+
+ ret = usb_charger_set_cur_limit_by_type(uchger, SDP_TYPE, sdp_limit);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_RW(sdp_limit);
>>>
>>> why RW ? Who's going to use these ? Also, you're not documenting this
>>> new sysfs file.
>>
>> Cause we have show and store operation for SDP type. If users want to
>> know or set the SDP current, they can use the sysfs file.
>> I'll add the documentation for it.
>
> but why would the user change it ? Here's the thing: you have a few
> posibilities for this:
>
> a) you are connected to a dedicated charger
>
> In this case, you can get up to 2000mA depending on the charger.
>
> If $this charger can give you or not 2000mA is not detectable,
> so what do charging ICs do ? They slowly increase the attached
> load accross VBUS/GND and measure VBUS value. When IC notices
> VBUS dropping bit, step back to previous load.
>
> This means you will always charger with maximum rating of DCP.
>
> Why would user change this ? More is unsafe, less is just
> stupid.
>
> b) you are connected to a host charging port and get enumerated with
> your 500mA configuration.
>
> you *know* 500mA is okay, but you _can_ get more (it is a
> charging port after all). So charging IC will connect a 500mA
> load and step upwards until VBUS drops a little, just like (a)
> above.
>
> This means you will always charger with maximum rating for this
> host charging port.
>
> Why would user change this ? More is unsafe, less is just
> stupid.
>
> c) you are connected to a standard port and get enumerated with your
> 500mA configuration.
>
>
Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system
Hi,
(please don't top-post)
"Thang Q. Nguyen" writes:
> Hi Balbi,
> If CONFIG_DMA_CMA=y, dma mask is set properly. The issue just happen
> when CONFIG_DMA_CMA is not set. In this case, dma mask is not set and
> we need this code to check if dma mask should be manually set to 32 or
> 64.
Can you point me to the code which has this conditional ? Why would
DMA_CMA=n mean that dma_mask isn't initialized ? According to DMA_CMA's
help text (see below) that's supposed to allow drivers to *allocate*
large contiguous buffers, but that's not the case here.
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
help
This enables the Contiguous Memory Allocator which allows drivers
to allocate big physically-contiguous blocks of memory for use with
hardware components that do not support I/O map nor scatter-gather.
You can disable CMA by specifying "cma=0" on the kernel's command
line.
For more information see .
If unsure, say "n".
>
>
> Thang
>
> On Wed, Mar 30, 2016 at 8:09 PM, Felipe Balbi
> wrote:
>>
>> Hi,
>>
>> "Thang Q. Nguyen" writes:
>>> From: "Thang Q. Nguyen"
>>>
>>> Add 64-bit DMA operation support to the USB DWC3 driver.
>>> First attempt to set the coherent DMA mask for 64-bit DMA.
>>> If that failed, attempt again with 32-bit DMA.
>>>
>>> Changes from v2:
>>> - None.
>>>
>>> Changes from v1:
>>> - Remove WARN_ON if dma_mask is NULL
>>
>> these changes lines should be between the tearline (---) and diffstat
>> below.
>>
>>> Signed-off-by: Thang Q. Nguyen
>>> ---
>>> drivers/usb/dwc3/core.c | 15 +++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index de5e01f..2479c24 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>>> dwc->mem = mem;
>>> dwc->dev = dev;
>>>
>>> + /* Try to set 64-bit DMA first */
>>> + if (!pdev->dev.dma_mask)
>>> + /* Platform did not initialize dma_mask */
>>> + ret = dma_coerce_mask_and_coherent(&pdev->dev,
>>> +DMA_BIT_MASK(64));
>>> + else
>>> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>> +
>>> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
>>> + if (ret) {
>>> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>>> + if (ret)
>>> + return ret;
>>> + }
>>
>> Also, why is it so that you need this now ? glue layers are copying dma
>> mask from parent device and that should be set properly. This really
>> shouldn't be necessary in dwc3-core; it would mean that glue layer
>> didn't set this device up properly.
>>
>> --
>> balbi
>
>
>
> --
>
> Thang Q. Nguyen| Staff SW Eng.
>
> C: +849.7684.7606 | O: +848.3770.0640
>
> F: +848.3770.0641 | tqngu...@apm.com
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v3 2/2] usb:dwc3: pass arch data to xhci-hcd child
"Thang Q. Nguyen" writes: > [ text/plain ] > Thanks Grygorii for information. > I checked but do not see dma_init_dev_from_parent is used in > linux-next repository. Can you give me more information for what > branch I can checkout to use it for USB DWC3? dma_init_dev_from_parent() is still a proposal ;-) -- balbi signature.asc Description: PGP signature
Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's
Hi Heiner, On 03/30/2016 03:59 PM, Heiner Kallweit wrote: On Wed, Mar 30, 2016 at 3:03 PM, Pavel Machek wrote: Hi! Ok, so: a) Do we want RGB leds to be handled by existing subsystem, or do we need separate layer on top of that? b) Does RGB make sense, or HSV? RGB is quite widely used in graphics, and it is what hardware implements. (But we'd need to do gamma correction). c) My hardware has "acceleration engine", LED is independend from CPU. That's rather big deal. Does yours? It seems to be quite common, at least in cellphones. Ideally, I'd like to have "triggers", but different ones. As in: if charging, do yellow " .xX" pattern. If fully charged, do green steady light. If message is waiting, do blue " x x" pattern. If none of above, do slow white blinking. (Plus priorities of events). But that's quite different from existing support...) Please note that HSV colour scheme allows to neatly project monochrome brightness semantics on the RGB realm. I.e. you can have fixed hue and saturation, and by changing the brightness component a perceived colour intensity can be altered. I see HSV has some advantages. But we already have LEDs with multiple colors, and kernel already handles them: pavel@duo:~$ ls -1 /sys/class/leds/ tpacpi:green:batt tpacpi:orange:batt This is physically 2 leds but hidden under one indicator, so you got "off", "green", "orange" and "green+orange". That's a good example. As long as you can recognize green+orange as separate lights/colors (w/o magnifying glass) I wouldn't call it "a LED with multiple colors" but "multiple LED devices". In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs. And it's not only about using a handful of discrete colors but about displaying any arbitrary color. So far the kernel exposes the physical RGB LEDs as separate LEDs only and I can't use a trigger to e.g. set "magenta with 50% brightness". I think that we should consult more people before pushing the solution upstream. Would you mind writing a message with an explanation of the issue to linux-api list? Please keep in mind also the information from the "Attributes" section of Documentation/filesystems/sysfs.txt. -- Best regards, Jacek Anaszewski -- 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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
Hi Baolin, Baolin Wang writes: >>> >> Make sense. In our company's solution, charger detection can be done >>> >> by hardware from PMIC at first, then it will not affect the DP/DM >>> >> line when gadget starts to enumeration. >>> > >>> > I see, charger type detection is done automatically by PMIC when VBUS >>> > is detected in your case, you just assume the process is complete >>> > before SW do gadget connect. To make the framework common, you may do >>> one time charger type check when vbus is on, and save it to avoid repeat >>> charger type check. >>> >>> OK. I'll add one judgement to check if the charger type is set in >>> 'usb_charger_detect_type()' function. >> >> Just adding a judgement isn't enough here, your framework should make sure >> usb_charger_detect_type() is called before gadget connect, with that, the >> existing caller place just gets the charger type from the saved value. >> The real charger type detection done by usb_charger_detect_type() can >> be called only when vbus is on. >> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control(). > > Yeah, Like Felipe suggested, I think we need to introduce one > 'charger_detect()' method to do the SW charger type detection at the > right gadget state. Thanks for your comments. Just to be clear, we add ->charger_detect() when we know of a platform which needs to manually detect the charger type. Until then, we ignore that situation. It might be a good idea, however, do document this in comments on your structure definition stating that if we need to detect charger type, a new method should be added ;-) cheers -- balbi signature.asc Description: PGP signature
Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework
Hi,
Baolin Wang writes:
> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
According to the spec we should always be talking about unit loads (1
unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
work for SS capable ports and SS gadgets (we have quite a few of them,
actually). You're missing the opportunity of charging at 900mA.
>>>
>>> I follow the DCP/SDP/CDP/ACA type's default current limitation and
>>> user can set them what they want.
>>
>> no, the user CANNOT set it to what they want. If you get enumerated
>> @100mA and the user just decides to set it to 2000mA, s/he could even
>> melt the USB connector. The kernel _must_ prevent such cases.
>>
>> In any case, DEFAULT_SDP_CUR_LIMIT shouldn't be a constant, it should be
>> variable because if you enumerate in SS, you _can_ get up to 900mA.
>
> Make sense. But these are just default values. They can be changed
> safely by power driver with 'usb_charger_set_cur_limit_by_type()'
> function to set 900mA.
oh okay. Still, the default value should be a function of gadget->speed,
IMO ;-)
> +
> +/* USB charger state */
> +enum usb_charger_state {
> + USB_CHARGER_DEFAULT,
> + USB_CHARGER_PRESENT,
> + USB_CHARGER_REMOVE,
> +};
userland really doesn't need these two ?
>>>
>>> We've reported to userspace by kobject_uevent in
>>> 'usb_charger_notify_others()' function.
>>
>> I mean as a type ;-) So userspace doesn't have to redefine these for
>> their applications.
>
> Make sense. I can introduce some sysfs files for userspace. Thanks for
> your comments.
okay, my reply was a bit cryptic, but what I mean here is that enum
usb_charger_state could be moved your include/uapi header. My question
is, then, does userland need to have knowledge of enum
usb_charger_state ?
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On 31 March 2016 at 16:15, Felipe Balbi wrote: > > Hi Baolin, > > Baolin Wang writes: >> Make sense. In our company's solution, charger detection can be done >> by hardware from PMIC at first, then it will not affect the DP/DM >> line when gadget starts to enumeration. > > I see, charger type detection is done automatically by PMIC when VBUS > is detected in your case, you just assume the process is complete > before SW do gadget connect. To make the framework common, you may do one time charger type check when vbus is on, and save it to avoid repeat charger type check. OK. I'll add one judgement to check if the charger type is set in 'usb_charger_detect_type()' function. >>> >>> Just adding a judgement isn't enough here, your framework should make sure >>> usb_charger_detect_type() is called before gadget connect, with that, the >>> existing caller place just gets the charger type from the saved value. >>> The real charger type detection done by usb_charger_detect_type() can >>> be called only when vbus is on. >>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control(). >> >> Yeah, Like Felipe suggested, I think we need to introduce one >> 'charger_detect()' method to do the SW charger type detection at the >> right gadget state. Thanks for your comments. > > Just to be clear, we add ->charger_detect() when we know of a platform > which needs to manually detect the charger type. Until then, we ignore > that situation. It might be a good idea, however, do document this in > comments on your structure definition stating that if we need to detect > charger type, a new method should be added ;-) Make sense. Thanks. > > cheers > > -- > balbi -- Baolin.wang Best Regards -- 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 v7 1/4] gadget: Introduce the usb charger framework
On 31 March 2016 at 16:18, Felipe Balbi wrote:
>
> Hi,
>
> Baolin Wang writes:
>> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
>
> According to the spec we should always be talking about unit loads (1
> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
> work for SS capable ports and SS gadgets (we have quite a few of them,
> actually). You're missing the opportunity of charging at 900mA.
I follow the DCP/SDP/CDP/ACA type's default current limitation and
user can set them what they want.
>>>
>>> no, the user CANNOT set it to what they want. If you get enumerated
>>> @100mA and the user just decides to set it to 2000mA, s/he could even
>>> melt the USB connector. The kernel _must_ prevent such cases.
>>>
>>> In any case, DEFAULT_SDP_CUR_LIMIT shouldn't be a constant, it should be
>>> variable because if you enumerate in SS, you _can_ get up to 900mA.
>>
>> Make sense. But these are just default values. They can be changed
>> safely by power driver with 'usb_charger_set_cur_limit_by_type()'
>> function to set 900mA.
>
> oh okay. Still, the default value should be a function of gadget->speed,
Sorry, I did not get your suggestion, could you give me an example? Thanks.
> IMO ;-)
>
>> +
>> +/* USB charger state */
>> +enum usb_charger_state {
>> + USB_CHARGER_DEFAULT,
>> + USB_CHARGER_PRESENT,
>> + USB_CHARGER_REMOVE,
>> +};
>
> userland really doesn't need these two ?
We've reported to userspace by kobject_uevent in
'usb_charger_notify_others()' function.
>>>
>>> I mean as a type ;-) So userspace doesn't have to redefine these for
>>> their applications.
>>
>> Make sense. I can introduce some sysfs files for userspace. Thanks for
>> your comments.
>
> okay, my reply was a bit cryptic, but what I mean here is that enum
> usb_charger_state could be moved your include/uapi header. My question
> is, then, does userland need to have knowledge of enum
> usb_charger_state ?
I am not sure if userland need the enum usb_charger_state. But I
remember you want to report the charger state to userland in previous
email.
>
> --
> balbi
--
Baolin.wang
Best Regards
--
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: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems
Hi John, Am 29.03.2016 um 04:36 schrieb John Youn: > Hi, > > The following patch series addresses the core reset and force mode > delay problems we have been seeing on dwc2 for some platforms. > > I think I have identified the source of the inconsistencies between > platforms and this series attempts to address them. > > Basically everything stems from the IDDIG debounce filter delay, which > is a function of the PHY clock speed and can range from 5-50 ms if > enabled. This delay must be taken into account on core reset and force > modes. A full explanation is provided in the patch commit log and code > comments. > > The first two patches are prerequisites to the force mode fixes, > including one patch that was sent separately by Przemek Rudy. I have > resubmitted it with this series for convenience. > > Please help by reviewing and testing on your platforms. > > Patches were tested on: > * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL i applied the complete patch series on 4.6rc-1 and successfully tested it with my Raspberry Pi B (IP 2.80a, dr_mode=HOST). Probing and usual USB devices (network, keyboard) still works fine. Do you need any other specific tests? Regards Stefan > > Regards, > John > > John Youn (3): > usb: dwc2: gadget: Only initialize device if in device mode > usb: dwc2: Add delay to core soft reset > usb: dwc2: Properly account for the force mode delays > > Przemek Rudy (1): > usb: dwc2: do not override forced dr_mode in gadget setup > > drivers/usb/dwc2/core.c | 195 > > drivers/usb/dwc2/core.h | 2 +- > drivers/usb/dwc2/gadget.c | 30 +-- > drivers/usb/dwc2/hcd.c | 6 +- > drivers/usb/dwc2/hw.h | 1 + > drivers/usb/dwc2/platform.c | 9 +- > 6 files changed, 161 insertions(+), 82 deletions(-) > -- 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 v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups
Peter, On 31/03/16 10:08, Peter Chen wrote: > On Wed, Mar 30, 2016 at 12:56:27PM +0300, Roger Quadros wrote: >> Hi, >> >> Add documentation for struct otg_fsm with some trivial cleanups. >> All patches have been Acked by otg-fsm maintainer (Peter Chen). >> > > I will queue them, thanks. > Patch 3 might cause a build failure for drivers/usb/phy/phy-fsl-usb.c since it relies on VDBG that was defined in include/linux/usb/otg-fsm.h I will send a revised patch 3 that locally defines VDBG for phy-fsl-usb.c cheers, -roger -- 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 v7 3/3] 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
---
v7: define VDBG locally in phy-fsl-usb.c
drivers/usb/chipidea/otg_fsm.c | 1 +
drivers/usb/common/usb-otg-fsm.c | 12 +++-
drivers/usb/phy/phy-fsl-usb.c| 8
include/linux/usb/otg-fsm.h | 19 ---
4 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index de8e22e..5f169b3 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -805,6 +805,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;
ci->gadget.hnp_polling_support = 1;
ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
if (!ci->fsm.host_req_flag)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 9059b7d..c5a61fe 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);
@@ -208,7 +209,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:
@@ -338,7 +339,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
@@ -446,7 +447,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..c57ef5c 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -44,6 +44,13 @@
#include "phy-fsl-usb.h"
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s] " fmt , \
+ __func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
#define DRIVER_VERSION "Rev. 1.55"
#define DRIVER_AUTHOR "Jerry Huang/Li Yang"
#define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
@@ -817,6 +824,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 7a03505..47b8392 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)
@@ -211,6 +197,9 @@ struct otg_fsm {
u8 *host_req_flag;
st
Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
Dirk Gouders writes: > Roger Quadros writes: > >> On 31/03/16 01:16, Ruslan Bilovol wrote: >>> Hi, >>> >>> On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu wrote: Hi, On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders wrote: > Bin Liu writes: > >> Dirk, >> >> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders wrote: >>> Bin Liu writes: >>> All, On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN wrote: > Dirk, All, > > On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly: >> If choices consist of choice_values that depend on symbols set to >> 'm', >> those choice_values are not set to 'n' if the choice is changed from >> 'm' to 'y' (in which case only one active choice_value is allowed). >> Those values are also written to the config file causing modules to >> be >> built when they should not. >> >> The following config can be used to reproduce and examine the >> problem; >> with the frontend of your choice set "Choice 0" and "Choice 1" to >> 'm', >> then set "Tristate Choice" to 'y' and save the configuration: >> >> config modules >> boolean modules >> default y >> option modules >> >> config dependency >> tristate "Dependency" >> default m >> >> choice >> prompt "Tristate Choice" >> default choice0 >> >> config choice0 >> tristate "Choice 0" >> >> config choice1 >> tristate "Choice 1" >> depends on dependency >> >> endchoice >> >> This patch sets choice_values' visibility that depend on symbols set >> to 'm' to 'n' if the corresponding choice is set to 'y'. This makes >> them disappear from the choice list and will also cause the >> choice_values' value set to 'n' in sym_calc_value() and as a result >> they are written as "not set" to the resulting .config file. >> >> Reported-by: Sebastian Andrzej Siewior >> Signed-off-by: Dirk Gouders >> Tested-by: Sebastian Andrzej Siewior > > Acked-by: "Yann E. MORIN" > > It will be in my tree soon. Thanks! I don't see this patch in 3.16 but 3.16 does not have the issue any more. Anyone has an idea how the issue got fixed? I am trying to find the right patch to backport. >>> >>> With the above sample kconfig I still see the issue. How did you >>> notice the issue got fixed? >> >> I did not pay much attention on the above sample kconfig, but just >> focused on the USB gadget driver kconfig issue initially reported by >> Sebastian. I saw the issue exists in 3.14, but does not in 3.16, >> unless I messed up with my test. I will test 3.16 again some time next >> week. > > Hi Bin, > > I now also re-tested the initially reported steps to reproduce the > issue: > > >> in USB gadget menu (that is Device Drivers ---> USB support ---> USB >> Gadget Support ---> USB Gadget Drivers) I can create a configuration >> which is "lost". Here is how to reproduce it: >> >> - first config two gadgets as M: >> USB Gadget Drivers >> Audio Gadget >> Ethernet Gadget >> MIDI Gadget >> >> save config & leave >> >> - now start menu config again and go to the same menu, now select >> built-in: >> <*> USB Gadget Drivers (Ethernet Gadget >> the ethernet gadget is chosen automatically because we can have only >> one gadget selected. >> save config & leave >> >> - step three, go back to the menu and you will see that everything is >> as it was (the <*> is ignored). > > > Here, I still see the problem (I was wondering if the issue has been > solved/gone by a kconfig-file modification). This issue was gone since 3.16, but came back again due to commit 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules. >>> >>> I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet) >>> >> >> Issue is present on v4.6-rc1 as well and the $subject patch fixes the issue. > > I would say the issue is present until the code is fixed. Whether it is > triggered depends on the Kconfig used. > > Perhaps, Michal could take a look at the patch that Yann ack'ed, some > time ago. Apologies if this sounded rude (I am not a native speaker). What I had in mind was that as far as I remember this patch was posted at the time when Yann changed his inte
Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework
Baolin Wang writes:
> [ text/plain ]
> On 31 March 2016 at 16:18, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Baolin Wang writes:
>>> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
>>
>> According to the spec we should always be talking about unit loads (1
>> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
>> work for SS capable ports and SS gadgets (we have quite a few of them,
>> actually). You're missing the opportunity of charging at 900mA.
>
> I follow the DCP/SDP/CDP/ACA type's default current limitation and
> user can set them what they want.
no, the user CANNOT set it to what they want. If you get enumerated
@100mA and the user just decides to set it to 2000mA, s/he could even
melt the USB connector. The kernel _must_ prevent such cases.
In any case, DEFAULT_SDP_CUR_LIMIT shouldn't be a constant, it should be
variable because if you enumerate in SS, you _can_ get up to 900mA.
>>>
>>> Make sense. But these are just default values. They can be changed
>>> safely by power driver with 'usb_charger_set_cur_limit_by_type()'
>>> function to set 900mA.
>>
>> oh okay. Still, the default value should be a function of gadget->speed,
>
> Sorry, I did not get your suggestion, could you give me an example? Thanks.
int default_current_limit = 500;
if (gadget->speed >= USB_SPEED_SUPER)
default_current_limit = 900;
>>> +
>>> +/* USB charger state */
>>> +enum usb_charger_state {
>>> + USB_CHARGER_DEFAULT,
>>> + USB_CHARGER_PRESENT,
>>> + USB_CHARGER_REMOVE,
>>> +};
>>
>> userland really doesn't need these two ?
>
> We've reported to userspace by kobject_uevent in
> 'usb_charger_notify_others()' function.
I mean as a type ;-) So userspace doesn't have to redefine these for
their applications.
>>>
>>> Make sense. I can introduce some sysfs files for userspace. Thanks for
>>> your comments.
>>
>> okay, my reply was a bit cryptic, but what I mean here is that enum
>> usb_charger_state could be moved your include/uapi header. My question
>> is, then, does userland need to have knowledge of enum
>> usb_charger_state ?
>
> I am not sure if userland need the enum usb_charger_state. But I
> remember you want to report the charger state to userland in previous
> email.
right, which means this enumeration definition could be placed in the
UAPI header. Unless, of course, we're reporting strings, rather than
integers, in the sysfs file.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols
Roger Quadros writes: > On 31/03/16 01:16, Ruslan Bilovol wrote: >> Hi, >> >> On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu wrote: >>> Hi, >>> >>> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders wrote: Bin Liu writes: > Dirk, > > On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders wrote: >> Bin Liu writes: >> >>> All, >>> >>> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN >>> wrote: Dirk, All, On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly: > If choices consist of choice_values that depend on symbols set to 'm', > those choice_values are not set to 'n' if the choice is changed from > 'm' to 'y' (in which case only one active choice_value is allowed). > Those values are also written to the config file causing modules to be > built when they should not. > > The following config can be used to reproduce and examine the problem; > with the frontend of your choice set "Choice 0" and "Choice 1" to 'm', > then set "Tristate Choice" to 'y' and save the configuration: > > config modules > boolean modules > default y > option modules > > config dependency > tristate "Dependency" > default m > > choice > prompt "Tristate Choice" > default choice0 > > config choice0 > tristate "Choice 0" > > config choice1 > tristate "Choice 1" > depends on dependency > > endchoice > > This patch sets choice_values' visibility that depend on symbols set > to 'm' to 'n' if the corresponding choice is set to 'y'. This makes > them disappear from the choice list and will also cause the > choice_values' value set to 'n' in sym_calc_value() and as a result > they are written as "not set" to the resulting .config file. > > Reported-by: Sebastian Andrzej Siewior > Signed-off-by: Dirk Gouders > Tested-by: Sebastian Andrzej Siewior Acked-by: "Yann E. MORIN" It will be in my tree soon. Thanks! >>> >>> I don't see this patch in 3.16 but 3.16 does not have the issue any >>> more. Anyone has an idea how the issue got fixed? I am trying to find >>> the right patch to backport. >> >> With the above sample kconfig I still see the issue. How did you >> notice the issue got fixed? > > I did not pay much attention on the above sample kconfig, but just > focused on the USB gadget driver kconfig issue initially reported by > Sebastian. I saw the issue exists in 3.14, but does not in 3.16, > unless I messed up with my test. I will test 3.16 again some time next > week. Hi Bin, I now also re-tested the initially reported steps to reproduce the issue: > in USB gadget menu (that is Device Drivers ---> USB support ---> USB > Gadget Support ---> USB Gadget Drivers) I can create a configuration > which is "lost". Here is how to reproduce it: > > - first config two gadgets as M: > USB Gadget Drivers > Audio Gadget > Ethernet Gadget > MIDI Gadget > > save config & leave > > - now start menu config again and go to the same menu, now select > built-in: > <*> USB Gadget Drivers (Ethernet Gadget > the ethernet gadget is chosen automatically because we can have only > one gadget selected. > save config & leave > > - step three, go back to the menu and you will see that everything is > as it was (the <*> is ignored). Here, I still see the problem (I was wondering if the issue has been solved/gone by a kconfig-file modification). >>> >>> This issue was gone since 3.16, but came back again due to commit >>> 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules. >>> >> >> I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet) >> > > Issue is present on v4.6-rc1 as well and the $subject patch fixes the issue. I would say the issue is present until the code is fixed. Whether it is triggered depends on the Kconfig used. Perhaps, Michal could take a look at the patch that Yann ack'ed, some time ago. Dirk -- 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 v7 1/4] gadget: Introduce the usb charger framework
On 31 March 2016 at 18:06, Felipe Balbi wrote:
> Baolin Wang writes:
>> [ text/plain ]
>> On 31 March 2016 at 16:18, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Baolin Wang writes:
+#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
>>>
>>> According to the spec we should always be talking about unit loads (1
>>> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
>>> work for SS capable ports and SS gadgets (we have quite a few of them,
>>> actually). You're missing the opportunity of charging at 900mA.
>>
>> I follow the DCP/SDP/CDP/ACA type's default current limitation and
>> user can set them what they want.
>
> no, the user CANNOT set it to what they want. If you get enumerated
> @100mA and the user just decides to set it to 2000mA, s/he could even
> melt the USB connector. The kernel _must_ prevent such cases.
>
> In any case, DEFAULT_SDP_CUR_LIMIT shouldn't be a constant, it should be
> variable because if you enumerate in SS, you _can_ get up to 900mA.
Make sense. But these are just default values. They can be changed
safely by power driver with 'usb_charger_set_cur_limit_by_type()'
function to set 900mA.
>>>
>>> oh okay. Still, the default value should be a function of gadget->speed,
>>
>> Sorry, I did not get your suggestion, could you give me an example? Thanks.
>
> int default_current_limit = 500;
>
> if (gadget->speed >= USB_SPEED_SUPER)
> default_current_limit = 900;
Make sense.
>
+
+/* USB charger state */
+enum usb_charger_state {
+ USB_CHARGER_DEFAULT,
+ USB_CHARGER_PRESENT,
+ USB_CHARGER_REMOVE,
+};
>>>
>>> userland really doesn't need these two ?
>>
>> We've reported to userspace by kobject_uevent in
>> 'usb_charger_notify_others()' function.
>
> I mean as a type ;-) So userspace doesn't have to redefine these for
> their applications.
Make sense. I can introduce some sysfs files for userspace. Thanks for
your comments.
>>>
>>> okay, my reply was a bit cryptic, but what I mean here is that enum
>>> usb_charger_state could be moved your include/uapi header. My question
>>> is, then, does userland need to have knowledge of enum
>>> usb_charger_state ?
>>
>> I am not sure if userland need the enum usb_charger_state. But I
>> remember you want to report the charger state to userland in previous
>> email.
>
> right, which means this enumeration definition could be placed in the
> UAPI header. Unless, of course, we're reporting strings, rather than
> integers, in the sysfs file.
OK. Thanks.
>
> --
> balbi
--
Baolin.wang
Best Regards
--
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] uas: Add a new NO_REPORT_LUNS quirk
Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with
an usb-id of: 0bc2:331a, as these will fail to respond to a
REPORT_LUNS command.
Cc: sta...@vger.kernel.org
Reported-and-tested-by: David Webb
Signed-off-by: Hans de Goede
---
drivers/usb/storage/uas.c | 14 +-
drivers/usb/storage/unusual_uas.h | 7 +++
drivers/usb/storage/usb.c | 5 -
include/linux/usb_usual.h | 2 ++
4 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index c68e724147..5e15ed03 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -2,7 +2,7 @@
* USB Attached SCSI
* Note that this is not the same as the USB Mass Storage driver
*
- * Copyright Hans de Goede for Red Hat, Inc. 2013 - 2014
+ * Copyright Hans de Goede for Red Hat, Inc. 2013 - 2016
* Copyright Matthew Wilcox for Intel Corp, 2010
* Copyright Sarah Sharp for Intel Corp, 2010
*
@@ -781,6 +781,17 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
return SUCCESS;
}
+static int uas_target_alloc(struct scsi_target *starget)
+{
+ struct uas_dev_info *devinfo = (struct uas_dev_info *)
+ dev_to_shost(starget->dev.parent)->hostdata;
+
+ if (devinfo->flags & US_FL_NO_REPORT_LUNS)
+ starget->no_report_luns = 1;
+
+ return 0;
+}
+
static int uas_slave_alloc(struct scsi_device *sdev)
{
struct uas_dev_info *devinfo =
@@ -831,6 +842,7 @@ static struct scsi_host_template uas_host_template = {
.module = THIS_MODULE,
.name = "uas",
.queuecommand = uas_queuecommand,
+ .target_alloc = uas_target_alloc,
.slave_alloc = uas_slave_alloc,
.slave_configure = uas_slave_configure,
.eh_abort_handler = uas_eh_abort_handler,
diff --git a/drivers/usb/storage/unusual_uas.h
b/drivers/usb/storage/unusual_uas.h
index ccc113e..53341a7 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -64,6 +64,13 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x, 0x,
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_NO_ATA_1X),
+/* Reported-by: David Webb */
+UNUSUAL_DEV(0x0bc2, 0x331a, 0x, 0x,
+ "Seagate",
+ "Expansion Desk",
+ USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+ US_FL_NO_REPORT_LUNS),
+
/* Reported-by: Hans de Goede */
UNUSUAL_DEV(0x0bc2, 0x3320, 0x, 0x,
"Seagate",
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 43576ed..9de988a 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -482,7 +482,7 @@ void usb_stor_adjust_quirks(struct usb_device *udev,
unsigned long *fflags)
US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
- US_FL_MAX_SECTORS_240);
+ US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS);
p = quirks;
while (*p) {
@@ -532,6 +532,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev,
unsigned long *fflags)
case 'i':
f |= US_FL_IGNORE_DEVICE;
break;
+ case 'j':
+ f |= US_FL_NO_REPORT_LUNS;
+ break;
case 'l':
f |= US_FL_NOT_LOCKABLE;
break;
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index 7f5f78b..245f57d 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -79,6 +79,8 @@
/* Cannot handle MI_REPORT_SUPPORTED_OPERATION_CODES */ \
US_FLAG(MAX_SECTORS_240,0x0800) \
/* Sets max_sectors to 240 */ \
+ US_FLAG(NO_REPORT_LUNS, 0x1000) \
+ /* Cannot handle REPORT_LUNS */ \
#define US_FLAG(name, value) US_FL_##name = value ,
enum { US_DO_ALL_FLAGS };
--
2.7.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
[PATCH 1/1] net: usb: cdc_ncm: adding Telit LE910 V2 mobile broadband card
Telit LE910 V2 is a mobile broadband card with no ARP capabilities:
the patch makes this device to use wwan_noarp_info struct
Signed-off-by: Daniele Palmas
---
drivers/net/usb/cdc_ncm.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 86ba30b..2fb31ed 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1626,6 +1626,13 @@ static const struct usb_device_id cdc_devs[] = {
.driver_info = (unsigned long) &wwan_info,
},
+ /* Telit LE910 V2 */
+ { USB_DEVICE_AND_INTERFACE_INFO(0x1bc7, 0x0036,
+ USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long)&wwan_noarp_info,
+ },
+
/* DW5812 LTE Verizon Mobile Broadband Card
* Unlike DW5550 this device requires FLAG_NOARP
*/
--
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
[PATCH 0/1] net: usb: cdc_ncm: adding Telit LE910 V2 mobile broadband card
Telit LE910 V2 is a mobile broadband card with no ARP capabilities: the patch makes this device to use wwan_noarp_info struct. Following the device details: Bus 001 Device 037: ID 1bc7:0036 Telit Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 ? bDeviceProtocol 1 Interface Association bMaxPacketSize064 idVendor 0x1bc7 Telit idProduct 0x0036 bcdDevice 17.30 iManufacturer 1 Telit iProduct2 FIH7160 iSerial 3 351622079900094 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 490 bNumInterfaces 14 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower 100mA Interface Association: bLength 8 bDescriptorType11 bFirstInterface 0 bInterfaceCount 2 bFunctionClass 2 Communications bFunctionSubClass 2 Abstract (modem) bFunctionProtocol 1 AT-commands (v.25ter) iFunction 4 CDC ACM Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 2 Communications bInterfaceSubClass 2 Abstract (modem) bInterfaceProtocol 1 AT-commands (v.25ter) iInterface 5 CDC ACM CDC Header: bcdCDC 1.20 CDC Union: bMasterInterface0 bSlaveInterface 1 CDC Call Management: bmCapabilities 0x00 bDataInterface 1 CDC ACM: bmCapabilities 0x07 sends break line coding and serial state get/set/clear comm features Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 4 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber1 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass10 CDC Data bInterfaceSubClass 0 Unused bInterfaceProtocol 0 iInterface 6 Data Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Interface Association: bLength 8 bDescriptorType11 bFirstInterface 2 bInterfaceCount 2 bFunctionClass 2 Communications bFunctionSubClass 2 Abstract (modem) bFunctionProtocol 1 AT-commands (v.25ter) iFunction 7 CDC ACM Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber2 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 2 Communications bInterfaceSubClass 2 Abstract (modem) bInterfaceProtocol 1 AT-commands (v.25ter) iInterface 8 CDC ACM CDC Header: bcdCDC 1.20 CDC Union: bMasterInterface2 bSlaveInterface 3 CDC Call Management: bmCapabilities 0x00 bDataInterface 3 CDC ACM: bmCapabilities 0x07 sends break line coding and serial state get/set/clear comm features Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040
Re: [PATCH v5 1/2] USB: serial: cp210x: Adding GPIO support for CP2105
On Tue, Mar 29, 2016 at 6:47 PM, Martyn Welch wrote: > This patch adds support for the GPIO found on the CP2105. Unlike the GPIO > provided by some of the other devices supported by the cp210x driver, the > GPIO on the CP2015 is muxed on pins otherwise used for serial control > lines. The GPIO have been configured in 2 separate banks as the choice to > configure the pins for GPIO is made separately for pins shared with each > of the 2 serial ports this device provides, though the choice is made for > all pins associated with that port in one go. The choice of whether to use > the pins for GPIO or serial is made by adding configuration to a one-time > programable PROM in the chip and can not be changed at runtime. The device > defaults to GPIO. > > This device supports either push-pull or open-drain modes, it doesn't > provide an explicit input mode, though the state of the GPIO can be read > when used in open-drain mode. Like with pin use, the mode is configured in > the one-time programable PROM and can't be changed at runtime. > > Signed-off-by: Martyn Welch Looking good from a GPIO point of view. Acked-by: Linus Walleij Yours, Linus Walleij -- 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
To Your Attention
To Your Attention, On behalf of the board and management of International Monetary Committees it was resolved by the federal executive council and agreed that your Inheritance/Contract Fund valued at $10.5M would be released to you via DIPLOMATIC CHANNEL immediately your attention is received so you are advice to confirm the following information.send your reply to: elderifr...@gmail.com Your Name,=== Address,=== Next of kin.== Your Occupation= A scanned copy of your ID or passport.= Direct TelePhone Number=== Regards Elder Idris Frank Federal Ministry of Finance Email:elderifr...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
On 30/03/16 21:44, John Youn wrote: > On 3/23/2016 11:52 PM, Felipe Balbi wrote: >> >> Hi, >> >> John Youn writes: >>> [ text/plain ] >>> On 3/21/2016 11:40 PM, Felipe Balbi wrote: Hi, John Youn writes: > [ text/plain ] > On 3/18/2016 12:17 PM, John Youn wrote: >> On 3/16/2016 6:56 AM, Felipe Balbi wrote: >>> >>> heh, +john >>> >>> Felipe Balbi writes: [ text/plain ] Hi, Roger Quadros writes: > [ text/plain ] > We will need this function for a workaround. > The function issues a softreset only to the device > controller and performs minimal re-initialization > so that the device controller can be usable. > > As some code is similar to dwc3_core_init() take out > common code into dwc3_get_gctl_quirks(). > > We add a new member (prtcap_mode) to struct dwc3 to > keep track of the current mode in the PRTCAPDIR register. > > Signed-off-by: Roger Quadros I must say, I don't like this at all :-p There's ONE known silicon which needs this because of a poor silicon integration which took an IP with a known erratum where it can't be made to work on lower speeds and STILL was integrated without a superspeed PHY. There's a reason why I never tried to push this upstream myself ;-) I'm really thinking we might be better off adding a quirk flag to skip the metastability workaround and allow this ONE silicon to set the controller to lower speed. John, can you check with your colleagues if we would ever fall into STAR#9000525659 if we set maximum speed to high speed during driver probe and never touch it again ? I would assume we don't really fall into the metastability workaround, right ? We're not doing any sort of PM for dwc3... > > Hi Felipe, > > Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS? > I don't see an issue with that as long as we always ignore > dwc->maximum_speed when programming DCFG.speed for all affected > versions of the core. As long as the DCFG.speed = SS, you should not > hit the STAR. I actually mean changing DCFG.speed during driver probe and never touching it again. Would that still cause problems ? >>> >>> In that case I'm not sure. The engineer who would know is off until >>> next week so I'll get back to you as soon as I can talk to him about >>> it. >> > > So the engineers said that this issue can occur while set to HS and > the run/stop bit is changed so it seems that won't work. Thanks John. Felipe, any suggestion how we can fix this upstream? cheers, -roger -- 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] uas: Add a new NO_REPORT_LUNS quirk
On Thu, 31 Mar 2016, Hans de Goede wrote: > Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with > an usb-id of: 0bc2:331a, as these will fail to respond to a > REPORT_LUNS command. > @@ -532,6 +532,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, > unsigned long *fflags) > case 'i': > f |= US_FL_IGNORE_DEVICE; > break; > + case 'j': > + f |= US_FL_NO_REPORT_LUNS; > + break; > case 'l': > f |= US_FL_NOT_LOCKABLE; > break; You forgot to document this new module parameter flag in Documentation/kernel-parameters.txt. 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] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
On 31.03.2016 06:51, Rajesh Bhagat wrote:
Hello Mathias,
Thanks a lot for your support.
Attached patch is working and allows the completion handler to be called. And
resume operation completes
and shell comes back (but with lot of errors).
Now, some other points which needs our attention here are:
1. usb core hub code is not checking the error status of
hcd->driver->reset_device(xhci_discover_or_reset_device)
and continues considering reset_device was successful (and causes other
issues). Hence, a check is needed on return
value of reset_device and proper action is required on it.
2. Even if above return value is checked and reset_device status is used. USB
core hub retries for N times which is not
required in this case as adding to the resume operation time. But if in
this case we return -ENOTCONN instead of -EINVAL
from hcd->driver->reset_device(xhci_discover_or_reset_device), code
returns early and doesn't retry.
Proposed Solution for above with your suggested patches is as below:
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 7cad1fa..efeba31 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2807,13 +2807,17 @@ done:
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
update_devnum(udev, 0);
- /* The xHC may think the device is already reset,
-* so ignore the status.
+ /*
+* check the status of resetting the device and update
+* the udev status.
*/
if (hcd->driver->reset_device)
- hcd->driver->reset_device(hcd, udev);
+ status = hcd->driver->reset_device(hcd, udev);
- usb_set_device_state(udev, USB_STATE_DEFAULT);
+ if (status == 0)
+ usb_set_device_state(udev, USB_STATE_DEFAULT);
+ else
+ usb_set_device_state(udev,
USB_STATE_NOTATTACHED);
}
} else {
if (udev)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b3a0a22..9427175f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -310,6 +310,10 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
return -ESHUTDOWN;
}
+ /* writing the CMD_RING_ABORT bit should create a command completion
+* event, add a command completion timeout for it as well
+*/
+ mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT);
return 0;
}
@@ -1243,6 +1247,7 @@ void xhci_handle_command_timeout(unsigned long data)
int ret;
unsigned long flags;
u64 hw_ring_state;
+ u32 old_status;
struct xhci_command *cur_cmd = NULL;
xhci = (struct xhci_hcd *) data;
@@ -1250,6 +1255,7 @@ void xhci_handle_command_timeout(unsigned long data)
spin_lock_irqsave(&xhci->lock, flags);
if (xhci->current_cmd) {
cur_cmd = xhci->current_cmd;
+ old_status = cur_cmd->status;
cur_cmd->status = COMP_CMD_ABORT;
}
@@ -1272,7 +1278,15 @@ void xhci_handle_command_timeout(unsigned long data)
}
/* command timeout on stopped ring, ring can't be aborted */
xhci_dbg(xhci, "Command timeout on stopped ring\n");
- xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
+
+ /* is this the second timeout for the same command? ring wont restart */
+ if (old_status == COMP_CMD_ABORT) {
+ xhci_err(xhci, "Abort timeout, Fail to restart cmd ring\n");
+ xhci_cleanup_command_queue(xhci);
+ /* everything else here to halt, cleanup, free etc kill xhc */
+ } else
+ xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
+
spin_unlock_irqrestore(&xhci->lock, flags);
return;
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c502c22..bd18966 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3450,7 +3450,7 @@ int xhci_discover_or_reset_device(struct usb_hcd *hcd,
struct usb_device *udev)
if (ret == 1)
return 0;
else
- return -EINVAL;
+ return -ENOTCONN; /* Don't retry */
}
if (virt_dev->tt_info)
Sample output after above patch (timer is set as wakeup source):
root@phoenix:~# echo mem > /sys/power/state
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.001 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
PM: suspend of devices complete after 155.658 msecs
PM: late suspend of devices complete after 1.594 msecs
PM: noirq suspend
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
Hi, Roger Quadros writes: >> We will need this function for a workaround. >> The function issues a softreset only to the device >> controller and performs minimal re-initialization >> so that the device controller can be usable. >> >> As some code is similar to dwc3_core_init() take out >> common code into dwc3_get_gctl_quirks(). >> >> We add a new member (prtcap_mode) to struct dwc3 to >> keep track of the current mode in the PRTCAPDIR register. >> >> Signed-off-by: Roger Quadros > > I must say, I don't like this at all :-p There's ONE known silicon > which > needs this because of a poor silicon integration which took an IP > with a > known erratum where it can't be made to work on lower speeds and STILL > was integrated without a superspeed PHY. > > There's a reason why I never tried to push this upstream myself ;-) > > I'm really thinking we might be better off adding a quirk flag to skip > the metastability workaround and allow this ONE silicon to set the > controller to lower speed. > > John, can you check with your colleagues if we would ever fall into > STAR#9000525659 if we set maximum speed to high speed during driver > probe and never touch it again ? I would assume we don't really fall > into the metastability workaround, right ? We're not doing any sort of > PM for dwc3... > >> >> Hi Felipe, >> >> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS? >> I don't see an issue with that as long as we always ignore >> dwc->maximum_speed when programming DCFG.speed for all affected >> versions of the core. As long as the DCFG.speed = SS, you should not >> hit the STAR. > > I actually mean changing DCFG.speed during driver probe and never > touching it again. Would that still cause problems ? > In that case I'm not sure. The engineer who would know is off until next week so I'll get back to you as soon as I can talk to him about it. >>> >> >> So the engineers said that this issue can occur while set to HS and >> the run/stop bit is changed so it seems that won't work. > > Thanks John. > > Felipe, > > any suggestion how we can fix this upstream? no idea, I don't have a lot of memory about this problem. I really don't remember the details about this, is there an openly available errata document which I could read ? /me goes search for it. I found [1] which tells me, the following: | i819| A Device Control Bit Meta-Stability for USB3.0 Controller in USB2.0 Mode | |-+| | Criticality | Medium | | | | | Descritiion | When USB3.0 controller core is programmed to be a USB 2.0-only device | | | hardware meta-stability on USB_DCTL[31]RUNSTOP bit causing the core to | | | attempt high speed as well as SuperSpeed connection or completely miss | | | the attach request. | | | | | Workaround | If the requirement is to always function in USB 2.0 mode, there is no | | | workaround. | | | Otherwise, you can always program the USB controller core to be SuperSpeed | | | 3.0 capable (USB_DCFG[2:0]DEVSPD = 0x4) | | | | | Revisions | SR 1.1, 2.0 | | Impacted| | |-+| So, TI's own documentation says that there is _no_ workaround. My question is, then: How are you sure that resetting the device actually solves the issue ? Did you really hit the metastability problem and noted that it works after a soft-reset ? How did you verify that Run/Stop was in a metastable state, considering that Run/Stop signal is not visible outside the die ? It seems to me that resetting the IP is just as "dangerous" as setting the IP to High-speed in the first place. No ? [1] http://www.ti.com/lit/er/sprz429h/sprz429h.pdf -- balbi signature.asc Description: PGP signature
Re: [PATCH] uas: Add a new NO_REPORT_LUNS quirk
On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: > Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with > an usb-id of: 0bc2:331a, as these will fail to respond to a > REPORT_LUNS command. Actually, if we're sending them a report luns command, they must be reporting in at SCSI-3 SPC or higher. Should we be quirking them down to SCSI-2 instead because it reduces the risk of running into something else they're not doing from the SPC command set? James -- 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] uas: Add a new NO_REPORT_LUNS quirk
Hi, On 31-03-16 16:48, James Bottomley wrote: On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with an usb-id of: 0bc2:331a, as these will fail to respond to a REPORT_LUNS command. Actually, if we're sending them a report luns command, they must be reporting in at SCSI-3 SPC or higher. Should we be quirking them down to SCSI-2 instead because it reduces the risk of running into something else they're not doing from the SPC command set? These are fairly new devices, so they should really be scsi3, but the usb <-> sata bridge (presumably) used does not seem to like report_luns. Note that usb-storage simple sets no_report_luns conditionally for all usb-storage devices. The scsi people have repeatedly asked me to not do this kinda blanket blacklisting for uas devices, because they hope that uas will allow them to more or less do proper scsi over usb, so we end up with blacklisting specific commands every now and then to get devices to work. Regards, Hans -- 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 2/2] usb:dwc3: pass arch data to xhci-hcd child
On 03/31/2016 11:04 AM, Felipe Balbi wrote:
> "Thang Q. Nguyen" writes:
>> [ text/plain ]
>> Thanks Grygorii for information.
>> I checked but do not see dma_init_dev_from_parent is used in
>> linux-next repository. Can you give me more information for what
>> branch I can checkout to use it for USB DWC3?
>
> dma_init_dev_from_parent() is still a proposal ;-)
>
Felipe,
After some experiments I came up with below fix (not common, but fixes USB
case on keystone 2). if you agree with proposed fix I'll send proper
patches to fix usb_add_gadget_udc_release() and dwc3_host_init() in the same
way.
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index c679f63..3fe1c65 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -17,6 +17,7 @@
#include
#include
+#include
#include "core.h"
@@ -35,8 +36,6 @@ int dwc3_host_init(struct dwc3 *dwc)
dma_set_coherent_mask(&xhci->dev, dwc->dev->coherent_dma_mask);
xhci->dev.parent= dwc->dev;
- xhci->dev.dma_mask = dwc->dev->dma_mask;
- xhci->dev.dma_parms = dwc->dev->dma_parms;
dwc->xhci = xhci;
@@ -62,6 +61,12 @@ int dwc3_host_init(struct dwc3 *dwc)
phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
dev_name(&xhci->dev));
+ if (!dwc->dev->of_node) {
+ xhci->dev.dma_mask = dwc->dev->dma_mask;
+ xhci->dev.dma_parms = dwc->dev->dma_parms;
+ } else
+ of_dma_configure(&xhci->dev, dwc->dev->of_node);
+
ret = platform_device_add(xhci);
if (ret) {
dev_err(dwc->dev, "failed to register xHCI device\n");
--
regards,
-grygorii
--
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] uas: Add a new NO_REPORT_LUNS quirk
On Thu, 2016-03-31 at 17:03 +0200, Hans de Goede wrote: > Hi, > > On 31-03-16 16:48, James Bottomley wrote: > > On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: > > > Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with > > > an usb-id of: 0bc2:331a, as these will fail to respond to a > > > REPORT_LUNS command. > > > > Actually, if we're sending them a report luns command, they must be > > reporting in at SCSI-3 SPC or higher. Should we be quirking them > > down to SCSI-2 instead because it reduces the risk of running into > > something else they're not doing from the SPC command set? > > These are fairly new devices, so they should really be scsi3, but the > usb <-> sata bridge (presumably) used does not seem to like > report_luns. That's what I'm questioning: REPORT LUNS is one of the big SCSI-3 changes, if they don't support that, it really looks like someone picked up an old engine and just fuzzed the inquiry data to return SCSI -3. In which case we should put it back to SCSI-2 where it belongs. Also, if it's USB<->SCSI bridge, that isn't really UAS, is it? > Note that usb-storage simple sets no_report_luns conditionally for > all usb-storage devices. The scsi people have repeatedly asked me to > not do this kinda blanket blacklisting for uas devices, because they > hope that uas will allow them to more or less do proper scsi over > usb, so we end up with blacklisting specific commands every now and > then to get devices to work. Well, we were hoping that with UAS the USB device creators would actually learn what a standard was when it bit them, yes. The fact that Seagate can release a SCSI-3 UAS device that doesn't do REPORT LUNS kind of dashes that hope. James -- 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] uas: Add a new NO_REPORT_LUNS quirk
Hi, On 31-03-16 17:11, James Bottomley wrote: On Thu, 2016-03-31 at 17:03 +0200, Hans de Goede wrote: Hi, On 31-03-16 16:48, James Bottomley wrote: On Thu, 2016-03-31 at 14:22 +0200, Hans de Goede wrote: Add a new NO_REPORT_LUNS quirk and set it for Seagate drives with an usb-id of: 0bc2:331a, as these will fail to respond to a REPORT_LUNS command. Actually, if we're sending them a report luns command, they must be reporting in at SCSI-3 SPC or higher. Should we be quirking them down to SCSI-2 instead because it reduces the risk of running into something else they're not doing from the SPC command set? These are fairly new devices, so they should really be scsi3, but the usb <-> sata bridge (presumably) used does not seem to like report_luns. That's what I'm questioning: REPORT LUNS is one of the big SCSI-3 changes, if they don't support that, it really looks like someone picked up an old engine and just fuzzed the inquiry data to return SCSI -3. In which case we should put it back to SCSI-2 where it belongs. Actually it does support REPORT LUNS, some of the time. When you first boot the computer with uas blacklisted for this device, so initialize it once with usb-storage, and then reboot with out the blacklist (and without removing power to the drive) uas will work with REPORT LUNS bit cold-booting directly into uas mode and then doing a REPORT LUNS upsets the drive / disk enclosure (this has all been observed by David Webb, I do not own such a drive). Also, if it's USB<->SCSI bridge, that isn't really UAS, is it? I assume you mean that a USB<->sata bridge is not really a SCSI device but more of a scsi emulating device. I'm not going to argue that, but all devices talking the uas protocol I've seen sofar are USB<->sata bridges. Note that usb-storage simple sets no_report_luns conditionally for all usb-storage devices. The scsi people have repeatedly asked me to not do this kinda blanket blacklisting for uas devices, because they hope that uas will allow them to more or less do proper scsi over usb, so we end up with blacklisting specific commands every now and then to get devices to work. Well, we were hoping that with UAS the USB device creators would actually learn what a standard was when it bit them, yes. The fact that Seagate can release a SCSI-3 UAS device that doesn't do REPORT LUNS kind of dashes that hope. See above it does sortof do REPORT LUNS just not reliable (and thus not usable). Also for some reason Seagate seems to be particularly bad in their uas implementation, we have a ton of quirks for Seagate uas devices in drivers/usb/storage/unusual_uas.h, so far all of them stop responding until reset after ATA_12 or ATA_16 CDBs so we filter those out. This REPORT LUNS issue is new. Regards, Hans -- 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: need to resubmit serial fixes?
On Thu, Mar 31, 2016 at 09:56:34AM +0200, Oliver Neukum wrote: > Hi, > > do I need to resubmit the serial fixes I submitted for > the issues with the forged device descriptors leading > to crashes? No, that's not necessary. I'm on the road for another couple of days but I'll forward these to Greg before processing the rest of my queue. Found an issue with the digi patch that I fixed up. Thanks for fixing this. 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 v2] digi_acceleport: do sanity checking for the number of ports
On Mon, Mar 21, 2016 at 03:57:37PM +0100, Oliver Neukum wrote:
> The driver can be crashed with devices that expose crafted
> descriptors with too few endpoints.
> See:
> http://seclists.org/bugtraq/2016/Mar/61
>
> Signed-off-by: Oliver Neukum
>
> v1 - added sanity checks
> v2 - moved them to probe() to fix problems Johan pointed out
> ---
> drivers/usb/serial/digi_acceleport.c | 24 +++-
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/serial/digi_acceleport.c
> b/drivers/usb/serial/digi_acceleport.c
> index 12b0e67..dab1dcf 100644
> --- a/drivers/usb/serial/digi_acceleport.c
> +++ b/drivers/usb/serial/digi_acceleport.c
> @@ -1252,7 +1252,8 @@ static int digi_port_init(struct usb_serial_port *port,
> unsigned port_num)
> static int digi_startup(struct usb_serial *serial)
> {
> struct digi_serial *serial_priv;
> - int ret;
> + int ret = -ENODEV;
> + int i;
>
> serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
> if (!serial_priv)
> @@ -1260,18 +1261,31 @@ static int digi_startup(struct usb_serial *serial)
>
> spin_lock_init(&serial_priv->ds_serial_lock);
> serial_priv->ds_oob_port_num = serial->type->num_ports;
> +
> + /* Check whether the expected number of ports matches the device */
> + if (serial->num_ports < serial_priv->ds_oob_port_num)
> + goto error;
This should be
if (serial->num_port_pointers < serial->type->num_ports + 1)
as serial->num_ports will (generally) equal serial->type->num_ports, and
we need to check that we got one more port structure than we requested.
I fixed that up and moved the check above the private-data allocation.
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
[PATCH 3/3] USB: digi_acceleport: do sanity checking for the number of ports
From: Oliver Neukum
The driver can be crashed with devices that expose crafted descriptors
with too few endpoints.
See: http://seclists.org/bugtraq/2016/Mar/61
Signed-off-by: Oliver Neukum
[johan: fix OOB endpoint check and add error messages ]
Cc: stable
Signed-off-by: Johan Hovold
---
drivers/usb/serial/digi_acceleport.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/serial/digi_acceleport.c
b/drivers/usb/serial/digi_acceleport.c
index 010a42a92688..16e8e37b3b36 100644
--- a/drivers/usb/serial/digi_acceleport.c
+++ b/drivers/usb/serial/digi_acceleport.c
@@ -1251,8 +1251,27 @@ static int digi_port_init(struct usb_serial_port *port,
unsigned port_num)
static int digi_startup(struct usb_serial *serial)
{
+ struct device *dev = &serial->interface->dev;
struct digi_serial *serial_priv;
int ret;
+ int i;
+
+ /* check whether the device has the expected number of endpoints */
+ if (serial->num_port_pointers < serial->type->num_ports + 1) {
+ dev_err(dev, "OOB endpoints missing\n");
+ return -ENODEV;
+ }
+
+ for (i = 0; i < serial->type->num_ports + 1 ; i++) {
+ if (!serial->port[i]->read_urb) {
+ dev_err(dev, "bulk-in endpoint missing\n");
+ return -ENODEV;
+ }
+ if (!serial->port[i]->write_urb) {
+ dev_err(dev, "bulk-out endpoint missing\n");
+ return -ENODEV;
+ }
+ }
serial_priv = kzalloc(sizeof(*serial_priv), GFP_KERNEL);
if (!serial_priv)
--
2.7.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
[PATCH 2/3] USB: cypress_m8: add endpoint sanity check
From: Oliver Neukum
An attack using missing endpoints exists.
CVE-2016-3137
Signed-off-by: Oliver Neukum
CC: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/usb/serial/cypress_m8.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index b283eb8b86d6..bbeeb2bd55a8 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -447,6 +447,11 @@ static int cypress_generic_port_probe(struct
usb_serial_port *port)
struct usb_serial *serial = port->serial;
struct cypress_private *priv;
+ if (!port->interrupt_out_urb || !port->interrupt_in_urb) {
+ dev_err(&port->dev, "required endpoint is missing\n");
+ return -ENODEV;
+ }
+
priv = kzalloc(sizeof(struct cypress_private), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -606,12 +611,6 @@ static int cypress_open(struct tty_struct *tty, struct
usb_serial_port *port)
cypress_set_termios(tty, port, &priv->tmp_termios);
/* setup the port and start reading from the device */
- if (!port->interrupt_in_urb) {
- dev_err(&port->dev, "%s - interrupt_in_urb is empty!\n",
- __func__);
- return -1;
- }
-
usb_fill_int_urb(port->interrupt_in_urb, serial->dev,
usb_rcvintpipe(serial->dev, port->interrupt_in_endpointAddress),
port->interrupt_in_urb->transfer_buffer,
--
2.7.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
[PATCH 0/3] USB: serial: add missing endpoint checks
Greg, These patches from Oliver add some sanity checks for missing endpoints and should go into v4.6-rc2. Would you mind picking these up directly? I expect to process the rest of my queue early next week. Note that this series has been compile-tested only. Thanks, Johan Oliver Neukum (3): USB: mct_u232: add sanity checking in probe USB: cypress_m8: add endpoint sanity check USB: digi_acceleport: do sanity checking for the number of ports drivers/usb/serial/cypress_m8.c | 11 +-- drivers/usb/serial/digi_acceleport.c | 19 +++ drivers/usb/serial/mct_u232.c| 9 - 3 files changed, 32 insertions(+), 7 deletions(-) -- 2.7.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
[PATCH 1/3] USB: mct_u232: add sanity checking in probe
From: Oliver Neukum
An attack using the lack of sanity checking in probe is known. This
patch checks for the existence of a second port.
CVE-2016-3136
Signed-off-by: Oliver Neukum
CC: sta...@vger.kernel.org
[johan: add error message ]
Signed-off-by: Johan Hovold
---
drivers/usb/serial/mct_u232.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/mct_u232.c b/drivers/usb/serial/mct_u232.c
index 4446b8d70ac2..885655315de1 100644
--- a/drivers/usb/serial/mct_u232.c
+++ b/drivers/usb/serial/mct_u232.c
@@ -376,14 +376,21 @@ static void mct_u232_msr_to_state(struct usb_serial_port
*port,
static int mct_u232_port_probe(struct usb_serial_port *port)
{
+ struct usb_serial *serial = port->serial;
struct mct_u232_private *priv;
+ /* check first to simplify error handling */
+ if (!serial->port[1] || !serial->port[1]->interrupt_in_urb) {
+ dev_err(&port->dev, "expected endpoint missing\n");
+ return -ENODEV;
+ }
+
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
/* Use second interrupt-in endpoint for reading. */
- priv->read_urb = port->serial->port[1]->interrupt_in_urb;
+ priv->read_urb = serial->port[1]->interrupt_in_urb;
priv->read_urb->context = port;
spin_lock_init(&priv->lock);
--
2.7.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
Re: musb: kernel 3.18.29 build issue
On Thu, Mar 31, 2016 at 07:29:21AM +0200, Yegor Yefremov wrote: > Hello Greg, > > 3.18.x is missing this patch [1] in order to perform a successful > build. The reason: following patch [2] in 3.18.x uses a macro name > introduced in the missing patch [1]. > > Could you queue it for 3.18.30? Nope, I have nothing to do with 3.18-stable, sorry. Please read Documentation/stable_kernel_rules.txt for how to get patches into stable kernel releases. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] USB: serial: add missing endpoint checks
On Thu, Mar 31, 2016 at 12:04:23PM -0400, Johan Hovold wrote: > Greg, > > These patches from Oliver add some sanity checks for missing endpoints > and should go into v4.6-rc2. > > Would you mind picking these up directly? I expect to process the rest > of my queue early next week. Now applied, thanks for sending me these. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB gadgets with configfs hang reboot
On 30.03.2016 22:25, Alan Stern wrote:
On Wed, 30 Mar 2016, Ivaylo Dimitrov wrote:
seems to be created twice :). Maybe the problem is that the first time
musb-hdrc is probed it fails with -EPROBE_DEFER, however that failure is
after gadget drivers got loaded and noone unloads them.
gadget drivers will get added to a pending list, then later they'll
bind. But they shouldn't bind() twice, unless there are multiple
interfaces for them.
Well, then it seems we have problem, as the 2 unbind() calls are with
one and the same "common" pointer (again, from memory).
Just some wild guesses based on my memories as I've lost the logs (power
outage). For sure I can recreate them if needed.
okay.
I will redo dump_stack() and printks and will provide logs as soon as I
have some time, so to stop counting on my memories.
Please find attached the relevant logs. It really seems that g_nokia is
probed twice, with all the gadgets in it created two times. I am
starting to suspect 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb:
gadget: udc-core: independent registration of gadgets and gadget
drivers") has something to do with the problem, though reverting it
resulted in g_nokia not being probed at all :)
The problem is not caused by nokia_bind() getting called twice. The
log clearly shows that nokia_bind() is called only once, but it calls
usb_add_config() from two different places:
Jan 1 02:00:10 Nokia-N900 kernel: [8.002838] [] (usb_add_config) from
[] (nokia_bind+0x160/0x2f0)
Jan 1 02:00:10 Nokia-N900 kernel: [8.014526] [] (nokia_bind) from
[] (composite_bind+0x68/0x1a0)
...
Jan 1 02:00:10 Nokia-N900 kernel: [8.381286] [] (usb_add_config) from
[] (nokia_bind+0x178/0x2f0)
Jan 1 02:00:10 Nokia-N900 kernel: [8.394348] [] (nokia_bind) from
[] (composite_bind+0x68/0x1a0)
Everything else along the two pathways is the same, so this is where
the multiple binds come from. And indeed, looking at the code in
nokia_bind() you can see the two calls:
/* finally register the configuration */
status = usb_add_config(cdev, &nokia_config_500ma_driver,
nokia_bind_config);
if (status < 0)
goto err_msg_luns;
status = usb_add_config(cdev, &nokia_config_100ma_driver,
nokia_bind_config);
Right, somehow I've overslept there are two different nokia_bind+XXX values.
This isn't supposed to cause any problems. The two instances should
never run at the same time, because they belong to different configs.
The problem seems to be that fsg driver creates a thread for every
fsg_bind() call, which overwrites common->thread_task without first
checking if a thread is already created. I don't know the idea behind -
should it have only one thread for all instances, or there should be a
thread per instance, but anyway the current implementation looks wrong.
Ivo
--
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 gadgets with configfs hang reboot
On Thu, 31 Mar 2016, Ivaylo Dimitrov wrote: > > This isn't supposed to cause any problems. The two instances should > > never run at the same time, because they belong to different configs. > > > > The problem seems to be that fsg driver creates a thread for every > fsg_bind() call, which overwrites common->thread_task without first > checking if a thread is already created. I don't know the idea behind - > should it have only one thread for all instances, or there should be a > thread per instance, but anyway the current implementation looks wrong. You've put your finger on the problem. Michal, I'm not sure how you intended to handle this. We only need to have one thread per interface (even if there's more than one LUN), but if there are multiple mass-storage interfaces in one configuration then you might want to have multiple threads. Of course, then you would have to handle cases where one config has one mass-storage interface and another config has two... It's your decision, but clearly the driver needs to be fixed. 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 v7 1/4] gadget: Introduce the usb charger framework
On Thu, Mar 31, 2016 at 09:42:58AM +0300, Felipe Balbi wrote: > Baolin Wang writes: > > I want to use bus structure to manage the charger device. Maybe choose > > class to manage them? > I guess a class would fit better in this case. IIRC Greg didn't want new classes? signature.asc Description: PGP signature
Re: [PATCH] uas: Add a new NO_REPORT_LUNS quirk
Hi, > Actually it does support REPORT LUNS, some of the time. When you first > boot the computer with uas blacklisted for this device, so initialize > it once with usb-storage, and then reboot with out the blacklist > (and without removing power to the drive) uas will work with REPORT LUNS > bit cold-booting directly into uas mode and then doing a REPORT LUNS > upsets the drive / disk enclosure (this has all been observed by > David Webb, I do not own such a drive). Just to confirm what Hans has reported. After power has been removed the Seagate Expansion usb disk always produces faults unless it has been blacklisted in some way. Once the disk is working the computer can be powered off and restarted without the blacklist and, as long as its power has not been removed, the disk can be reconnected many times without any error. With Hans's changes the disk mounts correctly with the uas module every time. My guess is that one of the interface registers is not or cannot be initialized correctly. If after a failure I try unplugging and reinserting the usb connector many times it has sometimes connected correctly - which to me means that some random bit eventually has the right value. Regards, David Webb. -- 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] Input: gtco: fix crash on detecting device without endpoints
On Fri, Mar 18, 2016 at 07:35:00PM +0100, Vladis Dronov wrote:
> The gtco driver expects at least one valid endpoint. If given
> malicious descriptors that specify 0 for the number of endpoints,
> it will crash in the probe function. Ensure there is at least
> one endpoint on the interface before using it. Fix minor coding
> style issue.
>
> The full report of this issue can be found here:
> http://seclists.org/bugtraq/2016/Mar/86
>
> Reported-by: Ralf Spenneberg
> Signed-off-by: Vladis Dronov
Applied, thank you.
> ---
> drivers/input/tablet/gtco.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
> index 3a7f3a4..7c18249 100644
> --- a/drivers/input/tablet/gtco.c
> +++ b/drivers/input/tablet/gtco.c
> @@ -858,6 +858,14 @@ static int gtco_probe(struct usb_interface *usbinterface,
> goto err_free_buf;
> }
>
> + /* Sanity check that a device has an endpoint */
> + if (usbinterface->altsetting[0].desc.bNumEndpoints < 1) {
> + dev_err(&usbinterface->dev,
> + "Invalid number of endpoints\n");
> + error = -EINVAL;
> + goto err_free_urb;
> + }
> +
> /*
>* The endpoint is always altsetting 0, we know this since we know
>* this device only has one interrupt endpoint
> @@ -879,7 +887,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
>* HID report descriptor
>*/
> if (usb_get_extra_descriptor(usbinterface->cur_altsetting,
> - HID_DEVICE_TYPE, &hid_desc) != 0){
> + HID_DEVICE_TYPE, &hid_desc) != 0) {
> dev_err(&usbinterface->dev,
> "Can't retrieve exta USB descriptor to get hid report
> descriptor length\n");
> error = -EIO;
> --
> 2.5.0
--
Dmitry
--
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/4] tablet: remove private copy to USB device
On Tue, Mar 22, 2016 at 04:04:53PM +0100, Oliver Neukum wrote: > We now have a macro to easily get to the USB device from the interface. > So we are cleaning up all drivers to not store a private pointer. Applied all 4, thank you. > > Oliver Neukum (4): > acecad: stop saving struct usb_device > aiptek: stop saving struct usb_device > gtco: stop saving struct usb_device > kbtab: stop saving struct usb_device > > drivers/input/tablet/acecad.c | 12 ++-- > drivers/input/tablet/aiptek.c | 18 +- > drivers/input/tablet/gtco.c | 24 > drivers/input/tablet/kbtab.c | 8 > 4 files changed, 31 insertions(+), 31 deletions(-) > > -- > 2.1.4 > -- Dmitry -- 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/4] acecad: stop saving struct usb_device
On Tue, Mar 22, 2016 at 04:04:54PM +0100, Oliver Neukum wrote:
> static int usb_acecad_open(struct input_dev *dev)
> {
> struct usb_acecad *acecad = input_get_drvdata(dev);
>
> - acecad->irq->dev = acecad->usbdev;
> + acecad->irq->dev = interface_to_usbdev(acecad->intf);
By the way, do we still need to do this assignment before submitting
urb?
> if (usb_submit_urb(acecad->irq, GFP_KERNEL))
> return -EIO;
>
Thanks.
--
Dmitry
--
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: musb: kernel 3.18.29 build issue
Hi Greg, On Thu, Mar 31, 2016 at 09:06:10AM -0700, Greg KH wrote: > On Thu, Mar 31, 2016 at 07:29:21AM +0200, Yegor Yefremov wrote: > > Hello Greg, > > > > 3.18.x is missing this patch [1] in order to perform a successful > > build. The reason: following patch [2] in 3.18.x uses a macro name > > introduced in the missing patch [1]. > > > > Could you queue it for 3.18.30? > > Nope, I have nothing to do with 3.18-stable, sorry. > > Please read Documentation/stable_kernel_rules.txt for how to get patches > into stable kernel releases. The related discussion on v3.12 is [1], it seems for some reason commit cb83df7 has been back ported to stable, but the dependent patch 0149b07 is missed, which causes build error. Regards, -Bin. [1] http://marc.info/?l=linux-kernel&m=145691452421076&w=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
RE: Chipidea in device mode & VBUS
>If you find it can't work, there are may be something wrong at other places, >eg, the gadget driver is not >loaded successfully. Would you please help to >check more? Found the problem in my setup. The patch is working great now. Thanks, Svet. -- 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: musb: kernel 3.18.29 build issue
On Thu, Mar 31, 2016 at 02:10:59PM -0500, Bin Liu wrote: > Hi Greg, > > On Thu, Mar 31, 2016 at 09:06:10AM -0700, Greg KH wrote: > > On Thu, Mar 31, 2016 at 07:29:21AM +0200, Yegor Yefremov wrote: > > > Hello Greg, > > > > > > 3.18.x is missing this patch [1] in order to perform a successful > > > build. The reason: following patch [2] in 3.18.x uses a macro name > > > introduced in the missing patch [1]. > > > > > > Could you queue it for 3.18.30? > > > > Nope, I have nothing to do with 3.18-stable, sorry. > > > > Please read Documentation/stable_kernel_rules.txt for how to get patches > > into stable kernel releases. > > The related discussion on v3.12 is [1], it seems for some reason commit > cb83df7 has been back ported to stable, but the dependent patch 0149b07 > is missed, which causes build error. Again, that's nice, but I have nothing to do with 3.18-stable, so there's nothing I can do here, please read that file for how to get this resolved. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: musb: kernel 3.18.29 build issue
On Thu, Mar 31, 2016 at 12:48:39PM -0700, Greg KH wrote: > On Thu, Mar 31, 2016 at 02:10:59PM -0500, Bin Liu wrote: > > Hi Greg, > > > > On Thu, Mar 31, 2016 at 09:06:10AM -0700, Greg KH wrote: > > > On Thu, Mar 31, 2016 at 07:29:21AM +0200, Yegor Yefremov wrote: > > > > Hello Greg, > > > > > > > > 3.18.x is missing this patch [1] in order to perform a successful > > > > build. The reason: following patch [2] in 3.18.x uses a macro name > > > > introduced in the missing patch [1]. > > > > > > > > Could you queue it for 3.18.30? > > > > > > Nope, I have nothing to do with 3.18-stable, sorry. > > > > > > Please read Documentation/stable_kernel_rules.txt for how to get patches > > > into stable kernel releases. > > > > The related discussion on v3.12 is [1], it seems for some reason commit > > cb83df7 has been back ported to stable, but the dependent patch 0149b07 > > is missed, which causes build error. > > Again, that's nice, but I have nothing to do with 3.18-stable, so > there's nothing I can do here, please read that file for how to get this > resolved. Sorry, I should have checked first - you are not maintaining 3.18-stable. -- 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: musb: kernel 3.18.29 build issue
On Thu, Mar 31, 2016 at 02:58:47PM -0500, Bin Liu wrote: > On Thu, Mar 31, 2016 at 12:48:39PM -0700, Greg KH wrote: > > On Thu, Mar 31, 2016 at 02:10:59PM -0500, Bin Liu wrote: > > > Hi Greg, > > > > > > On Thu, Mar 31, 2016 at 09:06:10AM -0700, Greg KH wrote: > > > > On Thu, Mar 31, 2016 at 07:29:21AM +0200, Yegor Yefremov wrote: > > > > > Hello Greg, > > > > > > > > > > 3.18.x is missing this patch [1] in order to perform a successful > > > > > build. The reason: following patch [2] in 3.18.x uses a macro name > > > > > introduced in the missing patch [1]. > > > > > > > > > > Could you queue it for 3.18.30? > > > > > > > > Nope, I have nothing to do with 3.18-stable, sorry. > > > > > > > > Please read Documentation/stable_kernel_rules.txt for how to get patches > > > > into stable kernel releases. > > > > > > The related discussion on v3.12 is [1], it seems for some reason commit > > > cb83df7 has been back ported to stable, but the dependent patch 0149b07 > > > is missed, which causes build error. > > > > Again, that's nice, but I have nothing to do with 3.18-stable, so > > there's nothing I can do here, please read that file for how to get this > > resolved. > > Sorry, I should have checked first - you are not maintaining > 3.18-stable. And again, even if I was, you need to email the address of the list, not the individual maintainer, so that all of the proper trees can pick things up as-needed. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems
Hi John, Am 29.03.2016 um 04:36 schrieb John Youn : > Hi, > > The following patch series addresses the core reset and force mode > delay problems we have been seeing on dwc2 for some platforms. > > I think I have identified the source of the inconsistencies between > platforms and this series attempts to address them. > > Basically everything stems from the IDDIG debounce filter delay, which > is a function of the PHY clock speed and can range from 5-50 ms if > enabled. This delay must be taken into account on core reset and force > modes. A full explanation is provided in the patch commit log and code > comments. > > The first two patches are prerequisites to the force mode fixes, > including one patch that was sent separately by Przemek Rudy. I have > resubmitted it with this series for convenience. > > Please help by reviewing and testing on your platforms. > > Patches were tested on: > * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL > > Regards, > John > > John Youn (3): > usb: dwc2: gadget: Only initialize device if in device mode > usb: dwc2: Add delay to core soft reset > usb: dwc2: Properly account for the force mode delays > > Przemek Rudy (1): > usb: dwc2: do not override forced dr_mode in gadget setup > > drivers/usb/dwc2/core.c | 195 > drivers/usb/dwc2/core.h | 2 +- > drivers/usb/dwc2/gadget.c | 30 +-- > drivers/usb/dwc2/hcd.c | 6 +- > drivers/usb/dwc2/hw.h | 1 + > drivers/usb/dwc2/platform.c | 9 +- > 6 files changed, 161 insertions(+), 82 deletions(-) > > -- > 2.7.4 > after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188. Besides that I get "dwc2 1018.usb: dwc2_wait_for_mode: Couldn't set host mode“ repeatedly. Currently this works for me: - Revert "usb: dwc2: Fix probe problem on bcm2835“ - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()" Best regards Michael -- 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/3] usb: dwc3: increase maximum number of TRBs per endpoint
Hi, On Thu, Mar 31, 2016 at 09:16:19AM +0300, Felipe Balbi wrote: > > Hi, > > Bin Liu writes: > > [ text/plain ] > > Hi, > > > > On Wed, Mar 30, 2016 at 09:14:18AM +0300, Felipe Balbi wrote: > >> > >> Hi again, > >> > >> Felipe Balbi writes: > >> > Bin Liu writes: > >> >> On Fri, Mar 18, 2016 at 08:59:48AM +0200, Felipe Balbi wrote: > >> >>> > >> >>> Hi, > >> >>> > >> >>> Bin Liu writes: > >> >>> > [ text/plain ] > >> >>> > Hi, > >> >>> > > >> >>> > On Fri, Mar 11, 2016 at 6:54 AM, Felipe Balbi > >> >>> > wrote: > >> >>> >> previously we were using a maximum of 32 TRBs per > >> >>> >> endpoint. With each TRB being 16 bytes long, we were > >> >>> >> using 512 bytes of memory for each endpoint. > >> >>> >> > >> >>> >> However, SLAB/SLUB will always allocate PAGE_SIZE > >> >>> >> chunks. In order to better utilize the memory we > >> >>> >> allocate and to allow deeper queues for gadgets > >> >>> >> which would benefit from it (g_ether comes to mind), > >> >>> >> let's increase the maximum to 256 TRBs which rounds > >> >>> >> up to 4096 bytes for each endpoint. > >> >>> > > >> >>> > Do we want to increase the same for event ring buffers as > >> >>> > while, which is allocated by dma_alloc_coherent(), which > >> >>> > is also at PAGE_SIZE chunks, right? > >> >>> > >> >>> I could, but that's much less important. Currently we have up to 2 > >> >> > >> >> I agree it is less important, the only issue I see is wasting of memory. > >> >> The device I am working on right now has two dwc3 controllers, each > >> >> allocates 16 event buffers, so for the total of 128KB allocated pages, > >> >> only 8KB is really used, 120KB is wasted. > >> >> > >> >> Seems dma pool makes more sense in here? > >> > > >> > I don't know. I think the real thing is that I really need to revisit > >> > that part of the code/databook. The whole "multiple interrupters" seems > >> > like it's only really necessary for host side. Which means that we could > >> > drop all the loops for multiple event buffers and always use a single > >> > one. > >> > > >> > Do you wanna test the following ? > >> > > >> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > >> > index 17fd81447c9f..ebb3ee9c06f1 100644 > >> > --- a/drivers/usb/dwc3/core.c > >> > +++ b/drivers/usb/dwc3/core.c > >> > @@ -237,8 +237,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 > >> > *dwc, unsigned length) > >> > int num; > >> > int i; > >> > > >> > -num = DWC3_NUM_INT(dwc->hwparams.hwparams1); > >> > -dwc->num_event_buffers = num; > >> > +dwc->num_event_buffers = 1; > >> > > >> > dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * > >> > num, > >> > GFP_KERNEL); > >> > > >> > I'll re-read what these bits actually mean. I have a strong feeling we > >> > could (should?) be ignoring them for the peripheral side. > >> > >> Okay, so when we're configuring the endpoints, we could route endpoint > >> interrupts to different event buffers. I really think that's really > >> unimportant for us, specially since we end up using a single IRQ line. > >> > >> I guess I'll just go ahead and remove that code. If it turns out we > >> decide to use it, we shouldn't really be using a loop in the hardirq > >> handler anyway. > > > > Sounds good to me, I only see one evt buffer is used in all my devices, > > even thought multi buffers are allocated based on hwparams1. > > I sent some patches yesterday. You might wanna give it a review ;-) The 3 patches are all look good to me. I bet you already tested it, so I didn't do so. ;) Regards, -Bin. -- 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/1] net: usb: cdc_ncm: adding Telit LE910 V2 mobile broadband card
From: Daniele Palmas Date: Thu, 31 Mar 2016 15:16:47 +0200 > Telit LE910 V2 is a mobile broadband card with no ARP capabilities: > the patch makes this device to use wwan_noarp_info struct > > Signed-off-by: Daniele Palmas Bjorn, can you take a quick look at this? Thank you. -- 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: phy: bcm-ns-usb3: new driver for USB 3.0 PHY on Northstar
On 30 March 2016 at 12:55, Felipe Balbi wrote: > Rafał Miłecki writes: >> [ text/plain ] >> On 30 March 2016 at 12:13, Felipe Balbi wrote: >>> Rafał Miłecki writes: Northstar is a family of SoCs used in home routers. They have USB 2.0 and 3.0 controllers with PHYs that need to be properly initialized. This driver provides PHY init support in a generic way and can be bound with XHCI controller driver. Signed-off-by: Rafał Miłecki --- .../devicetree/bindings/usb/ns-usb3-phy.txt| 14 ++ drivers/usb/phy/Kconfig| 8 + drivers/usb/phy/Makefile | 1 + drivers/usb/phy/phy-bcm-ns-usb3.c | 256 + >>> >>> new drivers should be using drivers/phy/ framework instead. Sorry. >> >> How can I use generic PHY driver (/drivers/phy/) with xhci-platform >> (drivers/usb/host/xhci-plat.c)? I didn't think it's possible. > > well, you can always patch xhci-plat.c, right ? ;-) It seems I was wrong. Since "xhci-platform" uses usb_add_hcd, it can be easily used with a generic PHY driver. I didn't notice there is also support for (generic) PHY in a hcd itself. -- Rafał -- 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/1] net: usb: cdc_ncm: adding Telit LE910 V2 mobile broadband card
David Miller writes: > From: Daniele Palmas > Date: Thu, 31 Mar 2016 15:16:47 +0200 > >> Telit LE910 V2 is a mobile broadband card with no ARP capabilities: >> the patch makes this device to use wwan_noarp_info struct >> >> Signed-off-by: Daniele Palmas > > Bjorn, can you take a quick look at this? Looks fine to me. Reviewed-by: Bjørn Mork 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 1/1] net: usb: cdc_ncm: adding Telit LE910 V2 mobile broadband card
From: Bjørn Mork Date: Thu, 31 Mar 2016 23:07:30 +0200 > David Miller writes: >> From: Daniele Palmas >> Date: Thu, 31 Mar 2016 15:16:47 +0200 >> >>> Telit LE910 V2 is a mobile broadband card with no ARP capabilities: >>> the patch makes this device to use wwan_noarp_info struct >>> >>> Signed-off-by: Daniele Palmas >> >> Bjorn, can you take a quick look at this? > > Looks fine to me. > > Reviewed-by: Bjørn Mork Thanks, applied. -- 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: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems
Hi John, Am 29.03.2016 um 04:36 schrieb John Youn : > Hi, > > The following patch series addresses the core reset and force mode > delay problems we have been seeing on dwc2 for some platforms. > > I think I have identified the source of the inconsistencies between > platforms and this series attempts to address them. > > Basically everything stems from the IDDIG debounce filter delay, which > is a function of the PHY clock speed and can range from 5-50 ms if > enabled. This delay must be taken into account on core reset and force > modes. A full explanation is provided in the patch commit log and code > comments. > > The first two patches are prerequisites to the force mode fixes, > including one patch that was sent separately by Przemek Rudy. I have > resubmitted it with this series for convenience. > > Please help by reviewing and testing on your platforms. > > Patches were tested on: > * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL > > Regards, > John > > John Youn (3): > usb: dwc2: gadget: Only initialize device if in device mode > usb: dwc2: Add delay to core soft reset > usb: dwc2: Properly account for the force mode delays > > Przemek Rudy (1): > usb: dwc2: do not override forced dr_mode in gadget setup > > drivers/usb/dwc2/core.c | 195 > drivers/usb/dwc2/core.h | 2 +- > drivers/usb/dwc2/gadget.c | 30 +-- > drivers/usb/dwc2/hcd.c | 6 +- > drivers/usb/dwc2/hw.h | 1 + > drivers/usb/dwc2/platform.c | 9 +- > 6 files changed, 161 insertions(+), 82 deletions(-) > > -- > 2.7.4 > after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188. Besides that I get "dwc2 1018.usb: dwc2_wait_for_mode: Couldn't set host mode“ repeatedly. Currently this works for me: - Revert "usb: dwc2: Fix probe problem on bcm2835“ - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()" Best regards Michael -- 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 15/16] usb: musb: da8xx: Use devm in probe
Hi,
On Thu, Mar 24, 2016 at 06:51:40PM -0500, David Lechner wrote:
> Simplify things a bit by using devm functions where possible.
>
> Signed-off-by: David Lechner
> ---
>
> v3 changes:
>
> * Kept clk variable to minimize noise.
>
>
> drivers/usb/musb/da8xx.c | 19 +--
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index b03d3b8..0c1997c 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -490,20 +490,18 @@ static int da8xx_probe(struct platform_device *pdev)
> struct da8xx_glue *glue;
> struct platform_device_info pinfo;
> struct clk *clk;
> + int ret;
>
> - int ret = -ENOMEM;
> -
> - glue = kzalloc(sizeof(*glue), GFP_KERNEL);
> + glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
> if (!glue) {
> dev_err(&pdev->dev, "failed to allocate glue context\n");
> - goto err0;
> + return -ENOMEM;
> }
>
> - clk = clk_get(&pdev->dev, "usb20");
> + clk = devm_clk_get(&pdev->dev, "usb20");
> if (IS_ERR(clk)) {
> dev_err(&pdev->dev, "failed to get clock\n");
> - ret = PTR_ERR(clk);
> - goto err3;
> + return PTR_ERR(clk);
memory leak due to not kfree(glue).
> }
>
> ret = clk_enable(clk);
> @@ -560,12 +558,7 @@ err5:
> clk_disable(clk);
>
> err4:
> - clk_put(clk);
> -
> -err3:
> - kfree(glue);
>
> -err0:
> return ret;
> }
>
> @@ -576,8 +569,6 @@ static int da8xx_remove(struct platform_device *pdev)
> platform_device_unregister(glue->musb);
> usb_phy_generic_unregister(glue->phy);
> clk_disable(glue->clk);
> - clk_put(glue->clk);
> - kfree(glue);
Doesn't match with $subject, I'd put them into a seperate patch.
Regards,
-Bin.
--
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 15/16] usb: musb: da8xx: Use devm in probe
On 03/31/2016 05:21 PM, Bin Liu wrote:
- glue = kzalloc(sizeof(*glue), GFP_KERNEL);
+ glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
dev_err(&pdev->dev, "failed to allocate glue context\n");
- goto err0;
+ return -ENOMEM;
}
- clk = clk_get(&pdev->dev, "usb20");
+ clk = devm_clk_get(&pdev->dev, "usb20");
if (IS_ERR(clk)) {
dev_err(&pdev->dev, "failed to get clock\n");
- ret = PTR_ERR(clk);
- goto err3;
+ return PTR_ERR(clk);
memory leak due to not kfree(glue).
It is my understanding that since glue is allocated with devm_kzalloc(),
that if the probe function returns and error, glue and everything else
allocated with devm_* will be automatically freed.
If this is not the case, wouldn't devm_kfree() be the appropriate
function instead?
@@ -576,8 +569,6 @@ static int da8xx_remove(struct platform_device *pdev)
platform_device_unregister(glue->musb);
usb_phy_generic_unregister(glue->phy);
clk_disable(glue->clk);
- clk_put(glue->clk);
- kfree(glue);
Doesn't match with $subject, I'd put them into a seperate patch.
I disagree. Since these are now automatically freed because of changes
in the probe function, these changes belong in the same patch.
--
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: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout
> -Original Message-
> From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com]
> Sent: Thursday, March 31, 2016 8:07 PM
> To: Rajesh Bhagat
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; Sriram Dash
> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
> commmand timeout
>
> On 31.03.2016 06:51, Rajesh Bhagat wrote:
> >
> >
> >
> > Hello Mathias,
> >
> > Thanks a lot for your support.
> >
> > Attached patch is working and allows the completion handler to be
> > called. And resume operation completes and shell comes back (but with lot of
> errors).
> >
> > Now, some other points which needs our attention here are:
> > 1. usb core hub code is not checking the error status of hcd->driver-
> >reset_device(xhci_discover_or_reset_device)
> > and continues considering reset_device was successful (and causes
> > other issues).
> Hence, a check is needed on return
> > value of reset_device and proper action is required on it.
> > 2. Even if above return value is checked and reset_device status is used.
> > USB core
> hub retries for N times which is not
> > required in this case as adding to the resume operation time. But if
> > in this case
> we return -ENOTCONN instead of -EINVAL
> > from hcd->driver->reset_device(xhci_discover_or_reset_device), code
> > returns
> early and doesn't retry.
> >
> > Proposed Solution for above with your suggested patches is as below:
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index
> > 7cad1fa..efeba31 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -2807,13 +2807,17 @@ done:
> > struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> >
> > update_devnum(udev, 0);
> > - /* The xHC may think the device is already reset,
> > -* so ignore the status.
> > + /*
> > +* check the status of resetting the device and
> > update
> > +* the udev status.
> > */
> > if (hcd->driver->reset_device)
> > - hcd->driver->reset_device(hcd, udev);
> > + status =
> > + hcd->driver->reset_device(hcd, udev);
> >
> > - usb_set_device_state(udev, USB_STATE_DEFAULT);
> > + if (status == 0)
> > + usb_set_device_state(udev,
> > USB_STATE_DEFAULT);
> > + else
> > + usb_set_device_state(udev,
> > + USB_STATE_NOTATTACHED);
> > }
> > } else {
> > if (udev)
> > diff --git a/drivers/usb/host/xhci-ring.c
> > b/drivers/usb/host/xhci-ring.c index b3a0a22..9427175f 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -310,6 +310,10 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
> > return -ESHUTDOWN;
> > }
> >
> > + /* writing the CMD_RING_ABORT bit should create a command completion
> > +* event, add a command completion timeout for it as well
> > +*/
> > + mod_timer(&xhci->cmd_timer, jiffies +
> > + XHCI_CMD_DEFAULT_TIMEOUT);
> > return 0;
> > }
> >
> > @@ -1243,6 +1247,7 @@ void xhci_handle_command_timeout(unsigned long data)
> > int ret;
> > unsigned long flags;
> > u64 hw_ring_state;
> > + u32 old_status;
> > struct xhci_command *cur_cmd = NULL;
> > xhci = (struct xhci_hcd *) data;
> >
> > @@ -1250,6 +1255,7 @@ void xhci_handle_command_timeout(unsigned long data)
> > spin_lock_irqsave(&xhci->lock, flags);
> > if (xhci->current_cmd) {
> > cur_cmd = xhci->current_cmd;
> > + old_status = cur_cmd->status;
> > cur_cmd->status = COMP_CMD_ABORT;
> > }
> >
> > @@ -1272,7 +1278,15 @@ void xhci_handle_command_timeout(unsigned long
> data)
> > }
> > /* command timeout on stopped ring, ring can't be aborted */
> > xhci_dbg(xhci, "Command timeout on stopped ring\n");
> > - xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
> > +
> > + /* is this the second timeout for the same command? ring wont
> > restart */
> > + if (old_status == COMP_CMD_ABORT) {
> > + xhci_err(xhci, "Abort timeout, Fail to restart cmd ring\n");
> > + xhci_cleanup_command_queue(xhci);
> > + /* everything else here to halt, cleanup, free etc kill xhc */
> > + } else
> > + xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
> > +
> > spin_unlock_irqrestore(&xhci->lock, flags);
> > return;
> > }
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index
> > c502c22..bd18966 100644
> > ---
Re: [PATCH 2/3] usb: dwc3: increase maximum number of TRBs per endpoint
Hi, Bin Liu writes: >> >> Felipe Balbi writes: >> >> > Bin Liu writes: >> >> >> On Fri, Mar 18, 2016 at 08:59:48AM +0200, Felipe Balbi wrote: >> >> >>> >> >> >>> Hi, >> >> >>> >> >> >>> Bin Liu writes: >> >> >>> > [ text/plain ] >> >> >>> > Hi, >> >> >>> > >> >> >>> > On Fri, Mar 11, 2016 at 6:54 AM, Felipe Balbi >> >> >>> > wrote: >> >> >>> >> previously we were using a maximum of 32 TRBs per >> >> >>> >> endpoint. With each TRB being 16 bytes long, we were >> >> >>> >> using 512 bytes of memory for each endpoint. >> >> >>> >> >> >> >>> >> However, SLAB/SLUB will always allocate PAGE_SIZE >> >> >>> >> chunks. In order to better utilize the memory we >> >> >>> >> allocate and to allow deeper queues for gadgets >> >> >>> >> which would benefit from it (g_ether comes to mind), >> >> >>> >> let's increase the maximum to 256 TRBs which rounds >> >> >>> >> up to 4096 bytes for each endpoint. >> >> >>> > >> >> >>> > Do we want to increase the same for event ring buffers as >> >> >>> > while, which is allocated by dma_alloc_coherent(), which >> >> >>> > is also at PAGE_SIZE chunks, right? >> >> >>> >> >> >>> I could, but that's much less important. Currently we have up to 2 >> >> >> >> >> >> I agree it is less important, the only issue I see is wasting of >> >> >> memory. >> >> >> The device I am working on right now has two dwc3 controllers, each >> >> >> allocates 16 event buffers, so for the total of 128KB allocated pages, >> >> >> only 8KB is really used, 120KB is wasted. >> >> >> >> >> >> Seems dma pool makes more sense in here? >> >> > >> >> > I don't know. I think the real thing is that I really need to revisit >> >> > that part of the code/databook. The whole "multiple interrupters" seems >> >> > like it's only really necessary for host side. Which means that we could >> >> > drop all the loops for multiple event buffers and always use a single >> >> > one. >> >> > >> >> > Do you wanna test the following ? >> >> > >> >> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> >> > index 17fd81447c9f..ebb3ee9c06f1 100644 >> >> > --- a/drivers/usb/dwc3/core.c >> >> > +++ b/drivers/usb/dwc3/core.c >> >> > @@ -237,8 +237,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 >> >> > *dwc, unsigned length) >> >> > int num; >> >> > int i; >> >> > >> >> > - num = DWC3_NUM_INT(dwc->hwparams.hwparams1); >> >> > - dwc->num_event_buffers = num; >> >> > + dwc->num_event_buffers = 1; >> >> > >> >> > dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * >> >> > num, >> >> > GFP_KERNEL); >> >> > >> >> > I'll re-read what these bits actually mean. I have a strong feeling we >> >> > could (should?) be ignoring them for the peripheral side. >> >> >> >> Okay, so when we're configuring the endpoints, we could route endpoint >> >> interrupts to different event buffers. I really think that's really >> >> unimportant for us, specially since we end up using a single IRQ line. >> >> >> >> I guess I'll just go ahead and remove that code. If it turns out we >> >> decide to use it, we shouldn't really be using a loop in the hardirq >> >> handler anyway. >> > >> > Sounds good to me, I only see one evt buffer is used in all my devices, >> > even thought multi buffers are allocated based on hwparams1. >> >> I sent some patches yesterday. You might wanna give it a review ;-) > > The 3 patches are all look good to me. I bet you already tested it, so I > didn't do so. ;) yeah, tested with 3 intel platforms. -- balbi signature.asc Description: PGP signature
Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework
Hi, Mark Brown writes: > On Thu, Mar 31, 2016 at 09:42:58AM +0300, Felipe Balbi wrote: >> Baolin Wang writes: > >> > I want to use bus structure to manage the charger device. Maybe choose >> > class to manage them? > >> I guess a class would fit better in this case. > > IIRC Greg didn't want new classes? good point. Still, this doesn't seem to fit a but_type IMO. -- balbi signature.asc Description: PGP signature
[PATCH] usb: host: focus the development community's efforts
Linux has been growing larger and adopting more and more users. In order to streamline the development process, I propose that we focus USB efforts on XHCI alone, which is the HCD of the future. Anybody still using non-XHCI USB Host Controllers should either buy an XHCI PCIe card to talk to their HW designers to upgrade their SoC. Note that because of this patch and the process improvements it will give us, the community can focus on more interesting ideas like exposing the new leftpad() system call [1] over XHCI. [1] http://marc.info/?l=linux-api&m=145946363603177&w=2 Signed-off-by: Felipe Balbi --- drivers/usb/host/Kconfig | 728 - drivers/usb/host/Makefile | 53 - drivers/usb/host/bcma-hcd.c | 445 --- drivers/usb/host/ehci-atmel.c | 254 -- drivers/usb/host/ehci-dbg.c | 1123 drivers/usb/host/ehci-exynos.c| 349 --- drivers/usb/host/ehci-fsl.c | 715 - drivers/usb/host/ehci-fsl.h | 66 - drivers/usb/host/ehci-grlib.c | 192 -- drivers/usb/host/ehci-hcd.c | 1404 - drivers/usb/host/ehci-hub.c | 1333 - drivers/usb/host/ehci-mem.c | 238 -- drivers/usb/host/ehci-msm.c | 255 -- drivers/usb/host/ehci-mv.c| 321 --- drivers/usb/host/ehci-mxc.c | 225 -- drivers/usb/host/ehci-omap.c | 327 --- drivers/usb/host/ehci-orion.c | 348 --- drivers/usb/host/ehci-pci.c | 441 --- drivers/usb/host/ehci-platform.c | 438 --- drivers/usb/host/ehci-pmcmsp.c| 329 --- drivers/usb/host/ehci-ppc-of.c| 239 -- drivers/usb/host/ehci-ps3.c | 266 -- drivers/usb/host/ehci-q.c | 1537 -- drivers/usb/host/ehci-sched.c | 2517 drivers/usb/host/ehci-sead3.c | 185 -- drivers/usb/host/ehci-sh.c| 195 -- drivers/usb/host/ehci-spear.c | 191 -- drivers/usb/host/ehci-st.c| 368 --- drivers/usb/host/ehci-sysfs.c | 187 -- drivers/usb/host/ehci-tegra.c | 627 drivers/usb/host/ehci-tilegx.c| 216 -- drivers/usb/host/ehci-timer.c | 436 --- drivers/usb/host/ehci-w90x900.c | 148 - drivers/usb/host/ehci-xilinx-of.c | 241 -- drivers/usb/host/ehci.h | 899 -- drivers/usb/host/fhci-dbg.c | 135 - drivers/usb/host/fhci-hcd.c | 836 -- drivers/usb/host/fhci-hub.c | 340 --- drivers/usb/host/fhci-mem.c | 114 - drivers/usb/host/fhci-q.c | 285 -- drivers/usb/host/fhci-sched.c | 898 -- drivers/usb/host/fhci-tds.c | 626 drivers/usb/host/fhci.h | 594 drivers/usb/host/fotg210-hcd.c| 5738 - drivers/usb/host/fotg210.h| 692 - drivers/usb/host/fsl-mph-dr-of.c | 374 --- drivers/usb/host/hwa-hc.c | 889 -- drivers/usb/host/imx21-dbg.c | 531 drivers/usb/host/imx21-hcd.c | 1946 - drivers/usb/host/imx21-hcd.h | 444 --- drivers/usb/host/isp116x-hcd.c| 1717 --- drivers/usb/host/isp116x.h| 595 drivers/usb/host/isp1362-hcd.c| 2788 -- drivers/usb/host/isp1362.h| 1014 --- drivers/usb/host/max3421-hcd.c| 1948 - drivers/usb/host/ohci-at91.c | 690 - drivers/usb/host/ohci-da8xx.c | 438 --- drivers/usb/host/ohci-dbg.c | 811 -- drivers/usb/host/ohci-exynos.c| 311 -- drivers/usb/host/ohci-hcd.c | 1385 - drivers/usb/host/ohci-hub.c | 796 - drivers/usb/host/ohci-jz4740.c| 245 -- drivers/usb/host/ohci-mem.c | 139 - drivers/usb/host/ohci-nxp.c | 283 -- drivers/usb/host/ohci-omap.c | 513 drivers/usb/host/ohci-omap3.c | 211 -- drivers/usb/host/ohci-pci.c | 315 -- drivers/usb/host/ohci-platform.c | 393 --- drivers/usb/host/ohci-ppc-of.c| 235 -- drivers/usb/host/ohci-ps3.c | 251 -- drivers/usb/host/ohci-pxa27x.c| 639 - drivers/usb/host/ohci-q.c | 1230 drivers/usb/host/ohci-s3c2410.c | 502 drivers/usb/host/ohci-sa.c| 265 -- drivers/usb/host/ohci-sm501.c | 271 -- drivers/usb/host/ohci-spear.c | 204 -- drivers/usb/host/ohci-st.c| 347 --- drivers/usb/host/ohci-tilegx.c| 205 -- drivers/usb/host/ohci-tmio.c | 372 --- drivers/usb/host/ohci.h | 742 - drivers/usb/host/oxu210hp-hcd.c | 3916 - drivers/usb/host/oxu210hp.h | 447 --- drivers/usb/host/pci-quirks.c | 1091 --- drivers/usb/host/pci-quirks.h | 26 - drivers/usb/host/r8a66597-hcd.c | 2538 drivers/usb/host/r8a66597.h | 345 --- drivers/usb/host/sl811-hcd.c | 1816 drivers/usb/host/sl811.h | 249 -- drivers/usb/host/sl811_cs.c | 202 -- drivers/usb/host/ssb-hcd.c| 275 -- drivers/usb/host/u132-hcd.c | 3236 - drivers/us
Re: [PATCH] usb: host: focus the development community's efforts
Hi, Felipe Balbi writes: > Linux has been growing larger and adopting more and > more users. In order to streamline the development > process, I propose that we focus USB efforts on XHCI > alone, which is the HCD of the future. > > Anybody still using non-XHCI USB Host Controllers > should either buy an XHCI PCIe card to talk to their > HW designers to upgrade their SoC. > > Note that because of this patch and the process > improvements it will give us, the community can > focus on more interesting ideas like exposing the > new leftpad() system call [1] over XHCI. > > [1] http://marc.info/?l=linux-api&m=145946363603177&w=2 > > Signed-off-by: Felipe Balbi I forgot to add. I tested this on 6th generation core-i7 and had no regressions with USB3.0. -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: host: focus the development community's efforts
On Fri, Apr 1, 2016 at 12:55 AM, Felipe Balbi wrote: > Linux has been growing larger and adopting more and > more users. In order to streamline the development > process, I propose that we focus USB efforts on XHCI > alone, which is the HCD of the future. > > Anybody still using non-XHCI USB Host Controllers > should either buy an XHCI PCIe card to talk to their > HW designers to upgrade their SoC. > > Note that because of this patch and the process > improvements it will give us, the community can > focus on more interesting ideas like exposing the > new leftpad() system call [1] over XHCI. > > [1] http://marc.info/?l=linux-api&m=145946363603177&w=2 > > Signed-off-by: Felipe Balbi Hey, you indeed shocked me for 15 mins... Good job! Regards, -Bin. -- 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
