Re: [PATCH] USB: serial: FTDI: Add device IDs for Sienna and Echelon PL-20
On Thu, Sep 05, 2019 at 12:26:20AM +0200, beni.mah...@gmx.net wrote: > From: Beni Mahler > > Both devices added here have a FTDI chip inside. The device from Echelon > is called 'Network Interface' it is actually a LON network gateway. > > ID 0403:8348 Future Technology Devices International, Ltd > > https://www.eltako.com/fileadmin/downloads/de/datenblatt/Datenblatt_PL-SW-PROF.pdf > > ID 0920:7500 Network Interface > https://www.echelon.com/products/u20-usb-network-interface It wasn't obvious to me that these two are essentially the same product (?) using different IDs so at first I wondered why you included them in the same patch. > Signed-off-by: Beni Mahler Now applied, thanks. Johan
Re: [PATCH v2 1/1] usb: serial: option: add Telit FN980 compositions
On Mon, Sep 23, 2019 at 12:23:28PM +0200, Daniele Palmas wrote: > This patch adds the following Telit FN980 compositions: > > 0x1050: tty, adb, rmnet, tty, tty, tty, tty > 0x1051: tty, adb, mbim, tty, tty, tty, tty > 0x1052: rndis, tty, adb, tty, tty, tty, tty > 0x1053: tty, adb, ecm, tty, tty, tty, tty > > Signed-off-by: Daniele Palmas > --- > Sorry, forgot to add Signed-off-by tag... > > v2: added Signed-off-by tag Now applied, thanks. Johan
[PATCH 1/2] ARM: dts: sunxi: Revert phy-names removal for ECHI and OHCI
This reverts commits 3d109bdca981 ("ARM: dts: sunxi: Remove useless phy-names from EHCI and OHCI"), 0a3df8bb6dad ("ARM: dts: sunxi: h3/h5: Remove useless phy-names from EHCI and OHCI") and 3c7ab90aaa28 ("arm64: dts: allwinner: Remove useless phy-names from EHCI and OHCI"). It turns out that while the USB bindings were not mentionning it, the PHY client bindings were mandating that phy-names is set when phys is. Let's add it back. Fixes: 3d109bdca981 ("ARM: dts: sunxi: Remove useless phy-names from EHCI and OHCI") Fixes: 0a3df8bb6dad ("ARM: dts: sunxi: h3/h5: Remove useless phy-names from EHCI and OHCI") Fixes: 3c7ab90aaa28 ("arm64: dts: allwinner: Remove useless phy-names from EHCI and OHCI") Reported-by: Emmanuel Vadot Signed-off-by: Maxime Ripard --- arch/arm/boot/dts/sun4i-a10.dtsi | 4 arch/arm/boot/dts/sun5i.dtsi | 2 ++ arch/arm/boot/dts/sun6i-a31.dtsi | 4 arch/arm/boot/dts/sun7i-a20.dtsi | 4 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 ++ arch/arm/boot/dts/sun8i-a83t.dtsi | 3 +++ arch/arm/boot/dts/sun8i-r40.dtsi | 4 arch/arm/boot/dts/sun9i-a80.dtsi | 5 + arch/arm/boot/dts/sunxi-h3-h5.dtsi| 6 ++ arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 2 ++ arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 2 ++ arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 ++ 12 files changed, 40 insertions(+) diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi index ce823c44e98a..4c268b70b735 100644 --- a/arch/arm/boot/dts/sun4i-a10.dtsi +++ b/arch/arm/boot/dts/sun4i-a10.dtsi @@ -520,6 +520,7 @@ interrupts = <39>; clocks = <&ccu CLK_AHB_EHCI0>; phys = <&usbphy 1>; + phy-names = "usb"; status = "disabled"; }; @@ -529,6 +530,7 @@ interrupts = <64>; clocks = <&ccu CLK_USB_OHCI0>, <&ccu CLK_AHB_OHCI0>; phys = <&usbphy 1>; + phy-names = "usb"; status = "disabled"; }; @@ -608,6 +610,7 @@ interrupts = <40>; clocks = <&ccu CLK_AHB_EHCI1>; phys = <&usbphy 2>; + phy-names = "usb"; status = "disabled"; }; @@ -617,6 +620,7 @@ interrupts = <65>; clocks = <&ccu CLK_USB_OHCI1>, <&ccu CLK_AHB_OHCI1>; phys = <&usbphy 2>; + phy-names = "usb"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi index cfb1efc8828c..6befa236ba99 100644 --- a/arch/arm/boot/dts/sun5i.dtsi +++ b/arch/arm/boot/dts/sun5i.dtsi @@ -391,6 +391,7 @@ interrupts = <39>; clocks = <&ccu CLK_AHB_EHCI>; phys = <&usbphy 1>; + phy-names = "usb"; status = "disabled"; }; @@ -400,6 +401,7 @@ interrupts = <40>; clocks = <&ccu CLK_USB_OHCI>, <&ccu CLK_AHB_OHCI>; phys = <&usbphy 1>; + phy-names = "usb"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi index bbeb743633c6..ac7638078420 100644 --- a/arch/arm/boot/dts/sun6i-a31.dtsi +++ b/arch/arm/boot/dts/sun6i-a31.dtsi @@ -545,6 +545,7 @@ clocks = <&ccu CLK_AHB1_EHCI0>; resets = <&ccu RST_AHB1_EHCI0>; phys = <&usbphy 1>; + phy-names = "usb"; status = "disabled"; }; @@ -555,6 +556,7 @@ clocks = <&ccu CLK_AHB1_OHCI0>, <&ccu CLK_USB_OHCI0>; resets = <&ccu RST_AHB1_OHCI0>; phys = <&usbphy 1>; + phy-names = "usb"; status = "disabled"; }; @@ -565,6 +567,7 @@ clocks = <&ccu CLK_AHB1_EHCI1>; resets = <&ccu RST_AHB1_EHCI1>; phys = <&usbphy 2>; + phy-names = "usb"; status = "disabled"; }; @@ -575,6 +578,7 @@ clocks = <&ccu CLK_AHB1_OHCI1>, <&ccu CLK_USB_OHCI1>; resets = <&ccu RST_AHB1_OHCI1>; phys = <&usbphy 2>; + phy-names = "usb"; status = "disabled";
[PATCH 2/2] dt-bindings: usb: Bring back phy-names
While the original bindings that were superseeded by the YAML schemas didn't mention that phy-names was needed, it turns out that phy-names is required if phys is set according to phy/phy-bindings.txt. Let's add back those properties. Fixes: 14ec072a19ad ("dt-bindings: usb: Convert USB HCD generic binding to YAML") Fixes: c93bcace1098 ("dt-bindings: usb: Convert the generic OHCI binding to YAML") Fixes: c3e2485d5f4f ("dt-bindings: usb: Convert the generic EHCI binding to YAML") Reported-by: Emmanuel Vadot Signed-off-by: Maxime Ripard --- Documentation/devicetree/bindings/usb/generic-ehci.yaml | 7 ++- Documentation/devicetree/bindings/usb/generic-ohci.yaml | 7 ++- Documentation/devicetree/bindings/usb/usb-hcd.yaml | 5 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml index 059f6ef1ad4a..1ca64c85191a 100644 --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml @@ -63,7 +63,11 @@ properties: description: Set this flag to force EHCI reset after resume. - phys: true + phys: +description: PHY specifier for the USB PHY + + phy-names: +const: usb required: - compatible @@ -89,6 +93,7 @@ examples: interrupts = <39>; clocks = <&ahb_gates 1>; phys = <&usbphy 1>; +phy-names = "usb"; }; ... diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml b/Documentation/devicetree/bindings/usb/generic-ohci.yaml index da5a14becbe5..bcffec1f1341 100644 --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml @@ -67,7 +67,11 @@ properties: description: Overrides the detected port count - phys: true + phys: +description: PHY specifier for the USB PHY + + phy-names: +const: usb required: - compatible @@ -84,6 +88,7 @@ examples: interrupts = <64>; clocks = <&usb_clk 6>, <&ahb_gates 2>; phys = <&usbphy 1>; + phy-names = "usb"; }; ... diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml index 9c8c56d3a792..7263b7f2b510 100644 --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml @@ -18,8 +18,13 @@ properties: description: List of all the USB PHYs on this HCD + phy-names: +description: + Name specifier for the USB PHY + examples: - | usb { phys = <&usb2_phy1>, <&usb3_phy1>; +phy-names = "usb"; }; -- 2.23.0
[bug report] usb: chipidea: imx: enable vbus and id wakeup only for OTG events
Hello Li Jun, The patch 15b80f7c3a7f: "usb: chipidea: imx: enable vbus and id wakeup only for OTG events" from Sep 9, 2019, leads to the following static checker warning: drivers/usb/chipidea/ci_hdrc_imx.c:438 ci_hdrc_imx_probe() warn: 'data->usbmisc_data' can also be NULL drivers/usb/chipidea/ci_hdrc_imx.c 317 data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); 318 if (!data) 319 return -ENOMEM; 320 321 data->plat_data = imx_platform_flag; 322 pdata.flags |= imx_platform_flag->flags; 323 platform_set_drvdata(pdev, data); 324 data->usbmisc_data = usbmisc_get_init_data(dev); ^^^ This can return NULL or error pointer. 325 if (IS_ERR(data->usbmisc_data)) 326 return PTR_ERR(data->usbmisc_data); 327 328 if ((of_usb_get_phy_mode(dev->of_node) == USBPHY_INTERFACE_MODE_HSIC) 329 && data->usbmisc_data) { ^^ This code checks for NULL. 330 pdata.flags |= CI_HDRC_IMX_IS_HSIC; 331 data->usbmisc_data->hsic = 1; 332 data->pinctrl = devm_pinctrl_get(dev); 333 if (IS_ERR(data->pinctrl)) { 334 dev_err(dev, "pinctrl get failed, err=%ld\n", 335 PTR_ERR(data->pinctrl)); 336 return PTR_ERR(data->pinctrl); 337 } 338 339 pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle"); 340 if (IS_ERR(pinctrl_hsic_idle)) { 341 dev_err(dev, 342 "pinctrl_hsic_idle lookup failed, err=%ld\n", 343 PTR_ERR(pinctrl_hsic_idle)); 344 return PTR_ERR(pinctrl_hsic_idle); 345 } 346 347 ret = pinctrl_select_state(data->pinctrl, pinctrl_hsic_idle); 348 if (ret) { 349 dev_err(dev, "hsic_idle select failed, err=%d\n", ret); 350 return ret; 351 } 352 353 data->pinctrl_hsic_active = pinctrl_lookup_state(data->pinctrl, 354 "active"); 355 if (IS_ERR(data->pinctrl_hsic_active)) { 356 dev_err(dev, 357 "pinctrl_hsic_active lookup failed, err=%ld\n", 358 PTR_ERR(data->pinctrl_hsic_active)); 359 return PTR_ERR(data->pinctrl_hsic_active); 360 } 361 362 data->hsic_pad_regulator = devm_regulator_get(dev, "hsic"); 363 if (PTR_ERR(data->hsic_pad_regulator) == -EPROBE_DEFER) { 364 return -EPROBE_DEFER; 365 } else if (PTR_ERR(data->hsic_pad_regulator) == -ENODEV) { 366 /* no pad regualator is needed */ 367 data->hsic_pad_regulator = NULL; 368 } else if (IS_ERR(data->hsic_pad_regulator)) { 369 dev_err(dev, "Get HSIC pad regulator error: %ld\n", 370 PTR_ERR(data->hsic_pad_regulator)); 371 return PTR_ERR(data->hsic_pad_regulator); 372 } 373 374 if (data->hsic_pad_regulator) { 375 ret = regulator_enable(data->hsic_pad_regulator); 376 if (ret) { 377 dev_err(dev, 378 "Failed to enable HSIC pad regulator\n"); 379 return ret; 380 } 381 } 382 } 383 384 if (pdata.flags & CI_HDRC_PMQOS) 385 pm_qos_add_request(&data->pm_qos_req, 386 PM_QOS_CPU_DMA_LATENCY, 0); 387 388 ret = imx_get_clks(dev); 389 if (ret) 390 goto disable_hsic_regulator; 391 392 ret = imx_prepare_enable_clks(dev); 393 if (ret) 394 goto disable_hsic_regulator; 395 396 data->phy = devm_usb_get_phy_by_phandle(dev, "fsl,usbphy", 0); 397 if (IS_ERR(data->phy)) { 398 ret = PTR_ERR(data->phy); 399 /* Return -EINVAL if no usbphy is available */ 400 if (ret == -ENODEV) 401 data->phy = NULL; 402 else 40
Regression: USB/xhci issues on some systems with newer kernel versions
Hi, There has been a regression in the xhci driver since kernel version 4.20, on some systems some usb devices won't work until the system gets rebooted. The error message in dmesg is "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state", although for some reason there are some usb devices that are affected by this issue but don't throw the error message(including the device I'm using, I got the error in previous kernel versions though). It seems like this bug can also lead to system instability, one user reported in the bug tracker(https://bugzilla.kernel.org/show_bug.cgi?id=202541#c58) that he got a system freeze because of this when using kernel 5.3.1. When looking at the responses in the bug tracker, it looks like it mostly affects Ryzen based systems with 300 series motherboards, although there are some other affected systems as well. It doesn't only affect wifi/bluetooth sticks, some users even got this issue when connecting their smartphone or their external hard drive to their PC. After enabling kernel debugging/tracing for xhci_hcd I got the following messages in dmesg(short version, link to the whole file below): [ 231.185635] xhci_hcd :38:00.4: xhci_get_isoc_frame_id: index 0, reg 0x1b29 start_frame_id 0x366, end_frame_id 0x6e4, start_frame 0x372 [ 231.185642] xhci_hcd :38:00.4: xhci_get_isoc_frame_id: index 1, reg 0x1b29 start_frame_id 0x366, end_frame_id 0x6e4, start_frame 0x373 [ 231.185646] xhci_hcd :38:00.4: xhci_get_isoc_frame_id: index 2, reg 0x1b29 start_frame_id 0x366, end_frame_id 0x6e4, start_frame 0x374 .. [ 231.887681] xhci_hcd :38:00.4: xhci_get_isoc_frame_id: index 4, reg 0x3119 start_frame_id 0x624, end_frame_id 0x1a2, start_frame 0x633 [ 231.887687] xhci_hcd :38:00.4: xhci_get_isoc_frame_id: index 5, reg 0x3119 start_frame_id 0x624, end_frame_id 0x1a2, start_frame 0x634 [ 231.892346] xhci_hcd :38:00.4: Cancel URB 8599ca58, dev 1, ep 0x1, starting at offset 0xff388ea0 [ 231.892355] xhci_hcd :38:00.4: // Ding dong! [ 231.892363] xhci_hcd :38:00.4: Cancel URB 0d35fd5d, dev 1, ep 0x1, starting at offset 0xff388ef0 [ 231.892368] xhci_hcd :38:00.4: Cancel URB 74e3ee88, dev 1, ep 0x1, starting at offset 0xff388e40 [ 231.892640] xhci_hcd :38:00.4: Stopped on Transfer TRB for slot 1 ep 1 [ 231.892647] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388ea0 (dma). [ 231.892651] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388eb0 (dma). [ 231.892653] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388ec0 (dma). [ 231.892656] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388ed0 (dma). [ 231.892658] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388ee0 (dma). [ 231.892661] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388ef0 (dma). [ 231.892663] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388f00 (dma). [ 231.892666] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388f10 (dma). [ 231.892668] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388f20 (dma). [ 231.892670] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388f30 (dma). [ 231.892672] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388f40 (dma). [ 231.892675] xhci_hcd :38:00.4: Removing canceled TD starting at 0xff388e90 (dma). [ 231.892677] xhci_hcd :38:00.4: Finding endpoint context [ 231.892679] xhci_hcd :38:00.4: Cycle state = 0x1 [ 231.892682] xhci_hcd :38:00.4: New dequeue segment = 5d174923 (virtual) [ 231.892685] xhci_hcd :38:00.4: New dequeue pointer = 0xff388ea0 (DMA) [ 231.892688] xhci_hcd :38:00.4: Set TR Deq Ptr cmd, new deq seg = 5d174923 (0xff388000 dma), new deq ptr = d5c5ed2a (0xff388ea0 dma), new cycle = 1 [ 231.892693] xhci_hcd :38:00.4: // Ding dong! [ 231.892728] xhci_hcd :38:00.4: Successful Set TR Deq Ptr cmd, deq = @ff388ea0 [ 231.897107] xhci_hcd :38:00.4: xhci_drop_endpoint called for udev 43fc1c1f [ 231.897126] xhci_hcd :38:00.4: drop ep 0x1, slot id 1, new drop flags = 0x4, new add flags = 0x0 [ 231.897129] xhci_hcd :38:00.4: xhci_check_bandwidth called for udev 43fc1c1f [ 231.897137] xhci_hcd :38:00.4: // Ding dong! [ 231.898523] xhci_hcd :38:00.4: Successful Endpoint Configure command I have uploaded the whole dmesg file and the tracing file to transfer.sh: https://transfer.sh/zYohl/dmesg and https://transfer.sh/KNbFL/xhci-trace The issues occur since commit f8f80be501aa2f10669585c3e328fad079d8cb3a "xhci: Use soft retry to recover faster from transaction errors". I think this commit should be reverted at least until a workaround has been found, especially since the next two kernel versions will be used by a lot of distributions(5.4 because it's a LTS kernel and 5.5 will probably be used in Ubuntu 20.04) so more users
[PATCH] usb: chipidea: tegra: clean up tegra_udc flag code
All Tegra devices handled by tegra-udc use the same flags. Consolidate all the entries under one roof. Signed-off-by: Peter Geis Acked-by: Thierry Reding --- drivers/usb/chipidea/ci_hdrc_tegra.c | 22 +- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c index 12025358bb3c..0c9911d44ee5 100644 --- a/drivers/usb/chipidea/ci_hdrc_tegra.c +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c @@ -24,35 +24,23 @@ struct tegra_udc_soc_info { unsigned long flags; }; -static const struct tegra_udc_soc_info tegra20_udc_soc_info = { - .flags = CI_HDRC_REQUIRES_ALIGNED_DMA, -}; - -static const struct tegra_udc_soc_info tegra30_udc_soc_info = { - .flags = CI_HDRC_REQUIRES_ALIGNED_DMA, -}; - -static const struct tegra_udc_soc_info tegra114_udc_soc_info = { - .flags = CI_HDRC_REQUIRES_ALIGNED_DMA, -}; - -static const struct tegra_udc_soc_info tegra124_udc_soc_info = { +static const struct tegra_udc_soc_info tegra_udc_soc_info = { .flags = CI_HDRC_REQUIRES_ALIGNED_DMA, }; static const struct of_device_id tegra_udc_of_match[] = { { .compatible = "nvidia,tegra20-udc", - .data = &tegra20_udc_soc_info, + .data = &tegra_udc_soc_info, }, { .compatible = "nvidia,tegra30-udc", - .data = &tegra30_udc_soc_info, + .data = &tegra_udc_soc_info, }, { .compatible = "nvidia,tegra114-udc", - .data = &tegra114_udc_soc_info, + .data = &tegra_udc_soc_info, }, { .compatible = "nvidia,tegra124-udc", - .data = &tegra124_udc_soc_info, + .data = &tegra_udc_soc_info, }, { /* sentinel */ } -- 2.17.1
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On Tue, Oct 01, 2019 at 06:08:17AM -0700, Guenter Roeck wrote: > On 10/1/19 2:48 AM, Heikki Krogerus wrote: > > Copying everything from struct typec_capability to struct > > typec_port during port registration. > > > What is the purpose of this patch ? To reduce the number of indirections at > runtime, or to avoid having to have cap around ? To get rid of the cap member. > FWIW, it looks like the code doesn't copy _all_ variables (eg cap->try_role), > and it doesn't drop port->cap. Am I missing something ? We can't drop port->cap at this point because the drivers still depend on it. This patch is the "prepare" phase of the series. The last patch in the series finally drops the member. I'll improve the commit message. > > Signed-off-by: Heikki Krogerus > > --- > > drivers/usb/typec/class.c | 55 +-- > > 1 file changed, 35 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > index 94a3eda62add..3835e2d9fba6 100644 > > --- a/drivers/usb/typec/class.c > > +++ b/drivers/usb/typec/class.c > > @@ -46,8 +46,14 @@ struct typec_port { > > enum typec_role vconn_role; > > enum typec_pwr_opmode pwr_opmode; > > enum typec_port_typeport_type; > > + enum typec_port_typefixed_role; > > + enum typec_port_dataport_roles; > > + enum typec_accessoryaccessory[TYPEC_MAX_ACCESSORY]; > > Would a pointer to cap->accessory be sufficient ? Or is there a reason to copy > the actual array ? No. The point is to get rid of the cap member. > > struct mutexport_type_lock; > > + u16 revision; > > + u16 pd_revision; > > + > > enum typec_orientation orientation; > > struct typec_switch *sw; > > struct typec_mux*mux; > > @@ -950,7 +956,7 @@ preferred_role_store(struct device *dev, struct > > device_attribute *attr, > > int role; > > int ret; > > - if (port->cap->type != TYPEC_PORT_DRP) { > > + if (port->fixed_role != TYPEC_PORT_DRP) { > > dev_dbg(dev, "Preferred role only supported with DRP ports\n"); > > return -EOPNOTSUPP; > > } > > @@ -982,7 +988,7 @@ preferred_role_show(struct device *dev, struct > > device_attribute *attr, > > { > > struct typec_port *port = to_typec_port(dev); > > - if (port->cap->type != TYPEC_PORT_DRP) > > + if (port->fixed_role != TYPEC_PORT_DRP) > > return 0; > > if (port->prefer_role < 0) > > @@ -1009,7 +1015,7 @@ static ssize_t data_role_store(struct device *dev, > > return ret; > > mutex_lock(&port->port_type_lock); > > - if (port->cap->data != TYPEC_PORT_DRD) { > > + if (port->port_roles != TYPEC_PORT_DRD) { > > ret = -EOPNOTSUPP; > > goto unlock_and_ret; > > } > > @@ -1029,7 +1035,7 @@ static ssize_t data_role_show(struct device *dev, > > { > > struct typec_port *port = to_typec_port(dev); > > - if (port->cap->data == TYPEC_PORT_DRD) > > + if (port->port_roles == TYPEC_PORT_DRD) > > return sprintf(buf, "%s\n", port->data_role == TYPEC_HOST ? > >"[host] device" : "host [device]"); > > @@ -1044,7 +1050,7 @@ static ssize_t power_role_store(struct device *dev, > > struct typec_port *port = to_typec_port(dev); > > int ret; > > - if (!port->cap->pd_revision) { > > + if (!port->pd_revision) { > > dev_dbg(dev, "USB Power Delivery not supported\n"); > > return -EOPNOTSUPP; > > } > > @@ -1064,9 +1070,9 @@ static ssize_t power_role_store(struct device *dev, > > return ret; > > mutex_lock(&port->port_type_lock); > > - if (port->port_type != TYPEC_PORT_DRP) { > > + if (port->fixed_role != TYPEC_PORT_DRP) { > > This is a semantic change: Previously, it compared the _current_ port type. > Now it compares the initial (fixed) port type. Is this on purpose ? > > [ comment written before I noticed the change below. See there. ] > > > dev_dbg(dev, "port type fixed at \"%s\"", > > -typec_port_power_roles[port->port_type]); > > +typec_port_power_roles[port->fixed_role]); > > ret = -EOPNOTSUPP; > > goto unlock_and_ret; > > } > > @@ -1086,7 +1092,7 @@ static ssize_t power_role_show(struct device *dev, > > { > > struct typec_port *port = to_typec_port(dev); > > - if (port->cap->type == TYPEC_PORT_DRP) > > + if (port->fixed_role == TYPEC_PORT_DRP) > > return sprintf(buf, "%s\n", port->pwr_role == TYPEC_SOURCE ? > >"[source] sink" : "source [sink]"); > > @@ -1102,7 +1108,7 @@ port_type_store(struct device *dev, struct > > device_attribute *attr, > > int ret; > > enum typec_port_type type; > > - if (!port->cap->port_type_set || port->cap->type
[PATCH v2 3/3] ARM: dts: exynos: Rename power domain nodes to "power-domain" in Exynos4
The device node name should reflect generic class of a device so rename power domain nodes to "power-domain". No functional change. Signed-off-by: Krzysztof Kozlowski --- arch/arm/boot/dts/exynos4.dtsi| 14 +++--- arch/arm/boot/dts/exynos4210.dtsi | 2 +- arch/arm/boot/dts/exynos4412.dtsi | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index 433f109d97ca..d2779a790ce3 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -111,28 +111,28 @@ syscon = <&pmu_system_controller>; }; - pd_mfc: mfc-power-domain@10023c40 { + pd_mfc: power-domain@10023c40 { compatible = "samsung,exynos4210-pd"; reg = <0x10023C40 0x20>; #power-domain-cells = <0>; label = "MFC"; }; - pd_g3d: g3d-power-domain@10023c60 { + pd_g3d: power-domain@10023c60 { compatible = "samsung,exynos4210-pd"; reg = <0x10023C60 0x20>; #power-domain-cells = <0>; label = "G3D"; }; - pd_lcd0: lcd0-power-domain@10023c80 { + pd_lcd0: power-domain@10023c80 { compatible = "samsung,exynos4210-pd"; reg = <0x10023C80 0x20>; #power-domain-cells = <0>; label = "LCD0"; }; - pd_tv: tv-power-domain@10023c20 { + pd_tv: power-domain@10023c20 { compatible = "samsung,exynos4210-pd"; reg = <0x10023C20 0x20>; #power-domain-cells = <0>; @@ -140,21 +140,21 @@ label = "TV"; }; - pd_cam: cam-power-domain@10023c00 { + pd_cam: power-domain@10023c00 { compatible = "samsung,exynos4210-pd"; reg = <0x10023C00 0x20>; #power-domain-cells = <0>; label = "CAM"; }; - pd_gps: gps-power-domain@10023ce0 { + pd_gps: power-domain@10023ce0 { compatible = "samsung,exynos4210-pd"; reg = <0x10023CE0 0x20>; #power-domain-cells = <0>; label = "GPS"; }; - pd_gps_alive: gps-alive-power-domain@10023d00 { + pd_gps_alive: power-domain@10023d00 { compatible = "samsung,exynos4210-pd"; reg = <0x10023D00 0x20>; #power-domain-cells = <0>; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index f220716239db..ff9a3fb21a85 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -90,7 +90,7 @@ }; }; - pd_lcd1: lcd1-power-domain@10023ca0 { + pd_lcd1: power-domain@10023ca0 { compatible = "samsung,exynos4210-pd"; reg = <0x10023CA0 0x20>; #power-domain-cells = <0>; diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index d20db2dfe8e2..1c40bd56ce00 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -206,7 +206,7 @@ }; }; - pd_isp: isp-power-domain@10023ca0 { + pd_isp: power-domain@10023ca0 { compatible = "samsung,exynos4210-pd"; reg = <0x10023CA0 0x20>; #power-domain-cells = <0>; -- 2.17.1
[PATCH v2 1/3] dt-bindings: power: Convert Generic Power Domain bindings to json-schema
Convert Generic Power Domain bindings to DT schema format using json-schema. The consumer bindings are split to separate file. Signed-off-by: Krzysztof Kozlowski --- Changes since v1: 1. Select all nodes for consumers, 2. Remove from consumers duplicated properties with dt-schema, 3. Fix power domain pattern, 4. Remove unneeded types. --- .../devicetree/bindings/arm/arm,scmi.txt | 2 +- .../devicetree/bindings/arm/arm,scpi.txt | 2 +- .../bindings/arm/freescale/fsl,scu.txt| 2 +- .../bindings/clock/clk-exynos-audss.txt | 2 +- .../bindings/clock/exynos5433-clock.txt | 4 +- .../bindings/clock/renesas,cpg-mssr.txt | 2 +- .../clock/renesas,r8a7778-cpg-clocks.txt | 2 +- .../clock/renesas,r8a7779-cpg-clocks.txt | 2 +- .../clock/renesas,rcar-gen2-cpg-clocks.txt| 2 +- .../bindings/clock/renesas,rz-cpg-clocks.txt | 2 +- .../bindings/clock/ti/davinci/psc.txt | 2 +- .../bindings/display/etnaviv/etnaviv-drm.txt | 2 +- .../devicetree/bindings/display/msm/dpu.txt | 2 +- .../devicetree/bindings/display/msm/mdp5.txt | 2 +- .../devicetree/bindings/dsp/fsl,dsp.yaml | 2 +- .../firmware/nvidia,tegra186-bpmp.txt | 2 +- .../bindings/media/imx7-mipi-csi2.txt | 3 +- .../bindings/media/mediatek-jpeg-decoder.txt | 3 +- .../bindings/media/mediatek-mdp.txt | 3 +- .../bindings/opp/qcom-nvmem-cpufreq.txt | 2 +- .../devicetree/bindings/pci/pci-keystone.txt | 2 +- .../bindings/phy/ti,phy-am654-serdes.txt | 2 +- .../bindings/power/amlogic,meson-gx-pwrc.txt | 2 +- .../devicetree/bindings/power/fsl,imx-gpc.txt | 2 +- .../bindings/power/fsl,imx-gpcv2.txt | 2 +- .../power/power-domain-consumers.yaml | 105 + .../bindings/power/power-domain.yaml | 134 .../bindings/power/power_domain.txt | 205 -- .../devicetree/bindings/power/qcom,rpmpd.txt | 2 +- .../bindings/power/renesas,rcar-sysc.txt | 2 +- .../bindings/power/renesas,sysc-rmobile.txt | 2 +- .../bindings/power/xlnx,zynqmp-genpd.txt | 2 +- .../bindings/soc/bcm/brcm,bcm2835-pm.txt | 2 +- .../bindings/soc/mediatek/scpsys.txt | 2 +- .../bindings/soc/ti/sci-pm-domain.txt | 2 +- .../bindings/usb/nvidia,tegra124-xusb.txt | 4 +- MAINTAINERS | 2 +- 37 files changed, 278 insertions(+), 241 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/power-domain-consumers.yaml create mode 100644 Documentation/devicetree/bindings/power/power-domain.yaml delete mode 100644 Documentation/devicetree/bindings/power/power_domain.txt diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt index 083dbf96ee00..f493d69e6194 100644 --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt @@ -100,7 +100,7 @@ Required sub-node properties: [0] http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/index.html [1] Documentation/devicetree/bindings/clock/clock-bindings.txt -[2] Documentation/devicetree/bindings/power/power_domain.txt +[2] Documentation/devicetree/bindings/power/power-domain.yaml [3] Documentation/devicetree/bindings/thermal/thermal.txt [4] Documentation/devicetree/bindings/sram/sram.txt [5] Documentation/devicetree/bindings/reset/reset.txt diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt index 401831973638..7b83ef43b418 100644 --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt @@ -110,7 +110,7 @@ Required properties: [1] Documentation/devicetree/bindings/clock/clock-bindings.txt [2] Documentation/devicetree/bindings/thermal/thermal.txt [3] Documentation/devicetree/bindings/sram/sram.txt -[4] Documentation/devicetree/bindings/power/power_domain.txt +[4] Documentation/devicetree/bindings/power/power-domain.yaml Example: diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt index c149fadc6f47..6c8a61b971f1 100644 --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt @@ -124,7 +124,7 @@ Required properties for Pinctrl sub nodes: CONFIG settings. [1] Documentation/devicetree/bindings/clock/clock-bindings.txt -[2] Documentation/devicetree/bindings/power/power_domain.txt +[2] Documentation/devicetree/bindings/power/power-domain.yaml [3] Documentation/devicetree/bindings/pinctrl/fsl,imx-pinctrl.txt RTC bindings based on SCU Message Protocol diff --git a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt index 6030af
Re: [chipidea] continuous USB resets on NXP i.MX 6ULL device
+ Li Jun Hi Peter, Li, On Mon, Sep 30, 2019 at 2:35 PM Igor Opaniuk wrote: > > Hi Peter, > > On Wed, Sep 25, 2019 at 3:44 AM Peter Chen wrote: > > > > > > > > > > Hi Fabio, Peter, Stefan, > > > > > > I've incidentally discovered your last year discussion in ML [1] (I hope > > > it rings the > > > bell) regarding the issue I'm still observing on the same device with the > > > mainline > > > kernel. > > > > > > The difference between i.MX 6ULL EVK and this particular device, is that > > > usbotg2 is > > > used only in host mode with the usb hub integrated on the carrier board > > > [2] [3]. > > > > > > root@colibri-imx6:~# lsusb -s 1:1 --tree > > > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ci_hdrc/1p, 480M > > > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M > > > > > > However I can't reproduce the same behavior with i.MX 6ULL EVK with > > > connected > > > usb hub to usbotg2. Also this behavior can't be reproduced with NXP > > > downstream > > > kernel (Linux version 4.9.144) on my device. > > > > > > After digging in a bit I found out that this happens only when > > > autosuspend is > > > enabled for the usb controller: > > > echo auto > /sys/bus/usb/devices/1-1/power/control > > > > > > It tries to go to suspend mode, but everytime fails and resumes: > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status > > > suspended > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status > > > resuming > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status > > > suspended > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status > > > suspended > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status > > > suspended > > > root@colibri-imx6:~# cat /sys/bus/usb/devices/1-1/power/runtime_status > > > resuming > > > > > > I'm observing ~2 seconds wakeup interrupts handled in ci_irq() in core.c > > > and > > > subsequent invocation of imx_controller_resume(). > > > > > > Meantime usboh3 remains enabled all the time (though > > > imx_disable_unprepare_clks() should disable it): > > > root@colibri-imx6:~# cat /sys/kernel/debug/clk/clk_summary | grep usb > > > usbphy2_gate 110 0 > > > 0 0 5 > > > usbphy1_gate 110 0 > > > 0 0 5 > > > pll7_usb_host 110 48000 > > > 0 0 5 > > > usbphy2 110 48000 > > > 0 0 5 > > > pll3_usb_otg230 48000 > > > 0 0 5 > > > usbphy1 000 48000 > > > 0 0 5 > > > usboh3 1106600 > > > 0 0 5 > > > > > > While I'm trying to localize the root cause, maybe you can give some > > > hints where to > > > look into? > > > > > > > Would you please look below two downstream patches see if it helps? > > > > commit e8e95658fe75f3873e06d5ad72a6bf0bad40f068 > > Author: Li Jun > > Date: Mon Oct 16 23:13:19 2017 +0800 > > > > MLK-16576 usb: phy: mxs: set hold_ring_off for USB2 PLL power up > > > > USB2 PLL use ring VCO, when the PLL power up, the ring VCO’s supply also > > ramp up. There is a possibility that the ring VCO start oscillation at > > multi nodes in this phase, especially for VCO which has many stages, > > then > > the multiwave will kept until PLL power down. Hold_ring_off(bit11) can > > force the VCO in one determined state when VCO supply start ramp up, to > > avoid this multiwave issue. Per IC design's suggestion it's better this > > bit can be off from 25us after pll power up to 25us before USB TX/RX. > > > > > > commit aa4680d844a340a5b6b60484f6e04cb9ba613c65 > > Author: Peter Chen > > Date: Wed Sep 7 12:16:59 2016 +0800 > > > > MLK-13125 usb: phy: phy-mxs-usb: enable weak 1p1 regulator for imx6ul > > during suspend > > > > For imx6ul PHY, when the system enters suspend, its 1p1 is off by > > default, > > that may cause the PHY get inaccurate USB DP/DM value. If the USB wakeup > > is enabled at this time, the unexpected wakeup may occur when the system > > enters suspend. > > > > In this patch, when the vbus is there, we enable weak 1p1 during the PHY > > suspend API, in that case, the USB DP/DM will be accurate for USB PHY, > > then unexpected usb wakeup will not be occurred, especially for the USB > > charger is connected scenario. The user needs to enable PHY wakeup for > > USB wakeup function using below setting. > > > > Peter > > > > > Thanks for helping me! > > > > > > [1] > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmarc.info%2 > > > F%3Fl%3Dlinux- > > > usb%26m%3D151844741732751&data=02%7C01
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On Wed, Oct 02, 2019 at 07:06:52PM +0300, Heikki Krogerus wrote: > On Tue, Oct 01, 2019 at 06:08:17AM -0700, Guenter Roeck wrote: > > On 10/1/19 2:48 AM, Heikki Krogerus wrote: > > > Copying everything from struct typec_capability to struct > > > typec_port during port registration. > > > > > What is the purpose of this patch ? To reduce the number of indirections at > > runtime, or to avoid having to have cap around ? > > To get rid of the cap member. > > > FWIW, it looks like the code doesn't copy _all_ variables (eg > > cap->try_role), > > and it doesn't drop port->cap. Am I missing something ? > > We can't drop port->cap at this point because the drivers still depend > on it. This patch is the "prepare" phase of the series. The last patch > in the series finally drops the member. I'll improve the commit message. > Yes, I figured that with the later patches. Sorry for the noise. > > > Signed-off-by: Heikki Krogerus > > > --- > > > drivers/usb/typec/class.c | 55 +-- > > > 1 file changed, 35 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > > index 94a3eda62add..3835e2d9fba6 100644 > > > --- a/drivers/usb/typec/class.c > > > +++ b/drivers/usb/typec/class.c > > > @@ -46,8 +46,14 @@ struct typec_port { > > > enum typec_role vconn_role; > > > enum typec_pwr_opmode pwr_opmode; > > > enum typec_port_typeport_type; > > > + enum typec_port_typefixed_role; > > > + enum typec_port_dataport_roles; > > > + enum typec_accessoryaccessory[TYPEC_MAX_ACCESSORY]; > > > > Would a pointer to cap->accessory be sufficient ? Or is there a reason to > > copy > > the actual array ? > > No. The point is to get rid of the cap member. > > > > struct mutexport_type_lock; > > > + u16 revision; > > > + u16 pd_revision; > > > + > > > enum typec_orientation orientation; > > > struct typec_switch *sw; > > > struct typec_mux*mux; > > > @@ -950,7 +956,7 @@ preferred_role_store(struct device *dev, struct > > > device_attribute *attr, > > > int role; > > > int ret; > > > - if (port->cap->type != TYPEC_PORT_DRP) { > > > + if (port->fixed_role != TYPEC_PORT_DRP) { > > > dev_dbg(dev, "Preferred role only supported with DRP > > > ports\n"); > > > return -EOPNOTSUPP; > > > } > > > @@ -982,7 +988,7 @@ preferred_role_show(struct device *dev, struct > > > device_attribute *attr, > > > { > > > struct typec_port *port = to_typec_port(dev); > > > - if (port->cap->type != TYPEC_PORT_DRP) > > > + if (port->fixed_role != TYPEC_PORT_DRP) > > > return 0; > > > if (port->prefer_role < 0) > > > @@ -1009,7 +1015,7 @@ static ssize_t data_role_store(struct device *dev, > > > return ret; > > > mutex_lock(&port->port_type_lock); > > > - if (port->cap->data != TYPEC_PORT_DRD) { > > > + if (port->port_roles != TYPEC_PORT_DRD) { > > > ret = -EOPNOTSUPP; > > > goto unlock_and_ret; > > > } > > > @@ -1029,7 +1035,7 @@ static ssize_t data_role_show(struct device *dev, > > > { > > > struct typec_port *port = to_typec_port(dev); > > > - if (port->cap->data == TYPEC_PORT_DRD) > > > + if (port->port_roles == TYPEC_PORT_DRD) > > > return sprintf(buf, "%s\n", port->data_role == > > > TYPEC_HOST ? > > > "[host] device" : "host [device]"); > > > @@ -1044,7 +1050,7 @@ static ssize_t power_role_store(struct device *dev, > > > struct typec_port *port = to_typec_port(dev); > > > int ret; > > > - if (!port->cap->pd_revision) { > > > + if (!port->pd_revision) { > > > dev_dbg(dev, "USB Power Delivery not supported\n"); > > > return -EOPNOTSUPP; > > > } > > > @@ -1064,9 +1070,9 @@ static ssize_t power_role_store(struct device *dev, > > > return ret; > > > mutex_lock(&port->port_type_lock); > > > - if (port->port_type != TYPEC_PORT_DRP) { > > > + if (port->fixed_role != TYPEC_PORT_DRP) { > > > > This is a semantic change: Previously, it compared the _current_ port type. > > Now it compares the initial (fixed) port type. Is this on purpose ? > > > > [ comment written before I noticed the change below. See there. ] > > > > > dev_dbg(dev, "port type fixed at \"%s\"", > > > - typec_port_power_roles[port->port_type]); > > > + typec_port_power_roles[port->fixed_role]); > > > ret = -EOPNOTSUPP; > > > goto unlock_and_ret; > > > } > > > @@ -1086,7 +1092,7 @@ static ssize_t power_role_show
Re: musb: cppi41: broken high speed FTDI functionality when connected to musb directly
* Yegor Yefremov [191002 06:57]: > On Wed, Oct 2, 2019 at 12:03 AM Tony Lindgren wrote: > > The other way to fix this would be to just wake up cpp41 in > > cppi41_dma_prep_slave_sg() and return NULL so that we can > > have musb_ep_program() continue with PIO while cppi41 is > > asleep. > > > > Anyways, care to try it out and see if it fixes your issue? > > The fix is working but on the first invocation, I get this output > (minicom provokes the same output): > # serialtest.py -c 2 /dev/ttyUSB0 /dev/ttyUSB0 ... > [ 210.940612] [] (__rpm_callback) from [] > (rpm_callback+0x20/0x80) > [ 210.948402] [] (rpm_callback) from [] > (rpm_resume+0x468/0x7a0) > [ 210.956018] [] (rpm_resume) from [] > (__pm_runtime_resume+0x4c/0x64) > [ 210.964086] [] (__pm_runtime_resume) from [] > (cppi41_dma_prep_slave_sg+0x20/0xfc [cppi41]) OK so that won't work, thanks for testing. Here's the alternative patch to try along the lines described above that just wakes up cppi41 and returns NULL so musb_ep_program() can continue with PIO until cppi41 is awake. Regards, Tony 8< --- diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c --- a/drivers/dma/ti/cppi41.c +++ b/drivers/dma/ti/cppi41.c @@ -586,9 +586,22 @@ static struct dma_async_tx_descriptor *cppi41_dma_prep_slave_sg( enum dma_transfer_direction dir, unsigned long tx_flags, void *context) { struct cppi41_channel *c = to_cpp41_chan(chan); + struct dma_async_tx_descriptor *txd = NULL; + struct cppi41_dd *cdd = c->cdd; struct cppi41_desc *d; struct scatterlist *sg; unsigned int i; + int error; + + error = pm_runtime_get(cdd->ddev.dev); + if (error < 0) { + pm_runtime_put_noidle(cdd->ddev.dev); + + return NULL; + } + + if (cdd->is_suspended) + goto err_out_not_ready; d = c->desc; for_each_sg(sgl, sg, sg_len, i) { @@ -611,7 +624,13 @@ static struct dma_async_tx_descriptor *cppi41_dma_prep_slave_sg( d++; } - return &c->txd; + txd = &c->txd; + +err_out_not_ready: + pm_runtime_mark_last_busy(cdd->ddev.dev); + pm_runtime_put_autosuspend(cdd->ddev.dev); + + return txd; } static void cppi41_compute_td_desc(struct cppi41_desc *d) -- 2.23.0
[PATCH] drivers: musb: removed unused status variable
From: Aliasgar Surti Status variable is initialized and returned without updating it's value. Removed status variable and returned value directly. Signed-off-by: Aliasgar Surti --- drivers/usb/musb/musb_gadget.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index ffe462a657b1..2cb31fc0cd60 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1085,7 +1085,6 @@ static int musb_gadget_disable(struct usb_ep *ep) u8 epnum; struct musb_ep *musb_ep; void __iomem*epio; - int status = 0; musb_ep = to_musb_ep(ep); musb = musb_ep->musb; @@ -1118,7 +1117,7 @@ static int musb_gadget_disable(struct usb_ep *ep) musb_dbg(musb, "%s", musb_ep->end_point.name); - return status; + return 0; } /* -- 2.17.1
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On Wed, Oct 02, 2019 at 09:36:39AM -0700, Guenter Roeck wrote: > port->cap->type used to be the role provided by the low level driver. > That was static, and it was not possible to override it. It did not > resemble the current port type, or a configured port type, it resembled > port capabilities. > > Your code changes that, meaning even if the low level driver (effectively > the hardware) states that it can not support DRP, it is now configurable > anyway. That seems to me like a substantial change to the original meaning > of this attribute. > > Effectiv ely there are now three values, > - port->port_type the current port tyle > - port->fixed_typethe type selected by the user > - port->cap->type the type provided by low level code, > now no longer available / used > > Even if the low level driver (hardware) says that it can not support > TYPEC_PORT_DRP, that can be overwritten by the user. Maybe there is a > good reason for that, but I don't see it, sorry. No, that was not my intention. Clearly there is a bug in my code. > Maybe it would make sense to introduce port->fixed_type in a separate patch. > As part of that patch you could explain why port->cap->type, ie a reflection > of the port capabilities, is no longer needed. Or, I could make this really simple. I could just copy the content of the cap as is to another struct typec_capability during port registration. My goal is really just remove the need for the device drivers keep the struct typec_capability instance if they don't need it, and I don't need to remove the cap member to achieve that. Nevertheless, IMO this attribute file really needs to be deprecated. On top of making things unnecessarily complicated for the userspace, it's making it difficult to make changes to the rest of the class code. We may not be able to get rid of the file, but there is nothing preventing us from supplying a better solution as an option. thanks, -- heikki
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On Wed, Oct 02, 2019 at 07:06:55PM +0300, Heikki Krogerus wrote: > This is a bit off topic, but that attribute file is really horrible. > Right now there is no way to know the actual capability of the > port in user space. If something changes a DRP port into sink or > source, there is no way to know after that that the port is actually > DRP capable. That statement is not correct. I'm sorry. I don't know why did I put it that way. I wanted to say that now the userpsace is forced to keep track of a specific sysfs file in order to be sure of the currently supported role, That alone is wrong, but there is no way to guarantee that one day we would not need to support another capability in a similar fashion, and that would mean another sysfs file, and we just forced the userspace to be modified. The capabilities of the port really need to be static. We can handle the capability changes like I propose below without affecting the userspace. > So that ABI is "buggy", but even without the problem, I still really > think that allowing the capabilities to be changed like that in > general is completely wrong. I don't have a problem with changing the > capabilities, but IMO it should be handled at one level higher, at the > controller device level. If the capabilities of a port need to be > changed, the old port should be removed, and a new with the new > capabilities registered. That is the only way to handle it without > making things unnecessarily difficult for the user space. > > I'm pretty sure that that was my counter proposal already at the time > when the attribute file was introduced, but I don't remember why > wasn't it accepted at the time? My protest against adding that > attribute file was in any case ignored :-(. In any case, my plan was > to later propose a new sysfs group that we offer to the parent > controller devices instead assigning it to the port devices, and that > group is meant to allow port capability changes the way I explained > above. Unless of course you are against it? > > thanks, > > -- > heikki -- heikki
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On 10/2/19 11:29 AM, Heikki Krogerus wrote: On Wed, Oct 02, 2019 at 09:36:39AM -0700, Guenter Roeck wrote: port->cap->type used to be the role provided by the low level driver. That was static, and it was not possible to override it. It did not resemble the current port type, or a configured port type, it resembled port capabilities. Your code changes that, meaning even if the low level driver (effectively the hardware) states that it can not support DRP, it is now configurable anyway. That seems to me like a substantial change to the original meaning of this attribute. Effectiv ely there are now three values, - port->port_typethe current port tyle - port->fixed_type the type selected by the user - port->cap->type the type provided by low level code, now no longer available / used Even if the low level driver (hardware) says that it can not support TYPEC_PORT_DRP, that can be overwritten by the user. Maybe there is a good reason for that, but I don't see it, sorry. No, that was not my intention. Clearly there is a bug in my code. Maybe it would make sense to introduce port->fixed_type in a separate patch. As part of that patch you could explain why port->cap->type, ie a reflection of the port capabilities, is no longer needed. Or, I could make this really simple. I could just copy the content of the cap as is to another struct typec_capability during port registration. My goal is really just remove the need for the device drivers keep the struct typec_capability instance if they don't need it, and I don't need to remove the cap member to achieve that. Maybe just use devm_kmemdup() ? Guenter Nevertheless, IMO this attribute file really needs to be deprecated. On top of making things unnecessarily complicated for the userspace, it's making it difficult to make changes to the rest of the class code. We may not be able to get rid of the file, but there is nothing preventing us from supplying a better solution as an option.
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On 10/2/19 12:16 PM, Heikki Krogerus wrote: On Wed, Oct 02, 2019 at 07:06:55PM +0300, Heikki Krogerus wrote: This is a bit off topic, but that attribute file is really horrible. Right now there is no way to know the actual capability of the port in user space. If something changes a DRP port into sink or source, there is no way to know after that that the port is actually DRP capable. That statement is not correct. I'm sorry. I don't know why did I put it that way. I wanted to say that now the userpsace is forced to keep track of a specific sysfs file in order to be sure of the currently supported role, That alone is wrong, but there is no way to guarantee that one day we would not need to support another capability in a similar fashion, and that would mean another sysfs file, and we just forced the userspace to be modified. The capabilities of the port really need to be static. I assume you refer to port_type. If I remember correctly, this is actually used by Android userspace code to specifically select if a port can be source, sink, or drp. I am not sure if it is a good idea to enforce port removal and re-registration in conjunction with this - the port can, after all, be connected to some storage device or to a monitor. I am not sure how we could "sell" to users the idea that if they change the port type, their screen will go dark for a few seconds. Am I missing something ? Thanks, Guenter We can handle the capability changes like I propose below without affecting the userspace. So that ABI is "buggy", but even without the problem, I still really think that allowing the capabilities to be changed like that in general is completely wrong. I don't have a problem with changing the capabilities, but IMO it should be handled at one level higher, at the controller device level. If the capabilities of a port need to be changed, the old port should be removed, and a new with the new capabilities registered. That is the only way to handle it without making things unnecessarily difficult for the user space. I'm pretty sure that that was my counter proposal already at the time when the attribute file was introduced, but I don't remember why wasn't it accepted at the time? My protest against adding that attribute file was in any case ignored :-(. In any case, my plan was to later propose a new sysfs group that we offer to the parent controller devices instead assigning it to the port devices, and that group is meant to allow port capability changes the way I explained above. Unless of course you are against it? thanks, -- heikki