Re: [RFC PATCH v2 4/4] ARM: dts: vf-colibri: USB device/host switch using extcon gpio

2016-05-13 Thread Peter Chen
On Thu, May 12, 2016 at 02:47:09PM +0530, maitysancha...@gmail.com wrote:
> Hello Peter,
> 
> On 16-05-12 16:40:16, Peter Chen wrote:
> > On Wed, May 11, 2016 at 06:11:36PM +0530, Sanchayan Maity wrote:
> > > Use USBC_DET feature of standard Colibri SODIMM pin 137 for USB
> > > device/host switching using the generic extcon USB gpio implementation.
> > > 
> > > Signed-off-by: Sanchayan Maity 
> > > ---
> > >  arch/arm/boot/dts/vf-colibri-eval-v3.dtsi | 12 
> > >  arch/arm/boot/dts/vf-colibri.dtsi |  7 +++
> > >  2 files changed, 19 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi 
> > > b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
> > > index a8a8e43..a012f80 100644
> > > --- a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
> > > +++ b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
> > > @@ -50,6 +50,14 @@
> > >   clock-frequency = <1600>;
> > >   };
> > >  
> > > + extcon_usbc_det: usbc_det {
> > > + compatible = "linux,extcon-usb-gpio";
> > > + debounce = <25>;
> > > + id-gpio = <&gpio3 6 GPIO_ACTIVE_HIGH>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&pinctrl_usbc_det>;
> > > + };
> > > +
> > >   panel: panel {
> > >   compatible = "edt,et057090dhu";
> > >   backlight = <&bl>;
> > > @@ -162,6 +170,10 @@
> > >   status = "okay";
> > >  };
> > >  
> > > +&usbdev0 {
> > > + extcon = <&extcon_usbc_det>, <&extcon_usbc_det>;
> > > +};
> > > +
> > 
> > I still not understand why vbus needs id-extcon too? Per my understand,
> > this pin will not be changed between connects to pc and disconnects to
> > pc.
> 
> While the ID extcon will take care of switching the role, without the
> vbus gadget connect/disconnect being called appropriately as per the
> current state, I have observed the client configuration to not work.
> 
> From the descriptions of usb_gadget_vbus_connect/disconnect I understand
> this to be required. Do I understand wrong?

Your understand is right, but most of problem is your platform is lack
of vbus detect, so you don't know when to call usb_gadget_vbus_connect/
disconnect.

extcon_usbc_det is the external connector for ID, you can't use it for
both ID and VBUS. If you can add one gpio for vbus, then current framework
can support.

Roger is working on DRD/OTG framework [1], currently, it needs both ID
and VBUS input for state machine. Your case is the new case, it has only
one input (ID), but role switch is needed to support too.

[1] http://www.spinics.net/lists/linux-usb/msg140138.html
-- 

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


[PATCH v2 3/5] usb: dwc3: add phyif_utmi_quirk

2016-05-13 Thread William Wu
Add a quirk to configure the core to support the
UTMI+ PHY with an 8- or 16-bit interface. UTMI+ PHY
interface is hardware property, and it's platform
dependent. Normall, the PHYIf can be configured
during coreconsultant. But for some specific usb
cores(e.g. rk3399 soc dwc3), the default PHYIf
configuration value is fault, so we need to
reconfigure it by software.

And refer to the dwc3 databook, the GUSB2PHYCFG.USBTRDTIM
must be set to the corresponding value according to
the UTMI+ PHY interface.

Signed-off-by: William Wu 
---
Changes in v2:
- add a quirk for phyif_utmi (Felipe)

 Documentation/devicetree/bindings/usb/dwc3.txt |  4 
 drivers/usb/dwc3/core.c| 23 +++
 drivers/usb/dwc3/core.h| 12 
 drivers/usb/dwc3/platform_data.h   |  2 ++
 4 files changed, 41 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 1ada121..34d13a5 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -42,6 +42,10 @@ Optional properties:
  - snps,dis_u2_freeclk_exists_quirk: when set, clear the u2_freeclk_exists
in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
a free-running PHY clock.
+ - snps,phyif_utmi_quirk: when set core will set phyif UTMI+ interface.
+ - snps,phyif_utmi: the value to configure the core to support a UTMI+ PHY
+   with an 8- or 16-bit interface. Value 0 select 8-bit
+   interface, value 1 select 16-bit interface.
  - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
utmi_l1_suspend_n, false when asserts utmi_sleep_n
  - snps,hird-threshold: HIRD threshold
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 8bcd3cc..d99c170 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -410,6 +410,7 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc)
 static int dwc3_phy_setup(struct dwc3 *dwc)
 {
u32 reg;
+   u32 usbtrdtim;
int ret;
 
reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
@@ -505,6 +506,15 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_u2_freeclk_exists_quirk)
reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
 
+   if (dwc->phyif_utmi_quirk) {
+   reg &= ~(DWC3_GUSB2PHYCFG_PHYIF_MASK |
+  DWC3_GUSB2PHYCFG_USBTRDTIM_MASK);
+   usbtrdtim = dwc->phyif_utmi ? USBTRDTIM_UTMI_16_BIT :
+   USBTRDTIM_UTMI_8_BIT;
+   reg |= DWC3_GUSB2PHYCFG_PHYIF(dwc->phyif_utmi) |
+  DWC3_GUSB2PHYCFG_USBTRDTIM(usbtrdtim);
+   }
+
dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
return 0;
@@ -800,6 +810,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct resource *res;
struct dwc3 *dwc;
u8  lpm_nyet_threshold;
+   u8  phyif_utmi;
u8  tx_de_emphasis;
u8  hird_threshold;
u32 fladj = 0;
@@ -857,6 +868,9 @@ static int dwc3_probe(struct platform_device *pdev)
/* default to highest possible threshold */
lpm_nyet_threshold = 0xff;
 
+   /* default to UTMI+ 8-bit interface */
+   phyif_utmi = 0;
+
/* default to -3.5dB de-emphasis */
tx_de_emphasis = 1;
 
@@ -907,6 +921,10 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
"snps,dis_u2_freeclk_exists_quirk");
 
+   dwc->phyif_utmi_quirk = device_property_read_bool(dev,
+   "snps,phyif_utmi_quirk");
+   device_property_read_u8(dev, "snps,phyif_utmi",
+   &phyif_utmi);
dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
"snps,tx_de_emphasis_quirk");
device_property_read_u8(dev, "snps,tx_de_emphasis",
@@ -943,6 +961,10 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->dis_u2_freeclk_exists_quirk =
pdata->dis_u2_freeclk_exists_quirk;
 
+   dwc->phyif_utmi_quirk = pdata->phyif_utmi_quirk;
+   if (pdata->phyif_utmi)
+   phyif_utmi = pdata->phyif_utmi;
+
dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
if (pdata->tx_de_emphasis)
tx_de_emphasis = pdata->tx_de_emphasis;
@@ -952,6 +974,7 @@ static int dwc3_probe(struct platform_device *pdev)
}
 
dwc->lpm_nyet_threshold = lpm_nyet_threshold;
+   dwc->phyif_utmi = phyif_utmi;
dwc->tx_de_emphasis = tx_de_emphasis;
 
dwc->hi

[PATCH v2 4/5] usb: dwc3: add dis_del_phy_power_chg_quirk

2016-05-13 Thread William Wu
Add a quirk to clear the GUSB3PIPECTL.DELAYP1TRANS bit,
which specifies whether disable delay PHY power change
from P0 to P1/P2/P3 when link state changing from U0
to U1/U2/U3 respectively.

Signed-off-by: William Wu 
---
Changes in v2:
- None

 Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
 drivers/usb/dwc3/core.c| 7 +++
 drivers/usb/dwc3/core.h| 3 +++
 drivers/usb/dwc3/platform_data.h   | 1 +
 4 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 34d13a5..bd5bef0 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -42,6 +42,8 @@ Optional properties:
  - snps,dis_u2_freeclk_exists_quirk: when set, clear the u2_freeclk_exists
in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
a free-running PHY clock.
+ - snps,dis_del_phy_power_chg_quirk: when set core will change PHY power
+   from P0 to P1/P2/P3 without delay.
  - snps,phyif_utmi_quirk: when set core will set phyif UTMI+ interface.
  - snps,phyif_utmi: the value to configure the core to support a UTMI+ PHY
with an 8- or 16-bit interface. Value 0 select 8-bit
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index d99c170..c06870c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -451,6 +451,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_u3_susphy_quirk)
reg &= ~DWC3_GUSB3PIPECTL_SUSPHY;
 
+   if (dwc->dis_del_phy_power_chg_quirk)
+   reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
+
dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
 
reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
@@ -920,6 +923,8 @@ static int dwc3_probe(struct platform_device *pdev)
"snps,dis_rxdet_inp3_quirk");
dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
"snps,dis_u2_freeclk_exists_quirk");
+   dwc->dis_del_phy_power_chg_quirk = device_property_read_bool(dev,
+   "snps,dis_del_phy_power_chg_quirk");
 
dwc->phyif_utmi_quirk = device_property_read_bool(dev,
"snps,phyif_utmi_quirk");
@@ -960,6 +965,8 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->dis_rxdet_inp3_quirk = pdata->dis_rxdet_inp3_quirk;
dwc->dis_u2_freeclk_exists_quirk =
pdata->dis_u2_freeclk_exists_quirk;
+   dwc->dis_del_phy_power_chg_quirk =
+   pdata->dis_del_phy_power_chg_quirk;
 
dwc->phyif_utmi_quirk = pdata->phyif_utmi_quirk;
if (pdata->phyif_utmi)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e1fcae8..abed84f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -780,6 +780,8 @@ struct dwc3_scratchpad_array {
  * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists
  * in GUSB2PHYCFG, specify that USB2 PHY doesn't
  * provide a free-running PHY clock.
+ * @dis_del_phy_power_chg_quirk: set if we disable delay phy power
+ * change quirk.
  * @phyif_utmi_quirk: set if we enable phyif UTMI+ quirk
  * @phyif_utmi: UTMI+ PHY interface value
  * 0   - 8 bits
@@ -928,6 +930,7 @@ struct dwc3 {
unsigneddis_enblslpm_quirk:1;
unsigneddis_rxdet_inp3_quirk:1;
unsigneddis_u2_freeclk_exists_quirk:1;
+   unsigneddis_del_phy_power_chg_quirk:1;
 
unsignedphyif_utmi_quirk:1;
unsignedphyif_utmi:1;
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index b521565..ab45d91 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -44,6 +44,7 @@ struct dwc3_platform_data {
unsigned dis_enblslpm_quirk:1;
unsigned dis_rxdet_inp3_quirk:1;
unsigned dis_u2_freeclk_exists_quirk:1;
+   unsigned dis_del_phy_power_chg_quirk:1;
 
unsigned phyif_utmi_quirk:1;
unsigned phyif_utmi:1;
-- 
1.9.1


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


[PATCH v2 1/5] usb: dwc3: of-simple: add compatible for rockchip

2016-05-13 Thread William Wu
Rockchip platform merely enable usb3 clocks and
populate its children. So we can use this generic
glue layer to support Rockchip dwc3.

Signed-off-by: William Wu 
---
Changes in v2:
- sort the list of_dwc3_simple_match (Doug)

 drivers/usb/dwc3/dwc3-of-simple.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index 9743353..6da9656 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -161,6 +161,7 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {
 
 static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "qcom,dwc3" },
+   { .compatible = "rockchip,dwc3" },
{ .compatible = "xlnx,zynqmp-dwc3" },
{ /* Sentinel */ }
 };
-- 
1.9.1


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


[PATCH v2 0/5] support rockchip dwc3 driver

2016-05-13 Thread William Wu
This series add support for rockchip dwc3 driver,
and add additional optional properties for specific
platforms (e.g., rockchip platform).

William Wu (5):
  usb: dwc3: of-simple: add compatible for rockchip
  usb: dwc3: add dis_u2_freeclk_exists_quirk
  usb: dwc3: add phyif_utmi_quirk
  usb: dwc3: add dis_del_phy_power_chg_quirk
  usb: dwc3: rockchip: add devicetree bindings documentation

 Documentation/devicetree/bindings/usb/dwc3.txt |  9 +
 .../devicetree/bindings/usb/rockchip,dwc3.txt  | 45 ++
 drivers/usb/dwc3/core.c| 39 ++-
 drivers/usb/dwc3/core.h| 20 ++
 drivers/usb/dwc3/dwc3-of-simple.c  |  1 +
 drivers/usb/dwc3/platform_data.h   |  4 ++
 6 files changed, 117 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

-- 
1.9.1


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


[PATCH v2 2/5] usb: dwc3: add dis_u2_freeclk_exists_quirk

2016-05-13 Thread William Wu
Add a quirk to clear the GUSB2PHYCFG.U2_FREECLK_EXISTS bit,
which specifies whether the USB2.0 PHY provides a free-running
PHY clock, which is active when the clock control input is active.

Signed-off-by: William Wu 
---
Changes in v2:
- None

 Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++
 drivers/usb/dwc3/core.c| 7 +++
 drivers/usb/dwc3/core.h| 5 +
 drivers/usb/dwc3/platform_data.h   | 1 +
 4 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 7d7ce08..1ada121 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -39,6 +39,9 @@ Optional properties:
disabling the suspend signal to the PHY.
  - snps,dis_rxdet_inp3_quirk: when set core will disable receiver detection
in PHY P3 power state.
+ - snps,dis_u2_freeclk_exists_quirk: when set, clear the u2_freeclk_exists
+   in GUSB2PHYCFG, specify that USB2 PHY doesn't provide
+   a free-running PHY clock.
  - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
utmi_l1_suspend_n, false when asserts utmi_sleep_n
  - snps,hird-threshold: HIRD threshold
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index a590cd2..8bcd3cc 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -502,6 +502,9 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_enblslpm_quirk)
reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
 
+   if (dwc->dis_u2_freeclk_exists_quirk)
+   reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
+
dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
return 0;
@@ -901,6 +904,8 @@ static int dwc3_probe(struct platform_device *pdev)
"snps,dis_enblslpm_quirk");
dwc->dis_rxdet_inp3_quirk = device_property_read_bool(dev,
"snps,dis_rxdet_inp3_quirk");
+   dwc->dis_u2_freeclk_exists_quirk = device_property_read_bool(dev,
+   "snps,dis_u2_freeclk_exists_quirk");
 
dwc->tx_de_emphasis_quirk = device_property_read_bool(dev,
"snps,tx_de_emphasis_quirk");
@@ -935,6 +940,8 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->dis_u2_susphy_quirk = pdata->dis_u2_susphy_quirk;
dwc->dis_enblslpm_quirk = pdata->dis_enblslpm_quirk;
dwc->dis_rxdet_inp3_quirk = pdata->dis_rxdet_inp3_quirk;
+   dwc->dis_u2_freeclk_exists_quirk =
+   pdata->dis_u2_freeclk_exists_quirk;
 
dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
if (pdata->tx_de_emphasis)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 7ddf944..ac2e6b5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -196,6 +196,7 @@
 
 /* Global USB2 PHY Configuration Register */
 #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31)
+#defineDWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS  (1 << 30)
 #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6)
 #define DWC3_GUSB2PHYCFG_ULPI_UTMI (1 << 4)
 #define DWC3_GUSB2PHYCFG_ENBLSLPM  (1 << 8)
@@ -770,6 +771,9 @@ struct dwc3_scratchpad_array {
  * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy
  * @dis_enblslpm_quirk: set if we clear enblslpm in GUSB2PHYCFG,
  *  disabling the suspend signal to the PHY.
+ * @dis_u2_freeclk_exists_quirk : set if we clear u2_freeclk_exists
+ * in GUSB2PHYCFG, specify that USB2 PHY doesn't
+ * provide a free-running PHY clock.
  * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
  * @tx_de_emphasis: Tx de-emphasis value
  * 0   - -6dB de-emphasis
@@ -913,6 +917,7 @@ struct dwc3 {
unsigneddis_u2_susphy_quirk:1;
unsigneddis_enblslpm_quirk:1;
unsigneddis_rxdet_inp3_quirk:1;
+   unsigneddis_u2_freeclk_exists_quirk:1;
 
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h
index 8826cca..e1a1631 100644
--- a/drivers/usb/dwc3/platform_data.h
+++ b/drivers/usb/dwc3/platform_data.h
@@ -43,6 +43,7 @@ struct dwc3_platform_data {
unsigned dis_u2_susphy_quirk:1;
unsigned dis_enblslpm_quirk:1;
unsigned dis_rxdet_inp3_quirk:1;
+   unsigned dis_u2_freeclk_exists_quirk:1;
 
unsigned tx_de_emphasis_quirk:1;
unsigned tx_de_emphasis:2;
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majo

Re: [PATCH v2 0/5] support rockchip dwc3 driver

2016-05-13 Thread Heiko Stuebner
Hi William,

Am Freitag, 13. Mai 2016, 17:24:56 schrieb William Wu:
> This series add support for rockchip dwc3 driver,
> and add additional optional properties for specific
> platforms (e.g., rockchip platform).

when submitting new versions of patchsets please also start a new thread.
It is hard to find such new versions in a big threaded mail view later on.

No need to resend this v2, just to keep in mind for later patches.


Heiko
--
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 v2 0/5] support rockchip dwc3 driver

2016-05-13 Thread Felipe Balbi

Hi,

William Wu  writes:
> This series add support for rockchip dwc3 driver,
> and add additional optional properties for specific
> platforms (e.g., rockchip platform).
>
> William Wu (5):
>   usb: dwc3: of-simple: add compatible for rockchip
>   usb: dwc3: add dis_u2_freeclk_exists_quirk
>   usb: dwc3: add phyif_utmi_quirk
>   usb: dwc3: add dis_del_phy_power_chg_quirk
>   usb: dwc3: rockchip: add devicetree bindings documentation
>
>  Documentation/devicetree/bindings/usb/dwc3.txt |  9 +
>  .../devicetree/bindings/usb/rockchip,dwc3.txt  | 45 
> ++
>  drivers/usb/dwc3/core.c| 39 ++-
>  drivers/usb/dwc3/core.h| 20 ++
>  drivers/usb/dwc3/dwc3-of-simple.c  |  1 +
>  drivers/usb/dwc3/platform_data.h   |  4 ++
>  6 files changed, 117 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

I didn't get patch 5/5 :-s

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v2 0/5] support rockchip dwc3 driver

2016-05-13 Thread William Wu

Dear Felipe,
On 05/13/2016 05:37 PM, Felipe Balbi wrote:

Hi,

William Wu  writes:

This series add support for rockchip dwc3 driver,
and add additional optional properties for specific
platforms (e.g., rockchip platform).

William Wu (5):
   usb: dwc3: of-simple: add compatible for rockchip
   usb: dwc3: add dis_u2_freeclk_exists_quirk
   usb: dwc3: add phyif_utmi_quirk
   usb: dwc3: add dis_del_phy_power_chg_quirk
   usb: dwc3: rockchip: add devicetree bindings documentation

  Documentation/devicetree/bindings/usb/dwc3.txt |  9 +
  .../devicetree/bindings/usb/rockchip,dwc3.txt  | 45 ++
  drivers/usb/dwc3/core.c| 39 ++-
  drivers/usb/dwc3/core.h| 20 ++
  drivers/usb/dwc3/dwc3-of-simple.c  |  1 +
  drivers/usb/dwc3/platform_data.h   |  4 ++
  6 files changed, 117 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

I didn't get patch 5/5 :-s

Sorry, maybe there are something wrong with our mailbox server.
I'll resend patch 5/5 later.





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


[PATCH v2 5/5] usb: dwc3: rockchip: add devicetree bindings documentation

2016-05-13 Thread William Wu
This patch documents the device tree documentation required for
Rockchip USB3.0 core wrapper consist of USB3.0 IP from Synopsys.

It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).

Signed-off-by: William Wu 
---
Changes in v2:
- add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (Felipe, Brian)

 .../devicetree/bindings/usb/rockchip,dwc3.txt  | 45 ++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt

diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt 
b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
new file mode 100644
index 000..10303d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
@@ -0,0 +1,45 @@
+Rockchip SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:  should contain "rockchip,dwc3"
+- clocks:  A list of phandle + clock-specifier pairs for the
+   clocks listed in clock-names
+- clock-names: Should contain the following:
+  "clk_usb3otg0_ref"   Controller reference clk
+  "clk_usb3otg0_suspend"Controller suspend clk, can use 24 MHz or 32 KHz
+  "aclk_usb3"  Master/Core clock, have to be >= 62.5 MHz for SS 
operation
+
+
+Optional clocks:
+  "aclk_usb3otg0"  Aclk for specific usb controller clock.
+  "aclk_usb3_rksoc_axi_perf"  USB AXI perf clock.  Not present on all 
platforms.
+  "aclk_usb3_grf"  USB grf clock.  Not present on all platforms.
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Phy documentation is provided in the following places:
+
+Example device nodes:
+
+   usbdrd3_0: usb@fe80 {
+   compatible = "rockchip,dwc3";
+   clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+<&cru ACLK_USB3>, <&cru ACLK_USB3OTG0>,
+<&cru ACLK_USB3_RKSOC_AXI_PERF>, <&cru ACLK_USB3_GRF>;
+   clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
+ "aclk_usb3", "aclk_usb3otg0",
+ "aclk_usb3_rksoc_axi_perf", "aclk_usb3_grf";
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+   status = "disabled";
+   usbdrd_dwc3_0: dwc3 {
+   compatible = "snps,dwc3";
+   reg = <0x0 0xfe80 0x0 0x10>;
+   interrupts = ;
+   dr_mode = "otg";
+   status = "disabled";
+   };
+   };
-- 
1.9.1


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


[PATCH v8 14/14] usb: host: xhci-plat: Add otg device to platform data

2016-05-13 Thread Roger Quadros
Host controllers that are part of an OTG/dual-role instance
need to somehow pass the OTG controller device information
to the HCD core.

We use platform data to pass the OTG controller device.

Signed-off-by: Roger Quadros 
Reviewed-by: Peter Chen 
---
 drivers/usb/host/xhci-plat.c | 35 ---
 include/linux/usb/xhci_pdriver.h |  3 +++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 5c15e9b..84ebe18 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -230,11 +230,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto put_usb3_hcd;
}
 
-   ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+   if (pdata && pdata->otg_dev)
+   ret = usb_otg_add_hcd(hcd, irq, IRQF_SHARED, pdata->otg_dev);
+   else
+   ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+
if (ret)
goto disable_usb_phy;
 
-   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+   if (pdata && pdata->otg_dev)
+   ret = usb_otg_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED,
+ pdata->otg_dev);
+   else
+   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+
if (ret)
goto dealloc_usb2_hcd;
 
@@ -242,7 +251,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
 
 dealloc_usb2_hcd:
-   usb_remove_hcd(hcd);
+   if (pdata && pdata->otg_dev)
+   usb_otg_remove_hcd(hcd);
+   else
+   usb_remove_hcd(hcd);
 
 disable_usb_phy:
usb_phy_shutdown(hcd->usb_phy);
@@ -260,16 +272,25 @@ put_hcd:
return ret;
 }
 
