RE: [PATCH v3 4/6] usb: chipidea: Fix otg event handler
> v2: v3: no change > > drivers/usb/chipidea/otg.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index > db4ceff..f25d482 100644 > --- a/drivers/usb/chipidea/otg.c > +++ b/drivers/usb/chipidea/otg.c > @@ -203,14 +203,17 @@ static void ci_otg_work(struct work_struct *work) > } > > pm_runtime_get_sync(ci->dev); > + > if (ci->id_event) { > ci->id_event = false; > ci_handle_id_switch(ci); > - } else if (ci->b_sess_valid_event) { > + } > + > + if (ci->b_sess_valid_event) { > ci->b_sess_valid_event = false; > ci_handle_vbus_change(ci); > - } else > - dev_err(ci->dev, "unexpected event occurs at %s\n", __func__); > + } > + > pm_runtime_put_sync(ci->dev); > > enable_irq(ci->irq); > -- For chipidea changes, I will apply them. Thanks. Peter
Re: musb_hdrc HNP?
Hello. >Is commit [1] reverted or not in this experiment? In fact I noticed I am looking into TI kernel which is distributed in official debian image for pocketbeagle. So, the behavior may not be the same as mainline kernel code. I think I continue look into what I have now, and later check mainline status when I need to support (or try) other boards as well. From: linux-usb-ow...@vger.kernel.org on behalf of Bin Liu Sent: Tuesday, September 4, 2018 11:00 PM To: Takashi Matsuzawa Cc: linux-usb@vger.kernel.org Subject: Re: musb_hdrc HNP? Hi, On Fri, Aug 31, 2018 at 06:35:50AM +, Takashi Matsuzawa wrote: > Hello. > I just confirmed what I wanted to see. > I could do lsusb to list A-device (from b_host) and B-device (from a_host). > Suspending from either side kicked role change between A-device and B-device > (in both direction). > > I needed to wait 20ms after B-device seeing MUSB_INTR_CONNECT and before > calling musb_host_poke_root_hub(). > > I suppose seeing CONNECT inturrupt B-device can expect that A-device is > reset, but it may take some time and B-device may need to wait. > On technial nodes, this is mentioned as something like this: > > "Reset Signaling. If the RESET bit in the POWER register (bit 3) is set while > the controller is in Host mode, it will generate Reset signaling on the bus. > If the HSENAB bit in the POWER register (bit 5) was set, it will also try to > negotiate for high-speed operation. The software should keep the RESET bit > set for at least 20 ms to ensure correct resetting of the target device." > > Note I still see errors like below after role change (b_host -> > b_peripheral), perhaps some necessary cleanup is not there. > But anyway they role-switched in both direction.. Is commit [1] reverted or not in this experiment? [1] 0a9134bd733b usb: musb: disable otg protocol support > > [ 1204.225843] usb 2-1: USB disconnect, device number 7 > [ 1204.274238] hub 2-0:1.0: USB hub found > [ 1204.282564] hub 2-0:1.0: 1 port detected > [ 1204.496301] musb-hdrc musb-hdrc.1: Babble > [ 1204.622799] musb_h_ep0_irq 1192: no URB for end 0 > [ 1208.474661] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1210.063965] zero gadget: high-speed config #3: source/sink > [ 1212.566697] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1212.573607] usb usb2-port1: attempt power cycle > [ 1216.974544] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1221.066539] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1221.073328] usb usb2-port1: unable to enumerate USB device > debian@beaglebone:~/wk-b$ Regards, -Bin. > === > > > From: linux-usb-ow...@vger.kernel.org on > behalf of Takashi Matsuzawa > Sent: Friday, August 31, 2018 1:50 PM > To: Bin Liu > Cc: linux-usb@vger.kernel.org > Subject: Re: musb_hdrc HNP? > > Hello. > FYI. I made a progress on this, but no solution yet. > > >The smartphone does use HNP, it is not iPhone Carplay, correct? > > At this point, I am trying to see original HNP behavior between two > pocketbeagles. > (After seeing it works, I plan to replace B-device with a phone, and so > customization on A-device stack.) > > >1. MUSB uses one register bit to report babble and reset event, so driver > >bug could report bus reset as babble if it doesn't trace the controller role > >correctly; > > As mentioned below, it may be related to how driver on B-device responds to > RESET/BABBLE interrupts (or its sideeffets). > > 1) I could see CONFIG_USB_OTG was not set on "Debian 9.4 2018-06-17" image so > I turned it on. > > This improved the situation a bit, like A-device side b_hnp_enable flag > (which is set when B-device b_hnp_enable is set.) > > 2) Now I can see A-device and B-device turns to expected modes. > > A-device: > > a_host -> a_peripheral > After suspending port, sees DISCONNECT and RESET events. > > B-device: > > b_peripheral -> b_host > Sees SUSPEND, CONNECT events. > > 3) But I do not see B-device enumerate A-device at this point, and instead on > B-device (now b_host) RESET(or BUBBLE) events are seen. > Then after that, immediately, SUSPEND is seen on A-device, looks like now > A-device is resuming as a_host and B-device back to b_peripheral. > > 4) I expect at 2) B-device should enumerate A-device and both stays in new > mode (and I can, say do lsusb on B-device and see A-device listed), but it > does not happen. > > Ignoring RESET(BUBBLE) events on B-device (b_host) at 3) did not improve the > situgation. (Now I see SUSPEND on B-device instead of DISCONNECT.) > > It may be that driver behavior after 2) (to be initiated as a_peripheral and > b_host and restarting) having some problem. > > > From: linux-usb-ow...@vger.kernel.org on > behalf of Bin Liu > Sent: Monday, August 27, 2018 10:33 PM > To: Takashi Matsuzawa > Cc: linux-usb@vger.kernel.org
Re: [hid:for-4.20/multitouch 1/1] drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no member named 'application'
On Wed, 5 Sep 2018, kbuild test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git > for-4.20/multitouch > head: 93c61d2e6bc4849a5527e32568479ff67154870e > commit: 93c61d2e6bc4849a5527e32568479ff67154870e [1/1] HID: multitouch: > simplify the application retrieval > config: x86_64-randconfig-x018-201835 (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > git checkout 93c61d2e6bc4849a5527e32568479ff67154870e > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > >drivers/hid/hid-multitouch.c: In function 'mt_input_mapping': > >> drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no > >> member named 'application' > hi->application = HID_DG_STYLUS; > ^~ >drivers/hid/hid-multitouch.c: In function 'mt_input_configured': >drivers/hid/hid-multitouch.c:1527:12: error: 'struct hid_input' has no > member named 'application' > switch (hi->application) { >^~ That is my mismerge. I've fixed it now. Thanks for the report and sorry for the noise. -- Jiri Kosina SUSE Labs
[hid:for-4.20/multitouch 1/1] drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no member named 'application'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-4.20/multitouch head: 93c61d2e6bc4849a5527e32568479ff67154870e commit: 93c61d2e6bc4849a5527e32568479ff67154870e [1/1] HID: multitouch: simplify the application retrieval config: x86_64-randconfig-x018-201835 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: git checkout 93c61d2e6bc4849a5527e32568479ff67154870e # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/hid/hid-multitouch.c: In function 'mt_input_mapping': >> drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no member >> named 'application' hi->application = HID_DG_STYLUS; ^~ drivers/hid/hid-multitouch.c: In function 'mt_input_configured': drivers/hid/hid-multitouch.c:1527:12: error: 'struct hid_input' has no member named 'application' switch (hi->application) { ^~ vim +1327 drivers/hid/hid-multitouch.c 1257 1258 #define mt_map_key_clear(c) hid_map_usage_clear(hi, usage, bit, \ 1259 max, EV_KEY, (c)) 1260 static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, 1261 struct hid_field *field, struct hid_usage *usage, 1262 unsigned long **bit, int *max) 1263 { 1264 struct mt_device *td = hid_get_drvdata(hdev); 1265 struct mt_application *application; 1266 struct mt_report_data *rdata; 1267 1268 rdata = mt_find_report_data(td, field->report); 1269 if (!rdata) { 1270 hid_err(hdev, "failed to allocate data for report\n"); 1271 return 0; 1272 } 1273 1274 application = rdata->application; 1275 1276 /* 1277 * If mtclass.export_all_inputs is not set, only map fields from 1278 * TouchScreen or TouchPad collections. We need to ignore fields 1279 * that belong to other collections such as Mouse that might have 1280 * the same GenericDesktop usages. 1281 */ 1282 if (!td->mtclass.export_all_inputs && 1283 field->application != HID_DG_TOUCHSCREEN && 1284 field->application != HID_DG_PEN && 1285 field->application != HID_DG_TOUCHPAD && 1286 field->application != HID_GD_KEYBOARD && 1287 field->application != HID_GD_SYSTEM_CONTROL && 1288 field->application != HID_CP_CONSUMER_CONTROL && 1289 field->application != HID_GD_WIRELESS_RADIO_CTLS && 1290 field->application != HID_GD_SYSTEM_MULTIAXIS && 1291 !(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS && 1292application->quirks & MT_QUIRK_ASUS_CUSTOM_UP)) 1293 return -1; 1294 1295 /* 1296 * Some Asus keyboard+touchpad devices have the hotkeys defined in the 1297 * touchpad report descriptor. We need to treat these as an array to 1298 * map usages to input keys. 1299 */ 1300 if (field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS && 1301 application->quirks & MT_QUIRK_ASUS_CUSTOM_UP && 1302 (usage->hid & HID_USAGE_PAGE) == HID_UP_CUSTOM) { 1303 set_bit(EV_REP, hi->input->evbit); 1304 if (field->flags & HID_MAIN_ITEM_VARIABLE) 1305 field->flags &= ~HID_MAIN_ITEM_VARIABLE; 1306 switch (usage->hid & HID_USAGE) { 1307 case 0x10: mt_map_key_clear(KEY_BRIGHTNESSDOWN); break; 1308 case 0x20: mt_map_key_clear(KEY_BRIGHTNESSUP); break; 1309 case 0x35: mt_map_key_clear(KEY_DISPLAY_OFF); break; 1310 case 0x6b: mt_map_key_clear(KEY_F21); break; 1311 case 0x6c: mt_map_key_clear(KEY_SLEEP); break; 1312 default: 1313 return -1; 1314 } 1315 return 1; 1316 } 1317 1318 if (rdata->is_mt_collection) 1319 return mt_touch_input_mapping(hdev, hi, field, usage, bit, max, 1320application); 1321 1322 /* 1323 * some egalax touchscreens have "application == DG_TOUCHSCREEN" 1324 * for the stylus. Overwrite the hid_input application 1325 */ 1326 if (field->physical == HID_DG_STYLUS) > 1327 hi->application = HID_DG_STYLUS; 1328 1329 /* let hid-core decide for the others */ 1330 return 0; 1331 } 1332 --- 0-DAY kernel test infrastructureOpen Source
Re: [PATCH v2] usb: Avoid use-after-free by flushing endpoints early in usb_set_interface()
On Mon, 3 Sep 2018, Mathias Nyman wrote: > The steps taken by usb core to set a new interface is very different from > what is done on the xHC host side. > > xHC hardware will do everything in one go. One command is used to set up > new endpoints, free old endpoints, check bandwidth, and run the new > endpoints. > > All this is done by xHC when usb core asks the hcd to check for > available bandwidth. At this point usb core has not yet flushed the old > endpoints, which will cause use-after-free issues in xhci driver as > queued URBs are cancelled on a re-allocated endpoint. > > To resolve this add a call to usb_disable_interface() which will flush > the endpoints before calling usb_hcd_alloc_bandwidth() > > Additional checks in xhci driver will also be implemented to gracefully > handle stale URB cancel on freed and re-allocated endpoints > > Cc: > Reported-by: Sudip Mukherjee > Signed-off-by: Mathias Nyman > --- > v2: update kerneldoc as well > --- > drivers/usb/core/message.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > index 228672f..bfa5eda 100644 > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -1341,6 +1341,11 @@ void usb_enable_interface(struct usb_device *dev, > * is submitted that needs that bandwidth. Some other operating systems > * allocate bandwidth early, when a configuration is chosen. > * > + * xHCI reserves bandwidth and configures the alternate setting in > + * usb_hcd_alloc_bandwidth(). If it fails the original interface altsetting > + * may be disabled. Drivers cannot rely on any particular alternate > + * setting being in effect after a failure. > + * > * This call is synchronous, and may not be used in an interrupt context. > * Also, drivers must not change altsettings while urbs are scheduled for > * endpoints in that interface; all such urbs must first be completed > @@ -1376,6 +1381,12 @@ int usb_set_interface(struct usb_device *dev, int > interface, int alternate) >alternate); > return -EINVAL; > } > + /* > + * usb3 hosts configure the interface in usb_hcd_alloc_bandwidth, > + * including freeing dropped endpoint ring buffers. > + * Make sure the interface endpoints are flushed before that > + */ > + usb_disable_interface(dev, iface, false); > > /* Make sure we have enough bandwidth for this alternate interface. >* Remove the current alt setting and add the new alt setting. Acked-by: Alan Stern
Re: A few questions about gadgetfs
On Tue, 4 Sep 2018, Andrey Konovalov wrote: > Hi Alan, > > Another question. According to the USB spec: > > """ > The Data stage of a control transfer from an endpoint to the host is > complete when the endpoint does one of the following: > * Has transferred exactly the amount of data specified during the Setup stage > * Transfers a packet with a payload size less than wMaxPacketSize or > transfers a zero-length packet > """ > > AFAIU, this means that a device only needs to send a zero-length > packet when the last data packet happened to be equal to > wMaxPacketSize to indicate that the data stage is over. And if the total reply size is less than wLength in the Setup stage. > Gadgetfs seems > to be doing something different, it sets the req->zero flag (for the > response passed to usb_ep_queue) when value < w_length, where value is > the length of the response being submitted and w_length is the value > of the wLength field of the control request. Is there something I > misunderstand? No, you got it. But you're missing an important fact: The USB controller drivers will ignore the ->zero flag in cases where the message length isn't an exact multiple of wMaxPacketSize. This is what makes gadgetfs's behavior correct. Alan Stern
Re: [PATCH] usb: Don't die twice if PCI xhci host is not responding in resume
On Tue, 4 Sep 2018, Mathias Nyman wrote: > usb_hc_died() should only be called once, and with the primary HCD > as parameter. It will mark both primary and secondary hcd's dead. > > Remove the extra call to usb_cd_died with the shared hcd as parameter. > > Fixes: ff9d78b36f76 ("USB: Set usb_hcd->state and flags for shared roothubs") > Signed-off-by: Mathias Nyman > --- > drivers/usb/core/hcd-pci.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c > index 66fe1b7..0343246 100644 > --- a/drivers/usb/core/hcd-pci.c > +++ b/drivers/usb/core/hcd-pci.c > @@ -515,8 +515,6 @@ static int resume_common(struct device *dev, int event) > event == PM_EVENT_RESTORE); > if (retval) { > dev_err(dev, "PCI post-resume error %d!\n", retval); > - if (hcd->shared_hcd) > - usb_hc_died(hcd->shared_hcd); > usb_hc_died(hcd); > } > } Acked-by: Alan Stern
Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space
On Tue, 4 Sep 2018, Johan Hovold wrote: > On Tue, Sep 04, 2018 at 12:21:09PM +0200, Oliver Neukum wrote: > > On Di, 2018-09-04 at 11:31 +0200, Johan Hovold wrote: > > > On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote: > > > > For those people who run with panic_on_warn a WARN() triggered > > > > from user space is a DOS. It is worth returning to dev_err() > > > > > > I think this should be dev_warn() unless you want to bring back the > > > returning of errors on these conditions as well (i.e. as was the case > > > prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control > > > flow")). > > > > Should I? A warning in syslog is pretty hardcore, so I have no idea > > whether dev_warn() is enough. > > Perhaps there are two sides to this. If something really should not be > happening and needs to be addressed (i.e. it's a driver bug) that > dev_WARN is warranted. If user space can be pass in bogus flags that > gets propagated to USB core, perhaps those need to be sanitised sooner > (in the vain of "don't trust anything coming from user space"). I'd go along with this. The usbfs code should fix or reject URBs submitted from userspace with bogus flags or an incorrect pipe value. (In fact, we already sanitize the flags to some extent, but we could do more: ISO_ASAP should apply only to isochronous URBs, and ZERO_PACKET should apply only to bulk-OUT URBS.) Similar errors coming from kernel drivers should be reported as actual bugs. Alan > In general though, I believe dev_warn() is more appropriate for cases > where something odd is happening, but we try to recover and proceed > anyway (e.g. by sanitising the flags without bailing out as is the case > here) instead of aborting. > > Johan
Re: A few questions about gadgetfs
Hi Alan, Another question. According to the USB spec: """ The Data stage of a control transfer from an endpoint to the host is complete when the endpoint does one of the following: * Has transferred exactly the amount of data specified during the Setup stage * Transfers a packet with a payload size less than wMaxPacketSize or transfers a zero-length packet """ AFAIU, this means that a device only needs to send a zero-length packet when the last data packet happened to be equal to wMaxPacketSize to indicate that the data stage is over. Gadgetfs seems to be doing something different, it sets the req->zero flag (for the response passed to usb_ep_queue) when value < w_length, where value is the length of the response being submitted and w_length is the value of the wLength field of the control request. Is there something I misunderstand? Thanks!
[PATCH v3 6/6] arm: dts: qcom: db410c: Enable USB OTG support
The Dragonboard-410c is able to act either as USB Host or Device. The role can be determined at runtime via the USB_HS_ID pin which is derived from the micro-usb port VBUS pin. In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB. In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port. Routing is selected via USB_SW_SEL_PM gpio. In device role USB HUB can be held in reset. chipidea driver expects two extcon device pointers, one for the EXTCON_USB event and one for the EXTCON_USB_HOST event. Since the extcon-usb-gpio device is capable of generating both these events, point two times to this extcon device. Signed-off-by: Loic Poulain --- v2: no change v3: Point two times to the same extcon-usb-device arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 20 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 11 ++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi index ec2f0de..99787cc 100644 --- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi @@ -8,6 +8,16 @@ pinconf { pins = "gpio3"; function = PMIC_GPIO_FUNC_NORMAL; + input-disable; + output-high; + }; + }; + + usb_hub_reset_pm_device: usb_hub_reset_pm_device { + pinconf { + pins = "gpio3"; + function = PMIC_GPIO_FUNC_NORMAL; + input-disable; output-low; }; }; @@ -22,6 +32,16 @@ }; }; + usb_sw_sel_pm_device: usb_sw_sel_pm_device { + pinconf { + pins = "gpio4"; + function = PMIC_GPIO_FUNC_NORMAL; + power-source = ; + input-disable; + output-low; + }; + }; + pm8916_gpios_leds: pm8916_gpios_leds { pinconf { pins = "gpio1", "gpio2"; diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi index 78ce397..1f7dc1c 100644 --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi @@ -366,14 +366,15 @@ }; usb@78d9000 { - extcon = <_id>; + extcon = <_id>, <_id>; status = "okay"; adp-disable; hnp-disable; srp-disable; - dr_mode = "host"; - pinctrl-names = "default"; - pinctrl-0 = <_sw_sel_pm>; + dr_mode = "otg"; + pinctrl-names = "default", "device"; + pinctrl-0 = <_sw_sel_pm _hub_reset_pm>; + pinctrl-1 = <_sw_sel_pm_device _hub_reset_pm_device>; ulpi { phy { v1p8-supply = <_l7>; @@ -512,7 +513,7 @@ usb_id: usb-id { compatible = "linux,extcon-usb-gpio"; - vbus-gpio = < 121 GPIO_ACTIVE_HIGH>; + id-gpio = < 121 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <_id_default>; }; -- 2.7.4
[PATCH v3 5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration
Phy power on/off cycle can happen several times during device life. We then need to balance the extcon notifier registration accordingly. Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API") Signed-off-by: Loic Poulain --- v2: don't use devres version (power-on always followed by power-off) v3: no change drivers/phy/qualcomm/phy-qcom-usb-hs.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c index abbbe75..7153dde 100644 --- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c @@ -160,8 +160,8 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy) /* setup initial state */ qcom_usb_hs_phy_vbus_notifier(>vbus_notify, state, uphy->vbus_edev); - ret = devm_extcon_register_notifier(>dev, uphy->vbus_edev, - EXTCON_USB, >vbus_notify); + ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB, + >vbus_notify); if (ret) goto err_ulpi; } @@ -182,6 +182,11 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy) { struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy); + if (uphy->vbus_edev) { + extcon_unregister_notifier(uphy->vbus_edev, EXTCON_USB, + >vbus_notify); + } + regulator_disable(uphy->v3p3); regulator_disable(uphy->v1p8); clk_disable_unprepare(uphy->sleep_clk); -- 2.7.4
[PATCH v3 4/6] usb: chipidea: Fix otg event handler
At OTG work running time, it's possible that several events need to be addressed (e.g. ID and VBUS events). The current implementation handles only one event at a time which leads to ignoring the other one. Fix it. Signed-off-by: Loic Poulain --- v2: v3: no change drivers/usb/chipidea/otg.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index db4ceff..f25d482 100644 --- a/drivers/usb/chipidea/otg.c +++ b/drivers/usb/chipidea/otg.c @@ -203,14 +203,17 @@ static void ci_otg_work(struct work_struct *work) } pm_runtime_get_sync(ci->dev); + if (ci->id_event) { ci->id_event = false; ci_handle_id_switch(ci); - } else if (ci->b_sess_valid_event) { + } + + if (ci->b_sess_valid_event) { ci->b_sess_valid_event = false; ci_handle_vbus_change(ci); - } else - dev_err(ci->dev, "unexpected event occurs at %s\n", __func__); + } + pm_runtime_put_sync(ci->dev); enable_irq(ci->irq); -- 2.7.4
[PATCH v3 1/6] usb: chipidea: Add dynamic pinctrl selection
Some hardware implementations require to configure pins differently according to the USB role (host/device), this can be an update of the pins routing or a simple GPIO value change. This patch introduces new optional "host" and "device" pinctrls. If these pinctrls are defined by the device, they are respectively selected on host/device role start. If a default pinctrl exist, it is restored on host/device role stop. Signed-off-by: Loic Poulain --- v2: includes ordering v3: no change drivers/usb/chipidea/core.c | 19 +++ drivers/usb/chipidea/host.c | 9 + drivers/usb/chipidea/udc.c | 9 + include/linux/usb/chipidea.h | 6 ++ 4 files changed, 43 insertions(+) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 85fc6db..7bfcbb2 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include #include @@ -723,6 +724,24 @@ static int ci_get_platdata(struct device *dev, else cable->connected = false; } + + platdata->pctl = devm_pinctrl_get(dev); + if (!IS_ERR(platdata->pctl)) { + struct pinctrl_state *p; + + p = pinctrl_lookup_state(platdata->pctl, "default"); + if (!IS_ERR(p)) + platdata->pins_default = p; + + p = pinctrl_lookup_state(platdata->pctl, "host"); + if (!IS_ERR(p)) + platdata->pins_host = p; + + p = pinctrl_lookup_state(platdata->pctl, "device"); + if (!IS_ERR(p)) + platdata->pins_device = p; + } + return 0; } diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c index 4638d9b..d858a82 100644 --- a/drivers/usb/chipidea/host.c +++ b/drivers/usb/chipidea/host.c @@ -13,6 +13,7 @@ #include #include #include +#include #include "../host/ehci.h" @@ -153,6 +154,10 @@ static int host_start(struct ci_hdrc *ci) } } + if (ci->platdata->pins_host) + pinctrl_select_state(ci->platdata->pctl, +ci->platdata->pins_host); + ret = usb_add_hcd(hcd, 0, 0); if (ret) { goto disable_reg; @@ -197,6 +202,10 @@ static void host_stop(struct ci_hdrc *ci) } ci->hcd = NULL; ci->otg.host = NULL; + + if (ci->platdata->pins_host && ci->platdata->pins_default) + pinctrl_select_state(ci->platdata->pctl, +ci->platdata->pins_default); } diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 9852ec5..829e947 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -1965,6 +1966,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci) static int udc_id_switch_for_device(struct ci_hdrc *ci) { + if (ci->platdata->pins_device) + pinctrl_select_state(ci->platdata->pctl, +ci->platdata->pins_device); + if (ci->is_otg) /* Clear and enable BSV irq */ hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE, @@ -1983,6 +1988,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci) hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS); ci->vbus_active = 0; + + if (ci->platdata->pins_device && ci->platdata->pins_default) + pinctrl_select_state(ci->platdata->pctl, +ci->platdata->pins_default); } /** diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h index 07f9936..63758c3 100644 --- a/include/linux/usb/chipidea.h +++ b/include/linux/usb/chipidea.h @@ -77,6 +77,12 @@ struct ci_hdrc_platform_data { struct ci_hdrc_cablevbus_extcon; struct ci_hdrc_cableid_extcon; u32 phy_clkgate_delay_us; + + /* pins */ + struct pinctrl *pctl; + struct pinctrl_state *pins_default; + struct pinctrl_state *pins_host; + struct pinctrl_state *pins_device; }; /* Default offset of capability registers */ -- 2.7.4
[PATCH v3 3/6] usb: chipidea: Prevent unbalanced IRQ disable
The ChipIdea IRQ is disabled before scheduling the otg work and re-enabled on otg work completion. However if the job is already scheduled we have to undo the effect of disable_irq int order to balance the IRQ disable-depth value. Fixes: be6b0c1bd0be ("usb: chipidea: using one inline function to cover queue work operations") Signed-off-by: Loic Poulain --- v2: v3: no change drivers/usb/chipidea/otg.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h index 7e7428e..4f8b817 100644 --- a/drivers/usb/chipidea/otg.h +++ b/drivers/usb/chipidea/otg.h @@ -17,7 +17,8 @@ void ci_handle_vbus_change(struct ci_hdrc *ci); static inline void ci_otg_queue_work(struct ci_hdrc *ci) { disable_irq_nosync(ci->irq); - queue_work(ci->wq, >work); + if (queue_work(ci->wq, >work) == false) + enable_irq(ci->irq); } #endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */ -- 2.7.4
[PATCH v3 2/6] doc: usb: ci-hdrc-usb2: Add pinctrl properties definition
Some hardware implementations require to configure pins differently according to the USB role (host/device), this can be an update of the pins routing or a simple GPIO value change. This patch introduces new optional "host" and "device" pinctrls. If these pinctrls are defined by the device, they are respectively selected on host/device role start. Signed-off-by: Loic Poulain --- v2: Add new pin modes documentation (host, device) v3: rebase on usb-next Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 2e93181..529e518 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -80,6 +80,8 @@ Optional properties: controller. It's expected that a mux state of 0 indicates device mode and a mux state of 1 indicates host mode. - mux-control-names: Shall be "usb_switch" if mux-controls is specified. +- pinctrl-names: Names for optional pin modes in "default", "host", "device" +- pinctrl-n: alternate pin modes i.mx specific properties - fsl,usbmisc: phandler of non-core register device, with one -- 2.7.4
Re: Aw: Re: Fw: Re: keyboard-problem on bpi-r2 since 4.17
On 04.09.2018 17:51, Frank Wunderlich wrote: this fixes the issue: adding the following lines to drivers/usb/host/xhci-mem.c line:1616 if (xhci->quirks & XHCI_MTK_HOST) { in_ep_ctx->reserved[0] = out_ep_ctx->reserved[0]; in_ep_ctx->reserved[1] = out_ep_ctx->reserved[1]; } commit below is wrong...i got the response: This issue is caused by the commit : 27082e2654dc (“xhci: Clear the host side toggle manually when endpoint is ‘soft reset’”)... i think this commit is meant: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f5249461b504d35aa1a40140983b7ec415807d9e sorry for confusion, had searched and not compared the result again :( pushed to my github: https://github.com/frank-w/BPI-R2-4.14/commit/c9acc5a4285d0de71171c7c1fb3ae7dc0bffec34 official patch will be send by mediatek regards Frank Ok, thanks, makes sense. I'll wait for that patch I'm offline rest of the week so responses will be delayed -Mathias
Aw: Re: Fw: Re: keyboard-problem on bpi-r2 since 4.17
this fixes the issue: adding the following lines to drivers/usb/host/xhci-mem.c line:1616 if (xhci->quirks & XHCI_MTK_HOST) { in_ep_ctx->reserved[0] = out_ep_ctx->reserved[0]; in_ep_ctx->reserved[1] = out_ep_ctx->reserved[1]; } commit below is wrong...i got the response: This issue is caused by the commit : 27082e2654dc (“xhci: Clear the host side toggle manually when endpoint is ‘soft reset’”)... i think this commit is meant: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f5249461b504d35aa1a40140983b7ec415807d9e sorry for confusion, had searched and not compared the result again :( pushed to my github: https://github.com/frank-w/BPI-R2-4.14/commit/c9acc5a4285d0de71171c7c1fb3ae7dc0bffec34 official patch will be send by mediatek regards Frank Gesendet: Dienstag, 04. September 2018 um 15:45 Uhr Von: "Mathias Nyman" An: "Frank Wunderlich" , linux-usb@vger.kernel.org Betreff: Re: Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17 On 04.09.2018 16:14, Frank Wunderlich wrote: > Hi, > > i got an info that my issue was caused by this commit: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=217491487c43892318c18b6dcafe33b6353efd40 > > Patch is in progress... That patch has been there since 4.13 If 4.16 works for you then it's probably something else >> >> is there another way to show more debug-messages? >> >> i have a working 4.16-tree, maybe i can merge newer versions (go >> forward from it instead of backwards). >> xhci debugging can be added by: mount -t debugfs none /sys/kernel/debug echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control and tracing with: (before pressing keyboard key) (mount -t debugfs none /sys/kernel/debug) echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable after pressing key traces are found at: /sys/kernel/debug/tracing/trace -Mathias
[PATCH] usb: Don't die twice if PCI xhci host is not responding in resume
usb_hc_died() should only be called once, and with the primary HCD as parameter. It will mark both primary and secondary hcd's dead. Remove the extra call to usb_cd_died with the shared hcd as parameter. Fixes: ff9d78b36f76 ("USB: Set usb_hcd->state and flags for shared roothubs") Signed-off-by: Mathias Nyman --- drivers/usb/core/hcd-pci.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c index 66fe1b7..0343246 100644 --- a/drivers/usb/core/hcd-pci.c +++ b/drivers/usb/core/hcd-pci.c @@ -515,8 +515,6 @@ static int resume_common(struct device *dev, int event) event == PM_EVENT_RESTORE); if (retval) { dev_err(dev, "PCI post-resume error %d!\n", retval); - if (hcd->shared_hcd) - usb_hc_died(hcd->shared_hcd); usb_hc_died(hcd); } } -- 2.7.4
Re: musb_hdrc HNP?
Hi, On Fri, Aug 31, 2018 at 06:35:50AM +, Takashi Matsuzawa wrote: > Hello. > I just confirmed what I wanted to see. > I could do lsusb to list A-device (from b_host) and B-device (from a_host). > Suspending from either side kicked role change between A-device and B-device > (in both direction). > > I needed to wait 20ms after B-device seeing MUSB_INTR_CONNECT and before > calling musb_host_poke_root_hub(). > > I suppose seeing CONNECT inturrupt B-device can expect that A-device is > reset, but it may take some time and B-device may need to wait. > On technial nodes, this is mentioned as something like this: > > "Reset Signaling. If the RESET bit in the POWER register (bit 3) is set while > the controller is in Host mode, it will generate Reset signaling on the bus. > If the HSENAB bit in the POWER register (bit 5) was set, it will also try to > negotiate for high-speed operation. The software should keep the RESET bit > set for at least 20 ms to ensure correct resetting of the target device." > > Note I still see errors like below after role change (b_host -> > b_peripheral), perhaps some necessary cleanup is not there. > But anyway they role-switched in both direction.. Is commit [1] reverted or not in this experiment? [1] 0a9134bd733b usb: musb: disable otg protocol support > > [ 1204.225843] usb 2-1: USB disconnect, device number 7 > [ 1204.274238] hub 2-0:1.0: USB hub found > [ 1204.282564] hub 2-0:1.0: 1 port detected > [ 1204.496301] musb-hdrc musb-hdrc.1: Babble > [ 1204.622799] musb_h_ep0_irq 1192: no URB for end 0 > [ 1208.474661] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1210.063965] zero gadget: high-speed config #3: source/sink > [ 1212.566697] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1212.573607] usb usb2-port1: attempt power cycle > [ 1216.974544] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1221.066539] usb usb2-port1: Cannot enable. Maybe the USB cable is bad? > [ 1221.073328] usb usb2-port1: unable to enumerate USB device > debian@beaglebone:~/wk-b$ Regards, -Bin. > === > > > From: linux-usb-ow...@vger.kernel.org on > behalf of Takashi Matsuzawa > Sent: Friday, August 31, 2018 1:50 PM > To: Bin Liu > Cc: linux-usb@vger.kernel.org > Subject: Re: musb_hdrc HNP? > > Hello. > FYI. I made a progress on this, but no solution yet. > > >The smartphone does use HNP, it is not iPhone Carplay, correct? > > At this point, I am trying to see original HNP behavior between two > pocketbeagles. > (After seeing it works, I plan to replace B-device with a phone, and so > customization on A-device stack.) > > >1. MUSB uses one register bit to report babble and reset event, so driver > >bug could report bus reset as babble if it doesn't trace the controller role > >correctly; > > As mentioned below, it may be related to how driver on B-device responds to > RESET/BABBLE interrupts (or its sideeffets). > > 1) I could see CONFIG_USB_OTG was not set on "Debian 9.4 2018-06-17" image so > I turned it on. > > This improved the situation a bit, like A-device side b_hnp_enable flag > (which is set when B-device b_hnp_enable is set.) > > 2) Now I can see A-device and B-device turns to expected modes. > > A-device: > > a_host -> a_peripheral > After suspending port, sees DISCONNECT and RESET events. > > B-device: > > b_peripheral -> b_host > Sees SUSPEND, CONNECT events. > > 3) But I do not see B-device enumerate A-device at this point, and instead on > B-device (now b_host) RESET(or BUBBLE) events are seen. > Then after that, immediately, SUSPEND is seen on A-device, looks like now > A-device is resuming as a_host and B-device back to b_peripheral. > > 4) I expect at 2) B-device should enumerate A-device and both stays in new > mode (and I can, say do lsusb on B-device and see A-device listed), but it > does not happen. > > Ignoring RESET(BUBBLE) events on B-device (b_host) at 3) did not improve the > situgation. (Now I see SUSPEND on B-device instead of DISCONNECT.) > > It may be that driver behavior after 2) (to be initiated as a_peripheral and > b_host and restarting) having some problem. > > > From: linux-usb-ow...@vger.kernel.org on > behalf of Bin Liu > Sent: Monday, August 27, 2018 10:33 PM > To: Takashi Matsuzawa > Cc: linux-usb@vger.kernel.org > Subject: Re: musb_hdrc HNP? > > Hi, > > On Mon, Aug 27, 2018 at 12:57:55AM +, Takashi Matsuzawa wrote: > > Thank you for your suggestion. > > Yes, I am aware that full-OTG support code is being wiped out of the > > latest mainline kernels. > > Okay. Let me know if reverting that patch can magically make HNP works. > > > I am trying this for smartphone connectivity where OTG based > > role-switch is being used, which may not be of interest of everyone. > > The smartphone does use HNP, it is not iPhone Carplay, correct? > > > I picked pocketbeagle since
Re: [PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT
Hi, On 04-09-18 14:13, Andy Shevchenko wrote: On Tue, Sep 4, 2018 at 2:22 PM Heikki Krogerus wrote: Hi, These patches will introduce a few improvements to the USB Type-C support on Intel CHT platform. In this series I'm preparing Intel CHT mux handling for DisplayPort Alt Mode support, but please note that, after these patches the DisplayPort alternate mode will still not work out of the box on CHT platform. Changes to the fusb302.c, and possibly tcpm.c are still needed. This version will only fix the issue Hans pointed out in v2. Instead of replacing the connection that assigned the mux to the USB Type-C port with a connection that assigns the mux to the alternate mode device, we keep all the existing connections and add a new one for the alternate mode device. That way USB3 devices continue to be enumerated as USB3 devices instead of USB2 devices. The origin thread can be read here: https://www.spinics.net/lists/linux-usb/msg172033.html It seems you forgot my tag Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is okay with them. I'm ok with them now. The entire series is: Reviewed-by: Hans de Goede Greg, I believe that the intent was for the entire series to be merged through your tree and I believe that this series is ready for merging now. Regards, Hans Heikki Krogerus (10): usb: typec: Take care of driver module reference counting usb: roles: Handle driver reference counting platform: x86: intel_cht_int33fe: Add dependency on muxes drivers: base: Helpers for adding device connection descriptions platform: x86: intel_cht_int33fe: Register all connections at once platform: x86: intel_cht_int33fe: Add connection for the DP alt mode platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port usb: typec: class: Don't use port parent for getting mux handles platform: x86: intel_cht_int33fe: Remove the old connections for the muxes usb: typec: fusb302: reorganizing the probe function a little drivers/platform/x86/Kconfig | 2 ++ drivers/platform/x86/intel_cht_int33fe.c | 27 - drivers/usb/common/roles.c | 15 -- drivers/usb/typec/class.c| 38 ++-- drivers/usb/typec/fusb302/fusb302.c | 25 ++-- drivers/usb/typec/mux.c | 17 --- include/linux/device.h | 24 +++ 7 files changed, 87 insertions(+), 61 deletions(-) -- 2.18.0
Re: Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17
On 04.09.2018 16:14, Frank Wunderlich wrote: Hi, i got an info that my issue was caused by this commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=217491487c43892318c18b6dcafe33b6353efd40 Patch is in progress... That patch has been there since 4.13 If 4.16 works for you then it's probably something else is there another way to show more debug-messages? i have a working 4.16-tree, maybe i can merge newer versions (go forward from it instead of backwards). xhci debugging can be added by: mount -t debugfs none /sys/kernel/debug echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control and tracing with: (before pressing keyboard key) (mount -t debugfs none /sys/kernel/debug) echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable after pressing key traces are found at: /sys/kernel/debug/tracing/trace -Mathias
Re: Nothing in /sys/class/udc
Hi, Ranran writes: >> Ranran writes: >> > > A fix for my last comment: >> >> > usb device controller should be supported with conga-MA5 (contains >> >> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc >> >> > empty. >> >> > In congatec MA-5 document it is said: >> >> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express >> >> > connector including one USB 2.0 Dual Role port. The xHCI host >> >> > controller in the SoC complies with USB standard 1.1 and 2.0 with >> >> > high-speed, full-speed and low-speed USB signalling. Note USB Dual >> >> > Role is only supported under Linux. The port is a standard USB Host >> >> > port under Windows." >> >> > Maybe some configuration in my .config is missing ? (I enabled all >> >> > controllers in device-drivers->usb->usb-> gadget->usb peripheral >> >> > control , so I am not sure what is still missing) >> >> >> >> do you have the PCI device enabled? what do you get with lspci - ? >> >> >> > >> > Yes, here it is: >> >> it's not. APL's dwc3 ID 0x5aaa. See dwc3-pci.c: >> >> #define PCI_DEVICE_ID_INTEL_APL 0x5aaa >> >> you only have xhci enabled, not dwc3. >> > Right, I made big confusion in things. sorry ! > So, it seems that I don't have any usb peripheral controller (xhci is > only host and there is no other usb device controller listed above), > Right ? right -- balbi
Re: [PATCH] dwc3: Remove wait for wait_end_transfer event.
Hello, Le jeu. 19 juil. 2018 à 09:24, Felipe Balbi a écrit : > > > Hi, > > lemag...@gmail.com writes: > > From: Pierre Le Magourou > > > > We can't wait for END_OF_TRANSFER event in dwc3_gadget_ep_dequeue() > > because it can be called in an interruption context. > > > > TRBs are now cleared after the END_OF_TRANSFER completion, in the > > interrupt handler. > > > > Signed-off-by: Pierre Le Magourou > > --- > > you forgot to Cc linux-usb > > > drivers/usb/dwc3/core.h | 14 ++ > > drivers/usb/dwc3/gadget.c | 122 > > +++--- > > 2 files changed, 85 insertions(+), 51 deletions(-) > > > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index 7a217a36c36b..5e2070183e3a 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -627,6 +627,7 @@ struct dwc3_event_buffer { > > * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer > > complete > > * @lock: spinlock for endpoint request queue traversal > > * @regs: pointer to first endpoint register > > + * @pending_trb: pointer to pending TRB structure > > * @trb_pool: array of transaction buffers > > * @trb_pool_dma: dma address of @trb_pool > > * @trb_enqueue: enqueue 'pointer' into TRB array > > @@ -654,6 +655,7 @@ struct dwc3_ep { > > void __iomem*regs; > > > > struct dwc3_trb *trb_pool; > > + struct dwc3_pending_trb *pending_trb; > > dma_addr_t trb_pool_dma; > > struct dwc3 *dwc; > > > > @@ -777,6 +779,18 @@ struct dwc3_trb { > > u32 ctrl; > > } __packed; > > > > +/** > > + * struct dwc3_pending_trb > > + * @dirty_trb: pointer to TRB waiting for END_TRANSFER completion > > + * @extra_trb: true if extra TRB was set to align transfer > > + * @num_pending_sgs: number of pending SGs > > + */ > > +struct dwc3_pending_trb { > > + struct dwc3_trb *dirty_trb; > > + bool extra_trb; > > + unsigned int num_pending_sgs; > > +}; > > + > > /** > > * struct dwc3_hwparams - copy of HWPARAMS registers > > * @hwparams0: GHWPARAMS0 > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 69bf137aab37..dd1f2b74723b 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -1341,6 +1341,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > > > > struct dwc3_ep *dep = to_dwc3_ep(ep); > > struct dwc3 *dwc = dep->dwc; > > + struct dwc3_pending_trb *p_trb; > > > > unsigned long flags; > > int ret = 0; > > @@ -1367,63 +1368,29 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > >* If request was already started, this means we had > > to > >* stop the transfer. With that we also need to ignore > >* all TRBs used by the request, however TRBs can only > > - * be modified after completion of END_TRANSFER > > - * command. So what we do here is that we wait for > > - * END_TRANSFER completion and only after that, we > > jump > > - * over TRBs by clearing HWO and incrementing dequeue > > - * pointer. > > + * be modified after completion of END_TRANSFER > > command. > >* > > - * Note that we have 2 possible types of transfers > > here: > > - * > > - * i) Linear buffer request > > - * ii) SG-list based request > > - * > > - * SG-list based requests will have r->num_pending_sgs > > - * set to a valid number (> 0). Linear requests, > > - * normally use a single TRB. > > - * > > - * For each of these two cases, if r->unaligned flag > > is > > - * set, one extra TRB has been used to align transfer > > - * size to wMaxPacketSize. > > - * > > - * All of these cases need to be taken into > > - * consideration so we don't mess up our TRB ring > > - * pointers. > > + * Don't wait for END_TRANSFER completion here because > > + * dwc3_gadget_ep_dequeue() can be called from within > > + * the controller's interrupt handler. Save the TRB > > + * pointer, the number of pending sgs, and the > > + * extra_trb flag, so that TRBs HWO can be cleared and > > + * dequeue pointer incremented when the END_TRANSFER > > + * completion is received. > >*/ > > -
Re: Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17
Hi, i got an info that my issue was caused by this commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=217491487c43892318c18b6dcafe33b6353efd40 Patch is in progress... Regards Frank Am 4. September 2018 13:16:12 MESZ schrieb Frank Wunderlich : >Hi, thank you for quick answer. >i will try it, but afaik git bisect resets full git tree to the commit, >right? >i need to do some adjustments to get system booting (config,swap mmc in >dts). > > >is there another way to show more debug-messages? > >i have a working 4.16-tree, maybe i can merge newer versions (go >forward from it instead of backwards). > >regards frank > >Gesendet: Dienstag, 04. September 2018 um 07:26 Uhr >Von: "Greg KH" >An: "Frank Wunderlich" >Cc: linux-usb@vger.kernel.org >Betreff: Re: Fw: keyboard-problem on bpi-r2 since 4.17 >On Tue, Sep 04, 2018 at 06:39:20AM +0200, Frank Wunderlich wrote: >> >> >> >> Hi, >> >> i have problems with usb-keyboard on bananapi-r2 since 4.17. same >keyboard works till 4.16. >> In 4.19-rc1 same issue occours. Keyboard is recognized and on >keypress it is disconnected >> and connected again without anything written to console. >> >> dmesg (printk-debuglevel=8) looks like this: >> >> [ 77.292068] usb 1-1: USB disconnect, device number 2 >> [ 77.292068] usb 1-1: USB disconnect, device number 2 >> [ 77.472554] evbug: Disconnected device: input0 >> [ 77.472554] evbug: Disconnected device: input0 >> [ 77.632390] evbug: Disconnected device: input1 >> [ 77.632390] evbug: Disconnected device: input1 >> [ 77.773255] evbug: Disconnected device: input2 >> [ 77.773255] evbug: Disconnected device: input2 >> [ 78.342590] usb 1-1: new low-speed USB device number 3 using >xhci-mtk >> [ 78.342590] usb 1-1: new low-speed USB device number 3 using >xhci-mtk >> [ 78.545576] input: USB USB Keykoard as >> >/devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 >> [ 78.545576] input: USB USB Keykoard as >> >/devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 >> [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at >> usb-1a1c.usb-1/input0) >> [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID >> v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 >> [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at >> usb-1a1c.usb-1/input0) >> [ 78.655044] input: USB USB Keykoard Consumer Control as >> >/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 >> [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID >> v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 >> [ 78.655044] input: USB USB Keykoard Consumer Control as >> >/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 >> [ 78.734340] evbug: Connected device: input5 (USB USB Keykoard >> Consumer Control at usb-1a1c.usb-1/input1) >> [ 78.746118] input: USB USB Keykoard System Control as >> >/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 >> [ 78.734340] e[ 78.760069] evbug: Connected device: input6 (USB USB >> Keykoard System Control at usb-1a1c.usb-1/input1) >> vbug: Connected [ 78.770893] hid-generic 0003:1C4F:0002.0004: >> input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on >> usb-1a1c.usb-1/inpu >> t1 >> device: input5 (USB USB Keykoard Consumer Control at >> usb-1a1c.usb-1/input1) >> [ 78.746118] input: USB USB Keykoard System Control as >> >/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 >> [ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System >> Control at usb-1a1c.usb-1/input1) >> [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID >> v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/input1 >> >> can i debug this further? >> >> i see that mtk_xhci-driver was rewritten to use new api >> >> >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/host/xhci-mtk.c?h=v4.17-rc1=6ae9f5062aa6f5a301c16715c601c05bc9aa450e >> >> maybe this is the reason, but i don't know how to debug it. i tried >> disabling acpi and apic via cmdline to exclude anything acpi-related. >> >> maybe you have an idea... > >Can you use 'git bisect' to track down the offending commit? > >thanks, > >greg k-h Mit freundlichen Grüßen Frank Wunderlich
Re: gadgetfs & endpoints
On Tue, Sep 4, 2018 at 2:36 PM Ranran wrote: > > Hello, > > In gadgetfs documentation I find no mention of endpoints: > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt > > Is it possible to create endpoints using gadgetfs ? > The question need to be phrased differently: Is it possible to create HID using gadgetfs. I see that it is possible to create usb device with endpoints as described here: http://www.linux-usb.org/gadget/usb.c But what about HID device ? How can we implement HID device using gadget ? Best Regards, ranran > Thank you, > ranran
Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO
On Sat, Aug 04, 2018 at 12:17:15PM +0200, Loic Poulain wrote: > Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS > pins, allowing host to control them via simple USB control transfers. > To make use of a CBUS pin in Bit Bang mode, the pin must be configured > to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular > USB to Serial function. > > In this implementation, a GPIO controller is registered on FTDI probe > if at least one CBUS pin is configured for I/O mode. For now, only > FTX devices are supported. > > This patch is based on previous Stefan Agner implementation tentative > on LKML [1]. > > [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch > > Signed-off-by: Loic Poulain > --- > v2: Use message-id for LKML reference > Rework read_eeprom according to Andy's comment and return read count > Remove noisy messages > Comment style alignment > Add defines for magic values > Cannot use devm, because gpiochip is linked to the port not to the udev > v3: >Fix typo in CBUS mask description comment First, thanks for looking into this; seems like we're finally about to get support for the CBUS gpios. But now I get not one, but two, competing implementations for this posted in one month: https://lkml.kernel.org/r/20180825204744.2307-1-pa...@pados.hu And it looks like you both have been considering at least some of the earlier attempts, which is great. I've reviewed both patches and it seems to me that Karoly's version is closer to what I'd like to see as the end-result. Specifically this means having: - fixed offsets for the physical pins rather than having the offsets depend on eeprom configuration - better type abstraction (we want to add support for ft232r and ft232h eventually as well) The other patch is also more complete in that it: - considers the initial pin state - implements a request() callback - implements a get_direction() callback In contrast, with this implementation as it stands, we could end up with a driven pin being reported as an input (corner case, but still). Implementation-wise, the other patch is also closer to what I'd like to see (e.g. not using atomic bit ops, and getting the error handling right from the start). There are some issues that needs to be addressed in the other patch as well, and in doing so it would be wise to look at your patch for inspiration (e.g. naming issues and adding an eeprom helper to only read the couple of configuration words needed). In the end, going with the other patch means less work for me, even if you both (by unfortunate timing) have forced me to do more than twice the amount of review work already. Hopefully we can all work together on getting the missing bits, including further device support, in place. For completeness, I've included my review comments below. Thanks, Johan > drivers/usb/serial/Kconfig| 9 ++ > drivers/usb/serial/ftdi_sio.c | 238 > ++ > drivers/usb/serial/ftdi_sio.h | 83 +++ > 3 files changed, 330 insertions(+) > > diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig > index 533f127..64c9f2e 100644 > --- a/drivers/usb/serial/Kconfig > +++ b/drivers/usb/serial/Kconfig > @@ -181,6 +181,15 @@ config USB_SERIAL_FTDI_SIO > To compile this driver as a module, choose M here: the > module will be called ftdi_sio. > > +config USB_SERIAL_FTDI_SIO_CBUS_GPIO > + bool "USB FDTI CBUS GPIO support" > + depends on USB_SERIAL_FTDI_SIO > + depends on GPIOLIB > + help > + Say yes here to add support for the CBUS bit-bang mode, allowing CBUS > + pins to act as GPIOs. Note that pins must first be configured for GPIO > + in the device's EEPROM. The FT232R and FT-X series support this mode. I understand you're referring to hardware, but this is ambiguous as you don't implement support for FT232R in the driver. > + > config USB_SERIAL_VISOR > tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver" > help > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index b5cef32..e90cae6 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -33,6 +33,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -40,12 +41,21 @@ > #include > #include > #include > + Unrelated change. > #include "ftdi_sio.h" > #include "ftdi_sio_ids.h" > > #define DRIVER_AUTHOR "Greg Kroah-Hartman , Bill Ryder > , Kuba Ober , Andreas Mohr, Johan > Hovold " > #define DRIVER_DESC "USB FTDI Serial Converters Driver" > > +#define FTDI_CBUS_PIN_COUNT 4 > + > +struct ftdi_gpiochip { > + struct gpio_chip gc; > + struct usb_serial_port *port; > + unsigned int cbus_map[FTDI_CBUS_PIN_COUNT]; Looks like u8 will do here since it's just an index, right? But see more about this mapping below first.
Re: Nothing in /sys/class/udc
On Tue, Sep 4, 2018 at 3:26 PM Felipe Balbi wrote: > > > hi, > > Ranran writes: > > > A fix for my last comment: > >> > usb device controller should be supported with conga-MA5 (contains > >> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc > >> > empty. > >> > In congatec MA-5 document it is said: > >> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express > >> > connector including one USB 2.0 Dual Role port. The xHCI host > >> > controller in the SoC complies with USB standard 1.1 and 2.0 with > >> > high-speed, full-speed and low-speed USB signalling. Note USB Dual > >> > Role is only supported under Linux. The port is a standard USB Host > >> > port under Windows." > >> > Maybe some configuration in my .config is missing ? (I enabled all > >> > controllers in device-drivers->usb->usb-> gadget->usb peripheral > >> > control , so I am not sure what is still missing) > >> > >> do you have the PCI device enabled? what do you get with lspci - ? > >> > > > > Yes, here it is: > > it's not. APL's dwc3 ID 0x5aaa. See dwc3-pci.c: > > #define PCI_DEVICE_ID_INTEL_APL 0x5aaa > > you only have xhci enabled, not dwc3. > Right, I made big confusion in things. sorry ! So, it seems that I don't have any usb peripheral controller (xhci is only host and there is no other usb device controller listed above), Right ? > -- > balbi
Re: Nothing in /sys/class/udc
hi, Ranran writes: > > A fix for my last comment: >> > usb device controller should be supported with conga-MA5 (contains >> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc >> > empty. >> > In congatec MA-5 document it is said: >> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express >> > connector including one USB 2.0 Dual Role port. The xHCI host >> > controller in the SoC complies with USB standard 1.1 and 2.0 with >> > high-speed, full-speed and low-speed USB signalling. Note USB Dual >> > Role is only supported under Linux. The port is a standard USB Host >> > port under Windows." >> > Maybe some configuration in my .config is missing ? (I enabled all >> > controllers in device-drivers->usb->usb-> gadget->usb peripheral >> > control , so I am not sure what is still missing) >> >> do you have the PCI device enabled? what do you get with lspci - ? >> > > Yes, here it is: it's not. APL's dwc3 ID 0x5aaa. See dwc3-pci.c: #define PCI_DEVICE_ID_INTEL_APL 0x5aaa you only have xhci enabled, not dwc3. -- balbi
Re: [PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT
On Tue, Sep 4, 2018 at 2:22 PM Heikki Krogerus wrote: > > Hi, > > These patches will introduce a few improvements to the USB Type-C > support on Intel CHT platform. In this series I'm preparing Intel CHT > mux handling for DisplayPort Alt Mode support, but please note that, > after these patches the DisplayPort alternate mode will still not work > out of the box on CHT platform. Changes to the fusb302.c, and possibly > tcpm.c are still needed. > > This version will only fix the issue Hans pointed out in v2. Instead > of replacing the connection that assigned the mux to the USB Type-C > port with a connection that assigns the mux to the alternate mode > device, we keep all the existing connections and add a new one for the > alternate mode device. That way USB3 devices continue to be enumerated > as USB3 devices instead of USB2 devices. > > The origin thread can be read here: > https://www.spinics.net/lists/linux-usb/msg172033.html > > It seems you forgot my tag Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is okay with them. > Heikki Krogerus (10): > usb: typec: Take care of driver module reference counting > usb: roles: Handle driver reference counting > platform: x86: intel_cht_int33fe: Add dependency on muxes > drivers: base: Helpers for adding device connection descriptions > platform: x86: intel_cht_int33fe: Register all connections at once > platform: x86: intel_cht_int33fe: Add connection for the DP alt mode > platform: x86: intel_cht_int33fe: Add connections for the USB Type-C > port > usb: typec: class: Don't use port parent for getting mux handles > platform: x86: intel_cht_int33fe: Remove the old connections for the > muxes > usb: typec: fusb302: reorganizing the probe function a little > > drivers/platform/x86/Kconfig | 2 ++ > drivers/platform/x86/intel_cht_int33fe.c | 27 - > drivers/usb/common/roles.c | 15 -- > drivers/usb/typec/class.c| 38 ++-- > drivers/usb/typec/fusb302/fusb302.c | 25 ++-- > drivers/usb/typec/mux.c | 17 --- > include/linux/device.h | 24 +++ > 7 files changed, 87 insertions(+), 61 deletions(-) > > -- > 2.18.0 > -- With Best Regards, Andy Shevchenko
Re: Nothing in /sys/class/udc
On Tue, Sep 4, 2018 at 12:12 PM Felipe Balbi wrote: > > > Hi, > > Ranran writes: > > On Mon, Sep 3, 2018 at 4:50 PM Ranran wrote: > >> > >> On Mon, Sep 3, 2018 at 4:22 PM Ranran wrote: > >> > > >> > On Mon, Sep 3, 2018 at 3:40 PM Greg KH > >> > wrote: > >> > > > >> > > On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote: > >> > > > Hello, > >> > > > > >> > > > I try to add gadget configfs as described in: > >> > > > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt > >> > > > Yet, I find nothing in /sys/class/udc: > >> > > > > >> > > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al > >> > > > total 0 > >> > > > drwxr-xr-x 2 root root 0 Sep 3 00:30 . > >> > > > drwxr-xr-x 58 root root 0 Sep 3 00:30 .. > >> > > > > >> > > > I also don't have dwc2, but dwc3: > >> > > > user@user-VirtualBox:~/tegra$ lsmod | grep dw > >> > > > dwc3 90112 0 > >> > > > ulpi 16384 1 dwc3 > >> > > > udc_core 24576 2 dwc3,libcomposite > >> > > > user@user-VirtualBox:~/tegra$ > >> > > > > >> > > > Kernel is 4.4.50. > >> > > > >> > > Why do you think the dwc3 driver will work here? Do you really have > >> > > this hardware being emulated? > >> > > > >> > Hi, > >> > > >> > I check it with Intel's E3900. > >> > In datasheet, I can't find it being described in details in datasheet, > >> > yet being linked with another document ("see Section Little-Endian and > >> > Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" ) > >> > https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf > >> > I have installed module dwc3 and also dwc2 (both compiled as modules), > >> > but nothing appear in /sys/class/udc > >> > I also tried to add all options in device-drivers->usb->usb > >> > gadget->usb peripheral control into kernel, (not as modules, but built > >> > into kernel). > >> > Yet, on checking > >> > ls /sys/class/udc > >> > I only find dummy_udc.0 , and nothing else. > >> > > >> > >> I suspect that there is no device controller supported with this com > >> express (congatec type 10), probably this is the only explanations for > >> the empty /sys/class/udc. > >> > > > > A fix for my last comment: > > usb device controller should be supported with conga-MA5 (contains > > Intel's appolo lake), so I am still not sure why I find /sys/class/udc > > empty. > > In congatec MA-5 document it is said: > > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express > > connector including one USB 2.0 Dual Role port. The xHCI host > > controller in the SoC complies with USB standard 1.1 and 2.0 with > > high-speed, full-speed and low-speed USB signalling. Note USB Dual > > Role is only supported under Linux. The port is a standard USB Host > > port under Windows." > > Maybe some configuration in my .config is missing ? (I enabled all > > controllers in device-drivers->usb->usb-> gadget->usb peripheral > > control , so I am not sure what is still missing) > > do you have the PCI device enabled? what do you get with lspci - ? > Yes, here it is: 00:00.0 Host bridge [0600]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series Host Bridge [8086:5af0] (rev 0b) 00:02.0 VGA compatible controller [0300]: Intel Corporation Device [8086:5a85] (rev 0b) 00:0e.0 Audio device [0403]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series Audio Cluster [8086:5a98] (rev 0b) 00:0f.0 Communication controller [0780]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series Trusted Execution Engine [8086:5a9a] (rev 0b) 00:12.0 SATA controller [0106]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series SATA AHCI Controller [8086:5ae3] (rev 0b) 00:13.0 PCI bridge [0604]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series PCI Express Port A #1 [8086:5ad8] (rev fb) 00:14.0 PCI bridge [0604]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series PCI Express Port B #1 [8086:5ad6] (rev fb) 00:15.0 USB controller [0c03]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series USB xHCI [8086:5aa8] (rev 0b) 00:1b.0 SD Host controller [0805]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series SDXC/MMC Host Controller [8086:5aca] (rev 0b) 00:1c.0 SD Host controller [0805]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series eMMC Controller [8086:5acc] (rev 0b) 00:1f.0 ISA bridge [0601]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series Low Pin Count Interface [8086:5ae8] (rev 0b) 00:1f.1 SMBus [0c05]: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series SMBus Controller [8086:5ad4] (rev 0b) 02:00.0 Ethernet controller [0200]: Intel Corporation I210 Gigabit Network Connection [8086:157b] (rev 03) Thank you, ranran > -- > balbi
gadgetfs & endpoints
Hello, In gadgetfs documentation I find no mention of endpoints: https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt Is it possible to create endpoints using gadgetfs ? Thank you, ranran
[PATCH v3 05/10] platform: x86: intel_cht_int33fe: Register all connections at once
We can register all device connection descriptors with a single call to device_connections_add(). Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index 39d4100c60a2..b0cef48f77af 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -34,7 +34,7 @@ struct cht_int33fe_data { struct i2c_client *fusb302; struct i2c_client *pi3usb30532; /* Contain a list-head must be per device */ - struct device_connection connections[3]; + struct device_connection connections[4]; }; /* @@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; data->connections[2].id = "usb-role-switch"; - device_connection_add(>connections[0]); - device_connection_add(>connections[1]); - device_connection_add(>connections[2]); + device_connections_add(data->connections); memset(_info, 0, sizeof(board_info)); strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE); @@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client) if (data->max17047) i2c_unregister_device(data->max17047); - device_connection_remove(>connections[2]); - device_connection_remove(>connections[1]); - device_connection_remove(>connections[0]); + device_connections_remove(data->connections); return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */ } @@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c) if (data->max17047) i2c_unregister_device(data->max17047); - device_connection_remove(>connections[2]); - device_connection_remove(>connections[1]); - device_connection_remove(>connections[0]); + device_connections_remove(data->connections); return 0; } -- 2.18.0
[PATCH v3 09/10] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes
USB Type-C class driver now expects the muxes to be always assigned to the ports and not controllers, so the connections for the mux and fusb302 can be removed. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 18 -- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index 419ce8c8ffb5..a26f410800c2 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -34,7 +34,7 @@ struct cht_int33fe_data { struct i2c_client *fusb302; struct i2c_client *pi3usb30532; /* Contain a list-head must be per device */ - struct device_connection connections[8]; + struct device_connection connections[5]; }; /* @@ -174,29 +174,19 @@ static int cht_int33fe_probe(struct i2c_client *client) return -EPROBE_DEFER; /* Wait for i2c-adapter to load */ } - data->connections[0].endpoint[0] = "i2c-fusb302"; + data->connections[0].endpoint[0] = "port0"; data->connections[0].endpoint[1] = "i2c-pi3usb30532"; data->connections[0].id = "typec-switch"; - data->connections[1].endpoint[0] = "i2c-fusb302"; + data->connections[1].endpoint[0] = "port0"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; data->connections[1].id = "typec-mux"; - data->connections[2].endpoint[0] = "i2c-fusb302"; + data->connections[2].endpoint[0] = "port0"; data->connections[2].endpoint[1] = "i2c-pi3usb30532"; data->connections[2].id = "idff01m01"; data->connections[3].endpoint[0] = "i2c-fusb302"; data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch"; data->connections[3].id = "usb-role-switch"; - data->connections[4].endpoint[0] = "port0"; - data->connections[4].endpoint[1] = "i2c-pi3usb30532"; - data->connections[4].id = "typec-switch"; - data->connections[5].endpoint[0] = "port0"; - data->connections[5].endpoint[1] = "i2c-pi3usb30532"; - data->connections[5].id = "typec-mux"; - data->connections[6].endpoint[0] = "port0"; - data->connections[6].endpoint[1] = "i2c-pi3usb30532"; - data->connections[6].id = "idff01m01"; - device_connections_add(data->connections); memset(_info, 0, sizeof(board_info)); -- 2.18.0
[PATCH v3 04/10] drivers: base: Helpers for adding device connection descriptions
Introducing helpers for adding and removing multiple device connection descriptions at once. Signed-off-by: Heikki Krogerus --- include/linux/device.h | 24 1 file changed, 24 insertions(+) diff --git a/include/linux/device.h b/include/linux/device.h index 8f882549edee..3f1066a9e1c3 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, const char *con_id); void device_connection_add(struct device_connection *con); void device_connection_remove(struct device_connection *con); +/** + * device_connections_add - Add multiple device connections at once + * @cons: Zero terminated array of device connection descriptors + */ +static inline void device_connections_add(struct device_connection *cons) +{ + struct device_connection *c; + + for (c = cons; c->endpoint[0]; c++) + device_connection_add(c); +} + +/** + * device_connections_remove - Remove multiple device connections at once + * @cons: Zero terminated array of device connection descriptors + */ +static inline void device_connections_remove(struct device_connection *cons) +{ + struct device_connection *c; + + for (c = cons; c->endpoint[0]; c++) + device_connection_remove(c); +} + /** * enum device_link_state - Device link states. * @DL_STATE_NONE: The presence of the drivers is not being tracked. -- 2.18.0
[PATCH v3 08/10] usb: typec: class: Don't use port parent for getting mux handles
It is not possible to use the parent of the port device when requesting mux handles as the parent may be a multiport USB Type-C or PD controller. The muxes must be assigned to the ports, not the controllers. This will also move the requesting of the muxes after the port device is initialized. Signed-off-by: Heikki Krogerus --- drivers/usb/typec/class.c | 38 +++--- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index c202975f8097..5b969339d1b3 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port, sprintf(id, "id%04xm%02x", desc->svid, desc->mode); - mux = typec_mux_get(port->dev.parent, id); + mux = typec_mux_get(>dev, id); if (IS_ERR(mux)) return ERR_CAST(mux); @@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device *parent, return ERR_PTR(id); } - port->sw = typec_switch_get(cap->fwnode ? >dev : parent); - if (IS_ERR(port->sw)) { - ret = PTR_ERR(port->sw); - goto err_switch; - } - - port->mux = typec_mux_get(parent, "typec-mux"); - if (IS_ERR(port->mux)) { - ret = PTR_ERR(port->mux); - goto err_mux; - } - switch (cap->type) { case TYPEC_PORT_SRC: port->pwr_role = TYPEC_SOURCE; @@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device *parent, port->port_type = cap->type; port->prefer_role = cap->prefer_role; + device_initialize(>dev); port->dev.class = typec_class; port->dev.parent = parent; port->dev.fwnode = cap->fwnode; port->dev.type = _port_dev_type; dev_set_name(>dev, "port%d", id); - ret = device_register(>dev); + port->sw = typec_switch_get(>dev); + if (IS_ERR(port->sw)) { + put_device(>dev); + return ERR_CAST(port->sw); + } + + port->mux = typec_mux_get(>dev, "typec-mux"); + if (IS_ERR(port->mux)) { + put_device(>dev); + return ERR_CAST(port->mux); + } + + ret = device_add(>dev); if (ret) { dev_err(parent, "failed to register port (%d)\n", ret); put_device(>dev); @@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device *parent, } return port; - -err_mux: - typec_switch_put(port->sw); - -err_switch: - ida_simple_remove(_index_ida, port->id); - kfree(port); - - return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(typec_register_port); -- 2.18.0
[PATCH v3 07/10] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port
Assigning the mux to the USB Type-C port on top of fusb302. That will prepare this driver for the change in the USB Type-C class code, where the class driver will assume the muxes to be always assigned to the ports and not the controllers. Once the USB Type-C class driver has been updated, the connections between the mux and fusb302 can be dropped. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index 424064187124..419ce8c8ffb5 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -34,7 +34,7 @@ struct cht_int33fe_data { struct i2c_client *fusb302; struct i2c_client *pi3usb30532; /* Contain a list-head must be per device */ - struct device_connection connections[5]; + struct device_connection connections[8]; }; /* @@ -187,6 +187,16 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch"; data->connections[3].id = "usb-role-switch"; + data->connections[4].endpoint[0] = "port0"; + data->connections[4].endpoint[1] = "i2c-pi3usb30532"; + data->connections[4].id = "typec-switch"; + data->connections[5].endpoint[0] = "port0"; + data->connections[5].endpoint[1] = "i2c-pi3usb30532"; + data->connections[5].id = "typec-mux"; + data->connections[6].endpoint[0] = "port0"; + data->connections[6].endpoint[1] = "i2c-pi3usb30532"; + data->connections[6].id = "idff01m01"; + device_connections_add(data->connections); memset(_info, 0, sizeof(board_info)); -- 2.18.0
[PATCH v3 10/10] usb: typec: fusb302: reorganizing the probe function a little
The debugfs needs to be initialized as the last step in probe in this case. The struct dentry *rootdir can't be pointing to anything unless driver probe really finishes successfully. It is also not necessary to clear the i2c clientdata if the probe fails, so removing the extra label used for that. Signed-off-by: Heikki Krogerus --- drivers/usb/typec/fusb302/fusb302.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c index 82bed9810be6..5f5864273e16 100644 --- a/drivers/usb/typec/fusb302/fusb302.c +++ b/drivers/usb/typec/fusb302/fusb302.c @@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client, return -ENOMEM; chip->i2c_client = client; - i2c_set_clientdata(client, chip); chip->dev = >dev; chip->tcpc_config = fusb302_tcpc_config; chip->tcpc_dev.config = >tcpc_config; @@ -1756,22 +1755,17 @@ static int fusb302_probe(struct i2c_client *client, return -EPROBE_DEFER; } - fusb302_debugfs_init(chip); + chip->vbus = devm_regulator_get(chip->dev, "vbus"); + if (IS_ERR(chip->vbus)) + return PTR_ERR(chip->vbus); chip->wq = create_singlethread_workqueue(dev_name(chip->dev)); - if (!chip->wq) { - ret = -ENOMEM; - goto clear_client_data; - } + if (!chip->wq) + return -ENOMEM; + INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work); init_tcpc_dev(>tcpc_dev); - chip->vbus = devm_regulator_get(chip->dev, "vbus"); - if (IS_ERR(chip->vbus)) { - ret = PTR_ERR(chip->vbus); - goto destroy_workqueue; - } - if (client->irq) { chip->gpio_int_n_irq = client->irq; } else { @@ -1797,15 +1791,15 @@ static int fusb302_probe(struct i2c_client *client, goto tcpm_unregister_port; } enable_irq_wake(chip->gpio_int_n_irq); + fusb302_debugfs_init(chip); + i2c_set_clientdata(client, chip); + return ret; tcpm_unregister_port: tcpm_unregister_port(chip->tcpm_port); destroy_workqueue: destroy_workqueue(chip->wq); -clear_client_data: - i2c_set_clientdata(client, NULL); - fusb302_debugfs_exit(chip); return ret; } @@ -1816,7 +1810,6 @@ static int fusb302_remove(struct i2c_client *client) tcpm_unregister_port(chip->tcpm_port); destroy_workqueue(chip->wq); - i2c_set_clientdata(client, NULL); fusb302_debugfs_exit(chip); return 0; -- 2.18.0
[PATCH v3 06/10] platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
Adding a connection for the DisplayPort alternate mode. PI3USB30532 is used for muxing the port to DisplayPort on CHT platforms. The connection allows the alternate mode device to get handle to the mux, and therefore make it possible to use the USB Type-C connector as DisplayPort. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index b0cef48f77af..424064187124 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -34,7 +34,7 @@ struct cht_int33fe_data { struct i2c_client *fusb302; struct i2c_client *pi3usb30532; /* Contain a list-head must be per device */ - struct device_connection connections[4]; + struct device_connection connections[5]; }; /* @@ -181,8 +181,11 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[1].endpoint[1] = "i2c-pi3usb30532"; data->connections[1].id = "typec-mux"; data->connections[2].endpoint[0] = "i2c-fusb302"; - data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; - data->connections[2].id = "usb-role-switch"; + data->connections[2].endpoint[1] = "i2c-pi3usb30532"; + data->connections[2].id = "idff01m01"; + data->connections[3].endpoint[0] = "i2c-fusb302"; + data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch"; + data->connections[3].id = "usb-role-switch"; device_connections_add(data->connections); -- 2.18.0
[PATCH v3 03/10] platform: x86: intel_cht_int33fe: Add dependency on muxes
The connections create clear dependency on the muxes. fusb302 fails to probe unless we have the mux drivers available. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0c1aa6c314f5..bdac939de223 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE tristate "Intel Cherry Trail ACPI INT33FE Driver" depends on X86 && ACPI && I2C && REGULATOR depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m) + depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m) + depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m) ---help--- This driver add support for the INT33FE ACPI device found on some Intel Cherry Trail devices. -- 2.18.0
[PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT
Hi, These patches will introduce a few improvements to the USB Type-C support on Intel CHT platform. In this series I'm preparing Intel CHT mux handling for DisplayPort Alt Mode support, but please note that, after these patches the DisplayPort alternate mode will still not work out of the box on CHT platform. Changes to the fusb302.c, and possibly tcpm.c are still needed. This version will only fix the issue Hans pointed out in v2. Instead of replacing the connection that assigned the mux to the USB Type-C port with a connection that assigns the mux to the alternate mode device, we keep all the existing connections and add a new one for the alternate mode device. That way USB3 devices continue to be enumerated as USB3 devices instead of USB2 devices. The origin thread can be read here: https://www.spinics.net/lists/linux-usb/msg172033.html Heikki Krogerus (10): usb: typec: Take care of driver module reference counting usb: roles: Handle driver reference counting platform: x86: intel_cht_int33fe: Add dependency on muxes drivers: base: Helpers for adding device connection descriptions platform: x86: intel_cht_int33fe: Register all connections at once platform: x86: intel_cht_int33fe: Add connection for the DP alt mode platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port usb: typec: class: Don't use port parent for getting mux handles platform: x86: intel_cht_int33fe: Remove the old connections for the muxes usb: typec: fusb302: reorganizing the probe function a little drivers/platform/x86/Kconfig | 2 ++ drivers/platform/x86/intel_cht_int33fe.c | 27 - drivers/usb/common/roles.c | 15 -- drivers/usb/typec/class.c| 38 ++-- drivers/usb/typec/fusb302/fusb302.c | 25 ++-- drivers/usb/typec/mux.c | 17 --- include/linux/device.h | 24 +++ 7 files changed, 87 insertions(+), 61 deletions(-) -- 2.18.0
[PATCH v3 01/10] usb: typec: Take care of driver module reference counting
Functions typec_mux_get() and typec_switch_get() already make sure that the mux device reference count is incremented, but the same must be done to the driver module as well to prevent the drivers from being unloaded in the middle of operation. This fixes a potential "BUG: unable to handle kernel paging request at ..." from happening. Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function parameter") Cc: Signed-off-by: Heikki Krogerus --- drivers/usb/typec/mux.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c index ddaac63ecf12..d990aa510fab 100644 --- a/drivers/usb/typec/mux.c +++ b/drivers/usb/typec/mux.c @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev) mutex_lock(_lock); sw = device_connection_find_match(dev, "typec-switch", NULL, typec_switch_match); - if (!IS_ERR_OR_NULL(sw)) + if (!IS_ERR_OR_NULL(sw)) { + WARN_ON(!try_module_get(sw->dev->driver->owner)); get_device(sw->dev); + } mutex_unlock(_lock); return sw; @@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get); */ void typec_switch_put(struct typec_switch *sw) { - if (!IS_ERR_OR_NULL(sw)) + if (!IS_ERR_OR_NULL(sw)) { + module_put(sw->dev->driver->owner); put_device(sw->dev); + } } EXPORT_SYMBOL_GPL(typec_switch_put); @@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const char *name) mutex_lock(_lock); mux = device_connection_find_match(dev, name, NULL, typec_mux_match); - if (!IS_ERR_OR_NULL(mux)) + if (!IS_ERR_OR_NULL(mux)) { + WARN_ON(!try_module_get(mux->dev->driver->owner)); get_device(mux->dev); + } mutex_unlock(_lock); return mux; @@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get); */ void typec_mux_put(struct typec_mux *mux) { - if (!IS_ERR_OR_NULL(mux)) + if (!IS_ERR_OR_NULL(mux)) { + module_put(mux->dev->driver->owner); put_device(mux->dev); + } } EXPORT_SYMBOL_GPL(typec_mux_put); -- 2.18.0
[PATCH v3 02/10] usb: roles: Handle driver reference counting
This fixes potential "BUG: unable to handle kernel paging request at ..." from happening. Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches") Cc: Signed-off-by: Heikki Krogerus --- drivers/usb/common/roles.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c index 15cc76e22123..3d8a776e55ee 100644 --- a/drivers/usb/common/roles.c +++ b/drivers/usb/common/roles.c @@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct device_connection *con, int ep, */ struct usb_role_switch *usb_role_switch_get(struct device *dev) { - return device_connection_find_match(dev, "usb-role-switch", NULL, - usb_role_switch_match); + struct usb_role_switch *sw; + + sw = device_connection_find_match(dev, "usb-role-switch", NULL, + usb_role_switch_match); + + if (!IS_ERR(sw)) + WARN_ON(!try_module_get(sw->dev.parent->driver->owner)); + + return sw; } EXPORT_SYMBOL_GPL(usb_role_switch_get); @@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get); */ void usb_role_switch_put(struct usb_role_switch *sw) { - if (!IS_ERR_OR_NULL(sw)) + if (!IS_ERR_OR_NULL(sw)) { put_device(>dev); + module_put(sw->dev.parent->driver->owner); + } } EXPORT_SYMBOL_GPL(usb_role_switch_put); -- 2.18.0
Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17
Hi, thank you for quick answer. i will try it, but afaik git bisect resets full git tree to the commit, right? i need to do some adjustments to get system booting (config,swap mmc in dts). is there another way to show more debug-messages? i have a working 4.16-tree, maybe i can merge newer versions (go forward from it instead of backwards). regards frank Gesendet: Dienstag, 04. September 2018 um 07:26 Uhr Von: "Greg KH" An: "Frank Wunderlich" Cc: linux-usb@vger.kernel.org Betreff: Re: Fw: keyboard-problem on bpi-r2 since 4.17 On Tue, Sep 04, 2018 at 06:39:20AM +0200, Frank Wunderlich wrote: > > > > Hi, > > i have problems with usb-keyboard on bananapi-r2 since 4.17. same keyboard > works till 4.16. > In 4.19-rc1 same issue occours. Keyboard is recognized and on keypress it is > disconnected > and connected again without anything written to console. > > dmesg (printk-debuglevel=8) looks like this: > > [ 77.292068] usb 1-1: USB disconnect, device number 2 > [ 77.292068] usb 1-1: USB disconnect, device number 2 > [ 77.472554] evbug: Disconnected device: input0 > [ 77.472554] evbug: Disconnected device: input0 > [ 77.632390] evbug: Disconnected device: input1 > [ 77.632390] evbug: Disconnected device: input1 > [ 77.773255] evbug: Disconnected device: input2 > [ 77.773255] evbug: Disconnected device: input2 > [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk > [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk > [ 78.545576] input: USB USB Keykoard as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 > [ 78.545576] input: USB USB Keykoard as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 > [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at > usb-1a1c.usb-1/input0) > [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID > v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 > [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at > usb-1a1c.usb-1/input0) > [ 78.655044] input: USB USB Keykoard Consumer Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 > [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID > v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 > [ 78.655044] input: USB USB Keykoard Consumer Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 > [ 78.734340] evbug: Connected device: input5 (USB USB Keykoard > Consumer Control at usb-1a1c.usb-1/input1) > [ 78.746118] input: USB USB Keykoard System Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 > [ 78.734340] e[ 78.760069] evbug: Connected device: input6 (USB USB > Keykoard System Control at usb-1a1c.usb-1/input1) > vbug: Connected [ 78.770893] hid-generic 0003:1C4F:0002.0004: > input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on > usb-1a1c.usb-1/inpu > t1 > device: input5 (USB USB Keykoard Consumer Control at > usb-1a1c.usb-1/input1) > [ 78.746118] input: USB USB Keykoard System Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 > [ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System > Control at usb-1a1c.usb-1/input1) > [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID > v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/input1 > > can i debug this further? > > i see that mtk_xhci-driver was rewritten to use new api > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/host/xhci-mtk.c?h=v4.17-rc1=6ae9f5062aa6f5a301c16715c601c05bc9aa450e > > maybe this is the reason, but i don't know how to debug it. i tried > disabling acpi and apic via cmdline to exclude anything acpi-related. > > maybe you have an idea... Can you use 'git bisect' to track down the offending commit? thanks, greg k-h
Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space
On Tue, Sep 04, 2018 at 12:21:09PM +0200, Oliver Neukum wrote: > On Di, 2018-09-04 at 11:31 +0200, Johan Hovold wrote: > > On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote: > > > For those people who run with panic_on_warn a WARN() triggered > > > from user space is a DOS. It is worth returning to dev_err() > > > > I think this should be dev_warn() unless you want to bring back the > > returning of errors on these conditions as well (i.e. as was the case > > prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control > > flow")). > > Should I? A warning in syslog is pretty hardcore, so I have no idea > whether dev_warn() is enough. Perhaps there are two sides to this. If something really should not be happening and needs to be addressed (i.e. it's a driver bug) that dev_WARN is warranted. If user space can be pass in bogus flags that gets propagated to USB core, perhaps those need to be sanitised sooner (in the vain of "don't trust anything coming from user space"). In general though, I believe dev_warn() is more appropriate for cases where something odd is happening, but we try to recover and proceed anyway (e.g. by sanitising the flags without bailing out as is the case here) instead of aborting. Johan
Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space
On Di, 2018-09-04 at 11:31 +0200, Johan Hovold wrote: > On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote: > > For those people who run with panic_on_warn a WARN() triggered > > from user space is a DOS. It is worth returning to dev_err() > > I think this should be dev_warn() unless you want to bring back the > returning of errors on these conditions as well (i.e. as was the case > prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control > flow")). Should I? A warning in syslog is pretty hardcore, so I have no idea whether dev_warn() is enough. Regards Oliver
Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space
On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote: > For those people who run with panic_on_warn a WARN() triggered > from user space is a DOS. It is worth returning to dev_err() I think this should be dev_warn() unless you want to bring back the returning of errors on these conditions as well (i.e. as was the case prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control flow")). > Signed-off-by: Oliver Neukum > Fixes: 0cb54a3e47cb4baf0bc7463f0a64cfeae5e35697 This should be: Fixes: 0cb54a3e47cb ("USB: debugging code shouldn't alter control flow") > Reported-by: syzbot+843efa30c8821bd69...@syzkaller.appspotmail.com > --- > drivers/usb/core/urb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c > index f51750bcd152..3fe65a774e6c 100644 > --- a/drivers/usb/core/urb.c > +++ b/drivers/usb/core/urb.c > @@ -475,7 +475,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) > > /* Check that the pipe's type matches the endpoint's type */ > if (usb_urb_ep_type_check(urb)) > - dev_WARN(>dev, "BOGUS urb xfer, pipe %x != type %x\n", > + dev_err(>dev, "BOGUS urb xfer, pipe %x != type %x\n", > usb_pipetype(urb->pipe), pipetypes[xfertype]); > > /* Check against a simple/standard policy */ > @@ -499,7 +499,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) > > /* warn if submitter gave bogus flags */ > if (allowed != urb->transfer_flags) > - dev_WARN(>dev, "BOGUS urb flags, %x --> %x\n", > + dev_err(>dev, "BOGUS urb flags, %x --> %x\n", > urb->transfer_flags, allowed); > > /* Johan
Re: Nothing in /sys/class/udc
Hi, Ranran writes: > On Mon, Sep 3, 2018 at 4:50 PM Ranran wrote: >> >> On Mon, Sep 3, 2018 at 4:22 PM Ranran wrote: >> > >> > On Mon, Sep 3, 2018 at 3:40 PM Greg KH wrote: >> > > >> > > On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote: >> > > > Hello, >> > > > >> > > > I try to add gadget configfs as described in: >> > > > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt >> > > > Yet, I find nothing in /sys/class/udc: >> > > > >> > > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al >> > > > total 0 >> > > > drwxr-xr-x 2 root root 0 Sep 3 00:30 . >> > > > drwxr-xr-x 58 root root 0 Sep 3 00:30 .. >> > > > >> > > > I also don't have dwc2, but dwc3: >> > > > user@user-VirtualBox:~/tegra$ lsmod | grep dw >> > > > dwc3 90112 0 >> > > > ulpi 16384 1 dwc3 >> > > > udc_core 24576 2 dwc3,libcomposite >> > > > user@user-VirtualBox:~/tegra$ >> > > > >> > > > Kernel is 4.4.50. >> > > >> > > Why do you think the dwc3 driver will work here? Do you really have >> > > this hardware being emulated? >> > > >> > Hi, >> > >> > I check it with Intel's E3900. >> > In datasheet, I can't find it being described in details in datasheet, >> > yet being linked with another document ("see Section Little-Endian and >> > Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" ) >> > https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf >> > I have installed module dwc3 and also dwc2 (both compiled as modules), >> > but nothing appear in /sys/class/udc >> > I also tried to add all options in device-drivers->usb->usb >> > gadget->usb peripheral control into kernel, (not as modules, but built >> > into kernel). >> > Yet, on checking >> > ls /sys/class/udc >> > I only find dummy_udc.0 , and nothing else. >> > >> >> I suspect that there is no device controller supported with this com >> express (congatec type 10), probably this is the only explanations for >> the empty /sys/class/udc. >> > > A fix for my last comment: > usb device controller should be supported with conga-MA5 (contains > Intel's appolo lake), so I am still not sure why I find /sys/class/udc > empty. > In congatec MA-5 document it is said: > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express > connector including one USB 2.0 Dual Role port. The xHCI host > controller in the SoC complies with USB standard 1.1 and 2.0 with > high-speed, full-speed and low-speed USB signalling. Note USB Dual > Role is only supported under Linux. The port is a standard USB Host > port under Windows." > Maybe some configuration in my .config is missing ? (I enabled all > controllers in device-drivers->usb->usb-> gadget->usb peripheral > control , so I am not sure what is still missing) do you have the PCI device enabled? what do you get with lspci - ? -- balbi
[PATCH] USB: change dev_WARN to dev_err triggerable from user space
For those people who run with panic_on_warn a WARN() triggered from user space is a DOS. It is worth returning to dev_err() Signed-off-by: Oliver Neukum Fixes: 0cb54a3e47cb4baf0bc7463f0a64cfeae5e35697 Reported-by: syzbot+843efa30c8821bd69...@syzkaller.appspotmail.com --- drivers/usb/core/urb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c index f51750bcd152..3fe65a774e6c 100644 --- a/drivers/usb/core/urb.c +++ b/drivers/usb/core/urb.c @@ -475,7 +475,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) /* Check that the pipe's type matches the endpoint's type */ if (usb_urb_ep_type_check(urb)) - dev_WARN(>dev, "BOGUS urb xfer, pipe %x != type %x\n", + dev_err(>dev, "BOGUS urb xfer, pipe %x != type %x\n", usb_pipetype(urb->pipe), pipetypes[xfertype]); /* Check against a simple/standard policy */ @@ -499,7 +499,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags) /* warn if submitter gave bogus flags */ if (allowed != urb->transfer_flags) - dev_WARN(>dev, "BOGUS urb flags, %x --> %x\n", + dev_err(>dev, "BOGUS urb flags, %x --> %x\n", urb->transfer_flags, allowed); /* -- 2.16.4
Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection
Hi, On 04-09-18 09:44, Heikki Krogerus wrote: On Tue, Sep 04, 2018 at 09:40:35AM +0200, Hans de Goede wrote: HI, On 04-09-18 09:37, Heikki Krogerus wrote: Hi Hans, On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote: Hi, On 03-09-18 15:36, Heikki Krogerus wrote: PI3USB30532 is used for muxing the port to DisplayPort on CHT platforms, so changing the connection ID so that the mux will get assigned to the alternate mode device and not the port device. Connection ID "typec-mux" is now reserved for Accessory Modes. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index b0cef48f77af..f8c881f871f9 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[0].id = "typec-switch"; data->connections[1].endpoint[0] = "i2c-fusb302"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; - data->connections[1].id = "typec-mux"; + data->connections[1].id = "idff01m01"; Hmm, so the mux will start in open connection and needs to be moved from open to TYPEC_STATE_USB when doing USB3, I assume the alternative driver is only going to come into play when actually using a DP over Type-C capable device/dongle, so what will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ? Ah, for some reason I assumed that the orientation switch will take care of that, but of course it does not. So we'll keep the existing connections, and just add a new one for the alt mode device. Ok, so I assume you are going to send a v3 for this ? Yes. Let me know if there is any other problems. Otherwise the series looks good to me. Regards, Hans
Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection
On Tue, Sep 04, 2018 at 09:40:35AM +0200, Hans de Goede wrote: > HI, > > On 04-09-18 09:37, Heikki Krogerus wrote: > > Hi Hans, > > > > On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote: > > > Hi, > > > > > > On 03-09-18 15:36, Heikki Krogerus wrote: > > > > PI3USB30532 is used for muxing the port to DisplayPort on > > > > CHT platforms, so changing the connection ID so that the > > > > mux will get assigned to the alternate mode device and not > > > > the port device. Connection ID "typec-mux" is now reserved > > > > for Accessory Modes. > > > > > > > > Signed-off-by: Heikki Krogerus > > > > --- > > > >drivers/platform/x86/intel_cht_int33fe.c | 2 +- > > > >1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c > > > > b/drivers/platform/x86/intel_cht_int33fe.c > > > > index b0cef48f77af..f8c881f871f9 100644 > > > > --- a/drivers/platform/x86/intel_cht_int33fe.c > > > > +++ b/drivers/platform/x86/intel_cht_int33fe.c > > > > @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client > > > > *client) > > > > data->connections[0].id = "typec-switch"; > > > > data->connections[1].endpoint[0] = "i2c-fusb302"; > > > > data->connections[1].endpoint[1] = "i2c-pi3usb30532"; > > > > - data->connections[1].id = "typec-mux"; > > > > + data->connections[1].id = "idff01m01"; > > > > > > Hmm, so the mux will start in open connection and needs to > > > be moved from open to TYPEC_STATE_USB when doing USB3, I assume > > > the alternative driver is only going to come into play when > > > actually using a DP over Type-C capable device/dongle, so what > > > will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ? > > > > Ah, for some reason I assumed that the orientation switch will take > > care of that, but of course it does not. So we'll keep the existing > > connections, and just add a new one for the alt mode device. > > Ok, so I assume you are going to send a v3 for this ? Yes. Let me know if there is any other problems. Thanks, -- heikki
Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection
HI, On 04-09-18 09:37, Heikki Krogerus wrote: Hi Hans, On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote: Hi, On 03-09-18 15:36, Heikki Krogerus wrote: PI3USB30532 is used for muxing the port to DisplayPort on CHT platforms, so changing the connection ID so that the mux will get assigned to the alternate mode device and not the port device. Connection ID "typec-mux" is now reserved for Accessory Modes. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index b0cef48f77af..f8c881f871f9 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[0].id = "typec-switch"; data->connections[1].endpoint[0] = "i2c-fusb302"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; - data->connections[1].id = "typec-mux"; + data->connections[1].id = "idff01m01"; Hmm, so the mux will start in open connection and needs to be moved from open to TYPEC_STATE_USB when doing USB3, I assume the alternative driver is only going to come into play when actually using a DP over Type-C capable device/dongle, so what will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ? Ah, for some reason I assumed that the orientation switch will take care of that, but of course it does not. So we'll keep the existing connections, and just add a new one for the alt mode device. Ok, so I assume you are going to send a v3 for this ? Regards, Hans
Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection
Hi Hans, On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote: > Hi, > > On 03-09-18 15:36, Heikki Krogerus wrote: > > PI3USB30532 is used for muxing the port to DisplayPort on > > CHT platforms, so changing the connection ID so that the > > mux will get assigned to the alternate mode device and not > > the port device. Connection ID "typec-mux" is now reserved > > for Accessory Modes. > > > > Signed-off-by: Heikki Krogerus > > --- > > drivers/platform/x86/intel_cht_int33fe.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c > > b/drivers/platform/x86/intel_cht_int33fe.c > > index b0cef48f77af..f8c881f871f9 100644 > > --- a/drivers/platform/x86/intel_cht_int33fe.c > > +++ b/drivers/platform/x86/intel_cht_int33fe.c > > @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client) > > data->connections[0].id = "typec-switch"; > > data->connections[1].endpoint[0] = "i2c-fusb302"; > > data->connections[1].endpoint[1] = "i2c-pi3usb30532"; > > - data->connections[1].id = "typec-mux"; > > + data->connections[1].id = "idff01m01"; > > Hmm, so the mux will start in open connection and needs to > be moved from open to TYPEC_STATE_USB when doing USB3, I assume > the alternative driver is only going to come into play when > actually using a DP over Type-C capable device/dongle, so what > will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ? Ah, for some reason I assumed that the orientation switch will take care of that, but of course it does not. So we'll keep the existing connections, and just add a new one for the alt mode device. Sorry for this. Thanks, -- heikki
Re: [PATCH] usb/typec: fix kernel-doc notation warning for typec_match_altmode
On Mon, Sep 03, 2018 at 12:58:35PM -0700, Randy Dunlap wrote: > From: Randy Dunlap > > Fix kernel-doc warning for missing function parameter 'mode' description: > > ../drivers/usb/typec/bus.c:268: warning: Function parameter or member 'mode' > not described in 'typec_match_altmode' > > Also fix typos for same function documentation. > > Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes") > > Signed-off-by: Randy Dunlap > Cc: Heikki Krogerus Acked-by: Heikki Krogerus > --- > drivers/usb/typec/bus.c |7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > --- lnx-419-rc2.orig/drivers/usb/typec/bus.c > +++ lnx-419-rc2/drivers/usb/typec/bus.c > @@ -255,12 +255,13 @@ EXPORT_SYMBOL_GPL(typec_altmode_unregist > /* API for the port drivers */ > > /** > - * typec_match_altmode - Match SVID to an array of alternate modes > + * typec_match_altmode - Match SVID and mode to an array of alternate modes > * @altmodes: Array of alternate modes > - * @n: Number of elements in the array, or -1 for NULL termiated arrays > + * @n: Number of elements in the array, or -1 for NULL terminated arrays > * @svid: Standard or Vendor ID to match with > + * @mode: Mode to match with > * > - * Return pointer to an alternate mode with SVID mathing @svid, or NULL when > no > + * Return pointer to an alternate mode with SVID matching @svid, or NULL > when no > * match is found. > */ > struct typec_altmode *typec_match_altmode(struct typec_altmode **altmodes, -- heikki