[PATCH v2] usb: dwc3: host: Set the dma_ops for xhci device

[Baolin Wang]

On ARM64 platform, it will set 'dummy_dma_ops' for device dma_ops if
it did not call 'arch_setup_dma_ops' at device creation time, that will
cause failure when setting the dma mask for device.

Thus this patch set the xhci device dma_ops from the parent device if
the xhci device dma_ops is 'dummy_dma_ops'.

Changes since v1:
 - Add CONFIG_ARM64 macro.

Signed-off-by: Baolin Wang 
---
 drivers/usb/dwc3/host.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index c679f63..edb666d 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -32,6 +32,11 @@ int dwc3_host_init(struct dwc3 *dwc)
return -ENOMEM;
}
 
+#ifdef CONFIG_ARM64
+   if (get_dma_ops(>dev) == get_dma_ops(NULL))
+   xhci->dev.archdata.dma_ops = get_dma_ops(dwc->dev);
+#endif
+
dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
 
xhci->dev.parent= dwc->dev;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2, 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy

[Frank Wang]

Hi Heiko & Guenter,

On 2016/6/4 5:24, Heiko Stübner wrote:

Am Freitag, 3. Juni 2016, 12:59:22 schrieb Guenter Roeck:

On Thu, Jun 02, 2016 at 02:48:10PM +0800, Frank Wang wrote:

The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
than rk3288 and before, and most of phy-related registers are also
different from the past, so a new phy driver is required necessarily.

Signed-off-by: Frank Wang 
---

Changes in v2:
  - Changed vbus_host operation from gpio to regulator in *_probe.
  - Improved the fault treatment relate to 480m clock register.
  - Cleaned up some meaningless codes in *_clk480m_disable.
  - made more clear the comment of *_sm_work.
  
[...]


+static struct clk *
+rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
+{
+   struct device_node *node = rphy->dev->of_node;
+   struct clk *clk;
+   struct clk_init_data init;
+   int ret;
+
+   init.name = "clk_usbphy_480m";
+   init.ops = _usb2phy_clkout_ops;
+   init.flags = CLK_IS_ROOT;

One more comment:

include/linux/clk-provider.h:#define CLK_IS_ROOTBIT(4) /*
Deprecated: Don't use */

... and the definition has been removed in linux-next.

also, the created clock should be a child of the phy-supply clock. Please take
a look at the picophy-variant (phy-rockchip-usb) that already handles that
parent assignment.



Okay, I will refer and correct them in the next patch (patch v3) .

BR.
Frank

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2, 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy

[Frank Wang]

Hi Guenter,

On 2016/6/4 3:57, Guenter Roeck wrote:

On Thu, Jun 02, 2016 at 02:48:10PM +0800, Frank Wang wrote:

The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
than rk3288 and before, and most of phy-related registers are also
different from the past, so a new phy driver is required necessarily.

Signed-off-by: Frank Wang 
---

Changes in v2:
  - Changed vbus_host operation from gpio to regulator in *_probe.
  - Improved the fault treatment relate to 480m clock register.
  - Cleaned up some meaningless codes in *_clk480m_disable.
  - made more clear the comment of *_sm_work.

  drivers/phy/Kconfig  |7 +
  drivers/phy/Makefile |1 +
  drivers/phy/phy-rockchip-inno-usb2.c |  604 ++
  3 files changed, 612 insertions(+)
  create mode 100644 drivers/phy/phy-rockchip-inno-usb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index b869b98..29ef15c 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -347,6 +347,13 @@ config PHY_ROCKCHIP_USB
help
  Enable this to support the Rockchip USB 2.0 PHY.

+config PHY_ROCKCHIP_INNO_USB2
+   tristate "Rockchip INNO USB2PHY Driver"
+   depends on ARCH_ROCKCHIP && OF
+   select GENERIC_PHY
+   help
+ Support for Rockchip USB2.0 PHY with Innosilicon IP block.
+
  config PHY_ROCKCHIP_EMMC
tristate "Rockchip EMMC PHY Driver"
depends on ARCH_ROCKCHIP && OF
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9c3e73c..4963fbc 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -38,6 +38,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)+= 
phy-s5pv210-usb2.o
  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)  += phy-exynos5-usbdrd.o
  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)   += phy-qcom-apq8064-sata.o
  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
+obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)   += phy-rockchip-inno-usb2.o
  obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
  obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
  obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)   += phy-qcom-ipq806x-sata.o