-static int xhci_plat_remove(struct platform_device *dev)
+static int xhci_plat_remove(struct platform_device *pdev)
 {
-   struct usb_hcd  *hcd = platform_get_drvdata(dev);
+   struct usb_hcd  *hcd = platform_get_drvdata(pdev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
+   struct usb_xhci_pdata *pdata = dev_get_platdata(&pdev->dev);
+
+   if (pdata && pdata->otg_dev)
+   usb_otg_remove_hcd(xhci->shared_hcd);
+   else
+   usb_remove_hcd(xhci->shared_hcd);
 
-   usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
 
-   usb_remove_hcd(hcd);
+   if (pdata && pdata->otg_dev)
+   usb_otg_remove_hcd(hcd);
+   else
+   usb_remove_hcd(hcd);
+
usb_put_hcd(xhci->shared_hcd);
 
if (!IS_ERR(clk))
diff --git a/include/linux/usb/xhci_pdriver.h b/include/linux/usb/xhci_pdriver.h
index 376654b..5c68b83 100644
--- a/include/linux/usb/xhci_pdriver.h
+++ b/include/linux/usb/xhci_pdriver.h
@@ -18,10 +18,13 @@
  *
  * @usb3_lpm_capable:  determines if this xhci platform supports USB3
  * LPM capability
+ * @otg_dev:   OTG controller device. Only requied if part of
+ * OTG/dual-role.
  *
  */
 struct usb_xhci_pdata {
unsignedusb3_lpm_capable:1;
+   struct device   *otg_dev;
 };
 
 #endif /* __USB_CORE_XHCI_PDRIVER_H */
-- 
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 v8 11/14] usb: otg: use dev_dbg() instead of VDBG()

2016-05-13 Thread Roger Quadros
Now that we have a device reference in struct usb_otg
let's use dev_dbg() for debug messages.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 drivers/usb/common/usb-otg-fsm.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 2986b66..e6e58c2 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -30,13 +30,6 @@
 #include 
 #include 
 
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
 /* Change USB protocol when there is a protocol change */
 static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 {
@@ -44,8 +37,9 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
int ret = 0;
 
if (fsm->protocol != protocol) {
-   VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
-   fsm->protocol, protocol);
+   dev_vdbg(otg->dev,
+"Changing role fsm->protocol= %d; new protocol= %d\n",
+fsm->protocol, protocol);
/* stop old protocol */
if (fsm->protocol == PROTO_HOST)
ret = otg_start_host(otg, 0);
@@ -226,7 +220,7 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
usb_otg_state new_state)
 
if (otg->state == new_state)
return 0;
-   VDBG("Set state: %s\n", usb_otg_state_string(new_state));
+   dev_vdbg(otg->dev, "Set state: %s\n", usb_otg_state_string(new_state));
otg_leave_state(fsm, otg->state);
switch (new_state) {
case OTG_STATE_B_IDLE:
@@ -358,7 +352,7 @@ int otg_statemachine(struct usb_otg *otg)
 
switch (state) {
case OTG_STATE_UNDEFINED:
-   VDBG("fsm->id = %d\n", fsm->id);
+   dev_vdbg(otg->dev, "fsm->id = %d\n", fsm->id);
if (fsm->id)
otg_set_state(fsm, OTG_STATE_B_IDLE);
else
@@ -466,7 +460,8 @@ int otg_statemachine(struct usb_otg *otg)
}
mutex_unlock(&fsm->lock);
 
-   VDBG("quit statemachine, changed = %d\n", fsm->state_changed);
+   dev_vdbg(otg->dev, "quit statemachine, changed = %d\n",
+fsm->state_changed);
return fsm->state_changed;
 }
 EXPORT_SYMBOL_GPL(otg_statemachine);
-- 
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 v8 09/14] usb: of: add an API to get OTG device from USB controller node

2016-05-13 Thread Roger Quadros
The OTG controller and the USB controller can be linked via the
'otg-controller' property in the USB controller's device node.

of_usb_get_otg() can be used to get the OTG controller device
from the USB controller's device node.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 Documentation/devicetree/bindings/usb/generic.txt |  3 +++
 drivers/usb/common/common.c   | 27 +++
 include/linux/usb/of.h|  9 
 3 files changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
b/Documentation/devicetree/bindings/usb/generic.txt
index bba8257..f6866c1 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -24,6 +24,9 @@ Optional properties:
optional for OTG device.
  - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
optional for OTG device.
+ - otg-controller: phandle to otg controller. Host or gadget controllers can
+   contain this property to link it to a particular OTG
+   controller.
 
 This is an attribute to a USB controller such as:
 
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index e3d0161..d7ec471 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -238,6 +238,33 @@ int of_usb_update_otg_caps(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(of_usb_update_otg_caps);
 
+#ifdef CONFIG_USB_OTG
+/**
+ * of_usb_get_otg - get the OTG controller linked to the USB controller
+ * @np: Pointer to the device_node of the USB controller
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * Returns the OTG controller device or NULL on error.
+ */
+struct device *of_usb_get_otg(struct device_node *np)
+{
+   struct device_node *otg_np;
+   struct platform_device *pdev;
+
+   otg_np = of_parse_phandle(np, "otg-controller", 0);
+   if (!otg_np)
+   return NULL;
+
+   pdev = of_find_device_by_node(otg_np);
+   of_node_put(otg_np);
+   if (!pdev)
+   return NULL;
+
+   return &pdev->dev;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_otg);
+#endif
+
 #endif
 
 MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
index de3237f..499a4e8 100644
--- a/include/linux/usb/of.h
+++ b/include/linux/usb/of.h
@@ -40,6 +40,15 @@ static inline struct device_node *usb_of_get_child_node
 }
 #endif
 
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_OTG)
+struct device *of_usb_get_otg(struct device_node *np);
+#else
+static inline struct device *of_usb_get_otg(struct device_node *np)
+{
+   return NULL;
+}
+#endif
+
 #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_USB_SUPPORT)
 enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np);
 #else
-- 
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 v8 12/14] usb: hcd: Adapt to OTG core

2016-05-13 Thread Roger Quadros
Introduce usb_otg_add/remove_hcd() for use by host
controllers that are part of OTG/dual-role port.

Non Device tree platforms can use the otg_dev argument
to specify the OTG controller device. If otg_dev is NULL
then the device tree node's otg-controller property is used to
get the otg_dev device.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 drivers/usb/core/hcd.c  | 55 +
 include/linux/usb/hcd.h |  4 
 2 files changed, 59 insertions(+)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 9484539..cfc8232 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -46,6 +46,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+#include 
+#include 
 
 #include "usb.h"
 
@@ -3013,6 +3018,56 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(usb_remove_hcd);
 
+static struct otg_hcd_ops otg_hcd_intf = {
+   .add = usb_add_hcd,
+   .remove = usb_remove_hcd,
+   .usb_bus_start_enum = usb_bus_start_enum,
+   .usb_control_msg = usb_control_msg,
+   .usb_hub_find_child = usb_hub_find_child,
+};
+
+/**
+ * usb_otg_add_hcd - Register the HCD with OTG core.
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ * @otg_dev: OTG controller device managing this HCD
+ *
+ * Registers the HCD with OTG core. OTG core will call usb_add_hcd()
+ * or usb_remove_hcd() as necessary.
+ * If otg_dev is NULL then device tree node is checked for OTG
+ * controller device via the otg-controller property.
+ */
+int usb_otg_add_hcd(struct usb_hcd *hcd,
+   unsigned int irqnum, unsigned long irqflags,
+   struct device *otg_dev)
+{
+   struct device *dev = hcd->self.controller;
+
+   if (!otg_dev) {
+   hcd->otg_dev = of_usb_get_otg(dev->of_node);
+   if (!hcd->otg_dev)
+   return -ENODEV;
+   } else {
+   hcd->otg_dev = otg_dev;
+   }
+
+   return usb_otg_register_hcd(hcd, irqnum, irqflags, &otg_hcd_intf);
+}
+EXPORT_SYMBOL_GPL(usb_otg_add_hcd);
+
+/**
+ * usb_otg_remove_hcd - Unregister the HCD with OTG core.
+ * @hcd: the usb_hcd structure to remove
+ *
+ * Unregisters the HCD from the OTG core.
+ */
+void usb_otg_remove_hcd(struct usb_hcd *hcd)
+{
+   usb_otg_unregister_hcd(hcd);
+}
+EXPORT_SYMBOL_GPL(usb_otg_remove_hcd);
+
 void
 usb_hcd_platform_shutdown(struct platform_device *dev)
 {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 2017cd4..adcf2e7 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -472,6 +472,10 @@ extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd);
 extern int usb_add_hcd(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags);
 extern void usb_remove_hcd(struct usb_hcd *hcd);
+extern int usb_otg_add_hcd(struct usb_hcd *hcd,
+  unsigned int irqnum, unsigned long irqflags,
+  struct device *otg_dev);
+extern void usb_otg_remove_hcd(struct usb_hcd *hcd);
 extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1);
 
 struct platform_device;
-- 
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 v8 13/14] usb: gadget: udc: adapt to OTG core

2016-05-13 Thread Roger Quadros
The OTG state machine needs a mechanism to start and
stop the gadget controller as well as connect/disconnect
from the bus. Add usb_gadget_start(), usb_gadget_stop()
and usb_gadget_connect_control().

Introduce usb_otg_add_gadget_udc() to allow controller drivers
to register a gadget controller that is part of an OTG instance.

Register with OTG core when gadget function driver
is available and unregister when function driver is unbound.

We need to unlock the usb_lock mutex before calling
usb_otg_register_gadget() in udc_bind_to_driver() and
usb_gadget_remove_driver() else it will cause a circular
locking dependency.

Ignore softconnect sysfs control when we're in OTG
mode as OTG FSM takes care of gadget softconnect using
the b_bus_req mechanism.

Signed-off-by: Roger Quadros 
---
 drivers/usb/gadget/udc/udc-core.c | 194 --
 include/linux/usb/gadget.h|   4 +
 2 files changed, 189 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index 4151597..21c85ef 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+
+#include 
+#include 
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -325,6 +330,119 @@ static inline void usb_gadget_udc_stop(struct usb_udc 
*udc)
 }
 
 /**
+ * usb_gadget_to_udc - get the UDC owning the gadget
+ *
+ * udc_lock must be held.
+ * Returs NULL if UDC is not found.
+ */
+static struct usb_udc *usb_gadget_to_udc(struct usb_gadget *gadget)
+{
+   struct usb_udc *udc;
+
+   list_for_each_entry(udc, &udc_list, list)
+   if (udc->gadget == gadget)
+   return udc;
+
+   return NULL;
+}
+
+/**
+ * usb_gadget_start - start the usb gadget controller
+ * @gadget: the gadget device to start
+ *
+ * This is external API for use by OTG core.
+ *
+ * Start the usb device controller. Does not connect to the bus.
+ */
+static int usb_gadget_start(struct usb_gadget *gadget)
+{
+   int ret;
+   struct usb_udc *udc;
+
+   mutex_lock(&udc_lock);
+   udc = usb_gadget_to_udc(gadget);
+   if (!udc) {
+   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+   __func__);
+   mutex_unlock(&udc_lock);
+   return -EINVAL;
+   }
+
+   ret = usb_gadget_udc_start(udc);
+   if (ret)
+   dev_err(&udc->dev, "USB Device Controller didn't start: %d\n",
+   ret);
+
+   mutex_unlock(&udc_lock);
+
+   return ret;
+}
+
+/**
+ * usb_gadget_stop - stop the usb gadget controller
+ * @gadget: the gadget device we want to stop
+ *
+ * This is external API for use by OTG core.
+ *
+ * Stop the gadget controller. Does not disconnect from the bus.
+ * Caller must ensure that gadget has disconnected from the bus
+ * before calling usb_gadget_stop().
+ */
+static int usb_gadget_stop(struct usb_gadget *gadget)
+{
+   struct usb_udc *udc;
+
+   mutex_lock(&udc_lock);
+   udc = usb_gadget_to_udc(gadget);
+   if (!udc) {
+   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+   __func__);
+   mutex_unlock(&udc_lock);
+   return -EINVAL;
+   }
+
+   if (gadget->connected) {
+   dev_err(gadget->dev.parent,
+   "%s: called while still connected\n", __func__);
+   mutex_unlock(&udc_lock);
+   return -EINVAL;
+   }
+
+   usb_gadget_udc_stop(udc);
+   mutex_unlock(&udc_lock);
+
+   return 0;
+}
+
+static int usb_gadget_connect_control(struct usb_gadget *gadget, bool connect)
+{
+   struct usb_udc *udc;
+
+   mutex_lock(&udc_lock);
+   udc = usb_gadget_to_udc(gadget);
+   if (!udc) {
+   dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
+   __func__);
+   mutex_unlock(&udc_lock);
+   return -EINVAL;
+   }
+
+   if (connect) {
+   if (!gadget->connected)
+   usb_gadget_connect(udc->gadget);
+   } else {
+   if (gadget->connected) {
+   usb_gadget_disconnect(udc->gadget);
+   udc->driver->disconnect(udc->gadget);
+   }
+   }
+
+   mutex_unlock(&udc_lock);
+
+   return 0;
+}
+
+/**
  * usb_udc_release - release the usb_udc struct
  * @dev: the dev member within usb_udc
  *
@@ -486,6 +604,33 @@ int usb_add_gadget_udc(struct device *parent, struct 
usb_gadget *gadget)
 }
 EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
 
+/**
+ * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
+ * @parent: the parent device to this udc. Usually the controller
+ * driver's device.
+ * @gadget: the gadget to be added to the list
+ * @otg_dev: the OTG controller device
+ *
+ * If otg_dev is N

[PATCH] usb: dwc3: add DWC3_GUCTL1 reg for debug

2016-05-13 Thread William Wu
GUCTL1 reg has some useful functions which can be
written by user. For rockchip platform, we set
GUCTL1.DEV_FORCE_20_CLK_FOR_30_CLK (bit26, applicable
for the core is programmed to operate in 2.0 device
only) to 1 in bootrom, and after start the kernel,
we want to check whether this bit can be reset to
default 0 after the core reset. Dump GUCTL1 reg from
debugfs is more convenient for us.

Signed-off-by: William Wu 
---
Changes in v2:
- add commit log

 drivers/usb/dwc3/core.h| 1 +
 drivers/usb/dwc3/debugfs.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index abed84f..c4758d5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -86,6 +86,7 @@
 #define DWC3_GCTL  0xc110
 #define DWC3_GEVTEN0xc114
 #define DWC3_GSTS  0xc118
+#define DWC3_GUCTL10xc11c
 #define DWC3_GSNPSID   0xc120
 #define DWC3_GGPIO 0xc124
 #define DWC3_GUID  0xc128
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index b1dd3c6..f3c9f44 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -47,6 +47,7 @@ static const struct debugfs_reg32 dwc3_regs[] = {
dump_register(GCTL),
dump_register(GEVTEN),
dump_register(GSTS),
+   dump_register(GUCTL1),
dump_register(GSNPSID),
dump_register(GGPIO),
dump_register(GUID),
-- 
1.9.1


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


[PATCH v8 07/14] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG

2016-05-13 Thread Roger Quadros
Let's use CONFIG_USB_OTG as a single config option to enable
USB OTG and the OTG FSM. This makes things a lot less confusing.

Update all users of CONFIG_USB_OTG_FSM to CONFIG_USB_OTG.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 Documentation/usb/chipidea.txt | 2 +-
 drivers/usb/chipidea/Makefile  | 2 +-
 drivers/usb/chipidea/ci.h  | 2 +-
 drivers/usb/chipidea/otg_fsm.h | 2 +-
 drivers/usb/common/Makefile| 3 ++-
 drivers/usb/core/Kconfig   | 8 
 drivers/usb/phy/Kconfig| 2 +-
 7 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index 678741b..3b1f263 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -5,7 +5,7 @@ with 2 Freescale i.MX6Q sabre SD boards.
 
 1.1 How to enable OTG FSM in menuconfig
 ---
-Select CONFIG_USB_OTG_FSM, rebuild kernel Image and modules.
+Select CONFIG_USB_OTG, rebuild kernel Image and modules.
 If you want to check some internal variables for otg fsm,
 mount debugfs, there are 2 files which can show otg fsm
 variables and some controller registers value:
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index 518e445..45aa24d 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -3,7 +3,7 @@ obj-$(CONFIG_USB_CHIPIDEA)  += ci_hdrc.o
 ci_hdrc-y  := core.o otg.o debug.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC) += udc.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)+= host.o
-ci_hdrc-$(CONFIG_USB_OTG_FSM)  += otg_fsm.o
+ci_hdrc-$(CONFIG_USB_OTG)  += otg_fsm.o
 
 # Glue/Bridge layers go here
 
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index c523975..1a32b8c 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -406,7 +406,7 @@ static inline u32 hw_test_and_write(struct ci_hdrc *ci, 
enum ci_hw_regs reg,
  */
 static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
 {
-#ifdef CONFIG_USB_OTG_FSM
+#ifdef CONFIG_USB_OTG
struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
 
return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
index 6366fe3..2d451bb 100644
--- a/drivers/usb/chipidea/otg_fsm.h
+++ b/drivers/usb/chipidea/otg_fsm.h
@@ -64,7 +64,7 @@
 
 #define TB_AIDL_BDIS (20)  /* 4ms ~ 150ms, section 5.2.1 */
 
-#if IS_ENABLED(CONFIG_USB_OTG_FSM)
+#if IS_ENABLED(CONFIG_USB_OTG)
 
 int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
 int ci_otg_fsm_work(struct ci_hdrc *ci);
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 6bbb3ec..f8f2c88 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)  += usb-common.o
 usb-common-y += common.o
 usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
-obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
 obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
+usbotg-y   := usb-otg-fsm.o
+obj-$(CONFIG_USB_OTG)  += usbotg.o
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index dd28010..ae228d0 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -75,14 +75,6 @@ config USB_OTG_BLACKLIST_HUB
  and software costs by not supporting external hubs.  So
  are "Embedded Hosts" that don't offer OTG support.
 
-config USB_OTG_FSM
-   tristate "USB 2.0 OTG FSM implementation"
-   depends on USB && USB_OTG
-   select USB_PHY
-   help
- Implements OTG Finite State Machine as specified in On-The-Go
- and Embedded Host Supplement to the USB Revision 2.0 Specification.
-
 config USB_ULPI_BUS
tristate "USB ULPI PHY interface support"
depends on USB_SUPPORT
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index c690474..06794e2 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -20,7 +20,7 @@ config AB8500_USB
 
 config FSL_USB2_OTG
bool "Freescale USB OTG Transceiver Driver"
-   depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM
+   depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG && PM
select USB_PHY
help
  Enable this to support Freescale USB OTG transceiver.
-- 
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 v8 06/14] usb: gadget.h: Add OTG to gadget interface

2016-05-13 Thread Roger Quadros
The OTG core will use struct otg_gadget_ops to
start/stop the gadget controller.

The main purpose of this interface is to avoid directly
calling usb_gadget_start/stop() from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB_GADGET is m.

Signed-off-by: Roger Quadros 
---
 include/linux/usb/gadget.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 5d4e151..20a2f8a 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1100,6 +1100,22 @@ struct usb_gadget_driver {
 };
 
 
+/*-*/
+
+/**
+ * struct otg_gadget_ops - Interface between OTG core and gadget
+ *
+ * Provided by the gadget core to allow the OTG core to start/stop the gadget
+ *
+ * @start: function to start the gadget
+ * @stop: function to stop the gadget
+ * @connect_control: function to connect/disconnect from the bus
+ */
+struct otg_gadget_ops {
+   int (*start)(struct usb_gadget *gadget);
+   int (*stop)(struct usb_gadget *gadget);
+   int (*connect_control)(struct usb_gadget *gadget, bool connect);
+};
 
 /*-*/
 
-- 
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 v8 05/14] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops

2016-05-13 Thread Roger Quadros
This is to prevent missing symbol build error if OTG is
enabled (built-in) and HCD core (CONFIG_USB) is module.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 drivers/usb/chipidea/otg_fsm.c   |  7 +++
 drivers/usb/common/usb-otg-fsm.c | 15 +++
 drivers/usb/phy/phy-fsl-usb.c|  7 +++
 include/linux/usb/otg.h  |  2 ++
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 1c0c750..2d8d659 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -582,6 +582,12 @@ static struct otg_fsm_ops ci_otg_ops = {
.start_gadget = ci_otg_start_gadget,
 };
 
