[PATCH v4] input: tablet: add Pegasus Notetaker tablet driver
This adds a driver for the Pegasus Notetaker Pen. When connected, this uses the Pen as an input tablet. This device was sold in various different brandings, for example "Pegasus Mobile Notetaker M210", "Genie e-note The Notetaker", "Staedtler Digital ballpoint pen 990 01", "IRISnotes Express" or "NEWLink Digital Note Taker". Here's an example, so that you know what we are talking about: http://www.staedtler.com/en/products/ink-writing-instruments/ballpoint-pens/digital-pen-990-01-digital-ballpoint-pen http://pegatech.blogspot.com/ seems to be a remaining official resource. This device can also transfer saved (offline recorded handwritten) data and there are userspace programs that do this, see https://launchpad.net/m210 (Well, alternatively there are really fast scanners out there :) It's *really* fun to use as an input tablet though! So let's support this for everybody. There's no way to disable the device. When the pen is out of range, we just don't get any URBs and don't do anything. Like all other mouses or input tablets, we don't use runtime PM. Signed-off-by: Martin Kepplinger --- Thanks for having a look. Any more suggestions on this? revision history v4 use normal work queue instead of a kernel thread (thanks to Oliver Neukum) v3 fix reporting low pen battery and add USB list to CC v2 minor cleanup (remove unnecessary variables) v1 initial release drivers/input/tablet/Kconfig | 15 ++ drivers/input/tablet/Makefile| 1 + drivers/input/tablet/pegasus_notetaker.c | 373 +++ 3 files changed, 389 insertions(+) create mode 100644 drivers/input/tablet/pegasus_notetaker.c diff --git a/drivers/input/tablet/Kconfig b/drivers/input/tablet/Kconfig index 623bb9e..a2b9f97 100644 --- a/drivers/input/tablet/Kconfig +++ b/drivers/input/tablet/Kconfig @@ -73,6 +73,21 @@ config TABLET_USB_KBTAB To compile this driver as a module, choose M here: the module will be called kbtab. +config TABLET_USB_PEGASUS + tristate "Pegasus Mobile Notetaker Pen input tablet support" + depends on USB_ARCH_HAS_HCD + select USB + help + Say Y here if you want to use the Pegasus Mobile Notetaker, + also known as: + Genie e-note The Notetaker, + Staedtler Digital ballpoint pen 990 01, + IRISnotes Express or + NEWLink Digital Note Taker. + + To compile this driver as a module, choose M here: the + module will be called pegasus_notetaker. + config TABLET_SERIAL_WACOM4 tristate "Wacom protocol 4 serial tablet support" select SERIO diff --git a/drivers/input/tablet/Makefile b/drivers/input/tablet/Makefile index 2e13010..200fc4e 100644 --- a/drivers/input/tablet/Makefile +++ b/drivers/input/tablet/Makefile @@ -8,4 +8,5 @@ obj-$(CONFIG_TABLET_USB_AIPTEK) += aiptek.o obj-$(CONFIG_TABLET_USB_GTCO) += gtco.o obj-$(CONFIG_TABLET_USB_HANWANG) += hanwang.o obj-$(CONFIG_TABLET_USB_KBTAB) += kbtab.o +obj-$(CONFIG_TABLET_USB_PEGASUS) += pegasus_notetaker.o obj-$(CONFIG_TABLET_SERIAL_WACOM4) += wacom_serial4.o diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c new file mode 100644 index 000..b7bf585 --- /dev/null +++ b/drivers/input/tablet/pegasus_notetaker.c @@ -0,0 +1,373 @@ +/* + * Pegasus Mobile Notetaker Pen input tablet driver + * + * Copyright (c) 2016 Martin Kepplinger + */ + +/* + * request packet (control endpoint): + * |-| + * | Report ID | Nr of bytes | command | + * | (1 byte) | (1 byte)| (n bytes) | + * |-| + * | 0x02 | n | | + * |-| + * + * data packet after set xy mode command, 0x80 0xb5 0x02 0x01 + * and pen is in range: + * + * bytebyte name value (bits) + * + * 0 status 0 1 0 0 0 0 X X + * 1 color 0 0 0 0 H 0 S T + * 2 X low + * 3 X high + * 4 Y low + * 5 Y high + * + * X X battery state: + * no state reported 0x00 + * battery low 0x01 + * battery good0x02 + * + * H Hovering + * S Switch 1 (pen button) + * T Tip + */ + +#include +#include +#include +#include + +/* USB HID defines */ +#define USB_REQ_GET_REPORT 0x01 +#define USB_REQ_SET_REPORT 0x09 + +#define USB_VENDOR_ID_PEGASUSTECH 0x0e20 +#define USB_DEVICE_ID_PEGASUS_NOTETAKER_EN100 0x0101 + +/* device specific defines */ +#define NOTETAKER_REPORT_ID0x02 +#define NOTETAKER_SET_CMD 0x80 +#define NOTETAKER_SET_MODE 0xb5 + +#define NOTETAKER_LED_MOUSE 0x02 +#define PEN_MODE_XY 0x01 + +#define SPECIAL_COMMAND0x80 +#define BUTTON_PRESSED 0xb5 +#
Re: [PATCH] dwc3-exynos: Fix deferred probing storm.
On 05/24/2016 08:13 PM, Steinar H. Gunderson wrote:
> dwc3-exynos has two problems during init if the regulators are slow
> to come up (for instance if the I2C bus driver is not on the initramfs)
> and return probe deferral. First, every time this happens, the driver
> leaks the USB phys created; they need to be deallocated on error.
>
> Second, since the phy devices are created before the regulators fail,
> this means that there's a new device to re-trigger deferred probing,
> which causes it to essentially go into a busy loop of re-probing the
> device until the regulators come up.
>
> Move the phy creation to after the regulators have succeeded, and also
> fix cleanup on failure. On my ODROID XU4 system (with Debian's initramfs
> which doesn't contain the I2C driver), this reduces the number of probe
> attempts (for each of the two controllers) from more than 2000 to eight.
>
> Reported-by: Steinar H. Gunderson
> Signed-off-by: Steinar H. Gunderson
This is the same person so no need for "Reported-by". The reported-by is
used if someone else submits patch after your bug report.
Please (when resubmitting or applying) add following tags:
Fixes: d720f057fda4 ("usb: dwc3: exynos: add nop transceiver support")
Cc:
This will help downstream (like Debian) using stable kernels.
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 19 +++
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index dd5cb55..2f1fb7e 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -128,12 +128,6 @@ static int dwc3_exynos_probe(struct platform_device
> *pdev)
>
> platform_set_drvdata(pdev, exynos);
>
> - ret = dwc3_exynos_register_phys(exynos);
> - if (ret) {
> - dev_err(dev, "couldn't register PHYs\n");
> - return ret;
> - }
> -
> exynos->dev = dev;
>
> exynos->clk = devm_clk_get(dev, "usbdrd30");
> @@ -183,20 +177,29 @@ static int dwc3_exynos_probe(struct platform_device
> *pdev)
> goto err3;
> }
>
> + ret = dwc3_exynos_register_phys(exynos);
> + if (ret) {
> + dev_err(dev, "couldn't register PHYs\n");
> + goto err4;
> + }
> +
> if (node) {
> ret = of_platform_populate(node, NULL, NULL, dev);
> if (ret) {
> dev_err(dev, "failed to add dwc3 core\n");
> - goto err4;
> + goto err5;
> }
> } else {
> dev_err(dev, "no device node, failed to add dwc3 core\n");
> ret = -ENODEV;
> - goto err4;
> + goto err5;
> }
>
> return 0;
>
> +err5:
> + platform_device_unregister(exynos->usb2_phy);
> + platform_device_unregister(exynos->usb3_phy);
> err4:
> regulator_disable(exynos->vdd10);
> err3:
>
--
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 5/5] usb: dwc3: rockchip: add devicetree bindings documentation
Hi Felipe,
On 05/24/2016 05:32 PM, Felipe Balbi wrote:
Hi,
William Wu writes:
This patch documents the device tree documentation required for
Rockchip USB3.0 core wrapper consist of USB3.0 IP from Synopsys.
It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).
Signed-off-by: William Wu
---
Changes in v2:
- add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (Felipe, Brian)
.../devicetree/bindings/usb/rockchip,dwc3.txt | 45 ++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
new file mode 100644
index 000..10303d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
@@ -0,0 +1,45 @@
+Rockchip SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible: should contain "rockchip,dwc3"
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ "clk_usb3otg0_ref" Controller reference clk
+ "clk_usb3otg0_suspend"Controller suspend clk, can use 24 MHz or 32 KHz
+ "aclk_usb3"Master/Core clock, have to be >= 62.5 MHz for SS
operation
+
+
+Optional clocks:
+ "aclk_usb3otg0"Aclk for specific usb controller clock.
+ "aclk_usb3_rksoc_axi_perf" USB AXI perf clock. Not present on all
platforms.
+ "aclk_usb3_grf"USB grf clock. Not present on all platforms.
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Phy documentation is provided in the following places:
+
+Example device nodes:
+
+ usbdrd3_0: usb@fe80 {
+
no reg property?
For now, we don't need reg property here. Because we only need to do
enable some clocks and populate its children in
drivers/usb/dwc3/dwc3-of-simple.c.
And it's similar to arch/arm/boot/dts/exynos5420.dtsi usbdrd3_0 node.
compatible = "rockchip,dwc3";
+ clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+<&cru ACLK_USB3>, <&cru ACLK_USB3OTG0>,
+<&cru ACLK_USB3_RKSOC_AXI_PERF>, <&cru ACLK_USB3_GRF>;
+ clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
+ "aclk_usb3", "aclk_usb3otg0",
+ "aclk_usb3_rksoc_axi_perf", "aclk_usb3_grf";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+ status = "disabled";
+ usbdrd_dwc3_0: dwc3 {
no address here?
I think here don't necessarily need address. The child node dwc3 can
inherit address from the parent node.
And with this dtsi patch, the dev path show as follows:
/sys/devices/platform/usb@fe80/fe80.dwc3
Is it need for coding style or other reason?
+ compatible = "snps,dwc3";
+ reg = <0x0 0xfe80 0x0 0x10>;
+ interrupts = ;
+ dr_mode = "otg";
+ status = "disabled";
+ };
+ };
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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 5/5] usb: dwc3: rockchip: add devicetree bindings documentation
Hi,
William Wu writes:
> Hi Felipe,
>
> On 05/24/2016 05:32 PM, Felipe Balbi wrote:
>> Hi,
>>
>> William Wu writes:
>>> This patch documents the device tree documentation required for
>>> Rockchip USB3.0 core wrapper consist of USB3.0 IP from Synopsys.
>>>
>>> It could operate in device mode (SS, HS, FS) and host
>>> mode (SS, HS, FS, LS).
>>>
>>> Signed-off-by: William Wu
>>> ---
>>> Changes in v2:
>>> - add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (Felipe,
>>> Brian)
>>>
>>> .../devicetree/bindings/usb/rockchip,dwc3.txt | 45
>>> ++
>>> 1 file changed, 45 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
>>> b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
>>> new file mode 100644
>>> index 000..10303d9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
>>> @@ -0,0 +1,45 @@
>>> +Rockchip SuperSpeed DWC3 USB SoC controller
>>> +
>>> +Required properties:
>>> +- compatible: should contain "rockchip,dwc3"
>>> +- clocks: A list of phandle + clock-specifier pairs for the
>>> + clocks listed in clock-names
>>> +- clock-names: Should contain the following:
>>> + "clk_usb3otg0_ref" Controller reference clk
>>> + "clk_usb3otg0_suspend"Controller suspend clk, can use 24 MHz or 32 KHz
>>> + "aclk_usb3" Master/Core clock, have to be >= 62.5 MHz for
>>> SS operation
>>> +
>>> +
>>> +Optional clocks:
>>> + "aclk_usb3otg0" Aclk for specific usb controller clock.
>>> + "aclk_usb3_rksoc_axi_perf" USB AXI perf clock. Not present on all
>>> platforms.
>>> + "aclk_usb3_grf" USB grf clock. Not present on all platforms.
>>> +
>>> +Required child node:
>>> +A child node must exist to represent the core DWC3 IP block. The name of
>>> +the node is not important. The content of the node is defined in dwc3.txt.
>>> +
>>> +Phy documentation is provided in the following places:
>>> +
>>> +Example device nodes:
>>> +
>>> + usbdrd3_0: usb@fe80 {
>>> +
>> no reg property?
> For now, we don't need reg property here. Because we only need to do
> enable some clocks and populate its children in
> drivers/usb/dwc3/dwc3-of-simple.c.
> And it's similar to arch/arm/boot/dts/exynos5420.dtsi usbdrd3_0 node.
>> compatible = "rockchip,dwc3";
>>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
>>> +<&cru ACLK_USB3>, <&cru ACLK_USB3OTG0>,
>>> +<&cru ACLK_USB3_RKSOC_AXI_PERF>, <&cru ACLK_USB3_GRF>;
>>> + clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
>>> + "aclk_usb3", "aclk_usb3otg0",
>>> + "aclk_usb3_rksoc_axi_perf", "aclk_usb3_grf";
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + ranges;
>>> + status = "disabled";
>>> + usbdrd_dwc3_0: dwc3 {
>> no address here?
> I think here don't necessarily need address. The child node dwc3 can
> inherit address from the parent node.
> And with this dtsi patch, the dev path show as follows:
> /sys/devices/platform/usb@fe80/fe80.dwc3
>
> Is it need for coding style or other reason?
I don't think your arguments match what devicetree folks want to see in
DT. Let's ask them. Rob, care to look at this one?
>
>>
>>> + compatible = "snps,dwc3";
>>> + reg = <0x0 0xfe80 0x0 0x10>;
>>> + interrupts = ;
>>> + dr_mode = "otg";
>>> + status = "disabled";
>>> + };
>>> + };
>>> --
>>> 1.9.1
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> --
> 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
--
balbi
signature.asc
Description: PGP signature
usb 1-2: reset high-speed USB device number 3 using ehci-pci
Hi, I am trying to track down an issue that happen during Debian Installation from a USB key onto a Mac Mini G4 (PPC) [*]. Basically the USB key is working nicely (`badblocks` is fine), however under this specific circumstances, the USB key needs to be physically removed and then re-plugged so that it can be read. dmesg log excerpt: [4.259597] scsi 2:0:0:0: Direct-Access SanDisk Cruzer Mini 0.2 PQ: 0 ANSI: 2 [4.265282] sd 2:0:0:0: [sdb] 2001888 512-byte logical blocks: (1.02 GB/977 MiB) [4.267027] sd 2:0:0:0: [sdb] Write Protect is off [4.267040] sd 2:0:0:0: [sdb] Mode Sense: 03 00 00 00 [4.268149] sd 2:0:0:0: [sdb] No Caching mode page found [4.268162] sd 2:0:0:0: [sdb] Assuming drive cache: write through [4.393824] usb 1-2: reset high-speed USB device number 3 using ehci-pci [4.653835] usb 1-2: reset high-speed USB device number 3 using ehci-pci [4.913832] usb 1-2: reset high-speed USB device number 3 using ehci-pci [5.177821] usb 1-2: reset high-speed USB device number 3 using ehci-pci [5.437844] usb 1-2: reset high-speed USB device number 3 using ehci-pci [5.697847] usb 1-2: reset high-speed USB device number 3 using ehci-pci [5.831893] sd 2:0:0:0: [sdb] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [5.831913] sd 2:0:0:0: [sdb] tag#0 CDB: Read(10) 28 00 00 00 00 00 00 00 08 00 [5.831919] blk_update_request: I/O error, dev sdb, sector 0 [5.831926] Buffer I/O error on dev sdb, logical block 0, async page read I failed so far to reproduce it, since anytime I try to `mount` it it works. Comments welcome on how to track this down ? Thanks, [*] https://bugs.debian.org/825171#10 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] input: tablet: add Pegasus Notetaker tablet driver
On Wed, 2016-05-25 at 09:44 +0200, Martin Kepplinger wrote:
> This adds a driver for the Pegasus Notetaker Pen. When connected,
> this uses the Pen as an input tablet.
>
> This device was sold in various different brandings, for example
> "Pegasus Mobile Notetaker M210",
> "Genie e-note The Notetaker",
> "Staedtler Digital ballpoint pen 990 01",
> "IRISnotes Express" or
> "NEWLink Digital Note Taker".
>
> Here's an example, so that you know what we are talking about:
> http://www.staedtler.com/en/products/ink-writing-instruments/ballpoint-pens/digital-pen-990-01-digital-ballpoint-pen
>
> http://pegatech.blogspot.com/ seems to be a remaining official resource.
>
> This device can also transfer saved (offline recorded handwritten) data and
> there are userspace programs that do this, see https://launchpad.net/m210
> (Well, alternatively there are really fast scanners out there :)
>
> It's *really* fun to use as an input tablet though! So let's support this
> for everybody.
>
> There's no way to disable the device. When the pen is out of range, we just
> don't get any URBs and don't do anything.
> Like all other mouses or input tablets, we don't use runtime PM.
>
> Signed-off-by: Martin Kepplinger
> ---
>
> Thanks for having a look. Any more suggestions on this?
>
> revision history
>
> v4 use normal work queue instead of a kernel thread (thanks to Oliver Neukum)
> v3 fix reporting low pen battery and add USB list to CC
> v2 minor cleanup (remove unnecessary variables)
> v1 initial release
>
Hi,
almost there.
Regards
Oliver
> +static void pegasus_close(struct input_dev *dev)
> +{
> + struct pegasus *pegasus = input_get_drvdata(dev);
> +
> + cancel_work_sync(&pegasus->init);
> +
> + usb_kill_urb(pegasus->irq);
> +}
This is a race condition. The URB can trigger the work.
Therefore the URB needs to die first.
--
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 5/5] usb: dwc3: rockchip: add devicetree bindings documentation
Hi Felipe & Rob,
On 05/25/2016 04:04 PM, Felipe Balbi wrote:
Hi,
William Wu writes:
Hi Felipe,
On 05/24/2016 05:32 PM, Felipe Balbi wrote:
Hi,
William Wu writes:
This patch documents the device tree documentation required for
Rockchip USB3.0 core wrapper consist of USB3.0 IP from Synopsys.
It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).
Signed-off-by: William Wu
---
Changes in v2:
- add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (Felipe, Brian)
.../devicetree/bindings/usb/rockchip,dwc3.txt | 45
++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
new file mode 100644
index 000..10303d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
@@ -0,0 +1,45 @@
+Rockchip SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible: should contain "rockchip,dwc3"
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ "clk_usb3otg0_ref" Controller reference clk
+ "clk_usb3otg0_suspend"Controller suspend clk, can use 24 MHz or 32 KHz
+ "aclk_usb3"Master/Core clock, have to be >= 62.5 MHz for SS
operation
+
+
+Optional clocks:
+ "aclk_usb3otg0"Aclk for specific usb controller clock.
+ "aclk_usb3_rksoc_axi_perf" USB AXI perf clock. Not present on all
platforms.
+ "aclk_usb3_grf"USB grf clock. Not present on all platforms.
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Phy documentation is provided in the following places:
+
+Example device nodes:
+
+ usbdrd3_0: usb@fe80 {
+
no reg property?
For now, we don't need reg property here. Because we only need to do
enable some clocks and populate its children in
drivers/usb/dwc3/dwc3-of-simple.c.
And it's similar to arch/arm/boot/dts/exynos5420.dtsi usbdrd3_0 node.
compatible = "rockchip,dwc3";
+ clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+<&cru ACLK_USB3>, <&cru ACLK_USB3OTG0>,
+<&cru ACLK_USB3_RKSOC_AXI_PERF>, <&cru ACLK_USB3_GRF>;
+ clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
+ "aclk_usb3", "aclk_usb3otg0",
+ "aclk_usb3_rksoc_axi_perf", "aclk_usb3_grf";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+ status = "disabled";
+ usbdrd_dwc3_0: dwc3 {
no address here?
I think here don't necessarily need address. The child node dwc3 can
inherit address from the parent node.
And with this dtsi patch, the dev path show as follows:
/sys/devices/platform/usb@fe80/fe80.dwc3
Is it need for coding style or other reason?
I don't think your arguments match what devicetree folks want to see in
DT. Let's ask them. Rob, care to look at this one?
Sorry, I need to correct myself. I have done some test, and the result
shows that
the child node dwc3 don't inherit address from the parent node, but get
address
from its reg property. And It seems that whether I add address here or
not, the
dwc3 node always get address from reg property.
However, I don't know much about the DT. But I think it's better to add
address here than no.
+ compatible = "snps,dwc3";
+ reg = <0x0 0xfe80 0x0 0x10>;
+ interrupts = ;
+ dr_mode = "otg";
+ status = "disabled";
+ };
+ };
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
--
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: AW: ffs-test fails with warning (-19) No such device
On 05/24/2016 05:31 PM, jan.hu...@k2l.de wrote: >> You shouldn't use epX where X != 0 until you get enable event. You may >> refer to ffs-aio examples which have a suitable flag to wait for that event. > > I got the ffs-aio example (simple and multibuff) to work. > I also used the ffs-aio example as a reference and tried to get the ffs-test > example to work. So I am waiting for both events (Event BIND, Event ENABLE) > before accessing epX where X != 0. > However I still get the same error (-19) No such device when using ffs-test > example. > > For testing I added a read()-operation to the working ffs-aio-example to make > sure the endpoint descriptors are not faulty in ffs-test. Also here, I get > the same error (-19). > >>From my understanding, FunctionFS is the successor of GadgetFS. It provides a >>file for each endpoint (ep0, ep1, ep2, ...). The endpoints can then be >>accessed with read() / write() operations - please correct me if I'm wrong. > > Questions remaining: > 1) Why does the ffs-test example not work even when waiting for Event > BIND/Event ENABLE before accessing epX where X != 0? > 2) Is it not possible to access endpoints with read()/write() > 3) if 2) is not possible, how did ffs-test ever work? How can the endpoints > be accessed now? It should be possible and this example should work. could you please test your setup with the newest kernel release? around 4.1 there was some refactoring of IO functions in functionfs. There is at least one patch which fix this[1] but I'm not sure if it will fix anything for you as you have different error code. So once again please check newer kernel. Footnotes: 1 - http://permalink.gmane.org/gmane.linux.usb.general/139316 Best regards, -- Krzysztof Opasiak Samsung R&D 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
[PATCH] chaoskey: Be conservative with the initial estimate of quality.
There is a principal problem with randomness gained from wired
USB devices. The kernel needs to be sure that the device actually
is what it claims to be. (Wirebound) USB by itself does not
provide a means of authentication. Anything connected is trusted.
Making sure that a device really is what it claims to be is left
to the individual drivers. If that cannot be done in kernel
space, the admin still can call a device trustworthy.
To guard the entropy pool against malicious spoof we assume
the quality of an unverified source's entropy to be 0.
Signed-off-by: Oliver Neukum
---
drivers/usb/misc/chaoskey.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index 76350e4..fcfd769 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -84,6 +84,7 @@ struct chaoskey {
int open; /* open count */
bool present; /* device not disconnected */
bool reading; /* ongoing IO */
+ bool verified; /* the device is sure to be genuine */
int size; /* size of buf */
int valid; /* bytes of buf read */
int used; /* bytes of buf consumed */
@@ -207,7 +208,12 @@ static int chaoskey_probe(struct usb_interface *interface,
dev->hwrng.name = dev->name ? dev->name : chaoskey_driver.name;
dev->hwrng.read = chaoskey_rng_read;
- /* Set the 'quality' metric. Quality is measured in units of
+ /*
+* There is a problem here. We need to be sure that the other side
+* is a genuine chaoskey. So to be on the safe side we need to set
+* the initial quality to 0 if we are not. An admin can raise it later.
+*
+* Quality is measured in units of
* 1/1024's of a bit ("mills"). This should be set to 1024,
* but there is a bug in the hwrng core which masks it with
* 1023.
@@ -218,7 +224,7 @@ static int chaoskey_probe(struct usb_interface *interface,
* merged and 1024 afterwards. We'll patch this driver once
* both bits of code are in the same tree.
*/
- dev->hwrng.quality = 1024 + 1023;
+ dev->hwrng.quality = dev->verified ? 1024 + 1023 : 0;
dev->hwrng_registered = (hwrng_register(&dev->hwrng) == 0);
if (!dev->hwrng_registered)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: uas: Fix slave queue_depth not being set
Hi,
On 24-05-16 14:44, James Bottomley wrote:
On Tue, 2016-05-24 at 08:53 +0200, Hans de Goede wrote:
Hi,
On 23-05-16 19:36, James Bottomley wrote:
On Mon, 2016-05-23 at 13:49 +0200, Hans de Goede wrote:
Commit 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host
level")
removed the scsi_change_queue_depth() call from
uas_slave_configure() assuming that the slave would inherit the
host's queue_depth, which that commit sets to the same value.
This is incorrect, without the scsi_change_queue_depth() call the
slave's queue_depth defaults to 1, introducing a performance
regression.
This commit restores the call, fixing the performance regression.
Cc: sta...@vger.kernel.org
Fixes: 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host
level")
Reported-by: Tom Yan
Signed-off-by: Hans de Goede
---
drivers/usb/storage/uas.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/storage/uas.c
b/drivers/usb/storage/uas.c
index 16bc679..ecc7d4b 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -835,6 +835,7 @@ static int uas_slave_configure(struct
scsi_device
*sdev)
if (devinfo->flags & US_FL_BROKEN_FUA)
sdev->broken_fua = 1;
+ scsi_change_queue_depth(sdev, devinfo->qdepth - 2);
Are you sure about this? For spinning rust, experiments imply that
the optimal queue depth per device is somewhere between 2 and 4.
Obviously that's not true for SSDs, so it depends on your use
case. Plus, for ATA NCQ devices (which I believe most UAS is
bridged to) you have a maximum NCQ depth of 31.
So this value is the same as host.can_queue, and is what uas has
always used, basically this says it is ok to queue as much as the
bridge can handle. We've seen a few rare multi-lun devices, but
typically almost all uas devices have one lun, what I really want to
do here is give a maximum and let say the sd driver lower that if it
is sub-optimal.
If that's what you actually want, you should be setting sdev
->max_queue_depth and .track_queue_depth = 1 in the template.
Hmm, I've been looking into this, but that does not seem right.
max_queue_depth is never set by drivers, it is set to sdev->queue_depth
in scsi_scan.c: scsi_add_lun() after calling the host drivers'
slave_configure callback. So it seems that the right way to set
max_queue_depth is to actually set queue_depth, or iow restore the
call to scsi_change_queue_depth() as the patch we're discussing does.
As for track_queue_depth = 1 that seems to be only set by some drivers
under drivers/scsi and is never set by any drivers under drivers/ata,
and we're almost exclusively dealing with sata disks in uas. It seems
that track_queue_depth = 1 is mostly used for iscsi and fibre channel
iow enterprise class storage stuff, so looking at existing drivers
usage of this flag using it does not seem a good idea.
Anyways this patch is a (partial) revert of a previous bug-fix patch
(which has also gone to stable) so for now I would really like to
move forward with this patch and get it upstream and in stable
to fix the performance regressions people are seeing caused by
me wrongly dropping the scsi_change_queue_depth() call.
Then if we want to tweak the queuing further we can do that
on top of this fix, and put that in next and let it get some testing
first.
So are you ok with moving this patch forward ?
Regards,
Hans
You might also need to add calls to scsi_track_queue_full() but only if
the devices aren't responding QUEUE_FULL correctly.
James
Also notice that uas is used a lot with ssd-s, that is mostly what
I want to optimize for, but it is definitely also used with spinning
rust.
And yes almost all uas devices are bridged sata devices (this may
change in the near future though, with ssd-s specifically designed
for usb-3 attachment, although sofar these all seem to use an
embbeded sata bridge), so from this pov an upper limit of 31 makes
sense, I guess, but I've not seen any bridges which actually do more
then 32 streams anyways.
Still this is a bug-fix patch, essentially a partial revert, to
address performance regressions, so lets get this out as is and take
our time to come up with some tweaks (if necessary) for the say 4.8.
There's a good reason why you don't want a queue deeper than you
can handle: it tends to interfere with writeback because you build
up a lot of pending I/O in the queue which can't be issued (it's
very similar to why bufferbloat is a problem in networks). In
theory, as long as your devices return the correct indicator
(QUEUE_FULL status), we'll handle most of this in the mid-layer by
plugging the block queue, but given what I've seen from UAS
devices, that's less than probable.
So any smart ideas how to be nicer to spinning rust, without
negatively impacting ssd-s? As said if I've to choice I think we
should chose optimizing ssd-s, as that is where uas is used a lot
(although usb attached harddisks are switching over to it too).
Note I just checked the 1TB sata/ahci
Re: [PATCH v8 2/7] usb: mux: add generic code for dual role port mux
Hi Baolu,
Sorry to comment this so late, but we got hardware that needs to
configure the mux in OS, and I noticed some problem. We are missing
means to bind a port to the correct mux on multiport systems. That we
need to solve later in any case, but there is an other issue related
to the fact that the notifiers now have to be extcon devices. It's
related, as extcon offers no means to solve the multiport issue, but
in any case..
> +struct portmux_dev *portmux_register(struct portmux_desc *desc)
> +{
> + static atomic_t portmux_no = ATOMIC_INIT(-1);
> + struct portmux_dev *pdev;
> + struct extcon_dev *edev = NULL;
> + struct device *dev;
> + int ret;
> +
> + /* parameter sanity check */
> + if (!desc || !desc->name || !desc->ops || !desc->dev ||
> + !desc->ops->cable_set_cb || !desc->ops->cable_unset_cb)
> + return ERR_PTR(-EINVAL);
> +
> + dev = desc->dev;
> +
> + if (desc->extcon_name) {
> + edev = extcon_get_extcon_dev(desc->extcon_name);
> + if (IS_ERR_OR_NULL(edev))
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> + if (!pdev)
> + return ERR_PTR(-ENOMEM);
> +
> + pdev->desc = desc;
> + pdev->edev = edev;
> + pdev->nb.notifier_call = usb_mux_notifier;
> + mutex_init(&pdev->mux_mutex);
> +
> + pdev->dev.parent = dev;
> + dev_set_name(&pdev->dev, "portmux.%lu",
> + (unsigned long)atomic_inc_return(&portmux_no));
> + pdev->dev.groups = portmux_group;
> + ret = device_register(&pdev->dev);
> + if (ret)
> + goto cleanup_mem;
> +
> + dev_set_drvdata(&pdev->dev, pdev);
> +
> + if (edev) {
> + ret = extcon_register_notifier(edev, EXTCON_USB_HOST,
> +&pdev->nb);
> + if (ret < 0) {
> + dev_err(dev, "failed to register extcon notifier\n");
> + goto cleanup_dev;
> + }
> + }
So I don't actually think this is correct approach. We are forcing the
notifying drivers, on top of depending on this framework, depend on
extcon too, and that simply is too much. I don't think a USB PHY or
charger detection driver should be forced to generate an extcon device
just to satisfy the mux in general.
Instead IMO, this framework should provide an API also for the
notifiers. The drivers that do the notification should not need to
depend on extcon at all. Instead they should be able to just request
an optional handle to a portmux, and use it with the function that you
already provide (usb_mux_change_state(), which of course needs to be
exported). That would make it much easier for us to make problems like
figuring out the correct mux for the correct port a problem for the
framework and not the drivers.
extcon does not really add any value here, but it does complicate
things a lot. We are even exposing new sysfs attributes to control the
mux, complete separate from extcon.
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 v5 2/5] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
On Tuesday 10 May 2016 10:14 PM, David Lechner wrote:
> On 05/10/2016 06:26 AM, Sergei Shtylyov wrote:
>> On 5/10/2016 2:46 AM, David Lechner wrote:
>>
>>> The CFGCHIP registers are used by a number of devices, so using a syscon
>>> device to share them. The first consumer of this will by the
>>> phy-da8xx-usb
>>> driver.
>>>
>>> Signed-off-by: David Lechner
>> [...]
>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c
>>> b/arch/arm/mach-davinci/devices-da8xx.c
>>> index 725e693..69d11a1 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -11,6 +11,7 @@
>>> * (at your option) any later version.
>>> */
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -1109,3 +1110,30 @@ int __init da850_register_sata(unsigned long
>>> refclkpn)
>>> return platform_device_register(&da850_sata_device);
>>> }
>>> #endif
>>> +
>>> +static struct syscon_platform_data da8xx_cfgchip_platform_data = {
>>> +.label= "cfgchip",
>>> +};
>>> +
>>> +static struct resource da8xx_cfgchip_resources[] = {
>>> +{
>>> +.start= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP0_REG,
>>> +.end= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP4_REG + 3,
>>> +.flags= IORESOURCE_MEM,
>>> +},
>>> +};
>>> +
>>> +static struct platform_device da8xx_cfgchip_device = {
>>> +.name= "syscon",
>>> +.id= 0,
>>
>> Again, -1.
>>
>> [...]
>>
>> MBR, Sergei
>>
>
> I wish you would have noticed this when I first submitted it. I remember
> going back and forth about this. But it has been too long and I can't
> remember the reason why I chose to go this way.
>
> It seems like changing it broke something with either this one or the
> phy device and I opted to keep it this way on both to be consistent. For
> example, the USB devices both use id = 0 as well even though there are
> only one of each type.
Agree with Sergei here. Can you confirm what broke exactly? I think the
USB needs to be fixed too.
Thanks,
Sekhar
--
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 v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
On Monday 23 May 2016 08:44 PM, David Lechner wrote: > On 05/09/2016 06:46 PM, David Lechner wrote: >> v5 changes: renamed "usbphy" to "usb_phy" or "usb-phy" as appropriate >> >> David Lechner (5): >>ARM: davinci: da8xx: add usb phy clocks >>ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration. >>ARM: davinci: da8xx: Add USB PHY platform declaration >>ARM: DTS: da850: Add cfgchip syscon node >>ARM: DTS: da850: Add usb phy node >> >> arch/arm/boot/dts/da850.dtsi| 9 ++ >> arch/arm/mach-davinci/board-da830-evm.c | 52 +++ >> arch/arm/mach-davinci/board-da850-evm.c | 4 + >> arch/arm/mach-davinci/board-mityomapl138.c | 4 + >> arch/arm/mach-davinci/board-omapl138-hawk.c | 23 ++- >> arch/arm/mach-davinci/devices-da8xx.c | 28 >> arch/arm/mach-davinci/include/mach/da8xx.h | 6 + >> arch/arm/mach-davinci/usb-da8xx.c | 230 >> +++- >> 8 files changed, 314 insertions(+), 42 deletions(-) >> > > What should I be doing to keep this moving along? We need the related driver changes to be applied first. I could then use an immutable branch to push the platform changes against. I did take a look at the patches and they look good to me. Except the one comment from Sergei which I just now indicated that I agree with. Thanks, Sekhar -- 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 PATCHv2] usb: USB Type-C Connector Class
Hi, On Tue, May 24, 2016 at 02:51:40PM +0200, Oliver Neukum wrote: > On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote: > > Hi, > > as this discussion seems to go in circles, I am starting anew > at the top. > > > Like I've told some of you guys, I'm trying to implement a bus for > > the Alternate Modes, but I'm still nowhere near finished with that > > one, so let's just get the class ready now. The altmode bus should in > > any case not affect the userspace interface proposed in this patch. > > > > As you can see, the Alternate Modes are handled completely differently > > compared to the original proposal. Every Alternate Mode will have > > their own device instance (which will be then later bound to an > > Alternate Mode specific driver once we have the bus), but also every > > partner, cable and cable plug will have their own device instances > > representing them. > > The API works for a DFP. I fail to see how the UFP learns about entering > an alternate mode. > Secondly, support to trigger a reset is missing I'm fine with adding an attribute for port and cable resets if it's something that is needed. So do you want to be able to execute hard reset on a port? But could you please explain the case(s) where you need to tricker a reset. > > An other change is that the data role is now handled in two ways. > > The current_data_role file will represent static mode of the port, and > > it will use the names for the roles as they are defined in the spec: > > DFP, UFP and DRP. This file should be used if the port needs to be > > Good, but support for Try.SRC and Try.SNK is missing. OK, but what is Try.SNK? It's not in the specs? > An additional problem with that is that it needs to work > without user space during boot. So I think module parameters > to set the default are necessary. I don't have a problem with that, but what does Greg say? > > fixed to one specific role with DRP ports. So this approach will > > replace the suggestions for "preferred" data role we had. The > > current_usb_data_role will use values "host" and "device" and it will > > be used for data role swapping when already connected. > > > > The tree of devices that will be populated when the cable is active > > and when the cable has controller on both plug, will look as > > following: > > > > usbc0 > > |- usbc0-cable > > | |- usbc0-plug0 > > | | |- usbc0-plug.svid:xxx > > | | | |-mode0 > > | | | | |- vdo > > | | | | |- desc > > | | | | |- active > > ... > > | |- usbc0-plug1 > > | | |-usbc0-partner > > | | | |- usbc0-partner.svid: > > | | | | |-mode0 > > | | | | | |- vdo > > | | | | | |- desc > > | | | | | |- active > > | | | | |-mode1 > > ... > > | | |- usbc0-plug1.svid:xxx > > | | | |-mode0 > > | | | | |- vdo > > ... > > > > If there is no active cable, the partner will be directly attached to > > the port, but symlink to the partner is now always added to the port > > folder in any case. I'm not sure about this approach. There is a > > question about it in the code. Please check it. > > This approach looks workable. > An the whole the approach looks good, but needs to be extended. 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: [RFC PATCHv2] usb: USB Type-C Connector Class
Hi,
On Tue, May 24, 2016 at 06:42:09AM -0700, Guenter Roeck wrote:
> > +struct typec_capability {
> > + enum typec_data_rolerole;
> > + unsigned intusb_pd:1;
> > + struct typec_altmode*alt_modes;
> > + unsigned intaudio_accessory:1;
> > + unsigned intdebug_accessory:1;
> > +
> > + int (*fix_role)(struct typec_port *,
> > + enum typec_data_role);
> > +
> > + int (*dr_swap)(struct typec_port *);
> > + int (*pr_swap)(struct typec_port *);
> > + int (*vconn_swap)(struct typec_port *);
> > +
>
> The function parameter in those calls is all but useless to the caller.
> It needs to store the typec_port returned from typec_register(), create a
> list of ports, and then search through this list each time one of the
> functions is called. This is quite expensive for no good reason.
>
> Previously, with typec_port exported, the called code could use the stored
> caps pointer to map to its internal data structures. This is no longer
> possible.
True, the API now is in practice broken.
> I think it would be useful to provide a better means for the called function
> to identify its context. Maybe provide a pointer to the private data in
> the registration function and use it as parameter in the callback functions ?
Sounds reasonable.
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: [RFC PATCHv2] usb: USB Type-C Connector Class
On Tue, May 24, 2016 at 12:28:26PM -0700, Guenter Roeck wrote:
> On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote:
> > The purpose of this class is to provide unified interface for user
> > space to get the status and basic information about USB Type-C
> > Connectors in the system, control data role swapping, and when USB PD
> > is available, also power role swapping and Alternate Modes.
> >
> > Signed-off-by: Heikki Krogerus
>
> [ ... ]
>
> > +
> > +static void typec_remove_partner(struct typec_port *port)
> > +{
> > + sysfs_remove_link(&port->dev.kobj, "partner");
> > + typec_unregister_altmodes(port->partner->alt_modes);
>
> This only unregisters alternate modes registered through typec_add_partner(),
> but not alternate modes registered separately. Or is the calling code expected
> to set port->partner->alt_modes when calling typec_register_altmodes()
> directly ?
The altmodes for the partner are not meant to be registered
separately. With the partners and also cable plugs the class is in
control of registering and unregistering of the altmode devices after
typec_connect() is called.
The idea was that only the ports will register the alternate modes
they support separately, but I think we have to change that too. So I
don't think we'll export the typec_un/register_altmodes() at all.
We will have to prevent any drivers from being bound to the port
alternate mode devices when we add the alternate mode bus, and I had
some idea where by making the port drivers themselves in charge of
registering the port alternate modes, we could prevent it easily. But
it's probable easier to just handle those in the class driver as well.
> > +
> > +void typec_unregister_altmodes(struct typec_altmode *alt_modes)
> > +{
> > + struct typec_altmode *alt;
> > +
> This will crash if alt_modes is NULL, which will happen if
> partner->alt_modes is NULL at connection time. Semantically
> this is different to typec_register_altmodes(), which does
> have a NULL check.
Yes, need to fix that.
Thanks Guenter,
--
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] Re: Endless "supply vcc not found, using dummy regulator"
Hi Steinar,
On 25 May 2016 at 00:54, Steinar H. Gunderson wrote:
> On Tue, May 24, 2016 at 05:53:34PM +0200, Krzysztof Kozlowski wrote:
>> exynos->clk = devm_clk_get(dev, "usbdrd30");
>> if (IS_ERR(exynos->clk)) {
>> + // On each error path since here we need to
>> + // revert work done by dwc3_exynos_register_phys()
>> dev_err(dev, "couldn't get clock\n");
>> return -EINVAL;
>> }
>> clk_prepare_enable(exynos->clk);
>
> OK, so I took Mark's advice and moved the phy instantiation towards the end
> of the function (after the regulators have successfully come up). It reduced
> the number of probes, even with the original initramfs, dramatically, so
> it seems to work quite well. It also reduces the text for each deferred probe
> by a lot, since we no longer have the dummy regulator message for each one
> (only the message about “no suspend clk specified” is left). Finally, this
> arrangement reduced the need for extra error handling to a minimum.
>
> Cc-ing Felipe and and linux-usb@, and adding the patch as a reply to this
> message.
>
> /* Steinar */
> --
> Homepage: https://www.sesse.net/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Actually their are some missing patches to tune the usb3 phy.
https://lkml.org/lkml/2014/10/31/266
Best Regards
-Anand Moon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
On 25/05/16 05:44, Peter Chen wrote:
> On Tue, May 24, 2016 at 12:45:46PM +0300, Roger Quadros wrote:
>> Hi Peter,
>>
>> I have one question here. Please see below.
>>
>> On 13/05/16 13:03, Roger Quadros wrote:
>>> It provides APIs for the following tasks
>>>
>>> - Registering an OTG/dual-role capable controller
>>> - Registering Host and Gadget controllers to OTG core
>>> - Providing inputs to and kicking the OTG state machine
>>>
>>> Provide a dual-role device (DRD) state machine.
>>> DRD mode is a reduced functionality OTG mode. In this mode
>>> we don't support SRP, HNP and dynamic role-swap.
>>>
>>> In DRD operation, the controller mode (Host or Peripheral)
>>> is decided based on the ID pin status. Once a cable plug (Type-A
>>> or Type-B) is attached the controller selects the state
>>> and doesn't change till the cable in unplugged and a different
>>> cable type is inserted.
>>>
>>> As we don't need most of the complex OTG states and OTG timers
>>> we implement a lean DRD state machine in usb-otg.c.
>>> The DRD state machine is only interested in 2 hardware inputs
>>> 'id' and 'b_sess_vld'.
>>>
>>> Signed-off-by: Roger Quadros
>>> ---
>>> drivers/usb/common/Makefile |2 +-
>>> drivers/usb/common/usb-otg.c | 1042
>>> ++
>>> drivers/usb/core/Kconfig |4 +-
>>> include/linux/usb/gadget.h |2 +
>>> include/linux/usb/hcd.h |1 +
>>> include/linux/usb/otg-fsm.h |7 +
>>> include/linux/usb/otg.h | 154 ++-
>>> 7 files changed, 1206 insertions(+), 6 deletions(-)
>>> create mode 100644 drivers/usb/common/usb-otg.c
>>>
>>
>>
>>
>>> +
>>> +/**
>>> + * usb_otg_register() - Register the OTG/dual-role device to OTG core
>>> + * @dev: OTG/dual-role controller device.
>>> + * @config: OTG configuration.
>>> + *
>>> + * Registers the OTG/dual-role controller device with the USB OTG core.
>>> + *
>>> + * Return: struct usb_otg * if success, ERR_PTR() if error.
>>> + */
>>> +struct usb_otg *usb_otg_register(struct device *dev,
>>> +struct usb_otg_config *config)
>>> +{
>>> + struct usb_otg *otg;
>>> + struct otg_wait_data *wait;
>>> + int ret = 0;
>>> +
>>> + if (!dev || !config || !config->fsm_ops)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + /* already in list? */
>>> + mutex_lock(&otg_list_mutex);
>>> + if (usb_otg_get_data(dev)) {
>>> + dev_err(dev, "otg: %s: device already in otg list\n",
>>> + __func__);
>>> + ret = -EINVAL;
>>> + goto unlock;
>>> + }
>>> +
>>> + /* allocate and add to list */
>>> + otg = kzalloc(sizeof(*otg), GFP_KERNEL);
>>> + if (!otg) {
>>> + ret = -ENOMEM;
>>> + goto unlock;
>>> + }
>>> +
>>> + otg->dev = dev;
>>> + otg->caps = config->otg_caps;
>>
>> Here, we should be checking if user needs to disable any OTG features. So,
>>
>> if (dev->of_node)
>> of_usb_update_otg_caps(dev->of_node, &otg->caps);
>>
>> Do you agree?
>> This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
>> to 'struct usb_otg_caps caps;' so that we can modify the local copy instead
>> of the one passed by the OTG controller.
>
> Why can't modify the one from OTG controller directly?
>
There are 2 things.
1) OTG features supported by hardware. This is the controller's config->otg_caps
2) OTG features needed by system designer. This can be a subset of (1).
This needs to be maintained in the OTG core, based on config->otg_caps and
device tree disable flags.
So we can't use OTG controller config->otg_caps directly.
>>
>> We can also move of_usb_update_otg_caps() to otg.h.
>>
>> We will also need to modify the udc-core code so that it sets
>> gadget->otg_caps
>> to the modified otg_caps from the OTG core. This will ensure that the right
>> OTG descriptors are sent.
>>
>> So we will have to introduce an API.
>>
>> struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
>>
>> And in udc-core.c,
>>
>> static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver
>> *driver)
>> {
>> ..
>> ret = driver->bind(udc->gadget, driver);
>> if (ret)
>> goto err1;
>>
>> /* If OTG, the otg core starts the UDC when needed */
>> if (udc->gadget->otg_dev) {
>> +udc->gadget->is_otg = true;
>
> gadget->is_otg is only set to true if fully OTG is supported and it
> needs to send OTG descriptors at this case. DRD devices should not send OTG
> descriptors.
Agreed.
>
>> +udc->gadget->otg_caps =
>> usb_otg_get_otg_caps(udc->gadget->otg_dev);
>
> Getting otg capabilities should be prior to driver->bind since
> usb_otg_descriptor_init is called at that. Besides, Gadget driver
Correct.
> may be probed before otg driver is registered
udc_bind_to_driver gets called only when both, udc driver and function driver
are available. Defer probing will have to be done in
usb_add_gadget_udc_release(
Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
On 25/05/16 06:19, Jun Li wrote:
>
>
>> -Original Message-
>> From: Peter Chen [mailto:hzpeterc...@gmail.com]
>> Sent: Wednesday, May 25, 2016 10:44 AM
>> To: Roger Quadros
>> Cc: peter.c...@freescale.com; ba...@kernel.org; t...@atomide.com;
>> gre...@linuxfoundation.org; dan.j.willi...@intel.com;
>> mathias.ny...@linux.intel.com; joao.pi...@synopsys.com;
>> sergei.shtyl...@cogentembedded.com; jun...@freescale.com;
>> grygorii.stras...@ti.com; yoshihiro.shimoda...@renesas.com;
>> r...@kernel.org; nsek...@ti.com; b-...@ti.com; linux-usb@vger.kernel.org;
>> linux-o...@vger.kernel.org; linux-ker...@vger.kernel.org;
>> devicet...@vger.kernel.org
>> Subject: Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
>>
>> On Tue, May 24, 2016 at 12:45:46PM +0300, Roger Quadros wrote:
>>> Hi Peter,
>>>
>>> I have one question here. Please see below.
>>>
>>> On 13/05/16 13:03, Roger Quadros wrote:
It provides APIs for the following tasks
- Registering an OTG/dual-role capable controller
- Registering Host and Gadget controllers to OTG core
- Providing inputs to and kicking the OTG state machine
Provide a dual-role device (DRD) state machine.
DRD mode is a reduced functionality OTG mode. In this mode we don't
support SRP, HNP and dynamic role-swap.
In DRD operation, the controller mode (Host or Peripheral) is
decided based on the ID pin status. Once a cable plug (Type-A or
Type-B) is attached the controller selects the state and doesn't
change till the cable in unplugged and a different cable type is
inserted.
As we don't need most of the complex OTG states and OTG timers we
implement a lean DRD state machine in usb-otg.c.
The DRD state machine is only interested in 2 hardware inputs 'id'
and 'b_sess_vld'.
Signed-off-by: Roger Quadros
---
drivers/usb/common/Makefile |2 +-
drivers/usb/common/usb-otg.c | 1042
>> ++
drivers/usb/core/Kconfig |4 +-
include/linux/usb/gadget.h |2 +
include/linux/usb/hcd.h |1 +
include/linux/usb/otg-fsm.h |7 +
include/linux/usb/otg.h | 154 ++-
7 files changed, 1206 insertions(+), 6 deletions(-) create mode
100644 drivers/usb/common/usb-otg.c
>>>
>>>
>>>
+
+/**
+ * usb_otg_register() - Register the OTG/dual-role device to OTG
+core
+ * @dev: OTG/dual-role controller device.
+ * @config: OTG configuration.
+ *
+ * Registers the OTG/dual-role controller device with the USB OTG
>> core.
+ *
+ * Return: struct usb_otg * if success, ERR_PTR() if error.
+ */
+struct usb_otg *usb_otg_register(struct device *dev,
+ struct usb_otg_config *config) {
+ struct usb_otg *otg;
+ struct otg_wait_data *wait;
+ int ret = 0;
+
+ if (!dev || !config || !config->fsm_ops)
+ return ERR_PTR(-EINVAL);
+
+ /* already in list? */
+ mutex_lock(&otg_list_mutex);
+ if (usb_otg_get_data(dev)) {
+ dev_err(dev, "otg: %s: device already in otg list\n",
+ __func__);
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ /* allocate and add to list */
+ otg = kzalloc(sizeof(*otg), GFP_KERNEL);
+ if (!otg) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ otg->dev = dev;
+ otg->caps = config->otg_caps;
>>>
>>> Here, we should be checking if user needs to disable any OTG features.
>>> So,
>>>
>>> if (dev->of_node)
>>> of_usb_update_otg_caps(dev->of_node, &otg->caps);
>>>
>>> Do you agree?
>>> This means we need to change otg->caps from 'struct usb_otg_caps *caps;'
>>> to 'struct usb_otg_caps caps;' so that we can modify the local copy
>>> instead of the one passed by the OTG controller.
>>
>> Why can't modify the one from OTG controller directly?
>
> Yes, if user wants to disable any OTG features, this should have been
> done in 'config->otg_caps', if not, 'config->otg_caps' from controller
> driver is invalid and making no sense.
I've tried to why to Peter's mail.
>
>>
>>>
>>> We can also move of_usb_update_otg_caps() to otg.h.
>>>
>>> We will also need to modify the udc-core code so that it sets
>>> gadget->otg_caps to the modified otg_caps from the OTG core. This will
>>> ensure that the right OTG descriptors are sent.
>>>
>>> So we will have to introduce an API.
>>>
>>> struct usb_otg_caps *usb_otg_get_otg_caps(struct device *otg_dev)
>>>
>>> And in udc-core.c,
>>>
>>> static int udc_bind_to_driver(struct usb_udc *udc, struct
>>> usb_gadget_driver *driver) { ..
>>> ret = driver->bind(udc->gadget, driver);
>>> if (ret)
>>> goto err1;
>>>
>>> /* If OTG, the otg core starts the UDC when needed */
>>> if (udc->gadget->otg_dev)
Re: [RFC PATCHv2] usb: USB Type-C Connector Class
On 05/25/2016 04:30 AM, Heikki Krogerus wrote:
Hi,
On Tue, May 24, 2016 at 06:42:09AM -0700, Guenter Roeck wrote:
+struct typec_capability {
+ enum typec_data_rolerole;
+ unsigned intusb_pd:1;
+ struct typec_altmode*alt_modes;
+ unsigned intaudio_accessory:1;
+ unsigned intdebug_accessory:1;
+
+ int (*fix_role)(struct typec_port *,
+ enum typec_data_role);
+
+ int (*dr_swap)(struct typec_port *);
+ int (*pr_swap)(struct typec_port *);
+ int (*vconn_swap)(struct typec_port *);
+
The function parameter in those calls is all but useless to the caller.
It needs to store the typec_port returned from typec_register(), create a
list of ports, and then search through this list each time one of the
functions is called. This is quite expensive for no good reason.
Previously, with typec_port exported, the called code could use the stored
caps pointer to map to its internal data structures. This is no longer
possible.
True, the API now is in practice broken.
I think it would be useful to provide a better means for the called function
to identify its context. Maybe provide a pointer to the private data in
the registration function and use it as parameter in the callback functions ?
Sounds reasonable.
I'll send a follow-up patch hopefully later today which fixes all the problems
I have found so far.
Guenter
--
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 PATCHv2] usb: USB Type-C Connector Class
On 05/25/2016 04:51 AM, Heikki Krogerus wrote:
On Tue, May 24, 2016 at 12:28:26PM -0700, Guenter Roeck wrote:
On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote:
The purpose of this class is to provide unified interface for user
space to get the status and basic information about USB Type-C
Connectors in the system, control data role swapping, and when USB PD
is available, also power role swapping and Alternate Modes.
Signed-off-by: Heikki Krogerus
[ ... ]
+
+static void typec_remove_partner(struct typec_port *port)
+{
+ sysfs_remove_link(&port->dev.kobj, "partner");
+ typec_unregister_altmodes(port->partner->alt_modes);
This only unregisters alternate modes registered through typec_add_partner(),
but not alternate modes registered separately. Or is the calling code expected
to set port->partner->alt_modes when calling typec_register_altmodes()
directly ?
The altmodes for the partner are not meant to be registered
separately. With the partners and also cable plugs the class is in
control of registering and unregistering of the altmode devices after
typec_connect() is called.
The idea was that only the ports will register the alternate modes
they support separately, but I think we have to change that too. So I
don't think we'll export the typec_un/register_altmodes() at all.
We will have to prevent any drivers from being bound to the port
alternate mode devices when we add the alternate mode bus, and I had
some idea where by making the port drivers themselves in charge of
registering the port alternate modes, we could prevent it easily. But
it's probable easier to just handle those in the class driver as well.
Alternate mode discovery is an orthogonal process to the connection
state machine, and may take a while to complete. Are you saying
that the call to typec_connect() should be delayed until after
alternate mode discovery completes or times out ?
So far I call typec_connect() in SRC.Ready and SNK.Ready, and
typec_register_altmodes() after mode discovery is complete.
It is also orthogonal, meaning it is only called if and when alternate
mode discovery completes, and the alternate mode discovery state machine
is separate to the port state machine.
No problem for me to change that, just making sure that the registration
delay is understood and accepted.
Thanks,
Guenter
+
+void typec_unregister_altmodes(struct typec_altmode *alt_modes)
+{
+ struct typec_altmode *alt;
+
This will crash if alt_modes is NULL, which will happen if
partner->alt_modes is NULL at connection time. Semantically
this is different to typec_register_altmodes(), which does
have a NULL check.
Yes, need to fix that.
Thanks Guenter,
--
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 PATCHv2] usb: USB Type-C Connector Class
On Wed, May 25, 2016 at 06:21:54AM -0700, Guenter Roeck wrote:
> On 05/25/2016 04:51 AM, Heikki Krogerus wrote:
> > On Tue, May 24, 2016 at 12:28:26PM -0700, Guenter Roeck wrote:
> > > On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote:
> > > > The purpose of this class is to provide unified interface for user
> > > > space to get the status and basic information about USB Type-C
> > > > Connectors in the system, control data role swapping, and when USB PD
> > > > is available, also power role swapping and Alternate Modes.
> > > >
> > > > Signed-off-by: Heikki Krogerus
> > >
> > > [ ... ]
> > >
> > > > +
> > > > +static void typec_remove_partner(struct typec_port *port)
> > > > +{
> > > > + sysfs_remove_link(&port->dev.kobj, "partner");
> > > > + typec_unregister_altmodes(port->partner->alt_modes);
> > >
> > > This only unregisters alternate modes registered through
> > > typec_add_partner(),
> > > but not alternate modes registered separately. Or is the calling code
> > > expected
> > > to set port->partner->alt_modes when calling typec_register_altmodes()
> > > directly ?
> >
> > The altmodes for the partner are not meant to be registered
> > separately. With the partners and also cable plugs the class is in
> > control of registering and unregistering of the altmode devices after
> > typec_connect() is called.
> >
> > The idea was that only the ports will register the alternate modes
> > they support separately, but I think we have to change that too. So I
> > don't think we'll export the typec_un/register_altmodes() at all.
> >
> > We will have to prevent any drivers from being bound to the port
> > alternate mode devices when we add the alternate mode bus, and I had
> > some idea where by making the port drivers themselves in charge of
> > registering the port alternate modes, we could prevent it easily. But
> > it's probable easier to just handle those in the class driver as well.
> >
>
> Alternate mode discovery is an orthogonal process to the connection
> state machine, and may take a while to complete. Are you saying
> that the call to typec_connect() should be delayed until after
> alternate mode discovery completes or times out ?
>
> So far I call typec_connect() in SRC.Ready and SNK.Ready, and
> typec_register_altmodes() after mode discovery is complete.
> It is also orthogonal, meaning it is only called if and when alternate
> mode discovery completes, and the alternate mode discovery state machine
> is separate to the port state machine.
>
> No problem for me to change that, just making sure that the registration
> delay is understood and accepted.
I'm not against leaving the responsibility of registering the alternate
modes to the drivers. I'm a little bit worried about relying then on
the drivers to also handle the unregistering accordingly, but I can
live with that. But we just shouldn't share the responsibility of
un/registering them between the class and the drivers, so the driver
should then handle the registration always.
Oliver, what do you think?
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] USB: uas: Fix slave queue_depth not being set
What if a UAS bridge requires specific SCSI command (e.g. UNMAP) to be
issued unqueued/untagged? Would track_queue_depth help?
On 25 May 2016 at 19:04, Hans de Goede wrote:
> Hi,
>
>
> On 24-05-16 14:44, James Bottomley wrote:
>>
>> On Tue, 2016-05-24 at 08:53 +0200, Hans de Goede wrote:
>>>
>>> Hi,
>>>
>>> On 23-05-16 19:36, James Bottomley wrote:
On Mon, 2016-05-23 at 13:49 +0200, Hans de Goede wrote:
>
> Commit 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host
> level")
> removed the scsi_change_queue_depth() call from
> uas_slave_configure() assuming that the slave would inherit the
> host's queue_depth, which that commit sets to the same value.
>
> This is incorrect, without the scsi_change_queue_depth() call the
> slave's queue_depth defaults to 1, introducing a performance
> regression.
>
> This commit restores the call, fixing the performance regression.
>
> Cc: sta...@vger.kernel.org
> Fixes: 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host
> level")
> Reported-by: Tom Yan
> Signed-off-by: Hans de Goede
> ---
> drivers/usb/storage/uas.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/storage/uas.c
> b/drivers/usb/storage/uas.c
> index 16bc679..ecc7d4b 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -835,6 +835,7 @@ static int uas_slave_configure(struct
> scsi_device
> *sdev)
> if (devinfo->flags & US_FL_BROKEN_FUA)
> sdev->broken_fua = 1;
>
> + scsi_change_queue_depth(sdev, devinfo->qdepth - 2);
Are you sure about this? For spinning rust, experiments imply that
the optimal queue depth per device is somewhere between 2 and 4.
Obviously that's not true for SSDs, so it depends on your use
case. Plus, for ATA NCQ devices (which I believe most UAS is
bridged to) you have a maximum NCQ depth of 31.
>>>
>>>
>>> So this value is the same as host.can_queue, and is what uas has
>>> always used, basically this says it is ok to queue as much as the
>>> bridge can handle. We've seen a few rare multi-lun devices, but
>>> typically almost all uas devices have one lun, what I really want to
>>> do here is give a maximum and let say the sd driver lower that if it
>>> is sub-optimal.
>>
>>
>> If that's what you actually want, you should be setting sdev
>> ->max_queue_depth and .track_queue_depth = 1 in the template.
>
>
> Hmm, I've been looking into this, but that does not seem right.
>
> max_queue_depth is never set by drivers, it is set to sdev->queue_depth
> in scsi_scan.c: scsi_add_lun() after calling the host drivers'
> slave_configure callback. So it seems that the right way to set
> max_queue_depth is to actually set queue_depth, or iow restore the
> call to scsi_change_queue_depth() as the patch we're discussing does.
>
> As for track_queue_depth = 1 that seems to be only set by some drivers
> under drivers/scsi and is never set by any drivers under drivers/ata,
> and we're almost exclusively dealing with sata disks in uas. It seems
> that track_queue_depth = 1 is mostly used for iscsi and fibre channel
> iow enterprise class storage stuff, so looking at existing drivers
> usage of this flag using it does not seem a good idea.
>
> Anyways this patch is a (partial) revert of a previous bug-fix patch
> (which has also gone to stable) so for now I would really like to
> move forward with this patch and get it upstream and in stable
> to fix the performance regressions people are seeing caused by
> me wrongly dropping the scsi_change_queue_depth() call.
>
> Then if we want to tweak the queuing further we can do that
> on top of this fix, and put that in next and let it get some testing
> first.
>
> So are you ok with moving this patch forward ?
>
> Regards,
>
> Hans
>
>
>
>
>> You might also need to add calls to scsi_track_queue_full() but only if
>> the devices aren't responding QUEUE_FULL correctly.
>>
>> James
>>
>>> Also notice that uas is used a lot with ssd-s, that is mostly what
>>> I want to optimize for, but it is definitely also used with spinning
>>> rust.
>>>
>>> And yes almost all uas devices are bridged sata devices (this may
>>> change in the near future though, with ssd-s specifically designed
>>> for usb-3 attachment, although sofar these all seem to use an
>>> embbeded sata bridge), so from this pov an upper limit of 31 makes
>>> sense, I guess, but I've not seen any bridges which actually do more
>>> then 32 streams anyways.
>>>
>>> Still this is a bug-fix patch, essentially a partial revert, to
>>> address performance regressions, so lets get this out as is and take
>>> our time to come up with some tweaks (if necessary) for the say 4.8.
>>>
There's a good reason why you don't want a queue deeper than you
can handle: it tends to interfere with writeback because you build
up a lot of pending I/O in t
Re: usb 1-2: reset high-speed USB device number 3 using ehci-pci
On Wed, 25 May 2016, Mathieu Malaterre wrote: > Hi, > > I am trying to track down an issue that happen during Debian > Installation from a USB key onto a Mac Mini G4 (PPC) [*]. > > Basically the USB key is working nicely (`badblocks` is fine), however > under this specific circumstances, the USB key needs to be physically > removed and then re-plugged so that it can be read. > > dmesg log excerpt: > > [4.259597] scsi 2:0:0:0: Direct-Access SanDisk Cruzer Mini > 0.2 PQ: 0 ANSI: 2 > [4.265282] sd 2:0:0:0: [sdb] 2001888 512-byte logical blocks: > (1.02 GB/977 MiB) > [4.267027] sd 2:0:0:0: [sdb] Write Protect is off > [4.267040] sd 2:0:0:0: [sdb] Mode Sense: 03 00 00 00 > [4.268149] sd 2:0:0:0: [sdb] No Caching mode page found > [4.268162] sd 2:0:0:0: [sdb] Assuming drive cache: write through > [4.393824] usb 1-2: reset high-speed USB device number 3 using ehci-pci > [4.653835] usb 1-2: reset high-speed USB device number 3 using ehci-pci > [4.913832] usb 1-2: reset high-speed USB device number 3 using ehci-pci > [5.177821] usb 1-2: reset high-speed USB device number 3 using ehci-pci > [5.437844] usb 1-2: reset high-speed USB device number 3 using ehci-pci > [5.697847] usb 1-2: reset high-speed USB device number 3 using ehci-pci > [5.831893] sd 2:0:0:0: [sdb] tag#0 FAILED Result: > hostbyte=DID_ERROR driverbyte=DRIVER_OK > [5.831913] sd 2:0:0:0: [sdb] tag#0 CDB: Read(10) 28 00 00 00 00 00 > 00 00 08 00 > [5.831919] blk_update_request: I/O error, dev sdb, sector 0 > [5.831926] Buffer I/O error on dev sdb, logical block 0, async page read > > > I failed so far to reproduce it, since anytime I try to `mount` it it works. > > Comments welcome on how to track this down ? If you can't reproduce a failure, you can't track it down. Also, an installation kernel is not well suited to tracking failures. It's a somewhat unstable environment without all the resources that would normally be available. If you can acquire a usbmon trace showing what happens when the flash drive is plugged in and the reset occurs, that would help. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv2] usb: USB Type-C Connector Class
On Wed, 2016-05-25 at 17:04 +0300, Heikki Krogerus wrote: > I'm not against leaving the responsibility of registering the alternate > modes to the drivers. I'm a little bit worried about relying then on > the drivers to also handle the unregistering accordingly, but I can > live with that. But we just shouldn't share the responsibility of > un/registering them between the class and the drivers, so the driver > should then handle the registration always. > > Oliver, what do you think? Either will do for me. Registration by the drivers is a bit better. But it has to be the one or the other. Mixing is indeed bad. 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: [PATCH v8 08/14] usb: otg: add OTG/dual-role core
Hi Roger > >> > >> Here, we should be checking if user needs to disable any OTG > >> features. So, > >> > >>if (dev->of_node) > >>of_usb_update_otg_caps(dev->of_node, &otg->caps); > >> > >> Do you agree? > >> This means we need to change otg->caps from 'struct usb_otg_caps > *caps;' > >> to 'struct usb_otg_caps caps;' so that we can modify the local copy > >> instead of the one passed by the OTG controller. > > > > Why can't modify the one from OTG controller directly? > > > > There are 2 things. > 1) OTG features supported by hardware. This is the controller's config- > >otg_caps > 2) OTG features needed by system designer. This can be a subset of (1). Let's make things simple, we only need this subnet, which can be set by controller driver in config->otg_caps before pass (its address) to OTG core. So controller driver should get the capability of HW(+SW) and user config by whatever approach, then set its config->otg_caps. Li Jun -- 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 PATCHv2] usb: USB Type-C Connector Class
On Wed, May 25, 2016 at 04:20:56PM +0200, Oliver Neukum wrote: > On Wed, 2016-05-25 at 17:04 +0300, Heikki Krogerus wrote: > > > I'm not against leaving the responsibility of registering the alternate > > modes to the drivers. I'm a little bit worried about relying then on > > the drivers to also handle the unregistering accordingly, but I can > > live with that. But we just shouldn't share the responsibility of > > un/registering them between the class and the drivers, so the driver > > should then handle the registration always. > > > > Oliver, what do you think? > > Either will do for me. Registration by the drivers is a bit better. > But it has to be the one or the other. Mixing is indeed bad. > Same here. I don't have any problems handling unregistering from the driver. I just have to keep track of the state and call typec_unregister_altmodes() before calling typec_disconnect(). Having to wait for mode discovery to complete before calling typec_connect() is much more complicated, at least with my current code. Guenter -- 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 PATCHv2] usb: USB Type-C Connector Class
On Wed, May 25, 2016 at 02:28:46PM +0300, Heikki Krogerus wrote: > Hi, > > On Tue, May 24, 2016 at 02:51:40PM +0200, Oliver Neukum wrote: > > On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote: > > > > Hi, > > > > as this discussion seems to go in circles, I am starting anew > > at the top. > > > > > Like I've told some of you guys, I'm trying to implement a bus for > > > the Alternate Modes, but I'm still nowhere near finished with that > > > one, so let's just get the class ready now. The altmode bus should in > > > any case not affect the userspace interface proposed in this patch. > > > > > > As you can see, the Alternate Modes are handled completely differently > > > compared to the original proposal. Every Alternate Mode will have > > > their own device instance (which will be then later bound to an > > > Alternate Mode specific driver once we have the bus), but also every > > > partner, cable and cable plug will have their own device instances > > > representing them. > > > > The API works for a DFP. I fail to see how the UFP learns about entering > > an alternate mode. > > Secondly, support to trigger a reset is missing > > I'm fine with adding an attribute for port and cable resets if it's > something that is needed. So do you want to be able to execute hard > reset on a port? > > But could you please explain the case(s) where you need to tricker a > reset. > > > > An other change is that the data role is now handled in two ways. > > > The current_data_role file will represent static mode of the port, and > > > it will use the names for the roles as they are defined in the spec: > > > DFP, UFP and DRP. This file should be used if the port needs to be > > > > Good, but support for Try.SRC and Try.SNK is missing. > > OK, but what is Try.SNK? It's not in the specs? > It is not in the USB PD specification, but in "USB Type-C Specification Release 1.2 - Cable and Connector Specification". Section 4.5.2.2.11, "Try.SNK State", says "Note: if both Try.SRC and Try.SNK mechanisms are implemented, only one shall be enabled by the port at any given time. Deciding which of these two mechanisms is enabled is product design-specific." ... which is why I suggested earlier that there should be a platform parameter to enable one or the other. Guenter -- 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] chaoskey: Be conservative with the initial estimate of quality.
Oliver Neukum writes: > To guard the entropy pool against malicious spoof we assume > the quality of an unverified source's entropy to be 0. This removes most of the utility of the device, which is to provide entropy to a system early in the boot process. If you have physical security, then a USB device is just as reliable as any other piece of hardware. If you don't have physical security, then the entire system is suspect. -- -keith signature.asc Description: PGP signature
Re: [RFT PATCH] xhci: Fix handling timeouted commands on hosts in weird states.
On 05/12/2016 07:57 AM, Mathias Nyman wrote:
> If commands timeout we mark them for abortion, then stop the command
> ring, and turn the commands to no-ops and finally restart the command
> ring.
>
> If the host is working properly the no-op commands will finish and
> pending completions are called.
> If we notice the host is failing driver clears the command ring and
> completes, deletes and frees all pending commands.
>
> There are two separate cases reported where host is believed to work
> properly but is not. In the first case we successfully stop the ring
> but no abort or stop commnand ring event is ever sent and host locks up.
>
> The second case is if a host is removed, command times out and driver
> believes the ring is stopped, and assumes it be restarted, but actually
> ends up timing out on the same command forever.
> If one of the pending commands has the xhci->mutex held it will block
> xhci_stop() in the remove codepath which otherwise would cleanup pending
> commands.
>
> Add a check that clears all pending commands in case host is removed,
> or we are stuck timeouting on the same command. Also restart the
> command timeout timer when stopping the command ring to ensure we
> recive an ring stop/abort event.
>
> Cc: stable
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/host/xhci-ring.c | 27 ++-
> 1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 52deae4..c82570d 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -290,6 +290,14 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
>
> temp_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
> xhci->cmd_ring_state = CMD_RING_STATE_ABORTED;
> +
> + /*
> + * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
> + * however on some host hw the CMD_RING_RUNNING bit is correctly cleared
> + * but the completion event in never sent. Use the cmd timeout timer to
> + * handle those cases. Use twice the time to cover the bit polling retry
> + */
> + mod_timer(&xhci->cmd_timer, jiffies + (2 * XHCI_CMD_DEFAULT_TIMEOUT));
> xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
> &xhci->op_regs->cmd_ring);
>
> @@ -314,6 +322,7 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
>
> xhci_err(xhci, "Stopped the command ring failed, "
> "maybe the host is dead\n");
> + del_timer(&xhci->cmd_timer);
> xhci->xhc_state |= XHCI_STATE_DYING;
> xhci_quiesce(xhci);
> xhci_halt(xhci);
> @@ -1246,22 +1255,21 @@ void xhci_handle_command_timeout(unsigned long data)
> int ret;
> unsigned long flags;
> u64 hw_ring_state;
> - struct xhci_command *cur_cmd = NULL;
> + bool second_timeout = false;
> xhci = (struct xhci_hcd *) data;
>
> /* mark this command to be cancelled */
> spin_lock_irqsave(&xhci->lock, flags);
> if (xhci->current_cmd) {
> - cur_cmd = xhci->current_cmd;
> - cur_cmd->status = COMP_CMD_ABORT;
> + if (xhci->current_cmd->status == COMP_CMD_ABORT)
> + second_timeout = true;
> + xhci->current_cmd->status = COMP_CMD_ABORT;
> }
>
> -
> /* Make sure command ring is running before aborting it */
> hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
> if ((xhci->cmd_ring_state & CMD_RING_STATE_RUNNING) &&
> (hw_ring_state & CMD_RING_RUNNING)) {
> -
> spin_unlock_irqrestore(&xhci->lock, flags);
> xhci_dbg(xhci, "Command timeout\n");
> ret = xhci_abort_cmd_ring(xhci);
> @@ -1273,6 +1281,15 @@ void xhci_handle_command_timeout(unsigned long data)
> }
> return;
> }
> +
> + /* command ring failed to restart, or host removed. Bail out */
> + if (second_timeout || xhci->xhc_state & XHCI_STATE_REMOVING) {
> + spin_unlock_irqrestore(&xhci->lock, flags);
> + xhci_dbg(xhci, "command timed out twice, ring start fail?\n");
> + xhci_cleanup_command_queue(xhci);
> + return;
> + }
> +
> /* command timeout on stopped ring, ring can't be aborted */
> xhci_dbg(xhci, "Command timeout on stopped ring\n");
> xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
>
So far so good, feel free to add my Tested-by tag.
-- Joe
--
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] Re: Endless "supply vcc not found, using dummy regulator"
On Wed, May 25, 2016 at 05:46:51PM +0530, Anand Moon wrote: > Actually their are some missing patches to tune the usb3 phy. > > https://lkml.org/lkml/2014/10/31/266 This explains why the default networking speed refused to go above ~300 Mbit/sec! What happened to the patches, I wonder? /* Steinar */ -- Homepage: https://www.sesse.net/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: fix spinlock dead lock in gadgetfs
[ 40.467381] =
[ 40.473013] [ INFO: possible recursive locking detected ]
[ 40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted
[ 40.483466] -
[ 40.489098] usb/733 is trying to acquire lock:
[ 40.493734] (&(&dev->lock)->rlock){-.}, at: []
ep0_complete+0x18/0xdc [gadgetfs]
[ 40.502882]
[ 40.502882] but task is already holding lock:
[ 40.508967] (&(&dev->lock)->rlock){-.}, at: []
ep0_read+0x20/0x5e0 [gadgetfs]
[ 40.517811]
[ 40.517811] other info that might help us debug this:
[ 40.524623] Possible unsafe locking scenario:
[ 40.524623]
[ 40.530798]CPU0
[ 40.533346]
[ 40.535894] lock(&(&dev->lock)->rlock);
[ 40.540088] lock(&(&dev->lock)->rlock);
[ 40.544284]
[ 40.544284] *** DEADLOCK ***
[ 40.544284]
[ 40.550461] May be due to missing lock nesting notation
[ 40.550461]
[ 40.557544] 2 locks held by usb/733:
[ 40.561271] #0: (&f->f_pos_lock){+.+.+.}, at: []
__fdget_pos+0x40/0x48
[ 40.569219] #1: (&(&dev->lock)->rlock){-.}, at: []
ep0_read+0x20/0x5e0 [gadgetfs]
[ 40.578523]
[ 40.578523] stack backtrace:
[ 40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37
[ 40.590246] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 40.596625] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 40.604718] [] (show_stack) from []
(dump_stack+0xb0/0xe4)
[ 40.612267] [] (dump_stack) from []
(__lock_acquire+0xf68/0x1994)
[ 40.620440] [] (__lock_acquire) from []
(lock_acquire+0xd8/0x238)
[ 40.628621] [] (lock_acquire) from []
(_raw_spin_lock_irqsave+0x38/0x4c)
[ 40.637440] [] (_raw_spin_lock_irqsave) from []
(ep0_complete+0x18/0xdc [gadgetfs])
[ 40.647339] [] (ep0_complete [gadgetfs]) from []
(musb_g_giveback+0x118/0x1b0 [musb_hdrc])
[ 40.657842] [] (musb_g_giveback [musb_hdrc]) from []
(musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
[ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from []
(ep0_read+0x544/0x5e0 [gadgetfs])
[ 40.678963] [] (ep0_read [gadgetfs]) from []
(__vfs_read+0x20/0x110)
[ 40.687414] [] (__vfs_read) from [] (vfs_read+0x88/0x114)
[ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c)
[ 40.702051] [] (SyS_read) from []
(ret_fast_syscall+0x0/0x1c)
Cc: # v3.16+
Signed-off-by: Bin Liu
---
drivers/usb/gadget/legacy/inode.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index e64479f..1251e1d 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -938,8 +938,10 @@ ep0_read (struct file *fd, char __user *buf, size_t len,
loff_t *ptr)
struct usb_ep *ep = dev->gadget->ep0;
struct usb_request *req = dev->req;
+ spin_unlock_irq (&dev->lock);
if ((retval = setup_req (ep, req, 0)) == 0)
retval = usb_ep_queue (ep, req, GFP_ATOMIC);
+ spin_lock_irq (&dev->lock);
dev->state = STATE_DEV_CONNECTED;
/* assume that was SET_CONFIGURATION */
--
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
[RFC PATCH] usb: typec: Various API updates and fixes
From: Guenter Roeck
New API functions (calls into class code)
typec_set_usb_role()
typec_set_pwr_role()
typec_set_vconn_role()
typec_set_pwr_opmode()
Modified API functions (calls into class code):
typec_register_port(dev, cap) ->
typec_register_port(dev, cap, driver_data)
Modified callback functions:
dr_swap(port) -> dr_set(port, driver_data, role);
pr_swap(port) -> pr_set(port, driver_data, role);
vconn_swap(port) -> vconn_set(port, driver_data, role);
fix_role(port) -> fix_role(port, driver_data, role);
activate_mode(...) -> activate_mode(..., driver_data, ...);
New sysfs attribute:
current_vconn_role
Other:
- Extract role initialization to new function typec_init_roles()
- Call driver code unconditionally on role changes
- Add NULL check in typec_unregister_altmodes()
- If an alternate mode description pointer is NULL, display
an empty string.
Signed-off-by: Guenter Roeck
Signed-off-by: Guenter Roeck
---
This patch applies on top of '[RFC PATCHv2] usb: USB Type-C Connector Class'
from Heikki Krogerus. It provided the changes I made to get the code
operational.
drivers/usb/type-c/typec.c | 134 -
include/linux/usb/typec.h | 26 ++---
2 files changed, 125 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/type-c/typec.c b/drivers/usb/type-c/typec.c
index 8028b7df0951..6836e972b681 100644
--- a/drivers/usb/type-c/typec.c
+++ b/drivers/usb/type-c/typec.c
@@ -27,6 +27,8 @@ struct typec_port {
struct typec_partner*partner;
struct typec_cable *cable;
+ void*driver_data;
+
unsigned intconnected:1;
int n_altmode;
@@ -324,6 +326,20 @@ static void typec_remove_cable(struct typec_port *port)
device_unregister(&port->cable->dev);
}
+static void typec_init_roles(struct typec_port *port)
+{
+ if (port->fixed_role == TYPEC_PORT_DFP) {
+ port->usb_role = TYPEC_HOST;
+ port->pwr_role = TYPEC_PWR_SOURCE;
+ port->vconn_role = TYPEC_PWR_SOURCE;
+ } else {
+ /* Device mode as default also with DRP ports */
+ port->usb_role = TYPEC_DEVICE;
+ port->pwr_role = TYPEC_PWR_SINK;
+ port->vconn_role = TYPEC_PWR_SINK;
+ }
+}
+
/* */
int typec_connect(struct typec_port *port, struct typec_connection *con)
@@ -378,16 +394,7 @@ void typec_disconnect(struct typec_port *port)
port->pwr_opmode = TYPEC_PWR_MODE_USB;
- if (port->fixed_role == TYPEC_PORT_DFP) {
- port->usb_role = TYPEC_HOST;
- port->pwr_role = TYPEC_PWR_SOURCE;
- port->vconn_role = TYPEC_PWR_SOURCE;
- } else {
- /* Device mode as default also with DRP ports */
- port->usb_role = TYPEC_DEVICE;
- port->pwr_role = TYPEC_PWR_SINK;
- port->vconn_role = TYPEC_PWR_SINK;
- }
+ typec_init_roles(port);
kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
}
@@ -405,6 +412,34 @@ struct typec_port *typec_dev2port(struct device *dev)
}
EXPORT_SYMBOL_GPL(typec_dev2port);
+/* --- */
+/* Driver callbacks to report role updates */
+
+void typec_set_usb_role(struct typec_port *port, enum typec_usb_role role)
+{
+ port->usb_role = role;
+}
+EXPORT_SYMBOL(typec_set_usb_role);
+
+void typec_set_pwr_role(struct typec_port *port, enum typec_pwr_role role)
+{
+ port->pwr_role = role;
+}
+EXPORT_SYMBOL(typec_set_pwr_role);
+
+void typec_set_vconn_role(struct typec_port *port, enum typec_pwr_role role)
+{
+ port->vconn_role = role;
+}
+EXPORT_SYMBOL(typec_set_vconn_role);
+
+void typec_set_pwr_opmode(struct typec_port *port,
+ enum typec_pwr_opmode opmode)
+{
+ port->pwr_opmode = opmode;
+}
+EXPORT_SYMBOL(typec_set_pwr_opmode);
+
/* */
/* Alternate Modes */
@@ -451,7 +486,7 @@ typec_altmode_desc_show(struct device *dev, struct
device_attribute *attr,
struct typec_mode *mode = container_of(attr, struct typec_mode,
desc_attr);
- return sprintf(buf, "%s\n", mode->desc);
+ return sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
}
static ssize_t
@@ -561,6 +596,9 @@ void typec_unregister_altmodes(struct typec_altmode
*alt_modes)
{
struct typec_altmode *alt;
+ if (!alt_modes)
+ return;
+
for (alt = alt_modes; alt->svid; alt++)
device_unregister(&alt->dev);
}
@@ -581,7 +619,7 @@ current_usb_data_role_store(struct device *dev, struct
device_attribute *attr,
return -EOPNOTSUPP;
}
- if (!port->cap->dr_swap) {
+ if (!port->cap->dr_set) {
Re: [PATCH] usb: gadget: fix spinlock dead lock in gadgetfs
On Wed, 25 May 2016, Bin Liu wrote:
> [ 40.467381] =
> [ 40.473013] [ INFO: possible recursive locking detected ]
> [ 40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted
> [ 40.483466] -
> [ 40.489098] usb/733 is trying to acquire lock:
> [ 40.493734] (&(&dev->lock)->rlock){-.}, at: []
> ep0_complete+0x18/0xdc [gadgetfs]
> [ 40.502882]
> [ 40.502882] but task is already holding lock:
> [ 40.508967] (&(&dev->lock)->rlock){-.}, at: []
> ep0_read+0x20/0x5e0 [gadgetfs]
> [ 40.517811]
> [ 40.517811] other info that might help us debug this:
> [ 40.524623] Possible unsafe locking scenario:
> [ 40.524623]
> [ 40.530798]CPU0
> [ 40.533346]
> [ 40.535894] lock(&(&dev->lock)->rlock);
> [ 40.540088] lock(&(&dev->lock)->rlock);
> [ 40.544284]
> [ 40.544284] *** DEADLOCK ***
> [ 40.544284]
> [ 40.550461] May be due to missing lock nesting notation
> [ 40.550461]
> [ 40.557544] 2 locks held by usb/733:
> [ 40.561271] #0: (&f->f_pos_lock){+.+.+.}, at: []
> __fdget_pos+0x40/0x48
> [ 40.569219] #1: (&(&dev->lock)->rlock){-.}, at: []
> ep0_read+0x20/0x5e0 [gadgetfs]
> [ 40.578523]
> [ 40.578523] stack backtrace:
> [ 40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37
> [ 40.590246] Hardware name: Generic AM33XX (Flattened Device Tree)
> [ 40.596625] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 40.604718] [] (show_stack) from []
> (dump_stack+0xb0/0xe4)
> [ 40.612267] [] (dump_stack) from []
> (__lock_acquire+0xf68/0x1994)
> [ 40.620440] [] (__lock_acquire) from []
> (lock_acquire+0xd8/0x238)
> [ 40.628621] [] (lock_acquire) from []
> (_raw_spin_lock_irqsave+0x38/0x4c)
> [ 40.637440] [] (_raw_spin_lock_irqsave) from []
> (ep0_complete+0x18/0xdc [gadgetfs])
> [ 40.647339] [] (ep0_complete [gadgetfs]) from []
> (musb_g_giveback+0x118/0x1b0 [musb_hdrc])
> [ 40.657842] [] (musb_g_giveback [musb_hdrc]) from []
> (musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
> [ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from []
> (ep0_read+0x544/0x5e0 [gadgetfs])
> [ 40.678963] [] (ep0_read [gadgetfs]) from []
> (__vfs_read+0x20/0x110)
> [ 40.687414] [] (__vfs_read) from []
> (vfs_read+0x88/0x114)
> [ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c)
> [ 40.702051] [] (SyS_read) from []
> (ret_fast_syscall+0x0/0x1c)
>
> Cc: # v3.16+
> Signed-off-by: Bin Liu
> ---
> drivers/usb/gadget/legacy/inode.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/gadget/legacy/inode.c
> b/drivers/usb/gadget/legacy/inode.c
> index e64479f..1251e1d 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -938,8 +938,10 @@ ep0_read (struct file *fd, char __user *buf, size_t len,
> loff_t *ptr)
> struct usb_ep *ep = dev->gadget->ep0;
> struct usb_request *req = dev->req;
>
> + spin_unlock_irq (&dev->lock);
> if ((retval = setup_req (ep, req, 0)) == 0)
> retval = usb_ep_queue (ep, req, GFP_ATOMIC);
> + spin_lock_irq (&dev->lock);
Since you are enabling IRQs around this call to usb_ep_queue(), the
last argument should be changed to GFP_KERNEL.
> dev->state = STATE_DEV_CONNECTED;
>
> /* assume that was SET_CONFIGURATION */
You missed two other deadlock sources just like this one, in
gadgetfs_setup(): the two calls to usb_ep_queue() following the
"delegate:" statement label.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: fix spinlock dead lock in gadgetfs
[ 40.467381] =
[ 40.473013] [ INFO: possible recursive locking detected ]
[ 40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted
[ 40.483466] -
[ 40.489098] usb/733 is trying to acquire lock:
[ 40.493734] (&(&dev->lock)->rlock){-.}, at: []
ep0_complete+0x18/0xdc [gadgetfs]
[ 40.502882]
[ 40.502882] but task is already holding lock:
[ 40.508967] (&(&dev->lock)->rlock){-.}, at: []
ep0_read+0x20/0x5e0 [gadgetfs]
[ 40.517811]
[ 40.517811] other info that might help us debug this:
[ 40.524623] Possible unsafe locking scenario:
[ 40.524623]
[ 40.530798]CPU0
[ 40.533346]
[ 40.535894] lock(&(&dev->lock)->rlock);
[ 40.540088] lock(&(&dev->lock)->rlock);
[ 40.544284]
[ 40.544284] *** DEADLOCK ***
[ 40.544284]
[ 40.550461] May be due to missing lock nesting notation
[ 40.550461]
[ 40.557544] 2 locks held by usb/733:
[ 40.561271] #0: (&f->f_pos_lock){+.+.+.}, at: []
__fdget_pos+0x40/0x48
[ 40.569219] #1: (&(&dev->lock)->rlock){-.}, at: []
ep0_read+0x20/0x5e0 [gadgetfs]
[ 40.578523]
[ 40.578523] stack backtrace:
[ 40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37
[ 40.590246] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 40.596625] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 40.604718] [] (show_stack) from []
(dump_stack+0xb0/0xe4)
[ 40.612267] [] (dump_stack) from []
(__lock_acquire+0xf68/0x1994)
[ 40.620440] [] (__lock_acquire) from []
(lock_acquire+0xd8/0x238)
[ 40.628621] [] (lock_acquire) from []
(_raw_spin_lock_irqsave+0x38/0x4c)
[ 40.637440] [] (_raw_spin_lock_irqsave) from []
(ep0_complete+0x18/0xdc [gadgetfs])
[ 40.647339] [] (ep0_complete [gadgetfs]) from []
(musb_g_giveback+0x118/0x1b0 [musb_hdrc])
[ 40.657842] [] (musb_g_giveback [musb_hdrc]) from []
(musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
[ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from []
(ep0_read+0x544/0x5e0 [gadgetfs])
[ 40.678963] [] (ep0_read [gadgetfs]) from []
(__vfs_read+0x20/0x110)
[ 40.687414] [] (__vfs_read) from [] (vfs_read+0x88/0x114)
[ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c)
[ 40.702051] [] (SyS_read) from []
(ret_fast_syscall+0x0/0x1c)
Cc: # v3.16+
Signed-off-by: Bin Liu
---
v2:
- lock protects setup_req(), only unlock for usb_ep_queue()
- use GFP_KERNEL in usb_ep_queue()
- fix the same in two places in gadgetfs_setup() too
drivers/usb/gadget/legacy/inode.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index e64479f..a333e42 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -938,8 +938,11 @@ ep0_read (struct file *fd, char __user *buf, size_t len,
loff_t *ptr)
struct usb_ep *ep = dev->gadget->ep0;
struct usb_request *req = dev->req;
- if ((retval = setup_req (ep, req, 0)) == 0)
- retval = usb_ep_queue (ep, req, GFP_ATOMIC);
+ if ((retval = setup_req (ep, req, 0)) == 0) {
+ spin_unlock_irq (&dev->lock);
+ retval = usb_ep_queue (ep, req, GFP_KERNEL);
+ spin_lock_irq (&dev->lock);
+ }
dev->state = STATE_DEV_CONNECTED;
/* assume that was SET_CONFIGURATION */
@@ -1457,8 +1460,11 @@ delegate:
w_length);
if (value < 0)
break;
+
+ spin_unlock_irq (&dev->lock);
value = usb_ep_queue (gadget->ep0, dev->req,
- GFP_ATOMIC);
+ GFP_KERNEL);
+ spin_lock_irq (&dev->lock);
if (value < 0) {
clean_req (gadget->ep0, dev->req);
break;
@@ -1481,7 +1487,10 @@ delegate:
if (value >= 0 && dev->state != STATE_DEV_SETUP) {
req->length = value;
req->zero = value < w_length;
- value = usb_ep_queue (gadget->ep0, req, GFP_ATOMIC);
+
+ spin_unlock_irq (&dev->lock);
+ value = usb_ep_queue (gadget->ep0, req, GFP_KERNEL);
+ spin_lock_irq (&dev->lock);
if (value < 0) {
DBG (dev, "ep_queue --> %d\n", value);
req->status = 0;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vg
Re: [PATCH v2] usb: gadget: fix spinlock dead lock in gadgetfs
On Wed, May 25, 2016 at 02:18:34PM -0500, Bin Liu wrote:
> [ 40.467381] =
> [ 40.473013] [ INFO: possible recursive locking detected ]
> [ 40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted
> [ 40.483466] -
> [ 40.489098] usb/733 is trying to acquire lock:
> [ 40.493734] (&(&dev->lock)->rlock){-.}, at: []
> ep0_complete+0x18/0xdc [gadgetfs]
> [ 40.502882]
> [ 40.502882] but task is already holding lock:
> [ 40.508967] (&(&dev->lock)->rlock){-.}, at: []
> ep0_read+0x20/0x5e0 [gadgetfs]
> [ 40.517811]
> [ 40.517811] other info that might help us debug this:
> [ 40.524623] Possible unsafe locking scenario:
> [ 40.524623]
> [ 40.530798]CPU0
> [ 40.533346]
> [ 40.535894] lock(&(&dev->lock)->rlock);
> [ 40.540088] lock(&(&dev->lock)->rlock);
> [ 40.544284]
> [ 40.544284] *** DEADLOCK ***
> [ 40.544284]
> [ 40.550461] May be due to missing lock nesting notation
> [ 40.550461]
> [ 40.557544] 2 locks held by usb/733:
> [ 40.561271] #0: (&f->f_pos_lock){+.+.+.}, at: []
> __fdget_pos+0x40/0x48
> [ 40.569219] #1: (&(&dev->lock)->rlock){-.}, at: []
> ep0_read+0x20/0x5e0 [gadgetfs]
> [ 40.578523]
> [ 40.578523] stack backtrace:
> [ 40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37
> [ 40.590246] Hardware name: Generic AM33XX (Flattened Device Tree)
> [ 40.596625] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 40.604718] [] (show_stack) from []
> (dump_stack+0xb0/0xe4)
> [ 40.612267] [] (dump_stack) from []
> (__lock_acquire+0xf68/0x1994)
> [ 40.620440] [] (__lock_acquire) from []
> (lock_acquire+0xd8/0x238)
> [ 40.628621] [] (lock_acquire) from []
> (_raw_spin_lock_irqsave+0x38/0x4c)
> [ 40.637440] [] (_raw_spin_lock_irqsave) from []
> (ep0_complete+0x18/0xdc [gadgetfs])
> [ 40.647339] [] (ep0_complete [gadgetfs]) from []
> (musb_g_giveback+0x118/0x1b0 [musb_hdrc])
> [ 40.657842] [] (musb_g_giveback [musb_hdrc]) from []
> (musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
> [ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from []
> (ep0_read+0x544/0x5e0 [gadgetfs])
> [ 40.678963] [] (ep0_read [gadgetfs]) from []
> (__vfs_read+0x20/0x110)
> [ 40.687414] [] (__vfs_read) from []
> (vfs_read+0x88/0x114)
> [ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c)
> [ 40.702051] [] (SyS_read) from []
> (ret_fast_syscall+0x0/0x1c)
>
> Cc: # v3.16+
> Signed-off-by: Bin Liu
> ---
> v2:
> - lock protects setup_req(), only unlock for usb_ep_queue()
> - use GFP_KERNEL in usb_ep_queue()
> - fix the same in two places in gadgetfs_setup() too
Never mind. Sent the patch too soon. It gives the following problem.
[ 179.810367] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2724
_raw_spin_unlock_irq+0x24/0x2c
[ 179.819719] DEBUG_LOCKS_WARN_ON(current->hardirq_context)
Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers: usb: dwc3 : Configure DMA properties and ops from DT
On Wed, May 4, 2016 at 2:57 AM, Felipe Balbi wrote: > > Hi, > > Rajesh Bhagat writes: >> On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set >> to be able to do DMA allocations, so use the of_dma_configure() helper >> to populate the dma properties and assign an appropriate dma_ops. >> >> Signed-off-by: Rajesh Bhagat >> Reviewed-by: Yang-Leo Li > > Cool, nxp is also using dwc3 :-) C'mon Rajesh, send us a glue layer :) > >> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >> index c679f63..4d5b783 100644 >> --- a/drivers/usb/dwc3/host.c >> +++ b/drivers/usb/dwc3/host.c >> @@ -17,6 +17,7 @@ >> >> #include >> #include >> +#include >> >> #include "core.h" >> >> @@ -32,6 +33,9 @@ int dwc3_host_init(struct dwc3 *dwc) >> return -ENOMEM; >> } >> >> + if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node) >> + of_dma_configure(&xhci->dev, dwc->dev->of_node); > > okay, so we have a long discussion about this going on. You can catch up > with it starting here: > > http://marc.info/?i=1461612094-30939-1-git-send-email-grygorii.stras...@ti.com > > At least for now, this patch will be applied. We need to have a better > solution for this, one that helps not only DT platforms. Balbi, Has the patch from Grygorii been applied? I don't see it in the mainline tree yet. Without fix, the dwc3 driver will fail for all ARM64 SoCs. Regards, Leo -- 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 0/2] usb: musb: fix dropped packets
Hi, On Tue, May 24, 2016 at 09:22:32AM +0100, Andrew Goodbody wrote: > The musb driver can drop rx packets when heavily loaded. These two > patches address two issues that can cause this. Both issues arose > when an endpoint was reprogrammed. The first patch is a logic bug > that resulted in a shared_fifo in rx mode not having its state > cleared out. The second patch fixes a race condition caused by > not stopping the dedicated endpoint for bulk packets before > rotating its queue which allowed a packet to be recieved and then > thrown away. > > V3 Updated the comment to better reference the manual > V2 added a comment and removed debugging code > > Andrew Goodbody (2): > usb: musb: Ensure rx reinit occurs for shared_fifo endpoints > usb: musb: Stop bulk endpoint while queue is rotated Signed-off-by: Bin Liu Regards, -Bin. > > drivers/usb/musb/musb_host.c | 21 + > 1 file changed, 13 insertions(+), 8 deletions(-) > > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] input: tablet: add Pegasus Notetaker tablet driver
Hi Martin, On Wed, May 25, 2016 at 09:44:34AM +0200, Martin Kepplinger wrote: > This adds a driver for the Pegasus Notetaker Pen. When connected, > this uses the Pen as an input tablet. > > This device was sold in various different brandings, for example > "Pegasus Mobile Notetaker M210", > "Genie e-note The Notetaker", > "Staedtler Digital ballpoint pen 990 01", > "IRISnotes Express" or > "NEWLink Digital Note Taker". > > Here's an example, so that you know what we are talking about: > http://www.staedtler.com/en/products/ink-writing-instruments/ballpoint-pens/digital-pen-990-01-digital-ballpoint-pen > > http://pegatech.blogspot.com/ seems to be a remaining official resource. > > This device can also transfer saved (offline recorded handwritten) data and > there are userspace programs that do this, see https://launchpad.net/m210 > (Well, alternatively there are really fast scanners out there :) > > It's *really* fun to use as an input tablet though! So let's support this > for everybody. > > There's no way to disable the device. When the pen is out of range, we just > don't get any URBs and don't do anything. > Like all other mouses or input tablets, we don't use runtime PM. > > Signed-off-by: Martin Kepplinger > --- > > Thanks for having a look. Any more suggestions on this? > > revision history > > v4 use normal work queue instead of a kernel thread (thanks to Oliver Neukum) > v3 fix reporting low pen battery and add USB list to CC > v2 minor cleanup (remove unnecessary variables) > v1 initial release > > > > drivers/input/tablet/Kconfig | 15 ++ > drivers/input/tablet/Makefile| 1 + > drivers/input/tablet/pegasus_notetaker.c | 373 > +++ > 3 files changed, 389 insertions(+) > create mode 100644 drivers/input/tablet/pegasus_notetaker.c > > diff --git a/drivers/input/tablet/Kconfig b/drivers/input/tablet/Kconfig > index 623bb9e..a2b9f97 100644 > --- a/drivers/input/tablet/Kconfig > +++ b/drivers/input/tablet/Kconfig > @@ -73,6 +73,21 @@ config TABLET_USB_KBTAB > To compile this driver as a module, choose M here: the > module will be called kbtab. > > +config TABLET_USB_PEGASUS > + tristate "Pegasus Mobile Notetaker Pen input tablet support" > + depends on USB_ARCH_HAS_HCD > + select USB > + help > + Say Y here if you want to use the Pegasus Mobile Notetaker, > + also known as: > + Genie e-note The Notetaker, > + Staedtler Digital ballpoint pen 990 01, > + IRISnotes Express or > + NEWLink Digital Note Taker. > + > + To compile this driver as a module, choose M here: the > + module will be called pegasus_notetaker. > + > config TABLET_SERIAL_WACOM4 > tristate "Wacom protocol 4 serial tablet support" > select SERIO > diff --git a/drivers/input/tablet/Makefile b/drivers/input/tablet/Makefile > index 2e13010..200fc4e 100644 > --- a/drivers/input/tablet/Makefile > +++ b/drivers/input/tablet/Makefile > @@ -8,4 +8,5 @@ obj-$(CONFIG_TABLET_USB_AIPTEK) += aiptek.o > obj-$(CONFIG_TABLET_USB_GTCO)+= gtco.o > obj-$(CONFIG_TABLET_USB_HANWANG) += hanwang.o > obj-$(CONFIG_TABLET_USB_KBTAB) += kbtab.o > +obj-$(CONFIG_TABLET_USB_PEGASUS) += pegasus_notetaker.o > obj-$(CONFIG_TABLET_SERIAL_WACOM4) += wacom_serial4.o > diff --git a/drivers/input/tablet/pegasus_notetaker.c > b/drivers/input/tablet/pegasus_notetaker.c > new file mode 100644 > index 000..b7bf585 > --- /dev/null > +++ b/drivers/input/tablet/pegasus_notetaker.c > @@ -0,0 +1,373 @@ > +/* > + * Pegasus Mobile Notetaker Pen input tablet driver > + * > + * Copyright (c) 2016 Martin Kepplinger > + */ > + > +/* > + * request packet (control endpoint): > + * |-| > + * | Report ID | Nr of bytes | command | > + * | (1 byte) | (1 byte)| (n bytes) | > + * |-| > + * | 0x02 | n | | > + * |-| > + * > + * data packet after set xy mode command, 0x80 0xb5 0x02 0x01 > + * and pen is in range: > + * > + * byte byte name value (bits) > + * > + * 0 status 0 1 0 0 0 0 X X > + * 1 color 0 0 0 0 H 0 S T > + * 2 X low > + * 3 X high > + * 4 Y low > + * 5 Y high > + * > + * X X battery state: > + * no state reported 0x00 > + * battery low 0x01 > + * battery good0x02 > + * > + * H Hovering > + * S Switch 1 (pen button) > + * T Tip > + */ > + > +#include > +#include > +#include > +#include > + > +/* USB HID defines */ > +#define USB_REQ_GET_REPORT 0x01 > +#define USB_REQ_SET_REPORT 0x09 > + > +#define USB_VENDOR_ID_PEGASUSTECH0x0e20 > +#define USB_DEVICE_ID_PEGASUS_NOTETAKER_EN1000x0101 > + > +/* device specific defines */ > +#define NOTE
Re: [PATCH v8 2/7] usb: mux: add generic code for dual role port mux
Hi Heikki,
On 05/25/2016 07:06 PM, Heikki Krogerus wrote:
> Hi Baolu,
>
> Sorry to comment this so late, but we got hardware that needs to
> configure the mux in OS, and I noticed some problem.
Comments are always welcome. :-)
> We are missing
> means to bind a port to the correct mux on multiport systems. That we
> need to solve later in any case, but there is an other issue related
> to the fact that the notifiers now have to be extcon devices. It's
> related, as extcon offers no means to solve the multiport issue, but
> in any case..
>
>> +struct portmux_dev *portmux_register(struct portmux_desc *desc)
>> +{
>> +static atomic_t portmux_no = ATOMIC_INIT(-1);
>> +struct portmux_dev *pdev;
>> +struct extcon_dev *edev = NULL;
>> +struct device *dev;
>> +int ret;
>> +
>> +/* parameter sanity check */
>> +if (!desc || !desc->name || !desc->ops || !desc->dev ||
>> +!desc->ops->cable_set_cb || !desc->ops->cable_unset_cb)
>> +return ERR_PTR(-EINVAL);
>> +
>> +dev = desc->dev;
>> +
>> +if (desc->extcon_name) {
>> +edev = extcon_get_extcon_dev(desc->extcon_name);
>> +if (IS_ERR_OR_NULL(edev))
>> +return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
>> +if (!pdev)
>> +return ERR_PTR(-ENOMEM);
>> +
>> +pdev->desc = desc;
>> +pdev->edev = edev;
>> +pdev->nb.notifier_call = usb_mux_notifier;
>> +mutex_init(&pdev->mux_mutex);
>> +
>> +pdev->dev.parent = dev;
>> +dev_set_name(&pdev->dev, "portmux.%lu",
>> + (unsigned long)atomic_inc_return(&portmux_no));
>> +pdev->dev.groups = portmux_group;
>> +ret = device_register(&pdev->dev);
>> +if (ret)
>> +goto cleanup_mem;
>> +
>> +dev_set_drvdata(&pdev->dev, pdev);
>> +
>> +if (edev) {
>> +ret = extcon_register_notifier(edev, EXTCON_USB_HOST,
>> + &pdev->nb);
>> +if (ret < 0) {
>> +dev_err(dev, "failed to register extcon notifier\n");
>> +goto cleanup_dev;
>> +}
>> +}
> So I don't actually think this is correct approach. We are forcing the
> notifying drivers, on top of depending on this framework, depend on
> extcon too, and that simply is too much. I don't think a USB PHY or
> charger detection driver should be forced to generate an extcon device
> just to satisfy the mux in general.
Fair enough.
>
> Instead IMO, this framework should provide an API also for the
> notifiers. The drivers that do the notification should not need to
> depend on extcon at all. Instead they should be able to just request
> an optional handle to a portmux, and use it with the function that you
> already provide (usb_mux_change_state(), which of course needs to be
> exported). That would make it much easier for us to make problems like
> figuring out the correct mux for the correct port a problem for the
> framework and not the drivers.
>
> extcon does not really add any value here, but it does complicate
> things a lot. We are even exposing new sysfs attributes to control the
> mux, complete separate from extcon.
I agree with you that we should move extcon out of the framework.
In order to support multiport systems, I have below proposal.
Currently, we have below interfaces.
struct portmux_dev *portmux_register(struct portmux_desc *desc);
void portmux_unregister(struct portmux_dev *pdev);
I will add below ones.
struct portmux_dev *portmux_lookup_by_name(char *name);
int portmux_switch(struct portmux_dev *pdev, enum port_role);
The normal usage mode is
1) the mux device is registered as soon as a mux is detected with
portmux_register();
2) In components like USB PHY or changer drivers, the mux could
be retrieved with portmux_lookup_by_name() and controlled via
portmux_switch().
Is this helpful?
Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 05/13] usb: dwc2: gadget: Fix transfer stop programming for out endpoint
From: Vardan Mikayelyan
According DWC-OTG databook, "GOUTNakEff" is read only and can be
cleared only by "DCTL.CGOUTNak", but here we do not need to clear
it because DWC-OTG programming guide says that before disabling
any OUT endpoint, the application must enable Global OUT NAK mode,
so if this mode is enabled we can continue without this step.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index a62a2b1..22d4f5b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2866,10 +2866,8 @@ static void dwc2_hsotg_ep_stop_xfr(struct dwc2_hsotg
*hsotg,
dev_warn(hsotg->dev,
"%s: timeout DIEPINT.NAKEFF\n", __func__);
} else {
- /* Clear any pending nak effect interrupt */
- dwc2_writel(GINTSTS_GOUTNAKEFF, hsotg->regs + GINTSTS);
-
- __orr32(hsotg->regs + DCTL, DCTL_SGOUTNAK);
+ if (!(dwc2_readl(hsotg->regs + GINTSTS) & GINTSTS_GOUTNAKEFF))
+ __orr32(hsotg->regs + DCTL, DCTL_SGOUTNAK);
/* Wait for global nak to take effect */
if (dwc2_hsotg_wait_bit_set(hsotg, GINTSTS,
--
2.8.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
[PATCH v3 00/13] usb: dwc2: Fix up gadget isochronous support
The following patch series fixes up isochronous support for the dwc2 gadget. The existing isochronous support lacked a few features. Most notably it did not properly sync up with the first packet and it didn't handle the Incomplete ISO IN/OUT interrupts. These patches have been sitting in our internal tree for a while and tested mostly on a HAPS-based FPGA IP validation platform. v3: * Simplified setting endpoint interval in patch 7/13 v2: * Changed if/else to switch statement * Changed frame_overrun to bool * Downgraded a info message to dbg * Added John Keeping's tested-by Regards, John Vardan Mikayelyan (13): usb: dwc2: Add missing register field definitions usb: dwc2: gadget: Remove unnecessary line usb: dwc2: gadget: Remove unnecessary code usb: dwc2: gadget: Corrected field names usb: dwc2: gadget: Fix transfer stop programming for out endpoint usb: dwc2: gadget: Add dwc2_gadget_incr_frame_num() usb: dwc2: gadget: Corrected interval calculation usb: dwc2: gadget: Add dwc2_gadget_read_ep_interrupts function usb: dwc2: gadget: Add dwc2_gadget_start_next_request function usb: dwc2: gadget: Add OUTTKNEPDIS and NAKINTRPT handlers usb: dwc2: gadget: Add Incomplete ISO IN/OUT Interrupt handlers usb: dwc2: gadget: Add EP disabled interrupt handler usb: dwc2: gadget: Final fixes for BDMA ISOC drivers/usb/dwc2/core.h | 8 +- drivers/usb/dwc2/gadget.c | 574 -- drivers/usb/dwc2/hw.h | 14 ++ 3 files changed, 470 insertions(+), 126 deletions(-) -- 2.8.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
[PATCH v3 02/13] usb: dwc2: gadget: Remove unnecessary line
From: Vardan Mikayelyan Removed "ctrl |= DXEPCTL_USBACTEP" from dwc2_hsotg_start_req() function because this step is done in dwc2_hsotg_ep_enable(). Tested-by: John Keeping Signed-off-by: Vardan Mikayelyan Signed-off-by: John Youn --- drivers/usb/dwc2/gadget.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4c5e300..5521c70 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -632,7 +632,6 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg, } ctrl |= DXEPCTL_EPENA; /* ensure ep enabled */ - ctrl |= DXEPCTL_USBACTEP; dev_dbg(hsotg->dev, "ep0 state:%d\n", hsotg->ep0_state); -- 2.8.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
[PATCH v3 04/13] usb: dwc2: gadget: Corrected field names
From: Vardan Mikayelyan
No-op change. Changed field names to prevent misunderstanding.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e9595be..a62a2b1 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2037,20 +2037,20 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg,
unsigned int idx,
if (dir_in && !hs_ep->isochronous) {
/* not sure if this is important, but we'll clear it anyway */
- if (ints & DIEPMSK_INTKNTXFEMPMSK) {
+ if (ints & DXEPINT_INTKNTXFEMP) {
dev_dbg(hsotg->dev, "%s: ep%d: INTknTXFEmpMsk\n",
__func__, idx);
}
/* this probably means something bad is happening */
- if (ints & DIEPMSK_INTKNEPMISMSK) {
+ if (ints & DXEPINT_INTKNEPMIS) {
dev_warn(hsotg->dev, "%s: ep%d: INTknEP\n",
__func__, idx);
}
/* FIFO has space or is empty (see GAHBCFG) */
if (hsotg->dedicated_fifos &&
- ints & DIEPMSK_TXFIFOEMPTY) {
+ ints & DXEPINT_TXFEMP) {
dev_dbg(hsotg->dev, "%s: ep%d: TxFIFOEmpty\n",
__func__, idx);
if (!using_dma(hsotg))
--
2.8.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
[PATCH v3 01/13] usb: dwc2: Add missing register field definitions
From: Vardan Mikayelyan Added register field definitions, register names are according DWC-OTG databook. Tested-by: John Keeping Signed-off-by: Vardan Mikayelyan Signed-off-by: John Youn --- drivers/usb/dwc2/hw.h | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h index 281b57b..1126141 100644 --- a/drivers/usb/dwc2/hw.h +++ b/drivers/usb/dwc2/hw.h @@ -459,6 +459,9 @@ #define DSTS_SUSPSTS (1 << 0) #define DIEPMSKHSOTG_REG(0x810) +#define DIEPMSK_NAKMSK (1 << 13) +#define DIEPMSK_BNAININTRMSK (1 << 9) +#define DIEPMSK_TXFIFOUNDRNMSK (1 << 8) #define DIEPMSK_TXFIFOEMPTY(1 << 7) #define DIEPMSK_INEPNAKEFFMSK (1 << 6) #define DIEPMSK_INTKNEPMISMSK (1 << 5) @@ -470,6 +473,7 @@ #define DOEPMSKHSOTG_REG(0x814) #define DOEPMSK_BACK2BACKSETUP (1 << 6) +#define DOEPMSK_STSPHSERCVDMSK (1 << 5) #define DOEPMSK_OUTTKNEPDISMSK (1 << 4) #define DOEPMSK_SETUPMSK (1 << 3) #define DOEPMSK_AHBERRMSK (1 << 2) @@ -486,6 +490,7 @@ #define DTKNQR2HSOTG_REG(0x824) #define DTKNQR3HSOTG_REG(0x830) #define DTKNQR4HSOTG_REG(0x834) +#define DIEPEMPMSK HSOTG_REG(0x834) #define DVBUSDIS HSOTG_REG(0x828) #define DVBUSPULSE HSOTG_REG(0x82C) @@ -544,6 +549,14 @@ #define DIEPINT(_a)HSOTG_REG(0x908 + ((_a) * 0x20)) #define DOEPINT(_a)HSOTG_REG(0xB08 + ((_a) * 0x20)) #define DXEPINT_SETUP_RCVD (1 << 15) +#define DXEPINT_NYETINTRPT (1 << 14) +#define DXEPINT_NAKINTRPT (1 << 13) +#define DXEPINT_BBLEERRINTRPT (1 << 12) +#define DXEPINT_PKTDRPSTS (1 << 11) +#define DXEPINT_BNAINTR(1 << 9) +#define DXEPINT_TXFIFOUNDRN(1 << 8) +#define DXEPINT_OUTPKTERR (1 << 8) +#define DXEPINT_TXFEMP (1 << 7) #define DXEPINT_INEPNAKEFF (1 << 6) #define DXEPINT_BACK2BACKSETUP (1 << 6) #define DXEPINT_INTKNEPMIS (1 << 5) -- 2.8.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
[PATCH v3 03/13] usb: dwc2: gadget: Remove unnecessary code
From: Vardan Mikayelyan This chunk is not needed here. There is no functionality depend on this, so if no-op, I think we do not need to have this interrupt unmasked. Tested-by: John Keeping Signed-off-by: Vardan Mikayelyan Signed-off-by: John Youn --- drivers/usb/dwc2/gadget.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 5521c70..e9595be 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -658,14 +658,6 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg, } /* -* clear the INTknTXFEmpMsk when we start request, more as a aide -* to debugging to see what is going on. -*/ - if (dir_in) - dwc2_writel(DIEPMSK_INTKNTXFEMPMSK, - hsotg->regs + DIEPINT(index)); - - /* * Note, trying to clear the NAK here causes problems with transmit * on the S3C6400 ending up with the TXFIFO becoming full. */ -- 2.8.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
[PATCH v3 07/13] usb: dwc2: gadget: Corrected interval calculation
From: Vardan Mikayelyan
Calculate the interval according to the USB 2.0 specification section
9.6.6.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/core.h | 2 +-
drivers/usb/dwc2/gadget.c | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 22fa1f5..f2e7de7 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -139,7 +139,7 @@ struct dwc2_hsotg_req;
* means that it is sending data to the Host.
* @index: The index for the endpoint registers.
* @mc: Multi Count - number of transactions per microframe
- * @interval - Interval for periodic endpoints
+ * @interval - Interval for periodic endpoints, in frames or microframes.
* @name: The name array passed to the USB core.
* @halted: Set if the endpoint has been halted.
* @periodic: Set if this is a periodic ep, such as Interrupt
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7f15bc0..2e98241 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2693,16 +2693,13 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
hs_ep->periodic = 0;
hs_ep->halted = 0;
hs_ep->interval = desc->bInterval;
- hs_ep->has_correct_parity = 0;
-
- if (hs_ep->interval > 1 && hs_ep->mc > 1)
- dev_err(hsotg->dev, "MC > 1 when interval is not 1\n");
switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_ISOC:
epctrl |= DXEPCTL_EPTYPE_ISO;
epctrl |= DXEPCTL_SETEVENFR;
hs_ep->isochronous = 1;
+ hs_ep->interval = 1 << (desc->bInterval - 1);
if (dir_in)
hs_ep->periodic = 1;
break;
@@ -2715,6 +2712,9 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
if (dir_in)
hs_ep->periodic = 1;
+ if (hsotg->gadget.speed == USB_SPEED_HIGH)
+ hs_ep->interval = 1 << (desc->bInterval - 1);
+
epctrl |= DXEPCTL_EPTYPE_INTERRUPT;
break;
--
2.8.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
[PATCH v3 10/13] usb: dwc2: gadget: Add OUTTKNEPDIS and NAKINTRPT handlers
From: Vardan Mikayelyan
NAKINTRPT interrupt is starting point for isoc-in transfer,
synchronization done with first in token received from host,
core asserts this interrupt when responds with 0 length data
to in token, received from host.
The first IN token is asynchronous for device - device does not
know when first one token will arrive from host. On first token
arrival HW generates 2 interrupts: 'in token received while FIFO
empty' and 'NAK'. NAK interrupt for ISOC in means that token has
arrived and ZLP was sent in response to that as there was no data
in FIFO. SW is basing on this interrupt to obtain frame in which
token has come and then based on the interval calculates next
frame for transfer.
OUTTKNEPDIS interrupt is starting point for isoc-out transfer,
synchronization done with first out token received from host
while corresponding ep is disabled.
For OUTs the reason is same - device does not know initial frame
in which out token will come. For this HW generates OUTTKNEPDIS
- out token is received while EP is disabled. Upon getting this
interrupt SW starts calculation for next transfer frame.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 94 +++
1 file changed, 94 insertions(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 8faddbd..adc3381 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2000,6 +2000,94 @@ static u32 dwc2_gadget_read_ep_interrupts(struct
dwc2_hsotg *hsotg,
}
/**
+ * dwc2_gadget_handle_out_token_ep_disabled - handle DXEPINT_OUTTKNEPDIS
+ * @hs_ep: The endpoint on which interrupt is asserted.
+ *
+ * This is starting point for ISOC-OUT transfer, synchronization done with
+ * first out token received from host while corresponding EP is disabled.
+ *
+ * Device does not know initial frame in which out token will come. For this
+ * HW generates OUTTKNEPDIS - out token is received while EP is disabled. Upon
+ * getting this interrupt SW starts calculation for next transfer frame.
+ */
+static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
+{
+ struct dwc2_hsotg *hsotg = ep->parent;
+ int dir_in = ep->dir_in;
+ u32 doepmsk;
+
+ if (dir_in || !ep->isochronous)
+ return;
+
+ dwc2_hsotg_complete_request(hsotg, ep, get_ep_head(ep), -ENODATA);
+
+ if (ep->interval > 1 &&
+ ep->target_frame == TARGET_FRAME_INITIAL) {
+ u32 dsts;
+ u32 ctrl;
+
+ dsts = dwc2_readl(hsotg->regs + DSTS);
+ ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+ dwc2_gadget_incr_frame_num(ep);
+
+ ctrl = dwc2_readl(hsotg->regs + DOEPCTL(ep->index));
+ if (ep->target_frame & 0x1)
+ ctrl |= DXEPCTL_SETODDFR;
+ else
+ ctrl |= DXEPCTL_SETEVENFR;
+
+ dwc2_writel(ctrl, hsotg->regs + DOEPCTL(ep->index));
+ }
+
+ dwc2_gadget_start_next_request(ep);
+ doepmsk = dwc2_readl(hsotg->regs + DOEPMSK);
+ doepmsk &= ~DOEPMSK_OUTTKNEPDISMSK;
+ dwc2_writel(doepmsk, hsotg->regs + DOEPMSK);
+}
+
+/**
+* dwc2_gadget_handle_nak - handle NAK interrupt
+* @hs_ep: The endpoint on which interrupt is asserted.
+*
+* This is starting point for ISOC-IN transfer, synchronization done with
+* first IN token received from host while corresponding EP is disabled.
+*
+* Device does not know when first one token will arrive from host. On first
+* token arrival HW generates 2 interrupts: 'in token received while FIFO empty'
+* and 'NAK'. NAK interrupt for ISOC-IN means that token has arrived and ZLP was
+* sent in response to that as there was no data in FIFO. SW is basing on this
+* interrupt to obtain frame in which token has come and then based on the
+* interval calculates next frame for transfer.
+*/
+static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep *hs_ep)
+{
+ struct dwc2_hsotg *hsotg = hs_ep->parent;
+ int dir_in = hs_ep->dir_in;
+
+ if (!dir_in || !hs_ep->isochronous)
+ return;
+
+ if (hs_ep->target_frame == TARGET_FRAME_INITIAL) {
+ hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+ if (hs_ep->interval > 1) {
+ u32 ctrl = dwc2_readl(hsotg->regs +
+ DIEPCTL(hs_ep->index));
+ if (hs_ep->target_frame & 0x1)
+ ctrl |= DXEPCTL_SETODDFR;
+ else
+ ctrl |= DXEPCTL_SETEVENFR;
+
+ dwc2_writel(ctrl, hsotg->regs + DIEPCTL(hs_ep->index));
+ }
+
+ dwc2_hsotg_complete_request(hsotg, hs_ep,
+ get_ep_head(hs_ep), 0);
+ }
+
+ dwc2_gadget_incr_frame_num(hs_ep);
[PATCH v3 12/13] usb: dwc2: gadget: Add EP disabled interrupt handler
From: Vardan Mikayelyan
Reimplemented EP disabled interrupt handler and moved to
corresponding function.
This interrupt indicates that the endpoint has been disabled per
the application's request.
For IN endpoints flushes txfifo, in case of BULK clears DCTL_CGNPINNAK,
in case of ISOC completes current request.
For ISOC-OUT endpoints completes expired requests. If there is
remaining request starts it. This is the part of ISOC-OUT transfer
drop flow. When ISOC-OUT transfer expired we must disable ep to drop
ongoing transfer.
Tested-by: John Keeping
Reviewed-by: Vahram Aharonyan
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 87 ++-
1 file changed, 70 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4f83e04..12ec6a1 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2024,6 +2024,74 @@ static u32 dwc2_gadget_read_ep_interrupts(struct
dwc2_hsotg *hsotg,
}
/**
+ * dwc2_gadget_handle_ep_disabled - handle DXEPINT_EPDISBLD
+ * @hs_ep: The endpoint on which interrupt is asserted.
+ *
+ * This interrupt indicates that the endpoint has been disabled per the
+ * application's request.
+ *
+ * For IN endpoints flushes txfifo, in case of BULK clears DCTL_CGNPINNAK,
+ * in case of ISOC completes current request.
+ *
+ * For ISOC-OUT endpoints completes expired requests. If there is remaining
+ * request starts it.
+ */
+static void dwc2_gadget_handle_ep_disabled(struct dwc2_hsotg_ep *hs_ep)
+{
+ struct dwc2_hsotg *hsotg = hs_ep->parent;
+ struct dwc2_hsotg_req *hs_req;
+ unsigned char idx = hs_ep->index;
+ int dir_in = hs_ep->dir_in;
+ u32 epctl_reg = dir_in ? DIEPCTL(idx) : DOEPCTL(idx);
+ int dctl = dwc2_readl(hsotg->regs + DCTL);
+
+ dev_dbg(hsotg->dev, "%s: EPDisbld\n", __func__);
+
+ if (dir_in) {
+ int epctl = dwc2_readl(hsotg->regs + epctl_reg);
+
+ dwc2_hsotg_txfifo_flush(hsotg, hs_ep->fifo_index);
+
+ if (hs_ep->isochronous) {
+ dwc2_hsotg_complete_in(hsotg, hs_ep);
+ return;
+ }
+
+ if ((epctl & DXEPCTL_STALL) && (epctl & DXEPCTL_EPTYPE_BULK)) {
+ int dctl = dwc2_readl(hsotg->regs + DCTL);
+
+ dctl |= DCTL_CGNPINNAK;
+ dwc2_writel(dctl, hsotg->regs + DCTL);
+ }
+ return;
+ }
+
+ if (dctl & DCTL_GOUTNAKSTS) {
+ dctl |= DCTL_CGOUTNAK;
+ dwc2_writel(dctl, hsotg->regs + DCTL);
+ }
+
+ if (!hs_ep->isochronous)
+ return;
+
+ if (list_empty(&hs_ep->queue)) {
+ dev_dbg(hsotg->dev, "%s: complete_ep 0x%p, ep->queue empty!\n",
+ __func__, hs_ep);
+ return;
+ }
+
+ do {
+ hs_req = get_ep_head(hs_ep);
+ if (hs_req)
+ dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req,
+ -ENODATA);
+ dwc2_gadget_incr_frame_num(hs_ep);
+ } while (dwc2_gadget_target_frame_elapsed(hs_ep));
+
+ dwc2_gadget_start_next_request(hs_ep);
+}
+
+/**
* dwc2_gadget_handle_out_token_ep_disabled - handle DXEPINT_OUTTKNEPDIS
* @hs_ep: The endpoint on which interrupt is asserted.
*
@@ -2177,23 +2245,8 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg,
unsigned int idx,
}
}
- if (ints & DXEPINT_EPDISBLD) {
- dev_dbg(hsotg->dev, "%s: EPDisbld\n", __func__);
-
- if (dir_in) {
- int epctl = dwc2_readl(hsotg->regs + epctl_reg);
-
- dwc2_hsotg_txfifo_flush(hsotg, hs_ep->fifo_index);
-
- if ((epctl & DXEPCTL_STALL) &&
- (epctl & DXEPCTL_EPTYPE_BULK)) {
- int dctl = dwc2_readl(hsotg->regs + DCTL);
-
- dctl |= DCTL_CGNPINNAK;
- dwc2_writel(dctl, hsotg->regs + DCTL);
- }
- }
- }
+ if (ints & DXEPINT_EPDISBLD)
+ dwc2_gadget_handle_ep_disabled(hs_ep);
if (ints & DXEPINT_OUTTKNEPDIS)
dwc2_gadget_handle_out_token_ep_disabled(hs_ep);
--
2.8.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
[PATCH v3 13/13] usb: dwc2: gadget: Final fixes for BDMA ISOC
From: Vardan Mikayelyan
Done fixes and tested hsotg gadget's BDMA mode. Tested Control,
Bulk, Isoc, Inter transfers. Added code for isoc transfers,
removed unusable code, done minor fixes. Affected functions
and IRQ handlers:
- dwc2_hsotg_start_req(),
- dwc2_hsotg_ep_enable(),
- dwc2_hsotg_ep_queue(),
- dwc2_hsotg_handle_outdone(),
- GINTSTS_GOUTNAKEFF handler,
Removed 'has_correct_parity' flag from 'dwc2_hsotg_ep' struct.
Before this patch series, to set the data pid the DWC2 gadget
driver was toggling the even/odd until it match, then were
leaving it set. But now I have added mechanism to set pid and
excluded all code where this flag was set.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/core.h | 1 -
drivers/usb/dwc2/gadget.c | 97 ++-
drivers/usb/dwc2/hw.h | 1 +
3 files changed, 71 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index f2e7de7..1902c54 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -188,7 +188,6 @@ struct dwc2_hsotg_ep {
unsigned intperiodic:1;
unsigned intisochronous:1;
unsigned intsend_zlp:1;
- unsigned inthas_correct_parity:1;
unsigned inttarget_frame;
#define TARGET_FRAME_INITIAL 0x
boolframe_overrun;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 12ec6a1..e3c5bfa 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -667,6 +667,16 @@ static void dwc2_hsotg_start_req(struct dwc2_hsotg *hsotg,
__func__, &ureq->dma, dma_reg);
}
+ if (hs_ep->isochronous && hs_ep->interval == 1) {
+ hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+ dwc2_gadget_incr_frame_num(hs_ep);
+
+ if (hs_ep->target_frame & 0x1)
+ ctrl |= DXEPCTL_SETODDFR;
+ else
+ ctrl |= DXEPCTL_SETEVENFR;
+ }
+
ctrl |= DXEPCTL_EPENA; /* ensure ep enabled */
dev_dbg(hsotg->dev, "ep0 state:%d\n", hsotg->ep0_state);
@@ -863,9 +873,18 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct
usb_request *req,
first = list_empty(&hs_ep->queue);
list_add_tail(&hs_req->queue, &hs_ep->queue);
- if (first)
- dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
+ if (first) {
+ if (!hs_ep->isochronous) {
+ dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
+ return 0;
+ }
+ while (dwc2_gadget_target_frame_elapsed(hs_ep))
+ dwc2_gadget_incr_frame_num(hs_ep);
+
+ if (hs_ep->target_frame != TARGET_FRAME_INITIAL)
+ dwc2_hsotg_start_req(hs, hs_ep, hs_req, false);
+ }
return 0;
}
@@ -1673,9 +1692,10 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg
*hsotg, int epnum)
* adjust the ISOC parity here.
*/
if (!using_dma(hsotg)) {
- hs_ep->has_correct_parity = 1;
if (hs_ep->isochronous && hs_ep->interval == 1)
dwc2_hsotg_change_ep_iso_parity(hsotg, DOEPCTL(epnum));
+ else if (hs_ep->isochronous && hs_ep->interval > 1)
+ dwc2_gadget_incr_frame_num(hs_ep);
}
dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, result);
@@ -2216,11 +2236,10 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg,
unsigned int idx,
if (idx == 0 && (ints & (DXEPINT_SETUP | DXEPINT_SETUP_RCVD)))
ints &= ~DXEPINT_XFERCOMPL;
- if (ints & DXEPINT_XFERCOMPL) {
- hs_ep->has_correct_parity = 1;
- if (hs_ep->isochronous && hs_ep->interval == 1)
- dwc2_hsotg_change_ep_iso_parity(hsotg, epctl_reg);
+ if (ints & DXEPINT_STSPHSERCVD)
+ dev_dbg(hsotg->dev, "%s: StsPhseRcvd asserted\n", __func__);
+ if (ints & DXEPINT_XFERCOMPL) {
dev_dbg(hsotg->dev,
"%s: XferCompl: DxEPCTL=0x%08x, DXEPTSIZ=%08x\n",
__func__, dwc2_readl(hsotg->regs + epctl_reg),
@@ -2231,7 +2250,12 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg,
unsigned int idx,
* at completing IN requests here
*/
if (dir_in) {
+ if (hs_ep->isochronous && hs_ep->interval > 1)
+ dwc2_gadget_incr_frame_num(hs_ep);
+
dwc2_hsotg_complete_in(hsotg, hs_ep);
+ if (ints & DXEPINT_NAKINTRPT)
+ ints &= ~DXEPINT_NAKINTRPT;
if (idx == 0 && !hs_ep->req)
dwc2_hsotg_
[PATCH v3 08/13] usb: dwc2: gadget: Add dwc2_gadget_read_ep_interrupts function
From: Vardan Mikayelyan
Reads and returns interrupts for given endpoint, by masking epint_reg
with corresponding mask.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 2e98241..051eb11 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1947,6 +1947,34 @@ static void dwc2_hsotg_complete_in(struct dwc2_hsotg
*hsotg,
}
/**
+ * dwc2_gadget_read_ep_interrupts - reads interrupts for given ep
+ * @hsotg: The device state.
+ * @idx: Index of ep.
+ * @dir_in: Endpoint direction 1-in 0-out.
+ *
+ * Reads for endpoint with given index and direction, by masking
+ * epint_reg with coresponding mask.
+ */
+static u32 dwc2_gadget_read_ep_interrupts(struct dwc2_hsotg *hsotg,
+ unsigned int idx, int dir_in)
+{
+ u32 epmsk_reg = dir_in ? DIEPMSK : DOEPMSK;
+ u32 epint_reg = dir_in ? DIEPINT(idx) : DOEPINT(idx);
+ u32 ints;
+ u32 mask;
+ u32 diepempmsk;
+
+ mask = dwc2_readl(hsotg->regs + epmsk_reg);
+ diepempmsk = dwc2_readl(hsotg->regs + DIEPEMPMSK);
+ mask |= ((diepempmsk >> idx) & 0x1) ? DIEPMSK_TXFIFOEMPTY : 0;
+ mask |= DXEPINT_SETUP_RCVD;
+
+ ints = dwc2_readl(hsotg->regs + epint_reg);
+ ints &= mask;
+ return ints;
+}
+
+/**
* dwc2_hsotg_epint - handle an in/out endpoint interrupt
* @hsotg: The driver state
* @idx: The index for the endpoint (0..15)
@@ -1964,7 +1992,7 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg,
unsigned int idx,
u32 ints;
u32 ctrl;
- ints = dwc2_readl(hsotg->regs + epint_reg);
+ ints = dwc2_gadget_read_ep_interrupts(hsotg, idx, dir_in);
ctrl = dwc2_readl(hsotg->regs + epctl_reg);
/* Clear endpoint interrupts */
--
2.8.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
[PATCH v3 09/13] usb: dwc2: gadget: Add dwc2_gadget_start_next_request function
From: Vardan Mikayelyan
Replaced repeating code with function call.
Starts next request from ep queue.
If queue is empty and ep is isoc
-In case of OUT-EP unmasks OUTTKNEPDIS.
OUTTKNEPDIS is masked in it's handler, so we need to unmask it here
to be able to do resynchronization.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 51 +++
1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 051eb11..8faddbd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1045,6 +1045,42 @@ static struct dwc2_hsotg_req *get_ep_head(struct
dwc2_hsotg_ep *hs_ep)
}
/**
+ * dwc2_gadget_start_next_request - Starts next request from ep queue
+ * @hs_ep: Endpoint structure
+ *
+ * If queue is empty and EP is ISOC-OUT - unmasks OUTTKNEPDIS which is masked
+ * in its handler. Hence we need to unmask it here to be able to do
+ * resynchronization.
+ */
+static void dwc2_gadget_start_next_request(struct dwc2_hsotg_ep *hs_ep)
+{
+ u32 mask;
+ struct dwc2_hsotg *hsotg = hs_ep->parent;
+ int dir_in = hs_ep->dir_in;
+ struct dwc2_hsotg_req *hs_req;
+ u32 epmsk_reg = dir_in ? DIEPMSK : DOEPMSK;
+
+ if (!list_empty(&hs_ep->queue)) {
+ hs_req = get_ep_head(hs_ep);
+ dwc2_hsotg_start_req(hsotg, hs_ep, hs_req, false);
+ return;
+ }
+ if (!hs_ep->isochronous)
+ return;
+
+ if (dir_in) {
+ dev_dbg(hsotg->dev, "%s: No more ISOC-IN requests\n",
+ __func__);
+ } else {
+ dev_dbg(hsotg->dev, "%s: No more ISOC-OUT requests\n",
+ __func__);
+ mask = dwc2_readl(hsotg->regs + epmsk_reg);
+ mask |= DOEPMSK_OUTTKNEPDISMSK;
+ dwc2_writel(mask, hsotg->regs + epmsk_reg);
+ }
+}
+
+/**
* dwc2_hsotg_process_req_feature - process request {SET,CLEAR}_FEATURE
* @hsotg: The device state
* @ctrl: USB control request
@@ -1054,7 +1090,6 @@ static int dwc2_hsotg_process_req_feature(struct
dwc2_hsotg *hsotg,
{
struct dwc2_hsotg_ep *ep0 = hsotg->eps_out[0];
struct dwc2_hsotg_req *hs_req;
- bool restart;
bool set = (ctrl->bRequest == USB_REQ_SET_FEATURE);
struct dwc2_hsotg_ep *ep;
int ret;
@@ -1137,12 +1172,7 @@ static int dwc2_hsotg_process_req_feature(struct
dwc2_hsotg *hsotg,
/* If we have pending request, then start it */
if (!ep->req) {
- restart = !list_empty(&ep->queue);
- if (restart) {
- hs_req = get_ep_head(ep);
- dwc2_hsotg_start_req(hsotg, ep,
- hs_req, false);
- }
+ dwc2_gadget_start_next_request(ep);
}
}
@@ -1383,7 +1413,6 @@ static void dwc2_hsotg_complete_request(struct dwc2_hsotg
*hsotg,
struct dwc2_hsotg_req *hs_req,
int result)
{
- bool restart;
if (!hs_req) {
dev_dbg(hsotg->dev, "%s: nothing to complete?\n", __func__);
@@ -1427,11 +1456,7 @@ static void dwc2_hsotg_complete_request(struct
dwc2_hsotg *hsotg,
*/
if (!hs_ep->req && result >= 0) {
- restart = !list_empty(&hs_ep->queue);
- if (restart) {
- hs_req = get_ep_head(hs_ep);
- dwc2_hsotg_start_req(hsotg, hs_ep, hs_req, false);
- }
+ dwc2_gadget_start_next_request(hs_ep);
}
}
--
2.8.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
[PATCH v3 11/13] usb: dwc2: gadget: Add Incomplete ISO IN/OUT Interrupt handlers
From: Vardan Mikayelyan
Incomplete ISO IN interrupt indicates one of the following conditions
occurred while transmitting an ISOC transaction.
- Corrupted IN Token for ISOC EP.
- Packet not complete in FIFO.
Incomplete ISO OUT indicates that there is at least one isochronous OUT
endpoint on which the transfer is not completed in the current
microframe.
The following actions will be taken:
In case of EP-IN
- Determine the EP
- Disable EP directly from this handler; when "Endpoint Disabled"
interrupt is received flush FIFO
In case of EP-OUT
- Determine the EP
- If target frame elapsed set DCTL_SGOUTNAK, unmask GOUTNAKEFF and
proceed as described in section 7.5.1 of DWC-HSOTG Programming Guide
Also added dwc2_gadget_target_frame_elapsed() helper function which
will be used in Incomplete ISO IN/OUT Interrupt handlers.
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/gadget.c | 173 +-
1 file changed, 124 insertions(+), 49 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index adc3381..4f83e04 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -523,6 +523,23 @@ static unsigned get_ep_limit(struct dwc2_hsotg_ep *hs_ep)
}
/**
+* dwc2_hsotg_read_frameno - read current frame number
+* @hsotg: The device instance
+*
+* Return the current frame number
+*/
+static u32 dwc2_hsotg_read_frameno(struct dwc2_hsotg *hsotg)
+{
+ u32 dsts;
+
+ dsts = dwc2_readl(hsotg->regs + DSTS);
+ dsts &= DSTS_SOFFN_MASK;
+ dsts >>= DSTS_SOFFN_SHIFT;
+
+ return dsts;
+}
+
+/**
* dwc2_hsotg_start_req - start a USB request from an endpoint's queue
* @hsotg: The controller state.
* @hs_ep: The endpoint to process a request for
@@ -783,6 +800,30 @@ static void
dwc2_hsotg_handle_unaligned_buf_complete(struct dwc2_hsotg *hsotg,
hs_req->saved_req_buf = NULL;
}
+/**
+ * dwc2_gadget_target_frame_elapsed - Checks target frame
+ * @hs_ep: The driver endpoint to check
+ *
+ * Returns 1 if targeted frame elapsed. If returned 1 then we need to drop
+ * corresponding transfer.
+ */
+static bool dwc2_gadget_target_frame_elapsed(struct dwc2_hsotg_ep *hs_ep)
+{
+ struct dwc2_hsotg *hsotg = hs_ep->parent;
+ u32 target_frame = hs_ep->target_frame;
+ u32 current_frame = dwc2_hsotg_read_frameno(hsotg);
+ bool frame_overrun = hs_ep->frame_overrun;
+
+ if (!frame_overrun && current_frame >= target_frame)
+ return true;
+
+ if (frame_overrun && current_frame >= target_frame &&
+ ((current_frame - target_frame) < DSTS_SOFFN_LIMIT / 2))
+ return true;
+
+ return false;
+}
+
static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct usb_request *req,
gfp_t gfp_flags)
{
@@ -1641,23 +1682,6 @@ static void dwc2_hsotg_handle_outdone(struct dwc2_hsotg
*hsotg, int epnum)
}
/**
- * dwc2_hsotg_read_frameno - read current frame number
- * @hsotg: The device instance
- *
- * Return the current frame number
- */
-static u32 dwc2_hsotg_read_frameno(struct dwc2_hsotg *hsotg)
-{
- u32 dsts;
-
- dsts = dwc2_readl(hsotg->regs + DSTS);
- dsts &= DSTS_SOFFN_MASK;
- dsts >>= DSTS_SOFFN_SHIFT;
-
- return dsts;
-}
-
-/**
* dwc2_hsotg_handle_rx - RX FIFO has data
* @hsotg: The device instance
*
@@ -2571,6 +2595,85 @@ void dwc2_hsotg_core_connect(struct dwc2_hsotg *hsotg)
}
/**
+ * dwc2_gadget_handle_incomplete_isoc_in - handle incomplete ISO IN Interrupt.
+ * @hsotg: The device state:
+ *
+ * This interrupt indicates one of the following conditions occurred while
+ * transmitting an ISOC transaction.
+ * - Corrupted IN Token for ISOC EP.
+ * - Packet not complete in FIFO.
+ *
+ * The following actions will be taken:
+ * - Determine the EP
+ * - Disable EP; when 'Endpoint Disabled' interrupt is received Flush FIFO
+ */
+static void dwc2_gadget_handle_incomplete_isoc_in(struct dwc2_hsotg *hsotg)
+{
+ struct dwc2_hsotg_ep *hs_ep;
+ u32 epctrl;
+ u32 idx;
+
+ dev_dbg(hsotg->dev, "Incomplete isoc in interrupt received:\n");
+
+ for (idx = 1; idx <= hsotg->num_of_eps; idx++) {
+ hs_ep = hsotg->eps_in[idx];
+ epctrl = dwc2_readl(hsotg->regs + DIEPCTL(idx));
+ if ((epctrl & DXEPCTL_EPENA) && hs_ep->isochronous &&
+ dwc2_gadget_target_frame_elapsed(hs_ep)) {
+ epctrl |= DXEPCTL_SNAK;
+ epctrl |= DXEPCTL_EPDIS;
+ dwc2_writel(epctrl, hsotg->regs + DIEPCTL(idx));
+ }
+ }
+
+ /* Clear interrupt */
+ dwc2_writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS);
+}
+
+/**
+ * dwc2_gadget_handle_incomplete_isoc_out - handle incomplete ISO OUT Interrupt
+ * @hsotg: The device state:
+ *
+ * This interrupt indicates one of the following conditions occurred
[PATCH v3 06/13] usb: dwc2: gadget: Add dwc2_gadget_incr_frame_num()
From: Vardan Mikayelyan
Increases and checks targeted frame number of current ep
if overrun happened, sets flag and masks with DSTS_SOFFN_LIMIT
Added following fields to struct dwc2_hsotg_ep
-target_frame: Targeted frame num to setup next ISOC transfer
-frame_overrun: Indicates SOF number overrun in DSTS
Tested-by: John Keeping
Signed-off-by: Vardan Mikayelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/core.h | 5 +
drivers/usb/dwc2/gadget.c | 19 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3c58d63..22fa1f5 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -150,6 +150,8 @@ struct dwc2_hsotg_req;
* @fifo_load: The amount of data loaded into the FIFO (periodic IN)
* @last_load: The offset of data for the last start of request.
* @size_loaded: The last loaded size for DxEPTSIZE for periodic IN
+ * @target_frame: Targeted frame num to setup next ISOC transfer
+ * @frame_overrun: Indicates SOF number overrun in DSTS
*
* This is the driver's state for each registered enpoint, allowing it
* to keep track of transactions that need doing. Each endpoint has a
@@ -187,6 +189,9 @@ struct dwc2_hsotg_ep {
unsigned intisochronous:1;
unsigned intsend_zlp:1;
unsigned inthas_correct_parity:1;
+ unsigned inttarget_frame;
+#define TARGET_FRAME_INITIAL 0x
+ boolframe_overrun;
charname[10];
};
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 22d4f5b..7f15bc0 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -97,6 +97,25 @@ static inline bool using_dma(struct dwc2_hsotg *hsotg)
}
/**
+ * dwc2_gadget_incr_frame_num - Increments the targeted frame number.
+ * @hs_ep: The endpoint
+ * @increment: The value to increment by
+ *
+ * This function will also check if the frame number overruns DSTS_SOFFN_LIMIT.
+ * If an overrun occurs it will wrap the value and set the frame_overrun flag.
+ */
+static inline void dwc2_gadget_incr_frame_num(struct dwc2_hsotg_ep *hs_ep)
+{
+ hs_ep->target_frame += hs_ep->interval;
+ if (hs_ep->target_frame > DSTS_SOFFN_LIMIT) {
+ hs_ep->frame_overrun = 1;
+ hs_ep->target_frame &= DSTS_SOFFN_LIMIT;
+ } else {
+ hs_ep->frame_overrun = 0;
+ }
+}
+
+/**
* dwc2_hsotg_en_gsint - enable one or more of the general interrupt
* @hsotg: The device state
* @ints: A bitmask of the interrupts to enable
--
2.8.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
[PATCH] reset: Put back *_optional variants
Prior to commit 6c96f05c8bb8 ("reset: Make [of_]reset_control_get[_foo]
functions wrappers"), the optional variants returned -ENOTSUPP when
CONFIG_RESET_CONTROLLER was not set. This patch reverts to this
behavior. Otherwise those calls will return -EINVAL causing users to
think that an error occurred when CONFIG_RESET_CONTROLLER is not set.
Fixes: 6c96f05c8bb8 ("reset: Make [of_]reset_control_get[_foo] functions
wrappers")
Signed-off-by: John Youn
---
Hi Philipp, Hans,
The commit referenced above breaks an upcoming patch for the dwc2
driver that adds an optional reset control.
https://marc.info/?l=linux-usb&m=146161328211584&w=2
I've attempted to add the optional variants back the way they were
working before. Let me know if I need to do anything else to fix it or
if it should be done another way.
Regards,
John
include/linux/reset.h | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/include/linux/reset.h b/include/linux/reset.h
index ec0306ce..739d33d 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -14,10 +14,24 @@ int reset_control_status(struct reset_control *rstc);
struct reset_control *__of_reset_control_get(struct device_node *node,
const char *id, int index, int shared);
+
+struct reset_control *__of_reset_control_get_optional(struct device_node *node,
+const char *id, int index, int shared)
+{
+ return __of_reset_control_get(node, id, index, shared);
+}
+
void reset_control_put(struct reset_control *rstc);
struct reset_control *__devm_reset_control_get(struct device *dev,
const char *id, int index, int shared);
+static inline struct reset_control *__devm_reset_control_get_optional(
+ struct device *dev,
+ const char *id, int index, int shared)
+{
+ return __devm_reset_control_get(dev, id, index, shared);
+}
+
int __must_check device_reset(struct device *dev);
static inline int device_reset_optional(struct device *dev)
@@ -74,6 +88,13 @@ static inline struct reset_control *__of_reset_control_get(
return ERR_PTR(-EINVAL);
}
+static inline struct reset_control *__of_reset_control_get_optional(
+ struct device_node *node,
+ const char *id, int index, int shared)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
static inline struct reset_control *__devm_reset_control_get(
struct device *dev,
const char *id, int index, int shared)
@@ -81,6 +102,13 @@ static inline struct reset_control
*__devm_reset_control_get(
return ERR_PTR(-EINVAL);
}
+static inline struct reset_control *__devm_reset_control_get_optional(
+ struct device *dev,
+ const char *id, int index, int shared)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
#endif /* CONFIG_RESET_CONTROLLER */
/**
@@ -110,7 +138,8 @@ static inline struct reset_control *__must_check
reset_control_get(
static inline struct reset_control *reset_control_get_optional(
struct device *dev, const char *id)
{
- return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 0);
+ return __of_reset_control_get_optional(dev ? dev->of_node : NULL,
+ id, 0, 0);
}
/**
@@ -194,7 +223,7 @@ static inline struct reset_control *__must_check
devm_reset_control_get(
static inline struct reset_control *devm_reset_control_get_optional(
struct device *dev, const char *id)
{
- return __devm_reset_control_get(dev, id, 0, 0);
+ return __devm_reset_control_get_optional(dev, id, 0, 0);
}
/**
--
2.8.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
[PATCH] usb: dwc3: Set the ClearPendIN bit on Clear Stall EP command
As of core revision 2.60a the recommended programming model is to set
the ClearPendIN bit when issuing a Clear Stall EP command for IN
endpoints. This is to prevent an issue where some hosts may not send ACK
TPs for pending IN transfers due to various error conditions. Synopsys
STAR 9000614252.
Signed-off-by: John Youn
---
drivers/usb/dwc3/core.h | 1 +
drivers/usb/dwc3/gadget.c | 23 +--
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 74dff47..dcdba14 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -417,6 +417,7 @@
#define DWC3_DEPCMD_GET_RSC_IDX(x) (((x) >> DWC3_DEPCMD_PARAM_SHIFT) &
0x7f)
#define DWC3_DEPCMD_STATUS(x) (((x) >> 12) & 0x0F)
#define DWC3_DEPCMD_HIPRI_FORCERM (1 << 11)
+#define DWC3_DEPCMD_CLEARPENDIN(1 << 11)
#define DWC3_DEPCMD_CMDACT (1 << 10)
#define DWC3_DEPCMD_CMDIOC (1 << 8)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 7732650..86d9edb 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -334,6 +334,21 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned
cmd,
return ret;
}
+static int dwc3_send_clear_stall_ep_cmd(struct dwc3_ep *dep)
+{
+ struct dwc3 *dwc = dep->dwc;
+ struct dwc3_gadget_ep_cmd_params params;
+ u32 cmd = DWC3_DEPCMD_CLEARSTALL;
+
+ if (dep->direction &&
+ (dwc3_is_usb31(dwc) || (dwc->revision >= DWC3_REVISION_260A)))
+ cmd |= DWC3_DEPCMD_CLEARPENDIN;
+
+ memset(¶ms, 0, sizeof(params));
+
+ return dwc3_send_gadget_ep_cmd(dep, cmd, ¶ms);
+}
+
static dma_addr_t dwc3_trb_dma_offset(struct dwc3_ep *dep,
struct dwc3_trb *trb)
{
@@ -1290,8 +1305,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int
value, int protocol)
else
dep->flags |= DWC3_EP_STALL;
} else {
- ret = dwc3_send_gadget_ep_cmd(dep, DWC3_DEPCMD_CLEARSTALL,
- ¶ms);
+ ret = dwc3_send_clear_stall_ep_cmd(dep);
if (ret)
dev_err(dwc->dev, "failed to clear STALL on %s\n",
dep->name);
@@ -2300,7 +2314,6 @@ static void dwc3_clear_stall_all_ep(struct dwc3 *dwc)
for (epnum = 1; epnum < DWC3_ENDPOINTS_NUM; epnum++) {
struct dwc3_ep *dep;
- struct dwc3_gadget_ep_cmd_params params;
int ret;
dep = dwc->eps[epnum];
@@ -2312,9 +2325,7 @@ static void dwc3_clear_stall_all_ep(struct dwc3 *dwc)
dep->flags &= ~DWC3_EP_STALL;
- memset(¶ms, 0, sizeof(params));
- ret = dwc3_send_gadget_ep_cmd(dep, DWC3_DEPCMD_CLEARSTALL,
- ¶ms);
+ ret = dwc3_send_clear_stall_ep_cmd(dep);
WARN_ON_ONCE(ret);
}
}
--
2.8.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] usb: core: add debugobjects support for urb object
> On Tue, May 24, 2016 at 03:53:53PM +0800, changbin...@intel.com wrote: > > From: "Du, Changbin" > > > > Add debugobject support to track the life time of struct urb. > > This feature help us detect violation of urb operations by > > generating a warning message from debugobject core. And we fix > > the possible issues at runtime to avoid oops if we can. > > > > I have done some tests with some class drivers, no violation > > found in them which is good. Expect this feature can be used > > for debugging future problems. > > > > Signed-off-by: Du, Changbin > > I agree with Alan, what use is this code? Existing drivers all work > properly because the reference counting of urbs is already present, why > add duplicate counters? That actually leads to bugs. I don't see the > need for this code, please explain it better if you wish for it to be > accepted. What has it found/fixed that we have not found yet? What > does it allow you to do that you can't do today with the existing code? > > thanks, > > greg k-h > I agree with you two now after checking all urb code. I am sorry to disturb you. I did have an object lifetime issue, but it is of usb_request. I expect the cause is usb_request is freed but still pending in udc core. Then I cannot reproduce it, I even cannot know which function driver the usb_request came from. I originally want to use debugojects to debug that issue, then I though this can also help urb debugging. Obviously I am wrong. As you said reference counting of urbs is already present. :) I may add debugojects for device mode usb_request farther. Sometimes it helps. Thank you, Du, Changbin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 0/4] usb: dwc3: get rid of platform data
On 4/22/2016 1:18 AM, Heikki Krogerus wrote: > Changes since v1: > - Slit the refactoring and cleanup into separate patches > > > Heikki Krogerus (4): > usb: dwc3: pci: make build-in device properties available > usb: dwc3: pci: pass the platform device as a parameter to > dwc3_pci_quirks() > usb: dwc3: pci: use build-in properties instead of platform data > usb: dwc3: remove handling of platform data > > drivers/usb/dwc3/core.c | 35 - > drivers/usb/dwc3/dwc3-pci.c | 84 > ++-- > drivers/usb/dwc3/platform_data.h | 53 - > 3 files changed, 38 insertions(+), 134 deletions(-) > delete mode 100644 drivers/usb/dwc3/platform_data.h > Tested-by: John Youn I tested patch 3 and 4 against linux-next on a HAPS PCI platform. Needs a couple minor tweaks to apply but otherwise works as expected. Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFT PATCH] xhci: Fix handling timeouted commands on hosts in weird states.
> -Original Message-
> From: Joe Lawrence [mailto:joe.lawre...@stratus.com]
> Sent: Wednesday, May 25, 2016 9:04 PM
> To: Mathias Nyman
> Cc: linux-usb@vger.kernel.org; derek.sh...@stratus.com; Rajesh Bhagat
> ; stable
> Subject: Re: [RFT PATCH] xhci: Fix handling timeouted commands on hosts in
> weird
> states.
>
> On 05/12/2016 07:57 AM, Mathias Nyman wrote:
> > If commands timeout we mark them for abortion, then stop the command
> > ring, and turn the commands to no-ops and finally restart the command
> > ring.
> >
> > If the host is working properly the no-op commands will finish and
> > pending completions are called.
> > If we notice the host is failing driver clears the command ring and
> > completes, deletes and frees all pending commands.
> >
> > There are two separate cases reported where host is believed to work
> > properly but is not. In the first case we successfully stop the ring
> > but no abort or stop commnand ring event is ever sent and host locks up.
> >
> > The second case is if a host is removed, command times out and driver
> > believes the ring is stopped, and assumes it be restarted, but
> > actually ends up timing out on the same command forever.
> > If one of the pending commands has the xhci->mutex held it will block
> > xhci_stop() in the remove codepath which otherwise would cleanup
> > pending commands.
> >
> > Add a check that clears all pending commands in case host is removed,
> > or we are stuck timeouting on the same command. Also restart the
> > command timeout timer when stopping the command ring to ensure we
> > recive an ring stop/abort event.
> >
> > Cc: stable
> > Signed-off-by: Mathias Nyman
> > ---
> > drivers/usb/host/xhci-ring.c | 27 ++-
> > 1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c
> > b/drivers/usb/host/xhci-ring.c index 52deae4..c82570d 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -290,6 +290,14 @@ static int xhci_abort_cmd_ring(struct xhci_hcd
> > *xhci)
> >
> > temp_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
> > xhci->cmd_ring_state = CMD_RING_STATE_ABORTED;
> > +
> > + /*
> > +* Writing the CMD_RING_ABORT bit should cause a cmd completion event,
> > +* however on some host hw the CMD_RING_RUNNING bit is correctly cleared
> > +* but the completion event in never sent. Use the cmd timeout timer to
> > +* handle those cases. Use twice the time to cover the bit polling retry
> > +*/
> > + mod_timer(&xhci->cmd_timer, jiffies + (2 *
> > +XHCI_CMD_DEFAULT_TIMEOUT));
> > xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
> > &xhci->op_regs->cmd_ring);
> >
> > @@ -314,6 +322,7 @@ static int xhci_abort_cmd_ring(struct xhci_hcd
> > *xhci)
> >
> > xhci_err(xhci, "Stopped the command ring failed, "
> > "maybe the host is dead\n");
> > + del_timer(&xhci->cmd_timer);
> > xhci->xhc_state |= XHCI_STATE_DYING;
> > xhci_quiesce(xhci);
> > xhci_halt(xhci);
> > @@ -1246,22 +1255,21 @@ void xhci_handle_command_timeout(unsigned long
> data)
> > int ret;
> > unsigned long flags;
> > u64 hw_ring_state;
> > - struct xhci_command *cur_cmd = NULL;
> > + bool second_timeout = false;
> > xhci = (struct xhci_hcd *) data;
> >
> > /* mark this command to be cancelled */
> > spin_lock_irqsave(&xhci->lock, flags);
> > if (xhci->current_cmd) {
> > - cur_cmd = xhci->current_cmd;
> > - cur_cmd->status = COMP_CMD_ABORT;
> > + if (xhci->current_cmd->status == COMP_CMD_ABORT)
> > + second_timeout = true;
> > + xhci->current_cmd->status = COMP_CMD_ABORT;
> > }
> >
> > -
> > /* Make sure command ring is running before aborting it */
> > hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
> > if ((xhci->cmd_ring_state & CMD_RING_STATE_RUNNING) &&
> > (hw_ring_state & CMD_RING_RUNNING)) {
> > -
> > spin_unlock_irqrestore(&xhci->lock, flags);
> > xhci_dbg(xhci, "Command timeout\n");
> > ret = xhci_abort_cmd_ring(xhci);
> > @@ -1273,6 +1281,15 @@ void xhci_handle_command_timeout(unsigned long
> data)
> > }
> > return;
> > }
> > +
> > + /* command ring failed to restart, or host removed. Bail out */
> > + if (second_timeout || xhci->xhc_state & XHCI_STATE_REMOVING) {
> > + spin_unlock_irqrestore(&xhci->lock, flags);
> > + xhci_dbg(xhci, "command timed out twice, ring start fail?\n");
> > + xhci_cleanup_command_queue(xhci);
> > + return;
> > + }
> > +
> > /* command timeout on stopped ring, ring can't be aborted */
> > xhci_dbg(xhci, "Command timeout on stopped ring\n");
> > xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
> >
>
Hello Mathias,
I'm using kernel versi
[PATCH] dwc3: gadget: Introduce dwc3_endpoint_xfer_xxx() to check endpoint type
When handling the endpoint interrupt handler, it maybe disable the endpoint
from another core user to set the USB endpoint descriptor pointor to be NULL
while issuing usb_gadget_giveback_request() function to release lock. So it
will be one bug to check the endpoint type by usb_endpoint_xfer_xxx() APIs with
one NULL USB endpoint descriptor.
Thus this patch introduces safety dwc3_endpoint_xfer_xxx() APIs to check
endpoint type with checking if the endpoint flag 'DWC3_EP_ENABLED' is set or
not.
[ 4007.812527] c1 init(1) call enable_store(0)
[ 4007.812633] c0 dwc3 2050.dwc3: ep1out disabled while handling ep event
[ 4007.812657] c0 Unable to handle kernel NULL pointer dereference at virtual
address 0003
[ 4007.812669] c0 pgd = ffc0260a8000
[ 4007.812681] c0 [0003] *pgd=b4ab4003, *pud=b4ab4003,
*pmd=
[ 4007.812709] c0 Internal error: Oops: 9606 [#1] PREEMPT SMP
[ 4007.818441] c0 Modules linked in: bcmdhd
[ 4007.821854] c3 warning saudio_recv cache is empty!
[ 4007.821860] c3 aud_recv_cmd ENODATA
[ 4007.821869] c3 aud_send_cmd in,cmd =11 id:10 ret-value:0
[ 4007.821876] c3 saudio_send: dst=1, channel=0, timeout=-1
[ 4007.840767] c0 mali_kbase(O)
[ 4007.843707] c0
[ 4007.843889] c0 CPU: 0 PID: 8126 Comm: adbd Tainted: GW O 3.18.12+
#1
[ 4007.851153] c0 Hardware name: Spreadtrum SP9860g Board (DT)
[ 4007.856693] c0 task: ffc02638a400 ti: ffc026148000 task.ti:
ffc026148000
[ 4007.864404] c0 PC is at dwc3_interrupt_bh+0x710/0x112c
[ 4007.869503] c0 LR is at dwc3_interrupt_bh+0x70c/0x112c
[ 4007.874604] c0 pc : [] lr : [] pstate:
21c5
[ 4007.882218] c0 sp : ffc02614b970
[ 4007.885766] c0 x29: ffc02614b970 x28: ffc000c75f90
[ 4007.891044] c0 x27: 0008 x26:
[ 4007.896323] c0 x25: x24: 030c
[ 4007.901602] c0 x23: ffc000ff1000 x22: 0004
[ 4007.906881] c0 x21: ffc03d9fb5d8 x20: ffc13e573600
[ 4007.912160] c0 x19: ffc13e133020 x18: 0007
[ 4007.917437] c0 x17: 000e x16: 0001
[ 4007.922717] c0 x15: 0007 x14: 0fff
[ 4007.927996] c0 x13: 0001 x12: 0101010101010101
[ 4007.933274] c0 x11: 000c4f5d x10: 0002
[ 4007.938553] c0 x9 : x8 : 000c4f5e
[ 4007.943833] c0 x7 : 696c646e61682065 x6 : ffc000fa6cdc
[ 4007.949111] c0 x5 : x4 : 0105
[ 4007.954390] c0 x3 : x2 :
[ 4007.959669] c0 x1 : 5533250a x0 :
..
[ 4009.150699] c0 Call trace:
[ 4009.153394] c0 [] dwc3_interrupt_bh+0x710/0x112c
[ 4009.159532] c0 [] tasklet_action+0xb0/0x188
[ 4009.165243] c0 [] __do_softirq+0x114/0x3b4
[ 4009.170867] c0 [] irq_exit+0xa8/0xdc
[ 4009.175975] c0 [] __handle_domain_irq+0x90/0xf8
[ 4009.182030] c0 [] gic_handle_irq+0x58/0x1b4
[ 4009.187739] c0 Exception stack(0xffc02614bb70 to 0xffc02614bc90)
[ 4009.194404] c0 bb60: 0140
3e133108 ffc1
[ 4009.202802] c0 bb80: 2614bcd0 ffc0 009cff0c ffc0 8145
0018
[ 4009.211194] c0 bba0: 0400 00459900 ffc0
00d1aa50 ffc0
[ 4009.219589] c0 bbc0: 2614bc00 ffc0 26148000 ffc0 0001
3d2ce4e0 ffc1
[ 4009.227984] c0 bbe0: bdc0 0001 2638a980 ffc0
2614ba40 ffc0
[ 4009.236379] c0 bc00: 0400 0001 1d37bbb0 ffc0
0013
[ 4009.244772] c0 bc20: 000e 0007 0001
000e
[ 4009.253167] c0 bc40: 0007 0140 3e133108 ffc1
3e133108 ffc1
[ 4009.261560] c0 bc60: 0140 3eb833c0 ffc1 0018
0400
[ 4009.269951] c0 bc80: 1eb29b80 ffc1 3acd7c00 ffc0
[ 4009.275233] c0 [] el1_irq+0x68/0xdc
[ 4009.280255] c0 [] dwc3_gadget_ep_dequeue+0x180/0x1b8
[ 4009.286741] c0 [] ffs_epfile_io+0x330/0x374
[ 4009.292453] c0 [] ffs_epfile_read+0x30/0x40
[ 4009.298169] c0 [] vfs_read+0xa0/0x1dc
[ 4009.303357] c0 [] SyS_read+0x50/0xb0
Signed-off-by: Baolin Wang
---
drivers/usb/dwc3/gadget.c | 95 ++---
1 file changed, 72 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8e4a1b1..5d095f2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -36,6 +36,56 @@
#include "io.h"
/**
+* dwc3_endpoint_xfer_bulk - check if the endpoint has bulk transfer type
+* @ep: endpoint to be checked
+*
+* Returns true if the endpoint is of type bulk, otherwise it returns false.
+*/
+static inline int dwc3_endpoint_xfer_bulk(struct dwc3_ep *ep)
+{
+ return ((ep->flags & DWC3_EP_ENABLED) &&
+ ep->type == USB_ENDPOINT_XFER_BULK);
+}
+
+/**
+* dwc3_endpoint_xfer_control - check if the endpoint h
Re: [PATCH] dwc3: gadget: Introduce dwc3_endpoint_xfer_xxx() to check endpoint type
Hi, Baolin Wang writes: > When handling the endpoint interrupt handler, it maybe disable the endpoint > from another core user to set the USB endpoint descriptor pointor to be NULL > while issuing usb_gadget_giveback_request() function to release lock. So it > will be one bug to check the endpoint type by usb_endpoint_xfer_xxx() APIs > with > one NULL USB endpoint descriptor. too complex, Baolin :-) Can you see if this helps: https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?id=88bf752cfb55e57a78e27c931c9fef63240c739a The only situation when that can happen is while we drop our lock on dwc3_gadget_giveback(). -- balbi signature.asc Description: PGP signature
Re: [PATCH v2] usb: gadget: fix spinlock dead lock in gadgetfs
Hi,
Bin Liu writes:
> On Wed, May 25, 2016 at 02:18:34PM -0500, Bin Liu wrote:
>> [ 40.467381] =
>> [ 40.473013] [ INFO: possible recursive locking detected ]
>> [ 40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted
>> [ 40.483466] -
>> [ 40.489098] usb/733 is trying to acquire lock:
>> [ 40.493734] (&(&dev->lock)->rlock){-.}, at: []
>> ep0_complete+0x18/0xdc [gadgetfs]
>> [ 40.502882]
>> [ 40.502882] but task is already holding lock:
>> [ 40.508967] (&(&dev->lock)->rlock){-.}, at: []
>> ep0_read+0x20/0x5e0 [gadgetfs]
>> [ 40.517811]
>> [ 40.517811] other info that might help us debug this:
>> [ 40.524623] Possible unsafe locking scenario:
>> [ 40.524623]
>> [ 40.530798]CPU0
>> [ 40.533346]
>> [ 40.535894] lock(&(&dev->lock)->rlock);
>> [ 40.540088] lock(&(&dev->lock)->rlock);
>> [ 40.544284]
>> [ 40.544284] *** DEADLOCK ***
>> [ 40.544284]
>> [ 40.550461] May be due to missing lock nesting notation
>> [ 40.550461]
>> [ 40.557544] 2 locks held by usb/733:
>> [ 40.561271] #0: (&f->f_pos_lock){+.+.+.}, at: []
>> __fdget_pos+0x40/0x48
>> [ 40.569219] #1: (&(&dev->lock)->rlock){-.}, at: []
>> ep0_read+0x20/0x5e0 [gadgetfs]
>> [ 40.578523]
>> [ 40.578523] stack backtrace:
>> [ 40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37
>> [ 40.590246] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [ 40.596625] [] (unwind_backtrace) from []
>> (show_stack+0x10/0x14)
>> [ 40.604718] [] (show_stack) from []
>> (dump_stack+0xb0/0xe4)
>> [ 40.612267] [] (dump_stack) from []
>> (__lock_acquire+0xf68/0x1994)
>> [ 40.620440] [] (__lock_acquire) from []
>> (lock_acquire+0xd8/0x238)
>> [ 40.628621] [] (lock_acquire) from []
>> (_raw_spin_lock_irqsave+0x38/0x4c)
>> [ 40.637440] [] (_raw_spin_lock_irqsave) from []
>> (ep0_complete+0x18/0xdc [gadgetfs])
>> [ 40.647339] [] (ep0_complete [gadgetfs]) from []
>> (musb_g_giveback+0x118/0x1b0 [musb_hdrc])
>> [ 40.657842] [] (musb_g_giveback [musb_hdrc]) from []
>> (musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
>> [ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from []
>> (ep0_read+0x544/0x5e0 [gadgetfs])
>> [ 40.678963] [] (ep0_read [gadgetfs]) from []
>> (__vfs_read+0x20/0x110)
>> [ 40.687414] [] (__vfs_read) from []
>> (vfs_read+0x88/0x114)
>> [ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c)
>> [ 40.702051] [] (SyS_read) from []
>> (ret_fast_syscall+0x0/0x1c)
>>
>> Cc: # v3.16+
>> Signed-off-by: Bin Liu
>> ---
>> v2:
>> - lock protects setup_req(), only unlock for usb_ep_queue()
>> - use GFP_KERNEL in usb_ep_queue()
>> - fix the same in two places in gadgetfs_setup() too
>
> Never mind. Sent the patch too soon. It gives the following problem.
>
> [ 179.810367] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2724
> _raw_spin_unlock_irq+0x24/0x2c
> [ 179.819719] DEBUG_LOCKS_WARN_ON(current->hardirq_context)
thanks for letting us know. I'm dropping this from my queue. Please
resend final patch once you have it ;-)
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH] usb: host: ehci-msm: Conditionally call ehci suspend/resume
On 21 May 2016 at 03:05, Andy Gross wrote: > This patch fixes a suspend/resume issue where the driver is blindly > calling ehci_suspend/resume functions when the ehci hasn't been setup. > This results in a crash during suspend/resume operations. > > Signed-off-by: Andy Gross Fixes below crash while doing a system suspend: root@linaro-alip:~# echo freeze > /sys/power/state [ 22.348960] PM: Syncing filesystems ... done. [ 22.387778] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 22.393614] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 22.512736] mmc0: Reset 0x1 never completed. [ 22.514296] Unable to handle kernel NULL pointer dereference at virtual address 04e8 [ 22.516068] pgd = 80003817d000 [ 22.524161] [04e8] *pgd=b817e003, *pud=b817f003, *pmd= [ 22.535414] Internal error: Oops: 9606 [#1] PREEMPT SMP [ 22.535680] Modules linked in: [ 22.544183] CPU: 3 PID: 1499 Comm: bash Not tainted 4.6.0+ #65 [ 22.544275] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) [ 22.550006] task: 800039abd780 ti: 80003928c000 task.ti: 80003928c000 [ 22.556870] PC is at ehci_suspend+0x34/0xe4 [ 22.564240] LR is at ehci_msm_pm_suspend+0x2c/0x34 [ 22.568232] pc : [] lr : [] pstate: a145 Tested-by: Pramod Gurav > --- > drivers/usb/host/ehci-msm.c | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) > Regards, Pramod -- 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
