Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 10:36 AM, robert.jarz...@free.fr wrote: - Mail original - De: Fabio Estevam feste...@gmail.com À: Robert Jarzmik robert.jarz...@free.fr Cc: Felipe Balbi ba...@ti.com, Linus Walleij linus.wall...@linaro.org, Philipp Zabel philipp.za...@gmail.com, USB list linux-usb@vger.kernel.org Envoyé: Mardi 27 Janvier 2015 12:23:43 Objet: Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc I'll amend my patch just as you said, my bad. but the problem I see is that even if I use: nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); The 'reset-gpio' pin cannot be retrieved. This is the point I don't understand currently. If the reset-gpio device-tree property doesn't exist, devm_gpiod_get_optional() will return a NULL pointer, and nop-gpiod_reset = NULL. I agree, but 'reset-gpio' property exists in imx51-babbage.dts, but devm_gpiod_get_optional(dev, reset) is returning NULL. This is the problem that puzzles me at the moment. Thanks -- 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 1/3] usb: phy: generic: migrate to gpio_desc
- Mail original - De: Fabio Estevam feste...@gmail.com À: Robert Jarzmik robert.jarz...@free.fr Cc: Felipe Balbi ba...@ti.com, Linus Walleij linus.wall...@linaro.org, Philipp Zabel philipp.za...@gmail.com, USB list linux-usb@vger.kernel.org Envoyé: Mardi 27 Janvier 2015 13:41:57 Objet: Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc I agree, but 'reset-gpio' property exists in imx51-babbage.dts, but devm_gpiod_get_optional(dev, reset) is returning NULL. This is the problem that puzzles me at the moment. Is in the right node, ie. the node that has compatible == usb-nop-xceiv ? Cheers. -- Robert -- 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] dt: usb: jz4740: Add DT binding document for OHCI
From: Paul Burton paul.bur...@imgtec.com
Add the binding documentation for the JZ47xx OHCI controller.
Signed-off-by: Paul Burton paul.bur...@imgtec.com
Signed-off-by: Zubair Lutfullah Kakakhel zubair.kakak...@imgtec.com
---
The jz4740 is platform only at the moment.
But DT support is being added
See http://patchwork.linux-mips.org/bundle/paulburton/ci20-v3.20/
jz47xx is used because jz4780 will also use this driver
---
.../bindings/usb/ingenic,jz47xx-ohci.txt | 28 ++
1 file changed, 28 insertions(+)
create mode 100644
Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
diff --git a/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
b/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
new file mode 100644
index 000..2fada8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
@@ -0,0 +1,28 @@
+Ingenic JZ47XX SoC OHCI controller binding
+
+The Ingenic JZ47XX SoC includes an OHCI compliant USB host controller
+interface for use with USB 1.1 devices.
+
+Required properties:
+ - compatible: Should be ingenic,jz4740-ohci
+ - reg: Should contain the address size of the OHCI controller registers.
+ - interrupt-parent: Should be the phandle of the interrupt controller that
+ delivers interrupts to the OHCI block.
+ - interrupts: Should specify the interrupt provided by interrupt-parent.
+ - clocks: Should contain a single clock specifier for the SoC UHC clock.
+
+Example SoC include file for jz4780:
+
+/ {
+ ohci: jz4780-ohci@0x134a {
+ compatible = ingenic,jz4780-ohci;
+ reg = 0x134a 0x1;
+
+ interrupt-parent = intc;
+ interrupts = 5;
+
+ clocks = cgu JZ4780_CLK_UHC;
+ clock-names = uhc;
+ };
+};
+
--
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] chipidea: pci: register nop PHY
Since PHY for ChipIdea is optional (not all SoCs having PHY for ChipIdea should
be programmed), we register 'nop' PHY for platforms that do not have
programmable PHY.
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
Since v1:
- address Felipe's comment regarding the commit message
- address Peter's comment regarding an empty line
drivers/usb/chipidea/ci_hdrc_pci.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c
b/drivers/usb/chipidea/ci_hdrc_pci.c
index 241ae34..f997eda 100644
--- a/drivers/usb/chipidea/ci_hdrc_pci.c
+++ b/drivers/usb/chipidea/ci_hdrc_pci.c
@@ -16,10 +16,16 @@
#include linux/interrupt.h
#include linux/usb/gadget.h
#include linux/usb/chipidea.h
+#include linux/usb/usb_phy_generic.h
/* driver name */
#define UDC_DRIVER_NAME ci_hdrc_pci
+struct ci_hdrc_pci {
+ struct platform_device *ci;
+ struct platform_device *phy;
+};
+
/**
* PCI block
*/
@@ -52,7 +58,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
struct ci_hdrc_platform_data *platdata = (void *)id-driver_data;
- struct platform_device *plat_ci;
+ struct ci_hdrc_pci *ci;
struct resource res[3];
int retval = 0, nres = 2;
@@ -61,6 +67,10 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
return -ENODEV;
}
+ ci = devm_kzalloc(pdev-dev, sizeof(*ci), GFP_KERNEL);
+ if (!ci)
+ return -ENOMEM;
+
retval = pcim_enable_device(pdev);
if (retval)
return retval;
@@ -73,6 +83,11 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
pci_set_master(pdev);
pci_try_set_mwi(pdev);
+ /* register a nop PHY */
+ ci-phy = usb_phy_generic_register();
+ if (!ci-phy)
+ return -ENOMEM;
+
memset(res, 0, sizeof(res));
res[0].start= pci_resource_start(pdev, 0);
res[0].end = pci_resource_end(pdev, 0);
@@ -80,13 +95,14 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
res[1].start= pdev-irq;
res[1].flags= IORESOURCE_IRQ;
- plat_ci = ci_hdrc_add_device(pdev-dev, res, nres, platdata);
- if (IS_ERR(plat_ci)) {
+ ci-ci = ci_hdrc_add_device(pdev-dev, res, nres, platdata);
+ if (IS_ERR(ci-ci)) {
dev_err(pdev-dev, ci_hdrc_add_device failed!\n);
- return PTR_ERR(plat_ci);
+ usb_phy_generic_unregister(ci-phy);
+ return PTR_ERR(ci-ci);
}
- pci_set_drvdata(pdev, plat_ci);
+ pci_set_drvdata(pdev, ci);
return 0;
}
@@ -101,9 +117,10 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
*/
static void ci_hdrc_pci_remove(struct pci_dev *pdev)
{
- struct platform_device *plat_ci = pci_get_drvdata(pdev);
+ struct ci_hdrc_pci *ci = pci_get_drvdata(pdev);
- ci_hdrc_remove_device(plat_ci);
+ ci_hdrc_remove_device(ci-ci);
+ usb_phy_generic_unregister(ci-phy);
}
/**
--
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
[PATCH 3/3] usb: ohci: jz4740: prepare the clock before enabling it
From: Paul Burton paul.bur...@imgtec.com The clock must have been prepared before enabling it. Signed-off-by: Paul Burton paul.bur...@imgtec.com --- drivers/usb/host/ohci-jz4740.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ohci-jz4740.c b/drivers/usb/host/ohci-jz4740.c index bb69733..1455a8b 100644 --- a/drivers/usb/host/ohci-jz4740.c +++ b/drivers/usb/host/ohci-jz4740.c @@ -189,7 +189,7 @@ static int jz4740_ohci_probe(struct platform_device *pdev) clk_set_rate(jz4740_ohci-clk, 4800); - clk_enable(jz4740_ohci-clk); + clk_prepare_enable(jz4740_ohci-clk); if (jz4740_ohci-vbus) ohci_jz4740_set_vbus_power(jz4740_ohci, true); -- 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 0/3] usb: ohci: jz4740: Add DT support and a fix
Hi, Here are a few simple patches for the jz4740. First adds a simple DT binding. Seconds adds DT support. Third is a minor fix in clock enabling. Patches are based on 3.19-rc6. Quite disjoint and stay within jz4740 so should apply easily on other trees. If you would like to have them rebased to a different tree, please tell. Thank-you ZubairLK Paul Burton (3): dt: usb: jz4740: Add DT binding document for OHCI usb: ohci: jz4740: add DT support usb: ohci: jz4740: prepare the clock before enabling it .../bindings/usb/ingenic,jz47xx-ohci.txt | 28 ++ drivers/usb/host/ohci-jz4740.c | 11 - 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.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 2/3] usb: ohci: jz4740: add DT support
From: Paul Burton paul.bur...@imgtec.com
This is a simple matter of providing a match table, the probe code needs
no modification.
Signed-off-by: Paul Burton paul.bur...@imgtec.com
---
drivers/usb/host/ohci-jz4740.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/host/ohci-jz4740.c b/drivers/usb/host/ohci-jz4740.c
index 8ddd8f5..bb69733 100644
--- a/drivers/usb/host/ohci-jz4740.c
+++ b/drivers/usb/host/ohci-jz4740.c
@@ -234,11 +234,20 @@ static int jz4740_ohci_remove(struct platform_device
*pdev)
return 0;
}
+#ifdef CONFIG_OF
+static struct of_device_id jz4740_ohci_of_match[] = {
+ { .compatible = ingenic,jz4740-ohci, },
+ { },
+};
+MODULE_DEVICE_TABLE(of, jz4740_ohci_of_match);
+#endif
+
static struct platform_driver ohci_hcd_jz4740_driver = {
.probe = jz4740_ohci_probe,
.remove = jz4740_ohci_remove,
.driver = {
.name = jz4740-ohci,
+ .of_match_table = of_match_ptr(jz4740_ohci_of_match),
.owner = THIS_MODULE,
},
};
--
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/3] usb: ohci: jz4740: Add DT support and a fix
Hi, Here are a few simple patches for the jz4740. First adds a simple DT binding. Seconds adds DT support. Third is a minor fix in clock enabling. Patches are based on 3.19-rc6. Quite disjoint and stay within jz4740 so should apply easily on other trees. If you would like to have them rebased to a different tree, please tell. Thank-you V2 changes Removed an interrupt parent binding. Forgot a binding Unprepared clock when disabling ZubairLK Paul Burton (3): dt: usb: jz4740: Add DT binding document for OHCI usb: ohci: jz4740: add DT support usb: ohci: jz4740: prepare the clock before enabling it .../bindings/usb/ingenic,jz47xx-ohci.txt | 26 ++ drivers/usb/host/ohci-jz4740.c | 15 ++--- 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.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/3] usb: ohci: jz4740: add DT support
From: Paul Burton paul.bur...@imgtec.com
This is a simple matter of providing a match table, the probe code needs
no modification.
Signed-off-by: Paul Burton paul.bur...@imgtec.com
---
drivers/usb/host/ohci-jz4740.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/host/ohci-jz4740.c b/drivers/usb/host/ohci-jz4740.c
index 8ddd8f5..bb69733 100644
--- a/drivers/usb/host/ohci-jz4740.c
+++ b/drivers/usb/host/ohci-jz4740.c
@@ -234,11 +234,20 @@ static int jz4740_ohci_remove(struct platform_device
*pdev)
return 0;
}
+#ifdef CONFIG_OF
+static struct of_device_id jz4740_ohci_of_match[] = {
+ { .compatible = ingenic,jz4740-ohci, },
+ { },
+};
+MODULE_DEVICE_TABLE(of, jz4740_ohci_of_match);
+#endif
+
static struct platform_driver ohci_hcd_jz4740_driver = {
.probe = jz4740_ohci_probe,
.remove = jz4740_ohci_remove,
.driver = {
.name = jz4740-ohci,
+ .of_match_table = of_match_ptr(jz4740_ohci_of_match),
.owner = THIS_MODULE,
},
};
--
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/3] dt: usb: jz4740: Add DT binding document for OHCI
From: Paul Burton paul.bur...@imgtec.com
Add the binding documentation for the JZ47xx OHCI controller.
Signed-off-by: Paul Burton paul.bur...@imgtec.com
Signed-off-by: Zubair Lutfullah Kakakhel zubair.kakak...@imgtec.com
---
The jz4740 is platform only at the moment.
But DT support is being added
See http://patchwork.linux-mips.org/bundle/paulburton/ci20-v3.20/
jz47xx is used because jz4780 will also use this driver
V2 Changes: Removed interrupt parent binding as that can be inherited.
Forgot a binding for clock-names
---
.../bindings/usb/ingenic,jz47xx-ohci.txt | 26 ++
1 file changed, 26 insertions(+)
create mode 100644
Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
diff --git a/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
b/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
new file mode 100644
index 000..2e5dce5
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
@@ -0,0 +1,26 @@
+Ingenic JZ47XX SoC OHCI controller binding
+
+The Ingenic JZ47XX SoC includes an OHCI compliant USB host controller
+interface for use with USB 1.1 devices.
+
+Required properties:
+ - compatible: Should be ingenic,jz4740-ohci
+ - reg: Should contain the address size of the OHCI controller registers.
+ - interrupts: Should specify the interrupt line number
+ - clocks: Should contain a single clock specifier for the SoC UHC clock.
+ - clock-names: Must be uhc
+
+Example for jz4780:
+
+/ {
+ ohci: jz4780-ohci@0x134a {
+ compatible = ingenic,jz4780-ohci;
+ reg = 0x134a 0x1;
+
+ interrupts = 5;
+
+ clocks = cgu JZ4780_CLK_UHC;
+ clock-names = uhc;
+ };
+};
+
--
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 3/3] usb: ohci: jz4740: prepare the clock before enabling it
From: Paul Burton paul.bur...@imgtec.com The clock must have been prepared before enabling it. Signed-off-by: Paul Burton paul.bur...@imgtec.com -- V2 changes. Add disable_unprepare as well --- drivers/usb/host/ohci-jz4740.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/ohci-jz4740.c b/drivers/usb/host/ohci-jz4740.c index bb69733..129f6b9 100644 --- a/drivers/usb/host/ohci-jz4740.c +++ b/drivers/usb/host/ohci-jz4740.c @@ -189,7 +189,7 @@ static int jz4740_ohci_probe(struct platform_device *pdev) clk_set_rate(jz4740_ohci-clk, 4800); - clk_enable(jz4740_ohci-clk); + clk_prepare_enable(jz4740_ohci-clk); if (jz4740_ohci-vbus) ohci_jz4740_set_vbus_power(jz4740_ohci, true); @@ -209,7 +209,7 @@ static int jz4740_ohci_probe(struct platform_device *pdev) err_disable: if (jz4740_ohci-vbus) regulator_disable(jz4740_ohci-vbus); - clk_disable(jz4740_ohci-clk); + clk_disable_unprepare(jz4740_ohci-clk); err_free: usb_put_hcd(hcd); @@ -227,7 +227,7 @@ static int jz4740_ohci_remove(struct platform_device *pdev) if (jz4740_ohci-vbus) regulator_disable(jz4740_ohci-vbus); - clk_disable(jz4740_ohci-clk); + clk_disable_unprepare(jz4740_ohci-clk); usb_put_hcd(hcd); -- 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 4/4] usb: dwc3: remove reliance on dev_vdbg()
By moving all dev_vdbg() to tracepoints, we
can finally get rid of dev_vdbg() usage from
dwc3.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/Kconfig | 6 -
drivers/usb/dwc3/Makefile | 1 -
drivers/usb/dwc3/core.c | 2 +-
drivers/usb/dwc3/gadget.c | 65 +++
4 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 58b5b2cde4c5..edbf9c85af7e 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -104,12 +104,6 @@ config USB_DWC3_DEBUG
help
Say Y here to enable debugging messages on DWC3 Driver.
-config USB_DWC3_VERBOSE
- bool Enable Verbose Debugging Messages
- depends on USB_DWC3_DEBUG
- help
- Say Y here to enable verbose debugging messages on DWC3 Driver.
-
config DWC3_HOST_USB3_LPM_ENABLE
bool Enable USB3 LPM Capability
depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index bb34fbcfeab3..46172f47f02d 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -2,7 +2,6 @@
CFLAGS_trace.o := -I$(src)
ccflags-$(CONFIG_USB_DWC3_DEBUG) := -DDEBUG
-ccflags-$(CONFIG_USB_DWC3_VERBOSE) += -DVERBOSE_DEBUG
obj-$(CONFIG_USB_DWC3) += dwc3.o
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 25ddc39efad8..9f0e209b8f6c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -345,7 +345,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
dwc-num_in_eps = DWC3_NUM_IN_EPS(parms);
dwc-num_out_eps = DWC3_NUM_EPS(parms) - dwc-num_in_eps;
- dev_vdbg(dwc-dev, found %d IN and %d OUT endpoints\n,
+ dwc3_trace(trace_dwc3_core, found %d IN and %d OUT endpoints,
dwc-num_in_eps, dwc-num_out_eps);
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c6eec261a304..7cce00e7102b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -139,7 +139,8 @@ int dwc3_gadget_set_link_state(struct dwc3 *dwc, enum
dwc3_link_state state)
udelay(5);
}
- dev_vdbg(dwc-dev, link state change request timed out\n);
+ dwc3_trace(trace_dwc3_gadget,
+ link state change request timed out);
return -ETIMEDOUT;
}
@@ -219,7 +220,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc)
fifo_size |= (last_fifo_depth 16);
- dev_vdbg(dwc-dev, %s: Fifo Addr %04x Size %d\n,
+ dwc3_trace(trace_dwc3_gadget, %s: Fifo Addr %04x Size %d,
dep-name, last_fifo_depth, fifo_size 0x);
dwc3_writel(dwc-regs, DWC3_GTXFIFOSIZ(num), fifo_size);
@@ -287,7 +288,8 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
unsigned cmd, u32 param)
do {
reg = dwc3_readl(dwc-regs, DWC3_DGCMD);
if (!(reg DWC3_DGCMD_CMDACT)) {
- dev_vdbg(dwc-dev, Command Complete -- %d\n,
+ dwc3_trace(trace_dwc3_gadget,
+ Command Complete -- %d,
DWC3_DGCMD_STATUS(reg));
return 0;
}
@@ -297,8 +299,11 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc,
unsigned cmd, u32 param)
* interrupt context.
*/
timeout--;
- if (!timeout)
+ if (!timeout) {
+ dwc3_trace(trace_dwc3_gadget,
+ Command Timed Out);
return -ETIMEDOUT;
+ }
udelay(1);
} while (1);
}
@@ -320,7 +325,8 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
do {
reg = dwc3_readl(dwc-regs, DWC3_DEPCMD(ep));
if (!(reg DWC3_DEPCMD_CMDACT)) {
- dev_vdbg(dwc-dev, Command Complete -- %d\n,
+ dwc3_trace(trace_dwc3_gadget,
+ Command Complete -- %d,
DWC3_DEPCMD_STATUS(reg));
return 0;
}
@@ -330,8 +336,11 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
* interrupt context.
*/
timeout--;
- if (!timeout)
+ if (!timeout) {
+ dwc3_trace(trace_dwc3_gadget,
+ Command Timed Out);
return -ETIMEDOUT;
+ }
udelay(1);
} while (1);
@@ -489,7 +498,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
u32 reg;
int ret;
-
[PATCH 0/4] usb: dwc3: remove VERBOSE_DEBUG
Hi, these patches move all dev_vdbg() messages to tracepoints. Below there's a sample output with all dwc3 tracepoints enabled. David, I've Cced you because you raised a concern about removing dev_* from dwc3. IMHO, traditional printk() changes the behavior way too much for any real USB3 debugging to go on. With tracepoints, we can even trace all register accesses (as can be seen below) and the overhead is still pretty negligible, when compared to traditional printk. Note that if you're in a situation where the platform hangs due to an error, you can still call ftrace_dump() from the point of failure to still get trace buffer dumped so you can look at logs. When using trace, I generally use a rather large trace buffer (512KiB at least, but I've used over 20MiB buffers, for example) which gives a much better possibility of actually catching errors. # cat /sys/kernel/debug/tracing/trace # tracer: nop # # entries-in-buffer/entries-written: 961/424125 #P:1 # # _-= irqs-off # / _= need-resched #| / _---= hardirq/softirq #|| / _--= preempt-depth #||| / delay # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | irq/144-dwc3-227 [000] d... 114.059241: dwc3_writel: addr fa39c408 value 0100 idle-0 [000] d.h. 114.059387: dwc3_readl: addr fa39c40c value 0004 idle-0 [000] d.h. 114.059392: dwc3_readl: addr fa39c408 value 0100 idle-0 [000] d.h. 114.059396: dwc3_writel: addr fa39c408 value 8100 idle-0 [000] d.h. 114.059399: dwc3_readl: addr fa39c41c value idle-0 [000] d.h. 114.059402: dwc3_readl: addr fa39c42c value idle-0 [000] d.h. 114.059405: dwc3_readl: addr fa39c43c value irq/144-dwc3-227 [000] d... 114.059495: dwc3_event: event 4086 irq/144-dwc3-227 [000] d... 114.059501: dwc3_complete_trb: ep1in-bulk: trb f2e2 bph bpl ad2d8000 size ctrl 0810 irq/144-dwc3-227 [000] d... 114.059509: dwc3_gadget_giveback: ep1in-bulk: req ec8245c0 length 4096/4096 == 0 irq/144-dwc3-227 [000] d... 114.059527: dwc3_writel: addr fa39c40c value 0004 irq/144-dwc3-227 [000] d... 114.059532: dwc3_readl: addr fa39c408 value 8100 irq/144-dwc3-227 [000] d... 114.059535: dwc3_writel: addr fa39c408 value 0100 file-storage-226 [000] d... 114.059557: dwc3_ep_queue: ep1out-bulk: req ed1a7440 length 31/512 == 0 idle-0 [000] d.h. 114.059622: dwc3_readl: addr fa39c40c value 0004 idle-0 [000] d.h. 114.059630: dwc3_readl: addr fa39c408 value 0100 idle-0 [000] d.h. 114.059633: dwc3_writel: addr fa39c408 value 8100 idle-0 [000] d.h. 114.059637: dwc3_readl: addr fa39c41c value idle-0 [000] d.h. 114.059640: dwc3_readl: addr fa39c42c value idle-0 [000] d.h. 114.059643: dwc3_readl: addr fa39c43c value irq/144-dwc3-227 [000] d... 114.059699: dwc3_event: event c046 irq/144-dwc3-227 [000] d... 114.059703: dwc3_complete_trb: ep1in-bulk: trb f2e20010 bph bpl ad754000 size ctrl 0812 irq/144-dwc3-227 [000] d... 114.059709: dwc3_gadget_giveback: ep1in-bulk: req ed1a71c0 length 13/13 == 0 irq/144-dwc3-227 [000] d... 114.059726: dwc3_writel: addr fa39c40c value 0004 irq/144-dwc3-227 [000] d... 114.059730: dwc3_readl: addr fa39c408 value 8100 irq/144-dwc3-227 [000] d... 114.059733: dwc3_writel: addr fa39c408 value 0100 idle-0 [000] d.h. 114.060246: dwc3_readl: addr fa39c40c value 0004 idle-0 [000] d.h. 114.060259: dwc3_readl: addr fa39c408 value 0100 idle-0 [000] d.h. 114.060262: dwc3_writel: addr fa39c408 value 8100 idle-0 [000] d.h. 114.060265: dwc3_readl: addr fa39c41c value idle-0 [000] d.h. 114.060272: dwc3_readl: addr fa39c42c value idle-0 [000] d.h. 114.060275: dwc3_readl: addr fa39c43c value irq/144-dwc3-227 [000] d... 114.060314: dwc3_event: event 00c4 irq/144-dwc3-227 [000] d... 114.060318: dwc3_gadget: ep1out-bulk: reason Transfer Not Active irq/144-dwc3-227 [000] d... 114.060325: dwc3_gadget: ep1out-bulk: req ed1a7440 dma ad2d8000 length 512 last irq/144-dwc3-227 [000] d... 114.060333: dwc3_prepare_trb: ep1out-bulk: trb f2e02000 bph bpl ad2d8000 size 0200 ctrl 0813 irq/144-dwc3-227 [000] d... 114.060338: dwc3_gadget_ep_cmd: ep1out-bulk: cmd 'Start Transfer' [6] params bf087000 irq/144-dwc3-227 [000] d... 114.060341: dwc3_writel: addr
[PATCH 3/4] usb: dwc3: trace: add trace logs for core and gadget
Sometimes we want to just print a formatted string without passing any extra data. The following will be used for removing reliance on dev_vdbg() from dwc3. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/dwc3/trace.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h index 9fc20b33dd8e..9c10669ab91f 100644 --- a/drivers/usb/dwc3/trace.h +++ b/drivers/usb/dwc3/trace.h @@ -47,6 +47,16 @@ DEFINE_EVENT(dwc3_log_msg, dwc3_writel, TP_ARGS(vaf) ); +DEFINE_EVENT(dwc3_log_msg, dwc3_gadget, + TP_PROTO(struct va_format *vaf), + TP_ARGS(vaf) +); + +DEFINE_EVENT(dwc3_log_msg, dwc3_core, + TP_PROTO(struct va_format *vaf), + TP_ARGS(vaf) +); + DEFINE_EVENT(dwc3_log_msg, dwc3_ep0, TP_PROTO(struct va_format *vaf), TP_ARGS(vaf) -- 2.3.0-rc1 -- 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/4] usb: dwc3: gadget: WARN() in case of unknown IRQ
if an unknown IRQ event is triggered, that means the HW is really misbehaving. Instead of printing a debug message, let's WARN() so users report when that happens. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/dwc3/gadget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 944036d4c83d..c6eec261a304 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2518,7 +2518,7 @@ static void dwc3_gadget_interrupt(struct dwc3 *dwc, dev_vdbg(dwc-dev, Overflow\n); break; default: - dev_dbg(dwc-dev, UNKNOWN IRQ %d\n, event-type); + dev_WARN(dwc-dev, UNKNOWN IRQ %d\n, event-type); } } -- 2.3.0-rc1 -- 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: cdc-acm: check for descriptors with invalid length
On Fri, 2015-01-23 at 23:08 +0800, Adam Lee wrote:
Hi,
could you test and review this?
Regards
Oliver
From 1ddabb5731135fe43f67f3b4645445c2de06dada Mon Sep 17 00:00:00 2001
From: Oliver Neukum oneu...@suse.de
Date: Tue, 13 Jan 2015 16:55:52 +0100
Subject: [PATCH] cdc-acm: add sanity checks
Check the special CDC headers for a plausible minimum length.
Another big operating systems ignores such garbage.
Signed-off-by: Oliver Neukum oneu...@suse.de
---
drivers/usb/class/cdc-acm.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 546a17e8..3dd540d 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1091,6 +1091,7 @@ static int acm_probe(struct usb_interface *intf,
unsigned long quirks;
int num_rx_buf;
int i;
+ unsigned int elength = 0;
int combined_interfaces = 0;
struct device *tty_dev;
int rv = -ENOMEM;
@@ -1136,9 +1137,12 @@ static int acm_probe(struct usb_interface *intf,
dev_err(intf-dev, skipping garbage\n);
goto next_desc;
}
+ elength = buffer[0];
switch (buffer[2]) {
case USB_CDC_UNION_TYPE: /* we've found it */
+ if (elength sizeof(struct usb_cdc_union_desc))
+goto next_desc;
if (union_header) {
dev_err(intf-dev, More than one
union descriptor, skipping ...\n);
@@ -1147,29 +1151,36 @@ static int acm_probe(struct usb_interface *intf,
union_header = (struct usb_cdc_union_desc *)buffer;
break;
case USB_CDC_COUNTRY_TYPE: /* export through sysfs*/
+ if (elength sizeof(struct usb_cdc_country_functional_desc))
+goto next_desc;
cfd = (struct usb_cdc_country_functional_desc *)buffer;
break;
case USB_CDC_HEADER_TYPE: /* maybe check version */
break; /* for now we ignore it */
case USB_CDC_ACM_TYPE:
+ if (elength 4)
+goto next_desc;
ac_management_function = buffer[3];
break;
case USB_CDC_CALL_MANAGEMENT_TYPE:
+ if (elength 5)
+goto next_desc;
call_management_function = buffer[3];
call_interface_num = buffer[4];
break;
default:
- /* there are LOTS more CDC descriptors that
+ /*
+ * there are LOTS more CDC descriptors that
* could legitimately be found here.
*/
dev_dbg(intf-dev, Ignoring descriptor:
- type %02x, length %d\n,
- buffer[2], buffer[0]);
+ type %02x, length %ud\n,
+ buffer[2], elength);
break;
}
next_desc:
- buflen -= buffer[0];
- buffer += buffer[0];
+ buflen -= elength;
+ buffer += elength;
}
if (!union_header) {
--
1.8.4.5
Re: [PATCH] usb: phy-generic: Don't fail on missing gpio reset
On Tue, Jan 27, 2015 at 11:13:08AM -0800, Sören Brinkmann wrote:
On Tue, 2015-01-27 at 09:20AM -0600, Felipe Balbi wrote:
On Mon, Jan 26, 2015 at 05:45:29PM -0800, Soren Brinkmann wrote:
A reset through a GPIO is optional. Don't fail probing when it is
missing.
Reported-by: Andreas Färber afaer...@suse.de
Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
---
Hi Andreas,
does this do the trick?
Thanks,
Sören
drivers/usb/phy/phy-generic.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254241fb..a73d4c738f0b 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -241,10 +241,8 @@ int usb_phy_gen_create_phy(struct device *dev,
struct usb_phy_generic *nop,
if (err == -EPROBE_DEFER)
return -EPROBE_DEFER;
- if (err) {
- dev_err(dev, Error requesting RESET GPIO\n);
- return err;
- }
+ if (err)
+ nop-gpiod_reset = NULL;
there's a better patch to use gpiod_get_optional(), instead.
Great, apparently that wasn't in linux-next yesterday. I'll give it a
shot once it arrives there.
It's still under discussion ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: phy-generic: Don't fail on missing gpio reset
On Tue, 2015-01-27 at 09:20AM -0600, Felipe Balbi wrote:
On Mon, Jan 26, 2015 at 05:45:29PM -0800, Soren Brinkmann wrote:
A reset through a GPIO is optional. Don't fail probing when it is
missing.
Reported-by: Andreas Färber afaer...@suse.de
Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
---
Hi Andreas,
does this do the trick?
Thanks,
Sören
drivers/usb/phy/phy-generic.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254241fb..a73d4c738f0b 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -241,10 +241,8 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
if (err == -EPROBE_DEFER)
return -EPROBE_DEFER;
- if (err) {
- dev_err(dev, Error requesting RESET GPIO\n);
- return err;
- }
+ if (err)
+ nop-gpiod_reset = NULL;
there's a better patch to use gpiod_get_optional(), instead.
Great, apparently that wasn't in linux-next yesterday. I'll give it a
shot once it arrives there.
Sören
--
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 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 05:42:02PM -0200, Fabio Estevam wrote: On Tue, Jan 27, 2015 at 12:16 PM, Fabio Estevam feste...@gmail.com wrote: On Tue, Jan 27, 2015 at 11:37 AM, robert.jarz...@free.fr wrote: Is in the right node, ie. the node that has compatible == usb-nop-xceiv ? Yes, it is correct since this commit: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/imx51-babbage.dts?id=7a9f0604bd56936b2b18f49824e0e392dc7878c3 This is the patch I have applied: --- a/drivers/usb/phy/phy-generic.c +++ b/drivers/usb/phy/phy-generic.c @@ -218,11 +218,13 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_ clk_rate = 0; needs_vcc = of_property_read_bool(node, vcc-supply); - nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios); + nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); + if (!nop-gpiod_reset) wrong test, you should use IS_ERR(), NULL is a valid gpio descriptor. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: composite: Provide list of registered functions
Hi, On Tue, Jan 27, 2015 at 08:20:58PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Tuesday, January 27, 2015 7:45 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions Hi, On Tue, Jan 27, 2015 at 06:24:42PM +0100, Krzysztof Opasiak wrote: So I must have misunderstood something. I'm not sure if this is a good idea. Some userspace depends on assumption that sys/kernel/config directory is empty and it's default place for mounting configfs. and that's fine, they can certainly assume that. Once configfs is mounted, we will have a new /sys/kernel/config/usb-functions directory. Inside that directory we should have a file which contains your list of available functions :-) I don't see what the problem is with that :-s Please hold done I don't get it. Let's clarify. _ / Hello! I'm a programmer cow who can \ | explain ConfigFS-related questions much | better than Felipe Balbi. | What we need | | in this case is just for the our USB| | Gadget ConfigFS port to create the file | | when that is mounted. Just have the | | file come with /usb_gadget. Not ProcFS, | not SysFS, but with ConfigFS | and we're | | good to go. Let me know if I can| \ explain anything further/ - \ ^__^ \ (Oo)\___ (__)\ )\/\ ||w | || || Would you like to register a separate usb-functions subsystem in configfs only to expose there a list of available functions? As far as I know it's the only way of creating anything in configfs root. So you would get: $ ls /sys/kernel/config usb-gadget usb-functions Or it is just a typo and you would like to place a usb-functions attribute in usb-gadget directory? In this option we will get: $ ls /sys/kernel/config usb-gadget $ ls /sys/kernel/config/usb-gadget usb-function $ cat usb-function acm ecm ... __ / Hello again. This is exactly what I \ | have in mind. Thank you. Let's just | call it a more descriptive name | like | \ 'available_functions'/ -- \ ^__^ \ (Oo)\___ (__)\ )\/\ ||w | || || I have written this few times in previous emails but our new friend could miss those emails so once again esp. for beautiful cow from a rubber duck: IMPORTANT__ /\ | Adding a file to usb_gadget directory is an *ABI break*.| an ABI break would be removing an existing file, not adding a new one. \_ _ / // // ..---..// .' _`. __..' (o): `..__ ; `. / ; `..---...___ .' `~-. .-') . ' _.' : : \ ' + J `._ _.' `~--___...---~' mh Currently each valid file name is also suitable for gadget. There is no additional restrictions. No matter what name we will give to this new file this name could be used previously by someone in userspace. His program could be broken after kernel upgrade. the first thing a user of usb-gadget has to do, is create a directory, not a file. Moreover some broken userspace programs may relay on fact that usb-gadget directory contains only gadget dirs. And some other things there is a lot of possible breaks in userspace. if there are any users who rely on the fact that *only* directories exist under usb-gadget, that's wrong. Imagine is sysfs would be forbidden of adding new files and directories. That's inside, it's crazy-talk :-) -- balbi signature.asc Description: Digital signature
RE: [PATCH] usb: gadget: composite: Provide list of registered functions
-Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Tuesday, January 27, 2015 7:45 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions Hi, On Tue, Jan 27, 2015 at 06:24:42PM +0100, Krzysztof Opasiak wrote: So I must have misunderstood something. I'm not sure if this is a good idea. Some userspace depends on assumption that sys/kernel/config directory is empty and it's default place for mounting configfs. and that's fine, they can certainly assume that. Once configfs is mounted, we will have a new /sys/kernel/config/usb-functions directory. Inside that directory we should have a file which contains your list of available functions :-) I don't see what the problem is with that :-s Please hold done I don't get it. Let's clarify. _ / Hello! I'm a programmer cow who can \ | explain ConfigFS-related questions much | better than Felipe Balbi. | What we need | | in this case is just for the our USB| | Gadget ConfigFS port to create the file | | when that is mounted. Just have the | | file come with /usb_gadget. Not ProcFS, | not SysFS, but with ConfigFS | and we're | | good to go. Let me know if I can| \ explain anything further/ - \ ^__^ \ (Oo)\___ (__)\ )\/\ ||w | || || Would you like to register a separate usb-functions subsystem in configfs only to expose there a list of available functions? As far as I know it's the only way of creating anything in configfs root. So you would get: $ ls /sys/kernel/config usb-gadget usb-functions Or it is just a typo and you would like to place a usb-functions attribute in usb-gadget directory? In this option we will get: $ ls /sys/kernel/config usb-gadget $ ls /sys/kernel/config/usb-gadget usb-function $ cat usb-function acm ecm ... __ / Hello again. This is exactly what I \ | have in mind. Thank you. Let's just | call it a more descriptive name | like | \ 'available_functions'/ -- \ ^__^ \ (Oo)\___ (__)\ )\/\ ||w | || || I have written this few times in previous emails but our new friend could miss those emails so once again esp. for beautiful cow from a rubber duck: IMPORTANT__ /\ | Adding a file to usb_gadget directory is an *ABI break*.| \_ _ / // // ..---..// .' _`. __..' (o): `..__ ; `. / ; `..---...___ .' `~-. .-') . ' _.' : : \ ' + J `._ _.' `~--___...---~' mh Currently each valid file name is also suitable for gadget. There is no additional restrictions. No matter what name we will give to this new file this name could be used previously by someone in userspace. His program could be broken after kernel upgrade. Moreover some broken userspace programs may relay on fact that usb-gadget directory contains only gadget dirs. And some other things there is a lot of possible breaks in userspace. Best regards, -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics k.opas...@samsung.com -- 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/4] usb: dwc3: gadget: avoid variable shadowing
We already have both ret and dwc defined in this
same function.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/gadget.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 6c5e344822b9..944036d4c83d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1110,15 +1110,10 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
* handled.
*/
if (dep-stream_capable) {
- int ret;
-
ret = __dwc3_gadget_kick_transfer(dep, 0, true);
- if (ret ret != -EBUSY) {
- struct dwc3 *dwc = dep-dwc;
-
+ if (ret ret != -EBUSY)
dev_dbg(dwc-dev, %s: failed to kick transfers\n,
dep-name);
- }
}
return 0;
--
2.3.0-rc1
--
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: composite: Provide list of registered functions
Hi, On Tue, Jan 27, 2015 at 06:24:42PM +0100, Krzysztof Opasiak wrote: So I must have misunderstood something. I'm not sure if this is a good idea. Some userspace depends on assumption that sys/kernel/config directory is empty and it's default place for mounting configfs. and that's fine, they can certainly assume that. Once configfs is mounted, we will have a new /sys/kernel/config/usb-functions directory. Inside that directory we should have a file which contains your list of available functions :-) I don't see what the problem is with that :-s Please hold done I don't get it. Let's clarify. _ / Hello! I'm a programmer cow who can \ | explain ConfigFS-related questions much | | better than Felipe Balbi. What we need | | in this case is just for the our USB| | Gadget ConfigFS port to create the file | | when that is mounted. Just have the | | file come with /usb_gadget. Not ProcFS, | | not SysFS, but with ConfigFS and we're | | good to go. Let me know if I can| \ explain anything further/ - \ ^__^ \ (Oo)\___ (__)\ )\/\ ||w | || || Would you like to register a separate usb-functions subsystem in configfs only to expose there a list of available functions? As far as I know it's the only way of creating anything in configfs root. So you would get: $ ls /sys/kernel/config usb-gadget usb-functions Or it is just a typo and you would like to place a usb-functions attribute in usb-gadget directory? In this option we will get: $ ls /sys/kernel/config usb-gadget $ ls /sys/kernel/config/usb-gadget usb-function $ cat usb-function acm ecm ... __ / Hello again. This is exactly what I \ | have in mind. Thank you. Let's just | | call it a more descriptive name like | \ 'available_functions'/ -- \ ^__^ \ (Oo)\___ (__)\ )\/\ ||w | || || -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 12:16 PM, Fabio Estevam feste...@gmail.com wrote:
On Tue, Jan 27, 2015 at 11:37 AM, robert.jarz...@free.fr wrote:
Is in the right node, ie. the node that has compatible == usb-nop-xceiv ?
Yes, it is correct since this commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/imx51-babbage.dts?id=7a9f0604bd56936b2b18f49824e0e392dc7878c3
This is the patch I have applied:
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,11 +218,13 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ if (!nop-gpiod_reset)
+ pr_err(*** failed to retrieve reset-gpios\n);
err = PTR_ERR(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
-vbus-detect-gpio);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+vbus-detect);
err = PTR_ERR(nop-gpiod_vbus);
}
} else if (pdata) {
and I am getting:
[0.127270] usbcore: registered new device driver usb
[0.127564] *** failed to retrieve reset-gpios
[0.127677] usbphy:usbphy@0 supply vcc not found, using dummy regulator
[0.128234] usb_phy_generic usbphy:usbh1phy@0: Error requesting RESET GPIO
[0.128323] usb_phy_generic: probe of usbphy:usbh1phy@0 failed with
error -1643301316
--
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 8/8] phy: add driver for TI TUSB1210 ULPI PHY
On Mon, Jan 26, 2015 at 11:23:37AM -0800, David Cohen wrote:
On Mon, Jan 26, 2015 at 02:55:03PM +0200, Heikki Krogerus wrote:
On Sat, Jan 24, 2015 at 03:58:11PM -0800, David Cohen wrote:
+static int tusb1210_power_on(struct phy *phy)
+{
+ struct tusb1210 *tusb = phy_get_drvdata(phy);
+
+ gpiod_set_value_cansleep(tusb-gpio_reset, 1);
+ gpiod_set_value_cansleep(tusb-gpio_cs, 1);
+
+ /* Restore eye diagram optimisation value */
+ ulpi_write(tusb-ulpi, TUSB1210_VENDOR_SPECIFIC2,
+ tusb-eye_diagram_tuning);
After you power on phy, ulpi bus may not be available right away. In
intel case, phy power on happens during dwc3 power on. ULPI bus is not
available until OTG controller and phy are in sync.
In resume, you can't restore eye diagram from here.
I'm sorry but I don't think I understand? Where do we power on the phy
before dwc3 is powered on? Or is this a Baytrail-CR specific problem?
I can't see any problems with the hardware I have.
You can't see in single accesses. But you may see when running stability
tests overnight.
Anyway, look for dwc3_core_soft_reset() function:
- dwc3 goes to reset state
- phy is initialized (or at least gets ready to sync clocks)
- dwc3 goes out or reset state
And if you look at dwc3_probe, you'll notice that it calls
phy_power_on after that, and ulpi will most definitely be accessible
at that point.
I'm only using the init and exit hooks instead of just
power_on/power_off because I'm not sure which the controllers will
use. For example, now dwc3 calls phy_init() in it's resume routine and
not phy_power_on() like I would expect. I know the problem has been
pointed out by others, so I'm expecting we'll get guidelines for it
later. But before we do, I see no harm in having both init and
power_on hooks in this driver.
During tusb1210 phy init from dwc3, you shouldn't access ulpi bus.
We will end up executing one failed write followed by write that we
know will succeed. Ideally we didn't have to do the first one at all,
but I don't see any risk here.
+ gpio = devm_gpiod_get(ulpi-dev, cs);
+ if (!IS_ERR(gpio)) {
+ ret = gpiod_direction_output(gpio, 0);
+ if (ret)
+ return ret;
+ tusb-gpio_cs = gpio;
+ }
+
+ /* Store initial eye diagram optimisation value */
+ ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
It's unclear if ulpi is accessible at this point. You can't read it at
this point.
We wouldn't have reached this point if ulpi wasn't accessible.
Registering the ulpi interface would have already failed so no driver
would have been probed.
You have a chicken/egg problem here:
- dwc3 needs phy to complete soft reset during probe
- tusb1210 needs dwc3 soft reset completed to be accessible via ULPI
Can you share how tusb1210 is connected on the platform you're using as
test for this patch? I don't think this driver would work reliably with
this device:
http://liliputing.com/2014/11/trekstor-launches-first-android-tablet-based-intels-irda-reference-design.html
The only reason why that board doesn't work is because of very much
Baytrail-CR specific problems. These are are two issues, but the first
one is critical for getting it working. Both will be handled, but
separately from this set:
1) The firmware leaves the PHY in reset, forcing us to enable it
somehow in OS before accessing ulpi. Unless we can get a firmware fix
for that (it's starting to look like it's not going to happen; please
correct me if you know something else!), we need to add a quirk for
Baytrails (attached), which is probable still OK. But IMO this really
should be fixed in the firmware.
2) Since the gpio resources are given to the controller device in ACPI
tables and there isn't separate device object for the PHY at all, we
need to deliver the gpios somehow separately to the phy driver. There
is a thread where we are talking about how to do that:
https://lkml.org/lkml/2014/12/18/82
Thanks,
--
heikki
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 8d95056..53902ea 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -21,6 +21,7 @@
#include linux/slab.h
#include linux/pci.h
#include linux/platform_device.h
+#include linux/gpio/consumer.h
#include platform_data.h
@@ -35,6 +36,24 @@
static int dwc3_pci_quirks(struct pci_dev *pdev)
{
+ if (pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_BYT) {
+ struct gpio_desc *gpio;
+
+ gpio = gpiod_get_index(pdev-dev, reset, 0);
+ if (!IS_ERR(gpio)) {
+ gpiod_direction_output(gpio, 0);
+ gpiod_set_value_cansleep(gpio, 1);
+ gpiod_put(gpio);
+ }
+
Re: XHCI, brain-dead scanner, and microframe rounding
On Jan 22, 2015, at 7:23 AM, Mathias Nyman wrote: I was doing this on your ep_reset_halt_test branch, which has a lot of MATTU messages scrolling by, but I'm pretty sure that the microframe rounding message was not present when running with either of these changes. So that may be a red herring after all... I wrote a new hack to test, its in the ep_reset_halt_test branch (forced update). It re-configures the endpoint every time a usb device driver clears a halt to make the toggle and sequence stay in sync between xhci and the device. I'm coding in the dark here, the scanner I test on has always worked so I need your help in testing this. Code is in the same place, the ep_reset_halt_test branch: git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git ep_reset_halt_test A dmesg log with xhci debugging of a failing case with this hack would be appreciated echo -n 'module xhci_hcd =p' /sys/kernel/debug/dynamic_debug/control (Unless, ofcourse I blindly got it right at the first try and everthing works flawlessly:) ... which appears to have been the case, actually. I love it when code works the first time. :) Here's the dmesg log in case there's anything you need to know in there: http://spark.crystalorb.net/mikem/dmesg.log I plug in the scanner at about 425 seconds, and start the scan around 477. This is awesome! I'm not familiar with how long this sort of fix usually takes to show up in official kernels; when might that happen? I'd be interested to try and get it picked up by distribution kernel packages (if the patch applies cleanly) so I can start using them again. Thanks so much for working on this! Mike-- 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 1/3] usb: phy: generic: migrate to gpio_desc
Hi Robert, On Tue, Jan 27, 2015 at 3:36 AM, Robert Jarzmik robert.jarz...@free.fr wrote: Robert Jarzmik robert.jarz...@free.fr writes: I'm not convinced of the so many issues. So far I see the gpiod_get_optional() requirement, which is a one liner patch. Would you try the following patch to see if it fixes all your concerns please ? Thanks for your help. ---8--- From 7b34a3aa2a0717b53cd458611048f50edde53d0c Mon Sep 17 00:00:00 2001 From: Robert Jarzmik robert.jarz...@free.fr Date: Tue, 27 Jan 2015 06:27:03 +0100 Subject: [PATCH] usb: phy: generic: fix the gpios to be optional All the gpios, ie. reset-gpios and vbus-detect-gpio, should be optional and not prevent the driver from working. Fix the regression in the behavior introduced by commit usb: phy: generic: migrate to gpio_desc. Signed-off-by: Robert Jarzmik robert.jarz...@free.fr --- drivers/usb/phy/phy-generic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c index dd05254..c767bf0 100644 --- a/drivers/usb/phy/phy-generic.c +++ b/drivers/usb/phy/phy-generic.c @@ -218,10 +218,10 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop, clk_rate = 0; needs_vcc = of_property_read_bool(node, vcc-supply); - nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios); + nop-gpiod_reset = devm_gpiod_get_optional(dev, reset-gpios); According to Documentation/gpio/board.txt: GPIOs mappings are defined in the consumer device's node, in a property named function-gpios so this should be: nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); On imx51-babbage.dts we do pass 'reset-gpios', but even devm_gpiod_get_optional(dev, reset) returns NULL. The issue I see is that I am never able to retrieve the reset-gpios when using the gpiod API. Any ideas? Thanks -- 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 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 6:03 PM, Felipe Balbi ba...@ti.com wrote:
wrong test, you should use IS_ERR(), NULL is a valid gpio descriptor.
Ok, here we go:
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,11 +218,13 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ if (IS_ERR(nop-gpiod_reset))
+ pr_err(*** failed to retrieve reset-gpios\n);
err = PTR_ERR(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
-vbus-detect-gpio);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+vbus-detect);
err = PTR_ERR(nop-gpiod_vbus);
}
} else if (pdata) {
which leads to:
[0.126378] usbcore: registered new interface driver hub
[0.126617] usbcore: registered new device driver usb
[0.127025] usbphy:usbphy@0 supply vcc not found, using dummy regulator
[0.127588] usb_phy_generic usbphy:usbh1phy@0: Error requesting RESET GPIO
[0.127677] usb_phy_generic: probe of usbphy:usbh1phy@0 failed with
error -1643301316
--
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 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 10:40:39PM +0100, Robert Jarzmik wrote:
Felipe Balbi ba...@ti.com writes:
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 9a826ff31951..34231c31cd1a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -219,11 +219,11 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
needs_vcc = of_property_read_bool(node, vcc-supply);
nop-gpiod_reset = devm_gpiod_get_optional(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
nop-gpiod_vbus = devm_gpiod_get_optional(dev,
vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_vbus);
}
} else if (pdata) {
type = pdata-type;
Funny, we ended up on the same solution ... almost, I add the reset-gpios
into reset and vbus-detect-gpio into vbus-detect also. But it's a good
sign we've ended up on the same code, isn't it ;) ?
Do you want me to rebase on your next tree and submit ? Or do you want to
submit
the patch yourself ?
your patch is more complete :-) Please send your version.
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
Fabio Estevam feste...@gmail.com writes:
On Tue, Jan 27, 2015 at 12:16 PM, Fabio Estevam feste...@gmail.com wrote:
On Tue, Jan 27, 2015 at 11:37 AM, robert.jarz...@free.fr wrote:
Is in the right node, ie. the node that has compatible == usb-nop-xceiv ?
Yes, it is correct since this commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/imx51-babbage.dts?id=7a9f0604bd56936b2b18f49824e0e392dc7878c3
This is the patch I have applied:
...zip...
This is my device-tree blob:
usb2phy: gpio-vbus@13 {
compatible = usb-nop-xceiv;
vbus-detect-gpio = gpio 13 GPIO_ACTIVE_LOW;
wakeup;
};
And this is what I have with this latest patch :
[0.617927] usb_phy_generic pxabus:gpio-vbus@13: GPIO lookup for consumer
reset
[0.618177] usb_phy_generic pxabus:gpio-vbus@13: using device tree for GPIO
lookup
[0.618306] of_get_named_gpiod_flags: can't parse 'reset-gpios' property of
node '/pxabus/gpio-vbus@13[0]'
[0.618425] of_get_named_gpiod_flags: can't parse 'reset-gpio' property of
node '/pxabus/gpio-vbus@13[0]'
[0.618538] usb_phy_generic pxabus:gpio-vbus@13: using lookup tables for
GPIO lookup
[0.618643] usb_phy_generic pxabus:gpio-vbus@13: lookup for GPIO reset failed
[0.618743] usb_phy_generic pxabus:gpio-vbus@13: GPIO lookup for consumer
vbus-detect
[0.618842] usb_phy_generic pxabus:gpio-vbus@13: using device tree for GPIO
lookup
[0.618943] of_get_named_gpiod_flags: can't parse 'vbus-detect-gpios'
property of node '/pxabus/gpio-vbus@13[0]'
[0.619093] of_get_named_gpiod_flags: parsed 'vbus-detect-gpio' property of
node '/pxabus/gpio-vbus@13[0]' - status (0)
[0.619221] RJK2: nop-gpiod_vbus = c06bf6dc
So I'll submit that one in [1]. It has to work in your case too, it works in
mine ...
Cheers.
--
Robert
---8---
[1] Patch
From 4005a6abf519272267399ac4030a5671f7877ca4 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik robert.jarz...@free.fr
Date: Tue, 27 Jan 2015 06:27:03 +0100
Subject: [PATCH] usb: phy: generic: fix the gpios to be optional
All the gpios, ie. reset-gpios and vbus-detect-gpio, should be optional
and not prevent the driver from working. Fix the regression in the
behavior introduced by commit usb: phy: generic: migrate to gpio_desc.
Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
---
drivers/usb/phy/phy-generic.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254..012534a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,12 +218,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
-vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+vbus-detect);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_vbus);
}
} else if (pdata) {
type = pdata-type;
@@ -242,7 +242,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
if (err == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (err) {
- dev_err(dev, Error requesting RESET GPIO\n);
+ dev_err(dev, Error requesting RESET or VBUS GPIO\n);
return err;
}
--
2.1.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 v2 1/3] usb: phy: generic: migrate to gpio_desc
Hi,
On Tue, Jan 27, 2015 at 07:29:49PM -0200, Fabio Estevam wrote:
On Tue, Jan 27, 2015 at 7:20 PM, Robert Jarzmik robert.jarz...@free.fr
wrote:
From 4005a6abf519272267399ac4030a5671f7877ca4 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik robert.jarz...@free.fr
Date: Tue, 27 Jan 2015 06:27:03 +0100
Subject: [PATCH] usb: phy: generic: fix the gpios to be optional
All the gpios, ie. reset-gpios and vbus-detect-gpio, should be optional
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254..012534a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,12 +218,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
-
vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+vbus-detect);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_vbus);
With this patch applied I see a kernel hang:
[1.340497] ci_hdrc ci_hdrc.1: doesn't support gadget
[1.345673] ci_hdrc ci_hdrc.1: EHCI Host Controller
[1.350887] ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number
1
[1.372931] ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
[1.383491] hub 1-0:1.0: USB hub found
[1.387585] hub 1-0:1.0: 1 port detected
(hangs here)
sounds like a bug in chipidea now. There are no errors from the PHY,
which means that the PHY is code is behaving.
Unless, there's a bug with now NULL gpio descriptors are handled, of
course.
--
balbi
signature.asc
Description: Digital signature
[PATCH v3] usb: Retry port status check on resume to work around RH bugs
The EHCI controller on the RK3288 SoC is violating basic parts of the
USB spec and thereby unable to properly resume a suspended port. It does
not start SOF generation within 3ms of finishing resume signaling, so
the attached device will drop of the bus again. This is a particular
problem with runtime PM, where accessing the device will trigger a
resume that immediately makes it unavailable (and reenumerate with a new
handle).
Thankfully, the persist feature is generally able to work around stuff
like that. Unfortunately, it doesn't quite work in this particular case
because the controller will turn off the CurrentConnectStatus bit for an
instant while the device is reconnecting, which causes the kernel to
conclude that it permanently disappeared. This patch adds a tiny retry
mechanism to the core port resume code which will catch this case and
shouldn't have any notable impact on other controllers.
Signed-off-by: Julius Werner jwer...@chromium.org
---
drivers/usb/core/hub.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index aeb50bb..26bdd96 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2896,10 +2896,12 @@ static int port_is_suspended(struct usb_hub *hub,
unsigned portstatus)
*/
static int check_port_resume_type(struct usb_device *udev,
struct usb_hub *hub, int port1,
- int status, unsigned portchange, unsigned portstatus)
+ int status, u16 portchange, u16 portstatus)
{
struct usb_port *port_dev = hub-ports[port1 - 1];
+ int retries = 3;
+ retry:
/* Is a warm reset needed to recover the connection? */
if (status == 0 udev-reset_resume
hub_port_warm_reset_required(hub, port1, portstatus)) {
@@ -2907,10 +2909,17 @@ static int check_port_resume_type(struct usb_device
*udev,
}
/* Is the device still present? */
else if (status || port_is_suspended(hub, portstatus) ||
- !port_is_power_on(hub, portstatus) ||
- !(portstatus USB_PORT_STAT_CONNECTION)) {
+ !port_is_power_on(hub, portstatus)) {
if (status = 0)
status = -ENODEV;
+ } else if (!(portstatus USB_PORT_STAT_CONNECTION)) {
+ if (retries--) {
+ usleep_range(200, 300);
+ status = hub_port_status(hub, port1, portstatus,
+portchange);
+ goto retry;
+ }
+ status = -ENODEV;
}
/* Can't do a normal resume if the port isn't enabled,
--
2.1.2
--
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 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 7:20 PM, Robert Jarzmik robert.jarz...@free.fr wrote:
From 4005a6abf519272267399ac4030a5671f7877ca4 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik robert.jarz...@free.fr
Date: Tue, 27 Jan 2015 06:27:03 +0100
Subject: [PATCH] usb: phy: generic: fix the gpios to be optional
All the gpios, ie. reset-gpios and vbus-detect-gpio, should be optional
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254..012534a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,12 +218,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
-vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+vbus-detect);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_vbus);
With this patch applied I see a kernel hang:
[1.340497] ci_hdrc ci_hdrc.1: doesn't support gadget
[1.345673] ci_hdrc ci_hdrc.1: EHCI Host Controller
[1.350887] ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 1
[1.372931] ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
[1.383491] hub 1-0:1.0: USB hub found
[1.387585] hub 1-0:1.0: 1 port detected
(hangs here)
--
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 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 10:20:39PM +0100, Robert Jarzmik wrote:
Fabio Estevam feste...@gmail.com writes:
On Tue, Jan 27, 2015 at 12:16 PM, Fabio Estevam feste...@gmail.com wrote:
On Tue, Jan 27, 2015 at 11:37 AM, robert.jarz...@free.fr wrote:
Is in the right node, ie. the node that has compatible == usb-nop-xceiv
?
Yes, it is correct since this commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/imx51-babbage.dts?id=7a9f0604bd56936b2b18f49824e0e392dc7878c3
This is the patch I have applied:
...zip...
This is my device-tree blob:
usb2phy: gpio-vbus@13 {
compatible = usb-nop-xceiv;
vbus-detect-gpio = gpio 13 GPIO_ACTIVE_LOW;
wakeup;
};
And this is what I have with this latest patch :
[0.617927] usb_phy_generic pxabus:gpio-vbus@13: GPIO lookup for consumer
reset
[0.618177] usb_phy_generic pxabus:gpio-vbus@13: using device tree for
GPIO lookup
[0.618306] of_get_named_gpiod_flags: can't parse 'reset-gpios' property
of node '/pxabus/gpio-vbus@13[0]'
[0.618425] of_get_named_gpiod_flags: can't parse 'reset-gpio' property of
node '/pxabus/gpio-vbus@13[0]'
[0.618538] usb_phy_generic pxabus:gpio-vbus@13: using lookup tables for
GPIO lookup
[0.618643] usb_phy_generic pxabus:gpio-vbus@13: lookup for GPIO reset
failed
[0.618743] usb_phy_generic pxabus:gpio-vbus@13: GPIO lookup for consumer
vbus-detect
[0.618842] usb_phy_generic pxabus:gpio-vbus@13: using device tree for
GPIO lookup
[0.618943] of_get_named_gpiod_flags: can't parse 'vbus-detect-gpios'
property of node '/pxabus/gpio-vbus@13[0]'
[0.619093] of_get_named_gpiod_flags: parsed 'vbus-detect-gpio' property
of node '/pxabus/gpio-vbus@13[0]' - status (0)
[0.619221] RJK2: nop-gpiod_vbus = c06bf6dc
So I'll submit that one in [1]. It has to work in your case too, it works in
mine ...
Cheers.
--
Robert
---8---
[1] Patch
From 4005a6abf519272267399ac4030a5671f7877ca4 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik robert.jarz...@free.fr
Date: Tue, 27 Jan 2015 06:27:03 +0100
Subject: [PATCH] usb: phy: generic: fix the gpios to be optional
All the gpios, ie. reset-gpios and vbus-detect-gpio, should be optional
and not prevent the driver from working. Fix the regression in the
behavior introduced by commit usb: phy: generic: migrate to gpio_desc.
Signed-off-by: Robert Jarzmik robert.jarz...@free.fr
---
drivers/usb/phy/phy-generic.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254..012534a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,12 +218,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
- vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+ vbus-detect);
the get optional part is already in my testing/next. We only need the
PTR_ERR_OR_ZERO() part.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
Hi,
On Tue, Jan 27, 2015 at 07:17:57PM -0200, Fabio Estevam wrote:
On Tue, Jan 27, 2015 at 6:03 PM, Felipe Balbi ba...@ti.com wrote:
wrong test, you should use IS_ERR(), NULL is a valid gpio descriptor.
Ok, here we go:
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,11 +218,13 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_
clk_rate = 0;
needs_vcc = of_property_read_bool(node, vcc-supply);
- nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
+ nop-gpiod_reset = devm_gpiod_get_optional(dev, reset);
+ if (IS_ERR(nop-gpiod_reset))
+ pr_err(*** failed to retrieve reset-gpios\n);
err = PTR_ERR(nop-gpiod_reset);
if (!err) {
- nop-gpiod_vbus = devm_gpiod_get(dev,
-vbus-detect-gpio);
+ nop-gpiod_vbus = devm_gpiod_get_optional(dev,
+vbus-detect);
err = PTR_ERR(nop-gpiod_vbus);
}
} else if (pdata) {
which leads to:
[0.126378] usbcore: registered new interface driver hub
[0.126617] usbcore: registered new device driver usb
[0.127025] usbphy:usbphy@0 supply vcc not found, using dummy regulator
[0.127588] usb_phy_generic usbphy:usbh1phy@0: Error requesting RESET GPIO
[0.127677] usb_phy_generic: probe of usbphy:usbh1phy@0 failed with
error -1643301316
weird, that seems to be a valid pointer. Can you try below:
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 9a826ff31951..34231c31cd1a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -219,11 +219,11 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
needs_vcc = of_property_read_bool(node, vcc-supply);
nop-gpiod_reset = devm_gpiod_get_optional(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
nop-gpiod_vbus = devm_gpiod_get_optional(dev,
vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_vbus);
}
} else if (pdata) {
type = pdata-type;
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: phy: make GPIOs optional for the generic phy
On Fri, 2015-01-16 at 12:14PM +0100, Robert Jarzmik wrote: Paul Zimmerman paul.zimmer...@synopsys.com writes: From 47bd18e210fecf701d493c27884e13c69bc449ea Mon Sep 17 00:00:00 2001 From: Paul Zimmerman pa...@synopsys.com Date: Wed, 14 Jan 2015 18:15:58 -0800 Subject: [PATCH] usb: phy: make GPIOs optional for the generic phy The use of GPIOs should be optional for the generic phy, otherwise the Altera SOCFPGA platform at least is broken. Fixes breakage caused by a combination of e9f2cefb0cd usb: phy: generic: migrate to gpio_desc and 135b3c4304d usb: dwc2: platform: add generic PHY framework support. Signed-off-by: Paul Zimmerman pa...@synopsys.com Reviewed-by: Robert Jarzmik robert.jarz...@free.fr Tested-by: Soren Brinkmann soren.brinkm...@xilinx.com Zynq suffers of the same problem. Sören -- 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 1/3] usb: phy: generic: migrate to gpio_desc
Felipe Balbi ba...@ti.com writes:
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 9a826ff31951..34231c31cd1a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -219,11 +219,11 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
needs_vcc = of_property_read_bool(node, vcc-supply);
nop-gpiod_reset = devm_gpiod_get_optional(dev, reset-gpios);
- err = PTR_ERR(nop-gpiod_reset);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_reset);
if (!err) {
nop-gpiod_vbus = devm_gpiod_get_optional(dev,
vbus-detect-gpio);
- err = PTR_ERR(nop-gpiod_vbus);
+ err = PTR_ERR_OR_ZERO(nop-gpiod_vbus);
}
} else if (pdata) {
type = pdata-type;
Funny, we ended up on the same solution ... almost, I add the reset-gpios
into reset and vbus-detect-gpio into vbus-detect also. But it's a good
sign we've ended up on the same code, isn't it ;) ?
Do you want me to rebase on your next tree and submit ? Or do you want to submit
the patch yourself ?
Cheers.
--
Robert
--
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: [RFC PATCH 2/4] usb: chipidea: udc: add set_selfpowered gaget ops
On Tue, Jan 27, 2015 at 11:27:57AM -0500, Alan Stern wrote:
On Tue, 27 Jan 2015, Peter Chen wrote:
The gadget power property will be used at get_status request.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/udc.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 540db0e..45914ba 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -841,6 +841,7 @@ __acquires(hwep-lock)
if ((setup-bRequestType USB_RECIP_MASK) == USB_RECIP_DEVICE) {
/* Assume that device is bus powered for now. */
*(u16 *)req-buf = ci-remote_wakeup 1;
+ *(u16 *)req-buf |= ci-gadget.is_selfpowered 1;
} else if ((setup-bRequestType USB_RECIP_MASK) \
When you add this line, don't you want to remove the comment two lines
above?
Also, you can simplify the code slightly by writing it like this:
*(u16 *) req-buf = (ci-remote_wakeup 1) |
ci-gadget.is_selfpowered;
Note that is_selfpowered should not be bit-shifted;
USB_DEVICE_SELF_POWERED is 0.
Thanks, I will change.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
On Tue, Jan 27, 2015 at 11:37 AM, robert.jarz...@free.fr wrote:
Is in the right node, ie. the node that has compatible == usb-nop-xceiv ?
Yes, it is correct since this commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/imx51-babbage.dts?id=7a9f0604bd56936b2b18f49824e0e392dc7878c3
usbh1phy: usbh1phy@0 {
compatible = usb-nop-xceiv;
reg = 0;
clocks = clks IMX5_CLK_DUMMY;
clock-names = main_clk;
reset-gpios = gpio2 5 GPIO_ACTIVE_LOW;
};
--
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 6/8] usb: dwc3: add ULPI interface support
look at your patch again. In case DWC3_ULPI isn't enabled, this file won't be linked at all. You're using stubs, so taht's fine. In case it _is_ enabled, you're breaking out early if you can't register ulpi interface, meaning you're exitting probe() (which, in fact, seems wrong as I want to be able to run dwc3 with ULPI enabled on a platform that was configured with ULPI+UTMI, but uses UTMI PHY). I still think this patch won't work in all cases. OK. So in case of ULPI+UTMI we'll try the ulpi interface, and if it fails, fall back to UTMI. Or can we use some other method to determine to which interface the PHY is really attached to in that 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: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
- Mail original - De: Fabio Estevam feste...@gmail.com À: Robert Jarzmik robert.jarz...@free.fr Cc: Felipe Balbi ba...@ti.com, Linus Walleij linus.wall...@linaro.org, Philipp Zabel philipp.za...@gmail.com, USB list linux-usb@vger.kernel.org Envoyé: Mardi 27 Janvier 2015 10:40:35 Objet: Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc - nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios); + nop-gpiod_reset = devm_gpiod_get_optional(dev, reset-gpios); According to Documentation/gpio/board.txt: GPIOs mappings are defined in the consumer device's node, in a property named function-gpios so this should be: nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); Ah you're right, I see it now ... in of_find_gpio(), the match is done on %s-%s, where the first one is the string in devm_gpiod_get(), and the second one is either gpio or gpios ... I'll amend my patch just as you said, my bad. Cheers. -- Robert -- 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 1/3] usb: phy: generic: migrate to gpio_desc
Hi Robert, On Tue, Jan 27, 2015 at 8:35 AM, robert.jarz...@free.fr wrote: - Mail original - De: Fabio Estevam feste...@gmail.com À: Robert Jarzmik robert.jarz...@free.fr Cc: Felipe Balbi ba...@ti.com, Linus Walleij linus.wall...@linaro.org, Philipp Zabel philipp.za...@gmail.com, USB list linux-usb@vger.kernel.org Envoyé: Mardi 27 Janvier 2015 10:40:35 Objet: Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc - nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios); + nop-gpiod_reset = devm_gpiod_get_optional(dev, reset-gpios); According to Documentation/gpio/board.txt: GPIOs mappings are defined in the consumer device's node, in a property named function-gpios so this should be: nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); Ah you're right, I see it now ... in of_find_gpio(), the match is done on %s-%s, where the first one is the string in devm_gpiod_get(), and the second one is either gpio or gpios ... I'll amend my patch just as you said, my bad. but the problem I see is that even if I use: nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); The 'reset-gpio' pin cannot be retrieved. This is the point I don't understand currently. -- 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: XHCI, brain-dead scanner, and microframe rounding
(Unless, ofcourse I blindly got it right at the first try and everthing works flawlessly:) ... which appears to have been the case, actually. I love it when code works the first time. :) Here's the dmesg log in case there's anything you need to know in there: http://spark.crystalorb.net/mikem/dmesg.log I plug in the scanner at about 425 seconds, and start the scan around 477. This is awesome! I'm not familiar with how long this sort of fix usually takes to show up in official kernels; when might that happen? I'd be interested to try and get it picked up by distribution kernel packages (if the patch applies cleanly) so I can start using them again. Great, that's good news. The patch is affecting others than just scanner so I'm going to run some basic tests with it before sending it further. we're late for 3.19, but could make it as a early fix for 3.20-rc kernel, ending up in final 3.20. crystal ball says final 3.20 is out in mid April. http://phb-crystal-ball.org/ -Mathias -- 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/2] Fix force effect modifications for the Microsoft Sidewinder Force Feedback Pro 2 joystick
---
The FF2 driver (usbhid/hid-pidff.c) does not set the effect ID when
uploading an effect. The result is that the initial upload works but
subsequent uploads to modify effect parameters are all directed at the
last-created effect.
The targeted effect ID must be passed back to the device when effect
parameters are changed. This is done at the start of
pidff_set_condition_report, pidff_set_periodic_report etc. based on
the value of pidff-block_load[PID_EFFECT_
BLOCK_INDEX].value[0].
This value is only ever set during pidff_request_effect_upload.
The result is stored in pidff-pid_id[effect-id] at the end of
pid_upload_effect, for later use. However, if an effect is modified and
re-sent then this identifier is not being copied back from
pidff-pid_id[effect-id] before sending the command to the device. The
fix is to do this at the start of pidff_upload_effect.
Signed-off-by: Jim Keir jimk...@oracledbadirect.com
drivers/hid/usbhid/hid-pidff.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 0b531c6..1b3fa70 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -568,6 +568,12 @@ static int pidff_upload_effect(struct input_dev *dev,
struct ff_effect *effect,
int type_id;
int error;
+ pidff-block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
+ if (old effect) {
+ pidff-block_load[PID_EFFECT_BLOCK_INDEX].value[0] =
+ pidff-pid_id[effect-id];
+ }
+
switch (effect-type) {
case FF_CONSTANT:
if (!old) {
--
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
xhci_hcd: host died
Hi, I'm not sure if this is the right place to report this, and would be glad if somebody could point me to the right one if I should be wrong. Anyway, since the main error message comes from xhci_hcd: When resuming from suspend to RAM, sometimes (= about every 4. or 5. suspend) my external USB3 harddisks filesystem (XFS) gets corrupted with I/O block errors. This is what dmesg says: [] [ 274.103912] nouveau [ CLK][:01:00.0] --: core 500 MHz shader 1250 MHz memory 333 MHz [ 274.104561] nouveau [ DRM] resuming client object trees... [ 274.104923] nouveau [ DRM] resuming display... [ 274.155454] nouveau [ DRM] resuming console... [ 274.156877] xhci_hcd :02:00.0: Host took too long to start, waited 16000 microseconds. [ 274.156896] xhci_hcd :02:00.0: PCI post-resume error -19! [ 274.156897] xhci_hcd :02:00.0: HC died; cleaning up [ 274.156901] xhci_hcd :02:00.0: HC died; cleaning up [ 274.156907] dpm_run_callback(): pci_pm_resume+0x0/0xe0 returns -19 [ 274.156914] PM: Device :02:00.0 failed to resume async: error -19 [ 274.264025] PM: resume of devices complete after 274.046 msecs [ 274.296112] ata3: SATA link down (SStatus 0 SControl 300) [ 274.296156] ata1: SATA link down (SStatus 0 SControl 300) [ 274.296199] ata6: SATA link down (SStatus 0 SControl 300) [ 274.296264] ata4: SATA link down (SStatus 0 SControl 300) [ 274.309726] PM: Finishing wakeup. [ 274.309731] Restarting tasks ... [ 274.310277] usb 2-2: USB disconnect, device number 2 [ 274.311581] done. [ 274.450970] ata5: softreset failed (device not ready) [ 274.450976] ata5: applying PMP SRST workaround and retrying [ 274.450996] ata2: softreset failed (device not ready) [ 274.451002] ata2: applying PMP SRST workaround and retrying [ 274.602901] firewire_core :04:0e.0: rediscovered device fw0 [ 274.605896] ata5: SATA link up 3.0 Gbps (SStatus 123 SControl 300) [ 274.605925] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300) [ 274.607052] ata5.00: configured for UDMA/133 [ 274.607254] ata2.00: configured for UDMA/100 [ 274.611077] ata5.00: configured for UDMA/133 [ 274.611080] ata5: EH complete [ 274.725279] IPv6: ADDRCONF(NETDEV_UP): enp3s0: link is not ready [ 274.732125] r8169 :03:00.0 enp3s0: link down [ 276.448347] r8169 :03:00.0 enp3s0: link up [ 316.728970] XFS (dm-0): metadata I/O error: block 0x15d5ed50 (xfs_trans_read_buf_map) error 19 numblks 8 [ 322.530206] XFS (dm-0): metadata I/O error: block 0x15d5ed50 (xfs_trans_read_buf_map) error 19 numblks 8 [ 325.957568] XFS (dm-0): metadata I/O error: block 0x15dbf80 (xfs_trans_read_buf_map) error 19 numblks 16 [ 325.957574] XFS (dm-0): xfs_imap_to_bp: xfs_trans_read_buf() returned error -19. [ 329.603399] XFS (dm-0): metadata I/O error: block 0x2bb5dc30 (xfs_trans_read_buf_map) error 19 numblks 16 [ 329.603405] XFS (dm-0): xfs_imap_to_bp: xfs_trans_read_buf() returned error -19. The USB3 host controller is this one: 02:00.0 USB controller: Renesas Technology Corp. uPD720202 USB 3.0 Host Controller (rev 02) (prog-if 30 [XHCI]) Flags: bus master, fast devsel, latency 0, IRQ 17 Memory at fdafe000 (64-bit, non-prefetchable) [size=8K] Capabilities: [50] Power Management version 3 Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+ Capabilities: [90] MSI-X: Enable+ Count=8 Masked- Capabilities: [a0] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [150] Latency Tolerance Reporting Kernel driver in use: xhci_hcd Kernel modules: xhci_pci And this is the HDD: Bus 009 Device 002: ID 174c:5106 ASMedia Technology Inc. Transcend StoreJet 25M3 Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x174c ASMedia Technology Inc. idProduct 0x5106 Transcend StoreJet 25M3 bcdDevice0.01 iManufacturer 2 ASMedia iProduct3 AS2105 iSerial 1 WD-WX31CC193411 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 44 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType
Re: XHCI, brain-dead scanner, and microframe rounding
Mathias Nyman mathias.nyman@... writes: Great, that's good news. Indeed! The patch is affecting others than just scanner so I'm going to run some basic tests with it before sending it further. It might even address some usbstick issues, some people have voiced with xhci. we're late for 3.19, but could make it as a early fix for 3.20-rc kernel, ending up in final 3.20. What about a stable backport? Do you think, it's feasible, too much work, or too risky? I expect a considerable amount of people to suffer from this. Pete -- 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: [RFC PATCH 0/4] usb: gadget: add is_selfpowered
On Tue, Jan 27, 2015 at 10:37:30AM +0800, Peter Chen wrote: Hi Felipe, Hi Felipe, Except for Alan's comments, do you agree with this change for common struct, if you agree with it, I will send formal patch with other udc driver's cleanup. Peter We can use a common is_selfpowered flag at usb_gadget to indicate whether the gadget is self-powered or bus-powered, it can avoid every udc driver defining a similar flag, and the user can read it through udc core sys entry. In the 1st patch, I introduce is_selfpowered for usb_gadget. In the 2nd patch, I take chipidea driver as an example to show how udc driver uses this flag. In the 3rd patch, I clean up at91 udc driver for this flag. In the 4th patch, I add this entry to sys through udc core. If you agree with this change, I will submit a formal patch set after cleaning up other udc drivers. Peter Chen (4): usb: gadget: introduce is_selfpowered for usb_gadget usb: chipidea: udc: add set_selfpowered gaget ops usb: gadget: at91_udc: use common is_selfpowered usb: udc-core: add is_selfpowered sys entry drivers/usb/chipidea/udc.c| 14 ++ drivers/usb/gadget/udc/at91_udc.c | 8 drivers/usb/gadget/udc/at91_udc.h | 1 - drivers/usb/gadget/udc/udc-core.c | 2 ++ include/linux/usb/gadget.h| 2 ++ 5 files changed, 22 insertions(+), 5 deletions(-) -- 1.9.1 -- 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 v4 3/3] usb: udc: add usb_udc_activation_handler
During this API, the deactivation count will be update, and it
will try to connect or disconnect gadget. It can be used to
enable functions for gadget.
Signed-off-by: Peter Chen peter.c...@freescale.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/gadget/udc/udc-core.c | 28 +++-
include/linux/usb/gadget.h| 5 +
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 9396a86..86d4d19 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -37,6 +37,7 @@
* @list - for use by the udc class driver
* @vbus - for udcs who care about vbus status, this value is real vbus status;
* for udcs who do not care about vbus status, this value is always true
+ * @deactivations - the deactivation count to connect or disconnect gadget
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -47,6 +48,7 @@ struct usb_udc {
struct device dev;
struct list_headlist;
boolvbus;
+ int deactivations;
};
static struct class *udc_class;
@@ -168,13 +170,37 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
static void usb_udc_connect_control(struct usb_udc *udc)
{
- if (udc-vbus)
+ if (udc-vbus !udc-deactivations)
usb_gadget_connect(udc-gadget);
else
usb_gadget_disconnect(udc-gadget);
}
/**
+ * usb_udc_activation_handler - updates udc's deactivation count and
+ * try to connect or disconnect
+ *
+ * @gadget: The gadget which the function is at
+ * @active: the function needs to be active or not
+ *
+ * The composite core calls it when it wants to activate or deactivate
+ * function.
+ */
+void usb_udc_activation_handler(struct usb_gadget *gadget, bool active)
+{
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
+
+ if (udc) {
+ if (active)
+ udc-deactivations--;
+ else
+ udc-deactivations++;
+ usb_udc_connect_control(udc);
+ }
+}
+EXPORT_SYMBOL_GPL(usb_udc_activation_handler);
+
+/**
* usb_udc_vbus_handler - updates the udc core vbus status, and try to
* connect or disconnect gadget
* @gadget: The gadget which vbus change occurs
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 2be007a..4d1adea 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1034,6 +1034,11 @@ extern void usb_udc_vbus_handler(struct usb_gadget
*gadget, bool status);
/*-*/
+/* utility to activate or deactive function */
+extern void usb_udc_activation_handler(struct usb_gadget *gadget, bool active);
+
+/*-*/
+
/* utility wrapping a simple endpoint selection policy */
extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
--
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 v4 0/3] usb: udc: Unify dp control
Hi Felipe, This is the follow-up: http://marc.info/?l=linux-usbm=140979297227434w=2. Sorry for late. In the first patch, we introduce an internal APIs which are used to find corresponding udc according to usb_gadget, it can simplify the code structure. In the 2nd patch, it maintains flag 'vbus' as vbus status, and try to connect or disconnect gadget according to vbus status. In the 3rd patch, it maintains count 'deactivations' for connect or disconnect gadget according to function/user behavior. If you agree with handing dp like above way, I will have two patchset, one for udc driver for 'vbus' and another for function driver for 'deactivations'. Thank you. Changes for v4: - Add Alan's Ack Changes for v3: - Add mutex_lock protect for list_del(udc-list) at usb_del_gadget_udc [Patch 1/3] - Drop former [Patch 2/4] which is useless Changes for v2: - introduce two internal APIs to simplify the code structure. - Teach patch 3/4 and 4/4 to use the new introduced API. Peter Chen (3): usb: udc: add usb_gadget_find_udc usb: udc: add usb_udc_vbus_handler usb: udc: add usb_udc_activation_handler drivers/usb/gadget/udc/udc-core.c | 111 +- include/linux/usb/gadget.h| 9 2 files changed, 94 insertions(+), 26 deletions(-) -- 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 v4 2/3] usb: udc: add usb_udc_vbus_handler
This commit updates udc core vbus status, and try to connect
or disconnect gadget.
Signed-off-by: Peter Chen peter.c...@freescale.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/gadget/udc/udc-core.c | 34 +-
include/linux/usb/gadget.h| 4
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 36c58c9..9396a86 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -35,6 +35,8 @@
* @dev - the child device to the actual controller
* @gadget - the gadget. For use by the class code
* @list - for use by the udc class driver
+ * @vbus - for udcs who care about vbus status, this value is real vbus status;
+ * for udcs who do not care about vbus status, this value is always true
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -44,6 +46,7 @@ struct usb_udc {
struct usb_gadget *gadget;
struct device dev;
struct list_headlist;
+ boolvbus;
};
static struct class *udc_class;
@@ -163,6 +166,34 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
/* - */
+static void usb_udc_connect_control(struct usb_udc *udc)
+{
+ if (udc-vbus)
+ usb_gadget_connect(udc-gadget);
+ else
+ usb_gadget_disconnect(udc-gadget);
+}
+
+/**
+ * usb_udc_vbus_handler - updates the udc core vbus status, and try to
+ * connect or disconnect gadget
+ * @gadget: The gadget which vbus change occurs
+ * @status: The vbus status
+ *
+ * The udc driver calls it when it wants to connect or disconnect gadget
+ * according to vbus status.
+ */
+void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
+{
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
+
+ if (udc) {
+ udc-vbus = status;
+ usb_udc_connect_control(udc);
+ }
+}
+EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
+
/**
* usb_gadget_udc_reset - notifies the udc core that bus reset occurs
* @gadget: The gadget which bus reset occurs
@@ -295,6 +326,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err4;
usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
+ udc-vbus = true;
mutex_unlock(udc_lock);
@@ -398,7 +430,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct
usb_gadget_driver *dri
driver-unbind(udc-gadget);
goto err1;
}
- usb_gadget_connect(udc-gadget);
+ usb_udc_connect_control(udc);
kobject_uevent(udc-dev.kobj, KOBJ_CHANGE);
return 0;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 70ddb39..2be007a 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1027,6 +1027,10 @@ extern void usb_gadget_udc_reset(struct usb_gadget
*gadget,
extern void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req);
+/*-*/
+
+/* utility to update vbus status for udc core, it may be scheduled */
+extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
/*-*/
--
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 v4 1/3] usb: udc: add usb_gadget_find_udc
This is an internal API, and is used to find corresponding udc according
to gadget.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/udc/udc-core.c | 51 ---
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index e31d574..36c58c9 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -52,6 +52,25 @@ static DEFINE_MUTEX(udc_lock);
/* - */
+static struct usb_udc *usb_gadget_find_udc(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc = NULL;
+
+ mutex_lock(udc_lock);
+ list_for_each_entry(udc, udc_list, list)
+ if (udc-gadget == gadget)
+ goto found;
+ mutex_unlock(udc_lock);
+ dev_err(gadget-dev.parent, gadget not registered.\n);
+
+ return NULL;
+
+found:
+ mutex_unlock(udc_lock);
+ return udc;
+}
+
+/* - */
#ifdef CONFIG_HAS_DMA
int usb_gadget_map_request(struct usb_gadget *gadget,
@@ -128,21 +147,10 @@ EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
static void usb_gadget_state_work(struct work_struct *work)
{
- struct usb_gadget *gadget = work_to_gadget(work);
- struct usb_udc *udc = NULL;
-
- mutex_lock(udc_lock);
- list_for_each_entry(udc, udc_list, list)
- if (udc-gadget == gadget)
- goto found;
- mutex_unlock(udc_lock);
-
- return;
-
-found:
- mutex_unlock(udc_lock);
+ struct usb_udc *udc = usb_gadget_find_udc(work_to_gadget(work));
- sysfs_notify(udc-dev.kobj, NULL, state);
+ if (udc)
+ sysfs_notify(udc-dev.kobj, NULL, state);
}
void usb_gadget_set_state(struct usb_gadget *gadget,
@@ -348,21 +356,14 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
*/
void usb_del_gadget_udc(struct usb_gadget *gadget)
{
- struct usb_udc *udc = NULL;
-
- mutex_lock(udc_lock);
- list_for_each_entry(udc, udc_list, list)
- if (udc-gadget == gadget)
- goto found;
-
- dev_err(gadget-dev.parent, gadget not registered.\n);
- mutex_unlock(udc_lock);
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
- return;
+ if (!udc)
+ return;
-found:
dev_vdbg(gadget-dev.parent, unregistering gadget\n);
+ mutex_lock(udc_lock);
list_del(udc-list);
mutex_unlock(udc_lock);
--
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: [RFC PATCH 0/4] usb: gadget: add is_selfpowered
Hi Felipe, Except for Alan's comments, do you agree with this change for common struct, if you agree with it, I will send formal patch with other udc driver's cleanup. looks good to me. Will you provide patches to all other UDC drivers ? it's certainly not mandatory, and I can help with both musb and dwc3 (probably a few others too), but it'll help me if you do. :-) I will do it for all udcs. Peter -- 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: [RFC PATCH 0/4] usb: gadget: add is_selfpowered
On Wed, Jan 28, 2015 at 04:32:59AM +, Peter Chen wrote: Hi Felipe, Except for Alan's comments, do you agree with this change for common struct, if you agree with it, I will send formal patch with other udc driver's cleanup. looks good to me. Will you provide patches to all other UDC drivers ? it's certainly not mandatory, and I can help with both musb and dwc3 (probably a few others too), but it'll help me if you do. :-) I will do it for all udcs. cool, thanks. I'll make sure to test on platforms I have around. cheers -- balbi signature.asc Description: Digital signature
[PATCH v3 4/5] usb: serial: remove unused function for F81232
remove unused function set_control_lines()
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/usb/serial/f81232.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 9a54f56..11a236b 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -323,13 +323,6 @@ static void f81232_process_read_urb(struct urb *urb)
tty_flip_buffer_push(port-port);
}
-
-}
-
-static int set_control_lines(struct usb_device *dev, u8 value)
-{
- /* FIXME - Stubbed out for now */
- return 0;
}
static void f81232_break_ctl(struct tty_struct *tty, int break_state)
--
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 v3 5/5] usb: serial: implement CMSPAR for F81232
This patch implement CMSPAR in set_termios,
and fix some warnning from checkpatch.pl
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/usb/serial/f81232.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 11a236b..79592d6 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -175,7 +175,7 @@ static void f81232_read_msr(struct f81232_private *priv)
static inline int update_mctrl(struct f81232_private *port_priv,
- unsigned int set, unsigned int clear)
+ unsigned int set, unsigned int clear)
{
struct usb_device *dev = port_priv-port-serial-dev;
u8 urb_value;
@@ -361,12 +361,21 @@ static void f81232_set_termios(struct tty_struct *tty,
if (cflag PARENB) {
- if (cflag PARODD)
- new_lcr |= UART_LCR_PARITY; /* odd */
- else
- new_lcr |= SERIAL_EVEN_PARITY; /* even */
+ if (cflag CMSPAR) {
+ if (cflag PARODD)
+ new_lcr |= (UART_LCR_PARITY | UART_LCR_SPAR);
+ else
+ new_lcr |= (SERIAL_EVEN_PARITY
+ | UART_LCR_SPAR);
+ } else {
+ if (cflag PARODD)
+ new_lcr |= UART_LCR_PARITY; /* odd */
+ else
+ new_lcr |= SERIAL_EVEN_PARITY; /* even */
+ }
}
+
if (cflag CSTOPB)
new_lcr |= UART_LCR_STOP;
else
@@ -445,7 +454,7 @@ static int f81232_open(struct tty_struct *tty, struct
usb_serial_port *port)
result = usb_submit_urb(port-interrupt_in_urb, GFP_KERNEL);
if (result) {
- dev_err(port-dev, failed submitting interrupt urb, error
%d\n,
+ dev_err(port-dev, failed submitting urb, error %d\n,
result);
return result;
}
--
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 v3 2/5] usb: serial: fix callback wrong process for F81232
Our int callback will return IIR, not LSR, and bulk-in callback will return
[LSR+Data][LSR+Data], so need to rewrite it
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/usb/serial/f81232.c | 82 +
1 file changed, 39 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index efd45a7..b372975 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -230,17 +230,33 @@ static inline int update_mctrl(struct f81232_private
*port_priv,
return status;
}
-static void f81232_update_line_status(struct usb_serial_port *port,
+
+static void f81232_iir_status(struct usb_serial_port *port,
unsigned char *data,
unsigned int actual_length)
{
- /*
-* FIXME: Update port-icount, and call
-*
-* wake_up_interruptible(port-port.delta_msr_wait);
-*
-*on MSR changes.
-*/
+ struct f81232_private *priv = usb_get_serial_port_data(port);
+ struct usb_device *dev = port-serial-dev;
+
+ if (!actual_length)
+ return;
+
+ switch (data[0] 0x07) {
+ case 0x00: /* msr change */
+ dev_dbg(dev-dev, IIR: MSR Change: %x\n, data[0]);
+ schedule_work(priv-int_worker);
+ break;
+
+ case 0x02: /* tx-empty */
+ break;
+
+ case 0x04: /* rx data available */
+ break;
+
+ case 0x06: /* lsr change */
+ dev_dbg(dev-dev, IIR: IIR: LSR Change: %x\n, data[0]);
+ break;
+ }
}
static void f81232_read_int_callback(struct urb *urb)
@@ -271,7 +287,7 @@ static void f81232_read_int_callback(struct urb *urb)
usb_serial_debug_data(port-dev, __func__,
urb-actual_length, urb-transfer_buffer);
- f81232_update_line_status(port, data, actual_length);
+ f81232_iir_status(port, data, actual_length);
exit:
retval = usb_submit_urb(urb, GFP_ATOMIC);
@@ -284,47 +300,30 @@ exit:
static void f81232_process_read_urb(struct urb *urb)
{
struct usb_serial_port *port = urb-context;
- struct f81232_private *priv = usb_get_serial_port_data(port);
unsigned char *data = urb-transfer_buffer;
char tty_flag = TTY_NORMAL;
- unsigned long flags;
- u8 line_status;
+ u8 line_status = 0;
int i;
- /* update line status */
- spin_lock_irqsave(priv-lock, flags);
- line_status = priv-line_status;
- priv-line_status = ~UART_STATE_TRANSIENT_MASK;
- spin_unlock_irqrestore(priv-lock, flags);
if (!urb-actual_length)
return;
- /* break takes precedence over parity, */
- /* which takes precedence over framing errors */
- if (line_status UART_BREAK_ERROR)
- tty_flag = TTY_BREAK;
- else if (line_status UART_PARITY_ERROR)
- tty_flag = TTY_PARITY;
- else if (line_status UART_FRAME_ERROR)
- tty_flag = TTY_FRAME;
- dev_dbg(port-dev, %s - tty_flag = %d\n, __func__, tty_flag);
-
- /* overrun is special, not associated with a char */
- if (line_status UART_OVERRUN_ERROR)
- tty_insert_flip_char(port-port, 0, TTY_OVERRUN);
-
- if (port-port.console port-sysrq) {
- for (i = 0; i urb-actual_length; ++i)
- if (!usb_serial_handle_sysrq_char(port, data[i]))
- tty_insert_flip_char(port-port, data[i],
- tty_flag);
- } else {
- tty_insert_flip_string_fixed_flag(port-port, data, tty_flag,
- urb-actual_length);
+ if (urb-actual_length = 2) {
+
+ for (i = 0 ; i urb-actual_length ; i += 2) {
+ line_status |= data[i+0];
+ tty_insert_flip_string_fixed_flag(port-port,
+ data[i+1], tty_flag, 1);
+ }
+
+ if (unlikely(line_status UART_OVERRUN_ERROR))
+ tty_insert_flip_char(port-port, 0, TTY_OVERRUN);
+
+ tty_flip_buffer_push(port-port);
}
- tty_flip_buffer_push(port-port);
+
}
static int set_control_lines(struct usb_device *dev, u8 value)
@@ -456,9 +455,6 @@ static int f81232_ioctl(struct tty_struct *tty,
return -ENOIOCTLCMD;
}
-
-
-
static void f81232_int_work_wq(struct work_struct *work)
{
struct f81232_private *priv =
--
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 v3 1/5] usb: serial: add register map for F81232
Add register map for F81232. and add some function to operating this device.
etc. f81232_get_register()/f81232_set_register() to work with USB control
point. and worker f81232_int_work_wq() to read MSR when IIR acquired.
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/usb/serial/f81232.c | 229 +---
1 file changed, 214 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index c5dc233..efd45a7 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -23,6 +23,8 @@
#include linux/uaccess.h
#include linux/usb.h
#include linux/usb/serial.h
+#include linux/serial_reg.h
+#include linux/version.h
static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x1934, 0x0706) },
@@ -37,19 +39,197 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define UART_STATE_TRANSIENT_MASK 0x74
#define UART_DCD 0x01
#define UART_DSR 0x02
-#define UART_BREAK_ERROR 0x04
#define UART_RING 0x08
-#define UART_FRAME_ERROR 0x10
-#define UART_PARITY_ERROR 0x20
-#define UART_OVERRUN_ERROR 0x40
#define UART_CTS 0x80
+#define UART_BREAK_ERROR 0x10
+#define UART_FRAME_ERROR 0x08
+#define UART_PARITY_ERROR 0x04
+#define UART_OVERRUN_ERROR 0x02
+#define SERIAL_EVEN_PARITY (UART_LCR_PARITY | UART_LCR_EPAR)
+
+#define REGISTER_REQUEST 0xA0
+#define GET_REGISTER 0xc0
+#define SET_REGISTER 0x40
+#define F81232_USB_TIMEOUT 1000
+#define F81232_USB_RETRY 20
+
+#define SERIAL_BASE_ADDRESS (0x0120)
+#define RECEIVE_BUFFER_REGISTER(0x00 + SERIAL_BASE_ADDRESS)
+#define TRANSMIT_HOLDING_REGISTER (0x00 + SERIAL_BASE_ADDRESS)
+#define INTERRUPT_ENABLE_REGISTER (0x01 + SERIAL_BASE_ADDRESS)
+#define INTERRUPT_IDENT_REGISTER (0x02 + SERIAL_BASE_ADDRESS)
+#define FIFO_CONTROL_REGISTER (0x02 + SERIAL_BASE_ADDRESS)
+#define LINE_CONTROL_REGISTER (0x03 + SERIAL_BASE_ADDRESS)
+#define MODEM_CONTROL_REGISTER (0x04 + SERIAL_BASE_ADDRESS)
+#define LINE_STATUS_REGISTER (0x05 + SERIAL_BASE_ADDRESS)
+#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
+
struct f81232_private {
spinlock_t lock;
u8 line_control;
u8 line_status;
+
+ struct work_struct int_worker;
+ struct usb_serial_port *port;
};
+static inline int calc_baud_divisor(u32 baudrate)
+{
+ u32 divisor, rem;
+
+ divisor = 115200L / baudrate;
+ rem = 115200L % baudrate;
+
+ /* Round to nearest divisor */
+ if (((rem * 2) = baudrate) (baudrate != 110))
+ divisor++;
+
+ return divisor;
+}
+
+
+static inline int f81232_get_register(struct usb_device *dev,
+ u16 reg, u8 *data)
+{
+ int status;
+ int i = F81232_USB_RETRY;
+
+ while (i--) {
+ status = usb_control_msg(dev,
+usb_rcvctrlpipe(dev, 0),
+REGISTER_REQUEST,
+GET_REGISTER,
+reg,
+0,
+data,
+sizeof(*data),
+F81232_USB_TIMEOUT);
+
+ if (status 0) {
+ dev_dbg(dev-dev,
+ f81232_get_register status: %d, fail:%d\n,
+ status, i);
+ } else
+ break;
+ }
+
+ return status;
+}
+
+
+static inline int f81232_set_register(struct usb_device *dev,
+ u16 reg, u8 data)
+{
+ int status;
+ int i = F81232_USB_RETRY;
+
+ while (i--) {
+ status = usb_control_msg(dev,
+usb_sndctrlpipe(dev, 0),
+REGISTER_REQUEST,
+SET_REGISTER,
+reg,
+0,
+data,
+1,
+F81232_USB_TIMEOUT);
+
+ if (status 0)
+ dev_dbg(dev-dev,
+ f81232_set_register status: %d, fail:%d\n,
+ status, i);
+ else
+ break;
+ }
+
+ return status;
+}
+
+static void f81232_read_msr(struct f81232_private *priv)
+{
+ unsigned long flags;
+ u8 current_msr, old_msr;
+ struct usb_device *dev = priv-port-serial-dev;
+
+ f81232_get_register(dev, MODEM_STATUS_REGISTER, current_msr);
+
+ spin_lock_irqsave(priv-lock, flags);
+ old_msr = priv-line_status;
+
[PATCH v3 3/5] usb: serial: implement function for F81232
This patch implement the following function:
set_termios, tiocmset, tiocmget, dtr_rts
Signed-off-by: Peter Hung hpeter+linux_ker...@gmail.com
---
drivers/usb/serial/f81232.c | 103
1 file changed, 84 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index b372975..9a54f56 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -346,27 +346,99 @@ static void f81232_break_ctl(struct tty_struct *tty, int
break_state)
static void f81232_set_termios(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios)
{
- /* FIXME - Stubbed out for now */
+ u16 divisor;
+ u16 new_lcr = 0;
+ int status;
+ struct ktermios *termios = tty-termios;
+ struct usb_device *dev = port-serial-dev;
+ unsigned int cflag = termios-c_cflag;
- /* Don't change anything if nothing has changed */
- if (old_termios !tty_termios_hw_change(tty-termios, old_termios))
- return;
+ divisor = calc_baud_divisor(tty_get_baud_rate(tty));
+
+ status = f81232_set_register(dev, LINE_CONTROL_REGISTER,
+UART_LCR_DLAB); /* DLAB */
+
+ status = f81232_set_register(dev, RECEIVE_BUFFER_REGISTER,
+divisor 0x00ff); /* low */
+
+ status = f81232_set_register(dev, INTERRUPT_ENABLE_REGISTER,
+(divisor 0xff00) 8); /* high */
+
+ status = f81232_set_register(dev, LINE_CONTROL_REGISTER, 0x00);
+
+
+ if (cflag PARENB) {
+ if (cflag PARODD)
+ new_lcr |= UART_LCR_PARITY; /* odd */
+ else
+ new_lcr |= SERIAL_EVEN_PARITY; /* even */
+ }
+
+ if (cflag CSTOPB)
+ new_lcr |= UART_LCR_STOP;
+ else
+ new_lcr = ~UART_LCR_STOP;
+
+ switch (cflag CSIZE) {
+ case CS5:
+ new_lcr |= UART_LCR_WLEN5;
+ break;
+ case CS6:
+ new_lcr |= UART_LCR_WLEN6;
+ break;
+ case CS7:
+ new_lcr |= UART_LCR_WLEN7;
+ break;
+ default:
+ case CS8:
+ new_lcr |= UART_LCR_WLEN8;
+ break;
+ }
+
+ status |= f81232_set_register(dev, LINE_CONTROL_REGISTER, new_lcr);
+
+ status |= f81232_set_register(dev, FIFO_CONTROL_REGISTER,
+ 0x87); /* fifo, trigger8 */
+ status |= f81232_set_register(dev,
+ INTERRUPT_ENABLE_REGISTER, 0xf); /* IER */
+
+ if (status 0)
+ dev_err(port-dev, LINE_CONTROL_REGISTER set error: %d\n,
+ status);
- /* Do the real work here... */
- if (old_termios)
- tty_termios_copy_hw(tty-termios, old_termios);
}
static int f81232_tiocmget(struct tty_struct *tty)
{
- /* FIXME - Stubbed out for now */
- return 0;
+ int r;
+ struct usb_serial_port *port = tty-driver_data;
+ struct f81232_private *port_priv = usb_get_serial_port_data(port);
+ unsigned long flags;
+ u8 mcr, msr;
+
+ spin_lock_irqsave(port_priv-lock, flags);
+ mcr = port_priv-line_control;
+ msr = port_priv-line_status;
+ spin_unlock_irqrestore(port_priv-lock, flags);
+
+ r = (mcr UART_MCR_DTR ? TIOCM_DTR : 0) |
+ (mcr UART_MCR_RTS ? TIOCM_RTS : 0) |
+ (msr UART_MSR_CTS ? TIOCM_CTS : 0) |
+ (msr UART_MSR_DCD ? TIOCM_CAR : 0) |
+ (msr UART_MSR_RI ? TIOCM_RI : 0) |
+ (msr UART_MSR_DSR ? TIOCM_DSR : 0);
+
+ return r;
}
static int f81232_tiocmset(struct tty_struct *tty,
unsigned int set, unsigned int clear)
{
- /* FIXME - Stubbed out for now */
+ struct usb_serial_port *port = tty-driver_data;
+ struct f81232_private *port_priv =
+ usb_get_serial_port_data(port);
+
+ update_mctrl(port_priv, set, clear);
return 0;
}
@@ -403,18 +475,11 @@ static void f81232_close(struct usb_serial_port *port)
static void f81232_dtr_rts(struct usb_serial_port *port, int on)
{
struct f81232_private *priv = usb_get_serial_port_data(port);
- unsigned long flags;
- u8 control;
- spin_lock_irqsave(priv-lock, flags);
- /* Change DTR and RTS */
if (on)
- priv-line_control |= (CONTROL_DTR | CONTROL_RTS);
+ update_mctrl(priv, TIOCM_DTR | TIOCM_RTS, 0);
else
- priv-line_control = ~(CONTROL_DTR | CONTROL_RTS);
- control = priv-line_control;
- spin_unlock_irqrestore(priv-lock, flags);
- set_control_lines(port-serial-dev, control);
+ update_mctrl(priv, 0, TIOCM_DTR | TIOCM_RTS);
}
static int f81232_carrier_raised(struct usb_serial_port *port)
--
1.9.1
--
To
RE: [PATCH] usb: gadget: composite: Provide list of registered functions
-Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, January 23, 2015 6:01 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Fri, Jan 23, 2015 at 05:56:22PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, January 23, 2015 5:27 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Fri, Jan 23, 2015 at 05:09:07PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Monday, January 19, 2015 7:59 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak wrote: Driver which provides implementation of some USB functions registers usb_function_driver structure in composite framework. Function drivers are identifed using registered name. When gadget is composed using configfs user must know what names has been registered. If function is compiled as a module this information can be found in modules.alias file. If function is compiled- in, there is no way to discover what usb functions are available in currently running kernel. Such situation is nothing new for linux kernel. Similar situation is with file systems. While mounting user can provide an fs type argument using -t option in mount. Those type names are registered by drivers. To make those names discoverable there is a /proc/filesystems which exports the list of currently registered file systems. This patch adds /proc/usb-functions attribute which exports the list of currently registered function drivers. This allows user to discover list of both compiled-in functions and from loaded kernel modules. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com you need to document the new file under Documentation/ABI/ I have just sent v2 version with documentation. I have done some more research and it looks like /sys/kernel could be a good alternative if you find that proc usage is not a good idea. What do you thing? Should we continue with /proc/usb- functions or replace this patch with /sys/kernel/usb-functions or maybe why don't we place the file at the same directory as our configfs has been mounted ? Default place for mounting configfs is /sys/kernel/config. It is created in init function of configfs module. For example systemd mounts there configfs on system startup. So summing up, desired location should be /sys/kernel/usb- functions? shouldn't this be /sys/kernel/config/usb-functions ? in fact, thinking about absolute paths isn't really the best. I can decide to mount configfs anywhere. What we want is for the file to be available on the root directory of our usb-functions configfs interface :-) So I must have misunderstood something. I'm not sure if this is a good idea. Some userspace depends on assumption that sys/kernel/config directory is empty and it's default place for mounting configfs. So why place there any attribute if we will mount there filesystem and we won't see it? I have prepared list of options related to your proposition: 1) Add a sysfs entry in config directory. This is easy but from userspace point of view It looks a little bit weird to place an attribute In canonical location for mounting some filesystem. Moreover config directory is maintained by configfs module and adding our subsystem specific file to this generic configfs location doesn't look good. 2) Add a file in configfs: 2a) In configfs root (next to usb-gadget subdir) I don't think that it is not a good idea. ConfigFS is generic filesystem and its root directory is reserved only for subsystem dirs. Adding there a file which is specific for only one of subsystems doesn't sound good to me.
Re: [PATCH] usb: gadget: composite: Provide list of registered functions
Hi, On Tue, Jan 27, 2015 at 05:51:30PM +0100, Krzysztof Opasiak wrote: From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, January 23, 2015 6:01 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Fri, Jan 23, 2015 at 05:56:22PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, January 23, 2015 5:27 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Fri, Jan 23, 2015 at 05:09:07PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Monday, January 19, 2015 7:59 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak wrote: Driver which provides implementation of some USB functions registers usb_function_driver structure in composite framework. Function drivers are identifed using registered name. When gadget is composed using configfs user must know what names has been registered. If function is compiled as a module this information can be found in modules.alias file. If function is compiled- in, there is no way to discover what usb functions are available in currently running kernel. Such situation is nothing new for linux kernel. Similar situation is with file systems. While mounting user can provide an fs type argument using -t option in mount. Those type names are registered by drivers. To make those names discoverable there is a /proc/filesystems which exports the list of currently registered file systems. This patch adds /proc/usb-functions attribute which exports the list of currently registered function drivers. This allows user to discover list of both compiled-in functions and from loaded kernel modules. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com you need to document the new file under Documentation/ABI/ I have just sent v2 version with documentation. I have done some more research and it looks like /sys/kernel could be a good alternative if you find that proc usage is not a good idea. What do you thing? Should we continue with /proc/usb- functions or replace this patch with /sys/kernel/usb-functions or maybe why don't we place the file at the same directory as our configfs has been mounted ? Default place for mounting configfs is /sys/kernel/config. It is created in init function of configfs module. For example systemd mounts there configfs on system startup. So summing up, desired location should be /sys/kernel/usb- functions? shouldn't this be /sys/kernel/config/usb-functions ? in fact, thinking about absolute paths isn't really the best. I can decide to mount configfs anywhere. What we want is for the file to be available on the root directory of our usb-functions configfs interface :-) So I must have misunderstood something. I'm not sure if this is a good idea. Some userspace depends on assumption that sys/kernel/config directory is empty and it's default place for mounting configfs. and that's fine, they can certainly assume that. Once configfs is mounted, we will have a new /sys/kernel/config/usb-functions directory. Inside that directory we should have a file which contains your list of available functions :-) I don't see what the problem is with that :-s So why place there any attribute if we will mount there filesystem and we won't see it? the attribute will only be availabe once we mount configfs, and it'll be part of our usb-fucntions directory. -- balbi signature.asc Description: Digital signature
Re: Fwd: USB device uses dma on its MMIO region?
On Thu, Jan 22, 2015 at 3:41 PM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 22 Jan 2015, Elena Ufimtseva wrote: On Thu, Jan 22, 2015 at 1:10 PM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 22 Jan 2015, Elena Ufimtseva wrote: Hello While working on IOMMU related problem for Xen Hypervisor, it was discovered that on some machines IOMMU page faults are triggered on certain addresses and requests that cause this are DMA requests with source device identified as USB Host controllers. Thank you for taking a look at this Alan. You mean EHCI controllers? Right, EHCI controllers. Further analysis showed that faulting addresses are not mapped with IOMMU with RW as these regions are not enumerated as RMRRs. Xen does make sure that RMRRs are mapped RW in IOMMU. These faulting addresses fall into reserved region within e820 map. Are you saying that the addresses aren't mapped at all, or that they are mapped RO rather than RW? These are not mapped. They dont have EPTs only because in code there is a condition missing. RMRRs enumerated by ACPI RMRRs are mapped with RW access. I think this lead to a question being discussed on xen-devel on why there is a device that would initiate DMA request on reserved and not rmrr marked region. I don't know what EPT and RMRR mean. RMRR is ACPI Reserved memory Region Reporting Structure reported for iommu unit. EPT - extended page tables in Intel VT-x. This page faults with addresses/devices are being logged under Xen: (XEN) Scrubbing Free RAM on 1 nodes using 4 CPUs (XEN) IOMMU Page fault! (XEN) IOMMU Page fault! (XEN) [VT-D]iommu.c:875: iommu_fault_status: Fault Overflow (XEN) [VT-D]iommu.c:877: iommu_fault_status: Primary Pending Fault (XEN) [VT-D]iommu.c:855: DMAR:[DMA Read] Request device [:00:1a.0] fault addr d5d46000, iommu reg = 82c000203000 Why do you think d6d46000 is in the MMIO region for the :00:1a.0 device? According to the log: [ 13.340574] ehci-pci :00:1a.0: irq 17, io mem 0xf7c38000 So the MMIO region starts at f7c38000. Right, I agree and I saw it. Its not MMIO, rather its assumed that if its not RAM in e820 map, that it may be MMIO. I probably should not use this term. In this case faulting addresses do not match any of MMIOs reported per device or RMRRs. At the same time the identified devices do initiate DMA reads for the addresses mentioned. I just wanted to see what it can be and understand the better way to solve it. If that address isn't mapped to RAM then there's no reason for the EHCI controller to issue a DMA using the address. Understood. On the other hand, it's not easy to find all the DMA addresses that an EHCI controller uses. There are a few coherent DMA pools, some static coherent mappings, and some dynamic streaming mappings. One place to start looking is in the files under /sys/kernel/debug/usb/ehci/:00:1a.0/. Thank you Alan, I will look into this. Alan Stern -- Elena -- 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 20/23] musb_virthub: use HUB_CHAR_*
Hello. On 01/27/2015 06:35 PM, Felipe Balbi wrote: Fix using the bare numbers to set the 'wHubCharacteristics' field of the Hub Descriptor while the values are #define'd in linux/usb/ch11.h. Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- drivers/usb/musb/musb_virthub.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Index: usb/drivers/usb/musb/musb_virthub.c === --- usb.orig/drivers/usb/musb/musb_virthub.c +++ usb/drivers/usb/musb/musb_virthub.c @@ -349,9 +349,10 @@ int musb_hub_control( desc-bDescriptorType = 0x29; desc-bNbrPorts = 1; desc-wHubCharacteristics = cpu_to_le16( - 0x0001/* per-port power switching */ - | 0x0010/* no overcurrent reporting */ - ); + HUB_CHAR_INDV_PORT_LPSM /* per-port power switching */ + Ugh, don't know how this empty line sneaked into the patch. Greg, could you please drop it? fixed You mean you've merged the patch to your tree? If so, be prepared for the conflict with Greg's tree since Greg has already merged the patch to his 'usb-next' branch... Thanks anyway! WBR, Sergei -- 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 20/23] musb_virthub: use HUB_CHAR_*
On Tue, Jan 27, 2015 at 07:06:14PM +0300, Sergei Shtylyov wrote: Hello. On 01/27/2015 06:35 PM, Felipe Balbi wrote: Fix using the bare numbers to set the 'wHubCharacteristics' field of the Hub Descriptor while the values are #define'd in linux/usb/ch11.h. Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- drivers/usb/musb/musb_virthub.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Index: usb/drivers/usb/musb/musb_virthub.c === --- usb.orig/drivers/usb/musb/musb_virthub.c +++ usb/drivers/usb/musb/musb_virthub.c @@ -349,9 +349,10 @@ int musb_hub_control( desc-bDescriptorType = 0x29; desc-bNbrPorts = 1; desc-wHubCharacteristics = cpu_to_le16( -0x0001/* per-port power switching */ - | 0x0010/* no overcurrent reporting */ - ); +HUB_CHAR_INDV_PORT_LPSM /* per-port power switching */ + Ugh, don't know how this empty line sneaked into the patch. Greg, could you please drop it? fixed You mean you've merged the patch to your tree? If so, be prepared for the conflict with Greg's tree since Greg has already merged the patch to his 'usb-next' branch... Thanks anyway! musb goes through me, right ? :-) Conflicts are easy to solve, no problem. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY
Hi guys, I'm only using the init and exit hooks instead of just power_on/power_off because I'm not sure which the controllers will use. For example, now dwc3 calls phy_init() in it's resume routine and not phy_power_on() like I would expect. I know the problem has been pointed out by others, so I'm expecting we'll get guidelines for it later. But before we do, I see no harm in having both init and power_on hooks in this driver. I'm unable to find the thread where somebody mentioned that, but I don't actually think there is any reason why we couldn't just call phy_power_on/off instead of phy_init/exit in dwc3_suspend/resume like I think we should. I'll add one more patch where I'll change it. Felipe, is it OK? 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: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc
- Mail original - De: Fabio Estevam feste...@gmail.com À: Robert Jarzmik robert.jarz...@free.fr Cc: Felipe Balbi ba...@ti.com, Linus Walleij linus.wall...@linaro.org, Philipp Zabel philipp.za...@gmail.com, USB list linux-usb@vger.kernel.org Envoyé: Mardi 27 Janvier 2015 12:23:43 Objet: Re: [PATCH v2 1/3] usb: phy: generic: migrate to gpio_desc I'll amend my patch just as you said, my bad. but the problem I see is that even if I use: nop-gpiod_reset = devm_gpiod_get_optional(dev, reset); The 'reset-gpio' pin cannot be retrieved. This is the point I don't understand currently. If the reset-gpio device-tree property doesn't exist, devm_gpiod_get_optional() will return a NULL pointer, and nop-gpiod_reset = NULL. Then in nop_reset_set(), the first test will assert that (nop-gpiod_reset == NULL) and return. For the longer story, devm_gpiod_get_optional() returns NULL : - because gpiod_get_index_optional() returns NULL - because gpiod_get_index returns -ENOENT - because of_find_gpio() returns -ENOENT Does that answer you question ? Cheers. -- Robert -- 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: XHCI, brain-dead scanner, and microframe rounding
On 27.01.2015 13:57, Hans-Peter Jansen wrote: What about a stable backport? Do you think, it's feasible, too much work, or too risky? This should definitely be backported to 3.18 stable, which should be easy. Older longterms kernels are a bit tricky as the halt and reset code around this was heavily modified in 3.18 -Mathias -- 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: ffs: add eventfd notification about ffs events
On Fri, Jan 23, 2015 at 01:34:45PM +0100, Robert Baldyga wrote:
On 01/23/2015 01:21 PM, Michał Nazarewicz wrote:
23 sty 2015 10:28 Robert Baldyga r.bald...@samsung.com
mailto:r.bald...@samsung.com napisał(a):
Add eventfd which notifies userspace about ep0 events and AIO completion
events. It simplifies using of FunctionFS with event loop, because now
we need to poll on single file (instead of polling on ep0 and eventfd's
supplied to AIO layer).
FunctionFS eventfd is not triggered if another eventfd is supplied to
AIO layer (in AIO request). It can be useful, for example, when we want
to handle AIO transations for chosen endpoint in separate thread.
Signed-off-by: Robert Baldyga
I don't know much about eventfd interface bit from ffs point of view:
Acked-by: Michal Nazarewicz min...@mina86.com mailto:min...@mina86.com
@@ -2180,6 +2191,20 @@ static int __ffs_data_got_descs(struct ffs_data
*ffs,
goto error;
}
+ if (flags FUNCTIONFS_EVENTFD) {
+ if (len 4)
+ goto error;
+ ffs-ffs_eventfd =
+ eventfd_ctx_fdget((int)get_unaligned_le32(data));
+ if (IS_ERR(ffs-ffs_eventfd)) {
+ ffs-ffs_eventfd = NULL;
+ ret = PTR_ERR(ffs-ffs_eventfd);
Need to swap those lines.
+ goto error;
+ }
+ data += 4;
+ len -= 4;
+ }
+
/* Read fs_count, hs_count and ss_count (if present) */
for (i = 0; i 3; ++i) {
if (!(flags (1 i))) {
Good catch, thanks!
Are you sending a new version, or should I fix this myself?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 6/8] usb: dwc3: add ULPI interface support
On Tue, Jan 27, 2015 at 01:09:25PM +0200, Heikki Krogerus wrote: look at your patch again. In case DWC3_ULPI isn't enabled, this file won't be linked at all. You're using stubs, so taht's fine. In case it _is_ enabled, you're breaking out early if you can't register ulpi interface, meaning you're exitting probe() (which, in fact, seems wrong as I want to be able to run dwc3 with ULPI enabled on a platform that was configured with ULPI+UTMI, but uses UTMI PHY). I still think this patch won't work in all cases. OK. So in case of ULPI+UTMI we'll try the ulpi interface, and if it fails, fall back to UTMI. Or can we use some other method to determine to which interface the PHY is really attached to in that case? I think we would need a phy_interface_sel pdata/DT binding for those cases. It should be optional and probably only needed for a few odd devices. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: phy-generic: Don't fail on missing gpio reset
On Monday 26 January 2015 17:45:29 Soren Brinkmann wrote:
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254241fb..a73d4c738f0b 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -241,10 +241,8 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
if (err == -EPROBE_DEFER)
return -EPROBE_DEFER;
- if (err) {
- dev_err(dev, Error requesting RESET GPIO\n);
- return err;
- }
+ if (err)
+ nop-gpiod_reset = NULL;
You might want to distinguish between a missing property and a
reset gpio that was specified but for some reason cannot be used.
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
Re: [PATCH v3 3/3] usb: udc: add usb_udc_activation_handler
On Mon, Jan 19, 2015 at 11:25:05AM -0500, Alan Stern wrote: On Mon, 19 Jan 2015, Peter Chen wrote: On Fri, Jan 16, 2015 at 11:11:56AM -0500, Alan Stern wrote: On Fri, 16 Jan 2015, Peter Chen wrote: During this API, the deactivation count will be update, and it will try to connect or disconnect gadget. It can be used to enable functions for gadget. Signed-off-by: Peter Chen peter.c...@freescale.com --- drivers/usb/gadget/udc/udc-core.c | 28 +++- include/linux/usb/gadget.h| 5 + 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 9396a86..86d4d19 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -37,6 +37,7 @@ * @list - for use by the udc class driver * @vbus - for udcs who care about vbus status, this value is real vbus status; * for udcs who do not care about vbus status, this value is always true + * @deactivations - the deactivation count to connect or disconnect gadget How will this field get initialized? Thanks, Alan. The function driver will be deactivated default, the patch likes below: http://permalink.gmane.org/gmane.linux.usb.general/112747 (Call usb_udc_activation_handler instead of usb_function_deactivate) The function driver can activate itself at the .bind or on the demand. http://permalink.gmane.org/gmane.linux.usb.general/112748 http://permalink.gmane.org/gmane.linux.usb.general/112752 Okay, then I have no objections. Acked-by: Alan Stern st...@rowland.harvard.edu Peter, care to resend your series with Alan's Ack ? thanks -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/2] drivers: usb: dwc2: remove 'force' parameter from kill_all_requests()
On Mon, Jan 05, 2015 at 10:31:35AM +0100, Robert Baldyga wrote: This patch fixes in simpler way the bug described in [1] and [2]. It looks like DWC2 is the only UDC driver that doesn't force usb requests to complete in ep_disable() function. This causes described problem, because we have no guarantee that all requests will be completed before unbind of usb function. To fix this problem we force all requests of disabled endpoint to complete. Also currently running request is not handled. This allowed to simplify code of kill_all_requests() function, because 'force' parameter is always set to true, so we don't need it anymore. In s3c_hsotg_rx_data() we change function used to print message when active request is NULL from dev_warn() to dev_dbg(), because such situation is harmless for driver and now it can take place during normal endpoint disabling. [1] https://lkml.org/lkml/2014/12/9/283 [2] https://lkml.org/lkml/2014/12/12/360 Signed-off-by: Robert Baldyga r.bald...@samsung.com this fails to apply: checking file drivers/usb/dwc2/gadget.c Hunk #1 succeeded at 2602 (offset 35 lines). Hunk #2 FAILED at 2588. Hunk #3 succeeded at 2645 (offset 36 lines). Hunk #4 FAILED at 2924. 2 out of 4 hunks FAILED -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] chipidea: pci: register nop PHY
On Tue, Jan 27, 2015 at 03:50:18PM +0200, Andy Shevchenko wrote: Since PHY for ChipIdea is optional (not all SoCs having PHY for ChipIdea should be programmed), we register 'nop' PHY for platforms that do not have programmable PHY. Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com looks good to me. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] usb: gadget: composite: Provide list of registered functions
Hi, On Fri, Jan 23, 2015 at 05:00:18PM +0100, Krzysztof Opasiak wrote: @@ -1558,13 +1559,63 @@ void unregister_gadget_item(struct config_item *item) } EXPORT_SYMBOL_GPL(unregister_gadget_item); +#ifdef CONFIG_PROC_FS why proc when I requested repeatedly to put this file under the same configfs directory where functionfs is mounted ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 0/5] Add support for Fujitsu USB host controller
Hi, On Sun, Jan 25, 2015 at 04:13:23PM +0800, Sneeker Yeh wrote: These patches add support for XHCI compliant Host controller found on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/ The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver and last four patch is about quirk implementation of errata in Synopsis DesignWare USB3 IP. Patch 1 introduces a quirk with device disconnection management necessary Synopsys Designware USB3 IP with versions 3.00a and hardware configuration DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the quirk, that host controller will die after a usb device is disconnected from port of root hub. Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform data. Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3 IP core driver. Patch 4 introduces using a quirk based on a errata of Synopsis DesignWare USB3 IP which is versions 3.00a and has hardware configuration DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a result this quirk has to be enabled via platform data or device tree. Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP driver. Mathias, let me know how you want to handle this. Either I take them all, or you take them all. What do you prefer ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 20/23] musb_virthub: use HUB_CHAR_*
On Sun, Jan 25, 2015 at 07:51:49PM +0300, Sergei Shtylyov wrote: Hello. On 01/19/2015 01:57 AM, Sergei Shtylyov wrote: Fix using the bare numbers to set the 'wHubCharacteristics' field of the Hub Descriptor while the values are #define'd in linux/usb/ch11.h. Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- drivers/usb/musb/musb_virthub.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Index: usb/drivers/usb/musb/musb_virthub.c === --- usb.orig/drivers/usb/musb/musb_virthub.c +++ usb/drivers/usb/musb/musb_virthub.c @@ -349,9 +349,10 @@ int musb_hub_control( desc-bDescriptorType = 0x29; desc-bNbrPorts = 1; desc-wHubCharacteristics = cpu_to_le16( - 0x0001/* per-port power switching */ -| 0x0010/* no overcurrent reporting */ -); + HUB_CHAR_INDV_PORT_LPSM /* per-port power switching */ + Ugh, don't know how this empty line sneaked into the patch. Greg, could you please drop it? fixed -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/7] extcon: usb-gpio: Introduce gpio usb extcon driver
Chanwoo,
On 27/01/15 03:54, Chanwoo Choi wrote:
Hi Roger,
On 01/27/2015 01:27 AM, Roger Quadros wrote:
Hi Chanwoo,
All your comments are valid. Need some clarification on one comment.
On 26/01/15 15:56, Chanwoo Choi wrote:
Hi Roger,
This patch looks good to me. But I add some comment.
If you modify some comment, I'll apply this patch on 3.21 queue.
On Mon, Jan 26, 2015 at 9:15 PM, Roger Quadros rog...@ti.com wrote:
This driver observes the USB ID pin connected over a GPIO and
updates the USB cable extcon states accordingly.
The existing GPIO extcon driver is not suitable for this purpose
as it needs to be taught to understand USB cable states and it
can't handle more than one cable per instance.
For the USB case we need to handle 2 cable states.
1) USB (attach/detach)
2) USB-Host (attach/detach)
This driver can be easily updated in the future to handle VBUS
events in case it happens to be available on GPIO for any platform.
Signed-off-by: Roger Quadros rog...@ti.com
---
.../devicetree/bindings/extcon/extcon-usb-gpio.txt | 20 ++
drivers/extcon/Kconfig | 7 +
drivers/extcon/Makefile| 1 +
drivers/extcon/extcon-usb-gpio.c | 214
+
4 files changed, 242 insertions(+)
create mode 100644
Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
create mode 100644 drivers/extcon/extcon-usb-gpio.c
snip
+
+static int usb_extcon_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct device_node *np = dev-of_node;
+ struct usb_extcon_info *info;
+ int ret;
+
+ if (!np)
+ return -EINVAL;
+
+ info = devm_kzalloc(pdev-dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info-dev = dev;
+ info-id_gpiod = devm_gpiod_get(pdev-dev, id);
+ if (IS_ERR(info-id_gpiod)) {
+ dev_err(dev, failed to get ID GPIO\n);
+ return PTR_ERR(info-id_gpiod);
+ }
+
+ ret = gpiod_set_debounce(info-id_gpiod,
+USB_GPIO_DEBOUNCE_MS * 1000);
+ if (ret 0)
+ info-debounce_jiffies =
msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
+
+ INIT_DELAYED_WORK(info-wq_detcable, usb_extcon_detect_cable);
+
+ info-id_irq = gpiod_to_irq(info-id_gpiod);
+ if (info-id_irq 0) {
+ dev_err(dev, failed to get ID IRQ\n);
+ return info-id_irq;
+ }
+
+ ret = devm_request_threaded_irq(dev, info-id_irq, NULL,
+ usb_irq_handler,
+ IRQF_SHARED | IRQF_ONESHOT |
+ IRQF_NO_SUSPEND,
+ pdev-name, info);
use of IRQF_NO_SUSPEND is not recommended to be used together with
IRQF_SHARED so
I'll remove IRQF_SHARED from here if we decide to stick with IRQF_NO_SUSPEND.
More on this below.
+ if (ret 0) {
+ dev_err(dev, failed to request handler for ID IRQ\n);
+ return ret;
+ }
+
+ info-edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
+ if (IS_ERR(info-edev)) {
+ dev_err(dev, failed to allocate extcon device\n);
+ return -ENOMEM;
+ }
+
+ ret = devm_extcon_dev_register(dev, info-edev);
+ if (ret 0) {
+ dev_err(dev, failed to register extcon device\n);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, info);
I prefer to execute the device_init_wakeup() function as following
for suspend/resume function:
device_init_wakeup(pdev-dev, 1);
+
+ /* Perform initial detection */
+ usb_extcon_detect_cable(info-wq_detcable.work);
+
+ return 0;
+}
+
+static int usb_extcon_remove(struct platform_device *pdev)
+{
+ struct usb_extcon_info *info = platform_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(info-wq_detcable);
Need to add blank line.
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int usb_extcon_suspend(struct device *dev)
+{
+ struct usb_extcon_info *info = dev_get_drvdata(dev);
+
+ enable_irq_wake(info-id_irq);
I prefer to use device_may_wakeup() function for whether
executing enable_irq_wake() or not. Also, The disable_irq()
in the suspend function would prevent us from discarding interrupt
before wakeup from suspend completely.
I need more clarification here.
If we are going to use enable_irq_wake() here then what is the point of
IRQF_NO_SUSPEND?
From Documentation/power/suspend-and-interrupts.txt I see that interrupts
marked
as IRQF_NO_SUSPEND should not be configured for system wakeup using
enable_irq_wake().
what is your preference?
Is it good enough to not use IRQF_NO_SUSPEND but use enable_irq_wake()
Re: [PATCH] usb: phy-generic: Don't fail on missing gpio reset
On Mon, Jan 26, 2015 at 05:45:29PM -0800, Soren Brinkmann wrote:
A reset through a GPIO is optional. Don't fail probing when it is
missing.
Reported-by: Andreas Färber afaer...@suse.de
Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
---
Hi Andreas,
does this do the trick?
Thanks,
Sören
drivers/usb/phy/phy-generic.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254241fb..a73d4c738f0b 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -241,10 +241,8 @@ int usb_phy_gen_create_phy(struct device *dev, struct
usb_phy_generic *nop,
if (err == -EPROBE_DEFER)
return -EPROBE_DEFER;
- if (err) {
- dev_err(dev, Error requesting RESET GPIO\n);
- return err;
- }
+ if (err)
+ nop-gpiod_reset = NULL;
there's a better patch to use gpiod_get_optional(), instead.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: gadget: ffs: add eventfd notification about ffs events
On Tue, Jan 27, 2015 at 09:25:30AM -0600, Felipe Balbi wrote:
On Fri, Jan 23, 2015 at 01:34:45PM +0100, Robert Baldyga wrote:
On 01/23/2015 01:21 PM, Michał Nazarewicz wrote:
23 sty 2015 10:28 Robert Baldyga r.bald...@samsung.com
mailto:r.bald...@samsung.com napisał(a):
Add eventfd which notifies userspace about ep0 events and AIO completion
events. It simplifies using of FunctionFS with event loop, because now
we need to poll on single file (instead of polling on ep0 and eventfd's
supplied to AIO layer).
FunctionFS eventfd is not triggered if another eventfd is supplied to
AIO layer (in AIO request). It can be useful, for example, when we want
to handle AIO transations for chosen endpoint in separate thread.
Signed-off-by: Robert Baldyga
I don't know much about eventfd interface bit from ffs point of view:
Acked-by: Michal Nazarewicz min...@mina86.com mailto:min...@mina86.com
@@ -2180,6 +2191,20 @@ static int __ffs_data_got_descs(struct ffs_data
*ffs,
goto error;
}
+ if (flags FUNCTIONFS_EVENTFD) {
+ if (len 4)
+ goto error;
+ ffs-ffs_eventfd =
+ eventfd_ctx_fdget((int)get_unaligned_le32(data));
+ if (IS_ERR(ffs-ffs_eventfd)) {
+ ffs-ffs_eventfd = NULL;
+ ret = PTR_ERR(ffs-ffs_eventfd);
Need to swap those lines.
+ goto error;
+ }
+ data += 4;
+ len -= 4;
+ }
+
/* Read fs_count, hs_count and ss_count (if present) */
for (i = 0; i 3; ++i) {
if (!(flags (1 i))) {
Good catch, thanks!
Are you sending a new version, or should I fix this myself?
nevermind, found v2 :)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v1 00/13] usb: second series of updates for dwc2 driver
On Thu, Jan 22, 2015 at 02:28:33AM +, John Youn wrote: From: Mian Yousaf Kaukab [mailto:yousaf.kau...@intel.com] Sent: Wednesday, January 21, 2015 6:37 AM Hi, This patchset consists of some bug fixes, feature enhancements and cosmetic changes for the dwc2 driver. All the patches are verified on dwc2 v3.0a with dedicated fifos. Main focus of testing was with dma enabled. Although basic testing without dma was also done. This is based on testing/next branch in Felipe's git. Removed Paul Zimmerman from CC as his Synopsys address is not valid any longer. Thank you, Best regards, Yousaf History: v1: - Fixed comments from Sergei Shtylyov and Robert Baldyga Gregory Herrero (8): usb: dwc2: host: register hcd handle to the phy usb: dwc2: host: resume root hub on remote wakeup usb: dwc2: gadget: fix clear halt feature handling usb: dwc2: gadget: add TEST_MODE feature support usb: dwc2: gadget: fix a typo in comment usb: dwc2: gadget: add reset flag in init function usb: dwc2: gadget: don't modify pullup status during reset usb: dwc2: gadget: initialize controller in pullup callback Mian Yousaf Kaukab (5): usb: dwc2: gadget: remove hardcoded if (0) and if (1) checks usb: dwc2: gadget: add unaligned buffers support usb: dwc2: gadget: fix debug message for zlp usb: dwc2: gadget: fix phy interface configuration usb: dwc2: gadget: replace constants with defines drivers/usb/dwc2/core.h | 12 +- drivers/usb/dwc2/gadget.c | 359 ++ drivers/usb/dwc2/hcd.c| 43 -- drivers/usb/dwc2/hw.h | 1 + 4 files changed, 349 insertions(+), 66 deletions(-) -- 1.9.1 Hi Yousaf, I'm working on testing this series now on our platform. Please give me a couple days as I am still getting up to speed. there's very little time to get this in v3.20 merge window, we might have to delay to 3.21. I prefer that you test this properly then us pushing something that causes regressiosn :-s -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] usb: Retry port status check on resume to work around RH bugs
On Mon, 26 Jan 2015, Julius Werner wrote:
The EHCI controller on the RK3288 SoC is violating basic parts of the
USB spec and thereby unable to properly resume a suspended port. It does
not start SOF generation within 3ms of finishing resume signaling, so
the attached device will drop off the bus again. This is a particular
problem with runtime PM, where accessing the device will trigger a
resume that immediately makes it unavailabe (and reenumerate with a new
Typo: unavailabe
handle).
Thankfully, the persist feature is generally able to work around stuff
like that. Unfortunately, it doesn't quite work in this particular case
because the controller will turn off the CurrentConnectStatus bit for an
instant while the device is reconnecting, which causes the kernel to
conclude that it permanently disappeared. This patch adds a tiny retry
mechanism to the core port resume code which will catch this case and
shouldn't have any notable impact on other controllers.
Signed-off-by: Julius Werner jwer...@chromium.org
---
drivers/usb/core/hub.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index aeb50bb..d1d0a66 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2896,10 +2896,12 @@ static int port_is_suspended(struct usb_hub *hub,
unsigned portstatus)
*/
static int check_port_resume_type(struct usb_device *udev,
struct usb_hub *hub, int port1,
- int status, unsigned portchange, unsigned portstatus)
+ int status, u16 portchange, u16 portstatus)
{
struct usb_port *port_dev = hub-ports[port1 - 1];
+ int retries = 3;
+retry:
I prefer to indent statement labels by one space, so that diff doesn't
see them as function declarations.
/* Is a warm reset needed to recover the connection? */
if (status == 0 udev-reset_resume
hub_port_warm_reset_required(hub, port1, portstatus)) {
@@ -2909,8 +2911,15 @@ static int check_port_resume_type(struct usb_device
*udev,
else if (status || port_is_suspended(hub, portstatus) ||
!port_is_power_on(hub, portstatus) ||
!(portstatus USB_PORT_STAT_CONNECTION)) {
- if (status = 0)
+ if (status = 0) {
Thinking about this more carefully, it looks like the only thing you
care about here is the !(portstatus USB_PORT_STAT_CONNECTION) case.
For everything else, you don't need to retry.
So why not separate out that one case?
else if (status || port_is_suspended(hub, portstatus) ||
!port_is_power_on(hub, portstatus)) {
if (status = 0)
status = -ENODEV;
} else if (!(portstatus USB_PORT_STAT_CONNECTION)) {
retry stuff...
}
+ if (retries--) {
+ usleep_range(200, 300);
+ status = hub_port_status(hub, port1,
+ portstatus, portchange);
+ goto retry;
+ }
status = -ENODEV;
+ }
}
/* Can't do a normal resume if the port isn't enabled,
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: composite: Provide list of registered functions
-Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Tuesday, January 27, 2015 5:58 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions Hi, On Tue, Jan 27, 2015 at 05:51:30PM +0100, Krzysztof Opasiak wrote: From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, January 23, 2015 6:01 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Fri, Jan 23, 2015 at 05:56:22PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Friday, January 23, 2015 5:27 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Fri, Jan 23, 2015 at 05:09:07PM +0100, Krzysztof Opasiak wrote: -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Monday, January 19, 2015 7:59 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH] usb: gadget: composite: Provide list of registered functions On Mon, Jan 19, 2015 at 02:17:19PM +0100, Krzysztof Opasiak wrote: Driver which provides implementation of some USB functions registers usb_function_driver structure in composite framework. Function drivers are identifed using registered name. When gadget is composed using configfs user must know what names has been registered. If function is compiled as a module this information can be found in modules.alias file. If function is compiled- in, there is no way to discover what usb functions are available in currently running kernel. Such situation is nothing new for linux kernel. Similar situation is with file systems. While mounting user can provide an fs type argument using -t option in mount. Those type names are registered by drivers. To make those names discoverable there is a /proc/filesystems which exports the list of currently registered file systems. This patch adds /proc/usb-functions attribute which exports the list of currently registered function drivers. This allows user to discover list of both compiled-in functions and from loaded kernel modules. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com you need to document the new file under Documentation/ABI/ I have just sent v2 version with documentation. I have done some more research and it looks like /sys/kernel could be a good alternative if you find that proc usage is not a good idea. What do you thing? Should we continue with /proc/usb- functions or replace this patch with /sys/kernel/usb-functions or maybe why don't we place the file at the same directory as our configfs has been mounted ? Default place for mounting configfs is /sys/kernel/config. It is created in init function of configfs module. For example systemd mounts there configfs on system startup. So summing up, desired location should be /sys/kernel/usb- functions? shouldn't this be /sys/kernel/config/usb-functions ? in fact, thinking about absolute paths isn't really the best. I can decide to mount configfs anywhere. What we want is for the file to be available on the root directory of our usb-functions configfs interface :-) So I must have misunderstood something. I'm not sure if this is a good idea. Some userspace depends on assumption that sys/kernel/config directory is empty and it's default place for mounting configfs. and that's fine, they can certainly assume that. Once configfs is mounted, we will have a new /sys/kernel/config/usb-functions directory. Inside that directory we should have a file which contains your
Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY
On Tue, Jan 27, 2015 at 11:28:56AM +0200, Heikki Krogerus wrote:
On Mon, Jan 26, 2015 at 11:23:37AM -0800, David Cohen wrote:
On Mon, Jan 26, 2015 at 02:55:03PM +0200, Heikki Krogerus wrote:
On Sat, Jan 24, 2015 at 03:58:11PM -0800, David Cohen wrote:
+static int tusb1210_power_on(struct phy *phy)
+{
+ struct tusb1210 *tusb = phy_get_drvdata(phy);
+
+ gpiod_set_value_cansleep(tusb-gpio_reset, 1);
+ gpiod_set_value_cansleep(tusb-gpio_cs, 1);
+
+ /* Restore eye diagram optimisation value */
+ ulpi_write(tusb-ulpi, TUSB1210_VENDOR_SPECIFIC2,
+tusb-eye_diagram_tuning);
After you power on phy, ulpi bus may not be available right away. In
intel case, phy power on happens during dwc3 power on. ULPI bus is not
available until OTG controller and phy are in sync.
In resume, you can't restore eye diagram from here.
I'm sorry but I don't think I understand? Where do we power on the phy
before dwc3 is powered on? Or is this a Baytrail-CR specific problem?
I can't see any problems with the hardware I have.
You can't see in single accesses. But you may see when running stability
tests overnight.
Anyway, look for dwc3_core_soft_reset() function:
- dwc3 goes to reset state
- phy is initialized (or at least gets ready to sync clocks)
- dwc3 goes out or reset state
And if you look at dwc3_probe, you'll notice that it calls
phy_power_on after that, and ulpi will most definitely be accessible
at that point.
I'm only using the init and exit hooks instead of just
power_on/power_off because I'm not sure which the controllers will
use. For example, now dwc3 calls phy_init() in it's resume routine and
not phy_power_on() like I would expect. I know the problem has been
pointed out by others, so I'm expecting we'll get guidelines for it
later. But before we do, I see no harm in having both init and
power_on hooks in this driver.
It sounds hackish to me. You could clearly change the logic on
init/power_on callbacks to avoid this problem.
During tusb1210 phy init from dwc3, you shouldn't access ulpi bus.
We will end up executing one failed write followed by write that we
know will succeed. Ideally we didn't have to do the first one at all,
but I don't see any risk here.
Ditto.
+ gpio = devm_gpiod_get(ulpi-dev, cs);
+ if (!IS_ERR(gpio)) {
+ ret = gpiod_direction_output(gpio, 0);
+ if (ret)
+ return ret;
+ tusb-gpio_cs = gpio;
+ }
+
+ /* Store initial eye diagram optimisation value */
+ ret = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2);
It's unclear if ulpi is accessible at this point. You can't read it at
this point.
We wouldn't have reached this point if ulpi wasn't accessible.
Registering the ulpi interface would have already failed so no driver
would have been probed.
You have a chicken/egg problem here:
- dwc3 needs phy to complete soft reset during probe
- tusb1210 needs dwc3 soft reset completed to be accessible via ULPI
Can you share how tusb1210 is connected on the platform you're using as
test for this patch? I don't think this driver would work reliably with
this device:
http://liliputing.com/2014/11/trekstor-launches-first-android-tablet-based-intels-irda-reference-design.html
The only reason why that board doesn't work is because of very much
Baytrail-CR specific problems. These are are two issues, but the first
That's not BYT-CR specific problems. That's just dwc3 and tusb1210
interacting as they're expecting to.
one is critical for getting it working. Both will be handled, but
separately from this set:
1) The firmware leaves the PHY in reset, forcing us to enable it
somehow in OS before accessing ulpi. Unless we can get a firmware fix
for that (it's starting to look like it's not going to happen; please
correct me if you know something else!), we need to add a quirk for
Baytrails (attached), which is probable still OK. But IMO this really
should be fixed in the firmware.
It seems you're expecting the PHY to be fully operational in order to
probe it. That's wrong assumption. BYT-CR's BIOS is doing nothing wrong
by leaving PHY on reset state. The real problem is what I stated above.
With your current logic, you'll get stuck with checking/egg problem: you
need phy probed to probe dwc3, but need dwc3 probed to power on phy.
You need a logic to break this circular dependency.
2) Since the gpio resources are given to the controller device in ACPI
tables and there isn't separate device object for the PHY at all, we
need to deliver the gpios somehow separately to the phy driver. There
is a thread where we are talking about how to do that:
https://lkml.org/lkml/2014/12/18/82
How about just leave the logic the way it is:
dwc3-pci.c registers platform_device with
Re: [PATCH v1] ehci-pci: disable for Intel MID platforms
On Tue, 27 Jan 2015, Peter Chen wrote: Please fix this properly. I don't know the right way to fix this. Alan, has you any suggestion? It depends. How did the code before the adfa79d1c06a commit avoid this problem? By simply not enabling CONFIG_USB_EHCI_HCD? Hi Andy, Would you define pci_id at ci_hdrc_pci.c (instead of ehci-pci.c) at your platform code/table to fix this problem? I am not familiar with pci, but it works at other platforms, like ARM. You can't remove the definition of pci_id in ehci-pci.c; if you did then ehci-pci wouldn't bind to PCI controllers. It may turn out that the best way to fix this is to include a quirk in ehci-pci.c to prevent it from binding to the Intel MID on-board EHCI controller. 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: [RFC PATCH 2/4] usb: chipidea: udc: add set_selfpowered gaget ops
On Tue, 27 Jan 2015, Peter Chen wrote:
The gadget power property will be used at get_status request.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/udc.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 540db0e..45914ba 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -841,6 +841,7 @@ __acquires(hwep-lock)
if ((setup-bRequestType USB_RECIP_MASK) == USB_RECIP_DEVICE) {
/* Assume that device is bus powered for now. */
*(u16 *)req-buf = ci-remote_wakeup 1;
+ *(u16 *)req-buf |= ci-gadget.is_selfpowered 1;
} else if ((setup-bRequestType USB_RECIP_MASK) \
When you add this line, don't you want to remove the comment two lines
above?
Also, you can simplify the code slightly by writing it like this:
*(u16 *) req-buf = (ci-remote_wakeup 1) |
ci-gadget.is_selfpowered;
Note that is_selfpowered should not be bit-shifted;
USB_DEVICE_SELF_POWERED is 0.
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 1/3] dt: usb: jz4740: Add DT binding document for OHCI
Hello.
On 01/27/2015 04:47 PM, Zubair Lutfullah Kakakhel wrote:
From: Paul Burton paul.bur...@imgtec.com
Add the binding documentation for the JZ47xx OHCI controller.
Signed-off-by: Paul Burton paul.bur...@imgtec.com
Signed-off-by: Zubair Lutfullah Kakakhel zubair.kakak...@imgtec.com
---
The jz4740 is platform only at the moment.
But DT support is being added
See http://patchwork.linux-mips.org/bundle/paulburton/ci20-v3.20/
jz47xx is used because jz4780 will also use this driver
---
.../bindings/usb/ingenic,jz47xx-ohci.txt | 28 ++
1 file changed, 28 insertions(+)
create mode 100644
Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
diff --git a/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
b/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
new file mode 100644
index 000..2fada8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ingenic,jz47xx-ohci.txt
@@ -0,0 +1,28 @@
+Ingenic JZ47XX SoC OHCI controller binding
I think JZ47xx would look a bit better.
+
+The Ingenic JZ47XX SoC includes an OHCI compliant USB host controller
+interface for use with USB 1.1 devices.
+
+Required properties:
+ - compatible: Should be ingenic,jz4740-ohci
+ - reg: Should contain the address size of the OHCI controller registers.
+ - interrupt-parent: Should be the phandle of the interrupt controller that
+ delivers interrupts to the OHCI block.
This is never a required prop, it can be inherited from a parent node.
+ - interrupts: Should specify the interrupt provided by interrupt-parent.
+ - clocks: Should contain a single clock specifier for the SoC UHC clock.
+
+Example SoC include file for jz4780:
+
+/ {
+ ohci: jz4780-ohci@0x134a {
The ePAPR standard says we shouldn't name the nodes after the chips used
but use the generic names, which would be usb in this case. And please drop
0x from the unit-address part of the name.
+ compatible = ingenic,jz4780-ohci;
+ reg = 0x134a 0x1;
+
+ interrupt-parent = intc;
+ interrupts = 5;
+
+ clocks = cgu JZ4780_CLK_UHC;
+ clock-names = uhc;
You forgot to mention this prop above.
[...]
WBR, Sergei
--
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: Kernel Ooops upon USB disconnect of a Western Digital My Passport 1TB drive
On Tue, 27 Jan 2015, Athlion wrote: Hello all. On my Thunkpad T420i with 3.18.2 and 3.18.3 kernels, I can reproduce the following behaviour 100% of the time: 1. Connect one of my (NTFS-formatted) WD 1TB 'My Passport' drives via USB. 2. Disconnect the cable. 3. Kernel Ooops. For last entries of the journal (it actually caught the kernel oops one time, I'm amazed) see the photo here: https://dl.dropboxusercontent.com/u/63420/journal.jpg Please note that the drive need not be mounted at all. In fact, this even happens if I stop all graphical and automount services and plug-in and then disconnect the cable. Also, other USB drives and USB flash disks do not exhibit this behaviour... Go figure! If I'm on a console, I can only see the last 45 or so lines of the stack trace (using KVM 1600x900 resolution console). The first lines (along with the initial oops message (caught in the journal) get scrolled off so I cannot post anything). What can I do to further debug this? I'm uncomfortable having any plug-in utility writing to my disk after the crash since I'm using this laptop for work and have crucial data on it. If you have another computer handy, you can set up a network console (or a serial console) so that the entire oops message gets captured. Also, you could try running the latest 3.19-rc kernel to see if the problem has already been fixed. 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 v1] ehci-pci: disable for Intel MID platforms
On Tue, 27 Jan 2015, Andy Shevchenko wrote: On Tue, 2015-01-27 at 11:21 -0500, Alan Stern wrote: On Tue, 27 Jan 2015, Peter Chen wrote: Please fix this properly. I don't know the right way to fix this. Alan, has you any suggestion? It depends. How did the code before the adfa79d1c06a commit avoid this problem? By simply not enabling CONFIG_USB_EHCI_HCD? Hi Andy, Would you define pci_id at ci_hdrc_pci.c (instead of ehci-pci.c) at your platform code/table to fix this problem? I am not familiar with pci, but it works at other platforms, like ARM. You can't remove the definition of pci_id in ehci-pci.c; if you did then ehci-pci wouldn't bind to PCI controllers. It may turn out that the best way to fix this is to include a quirk in ehci-pci.c to prevent it from binding to the Intel MID on-board EHCI controller. That was one of my thoughts. I would do that way. Then go ahead and write a patch to do this. Regarding to the initial issue, it was along time ago, and I totally forgot what we do to make it working. Thus, it possible that the split does nothing with the problem. If you don't know that the adfa79d1c06a commit caused the problem then your Changelog shouldn't claim that it did. 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_V2 0/3] usb: ohci: jz4740: Add DT support and a fix
On Tue, 27 Jan 2015, Zubair Lutfullah Kakakhel wrote: Hi, Here are a few simple patches for the jz4740. First adds a simple DT binding. Seconds adds DT support. Third is a minor fix in clock enabling. Patches are based on 3.19-rc6. Quite disjoint and stay within jz4740 so should apply easily on other trees. If you would like to have them rebased to a different tree, please tell. Patches 2/3 and 3/3: Acked-by: Alan Stern st...@rowland.harvard.edu -- 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] usb: gadget: composite: Provide list of registered functions
-Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Tuesday, January 27, 2015 4:57 PM To: Krzysztof Opasiak Cc: ba...@ti.com; linux-usb@vger.kernel.org; gre...@linuxfoundation.org; bige...@breakpoint.cc; s.wa...@samsung.com; k.lewando...@samsung.com; m.szyprow...@samsung.com; andrze...@samsung.com Subject: Re: [PATCH v2] usb: gadget: composite: Provide list of registered functions Hi, On Fri, Jan 23, 2015 at 05:00:18PM +0100, Krzysztof Opasiak wrote: @@ -1558,13 +1559,63 @@ void unregister_gadget_item(struct config_item *item) } EXPORT_SYMBOL_GPL(unregister_gadget_item); +#ifdef CONFIG_PROC_FS why proc when I requested repeatedly to put this file under the same configfs directory where functionfs is mounted ? This patch has been sent before I got your suggestion to place it in sysfs. I will hold-on with next versions of his patch till we agree where exactly it should be place. Please see my response[1] for some details. Footnotes: 1 - http://marc.info/?l=linux-usbm=142237749620163w=2 BR's -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- 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: USB autosuspend causing trouble with bluetooth
On Mon, 2015-01-26 at 21:00 +0400, Kirill Elagin wrote: Things just got way worse with this hotplug thing. Is that a new test or did you update? When the host power/control is set to `auto`, as soon as I insert the Unifying receiver one of kworkers starts eating 100% of one of the But it does not enumerate the dongle, does it? cores, according to `htop`. As soon as I `echo on` to power/control of the relevant USB hub the kworker stops doing this. This also happens with that other USB1.1 BT-dongle, so I assume any USB1.1 device would do. Also just unplugging the device is not enough, the kworker keeps eating CPU until I `echo on` to power/control. Should I start a separate thread? Right now I'm on 3.18.2-gentoo. Can you tell at which kernel version this started? Regards Oliver -- 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: [RFC PATCH 0/4] usb: gadget: add is_selfpowered
Hi, On Wed, Jan 28, 2015 at 10:05:39AM +0800, Peter Chen wrote: On Tue, Jan 27, 2015 at 10:37:30AM +0800, Peter Chen wrote: Hi Felipe, Hi Felipe, Except for Alan's comments, do you agree with this change for common struct, if you agree with it, I will send formal patch with other udc driver's cleanup. looks good to me. Will you provide patches to all other UDC drivers ? -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 0/4] usb: gadget: add is_selfpowered
On Tue, Jan 27, 2015 at 10:07:02PM -0600, Felipe Balbi wrote: Hi, On Wed, Jan 28, 2015 at 10:05:39AM +0800, Peter Chen wrote: On Tue, Jan 27, 2015 at 10:37:30AM +0800, Peter Chen wrote: Hi Felipe, Hi Felipe, Except for Alan's comments, do you agree with this change for common struct, if you agree with it, I will send formal patch with other udc driver's cleanup. looks good to me. Will you provide patches to all other UDC drivers ? it's certainly not mandatory, and I can help with both musb and dwc3 (probably a few others too), but it'll help me if you do. :-) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/7] extcon: usb-gpio: Introduce gpio usb extcon driver
Hi Roger,
On 01/28/2015 12:38 AM, Roger Quadros wrote:
Chanwoo,
On 27/01/15 03:54, Chanwoo Choi wrote:
Hi Roger,
On 01/27/2015 01:27 AM, Roger Quadros wrote:
Hi Chanwoo,
All your comments are valid. Need some clarification on one comment.
On 26/01/15 15:56, Chanwoo Choi wrote:
Hi Roger,
This patch looks good to me. But I add some comment.
If you modify some comment, I'll apply this patch on 3.21 queue.
On Mon, Jan 26, 2015 at 9:15 PM, Roger Quadros rog...@ti.com wrote:
This driver observes the USB ID pin connected over a GPIO and
updates the USB cable extcon states accordingly.
The existing GPIO extcon driver is not suitable for this purpose
as it needs to be taught to understand USB cable states and it
can't handle more than one cable per instance.
For the USB case we need to handle 2 cable states.
1) USB (attach/detach)
2) USB-Host (attach/detach)
This driver can be easily updated in the future to handle VBUS
events in case it happens to be available on GPIO for any platform.
Signed-off-by: Roger Quadros rog...@ti.com
---
.../devicetree/bindings/extcon/extcon-usb-gpio.txt | 20 ++
drivers/extcon/Kconfig | 7 +
drivers/extcon/Makefile| 1 +
drivers/extcon/extcon-usb-gpio.c | 214
+
4 files changed, 242 insertions(+)
create mode 100644
Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
create mode 100644 drivers/extcon/extcon-usb-gpio.c
snip
+
+static int usb_extcon_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct device_node *np = dev-of_node;
+ struct usb_extcon_info *info;
+ int ret;
+
+ if (!np)
+ return -EINVAL;
+
+ info = devm_kzalloc(pdev-dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info-dev = dev;
+ info-id_gpiod = devm_gpiod_get(pdev-dev, id);
+ if (IS_ERR(info-id_gpiod)) {
+ dev_err(dev, failed to get ID GPIO\n);
+ return PTR_ERR(info-id_gpiod);
+ }
+
+ ret = gpiod_set_debounce(info-id_gpiod,
+USB_GPIO_DEBOUNCE_MS * 1000);
+ if (ret 0)
+ info-debounce_jiffies =
msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
+
+ INIT_DELAYED_WORK(info-wq_detcable, usb_extcon_detect_cable);
+
+ info-id_irq = gpiod_to_irq(info-id_gpiod);
+ if (info-id_irq 0) {
+ dev_err(dev, failed to get ID IRQ\n);
+ return info-id_irq;
+ }
+
+ ret = devm_request_threaded_irq(dev, info-id_irq, NULL,
+ usb_irq_handler,
+ IRQF_SHARED | IRQF_ONESHOT |
+ IRQF_NO_SUSPEND,
+ pdev-name, info);
use of IRQF_NO_SUSPEND is not recommended to be used together with
IRQF_SHARED so
I'll remove IRQF_SHARED from here if we decide to stick with
IRQF_NO_SUSPEND.
More on this below.
+ if (ret 0) {
+ dev_err(dev, failed to request handler for ID IRQ\n);
+ return ret;
+ }
+
+ info-edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
+ if (IS_ERR(info-edev)) {
+ dev_err(dev, failed to allocate extcon device\n);
+ return -ENOMEM;
+ }
+
+ ret = devm_extcon_dev_register(dev, info-edev);
+ if (ret 0) {
+ dev_err(dev, failed to register extcon device\n);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, info);
I prefer to execute the device_init_wakeup() function as following
for suspend/resume function:
device_init_wakeup(pdev-dev, 1);
+
+ /* Perform initial detection */
+ usb_extcon_detect_cable(info-wq_detcable.work);
+
+ return 0;
+}
+
+static int usb_extcon_remove(struct platform_device *pdev)
+{
+ struct usb_extcon_info *info = platform_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(info-wq_detcable);
Need to add blank line.
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int usb_extcon_suspend(struct device *dev)
+{
+ struct usb_extcon_info *info = dev_get_drvdata(dev);
+
+ enable_irq_wake(info-id_irq);
I prefer to use device_may_wakeup() function for whether
executing enable_irq_wake() or not. Also, The disable_irq()
in the suspend function would prevent us from discarding interrupt
before wakeup from suspend completely.
I need more clarification here.
If we are going to use enable_irq_wake() here then what is the point of
IRQF_NO_SUSPEND?
From Documentation/power/suspend-and-interrupts.txt I see that interrupts
marked
as IRQF_NO_SUSPEND should not be configured for system wakeup using
enable_irq_wake().
what is your preference?
Is it good
Re: [PATCH v1] ehci-pci: disable for Intel MID platforms
On Tue, Jan 27, 2015 at 11:21:56AM -0500, Alan Stern wrote: On Tue, 27 Jan 2015, Peter Chen wrote: Please fix this properly. I don't know the right way to fix this. Alan, has you any suggestion? It depends. How did the code before the adfa79d1c06a commit avoid this problem? By simply not enabling CONFIG_USB_EHCI_HCD? Hi Andy, Would you define pci_id at ci_hdrc_pci.c (instead of ehci-pci.c) at your platform code/table to fix this problem? I am not familiar with pci, but it works at other platforms, like ARM. You can't remove the definition of pci_id in ehci-pci.c; if you did then ehci-pci wouldn't bind to PCI controllers. It may turn out that the best way to fix this is to include a quirk in ehci-pci.c to prevent it from binding to the Intel MID on-board EHCI controller. Alan Stern Then, it is strange. Do we need even two glue layer drivers for pci device? Look at usb/chipidea/ci_hdrc_pci.c it has pci_register_driver, and its host driver will call ehci_init_driver, it is definitely duplicated with usb/host/ehci-pci.c. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: cdc-acm: check for descriptors with invalid length
On Tue, Jan 27, 2015 at 08:36:07PM +0100, Oliver Neukum wrote: Hi, could you test and review this? $ git am ~/0001-cdc-acm-add-sanity-checks.patch Applying: cdc-acm: add sanity checks /home/adamlee/ubuntu-vivid/.git/rebase-apply/patch:26: trailing whitespace. goto next_desc; warning: 1 line adds whitespace errors. It works on my env, also the logic of USB_CDC_CALL_MANAGEMENT_TYPE part is the same as my patch, so ACK. Signed-off-by: Adam Lee adam8...@gmail.com -- Adam Lee http://adam8157.info -- 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
