Re: xHCI problem? [was Re: Erratic USB device behavior and device loss]
On 25 August 2016 at 19:17, Alan Stern wrote:
> Ulf:
>
> Ritesh has collected logs showing that his Realtek RTS5129 USB card
> reader (drivers/mfd/rtsx_usb.c, drivers/mmc/host/rtsx_usb_sdmmc.c) goes
> into runtime autosuspend every 3 seconds and then immediately resumes.
> This sounds like something is failing to call
> pm_runtime_mark_last_busy(). He's using a 4.7 kernel.
>
> In addition, the device gets disconnected from the USB bus from time to
> time. This appears to be a completely separate issue.
>
> For now, I'd like to fix the runtime PM problem. But I don't know
> anything about the mmc core, so perhaps you can help.
>
Sorry for the delay! We have had some regressions for 4.8 rc1 in the
mmc block layer. Those problem should be resolved by now.
By reading from the runtime PM issues you have, the problems could
very well be related. Although I don't believe the issues was present
in a 4.7 kernel.
Perhaps you can run a test on a 4.8 rc4 kernel, just to double check.
The 4.8 rc4, contains the following fixes in the mmc block layer.
commit 7afafc8a44bf ("block: Fix secure erase")
commit 869c554808cc ("mmc: fix use-after-free of struct request")
Kind regards
Uffe
>
> On Thu, 25 Aug 2016, Ritesh Raj Sarraf wrote:
>
>> > Do you happen to know which driver is being used: the memstick
>> > (rtsx_usb_ms) or mmc (rtsx_usb_sdmmc) driver? I suppose this may
>> > depend on what type of card you insert in the reader.
>> >
>>
>>
>> I think it is the rtsx_usb_sdmmc which is in use. I removed the rtsx_usb_ms
>> kernel module and still was able to access the sdcard.
>>
>> rrs@learner:~$ lsmod | grep usb_ms
>> 2016-08-25 / 18:45:52 ♒♒♒ ☹ => 1
>>
>> rrs@learner:~$ lsmod | grep usb_sd
>> rtsx_usb_sdmmc 24576 0
>> rtsx_usb 24576 1 rtsx_usb_sdmmc
>> mmc_core 135168 4 mmc_block,sdhci,sdhci_acpi,rtsx_usb_sdmmc
>> 2016-08-25 / 18:45:55 ♒♒♒ ☺
>>
>>
>> The interesting bit is that when I enter the adapter into the reader, I get
>> the
>> following error 5 times, and then it can access the card.
>>
>> [ 496.822613] mmc0: tuning execution failed: -22
>> [ 496.822629] mmc0: error -22 whilst initialising SD card
>> [ 501.980908] mmc0: tuning execution failed: -22
>> [ 501.980922] mmc0: error -22 whilst initialising SD card
>> [ 507.119953] mmc0: tuning execution failed: -22
>> [ 507.119968] mmc0: error -22 whilst initialising SD card
>> [ 513.148143] mmc0: tuning execution failed: -22
>> [ 513.148157] mmc0: error -22 whilst initialising SD card
>> [ 518.702215] mmc0: tuning execution failed: -22
>> [ 518.70] mmc0: error -22 whilst initialising SD card
>> [ 524.081122] mmc0: new ultra high speed SDR50 SDHC card at address 0002
>> [ 524.082596] mmcblk0: mmc0:0002 NCard 14.9 GiB
>> [ 524.084240] mmcblk0: p1
>> [ 524.306434] FAT-fs (mmcblk0p1): utf8 is not a recommended IO charset for
>> FAT
>> filesystems, filesystem will be case sensitive!
>
> I can't tell why those errors occur. It would require more debugging.
> At least they don't seem to cause any serious problems.
>
>> With your patch applied, the initial errors messages (xhci_hcd :00:14.0:
>> dev
>> 4 ep1out scatterlist error -104/-110) are not seen so far.
>
> This is because those errors occur when the device goes into runtime
> autosuspend and the computer tries to communicate with it while it is
> suspended. Both things (the autosuspend and the communication attempt)
> are bugs in the drivers.
>
>> The device does reset (as you had mentioned), but it doesn't seem to have any
>> power drain related negative effects.
>>
>>
>> rrs@learner:~$ less /var/tmp/dmesg-post-patch.txt | tail -n 25
>> [11922.283067] wlan0: RX AssocResp from 00:40:77:bb:55:12 (capab=0x411
>> status=0
>> aid=1)
>> [11922.283743] wlan0: associated
>> [11922.283801] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
>> [11922.883849] systemd[1]: apt-daily.timer: Adding 8h 36min 18.323966s random
>> time.
>> [11923.081426] systemd[1]: apt-daily.timer: Adding 2h 23min 22.221062s random
>> time.
>> [13799.616838] atkbd serio0: Unknown key pressed (translated set 2, code
>> 0xbe on
>> isa0060/serio0).
>> [13799.616843] atkbd serio0: Use 'setkeycodes e03e ' to make it
>> known.
>> [13799.625901] atkbd serio0: Unknown key released (translated set 2, code
>> 0xbe
>> on isa0060/serio0).
>> [13799.625905] atkbd serio0: Use 'setkeycodes e03e ' to make it
>> known.
>> [13800.547966] usb 1-4: USB disconnect, device number 15
>
> Spontaneous disconnect followed by reconnect a little later...
>
>> [13801.707137] usb 1-4: new high-speed USB device number 16 using xhci_hcd
>> [13801.880788] usb 1-4: New USB device found, idVendor=0bda, idProduct=0129
>> [13801.880791] usb 1-4: New USB device strings: Mfr=1, Product=2,
>> SerialNumber=3
>> [13801.880792] usb 1-4: Product: USB2.0-CRW
>> [13801.880793] usb 1-4: Manufacturer: Generic
>> [13801.880794] usb 1-4: SerialNumber: 2010020139600
>> [13802.809031] usb 1-4: USB disconnect, d
Re: [PATCHv6 1/3] usb: USB Type-C connector class
Hi Guenter, On Mon, Aug 29, 2016 at 11:50:49AM -0700, Guenter Roeck wrote: > Hello Heikki, > > On Mon, Aug 29, 2016 at 05:07:39PM +0300, Heikki Krogerus wrote: > > Hi Guenter, > > > > > > Overall this is quite vague and, especially for chargers, most of the > > > > time > > > > misses the point. > > > > > > > > I would really prefer if we could stay closer to the specification in > > > > this > > > > case, and not try to merge multiple orthogonal attributes into one. > > > > > > OK. So what would you propose? > > > > I'm actually only conserned about the accessory case, as there we are > > really not a source/sink/DRP, nor are we DPF/UFP/DRD. Should we use > > this attribute to only express if the type of the partner is "normal" > > or an accessory? > > > > We currently have three attributes to cover accessory modes. > > supported_accessory_modes > Lists the Accessory Modes, defined in the USB Type-C > specification, the port supports. > > [ This is a bit vague. I think we should list the actual strings. > The modes are called "Audio Adapter Accessory Mode" and "Debug > Accessory Mode", yet the reported text is "Audio" and "Debug". > Also, "Digital Audio" isn't supported as of specification revision > 1.2. So the strings doesn't exactly follow the specification. ] I'm fine if we want to use more precise strings. > accessory > Shows the name of the Accessory Mode. The Accessory Modes are > defined in USB Type-C Specification. > > type > Shows the type of the partner. > > One of the possible accessory modes is TYPEC_ACCESSORY_NONE. > > If you are only interested in accessory mode support, maybe we don't need > the 'type' attribute at all. We could make the 'accessory' attribute always > visible and display one of "none", "Audio", "Debug", or "Digital Audio". > It might also make sense to rename the attribute to "accessory_mode". That works for me. How about if I add the "supports_usb_power_delivery" attribute for the partners instead to give some details about them. Any objections? > On a side note, while looking into this, I noticed the following: > > + if (port->cap->accessory) > + for (accessory = port->cap->accessory, i = 0; > +i < port->cap->num_accessory; accessory++, i++) > + ret += sprintf(buf, "%s\n", > + typec_accessory_modes[*accessory]); > > This means the list of supported accessories always starts with ", ". Where does it print ", "? I'm not sure what is wrong here, but I'll update this code in any case. I'll change the accessory member in typec_capability into fixed size array to make it easier to deal with for now. Thanks, -- heikki -- 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: [PATCHv6 3/3] mfd: intel_soc_pmic_bxtwc: add support for USB Type-C PHY on WhiskeyCove
On Mon, 29 Aug 2016, Heikki Krogerus wrote:
> Intel WhiskeyCove PMIC has also a USB Type-C PHY, so let's
> create a device for it.
>
> Signed-off-by: Heikki Krogerus
> Cc: Lee Jones
> ---
> drivers/mfd/intel_soc_pmic_bxtwc.c | 11 +++
> 1 file changed, 11 insertions(+)
Applied, thanks.
> diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c
> b/drivers/mfd/intel_soc_pmic_bxtwc.c
> index b942876..0e61dde 100644
> --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> @@ -84,6 +84,7 @@ enum bxtwc_irqs_level2 {
> BXTWC_THRM2_IRQ,
> BXTWC_BCU_IRQ,
> BXTWC_ADC_IRQ,
> + BXTWC_USBC_IRQ,
> BXTWC_CHGR0_IRQ,
> BXTWC_CHGR1_IRQ,
> BXTWC_GPIO0_IRQ,
> @@ -109,6 +110,7 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[]
> = {
> REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
> REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
> REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
> + REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
> REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
> REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
> REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
> @@ -143,6 +145,10 @@ static struct resource adc_resources[] = {
> DEFINE_RES_IRQ_NAMED(BXTWC_ADC_IRQ, "ADC"),
> };
>
> +static struct resource usbc_resources[] = {
> + DEFINE_RES_IRQ(BXTWC_USBC_IRQ),
> +};
> +
> static struct resource charger_resources[] = {
> DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "CHARGER"),
> DEFINE_RES_IRQ_NAMED(BXTWC_CHGR1_IRQ, "CHARGER1"),
> @@ -170,6 +176,11 @@ static struct mfd_cell bxt_wc_dev[] = {
> .resources = thermal_resources,
> },
> {
> + .name = "bxt_wcove_usbc",
> + .num_resources = ARRAY_SIZE(usbc_resources),
> + .resources = usbc_resources,
> + },
> + {
> .name = "bxt_wcove_ext_charger",
> .num_resources = ARRAY_SIZE(charger_resources),
> .resources = charger_resources,
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: [PATCHv6 1/3] usb: USB Type-C connector class
On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote: > +What: /sys/class/typec//current_data_role > +Date: June 2016 > +Contact: Heikki Krogerus > +Description: > + The current USB data role the port is operating in. > This > + attribute can be used for requesting data role > swapping on the > + port. Swapping is only supported as an asynchronous > operation > + and requires polling of the attribute in order to know > the > + result, so successful write operation does not mean > successful > + swap. > + That is badly formulated. Does it mean that poll() or select() can be used or does the value need to be repearedly read? And how would you learn about an error? 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: [PATCHv6 1/3] usb: USB Type-C connector class
Hi Oliver, On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote: > On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote: > > +What: /sys/class/typec//current_data_role > > +Date: June 2016 > > +Contact: Heikki Krogerus > > +Description: > > + The current USB data role the port is operating in. > > This > > + attribute can be used for requesting data role > > swapping on the > > + port. Swapping is only supported as an asynchronous > > operation > > + and requires polling of the attribute in order to know > > the > > + result, so successful write operation does not mean > > successful > > + swap. > > + > > That is badly formulated. Does it mean that poll() or select() > can be used or does the value need to be repearedly read? Does polling not always mean poll/select? > And how would you learn about an error? This is what I'm also really worried about. I'm now wondering did I give up too easily on this to Guenter in hope to move this thing forward. He said it's problematic to do these calls synchronously for him. Was it something related to potential conflicting role swaps from both ends? Guenter, can you please elaborate? And how do you plan to report failures with the swaps? Thanks, -- heikki -- 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: Problem(s) with Seagate Backup Plus 4TB portable drive in UAS mode
On Thu, 2016-08-25 at 22:55 -0400, Robert Krawitz wrote: > Then, when I write large volumes of data to the drive, I get the > following every few minutes or so, with the drive temporarily > stalling. The result is that on a drive capable of about 120 MB/sec I > actually get 40-60 MB/sec (it varies). > > [ 475.006500] sd 6:0:0:0: [sdd] tag#15 uas_eh_abort_handler 0 uas-tag > 16 inflight: CMD IN > [ 475.006509] sd 6:0:0:0: [sdd] tag#15 CDB: Read(16) 88 00 00 00 00 > 00 a3 81 00 4f 00 00 00 08 00 00 This is the typical error handling of UAS, which works in your case, albeit with a performance penalty. But the actual cause of kicking in the error handler is missing in your log. You cut too much at the beginning. 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: [PATCHv6 1/3] usb: USB Type-C connector class
On Tue, 2016-08-30 at 13:04 +0300, Heikki Krogerus wrote: > On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote: Hi, > > On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote: > > > +What: /sys/class/typec//current_data_role > > > +Date: June 2016 > > > +Contact: Heikki Krogerus > > > +Description: > > > + The current USB data role the port is operating in. > > > This > > > + attribute can be used for requesting data role > > > swapping on the > > > + port. Swapping is only supported as an asynchronous > > > operation > > > + and requires polling of the attribute in order to know > > > the > > > + result, so successful write operation does not mean > > > successful > > > + swap. > > > + > > > > That is badly formulated. Does it mean that poll() or select() > > can be used or does the value need to be repearedly read? > > Does polling not always mean poll/select? No, it does not. > > And how would you learn about an error? > > This is what I'm also really worried about. I'm now wondering did I > give up too easily on this to Guenter in hope to move this thing > forward. He said it's problematic to do these calls synchronously for Error reporting does not require a synchronous operation. Reporting it in the next read() or write() and making it pollable is perfectly viable. It just must not be silently dropped. 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: [PATCHv6 1/3] usb: USB Type-C connector class
On Tue, Aug 30, 2016 at 01:16:46PM +0200, Oliver Neukum wrote: > On Tue, 2016-08-30 at 13:04 +0300, Heikki Krogerus wrote: > > On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote: > > Hi, > > > > On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote: > > > > +What: /sys/class/typec//current_data_role > > > > +Date: June 2016 > > > > +Contact: Heikki Krogerus > > > > +Description: > > > > + The current USB data role the port is operating in. > > > > This > > > > + attribute can be used for requesting data role > > > > swapping on the > > > > + port. Swapping is only supported as an asynchronous > > > > operation > > > > + and requires polling of the attribute in order to know > > > > the > > > > + result, so successful write operation does not mean > > > > successful > > > > + swap. > > > > + > > > > > > That is badly formulated. Does it mean that poll() or select() > > > can be used or does the value need to be repearedly read? > > > > Does polling not always mean poll/select? > > No, it does not. > > > > And how would you learn about an error? > > > > This is what I'm also really worried about. I'm now wondering did I > > give up too easily on this to Guenter in hope to move this thing > > forward. He said it's problematic to do these calls synchronously for > > Error reporting does not require a synchronous operation. Reporting > it in the next read() or write() and making it pollable is perfectly > viable. It just must not be silently dropped. OK, I think I got it. I need to document that. I'll also add get_pr/dr/vconn hooks to the API for getting the status. Thanks Oliver, -- heikki -- 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] usb: host: xhci: Move the xhci quirks checking to the right place
Hi Mathias, On 18 August 2016 at 15:17, Felipe Balbi wrote: > > Hi, > > Baolin Wang writes: >> It will reset the xhci quirks in xhci_gen_setup() function when xhci try to >> add one hcd, thus we need to move the XHCI_LPM_SUPPORT quirk checking after >> adding hcd. >> >> Signed-off-by: Baolin Wang >> --- >> drivers/usb/host/xhci-plat.c |8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c >> index 1f3f981..e2e2487 100644 >> --- a/drivers/usb/host/xhci-plat.c >> +++ b/drivers/usb/host/xhci-plat.c >> @@ -223,10 +223,6 @@ static int xhci_plat_probe(struct platform_device *pdev) >> goto disable_clk; >> } >> >> - if ((node && of_property_read_bool(node, "usb3-lpm-capable")) || >> - (pdata && pdata->usb3_lpm_capable)) >> - xhci->quirks |= XHCI_LPM_SUPPORT; >> - >> if (HCC_MAX_PSA(xhci->hcc_params) >= 4) >> xhci->shared_hcd->can_do_streams = 1; >> >> @@ -250,6 +246,10 @@ static int xhci_plat_probe(struct platform_device *pdev) >> if (ret) >> goto dealloc_usb2_hcd; >> >> + if ((node && of_property_read_bool(node, "usb3-lpm-capable")) || >> + (pdata && pdata->usb3_lpm_capable)) >> + xhci->quirks |= XHCI_LPM_SUPPORT; >> + > > Mathias, any comments here? Do you have any comments about this? Thanks. -- 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 V4] leds: trigger: Introduce an USB port trigger
On Fri, Aug 26, 2016 at 05:38:05PM +0200, Rafał Miłecki wrote:
> On 25 August 2016 at 14:49, Greg KH wrote:
> > On Thu, Aug 25, 2016 at 10:03:52AM +0200, Rafał Miłecki wrote:
> >> +static void usbport_trig_activate(struct led_classdev *led_cdev)
> >> +{
> >> + struct usbport_trig_data *usbport_data;
> >> + int err;
> >> +
> >> + usbport_data = kzalloc(sizeof(*usbport_data), GFP_KERNEL);
> >> + if (!usbport_data)
> >> + return;
> >> + usbport_data->led_cdev = led_cdev;
> >> +
> >> + /* Storing ports */
> >> + INIT_LIST_HEAD(&usbport_data->ports);
> >> + usbport_data->ports_dir = kobject_create_and_add("ports",
> >> +
> >> &led_cdev->dev->kobj);
> >
> > If you _ever_ find yourself in a driver having to use kobject calls,
> > it's a HUGE hint that you are doing something wrong.
> >
> > Hint, you are doing this wrong :)
> >
> > Use an attribute group if you need a subdirectory in sysfs, using a
> > "raw" kobject like this hides it from all userspace tools and so no
> > userspace program can see it (hint, try using libudev to access these
> > files attached to the device...)
>
> Oops. Thanks for pointing groups to me. I was looking at sysfs.h
> earlier but I didn't realize group can be a subdirectory. I can see
> these sysfs_create_group(s) and friends now, thanks.
No, use an attribute group, and name it, and the subdir will be created
automagically for you.
> >> + if (!usbport_data->ports_dir)
> >> + goto err_free;
> >> +
> >> + /* API for ports management */
> >> + err = device_create_file(led_cdev->dev, &dev_attr_new_port);
> >> + if (err)
> >> + goto err_put_ports;
> >> + err = device_create_file(led_cdev->dev, &dev_attr_remove_port);
> >> + if (err)
> >> + goto err_remove_new_port;
> >
> > Doesn't this race with userspace and fail? Shouldn't the led core be
> > creating your leds for you?
>
> These questions aren't clear to me. What kind of race? Doing
> echo usbport > trigger
> results in trigger core calling usbport_trig_activate. We create new
> attributes and then we return.
Why aren't the attributes present when the device is found? What is
"trigger" for?
> I'm also not creating any leds there. This already has to be LED
> available if you want to assign some trigger to it.
That sounds really odd...
> >> +
> >> + /* Notifications */
> >> + usbport_data->nb.notifier_call = usbport_trig_notify,
> >> + led_cdev->trigger_data = usbport_data;
> >> + usb_register_notify(&usbport_data->nb);
> >
> > Don't abuse the USB notifier chain with stuff like this please, is that
> > really necessary? Why can't your hardware just tell you what USB ports
> > are in use "out of band"?
>
> I totally don't understand this part. This LED trigger is supposed to
> react to USB devices appearing at specified ports. How else can I know
> there is a new USB device if not by notifications?
> I'm wondering if we are on the same page. Could it be my idea of this
> LED trigger is not clear at all? Could you take a look at commit
> message again, please? Mostly this part:
> > This commit adds a new trigger responsible for turning on LED when USB
> > device gets connected to the specified USB port. This can can useful for
> > various home routers that have USB port(s) and a proper LED telling user
> > a device is connected.
>
> Can I add something more to make it clear what this trigger is responsible
> for?
Yes please, as I totally missed that.
And the USB notifier was created for some "special" parts of the USB
core to use, this feels like an abuse of that in some way. But it's
hard to define, I know...
> >> +
> >> + led_cdev->activated = true;
> >> + return;
> >> +
> >> +err_remove_new_port:
> >> + device_remove_file(led_cdev->dev, &dev_attr_new_port);
> >> +err_put_ports:
> >> + kobject_put(usbport_data->ports_dir);
> >> +err_free:
> >> + kfree(usbport_data);
> >> +}
> >
> > And again, why is this USB specific? Why can't you use this same
> > userspace api and codebase for PCI ports? For a sdcard port? For a
> > thunderbolt port?
>
> I'm leaving this one unanswered as discussion on this continued in
> V3.5 thread below my reply:
> On 25 August 2016 at 07:14, Rafał Miłecki wrote:
> > Good question. I would like to extend this USB port trigger in the
> > future by reacting to USB activity. This involves playing with URBs
> > and I believe that at that point it'd be getting too much USB specific
> > to /rule them all/.
No, I mean why not have this specific activity LED work for all types of
devices, not just USB.
Not that this specific LED works for other types of USB activity.
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: [GIT PULL] usb: chipidea fixes for v4.8
On Fri, Aug 19, 2016 at 10:11:22AM +0800, Peter Chen wrote: > The following changes since commit f1f6d9a8b540df22b87a5bf6bc104edaade81f47: > > xhci: don't dereference a xhci member after removing xhci (2016-08-16 > 09:42:47 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ > tags/usb-ci-v4.8-rc3 Pulled and pushed out, 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: Problem(s) with Seagate Backup Plus 4TB portable drive in UAS mode
On Tue, 30 Aug 2016 12:40:48 +0200, Oliver Neukum wrote: > On Thu, 2016-08-25 at 22:55 -0400, Robert Krawitz wrote: >> Then, when I write large volumes of data to the drive, I get the >> following every few minutes or so, with the drive temporarily >> stalling. The result is that on a drive capable of about 120 MB/sec I >> actually get 40-60 MB/sec (it varies). >> >> [ 475.006500] sd 6:0:0:0: [sdd] tag#15 uas_eh_abort_handler 0 uas-tag >> 16 inflight: CMD IN >> [ 475.006509] sd 6:0:0:0: [sdd] tag#15 CDB: Read(16) 88 00 00 00 00 >> 00 a3 81 00 4f 00 00 00 08 00 00 > > This is the typical error handling of UAS, which works in your case, > albeit with a performance penalty. But the actual cause of kicking in > the error handler is missing in your log. You cut too much at the > beginning. Here's an excerpt from the full log, from the very first message after I physically plugged in the drive: [67070.554819] usb 3-1: new SuperSpeed USB device number 29 using xhci_hcd [67070.577539] usb 3-1: New USB device found, idVendor=0bc2, idProduct=ab28 [67070.577547] usb 3-1: New USB device strings: Mfr=2, Product=3, SerialNumber=1 [67070.577552] usb 3-1: Product: BUP BK [67070.577557] usb 3-1: Manufacturer: Seagate [67070.577562] usb 3-1: SerialNumber: NA7PJA0K [67070.581748] scsi host6: uas [67070.582566] scsi 6:0:0:0: Direct-Access Seagate BUP BK 0304 PQ: 0 ANSI: 6 [67070.614993] sd 6:0:0:0: Attached scsi generic sg4 type 0 [67070.615100] sd 6:0:0:0: [sdd] Spinning up disk... [67071.618564] ...ready [67077.646605] sd 6:0:0:0: [sdd] 7814037167 512-byte logical blocks: (4.00 TB/3.64 TiB) [67077.646608] sd 6:0:0:0: [sdd] 2048-byte physical blocks [67077.714588] sd 6:0:0:0: [sdd] Write Protect is off [67077.714596] sd 6:0:0:0: [sdd] Mode Sense: 4f 00 00 00 [67077.714945] sd 6:0:0:0: [sdd] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [67077.915152] sd 6:0:0:0: [sdd] Attached SCSI disk [67086.589616] device-mapper: table: 254:0: adding target device sdd caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=4096 [67086.589621] device-mapper: table: 254:0: adding target device sdd caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=4096 [67086.822573] device-mapper: table: 254:0: adding target device sdd caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=33553920 [67086.822578] device-mapper: table: 254:0: adding target device sdd caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=33553920 [67112.197916] EXT4-fs (dm-0): mounted filesystem with ordered data mode. Opts: (null) [67570.133929] sd 6:0:0:0: [sdd] tag#0 uas_eh_abort_handler 0 uas-tag 1 inflight: CMD OUT [67570.133935] sd 6:0:0:0: [sdd] tag#0 CDB: Write(16) 8a 00 00 00 00 00 89 9c 7f ff 00 00 04 00 00 00 [67570.134140] sd 6:0:0:0: [sdd] tag#7 uas_eh_abort_handler 0 uas-tag 8 inflight: CMD OUT [67570.134143] sd 6:0:0:0: [sdd] tag#7 CDB: Write(16) 8a 00 00 00 00 00 89 9c 7b ff 00 00 04 00 00 00 [67570.134408] sd 6:0:0:0: [sdd] tag#18 uas_eh_abort_handler 0 uas-tag 19 inflight: CMD OUT [67570.134411] sd 6:0:0:0: [sdd] tag#18 CDB: Write(16) 8a 00 00 00 00 00 89 9c 77 ff 00 00 04 00 00 00 [67570.134657] sd 6:0:0:0: [sdd] tag#6 uas_eh_abort_handler 0 uas-tag 7 inflight: CMD OUT [67570.134660] sd 6:0:0:0: [sdd] tag#6 CDB: Write(16) 8a 00 00 00 00 00 89 9c 73 ff 00 00 04 00 00 00 [67570.134918] sd 6:0:0:0: [sdd] tag#22 uas_eh_abort_handler 0 uas-tag 23 inflight: CMD OUT [67570.134921] sd 6:0:0:0: [sdd] tag#22 CDB: Write(16) 8a 00 00 00 00 00 89 9c 6f ff 00 00 04 00 00 00 [67570.135122] sd 6:0:0:0: [sdd] tag#5 uas_eh_abort_handler 0 uas-tag 6 inflight: CMD OUT [67570.135127] sd 6:0:0:0: [sdd] tag#5 CDB: Write(16) 8a 00 00 00 00 00 89 9c 6b ff 00 00 04 00 00 00 [67570.135221] sd 6:0:0:0: [sdd] tag#4 uas_eh_abort_handler 0 uas-tag 5 inflight: CMD OUT [67570.135224] sd 6:0:0:0: [sdd] tag#4 CDB: Write(16) 8a 00 00 00 00 00 89 9c 67 ff 00 00 04 00 00 00 [67570.135413] sd 6:0:0:0: [sdd] tag#19 uas_eh_abort_handler 0 uas-tag 20 inflight: CMD OUT [67570.135415] sd 6:0:0:0: [sdd] tag#19 CDB: Write(16) 8a 00 00 00 00 00 89 9c 63 ff 00 00 04 00 00 00 [67571.085955] sd 6:0:0:0: [sdd] tag#2 uas_eh_abort_handler 0 uas-tag 3 inflight: CMD OUT [67571.085966] sd 6:0:0:0: [sdd] tag#2 CDB: Write(16) 8a 00 00 00 00 00 89 9c 83 ff 00 00 04 00 00 00 [67571.086072] scsi host6: uas_eh_bus_reset_handler start [67571.088126] sd 6:0:0:0: [sdd] tag#1 uas_zap_pending 0 uas-tag 2 inflight: CMD [67571.088129] sd 6:0:0:0: [sdd] tag#1 CDB: Write(16) 8a 00 00 00 00 00 89 9c 8f ff 00 00 04 00 00 00 [67571.088131] sd 6:0:0:0: [sdd] tag#8 uas_zap_pending 0 uas-tag 9 inflight: CMD [67571.088133] sd 6:0:0:0: [sdd] tag#8 CDB: Write(16) 8a 00 00 00 00 00 89 9c 97 ff 00 00 04 00 00 00 [67571.
Re: [PATCHv6 1/3] usb: USB Type-C connector class
Hi, On Tue, Aug 30, 2016 at 02:49:50PM +0300, Heikki Krogerus wrote: > On Tue, Aug 30, 2016 at 01:16:46PM +0200, Oliver Neukum wrote: > > Error reporting does not require a synchronous operation. Reporting > > it in the next read() or write() and making it pollable is perfectly > > viable. It just must not be silently dropped. > > OK, I think I got it. I need to document that. I'll also add > get_pr/dr/vconn hooks to the API for getting the status. Would the attached diff do the trick? It also includes the other suggestions from Guenter. Thanks, -- heikki diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec index 53fdd11..9f249b2 100644 --- a/Documentation/ABI/testing/sysfs-class-typec +++ b/Documentation/ABI/testing/sysfs-class-typec @@ -6,10 +6,12 @@ Contact: Heikki Krogerus Description: The current USB data role the port is operating in. This attribute can be used for requesting data role swapping on the - port. Swapping is only supported as an asynchronous operation - and requires polling of the attribute in order to know the - result, so successful write operation does not mean successful - swap. + port. Swapping is only supported as an asynchronous operation, + so successful write operation does not mean successful swap. + The attribute can be polled with poll() or select() to get + notification on finished operation. In case of failures, the + following read/write after finished operation will return error + code identifying the cause of the failure. Valid values: - host @@ -22,9 +24,11 @@ Description: The current power role of the port. This attribute can be used to request power role swap on the port when the port supports USB Power Delivery. Swapping is only supported as an - asynchronous operation and requires polling of the attribute in - order to know the result, so successful write operation does not - mean successful swap. + asynchronous operation, so successful write operation does not + mean successful swap. The attribute can be polled with poll() or + select() to get notification on finished operation. In case of + failures, the following read/write after finished operation will + return error code identifying the cause of the failure. Valid values: - source @@ -37,6 +41,12 @@ Description: Shows is the port VCONN Source. This attribute can be used to request VCONN swap to change the VCONN Source during connection when both the port and the partner support USB Power Delivery. + Swapping is only supported as an asynchronous operation, so + successful write operation does not mean successful swap. The + attribute can be polled with poll() or select() to get + notification on finished operation. In case of failures, the + following read/write after finished operation will return error + code identifying the cause of the failure. Valid values are: - 0 when the port is not the VCONN Source @@ -113,28 +123,21 @@ Description: USB Type-C partner devices (eg. /sys/class/typec/usbc0-partner/) -What: /sys/class/typec/-partner/accessory +What: /sys/class/typec/-partner/accessory_mode Date: June 2016 Contact: Heikki Krogerus Description: - The attribute is visible only when the partner's type is - "Accessory". The type can be read from its own attribute. + Shows the Accessory Mode name or "no" when the partner is not an + Accesory Mode. The Accessory Modes are defined in USB Type-C + Specification. - Shows the name of the Accessory Mode. The Accessory Modes are - defined in USB Type-C Specification. - -What: /sys/class/typec/-partner/type +What: /sys/class/typec/-partner/supports_usb_power_delivery Date: June 2016 Contact: Heikki Krogerus Description: - Shows the type of the partner. Can be one of the following: - - USB - When the partner is normal USB host/peripheral. - - Charger - When the partner has been identified as dedicated - charger. - - Alternate Mode - When the partner supports Alternate Modes. - - Accessory - When the partner is one of the accessories with - specific Accessory Mode defined in USB Type-C - specification. + Shows if t
Re: [PATCHv6 3/3] mfd: intel_soc_pmic_bxtwc: add support for USB Type-C PHY on WhiskeyCove
On Tue, 30 Aug 2016, Lee Jones wrote:
> On Mon, 29 Aug 2016, Heikki Krogerus wrote:
>
> > Intel WhiskeyCove PMIC has also a USB Type-C PHY, so let's
> > create a device for it.
> >
> > Signed-off-by: Heikki Krogerus
> > Cc: Lee Jones
> > ---
> > drivers/mfd/intel_soc_pmic_bxtwc.c | 11 +++
> > 1 file changed, 11 insertions(+)
>
> Applied, thanks.
This patch conflicts with ...
"mfd: intel_soc_pmic_bxtwc: Add bxt_wcove_usbc device"
Please re-base your changes on my tree (or -next) and resubmit if this
change is still relevant.
> > diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c
> > b/drivers/mfd/intel_soc_pmic_bxtwc.c
> > index b942876..0e61dde 100644
> > --- a/drivers/mfd/intel_soc_pmic_bxtwc.c
> > +++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
> > @@ -84,6 +84,7 @@ enum bxtwc_irqs_level2 {
> > BXTWC_THRM2_IRQ,
> > BXTWC_BCU_IRQ,
> > BXTWC_ADC_IRQ,
> > + BXTWC_USBC_IRQ,
> > BXTWC_CHGR0_IRQ,
> > BXTWC_CHGR1_IRQ,
> > BXTWC_GPIO0_IRQ,
> > @@ -109,6 +110,7 @@ static const struct regmap_irq
> > bxtwc_regmap_irqs_level2[] = {
> > REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
> > REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
> > REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
> > + REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
> > REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
> > REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
> > REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
> > @@ -143,6 +145,10 @@ static struct resource adc_resources[] = {
> > DEFINE_RES_IRQ_NAMED(BXTWC_ADC_IRQ, "ADC"),
> > };
> >
> > +static struct resource usbc_resources[] = {
> > + DEFINE_RES_IRQ(BXTWC_USBC_IRQ),
> > +};
> > +
> > static struct resource charger_resources[] = {
> > DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "CHARGER"),
> > DEFINE_RES_IRQ_NAMED(BXTWC_CHGR1_IRQ, "CHARGER1"),
> > @@ -170,6 +176,11 @@ static struct mfd_cell bxt_wc_dev[] = {
> > .resources = thermal_resources,
> > },
> > {
> > + .name = "bxt_wcove_usbc",
> > + .num_resources = ARRAY_SIZE(usbc_resources),
> > + .resources = usbc_resources,
> > + },
> > + {
> > .name = "bxt_wcove_ext_charger",
> > .num_resources = ARRAY_SIZE(charger_resources),
> > .resources = charger_resources,
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: [PATCHv6 1/3] usb: USB Type-C connector class
On 08/30/2016 03:04 AM, Heikki Krogerus wrote:
Hi Oliver,
On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote:
On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote:
+What: /sys/class/typec//current_data_role
+Date: June 2016
+Contact: Heikki Krogerus
+Description:
+ The current USB data role the port is operating in.
This
+ attribute can be used for requesting data role
swapping on the
+ port. Swapping is only supported as an asynchronous
operation
+ and requires polling of the attribute in order to know
the
+ result, so successful write operation does not mean
successful
+ swap.
+
That is badly formulated. Does it mean that poll() or select()
can be used or does the value need to be repearedly read?
Does polling not always mean poll/select?
And how would you learn about an error?
This is what I'm also really worried about. I'm now wondering did I
give up too easily on this to Guenter in hope to move this thing
forward. He said it's problematic to do these calls synchronously for
him. Was it something related to potential conflicting role swaps from
both ends?
Guenter, can you please elaborate? And how do you plan to report
failures with the swaps?
I thought we had this sorted out. When I said "asynchronous", I did not mean
that the sysfs operation would not wait for the operation to complete. I meant
that the Type-C state machine operates in a different context than the
sysfs/class
code. Since the state machine operates in a different context, it may have
to execute a callback into the class code at any time, independently of
any pending role changes triggered through sysfs. Please have a look into
the patch set I submitted for details. Roughly it works as follows.
Class code context State machine context
User requests role change
Class code calls {dr,pr,vconn}_set
{dr,pr,vconn}_set code validates request
{dr,pr,vconn}_set code sends role change
request to state machineState machine gets role change
request
{dr,pr,vconn}_set code waits for completion
State machine sends role change
request
to link partner
Partner reports Accept or Reject
State machine changes state as
requested
State machine reports new role
to class code
via callbacks
State machine informs Class
code that request
is complete
{dr,pr,vconn}_set code gets results
and returns to caller
Class code reports results to user
From user perspective, everything is synchronous. However, the state machine
has to be
able to run independently and report role and other state changes to the class
code while
a role change request from the class code is pending. For example, it has to be
able to
handle incoming role change requests from the link partner, and it has to be
able to
handle link state changes. All those have to be reported to the class code.
This is
impossible if the class code holds a lock while a role change triggered from
user space
is pending, which is why I asked for the locks in the class code to be removed.
Maybe my use of the term "asynchronous" was misleading, and I should have said
"operates
in a different context" instead. My apologies.
Thanks,
Guenter
--
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: [PATCHv6 1/3] usb: USB Type-C connector class
On 08/30/2016 06:11 AM, Heikki Krogerus wrote: Hi, On Tue, Aug 30, 2016 at 02:49:50PM +0300, Heikki Krogerus wrote: On Tue, Aug 30, 2016 at 01:16:46PM +0200, Oliver Neukum wrote: Error reporting does not require a synchronous operation. Reporting it in the next read() or write() and making it pollable is perfectly viable. It just must not be silently dropped. OK, I think I got it. I need to document that. I'll also add get_pr/dr/vconn hooks to the API for getting the status. Would the attached diff do the trick? It also includes the other suggestions from Guenter. It is not at all what I meant or asked for :-(. I'll have a closer look into the latest patch set later today. Guenter -- 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: [PATCHv6 1/3] usb: USB Type-C connector class
On Tue, Aug 30, 2016 at 06:46:24AM -0700, Guenter Roeck wrote:
> On 08/30/2016 03:04 AM, Heikki Krogerus wrote:
> > Hi Oliver,
> >
> > On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote:
> > > On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote:
> > > > +What: /sys/class/typec//current_data_role
> > > > +Date: June 2016
> > > > +Contact: Heikki Krogerus
> > > > +Description:
> > > > + The current USB data role the port is operating in.
> > > > This
> > > > + attribute can be used for requesting data role
> > > > swapping on the
> > > > + port. Swapping is only supported as an asynchronous
> > > > operation
> > > > + and requires polling of the attribute in order to know
> > > > the
> > > > + result, so successful write operation does not mean
> > > > successful
> > > > + swap.
> > > > +
> > >
> > > That is badly formulated. Does it mean that poll() or select()
> > > can be used or does the value need to be repearedly read?
> >
> > Does polling not always mean poll/select?
> >
> > > And how would you learn about an error?
> >
> > This is what I'm also really worried about. I'm now wondering did I
> > give up too easily on this to Guenter in hope to move this thing
> > forward. He said it's problematic to do these calls synchronously for
> > him. Was it something related to potential conflicting role swaps from
> > both ends?
> >
> > Guenter, can you please elaborate? And how do you plan to report
> > failures with the swaps?
> >
>
> I thought we had this sorted out. When I said "asynchronous", I did not mean
> that the sysfs operation would not wait for the operation to complete. I meant
> that the Type-C state machine operates in a different context than the
> sysfs/class
> code. Since the state machine operates in a different context, it may have
> to execute a callback into the class code at any time, independently of
> any pending role changes triggered through sysfs. Please have a look into
> the patch set I submitted for details. Roughly it works as follows.
>
> Class code contextState machine context
>
> User requests role change
> Class code calls {dr,pr,vconn}_set
> {dr,pr,vconn}_set code validates request
> {dr,pr,vconn}_set code sends role change
> request to state machineState machine gets role change
> request
> {dr,pr,vconn}_set code waits for completion
> State machine sends role change
> request
> to link partner
> Partner reports Accept or Reject
> State machine changes state as
> requested
> State machine reports new role
> to class code
> via callbacks
> State machine informs Class
> code that request
> is complete
> {dr,pr,vconn}_set code gets results
> and returns to caller
> Class code reports results to user
>
> From user perspective, everything is synchronous. However, the state machine
> has to be
> able to run independently and report role and other state changes to the
> class code while
> a role change request from the class code is pending. For example, it has to
> be able to
> handle incoming role change requests from the link partner, and it has to be
> able to
> handle link state changes. All those have to be reported to the class code.
> This is
> impossible if the class code holds a lock while a role change triggered from
> user space
> is pending, which is why I asked for the locks in the class code to be
> removed.
>
> Maybe my use of the term "asynchronous" was misleading, and I should have
> said "operates
> in a different context" instead. My apologies.
Thanks for the explanation. I remember you did explain this before I
started my parental leave, but I forgot all about it.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver
Add a new USB Phy driver for Broadcom STB SoCs. This driver
supports all Broadcom STB ARM SoCs. This driver in combination
with the generic ohci, ehci and xhci platform drivers will enable
USB1.1, USB2.0 and USB3.0 support. This Phy driver also supports
the Broadcom UDC gadget driver.
Signed-off-by: Al Cooper
---
.../bindings/phy/brcm,brcmstb-usb-phy.txt | 39 +
MAINTAINERS| 7 +
drivers/phy/Kconfig| 10 +
drivers/phy/Makefile | 2 +
drivers/phy/phy-brcm-usb-init.c| 792 +
drivers/phy/phy-brcm-usb-init.h| 49 ++
drivers/phy/phy-brcm-usb.c | 206 ++
7 files changed, 1105 insertions(+)
create mode 100644
Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
create mode 100644 drivers/phy/phy-brcm-usb-init.c
create mode 100644 drivers/phy/phy-brcm-usb-init.h
create mode 100644 drivers/phy/phy-brcm-usb.c
diff --git a/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
new file mode 100644
index 000..34fa9dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt
@@ -0,0 +1,39 @@
+Broadcom STB USB PHY
+
+Required properties:
+ - compatible: brcm,brcmstb-usb-phy
+ - reg: two offset and length pairs. The second pair specifies the
+USB 3.0 related registers and is only required for PHYs
+that support USB 3.0
+ - #phy-cells: Shall be 1 as it expects one argument for setting
+ the type of the PHY. Possible values are 0 (1.1 and 2.0),
+ 1 (3.0)
+
+
+Optional Properties:
+- clocks : phandle + clock specifier for the phy clocks
+- clock-names: string, clock name
+- ipp: Invert Port Power
+- ioc: Invert Over Current detection
+- has_xhci: Contains an optional 3.0 PHY
+- device: PHY Device mode. Possible values are: 0 (Host), 1 (Device)
+ or 2 (DRD)
+
+
+
+Example:
+
+usbphy_0: usb-phy@f0470200 {
+ reg = <0xf0470200 0xb8>,
+ <0xf0471940 0x6c0>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "brcm,brcmstb-usb-phy";
+ ioc = <1>;
+ ipp = <1>;
+ #phy-cells = <1>;
+ ranges;
+ has_xhci;
+ clocks = <&sw_usb20>;
+ clock-names = "sw_usb";
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b1..d58b124 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2697,6 +2697,13 @@ S: Maintained
F: drivers/bcma/
F: include/linux/bcma/
+BROADCOM STB USB PHY DRIVER
+M: Al Cooper
+L: linux-usb@vger.kernel.org
+L: bcm-kernel-feedback-l...@broadcom.com
+S: Supported
+F: drivers/phy/phy-brcm-usb*
+
BROADCOM SYSTEMPORT ETHERNET DRIVER
M: Florian Fainelli
L: net...@vger.kernel.org
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19bff3a..5ff5e47 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -443,6 +443,16 @@ config PHY_CYGNUS_PCIE
Enable this to support the Broadcom Cygnus PCIe PHY.
If unsure, say N.
+config BRCM_USB_PHY
+ tristate "Broadcom USB PHY driver"
+ depends on OF && USB && ARCH_BRCMSTB
+ select GENERIC_PHY
+ default y
+ help
+ Enable this to support the Broadcom USB PHY on
+ Broadcom STB SoCs.
+ If unsure, say Y.
+
source "drivers/phy/tegra/Kconfig"
config PHY_NS2_PCIE
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 90ae198..f4ff6c7 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_PHY_DA8XX_USB) += phy-da8xx-usb.o
obj-$(CONFIG_PHY_DM816X_USB) += phy-dm816x-usb.o
obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
+obj-$(CONFIG_BRCM_USB_PHY) += phy-brcm-usb.o
+obj-$(CONFIG_BRCM_USB_PHY) += phy-brcm-usb-init.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o
diff --git a/drivers/phy/phy-brcm-usb-init.c b/drivers/phy/phy-brcm-usb-init.c
new file mode 100644
index 000..f5d8c32
--- /dev/null
+++ b/drivers/phy/phy-brcm-usb-init.c
@@ -0,0 +1,792 @@
+/*
+ * Copyright (C) 2014-2016 Broadcom
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distrib
[PATCH V3 1/2] soc: brcmstb: Add Product ID and Family ID helper functions
Signed-off-by: Al Cooper
---
drivers/soc/bcm/brcmstb/common.c| 12
include/linux/soc/brcmstb/brcmstb.h | 10 ++
2 files changed, 22 insertions(+)
diff --git a/drivers/soc/bcm/brcmstb/common.c b/drivers/soc/bcm/brcmstb/common.c
index 94e7335..454f4c2 100644
--- a/drivers/soc/bcm/brcmstb/common.c
+++ b/drivers/soc/bcm/brcmstb/common.c
@@ -40,6 +40,18 @@ bool soc_is_brcmstb(void)
return of_match_node(brcmstb_machine_match, root) != NULL;
}
+u32 brcmstb_get_family_id(void)
+{
+ return family_id;
+}
+EXPORT_SYMBOL(brcmstb_get_family_id);
+
+u32 brcmstb_get_product_id(void)
+{
+ return product_id;
+}
+EXPORT_SYMBOL(brcmstb_get_product_id);
+
static const struct of_device_id sun_top_ctrl_match[] = {
{ .compatible = "brcm,brcmstb-sun-top-ctrl", },
{ }
diff --git a/include/linux/soc/brcmstb/brcmstb.h
b/include/linux/soc/brcmstb/brcmstb.h
index 337ce41..4bf5edd 100644
--- a/include/linux/soc/brcmstb/brcmstb.h
+++ b/include/linux/soc/brcmstb/brcmstb.h
@@ -1,10 +1,20 @@
#ifndef __BRCMSTB_SOC_H
#define __BRCMSTB_SOC_H
+#define BRCM_ID(reg) ((u32)reg >> 28 ? (u32)reg >> 16 : (u32)reg >> 8)
+#define BRCM_REV(reg) ((u32)reg & 0xff)
+
/*
* Bus Interface Unit control register setup, must happen early during boot,
* before SMP is brought up, called by machine entry point.
*/
void brcmstb_biuctrl_init(void);
+/*
+* Helper functions for getting family or product id from the
+* SoC driver.
+*/
+u32 brcmstb_get_family_id(void);
+u32 brcmstb_get_product_id(void);
+
#endif /* __BRCMSTB_SOC_H */
--
1.9.0.138.g2de3478
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3 0/2] Add Broadcom USB PHY driver for Broadcom STB SoCs
Add Broadcom USB PHY driver for Broadcom STB SoCs. This driver in combination with the generic ohci, ehci and xhci platform drivers will enable USB1.1, USB2.0 and USB3.0 support. V3 - Rebase to latest V2 - Change compatible name from "brcm,usb-phy" to "brcm,brcmstb-usb-phy" Al Cooper (2): soc: brcmstb: Add Product ID and Family ID helper functions usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver .../bindings/phy/brcm,brcmstb-usb-phy.txt | 39 + MAINTAINERS| 7 + drivers/phy/Kconfig| 10 + drivers/phy/Makefile | 2 + drivers/phy/phy-brcm-usb-init.c| 792 + drivers/phy/phy-brcm-usb-init.h| 49 ++ drivers/phy/phy-brcm-usb.c | 206 ++ drivers/soc/bcm/brcmstb/common.c | 12 + include/linux/soc/brcmstb/brcmstb.h| 10 + 9 files changed, 1127 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/brcm,brcmstb-usb-phy.txt create mode 100644 drivers/phy/phy-brcm-usb-init.c create mode 100644 drivers/phy/phy-brcm-usb-init.h create mode 100644 drivers/phy/phy-brcm-usb.c -- 1.9.0.138.g2de3478 -- 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] mfd: rtsx_usb: avoid setting ucr->current_sg.status
On Thu, 11 Aug 2016, Lu Baolu wrote: > Member "status" of struct usb_sg_request is managed by usb core. A > spin lock is used to serialize the change of it. The driver could > check the value of req->status, but should avoid changing it without > the hold of the spinlock. Otherwise, it could cause race or error > in usb core. > > This patch could be backported to stable kernels with version later > than v3.14. > > Cc: sta...@vger.kernel.org # 3.14+ > Cc: Alan Stern > Cc: Roger Tseng > Signed-off-by: Lu Baolu > --- > drivers/mfd/rtsx_usb.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Applied, thanks. > diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c > index dbd907d..691dab7 100644 > --- a/drivers/mfd/rtsx_usb.c > +++ b/drivers/mfd/rtsx_usb.c > @@ -46,9 +46,6 @@ static void rtsx_usb_sg_timed_out(unsigned long data) > > dev_dbg(&ucr->pusb_intf->dev, "%s: sg transfer timed out", __func__); > usb_sg_cancel(&ucr->current_sg); > - > - /* we know the cancellation is caused by time-out */ > - ucr->current_sg.status = -ETIMEDOUT; > } > > static int rtsx_usb_bulk_transfer_sglist(struct rtsx_ucr *ucr, > @@ -67,12 +64,15 @@ static int rtsx_usb_bulk_transfer_sglist(struct rtsx_ucr > *ucr, > ucr->sg_timer.expires = jiffies + msecs_to_jiffies(timeout); > add_timer(&ucr->sg_timer); > usb_sg_wait(&ucr->current_sg); > - del_timer_sync(&ucr->sg_timer); > + if (!del_timer_sync(&ucr->sg_timer)) > + ret = -ETIMEDOUT; > + else > + ret = ucr->current_sg.status; > > if (act_len) > *act_len = ucr->current_sg.bytes; > > - return ucr->current_sg.status; > + return ret; > } > > int rtsx_usb_transfer_data(struct rtsx_ucr *ucr, unsigned int pipe, -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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: [PATCHv6 1/3] usb: USB Type-C connector class
Hello Heikki, On Tue, Aug 30, 2016 at 11:22:27AM +0300, Heikki Krogerus wrote: > > > > If you are only interested in accessory mode support, maybe we don't need > > the 'type' attribute at all. We could make the 'accessory' attribute always > > visible and display one of "none", "Audio", "Debug", or "Digital Audio". > > It might also make sense to rename the attribute to "accessory_mode". > > That works for me. > > How about if I add the "supports_usb_power_delivery" attribute for the > partners instead to give some details about them. Any objections? > At first glance, the attribute name looks a bit awkward. Let me look into the specification to see what might make sense to report. On top of my head, I don't recall if we are able to report this for a dock which isn't currently connected to power. > > On a side note, while looking into this, I noticed the following: > > > > + if (port->cap->accessory) > > + for (accessory = port->cap->accessory, i = 0; > > +i < port->cap->num_accessory; accessory++, i++) > > + ret += sprintf(buf, "%s\n", > > + typec_accessory_modes[*accessory]); > > > > This means the list of supported accessories always starts with ", ". > > Where does it print ", "? > > I'm not sure what is wrong here, but I'll update this code in any Nothing. Looks like I lost my ability to read code. Somehow the ',' above made it into the string. There is some inconsistency in the output when compared to the other "supported" attributes, though. Here the supported modes are printed in consecutive lines; elsewhere they are printed in a single line with ',' as separator. Thanks, Guenter -- 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: [PATCHv6 1/3] usb: USB Type-C connector class
Heikki, On Tue, Aug 30, 2016 at 01:04:37PM +0300, Heikki Krogerus wrote: > Hi Oliver, > > On Tue, Aug 30, 2016 at 11:32:01AM +0200, Oliver Neukum wrote: > > On Mon, 2016-08-29 at 15:36 +0300, Heikki Krogerus wrote: > > > +What: /sys/class/typec//current_data_role > > > +Date: June 2016 > > > +Contact: Heikki Krogerus > > > +Description: > > > + The current USB data role the port is operating in. > > > This > > > + attribute can be used for requesting data role > > > swapping on the > > > + port. Swapping is only supported as an asynchronous > > > operation > > > + and requires polling of the attribute in order to know > > > the > > > + result, so successful write operation does not mean > > > successful > > > + swap. > > > + > > > > That is badly formulated. Does it mean that poll() or select() > > can be used or does the value need to be repearedly read? > > Does polling not always mean poll/select? > > > And how would you learn about an error? > > This is what I'm also really worried about. I'm now wondering did I > give up too easily on this to Guenter in hope to move this thing > forward. He said it's problematic to do these calls synchronously for > him. Was it something related to potential conflicting role swaps from > both ends? > > Guenter, can you please elaborate? And how do you plan to report > failures with the swaps? > Following up on this again. For the record, I never meant to suggest that the ABI to userspace should be asynchronous. On the contrary, I think it should be synchronous. Reason is that it simplifies user space for the general case, where user space does not mind the wait and wants to get a valid return code as part of the operation. The more complex case, where user space _does_ mind the wait, can always be handled by implementing a separate thread to write into sysfs attributes and wait for the result. This can be hidden in an application library which can also implement callbacks or signals to other threads as needed. It can also be tied with udev event handling to observe actual role changes (which will probably be necessary anyway). While this may sound complicated, the code necessary to implement and support an asynchronous kernel ABI would be at least as complicated, and would probably also require a polling thread and callbacks to report results to other threads. So I don't really see a gain for user space by providing an asynchronous kernel ABI. Thanks, Guenter -- 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 08/10] usb: gadget: remove useless parameter in alloc_ep_req()
On 29/08/16 08:55, Felipe Balbi wrote:
>
> Hi,
>
> Felipe Ferreri Tonello writes:
>>> Felipe Ferreri Tonello writes:
> "Felipe F. Tonello" writes:
>> The default_length parameter of alloc_ep_req was not really necessary
>> and gadget drivers would almost always create an inline function to pass
>> the same value to len and default_len.
>>
>> So this patch also removes duplicate code from few drivers.
>>
>> Signed-off-by: Felipe F. Tonello
>> ---
>> drivers/usb/gadget/function/f_hid.c| 10 ++
>> drivers/usb/gadget/function/f_loopback.c | 9 +
>> drivers/usb/gadget/function/f_midi.c | 10 ++
>> drivers/usb/gadget/function/f_sourcesink.c | 11 ++-
>> drivers/usb/gadget/u_f.c | 7 +++
>> drivers/usb/gadget/u_f.h | 3 +--
>> 6 files changed, 11 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_hid.c
>> b/drivers/usb/gadget/function/f_hid.c
>> index 51980c50546d..e82a7468252e 100644
>> --- a/drivers/usb/gadget/function/f_hid.c
>> +++ b/drivers/usb/gadget/function/f_hid.c
>> @@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, struct
>> file *fd)
>>
>> /*-*/
>> /*usb_function
>>*/
>>
>> -static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
>> -unsigned length)
>> -{
>> -return alloc_ep_req(ep, length, length);
>> -}
>
> actually, I prefer to keep these little helpers. I was recently playing
> with adding SG list support to g_zero (I should have patches soon) and
> it was actually very nice to have the sourcesink helper as I could just
> ditch alloc_ep_req(). The change to the driver was local to
> ss_alloc_ep_req() and nothing else changed :-)
>
Right, but then it is worth to have the helper function. In this
particular case, I am removing a useless helper functions, especially
that the previous patch removes the need for the optional parameter in
alloc_ep_req.
>>>
>>> it's a static inline :-) It won't do any bad to keep it. And, as I said,
>>> if we want to ditch aloc_ep_req() eventually, then we have just one
>>> place to patch ;-)
>>
>> Yes, sure. But why drop alloc_ep_req()?
>
> because we can't find a generic way of allocating sglist with enough
> entries :-) Some drivers (like f_fs.c) could actually have zero-copy
> sglist by just pinning user pages with get_user_pages_fast() and
> following with sg_alloc_from_pages().
>
> g_zero, however, would just "emulate" sglist by just allocating a
> statically sized sg table and initializing chunks of the linear req->buf
> to each sg entry.
I see. :)
>
>> So should I keep all these helper functions? If so, I actually still
>> need to fix them to use the newer alloc_ep_req() API.
>
> yeah, keeping the helper functions would be nice. IMHO, alloc_ep_req()
> doesn't have a long life, but it's pretty good for the time being.
Ok, I did it on my last patchset I sent, I think you already applied to
your tree.
--
Felipe
0x92698E6A.asc
Description: application/pgp-keys
Re: [PATCH] usb: renesas_usbhs: add a compatible string for r8a7796
On Wed, Aug 24, 2016 at 04:39:53PM +0900, Yoshihiro Shimoda wrote: > This patch adds support for r8a7796 (R-Car M3-W). > > Signed-off-by: Yoshihiro Shimoda > --- > Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 1 + > drivers/usb/renesas_usbhs/common.c | 4 > 2 files changed, 5 insertions(+) Acked-by: Rob Herring -- 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] net: pegasus: Remove deprecated create_singlethread_workqueue
The workqueue "pegasus_workqueue" queues a single work item per pegasus
instance and hence it doesn't require execution ordering. Hence,
alloc_workqueue has been used to replace the deprecated
create_singlethread_workqueue instance.
The WQ_MEM_RECLAIM flag has been set to ensure forward progress under
memory pressure since it's a network driver.
Since there are fixed number of work items, explicit concurrency
limit is unnecessary here.
Signed-off-by: Bhaktipriya Shridhar
---
drivers/net/usb/pegasus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 9bbe0161..1434e5d 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -1129,7 +1129,8 @@ static int pegasus_probe(struct usb_interface *intf,
return -ENODEV;
if (pegasus_count == 0) {
- pegasus_workqueue = create_singlethread_workqueue("pegasus");
+ pegasus_workqueue = alloc_workqueue("pegasus", WQ_MEM_RECLAIM,
+ 0);
if (!pegasus_workqueue)
return -ENOMEM;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: pegasus: Remove deprecated create_singlethread_workqueue
On 16-08-30 22:02:47, Bhaktipriya Shridhar wrote:
> The workqueue "pegasus_workqueue" queues a single work item per pegasus
> instance and hence it doesn't require execution ordering. Hence,
> alloc_workqueue has been used to replace the deprecated
> create_singlethread_workqueue instance.
>
> The WQ_MEM_RECLAIM flag has been set to ensure forward progress under
> memory pressure since it's a network driver.
>
> Since there are fixed number of work items, explicit concurrency
> limit is unnecessary here.
>
> Signed-off-by: Bhaktipriya Shridhar
> ---
> drivers/net/usb/pegasus.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 9bbe0161..1434e5d 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -1129,7 +1129,8 @@ static int pegasus_probe(struct usb_interface *intf,
> return -ENODEV;
>
> if (pegasus_count == 0) {
> - pegasus_workqueue = create_singlethread_workqueue("pegasus");
> + pegasus_workqueue = alloc_workqueue("pegasus", WQ_MEM_RECLAIM,
> + 0);
> if (!pegasus_workqueue)
> return -ENOMEM;
> }
> --
> 2.1.4
Nope, there is no need for singlethread-ness here. As long as the flag you
used
is doing the right thing i am OK with the patch.
Petko
--
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: [PATCHv6 1/3] usb: USB Type-C connector class
Heikki, On Tue, Aug 30, 2016 at 11:22:27AM +0300, Heikki Krogerus wrote: > > How about if I add the "supports_usb_power_delivery" attribute for the > partners instead to give some details about them. Any objections? > After looking into the code again, I assume the idea is to have the existing supports_usb_power_delivery attribute report if the local port supports the PD, and to have the partner attribute report if the partner supports the PD protocol. In other words, it would report the value of usb_pd in struct typec_partner. If so, I am ok with it. You might actually consider adding the same attribute to the cable attributes as well. Thanks, Guenter -- 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 00/28] media: don't print error when allocating urb fails
On Thu, Aug 11, 2016 at 11:03:36PM +0200, Wolfram Sang wrote: > This per-subsystem series is part of a tree wide cleanup. usb_alloc_urb() uses > kmalloc which already prints enough information on failure. So, let's simply > remove those "allocation failed" messages from drivers like we did already for > other -ENOMEM cases. gkh acked this approach when we talked about it at LCJ in > Tokyo a few weeks ago. I've taken all of these through the usb tree given the delay in response from the media developers :) 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] usb-storage: MAINTAINERS: Alan Stern is the new maintainer
On Thu, Aug 25, 2016 at 08:52:38AM -0700, Matthew Dharm wrote: > On Thu, Aug 25, 2016 at 8:30 AM, Alan Stern wrote: > > At Matt Dharm's request, I am taking over maintainership of the > > usb-storage driver. > > Alan is being too modest. I think he's been the de facto maintainer > already for quite some time; he's certainly much more active with > responding to questions and bug reports on the mailing lists. This > change just makes it official. It's something I've been thinking > about doing for a long time, and a recent e-mail from a project noting > that there are maintainers who have not submitted any patches in at > least 3 years finally got me off-the-bubble. > > > Signed-off-by: Alan Stern > Acked-by: Matthew Dharm Thanks Matthew for all of your work on the driver and the USB stack over the years, I hope you aren't going to run away, we can always use your help :) 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: chipidea: udc: kernel panic in isr_setup_status_phase
On Mon, Aug 29, 2016 at 06:24:03PM +0800, Peter Chen wrote: > Would you please measure the voltage of vbus within 1s at below two > conditions: > > - Just connect cable > - Just disconnect cable We found out that there was a problem with our hardware design! But first, here is the VBUS measurement during cable plug-in. https://s17.postimg.org/8ba3rgl6n/linux_kernel_panic_usb_otg_vbus_is_yellow.jpg (The yellow signal is USB_OTG_VBUS, please ignore the red one) The kernel panic occurs where the slip of paper with the arrow is. The signal looks normal to me, I don't think VBUS was the problem. The other theory was a GND problem.. After thorough investigation, we discovered that we connected all SHIELD pins of the USB micro OTG connector to the protective-earth ground (GND_PE) with a 1 MOhm resistor and a 1 nF cap in parallel. We changed that to connect the SHIELD pins to the same internal GND as the USB signals have and also replaced the 1 MOhm resistor with a 100 Ohm resistor as seen in the MCIMX6Q-SDB schematics. Since that change, the error did not appear again! :) The only signals I never checked are the differential high speed signals (USB_OTG_DN and USB_OTG_DP) because I don't have the necessary equipment like active differential probes for very high frequency measurements, .. Maybe with the connection of the shield to PE, the cable acted as an antenna and interfered with the high speed signals? (Although they are differential, so the same noise to both should not be a problem? It is still a mystery to me, what was really causing it) -- What do you think about still fixing that kernel panic in case something similar happens again for other users? If such a situation occurs, should we only avoid the NULL pointer dereference or print out an error message and stop the gadget driver? Regards, Clemens -- 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/9] staging: sm750fb: fix line length coding style issue in ddk750_chip.c
I was testing a patchset and I accidently sent it to a mailing list (I only intended to send it to myself) Apologies for the mixup. On 30 August 2016 at 08:36, Greg KH wrote: > On Mon, Aug 29, 2016 at 11:50:01PM +0300, Moshe Green wrote: >> Fix a line length warning found by the checkpatch.pl tool in >> ddk750_chip.c. >> >> Signed-off-by: Moshe Green >> --- >> drivers/staging/sm750fb/ddk750_chip.c | 2 +- > > What does this file have to do with the linux-usb mailing list? And > where are the other 8 patches in this series? > > confused, > > 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
crash in usb_hc_died+0x16 when unplugging usb-c dock
I have a USB-3.1 dock; sometimes I see a kernel panic when I unplug it, which hangs the entire system. It appears that the first unplug is successful; the second one normally triggers a crash. The situation is complicated by the fact that this USB bridge is behind a chain of PCI bridges, which appear to power down when nothing is plugged into the USB-C port. Greg K-H hinted via Bugzilla that this might be an area which is already under development, in which case, apologies for the noise; but I'd be interested in understanding a bit more about the problem, in any case. The diagnostics below were all collected on kernel 4.7.0. The start of the panic, as captured over a serial console, is as follows: [ 44.010232] BUG: unable to handle kernel NULL pointer dereference at (null) [ 44.010274] IP: [] usb_hc_died+0x16/0xc0 [ 44.010292] PGD 0 [ 44.010300] Oops: [#1] SMP [ 44.010310] Modules linked in: xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 ... and that's as far as it gets. I captured a dump via kdump; here is the result of 'bt' from 'crash': PID: 0 TASK: 88046c72cc40 CPU: 3 COMMAND: "swapper/3" #0 [88047ed83a70] machine_kexec at 8105abfb #1 [88047ed83ad0] __crash_kexec at 8111076d #2 [88047ed83b98] __crash_kexec at 81110845 #3 [88047ed83bb0] crash_kexec at 8111088b #4 [88047ed83bd0] oops_end at 81030b4b #5 [88047ed83bf8] no_context at 81069c1a #6 [88047ed83c58] __bad_area_nosemaphore at 81069ee1 #7 [88047ed83ca8] bad_area_nosemaphore at 8106a034 #8 [88047ed83cb8] __do_page_fault at 8106a3d0 #9 [88047ed83d20] do_page_fault at 8106a802 #10 [88047ed83d40] page_fault at 8185c2f8 [exception RIP: usb_hc_died+22] RIP: 81630316 RSP: 88047ed83df8 RFLAGS: 00010286 RAX: 88007a342000 RBX: RCX: 000b RDX: 81cf6f23 RSI: 81cf28fd RDI: RBP: 88047ed83e08 R8: 0002 R9: 000181c1 R10: 880466215b30 R11: R12: 880466214218 R13: 0296 R14: 001f R15: 88007a3422a4 ORIG_RAX: CS: 0010 SS: 0018 #11 [88047ed83e10] xhci_stop_endpoint_command_watchdog at 8167d997 #12 [88047ed83e80] call_timer_fn at 810ef755 #13 [88047ed83eb8] run_timer_softirq at 810f0106 #14 [88047ed83f28] __softirqentry_text_start at 8185cdd6 #15 [88047ed83f88] irq_exit at 81086513 #16 [88047ed83f98] smp_apic_timer_interrupt at 8185cbf2 #17 [88047ed83fb0] apic_timer_interrupt at 8185aee2 --- --- #18 [88046c747db0] apic_timer_interrupt at 8185aee2 [exception RIP: unknown or invalid address] RIP: 001f RSP: RFLAGS: 88047ed954d8 RAX: 0008 RBX: 01dc1e533b10 RCX: 0e98 RDX: RSI: 0018 RDI: 0001 RBP: 0001 R8: 88046c747eb8 R9: 0008 R10: 88047ed9fc08 R11: 81eb2978 R12: 01dc1fec1b00 R13: 88047ed8d640 R14: 810feab2 R15: 88046c747dd8 ORIG_RAX: CS: SS: ff10 bt: WARNING: possibly bogus exception frame #19 [88046c747e58] cpuidle_enter_state at 816e6e3b #20 [88046c747ec0] cpuidle_enter at 816e7017 #21 [88046c747ed0] call_cpuidle at 810c4a1a #22 [88046c747ee0] cpu_startup_entry at 810c4dfe #23 [88046c747f30] start_secondary at 81050675 Here's what lspci -vv reports, with the hub plugged in (without it, 01:00.0, 02:00.0, 02:01.0, 02:02.0 and 39:00.0 disappear): 00:00.0 Host bridge: Intel Corporation Sky Lake Host Bridge/DRAM Registers (rev 09) Subsystem: Dell Skylake Host Bridge/DRAM Registers Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0 Capabilities: [e0] Vendor Specific Information: Len=10 00:02.0 VGA compatible controller: Intel Corporation Sky Lake Integrated Graphics (rev 0a) (prog-if 00 [VGA controller]) DeviceName: Onboard IGD Subsystem: Dell Skylake Integrated Graphics Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0 Interrupt: pin A routed to IRQ 283 Region 0: Memory at db00 (64-bit, non-prefetchable) [size=16M] Region 2: Memory at 9000 (64-bit, prefetchable) [size=256M]
Re: [RESEND PATCH, v5 4/5] usb: Add MediaTek USB3 DRD Driver
On Fri, Aug 26, 2016 at 05:38:27PM +0800, chunfeng yun wrote:
> Hi,
>
> On Thu, 2016-08-25 at 10:32 +0200, Oliver Neukum wrote:
> > On Thu, 2016-08-25 at 11:05 +0800, Chunfeng Yun wrote:
> > > This patch adds support for the MediaTek USB3 controller
> > > integrated into MT8173. It can be configured as Dual-Role
> > > Device (DRD), Peripheral Only and Host Only (xHCI) modes.
> > >
> >
> > > +/**
> > > + * General Purpose Descriptor (GPD):
> > > + * The format of TX GPD is a little different from RX one.
> > > + * And the size of GPD is 16 bytes.
> > > + *
> > > + * @flag:
> > > + * bit0: Hardware Own (HWO)
> > > + * bit1: Buffer Descriptor Present (BDP), always 0, BD is not
> > > supported
> > > + * bit2: Bypass (BPS), 1: HW skips this GPD if HWO = 1
> > > + * bit7: Interrupt On Completion (IOC)
> > > + * @chksum: This is used to validate the contents of this GPD;
> > > + * If TXQ_CS_EN / RXQ_CS_EN bit is set, an interrupt is issued
> > > + * when checksum validation fails;
> > > + * Checksum value is calculated over the 16 bytes of the GPD by
> > > default;
> > > + * @data_buf_len (RX ONLY): This value indicates the length of
> > > + * the assigned data buffer
> > > + * @next_gpd: Physical address of the next GPD
> > > + * @buffer: Physical address of the data buffer
> > > + * @buf_len:
> > > + * (TX): This value indicates the length of the assigned data
> > > buffer
> > > + * (RX): The total length of data received
> > > + * @ext_len: reserved
> > > + * @ext_flag:
> > > + * bit5 (TX ONLY): Zero Length Packet (ZLP),
> > > + */
> > > +struct qmu_gpd {
> > > + u8 flag;
> > > + u8 chksum;
> > > + u16 data_buf_len;
> > > + u32 next_gpd;
> > > + u32 buffer;
> > > + u16 buf_len;
> > > + u8 ext_len;
> > > + u8 ext_flag;
> > > +} __packed;
> >
> > It looks like this is shared with hardware in memory.
> > But you leave the endianness of the bigger fields undeclared.
> >
> The driver only supports Little-Endian SoCs currently, because I have no
> Big-Endian platform to test it.
that's ok, you still have to mark the endian of the data and use it in
that manner, you can't just not worry about it.
Please fix up.
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 1/9] staging: sm750fb: fix line length coding style issue in ddk750_chip.c
On Tue, Aug 30, 2016 at 08:21:28PM +0300, moshe green wrote: > I was testing a patchset and I accidently sent it to a mailing list (I > only intended to send it to myself) > Apologies for the mixup. Ok, but it's odd that this even went to this mailing list, how did that even happen? Anyway, not a big deal, thanks for letting us know. 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: isoc pkt loss with pwc
Hi,
On Sun, Aug 28, 2016 at 01:13:55PM +0300, Matwey V. Kornilov wrote:
> Hello Bin,
>
> I would like to start new thread on my issue. Let me recall where the issue
> is:
> There is 100% frame lost in pwc webcam driver due to lots of
> zero-length packages coming from musb driver.
What is the video resolution and fps?
> The issue is present in all kernels (including 4.8) starting from the commit:
>
> f551e13529833e052f75ec628a8af7b034af20f9 ("Revert "usb: musb:
> musb_host: Enable HCD_BH flag to handle urb return in bottom half"")
What is the behavior without this commit?
>
> The issue is here both when DMA enabled and DMA disabled.
>
> Attached here is a plot. The vertical axis designates the value of
> rx_count variable from function musb_host_packet_rx(). One may see
> that there are only two possibilities: 0 bytes and 956 bytes.
> The horizontal axis is the last three digits of the timestamp when
> musb_host_packet_rx() invoked. I.e for [ 38.115379] it is 379. Given
> that my webcam is USB 1.1 and base time is 1 ms, then all frames
> should be grouped close to some single value. (Repeating package
> receive event every 1 ms won't change last tree digits considerably)
> One may see that it is not true, in practice there are two groups. And
> receive time strongly correlates with the package size. Packages
> received near round ms are 956 bytes long, packages received near 400
> us are 0 bytes long.
When the host IN packet passed the deadline, the device drops the video
data, so device only sends 0 byte packet (or 12 bytes which is only the
uvc header without data payload).
>
> I don't know how exactly SOF and IN are synchronized in musb, could
> someone give a hint? But from what I see the time difference between
> subsequent IN package requests is sometimes more than 1 ms due to
> heavy urb->complete() callback. After such events only zero length
Why musb delayed the IN packets, it needs to be investigated. I will
start to look at this webcam issue with musb soon, after I cleaned up
the musb patches queued recently. I will update once I have progress in
my investigation.
> packages are received. Surprisingly, that `synchronization' is
> recovered sometimes in the middle of URB like the following:
>
> [ 163.207363] musb int
> [ 163.207380] rx_count 0
> [ 163.207393] req pkt c9c76200 // Expected musb int at 163.208393
> [ 163.207403] int end
> // No interrupt at 163.208393
> [ 163.209001] musb int
> [ 163.209017] rx_count 956
> [ 163.209108] req pkt c9c76200
> [ 163.209118] int end
It looks like you used plain printk for these debug logs, which normally
is not a good idea for this type of performance debugging. printk
changes timing especially if the log is printed via uart console.
> And then the series of 956 bytes long packages are received until URB
> giveback will occasionally break it again.
> Do I understand correctly, that SOF is generated every 1 ms by
> hardware and should be followed by IN immediately?
My understanding is that is does not have to be 'immediately', for
example, in a few ns, it should be okay as long as the interval of IN
packets is fixed, but I forgot what the tolerance is, I haven't read the
related Specs for a long time.
> If so, it is not clear to me how they should be aligned when the time
> difference between to subsequent INs is greater than 1ms.
It is up to the device firmware, which should have an internal clock to
sync the received IN packets, and adjust the data payload to be send.
This is the basics in video/audio applications.
Regards,
-Bin.
>
> --
> With best regards,
> Matwey V. Kornilov.
> Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
> 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382
--
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: crash in usb_hc_died+0x16 when unplugging usb-c dock
On Tue, 30 Aug 2016, Richard van der Hoff wrote: > I have a USB-3.1 dock; sometimes I see a kernel panic when I unplug it, > which hangs the entire system. It appears that the first unplug is > successful; the second one normally triggers a crash. > > The situation is complicated by the fact that this USB bridge is behind > a chain of PCI bridges, which appear to power down when nothing is > plugged into the USB-C port. > > Greg K-H hinted via Bugzilla that this might be an area which is already > under development, in which case, apologies for the noise; but I'd be > interested in understanding a bit more about the problem, in any case. > > > The diagnostics below were all collected on kernel 4.7.0. > > The start of the panic, as captured over a serial console, is as follows: > > [ 44.010232] BUG: unable to handle kernel NULL pointer dereference at >(null) > [ 44.010274] IP: [] usb_hc_died+0x16/0xc0 > [ 44.010292] PGD 0 > [ 44.010300] Oops: [#1] SMP > [ 44.010310] Modules linked in: xt_CHECKSUM iptable_mangle > ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat > nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT > nf_reject_ipv4 > > ... and that's as far as it gets. Please try applying the patch at the bottom of this email: http://marc.info/?l=linux-usb&m=147195533502706&w=2 There's a good chance it will fix your problem. 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-storage: MAINTAINERS: Alan Stern is the new maintainer
On Tue, Aug 30, 2016 at 10:19 AM, Greg KH wrote: > On Thu, Aug 25, 2016 at 08:52:38AM -0700, Matthew Dharm wrote: >> On Thu, Aug 25, 2016 at 8:30 AM, Alan Stern >> wrote: >> > At Matt Dharm's request, I am taking over maintainership of the >> > usb-storage driver. >> >> > Signed-off-by: Alan Stern >> Acked-by: Matthew Dharm > > Thanks Matthew for all of your work on the driver and the USB stack over > the years, I hope you aren't going to run away, we can always use your > help :) Oh, I think I'll keep lurking like I have been for a few years now. I recognize that I've got a lot of historical perspective, and every so often someone asks about why a particular design decision was made. I'm also still hosting the usb-storage list, which doesn't get much traffic, but occasionally people go there for help; I'll continue to do that for as long as it's reasonably possible. Matt -- Matthew Dharm Former Maintainer, USB Mass Storage driver for Linux -- 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: Queries regrading USB-driver software-architecture
Hi Greg. Thanks for your support so far. I also contacted the support-personnel of the pc-in-a-box, and they too suggested upgrading the kernel. So, through yours and theirs inspiration, I did the following :: a) Upgraded from Ubuntu-14.04.2 to Ubuntu-14.04.3. b) Then (on 14.04.3), I upgraded to the latest 3.19 kernel, as per the steps at http://www.linuxandubuntu.com/home/linux-kernel-3-19-has-reached-to-end-with-last-release-kernel-3-19-8-install-upgrade-kernel-3-19-8-in-ubuntu-linux-mint The above caused the usb-serial-communication to start working perfectly. I guess step a) *could* be skipped, but I have a bit of time-constraint, so will stick to the two-step formula :P Thanks again to everyone for the help !!! Thanks and Regards, Ajay On Mon, Aug 29, 2016 at 6:38 PM, Greg KH wrote: > On Mon, Aug 29, 2016 at 06:26:30PM +0530, Ajay Garg wrote: >> Thanks Felipe and Greg for the replies. >> >> There is no one forcing us to use this kernel. >> Would >> http://releases.ubuntu.com/16.04.1/ubuntu-16.04.1-desktop-i386.iso.torrent?_ga=1.67231377.145502248.1467027188 >> be good enough to install? > > I have no idea, sorry, I don't use that distro :) > > greg k-h -- Regards, Ajay -- 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] leds: trigger: Introduce an USB port trigger
On 30 August 2016 at 14:05, Greg KH wrote:
> On Fri, Aug 26, 2016 at 05:38:05PM +0200, Rafał Miłecki wrote:
>> On 25 August 2016 at 14:49, Greg KH wrote:
>> > On Thu, Aug 25, 2016 at 10:03:52AM +0200, Rafał Miłecki wrote:
>> >> +static void usbport_trig_activate(struct led_classdev *led_cdev)
>> >> +{
>> >> + struct usbport_trig_data *usbport_data;
>> >> + int err;
>> >> +
>> >> + usbport_data = kzalloc(sizeof(*usbport_data), GFP_KERNEL);
>> >> + if (!usbport_data)
>> >> + return;
>> >> + usbport_data->led_cdev = led_cdev;
>> >> +
>> >> + /* Storing ports */
>> >> + INIT_LIST_HEAD(&usbport_data->ports);
>> >> + usbport_data->ports_dir = kobject_create_and_add("ports",
>> >> +
>> >> &led_cdev->dev->kobj);
>> >
>> > If you _ever_ find yourself in a driver having to use kobject calls,
>> > it's a HUGE hint that you are doing something wrong.
>> >
>> > Hint, you are doing this wrong :)
>> >
>> > Use an attribute group if you need a subdirectory in sysfs, using a
>> > "raw" kobject like this hides it from all userspace tools and so no
>> > userspace program can see it (hint, try using libudev to access these
>> > files attached to the device...)
>>
>> Oops. Thanks for pointing groups to me. I was looking at sysfs.h
>> earlier but I didn't realize group can be a subdirectory. I can see
>> these sysfs_create_group(s) and friends now, thanks.
>
> No, use an attribute group, and name it, and the subdir will be created
> automagically for you.
Oh, so I got it wrong again. Thanks for explaining.
>> >> + if (!usbport_data->ports_dir)
>> >> + goto err_free;
>> >> +
>> >> + /* API for ports management */
>> >> + err = device_create_file(led_cdev->dev, &dev_attr_new_port);
>> >> + if (err)
>> >> + goto err_put_ports;
>> >> + err = device_create_file(led_cdev->dev, &dev_attr_remove_port);
>> >> + if (err)
>> >> + goto err_remove_new_port;
>> >
>> > Doesn't this race with userspace and fail? Shouldn't the led core be
>> > creating your leds for you?
>>
>> These questions aren't clear to me. What kind of race? Doing
>> echo usbport > trigger
>> results in trigger core calling usbport_trig_activate. We create new
>> attributes and then we return.
>
> Why aren't the attributes present when the device is found? What is
> "trigger" for?
>
>> I'm also not creating any leds there. This already has to be LED
>> available if you want to assign some trigger to it.
>
> That sounds really odd...
Please take a look at Documentation to get some idea of LED triggers:
Documentation/leds/leds-class.txt
Basically a LED (/sys/class/leds/foo/) can be controller with
"brightness" sysfs file like this:
echo 0 > brightness
echo 5 > brightness
echo 255 > brightness
(most LEDs react only 0 and non-0 values).
As you quite often need more complex LED management, there are
triggers that were introduced in 2006 by c3bc9956ec52f ("[PATCH] LED:
add LED trigger tupport"). Some triggers are trivial and could be
implemented in userspace as well (e.g. "timer"). Some had to be
implemented in kernelspace (CPU activity, MTD activity, etc.). Having
few triggers compiled, you can assign them to LEDs at it pleases you.
Your hardware may have generic LED (not labeled) and you can
dynamically assign various triggers to it, depending e.g. on user
actions. E.g. if user (using GUI or whatever) wants to see flash
activity, your userspace script should do:
echo mtd > /sys/class/leds/foo/trigger
I hope it explains things a big and you can see know why trigger code
is independent of creating LEDs. It's about layers and making things
generic I believe.
There is a place for LED driver that is responsible for creating
"struct led_classdev" with proper callback setting brightness. It
provides LED name, but is unaware of the way it should be
lighted/turned off.
There is LED subsystem.
There are triggers that are unaware of LED hardware details and only
set LED brightness using generic API.
I'm obviously not author of all of that and I can't explain some
design decisions, but I hope I gave you a clue how it works.
>> >> +
>> >> + /* Notifications */
>> >> + usbport_data->nb.notifier_call = usbport_trig_notify,
>> >> + led_cdev->trigger_data = usbport_data;
>> >> + usb_register_notify(&usbport_data->nb);
>> >
>> > Don't abuse the USB notifier chain with stuff like this please, is that
>> > really necessary? Why can't your hardware just tell you what USB ports
>> > are in use "out of band"?
>>
>> I totally don't understand this part. This LED trigger is supposed to
>> react to USB devices appearing at specified ports. How else can I know
>> there is a new USB device if not by notifications?
>> I'm wondering if we are on the same page. Could it be my idea of this
>> LED trigger is not clear at all? Could you take a look at commit
>> message again, please? Mostly this part:
>> > This commit adds
Re: musb: isoc pkt loss with pwc
2016-08-30 21:30 GMT+03:00 Bin Liu :
> Hi,
>
> On Sun, Aug 28, 2016 at 01:13:55PM +0300, Matwey V. Kornilov wrote:
>> Hello Bin,
>>
>> I would like to start new thread on my issue. Let me recall where the issue
>> is:
>> There is 100% frame lost in pwc webcam driver due to lots of
>> zero-length packages coming from musb driver.
>
> What is the video resolution and fps?
640x480 YUV420 10 frames per second.
pwc uses proprietary compression during device-host transmission, but
I don't know how effective it is.
>
>> The issue is present in all kernels (including 4.8) starting from the commit:
>>
>> f551e13529833e052f75ec628a8af7b034af20f9 ("Revert "usb: musb:
>> musb_host: Enable HCD_BH flag to handle urb return in bottom half"")
>
> What is the behavior without this commit?
Without this commit all frames are being received correctly. Single
one issue is a number of zero byte package at the beginning of iso
stream (probably during camera internal sync, I have to check how the
stream is started on x86 later). But they are never repeated after
this.
>>
>> The issue is here both when DMA enabled and DMA disabled.
>>
>> Attached here is a plot. The vertical axis designates the value of
>> rx_count variable from function musb_host_packet_rx(). One may see
>> that there are only two possibilities: 0 bytes and 956 bytes.
>> The horizontal axis is the last three digits of the timestamp when
>> musb_host_packet_rx() invoked. I.e for [ 38.115379] it is 379. Given
>> that my webcam is USB 1.1 and base time is 1 ms, then all frames
>> should be grouped close to some single value. (Repeating package
>> receive event every 1 ms won't change last tree digits considerably)
>> One may see that it is not true, in practice there are two groups. And
>> receive time strongly correlates with the package size. Packages
>> received near round ms are 956 bytes long, packages received near 400
>> us are 0 bytes long.
>
> When the host IN packet passed the deadline, the device drops the video
> data, so device only sends 0 byte packet (or 12 bytes which is only the
> uvc header without data payload).
Doesn't it mean that free part of the frame, i.e (1 msec - (t_IN -
t_SOF)) is not enough to transmit 956 bytes in device firmware
opinion?
>
>>
>> I don't know how exactly SOF and IN are synchronized in musb, could
>> someone give a hint? But from what I see the time difference between
>> subsequent IN package requests is sometimes more than 1 ms due to
>> heavy urb->complete() callback. After such events only zero length
>
> Why musb delayed the IN packets, it needs to be investigated. I will
> start to look at this webcam issue with musb soon, after I cleaned up
> the musb patches queued recently. I will update once I have progress in
> my investigation.
The last package in URB has different code path.
IN after the last package of URB is not requested in musb_host_packet_rx().
Instead, IN request is performed in the end of musb_advance_schedule()
by musb_start_urb().
musb_advance_schedule() has the following code:
qh->is_ready = 0;
musb_giveback(musb, urb, status);
qh->is_ready = ready;
Which can be executed pretty long if urb->complete() handler is not
postphoned by HCD_BH.
In my case, it takes about 300 us for pwc urb->complete() to run.
Probably, the logic should be modified here, to run giveback on
current URB after the start of next URB.
>
>> packages are received. Surprisingly, that `synchronization' is
>> recovered sometimes in the middle of URB like the following:
>>
>> [ 163.207363] musb int
>> [ 163.207380] rx_count 0
>> [ 163.207393] req pkt c9c76200 // Expected musb int at 163.208393
>> [ 163.207403] int end
>> // No interrupt at 163.208393
>> [ 163.209001] musb int
>> [ 163.209017] rx_count 956
>> [ 163.209108] req pkt c9c76200
>> [ 163.209118] int end
>
> It looks like you used plain printk for these debug logs, which normally
> is not a good idea for this type of performance debugging. printk
> changes timing especially if the log is printed via uart console.
>
I think next time I will use tracepoints or something like that. Thank
you for pointing.
>> And then the series of 956 bytes long packages are received until URB
>> giveback will occasionally break it again.
>> Do I understand correctly, that SOF is generated every 1 ms by
>> hardware and should be followed by IN immediately?
>
> My understanding is that is does not have to be 'immediately', for
> example, in a few ns, it should be okay as long as the interval of IN
> packets is fixed, but I forgot what the tolerance is, I haven't read the
> related Specs for a long time.
But, If IN is postponed by 500 usec, then it means that half of frame
is wasted for isochronous transmission. Right?
>
>> If so, it is not clear to me how they should be aligned when the time
>> difference between to subsequent INs is greater than 1ms.
>
> It is up to the device firmware, which should have an internal clock to
> sync the received IN
Re: crash in usb_hc_died+0x16 when unplugging usb-c dock
On 30/08/16 19:48, Alan Stern wrote: On Tue, 30 Aug 2016, Richard van der Hoff wrote: I have a USB-3.1 dock; sometimes I see a kernel panic when I unplug it, which hangs the entire system. It appears that the first unplug is successful; the second one normally triggers a crash. Please try applying the patch at the bottom of this email: http://marc.info/?l=linux-usb&m=147195533502706&w=2 There's a good chance it will fix your problem. Ah yup, that does look promising. Thanks for the pointer, will give it a go and report back... -- 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] leds: trigger: Introduce an USB port trigger
On Tue, 30 Aug 2016, Rafał Miłecki wrote:
> Please take a look at Documentation to get some idea of LED triggers:
> Documentation/leds/leds-class.txt
>
> Basically a LED (/sys/class/leds/foo/) can be controller with
> "brightness" sysfs file like this:
> echo 0 > brightness
> echo 5 > brightness
> echo 255 > brightness
> (most LEDs react only 0 and non-0 values).
>
> As you quite often need more complex LED management, there are
> triggers that were introduced in 2006 by c3bc9956ec52f ("[PATCH] LED:
> add LED trigger tupport"). Some triggers are trivial and could be
> implemented in userspace as well (e.g. "timer"). Some had to be
> implemented in kernelspace (CPU activity, MTD activity, etc.). Having
> few triggers compiled, you can assign them to LEDs at it pleases you.
> Your hardware may have generic LED (not labeled) and you can
> dynamically assign various triggers to it, depending e.g. on user
> actions. E.g. if user (using GUI or whatever) wants to see flash
> activity, your userspace script should do:
> echo mtd > /sys/class/leds/foo/trigger
So for example, you might want to do:
echo usb1-4 >/sys/class/leds/foo/trigger
and then have the "foo" LED toggle whenever an URB was submitted or
completed for a device attached to the 1-4 port. Right?
> I hope it explains things a big and you can see know why trigger code
> is independent of creating LEDs. It's about layers and making things
> generic I believe.
>
> There is a place for LED driver that is responsible for creating
> "struct led_classdev" with proper callback setting brightness. It
> provides LED name, but is unaware of the way it should be
> lighted/turned off.
> There is LED subsystem.
> There are triggers that are unaware of LED hardware details and only
> set LED brightness using generic API.
>
> I'm obviously not author of all of that and I can't explain some
> design decisions, but I hope I gave you a clue how it works.
> >> >> + /* Notifications */
> >> >> + usbport_data->nb.notifier_call = usbport_trig_notify,
> >> >> + led_cdev->trigger_data = usbport_data;
> >> >> + usb_register_notify(&usbport_data->nb);
> >> >
> >> > Don't abuse the USB notifier chain with stuff like this please, is that
> >> > really necessary? Why can't your hardware just tell you what USB ports
> >> > are in use "out of band"?
> >>
> >> I totally don't understand this part. This LED trigger is supposed to
> >> react to USB devices appearing at specified ports. How else can I know
> >> there is a new USB device if not by notifications?
> >> I'm wondering if we are on the same page. Could it be my idea of this
> >> LED trigger is not clear at all? Could you take a look at commit
> >> message again, please? Mostly this part:
> >> > This commit adds a new trigger responsible for turning on LED when USB
> >> > device gets connected to the specified USB port. This can can useful for
> >> > various home routers that have USB port(s) and a proper LED telling user
> >> > a device is connected.
> >>
> >> Can I add something more to make it clear what this trigger is responsible
> >> for?
> >
> > Yes please, as I totally missed that.
> >
> > And the USB notifier was created for some "special" parts of the USB
> > core to use, this feels like an abuse of that in some way. But it's
> > hard to define, I know...
>
> I didn't mean to abuse this USB notifier, can you think of any other
> way to achieve the same result? We could think about modifying USB
> core to call some symbol from the trigger driver (see usage of
> ledtrig_mtd_activity symbol) but is it any better than using
> notification block?
I don't think this is such a bad use of the USB notifier. All you need
to know is when a device is attached to or unplugged from an LED's
port. Neither of these is a very frequent event.
However, there is still the question of how to know when an URB is
submitted or completed for the device in question. Obviously the USB
core would have to call an LED routine. But how would you determine
which LED(s) to toggle? Go through the entire list, looking for ones
that are bound to the USB device in question? This seems inefficient.
Use a hash table?
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 V4] leds: trigger: Introduce an USB port trigger
On 30 August 2016 at 22:54, Alan Stern wrote:
> On Tue, 30 Aug 2016, Rafał Miłecki wrote:
>
>> Please take a look at Documentation to get some idea of LED triggers:
>> Documentation/leds/leds-class.txt
>>
>> Basically a LED (/sys/class/leds/foo/) can be controller with
>> "brightness" sysfs file like this:
>> echo 0 > brightness
>> echo 5 > brightness
>> echo 255 > brightness
>> (most LEDs react only 0 and non-0 values).
>>
>> As you quite often need more complex LED management, there are
>> triggers that were introduced in 2006 by c3bc9956ec52f ("[PATCH] LED:
>> add LED trigger tupport"). Some triggers are trivial and could be
>> implemented in userspace as well (e.g. "timer"). Some had to be
>> implemented in kernelspace (CPU activity, MTD activity, etc.). Having
>> few triggers compiled, you can assign them to LEDs at it pleases you.
>> Your hardware may have generic LED (not labeled) and you can
>> dynamically assign various triggers to it, depending e.g. on user
>> actions. E.g. if user (using GUI or whatever) wants to see flash
>> activity, your userspace script should do:
>> echo mtd > /sys/class/leds/foo/trigger
>
> So for example, you might want to do:
>
> echo usb1-4 >/sys/class/leds/foo/trigger
>
> and then have the "foo" LED toggle whenever an URB was submitted or
> completed for a device attached to the 1-4 port. Right?
Not really as it won't cover some pretty common use cases. Many home
routers have few USB ports (2-5) and only 1 USB LED. It has to be
possible to assign few USB ports to a single LED (trigger). That way
LED should be turned on (and kept on) if there is at least 1 USB
device connected. You obviously can't do:
echo "usb1-1 usb1-2 usb2-1" > /sys/class/leds/foo/trigger
This was already brought up by Rob (who mentioned CPU trigger) and I
replied him pretty much the same way in:
https://lkml.org/lkml/2016/7/29/38
(reply starts with "Anyway, the serious limitation I see").
>> I hope it explains things a big and you can see know why trigger code
>> is independent of creating LEDs. It's about layers and making things
>> generic I believe.
>>
>> There is a place for LED driver that is responsible for creating
>> "struct led_classdev" with proper callback setting brightness. It
>> provides LED name, but is unaware of the way it should be
>> lighted/turned off.
>> There is LED subsystem.
>> There are triggers that are unaware of LED hardware details and only
>> set LED brightness using generic API.
>>
>> I'm obviously not author of all of that and I can't explain some
>> design decisions, but I hope I gave you a clue how it works.
>
>> >> >> + /* Notifications */
>> >> >> + usbport_data->nb.notifier_call = usbport_trig_notify,
>> >> >> + led_cdev->trigger_data = usbport_data;
>> >> >> + usb_register_notify(&usbport_data->nb);
>> >> >
>> >> > Don't abuse the USB notifier chain with stuff like this please, is that
>> >> > really necessary? Why can't your hardware just tell you what USB ports
>> >> > are in use "out of band"?
>> >>
>> >> I totally don't understand this part. This LED trigger is supposed to
>> >> react to USB devices appearing at specified ports. How else can I know
>> >> there is a new USB device if not by notifications?
>> >> I'm wondering if we are on the same page. Could it be my idea of this
>> >> LED trigger is not clear at all? Could you take a look at commit
>> >> message again, please? Mostly this part:
>> >> > This commit adds a new trigger responsible for turning on LED when USB
>> >> > device gets connected to the specified USB port. This can can useful for
>> >> > various home routers that have USB port(s) and a proper LED telling user
>> >> > a device is connected.
>> >>
>> >> Can I add something more to make it clear what this trigger is
>> >> responsible for?
>> >
>> > Yes please, as I totally missed that.
>> >
>> > And the USB notifier was created for some "special" parts of the USB
>> > core to use, this feels like an abuse of that in some way. But it's
>> > hard to define, I know...
>>
>> I didn't mean to abuse this USB notifier, can you think of any other
>> way to achieve the same result? We could think about modifying USB
>> core to call some symbol from the trigger driver (see usage of
>> ledtrig_mtd_activity symbol) but is it any better than using
>> notification block?
>
> I don't think this is such a bad use of the USB notifier. All you need
> to know is when a device is attached to or unplugged from an LED's
> port. Neither of these is a very frequent event.
>
> However, there is still the question of how to know when an URB is
> submitted or completed for the device in question. Obviously the USB
> core would have to call an LED routine. But how would you determine
> which LED(s) to toggle? Go through the entire list, looking for ones
> that are bound to the USB device in question? This seems inefficient.
> Use a hash table?
I was hoping to bring it to discussion later, as it seems even the
basic version of t
Re: [PATCH V3 2/2] usb: phy: phy-brcm-usb: Add Broadcom STB USB Phy driver
Hi Al, [auto build test ERROR on phy/next] [also build test ERROR on next-20160825] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] [Suggest to use git(>=2.9.0) format-patch --base= (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on] [Check https://git-scm.com/docs/git-format-patch for more information] url: https://github.com/0day-ci/linux/commits/Al-Cooper/soc-brcmstb-Add-Product-ID-and-Family-ID-helper-functions/20160830-224057 base: https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): >> ERROR: "brcm_usb_common_init" [drivers/phy/phy-brcm-usb.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 2/2] dmaengine: cppi41: Add basic PM runtime support
On Fri, Aug 19, 2016 at 03:59:40PM -0700, Tony Lindgren wrote: > Let's keep the device enabled between cppi41_dma_issue_pending() > and dmaengine_desc_get_callback_invoke() and rely on the PM runtime > autoidle timeout elsewhere. > > As the PM runtime is for whole device, not for each channel, > we need to queue pending transfers if the device is PM runtime > suspended. Then we start the pending transfers in PM runtime > resume. This fails to apply for me :( Can you please rebase and resend this one. -- ~Vinod -- 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] dmaengine: cppi41: Prepare to add PM runtime support
On Fri, Aug 19, 2016 at 03:59:39PM -0700, Tony Lindgren wrote: > Let's just move code from cppi41_dma_issue_pending() to > push_desc_queue() as that's the only call to push_desc_queue(). > > We want to do this for PM runtime as we need to call push_desc_queue() > also for pending queued transfers from PM runtime resume. > > No functional changes, just moves code around. Applied, now -- ~Vinod -- 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