diff --git a/drivers/phy/phy-rockchip-inno-usb2.c 
b/drivers/phy/phy-rockchip-inno-usb2.c
new file mode 100644
index 000..eca46ff
--- /dev/null
+++ b/drivers/phy/phy-rockchip-inno-usb2.c
@@ -0,0 +1,604 @@
+/*
+ * Rockchip USB2.0 PHY with Innosilicon IP block driver
+ *
+ * Copyright (C) 2016 Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define BIT_WRITEABLE_SHIFT16
+#define SCHEDULE_DELAY (60 * HZ)
+
+enum rockchip_usb2phy_port_id {
+   USB2PHY_PORT_OTG,
+   USB2PHY_PORT_HOST,
+   USB2PHY_NUM_PORTS,
+};
+
+enum rockchip_usb2phy_host_state {
+   PHY_STATE_HS_ONLINE = 0,
+   PHY_STATE_DISCONNECT= 1,
+   PHY_STATE_HS_CONNECT= 2,
+   PHY_STATE_FS_CONNECT= 4,
+};
+
+struct usb2phy_reg {
+   unsigned intoffset;
+   unsigned intbitend;
+   unsigned intbitstart;
+   unsigned intdisable;
+   unsigned intenable;
+};
+
+/**
+ * struct rockchip_usb2phy_port_cfg: usb-phy port configuration.
+ * @phy_sus: phy suspend register.
+ * @ls_det_en: linestate detection enable register.
+ * @ls_det_st: linestate detection state register.
+ * @ls_det_clr: linestate detection clear register.
+ * @utmi_ls: utmi linestate state register.
+ * @utmi_hstdet: utmi host disconnect register.
+ */
+struct rockchip_usb2phy_port_cfg {
+   struct usb2phy_reg  phy_sus;
+   struct usb2phy_reg  ls_det_en;
+   struct usb2phy_reg  ls_det_st;
+   struct usb2phy_reg  ls_det_clr;
+   struct usb2phy_reg  utmi_ls;
+   struct usb2phy_reg  utmi_hstdet;
+};
+
+/**
+ * struct rockchip_usb2phy_cfg: usb-phy configuration.
+ * @num_ports: specify how many ports that the phy has.
+ * @clkout_ctl: keep on/turn off output clk of phy.
+ */
+struct rockchip_usb2phy_cfg {
+   unsigned intnum_ports;
+   struct usb2phy_reg  clkout_ctl;
+   const struct rockchip_usb2phy_port_cfg  *port_cfgs;
+};
+
+/**
+ * struct rockchip_usb2phy_port: usb-phy port data.
+ * @port_id: flag for otg port or host port.
+ * @suspended: phy suspended flag.
+ * @ls_irq: IRQ number assigned for linestate detection.
+ * @mutex: for register updating in 

Re: [v2, 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY

[Frank Wang]

Hi Guenter,

On 2016/6/4 3:25, Guenter Roeck wrote:

On Thu, Jun 02, 2016 at 02:48:09PM +0800, Frank Wang wrote:

Signed-off-by: Frank Wang 
---

Changes in v2:
  - Changed vbus_host optional property from gpio to regulator.
  - Specified vbus_otg-supply optional property.
  - Specified otg_id and otg_bvalid property.

  .../bindings/phy/phy-rockchip-inno-usb2.txt|   55 
  1 file changed, 55 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt 
b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
new file mode 100644
index 000..132e707
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
@@ -0,0 +1,55 @@
+ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
+
+Required properties (phy (parent) node):
+ - compatible : should contain:
+   * "rockchip,rk3366-usb2phy"
+   * "rockchip,rk3399-usb2phy"
+ - #clock-cells : should be 0.
+ - clock-names : specify the 480m output clk name.
+
+Optional properties:
+ - vbus_host-supply : phandle to a regulator that supplies host vbus.
+ - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
+
+Required nodes : a sub-node is required for each port the phy provides.
+The sub-node name is used to identify host or otg port.
+

Would it make sense to also specify the sub-node names ? The driver matches
"host-port", and presumably once supported "otg-port". Someone not knowing
this may specify "hostport" or some other name variant and wonder why nothing
works as expected.

Well, Yeah, I will correct it in the next patch (patch v3).

+Required properties (port (child) node):
+ - #phy-cells : must be 0. See ./phy-bindings.txt for details.
+ - interrupts : specify an interrupt for each entry in interrupt-names.
+ - interrupt-names : a list which shall be the following entries:
+   * "otg_id" : for the otg id interrupt.
+   * "otg_bvalid" : for the otg vbus interrupt.
+   * "linestate" : for the host or otg linestate interrupt.
+
+Example:
+
+grf: syscon@ff77 {
+   compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+...
+
+   u2phy: usb2-phy {
+   compatible = "rockchip,rk3366-usb2phy";
+   #clock-cells = <0>;
+   clock-output-names = "sclk_otgphy0_480m";
+
+   u2phy_otg: otg-port {
+   #phy-cells = <0>;
+   interrupts = ,
+,
+;
+   interrupt-names = "otg_id", "otg_bvalid", "linestate";
+   status = "okay";
+   };
+
+   u2phy_host: host-port {
+   #phy-cells = <0>;
+   interrupts = ;
+   interrupt-names = "linestate";
+   status = "okay";
+   };
+   };
+};






--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 06/12] power: pwrseq: simple: Add support for regulator and generic property

[Peter Chen]

On Fri, Jun 03, 2016 at 02:35:08PM +0200, Krzysztof Kozlowski wrote:
> On 06/03/2016 04:02 AM, Rob Herring wrote:
> > On Wed, Jun 01, 2016 at 10:02:15AM +0200, Krzysztof Kozlowski wrote:
> >> Some devices need real hard-reset by cutting the power.  During power
> >> sequence turn off and on the regulator, if it is provided.
> >>
> >> Additionally add support for instantiating the pwrseq-simple device on a
> >> generic property 'power-sequence'.  The device will attach itself to the
> >> node containing the property and parse the node's properties like
> >> reset-gpios, ext-supply etc.
> >>
> >> Signed-off-by: Krzysztof Kozlowski 
> >> ---
> >>  .../bindings/power/pwrseq/pwrseq-simple.txt| 29 +++-
> >>  drivers/power/pwrseq/pwrseq_simple.c   | 85 
> >> +-
> >>  2 files changed, 107 insertions(+), 7 deletions(-)
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt 
> >> b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
> >> index ce0e76749671..a8c3f13ee83f 100644
> >> --- a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
> >> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-simple.txt
> >> @@ -1,11 +1,17 @@
> >> -* The simple MMC power sequence provider
> >> +* The simple power sequence provider
> >>  
> >> -The purpose of the simple MMC power sequence provider is to supports a 
> >> set of
> >> +The purpose of the simple power sequence provider is to supports a set of
> >>  common properties between various SOC designs. It thus enables us to use 
> >> the
> >>  same provider for several SOC designs.
> >>  
> >> -Required properties:
> >> -- compatible : contains "mmc-pwrseq-simple".
> >> +The driver supports two types of bindings:
> >> +1. Separate node
> >> +   Required properties:
> >> +   - compatible : contains "mmc-pwrseq-simple".
> > 
> > Please note that this is not recommended for new users.
> 
> Sure.
> 
> > 
> >> +
> >> +2. Property for any node
> >> +   Required properties:
> >> +   - power-sequence
> >>  
> >>  Optional properties:
> >>  - reset-gpios : contains a list of GPIO specifiers. The reset GPIOs are 
> >> asserted
> >> @@ -16,6 +22,7 @@ Optional properties:
> >>See ../clocks/clock-bindings.txt for details.
> >>  - clock-names : Must include the following entry:
> >>"ext_clock" (External clock provided to the card).
> >> +- ext-supply : External regulator supply
> > 
> > What happens when there are 2 supplies?
> > 
> > I'd prefer the name not be genericish and use the real supply names. 
> > Then the power seq code should just turn on all supplies it finds. If 
> > the order or timing to turn on matters, then sorry, no generic sequence.
> 
> Understood. I'll change the code to use any supply.
> 
> As for the genericness of this approach, Sylwester Nawrocki pointed an
> old thread:
> [PATCH v6 0/4] Runtime Interpreted Power Sequences
> https://lkml.org/lkml/2012/9/12/127
> 
> How do you like that approach?
> 

Rob, I am trying to implement the dts layout you suggested (see below),
but I find it is very hard to it due to the device is still not created,
without device, it is hard to manage the resources under this device (
Eg, de-initialization for probe deferral case). So, a common driver
is suitable for this power sequence case.

 {
vbus-supply = <_usb_otg1_vbus>;
pinctrl-names = "default";
pinctrl-0 = <_usb_otg1_id>;
status = "okay";

#address-cells = <1>;
#size-cells = <0>;
hub: genesys@1 {
compatible = "usb5e3,608";
reg = <1>;
reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
reset-duration-us = <10>;
clocks = < IMX6SX_CLK_CKO>;
};
};

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Lu Baolu]

Hi Peter,

On 06/06/2016 09:25 AM, Peter Chen wrote:
> On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
>> Hi Peter,
>>
>> On 06/04/2016 10:28 AM, Peter Chen wrote:
>>> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> from my point,it is a dual-role switch
> driver too,
 No, it's not a dual-role switch driver, but a driver for USB port 
 multiplexing.

 One example of port multiplexing can be found in several Intel SOC and PCH
 chips, inside of which, there are two independent USB controllers: host and
 device. They might share a single port and this port could be configured to
 route the line to one of these two controllers. This patch introduced a 
 generic
 framework for port mux drivers. It aids the drivers to handle port mux by
 providing interfaces to 1) register/unregister a mux device; 2) lookup the
 mux device; and 3) switch the port.

>>> For this case, I can't see it is different with dual-role switch.
>> Port mux is part of dual role switch, but not the whole thing.
>>
>> Dual role switch includes at least below things:
>>  - ID or type-C event detection
>>  - port mux
>>  - VBUS management
>>  - start/stop host/device controllers
>>
>> An OTG/Dual-role framework can be used to keep all these
>> things run together with an internal state machine. But it's
>> not duplicated with a generic framework for port mux and
>> the port mux drivers.
> You have admitted port mux is one of the ports of dual-role switch,
> Then, how they can co-work with each other? If can't, the dual-role
> switch framework needs another input events management for switching.

My point is we need a generic framework for the port mux devices,
just like we have that for PHY and regulator. OTG framework
manages the port mux devices through the common interfaces
provided by the port mux framework.

If we integrate the port mux device support into OTG itself, this  will
force every use case of port mux to rely on the big OTG framework,
although what it needs is only a single driver. That causes unnecessary
software complexity.

>
>>> Your
>>> case is just like Renesas case, which uses two different drivers between
>>> peripheral and host[1].
>> In my case, the port mux devices are physical devices and they
>> can be controlled through GPIO pins or device registers. They
>> are independent of both peripheral and host controllers.
>>
> Yes, it is the same. GPIO pin or device registers is like ID pin
> event.
>



>> But this code is better co-work with OTG/Dual-role framework, we'd
>> better have only interface that the user can know which role for the
>> current port.
>> OTG/Dual-role framework and portmux framework are not overlapped.
>> The sysfs interface shouldn't be overlapped as well. Say, I have a port
>> mux device and I have a driver for it. I am able to read the status of my
>> port mux device through sysfs. This is not part of OTG/Dual-role as far
>> as I can see.
>>
> Then how the user wants to switch the role through the mux driver's
> sysfs or dual-role switch sysfs?
>

It depends. If you have an OTG/DRD capable controllers, you need to
do this through OTG sysfs; otherwise you only need to switch the port.

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Lu Baolu]

Hi Peter,

On 06/06/2016 10:05 AM, Peter Chen wrote:
> On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 06/05/2016 04:33 PM, Jun Li wrote:
 Port mux is part of dual role switch, but not the whole thing.
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
>
> An OTG/Dual-role framework can be used to keep all these things run
> together with an internal state machine. But it's not duplicated with a
> generic framework for port mux and the port mux drivers.
>
>>> Your
>>> case is just like Renesas case, which uses two different drivers
>>> between peripheral and host[1].
> In my case, the port mux devices are physical devices and they can be
> controlled through GPIO pins or device registers. They are independent of
> both peripheral and host controllers.
>
>>> I also think current OTG/Dual role framework can support your case, if you
>>> find there is any limitation of it which can't meet your requirement, we
>>> should improve it, Roger also provide an example of dual role switch with
>>> USB3 based on his OTG core.
>> Why do we need an OTG framework to support a device driver?
> Just like you said above, OTG framework can manage role switch, the
> role switch may need to start or stop host/gadget driver according to
> different hardware signals or user input.

We don't have any OTG or dual-role (reduced OTG) capable
controllers. So we don't need to aid OTG framework to
start/stop host/gadget drivers.

>
>> Is it something like a bus or class driver?
> The DRD/OTG framework uses the same device structure with the caller,
> the caller can be a dual-role controller driver (like dwc3, chipidea,
> etc), or a separate switch driver which like your mux port driver.
>

>From my point of view, this isn't the right way to handle a port
mux device.

We have many kinds of port mux devices across multiple archs,
we should have a generic framework for them, so that consumers,
(like OTG framework) can manipulate port mux devices through a
common interfaces. Just like we already have frameworks for PHY,
VBUS regulator and ...

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Lu Baolu]

Hi Jun,

On 06/06/2016 09:08 AM, Jun Li wrote:
>
>> -Original Message-
>> From: Lu Baolu [mailto:baolu...@linux.intel.com]
>> Sent: Sunday, June 05, 2016 4:47 PM
>> To: Jun Li ; Peter Chen 
>> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
>> Greg Kroah-Hartman ; Lee Jones
>> ; Heikki Krogerus ;
>> Liam Girdwood ; Mark Brown ;
>> linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Roger Quadros
>> 
>> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
>> mux
>>
>> Hi,
>>
>> On 06/05/2016 04:33 PM, Jun Li wrote:
 Port mux is part of dual role switch, but not the whole thing.
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
>
> An OTG/Dual-role framework can be used to keep all these things run
> together with an internal state machine. But it's not duplicated
> with a generic framework for port mux and the port mux drivers.
>
>>> Your
>>> case is just like Renesas case, which uses two different drivers
>>> between peripheral and host[1].
> In my case, the port mux devices are physical devices and they can
> be controlled through GPIO pins or device registers. They are
> independent of both peripheral and host controllers.
>
>>> I also think current OTG/Dual role framework can support your case, if
>>> you find there is any limitation of it which can't meet your
>>> requirement, we should improve it, Roger also provide an example of
>>> dual role switch with
>>> USB3 based on his OTG core.
>> Why do we need an OTG framework to support a device driver?
> Currently there are many controller drivers which are dual role
> capable, all has its specific approach/implementation, your case
> is another one, actually there are common part we can share and
> reuse, Roger is introducing a common framework which cooperates
> into usb core and udc-core. With that, each OTG/dual role user
> only need take care of its small specific part.

Intel's USB controllers aren't dual role capable, and we don't
need any dual role capable drivers either. It's just two USB
controllers which shares a single port and it has a physical
port mux device which could control the route of lines. We
only need drivers for the port mux devices.

>   
>> Is it something like a bus or class driver?
> It's not actually a driver, instead, it's more like a lib or
> helper routines. You just need register your host and gadget
> into OTG core, and define the ops routines if required, OTG state
> machine will help you do the switch.

As I said above, we don't have any dual role capable controllers.
We don't need to have any dual role capable host or gadget drivers
to register into OTG core. We only provide drivers for port mux
devices, just like what we already have for phy or regulator
devices.

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] usb: configfs: allow UDC binding rule configured as binding to *any* UDC

[Du, Changbin]

Thanks, Machek, This patch has already been dropped.

> On Tue 2016-05-03 11:04:24, changbin...@intel.com wrote:
> > From: "Du, Changbin" 
> >
> > On most platforms, there is only one device controller available.
> > In this case, we desn't care the UDC's name. So let's ignore the
> > name by setting 'UDC' to 'any'. And also we can change UDC name
> > at any time if it is not binded (no need set to "" first).
> 
> making "any" special does not look like a good idea. What if it really
> is "any"?
> 
> Return nothing instead, not even \n?
> 
> > Signed-off-by: Du, Changbin 
> > Signed-off-by: Du, Changbin 
> 
> I don't think this is how you should sign it off.
> 
> Best regards,
> 
>   Pavel
> 
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Peter Chen]

On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
> Hi,
> 
> On 06/05/2016 04:33 PM, Jun Li wrote:
> >> Port mux is part of dual role switch, but not the whole thing.
> >> > 
> >> > Dual role switch includes at least below things:
> >> >  - ID or type-C event detection
> >> >  - port mux
> >> >  - VBUS management
> >> >  - start/stop host/device controllers
> >> > 
> >> > An OTG/Dual-role framework can be used to keep all these things run
> >> > together with an internal state machine. But it's not duplicated with a
> >> > generic framework for port mux and the port mux drivers.
> >> > 
> >>> > > Your
> >>> > > case is just like Renesas case, which uses two different drivers
> >>> > > between peripheral and host[1].
> >> > 
> >> > In my case, the port mux devices are physical devices and they can be
> >> > controlled through GPIO pins or device registers. They are independent of
> >> > both peripheral and host controllers.
> >> > 
> > I also think current OTG/Dual role framework can support your case, if you
> > find there is any limitation of it which can't meet your requirement, we
> > should improve it, Roger also provide an example of dual role switch with
> > USB3 based on his OTG core.
> 
> Why do we need an OTG framework to support a device driver?

Just like you said above, OTG framework can manage role switch, the
role switch may need to start or stop host/gadget driver according to
different hardware signals or user input.

> Is it something like a bus or class driver?

The DRD/OTG framework uses the same device structure with the caller,
the caller can be a dual-role controller driver (like dwc3, chipidea,
etc), or a separate switch driver which like your mux port driver.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Peter Chen]

On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/04/2016 10:28 AM, Peter Chen wrote:
> > On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>> from my point,it is a dual-role switch
> >>> driver too,
> >> No, it's not a dual-role switch driver, but a driver for USB port 
> >> multiplexing.
> >>
> >> One example of port multiplexing can be found in several Intel SOC and PCH
> >> chips, inside of which, there are two independent USB controllers: host and
> >> device. They might share a single port and this port could be configured to
> >> route the line to one of these two controllers. This patch introduced a 
> >> generic
> >> framework for port mux drivers. It aids the drivers to handle port mux by
> >> providing interfaces to 1) register/unregister a mux device; 2) lookup the
> >> mux device; and 3) switch the port.
> >>
> > For this case, I can't see it is different with dual-role switch.
> 
> Port mux is part of dual role switch, but not the whole thing.
> 
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
> 
> An OTG/Dual-role framework can be used to keep all these
> things run together with an internal state machine. But it's
> not duplicated with a generic framework for port mux and
> the port mux drivers.

You have admitted port mux is one of the ports of dual-role switch,
Then, how they can co-work with each other? If can't, the dual-role
switch framework needs another input events management for switching.

> 
> > Your
> > case is just like Renesas case, which uses two different drivers between
> > peripheral and host[1].
> 
> In my case, the port mux devices are physical devices and they
> can be controlled through GPIO pins or device registers. They
> are independent of both peripheral and host controllers.
> 

Yes, it is the same. GPIO pin or device registers is like ID pin
event.

> 
> >> Port multiplexing isn't equal to USB dual role. There are other cases in 
> >> today's
> >> systems. In several Intel PCH chips, there equips two USB host 
> >> controllers: ehci
> >> and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees 
> >> all
> >> USB ports work even running an old version of OS which lacks of USB3 
> >> support.
> >> In theory, we can create a driver for the port mux and switch the ports 
> >> between
> >> xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)
> >>
> >> Another case is xHCI debug capability. The xHCI host controller might equip
> >> a unit for system debugging (refer to 7.6 of xHCI spec). The debugging 
> >> unit is
> >> independent of xhci host controller. But it shares its port with xhci. 
> >> Software
> >> could switch the port between xhci and the debugging unit through the 
> >> registers
> >> defined in xHCI spec.
> >>
> > Yes, above two are different with dual role switch. But in your code and
> > Kconfig, it seems this framework is dedicated for dual-role. Eg:
> >
> > +menuconfig USB_PORTMUX
> > +   bool "USB dual role port MUX support"
> > +   help
> > + Generic USB dual role port mux support.
> 
> Above two cases are examples for port multiplexing, but I don't think we
> need drivers for them. At this moment, this framework is only for dual
> role port mux devices.
> 
> >
> > I think a general dual role port mux is necessary, it can be used to
> > manage different dual-role switch method, eg
> 
> Yes, I agree.
> 
> > - ID pin
> > - External connector through GPIO
> > - SoC register
> > - sysfs
> > - type-C events
> 
> ID pin and type-C events are the *reasons* which trigger the port
> mux switch. Currently, on our platforms, gpio, registers and sysfs
> are methods to control the mux.
> 
> >
> > But this code is better co-work with OTG/Dual-role framework, we'd
> > better have only interface that the user can know which role for the
> > current port.
> 
> OTG/Dual-role framework and portmux framework are not overlapped.
> The sysfs interface shouldn't be overlapped as well. Say, I have a port
> mux device and I have a driver for it. I am able to read the status of my
> port mux device through sysfs. This is not part of OTG/Dual-role as far
> as I can see.
> 

Then how the user wants to switch the role through the mux driver's
sysfs or dual-role switch sysfs?

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Jun Li]

> -Original Message-
> From: Lu Baolu [mailto:baolu...@linux.intel.com]
> Sent: Sunday, June 05, 2016 4:47 PM
> To: Jun Li ; Peter Chen 
> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
> Greg Kroah-Hartman ; Lee Jones
> ; Heikki Krogerus ;
> Liam Girdwood ; Mark Brown ;
> linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Roger Quadros
> 
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi,
> 
> On 06/05/2016 04:33 PM, Jun Li wrote:
> >> Port mux is part of dual role switch, but not the whole thing.
> >> >
> >> > Dual role switch includes at least below things:
> >> >  - ID or type-C event detection
> >> >  - port mux
> >> >  - VBUS management
> >> >  - start/stop host/device controllers
> >> >
> >> > An OTG/Dual-role framework can be used to keep all these things run
> >> > together with an internal state machine. But it's not duplicated
> >> > with a generic framework for port mux and the port mux drivers.
> >> >
> >>> > > Your
> >>> > > case is just like Renesas case, which uses two different drivers
> >>> > > between peripheral and host[1].
> >> >
> >> > In my case, the port mux devices are physical devices and they can
> >> > be controlled through GPIO pins or device registers. They are
> >> > independent of both peripheral and host controllers.
> >> >
> > I also think current OTG/Dual role framework can support your case, if
> > you find there is any limitation of it which can't meet your
> > requirement, we should improve it, Roger also provide an example of
> > dual role switch with
> > USB3 based on his OTG core.
> 
> Why do we need an OTG framework to support a device driver?

Currently there are many controller drivers which are dual role
capable, all has its specific approach/implementation, your case
is another one, actually there are common part we can share and
reuse, Roger is introducing a common framework which cooperates
into usb core and udc-core. With that, each OTG/dual role user
only need take care of its small specific part.
  
> Is it something like a bus or class driver?

It's not actually a driver, instead, it's more like a lib or
helper routines. You just need register your host and gadget
into OTG core, and define the ops routines if required, OTG state
machine will help you do the switch.

Li Jun
 
> 
> Best regards,
> Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: OHCI: NULL or LIST_POISON dereference on ueagle-atm disconnection

[Alan Stern]

On Sun, 5 Jun 2016, Michał Pecio wrote:

> OK, so I added those printks and I think I know what's going on.
> 
> Apparently, ed_schedule failure triggers 'goto fail' and leaves the ed
> in some non-IDLE state so that it never gets added to the list indeed.
> 
> The failing EP 0x88 is 1kB isochronous, btw, so it all looks plausible.

Yes, you spotted it.  The problem is indeed in ed_schedule().

Try moving the first executable line:

ed->state = ED_OPER;

to the end of the routine, just before the "return 0;".  That should 
fix the problem.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: clear halt

[Dave Mielke]

[quoted lines by Greg KH on 2016/06/05 at 14:23 -0700]

>> Using USBFS: If a device doesn't support clear halt (or, perhaps, not 
>> properly), is there an alternative? For example, is there a way to ask which 
>> state the device's data toggle is in and then tell the host what state to 
>> reset 
>> its end to?
>
>Nope, sorry, see the USB spec for details :(

Thanks. I was just hoping that I'd missed something.

-- 
Dave Mielke   | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: d...@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.org/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: clear halt

[Greg KH]

On Sun, Jun 05, 2016 at 05:00:12PM -0400, Dave Mielke wrote:
> Using USBFS: If a device doesn't support clear halt (or, perhaps, not 
> properly), is there an alternative? For example, is there a way to ask which 
> state the device's data toggle is in and then tell the host what state to 
> reset 
> its end to?

Nope, sorry, see the USB spec for details :(
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

clear halt

[Dave Mielke]

Using USBFS: If a device doesn't support clear halt (or, perhaps, not 
properly), is there an alternative? For example, is there a way to ask which 
state the device's data toggle is in and then tell the host what state to reset 
its end to?

-- 
Dave Mielke   | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: d...@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.org/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: OHCI: NULL or LIST_POISON dereference on ueagle-atm disconnection

[Michał Pecio]

OK, so I added those printks and I think I know what's going on.

Apparently, ed_schedule failure triggers 'goto fail' and leaves the ed
in some non-IDLE state so that it never gets added to the list indeed.

The failing EP 0x88 is 1kB isochronous, btw, so it all looks plausible.

Code (v3.18, newer is similar):

/* schedule the ed if needed */
if (ed->state == ED_IDLE) {
printk(KERN_INFO "ed %p is ED_IDLE\n", ed);
retval = ed_schedule (ohci, ed);
if (retval < 0) {
printk(KERN_INFO "ed %p ed_schedule()=%d\n", ed, 
retval);
usb_hcd_unlink_urb_from_ep(hcd, urb);
goto fail;
}

--- snip ---

printk(KERN_INFO "list_add ed %p\n", ed);
list_add(>in_use_list, >eds_in_use);

Log (8800dface5b0 is the bad ed):

[   30.115100] got ed 8800dface5b0 for ep 8800dfbcb1e0 (0x88) urb 
8800dfbac000
[   30.116053] list_del ed 8800dface4d0
[   30.127030] ed 8800dface5b0 state=0 in_use_list.next=  (null)
[   30.133817] ed 8800dface5b0 is ED_IDLE
[   30.137914] ohci-pci :00:12.0: ERR -28, interval 1 msecs, load 793
[   30.144439] ed 8800dface5b0 ed_schedule()=-28
[   30.149143] ATM dev 0: usbatm_submit_urb: urb 0x8800dfbac000 submission 
failed (-28)!
[   30.157318] got ed 8800dface5b0 for ep 8800dfbcb1e0 (0x88) urb 
8800dfbac800
[   30.165313] ed 8800dface5b0 state=2 in_use_list.next=  (null)
[   30.172119] got ed 8800dface5b0 for ep 8800dfbcb1e0 (0x88) urb 
8800dfbac200
[   30.180116] ed 8800dface5b0 state=2 in_use_list.next=  (null)
[   30.186905] got ed 8800dface5b0 for ep 8800dfbcb1e0 (0x88) urb 
8800dfbac300
[   30.194900] ed 8800dface5b0 state=2 in_use_list.next=  (null)
[   30.201708] usb 1-3: [ueagle-atm] (re)booting started
[   30.206785] got ed 8800dface4d0 for ep 8800dfae5048 (0x00) urb 
8800dfb69f00
[   30.214792] ed 8800dface4d0 state=0 in_use_list.next=dead0100
[   30.221571] ed 8800dface4d0 is ED_IDLE
[   30.225666] list_add ed 8800dface4d0
[   30.230147] got ed 8800dface4d0 for ep 8800dfae5048 (0x00) urb 
8800dfb96600
[   30.231115] list_del ed 8800dface4d0
[   30.242073] ed 8800dface4d0 state=0 in_use_list.next=dead0100
[   30.248853] ed 8800dface4d0 is ED_IDLE
[   30.252945] list_add ed 8800dface4d0
[   30.279862] got ed 8800dface5b0 for ep 8800dfbcb1e0 (0x88) urb 
8800dfbac000
[   30.287870] ed 8800dface5b0 state=2 in_use_list.next=  (null)

--- snip ---

[   36.405670] usb 1-3: USB disconnect, device number 4
[   36.410663] ohci-pci :00:12.0: unlink iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.419543] ATM dev 0: usbatm_complete: urb 0x8800dfbac000 failed (-2)!
[   36.426515] ohci-pci :00:12.0: link iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.435056] ohci-pci :00:12.0: unlink iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.444550] ATM dev 0: usbatm_complete: urb 0x8800dfbac800 failed (-2)!
[   36.451515] ohci-pci :00:12.0: link iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.460057] ohci-pci :00:12.0: unlink iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.469571] ATM dev 0: usbatm_complete: urb 0x8800dfbac200 failed (-2)!
[   36.476540] ohci-pci :00:12.0: link iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.485083] ohci-pci :00:12.0: unlink iso ed 8800dface5b0 branch 0 
[793us.], interval 1
[   36.494602] ATM dev 0: usbatm_complete: urb 0x8800dfbac300 failed (-2)!
[   36.501572] list_del ed 8800dface5b0
[   36.505492] fuck fuck fuck null null null
[   36.508266] usb 1-3: [UEAGLE-ATM] usb_control_msg error -19
[   36.513872] usb 1-3: [ueagle-atm] ADSL device removed
[   36.518978] freeing ed 8800dface4d0
[   36.522813] freeing ed 8800dface620
[   36.526653] freeing ed 8800dface540
[   36.530486] freeing ed 8800dface5b0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: usbip: fix null pointer dereference