+static struct otg_hcd_ops ci_hcd_ops = {
+   .usb_bus_start_enum = usb_bus_start_enum,
+   .usb_control_msg = usb_control_msg,
+   .usb_hub_find_child = usb_hub_find_child,
+};
+
 int ci_otg_fsm_work(struct ci_hdrc *ci)
 {
/*
@@ -804,6 +810,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
ci->otg.fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
ci->otg.state = OTG_STATE_UNDEFINED;
ci->otg.fsm.ops = &ci_otg_ops;
+   ci->otg.hcd_ops = &ci_hcd_ops;
ci->gadget.hnp_polling_support = 1;
ci->otg.fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
if (!ci->otg.fsm.host_req_flag)
diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 4bfc6a5..2986b66 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -141,11 +141,16 @@ static void otg_hnp_polling_work(struct work_struct *work)
enum usb_otg_state state = otg->state;
u8 flag;
int retval;
+   struct otg_hcd_ops *hcd_ops = otg->hcd_ops;
 
if (state != OTG_STATE_A_HOST && state != OTG_STATE_B_HOST)
return;
 
-   udev = usb_hub_find_child(otg->host->root_hub, 1);
+   if (!hcd_ops || !hcd_ops->usb_control_msg ||
+   !hcd_ops->usb_hub_find_child)
+   return;
+
+   udev = hcd_ops->usb_hub_find_child(otg->host->root_hub, 1);
if (!udev) {
dev_err(otg->host->controller,
"no usb dev connected, can't start HNP polling\n");
@@ -154,7 +159,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
 
*fsm->host_req_flag = 0;
/* Get host request flag from connected USB device */
-   retval = usb_control_msg(udev,
+   retval = hcd_ops->usb_control_msg(udev,
usb_rcvctrlpipe(udev, 0),
USB_REQ_GET_STATUS,
USB_DIR_IN | USB_RECIP_DEVICE,
@@ -183,7 +188,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
if (state == OTG_STATE_A_HOST) {
/* Set b_hnp_enable */
if (!otg->host->b_hnp_enable) {
-   retval = usb_control_msg(udev,
+   retval = hcd_ops->usb_control_msg(udev,
usb_sndctrlpipe(udev, 0),
USB_REQ_SET_FEATURE, 0,
USB_DEVICE_B_HNP_ENABLE,
@@ -262,7 +267,9 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
usb_otg_state new_state)
otg_loc_conn(otg, 0);
otg_loc_sof(otg, 1);
otg_set_protocol(fsm, PROTO_HOST);
-   usb_bus_start_enum(otg->host, otg->host->otg_port);
+   if (otg->hcd_ops && otg->hcd_ops->usb_bus_start_enum)
+   otg->hcd_ops->usb_bus_start_enum(otg->host,
+otg->host->otg_port);
otg_start_hnp_polling(fsm);
break;
case OTG_STATE_A_IDLE:
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 587a187..9dbd9f0 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -792,6 +792,12 @@ static struct otg_fsm_ops fsl_otg_ops = {
.start_gadget = fsl_otg_start_gadget,
 };
 
+static struct otg_hcd_ops fsl_hcd_ops = {
+   .usb_bus_start_enum = usb_bus_start_enum,
+   .usb_control_msg = usb_control_msg,
+   .usb_hub_find_child = usb_hub_find_child,
+};
+
 /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
 static int fsl_otg_conf(struct platform_device *pdev)
 {
@@ -820,6 +826,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
 
/* Set OTG state machine operations */
fsl_otg_tc->otg.fsm.ops = &fsl_otg_ops;
+   fsl_otg_tc->otg.hcd_ops = &fsl_hcd_ops;
 
/* initialize the otg structure */
fsl_otg_tc->phy.label = DRIVER_DESC;
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index e8a14dc..85b8fb5 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct usb_otg {
u8  

[PATCH v8 04/14] usb: otg-fsm: use usb_otg wherever possible

2016-05-13 Thread Roger Quadros
Move otg_fsm into usb_otg and use usb_otg wherever possible
in the usb_otg APIs.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 drivers/usb/chipidea/ci.h|   1 -
 drivers/usb/chipidea/core.c  |  14 +--
 drivers/usb/chipidea/debug.c |   2 +-
 drivers/usb/chipidea/otg_fsm.c   | 169 ++--
 drivers/usb/chipidea/udc.c   |  17 ++--
 drivers/usb/common/usb-otg-fsm.c | 180 ---
 drivers/usb/phy/phy-fsl-usb.c| 141 +++---
 drivers/usb/phy/phy-fsl-usb.h|   3 +-
 include/linux/usb/otg-fsm.h  | 132 +++-
 include/linux/usb/otg.h  | 107 +++
 10 files changed, 383 insertions(+), 383 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index cd41455..c523975 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -209,7 +209,6 @@ struct ci_hdrc {
enum ci_rolerole;
boolis_otg;
struct usb_otg  otg;
-   struct otg_fsm  fsm;
struct hrtimer  otg_fsm_hrtimer;
ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS];
unsignedenabled_otg_timer_bits;
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..56b6ac6 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1085,8 +1085,8 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 /* Prepare wakeup by SRP before suspend */
 static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc *ci)
 {
-   if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
-   !hw_read_otgsc(ci, OTGSC_ID)) {
+   if ((ci->otg.state == OTG_STATE_A_IDLE) &&
+   !hw_read_otgsc(ci, OTGSC_ID)) {
hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_PP,
PORTSC_PP);
hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_WKCN,
@@ -1097,13 +1097,13 @@ static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc 
*ci)
 /* Handle SRP when wakeup by data pulse */
 static void ci_otg_fsm_wakeup_by_srp(struct ci_hdrc *ci)
 {
-   if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
-   (ci->fsm.a_bus_drop == 1) && (ci->fsm.a_bus_req == 0)) {
+   if ((ci->otg.state == OTG_STATE_A_IDLE) &&
+   (ci->otg.fsm.a_bus_drop == 1) && (ci->otg.fsm.a_bus_req == 0)) {
if (!hw_read_otgsc(ci, OTGSC_ID)) {
-   ci->fsm.a_srp_det = 1;
-   ci->fsm.a_bus_drop = 0;
+   ci->otg.fsm.a_srp_det = 1;
+   ci->otg.fsm.a_bus_drop = 0;
} else {
-   ci->fsm.id = 1;
+   ci->otg.fsm.id = 1;
}
ci_otg_queue_work(ci);
}
diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index 6d23eed..374cdaa 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -224,7 +224,7 @@ static int ci_otg_show(struct seq_file *s, void *unused)
if (!ci || !ci_otg_is_fsm_mode(ci))
return 0;
 
-   fsm = &ci->fsm;
+   fsm = &ci->otg.fsm;
 
/* -- State - */
seq_printf(s, "OTG state: %s\n\n",
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index de8e22e..1c0c750 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -40,7 +40,7 @@ get_a_bus_req(struct device *dev, struct device_attribute 
*attr, char *buf)
 
next = buf;
size = PAGE_SIZE;
-   t = scnprintf(next, size, "%d\n", ci->fsm.a_bus_req);
+   t = scnprintf(next, size, "%d\n", ci->otg.fsm.a_bus_req);
size -= t;
next += t;
 
@@ -56,25 +56,25 @@ set_a_bus_req(struct device *dev, struct device_attribute 
*attr,
if (count > 2)
return -1;
 
-   mutex_lock(&ci->fsm.lock);
+   mutex_lock(&ci->otg.fsm.lock);
if (buf[0] == '0') {
-   ci->fsm.a_bus_req = 0;
+   ci->otg.fsm.a_bus_req = 0;
} else if (buf[0] == '1') {
/* If a_bus_drop is TRUE, a_bus_req can't be set */
-   if (ci->fsm.a_bus_drop) {
-   mutex_unlock(&ci->fsm.lock);
+   if (ci->otg.fsm.a_bus_drop) {
+   mutex_unlock(&ci->otg.fsm.lock);
return count;
}
-   ci->fsm.a_bus_req = 1;
-   if (ci->fsm.otg->state == OTG_STATE_A_PERIPHERAL) {
+   ci->otg.fsm.a_bus_req = 1;
+   if (ci->otg.state == OTG_STATE_A_PERIPHERAL) {
ci->gadget.host_request_flag = 1;
-   mutex_unlock(&ci->fsm.lock);
+   mutex_unlock(&ci->otg

[PATCH v8 08/14] usb: otg: add OTG/dual-role core

2016-05-13 Thread Roger Quadros
It provides APIs for the following tasks

- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine

Provide a dual-role device (DRD) state machine.
DRD mode is a reduced functionality OTG mode. In this mode
we don't support SRP, HNP and dynamic role-swap.

In DRD operation, the controller mode (Host or Peripheral)
is decided based on the ID pin status. Once a cable plug (Type-A
or Type-B) is attached the controller selects the state
and doesn't change till the cable in unplugged and a different
cable type is inserted.

As we don't need most of the complex OTG states and OTG timers
we implement a lean DRD state machine in usb-otg.c.
The DRD state machine is only interested in 2 hardware inputs
'id' and 'b_sess_vld'.

Signed-off-by: Roger Quadros 
---
 drivers/usb/common/Makefile  |2 +-
 drivers/usb/common/usb-otg.c | 1042 ++
 drivers/usb/core/Kconfig |4 +-
 include/linux/usb/gadget.h   |2 +
 include/linux/usb/hcd.h  |1 +
 include/linux/usb/otg-fsm.h  |7 +
 include/linux/usb/otg.h  |  154 ++-
 7 files changed, 1206 insertions(+), 6 deletions(-)
 create mode 100644 drivers/usb/common/usb-otg.c

diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index f8f2c88..730d928 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -7,5 +7,5 @@ usb-common-y  += common.o
 usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
 obj-$(CONFIG_USB_ULPI_BUS) += ulpi.o
-usbotg-y   := usb-otg-fsm.o
+usbotg-y   := usb-otg.o usb-otg-fsm.o
 obj-$(CONFIG_USB_OTG)  += usbotg.o
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
new file mode 100644
index 000..64be6df
--- /dev/null
+++ b/drivers/usb/common/usb-otg.c
@@ -0,0 +1,1042 @@
+/**
+ * drivers/usb/common/usb-otg.c - USB OTG core
+ *
+ * Copyright (C) 2016 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Roger Quadros 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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 
+
+struct otg_gcd {
+   struct usb_gadget *gadget;
+   struct otg_gadget_ops *ops;
+};
+
+/* OTG device list */
+LIST_HEAD(otg_list);
+static DEFINE_MUTEX(otg_list_mutex);
+
+/* Hosts and Gadgets waiting for OTG controller */
+struct otg_wait_data {
+   struct device *dev; /* OTG controller device */
+
+   struct otg_hcd primary_hcd;
+   struct otg_hcd shared_hcd;
+   struct otg_gcd gcd;
+   struct list_head list;
+};
+
+LIST_HEAD(wait_list);
+static DEFINE_MUTEX(wait_list_mutex);
+
+static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
+{
+   if (!hcd->primary_hcd)
+   return 1;
+   return hcd == hcd->primary_hcd;
+}
+
+/**
+ * Check if the OTG device is in our wait list and return
+ * otg_wait_data, else NULL.
+ *
+ * wait_list_mutex must be held.
+ */
+static struct otg_wait_data *usb_otg_get_wait(struct device *otg_dev)
+{
+   struct otg_wait_data *wait;
+
+   if (!otg_dev)
+   return NULL;
+
+   /* is there an entry for this otg_dev ?*/
+   list_for_each_entry(wait, &wait_list, list) {
+   if (wait->dev == otg_dev)
+   return wait;
+   }
+
+   return NULL;
+}
+
+/**
+ * Add the hcd to our wait list
+ */
+static int usb_otg_hcd_wait_add(struct device *otg_dev, struct usb_hcd *hcd,
+   unsigned int irqnum, unsigned long irqflags,
+   struct otg_hcd_ops *ops)
+{
+   struct otg_wait_data *wait;
+   int ret = -EINVAL;
+
+   mutex_lock(&wait_list_mutex);
+
+   wait = usb_otg_get_wait(otg_dev);
+   if (!wait) {
+   /* Not yet in wait list? allocate and add */
+   wait = kzalloc(sizeof(*wait), GFP_KERNEL);
+   if (!wait) {
+   ret = -ENOMEM;
+   goto fail;
+   }
+
+   wait->dev = otg_dev;
+   list_add_tail(&wait->list, &wait_list);
+   }
+
+   if (usb_otg_hcd_is_primary_hcd(hcd)) {
+   if (wait->primary_hcd.hcd)  /* already assigned? */
+   goto fail;
+
+   wait->primary_hcd.hcd = hcd;
+   wait->primary_hcd.irqnum = irqnum;
+   wait->primary_hcd.irqflags = irqflags;
+   wait->primary_hcd.ops = ops;
+   wait->primary_hcd.otg_dev = otg

[PATCH v8 10/14] usb: otg: add hcd companion support

2016-05-13 Thread Roger Quadros
From: Yoshihiro Shimoda 

Since some host controller (e.g. EHCI) needs a companion host controller
(e.g. OHCI), this patch adds such a configuration to use it in the OTG
core.

Signed-off-by: Yoshihiro Shimoda 
Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 Documentation/devicetree/bindings/usb/generic.txt |  3 +++
 drivers/usb/common/usb-otg.c  | 32 ---
 include/linux/usb/otg.h   |  7 -
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
b/Documentation/devicetree/bindings/usb/generic.txt
index f6866c1..1db1c33 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -27,6 +27,9 @@ Optional properties:
  - otg-controller: phandle to otg controller. Host or gadget controllers can
contain this property to link it to a particular OTG
controller.
+ - hcd-needs-companion: must be present if otg controller is dealing with
+   EHCI host controller that needs a companion OHCI host
+   controller.
 
 This is an attribute to a USB controller such as:
 
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
index 64be6df..8a2a0d4 100644
--- a/drivers/usb/common/usb-otg.c
+++ b/drivers/usb/common/usb-otg.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -584,6 +585,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
else
INIT_WORK(&otg->work, usb_drd_work);
 
+   if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
+   config->hcd_needs_companion)/* needs companion ? */
+   otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
+
otg->wq = create_freezable_workqueue("usb_otg");
if (!otg->wq) {
dev_err(dev, "otg: %s: can't create workqueue\n",
@@ -807,15 +812,18 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
int irqnum,
/* HCD will be started by OTG fsm when needed */
mutex_lock(&otg->fsm.lock);
if (otg->primary_hcd.hcd) {
-   /* probably a shared HCD ? */
-   if (usb_otg_hcd_is_primary_hcd(hcd)) {
+   /* probably a shared HCD or a companion OHCI HCD ? */
+   if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+   usb_otg_hcd_is_primary_hcd(hcd)) {
dev_err(otg_dev, "otg: primary host already 
registered\n");
goto err;
}
 
-   if (hcd->shared_hcd == otg->primary_hcd.hcd) {
+   if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
+   (hcd->shared_hcd == otg->primary_hcd.hcd)) {
if (otg->shared_hcd.hcd) {
-   dev_err(otg_dev, "otg: shared host already 
registered\n");
+   dev_err(otg_dev,
+   "otg: shared/companion host already 
registered\n");
goto err;
}
 
@@ -823,10 +831,12 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
int irqnum,
otg->shared_hcd.irqnum = irqnum;
otg->shared_hcd.irqflags = irqflags;
otg->shared_hcd.ops = ops;
-   dev_info(otg_dev, "otg: shared host %s registered\n",
+   dev_info(otg_dev,
+"otg: shared/companion host %s registered\n",
 dev_name(hcd->self.controller));
} else {
-   dev_err(otg_dev, "otg: invalid shared host %s\n",
+   dev_err(otg_dev,
+   "otg: invalid shared/companion host %s\n",
dev_name(hcd->self.controller));
goto err;
}
@@ -849,14 +859,17 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
int irqnum,
 * we're ready only if we have shared HCD
 * or we don't need shared HCD.
 */
-   if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
+   if (otg->shared_hcd.hcd ||
+   (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+!otg->primary_hcd.hcd->shared_hcd)) {
otg->host = hcd_to_bus(hcd);
/* FIXME: set bus->otg_port if this is true OTG port with HNP */
 
/* start FSM */
usb_otg_start_fsm(otg);
} else {
-   dev_dbg(otg_dev, "otg: can't start till shared host 
registers\n");
+   dev_dbg(otg_dev,
+   "otg: can't start till shared/companion host 
registers\n");
}
 
mutex_unlock(&otg->fsm.lock);
@@ -907,7 +920,8 @@ int usb_otg_unregister_

[PATCH v8 02/14] usb: otg-fsm: Prevent build warning "VDBG" redefined

2016-05-13 Thread Roger Quadros
If usb/otg-fsm.h and usb/composite.h are included together
then it results in the build warning [1].

Prevent that by defining VDBG locally.

Also get rid of MPC_LOC which doesn't seem to be used
by anyone.

[1] - warning fixed by this patch:

In file included from drivers/usb/dwc3/core.h:33,
   from drivers/usb/dwc3/ep0.c:33:
   include/linux/usb/otg-fsm.h:30:1: warning: "VDBG" redefined
   In file included from drivers/usb/dwc3/ep0.c:31:
   include/linux/usb/composite.h:615:1: warning: this is the location
   of the previous definition

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 drivers/usb/common/usb-otg-fsm.c |  7 +++
 drivers/usb/phy/phy-fsl-usb.c|  7 +++
 include/linux/usb/otg-fsm.h  | 15 ---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index 9059b7d..015cf41 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -30,6 +30,13 @@
 #include 
 #include 
 
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
+__func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
 /* Change USB protocol when there is a protocol change */
 static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
 {
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..dd8a1ad 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -44,6 +44,13 @@
 
 #include "phy-fsl-usb.h"
 
+#ifdef VERBOSE
+#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
+__func__, ## args)
+#else
+#define VDBG(stuff...) do {} while (0)
+#endif
+
 #define DRIVER_VERSION "Rev. 1.55"
 #define DRIVER_AUTHOR "Jerry Huang/Li Yang"
 #define DRIVER_DESC "Freescale USB OTG Transceiver Driver"
diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 7a03505..a0a8f87 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -21,21 +21,6 @@
 #include 
 #include 
 
-#undef VERBOSE
-
-#ifdef VERBOSE
-#define VDBG(fmt, args...) pr_debug("[%s]  " fmt , \
-__func__, ## args)
-#else
-#define VDBG(stuff...) do {} while (0)
-#endif
-
-#ifdef VERBOSE
-#define MPC_LOC printk("Current Location [%s]:[%d]\n", __FILE__, __LINE__)
-#else
-#define MPC_LOC do {} while (0)
-#endif
-
 #define PROTO_UNDEF(0)
 #define PROTO_HOST (1)
 #define PROTO_GADGET   (2)
-- 
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 v8 03/14] usb: hcd.h: Add OTG to HCD interface

2016-05-13 Thread Roger Quadros
The OTG core will use struct otg_hcd_ops to interface
with the HCD controller.

The main purpose of this interface is to avoid directly
calling HCD APIs from the OTG core as they
wouldn't be defined in the built-in symbol table if
CONFIG_USB is m.

Signed-off-by: Roger Quadros 
Acked-by: Peter Chen 
---
 include/linux/usb/hcd.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index b98f831..861ccaa 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -399,6 +399,30 @@ struct hc_driver {
 
 };
 
+/**
+ * struct otg_hcd_ops - Interface between OTG core and HCD
+ *
+ * Provided by the HCD core to allow the OTG core to interface with the HCD
+ *
+ * @add: function to add the HCD
+ * @remove: function to remove the HCD
+ * @usb_bus_start_enum: function to immediately start bus enumeration
+ * @usb_control_msg: function to build and send of a control urb
+ * @usb_hub_find_child: function to get pointer to the child device
+ */
+struct otg_hcd_ops {
+   int (*add)(struct usb_hcd *hcd,
+  unsigned int irqnum, unsigned long irqflags);
+   void (*remove)(struct usb_hcd *hcd);
+   int (*usb_bus_start_enum)(struct usb_bus *bus, unsigned int port_num);
+   int (*usb_control_msg)(struct usb_device *dev, unsigned int pipe,
+  __u8 request, __u8 requesttype, __u16 value,
+  __u16 index, void *data, __u16 size,
+  int timeout);
+   struct usb_device * (*usb_hub_find_child)(struct usb_device *hdev,
+ int port1);
+};
+
 static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
 {
return hcd->driver->flags & HCD_BH;
-- 
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 v8 01/14] usb: hcd: Initialize hcd->flags to 0

2016-05-13 Thread Roger Quadros
When using the OTG/drd library we can call hcd_add/remove
consecutively without calling usb_put_hcd/usb_create_hcd in between
so hcd->flags can be stale.

If the HC dies due to whatever reason then without this
patch we get the below error on next hcd_add.

[   91.494257] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
[   91.502068] hub 3-0:1.0: state 0 ports 1 chg  evt 
[   91.510240] xhci-hcd xhci-hcd.0.auto: xHCI Host Controller
[   91.516940] xhci-hcd xhci-hcd.0.auto: new USB bus registered, assigned bus 
number 4
[   91.529745] usb usb4: We don't know the algorithms for LPM for this host, 
disabling LPM.
[   91.540637] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003
[   91.757865] irq 254: nobody cared (try booting with the "irqpoll" option)
[   91.757880] CPU: 0 PID: 68 Comm: kworker/u2:2 Not tainted 
4.1.4-00828-g1f0ed8c-dirty #44
[   91.757885] Hardware name: Generic AM43 (Flattened Device Tree)
[   91.757914] Workqueue: usb_otg usb_otg_work
[   91.757921] Backtrace:
[   91.757954] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[   91.757972]  r6:c089d4a4 r5: r4: r3:ee44
[   91.757991] [] (show_stack) from [] 
(dump_stack+0x84/0xd0)
[   91.758008] [] (dump_stack) from [] 
(__report_bad_irq+0x28/0xc8)
[   91.758024]  r7: r6:00fe r5: r4:ee514c40
[   91.758037] [] (__report_bad_irq) from [] 
(note_interrupt+0x24c/0x2ac)
[   91.758052]  r6:00fe r5: r4:ee514c40 r3:
[   91.758065] [] (note_interrupt) from [] 
(handle_irq_event_percpu+0xb0/0x158)
[   91.758085]  r10:ee514c40 r9:c08ce49a r8:00fe r7: r6: 
r5:
[   91.758094]  r4: r3:
[   91.758105] [] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x44/0x64)
[   91.758126]  r10:0001 r9:ee441ab0 r8:ee441bb8 r7:c0858b4c r6:ed174280 
r5:ee514ca0
[   91.758132]  r4:ee514c40
[   91.758144] [] (handle_irq_event) from [] 
(handle_fasteoi_irq+0x100/0x1bc)
[   91.758159]  r6:c085dba0 r5:ee514ca0 r4:ee514c40 r3:
[   91.758171] [] (handle_fasteoi_irq) from [] 
(generic_handle_irq+0x28/0x38)
[   91.758186]  r7:c0853d40 r6:c0858b4c r5:00fe r4:00fe
[   91.758197] [] (generic_handle_irq) from [] 
(__handle_domain_irq+0x98/0x12c)
[   91.758207]  r4:c0853d40 r3:0100
[   91.758219] [] (__handle_domain_irq) from [] 
(gic_handle_irq+0x28/0x68)
[   91.758239]  r10:0001 r9:ee441bb8 r8:fa240100 r7:c0858d70 r6:ee441ab0 
r5:00b8
[   91.758245]  r4:fa24010c
[   91.758264] [] (gic_handle_irq) from [] 
(__irq_svc+0x40/0x74)
[   91.758271] Exception stack(0xee441ab0 to 0xee441af8)
[   91.758280] 1aa0:  c08d2980 
ee441ac0 
[   91.758292] 1ac0: 0008 0089 c0858b4c c0858080  ee441bb8 
0001 ee441b3c
[   91.758301] 1ae0: 0101 ee441af8 c02fc418 c0046a1c 2113 
[   91.758321]  r8: r7:ee441ae4 r6: r5:2113 r4:c0046a1c 
r3:c02fc418
[   91.758347] [] (__do_softirq) from [] 
(irq_exit+0xb8/0x104)
[   91.758367]  r10:0001 r9:ee441bb8 r8: r7:c0853d40 r6:c0858b4c 
r5:0089
[   91.758373]  r4:
[   91.758386] [] (irq_exit) from [] 
(__handle_domain_irq+0xa0/0x12c)
[   91.758395]  r4: r3:0100
[   91.758406] [] (__handle_domain_irq) from [] 
(gic_handle_irq+0x28/0x68)
[   91.758426]  r10:c08e3510 r9:2013 r8:fa240100 r7:c0858d70 r6:ee441bb8 
r5:0039
[   91.758433]  r4:fa24010c
[   91.758445] [] (gic_handle_irq) from [] 
(__irq_svc+0x40/0x74)
[   91.758450] Exception stack(0xee441bb8 to 0xee441c00)
[   91.758457] 1ba0:   
 0001
[   91.758468] 1bc0:  ee44 c08e2524 004d 0274  
 2013
[   91.758479] 1be0: c08e3510 ee441c4c ee441b60 ee441c00 c03acfec c0080d4c 
6013 
[   91.758499]  r8: r7:ee441bec r6: r5:6013 r4:c0080d4c 
r3:c03acfec
[   91.758524] [] (console_unlock) from [] 
(vprintk_emit+0x20c/0x500)
[   91.758544]  r10:ee441cc0 r9:c08d3550 r8:c08e3ea0 r7: r6:0001 
r5:003d
[   91.758551]  r4:c08d3550
[   91.758573] [] (vprintk_emit) from [] 
(dev_vprintk_emit+0x104/0x1ac)
[   91.758593]  r10:ee441d8c r9:000e r8:c07951e0 r7:0006 r6:ee441cc0 
r5:000d
[   91.758599]  r4:ee731068
[   91.758612] [] (dev_vprintk_emit) from [] 
(dev_printk_emit+0x28/0x30)
[   91.758632]  r10:0001 r9:ee5f8410 r8:ee731000 r7:ed429000 r6:0006 
r5:ee441dc0
[   91.758638]  r4:ee731068
[   91.758651] [] (dev_printk_emit) from [] 
(__dev_printk+0x50/0x70)
[   91.758660]  r3:bf2268cc r2:c07951e0
[   91.758673] [] (__dev_printk) from [] 
(_dev_info+0x3c/0x48)
[   91.758686]  r6: r5:ee731068 r4:ee731000
[   91.758790] [] (_dev_info) from [] 
(usb_new_device+0x11c/0x518 [usbcore])
[   91.758804]  r3:0003 r2:1d6b r1:bf225bc4
[   91.758881] [] (usb_new_device [usbcore]) from [] 
(usb_otg_add_hcd+0x514/0x7f8 [usbcore])
[   91.758903]  r10:0001 r9

Re: [PATCH] usb: dwc3: add DWC3_GUCTL1 reg for debug

2016-05-13 Thread William Wu

On 05/13/2016 06:05 PM, William Wu wrote:

GUCTL1 reg has some useful functions which can be
written by user. For rockchip platform, we set
GUCTL1.DEV_FORCE_20_CLK_FOR_30_CLK (bit26, applicable
for the core is programmed to operate in 2.0 device
only) to 1 in bootrom, and after start the kernel,
we want to check whether this bit can be reset to
default 0 after the core reset. Dump GUCTL1 reg from
debugfs is more convenient for us.

Signed-off-by: William Wu 
---
Changes in v2:
- add commit log

  drivers/usb/dwc3/core.h| 1 +
  drivers/usb/dwc3/debugfs.c | 1 +
  2 files changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index abed84f..c4758d5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -86,6 +86,7 @@
  #define DWC3_GCTL 0xc110
  #define DWC3_GEVTEN   0xc114
  #define DWC3_GSTS 0xc118
+#define DWC3_GUCTL10xc11c
  #define DWC3_GSNPSID  0xc120
  #define DWC3_GGPIO0xc124
  #define DWC3_GUID 0xc128
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index b1dd3c6..f3c9f44 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -47,6 +47,7 @@ static const struct debugfs_reg32 dwc3_regs[] = {
dump_register(GCTL),
dump_register(GEVTEN),
dump_register(GSTS),
+   dump_register(GUCTL1),
dump_register(GSNPSID),
dump_register(GGPIO),
dump_register(GUID),

I'm sorry for forgot to add v2 in commit. I'll resend it later.

--
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 v8 00/14] USB OTG/dual-role framework

2016-05-13 Thread Roger Quadros
Hi,

This series centralizes OTG/Dual-role functionality in the kernel.
As of now I've got Dual-role functionality working pretty reliably on
dra7-evm and am437x-gp-evm.
NOTE: my am437x-gp-evm broke so I couldn't test v8 on it.
But the changes since v7 are trivial and shouldn't impact am437x-gp-evm.

DWC3 controller and platform related patches will be sent separately.

Series is based on v4.6-rc1 and depends on first 2 patches of [1]
[1] - OTG fsm cleanup - https://lkml.org/lkml/2016/3/30/186

Why?:


Currently there is no central location where OTG/dual-role functionality is
implemented in the Linux USB stack and every USB controller driver is
doing their own thing for OTG/dual-role. We can benefit from code-reuse
and simplicity by adding the OTG/dual-role core driver.

Newer OTG cores support standard host interface (e.g. xHCI) so
host and gadget functionality are no longer closely knit like older
cores. There needs to be a way to co-ordinate the operation of the
host and gadget controllers in dual-role mode. i.e. to stop and start them
from a central location. This central location should be the
USB OTG/dual-role core.

Host and gadget controllers might be sharing resources and can't
be always running. One has to be stopped for the other to run.
This couldn't be done till now but can be done from the OTG core.

What?:
-

The OTG/dual-role core consists of a set of APIs that allow
registration of OTG controller device and OTG capable host and gadget
controllers.

- The OTG controller driver can provide the OTG capabilities and the
Finite State Machine work function via 'struct usb_otg_config'
at the time of registration i.e. usb_otg_register();

struct usb_otg *usb_otg_register(struct device *dev,
 struct usb_otg_config *config);
int usb_otg_unregister(struct device *dev);
/**
 * struct usb_otg_config - otg controller configuration
 * @caps: otg capabilities of the controller
 * @ops: otg fsm operations
 * @otg_work: optional custom otg state machine work function
 */
struct usb_otg_config {
struct usb_otg_caps *otg_caps;
struct otg_fsm_ops *fsm_ops;
void (*otg_work)(struct work_struct *work);
};

The dual-role state machine is built-into the OTG core so nothing
special needs to be provided if only dual-role functionality is desired.
The low level OTG controller driver ops are povided via
'struct otg_fsm_ops *fsm_ops' in the 'struct usb_otg_config'.

After registration, the OTG core waits for host, gadget controller
and the gadget function driver to be registered. Once all resources are
available it instantiates the Finite State Machine (FSM).
The host/gadget controllers are started/stopped according to the FSM.

- Host and gadget controllers that are a part of OTG/dual-role port must
use the OTG core provided APIs to add/remove the host/gadget.
i.e. hosts must use usb_otg_add_hcd() usb_otg_remove_hcd(),,
gadgets must use usb_otg_add_gadget_udc() usb_del_gadget_udc().
This ensures that the host and gadget controllers are not started till
the state machine is ready and the right bus conditions are met.
It also allows the host and gadget controllers to provide the OTG
controller device to link them together. For Device tree boots
the related OTG controller is automatically picked up via the
'otg-controller' property in the Host/Gadget controller nodes.

int usb_otg_add_hcd(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags,
struct device *otg_dev);
void usb_otg_remove_hcd(struct usb_hcd *hcd);

int usb_otg_add_gadget_udc(struct device *parent,
   struct usb_gadget *gadget,
   struct device *otg_dev);
usb_del_gadget_udc() must be used for removal.


- During the lifetime of the FSM, the OTG controller driver can provide
inputs event changes using usb_otg_sync_inputs(). The OTG core will
then schedule the FSM work function (or internal dual-role state machine)
to update the FSM state. The FSM then calls the OTG controller
operations (fsm_ops) as necessary.
void usb_otg_sync_inputs(struct usb_otg *otg);

- The following 2 functions are provided as helpers for use by the
OTG controller driver to start/stop the host/gadget controllers.
int usb_otg_start_host(struct usb_otg *otg, int on);
int usb_otg_start_gadget(struct usb_otg *otg, int on);

- The following function is provided for use by the USB host stack
to sync OTG related events to the OTG state machine.
e.g. change in host_bus->b_hnp_enable, gadget->b_hnp_enable
int usb_otg_kick_fsm(struct device *otg_device);

Changelog:
-
v8:
- split out start/stop gadget and connect/disconnect operations.
- make CONFIG_OTG dpend on CONFIG_USB_GADGET as well apart from CONFIG_USB
- use create_

[PATCH v2] usb: dwc3: add DWC3_GUCTL1 reg for debug

2016-05-13 Thread William Wu
GUCTL1 reg has some useful functions which can be
written by user. For rockchip platform, we set
GUCTL1.DEV_FORCE_20_CLK_FOR_30_CLK (bit26, applicable
for the core is programmed to operate in 2.0 device
only) to 1 in bootrom, and after start the kernel,
we want to check whether this bit can be reset to
default 0 after the core reset. Dump GUCTL1 reg from
debugfs is more convenient for us.

Signed-off-by: William Wu 
---
Changes in v2:
- add commit log

 drivers/usb/dwc3/core.h| 1 +
 drivers/usb/dwc3/debugfs.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index abed84f..c4758d5 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -86,6 +86,7 @@
 #define DWC3_GCTL  0xc110
 #define DWC3_GEVTEN0xc114
 #define DWC3_GSTS  0xc118
+#define DWC3_GUCTL10xc11c
 #define DWC3_GSNPSID   0xc120
 #define DWC3_GGPIO 0xc124
 #define DWC3_GUID  0xc128
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index b1dd3c6..f3c9f44 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -47,6 +47,7 @@ static const struct debugfs_reg32 dwc3_regs[] = {
dump_register(GCTL),
dump_register(GEVTEN),
dump_register(GSTS),
+   dump_register(GUCTL1),
dump_register(GSNPSID),
dump_register(GGPIO),
dump_register(GUID),
-- 
1.9.1


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


ftdi_sio: overrun errors

2016-05-13 Thread michael

Hi,

if the internal buffer is full, a read() returns a steady stream of 
zeros until one valid character is received. According to my experiments 
this happens if the FT232 receives characters while the device is not 
opened. After the 257th byte the device returns overrun errors, which 
are translated to zero bytes (with TTY_OVERRUN flag set). The 257 bytes 
makes sense because the internal RX FIFO is 256 bytes and there is room 
for one more byte in the receive register, before the overrun occurs. 
Unfortunately, this happens until one (valid?) character is received.


The FT232RL seems to set the overrun flag and only clears it on the next 
received character. This flag is polled every $poll_interval 
microseconds and for each single poll there is one zero character placed 
in the tty buffer (and the overrun error is incremented).


How to reproduce:
 - Connect a FT232RL based adapter (ttyUSB0) to another serial port 
(ttyS0) using a null modem cable

 - make sure the serial settings are correct
 $ stty -F /dev/ttyUSB0 115200 raw -echo clocal
 $ stty -F /dev/ttyS0 115200 raw -echo clocal
 - open ttyUSB0 at least one time
 $ cat /dev/ttyUSB0  (cancel with ^C)
 - write 258 bytes to ttyS0
 $ dd if=/dev/zero of=/dev/ttyS0 bs=258 count=1
 - open and read from ttyUSB0
 $ od -A none -t x1 -w1 -v /dev/ttyUSB0
 - now you should see some zeros
 - write to ttyUSB0 again
 $ echo -n hello world > /dev/ttyS0
 - there should be more zeros before the "hello world" characters.

Alternatively you could read the overrun count of the icount struct 
(ioctl(TIOCGICOUNT)). You can find my example code at [1]:

  - setup the ttyS0 and ttyUSB0 like in the example above
  - write at least 258 bytes to ttyS0
  $ dd if=/dev/zero of=/dev/ttyS0 bs=258 count=1
  - read the overrun count of ttyUSB0
  $ tty_icount /dev/ttyUSB0
you should see the overrun counter going crazy until you send one 
more character.



Before commit cc01f17d5 (USB: ftdi_sio: re-implement read processing) 
there was a similar note in the driver:


/* if a parity error is detected you get status packets forever
   until a character is sent without a parity error.
   This doesn't work well since the application receives a
   never ending stream of bad data - even though new data
   hasn't been sent. Therefore I (bill) have taken this out.
   However - this might make sense for framing errors and so on
   so I am leaving the code in for now.
*/

So I guess this is still true for the parity error, but in this case 
there will be no character inserted into the tty buffer. Only the 
counter will be incremented indefinitely. I guess this is not the 
correct behaviour, either?


The first byte of the status packet is compared to a saved 
"prev_status", maybe we should do the same for the second byte, too?


Btw. this does not happen if, the adapter is plugged in for the first 
time. I see that the open causes issues a reset, I guess this doesn't 
reset this error condition.


-michael

[1] https://gist.github.com/mwalle/484dff9249d78664feee17fec970d3cd
--
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] dwc3: gadget: Defer starting the gadget device until gadget is power on

2016-05-13 Thread Baolin Wang
Currently on some platforms, the gadget device can be power off to save power
when the Vbus is off, which means no cable plugging in now. In this situation
we should defer starting the gadget until the gadget device is power on by
connecting host.

Signed-off-by: Baolin Wang 
---
 drivers/usb/dwc3/core.c  |5 +-
 drivers/usb/dwc3/core.h  |   14 
 drivers/usb/dwc3/gadget.c|  144 ++
 drivers/usb/dwc3/platform_data.h |1 +
 4 files changed, 131 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 34277ce..825462a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -109,7 +109,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
  * dwc3_soft_reset - Issue soft reset
  * @dwc: Pointer to our controller context structure
  */
-static int dwc3_soft_reset(struct dwc3 *dwc)
+int dwc3_soft_reset(struct dwc3 *dwc)
 {
unsigned long timeout;
u32 reg;
@@ -253,7 +253,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
unsigned length)
  *
  * Returns 0 on success otherwise negative errno.
  */
-static int dwc3_event_buffers_setup(struct dwc3 *dwc)
+int dwc3_event_buffers_setup(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
int n;
@@ -948,6 +948,7 @@ static int dwc3_probe(struct platform_device *pdev)
 
dwc->hsphy_interface = pdata->hsphy_interface;
fladj = pdata->fladj_value;
+   dwc->can_save_power = pdata->can_save_power;
}
 
dwc->lpm_nyet_threshold = lpm_nyet_threshold;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6254b2f..dada5c6 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -734,6 +734,9 @@ struct dwc3_scratchpad_array {
  * 1   - -3.5dB de-emphasis
  * 2   - No de-emphasis
  * 3   - Reserved
+ * @can_save_power: set if the gadget will power off when no cable plug in.
+ * @need_restart: set if we need to restart the gadget.
+ * @cable_connected: set if one usb cable is plugging in.
  */
 struct dwc3 {
struct usb_ctrlrequest  *ctrl_req;
@@ -876,6 +879,9 @@ struct dwc3 {
 
unsignedtx_de_emphasis_quirk:1;
unsignedtx_de_emphasis:2;
+   unsignedcan_save_power:1;
+   unsignedneed_restart:1;
+   unsignedcable_connected:1;
 };
 
 /* -- 
*/
@@ -1026,6 +1032,8 @@ struct dwc3_gadget_ep_cmd_params {
 /* prototypes */
 void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
 int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
+int dwc3_soft_reset(struct dwc3 *dwc);
+int dwc3_event_buffers_setup(struct dwc3 *dwc);
 
 /* check whether we are on the DWC_usb31 core */
 static inline bool dwc3_is_usb31(struct dwc3 *dwc)
@@ -1052,6 +1060,8 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum 
dwc3_link_state state);
 int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
unsigned cmd, struct dwc3_gadget_ep_cmd_params *params);
 int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned cmd, u32 
param);
+void dwc3_gadget_connect(struct dwc3 *dwc);
+void dwc3_gadget_disconnect(struct dwc3 *dwc);
 #else
 static inline int dwc3_gadget_init(struct dwc3 *dwc)
 { return 0; }
@@ -1071,6 +1081,10 @@ static inline int dwc3_send_gadget_ep_cmd(struct dwc3 
*dwc, unsigned ep,
 static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
int cmd, u32 param)
 { return 0; }
+static inline void dwc3_gadget_connect(struct dwc3 *dwc)
+{ }
+static inline void dwc3_gadget_disconnect(struct dwc3 *dwc)
+{ }
 #endif
 
 /* power management interface */
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8e4a1b1..90805f9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1512,13 +1512,75 @@ static int dwc3_gadget_set_selfpowered(struct 
usb_gadget *g,
return 0;
 }
 
+void dwc3_gadget_connect(struct dwc3 *dwc)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   dwc->cable_connected = true;
+   spin_unlock_irqrestore(&dwc->lock, flags);
+}
+
+void dwc3_gadget_disconnect(struct dwc3 *dwc)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   dwc->cable_connected = false;
+   spin_unlock_irqrestore(&dwc->lock, flags);
+}
+
+static bool dwc3_gadget_is_connected(struct dwc3 *dwc)
+{
+   /*
+* If the gadget is always power on, then no need to check if the
+* cable is plugin or not.
+*/
+   if (!dwc->can_save_power)
+   return true;
+
+   return dwc->cable_connected;
+}
+
+static int __dwc3_gadget_start(struct dwc3 *dwc);
+
 static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
 {
u32

RE: [PATCH] usb: gadget: f_fs: report error if excess data received

2016-05-13 Thread Du, Changbin
Hi,

> >> Yeah, it probably deserves a pr_err() or pr_debug(), but host sending
> >> more data than it should, is another problem altogether which needs to
> >> be addressed at the host.
> >>
> >> Adding a print to aid debugging is a good idea, but bailing out on the
> >> peripheral side is not :-s
> >>
> > Ok, if we think this is a problem at host side that the transfer is not 
> > device
> > expected, then device side should not accept the data or deliver the
> > transferred data to userspace. But now we take part of the data to
> userspace
> > and says it is ok.
> > Do you agree with this point?
> 
> We deliver to userspace the part userspace requested, right? So that's
> okay. The USB details WRT e.g. babble or host trying to send more data
> than expected, needs to be handled within the kernel.
> 
*babble* error is a good example. As you know, when xhci received more
data than expected, how does it process it? It doesn't deliver to software
the part userspace requested, but just give an error. And the xhci driver
set the urb status to -EOVERFLOW. This is similar to device side between
kernel and userspace.

> > IMO, we expose usb transfer as a file on device side. But file read() 
> > doesn't
> > have a requirement that "sorry, you cannot read so little! you need read all
> > once, else we may drop data for you. :) ".
> 
> but that's not how read() semantics work. When userspace asks to read(x)
> bytes, we have three possible outcomes:
> 
> i. We have x bytes to return, so we copy_to_user(x)
> 
> ii. We have y < x bytes to return, so we copy_to_user(y)
> 
> iii. We have y > x bytes to return, so we copy_to_user(x)
> 
I totally agree with these. They are all right. But what if userspace read
Twice? EG. When it want read a its packet, it may first read a head size,
Then read body.
read(20) = read(5) + read(15)
If this normal for a file, and works well, right? But if it happens on 
FunctionFS, the first read success, but the remailing 15 bytes lost because
they dropped by kernel. You may think it is host's bug, which also means
FunctionFS file does be different. You can compare usb with network,
Does network driver drop data? Afaik, You can read any bytes from a
socket, and data never lost.

For example, the host may send "aaabbb" and "cccddd", and device
side app May get "aaaccc" and all read success! Host send also success! 
But "bbb" "ddd" are lost.

The different views we have is on how to process the extra data. So we
can focus discussion here.

> This is exactly how the kernel is behaving. The only "detail" we have is
> that, for some reason, host is sending too much data. what I still don't
> know is if this extra data is garbage or something userspace genuinely
> cares about. Do you know the answer to this?
> 
IMO, FunctionFS should not care about what the data is. That is userspace' s
logic and meaningless for kernel. Kernel should not assume the data is garbage
or not. Is it right?

PS, for adb, it will re-open FunctionFS file after it detect unexpected data.

> > And some library that may retry read() until get enough data (which is
> > normal For a general read). Then sometimes the buffer size for
> > sys_read may not as expected. This is why I think ioctl approach is
> > more appropriate for usb transfer.
> 
> no, this won't change anything. Besides, it's a pointless discussion as
> cannot break userspace ABI. GadgetFS and FunctionFS have been shipping
> in kernel for many years.
> 
That because developers know the special requirement of FunctionFS,
just like adb.

> --
> Balbi

Do you mind if I modify my original patch for print error instead of return
error?

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


[PATCH] usb: gadget: composite: don't queue OS desc req if length is invalid

2016-05-13 Thread William Wu
In OS descriptors handling, if ctrl->bRequestType is
USB_RECIP_DEVICE and w_index != 0x4 or (w_value >> 8)
is true, it will not assign a valid value to req->length,
but use the default value(-EOPNOTSUPP), and queue an
OS desc request with the invalid req->length. It always
happens on the platforms which use os_desc (for example:
rk3366, rk3399), and cause kernel panic as follows
(use dwc3 driver):

Unable to handle kernel paging request at virtual address ffc0f7e0
Internal error: Oops: 96000146 [#1] PREEMPT SMP
PC is at __dma_clean_range+0x18/0x30
LR is at __swiotlb_map_page+0x50/0x64
Call trace:
[] __dma_clean_range+0x18/0x30
[] usb_gadget_map_request+0x134/0x1b0
[] __dwc3_ep0_do_control_data+0x110/0x14c
[] __dwc3_gadget_ep0_queue+0x198/0x1b8
[] dwc3_gadget_ep0_queue+0xc0/0xe8
[] composite_ep0_queue.constprop.14+0x34/0x98
[] composite_setup+0xf60/0x100c
[] android_setup+0xd8/0x138
[] dwc3_ep0_delegate_req+0x34/0x50
[] dwc3_ep0_interrupt+0x5dc/0xb58
[] dwc3_thread_interrupt+0x15c/0xa24

With this patch, the gadget driver will not queue
a request and return immediately if req->length is
invalid. And the usb controller driver can handle
the unsupport request correctly.

Signed-off-by: William Wu 
---
 drivers/usb/gadget/composite.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index d67de0d..eb64848 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1868,14 +1868,19 @@ unknown:
}
break;
}
-   req->length = value;
-   req->context = cdev;
-   req->zero = value < w_length;
-   value = composite_ep0_queue(cdev, req, GFP_ATOMIC);
-   if (value < 0) {
-   DBG(cdev, "ep_queue --> %d\n", value);
-   req->status = 0;
-   composite_setup_complete(gadget->ep0, req);
+
+   if (value >= 0) {
+   req->length = value;
+   req->context = cdev;
+   req->zero = value < w_length;
+   value = composite_ep0_queue(cdev, req,
+   GFP_ATOMIC);
+   if (value < 0) {
+   DBG(cdev, "ep_queue --> %d\n", value);
+   req->status = 0;
+   composite_setup_complete(gadget->ep0,
+req);
+   }
}
return value;
}
-- 
1.9.1


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


Re: [PATCH] dwc3: gadget: Defer starting the gadget device until gadget is power on

2016-05-13 Thread Felipe Balbi

Hi,

Baolin Wang  writes:
> Currently on some platforms, the gadget device can be power off to
> save power when the Vbus is off, which means no cable plugging in
> now. In this situation we should defer starting the gadget until the
> gadget device is power on by connecting host.

okay, you need to be a looot more specific about this. From a
basic look at the patch, there's no way I'll ever accept it, see below

> Signed-off-by: Baolin Wang 
> ---
>  drivers/usb/dwc3/core.c  |5 +-
>  drivers/usb/dwc3/core.h  |   14 
>  drivers/usb/dwc3/gadget.c|  144 
> ++
>  drivers/usb/dwc3/platform_data.h |1 +
>  4 files changed, 131 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 34277ce..825462a 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -109,7 +109,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>   * dwc3_soft_reset - Issue soft reset
>   * @dwc: Pointer to our controller context structure
>   */
> -static int dwc3_soft_reset(struct dwc3 *dwc)
> +int dwc3_soft_reset(struct dwc3 *dwc)

this *CANNOT* and *WILL* not be exposed to anything other than
core.c. We don't want anybody else resetting dwc3 willy-nilly.

> @@ -253,7 +253,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
> unsigned length)
>   *
>   * Returns 0 on success otherwise negative errno.
>   */
> -static int dwc3_event_buffers_setup(struct dwc3 *dwc)
> +int dwc3_event_buffers_setup(struct dwc3 *dwc)

likewise

> @@ -948,6 +948,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>   dwc->hsphy_interface = pdata->hsphy_interface;
>   fladj = pdata->fladj_value;
> + dwc->can_save_power = pdata->can_save_power;

sounds like a pointless flag IMHO.

>   }
>  
>   dwc->lpm_nyet_threshold = lpm_nyet_threshold;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 6254b2f..dada5c6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -734,6 +734,9 @@ struct dwc3_scratchpad_array {
>   *   1   - -3.5dB de-emphasis
>   *   2   - No de-emphasis
>   *   3   - Reserved
> + * @can_save_power: set if the gadget will power off when no cable plug in.

nope

> + * @need_restart: set if we need to restart the gadget.

why does it need restart? Why is dwc3 powered off? Who powers it off?

This looks like a *really* bad power management implementation. Do you
have hibernation enabled? Do you have Clock gating enabled? Which dwc3
version are you using? How was it configured?

> + * @cable_connected: set if one usb cable is plugging in.

not necessary, we already infer that from RUN/STOP and reset interrupt.

> @@ -876,6 +879,9 @@ struct dwc3 {
>  
>   unsignedtx_de_emphasis_quirk:1;
>   unsignedtx_de_emphasis:2;
> + unsignedcan_save_power:1;
> + unsignedneed_restart:1;
> + unsignedcable_connected:1;
>  };
>  
>  /* 
> -- */
> @@ -1026,6 +1032,8 @@ struct dwc3_gadget_ep_cmd_params {
>  /* prototypes */
>  void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
>  int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
> +int dwc3_soft_reset(struct dwc3 *dwc);
> +int dwc3_event_buffers_setup(struct dwc3 *dwc);

makes me cringe

> @@ -1052,6 +1060,8 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum 
> dwc3_link_state state);
>  int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
>   unsigned cmd, struct dwc3_gadget_ep_cmd_params *params);
>  int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned cmd, u32 
> param);
> +void dwc3_gadget_connect(struct dwc3 *dwc);
> +void dwc3_gadget_disconnect(struct dwc3 *dwc);

hell no

>  #else
>  static inline int dwc3_gadget_init(struct dwc3 *dwc)
>  { return 0; }
> @@ -1071,6 +1081,10 @@ static inline int dwc3_send_gadget_ep_cmd(struct dwc3 
> *dwc, unsigned ep,
>  static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
>   int cmd, u32 param)
>  { return 0; }
> +static inline void dwc3_gadget_connect(struct dwc3 *dwc)
> +{ }
> +static inline void dwc3_gadget_disconnect(struct dwc3 *dwc)
> +{ }
>  #endif
>  
>  /* power management interface */
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 8e4a1b1..90805f9 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1512,13 +1512,75 @@ static int dwc3_gadget_set_selfpowered(struct 
> usb_gadget *g,
>   return 0;
>  }
>  
> +void dwc3_gadget_connect(struct dwc3 *dwc)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);
> + dwc->cable_connected = true;
> + spin_unlock_irqrestore(&dwc->lock, flags);
> +}
> +
> +void dwc3_gadget_disconnect(struct dwc3 *dwc)
> +{
> + unsigned long flags;
> +
> +

Re: [PATCH v8 1/7] regulator: fixed: add support for ACPI interface

2016-05-13 Thread Mark Brown
On Thu, May 05, 2016 at 01:32:57PM +0800, Lu Baolu wrote:

> + gpiod = gpiod_get(dev, "gpio", GPIOD_ASIS);
> + if (IS_ERR(gpiod))
> + return ERR_PTR(-ENODEV);

> + config->gpio = desc_to_gpio(gpiod);
> + config->enable_high = device_property_read_bool(dev,
> + "enable-active-high");

> + gpiod_put(gpiod);

This isn't going to work at all if the GPIO is shared between multiple
regulators but I can't immediately see a sensible way to fix that
without some surgery on the GPIO APIs so let's leave it for now.


signature.asc
Description: PGP signature


Re: [PATCH] dwc3: gadget: Defer starting the gadget device until gadget is power on

2016-05-13 Thread Baolin Wang
Hi Felipe,

On 13 May 2016 at 18:40, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> Currently on some platforms, the gadget device can be power off to
>> save power when the Vbus is off, which means no cable plugging in
>> now. In this situation we should defer starting the gadget until the
>> gadget device is power on by connecting host.
>
> okay, you need to be a looot more specific about this. From a
> basic look at the patch, there's no way I'll ever accept it, see below
>
>> Signed-off-by: Baolin Wang 
>> ---
>>  drivers/usb/dwc3/core.c  |5 +-
>>  drivers/usb/dwc3/core.h  |   14 
>>  drivers/usb/dwc3/gadget.c|  144 
>> ++
>>  drivers/usb/dwc3/platform_data.h |1 +
>>  4 files changed, 131 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 34277ce..825462a 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -109,7 +109,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
>>   * dwc3_soft_reset - Issue soft reset
>>   * @dwc: Pointer to our controller context structure
>>   */
>> -static int dwc3_soft_reset(struct dwc3 *dwc)
>> +int dwc3_soft_reset(struct dwc3 *dwc)
>
> this *CANNOT* and *WILL* not be exposed to anything other than
> core.c. We don't want anybody else resetting dwc3 willy-nilly.
>
>> @@ -253,7 +253,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
>> unsigned length)
>>   *
>>   * Returns 0 on success otherwise negative errno.
>>   */
>> -static int dwc3_event_buffers_setup(struct dwc3 *dwc)
>> +int dwc3_event_buffers_setup(struct dwc3 *dwc)
>
> likewise
>
>> @@ -948,6 +948,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>
>>   dwc->hsphy_interface = pdata->hsphy_interface;
>>   fladj = pdata->fladj_value;
>> + dwc->can_save_power = pdata->can_save_power;
>
> sounds like a pointless flag IMHO.
>
>>   }
>>
>>   dwc->lpm_nyet_threshold = lpm_nyet_threshold;
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 6254b2f..dada5c6 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -734,6 +734,9 @@ struct dwc3_scratchpad_array {
>>   *   1   - -3.5dB de-emphasis
>>   *   2   - No de-emphasis
>>   *   3   - Reserved
>> + * @can_save_power: set if the gadget will power off when no cable plug in.
>
> nope
>
>> + * @need_restart: set if we need to restart the gadget.
>
> why does it need restart? Why is dwc3 powered off? Who powers it off?

Because when the dwc3 Vbus is off (no cable pluging in now),
especially for some mobile device, the system need to power off the
dwc3 to save power in this situation.

>
> This looks like a *really* bad power management implementation. Do you
> have hibernation enabled? Do you have Clock gating enabled? Which dwc3
> version are you using? How was it configured?

This is not hibernation, we want to power off the dwc3 to save power
when no cable plugging in. Yes, we have clock gating, at this
situation we will disable the clock and shutdown the phy to save
power. For mobile device, most time no cable plugging in, so we need
to think about the power consuming. How do you think this requirement?
Thanks.

>
>> + * @cable_connected: set if one usb cable is plugging in.
>
> not necessary, we already infer that from RUN/STOP and reset interrupt.
>
>> @@ -876,6 +879,9 @@ struct dwc3 {
>>
>>   unsignedtx_de_emphasis_quirk:1;
>>   unsignedtx_de_emphasis:2;
>> + unsignedcan_save_power:1;
>> + unsignedneed_restart:1;
>> + unsignedcable_connected:1;
>>  };
>>
>>  /* 
>> -- */
>> @@ -1026,6 +1032,8 @@ struct dwc3_gadget_ep_cmd_params {
>>  /* prototypes */
>>  void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
>>  int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>> +int dwc3_soft_reset(struct dwc3 *dwc);
>> +int dwc3_event_buffers_setup(struct dwc3 *dwc);
>
> makes me cringe
>
>> @@ -1052,6 +1060,8 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum 
>> dwc3_link_state state);
>>  int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
>>   unsigned cmd, struct dwc3_gadget_ep_cmd_params *params);
>>  int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned cmd, u32 
>> param);
>> +void dwc3_gadget_connect(struct dwc3 *dwc);
>> +void dwc3_gadget_disconnect(struct dwc3 *dwc);
>
> hell no
>
>>  #else
>>  static inline int dwc3_gadget_init(struct dwc3 *dwc)
>>  { return 0; }
>> @@ -1071,6 +1081,10 @@ static inline int dwc3_send_gadget_ep_cmd(struct dwc3 
>> *dwc, unsigned ep,
>>  static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
>>   int cmd, u32 param)
>>  { return 0; }
>> +static inline void dwc3_gadget_connect(struct dwc3 *dwc)
>> +{ }
>> +static inline voi

Re: [PATCH] dwc3: gadget: Defer starting the gadget device until gadget is power on

2016-05-13 Thread Felipe Balbi

Hi,

Baolin Wang  writes:
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 6254b2f..dada5c6 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -734,6 +734,9 @@ struct dwc3_scratchpad_array {
>>>   *   1   - -3.5dB de-emphasis
>>>   *   2   - No de-emphasis
>>>   *   3   - Reserved
>>> + * @can_save_power: set if the gadget will power off when no cable plug in.
>>
>> nope
>>
>>> + * @need_restart: set if we need to restart the gadget.
>>
>> why does it need restart? Why is dwc3 powered off? Who powers it off?
>
> Because when the dwc3 Vbus is off (no cable pluging in now),
> especially for some mobile device, the system need to power off the
> dwc3 to save power in this situation.

but dwc3 doesn't do this by itself, so who's doing it?

>> This looks like a *really* bad power management implementation. Do you
>> have hibernation enabled? Do you have Clock gating enabled? Which dwc3
>> version are you using? How was it configured?
>
> This is not hibernation, we want to power off the dwc3 to save power
> when no cable plugging in. Yes, we have clock gating, at this
> situation we will disable the clock and shutdown the phy to save
> power. For mobile device, most time no cable plugging in, so we need
> to think about the power consuming. How do you think this requirement?

Well, seems like you're missing *proper* runtime PM. I've been meaning
to work on it for weeks, but I still have a few other things to do
before I get to that. In any case, we don't need to do what you did
here. There are better ways.

>> Anyway, which platform are you dealing with? Why is dwc3 off while VBUS
>> is off? How do you handle host mode?
>
> On Spreadtrum platform, for thinking about some mobile devices with

I meant the SoC ;-) It's their own SoC? Are we getting glue-layer
patches any time soon?

> strict power management. This is just for gadget mode, we don't power
> off the dwc3 when it is host mode. Thanks.

okay, thanks

-- 
balbi


signature.asc
Description: PGP signature


[PATCH 1/1] usb: gadget: f_fs: Fix kernel panic if use_os_string not set

2016-05-13 Thread Jim Lin
If c->cdev->use_os_string flag is not set,
don't need to invoke ffs_do_os_descs() in _ffs_func_bind.
So uninitialized ext_compat_id pointer won't be accessed by
__ffs_func_bind_do_os_desc to cause kernel panic.

Signed-off-by: Jim Lin 
---
 drivers/usb/gadget/function/f_fs.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 15b648c..af3fdbd 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2851,7 +2851,7 @@ static int _ffs_func_bind(struct usb_configuration *c,
goto error;
 
func->function.os_desc_table = vla_ptr(vlabuf, d, os_desc_table);
-   if (c->cdev->use_os_string)
+   if (c->cdev->use_os_string) {
for (i = 0; i < ffs->interfaces_count; ++i) {
struct usb_os_desc *desc;
 
@@ -2862,13 +2862,15 @@ static int _ffs_func_bind(struct usb_configuration *c,
vla_ptr(vlabuf, d, ext_compat) + i * 16;
INIT_LIST_HEAD(&desc->ext_prop);
}
-   ret = ffs_do_os_descs(ffs->ms_os_descs_count,
- vla_ptr(vlabuf, d, raw_descs) +
- fs_len + hs_len + ss_len,
- d_raw_descs__sz - fs_len - hs_len - ss_len,
- __ffs_func_bind_do_os_desc, func);
-   if (unlikely(ret < 0))
-   goto error;
+   ret = ffs_do_os_descs(ffs->ms_os_descs_count,
+ vla_ptr(vlabuf, d, raw_descs) +
+ fs_len + hs_len + ss_len,
+ d_raw_descs__sz - fs_len - hs_len -
+ ss_len,
+ __ffs_func_bind_do_os_desc, func);
+   if (unlikely(ret < 0))
+   goto error;
+   }
func->function.os_desc_n =
c->cdev->use_os_string ? ffs->interfaces_count : 0;
 
-- 
1.9.1

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


Re: [PATCH] dwc3: gadget: Defer starting the gadget device until gadget is power on

2016-05-13 Thread Baolin Wang
On 13 May 2016 at 20:09, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
 index 6254b2f..dada5c6 100644
 --- a/drivers/usb/dwc3/core.h
 +++ b/drivers/usb/dwc3/core.h
 @@ -734,6 +734,9 @@ struct dwc3_scratchpad_array {
   *   1   - -3.5dB de-emphasis
   *   2   - No de-emphasis
   *   3   - Reserved
 + * @can_save_power: set if the gadget will power off when no cable plug 
 in.
>>>
>>> nope
>>>
 + * @need_restart: set if we need to restart the gadget.
>>>
>>> why does it need restart? Why is dwc3 powered off? Who powers it off?
>>
>> Because when the dwc3 Vbus is off (no cable pluging in now),
>> especially for some mobile device, the system need to power off the
>> dwc3 to save power in this situation.
>
> but dwc3 doesn't do this by itself, so who's doing it?

Yes, the dwc3 clock is controlled by the Soc system, so the Soc system
can disable the dwc3 clock when there is no cable plugging in.

>
>>> This looks like a *really* bad power management implementation. Do you
>>> have hibernation enabled? Do you have Clock gating enabled? Which dwc3
>>> version are you using? How was it configured?
>>
>> This is not hibernation, we want to power off the dwc3 to save power
>> when no cable plugging in. Yes, we have clock gating, at this
>> situation we will disable the clock and shutdown the phy to save
>> power. For mobile device, most time no cable plugging in, so we need
>> to think about the power consuming. How do you think this requirement?
>
> Well, seems like you're missing *proper* runtime PM. I've been meaning
> to work on it for weeks, but I still have a few other things to do
> before I get to that. In any case, we don't need to do what you did
> here. There are better ways.

Make sense.

>
>>> Anyway, which platform are you dealing with? Why is dwc3 off while VBUS
>>> is off? How do you handle host mode?
>>
>> On Spreadtrum platform, for thinking about some mobile devices with
>
> I meant the SoC ;-) It's their own SoC? Are we getting glue-layer
> patches any time soon?

Yes, it's their own SoC. Thanks.

>
>> strict power management. This is just for gadget mode, we don't power
>> off the dwc3 when it is host mode. Thanks.
>
> okay, thanks
>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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] dwc3: gadget: Defer starting the gadget device until gadget is power on

2016-05-13 Thread Felipe Balbi

Hi,

Baolin Wang  writes:
 why does it need restart? Why is dwc3 powered off? Who powers it off?
>>>
>>> Because when the dwc3 Vbus is off (no cable pluging in now),
>>> especially for some mobile device, the system need to power off the
>>> dwc3 to save power in this situation.
>>
>> but dwc3 doesn't do this by itself, so who's doing it?
>
> Yes, the dwc3 clock is controlled by the Soc system, so the Soc system
> can disable the dwc3 clock when there is no cable plugging in.

understood.

 This looks like a *really* bad power management implementation. Do you
 have hibernation enabled? Do you have Clock gating enabled? Which dwc3
 version are you using? How was it configured?
>>>
>>> This is not hibernation, we want to power off the dwc3 to save power
>>> when no cable plugging in. Yes, we have clock gating, at this
>>> situation we will disable the clock and shutdown the phy to save
>>> power. For mobile device, most time no cable plugging in, so we need
>>> to think about the power consuming. How do you think this requirement?
>>
>> Well, seems like you're missing *proper* runtime PM. I've been meaning
>> to work on it for weeks, but I still have a few other things to do
>> before I get to that. In any case, we don't need to do what you did
>> here. There are better ways.
>
> Make sense.

cool, if you wanna work on it, let me know and I can give some details
of what I have in mind.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v3] usb: dwc2: fix regression on big-endian PowerPC/ARM systems

2016-05-13 Thread Arnd Bergmann
On Thursday 12 May 2016 23:32:18 John Youn wrote:
> 
> Hi Arnd,
> 
> The capitalization issue is still there in this patch.
> 
> There's also a few checkpatch issues.

Fixed now, thanks. I'll send a v4 in a bit.

> And should the barrier be moved after the write like it says in the
> comment? That seems to have been removed since earlier versions of
> the patch.

I've clarified the comment, so we refer to the __raw_writel
not the writel. It's also possible that this was just another
bug in the patch that broke powerpc, but I don't know anything
about MIPS barrier semantics, so I prefer not to touch that.

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


[PATCH v4] usb: dwc2: fix regression on big-endian PowerPC/ARM systems

2016-05-13 Thread Arnd Bergmann
A patch that went into Linux-4.4 to fix big-endian mode on a Lantiq
MIPS system unfortunately broke big-endian operation on PowerPC
APM82181 as reported by Christian Lamparter, and likely other
systems.

It actually introduced multiple issues:

- it broke big-endian ARM kernels: any machine that was working
  correctly with a little-endian kernel is no longer using byteswaps
  on big-endian kernels, which clearly breaks them.
- On PowerPC the same thing must be true: if it was working before,
  using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC
  usually uses big-endian kernels, so they are likely all broken.
- The barrier for dwc2_writel is on the wrong side of the __raw_writel(),
  so the MMIO no longer synchronizes with DMA operations.
- On architectures that require specific CPU instructions for MMIO
  access, using the __raw_ variant may turn this into a pointer
  dereference that does not have the same effect as the readl/writel.

This patch is a simple revert for all architectures other than MIPS,
in the hope that we can more easily backport it to fix the regression
on PowerPC and ARM systems without breaking the Lantiq system again.

We should follow this up with a more elaborate change to add runtime
detection of endianness, to make sure it also works on all other
combinations of architectures and implementations of the usb-dwc2
device. That patch however will be fairly large and not appropriate
for backports to stable kernels.

Felipe suggested a different approach, using an endianness switching
register to always put the device into LE mode, but unfortunately
the dwc2 hardware does not provide a generic way to do that. Also,
I see no practical way of addressing the problem more generally by
patching architecture specific code on MIPS.

Signed-off-by: Arnd Bergmann 
Fixes: 95c8bc360944 ("usb: dwc2: Use platform endianness when accessing 
registers")
---
 drivers/usb/dwc2/core.h | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3c58d633ce80..dec0b21fc626 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -64,6 +64,17 @@
DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),\
dev_name(hsotg->dev), ##__VA_ARGS__)
 
+#ifdef CONFIG_MIPS
+/*
+ * There are some MIPS machines that can run in either big-endian
+ * or little-endian mode and that use the dwc2 register without
+ * a byteswap in both ways.
+ * Unlike other architectures, MIPS apparently does not require a
+ * barrier before the __raw_writel() to synchronize with DMA but does
+ * require the barrier after the __raw_writel() to serialize a set of
+ * writes. This set of operations was added specifically for MIPS and
+ * should only be used there.
+ */
 static inline u32 dwc2_readl(const void __iomem *addr)
 {
u32 value = __raw_readl(addr);
@@ -90,6 +101,22 @@ static inline void dwc2_writel(u32 value, void __iomem 
*addr)
pr_info("INFO:: wrote %08x to %p\n", value, addr);
 #endif
 }
+#else
+/* Normal architectures just use readl/write */
+static inline u32 dwc2_readl(const void __iomem *addr)
+{
+   return readl(addr);
+}
+
+static inline void dwc2_writel(u32 value, void __iomem *addr)
+{
+   writel(value, addr);
+
+#ifdef DWC2_LOG_WRITES
+   pr_info("info:: wrote %08x to %p\n", value, addr);
+#endif
+}
+#endif
 
 /* Maximum number of Endpoints/HostChannels */
 #define MAX_EPS_CHANNELS   16
-- 
2.7.0

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


Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

2016-05-13 Thread Heikki Krogerus
Hi,

On Wed, May 11, 2016 at 07:47:18AM -0700, Guenter Roeck wrote:
> Hi,
> 
> On Wed, May 11, 2016 at 12:40:11PM +0300, Heikki Krogerus wrote:
> > On Tue, May 10, 2016 at 08:14:34PM -0700, Guenter Roeck wrote:
> > > Heikki,
> > > 
> > > On 05/06/2016 01:08 AM, Heikki Krogerus wrote:
> > > > Hi,
> > > > 
> > > [ ... ]
> > > > 
> > > > I don't have not made any new code for the class driver yet, but I'm
> > > > attempting to prepare v2 next week.
> > > > 
> > > Would it make sense to send feedback about v1 now, or should I wait for 
> > > v2 ?
> > 
> > I don't think I'm able to send v2 today, or even tomorrow, so feel
> > free to give the feedback. Just be aware that I've rewritten the
> > alternate mode part completely.
> > 
> Alternate mode handling was my major concern, actually.
> 
> > I'm creating a separate device for the partner and also the cable
> > during connection. I'm also already going to introduce a small bus for
> > the AltModes. It's clear that we need to have AltMode specific
> > drivers. The generic parts can't take care of all the AltMode specific
> > requirements and VDMs. The bus will give us a nice way to bind those
> > drivers to the actual AltModes a partner and the cable plugs offer.
> > 
> > So if there are dependencies between the altmodes, for example if the
> > cable plugs needs to be in a certain mode in order for the partner to
> > be able to function in some specific mode, the responsibility of
> > taking care of those will fall primarily to in the AltMode drivers.
> > So not userspace.
> > 
> > The AltMode drivers actually are useful also as they can be part of
> > the relevant frameworks, for example DP in some graphics framework.
> > For example in case of DP, the number of lanes (I guess 2 or 4) should
> > be ideally known if I have understood correctly. Knowledge about the
> > connection seems to also be needed, and I've so far seen some pretty
> > weird solutions for hotplug events with the DP AltMode. With the
> > driver we should be able to avoid those.
> > 
> > But in any case, every SVIDs a partner (or plug) offers will have
> > their own device registered with the partner (or cable) itself as
> > parent in this design. I'm expecting a little bit of conversation
> > about this plan, but right now I feel confident about it.
> > 
> > How does this sound to you?
> > 
> Looking forward to it. My major problem so far was that alternate mode
> handling is very platform specific, which didn't seem to be well supported
> in v1 of your patch. I thought about implementing a hierarchy of drivers
> below the type-c class to solve that problem. Looks like you just solved
> it for me.
> 
> Other than that, my major concern is the lack of synchronization/protection
> between the type-c class and the drivers. Setting port parameters (data role,
> power role, operational power role, partner alternate modes, partner type)
> from registered drivers may need to be synchronzed/protected. For example,
> data and power role are set during connection establishment, but can be
> overwritten from the typec class code. Right now I am just setting the
> respective variables in struct typec_port directly, but that doesn't seem
> right.

I'm actually struggling with this same question. I decided to protect
the whole struct typec_port by not allowing the drivers to touch it,
but I'm still working on it..

> For partner_type, I don't really know how to map the options to the identity
> reported by the partner. The reported product types are unknown / hub /
> peripheral / passive cable / active cable / alternate mode adapter.
> The available partner types are unknown / USB / Charger / Alternate Mode /
> Audio Accessory / Debug Accessory. What am I missing here ?

The partner types don't map directly to the USB PD Product Types. We
need to describe the partner even when USB PD is not available.
Accessory Modes are for example out side the scope of USB PD.

But I'll try to propose something for those.

> The rest is just nitpicks.
> 
> - alternate_modes_show() and partner_alt_modes_show() discard the last byte
>   of the generated string and replace it with \0.
> - s/Accessroy/Accessory/
> - typec_connect() and typec_disconnect() should probably also set
>   port->connected.

OK. I'll check these.

So I failed to finish my proposal for v2 this week. On top of the
sync/protection problems, I'm also still trying to tune my AltMode
bus. I'm going to attempt to send it as an RFC on Monday or Tuesday in
any case.


Thanks,

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


Re: Downloading tracks from gps via usb

2016-05-13 Thread Alan Stern
On Thu, 12 May 2016, Stephen Furner wrote:

> Alan,
> 
> The permissions for the usb ports are:
> 
> stephen@stephen-N150P:/$ ls -l dev/bus/usb
> total 0
> drwxr-xr-x 2 root root 100 May 11 14:39 001
> drwxr-xr-x 2 root root  60 May 12 01:48 002
> drwxr-xr-x 2 root root  80 May 12 21:04 003
> drwxr-xr-x 2 root root  60 May 11 14:39 004
> drwxr-xr-x 2 root root  60 May 11 14:39 005

What about the permissions for the files inside those directories?

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: [PATCH] usb: gadget: f_fs: report error if excess data received

2016-05-13 Thread Alan Stern
On Fri, 13 May 2016, Felipe Balbi wrote:

> We deliver to userspace the part userspace requested, right? So that's
> okay. The USB details WRT e.g. babble or host trying to send more data
> than expected, needs to be handled within the kernel.

The point is that you don't know whether the host sent more data than
expected.  All you know is that the host sent more data than the user
asked the kernel for -- but maybe the user didn't ask for all the data
that he expected.  Maybe the user wanted to retrieve the full set of
data using two read() system calls.

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: Fwd: Downloading tracks from gps via usb

2016-05-13 Thread Alan Stern
On Thu, 12 May 2016, Stephen Furner wrote:

> I apologise if this is a duplicate I had a bounce back from the list
> due to it since I had inadvertently included HTML. So I am resending.
> 
> To: Alan Stern 
> Cc: USB list 
> 
> Alan,
> 
> I have looked through the strace output the following excerpt below
> directly includes references gebabbel. Is this more in line with what
> you were expecting it to reveal?

The relevant lines would be those referring to /dev/bus/usb and the 
lines following such references.

> I can of course send the full file to
> anyone interested in the complete output. It is too large to insert
> into an e-mail and there is a lot of repetition of my original
> excerpt.
> 
> Good point about ubuntu forums. This issue is also question #293560 on
> the forum https://answers.launchpad.net/ubuntu

The suggestion about adding an /etc/udev/rules.d file looks like a good 
one.

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: [RFT PATCH] xhci: Fix handling timeouted commands on hosts in weird states.

2016-05-13 Thread Joe Lawrence
On 05/12/2016 07:57 AM, Mathias Nyman wrote:
> If commands timeout we mark them for abortion, then stop the command
> ring, and turn the commands to no-ops and finally restart the command
> ring.
> 
> If the host is working properly the no-op commands will finish and
> pending completions are called.
> If we notice the host is failing driver clears the command ring and
> completes, deletes and frees all pending commands.
> 
> There are two separate cases reported where host is believed to work
> properly but is not. In the first case we successfully stop the ring
> but no abort or stop commnand ring event is ever sent and host locks up.

s/commnand/command/

> 
> The second case is if a host is removed, command times out and driver
> believes the ring is stopped, and assumes it be restarted, but actually
> ends up timing out on the same command forever.
> If one of the pending commands has the xhci->mutex held it will block
> xhci_stop() in the remove codepath which otherwise would cleanup pending
> commands.
> 
> Add a check that clears all pending commands in case host is removed,
> or we are stuck timeouting on the same command. Also restart the

s/timeouting/timing out/

> command timeout timer when stopping the command ring to ensure we
> recive an ring stop/abort event.

s/recive/receive/

> 
> Cc: stable 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci-ring.c | 27 ++-
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 52deae4..c82570d 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -290,6 +290,14 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
>  
>   temp_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
>   xhci->cmd_ring_state = CMD_RING_STATE_ABORTED;
> +
> + /*
> +  * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
> +  * however on some host hw the CMD_RING_RUNNING bit is correctly cleared
> +  * but the completion event in never sent. Use the cmd timeout timer to
> +  * handle those cases. Use twice the time to cover the bit polling retry
> +  */
> + mod_timer(&xhci->cmd_timer, jiffies + (2 * XHCI_CMD_DEFAULT_TIMEOUT));
>   xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
>   &xhci->op_regs->cmd_ring);
>  
> @@ -314,6 +322,7 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
>  
>   xhci_err(xhci, "Stopped the command ring failed, "
>   "maybe the host is dead\n");
> + del_timer(&xhci->cmd_timer);
>   xhci->xhc_state |= XHCI_STATE_DYING;
>   xhci_quiesce(xhci);
>   xhci_halt(xhci);
> @@ -1246,22 +1255,21 @@ void xhci_handle_command_timeout(unsigned long data)
>   int ret;
>   unsigned long flags;
>   u64 hw_ring_state;
> - struct xhci_command *cur_cmd = NULL;
> + bool second_timeout = false;
>   xhci = (struct xhci_hcd *) data;
>  
>   /* mark this command to be cancelled */
>   spin_lock_irqsave(&xhci->lock, flags);
>   if (xhci->current_cmd) {
> - cur_cmd = xhci->current_cmd;
> - cur_cmd->status = COMP_CMD_ABORT;
> + if (xhci->current_cmd->status == COMP_CMD_ABORT)
> + second_timeout = true;
> + xhci->current_cmd->status = COMP_CMD_ABORT;
>   }
>  
> -
>   /* Make sure command ring is running before aborting it */
>   hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
>   if ((xhci->cmd_ring_state & CMD_RING_STATE_RUNNING) &&
>   (hw_ring_state & CMD_RING_RUNNING))  {
> -
>   spin_unlock_irqrestore(&xhci->lock, flags);
>   xhci_dbg(xhci, "Command timeout\n");
>   ret = xhci_abort_cmd_ring(xhci);
> @@ -1273,6 +1281,15 @@ void xhci_handle_command_timeout(unsigned long data)
>   }
>   return;
>   }
> +
> + /* command ring failed to restart, or host removed. Bail out */
> + if (second_timeout || xhci->xhc_state & XHCI_STATE_REMOVING) {
> + spin_unlock_irqrestore(&xhci->lock, flags);
> + xhci_dbg(xhci, "command timed out twice, ring start fail?\n");
> + xhci_cleanup_command_queue(xhci);
> + return;
> + }
> +
>   /* command timeout on stopped ring, ring can't be aborted */
>   xhci_dbg(xhci, "Command timeout on stopped ring\n");
>   xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
> 

Hi Mathias,

At first glance, this patch looks good.  As I mentioned earlier, after
applying "xhci_mem_cleanup after second hcd" a lot of issues we'd been
seeing on Stratus HW cleared up.  That said, I'll add this patch to my
testing and report any problems.

Thanks!

-- Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
Mor

[PATCH] cdc-wdm: fix "out-of-sync" due to missing notifications

2016-05-13 Thread Bjørn Mork
The driver enforces a strict one-to-one relationship between the
received RESPONSE_AVAILABLE notifications and messages read from
the device. At the same time, it will cancel the interrupt URB
when there is no client holding the character device open.

Many devices do not cope well with this behaviour.  They maintain
a FIFO queue of messages, and send notifications on a best effort
basis.  Messages are queued regardless of whether the notification
is successful or not. So if the driver loses a single notification,
which can easily happen when the interrupt URB is cancelled, then
the device and driver becomes out-of-sync. New messages end up
at the end of the queue, while the associated notification makes
the driver read only the first message from the queue.

This state is permanent from a user point of view. There is no
no way to flush the device queue without resetting the device or
using another driver.

The problem is easy to hit with current QMI and MBIM command line
tools, which typically close the character device after seeing
the reply they expect. Any pending unsolicited messages from the
device will then trigger the driver bug.

Fix by always reading all queued messages from the device when
the notification URB is first submitted.  This is expected to
end with an -EPIPE status when there are no more pending
messages, so demote the printk associated with -EPIPE to debug
level.

Cc: 
Signed-off-by: Bjørn Mork 
---

Hello,

this fixes a long standing problem which has become increasingly
annoying with each new generation of MBIM and QMI modems. They
simply do not expect the strict notification scheme we try to
enforce.

For a while I've tried to convice users to force the /dev/cdc-wdmX
device open, using tricks like "cat >/dev/cdc-wdm0". But semantics
like that are of course not acceptable.  Opening and closing the
char device should never cause the permanent host/device communcation
failure it currently does. To make this worse, the only reset tool
available to users is typically unplugging and replugging. And
even that can be difficult if it is a laptop internal modem.

Please backport this to stable as well.  The problem hits common
OpenWrt tools like umbim really hard.


Bjørn


 drivers/usb/class/cdc-wdm.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 61ea87917433..f70972be2ee9 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -178,7 +178,7 @@ static void wdm_in_callback(struct urb *urb)
"nonzero urb status received: -ESHUTDOWN");
goto skip_error;
case -EPIPE:
-   dev_err(&desc->intf->dev,
+   dev_dbg(&desc->intf->dev,
"nonzero urb status received: -EPIPE\n");
break;
default:
@@ -200,6 +200,19 @@ static void wdm_in_callback(struct urb *urb)
desc->reslength = length;
}
}
+
+   /*
+* If desc->resp_count is unset, then the urb was submitted
+* without a prior notification.  If the device returned any
+* data, then this implies that it had messages queued without
+* notifying us.  Continue reading until that queue is flushed.
+*/
+   if (!desc->resp_count && length) {
+   dev_dbg(&desc->intf->dev, "got %d bytes without 
notification\n", length);
+   set_bit(WDM_RESPONDING, &desc->flags);
+   usb_submit_urb(desc->response, GFP_ATOMIC);
+   }
+
 skip_error:
wake_up(&desc->wait);
 
@@ -647,6 +660,16 @@ static int wdm_open(struct inode *inode, struct file *file)
dev_err(&desc->intf->dev,
"Error submitting int urb - %d\n", rv);
rv = usb_translate_errors(rv);
+   } else {
+   /*
+* Some devices keep pending messages queued
+* without resending notifications.  We must
+* flush the message queue before we can
+* assume a one-to-one relationship between
+* notifications and messages in the queue
+*/
+   set_bit(WDM_RESPONDING, &desc->flags);
+   rv = usb_submit_urb(desc->response, GFP_KERNEL);
}
} else {
rv = 0;
-- 
2.1.4

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


Re: [PATCH 3/3] USB: ohci-jz4740: Remove obsolete driver

2016-05-13 Thread Ralf Baechle
Maarten,

if you submit a USB change to the USB mailing list and maintainer the
probability for the maintainer to ack this patch will actuall rise
significantly ;-)

Greg, I assume this patch is ok to merge or do you want to funnel it
hrough your tree?  I think it would be good to take this through the
MIPS tree together with the remainder of the series.

  Ralf

On Mon, Apr 18, 2016 at 08:58:53PM +0200, Maarten ter Huurne wrote:

> The ohci-platform driver can control the clock, while usb-nop-xceiv
> as the PHY can control the vbus regulator. So this JZ4740-specific
> glue is not needed anymore.
> 
> Signed-off-by: Maarten ter Huurne 
> ---
>  drivers/usb/host/ohci-hcd.c|   5 -
>  drivers/usb/host/ohci-jz4740.c | 245 
> -
>  2 files changed, 250 deletions(-)
>  delete mode 100644 drivers/usb/host/ohci-jz4740.c
> 
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 04dcedf..0449235 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -1245,11 +1245,6 @@ MODULE_LICENSE ("GPL");
>  #define TMIO_OHCI_DRIVER ohci_hcd_tmio_driver
>  #endif
>  
> -#ifdef CONFIG_MACH_JZ4740
> -#include "ohci-jz4740.c"
> -#define PLATFORM_DRIVER  ohci_hcd_jz4740_driver
> -#endif
> -
>  #ifdef CONFIG_TILE_USB
>  #include "ohci-tilegx.c"
>  #define PLATFORM_DRIVER  ohci_hcd_tilegx_driver
> diff --git a/drivers/usb/host/ohci-jz4740.c b/drivers/usb/host/ohci-jz4740.c
> deleted file mode 100644
> index 4db78f1..000
> --- a/drivers/usb/host/ohci-jz4740.c
> +++ /dev/null
> @@ -1,245 +0,0 @@
> -/*
> - *  Copyright (C) 2010, Lars-Peter Clausen 
> - *
> - *  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, or (at your
> - *  option) any later version.
> - *
> - *  You should have received a copy of the  GNU General Public License along
> - *  with this program; if not, write  to the Free Software Foundation, Inc.,
> - *  675 Mass Ave, Cambridge, MA 02139, USA.
> - *
> - */
> -
> -#include 
> -#include 
> -#include 
> -
> -struct jz4740_ohci_hcd {
> - struct ohci_hcd ohci_hcd;
> -
> - struct regulator *vbus;
> - bool vbus_enabled;
> - struct clk *clk;
> -};
> -
> -static inline struct jz4740_ohci_hcd *hcd_to_jz4740_hcd(struct usb_hcd *hcd)
> -{
> - return (struct jz4740_ohci_hcd *)(hcd->hcd_priv);
> -}
> -
> -static inline struct usb_hcd *jz4740_hcd_to_hcd(struct jz4740_ohci_hcd 
> *jz4740_ohci)
> -{
> - return container_of((void *)jz4740_ohci, struct usb_hcd, hcd_priv);
> -}
> -
> -static int ohci_jz4740_start(struct usb_hcd *hcd)
> -{
> - struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> - int ret;
> -
> - ret = ohci_init(ohci);
> - if (ret < 0)
> - return ret;
> -
> - ohci->num_ports = 1;
> -
> - ret = ohci_run(ohci);
> - if (ret < 0) {
> - dev_err(hcd->self.controller, "Can not start %s",
> - hcd->self.bus_name);
> - ohci_stop(hcd);
> - return ret;
> - }
> - return 0;
> -}
> -
> -static int ohci_jz4740_set_vbus_power(struct jz4740_ohci_hcd *jz4740_ohci,
> - bool enabled)
> -{
> - int ret = 0;
> -
> - if (!jz4740_ohci->vbus)
> - return 0;
> -
> - if (enabled && !jz4740_ohci->vbus_enabled) {
> - ret = regulator_enable(jz4740_ohci->vbus);
> - if (ret)
> - dev_err(jz4740_hcd_to_hcd(jz4740_ohci)->self.controller,
> - "Could not power vbus\n");
> - } else if (!enabled && jz4740_ohci->vbus_enabled) {
> - ret = regulator_disable(jz4740_ohci->vbus);
> - }
> -
> - if (ret == 0)
> - jz4740_ohci->vbus_enabled = enabled;
> -
> - return ret;
> -}
> -
> -static int ohci_jz4740_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
> wValue,
> - u16 wIndex, char *buf, u16 wLength)
> -{
> - struct jz4740_ohci_hcd *jz4740_ohci = hcd_to_jz4740_hcd(hcd);
> - int ret = 0;
> -
> - switch (typeReq) {
> - case SetPortFeature:
> - if (wValue == USB_PORT_FEAT_POWER)
> - ret = ohci_jz4740_set_vbus_power(jz4740_ohci, true);
> - break;
> - case ClearPortFeature:
> - if (wValue == USB_PORT_FEAT_POWER)
> - ret = ohci_jz4740_set_vbus_power(jz4740_ohci, false);
> - break;
> - }
> -
> - if (ret)
> - return ret;
> -
> - return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
> -}
> -
> -
> -static const struct hc_driver ohci_jz4740_hc_driver = {
> - .description =  hcd_name,
> - .product_desc = "JZ4740 OHCI",
> - .hcd_priv_size =sizeof(struct jz4740_ohci_hcd),
> -
> - /*
> -  * generic hardware linkage
> -  */
> - .irq =  

Re: [PATCH 3/3] USB: ohci-jz4740: Remove obsolete driver

2016-05-13 Thread Greg Kroah-Hartman
On Fri, May 13, 2016 at 06:40:17PM +0200, Ralf Baechle wrote:
> Maarten,
> 
> if you submit a USB change to the USB mailing list and maintainer the
> probability for the maintainer to ack this patch will actuall rise
> significantly ;-)
> 
> Greg, I assume this patch is ok to merge or do you want to funnel it
> hrough your tree?  I think it would be good to take this through the
> MIPS tree together with the remainder of the series.

Deleting code?  Yes, I'm all for that :)

Acked-by: Greg Kroah-Hartman 

And yes, actually letting the maintainers know is a good thing, unless
you don't want your patches applied...

thanks,

greg k-h
--
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] cdc-wdm: fix "out-of-sync" due to missing notifications

2016-05-13 Thread Bjørn Mork
Bjørn Mork  writes:

> The driver enforces a strict one-to-one relationship between the
> received RESPONSE_AVAILABLE notifications and messages read from
> the device. At the same time, it will cancel the interrupt URB
> when there is no client holding the character device open.

Never mind.  Forget it.

This patch breaks other devices again.  The immediate and unconditional
reading make them barf. I guess it can be worked around by delaying the
flushing until at least one notification is received, but I obviously
have to test this theory thoroughly on all devices I have.



Bjørn
--
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: Fwd: Downloading tracks from gps via usb

2016-05-13 Thread Stephen Furner
The only reference I can find to usb in a search of the strace output is:

0.001723 stat("usb:", 0x7ffe93f30be0) = -1 ENOENT (No such file or directory)

Yes from what Manfred says it seems likely that the ubuntu
distribution I'm using has only half the hotplug fix implemented. It
does not have the additional driver problem but still has USB access
restricted to root.


Stephen

On Fri, May 13, 2016 at 4:11 PM, Alan Stern  wrote:
> On Thu, 12 May 2016, Stephen Furner wrote:
>
>> I apologise if this is a duplicate I had a bounce back from the list
>> due to it since I had inadvertently included HTML. So I am resending.
>>
>> To: Alan Stern 
>> Cc: USB list 
>>
>> Alan,
>>
>> I have looked through the strace output the following excerpt below
>> directly includes references gebabbel. Is this more in line with what
>> you were expecting it to reveal?
>
> The relevant lines would be those referring to /dev/bus/usb and the
> lines following such references.
>
>> I can of course send the full file to
>> anyone interested in the complete output. It is too large to insert
>> into an e-mail and there is a lot of repetition of my original
>> excerpt.
>>
>> Good point about ubuntu forums. This issue is also question #293560 on
>> the forum https://answers.launchpad.net/ubuntu
>
> The suggestion about adding an /etc/udev/rules.d file looks like a good
> one.
>
> 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: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

2016-05-13 Thread Guenter Roeck
Hi,

On Fri, May 13, 2016 at 05:23:21PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> On Wed, May 11, 2016 at 07:47:18AM -0700, Guenter Roeck wrote:
> > Hi,
> > 
> > On Wed, May 11, 2016 at 12:40:11PM +0300, Heikki Krogerus wrote:
> > > On Tue, May 10, 2016 at 08:14:34PM -0700, Guenter Roeck wrote:
> > > > Heikki,
> > > > 
> > > > On 05/06/2016 01:08 AM, Heikki Krogerus wrote:
> > > > > Hi,
> > > > > 
> > > > [ ... ]
> > > > > 
> > > > > I don't have not made any new code for the class driver yet, but I'm
> > > > > attempting to prepare v2 next week.
> > > > > 
> > > > Would it make sense to send feedback about v1 now, or should I wait for 
> > > > v2 ?
> > > 
> > > I don't think I'm able to send v2 today, or even tomorrow, so feel
> > > free to give the feedback. Just be aware that I've rewritten the
> > > alternate mode part completely.
> > > 
> > Alternate mode handling was my major concern, actually.
> > 
> > > I'm creating a separate device for the partner and also the cable
> > > during connection. I'm also already going to introduce a small bus for
> > > the AltModes. It's clear that we need to have AltMode specific
> > > drivers. The generic parts can't take care of all the AltMode specific
> > > requirements and VDMs. The bus will give us a nice way to bind those
> > > drivers to the actual AltModes a partner and the cable plugs offer.
> > > 
> > > So if there are dependencies between the altmodes, for example if the
> > > cable plugs needs to be in a certain mode in order for the partner to
> > > be able to function in some specific mode, the responsibility of
> > > taking care of those will fall primarily to in the AltMode drivers.
> > > So not userspace.
> > > 
> > > The AltMode drivers actually are useful also as they can be part of
> > > the relevant frameworks, for example DP in some graphics framework.
> > > For example in case of DP, the number of lanes (I guess 2 or 4) should
> > > be ideally known if I have understood correctly. Knowledge about the
> > > connection seems to also be needed, and I've so far seen some pretty
> > > weird solutions for hotplug events with the DP AltMode. With the
> > > driver we should be able to avoid those.
> > > 
> > > But in any case, every SVIDs a partner (or plug) offers will have
> > > their own device registered with the partner (or cable) itself as
> > > parent in this design. I'm expecting a little bit of conversation
> > > about this plan, but right now I feel confident about it.
> > > 
> > > How does this sound to you?
> > > 
> > Looking forward to it. My major problem so far was that alternate mode
> > handling is very platform specific, which didn't seem to be well supported
> > in v1 of your patch. I thought about implementing a hierarchy of drivers
> > below the type-c class to solve that problem. Looks like you just solved
> > it for me.
> > 
> > Other than that, my major concern is the lack of synchronization/protection
> > between the type-c class and the drivers. Setting port parameters (data 
> > role,
> > power role, operational power role, partner alternate modes, partner type)
> > from registered drivers may need to be synchronzed/protected. For example,
> > data and power role are set during connection establishment, but can be
> > overwritten from the typec class code. Right now I am just setting the
> > respective variables in struct typec_port directly, but that doesn't seem
> > right.
> 
> I'm actually struggling with this same question. I decided to protect
> the whole struct typec_port by not allowing the drivers to touch it,
> but I'm still working on it..
> 
Sounds good to me, as long as you provide APIs to change the values.

> > For partner_type, I don't really know how to map the options to the identity
> > reported by the partner. The reported product types are unknown / hub /
> > peripheral / passive cable / active cable / alternate mode adapter.
> > The available partner types are unknown / USB / Charger / Alternate Mode /
> > Audio Accessory / Debug Accessory. What am I missing here ?
> 
> The partner types don't map directly to the USB PD Product Types. We
> need to describe the partner even when USB PD is not available.
> Accessory Modes are for example out side the scope of USB PD.
> 
> But I'll try to propose something for those.
> 
Ok.

> > The rest is just nitpicks.
> > 
> > - alternate_modes_show() and partner_alt_modes_show() discard the last byte
> >   of the generated string and replace it with \0.
> > - s/Accessroy/Accessory/
> > - typec_connect() and typec_disconnect() should probably also set
> >   port->connected.
> 
> OK. I'll check these.
> 
> So I failed to finish my proposal for v2 this week. On top of the
> sync/protection problems, I'm also still trying to tune my AltMode
> bus. I'm going to attempt to send it as an RFC on Monday or Tuesday in
> any case.
> 
No worries. It turns out the PD code in existing type-c devices isn't
exactly stable or predictable, so I end up spending a lot of time
trying to sta

Re: Fwd: Downloading tracks from gps via usb

2016-05-13 Thread Alan Stern
On Fri, 13 May 2016, Stephen Furner wrote:

> The only reference I can find to usb in a search of the strace output is:
> 
> 0.001723 stat("usb:", 0x7ffe93f30be0) = -1 ENOENT (No such file or directory)
> 
> Yes from what Manfred says it seems likely that the ubuntu
> distribution I'm using has only half the hotplug fix implemented. It
> does not have the additional driver problem but still has USB access
> restricted to root.

There ought to be at least one mention of that error message you 
received.  If you find it, you can look backwards from there.  But 
there may be other reasons why it doesn't show up.  For example, the 
program may fork some child processes and one of these children might 
be the source of the error.

Anyway, fixing the file permissions is probably the right approach.

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


[GIT PULL] USB-serial updates for v4.7-rc1

2016-05-13 Thread Johan Hovold
Hi Greg,

Here are my updates for 4.7-rc1. All have been in linux-next without any
reported issues.

Thanks,
Johan


The following changes since commit 02da2d72174c61988eb4456b53f405e3ebdebce4:

  Linux 4.6-rc5 (2016-04-24 16:17:05 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
tags/usb-serial-4.7-rc1

for you to fetch changes up to b923c6c62981cec5e2d2187fd700c2fc4386fc45:

  USB: serial: ti_usb_3410_5052: add MOXA UPORT 11x0 support (2016-05-11 
11:31:44 +0200)


USB-serial updates for v4.7-rc1

These updates fixes a number of issues where resources were not properly
released on probe errors. Included is also a fix for hardware
flow-control disable for cp210x.

Support for Moxa UPort 11x0 is added to the ti_usb_3410_5052 driver, and
included are also some general code clean ups.

Signed-off-by: Johan Hovold 


Javier Martinez Canillas (1):
  USB: serial: use IS_ENABLED() instead of checking for FOO || FOO_MODULE

Johan Hovold (8):
  USB: serial: io_edgeport: fix memory leaks in attach error path
  USB: serial: io_edgeport: fix memory leaks in probe error path
  USB: serial: keyspan: fix use-after-free in probe error path
  USB: serial: keyspan: fix URB unlink
  USB: serial: keyspan: fix debug and error messages
  USB: serial: mxuport: fix use-after-free in probe error path
  USB: serial: quatech2: fix use-after-free in probe error path
  USB: serial: fix minor-number allocation

Julia Lawall (1):
  USB: serial: ftdi_sio: constify ftdi_sio_quirk structures

Konstantin Shkolnyy (3):
  USB: serial: cp210x: fix hardware flow-control disable
  USB: serial: cp210x: get rid of magic numbers in CRTSCTS flag code
  USB: serial: cp210x: clean up CRTSCTS flag code

Mathieu OTHACEHE (1):
  USB: serial: ti_usb_3410_5052: add MOXA UPORT 11x0 support

 drivers/usb/serial/cp210x.c   | 102 --
 drivers/usb/serial/ftdi_sio.c |  16 +++---
 drivers/usb/serial/io_edgeport.c  |  56 +--
 drivers/usb/serial/keyspan.c  |  72 +---
 drivers/usb/serial/mxuport.c  |  10 
 drivers/usb/serial/quatech2.c |   1 +
 drivers/usb/serial/ti_usb_3410_5052.c |  55 +-
 drivers/usb/serial/ti_usb_3410_5052.h |   8 +++
 drivers/usb/serial/usb-serial.c   |   5 +-
 9 files changed, 233 insertions(+), 92 deletions(-)
--
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 v8 10/14] usb: otg: add hcd companion support

2016-05-13 Thread Rob Herring
On Fri, May 13, 2016 at 5:03 AM, Roger Quadros  wrote:
> From: Yoshihiro Shimoda 
>
> Since some host controller (e.g. EHCI) needs a companion host controller
> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> core.
>
> Signed-off-by: Yoshihiro Shimoda 
> Signed-off-by: Roger Quadros 
> Acked-by: Peter Chen 
> ---
>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
>  drivers/usb/common/usb-otg.c  | 32 
> ---
>  include/linux/usb/otg.h   |  7 -
>  3 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
> b/Documentation/devicetree/bindings/usb/generic.txt
> index f6866c1..1db1c33 100644
> --- a/Documentation/devicetree/bindings/usb/generic.txt
> +++ b/Documentation/devicetree/bindings/usb/generic.txt
> @@ -27,6 +27,9 @@ Optional properties:
>   - otg-controller: phandle to otg controller. Host or gadget controllers can
> contain this property to link it to a particular OTG
> controller.
> + - hcd-needs-companion: must be present if otg controller is dealing with
> +   EHCI host controller that needs a companion OHCI host
> +   controller.

I thought the conclusion was this is not needed?

One thing that is not clear here is otg-controller is a host or device
property while hcd-needs-companion is an OTG controller property.
These lists should be separated.

Rob
--
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: Downloading tracks from gps via usb

2016-05-13 Thread Stephen Furner
This is the permissions on the files for the USB being used by the Garmin

stephen@stephen-N150P:/$ lsusb
Bus 001 Device 004: ID 0ac8:c33f Z-Star Microelectronics Corp. Webcam
Bus 001 Device 003: ID 0781:5567 SanDisk Corp. Cruzer Blade
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 002: ID 0a5c:219c Broadcom Corp.
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 004: ID 091e:0003 Garmin International GPS (various models)
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub

stephen@stephen-N150P:/$ ls -l dev/bus/usb/002
total 0
crw-rw-r-- 1 root root 189, 128 May 11 13:39 001
crw-rw-r-- 1 root root 189, 131 May 13 19:03 004

Stephen

On Fri, May 13, 2016 at 3:25 PM, Alan Stern  wrote:
> On Thu, 12 May 2016, Stephen Furner wrote:
>
>> Alan,
>>
>> The permissions for the usb ports are:
>>
>> stephen@stephen-N150P:/$ ls -l dev/bus/usb
>> total 0
>> drwxr-xr-x 2 root root 100 May 11 14:39 001
>> drwxr-xr-x 2 root root  60 May 12 01:48 002
>> drwxr-xr-x 2 root root  80 May 12 21:04 003
>> drwxr-xr-x 2 root root  60 May 11 14:39 004
>> drwxr-xr-x 2 root root  60 May 11 14:39 005
>
> What about the permissions for the files inside those directories?
>
> 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: [PATCH 2/2] musb: sunxi: Set state to A_WAIT_VRISE when enabling VBus

2016-05-13 Thread Bin Liu
Hi,

On Thu, May 12, 2016 at 08:31:09PM +0200, Hans de Goede wrote:
> When the board is powering attached usb devices via the otg port
> sometimes / on some devices it takes slightly too long for the VBus
> detection code in phy-sun4i-usb.c to signal that VBus is high after
> enabling VBus and the musb hardware signals a MUSB_INTR_VBUSERROR
> interrupt.
> 
> This commit sets the otg state to A_WAIT_VRISE upon enabling Vbus
> making musb_stage0_irq() ignore the first VBUSERR_RETRY_COUNT
> VBUSERROR interrupts, fixing connection issues in these cases.
> 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/usb/musb/sunxi.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 2c33d9b..2ee48fb 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -112,7 +112,7 @@ static void sunxi_musb_work(struct work_struct *work)
>   if (test_bit(SUNXI_MUSB_FL_HOSTMODE, &glue->flags)) {
>   set_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
>   musb->xceiv->otg->default_a = 1;
> - musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> + musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
>   MUSB_HST_MODE(musb);
>   devctl |= MUSB_DEVCTL_SESSION;
>   } else {
> @@ -145,9 +145,10 @@ static void sunxi_musb_set_vbus(struct musb *musb, int 
> is_on)
>  {
>   struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
>  
> - if (is_on)
> + if (is_on) {
>   set_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
> - else
> + musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;

Why this has to be set in both sunxi_musb_work() and here? Only setting
it in the work is not efficient?

Regards,
-Bin.

> + } else
>   clear_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
>  
>   schedule_work(&glue->work);
> @@ -325,6 +326,7 @@ static int sunxi_set_mode(struct musb *musb, u8 mode)
>   set_bit(SUNXI_MUSB_FL_PHY_ON, &glue->flags);
>   /* Stop musb work from turning vbus off again */
>   set_bit(SUNXI_MUSB_FL_VBUS_ON, &glue->flags);
> + musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
>   }
>  
>   return 0;
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Downloading tracks from gps via usb

2016-05-13 Thread Alan Stern
On Fri, 13 May 2016, Stephen Furner wrote:

> This is the permissions on the files for the USB being used by the Garmin
> 
> stephen@stephen-N150P:/$ lsusb
> Bus 001 Device 004: ID 0ac8:c33f Z-Star Microelectronics Corp. Webcam
> Bus 001 Device 003: ID 0781:5567 SanDisk Corp. Cruzer Blade
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 003 Device 002: ID 0a5c:219c Broadcom Corp.
> Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 002 Device 004: ID 091e:0003 Garmin International GPS (various models)
> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> 
> stephen@stephen-N150P:/$ ls -l dev/bus/usb/002
> total 0
> crw-rw-r-- 1 root root 189, 128 May 11 13:39 001
> crw-rw-r-- 1 root root 189, 131 May 13 19:03 004

There it is: write permission only for root.  Have you tried installing 
the udev rule recommended on the Ubuntu forum?

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 1/3] properly report a single frame rate of 60 FPS

2016-05-13 Thread Florian Echtler
The device hardware is always running at 60 FPS, so report this both via
PARM_IOCTL and ENUM_FRAMEINTERVALS.

Signed-off-by: Martin Kaltenbrunner 
Signed-off-by: Florian Echtler 
---
 drivers/input/touchscreen/sur40.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c 
b/drivers/input/touchscreen/sur40.c
index 880c40b..fcc5934 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -788,6 +788,16 @@ static int sur40_vidioc_fmt(struct file *file, void *priv,
return 0;
 }
 
+static int sur40_ioctl_parm(struct file *file, void *priv,
+   struct v4l2_streamparm *p)
+{
+   if (p->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+   p->parm.capture.timeperframe.numerator = 1;
+   p->parm.capture.timeperframe.denominator = 60;
+   }
+   return 0;
+}
+
 static int sur40_vidioc_enum_fmt(struct file *file, void *priv,
 struct v4l2_fmtdesc *f)
 {
@@ -814,13 +824,13 @@ static int sur40_vidioc_enum_framesizes(struct file 
*file, void *priv,
 static int sur40_vidioc_enum_frameintervals(struct file *file, void *priv,
struct v4l2_frmivalenum *f)
 {
-   if ((f->index > 1) || (f->pixel_format != V4L2_PIX_FMT_GREY)
+   if ((f->index > 0) || (f->pixel_format != V4L2_PIX_FMT_GREY)
|| (f->width  != sur40_video_format.width)
|| (f->height != sur40_video_format.height))
return -EINVAL;
 
f->type = V4L2_FRMIVAL_TYPE_DISCRETE;
-   f->discrete.denominator  = 60/(f->index+1);
+   f->discrete.denominator  = 60;
f->discrete.numerator = 1;
return 0;
 }
@@ -880,6 +890,9 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
.vidioc_enum_framesizes = sur40_vidioc_enum_framesizes,
.vidioc_enum_frameintervals = sur40_vidioc_enum_frameintervals,
 
+   .vidioc_g_parm = sur40_ioctl_parm,
+   .vidioc_s_parm = sur40_ioctl_parm,
+
.vidioc_enum_input  = sur40_vidioc_enum_input,
.vidioc_g_input = sur40_vidioc_g_input,
.vidioc_s_input = sur40_vidioc_s_input,
-- 
1.9.1

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


[PATCH 2/3] lower poll interval to fix occasional FPS drops to ~56 FPS

2016-05-13 Thread Florian Echtler
The framerate sometimes drops below 60 Hz if the poll interval is too high.
Lowering it to the minimum of 1 ms fixes this.

Signed-off-by: Martin Kaltenbrunner 
Signed-off-by: Florian Echtler 
---
 drivers/input/touchscreen/sur40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c 
b/drivers/input/touchscreen/sur40.c
index fcc5934..7b1052a1 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -126,7 +126,7 @@ struct sur40_image_header {
 #define VIDEO_PACKET_SIZE  16384
 
 /* polling interval (ms) */
-#define POLL_INTERVAL 4
+#define POLL_INTERVAL 1
 
 /* maximum number of contacts FIXME: this is a guess? */
 #define MAX_CONTACTS 64
-- 
1.9.1

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


[PATCH 3/3] fix occasional oopses on device close

2016-05-13 Thread Florian Echtler
Closing the V4L2 device sometimes triggers a kernel oops.
Present patch fixes this.

Signed-off-by: Martin Kaltenbrunner 
Signed-off-by: Florian Echtler 
---
 drivers/input/touchscreen/sur40.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c 
b/drivers/input/touchscreen/sur40.c
index 7b1052a1..38ebb24 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -448,7 +448,7 @@ static void sur40_process_video(struct sur40_state *sur40)
 
/* return error if streaming was stopped in the meantime */
if (sur40->sequence == -1)
-   goto err_poll;
+   return;
 
/* mark as finished */
new_buf->vb.vb2_buf.timestamp = ktime_get_ns();
@@ -736,6 +736,7 @@ static int sur40_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 static void sur40_stop_streaming(struct vb2_queue *vq)
 {
struct sur40_state *sur40 = vb2_get_drv_priv(vq);
+   vb2_wait_for_all_buffers(vq);
sur40->sequence = -1;
 
/* Release all active buffers */
-- 
1.9.1

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


Re: [PATCH 1/3] properly report a single frame rate of 60 FPS

2016-05-13 Thread Hans Verkuil
On 05/13/2016 08:41 PM, Florian Echtler wrote:
> The device hardware is always running at 60 FPS, so report this both via
> PARM_IOCTL and ENUM_FRAMEINTERVALS.

Florian, can you post these three patches to linux-media as well? These are all 
V4L2 related
so they should be reviewed there.

By posting to linux-media these pathes will automatically turn up in patchwork, 
that way
they won't be forgotten.

Regards,

Hans

> 
> Signed-off-by: Martin Kaltenbrunner 
> Signed-off-by: Florian Echtler 
> ---
>  drivers/input/touchscreen/sur40.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c 
> b/drivers/input/touchscreen/sur40.c
> index 880c40b..fcc5934 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -788,6 +788,16 @@ static int sur40_vidioc_fmt(struct file *file, void 
> *priv,
>   return 0;
>  }
>  
> +static int sur40_ioctl_parm(struct file *file, void *priv,
> + struct v4l2_streamparm *p)
> +{
> + if (p->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> + p->parm.capture.timeperframe.numerator = 1;
> + p->parm.capture.timeperframe.denominator = 60;
> + }
> + return 0;
> +}
> +
>  static int sur40_vidioc_enum_fmt(struct file *file, void *priv,
>struct v4l2_fmtdesc *f)
>  {
> @@ -814,13 +824,13 @@ static int sur40_vidioc_enum_framesizes(struct file 
> *file, void *priv,
>  static int sur40_vidioc_enum_frameintervals(struct file *file, void *priv,
>   struct v4l2_frmivalenum *f)
>  {
> - if ((f->index > 1) || (f->pixel_format != V4L2_PIX_FMT_GREY)
> + if ((f->index > 0) || (f->pixel_format != V4L2_PIX_FMT_GREY)
>   || (f->width  != sur40_video_format.width)
>   || (f->height != sur40_video_format.height))
>   return -EINVAL;
>  
>   f->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> - f->discrete.denominator  = 60/(f->index+1);
> + f->discrete.denominator  = 60;
>   f->discrete.numerator = 1;
>   return 0;
>  }
> @@ -880,6 +890,9 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops 
> = {
>   .vidioc_enum_framesizes = sur40_vidioc_enum_framesizes,
>   .vidioc_enum_frameintervals = sur40_vidioc_enum_frameintervals,
>  
> + .vidioc_g_parm = sur40_ioctl_parm,
> + .vidioc_s_parm = sur40_ioctl_parm,
> +
>   .vidioc_enum_input  = sur40_vidioc_enum_input,
>   .vidioc_g_input = sur40_vidioc_g_input,
>   .vidioc_s_input = sur40_vidioc_s_input,
> 
--
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 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Bin Liu
Hi,

On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> * Tony Lindgren  [160511 17:56]:
> > We may have drivers loaded but no configured gadgets and MUSB may be in
> > host mode. If gadgets are configured during host mode, PM runtime will
> > get confused.
> ...
> 
> > @@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
> > *glue)
> > dev_dbg(dev, "ID float\n");
> > }
> >  
> > +   if (!glue->cable_connected)
> > +   omap2430_set_power(musb, glue->enabled, cable_connected);
> > +
> 
> Oops sorry I messed up while cleaning up. This should be just cable_connected
> here instead of glue->cable_connected. Otherwise we won't idle on cable
> unplug as we're always using the cached value instead of the current
> value.
> 
> Updated patch below.
> 
> Regards,
> 
> Tony
> 
> 8< -
> From: Tony Lindgren 
> Date: Mon, 9 May 2016 07:50:57 -0700
> Subject: [PATCH] usb: musb: Handle cable status better for 2430 glue layer
> 
> We may have drivers loaded but no configured gadgets and MUSB may be in
> host mode. If gadgets are configured during host mode, PM runtime will
> get confused.
> 
> Disable PM runtime from gadget state, and do it based on the cable
> and last state.
> 
> Note that we may get multiple cable events, so we need to keep track
> of the power state.
> 
> Signed-off-by: Tony Lindgren 
> 
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -49,6 +49,9 @@ struct omap2430_glue {
>   enum musb_vbus_id_status status;
>   struct work_struct  omap_musb_mailbox_work;
>   struct device   *control_otghs;
> + boolcable_connected;
> + boolenabled;
> + boolpowered;

This variable is only used within omap2430_set_power(), so it can be
local within that function to save a few bytes.

Regards,
-Bin.

>  };
>  #define glue_to_musb(g)  platform_get_drvdata(g->musb)
>  
> @@ -234,6 +237,45 @@ static inline void omap2430_low_level_init(struct musb 
> *musb)
>   musb_writel(musb->mregs, OTG_FORCESTDBY, l);
>  }
>  
> +/*
> + * We can get multiple cable events so we need to keep track
> + * of the power state. Only keep power enabled if USB cable is
> + * connected and a gadget is started.
> + */
> +static void omap2430_set_power(struct musb *musb, bool enabled, bool cable)
> +{
> + struct device *dev = musb->controller;
> + struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> + bool power_up;
> + int res;
> +
> + if (glue->enabled != enabled)
> + glue->enabled = enabled;
> +
> + if (glue->cable_connected != cable)
> + glue->cable_connected = cable;
> +
> + power_up = glue->enabled && glue->cable_connected;
> + if (power_up == glue->powered) {
> + dev_warn(musb->controller, "power state already %i\n",
> +  power_up);
> + return;
> + }
> +
> + glue->powered = power_up;
> +
> + if (power_up) {
> + res = pm_runtime_get_sync(musb->controller);
> + if (res < 0) {
> + dev_err(musb->controller, "could not enable: %i", res);
> + glue->powered = false;
> + }
> + } else {
> + pm_runtime_mark_last_busy(musb->controller);
> + pm_runtime_put_autosuspend(musb->controller);
> + }
> +}
> +
>  static void omap2430_musb_mailbox(enum musb_vbus_id_status status)
>  {
>   struct omap2430_glue*glue = _glue;
> @@ -259,6 +301,13 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
> *glue)
>   struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
>   struct omap_musb_board_data *data = pdata->board_data;
>   struct usb_otg *otg = musb->xceiv->otg;
> + bool cable_connected;
> +
> + cable_connected = ((glue->status == MUSB_ID_GROUND) ||
> +(glue->status == MUSB_VBUS_VALID));
> +
> + if (cable_connected)
> + omap2430_set_power(musb, glue->enabled, cable_connected);
>  
>   switch (glue->status) {
>   case MUSB_ID_GROUND:
> @@ -268,7 +317,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
> *glue)
>   musb->xceiv->otg->state = OTG_STATE_A_IDLE;
>   musb->xceiv->last_event = USB_EVENT_ID;
>   if (musb->gadget_driver) {
> - pm_runtime_get_sync(dev);
>   omap_control_usb_set_mode(glue->control_otghs,
>   USB_MODE_HOST);
>   omap2430_musb_set_vbus(musb, 1);
> @@ -281,8 +329,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
> *glue)
>   otg->default_a = false;
>   musb->xceiv->otg->state = OTG_STATE_B_IDLE;
>   musb->xceiv->last_event = USB_EVENT_VBUS;
> - if (musb->gadget_driver)
> - pm_runtime_get_sync(dev);
> 

Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Bin Liu  [160513 12:58]:
> On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > enum musb_vbus_id_status status;
> > struct work_struct  omap_musb_mailbox_work;
> > struct device   *control_otghs;
> > +   boolcable_connected;
> > +   boolenabled;
> > +   boolpowered;
> 
> This variable is only used within omap2430_set_power(), so it can be
> local within that function to save a few bytes.

Well it needs to be static as we keep things powered only if
glue is enabled and cable is connected.

I'd rather not add any more static stuff that's not specific to the
glue instance.. Got any better ideas?

BTW, we still have also at least _glue remaining in the 2430 glue.

Regards,

Tony
--
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 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Bin Liu
Hi,

On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> * Bin Liu  [160513 12:58]:
> > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > --- a/drivers/usb/musb/omap2430.c
> > > +++ b/drivers/usb/musb/omap2430.c
> > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > >   enum musb_vbus_id_status status;
> > >   struct work_struct  omap_musb_mailbox_work;
> > >   struct device   *control_otghs;
> > > + boolcable_connected;
> > > + boolenabled;
> > > + boolpowered;
> > 
> > This variable is only used within omap2430_set_power(), so it can be
> > local within that function to save a few bytes.
> 
> Well it needs to be static as we keep things powered only if
> glue is enabled and cable is connected.

I think it does not have to static in omap2430_glue, how about in the
very beginning of omap2430_set_power(),

bool powered = glue->enabled && glue->cable_connected;

before changing glue->enabled and glue->cable_connected, then this
local powered variable is equivalent to glue->powered.

> 
> I'd rather not add any more static stuff that's not specific to the
> glue instance.. Got any better ideas?

I didn't think hard enough for this, but I think your idea is acceptable
;).

Regards,
-Bin.

> 
> BTW, we still have also at least _glue remaining in the 2430 glue.
> 
> Regards,
> 
> Tony
--
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 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Bin Liu
On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> Hi,
> 
> On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > * Bin Liu  [160513 12:58]:
> > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > --- a/drivers/usb/musb/omap2430.c
> > > > +++ b/drivers/usb/musb/omap2430.c
> > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > enum musb_vbus_id_status status;
> > > > struct work_struct  omap_musb_mailbox_work;
> > > > struct device   *control_otghs;
> > > > +   boolcable_connected;
> > > > +   boolenabled;
> > > > +   boolpowered;
> > > 
> > > This variable is only used within omap2430_set_power(), so it can be
> > > local within that function to save a few bytes.
> > 
> > Well it needs to be static as we keep things powered only if
> > glue is enabled and cable is connected.
> 
> I think it does not have to static in omap2430_glue, how about in the

I meant in omap2430_set_power().

> very beginning of omap2430_set_power(),
> 
>   bool powered = glue->enabled && glue->cable_connected;
> 
> before changing glue->enabled and glue->cable_connected, then this
> local powered variable is equivalent to glue->powered.
> 
> > 
> > I'd rather not add any more static stuff that's not specific to the
> > glue instance.. Got any better ideas?
> 
> I didn't think hard enough for this, but I think your idea is acceptable
> ;).
> 
> Regards,
> -Bin.
> 
> > 
> > BTW, we still have also at least _glue remaining in the 2430 glue.
> > 
> > Regards,
> > 
> > Tony
--
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 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Bin Liu  [160513 13:26]:
> On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> > On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > > * Bin Liu  [160513 12:58]:
> > > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > > --- a/drivers/usb/musb/omap2430.c
> > > > > +++ b/drivers/usb/musb/omap2430.c
> > > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > >   enum musb_vbus_id_status status;
> > > > >   struct work_struct  omap_musb_mailbox_work;
> > > > >   struct device   *control_otghs;
> > > > > + boolcable_connected;
> > > > > + boolenabled;
> > > > > + boolpowered;
> > > > 
> > > > This variable is only used within omap2430_set_power(), so it can be
> > > > local within that function to save a few bytes.
> > > 
> > > Well it needs to be static as we keep things powered only if
> > > glue is enabled and cable is connected.
> > 
> > I think it does not have to static in omap2430_glue, how about in the
> 
> I meant in omap2430_set_power().
> 
> > very beginning of omap2430_set_power(),
> > 
> > bool powered = glue->enabled && glue->cable_connected;
> > 
> > before changing glue->enabled and glue->cable_connected, then this
> > local powered variable is equivalent to glue->powered.

Hmm but pm_runtime_get_sync() can fail too. I was seeing -EACCES
there while chasing various PM runtime issues. That can happen for
example if the parent and child PM runtime use counts get out of sync
like happened in the -EPROBE_DEFER case. Now we at least know if
something goes wrong with the "could not enable" error.

Regards,

Tony
--
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 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Bin Liu
Hi,

On Fri, May 13, 2016 at 01:33:30PM -0700, Tony Lindgren wrote:
> * Bin Liu  [160513 13:26]:
> > On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> > > On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > > > * Bin Liu  [160513 12:58]:
> > > > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > > > --- a/drivers/usb/musb/omap2430.c
> > > > > > +++ b/drivers/usb/musb/omap2430.c
> > > > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > > > enum musb_vbus_id_status status;
> > > > > > struct work_struct  omap_musb_mailbox_work;
> > > > > > struct device   *control_otghs;
> > > > > > +   boolcable_connected;
> > > > > > +   boolenabled;
> > > > > > +   boolpowered;
> > > > > 
> > > > > This variable is only used within omap2430_set_power(), so it can be
> > > > > local within that function to save a few bytes.
> > > > 
> > > > Well it needs to be static as we keep things powered only if
> > > > glue is enabled and cable is connected.
> > > 
> > > I think it does not have to static in omap2430_glue, how about in the
> > 
> > I meant in omap2430_set_power().
> > 
> > > very beginning of omap2430_set_power(),
> > > 
> > >   bool powered = glue->enabled && glue->cable_connected;
> > > 
> > > before changing glue->enabled and glue->cable_connected, then this
> > > local powered variable is equivalent to glue->powered.
> 
> Hmm but pm_runtime_get_sync() can fail too. I was seeing -EACCES
> there while chasing various PM runtime issues. That can happen for
> example if the parent and child PM runtime use counts get out of sync
> like happened in the -EPROBE_DEFER case. Now we at least know if
> something goes wrong with the "could not enable" error.

Well, cannot think of why this variable could cause this problem. But
anyway, please ignore my comments, it is only about 4 bytes.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
--
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 07/15] usb: musb: Handle cable status better for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Bin Liu  [160513 13:43]:
> Hi,
> 
> On Fri, May 13, 2016 at 01:33:30PM -0700, Tony Lindgren wrote:
> > * Bin Liu  [160513 13:26]:
> > > On Fri, May 13, 2016 at 03:21:55PM -0500, Bin Liu wrote:
> > > > On Fri, May 13, 2016 at 01:09:02PM -0700, Tony Lindgren wrote:
> > > > > * Bin Liu  [160513 12:58]:
> > > > > > On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> > > > > > > --- a/drivers/usb/musb/omap2430.c
> > > > > > > +++ b/drivers/usb/musb/omap2430.c
> > > > > > > @@ -49,6 +49,9 @@ struct omap2430_glue {
> > > > > > >   enum musb_vbus_id_status status;
> > > > > > >   struct work_struct  omap_musb_mailbox_work;
> > > > > > >   struct device   *control_otghs;
> > > > > > > + boolcable_connected;
> > > > > > > + boolenabled;
> > > > > > > + boolpowered;
> > > > > > 
> > > > > > This variable is only used within omap2430_set_power(), so it can be
> > > > > > local within that function to save a few bytes.
> > > > > 
> > > > > Well it needs to be static as we keep things powered only if
> > > > > glue is enabled and cable is connected.
> > > > 
> > > > I think it does not have to static in omap2430_glue, how about in the
> > > 
> > > I meant in omap2430_set_power().
> > > 
> > > > very beginning of omap2430_set_power(),
> > > > 
> > > > bool powered = glue->enabled && glue->cable_connected;
> > > > 
> > > > before changing glue->enabled and glue->cable_connected, then this
> > > > local powered variable is equivalent to glue->powered.
> > 
> > Hmm but pm_runtime_get_sync() can fail too. I was seeing -EACCES
> > there while chasing various PM runtime issues. That can happen for
> > example if the parent and child PM runtime use counts get out of sync
> > like happened in the -EPROBE_DEFER case. Now we at least know if
> > something goes wrong with the "could not enable" error.
> 
> Well, cannot think of why this variable could cause this problem. But
> anyway, please ignore my comments, it is only about 4 bytes.

It's because we currently need to keep track of powered in addition
to cable_connected and enabled. Otherwise we don't get get warnings
related to PM runtime issues.

And if we use static bool powered in omap2430_set_power(), it won't
be specific to the glue layer driver instance :)

Tony



--
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 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Bin Liu
Hi,

On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> At least 2430 glue layer pulls d+ high on start up even if there are
> no gadgets configured. This is bad at least for anything using a separate
> battery charger chip as it can confuse the charger detection.
> 
> Let's fix the issue by getting rid of omap2430_musb_set_mode() and only

By doing so, you lost the feature of switching mode from sysfs, I am not
sure if there is anyone using it though, still, it is a regression.

> write session bit in omap2430_musb_enable().

I don't think session bit needs to be set in _enable(), musb_start() in
core takes care of this bit.

> 
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/usb/musb/omap2430.c | 14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 79e4cd7..958ae6a 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -122,16 +122,6 @@ static void omap2430_musb_set_vbus(struct musb *musb, 
> int is_on)
>   musb_readb(musb->mregs, MUSB_DEVCTL));
>  }
>  
> -static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
> -{
> - u8  devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> -
> - devctl |= MUSB_DEVCTL_SESSION;
> - musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);

In stead of removing it, session bit should only be set when musb_mode
== MUSB_HOST, will this fix the D+ pullup problem?

> -
> - return 0;
> -}
> -
>  static inline void omap2430_low_level_exit(struct musb *musb)
>  {
>   u32 l;
> @@ -428,6 +418,9 @@ static void omap2430_musb_enable(struct musb *musb)
>  
>   case MUSB_VBUS_VALID:
>   omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> + devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> + devctl |= MUSB_DEVCTL_SESSION;
> + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);

session bit is only set for host mode, VBUS_VALID is not a signal for
host mode. So you don't have to set the bit here.

>   break;
>  
>   default:
> @@ -472,7 +465,6 @@ static const struct musb_platform_ops omap2430_ops = {
>   .init   = omap2430_musb_init,
>   .exit   = omap2430_musb_exit,
>  
> - .set_mode   = omap2430_musb_set_mode,
>   .set_vbus   = omap2430_musb_set_vbus,
>  
>   .enable = omap2430_musb_enable,
> -- 
> 2.8.1
> 

Regards,
-Bin.

--
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 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Bin Liu  [160513 14:05]:
> Hi,
> 
> On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > At least 2430 glue layer pulls d+ high on start up even if there are
> > no gadgets configured. This is bad at least for anything using a separate
> > battery charger chip as it can confuse the charger detection.
> > 
> > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> 
> By doing so, you lost the feature of switching mode from sysfs, I am not
> sure if there is anyone using it though, still, it is a regression.

Oh right, that's a good point.

How about we change musb_core to call the optional set_mode() if implemented,
and then set the session bit in host mode only? That way we can get rid of
the musb core tinkering in the glue layer drivers eventually?

> > write session bit in omap2430_musb_enable().
> 
> I don't think session bit needs to be set in _enable(), musb_start() in
> core takes care of this bit.

OK sounds like that should just work then.

> > -static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
> > -{
> > -   u8  devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > -
> > -   devctl |= MUSB_DEVCTL_SESSION;
> > -   musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> 
> In stead of removing it, session bit should only be set when musb_mode
> == MUSB_HOST, will this fix the D+ pullup problem?

Good point, I forgot about it being specific to host mode. I'll check.

> >  static inline void omap2430_low_level_exit(struct musb *musb)
> >  {
> > u32 l;
> > @@ -428,6 +418,9 @@ static void omap2430_musb_enable(struct musb *musb)
> >  
> > case MUSB_VBUS_VALID:
> > omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> > +   devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > +   devctl |= MUSB_DEVCTL_SESSION;
> > +   musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> 
> session bit is only set for host mode, VBUS_VALID is not a signal for
> host mode. So you don't have to set the bit here.

OK thanks.

Tony
--
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 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Bin Liu
Hi,

On Fri, May 13, 2016 at 02:17:39PM -0700, Tony Lindgren wrote:
> * Bin Liu  [160513 14:05]:
> > Hi,
> > 
> > On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > > At least 2430 glue layer pulls d+ high on start up even if there are
> > > no gadgets configured. This is bad at least for anything using a separate
> > > battery charger chip as it can confuse the charger detection.
> > > 
> > > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > 
> > By doing so, you lost the feature of switching mode from sysfs, I am not
> > sure if there is anyone using it though, still, it is a regression.
> 
> Oh right, that's a good point.
> 
> How about we change musb_core to call the optional set_mode() if implemented,

The core already does so. Please check musb_core.h.

Regards,
-Bin.

> and then set the session bit in host mode only? That way we can get rid of
> the musb core tinkering in the glue layer drivers eventually?
> 
> > > write session bit in omap2430_musb_enable().
> > 
> > I don't think session bit needs to be set in _enable(), musb_start() in
> > core takes care of this bit.
> 
> OK sounds like that should just work then.
> 
> > > -static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
> > > -{
> > > - u8  devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > > -
> > > - devctl |= MUSB_DEVCTL_SESSION;
> > > - musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> > 
> > In stead of removing it, session bit should only be set when musb_mode
> > == MUSB_HOST, will this fix the D+ pullup problem?
> 
> Good point, I forgot about it being specific to host mode. I'll check.
> 
> > >  static inline void omap2430_low_level_exit(struct musb *musb)
> > >  {
> > >   u32 l;
> > > @@ -428,6 +418,9 @@ static void omap2430_musb_enable(struct musb *musb)
> > >  
> > >   case MUSB_VBUS_VALID:
> > >   omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> > > + devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> > > + devctl |= MUSB_DEVCTL_SESSION;
> > > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
> > 
> > session bit is only set for host mode, VBUS_VALID is not a signal for
> > host mode. So you don't have to set the bit here.
> 
> OK thanks.
> 
> Tony
--
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 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Bin Liu  [160513 14:24]:
> Hi,
> 
> On Fri, May 13, 2016 at 02:17:39PM -0700, Tony Lindgren wrote:
> > * Bin Liu  [160513 14:05]:
> > > Hi,
> > > 
> > > On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > > > At least 2430 glue layer pulls d+ high on start up even if there are
> > > > no gadgets configured. This is bad at least for anything using a 
> > > > separate
> > > > battery charger chip as it can confuse the charger detection.
> > > > 
> > > > Let's fix the issue by getting rid of omap2430_musb_set_mode() and only
> > > 
> > > By doing so, you lost the feature of switching mode from sysfs, I am not
> > > sure if there is anyone using it though, still, it is a regression.
> > 
> > Oh right, that's a good point.
> > 
> > How about we change musb_core to call the optional set_mode() if 
> > implemented,
> 
> The core already does so. Please check musb_core.h.

Oh do you have some pending patches for this already not yet
in Linux next?

> > and then set the session bit in host mode only? That way we can get rid of
> > the musb core tinkering in the glue layer drivers eventually?

So currently we have this in musb_core.h:

static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
{
if (!musb->ops->set_mode)
return 0;

return musb->ops->set_mode(musb, mode);
}

What I meant is we could add generic support for the session bit:

static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
{
if (!musb->ops->set_mode)
return musb_default_set_mode(musb, mode);

return musb->ops->set_mode(musb, mode);
}

Regards,

Tony
--
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 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Bin Liu
Hi,

On Fri, May 13, 2016 at 02:39:01PM -0700, Tony Lindgren wrote:
> * Bin Liu  [160513 14:24]:
> > Hi,
> > 
> > On Fri, May 13, 2016 at 02:17:39PM -0700, Tony Lindgren wrote:
> > > * Bin Liu  [160513 14:05]:
> > > > Hi,
> > > > 
> > > > On Wed, May 11, 2016 at 05:53:11PM -0700, Tony Lindgren wrote:
> > > > > At least 2430 glue layer pulls d+ high on start up even if there are
> > > > > no gadgets configured. This is bad at least for anything using a 
> > > > > separate
> > > > > battery charger chip as it can confuse the charger detection.
> > > > > 
> > > > > Let's fix the issue by getting rid of omap2430_musb_set_mode() and 
> > > > > only
> > > > 
> > > > By doing so, you lost the feature of switching mode from sysfs, I am not
> > > > sure if there is anyone using it though, still, it is a regression.
> > > 
> > > Oh right, that's a good point.
> > > 
> > > How about we change musb_core to call the optional set_mode() if 
> > > implemented,
> > 
> > The core already does so. Please check musb_core.h.
> 
> Oh do you have some pending patches for this already not yet
> in Linux next?

No.

> 
> > > and then set the session bit in host mode only? That way we can get rid of
> > > the musb core tinkering in the glue layer drivers eventually?
> 
> So currently we have this in musb_core.h:
> 
> static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> {
>   if (!musb->ops->set_mode)
>   return 0;
> 
>   return musb->ops->set_mode(musb, mode);
> }
> 
> What I meant is we could add generic support for the session bit:
> 
> static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
> {
>   if (!musb->ops->set_mode)
>   return musb_default_set_mode(musb, mode);
> 
>   return musb->ops->set_mode(musb, mode);
> }

But what would be in musb_default_set_mode()? Currently only am35x,
da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
have any in common. Only omap2430 sets session bit in _set_mode(), no
one else does so.

Regards,
-Bin.

> 
> Regards,
> 
> Tony
--
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/3] properly report a single frame rate of 60 FPS

2016-05-13 Thread Florian Echtler
Fixed, sorry for the noise. One more question: Martin and I would also
like to see these patches in the 4.4 longterm kernel; do we have to
submit them separately, or will Greg KH pick them up eventually?

Thanks & best regards, Florian

On 13.05.2016 11:53, Hans Verkuil wrote:
> On 05/13/2016 08:41 PM, Florian Echtler wrote:
>> The device hardware is always running at 60 FPS, so report this both via
>> PARM_IOCTL and ENUM_FRAMEINTERVALS.
> 
> Florian, can you post these three patches to linux-media as well? These are 
> all V4L2 related
> so they should be reviewed there.
> 
> By posting to linux-media these pathes will automatically turn up in 
> patchwork, that way
> they won't be forgotten.
> 
> Regards,
> 
>   Hans
> 
>>
>> Signed-off-by: Martin Kaltenbrunner 
>> Signed-off-by: Florian Echtler 
>> ---
>>  drivers/input/touchscreen/sur40.c | 17 +++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/sur40.c 
>> b/drivers/input/touchscreen/sur40.c
>> index 880c40b..fcc5934 100644
>> --- a/drivers/input/touchscreen/sur40.c
>> +++ b/drivers/input/touchscreen/sur40.c
>> @@ -788,6 +788,16 @@ static int sur40_vidioc_fmt(struct file *file, void 
>> *priv,
>>  return 0;
>>  }
>>  
>> +static int sur40_ioctl_parm(struct file *file, void *priv,
>> +struct v4l2_streamparm *p)
>> +{
>> +if (p->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
>> +p->parm.capture.timeperframe.numerator = 1;
>> +p->parm.capture.timeperframe.denominator = 60;
>> +}
>> +return 0;
>> +}
>> +
>>  static int sur40_vidioc_enum_fmt(struct file *file, void *priv,
>>   struct v4l2_fmtdesc *f)
>>  {
>> @@ -814,13 +824,13 @@ static int sur40_vidioc_enum_framesizes(struct file 
>> *file, void *priv,
>>  static int sur40_vidioc_enum_frameintervals(struct file *file, void *priv,
>>  struct v4l2_frmivalenum *f)
>>  {
>> -if ((f->index > 1) || (f->pixel_format != V4L2_PIX_FMT_GREY)
>> +if ((f->index > 0) || (f->pixel_format != V4L2_PIX_FMT_GREY)
>>  || (f->width  != sur40_video_format.width)
>>  || (f->height != sur40_video_format.height))
>>  return -EINVAL;
>>  
>>  f->type = V4L2_FRMIVAL_TYPE_DISCRETE;
>> -f->discrete.denominator  = 60/(f->index+1);
>> +f->discrete.denominator  = 60;
>>  f->discrete.numerator = 1;
>>  return 0;
>>  }
>> @@ -880,6 +890,9 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops 
>> = {
>>  .vidioc_enum_framesizes = sur40_vidioc_enum_framesizes,
>>  .vidioc_enum_frameintervals = sur40_vidioc_enum_frameintervals,
>>  
>> +.vidioc_g_parm = sur40_ioctl_parm,
>> +.vidioc_s_parm = sur40_ioctl_parm,
>> +
>>  .vidioc_enum_input  = sur40_vidioc_enum_input,
>>  .vidioc_g_input = sur40_vidioc_g_input,
>>  .vidioc_s_input = sur40_vidioc_s_input,
>>


-- 
SENT FROM MY DEC VT50 TERMINAL



signature.asc
Description: OpenPGP digital signature


Re: [PATCH 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Bin Liu  [160513 15:04]:
> 
> But what would be in musb_default_set_mode()? Currently only am35x,
> da8xx, dsps, and omap2430 glues implement _set_mode(), but they don't
> have any in common. Only omap2430 sets session bit in _set_mode(), no
> one else does so.

Well how about the following if no glue specific configuration of the
ID pin is possible?

Tony

8< ---
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -416,6 +416,26 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, 
const u8 *src)
return hw_ep->musb->io.write_fifo(hw_ep, len, src);
 }
 
+static int musb_try_set_mode(struct musb *musb, u8 mode)
+{
+   if (musb->ops->set_mode)
+   return musb->ops->set_mode(musb, mode);
+
+   if (mode == MUSB_HOST) {
+   u8 devctl;
+
+   devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
+   devctl |= MUSB_DEVCTL_SESSION;
+   musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
+   } else {
+   dev_warn(musb->controller,
+"platform specific set_mode not implemnted: %i\n",
+mode);
+   }
+
+   return 0;
+}
+
 /*-*/
 
 /* for high speed test mode; see USB 2.0 spec 7.1.20 */
@@ -1723,11 +1743,11 @@ musb_mode_store(struct device *dev, struct 
device_attribute *attr,
 
spin_lock_irqsave(&musb->lock, flags);
if (sysfs_streq(buf, "host"))
-   status = musb_platform_set_mode(musb, MUSB_HOST);
+   status = musb_try_set_mode(musb, MUSB_HOST);
else if (sysfs_streq(buf, "peripheral"))
-   status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+   status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
else if (sysfs_streq(buf, "otg"))
-   status = musb_platform_set_mode(musb, MUSB_OTG);
+   status = musb_try_set_mode(musb, MUSB_OTG);
else
status = -EINVAL;
spin_unlock_irqrestore(&musb->lock, flags);
@@ -2185,13 +2205,13 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
status = musb_host_setup(musb, plat->power);
if (status < 0)
goto fail3;
-   status = musb_platform_set_mode(musb, MUSB_HOST);
+   status = musb_try_set_mode(musb, MUSB_HOST);
break;
case MUSB_PORT_MODE_GADGET:
status = musb_gadget_setup(musb);
if (status < 0)
goto fail3;
-   status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+   status = musb_try_set_mode(musb, MUSB_PERIPHERAL);
break;
case MUSB_PORT_MODE_DUAL_ROLE:
status = musb_host_setup(musb, plat->power);
@@ -2202,7 +,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
musb_host_cleanup(musb);
goto fail3;
}
-   status = musb_platform_set_mode(musb, MUSB_OTG);
+   status = musb_try_set_mode(musb, MUSB_OTG);
break;
default:
dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -556,14 +556,6 @@ static inline void musb_platform_disable(struct musb *musb)
musb->ops->disable(musb);
 }
 
-static inline int musb_platform_set_mode(struct musb *musb, u8 mode)
-{
-   if (!musb->ops->set_mode)
-   return 0;
-
-   return musb->ops->set_mode(musb, mode);
-}
-
 static inline void musb_platform_try_idle(struct musb *musb,
unsigned long timeout)
 {
--
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 10/15] usb: musb: Don't set d+ high before enable for 2430 glue layer

2016-05-13 Thread Tony Lindgren
* Tony Lindgren  [160513 14:19]:
> * Bin Liu  [160513 14:05]:
> > In stead of removing it, session bit should only be set when musb_mode
> > == MUSB_HOST, will this fix the D+ pullup problem?
> 
> Good point, I forgot about it being specific to host mode. I'll check.

Yeah good call, the patch below fixes the issue. Then we can remove the
set_mode later if the generic approach looks OK to you.

Regards,

Tony

8< --
From: Tony Lindgren 
Date: Fri, 13 May 2016 07:59:35 -0700
Subject: [PATCH] usb: musb: Don't set d+ high before enable for 2430 glue
 layer

At least 2430 glue layer pulls d+ high on start up even if there are
no gadgets configured. This is bad at least for anything using a separate
battery charger chip as it can confuse the charger detection.

Let's fix the issue by only setting the SESSION bit in host mode
as suggested by Bin Liu .

Signed-off-by: Tony Lindgren 

--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -124,8 +124,12 @@ static void omap2430_musb_set_vbus(struct musb *musb, int 
is_on)
 
 static int omap2430_musb_set_mode(struct musb *musb, u8 musb_mode)
 {
-   u8  devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
+   u8 devctl;
 
+   if (musb_mode != MUSB_HOST)
+   return 0;
+
+   devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
devctl |= MUSB_DEVCTL_SESSION;
musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
 
--
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 v4] usb: dwc2: fix regression on big-endian PowerPC/ARM systems

2016-05-13 Thread John Youn
On 5/13/2016 6:53 AM, Arnd Bergmann wrote:
> A patch that went into Linux-4.4 to fix big-endian mode on a Lantiq
> MIPS system unfortunately broke big-endian operation on PowerPC
> APM82181 as reported by Christian Lamparter, and likely other
> systems.
> 
> It actually introduced multiple issues:
> 
> - it broke big-endian ARM kernels: any machine that was working
>   correctly with a little-endian kernel is no longer using byteswaps
>   on big-endian kernels, which clearly breaks them.
> - On PowerPC the same thing must be true: if it was working before,
>   using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC
>   usually uses big-endian kernels, so they are likely all broken.
> - The barrier for dwc2_writel is on the wrong side of the __raw_writel(),
>   so the MMIO no longer synchronizes with DMA operations.
> - On architectures that require specific CPU instructions for MMIO
>   access, using the __raw_ variant may turn this into a pointer
>   dereference that does not have the same effect as the readl/writel.
> 
> This patch is a simple revert for all architectures other than MIPS,
> in the hope that we can more easily backport it to fix the regression
> on PowerPC and ARM systems without breaking the Lantiq system again.
> 
> We should follow this up with a more elaborate change to add runtime
> detection of endianness, to make sure it also works on all other
> combinations of architectures and implementations of the usb-dwc2
> device. That patch however will be fairly large and not appropriate
> for backports to stable kernels.
> 
> Felipe suggested a different approach, using an endianness switching
> register to always put the device into LE mode, but unfortunately
> the dwc2 hardware does not provide a generic way to do that. Also,
> I see no practical way of addressing the problem more generally by
> patching architecture specific code on MIPS.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 95c8bc360944 ("usb: dwc2: Use platform endianness when accessing 
> registers")
> ---
>  drivers/usb/dwc2/core.h | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 3c58d633ce80..dec0b21fc626 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -64,6 +64,17 @@
>   DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),\
>   dev_name(hsotg->dev), ##__VA_ARGS__)
>  
> +#ifdef CONFIG_MIPS
> +/*
> + * There are some MIPS machines that can run in either big-endian
> + * or little-endian mode and that use the dwc2 register without
> + * a byteswap in both ways.
> + * Unlike other architectures, MIPS apparently does not require a
> + * barrier before the __raw_writel() to synchronize with DMA but does
> + * require the barrier after the __raw_writel() to serialize a set of
> + * writes. This set of operations was added specifically for MIPS and
> + * should only be used there.
> + */
>  static inline u32 dwc2_readl(const void __iomem *addr)
>  {
>   u32 value = __raw_readl(addr);
> @@ -90,6 +101,22 @@ static inline void dwc2_writel(u32 value, void __iomem 
> *addr)
>   pr_info("INFO:: wrote %08x to %p\n", value, addr);
>  #endif
>  }
> +#else
> +/* Normal architectures just use readl/write */
> +static inline u32 dwc2_readl(const void __iomem *addr)
> +{
> + return readl(addr);
> +}
> +
> +static inline void dwc2_writel(u32 value, void __iomem *addr)
> +{
> + writel(value, addr);
> +
> +#ifdef DWC2_LOG_WRITES
> + pr_info("info:: wrote %08x to %p\n", value, addr);
> +#endif
> +}
> +#endif
>  
>  /* Maximum number of Endpoints/HostChannels */
>  #define MAX_EPS_CHANNELS 16
> 

Thanks Arnd.

Acked-by: John Youn 

John


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


Re: Downloading tracks from gps via usb

2016-05-13 Thread Stephen Furner
There seems to be something in addition to just the usb file access
priverlages. When I changed them it changed the error message it did
not resolve the problem as was expected.

I used the following CHMOD command to change the access priverlages
for the usb files

chmod 666 /dev/bus/usb -R

This gave me rw priverlages on the usb files for all users

stephen@stephen-N150P:/$ sudo ls -l dev/bus/usb
[sudo] password for stephen:
total 0
drw-rw-rw- 2 root root 100 May 11 14:39 001
drw-rw-rw- 2 root root  80 May 14 03:18 002
drw-rw-rw- 2 root root  80 May 12 21:04 003
drw-rw-rw- 2 root root  60 May 11 14:39 004
drw-rw-rw- 2 root root  60 May 11 14:39 005

When I tried to use gebabbel or viking to download the tracks from the
gps I got the error message "Found no Garmin USB devices."

listing the usb devices does however show it is present.

stephen@stephen-N150P:/$ lsusb
Bus 001 Device 004: ID 0ac8:c33f Z-Star Microelectronics Corp. Webcam
Bus 001 Device 003: ID 0781:5567 SanDisk Corp. Cruzer Blade
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 002: ID 0a5c:219c Broadcom Corp.
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 005: ID 091e:0003 Garmin International GPS (various models)
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub


Using gpsbabel on the command line without root privileges to download
from the gps also generates the same error message. An strace of
gpsbabel provided the following excerpt.

openat(AT_FDCWD, "/dev/bus/usb", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 7 entries */, 32768) = 168
close(3)= 0
openat(AT_FDCWD, "/dev/bus/usb", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 7 entries */, 32768) = 168
getdents(3, /* 0 entries */, 32768) = 0
close(3)= 0
openat(AT_FDCWD, "/dev/bus/usb/005",
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = -1 EACCES (Permission
denied)
write(2, "Found no Garmin USB devices.\n", 29Found no Garmin USB devices.
) = 29
exit_group(1)



On Fri, May 13, 2016 at 7:47 PM, Alan Stern  wrote:
> On Fri, 13 May 2016, Stephen Furner wrote:
>
>> This is the permissions on the files for the USB being used by the Garmin
>>
>> stephen@stephen-N150P:/$ lsusb
>> Bus 001 Device 004: ID 0ac8:c33f Z-Star Microelectronics Corp. Webcam
>> Bus 001 Device 003: ID 0781:5567 SanDisk Corp. Cruzer Blade
>> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>> Bus 003 Device 002: ID 0a5c:219c Broadcom Corp.
>> Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>> Bus 002 Device 004: ID 091e:0003 Garmin International GPS (various models)
>> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>>
>> stephen@stephen-N150P:/$ ls -l dev/bus/usb/002
>> total 0
>> crw-rw-r-- 1 root root 189, 128 May 11 13:39 001
>> crw-rw-r-- 1 root root 189, 131 May 13 19:03 004
>
> There it is: write permission only for root.  Have you tried installing
> the udev rule recommended on the Ubuntu forum?
>
> 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