Re: [Bug] Suspend fails due to PM: Device 0000:00:1d.0 failed to suspend async: error -16
On Thu, Feb 22, 2018 at 11:26:37PM +, De La Torre Mena, ElizabethX wrote: > According to https://bugzilla.kernel.org/show_bug.cgi?id=198893#c1, I'm > sending this to report the following bug: > > It happens easily if I ran the test, that sends the machine to S3 and > autoresume, twice giving back an rtcwake error. I'm attaching the dmesg with > latest drm-tip. > Kernel 4.16.0-rc1-commit-67f1480+ > > From Bugzilla: > > I'm opening this case to follow up the issue on > https://bugs.freedesktop.org/show_bug.cgi?id=105130. > Machine is an IVB. > > Test outputs: > > > > igt@gem_exec_suspend@basic-s3: > > [...] > > Subtest basic-S3: FAIL (1.372s) > > [...] > > (gem_exec_suspend:12804) igt-core-INFO: [cmd] rtcwake: wakeup from "mem" > > using /dev/rtc0 at Thu Jan 25 06:10:24 2018 > > (gem_exec_suspend:12804) igt-core-WARNING: [cmd] rtcwake: write error > > (gem_exec_suspend:12804) igt-aux-WARNING: rtcwake failed with 1 > > Check dmesg for further details. > > (gem_exec_suspend:12804) igt-aux-DEBUG: suspend_stats: > > success: 0 > > fail: 1 > > failed_freeze: 0 > > failed_prepare: 0 > > failed_suspend: 1 > > failed_suspend_late: 0 > > failed_suspend_noirq: 0 > > failed_resume: 0 > > failed_resume_early: 0 > > failed_resume_noirq: 0 > > failures: > > last_failed_dev::00:1d.0 > > > > last_failed_errno: -16 > > 0 > > last_failed_step: suspend > > > > based on the above: > > $ lspci -s :00:1d.0 > > 00:1d.0 USB controller > > from dmesg: > > [ 133.856853] pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16 > [ 133.856863] dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns -16 > [ 133.856873] PM: Device :00:1d.0 failed to suspend async: error -16 > [ 133.957621] hp_wmi: Unknown event_id - 14 - 0x0 > [ 134.341326] PM: Some devices failed to suspend, or early wake event > detected > How is this a USB bug? Shouldn't this be a power management / PCI issue? 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
[Bug] Suspend fails due to PM: Device 0000:00:1d.0 failed to suspend async: error -16
According to https://bugzilla.kernel.org/show_bug.cgi?id=198893#c1, I'm sending this to report the following bug: It happens easily if I ran the test, that sends the machine to S3 and autoresume, twice giving back an rtcwake error. I'm attaching the dmesg with latest drm-tip. Kernel 4.16.0-rc1-commit-67f1480+ From Bugzilla: I'm opening this case to follow up the issue on https://bugs.freedesktop.org/show_bug.cgi?id=105130. Machine is an IVB. > Test outputs: > > igt@gem_exec_suspend@basic-s3: > [...] > Subtest basic-S3: FAIL (1.372s) > [...] > (gem_exec_suspend:12804) igt-core-INFO: [cmd] rtcwake: wakeup from "mem" > using /dev/rtc0 at Thu Jan 25 06:10:24 2018 > (gem_exec_suspend:12804) igt-core-WARNING: [cmd] rtcwake: write error > (gem_exec_suspend:12804) igt-aux-WARNING: rtcwake failed with 1 > Check dmesg for further details. > (gem_exec_suspend:12804) igt-aux-DEBUG: suspend_stats: > success: 0 > fail: 1 > failed_freeze: 0 > failed_prepare: 0 > failed_suspend: 1 > failed_suspend_late: 0 > failed_suspend_noirq: 0 > failed_resume: 0 > failed_resume_early: 0 > failed_resume_noirq: 0 > failures: > last_failed_dev::00:1d.0 > > last_failed_errno: -16 > 0 > last_failed_step: suspend based on the above: $ lspci -s :00:1d.0 00:1d.0 USB controller from dmesg: [ 133.856853] pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16 [ 133.856863] dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns -16 [ 133.856873] PM: Device :00:1d.0 failed to suspend async: error -16 [ 133.957621] hp_wmi: Unknown event_id - 14 - 0x0 [ 134.341326] PM: Some devices failed to suspend, or early wake event detected Also this seems like https://bugs.freedesktop.org/show_bug.cgi?id=103520, which was closed as fixed but never was found what actually fixed it. [reply] [−] Comment 1 Greg Kroah-Hartman 2018-02-22 21:19:12 UTC On Thu, Feb 22, 2018 at 06:55:58PM +, bugzilla-dae...@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=198893 > > Bug ID: 198893 >Summary: Suspend fails due to PM: Device :00:1d.0 failed > to suspend async: error -16 >Product: Drivers >Version: 2.5 > Kernel Version: 4.16.0-rc1 All USB bugs should be sent to the linux-usb@vger.kernel.org mailing list, and not entered into bugzilla. Please bring this issue up there, if it is still a problem in the latest kernel release. dmesg_log_pm_usb Description: dmesg_log_pm_usb
Re: [RFC PATCH] usb: hcd: complete URBs in threaded-IRQ context instead of tasklet
On Thu, 22 Feb 2018, Sebastian Andrzej Siewior wrote: > On 2018-02-16 16:46:41 [-0500], Alan Stern wrote: > > > The theaded interrupt runs SCHED_FIFO priority 50 by default. The only > > > thing that can interrupt it are interrupts, a softirq (not ksoftirqd) > > > and other tasks with a higher priority than 50. > > > There should be no downside performance wise. > > > > Maybe. It would be nice to see some real measurements. > > I had an usb3 flash stick behind the EHCI controller which was passed > through from the host to a kvm guest. The performance numbers in the > guest were equal (some noise was there) with and without the patch. > The numbers with the patch were worse if lockdep was enabled which isn't > much of a surprise. > If you have anything specific requirements for a measurement then please > let me know and I see what I can do. No, I didn't have anything more specific in mind. In principle then, using threaded-interrupt bottom halves instead of tasklets should be fine. I don't object to making such a change. However, using a work queue for root-hub URBs is pretty ugly. It would be better to reinstate the code that dropped hcd_root_hub_lock around root-hub givebacks, which was removed by commit 94dfd7edfd5c ("USB: HCD: support giveback of URB in tasklet context"); then it would be safe to give back those URBs in the bottom half. 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 v2] usb: host: ehci-platform: add support for optional external vbus supply
On Thu, 22 Feb 2018, Amelie DELAUNAY wrote: > > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > > @@ -19,6 +19,7 @@ Optional properties: > > - phys : phandle + phy specifier pair > > - phy-names : "usb" > > - resets : phandle + reset specifier pair > > + - vbus-supply : phandle of regulator supplying vbus > > > > Can platforms have more than one regulator e.g. one regulator per port? > > >>> > >>> I imagine that yes, platforms could have one regulator per port. > >>> Regulator consumers bindings impose a -supply property per > >>> regulator, so, what do you think about : > >>> vbus0-supply for port#0 > >>> vbus1-supply for port#1 > >>> ... > >>> vbusN-supply for port#N > >>> > >>> And then in probe, allocate 'struct regulator *vbus_supplies' with a > >>> size corresponding to 'HCS_N_PORTS(ehci->hcs_params) * sizeof(struct > >>> regulator *)'. > >>> And loop to get optional regulator vbus0, vbus1,..., vbusN. > >>> And then enable/disable the corresponding regulator in > >>> ehci_platform_port_power thanks to portnum. > >> > >> Looks fine to me but we need to get Alan's opinion if this is worth the > >> effort. > >> If there isn't a single platform needing it we could probably do without it > >> but the DT binding must be scalable to add this feature in the future. > > > > I agree that for now there don't seem to be any platforms requiring > > more than one regulator, but this should be implemented in a way that > > could be expanded if necessary. > > > > Anyway, the basic idea is reasonable. I don't know to what extent > > people want to power-off their EHCI ports, but if they do then we ought > > to turn off external regulators at the same time. > > > > Is there a real-life use case for this? > > > > Alan Stern > > > > On my setup I have the following: > > regulator_vbus > _ \ > | EHCI controller |-port0-[USB connector] > |_|-port1-X > > So, I have one regulator only for port0. But I could I have one more if > port1 was connected. My current regulator could also supplies port1. > > To allocate a vbus_supplies array depending on N_PORTS, I have to move > this initialization from probe to ehci_platform_reset, after ehci_setup > is done. > Then, I have to define each regulator id depending on the port number. > This imposes a binding like > - portN_vbus-supply : phandle of regulator supplying vbus for port N > But I don't know if we can describe it like this in dt-bindings ? > > &ehci { > ... > port0_vbus-supply = <&vbus_reg>; > port1_vbus-supply = <&vbus_reg>; //Could be another regulator, or not > specified if port is not connected. > ... > }; > > Is it ok to move vbus_supplies initialization in ehci_platform_reset ? Yes, it's okay to move the code if you need to. However, I can not speak on the DT implications. Someone who knows more about it should chime in. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbip: vudc: fix null pointer dereference on udc->lock
From: Colin Ian King Currently the driver attempts to spin lock on udc->lock before a NULL pointer check is performed on udc, hence there is a potential null pointer dereference on udc->lock. Fix this by moving the null check on udc before the lock occurs. Fixes: ea6873a45a22 ("usbip: vudc: Add SysFS infrastructure for VUDC") Signed-off-by: Colin Ian King --- drivers/usb/usbip/vudc_sysfs.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c index d86f7291..6dcd3ff655c3 100644 --- a/drivers/usb/usbip/vudc_sysfs.c +++ b/drivers/usb/usbip/vudc_sysfs.c @@ -105,10 +105,14 @@ static ssize_t usbip_sockfd_store(struct device *dev, struct device_attribute *a if (rv != 0) return -EINVAL; + if (!udc) { + dev_err(dev, "no device"); + return -ENODEV; + } spin_lock_irqsave(&udc->lock, flags); /* Don't export what we don't have */ - if (!udc || !udc->driver || !udc->pullup) { - dev_err(dev, "no device or gadget not bound"); + if (!udc->driver || !udc->pullup) { + dev_err(dev, "gadget not bound"); ret = -ENODEV; goto unlock; } -- 2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] usb: hcd: complete URBs in threaded-IRQ context instead of tasklet
On 2018-02-16 16:46:41 [-0500], Alan Stern wrote: > > The theaded interrupt runs SCHED_FIFO priority 50 by default. The only > > thing that can interrupt it are interrupts, a softirq (not ksoftirqd) > > and other tasks with a higher priority than 50. > > There should be no downside performance wise. > > Maybe. It would be nice to see some real measurements. I had an usb3 flash stick behind the EHCI controller which was passed through from the host to a kvm guest. The performance numbers in the guest were equal (some noise was there) with and without the patch. The numbers with the patch were worse if lockdep was enabled which isn't much of a surprise. If you have anything specific requirements for a measurement then please let me know and I see what I can do. > Alan Stern Sebastian -- 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 3/7] usb: dwc3: pci: Store device properties dynamically
On Thu, Feb 22, 2018 at 1:57 AM, Thinh Nguyen wrote: > On 2/21/2018 6:46 AM, Andy Shevchenko wrote: >> On Tue, Feb 20, 2018 at 11:12 PM, Thinh Nguyen >> wrote: >>> On 2/17/2018 7:29 AM, Andy Shevchenko wrote: On Fri, Feb 16, 2018 at 11:55 PM, Thinh Nguyen wrote: > Add the ability to add device properties dynamically. Currently, device > properties are added to platform device using > platform_device_add_properties(). However, this function does not allow > adding properties incrementally. It is useful to have this ability when > the driver needs to set common device properties across different HW I'm not sure it's useful anyhow. > or > if IP and FPGA validation test different configurations for different > HW. Shouldn't be a separate stuff for FPGA exclusively? >>> >>> Can you clarify/expand your question? >> >> FPGA is the one which might have different properties at run time for >> the same device. >> So, why do we care on driver / generic level of it? >> >> Shouldn't be FPGA manager take care of it (via DT overlays, for example)? > > Normally it is handled using DT overlays. But for using FPGA as PCI > device, I'm not aware of a better solution. If it's a PCI device, it may utilize PCI (hot plug if you would like to change IP at run time) with appropriate IDs and stuff, right? > Introduce two new functions to do so: >* dwc3_pci_add_one_property() - this function adds one property to > dwc->properties array and increase its size as needed >* dwc3_pci_add_properties() - this function takes a null terminated > array of device properties and add them to dwc->properties So, why you can't use ACPI / DT here? >>> dwc3_pci_add_properties() is a convenient function that takes statically >>> allocated array of (quirks) properties for different HW and store them >>> to dwc->properties. The idea is to add more properties on top of these >>> required quirks. >> >> Yes, I understand that. What's wrong with DT? The built-in device >> properties have the same nature as usual properties for DT. >> Whenever driver calls device_property_read_uXX() or alike it would >> check all property provides for asked one. > > I may not have explained fully in my commit message the purpose of this > change. That's why I think I misinterpreted your previous question. > With this new debugging feature, we want to delay adding device > properties until the user initiates it. So, see above. When user wants to enable this IP at run time it will be a PCI hot plug event which makes device appear behind PCI switch. When device appears it would have it's own VndrID/DevID + sub pair. Based on that IDs you may apply hard coded quirks (though I am against quirks in new hardware) as it's done right now. > Because the driver cannot use > platform_device_add_properties() to incrementally add properties to > platform device, we need a way to store properties so they can be added > in later time. So what? You don't need that if you do the right things right. > That's why I added these 2 new functions. > >> Other than that, quirks esp. for FPGA sounds so wrong. Why in the >> first place not to make non-broken hardware?! > > I may have used the term 'quirk' incorrectly since they were placed in > dwc3_pci_quirk(). There's no quirk for FPGA, but there are some initial > setup properties for FPGA. To be specific, these properties: > PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"), > PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"), > PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"), > PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"), OK. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Some questions about the UVC gadget
Hello Kelly, (CC'ing Paul Elder) On Thursday, 22 February 2018 11:03:42 EET Felipe Balbi wrote: > Kelly Huang writes: > > Dear Mr.Balbi, > > > > I am a college student from China. Recently, I am doing some research on > > the UVC gadget. After reading the source code, I found that the uvc gadget > > framework only supports two types of video streaming format, the > > UNCOMPRESSED and the MJPEG. > > > > Now, I am trying to add H.264 support. I wonder if the Linux kernel has > > already support it or not. It will be appreciated if you can give me some > > advice. > > > > Thank you for your time. > > It's a good idea to add mailing lists and other relevant people to the > loop. I'm afraid the Linux UVC gadget driver doesn't support H.264. While H.264 support could be implemented using UVC 1.1, I wouldn't recommend this as the UVC 1.1 H.264 specification is a hack that is not and will not be supported in the Linux UVC host driver. UVC 1.5 is the way to go for H.264. This shouldn't be too difficult to implement on the gadget side, but the host UVC driver also misses UVC 1.5 support. Paul has recently started working on the UVC gadget driver to revive it along with the userspace helper application. Further down on his to-do list he told me he would like to implement UVC 1.5 support on the host side, but that won't be for the near future (no pressure Paul :-)). -- Regards, Laurent Pinchart -- 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: Host & gadget transparent driver
On 02/22/2018 11:17 AM, Greg KH wrote: On Thu, Feb 22, 2018 at 10:42:17AM +0100, Krzysztof Opasiak wrote: On 02/22/2018 10:32 AM, Ran Shalit wrote: [...] I don't know the exact case nor I'm license specialist but I think it's doable without license violation. Take a look at official displaylink drivers[1]... They consist of two parts. First one is Kernel GPL code[2] which is obviously published to not violate the license and second part is userspace binary blob that receives video stream from this "open source driver" and then uses libusb to communicate with the device... Footnotes: 1 - http://www.displaylink.com/downloads/ubuntu 2 - https://github.com/DisplayLink/evdi Best regards, Krzysztof , I will check the proxy git . What's the difference between the proxy you've given before to the DisplayLink? Displaylink driver is unrelated to the proxy. It's just an example of how to not violate GPL license and not publish your USB protocol and provide only binary blob with it;) Um, do not say "how not to violate GPL" without being a lawyer please. You are absolutely right and what I wrote above it's only my own "feeling" not any lawyer statement. I wouldn't bet that what they are doing is all so cut-and-dry... Me neither so I'm only letting know that such things are happening in the wild and if you know some specialist you may let him know about this;) If it turns out that it's bad and you/LF manage to persuade DL to open their driver because of that, then oh man you are a magician for me:) Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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
[GIT PULL] USB fixes for 4.16-rc3
The following changes since commit 7928b2cbe55b2a410a0f5c1f154610059c57b1b2: Linux 4.16-rc1 (2018-02-11 15:04:29 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.16-rc3 for you to fetch changes up to 44eb5e12b845cc8a0634f21b70ef07d774eb4b25: Revert "usb: musb: host: don't start next rx urb if current one failed" (2018-02-20 15:02:46 +0100) USB fixes for 4.16-rc3 Here are a number of USB fixes for 4.16-rc3 Nothing major, but a number of different fixes all over the place in the USB stack for reported issues. Mostly gadget driver fixes, although the typical set of xhci bugfixes are there, along with some new quirks additions as well. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman AMAN DEEP (1): usb: ohci: Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks() Andreas Kemnade (1): usb: musb: fix enumeration after resume Bin Liu (1): Revert "usb: musb: host: don't start next rx urb if current one failed" Brian Norris (1): usb: dwc3: Undo PHY init if soft reset fails Dominik Bozek (1): usb: cdc_acm: prevent race at write to acm while system resumes Enric Balletbo i Serra (1): usb: dwc3: of-simple: fix oops by unbalanced clk disable call Fabio Estevam (1): usb: phy: mxs: Fix NULL pointer dereference on i.MX23/28 Greg Kroah-Hartman (1): Merge tag 'fixes-for-v4.16-rc2' of git://git.kernel.org/.../balbi/usb into usb-linus Jack Pham (2): usb: gadget: f_fs: Process all descriptors during bind usb: gadget: f_fs: Use config_ep_by_speed() Jack Stocker (1): Add delay-init quirk for Corsair K70 RGB keyboards Joe Lee (1): xhci: workaround for AMD Promontory disabled ports wakeup John Keeping (1): usb: gadget: f_uac2: fix bFirstInterface in composite gadget Karsten Koop (1): usb: ldusb: add PIDs for new CASSY devices supported by this driver Kristian Evensen (1): USB: serial: option: Add support for Quectel EP06 Manu Gautam (2): usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode usb: gadget: core: Fix use-after-free of usb_request Mathias Nyman (1): xhci: Don't print a warning when setting link state for disabled ports Minas Harutyunyan (2): usb: dwc2: Add safety check in setting of descriptor chain pointers usb: dwc2: Add safety check for STSPHSERCVD intr Peter Chen (2): usb: host: ehci: use correct device pointer for dma ops usb: host: ehci: always enable interrupt for qtd completion at test mode Roger Quadros (2): usb: dwc3: omap: don't miss events during suspend/resume usb: dwc3: core: Fix ULPI PHYs and prevent phy_get/ulpi_init during suspend/resume Shigeru Yoshida (1): ohci-hcd: Fix race condition caused by ohci_urb_enqueue() and io_watchdog_func() Shuah Khan (1): usbip: keep usbip_device sockfd state in sync with tcp_socket Stefan Agner (1): usb: gadget: fsl_udc_core: fix ep valid checks Thinh Nguyen (3): usb: dwc3: gadget: Set maxpacket size for ep0 IN usb: dwc3: ep0: Reset TRB counter for ep0 IN usb: dwc3: Fix GDBGFIFOSPACE_TYPE values Ulf Magnusson (1): usb: gadget: udc: Remove USB_GADGET_DUALSPEED select Vardan Mikayelyan (1): usb: dwc2: Fix dwc2_hsotg_core_init_disconnected() Wei Yongjun (1): USB: gadget: udc: Add missing platform_device_put() on error in bdc_pci_probe() Yoshihiro Shimoda (3): usb: gadget: udc: renesas_usb3: fix oops in renesas_usb3_remove() usb: renesas_usbhs: missed the "running" flag in usb_dmac with rx path usb: renesas_usbhs: missed the "running" flag in usb_dmac with rx path Zhengjun Xing (4): xhci: Fix NULL pointer in xhci debugfs xhci: Fix xhci debugfs devices node disappearance after hibernation xhci: xhci debugfs device nodes weren't removed after device plugged out xhci: fix xhci debugfs errors in xhci_stop drivers/hid/hid-ids.h | 3 + drivers/hid/hid-quirks.c | 3 + drivers/usb/class/cdc-acm.c | 9 ++- drivers/usb/core/quirks.c | 3 + drivers/usb/dwc2/gadget.c | 26 drivers/usb/dwc3/core.c | 86 +++ drivers/usb/dwc3/core.h | 21 --- drivers/usb/dwc3/dwc3-of-simple.c | 1 + drivers/usb/dwc3/dwc3-omap.c | 16 + drivers/usb/dwc3/ep0.c| 7 ++- drivers/usb/dwc3/gadget.c | 2 + drivers/usb/gadget/function/f_fs.c| 44 +++--- drivers/usb/gadget/function/f_uac2.c | 2 + drivers/usb/gadget/udc/Kconfig| 1 - drivers/usb/gadget/udc/bdc/bdc_pci.c | 1 + drivers/usb/gadget/ud
Re: [PATCH v2] usb: host: ehci-platform: add support for optional external vbus supply
Hi, On 02/20/2018 07:10 PM, Alan Stern wrote: > On Tue, 20 Feb 2018, Roger Quadros wrote: > >> On 20/02/18 16:46, Amelie DELAUNAY wrote: >>> Hi, >>> >>> On 02/20/2018 03:00 PM, Roger Quadros wrote: Hi, On 20/02/18 14:58, Amelie Delaunay wrote: > On some boards, especially when vbus supply requires large current, > and the charge pump on the PHY isn't enough, an external vbus power switch > may be used. > Add support for this optional external vbus supply in ehci-platform. > > Signed-off-by: Amelie Delaunay > > --- > Changes in v2: >* Address Roger Quadros comments: move regulator_enable/disable from > ehci_platform_power_on/off to ehci_platform_port_power. > --- >Documentation/devicetree/bindings/usb/usb-ehci.txt | 1 + >drivers/usb/host/ehci-platform.c | 31 > ++ >2 files changed, 32 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt > b/Documentation/devicetree/bindings/usb/usb-ehci.txt > index 3efde12..fc480cd 100644 > --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt > @@ -19,6 +19,7 @@ Optional properties: > - phys : phandle + phy specifier pair > - phy-names : "usb" > - resets : phandle + reset specifier pair > + - vbus-supply : phandle of regulator supplying vbus > Can platforms have more than one regulator e.g. one regulator per port? >>> >>> I imagine that yes, platforms could have one regulator per port. >>> Regulator consumers bindings impose a -supply property per >>> regulator, so, what do you think about : >>> vbus0-supply for port#0 >>> vbus1-supply for port#1 >>> ... >>> vbusN-supply for port#N >>> >>> And then in probe, allocate 'struct regulator *vbus_supplies' with a >>> size corresponding to 'HCS_N_PORTS(ehci->hcs_params) * sizeof(struct >>> regulator *)'. >>> And loop to get optional regulator vbus0, vbus1,..., vbusN. >>> And then enable/disable the corresponding regulator in >>> ehci_platform_port_power thanks to portnum. >> >> Looks fine to me but we need to get Alan's opinion if this is worth the >> effort. >> If there isn't a single platform needing it we could probably do without it >> but the DT binding must be scalable to add this feature in the future. > > I agree that for now there don't seem to be any platforms requiring > more than one regulator, but this should be implemented in a way that > could be expanded if necessary. > > Anyway, the basic idea is reasonable. I don't know to what extent > people want to power-off their EHCI ports, but if they do then we ought > to turn off external regulators at the same time. > > Is there a real-life use case for this? > > Alan Stern > On my setup I have the following: regulator_vbus _ \ | EHCI controller |-port0-[USB connector] |_|-port1-X So, I have one regulator only for port0. But I could I have one more if port1 was connected. My current regulator could also supplies port1. To allocate a vbus_supplies array depending on N_PORTS, I have to move this initialization from probe to ehci_platform_reset, after ehci_setup is done. Then, I have to define each regulator id depending on the port number. This imposes a binding like - portN_vbus-supply : phandle of regulator supplying vbus for port N But I don't know if we can describe it like this in dt-bindings ? &ehci { ... port0_vbus-supply = <&vbus_reg>; port1_vbus-supply = <&vbus_reg>; //Could be another regulator, or not specified if port is not connected. ... }; Is it ok to move vbus_supplies initialization in ehci_platform_reset ? Regards, Amelie >> And what if it is ganged power? i.e. one regulator for more than one port. >> Probably they all can point to the same regulator instance and it should >> work. >> >>> >Example (Sequoia 440EPx): >ehci@e300 { > diff --git a/drivers/usb/host/ehci-platform.c > b/drivers/usb/host/ehci-platform.c > index b065a96..05be100 100644 > --- a/drivers/usb/host/ehci-platform.c > +++ b/drivers/usb/host/ehci-platform.c > @@ -29,6 +29,7 @@ >#include >#include >#include > +#include >#include >#include >#include > @@ -46,6 +47,7 @@ struct ehci_platform_priv { > struct reset_control *rsts; > struct phy **phys; > int num_phys; > + struct regulator *vbus_supply; > bool reset_on_resume; >}; > > @@ -76,6 +78,25 @@ static int ehci_platform_reset(struct usb_hcd *hcd) > return 0; >} > > +static int ehci_platform_port_power(struct usb_hcd *hcd, int portnum, > + bool enable) > +
Re: Host & gadget transparent driver
On Thu, Feb 22, 2018 at 10:42:17AM +0100, Krzysztof Opasiak wrote: > > > On 02/22/2018 10:32 AM, Ran Shalit wrote: > [...] > > > > > > I don't know the exact case nor I'm license specialist but I think it's > > > doable without license violation. Take a look at official displaylink > > > drivers[1]... They consist of two parts. First one is Kernel GPL code[2] > > > which is obviously published to not violate the license and second part is > > > userspace binary blob that receives video stream from this "open source > > > driver" and then uses libusb to communicate with the device... > > > > > > Footnotes: > > > 1 - http://www.displaylink.com/downloads/ubuntu > > > 2 - https://github.com/DisplayLink/evdi > > > Best regards, > > > > > > > Krzysztof , I will check the proxy git . > > What's the difference between the proxy you've given before to the > > DisplayLink? > > > > Displaylink driver is unrelated to the proxy. It's just an example of how to > not violate GPL license and not publish your USB protocol and provide only > binary blob with it;) Um, do not say "how not to violate GPL" without being a lawyer please. I wouldn't bet that what they are doing is all so cut-and-dry... good luck! 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] staging: typec: handle vendor defined part and modify drp toggling flow
Hi, > -Original Message- > From: linux-usb-ow...@vger.kernel.org > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of ShuFanLee > Sent: 2018年2月21日 23:02 > To: heikki.kroge...@linux.intel.com; li...@roeck-us.net; g...@kroah.com > Cc: shufan_...@richtek.com; cy_hu...@richtek.com; > linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org > Subject: [PATCH] staging: typec: handle vendor defined part and modify drp > toggling flow > > From: ShuFanLee > > Handle vendor defined behavior in tcpci_init, tcpci_set_vconn and export > tcpci_irq. > More operations can be extended in tcpci_data if needed. > According to TCPCI specification, 4.4.5.2 ROLE_CONTROL, TCPC shall not > start DRP toggling until subsequently the TCPM writes to the COMMAND > register to start DRP toggling. My understanding of above statement is TCPM(this Linux driver) can enable DRP and CC1/CC2 in one shot, but TCPC(your typec chip internal firmware) needs wait until TCPM writes to COMMAND register to start drp toggling. > DRP toggling flow is chagned as following: > - Write DRP = 0 & Rd/Rd Why fixed to be Rd/Rd? in this case, it means the starting value: Tcpci 4.4.5.2: "When initiating autonomous DRP toggling, the TCPM shall write B6 (DRP) =1b and the starting value of Rp/Rd to B3..0 (CC1/CC2) to indicate DRP autonomous toggling mode to the TCPC." > - Write DRP = 1 What's your problem if combine above 2 in one shot? > - Set LOOK4CONNECTION command > > Signed-off-by: ShuFanLee > --- > drivers/staging/typec/tcpci.c | 128 > +- > drivers/staging/typec/tcpci.h | 13 + > 2 files changed, 115 insertions(+), 26 deletions(-) > > patch changelogs between v1 & v2 > - Remove unnecessary i2c_client in the structure of tcpci > - Rename structure of tcpci_vendor_data to tcpci_data > - Not exporting tcpci read/write wrappers but register i2c regmap in glue > driver > - Add set_vconn ops in tcpci_data >(It is necessary for RT1711H to enable/disable idle mode before > disabling/enabling vconn) > - Export tcpci_irq so that vendor can call it in their own IRQ handler > > patch changelogs between v2 & v3 > - Change the return type of tcpci_irq from int to irqreturn_t > > diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c > index 9bd4412..4959c69 100644 > --- a/drivers/staging/typec/tcpci.c > +++ b/drivers/staging/typec/tcpci.c > @@ -21,7 +21,6 @@ > > struct tcpci { > struct device *dev; > - struct i2c_client *client; > > struct tcpm_port *port; > > @@ -30,6 +29,12 @@ struct tcpci { > bool controls_vbus; > > struct tcpc_dev tcpc; > + struct tcpci_data *data; > +}; > + > +struct tcpci_chip { > + struct tcpci *tcpci; > + struct tcpci_data data; > }; > > static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) @@ -37,8 > +42,7 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) > return container_of(tcpc, struct tcpci, tcpc); } > > -static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, > - u16 *val) > +static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 > +*val) > { > return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16)); } @@ > -98,8 +102,10 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum > typec_cc_status cc) static int tcpci_start_drp_toggling(struct tcpc_dev > *tcpc, > enum typec_cc_status cc) > { > + int ret; > struct tcpci *tcpci = tcpc_to_tcpci(tcpc); > - unsigned int reg = TCPC_ROLE_CTRL_DRP; > + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << > TCPC_ROLE_CTRL_CC1_SHIFT) | > +(TCPC_ROLE_CTRL_CC_RD << > TCPC_ROLE_CTRL_CC2_SHIFT); > > switch (cc) { > default: > @@ -117,7 +123,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev > *tcpc, > break; > } > > - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + if (ret < 0) > + return ret; > + usleep_range(500, 1000); Why need a wait here? It seems you actually don't want to do autonomously toggling from the very beginning, instead, you begin with a directly control on CC, keep it(Rd) for some time, then switch to use autonomously toggling, why you need such kind of sequence? I think it's special and not following tcpci spec. > + reg |= TCPC_ROLE_CTRL_DRP; > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + if (ret < 0) > + return ret; > + ret = regmap_write(tcpci->regmap, TCPC_COMMAND, > +TCPC_CMD_LOOK4CONNECTION); > + if (ret < 0) > + return ret; > + return 0; > } > > static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink) > @@ -178,6 +196,16 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, > bool enable) > struct tcpci *tcpci = tcpc_t
Re: Host & gadget transparent driver
On 02/22/2018 10:32 AM, Ran Shalit wrote: [...] I don't know the exact case nor I'm license specialist but I think it's doable without license violation. Take a look at official displaylink drivers[1]... They consist of two parts. First one is Kernel GPL code[2] which is obviously published to not violate the license and second part is userspace binary blob that receives video stream from this "open source driver" and then uses libusb to communicate with the device... Footnotes: 1 - http://www.displaylink.com/downloads/ubuntu 2 - https://github.com/DisplayLink/evdi Best regards, Krzysztof , I will check the proxy git . What's the difference between the proxy you've given before to the DisplayLink? Displaylink driver is unrelated to the proxy. It's just an example of how to not violate GPL license and not publish your USB protocol and provide only binary blob with it;) Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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: Host & gadget transparent driver
On Thu, Feb 22, 2018 at 10:46 AM, Krzysztof Opasiak wrote: > > > On 02/22/2018 07:53 AM, Greg KH wrote: >> >> On Wed, Feb 21, 2018 at 11:23:26PM +0200, Ran Shalit wrote: >>> >>> Hello, >>> >>> I am facing the following challenge: >>> >>> We have a camera device, and a ready drivers in the following >>> configuration: >>> >>> (1) host <--> camera >>> >>> The drivers for host is a binary, i.e. source code is probably not >>> available, and also the protocol datasheet is probably not available. >> >> >> Really? A USB host driver that is not released under the GPL? That's >> really difficult to imagine, but you are on your own here as that's an >> obvious license violation. Please go talk to your lawyers about this >> problem, or you will have bigger problems if you try to rely on this. >> It is using FLIR camera, a vision camera (FLIR) which uses generic Linux driver (Yet, I am not sure yet which because I don't have that camera with me yet) maybe it's uvc drivers ( do they support vision usb3 ?) , I also see usb3vision git, but didn't find it in mainline (?). https://github.com/ni/usb3vision/blob/master/u3v.h I know that it use standard GenICam (which kernel driver support it?) Yet, My question was more as a general question about if such "transparent drivers" exist and if not what the general concept of writing such driver. As Krzysztof said, it can actually be a userspace driver. > > I don't know the exact case nor I'm license specialist but I think it's > doable without license violation. Take a look at official displaylink > drivers[1]... They consist of two parts. First one is Kernel GPL code[2] > which is obviously published to not violate the license and second part is > userspace binary blob that receives video stream from this "open source > driver" and then uses libusb to communicate with the device... > > Footnotes: > 1 - http://www.displaylink.com/downloads/ubuntu > 2 - https://github.com/DisplayLink/evdi > Best regards, > Krzysztof , I will check the proxy git . What's the difference between the proxy you've given before to the DisplayLink? Thanks, Ran > -- > Krzysztof Opasiak > Samsung R&D Institute Poland > Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Some questions about the UVC gadget
Hi, Kelly Huang writes: > Dear Mr.Balbi, > > I am a college student from China. Recently, I am doing some research on > the UVC gadget. After reading the source code, I found that the uvc gadget > framework only supports two types of video streaming format, the > UNCOMPRESSED and the MJPEG. > > Now, I am trying to add H.264 support. I wonder if the Linux kernel has > already support it or not. It will be appreciated if you can give me some > advice. > > Thank you for your time. It's a good idea to add mailing lists and other relevant people to the loop. -- balbi signature.asc Description: PGP signature
Re: Host & gadget transparent driver
On 02/22/2018 07:53 AM, Greg KH wrote: On Wed, Feb 21, 2018 at 11:23:26PM +0200, Ran Shalit wrote: Hello, I am facing the following challenge: We have a camera device, and a ready drivers in the following configuration: (1) host <--> camera The drivers for host is a binary, i.e. source code is probably not available, and also the protocol datasheet is probably not available. Really? A USB host driver that is not released under the GPL? That's really difficult to imagine, but you are on your own here as that's an obvious license violation. Please go talk to your lawyers about this problem, or you will have bigger problems if you try to rely on this. I don't know the exact case nor I'm license specialist but I think it's doable without license violation. Take a look at official displaylink drivers[1]... They consist of two parts. First one is Kernel GPL code[2] which is obviously published to not violate the license and second part is userspace binary blob that receives video stream from this "open source driver" and then uses libusb to communicate with the device... Footnotes: 1 - http://www.displaylink.com/downloads/ubuntu 2 - https://github.com/DisplayLink/evdi Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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: Host & gadget transparent driver
On 02/21/2018 10:23 PM, Ran Shalit wrote: Hello, I am facing the following challenge: We have a camera device, and a ready drivers in the following configuration: (1) host <--> camera The drivers for host is a binary, i.e. source code is probably not available, and also the protocol datasheet is probably not available. But we need to use the camera in the following configuration: (2) host <---> embedded board <--> camera In other words, there is a transparent medium in between the host and device. I would like to ask what is the general concept for implementing the drivers in the embedded board ? Does these task requires reverse engineering ? Is there any known example or driver which does something similar ? I use to do such sick things like this. It can be easily done using USBProxy[1] (libusb + gadgetfs) but I'm not sure if it supports iso transfers which I assume is required for your camera... Footnotes: 1 - https://github.com/dominicgs/USBProxy -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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