Re: [RFC PATCH v2 4/4] ARM: dts: vf-colibri: USB device/host switch using extcon gpio
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
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
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
* 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
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
* 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
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
* 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
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
* 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
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
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
* 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
* 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
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
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