RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
Thanks, Sriram,
It is better to move this delay out of spin-lock.
Best Regards
Jerry Huang
-Original Message-
From: Sriram Dash
Sent: Thursday, November 24, 2016 7:17 PM
To: Jerry Huang ; st...@rowland.harvard.edu;
gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
julia.law...@lip6.fr; Jerry Huang ; Ramneek Mehresh
; Suresh Gupta
Subject: RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697
>From: Changming Huang [mailto:jerry.hu...@nxp.com] As per USB
>specification, in the Suspend state, the status bit does not change
>until the port is suspended. However, there may be a delay in
>suspending a port if there is a transaction currently in progress on the bus.
>
>In the USBDR controller, the PORTSCx[SUSP] bit changes immediately when
>the application sets it and not when the port is actually suspended
>
>Workaround for this issue involves waiting for a minimum of 10ms to
>allow the controller to go into SUSPEND state before proceeding ahead
>
>Signed-off-by: Changming Huang
>Signed-off-by: Ramneek Mehresh
>---
> drivers/usb/host/ehci-fsl.c |3 +++
> drivers/usb/host/ehci-hub.c |2 ++
> drivers/usb/host/ehci.h |6 ++
> drivers/usb/host/fsl-mph-dr-of.c |2 ++
> include/linux/fsl_devices.h |1 +
> 5 files changed, 14 insertions(+)
>
>diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
>index 9f5ffb6..91701cc 100644
>--- a/drivers/usb/host/ehci-fsl.c
>+++ b/drivers/usb/host/ehci-fsl.c
>@@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci)
> if (pdata->has_fsl_erratum_a005275 == 1)
> ehci->has_fsl_hs_errata = 1;
>
>+ if (pdata->has_fsl_erratum_a005697 == 1)
>+ ehci->has_fsl_susp_errata = 1;
>+
> if ((pdata->operating_mode == FSL_USB2_DR_HOST) ||
> (pdata->operating_mode == FSL_USB2_DR_OTG))
> if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0)) diff --git
>a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index
>74f62d6..86d154e 100644
>--- a/drivers/usb/host/ehci-hub.c
>+++ b/drivers/usb/host/ehci-hub.c
>@@ -305,6 +305,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> USB_PORT_STAT_HIGH_SPEED)
> fs_idle_delay = true;
> ehci_writel(ehci, t2, reg);
>+ if (ehci_has_fsl_susp_errata(ehci))
>+ mdelay(10);
Hi Jerry,
Move the delay out of the spin lock. Other than that, it looks fine to me.
> changed = 1;
> }
> }
>diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index
>3f3b74a..7706e4a 100644
>--- a/drivers/usb/host/ehci.h
>+++ b/drivers/usb/host/ehci.h
>@@ -219,6 +219,7 @@ struct ehci_hcd { /* one per controller */
> unsignedno_selective_suspend:1;
> unsignedhas_fsl_port_bug:1; /* FreeScale */
> unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */
>+ unsignedhas_fsl_susp_errata:1; /*Freescale SUSP quirk*/
> unsignedbig_endian_mmio:1;
> unsignedbig_endian_desc:1;
> unsignedbig_endian_capbase:1;
>@@ -703,10 +704,15 @@ struct ehci_tt {
> #if defined(CONFIG_PPC_85xx)
> /* Some Freescale processors have an erratum (USB A-005275) in which
> * incoming packets get corrupted in HS mode
>+ * Some Freescale processors have an erratum (USB A-005697) in which
>+ * we need to wait for 10ms for bus to fo into suspend mode after
>+ * setting SUSP bit
> */
> #define ehci_has_fsl_hs_errata(e) ((e)->has_fsl_hs_errata)
>+#define ehci_has_fsl_susp_errata(e) ((e)->has_fsl_susp_errata)
> #else
> #define ehci_has_fsl_hs_errata(e) (0)
>+#define ehci_has_fsl_susp_errata(e) (0)
> #endif
>
> /*
>diff --git a/drivers/usb/host/fsl-mph-dr-of.c
>b/drivers/usb/host/fsl-mph-dr-of.c
>index f07ccb2..e90ddb5 100644
>--- a/drivers/usb/host/fsl-mph-dr-of.c
>+++ b/drivers/usb/host/fsl-mph-dr-of.c
>@@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct
>platform_device *ofdev)
> of_property_read_bool(np, "fsl,usb-erratum-a007792");
> pdata->has_fsl_erratum_a005275 =
> of_property_read_bool(np, "fsl,usb-erratum-a005275");
>+ pdata->has_fsl_erratum_a005697 =
>+ of_property_read_bool(np, "fsl,usb_erratum-a005697");
>
> /*
>* Determine whether phy_clk_valid needs to be checked diff --git
>a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h index
>f291291..60cef82
>100644
>--- a/include/linux/fsl_devices.h
>+++ b/include/linux/fsl_devices.h
>@@ -100,6 +100,7 @@ struct fsl_usb2_platform_data {
> unsignedalready_suspended:1;
> unsignedhas_fsl_erratum_a007792:1;
> unsignedhas_fsl_erratum_a005275:1;
>+ unsign
RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Mark Lord [mailto:ml...@pobox.com] > Sent: Friday, November 25, 2016 3:11 AM [...] > On 16-11-24 02:00 PM, Greg KH wrote: > > On Thu, Nov 24, 2016 at 01:34:08PM -0500, Mark Lord wrote: > >> One thought: bulk data streams are byte streams, not packets. > >> Scheduling on the USB bus can break up larger transfers across > >> multiple in-kernel buffers. A "real" URB buffer on USB2 is max 512 bytes. > >> The driver is providing 16384-byte buffers, and assumes that data will > >> never spill over from one such buffer to the next. > >> Yet the observations here consistently show otherwise. > > > > Wait, how do you know that data will not spill over? What is making > > that guarantee? Will the USB device send a "zero packet" in order to > > show that all of the "logical" data is now sent for this specific > > endpoint? Is there some sort of "framing" that the device does with the > > USB data so that the driver "knows" where the end of packet is? > > Exactly my point. > > > Check the zero-packet stuff for this device, that's tripped up many a > > USB driver writer over the years, myself included. > > I haven't tripped over it myself, but only because we were careful > to allow for such in the USB drivers I have worked on. > > The r8152 driver just assumes it never happens. What is the value of /sys/bus/usb/devices/.../power/control ? Could you make sure it is "on" and try again? Or you could call usb_disable_autosuspend() in probe(). Best Regards, Hayes
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On Thu, Nov 24, 2016 at 10:49:33PM -0500, Mark Lord wrote: > There is no possibility for them to be used for anything other than > USB receive buffers, for this driver only. Nothing in the driver > or kernel ever writes to those buffers after initial allocation, > and only the driver and USB host controller ever have pointers to the buffers. You really are going to have to break out that USB monitor to verify that this is the data coming across the wire. Note, there are "cheap" USB monitors that can be quite handy and that work on Linux: http://www.totalphase.com/products/beagle-usb12/ Or most high-end scopes have a USB mode that you can use to catch stuff like this (but they are usually harder to use/trigger and only store a very limited buffer). 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 net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 04:53 AM, Greg KH wrote: > Note, there are "cheap" USB monitors that can be quite handy and that work on > Linux: > http://www.totalphase.com/products/beagle-usb12/ USD$455/each in quantity, vs. USD$8 for the USB ethernet dongle. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 01:11 AM, Hayes Wang wrote: > Mark Lord [mailto:ml...@pobox.com] .. >> If that "return 0" statement is ever executed, doesn't it result >> in the loss/leak of a buffer? > > They would be found back by calling rtl_start_rx(), when the rx > is restarted. Good. I figured it was probably something like that, but wasn't entirely sure about the control flow around stop/restart there. 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 net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 01:51 AM, Hayes Wang wrote: > > Forgive me. I provide wrong information. This is about RTL8153, not RTL8152. No problem. Thanks for trying though. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 07:34 AM, Mark Lord wrote: > On 16-11-25 04:53 AM, Greg KH wrote: >> Note, there are "cheap" USB monitors that can be quite handy and that work >> on Linux: >> http://www.totalphase.com/products/beagle-usb12/ > > USD$455/each in quantity, vs. USD$8 for the USB ethernet dongle. Oh, wrong model. That one doesn't do USB2. The USB2 version is a mere USD$1300 in quantity. Seems like rather a lot of money just to report a bug in a USB driver. Perhaps the Linux Foundation might purchase one and loan it for this task? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 04:53 AM, Greg KH wrote: > On Thu, Nov 24, 2016 at 10:49:33PM -0500, Mark Lord wrote: >> There is no possibility for them to be used for anything other than >> USB receive buffers, for this driver only. Nothing in the driver >> or kernel ever writes to those buffers after initial allocation, >> and only the driver and USB host controller ever have pointers to the >> buffers. > > You really are going to have to break out that USB monitor to verify > that this is the data coming across the wire. Not sure why, because there really is no other way for the data to appear where it does at the beginning of that URB buffer. This does seem a rather unexpected burden to place upon someone reporting a regression in a USB network driver that corrupts user data. I have already spent about 50 hours looking at this issue, and everything now points firmly at some kind of FIFO overflow within the dongle itself. There is no evidence to the contrary. I am very happy to test any driver updates, or data collection mods provided by the author, to help the author find/fix the issue. One idea, might be to have the author try testing with the dongle connected through a USB1.1 hub, forcing it to slower speeds. This might make reproducing the issue (if indeed a FIFO overflow) easier, as the host transfers will then be slower than the ethernet wire speed. I have access to the hardware here next Tuesday. If we can scrounge up the USB analyzer, cables, and a suitable MS-Windows (ugh) machine of some kind, then I'll see if it can be programmed to somewhow capture the event. Probably just set it in continuous capture mode, and have the target system halt when it sees bad data at offset zero. This can take days to reproduce, so don't hold your breaths. Something useful to do in the meanwhile, is to then think about "what next" after the analyzer confirms the issue. -- Mark Lord Real-Time Remedies Inc. ml...@pobox.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On Tue, Nov 22, 2016 at 09:40:07AM +1100, NeilBrown wrote: > I agree that the question of where the responsibility for information > aggregation lies is open for discussion. If fact all details on how > things should work are always open for discussion. > I don't agree that this is the main different between our positions, > though I can see how you might get that impression. > You could even fix them so they look *exactly* like the notifiers that > Baolin is proposing. This is my key point. It is not the end result > that I particularly object to (though I do object to some details). It Ah, OK. This really hadn't been at all clear - both Baolin and I had the impression that the it was both that were blockers for you. What were the details here? > is the process of getting to the end result that I don't like. If the > current system doesn't work and something different is needed, then the > correct thing to do is to transform the existing system into something > new that works better. This should be a clear series of steps. Each Sometimes there's something to be said for working out what we want things to look like before setting out to make these gradual refactorings and sometimes the refactorings are just more pain than they're worth, especially when they go across subsystems. In this case I do worry about the cross subsystem aspect causing hassle, it may be more practical to do anything that represents an interface change by adding the new interface, converting the users to it and then removing the old interface. At the very least the series should grow to incorporate conversion of the existing users though. Baolin, I think this does need adding to the series but probably best to think about how to do it - some of Neil's suggestions for incremental steps do seem like they should be useful for organizing things here, probably we can get some things that can be done internally within individual drivers merged while everything else is under discussion. > But I think here my key point got lost too, in part because it was hard > to refer to an actual instance. > My point was that in the present patch set, the "usb charger" is given > a name which is dependant on discovery order, and only supports > lookup-by-name. This cannot work. There's two bits here: one is the way names are assigned and the other is the lookup by name. I agree that the lookup by name isn't particularly useful as things stand, that could just be dropped until some naming mechanism is added. We'd be more likely to use phandles in DT systems, I don't know what ACPI systems would look like but I guess it'd be something similar. > If they supported lookup by phy-name or lookup-by-active (i.e. "find me > any usb-charger which has power available"), or look up by some other > attribute, then discover-order naming could work. But the only > lookup-mechanism is by-name, and the names aren't reliably stable. So > the name/lookup system proposed cannot possibly do anything useful > with more than one usb_charger. Baolin, I think adding a DT binding and lookup mechanism makes sense here - do you agree? signature.asc Description: PGP signature
Re: [PATCH v2] fsl/usb: Workarourd for USB erratum-A005697
Hello.
On 11/25/2016 06:24 AM, Changming Huang wrote:
The EHCI specification states the following in the SUSP bit description:
In the Suspend state, the port is senstive to resume detection.
Sensitive.
Note that the bit status does not change untile the port is suspended and
Until.
that there may be a delay in susupending a port if there is a transaction
Suspending.
currently in progress on the USB.
However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes immediately
when the application sets it and not when the port is actually suspended.
So the application must wait for at least 10 milliseconds after a port
indicates that it is suspended, to make sure this port has entered
suspended state before initiating this port resume using the Force Port
Resume bit. This bit is for NXP controller, not EHCI compatible.
Signed-off-by: Changming Huang
Signed-off-by: Ramneek Mehresh
[...]
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 74f62d6..81e2310 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -310,6 +310,13 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
spin_unlock_irq(&ehci->lock);
+ if (changed && ehci_has_fsl_susp_errata(ehci))
+ /* Wait for at least 10 millisecondes to ensure the controller
Milliseconds.
+* enter the suspend status before initiating a port resume
+* using the Fore Port Resume bit (Not-EHCI compatible).
Maybe force?
s/Not/non/ also.
+*/
+ usleep_range(1, 2);
+
if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
/*
* Wait for HCD to enter low-power mode or for the bus
[...]
@@ -703,10 +704,15 @@ struct ehci_tt {
#if defined(CONFIG_PPC_85xx)
/* Some Freescale processors have an erratum (USB A-005275) in which
* incoming packets get corrupted in HS mode
+ * Some Freescale processors have an erratum (USB A-005697) in which
+ * we need to wait for 10ms for bus to fo into suspend mode after
Fo?
[...]
MBR, Sergei
--
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] fsl/usb: Workarourd for USB erratum-A005697
Hi, Alan, Maybe other USB controller does not need this delay. However, our silicon errata point out, in the USBDR controller, the PORTCx[SUSP] bit changes immediately when the application sets it and not when the port is actually suspended, so the application must wait for at least 10 milliseconds after a port indicates that it is suspended to ensure the port has entered SUSPEND status before initiating this port resume using the Force Port Resume (which is one bit for NXP silicon, not-EHCI compatible). I will add more comment to understand why we need this delay in next version. Best Regards Jerry Huang -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Friday, November 25, 2016 12:54 AM To: Sriram Dash Cc: Jerry Huang ; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; julia.law...@lip6.fr; Ramneek Mehresh ; Suresh Gupta Subject: RE: [PATCH] fsl/usb: Workarourd for USB erratum-A005697 On Thu, 24 Nov 2016, Sriram Dash wrote: > >From: Changming Huang [mailto:jerry.hu...@nxp.com] As per USB > >specification, in the Suspend state, the status bit does not change > >until the port is suspended. However, there may be a delay in > >suspending a port if there is a transaction currently in progress on the bus. > > > >In the USBDR controller, the PORTSCx[SUSP] bit changes immediately > >when the application sets it and not when the port is actually > >suspended > > > >Workaround for this issue involves waiting for a minimum of 10ms to > >allow the controller to go into SUSPEND state before proceeding ahead The USB core guarantees that there won't be any data transactions in progress when a root hub is suspended. There might possibly be an SOF transaction, but that doesn't take more than a few microseconds at most. Certainly nowhere near 10 ms! Given that we already perform a 150-us delay, is this change really needed? 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 net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 04:52 AM, Hayes Wang wrote: .. > What is the value of /sys/bus/usb/devices/.../power/control ? That entry does not exist -- power control is completely disabled on this board. Good try, though -- USB power control still causes me trouble on PCs with mice and remote controls. But not here. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On Fri, Nov 25, 2016 at 07:41:42AM -0500, Mark Lord wrote: > On 16-11-25 07:34 AM, Mark Lord wrote: > > On 16-11-25 04:53 AM, Greg KH wrote: > >> Note, there are "cheap" USB monitors that can be quite handy and that work > >> on Linux: > >>http://www.totalphase.com/products/beagle-usb12/ > > > > USD$455/each in quantity, vs. USD$8 for the USB ethernet dongle. > > Oh, wrong model. That one doesn't do USB2. > The USB2 version is a mere USD$1300 in quantity. > > Seems like rather a lot of money just to report a bug in a USB driver. > Perhaps the Linux Foundation might purchase one and loan it for this task? You already have access to a USB analyzer you said, why would I try to buy one and ship it around the world instead? Makes no sense... 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 net 1/2] r8152: fix the sw rx checksum is unavailable
On 16-11-25 09:22 AM, Greg KH wrote: > On Fri, Nov 25, 2016 at 07:41:42AM -0500, Mark Lord wrote: >> On 16-11-25 07:34 AM, Mark Lord wrote: >>> On 16-11-25 04:53 AM, Greg KH wrote: Note, there are "cheap" USB monitors that can be quite handy and that work on Linux: http://www.totalphase.com/products/beagle-usb12/ >>> >>> USD$455/each in quantity, vs. USD$8 for the USB ethernet dongle. >> >> Oh, wrong model. That one doesn't do USB2. >> The USB2 version is a mere USD$1300 in quantity. >> >> Seems like rather a lot of money just to report a bug in a USB driver. >> Perhaps the Linux Foundation might purchase one and loan it for this task? > > You already have access to a USB analyzer you said, why would I try to > buy one and ship it around the world instead? Makes no sense... No, the company where I am consulting has a paperweight called a "USB analyzer". It doesn't work with Linux machines. You are the one who suggested purchase of a working Linux compatible unit, so I was just following up to see if you were serious about that. No worries. I'll see if the paperweight can be converted into something useful next week. Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: ohci: s3c2410: add device tree support
This series adds support for configuring Samsung's s3c2410 and compatible USB OHCI controller via devicetree. Tested on FriendlyARM mini2440, based on s3c2440 SoC. Sergio Prado (2): dt-bindings: usb: add DT binding for s3c2410 USB OHCI controller usb: ohci: s3c2410: allow probing from device tree .../devicetree/bindings/usb/s3c2410-usb.txt| 22 ++ drivers/usb/host/ohci-s3c2410.c| 8 2 files changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/s3c2410-usb.txt -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB OHCI controller
Adds the device tree bindings description for Samsung S3C2410 and
compatible USB OHCI controller.
Signed-off-by: Sergio Prado
---
.../devicetree/bindings/usb/s3c2410-usb.txt| 22 ++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/s3c2410-usb.txt
diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
new file mode 100644
index ..e45b38ce2986
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
@@ -0,0 +1,22 @@
+Samsung S3C2410 and compatible SoC USB controller
+
+OHCI
+
+Required properties:
+ - compatible: should be "samsung,s3c2410-ohci" for USB host controller
+ - reg: address and lenght of the controller memory mapped region
+ - interrupts: interrupt number for the USB OHCI controller
+ - clocks: Should reference the bus and host clocks
+ - clock-names: Should contain two strings
+ "usb-bus-host" for the USB bus clock
+ "usb-host" for the USB host clock
+
+Example:
+
+usb0: ohci@4900 {
+ compatible = "samsung,s3c2410-ohci";
+ reg = <0x4900 0x100>;
+ interrupts = <0 0 26 3>;
+ clocks = <&clocks UCLK>, <&clocks HCLK_USBH>;
+ clock-names = "usb-bus-host", "usb-host";
+};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: ohci: s3c2410: allow probing from device tree
Allows configuring Samsung's s3c2410 USB OHCI controller using a
devicetree.
Signed-off-by: Sergio Prado
---
drivers/usb/host/ohci-s3c2410.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c
index 7a1919ca543a..d8e03a801f2e 100644
--- a/drivers/usb/host/ohci-s3c2410.c
+++ b/drivers/usb/host/ohci-s3c2410.c
@@ -457,6 +457,13 @@ static int ohci_hcd_s3c2410_drv_resume(struct device *dev)
.resume = ohci_hcd_s3c2410_drv_resume,
};
+static const struct of_device_id ohci_hcd_s3c2410_dt_ids[] = {
+ { .compatible = "samsung,s3c2410-ohci" },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, ohci_hcd_s3c2410_dt_ids);
+
static struct platform_driver ohci_hcd_s3c2410_driver = {
.probe = ohci_hcd_s3c2410_drv_probe,
.remove = ohci_hcd_s3c2410_drv_remove,
@@ -464,6 +471,7 @@ static int ohci_hcd_s3c2410_drv_resume(struct device *dev)
.driver = {
.name = "s3c2410-ohci",
.pm = &ohci_hcd_s3c2410_pm_ops,
+ .of_match_table = ohci_hcd_s3c2410_dt_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
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
On Fri, Nov 25, 2016 at 07:49:35AM -0500, Mark Lord wrote: > On 16-11-25 04:53 AM, Greg KH wrote: > > On Thu, Nov 24, 2016 at 10:49:33PM -0500, Mark Lord wrote: > >> There is no possibility for them to be used for anything other than > >> USB receive buffers, for this driver only. Nothing in the driver > >> or kernel ever writes to those buffers after initial allocation, > >> and only the driver and USB host controller ever have pointers to the > >> buffers. > > > > You really are going to have to break out that USB monitor to verify > > that this is the data coming across the wire. > > Not sure why, because there really is no other way for the data to > appear where it does at the beginning of that URB buffer. Broken USB host controller driver, or the device really is sending that data to the host. It's either one or the other, and the only way you can rule one of them out is to look at the data on the wire. best of 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 v2] fsl/usb: Workarourd for USB erratum-A005697
On Fri, 25 Nov 2016, Changming Huang wrote:
> The EHCI specification states the following in the SUSP bit description:
> In the Suspend state, the port is senstive to resume detection.
> Note that the bit status does not change untile the port is suspended and
> that there may be a delay in susupending a port if there is a transaction
> currently in progress on the USB.
>
> However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes immediately
> when the application sets it and not when the port is actually suspended.
>
> So the application must wait for at least 10 milliseconds after a port
> indicates that it is suspended, to make sure this port has entered
> suspended state before initiating this port resume using the Force Port
> Resume bit. This bit is for NXP controller, not EHCI compatible.
>
> Signed-off-by: Changming Huang
> Signed-off-by: Ramneek Mehresh
> ---
> Change in v2:
> - move sleep out of spin-lock and add more comment for this workaround
This patch is incomplete.
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -310,6 +310,13 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> }
> spin_unlock_irq(&ehci->lock);
>
> + if (changed && ehci_has_fsl_susp_errata(ehci))
> + /* Wait for at least 10 millisecondes to ensure the controller
> + * enter the suspend status before initiating a port resume
> + * using the Fore Port Resume bit (Not-EHCI compatible).
> + */
The proper style for multi-line comments is:
/*
* Wait for ...
* ...
*/
Also, "Force" is misspelled.
> + usleep_range(1, 2);
> +
> if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
> /*
>* Wait for HCD to enter low-power mode or for the bus
The patch does not change the code around line 1190 in the original
file:
/* After above check the port must be connected.
* Set appropriate bit thus could put phy into low power
* mode if we have tdi_phy_lpm feature
*/
temp &= ~PORT_WKCONN_E;
temp |= PORT_WKDISC_E | PORT_WKOC_E;
ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
This code sets the PORT_SUSPEND bit but does not have a 10-ms delay.
You need to add a delay here.
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: Multiple problems with the Linux kernel on an AMD desktop
On Fri, Nov 25, 2016 at 02:05:48PM -0200, Rogério Brito wrote:
> In fact, I have quite a few computers that are not running Linux that well
> at this moment and I guess that lack of report from final users (or,
> perhaps, reports being lost in the way) prevents those problems from getting
> fixed.
CC me on those, I'd take a look.
> Ihope that my efforts will help other users to have fewer problems with
> Linux on older machines, at least.
> To speed things up a bit, I grabbed Ubuntu's precompiled 4.8 and 4.9-rc6
> (without any patches on top of Linus's tree) and booted on this machine.
>
> The scanner problem is still there with vanilla 4.8 (with the irqpoll
> option), but is gone with vanilla 4.9-rc6 (with the irqpoll option).
Does -rc6 work *without* irqpoll?
Also, you can diff dmesg from both kernels and see whether you can spot
something relevant.
> I guess that backports of fixes to this (once detected) are needed for
> -stable kernels that distributions are shipping with?
Yes, once we know what fixes the issues.
> The other problems ("nobody cared" and the flood of evbug/lost xx rtc
> interrupts messages) remain with 4.9-rc6.
>
> Interestingly, for a layman like me:
>
> * if I remove the irqpoll option, the "hpet1: lost xx rtc interrupts" messages
Aha, so irqpoll is crap. Just remove it.
> are gone, but I still get messages like
>
> [ 130.007219] evbug: Event. Dev: input6, Type: 0, Code: 0, Value: 0
> [ 130.167191] evbug: Event. Dev: input6, Type: 4, Code: 4, Value: 458767
> [ 130.167195] evbug: Event. Dev: input6, Type: 1, Code: 38, Value: 1
> [ 130.167197] evbug: Event. Dev: input6, Type: 0, Code: 0, Value: 0
> [ 130.247174] evbug: Event. Dev: input6, Type: 4, Code: 4, Value: 458767
>
> * if I keep the irqpoll option, I get both "hpet1: lost xx rtc interrupts"
> AND the evbug messages remain.
Just blacklist that module, it is for debugging input events.
> I'm attaching the dmesg of 4.9-rc6 both with and without irqpoll to this
> message.
Thanks.
[0.00] DMI: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled
by O.E.M., BIOS 0500 05/11/2010
Has your BIOS *ever* been updated? If not, why not?
Yap, that BIOS is "fun":
[0.00] Aperture pointing to e820 RAM. Ignoring.
[0.00] AGP: Your BIOS doesn't leave an aperture memory hole
[0.00] AGP: Please enable the IOMMU option in the BIOS setup
[0.00] AGP: This costs you 64MB of RAM
Do you have an IOMMU option in your BIOS?
[ 30.434052] usblp 5-2:1.1: usblp1: USB Bidirectional printer dev 2 if 1 alt
0 proto 2 vid 0x03F0 pid 0x4811
[ 34.157510] irq 18: nobody cared (try booting with the "irqpoll" option)
[ 34.157516] CPU: 3 PID: 0 Comm: swapper/3 Not tainted
4.9.0-040900rc6-generic #201611201731
[ 34.157518] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To
be filled by O.E.M., BIOS 0500 05/11/2010
[ 34.157520] 8a4cdfd83eb8 8f217542 8a4cd6fbb200
8a4cd6fbb2b4
[ 34.157524] 8a4cdfd83ee8 8eee5005 8a4cd6fbb200
[ 34.157527] 8fd5d560 0022 8a4cdfd83f20
8eee5393
[ 34.157529] Call Trace:
[ 34.157531]
[ 34.157537] [] dump_stack+0x63/0x81
[ 34.157540] [] __report_bad_irq+0x35/0xc0
[ 34.157542] [] note_interrupt+0x243/0x290
[ 34.157544] [] handle_irq_event_percpu+0x54/0x80
[ 34.157546] [] handle_irq_event+0x3e/0x60
[ 34.157548] [] handle_fasteoi_irq+0x9f/0x150
[ 34.157551] [] handle_irq+0x1a/0x30
[ 34.157554] [] do_IRQ+0x4b/0xd0
[ 34.157556] [] common_interrupt+0x82/0x82
[ 34.157557]
[ 34.157560] [] ? native_safe_halt+0x6/0x10
[ 34.157562] [] default_idle+0x20/0xd0
[ 34.157565] [] arch_cpu_idle+0xf/0x20
[ 34.157568] [] default_idle_call+0x23/0x30
[ 34.157570] [] cpu_startup_entry+0x1d0/0x240
[ 34.157573] [] start_secondary+0x151/0x190
[ 34.157575] handlers:
[ 34.157577] [] usb_hcd_irq
[ 34.157578] [] usb_hcd_irq
[ 34.157580] [] usb_hcd_irq
[ 34.157581] Disabling IRQ #18
Looks to me like that USB host controller driver doesn't want to handle
its interrupt.
Lemme add USB people as I have no clue here why...
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
--
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: Multiple problems with the Linux kernel on an AMD desktop
Hi, Clemens and Borislav.
On Nov 25 2016, Clemens Ladisch wrote:
> Rogério Brito wrote:
> > * I have never been able to boot this computer of mine without the option
> > irqpoll---otherwise, I get the nobody cared message.
>
> The "nobody cared" message indicates that there were too many interrupts
> that no driver felt responsible for, so the kernel has disabled that
> interrupt vector. The irqpoll option is a workaround to get the devices
> on that interrupt vector to work, but it's not perfect.
Ah, great to know. I don't know if this is related or not, but I read
somewhere (don't remember where) that the machine may have performance
slightly reduced when irqpoll is used.
> It's possible that most of your problems are caused by the irqpoll option.
Excellent to know.
> What IRQ is the problematic one (see the "nobody cared" message)? What
> devices are connected to it (see /proc/interrupts)?
>From the dmesg log, the interrupt is 18.
Here is part from /proc/interrupts that contains interrupt 18 *without* irqpoll:
---
CPU0 CPU1 CPU2 CPU3
0: 47 0 0 0 IO-APIC 2-edge timer
1: 0 0 0 2 IO-APIC 1-edge i8042
7: 0 0 0 0 IO-APIC 7-edge
parport0
8: 0 0 0 1 IO-APIC 8-edge rtc0
9: 0 0 0 0 IO-APIC 9-fasteoi acpi
10: 0 0 0 0 IO-APIC 10-edge radeon
12: 0 0 0 4 IO-APIC 12-edge i8042
16: 0 96 4990 IO-APIC 16-fasteoi
ohci_hcd:usb3, ohci_hcd:usb4, snd_hda_intel:card0
17: 0 2457 1140 IO-APIC 17-fasteoi
ehci_hcd:usb1
18: 1 11 43 99947 IO-APIC 18-fasteoi
ohci_hcd:usb5, ohci_hcd:usb6, ohci_hcd:usb7
19: 0 0 0 0 IO-APIC 19-fasteoi
ehci_hcd:usb2
22: 0 22169139 8731 IO-APIC 22-fasteoi
ahci[:00:11.0]
25: 0 0 11753 PCI-MSI 1048576-edge
eth0
(...)
---
Here is part from /proc/interrupts that contains interrupt 18 *with* irqpoll:
---
CPU0 CPU1 CPU2 CPU3
0: 46 0 0 0 IO-APIC 2-edge timer
1: 0 0 0 2 IO-APIC 1-edge i8042
7: 0 0 0 0 IO-APIC 7-edge
parport0
8: 0 0 0 1 IO-APIC 8-edge rtc0
9: 0 0 0 0 IO-APIC 9-fasteoi acpi
10: 0 0 0 0 IO-APIC 10-edge radeon
12: 0 0 0 4 IO-APIC 12-edge i8042
16: 0103 6983 IO-APIC 16-fasteoi
ohci_hcd:usb3, ohci_hcd:usb4, snd_hda_intel:card0
17: 0588 0144 IO-APIC 17-fasteoi
ehci_hcd:usb1
18: 0 0 0705 IO-APIC 18-fasteoi
ohci_hcd:usb5, ohci_hcd:usb6, ohci_hcd:usb7
19: 0 0 0 0 IO-APIC 19-fasteoi
ehci_hcd:usb2
22: 0 18049 4 8540 IO-APIC 22-fasteoi
ahci[:00:11.0]
25: 0 0 0327 PCI-MSI 1048576-edge
eth0
(...)
---
I'm attaching both files to this message.
> Does the problem go away when you prevent the corresponding driver(s) from
> loading?
Since the OHCI_HCD driver is built-in (as opposed to a module), I don't know
how to disable it. I can try to recompile the kernel with it as a module and
rename it as some garbage, so that it doesn't get loaded...
Thanks a lot,
--
Rogério Brito : rbrito@{ime.usp.br,gmail.com} : GPG key 4096R/BCFC
http://rb.doesntexist.org/blog : Projects : https://github.com/rbrito/
DebianQA: http://qa.debian.org/developer.php?login=rbrito%40ime.usp.br
CPU0 CPU1 CPU2 CPU3
0: 47 0 0 0 IO-APIC 2-edge timer
1: 0 0 0 2 IO-APIC 1-edge i8042
7: 0 0 0 0 IO-APIC 7-edge
parport0
8: 0 0 0 1 IO-APIC 8-edge rtc0
9: 0 0 0 0 IO-APIC 9-fasteoi acpi
10: 0 0 0 0 IO-APIC 10-edge radeon
12: 0 0 0 4 IO-APIC 12-edge i8042
16: 0 96 4990 IO-APIC 16-fasteoi
ohci_hcd:usb3, ohci_hcd:usb4, snd_hda_intel:card0
17: 0 24
Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord Date: Fri, 25 Nov 2016 07:49:35 -0500 > On 16-11-25 04:53 AM, Greg KH wrote: >> On Thu, Nov 24, 2016 at 10:49:33PM -0500, Mark Lord wrote: >>> There is no possibility for them to be used for anything other than >>> USB receive buffers, for this driver only. Nothing in the driver >>> or kernel ever writes to those buffers after initial allocation, >>> and only the driver and USB host controller ever have pointers to the >>> buffers. >> >> You really are going to have to break out that USB monitor to verify >> that this is the data coming across the wire. > > Not sure why, because there really is no other way for the data to > appear where it does at the beginning of that URB buffer. > > This does seem a rather unexpected burden to place upon someone > reporting a regression in a USB network driver that corrupts user data. If you are the only person who can actively reproduce this, which seems to be the case right now, this is unfortunately the only way to reach a proper analysis and fix. -- 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: Multiple problems with the Linux kernel on an AMD desktop
On Fri, Nov 25, 2016 at 02:53:00PM -0200, Rogério Brito wrote: > Here is part from /proc/interrupts that contains interrupt 18 *without* > irqpoll: > > --- >CPU0 CPU1 CPU2 CPU3 > 0: 47 0 0 0 IO-APIC 2-edge timer > 1: 0 0 0 2 IO-APIC 1-edge i8042 > 7: 0 0 0 0 IO-APIC 7-edge > parport0 > 8: 0 0 0 1 IO-APIC 8-edge rtc0 > 9: 0 0 0 0 IO-APIC 9-fasteoi acpi > 10: 0 0 0 0 IO-APIC 10-edge > radeon > 12: 0 0 0 4 IO-APIC 12-edge i8042 > 16: 0 96 4990 IO-APIC 16-fasteoi > ohci_hcd:usb3, ohci_hcd:usb4, snd_hda_intel:card0 > 17: 0 2457 1140 IO-APIC 17-fasteoi > ehci_hcd:usb1 > 18: 1 11 43 99947 IO-APIC 18-fasteoi > ohci_hcd:usb5, ohci_hcd:usb6, ohci_hcd:usb7 Can you connect the printer to a different port so that it doesn't use OCHI to see if it makes any difference? > 19: 0 0 0 0 IO-APIC 19-fasteoi > ehci_hcd:usb2 > 22: 0 22169139 8731 IO-APIC 22-fasteoi > ahci[:00:11.0] > 25: 0 0 11753 PCI-MSI 1048576-edge > eth0 > (...) > --- -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. -- 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: Multiple problems with the Linux kernel on an AMD desktop
Hi, Clemens and others.
On Nov 25 2016, Clemens Ladisch wrote:
> Rogério Brito wrote:
> > [ 130.007219] evbug: Event. Dev: input6, Type: 0, Code: 0, Value: 0
>
> The evbug module is intended for debugging; it dumps all input events
> into syslog. If you do not want these messages, do not load this module.
> (If it is loaded automatically, you have an actual bug.)
It *was* loaded automatically, and I didn't specifically asked it to be
loaded, but I'm not sure if other parts of userspace forced it to be
loaded. I will disable it, then.
Here is the relevant part of the config file:
,[ grep -i evbug /boot/config-4.9.0-040900rc6-generic ]
| CONFIG_INPUT_EVBUG=m
`
Thanks,
--
Rogério Brito : rbrito@{ime.usp.br,gmail.com} : GPG key 4096R/BCFC
http://rb.doesntexist.org/blog : Projects : https://github.com/rbrito/
DebianQA: http://qa.debian.org/developer.php?login=rbrito%40ime.usp.br
--
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: Amlogic Meson GXL/GXM USB support (dwc2 and dwc3)
On Thu, Nov 24, 2016 at 6:42 PM, Martin Blumenstingl wrote: > Hello, > > currently I am trying to get the USB controllers on the Amlogic Meson > GXL SoCs working: there is one dwc2 and dwc3 controller each. > > The SoC itself provides 3x USB2 PHYs and 1x USB3 PHY. > I wrote drivers for both of them based on the vendor kernel, see [0] > The PHY registers of the USB2 PHYs seem to be identical, regardless of > whether they are connected to dwc2 or dwc3. > The USB3 PHY also takes care of the OTG interrupts (to switch between > host and device) and seems to inform the USB2 PHY about mode-changes. > USB3 seems to be disabled in the dwc3 configuration, meaning it > provides only high-speed support. > > The dwc3 core fails to initialize currently due to some DMA issues > which will be fixed in Linux v4.10 - the corresponding patchset can be > found here: [1] > With these patches applied we get the dwc3 controller to initialize: > xhci-hcd xhci-hcd.0.auto: xHCI Host Controller > xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 1 > xhci-hcd xhci-hcd.0.auto: hcc params 0x0228f664 hci version 0x100 > quirks 0x00010010 > xhci-hcd xhci-hcd.0.auto: irq 20, io mem 0xc900 > hub 1-0:1.0: USB hub found > hub 1-0:1.0: 3 ports detected > xhci-hcd xhci-hcd.0.auto: xHCI Host Controller > xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 2 > usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. > hub 2-0:1.0: USB hub found > hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19) > (the last message seems fine, there are probably no USB3 ports enabled > in the dwc3 hardware configuration) > > strange fact #1: there are 3 USB2 PHYs enabled in the dwc3 core > (regdump from the vendor kernel - a full version is attached): > GUSB2PHYCFG(0) = 0x40102500 > GUSB2PHYCFG(1) = 0x40102540 > GUSB2PHYCFG(2) = 0x40102540 > (this explains the "hub 1-0:1.0: 3 ports detected" message on my GXM > board - other SoCs seem to have a different number of ports available > based on the vendor sources, GXL seem to have 2 ports, while "TXL" > seems to have 4 ports). > > I tried enabling all available PHYs in the SoC and giving > GUSB2PHYCFG(1 and 2) the same tickle that is currently done in > dwc3_phy_setup() for GUSB2PHYCFG(0). > The LED on my thumb drive flashes when I plug it into the dwc3 port, > but I don't get any interrupts and the kernel does not recognize any > new USB device. it turns out that this was a PHY problem. due to whatever reason it seems that we have to enable all 3 USB2 PHYs in the SoC to make *any* of these work! After lots of headache and refactoring I have my USB2 PHY driver now in a working state (in case someone is interested in this early code: [0]). > For the dwc2 controller I am probably missing a clock somewhere, > because it's reporting: > dwc2 c910.usb: Configuration mismatch. dr_mode forced to device > dwc2 c910.usb: dwc2_core_reset() HANG! Soft Reset GRSTCTL=8001 > dwc2 c910.usb: Specified GNPTXFDEP=1024 > 768 > dwc2 c910.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM > I must admit that I have been focusing on the dwc3 controller so far, > so I don't have any more information here. I do not have any (new) findings here as I was busy with the dwc3 PHY driver again. [0] https://github.com/xdarklight/linux/commits/meson-gx-integration-4.10-20161125 -- 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: Amlogic Meson GXL/GXM USB support (dwc2 and dwc3)
On Thu, Nov 24, 2016 at 6:42 PM, Martin Blumenstingl wrote: > Hello, > > currently I am trying to get the USB controllers on the Amlogic Meson > GXL SoCs working: there is one dwc2 and dwc3 controller each. > > The SoC itself provides 3x USB2 PHYs and 1x USB3 PHY. > I wrote drivers for both of them based on the vendor kernel, see [0] > The PHY registers of the USB2 PHYs seem to be identical, regardless of > whether they are connected to dwc2 or dwc3. > The USB3 PHY also takes care of the OTG interrupts (to switch between > host and device) and seems to inform the USB2 PHY about mode-changes. > USB3 seems to be disabled in the dwc3 configuration, meaning it > provides only high-speed support. > > The dwc3 core fails to initialize currently due to some DMA issues > which will be fixed in Linux v4.10 - the corresponding patchset can be > found here: [1] > With these patches applied we get the dwc3 controller to initialize: > xhci-hcd xhci-hcd.0.auto: xHCI Host Controller > xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 1 > xhci-hcd xhci-hcd.0.auto: hcc params 0x0228f664 hci version 0x100 > quirks 0x00010010 > xhci-hcd xhci-hcd.0.auto: irq 20, io mem 0xc900 > hub 1-0:1.0: USB hub found > hub 1-0:1.0: 3 ports detected > xhci-hcd xhci-hcd.0.auto: xHCI Host Controller > xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus number 2 > usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. > hub 2-0:1.0: USB hub found > hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19) > (the last message seems fine, there are probably no USB3 ports enabled > in the dwc3 hardware configuration) > > strange fact #1: there are 3 USB2 PHYs enabled in the dwc3 core > (regdump from the vendor kernel - a full version is attached): > GUSB2PHYCFG(0) = 0x40102500 > GUSB2PHYCFG(1) = 0x40102540 > GUSB2PHYCFG(2) = 0x40102540 > (this explains the "hub 1-0:1.0: 3 ports detected" message on my GXM > board - other SoCs seem to have a different number of ports available > based on the vendor sources, GXL seem to have 2 ports, while "TXL" > seems to have 4 ports). > > I tried enabling all available PHYs in the SoC and giving > GUSB2PHYCFG(1 and 2) the same tickle that is currently done in > dwc3_phy_setup() for GUSB2PHYCFG(0). > The LED on my thumb drive flashes when I plug it into the dwc3 port, > but I don't get any interrupts and the kernel does not recognize any > new USB device. > > For the dwc2 controller I am probably missing a clock somewhere, > because it's reporting: > dwc2 c910.usb: Configuration mismatch. dr_mode forced to device > dwc2 c910.usb: dwc2_core_reset() HANG! Soft Reset GRSTCTL=8001 > dwc2 c910.usb: Specified GNPTXFDEP=1024 > 768 > dwc2 c910.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM > I must admit that I have been focusing on the dwc3 controller so far, > so I don't have any more information here. I found out that the dwc2 block is not used on my GXM SoC. the configuration register forces it to "device" mode only, so it wouldn't be of much use anyways. with my latest PHY fixes I can now use both USB ports (both provided by the dwc3 controller's internal hub). the work-in-progress code can be found here: [0] Regards, Martin [0] https://github.com/xdarklight/linux/tree/meson-gx-integration-4.10-20161126 -- 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 v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
On Sat, Nov 26 2016, Mark Brown wrote: > [ Unknown signature status ] > On Tue, Nov 22, 2016 at 09:40:07AM +1100, NeilBrown wrote: > >> I agree that the question of where the responsibility for information >> aggregation lies is open for discussion. If fact all details on how >> things should work are always open for discussion. >> I don't agree that this is the main different between our positions, >> though I can see how you might get that impression. > >> You could even fix them so they look *exactly* like the notifiers that >> Baolin is proposing. This is my key point. It is not the end result >> that I particularly object to (though I do object to some details). It > > Ah, OK. This really hadn't been at all clear - both Baolin and I had > the impression that the it was both that were blockers for you. What > were the details here? I don't really like the idea of a separate "usb charger" object. It looks too much like a "midlayer" and they tend to get in the way. But if a convincing case could be made that changing from the current design to that aspect of the proposed design brings measurable benefits, then I would certainly assess that case on its merits. No such case was made, and the patchset didn't seem to even acknowledge the existing design. When I said "I do object to some details" it was details of the end result, not details of what took responsibility of information aggregation (in case that wasn't clear). Those details were everything that duplicated existing functionality, or ignored existing functionality, or was simply unworkable. e.g. the lack of proper integration with extcon, the new sysfs attributes, the name-lookup mechanism. Probably others. > >> is the process of getting to the end result that I don't like. If the >> current system doesn't work and something different is needed, then the >> correct thing to do is to transform the existing system into something >> new that works better. This should be a clear series of steps. Each > > Sometimes there's something to be said for working out what we want > things to look like before setting out to make these gradual > refactorings and sometimes the refactorings are just more pain than > they're worth, especially when they go across subsystems. In this case > I do worry about the cross subsystem aspect causing hassle, it may be > more practical to do anything that represents an interface change by > adding the new interface, converting the users to it and then removing > the old interface. Yes, you need a clear vision of the goal. You also need a clear vision of the starting point. There was no evidence of the latter. Yes, sometimes you need to create a new thing and transition users over, then discard the old. There was no discarding of the old. > > At the very least the series should grow to incorporate conversion of > the existing users though. Baolin, I think this does need adding to the > series but probably best to think about how to do it - some of Neil's > suggestions for incremental steps do seem like they should be useful > for organizing things here, probably we can get some things that can be > done internally within individual drivers merged while everything else > is under discussion. I would be very encouraged to see those simple things done first! Seeing the series grow isn't much fun, but seeing preliminary work land certainly is. > >> But I think here my key point got lost too, in part because it was hard >> to refer to an actual instance. >> My point was that in the present patch set, the "usb charger" is given >> a name which is dependant on discovery order, and only supports >> lookup-by-name. This cannot work. > > There's two bits here: one is the way names are assigned and the other > is the lookup by name. I agree that the lookup by name isn't > particularly useful as things stand, that could just be dropped until > some naming mechanism is added. We'd be more likely to use phandles in > DT systems, I don't know what ACPI systems would look like but I guess > it'd be something similar. > >> If they supported lookup by phy-name or lookup-by-active (i.e. "find me >> any usb-charger which has power available"), or look up by some other >> attribute, then discover-order naming could work. But the only >> lookup-mechanism is by-name, and the names aren't reliably stable. So >> the name/lookup system proposed cannot possibly do anything useful >> with more than one usb_charger. > > Baolin, I think adding a DT binding and lookup mechanism makes sense > here - do you agree? We already have a lookup mechanism for a battery charger to find the phy that it gets current from: devm_usb_get_phy_by_phandle() (or even devm_usb_get_phy() if there is known to only be one phy). We would need a case to be made that the existing mechanism cannot be used before we consider "adding a DT binding and lookup mechanism". Thanks, NeilBrown signature.asc Description: PGP signature
