Re: usbnet: driver_info->stop required to stop USB interrupts?
On Wed, 2014-03-19 at 17:58 -0700, Grant Grundler wrote:
> Oliver,
> So even with this additional change to usbnet_probe, the device is
> reporting a link but can't transmit packets.
> I've tried with three different USB dongles (AX88178, AX88772B, SMSC75xx).
>
> In the last case, I ran "ifconfig eth0 192.168.1.100" and then tried
> to ping 192.168.1.1. "ifconfig eth0" then reports "RX packets 0 bytes
> 0" and "TX packets 0 bytes 10796". The TX packets != bytes seems odd
> and suggests (to me) that data getting discarded someplace in the
> stack. I confirmed by running tcpdump on the other end of the link
> (192.168.1.1 side) and got nothing from 192.168.1.100.
>
> I believe the "10796" TX bytes is due to the connection manager
> "trying really hard" to get an IP address (via DHCP).
>
> Your patch looks like it should work. But it's missing something and I
> don't have a clue what it is. Ideas?
I have an idea. Could you test this patch?
Regards
Oliver
>From 9a4c4a62c113c5a1abd8345418ccbf95706efdd2 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Tue, 11 Mar 2014 09:59:48 +0100
Subject: [PATCH] usbnet: include wait queue head in device structure
This fixes a race which happens by freeing an object on the stack.
Quoting Julius:
> The issue is
> that it calls usbnet_terminate_urbs() before that, which temporarily
> installs a waitqueue in dev->wait in order to be able to wait on the
> tasklet to run and finish up some queues. The waiting itself looks
> okay, but the access to 'dev->wait' is totally unprotected and can
> race arbitrarily. I think in this case usbnet_bh() managed to succeed
> it's dev->wait check just before usbnet_terminate_urbs() sets it back
> to NULL. The latter then finishes and the waitqueue_t structure on its
> stack gets overwritten by other functions halfway through the
> wake_up() call in usbnet_bh().
The fix is to just not allocate the data structure on the stack.
As dev->wait is abused as a flag it also takes a runtime PM change
to fix this bug.
Signed-off-by: Oliver Neukum
---
drivers/net/usb/usbnet.c | 29 -
include/linux/usb/usbnet.h | 2 +-
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index dd10d58..a8da1af 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -752,14 +752,12 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
// precondition: never called in_interrupt
static void usbnet_terminate_urbs(struct usbnet *dev)
{
- DECLARE_WAIT_QUEUE_HEAD_ONSTACK(unlink_wakeup);
DECLARE_WAITQUEUE(wait, current);
int temp;
/* ensure there are no more active urbs */
- add_wait_queue(&unlink_wakeup, &wait);
+ add_wait_queue(&dev->wait, &wait);
set_current_state(TASK_UNINTERRUPTIBLE);
- dev->wait = &unlink_wakeup;
temp = unlink_urbs(dev, &dev->txq) +
unlink_urbs(dev, &dev->rxq);
@@ -773,15 +771,14 @@ static void usbnet_terminate_urbs(struct usbnet *dev)
"waited for %d urb completions\n", temp);
}
set_current_state(TASK_RUNNING);
- dev->wait = NULL;
- remove_wait_queue(&unlink_wakeup, &wait);
+ remove_wait_queue(&dev->wait, &wait);
}
int usbnet_stop (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
struct driver_info *info = dev->driver_info;
- int retval;
+ int retval, pm;
clear_bit(EVENT_DEV_OPEN, &dev->flags);
netif_stop_queue (net);
@@ -791,6 +788,8 @@ int usbnet_stop (struct net_device *net)
net->stats.rx_packets, net->stats.tx_packets,
net->stats.rx_errors, net->stats.tx_errors);
+ /* to not race resume */
+ pm = usb_autopm_get_interface(dev->intf);
/* allow minidriver to stop correctly (wireless devices to turn off
* radio etc) */
if (info->stop) {
@@ -817,6 +816,9 @@ int usbnet_stop (struct net_device *net)
dev->flags = 0;
del_timer_sync (&dev->delay);
tasklet_kill (&dev->bh);
+ if (!pm)
+ usb_autopm_put_interface(dev->intf);
+
if (info->manage_power &&
!test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags))
info->manage_power(dev, 0);
@@ -1438,10 +1440,9 @@ static void usbnet_bh (unsigned long param)
clear_bit(EVENT_RX_KILL, &dev->flags);
// waiting for all pending urbs to complete?
- if (dev->wait) {
- if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
- wake_up (dev->wait);
- }
+ if (waitqueue_active(&dev->wait)) {
+ if (dev->txq.qlen + dev->rxq.qlen + dev->done.qlen == 0)
+ wake_up_all(&dev->wait);
// or are we maybe short a few urbs?
} else if (netif_running (d
Re: [PATCH 1/2] extcon: of: Remove unnecessary function call by using the name of device_node
Hi,
On Thursday 20 March 2014 12:26 PM, Chanwoo Choi wrote:
> Hi,
>
> On 03/20/2014 02:20 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 20 March 2014 07:52 AM, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> On 03/19/2014 09:08 PM, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 18 March 2014 05:34 PM, Chanwoo Choi wrote:
> This patch remove unnecessary function call in of_extcon_get_extcon_dev()
> by using the name of device_node structure.
>
> Signed-off-by: Chanwoo Choi
> ---
> drivers/extcon/of_extcon.c | 12 ++--
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/extcon/of_extcon.c b/drivers/extcon/of_extcon.c
> index 72173ec..0a29f82 100644
> --- a/drivers/extcon/of_extcon.c
> +++ b/drivers/extcon/of_extcon.c
> @@ -32,7 +32,6 @@ struct extcon_dev *of_extcon_get_extcon_dev(struct
> device *dev, int index)
> {
> struct device_node *node;
> struct extcon_dev *edev;
> - struct platform_device *extcon_parent_dev;
>
> if (!dev->of_node) {
> dev_dbg(dev, "device does not have a device node entry\n");
> @@ -46,16 +45,9 @@ struct extcon_dev *of_extcon_get_extcon_dev(struct
> device *dev, int index)
> return ERR_PTR(-ENODEV);
> }
>
> - extcon_parent_dev = of_find_device_by_node(node);
> - if (!extcon_parent_dev) {
> - dev_dbg(dev, "unable to find device by node\n");
> - return ERR_PTR(-EPROBE_DEFER);
> - }
> -
> - edev = extcon_get_extcon_dev(dev_name(&extcon_parent_dev->dev));
> + edev = extcon_get_extcon_dev(node->name);
Since you no longer want to use device names I think you should add this
too to
warn users if they rely on using the device name.
>>>
>>> Previous of_extcon_get_extcon_dev() support only platform device using
>>> of_find_device_by_node.
>>>
>>> If extcon device is based on i2c/spi/pci and so on,
>>> of_extcon_get_extcon_dev() can't
>>> find device instance for device name. So, I change device name from the
>>> name of platform device
>>> to the name of dt node.
>>
>> That's fine. But we have to fix extcon_dev_register() too, to not use device
>> names right? We have to warn extcon users not to use device names right?
>
> I don't think so. extcon_get_edev_by_phandle() shows wrong node->name as
> following:
The wrong node->name was given by the extcon provider but the error is being
reported in the consumer.
> The consumer could detect the cause of problem which can't get extcon device
> after
> checking error log message.
>
> edev = extcon_get_extcon_dev(node->name);
> if (!edev) {
> dev_err(dev, "unable to get extcon device : %s\n", node->name);
> return ERR_PTR(-ENODEV);
> }
>
> I don't want to add more log message.
I'm not sure if that log message is helpful in anyway to identify the problem
was because of the wrong node->name.
>
>>>
>>>
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index bc4c789..025eb39 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -601,7 +601,6 @@ int extcon_dev_register(struct extcon_dev *edev)
edev->dev.class = extcon_class;
edev->dev.release = extcon_dev_release;
- edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
//The user should always pass the 'name' as we no longer use device name
while
getting extcon device. And this name should also be the 'node' name?
if (IS_ERR_OR_NULL(edev->name)) {
dev_err(&edev->dev,
"extcon device name is null\n");
Btw changing to node name from device name breaks dwc3 in OMAP5 and you
would
need this too..
diff --git a/drivers/extcon/extcon-palmas.c
b/drivers/extcon/extcon-palmas.c
index 2aea4bc..cea8cd3 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -188,6 +188,7 @@ static int palmas_usb_probe(struct platform_device
*pdev)
palmas_usb->edev.supported_cable = palmas_extcon_cable;
palmas_usb->edev.dev.parent = palmas_usb->dev;
+ palmas_usb->edev.name = "palmas_usb";
palmas_usb->edev.mutually_exclusive = mutually_exclusive;
status = extcon_dev_register(&palmas_usb->edev);
Cheers
Kishon
>>>
>>> If node name is same as extcon device name, don't need some modification.
>>> Also, you can modify node name in some OMAP dts file insead of modification
>>> of extcon-palmas.c
>>
>> node name in OMAP dts is already 'palmas_usb'. But since we were not setting
>> 'edev.name' in extcon-palmas.c, extcon_dev_register used device name of
>> palmas
>> (palmas_usb.7 in my case). So extcon-palmas.c needs the fix.
>
> For example,
> I
Problems identifying USB devices persistentely
Hi. We have some USB to CAN devices, and we have the typical problem that the device node (e.g. /dev/pcan0, /dev/pcan1, etc.) is not always the same for the same physical port. I understand that this is expected, but that is solved by inspecting a bit the information that the kernel provides to assign some symbolic link to the wanted one. Or at least that's what I've understood by reading this: http://www.linux-usb.org/FAQ.html#i6 http://comments.gmane.org/gmane.linux.usb.general/61040 http://www.jonnor.com/2010/08/determining-physical-topology-of-hotplugged-usb-devices-with-udev/ But it seems to be failing quite a lot for me, at least on some kernel versions. I was expecting that `udevadm info --query=path` or `lsusb -t` would report exactly the same across reboots if the devices are the same ones, plugged to the same physical ports. Does that make sense? I've tried with different kernels, just in case, and seems only 3.2.0 has the behavior that (maybe naively) I'm expecting (or maybe I didn't tried hard enough). Anyway, I need at least Linux >3.4 since I need to use Bluez 5. My tests: $ md5sum bus*.txt | sort 4a25e1a4ff120110d2447fc2530a22b5 bus-3.12-0.bpo.1-amd64-09-13-19.txt 4a25e1a4ff120110d2447fc2530a22b5 bus-3.12-0.bpo.1-amd64-09-26-57.txt 4a25e1a4ff120110d2447fc2530a22b5 bus-3.9-0.bpo.1-amd64-13-36-30.txt 4a25e1a4ff120110d2447fc2530a22b5 bus-3.9-0.bpo.1-amd64-14-41-29.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-08-22-31.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-08-25-11.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-08-30-16.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-14-44-45.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-14-45-43.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-14-47-22.txt 4b655c3913f0739ce69ca75a2360b3eb bus-3.2.0-4-amd64-14-49-14.txt 545539b21b29af8ca64bcfa5eb1391dc bus-3.9-0.bpo.1-amd64-13-33-37.txt a774c19e12ef21636122d5c573c03393 bus-3.9-0.bpo.1-amd64-13-30-50.txt b26135acc23564e9eee210ecccf5ec6e bus-3.12-0.bpo.1-amd64-09-21-22.txt b26135acc23564e9eee210ecccf5ec6e bus-3.12-0.bpo.1-amd64-09-41-09.txt ce16aca9f56f81101412e1e40f1d141f bus-3.12-0.bpo.1-amd64-09-08-42.txt eeae1db9bffd4016f18f5d4a75279cc6 bus-3.12-0.bpo.1-amd64-09-32-45.txt And contents of the logs I got by running said `udevadm info --query=path` and `lsusb -t`: http://pastie.org/8952893 Any hints are really appreciated. Thank you. -- Alejandro Exojo Piqueras ModpoW, S.L. Technova LaSalle | Sant Joan de la Salle 42 | 08022 Barcelona | www.modpow.es -- To unsubscribe from this list: send the line "unsubscribe 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: Problems identifying USB devices persistentely
Hola Alejandro, Alejandro Exojo wrote: > But it seems to be failing quite a lot for me, at least on some kernel > versions. I was expecting that `udevadm info --query=path` or `lsusb > -t` would report exactly the same across reboots if the devices are > the same ones, plugged to the same physical ports. Does that make > sense? I understand the expectation, but I don't think Linux meets it. The difference in your logs is the usbX bus number, which really can not persist beyond the lifetime of that bus. Think of a laptop with a CardBus slot. CardBus is just PCI. If you plug a USB host card into that laptop, which number will the new usbX bus have? What if that same card is plugged in during boot? What if the card is unplugged? The usbX goes away. And then the card is plugged back in. A new usbX comes in. X is only valid for the lifetime of that bus. For soldered-on buses like in the chipset this means until reboot. I think your best chance is to filter out the usbX bus name yourself. E.g. match on the DEVPATH prefix /devices/pci:00/:00:1d.7/ and on the suffix -4.1:1.0/usbmisc/pcanusb* That will always match the same pcanusb device plugged into the same physical port, as long as chipset or firmware (BIOS upgrade) does not remap PCI devices and/or device functions. While hardware might support such remapping, in particular if the hardware is virtualized, as is increasingly the case, devfns changing is rare. But you do need to coordinate this with whoever is your BIOS engineer if you want actual reliability. //Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] staging: octeon-usb: prevent memory corruption
From: Aaro Koskinen > octeon-hcd will crash the kernel when SLOB is used. This usually happens > after the 18-byte control transfer when a device descriptor is read. > The DMA engine is always transfering full 32-bit words and if the > transfer is shorter, some random garbage appears after the buffer. > The problem is not visible with SLUB since it rounds up the allocations > to word boundary, and the extra bytes will go undetected. > > Fix by providing quirk functions for DMA map/unmap that allocate a bigger > temporary buffer when necessary. Tested by booting EdgeRouter Lite > to USB stick root file system with SLAB, SLOB and SLUB kernels. Wouldn't it be simpler to just round up the existing allocation? (With a comment that some DMA controllers write whole words.) David -- To unsubscribe from this list: send the line "unsubscribe 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: Problems identifying USB devices persistentely
2014-03-20 10:52 GMT+01:00 Peter Stuge : > I understand the expectation, but I don't think Linux meets it. > > The difference in your logs is the usbX bus number, which really can > not persist beyond the lifetime of that bus. > > Think of a laptop with a CardBus slot. CardBus is just PCI. If you > plug a USB host card into that laptop, which number will the new usbX > bus have? > > What if that same card is plugged in during boot? > > What if the card is unplugged? The usbX goes away. And then the card > is plugged back in. A new usbX comes in. X is only valid for the > lifetime of that bus. For soldered-on buses like in the chipset this > means until reboot. OK. I don't fully understand the details, but I think I got your point. > I think your best chance is to filter out the usbX bus name yourself. > > E.g. match on the DEVPATH prefix /devices/pci:00/:00:1d.7/ > and on the suffix -4.1:1.0/usbmisc/pcanusb* That seems good enough to make an udev rule, I will try ASAP. :-) > That will always match the same pcanusb device plugged into the same > physical port, as long as chipset or firmware (BIOS upgrade) does not > remap PCI devices and/or device functions. While hardware might > support such remapping, in particular if the hardware is virtualized, > as is increasingly the case, devfns changing is rare. But you do need > to coordinate this with whoever is your BIOS engineer if you want > actual reliability. Ok, we will have that into account. I think it will be acceptable with our little knowledge on the issue. Thank you very much! We've struggled quite a bit with this. -- Alejandro Exojo Piqueras ModpoW, S.L. Technova LaSalle | Sant Joan de la Salle 42 | 08022 Barcelona | www.modpow.es -- To unsubscribe from this list: send the line "unsubscribe 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] coding style: fix quoted string split across lines
On Wed, 2014-03-19 at 23:04 +0100, Cédric Cabessa wrote:
[]
> diff --git a/drivers/staging/usbip/usbip_common.c
> b/drivers/staging/usbip/usbip_common.c
[]
> @@ -178,8 +178,8 @@ static void usbip_dump_usb_ctrlrequest(struct
> usb_ctrlrequest *cmd)
> }
>
> pr_debug(" ");
> - pr_debug("bRequestType(%02X) bRequest(%02X) wValue(%04X) wIndex(%04X) "
> - "wLength(%04X) ", cmd->bRequestType, cmd->bRequest,
> + pr_debug("bRequestType(%02X) bRequest(%02X) wValue(%04X) wIndex(%04X)
> wLength(%04X) ",
> + cmd->bRequestType, cmd->bRequest,
>cmd->wValue, cmd->wIndex, cmd->wLength);
> pr_debug("\n ");
While you didn't change it, these uses are broken.
pr_debug always starts a new line so the continuations
attempted in this block don't work.
Likely this was originally converted from some non pr_debug
mechanism without an understanding of how pr_debug varies
from a normal printf.
--
To unsubscribe from this list: send the line "unsubscribe 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: Problems identifying USB devices persistentely
On Thu, 20 Mar 2014, Alejandro Exojo wrote: > 2014-03-20 10:52 GMT+01:00 Peter Stuge : > > I understand the expectation, but I don't think Linux meets it. > > > > The difference in your logs is the usbX bus number, which really can > > not persist beyond the lifetime of that bus. > > > > Think of a laptop with a CardBus slot. CardBus is just PCI. If you > > plug a USB host card into that laptop, which number will the new usbX > > bus have? > > > > What if that same card is plugged in during boot? > > > > What if the card is unplugged? The usbX goes away. And then the card > > is plugged back in. A new usbX comes in. X is only valid for the > > lifetime of that bus. For soldered-on buses like in the chipset this > > means until reboot. > > OK. I don't fully understand the details, but I think I got your point. > > > I think your best chance is to filter out the usbX bus name yourself. > > > > E.g. match on the DEVPATH prefix /devices/pci:00/:00:1d.7/ > > and on the suffix -4.1:1.0/usbmisc/pcanusb* > > That seems good enough to make an udev rule, I will try ASAP. :-) > > > That will always match the same pcanusb device plugged into the same > > physical port, as long as chipset or firmware (BIOS upgrade) does not > > remap PCI devices and/or device functions. While hardware might > > support such remapping, in particular if the hardware is virtualized, > > as is increasingly the case, devfns changing is rare. But you do need > > to coordinate this with whoever is your BIOS engineer if you want > > actual reliability. > > Ok, we will have that into account. I think it will be acceptable with > our little knowledge on the issue. > > > Thank you very much! We've struggled quite a bit with this. A more general approach is to match based on the vendor and product IDs, and perhaps also on the serial number (except that lots of devices don't define meaningful serial nunbers). That way, it doesn't matter what port a device is plugged into. For example, if you can match simply on "pcanusb", that would probably be best. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] staging: octeon-usb: prevent memory corruption
On Thu, 20 Mar 2014, David Laight wrote: > From: Aaro Koskinen > > octeon-hcd will crash the kernel when SLOB is used. This usually happens > > after the 18-byte control transfer when a device descriptor is read. > > The DMA engine is always transfering full 32-bit words and if the > > transfer is shorter, some random garbage appears after the buffer. > > The problem is not visible with SLUB since it rounds up the allocations > > to word boundary, and the extra bytes will go undetected. > > > > Fix by providing quirk functions for DMA map/unmap that allocate a bigger > > temporary buffer when necessary. Tested by booting EdgeRouter Lite > > to USB stick root file system with SLAB, SLOB and SLUB kernels. > > Wouldn't it be simpler to just round up the existing allocation? > (With a comment that some DMA controllers write whole words.) No doubt it would be simpler. The problem is that octeon-hcd doesn't make these allocations; they are carried out by other parts of the kernel. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: fsl: Set dma_ops for FSL USB Gadget Device
From: Suresh Gupta DMA mapping functions are moved to common place in udc_core.c which expect the DMA operation of gadget device Signed-off-by: Suresh Gupta --- Changes from previous version: * Added description drivers/usb/gadget/fsl_udc_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c index d68aa77..35cb972 100644 --- a/drivers/usb/gadget/fsl_udc_core.c +++ b/drivers/usb/gadget/fsl_udc_core.c @@ -2456,6 +2456,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev) /* Setup gadget.dev and register with kernel */ dev_set_name(&udc_controller->gadget.dev, "gadget"); udc_controller->gadget.dev.of_node = pdev->dev.of_node; + set_dma_ops(&udc_controller->gadget.dev, pdev->dev.archdata.dma_ops); if (!IS_ERR_OR_NULL(udc_controller->transceiver)) udc_controller->gadget.is_otg = 1; -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] USB: Gadget: fsl driver pullup fix
This fix the fsl usb gadget driver in a way that the usb device will be only "pulled up" on requests only when vbus is powered Signed-off-by: Suresh Gupta --- Changes from previous version: * Removed re-factored code, Will send another patch for re-factoring duplicated code * Changed Description drivers/usb/gadget/fsl_udc_core.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c index 35cb972..49d66779 100644 --- a/drivers/usb/gadget/fsl_udc_core.c +++ b/drivers/usb/gadget/fsl_udc_core.c @@ -1219,6 +1219,10 @@ static int fsl_pullup(struct usb_gadget *gadget, int is_on) struct fsl_udc *udc; udc = container_of(gadget, struct fsl_udc, gadget); + + if(!udc->vbus_active) + return -EOPNOTSUPP; + udc->softconnect = (is_on != 0); if (can_pullup(udc)) fsl_writel((fsl_readl(&dr_regs->usbcmd) | USB_CMD_RUN_STOP), -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: gadget: fsl: Add FSL USB Gadget entry in platform device id
> > > > > @@ -2654,6 +2654,8 @@ static const struct platform_device_id
> > > fsl_udc_devtype[] = {
> > > > > }, {
> > > > > .name = "imx-udc-mx51",
> > > > > }, {
> > > > > + .name = "fsl-usb2-udc",
> > > >
> > > > why aren't you just using chipidea ?
> > > > [SuresH] This is our legacy driver for all previous and existing
> > > > ppc socs. Many of our customers are already using this, and we
> > > > need to support them on this driver. We do have plans to shift to
> > > > chipidea, but after some time.
> > >
> > > cool, you already have plans, so we will see a new glue layer for
> > > v3.16 right ? Which means I don't need to take this patch either.
> > >
> > we do have plans, but in remote future. Right now, we need to support
> > customers on the present legacy driver. We'll phase out this driver
> > slowly when we integrate chipidea. At this time I would request you to
> > please accept this patch
>
> Even if Felipe takes the patch, I'll reject it as you should be doing the
> correct thing here, and if it's accepted, it will never be changed...
Hi Greg, I agree that moving to the chipidea driver is the right thing to do.
However, does this mean that companies have to phase out their current legacy
drivers as soon as a new controller specific driver is introduced in kernel ??
Can't they decide their own schedule based on their own requirements. Our only
concern is to keep supporting our customers till we move to the new driver.
I would really appreciate if you could accept this as this would give us
some time to move to chipidea driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
What to use for usb transfer from gadget to host?
Hello! I am developing on a phyFlex iMx.6 and want to send data via USB to a Host connected via the OTG port. The data is images from an image sensor, so I need a high performance (USB 2.0). I am familiar with USB on the host side, but not the gadget side. What's the best/easiest way to implement a simple 2 endpoints communication with high data rates? During my search I came across functionfs, gadgetfs, configfs and libusbg. Can you recommend one of them that suits my needs? Thank you so much! -- To unsubscribe from this list: send the line "unsubscribe 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: Deadlock in usb-storage error handling
On Wed, 19 Mar 2014, James Bottomley wrote: > > Basically, usb-storage deadlocks when the SCSI error handler invokes > > the eh_device_reset_handler callback while a command is running. The > > command has timed out and will never complete normally, because the > > device's firmware has crashed. But usb-storage's device-reset routine > > waits for the current command to finish, which brings everything to a > > standstill. > > > > Is this design wrong? That is, should the device-reset routine wait > > for currently executing commands to finish, or should it abort them, or > > what? > > In some sense, yes, but not necessarily from the Point of View of USB. > What we assume in SCSI is that commands are forgettable, meaning there's > always some operation we can perform (be it abort or reset) that causes > the device to forget about all outstanding commands and reset its > internal state machine to a known good state. > > The cardinal SCSI assumption is that after we've successfully done an > abort or reset on a command that it will never come back to us from the > device. The same assumption is present in usb-storage. The difference appears to be that SCSI assumes a reset can be issued at any time, even while a command is running. In usb-storage, that's true for a _bus_ reset but it's not true for a _device_ reset. Perhaps this restriction on device resets could be lifted, but in real life it probably won't make much difference. The fact is, almost no USB mass-storage devices implement device reset correctly, whereas most of them do implement bus reset. (Possibly related factoid: MS Windows uses bus resets but doesn't use device resets in its USB mass-storage driver.) > > Or should the SCSI error handler abort the running command before > > invoking the eh_device_reset_handler callback? > > So this is rooted in the "Abort can be a Problem" issue: Abort > sometimes works well (and it's not very disruptive) but sometimes if the > device is having a problem in its command state machine, adding another > command (which is what the abort is) doesn't actually do anything, so we > need error escalation to reset. We can't wait for the abort or other > commands to complete because they never will. The reset is expected to > clear everything from the device (including the pending aborts). With usb-storage, aborts usually work pretty well. But sometimes they don't cure the underlying cause, so when the offending command is retried, it hangs up again. Something like that seems to be happening here. > > For the record, and in case anyone is curious, here's the detailed > > sequence of events during my test: > > > > sd issues a READ(10) command. For whatever reason, the device > > goes nuts and the command times out. > > > > scsi_times_out() calls scsi_abort_command(), which queues an > > abort request. > > > > scmd_eh_abort_handler() calls scsi_try_to_abort_cmd(), which > > succeeds in aborting the READ. > > > > The READ command is retried (I didn't trace through the details > > of this). The retry fails with a Unit Attention (SK=6, > > ASC=0x29, Reset or Bus Device Reset Occurred). > > > > The READ command is retried a second time, and it times out > > again. > > > > This time around, scsi_times_out() calls scsi_abort_command() > > unsuccessfully (because the SCSI_EH_ABORT_SCHEDULED flag is > > still set). > > From the first time we sent the abort? Yes. As far as I can see, the only place where SCSI_EH_ABORT_SCHEDULED gets cleared is in scsi_eh_finish_cmd(), which never got called. After the successful abort, scmd_eh_abort_handler() went directly into its "if (rtn == SUCCESS)" and "scmd %p retry aborted command" / scsi_queue_insert() code paths. > That sounds like a problem in > our state tracking. Could well be. How should I track it down further? Or can you suggest a patch just from this much information? > > As a result, scsi_error_handler() calls scsi_unjam_host(), > > which calls scsi_eh_get_sense(). > > > > That routine calls scsi_request_sense(), which goes into > > scsi_send_eh_cmnd(). > > I thought USB was autosense, so when it reports check condition, we > should already have sense ... or are we calling request_sense without > being sent a check condition status? usb-storage does autosense. As far as I can tell, the REQUEST SENSE is issued because there is no sense data, which is because the READ command is still running. Is this another state-tracking bug? > > The calls to shost->hostt->queuecommand() all fail, because the > > READ command is still running and usb-storage has a queue > > depth of 1. The error messages produced by these failures are > > disconcerting but not dangerous. > > > > Since the REQUEST SENSE command was never issued, > > scsi_eh_get_sense() returns 0. > > > > scsi_unjam_host() goes on to call scsi_eh_abort_cmds(), which > >
Re: [PATCH v7 03/16] usb: cleanup setting udev->removable from port_dev->connect_type
On Wed, 19 Mar 2014, Dan Williams wrote:
> Once usb-acpi has set the port's connect type the usb_device's
> ->removable attribute can be set in the standard location
> set_usb_port_removable().
>
> This also changes behavior in the case where the firmware says that the
> port connect type is unknown. In that case just use the default setting
> determined from the hub descriptor.
>
> Note, we no longer pass udev->portnum to acpi_find_child_device() in the
> root hub case since:
> 1/ the usb-core sets this to zero
> 2/ acpi always expects zero
> ...just pass zero.
>
> Suggested-by: Alan Stern
> Signed-off-by: Dan Williams
This is the one patch I haven't ACKed yet from among the first ten.
> @@ -155,40 +155,18 @@ static struct acpi_device
> *usb_acpi_find_companion(struct device *dev)
>*/
> if (is_usb_device(dev)) {
> udev = to_usb_device(dev);
> - port1 = udev->portnum;
> - if (udev->parent) {
> - struct usb_hub *hub;
> -
> - hub = usb_hub_to_struct_hub(udev->parent);
> - /*
> - * According usb port's connect type to set usb device's
> - * removability.
> - */
> - switch (hub->ports[port1 - 1]->connect_type) {
> - case USB_PORT_CONNECT_TYPE_HOT_PLUG:
> - udev->removable = USB_DEVICE_REMOVABLE;
> - break;
> - case USB_PORT_CONNECT_TYPE_HARD_WIRED:
> - udev->removable = USB_DEVICE_FIXED;
> - break;
> - default:
> - udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
> - break;
> - }
> -
> + if (udev->parent)
> return NULL;
> - }
We should try to find the device's ACPI companion here. That can be
added in a later patch, though.
>
> - /* root hub's parent is the usb hcd. */
> - return acpi_find_child_device(ACPI_COMPANION(dev->parent),
> - port1, false);
> + /* root hub is only child (_ADR=0) under it's parent, the hcd */
s/it's/its/
Remember, "it's" is a contraction of "it is".
s/the hcd/the HC/
"hcd" refers either to a data structure (struct usb_hcd) or a Host
Controller Driver. But here you are referring to the host controller
itself, or the ACPI device corresponding to it.
> + adev = ACPI_COMPANION(dev->parent);
> + return acpi_find_child_device(adev, 0, false);
> } else if (is_usb_port(dev)) {
> struct usb_port *port_dev = to_usb_port(dev);
> - struct acpi_device *adev = NULL;
> + int port1 = port_dev->portnum;
>
> /* Get the struct usb_device point of port's hub */
> udev = to_usb_device(dev->parent->parent);
> - port1 = port_dev->portnum;
>
> /*
>* The root hub ports' parent is the root hub. The non-root-hub
The rest of this comment says "The non-root-hub ports' parent is the
parent hub port which the hub is connected to." I'm not sure that's
really true.
In the ACPI code example shown in section D.1.1 of the xHCI spec, the
device path for port IP1 (which is a port on the integrated hub) is
USB0.RHUB.HCP2.IHUB.IP1. Thus, IP1's parent is the integrated hub,
IHUB, not the hub's upstream port, HCP2.
This also should be fixed later. For now, after you fix the spelling
errors above, you can add
Acked-by: Alan Stern
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: Gadget: fsl driver pullup fix
On Fri, Mar 21, 2014 at 01:57:18AM +0530, Suresh Gupta wrote: > This fix the fsl usb gadget driver in a way that the usb device > will be only "pulled up" on requests only when vbus is powered > > Signed-off-by: Suresh Gupta > --- > Changes from previous version: > * Removed re-factored code, Will send another patch for re-factoring > duplicated code > * Changed Description > > drivers/usb/gadget/fsl_udc_core.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/gadget/fsl_udc_core.c > b/drivers/usb/gadget/fsl_udc_core.c > index 35cb972..49d66779 100644 > --- a/drivers/usb/gadget/fsl_udc_core.c > +++ b/drivers/usb/gadget/fsl_udc_core.c > @@ -1219,6 +1219,10 @@ static int fsl_pullup(struct usb_gadget *gadget, int > is_on) > struct fsl_udc *udc; > > udc = container_of(gadget, struct fsl_udc, gadget); > + > + if(!udc->vbus_active) > + return -EOPNOTSUPP; Always run your patches through scripts/checkpatch.pl so they don't get rejected for silly things like the wrong coding style... -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: fsl: Add FSL USB Gadget entry in platform device id
On Thu, Mar 20, 2014 at 03:01:56PM +, suresh.gu...@freescale.com wrote:
> > > > > > @@ -2654,6 +2654,8 @@ static const struct platform_device_id
> > > > fsl_udc_devtype[] = {
> > > > > > }, {
> > > > > > .name = "imx-udc-mx51",
> > > > > > }, {
> > > > > > + .name = "fsl-usb2-udc",
> > > > >
> > > > > why aren't you just using chipidea ?
> > > > > [SuresH] This is our legacy driver for all previous and existing
> > > > > ppc socs. Many of our customers are already using this, and we
> > > > > need to support them on this driver. We do have plans to shift to
> > > > > chipidea, but after some time.
> > > >
> > > > cool, you already have plans, so we will see a new glue layer for
> > > > v3.16 right ? Which means I don't need to take this patch either.
> > > >
> > > we do have plans, but in remote future. Right now, we need to support
> > > customers on the present legacy driver. We'll phase out this driver
> > > slowly when we integrate chipidea. At this time I would request you to
> > > please accept this patch
> >
> > Even if Felipe takes the patch, I'll reject it as you should be doing the
> > correct thing here, and if it's accepted, it will never be changed...
>
> Hi Greg, I agree that moving to the chipidea driver is the right thing to do.
> However, does this mean that companies have to phase out their current legacy
> drivers as soon as a new controller specific driver is introduced in kernel ??
If their drivers aren't merged upstream, then yes they do, we can't have
duplicate drivers controlling the same hardware blobs.
> Can't they decide their own schedule based on their own requirements. Our
> only
> concern is to keep supporting our customers till we move to the new driver.
Your support issues / requirements is not any of our business, we just
can't accept duplicate code, which I'm sure you can understand.
> I would really appreciate if you could accept this as this would give us
> some time to move to chipidea driver.
What is preventing you from doing this within a week or so? Is it
really that hard of a transistion? If so, is this a problem with the
chipidea driver or something else?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Deadlock in usb-storage error handling
On Thu, 2014-03-20 at 11:36 -0400, Alan Stern wrote: > On Wed, 19 Mar 2014, James Bottomley wrote: > > > > Basically, usb-storage deadlocks when the SCSI error handler invokes > > > the eh_device_reset_handler callback while a command is running. The > > > command has timed out and will never complete normally, because the > > > device's firmware has crashed. But usb-storage's device-reset routine > > > waits for the current command to finish, which brings everything to a > > > standstill. > > > > > > Is this design wrong? That is, should the device-reset routine wait > > > for currently executing commands to finish, or should it abort them, or > > > what? > > > > In some sense, yes, but not necessarily from the Point of View of USB. > > What we assume in SCSI is that commands are forgettable, meaning there's > > always some operation we can perform (be it abort or reset) that causes > > the device to forget about all outstanding commands and reset its > > internal state machine to a known good state. > > > > The cardinal SCSI assumption is that after we've successfully done an > > abort or reset on a command that it will never come back to us from the > > device. > > The same assumption is present in usb-storage. > > The difference appears to be that SCSI assumes a reset can be issued at > any time, even while a command is running. In usb-storage, that's true > for a _bus_ reset but it's not true for a _device_ reset. > > Perhaps this restriction on device resets could be lifted, but in real > life it probably won't make much difference. The fact is, almost no > USB mass-storage devices implement device reset correctly, whereas most > of them do implement bus reset. (Possibly related factoid: MS Windows > uses bus resets but doesn't use device resets in its USB mass-storage > driver.) Actually, you don't have to lift the restriction. Just fail the device reset if you can't issue it (so if there's any outstanding commands). SCSI will move on to a bus reset which you can accept. > > > Or should the SCSI error handler abort the running command before > > > invoking the eh_device_reset_handler callback? > > > > So this is rooted in the "Abort can be a Problem" issue: Abort > > sometimes works well (and it's not very disruptive) but sometimes if the > > device is having a problem in its command state machine, adding another > > command (which is what the abort is) doesn't actually do anything, so we > > need error escalation to reset. We can't wait for the abort or other > > commands to complete because they never will. The reset is expected to > > clear everything from the device (including the pending aborts). > > With usb-storage, aborts usually work pretty well. But sometimes they > don't cure the underlying cause, so when the offending command is > retried, it hangs up again. Something like that seems to be happening > here. > > > > For the record, and in case anyone is curious, here's the detailed > > > sequence of events during my test: > > > > > > sd issues a READ(10) command. For whatever reason, the device > > > goes nuts and the command times out. > > > > > > scsi_times_out() calls scsi_abort_command(), which queues an > > > abort request. > > > > > > scmd_eh_abort_handler() calls scsi_try_to_abort_cmd(), which > > > succeeds in aborting the READ. > > > > > > The READ command is retried (I didn't trace through the details > > > of this). The retry fails with a Unit Attention (SK=6, > > > ASC=0x29, Reset or Bus Device Reset Occurred). > > > > > > The READ command is retried a second time, and it times out > > > again. > > > > > > This time around, scsi_times_out() calls scsi_abort_command() > > > unsuccessfully (because the SCSI_EH_ABORT_SCHEDULED flag is > > > still set). > > > > From the first time we sent the abort? > > Yes. As far as I can see, the only place where SCSI_EH_ABORT_SCHEDULED > gets cleared is in scsi_eh_finish_cmd(), which never got called. > After the successful abort, scmd_eh_abort_handler() went directly into > its "if (rtn == SUCCESS)" and "scmd %p retry aborted command" / > scsi_queue_insert() code paths. OK, that's a bug ... I'll see if I can find it. > > That sounds like a problem in > > our state tracking. > > Could well be. How should I track it down further? Or can you suggest > a patch just from this much information? > > > > As a result, scsi_error_handler() calls scsi_unjam_host(), > > > which calls scsi_eh_get_sense(). > > > > > > That routine calls scsi_request_sense(), which goes into > > > scsi_send_eh_cmnd(). > > > > I thought USB was autosense, so when it reports check condition, we > > should already have sense ... or are we calling request_sense without > > being sent a check condition status? > > usb-storage does autosense. As far as I can tell, the REQUEST SENSE is > issued because there is no sense data, which is because the READ > command is still running. Is this
Re: [PATCH] usb: gadget: fsl: Add FSL USB Gadget entry in platform device id
Hi,
On Thu, Mar 20, 2014 at 09:02:52AM -0700, gre...@linuxfoundation.org wrote:
> On Thu, Mar 20, 2014 at 03:01:56PM +, suresh.gu...@freescale.com wrote:
> > > > > > > @@ -2654,6 +2654,8 @@ static const struct platform_device_id
> > > > > fsl_udc_devtype[] = {
> > > > > > > }, {
> > > > > > > .name = "imx-udc-mx51",
> > > > > > > }, {
> > > > > > > + .name = "fsl-usb2-udc",
> > > > > >
> > > > > > why aren't you just using chipidea ?
> > > > > > [SuresH] This is our legacy driver for all previous and existing
> > > > > > ppc socs. Many of our customers are already using this, and we
> > > > > > need to support them on this driver. We do have plans to shift to
> > > > > > chipidea, but after some time.
> > > > >
> > > > > cool, you already have plans, so we will see a new glue layer for
> > > > > v3.16 right ? Which means I don't need to take this patch either.
> > > > >
> > > > we do have plans, but in remote future. Right now, we need to support
> > > > customers on the present legacy driver. We'll phase out this driver
> > > > slowly when we integrate chipidea. At this time I would request you to
> > > > please accept this patch
> > >
> > > Even if Felipe takes the patch, I'll reject it as you should be doing the
> > > correct thing here, and if it's accepted, it will never be changed...
> >
> > Hi Greg, I agree that moving to the chipidea driver is the right thing to
> > do.
> > However, does this mean that companies have to phase out their current
> > legacy
> > drivers as soon as a new controller specific driver is introduced in kernel
> > ??
>
> If their drivers aren't merged upstream, then yes they do, we can't have
> duplicate drivers controlling the same hardware blobs.
>
> > Can't they decide their own schedule based on their own requirements. Our
> > only
> > concern is to keep supporting our customers till we move to the new driver.
>
> Your support issues / requirements is not any of our business, we just
> can't accept duplicate code, which I'm sure you can understand.
>
> > I would really appreciate if you could accept this as this would give us
> > some time to move to chipidea driver.
>
> What is preventing you from doing this within a week or so? Is it
> really that hard of a transistion? If so, is this a problem with the
> chipidea driver or something else?
Greg, many other freescale SoCs are *already* using chipidea driver. I
suppose 1 week effort is overestimation, it shouldn't take much more
than a couple days.
If there are any bugs in chipidea, let's find them and fix them up. We
really cannot continue with code duplication in the tree, it's just
moronic to do so, it's an unnecessary maintenance burden, extra review
time, same fixes having to written for separate drivers, and so on.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v7 03/16] usb: cleanup setting udev->removable from port_dev->connect_type
On Thu, 2014-03-20 at 11:58 -0400, Alan Stern wrote:
> On Wed, 19 Mar 2014, Dan Williams wrote:
>
> > Once usb-acpi has set the port's connect type the usb_device's
> > ->removable attribute can be set in the standard location
> > set_usb_port_removable().
> >
> > This also changes behavior in the case where the firmware says that the
> > port connect type is unknown. In that case just use the default setting
> > determined from the hub descriptor.
> >
> > Note, we no longer pass udev->portnum to acpi_find_child_device() in the
> > root hub case since:
> > 1/ the usb-core sets this to zero
> > 2/ acpi always expects zero
> > ...just pass zero.
> >
> > Suggested-by: Alan Stern
> > Signed-off-by: Dan Williams
>
> This is the one patch I haven't ACKed yet from among the first ten.
>
> > @@ -155,40 +155,18 @@ static struct acpi_device
> > *usb_acpi_find_companion(struct device *dev)
> > */
> > if (is_usb_device(dev)) {
> > udev = to_usb_device(dev);
> > - port1 = udev->portnum;
> > - if (udev->parent) {
> > - struct usb_hub *hub;
> > -
> > - hub = usb_hub_to_struct_hub(udev->parent);
> > - /*
> > -* According usb port's connect type to set usb device's
> > -* removability.
> > -*/
> > - switch (hub->ports[port1 - 1]->connect_type) {
> > - case USB_PORT_CONNECT_TYPE_HOT_PLUG:
> > - udev->removable = USB_DEVICE_REMOVABLE;
> > - break;
> > - case USB_PORT_CONNECT_TYPE_HARD_WIRED:
> > - udev->removable = USB_DEVICE_FIXED;
> > - break;
> > - default:
> > - udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
> > - break;
> > - }
> > -
> > + if (udev->parent)
> > return NULL;
> > - }
>
> We should try to find the device's ACPI companion here. That can be
> added in a later patch, though.
>
> >
> > - /* root hub's parent is the usb hcd. */
> > - return acpi_find_child_device(ACPI_COMPANION(dev->parent),
> > - port1, false);
> > + /* root hub is only child (_ADR=0) under it's parent, the hcd */
>
> s/it's/its/
>
> Remember, "it's" is a contraction of "it is".
Not sure that extra clarification was worth the keystrokes, but typo
fixed.
>
> s/the hcd/the HC/
>
> "hcd" refers either to a data structure (struct usb_hcd) or a Host
> Controller Driver. But here you are referring to the host controller
> itself, or the ACPI device corresponding to it.
ok
>
> > + adev = ACPI_COMPANION(dev->parent);
> > + return acpi_find_child_device(adev, 0, false);
> > } else if (is_usb_port(dev)) {
> > struct usb_port *port_dev = to_usb_port(dev);
> > - struct acpi_device *adev = NULL;
> > + int port1 = port_dev->portnum;
> >
> > /* Get the struct usb_device point of port's hub */
> > udev = to_usb_device(dev->parent->parent);
> > - port1 = port_dev->portnum;
> >
> > /*
> > * The root hub ports' parent is the root hub. The non-root-hub
>
> The rest of this comment says "The non-root-hub ports' parent is the
> parent hub port which the hub is connected to." I'm not sure that's
> really true.
>
> In the ACPI code example shown in section D.1.1 of the xHCI spec, the
> device path for port IP1 (which is a port on the integrated hub) is
> USB0.RHUB.HCP2.IHUB.IP1. Thus, IP1's parent is the integrated hub,
> IHUB, not the hub's upstream port, HCP2.
>
> This also should be fixed later.
Indeed, I was overlooking the correctness of the existing
implementation. It seems it does need more revision.
> For now, after you fix the spelling
> errors above, you can add
>
> Acked-by: Alan Stern
>
Thanks Alan.
8<---
Subject: usb: cleanup setting udev->removable from port_dev->connect_type
From: Dan Williams
Once usb-acpi has set the port's connect type the usb_device's
->removable attribute can be set in the standard location
set_usb_port_removable().
This also changes behavior in the case where the firmware says that the
port connect type is unknown. In that case just use the default setting
determined from the hub descriptor.
Note, we no longer pass udev->portnum to acpi_find_child_device() in the
root hub case since:
1/ the usb-core sets this to zero
2/ acpi always expects zero
...just pass zero.
Suggested-by: Alan Stern
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 22 +-
drivers/usb/core/usb-acpi.c | 34 ++
2 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/co
Re: Deadlock in usb-storage error handling
On Thu, 20 Mar 2014, James Bottomley wrote: > OK, so I think we have three things to do > > 1. Investigate SCSI and fix it's abort state problem that's causing > it not to send the abort second time around > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset > with outstanding commands) > 3. Find out why we're sending a spurious request sense. > > I can look at 1 and 3 if you want to take 2. It's a deal! Thanks for your help. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usbnet: driver_info->stop required to stop USB interrupts?
On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum wrote:
...
> I have an idea. Could you test this patch?
...
> - if (dev->wait) {
..
> + if (waitqueue_active(&dev->wait)) {
Yes - building new image now (and transfer to USB and boot from USB).
Should know in an hour or so (doing other things in parallel).
I was sure the problem is in usbnet_bh() since that's the only code
change I'm actually exercising (so far). The way I was reading the
code, we might see extra wake_up calls...but there is clearly more
going on.
Can you please explain why we need to check if the waitqueue is active?
This patch should also add a comment to hint why waitqueue_active()
must be called.
Why? Several experienced people looked at the code and didn't see the
problem including the original author of the patch.
thanks,
grant
--
To unsubscribe from this list: send the line "unsubscribe 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: Deadlock in usb-storage error handling
On Thu, 2014-03-20 at 12:34 -0400, Alan Stern wrote:
> On Thu, 20 Mar 2014, James Bottomley wrote:
>
> > OK, so I think we have three things to do
> >
> > 1. Investigate SCSI and fix it's abort state problem that's causing
> > it not to send the abort second time around
> > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset
> > with outstanding commands)
> > 3. Find out why we're sending a spurious request sense.
> >
> > I can look at 1 and 3 if you want to take 2.
>
> It's a deal! Thanks for your help.
OK, I think this is the fix for 1, if you could try it out.
Thanks,
James
---
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 771c16b..c52bfb2 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -145,14 +145,14 @@ scmd_eh_abort_handler(struct work_struct *work)
"scmd %p retry "
"aborted command\n", scmd));
scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
- return;
+ goto out;
} else {
SCSI_LOG_ERROR_RECOVERY(3,
scmd_printk(KERN_WARNING, scmd,
"scmd %p finish "
"aborted command\n", scmd));
scsi_finish_command(scmd);
- return;
+ goto out;
}
} else {
SCSI_LOG_ERROR_RECOVERY(3,
@@ -162,6 +162,8 @@ scmd_eh_abort_handler(struct work_struct *work)
}
}
+ scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
+
if (!scsi_eh_scmd_add(scmd, 0)) {
SCSI_LOG_ERROR_RECOVERY(3,
scmd_printk(KERN_WARNING, scmd,
@@ -170,6 +172,10 @@ scmd_eh_abort_handler(struct work_struct *work)
scmd->result |= DID_TIME_OUT << 16;
scsi_finish_command(scmd);
}
+ return;
+ out:
+ scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
+ return;
}
/**
--
To unsubscribe from this list: send the line "unsubscribe 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: Deadlock in usb-storage error handling
On Thu, 2014-03-20 at 12:34 -0400, Alan Stern wrote: > On Thu, 20 Mar 2014, James Bottomley wrote: > > > OK, so I think we have three things to do > > > > 1. Investigate SCSI and fix it's abort state problem that's causing > > it not to send the abort second time around > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset > > with outstanding commands) > > 3. Find out why we're sending a spurious request sense. > > > > I can look at 1 and 3 if you want to take 2. > > It's a deal! Thanks for your help. And this looks to be 3: a bug in the way we attach sense data to commands (we shouldn't look for attached sense if the device error code didn't imply there would be any). James --- diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 771c16b..d020149 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1157,6 +1157,15 @@ int scsi_eh_get_sense(struct list_head *work_q, __func__)); break; } + if (status_byte(scmd->result) != CHECK_CONDITION) + /* +* don't request sense if there's no check condition +* status because the error we're processing isn't one +* that has a sense code (and some devices get +* confused by sense requests out of the blue) +*/ + continue; + SCSI_LOG_ERROR_RECOVERY(2, scmd_printk(KERN_INFO, scmd, "%s: requesting sense\n", current->comm)); -- To unsubscribe from this list: send the line "unsubscribe 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: Deadlock in usb-storage error handling
On Thu, 20 Mar 2014, James Bottomley wrote:
> On Thu, 2014-03-20 at 12:34 -0400, Alan Stern wrote:
> > On Thu, 20 Mar 2014, James Bottomley wrote:
> >
> > > OK, so I think we have three things to do
> > >
> > > 1. Investigate SCSI and fix it's abort state problem that's causing
> > > it not to send the abort second time around
> > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset
> > > with outstanding commands)
> > > 3. Find out why we're sending a spurious request sense.
> > >
> > > I can look at 1 and 3 if you want to take 2.
> >
> > It's a deal! Thanks for your help.
>
> And this looks to be 3: a bug in the way we attach sense data to
> commands (we shouldn't look for attached sense if the device error code
> didn't imply there would be any).
>
> James
>
> ---
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 771c16b..d020149 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1157,6 +1157,15 @@ int scsi_eh_get_sense(struct list_head *work_q,
>__func__));
> break;
> }
> + if (status_byte(scmd->result) != CHECK_CONDITION)
> + /*
> + * don't request sense if there's no check condition
> + * status because the error we're processing isn't one
> + * that has a sense code (and some devices get
> + * confused by sense requests out of the blue)
> + */
> + continue;
> +
> SCSI_LOG_ERROR_RECOVERY(2, scmd_printk(KERN_INFO, scmd,
> "%s: requesting sense\n",
> current->comm));
I tried this patch first, because fixing the earlier bug would mask
this one.
The patch sort of worked. But the first time I tried it, it failed in
a rather amusing way. While the second retry was running and hung,
scmd->result _was_ equal to CHECK_CONDITION -- because that was the
result from the _first_ retry, and it had never gotten cleared!
scmd->result needs to be set to 0 before the queuecommand callback is
invoked. I ended up adding this to your patch, and then it worked
perfectly:
Index: usb-3.14/drivers/scsi/scsi_error.c
===
--- usb-3.14.orig/drivers/scsi/scsi_error.c
+++ usb-3.14/drivers/scsi/scsi_error.c
@@ -924,6 +924,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd
memset(scmd->cmnd, 0, BLK_MAX_CDB);
memset(&scmd->sdb, 0, sizeof(scmd->sdb));
scmd->request->next_rq = NULL;
+ scmd->result = 0;
if (sense_bytes) {
scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
Index: usb-3.14/drivers/scsi/scsi_lib.c
===
--- usb-3.14.orig/drivers/scsi/scsi_lib.c
+++ usb-3.14/drivers/scsi/scsi_lib.c
@@ -159,6 +159,7 @@ static void __scsi_queue_insert(struct s
* lock such that the kblockd_schedule_work() call happens
* before blk_cleanup_queue() finishes.
*/
+ cmd->result = 0;
spin_lock_irqsave(q->queue_lock, flags);
blk_requeue_request(q, cmd->request);
kblockd_schedule_work(q, &device->requeue_work);
Maybe only the second one is necessary, but it seemed best to be
consistent.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Deadlock in usb-storage error handling
On Thu, 20 Mar 2014, James Bottomley wrote:
> On Thu, 2014-03-20 at 12:34 -0400, Alan Stern wrote:
> > On Thu, 20 Mar 2014, James Bottomley wrote:
> >
> > > OK, so I think we have three things to do
> > >
> > > 1. Investigate SCSI and fix it's abort state problem that's causing
> > > it not to send the abort second time around
> > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset
> > > with outstanding commands)
> > > 3. Find out why we're sending a spurious request sense.
> > >
> > > I can look at 1 and 3 if you want to take 2.
> >
> > It's a deal! Thanks for your help.
>
> OK, I think this is the fix for 1, if you could try it out.
>
> Thanks,
>
> James
>
> ---
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 771c16b..c52bfb2 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -145,14 +145,14 @@ scmd_eh_abort_handler(struct work_struct *work)
> "scmd %p retry "
> "aborted command\n", scmd));
> scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
> - return;
> + goto out;
> } else {
> SCSI_LOG_ERROR_RECOVERY(3,
> scmd_printk(KERN_WARNING, scmd,
> "scmd %p finish "
> "aborted command\n", scmd));
> scsi_finish_command(scmd);
> - return;
> + goto out;
> }
> } else {
> SCSI_LOG_ERROR_RECOVERY(3,
> @@ -162,6 +162,8 @@ scmd_eh_abort_handler(struct work_struct *work)
> }
> }
>
> + scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
> +
> if (!scsi_eh_scmd_add(scmd, 0)) {
> SCSI_LOG_ERROR_RECOVERY(3,
> scmd_printk(KERN_WARNING, scmd,
> @@ -170,6 +172,10 @@ scmd_eh_abort_handler(struct work_struct *work)
> scmd->result |= DID_TIME_OUT << 16;
> scsi_finish_command(scmd);
> }
> + return;
> + out:
> + scmd->eh_eflags &= ~SCSI_EH_ABORT_SCHEDULED;
> + return;
> }
>
> /**
This worked the first time. :-)
But I wonder, is it safe to access scmd after calling
scsi_finish_command()?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Deadlock in usb-storage error handling
On Thu, 20 Mar 2014, James Bottomley wrote: > OK, so I think we have three things to do > > 1. Investigate SCSI and fix it's abort state problem that's causing > it not to send the abort second time around > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset > with outstanding commands) > 3. Find out why we're sending a spurious request sense. > > I can look at 1 and 3 if you want to take 2. I wrote a patch for 2. It turned out not to help much, because I was wrong -- while a command is running, usb-storage won't do a bus reset either. The lock occurs in a separate part of the code, where I wasn't looking earlier. When I tested the patch, the device reset failed immediately (as desired) and then the attempted bus reset hung. Overall, not much of an improvement... In fact, this restriction on bus resets is important and necessary. USB devices can be composite, meaning they can contain several functions all packed into a single device. If a driver for one of the other functions needs to perform a bus reset, we don't want it to interrupt an ongoing mass-storage command. Thus, it is important for the reset routine to wait for an outstanding command to complete. Anyway, this looks to be a moot point. With your fix for 1, neither sort of reset was necessary. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Deadlock in usb-storage error handling
On Thu, 2014-03-20 at 15:48 -0400, Alan Stern wrote:
> On Thu, 20 Mar 2014, James Bottomley wrote:
>
> > On Thu, 2014-03-20 at 12:34 -0400, Alan Stern wrote:
> > > On Thu, 20 Mar 2014, James Bottomley wrote:
> > >
> > > > OK, so I think we have three things to do
> > > >
> > > > 1. Investigate SCSI and fix it's abort state problem that's causing
> > > > it not to send the abort second time around
> > > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset
> > > > with outstanding commands)
> > > > 3. Find out why we're sending a spurious request sense.
> > > >
> > > > I can look at 1 and 3 if you want to take 2.
> > >
> > > It's a deal! Thanks for your help.
> >
> > And this looks to be 3: a bug in the way we attach sense data to
> > commands (we shouldn't look for attached sense if the device error code
> > didn't imply there would be any).
> >
> > James
> >
> > ---
> >
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 771c16b..d020149 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -1157,6 +1157,15 @@ int scsi_eh_get_sense(struct list_head *work_q,
> > __func__));
> > break;
> > }
> > + if (status_byte(scmd->result) != CHECK_CONDITION)
> > + /*
> > +* don't request sense if there's no check condition
> > +* status because the error we're processing isn't one
> > +* that has a sense code (and some devices get
> > +* confused by sense requests out of the blue)
> > +*/
> > + continue;
> > +
> > SCSI_LOG_ERROR_RECOVERY(2, scmd_printk(KERN_INFO, scmd,
> > "%s: requesting sense\n",
> > current->comm));
>
> I tried this patch first, because fixing the earlier bug would mask
> this one.
>
> The patch sort of worked. But the first time I tried it, it failed in
> a rather amusing way. While the second retry was running and hung,
> scmd->result _was_ equal to CHECK_CONDITION -- because that was the
> result from the _first_ retry, and it had never gotten cleared!
>
> scmd->result needs to be set to 0 before the queuecommand callback is
> invoked. I ended up adding this to your patch, and then it worked
> perfectly:
Wow, the stale data bugs are just crawling out of the code. Thanks for
checking.
>
> Index: usb-3.14/drivers/scsi/scsi_error.c
> ===
> --- usb-3.14.orig/drivers/scsi/scsi_error.c
> +++ usb-3.14/drivers/scsi/scsi_error.c
> @@ -924,6 +924,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd
> memset(scmd->cmnd, 0, BLK_MAX_CDB);
> memset(&scmd->sdb, 0, sizeof(scmd->sdb));
> scmd->request->next_rq = NULL;
> + scmd->result = 0;
>
> if (sense_bytes) {
> scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
> Index: usb-3.14/drivers/scsi/scsi_lib.c
> ===
> --- usb-3.14.orig/drivers/scsi/scsi_lib.c
> +++ usb-3.14/drivers/scsi/scsi_lib.c
> @@ -159,6 +159,7 @@ static void __scsi_queue_insert(struct s
>* lock such that the kblockd_schedule_work() call happens
>* before blk_cleanup_queue() finishes.
>*/
> + cmd->result = 0;
> spin_lock_irqsave(q->queue_lock, flags);
> blk_requeue_request(q, cmd->request);
> kblockd_schedule_work(q, &device->requeue_work);
>
>
> Maybe only the second one is necessary, but it seemed best to be
> consistent.
Thanks, I'll add this one to the list as well and see if we can get it
into the merge window. I take it you'd like a cc to stable on these
three?
James
--
To unsubscribe from this list: send the line "unsubscribe 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: Deadlock in usb-storage error handling
On Thu, 2014-03-20 at 15:59 -0400, Alan Stern wrote: > On Thu, 20 Mar 2014, James Bottomley wrote: > > > OK, so I think we have three things to do > > > > 1. Investigate SCSI and fix it's abort state problem that's causing > > it not to send the abort second time around > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset > > with outstanding commands) > > 3. Find out why we're sending a spurious request sense. > > > > I can look at 1 and 3 if you want to take 2. > > I wrote a patch for 2. It turned out not to help much, because I was > wrong -- while a command is running, usb-storage won't do a bus reset > either. The lock occurs in a separate part of the code, where I wasn't > looking earlier. When I tested the patch, the device reset failed > immediately (as desired) and then the attempted bus reset hung. > Overall, not much of an improvement... Hm, yes, sorry, after the bus reset fails, we'll just take the device offline. > In fact, this restriction on bus resets is important and necessary. > USB devices can be composite, meaning they can contain several > functions all packed into a single device. If a driver for one of the > other functions needs to perform a bus reset, we don't want it to > interrupt an ongoing mass-storage command. Thus, it is important for > the reset routine to wait for an outstanding command to complete. > > Anyway, this looks to be a moot point. With your fix for 1, neither > sort of reset was necessary. Well as long as the tiny hammer works ... but there are going to be devices that need the bigger one one day. James -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] coding style: fix line over 80 characters
Hello.
On 03/20/2014 01:04 AM, Cédric Cabessa wrote:
Signed-off-by: Cédric Cabessa
---
drivers/staging/usbip/vhci_hcd.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
index f690668..1e84577 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16
typeReq, u16 wValue,
}
break;
case USB_PORT_FEAT_POWER:
- usbip_dbg_vhci_rh(" ClearPortFeature:
USB_PORT_FEAT_POWER\n");
+ usbip_dbg_vhci_rh(
+ " ClearPortFeature: USB_PORT_FEAT_POWER\n");
Your version of scripts/checkpatch.pl seems outdated. It shouldn't
complain about strings violating 80-column limit (and I've just verified it
doesn't).
@@ -539,7 +546,9 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb
*urb,
default:
/* NOT REACHED */
- dev_err(dev, "invalid request to devnum 0 bRequest %u, wValue
%u\n", ctrlreq->bRequest,
+ dev_err(dev,
+ "invalid request to devnum 0 bRequest %u, wValue
%u\n",
+ ctrlreq->bRequest,
ctrlreq->wValue);
ret = -EINVAL;
goto no_need_xmit;
@@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device *pdev,
pm_message_t state)
spin_unlock(&the_controller->lock);
if (connected > 0) {
- dev_info(&pdev->dev, "We have %d active connection%s. Do not suspend.\n",
connected, (connected == 1 ? "" : "s"));
+ dev_info(&pdev->dev,
+"We have %d active connection%s. Do not suspend.\n",
+connected, (connected == 1 ? "" : "s"));
ret = -EBUSY;
} else {
dev_info(&pdev->dev, "suspend vhci_hcd");
Hm, I don't see checkpatch.pl complaints about these either in the
'usb-next' branch of Greg's tree.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] coding style: fix line over 80 characters
On 03/21/2014 12:34 AM, Sergei Shtylyov wrote:
Signed-off-by: Cédric Cabessa
---
drivers/staging/usbip/vhci_hcd.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/usbip/vhci_hcd.c
b/drivers/staging/usbip/vhci_hcd.c
index f690668..1e84577 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
[...]
@@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device
*pdev, pm_message_t state)
spin_unlock(&the_controller->lock);
if (connected > 0) {
-dev_info(&pdev->dev, "We have %d active connection%s. Do not
suspend.\n", connected, (connected == 1 ? "" : "s"));
+dev_info(&pdev->dev,
+ "We have %d active connection%s. Do not suspend.\n",
+ connected, (connected == 1 ? "" : "s"));
ret = -EBUSY;
} else {
dev_info(&pdev->dev, "suspend vhci_hcd");
Hm, I don't see checkpatch.pl complaints about these either in the
'usb-next' branch of Greg's tree.
In fact, it does complain but says:
WARNING: quoted string split across lines
So, it looks like your patch is against the wrong tree.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Deadlock in usb-storage error handling
On Thu, 20 Mar 2014, James Bottomley wrote:
> > I tried this patch first, because fixing the earlier bug would mask
> > this one.
> >
> > The patch sort of worked. But the first time I tried it, it failed in
> > a rather amusing way. While the second retry was running and hung,
> > scmd->result _was_ equal to CHECK_CONDITION -- because that was the
> > result from the _first_ retry, and it had never gotten cleared!
> >
> > scmd->result needs to be set to 0 before the queuecommand callback is
> > invoked. I ended up adding this to your patch, and then it worked
> > perfectly:
>
> Wow, the stale data bugs are just crawling out of the code. Thanks for
> checking.
>
> >
> > Index: usb-3.14/drivers/scsi/scsi_error.c
> > ===
> > --- usb-3.14.orig/drivers/scsi/scsi_error.c
> > +++ usb-3.14/drivers/scsi/scsi_error.c
> > @@ -924,6 +924,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd
> > memset(scmd->cmnd, 0, BLK_MAX_CDB);
> > memset(&scmd->sdb, 0, sizeof(scmd->sdb));
> > scmd->request->next_rq = NULL;
> > + scmd->result = 0;
> >
> > if (sense_bytes) {
> > scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
> > Index: usb-3.14/drivers/scsi/scsi_lib.c
> > ===
> > --- usb-3.14.orig/drivers/scsi/scsi_lib.c
> > +++ usb-3.14/drivers/scsi/scsi_lib.c
> > @@ -159,6 +159,7 @@ static void __scsi_queue_insert(struct s
> > * lock such that the kblockd_schedule_work() call happens
> > * before blk_cleanup_queue() finishes.
> > */
> > + cmd->result = 0;
> > spin_lock_irqsave(q->queue_lock, flags);
> > blk_requeue_request(q, cmd->request);
> > kblockd_schedule_work(q, &device->requeue_work);
> >
> >
> > Maybe only the second one is necessary, but it seemed best to be
> > consistent.
>
> Thanks, I'll add this one to the list as well and see if we can get it
> into the merge window. I take it you'd like a cc to stable on these
> three?
Yes, please.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Deadlock in usb-storage error handling
On Thu, 20 Mar 2014, James Bottomley wrote: > On Thu, 2014-03-20 at 15:59 -0400, Alan Stern wrote: > > On Thu, 20 Mar 2014, James Bottomley wrote: > > > > > OK, so I think we have three things to do > > > > > > 1. Investigate SCSI and fix it's abort state problem that's causing > > > it not to send the abort second time around > > > 2. Fix usb-storage to fail a reset it can't do (i.e. device reset > > > with outstanding commands) > > > 3. Find out why we're sending a spurious request sense. > > > > > > I can look at 1 and 3 if you want to take 2. > > > > I wrote a patch for 2. It turned out not to help much, because I was > > wrong -- while a command is running, usb-storage won't do a bus reset > > either. The lock occurs in a separate part of the code, where I wasn't > > looking earlier. When I tested the patch, the device reset failed > > immediately (as desired) and then the attempted bus reset hung. > > Overall, not much of an improvement... > > Hm, yes, sorry, after the bus reset fails, we'll just take the device > offline. > > > In fact, this restriction on bus resets is important and necessary. > > USB devices can be composite, meaning they can contain several > > functions all packed into a single device. If a driver for one of the > > other functions needs to perform a bus reset, we don't want it to > > interrupt an ongoing mass-storage command. Thus, it is important for > > the reset routine to wait for an outstanding command to complete. > > > > Anyway, this looks to be a moot point. With your fix for 1, neither > > sort of reset was necessary. > > Well as long as the tiny hammer works ... but there are going to be > devices that need the bigger one one day. Maybe so. I'll worry about them when they appear. :-) Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] coding style: fix line over 80 characters
On Fri, 2014-03-21 at 00:34 +0300, Sergei Shtylyov wrote:
> On 03/20/2014 01:04 AM, Cédric Cabessa wrote:
> > diff --git a/drivers/staging/usbip/vhci_hcd.c
> > b/drivers/staging/usbip/vhci_hcd.c
[]
> > @@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16
> > typeReq, u16 wValue,
> > }
> > break;
> > case USB_PORT_FEAT_POWER:
> > - usbip_dbg_vhci_rh(" ClearPortFeature:
> > USB_PORT_FEAT_POWER\n");
> > + usbip_dbg_vhci_rh(
> > + " ClearPortFeature: USB_PORT_FEAT_POWER\n");
>
> Your version of scripts/checkpatch.pl seems outdated. It shouldn't
> complain about strings violating 80-column limit (and I've just verified it
> doesn't).
checkpatch complains about > 80 char lines for lines with
a function where the function name isn't understood to be
a logging use.
uspip_dbg_ doesn't take the general form so if the
string at the EOL exceeds 80 chars, a message is emitted.
Long standalone strings on a single line do not get warnings.
--
To unsubscribe from this list: send the line "unsubscribe 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: medtronic usb productId 0x8001: usbserial support, xhci enumeration
On Tue, Mar 18, 2014 at 1:10 AM, Johan Hovold wrote: > On Mon, Mar 17, 2014 at 11:58:57PM -0700, Benjamin West wrote: >> On Mon, Mar 17, 2014 at 11:40 AM, Johan Hovold wrote: >> >> issues. It's included below >> >> https://gist.github.com/bewest/6488955#file-lsusb >> >> > I'm responding to this mail with a patch for you to test. The patch is >> > also available here: >> > >> > >> > https://git.kernel.org/cgit/linux/kernel/git/johan/usb-serial.git/log/?h=carelink >> > >> > as a commit on top of usb-next. It's quite likely that the problem >> > you've been having has already been fixed (in v3.14-rc) so I suggest >> > just testing the carelink branch above. Remember to enable the >> > usb-serial-simple driver. Just speculating, git log -SNEW_SCHEME yields 48fc7dbd52c0559647291f33a10ccdc6cdbe4c72, it looks similar to the other interesting patches. >> Thanks very much for the patch; I tested your branch with my Carelink >> stick and it works sometimes. >> >> Here's an example of it working as expected: > That's good. Then the patch is correct (and should work for usb 2.0). > Thanks for testing. > >> And I was able to use the created device. >> However, most of the time, I get this: >> >> Mar 17 23:19:38 patient logger: Linux patient >> 3.14.0-rc6-bewest-test-carelink-01 #5 SMP Mon Mar 17 15:32:48 PDT 2014 >> x86_64 x86_64 x86_64 GNU/Linux > Just to make sure this isn't a new regression in usb-next you're > hitting, can you try applying the patch directly to v3.14-rc7? > > Thanks, > Johan Some mixed results: I tested your carelink branch many more times. For some reason, it started enumerating consistently for awhile, failed once, and then enumerated consistently until I gave up. I cherry-picked your e2c7df19e2734f5d54d0d942a57d1018539e02c4 on 3.14-rc7, which applied cleanly. Your changes work as expected here. The carelink usb stick did enumerate once or twice, here is a log: https://gist.github.com/bewest/6488955#file-example-3-14-0-rc7-working-log I still often get failure to enumerate after inserting the usb stick: I didn't keep an exact count, but this feels like a more consistent failure somehow. Recorded here: https://gist.githubusercontent.com/bewest/6488955/raw/213a79af21735e8822e00d8afa05abd63ffd67ee/syslog.log Mar 20 00:22:34 patient logger: Linux patient 3.14.0-rc7-bewest-test-3.14-rc7-carelink-01 #6 SMP Wed Mar 19 21:12:24 PDT 2014 x86_64 x86_64 x86_64 GNU/Linux Mar 20 00:22:40 patient kernel: [ 615.054659] usb 3-2: new full-speed USB device number 41 using xhci_hcd Mar 20 00:22:45 patient kernel: [ 620.167319] xhci_hcd :00:14.0: Timeout while waiting for setup address command Mar 20 00:22:50 patient kernel: [ 625.372047] xhci_hcd :00:14.0: Timeout while waiting for setup address command Mar 20 00:22:50 patient kernel: [ 625.576031] usb 3-2: device not accepting address 41, error -62 Mar 20 00:22:50 patient kernel: [ 625.688157] usb 3-2: new full-speed USB device number 42 using xhci_hcd Mar 20 00:23:06 patient kernel: [ 640.802272] usb 3-2: device descriptor read/64, error -110 Mar 20 00:23:06 patient kernel: [ 640.906255] xhci_hcd :00:14.0: Setup ERROR: setup context command for slot 11. Mar 20 00:23:06 patient kernel: [ 641.018244] usb 3-2: new full-speed USB device number 43 using xhci_hcd Mar 20 00:23:11 patient kernel: [ 646.01] xhci_hcd :00:14.0: Timeout while waiting for setup address command Mar 20 00:23:16 patient kernel: [ 651.223619] xhci_hcd :00:14.0: Timeout while waiting for setup address command Mar 20 00:23:16 patient kernel: [ 651.427649] usb 3-2: device not accepting address 43, error -62 Mar 20 00:23:16 patient kernel: [ 651.539702] usb 3-2: new full-speed USB device number 44 using xhci_hcd Mar 20 00:23:21 patient kernel: [ 656.540344] xhci_hcd :00:14.0: Timeout while waiting for setup address command Mar 20 00:23:27 patient kernel: [ 661.745070] xhci_hcd :00:14.0: Timeout while waiting for setup address command Mar 20 00:23:27 patient kernel: [ 661.949103] usb 3-2: device not accepting address 44, error -62 Mar 20 00:23:27 patient kernel: [ 661.949163] hub 3-0:1.0: unable to enumerate USB device on port 2 Mar 20 00:24:06 patient whoopsie[1220]: online Something else interesting happened as well. At some point, I got the following messages in the logs. After receiving these messages, I tried several different usb devices, but they all fail to enumerate after this. Sarah's branch seemed to enumerate consistently, but hard to quantify; I'll bang on it a bit more, and try to keep track of error vs success rates. -bewest P. S. I wasn't doing my usual log capture routine, but I believe I saw these messages, after which no log messages appear, even when inserting/removing other usb devices. https://gist.github.com/bewest/6488955#file-broken-usb-stack-log Linux patient 3.14.0-rc7-bewest-test-3.14-rc7-carelink-01 #6 SMP Wed Mar 19 21:12:24 PDT 2014 x86_64 x86_64 x86_64 GNU/Linux Mar 20 08:03:16 patient whoopsie[1
Re: [PATCH net-next v4 0/2] adjust MTU as indicated by MBIM extended functional descriptor
From: Ben Chan Date: Wed, 19 Mar 2014 14:00:04 -0700 > The MBIM extended functional descriptor, defined in "Universal Serial Bus > Communications Class Subclass Specification for Mobile Broadband Interface > Model, Revision 1.0, Errata-1" by USB-IF, indicates the operator preferred MTU > value via a wMTU field. > > This patch set ensures that the initial MTU value set by cdc_ncm on a MBIM net > device does not exceed the wMTU value, provided the MBIM device exposes a MBIM > extended functional descriptor. > > * Changelog > v2: Fixed a le16_to_cpu conversion issue in patch 2/2 pointed out by > Bjørn Mork > v3: No code changes. Resubmitted to include patch 1/2 as suggested by > David Miller > v4: No code changes. Resubmitted as suggested by David Miller: > - Added a summary of the patch set > - Carried the ACK from Greg Kroah-Hartman > - Added a specified the tree (net-next) to apply the patch set to Series applied to net-next, thanks Ben. -- To unsubscribe from this list: send the line "unsubscribe 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_hcd FAILS after one scan
On Wed, Mar 19, 2014 at 11:37:40AM +0100, Fiorenzo Zanotti wrote: > Hi, > I'm running an Ubuntu 13.10 32 bit desktop on a PC with ASROCK Z87 PRO4 > motherboard and I5-4670k cpu. > > Having Intel USB3 mode set to ENABLED or SMART or AUTO , a canon scanner > D660U attached to one of the usb2 port use xhci_hcd module. > > I can scan a Document right once , then I got an read error (timeout) on > the first bulk_write. > > Setting from UEFI Configuration Intel USB3 mode to DISABLED , Linux use > ehci module and everything work well. > > I don't know if this is the right list to ask , if so, hope this help you > to find a fix. This is a known issue. We have it tracked in our bug tracker, and Mathias is working on a fix on top of the global command queue patches to address this issue. Mathias will contact you when he gets a fix in place. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe 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: status of the xHCI clear halt bug?
On Sun, Mar 16, 2014 at 07:52:30PM +0100, Martin Dauskardt wrote: > Hi Sarah, > > I lost track on this issue. Is there a final patch? Is it ready for > integration into the kernel? Is there any kernel version that > includes a fix? Is there any timeline? Mathias was working on this fix, but wanted to address some issues in the global command queue patches before starting on this task. Mathias is taking over as xHCI maintainer after 3.15, so he can better provide a timeline for when it will get fixed. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe 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: usbnet: driver_info->stop required to stop USB interrupts?
On Thu, Mar 20, 2014 at 9:35 AM, Grant Grundler wrote:
> On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum wrote:
> ...
>> I have an idea. Could you test this patch?
> ...
>> - if (dev->wait) {
> ..
>> + if (waitqueue_active(&dev->wait)) {
>
> Yes - building new image now (and transfer to USB and boot from USB).
> Should know in an hour or so (doing other things in parallel).
Sorry...took a bit longer since my previous test method (bash
/tmp/reload_asix) was abusing a security exploit that is now
fixed...so had to move my script into a RO executable file system:
for i in `seq 1`; do echo -n "RELOAD $i " ; ssh $T
"/bin/reload_asix eth0 1000_full" ; J=$? ; if [ $J -eq 255 ] ; then
echo; " SSH timeout" ; break ; fi ; ssh $T "cat
/var/log/reload-asix.out" ; if [ $J -ne 0 ] ; then echo " ERROR $J" ;
fi ; sleep 3 ; done | tee ~/reload-AX88178-leon-192.168.1.100-06.out
This is running now and things look happy so far. :) This will take
more than 30h to complete.
So please add:
"Tested-by: Grant Grundler "
> Can you please explain why we need to check if the waitqueue is active?
and add a comment that answers the above question.
thanks!
grant
--
To unsubscribe from this list: send the line "unsubscribe 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_hcd] reset SuperSpeed, xhci_drop_endpoint called with disabled ep, Error in queuecommand_lck: task blocked
On Wed, Mar 19, 2014 at 12:37:37PM +0100, Andreas Reis wrote: > I've uploaded a dmesg with the new debugging patch to bugzilla: > https://bugzilla.kernel.org/attachment.cgi?id=130041 > > Andreas Reis > > On 18.03.2014 15:57, Alan Stern wrote:> > > The debugging information didn't go far enough. Try the patch below > > instead, which has some additional messages. > > > > There are two apparently separate problems here. First, why was the > > reset necessary? As far as I can tell, the only explanation seems to > > be a failure of Link Power Management. > > > > Second, why the errors in queuecommand_lck? I suspect the answer to > > that lies somewhere in the SCSI subystem, not USB. The new debugging > > patch should tell us for sure. Mathias, you might want to come up with a patch to disable link PM and see if it helps in this case. That could be as simple as not setting XHCI_INTEL_HOST for this PCI device ID, or could be as complex as exposing a sysfs file similar to usb2_hardware_lpm. It could also have something to do with how we set the U1/U2 timeouts for this particular device. Exposing a file to play with those settings may be interesting. I'll let you handle triaging the xHCI bug reports, but please let me know if you get stuck. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe 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] Add AsantePorter PID 0x7F38 to FTDI list
Johan and other usb-serial/ftdi maintainers,
Here is a small patch for you which adds the PID for Asante Snap
Pump's FTDI data cable to the device list. The Asante Snap pump is an
insulin pump, this device is a usb cable which connects to their
cradle.
The patch is listed below as well as here:
https://gist.github.com/bewest/9673924
Here is lsusb output from the device:
https://raw.githubusercontent.com/tidepool-org/asante-porter/master/lsusb
+ lsusb -d 0403:7f38 -v
Bus 002 Device 012: ID 0403:7f38 Future Technology Devices International, Ltd
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x0403 Future Technology Devices International, Ltd
idProduct 0x7f38
bcdDevice6.00
iManufacturer 1 FTDI
iProduct2 TTL-232R-3V3-AJ
iSerial 3 FTX1MKWH
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 32
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0x80
(Bus Powered)
MaxPower 90mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber0
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass255 Vendor Specific Subclass
bInterfaceProtocol255 Vendor Specific Protocol
iInterface 2 TTL-232R-3V3-AJ
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Device Status: 0x
(Bus Powered)
-bewest
Ben West
>From 82bf3dcd649af33d8f41aa2098bd61bfa94cbc04 Mon Sep 17 00:00:00 2001
From: Ben West
Date: Wed, 19 Mar 2014 20:34:33 -0700
Subject: [PATCH] Add AsantePorter PID 0x7F38 to FTDI list
This patch allows the ftdi usb serial module to create a device when
an Asante Porter usb data cable is plugged in.
Signed-off-by: Ben West
---
drivers/usb/serial/ftdi_sio.c | 1 +
drivers/usb/serial/ftdi_sio_ids.h | 5 +
2 files changed, 6 insertions(+)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 44ab129..7f26fa1 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -688,6 +688,7 @@ static const struct usb_device_id id_table_combined[] = {
{ USB_DEVICE(EVOLUTION_VID, EVO_HYBRID_PID) },
{ USB_DEVICE(EVOLUTION_VID, EVO_RCM4_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_ARTEMIS_PID) },
+ { USB_DEVICE(FTDI_VID, FTDI_ASANTE_PORTER_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_ATIK_ATK16_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_ATIK_ATK16C_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_ATIK_ATK16HR_PID) },
diff --git a/drivers/usb/serial/ftdi_sio_ids.h
b/drivers/usb/serial/ftdi_sio_ids.h
index e599fbf..0602fd6 100644
--- a/drivers/usb/serial/ftdi_sio_ids.h
+++ b/drivers/usb/serial/ftdi_sio_ids.h
@@ -194,6 +194,11 @@
#define FTDI_ARTEMIS_PID 0xDF28 /* All Artemis Cameras */
/*
+ * Definitions for Asante Porter, data cable for insulin pump.
+ */
+#define FTDI_ASANTE_PORTER_PID 0x7F38 /* Asante Porter */
+
+/*
* Definitions for ATIK Instruments astronomical USB based cameras
* Check it at http://www.atik-instruments.com/
*/
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] coding style: fix line over 80 characters
On 03/20/2014 11:45 PM, Joe Perches wrote:
diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
[]
@@ -271,12 +271,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16
typeReq, u16 wValue,
}
break;
case USB_PORT_FEAT_POWER:
- usbip_dbg_vhci_rh(" ClearPortFeature:
USB_PORT_FEAT_POWER\n");
+ usbip_dbg_vhci_rh(
+ " ClearPortFeature: USB_PORT_FEAT_POWER\n");
Your version of scripts/checkpatch.pl seems outdated. It shouldn't
complain about strings violating 80-column limit (and I've just verified it
doesn't).
checkpatch complains about > 80 char lines for lines with
a function where the function name isn't understood to be
a logging use.
uspip_dbg_ doesn't take the general form so if the
string at the EOL exceeds 80 chars, a message is emitted.
Long standalone strings on a single line do not get warnings.
OK, but there's still issue that the patch context doesn't correspond to
what can be seen in Greg's tree. What I am seeing there is broken up string.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 03/16] usb: cleanup setting udev->removable from port_dev->connect_type
Ok, so it looks like Alan finally acked all the port power off patches.
Greg, how are you feeling about taking these for 3.15? I hear that
Linus may have to release an -rc8 or even -rc9, so they could go in and
still have some time to marinate in linux-next, but it's really your
call.
I usually wouldn't accept patches this late, but Alan's done a good job
of ferreting out all the corner cases, so I have high confidence the
patches will work. They're also highly unlikely to have any impact on
systems that don't have the port power off mechanism.
It's up to you. Either way, I'll compile a kernel with these patches
for my non-Intel host machine (NEC 0.96 host) and make sure they don't
break anything.
Sarah Sharp
On Thu, Mar 20, 2014 at 09:21:56AM -0700, Dan Williams wrote:
> On Thu, 2014-03-20 at 11:58 -0400, Alan Stern wrote:
> > On Wed, 19 Mar 2014, Dan Williams wrote:
> >
> > > Once usb-acpi has set the port's connect type the usb_device's
> > > ->removable attribute can be set in the standard location
> > > set_usb_port_removable().
> > >
> > > This also changes behavior in the case where the firmware says that the
> > > port connect type is unknown. In that case just use the default setting
> > > determined from the hub descriptor.
> > >
> > > Note, we no longer pass udev->portnum to acpi_find_child_device() in the
> > > root hub case since:
> > > 1/ the usb-core sets this to zero
> > > 2/ acpi always expects zero
> > > ...just pass zero.
> > >
> > > Suggested-by: Alan Stern
> > > Signed-off-by: Dan Williams
> >
> > This is the one patch I haven't ACKed yet from among the first ten.
> >
> > > @@ -155,40 +155,18 @@ static struct acpi_device
> > > *usb_acpi_find_companion(struct device *dev)
> > >*/
> > > if (is_usb_device(dev)) {
> > > udev = to_usb_device(dev);
> > > - port1 = udev->portnum;
> > > - if (udev->parent) {
> > > - struct usb_hub *hub;
> > > -
> > > - hub = usb_hub_to_struct_hub(udev->parent);
> > > - /*
> > > - * According usb port's connect type to set usb device's
> > > - * removability.
> > > - */
> > > - switch (hub->ports[port1 - 1]->connect_type) {
> > > - case USB_PORT_CONNECT_TYPE_HOT_PLUG:
> > > - udev->removable = USB_DEVICE_REMOVABLE;
> > > - break;
> > > - case USB_PORT_CONNECT_TYPE_HARD_WIRED:
> > > - udev->removable = USB_DEVICE_FIXED;
> > > - break;
> > > - default:
> > > - udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
> > > - break;
> > > - }
> > > -
> > > + if (udev->parent)
> > > return NULL;
> > > - }
> >
> > We should try to find the device's ACPI companion here. That can be
> > added in a later patch, though.
> >
> > >
> > > - /* root hub's parent is the usb hcd. */
> > > - return acpi_find_child_device(ACPI_COMPANION(dev->parent),
> > > - port1, false);
> > > + /* root hub is only child (_ADR=0) under it's parent, the hcd */
> >
> > s/it's/its/
> >
> > Remember, "it's" is a contraction of "it is".
>
> Not sure that extra clarification was worth the keystrokes, but typo
> fixed.
>
> >
> > s/the hcd/the HC/
> >
> > "hcd" refers either to a data structure (struct usb_hcd) or a Host
> > Controller Driver. But here you are referring to the host controller
> > itself, or the ACPI device corresponding to it.
>
> ok
>
> >
> > > + adev = ACPI_COMPANION(dev->parent);
> > > + return acpi_find_child_device(adev, 0, false);
> > > } else if (is_usb_port(dev)) {
> > > struct usb_port *port_dev = to_usb_port(dev);
> > > - struct acpi_device *adev = NULL;
> > > + int port1 = port_dev->portnum;
> > >
> > > /* Get the struct usb_device point of port's hub */
> > > udev = to_usb_device(dev->parent->parent);
> > > - port1 = port_dev->portnum;
> > >
> > > /*
> > >* The root hub ports' parent is the root hub. The non-root-hub
> >
> > The rest of this comment says "The non-root-hub ports' parent is the
> > parent hub port which the hub is connected to." I'm not sure that's
> > really true.
> >
> > In the ACPI code example shown in section D.1.1 of the xHCI spec, the
> > device path for port IP1 (which is a port on the integrated hub) is
> > USB0.RHUB.HCP2.IHUB.IP1. Thus, IP1's parent is the integrated hub,
> > IHUB, not the hub's upstream port, HCP2.
> >
> > This also should be fixed later.
>
> Indeed, I was overlooking the correctness of the existing
> implementation. It seems it does need more revision.
>
> > For now, after you fix the spelling
> > errors above, you can add
> >
> > Acked-by: Alan Stern
> >
>
> Thanks
Re: [PATCH v7 03/16] usb: cleanup setting udev->removable from port_dev->connect_type
On Thu, Mar 20, 2014 at 02:33:33PM -0700, Sarah Sharp wrote: > Ok, so it looks like Alan finally acked all the port power off patches. > > Greg, how are you feeling about taking these for 3.15? I hear that > Linus may have to release an -rc8 or even -rc9, so they could go in and > still have some time to marinate in linux-next, but it's really your > call. I haven't even reviewed them as I didn't think they were ready yet. And no, it's to late for 3.15, my tree is closed, sorry. For 3.16 it's better, I want a lot of testing for these. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 03/16] usb: cleanup setting udev->removable from port_dev->connect_type
On Thu, Mar 20, 2014 at 02:52:25PM -0700, Greg KH wrote: > On Thu, Mar 20, 2014 at 02:33:33PM -0700, Sarah Sharp wrote: > > Ok, so it looks like Alan finally acked all the port power off patches. > > > > Greg, how are you feeling about taking these for 3.15? I hear that > > Linus may have to release an -rc8 or even -rc9, so they could go in and > > still have some time to marinate in linux-next, but it's really your > > call. > > I haven't even reviewed them as I didn't think they were ready yet. > > And no, it's to late for 3.15, my tree is closed, sorry. > > For 3.16 it's better, I want a lot of testing for these. Ok, makes sense. Mathias will be in charge of queuing these then. That reminds me, I need to send you a patch to change the maintainership of the driver over... Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe 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: usbnet: driver_info->stop required to stop USB interrupts?
>> Can you please explain why we need to check if the waitqueue is active? > > and add a comment that answers the above question. Oh the braces!!! Well, that's just mean... I expect that we don't really need the waitqueue_active() check there as long as we fix the patch to make sure the control flow in the rest of the statements actually stays the same. (That's why I really like to put comments for an else block next to or under the opening brace, instead of above with another empty line...) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] coding style: fix line over 80 characters
On 03/21/2014 12:34 AM, Sergei Shtylyov wrote:
Signed-off-by: Cédric Cabessa
---
drivers/staging/usbip/vhci_hcd.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/usbip/vhci_hcd.c
b/drivers/staging/usbip/vhci_hcd.c
index f690668..1e84577 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
[...]
@@ -539,7 +546,9 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct
urb *urb,
default:
/* NOT REACHED */
-dev_err(dev, "invalid request to devnum 0 bRequest %u, wValue
%u\n", ctrlreq->bRequest,
+dev_err(dev,
+"invalid request to devnum 0 bRequest %u, wValue %u\n",
+ctrlreq->bRequest,
ctrlreq->wValue);
ret = -EINVAL;
goto no_need_xmit;
@@ -1060,7 +1069,9 @@ static int vhci_hcd_suspend(struct platform_device
*pdev, pm_message_t state)
spin_unlock(&the_controller->lock);
if (connected > 0) {
-dev_info(&pdev->dev, "We have %d active connection%s. Do not
suspend.\n", connected, (connected == 1 ? "" : "s"));
+dev_info(&pdev->dev,
+ "We have %d active connection%s. Do not suspend.\n",
+ connected, (connected == 1 ? "" : "s"));
ret = -EBUSY;
} else {
dev_info(&pdev->dev, "suspend vhci_hcd");
Hm, I don't see checkpatch.pl complaints about these either in the
'usb-next' branch of Greg's tree.
Sorry, I forgot about Greg's separate staging.git repo. The patch looks
correct in this context.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What to use for usb transfer from gadget to host?
On Thu, Mar 20, 2014 at 10:11:42PM +0800, tzi...@me.com wrote: > Hello! > > I am developing on a phyFlex iMx.6 and want to send data via USB to a > Host connected via the OTG port. > The data is images from an image sensor, so I need a high performance (USB > 2.0). How is your image sensor driver implemented? If it is in v4l2 framework, then you can send your data to USB host using uvc framework. See: drivers/usb/gadget/webcam.c The above would be be the standard way of doing such application. Regards Pratyush > I am familiar with USB on the host side, but not the gadget side. > What's the best/easiest way to implement a simple 2 endpoints > communication with high data rates? > During my search I came across functionfs, gadgetfs, configfs and > libusbg. Can you recommend one of them that suits my needs? > Thank you so much! > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: Re: Support for new USB Modem 4G drive
Hello Bjørn,
I made the test like you suggested and I couldn't find any difference
between the printout for lsusb before and after the command.
What else I've been tried after read the page
http://www.draisberghof.de/usb_modeswitch/ was:
1 - For the file /etc/usb_modeswitch.conf I changed the EnableLogging to
1 to create a log file in /var/log, the file was created with the follow
name: /var/log/usb_modeswitch_1-1.3
2 - In the file /lib/udev/rules.d/40-usb_modeswitch.rules I copied the
information regarding to a device from the same vendor but with another
idProduct to my idProduct, so it was like this:
Information that I use as reference
# Olivetti Olicard 145, 155
ATTR{idVendor}=="0b3c", ATTR{idProduct}=="f000", RUN+="usb_modeswitch
'%b/%k'"
This lines was what I inserted in the file
# Olivetti Olicard 500, test only
ATTR{idVendor}=="0b3c", ATTR{idProduct}=="f017", RUN+="usb_modeswitch
'%b/%k'"
But it does not work, and shows the following information in the file
/var/log/usb_modeswitch_1-1.3
USB values from sysfs:
manufacturer USBModem
product Olicard 500
serial
bNumConfigurations is 1 - don't check for active configuration
Found packed config collection /usr/share/usb_modeswitch/configPack.tar.gz
Aargh! Config file missing for 0b3c:f017! Exit
3 - In the file /usr/share/usb_modeswitch/configPack.tar.gz I found the
configuration information regarding to a idVendor and idProduct like a
did in the file 40-usb_modeswitch.rules and I copy to a new file with
name 0b3c:f017 with the same information from the file 0d3c:f000.
Original file 0b3c:f000
julio@mynote:~/Documentos/Modem4G-Olicard500/tmp$ more 0b3c:f000
# Olivetti Olicard 145, 155
TargetVendor= 0x0b3c
TargetProductList="c003,c004"
MessageContent="5553424312345678c00080010606f504025270"
NeedResponse=1
New file 0b3c:f017 that I created:
# Olivetti Olicard 500, test only
TargetVendor= 0x0b3c
TargetProductList="c500"
MessageContent="5553424312345678c00080010606f504025270"
NeedResponse=1
As you can see I used the same MessageContent, but it does not work.
Is there anything else that I could try?
I'm not an expert but in my opinion my device is not recognizing the
message that we sent in the command usb_modeswitch, so is there a way to
figure out what should be the information that we should use to send the
message to the device?
Thanks,
Julio Araujo
Em 20-03-2014 13:02, Bjørn Mork escreveu:
Dan Williams writes:
From the information below, it appears the modem is not being switched
correctly to modem mode from storage mode. The tool that does that is
usb_modeswitch, but usb_modeswitch does not have specific support for
your new device.
So the next steps are for you to contact the usb_modeswitch project and
work with them to get the modem successfully switched from storage mode
to modem mode.
http://www.draisberghof.de/usb_modeswitch/
Hello, did you have a chance to look into this yet?
As Dan says, this is a mode switch problem initially. We will look
further at the drivers after the device has been successfully switched
to modem mode.
The device isn't yet known in the usb_modeswitch database. But chances
are good that it is similar to other, already known, Olivetti devices.
You can try installing the usb-modeswitch package from Ubuntu and a
config similar to the "Olivetti Olicard 145, 155" (0b3c:f000) one:
# Olivetti Olicard 145, 155
TargetVendor= 0x0b3c
TargetProductList="c003,c004"
MessageContent="5553424312345678c00080010606f504025270"
NeedResponse=1
To test, try something like this on the command line:
sudo usb_modeswitch -V 0b3c -P f017 -M
5553424312345678c00080010606f504025270
and see what happens. If it switches identities, then pleast rerun the
"lsusb -v -d ..." command using the new device ID.
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] USB: Gadget: fsl driver pullup fix
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Thursday, March 20, 2014 9:33 PM > To: Gupta Suresh-B42813 > Cc: ba...@ti.com; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH v2] USB: Gadget: fsl driver pullup fix > > On Fri, Mar 21, 2014 at 01:57:18AM +0530, Suresh Gupta wrote: > > This fix the fsl usb gadget driver in a way that the usb device will > > be only "pulled up" on requests only when vbus is powered > > > > Signed-off-by: Suresh Gupta > > --- > > Changes from previous version: > > * Removed re-factored code, Will send another patch for re-factoring > > duplicated code > > * Changed Description > > > > drivers/usb/gadget/fsl_udc_core.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/usb/gadget/fsl_udc_core.c > > b/drivers/usb/gadget/fsl_udc_core.c > > index 35cb972..49d66779 100644 > > --- a/drivers/usb/gadget/fsl_udc_core.c > > +++ b/drivers/usb/gadget/fsl_udc_core.c > > @@ -1219,6 +1219,10 @@ static int fsl_pullup(struct usb_gadget *gadget, > int is_on) > > struct fsl_udc *udc; > > > > udc = container_of(gadget, struct fsl_udc, gadget); > > + > > + if(!udc->vbus_active) > > + return -EOPNOTSUPP; > > Always run your patches through scripts/checkpatch.pl so they don't get > rejected for silly things like the wrong coding style... > Accepted, Sorry for such a inane mistake -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: Gadget: fsl driver pullup fix
This fix the fsl usb gadget driver in a way that the usb device will be only "pulled up" on requests only when vbus is powered Signed-off-by: Suresh Gupta --- Changes from previous version: * fixed checkpatch error drivers/usb/gadget/fsl_udc_core.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c index 35cb972..5a0f89c 100644 --- a/drivers/usb/gadget/fsl_udc_core.c +++ b/drivers/usb/gadget/fsl_udc_core.c @@ -1219,6 +1219,10 @@ static int fsl_pullup(struct usb_gadget *gadget, int is_on) struct fsl_udc *udc; udc = container_of(gadget, struct fsl_udc, gadget); + + if (!udc->vbus_active) + return -EOPNOTSUPP; + udc->softconnect = (is_on != 0); if (can_pullup(udc)) fsl_writel((fsl_readl(&dr_regs->usbcmd) | USB_CMD_RUN_STOP), -- 1.8.4.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: Fwd: Re: Support for new USB Modem 4G drive
On 2014-03-21 11:33, Julio Araujo wrote:
Hello Bjørn,
I made the test like you suggested and I couldn't find any difference
between the printout for lsusb before and after the command.
What else I've been tried after read the page
http://www.draisberghof.de/usb_modeswitch/ was:
1 - For the file /etc/usb_modeswitch.conf I changed the EnableLogging to
1 to create a log file in /var/log, the file was created with the follow
name: /var/log/usb_modeswitch_1-1.3
2 - In the file /lib/udev/rules.d/40-usb_modeswitch.rules I copied the
information regarding to a device from the same vendor but with another
idProduct to my idProduct, so it was like this:
Information that I use as reference
# Olivetti Olicard 145, 155
ATTR{idVendor}=="0b3c", ATTR{idProduct}=="f000", RUN+="usb_modeswitch
'%b/%k'"
This lines was what I inserted in the file
# Olivetti Olicard 500, test only
ATTR{idVendor}=="0b3c", ATTR{idProduct}=="f017", RUN+="usb_modeswitch
'%b/%k'"
But it does not work, and shows the following information in the file
/var/log/usb_modeswitch_1-1.3
USB values from sysfs:
manufacturerUSBModem
productOlicard 500
serial
bNumConfigurations is 1 - don't check for active configuration
Found packed config collection
/usr/share/usb_modeswitch/configPack.tar.gz
Aargh! Config file missing for 0b3c:f017! Exit
3 - In the file /usr/share/usb_modeswitch/configPack.tar.gz I found the
configuration information regarding to a idVendor and idProduct like a
did in the file 40-usb_modeswitch.rules and I copy to a new file with
name 0b3c:f017 with the same information from the file 0d3c:f000.
Original file 0b3c:f000
julio@mynote:~/Documentos/Modem4G-Olicard500/tmp$ more 0b3c:f000
# Olivetti Olicard 145, 155
TargetVendor= 0x0b3c
TargetProductList="c003,c004"
MessageContent="5553424312345678c00080010606f504025270"
NeedResponse=1
New file 0b3c:f017 that I created:
# Olivetti Olicard 500, test only
TargetVendor= 0x0b3c
TargetProductList="c500"
MessageContent="5553424312345678c00080010606f504025270"
NeedResponse=1
As you can see I used the same MessageContent, but it does not work.
Is there anything else that I could try?
I'm not an expert but in my opinion my device is not recognizing the
message that we sent in the command usb_modeswitch, so is there a way to
figure out what should be the information that we should use to send the
message to the device?
Thanks,
Julio Araujo
Em 20-03-2014 13:02, Bjørn Mork escreveu:
Dan Williams writes:
From the information below, it appears the modem is not being switched
correctly to modem mode from storage mode. The tool that does that is
usb_modeswitch, but usb_modeswitch does not have specific support for
your new device.
So the next steps are for you to contact the usb_modeswitch project and
work with them to get the modem successfully switched from storage mode
to modem mode.
http://www.draisberghof.de/usb_modeswitch/
Hello, did you have a chance to look into this yet?
As Dan says, this is a mode switch problem initially. We will look
further at the drivers after the device has been successfully switched
to modem mode.
The device isn't yet known in the usb_modeswitch database. But chances
are good that it is similar to other, already known, Olivetti devices.
You can try installing the usb-modeswitch package from Ubuntu and a
config similar to the "Olivetti Olicard 145, 155" (0b3c:f000) one:
# Olivetti Olicard 145, 155
TargetVendor= 0x0b3c
TargetProductList="c003,c004"
MessageContent="5553424312345678c00080010606f504025270"
NeedResponse=1
To test, try something like this on the command line:
sudo usb_modeswitch -V 0b3c -P f017 -M
5553424312345678c00080010606f504025270
and see what happens. If it switches identities, then pleast rerun the
"lsusb -v -d ..." command using the new device ID.
Bjørn
Those usb_modeswitch cmd line switches are not the correct ones so will
not work.
The linux_usb mailing list is not the right place for getting the device
supported for mode switching so I suggest you follow Dan Williams
advice and get over to the usb_modeswitch forum where Josh or I will try
to help you switching the device into modem mode.
I'm sure Björn will help you here later on by creating patches for the
linux drivers when we know which type of interfaces the device has after
mode switching.
http://www.draisberghof.de/usb_modeswitch/bb/viewforum.php?f=3 is the
right place
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
