Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
Hi, On 12/01/2016 03:35 PM, Baolin Wang wrote: > On 1 December 2016 at 14:35, Lu Baolu wrote: >> Hi, >> >> On 12/01/2016 02:04 PM, Baolin Wang wrote: >>> Hi Baolu, >>> >>> On 1 December 2016 at 13:45, Lu Baolu wrote: Hi, On 11/30/2016 05:02 PM, Baolin Wang wrote: > If the hardware never responds to the stop endpoint command, the > URBs will never be completed, and we might hang the USB subsystem. > The original watchdog timer is used to watch if one stop endpoint > command is timeout, if timeout, then the watchdog timer will set > XHCI_STATE_DYING, try to halt the xHCI host, and give back all > pending URBs. > > But now we already have one command timer to control command timeout, > thus we can also use the command timer to watch the stop endpoint > command, instead of one duplicate watchdog timer which need to be > removed. > > Meanwhile we don't need the 'stop_cmds_pending' flag to identy if > this is the last stop endpoint command of one endpoint. Since we > can make sure we only set one stop endpoint command for one endpoint > by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove > this flag. I am afraid you can't do this. "stop_cmds_pending" was added to fix the problem described in the comments that you want to remove. But I didn't find any fix of this problem in your patch. >>> Now we can not pending another stop endpoint command for the same one >>> endpoint, since will check 'EP_HALT_PENDING' flag in >>> xhci_urb_dequeue() function to avoid this. But after some >>> investigation, I think I missed the stop endpoint command in >>> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag, >>> maybe need to add 'EP_HALT_PENDING' flag checking in >>> xhci_stop_device() function. DId I miss something else? Thanks. >> Consider below three threads running on different CPUs at the same time. >> >> Thread A: xhci_handle_cmd_stop_ep() --- in interrupt handler >> Thread B: xhci_stop_endpoint_command_timeout() --- timer expired >> Thread C: xhci_urb_dequeue --- called by usb core >> >> They are serialized by xhci->lock. Let's consider below sequence: >> >> Thread A: >> - delete xhci->cmd_timer), but will fail due to Thread B. >> - clear EP_HALT_PENDING bit >> >> Thread B: >> - halt the host controller >> >> Thread C: >> - set EP_HALT_PENDING bit >> - enqueue another stop endpoint command >> - add the timer back > Ah, thanks for you comments. > If thread B halted the host, then the thread C xhci_urb_dequeue() will > check host state failed, then just return and can not set another stop > endpoint command. Not so simple. Thread B will release xhci->lock before xhci_halt(). > But from your example, I think your meaning is we > should not halt the host by thread B, since we have handled the stop > endpoint command event. > > So I think we need to check the xhci command of this stop endpoint > command in xhci_stop_endpoint_command_timeout(), if the xhci command > of this stop endpoint command is not in the command list (which means > the stop endpoint command event has been handled), then just return > and do not halt the controller. Right? > I'd like suggest you to put this change into a separated patch. It's actually a different topic from the main purpose of this patch. Best regards, Lu Baolu -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
On 1 December 2016 at 14:35, Lu Baolu wrote: > Hi, > > On 12/01/2016 02:04 PM, Baolin Wang wrote: >> Hi Baolu, >> >> On 1 December 2016 at 13:45, Lu Baolu wrote: >>> Hi, >>> >>> On 11/30/2016 05:02 PM, Baolin Wang wrote: If the hardware never responds to the stop endpoint command, the URBs will never be completed, and we might hang the USB subsystem. The original watchdog timer is used to watch if one stop endpoint command is timeout, if timeout, then the watchdog timer will set XHCI_STATE_DYING, try to halt the xHCI host, and give back all pending URBs. But now we already have one command timer to control command timeout, thus we can also use the command timer to watch the stop endpoint command, instead of one duplicate watchdog timer which need to be removed. Meanwhile we don't need the 'stop_cmds_pending' flag to identy if this is the last stop endpoint command of one endpoint. Since we can make sure we only set one stop endpoint command for one endpoint by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove this flag. >>> I am afraid you can't do this. "stop_cmds_pending" was added >>> to fix the problem described in the comments that you want to >>> remove. But I didn't find any fix of this problem in your patch. >> Now we can not pending another stop endpoint command for the same one >> endpoint, since will check 'EP_HALT_PENDING' flag in >> xhci_urb_dequeue() function to avoid this. But after some >> investigation, I think I missed the stop endpoint command in >> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag, >> maybe need to add 'EP_HALT_PENDING' flag checking in >> xhci_stop_device() function. DId I miss something else? Thanks. > > Consider below three threads running on different CPUs at the same time. > > Thread A: xhci_handle_cmd_stop_ep() --- in interrupt handler > Thread B: xhci_stop_endpoint_command_timeout() --- timer expired > Thread C: xhci_urb_dequeue --- called by usb core > > They are serialized by xhci->lock. Let's consider below sequence: > > Thread A: > - delete xhci->cmd_timer), but will fail due to Thread B. > - clear EP_HALT_PENDING bit > > Thread B: > - halt the host controller > > Thread C: > - set EP_HALT_PENDING bit > - enqueue another stop endpoint command > - add the timer back Ah, thanks for you comments. If thread B halted the host, then the thread C xhci_urb_dequeue() will check host state failed, then just return and can not set another stop endpoint command. But from your example, I think your meaning is we should not halt the host by thread B, since we have handled the stop endpoint command event. So I think we need to check the xhci command of this stop endpoint command in xhci_stop_endpoint_command_timeout(), if the xhci command of this stop endpoint command is not in the command list (which means the stop endpoint command event has been handled), then just return and do not halt the controller. Right? -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: OHCI: ohci-pxa27x: remove useless functions
On Wed, Nov 30, 2016 at 11:59:49PM +, csmanjuvi...@gmail.com wrote: > From: Manjunath Goudar > > The ohci_hcd_pxa27x_drv_probe function is not doing anything other > than calling usb_hcd_pxa27x_probe function so ohci_hcd_pxa27x_drv_probe > function is useless that is why removed ohci_hcd_pxa27x_drv_probe > function and renamed usb_hcd_pxa27x_probe function to > ohci_hcd_pxa27x_drv_probe for proper naming. > > The ohci_hcd_pxa27x_remove function is also not doing anything other than > calling usb_hcd_pxa27x_remove that is why removed ohci_hcd_pxa27x_remove > function and renamed usb_hcd_pxa27x_remove to ohci_hcd_pxa27x_remove for > proper naming. > > Signed-off-by: Manjunath Goudar > Cc: Alan Stern > Cc: Greg Kroah-Hartman > Cc: linux-usb@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > > Changelog v1 -> v2: > 1.Removed warnings and errors of checkpatch.pl script. > 2.Replaced unuseful with useless in patch commit message for proper meaning. > 3.Removed usb_disabled() from ohci_hcd_pxa27x_drv_probe function because it > is already existing in ohci_pxa27x_init function > --- > drivers/usb/host/ohci-pxa27x.c | 42 > ++ > 1 file changed, 14 insertions(+), 28 deletions(-) changelog should go below --- line. > > diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c > index a667cf2..171f28f2 100644 > --- a/drivers/usb/host/ohci-pxa27x.c > +++ b/drivers/usb/host/ohci-pxa27x.c > @@ -157,9 +157,9 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci > *pxa_ohci, int mode) > uhcrhdb |= (0x7<<17); > break; > default: > - printk( KERN_ERR > + printk(KERN_ERR No, don't do checkpatch cleanups in portions of the code you are not actually changing for your "real" patch. That's a mess. You can send checkpatch patches as follow-on patches, but not with this one. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
Hi, On 12/01/2016 02:04 PM, Baolin Wang wrote: > Hi Baolu, > > On 1 December 2016 at 13:45, Lu Baolu wrote: >> Hi, >> >> On 11/30/2016 05:02 PM, Baolin Wang wrote: >>> If the hardware never responds to the stop endpoint command, the >>> URBs will never be completed, and we might hang the USB subsystem. >>> The original watchdog timer is used to watch if one stop endpoint >>> command is timeout, if timeout, then the watchdog timer will set >>> XHCI_STATE_DYING, try to halt the xHCI host, and give back all >>> pending URBs. >>> >>> But now we already have one command timer to control command timeout, >>> thus we can also use the command timer to watch the stop endpoint >>> command, instead of one duplicate watchdog timer which need to be >>> removed. >>> >>> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if >>> this is the last stop endpoint command of one endpoint. Since we >>> can make sure we only set one stop endpoint command for one endpoint >>> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove >>> this flag. >> I am afraid you can't do this. "stop_cmds_pending" was added >> to fix the problem described in the comments that you want to >> remove. But I didn't find any fix of this problem in your patch. > Now we can not pending another stop endpoint command for the same one > endpoint, since will check 'EP_HALT_PENDING' flag in > xhci_urb_dequeue() function to avoid this. But after some > investigation, I think I missed the stop endpoint command in > xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag, > maybe need to add 'EP_HALT_PENDING' flag checking in > xhci_stop_device() function. DId I miss something else? Thanks. Consider below three threads running on different CPUs at the same time. Thread A: xhci_handle_cmd_stop_ep() --- in interrupt handler Thread B: xhci_stop_endpoint_command_timeout() --- timer expired Thread C: xhci_urb_dequeue --- called by usb core They are serialized by xhci->lock. Let's consider below sequence: Thread A: - delete xhci->cmd_timer), but will fail due to Thread B. - clear EP_HALT_PENDING bit Thread B: - halt the host controller Thread C: - set EP_HALT_PENDING bit - enqueue another stop endpoint command - add the timer back Best regards, Lu Baolu > >> - * The timer may also fire if the host takes a very long time to respond to >> the >> - * command, and the stop endpoint command completion handler cannot delete >> the >> - * timer before the timer function is called. Another endpoint >> cancellation may >> - * sneak in before the timer function can grab the lock, and that may queue >> - * another stop endpoint command and add the timer back. So we cannot use a >> - * simple flag to say whether there is a pending stop endpoint command for a >> - * particular endpoint. >> - * >> - * Instead we use a combination of that flag and a counter for the number of >> - * pending stop endpoint commands. If the timer is the tail end of the last >> - * stop endpoint command, and the endpoint's command is still pending, we >> assume >> - * the host is dying. >> >> Best regards, >> Lu Baolu >> >>> We also need to clean up the command queue before trying to halt the >>> xHCI host in xhci_stop_endpoint_command_timeout() function. >>> >>> Signed-off-by: Baolin Wang > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
On 1 December 2016 at 14:04, Baolin Wang wrote: > Hi Baolu, > > On 1 December 2016 at 13:45, Lu Baolu wrote: >> Hi, >> >> On 11/30/2016 05:02 PM, Baolin Wang wrote: >>> If the hardware never responds to the stop endpoint command, the >>> URBs will never be completed, and we might hang the USB subsystem. >>> The original watchdog timer is used to watch if one stop endpoint >>> command is timeout, if timeout, then the watchdog timer will set >>> XHCI_STATE_DYING, try to halt the xHCI host, and give back all >>> pending URBs. >>> >>> But now we already have one command timer to control command timeout, >>> thus we can also use the command timer to watch the stop endpoint >>> command, instead of one duplicate watchdog timer which need to be >>> removed. >>> >>> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if >>> this is the last stop endpoint command of one endpoint. Since we >>> can make sure we only set one stop endpoint command for one endpoint >>> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove >>> this flag. >> >> I am afraid you can't do this. "stop_cmds_pending" was added >> to fix the problem described in the comments that you want to >> remove. But I didn't find any fix of this problem in your patch. > > Now we can not pending another stop endpoint command for the same one > endpoint, since will check 'EP_HALT_PENDING' flag in > xhci_urb_dequeue() function to avoid this. But after some > investigation, I think I missed the stop endpoint command in > xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag, > maybe need to add 'EP_HALT_PENDING' flag checking in > xhci_stop_device() function. DId I miss something else? Thanks. Sorry, wait, we will wait for the stop endpoint command finished in xhci_stop_device() function. So we don't need check 'EP_HALT_PENDING' flag as the orignal code did. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
Hi Baolu, On 1 December 2016 at 13:45, Lu Baolu wrote: > Hi, > > On 11/30/2016 05:02 PM, Baolin Wang wrote: >> If the hardware never responds to the stop endpoint command, the >> URBs will never be completed, and we might hang the USB subsystem. >> The original watchdog timer is used to watch if one stop endpoint >> command is timeout, if timeout, then the watchdog timer will set >> XHCI_STATE_DYING, try to halt the xHCI host, and give back all >> pending URBs. >> >> But now we already have one command timer to control command timeout, >> thus we can also use the command timer to watch the stop endpoint >> command, instead of one duplicate watchdog timer which need to be >> removed. >> >> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if >> this is the last stop endpoint command of one endpoint. Since we >> can make sure we only set one stop endpoint command for one endpoint >> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove >> this flag. > > I am afraid you can't do this. "stop_cmds_pending" was added > to fix the problem described in the comments that you want to > remove. But I didn't find any fix of this problem in your patch. Now we can not pending another stop endpoint command for the same one endpoint, since will check 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function to avoid this. But after some investigation, I think I missed the stop endpoint command in xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag, maybe need to add 'EP_HALT_PENDING' flag checking in xhci_stop_device() function. DId I miss something else? Thanks. > > - * The timer may also fire if the host takes a very long time to respond to > the > - * command, and the stop endpoint command completion handler cannot delete > the > - * timer before the timer function is called. Another endpoint cancellation > may > - * sneak in before the timer function can grab the lock, and that may queue > - * another stop endpoint command and add the timer back. So we cannot use a > - * simple flag to say whether there is a pending stop endpoint command for a > - * particular endpoint. > - * > - * Instead we use a combination of that flag and a counter for the number of > - * pending stop endpoint commands. If the timer is the tail end of the last > - * stop endpoint command, and the endpoint's command is still pending, we > assume > - * the host is dying. > > Best regards, > Lu Baolu > >> >> We also need to clean up the command queue before trying to halt the >> xHCI host in xhci_stop_endpoint_command_timeout() function. >> >> Signed-off-by: Baolin Wang > -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
Hi, On 11/30/2016 05:02 PM, Baolin Wang wrote: > If the hardware never responds to the stop endpoint command, the > URBs will never be completed, and we might hang the USB subsystem. > The original watchdog timer is used to watch if one stop endpoint > command is timeout, if timeout, then the watchdog timer will set > XHCI_STATE_DYING, try to halt the xHCI host, and give back all > pending URBs. > > But now we already have one command timer to control command timeout, > thus we can also use the command timer to watch the stop endpoint > command, instead of one duplicate watchdog timer which need to be > removed. > > Meanwhile we don't need the 'stop_cmds_pending' flag to identy if > this is the last stop endpoint command of one endpoint. Since we > can make sure we only set one stop endpoint command for one endpoint > by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove > this flag. I am afraid you can't do this. "stop_cmds_pending" was added to fix the problem described in the comments that you want to remove. But I didn't find any fix of this problem in your patch. - * The timer may also fire if the host takes a very long time to respond to the - * command, and the stop endpoint command completion handler cannot delete the - * timer before the timer function is called. Another endpoint cancellation may - * sneak in before the timer function can grab the lock, and that may queue - * another stop endpoint command and add the timer back. So we cannot use a - * simple flag to say whether there is a pending stop endpoint command for a - * particular endpoint. - * - * Instead we use a combination of that flag and a counter for the number of - * pending stop endpoint commands. If the timer is the tail end of the last - * stop endpoint command, and the endpoint's command is still pending, we assume - * the host is dying. Best regards, Lu Baolu > > We also need to clean up the command queue before trying to halt the > xHCI host in xhci_stop_endpoint_command_timeout() function. > > Signed-off-by: Baolin Wang -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
On 30 November 2016 at 22:09, Mathias Nyman wrote: > On 30.11.2016 11:02, Baolin Wang wrote: >> >> If the hardware never responds to the stop endpoint command, the >> URBs will never be completed, and we might hang the USB subsystem. >> The original watchdog timer is used to watch if one stop endpoint >> command is timeout, if timeout, then the watchdog timer will set >> XHCI_STATE_DYING, try to halt the xHCI host, and give back all >> pending URBs. >> >> But now we already have one command timer to control command timeout, >> thus we can also use the command timer to watch the stop endpoint >> command, instead of one duplicate watchdog timer which need to be >> removed. >> >> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if >> this is the last stop endpoint command of one endpoint. Since we >> can make sure we only set one stop endpoint command for one endpoint >> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove >> this flag. >> >> We also need to clean up the command queue before trying to halt the >> xHCI host in xhci_stop_endpoint_command_timeout() function. > > > This isn't a bad idea. > > There are anyway some corner cases and details that need to be > checked, such as suspend (which will clear the command queue), module unload > and abrupt host removal (like pci hotplug removal of host controller) > we need to make sure we can trust the command timer to always return the > canceled URB Yes, you are right, we need to check these carefully. Suspend process, module unload and abrupt host removal, they all will issue usb_disconnect() firstly before clear the command queue, it will check URBs for every endpoint by usb_disconnect()--->usb_disable_device()--->usb_disable_endpoint(), which will make sure every URBs of endpoints will be cancelled by the stop endpoint command responding or the timeout function of stop endpoint command (xhci_stop_endpoint_command_timeout()) in usb_hcd_flush_endpoint(). From that point, we can make sure the command timer will be useful to handle stop endpoint command timeout. Please correct me if I said something wrong. Thanks. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 0/3] Try to connect hikey's usb phy to dwc2 driver
On Tue, Nov 22, 2016 at 7:46 PM, John Stultz wrote: > After earlier attempts[1] at submitting somewhat hackish fixes > to the dwc2 driver, I realized the core issue seemed to be the > overly simplistic phy driver. > > I've connected the phy-hi6220-usb.c driver to extcon so it now > gets connection and disconnection signals on the usb gadget > cable. And I modified the driver so it registers a usb-phy and > calls usb_gadget_vbus_connect/disconnect() appropriately. > > Unfortunately this doesn't quite work with the dwc2 driver, > so I've hacked that driver to allow it to function. > > With these changes, while likely not correct, things function > well, and I was able to drop two of the hackish fixups from the > earlier set. I still needed one patch to keep the usb bus from > suspending while in gadget mode, so I've included that in this > series. > > [1]: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1272880.html > > Feedback and guidance would be greatly appreciated! > > > John Stultz (3): > phy: phy-hi6220-usb: Wire up extconn support to hikey's phy driver > HACK: dwc2: force dual use of uphy and phy > usb: dwc2: Avoid suspending if we're in gadget mode Curious if there was any feedback on this patchset or the general approach? thanks -john -- 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 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
On 11/30/2016 4:47 AM, Felipe Balbi wrote: > > Hi, > > Raviteja Garimella writes: >> Hi Balbi, >> >> On Wed, Nov 30, 2016 at 4:10 PM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Raviteja Garimella writes: This is driver for Synopsys Designware Cores USB Device Controller (UDC) Subsystem with the AMBA Advanced High-Performance Bus (AHB). This driver works with Synopsys UDC20 products. Signed-off-by: Raviteja Garimella >>> >>> use drivers/usb/dwc2 instead of duplicating it. >> >> The ones we have in drivers/usb/dwc2 is for Designware high speed OTG >> controller IP. The one that I submitted for review is for USB Device >> controller IP (UDC). The IPs are different. > > I'll wait for John's confirmation that this really isn't compatible with > dwc2. John? > Hi Felipe, This is our older UDC IP, not compatible with HSOTG. It is also no longer supported by Synopsys and considered EOL. Regards, John -- 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 6/6] pinctrl: mt8173: set GPIO16 to usb iddig mode
On Wed, 2016-11-23 at 19:32 +0100, Matthias Brugger wrote: > Hi Hongzhou, > > On 12/05/16 04:55, Hongzhou Yang wrote: > > On Wed, 2016-05-11 at 19:09 -0700, Hongzhou Yang wrote: > >> On Thu, 2016-05-12 at 09:41 +0800, chunfeng yun wrote: > >>> Hi, > >>> > >>> On Wed, 2016-05-11 at 11:32 -0700, Hongzhou Yang wrote: > On Wed, 2016-05-11 at 13:56 +0200, Linus Walleij wrote: > > On Tue, May 10, 2016 at 10:23 AM, Chunfeng Yun > > wrote: > > > >> the default mode of GPIO16 pin is gpio, when set EINT16 to > >> IRQ_TYPE_LEVEL_HIGH, no interrupt is triggered, it can be > >> fixed when set its default mode as usb iddig. > >> > >> Signed-off-by: Chunfeng Yun > > > > Chunfeng, GPIO16 can be used as EINT16 mode, but the pinmux should be 0. > If you want to set its default mode to iddig, you should set it in dts. > > >>> I set it in DTS, but it didn't work, because when usb driver requested > >>> IRQ, pinmux was switched back to default mode set by > >>> MTK_EINT_FUNCTION(). > >>> > >> > >> After confirmed, there are something wrong with data sheet and pinmux > >> table, and GPIO16 can only receive interrupt by mode 1. So > >> > >> Acked-by: Hongzhou Yang > >> > > > > Linus, > > > > We find there are some other pins still have the same problem, so please > > hold on it. Sorry for so much noise. > > > > Did you made any progress on this? I didn't see any patch on the mailing > list. > > Regards, > Matthias Hi Matthias, Sorry for the late reply. I have double confirmed with HW designer, other special EINTs are built-in and they are using internal signal, they are not triggered by GPIO, only GPIO16 should set to mode 1. And, Chunfeng already re-sent this patch. Thank you very much. Yours, Hongzhou -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: Oops when unbinding an inactive gadget configfs configuration from UDC
Hello, > I had expected that writing "dummy_udc.0" to > /sys/kernel/config/usb_gadget/gser/UDC would fail, because the UDC is > already bound to another gadget configuration > (/sys/kernel/config/usb_gadget/acm/UDC). However, this doesn't give me > an error, but the ACM configuration remains active. After investigating this further, I came to the conclusion that the false success report after trying to bind the second gadget configuration to the UDC is the actual cause for the oops. See bugfix propsal for a more detailed description: [PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver() Regards, Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()
This fixes a regression which was introduced by commit f1bddbb, by reverting a small fragment of commit 855ed04. If the following conditions were met, usb_gadget_probe_driver() returned 0, although the call was unsuccessful: 1. A particular UDC was specified by thge gadget driver (using member "udc_name" of struct usb_gadget_driver). 2. The UDC with this name is available. 3. Another gadget driver is already bound to this gadget. 4. The gadget driver has the "match_existing_only" flag set. In this case, the return code variable "ret" is set to 0, the return code of a strcmp() call (to check for the second condition). This also fixes an oops which could occur in the following scenario: 1. Two usb gadget instances were configured using configfs. 2. The first gadget configuration was bound to a UDC (using the configfs attribute "UDC"). 3. It was tried to bind the second gadget configuration to the same UDC in the same way. This operation was then wrongly reported as being successful. 4. The second gadget configuration's "UDC" attribute is cleared, to unbind the (not really bound) second gadget configuration from the UDC. ] __list_del_entry+0x29/0xc0 PGD 41b4c5067 PUD 41a598067 PMD 0 Oops: [#1] SMP Modules linked in: cdc_acm usb_f_fs usb_f_serial usb_f_acm u_serial libcomposite configfs dummy_hcd bnep intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_hdmi irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd snd_hda_codec_realtek snd_hda_codec_generic serio_raw uvcvideo videobuf2_vmalloc btusb snd_usb_audio snd_hda_intel videobuf2_memops btrtl snd_hda_codec snd_hda_core snd_usbmidi_lib btbcm videobuf2_v4l2 btintel snd_hwdep videobuf2_core snd_seq_midi bluetooth snd_seq_midi_event videodev xpad efi_pstore snd_pcm_oss rfkill joydev media crc16 ff_memless snd_mixer_oss snd_rawmidi nls_ascii snd_pcm snd_seq snd_seq_device nls_cp437 mei_me snd_timer vfat sg udc_core lpc_ich fat efivars mfd_core mei snd soundcore battery nuvoton_cir rc_core evdev intel_smartconnect ie31200_edac edac_core shpchp tpm_tis tpm_tis_core tpm parport_pc ppdev lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom sd_mod ahci libahci nouveau i915 crc32c_intel i2c_algo_bit psmouse ttm xhci_pci libata scsi_mod ehci_pci drm_kms_helper xhci_hcd ehci_hcd r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280] CPU: 5 PID: 829 Comm: bash Not tainted 4.9.0-rc7 #1 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 task: 880419ce4040 task.stack: c90002ed4000 RIP: 0010:[] [] __list_del_entry+0x29/0xc0 RSP: 0018:c90002ed7d68 EFLAGS: 00010207 RAX: RBX: 88041787ec30 RCX: dead0200 RDX: RSI: 880417482002 RDI: 88041787ec30 RBP: c90002ed7d68 R08: R09: 0010 R10: R11: 880419ce4040 R12: 88041787eb68 R13: 88041787eaa8 R14: 88041560a2c0 R15: 0001 FS: 7fe4e49b8700() GS:88042f34() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00041b4c4000 CR4: 001406e0 Stack: c90002ed7d80 94f5e68d c0ae5ef0 c90002ed7da0 c0ae22aa 88041787e800 88041787e800 c90002ed7dc0 c0d7a727 952273fa 88041aba5760 c90002ed7df8 Call Trace: [] list_del+0xd/0x30 [] usb_gadget_unregister_driver+0xaa/0xc0 [udc_core] [] unregister_gadget+0x27/0x60 [libcomposite] [] ? mutex_lock+0x1a/0x30 [] gadget_dev_desc_UDC_store+0x88/0xe0 [libcomposite] [] configfs_write_file+0xa0/0x100 [configfs] [] __vfs_write+0x37/0x160 [] ? __fd_install+0x30/0xd0 [] ? _raw_spin_unlock+0xe/0x10 [] vfs_write+0xb8/0x1b0 [] SyS_write+0x58/0xc0 [] ? __close_fd+0x94/0xc0 [] entry_SYSCALL_64_fastpath+0x1e/0xad Code: 66 90 55 48 8b 07 48 b9 00 01 00 00 00 00 ad de 48 8b 57 08 48 89 e5 48 39 c8 74 29 48 b9 00 02 00 00 00 00 ad de 48 39 ca 74 3a <4c> 8b 02 4c 39 c7 75 52 4c 8b 40 08 4c 39 c7 75 66 48 89 50 08 RIP [] __list_del_entry+0x29/0xc0 RSP CR2: ---[ end trace 99fc090ab3ff6cbc ]--- Signed-off-by: Felix Hädicke --- drivers/usb/gadget/udc/core.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 9483489..0402177 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -1317,7 +1317,11 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver) if (!ret) break; } - if (!ret && !udc->driver) + if (ret) + ret = -ENODEV; + else if (udc->driver) + ret = -EBUSY; + else
Re: [PATCH net] cdc_ether: Fix handling connection notification
On Wed, Nov 30, 2016 at 10:51 PM, Bjørn Mork wrote:
> Kristian Evensen writes:
>
>> +void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
>> +{
>> + struct usb_cdc_notification *event;
>> +
>> + if (urb->actual_length < sizeof(*event))
>> + return;
>> +
>> + event = urb->transfer_buffer;
>> +
>> + if (event->bNotificationType != USB_CDC_NOTIFY_NETWORK_CONNECTION) {
>> + usbnet_cdc_status(dev, urb);
>> + return;
>> + }
>> +
>> + netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
>> + event->wValue ? "on" : "off");
>> +
>> + if (event->wValue &&
>> + !test_bit(__LINK_STATE_NOCARRIER, &dev->net->state))
>> + usbnet_link_change(dev, 0, 0);
>> +
>> + usbnet_link_change(dev, !!event->wValue, 0);
>> +}
>
> As Henning said: Use netif_carrier_ok instead of open coding it.
>
> But I also think you need to replace the first usbnet_link_change() with
> a plain netif_carrier_off(dev->net). Calling usbnet_link_change() twice
> here is only going to set off the "kevent XX may have been dropped"
> message since you call schedule_work() twice without giving the work
> queue a chance to be processed. No need to do that.
Thanks for the feedback and agree. Will submit a v2 tomorrow.
-Kristian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB OHCI controller
On Fri, Nov 25, 2016 at 12:47:28PM -0200, Sergio Prado wrote: > 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 Acked-by: Rob Herring -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net] cdc_ether: Fix handling connection notification
Kristian Evensen writes:
> +void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
> +{
> + struct usb_cdc_notification *event;
> +
> + if (urb->actual_length < sizeof(*event))
> + return;
> +
> + event = urb->transfer_buffer;
> +
> + if (event->bNotificationType != USB_CDC_NOTIFY_NETWORK_CONNECTION) {
> + usbnet_cdc_status(dev, urb);
> + return;
> + }
> +
> + netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
> + event->wValue ? "on" : "off");
> +
> + if (event->wValue &&
> + !test_bit(__LINK_STATE_NOCARRIER, &dev->net->state))
> + usbnet_link_change(dev, 0, 0);
> +
> + usbnet_link_change(dev, !!event->wValue, 0);
> +}
As Henning said: Use netif_carrier_ok instead of open coding it.
But I also think you need to replace the first usbnet_link_change() with
a plain netif_carrier_off(dev->net). Calling usbnet_link_change() twice
here is only going to set off the "kevent XX may have been dropped"
message since you call schedule_work() twice without giving the work
queue a chance to be processed. No need to do that.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: OHCI: ohci-pxa27x: remove unuseful functions
By the way, "unuseful" is not an English word. It should be "useless".
On Wed, 30 Nov 2016 csmanjuvi...@gmail.com wrote:
> From: Manjunath Goudar
>
> The ohci_hcd_pxa27x_drv_probe function is not doing anything other
> than calling usb_hcd_pxa27x_probe function so ohci_hcd_pxa27x_drv_probe
> function is unuseful that is why removed ohci_hcd_pxa27x_drv_probe
> function and renamed usb_hcd_pxa27x_probe function to
> ohci_hcd_pxa27x_drv_probe for proper naming.
>
> The ohci_hcd_pxa27x_remove function is also not doing anything other than
> calling usb_hcd_pxa27x_remove that is why removed ohci_hcd_pxa27x_remove
> function and renamed usb_hcd_pxa27x_remove to ohci_hcd_pxa27x_remove for
> proper naming.
A few things should be improved.
> Signed-off-by: Manjunath Goudar
> Cc: Alan Stern
> Cc: Greg Kroah-Hartman
> Cc: linux-usb@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> drivers/usb/host/ohci-pxa27x.c | 41 +++--
> 1 file changed, 15 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index a667cf2..c5efcd1 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -404,7 +404,7 @@ static int ohci_pxa_of_init(struct platform_device *pdev)
>
>
> /**
> - * usb_hcd_pxa27x_probe - initialize pxa27x-based HCDs
> + * ohci_hcd_pxa27x_probe - initialize pxa27x-based HCDs
> * Context: !in_interrupt()
> *
> * Allocates basic resources for this USB host controller, and
> @@ -412,7 +412,7 @@ static int ohci_pxa_of_init(struct platform_device *pdev)
> * through the hotplug entry's driver_data.
> *
> */
> -int usb_hcd_pxa27x_probe (const struct hc_driver *driver, struct
> platform_device *pdev)
> +static int ohci_hcd_pxa27x_probe (struct platform_device *pdev)
You should remove the space between "probe" and "(". checkpatch.pl
will warn you about this. Didn't you run it?
> {
> int retval, irq;
> struct usb_hcd *hcd;
> @@ -423,6 +423,11 @@ int usb_hcd_pxa27x_probe (const struct hc_driver
> *driver, struct platform_device
> struct clk *usb_clk;
> unsigned int i;
>
> + if (usb_disabled())
> + return -ENODEV;
> +
> + pr_debug ("In ohci_hcd_pxa27x_drv_probe");
> +
Leave these out. The driver already checks for usb_disabled() in
ohci_pxa27x_init(); there's no reason to check it again. And the debug
line seems rather unimportant.
> retval = ohci_pxa_of_init(pdev);
> if (retval)
> return retval;
> @@ -442,7 +447,7 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver,
> struct platform_device
> if (IS_ERR(usb_clk))
> return PTR_ERR(usb_clk);
>
> - hcd = usb_create_hcd (driver, &pdev->dev, "pxa27x");
> + hcd = usb_create_hcd (&ohci_pxa27x_hc_driver, &pdev->dev, "pxa27x");
Again, fix up warnings from checkpatch.pl. Here and below.
Alan Stern
> if (!hcd)
> return -ENOMEM;
>
> @@ -503,17 +508,18 @@ int usb_hcd_pxa27x_probe (const struct hc_driver
> *driver, struct platform_device
> /* may be called with controller, bus, and devices active */
>
> /**
> - * usb_hcd_pxa27x_remove - shutdown processing for pxa27x-based HCDs
> + * ohci_hcd_pxa27x_remove - shutdown processing for pxa27x-based HCDs
> * @dev: USB Host Controller being removed
> * Context: !in_interrupt()
> *
> - * Reverses the effect of usb_hcd_pxa27x_probe(), first invoking
> + * Reverses the effect of ohci_hcd_pxa27x_probe(), first invoking
> * the HCD's stop() method. It is always called from a thread
> * context, normally "rmmod", "apmd", or something similar.
> *
> */
> -void usb_hcd_pxa27x_remove (struct usb_hcd *hcd, struct platform_device
> *pdev)
> +static int ohci_hcd_pxa27x_remove (struct platform_device *pdev)
> {
> + struct usb_hcd *hcd = platform_get_drvdata(pdev);
> struct pxa27x_ohci *pxa_ohci = to_pxa27x_ohci(hcd);
> unsigned int i;
>
> @@ -524,28 +530,11 @@ void usb_hcd_pxa27x_remove (struct usb_hcd *hcd, struct
> platform_device *pdev)
> pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
>
> usb_put_hcd(hcd);
> + return 0;
> }
>
> /*-*/
>
> -static int ohci_hcd_pxa27x_drv_probe(struct platform_device *pdev)
> -{
> - pr_debug ("In ohci_hcd_pxa27x_drv_probe");
> -
> - if (usb_disabled())
> - return -ENODEV;
> -
> - return usb_hcd_pxa27x_probe(&ohci_pxa27x_hc_driver, pdev);
> -}
> -
> -static int ohci_hcd_pxa27x_drv_remove(struct platform_device *pdev)
> -{
> - struct usb_hcd *hcd = platform_get_drvdata(pdev);
> -
> - usb_hcd_pxa27x_remove(hcd, pdev);
> - return 0;
> -}
> -
> #ifdef CONFIG_PM
> static int ohci_hcd_pxa27x_drv_suspend(struct device *dev)
> {
> @@ -598,8 +587,8 @@ static const struct dev_pm_ops ohci_hcd_pxa27x_pm_ops = {
> #endif
>
> static struct platf
Re: [PATCH net] cdc_ether: Fix handling connection notification
Am Wed, 30 Nov 2016 19:42:16 +0100
schrieb Kristian Evensen :
> Commit bfe9b9d2df66 ("cdc_ether: Improve ZTE MF823/831/910 handling")
> introduced a work-around in usbnet_cdc_status() for devices that
> exported cdc carrier on twice on connect. Before the commit, this
> behavior caused the link state to be incorrect. It was assumed that
> all CDC Ethernet devices would either export this behavior, or send
> one off and then one on notification (which seems to be the default
> behavior).
>
> Unfortunately, it turns out multiple devices sends a connection
> notification multiple times per second (via an interrupt), even when
> connection state does not change. This has been observed with several
> different USB LAN dongles (at least), for example 13b1:0041 (Linksys).
> After bfe9b9d2df66, the link state has been set as down and then up
> for each notification. This has caused a flood of Netlink NEWLINK
> messages and syslog to be flooded with messages similar to:
>
> cdc_ether 2-1:2.0 eth1: kevent 12 may have been dropped
>
> This commit fixes the behavior by reverting usbnet_cdc_status() to
> how it was before bfe9b9d2df66. The work-around has been moved to a
> separate status-function which is only called when a known, affect
> device is detected.
>
> Fixes: bfe9b9d2df66 ("cdc_ether: Improve ZTE MF823/831/910 handling")
> Reported-by: Henning Schild
> Signed-off-by: Kristian Evensen
> ---
> drivers/net/usb/cdc_ether.c | 38
> +++--- 1 file changed, 31
> insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index 45e5e43..8c628ea 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -388,12 +388,6 @@ void usbnet_cdc_status(struct usbnet *dev,
> struct urb *urb) case USB_CDC_NOTIFY_NETWORK_CONNECTION:
> netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
> event->wValue ? "on" : "off");
> -
> - /* Work-around for devices with broken
> off-notifications */
> - if (event->wValue &&
> - !test_bit(__LINK_STATE_NOCARRIER,
> &dev->net->state))
> - usbnet_link_change(dev, 0, 0);
> -
> usbnet_link_change(dev, !!event->wValue, 0);
> break;
> case USB_CDC_NOTIFY_SPEED_CHANGE: /* tx/rx rates */
> @@ -466,6 +460,36 @@ static int usbnet_cdc_zte_rx_fixup(struct usbnet
> *dev, struct sk_buff *skb) return 1;
> }
>
> +/* Ensure correct link state
> + *
> + * Some devices (ZTE MF823/831/910) export two carrier on
> notifications when
> + * connected. This causes the link state to be incorrect. Work
> around this by
> + * always setting the state to off, then on.
> + */
> +void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
> +{
> + struct usb_cdc_notification *event;
> +
> + if (urb->actual_length < sizeof(*event))
> + return;
> +
> + event = urb->transfer_buffer;
> +
> + if (event->bNotificationType !=
> USB_CDC_NOTIFY_NETWORK_CONNECTION) {
> + usbnet_cdc_status(dev, urb);
> + return;
> + }
> +
> + netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
> + event->wValue ? "on" : "off");
> +
> + if (event->wValue &&
> + !test_bit(__LINK_STATE_NOCARRIER, &dev->net->state))
You should probably use netif_carrier_ok(dev->net) instead.
> + usbnet_link_change(dev, 0, 0);
> +
> + usbnet_link_change(dev, !!event->wValue, 0);
Would the following work?
usbnet_link_change(dev, !!event->wValue, event->wValue &&
netif_carrier_ok(dev->net));
> +}
> +
> static const struct driver_info cdc_info = {
> .description = "CDC Ethernet Device",
> .flags =FLAG_ETHER | FLAG_POINTTOPOINT,
> @@ -481,7 +505,7 @@ static const struct driver_info
> zte_cdc_info = { .flags = FLAG_ETHER | FLAG_POINTTOPOINT,
> .bind = usbnet_cdc_zte_bind,
> .unbind = usbnet_cdc_unbind,
> - .status = usbnet_cdc_status,
> + .status = usbnet_cdc_zte_status,
> .set_rx_mode = usbnet_cdc_update_filter,
> .manage_power = usbnet_manage_power,
> .rx_fixup = usbnet_cdc_zte_rx_fixup,
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net] cdc_ether: Fix handling connection notification
Commit bfe9b9d2df66 ("cdc_ether: Improve ZTE MF823/831/910 handling")
introduced a work-around in usbnet_cdc_status() for devices that exported
cdc carrier on twice on connect. Before the commit, this behavior caused
the link state to be incorrect. It was assumed that all CDC Ethernet
devices would either export this behavior, or send one off and then one on
notification (which seems to be the default behavior).
Unfortunately, it turns out multiple devices sends a connection
notification multiple times per second (via an interrupt), even when
connection state does not change. This has been observed with several
different USB LAN dongles (at least), for example 13b1:0041 (Linksys).
After bfe9b9d2df66, the link state has been set as down and then up for
each notification. This has caused a flood of Netlink NEWLINK messages and
syslog to be flooded with messages similar to:
cdc_ether 2-1:2.0 eth1: kevent 12 may have been dropped
This commit fixes the behavior by reverting usbnet_cdc_status() to how it
was before bfe9b9d2df66. The work-around has been moved to a separate
status-function which is only called when a known, affect device is
detected.
Fixes: bfe9b9d2df66 ("cdc_ether: Improve ZTE MF823/831/910 handling")
Reported-by: Henning Schild
Signed-off-by: Kristian Evensen
---
drivers/net/usb/cdc_ether.c | 38 +++---
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 45e5e43..8c628ea 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -388,12 +388,6 @@ void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
case USB_CDC_NOTIFY_NETWORK_CONNECTION:
netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
event->wValue ? "on" : "off");
-
- /* Work-around for devices with broken off-notifications */
- if (event->wValue &&
- !test_bit(__LINK_STATE_NOCARRIER, &dev->net->state))
- usbnet_link_change(dev, 0, 0);
-
usbnet_link_change(dev, !!event->wValue, 0);
break;
case USB_CDC_NOTIFY_SPEED_CHANGE: /* tx/rx rates */
@@ -466,6 +460,36 @@ static int usbnet_cdc_zte_rx_fixup(struct usbnet *dev,
struct sk_buff *skb)
return 1;
}
+/* Ensure correct link state
+ *
+ * Some devices (ZTE MF823/831/910) export two carrier on notifications when
+ * connected. This causes the link state to be incorrect. Work around this by
+ * always setting the state to off, then on.
+ */
+void usbnet_cdc_zte_status(struct usbnet *dev, struct urb *urb)
+{
+ struct usb_cdc_notification *event;
+
+ if (urb->actual_length < sizeof(*event))
+ return;
+
+ event = urb->transfer_buffer;
+
+ if (event->bNotificationType != USB_CDC_NOTIFY_NETWORK_CONNECTION) {
+ usbnet_cdc_status(dev, urb);
+ return;
+ }
+
+ netif_dbg(dev, timer, dev->net, "CDC: carrier %s\n",
+ event->wValue ? "on" : "off");
+
+ if (event->wValue &&
+ !test_bit(__LINK_STATE_NOCARRIER, &dev->net->state))
+ usbnet_link_change(dev, 0, 0);
+
+ usbnet_link_change(dev, !!event->wValue, 0);
+}
+
static const struct driver_infocdc_info = {
.description = "CDC Ethernet Device",
.flags =FLAG_ETHER | FLAG_POINTTOPOINT,
@@ -481,7 +505,7 @@ static const struct driver_info zte_cdc_info = {
.flags =FLAG_ETHER | FLAG_POINTTOPOINT,
.bind = usbnet_cdc_zte_bind,
.unbind = usbnet_cdc_unbind,
- .status = usbnet_cdc_status,
+ .status = usbnet_cdc_zte_status,
.set_rx_mode = usbnet_cdc_update_filter,
.manage_power = usbnet_manage_power,
.rx_fixup = usbnet_cdc_zte_rx_fixup,
--
2.9.3
--
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: cdc_ether flooding kernel logs since bfe9b9d2d
Hi, On Wed, Nov 30, 2016 at 4:17 PM, Oliver Neukum wrote: > On Tue, 2016-11-29 at 14:41 +0100, Kristian Evensen wrote: >> I don't know which of the two behaviors (if any) are "correct" or what >> the best way to solve this problem is. One options is to remove the >> code from the generic usbnet_cdc_status() function and move it to a >> ZTE-specific status function inside cdc_ether. > > Hi, > > please do so. This looks like the cleanest approach. Ok, great. I will do this right away and submit a patch tonight. Again, sorry for this. -Kristian -- 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: cdc_ether flooding kernel logs since bfe9b9d2d
On Tue, 2016-11-29 at 14:41 +0100, Kristian Evensen wrote: > I don't know which of the two behaviors (if any) are "correct" or what > the best way to solve this problem is. One options is to remove the > code from the generic usbnet_cdc_status() function and move it to a > ZTE-specific status function inside cdc_ether. Hi, please do so. This looks like the cleanest approach. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] fsl/usb: Workarourd for USB erratum-A005697
On Tue, 29 Nov 2016, Changming Huang wrote: > The EHCI specification states the following in the SUSP bit description: > In the Suspend state, the port is sensitive to resume detection. > Note that the bit status does not change until the port is suspended and > that there may be a delay in suspending 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 > --- > Changes in v4: > - release spinlock before sleeping > Changes in v3: > - add 10ms delay in function ehci_hub_control > - fix typos > Changes in v2: > - move sleep out of spin-lock > - add more comment for this workaround Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: no error logs in case of enomem
On Wed, Nov 30, 2016 at 02:32:07PM +, Atul Raj wrote: > All kmalloc-based functions print enough information on failures. > > Signed-off-by: Atul Raj > --- > drivers/usb/core/hub.c | 1 - > 1 file changed, 1 deletion(-) I told you to wait, relax, and work on drivers/staging/ instead. Sorry, I'm not going to take this as obviously you are not following directions very well and are now wasting other people's time for no good reason. 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
[PATCH] usb: hub: no error logs in case of enomem
All kmalloc-based functions print enough information on failures.
Signed-off-by: Atul Raj
---
drivers/usb/core/hub.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cbb1467..82059f26 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -817,7 +817,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
*/
clear = kmalloc(sizeof *clear, GFP_ATOMIC);
if (clear == NULL) {
- dev_err(&udev->dev, "can't save CLEAR_TT_BUFFER state\n");
/* FIXME recover somehow ... RESET_TT? */
return -ENOMEM;
}
--
2.10.2.windows.1
Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
On 30.11.2016 11:02, Baolin Wang wrote: If the hardware never responds to the stop endpoint command, the URBs will never be completed, and we might hang the USB subsystem. The original watchdog timer is used to watch if one stop endpoint command is timeout, if timeout, then the watchdog timer will set XHCI_STATE_DYING, try to halt the xHCI host, and give back all pending URBs. But now we already have one command timer to control command timeout, thus we can also use the command timer to watch the stop endpoint command, instead of one duplicate watchdog timer which need to be removed. Meanwhile we don't need the 'stop_cmds_pending' flag to identy if this is the last stop endpoint command of one endpoint. Since we can make sure we only set one stop endpoint command for one endpoint by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove this flag. We also need to clean up the command queue before trying to halt the xHCI host in xhci_stop_endpoint_command_timeout() function. This isn't a bad idea. There are anyway some corner cases and details that need to be checked, such as suspend (which will clear the command queue), module unload and abrupt host removal (like pci hotplug removal of host controller) we need to make sure we can trust the command timer to always return the canceled URB -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: return error code when platform_get_irq fails
On 30.11.2016 15:41, Matthias Brugger wrote:
On 29/11/16 13:57, Pan Bian wrote:
In function xhci_mtk_probe(), variable ret takes the return value. Its
value should be negative on failures. However, when the call to function
platform_get_irq() fails, it does not set the error code, and 0 will be
returned. 0 indicates no error. As a result, the callers of function
xhci_mtk_probe() will not be able to detect the error. This patch fixes
the bug by assigning the return value of platform_get_irq() to variable
ret if it fails.
Signed-off-by: Pan Bian
---
drivers/usb/host/xhci-mtk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 79959f1..f2365a4 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -560,8 +560,10 @@ static int xhci_mtk_probe(struct platform_device *pdev)
goto disable_ldos;
irq = platform_get_irq(pdev, 0);
-if (irq < 0)
+if (irq < 0) {
+ret = irq;
goto disable_clk;
+}
/* Initialize dma_mask and coherent_dma_mask to 32-bits */
ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
Reviewed-by: Matthias Brugger
Thanks, Added to queue
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: replace symbolic permission with octal
On Wed, Nov 30, 2016 at 01:49:02PM +, Amit Kumar Kushwaha wrote: > This patch handles warning message for preferring octal > permissions over Symbolic permission for module parameter That makes no sense at all. What warning message? Build time? Run time? Please do cleanup patches on drivers/staging/ until you know how to do them, don't start on the "core" kernel, as you will just annoy maintainers (hint, you cc:ed too many people...) 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
[PATCH] usb: host: replace symbolic permission with octal
This patch handles warning message for preferring octal permissions over Symbolic permission for module parameter Signed-off-by: Amit Kushwaha --- drivers/usb/host/xhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1a4ca02..a953d47 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -40,11 +40,11 @@ /* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared */ static int link_quirk; -module_param(link_quirk, int, S_IRUGO | S_IWUSR); +module_param(link_quirk, int, 0644); MODULE_PARM_DESC(link_quirk, "Don't clear the chain bit on a link TRB"); static unsigned int quirks; -module_param(quirks, uint, S_IRUGO); +module_param(quirks, uint, 0444); MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default"); /* TODO: copied from ehci-hcd.c - can this be refactored? */ -- 2.10.2.windows.1
Re: [PATCH 1/1] usb: return error code when platform_get_irq fails
On 29/11/16 13:57, Pan Bian wrote:
In function xhci_mtk_probe(), variable ret takes the return value. Its
value should be negative on failures. However, when the call to function
platform_get_irq() fails, it does not set the error code, and 0 will be
returned. 0 indicates no error. As a result, the callers of function
xhci_mtk_probe() will not be able to detect the error. This patch fixes
the bug by assigning the return value of platform_get_irq() to variable
ret if it fails.
Signed-off-by: Pan Bian
---
drivers/usb/host/xhci-mtk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 79959f1..f2365a4 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -560,8 +560,10 @@ static int xhci_mtk_probe(struct platform_device *pdev)
goto disable_ldos;
irq = platform_get_irq(pdev, 0);
- if (irq < 0)
+ if (irq < 0) {
+ ret = irq;
goto disable_clk;
+ }
/* Initialize dma_mask and coherent_dma_mask to 32-bits */
ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
Reviewed-by: Matthias Brugger
Next time please make sure to send the patch to all relevant lists and
people (get_maintainer.pl).
Regards,
Matthias
--
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 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
Hi, Raviteja Garimella writes: > Hi Balbi, > > On Wed, Nov 30, 2016 at 4:10 PM, Felipe Balbi wrote: >> >> Hi, >> >> Raviteja Garimella writes: >>> This is driver for Synopsys Designware Cores USB Device >>> Controller (UDC) Subsystem with the AMBA Advanced High-Performance >>> Bus (AHB). This driver works with Synopsys UDC20 products. >>> >>> Signed-off-by: Raviteja Garimella >> >> use drivers/usb/dwc2 instead of duplicating it. > > The ones we have in drivers/usb/dwc2 is for Designware high speed OTG > controller IP. The one that I submitted for review is for USB Device > controller IP (UDC). The IPs are different. I'll wait for John's confirmation that this really isn't compatible with dwc2. John? -- balbi signature.asc Description: PGP signature
Re: [PATCH 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
Hi Balbi, On Wed, Nov 30, 2016 at 4:10 PM, Felipe Balbi wrote: > > Hi, > > Raviteja Garimella writes: >> This is driver for Synopsys Designware Cores USB Device >> Controller (UDC) Subsystem with the AMBA Advanced High-Performance >> Bus (AHB). This driver works with Synopsys UDC20 products. >> >> Signed-off-by: Raviteja Garimella > > use drivers/usb/dwc2 instead of duplicating it. The ones we have in drivers/usb/dwc2 is for Designware high speed OTG controller IP. The one that I submitted for review is for USB Device controller IP (UDC). The IPs are different. Thanks, Ravi > > -- > balbi -- 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
Mark Lord [...] > > 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. I have tested it with iperf more than five days without any error. I would think if there is any other way to reproduce it. Best Regards, Hayes -- 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 2/4] usb: host: xhci: handle COMP_STOP from SETUP phase too
Hi,
Felipe Balbi writes:
> Stop Endpoint command can come at any point and we
> have no control of that. We should make sure to
> handle COMP_STOP on SETUP phase as well, otherwise
> urb->actual_lenght might be set to negative values
> in some occasions such as below:
>
> urb->length = 4;
> build_control_transfer_td_for(urb, ep);
>
> stop_endpoint(ep);
>
> COMP_STOP:
> [...]
> urb->actual_length = urb->length - trb->length;
>
> trb->length is 8 for SETUP stage (8 control request
> bytes), so actual_length would be set to -4 in this
> case.
>
> While doing that, also make sure to use TRB_TYPE
> field of the actual TRB instead of matching pointers
> to figure out in which stage of the control transfer
> we got our completion event.
>
> Signed-off-by: Felipe Balbi
> ---
> drivers/usb/host/xhci-ring.c | 30 ++
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 76402b44f832..70067b81cc8f 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1939,8 +1939,9 @@ static int process_ctrl_td(struct xhci_hcd *xhci,
> struct xhci_td *td,
> struct xhci_ep_ctx *ep_ctx;
> u32 trb_comp_code;
> u32 remaining, requested;
> - bool on_data_stage;
> + u32 trb_type;
>
> + trb_type = TRB_FIELD_TO_TYPE(le32_to_cpu(ep_trb->generic.field[3]));
> slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
> xdev = xhci->devs[slot_id];
> ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1;
> @@ -1950,14 +1951,11 @@ static int process_ctrl_td(struct xhci_hcd *xhci,
> struct xhci_td *td,
> requested = td->urb->transfer_buffer_length;
> remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
>
> - /* not setup (dequeue), or status stage means we are at data stage */
> - on_data_stage = (ep_trb != ep_ring->dequeue && ep_trb != td->last_trb);
> -
> switch (trb_comp_code) {
> case COMP_SUCCESS:
> - if (ep_trb != td->last_trb) {
> + if (trb_type != TRB_STATUS) {
> xhci_warn(xhci, "WARN: Success on ctrl %s TRB without
> IOC set?\n",
> - on_data_stage ? "data" : "setup");
> + (trb_type == TRB_DATA) ? "data" :
> "setup");
> *status = -ESHUTDOWN;
> break;
> }
> @@ -1967,15 +1965,23 @@ static int process_ctrl_td(struct xhci_hcd *xhci,
> struct xhci_td *td,
> *status = 0;
> break;
> case COMP_STOP_SHORT:
> - if (on_data_stage)
> + if (trb_type == TRB_DATA)
> td->urb->actual_length = remaining;
> else
> xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl
> setup or status TRB\n");
> goto finish_td;
> case COMP_STOP:
> - if (on_data_stage)
> + switch (trb_type) {
> + case TRB_SETUP:
> + td->urb->actual_length = 0;
> + goto finish_td;
> + case TRB_DATA:
there's a detail to fix here. Data stage can be composed of several
TRBs, in that case we will have one TRB_DATA and N TRB_NORMAL, so we
need to handle that too.
I'll update this patch (and consequently all other patches I've written)
and resend everything as a single series. Before doing that, however,
I'll wait a few more days for comments on any of the patches I've sent.
--
balbi
signature.asc
Description: PGP signature
Re: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first
On 28.11.2016 22:24, Guenter Roeck wrote: On Wed, Nov 23, 2016 at 02:24:27PM +0200, Mathias Nyman wrote: the tt_info provided by a HS hub might be in use to by a child device Make sure we free the devices in the correct order. This is needed in special cases such as when xhci controller is reset when resuming from hibernate, and all virt_devices are freed. Also free the virt_devices starting from max slot_id as children more commonly have higher slot_id than parent. CC: Signed-off-by: Mathias Nyman --- Guenter Roeck, does this work for you? Yes, it does. Tested-by: Guenter Roeck Thanks, Guenter Thanks, I'll send it forward with proper Reported-by and Tested-by tags after 4.10-rc1 -Mathias -- 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 12/12] usb: renesas_usbhs: Replace the deprecated extcon API
Hi, Chanwoo Choi writes: > This patch replaces the deprecated extcon API as following: > - extcon_get_cable_state_() -> extcon_get_state() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
[PATCH v6 4/4] usb: xhci: Add port test modes support for usb2.
For usb2 ports, the port test mode Test_J_State, Test_K_State,
Test_Packet, Test_SE0_NAK and Test_Force_En can be enabled
as described in usb2 spec.
Signed-off-by: Guoqing Zhang
---
drivers/usb/host/xhci-hub.c | 84 +
drivers/usb/host/xhci.h | 2 ++
2 files changed, 86 insertions(+)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index e6d6396..c53b59e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -569,6 +569,76 @@ static void xhci_set_port_power(struct xhci_hcd *xhci,
struct usb_hcd *hcd, u16
spin_lock_irqsave(&xhci->lock, flags);
}
+static void xhci_port_set_test_mode(struct xhci_hcd *xhci,
+ u16 test_mode, u16 wIndex)
+{
+ u32 temp;
+ __le32 __iomem *addr;
+
+ /* xhci only supports test mode for usb2 ports, i.e. xhci->main_hcd */
+ addr = xhci_get_port_io_addr(xhci->main_hcd, wIndex);
+ temp = readl(addr + PORTPMSC);
+ temp |= test_mode << PORT_TEST_MODE_SHIFT;
+ writel(temp, addr + PORTPMSC);
+ xhci->test_mode = test_mode;
+ if (test_mode == TEST_FORCE_EN)
+ xhci_start(xhci);
+}
+
+static int xhci_enter_test_mode(struct xhci_hcd *xhci,
+ u16 test_mode, u16 wIndex)
+{
+ int i, retval;
+
+ /* Disable all Device Slots */
+ xhci_dbg(xhci, "Disable all slots\n");
+ for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+ retval = xhci_disable_slot(xhci, NULL, i);
+ if (retval)
+ xhci_err(xhci, "Failed to disable slot %d, %d. Enter
test mode anyway\n", i, retval);
+ }
+ /* Put all ports to the Disable state by clear PP */
+ xhci_dbg(xhci, "Disable all port (PP = 0)\n");
+ /* Power off USB3 ports*/
+ for (i = 0; i < xhci->num_usb3_ports; i++)
+ xhci_set_port_power(xhci, xhci->shared_hcd, i, false);
+ /* Power off USB2 ports*/
+ for (i = 0; i < xhci->num_usb2_ports; i++)
+ xhci_set_port_power(xhci, xhci->main_hcd, i, false);
+ /* Stop the controller */
+ xhci_dbg(xhci, "Stop controller\n");
+ retval = xhci_halt(xhci);
+ if (retval)
+ return retval;
+ /* Disable runtime PM for test mode */
+ pm_runtime_forbid(xhci_to_hcd(xhci)->self.controller);
+ /* Set PORTPMSC.PTC field to enter selected test mode */
+ /* Port is selected by wIndex. port_id = wIndex + 1 */
+ xhci_dbg(xhci, "Enter Test Mode: %d, Port_id=%d\n",
+ test_mode, wIndex + 1);
+ xhci_port_set_test_mode(xhci, test_mode, wIndex);
+ return retval;
+}
+
+static int xhci_exit_test_mode(struct xhci_hcd *xhci)
+{
+ int retval;
+
+ if (!xhci->test_mode) {
+ xhci_err(xhci, "Not in test mode, do nothing.\n");
+ return 0;
+ }
+ if (xhci->test_mode == TEST_FORCE_EN &&
+ !(xhci->xhc_state & XHCI_STATE_HALTED)) {
+ retval = xhci_halt(xhci);
+ if (retval)
+ return retval;
+ }
+ pm_runtime_allow(xhci_to_hcd(xhci)->self.controller);
+ xhci->test_mode = 0;
+ return xhci_reset(xhci);
+}
+
void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
int port_id, u32 link_state)
{
@@ -924,6 +994,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
u16 link_state = 0;
u16 wake_mask = 0;
u16 timeout = 0;
+ u16 test_mode = 0;
max_ports = xhci_get_ports(hcd, &port_array);
bus_state = &xhci->bus_state[hcd_index(hcd)];
@@ -997,6 +1068,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16
wValue,
link_state = (wIndex & 0xff00) >> 3;
if (wValue == USB_PORT_FEAT_REMOTE_WAKE_MASK)
wake_mask = wIndex & 0xff00;
+ if (wValue == USB_PORT_FEAT_TEST)
+ test_mode = (wIndex & 0xff00) >> 8;
/* The MSB of wIndex is the U1/U2 timeout */
timeout = (wIndex & 0xff00) >> 8;
wIndex &= 0xff;
@@ -1161,6 +1234,14 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
temp |= PORT_U2_TIMEOUT(timeout);
writel(temp, port_array[wIndex] + PORTPMSC);
break;
+ case USB_PORT_FEAT_TEST:
+ /* 4.19.6 Port Test Modes (USB2 Test Mode) */
+ if (hcd->speed != HCD_USB2)
+ goto error;
+ if (test_mode > TEST_FORCE_EN || test_mode < TEST_J)
+ goto error;
+ retval = xhci_enter_test_mode(xhci, test_mode, wIndex);
+ break;
default:
[PATCH v6 3/4] usb: xhci: Expose xhci_start() function.
Change the visability of xhci_start() so that it
can be used when enabling test mode.
Signed-off-by: Guoqing Zhang
---
drivers/usb/host/xhci.c | 2 +-
drivers/usb/host/xhci.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 63055ee..2eec9ee 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -125,7 +125,7 @@ int xhci_halt(struct xhci_hcd *xhci)
/*
* Set the run bit and wait for the host to be running.
*/
-static int xhci_start(struct xhci_hcd *xhci)
+int xhci_start(struct xhci_hcd *xhci)
{
u32 temp;
int ret;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 3589a01..d13a0ed 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1844,6 +1844,7 @@ typedef void (*xhci_get_quirks_t)(struct device *, struct
xhci_hcd *);
int xhci_handshake(void __iomem *ptr, u32 mask, u32 done, int usec);
void xhci_quiesce(struct xhci_hcd *xhci);
int xhci_halt(struct xhci_hcd *xhci);
+int xhci_start(struct xhci_hcd *xhci);
int xhci_reset(struct xhci_hcd *xhci);
int xhci_init(struct usb_hcd *hcd);
int xhci_run(struct usb_hcd *hcd);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] usb: phy: qcom-8x16-usb: Replace the extcon API
Hi, Chanwoo Choi writes: > This patch uses the resource-managed extcon API for extcon_register_notifier() > and replaces the deprecated extcon API as following: > - extcon_get_cable_state_() -> extcon_get_state() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
[PATCH v6 2/4] usb: xhci: Add helper function xhci_disable_slot().
Refactoring slot disable related code into a helper
function xhci_disable_slot() which can be used when
enabling test mode.
Signed-off-by: Guoqing Zhang
---
drivers/usb/host/xhci.c | 49 +++--
drivers/usb/host/xhci.h | 2 ++
2 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 846df95..63055ee 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3588,8 +3588,6 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device
*udev)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_virt_device *virt_dev;
- unsigned long flags;
- u32 state;
int i, ret;
struct xhci_command *command;
@@ -3624,30 +3622,50 @@ void xhci_free_dev(struct usb_hcd *hcd, struct
usb_device *udev)
del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
}
+ xhci_disable_slot(xhci, command, udev->slot_id);
+ /*
+* Event command completion handler will free any data structures
+* associated with the slot. XXX Can free sleep?
+*/
+}
+
+int xhci_disable_slot(struct xhci_hcd *xhci, struct xhci_command *command,
+ u32 slot_id)
+{
+ unsigned long flags;
+ u32 state;
+ int ret = 0;
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci->devs[slot_id];
+ if(!virt_dev)
+ return -EINVAL;
+ if (!command)
+ command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+ if (!command)
+ return -ENOMEM;
+
spin_lock_irqsave(&xhci->lock, flags);
/* Don't disable the slot if the host controller is dead. */
state = readl(&xhci->op_regs->status);
if (state == 0x || (xhci->xhc_state & XHCI_STATE_DYING) ||
(xhci->xhc_state & XHCI_STATE_HALTED)) {
- xhci_free_virt_device(xhci, udev->slot_id);
+ xhci_free_virt_device(xhci, slot_id);
spin_unlock_irqrestore(&xhci->lock, flags);
kfree(command);
- return;
+ return ret;
}
- if (xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
- udev->slot_id)) {
+ ret = xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
+ slot_id);
+ if (ret) {
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_dbg(xhci, "FIXME: allocate a command ring segment\n");
- return;
+ return ret;
}
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
-
- /*
-* Event command completion handler will free any data structures
-* associated with the slot. XXX Can free sleep?
-*/
+ return ret;
}
/*
@@ -3755,14 +3773,9 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct
usb_device *udev)
disable_slot:
/* Disable slot, if we can do it without mem alloc */
- spin_lock_irqsave(&xhci->lock, flags);
command->completion = NULL;
command->status = 0;
- if (!xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
-udev->slot_id))
- xhci_ring_cmd_db(xhci);
- spin_unlock_irqrestore(&xhci->lock, flags);
- return 0;
+ return xhci_disable_slot(xhci, command, udev->slot_id);
}
/*
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index b2c1dc5..3589a01 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1852,6 +1852,8 @@ void xhci_shutdown(struct usb_hcd *hcd);
int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks);
void xhci_init_driver(struct hc_driver *drv,
const struct xhci_driver_overrides *over);
+int xhci_disable_slot(struct xhci_hcd *xhci,
+ struct xhci_command *command, u32 slot_id);
#ifdef CONFIG_PM
int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/12] usb: phy: tahvo: Replace the deprecated extcon API
Hi, Chanwoo Choi writes: > This patch replaces the deprecated extcon API as following: > - extcon_set_cable_state_() -> extcon_set_state_sync() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
[PATCH v6 1/4] usb: xhci: Add helper function xhci_set_power_on().
Refactoring port power on/off related code into
a helper function xhci_set_power_on() which can
be reused when enabling test mode.
Signed-off-by: Guoqing Zhang
---
drivers/usb/host/xhci-hub.c | 64 ++---
1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index d61fcc4..e6d6396 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -528,6 +528,47 @@ static int xhci_get_ports(struct usb_hcd *hcd, __le32
__iomem ***port_array)
return max_ports;
}
+static __le32 __iomem *xhci_get_port_io_addr(struct usb_hcd *hcd, int index)
+{
+ __le32 __iomem **port_array;
+
+ xhci_get_ports(hcd, &port_array);
+ return port_array[index];
+}
+
+/*
+ * xhci_set_port_power() must be called with xhci->lock held.
+ * It will release and re-aquire the lock while calling ACPI
+ * method.
+ */
+static void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd,
u16 index, bool on)
+{
+ __le32 __iomem *addr;
+ u32 temp;
+ unsigned long flags = 0;
+
+ addr = xhci_get_port_io_addr(hcd, index);
+ temp = readl(addr);
+ if (on) {
+ /* Power on */
+ writel(temp | PORT_POWER, addr);
+ temp = readl(addr);
+ xhci_dbg(xhci, "set port power, actual port %d status =
0x%x\n",
+ index, temp);
+ } else {
+ /* Power off */
+ writel(temp & ~PORT_POWER, addr);
+ }
+
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ temp = usb_acpi_power_manageable(hcd->self.root_hub,
+ index);
+ if (temp)
+ usb_acpi_set_power_state(hcd->self.root_hub,
+ index, on);
+ spin_lock_irqsave(&xhci->lock, flags);
+}
+
void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
int port_id, u32 link_state)
{
@@ -1081,18 +1122,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
* However, hub_wq will ignore the roothub events until
* the roothub is registered.
*/
- writel(temp | PORT_POWER, port_array[wIndex]);
-
- temp = readl(port_array[wIndex]);
- xhci_dbg(xhci, "set port power, actual port %d status
= 0x%x\n", wIndex, temp);
-
- spin_unlock_irqrestore(&xhci->lock, flags);
- temp = usb_acpi_power_manageable(hcd->self.root_hub,
- wIndex);
- if (temp)
- usb_acpi_set_power_state(hcd->self.root_hub,
- wIndex, true);
- spin_lock_irqsave(&xhci->lock, flags);
+ xhci_set_port_power(xhci, hcd, wIndex, true);
break;
case USB_PORT_FEAT_RESET:
temp = (temp | PORT_RESET);
@@ -1196,15 +1226,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
port_array[wIndex], temp);
break;
case USB_PORT_FEAT_POWER:
- writel(temp & ~PORT_POWER, port_array[wIndex]);
-
- spin_unlock_irqrestore(&xhci->lock, flags);
- temp = usb_acpi_power_manageable(hcd->self.root_hub,
- wIndex);
- if (temp)
- usb_acpi_set_power_state(hcd->self.root_hub,
- wIndex, false);
- spin_lock_irqsave(&xhci->lock, flags);
+ xhci_set_port_power(xhci, hcd, wIndex, false);
break;
default:
goto error;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] usb: phy: omap-otg: Replace the extcon API
Hi, Chanwoo Choi writes: > This patch uses the resource-managed extcon API for extcon_register_notifier() > and replaces the deprecated extcon API as following: > - extcon_get_cable_state_() -> extcon_get_state() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH 08/12] usb: phy: msm: Replace the extcon API
Hi, Chanwoo Choi writes: > This patch uses the resource-managed extcon API for extcon_register_notifier() > and replaces the deprecated extcon API as following: > - extcon_get_cable_state_() -> extcon_get_state() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
Hi, Raviteja Garimella writes: > This is driver for Synopsys Designware Cores USB Device > Controller (UDC) Subsystem with the AMBA Advanced High-Performance > Bus (AHB). This driver works with Synopsys UDC20 products. > > Signed-off-by: Raviteja Garimella use drivers/usb/dwc2 instead of duplicating it. -- balbi signature.asc Description: PGP signature
Re: [PATCH 06/12] usb: dwc3: omap: Replace the extcon API
Hi, Chanwoo Choi writes: > This patch uses the resource-managed extcon API for extcon_register_notifier() > and replaces the deprecated extcon API as following: > - extcon_get_cable_state_() -> extcon_get_state() > > Signed-off-by: Chanwoo Choi Acked-by: Felipe Balbi -- balbi signature.asc Description: PGP signature
Re: [PATCH 0/2] USB: serial: kl5kusb105: fix open error paths
On Tue, Nov 29, 2016 at 04:55:00PM +0100, Johan Hovold wrote: > Pan Bian found an issue with the kl5kusb105 open error handling, which > would not abort an open attempt when a vendor command to "enable read" > failed. > > Turns out there were more issues with this function, specifically any > urbs submitted would not be killed before returning on failures to > retrieve the line status. > > I fixed up the latter and rebased the Pan Bian's patch on top. > Johan Hovold (1): > USB: serial: kl5kusb105: fix open error path > > Pan Bian (1): > USB: serial: kl5kusb105: abort on open exception path Now applied for -next. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv13 2/3] usb: USB Type-C connector class
Hi Greg,
On Tue, Nov 29, 2016 at 05:27:44PM +0100, Greg KH wrote:
> > +struct typec_cable {
> > + struct device dev;
> > + enum typec_plug_typetype;
> > + u32 vdo;
> > + unsigned intusb_pd:1;
> > + unsigned intactive:1;
> > + unsigned intsop_pp_controller:1;
> > +
> > + struct typec_plug plug[2];
>
> WTF???
>
> Think about what this structure now represents. You have 3 different
> reference counted objects, all embedded in the same structure. Who
> "owns" the lifecycle of it? What happens if plug[1]'s reference count
> is grabbed a bunch by someone reading a lot of files for it, and then
> the "larger" typec_cable.dev reference count is decremented to 0 because
> the core is done with it. oops, boom, ick, splat, and if you are lucky
> the device reboots itself, if not, someone just got root and read your
> bank account information...
I have to ask. How could that happen since the cable is the parent?
> I'm being harsh here because this is really really really badly designed
Don't worry about it, I can handle it. In fact, one could argue that I
like getting slapped by you based on the comments I keep getting, but
I assure you that is not the case ;-)
> code. Go back and think about your data structures, what they are
> trying to represent, and _WHO_ owns and controls them. The typec core
> should be the one that allocates and manages the lifecycle of them, not
> some random external entity where you just hope and pray that they got
> it right (hint, they can not as they do not know what the core did with
> the reference counts.)
>
> Right now you are almost there, the typec core registers and tries to
> manage the structures, but it doesn't allocate/free them, and that's the
> big problem, because really, with a structure that has 3 different
> reference counts, no one can. That's an impossibility.
>
> This needs a lot more work, sorry.
I was trying to cut corners, which clearly was wrong. I know what I
need to do. All the parts simply need to be registered normally. No
shortcuts.
> I'm now going to require that you get other internal Intel developers to
> sign off on this code before I review it again. You have resources at
> your disposal that others do not with your internal mailing lists
> containing senior kernel developers. Use it and don't waste the
> community's time to do basic code review that they should be doing
> instead.
Fair enough.
> How did we get to a v13 of this patch series without anyone else even
> seeing this? That's worrysome as well...
I guess for somebody writing the port drivers my awesome shortcut felt
kinda nice. All they would have to do is announce connection when it
happens, and the class then tried figured out everything for them,
what needs to be created and so on. But yes, that is wrong!
But man, v14! I have got to be breaking the record with this one.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command
If the hardware never responds to the stop endpoint command, the
URBs will never be completed, and we might hang the USB subsystem.
The original watchdog timer is used to watch if one stop endpoint
command is timeout, if timeout, then the watchdog timer will set
XHCI_STATE_DYING, try to halt the xHCI host, and give back all
pending URBs.
But now we already have one command timer to control command timeout,
thus we can also use the command timer to watch the stop endpoint
command, instead of one duplicate watchdog timer which need to be
removed.
Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
this is the last stop endpoint command of one endpoint. Since we
can make sure we only set one stop endpoint command for one endpoint
by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
this flag.
We also need to clean up the command queue before trying to halt the
xHCI host in xhci_stop_endpoint_command_timeout() function.
Signed-off-by: Baolin Wang
---
drivers/usb/host/xhci-mem.c | 10 +--
drivers/usb/host/xhci-ring.c | 61 +-
drivers/usb/host/xhci.c |8 +-
drivers/usb/host/xhci.h |5
4 files changed, 21 insertions(+), 63 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d6f59a3..2a5cd89 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -844,14 +844,6 @@ void xhci_free_stream_info(struct xhci_hcd *xhci,
/* Device context manipulation */
-static void xhci_init_endpoint_timer(struct xhci_hcd *xhci,
- struct xhci_virt_ep *ep)
-{
- setup_timer(&ep->stop_cmd_timer, xhci_stop_endpoint_command_watchdog,
- (unsigned long)ep);
- ep->xhci = xhci;
-}
-
static void xhci_free_tt_info(struct xhci_hcd *xhci,
struct xhci_virt_device *virt_dev,
int slot_id)
@@ -1014,7 +1006,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int
slot_id,
/* Initialize the cancellation list and watchdog timers for each ep */
for (i = 0; i < 31; i++) {
- xhci_init_endpoint_timer(xhci, &dev->eps[i]);
+ dev->eps[i].xhci = xhci;
INIT_LIST_HEAD(&dev->eps[i].cancelled_td_list);
INIT_LIST_HEAD(&dev->eps[i].bw_endpoint_list);
}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2057d08..67a1bd6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -561,18 +561,6 @@ static void td_to_noop(struct xhci_hcd *xhci, struct
xhci_ring *ep_ring,
}
}
-static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci,
- struct xhci_virt_ep *ep)
-{
- ep->ep_state &= ~EP_HALT_PENDING;
- /* Can't del_timer_sync in interrupt, so we attempt to cancel. If the
-* timer is running on another CPU, we don't decrement stop_cmds_pending
-* (since we didn't successfully stop the watchdog timer).
-*/
- if (del_timer(&ep->stop_cmd_timer))
- ep->stop_cmds_pending--;
-}
-
/* Must be called with xhci->lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
struct xhci_td *cur_td, int status)
@@ -664,7 +652,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
int slot_id,
ep = &xhci->devs[slot_id]->eps[ep_index];
if (list_empty(&ep->cancelled_td_list)) {
- xhci_stop_watchdog_timer_in_irq(xhci, ep);
+ ep->ep_state &= ~EP_HALT_PENDING;
ep->stopped_td = NULL;
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
return;
@@ -719,7 +707,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
int slot_id,
list_del_init(&cur_td->td_list);
}
last_unlinked_td = cur_td;
- xhci_stop_watchdog_timer_in_irq(xhci, ep);
+ ep->ep_state &= ~EP_HALT_PENDING;
/* If necessary, queue a Set Transfer Ring Dequeue Pointer command */
if (deq_state.new_deq_ptr && deq_state.new_deq_seg) {
@@ -817,38 +805,19 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
}
}
-/* Watchdog timer function for when a stop endpoint command fails to complete.
+/* This timeout function for when a stop endpoint command fails to complete.
* In this case, we assume the host controller is broken or dying or dead. The
* host may still be completing some other events, so we have to be careful to
* let the event ring handler and the URB dequeueing/enqueueing functions know
* through xhci->state.
- *
- * The timer may also fire if the host takes a very long time to respond to the
- * command, and the stop endpoint command completion handler cannot delete the
- * timer before the timer function is called. Another endpoint cancellation
may
- * sneak in befo
Re: [PATCH 07/12] usb: sunxi: Uses the resource-managed extcon API when registering extcon notifier
On Wed, Nov 30, 2016 at 02:57:35PM +0900, Chanwoo Choi wrote: > This patch just uses the resource-managed extcon API when registering > the extcon notifier. > > Signed-off-by: Chanwoo Choi Acked-by: Maxime Ripard Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
RE: [PATCH v4 3/5] net: asix: Fix AX88772x resume failures
Dear Jon, Thanks a lot for your reminding. I will submit a new driver patch soon. --- Best regards, Allan Chou -Original Message- From: Jon Hunter [mailto:jonath...@nvidia.com] Sent: Wednesday, November 30, 2016 4:08 PM To: al...@asix.com.tw; fre...@asix.com.tw; dean_jenk...@mentor.com; mark_cra...@mentor.com; da...@davemloft.net; robert.f...@collabora.com; ivec...@redhat.com; john.stu...@linaro.org; vpala...@chromium.org; step...@networkplumber.org; grund...@chromium.org; changch...@gmail.com; and...@lunn.ch; trem...@gmail.com; colin.k...@canonical.com; linux-usb@vger.kernel.org; net...@vger.kernel.org; linux-ker...@vger.kernel.org; vpala...@google.com Subject: Re: [PATCH v4 3/5] net: asix: Fix AX88772x resume failures Hi Allan, On 30/11/16 03:03, ASIX_Allan [Office] wrote: > The change fixes AX88772x resume failure by > - Restore incorrect AX88772A PHY registers when resetting > - Need to stop MAC operation when suspending > - Need to restart MII when restoring PHY > > Signed-off-by: Allan Chou > Signed-off-by: Robert Foss > Tested-by: Robert Foss > Tested-by: Jon Hunter > Tested-by: Allan Chou V3 of this patch is already in the current mainline branch. So you need to send a patch on top of V3 (or v4.9-rc7) to get this fixed. Also you should highlight the fact that this is a fix needed for v4.9. Cheers Jon -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: asix: Fix AX88772_suspend() USB vendor commands failure issues
The change fixes AX88772_suspend() USB vendor commands failure issues. Signed-off-by: Allan Chou Tested-by: Allan Chou Tested-by: Jon Hunter --- --- a/drivers/net/usb/asix_devices.c2016-11-28 05:08:04.0 +0800 +++ b/drivers/net/usb/asix_devices.c2016-11-30 09:31:54.0 +0800 @@ -603,12 +603,12 @@ static void ax88772_suspend(struct usbne u16 medium; /* Stop MAC operation */ - medium = asix_read_medium_status(dev, 0); + medium = asix_read_medium_status(dev, 1); medium &= ~AX_MEDIUM_RE; - asix_write_medium_mode(dev, medium, 0); + asix_write_medium_mode(dev, medium, 1); netdev_dbg(dev->net, "ax88772_suspend: medium=0x%04x\n", - asix_read_medium_status(dev, 0)); + asix_read_medium_status(dev, 1)); /* Preserve BMCR for restoring */ priv->presvd_phy_bmcr = -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] net: asix: Fix AX88772x resume failures
Hi Allan, On 30/11/16 03:03, ASIX_Allan [Office] wrote: > The change fixes AX88772x resume failure by > - Restore incorrect AX88772A PHY registers when resetting > - Need to stop MAC operation when suspending > - Need to restart MII when restoring PHY > > Signed-off-by: Allan Chou > Signed-off-by: Robert Foss > Tested-by: Robert Foss > Tested-by: Jon Hunter > Tested-by: Allan Chou V3 of this patch is already in the current mainline branch. So you need to send a patch on top of V3 (or v4.9-rc7) to get this fixed. Also you should highlight the fact that this is a fix needed for v4.9. Cheers Jon -- nvpublic -- 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
