Re: [PATCH] USB: serial: FTDI: Add device IDs for Sienna and Echelon PL-20

2019-10-02 Thread Johan Hovold
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

2019-10-02 Thread Johan Hovold
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

2019-10-02 Thread Maxime Ripard
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

2019-10-02 Thread Maxime Ripard
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

2019-10-02 Thread Dan Carpenter
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

2019-10-02 Thread Bernhard Gebetsberger
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

2019-10-02 Thread Peter Geis
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

2019-10-02 Thread Heikki Krogerus
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

2019-10-02 Thread Krzysztof Kozlowski
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

2019-10-02 Thread Krzysztof Kozlowski
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

2019-10-02 Thread Igor Opaniuk
+ 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

2019-10-02 Thread Guenter Roeck
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

2019-10-02 Thread Tony Lindgren
* 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

2019-10-02 Thread aliasgar . surti500
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

2019-10-02 Thread Heikki Krogerus
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

2019-10-02 Thread Heikki Krogerus
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

2019-10-02 Thread Guenter Roeck

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

2019-10-02 Thread Guenter Roeck

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