[Sudip Mukherjee]

On Friday 03 June 2016 09:29 AM, Krzysztof Opasiak wrote:



On 06/02/2016 03:22 PM, Sudip Mukherjee wrote:

We have been dereferencing udc before checking it. Lets use it after it
has been checked.



To be honest I have mixed feelings about this patch.

On one hand it prevents us from dereferencing potential NULL ptr what is
generally good. But on the other hand it seems to be a little bit
pointless overhead. This function is called only in one place, it's
internal function of vudc driver and in addition generally it is
currently impossible that this function will get NULL ptr as parameter
as it's value is taken from container_of(). Not to mention that if this
is NULL or garbage we will end up in NULL ptr dereference much earlier
before calling this function.

So if there is something that you would like to fix with this patch and
you have a real problem with this function could you please provide us
some more details (for example stack trace)? If this patch is just to
prevent us from something that will never happen then I would rather to
not submit this. In my opinion if we get a NULL in this function this
means that we have some serious problem in UDC core and this check will
just mask this error.


Yes, I should have seen earlier that the only caller has already 
dereferenced udc. So maybe the following will be appropriate in this 
situation.


Regards
Sudip

diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c
index 99397fa..0f98f2c 100644
--- a/drivers/usb/usbip/vudc_sysfs.c
+++ b/drivers/usb/usbip/vudc_sysfs.c
@@ -40,7 +40,7 @@ int get_gadget_descs(struct vudc *udc)
struct usb_ctrlrequest req;
int ret;
 
