Re: [PATCH] CB/CBI storage devices not working with CONFIG_VMAP_STACK=y
On Thu, 10 Nov 2016, Petr Vandrovec wrote: > Hi, >I've built 4.9.0-rc4 with CONFIG_VMAP_STACK=y, and while I can use bulk > storage drives, I cannot use CB-based USB floppy drive: kernel complains > (once) that DMA address is invalid, and after that log is full of device > resets: > > Nov 7 12:18:42 petr-dev3 kernel: [ 25.929808] scsi 12:0:0:1: Direct-Access > Generic USB SM/MS/SD 1.00 PQ: 0 ANSI: 0 > Nov 7 12:18:42 petr-dev3 kernel: [ 25.969520] scsi 13:0:0:0: Direct-Access > CITIZEN X1DE-USB 1002 PQ: 0 ANSI: 0 CCS > Nov 7 12:18:42 petr-dev3 kernel: [ 25.978504] sd 13:0:0:0: Attached scsi > generic sg9 type 0 > Nov 7 12:18:42 petr-dev3 kernel: [ 25.981037] [ cut here > ] > Nov 7 12:18:42 petr-dev3 kernel: [ 25.981042] WARNING: CPU: 0 PID: 3829 at > /bhavesh/usr/src/git/libata-pv3/drivers/usb/core/hcd.c:1584 > usb_hcd_map_urb_for_dma+0x425/0x540 > Nov 7 12:18:42 petr-dev3 kernel: [ 25.981042] transfer buffer not dma > capable ... > Problem is that CB/CBI code passes CDB directly to the HCD for transmission - > and unfortunately SCSI error handling code creates CDB on stack > (scsi_eh_prep_cmnd > uses scsi_eh_save structure for CDB, and as far as I can tell, everybody > creates > scsi_eh_save structure on stack, rather than in kmalloc memory). > > As no other drivers does DMA directly from CDB field, I think if I try to > modify > USB storage code to not create scsi_eh_save on stack it will get broken again > anyway when someone else stores CDB on stack, so I went ahead with memcpy > solution instead: now CB/CBI code copies CDB into its private iobuf, and sends > command from there. That's the right thing to do. > With this fix in place USB floppy works again... > > Thanks, > Petr Vandrovec > > > > > > commit 07da00a2bb122bc7ffb287aa80f58714a17b1d9c > Author: Petr Vandrovec> Date: Wed Nov 9 14:46:35 2016 -0800 > > Fix UFI USB storage devices with vmalloced stacks > > Some code (all error handling) submits CDBs that are allocated > on stack. This breaks with UFI code that tries to create URB > directly from SCSI command buffer - which happens to be in > vmalloced memory with vmalloced kernel stacks. > > Let's make copy of cmd in usb_stor_CB_transport - it is pretty > cheap, and I cannot find any easy way to modify SCSI error > handling to not use on-stack structure for error handling > command. > > Signed-off-by: Petr Vandrovec > > diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c > index ffd0867..e46833b 100644 > --- a/drivers/usb/storage/transport.c > +++ b/drivers/usb/storage/transport.c > @@ -954,10 +954,16 @@ int usb_stor_CB_transport(struct scsi_cmnd *srb, struct > us_data *us) > > /* COMMAND STAGE */ > /* let's send the command via the control pipe */ > + /* > + * Command is sometime (f.e. after scsi_eh_prep_cmnd) on the stack. > + * Stack may be vmallocated. So no DMA for us. Make a copy. > + */ > + BUG_ON(srb->cmd_len > US_IOBUF_SIZE); Don't bother with the BUG_ON. The usb_stor_Bulk_transport() routine doesn't check for this, and it needs more buffer space than you do. > + memcpy(us->iobuf, srb->cmnd, srb->cmd_len); > result = usb_stor_ctrl_transfer(us, us->send_ctrl_pipe, > US_CBI_ADSC, > USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0, > - us->ifnum, srb->cmnd, srb->cmd_len); > + us->ifnum, us->iobuf, srb->cmd_len); > > /* check the return code for the command */ > usb_stor_dbg(us, "Call to usb_stor_ctrl_transfer() returned %d\n", Good work. Please submit an updated patch, following the format described in Documentation/SubmittingPatches (no preliminary text, don't indent the patch description, include a --- line after the Signed-off-by, and so on), and be sure to cc: Greg KH . Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Fix USB CB/CBI storage devices with CONFIG_VMAP_STACK=y
From: Petr VandrovecSome code (all error handling) submits CDBs that are allocated on the stack. This breaks with CB/CBI code that tries to create URB directly from SCSI command buffer - which happens to be in vmalloced memory with vmalloced kernel stacks. Let's make copy of the command in usb_stor_CB_transport. Signed-off-by: Petr Vandrovec Acked-by: Alan Stern --- diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c index ffd0867..1a59f33 100644 --- a/drivers/usb/storage/transport.c +++ b/drivers/usb/storage/transport.c @@ -954,10 +954,15 @@ int usb_stor_CB_transport(struct scsi_cmnd *srb, struct us_data *us) /* COMMAND STAGE */ /* let's send the command via the control pipe */ + /* +* Command is sometime (f.e. after scsi_eh_prep_cmnd) on the stack. +* Stack may be vmallocated. So no DMA for us. Make a copy. +*/ + memcpy(us->iobuf, srb->cmnd, srb->cmd_len); result = usb_stor_ctrl_transfer(us, us->send_ctrl_pipe, US_CBI_ADSC, USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0, - us->ifnum, srb->cmnd, srb->cmd_len); + us->ifnum, us->iobuf, srb->cmd_len); /* check the return code for the command */ usb_stor_dbg(us, "Call to usb_stor_ctrl_transfer() returned %d\n", -- 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] CB/CBI storage devices not working with CONFIG_VMAP_STACK=y
Hi, I've built 4.9.0-rc4 with CONFIG_VMAP_STACK=y, and while I can use bulk storage drives, I cannot use CB-based USB floppy drive: kernel complains (once) that DMA address is invalid, and after that log is full of device resets: Nov 7 12:18:42 petr-dev3 kernel: [ 25.929808] scsi 12:0:0:1: Direct-Access Generic USB SM/MS/SD 1.00 PQ: 0 ANSI: 0 Nov 7 12:18:42 petr-dev3 kernel: [ 25.969520] scsi 13:0:0:0: Direct-Access CITIZEN X1DE-USB 1002 PQ: 0 ANSI: 0 CCS Nov 7 12:18:42 petr-dev3 kernel: [ 25.978504] sd 13:0:0:0: Attached scsi generic sg9 type 0 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981037] [ cut here ] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981042] WARNING: CPU: 0 PID: 3829 at /bhavesh/usr/src/git/libata-pv3/drivers/usb/core/hcd.c:1584 usb_hcd_map_urb_for_dma+0x425/0x540 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981042] transfer buffer not dma capable Nov 7 12:18:42 petr-dev3 kernel: [ 25.981059] Modules linked in: snd_hda_codec_hdmi(+) snd_hda_codec_realtek snd_hda_codec_generic coretemp hwmon x86_pkg_temp_thermal crct10dif_pclmul crc32_pclmul crc32c_intel snd_hda_intel ghash_clmulni_intel snd_hda_codec aesni_intel uas aes_x86_64 snd_hwdep lrw dcdbas glue_helper ablk_helper usb_storage cryptd serio_raw snd_hda_core sr_mod i2c_i801 cdrom i2c_smbus snd_pcm_oss sg snd_mixer_oss snd_pcm snd_timer mei_me snd mei tpm_tis tpm_tis_core tpm e1000e ptp pps_core iTCO_wdt iTCO_vendor_support lpc_ich mfd_core nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables x_tables ipv6 crc_ccitt autofs4 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981060] CPU: 0 PID: 3829 Comm: usb-storage Not tainted 4.9.0-rc4-64-00017-ga6c2c43-dirty #110 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981061] Hardware name: Dell Inc. Precision T3610/09M8Y8, BIOS A11 04/22/2016 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981063] c9573ac8 813a1d60 c9573b18 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981064] c9573b08 8105e171 063fff59 88101dbc83c0 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981064] 881024a7d800 0001 881024e97000 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981065] Call Trace: Nov 7 12:18:42 petr-dev3 kernel: [ 25.981069] [] dump_stack+0x63/0x83 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981070] [] __warn+0xc1/0xe0 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981071] [] warn_slowpath_fmt+0x4a/0x50 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981072] [] usb_hcd_map_urb_for_dma+0x425/0x540 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981074] [] usb_hcd_submit_urb+0x330/0xb20 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981075] [] ? schedule+0x38/0x90 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981076] [] ? schedule_timeout+0x12c/0x180 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981078] [] ? cpuacct_charge+0x81/0x90 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981079] [] ? set_next_entity+0x45/0x9a0 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981081] [] usb_submit_urb+0x2ef/0x560 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981083] [] ? wake_up_q+0x80/0x80 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981085] [] usb_stor_msg_common+0x97/0x120 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981087] [] usb_stor_ctrl_transfer+0x9a/0xc0 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981088] [] usb_stor_CB_transport+0x4e/0x230 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981089] [] usb_stor_invoke_transport+0x23a/0x500 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981091] [] ? do_syscall_64+0x69/0x180 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981093] [] usb_stor_ufi_command+0x5d/0x90 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981095] [] usb_stor_control_thread+0x150/0x250 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981096] [] ? do_syscall_64+0x69/0x180 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981097] [] ? fill_inquiry_response+0x20/0x20 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981098] [] ? do_syscall_64+0x69/0x180 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981100] [] ? fill_inquiry_response+0x20/0x20 [usb_storage] Nov 7 12:18:42 petr-dev3 kernel: [ 25.981103] [] kthread+0xc5/0xe0 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981104] [] ? kthread_park+0x60/0x60 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981106] [] ret_from_fork+0x25/0x30 Nov 7 12:18:42 petr-dev3 kernel: [ 25.981107] ---[ end trace ec2cf5d1c79e1535 ]--- Nov 7 12:18:42 petr-dev3 kernel: [ 26.100014] usb 2-1.3.2: reset full-speed USB device number 5 using ehci-pci ... Nov 7 12:18:48 petr-dev3 kernel: [ 37.950059] usb 2-1.3.2: reset full-speed USB device number 5 using ehci-pci Nov 7 12:18:49 petr-dev3 kernel: [ 38.280055] usb 2-1.3.2: reset full-speed USB device number 5 using ehci-pci Nov 7 12:18:49 petr-dev3 kernel: [ 38.610061] usb 2-1.3.2: reset full-speed USB device number 5 using ehci-pci Nov 7 12:18:49
cdc_acm - fragmented notifications
Hi, I'm trying to build an usb device conforming to the CDC ACM device class. The device uses an interrupt IN endpoint with a max packet size of 8 bytes. I tried to send a SERIAL_STATE notification to the host (to report parity errors, etc.) and noticed, that it is not handled as I would expect: Because I have to transmit the notification in two consecutive packets due to the limited packe size (SERIAL_STATE notification consists of an 8 byte header + 2 byte data), acm_ctrl_irq is called twice, too. So the notification is not reassembled before processing in acm_ctrl_irq. Of course acm_ctrl_irq only handles "garbage" in this cases because both recieved packages are to short. Obviously the max packet size of the interrup IN endpoint needs to be equal (or greater) to the size of the largest notification the CDC device may transmit, to work with the cdc_acm module. Question: Is this just a (probably quite realistic) assumption made in the current kernel code, or is this constraint defined in the usb specification? I'm new to the usb stuff and am not sure if i missed something. Additionally I am confused, because Atmel provides some documentation how to build a CDC ACM device [1], where the fragmented transmission of notifications is explicitely stated as one possible implementation. I'm using kernel 4.4.0. Tobias [1] https://www.google.de/url?sa=t=j==s=web=1= rja=8=0ahUKEwjWvo- Ij5_QAhUFbRQKHUSlAa8QFggdMAA=http%3A%2F%2Fwww.atmel.com%2Fimages%2F doc6269.pdf=AFQjCNEQ-Oz__2Dq7ueSsd1JDAceCLPIHg -- 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: [PATHCv10 0/2] USB Type-C Connector class
On Mon, Sep 19, 2016 at 02:16:55PM +0300, Heikki Krogerus wrote: > The USB Type-C class is meant to provide unified interface to the > userspace to present the USB Type-C ports in a system. > Any idea what is happening with this patch series ? There was no further feedback (at least as far as I know), the series seems to be ready to go, yet I don't see it in -next. Thanks, Guenter > Changes since v9: > - Minor typec_wcove.c cleanup as proposed by Guenter Roeck. No > function affect. > > Changes since v8: > - checking sysfs_streq() result correctly in sysfs_strmatch > - fixed accessory check in supported_accessory_mode > - using "none" as the only string that can clear the preferred role > > Changes since v7: > - Removed "type" attribute from partners > - Added supports_usb_power_delivery attribute for partner and cable > > Changes since v6: > - current_vconn_role attr renamed to vconn_source (no API changes) > - Small documentation improvements proposed by Vincent Palatin > > Changes since v5: > - Only updating the roles based on driver notifications > - Added MODULE_ALIAS for the WhiskeyCove module > - Including the patch that creates the actual platform device for the > WhiskeyCove Type-C PHY in this series. > > Changes since v4: > - Remove the port lock completely > > Changes since v3: > - Documentation cleanup as proposed by Roger Quadros > - Setting partner altmodes member to NULL on removal and fixing a > warning, as proposed by Guenter Roeck > - Added the following attributes for partners and cables: > * supports_usb_power_delivery > * id_header_vdo > - "id_header_vdo" is visible only when the partner or cable supports > USB Power Delivery communication. > - Partner attribute "accessory" is hidden when the partner type is not > "Accessory". > > Changes since v2: > - Notification on role and alternate mode changes > - cleanups > > Changes since v1: > - Completely rewrote alternate mode support > - Patners, cables and cable plugs presented as devices. > > > Heikki Krogerus (2): > usb: USB Type-C connector class > usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY > > Documentation/ABI/testing/sysfs-class-typec | 218 ++ > Documentation/usb/typec.txt | 103 +++ > MAINTAINERS |9 + > drivers/usb/Kconfig |2 + > drivers/usb/Makefile|2 + > drivers/usb/typec/Kconfig | 21 + > drivers/usb/typec/Makefile |2 + > drivers/usb/typec/typec.c | 1075 > +++ > drivers/usb/typec/typec_wcove.c | 372 + > include/linux/usb/typec.h | 252 +++ > 10 files changed, 2056 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-typec > create mode 100644 Documentation/usb/typec.txt > create mode 100644 drivers/usb/typec/Kconfig > create mode 100644 drivers/usb/typec/Makefile > create mode 100644 drivers/usb/typec/typec.c > create mode 100644 drivers/usb/typec/typec_wcove.c > create mode 100644 include/linux/usb/typec.h > > -- > 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: [PATCH 0/4] musb fixes for v4.9-rc cycle
* Laurent Pinchart[161110 14:26]: > On Thursday 10 Nov 2016 22:29:34 Laurent Pinchart wrote: > Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite > understandably. It's not a fix though, just a workaround. Are you interested > in investigating this ? OK I think I have my hands full already.. Tony -- 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 0/4] musb fixes for v4.9-rc cycle
Hi Tony, On Thursday 10 Nov 2016 22:29:34 Laurent Pinchart wrote: > On Thursday 10 Nov 2016 10:50:33 Tony Lindgren wrote: > > * Laurent Pinchart[161110 10:18]: > > > I'll try to give it a go, but right now I'm stuck with a different MUSB > > > problem :-) I'm trying to boot my panda board over the SMSC95xx > > > ethernet, and have switched gadget support from being compiled in to > > > being compiled as a module. I then get > > > > > > [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while > > > active > > > > > > printed in a loop at boot time. I've traced musb->is_active being set to > > > 1 in musb_start() with > > > > > > musb->port_mode = MUSB_PORT_MODE_DUAL_ROLE > > > musb->xceiv->otg->state = OTG_STATE_A_IDLE > > > devctl = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_VBUS > > > > > > I can disable gadget support completely which will I believe fix the > > > problem, but that's just a workaround. Help would be appreciated. > > > > Do you have the ID pin grounded > > I've measured it on the board and I don't. Booting with the USB cable > unplugged and the board powered through the power jack gives the same > result. > > or how do you get to the A_IDLE state? > > Don't ask me :-) > > > I'm pretty much only testing with loadable modules as in > > omap2plus_defconfig, I don't think I've seen this issue.. > > > > Maybe check the changes in your .config against just plain > > omap2plus_defconfig? Are you trying to force host only mode? > > I started anew from omap2plus_defconfig and applied the following changes to > enable NFS root. > > -CONFIG_USB_NET_DRIVERS=m > -CONFIG_USB_USBNET=m > -CONFIG_USB_NET_SMSC95XX=m > -CONFIG_USB=m > -CONFIG_USB_EHCI_HCD=m > -CONFIG_USB_EHCI_HCD_OMAP=m > -CONFIG_NOP_USB_XCEIV=m > -CONFIG_USB_GADGET=m > -CONFIG_OMAP_USB2=m > +CONFIG_USB_NET_DRIVERS=y > +CONFIG_USB_USBNET=y > +CONFIG_USB_NET_SMSC95XX=y > +CONFIG_USB=y > +CONFIG_USB_EHCI_HCD=y > +CONFIG_USB_EHCI_HCD_OMAP=y > +CONFIG_NOP_USB_XCEIV=y > +# CONFIG_USB_GADGET is not set > +CONFIG_OMAP_USB2=y > > I had to disable CONFIG_USB_GADGET is compiling it as a module prevents > selecting CONFIG_NOP_USB_XCEIV=y, which is a dependency for > CONFIG_USB_EHCI_HCD_OMAP=m. > > The new configuration resulted in a few changes, among which the most > notable is > > -# CONFIG_USB_MUSB_HOST is not set > -# CONFIG_USB_MUSB_GADGET is not set > -CONFIG_USB_MUSB_DUAL_ROLE=y > +CONFIG_USB_MUSB_HOST=y > > I then get the same error as originally reported. Actually disabling CONFIG_USB_MUSB_HDRC gets rid of the problem, quite understandably. It's not a fix though, just a workaround. Are you interested in investigating this ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
On Sun, Nov 6, 2016 at 7:56 PM, Chen-Yu Tsaiwrote: > On Mon, Nov 7, 2016 at 9:29 AM, Peter Chen wrote: >> On Fri, Nov 04, 2016 at 01:51:34PM -0700, Stephen Boyd wrote: >>> Quoting Peter Chen (2016-10-24 18:16:32) >>> > On Mon, Oct 24, 2016 at 12:48:24PM -0700, Stephen Boyd wrote: >>> > > Quoting Chen-Yu Tsai (2016-10-24 05:19:05) >>> > > > Hi, >>> > > > >>> > > > On Tue, Oct 18, 2016 at 9:56 AM, Stephen Boyd >>> > > > wrote: >>> > > > > The ULPI bus can be built as a module, and it will soon be >>> > > > > calling these functions when it supports probing devices from DT. >>> > > > > Export them so they can be used by the ULPI module. >>> > > > > >>> > > > > Acked-by: Rob Herring >>> > > > > Cc: >>> > > > > Signed-off-by: Stephen Boyd >>> > > > > --- >>> > > > > drivers/of/device.c | 2 ++ >>> > > > > 1 file changed, 2 insertions(+) >>> > > > > >>> > > > > diff --git a/drivers/of/device.c b/drivers/of/device.c >>> > > > > index 8a22a253a830..6719ab35b62e 100644 >>> > > > > --- a/drivers/of/device.c >>> > > > > +++ b/drivers/of/device.c >>> > > > > @@ -225,6 +225,7 @@ ssize_t of_device_get_modalias(struct device >>> > > > > *dev, char *str, ssize_t len) >>> > > > > >>> > > > > return tsize; >>> > > > > } >>> > > > > +EXPORT_SYMBOL_GPL(of_device_get_modalias); >>> > > > > >>> > > > > int of_device_request_module(struct device *dev) >>> > > > > { >>> > > > > @@ -290,6 +291,7 @@ void of_device_uevent(struct device *dev, >>> > > > > struct kobj_uevent_env *env) >>> > > > > } >>> > > > > mutex_unlock(_mutex); >>> > > > > } >>> > > > > +EXPORT_SYMBOL_GPL(of_device_uevent_modalias); >>> > > > >>> > > > This is trailing the wrong function. >>> > > > >>> > > >>> > > Good catch. Must have been some bad rebase. >>> > > >>> > > Peter, can you fix it while applying or should I resend this patch? >>> > > >>> > >>> > But, this is device tree patch. I can only get chipidea part and other >>> > USB patches if Greg agrees. >>> > >>> >>> Were you expecting Rob to take the drivers/of/* patches? Sorry I thought >>> Rob acked them so they could go through usb with the other changes. >> >> I am just worried about possible merge error when linus pulls both OF >> and USB tree. Greg, is it ok the OF patches through USB tree with OF >> maintainer's ack? > > May I suggest putting the OF patches on an immutable branch so other > subsystems can pull them in without pulling in the USB patches? At > least I want to use them in the I2C subsystem, and in the sunxi-rsb > driver. Do you have patches using this already. If not, it is starting to get a bit late for v4.10. I can apply this, but then you'll just be pulling in other DT patches. Rob -- 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 0/4] musb fixes for v4.9-rc cycle
Hi Tony, On Thursday 10 Nov 2016 13:42:34 Tony Lindgren wrote: > * Laurent Pinchart[161110 13:29]: > > I had to disable CONFIG_USB_GADGET is compiling it as a module prevents > > selecting CONFIG_NOP_USB_XCEIV=y, which is a dependency for > > CONFIG_USB_EHCI_HCD_OMAP=m. > > > > The new configuration resulted in a few changes, among which the most > > notable is > > > > -# CONFIG_USB_MUSB_HOST is not set > > -# CONFIG_USB_MUSB_GADGET is not set > > -CONFIG_USB_MUSB_DUAL_ROLE=y > > +CONFIG_USB_MUSB_HOST=y > > > > I then get the same error as originally reported. > > Yeah OK, that's the problem.. We still have musb hardware > trying to do things on it's own and phy trying to detect > the state. > > Any ideas why we have a dependency like that in Kconfig? Well, with CONFIG_USB_GADGET disabled, I don't expect CONFIG_USB_MUSB_GADGET to be enabled. MUSB can only operate in host mode in that case, so musb- >xceiv->otg->state = OTG_STATE_A_IDLE makes sense. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] musb fixes for v4.9-rc cycle
Hi Tony, On Thursday 10 Nov 2016 10:50:33 Tony Lindgren wrote: > * Laurent Pinchart[161110 10:18]: > > I'll try to give it a go, but right now I'm stuck with a different MUSB > > problem :-) I'm trying to boot my panda board over the SMSC95xx ethernet, > > and have switched gadget support from being compiled in to being compiled > > as a module. I then get > > > > [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while > > active > > > > printed in a loop at boot time. I've traced musb->is_active being set to 1 > > in musb_start() with > > > > musb->port_mode = MUSB_PORT_MODE_DUAL_ROLE > > musb->xceiv->otg->state = OTG_STATE_A_IDLE > > devctl = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_VBUS > > > > I can disable gadget support completely which will I believe fix the > > problem, but that's just a workaround. Help would be appreciated. > > Do you have the ID pin grounded I've measured it on the board and I don't. Booting with the USB cable unplugged and the board powered through the power jack gives the same result. > or how do you get to the A_IDLE state? Don't ask me :-) > I'm pretty much only testing with loadable modules as in > omap2plus_defconfig, I don't think I've seen this issue.. > > Maybe check the changes in your .config against just plain > omap2plus_defconfig? Are you trying to force host only mode? I started anew from omap2plus_defconfig and applied the following changes to enable NFS root. -CONFIG_USB_NET_DRIVERS=m -CONFIG_USB_USBNET=m -CONFIG_USB_NET_SMSC95XX=m -CONFIG_USB=m -CONFIG_USB_EHCI_HCD=m -CONFIG_USB_EHCI_HCD_OMAP=m -CONFIG_NOP_USB_XCEIV=m -CONFIG_USB_GADGET=m -CONFIG_OMAP_USB2=m +CONFIG_USB_NET_DRIVERS=y +CONFIG_USB_USBNET=y +CONFIG_USB_NET_SMSC95XX=y +CONFIG_USB=y +CONFIG_USB_EHCI_HCD=y +CONFIG_USB_EHCI_HCD_OMAP=y +CONFIG_NOP_USB_XCEIV=y +# CONFIG_USB_GADGET is not set +CONFIG_OMAP_USB2=y I had to disable CONFIG_USB_GADGET is compiling it as a module prevents selecting CONFIG_NOP_USB_XCEIV=y, which is a dependency for CONFIG_USB_EHCI_HCD_OMAP=m. The new configuration resulted in a few changes, among which the most notable is -# CONFIG_USB_MUSB_HOST is not set -# CONFIG_USB_MUSB_GADGET is not set -CONFIG_USB_MUSB_DUAL_ROLE=y +CONFIG_USB_MUSB_HOST=y I then get the same error as originally reported. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
Alan Sternwrites: > On Thu, 10 Nov 2016, Kai-Heng Feng wrote: > >> Is there a way to force XHCI run at HighSpeed? > > Yes, like I said above: Use a USB-2 cable instead of a USB-3 cable. It's an m.2 form factor modem, so there most likely isn't any cable involved. But the principle is the same: Cover the USB3 diff pair pins with a piece of tape. 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 0/4] musb fixes for v4.9-rc cycle
* Laurent Pinchart[161110 13:29]: > I had to disable CONFIG_USB_GADGET is compiling it as a module prevents > selecting CONFIG_NOP_USB_XCEIV=y, which is a dependency for > CONFIG_USB_EHCI_HCD_OMAP=m. > > The new configuration resulted in a few changes, among which the most notable > is > > -# CONFIG_USB_MUSB_HOST is not set > -# CONFIG_USB_MUSB_GADGET is not set > -CONFIG_USB_MUSB_DUAL_ROLE=y > +CONFIG_USB_MUSB_HOST=y > > I then get the same error as originally reported. Yeah OK, that's the problem.. We still have musb hardware trying to do things on it's own and phy trying to detect the state. Any ideas why we have a dependency like that in Kconfig? Regards, Tony -- 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 1/4] usb: dbc: early driver for xhci debug capability
Hi Peter, On 11/10/2016 07:44 PM, Peter Zijlstra wrote: > On Thu, Nov 10, 2016 at 09:56:41AM +0100, Thomas Gleixner wrote: >> On Thu, 10 Nov 2016, Lu Baolu wrote: >>> This seems to be a common issue for all early printk drivers. >> No. The other early printk drivers like serial do not have that problem as >> they simply do: >> >>while (*buf) { >> while (inb(UART) & TX_BUSY) >> cpu_relax(); >> outb(*buf++, UART); >>} > Right, which is why actual UARTs rule. If only laptops still had pinouts > for them life would be so much better. > > Ideally the USB debug port would be a virtual UART and its interface as > simple and robust. > >> The wait for the UART to become ready is independent of the context as it >> solely depends on the hardware. >> >> As a result you can see the output from irq/nmi intermingled with the one >> from thread context, but that's the only problem they have. >> >> The only thing you can do to make this work is to prevent printing in NMI >> context: >> >> write() >> { >> if (in_nmi()) >> return; >> >> raw_spinlock_irqsave(, flags); >> >> >> That fully serializes the writes and just ignores NMI context printks. Not >> optimal, but I fear that's all you can do. > I would also suggest telling the hardware people they have designed > something near the brink of useless. If you cannot do random exception > context debugging (#DB, #NMI, #MCE etc..) then there's a whole host of > problems that simply cannot be debugged. > > Also note that kdb runs from NMI context, so you'll not be able to > support that either. > Things become complicated when it comes to USB debug port. But it's still addressable. At this time, we can do it like this. write() { if (in_nmi() && raw_spin_is_locked()) return; raw_spinlock_irqsave(, flags); This will filter some messages from NMI handler in case that another thread is holding the spinlock. I have no idea about how much chance could a debug user faces this. But it might further be fixed with below enhancement. write() { if (in_nmi() && raw_spin_is_locked()) { produce_a_pending_item_in_ring(); return; } raw_spinlock_irqsave(, flags); while (!pending_item_ring_is_empty) consume_a_pending_item_in_ring(); We can design the pending item ring in a producer-consumer model. It's easy to avoid race between the producer and consumer. 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: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability
Hi, On 11/10/2016 04:56 PM, Thomas Gleixner wrote: > On Thu, 10 Nov 2016, Lu Baolu wrote: >> On 11/09/2016 05:37 PM, Thomas Gleixner wrote: >>> On Tue, 1 Nov 2016, Lu Baolu wrote: +static void early_xdbc_write(struct console *con, const char *str, u32 n) +{ + int chunk, ret; + static char buf[XDBC_MAX_PACKET]; + int use_cr = 0; + + if (!xdbc.xdbc_reg) + return; + memset(buf, 0, XDBC_MAX_PACKET); >>> How is that dealing with reentrancy? >>> >>> early_printk() does not protect against it. Peter has a patch to prevent >>> concurrent access from different cpus, but it cannot and will never prevent >>> reentrancy on the same cpu (interrupt, nmi). >> I can use a spinlock_irq to protect reentrancy of interrupt on the same >> cpu. But I have no idea about the nmi one. > spinlock wont work due to NMIs. Yes, of course. > >> This seems to be a common issue for all early printk drivers. > No. The other early printk drivers like serial do not have that problem as > they simply do: > >while (*buf) { > while (inb(UART) & TX_BUSY) >cpu_relax(); > outb(*buf++, UART); >} > > The wait for the UART to become ready is independent of the context as it > solely depends on the hardware. > > As a result you can see the output from irq/nmi intermingled with the one > from thread context, but that's the only problem they have. Yes, you are right. > > The only thing you can do to make this work is to prevent printing in NMI > context: > > write() > { > if (in_nmi()) > return; > > raw_spinlock_irqsave(, flags); > > > That fully serializes the writes and just ignores NMI context printks. Not > optimal, but I fear that's all you can do. Yes. But I want to add a bit more. write() { if (in_nmi() && raw_spin_is_locked()) { trace("... ..."); return; } raw_spinlock_irqsave(, flags); 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: [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
2016-11-10 22:35 GMT+09:00 Greg Kroah-Hartman: > On Thu, Nov 10, 2016 at 10:24:21PM +0900, Masahiro Yamada wrote: >> >> sdhci_alloc_host() returns an error pointer when it fails. >> but mmc_alloc_host() cannot. >> >> This series allow to propagate a proper error code >> when host-allocation fails. > > Why? What can we really do about the error except give up? Why does > having a explicit error value make any difference to the caller, they > can't do anything different, right? The error code is shown in the console, like probe of 5a00.sdhc failed with error -12 The proper error code will give a clue why the driver failed to probe. > I suggest just leaving it as-is, it's simple, and you don't have to mess > with PTR_ERR() anywhere. Why? Most of driver just give up probing for any error, but we still do ERR_PTR()/PTR_ERR() here and there. I think this patch is the same pattern. If a function returns NULL on failure, we need to think about "what is the most common failure case". Currently, MMC drivers assume -ENOMEM is the best fit for mmc_alloc_host(), but the assumption is fragile. Already, mmc_alloc_host() calls a function that returns not only -ENOMEM, but also -ENOSPC. In the future, some other failure cases might be added to mmc_alloc_host(). Once we decide the API returns an error pointer, drivers just propagate the return value from the API. This is much more stable implementation. -- Best Regards Masahiro Yamada -- 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 1/4] usb: dbc: early driver for xhci debug capability
Hi, On 11/11/2016 10:24 AM, Lu Baolu wrote: >> The only thing you can do to make this work is to prevent printing in NMI >> > context: >> > >> > write() >> > { >> >if (in_nmi()) >> >return; >> > >> >raw_spinlock_irqsave(, flags); >> > >> > >> > That fully serializes the writes and just ignores NMI context printks. Not >> > optimal, but I fear that's all you can do. > Yes. But I want to add a bit more. > > write() > { > if (in_nmi() && raw_spin_is_locked()) { > trace("... ..."); > return; > } > > raw_spinlock_irqsave(, flags); > Or...? write() { if (in_nmi() && raw_spin_is_locked()) { save_nmi_message_in_local_buf(); set_nmi_message_pending_flag(); return; } if (nmi_message_pending_flag_is_set()) { write_nmi_message(); clear_nmi_message_pending_flag(); } raw_spinlock_irqsave(, flags); 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: [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
On Fri, Nov 11, 2016 at 5:42 AM, Rob Herringwrote: > On Sun, Nov 6, 2016 at 7:56 PM, Chen-Yu Tsai wrote: >> On Mon, Nov 7, 2016 at 9:29 AM, Peter Chen wrote: >>> On Fri, Nov 04, 2016 at 01:51:34PM -0700, Stephen Boyd wrote: Quoting Peter Chen (2016-10-24 18:16:32) > On Mon, Oct 24, 2016 at 12:48:24PM -0700, Stephen Boyd wrote: > > Quoting Chen-Yu Tsai (2016-10-24 05:19:05) > > > Hi, > > > > > > On Tue, Oct 18, 2016 at 9:56 AM, Stephen Boyd > > > wrote: > > > > The ULPI bus can be built as a module, and it will soon be > > > > calling these functions when it supports probing devices from DT. > > > > Export them so they can be used by the ULPI module. > > > > > > > > Acked-by: Rob Herring > > > > Cc: > > > > Signed-off-by: Stephen Boyd > > > > --- > > > > drivers/of/device.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/of/device.c b/drivers/of/device.c > > > > index 8a22a253a830..6719ab35b62e 100644 > > > > --- a/drivers/of/device.c > > > > +++ b/drivers/of/device.c > > > > @@ -225,6 +225,7 @@ ssize_t of_device_get_modalias(struct device > > > > *dev, char *str, ssize_t len) > > > > > > > > return tsize; > > > > } > > > > +EXPORT_SYMBOL_GPL(of_device_get_modalias); > > > > > > > > int of_device_request_module(struct device *dev) > > > > { > > > > @@ -290,6 +291,7 @@ void of_device_uevent(struct device *dev, > > > > struct kobj_uevent_env *env) > > > > } > > > > mutex_unlock(_mutex); > > > > } > > > > +EXPORT_SYMBOL_GPL(of_device_uevent_modalias); > > > > > > This is trailing the wrong function. > > > > > > > Good catch. Must have been some bad rebase. > > > > Peter, can you fix it while applying or should I resend this patch? > > > > But, this is device tree patch. I can only get chipidea part and other > USB patches if Greg agrees. > Were you expecting Rob to take the drivers/of/* patches? Sorry I thought Rob acked them so they could go through usb with the other changes. >>> >>> I am just worried about possible merge error when linus pulls both OF >>> and USB tree. Greg, is it ok the OF patches through USB tree with OF >>> maintainer's ack? >> >> May I suggest putting the OF patches on an immutable branch so other >> subsystems can pull them in without pulling in the USB patches? At >> least I want to use them in the I2C subsystem, and in the sunxi-rsb >> driver. > > Do you have patches using this already. If not, it is starting to get > a bit late for v4.10. > > I can apply this, but then you'll just be pulling in other DT patches. Not sure what you mean by "using" this... I have patches which use this to add DT-based modalias entries for module auto-loading to i2c and sunxi-rsb that I haven't sent. As far as DT usage goes, we already need this for the axp20x mfd driver. There are 2 variants, i2c and sunxi-rsb. For the I2C variant a fix was sent to fix module auto-loading by using the I2C client ID table: mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading https://git.kernel.org/cgit/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next=b7142a19321484bd7681aa547c1d50148c8e2825 But for the sunxi-rsb variant such a fix does not exist, as the bus does not have a separate ID table. It uses DT exclusively. Regards ChenYu -- 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 v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
Hello ChenYu On Fri, Nov 11, 2016 at 12:01 AM, Chen-Yu Tsaiwrote: > On Fri, Nov 11, 2016 at 5:42 AM, Rob Herring wrote: [snip] >> >> Do you have patches using this already. If not, it is starting to get >> a bit late for v4.10. >> >> I can apply this, but then you'll just be pulling in other DT patches. > > Not sure what you mean by "using" this... > > I have patches which use this to add DT-based modalias entries for > module auto-loading to i2c and sunxi-rsb that I haven't sent. > Unfortunately the I2C core can't be changed without breaking a lot of I2C drivers that are relying on the current behavior. I've already posted a RFC patch [0] for I2C that does this about a year ago and enumerated the issues that have to be addressed before the change can be made (and fixed some of the issues mentioned) on this series [1]. Another issue is that an I2C device ID table is a requirement anyways since I2C drivers expect an i2c_device_id as an argument of their probe function. Kieran already have patches [2] to change that which should land soon. I plan to fix the remaining I2C drivers once his patches are merged and re-post the RFC patch as a proper one. > As far as DT usage goes, we already need this for the axp20x mfd driver. > There are 2 variants, i2c and sunxi-rsb. For the I2C variant a fix was > sent to fix module auto-loading by using the I2C client ID table: > > mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading > > https://git.kernel.org/cgit/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next=b7142a19321484bd7681aa547c1d50148c8e2825 > Yes, this is the workaround used by most DT-only I2C drivers. The only reason that these drivers have an I2C device ID is due the restrictions imposed by the I2C core. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: xhci: handle endpoint error caused by TRB error
When xHCI host sees a malformed TRB in a transfer ring, it will generate a transfer event with the completion code set to COMP_TRB_ERR (5), and sets the endpoint state in output endpoint context to EP_STATE_ERROR. The endpoint enters ERROR state as the result. XHCI specification requires that Set TR Dequeue Pointer Command shall be used to transition the endpoint from Error to Stopped state. Current xHCI driver doesn't clear this endpoint error, hence the successive URB enqueue requests will result in error messages of "WARN waiting for error on ep to be cleared". And the corresponding USB device stays in unresponsive state. This patch enhances xHCI driver on this by printing out the malformed TRB and clearing the endpoint Error state. Tested-by: Wang WendySigned-off-by: Lu Baolu --- drivers/usb/host/xhci-ring.c | 35 +++ drivers/usb/host/xhci.c | 22 ++ drivers/usb/host/xhci.h | 2 ++ 3 files changed, 59 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 822f88a..f81c1be 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1773,6 +1773,20 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci, xhci_ring_cmd_db(xhci); } +static void xhci_cleanup_trb_error_endpoint(struct xhci_hcd *xhci, + unsigned int slot_id, unsigned int ep_index, + unsigned int stream_id, + struct xhci_td *td, union xhci_trb *event_trb) +{ + struct xhci_virt_ep *ep = >devs[slot_id]->eps[ep_index]; + + ep->stopped_stream = stream_id; + xhci_cleanup_trb_error_ring(xhci, ep_index, td); + ep->stopped_stream = 0; + + xhci_ring_cmd_db(xhci); +} + /* Check if an error has halted the endpoint ring. The class driver will * cleanup the halt for a non-default control endpoint if we indicate a stall. * However, a babble and other errors also halt the endpoint ring, and the class @@ -1860,6 +1874,13 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, */ xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index, ep_ring->stream_id, td, ep_trb); + } else if (trb_comp_code == COMP_TRB_ERR) { + /* Clear the Endpoint error caused by a TRB error by issuing +* a set dequeue command to move the dequeue pointer past the +* last TD. +*/ + xhci_cleanup_trb_error_endpoint(xhci, slot_id, ep_index, + ep_ring->stream_id, td, ep_trb); } else { /* Update ring dequeue pointer */ while (ep_ring->dequeue != td->last_trb) @@ -2474,6 +2495,20 @@ static int handle_tx_event(struct xhci_hcd *xhci, goto cleanup; } + /* +* Dump the original TRB which caused a transfer error with +* completion code set to TRB error. +*/ + if (trb_comp_code == COMP_TRB_ERR) { + xhci_err(xhci, "Malformed transfer TRB deteced:\n"); + xhci_err(xhci, "@%016llx %08x %08x %08x %08x\n", +(unsigned long long)ep_trb_dma, +le32_to_cpu(ep_trb->generic.field[0]), +le32_to_cpu(ep_trb->generic.field[1]), +le32_to_cpu(ep_trb->generic.field[2]), +le32_to_cpu(ep_trb->generic.field[3])); + } + /* update the urb's actual_length and give back to the core */ if (usb_endpoint_xfer_control(>urb->ep->desc)) process_ctrl_td(xhci, td, ep_trb, event, ep, ); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ba46c70..e10a490 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2944,6 +2944,28 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci, } } +void xhci_cleanup_trb_error_ring(struct xhci_hcd *xhci, + unsigned int ep_index, struct xhci_td *td) +{ + struct xhci_dequeue_state deq_state; + struct xhci_virt_ep *ep; + struct usb_device *udev = td->urb->dev; + + xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, + "Cleaning up trb error endpoint ring"); + ep = >devs[udev->slot_id]->eps[ep_index]; + + xhci_find_new_dequeue_state(xhci, udev->slot_id, + ep_index, ep->stopped_stream, td, _state); + + if (!deq_state.new_deq_ptr || !deq_state.new_deq_seg) + return; + + xhci_queue_new_dequeue_state(xhci, udev->slot_id, +ep_index, ep->stopped_stream, +_state); +} + /* Called when
[PATCH v3 3/5] usb: dwc3: gadget: Write GEVNTCOUNT in interrupt
Currently GEVNTCOUNT is written in the threaded interrupt handler while processing each event. This commit moves the GEVNTCOUNT write to the hard IRQ. We then copy the events to a separate buffer for the event handler to read from. This change is in preparation of working around an issue in core version 3.00a where the interrupt cannot be de-asserted in the normal way. However, if we enable interrupt moderation, we can also de-assert it by writing to GEVNTCOUNT. Signed-off-by: John Youn--- drivers/usb/dwc3/core.c | 4 drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/gadget.c | 22 +- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index af42346..f0bb6df 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -251,6 +251,10 @@ static struct dwc3_event_buffer *dwc3_alloc_one_event_buffer(struct dwc3 *dwc, evt->dwc= dwc; evt->length = length; + evt->cache = devm_kzalloc(dwc->dev, length, GFP_KERNEL); + if (!evt->cache) + return ERR_PTR(-ENOMEM); + evt->buf= dma_alloc_coherent(dwc->dev, length, >dma, GFP_KERNEL); if (!evt->buf) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 354de24..bf63756 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -472,6 +472,7 @@ struct dwc3_trb; /** * struct dwc3_event_buffer - Software event buffer representation * @buf: _THE_ buffer + * @cache: The buffer cache used in the threaded interrupt * @length: size of this buffer * @lpos: event offset * @count: cache of last read event count register @@ -481,6 +482,7 @@ struct dwc3_trb; */ struct dwc3_event_buffer { void*buf; + void*cache; unsignedlength; unsigned intlpos; unsigned intcount; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 22ccc34..f07dd84 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2829,18 +2829,16 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt) { struct dwc3 *dwc = evt->dwc; irqreturn_t ret = IRQ_NONE; - int left; + int idx; u32 reg; - left = evt->count; - if (!(evt->flags & DWC3_EVENT_PENDING)) return IRQ_NONE; - while (left > 0) { + for (idx = 0; idx < evt->count; idx += 4) { union dwc3_event event; - event.raw = *(u32 *) (evt->buf + evt->lpos); + event.raw = *(u32 *)(evt->cache + idx); dwc3_process_event_entry(dwc, ); @@ -2853,10 +2851,6 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt) * boundary so I worry about that once we try to handle * that. */ - evt->lpos = (evt->lpos + 4) % DWC3_EVENT_BUFFERS_SIZE; - left -= 4; - - dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 4); } evt->count = 0; @@ -2889,6 +2883,7 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) { struct dwc3 *dwc = evt->dwc; u32 count; + u32 amount; u32 reg; if (pm_runtime_suspended(dwc->dev)) { @@ -2906,6 +2901,15 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) evt->count = count; evt->flags |= DWC3_EVENT_PENDING; + amount = min(count, DWC3_EVENT_BUFFERS_SIZE - evt->lpos); + memcpy(evt->cache, evt->buf + evt->lpos, amount); + + if (amount < count) + memcpy(evt->cache + amount, evt->buf, count - amount); + + evt->lpos = (evt->lpos + count) % DWC3_EVENT_BUFFERS_SIZE; + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count); + /* Mask interrupt */ reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0)); reg |= DWC3_GEVNTSIZ_INTMASK; -- 2.10.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
[PATCH v3 1/5] usb: dwc3: Add a check for the DWC_usb3 core
Add a helper function to check if we are running on a DWC_usb3 core. Signed-off-by: John Youn--- drivers/usb/dwc3/core.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index b9903c6..354de24 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1148,6 +1148,12 @@ struct dwc3_gadget_ep_cmd_params { void dwc3_set_mode(struct dwc3 *dwc, u32 mode); u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type); +/* check whether we are on the DWC_usb3 core */ +static inline bool dwc3_is_usb3(struct dwc3 *dwc) +{ + return !(dwc->revision & DWC3_REVISION_IS_DWC31); +} + /* check whether we are on the DWC_usb31 core */ static inline bool dwc3_is_usb31(struct dwc3 *dwc) { -- 2.10.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
[PATCH v3 2/5] usb: dwc3: Add a function to check properties
Add a function to check properties and call it from probe. This will allow us to add check code without bloating the probe function. This needs to be done after dwc3_get_properties() and dwc3_core_init() so that all the properties and hardware configs are available. Signed-off-by: John Youn--- drivers/usb/dwc3/core.c | 59 +++-- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 037052d..af42346 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1019,6 +1019,38 @@ static void dwc3_get_properties(struct dwc3 *dwc) } +static void dwc3_check_params(struct dwc3 *dwc) +{ + struct device *dev = dwc->dev; + + /* Check the maximum_speed parameter */ + switch (dwc->maximum_speed) { + case USB_SPEED_LOW: + case USB_SPEED_FULL: + case USB_SPEED_HIGH: + case USB_SPEED_SUPER: + case USB_SPEED_SUPER_PLUS: + break; + default: + dev_err(dev, "invalid maximum_speed parameter %d\n", + dwc->maximum_speed); + /* fall through */ + case USB_SPEED_UNKNOWN: + /* default to superspeed */ + dwc->maximum_speed = USB_SPEED_SUPER; + + /* +* default to superspeed plus if we are capable. +*/ + if (dwc3_is_usb31(dwc) && + (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == +DWC3_GHWPARAMS3_SSPHY_IFC_GEN2)) + dwc->maximum_speed = USB_SPEED_SUPER_PLUS; + + break; + } +} + static int dwc3_probe(struct platform_device *pdev) { struct device *dev = >dev; @@ -1110,32 +1142,7 @@ static int dwc3_probe(struct platform_device *pdev) goto err4; } - /* Check the maximum_speed parameter */ - switch (dwc->maximum_speed) { - case USB_SPEED_LOW: - case USB_SPEED_FULL: - case USB_SPEED_HIGH: - case USB_SPEED_SUPER: - case USB_SPEED_SUPER_PLUS: - break; - default: - dev_err(dev, "invalid maximum_speed parameter %d\n", - dwc->maximum_speed); - /* fall through */ - case USB_SPEED_UNKNOWN: - /* default to superspeed */ - dwc->maximum_speed = USB_SPEED_SUPER; - - /* -* default to superspeed plus if we are capable. -*/ - if (dwc3_is_usb31(dwc) && - (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == -DWC3_GHWPARAMS3_SSPHY_IFC_GEN2)) - dwc->maximum_speed = USB_SPEED_SUPER_PLUS; - - break; - } + dwc3_check_params(dwc); ret = dwc3_core_init_mode(dwc); if (ret) -- 2.10.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
[PATCH v3 4/5] usb: dwc3: Implement interrupt moderation
Implement interrupt moderation which allows the interrupt rate to be throttled. To enable this feature the dwc->imod_interval must be set to 1 or greater. This value specifies the minimum inter-interrupt interval, in 250 ns increments. A value of 0 disables interrupt moderation. This applies for DWC_usb3 version 3.00a and higher and for DWC_usb31 version 1.20a and higher. Signed-off-by: John Youn--- drivers/usb/dwc3/core.c | 16 drivers/usb/dwc3/core.h | 15 +++ drivers/usb/dwc3/gadget.c | 16 3 files changed, 47 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index f0bb6df..dbd6408 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1021,12 +1021,28 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->hird_threshold = hird_threshold | (dwc->is_utmi_l1_suspend << 4); + dwc->imod_interval = 0; +} + +/* check whether the core supports IMOD */ +bool dwc3_has_imod(struct dwc3 *dwc) +{ + return ((dwc3_is_usb3(dwc) && +dwc->revision >= DWC3_REVISION_300A) || + (dwc3_is_usb31(dwc) && +dwc->revision >= DWC3_USB31_REVISION_120A)); } static void dwc3_check_params(struct dwc3 *dwc) { struct device *dev = dwc->dev; + /* Check for proper value of imod_interval */ + if (dwc->imod_interval && !dwc3_has_imod(dwc)) { + dev_warn(dwc->dev, "Interrupt moderation not supported\n"); + dwc->imod_interval = 0; + } + /* Check the maximum_speed parameter */ switch (dwc->maximum_speed) { case USB_SPEED_LOW: diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index bf63756..ef81fa5 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -67,6 +67,7 @@ #define DWC3_DEVICE_EVENT_OVERFLOW 11 #define DWC3_GEVNTCOUNT_MASK 0xfffc +#define DWC3_GEVNTCOUNT_EHB(1 << 31) #define DWC3_GSNPSID_MASK 0x #define DWC3_GSNPSREV_MASK 0x @@ -149,6 +150,8 @@ #define DWC3_DEPCMDPAR00x08 #define DWC3_DEPCMD0x0c +#define DWC3_DEV_IMOD(n) (0xca00 + (n * 0x4)) + /* OTG Registers */ #define DWC3_OCFG 0xcc00 #define DWC3_OCTL 0xcc04 @@ -465,6 +468,11 @@ #define DWC3_DEPCMD_TYPE_BULK 2 #define DWC3_DEPCMD_TYPE_INTR 3 +#define DWC3_DEV_IMOD_COUNT_SHIFT 16 +#define DWC3_DEV_IMOD_COUNT_MASK (0x << 16) +#define DWC3_DEV_IMOD_INTERVAL_SHIFT 0 +#define DWC3_DEV_IMOD_INTERVAL_MASK(0x << 0) + /* Structures */ struct dwc3_trb; @@ -846,6 +854,8 @@ struct dwc3_scratchpad_array { * 1 - -3.5dB de-emphasis * 2 - No de-emphasis * 3 - Reserved + * @imod_interval: set the interrupt moderation interval in 250ns + * increments or 0 to disable. */ struct dwc3 { struct usb_ctrlrequest *ctrl_req; @@ -933,6 +943,7 @@ struct dwc3 { */ #define DWC3_REVISION_IS_DWC31 0x8000 #define DWC3_USB31_REVISION_110A (0x3131302a | DWC3_REVISION_IS_DWC31) +#define DWC3_USB31_REVISION_120A (0x3132302a | DWC3_REVISION_IS_DWC31) enum dwc3_ep0_next ep0_next_event; enum dwc3_ep0_state ep0state; @@ -991,6 +1002,8 @@ struct dwc3 { unsignedtx_de_emphasis_quirk:1; unsignedtx_de_emphasis:2; + + u16 imod_interval; }; /* -- */ @@ -1162,6 +1175,8 @@ static inline bool dwc3_is_usb31(struct dwc3 *dwc) return !!(dwc->revision & DWC3_REVISION_IS_DWC31); } +bool dwc3_has_imod(struct dwc3 *dwc); + #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) int dwc3_host_init(struct dwc3 *dwc); void dwc3_host_exit(struct dwc3 *dwc); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index f07dd84..ae80ce3 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1685,6 +1685,17 @@ static int __dwc3_gadget_start(struct dwc3 *dwc) int ret = 0; u32 reg; + /* +* Use IMOD if enabled via dwc->imod_interval. Otherwise, if +* the core supports IMOD, disable it. +*/ + if (dwc->imod_interval) { + dwc3_writel(dwc->regs, DWC3_DEV_IMOD(0), dwc->imod_interval); + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), DWC3_GEVNTCOUNT_EHB); + } else if (dwc3_has_imod(dwc)) { + dwc3_writel(dwc->regs, DWC3_DEV_IMOD(0), 0); + } + reg = dwc3_readl(dwc->regs, DWC3_DCFG); reg &= ~(DWC3_DCFG_SPEED_MASK); @@ -2862,6 +2873,11 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt) reg &= ~DWC3_GEVNTSIZ_INTMASK; dwc3_writel(dwc->regs,
[PATCH v3 5/5] usb: dwc3: Workaround for irq mask issue
This is a workaround for STAR 9000961433 which affects only version 3.00a of the DWC_usb3 core. This prevents the controller interrupt from being masked while handling events. Enabling interrupt moderation allows us to work around this issue because once the GEVNTCOUNT.count is written the IRQ is immediately deasserted and won't be asserted again until GEVNTCOUNT.EHB is cleared. Signed-off-by: John Youn--- drivers/usb/dwc3/core.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index dbd6408..994ee4a 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1043,6 +1043,18 @@ static void dwc3_check_params(struct dwc3 *dwc) dwc->imod_interval = 0; } + /* +* Workaround for STAR 9000961433 which affects only version +* 3.00a of the DWC_usb3 core. This prevents the controller +* interrupt from being masked while handling events. IMOD +* allows us to work around this issue. Enable it for the +* affected version. +*/ + if (!dwc->imod_interval && + (dwc->revision == DWC3_REVISION_300A)) { + dwc->imod_interval = 1; + } + /* Check the maximum_speed parameter */ switch (dwc->maximum_speed) { case USB_SPEED_LOW: -- 2.10.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
[PATCH v3 0/5] usb: dwc3: Interrupt moderation
This patch series implements interrupt moderation and also uses it in implementing a workaround for STAR 9000961433. Felipe, I've implemented it with a full copy of the events as it started to get messy with just one. Let me know if you still want just one. v3: * Cache the events between irq and bh v2: * Remove the devicetree binding Regards, John John Youn (5): usb: dwc3: Add a check for the DWC_usb3 core usb: dwc3: Add a function to check properties usb: dwc3: gadget: Write GEVNTCOUNT in interrupt usb: dwc3: Implement interrupt moderation usb: dwc3: Workaround for irq mask issue drivers/usb/dwc3/core.c | 91 +-- drivers/usb/dwc3/core.h | 23 drivers/usb/dwc3/gadget.c | 38 +++- 3 files changed, 117 insertions(+), 35 deletions(-) -- 2.10.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
[PATCH 2/2] usb: dwc2: Fix coverity issue in hcd_queue.c
From: Vardan MikayelyanThis fixes the coverity issues related to unreachable code with debugging off. Signed-off-by: Vardan Mikayelyan Signed-off-by: John Youn --- drivers/usb/dwc2/hcd_queue.c | 69 +++- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c index 08293f0..5713f03 100644 --- a/drivers/usb/dwc2/hcd_queue.c +++ b/drivers/usb/dwc2/hcd_queue.c @@ -355,6 +355,37 @@ static void pmap_unschedule(unsigned long *map, int bits_per_period, } } +/** + * dwc2_get_ls_map() - Get the map used for the given qh + * + * @hsotg: The HCD state structure for the DWC OTG controller. + * @qh:QH for the periodic transfer. + * + * We'll always get the periodic map out of our TT. Note that even if we're + * running the host straight in low speed / full speed mode it appears as if + * a TT is allocated for us, so we'll use it. If that ever changes we can + * add logic here to get a map out of "hsotg" if !qh->do_split. + * + * Returns: the map or NULL if a map couldn't be found. + */ +static unsigned long *dwc2_get_ls_map(struct dwc2_hsotg *hsotg, + struct dwc2_qh *qh) +{ + unsigned long *map; + + /* Don't expect to be missing a TT and be doing low speed scheduling */ + if (WARN_ON(!qh->dwc_tt)) + return NULL; + + /* Get the map and adjust if this is a multi_tt hub */ + map = qh->dwc_tt->periodic_bitmaps; + if (qh->dwc_tt->usb_tt->multi) + map += DWC2_ELEMENTS_PER_LS_BITMAP * qh->ttport; + + return map; +} + +#ifdef DWC2_PRINT_SCHEDULE /* * cat_printf() - A printf() + strcat() helper * @@ -454,35 +485,6 @@ static void pmap_print(unsigned long *map, int bits_per_period, } } -/** - * dwc2_get_ls_map() - Get the map used for the given qh - * - * @hsotg: The HCD state structure for the DWC OTG controller. - * @qh:QH for the periodic transfer. - * - * We'll always get the periodic map out of our TT. Note that even if we're - * running the host straight in low speed / full speed mode it appears as if - * a TT is allocated for us, so we'll use it. If that ever changes we can - * add logic here to get a map out of "hsotg" if !qh->do_split. - * - * Returns: the map or NULL if a map couldn't be found. - */ -static unsigned long *dwc2_get_ls_map(struct dwc2_hsotg *hsotg, - struct dwc2_qh *qh) -{ - unsigned long *map; - - /* Don't expect to be missing a TT and be doing low speed scheduling */ - if (WARN_ON(!qh->dwc_tt)) - return NULL; - - /* Get the map and adjust if this is a multi_tt hub */ - map = qh->dwc_tt->periodic_bitmaps; - if (qh->dwc_tt->usb_tt->multi) - map += DWC2_ELEMENTS_PER_LS_BITMAP * qh->ttport; - - return map; -} struct dwc2_qh_print_data { struct dwc2_hsotg *hsotg; @@ -519,9 +521,6 @@ static void dwc2_qh_schedule_print(struct dwc2_hsotg *hsotg, * If we don't have tracing turned on, don't run unless the special * define is turned on. */ -#ifndef DWC2_PRINT_SCHEDULE - return; -#endif if (qh->schedule_low_speed) { unsigned long *map = dwc2_get_ls_map(hsotg, qh); @@ -559,8 +558,12 @@ static void dwc2_qh_schedule_print(struct dwc2_hsotg *hsotg, DWC2_HS_SCHEDULE_UFRAMES, "uFrame", "us", dwc2_qh_print, _data); } - + return; } +#else +static inline void dwc2_qh_schedule_print(struct dwc2_hsotg *hsotg, + struct dwc2_qh *qh) {}; +#endif /** * dwc2_ls_pmap_schedule() - Schedule a low speed QH -- 2.10.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
[PATCH 1/2] usb: dwc2: Fix dead code in hcd.c
From: Vardan MikayelyanBecause usb_pipetype() masks urb->pipe, the default case can never be hit. Remove it. This cleans up a coverity warning. Signed-off-by: Vardan Mikayelyan Signed-off-by: John Youn --- drivers/usb/dwc2/hcd.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 3ac0085..e7b6097 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -4510,9 +4510,6 @@ static void dwc2_dump_urb_info(struct usb_hcd *hcd, struct urb *urb, case PIPE_ISOCHRONOUS: pipetype = "ISOCHRONOUS"; break; - default: - pipetype = "UNKNOWN"; - break; } dev_vdbg(hsotg->dev, " Endpoint type: %s %s (%s)\n", pipetype, @@ -4609,8 +4606,6 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, case PIPE_INTERRUPT: ep_type = USB_ENDPOINT_XFER_INT; break; - default: - dev_warn(hsotg->dev, "Wrong ep type\n"); } dwc2_urb = dwc2_hcd_urb_alloc(hsotg, urb->number_of_packets, -- 2.10.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
[PATCH] usb: dwc3: gadget: Fix dead code writing GCTL.RAMCLKSEL
The register programming code in dwc2_updated_ram_clk_sel() will never be executed. And in fact the entire function can be removed as there is no way to override the default value of GCTL.RAMCLKSEL. Remove the function and add a comment explaining where GCTL.RAMCLKSEL should be programmed if needed in the future. This fixes dead code warnings in coverity. Signed-off-by: John Youn--- drivers/usb/dwc3/gadget.c | 35 --- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 22ccc34..230ffa3 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2468,32 +2468,6 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) dwc3_writel(dwc->regs, DWC3_DCFG, reg); } -static void dwc3_update_ram_clk_sel(struct dwc3 *dwc, u32 speed) -{ - u32 reg; - u32 usb30_clock = DWC3_GCTL_CLK_BUS; - - /* -* We change the clock only at SS but I dunno why I would want to do -* this. Maybe it becomes part of the power saving plan. -*/ - - if ((speed != DWC3_DSTS_SUPERSPEED) && - (speed != DWC3_DSTS_SUPERSPEED_PLUS)) - return; - - /* -* RAMClkSel is reset to 0 after USB reset, so it must be reprogrammed -* each time on Connect Done. -*/ - if (!usb30_clock) - return; - - reg = dwc3_readl(dwc->regs, DWC3_GCTL); - reg |= DWC3_GCTL_RAMCLKSEL(usb30_clock); - dwc3_writel(dwc->regs, DWC3_GCTL, reg); -} - static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc) { struct dwc3_ep *dep; @@ -2505,7 +2479,14 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc) speed = reg & DWC3_DSTS_CONNECTSPD; dwc->speed = speed; - dwc3_update_ram_clk_sel(dwc, speed); + /* +* RAMClkSel is reset to 0 after USB reset, so it must be reprogrammed +* each time on Connect Done. +* +* Currently we always use the reset value. If any platform +* wants to set this to a different value, we need to add a +* setting and update GCTL.RAMCLKSEL here. +*/ switch (speed) { case DWC3_DSTS_SUPERSPEED_PLUS: -- 2.10.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
[PATCH v2 1/1] usb: xhci: handle endpoint error caused by TRB error
When xHCI host sees a malformed TRB in a transfer ring, it will generate a transfer event with the completion code set to COMP_TRB_ERR (5), and sets the endpoint state in output endpoint context to EP_STATE_ERROR. The endpoint enters ERROR state as the result. XHCI specification requires that Set TR Dequeue Pointer Command shall be used to transition the endpoint from Error to Stopped state. Current xHCI driver doesn't clear this endpoint error, hence the successive URB enqueue requests will result in error messages of "WARN waiting for error on ep to be cleared". And the corresponding USB device stays in unresponsive state. This patch enhances xHCI driver on this by printing out the malformed TRB and clearing the endpoint Error state. Tested-by: Wang WendySigned-off-by: Lu Baolu --- Change log: v1->v2: - Correct the email of Tested-by. drivers/usb/host/xhci-ring.c | 35 +++ drivers/usb/host/xhci.c | 22 ++ drivers/usb/host/xhci.h | 2 ++ 3 files changed, 59 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 822f88a..f81c1be 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1773,6 +1773,20 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci, xhci_ring_cmd_db(xhci); } +static void xhci_cleanup_trb_error_endpoint(struct xhci_hcd *xhci, + unsigned int slot_id, unsigned int ep_index, + unsigned int stream_id, + struct xhci_td *td, union xhci_trb *event_trb) +{ + struct xhci_virt_ep *ep = >devs[slot_id]->eps[ep_index]; + + ep->stopped_stream = stream_id; + xhci_cleanup_trb_error_ring(xhci, ep_index, td); + ep->stopped_stream = 0; + + xhci_ring_cmd_db(xhci); +} + /* Check if an error has halted the endpoint ring. The class driver will * cleanup the halt for a non-default control endpoint if we indicate a stall. * However, a babble and other errors also halt the endpoint ring, and the class @@ -1860,6 +1874,13 @@ static int finish_td(struct xhci_hcd *xhci, struct xhci_td *td, */ xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index, ep_ring->stream_id, td, ep_trb); + } else if (trb_comp_code == COMP_TRB_ERR) { + /* Clear the Endpoint error caused by a TRB error by issuing +* a set dequeue command to move the dequeue pointer past the +* last TD. +*/ + xhci_cleanup_trb_error_endpoint(xhci, slot_id, ep_index, + ep_ring->stream_id, td, ep_trb); } else { /* Update ring dequeue pointer */ while (ep_ring->dequeue != td->last_trb) @@ -2474,6 +2495,20 @@ static int handle_tx_event(struct xhci_hcd *xhci, goto cleanup; } + /* +* Dump the original TRB which caused a transfer error with +* completion code set to TRB error. +*/ + if (trb_comp_code == COMP_TRB_ERR) { + xhci_err(xhci, "Malformed transfer TRB deteced:\n"); + xhci_err(xhci, "@%016llx %08x %08x %08x %08x\n", +(unsigned long long)ep_trb_dma, +le32_to_cpu(ep_trb->generic.field[0]), +le32_to_cpu(ep_trb->generic.field[1]), +le32_to_cpu(ep_trb->generic.field[2]), +le32_to_cpu(ep_trb->generic.field[3])); + } + /* update the urb's actual_length and give back to the core */ if (usb_endpoint_xfer_control(>urb->ep->desc)) process_ctrl_td(xhci, td, ep_trb, event, ep, ); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ba46c70..e10a490 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2944,6 +2944,28 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci, } } +void xhci_cleanup_trb_error_ring(struct xhci_hcd *xhci, + unsigned int ep_index, struct xhci_td *td) +{ + struct xhci_dequeue_state deq_state; + struct xhci_virt_ep *ep; + struct usb_device *udev = td->urb->dev; + + xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep, + "Cleaning up trb error endpoint ring"); + ep = >devs[udev->slot_id]->eps[ep_index]; + + xhci_find_new_dequeue_state(xhci, udev->slot_id, + ep_index, ep->stopped_stream, td, _state); + + if (!deq_state.new_deq_ptr || !deq_state.new_deq_seg) + return; + + xhci_queue_new_dequeue_state(xhci, udev->slot_id, +ep_index, ep->stopped_stream, +
Re: [PATCH 0/4] musb fixes for v4.9-rc cycle
Hello, On Friday 11 Nov 2016 00:27:52 Laurent Pinchart wrote: > On Thursday 10 Nov 2016 13:42:34 Tony Lindgren wrote: > > * Laurent Pinchart[161110 13:29]: > >> I had to disable CONFIG_USB_GADGET is compiling it as a module prevents > >> selecting CONFIG_NOP_USB_XCEIV=y, which is a dependency for > >> CONFIG_USB_EHCI_HCD_OMAP=m. > >> > >> The new configuration resulted in a few changes, among which the most > >> notable is > >> > >> -# CONFIG_USB_MUSB_HOST is not set > >> -# CONFIG_USB_MUSB_GADGET is not set > >> -CONFIG_USB_MUSB_DUAL_ROLE=y > >> +CONFIG_USB_MUSB_HOST=y > >> > >> I then get the same error as originally reported. > > > > Yeah OK, that's the problem.. We still have musb hardware > > trying to do things on it's own and phy trying to detect > > the state. > > > > Any ideas why we have a dependency like that in Kconfig? > > Well, with CONFIG_USB_GADGET disabled, I don't expect CONFIG_USB_MUSB_GADGET > to be enabled. MUSB can only operate in host mode in that case, so musb-> > xceiv->otg->state = OTG_STATE_A_IDLE makes sense. I've tried to investigate this but I'm not familiar enough with the MUSB driver to reach any conclusion. Felipe, if you have time could you give me a hand ? To summarize the issue, on a pandaboard-es where the MUSB is wired to an OTG connector with mode rightfully set to MUSB_PORT_MODE_DUAL_ROLE in DT, a kernel configured with CONFIG_USB_MUSB_HOST=y (for instance because CONFIG_USB_GADGET is disabled) will print an endless stream of the following meessage. [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while active musb_init_controller() performs the following actions case MUSB_PORT_MODE_DUAL_ROLE: status = musb_host_setup(musb, plat->power); if (status < 0) goto fail3; status = musb_gadget_setup(musb); if (status) { musb_host_cleanup(musb); goto fail3; } status = musb_platform_set_mode(musb, MUSB_OTG); break; and as gadget support is disabled musb_gadget_setup() compiles to a no-op inline function. The system is thus configured in host mode by musb_host_setup() with musb->xceiv->otg->state = OTG_STATE_A_IDLE. A later call to musb_start() verifies the condition if (musb->port_mode != MUSB_PORT_MODE_HOST && musb->xceiv->otg->state != OTG_STATE_A_WAIT_BCON && (devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) { and sets musb->is_active to 1. Later, musb_bus_suspend() complains because is_active is not 0. What is wrong in that sequence ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/8] power: add power sequence library
On Tue, Nov 8, 2016 at 3:51 AM, Peter Chenwrote: > We have an well-known problem that the device needs to do some power > sequence before it can be recognized by related host, the typical > example like hard-wired mmc devices and usb devices. > > This power sequence is hard to be described at device tree and handled by > related host driver, so we have created a common power sequence > library to cover this requirement. The core code has supplied > some common helpers for host driver, and individual power sequence > libraries handle kinds of power sequence for devices. The pwrseq > librares always need to allocate extra instance for compatible > string match. > > pwrseq_generic is intended for general purpose of power sequence, which > handles gpios and clocks currently, and can cover other controls in > future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off > if only one power sequence is needed, else call of_pwrseq_on_list > /of_pwrseq_off_list instead (eg, USB hub driver). > > For new power sequence library, it can add its compatible string > to pwrseq_of_match_table, then the pwrseq core will match it with > DT's, and choose this library at runtime. In the first place, please document this stuff better than you have so far. To a minimum, add kerneldoc comments to all new non-trivial new functions to document what they are for and how they are expected to be used (especially the ones exported to drivers). Also, is there any guidance available for people who may want to use it? > Signed-off-by: Peter Chen > Tested-by: Maciej S. Szmigiero > Tested-by Joshua Clayton > Reviewed-by: Matthias Kaehlcke > Tested-by: Matthias Kaehlcke > --- > MAINTAINERS | 9 ++ > drivers/power/Kconfig | 1 + > drivers/power/Makefile| 1 + > drivers/power/pwrseq/Kconfig | 19 > drivers/power/pwrseq/Makefile | 2 + > drivers/power/pwrseq/core.c | 191 > ++ > drivers/power/pwrseq/pwrseq_generic.c | 183 > include/linux/power/pwrseq.h | 72 + > 8 files changed, 478 insertions(+) > create mode 100644 drivers/power/pwrseq/Kconfig > create mode 100644 drivers/power/pwrseq/Makefile > create mode 100644 drivers/power/pwrseq/core.c > create mode 100644 drivers/power/pwrseq/pwrseq_generic.c > create mode 100644 include/linux/power/pwrseq.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1cd38a7..5dab975 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9612,6 +9612,15 @@ F: include/linux/pm_* > F: include/linux/powercap.h > F: drivers/powercap/ > > +POWER SEQUENCE LIBRARY > +M: Peter Chen > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git > +L: linux...@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/power/pwrseq/ > +F: drivers/power/pwrseq/ > +F: include/linux/power/pwrseq.h/ > + > POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS > M: Sebastian Reichel > L: linux...@vger.kernel.org > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig > index 63454b5..c1bb046 100644 > --- a/drivers/power/Kconfig > +++ b/drivers/power/Kconfig > @@ -1,3 +1,4 @@ > source "drivers/power/avs/Kconfig" > source "drivers/power/reset/Kconfig" > source "drivers/power/supply/Kconfig" > +source "drivers/power/pwrseq/Kconfig" > diff --git a/drivers/power/Makefile b/drivers/power/Makefile > index ff35c71..7db8035 100644 > --- a/drivers/power/Makefile > +++ b/drivers/power/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_POWER_AVS)+= avs/ > obj-$(CONFIG_POWER_RESET) += reset/ > obj-$(CONFIG_POWER_SUPPLY) += supply/ > +obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/ > diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig > new file mode 100644 > index 000..3859a67 > --- /dev/null > +++ b/drivers/power/pwrseq/Kconfig > @@ -0,0 +1,19 @@ > +# > +# Power Sequence library > +# > + > +config POWER_SEQUENCE > + bool > + > +menu "Power Sequence Support" > + > +config PWRSEQ_GENERIC > + bool "Generic power sequence control" > + depends on OF > + select POWER_SEQUENCE > + help > + It is used for drivers which needs to do power sequence > + (eg, turn on clock, toggle reset gpio) before the related > + devices can be found by hardware. This generic one can be > + used for common power sequence control. I wouldn't set it up this way. There are two problems here. First, say a distro is going to ship a multiplatform generic kernel. How they are going to figure out whether or not to set the new symbol in that kernel? Second, how users are supposed to know whether or not they will need
Re: [PATCH v9 2/8] power: add power sequence library
On Fri, Nov 11, 2016 at 02:05:01AM +0100, Rafael J. Wysocki wrote: > On Tue, Nov 8, 2016 at 3:51 AM, Peter Chenwrote: > > We have an well-known problem that the device needs to do some power > > sequence before it can be recognized by related host, the typical > > example like hard-wired mmc devices and usb devices. > > > > This power sequence is hard to be described at device tree and handled by > > related host driver, so we have created a common power sequence > > library to cover this requirement. The core code has supplied > > some common helpers for host driver, and individual power sequence > > libraries handle kinds of power sequence for devices. The pwrseq > > librares always need to allocate extra instance for compatible > > string match. > > > > pwrseq_generic is intended for general purpose of power sequence, which > > handles gpios and clocks currently, and can cover other controls in > > future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off > > if only one power sequence is needed, else call of_pwrseq_on_list > > /of_pwrseq_off_list instead (eg, USB hub driver). > > > > For new power sequence library, it can add its compatible string > > to pwrseq_of_match_table, then the pwrseq core will match it with > > DT's, and choose this library at runtime. > > In the first place, please document this stuff better than you have so > far. To a minimum, add kerneldoc comments to all new non-trivial new > functions to document what they are for and how they are expected to > be used (especially the ones exported to drivers). > Thanks for your comments. I will add kerneldoc for main APIs. > Also, is there any guidance available for people who may want to use it? No doc now, only some guidance in this commit log. > > +config PWRSEQ_GENERIC > > + bool "Generic power sequence control" > > + depends on OF > > + select POWER_SEQUENCE > > + help > > + It is used for drivers which needs to do power sequence > > + (eg, turn on clock, toggle reset gpio) before the related > > + devices can be found by hardware. This generic one can be > > + used for common power sequence control. > > I wouldn't set it up this way. > > There are two problems here. > > First, say a distro is going to ship a multiplatform generic kernel. > How they are going to figure out whether or not to set the new symbol > in that kernel? > > Second, how users are supposed to know whether or not they will need > it even if they build the kernel by themselves? > > It would be better IMO to set things up to select the new symbol from > places making use of the code depending on it. > Will change it like below: # # Power Sequence library # menuconfig POWER_SEQUENCE bool "Power sequence control" depends on OF help It is used for drivers which needs to do power sequence (eg, turn on clock, toggle reset gpio) before the related devices can be found by hardware. if POWER_SEQUENCE config PWRSEQ_GENERIC bool "Generic power sequence control" default y help This is the generic power sequence control library, and is supposed to support common power sequence usage. endif And the current user usb core will select POWER_SEQUENCE. -- Best Regards, Peter Chen -- 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: xhci: handle endpoint error caused by TRB error
Hi, On 11/11/2016 09:03 AM, Lu Baolu wrote: > When xHCI host sees a malformed TRB in a transfer ring, > it will generate a transfer event with the completion > code set to COMP_TRB_ERR (5), and sets the endpoint > state in output endpoint context to EP_STATE_ERROR. > The endpoint enters ERROR state as the result. > > XHCI specification requires that Set TR Dequeue Pointer > Command shall be used to transition the endpoint from > Error to Stopped state. Current xHCI driver doesn't > clear this endpoint error, hence the successive URB > enqueue requests will result in error messages of > "WARN waiting for error on ep to be cleared". And the > corresponding USB device stays in unresponsive state. > > This patch enhances xHCI driver on this by printing out > the malformed TRB and clearing the endpoint Error state. > > Tested-by: Wang Wendy^ intel.com I will correct it with a v2. 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: [PATCH v5 00/23] Support qcom's HSIC USB and rewrite USB2 HS support
On Tue, Oct 18, 2016 at 01:51:39PM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-10-18 02:31:40) > > On Mon, Oct 17, 2016 at 06:56:13PM -0700, Stephen Boyd wrote: > > > I've also sent separate patches for other minor pieces to make this > > > all work. The full tree can be found here[2], hacks and all to get > > > things working. I've tested this on the db410c, apq8074 dragonboard, > > > and ifc6410 with configfs gadgets and otg cables. > > > > > > Patches based on v4.8-rc1 > > Oops should be v4.9-rc1 here. > > > > > > > Changes from v4: > > > * Picked up Acks from Rob > > > * Updated HS phy init sequence DT property to restrict it to offsets > > > > I remembered that you got all my acks for chipidea patches, right? I did > > not check for this series. > > Sorry I've added in one more patch > >usb: chipidea: Emulate OTGSC interrupt enable path > > to fix extcon interrupt emulation even further. > > > > > Besides, the patch "gpu: Remove depends on RESET_CONTROLLER when not a > > provider" [1] still not be accepted, I need this patch to be merged > > first, then apply your chipidea part, otherwise, there is a building > > warning. > > > > [1] https://patchwork.kernel.org/patch/9322583/ > > Yes, I'm going to resend that patch now. I hope that David will apply it > for -rc2. Stephen, just a mind. I have rebased Greg's usb-next tree (v4.9-rc3 on it), your GPU fix is still not there. -- Best Regards, Peter Chen -- 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 0/2] r8152: rx patches
Let the rx sw checksum available and add some checks for rx desc. Hayes Wang (2): r8152: fix the sw rx checksum is unavailable r8152: rx descriptor check drivers/net/usb/r8152.c | 46 +- 1 file changed, 45 insertions(+), 1 deletion(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
Fix the hw rx checksum is always enabled, and the user couldn't switch it to sw rx checksum. Note that the RTL_VER_01 only supports sw rx checksum only. Besides, the hw rx checksum for RTL_VER_02 is disabled after commit b9a321b48af4 ("r8152: Fix broken RX checksums."). Re-enable it. Signed-off-by: Hayes Wang--- drivers/net/usb/r8152.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 75c5168..0e42a78 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -1730,7 +1730,7 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc *rx_desc) u8 checksum = CHECKSUM_NONE; u32 opts2, opts3; - if (tp->version == RTL_VER_01 || tp->version == RTL_VER_02) + if (!(tp->netdev->features & NETIF_F_RXCSUM)) goto return_result; opts2 = le32_to_cpu(rx_desc->opts2); @@ -4307,6 +4307,11 @@ static int rtl8152_probe(struct usb_interface *intf, NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | NETIF_F_IPV6_CSUM | NETIF_F_TSO6; + if (tp->version == RTL_VER_01) { + netdev->features &= ~NETIF_F_RXCSUM; + netdev->hw_features &= ~NETIF_F_RXCSUM; + } + netdev->ethtool_ops = netif_set_gso_max_size(netdev, RTL_LIMITED_TSO_SIZE); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net 2/2] r8152: rx descriptor check
For some platforms, the data in memory is not the same with the one from the device. That is, the data of memory is unbelievable. The check is used to find out this situation. Signed-off-by: Hayes Wang--- drivers/net/usb/r8152.c | 49 + 1 file changed, 49 insertions(+) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 0e42a78..e766121 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -1756,6 +1756,43 @@ static u8 r8152_rx_csum(struct r8152 *tp, struct rx_desc *rx_desc) return checksum; } +static int invalid_rx_desc(struct r8152 *tp, struct rx_desc *rx_desc) +{ + u32 opts1 = le32_to_cpu(rx_desc->opts1); + u32 opts2 = le32_to_cpu(rx_desc->opts2); + unsigned int pkt_len = opts1 & RX_LEN_MASK; + + switch (tp->version) { + case RTL_VER_01: + case RTL_VER_02: + if (pkt_len > RTL8152_RMS) + return -EIO; + break; + default: + if (pkt_len > RTL8153_RMS) + return -EIO; + break; + } + + switch (opts2 & (RD_IPV4_CS | RD_IPV6_CS)) { + case (RD_IPV4_CS | RD_IPV6_CS): + return -EIO; + case RD_IPV4_CS: + case RD_IPV6_CS: + switch (opts2 & (RD_UDP_CS | RD_TCP_CS)) { + case (RD_UDP_CS | RD_TCP_CS): + return -EIO; + default: + break; + } + break; + default: + break; + } + + return 0; +} + static int rx_bottom(struct r8152 *tp, int budget) { unsigned long flags; @@ -1812,6 +1849,18 @@ static int rx_bottom(struct r8152 *tp, int budget) unsigned int pkt_len; struct sk_buff *skb; + if (unlikely(invalid_rx_desc(tp, rx_desc))) { + if (net_ratelimit()) + netif_err(tp, rx_err, netdev, + "Memory unbelievable\n"); + if (tp->netdev->features & NETIF_F_RXCSUM) { + tp->netdev->features &= ~NETIF_F_RXCSUM; + netif_err(tp, rx_err, netdev, + "rx checksum off\n"); + } + break; + } + pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK; if (pkt_len < ETH_ZLEN) break; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
sdhci_alloc_host() returns an error pointer when it fails. but mmc_alloc_host() cannot. This series allow to propagate a proper error code when host-allocation fails. Masahiro Yamada (2): mmc: allow mmc_alloc_host() to return proper error code mmc: tmio: allow tmio_mmc_host_alloc() to return proper error code drivers/mmc/core/host.c | 11 ++- drivers/mmc/host/android-goldfish.c | 4 ++-- drivers/mmc/host/atmel-mci.c| 4 ++-- drivers/mmc/host/au1xmmc.c | 4 ++-- drivers/mmc/host/bfin_sdh.c | 4 ++-- drivers/mmc/host/cb710-mmc.c| 4 ++-- drivers/mmc/host/davinci_mmc.c | 4 ++-- drivers/mmc/host/dw_mmc.c | 4 ++-- drivers/mmc/host/jz4740_mmc.c | 4 ++-- drivers/mmc/host/mmc_spi.c | 9 ++--- drivers/mmc/host/mmci.c | 4 ++-- drivers/mmc/host/moxart-mmc.c | 4 ++-- drivers/mmc/host/mtk-sd.c | 4 ++-- drivers/mmc/host/mvsdio.c | 4 ++-- drivers/mmc/host/mxcmmc.c | 4 ++-- drivers/mmc/host/mxs-mmc.c | 4 ++-- drivers/mmc/host/omap.c | 4 ++-- drivers/mmc/host/omap_hsmmc.c | 4 ++-- drivers/mmc/host/pxamci.c | 4 ++-- drivers/mmc/host/rtsx_pci_sdmmc.c | 4 ++-- drivers/mmc/host/rtsx_usb_sdmmc.c | 4 ++-- drivers/mmc/host/s3cmci.c | 4 ++-- drivers/mmc/host/sdhci.c| 4 ++-- drivers/mmc/host/sdricoh_cs.c | 4 ++-- drivers/mmc/host/sh_mmcif.c | 4 ++-- drivers/mmc/host/sh_mobile_sdhi.c | 4 ++-- drivers/mmc/host/sunxi-mmc.c| 4 ++-- drivers/mmc/host/tifm_sd.c | 4 ++-- drivers/mmc/host/tmio_mmc.c | 4 +++- drivers/mmc/host/tmio_mmc_pio.c | 4 ++-- drivers/mmc/host/toshsd.c | 4 ++-- drivers/mmc/host/usdhi6rol0.c | 4 ++-- drivers/mmc/host/ushc.c | 4 ++-- drivers/mmc/host/via-sdmmc.c| 4 ++-- drivers/mmc/host/vub300.c | 4 ++-- drivers/mmc/host/wbsd.c | 4 ++-- drivers/mmc/host/wmt-sdmmc.c| 4 ++-- drivers/staging/greybus/sdio.c | 4 ++-- 38 files changed, 85 insertions(+), 79 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mmc: allow mmc_alloc_host() to return proper error code
Currently, mmc_alloc_host() returns NULL on error, so its callers cannot return anything but -ENOMEM when it fails, assuming the most failure cases are due to memory shortage, but it is not true. Allow mmc_alloc_host() to return an error pointer, then propagate the proper error code to its callers. Signed-off-by: Masahiro Yamada--- drivers/mmc/core/host.c | 11 ++- drivers/mmc/host/android-goldfish.c | 4 ++-- drivers/mmc/host/atmel-mci.c| 4 ++-- drivers/mmc/host/au1xmmc.c | 4 ++-- drivers/mmc/host/bfin_sdh.c | 4 ++-- drivers/mmc/host/cb710-mmc.c| 4 ++-- drivers/mmc/host/davinci_mmc.c | 4 ++-- drivers/mmc/host/dw_mmc.c | 4 ++-- drivers/mmc/host/jz4740_mmc.c | 4 ++-- drivers/mmc/host/mmc_spi.c | 9 ++--- drivers/mmc/host/mmci.c | 4 ++-- drivers/mmc/host/moxart-mmc.c | 4 ++-- drivers/mmc/host/mtk-sd.c | 4 ++-- drivers/mmc/host/mvsdio.c | 4 ++-- drivers/mmc/host/mxcmmc.c | 4 ++-- drivers/mmc/host/mxs-mmc.c | 4 ++-- drivers/mmc/host/omap.c | 4 ++-- drivers/mmc/host/omap_hsmmc.c | 4 ++-- drivers/mmc/host/pxamci.c | 4 ++-- drivers/mmc/host/rtsx_pci_sdmmc.c | 4 ++-- drivers/mmc/host/rtsx_usb_sdmmc.c | 4 ++-- drivers/mmc/host/s3cmci.c | 4 ++-- drivers/mmc/host/sdhci.c| 4 ++-- drivers/mmc/host/sdricoh_cs.c | 4 ++-- drivers/mmc/host/sh_mmcif.c | 4 ++-- drivers/mmc/host/sunxi-mmc.c| 4 ++-- drivers/mmc/host/tifm_sd.c | 4 ++-- drivers/mmc/host/tmio_mmc_pio.c | 2 +- drivers/mmc/host/toshsd.c | 4 ++-- drivers/mmc/host/usdhi6rol0.c | 4 ++-- drivers/mmc/host/ushc.c | 4 ++-- drivers/mmc/host/via-sdmmc.c| 4 ++-- drivers/mmc/host/vub300.c | 4 ++-- drivers/mmc/host/wbsd.c | 4 ++-- drivers/mmc/host/wmt-sdmmc.c| 4 ++-- drivers/staging/greybus/sdio.c | 4 ++-- 36 files changed, 79 insertions(+), 75 deletions(-) diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index 98f25ff..b3e13e0 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -349,7 +349,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) host = kzalloc(sizeof(struct mmc_host) + extra, GFP_KERNEL); if (!host) - return NULL; + return ERR_PTR(-ENOMEM); /* scanning will be enabled when we're ready */ host->rescan_disable = 1; @@ -357,7 +357,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) again: if (!ida_pre_get(_host_ida, GFP_KERNEL)) { kfree(host); - return NULL; + return ERR_PTR(-ENOMEM); } spin_lock(_host_lock); @@ -368,7 +368,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) goto again; } else if (err) { kfree(host); - return NULL; + return ERR_PTR(err); } dev_set_name(>class_dev, "mmc%d", host->index); @@ -379,9 +379,10 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev) device_initialize(>class_dev); device_enable_async_suspend(>class_dev); - if (mmc_gpio_alloc(host)) { + err = mmc_gpio_alloc(host); + if (err < 0) { put_device(>class_dev); - return NULL; + return ERR_PTR(err); } spin_lock_init(>lock); diff --git a/drivers/mmc/host/android-goldfish.c b/drivers/mmc/host/android-goldfish.c index dca5518..7363663 100644 --- a/drivers/mmc/host/android-goldfish.c +++ b/drivers/mmc/host/android-goldfish.c @@ -467,8 +467,8 @@ static int goldfish_mmc_probe(struct platform_device *pdev) return -ENXIO; mmc = mmc_alloc_host(sizeof(struct goldfish_mmc_host), >dev); - if (mmc == NULL) { - ret = -ENOMEM; + if (IS_ERR(mmc)) { + ret = PTR_ERR(mmc); goto err_alloc_host_failed; } diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c index 0ad8ef5..d0cb112 100644 --- a/drivers/mmc/host/atmel-mci.c +++ b/drivers/mmc/host/atmel-mci.c @@ -2296,8 +2296,8 @@ static int atmci_init_slot(struct atmel_mci *host, struct atmel_mci_slot *slot; mmc = mmc_alloc_host(sizeof(struct atmel_mci_slot), >pdev->dev); - if (!mmc) - return -ENOMEM; + if (IS_ERR(mmc)) + return PTR_ERR(mmc); slot = mmc_priv(mmc); slot->mmc = mmc; diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c index ed77fbfa..d97b409 100644 --- a/drivers/mmc/host/au1xmmc.c +++ b/drivers/mmc/host/au1xmmc.c @@ -951,9 +951,9 @@ static int au1xmmc_probe(struct platform_device *pdev) int ret, iflag; mmc
Re: [PATCH v3 3/5] net: asix: Fix AX88772x resume failures
Hi Robert, On 29/08/16 14:32, robert.f...@collabora.com wrote: > From: Robert Foss> > From: Allan Chou > > 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 After this commit, I have started seeing the following messages during system suspend on various tegra boards using asix ethernet dongles ... [ 288.667010] PM: Syncing filesystems ... done. [ 288.672223] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 288.680505] Double checking all user space processes after OOM killer disable... (elapsed 0.000 seconds) [ 288.690193] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 288.698987] Suspending console(s) (use no_console_suspend to debug) [ 288.706605] asix 1-1:1.0 eth0: Failed to read reg index 0x: -19 [ 288.706613] asix 1-1:1.0 eth0: Error reading Medium Status register: ffed [ 288.706621] asix 1-1:1.0 eth0: Failed to write reg index 0x: -19 [ 288.706629] asix 1-1:1.0 eth0: Failed to write Medium Mode mode to 0xfeed: ffed [ 288.759167] PM: suspend of devices complete after 52.772 msecs Interestingly, it only seems to happen if the ethernet is in a disconnected state when entering suspend. I have not had chance to look at this any further, but wanted to see if you had any thoughts. 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
RE: [PATCH v2 03/30] usb: dwc2: Make the DMA descriptor structure packed
From: John Youn > Sent: 10 November 2016 03:28 > From: Vahram Aharonyan> > Make the DMA descriptor structure packed to guarantee alignment and size > in memory. > > Signed-off-by: John Youn > --- > drivers/usb/dwc2/hw.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h > index 6031efe..ee827e8 100644 > --- a/drivers/usb/dwc2/hw.h > +++ b/drivers/usb/dwc2/hw.h > @@ -802,7 +802,7 @@ > struct dwc2_dma_desc { > u32 status; > u32 buf; > -}; > +} __packed; Nack (not that my nacks have much force). You clearly do not understand what __packed means. The dma descriptor almost certainly has to be 32bit aligned (if not 64). By adding __packed you are telling the compiler that the structure can be aligned to any boundary (eg 4n+1) so that it must use byte accesses and shifts to access the members on systems that can't do misaligned accesses. I can't help feeling this isn't what you want. I suspect you are trying to tell the compiler not to add hidden padding. While the C language allows arbitrary padding after structure elements, the implementations for specific architectures define when such padding is added. Padding will not be added if an item is already on its natural boundary in any of the sane architecture specific definitions. Typical variations are whether 64bit items only need 32bit alignment. Some odd architectures specify 32bit alignment for all structs (don't think linux has any like that). By using __packed you are relying on a compiler extension - and so can assume the compiler is used a sane structure layout. If you are being really picky you could add a compile-time assert that the structure is the correct size - but that isn't worth it for 8 bytes. David -- 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 0/4] musb fixes for v4.9-rc cycle
Hi Tony, Thank you for the patches. On Monday 07 Nov 2016 14:50:16 Tony Lindgren wrote: > Hi all, > > Here are musb fixes for the issues that I've been able to track down. > Not sure if these will help with the problem Ladis was seeing as I'm > not able to reproduce that one it seems. > > As many people depend on this driver I'd like to have these merged > for v4.9-rc cycle after review and testing. > > Please review and test. You need to use v4.9-rc3 or later for testing > because of the earlier fixes. The series fixes my problems, both with the original and latest version of patch 2/4. Tested-by: Laurent PinchartI have however seen the following warning once with the original version of 2/4. [3.094116] usb 1-1: New USB device found, idVendor=0424, idProduct=9514 [3.101257] usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [3.110626] [ cut here ] [3.110717] WARNING: CPU: 0 PID: 4 at /home/laurent/src/kernel/omap4/linux-2.6/drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x220/0x348 [3.110717] 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4CFG (Read): Data Access in User mode during Functional access [3.110717] Modules linked in: [3.110717] CPU: 0 PID: 4 Comm: kworker/0:0 Not tainted 4.9.0-rc4-00577-g1cd727de4a19 #29 [3.110717] Hardware name: Generic OMAP4 (Flattened Device Tree) [3.110717] Workqueue: events musb_irq_work [3.158813] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [3.158813] [] (show_stack) from [] (dump_stack+0xa8/0xe0) [3.158813] [] (dump_stack) from [] (__warn+0xd8/0x104) [3.158813] [] (__warn) from [] (warn_slowpath_fmt+0x38/0x48) [3.158813] [] (warn_slowpath_fmt) from [] (l3_interrupt_handler+0x220/0x348) [3.203613] [] (l3_interrupt_handler) from [] (__handle_irq_event_percpu+0x98/0x3ec) [3.203613] [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x1c/0x58) [3.203613] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x38/0x5c) [3.233123] [] (handle_irq_event) from [] (handle_fasteoi_irq+0xcc/0x1a4) [3.233123] [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x18/0x28) [3.233123] [] (generic_handle_irq) from [] (__handle_domain_irq+0x64/0xdc) [3.251190] [] (__handle_domain_irq) from [] (gic_handle_irq+0x48/0x9c) [3.251190] [] (gic_handle_irq) from [] (__irq_svc+0x70/0x98) [3.251190] Exception stack(0xee8c7e18 to 0xee8c7e60) [3.251190] 7e00: fc0ab000 0060 [3.282348] 7e20: 001f ef6a7ac0 fc0ab000 0060 c0dcda2c c06a2928 [3.282348] 7e40: c0d0512c c0d0512c ee8c7e68 c06a2928 c06a34b8 6153 [3.282348] [] (__irq_svc) from [] (musb_default_readb+0x48/0x184) [3.282348] [] (musb_default_readb) from [] (musb_irq_work+0x1c/0x1cc) [3.316528] [] (musb_irq_work) from [] (process_one_work+0x1d4/0x6b8) [3.325256] [] (process_one_work) from [] (worker_thread+0x164/0x488) [3.325256] [] (worker_thread) from [] (kthread+0xd0/0xec) [3.325256] [] (kthread) from [] (ret_from_fork+0x14/0x24) [3.325256] ---[ end trace 0c154bedf8e63312 ]--- [3.363372] twl6030_usb 4807.i2c:twl@48:usb-comparator: Initialized TWL6030 USB module [3.365051] hub 1-1:1.0: USB hub found [3.365325] hub 1-1:1.0: 5 ports detected It never occurred with the latest version of the patch, but I can't seem to reproduce it either with the original version. > Tony Lindgren (4): > usb: musb: Fix broken use of static variable for multiple instances > usb: musb: Fix sleeping function called from invalid context for hdrc > glue > usb: musb: Fix PM for hub disconnect > phy: twl4030-usb: Fix for musb session bit based PM > > drivers/phy/phy-twl4030-usb.c | 4 +- > drivers/usb/musb/musb_core.c | 107 -- > drivers/usb/musb/musb_core.h | 13 - > drivers/usb/musb/musb_dsps.c | 24 ++--- > drivers/usb/musb/musb_gadget.c | 28 --- > drivers/usb/musb/tusb6010.c| 6 +-- > 6 files changed, 148 insertions(+), 34 deletions(-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] mmc: allow mmc_alloc_host() and tmio_mmc_host_alloc()
On Thu, Nov 10, 2016 at 10:24:21PM +0900, Masahiro Yamada wrote: > > sdhci_alloc_host() returns an error pointer when it fails. > but mmc_alloc_host() cannot. > > This series allow to propagate a proper error code > when host-allocation fails. Why? What can we really do about the error except give up? Why does having a explicit error value make any difference to the caller, they can't do anything different, right? I suggest just leaving it as-is, it's simple, and you don't have to mess with PTR_ERR() anywhere. 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: [PATCH 2/2] usb: dwc3: gadget: Remove descriptor arguments to ep_config
Hi, John Younwrites: > This function has access to the descriptors via the usb_ep. > > Signed-off-by: John Youn > --- > drivers/usb/dwc3/gadget.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 0e73383..4914182 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -488,16 +488,19 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc, > struct dwc3_ep *dep) > } > > static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep, > - const struct usb_endpoint_descriptor *desc, > - const struct usb_ss_ep_comp_descriptor *comp_desc, > bool modify, bool restore) > { > struct dwc3_gadget_ep_cmd_params params; > + const struct usb_endpoint_descriptor *desc; > + const struct usb_ss_ep_comp_descriptor *comp_desc; > > if (dev_WARN_ONCE(dwc->dev, modify && restore, > "Can't modify and restore\n")) > return -EINVAL; > > + desc = dep->endpoint.desc; > + comp_desc = dep->endpoint.comp_desc; > + > memset(, 0x00, sizeof(params)); > > params.param0 = DWC3_DEPCFG_EP_TYPE(usb_endpoint_type(desc)) > @@ -592,7 +595,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, > > ep = >endpoint; > desc = ep->desc; > - ret = dwc3_gadget_set_ep_config(dwc, dep, desc, ep->comp_desc, > + ret = dwc3_gadget_set_ep_config(dwc, dep, > modify, restore); oh I see, you split into two patches :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH v12 0/9] usbip: exporting devices
On Thu, Oct 13, 2016 at 12:52:04PM +0900, Nobuo Iwata wrote: > Dear all, > > This series of patches adds exporting device operation to USB/IP. What do you mean by "exporting"? You never really explain that, so again, I don't understand why this patchset is needed :( 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: [PATCH v12 1/9] usbip: exporting devices: modifications to network header
On Thu, Oct 13, 2016 at 12:52:05PM +0900, Nobuo Iwata wrote: > Modification to export and un-export response in > tools/usb/usbip/src/usbip_network.h. It just changes return code type > from int to uint32_t as same as other responses. > > Added export and un-export request/response to > Documentation/usb/usbip_protocol.txt. > > Signed-off-by: Nobuo Iwata> --- > Documentation/usb/usbip_protocol.txt | 204 --- Documentation should be a last patch, as you really are not implementing these network changes in this patch :( -- 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 v12 2/9] usbip: exporting devices: modifications to host side libraries
On Thu, Oct 13, 2016 at 12:52:06PM +0900, Nobuo Iwata wrote: > usbip_get_device() method in usbip_host_driver_ops was not used. It is > modified as a function to find an exported device for new operations > 'connect' and 'disconnect'. > > bind and unbind function are exported for the new operations. > > Signed-off-by: Nobuo Iwata> --- > tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++ > tools/usb/usbip/libsrc/usbip_host_common.h | 8 > tools/usb/usbip/src/usbip.h| 3 +++ > tools/usb/usbip/src/usbip_bind.c | 7 --- > tools/usb/usbip/src/usbip_unbind.c | 7 --- > 5 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c > b/tools/usb/usbip/libsrc/usbip_host_common.c > index 9d41522..6a98d6c 100644 > --- a/tools/usb/usbip/libsrc/usbip_host_common.c > +++ b/tools/usb/usbip/libsrc/usbip_host_common.c > @@ -256,17 +256,15 @@ int usbip_export_device(struct usbip_exported_device > *edev, int sockfd) > } > > struct usbip_exported_device *usbip_generic_get_device( > - struct usbip_host_driver *hdriver, int num) > + struct usbip_host_driver *hdriver, char *busid) > { > struct list_head *i; > struct usbip_exported_device *edev; > - int cnt = 0; > > list_for_each(i, >edev_list) { > edev = list_entry(i, struct usbip_exported_device, node); > - if (num == cnt) > + if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE)) > return edev; > - cnt++; > } > > return NULL; > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h > b/tools/usb/usbip/libsrc/usbip_host_common.h > index a64b803..f9a9def 100644 > --- a/tools/usb/usbip/libsrc/usbip_host_common.h > +++ b/tools/usb/usbip/libsrc/usbip_host_common.h > @@ -38,7 +38,7 @@ struct usbip_host_driver_ops { > void (*close)(struct usbip_host_driver *hdriver); > int (*refresh_device_list)(struct usbip_host_driver *hdriver); > struct usbip_exported_device * (*get_device)( > - struct usbip_host_driver *hdriver, int num); > + struct usbip_host_driver *hdriver, char *busid); > > int (*read_device)(struct udev_device *sdev, > struct usbip_usb_device *dev); > @@ -86,11 +86,11 @@ static inline int usbip_refresh_device_list(struct > usbip_host_driver *hdriver) > } > > static inline struct usbip_exported_device * > -usbip_get_device(struct usbip_host_driver *hdriver, int num) > +usbip_get_device(struct usbip_host_driver *hdriver, char *busid) > { > if (!hdriver->ops.get_device) > return NULL; > - return hdriver->ops.get_device(hdriver, num); > + return hdriver->ops.get_device(hdriver, busid); > } > > /* Helper functions for implementing driver backend */ > @@ -99,6 +99,6 @@ void usbip_generic_driver_close(struct usbip_host_driver > *hdriver); > int usbip_generic_refresh_device_list(struct usbip_host_driver *hdriver); > int usbip_export_device(struct usbip_exported_device *edev, int sockfd); > struct usbip_exported_device *usbip_generic_get_device( > - struct usbip_host_driver *hdriver, int num); > + struct usbip_host_driver *hdriver, char *busid); > > #endif /* __USBIP_HOST_COMMON_H */ > diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h > index 84fe66a..c296910 100644 > --- a/tools/usb/usbip/src/usbip.h > +++ b/tools/usb/usbip/src/usbip.h > @@ -37,4 +37,7 @@ void usbip_list_usage(void); > void usbip_bind_usage(void); > void usbip_unbind_usage(void); > > +int usbip_bind_device(char *busid); > +int usbip_unbind_device(char *busid); > + > #endif /* __USBIP_H */ > diff --git a/tools/usb/usbip/src/usbip_bind.c > b/tools/usb/usbip/src/usbip_bind.c > index fa46141..1c09338 100644 > --- a/tools/usb/usbip/src/usbip_bind.c > +++ b/tools/usb/usbip/src/usbip_bind.c > @@ -1,5 +1,6 @@ > /* > - * Copyright (C) 2011 matt mooney > + * Copyright (C) 2015 Nobuo Iwata > + * 2011 matt mooney Minor nit, you are not modifying this file enough to claim copyright, according to the copyright rules I am familiar with, so please do not do so here. > * 2005-2007 Takahiro Hirofuchi > * > * This program is free software: you can redistribute it and/or modify > @@ -139,7 +140,7 @@ static int unbind_other(char *busid) > return status; > } > > -static int bind_device(char *busid) > +int usbip_bind_device(char *busid) > { > int rc; > struct udev *udev; > @@ -200,7 +201,7 @@ int usbip_bind(int argc, char *argv[]) > > switch (opt) { > case 'b': > - ret = bind_device(optarg); > + ret = usbip_bind_device(optarg); > goto out; > default: > goto
Re: [PATCH v12 4/9] usbip: exporting devices: new disconnect operation
On Thu, Oct 13, 2016 at 12:52:08PM +0900, Nobuo Iwata wrote: > New disconnect operation. We need a lot more text here than just this, as it does not explain why this patch is needed at all :( > > Signed-off-by: Nobuo Iwata> --- > tools/usb/usbip/src/Makefile.am| 2 +- > tools/usb/usbip/src/usbip.c| 6 + > tools/usb/usbip/src/usbip.h| 2 + > tools/usb/usbip/src/usbip_disconnect.c | 215 + > 4 files changed, 224 insertions(+), 1 deletion(-) > > diff --git a/tools/usb/usbip/src/Makefile.am b/tools/usb/usbip/src/Makefile.am > index 0947476..42760c3 100644 > --- a/tools/usb/usbip/src/Makefile.am > +++ b/tools/usb/usbip/src/Makefile.am > @@ -7,6 +7,6 @@ sbin_PROGRAMS := usbip usbipd > usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \ >usbip_attach.c usbip_detach.c usbip_list.c \ >usbip_bind.c usbip_unbind.c usbip_port.c \ > - usbip_connect.c > + usbip_connect.c usbip_disconnect.c > > usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c > diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c > index 584d7d5..f0e9e06 100644 > --- a/tools/usb/usbip/src/usbip.c > +++ b/tools/usb/usbip/src/usbip.c > @@ -83,6 +83,12 @@ static const struct command cmds[] = { > .usage = usbip_connect_usage > }, > { > + .name = "disconnect", > + .fn= usbip_disconnect, > + .help = "Disconnect a USB device from a remote computer", > + .usage = usbip_disconnect_usage > + }, > + { > .name = "list", > .fn= usbip_list, > .help = "List exportable or local USB devices", > diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h > index f365353..a8cbd16 100644 > --- a/tools/usb/usbip/src/usbip.h > +++ b/tools/usb/usbip/src/usbip.h > @@ -32,6 +32,7 @@ int usbip_bind(int argc, char *argv[]); > int usbip_unbind(int argc, char *argv[]); > int usbip_port_show(int argc, char *argv[]); > int usbip_connect(int argc, char *argv[]); > +int usbip_disconnect(int argc, char *argv[]); > > void usbip_attach_usage(void); > void usbip_detach_usage(void); > @@ -39,6 +40,7 @@ void usbip_list_usage(void); > void usbip_bind_usage(void); > void usbip_unbind_usage(void); > void usbip_connect_usage(void); > +void usbip_disconnect_usage(void); > > int usbip_bind_device(char *busid); > int usbip_unbind_device(char *busid); > diff --git a/tools/usb/usbip/src/usbip_disconnect.c > b/tools/usb/usbip/src/usbip_disconnect.c > new file mode 100644 > index 000..8155384 > --- /dev/null > +++ b/tools/usb/usbip/src/usbip_disconnect.c > @@ -0,0 +1,215 @@ > +/* > + * Copyright (C) 2015 Nobuo Iwata > + * 2011 matt mooney > + * 2005-2007 Takahiro Hirofuchi How can others have copyright on a brand new file? Is it based on their work? If so, please write that. > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 2 of the License, or > + * (at your option) any later version. Are you sure you mean "any later version"? 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: [PATCH v12 7/9] usbip: exporting devices: new application-side daemon
On Thu, Oct 13, 2016 at 12:52:11PM +0900, Nobuo Iwata wrote: > New application(vhci)-side daemon. Again, more text here please. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] USB hub_probe: rework ugly goto-into-compound-statement
On Wed, Nov 09, 2016 at 06:35:21PM +0300, Eugene Korenevsky wrote: > Rework smelling code (goto inside compound statement). Perhaps this is > legacy. Anyway such code is not appropriate for Linux kernel. > > Changes since v3: fix typo > Changes since v2: extract the code to static function > Changes since v1: fix spaces instead of tab, add missing 'Signed-off-by' changes go below the --- line, please read Documentation/SubmittingPatches for all of the details. 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: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability
On Thu, Nov 10, 2016 at 09:56:41AM +0100, Thomas Gleixner wrote: > On Thu, 10 Nov 2016, Lu Baolu wrote: > > This seems to be a common issue for all early printk drivers. > > No. The other early printk drivers like serial do not have that problem as > they simply do: > >while (*buf) { > while (inb(UART) & TX_BUSY) >cpu_relax(); > outb(*buf++, UART); >} Right, which is why actual UARTs rule. If only laptops still had pinouts for them life would be so much better. Ideally the USB debug port would be a virtual UART and its interface as simple and robust. > The wait for the UART to become ready is independent of the context as it > solely depends on the hardware. > > As a result you can see the output from irq/nmi intermingled with the one > from thread context, but that's the only problem they have. > > The only thing you can do to make this work is to prevent printing in NMI > context: > > write() > { > if (in_nmi()) > return; > > raw_spinlock_irqsave(, flags); > > > That fully serializes the writes and just ignores NMI context printks. Not > optimal, but I fear that's all you can do. I would also suggest telling the hardware people they have designed something near the brink of useless. If you cannot do random exception context debugging (#DB, #NMI, #MCE etc..) then there's a whole host of problems that simply cannot be debugged. Also note that kdb runs from NMI context, so you'll not be able to support that either. -- 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 v6 2/2] usbip: vhci extension: dynamic extension
On Fri, Sep 30, 2016 at 02:41:54PM +0900, Nobuo Iwata wrote: > Modification for dynamic device registration and unregistration. > > 1. kernel config > > Followings are added. > > USBIP_VHCI_HC_PORTS: Number of ports per USB/IP virtual host > controller. The default is 8 - same as current VHCI_NPORTS. > USBIP_VHCI_MAX_HCS: Muximum number of USB/IP virtual host controllers. > The default is 1. > USBIP_VHCI_INIT_HCS: Initial number of USB/IP virtual host controllers. > The default is 1. > Static number of devices: USBIP_VHCI_NR_HCS in patch 1/3 is removed > with this patch. There is no patch 1/3 in this series, it's only a 2 patch series! I am totally confused now. Please fix this up, and put a lot of the text in your 0/2 cover letter in these 2 patches so that we know what is going on with them. I think I do, but based on this line, I don't think so anymore :( 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: [PATCH v3 5/6] usb: dwc3: use bus->sysdev for DMA configuration
Hi, On 10 November 2016 at 16:20, Sriram Dashwrote: > From: Arnd Bergmann > > The dma ops for dwc3 devices are not set properly. So, use a > physical device sysdev, which will be inherited from parent, > to set the hardware / firmware parameters like dma. > > Signed-off-by: Arnd Bergmann > Signed-off-by: Sriram Dash > --- > Changes in v3: > - No update > > Changes in v2: > - integrate dwc3 driver changes together > > drivers/usb/dwc3/core.c | 28 +++- > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/ep0.c| 8 > drivers/usb/dwc3/gadget.c | 37 +++-- > drivers/usb/dwc3/host.c | 12 > 5 files changed, 43 insertions(+), 43 deletions(-) > Tested on my dwc3 platform, it can work well as host or gadget. You can add my tested tag for patch 1/4/5 after fixing Felipe's issue. Tested-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: [PATCH v12 3/9] usbip: exporting devices: new connect operation
On Thu, Oct 13, 2016 at 12:52:07PM +0900, Nobuo Iwata wrote: > New connect operation. Again, we need more text. -- 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 v12 6/9] usbip: exporting devices: modifications to attach and detach
On Thu, Oct 13, 2016 at 12:52:10PM +0900, Nobuo Iwata wrote: > Refactoring to attach and detatch operation. Common parts to new > application(vhci)-side daemon are moved to libsrc/vhci_driver.c. why do this refactoring? please say so why here. 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: [PATCH v12 5/9] usbip: exporting devices: modifications to daemon
On Thu, Oct 13, 2016 at 12:52:09PM +0900, Nobuo Iwata wrote: > Refactoring to the daemon. why? -- 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 v12 8/9] usbip: exporting devices: change to usbip_list.c
On Thu, Oct 13, 2016 at 12:52:12PM +0900, Nobuo Iwata wrote: > Correction to wording inconsistency around import and export in > usbip_list.c. > > Please, see also cover letter about wording. there is no cover letter when the commits are merged, so please put it here as well. 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: [PATCH v2 3/5] usb: dwc3: gadget: Write the event count in interrupt
Hi, Janusz Dziedzicwrites: >> John Youn writes: + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count); + >> After that evt->buf[lpos, lpos + count] seems goes back to HW, so >> thread should not rely on this? >> >> Or I miss something? > > Hi, > > Yes, you're right. That's a possibility and to be safe we can cache > those. > > We're relying on the same mechanism that keeps the event buffer from > becoming full. Since it is just as (un)likely a possibility. That's > why you must size the event buffer appropriately taking into account > your system's latency, etc. > > And if the event buffer becomes full, that indicates something really > wrong happened in the controller. You shouldn't be getting 100's of > events at a time. > > But yes, we can address the overwriting issue. > > Either: > > 1) Cache all incoming events. Requires double the event buffer space. > > 2) Cache just one event and write back only '4' during hard >interrupt. Then in threaded interrupt read the one event from >cache, and process the remaining events from buffer as before. >Doesn't require a large cache, but a bit messier. > > Any other thoughts or ideas? do you really need to clear at least one event for this? >>> >>> Unfortunately yes. That's how the workaround works. We need to write >>> this during IMOD to de-assert the interrupt since the mask bit doesn't >>> work. >> >> okay. Then we should cache and clear a single event. >> > Cache all incoming events looks better for me, while we don't need > care of Vendor Test Event (12Bytes) here - and we will always handle That doesn't matter. If we write 4 to GEVNTCOUNT(0), then only 4 bytes will be cleared. We can still read the following 8 bytes of data from the event buffer itself. > this correctly and can add simple implementation for that in bottom > half. we can still handle this all correctly :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
Hi On 8 November 2016 at 04:36, NeilBrownwrote: > On Mon, Nov 07 2016, Baolin Wang wrote: > >> On 3 November 2016 at 09:25, NeilBrown wrote: >>> On Tue, Nov 01 2016, Baolin Wang wrote: >> >> I agree with your most opinions, but these are optimization. > > I see them as bug fixes, not optimizations. > >> Firstly I >> think we should upstream the USB charger driver. > > I think you missed the point. The point is that we don't *need* your > "USB charger driver" because all the infrastructure we need is *already* > present in the kernel. It is buggy and not used uniformly, and could > usefully be polished and improved. But the structure is already > present. > > If everyone just added new infrastructure when they didn't like, or > didn't understand, what was already present, the kernel would become > like the Mad Hatter's tea party, full of dirty dishes. > >> What I want to ask is >> how can we notify power driver if we don't set the >> usb_register_notifier(), then I think you give the answer is: power >> driver can register by 'struct usb_phy->notifier'. But why usb phy >> should notify the power driver how much current should be drawn, and I >> still think we should notify the current in usb charger driver which >> is better, and do not need to notify current for power driver in usb >> phy driver. > > I accept that it isn't clear that the phy *should* be involved in > communicating the negotiated power availability, but nor is it clear > that it shouldn't. The power does travel through the physical > interface, so physically it plays a role. > > But more importantly, it already *does* get told (in some cases). > There is an interface "usb_phy_set_power()" which exists explicitly to > tell the phy what power has been negotiated. Given that infrastructure > exists and works, it make sense to use it. > > If you think it is a broken design and should be removed, then fine: > make a case for that. Examine the history. Make sure you know why it > is there (or make sure that information cannot be found), and then > present a case as to why it should be removed and replaced with > something else. But don't just leave it there and pretend it doesn't > exist and create something similar-but-different and hope people will > know why yours is better. That way lies madness. Like Peter said, it is not only PHY can detect the USB charger type, which means there are other places can detect the charger type. Second, some controller need to detect the charger type manually which USB phy did not support. Third, it did not handle what current should be drawn in USB phy. Fourth, we need integrate all charger plugin/out event in one framework, not from extcon, maybe type-c in future. In a word, we need one standard integration of this feature we need, though like you said we should do some clean up or fix to make it better. -- 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 v3 5/6] usb: dwc3: use bus->sysdev for DMA configuration
Hi, Sriram Dashwrites: > From: Arnd Bergmann > > The dma ops for dwc3 devices are not set properly. So, use a > physical device sysdev, which will be inherited from parent, > to set the hardware / firmware parameters like dma. > > Signed-off-by: Arnd Bergmann > Signed-off-by: Sriram Dash > --- > Changes in v3: > - No update > > Changes in v2: > - integrate dwc3 driver changes together > > drivers/usb/dwc3/core.c | 28 +++- > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/ep0.c| 8 > drivers/usb/dwc3/gadget.c | 37 +++-- > drivers/usb/dwc3/host.c | 12 > 5 files changed, 43 insertions(+), 43 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 7287a76..0af0dc0 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include I'd prefer to add a device property instead of checking for PCI bus type. > @@ -943,6 +944,13 @@ static int dwc3_probe(struct platform_device *pdev) > dwc = PTR_ALIGN(mem, DWC3_ALIGN_MASK + 1); > dwc->mem = mem; > dwc->dev = dev; > +#ifdef CONFIG_PCI > + /* TODO: or some other way of detecting this? */ > + if (dwc->dev->parent && dwc->dev->parent->bus == _bus_type) > + dwc->sysdev = dwc->dev->parent; > + else > +#endif IOW: dwc->sysdev_is_parent = device_property_read_bool(dev, "linux,sysdev_is_parent"); [...] if (dwc->sysdev_is_parent) dwc->sysdev = dwc->dev->parent; else dwc->sysdev = dwc->dev; > @@ -112,9 +108,9 @@ int dwc3_host_init(struct dwc3 *dwc) > return 0; > err2: > phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy", > - dev_name(>dev)); > + dev_name(dwc->dev)); > phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy", > - dev_name(>dev)); > + dev_name(dwc->dev)); > err1: > platform_device_put(xhci); > return ret; > @@ -123,8 +119,8 @@ int dwc3_host_init(struct dwc3 *dwc) > void dwc3_host_exit(struct dwc3 *dwc) > { > phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy", > - dev_name(>xhci->dev)); > + dev_name(dwc->dev)); > phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy", > - dev_name(>xhci->dev)); > + dev_name(dwc->dev)); this looks unrelated to $subject. Care to explain? -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable
Hi, John Younwrites: > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 2322863..b9903c6 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -539,7 +539,6 @@ struct dwc3_ep { > > struct dwc3_trb *trb_pool; > dma_addr_t trb_pool_dma; > - const struct usb_ss_ep_comp_descriptor *comp_desc; > struct dwc3 *dwc; > > u32 saved_state; > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 7e465ea..0e73383 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -576,13 +576,13 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 > *dwc, struct dwc3_ep *dep) > * Caller should take care of locking > */ > static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, > - const struct usb_endpoint_descriptor *desc, > - const struct usb_ss_ep_comp_descriptor *comp_desc, > bool modify, bool restore) > { > struct dwc3 *dwc = dep->dwc; > u32 reg; > int ret; > + struct usb_ep *ep; > + const struct usb_endpoint_descriptor *desc; > > if (!(dep->flags & DWC3_EP_ENABLED)) { > ret = dwc3_gadget_start_config(dwc, dep); > @@ -590,8 +590,10 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, > return ret; > } > > - ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify, > - restore); > + ep = >endpoint; > + desc = ep->desc; > + ret = dwc3_gadget_set_ep_config(dwc, dep, desc, ep->comp_desc, > + modify, restore); > if (ret) > return ret; this can be improved (see new version below). > @@ -713,11 +713,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) > dwc3_writel(dwc->regs, DWC3_DALEPENA, reg); > > dep->stream_capable = false; > - dep->endpoint.desc = NULL; > - dep->comp_desc = NULL; > dep->type = 0; > dep->flags &= DWC3_EP_END_TRANSFER_PENDING; > > + /* Clear out the ep descriptors for non-ep0 */ > + if (dep->number >> 1) { Do you mean dep->number > 1 ? > @@ -1891,6 +1893,12 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 > *dwc, > (epnum & 1) ? "in" : "out"); > > dep->endpoint.name = dep->name; > + > + if (!(dep->number >> 1)) { ditto new version follows: 8< From 3df1dffa98f1c40f3a67bb621317e43f3c29820f Mon Sep 17 00:00:00 2001 From: John Youn Date: Wed, 9 Nov 2016 16:36:28 -0800 Subject: [PATCH v2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable The __dwc3_gadget_endpoint_enable() function has access to the endpoint descriptors via the usb_ep. So we don't need to pass them in as arguments. The descriptors should be set by the caller prior to calling usb_ep_enable(). Signed-off-by: John Youn [felipe.ba...@linux.intel.com : minor improvements] Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/core.h | 1 - drivers/usb/dwc3/gadget.c | 44 +--- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 2322863e3cf7..b9903c6c3de4 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -539,7 +539,6 @@ struct dwc3_ep { struct dwc3_trb *trb_pool; dma_addr_t trb_pool_dma; - const struct usb_ss_ep_comp_descriptor *comp_desc; struct dwc3 *dwc; u32 saved_state; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 7e465ea6a211..22ccc346af2f 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -488,16 +488,19 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc, struct dwc3_ep *dep) } static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep, - const struct usb_endpoint_descriptor *desc, - const struct usb_ss_ep_comp_descriptor *comp_desc, bool modify, bool restore) { + const struct usb_ss_ep_comp_descriptor *comp_desc; + const struct usb_endpoint_descriptor *desc; struct dwc3_gadget_ep_cmd_params params; if (dev_WARN_ONCE(dwc->dev, modify && restore, "Can't modify and restore\n")) return -EINVAL; + comp_desc = dep->endpoint.comp_desc; + desc = dep->endpoint.desc; + memset(, 0x00, sizeof(params)); params.param0 = DWC3_DEPCFG_EP_TYPE(usb_endpoint_type(desc)) @@ -576,11 +579,11 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct
Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
On Thu, 2016-11-10 at 12:09 +0100, Bjørn Mork wrote: > Kai-Heng Fengwrites: > > On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork wrote: > >> Oliver Neukum writes: > >> > >>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote: > >>> > These problems could very well be caused by running at SuperSpeed > (USB-3) instead of high speed (USB-2). > > > > Yes, it's running at SuperSpeed, on a Kabylake laptop. > > > > It does not have this issue on a Broadwell laptop, also running at > > SuperSpeed. > > Then I must join Oliver, being very surprised by where in the stack you > attempt to fix the issue. What you write above indicates a problem in > pci bridge or usb host controller, doesn't it? Indeed. And this means we need an XHCI specialist. Mathias, we have a failure specific to one implementation of XHCI. 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 v3 00/10] Add DT support for ohci-da8xx
On Tue, Nov 08, 2016 at 05:37:41PM +0100, Axel Haslam wrote: > Hi, > > On Mon, Nov 7, 2016 at 9:39 PM, Axel Haslamwrote: > > The purpose of this patch series is to add DT support for the davinci > > ohci driver. > > > > To make it easier to review. I will split the arch/arm and driver > patches into separate series. I don't think it's easier, as now I have no idea what order, or what tree it should go through :( I'm guessing not mine, so all are now deleted from my patch queue... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
On Thu, 10 Nov 2016, Kai-Heng Feng wrote: > Hi, > > On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Morkwrote: > > Oliver Neukum writes: > > > >> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote: > >> > >>> These problems could very well be caused by running at SuperSpeed > >>> (USB-3) instead of high speed (USB-2). > > Yes, it's running at SuperSpeed, on a Kabylake laptop. > > It does not have this issue on a Broadwell laptop, also running at SuperSpeed. > > >>> > >>> Is there any way to test what happens when the device is attached to > >>> the computer by a USB-2 cable? That would prevent it from operating at > >>> SuperSpeed. > > I recall old Intel PCH can change the USB host from XHCI to EHCI, > newer PCH does not have this option. > > Is there a way to force XHCI run at HighSpeed? Yes, like I said above: Use a USB-2 cable instead of a USB-3 cable. 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 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
On Wed, Nov 09, 2016 at 10:54:38AM -0700, Tony Lindgren wrote: > * Johan Hovold[161109 08:40]: > > On Tue, Nov 08, 2016 at 06:26:07PM -0700, Tony Lindgren wrote: > > > * Johan Hovold [161108 12:03]: > > > > On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote: > > > > > * Johan Hovold [161108 10:09]: > > > > > > In fact, the dsps timer must also be cancelled on suspend, or you > > > > > > could > > > > > > end up calling dsps_check_status() while suspended (it is currently > > > > > > not > > > > > > cancelled until the parent device is suspended, which could be too > > > > > > late). > > > > > > > > > > And then this should no longer be an issue either. > > > > > > > > It would still be an issue as a system-suspending device could already > > > > have been runtime-resumed so that dsps_check_status() would be called > > > > directly from the timer function. > > > > > > The glue layers should do pm_runtime_get_sync(musb->controller) which > > > dsps glue already does. So that's the musb_core.c device instance. And > > > looks like we have dsps_suspend() call del_timer_sync(>timer) > > > already. I think we're safe here. > > > > But the point is that the controller might be RPM_ACTIVE if the > > controller was already runtime resumed when it is system suspended. > > > > Since this (and the previous) patch run the work directly from the timer > > callback if active, it could end up accessing the controller after it > > has been system suspended. Specifically, stopping the timer in the glue > > (parent) suspend callback is too late to avoid this. > > > > pm_runtime_get_sync(musb->controller); > > musb_runtime_resume() > > musb_restore_context(); > > > > ... > > > > musb_suspend() > > musb_save_context(); > > > > otg_timer() > > pm_runtime_get(); > > if (pm_runtime_active(musb->controller)) > > dsps_check_status(); > > pm_runtime_put_autosuspend(); > > > > dsps_suspend() > > del_timer_sync(); > > > > OK so we need to return without doing anything from otg_timer() on > pm_runtime_get() to avoid that. I'm afraid that won't work as pm_runtime_get() would still succeed (i.e. even after musb_suspend()). See 6f3c77b040fc ("PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, even when disabled, v2"). > In the long run it would be nice to make whatever optional state polling > musb generic with just a glue layer callback. Yes, and make sure to stop polling in musb_suspend(). Would it be possible to use the enable and disable ops for this until then? > > > +/* > > > + * Called to run work if device is active or else queue the work to > > > happen > > > + * on resume. Caller must take musb->lock. > > > > Caller must also hold an RPM reference. > > Good point, will add. > > > > + if (musb->is_runtime_suspended) { > > > + list_add_tail(>node, >pending_list); > > > + error = 0; > > > + } else { > > > + dev_err(musb->controller, "could not add resume work %p\n", > > > + callback); > > > + devm_kfree(musb->controller, w); > > > + error = -EINPROGRESS; > > > > But this means you should be able to run the callback below, right? It > > has to be run from somewhere so otherwise the caller needs to retry > > instead. > > Well there's no longer need to run the callback at that point any longer > and with that removed that should not be an issue. > > Anyways, musb->is_runtime_suspended is needed to protect anything from > being queued between runtime resume having called musb_run_resume_work() > and before pm_runtime_active() is true. At that point the caller could > just wait for pm_runtime_active() to be set and run the code. But based > on my tests that does not happen and queueing is faster than getting to > the pm_runtime_active() state so we just print errors in those cases if > we ever hit it later on. But for a generic solution, this race could still be an issue. What about musb_gadget_queue() for example? Would not failing to start I/O if racing with resume be a problem? > Sounds like the rest of your comments are no longer an issue, please > let me know if that's not the case. I think the barrier comment for the WARN_ON on musb_runtime_suspend() still applies. > 8< --- > From tony Mon Sep 17 00:00:00 2001 > From: Tony Lindgren > Date: Wed, 2 Nov 2016 19:59:05 -0700 > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > context for hdrc glue > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > that runs in softirq context. That causes a "BUG: sleeping function called > from invalid context" every time when polling the cable status: > > [] (__might_sleep) from []
Re: Multiple chatty devices on Intel 5 Series/3400 USB2 EHCI controller act erratic
On Wed, 9 Nov 2016 sonofa...@openmailbox.org wrote: > Back to our subject, I think there is something we can do for intel dual > EHCI chipsets so that it is easier to debug similar issues in the > future. PCI dumps from the machine that had the issue are needed. I don't see how PCI dumps would help with debugging this. I also don't see why this issue would be at all connected with the fact that there are two EHCI controllers on the chipset. A usbmon trace of the affected bus might be more helpful. 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 0/4] musb fixes for v4.9-rc cycle
* Laurent Pinchart[161110 05:46]: > Hi Tony, > > Thank you for the patches. > > On Monday 07 Nov 2016 14:50:16 Tony Lindgren wrote: > > Hi all, > > > > Here are musb fixes for the issues that I've been able to track down. > > Not sure if these will help with the problem Ladis was seeing as I'm > > not able to reproduce that one it seems. > > > > As many people depend on this driver I'd like to have these merged > > for v4.9-rc cycle after review and testing. > > > > Please review and test. You need to use v4.9-rc3 or later for testing > > because of the earlier fixes. > > The series fixes my problems, both with the original and latest version of > patch 2/4. > > Tested-by: Laurent Pinchart OK good to hear and thanks for testing. > I have however seen the following warning once with the original version of > 2/4. > > [3.094116] usb 1-1: New USB device found, idVendor=0424, idProduct=9514 > [3.101257] usb 1-1: New USB device strings: Mfr=0, Product=0, > SerialNumber=0 > [3.110626] [ cut here ] > [3.110717] WARNING: CPU: 0 PID: 4 at > /home/laurent/src/kernel/omap4/linux-2.6/drivers/bus/omap_l3_noc.c:147 > l3_interrupt_handler+0x220/0x348 > [3.110717] 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4CFG (Read): > Data Access in User mode during Functional access I've seen similar with interconnect target was not ready in time on ti81xx when booting that got fixed with commit ebf244148092 ("ARM: OMAP2+: Use srst_udelay for USB on dm814x"). It is possible that omap4 needs similar fix but hard to say unless we have it reproducable. Anyways, below is an untested patch for omap4 to play with. Maybe repeated reboot or modprobe test would make it reproducable? Regards, Tony 8< -- diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -2982,6 +2982,7 @@ static struct omap_hwmod_class_sysconfig omap44xx_usb_otg_hs_sysc = { .rev_offs = 0x0400, .sysc_offs = 0x0404, .syss_offs = 0x0408, + .srst_udelay= 2, .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_ENAWAKEUP | SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS), -- 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 v3 00/10] Add DT support for ohci-da8xx
On Thu, Nov 10, 2016 at 1:02 PM, Greg KHwrote: > On Tue, Nov 08, 2016 at 05:37:41PM +0100, Axel Haslam wrote: >> Hi, >> >> On Mon, Nov 7, 2016 at 9:39 PM, Axel Haslam wrote: >> > The purpose of this patch series is to add DT support for the davinci >> > ohci driver. >> > >> >> To make it easier to review. I will split the arch/arm and driver >> patches into separate series. > > I don't think it's easier, as now I have no idea what order, or what > tree it should go through :( Hi Greg, i will resend the series one by one and wait for each to be merged before sending the next, so that there is no confusion on the ordering or on what tree they should apply. Regards Axel. > > I'm guessing not mine, so all are now deleted from my patch queue... > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: Remove unused MUSB initialization code
* Laurent Pinchart[161110 10:56]: > Hi Tony, > > On Thursday 10 Nov 2016 10:54:02 Tony Lindgren wrote: > > * Laurent Pinchart [161110 10:43]: > > > With the removal of board-ldp.c and board-rx51.c, the last users of the > > > MUSB initialization board code are gone. Remove it. > > > > Thanks, I have the same patch already from 3 years ago in omap-for- > > v3.14/omap3-board-removal branch that I'll use. > > v3.14 ? That feels like cheating :-) :p > > Still need to rebase and check few other patches there before reposting them > > all. > > Will you get that one in v4.10 ? Yeah planning to assuming no problems and when out of the eternal musb regressions land. Regards, Tony -- 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/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
* Tony Lindgren[161110 10:50]: > * Johan Hovold [161110 09:04]: > > Yes, and make sure to stop polling in musb_suspend(). Would it be > > possible to use the enable and disable ops for this until then? > > Hmm care to explain a bit more? That is assuming that rpm_resume() > won't fail above.. And that using musb->is_disabled flag won't > work.. Oh you're talking about musb_enable/disable ops, not the pm_runtime/enable/disable ops :) Yeah that might work for disabling the dsps specific timer. Regards, Tony -- 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] usbnet: prevent device rpm suspend in usbnet_probe function
Kai-Heng Fengwrites: > On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork wrote: >> Oliver Neukum writes: >> >>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote: >>> These problems could very well be caused by running at SuperSpeed (USB-3) instead of high speed (USB-2). > > Yes, it's running at SuperSpeed, on a Kabylake laptop. > > It does not have this issue on a Broadwell laptop, also running at SuperSpeed. Then I must join Oliver, being very surprised by where in the stack you attempt to fix the issue. What you write above indicates a problem in pci bridge or usb host controller, doesn't it? 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 1/2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable
On 11/10/2016 3:15 AM, Felipe Balbi wrote: > > Hi, > > John Younwrites: >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index 2322863..b9903c6 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -539,7 +539,6 @@ struct dwc3_ep { >> >> struct dwc3_trb *trb_pool; >> dma_addr_t trb_pool_dma; >> -const struct usb_ss_ep_comp_descriptor *comp_desc; >> struct dwc3 *dwc; >> >> u32 saved_state; >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 7e465ea..0e73383 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -576,13 +576,13 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 >> *dwc, struct dwc3_ep *dep) >> * Caller should take care of locking >> */ >> static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, >> -const struct usb_endpoint_descriptor *desc, >> -const struct usb_ss_ep_comp_descriptor *comp_desc, >> bool modify, bool restore) >> { >> struct dwc3 *dwc = dep->dwc; >> u32 reg; >> int ret; >> +struct usb_ep *ep; >> +const struct usb_endpoint_descriptor *desc; >> >> if (!(dep->flags & DWC3_EP_ENABLED)) { >> ret = dwc3_gadget_start_config(dwc, dep); >> @@ -590,8 +590,10 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, >> return ret; >> } >> >> -ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify, >> -restore); >> +ep = >endpoint; >> +desc = ep->desc; >> +ret = dwc3_gadget_set_ep_config(dwc, dep, desc, ep->comp_desc, >> +modify, restore); >> if (ret) >> return ret; > > this can be improved (see new version below). > >> @@ -713,11 +713,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep >> *dep) >> dwc3_writel(dwc->regs, DWC3_DALEPENA, reg); >> >> dep->stream_capable = false; >> -dep->endpoint.desc = NULL; >> -dep->comp_desc = NULL; >> dep->type = 0; >> dep->flags &= DWC3_EP_END_TRANSFER_PENDING; >> >> +/* Clear out the ep descriptors for non-ep0 */ >> +if (dep->number >> 1) { > > Do you mean dep->number > 1 ? I did mean shift since bit 0 is the direction. But that also works. 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 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
On Thu, Nov 10, 2016 at 10:41:50AM -0700, Tony Lindgren wrote: > * Johan Hovold[161110 09:04]: > > On Wed, Nov 09, 2016 at 10:54:38AM -0700, Tony Lindgren wrote: > > > * Johan Hovold [161109 08:40]: > > > > On Tue, Nov 08, 2016 at 06:26:07PM -0700, Tony Lindgren wrote: > > > > > * Johan Hovold [161108 12:03]: > > > > > > On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote: > > > > > > > * Johan Hovold [161108 10:09]: > > > > > > > > > > In fact, the dsps timer must also be cancelled on suspend, or > > > > > > > > you could > > > > > > > > end up calling dsps_check_status() while suspended (it is > > > > > > > > currently not > > > > > > > > cancelled until the parent device is suspended, which could be > > > > > > > > too > > > > > > > > late). > > > > > > > > > > > > > > And then this should no longer be an issue either. > > > > > > > > > > > > It would still be an issue as a system-suspending device could > > > > > > already > > > > > > have been runtime-resumed so that dsps_check_status() would be > > > > > > called > > > > > > directly from the timer function. > > > > > > > > > > The glue layers should do pm_runtime_get_sync(musb->controller) which > > > > > dsps glue already does. So that's the musb_core.c device instance. And > > > > > looks like we have dsps_suspend() call del_timer_sync(>timer) > > > > > already. I think we're safe here. > > > > > > > > But the point is that the controller might be RPM_ACTIVE if the > > > > controller was already runtime resumed when it is system suspended. > > > > > > > > Since this (and the previous) patch run the work directly from the timer > > > > callback if active, it could end up accessing the controller after it > > > > has been system suspended. Specifically, stopping the timer in the glue > > > > (parent) suspend callback is too late to avoid this. > > > > > > > > pm_runtime_get_sync(musb->controller); > > > > musb_runtime_resume() > > > > musb_restore_context(); > > > > > > > > ... > > > > > > > > musb_suspend() > > > > musb_save_context(); > > > > > > > > otg_timer() > > > > pm_runtime_get(); > > > > if (pm_runtime_active(musb->controller)) > > > > dsps_check_status(); > > > > pm_runtime_put_autosuspend(); > > > > > > > > dsps_suspend() > > > > del_timer_sync(); > > > > > > > > > > OK so we need to return without doing anything from otg_timer() on > > > pm_runtime_get() to avoid that. > > > > I'm afraid that won't work as pm_runtime_get() would still succeed (i.e. > > even after musb_suspend()). > > > > See 6f3c77b040fc ("PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, > > even when disabled, v2"). > > But doesn't that assume that we have musb core as in musb->controller > in RPM_ACTIVE state? While if it's been suspended that's not the > case meaning rpm_resume would fail? Right, and it's still a good idea to check the return value of pm_runtime_get(). It just won't be enough for when RPM_ACTIVE. > If we have a window for a race there with RPM_ACTIVE set, we could > add musb->is_disabled flag that gets set in musb_suspend(). Yes. > > > In the long run it would be nice to make whatever optional state polling > > > musb generic with just a glue layer callback. > > > > Yes, and make sure to stop polling in musb_suspend(). Would it be > > possible to use the enable and disable ops for this until then? > > Hmm care to explain a bit more? That is assuming that rpm_resume() > won't fail above.. And that using musb->is_disabled flag won't > work.. By stopping the timer in musb->ops->disable which is called during suspend, the race could perhaps also be avoided. > > > > > + if (musb->is_runtime_suspended) { > > > > > + list_add_tail(>node, >pending_list); > > > > > + error = 0; > > > > > + } else { > > > > > + dev_err(musb->controller, "could not add resume work > > > > > %p\n", > > > > > + callback); > > > > > + devm_kfree(musb->controller, w); > > > > > + error = -EINPROGRESS; > > > > > > > > But this means you should be able to run the callback below, right? It > > > > has to be run from somewhere so otherwise the caller needs to retry > > > > instead. > > > > > > Well there's no longer need to run the callback at that point any longer > > > and with that removed that should not be an issue. > > > > > > Anyways, musb->is_runtime_suspended is needed to protect anything from > > > being queued between runtime resume having called musb_run_resume_work() > > > and before pm_runtime_active() is true. At that point the caller could > > > just wait for pm_runtime_active() to be set and run the code. But based > > > on my tests that does not happen and queueing is
Re: [PATCH 0/4] musb fixes for v4.9-rc cycle
Hi Tony, On Thursday 10 Nov 2016 08:01:52 Tony Lindgren wrote: > * Laurent Pinchart[161110 05:46]: > > On Monday 07 Nov 2016 14:50:16 Tony Lindgren wrote: > > > Hi all, > > > > > > Here are musb fixes for the issues that I've been able to track down. > > > Not sure if these will help with the problem Ladis was seeing as I'm > > > not able to reproduce that one it seems. > > > > > > As many people depend on this driver I'd like to have these merged > > > for v4.9-rc cycle after review and testing. > > > > > > Please review and test. You need to use v4.9-rc3 or later for testing > > > because of the earlier fixes. > > > > The series fixes my problems, both with the original and latest version of > > patch 2/4. > > > > Tested-by: Laurent Pinchart > > OK good to hear and thanks for testing. > > > I have however seen the following warning once with the original version > > of 2/4. > > > > [3.094116] usb 1-1: New USB device found, idVendor=0424, > > idProduct=9514 > > [3.101257] usb 1-1: New USB device strings: Mfr=0, Product=0, > > SerialNumber=0 [3.110626] [ cut here ] > > [3.110717] WARNING: CPU: 0 PID: 4 at > > /home/laurent/src/kernel/omap4/linux-2.6/drivers/bus/omap_l3_noc.c:147 > > l3_interrupt_handler+0x220/0x348 [3.110717] 4400.ocp:L3 Custom > > Error: MASTER MPU TARGET L4CFG (Read): Data Access in User mode during > > Functional access > I've seen similar with interconnect target was not ready in time > on ti81xx when booting that got fixed with commit ebf244148092 > ("ARM: OMAP2+: Use srst_udelay for USB on dm814x"). It is possible > that omap4 needs similar fix but hard to say unless we have it > reproducable. Anyways, below is an untested patch for omap4 to play > with. Maybe repeated reboot or modprobe test would make it > reproducable? I'll try to give it a go, but right now I'm stuck with a different MUSB problem :-) I'm trying to boot my panda board over the SMSC95xx ethernet, and have switched gadget support from being compiled in to being compiled as a module. I then get [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while active printed in a loop at boot time. I've traced musb->is_active being set to 1 in musb_start() with musb->port_mode = MUSB_PORT_MODE_DUAL_ROLE musb->xceiv->otg->state = OTG_STATE_A_IDLE devctl = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_VBUS I can disable gadget support completely which will I believe fix the problem, but that's just a workaround. Help would be appreciated. > 8< -- > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c --- > a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > @@ -2982,6 +2982,7 @@ static struct omap_hwmod_class_sysconfig > omap44xx_usb_otg_hs_sysc = { .rev_offs= 0x0400, > .sysc_offs = 0x0404, > .syss_offs = 0x0408, > + .srst_udelay= 2, > .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_ENAWAKEUP | > SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE | > SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS), -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] musb fixes for v4.9-rc cycle
On Thursday 10 Nov 2016 19:18:23 Laurent Pinchart wrote: > On Thursday 10 Nov 2016 08:01:52 Tony Lindgren wrote: > > * Laurent Pinchart[161110 05:46]: > >> On Monday 07 Nov 2016 14:50:16 Tony Lindgren wrote: > >>> Hi all, > >>> > >>> Here are musb fixes for the issues that I've been able to track down. > >>> Not sure if these will help with the problem Ladis was seeing as I'm > >>> not able to reproduce that one it seems. > >>> > >>> As many people depend on this driver I'd like to have these merged > >>> for v4.9-rc cycle after review and testing. > >>> > >>> Please review and test. You need to use v4.9-rc3 or later for testing > >>> because of the earlier fixes. > >> > >> The series fixes my problems, both with the original and latest version > >> of > >> patch 2/4. > >> > >> Tested-by: Laurent Pinchart > > > > OK good to hear and thanks for testing. > > > > I have however seen the following warning once with the original version > >> of 2/4. > >> > >> [3.094116] usb 1-1: New USB device found, idVendor=0424, > >> idProduct=9514 > >> [3.101257] usb 1-1: New USB device strings: Mfr=0, Product=0, > >> SerialNumber=0 [3.110626] [ cut here ] > >> [3.110717] WARNING: CPU: 0 PID: 4 at > >> /home/laurent/src/kernel/omap4/linux-2.6/drivers/bus/omap_l3_noc.c:147 > >> l3_interrupt_handler+0x220/0x348 [3.110717] 4400.ocp:L3 Custom > >> Error: MASTER MPU TARGET L4CFG (Read): Data Access in User mode during > >> Functional access > > > > I've seen similar with interconnect target was not ready in time > > on ti81xx when booting that got fixed with commit ebf244148092 > > ("ARM: OMAP2+: Use srst_udelay for USB on dm814x"). It is possible > > that omap4 needs similar fix but hard to say unless we have it > > reproducable. Anyways, below is an untested patch for omap4 to play > > with. Maybe repeated reboot or modprobe test would make it > > reproducable? > > I'll try to give it a go, but right now I'm stuck with a different MUSB > problem :-) I'm trying to boot my panda board over the SMSC95xx ethernet, > and have switched gadget support from being compiled in to being compiled > as a module. I then get > > [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while > active > > printed in a loop at boot time. I've traced musb->is_active being set to 1 > in musb_start() with > > musb->port_mode = MUSB_PORT_MODE_DUAL_ROLE > musb->xceiv->otg->state = OTG_STATE_A_IDLE > devctl = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_VBUS > > I can disable gadget support completely which will I believe fix the > problem, but that's just a workaround. Help would be appreciated. And on a side note, selecting CONFIG_USB_GADGET=m makes it impossible to build CONFIG_USB_NOT_XCEIV=y, making OMAP3 EHCI support only available as a module. *sigh* :-) Felipe, git blame points at you, can this be fixed ? > > 8< -- > > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > > b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c --- > > a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > > @@ -2982,6 +2982,7 @@ static struct omap_hwmod_class_sysconfig > > omap44xx_usb_otg_hs_sysc = { .rev_offs = 0x0400, > > > > .sysc_offs = 0x0404, > > .syss_offs = 0x0408, > > > > + .srst_udelay= 2, > > > > .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_ENAWAKEUP | > > > >SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE | > >SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS), -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP2+: Remove unused MUSB initialization code
With the removal of board-ldp.c and board-rx51.c, the last users of the MUSB initialization board code are gone. Remove it. Signed-off-by: Laurent Pinchart--- arch/arm/mach-omap2/Makefile | 1 - arch/arm/mach-omap2/usb-musb.c | 106 - arch/arm/mach-omap2/usb.h | 1 - 3 files changed, 108 deletions(-) delete mode 100644 arch/arm/mach-omap2/usb-musb.c diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index 5b37ec29996e..76e8ba70d952 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -242,7 +242,6 @@ obj-y += $(omap-flash-y) $(omap-flash-m) omap-hsmmc-$(CONFIG_MMC_OMAP_HS) := hsmmc.o obj-y += $(omap-hsmmc-m) $(omap-hsmmc-y) -obj-y += usb-musb.o obj-y += omap_phy_internal.o obj-$(CONFIG_MACH_OMAP2_TUSB6010) += usb-tusb6010.o diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c deleted file mode 100644 index e4562b2b973b.. --- a/arch/arm/mach-omap2/usb-musb.c +++ /dev/null @@ -1,106 +0,0 @@ -/* - * linux/arch/arm/mach-omap2/usb-musb.c - * - * This file will contain the board specific details for the - * MENTOR USB OTG controller on OMAP3430 - * - * Copyright (C) 2007-2008 Texas Instruments - * Copyright (C) 2008 Nokia Corporation - * Author: Vikram Pandita - * - * Generalization by: - * Felipe Balbi - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "omap_device.h" -#include "soc.h" -#include "mux.h" -#include "usb.h" - -static struct musb_hdrc_config musb_config = { - .multipoint = 1, - .dyn_fifo = 1, - .num_eps= 16, - .ram_bits = 12, -}; - -static struct musb_hdrc_platform_data musb_plat = { - .mode = MUSB_OTG, - - /* .clock is set dynamically */ - .config = _config, - - /* REVISIT charge pump on TWL4030 can supply up to -* 100 mA ... but this value is board-specific, like -* "mode", and should be passed to usb_musb_init(). -*/ - .power = 50, /* up to 100 mA */ -}; - -static u64 musb_dmamask = DMA_BIT_MASK(32); - -static struct omap_musb_board_data musb_default_board_data = { - .interface_type = MUSB_INTERFACE_ULPI, - .mode = MUSB_OTG, - .power = 100, -}; - -void __init usb_musb_init(struct omap_musb_board_data *musb_board_data) -{ - struct omap_hwmod *oh; - struct platform_device *pdev; - struct device *dev; - int bus_id = -1; - const char *oh_name, *name; - struct omap_musb_board_data *board_data; - - if (musb_board_data) - board_data = musb_board_data; - else - board_data = _default_board_data; - - /* -* REVISIT: This line can be removed once all the platforms using -* musb_core.c have been converted to use use clkdev. -*/ - musb_plat.clock = "ick"; - musb_plat.board_data = board_data; - musb_plat.power = board_data->power >> 1; - musb_plat.mode = board_data->mode; - musb_plat.extvbus = board_data->extvbus; - - oh_name = "usb_otg_hs"; - name = "musb-omap2430"; - -oh = omap_hwmod_lookup(oh_name); -if (WARN(!oh, "%s: could not find omap_hwmod for %s\n", - __func__, oh_name)) -return; - - pdev = omap_device_build(name, bus_id, oh, _plat, -sizeof(musb_plat)); - if (IS_ERR(pdev)) { - pr_err("Could not build omap_device for %s %s\n", - name, oh_name); - return; - } - - dev = >dev; - get_device(dev); - dev->dma_mask = _dmamask; - dev->coherent_dma_mask = musb_dmamask; - put_device(dev); -} diff --git a/arch/arm/mach-omap2/usb.h b/arch/arm/mach-omap2/usb.h index 3395365ef1db..1951535646d2 100644 --- a/arch/arm/mach-omap2/usb.h +++ b/arch/arm/mach-omap2/usb.h @@ -60,7 +60,6 @@ struct usbhs_phy_data { bool vcc_polarity; /* 1 active high, 0 active low */ }; -extern void usb_musb_init(struct omap_musb_board_data *board_data); extern void usbhs_init(struct usbhs_omap_platform_data *pdata); extern int usbhs_init_phys(struct usbhs_phy_data *phy, int num_phys); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe
Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
* Johan Hovold[161110 09:04]: > On Wed, Nov 09, 2016 at 10:54:38AM -0700, Tony Lindgren wrote: > > * Johan Hovold [161109 08:40]: > > > On Tue, Nov 08, 2016 at 06:26:07PM -0700, Tony Lindgren wrote: > > > > * Johan Hovold [161108 12:03]: > > > > > On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote: > > > > > > * Johan Hovold [161108 10:09]: > > > > > > > > In fact, the dsps timer must also be cancelled on suspend, or you > > > > > > > could > > > > > > > end up calling dsps_check_status() while suspended (it is > > > > > > > currently not > > > > > > > cancelled until the parent device is suspended, which could be too > > > > > > > late). > > > > > > > > > > > > And then this should no longer be an issue either. > > > > > > > > > > It would still be an issue as a system-suspending device could already > > > > > have been runtime-resumed so that dsps_check_status() would be called > > > > > directly from the timer function. > > > > > > > > The glue layers should do pm_runtime_get_sync(musb->controller) which > > > > dsps glue already does. So that's the musb_core.c device instance. And > > > > looks like we have dsps_suspend() call del_timer_sync(>timer) > > > > already. I think we're safe here. > > > > > > But the point is that the controller might be RPM_ACTIVE if the > > > controller was already runtime resumed when it is system suspended. > > > > > > Since this (and the previous) patch run the work directly from the timer > > > callback if active, it could end up accessing the controller after it > > > has been system suspended. Specifically, stopping the timer in the glue > > > (parent) suspend callback is too late to avoid this. > > > > > > pm_runtime_get_sync(musb->controller); > > > musb_runtime_resume() > > > musb_restore_context(); > > > > > > ... > > > > > > musb_suspend() > > > musb_save_context(); > > > > > > otg_timer() > > > pm_runtime_get(); > > > if (pm_runtime_active(musb->controller)) > > > dsps_check_status(); > > > pm_runtime_put_autosuspend(); > > > > > > dsps_suspend() > > > del_timer_sync(); > > > > > > > OK so we need to return without doing anything from otg_timer() on > > pm_runtime_get() to avoid that. > > I'm afraid that won't work as pm_runtime_get() would still succeed (i.e. > even after musb_suspend()). > > See 6f3c77b040fc ("PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, > even when disabled, v2"). But doesn't that assume that we have musb core as in musb->controller in RPM_ACTIVE state? While if it's been suspended that's not the case meaning rpm_resume would fail? If we have a window for a race there with RPM_ACTIVE set, we could add musb->is_disabled flag that gets set in musb_suspend(). > > In the long run it would be nice to make whatever optional state polling > > musb generic with just a glue layer callback. > > Yes, and make sure to stop polling in musb_suspend(). Would it be > possible to use the enable and disable ops for this until then? Hmm care to explain a bit more? That is assuming that rpm_resume() won't fail above.. And that using musb->is_disabled flag won't work.. > > > > + if (musb->is_runtime_suspended) { > > > > + list_add_tail(>node, >pending_list); > > > > + error = 0; > > > > + } else { > > > > + dev_err(musb->controller, "could not add resume work > > > > %p\n", > > > > + callback); > > > > + devm_kfree(musb->controller, w); > > > > + error = -EINPROGRESS; > > > > > > But this means you should be able to run the callback below, right? It > > > has to be run from somewhere so otherwise the caller needs to retry > > > instead. > > > > Well there's no longer need to run the callback at that point any longer > > and with that removed that should not be an issue. > > > > Anyways, musb->is_runtime_suspended is needed to protect anything from > > being queued between runtime resume having called musb_run_resume_work() > > and before pm_runtime_active() is true. At that point the caller could > > just wait for pm_runtime_active() to be set and run the code. But based > > on my tests that does not happen and queueing is faster than getting to > > the pm_runtime_active() state so we just print errors in those cases if > > we ever hit it later on. > > But for a generic solution, this race could still be an issue. What > about musb_gadget_queue() for example? Would not failing to start I/O > if racing with resume be a problem? It's probably safest to return an error for cases like that rather than attempt to do something and risk corrupting packets. I'll move the check to happen earlier musb_gadget_queue() so we don't do anything with the request if pm_runtime_get() fails there. That should make it easy to add further handling if
Re: [PATCH 0/4] musb fixes for v4.9-rc cycle
On Thursday 10 Nov 2016 19:18:23 Laurent Pinchart wrote: > Hi Tony, > > On Thursday 10 Nov 2016 08:01:52 Tony Lindgren wrote: > > * Laurent Pinchart[161110 05:46]: > > > On Monday 07 Nov 2016 14:50:16 Tony Lindgren wrote: > > > > Hi all, > > > > > > > > Here are musb fixes for the issues that I've been able to track down. > > > > Not sure if these will help with the problem Ladis was seeing as I'm > > > > not able to reproduce that one it seems. > > > > > > > > As many people depend on this driver I'd like to have these merged > > > > for v4.9-rc cycle after review and testing. > > > > > > > > Please review and test. You need to use v4.9-rc3 or later for testing > > > > because of the earlier fixes. > > > > > > The series fixes my problems, both with the original and latest version > > > of > > > patch 2/4. > > > > > > Tested-by: Laurent Pinchart > > > > OK good to hear and thanks for testing. > > > > > I have however seen the following warning once with the original version > > > of 2/4. > > > > > > [3.094116] usb 1-1: New USB device found, idVendor=0424, > > > idProduct=9514 > > > [3.101257] usb 1-1: New USB device strings: Mfr=0, Product=0, > > > SerialNumber=0 [3.110626] [ cut here ] > > > [3.110717] WARNING: CPU: 0 PID: 4 at > > > /home/laurent/src/kernel/omap4/linux-2.6/drivers/bus/omap_l3_noc.c:147 > > > l3_interrupt_handler+0x220/0x348 [3.110717] 4400.ocp:L3 Custom > > > Error: MASTER MPU TARGET L4CFG (Read): Data Access in User mode during > > > Functional access > > > > I've seen similar with interconnect target was not ready in time > > on ti81xx when booting that got fixed with commit ebf244148092 > > ("ARM: OMAP2+: Use srst_udelay for USB on dm814x"). It is possible > > that omap4 needs similar fix but hard to say unless we have it > > reproducable. Anyways, below is an untested patch for omap4 to play > > with. Maybe repeated reboot or modprobe test would make it > > reproducable? > > I'll try to give it a go, but right now I'm stuck with a different MUSB > problem :-) I'm trying to boot my panda board over the SMSC95xx ethernet, > and have switched gadget support from being compiled in to being compiled > as a module. I then get > > [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while > active > > printed in a loop at boot time. I've traced musb->is_active being set to 1 > in musb_start() with > > musb->port_mode = MUSB_PORT_MODE_DUAL_ROLE > musb->xceiv->otg->state = OTG_STATE_A_IDLE > devctl = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_VBUS > > I can disable gadget support completely which will I believe fix the > problem, but that's just a workaround. Help would be appreciated. Turns out it doesn't :-( CONFIG_USB_MUSB_HDRC=y CONFIG_USB_MUSB_HOST=y # CONFIG_USB_GADGET is not set and the problem still occurs. > > 8< -- > > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > > b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c --- > > a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c > > @@ -2982,6 +2982,7 @@ static struct omap_hwmod_class_sysconfig > > omap44xx_usb_otg_hs_sysc = { .rev_offs = 0x0400, > > > > .sysc_offs = 0x0404, > > .syss_offs = 0x0408, > > > > + .srst_udelay= 2, > > > > .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_ENAWAKEUP | > > > >SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE | > >SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS), -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: Remove unused MUSB initialization code
* Laurent Pinchart[161110 10:43]: > With the removal of board-ldp.c and board-rx51.c, the last users of the > MUSB initialization board code are gone. Remove it. Thanks, I have the same patch already from 3 years ago in omap-for-v3.14/omap3-board-removal branch that I'll use. Still need to rebase and check few other patches there before reposting them all. FYI, to avoid duplicate work, the old barnch is at [0]. Regards, Tony https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/log/?h=omap-for-v3.14/omap3-board-removal -- 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] ARM: OMAP2+: Remove unused MUSB initialization code
Hi Tony, On Thursday 10 Nov 2016 10:54:02 Tony Lindgren wrote: > * Laurent Pinchart[161110 10:43]: > > With the removal of board-ldp.c and board-rx51.c, the last users of the > > MUSB initialization board code are gone. Remove it. > > Thanks, I have the same patch already from 3 years ago in omap-for- > v3.14/omap3-board-removal branch that I'll use. v3.14 ? That feels like cheating :-) > Still need to rebase and check few other patches there before reposting them > all. Will you get that one in v4.10 ? > FYI, to avoid duplicate work, the old barnch is at [0]. > > Regards, > > Tony > > https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/log/?h=om > ap-for-v3.14/omap3-board-removal -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
On Thu, Nov 10, 2016 at 11:02:35AM -0700, Tony Lindgren wrote: > * Tony Lindgren[161110 10:50]: > > * Johan Hovold [161110 09:04]: > > > Yes, and make sure to stop polling in musb_suspend(). Would it be > > > possible to use the enable and disable ops for this until then? > > > > Hmm care to explain a bit more? That is assuming that rpm_resume() > > won't fail above.. And that using musb->is_disabled flag won't > > work.. > > Oh you're talking about musb_enable/disable ops, not the > pm_runtime/enable/disable ops :) Indeed, sorry for being unclear. > Yeah that might work for disabling the dsps specific timer. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] musb fixes for v4.9-rc cycle
* Laurent Pinchart[161110 10:18]: > I'll try to give it a go, but right now I'm stuck with a different MUSB > problem :-) I'm trying to boot my panda board over the SMSC95xx ethernet, and > have switched gadget support from being compiled in to being compiled as a > module. I then get > > [2.766174] musb_bus_suspend 2586: trying to suspend as a_idle while active > > printed in a loop at boot time. I've traced musb->is_active being set to 1 in > musb_start() with > > musb->port_mode = MUSB_PORT_MODE_DUAL_ROLE > musb->xceiv->otg->state = OTG_STATE_A_IDLE > devctl = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_VBUS > > I can disable gadget support completely which will I believe fix the problem, > but that's just a workaround. Help would be appreciated. Do you have the ID pin grounded or how do you get to the A_IDLE state? I'm pretty much only testing with loadable modules as in omap2plus_defconfig, I don't think I've seen this issue.. Maybe check the changes in your .config against just plain omap2plus_defconfig? Are you trying to force host only mode? Regards, Tony -- 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 v3 3/6] usb: ehci: fsl: use bus->sysdev for DMA configuration
From: Arnd BergmannFor the dual role ehci fsl driver, sysdev will handle the dma config. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash --- Changes in v3: - fix compile errors Changes in v2: - fix compile warnings drivers/usb/host/ehci-fsl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index 9f5ffb6..4d4ab42 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -96,8 +96,8 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev) } irq = res->start; - hcd = usb_create_hcd(_ehci_hc_driver, >dev, - dev_name(>dev)); + hcd = __usb_create_hcd(_ehci_hc_driver, pdev->dev.parent, + >dev, dev_name(>dev), NULL); if (!hcd) { retval = -ENOMEM; goto err1; -- 2.1.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
[PATCH v3 0/6] inherit dma configuration from parent dev
For xhci-hcd platform device, all the DMA parameters are not configured properly, notably dma ops for dwc3 devices. The idea here is that you pass in the parent of_node along with the child device pointer, so it would behave exactly like the parent already does. The difference is that it also handles all the other attributes besides the mask. Arnd Bergmann (6): usb: separate out sysdev pointer from usb_bus usb: chipidea: use bus->sysdev for DMA configuration usb: ehci: fsl: use bus->sysdev for DMA configuration usb: xhci: use bus->sysdev for DMA configuration usb: dwc3: use bus->sysdev for DMA configuration usb: dwc3: Do not set dma coherent mask drivers/usb/chipidea/core.c| 3 --- drivers/usb/chipidea/host.c| 3 ++- drivers/usb/chipidea/udc.c | 10 + drivers/usb/core/buffer.c | 12 +-- drivers/usb/core/hcd.c | 48 +- drivers/usb/core/usb.c | 18 drivers/usb/dwc3/core.c| 28 drivers/usb/dwc3/core.h| 1 + drivers/usb/dwc3/dwc3-exynos.c | 10 - drivers/usb/dwc3/dwc3-st.c | 1 - drivers/usb/dwc3/ep0.c | 8 +++ drivers/usb/dwc3/gadget.c | 37 drivers/usb/dwc3/host.c| 12 --- drivers/usb/host/ehci-fsl.c| 4 ++-- drivers/usb/host/xhci-mem.c| 12 +-- drivers/usb/host/xhci-plat.c | 33 +++-- drivers/usb/host/xhci.c| 15 + include/linux/usb.h| 1 + include/linux/usb/hcd.h| 3 +++ 19 files changed, 144 insertions(+), 115 deletions(-) -- 2.1.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
[PATCH v3 1/6] usb: separate out sysdev pointer from usb_bus
From: Arnd BergmannFor xhci-hcd platform device, all the DMA parameters are not configured properly, notably dma ops for dwc3 devices. The idea here is that you pass in the parent of_node along with the child device pointer, so it would behave exactly like the parent already does. The difference is that it also handles all the other attributes besides the mask. sysdev will represent the physical device, as seen from firmware or bus.Splitting the usb_bus->controller field into the Linux-internal device (used for the sysfs hierarchy, for printks and for power management) and a new pointer (used for DMA, DT enumeration and phy lookup) probably covers all that we really need. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash Cc: Felipe Balbi Cc: Grygorii Strashko Cc: Sinjan Kumar Cc: David Fisher Cc: Catalin Marinas Cc: "Thang Q. Nguyen" Cc: Yoshihiro Shimoda Cc: Stephen Boyd Cc: Bjorn Andersson Cc: Ming Lei Cc: Jon Masters Cc: Dann Frazier Cc: Peter Chen Cc: Leo Li --- Changes in v3: - usb is_device_dma_capable instead of directly accessing dma props. Changes in v2: - Split the patch wrt driver drivers/usb/core/buffer.c | 12 ++-- drivers/usb/core/hcd.c| 48 --- drivers/usb/core/usb.c| 18 +- include/linux/usb.h | 1 + include/linux/usb/hcd.h | 3 +++ 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c index 98e39f9..a6cd44a 100644 --- a/drivers/usb/core/buffer.c +++ b/drivers/usb/core/buffer.c @@ -63,7 +63,7 @@ int hcd_buffer_create(struct usb_hcd *hcd) int i, size; if (!IS_ENABLED(CONFIG_HAS_DMA) || - (!hcd->self.controller->dma_mask && + (!is_device_dma_capable(hcd->self.sysdev) && !(hcd->driver->flags & HCD_LOCAL_MEM))) return 0; @@ -72,7 +72,7 @@ int hcd_buffer_create(struct usb_hcd *hcd) if (!size) continue; snprintf(name, sizeof(name), "buffer-%d", size); - hcd->pool[i] = dma_pool_create(name, hcd->self.controller, + hcd->pool[i] = dma_pool_create(name, hcd->self.sysdev, size, size, 0); if (!hcd->pool[i]) { hcd_buffer_destroy(hcd); @@ -127,7 +127,7 @@ void *hcd_buffer_alloc( /* some USB hosts just use PIO */ if (!IS_ENABLED(CONFIG_HAS_DMA) || - (!bus->controller->dma_mask && + (!is_device_dma_capable(bus->sysdev) && !(hcd->driver->flags & HCD_LOCAL_MEM))) { *dma = ~(dma_addr_t) 0; return kmalloc(size, mem_flags); @@ -137,7 +137,7 @@ void *hcd_buffer_alloc( if (size <= pool_max[i]) return dma_pool_alloc(hcd->pool[i], mem_flags, dma); } - return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags); + return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags); } void hcd_buffer_free( @@ -154,7 +154,7 @@ void hcd_buffer_free( return; if (!IS_ENABLED(CONFIG_HAS_DMA) || - (!bus->controller->dma_mask && + (!is_device_dma_capable(bus->sysdev) && !(hcd->driver->flags & HCD_LOCAL_MEM))) { kfree(addr); return; @@ -166,5 +166,5 @@ void hcd_buffer_free( return; } } - dma_free_coherent(hcd->self.controller, size, addr, dma); + dma_free_coherent(hcd->self.sysdev, size, addr, dma); } diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 479e223..f8feb08 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1073,6 +1073,7 @@ static void usb_deregister_bus (struct usb_bus *bus) static int register_root_hub(struct usb_hcd *hcd) { struct device *parent_dev = hcd->self.controller; + struct device *sysdev = hcd->self.sysdev; struct usb_device *usb_dev = hcd->self.root_hub; const int devnum = 1; int retval; @@ -1119,7 +1120,7 @@ static int register_root_hub(struct usb_hcd *hcd) /* Did the HC die before the root hub was registered? */ if (HCD_DEAD(hcd)) usb_hc_died (hcd); /* This time clean up */ - usb_dev->dev.of_node = parent_dev->of_node; + usb_dev->dev.of_node = sysdev->of_node; } mutex_unlock(_bus_idr_lock); @@ -1465,19 +1466,19 @@
[PATCH v3 5/6] usb: dwc3: use bus->sysdev for DMA configuration
From: Arnd BergmannThe dma ops for dwc3 devices are not set properly. So, use a physical device sysdev, which will be inherited from parent, to set the hardware / firmware parameters like dma. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash --- Changes in v3: - No update Changes in v2: - integrate dwc3 driver changes together drivers/usb/dwc3/core.c | 28 +++- drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/ep0.c| 8 drivers/usb/dwc3/gadget.c | 37 +++-- drivers/usb/dwc3/host.c | 12 5 files changed, 43 insertions(+), 43 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 7287a76..0af0dc0 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -229,7 +230,7 @@ static void dwc3_frame_length_adjustment(struct dwc3 *dwc) static void dwc3_free_one_event_buffer(struct dwc3 *dwc, struct dwc3_event_buffer *evt) { - dma_free_coherent(dwc->dev, evt->length, evt->buf, evt->dma); + dma_free_coherent(dwc->sysdev, evt->length, evt->buf, evt->dma); } /** @@ -251,7 +252,7 @@ static struct dwc3_event_buffer *dwc3_alloc_one_event_buffer(struct dwc3 *dwc, evt->dwc= dwc; evt->length = length; - evt->buf= dma_alloc_coherent(dwc->dev, length, + evt->buf= dma_alloc_coherent(dwc->sysdev, length, >dma, GFP_KERNEL); if (!evt->buf) return ERR_PTR(-ENOMEM); @@ -370,11 +371,11 @@ static int dwc3_setup_scratch_buffers(struct dwc3 *dwc) if (!WARN_ON(dwc->scratchbuf)) return 0; - scratch_addr = dma_map_single(dwc->dev, dwc->scratchbuf, + scratch_addr = dma_map_single(dwc->sysdev, dwc->scratchbuf, dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL); - if (dma_mapping_error(dwc->dev, scratch_addr)) { - dev_err(dwc->dev, "failed to map scratch buffer\n"); + if (dma_mapping_error(dwc->sysdev, scratch_addr)) { + dev_err(dwc->sysdev, "failed to map scratch buffer\n"); ret = -EFAULT; goto err0; } @@ -398,7 +399,7 @@ static int dwc3_setup_scratch_buffers(struct dwc3 *dwc) return 0; err1: - dma_unmap_single(dwc->dev, dwc->scratch_addr, dwc->nr_scratch * + dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL); err0: @@ -417,7 +418,7 @@ static void dwc3_free_scratch_buffers(struct dwc3 *dwc) if (!WARN_ON(dwc->scratchbuf)) return; - dma_unmap_single(dwc->dev, dwc->scratch_addr, dwc->nr_scratch * + dma_unmap_single(dwc->sysdev, dwc->scratch_addr, dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL); kfree(dwc->scratchbuf); } @@ -943,6 +944,13 @@ static int dwc3_probe(struct platform_device *pdev) dwc = PTR_ALIGN(mem, DWC3_ALIGN_MASK + 1); dwc->mem = mem; dwc->dev = dev; +#ifdef CONFIG_PCI + /* TODO: or some other way of detecting this? */ + if (dwc->dev->parent && dwc->dev->parent->bus == _bus_type) + dwc->sysdev = dwc->dev->parent; + else +#endif + dwc->sysdev = dwc->dev; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { @@ -1051,12 +1059,6 @@ static int dwc3_probe(struct platform_device *pdev) spin_lock_init(>lock); - if (!dev->dma_mask) { - dev->dma_mask = dev->parent->dma_mask; - dev->dma_parms = dev->parent->dma_parms; - dma_set_coherent_mask(dev, dev->parent->coherent_dma_mask); - } - pm_runtime_set_active(dev); pm_runtime_use_autosuspend(dev); pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 6b60e42..b166b0a 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -851,6 +851,7 @@ struct dwc3 { spinlock_t lock; struct device *dev; + struct device *sysdev; struct platform_device *xhci; struct resource xhci_resources[DWC3_XHCI_RESOURCES_NUM]; diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index fe79d77..4d13723 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -974,8 +974,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc, u32 transfer_size = 0; u32 maxpacket; - ret = usb_gadget_map_request(>gadget, >request, - dep->number); + ret =
[PATCH v3 2/6] usb: chipidea: use bus->sysdev for DMA configuration
From: Arnd BergmannSet the dma for chipidea from sysdev. This is inherited from its parent node. Also, do not set dma mask for child as it is not required now. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash Acked-by: Peter Chen --- Changes in v3: - No update Changes in v2: - integrate chipidea driver changes together. drivers/usb/chipidea/core.c | 3 --- drivers/usb/chipidea/host.c | 3 ++- drivers/usb/chipidea/udc.c | 10 ++ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..8917a03 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -833,9 +833,6 @@ struct platform_device *ci_hdrc_add_device(struct device *dev, } pdev->dev.parent = dev; - pdev->dev.dma_mask = dev->dma_mask; - pdev->dev.dma_parms = dev->dma_parms; - dma_set_coherent_mask(>dev, dev->coherent_dma_mask); ret = platform_device_add_resources(pdev, res, nres); if (ret) diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c index 111b0e0b..3218b49 100644 --- a/drivers/usb/chipidea/host.c +++ b/drivers/usb/chipidea/host.c @@ -116,7 +116,8 @@ static int host_start(struct ci_hdrc *ci) if (usb_disabled()) return -ENODEV; - hcd = usb_create_hcd(_ehci_hc_driver, ci->dev, dev_name(ci->dev)); + hcd = __usb_create_hcd(_ehci_hc_driver, ci->dev->parent, + ci->dev, dev_name(ci->dev), NULL); if (!hcd) return -ENOMEM; diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 661f43f..bc55922 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -423,7 +423,8 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq) hwreq->req.status = -EALREADY; - ret = usb_gadget_map_request(>gadget, >req, hwep->dir); + ret = usb_gadget_map_request_by_dev(ci->dev->parent, + >req, hwep->dir); if (ret) return ret; @@ -603,7 +604,8 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq) list_del_init(>td); } - usb_gadget_unmap_request(>ci->gadget, >req, hwep->dir); + usb_gadget_unmap_request_by_dev(hwep->ci->dev->parent, + >req, hwep->dir); hwreq->req.actual += actual; @@ -1904,13 +1906,13 @@ static int udc_start(struct ci_hdrc *ci) INIT_LIST_HEAD(>gadget.ep_list); /* alloc resources */ - ci->qh_pool = dma_pool_create("ci_hw_qh", dev, + ci->qh_pool = dma_pool_create("ci_hw_qh", dev->parent, sizeof(struct ci_hw_qh), 64, CI_HDRC_PAGE_SIZE); if (ci->qh_pool == NULL) return -ENOMEM; - ci->td_pool = dma_pool_create("ci_hw_td", dev, + ci->td_pool = dma_pool_create("ci_hw_td", dev->parent, sizeof(struct ci_hw_td), 64, CI_HDRC_PAGE_SIZE); if (ci->td_pool == NULL) { -- 2.1.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
[PATCH v3 4/6] usb: xhci: use bus->sysdev for DMA configuration
From: Arnd BergmannFor xhci-hcd platform device, all the DMA parameters are not configured properly, notably dma ops for dwc3 devices. So, set the dma for xhci from sysdev. sysdev is pointing to device that is known to the system firmware or hardware. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash --- Changes in v3: - No update Changes in v2: - Separate out xhci driver changes apart drivers/usb/host/xhci-mem.c | 12 ++-- drivers/usb/host/xhci-plat.c | 33 ++--- drivers/usb/host/xhci.c | 15 +++ 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6afe323..79608df 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -586,7 +586,7 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci, unsigned int num_stream_ctxs, struct xhci_stream_ctx *stream_ctx, dma_addr_t dma) { - struct device *dev = xhci_to_hcd(xhci)->self.controller; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs; if (size > MEDIUM_STREAM_ARRAY_SIZE) @@ -614,7 +614,7 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci, unsigned int num_stream_ctxs, dma_addr_t *dma, gfp_t mem_flags) { - struct device *dev = xhci_to_hcd(xhci)->self.controller; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs; if (size > MEDIUM_STREAM_ARRAY_SIZE) @@ -1644,7 +1644,7 @@ void xhci_slot_copy(struct xhci_hcd *xhci, static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags) { int i; - struct device *dev = xhci_to_hcd(xhci)->self.controller; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2); xhci_dbg_trace(xhci, trace_xhci_dbg_init, @@ -1716,7 +1716,7 @@ static void scratchpad_free(struct xhci_hcd *xhci) { int num_sp; int i; - struct device *dev = xhci_to_hcd(xhci)->self.controller; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; if (!xhci->scratchpad) return; @@ -1792,7 +1792,7 @@ void xhci_free_command(struct xhci_hcd *xhci, void xhci_mem_cleanup(struct xhci_hcd *xhci) { - struct device *dev = xhci_to_hcd(xhci)->self.controller; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; int size; int i, j, num_ports; @@ -2334,7 +2334,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags) int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) { dma_addr_t dma; - struct device *dev = xhci_to_hcd(xhci)->self.controller; + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; unsigned intval, val2; u64 val_64; struct xhci_segment *seg; diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index ed56bf9..beb95c8 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -139,6 +140,7 @@ static int xhci_plat_probe(struct platform_device *pdev) { const struct of_device_id *match; const struct hc_driver *driver; + struct device *sysdev; struct xhci_hcd *xhci; struct resource *res; struct usb_hcd *hcd; @@ -155,22 +157,39 @@ static int xhci_plat_probe(struct platform_device *pdev) if (irq < 0) return -ENODEV; + /* +* sysdev must point to a device that is known to the system firmware +* or PCI hardware. We handle these three cases here: +* 1. xhci_plat comes from firmware +* 2. xhci_plat is child of a device from firmware (dwc3-plat) +* 3. xhci_plat is grandchild of a pci device (dwc3-pci) +*/ + sysdev = >dev; + if (sysdev->parent && !sysdev->of_node && sysdev->parent->of_node) + sysdev = sysdev->parent; +#ifdef CONFIG_PCI + else if (sysdev->parent && sysdev->parent->parent && +sysdev->parent->parent->bus == _bus_type) + sysdev = sysdev->parent->parent; +#endif + /* Try to set 64-bit DMA first */ - if (WARN_ON(!pdev->dev.dma_mask)) + if (WARN_ON(!sysdev->dma_mask)) /* Platform did not initialize dma_mask */ - ret = dma_coerce_mask_and_coherent(>dev, + ret = dma_coerce_mask_and_coherent(sysdev, DMA_BIT_MASK(64)); else - ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64)); + ret =
[PATCH v3 6/6] usb: dwc3: Do not set dma coherent mask
From: Arnd BergmannThe dma mask is correctly set up by the DT probe function, no need to override it any more. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash --- Changes in v3: - No update Changes in v2: - club the cleanup for dma coherent mask for device drivers/usb/dwc3/dwc3-exynos.c | 10 -- drivers/usb/dwc3/dwc3-st.c | 1 - 2 files changed, 11 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index 2f1fb7e..e27899b 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -117,15 +116,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev) if (!exynos) return -ENOMEM; - /* -* Right now device-tree probed devices don't get dma_mask set. -* Since shared usb code relies on it, set it here for now. -* Once we move to full device tree support this will vanish off. -*/ - ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); - if (ret) - return ret; - platform_set_drvdata(pdev, exynos); exynos->dev = dev; diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c index 89a2f71..4d7439c 100644 --- a/drivers/usb/dwc3/dwc3-st.c +++ b/drivers/usb/dwc3/dwc3-st.c @@ -218,7 +218,6 @@ static int st_dwc3_probe(struct platform_device *pdev) if (IS_ERR(regmap)) return PTR_ERR(regmap); - dma_set_coherent_mask(dev, dev->coherent_dma_mask); dwc3_data->dev = dev; dwc3_data->regmap = regmap; -- 2.1.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 v4 1/4] usb: dbc: early driver for xhci debug capability
On Thu, 10 Nov 2016, Lu Baolu wrote: > On 11/09/2016 05:37 PM, Thomas Gleixner wrote: > > On Tue, 1 Nov 2016, Lu Baolu wrote: > >> +static void early_xdbc_write(struct console *con, const char *str, u32 n) > >> +{ > >> + int chunk, ret; > >> + static char buf[XDBC_MAX_PACKET]; > >> + int use_cr = 0; > >> + > >> + if (!xdbc.xdbc_reg) > >> + return; > >> + memset(buf, 0, XDBC_MAX_PACKET); > > How is that dealing with reentrancy? > > > > early_printk() does not protect against it. Peter has a patch to prevent > > concurrent access from different cpus, but it cannot and will never prevent > > reentrancy on the same cpu (interrupt, nmi). > > I can use a spinlock_irq to protect reentrancy of interrupt on the same > cpu. But I have no idea about the nmi one. spinlock wont work due to NMIs. > This seems to be a common issue for all early printk drivers. No. The other early printk drivers like serial do not have that problem as they simply do: while (*buf) { while (inb(UART) & TX_BUSY) cpu_relax(); outb(*buf++, UART); } The wait for the UART to become ready is independent of the context as it solely depends on the hardware. As a result you can see the output from irq/nmi intermingled with the one from thread context, but that's the only problem they have. The only thing you can do to make this work is to prevent printing in NMI context: write() { if (in_nmi()) return; raw_spinlock_irqsave(, flags); That fully serializes the writes and just ignores NMI context printks. Not optimal, but I fear that's all you can do. Thanks, tglx -- 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: xhci-mem: use passed in GFP flags instead of GFP_KERNEL
We normally use the passed in gfp flags for allocations, it's just these two which were missed. Fixes: 22d45f01a836 ("usb/xhci: replace pci_*_consistent() with dma_*_coherent()") Signed-off-by: Dan Carpenter--- >From static analysis. Not tested. diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 6afe323..3c4cc29 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2384,7 +2384,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) * "physically contiguous and 64-byte (cache line) aligned". */ xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), , - GFP_KERNEL); + flags); if (!xhci->dcbaa) goto fail; memset(xhci->dcbaa, 0, sizeof *(xhci->dcbaa)); @@ -2480,7 +2480,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) xhci->erst.entries = dma_alloc_coherent(dev, sizeof(struct xhci_erst_entry) * ERST_NUM_SEGS, , - GFP_KERNEL); + flags); if (!xhci->erst.entries) goto fail; xhci_dbg_trace(xhci, trace_xhci_dbg_init, -- 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/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue
* Johan Hovold[161110 11:43]: > On Thu, Nov 10, 2016 at 10:41:50AM -0700, Tony Lindgren wrote: > > * Johan Hovold [161110 09:04]: > > > I'm afraid that won't work as pm_runtime_get() would still succeed (i.e. > > > even after musb_suspend()). > > > > > > See 6f3c77b040fc ("PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, > > > even when disabled, v2"). > > > > But doesn't that assume that we have musb core as in musb->controller > > in RPM_ACTIVE state? While if it's been suspended that's not the > > case meaning rpm_resume would fail? > > Right, and it's still a good idea to check the return value of > pm_runtime_get(). It just won't be enough for when RPM_ACTIVE. > > > If we have a window for a race there with RPM_ACTIVE set, we could > > add musb->is_disabled flag that gets set in musb_suspend(). > > Yes. > > > > > In the long run it would be nice to make whatever optional state polling > > > > musb generic with just a glue layer callback. > > > > > > Yes, and make sure to stop polling in musb_suspend(). Would it be > > > possible to use the enable and disable ops for this until then? > > > > Hmm care to explain a bit more? That is assuming that rpm_resume() > > won't fail above.. And that using musb->is_disabled flag won't > > work.. > > By stopping the timer in musb->ops->disable which is called during > suspend, the race could perhaps also be avoided. Yes I think that's the way to go here :) Updated version below again. Regards, Tony 8< -- >From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Wed, 2 Nov 2016 19:59:05 -0700 Subject: [PATCH] usb: musb: Fix sleeping function called from invalid context for hdrc glue Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer that runs in softirq context. That causes a "BUG: sleeping function called from invalid context" every time when polling the cable status: [] (__might_sleep) from [] (__pm_runtime_resume+0x9c/0xa0) [] (__pm_runtime_resume) from [] (otg_timer+0x3c/0x254) [] (otg_timer) from [] (call_timer_fn+0xfc/0x41c) [] (call_timer_fn) from [] (expire_timers+0x120/0x210) [] (expire_timers) from [] (run_timer_softirq+0xa4/0xdc) [] (run_timer_softirq) from [] (__do_softirq+0x12c/0x594) I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. And looks like also musb_gadget_queue() suffers from the same problem. Let's fix the issue by using a list of delayed work then call it on resume. Note that we want to do this only when musb core and it's parent devices are awake, and we need to make sure the DSPS glue timer is stopped as noted by Johan Hovold . Later on we may be able to remove other delayed work in the musb driver and just do it from pending_resume_work. But this should be done only for delayed work that does not have other timing requirements beyond just being run on resume. Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") Reported-by: Johan Hovold Signed-off-by: Tony Lindgren --- drivers/usb/musb/musb_core.c | 110 +++-- drivers/usb/musb/musb_core.h | 7 +++ drivers/usb/musb/musb_dsps.c | 35 + drivers/usb/musb/musb_gadget.c | 32 ++-- 4 files changed, 166 insertions(+), 18 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1969,6 +1969,7 @@ static struct musb *allocate_instance(struct device *dev, INIT_LIST_HEAD(>control); INIT_LIST_HEAD(>in_bulk); INIT_LIST_HEAD(>out_bulk); + INIT_LIST_HEAD(>pending_list); musb->vbuserr_retry = VBUSERR_RETRY_COUNT; musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; @@ -2018,6 +2019,85 @@ static void musb_free(struct musb *musb) musb_host_free(musb); } +struct musb_pending_work { + int (*callback)(struct musb *musb, void *data); + void *data; + struct list_head node; +}; + +/* + * Called from musb_runtime_resume(), musb_resume(), and + * musb_queue_resume_work(). Callers must take musb->lock and must hold + * an RPM reference. + */ +static int musb_run_resume_work(struct musb *musb) +{ + struct musb_pending_work *w, *_w; + unsigned long flags; + int error = 0; + + spin_lock_irqsave(>list_lock, flags); + list_for_each_entry_safe(w, _w, >pending_list, node) { + if (w->callback) { + error = w->callback(musb, w->data); + if (error < 0) { + dev_err(musb->controller, + "resume callback %p failed: %i\n", + w->callback, error); + } + } +
Re: [PATCH v2 03/30] usb: dwc2: Make the DMA descriptor structure packed
On 11/10/2016 4:28 AM, David Laight wrote: > From: John Youn >> Sent: 10 November 2016 03:28 >> From: Vahram Aharonyan>> >> Make the DMA descriptor structure packed to guarantee alignment and size >> in memory. >> >> Signed-off-by: John Youn >> --- >> drivers/usb/dwc2/hw.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h >> index 6031efe..ee827e8 100644 >> --- a/drivers/usb/dwc2/hw.h >> +++ b/drivers/usb/dwc2/hw.h >> @@ -802,7 +802,7 @@ >> struct dwc2_dma_desc { >> u32 status; >> u32 buf; >> -}; >> +} __packed; > > Nack (not that my nacks have much force). > > You clearly do not understand what __packed means. > > The dma descriptor almost certainly has to be 32bit aligned (if not 64). > > By adding __packed you are telling the compiler that the structure can be > aligned to any boundary (eg 4n+1) so that it must use byte accesses and > shifts to access the members on systems that can't do misaligned accesses. > I can't help feeling this isn't what you want. > > I suspect you are trying to tell the compiler not to add hidden padding. > While the C language allows arbitrary padding after structure elements, > the implementations for specific architectures define when such padding > is added. > Padding will not be added if an item is already on its natural boundary > in any of the sane architecture specific definitions. > > Typical variations are whether 64bit items only need 32bit alignment. > Some odd architectures specify 32bit alignment for all structs (don't > think linux has any like that). > > By using __packed you are relying on a compiler extension - and so > can assume the compiler is used a sane structure layout. > If you are being really picky you could add a compile-time assert > that the structure is the correct size - but that isn't worth it > for 8 bytes. > Ok, maybe I did a poor job of wording the commit message and should not have used "alignment" to describe what we are doing. We are only using __packed in the sense that the elements should be "packed" together, i.e. not padded, so that we can provide it to the hardware as-is, without any compiler muckery. Whether or not that's actually necessary for such a trivial struct... probably not, but I don't know. Do you think it is still wrong in this context? 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 v2 11/13] Documentation: devicetree: dwc2: Add host DMA binding
On Thu, Nov 03, 2016 at 05:56:10PM -0700, John Youn wrote: > Add the snps,host-dma-disable binding. This controls whether to disable > DMA in host mode. > > Signed-off-by: John Youn> --- > Documentation/devicetree/bindings/usb/dwc2.txt | 1 + > 1 file changed, 1 insertion(+) 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