Re: [PATCH] HID: remove use of DRIVER_LICENSE
On Jan 05 2017 or thereabouts, Grant Grundler wrote: > Local "#define DRIVER_LICENSE" obfuscates which license is used > in MODULE_LICENSE(). "fgrep -R MODULE_LICENSE" is more informative > when the string is hard coded in MODULE_LICENSE. > > Signed-off-by: Grant Grundler > --- > Most of the kernel already uses hard coded strings. > The few places that don't are easy to fix. FWIW: Reviewed-by: Benjamin Tissoires -- 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: [PATCHv14 2/3] usb: USB Type-C connector class
Hi guys, On Thu, Jan 05, 2017 at 05:54:02PM +0200, Mika Westerberg wrote: > > +static ssize_t > > +typec_altmode_roles_show(struct device *dev, struct device_attribute *attr, > > +char *buf) > > +{ > > + struct typec_mode *mode = container_of(attr, struct typec_mode, > > + roles_attr); > > + ssize_t ret; > > + > > + switch (mode->roles) { > > + case TYPEC_PORT_DFP: > > + ret = sprintf(buf, "source\n"); > > Extra space after '='. > > > + break; > > + case TYPEC_PORT_UFP: > > + ret = sprintf(buf, "sink\n"); > > + break; > > + case TYPEC_PORT_DRP: > > + default: > > + ret = sprintf(buf, "source\nsink\n"); > > I wonder if "source sink" instead is better? Along the lines of > /sys/power/state. > > Then you can print "[source] sink" when source is selected and so on. That is more or less how I originally proposed how we list the roles in general. I introduced the separate "current_*_role" and "supported_*_roles" attribute files because somebody wanted them. I don't remember the reason why they were preferred to be in separate attribute files. Oliver! Guenter! Do we really need to list the current and supported roles in separate attribute files? Can't we just have the "power_role" and "data_role" attribute files for the ports instead of the separate "supported_*_roles" and "current_*_role", and show the current role like Mika proposes? I definitely would prefer it that way because it is similar style used in other places like Mike pointed out. And since we are talking about the ABI, can we also change the listing of the accessory mode back to just "audio" and "debug" like I originally had it? I don't remember who and why wanted it to be changed to "Audio Adapter Accessory Mode" and "Debug Accessory Mode", but it differs from the style we list the other details. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/1] Platform driver support for 'amd5536udc' driver
On Friday, January 6, 2017 12:29:12 PM CET Raviteja Garimella wrote: > Hi Arnd, > > On Fri, Jan 6, 2017 at 3:33 AM, Arnd Bergmann wrote: > > On Thursday, January 5, 2017 1:53:16 PM CET Raviteja Garimella wrote: > >> The UDC is based on Synopsys Designware core USB (2.0) Device controller > >> IP. > > ... > >> This is a request for comments from maintainers/others regarding approach > >> on whether to have 2 different drivers (one each for AMD and Broadcom) > >> with a common library (3 files in total), or have a single driver like > >> it's done in this patch and have the driver filename changed to some > >> common name based on ther underlying IP, like snps_udc.c. > > > > I have not looked at the code at all, so sorry for my ignorance, but > > isn't the IP block you describe the one that drivers/usb/dwc2/ is for? > > Could you add support for the Broadcom hardware there instead? > > The current driver I submitted is for a different Synopsys IP (USB > Device Controller IP, > not the HS OTG). It's confirmed by John Youn (from Synopsys) earlier. > Ok, sounds fine the. I'd suggest taking the current driver than and splitting out the pci_driver front-end into a separate module that calls exported symbols of the main driver, with the new platform driver in a third file that also calls the same exported symbols. Arnd -- 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: dwc2: use u32 for DT binding parameters
Commit 05ee799f2021 ("usb: dwc2: Move gadget settings into core_params") changes to type u16 for DT binding "g-rx-fifo-size" and "g-np-tx-fifo-size" but use type u32 for "g-tx-fifo-size". Finally the the first two parameters cannot be passed successfully with wrong data format. This is found the data transferring broken on 96boards Hikey. This patch is to change all parameters to u32 type, and verified on Hikey board the DT parameters can pass successfully. Signed-off-by: Leo Yan --- drivers/usb/dwc2/core.h | 4 ++-- drivers/usb/dwc2/params.c | 10 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 935ef36..7b2bc39 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -496,8 +496,8 @@ struct dwc2_core_params { * properties and cannot be set directly in this structure. */ bool g_dma; - u16 g_rx_fifo_size; - u16 g_np_tx_fifo_size; + u32 g_rx_fifo_size; + u32 g_np_tx_fifo_size; u32 g_tx_fifo_size[MAX_EPS_CHANNELS]; }; diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index c96ae72..e7b12ef 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -402,16 +402,16 @@ static void dwc2_set_param(struct dwc2_hsotg *hsotg, void *param, } /** - * dwc2_set_param_u16() - Set a u16 parameter + * dwc2_set_param_u32() - Set a u32 parameter * * See dwc2_set_param(). */ -static void dwc2_set_param_u16(struct dwc2_hsotg *hsotg, u16 *param, +static void dwc2_set_param_u32(struct dwc2_hsotg *hsotg, u32 *param, bool lookup, char *property, u16 legacy, u16 def, u16 min, u16 max) { dwc2_set_param(hsotg, param, lookup, property, - legacy, def, min, max, 2); + legacy, def, min, max, 4); } /** @@ -1186,12 +1186,12 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg, * auto-detect if the hardware does not support the * default. */ - dwc2_set_param_u16(hsotg, &p->g_rx_fifo_size, + dwc2_set_param_u32(hsotg, &p->g_rx_fifo_size, true, "g-rx-fifo-size", 2048, hw->rx_fifo_size, 16, hw->rx_fifo_size); - dwc2_set_param_u16(hsotg, &p->g_np_tx_fifo_size, + dwc2_set_param_u32(hsotg, &p->g_np_tx_fifo_size, true, "g-np-tx-fifo-size", 1024, hw->dev_nperio_tx_fifo_size, 16, hw->dev_nperio_tx_fifo_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
Re: [PATCH] HID: remove use of DRIVER_LICENSE
On Thu, 5 Jan 2017, Grant Grundler wrote: > Local "#define DRIVER_LICENSE" obfuscates which license is used > in MODULE_LICENSE(). "fgrep -R MODULE_LICENSE" is more informative > when the string is hard coded in MODULE_LICENSE. Applied to hid.git#for-4.11/upstream. Thanks, -- Jiri Kosina SUSE Labs -- 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 3/4] ARM: dts: sunxi: add usb_otg and usbphy node for V3s SoC
On Tue, Jan 03, 2017 at 11:25:33PM +0800, Icenowy Zheng wrote: > V3s SoC features a USB PHY controller and a MUSB OTG controller. > > Add device nodes for them. > > Signed-off-by: Icenowy Zheng This can be merged in your other DTSI patch. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH 2/4] musb: sunxi: add support for the variant in H3/V3s SoC
On Tue, Jan 03, 2017 at 11:25:32PM +0800, Icenowy Zheng wrote: > Allwinner H3/V3s features a variant of MUSB controller, which lacks one > endpoint. > > Add support for it. > > Signed-off-by: Icenowy Zheng Acked-by: Maxime Ripard Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH 1/4] phy: sun4i-usb: add support for V3s USB PHY
On Tue, Jan 03, 2017 at 11:25:31PM +0800, Icenowy Zheng wrote: > Allwinner V3s come with a USB PHY controller slightly different to other > SoCs, with only one PHY. > > Add support for it. > > Signed-off-by: Icenowy Zheng Acked-by: Maxime Ripard Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH v11 0/8] power: add power sequence library
On Thu, Jan 05, 2017 at 02:01:51PM +0800, Peter Chen wrote: > Hi all, > > This is a follow-up for my last power sequence framework patch set [1]. > According to Rob Herring and Ulf Hansson's comments[2]. The kinds of > power sequence instances will be added at postcore_initcall, the match > criteria is compatible string first, if the compatible string is not > matched between dts and library, it will try to use generic power sequence. > > The host driver just needs to call of_pwrseq_on/of_pwrseq_off > if only one power sequence instance is needed, for more power sequences > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub > driver). > > In future, if there are special power sequence requirements, the special > power sequence library can be created. > > This patch set is tested on i.mx6 sabresx evk using a dts change, I use > two hot-plug devices to simulate this use case, the related binding > change is updated at patch [1/6], The udoo board changes were tested > using my last power sequence patch set.[3] > > Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also > need to power on itself before it can be found by ULPI bus. > > [1] http://www.spinics.net/lists/linux-usb/msg142755.html > [2] http://www.spinics.net/lists/linux-usb/msg143106.html > [3] http://www.spinics.net/lists/linux-usb/msg142815.html > Unfortunately I was unable to utilize this library to solve Odroid U3 usb3503/lan9730 power sequence (as a continuation of my old series [0][1]). The main problem is that power sequence instance (pwrseq_generic.c) is not a device. I don't get why it is not a device. It would be rather intuitive to me to make it a device. Also it would make life easier because you could use all device-like consumer APIs (of getting clocks, GPIOs etc). Since it is not a device, it is not possible to use regulator consumer API. I need a regulator because I need to toggle the power. Other encountered issue is that power sequence happens early, before the unused regulators are turned off by the system. However maybe this is the necessity from other point of view. My case is annoyingly over-complicated so I am slowly getting sertain that it is not generic. Anyway it looks like this: EHCI controller | |--HSIC0--lan9730 (reset is done only through regulator) |--HSIC1--usb3504 (it has a reset GPIO... but it is toggled by usb3504 driver) Overall, I want to reset the lan9730. This can be done only through power - buck8. buck8 state is an logical OR of register set by I2C and GPIO. Thus to turn it off, it is not sufficient just to set register to 0x0 because the GPIO is keeping it on. The best way is to switch the buck8 to gpio-enable-control thus only GPIO will be toggling it... still through the regulator consumer API (because it is an GPIO for the regulator, not for the reset!). However these two seems coupled. On invalid reset sequence, both chips die. The usb3504 has its own existing reset sequence and my findings show that it should happen after lan9730 reset sequence. Otherwise all devices might be lost... [0] http://www.spinics.net/lists/linux-mmc/msg37386.html [1] https://github.com/krzk/linux/commits/for-next/odroid-u3-usb3503-lan-boot-fixes-v4 Best regards, Krzysztof > Changes for v11: > - Fix warning: (USB) selects POWER_SEQUENCE which has unmet direct > dependencies (OF) > - Delete redundant copyright statement. > - Change pr_warn to pr_debug at wrseq_find_available_instance > - Refine kerneldoc > - %s/ENONET/ENOENT > - Allocate pwrseq list node before than carry out power sequence on > - Add mutex_lock/mutex_lock for pwrseq node browse at > pwrseq_find_available_instance > - Add pwrseq_suspend/resume for API both single instance and list > - Add .pwrseq_suspend/resume for pwrseq_generic.c > - Add pwrseq_suspend_list and pwrseq_resume_list for USB hub suspend > and resume routine > > Changes for v10: > - Improve the kernel-doc for power sequence core, including exported APIs and > main structure. [Patch 2/8] > - Change Kconfig, and let the user choose power sequence. [Patch 2/8] > - Delete EXPORT_SYMBOL and change related APIs as local, these APIs do not > be intended to export currently. [Patch 2/8] > - Selete POWER_SEQUENCE at USB core's Kconfig. [Patch 4/8] > > Changes for v9: > - Add Vaibhav Hiremath's reviewed-by [Patch 4/8] > - Rebase to v4.9-rc1 > > Changes for v8: > - Allocate one extra pwrseq instance if pwrseq_get has succeed, it can avoid > preallocate instances problem which the number of instance is decided at > compile time, thanks for Heiko Stuebner's suggestion [Patch 2/8] > - Delete pwrseq_compatible_sample.c which is the demo purpose to show > compatible > match method. [Patch 2/8] > - Add Maciej S. Szmigiero's tested-by. [Patch 7/8] > > Changes for v7: > - Create kinds of power sequence instance at postcore_initcall, and match > the instance with node using compatible strin
Re: Oops with dwc3 in device mode and functionfs
On Tue, 3 Jan 2017 18:18:02 +0100, Vincent Pelletier wrote: > Sadly, this fix alone is not enough to get a functional device. I did > not investigate much yet, and need to get some sleep. I investigated more. The next issue is that often (>9 times out of 10) the dwc3 fails to respond to the SET_CONFIGURATION standard request. As a result, only EP0 works. Host-side, syslog contains a -110 (ETIMEDOUT) error. Re-triggering a SET_CONFIGURATION causes the same timeout to happen again. Device-side, functionfs does trigger the "ENABLE" event. Altogether, ep0 does work (vendor-specific requests from host do get received by the program on device which listens to events on ep0 file). Host (comprehensibly) refuses to emit requests to endpoints which are not considered as configured, so I cannot test this part. Plugging my USB protocol analyser, I see corrupted USB PIDs. Here is a capture, starting at the beginning od the last GET_DESCRIPTOR transaction before the SET_CONFIGURATION one: 001:23.273'515"383n SETUP @054.00 DATA0 8B ACK 000 80 06 03 03 09 04 ff 00 001:23.273'518"000n IN @054.00 NAK 001:23.273'539"966n IN @054.00 NAK 001:23.273'582"983n IN @054.00 NAK 001:23.273'604"666n IN @054.00 DATA134B ACK 000 22 03 46 00 5a 00 45 00 44 00 34 00 34 00 33 00 ..F.Z.E.D.4.4.3. 010 44 00 30 00 31 00 54 00 34 00 32 00 35 00 30 00 D.0.1.T.4.2.5.0. 020 31 001. 001:23.273'607"666n OUT @054.00 DATA1 0B NAK 001:23.273'628"966n PING@054.00 NAK 001:23.273'649"650n PING@054.00 NAK 001:23.273'683"966n PING@054.00 ACK 001:23.273'685"300n OUT @054.00 DATA1 0B ACK 001:23.274'459"183n (bad pid) 0xf0 0x36 0xf8 001:23.275'036"750n (bad pid) 0x03 0x00 0x00 0x00 0x96 0x52 0x68 0xfa 001:23.275'037"266n SETUP @054.00 DATA0 8B ACK 000 00 09 01 00 00 00 00 00 001:23.275'039"750n IN @054.00 NAK 001:23.275'067"183n IN @054.00 NAK 001:23.275'110"183n IN @054.00 NAK 001:23.275'131"866n IN @054.00 NAK 001:23.275'153"183n IN @054.00 NAK 001:23.275'196"200n IN @054.00 NAK 001:23.275'217"883n IN @054.00 NAK 001:23.275'239"200n IN @054.00 NAK 001:23.275'260"883n IN @054.00 NAK 001:23.275'287"300n IN @054.00 NAK 001:23.275'308"900n IN @054.00 NAK 001:23.275'329"900n IN @054.00 NAK 001:23.275'356"216n IN @054.00 NAK 001:23.275'377"900n IN @054.00 NAK 001:23.275'399"216n IN @054.00 NAK 001:23.275'442"233n IN @054.00 NAK 001:23.275'463"916n IN @054.00 NAK 001:23.275'485"233n IN @054.00 NAK 001:23.275'506"916n IN @054.00 NAK 001:23.275'528"233n IN @054.00 NAK 001:23.275'571"250n IN @054.00 NAK 001:23.275'592"933n IN @054.00 NAK 001:23.275'614"250n IN @054.00 NAK 001:23.275'635"933n IN @054.00 NAK 001:23.275'662"350n IN @054.00 NAK 001:23.275'683"950n IN @054.00 NAK 001:23.275'706"266n IN @054.00 NAK 001:23.275'727"950n IN @054.00 NAK 001:23.275'749"266n IN @054.00 NAK 001:23.275'770"950n IN @054.00 NAK 001:23.275'811"200n (bad pid) 0xf0 0x36 0xf8 001:23.275'792"266n IN @054.00 NAK (incomplete transaction) 001:23.278'811"983n HS idle The format being: timestamp token_type "@"device"."endpoint data_type bytes"B" handshake_type data_stage_hexdump Strangely, when plugging the device on a hub seems to improve the situation. Sadly, my protocol analyser gets confused when plugged between that hub and the device, and fails to capture anything (it likely fails to detect the chirp handshake). There seems to be another issue (or is it just a consequence of this one ?) where it may become completely silent, unable to join the bus when udc identifier is writted to UDC file. Regards, -- Vincent Pelletier -- 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: [PATCHv14 2/3] usb: USB Type-C connector class
On Fri, Jan 06, 2017 at 12:54:05PM +0200, Heikki Krogerus wrote: > Hi guys, > > On Thu, Jan 05, 2017 at 05:54:02PM +0200, Mika Westerberg wrote: > > > +static ssize_t > > > +typec_altmode_roles_show(struct device *dev, struct device_attribute > > > *attr, > > > + char *buf) > > > +{ > > > + struct typec_mode *mode = container_of(attr, struct typec_mode, > > > +roles_attr); > > > + ssize_t ret; > > > + > > > + switch (mode->roles) { > > > + case TYPEC_PORT_DFP: > > > + ret = sprintf(buf, "source\n"); > > > > Extra space after '='. > > > > > + break; > > > + case TYPEC_PORT_UFP: > > > + ret = sprintf(buf, "sink\n"); > > > + break; > > > + case TYPEC_PORT_DRP: > > > + default: > > > + ret = sprintf(buf, "source\nsink\n"); > > > > I wonder if "source sink" instead is better? Along the lines of > > /sys/power/state. > > > > Then you can print "[source] sink" when source is selected and so on. > > That is more or less how I originally proposed how we list the roles > in general. I introduced the separate "current_*_role" and > "supported_*_roles" attribute files because somebody wanted them. I > don't remember the reason why they were preferred to be in separate > attribute files. > > Oliver! Guenter! Do we really need to list the current and supported > roles in separate attribute files? Can't we just have the "power_role" > and "data_role" attribute files for the ports instead of the separate > "supported_*_roles" and "current_*_role", and show the current role > like Mika proposes? I definitely would prefer it that way because it > is similar style used in other places like Mike pointed out. > Consistency with other drivers/attribute should be preferrable, but either way is ok with me. > And since we are talking about the ABI, can we also change the listing > of the accessory mode back to just "audio" and "debug" like I > originally had it? I don't remember who and why wanted it to be > changed to "Audio Adapter Accessory Mode" and "Debug Accessory Mode", > but it differs from the style we list the other details. > I prefer computer readable attributes over human readable, so the change is fine with me. Guenter -- 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 v11 0/8] power: add power sequence library
On Fri, Jan 06, 2017 at 05:18:41PM +0200, Krzysztof Kozlowski wrote: > On Thu, Jan 05, 2017 at 02:01:51PM +0800, Peter Chen wrote: > > Hi all, > > > > This is a follow-up for my last power sequence framework patch set [1]. > > According to Rob Herring and Ulf Hansson's comments[2]. The kinds of > > power sequence instances will be added at postcore_initcall, the match > > criteria is compatible string first, if the compatible string is not > > matched between dts and library, it will try to use generic power sequence. > > > > The host driver just needs to call of_pwrseq_on/of_pwrseq_off > > if only one power sequence instance is needed, for more power sequences > > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub > > driver). > > > > In future, if there are special power sequence requirements, the special > > power sequence library can be created. > > > > This patch set is tested on i.mx6 sabresx evk using a dts change, I use > > two hot-plug devices to simulate this use case, the related binding > > change is updated at patch [1/6], The udoo board changes were tested > > using my last power sequence patch set.[3] > > > > Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also > > need to power on itself before it can be found by ULPI bus. > > > > [1] http://www.spinics.net/lists/linux-usb/msg142755.html > > [2] http://www.spinics.net/lists/linux-usb/msg143106.html > > [3] http://www.spinics.net/lists/linux-usb/msg142815.html > > > > Unfortunately I was unable to utilize this library to solve Odroid U3 > usb3503/lan9730 power sequence (as a continuation of my old series [0][1]). > > The main problem is that power sequence instance (pwrseq_generic.c) is > not a device. I don't get why it is not a device. It would be rather > intuitive to me to make it a device. Also it would make life easier > because you could use all device-like consumer APIs (of getting clocks, > GPIOs etc). Since it is not a device, it is not possible to use > regulator consumer API. > > I need a regulator because I need to toggle the power. > > Other encountered issue is that power sequence happens early, before the > unused regulators are turned off by the system. However maybe this is > the necessity from other point of view. > > My case is annoyingly over-complicated so I am slowly getting sertain > that it is not generic. Anyway it looks like this: > > EHCI controller > | > |--HSIC0--lan9730 (reset is done only through regulator) > |--HSIC1--usb3504 (it has a reset GPIO... but it is toggled by > usb3504 driver) > > Overall, I want to reset the lan9730. This can be done only through > power - buck8. buck8 state is an logical OR of register set by I2C and > GPIO. Thus to turn it off, it is not sufficient just to set register to > 0x0 because the GPIO is keeping it on. The best way is to switch the > buck8 to gpio-enable-control thus only GPIO will be toggling it... still > through the regulator consumer API (because it is an GPIO for the > regulator, not for the reset!). > > However these two seems coupled. On invalid reset sequence, both chips > die. The usb3504 has its own existing reset sequence and my findings > show that it should happen after lan9730 reset sequence. Otherwise all > devices might be lost... Update - it's working! I skipped entirely the regulator API and I am playing with its GPIO. This sounds like a hack but finally after few days of different tries I was able to find a working, reproducible solution. It turned out that the yours pwrseq is working quite good. I'll send a follow-up for my board soon. Best regards, Krzysztof > > [0] http://www.spinics.net/lists/linux-mmc/msg37386.html > [1] > https://github.com/krzk/linux/commits/for-next/odroid-u3-usb3503-lan-boot-fixes-v4 > -- 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 01/14] USB: serial: ch341: fix initial modem-control state
DTR and RTS will be asserted by the tty-layer when the port is opened and deasserted on close (if HUPCL is set). Make sure the initial state is not-asserted before the port is first opened as well. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 2597b83a8ae2..d133e72fe888 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -258,7 +258,6 @@ static int ch341_port_probe(struct usb_serial_port *port) spin_lock_init(&priv->lock); priv->baud_rate = DEFAULT_BAUD_RATE; - priv->line_control = CH341_BIT_RTS | CH341_BIT_DTR; r = ch341_configure(port->serial->dev, priv); if (r < 0) -- 2.10.2 -- 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 00/14] USB: serial: ch341: fixes and improved CH340 support
This series fixes a number of problems with the ch341 driver, and adds support for a class of CH340 devices which does not seem to support changing the intial line settings (and does not support using the init vendor command to do so either). I noticed that using the init command to update the line settings has the undesired side effect of deasserting the DTR/RTS lines. And since CH341A can be supported also using direct register writes (and we have a quirky class of devices which clearly does not support using init at all), I decided to revert to using direct register writes again. These patches have been verified against CH340G and CH341A, but I need help with testing the quirky class of CH340 devices (which Russel and possibly also Eddie has), and any other devices which you may have access too. I intend to merge the first seven fixes for v4.10-rc, and save the remaining patches for 4.11. Thanks, Johan Changes in v2 - revert to using direct register writes to update line settings for all device types (avoids toggling DTR/RTS) - use GFP_NOIO in reset_resume - rename lcr variable in set_termios - update a few commit messages Johan Hovold (14): USB: serial: ch341: fix initial modem-control state USB: serial: ch341: fix open and resume after B0 USB: serial: ch341: fix modem-control and B0 handling USB: serial: ch341: fix open error handling USB: serial: ch341: fix resume after reset USB: serial: ch341: fix line settings after reset-resume USB: serial: ch341: fix baud rate and line-control handling USB: serial: ch341: fix modem-status handling USB: serial: ch341: fix control-message error handling USB: serial: ch341: clean up control debug messages USB: serial: ch341: rename shadow modem-control register USB: serial: ch341: rename modem-status register USB: serial: ch341: rename LCR variable in set_termios USB: serial: ch341: change initial line-control settings drivers/usb/serial/ch341.c | 212 ++--- 1 file changed, 121 insertions(+), 91 deletions(-) -- 2.10.2 -- 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 10/14] USB: serial: ch341: clean up control debug messages
Clean up the control-transfer debug messages by dropping redundant information and unnecessary casts. Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 86692d2f8523..84c10b63732b 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -107,8 +107,8 @@ static int ch341_control_out(struct usb_device *dev, u8 request, { int r; - dev_dbg(&dev->dev, "ch341_control_out(%02x,%02x,%04x,%04x)\n", - USB_DIR_OUT|0x40, (int)request, (int)value, (int)index); + dev_dbg(&dev->dev, "%s - (%02x,%04x,%04x)\n", __func__, + request, value, index); r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, @@ -125,9 +125,8 @@ static int ch341_control_in(struct usb_device *dev, { int r; - dev_dbg(&dev->dev, "ch341_control_in(%02x,%02x,%04x,%04x,%p,%u)\n", - USB_DIR_IN|0x40, (int)request, (int)value, (int)index, buf, - (int)bufsize); + dev_dbg(&dev->dev, "%s - (%02x,%04x,%04x,%u)\n", __func__, + request, value, index, bufsize); r = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, -- 2.10.2 -- 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 04/14] USB: serial: ch341: fix open error handling
Make sure to stop the interrupt URB before returning on errors during open. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 0cc5056b304d..8f41d4385f1c 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -319,7 +319,7 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port) r = ch341_configure(serial->dev, priv); if (r) - goto out; + return r; if (tty) ch341_set_termios(tty, port, NULL); @@ -329,12 +329,19 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port) if (r) { dev_err(&port->dev, "%s - failed to submit interrupt urb: %d\n", __func__, r); - goto out; + return r; } r = usb_serial_generic_open(tty, port); + if (r) + goto err_kill_interrupt_urb; + + return 0; + +err_kill_interrupt_urb: + usb_kill_urb(port->interrupt_in_urb); -out: return r; + return r; } /* Old_termios contains the original termios settings and -- 2.10.2 -- 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 09/14] USB: serial: ch341: fix control-message error handling
A short control transfer would currently fail to be detected, something which could lead to stale buffer data being used as valid input. Check for short transfers, and make sure to log any transfer errors. Fixes: 6ce76104781a ("USB: Driver for CH341 USB-serial adaptor") Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 32 +--- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index e3239df0e75f..86692d2f8523 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -113,6 +113,8 @@ static int ch341_control_out(struct usb_device *dev, u8 request, r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, value, index, NULL, 0, DEFAULT_TIMEOUT); + if (r < 0) + dev_err(&dev->dev, "failed to send control message: %d\n", r); return r; } @@ -130,7 +132,20 @@ static int ch341_control_in(struct usb_device *dev, r = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, value, index, buf, bufsize, DEFAULT_TIMEOUT); - return r; + if (r < bufsize) { + if (r >= 0) { + dev_err(&dev->dev, + "short control message received (%d < %u)\n", + r, bufsize); + r = -EIO; + } + + dev_err(&dev->dev, "failed to receive control message: %d\n", + r); + return r; + } + + return 0; } static int ch341_set_baudrate_lcr(struct usb_device *dev, @@ -181,9 +196,9 @@ static int ch341_set_handshake(struct usb_device *dev, u8 control) static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv) { + const unsigned int size = 2; char *buffer; int r; - const unsigned size = 8; unsigned long flags; buffer = kmalloc(size, GFP_KERNEL); @@ -194,14 +209,9 @@ static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - /* setup the private status if available */ - if (r == 2) { - r = 0; - spin_lock_irqsave(&priv->lock, flags); - priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT; - spin_unlock_irqrestore(&priv->lock, flags); - } else - r = -EPROTO; + spin_lock_irqsave(&priv->lock, flags); + priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT; + spin_unlock_irqrestore(&priv->lock, flags); out: kfree(buffer); return r; @@ -211,9 +221,9 @@ out:kfree(buffer); static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) { + const unsigned int size = 2; char *buffer; int r; - const unsigned size = 8; buffer = kmalloc(size, GFP_KERNEL); if (!buffer) -- 2.10.2 -- 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 14/14] USB: serial: ch341: change initial line-control settings
Some CH340 devices appear unable to change the initial LCR settings, so set a sane 8N1 default during probe to enable basic support for such devices. Also drop a redundant LCR read during device initialisation. Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index c51ec9802856..351745aec0e1 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -238,15 +238,6 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - /* expect two bytes 0x56 0x00 */ - r = ch341_control_in(dev, CH341_REQ_READ_REG, 0x2518, 0, buffer, size); - if (r < 0) - goto out; - - r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, 0x0050); - if (r < 0) - goto out; - r = ch341_set_baudrate_lcr(dev, priv, priv->lcr); if (r < 0) goto out; @@ -268,6 +259,11 @@ static int ch341_port_probe(struct usb_serial_port *port) spin_lock_init(&priv->lock); priv->baud_rate = DEFAULT_BAUD_RATE; + /* +* Some CH340 devices appear unable to change the initial LCR +* settings, so set a sane 8N1 default. +*/ + priv->lcr = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8; r = ch341_configure(port->serial->dev, priv); if (r < 0) -- 2.10.2 -- 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 06/14] USB: serial: ch341: fix line settings after reset-resume
A recent change added support for modifying the default line-control settings, but did not make sure that the modified settings were used as part of reconfiguration after a device has been reset during resume. This caused a port that was open before suspend to be unusable until being closed and reopened. Fixes: ba781bdf8662 ("USB: serial: ch341: add support for parity, frame length, stop bits") Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 5343d65f3b52..eabdd05a2147 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -95,6 +95,7 @@ struct ch341_private { unsigned baud_rate; /* set baud rate */ u8 line_control; /* set line control value RTS/DTR */ u8 line_status; /* active status of modem control inputs */ + u8 lcr; }; static void ch341_set_termios(struct tty_struct *tty, @@ -232,7 +233,7 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - r = ch341_init_set_baudrate(dev, priv, 0); + r = ch341_init_set_baudrate(dev, priv, priv->lcr); if (r < 0) goto out; @@ -397,6 +398,8 @@ static void ch341_set_termios(struct tty_struct *tty, if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); + } else if (r == 0) { + priv->lcr = ctrl; } } -- 2.10.2 -- 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 07/14] USB: serial: ch341: fix baud rate and line-control handling
Revert to using direct register writes to set the divisor and line-control registers. A recent change switched to using the init vendor command to update these registers, something which also enabled support for CH341A devices. It turns out that simply setting bit 7 in the divisor register is sufficient to support CH341A and specifically prevent data from being buffered until a full endpoint-size packet (32 bytes) has been received. Using the init command also had the side-effect of temporarily deasserting the DTR/RTS signals on every termios change (including initialisation on open) something which for example could cause problems in setups where DTR is used to trigger a reset. Fixes: 4e46c410e050 ("USB: serial: ch341: reinitialize chip on reconfiguration") Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 24 +--- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index eabdd05a2147..8d7b0847109b 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -133,8 +133,8 @@ static int ch341_control_in(struct usb_device *dev, return r; } -static int ch341_init_set_baudrate(struct usb_device *dev, - struct ch341_private *priv, unsigned ctrl) +static int ch341_set_baudrate_lcr(struct usb_device *dev, + struct ch341_private *priv, u8 lcr) { short a; int r; @@ -157,9 +157,19 @@ static int ch341_init_set_baudrate(struct usb_device *dev, factor = 0x1 - factor; a = (factor & 0xff00) | divisor; - /* 0x9c is "enable SFR_UART Control register and timer" */ - r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT, - 0x9c | (ctrl << 8), a | 0x80); + /* +* CH341A buffers data until a full endpoint-size packet (32 bytes) +* has been received unless bit 7 is set. +*/ + a |= BIT(7); + + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, a); + if (r) + return r; + + r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, lcr); + if (r) + return r; return r; } @@ -233,7 +243,7 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - r = ch341_init_set_baudrate(dev, priv, priv->lcr); + r = ch341_set_baudrate_lcr(dev, priv, priv->lcr); if (r < 0) goto out; @@ -394,7 +404,7 @@ static void ch341_set_termios(struct tty_struct *tty, if (baud_rate) { priv->baud_rate = baud_rate; - r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl); + r = ch341_set_baudrate_lcr(port->serial->dev, priv, ctrl); if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); -- 2.10.2 -- 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 08/14] USB: serial: ch341: fix modem-status handling
The modem-status register was read as part of device configuration at port_probe and then again at open (and reset-resume). During open (and reset-resume) the MSR was read before submitting the interrupt URB, something which could lead to an MSR-change going unnoticed when it races with open (reset-resume). Fix this by dropping the redundant reconfiguration of the port at every open, and only read the MSR after the interrupt URB has been submitted. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 8d7b0847109b..e3239df0e75f 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -238,21 +238,11 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - /* expect 0xff 0xee */ - r = ch341_get_status(dev, priv); - if (r < 0) - goto out; - r = ch341_set_baudrate_lcr(dev, priv, priv->lcr); if (r < 0) goto out; r = ch341_set_handshake(dev, priv->line_control); - if (r < 0) - goto out; - - /* expect 0x9f 0xee */ - r = ch341_get_status(dev, priv); out: kfree(buffer); return r; @@ -324,14 +314,9 @@ static void ch341_close(struct usb_serial_port *port) /* open this device, set default parameters */ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port) { - struct usb_serial *serial = port->serial; struct ch341_private *priv = usb_get_serial_port_data(port); int r; - r = ch341_configure(serial->dev, priv); - if (r) - return r; - if (tty) ch341_set_termios(tty, port, NULL); @@ -343,6 +328,12 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port) return r; } + r = ch341_get_status(port->serial->dev, priv); + if (r < 0) { + dev_err(&port->dev, "failed to read modem status: %d\n", r); + goto err_kill_interrupt_urb; + } + r = usb_serial_generic_open(tty, port); if (r) goto err_kill_interrupt_urb; @@ -609,6 +600,12 @@ static int ch341_reset_resume(struct usb_serial *serial) ret); return ret; } + + ret = ch341_get_status(port->serial->dev, priv); + if (ret < 0) { + dev_err(&port->dev, "failed to read modem status: %d\n", + ret); + } } return usb_serial_generic_resume(serial); -- 2.10.2 -- 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 13/14] USB: serial: ch341: rename LCR variable in set_termios
Rename the line-control-register variable in set_termios to "lcr" and use u8 type to match the shadow register. Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 2981d1934874..c51ec9802856 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -364,7 +364,7 @@ static void ch341_set_termios(struct tty_struct *tty, struct ch341_private *priv = usb_get_serial_port_data(port); unsigned baud_rate; unsigned long flags; - unsigned char ctrl; + u8 lcr; int r; /* redundant changes may cause the chip to lose bytes */ @@ -373,43 +373,43 @@ static void ch341_set_termios(struct tty_struct *tty, baud_rate = tty_get_baud_rate(tty); - ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; + lcr = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; switch (C_CSIZE(tty)) { case CS5: - ctrl |= CH341_LCR_CS5; + lcr |= CH341_LCR_CS5; break; case CS6: - ctrl |= CH341_LCR_CS6; + lcr |= CH341_LCR_CS6; break; case CS7: - ctrl |= CH341_LCR_CS7; + lcr |= CH341_LCR_CS7; break; case CS8: - ctrl |= CH341_LCR_CS8; + lcr |= CH341_LCR_CS8; break; } if (C_PARENB(tty)) { - ctrl |= CH341_LCR_ENABLE_PAR; + lcr |= CH341_LCR_ENABLE_PAR; if (C_PARODD(tty) == 0) - ctrl |= CH341_LCR_PAR_EVEN; + lcr |= CH341_LCR_PAR_EVEN; if (C_CMSPAR(tty)) - ctrl |= CH341_LCR_MARK_SPACE; + lcr |= CH341_LCR_MARK_SPACE; } if (C_CSTOPB(tty)) - ctrl |= CH341_LCR_STOP_BITS_2; + lcr |= CH341_LCR_STOP_BITS_2; if (baud_rate) { priv->baud_rate = baud_rate; - r = ch341_set_baudrate_lcr(port->serial->dev, priv, ctrl); + r = ch341_set_baudrate_lcr(port->serial->dev, priv, lcr); if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); } else if (r == 0) { - priv->lcr = ctrl; + priv->lcr = lcr; } } -- 2.10.2 -- 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 11/14] USB: serial: ch341: rename shadow modem-control register
Rename the shadow modem-control register currently named "line_control" to the less confusing "mcr". Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 84c10b63732b..cab7e2ca4e47 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -93,7 +93,7 @@ MODULE_DEVICE_TABLE(usb, id_table); struct ch341_private { spinlock_t lock; /* access lock */ unsigned baud_rate; /* set baud rate */ - u8 line_control; /* set line control value RTS/DTR */ + u8 mcr; u8 line_status; /* active status of modem control inputs */ u8 lcr; }; @@ -251,7 +251,7 @@ static int ch341_configure(struct usb_device *dev, struct ch341_private *priv) if (r < 0) goto out; - r = ch341_set_handshake(dev, priv->line_control); + r = ch341_set_handshake(dev, priv->mcr); out: kfree(buffer); return r; @@ -306,11 +306,11 @@ static void ch341_dtr_rts(struct usb_serial_port *port, int on) /* drop DTR and RTS */ spin_lock_irqsave(&priv->lock, flags); if (on) - priv->line_control |= CH341_BIT_RTS | CH341_BIT_DTR; + priv->mcr |= CH341_BIT_RTS | CH341_BIT_DTR; else - priv->line_control &= ~(CH341_BIT_RTS | CH341_BIT_DTR); + priv->mcr &= ~(CH341_BIT_RTS | CH341_BIT_DTR); spin_unlock_irqrestore(&priv->lock, flags); - ch341_set_handshake(port->serial->dev, priv->line_control); + ch341_set_handshake(port->serial->dev, priv->mcr); } static void ch341_close(struct usb_serial_port *port) @@ -415,12 +415,12 @@ static void ch341_set_termios(struct tty_struct *tty, spin_lock_irqsave(&priv->lock, flags); if (C_BAUD(tty) == B0) - priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS); + priv->mcr &= ~(CH341_BIT_DTR | CH341_BIT_RTS); else if (old_termios && (old_termios->c_cflag & CBAUD) == B0) - priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); + priv->mcr |= (CH341_BIT_DTR | CH341_BIT_RTS); spin_unlock_irqrestore(&priv->lock, flags); - ch341_set_handshake(port->serial->dev, priv->line_control); + ch341_set_handshake(port->serial->dev, priv->mcr); } static void ch341_break_ctl(struct tty_struct *tty, int break_state) @@ -476,14 +476,14 @@ static int ch341_tiocmset(struct tty_struct *tty, spin_lock_irqsave(&priv->lock, flags); if (set & TIOCM_RTS) - priv->line_control |= CH341_BIT_RTS; + priv->mcr |= CH341_BIT_RTS; if (set & TIOCM_DTR) - priv->line_control |= CH341_BIT_DTR; + priv->mcr |= CH341_BIT_DTR; if (clear & TIOCM_RTS) - priv->line_control &= ~CH341_BIT_RTS; + priv->mcr &= ~CH341_BIT_RTS; if (clear & TIOCM_DTR) - priv->line_control &= ~CH341_BIT_DTR; - control = priv->line_control; + priv->mcr &= ~CH341_BIT_DTR; + control = priv->mcr; spin_unlock_irqrestore(&priv->lock, flags); return ch341_set_handshake(port->serial->dev, control); @@ -577,7 +577,7 @@ static int ch341_tiocmget(struct tty_struct *tty) unsigned int result; spin_lock_irqsave(&priv->lock, flags); - mcr = priv->line_control; + mcr = priv->mcr; status = priv->line_status; spin_unlock_irqrestore(&priv->lock, flags); -- 2.10.2 -- 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 05/14] USB: serial: ch341: fix resume after reset
Fix reset-resume handling which failed to resubmit the read and interrupt URBs, thereby leaving a port that was open before suspend in a broken state until closed and reopened. Fixes: 1ded7ea47b88 ("USB: ch341 serial: fix port number changed after resume") Fixes: 2bfd1c96a9fb ("USB: serial: ch341: remove reset_resume callback") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 8f41d4385f1c..5343d65f3b52 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -582,14 +582,23 @@ static int ch341_tiocmget(struct tty_struct *tty) static int ch341_reset_resume(struct usb_serial *serial) { - struct ch341_private *priv; - - priv = usb_get_serial_port_data(serial->port[0]); + struct usb_serial_port *port = serial->port[0]; + struct ch341_private *priv = usb_get_serial_port_data(port); + int ret; /* reconfigure ch341 serial port after bus-reset */ ch341_configure(serial->dev, priv); - return 0; + if (tty_port_initialized(&port->port)) { + ret = usb_submit_urb(port->interrupt_in_urb, GFP_NOIO); + if (ret) { + dev_err(&port->dev, "failed to submit interrupt urb: %d\n", + ret); + return ret; + } + } + + return usb_serial_generic_resume(serial); } static struct usb_serial_driver ch341_device = { -- 2.10.2 -- 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 03/14] USB: serial: ch341: fix modem-control and B0 handling
The modem-control signals are managed by the tty-layer during open and should not be asserted prematurely when set_termios is called from driver open. Also make sure that the signals are asserted only when changing speed from B0. Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 6279df905c14..0cc5056b304d 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -384,10 +384,6 @@ static void ch341_set_termios(struct tty_struct *tty, ctrl |= CH341_LCR_STOP_BITS_2; if (baud_rate) { - spin_lock_irqsave(&priv->lock, flags); - priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); - spin_unlock_irqrestore(&priv->lock, flags); - priv->baud_rate = baud_rate; r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl); @@ -395,14 +391,16 @@ static void ch341_set_termios(struct tty_struct *tty, priv->baud_rate = tty_termios_baud_rate(old_termios); tty_termios_copy_hw(&tty->termios, old_termios); } - } else { - spin_lock_irqsave(&priv->lock, flags); - priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS); - spin_unlock_irqrestore(&priv->lock, flags); } - ch341_set_handshake(port->serial->dev, priv->line_control); + spin_lock_irqsave(&priv->lock, flags); + if (C_BAUD(tty) == B0) + priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS); + else if (old_termios && (old_termios->c_cflag & CBAUD) == B0) + priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); + spin_unlock_irqrestore(&priv->lock, flags); + ch341_set_handshake(port->serial->dev, priv->line_control); } static void ch341_break_ctl(struct tty_struct *tty, int break_state) -- 2.10.2 -- 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 02/14] USB: serial: ch341: fix open and resume after B0
The private baud_rate variable is used to configure the port at open and reset-resume and must never be set to (and left at) zero or reset-resume and all further open attempts will fail. Fixes: aa91def41a7b ("USB: ch341: set tty baud speed according to tty struct") Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS") Cc: stable Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index d133e72fe888..6279df905c14 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -355,7 +355,6 @@ static void ch341_set_termios(struct tty_struct *tty, baud_rate = tty_get_baud_rate(tty); - priv->baud_rate = baud_rate; ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; switch (C_CSIZE(tty)) { @@ -388,6 +387,9 @@ static void ch341_set_termios(struct tty_struct *tty, spin_lock_irqsave(&priv->lock, flags); priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); spin_unlock_irqrestore(&priv->lock, flags); + + priv->baud_rate = baud_rate; + r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl); if (r < 0 && old_termios) { priv->baud_rate = tty_termios_baud_rate(old_termios); -- 2.10.2 -- 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 12/14] USB: serial: ch341: rename modem-status register
Rename the shadow modem-status register currently named "line_status" to the less confusing "msr". Also rename the helper function used to parse the interrupt data. Signed-off-by: Johan Hovold --- drivers/usb/serial/ch341.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index cab7e2ca4e47..2981d1934874 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -94,7 +94,7 @@ struct ch341_private { spinlock_t lock; /* access lock */ unsigned baud_rate; /* set baud rate */ u8 mcr; - u8 line_status; /* active status of modem control inputs */ + u8 msr; u8 lcr; }; @@ -209,7 +209,7 @@ static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv) goto out; spin_lock_irqsave(&priv->lock, flags); - priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT; + priv->msr = (~(*buffer)) & CH341_BITS_MODEM_STAT; spin_unlock_irqrestore(&priv->lock, flags); out: kfree(buffer); @@ -293,7 +293,7 @@ static int ch341_port_remove(struct usb_serial_port *port) static int ch341_carrier_raised(struct usb_serial_port *port) { struct ch341_private *priv = usb_get_serial_port_data(port); - if (priv->line_status & CH341_BIT_DCD) + if (priv->msr & CH341_BIT_DCD) return 1; return 0; } @@ -489,7 +489,7 @@ static int ch341_tiocmset(struct tty_struct *tty, return ch341_set_handshake(port->serial->dev, control); } -static void ch341_update_line_status(struct usb_serial_port *port, +static void ch341_update_status(struct usb_serial_port *port, unsigned char *data, size_t len) { struct ch341_private *priv = usb_get_serial_port_data(port); @@ -504,8 +504,8 @@ static void ch341_update_line_status(struct usb_serial_port *port, status = ~data[2] & CH341_BITS_MODEM_STAT; spin_lock_irqsave(&priv->lock, flags); - delta = status ^ priv->line_status; - priv->line_status = status; + delta = status ^ priv->msr; + priv->msr = status; spin_unlock_irqrestore(&priv->lock, flags); if (data[1] & CH341_MULT_STAT) @@ -558,7 +558,7 @@ static void ch341_read_int_callback(struct urb *urb) } usb_serial_debug_data(&port->dev, __func__, len, data); - ch341_update_line_status(port, data, len); + ch341_update_status(port, data, len); exit: status = usb_submit_urb(urb, GFP_ATOMIC); if (status) { @@ -578,7 +578,7 @@ static int ch341_tiocmget(struct tty_struct *tty) spin_lock_irqsave(&priv->lock, flags); mcr = priv->mcr; - status = priv->line_status; + status = priv->msr; spin_unlock_irqrestore(&priv->lock, flags); result = ((mcr & CH341_BIT_DTR) ? TIOCM_DTR : 0) -- 2.10.2 -- 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: musb: dsps: switch to static id for musb-hdrc platform devices
The dsps glue uses PLATFORM_DEVID_AUTO when creating the musb-hdrc platform devices, this causes that the id will change in each system depending on the order of driver probe, the order of the usb instances defined in device-tree, or the list of enabled devices which use also PLATFORM_DEVID_AUTO in kernel config. This id inconsistency causes trouble in shell scripting or user guide documentation. So switch it to static id, starting from 0 to the musb instance with lower MMR offset. This scheme is also aligned to the naming in the SoC. Signed-off-by: Bin Liu --- drivers/usb/musb/musb_dsps.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index c722daddfb67..f7b3cb1aea6b 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -683,7 +683,8 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, resources[1] = *res; /* allocate the child platform device */ - musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO); + musb = platform_device_alloc("musb-hdrc", + (resources[0].start & 0xFFF) == 0x400 ? 0 : 1); if (!musb) { dev_err(dev, "failed to allocate musb device\n"); return -ENOMEM; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/6] usb: separate out sysdev pointer from usb_bus
On 12/02/2016 06:19 PM, Brian Norris wrote: > Hi all, > > On Thu, Nov 17, 2016 at 05:13:43PM +0530, Sriram Dash wrote: >> From: Arnd Bergmann >> >> 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. >> >> 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 >> Tested-by: Baolin Wang >> 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 v5: >> - No update >> >> Changes in v4: >> - No update >> >> Changes in v3: >> - usb is_device_dma_capable instead of directly accessing >> dma props. >> >> Changes in v2: >> - Split the patch wrt driver > > I didn't notice this series had been reposted a few times. For some > reason, this wasn't that easy to find in search engines... Anyway, when > the whole series is applied, this fixes my XHCI probe issues for DWC3 > host mode. Thanks! > > Tested-by: Brian Norris > > But I noticed that Felipe has applied patches 5 and 6 in -next, while > the rest are still outstanding. That means I hit the dma_mask WARN_ON() > in xhci-plat.c, and it eventually fails to probe with -EIO still: > > [2.060272] [ cut here ] > [2.064908] WARNING: CPU: 5 PID: 1 at drivers/usb/host/xhci-plat.c:159 > xhci_plat_probe+0x5c/0x444 > ... > [2.25] [] xhci_plat_probe+0x5c/0x444 > [2.294456] [] platform_drv_probe+0x60/0xac > [2.300200] [] driver_probe_device+0x12c/0x2a0 > [2.306204] [] __driver_attach+0x84/0xb0 > [2.311687] [] bus_for_each_dev+0x9c/0xcc > [2.317256] [] driver_attach+0x2c/0x34 > [2.322566] [] bus_add_driver+0xf0/0x1f4 > [2.328049] [] driver_register+0x9c/0xe8 > [2.333530] [] __platform_driver_register+0x60/0x6c > [2.339968] [] xhci_plat_init+0x2c/0x34 > [2.345366] [] do_one_initcall+0xa4/0x13c > [2.350936] [] kernel_init_freeable+0x1bc/0x274 > [2.357026] [] kernel_init+0x18/0x104 > [2.362247] [] ret_from_fork+0x10/0x50 > [2.374615] xhci-hcd: probe of xhci-hcd.1.auto failed with error -5 > [2.380962] [ cut here ] > [2.385588] WARNING: CPU: 4 PID: 1 at drivers/usb/host/xhci-plat.c:159 > xhci_plat_probe+0x5c/0x444 > ... > [2.637372] [] xhci_plat_probe+0x5c/0x444 > [2.642941] [] platform_drv_probe+0x60/0xac > [2.648685] [] driver_probe_device+0x12c/0x2a0 > [2.654688] [] __driver_attach+0x84/0xb0 > [2.660170] [] bus_for_each_dev+0x9c/0xcc > [2.665739] [] driver_attach+0x2c/0x34 > [2.671048] [] bus_add_driver+0xf0/0x1f4 > [2.676532] [] driver_register+0x9c/0xe8 > [2.682012] [] __platform_driver_register+0x60/0x6c > [2.688450] [] xhci_plat_init+0x2c/0x34 > [2.693845] [] do_one_initcall+0xa4/0x13c > [2.699415] [] kernel_init_freeable+0x1bc/0x274 > [2.705505] [] kernel_init+0x18/0x104 > [2.710726] [] ret_from_fork+0x10/0x50 > [2.716075] xhci-hcd: probe of xhci-hcd.2.auto failed with error -5 > > What's happening with patches 1-4? > I can observe this warning also with 4.10-rc2 11.709204] [ cut here ] [ 11.709218] WARNING: CPU: 0 PID: 163 at drivers/usb/host/xhci-plat.c:168 xhci_plat_probe+0x180/0x450 [xhci_plat_hcd] [ 11.709220] Modules linked in: xhci_plat_hcd(+) xhci_hcd usbcore dwc3 udc_core usb_common evdev snd_soc_simple_card snd_soc_simple_card_utils omapfb cfbfillrect cfbimgblt encoder_tpd12s015 connector_hdmi cfbcopyarea leds_gpi4 [ 11.709308] CPU: 0 PID: 163 Comm: systemd-udevd Not tainted 4.10.0-rc2-00328-g0eeded4-dirty #124 [ 11.709310] Hardware name: Generic DRA74X (Flattened Device Tree) [ 11.709326] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 11.709334] [] (show_stack) from [] (dump_stack+0xac/0xe0) [ 11.709345] [] (dump_stack) from [] (__warn+0xd8/0x104) [ 11.709354] [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [ 11.709365] [] (warn_slowpath_null) from [] (xhci_plat_probe+0x180/0x450 [xhci_plat_hcd]) [ 11.709401] [] (xhci_plat_probe [xhci_plat_hcd]) from [] (platform_drv_probe+0x4c/0xb0) [ 11.709409] [] (platform_drv_probe) from [] (driver_probe_device+0x200/0x2d4) [ 11.709418] []
Re: [PATCH] usb: musb: dsps: switch to static id for musb-hdrc platform devices
On Fri, Jan 06, 2017 at 01:29:49PM -0600, Bin Liu wrote: > The dsps glue uses PLATFORM_DEVID_AUTO when creating the musb-hdrc > platform devices, this causes that the id will change in each system > depending on the order of driver probe, the order of the usb instances > defined in device-tree, or the list of enabled devices which use also > PLATFORM_DEVID_AUTO in kernel config. This id inconsistency causes > trouble in shell scripting or user guide documentation. The discussion about this issue is in [1]. [1] http://marc.info/?t=14400914521&r=1&w=2 Regards, -Bin. > > So switch it to static id, starting from 0 to the musb instance with > lower MMR offset. This scheme is also aligned to the naming in the SoC. > > Signed-off-by: Bin Liu > --- > drivers/usb/musb/musb_dsps.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > index c722daddfb67..f7b3cb1aea6b 100644 > --- a/drivers/usb/musb/musb_dsps.c > +++ b/drivers/usb/musb/musb_dsps.c > @@ -683,7 +683,8 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, > resources[1] = *res; > > /* allocate the child platform device */ > - musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO); > + musb = platform_device_alloc("musb-hdrc", > + (resources[0].start & 0xFFF) == 0x400 ? 0 : 1); > if (!musb) { > dev_err(dev, "failed to allocate musb device\n"); > return -ENOMEM; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] dmaengine: cppi41: Fix list not empty warning on runtime suspend
On Wed, Jan 4, 2017 at 6:03 PM, Alexandre Bailon wrote: > Sometime, a transfer may not be queued due to a race between runtime pm > and cppi41_dma_issue_pending(). > Sometime, cppi41_runtime_resume() may be interrupted right before to > update device PM state to RESUMED. > When it happens, if a new dma transfer is issued, because the device is not > in active state, the descriptor will be added to the pendding list. > But because the descriptors in the pendding list are only queued to cppi41 > on runtime resume, the descriptor will not be queued. > On runtime suspend, the list is not empty, which is causing a warning. > Queue the descriptor if the device is active or resuming. > - if (likely(pm_runtime_active(cdd->ddev.dev))) > + active = pm_runtime_active(cdd->ddev.dev); > + if (!active) { > + /* > +* Runtime resume may be interrupted before runtime_status > +* has been updated. Test if device has resumed. > +*/ > + if (error == -EINPROGRESS) { > + spin_lock_irqsave(&cdd->lock, flags); > + if (list_empty(&cdd->pending)) > + active = true; active = !!list_empty(); > + spin_unlock_irqrestore(&cdd->lock, flags); > + } > + } > + > + if (likely(active)) > push_desc_queue(c); > else > pending_desc(c); -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: fix panic for coherent memory allocation
On Thu, Jan 5, 2017 at 6:50 PM, Leo Yan wrote: > When use configfs to configure USB port as as ethernet gadget, the > kernel has panic with below backtrace; it clearly indicates the > coherent memory allocation is happened in the interrupt context, > but the function __get_vm_area_node() is possible to be scheduling > out so function __get_vm_area_node() triggers panic if it's called > from interrupt context. > > To fix this issue, simply to change gfp_flag from GFP_KERNEL to > GFP_NOWAIT so it can support to allocate coherent memory from > interrupt context. This looks like it duplicates the similar patch I'm told should be queued for 4.10-rc3: https://lkml.org/lkml/2016/12/1/120 thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: use u32 for DT binding parameters
On Fri, Jan 6, 2017 at 4:45 AM, Leo Yan wrote: > Commit 05ee799f2021 ("usb: dwc2: Move gadget settings into core_params") > changes to type u16 for DT binding "g-rx-fifo-size" and > "g-np-tx-fifo-size" but use type u32 for "g-tx-fifo-size". Finally the > the first two parameters cannot be passed successfully with wrong data > format. This is found the data transferring broken on 96boards Hikey. > > This patch is to change all parameters to u32 type, and verified on > Hikey board the DT parameters can pass successfully. > > Signed-off-by: Leo Yan Nice! This patch (while it doesn't apply cleanly to v4.10-rc2) does resolve the regression I reported earlier here: https://www.spinics.net/lists/linux-usb/msg150766.html I didn't see the slight shift to u16s there. Leo, does the patch need a respin, or is it against Linus' HEAD and my tree is stale? Anyway, after re-applying it to my tree: Tested-by: John Stultz thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: gadget: Fix DMA memory freeing
On Thu, Jan 5, 2017 at 6:01 PM, John Youn wrote: > From: Vardan Mikayelyan > > Remove DMA memory free from EP disable flow by replacing > dma_alloc_coherent with dmam_alloc_coherent. > > Cc: John Stultz > Signed-off-by: Vardan Mikayelyan > Signed-off-by: John Youn > --- > > Hi John, > > Can you see if this fixes the issue on your platform? Seems to work great for me! Thanks so much for sorting this out! Tested-by: John Stultz thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] MIPS: OCTEON: Platform support for DWC3 USB controller.
Add all the necessary platform code to initialize the dwc3 USB host controller found on OCTEON III processors. This code initializes the clocks and resets the USB core with PHYs. It is then passed off to the platform independent DWC3 driver found in the 'drivers/usb/dwc3' directory. Based off code written by David Daney. Signed-off-by: Steven J. Hill --- arch/mips/cavium-octeon/Makefile | 1 + arch/mips/cavium-octeon/octeon-platform.c | 1 + arch/mips/cavium-octeon/octeon-usb.c | 555 ++ arch/mips/include/asm/octeon/cvmx-gpio-defs.h | 8 +- 4 files changed, 563 insertions(+), 2 deletions(-) create mode 100644 arch/mips/cavium-octeon/octeon-usb.c diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile index 2a59265..a3d6ad8 100644 --- a/arch/mips/cavium-octeon/Makefile +++ b/arch/mips/cavium-octeon/Makefile @@ -18,3 +18,4 @@ obj-y += crypto/ obj-$(CONFIG_MTD)+= flash_setup.o obj-$(CONFIG_SMP)+= smp.o obj-$(CONFIG_OCTEON_ILM) += oct_ilm.o +obj-$(CONFIG_USB) += octeon-usb.o diff --git a/arch/mips/cavium-octeon/octeon-platform.c b/arch/mips/cavium-octeon/octeon-platform.c index 37a932d..16083cf 100644 --- a/arch/mips/cavium-octeon/octeon-platform.c +++ b/arch/mips/cavium-octeon/octeon-platform.c @@ -448,6 +448,7 @@ static int __init octeon_ohci_device_init(void) { .compatible = "cavium,octeon-3860-bootbus", }, { .compatible = "cavium,mdio-mux", }, { .compatible = "gpio-leds", }, + { .compatible = "cavium,octeon-7130-usb-uctl", }, {}, }; diff --git a/arch/mips/cavium-octeon/octeon-usb.c b/arch/mips/cavium-octeon/octeon-usb.c new file mode 100644 index 000..773272d --- /dev/null +++ b/arch/mips/cavium-octeon/octeon-usb.c @@ -0,0 +1,555 @@ +/* + * XHCI HCD glue for Cavium Octeon III SOCs. + * + * Copyright (C) 2010-2017 Cavium Networks + * + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + */ + +#include +#include +#include +#include +#include + +#include +#include + +/* USB Control Register */ +union cvm_usbdrd_uctl_ctl { + uint64_t u64; + struct cvm_usbdrd_uctl_ctl_s { + /* 1 = BIST and set all USB RAMs to 0x0, 0 = BIST */ + __BITFIELD_FIELD(uint64_t clear_bist:1, + /* 1 = Start BIST and cleared by hardware */ + __BITFIELD_FIELD(uint64_t start_bist:1, + /* Reference clock select for SuperSpeed and HighSpeed PLLs: +* 0x0 = Both PLLs use DLMC_REF_CLK0 for reference clock +* 0x1 = Both PLLs use DLMC_REF_CLK1 for reference clock +* 0x2 = SuperSpeed PLL uses DLMC_REF_CLK0 for reference clock & +*HighSpeed PLL uses PLL_REF_CLK for reference clck +* 0x3 = SuperSpeed PLL uses DLMC_REF_CLK1 for reference clock & +*HighSpeed PLL uses PLL_REF_CLK for reference clck +*/ + __BITFIELD_FIELD(uint64_t ref_clk_sel:2, + /* 1 = Spread-spectrum clock enable, 0 = SS clock disable */ + __BITFIELD_FIELD(uint64_t ssc_en:1, + /* Spread-spectrum clock modulation range: +* 0x0 = -4980 ppm downspread +* 0x1 = -4492 ppm downspread +* 0x2 = -4003 ppm downspread +* 0x3 - 0x7 = Reserved +*/ + __BITFIELD_FIELD(uint64_t ssc_range:3, + /* Enable non-standard oscillator frequencies: +* [55:53] = modules -1 +* [52:47] = 2's complement push amount, 0 = Feature disabled +*/ + __BITFIELD_FIELD(uint64_t ssc_ref_clk_sel:9, + /* Reference clock multiplier for non-standard frequencies: +* 0x19 = 100MHz on DLMC_REF_CLK* if REF_CLK_SEL = 0x0 or 0x1 +* 0x28 = 125MHz on DLMC_REF_CLK* if REF_CLK_SEL = 0x0 or 0x1 +* 0x32 = 50MHz on DLMC_REF_CLK* if REF_CLK_SEL = 0x0 or 0x1 +* Other Values = Reserved +*/ + __BITFIELD_FIELD(uint64_t mpll_multiplier:7, + /* Enable reference clock to prescaler for SuperSpeed functionality. +* Should always be set to "1" +*/ + __BITFIELD_FIELD(uint64_t ref_ssp_en:1, + /* Divide the reference clock by 2 before entering the +* REF_CLK_FSEL divider: +* If REF_CLK_SEL = 0x0 or 0x1, then only 0x0 is legal +* If REF_CLK_SEL = 0x2 or 0x3, then: +* 0x1 = DLMC_REF_CLK* is 125MHz +* 0x0 = DLMC_REF_CLK* is another supported frequency +*/ + __BITFIELD_FIELD(uint64_t ref_clk_div2:1, + /* Select reference clock freqnuency for both PLL blocks: +* 0x27 = REF_CLK_SEL is 0x0 or 0x1 +* 0x07 = REF_CLK_SEL is 0x2 or 0x3 +*/ + __BITFIELD_FIELD(uint64_t ref_clk_fsel:6, + /* Reserved */ + __BITFIELD_FIELD
Re: [PATCH] MIPS: OCTEON: Platform support for DWC3 USB controller.
Hi, On Fri, Jan 06, 2017 at 05:42:24PM -0600, Steven J. Hill wrote: > Add all the necessary platform code to initialize the dwc3 > USB host controller found on OCTEON III processors. This > code initializes the clocks and resets the USB core with > PHYs. It is then passed off to the platform independent > DWC3 driver found in the 'drivers/usb/dwc3' directory. > Based off code written by David Daney. > > Signed-off-by: Steven J. Hill [...] > +/* USB Control Register */ > +union cvm_usbdrd_uctl_ctl { > + uint64_t u64; > + struct cvm_usbdrd_uctl_ctl_s { > + /* 1 = BIST and set all USB RAMs to 0x0, 0 = BIST */ > + __BITFIELD_FIELD(uint64_t clear_bist:1, > + /* 1 = Start BIST and cleared by hardware */ > + __BITFIELD_FIELD(uint64_t start_bist:1, > + /* Reference clock select for SuperSpeed and HighSpeed PLLs: > + * 0x0 = Both PLLs use DLMC_REF_CLK0 for reference clock > + * 0x1 = Both PLLs use DLMC_REF_CLK1 for reference clock > + * 0x2 = SuperSpeed PLL uses DLMC_REF_CLK0 for reference clock & > + *HighSpeed PLL uses PLL_REF_CLK for reference clck > + * 0x3 = SuperSpeed PLL uses DLMC_REF_CLK1 for reference clock & > + *HighSpeed PLL uses PLL_REF_CLK for reference clck > + */ Maybe use kernel-doc comment style to document the fields? > +typedef union cvm_usbdrd_uctl_ctl cvm_usbdrd_uctl_ctl_t; [...] > +typedef union cvm_usbdrd_uctl_host_cfg cvm_usbdrd_uctl_host_cfg_t; [...] > +typedef union cvm_usbdrd_uctl_shim_cfg cvm_usbdrd_uctl_shim_cfg_t; These typedefs are not used, so they should not be added. A. -- 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 v5 2/2] USB: usb timing value debug
add debugfs support for experimenting with USB timing delay values on the fly. Values are read/written from debugfs at /sys/kernel/debug/usb/timing. Signed-off-by: Todd Brandt --- v2: - moved the debug code from hub.c to usb.c - use debugfs instead of /sys/kernel/usb v5: - allow setting tdrstr drivers/usb/core/usb.c | 58 ++ 1 file changed, 58 insertions(+) diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index ac31263..bef73c2 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -39,6 +39,8 @@ #include #include #include +#include +#include #include #include @@ -1036,6 +1038,52 @@ struct dentry *usb_debug_root; EXPORT_SYMBOL_GPL(usb_debug_root); static struct dentry *usb_debug_devices; +static struct dentry *usb_debug_timing; + +static int usb_timing_show(struct seq_file *s, void *unused) +{ + seq_printf(s, "tdrsmdn=%d\n", usb_timing.tdrsmdn); + seq_printf(s, "trsmrcy=%d\n", usb_timing.trsmrcy); + seq_printf(s, "trstrcy=%d\n", usb_timing.trstrcy); + seq_printf(s, "tdrstr=%d\n", usb_timing.tdrstr); + return 0; +} + +static int usb_timing_open(struct inode *inode, struct file *file) +{ + return single_open(file, usb_timing_show, inode->i_private); +} + +static ssize_t usb_timing_write(struct file *file, + const char __user *ubuf, size_t count, loff_t *ppos) +{ + int val; + char buf[32]; + + if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) + return -EFAULT; + + if (sscanf(buf, "tdrsmdn=%u", &val) == 1) + usb_timing.tdrsmdn = max(val, USB_TIMING_TDRSMDN_MIN); + else if (sscanf(buf, "trsmrcy=%u", &val) == 1) + usb_timing.trsmrcy = max(val, USB_TIMING_TRSMRCY_MIN); + else if (sscanf(buf, "trstrcy=%u", &val) == 1) + usb_timing.trstrcy = max(val, USB_TIMING_TRSTRCY_MIN); + else if (sscanf(buf, "tdrstr=%u", &val) == 1) + usb_timing.tdrstr = max(val, USB_TIMING_TDRSTR_MIN); + else + return -EINVAL; + + return count; +} + +static const struct file_operations usbfs_timing_fops = { + .open = usb_timing_open, + .write= usb_timing_write, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; static int usb_debugfs_init(void) { @@ -1052,11 +1100,21 @@ static int usb_debugfs_init(void) return -ENOENT; } + usb_debug_timing = debugfs_create_file("timing", 0644, + usb_debug_root, NULL, + &usbfs_timing_fops); + if (!usb_debug_timing) { + debugfs_remove(usb_debug_root); + usb_debug_root = NULL; + return -ENOENT; + } + return 0; } static void usb_debugfs_cleanup(void) { + debugfs_remove(usb_debug_timing); debugfs_remove(usb_debug_devices); debugfs_remove(usb_debug_root); } -- 2.1.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 v5 1/2] USB: add switch to turn off padding of resume time delays
Add a kernel parameter that replaces the USB_RESUME_TIMEOUT and other hardcoded delay numbers with the USB spec minimums. The USB subsystem currently uses some padded values for USB spec timing delays. This patch keeps the current values by default, but if the kernel is booted with usbcore.timing_minimum=1 they are set to the spec minimums with no padding. The result is significant performance improvement in usb device resume. - tdrsmdn: resume signal time (min=20ms, cur=40ms) usb2.0 spec 7.1.7.7 - trsmrcy: resume recovery time (min=10ms, cur=10ms) usb2.0 spec 7.1.7.7 - trstrcy: reset recovery time (min=0ms, cur=50ms) usb2.0 spec 7.1.7.5 - tdrstr: root port reset time (min=50ms, cur=50ms) usb2.0 spec 7.1.7.5 Example analyze_suspend runs are provided here showing the benefits: https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays Signed-off-by: Todd Brandt --- v2: - moved the core code from hub.c to usb.c - param name is now usb_timing_minimum - configured isp1362-hcd and ohci-hub to use the new values v3: - changed param to usbcore.timing_minimum v4: - moved usb_timing object to usb-common v5: - added tdrstr and used it in uhci-hub Documentation/admin-guide/kernel-parameters.txt | 7 +++ drivers/usb/common/common.c | 8 drivers/usb/core/hub.c | 12 ++-- drivers/usb/core/usb.c | 10 ++ drivers/usb/dwc2/hcd.c | 2 +- drivers/usb/host/ehci-hcd.c | 4 ++-- drivers/usb/host/ehci-hub.c | 6 +++--- drivers/usb/host/fotg210-hcd.c | 2 +- drivers/usb/host/isp116x-hcd.c | 2 +- drivers/usb/host/isp1362-hcd.c | 2 +- drivers/usb/host/ohci-hub.c | 2 +- drivers/usb/host/oxu210hp-hcd.c | 4 ++-- drivers/usb/host/r8a66597-hcd.c | 2 +- drivers/usb/host/sl811-hcd.c| 2 +- drivers/usb/host/uhci-hub.c | 6 +++--- drivers/usb/host/xhci-hub.c | 6 +++--- drivers/usb/host/xhci-ring.c| 2 +- drivers/usb/isp1760/isp1760-hcd.c | 2 +- drivers/usb/musb/musb_core.c| 6 +++--- drivers/usb/musb/musb_virthub.c | 2 +- include/linux/usb.h | 26 - 21 files changed, 82 insertions(+), 33 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index be7c0d9..0810302 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4039,6 +4039,13 @@ unknown_nmi_panic [X86] Cause panic on unknown NMI. + usbcore.timing_minimum= + Set USB timing values to USB 2.0 spec minimums (default 0 = off). + This removes any padding added to the values to accommodate + older or troublesome hardware. This will reduce usb resume + time. Use this only if you know your USB hardware and device + setup can handle the spec minimums. + usbcore.authorized_default= [USB] Default USB device authorization: (default -1 = authorized except for wireless USB, diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 5ef8da6..b288f51 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -19,6 +19,14 @@ #include #include +struct usb_timing_config usb_timing = { + .tdrsmdn = USB_TIMING_TDRSMDN_DEF, + .trsmrcy = USB_TIMING_TRSMRCY_DEF, + .trstrcy = USB_TIMING_TRSTRCY_DEF, + .tdrstr = USB_TIMING_TDRSTR_DEF +}; +EXPORT_SYMBOL_GPL(usb_timing); + const char *usb_otg_state_string(enum usb_otg_state state) { static const char *const names[] = { diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 1fa5c0f..ffb8cfc 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2841,7 +2841,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1, done: if (status == 0) { /* TRSTRCY = 10 ms; plus some extra */ - msleep(10 + 40); + msleep(usb_timing.trstrcy); if (udev) { struct usb_hcd *hcd = bus_to_hcd(udev->bus); @@ -3433,10 +3433,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) if (status) { dev_dbg(&port_dev->dev, "can't resume, status %d\n", status); } else { - /* drive resume for USB_RESUME_TIMEOUT msec */ + /* drive resume for TDRSMDN msec */ dev_dbg(&udev->dev, "usb %sresume\n",
[PATCH v5 0/2] USB: resume time optimization by using spec minimums
The USB resume code in the kernel currently uses a set of hard coded delay values that are defined in the USB 2.0 spec. These are the most important ones: - tdrsmdn: resume signal time (20ms - infinity) usb 2.0 spec 7.1.7.7 - trsmrcy: resume recovery time (10ms) usb 2.0 spec 7.1.7.7 - trstrcy: reset recovery time (0ms - infinity) usb 2.0 spec 7.1.7.5 - tdrstr: root port reset time (50ms) usb2.0 spec 7.1.7.5 Some of these values have been padded considerably in order to accomodate non-compliant devices. - tdrsmdn: resume signal time = 40ms - trstrcy: reset recovery time = 50ms I propose that we provide a kernel parameter that sets the USB timing values to their spec minimums. The following patches do this by creating a global struct which contains these values. By default the values remain as they are now, but if usbcore.timing_minimum=1 is added to the kernel cmd line they're set to their minimums. This struct can be expanded over time to include other hardcoded values that have padding we can remove. The patch is also useful in padding delay values even further for really temperamental devices. I've created a blog entry on 01.org with some analyze_suspend test cases illustrating the benefits: - https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays Todd Brandt (2): USB: add switch to turn off padding of resume time delays USB: usb timing value debug Documentation/admin-guide/kernel-parameters.txt | 7 +++ drivers/usb/common/common.c | 8 +++ drivers/usb/core/hub.c | 12 ++--- drivers/usb/core/usb.c | 68 + drivers/usb/dwc2/hcd.c | 2 +- drivers/usb/host/ehci-hcd.c | 4 +- drivers/usb/host/ehci-hub.c | 6 +-- drivers/usb/host/fotg210-hcd.c | 2 +- drivers/usb/host/isp116x-hcd.c | 2 +- drivers/usb/host/isp1362-hcd.c | 2 +- drivers/usb/host/ohci-hub.c | 2 +- drivers/usb/host/oxu210hp-hcd.c | 4 +- drivers/usb/host/r8a66597-hcd.c | 2 +- drivers/usb/host/sl811-hcd.c| 2 +- drivers/usb/host/uhci-hub.c | 6 +-- drivers/usb/host/xhci-hub.c | 6 +-- drivers/usb/host/xhci-ring.c| 2 +- drivers/usb/isp1760/isp1760-hcd.c | 2 +- drivers/usb/musb/musb_core.c| 6 +-- drivers/usb/musb/musb_virthub.c | 2 +- include/linux/usb.h | 26 +- 21 files changed, 140 insertions(+), 33 deletions(-) -- 2.1.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
kernel parameter »usb-storage.quirks=....:....:p« leads to »end_request: critical target error...«
Dear reader, I'm using a Seagate Dockstar with Debian jessie kernel 3.16 and an usb-to-pata bridge from prolific, usb device id 067b:3507. On every boot, the kernel is saying »[5.058082] usb 1-1.4: new high-speed USB device number 3 using orion-ehci [5.291227] usb 1-1.4: New USB device found, idVendor=067b, idProduct=3507 [5.298168] usb 1-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [5.305519] usb 1-1.4: Product: ATAPI-6 Bridge Controller [5.310955] usb 1-1.4: Manufacturer: Prolific Technology Inc. [5.316743] usb 1-1.4: SerialNumber: 2E38 [5.336934] SCSI subsystem initialized [5.345501] usb-storage 1-1.4:1.0: USB Mass Storage device detected [5.352094] usb-storage 1-1.4:1.0: Quirks match for vid 067b pid 3507: 110 [5.359097] scsi0 : usb-storage 1-1.4:1.0 [5.364778] usbcore: registered new interface driver usb-storage [6.363192] scsi 0:0:0:0: Direct-Access SAMSUNG HD400LD WQ10 PQ: 0 ANSI: 0 [6.385758] sd 0:0:0:0: [sda] Adjusting the sector count from its reported value: 781420655 [6.394194] sd 0:0:0:0: [sda] 781420654 512-byte logical blocks: (400 GB/372 GiB) [6.402620] sd 0:0:0:0: [sda] Write Protect is off [6.407456] sd 0:0:0:0: [sda] Mode Sense: 03 00 00 00 [6.408367] sd 0:0:0:0: [sda] No Caching mode page found [6.413724] sd 0:0:0:0: [sda] Assuming drive cache: write through« Look at the last two lines above: »Assuming drive cache: write through« I tought would be definitively wrong, because the hdd behind the usb-pata-bridge has possibly a write-back cache. Ok, I never had problems with this "misconfiguration", but the computer has not been crashed often and the filesystems are not heavy in use... So I tried this kernel parameter: »usb-storage.quirks=067b:3507:p« then it looked better: »[6.403648] sd 0:0:0:0: [sda] No Caching mode page found [6.409003] sd 0:0:0:0: [sda] Assuming drive cache: write back« but a few seconds later: » 23.767645] end_request: critical target error, dev sda, sector 0 [ 23.776883] end_request: critical target error, dev sda, sector 0 [ 32.951535] end_request: critical target error, dev sda, sector 2103232 [ 32.958275] Aborting journal on device sda1-8. [ 33.179021] EXT4-fs error (device sda1): ext4_journal_check_start:56: Detected aborted journal [ 33.187827] EXT4-fs (sda1): Remounting filesystem read-only [ 34.545395] end_request: critical target error, dev sda, sector 197937613 [ 34.552309] Aborting journal on device sda2-8. [ 36.897247] EXT4-fs error (device sda2): ext4_journal_check_start:56: Detected aborted journal [ 36.906044] EXT4-fs (sda2): Remounting filesystem read-only [ 36.912845] EXT4-fs error (device sda2): ext4_journal_check_start:56: Detected aborted journal « So this was no good idea - but why is setting the usb-storage to "write back mode" no good? Kind regards, Jochen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: gadget: Fix DMA memory freeing
On 1/6/2017 3:10 PM, John Stultz wrote: > On Thu, Jan 5, 2017 at 6:01 PM, John Youn wrote: >> From: Vardan Mikayelyan >> >> Remove DMA memory free from EP disable flow by replacing >> dma_alloc_coherent with dmam_alloc_coherent. >> >> Cc: John Stultz >> Signed-off-by: Vardan Mikayelyan >> Signed-off-by: John Youn >> --- >> >> Hi John, >> >> Can you see if this fixes the issue on your platform? > > > Seems to work great for me! Thanks so much for sorting this out! Great. > > Tested-by: John Stultz > Felipe, Can you take this for fixes? Thanks, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: use u32 for DT binding parameters
From: Leo Yan Commit 05ee799f2021 ("usb: dwc2: Move gadget settings into core_params") changes to type u16 for DT binding "g-rx-fifo-size" and "g-np-tx-fifo-size" but use type u32 for "g-tx-fifo-size". Finally the the first two parameters cannot be passed successfully with wrong data format. This is found the data transferring broken on 96boards Hikey. This patch is to change all parameters to u32 type, and verified on Hikey board the DT parameters can pass successfully. [johnyoun: minor rebase] Signed-off-by: Leo Yan Signed-off-by: John Youn Tested-by: John Stultz --- drivers/usb/dwc2/core.h | 4 ++-- drivers/usb/dwc2/params.c | 10 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 9548d3e03453..302b8f5f7d27 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -513,8 +513,8 @@ struct dwc2_core_params { /* Gadget parameters */ bool g_dma; bool g_dma_desc; - u16 g_rx_fifo_size; - u16 g_np_tx_fifo_size; + u32 g_rx_fifo_size; + u32 g_np_tx_fifo_size; u32 g_tx_fifo_size[MAX_EPS_CHANNELS]; }; diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index 11fe68a4627b..bcd1e19b4076 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -385,16 +385,16 @@ static void dwc2_set_param(struct dwc2_hsotg *hsotg, void *param, } /** - * dwc2_set_param_u16() - Set a u16 parameter + * dwc2_set_param_u32() - Set a u32 parameter * * See dwc2_set_param(). */ -static void dwc2_set_param_u16(struct dwc2_hsotg *hsotg, u16 *param, +static void dwc2_set_param_u32(struct dwc2_hsotg *hsotg, u32 *param, bool lookup, char *property, u16 legacy, u16 def, u16 min, u16 max) { dwc2_set_param(hsotg, param, lookup, property, - legacy, def, min, max, 2); + legacy, def, min, max, 4); } /** @@ -1178,12 +1178,12 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg, * auto-detect if the hardware does not support the * default. */ - dwc2_set_param_u16(hsotg, &p->g_rx_fifo_size, + dwc2_set_param_u32(hsotg, &p->g_rx_fifo_size, true, "g-rx-fifo-size", 2048, hw->rx_fifo_size, 16, hw->rx_fifo_size); - dwc2_set_param_u16(hsotg, &p->g_np_tx_fifo_size, + dwc2_set_param_u32(hsotg, &p->g_np_tx_fifo_size, true, "g-np-tx-fifo-size", 1024, hw->dev_nperio_tx_fifo_size, 16, hw->dev_nperio_tx_fifo_size); -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: use u32 for DT binding parameters
On 1/6/2017 1:52 PM, John Stultz wrote: > On Fri, Jan 6, 2017 at 4:45 AM, Leo Yan wrote: >> Commit 05ee799f2021 ("usb: dwc2: Move gadget settings into core_params") >> changes to type u16 for DT binding "g-rx-fifo-size" and >> "g-np-tx-fifo-size" but use type u32 for "g-tx-fifo-size". Finally the >> the first two parameters cannot be passed successfully with wrong data >> format. This is found the data transferring broken on 96boards Hikey. >> >> This patch is to change all parameters to u32 type, and verified on >> Hikey board the DT parameters can pass successfully. >> >> Signed-off-by: Leo Yan > > Nice! > > This patch (while it doesn't apply cleanly to v4.10-rc2) does resolve > the regression I reported earlier here: > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dusb_msg150766.html&d=DgIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=pIpbe7JcuPH0X7-CYbBt2U3yE2WPSPtrGaMQRtcvQM0&s=hYurvvfAzlq9WTDnKKUznSRb5thSAvIO9doLJOzfUSo&e= > > > I didn't see the slight shift to u16s there. > > Leo, does the patch need a respin, or is it against Linus' HEAD and my > tree is stale? > > Anyway, after re-applying it to my tree: > Tested-by: John Stultz > Thanks Leo and John. I just sent out a rebased version for Felipe to pick up. 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: Oops with dwc3 in device mode and functionfs
On Fri, 6 Jan 2017 15:21:17 +, Vincent Pelletier wrote: > The next issue is that often (>9 times out of 10) the dwc3 fails to > respond to the SET_CONFIGURATION standard request. As a result, only > EP0 works. I captured a successful enumeration, there are also corrupted pids: 002:03.271'422"583n SETUP @054.00 DATA0 8B ACK 000 80 06 03 03 09 04 ff 00 002:03.271'425"200n IN @054.00 NAK 002:03.271'446"233n IN @054.00 NAK 002:03.271'478"233n IN @054.00 NAK 002:03.271'499"233n IN @054.00 DATA134B ACK 000 22 03 46 00 5a 00 45 00 44 00 34 00 34 00 33 00 ..F.Z.E.D.4.4.3. 010 44 00 30 00 31 00 54 00 34 00 32 00 35 00 30 00 D.0.1.T.4.2.5.0. 020 31 001. 002:03.271'502"133n OUT @054.00 DATA1 0B NAK 002:03.271'523"216n PING@054.00 NAK 002:03.271'556"233n PING@054.00 NAK 002:03.271'581"666n PING@054.00 ACK 002:03.271'583"050n OUT @054.00 DATA1 0B ACK 002:03.272'345"833n SETUP @054.00 DATA0 8B ACK 000 00 09 01 00 00 00 00 00 002:03.272'348"350n IN @054.00 NAK 002:03.272'369"350n IN @054.00 NAK [...] 002:03.273'082"100n IN @054.00 NAK 002:03.273'119"633n (bad pid) 0xf0 0x3e 0x88 002:03.273'103"450n IN @054.00 NAK (incomplete transaction) 002:03.273'123"333n (bad pid) 0xf0 0x3e 0x88 002:03.273'127"033n (bad pid) 0xf0 0x3e 0x88 002:03.273'145"450n IN @054.00 NAK 002:03.273'166"466n IN @054.00 NAK [...] 002:03.278'582"600n IN @054.00 NAK 002:03.278'604"200n IN @054.00 DATA1 0B ACK There are 285 NAKs between SETUP and the final ACK. A note about "incomplete transaction": this is a bug in my trace parser, the NAK transaction is complete, it is the next token (the bad pid) which confuses the state tracker, which also causes the timestamp non-monotonicity in parser's output too. There is one bad-CRC DATA0 packet after each bad pid. > There seems to be another issue (or is it just a consequence of this > one ?) where it may become completely silent, unable to join the bus > when udc identifier is written to UDC file. From what I can reproduce, it always happens in 3 stages: - first enumeration (whether SET_CONFIGURATION succeeds or not seems irrelevant) - second enumeration, which SET_CONFIGURATION always times-out in my experience (but given the low success rate, having two consecutive successes would be hard anyway) - 3rd enumeration attempt fails, no event on the bus. -- Vincent Pelletier -- 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