RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver
Peter Chen writes: >> >> >> +irqreturn_t ret = IRQ_NONE; >> >> >> +unsigned long flags; >> >> >> +u32 reg; >> >> >> + >> >> >> +priv_dev = cdns->gadget_dev; >> >> >> +spin_lock_irqsave(&priv_dev->lock, flags); >> >> > >> >> >you're already running in hardirq context. Why do you need this lock >> >> >at all? I would be better to use the hardirq handler to mask your >> >> >interrupts, so they don't fire again, then used the top-half >> >> >(softirq) handler to actually handle the interrupts. >> >> >> > >> > This controller may be ran at SMP environment, register and flag >> > access needs to be protected among CPUs running. >> >> in hardirq context? When interrupts are already disabled? > > Interrupt handler (hardirq context) at CPU0, and process at CPU1, eg > role switch, unload module, etc. the process at CPU1 would need to disable interrupts (spin_lock_irq() or spin_lock_irqsave()), not the hardirq on CPU0 as that already runs with interrrupts disabled. https://www.kernel.org/doc/html/latest/kernel-hacking/locking.html#table-of-minimum-requirements -- balbi signature.asc Description: PGP signature
RE: RESEND: About VBUS glitch happen on DWC3 host mode enabling process.
Hi Felipe & Mathias, It seems that my below reply mail failed to be shown on lkml.org. So I have to resend it. Like to have your comment/suggestion before I begin the solution implement work. Thanks Ran Wang wrote: > > Hello Felipe, > > Felipe Balbi wrote: > > > > Hi Ran, > > > > Ran Wang writes: > > >> > Then, DWC3 core driver continued to call function > > >> > dwc3_host_init()->platform_device_add(xhci)… > > >> > xhci_plat_probe()->usb_add_hcd()->xhci_plat_setup()->xhci_gen_set > > >> > up > > >> > -> xhci_reset(), which would reset xHCI controller. At this > > >> > -> point, > > >> > the VBUS EN pin (USB_DRVVBUS) was pulled low for about 15us, > > >> > causing the > > >> > > >> why is that pin pulled low? XHCI reset shouldn't be a global reset. > > >> Did your HW engineer tie all reset lines together? If so, there's > > >> nothing I can do to help. > > > > > > That's the point I also want to make clear: do you mean that the > > > VBUS control signal come from DWC3 IP should not be pulled down when > > > xHCI controller conduct reset? > > > And sorry that I am not quite sure about the 'global reset' you > > > mentioned. Do you mean to a DWC3 global reset or SoC reset? > > > > > > My understanding is that since VBUS control signal only be > > > meaningful in USB host mode (xHCI), so it might be in the > > > scope/control of xHCI controller, meaning that xHCI reset trigger > > > VBUS/USB_DRVVBUS(EN) pulled low might make sense, am I right? And > > > the information come from > > > DWC3 IP design has confirmed that PORTSC[PP] will be de-asserted > > > during HCRST, it seems this is native behavior on > > > DWC3 IP. > > > > okay, so the thing is about PP being dropped. Right, that should > > happen indeed. However, this still shouldn't cause any problems, since > > peripheral side shouldn't connect its pull-ups until VBUS is above > > session valid threshold. > > > > For how long is VBUS dropped in this case? > > The duration of VBUS drop is about 7.5ms (for USB_DRVVBUS is about 22us) I > have to admitted only that 2 brands of USB drives encountered failures, > others are fine, according my test results. Just thinking that this glitch > properly trigger those potential defect of that USB drives on the market > which might not totally follow USB spec, so like to do something in SW side > to make host more robust. > > > > >> > VBUS did the same drop too, then back to normal voltage when HW > > >> > reset complete. We have confirmed this whole process according to > > >> > scope waveform with test code on DWC3 driver. Impact is that VBUS > > >> > glitch has let some USB drives (such as Transcend 4GB USB2.0 > > >> > (jetflash) and Kingston 16GB USB2.0 DTGE9) malfunction during > > >> > enumeration (particularly happen when drive is connected to > > >> > root-hub port prior to Linux boot). > > >> > > >> okay > > >> > > >> > Per my understanding, VBUS need to keep +5V once enabled without > > >> > any drop/unstable. And above glitch looks like caused by the gap > > >> > between > > >> > DWC3 design and driver init procedure. > > >> > > >> why are you blaming the driver here? We don't know of any such > > >> platform that has problems with this. Do you mean to say that > > >> because your HW engineer made a choice of tying host reset to > > >> global reset, you end up having an issue? That's something else > > >> entirely that SW can't > > help you with. > > > > > > I didn't mean to blame driver alone, just found the time interval > > > between host mode enabling and host reset causing a observable VBUS > > > control signal glitch happen we didn't expected. And experiments > > > proved that VBUS on between host mode enabling and host reset might > > not be necessary and can avoid this potential risk. > > > > > >> I have no idea about anything nxp has done, no access to > > >> documentation, nothing at all. I need you to do a better job at > > >> explaining the situation starting with kernel version you're using, > > >> if platform is supported upstream, etc. > > > > > > Please see my above answer. > > > These Layerscape platforms are support upstream, I can run them with > > > pure upstream build directly. > > > > that's good, then we can debug this. Can you collect xhci tracepoints > > of when the problem happens? > > Sorry, did you mean open xhci dynamic printk support for xhci? > > Actually I have debugged this for a while, the enumeration failure is due to > that USB drive reported another USB device descriptor once encounter VBUS > glitch. It's interesting. Look like it suddenly become another USB drive and > finally fail at SCSI protocol communication (TEST UNIT READY feedback), I > attach the snapshot pic of USB trace log to this mail, not sure if you can > receive it. > > My judgement on this is that USB drive might has multiple device config > information stored in EPROM and report the wrong one in some corner case > (like encounter VBUS glitch) by accidently. And obviously that chosen d
RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver
> > > > Interrupt handler (hardirq context) at CPU0, and process at CPU1, eg > > role switch, unload module, etc. > > the process at CPU1 would need to disable interrupts (spin_lock_irq() or > spin_lock_irqsave()), not the hardirq on CPU0 as that already runs with > interrrupts > disabled. > > https://www.kernel.org/doc/html/latest/kernel-hacking/locking.html#table-of- > minimum-requirements > I see, thanks. Peter
RE: [PATCH] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
On 12 December 2018 02:47, Kyle Tso wrote: > On Mon, Dec 10, 2018 at 7:36 PM Adam Thomson > wrote: > > > > On 10 December 2018 09:01, Adam Thomson wrote: > > > > > On 06 December 2018 03:02, Kyle Tso wrote: > > > > > > > Current matching rules ensure that the voltage range of selected > > > > Source Capability is entirely within the range defined in one of > > > > the Sink Capabilities. This is reasonable but not practical > > > > because Sink may not support wide range of voltage when sinking > > > > power while Source could advertise its capabilities in raletively > > > > wider range. For example, a Source PDO advertising 3.3V-11V@3A (9V > > > > Prog of Fixed Nominal Voltage) will not be selected if the Sink > > > > requires 5V- 12V@3A PPS power. However, the Sink could work well > > > > if the requested voltage range in > > > RDOs is 5V-11V@3A. > > > > > > Is there a real world example of a sink requiring the 5V - 12V > > > range? In that scenario could we not add an additional sink > > > capability which allows for this range to be supported, and the current > implementation should work just fine? > > > > Ok, I maybe should have waited until after my morning coffee to > > respond. So because the lower limit on the sink side, is higher than > > the advertised source's PPS minimum voltage it never gets selected? > > Personally I'd prefer to keep the upper limit checking as is as I > > think that's an additional safety benefit helping to prevent > > over-voltage scenarios. I think if a PPS APDO can supply up to 11V > > then the system should be capable of handling that voltage, otherwise > > it shouldn't be considered at all. The Source provides limits checking > > as well to make sure the Sink doesn't request a value above the maximum > voltage limit for that selected APDO. > > > > If the over-voltage occurs, it means: > 1. the adapter malfunctioned. or > 2. the code on the Sink accidentally requests a voltage level which is over > the limit > of the Sink. > > For 1., it is difficult to predict the behaviors of a malfunctioned adapter. > The over- > voltage event may happen even if the Sink doesn't select the APDO from this > broken adapter. Yes, I agree it's almost impossible to do anything from software to mitigate this which is why the HW design has to have protection for this. > For 2., it is difficult to predict the behaviors from the careless code as > well. Yes, that's also true, but if it's coded with the intention not to select an option that's potentially higher than the system can handle then we're less likely to fall foul of over-voltage scenarios in my opinion. By selecting a PPS APDO with an upper threshold which falls within the board limits, assuming the code were to accidentally request something higher than the PPS APDO maximum then the Source should reject this. Just feels a little safer as we're talking about controlling an external power source. At the end of the day though the decision lies with the maintainers on this. > > For the lower limit I'm more inclined to agree with allowing a higher > > minimum on the sink side as that's less of a safety/damage issue as I > understand it. > > FWIW, what is the real world scenario? What happens if voltage drops below > 5V? > > > > Some products (in Sink mode) have under-voltage protection (the lower bound > might be around 3.8V - 4V before the calculation of IR-drop) that will cause > the > disconnection. Ok, so the system would just stop charging, correct? I guess the calling code to control the Source/adapter, via TCPM, wouldn't select a value below 4V in that scenario anyway? > > thanks, > Kyle
Re: [PATCH 1/6] driver core: Introduce device_iommu_mapped() function
On Tue, Dec 11, 2018 at 02:43:38PM +0100, Joerg Roedel wrote: > From: Joerg Roedel > > Some places in the kernel check the iommu_group pointer in > 'struct device' in order to find ot whether a device is > mapped by an IOMMU. > > This is not good way to make this check, as the pointer will > be moved to 'struct dev_iommu_data'. This way to make the > check is also not very readable. > > Introduce an explicit function to perform this check. > > Cc: Greg Kroah-Hartman > Acked-by: Greg Kroah-Hartman No need to have a cc: line if I have already acked it :) thanks, greg k-h
Re: [PATCH 1/6] driver core: Introduce device_iommu_mapped() function
On Wed, Dec 12, 2018 at 12:04:35PM +0100, Greg Kroah-Hartman wrote: > On Tue, Dec 11, 2018 at 02:43:38PM +0100, Joerg Roedel wrote: > > Cc: Greg Kroah-Hartman > > Acked-by: Greg Kroah-Hartman > > No need to have a cc: line if I have already acked it :) Right, I'll remove it, sorry for the noise. Regards, Joerg
[GIT PULL] USB changes for v4.21
Hi Greg, here's my pull request for the next merge window. Patches have been around soaking for quite a while and I've tested them on platforms I have available. Let me know if you want anything to be changed. Happy holidays The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436: Linux 4.20-rc4 (2018-11-25 14:19:31 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git usb-for-v4.21 for you to fetch changes up to 4fe4f9fecc36956fd53c8edf96dd0c691ef98ff9: usb: dwc2: Fix disable all EP's on disconnect (2018-12-11 15:42:39 +0200) USB changes for v4.21 So it looks like folks are interested in dwc3 again. Almost 64% of the changes are in dwc3 this time around with some other bits in gadget functions and dwc2. There are two important parts here: a. removal of the waitqueue from dwc3's dequeue implementation, which will guarantee that gadget functions can dequeue from any context and; b. better method for starting isochronous transfers to avoid, as much as possible, missed isoc frames. Apart from these, we have the usual set of non-critical fixes and new features all over the place. Andrzej Pietrasiewicz (1): usb: gadget: f_fs: Allow scatter-gather buffers Andy Shevchenko (3): usb: dwc3: drd: Switch to device property for 'extcon' handling usb: dwc3: drd: Add support for DR detection through extcon usb: dwc3: trace: add missing break statement to make compiler happy Anurag Kumar Vulisha (3): usb: dwc3: update stream id in depcmd usb: dwc3: don't issue no-op trb for stream capable endpoints usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb() Artur Petrosyan (2): usb: dwc2: gadget: Fix WkupAlert interrupt handler. usb: dwc2: gadget: Accept LPM token when TxFIFO is not empty Brian Norris (1): usb: dwc3: don't log probe deferrals; but do log other error codes Chunfeng Yun (5): usb: mtu3: remove QMU checksum usb: mtu3: enable hardware remote wakeup from L1 automatically usb: mtu3: fix the issue about SetFeature(U1/U2_Enable) usb: mtu3: enable SETUPENDISR interrupt usb: mtu3: clear SOFTCONN when clear USB3_EN if work as HS mode Colin Ian King (1): usb: gadget: udc: fix spelling mistake "intrerrupt" -> "interrupt" Felipe Balbi (11): usb: dwc3: gadget: combine unaligned and zero flags usb: dwc3: gadget: track number of TRBs per request usb: dwc3: gadget: use num_trbs when skipping TRBs on ->dequeue() usb: dwc3: gadget: extract dwc3_gadget_ep_skip_trbs() usb: dwc3: gadget: introduce cancelled_list usb: dwc3: gadget: move requests to cancelled_list usb: dwc3: gadget: remove wait_end_transfer usb: dwc3: gadget: return errors from __dwc3_gadget_start_isoc() usb: dwc3: trace: log ep commands in hex usb: dwc3: gadget: remove unnecessary dev_info() usb: dwc3: gadget: check if dep->frame_number is still valid Hsin-Yi, Wang (1): usb/mtu3: power down device ip at setup Jarkko Nikula (1): usb: renesas_usbhs: Remove dummy runtime PM callbacks Julia Lawall (2): usb: gadget: aspeed-vhub: constify usb_gadget_ops structure usb: gadget: uvc: constify vb2_ops structure Marek Szyprowski (1): usb: dwc2: Disable power down feature on Samsung SoCs Martin Blumenstingl (1): usb: dwc2: disable power_down on Amlogic devices Minas Harutyunyan (1): usb: dwc2: Fix disable all EP's on disconnect Stephan Gerhold (1): Revert "usb: dwc3: pci: Use devm functions to get the phy GPIOs" Tejas Joglekar (1): usb: dwc3: gadget: Disable CSP for stream OUT ep Terin Stock (1): usb: dwc2: host: use hrtimer for NAK retries Thinh Nguyen (14): usb: dwc3: debugfs: Properly name Tx/RxFIFO usb: dwc3: debugfs: Print eps Tx/RxFIFO in bytes usb: dwc3: debugfs: Dump internal LSP and ep registers usb: dwc3: debugfs: Properly print/set link state for HS usb: dwc3: debugfs: Print/set link state for peripheral mode usb: dwc3: Set GUSB2PHYCFG.ENBLSLPM usb: dwc3: Add a property to disable USB2 LPM usb: dwc3: Support option to disable USB2 LPM usb: dwc3: Set default mode for DWC_usb3 v3.30a and higher usb: dwc3: Track DWC_usb31 VERSIONTYPE usb: dwc3: Add disabling of start_transfer failure quirk usb: dwc3: Add workaround for isoc start transfer failure usb: gadget: Introduce frame_number to usb_request usb: dwc3: gadget: Report isoc transfer frame number Vincent Pelletier (1): usb: gadget: f_fs: Add support for CCID descriptors. Yangtao Li (1): USB: gadget: udc: s3c2410_udc: convert to DEFINE_SHOW_ATTRIBUTE Yoshihiro Shimoda (2): usb: gadget: udc: renesas_usb3: add a safety connection way for fo
Re: [PATCH v3 1/2] USB: quirks: Check device interface LPM capability
On Fri, Dec 07, 2018 at 08:47:21PM -0500, Kyle Williams wrote: > From: Kyle Williams > > Description: enable the ability to disable LPM for all devices matched > by interface information Why have "Description:" in here? Of course this is the description :) And the subject doesn't make much sense, what does this have to do with quirks? Also, you need more information here in the description about why you are doing all of this, as it is, I have no idea why this patch is needed at all. thanks, greg k-h
Re: [PATCH v3 2/2] USB: quirks: Disable LPM for Logitech UVC devices
On Fri, Dec 07, 2018 at 08:47:22PM -0500, Kyle Williams wrote: > From: Kyle Williams > > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of certain Logitech uvc devices and other powered media > devices such causes 'not enough bandwidth for new device state' error. Same thing with "Description:" :(
Re: [PATCH v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
On Mon, Dec 10, 2018 at 01:50:20PM +0100, Sebastian Andrzej Siewior wrote: > On 2018-12-10 09:04:43 [+0100], Greg KH wrote: > > From: Hui Peng > > > > The function hso_probe reads if_num from the USB device (as an u8) and uses > > it without a length check to index an array, resulting in an OOB memory read > > in hso_probe or hso_get_config_data. Added a length check for both locations > > and updated hso_probe to bail on error. > > The function hso_probe reads if_num from the USB device (as an u8) and uses > it without a length check to index an array, resulting in an OOB memory read > in hso_probe or hso_get_config_data. > > Add a length check for both locations and update hso_probe to bail on > error. > > > This issue has been assigned CVE-2018-19985. > > > > Reported-by: Hui Peng > > Reported-by: Mathias Payer > > Signed-off-by: Hui Peng > > Signed-off-by: Mathias Payer > > Signed-off-by: Greg Kroah-Hartman > > Reviewed-by: Sebastian Andrzej Siewior Thanks, will update the changelog and add resend. greg k-h
[PATCH v3] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
From: Hui Peng
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data.
Add a length check for both locations and updated hso_probe to bail on
error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng
Reported-by: Mathias Payer
Signed-off-by: Hui Peng
Signed-off-by: Mathias Payer
Reviewed-by: Sebastian Andrzej Siewior
Signed-off-by: Greg Kroah-Hartman
---
v3: redid the changelog text based on review comments from Sebastian
v2: fixed error check to just be < 0
Added CVE to changelog text
drivers/net/usb/hso.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface
*interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0
terminated
+* array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
--
2.20.0
[PATCH] usb: dwc2: Reset device address on EnumDone
Initially resetting device address was done in USB RESET interrupt handler. In case, when power saving mode enabled (hibernation) USB RESET interrupt handled in dwc2_handle_gpwrdn_intr() and then it not seen in dwc2_hsotg_irq() handler. This is why reset device address to zero moved from USB RESET handler to EnumDone handler. Signed-off-by: Minas Harutyunyan --- drivers/usb/dwc2/gadget.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 68ad75a7460d..7f922f19f8e1 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -3072,6 +3072,9 @@ static void dwc2_hsotg_irq_enumdone(struct dwc2_hsotg *hsotg) dev_dbg(hsotg->dev, "EnumDone (DSTS=0x%08x)\n", dsts); + /* Reset device address to zero */ + dwc2_clear_bit(hsotg, DCFG, DCFG_DEVADDR_MASK); + /* * note, since we're limited by the size of transfer on EP0, and * it seems IN transfers must be a even number of packets we do @@ -3614,9 +3617,6 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw) /* Report disconnection if it is not already done. */ dwc2_hsotg_disconnect(hsotg); - /* Reset device address to zero */ - dwc2_clear_bit(hsotg, DCFG, DCFG_DEVADDR_MASK); - if (usb_status & GOTGCTL_BSESVLD && connected) dwc2_hsotg_core_init_disconnected(hsotg, true); } -- 2.11.0
Re: [GIT PULL] USB changes for v4.21
On Wed, Dec 12, 2018 at 01:22:58PM +0200, Felipe Balbi wrote: > > Hi Greg, > > here's my pull request for the next merge window. Patches have been > around soaking for quite a while and I've tested them on platforms I > have available. > > Let me know if you want anything to be changed. > > Happy holidays > > The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436: > > Linux 4.20-rc4 (2018-11-25 14:19:31 -0800) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git usb-for-v4.21 Pulled and pushed out, thanks. greg k-h
[PATCH] usb: dwc2: gadget: Fix Remote Wakeup interrupt bit clearing
To clear GINTSTS2_WKUP_ALERT_INT bit in GINTSTS2 register
require to write 1. This bit is implemented as "Write to clear".
Fixes: 187c5298a122 ("usb: dwc2: gadget: Add handler for WkupAlert
interrupt")
Signed-off-by: Minas Harutyunyan
---
drivers/usb/dwc2/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 68ad75a7460d..55ef3cc2701b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -261,7 +261,7 @@ static void dwc2_gadget_wkup_alert_handler(struct
dwc2_hsotg *hsotg)
if (gintsts2 & GINTSTS2_WKUP_ALERT_INT) {
dev_dbg(hsotg->dev, "%s: Wkup_Alert_Int\n", __func__);
- dwc2_clear_bit(hsotg, GINTSTS2, GINTSTS2_WKUP_ALERT_INT);
+ dwc2_set_bit(hsotg, GINTSTS2, GINTSTS2_WKUP_ALERT_INT);
dwc2_set_bit(hsotg, DCTL, DCTL_RMTWKUPSIG);
}
}
--
2.11.0
Re: [PATCH] usb: dwc2: gadget: Fix Remote Wakeup interrupt bit clearing
Hi Greg, Filipe,
On 12/12/2018 4:44 PM, Minas Harutyunyan wrote:
> To clear GINTSTS2_WKUP_ALERT_INT bit in GINTSTS2 register
> require to write 1. This bit is implemented as "Write to clear".
>
> Fixes: 187c5298a122 ("usb: dwc2: gadget: Add handler for WkupAlert
> interrupt")
>
> Signed-off-by: Minas Harutyunyan
> ---
> drivers/usb/dwc2/gadget.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 68ad75a7460d..55ef3cc2701b 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -261,7 +261,7 @@ static void dwc2_gadget_wkup_alert_handler(struct
> dwc2_hsotg *hsotg)
>
> if (gintsts2 & GINTSTS2_WKUP_ALERT_INT) {
> dev_dbg(hsotg->dev, "%s: Wkup_Alert_Int\n", __func__);
> - dwc2_clear_bit(hsotg, GINTSTS2, GINTSTS2_WKUP_ALERT_INT);
> + dwc2_set_bit(hsotg, GINTSTS2, GINTSTS2_WKUP_ALERT_INT);
> dwc2_set_bit(hsotg, DCTL, DCTL_RMTWKUPSIG);
> }
> }
>
Sorry for last minute fix of this stupid bug.
Please get this commit together with "usb: dwc2: gadget: Fix WkupAlert
interrupt handler."
Thanks,
Minas
[PATCH net 2/2] net: usb: aqc111: Support for thermal throttling feature
From: Dmitry Bezrukov
Lab testing shows that chip may get overheated when
network link is connected on high speed (2.5G/5G).
Default hardware design uses only passive heatsink without a cooler,
and that makes things worse.
To prevent possible chip damage here we add throttling mechanism.
There is a worker that monitors the PHY's temperature via reading
PHY registers. When PHY's temperature reaches the critical threshold
(it is 106 degrees of Celsius) it changes the link speed to the lowest
regardless user/default link speed settings. It should reduce the PHY's
temperature to normal numbers.
When the PHY's temparature is back to normal (low threshold, it is
85 degrees) it restores user/default link speed settings.
Signed-off-by: Dmitry Bezrukov
Signed-off-by: Igor Russkikh
---
drivers/net/usb/aqc111.c | 78
drivers/net/usb/aqc111.h | 8 +
2 files changed, 86 insertions(+)
diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index 8867f6a3eaa7..fa6b9dfc2a6f 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "aqc111.h"
@@ -334,6 +335,9 @@ static void aqc111_set_phy_speed(struct usbnet *dev, u8
autoneg, u16 speed)
aqc111_data->phy_cfg |= (3 << AQ_DSH_RETRIES_SHIFT) &
AQ_DSH_RETRIES_MASK;
+ if (aqc111_data->thermal_throttling)
+ speed = SPEED_100;
+
if (autoneg == AUTONEG_ENABLE) {
switch (speed) {
case SPEED_5000:
@@ -714,6 +718,8 @@ static int aqc111_bind(struct usbnet *dev, struct
usb_interface *intf)
/* store aqc111_data pointer in device data field */
dev->driver_priv = aqc111_data;
+ aqc111_data->dev = dev;
+
/* Init the MAC address */
ret = aqc111_read_perm_mac(dev);
if (ret)
@@ -991,6 +997,71 @@ static int aqc111_link_reset(struct usbnet *dev)
return 0;
}
+int aqc111_get_temperature(struct usbnet *dev, u32 *temperature)
+{
+ u16 reg16 = 0;
+
+ if (aqc111_mdio_read(dev, AQ_GLB_THERMAL_STATUS2, AQ_PHY_GLOBAL_ADDR,
+®16) < 0)
+ goto err;
+
+ if (!(reg16 & AQ_THERM_ST_READY))
+ goto err;
+
+ if (aqc111_mdio_read(dev, AQ_GLB_THERMAL_STATUS1, AQ_PHY_GLOBAL_ADDR,
+®16) < 0)
+ goto err;
+
+ /*convert from 1/256 to 1/100 degrees of Celsius*/
+ *temperature = (u32)reg16 * 100 / 256;
+ return 0;
+
+err:
+ *temperature = 0;
+ return -1;
+}
+
+void aqc111_thermal_work_cb(struct work_struct *w)
+{
+ struct delayed_work *dw = to_delayed_work(w);
+ struct aqc111_data *aqc111_data = container_of(dw, struct aqc111_data,
+ therm_work);
+ unsigned long timeout = msecs_to_jiffies(AQ_THERMAL_TIMER_MS);
+ struct usbnet *dev = aqc111_data->dev;
+ u32 temperature = 0;
+ u8 reset_speed = 0;
+
+ if (!aqc111_data->link)
+ /* poll not so frequently */
+ timeout *= 2;
+
+ if (aqc111_get_temperature(dev, &temperature) != 0)
+ goto end;
+
+ if (aqc111_data->thermal_throttling &&
+ temperature <= AQ_NORMAL_TEMP_THRESHOLD) {
+ netdev_info(dev->net, "The temperature is back to normal(%u)",
+ AQ_NORMAL_TEMP_THRESHOLD / 100);
+ aqc111_data->thermal_throttling = 0;
+ reset_speed = 1;
+ }
+
+ if (!aqc111_data->thermal_throttling &&
+ temperature >= AQ_CRITICAL_TEMP_THRESHOLD) {
+ netdev_warn(dev->net, "Critical temperature(%u) is reached",
+ AQ_CRITICAL_TEMP_THRESHOLD / 100);
+ aqc111_data->thermal_throttling = 1;
+ reset_speed = 1;
+ }
+
+ if (reset_speed)
+ aqc111_set_phy_speed(dev, aqc111_data->autoneg,
+aqc111_data->advertised_speed);
+
+end:
+ schedule_delayed_work(&aqc111_data->therm_work, timeout);
+}
+
static int aqc111_reset(struct usbnet *dev)
{
struct aqc111_data *aqc111_data = dev->driver_priv;
@@ -1032,6 +1103,10 @@ static int aqc111_reset(struct usbnet *dev)
aqc111_set_phy_speed(dev, aqc111_data->autoneg,
aqc111_data->advertised_speed);
+ INIT_DELAYED_WORK(&aqc111_data->therm_work, &aqc111_thermal_work_cb);
+ schedule_delayed_work(&aqc111_data->therm_work,
+ msecs_to_jiffies(AQ_THERMAL_TIMER_MS));
+
return 0;
}
@@ -1040,6 +1115,9 @@ static int aqc111_stop(struct usbnet *dev)
struct aqc111_data *aqc111_data = dev->driver_priv;
u16 reg16 = 0;
+ cancel_delayed_work_sync(&aqc111_data->therm_work);
+ aqc111_data->thermal_throttling = 0;
+
aqc111_read1
[PATCH net 0/2] aqc111: Thermal throttling feature
This patches introduce the thermal throttling feature to prevent possible heat damage to the hardware. Dmitry Bezrukov (2): net: usb: aqc111: Add read_mdio operation net: usb: aqc111: Support for thermal throttling feature drivers/net/usb/aqc111.c | 83 drivers/net/usb/aqc111.h | 19 + 2 files changed, 102 insertions(+) -- 2.17.1
[PATCH net 1/2] net: usb: aqc111: Add read_mdio operation
From: Dmitry Bezrukov
Add necessary defines to read phy registers and temparature
Signed-off-by: Dmitry Bezrukov
Signed-off-by: Igor Russkikh
---
drivers/net/usb/aqc111.c | 5 +
drivers/net/usb/aqc111.h | 11 +++
2 files changed, 16 insertions(+)
diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index 57f1c94fca0b..8867f6a3eaa7 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -193,6 +193,11 @@ static int aqc111_write16_cmd_async(struct usbnet *dev, u8
cmd, u16 value,
sizeof(tmp), &tmp);
}
+static int aqc111_mdio_read(struct usbnet *dev, u16 value, u16 index, u16
*data)
+{
+ return aqc111_read16_cmd(dev, AQ_PHY_CMD, value, index, data);
+}
+
static void aqc111_get_drvinfo(struct net_device *net,
struct ethtool_drvinfo *info)
{
diff --git a/drivers/net/usb/aqc111.h b/drivers/net/usb/aqc111.h
index 4d68b3a6067c..359663635b49 100644
--- a/drivers/net/usb/aqc111.h
+++ b/drivers/net/usb/aqc111.h
@@ -18,9 +18,20 @@
#define AQ_ACCESS_MAC 0x01
#define AQ_FLASH_PARAMETERS0x20
#define AQ_PHY_POWER 0x31
+#define AQ_PHY_CMD 0x32
#define AQ_WOL_CFG 0x60
#define AQ_PHY_OPS 0x61
+#define AQC111_PHY_ID 0x00
+#define AQ_PHY_ADDR(mmd) ((AQC111_PHY_ID << 8) | (mmd))
+
+#define AQ_PHY_GLOBAL_MMD 0x1E
+#define AQ_PHY_GLOBAL_ADDR AQ_PHY_ADDR(AQ_PHY_GLOBAL_MMD)
+
+#define AQ_GLB_THERMAL_STATUS1 0xC820
+#define AQ_GLB_THERMAL_STATUS2 0xC821
+ #define AQ_THERM_ST_READY 0x0001
+
#define AQ_USB_PHY_SET_TIMEOUT 1
#define AQ_USB_SET_TIMEOUT 4000
--
2.17.1
RE: [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb requests
Hi Felipe, >-Original Message- >From: Alan Stern [mailto:st...@rowland.harvard.edu] >Sent: Friday, December 07, 2018 10:40 PM >To: Felipe Balbi >Cc: Anurag Kumar Vulisha ; Greg Kroah-Hartman >; Shuah Khan ; Johan Hovold >; Jaejoong Kim ; Benjamin >Herrenschmidt ; Roger Quadros ; Manu >Gautam ; martin.peter...@oracle.com; Bart Van >Assche ; Mike Christie ; Matthew >Wilcox ; Colin Ian King ; linux- >u...@vger.kernel.org; linux-ker...@vger.kernel.org; v.anuragku...@gmail.com; >Thinh Nguyen ; Tejas Joglekar >; Ajay Yugalkishore Pandey >Subject: RE: [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb >requests > >On Fri, 7 Dec 2018, Felipe Balbi wrote: > >> >> hi, >> >> Anurag Kumar Vulisha writes: >> >>Does the data book suggest a value for the timeout? >> >> >> > >> > No, the databook doesn't mention about the timeout value >> > >> >>> >At this point, it seems that the generic approach will be messier than >> >>> >having >every >> >>> >controller driver implement its own fix. At least, that's how it >> >>> >appears to me. >> >> Why, if the UDC implementation will, anyway, be a timer? > >It's mostly a question of what happens when the timer expires. (After >all, starting a timer is just as easy to do in a UDC driver as it is in >the core.) When the timer expires, what can the core do? > >Don't say it can cancel the offending request and resubmit it. That >leads to the sort of trouble we discussed earlier in this thread. In >particular, we don't want the class driver's completion routine to be >called when the cancel occurs. Furthermore, this leads to a race: >Suppose the class driver decides to cancel the request at the same time >as the core does a cancel and resubmit. Then the class driver's cancel >could get lost and the request would remain on the UDC's queue. > >What you really want to do is issue the appropriate stop and restart >commands to the hardware, while leaving the request logically active >the entire time. The UDC core can't do this, but a UDC driver can. > I agree with Alan's comment as it looks like there may be some corner cases where the issue may occur with dequeue approach. Are you okay if the timeout handler gets moved to the dwc3 driver (the timers still added into udc layer)? Please let us know your suggestion on this Thanks, Anurag Kumar Vulisha >> >>(Especially if dwc3 is the only driver affected.) >> > >> > As discussed above, the issue may happen with other gadgets too. As I got >> > divide >opinions >> > on this implementation and both the implementations looks fine to me, I am >> > little >confused >> > on which should be implemented. >> > >> > @Felipe: Do you agree with Alan's implementation? Please let us know your >suggestion >> > on this. >> >> I still think a generic timer is a better solution since it has other uses. > >Putting a struct timer into struct usb_request is okay with me, but I >wouldn't go any farther than that. > >> >>Since the purpose of the timeout is to detect a deadlock caused by a >> >>hardware bug, I suggest a fixed and relatively short timeout value such >> >>as one second. Cancelling and requeuing a few requests at 1-second >> >>intervals shouldn't add very much overhead. >> >> I wouldn't call this a HW bug though. This is just how the UDC >> behaves. There are N streams and host can move data in any stream at any >> time. This means that host & gadget _can_ disagree on what stream to >> start next. > >But the USB 3 spec says what should happen when the host and gadget >disagree in this way, doesn't it? And it doesn't say that they should >deadlock. :-) Or have I misread the spec? > >> One way to avoid this would be to never pre-start any streams and always >> rely on XferNotReady, but that would mean greatly reduced throughput for >> streams. > >It would be good if there was some way to actively detect the problem >instead of passively waiting for a timer to expire. > >Alan Stern
Re: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver
Peter, On Wed, Dec 12, 2018 at 10:04:25AM +0800, Peter Chen wrote: [snip] > > >I strongly advise against using dev_dbg() for debugging. Even more so > > >inside your IRQ handler. > > Felipe, I use Dynamic Debug for debugging, and show debug messages with > "dmesg" after testing/debugging. I see dwc3 using trace, any benefits > for switching > to trace? The benefits I see are - *by default*, the debug log doesn't have to go through uart console, which is slow. I typically 'cat trace_pipe' from a telnet which is much faster then uart. But Dynamic Debug log by default got printed on uart, I have to set printk level to not print them, which is an extra step. - tracepoint uses one place to decode the message vs DD has to repeat the similar print statement in the code; and tracepoint decodes it offline which reduce the kernel runtime overhead too. - with tracepoint, it is easier to turn on debug for a specific group of messages. - you can adjust the ftrace buffer size at runtime. can we do that for printk? I don't remember. Regards, -Bin.
Re: [PATCH] USB: serial: option: add Fibocom NL668 series
On Tue, Dec 11, 2018 at 02:22:50PM +0100, Jörgen Storvist wrote:
> Den Tue, 11 Dec 2018 09:32:36 +0100
> skrev Re: [PATCH] USB: serial: option: add Fibocom NL668 series:
>
> > On Tue, Dec 11, 2018 at 08:41:24AM +0100, Jörgen Storvist wrote:
> > >
> > > Added USB serial option driver support for Fibocom NL668 series cellular
> > > modules.
> > > Reserved USB endpoints 4, 5 and 6 for network + ADB interface.
> > >
> > > Signed-off-by: Jörgen Storvist
> > > ---
> > >
> > > Thanks for feedback!
> > > Changes:
> > > Removed name declarations for VID/PID
> > > Added reserved endpoint for ADB interface
> > >
> > > usb-devices
> > > T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 7 Spd=480 MxCh= 0
> > > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
> > > P: Vendor=1508 ProdID=1001 Rev=03.18
> > > S: Manufacturer=Nodecom NL668 Modem
> > > S: Product=Nodecom NL668-CN Modem
> > > S: SerialNumber=5ced6a52
> > > C: #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
> > > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > > I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > > I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > > I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > > I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether
> > > I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
> > > I: If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)
> >
> > Thanks for the update and usb-devices info (you can even put this in the
> > changelog).
> >
> > > 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 e24ff16..ca3e398 100644
> > > --- a/drivers/usb/serial/option.c
> > > +++ b/drivers/usb/serial/option.c
> > > @@ -1941,6 +1941,8 @@ static const struct usb_device_id option_ids[] = {
> > > { 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(0x1508, 0x1001), /* Fibocom NL668 series */
> > > + .driver_info = RSVD(4) | RSVD(5) | RSVD(6) },
> >
> > Looks like you can use USB_DEVICE_INTERFACE_CLASS() to match on the
> > vendor class instead of blacklisting interface 4 and 5.
> >
> > A more specific match is generally preferred over blacklisting (which
> > adds some overhead).
> >
> > Care to address that in a v3? Remember to include the patch revision in
> > the subject line as well (e.g. "[PATCH v3] USB: ...").
> It seems it would become problematic then if we change USB mode on the
> module to QMI instead of ECM network interface as they share same
> VID/PID value.
>
> Or is there other way to still get endpoints 0-3 bound to option
> driver on class / interface info? Subclass is different on the first
> diagnostical serial interface so USB_DEVICE_AND_INTERFACE_INFO()
> wouldn't be successful then either?
>
> T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 9 Spd=480 MxCh= 0
> D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
> P: Vendor=1508 ProdID=1001 Rev=03.18
> S: Manufacturer=Nodecom NL668 Modem
> S: Product=Nodecom NL668-CN Modem
> S: SerialNumber=5ced6a52
> C: #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
> I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option (Serial)
> I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option (Serial)
> I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option (Serial)
> I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option (Serial)
> I: If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
> (QMI/RMNET)
> I: If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none) (ADB)
And you also have ADB as interface five here.
I guess your original patch matching on all interfaces and
blacklisting interfaces 4, 5 and 6 is the best option. An alternative
could be two entries for ff/ff/ff and ff/00/00, but then you'd still
need to blacklist QMI so not that much better.
But please resubmit and include usb-devices for both messages in the
commit message.
Please also base this on my usb-linus branch which already holds some
new ids that prevents this one from applying.
https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/
Thanks,
Johan
Re: [PATCH] USB: serial: option: add GosunCn ZTE WeLink ME3630
On Tue, Dec 11, 2018 at 06:28:28PM +0100, Jörgen Storvist wrote: > Added USB serial option driver support for GosunCn ZTE WeLink ME3630 > series cellular modules for USB modes ECM/NCM and MBIM. > > Signed-off-by: Jörgen Storvist > --- > > usb-devices output MBIM mode: > T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 10 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=19d2 ProdID=0602 Rev=03.18 > S: Manufacturer=Android > S: Product=Android > S: SerialNumber=b950269c > C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=500mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > I: If#= 3 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim > I: If#= 4 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim > > usb-devices output ECM/NCM mode: > T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 11 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=19d2 ProdID=1476 Rev=03.18 > S: Manufacturer=Android > S: Product=Android > S: SerialNumber=b950269c > C: #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=500mA > I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option > I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > I: If#= 3 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether > I: If#= 4 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether I included these in the commit messages (without the serial numbers), and used the vendor define since we already have it. These looked too out of place with the other ZTE entries otherwise. :) Thanks, Johan
Re: [PATCH net 2/2] net: usb: aqc111: Support for thermal throttling feature
On Wed, Dec 12, 2018 at 01:50:10PM +, Igor Russkikh wrote:
> From: Dmitry Bezrukov
>
> Lab testing shows that chip may get overheated when
> network link is connected on high speed (2.5G/5G).
>
> Default hardware design uses only passive heatsink without a cooler,
> and that makes things worse.
>
> To prevent possible chip damage here we add throttling mechanism.
>
> There is a worker that monitors the PHY's temperature via reading
> PHY registers. When PHY's temperature reaches the critical threshold
> (it is 106 degrees of Celsius) it changes the link speed to the lowest
> regardless user/default link speed settings. It should reduce the PHY's
> temperature to normal numbers.
>
> When the PHY's temparature is back to normal (low threshold, it is
> 85 degrees) it restores user/default link speed settings.
Hi Igor
Please could you also export the temperature using HWMON. The Marvell
PHY drivers are good examples.
I also have a patch for driver/net/phy/aquantia.c which adds HWMON
support to that PHY.
>
> Signed-off-by: Dmitry Bezrukov
> Signed-off-by: Igor Russkikh
> ---
> drivers/net/usb/aqc111.c | 78
> drivers/net/usb/aqc111.h | 8 +
> 2 files changed, 86 insertions(+)
>
> diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
> index 8867f6a3eaa7..fa6b9dfc2a6f 100644
> --- a/drivers/net/usb/aqc111.c
> +++ b/drivers/net/usb/aqc111.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
>
> #include "aqc111.h"
>
> @@ -334,6 +335,9 @@ static void aqc111_set_phy_speed(struct usbnet *dev, u8
> autoneg, u16 speed)
> aqc111_data->phy_cfg |= (3 << AQ_DSH_RETRIES_SHIFT) &
> AQ_DSH_RETRIES_MASK;
>
> + if (aqc111_data->thermal_throttling)
> + speed = SPEED_100;
> +
That is a big jump. Do you need to cool is down quickly, or would
1Gbps be sufficient? I think as a user, i would prefer it ping-pongs
between 5G and 1G, not 5G and 100M.
> if (autoneg == AUTONEG_ENABLE) {
> switch (speed) {
> case SPEED_5000:
It looks like this only works when auto-neg is enabled. If i've fixed
configured it i don't think this works?
> @@ -714,6 +718,8 @@ static int aqc111_bind(struct usbnet *dev, struct
> usb_interface *intf)
> /* store aqc111_data pointer in device data field */
> dev->driver_priv = aqc111_data;
>
> + aqc111_data->dev = dev;
> +
> /* Init the MAC address */
> ret = aqc111_read_perm_mac(dev);
> if (ret)
> @@ -991,6 +997,71 @@ static int aqc111_link_reset(struct usbnet *dev)
> return 0;
> }
>
> +int aqc111_get_temperature(struct usbnet *dev, u32 *temperature)
> +{
> + u16 reg16 = 0;
> +
> + if (aqc111_mdio_read(dev, AQ_GLB_THERMAL_STATUS2, AQ_PHY_GLOBAL_ADDR,
> + ®16) < 0)
> + goto err;
> +
> + if (!(reg16 & AQ_THERM_ST_READY))
> + goto err;
> +
> + if (aqc111_mdio_read(dev, AQ_GLB_THERMAL_STATUS1, AQ_PHY_GLOBAL_ADDR,
> + ®16) < 0)
> + goto err;
> +
> + /*convert from 1/256 to 1/100 degrees of Celsius*/
> + *temperature = (u32)reg16 * 100 / 256;
> + return 0;
> +
> +err:
> + *temperature = 0;
> + return -1;
> +}
> +
> +void aqc111_thermal_work_cb(struct work_struct *w)
> +{
> + struct delayed_work *dw = to_delayed_work(w);
> + struct aqc111_data *aqc111_data = container_of(dw, struct aqc111_data,
> +therm_work);
> + unsigned long timeout = msecs_to_jiffies(AQ_THERMAL_TIMER_MS);
> + struct usbnet *dev = aqc111_data->dev;
> + u32 temperature = 0;
> + u8 reset_speed = 0;
> +
> + if (!aqc111_data->link)
> + /* poll not so frequently */
> + timeout *= 2;
> +
> + if (aqc111_get_temperature(dev, &temperature) != 0)
> + goto end;
> +
> + if (aqc111_data->thermal_throttling &&
> + temperature <= AQ_NORMAL_TEMP_THRESHOLD) {
> + netdev_info(dev->net, "The temperature is back to normal(%u)",
> + AQ_NORMAL_TEMP_THRESHOLD / 100);
How often do you see these messages? I'm wondering if they need to be
rate limited?
> + aqc111_data->thermal_throttling = 0;
> + reset_speed = 1;
> + }
> +
> + if (!aqc111_data->thermal_throttling &&
> + temperature >= AQ_CRITICAL_TEMP_THRESHOLD) {
Should there be some hysteresis in here? In fact, if temperature is
AQ_CRITICAL_TEMP_THRESHOLD it is both back to normal, and throttled at
the same time!
> + netdev_warn(dev->net, "Critical temperature(%u) is reached",
> + AQ_CRITICAL_TEMP_THRESHOLD / 100);
> + aqc111_data->thermal_throttling = 1;
> + reset_speed = 1;
update_speed might be a better name, since you are not always
resetting it.
Andrew
Re: [PATCH net 1/2] net: usb: aqc111: Add read_mdio operation
On Wed, Dec 12, 2018 at 01:50:08PM +, Igor Russkikh wrote: > From: Dmitry Bezrukov > > Add necessary defines to read phy registers and temparature Hi Igor [Puts tongue in cheek] I thought the firmware was supposed to manage the PHY. So maybe the better fix is to add code to allow firmware upgrade? And issue new firmware to linux-firmware? Andrew
Re: [PATCH] USB: serial: option: add Simcom SIM7500/SIM7600 (MBIM mode)
On Wed, Dec 12, 2018 at 08:39:39AM +0100, Jörgen Storvist wrote: > > Added USB serial option driver support for Simcom SIM7500/SIM7600 series > cellular modules exposing MBIM interface (VID 0x1e0e,PID 0x9003) > > Signed-off-by: Jörgen Storvist Applied, thanks. Johan
usb: CPU 100% hog upon device removal
Hi, I'm using the latest stable tree v4.19.8, and I'm noticing kworker and ksoftirqd hogging up the CPU upon removing any USB devices on my laptop. This issue was not present on any kernels below v4.19. Doing a simple: echo 1 > /sys/devices/pci:00/:00:14.0/usb1/1-6/remove (or any other USB device for that matter) and I'm able to reproduce the issue. Kernel log seems fine, ending with: "[ 464.546444] usb 1-6: USB disconnect, device number 3" top reports: PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND 273 root 20 0 0 0 0 R 81.4 0.0 1:33.15 kworker/4:1+pm 32 root 20 0 0 0 0 S 18.6 0.0 0:22.37 ksoftirqd/4 Doing a sysrq backtrace reports the following: [ 605.575727] NMI backtrace for cpu 4 [ 605.575728] CPU: 4 PID: 273 Comm: kworker/4:1 Tainted: G U 4.19.8-zen+ #1 [ 605.575729] Hardware name: LG Electronics 14ZD980-GX50K/14Z980, BIOS K2ZC0250 X64 03/08/2018 [ 605.575729] Workqueue: pm pm_runtime_work [ 605.575730] RIP: 0010:xhci_hub_control+0x4c2/0x1a80 [ 605.575732] Code: 10 0f 8f 45 fc ff ff 41 ff cc 48 8b 74 24 18 41 0f b7 c4 48 89 44 24 28 48 8d 04 c6 48 89 44 24 40 48 8b 00 48 8b 00 44 8b 38 <41> 83 ff ff 0f 84 f3 04 00 00 0f 1f 44 00 00 48 8b 84 24 80 00 00 [ 605.575732] RSP: 0018:8884279cb9a8 EFLAGS: 0007 [ 605.575733] RAX: c90001cc0520 RBX: 888431246000 RCX: 000b [ 605.575734] RDX: 000b RSI: 8884278a6ae0 RDI: 88843124627c [ 605.575734] RBP: 888431246000 R08: 88842d5c6760 R09: 0004 [ 605.575735] R10: 88842d5c6760 R11: 0004 R12: 000a [ 605.575735] R13: 88842d5c6760 R14: a300 R15: 02a0 [ 605.575736] FS: () GS:88843350() knlGS: [ 605.575736] CS: 0010 DS: ES: CR0: 80050033 [ 605.575737] CR2: 7f0862a03010 CR3: 04608005 CR4: 003606e0 [ 605.575737] Call Trace: [ 605.575738] ? pick_next_task_fair+0x485/0x510 [ 605.575738] ? _cond_resched+0x10/0x20 [ 605.575738] ? __kmalloc+0x16d/0x1a0 [ 605.575739] ? usb_hcd_submit_urb+0x6a5/0xa50 [ 605.575739] ? preempt_schedule_common+0xa/0x20 [ 605.575740] ? _cond_resched+0x18/0x20 [ 605.575740] ? wait_for_common+0x29/0x180 [ 605.575740] ? update_curr+0xa3/0x160 [ 605.575741] ? usb_start_wait_urb+0x50/0xc0 [ 605.575741] ? usb_control_msg+0xbe/0x110 [ 605.575742] ? hub_ext_port_status+0x86/0x130 [ 605.575742] ? hub_activate+0x1c3/0x5e0 [ 605.575742] ? hub_resume+0x1c/0xb0 [ 605.575743] ? hcd_bus_suspend+0x6c/0x130 [ 605.575743] ? usb_resume_interface.isra.2+0x6f/0xb0 [ 605.575744] ? usb_suspend_both+0x122/0x1f0 [ 605.575744] ? usb_probe_interface+0x290/0x290 [ 605.575745] ? usb_runtime_suspend+0x21/0x60 [ 605.575745] ? __rpm_callback+0x70/0x1c0 [ 605.575746] ? usb_probe_interface+0x290/0x290 [ 605.575746] ? rpm_callback+0x1a/0x70 [ 605.575746] ? usb_probe_interface+0x290/0x290 [ 605.575747] ? rpm_suspend+0x120/0x640 [ 605.575747] ? usb_runtime_resume+0x20/0x20 [ 605.575748] ? __pm_runtime_suspend+0x3c/0x70 [ 605.575748] ? usb_runtime_idle+0x26/0x30 [ 605.575748] ? __rpm_callback+0x70/0x1c0 [ 605.575749] ? rpm_idle+0x9f/0x300 [ 605.575749] ? pm_runtime_work+0x6e/0x90 [ 605.575750] ? process_one_work+0x1ff/0x3f0 [ 605.575750] ? worker_thread+0x28/0x3e0 [ 605.575751] ? kthread+0x107/0x120 [ 605.575751] ? process_one_work+0x3f0/0x3f0 [ 605.575751] ? kthread_park+0x80/0x80 [ 605.575752] ? ret_from_fork+0x1f/0x30 Here's a kernel log with grep -i 'usb\|hcd' before removal: [0.008613] ACPI: SSDT 0xAFFAB000 001099 (v02 LGE UsbCTabl 1000 INTL 20160527) [0.247206] ACPI: bus type USB registered [0.247206] usbcore: registered new interface driver usbfs [0.247206] usbcore: registered new interface driver hub [0.247206] usbcore: registered new device driver usb [0.509498] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [0.511081] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver [0.512623] uhci_hcd: USB Universal Host Controller Interface driver [0.513261] xhci_hcd :00:14.0: xHCI Host Controller [0.513780] xhci_hcd :00:14.0: new USB bus registered, assigned bus number 1 [0.515379] xhci_hcd :00:14.0: hcc params 0x200077c1 hci version 0x100 quirks 0x01109810 [0.515912] xhci_hcd :00:14.0: cache line size of 64 is not supported [0.516031] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 4.19 [0.516583] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [0.517120] usb usb1: Product: xHCI Host Controller [0.517661] usb usb1: Manufacturer: Linux 4.19.8-zen+ xhci-hcd [0.518197] usb usb1: SerialNumber: :00:14.0 [0.518838] hub 1-0:1.0: USB hub found [0.520851] xhci_hcd :00:14.0: xHCI Host Controller [0.521389] xhci_hc
[PATCH] usb: roles: Add a description for the class to Kconfig
That makes the USB role switch support option visible and
selectable for the user. The class driver is also moved to
drivers/usb/roles/ directory.
This will fix an issue that we have with the Intel USB role
switch driver on systems that don't have USB Type-C connectors:
Intel USB role switch driver depends on the USB role switch
class as it should, but since there was no way for the user
to enable the USB role switch class, there was also no way
to select that driver. USB Type-C drivers select the USB
role switch class which makes the Intel USB role switch
driver available and therefore hides the problem.
So in practice Intel USB role switch driver was depending on
USB Type-C drivers.
Fixes: f6fb9ec02be1 ("usb: roles: Add Intel xHCI USB role switch driver")
Cc:
Signed-off-by: Heikki Krogerus
---
drivers/usb/Kconfig | 4
drivers/usb/common/Makefile | 1 -
drivers/usb/roles/Kconfig | 13 +
drivers/usb/roles/Makefile| 4 +++-
drivers/usb/{common/roles.c => roles/class.c} | 0
5 files changed, 16 insertions(+), 6 deletions(-)
rename drivers/usb/{common/roles.c => roles/class.c} (100%)
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 987fc5ba6321..70e6c956c23c 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -205,8 +205,4 @@ config USB_ULPI_BUS
To compile this driver as a module, choose M here: the module will
be called ulpi.
-config USB_ROLE_SWITCH
- tristate
- select USB_COMMON
-
endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index fb4d5ef4165c..0a7c45e85481 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -9,4 +9,3 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
-obj-$(CONFIG_USB_ROLE_SWITCH) += roles.o
diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig
index f5a5e6f79f1b..e4194ac94510 100644
--- a/drivers/usb/roles/Kconfig
+++ b/drivers/usb/roles/Kconfig
@@ -1,3 +1,16 @@
+config USB_ROLE_SWITCH
+ tristate "USB Role Switch Support"
+ help
+ USB Role Switch is a device that can select the USB role - host or
+ device - for a USB port (connector). In most cases dual-role capable
+ USB controller will also represent the switch, but on some platforms
+ multiplexer/demultiplexer switch is used to route the data lines on
+ the USB connector between separate USB host and device controllers.
+
+ Say Y here if your USB connectors support both device and host roles.
+ To compile the driver as module, choose M here: the module will be
+ called roles.ko.
+
if USB_ROLE_SWITCH
config USB_ROLES_INTEL_XHCI
diff --git a/drivers/usb/roles/Makefile b/drivers/usb/roles/Makefile
index e44b179ba275..c02873206fc1 100644
--- a/drivers/usb/roles/Makefile
+++ b/drivers/usb/roles/Makefile
@@ -1 +1,3 @@
-obj-$(CONFIG_USB_ROLES_INTEL_XHCI) += intel-xhci-usb-role-switch.o
+obj-$(CONFIG_USB_ROLE_SWITCH) += roles.o
+roles-y:= class.o
+obj-$(CONFIG_USB_ROLES_INTEL_XHCI) += intel-xhci-usb-role-switch.o
diff --git a/drivers/usb/common/roles.c b/drivers/usb/roles/class.c
similarity index 100%
rename from drivers/usb/common/roles.c
rename to drivers/usb/roles/class.c
--
2.19.2
Re: usb: CPU 100% hog upon device removal
On Thu, Dec 13, 2018 at 01:28:36AM +0900, Ju Hyung Park wrote: > Hi, > > I'm using the latest stable tree v4.19.8, > and I'm noticing kworker and ksoftirqd hogging up the CPU > upon removing any USB devices on my laptop. > > This issue was not present on any kernels below v4.19. Can you run 'git bisect' to track down the offending commit? thanks, greg k-h
Re: usb: CPU 100% hog upon device removal
Hi, On Thu, Dec 13, 2018 at 3:17 AM Greg KH wrote: > > On Thu, Dec 13, 2018 at 01:28:36AM +0900, Ju Hyung Park wrote: > > Hi, > > > > I'm using the latest stable tree v4.19.8, > > and I'm noticing kworker and ksoftirqd hogging up the CPU > > upon removing any USB devices on my laptop. > > > > This issue was not present on any kernels below v4.19. > > Can you run 'git bisect' to track down the offending commit? I can, but I was hoping to get some pointers from the maintainers to quickly get down to the culprit. I will run 'git bisect' later when I get a chance, would greatly appreciate it if someone hints me towards possible culprits. I should probably also mention that the kernel seems to be fine with physical removal. Only when I do soft removal: sysfs echo 1 > remove seems to cause the issue. Thanks,
Re: [PATCH net 2/2] net: usb: aqc111: Support for thermal throttling feature
Hi Andrew,
>> When the PHY's temparature is back to normal (low threshold, it is
>> 85 degrees) it restores user/default link speed settings.
>
> Hi Igor
>
> Please could you also export the temperature using HWMON. The Marvell
> PHY drivers are good examples.
>
> I also have a patch for driver/net/phy/aquantia.c which adds HWMON
> support to that PHY.
We actually have almost ready patches to submit hwmon support separately
for both AQC USB and AQC PCI MAC drivers.
Will do that in near time.
>> +if (aqc111_data->thermal_throttling)
>> +speed = SPEED_100;
>> +
>
> That is a big jump. Do you need to cool is down quickly, or would
> 1Gbps be sufficient? I think as a user, i would prefer it ping-pongs
> between 5G and 1G, not 5G and 100M.
In case overheat happens that really means something bad is happening outside,
or heatsink is bad/detached. I'll consult our HW team once again, but 100M was
the original request from our phy/hw team. It seems it really much less on
power and
1G may not be enough.
>
>> if (autoneg == AUTONEG_ENABLE) {
>> switch (speed) {
>> case SPEED_5000:
>
> It looks like this only works when auto-neg is enabled. If i've fixed
> configured it i don't think this works?
Looks you are right, will check this.
>> +if (aqc111_data->thermal_throttling &&
>> +temperature <= AQ_NORMAL_TEMP_THRESHOLD) {
>> +netdev_info(dev->net, "The temperature is back to normal(%u)",
>> +AQ_NORMAL_TEMP_THRESHOLD / 100);
>
> How often do you see these messages? I'm wondering if they need to be
> rate limited?
In worst case that will be at least limited by link down/up internal,
which in case of 5G normally takes 3-5secs.
>> +if (!aqc111_data->thermal_throttling &&
>> +temperature >= AQ_CRITICAL_TEMP_THRESHOLD) {
>
> Should there be some hysteresis in here? In fact, if temperature is
> AQ_CRITICAL_TEMP_THRESHOLD it is both back to normal, and throttled at
> the same time!
NORMAL_TEMP and CRITICAL_TEMP values are actually a hysteresis.
In cool down case only after TEMP_NORMAL temperature link will be flipped back
again.
>
>> +netdev_warn(dev->net, "Critical temperature(%u) is reached",
>> +AQ_CRITICAL_TEMP_THRESHOLD / 100);
>> +aqc111_data->thermal_throttling = 1;
>> +reset_speed = 1;
>
> update_speed might be a better name, since you are not always
> resetting it.
Agreed.
Regards,
Igor
Re: [PATCH net 1/2] net: usb: aqc111: Add read_mdio operation
>> >> Add necessary defines to read phy registers and temparature > > Hi Igor > > [Puts tongue in cheek] > > I thought the firmware was supposed to manage the PHY. FW team due to various reasons do not want to have thermal throttling in their control, Thus at this moment we are trying to release the product which will not burn out the table under it ;-) > So maybe the better fix is to add code to allow firmware upgrade? And > issue new firmware to linux-firmware? I would say thats our long term future. On your request for linux-firmware, I've pushed the question to Phy team, will inform you on any news. Regards, Igor
Re: [PATCH net 2/2] net: usb: aqc111: Support for thermal throttling feature
> NORMAL_TEMP and CRITICAL_TEMP values are actually a hysteresis. In > cool down case only after TEMP_NORMAL temperature link will be > flipped back again. Ah, yes, sorry, i read that wrong. Andrew
[PATCH v2] USB: serial: option: add Fibocom NL668 series
Added USB serial option driver support for Fibocom NL668 series cellular
modules. Reserved USB endpoints 4, 5 and 6 for network + ADB interfaces.
usb-devices output (QMI mode)
T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 16 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1508 ProdID=1001 Rev=03.18
S: Manufacturer=Nodecom NL668 Modem
S: Product=Nodecom NL668-CN Modem
S: SerialNumber=
C: #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
I: If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)
usb-devices output (ECM mode)
T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 17 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1508 ProdID=1001 Rev=03.18
S: Manufacturer=Nodecom NL668 Modem
S: Product=Nodecom NL668-CN Modem
S: SerialNumber=
C: #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
I: If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I: If#= 4 Alt= 0 #EPs= 1 Cls=02(commc) Sub=06 Prot=00 Driver=cdc_ether
I: If#= 5 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=cdc_ether
I: If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)
Signed-off-by: Jörgen Storvist
---
Changelog:
Fixed line break issues
Added reservation for ADB interface also
Updated source option.c file to latest in usb-linus branch
Thanks Johan for the inputs, here goes:
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 53e8de5..ff2e635 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1949,6 +1949,8 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0xa31d, 0xff, 0x06, 0x13) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0xa31d, 0xff, 0x06, 0x14) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0xa31d, 0xff, 0x06, 0x1b) },
+ { USB_DEVICE(0x1508, 0x1001), /* Fibocom NL668 series */
+ .driver_info = RSVD(4) | RSVD(5) | RSVD(6) },
{ } /* Terminating entry */
};
MODULE_DEVICE_TABLE(usb, option_ids);
Re: usb: CPU 100% hog upon device removal
On Thu, 13 Dec 2018, Ju Hyung Park wrote: > Hi, > > On Thu, Dec 13, 2018 at 3:17 AM Greg KH wrote: > > > > On Thu, Dec 13, 2018 at 01:28:36AM +0900, Ju Hyung Park wrote: > > > Hi, > > > > > > I'm using the latest stable tree v4.19.8, > > > and I'm noticing kworker and ksoftirqd hogging up the CPU > > > upon removing any USB devices on my laptop. > > > > > > This issue was not present on any kernels below v4.19. > > > > Can you run 'git bisect' to track down the offending commit? > > I can, but I was hoping to get some pointers from the maintainers to > quickly get down to the culprit. > > I will run 'git bisect' later when I get a chance, > would greatly appreciate it if someone hints me towards possible culprits. > > I should probably also mention that the kernel seems to be fine > with physical removal. > > Only when I do soft removal: sysfs echo 1 > remove > seems to cause the issue. The information you provided wasn't ideal for suggesting culprits. You apparently don't have CONFIG_FRAME_POINTER enabled (which makes the backtrace very difficult to interpret), and you didn't turn on dynamic debugging for the usbcore and xhci-hcd drivers (which makes the dmesg log much less complete). For what it's worth, I just tried doing the same thing under Fedora's 4.19.5 kernel. Nothing went wrong; CPU usage is 99.8% idle. Alan Stern
Re: usb: CPU 100% hog upon device removal
Hi, I've just tried out Ubuntu's kernel PPA and this issue is still there. At least I didn't screw something up on my personal builds. I've used the following for activating USB dynamic debug: echo "file drivers/usb/* +p" > /sys/kernel/debug/dynamic_debug/control Let me know if I should have done something else. Kernel log stays quiet until I do echo 1 > remove, and it goes nuts. http://www.arter97.com/usblog.txt.xz (uncompressed size 27M) Please have a look. Thanks. On Thu, Dec 13, 2018 at 6:46 AM Alan Stern wrote: > > On Thu, 13 Dec 2018, Ju Hyung Park wrote: > > > Hi, > > > > On Thu, Dec 13, 2018 at 3:17 AM Greg KH wrote: > > > > > > On Thu, Dec 13, 2018 at 01:28:36AM +0900, Ju Hyung Park wrote: > > > > Hi, > > > > > > > > I'm using the latest stable tree v4.19.8, > > > > and I'm noticing kworker and ksoftirqd hogging up the CPU > > > > upon removing any USB devices on my laptop. > > > > > > > > This issue was not present on any kernels below v4.19. > > > > > > Can you run 'git bisect' to track down the offending commit? > > > > I can, but I was hoping to get some pointers from the maintainers to > > quickly get down to the culprit. > > > > I will run 'git bisect' later when I get a chance, > > would greatly appreciate it if someone hints me towards possible culprits. > > > > I should probably also mention that the kernel seems to be fine > > with physical removal. > > > > Only when I do soft removal: sysfs echo 1 > remove > > seems to cause the issue. > > The information you provided wasn't ideal for suggesting culprits. > You apparently don't have CONFIG_FRAME_POINTER enabled (which makes the > backtrace very difficult to interpret), and you didn't turn on dynamic > debugging for the usbcore and xhci-hcd drivers (which makes the dmesg > log much less complete). > > For what it's worth, I just tried doing the same thing under Fedora's > 4.19.5 kernel. Nothing went wrong; CPU usage is 99.8% idle. > > Alan Stern >
Re: [PATCH v3] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
From: Greg KH Date: Wed, 12 Dec 2018 12:42:24 +0100 > From: Hui Peng > > The function hso_probe reads if_num from the USB device (as an u8) and uses > it without a length check to index an array, resulting in an OOB memory read > in hso_probe or hso_get_config_data. > > Add a length check for both locations and updated hso_probe to bail on > error. > > This issue has been assigned CVE-2018-19985. > > Reported-by: Hui Peng > Reported-by: Mathias Payer > Signed-off-by: Hui Peng > Signed-off-by: Mathias Payer > Reviewed-by: Sebastian Andrzej Siewior > Signed-off-by: Greg Kroah-Hartman Applied and queued up for -stable, thanks Greg.
Re: [PATCH net 0/2] aqc111: Thermal throttling feature
From: Igor Russkikh Date: Wed, 12 Dec 2018 13:50:06 + > This patches introduce the thermal throttling feature to prevent possible > heat damage to the hardware. I see what seems to be a bit of a conflict here, maybe you can explain the situation better to me. Andrew suggested that the firmware manage the thermal aspects of the PHY since it manages all other aspects of the PHY too. And then the feedback was that the firmware folks don't want to do that right now. But then it was also stated that the long term goal is to support what Andrew asked for, firmware update in the driver and updated firmwares submitted to linux-firmware. If the firwmare will eventually have support for thermal management added, then the code in this series is going to be not used and just taking up space. Please explain how all of this is going to fit together, and how we'll not end up with having to keep this thermal code around forever. Thanks.
RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver
> > > > Felipe, I use Dynamic Debug for debugging, and show debug messages > > with "dmesg" after testing/debugging. I see dwc3 using trace, any > > benefits for switching to trace? > > The benefits I see are > Thanks, bin. > - *by default*, the debug log doesn't have to go through uart console, > which is slow. > > I typically 'cat trace_pipe' from a telnet which is much faster then > uart. But Dynamic Debug log by default got printed on uart, I have to > set printk level to not print them, which is an extra step. > It depends on the rootfs's setting. By default, the debug message level should be off ( console level = 7) > - tracepoint uses one place to decode the message vs DD has to repeat > the similar print statement in the code; and tracepoint decodes it > offline which reduce the kernel runtime overhead too. > offline? You mean when you run "cat trace_pipe"? > - with tracepoint, it is easier to turn on debug for a specific group of > messages. > The DD can, but we don't do it often. https://www.kernel.org/doc/html/latest/admin-guide/dynamic-debug-howto.html?highlight=dynamic%20debug > - you can adjust the ftrace buffer size at runtime. can we do that for > printk? I don't remember. It may can't. The log buf can only be changed by bootargs: log_buf_len Peter
[PATCH] USB: ehci-omap: Fix deferred probe for phy handling
The only case where we can bail out safely without a phy is port_mode
is OMAP_USBHS_PORT_MODE_UNUSED. It used to be that OMAP_EHCI_PORT_MODE_PHY
was optional, but that's not a good assumption. We should already have
"ehci-phy" in all the dts files using OMAP_EHCI_PORT_MODE_PHY.
Note that this fix should not be needed for kernels earlier than v4.19
as that's when we started moving devices to probe with ti-sysc. We now
probe l4 interconnects separately, which can cause deferred probe with
the phy being on a separate l4 interconnect from EHCI.
And old kernels would need to be checked for "ehci-phy" property for
this fix to avoid regressions.
Cc: Alan Stern
Cc: Johan Hovold
Cc: Ladislav Michl
Cc: Peter Ujfalusi
Cc: Roger Quadros
Reported-by: Peter Ujfalusi
Signed-off-by: Tony Lindgren
---
drivers/usb/host/ehci-omap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -159,8 +159,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
/* get the PHY device */
phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
if (IS_ERR(phy)) {
- /* Don't bail out if PHY is not absolutely necessary */
- if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
+ /* Don't bail out if PHY is unused */
+ if (pdata->port_mode[i] == OMAP_USBHS_PORT_MODE_UNUSED)
continue;
ret = PTR_ERR(phy);
--
2.19.2
[PATCH -next] usb: renesas_usbhs: Fix unused function warning when CONFIG_PM not set
with CONFIG_PM not set, gcc warning this:
drivers/usb/renesas_usbhs/common.c:844:12:
warning: 'usbhsc_suspend' defined but not used [-Wunused-function]
drivers/usb/renesas_usbhs/common.c:860:12:
warning: 'usbhsc_resume' defined but not used [-Wunused-function]
fix this by adding #ifdef around it.
Signed-off-by: YueHaibing
---
drivers/usb/renesas_usbhs/common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.c
b/drivers/usb/renesas_usbhs/common.c
index 02c1d2b..9390c76 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -841,6 +841,7 @@ static int usbhs_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
static int usbhsc_suspend(struct device *dev)
{
struct usbhs_priv *priv = dev_get_drvdata(dev);
@@ -873,6 +874,7 @@ static int usbhsc_resume(struct device *dev)
return 0;
}
+#endif
static SIMPLE_DEV_PM_OPS(usbhsc_pm_ops, usbhsc_suspend, usbhsc_resume);
--
2.7.4
[PATCH] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.
From: Macpaul Lin
Mediatek Preloader is a proprietary embedded boot loader for loading
Little Kernel and Linux into device DRAM.
This boot loader also handle firmware updating. Mediatek Preloader will be
enumerated as a virtual COM port when the device is connected to Windows
or Linux OS via CDC-ACM class driver. When the enumeration has been done,
Mediatek Preloader will send out handshake command "READY" to PC actively
instead of waiting command from the download tool.
Since Linux 4.12, the commit "tty: reset termios state on device
registration" (93857edd9829e144acb6c7e72d593f6e01aead66) causes Mediatek
Preloader receiving some abnoraml command like "READYXX" as it sended.
Which will be recognized as an incorrect response. This behavior change
also causes the handshake fail.
By disabling the ECHO termios flag could avoid this problem. However, it
cannot be done by user space configuration when download tool open
/dev/ttyACM0. This is because the device running Mediatek Preloader will
send handshake command "READY" immediately once the CDC-ACM driver is
ready.
This patch wants to fix above problem by introducing "DISABLE_ECHO"
property in driver_info. When Mediatek Preloader is connected, the
CDC-ACM driver could disable ECHO flag in termios to avoid the problem.
Signed-off-by: Macpaul Lin
---
drivers/usb/class/cdc-acm.c | 9 -
drivers/usb/class/cdc-acm.h | 1 +
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..2f744bb 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1156,6 +1156,10 @@ static int acm_probe(struct usb_interface *intf,
goto skip_normal_probe;
}
+ /* handle active handshake triggered by device */
+ if (quirks == DISABLE_ECHO)
+ acm_tty_driver->init_termios.c_lflag &= ~(ECHO);
+
/* normal probing*/
if (!buffer) {
dev_err(&intf->dev, "Weird descriptor references\n");
@@ -1655,7 +1659,10 @@ static int acm_pre_reset(struct usb_interface *intf)
.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
},
{ USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc;
andrey.ara...@gmail.com */
- .driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+ .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+ },
+ { USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+ .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
},
{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
#define QUIRK_CONTROL_LINE_STATE BIT(6)
#define CLEAR_HALT_CONDITIONS BIT(7)
#define SEND_ZERO_PACKET BIT(8)
+#define DISABLE_ECHO BIT(9)
--
1.9.1
RE: [PATCH -next] usb: renesas_usbhs: Fix unused function warning when CONFIG_PM not set
Hi, > From: YueHaibing, Sent: Thursday, December 13, 2018 12:19 PM > > with CONFIG_PM not set, gcc warning this: > drivers/usb/renesas_usbhs/common.c:844:12: > warning: 'usbhsc_suspend' defined but not used [-Wunused-function] > drivers/usb/renesas_usbhs/common.c:860:12: > warning: 'usbhsc_resume' defined but not used [-Wunused-function] > > fix this by adding #ifdef around it. > > Signed-off-by: YueHaibing Thank you for the patch. However, this issue is already fixed on the Greg's usb.git / usb-next branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=eaf3074e0a8c2a39c4c14aa8ef1c2ec09ace9c79 Best regards, Yoshihiro Shimoda
Re: [PATCH] USB: ehci-omap: Fix deferred probe for phy handling
On 13/12/2018 4.17, Tony Lindgren wrote:
> The only case where we can bail out safely without a phy is port_mode
> is OMAP_USBHS_PORT_MODE_UNUSED. It used to be that OMAP_EHCI_PORT_MODE_PHY
> was optional, but that's not a good assumption. We should already have
> "ehci-phy" in all the dts files using OMAP_EHCI_PORT_MODE_PHY.
>
> Note that this fix should not be needed for kernels earlier than v4.19
> as that's when we started moving devices to probe with ti-sysc. We now
> probe l4 interconnects separately, which can cause deferred probe with
> the phy being on a separate l4 interconnect from EHCI.
>
> And old kernels would need to be checked for "ehci-phy" property for
> this fix to avoid regressions.
Tested-by: Peter Ujfalusi
> Cc: Alan Stern
> Cc: Johan Hovold
> Cc: Ladislav Michl
> Cc: Peter Ujfalusi
> Cc: Roger Quadros
> Reported-by: Peter Ujfalusi
> Signed-off-by: Tony Lindgren
> ---
> drivers/usb/host/ehci-omap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -159,8 +159,8 @@ static int ehci_hcd_omap_probe(struct platform_device
> *pdev)
> /* get the PHY device */
> phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
> if (IS_ERR(phy)) {
> - /* Don't bail out if PHY is not absolutely necessary */
> - if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
> + /* Don't bail out if PHY is unused */
> + if (pdata->port_mode[i] == OMAP_USBHS_PORT_MODE_UNUSED)
> continue;
>
> ret = PTR_ERR(phy);
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH 4.19 014/110] usb: xhci: Prevent bus suspend if a port connect change or polling state is detected
On Wed, Dec 12, 2018 at 11:53:34PM +0100, Thomas Zeitlhofer wrote: > Hello, > > On Thu, Nov 29, 2018 at 03:11:45PM +0100, Greg Kroah-Hartman wrote: > > 4.19-stable review patch. If anyone has any objections, please let me > > know. > > > > -- > > > > From: Mathias Nyman > > > > commit 2f31a67f01a8beb22cae754c53522cb61a005750 upstream. > [...] > > on a current Thinkpad X1 Yoga, this breaks resume from hibernate such > that opening the lid has (in the regular use case, see below) no effect > any more: > > The system is configured to hibernate when the lid is closed. So, the > expected behavior, which is restored by reverting this patch, is: > > close lid => system hibernates > open lid => system resumes > > With this patch, the following two cases are observed: > > 1) > close lid => system hibernates > open lid => system stays off > press power button => system boots and resumes > > 2) > # systemctl hibernate => system hibernates > close lid > open lid => system resumes > So this is a problem in Linus's tree? If so, let's work to get this fixed there first. If not, then we have other issues :) thanks, greg k-h
Re: usb: CPU 100% hog upon device removal
On Thu, Dec 13, 2018 at 07:15:10AM +0900, Ju Hyung Park wrote: > Hi, > > I've just tried out Ubuntu's kernel PPA and this issue is still there. > At least I didn't screw something up on my personal builds. > > I've used the following for activating USB dynamic debug: > echo "file drivers/usb/* +p" > /sys/kernel/debug/dynamic_debug/control > Let me know if I should have done something else. > > Kernel log stays quiet until I do echo 1 > remove, and it goes nuts. > http://www.arter97.com/usblog.txt.xz > (uncompressed size 27M) > > Please have a look. Again, 'git bisect' is going to be the easiest way to help figure this out. thanks, greg k-h
