Re: [PATCH v2] USB: serial: cp210x: add support for ELV TFD500
On Mon, Sep 18, 2017 at 09:11:57PM +0200, Andreas Engel wrote: > Add the USB device id for the ELV TFD500 data logger. > > Signed-off-by: Andreas Engel Thanks for the v2. Now applied. Johan -- 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: ASM1153 detected as ASM1051 and breaking UAS
Am Dienstag, den 12.09.2017, 11:19 +0200 schrieb Massimo Burcheri: > > I asked the manufacturer service why the device reports the wrong product id. > I got the reply now: > > "In order to use UAS that must be enabled in the firmware as well. On that > device it is not enabled and the official product id wouldn't help there > either." > > What does that mean? Did I purchase a UAS capable hardware with the wrong Yes. That is what they told you. With a bit of sugar coating. > firmware? Is that possible to change? If they provide a tool for updating the firmware and updating the firmware is supported. Which may be the case or not. Only the vendor can tell you. 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
[PATCH] usb: gadget: core: fix ->udc_set_speed() logic
Consider the following case: udc controller supports SuperSpeed.
If we first load a HighSpeed gadget followed by a SuperSpeed gadget,
the SuperSpeed gadget will be limited to HighSpeed as UDC core
driver doesn't call ->udc_set_speed() in the second case.
Call ->udc_set_speed() unconditionally to fix this issue.
This will also fix the case for dwc3 controller driver when SuperSpeed
gadget is loaded first and works in HighSpeed only as udc_set_speed()
was never being called.
Fixes: 6099eca796ae ("usb: gadget: core: introduce ->udc_set_speed() method")
Cc: [v4.13+]
Signed-off-by: Roger Quadros
---
drivers/usb/gadget/udc/core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 75c51ca..d41d07a 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1320,8 +1320,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct
usb_gadget_driver *dri
udc->dev.driver = &driver->driver;
udc->gadget->dev.driver = &driver->driver;
- if (driver->max_speed < udc->gadget->max_speed)
- usb_gadget_udc_set_speed(udc, driver->max_speed);
+ usb_gadget_udc_set_speed(udc, driver->max_speed);
ret = driver->bind(udc->gadget, driver);
if (ret)
--
2.7.4
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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: Gadget mode advice sought
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 16/09/17 19:36, g...@novadsp.com wrote: > Greetings, > > I'm trying to find the simplest way to develop a bulk mode gadget that > exposes a standard userland IO interface. I've not been able to find > anything suitable but if such a beast does exist please do point me in the > right direction. Failing that please read on. You should use the serial (tty) interface for this (see f_serial.c / serial.c) > > The appliance collects data via video and other sensors. There is an > intermediate userland processing application that sources from video (V4L2), > SPI, etc. and would write its output to the host via USB. > > V4L2 on the target H3 SoC is only (currently) supported with a 3.4.X kernel. > Thus newer gadget FS options marked as experimental. This may be a > significant constraint, I am not yet sure. > > This is an embedded appliance and g_zero does work. Thus I am minded to > extend g_zero, adding fileops etc. so it appears as a standard character > mode driver in /dev. Read() and write() would simply be hooked in to the > source_sink_complete() handler. Does this sense? No. g_zero is meant only for testing purposes. > > Any comments/thought much appreciated. > > TAIA > > Jerry. > > -- > 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 > -- cheers, -roger -- 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 8/9] atmel_flexcom: Support backup mode
On 15/09/2017 at 16:04, Romain Izard wrote:
> The controller used by a flexcom module is configured at boot, and left
> alone after this. As the configuration will be lost after backup mode,
> restore the state of the flexcom driver on resume.
>
> Signed-off-by: Romain Izard
Tested-by: Nicolas Ferre
On sama5d2 Xplained board (i2c0 from flexcom 4).
and obviously:
Acked-by: Nicolas Ferre
Thanks Romain!
Regards,
> ---
> drivers/mfd/atmel-flexcom.c | 65
> ++---
> 1 file changed, 50 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
> index 064bde9cff5a..ef1235c4a179 100644
> --- a/drivers/mfd/atmel-flexcom.c
> +++ b/drivers/mfd/atmel-flexcom.c
> @@ -39,34 +39,44 @@
> #define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) &
> \
>FLEX_MR_OPMODE_MASK)
>
> +struct atmel_flexcom {
> + void __iomem *base;
> + u32 opmode;
> + struct clk *clk;
> +};
>
> static int atmel_flexcom_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> - struct clk *clk;
> struct resource *res;
> - void __iomem *base;
> - u32 opmode;
> + struct atmel_flexcom *afc;
> int err;
> + u32 val;
> +
> + afc = devm_kzalloc(&pdev->dev, sizeof(*afc), GFP_KERNEL);
> + if (!afc)
> + return -ENOMEM;
>
> - err = of_property_read_u32(np, "atmel,flexcom-mode", &opmode);
> + platform_set_drvdata(pdev, afc);
> +
> + err = of_property_read_u32(np, "atmel,flexcom-mode", &afc->opmode);
> if (err)
> return err;
>
> - if (opmode < ATMEL_FLEXCOM_MODE_USART ||
> - opmode > ATMEL_FLEXCOM_MODE_TWI)
> + if (afc->opmode < ATMEL_FLEXCOM_MODE_USART ||
> + afc->opmode > ATMEL_FLEXCOM_MODE_TWI)
> return -EINVAL;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> + afc->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(afc->base))
> + return PTR_ERR(afc->base);
>
> - clk = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(clk))
> - return PTR_ERR(clk);
> + afc->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(afc->clk))
> + return PTR_ERR(afc->clk);
>
> - err = clk_prepare_enable(clk);
> + err = clk_prepare_enable(afc->clk);
> if (err)
> return err;
>
> @@ -76,9 +86,10 @@ static int atmel_flexcom_probe(struct platform_device
> *pdev)
>* inaccessible and are read as zero. Also the external I/O lines of the
>* Flexcom are muxed to reach the selected device.
>*/
> - writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
> + val = FLEX_MR_OPMODE(afc->opmode);
> + writel(val, afc->base + FLEX_MR);
>
> - clk_disable_unprepare(clk);
> + clk_disable_unprepare(afc->clk);
>
> return devm_of_platform_populate(&pdev->dev);
> }
> @@ -89,10 +100,34 @@ static const struct of_device_id
> atmel_flexcom_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
>
> +#ifdef CONFIG_PM_SLEEP
> +static int atmel_flexcom_resume(struct device *dev)
> +{
> + struct atmel_flexcom *afc = dev_get_drvdata(dev);
> + int err;
> + u32 val;
> +
> + err = clk_prepare_enable(afc->clk);
> + if (err)
> + return err;
> +
> + val = FLEX_MR_OPMODE(afc->opmode),
> + writel(val, afc->base + FLEX_MR);
> +
> + clk_disable_unprepare(afc->clk);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
> + atmel_flexcom_resume);
> +
> static struct platform_driver atmel_flexcom_driver = {
> .probe = atmel_flexcom_probe,
> .driver = {
> .name = "atmel_flexcom",
> + .pm = &atmel_flexcom_pm_ops,
> .of_match_table = atmel_flexcom_of_match,
> },
> };
>
--
Nicolas Ferre
--
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 v5] xhci : AMD Promontory USB disable port support
From: asmtswfae
For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
settings, those ports will be enabled anyway after you remove a device on
that port and re-plug it in again. It's a known limitation of the chip.
As a workaround we can clear the PORT_WAKE_BITS.
Signed-off-by: asmtswfae
v5: Add check disable port setting before set PORT_WAKE_BITS
v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
v3: Fix some checkpatch.pl
---
drivers/usb/host/pci-quirks.c | 117 +-
drivers/usb/host/pci-quirks.h | 1 +
drivers/usb/host/xhci-hub.c | 7 ++-
3 files changed, 122 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 658d9d1..5926201 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -64,7 +64,22 @@
#defineAB_DATA(addr) ((addr) + 0x04)
#defineAX_INDXC0x30
#defineAX_DATAC0x34
-
+#define PT_ADDR_INDEX 0xE8
+#define PT_READ_INDEX 0xE4
+#define PT_SIG_1_ADDR 0xA520
+#define PT_SIG_2_ADDR 0xA521
+#define PT_SIG_3_ADDR 0xA522
+#define PT_SIG_4_ADDR 0xA523
+#define PT_SIG_1_DATA 0x78
+#define PT_SIG_2_DATA 0x56
+#define PT_SIG_3_DATA 0x34
+#define PT_SIG_4_DATA 0x12
+#define PT_4_PORT_1_REG0xB521
+#define PT_4_PORT_2_REG0xB522
+#define PT_2_PORT_1_REG0xD520
+#define PT_2_PORT_2_REG0xD521
+#define PT_1_PORT_1_REG0xD522
+#define PT_1_PORT_2_REG0xD523
#defineNB_PCIE_INDX_ADDR 0xe0
#defineNB_PCIE_INDX_DATA 0xe4
#definePCIE_P_CNTL 0x10040
@@ -511,6 +526,106 @@ void usb_amd_dev_put(void)
}
EXPORT_SYMBOL_GPL(usb_amd_dev_put);
+int usb_amd_pt_check_port(struct device *device, int port)
+{
+ unsigned char value;
+ struct pci_dev *pdev;
+
+ pdev = to_pci_dev(device);
+ pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_1_ADDR);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value != PT_SIG_1_DATA)
+ return 0;
+ pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_2_ADDR);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value != PT_SIG_2_DATA)
+ return 0;
+ pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_3_ADDR);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value != PT_SIG_3_DATA)
+ return 0;
+ pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_4_ADDR);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value != PT_SIG_4_DATA)
+ return 0;
+ if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
+ /* device is AMD_PROMONTORYA_4(0x43b9) or
+* PROMONTORYA_3(0x43ba)
+* disable port setting PT_4_PORT_1_REG[7..1] is
+* USB2.0 port6 to 0
+* PT_4_PORT_2_REG[6..0] is USB 13 to port 7
+* 0 : disable ;1 : enable
+*/
+ if (port > 6) {
+ pci_write_config_word(
+ pdev, PT_ADDR_INDEX, PT_4_PORT_2_REG);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value & (1<<(port - 7)))
+ return 0;
+ else
+ return 1;
+ } else {
+ pci_write_config_word(
+ pdev, PT_ADDR_INDEX, PT_4_PORT_1_REG);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value & (1<<(port + 1)))
+ return 0;
+ else
+ return 1;
+ }
+ } else if (pdev->device == 0x43bb) {
+ /* device is AMD_PROMONTORYA_2(0x43bb)
+* disable port setting PT_2_PORT_1_REG[7..5] is
+* USB2.0 port2 to
+* PT_2_PORT_2_REG[5..0] is USB9 to port3
+* 0 : disable ;1 : enable
+*/
+ if (port > 2) {
+ pci_write_config_word(
+ pdev, PT_ADDR_INDEX, PT_2_PORT_2_REG);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value & (1<<(port - 3)))
+ return 0;
+ else
+ return 1;
+ } else {
+ pci_write_config_word(
+ pdev, PT_ADDR_INDEX, PT_2_PORT_1_REG);
+ pci_read_config_byte(pdev, PT_READ_INDEX, &value);
+ if (value & (1<<(port + 5)))
+
Re: [PATCH v5] xhci : AMD Promontory USB disable port support
On Tue, Sep 19, 2017 at 05:40:04PM +0800, Joe Lee wrote:
> From: asmtswfae
Shouldn't the name here match the name up in your From: email line?
>
> For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
> settings, those ports will be enabled anyway after you remove a device on
> that port and re-plug it in again. It's a known limitation of the chip.
> As a workaround we can clear the PORT_WAKE_BITS.
>
> Signed-off-by: asmtswfae
Same here, use your "real" name.
> v5: Add check disable port setting before set PORT_WAKE_BITS
> v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
> v3: Fix some checkpatch.pl
These v: lines go below the --- line please.
>
>
>
> ---
> drivers/usb/host/pci-quirks.c | 117
> +-
> drivers/usb/host/pci-quirks.h | 1 +
> drivers/usb/host/xhci-hub.c | 7 ++-
> 3 files changed, 122 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 658d9d1..5926201 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -64,7 +64,22 @@
> #define AB_DATA(addr) ((addr) + 0x04)
> #define AX_INDXC0x30
> #define AX_DATAC0x34
> -
> +#define PT_ADDR_INDEX0xE8
> +#define PT_READ_INDEX0xE4
> +#define PT_SIG_1_ADDR0xA520
> +#define PT_SIG_2_ADDR0xA521
> +#define PT_SIG_3_ADDR0xA522
> +#define PT_SIG_4_ADDR0xA523
> +#define PT_SIG_1_DATA0x78
> +#define PT_SIG_2_DATA0x56
> +#define PT_SIG_3_DATA0x34
> +#define PT_SIG_4_DATA0x12
> +#define PT_4_PORT_1_REG 0xB521
> +#define PT_4_PORT_2_REG 0xB522
> +#define PT_2_PORT_1_REG 0xD520
> +#define PT_2_PORT_2_REG 0xD521
> +#define PT_1_PORT_1_REG 0xD522
> +#define PT_1_PORT_2_REG 0xD523
> #define NB_PCIE_INDX_ADDR 0xe0
> #define NB_PCIE_INDX_DATA 0xe4
> #define PCIE_P_CNTL 0x10040
> @@ -511,6 +526,106 @@ void usb_amd_dev_put(void)
> }
> EXPORT_SYMBOL_GPL(usb_amd_dev_put);
>
> +int usb_amd_pt_check_port(struct device *device, int port)
> +{
> + unsigned char value;
> + struct pci_dev *pdev;
> +
> + pdev = to_pci_dev(device);
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_1_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value != PT_SIG_1_DATA)
> + return 0;
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_2_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value != PT_SIG_2_DATA)
> + return 0;
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_3_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value != PT_SIG_3_DATA)
> + return 0;
> + pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_4_ADDR);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value != PT_SIG_4_DATA)
> + return 0;
> + if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
> + /* device is AMD_PROMONTORYA_4(0x43b9) or
> + * PROMONTORYA_3(0x43ba)
> + * disable port setting PT_4_PORT_1_REG[7..1] is
> + * USB2.0 port6 to 0
> + * PT_4_PORT_2_REG[6..0] is USB 13 to port 7
> + * 0 : disable ;1 : enable
> + */
> + if (port > 6) {
> + pci_write_config_word(
> + pdev, PT_ADDR_INDEX, PT_4_PORT_2_REG);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value & (1<<(port - 7)))
> + return 0;
> + else
> + return 1;
> + } else {
> + pci_write_config_word(
> + pdev, PT_ADDR_INDEX, PT_4_PORT_1_REG);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value & (1<<(port + 1)))
> + return 0;
> + else
> + return 1;
> + }
> + } else if (pdev->device == 0x43bb) {
> + /* device is AMD_PROMONTORYA_2(0x43bb)
> + * disable port setting PT_2_PORT_1_REG[7..5] is
> + * USB2.0 port2 to
> + * PT_2_PORT_2_REG[5..0] is USB9 to port3
> + * 0 : disable ;1 : enable
> + */
> + if (port > 2) {
> + pci_write_config_word(
> + pdev, PT_ADDR_INDEX, PT_2_PORT_2_REG);
> + pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> + if (value & (1<<(port - 3)))
> + return 0;
> +
Re: [PATCH v2 9/9] tty/serial: atmel: Prevent a warning on suspend
On 15/09/2017 at 16:04, Romain Izard wrote:
> The atmel serial port driver reported the following warning on suspend:
> atmel_usart f802.serial: ttyS1: Unable to drain transmitter
>
> As the ATMEL_US_TXEMPTY status bit in ATMEL_US_CSR is always cleared
> when the transmitter is disabled, we need to know the transmitter's
> state to return the real fifo state. And as ATMEL_US_CR is write-only,
> it is necessary to save the state of the transmitter in a local
> variable, and update the variable when TXEN and TXDIS is written in
> ATMEL_US_CR.
>
> After those changes, atmel_tx_empty can return "empty" on suspend, the
> warning in uart_suspend_port disappears, and suspending is 20ms shorter
> for each enabled Atmel serial port.
>
> Signed-off-by: Romain Izard
Tested-by: Nicolas Ferre
on sama5d2 Xplained.
Acked-by: Nicolas Ferre
> ---
> drivers/tty/serial/atmel_serial.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 7551cab438ff..783af6648be0 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -171,6 +171,7 @@ struct atmel_uart_port {
> boolhas_hw_timer;
> struct timer_list uart_timer;
>
> + booltx_stopped;
> boolsuspended;
> unsigned intpending;
> unsigned intpending_status;
> @@ -380,6 +381,10 @@ static int atmel_config_rs485(struct uart_port *port,
> */
> static u_int atmel_tx_empty(struct uart_port *port)
> {
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +
> + if (atmel_port->tx_stopped)
> + return TIOCSER_TEMT;
> return (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXEMPTY) ?
> TIOCSER_TEMT :
> 0;
> @@ -485,6 +490,7 @@ static void atmel_stop_tx(struct uart_port *port)
>* is fully transmitted.
>*/
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
> + atmel_port->tx_stopped = true;
>
> /* Disable interrupts */
> atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> @@ -492,6 +498,7 @@ static void atmel_stop_tx(struct uart_port *port)
> if ((port->rs485.flags & SER_RS485_ENABLED) &&
> !(port->rs485.flags & SER_RS485_RX_DURING_TX))
> atmel_start_rx(port);
> +
> }
>
> /*
> @@ -521,6 +528,7 @@ static void atmel_start_tx(struct uart_port *port)
>
> /* re-enable the transmitter */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> + atmel_port->tx_stopped = false;
> }
>
> /*
> @@ -1866,6 +1874,7 @@ static int atmel_startup(struct uart_port *port)
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> /* enable xmit & rcvr */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> + atmel_port->tx_stopped = false;
>
> setup_timer(&atmel_port->uart_timer,
> atmel_uart_timer_callback,
> @@ -2122,6 +2131,7 @@ static void atmel_set_termios(struct uart_port *port,
> struct ktermios *termios,
>
> /* disable receiver and transmitter */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
> + atmel_port->tx_stopped = true;
>
> /* mode */
> if (port->rs485.flags & SER_RS485_ENABLED) {
> @@ -2207,6 +2217,7 @@ static void atmel_set_termios(struct uart_port *port,
> struct ktermios *termios,
> atmel_uart_writel(port, ATMEL_US_BRGR, quot);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> + atmel_port->tx_stopped = false;
>
> /* restore interrupts */
> atmel_uart_writel(port, ATMEL_US_IER, imr);
> @@ -2450,6 +2461,7 @@ static void atmel_console_write(struct console *co,
> const char *s, u_int count)
>
> /* Make sure that tx path is actually able to send characters */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> + atmel_port->tx_stopped = false;
>
> uart_console_write(port, s, count, atmel_console_putchar);
>
> @@ -2511,6 +2523,7 @@ static int __init atmel_console_setup(struct console
> *co, char *options)
> {
> int ret;
> struct uart_port *port = &atmel_ports[co->index].uart;
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> int baud = 115200;
> int bits = 8;
> int parity = 'n';
> @@ -2528,6 +2541,7 @@ static int __init atmel_console_setup(struct console
> *co, char *options)
> atmel_uart_writel(port, ATMEL_US_IDR, -1);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> + atmel_port->tx_stopped = false;
>
> if (options)
> uart_parse_opt
Re: Fibocom L831-EAU and cdc_mbim
Andreas Böhler writes: > On 18/09/17 22:18, Bjørn Mork wrote: >> Andreas Böhler writes: >>> >>> I can also provide you with Wireshark USB traces, if that helps? >> >> It might help. We are obviously interacting badly with the firmware. >> Seeing where and how it stops would be useful. > > Attached are two Wireshark traces, I hope that I captured everything > relevant: > > 1. wwan_init_connect_failed.pcapng.gz > 2. wwan_reset_init_connect_success.pcapng.gz > > I disabled everything Modem-related (i.e. ModemManager), I blacklisted > the cdc* modules then I restarted the machine. The first trace was taken > directly after boot. I loaded the modules and I tried to enable > ModemManager and to connect to the Internet - it failed. > > The second trace was taken after disabling ModemManager and sending > "AT+CFUN=15" to the device, thus resetting it. I then enabled > ModemManager and connected to the Internet - which succeeded. Thanks a lot. These are very useful. The thing is that I really cannot see anything going wrong there. The "failed" capture just ends with a sequence of successful MBIM indications being read from the firmware. So whatever goes wrong looks like something entirely local to the driver. I'll try to poke around some more. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Gadget mode advice sought
Hello and thanks, > You should use the serial (tty) interface for this (see f_serial.c / serial.c) Not keen on that for binary data. > No. g_zero is meant only for testing purposes. Indeed. Problem is looking solved using GadgetFS working on the target board. Promise my next set of USB questions will be with for a contemporary kernel! Thanks again. Jerry. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/hid: slab-out-of-bounds read in usbhid_parse
Hi, Andrey Konovalov
Thanks for the report.
2017-09-19 2:33 GMT+09:00 Andrey Konovalov :
> Hi!
>
> I've got the following crash while fuzzing the kernel with syzkaller.
>
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>
> It seems that there's no proper check on the hdesc->bNumDescriptors
> value in usbhid_parse(). it iterates over hdesc->desc and accesses
> hdesc->desc[n] fields, which might be out-of-bounds.
The bNumDescriptors in hid descriptor means 'numeric expression
specifying the number of class descriptors'.
The value bNumberDescriptors identifies the number of additional class
specific descriptors present.
(refer to the 6.1.2 HID Descriptor in hid documents :
http://www.usb.org/developers/hidpage/HID1_11.pdf)
So, it can be out-of-bounds in hdesc->desc[n] if there is an
additional class descriptor.
Does the patch below fix this?
Thanks, jaejoong
--
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 089bad8..7b6a0b6 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -974,7 +974,7 @@ static int usbhid_parse(struct hid_device *hid)
u32 quirks = 0;
unsigned int rsize = 0;
char *rdesc;
- int ret, n;
+ int ret;
quirks = usbhid_lookup_quirk(le16_to_cpu(dev->descriptor.idVendor),
le16_to_cpu(dev->descriptor.idProduct));
@@ -1000,9 +1000,8 @@ static int usbhid_parse(struct hid_device *hid)
hid->version = le16_to_cpu(hdesc->bcdHID);
hid->country = hdesc->bCountryCode;
- for (n = 0; n < hdesc->bNumDescriptors; n++)
- if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
- rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength);
+ if (hdesc->desc[0].bDescriptorType == HID_DT_REPORT)
+ rsize = le16_to_cpu(hdesc->desc[0].wDescriptorLength);
if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) {
dbg_hid("weird size of report descriptor (%u)\n", rsize);
>
> ==
> BUG: KASAN: slab-out-of-bounds in usbhid_parse+0x9b1/0xa20
> Read of size 1 at addr 88006c5f8edf by task kworker/1:2/1261
>
> CPU: 1 PID: 1261 Comm: kworker/1:2 Not tainted
> 4.14.0-rc1-42251-gebb2c2437d80 #169
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> __dump_stack lib/dump_stack.c:16
> dump_stack+0x292/0x395 lib/dump_stack.c:52
> print_address_description+0x78/0x280 mm/kasan/report.c:252
> kasan_report_error mm/kasan/report.c:351
> kasan_report+0x22f/0x340 mm/kasan/report.c:409
> __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427
> usbhid_parse+0x9b1/0xa20 drivers/hid/usbhid/hid-core.c:1004
> hid_add_device+0x16b/0xb30 drivers/hid/hid-core.c:2944
> usbhid_probe+0xc28/0x1100 drivers/hid/usbhid/hid-core.c:1369
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
> hub_port_connect drivers/usb/core/hub.c:4903
> hub_port_connect_change drivers/usb/core/hub.c:5009
> port_event drivers/usb/core/hub.c:5115
> hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Allocated by task 1261:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> __kmalloc+0x14e/0x310 mm/slub.c:3782
> kmalloc ./include/linux/slab.h:498
> usb_get_configuration+0x372/0x2a60 drivers/usb/core/config.c:848
> usb_enumerate_device drivers/usb/core/hub.c:2290
> usb_new_device+0xaae/0x1020 drivers/usb/core/hub.c:2426
> hub_port_conn
Re: usb/core: slab-out-of-bounds in usb_set_configuration
On Mon, Sep 18, 2017 at 8:50 PM, Greg Kroah-Hartman
wrote:
> On Mon, Sep 18, 2017 at 07:22:24PM +0200, Andrey Konovalov wrote:
>> Hi!
>>
>> I've got the following crash while fuzzing the kernel with syzkaller.
>>
>> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>>
>> It seems there's no proper size check of a
>> USB_DT_INTERFACE_ASSOCIATION descriptor. It's only checked that the
>> size is >= 2 in usb_parse_configuration(), so find_iad() might do
>> out-of-bounds access to intf_assoc->bInterfaceCount.
>
> Ah, nice catch!
>
> Does the patch below fix this?
Hi Greg,
I believe it does and the bug is no longer triggered with the
reproducer that I have.
Tested-by: Andrey Konovalov
Thanks!
>
> thanks,
>
> greg k-h
>
> ---
>
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 4be52c602e9b..a3dbac1938ec 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -643,15 +643,23 @@ static int usb_parse_configuration(struct usb_device
> *dev, int cfgidx,
>
> } else if (header->bDescriptorType ==
> USB_DT_INTERFACE_ASSOCIATION) {
> + struct usb_interface_assoc_descriptor *d;
> +
> + d = (struct usb_interface_assoc_descriptor *)header;
> + if (d->bLength < USB_DT_INTERFACE_ASSOCIATION_SIZE) {
> + dev_warn(ddev,
> +"config %d has an invalid interface
> association descriptor of length %d, skipping\n",
> +cfgno, d->bLength);
> + continue;
> + }
> +
> if (iad_num == USB_MAXIADS) {
> dev_warn(ddev, "found more Interface "
>"Association Descriptors "
>"than allocated for in "
>"configuration %d\n", cfgno);
> } else {
> - config->intf_assoc[iad_num] =
> - (struct usb_interface_assoc_descriptor
> - *)header;
> + config->intf_assoc[iad_num] = d;
> iad_num++;
> }
>
> diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
> index ce1169af39d7..2a5d63040a0b 100644
> --- a/include/uapi/linux/usb/ch9.h
> +++ b/include/uapi/linux/usb/ch9.h
> @@ -780,6 +780,7 @@ struct usb_interface_assoc_descriptor {
> __u8 iFunction;
> } __attribute__ ((packed));
>
> +#define USB_DT_INTERFACE_ASSOCIATION_SIZE 8
>
> /*-*/
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
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: Fibocom L831-EAU and cdc_mbim
Bjørn Mork writes:
> Thanks a lot. These are very useful. The thing is that I really cannot
> see anything going wrong there. The "failed" capture just ends with a
> sequence of successful MBIM indications being read from the firmware.
>
> So whatever goes wrong looks like something entirely local to the
> driver. I'll try to poke around some more.
I am wondering if Robert added a hint for us here:
if (desc->rerr) {
/*
* Since there was an error, userspace may decide to not read
* any data after poll'ing.
* We should respond to further attempts from the device to send
* data, so that we can get unstuck.
*/
service_outstanding_interrupt(desc);
}
This might solve only half the problem. The driver does not stop
reading from the firmware as we can see in your pcaps. But if userspace
decided to not read any more data from the driver, then that doesn't
help much, does it?
What happens if you kill the mbim-proxy process? Are you then able to
use the modem again without resetting it?
Maybe we should simply ignore stalls completely? They should not happen
with any real WDM device anyway.
What happens if you e.g. change the if-test here:
/*
* only set a new error if there is no previous error.
* Errors are only cleared during read/open
*/
if (desc->rerr == 0)
desc->rerr = status;
to
if (desc->rerr == 0 && status != -EPIPE)
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling
Hi,
sorry about the long delay
On 07.09.2017 18:49, Hans de Goede wrote:
Hi,
On 07-09-17 15:14, Mathias Nyman wrote:
On 05.09.2017 19:42, Hans de Goede wrote:
The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.
I think it would be better to have one place where we add handlers for
vendor specific extended capabilities.
Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
there's a xhci-ext-caps.h header already
We could walk through the capability list once and add the needed handlers.
Something like:
+int xhci_ext_cap_init(void __iomem *base)
This will need to take a struct xhci_hcd *xhci param instead
as some of the ext_cap handling (including the cht mux code)
will need access to this.
yes, sample code added in second patch for reference/testing.
So I see 2 options here (without making this function PCI specific)
1) Add an u32 product_id field to struct xhci_hcd; or
2) Use a quirk flag as my current code is doing.
I'm fine with doing this either way, please let me know your preference.
Lets go with the quirk for now, I'll sort that out later
Can you do a "git format-patch" of that and send it to me? If you
can give me that + your preference for how to check if we're
dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
with your suggestions applied.
Ended up modifying xhci_find_next_ext_cap() using id = 0 for
the next capability in list. Patch attached,
Second patch is just for reference how to use it.
Thanks
-Mathias
>From d5f26c1595f211ea7d46fca91551f70d1207330d Mon Sep 17 00:00:00 2001
From: Mathias Nyman
Date: Tue, 19 Sep 2017 14:54:58 +0300
Subject: [PATCH 1/2] xhci: Add option to get next extended capability in list
by passing id = 0
Modify xhci_find_next_ext_cap(base, offset, id) to return the next
capability offset if 0 is passed for id. Otherwise it will behave as
previously and return the offset of the next capability with matching id
capability id 0 is not used by xhci (reserved)
This is useful when we want to loop through all capabilities.
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ext-caps.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index 28deea5..c1b4042 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -96,7 +96,8 @@
* @base PCI MMIO registers base address.
* @start address at which to start looking, (0 or HCC_PARAMS to start at
* beginning of list)
- * @id Extended capability ID to search for.
+ * @id Extended capability ID to search for, or 0 for the next
+ * capability
*
* Returns the offset of the next matching extended capability structure.
* Some capabilities can occur several times, e.g., the XHCI_EXT_CAPS_PROTOCOL,
@@ -122,7 +123,7 @@ static inline int xhci_find_next_ext_cap(void __iomem *base, u32 start, int id)
val = readl(base + offset);
if (val == ~0)
return 0;
- if (XHCI_EXT_CAPS_ID(val) == id && offset != start)
+ if (offset != start && (id == 0 || XHCI_EXT_CAPS_ID(val) == id))
return offset;
next = XHCI_EXT_CAPS_NEXT(val);
--
1.9.1
>From da44e961605d382829f90fdcfb90b61fa5ca9590 Mon Sep 17 00:00:00 2001
From: Mathias Nyman
Date: Tue, 19 Sep 2017 14:58:40 +0300
Subject: [PATCH 2/2] xhci: test xhci_find_next_ext_cap with 0 id
NOT for UPSTREAM, just testing/reference code
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ext-caps.h | 3 +++
drivers/usb/host/xhci.c | 30 ++
2 files changed, 33 insertions(+)
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index c1b4042..7deb9e7 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -51,6 +51,9 @@
#define XHCI_EXT_CAPS_ROUTE 5
/* IDs 6-9 reserved */
#define XHCI_EXT_CAPS_DEBUG 10
+/* IDs 192 - 255 vendor specific extensions */
+#define XHCI_EXT_CAPS_VENDOR_INTEL 192
+
/* USB Legacy Support Capability - section 7.1.1 */
#define XHCI_HC_BIOS_OWNED (1 << 16)
#define XHCI_HC_OS_OWNED (1 << 24)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 870093a..99c804a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4772,6 +4772,34 @@ static int xhci_get_frame(struct usb_hcd *hcd)
return readl(&xhci->run_regs->microframe_index) >> 3;
}
+int xhci_ext_cap_init(struct xhci_hcd *xhci)
+{
+ void __iomem *base;
+ u32 cap_offset;
+ u32 val;
+
+ base = &xhci->cap_regs->hc_capbase;
+ cap_offset = xh
Re: usb/hid: slab-out-of-bounds read in usbhid_parse
On Tue, Sep 19, 2017 at 1:47 PM, Kim Jaejoong wrote:
> Hi, Andrey Konovalov
>
> Thanks for the report.
>
> 2017-09-19 2:33 GMT+09:00 Andrey Konovalov :
>> Hi!
>>
>> I've got the following crash while fuzzing the kernel with syzkaller.
>>
>> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>>
>> It seems that there's no proper check on the hdesc->bNumDescriptors
>> value in usbhid_parse(). it iterates over hdesc->desc and accesses
>> hdesc->desc[n] fields, which might be out-of-bounds.
>
> The bNumDescriptors in hid descriptor means 'numeric expression
> specifying the number of class descriptors'.
> The value bNumberDescriptors identifies the number of additional class
> specific descriptors present.
> (refer to the 6.1.2 HID Descriptor in hid documents :
> http://www.usb.org/developers/hidpage/HID1_11.pdf)
>
> So, it can be out-of-bounds in hdesc->desc[n] if there is an
> additional class descriptor.
>
> Does the patch below fix this?
Hi Kim,
I'm not sure. Is there a check on the bLength field of a
hid_descriptor struct? Can it be less than sizeof(struct
hid_descriptor)? If so, we still do an out-of-bounds access to
hdesc->desc[0] (or some other fields).
Thanks!
>
> Thanks, jaejoong
> --
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 089bad8..7b6a0b6 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -974,7 +974,7 @@ static int usbhid_parse(struct hid_device *hid)
> u32 quirks = 0;
> unsigned int rsize = 0;
> char *rdesc;
> - int ret, n;
> + int ret;
>
> quirks = usbhid_lookup_quirk(le16_to_cpu(dev->descriptor.idVendor),
> le16_to_cpu(dev->descriptor.idProduct));
> @@ -1000,9 +1000,8 @@ static int usbhid_parse(struct hid_device *hid)
> hid->version = le16_to_cpu(hdesc->bcdHID);
> hid->country = hdesc->bCountryCode;
>
> - for (n = 0; n < hdesc->bNumDescriptors; n++)
> - if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
> - rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength);
> + if (hdesc->desc[0].bDescriptorType == HID_DT_REPORT)
> + rsize = le16_to_cpu(hdesc->desc[0].wDescriptorLength);
>
> if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) {
> dbg_hid("weird size of report descriptor (%u)\n", rsize);
>
>
>>
>> ==
>> BUG: KASAN: slab-out-of-bounds in usbhid_parse+0x9b1/0xa20
>> Read of size 1 at addr 88006c5f8edf by task kworker/1:2/1261
>>
>> CPU: 1 PID: 1261 Comm: kworker/1:2 Not tainted
>> 4.14.0-rc1-42251-gebb2c2437d80 #169
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> Workqueue: usb_hub_wq hub_event
>> Call Trace:
>> __dump_stack lib/dump_stack.c:16
>> dump_stack+0x292/0x395 lib/dump_stack.c:52
>> print_address_description+0x78/0x280 mm/kasan/report.c:252
>> kasan_report_error mm/kasan/report.c:351
>> kasan_report+0x22f/0x340 mm/kasan/report.c:409
>> __asan_report_load1_noabort+0x19/0x20 mm/kasan/report.c:427
>> usbhid_parse+0x9b1/0xa20 drivers/hid/usbhid/hid-core.c:1004
>> hid_add_device+0x16b/0xb30 drivers/hid/hid-core.c:2944
>> usbhid_probe+0xc28/0x1100 drivers/hid/usbhid/hid-core.c:1369
>> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
>> really_probe drivers/base/dd.c:413
>> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
>> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
>> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
>> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
>> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
>> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
>> device_add+0xd0b/0x1660 drivers/base/core.c:1835
>> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
>> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
>> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
>> really_probe drivers/base/dd.c:413
>> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
>> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
>> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
>> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
>> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
>> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
>> device_add+0xd0b/0x1660 drivers/base/core.c:1835
>> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
>> hub_port_connect drivers/usb/core/hub.c:4903
>> hub_port_connect_change drivers/usb/core/hub.c:5009
>> port_event drivers/usb/core/hub.c:5115
>> hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
>> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
>> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
>> kthread+0x3a1/0x470 kernel/kthread.c:231
>> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>>
>> Allocated by task 1261:
>> save_stack_trace+0x1b/0x2
Re: usb/core: slab-out-of-bounds in usb_set_configuration
On Tue, Sep 19, 2017 at 01:54:57PM +0200, Andrey Konovalov wrote: > On Mon, Sep 18, 2017 at 8:50 PM, Greg Kroah-Hartman > wrote: > > On Mon, Sep 18, 2017 at 07:22:24PM +0200, Andrey Konovalov wrote: > >> Hi! > >> > >> I've got the following crash while fuzzing the kernel with syzkaller. > >> > >> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). > >> > >> It seems there's no proper size check of a > >> USB_DT_INTERFACE_ASSOCIATION descriptor. It's only checked that the > >> size is >= 2 in usb_parse_configuration(), so find_iad() might do > >> out-of-bounds access to intf_assoc->bInterfaceCount. > > > > Ah, nice catch! > > > > Does the patch below fix this? > > Hi Greg, > > I believe it does and the bug is no longer triggered with the > reproducer that I have. > > Tested-by: Andrey Konovalov Thanks for testing, I'll go queue this up. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci bandwidth problem with isochronous endpoints
On 08.09.2017 20:35, Curt Meyers wrote: On 09/05/2017 02:56 PM, Curt Meyers wrote: On 09/04/2017 04:13 AM, Mathias Nyman wrote: On 04.09.2017 13:46, Felipe Balbi wrote: Hi, Mathias Nyman writes: Unfortunately config endpoint command doesn't log endpoint context in this log, it should call trace_xhci_handle_cmd_config_ep(ep_ctx), I don't know why it's missing. That's called conditionally: case TRB_CONFIG_EP: if (!cmd->completion) xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code); Yep, need to change the tracing so we get it for every config endpoint command But later on at Set TR Dequeue Pointer Command it logs the endpiont context: 259.147237: xhci_handle_cmd_set_deq: RS 0 super-speed Ctx Entries 7 MEL 512 us Port# 19/0 [TT Slot 0 Port# 0 TTT 0 Intr 0] Addr 1 State configured 259.147238: xhci_handle_cmd_set_deq_ep: State stopped mult 1 max P. Streams 0 interval 125 us max ESIT payload 201326592 CErr 0 Type Isoc IN burst 2 maxp 1024 deq 0003f9fd6510 avg trb len 3072 This looks odd, 201326592 bytes per ESIT, way too much. So much that I suspect tracing decodes it wrong try this: modified drivers/usb/host/xhci.h @@ -2540,9 +2540,7 @@ static inline const char *xhci_decode_ep_context(u32 info, u32 info2, u64 deq, u8 lsa; u8 hid; -esit = EP_MAX_ESIT_PAYLOAD_HI(info) << 16 | -EP_MAX_ESIT_PAYLOAD_LO(tx_info); - +esit = CTX_TO_MAX_ESIT_PAYLOAD(info); ep_state = info & EP_STATE_MASK; max_pstr = info & EP_MAXPSTREAMS_MASK; interval = CTX_TO_EP_INTERVAL(info); Yes, I noticed the same, and there's also a high ESIT bit field for new hosts, I pushed a fix to my for-usb-linus branch for that +#define CTX_TO_MAX_ESIT_PAYLOAD_HI(p) (((p) >> 24) & 0xff) ... - esit = EP_MAX_ESIT_PAYLOAD_HI(info) << 16 | - EP_MAX_ESIT_PAYLOAD_LO(tx_info); + esit = CTX_TO_MAX_ESIT_PAYLOAD_HI(info) << 16 | + CTX_TO_MAX_ESIT_PAYLOAD(tx_info); -Mathias Hi Mathias, Hi Sorry about the delay I have made the above suggested changed and produced a new trace file. I hope this helps out. Curt Would it help if I sent you a couple of these USB camera devices to speed up debugging? Not really, It's just my TODO list is already quite long, and then there's always some urgent matter that requires attention. The new log however still shows the wrong ESIT payload values, xhci_handle_cmd_set_deq_ep: State stopped mult 1 max P. Streams 0 interval 125 us max ESIT payload 201326592 -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/gadget: stalls in dummy_timer
On Fri, Sep 15, 2017 at 8:57 PM, Alan Stern wrote:
> On Thu, 14 Sep 2017, Andrey Konovalov wrote:
>
>> On Thu, Sep 14, 2017 at 7:49 PM, Alan Stern
>> wrote:
>> > On Thu, 14 Sep 2017, Andrey Konovalov wrote:
>> >
>> >> Looked at this a little more.
>> >>
>> >> dummy_timer() stucks in an infinite loop. It calls
>> >> usb_hcd_giveback_urb(), which in turn calls usbtouch_irq(), which
>> >> calls usb_submit_urb(), which calls dummy_urb_enqueue() and puts urb
>> >> back into dummy urb queue. dummy_timer() then does goto restart, finds
>> >> the urb and calls usb_hcd_giveback_urb() again. And this process goes
>> >> on again and again. It seems that something should either process the
>> >> urb and set urb->status or it should just expire.
>> >
>> > There is some throttling code, but it applies only to bulk transfers.
>> > Probably because the bandwidth limits for other types are slightly
>> > different. However, I don't think we need to worry about this level of
>> > detail, since the driver makes a number of other approximations anyway.
>> >
>> > Try the patch below; it should fix the problem.
>>
>> Hi Alan,
>>
>> Just tried your patch, my reproducer still hangs the kernel until all
>> memory is exhausted.
>>
>> Thanks!
>
> Hmmm. Your reproducer doesn't run on my system. The mmap system call
> fails, perhaps because this laptop has a 32-bit kernel. So I can't
> tell what's going on.
>
> Can you collect a usbmon trace that shows what happens while the
> reproducer runs? If it turns out to be extremely large, just post an
> initial portion of it.
I've attached the usbmon trace. It's actually quite short, probably
due to the fact that the kernel enters infinite loop.
I've also attached a reproducer that should compile on a 32 bit
system, however I haven't tested whether it reproduces the issue.
>
> Alan Stern
>
dummy_timer-stall-usbmon
Description: Binary data
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
void gadgetfs_mkdir()
{
mkdir("/dev/gadget", 0755) != 0;
}
void gadgetfs_mount()
{
while (mount("none", "/dev/gadget", "gadgetfs", 0, NULL) != 0) {
usleep(10 * 1000);
umount2("/dev/gadget", MNT_FORCE | MNT_DETACH);
}
}
static int
gfs_handle_event_setup_get_descriptor(int fd,
struct usb_ctrlrequest* setup)
{
char buffer[128];
int rv;
switch (setup->wValue >> 8) {
case USB_DT_STRING:
buffer[0] = 4;
buffer[1] = USB_DT_STRING;
if ((setup->wValue & 0x0ff) == 0) {
buffer[2] = 0x09;
buffer[3] = 0x04;
} else {
buffer[2] = 0x61;
buffer[3] = 0x00;
}
rv = write(fd, &buffer[0], 4);
if (rv != 4) {
return -1;
}
break;
default:
break;
}
return 0;
}
static int
gfs_handle_event_setup_set_configuration(int fd,
struct usb_ctrlrequest* setup)
{
int rv = read(fd, &rv, 0);
if (rv != 0) {
return -1;
}
return 0;
}
static int gfs_handle_event_setup(int fd, struct usb_ctrlrequest* setup)
{
switch (setup->bRequest) {
case USB_REQ_GET_DESCRIPTOR:
gfs_handle_event_setup_get_descriptor(fd, setup);
break;
case USB_REQ_SET_CONFIGURATION:
gfs_handle_event_setup_set_configuration(fd, setup);
break;
case USB_REQ_GET_INTERFACE:
break;
case USB_REQ_SET_INTERFACE:
break;
default:
break;
}
return 0;
}
static int gfs_handle_event(int fd, struct usb_gadgetfs_event* event)
{
switch (event->type) {
case GADGETFS_NOP:
break;
case GADGETFS_CONNECT:
break;
case GADGETFS_SETUP:
gfs_handle_event_setup(fd, &event->u.setup);
break;
case GADGETFS_DISCONNECT:
break;
case GADGETFS_SUSPEND:
break;
default:
break;
}
return 0;
}
#define GFS_MAX_EVENTS 1
static int gfs_handle_ep0(int fd)
{
struct pollfd pfd;
struct usb_gadgetfs_event events[GFS_MAX_EVENTS];
pfd.fd = fd;
pfd.events = POLLIN | POLLOUT | POLLHUP;
int i;
for (i = 0; i < 15; i++) {
int rv = poll(&pfd, 1, 100);
if (rv < 0) {
return -1;
}
Re: Fibocom L831-EAU and cdc_mbim
On 19/09/17 14:25, Bjørn Mork wrote: > Bjørn Mork writes: > > What happens if you kill the mbim-proxy process? Are you then able to > use the modem again without resetting it? I was unable to test that: It can only be killed using SIGKILL, any new mbim-proxy processes are then unable to open the device. > > Maybe we should simply ignore stalls completely? They should not happen > with any real WDM device anyway. > > What happens if you e.g. change the if-test here: > > /* >* only set a new error if there is no previous error. >* Errors are only cleared during read/open >*/ > if (desc->rerr == 0) > desc->rerr = status; > > > to > > if (desc->rerr == 0 && status != -EPIPE) Well, this looks very promising: Although the error messages persist every now and then, the device now works! I'll make the new module the default and I'll test it more thouroughly during the next few days. Are you planning on adding a more "proper" fix, i.e. without ignoring the error? Or is this something that should be "fixed" in libmbim? Thanks for working on this issue, Andreas -- 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 8/9] atmel_flexcom: Support backup mode
On Tue, 19 Sep 2017, Nicolas Ferre wrote: > On 15/09/2017 at 16:04, Romain Izard wrote: > > The controller used by a flexcom module is configured at boot, and left > > alone after this. As the configuration will be lost after backup mode, > > restore the state of the flexcom driver on resume. > > > > Signed-off-by: Romain Izard > > Tested-by: Nicolas Ferre > On sama5d2 Xplained board (i2c0 from flexcom 4). > and obviously: > Acked-by: Nicolas Ferre > > Thanks Romain! > > Regards, > > > --- > > drivers/mfd/atmel-flexcom.c | 65 > > ++--- > > 1 file changed, 50 insertions(+), 15 deletions(-) This is the first time I've seen this patch. Why's that? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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: Fibocom L831-EAU and cdc_mbim
Andreas Böhler writes:
> On 19/09/17 14:25, Bjørn Mork wrote:
>> Bjørn Mork writes:
>>
>> What happens if you kill the mbim-proxy process? Are you then able to
>> use the modem again without resetting it?
>
> I was unable to test that: It can only be killed using SIGKILL, any new
> mbim-proxy processes are then unable to open the device.
>
>>
>> Maybe we should simply ignore stalls completely? They should not happen
>> with any real WDM device anyway.
>>
>> What happens if you e.g. change the if-test here:
>>
>> /*
>> * only set a new error if there is no previous error.
>> * Errors are only cleared during read/open
>> */
>> if (desc->rerr == 0)
>> desc->rerr = status;
>>
>>
>> to
>>
>> if (desc->rerr == 0 && status != -EPIPE)
>
> Well, this looks very promising: Although the error messages persist
> every now and then, the device now works!
>
> I'll make the new module the default and I'll test it more thouroughly
> during the next few days.
>
> Are you planning on adding a more "proper" fix, i.e. without ignoring
> the error? Or is this something that should be "fixed" in libmbim?
If I only knew what the proper fix was... Ignoring the "error" as such
should not be a big problem. It's really just a signal from the
firmware that there is no data available to read. There are many
similar examples in the Linux USB code. For example from the core code
in drivers/usb/core/message.c:
static int usb_get_string(struct usb_device *dev, unsigned short langid,
unsigned char index, void *buf, int size)
{
int i;
int result;
for (i = 0; i < 3; ++i) {
/* retry on length 0 or stall; some devices are flakey */
result = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
(USB_DT_STRING << 8) + index, langid, buf, size,
USB_CTRL_GET_TIMEOUT);
if (result == 0 || result == -EPIPE)
continue;
if (result > 1 && ((u8 *) buf)[1] != USB_DT_STRING) {
result = -ENODATA;
continue;
}
break;
}
return result;
}
We cannot do the same looping since we are using async requests of
course. We could add a counter to catch repeated errors, but I feel that
is overkill. Ignoring will do.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
Every once in a while when my system is under a bit of stress I see
some spammy messages show up in my logs that say:
kevent X may have been dropped
As far as I can tell these messages aren't terribly useful. The
comments around the messages make me think that either workqueues used
to work differently or that the original author of the code missed a
sublety related to them. The error message appears to predate the git
conversion of the kernel so it's somewhat hard to tell.
Specifically, workqueues should work like this:
A) If a workqueue hasn't been scheduled then schedule_work() schedules
it and returns true.
B) If a workqueue has been scheduled (but hasn't started) then
schedule_work() will do nothing and return false.
C) If a workqueue has been scheduled (and has started) then
schedule_work() will put it on the queue to run again and return
true.
Said another way: if you call schedule_work() you can guarantee that
at least one full runthrough of the work will happen again. That
should mean that the work will get processed and I don't see any
reason to think something should be dropped.
Reading the comments in in usbnet_defer_kevent() made me think that B)
and C) would be treated the same. That is: even if we've started the
work and are 99% of the way through then schedule_work() would return
false and the work wouldn't be queued again. If schedule_work()
really did behave that way then, truly, some amount of work would be
lost. ...but it doesn't.
NOTE: if somehow these warnings are useful to mean something then
perhaps we should change them to make it more obvious. If it's
interesting to know when the work is backlogged then we should change
the spam to say "warning: usbnet is backlogged".
ALSO NOTE: If somehow some of the types of work need to be repeated if
usbnet_defer_kevent() is called multiple times then that should be
quite easy to accomplish without dropping any work on the floor. We
can just keep an atomic count for that type of work and add a loop
into usbnet_deferred_kevent().
Signed-off-by: Douglas Anderson
---
drivers/net/usb/usbnet.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 6510e5cc1817..a3e8dbaadcf9 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -450,19 +450,17 @@ static enum skb_state defer_bh(struct usbnet *dev, struct
sk_buff *skb,
}
/* some work can't be done in tasklets, so we use keventd
- *
- * NOTE: annoying asymmetry: if it's active, schedule_work() fails,
- * but tasklet_schedule() doesn't. hope the failure is rare.
*/
void usbnet_defer_kevent (struct usbnet *dev, int work)
{
set_bit (work, &dev->flags);
- if (!schedule_work (&dev->kevent)) {
- if (net_ratelimit())
- netdev_err(dev->net, "kevent %d may have been
dropped\n", work);
- } else {
- netdev_dbg(dev->net, "kevent %d scheduled\n", work);
- }
+
+ /* If work is already started this will mark it to run again when it
+* finishes; if we already had work pending and it hadn't started
+* yet then that's fine too.
+*/
+ schedule_work (&dev->kevent);
+ netdev_dbg(dev->net, "kevent %d scheduled\n", work);
}
EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
--
2.14.1.690.gbb1197296e-goog
--
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 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
In general when you've got a flag communicating that "something needs
to be done" you want to clear that flag _before_ doing the task. If
you clear the flag _after_ doing the task you end up with the risk
that this will happen:
1. Requester sets flag saying task A needs to be done.
2. Worker comes and stars doing task A.
3. Worker finishes task A but hasn't yet cleared the flag.
4. Requester wants to set flag saying task A needs to be done again.
5. Worker clears the flag without doing anything.
Let's make the usbnet codebase consistently clear the flag _before_ it
does the requested work. That way if there's another request to do
the work while the work is already in progress it won't be lost.
NOTES:
- No known bugs are fixed by this; it's just found by code inspection.
- This changes the semantics in some of the error conditions.
-> If we fail to clear the "tx halt" or "rx halt" we still clear the
flag and thus won't retry the clear next time we happen to be in
the work function. Had the old code really wanted to retry these
events it should have re-scheduled the worker anyway.
-> If we fail to allocate memory in usb_alloc_urb() we will still
clear the EVENT_RX_MEMORY flag. This makes it consistent with
how we would deal with other failures, including failure to
allocate a memory chunk in rx_submit(). It can also be noted
that usb_alloc_urb() in this case is allocating much less than 4K
worth of data and probably never fails.
Signed-off-by: Douglas Anderson
---
drivers/net/usb/usbnet.c | 50 +---
1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index a3e8dbaadcf9..e72547d8d0e6 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1103,8 +1103,6 @@ static void __handle_link_change(struct usbnet *dev)
/* hard_mtu or rx_urb_size may change during link change */
usbnet_update_max_qlen(dev);
-
- clear_bit(EVENT_LINK_CHANGE, &dev->flags);
}
static void usbnet_set_rx_mode(struct net_device *net)
@@ -1118,8 +1116,6 @@ static void __handle_set_rx_mode(struct usbnet *dev)
{
if (dev->driver_info->set_rx_mode)
(dev->driver_info->set_rx_mode)(dev);
-
- clear_bit(EVENT_SET_RX_MODE, &dev->flags);
}
/* work that cannot be done in interrupt context uses keventd.
@@ -1135,7 +1131,7 @@ usbnet_deferred_kevent (struct work_struct *work)
int status;
/* usb_clear_halt() needs a thread context */
- if (test_bit (EVENT_TX_HALT, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_TX_HALT, &dev->flags)) {
unlink_urbs (dev, &dev->txq);
status = usb_autopm_get_interface(dev->intf);
if (status < 0)
@@ -1150,12 +1146,11 @@ usbnet_deferred_kevent (struct work_struct *work)
netdev_err(dev->net, "can't clear tx halt,
status %d\n",
status);
} else {
- clear_bit (EVENT_TX_HALT, &dev->flags);
if (status != -ESHUTDOWN)
netif_wake_queue (dev->net);
}
}
- if (test_bit (EVENT_RX_HALT, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_RX_HALT, &dev->flags)) {
unlink_urbs (dev, &dev->rxq);
status = usb_autopm_get_interface(dev->intf);
if (status < 0)
@@ -1170,41 +1165,39 @@ usbnet_deferred_kevent (struct work_struct *work)
netdev_err(dev->net, "can't clear rx halt,
status %d\n",
status);
} else {
- clear_bit (EVENT_RX_HALT, &dev->flags);
tasklet_schedule (&dev->bh);
}
}
/* tasklet could resubmit itself forever if memory is tight */
- if (test_bit (EVENT_RX_MEMORY, &dev->flags)) {
+ if (test_and_clear_bit (EVENT_RX_MEMORY, &dev->flags)) {
struct urb *urb = NULL;
int resched = 1;
- if (netif_running (dev->net))
+ if (netif_running (dev->net)) {
urb = usb_alloc_urb (0, GFP_KERNEL);
- else
- clear_bit (EVENT_RX_MEMORY, &dev->flags);
- if (urb != NULL) {
- clear_bit (EVENT_RX_MEMORY, &dev->flags);
- status = usb_autopm_get_interface(dev->intf);
- if (status < 0) {
- usb_free_urb(urb);
- goto fail_lowmem;
- }
- if (rx_submit (dev, urb, GFP_KERNEL) == -ENOLINK)
- resched = 0;
- usb_autopm_put_interface(dev->intf);
+
[RFC PATCH 3/3] usbnet: Fix memory leak when rx_submit() fails
If rx_submit() returns an error code then nobody calls usb_free_urb().
That means it's leaked.
NOTE: This problem was found solely by code inspection and not due to
any failing test cases.
Signed-off-by: Douglas Anderson
---
drivers/net/usb/usbnet.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e72547d8d0e6..4c067aaeea5a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1182,9 +1182,12 @@ usbnet_deferred_kevent (struct work_struct *work)
usb_free_urb(urb);
goto fail_lowmem;
}
- if (rx_submit (dev, urb, GFP_KERNEL) ==
- -ENOLINK)
- resched = 0;
+ status = rx_submit (dev, urb, GFP_KERNEL);
+ if (status) {
+ usb_free_urb(urb);
+ if (status == -ENOLINK)
+ resched = 0;
+ }
usb_autopm_put_interface(dev->intf);
fail_lowmem:
if (resched)
--
2.14.1.690.gbb1197296e-goog
--
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 05/11] mux: Add Intel Cherrytrail USB mux driver
Hi, Thank you for the reviews and patches! On 09/08/2017 05:45 PM, Peter Rosin wrote: On 2017-09-05 18:42, Hans de Goede wrote: Intel Cherrytrail SoCs have an internal USB mux for muxing the otg-port USB data lines between the xHCI host controller and the dwc3 gadget controller. On some Cherrytrail systems this mux is controlled through AML code reacting on a GPIO IRQ connected to the USB OTG id pin (through an _AIE ACPI method) so things just work. But on other Cherrytrail systems we need to control the mux ourselves this driver exports the mux through the mux subsys, so that other drivers can control it if necessary. This driver also updates the vbus-valid reporting to the dwc3 gadget controller, as this uses the same registers as the mux. This is needed for gadget/device mode to work properly (even on systems which control the mux from their AML code). Note this depends on the xhci driver registering a platform device named "intel_cht_usb_mux", which has an IOMEM resource 0 which points to the Intel Vendor Defined XHCI extended capabilities region. Signed-off-by: Hans de Goede --- Changes in v2: -Complete rewrite as a stand-alone platform-driver rather then as a phy driver, since this is just a mux, not a phy Changes in v3: -Make this a mux subsys driver instead of listening to USB_HOST extcon cable events and responding to those Changes in v4 (of patch, v2 of new mux based series): -Rename C-file to use - in name -Add MAINTAINERS entry -Various code-style fixes --- MAINTAINERS | 5 + drivers/mux/Kconfig | 11 ++ drivers/mux/Makefile| 10 +- drivers/mux/intel-cht-usb-mux.c | 257 4 files changed, 279 insertions(+), 4 deletions(-) create mode 100644 drivers/mux/intel-cht-usb-mux.c diff --git a/MAINTAINERS b/MAINTAINERS index 14a2fd905217..dfaed958db85 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8991,6 +8991,11 @@ F: include/linux/dt-bindings/mux/ F:include/linux/mux/ F:drivers/mux/ +MULTIPLEXER SUBSYSTEM INTEL CHT USB MUX DRIVER +M: Hans de Goede +S: Maintained +F: drivers/mix/intel-cht-usb-mux.c s/mix/mux/ (also in patch 06/11) + MULTISOUND SOUND DRIVER M:Andrew Veliath S:Maintained diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index 19e4e904c9bf..947cfd7af02a 100644 --- a/drivers/mux/Kconfig +++ b/drivers/mux/Kconfig @@ -34,6 +34,17 @@ config MUX_GPIO To compile the driver as a module, choose M here: the module will be called mux-gpio. +config MUX_INTEL_CHT_USB_MUX + tristate "Intel Cherrytrail USB Multiplexer" + depends on ACPI && X86 && EXTCON + help + This driver adds support for the internal USB mux for muxing the OTG + USB data lines between the xHCI host controller and the dwc3 gadget + controller found on Intel Cherrytrail SoCs. + + To compile the driver as a module, choose M here: the module will + be called mux-intel_cht_usb_mux. s/_/-/g + config MUX_MMIO tristate "MMIO register bitfield-controlled Multiplexer" depends on (OF && MFD_SYSCON) || COMPILE_TEST diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile index 0e1e59760e3f..6cf41be2754f 100644 --- a/drivers/mux/Makefile +++ b/drivers/mux/Makefile @@ -5,9 +5,11 @@ mux-core-objs := core.o mux-adg792a-objs := adg792a.o mux-gpio-objs := gpio.o +mux-intel-cht-usb-mux-objs := intel-cht-usb-mux.o mux-mmio-objs := mmio.o -obj-$(CONFIG_MULTIPLEXER) += mux-core.o -obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o -obj-$(CONFIG_MUX_GPIO) += mux-gpio.o -obj-$(CONFIG_MUX_MMIO) += mux-mmio.o +obj-$(CONFIG_MULTIPLEXER) += mux-core.o +obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o +obj-$(CONFIG_MUX_GPIO) += mux-gpio.o +obj-$(CONFIG_MUX_INTEL_CHT_USB_MUX)+= mux-intel-cht-usb-mux.o +obj-$(CONFIG_MUX_MMIO) += mux-mmio.o diff --git a/drivers/mux/intel-cht-usb-mux.c b/drivers/mux/intel-cht-usb-mux.c new file mode 100644 index ..9cd1a1f5027f --- /dev/null +++ b/drivers/mux/intel-cht-usb-mux.c @@ -0,0 +1,257 @@ +/* + * Intel Cherrytrail USB OTG MUX driver + * + * Copyright (c) 2016 Hans de Goede 2017? Actually I stated working on this (in a different form before the mux framework was merge) quite a while back I will make this 2016-2017. + * + * Loosely based on android x86 kernel code which is: + * + * Copyright (C) 2014 Intel Corp. + * + * Author: Wu, Hao + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation, or (at your option) + * any later version. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#inclu
Re: [RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
On Tue, Sep 19, 2017 at 9:15 AM, Douglas Anderson wrote:
> Every once in a while when my system is under a bit of stress I see
> some spammy messages show up in my logs that say:
>
> kevent X may have been dropped
>
> As far as I can tell these messages aren't terribly useful. The
> comments around the messages make me think that either workqueues used
> to work differently or that the original author of the code missed a
> sublety related to them. The error message appears to predate the git
> conversion of the kernel so it's somewhat hard to tell.
>
> Specifically, workqueues should work like this:
>
> A) If a workqueue hasn't been scheduled then schedule_work() schedules
>it and returns true.
>
> B) If a workqueue has been scheduled (but hasn't started) then
>schedule_work() will do nothing and return false.
>
> C) If a workqueue has been scheduled (and has started) then
>schedule_work() will put it on the queue to run again and return
>true.
>
> Said another way: if you call schedule_work() you can guarantee that
> at least one full runthrough of the work will happen again. That
> should mean that the work will get processed and I don't see any
> reason to think something should be dropped.
>
> Reading the comments in in usbnet_defer_kevent() made me think that B)
> and C) would be treated the same. That is: even if we've started the
> work and are 99% of the way through then schedule_work() would return
> false and the work wouldn't be queued again. If schedule_work()
> really did behave that way then, truly, some amount of work would be
> lost. ...but it doesn't.
>
> NOTE: if somehow these warnings are useful to mean something then
> perhaps we should change them to make it more obvious. If it's
> interesting to know when the work is backlogged then we should change
> the spam to say "warning: usbnet is backlogged".
>
> ALSO NOTE: If somehow some of the types of work need to be repeated if
> usbnet_defer_kevent() is called multiple times then that should be
> quite easy to accomplish without dropping any work on the floor. We
> can just keep an atomic count for that type of work and add a loop
> into usbnet_deferred_kevent().
>
> Signed-off-by: Douglas Anderson
Reviewed-by: Guenter Roeck
> ---
>
> drivers/net/usb/usbnet.c | 16 +++-
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 6510e5cc1817..a3e8dbaadcf9 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -450,19 +450,17 @@ static enum skb_state defer_bh(struct usbnet *dev,
> struct sk_buff *skb,
> }
>
> /* some work can't be done in tasklets, so we use keventd
> - *
> - * NOTE: annoying asymmetry: if it's active, schedule_work() fails,
> - * but tasklet_schedule() doesn't. hope the failure is rare.
> */
> void usbnet_defer_kevent (struct usbnet *dev, int work)
> {
> set_bit (work, &dev->flags);
> - if (!schedule_work (&dev->kevent)) {
> - if (net_ratelimit())
> - netdev_err(dev->net, "kevent %d may have been
> dropped\n", work);
> - } else {
> - netdev_dbg(dev->net, "kevent %d scheduled\n", work);
> - }
> +
> + /* If work is already started this will mark it to run again when it
> +* finishes; if we already had work pending and it hadn't started
> +* yet then that's fine too.
> +*/
> + schedule_work (&dev->kevent);
> + netdev_dbg(dev->net, "kevent %d scheduled\n", work);
> }
> EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
>
> --
> 2.14.1.690.gbb1197296e-goog
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/gadget: stalls in dummy_timer
On Tue, 19 Sep 2017, Andrey Konovalov wrote:
> On Fri, Sep 15, 2017 at 8:57 PM, Alan Stern wrote:
> > On Thu, 14 Sep 2017, Andrey Konovalov wrote:
> >
> >> On Thu, Sep 14, 2017 at 7:49 PM, Alan Stern
> >> wrote:
> >> > On Thu, 14 Sep 2017, Andrey Konovalov wrote:
> >> >
> >> >> Looked at this a little more.
> >> >>
> >> >> dummy_timer() stucks in an infinite loop. It calls
> >> >> usb_hcd_giveback_urb(), which in turn calls usbtouch_irq(), which
> >> >> calls usb_submit_urb(), which calls dummy_urb_enqueue() and puts urb
> >> >> back into dummy urb queue. dummy_timer() then does goto restart, finds
> >> >> the urb and calls usb_hcd_giveback_urb() again. And this process goes
> >> >> on again and again. It seems that something should either process the
> >> >> urb and set urb->status or it should just expire.
> >> >
> >> > There is some throttling code, but it applies only to bulk transfers.
> >> > Probably because the bandwidth limits for other types are slightly
> >> > different. However, I don't think we need to worry about this level of
> >> > detail, since the driver makes a number of other approximations anyway.
> >> >
> >> > Try the patch below; it should fix the problem.
> >>
> >> Hi Alan,
> >>
> >> Just tried your patch, my reproducer still hangs the kernel until all
> >> memory is exhausted.
> >>
> >> Thanks!
> >
> > Hmmm. Your reproducer doesn't run on my system. The mmap system call
> > fails, perhaps because this laptop has a 32-bit kernel. So I can't
> > tell what's going on.
> >
> > Can you collect a usbmon trace that shows what happens while the
> > reproducer runs? If it turns out to be extremely large, just post an
> > initial portion of it.
>
> I've attached the usbmon trace. It's actually quite short, probably
> due to the fact that the kernel enters infinite loop.
>
> I've also attached a reproducer that should compile on a 32 bit
> system, however I haven't tested whether it reproduces the issue.
Got it, thanks. Can you test the patch below in place of (or in
addition to) the earlier patch?
Alan Stern
Index: usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c
===
--- usb-4.x.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c
@@ -237,6 +237,8 @@ struct dummy_hcd {
struct usb_device *udev;
struct list_headurbp_list;
+ struct urbp *next_frame_urbp;
+
u32 stream_en_ep;
u8 num_stream[30 / 2];
@@ -1252,6 +1254,8 @@ static int dummy_urb_enqueue(
list_add_tail(&urbp->urbp_list, &dum_hcd->urbp_list);
urb->hcpriv = urbp;
+ if (!dum_hcd->next_frame_urbp)
+ dum_hcd->next_frame_urbp = urbp;
if (usb_pipetype(urb->pipe) == PIPE_CONTROL)
urb->error_count = 1; /* mark as a new urb */
@@ -1768,6 +1772,7 @@ static void dummy_timer(unsigned long _d
spin_unlock_irqrestore(&dum->lock, flags);
return;
}
+ dum_hcd->next_frame_urbp = NULL;
for (i = 0; i < DUMMY_ENDPOINTS; i++) {
if (!ep_info[i].name)
@@ -1784,6 +1789,10 @@ restart:
int type;
int status = -EINPROGRESS;
+ /* stop when we reach URBs queued after the timer interrupt */
+ if (urbp == dum_hcd->next_frame_urbp)
+ break;
+
urb = urbp->urb;
if (urb->unlinked)
goto return_urb;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/gadget: stalls in dummy_timer
On Tue, Sep 19, 2017 at 7:17 PM, Alan Stern wrote:
> On Tue, 19 Sep 2017, Andrey Konovalov wrote:
>
>> On Fri, Sep 15, 2017 at 8:57 PM, Alan Stern
>> wrote:
>> > On Thu, 14 Sep 2017, Andrey Konovalov wrote:
>> >
>> >> On Thu, Sep 14, 2017 at 7:49 PM, Alan Stern
>> >> wrote:
>> >> > On Thu, 14 Sep 2017, Andrey Konovalov wrote:
>> >> >
>> >> >> Looked at this a little more.
>> >> >>
>> >> >> dummy_timer() stucks in an infinite loop. It calls
>> >> >> usb_hcd_giveback_urb(), which in turn calls usbtouch_irq(), which
>> >> >> calls usb_submit_urb(), which calls dummy_urb_enqueue() and puts urb
>> >> >> back into dummy urb queue. dummy_timer() then does goto restart, finds
>> >> >> the urb and calls usb_hcd_giveback_urb() again. And this process goes
>> >> >> on again and again. It seems that something should either process the
>> >> >> urb and set urb->status or it should just expire.
>> >> >
>> >> > There is some throttling code, but it applies only to bulk transfers.
>> >> > Probably because the bandwidth limits for other types are slightly
>> >> > different. However, I don't think we need to worry about this level of
>> >> > detail, since the driver makes a number of other approximations anyway.
>> >> >
>> >> > Try the patch below; it should fix the problem.
>> >>
>> >> Hi Alan,
>> >>
>> >> Just tried your patch, my reproducer still hangs the kernel until all
>> >> memory is exhausted.
>> >>
>> >> Thanks!
>> >
>> > Hmmm. Your reproducer doesn't run on my system. The mmap system call
>> > fails, perhaps because this laptop has a 32-bit kernel. So I can't
>> > tell what's going on.
>> >
>> > Can you collect a usbmon trace that shows what happens while the
>> > reproducer runs? If it turns out to be extremely large, just post an
>> > initial portion of it.
>>
>> I've attached the usbmon trace. It's actually quite short, probably
>> due to the fact that the kernel enters infinite loop.
>>
>> I've also attached a reproducer that should compile on a 32 bit
>> system, however I haven't tested whether it reproduces the issue.
>
> Got it, thanks. Can you test the patch below in place of (or in
> addition to) the earlier patch?
With this patch (just this one, without the other one) the reproducer
no longer hangs the kernel :)
Tested-by: Andrey Konovalov
Thanks!
>
> Alan Stern
>
>
>
> Index: usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c
> ===
> --- usb-4.x.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -237,6 +237,8 @@ struct dummy_hcd {
>
> struct usb_device *udev;
> struct list_headurbp_list;
> + struct urbp *next_frame_urbp;
> +
> u32 stream_en_ep;
> u8 num_stream[30 / 2];
>
> @@ -1252,6 +1254,8 @@ static int dummy_urb_enqueue(
>
> list_add_tail(&urbp->urbp_list, &dum_hcd->urbp_list);
> urb->hcpriv = urbp;
> + if (!dum_hcd->next_frame_urbp)
> + dum_hcd->next_frame_urbp = urbp;
> if (usb_pipetype(urb->pipe) == PIPE_CONTROL)
> urb->error_count = 1; /* mark as a new urb */
>
> @@ -1768,6 +1772,7 @@ static void dummy_timer(unsigned long _d
> spin_unlock_irqrestore(&dum->lock, flags);
> return;
> }
> + dum_hcd->next_frame_urbp = NULL;
>
> for (i = 0; i < DUMMY_ENDPOINTS; i++) {
> if (!ep_info[i].name)
> @@ -1784,6 +1789,10 @@ restart:
> int type;
> int status = -EINPROGRESS;
>
> + /* stop when we reach URBs queued after the timer interrupt */
> + if (urbp == dum_hcd->next_frame_urbp)
> + break;
> +
> urb = urbp->urb;
> if (urb->unlinked)
> goto return_urb;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: exynos: Add dwc3 SUSPHY quirk
On Mon, Sep 18, 2017 at 12:02:13PM +0200, Andrzej Pietrasiewicz wrote: > Odroid XU4 board does not enumerate SuperSpeed devices. > This patch makes exynos5 series chips use USB SUSPHY quirk, > which solves the problem. > > Signed-off-by: Andrzej Pietrasiewicz > --- > arch/arm/boot/dts/exynos54xx.dtsi | 2 ++ > 1 file changed, 2 insertions(+) Makes sense to me... was it tested also on XU3 and XU? Best regards, Krzysztof > > diff --git a/arch/arm/boot/dts/exynos54xx.dtsi > b/arch/arm/boot/dts/exynos54xx.dtsi > index 0389e8a..8ca4fef 100644 > --- a/arch/arm/boot/dts/exynos54xx.dtsi > +++ b/arch/arm/boot/dts/exynos54xx.dtsi > @@ -134,6 +134,7 @@ > interrupts = ; > phys = <&usbdrd_phy0 0>, <&usbdrd_phy0 1>; > phy-names = "usb2-phy", "usb3-phy"; > + snps,dis_u3_susphy_quirk; > }; > }; > > @@ -154,6 +155,7 @@ > reg = <0x1240 0x1>; > phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>; > phy-names = "usb2-phy", "usb3-phy"; > + snps,dis_u3_susphy_quirk; > }; > }; > > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3/3] usbnet: Fix memory leak when rx_submit() fails
Douglas Anderson writes:
> If rx_submit() returns an error code then nobody calls usb_free_urb().
> That means it's leaked.
Nope. rx_submit() will call usb_free_urb() before returning an error:
static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags)
..
if (!skb) {
netif_dbg(dev, rx_err, dev->net, "no rx skb\n");
usbnet_defer_kevent (dev, EVENT_RX_MEMORY);
usb_free_urb (urb);
return -ENOMEM;
}
..
if (retval) {
dev_kfree_skb_any (skb);
usb_free_urb (urb);
}
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
Douglas Anderson writes: > Every once in a while when my system is under a bit of stress I see > some spammy messages show up in my logs that say: > > kevent X may have been dropped > > As far as I can tell these messages aren't terribly useful. I agree, FWIW. These messages just confuse users for no purpose at all. > + /* If work is already started this will mark it to run again when it > + * finishes; if we already had work pending and it hadn't started > + * yet then that's fine too. > + */ > + schedule_work (&dev->kevent); > + netdev_dbg(dev->net, "kevent %d scheduled\n", work); Or maybe if (schedule_work (&dev->kevent)) netdev_dbg(dev->net, "kevent %d scheduled\n", work); ? Not that I think it matters much. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: exynos: Add dwc3 SUSPHY quirk
On 19/09/17 18:40, Krzysztof Kozlowski wrote: > On Mon, Sep 18, 2017 at 12:02:13PM +0200, Andrzej Pietrasiewicz wrote: >> Odroid XU4 board does not enumerate SuperSpeed devices. >> This patch makes exynos5 series chips use USB SUSPHY quirk, >> which solves the problem. >> >> Signed-off-by: Andrzej Pietrasiewicz >> --- >> arch/arm/boot/dts/exynos54xx.dtsi | 2 ++ >> 1 file changed, 2 insertions(+) > > Makes sense to me... was it tested also on XU3 and XU? Well, it at least doesn't make USB3 on my XU any more or less broken than it was before ;) (both ports still report an over-current condition even with nothing plugged in - I suspect there's probably still some pinctrl/regulator stuff missing) Robin. > > Best regards, > Krzysztof > >> >> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi >> b/arch/arm/boot/dts/exynos54xx.dtsi >> index 0389e8a..8ca4fef 100644 >> --- a/arch/arm/boot/dts/exynos54xx.dtsi >> +++ b/arch/arm/boot/dts/exynos54xx.dtsi >> @@ -134,6 +134,7 @@ >> interrupts = ; >> phys = <&usbdrd_phy0 0>, <&usbdrd_phy0 1>; >> phy-names = "usb2-phy", "usb3-phy"; >> +snps,dis_u3_susphy_quirk; >> }; >> }; >> >> @@ -154,6 +155,7 @@ >> reg = <0x1240 0x1>; >> phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>; >> phy-names = "usb2-phy", "usb3-phy"; >> +snps,dis_u3_susphy_quirk; >> }; >> }; >> >> -- >> 1.9.1 >> > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- 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: [regression] Force hard reset of Renesas uPD72020x USB controller
On Mon, Sep 18 2017 at 3:01:32 pm BST, Albert Weichselbraun wrote: > On Mon, 2017-09-18 at 12:46 +0100, Marc Zyngier wrote: >> On 18/09/17 09:49, Albert Weichselbraun wrote: >> > Hi Marc, >> > >> > 100% ack >> > - Booting with a kernel that does not do a PCI reset yields the >> > following topology: >> > >> > >> > -[:00] - >> > +-00.0 Intel Corporation 2nd Generation Core Processor Family >> > DRAM >> > Controller >> > +-02.0 Intel Corporation 2nd Generation Core Processor Family >> > Integrated Graphics Controller >> > +-16.0 Intel Corporation 6 Series/C200 Series Chipset Family MEI >> > Controller #1 >> > +-19.0 Intel Corporation 82579LM Gigabit Network Connection >> > +-1a.0 Intel Corporation 6 Series/C200 Series Chipset Family USB >> > Enhanced Host Controller #2 >> > +-1b.0 Intel Corporation 6 Series/C200 Series Chipset Family High >> > Definition Audio Controller >> > +-1c.0-[02]-- >> > +-1c.1-[03]00.0 Intel Corporation Centrino Advanced-N 6205 >> > [Taylor Peak] >> > +-1c.3-[05-0c]00.0 Renesas Technology Corp. uPD720202 USB 3.0 >> > Host Controller >> > +-1c.4-[0d]00.0 Ricoh Co Ltd MMC/SD Host Controller >> > +-1d.0 Intel Corporation 6 Series/C200 Series Chipset Family USB >> > Enhanced Host Controller #1 >> > +-1f.0 Intel Corporation QM67 Express Chipset Family LPC >> > Controller >> > +-1f.2 Intel Corporation 6 Series/C200 Series Chipset Family 6 >> > port >> > SATA AHCI Controller >> > \-1f.3 Intel Corporation 6 Series/C200 Series Chipset Family >> > SMBus >> > Controller >> > >> > >> > Unplugging and replugging the card (regardless of the kernel >> > version) >> > or booting with a kernel that does the reset leads to the card not >> > being correctly recognized and the problems I have observed. >> >> Hmmm. Just to make sure I understand you correctly: >> >> With a non-reset kernel: if you boot *without* the card inserted, but >> insert it at a later time, the card is unusable? If that's the case, >> that's quite unexpected too... > > This is exactly what is happening when booting with a non-reset kernel. > If you insert the card at a later time it is unusable and the system > starts to behave sluggish. > There is also not a single line logged in /var/log/kern.log. > > Once the card is removed everything is back to normal. Right. So that's just a new version of an existing problem. The issue is that the new reset behaves like an eject/insert sequence as far as the card is concerned, and this triggers a pre-existing issue. That's not to say that we should ignore your report, but I think we should try to fix the root cause, which is that your ExpressCard bridge and your car don't seem to like each other very much. Has this been ever reported on the PCI mailing list? Thanks, M. -- Jazz is not dead. It just smells funny. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mux: add mux_control_get_optional() API
Hi, On 09/08/2017 05:54 PM, Peter Rosin wrote: On 2017-09-08 17:45, Peter Rosin wrote: From: Stephen Boyd Sometimes drivers only use muxes under certain scenarios. For example, the chipidea usb controller may be connected to a usb switch on some platforms, and connected directly to a usb port on others. The driver won't know one way or the other though, so add a mux_control_get_optional() API that allows the driver to differentiate errors getting the mux from there not being a mux for the driver to use at all. --- Documentation/driver-model/devres.txt | 1 + drivers/mux/core.c| 104 +++--- include/linux/mux/consumer.h | 4 ++ 3 files changed, 89 insertions(+), 20 deletions(-) I haven't tested this patch, and hence I have not signed it and I also removed the sign-off from Stephen... Huh, I definitely intended to mention that this patch is based on [1] from Stephen, but that I've made changes according to the comments in that thread (and more). And those changes are what made me remove the sign-off from Stephen... AFAIK normally a Signed-off-by is kept if some (small-ish) changes are made. The S-o-b is mostly an indication that the author is ok with adding the code to the kernel under the GPL. Anyways, Stephen are you ok with re-adding your S-o-b to the version modified by Peter? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson: > > ALSO NOTE: If somehow some of the types of work need to be repeated if > usbnet_defer_kevent() is called multiple times then that should be > quite easy to accomplish without dropping any work on the floor. We > can just keep an atomic count for that type of work and add a loop > into usbnet_deferred_kevent(). Thanks for doing this, it is overdue. Regards Oliver > Signed-off-by: Douglas Anderson Acked-by: Oliver Neukum -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson: > In general when you've got a flag communicating that "something needs > to be done" you want to clear that flag _before_ doing the task. If > you clear the flag _after_ doing the task you end up with the risk > that this will happen: > > 1. Requester sets flag saying task A needs to be done. > 2. Worker comes and stars doing task A. > 3. Worker finishes task A but hasn't yet cleared the flag. > 4. Requester wants to set flag saying task A needs to be done again. > 5. Worker clears the flag without doing anything. > > Let's make the usbnet codebase consistently clear the flag _before_ it > does the requested work. That way if there's another request to do > the work while the work is already in progress it won't be lost. > > NOTES: > - No known bugs are fixed by this; it's just found by code inspection. Hi, unfortunately the patch is wrong. The flags must be cleared only in case the handler is successful. That is not guaranteed. Regards Oliver NACK -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
On Tue, Sep 19, 2017 at 1:37 PM, Oliver Neukum wrote: > Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson: >> In general when you've got a flag communicating that "something needs >> to be done" you want to clear that flag _before_ doing the task. If >> you clear the flag _after_ doing the task you end up with the risk >> that this will happen: >> >> 1. Requester sets flag saying task A needs to be done. >> 2. Worker comes and stars doing task A. >> 3. Worker finishes task A but hasn't yet cleared the flag. >> 4. Requester wants to set flag saying task A needs to be done again. >> 5. Worker clears the flag without doing anything. >> >> Let's make the usbnet codebase consistently clear the flag _before_ it >> does the requested work. That way if there's another request to do >> the work while the work is already in progress it won't be lost. >> >> NOTES: >> - No known bugs are fixed by this; it's just found by code inspection. > > Hi, > > unfortunately the patch is wrong. The flags must be cleared only > in case the handler is successful. That is not guaranteed. > Just out of curiosity, what is the retry mechanism ? Whenever a new, possibly unrelated, event is scheduled ? 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 PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
Hi, On Tue, Sep 19, 2017 at 1:37 PM, Oliver Neukum wrote: > Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson: >> In general when you've got a flag communicating that "something needs >> to be done" you want to clear that flag _before_ doing the task. If >> you clear the flag _after_ doing the task you end up with the risk >> that this will happen: >> >> 1. Requester sets flag saying task A needs to be done. >> 2. Worker comes and stars doing task A. >> 3. Worker finishes task A but hasn't yet cleared the flag. >> 4. Requester wants to set flag saying task A needs to be done again. >> 5. Worker clears the flag without doing anything. >> >> Let's make the usbnet codebase consistently clear the flag _before_ it >> does the requested work. That way if there's another request to do >> the work while the work is already in progress it won't be lost. >> >> NOTES: >> - No known bugs are fixed by this; it's just found by code inspection. > > Hi, > > unfortunately the patch is wrong. The flags must be cleared only > in case the handler is successful. That is not guaranteed. > > Regards > Oliver > > NACK OK, thanks for reviewing! I definitely wasn't super confident about the patch (hence the RFC). Do you think that the races I identified are possible to hit? In other words: should I try to rework the patch somehow or just drop it? Originally I had the patch setting the flags back to true in the failure cases, but then I convinced myself that wasn't needed. I can certainly go back and try it that way... -Doug -- 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] staging: pl2303: adding TD620 Device ID
Signed-off-by: Michael Schoon
---
drivers/usb/serial/pl2303.c | 1 +
drivers/usb/serial/pl2303.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index a585b47..eab0e55 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -94,6 +94,7 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
{ USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
{ USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
+ { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },
{ USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
{ USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
{ USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
diff --git a/drivers/usb/serial/pl2303.h b/drivers/usb/serial/pl2303.h
index 3b5a15d..ca0bfba 100644
--- a/drivers/usb/serial/pl2303.h
+++ b/drivers/usb/serial/pl2303.h
@@ -126,6 +126,7 @@
#define HP_LCM220_PRODUCT_ID 0x3139
#define HP_LCM960_PRODUCT_ID 0x3239
#define HP_LD220_PRODUCT_ID 0x3524
+#define HP_TD620_PRODUCT_ID 0x0956
/* Cressi Edy (diving computer) PC interface */
#define CRESSI_VENDOR_ID 0x04b8
--
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] HID: usbhid: get/put around clearing needs_remote_wakeup
On Mon, Sep 11, 2017 at 4:29 PM, Benson Leung wrote: > Hi Oliver, > > On Mon, Sep 11, 2017 at 01:02:52PM -0700, Benson Leung wrote: >> Thanks for the pointer. I'll respin the patch with the no_resume version of >> usb_autopm_get_interface and retest. >> > > I went and tried this patch but with usb_autopm_get_interface_no_resume > instead > and the original bug I was trying to fix with a Yubikey hid security key came > back, so something is still wrong here. > > I went ahead and filed a bug to track this here: > https://bugs.chromium.org/p/chromium/issues/detail?id=764112 > > I'll find some time to dig into this deeper and understand why this device > ends up getting stuck in active instead of suspending when all handles are > closed. Meh, it is problem in hidraw that basically does usb_autopm_put_interface() before letting usbhid_close() to run. Once I swap hid_hw_power() and hid_hw_close() it autosuspends properly. I just sent out a patch. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/hid: slab-out-of-bounds read in usbhid_parse
Hi Andrey
2017-09-19 21:38 GMT+09:00 Andrey Konovalov :
> On Tue, Sep 19, 2017 at 1:47 PM, Kim Jaejoong wrote:
>> Hi, Andrey Konovalov
>>
>> Thanks for the report.
>>
>> 2017-09-19 2:33 GMT+09:00 Andrey Konovalov :
>>> Hi!
>>>
>>> I've got the following crash while fuzzing the kernel with syzkaller.
>>>
>>> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
>>>
>>> It seems that there's no proper check on the hdesc->bNumDescriptors
>>> value in usbhid_parse(). it iterates over hdesc->desc and accesses
>>> hdesc->desc[n] fields, which might be out-of-bounds.
>>
>> The bNumDescriptors in hid descriptor means 'numeric expression
>> specifying the number of class descriptors'.
>> The value bNumberDescriptors identifies the number of additional class
>> specific descriptors present.
>> (refer to the 6.1.2 HID Descriptor in hid documents :
>> http://www.usb.org/developers/hidpage/HID1_11.pdf)
>>
>> So, it can be out-of-bounds in hdesc->desc[n] if there is an
>> additional class descriptor.
>>
>> Does the patch below fix this?
>
> Hi Kim,
>
> I'm not sure. Is there a check on the bLength field of a
> hid_descriptor struct? Can it be less than sizeof(struct
> hid_descriptor)? If so, we still do an out-of-bounds access to
> hdesc->desc[0] (or some other fields).
You are right. I add hid descriptr size from HID device with bLength field.
Could you test and review below patch?
To. usb & input guys.
While dig this report, i was wondering about bNumDescriptors in HID descriptor.
HID document from usb.org said, 'this number must be at least one (1)
as a Report descriptor will always be present.'
There is no mention of the order of class descriptors. Suppose you
have a HID device with a report descriptor and a physical descriptor.
If you have the following hid descriptor in this case,
HID descriptor
bLength: 12
bDescriptor Type: HID
.. skip
bNumDescriptors: 2
bDescriptorType: physical
bDescriptorLength: any
bDescriptorType: Report
bDescriptorLength: any
If the order of the report descriptor is the second as above,
usbhid_parse () will fail because my patch is only check the first
bDescriptor Type.
But If the order of the report descriptor is always first, there is no
problem. How do you think this?
Thanks, jaejoong
-
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 089bad8..94c3805 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -974,7 +974,7 @@ static int usbhid_parse(struct hid_device *hid)
u32 quirks = 0;
unsigned int rsize = 0;
char *rdesc;
- int ret, n;
+ int ret;
quirks = usbhid_lookup_quirk(le16_to_cpu(dev->descriptor.idVendor),
le16_to_cpu(dev->descriptor.idProduct));
@@ -997,12 +997,16 @@ static int usbhid_parse(struct hid_device *hid)
return -ENODEV;
}
+ if (hdesc->bLength < sizeof(*hdesc)) {
+ dbg_hid("hid descriptor is too short\n");
+ return -EINVAL;
+ }
+
hid->version = le16_to_cpu(hdesc->bcdHID);
hid->country = hdesc->bCountryCode;
- for (n = 0; n < hdesc->bNumDescriptors; n++)
- if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
- rsize = le16_to_cpu(hdesc->desc[n].wDescriptorLength);
+ if (hdesc->desc[0].bDescriptorType == HID_DT_REPORT)
+ rsize = le16_to_cpu(hdesc->desc[0].wDescriptorLength);
if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) {
dbg_hid("weird size of report descriptor (%u)\n", rsize);
---
>
> Thanks!
>
>>
>> Thanks, jaejoong
>> --
>>
>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>> index 089bad8..7b6a0b6 100644
>> --- a/drivers/hid/usbhid/hid-core.c
>> +++ b/drivers/hid/usbhid/hid-core.c
>> @@ -974,7 +974,7 @@ static int usbhid_parse(struct hid_device *hid)
>> u32 quirks = 0;
>> unsigned int rsize = 0;
>> char *rdesc;
>> - int ret, n;
>> + int ret;
>>
>> quirks = usbhid_lookup_quirk(le16_to_cpu(dev->descriptor.idVendor),
>> le16_to_cpu(dev->descriptor.idProduct));
>> @@ -1000,9 +1000,8 @@ static int usbhid_parse(struct hid_device *hid)
>> hid->version = le16_to_cpu(hdesc->bcdHID);
>> hid->country = hdesc->bCountryCode;
>>
>> - for (n = 0; n < hdesc->bNumDescriptors; n++)
>> - if (hdesc->desc[n].bDescriptorType == HID_DT_REPORT)
>> - rsize =
>> le16_to_cpu(hdesc->desc[n].wDescriptorLength);
>> + if (hdesc->desc[0].bDescriptorType == HID_DT_REPORT)
>> + rsize = le16_to_cpu(hdesc->desc[0].wDescriptorLength);
>>
>> if (!rsize || rsize > HID_MAX_DESCRIPTOR_SIZE) {
>> dbg_hid("weird size of report descriptor (%u)\n", rsize);
>>
>>
>>>
>>> ==
>>> BUG: KASAN: sla
Re: [PATCH] staging: pl2303: adding TD620 Device ID
On Tue, Sep 19, 2017 at 09:41:48PM +, Schoon, Michael wrote:
> Signed-off-by: Michael Schoon
No changelog at all? I know I can't take patches like that, hopefully
Johan has the same rule :)
> ---
> drivers/usb/serial/pl2303.c | 1 +
> drivers/usb/serial/pl2303.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index a585b47..eab0e55 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -94,6 +94,7 @@ static const struct usb_device_id id_table[] = {
> { USB_DEVICE(HP_VENDOR_ID, HP_LD960_PRODUCT_ID) },
> { USB_DEVICE(HP_VENDOR_ID, HP_LCM220_PRODUCT_ID) },
> { USB_DEVICE(HP_VENDOR_ID, HP_LCM960_PRODUCT_ID) },
> + { USB_DEVICE(HP_VENDOR_ID, HP_TD620_PRODUCT_ID) },
> { USB_DEVICE(CRESSI_VENDOR_ID, CRESSI_EDY_PRODUCT_ID) },
> { USB_DEVICE(ZEAGLE_VENDOR_ID, ZEAGLE_N2ITION3_PRODUCT_ID) },
> { USB_DEVICE(SONY_VENDOR_ID, SONY_QN3USB_PRODUCT_ID) },
Your patch has had the tabs turned into spaces, making this impossible
to apply. Try testing it out first by emailing it to yourself.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