-   if (!udc || !udc->driver || !udc->pullup)
+   if (!udc->driver || !udc->pullup)
return -EINVAL;
 
req.bRequestType = USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE;

Re: OHCI: NULL or LIST_POISON dereference on ueagle-atm disconnection

[Alan Stern]

On Sat, 4 Jun 2016, Greg KH wrote:

> On Sat, Jun 04, 2016 at 10:05:27AM +0200, Michał Pecio wrote:
> > > Hi, I think I have a bug in the OHCI driver.
> > > 
> > > Kernel version: 4.4.11 (some old 3.14 seems fine, didn't try others)
> > 
> > 3.18.34 and 4.7-rc1 are affected too.
> > 
> > The bug seems to have been introduced by "add I/O watchdog for orphan
> > TDs" (81e38333). I couldn't run bisect because those old kernels don't
> > build with gcc5+, but I started reverting commits to ohci-hcd.c and
> > ohci-q.hcd on top of 3.18.34 and got things to work at this point:
> > 
> > commit 0831cb21003464f0764581fed26de83ac097d799
> > Author: Michal Pecio 
> > Date:   Sat Jun 4 09:55:25 2016 +0200
> > 
> > Revert "USB: OHCI: add I/O watchdog for orphan TDs"
> > 
> > This reverts commit 81e38333513cec155c720432226dabe9f9f76a77.
> > 
> > commit cce8b2ec8e97a7c01fda1275e2a5732a1d0a1d88
> > Author: Michal Pecio 
> > Date:   Sat Jun 4 09:53:58 2016 +0200
> > 
> > Revert "USB: OHCI: add check for stopped frame counter"
> > 
> > This reverts commit 499b3803d3e2f062f73bf22372b38393369ffcbf.
> 
> Alan, any ideas here?
> 
> Michał, please always cc: the people who wrote the patches that you
> found problems with, makes things much easier :)

