Re: patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality
On Thu, 2018-04-12 at 07:20 +0200, Ard Biesheuvel wrote: > On 12 April 2018 at 07:05, Bockholdt Arne >wrote: > > > > On Wed, 2018-04-11 at 15:02 +0100, Marc Zyngier wrote: > > > > On Wed, 11 Apr 2018 14:11:52 +0100, > > Bockholdt Arne wrote: > > > > > > Hi all, > > > > is there anything new, I've just tried the new stable 4.16.1 kernel > > without any change. The Renesys USB3 controller is still not > > functional. I'm willing to test any patch that is based on a stable > > kernel version because the machine is in production use. > > > > > > Have you tested the branch[1] I mentioned in my previous email? > > Without your feedback, I cannot really make much progress on this. > > > > Thanks, > > > > M. > > > > [1] https://www.spinics.net/lists/linux-usb/msg167301.html > > > > > > It seems the repo is based on 3.9 kernel and not on a current > > stable branch, > > isn't it? It's rather old, I'm not sure my setup will work with > > this old > > kernel. > > > > Are you sure you pulled the correct branch? usb/uPD720202-reset has > the following on top of *v4.16-rc6* > > Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA > issue" > xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers > > (https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.gi > t/log/?h=usb/uPD720202-reset) Okay, this was my fault, sorry. I'm just compiling the kernel with my config and will give it a try at the weekend. Thanks, ArneN�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
Re: patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality
On 12 April 2018 at 07:05, Bockholdt Arnewrote: > > On Wed, 2018-04-11 at 15:02 +0100, Marc Zyngier wrote: > > On Wed, 11 Apr 2018 14:11:52 +0100, > Bockholdt Arne wrote: > > > Hi all, > > is there anything new, I've just tried the new stable 4.16.1 kernel > without any change. The Renesys USB3 controller is still not > functional. I'm willing to test any patch that is based on a stable > kernel version because the machine is in production use. > > > Have you tested the branch[1] I mentioned in my previous email? > Without your feedback, I cannot really make much progress on this. > > Thanks, > > M. > > [1] https://www.spinics.net/lists/linux-usb/msg167301.html > > > It seems the repo is based on 3.9 kernel and not on a current stable branch, > isn't it? It's rather old, I'm not sure my setup will work with this old > kernel. > Are you sure you pulled the correct branch? usb/uPD720202-reset has the following on top of *v4.16-rc6* Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue" xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers (https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=usb/uPD720202-reset) > Do you have a patch for a less dated kernel? > > Thank you, > > Arne -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] usb: gadget: udc: renesas_usb3: add property "renesas,ignore-id"
Hi Simon-san, > From: Simon Horman, Sent: Wednesday, April 11, 2018 4:28 PM > > On Tue, Apr 10, 2018 at 09:13:53PM +0900, Yoshihiro Shimoda wrote: > > This patch adds a new property to ignore the ID signal on a board. > > > > Signed-off-by: Yoshihiro Shimoda> > --- > > Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++ > > drivers/usb/gadget/udc/renesas_usb3.c | 10 ++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > index 2c071bb..53949bd 100644 > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > @@ -19,6 +19,8 @@ Required properties: > > Optional properties: > >- phys: phandle + phy specifier pair > >- phy-names: must be "usb" > > + - renesas,ignore-id: when a board doesn't use ID pin, you can add this > > + property to ignore the ID state. > > > > Example of R-Car H3 ES1.x: > > usb3_peri0: usb@ee02 { > > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c > > b/drivers/usb/gadget/udc/renesas_usb3.c > > index 409cde4..59e1485 100644 > > --- a/drivers/usb/gadget/udc/renesas_usb3.c > > +++ b/drivers/usb/gadget/udc/renesas_usb3.c > > @@ -350,6 +350,7 @@ struct renesas_usb3 { > > bool extcon_host; /* check id and set EXTCON_USB_HOST */ > > bool extcon_usb;/* check vbus and set EXTCON_USB */ > > bool forced_b_device; > > + bool ignore_id; > > }; > > > > #define gadget_to_renesas_usb3(_gadget)\ > > @@ -645,6 +646,9 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3) > > > > static void usb3_set_mode(struct renesas_usb3 *usb3, bool host) > > { > > + if (usb3->ignore_id && !usb3->forced_b_device) > > + return; > > + > > if (host) > > usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); > > else > > @@ -675,6 +679,9 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, > > bool host, bool a_dev) > > > > static bool usb3_is_a_device(struct renesas_usb3 *usb3) > > { > > + if (usb3->ignore_id) > > + return false; > > + > > return !(usb3_read(usb3, USB3_USB_OTG_STA) & USB_OTG_IDMON); > > } > > > > @@ -2632,6 +2639,9 @@ static int renesas_usb3_probe(struct platform_device > > *pdev) > > if (ret < 0) > > goto err_add_udc; > > > > + if (of_property_read_bool(pdev->dev.of_node, "renesas,no-id")) > > + usb3->ignore_id = true; > > I wonder if this is better expressed as: > > usb3->ignore_id = of_property_read_bool(pdev->dev.of_node, > "renesas,no-id")); Thank you for the pointed out! I agree with you. However, I would like to recall this adding property for now because I'm thinking that the OF graph of usb-connector can assume this via connector-type instead of such a local property. So, I'll try to use the usb-connector bindings on this driver. Best regards, Yoshihiro Shimoda > > + > > ret = device_create_file(>dev, _attr_role); > > if (ret < 0) > > goto err_dev_create; > > -- > > 1.9.1 > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT/PATCH 18/38] usb: dwc3: gadget: check for Missed Isoc from event status
Hi, On 4/11/2018 1:21 AM, Felipe Balbi wrote: > > Hi, > > Felipe Balbiwrites: Without XferNotReady, we won't have a reliable way to know the uFrame number. Read the Isochronous programming sequence from your databook. >>> >>> Right. We need XferNotReady to know when to start isoc transfer. But if >>> there are still queued requests, DWC3 can just wait to see if any of >>> them will succeed to continue with the transfer just as how DWC3 is >>> handling it now. >> >> That's not what the databook says though. And that's also not intention >> of how the code is written as of now either. The way the code is written >> is the following: >> >> queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() -> >> queue() -> end_transfer. >> >> That's not really waiting for the queue to be consumed, it's just >> delaying end transfer until we get another queue(). IOW, it just >> *happens* to give the controller time to go through the list of started >> requests. >> >>> If we end and restart the transfer right away, then we may lose more >>> isoc data than necessary (due to isoc scheduling at least 4 uFrame >>> ahead of time). Is there something you see that doesn't work with the >>> current implementation? >> >> Not _really_, I'm just trying to make the code easier to read and, I >> think, I've achieved that. Now, if we need to delay end transfer in the >> case where we have more requests in the controller's queue, that's easy >> enough to implement: >> >> @@ -2371,7 +2371,8 @@ static void >> dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, >> if (event->status & DEPEVT_STATUS_BUSERR) >> status = -ECONNRESET; >> >> -if (event->status & DEPEVT_STATUS_MISSED_ISOC) { >> +if (event->status & DEPEVT_STATUS_MISSED_ISOC && >> +list_empty(>started_list) { >> status = -EXDEV; Maybe we should return the -EXDEV status every time there's a missed isoc. >> stop = true; >> } >> >> I'm not sure this is a good idea though. Once we miss an interval, don't >> we need to know the next frame when transfer needs to be scheduled? >> >> Meaning we would need XferNotReady to properly schedule the new >> transfer? > > thinking about this a little more. This extra list_empty() check is not > wrong at all :-) I've amended this series with the 3 patches below. I'll > resend the series once I've given more time for people to test. Patches > have been updated to the branch on kernel.org as well, btw. Great! :) Thanks for all the new updates. I'll test it out when I have a chance. Thinh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] usbip: usbip_host: run rebind from exit when module is removed
After removing usbip_host module, devices it releases are left without a driver. For example, when a keyboard or a mass storage device are bound to usbip_host when it is removed, these devices are no longer bound to any driver. Fix it to run device_attach() from the module exit routine to restore the devices to their original drivers. This includes cleanup changes and moving device_attach() code to a common routine to be called from rebind_store() and usbip_host_exit(). Signed-off-by: Shuah Khan--- drivers/usb/usbip/stub_dev.c | 6 + drivers/usb/usbip/stub_main.c | 60 --- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c index 7813c1862941..9d0425113c4b 100644 --- a/drivers/usb/usbip/stub_dev.c +++ b/drivers/usb/usbip/stub_dev.c @@ -448,12 +448,8 @@ static void stub_disconnect(struct usb_device *udev) busid_priv->sdev = NULL; stub_device_free(sdev); - if (busid_priv->status == STUB_BUSID_ALLOC) { + if (busid_priv->status == STUB_BUSID_ALLOC) busid_priv->status = STUB_BUSID_ADDED; - } else { - busid_priv->status = STUB_BUSID_OTHER; - del_match_busid((char *)udev_busid); - } } #ifdef CONFIG_PM diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c index fb46bd62d538..587b9bc10042 100644 --- a/drivers/usb/usbip/stub_main.c +++ b/drivers/usb/usbip/stub_main.c @@ -14,6 +14,7 @@ #define DRIVER_DESC "USB/IP Host Driver" struct kmem_cache *stub_priv_cache; + /* * busid_tables defines matching busids that usbip can grab. A user can change * dynamically what device is locally used and what device is exported to a @@ -169,6 +170,51 @@ static ssize_t match_busid_store(struct device_driver *dev, const char *buf, } static DRIVER_ATTR_RW(match_busid); +static int do_rebind(char *busid, struct bus_id_priv *busid_priv) +{ + int ret; + + /* device_attach() callers should hold parent lock for USB */ + if (busid_priv->udev->dev.parent) + device_lock(busid_priv->udev->dev.parent); + ret = device_attach(_priv->udev->dev); + if (busid_priv->udev->dev.parent) + device_unlock(busid_priv->udev->dev.parent); + if (ret < 0) { + dev_err(_priv->udev->dev, "rebind failed\n"); + return ret; + } + return 0; +} + +static void stub_device_rebind(void) +{ +#if IS_MODULE(CONFIG_USBIP_HOST) + struct bus_id_priv *busid_priv; + int i; + + /* update status to STUB_BUSID_OTHER so probe ignores the device */ + spin_lock(_table_lock); + for (i = 0; i < MAX_BUSID; i++) { + if (busid_table[i].name[0] && + busid_table[i].shutdown_busid) { + busid_priv = &(busid_table[i]); + busid_priv->status = STUB_BUSID_OTHER; + } + } + spin_unlock(_table_lock); + + /* now run rebind */ + for (i = 0; i < MAX_BUSID; i++) { + if (busid_table[i].name[0] && + busid_table[i].shutdown_busid) { + busid_priv = &(busid_table[i]); + do_rebind(busid_table[i].name, busid_priv); + } + } +#endif +} + static ssize_t rebind_store(struct device_driver *dev, const char *buf, size_t count) { @@ -189,16 +235,9 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf, /* mark the device for deletion so probe ignores it during rescan */ bid->status = STUB_BUSID_OTHER; - /* device_attach() callers should hold parent lock for USB */ - if (bid->udev->dev.parent) - device_lock(bid->udev->dev.parent); - ret = device_attach(>udev->dev); - if (bid->udev->dev.parent) - device_unlock(bid->udev->dev.parent); - if (ret < 0) { - dev_err(>udev->dev, "rebind failed\n"); + ret = do_rebind((char *) buf, bid); + if (ret < 0) return ret; - } /* delete device from busid_table */ del_match_busid((char *) buf); @@ -323,6 +362,9 @@ static void __exit usbip_host_exit(void) */ usb_deregister_device_driver(_driver); + /* initiate scan to attach devices */ + stub_device_rebind(); + kmem_cache_destroy(stub_priv_cache); } -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usbip: usbip_host: refine probe and disconnect debug msgs to be useful
Refine probe and disconnect debug msgs to be useful and say what is in progress. Signed-off-by: Shuah Khan--- drivers/usb/usbip/stub_dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c index dd8ef36ab10e..7813c1862941 100644 --- a/drivers/usb/usbip/stub_dev.c +++ b/drivers/usb/usbip/stub_dev.c @@ -302,7 +302,7 @@ static int stub_probe(struct usb_device *udev) struct bus_id_priv *busid_priv; int rc; - dev_dbg(>dev, "Enter\n"); + dev_dbg(>dev, "Enter probe\n"); /* check we should claim or not by busid_table */ busid_priv = get_busid_priv(udev_busid); @@ -404,7 +404,7 @@ static void stub_disconnect(struct usb_device *udev) struct bus_id_priv *busid_priv; int rc; - dev_dbg(>dev, "Enter\n"); + dev_dbg(>dev, "Enter disconnect\n"); busid_priv = get_busid_priv(udev_busid); if (!busid_priv) { -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] usbip: usbip_host: delete device from busid_table after rebind
Device is left in the busid_table after unbind and rebind. Rebind initiates usb bus scan and the original driver claims the device. After rescan the device should be deleted from the busid_table as it no longer belongs to usbip_host. Fix it to delete the device after device_attach() succeeds. Signed-off-by: Shuah Khan--- drivers/usb/usbip/stub_main.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c index d41d0cdeec0f..fb46bd62d538 100644 --- a/drivers/usb/usbip/stub_main.c +++ b/drivers/usb/usbip/stub_main.c @@ -186,6 +186,9 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf, if (!bid) return -ENODEV; + /* mark the device for deletion so probe ignores it during rescan */ + bid->status = STUB_BUSID_OTHER; + /* device_attach() callers should hold parent lock for USB */ if (bid->udev->dev.parent) device_lock(bid->udev->dev.parent); @@ -197,6 +200,9 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf, return ret; } + /* delete device from busid_table */ + del_match_busid((char *) buf); + return count; } -- 2.14.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lan78xx: Don't reset the interface on open
From: Phil ElwellDate: Tue, 10 Apr 2018 13:18:25 +0100 > Commit 92571a1aae40 ("lan78xx: Connect phy early") moves the PHY > initialisation into lan78xx_probe, but lan78xx_open subsequently calls > lan78xx_reset. As well as forcing a second round of link negotiation, > this reset frequently prevents the phy interrupt from being generated > (even though the link is up), rendering the interface unusable. > > Fix this issue by removing the lan78xx_reset call from lan78xx_open. > > Fixes: 92571a1aae40 ("lan78xx: Connect phy early") > Signed-off-by: Phil Elwell Applied and queued up for -stable, thanks. -- 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] cdc_ether: flag the Cinterion AHS8 modem by gemalto as WWAN
From: Oliver NeukumDate: Wed, 11 Apr 2018 13:25:38 +0200 > Am Mittwoch, den 11.04.2018, 13:15 +0200 schrieb Bassem Boubaker: >> The Cinterion AHS8 is a 3G device with one embedded WWAN interface >> using cdc_ether as a driver. >> >> The modem is controlled via AT commands through the exposed TTYs. >> >> AT+CGDCONT write command can be used to activate or deactivate a WWAN >> connection for a PDP context defined with the same command. UE supports >> one WWAN adapter. >> >> Signed-off-by: Bassem Boubaker > Acked-by: Oliver Neukum Applied and queued up for -stable. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] lan78xx: Don't reset the interface on open
Hi Phil, > Hi Nisar, > > On 10/04/2018 15:16, nisar.sa...@microchip.com wrote: > > Thanks Phil, for identifying the issues. > > > >> - ret = lan78xx_reset(dev); > >> - if (ret < 0) > >> - goto done; > >> - > >>phy_start(net->phydev); > >> > >>netif_dbg(dev, ifup, dev->net, "phy initialised successfully"); > >> -- > > > > You may need to start the interrupts before "phy_start" instead of > suppressing call to "lan78xx_reset". > > > > + if (dev->domain_data.phyirq > 0) > > + phy_start_interrupts(dev->net->phydev); > > Shouldn't phy_connect_direct, called from lan78xx_phy_init, already have > enabled interrupts for us? > > This patch addresses two problems - time wasted by renegotiating the link > after the reset and the missed interrupt - and I'd like both to be fixed. > Unless > you can come up with a good reason for performing the reset from the open > handler I think it should be removed. > > Phil Thanks, we have verified suspected test cases and these are passed, the changes are good to go. - Nisar
Re: [PATCH] lan78xx: Avoid spurious kevent 4 "error"
From: Phil ElwellDate: Wed, 11 Apr 2018 12:02:47 +0100 > lan78xx_defer_event generates an error message whenever the work item > is already scheduled. lan78xx_open defers three events - > EVENT_STAT_UPDATE, EVENT_DEV_OPEN and EVENT_LINK_RESET. Being aware > of the likelihood (or certainty) of an error message, the DEV_OPEN > event is added to the set of pending events directly, relying on > the subsequent deferral of the EVENT_LINK_RESET call to schedule the > work. Take the same precaution with EVENT_STAT_UPDATE to avoid a > totally unnecessary error message. > > Signed-off-by: Phil Elwell Applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lan78xx: Correctly indicate invalid OTP
From: Phil ElwellDate: Wed, 11 Apr 2018 10:59:17 +0100 > lan78xx_read_otp tries to return -EINVAL in the event of invalid OTP > content, but the value gets overwritten before it is returned and the > read goes ahead anyway. Make the read conditional as it should be > and preserve the error code. > > Signed-off-by: Phil Elwell Applied with appropriate Fixes: tag added, and queud up for -stable. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: ftdi_sio: Use jtag quirk for Arrow USB Blaster
Arrow USB Blaster integrated on MAX1000 board uses the same vendor ID (0x0403) and product ID (0x6010) as the "original" FTDI device. This patch avoids picking up by ftdi_sio of the first interface of this USB device. After that this device can be used by Arrow user-space JTAG driver. Signed-off-by: Vasyl Vavrychuk--- drivers/usb/serial/ftdi_sio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index 87202ad5a50d..3bab6f83f6de 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -1899,7 +1899,8 @@ static int ftdi_8u2232c_probe(struct usb_serial *serial) if (udev->product && (!strcmp(udev->product, "BeagleBone/XDS100V2") || -!strcmp(udev->product, "SNAP Connect E10"))) +!strcmp(udev->product, "SNAP Connect E10") || +!strcmp(udev->product, "Arrow USB Blaster"))) return ftdi_jtag_probe(serial); return 0; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality
On Wed, 11 Apr 2018 14:11:52 +0100, Bockholdt Arne wrote: > > Hi all, > > is there anything new, I've just tried the new stable 4.16.1 kernel > without any change. The Renesys USB3 controller is still not > functional. I'm willing to test any patch that is based on a stable > kernel version because the machine is in production use. Have you tested the branch[1] I mentioned in my previous email? Without your feedback, I cannot really make much progress on this. Thanks, M. [1] https://www.spinics.net/lists/linux-usb/msg167301.html -- Jazz is not dead, it just smell funny. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
usb: storage: uas: want to print the all IN ACK(Data-in)
Hi, Talking about UASP Rev 1.0.pdf (http://www.usb.org/developers/docs/devclass_docs/) "5.4.3 Data-in transfer" As part of my debug..want to understand the flow. suspecting IN ACK(Data-in) is missing as per the lecroy log. And same I want to check/print. Is there any way to do so? I am receiving ERDY(Data-in) from device but not IN ACK(data-in), I could see on leCroy. -- Best Regards, Tushar R Nimkar QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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] lan78xx: Correctly indicate invalid OTP
Hi Andrew. On 11/04/2018 13:57, Andrew Lunn wrote: > On Wed, Apr 11, 2018 at 10:59:17AM +0100, Phil Elwell wrote: >> lan78xx_read_otp tries to return -EINVAL in the event of invalid OTP >> content, but the value gets overwritten before it is returned and the >> read goes ahead anyway. Make the read conditional as it should be >> and preserve the error code. > > Hi Phil > > Do you know that the Fixes: tag should be for this? When did it break? It's been broken since day 1, so: Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver") -- 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] lan78xx: Correctly indicate invalid OTP
On Wed, Apr 11, 2018 at 10:59:17AM +0100, Phil Elwell wrote: > lan78xx_read_otp tries to return -EINVAL in the event of invalid OTP > content, but the value gets overwritten before it is returned and the > read goes ahead anyway. Make the read conditional as it should be > and preserve the error code. Hi Phil Do you know that the Fixes: tag should be for this? When did it break? Thanks Andrew -- 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] cdc_ether: flag the Cinterion AHS8 modem by gemalto as WWAN
Am Mittwoch, den 11.04.2018, 13:15 +0200 schrieb Bassem Boubaker: > The Cinterion AHS8 is a 3G device with one embedded WWAN interface > using cdc_ether as a driver. > > The modem is controlled via AT commands through the exposed TTYs. > > AT+CGDCONT write command can be used to activate or deactivate a WWAN > connection for a PDP context defined with the same command. UE supports > one WWAN adapter. > > Signed-off-by: Bassem BoubakerAcked-by: Oliver Neukum -- 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: Lenovo ThinkVision X1 27" USB-C occasionally hangs the kernel (usbhid) and isn't setting a proper resolution when USB 3.0 (FHD) switched to USB 2.0 (UHD)
On Wed, Apr 11, 2018 at 02:09:29PM +0300, Heikki Krogerus wrote: > On Wed, Apr 11, 2018 at 08:28:44AM +, andrey.ara...@nixaid.com wrote: > > Thank you for the insights, Heikki! > > > > Please find the acpi.dump.tgz file is a attached. > > > > I do not have the USBC* and INT3515* devices, > > OK. That means we don't have any way to interface with the USB Type-C > ports directly unfortunately. The ports are quite simply not visible > to us. We can't do anything from USB side. > > The problem is in any case a graphics problem, so maybe it's better to > let those guys take a look at this. I think this MacBook Pro has AMD > GPU, so adding AMD driver maintainers and lists. Sorry Andrey, the previous mail did not get to you. I managed to change your email address somehow. :-) > > so I have attached the following file as well: > > > > # ls -1 /sys/bus/acpi/devices/*/status | while read dev; do echo $dev: > > $(cat $dev); done | gzip -c > /tmp/all-device-status.log.gz > > > > > > KR, > > Andrey Arapov > > > > April 10, 2018 4:35 PM, "Heikki Krogerus"> > wrote: > > > > > On Tue, Apr 10, 2018 at 09:05:07AM +, andrey.ara...@nixaid.com wrote: > > > > > >> Dear Linux Kernel Devs, > > >> > > >> I have recently got a Lenovo ThinkVision X1 27" monitor, it is connected > > >> to my > > >> laptop over a USB-C cable (DisplayPort). > > >> > > >> This monitor has a built-in USB hub with a toggle button, when pressed it > > >> shows two options on the screen: > > >> > > >> Press USB Switch to toggle between: > > >> A) 3840x2160 UHD USB 2.0 > > >> B) 1920x1080 FHD USB 3.0 > > >> > > >> By default it is always set to FHD, which is why Linux sees the > > >> 1920x1080 as a > > >> maximum possible resolution. > > >> > > >> Whenever I am changing it to the UHD, Linux is not changing the > > >> resolution to > > >> 3840x2160, instead it sets to somewhere around 2560x. > > >> > > >> To work this around, I am running the following commands manually: > > >> > > >> xrandr --newmode "3840x2160_60.00" 533.25 3840 3888 3920 4000 2160 2163 > > >> 2168 +hsync -vsync > > >> xrandr --addmode DisplayPort-2 3840x2160_60.00 > > >> xrandr --output DisplayPort-2 --mode 3840x2160_60.00 > > >> > > >> Then, when I was trying to switch it back to FHD and again back to UHD, > > >> sometimes it causes a kernel panic. The panic also happened when I was > > >> plugging the cable in/out and back again. What I was able to spot is > > >> that the > > >> last unloaded kernel module was usbhid. > > >> > > >> Please advise, what can I try to help investigating this issue further? > > >> > > >> I have attached the output from "dmesg -T" command as "4.16.1-dmesg.txt" > > >> file. > > >> The logs are related to when the monitor was connected via a USB-C cable > > >> to a > > >> DisplayPort-2, the default resolution picked was FHD (USB 3.0) and then > > >> I have > > >> pressed the toggle button to use UHD (USB 2.0), then have applied the > > >> "xrandr" > > >> commands and have closed the lid of my laptop so I would be using my > > >> monitor > > >> as a primary display. > > > > > > The board seems to support Thunderbold, however, in this case my guess is > > > that > > > the monitor is actually using just the DP alternate mode (Thundebolt can > > > pipe > > > DisplayPort protocol). > > > > > > I'm guessing the problem is related to the DisplayPort lane counts. With > > > the > > > highest resolutions you need all four lanes, but if you want to use USB3 > > > with > > > USB Type-C connectors at the same time, two have to be allocated for USB3 > > > leaving only two for DisplayPort allowing lower resolution. The problem > > > is that > > > the GPU drivers need to know how many lanes the DisplayPort has in use. > > > That > > > information will in normal case come from USB Type-C drivers. > > > Unfortunately we > > > do not have support for that in Linux kernel yet, but I have made a > > > proposal for > > > a solution. > > > > > > Let's start by checking details of your board. Can you send us acpidump > > > output? > > > > > > % acpidump -o > > > > > > Also, please check the status of these acpi devices: > > > > > > % cat /sys/bus/acpi/devices/USBC*/status > > > % cat /sys/bus/acpi/devices/INT3515*/status Cheers, -- 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] cdc_ether: flag the Cinterion AHS8 modem by gemalto as WWAN
The Cinterion AHS8 is a 3G device with one embedded WWAN interface using cdc_ether as a driver. The modem is controlled via AT commands through the exposed TTYs. AT+CGDCONT write command can be used to activate or deactivate a WWAN connection for a PDP context defined with the same command. UE supports one WWAN adapter. Signed-off-by: Bassem Boubaker--- drivers/net/usb/cdc_ether.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c index fff4b13..5c42cf8 100644 --- a/drivers/net/usb/cdc_ether.c +++ b/drivers/net/usb/cdc_ether.c @@ -902,6 +902,12 @@ static const struct usb_device_id products[] = { USB_CDC_PROTO_NONE), .driver_info = (unsigned long)_info, }, { + /* Cinterion AHS3 modem by GEMALTO */ + USB_DEVICE_AND_INTERFACE_INFO(0x1e2d, 0x0055, USB_CLASS_COMM, + USB_CDC_SUBCLASS_ETHERNET, + USB_CDC_PROTO_NONE), + .driver_info = (unsigned long)_info, +}, { USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE), .driver_info = (unsigned long) _info, -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] lan78xx: Avoid spurious kevent 4 "error"
lan78xx_defer_event generates an error message whenever the work item is already scheduled. lan78xx_open defers three events - EVENT_STAT_UPDATE, EVENT_DEV_OPEN and EVENT_LINK_RESET. Being aware of the likelihood (or certainty) of an error message, the DEV_OPEN event is added to the set of pending events directly, relying on the subsequent deferral of the EVENT_LINK_RESET call to schedule the work. Take the same precaution with EVENT_STAT_UPDATE to avoid a totally unnecessary error message. Signed-off-by: Phil Elwell--- drivers/net/usb/lan78xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 32cf217..3102374 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2507,7 +2507,7 @@ static void lan78xx_init_stats(struct lan78xx_net *dev) dev->stats.rollover_max.eee_tx_lpi_transitions = 0x; dev->stats.rollover_max.eee_tx_lpi_time = 0x; - lan78xx_defer_kevent(dev, EVENT_STAT_UPDATE); + set_bit(EVENT_STAT_UPDATE, >flags); } static int lan78xx_open(struct net_device *net) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] lan78xx: Correctly indicate invalid OTP
lan78xx_read_otp tries to return -EINVAL in the event of invalid OTP content, but the value gets overwritten before it is returned and the read goes ahead anyway. Make the read conditional as it should be and preserve the error code. Signed-off-by: Phil Elwell--- drivers/net/usb/lan78xx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 55a78eb..32cf217 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -928,7 +928,8 @@ static int lan78xx_read_otp(struct lan78xx_net *dev, u32 offset, offset += 0x100; else ret = -EINVAL; - ret = lan78xx_read_raw_otp(dev, offset, length, data); + if (!ret) + ret = lan78xx_read_raw_otp(dev, offset, length, data); } return ret; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs
Hi Heikki, Thank you for the comments! > From: Heikki Krogerus, Sent: Wednesday, April 11, 2018 5:02 PM > > On Wed, Apr 11, 2018 at 03:15:23AM +, Yoshihiro Shimoda wrote: > > > > + host_node = of_parse_phandle(pdev->dev.of_node, "renesas,host", > > > > 0); > > > > + if (!host_node) > > > > + return -ENODEV; > > > > + > > > > + pdev_host = of_find_device_by_node(host_node); > > > > + if (!pdev_host) > > > > + return -ENODEV; > > > > + > > > > + of_node_put(host_node); > > > > > > Isn't what Heikki tried to solve with graphs and stuff like that? > > > > Heikki prepared "device connection" framework (devcon) [1] to get a device > > pointer. > > However, IIUC, we need to improve the framework for device tree environment > > because: > > - The devcon needs each dev_name() through the endpoint array of struct > > device_connection. > > - Each dev_name() on device tree environment will be > register>., > >f.e. "ee02.usb". So, I don???t think adding such strings to a driver > > is a good way. > > That is how the build-in connection descriptions are handled, but in > this case you want to describe them in your bindings, and to do that you > should use remote-endpoints, ie. OF graph bindings: bindings/graph.txt I understood it. As I mentioned other thread, I'll try to use the usb-connector bindings. > > So, I wrote such a code by using existing APIs. > > Should I improve the framework first somehow? Or, is my understanding > > incorrect? > > You don't necessarily need to propose any changes to the device > connection framework at this point, but you do need to use graph for > describing that connection. Once we add device graph handling to the > device connection framework, it is no problem to update this driver. I got it. > Of course, if you have time and interest in preparing a proposal for > graph handling to the device connection framework, I would appreciate > it. I'll try to add graph handling to the device connection framework by end of next week. If failed, I'll inform in this thread. Best regards, Yoshihiro Shimoda > > 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: [PATCH] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs
Hi Heikki, > From: Heikki Krogerus, Sent: Wednesday, April 11, 2018 4:26 PM > > On Tue, Apr 10, 2018 at 09:03:46PM +0900, Yoshihiro Shimoda wrote: > > +Example of R-Car H3 ES2.0: > > + usb3_peri0: usb@ee02 { > > + compatible = "renesas,r8a7795-usb3-peri", > > +"renesas,rcar-gen3-usb3-peri"; > > + reg = <0 0xee02 0 0x400>; > > + interrupts = ; > > + clocks = < CPG_MOD 328>; > > + power-domains = < R8A7795_PD_ALWAYS_ON>; > > + resets = < 328>; > > + > > + usb3-role-sw { > > + compatible = "renesas,rcar-usb3-role-switch"; > > + renesas,host = <>; > > I pretty sure you should model that connection using OF graph > bindings. That way I believe this will also fit nicely to the > usb-connector bindings. Check bindings/connector/usb-connector.txt Oh, I didn't know the usb-connector bindings has been merged into the linux-next. Thank you for the suggestion. I will try to use it. Best regards, Yoshihiro Shimoda > > + }; > > + }; > > > Br, > > -- > 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: [RFT/PATCH 18/38] usb: dwc3: gadget: check for Missed Isoc from event status
Hi, Felipe Balbiwrites: >>> Without XferNotReady, we won't have a reliable way to know the uFrame >>> number. Read the Isochronous programming sequence from your databook. >> >> Right. We need XferNotReady to know when to start isoc transfer. But if >> there are still queued requests, DWC3 can just wait to see if any of >> them will succeed to continue with the transfer just as how DWC3 is >> handling it now. > > That's not what the databook says though. And that's also not intention > of how the code is written as of now either. The way the code is written > is the following: > > queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() -> > queue() -> end_transfer. > > That's not really waiting for the queue to be consumed, it's just > delaying end transfer until we get another queue(). IOW, it just > *happens* to give the controller time to go through the list of started > requests. > >> If we end and restart the transfer right away, then we may lose more >> isoc data than necessary (due to isoc scheduling at least 4 uFrame >> ahead of time). Is there something you see that doesn't work with the >> current implementation? > > Not _really_, I'm just trying to make the code easier to read and, I > think, I've achieved that. Now, if we need to delay end transfer in the > case where we have more requests in the controller's queue, that's easy > enough to implement: > > @@ -2371,7 +2371,8 @@ static void > dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, > if (event->status & DEPEVT_STATUS_BUSERR) > status = -ECONNRESET; > > - if (event->status & DEPEVT_STATUS_MISSED_ISOC) { > + if (event->status & DEPEVT_STATUS_MISSED_ISOC && > + list_empty(>started_list) { > status = -EXDEV; > stop = true; > } > > I'm not sure this is a good idea though. Once we miss an interval, don't > we need to know the next frame when transfer needs to be scheduled? > > Meaning we would need XferNotReady to properly schedule the new > transfer? thinking about this a little more. This extra list_empty() check is not wrong at all :-) I've amended this series with the 3 patches below. I'll resend the series once I've given more time for people to test. Patches have been updated to the branch on kernel.org as well, btw. 8< From 811476d1799c606b3af2b40022fe333cef586387 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Wed, 11 Apr 2018 10:31:53 +0300 Subject: [PATCH 1/3] usb: dwc3: debug: decode uFrame from event too XferNotReady and XferInProgress give us the uFrame number we're currently in. Printing that out on tracepoints may help us find bugs in transfer scheduling. Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/debug.h | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h index 0be6a554be57..c66d216dcc30 100644 --- a/drivers/usb/dwc3/debug.h +++ b/drivers/usb/dwc3/debug.h @@ -493,14 +493,18 @@ dwc3_ep_event_string(char *str, const struct dwc3_event_depevt *event, case DWC3_DEPEVT_XFERINPROGRESS: len = strlen(str); - sprintf(str + len, "Transfer In Progress (%c%c%c)", + sprintf(str + len, "Transfer In Progress [%d] (%c%c%c)", + event->parameters, status & DEPEVT_STATUS_SHORT ? 'S' : 's', status & DEPEVT_STATUS_IOC ? 'I' : 'i', status & DEPEVT_STATUS_LST ? 'M' : 'm'); break; case DWC3_DEPEVT_XFERNOTREADY: - strcat(str, "Transfer Not Ready"); - strcat(str, status & DEPEVT_STATUS_TRANSFER_ACTIVE ? + len = strlen(str); + + sprintf(str + len, "Transfer Not Ready [%d]%s", + event->parameters, + status & DEPEVT_STATUS_TRANSFER_ACTIVE ? " (Active)" : " (Not Active)"); /* Control Endpoints */ -- 2.16.1 8< From 77f9d84d785c2d088e82c90b7d3ad844ce59d668 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Wed, 11 Apr 2018 10:32:52 +0300 Subject: [PATCH 2/3] usb: dwc3: gadget: don't issue End Transfer if we have started reqs In case we have many started requests and one of them in the middle is completed with Missed Isoc, let's not End Transfer as that would result in us loosing (possibly) many more intervals. Instead, let's allow the controller to go through its list of started requests. Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/gadget.c | 3 ++- 1 file changed, 2 insertions(+),
Re: [PATCH] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs
On Wed, Apr 11, 2018 at 03:15:23AM +, Yoshihiro Shimoda wrote: > > > + host_node = of_parse_phandle(pdev->dev.of_node, "renesas,host", > > > 0); > > > + if (!host_node) > > > + return -ENODEV; > > > + > > > + pdev_host = of_find_device_by_node(host_node); > > > + if (!pdev_host) > > > + return -ENODEV; > > > + > > > + of_node_put(host_node); > > > > Isn't what Heikki tried to solve with graphs and stuff like that? > > Heikki prepared "device connection" framework (devcon) [1] to get a device > pointer. > However, IIUC, we need to improve the framework for device tree environment > because: > - The devcon needs each dev_name() through the endpoint array of struct > device_connection. > - Each dev_name() on device tree environment will be register>., >f.e. "ee02.usb". So, I don???t think adding such strings to a driver > is a good way. That is how the build-in connection descriptions are handled, but in this case you want to describe them in your bindings, and to do that you should use remote-endpoints, ie. OF graph bindings: bindings/graph.txt > So, I wrote such a code by using existing APIs. > Should I improve the framework first somehow? Or, is my understanding > incorrect? You don't necessarily need to propose any changes to the device connection framework at this point, but you do need to use graph for describing that connection. Once we add device graph handling to the device connection framework, it is no problem to update this driver. Of course, if you have time and interest in preparing a proposal for graph handling to the device connection framework, I would appreciate 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
Re: [PATCH 1/2] usb: gadget: udc: renesas_usb3: add property "renesas,ignore-id"
On Tue, Apr 10, 2018 at 09:13:53PM +0900, Yoshihiro Shimoda wrote: > This patch adds a new property to ignore the ID signal on a board. > > Signed-off-by: Yoshihiro Shimoda> --- > Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++ > drivers/usb/gadget/udc/renesas_usb3.c | 10 ++ > 2 files changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > index 2c071bb..53949bd 100644 > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > @@ -19,6 +19,8 @@ Required properties: > Optional properties: >- phys: phandle + phy specifier pair >- phy-names: must be "usb" > + - renesas,ignore-id: when a board doesn't use ID pin, you can add this > +property to ignore the ID state. > > Example of R-Car H3 ES1.x: > usb3_peri0: usb@ee02 { > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c > b/drivers/usb/gadget/udc/renesas_usb3.c > index 409cde4..59e1485 100644 > --- a/drivers/usb/gadget/udc/renesas_usb3.c > +++ b/drivers/usb/gadget/udc/renesas_usb3.c > @@ -350,6 +350,7 @@ struct renesas_usb3 { > bool extcon_host; /* check id and set EXTCON_USB_HOST */ > bool extcon_usb;/* check vbus and set EXTCON_USB */ > bool forced_b_device; > + bool ignore_id; > }; > > #define gadget_to_renesas_usb3(_gadget) \ > @@ -645,6 +646,9 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3) > > static void usb3_set_mode(struct renesas_usb3 *usb3, bool host) > { > + if (usb3->ignore_id && !usb3->forced_b_device) > + return; > + > if (host) > usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); > else > @@ -675,6 +679,9 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, > bool host, bool a_dev) > > static bool usb3_is_a_device(struct renesas_usb3 *usb3) > { > + if (usb3->ignore_id) > + return false; > + > return !(usb3_read(usb3, USB3_USB_OTG_STA) & USB_OTG_IDMON); > } > > @@ -2632,6 +2639,9 @@ static int renesas_usb3_probe(struct platform_device > *pdev) > if (ret < 0) > goto err_add_udc; > > + if (of_property_read_bool(pdev->dev.of_node, "renesas,no-id")) > + usb3->ignore_id = true; I wonder if this is better expressed as: usb3->ignore_id = of_property_read_bool(pdev->dev.of_node, "renesas,no-id")); > + > ret = device_create_file(>dev, _attr_role); > if (ret < 0) > goto err_dev_create; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs
On Tue, Apr 10, 2018 at 09:03:46PM +0900, Yoshihiro Shimoda wrote: > This patch adds role switch support for R-Car SoCs. Some R-Car SoCs > (e.g. R-Car H3) have USB 3.0 dual-role device controller which has > the USB 3.0 xHCI host and Renesas USB 3.0 peripheral. > > Unfortunately, the mode change register contains the USB 3.0 peripheral > controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) > manages this register. However, in peripheral mode, the host should > stop. Also the host hardware needs to reinitialize its own registers > when the mode changes from peripheral to host mode. Otherwise, > the host cannot work correctly (e.g. detect a device as high-speed). > > To achieve this by a driver, this role switch driver manages > the mode change register and attach/release the xhci-plat driver. > The renesas_usb3 udc driver should call devm_of_platform_populate() > to probe this driver. > > Signed-off-by: Yoshihiro Shimoda> --- > .../bindings/usb/renesas,rcar-usb3-role-sw.txt | 23 +++ > drivers/usb/roles/Kconfig | 12 ++ > drivers/usb/roles/Makefile | 1 + > drivers/usb/roles/rcar-usb3-role-switch.c | 200 > + > 4 files changed, 236 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt > create mode 100644 drivers/usb/roles/rcar-usb3-role-switch.c > > diff --git > a/Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt > b/Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt > new file mode 100644 > index 000..752bc16 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt > @@ -0,0 +1,23 @@ > +Renesas Electronics R-Car USB 3.0 role switch driver > + > +A renesas_usb3's node can contain this node. > + > +Required properties: > + - compatible: Must contain "renesas,rcar-usb3-role-switch". > + - renesas,host: phandle of the usb3.0 host. > + > +Example of R-Car H3 ES2.0: > + usb3_peri0: usb@ee02 { > + compatible = "renesas,r8a7795-usb3-peri", > + "renesas,rcar-gen3-usb3-peri"; > + reg = <0 0xee02 0 0x400>; > + interrupts = ; > + clocks = < CPG_MOD 328>; > + power-domains = < R8A7795_PD_ALWAYS_ON>; > + resets = < 328>; > + > + usb3-role-sw { > + compatible = "renesas,rcar-usb3-role-switch"; > + renesas,host = <>; I pretty sure you should model that connection using OF graph bindings. That way I believe this will also fit nicely to the usb-connector bindings. Check bindings/connector/usb-connector.txt > + }; > + }; Br, -- 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: [RFT/PATCH 18/38] usb: dwc3: gadget: check for Missed Isoc from event status
Hi, Thinh Nguyenwrites: >>> On 4/9/2018 4:28 AM, Felipe Balbi wrote: In case we get an event with status set to Missed Isoc, this means we have missed an isochronous interval and should issue End Transfer command and wait for the following XferNotReady. >>> >>> Why does DWC3 need to issue End Transfer if there are still queued requests? >> >> Without XferNotReady, we won't have a reliable way to know the uFrame >> number. Read the Isochronous programming sequence from your databook. > > Right. We need XferNotReady to know when to start isoc transfer. But if > there are still queued requests, DWC3 can just wait to see if any of > them will succeed to continue with the transfer just as how DWC3 is > handling it now. That's not what the databook says though. And that's also not intention of how the code is written as of now either. The way the code is written is the following: queue() -> XferNotReady -> start_isoc() -> if (missed) do_nothing() -> queue() -> end_transfer. That's not really waiting for the queue to be consumed, it's just delaying end transfer until we get another queue(). IOW, it just *happens* to give the controller time to go through the list of started requests. > If we end and restart the transfer right away, then we may lose more > isoc data than necessary (due to isoc scheduling at least 4 uFrame > ahead of time). Is there something you see that doesn't work with the > current implementation? Not _really_, I'm just trying to make the code easier to read and, I think, I've achieved that. Now, if we need to delay end transfer in the case where we have more requests in the controller's queue, that's easy enough to implement: @@ -2371,7 +2371,8 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_BUSERR) status = -ECONNRESET; - if (event->status & DEPEVT_STATUS_MISSED_ISOC) { + if (event->status & DEPEVT_STATUS_MISSED_ISOC && + list_empty(>started_list) { status = -EXDEV; stop = true; } I'm not sure this is a good idea though. Once we miss an interval, don't we need to know the next frame when transfer needs to be scheduled? Meaning we would need XferNotReady to properly schedule the new transfer? -- balbi signature.asc Description: PGP signature
Re: [PATCH 4/4] usb: gadget: udc: renesas_usb3: should call devm_phy_get() before add udc
On Tue, Apr 10, 2018 at 09:44:31AM +, Yoshihiro Shimoda wrote: > Hi Simon-san, > > > From: Simon Horman, Sent: Tuesday, April 10, 2018 6:34 PM > > > > On Tue, Apr 10, 2018 at 01:28:33AM +, Yoshihiro Shimoda wrote: > > > Hi Simon-san, > > > > > > > From: Simon Horman, Sent: Monday, April 9, 2018 8:58 PM > > > > > > > > On Mon, Apr 02, 2018 at 09:21:34PM +0900, Yoshihiro Shimoda wrote: > > > > > This patch fixes an issue that this driver cannot call phy_init() > > > > > if a gadget driver is alreadly loaded because usb_add_gadget_udc() > > > > > might call renesas_usb3_start() via .udc_start. > > > > > > > > > > Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for > > > > > generic phy") > > > > > Cc:# v4.15+ > > > > > Signed-off-by: Yoshihiro Shimoda > > > > > --- > > > > > drivers/usb/gadget/udc/renesas_usb3.c | 16 > > > > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > > > > > Reviewed-by: Simon Horman > > > > > > > > > > > > Please see some suggestions for follow-up lower-priority patches below. > > > > > > Thank you for the review and suggestions! > > > > > > > > > > > > > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c > > > > > b/drivers/usb/gadget/udc/renesas_usb3.c > > > > > index 738b734..671bac3 100644 > > > > > --- a/drivers/usb/gadget/udc/renesas_usb3.c > > > > > +++ b/drivers/usb/gadget/udc/renesas_usb3.c > > > > > @@ -2632,6 +2632,14 @@ static int renesas_usb3_probe(struct > > > > > platform_device *pdev) > > > > > if (ret < 0) > > > > > goto err_alloc_prd; > > > > > > > > > > + /* > > > > > + * This is an optional. So, if this driver cannot get a phy, > > > > > + * this driver will not handle a phy anymore. > > > > > + */ > > > > > > > > nit: s/an optional/optional/ > > > > > > Oops. I will fix and submit v2 patches. > > > > > > > > + usb3->phy = devm_phy_get(>dev, "usb"); > > > > > + if (IS_ERR(usb3->phy)) > > > > > + usb3->phy = NULL; > > > > > > > > I think this will unintentionally ignore errors other than the > > > > non-existence of the device, f.e. a memory allocation failure > > > > in devm_phy_get(). > > > > > > > > So perhaps something like this, as per phy_optional_get(), would be > > > > better? > > > > > > > > usb3->phy = devm_phy_get(>dev, "usb"); > > > > if (IS_ERR(usb3->phy) && (PTR_ERR(usb3->phy) == -ENODEV)) > > > > usb3->phy = NULL; > > > > > > Thank you for the suggestion. I agree with you. > > > I think I should use devm_phy_optional_get() instead of dev_phy_get(), as > > > you mentioned :) > > > So, I will fix the code by using devm_phy_optional_get() first, and then > > > fix this. > > > > Thanks, I agree that would be a good fix. > > > > But perhaps it is better as a follow-up? > > Oops. Yes, I changed my mind after prepared the patch v2 and submitted v2. > I should have sent an email about this to you... > Anyway, thank you very much for reviewing the v2 patches! v2 looks good to me :) -- 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: serial: option: add HP LT4220 support
On Wed, Apr 11, 2018 at 02:36:52PM +0800, Edward Chang wrote: > This patch adds support for HP LT4220 > > Signed-off-by: Edward Chang> --- Thanks for the update. Please always include a changelog here when revising patches so we know what has changed since the previous version(s). Also include the patch version in the subject line (e.g. "[PATCH v2] USB: ..."). > drivers/usb/serial/option.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c > index c3f2522..d866cc0 100644 > --- a/drivers/usb/serial/option.c > +++ b/drivers/usb/serial/option.c > @@ -1927,6 +1927,8 @@ static int option_probe(struct usb_serial *serial, > { USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, > WETELECOM_PRODUCT_6802, 0xff, 0xff, 0xff) }, > { USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, > WETELECOM_PRODUCT_WMD300, 0xff, 0xff, 0xff) }, > { USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x421d, 0xff, 0xff, 0xff) }, /* > HP lt2523 (Novatel E371) */ > + { USB_DEVICE_INTERFACE_CLASS(0x03f0, 0x0857, 0xff), /* HP lt4220 */ > + .driver_info = (kernel_ulong_t)_le920a4_blacklist_1 }, So you dropped the id defines, but this still would not compile due to the updated blacklist mechanism I mentioned. Take a look at the commit I referred to for examples of how to specify the blacklist masks. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
Hi Heiko, On 4/10/2018 7:37 PM, Heiko Stübner wrote: > Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan: >> Hi Heiko, >> >> On 4/10/2018 4:28 PM, Heiko Stuebner wrote: >>> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso: devm_regulator_get_optional returns -ENODEV if the regulator isn't there, so if that's the case we have to make sure not to leave -ENODEV in the regulator pointer. Also, make sure we return 0 in that case, but correctly propagate any other errors. Also propagate the error from _dwc2_hcd_start. Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply") Cc: Amelie DelaunaySigned-off-by: Tomeu Vizoso >>> >>> The patch that gets fixed here also breaks display-output on dwc2-based >>> Rockchip devices (likely even more), probably due to making the regulator >>> framework hickup. >> >> Could you please elaborate what mean "breaks display-output". >> On which Kernel version you apply this patch? > > I think I may have written that poorly. _Without_ this patch I get > display breakage on the most recent torvalds/master (merge-window) > where "usb: dwc2: add support for host mode external vbus supply" is > applied and this patch fixes the issue. > > "breaks display output" means both hdmi + edp output are missing > also including the backlight staying off. > > The patch we're fixing here, causes a null-pointer dereference in the > regulator framework, which seems to also cause issues when other > regulators are enabled, which I think is what I'm seeing here. > > Thank you for clarification. Earlier you added "reviewed" tag, this feedback can assumed as "tested" tag. Thanks, Minas > Heiko > >> >> Thanks, >> Minas >> >>> With this patch applied, apart from not seeing the NULL-ptr, I also get >>> display output on my rk3288-veycron Chromebook again, so >>> >>> Tested-by: Heiko Stuebner >>> v2: Only overwrite the error in the pointer after checking it (Heiko Stübner ) v3: Remove hunks that shouldn't be in this patch v4: Don't overwrite the error code before returning it (kbuild test robot ) --- drivers/usb/dwc2/hcd.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 190f95964000..c51b73b3e048 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)>> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg) { + int ret; + hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus"); - if (IS_ERR(hsotg->vbus_supply)) - return 0; + if (IS_ERR(hsotg->vbus_supply)) { + ret = PTR_ERR(hsotg->vbus_supply); + hsotg->vbus_supply = NULL; + return ret == -ENODEV ? 0 : ret; + } return regulator_enable(hsotg->vbus_supply); } @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd) spin_unlock_irqrestore(>lock, flags); - dwc2_vbus_supply_init(hsotg); - - return 0; + return dwc2_vbus_supply_init(hsotg); } /* > > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: serial: option: add HP LT4220 support
This patch adds support for HP LT4220 Signed-off-by: Edward Chang--- drivers/usb/serial/option.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index c3f2522..d866cc0 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -1927,6 +1927,8 @@ static int option_probe(struct usb_serial *serial, { USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, WETELECOM_PRODUCT_6802, 0xff, 0xff, 0xff) }, { USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, WETELECOM_PRODUCT_WMD300, 0xff, 0xff, 0xff) }, { USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x421d, 0xff, 0xff, 0xff) }, /* HP lt2523 (Novatel E371) */ + { USB_DEVICE_INTERFACE_CLASS(0x03f0, 0x0857, 0xff), /* HP lt4220 */ + .driver_info = (kernel_ulong_t)_le920a4_blacklist_1 }, { } /* Terminating entry */ }; MODULE_DEVICE_TABLE(usb, option_ids); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html