I haven't taken the time to go through it in detail yet.  It looks like
the driver is trying to remove an ED from the in_use_list before it was
added to the list, which doesn't make sense.

Michał, you could add some debugging lines to the source code that 
might help pin down the problem.  ed->in_use_list gets modified in only 
two places: in ohci_urb_enqueue() in ohci-hcd.c and in finish_unlinks() 
in ohci-q.c.  Just before each of those lines, you could write out the 
value of ed to the kernel log.  That would tell us if the ED really is 
being removed before it is added to the list.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: EHCI fails to schedule large full-speed isochronous transfers

[Alan Stern]

yOn Sat, 4 Jun 2016, Greg KH wrote:

> On Sat, Jun 04, 2016 at 08:32:43PM +0200, Michał Pecio wrote:
> > Hi,
> > 
> > Why high-bandwidth USB 1.1 isochronous devices don't work on 2.0 hosts?

You mean, on EHCI hosts -- they do work on other USB-2 host 
controllers, as far as I know.

Basically, the answer is that the protocol is too complicated and
nobody has done all the work needed to implement it fully.  Demand has
been relatively small, and USB-2 is in the process of being replaced by
USB-3 anyway.

IMO the USB-IF used bad judgement when they settled on the design for
Transaction Translators (those are the hardware components that connect
high-speed USB bus segments (USB-2) to full- and low-speed segments
(USB-1.1)).  They decided to put most of the burden on the software
instead of the hardware, where it would have been much simpler.  As a
result, EHCI drivers for low- and full-speed devices have to be
incredibly complicated, and nobody wants to put in the work required to 
make one that is fully functional for Linux.

> > I have an ADSL modem of this kind which I would like to use on a
> > companion-less 2.0 host in my router, but I can't because it fails with
> > ENOSPC errors. Same thing happens on my PC when connected through a 2.0
> > hub, but it works perfectly on 1.1 and 3.0 hosts, with or without hubs.
> > 
> > This seems to be an old and widely known issue, but the only "solution"
> > I found so far is "well, just don't use hubs and let the companion take
> > it over", which obviously doesn't solve the case of pure-EHCI SoCs.
> > 
> > (Well, another solutions applicable to those particular modems is to
> > switch them to bulk mode, but it's slower.)
> > 
> > Is this some limitation of the USB 2.0 spec or just a Linux issue?

Just a Linux issue.  But it is the result of a bad choice in the spec 
(again, in my opinion).

> > Any possible fixes or hackarounds?
> 
> Don't use a 2.0 hub, or connect it directly as you have, or use a 3.0
> controller :)
> 
> Seriously, handling 1.1 transactions through a 2.0 hub is hard, and
> messy.  It's been a known issue for decades, and given that 3.0 pretty
> much resolved most of these issues, no one is working to resolve them
> anymore, it's just so much simpler to change hardware.

As Greg says, changing hardware would be much less effort and cost a 
lot less than trying to fix up the software.  Of course, if you want to 
work on the driver software, you are free to do so -- I'll be willing 
to help.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/4] musb: sunxi: Simplify dr_mode handling

[Hans de Goede]

phy-sun4i-usb now has proper dr_mode handling, it always registers an
extcon, and sends a notify with the mode (even when in peripheral- /
host-only mode) at least once.

So we can simply the sunxi musb glue by always registering its extcon
notifier and relying on sunxi_musb_work() to enable vbus when in
host-only mode.

This also enables host- and peripheral-only mode with vbus monitoring.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-No changes
Changes in v3:
-No changes
---
 drivers/usb/musb/sunxi.c | 68 ++--
 1 file changed, 25 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index e7d4617..b88a2f6 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -255,12 +255,10 @@ static int sunxi_musb_init(struct musb *musb)
writeb(SUNXI_MUSB_VEND0_PIO_MODE, musb->mregs + SUNXI_MUSB_VEND0);
 
/* Register notifier before calling phy_init() */
-   if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE) {
-   ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
-  >host_nb);
-   if (ret)
-   goto error_reset_assert;
-   }
+   ret = extcon_register_notifier(glue->extcon, EXTCON_USB_HOST,
+  >host_nb);
+   if (ret)
+   goto error_reset_assert;
 
ret = phy_init(glue->phy);
if (ret)
@@ -274,9 +272,8 @@ static int sunxi_musb_init(struct musb *musb)
return 0;
 
 error_unregister_notifier:
-   if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-   extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-  >host_nb);
+   extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+  >host_nb);
 error_reset_assert:
if (test_bit(SUNXI_MUSB_FL_HAS_RESET, >flags))
reset_control_assert(glue->rst);
@@ -300,9 +297,8 @@ static int sunxi_musb_exit(struct musb *musb)
 
phy_exit(glue->phy);
 
-   if (musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
-   extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
-  >host_nb);
+   extcon_unregister_notifier(glue->extcon, EXTCON_USB_HOST,
+  >host_nb);
 
if (test_bit(SUNXI_MUSB_FL_HAS_RESET, >flags))
reset_control_assert(glue->rst);
@@ -314,25 +310,6 @@ static int sunxi_musb_exit(struct musb *musb)
return 0;
 }
 
-static int sunxi_set_mode(struct musb *musb, u8 mode)
-{
-   struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
-   int ret;
-
-   if (mode == MUSB_HOST) {
-   ret = phy_power_on(glue->phy);
-   if (ret)
-   return ret;
-
-   set_bit(SUNXI_MUSB_FL_PHY_ON, >flags);
-   /* Stop musb work from turning vbus off again */
-   set_bit(SUNXI_MUSB_FL_VBUS_ON, >flags);
-   musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-   }
-
-   return 0;
-}
-
 static void sunxi_musb_enable(struct musb *musb)
 {
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -579,7 +556,6 @@ static const struct musb_platform_ops sunxi_musb_ops = {
.exit   = sunxi_musb_exit,
.enable = sunxi_musb_enable,
.disable= sunxi_musb_disable,
-   .set_mode   = sunxi_set_mode,
.fifo_offset= sunxi_musb_fifo_offset,
.ep_offset  = sunxi_musb_ep_offset,
.busctl_offset  = sunxi_musb_busctl_offset,
@@ -635,10 +611,6 @@ static int sunxi_musb_probe(struct platform_device *pdev)
return -EINVAL;
}
 
-   glue = devm_kzalloc(>dev, sizeof(*glue), GFP_KERNEL);
-   if (!glue)
-   return -ENOMEM;
-
memset(, 0, sizeof(pdata));
switch (usb_get_dr_mode(>dev)) {
 #if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_HOST
@@ -646,15 +618,13 @@ static int sunxi_musb_probe(struct platform_device *pdev)
pdata.mode = MUSB_PORT_MODE_HOST;
break;
 #endif
+#if defined CONFIG_USB_MUSB_DUAL_ROLE || defined CONFIG_USB_MUSB_GADGET
+   case USB_DR_MODE_PERIPHERAL:
+   pdata.mode = MUSB_PORT_MODE_GADGET;
+   break;
+#endif
 #ifdef CONFIG_USB_MUSB_DUAL_ROLE
case USB_DR_MODE_OTG:
-   glue->extcon = extcon_get_edev_by_phandle(>dev, 0);
-   if (IS_ERR(glue->extcon)) {
-   if (PTR_ERR(glue->extcon) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
-   dev_err(>dev, "Invalid or missing extcon\n");
-   return PTR_ERR(glue->extcon);
-   }
pdata.mode = MUSB_PORT_MODE_DUAL_ROLE;

[PATCH v3 3/4] phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31

[Hans de Goede]

The A31 companion pmic (axp221) does not generate vbus change interrupts
when the board is driving vbus, so we must poll when using the pmic for
vbus-det _and_ we're driving vbus.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-No changes
Changes in v3:
-No changes
---
 drivers/phy/phy-sun4i-usb.c | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index e3cbaae..a7abae6 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -95,6 +95,7 @@
 
 enum sun4i_usb_phy_type {
sun4i_a10_phy,
+   sun6i_a31_phy,
sun8i_a33_phy,
sun8i_h3_phy,
 };
@@ -125,7 +126,6 @@ struct sun4i_usb_phy_data {
/* phy0 / otg related variables */
struct extcon_dev *extcon;
bool phy0_init;
-   bool phy0_poll;
struct gpio_desc *id_det_gpio;
struct gpio_desc *vbus_det_gpio;
struct power_supply *vbus_power_supply;
@@ -353,6 +353,24 @@ static bool sun4i_usb_phy0_have_vbus_det(struct 
sun4i_usb_phy_data *data)
return data->vbus_det_gpio || data->vbus_power_supply;
 }
 
+static bool sun4i_usb_phy0_poll(struct sun4i_usb_phy_data *data)
+{
+   if ((data->id_det_gpio && data->id_det_irq < 0) ||
+   (data->vbus_det_gpio && data->vbus_det_irq < 0))
+   return true;
+
+   /*
+* The A31 companion pmic (axp221) does not generate vbus change
+* interrupts when the board is driving vbus, so we must poll
+* when using the pmic for vbus-det _and_ we're driving vbus.
+*/
+   if (data->cfg->type == sun6i_a31_phy &&
+   data->vbus_power_supply && data->phys[0].regulator_on)
+   return true;
+
+   return false;
+}
+
 static int sun4i_usb_phy_power_on(struct phy *_phy)
 {
struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
@@ -374,7 +392,7 @@ static int sun4i_usb_phy_power_on(struct phy *_phy)
phy->regulator_on = true;
 
/* We must report Vbus high within OTG_TIME_A_WAIT_VRISE msec. */
-   if (phy->index == 0 && data->vbus_det_gpio && data->phy0_poll)
+   if (phy->index == 0 && sun4i_usb_phy0_poll(data))
mod_delayed_work(system_wq, >detect, DEBOUNCE_TIME);
 
return 0;
@@ -395,7 +413,7 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
 * phy0 vbus typically slowly discharges, sometimes this causes the
 * Vbus gpio to not trigger an edge irq on Vbus off, so force a rescan.
 */
-   if (phy->index == 0 && data->vbus_det_gpio && !data->phy0_poll)
+   if (phy->index == 0 && !sun4i_usb_phy0_poll(data))
mod_delayed_work(system_wq, >detect, POLL_TIME);
 
return 0;
@@ -483,7 +501,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
if (vbus_notify)
extcon_set_cable_state_(data->extcon, EXTCON_USB, vbus_det);
 
-   if (data->phy0_poll)
+   if (sun4i_usb_phy0_poll(data))
queue_delayed_work(system_wq, >detect, POLL_TIME);
 }
 
@@ -668,11 +686,6 @@ static int sun4i_usb_phy_probe(struct platform_device 
*pdev)
}
 
data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
-   data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
-   if ((data->id_det_gpio && data->id_det_irq < 0) ||
-   (data->vbus_det_gpio && data->vbus_det_irq < 0))
-   data->phy0_poll = true;
-
if (data->id_det_irq >= 0) {
ret = devm_request_irq(dev, data->id_det_irq,
sun4i_usb_phy0_id_vbus_det_irq,
@@ -684,6 +697,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
}
}
 
+   data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
if (data->vbus_det_irq >= 0) {
ret = devm_request_irq(dev, data->vbus_det_irq,
sun4i_usb_phy0_id_vbus_det_irq,
@@ -735,7 +749,7 @@ static const struct sun4i_usb_phy_cfg sun5i_a13_cfg = {
 
 static const struct sun4i_usb_phy_cfg sun6i_a31_cfg = {
.num_phys = 3,
-   .type = sun4i_a10_phy,
+   .type = sun6i_a31_phy,
.disc_thresh = 3,
.phyctl_offset = REG_PHYCTL_A10,
.dedicated_clocks = true,
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/4] USB: Fix of_usb_get_dr_mode_by_phy with a shared phy block

[Hans de Goede]

Some SoCs have a single phy-hw-block with multiple phys, this is
modelled by a single phy dts node, so we end up with multiple
controller nodes with a phys property pointing to the phy-node
of the otg-phy.

Only one of these controllers typically is an otg controller, yet we
were checking the first controller who uses a phy from the block and
then end up looking for a dr_mode property in e.g. the ehci controller.

This commit fixes this by adding an arg0 parameter to
of_usb_get_dr_mode_by_phy and make of_usb_get_dr_mode_by_phy
check that this matches the phandle args[0] value when looking for
the otg controller.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Add a args0 parameter instead of looking for nodes with a dr_mode property
Changes in v3:
-No changes
---
 drivers/usb/common/common.c  | 31 ++-
 drivers/usb/phy/phy-am335x.c |  2 +-
 include/linux/usb/of.h   |  4 ++--
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index e3d0161..bd716f4 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -131,32 +131,37 @@ EXPORT_SYMBOL_GPL(usb_get_dr_mode);
  * of_usb_get_dr_mode_by_phy - Get dual role mode for the controller device
  * which is associated with the given phy device_node
  * @np:Pointer to the given phy device_node
+ * @arg0: phandle args[0] for phy's with #phy-cells >= 1
  *
  * In dts a usb controller associates with phy devices.  The function gets
  * the string from property 'dr_mode' of the controller associated with the
  * given phy device node, and returns the correspondig enum usb_dr_mode.
  */
-enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *phy_np)
+enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 {
struct device_node *controller = NULL;
-   struct device_node *phy;
+   struct of_phandle_args args;
const char *dr_mode;
int index;
int err;
+   bool found = false;
 
do {
controller = of_find_node_with_property(controller, "phys");
-   index = 0;
-   do {
-   phy = of_parse_phandle(controller, "phys", index);
-   of_node_put(phy);
-   if (phy == phy_np)
-   goto finish;
-   index++;
-   } while (phy);
-   } while (controller);
-
-finish:
+   for (index = 0; !found; index++) {
+   err = of_parse_phandle_with_args(controller, "phys",
+   "#phy-cells", index, );
+   if (err)
+   break;
+
+   if (args.np == np && (args.args_count == 0 ||
+ args.args[0] == arg0))
+   found = true;
+
+   of_node_put(args.np);
+   }
+   } while (controller && !found);
+
err = of_property_read_string(controller, "dr_mode", _mode);
of_node_put(controller);
 
diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index a262a43..7e5aece 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -54,7 +54,7 @@ static int am335x_phy_probe(struct platform_device *pdev)
return am_phy->id;
}
 
-   am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node);
+   am_phy->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1);
 
ret = usb_phy_gen_create_phy(dev, _phy->usb_phy_gen, NULL);
if (ret)
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index de3237f..5ff9032 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -12,7 +12,7 @@
 #include 
 
 #if IS_ENABLED(CONFIG_OF)
-enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *phy_np);
+enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0);
 bool of_usb_host_tpl_support(struct device_node *np);
 int of_usb_update_otg_caps(struct device_node *np,
struct usb_otg_caps *otg_caps);
@@ -20,7 +20,7 @@ struct device_node *usb_of_get_child_node(struct device_node 
*parent,
int portnum);
 #else
 static inline enum usb_dr_mode
-of_usb_get_dr_mode_by_phy(struct device_node *phy_np)
+of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 {
return USB_DR_MODE_UNKNOWN;
 }
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/4] phy-sun4i-usb: Add support for peripheral-only mode

[Hans de Goede]

Use the new of_usb_get_dr_mode_by_phy() function to get the dr_mode
from the musb controller node instead of assuming that having an id_det
gpio means otg mode, and not having one means host mode.

Implement peripheral-only mode by adding a sun4i_usb_phy0_get_id_det
helper which looks at the dr_mode, always registering our extcon and
always monitoring vbus.

If dr_mode is not specified in the dts, do not register phy0 as we then
do not know how to treat it. This is actually a good thing as this means
we will not be registering phy0 on devices where the otg controller is
not enabled in the devicetree.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Adjust for of_usb_get_dr_mode_by_phy now taking an args0 parameter
Changes in v3:
-Only toggle the phy vbus-det bit on id-change on systems without vbus-det
 when in otg mode
---
 drivers/phy/phy-sun4i-usb.c | 68 ++---
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index bae54f7..e3cbaae 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define REG_ISCR   0x00
@@ -109,6 +110,7 @@ struct sun4i_usb_phy_cfg {
 struct sun4i_usb_phy_data {
void __iomem *base;
const struct sun4i_usb_phy_cfg *cfg;
+   enum usb_dr_mode dr_mode;
struct mutex mutex;
struct sun4i_usb_phy {
struct phy *phy;
@@ -119,6 +121,7 @@ struct sun4i_usb_phy_data {
bool regulator_on;
int index;
} phys[MAX_PHYS];
+   int first_phy;
/* phy0 / otg related variables */
struct extcon_dev *extcon;
bool phy0_init;
@@ -285,16 +288,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_DPDM_PULLUP_EN);
sun4i_usb_phy0_update_iscr(_phy, 0, ISCR_ID_PULLUP_EN);
 
-   if (data->id_det_gpio) {
-   /* OTG mode, force ISCR and cable state updates */
-   data->id_det = -1;
-   data->vbus_det = -1;
-   queue_delayed_work(system_wq, >detect, 0);
-   } else {
-   /* Host only mode */
-   sun4i_usb_phy0_set_id_detect(_phy, 0);
-   sun4i_usb_phy0_set_vbus_detect(_phy, 1);
-   }
+   /* Force ISCR and cable state updates */
+   data->id_det = -1;
+   data->vbus_det = -1;
+   queue_delayed_work(system_wq, >detect, 0);
}
 
return 0;
@@ -319,6 +316,19 @@ static int sun4i_usb_phy_exit(struct phy *_phy)
return 0;
 }
 
+static int sun4i_usb_phy0_get_id_det(struct sun4i_usb_phy_data *data)
+{
+   switch (data->dr_mode) {
+   case USB_DR_MODE_OTG:
+   return gpiod_get_value_cansleep(data->id_det_gpio);
+   case USB_DR_MODE_HOST:
+   return 0;
+   case USB_DR_MODE_PERIPHERAL:
+   default:
+   return 1;
+   }
+}
+
 static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
 {
if (data->vbus_det_gpio)
@@ -414,7 +424,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
struct phy *phy0 = data->phys[0].phy;
int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
 
-   id_det = gpiod_get_value_cansleep(data->id_det_gpio);
+   if (phy0 == NULL)
+   return;
+
+   id_det = sun4i_usb_phy0_get_id_det(data);
vbus_det = sun4i_usb_phy0_get_vbus_det(data);
 
mutex_lock(>mutex);
@@ -430,7 +443,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
 * without vbus detection report vbus low for long enough for
 * the musb-ip to end the current device session.
 */
-   if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
+   if (data->dr_mode == USB_DR_MODE_OTG &&
+   !sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
sun4i_usb_phy0_set_vbus_detect(phy0, 0);
msleep(200);
sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -456,7 +470,8 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct 
work_struct *work)
 * without vbus detection report vbus low for long enough to
 * the musb-ip to end the current host session.
 */
-   if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
+   if (data->dr_mode == USB_DR_MODE_OTG &&
+   !sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
mutex_lock(>mutex);
sun4i_usb_phy0_set_vbus_detect(phy0, 0);
msleep(1000);
@@ -501,7 +516,8 @@ 

Re: [PATCH v2,3/5] dt-bindings: mtu3: add devicetree bindings

[chunfeng yun]

On Fri, 2016-06-03 at 15:34 +0200, Matthias Brugger wrote:
> 
> On 31/05/16 07:52, Chunfeng Yun wrote:
> > add a DT binding doc for MediaTek USB3 DRD driver
> >
> > Signed-off-by: Chunfeng Yun 
> > ---
> >   Documentation/devicetree/bindings/usb/mtu3.txt |   85 
> > 
> >   1 file changed, 85 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/usb/mtu3.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/mtu3.txt 
> > b/Documentation/devicetree/bindings/usb/mtu3.txt
> > new file mode 100644
> > index 000..571ae8b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/mtu3.txt
> 
> The file should be called mt8173-mtu3.txt to be in line with mt8173-xhci.txt
> Ok, I will rename it
> 
> > @@ -0,0 +1,85 @@
> > +The device node for Mediatek USB3.0 DRD controller
> > +
> > +Required properties:
> > + - compatible : should be "mediatek,mt8173-mtu3"
> > + - reg : specifies physical base address and size of the registers
> > + - reg-names: should be "mac" for device IP and "ippc" for IP port control
> > + - interrupts : interrupt used by the device IP
> > + - power-domains : a phandle to USB power domain node to control USB's
> > +   mtcmos
> > + - vusb33-supply : regulator of USB avdd3.3v
> > + - clocks : a list of phandle + clock-specifier pairs, one for each
> > +   entry in clock-names
> > + - clock-names : must contain "sys_ck" for clock of controller;
> > +   "wakeup_deb_p0" and "wakeup_deb_p1" are optional, they are
> > +   depends on "mediatek,enable-wakeup"
> > + - phys : a list of phandle + phy specifier pairs
> > + - dr_mode : should be one of "host", "peripheral" or "otg",
> > +   refer to usb/generic.txt
> > +
> > +Optional properties:
> > + - #address-cells, #size-cells : should be '2' if the device has sub-nodes
> > +   with 'reg' property
> > + - ranges : allows valid 1:1 translation between child's address space and
> > +   parent's address space
> > + - extcon : external connector for vbus and idpin changes detection, needed
> > +   when supports dual-role mode.
> > + - vbus-supply : reference to the VBUS regulator, needed when supports
> > +   dual-role mode.
> > + - pinctl-names : a pinctrl state named "default" must be defined,
> > +   "id_float" and "id_ground" are optinal which depends on
> > +   "mediatek,enable-manual-drd"
> 
> So pinctrl is optional, but if you put it, you need at least "default"?
Yes, the first one is "default".

> 
> Apart from that, DT maintainers prefer to have the binding patches at 
> the beginning of a series, so it's easier for them to not oversee any.
> Ok.

Thanks a lot.
> Regards,
> Matthias



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Lu Baolu]

Hi,

On 06/05/2016 04:33 PM, Jun Li wrote:
>> Port mux is part of dual role switch, but not the whole thing.
>> > 
>> > Dual role switch includes at least below things:
>> >  - ID or type-C event detection
>> >  - port mux
>> >  - VBUS management
>> >  - start/stop host/device controllers
>> > 
>> > An OTG/Dual-role framework can be used to keep all these things run
>> > together with an internal state machine. But it's not duplicated with a
>> > generic framework for port mux and the port mux drivers.
>> > 
>>> > > Your
>>> > > case is just like Renesas case, which uses two different drivers
>>> > > between peripheral and host[1].
>> > 
>> > In my case, the port mux devices are physical devices and they can be
>> > controlled through GPIO pins or device registers. They are independent of
>> > both peripheral and host controllers.
>> > 
> I also think current OTG/Dual role framework can support your case, if you
> find there is any limitation of it which can't meet your requirement, we
> should improve it, Roger also provide an example of dual role switch with
> USB3 based on his OTG core.

Why do we need an OTG framework to support a device driver?
Is it something like a bus or class driver?

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Jun Li]

Hi

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Lu Baolu
> Sent: Sunday, June 05, 2016 2:56 PM
> To: Peter Chen 
> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
> Greg Kroah-Hartman ; Lee Jones
> ; Heikki Krogerus ;
> Liam Girdwood ; Mark Brown ;
> linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Roger Quadros
> 
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi Peter,
> 
> On 06/04/2016 10:28 AM, Peter Chen wrote:
> > On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>> from my point,it is a dual-role switch driver too,
> >> No, it's not a dual-role switch driver, but a driver for USB port
> multiplexing.
> >>
> >> One example of port multiplexing can be found in several Intel SOC
> >> and PCH chips, inside of which, there are two independent USB
> >> controllers: host and device. They might share a single port and this
> >> port could be configured to route the line to one of these two
> >> controllers. This patch introduced a generic framework for port mux
> >> drivers. It aids the drivers to handle port mux by providing
> >> interfaces to 1) register/unregister a mux device; 2) lookup the mux
> device; and 3) switch the port.
> >>
> > For this case, I can't see it is different with dual-role switch.
> 
> Port mux is part of dual role switch, but not the whole thing.
> 
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
> 
> An OTG/Dual-role framework can be used to keep all these things run
> together with an internal state machine. But it's not duplicated with a
> generic framework for port mux and the port mux drivers.
> 
> > Your
> > case is just like Renesas case, which uses two different drivers
> > between peripheral and host[1].
> 
> In my case, the port mux devices are physical devices and they can be
> controlled through GPIO pins or device registers. They are independent of
> both peripheral and host controllers.
> 

I also think current OTG/Dual role framework can support your case, if you
find there is any limitation of it which can't meet your requirement, we
should improve it, Roger also provide an example of dual role switch with
USB3 based on his OTG core.

> 
> >> Port multiplexing isn't equal to USB dual role. There are other cases
> >> in today's systems. In several Intel PCH chips, there equips two USB
> >> host controllers: ehci and xhci. The xhci USB2 ports are multiplexed
> >> with ehci. This guarantees all USB ports work even running an old
> version of OS which lacks of USB3 support.
> >> In theory, we can create a driver for the port mux and switch the
> >> ports between xhci and ehci, but that's silly, isn't it? Why not
> >> always USB3?:-)
> >>
> >> Another case is xHCI debug capability. The xHCI host controller might
> >> equip a unit for system debugging (refer to 7.6 of xHCI spec). The
> >> debugging unit is independent of xhci host controller. But it shares
> >> its port with xhci. Software could switch the port between xhci and
> >> the debugging unit through the registers defined in xHCI spec.
> >>
> > Yes, above two are different with dual role switch. But in your code
> > and Kconfig, it seems this framework is dedicated for dual-role. Eg:
> >
> > +menuconfig USB_PORTMUX
> > +   bool "USB dual role port MUX support"
> > +   help
> > + Generic USB dual role port mux support.
> 
> Above two cases are examples for port multiplexing, but I don't think we
> need drivers for them. At this moment, this framework is only for dual
> role port mux devices.
> 
> >
> > I think a general dual role port mux is necessary, it can be used to
> > manage different dual-role switch method, eg
> 
> Yes, I agree.
> 
> > - ID pin
> > - External connector through GPIO
> > - SoC register
> > - sysfs
> > - type-C events
> 
> ID pin and type-C events are the *reasons* which trigger the port mux
> switch. Currently, on our platforms, gpio, registers and sysfs are methods
> to control the mux.

Those methods also can be mapped/added into OTG/dual role core framework.

> 
> >
> > But this code is better co-work with OTG/Dual-role framework, we'd
> > better have only interface that the user can know which role for the
> > current port.
> 
> OTG/Dual-role framework and portmux framework are not overlapped.

>From previous discussion, I still think this is a special dual role
switch:) which should be covered by general OTG/dual role core(this
core framework is target to support one port with both host and
device capable by any HW/IC implementations).

Li Jun

> The sysfs interface shouldn't be overlapped as well. Say, I have a port
> mux device and 

Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

[Lu Baolu]

Hi Peter,

On 06/04/2016 10:28 AM, Peter Chen wrote:
> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
>>> from my point,it is a dual-role switch
>>> driver too,
>> No, it's not a dual-role switch driver, but a driver for USB port 
>> multiplexing.
>>
>> One example of port multiplexing can be found in several Intel SOC and PCH
>> chips, inside of which, there are two independent USB controllers: host and
>> device. They might share a single port and this port could be configured to
>> route the line to one of these two controllers. This patch introduced a 
>> generic
>> framework for port mux drivers. It aids the drivers to handle port mux by
>> providing interfaces to 1) register/unregister a mux device; 2) lookup the
>> mux device; and 3) switch the port.
>>
> For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
 - ID or type-C event detection
 - port mux
 - VBUS management
 - start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these
things run together with an internal state machine. But it's
not duplicated with a generic framework for port mux and
the port mux drivers.

> Your
> case is just like Renesas case, which uses two different drivers between
> peripheral and host[1].

In my case, the port mux devices are physical devices and they
can be controlled through GPIO pins or device registers. They
are independent of both peripheral and host controllers.


>> Port multiplexing isn't equal to USB dual role. There are other cases in 
>> today's
>> systems. In several Intel PCH chips, there equips two USB host controllers: 
>> ehci
>> and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all
>> USB ports work even running an old version of OS which lacks of USB3 support.
>> In theory, we can create a driver for the port mux and switch the ports 
>> between
>> xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)
>>
>> Another case is xHCI debug capability. The xHCI host controller might equip
>> a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit 
>> is
>> independent of xhci host controller. But it shares its port with xhci. 
>> Software
>> could switch the port between xhci and the debugging unit through the 
>> registers
>> defined in xHCI spec.
>>
> Yes, above two are different with dual role switch. But in your code and
> Kconfig, it seems this framework is dedicated for dual-role. Eg:
>
> +menuconfig USB_PORTMUX
> +   bool "USB dual role port MUX support"
> +   help
> + Generic USB dual role port mux support.

Above two cases are examples for port multiplexing, but I don't think we
need drivers for them. At this moment, this framework is only for dual
role port mux devices.

>
> I think a general dual role port mux is necessary, it can be used to
> manage different dual-role switch method, eg

Yes, I agree.

> - ID pin
> - External connector through GPIO
> - SoC register
> - sysfs
> - type-C events

ID pin and type-C events are the *reasons* which trigger the port
mux switch. Currently, on our platforms, gpio, registers and sysfs
are methods to control the mux.

>
> But this code is better co-work with OTG/Dual-role framework, we'd
> better have only interface that the user can know which role for the
> current port.

OTG/Dual-role framework and portmux framework are not overlapped.
The sysfs interface shouldn't be overlapped as well. Say, I have a port
mux device and I have a driver for it. I am able to read the status of my
port mux device through sysfs. This is not part of OTG/Dual-role as far
as I can see.

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html