[PATCH v2] ehci-hcd: Cleanup memory resources when ehci_halt fails
The driver calls ehci_mem_init to allocate memory resources.
But these resources are not freed when ehci_halt fails.
This patch adds "ehci_mem_cleanup" in error handling code to fix this problem.
Signed-off-by: Jia-Ju Bai
---
drivers/usb/host/ehci-hcd.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 48c92bf..015b411 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -675,8 +675,10 @@ int ehci_setup(struct usb_hcd *hcd)
return retval;
retval = ehci_halt(ehci);
- if (retval)
+ if (retval) {
+ ehci_mem_cleanup(ehci);
return retval;
+ }
ehci_reset(ehci);
--
1.7.9.5
--
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] ehci-hcd: Disable memory-write-invalidate when the driver is removed
The driver calls pci_set_mwi to enable memory-write-invalidate when it
is initialized, but does not call pci_clear_mwi when it is removed. Many
other drivers calls pci_clear_mwi when pci_set_mwi is called, such as
r8169, 8139cp and e1000.
This patch adds a function "ehci_pci_remove" to remove the pci driver.
This function calls pci_clear_mwi and usb_hcd_pci_remove, which can
fix the problem.
Signed-off-by: Jia-Ju Bai
---
drivers/usb/host/ehci-pci.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 2a5d2fd..3b3649d 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -377,6 +377,12 @@ static int ehci_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *id)
return usb_hcd_pci_probe(pdev, id);
}
+static void ehci_pci_remove(struct pci_dev *pdev)
+{
+ pci_clear_mwi(pdev);
+ usb_hcd_pci_remove(pdev);
+}
+
/* PCI driver selection metadata; PCI hotplugging uses this */
static const struct pci_device_id pci_ids [] = { {
/* handle any USB 2.0 EHCI controller */
@@ -396,7 +402,7 @@ static struct pci_driver ehci_pci_driver = {
.id_table = pci_ids,
.probe =ehci_pci_probe,
- .remove = usb_hcd_pci_remove,
+ .remove = ehci_pci_remove,
.shutdown = usb_hcd_pci_shutdown,
#ifdef CONFIG_PM
--
1.7.9.5
--
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: serial: return -ENODEV if disconnected during TIOCMIWAIT
On Sun, 2016-01-03 at 17:22 +0100, Johan Hovold wrote: > On Mon, Dec 21, 2015 at 02:17:22PM +0100, Oliver Neukum wrote: > > Disconnecting a device is not just a hang up. The device is gone. > > We should tell user space the truth immediately. > > I'm not sure about this one. The usb-serial drivers have always returned > -EIO on hangup, whatever the reason, and there's nothing wrong with > that implementation. > > Also note that any further ioctls after returning will also fail with > -EIO. > > I think it makes sense to just stick to -EIO here. But write() is happy to return -ENODEV. I'd say that -EIO is the error return of last resort. If we have anything specific to report we ought to use it. And if a device is gone, it is gone. There is no use hiding it. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: serial: return -ENODEV if disconnected during TIOCMIWAIT
On Mon, Jan 04, 2016 at 09:09:55AM +0100, Oliver Neukum wrote: > On Sun, 2016-01-03 at 17:22 +0100, Johan Hovold wrote: > > On Mon, Dec 21, 2015 at 02:17:22PM +0100, Oliver Neukum wrote: > > > Disconnecting a device is not just a hang up. The device is gone. > > > We should tell user space the truth immediately. > > > > I'm not sure about this one. The usb-serial drivers have always returned > > -EIO on hangup, whatever the reason, and there's nothing wrong with > > that implementation. > > > > Also note that any further ioctls after returning will also fail with > > -EIO. > > > > I think it makes sense to just stick to -EIO here. > > But write() is happy to return -ENODEV. A disconnect during write() typically also sets errno to EIO (or isn't detected at all if the buffer is small enough). Consecutive calls also fail with -EIO. We always return -ENODEV for usb-serial devices that do not have a bulk out end-point though, but then the errno does not imply that the device is gone but rather that the operation is not supported by the device. You may also be "lucky" and have write return -ENODEV in case you get a disconnect that has not been fully processed just before a first write (that calls usb_submit_urb instead of cache the data). But this would be the exception and not something that userspace should rely on. And if the timing is slightly altered, the tty layer will again set errno to -EIO. > I'd say that -EIO is the error return of last resort. If we have > anything specific to report we ought to use it. And if a device is > gone, it is gone. There is no use hiding it. What errno is set to for a failing ioctl is entirely up to the driver, and all tty drivers (but recently cdc-acm) returns -EIO on hangup, which is perfectly fine. Now, we could extend this long-since established behaviour, but I'm not sure it's worth it given that any further calls using the hung up tty device will also return with errno set to -EIO by the tty layer. And this we can't easily change. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: serial: return -ENODEV if disconnected during TIOCMIWAIT
On Mon, 2016-01-04 at 12:06 +0100, Johan Hovold wrote: > Now, we could extend this long-since established behaviour, but I'm > not > sure it's worth it given that any further calls using the hung up tty > device will also return with errno set to -EIO by the tty layer. And > this we can't easily change. That is a valid point. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ftdi_sio bug: setting two custom baud rates in a row fails
On Sun, Jan 03, 2016 at 06:11:07PM -0500, Edwin Olson wrote: > Hi Johan et al, > > Now that I understand the behavior, and know how to work-around the > issue, I won't press too hard for functionality changes. But perhaps > the documentation can be improved by suggesting that the custom > divisor be set before the baud rate is set to B38400. E.g., by just > swapping the order of the instructions below. That's a good idea. I'll also add a comment and warning message about this being deprecated. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 RESEND] usb: Use memdup_user to reuse the code
On Wed, Dec 30, 2015 at 09:15:55AM +, Pathak, Rahul (R.) wrote: > Hello Greg, > > This patch is not yet merged, its already been reviewed and acked by Alan. > Relax. Everyone is on vacation. This stuff is not life or death that it must be done right away. regards, dan carpenter -- 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] ehci-hcd: Cleanup memory resources when ehci_halt fails
On Mon, 4 Jan 2016, Jia-Ju Bai wrote:
> The driver calls ehci_mem_init to allocate memory resources.
> But these resources are not freed when ehci_halt fails.
>
> This patch adds "ehci_mem_cleanup" in error handling code to fix this problem.
>
> Signed-off-by: Jia-Ju Bai
> ---
> drivers/usb/host/ehci-hcd.c |4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 48c92bf..015b411 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -675,8 +675,10 @@ int ehci_setup(struct usb_hcd *hcd)
> return retval;
>
> retval = ehci_halt(ehci);
> - if (retval)
> + if (retval) {
> + ehci_mem_cleanup(ehci);
> return retval;
> + }
>
> ehci_reset(ehci);
>
Acked-by: 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] ehci-hcd: Disable memory-write-invalidate when the driver is removed
On Mon, 4 Jan 2016, Jia-Ju Bai wrote:
> The driver calls pci_set_mwi to enable memory-write-invalidate when it
> is initialized, but does not call pci_clear_mwi when it is removed. Many
> other drivers calls pci_clear_mwi when pci_set_mwi is called, such as
> r8169, 8139cp and e1000.
>
> This patch adds a function "ehci_pci_remove" to remove the pci driver.
> This function calls pci_clear_mwi and usb_hcd_pci_remove, which can
> fix the problem.
>
> Signed-off-by: Jia-Ju Bai
> ---
> drivers/usb/host/ehci-pci.c |8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 2a5d2fd..3b3649d 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -377,6 +377,12 @@ static int ehci_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
> return usb_hcd_pci_probe(pdev, id);
> }
>
> +static void ehci_pci_remove(struct pci_dev *pdev)
> +{
> + pci_clear_mwi(pdev);
> + usb_hcd_pci_remove(pdev);
> +}
> +
> /* PCI driver selection metadata; PCI hotplugging uses this */
> static const struct pci_device_id pci_ids [] = { {
> /* handle any USB 2.0 EHCI controller */
> @@ -396,7 +402,7 @@ static struct pci_driver ehci_pci_driver = {
> .id_table = pci_ids,
>
> .probe =ehci_pci_probe,
> - .remove = usb_hcd_pci_remove,
> + .remove = ehci_pci_remove,
> .shutdown = usb_hcd_pci_shutdown,
>
> #ifdef CONFIG_PM
Acked-by: 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] cdc_acm: blacklist Samsung Phones in firmware update mode
On 2016-01-04 13:43, Oliver Neukum wrote: On Mon, 2016-01-04 at 12:32 +0700, Lars Melin wrote: K 04e8:6601 is also used for Samsung SCH-U209 CDMA modem so can not be ignored. 04e8:685d is an update mode id so it can be ignored by cdc-acm. 04e8:68c3 does not look like update mode, it has cdc-acm modem and cdc-acm device management interfaces. Ok, there is trouble. And some stupid vendors. Jose, could you break up this patch intro three parts? Lars, do you have any sure information about 04e8:68c3? 04e8:6601 may need a special case in probe(), but we should discuss the series separately. I have the verbose lsusb listings for 04e8:6601 and 04e8:685d but for 04e8:68c3 only the interface descriptions from the MS Win driver. I assume that Jose who has submitted the patch knows more about the various interfaces and can provide a verbose lsusb listing in order to back up his patch. br Lars -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cdc_acm: blacklist Samsung Phones in firmware update mode
On Mon, 2016-01-04 at 23:06 +0700, Lars Melin wrote: > On 2016-01-04 13:43, Oliver Neukum wrote: > > On Mon, 2016-01-04 at 12:32 +0700, Lars Melin wrote: > K > >> > >> 04e8:6601 is also used for Samsung SCH-U209 CDMA modem so can not be > >> ignored. > >> 04e8:685d is an update mode id so it can be ignored by cdc-acm. > >> 04e8:68c3 does not look like update mode, it has cdc-acm modem and > >> cdc-acm device management interfaces. > > > > Ok, there is trouble. And some stupid vendors. Jose, could > > you break up this patch intro three parts? > > Lars, do you have any sure information about 04e8:68c3? > > 04e8:6601 may need a special case in probe(), but we should > > discuss the series separately. > > > > I have the verbose lsusb listings for 04e8:6601 and 04e8:685d but for For both 04e8:6601? > 04e8:68c3 only the interface descriptions from the MS Win driver. > I assume that Jose who has submitted the patch knows more about the > various interfaces and can provide a verbose lsusb listing in order to > back up his patch. That will be necessary. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cdc_acm: blacklist Samsung Phones in firmware update mode
On Mon, 2016-01-04 at 18:23 +0100, Oliver Neukum wrote: > On Mon, 2016-01-04 at 23:06 +0700, Lars Melin wrote: > > On 2016-01-04 13:43, Oliver Neukum wrote: > > > On Mon, 2016-01-04 at 12:32 +0700, Lars Melin wrote: > > K > > > > > > > > 04e8:6601 is also used for Samsung SCH-U209 CDMA modem so can > > > > not be > > > > ignored. > > > > 04e8:685d is an update mode id so it can be ignored by cdc-acm. > > > > 04e8:68c3 does not look like update mode, it has cdc-acm modem > > > > and > > > > cdc-acm device management interfaces. > > > > > > Ok, there is trouble. And some stupid vendors. Jose, could > > > you break up this patch intro three parts? > > > Lars, do you have any sure information about 04e8:68c3? > > > 04e8:6601 may need a special case in probe(), but we should > > > discuss the series separately. > > > > > > > I have the verbose lsusb listings for 04e8:6601 and 04e8:685d but > > for > > For both 04e8:6601? > > > 04e8:68c3 only the interface descriptions from the MS Win driver. > > I assume that Jose who has submitted the patch knows more about > > the > > various interfaces and can provide a verbose lsusb listing in order > > to > > back up his patch. > > That will be necessary. > Initially I made the patch and tested for the device 04e8:685d. For completeness, I read in the sources of Heimdall that there are two other devices used by Samsung. I had no access to these devices. I am contacting the author of Heimdall if he have more information about these devices. Jose Alonso -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: core: call init and shutdown for the usb phy
Hi,
On Fri, Dec 18, 2015 at 5:02 AM, Uwe Kleine-König
wrote:
> The phy's init routine must be called before it can be used. Do so in
> musb_init_controller and the matching shutdown in musb_remove.
>
> Signed-off-by: Uwe Kleine-König
> ---
> Note I'm not entirely sure this is the right place, but this made usb
> working on my omap3 machine here (at least somewhat, still fighting with
> the last details).
> ---
> drivers/usb/musb/musb_core.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 2c624a10748d..ebb8a5455d2f 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2108,6 +2108,10 @@ musb_init_controller(struct device *dev, int nIrq,
> void __iomem *ctrl)
>
> pm_runtime_get_sync(musb->controller);
>
> + status = usb_phy_init(musb->xceiv);
> + if (status < 0)
> + goto err_usb_phy_init;
> +
Please check the comment around Line 2000, which states
musb_platform_init() should call for phy init. So the glue driver
should handle phy init/shutdown, not the core. This patch will cause
the phy init to be called twice for those devices which already handle
phy control in the glue driver.
Regards,
-Bin.
> if (use_dma && dev->dma_mask) {
> musb->dma_controller =
> musb_dma_controller_create(musb, musb->mregs);
> @@ -,7 +2226,11 @@ fail3:
> cancel_delayed_work_sync(&musb->deassert_reset_work);
> if (musb->dma_controller)
> musb_dma_controller_destroy(musb->dma_controller);
> +
> fail2_5:
> + usb_phy_shutdown(musb->xceiv);
> +
> +err_usb_phy_init:
> pm_runtime_put_sync(musb->controller);
>
> fail2:
> @@ -2282,6 +2290,8 @@ static int musb_remove(struct platform_device *pdev)
> if (musb->dma_controller)
> musb_dma_controller_destroy(musb->dma_controller);
>
> + usb_phy_shutdown(musb->xceiv);
> +
> cancel_work_sync(&musb->irq_work);
> cancel_delayed_work_sync(&musb->finish_resume_work);
> cancel_delayed_work_sync(&musb->deassert_reset_work);
> --
> 2.6.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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: core: call init and shutdown for the usb phy
Hi, Bin Liu writes: > Hi, > > On Fri, Dec 18, 2015 at 5:02 AM, Uwe Kleine-König > wrote: >> The phy's init routine must be called before it can be used. Do so in >> musb_init_controller and the matching shutdown in musb_remove. >> >> Signed-off-by: Uwe Kleine-König >> --- >> Note I'm not entirely sure this is the right place, but this made usb >> working on my omap3 machine here (at least somewhat, still fighting with >> the last details). >> --- >> drivers/usb/musb/musb_core.c | 10 ++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c >> index 2c624a10748d..ebb8a5455d2f 100644 >> --- a/drivers/usb/musb/musb_core.c >> +++ b/drivers/usb/musb/musb_core.c >> @@ -2108,6 +2108,10 @@ musb_init_controller(struct device *dev, int nIrq, >> void __iomem *ctrl) >> >> pm_runtime_get_sync(musb->controller); >> >> + status = usb_phy_init(musb->xceiv); >> + if (status < 0) >> + goto err_usb_phy_init; >> + > > Please check the comment around Line 2000, which states > musb_platform_init() should call for phy init. So the glue driver > should handle phy init/shutdown, not the core. This patch will cause > the phy init to be called twice for those devices which already handle > phy control in the glue driver. I guess we need to change the glue at this point. Only dsps glue calls usb_phy_init(), which means all other glues are wrong. Instead of fixing them all, we can let musb-core handle it for the glues. -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/3] usb: musb: core: call init and shutdown for the usb phy
Hi, On Mon, Jan 4, 2016 at 12:22 PM, Felipe Balbi wrote: > > Hi, > > Bin Liu writes: >> Hi, >> >> On Fri, Dec 18, 2015 at 5:02 AM, Uwe Kleine-König >> wrote: >>> The phy's init routine must be called before it can be used. Do so in >>> musb_init_controller and the matching shutdown in musb_remove. >>> >>> Signed-off-by: Uwe Kleine-König >>> --- >>> Note I'm not entirely sure this is the right place, but this made usb >>> working on my omap3 machine here (at least somewhat, still fighting with >>> the last details). >>> --- >>> drivers/usb/musb/musb_core.c | 10 ++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c >>> index 2c624a10748d..ebb8a5455d2f 100644 >>> --- a/drivers/usb/musb/musb_core.c >>> +++ b/drivers/usb/musb/musb_core.c >>> @@ -2108,6 +2108,10 @@ musb_init_controller(struct device *dev, int nIrq, >>> void __iomem *ctrl) >>> >>> pm_runtime_get_sync(musb->controller); >>> >>> + status = usb_phy_init(musb->xceiv); >>> + if (status < 0) >>> + goto err_usb_phy_init; >>> + >> >> Please check the comment around Line 2000, which states >> musb_platform_init() should call for phy init. So the glue driver >> should handle phy init/shutdown, not the core. This patch will cause >> the phy init to be called twice for those devices which already handle >> phy control in the glue driver. > > I guess we need to change the glue at this point. Only dsps glue calls > usb_phy_init(), which means all other glues are wrong. Instead of fixing > them all, we can let musb-core handle it for the glues. Yeah, make sense. Regards, -Bin. > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cdc_acm: blacklist Samsung Phones in firmware update mode
On Mon, 2016-01-04 at 12:32 +0700, Lars Melin wrote:
> On 2016-01-04 05:00, Jose Alonso wrote:
> >
> > The program Heimdall (http://glassechidna.com.au/heimdall) is used
> > to flash a new firmware in Samsung Mobile Phones. It uses only the
> > library libusb to access the device.
> >
> > The module "cdc_acm" recognizes it as CDC ACM device, creates a
> > device /dev/ttyACM* and send a USB_CDC_REQ_SET_LINE_CODING. Then,
> > for some phones when Heimdall call libusb_set_interface_alt_setting
> > or libusb_get_string_descriptor_ascii the device locks.
> > Also, the ModemManager initialization locks the device.
> >
> > There are 3 devices used by Samsung in firmware update mode:
> > idVendor=0x04e8 idProduct=0x6601 idProduct=0x685d IdProduct=0x68c3
> > source:
> > (https://github.com/Benjamin-Dobell/Heimdall/blob/master/heimdall/
> > source/BridgeManager.h)
> >
> > Signed-off-by: Jose Alonso
> > ---
> > drivers/usb/class/cdc-acm.c | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc
> > -acm.c
> > index 26ca4f9..8728afc 100644
> > --- a/drivers/usb/class/cdc-acm.c
> > +++ b/drivers/usb/class/cdc-acm.c
> > @@ -1843,6 +1843,17 @@ static const struct usb_device_id acm_ids[]
> > = {
> > .driver_info = IGNORE_DEVICE,
> > },
> >
> > + /* Exclude Samsung Mobile Phones in firmware update mode
> > */
> > + { USB_DEVICE(0x04e8, 0x6601),
> > + .driver_info = IGNORE_DEVICE,
> > + },
> > + { USB_DEVICE(0x04e8, 0x685d),
> > + .driver_info = IGNORE_DEVICE,
> > + },
> > + { USB_DEVICE(0x04e8, 0x68c3),
> > + .driver_info = IGNORE_DEVICE,
> > + },
> > +
> > /* control interfaces without any protocol set */
> > { USB_INTERFACE_INFO(USB_CLASS_COMM,
> > USB_CDC_SUBCLASS_ACM,
> > USB_CDC_PROTO_NONE) },
> > --
>
> NAK
>
> 04e8:6601 is also used for Samsung SCH-U209 CDMA modem so can not be
> ignored.
Probably used by a bunch of Samsung WWAN modems. I have a SGH-Z810
(UMTS) that also uses this ID in regular modem mode.
Dan
> 04e8:685d is an update mode id so it can be ignored by cdc-acm.
> 04e8:68c3 does not look like update mode, it has cdc-acm modem and
> cdc-acm device management interfaces.
>
>
> wbr
> Lars
> --
> 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
[PATCH v2 3/4] USB: mxu11x0: remove duplicated set_termios call
The function mxu1_set_termios is called twice in open callback. Only the first call is necessary. Signed-off-by: Mathieu OTHACEHE --- drivers/usb/serial/mxu11x0.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c index 7ad2727..6326cda 100644 --- a/drivers/usb/serial/mxu11x0.c +++ b/drivers/usb/serial/mxu11x0.c @@ -830,9 +830,6 @@ static int mxu1_open(struct tty_struct *tty, struct usb_serial_port *port) usb_clear_halt(serial->dev, port->write_urb->pipe); usb_clear_halt(serial->dev, port->read_urb->pipe); - if (tty) - mxu1_set_termios(tty, port, NULL); - status = usb_serial_generic_open(tty, port); if (status) goto unlink_int_urb; -- 2.6.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] USB: mxu11x0: clean device control commands
Sending OPEN and START commands twice is not necessary for this driver.
Also send STOP command at close.
Signed-off-by: Mathieu OTHACEHE
---
Changes in v2:
* Only remove OPEN_PORT and START_PORT in open callback.
drivers/usb/serial/mxu11x0.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
index 6196073..7ad2727 100644
--- a/drivers/usb/serial/mxu11x0.c
+++ b/drivers/usb/serial/mxu11x0.c
@@ -833,20 +833,6 @@ static int mxu1_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (tty)
mxu1_set_termios(tty, port, NULL);
- status = mxu1_send_ctrl_urb(serial, MXU1_OPEN_PORT,
- open_settings, MXU1_UART1_PORT);
- if (status) {
- dev_err(&port->dev, "cannot send open command: %d\n", status);
- goto unlink_int_urb;
- }
-
- status = mxu1_send_ctrl_urb(serial, MXU1_START_PORT,
- 0, MXU1_UART1_PORT);
- if (status) {
- dev_err(&port->dev, "cannot send start command: %d\n", status);
- goto unlink_int_urb;
- }
-
status = usb_serial_generic_open(tty, port);
if (status)
goto unlink_int_urb;
@@ -866,6 +852,13 @@ static void mxu1_close(struct usb_serial_port *port)
usb_serial_generic_close(port);
usb_kill_urb(port->interrupt_in_urb);
+ status = mxu1_send_ctrl_urb(port->serial, MXU1_STOP_PORT,
+ 0, MXU1_UART1_PORT);
+ if (status) {
+ dev_err(&port->dev, "failed to send stop port command: %d\n",
+ status);
+ }
+
status = mxu1_send_ctrl_urb(port->serial, MXU1_CLOSE_PORT,
0, MXU1_UART1_PORT);
if (status) {
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] USB: mxu11x0: move firmware download and endpoint testing to probe callback
Move interrupt in endpoint test and firmware download to a new probe
callback. This avoids unnecessary memory allocations done by core
before port_probe callback is called.
If the device has to be reseted (firmware downloaded) or if the
interface is incorrect (no interrupt in endpoint), the probe fails
by returning ENODEV.
Signed-off-by: Mathieu OTHACEHE
---
Changes in v2:
* Remove useless paranthesis
* Fix multi-line comment
drivers/usb/serial/mxu11x0.c | 131 ++-
1 file changed, 79 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
index 6326cda..b942aeb 100644
--- a/drivers/usb/serial/mxu11x0.c
+++ b/drivers/usb/serial/mxu11x0.c
@@ -272,7 +272,8 @@ static int mxu1_send_ctrl_urb(struct usb_serial *serial,
}
static int mxu1_download_firmware(struct usb_serial *serial,
- const struct firmware *fw_p)
+ const struct firmware *fw_p,
+ struct usb_endpoint_descriptor *endpoint)
{
int status = 0;
int buffer_size;
@@ -285,7 +286,7 @@ static int mxu1_download_firmware(struct usb_serial *serial,
struct mxu1_firmware_header *header;
unsigned int pipe;
- pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
+ pipe = usb_sndbulkpipe(dev, endpoint->bEndpointAddress);
buffer_size = fw_p->size + sizeof(*header);
buffer = kmalloc(buffer_size, GFP_KERNEL);
@@ -328,16 +329,86 @@ static int mxu1_download_firmware(struct usb_serial
*serial,
return 0;
}
-static int mxu1_port_probe(struct usb_serial_port *port)
+static int mxu1_probe(struct usb_serial *serial, const struct usb_device_id
*id)
{
- struct mxu1_port *mxport;
- struct mxu1_device *mxdev;
+ struct usb_host_interface *cur_altsetting;
+ char fw_name[32];
+ const struct firmware *fw_p = NULL;
+ struct usb_device *dev = serial->dev;
+ u16 model;
+ int err;
+ struct usb_endpoint_descriptor *endpoint, *interrupt_in, *bulk_out;
+ int i;
+
+ dev_dbg(&serial->interface->dev, "%s - product 0x%04X, num
configurations %d, configuration value %d\n",
+ __func__, le16_to_cpu(dev->descriptor.idProduct),
+ dev->descriptor.bNumConfigurations,
+ dev->actconfig->desc.bConfigurationValue);
+
+ cur_altsetting = serial->interface->cur_altsetting;
+ interrupt_in = NULL;
+ bulk_out = NULL;
+
+ for (i = 0; i < cur_altsetting->desc.bNumEndpoints; i++) {
+ endpoint = &cur_altsetting->endpoint[i].desc;
+ if (usb_endpoint_is_bulk_out(endpoint)) {
+ dev_dbg(&serial->interface->dev,
+ "found bulk out on endpoint %d\n", i);
+ bulk_out = endpoint;
+ }
+
+ if (usb_endpoint_is_int_in(endpoint)) {
+ dev_dbg(&serial->interface->dev,
+ "found interrupt in on endpoint %d\n", i);
+ interrupt_in = endpoint;
+ }
+ }
+
+ /* if we have only 1 bulk out endpoint, download firmware */
+ if (bulk_out && cur_altsetting->desc.bNumEndpoints == 1) {
+
+ model = le16_to_cpu(dev->descriptor.idProduct);
+ snprintf(fw_name,
+sizeof(fw_name),
+"moxa/moxa-%04x.fw",
+model);
+
+ err = request_firmware(&fw_p, fw_name, &serial->interface->dev);
+ if (err) {
+ dev_err(&serial->interface->dev, "failed to request
firmware: %d\n",
+ err);
+ return -ENODEV;
+ }
+
+ err = mxu1_download_firmware(serial, fw_p, bulk_out);
+ if (err)
+ goto err_release_firmware;
- if (!port->interrupt_in_urb) {
- dev_err(&port->dev, "no interrupt urb\n");
+ /* device is being reset */
+ err = -ENODEV;
+ goto err_release_firmware;
+
+ } else if (!interrupt_in) {
+ /*
+* firmware is already loaded but there is
+* no interrupt in endpoint available
+*/
+ dev_err(&serial->interface->dev, "no interrupt endpoint\n");
return -ENODEV;
}
+ return 0;
+
+err_release_firmware:
+ release_firmware(fw_p);
+ return err;
+}
+
+static int mxu1_port_probe(struct usb_serial_port *port)
+{
+ struct mxu1_port *mxport;
+ struct mxu1_device *mxdev;
+
mxport = kzalloc(sizeof(struct mxu1_port), GFP_KERNEL);
if (!mxport)
return -ENOMEM;
@@ -381,16 +452,6 @@ static int mxu1_port_remove(struct usb_serial_port *port)
static int mxu1_start
[PATCH v2 0/4] USB: mxu11x0: fixes and follow ups
Hi, Here are the follow up commits proposed during last Johan review of the new mxu11x0 driver. I also patched a memory leak on usb_serial private data. In v2, I fixed the trivial issues reported by Johan in commits 1/4 and 4/4. About commit 2/4, I was wrong in last cover-letter message. The exact command order sniffed on Windows during opening is : SET_CONFIG (set_termios) -> OPEN_PORT -> START_PORT To be compliant, commit 2/4 only deletes duplicated OPEN_PORT and START_PORT commands. The new commit 3/4 deletes the duplicated set_termios in open callback. Thanks, Mathieu Mathieu OTHACEHE (4): USB: mxu11x0: fix memory leak on usb_serial private data USB: mxu11x0: clean device control commands USB: mxu11x0: remove duplicated set_termios call USB: mxu11x0: move firmware download and endpoint testing to probe callback drivers/usb/serial/mxu11x0.c | 171 ++- 1 file changed, 104 insertions(+), 67 deletions(-) -- 2.6.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial private data
On nominal execution, private data allocated on port_probe and attach
are never freed. Add port_remove and release callbacks to free them
respectively.
Signed-off-by: Mathieu OTHACEHE
---
Changes in v2:
* Move release callback after attach callback
drivers/usb/serial/mxu11x0.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
index e3c3f57c..6196073 100644
--- a/drivers/usb/serial/mxu11x0.c
+++ b/drivers/usb/serial/mxu11x0.c
@@ -368,6 +368,16 @@ static int mxu1_port_probe(struct usb_serial_port *port)
return 0;
}
+static int mxu1_port_remove(struct usb_serial_port *port)
+{
+ struct mxu1_port *mxport;
+
+ mxport = usb_get_serial_port_data(port);
+ kfree(mxport);
+
+ return 0;
+}
+
static int mxu1_startup(struct usb_serial *serial)
{
struct mxu1_device *mxdev;
@@ -427,6 +437,14 @@ err_free_mxdev:
return err;
}
+static void mxu1_release(struct usb_serial *serial)
+{
+ struct mxu1_device *mxdev;
+
+ mxdev = usb_get_serial_data(serial);
+ kfree(mxdev);
+}
+
static int mxu1_write_byte(struct usb_serial_port *port, u32 addr,
u8 mask, u8 byte)
{
@@ -957,7 +975,9 @@ static struct usb_serial_driver mxu11x0_device = {
.id_table = mxu1_idtable,
.num_ports = 1,
.port_probe = mxu1_port_probe,
+ .port_remove= mxu1_port_remove,
.attach = mxu1_startup,
+ .release= mxu1_release,
.open = mxu1_open,
.close = mxu1_close,
.ioctl = mxu1_ioctl,
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: core: call init and shutdown for the usb phy
Hi, Bin Liu writes: > Hi, > > On Mon, Jan 4, 2016 at 12:22 PM, Felipe Balbi wrote: >> >> Hi, >> >> Bin Liu writes: >>> Hi, >>> >>> On Fri, Dec 18, 2015 at 5:02 AM, Uwe Kleine-König >>> wrote: The phy's init routine must be called before it can be used. Do so in musb_init_controller and the matching shutdown in musb_remove. Signed-off-by: Uwe Kleine-König --- Note I'm not entirely sure this is the right place, but this made usb working on my omap3 machine here (at least somewhat, still fighting with the last details). --- drivers/usb/musb/musb_core.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 2c624a10748d..ebb8a5455d2f 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -2108,6 +2108,10 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) pm_runtime_get_sync(musb->controller); + status = usb_phy_init(musb->xceiv); + if (status < 0) + goto err_usb_phy_init; + >>> >>> Please check the comment around Line 2000, which states >>> musb_platform_init() should call for phy init. So the glue driver >>> should handle phy init/shutdown, not the core. This patch will cause >>> the phy init to be called twice for those devices which already handle >>> phy control in the glue driver. >> >> I guess we need to change the glue at this point. Only dsps glue calls >> usb_phy_init(), which means all other glues are wrong. Instead of fixing >> them all, we can let musb-core handle it for the glues. during the -rc we can patch dsps to remove those calls and all should be good. Sorry I didn't notice this earlier :-s cheers -- balbi signature.asc Description: PGP signature
[PATCH 09/17] usb: host: ehci-dbg: fix up function definitions
Functions must have the opening brace at the beginning of the next line
and body conforming indentation.
This patch also reduces qh_lines() header definition to two lines.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 44 +---
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index edae79e..a365d9d 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -54,7 +54,9 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
}
#else
-static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
+{
+}
#endif
@@ -94,7 +96,9 @@ static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
}
#else
-static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
+{
+}
#endif
@@ -284,23 +288,32 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
int port, u32 status)
#else
static inline void __maybe_unused
dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
-{}
+{
+}
static inline int __maybe_unused
dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
-{ return 0; }
+{
+ return 0;
+}
static inline int __maybe_unused
dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
-{ return 0; }
+{
+ return 0;
+}
static inline int __maybe_unused
dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
-{ return 0; }
+{
+ return 0;
+}
static inline int __maybe_unused
dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status)
-{ return 0; }
+{
+ return 0;
+}
#endif /* CONFIG_DYNAMIC_DEBUG */
@@ -327,8 +340,13 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
int port, u32 status)
#ifdef STUB_DEBUG_FILES
-static inline void create_debug_files(struct ehci_hcd *bus) { }
-static inline void remove_debug_files(struct ehci_hcd *bus) { }
+static inline void create_debug_files(struct ehci_hcd *bus)
+{
+}
+
+static inline void remove_debug_files(struct ehci_hcd *bus)
+{
+}
#else
@@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci,
__hc32 token)
return '/';
}
-static void qh_lines(
- struct ehci_hcd *ehci,
- struct ehci_qh *qh,
- char **nextp,
- unsigned *sizep
-)
+static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
+ char **nextp, unsigned *sizep)
{
u32 scratch;
u32 hw_curr;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/17] usb: host: ehci-dbg: move trailing statements to next line
This patch fixes coding style issues reported by checkpatch concerning
to switch case statements. There are few additional changes made to fix
other coding styles issues.
These additional changes are:
- The compound statement "({...})" on line 474 is pulled out from
snprintf parameters.
- On line 723 the constant "0x03" is moved to right.
Signed-off-by: Geyslan G. Bem
---
Notes:
Before this patch there are 14 warnings about trailing statements that
should be on next line. After there are still 4. These lines concern to
a macro that will be modified to inline function in sequential patches.
Tested by compilation only.
drivers/usb/host/ehci-dbg.c | 54 +++--
1 file changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index df9f598..c409e4f 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -243,10 +243,18 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
int port, u32 status)
/* signaling state */
switch (status & (3 << 10)) {
- case 0 << 10: sig = "se0"; break;
- case 1 << 10: sig = "k"; break; /* low speed */
- case 2 << 10: sig = "j"; break;
- default: sig = "?"; break;
+ case 0 << 10:
+ sig = "se0";
+ break;
+ case 1 << 10: /* low speed */
+ sig = "k";
+ break;
+ case 2 << 10:
+ sig = "j";
+ break;
+ default:
+ sig = "?";
+ break;
}
return scnprintf(buf, len,
@@ -451,6 +459,8 @@ static void qh_lines(
/* hc may be modifying the list as we read it ... */
list_for_each(entry, &qh->qtd_list) {
+ char *type;
+
td = list_entry(entry, struct ehci_qtd, qtd_list);
scratch = hc32_to_cpup(ehci, &td->hw_token);
mark = ' ';
@@ -464,16 +474,24 @@ static void qh_lines(
else if (td->hw_alt_next != list_end)
mark = '/';
}
+ switch ((scratch >> 8) & 0x03) {
+ case 0:
+ type = "out";
+ break;
+ case 1:
+ type = "in";
+ break;
+ case 2:
+ type = "setup";
+ break;
+ default:
+ type = "?";
+ break;
+ }
temp = snprintf(next, size,
"\n\t%p%c%s len=%d %08x urb %p"
" [td %08x buf[0] %08x]",
- td, mark, ({ char *tmp;
-switch ((scratch>>8)&0x03) {
-case 0: tmp = "out"; break;
-case 1: tmp = "in"; break;
-case 2: tmp = "setup"; break;
-default: tmp = "?"; break;
-} tmp;}),
+ td, mark, type,
(scratch >> 16) & 0x7fff,
scratch,
td->urb,
@@ -702,11 +720,15 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
*buf)
&p.qh->qtd_list,
qtd_list) {
temp++;
- switch (0x03 & (hc32_to_cpu(
- ehci,
- qtd->hw_token) >> 8)) {
- case 0: type = "out"; continue;
- case 1: type = "in"; continue;
+ switch ((hc32_to_cpu(ehci,
+ qtd->hw_token) >> 8)
+ & 0x03) {
+ case 0:
+ type = "out";
+ continue;
+ case 1:
+ type = "in";
+ continue;
}
}
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/17] usb: host: ehci-dbg: enclose conditional blocks with braces
This patch fixes coding style issues reported by checkpatch concerning
to conditional blocks without braces.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index bf3ec19..7f8ad18 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -491,11 +491,11 @@ static void qh_lines(struct ehci_hcd *ehci, struct
ehci_qh *qh,
td = list_entry(entry, struct ehci_qtd, qtd_list);
scratch = hc32_to_cpup(ehci, &td->hw_token);
mark = ' ';
- if (hw_curr == td->qtd_dma)
+ if (hw_curr == td->qtd_dma) {
mark = '*';
- else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma))
+ } else if (hw->hw_qtd_next == cpu_to_hc32(ehci, td->qtd_dma)) {
mark = '+';
- else if (QTD_LENGTH(scratch)) {
+ } else if (QTD_LENGTH(scratch)) {
if (td->hw_alt_next == ehci->async->hw->hw_alt_next)
mark = '#';
else if (td->hw_alt_next != list_end)
@@ -772,8 +772,9 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
*buf)
if (seen_count < DBG_SCHED_LIMIT)
seen[seen_count++].qh = p.qh;
- } else
+ } else {
temp = 0;
+ }
tag = Q_NEXT_TYPE(ehci, hw->hw_next);
p = p.qh->qh_next;
break;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function
This patch fixes a coding style issue reported by checkpatch related to
many leading tabs, removing a 'do while' loop and making use of goto tag
instead.
Others changes in this patch are:
- Some multiline statements are reduced (718, 729, 780, 786, 790).
- A constant is moved to right on line 770.
Signed-off-by: Geyslan G. Bem
---
Notes:
Tested by compilation only.
drivers/usb/host/ehci-dbg.c | 180 ++--
1 file changed, 88 insertions(+), 92 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 2268756..278333d 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
*buf)
*/
spin_lock_irqsave(&ehci->lock, flags);
for (i = 0; i < ehci->periodic_size; i++) {
+ struct ehci_qh_hw *hw;
+
p = ehci->pshadow[i];
if (likely(!p.ptr))
continue;
@@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
*buf)
size -= temp;
next += temp;
- do {
- struct ehci_qh_hw *hw;
-
- switch (hc32_to_cpu(ehci, tag)) {
- case Q_TYPE_QH:
- hw = p.qh->hw;
- temp = scnprintf(next, size, " qh%d-%04x/%p",
- p.qh->ps.period,
- hc32_to_cpup(ehci,
- &hw->hw_info2)
- /* uframe masks */
- & (QH_CMASK | QH_SMASK),
- p.qh);
- size -= temp;
- next += temp;
- /* don't repeat what follows this qh */
- for (temp = 0; temp < seen_count; temp++) {
- if (seen[temp].ptr != p.ptr)
+do_loop:
+ switch (hc32_to_cpu(ehci, tag)) {
+ case Q_TYPE_QH:
+ hw = p.qh->hw;
+ temp = scnprintf(next, size, " qh%d-%04x/%p",
+ p.qh->ps.period,
+ hc32_to_cpup(ehci, &hw->hw_info2)
+ /* uframe masks */
+ & (QH_CMASK | QH_SMASK),
+ p.qh);
+ size -= temp;
+ next += temp;
+ /* don't repeat what follows this qh */
+ for (temp = 0; temp < seen_count; temp++) {
+ if (seen[temp].ptr != p.ptr)
+ continue;
+ if (p.qh->qh_next.ptr) {
+ temp = scnprintf(next, size, " ...");
+ size -= temp;
+ next += temp;
+ }
+ break;
+ }
+ /* show more info the first time around */
+ if (temp == seen_count) {
+ u32 scratch = hc32_to_cpup(ehci,
+ &hw->hw_info1);
+ struct ehci_qtd *qtd;
+ char*type = "";
+
+ /* count tds, get ep direction */
+ temp = 0;
+ list_for_each_entry(qtd,
+ &p.qh->qtd_list,
+ qtd_list) {
+ temp++;
+ switch ((hc32_to_cpu(ehci,
+ qtd->hw_token) >> 8)
+ & 0x03) {
+ case 0:
+ type = "out";
+ continue;
+ case 1:
+ type = "in";
continue;
- if (p.qh->qh_next.ptr) {
- temp = scnprintf(next, size,
- " ...");
- size -= temp;
- next += temp;
}
-
[PATCH 14/17] usb: host: ehci-dbg: replace sizeof operand
This patch fixes a coding style issue reported by checkpatch concerning
to usage of sizeof operand as a variable instead the type.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index b9bbac7..bf3ec19 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -996,7 +996,7 @@ static struct debug_buffer *alloc_buffer(struct usb_bus
*bus,
{
struct debug_buffer *buf;
- buf = kzalloc(sizeof(struct debug_buffer), GFP_KERNEL);
+ buf = kzalloc(sizeof(*buf), GFP_KERNEL);
if (buf) {
buf->bus = bus;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/17] usb: host: ehci-dbg: prefer kmalloc_array over kmalloc times size
This patch fixes a coding style issue reported by checkpatch related to kmalloc_array usage. On same line the sizeof operand was enclosed by parenthesis. Signed-off-by: Geyslan G. Bem --- drivers/usb/host/ehci-dbg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c index 7f8ad18..2268756 100644 --- a/drivers/usb/host/ehci-dbg.c +++ b/drivers/usb/host/ehci-dbg.c @@ -678,7 +678,7 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf) unsignedi; __hc32 tag; - seen = kmalloc(DBG_SCHED_LIMIT * sizeof *seen, GFP_ATOMIC); + seen = kmalloc_array(DBG_SCHED_LIMIT, sizeof(*seen), GFP_ATOMIC); if (!seen) return 0; seen_count = 0; -- 2.6.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/17] usb: host: ehci-dbg: use a blank line after struct declarations
This patch fixes coding style issues reported by checkpatch concerning
to missing line after struct declarations.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index a365d9d..4ba577d 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -367,6 +367,7 @@ static const struct file_operations debug_async_fops = {
.release= debug_close,
.llseek = default_llseek,
};
+
static const struct file_operations debug_bandwidth_fops = {
.owner = THIS_MODULE,
.open = debug_bandwidth_open,
@@ -374,6 +375,7 @@ static const struct file_operations debug_bandwidth_fops = {
.release= debug_close,
.llseek = default_llseek,
};
+
static const struct file_operations debug_periodic_fops = {
.owner = THIS_MODULE,
.open = debug_periodic_open,
@@ -381,6 +383,7 @@ static const struct file_operations debug_periodic_fops = {
.release= debug_close,
.llseek = default_llseek,
};
+
static const struct file_operations debug_registers_fops = {
.owner = THIS_MODULE,
.open = debug_registers_open,
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/17] usb: host: ehci-dbg: remove blank line before close brace
This patch fixes coding style issue reported by checkpatch concerning to an unnecessary line before close brace. Signed-off-by: Geyslan G. Bem --- drivers/usb/host/ehci-dbg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c index a42b05b..b9bbac7 100644 --- a/drivers/usb/host/ehci-dbg.c +++ b/drivers/usb/host/ehci-dbg.c @@ -1052,7 +1052,6 @@ static ssize_t debug_output(struct file *file, char __user *user_buf, out: return ret; - } static int debug_close(struct inode *inode, struct file *file) -- 2.6.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/17] usb: host: ehci-dbg: add blank line after declarations
This patch fixes coding style issues reported by checkpatch concerning
to missing line after variable declarations.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index db28992..a42b05b 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -1085,6 +1085,7 @@ static int debug_bandwidth_open(struct inode *inode,
struct file *file)
static int debug_periodic_open(struct inode *inode, struct file *file)
{
struct debug_buffer *buf;
+
buf = alloc_buffer(inode->i_private, fill_periodic_buffer);
if (!buf)
return -ENOMEM;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/17] usb: host: ehci-dbg: convert macro to inline function
This patch converts macros into inline functions since the usage of
second is encouraged by Coding Style instead of the first.
Macros converted to functions:
- dbg_status
- dbg_cmd
- dbg_port
- speed_char
The size after changes remains the same.
Before:
text data bss dec hex filename
36920 81 12 37013 9095 drivers/usb/host/ehci-hcd.o
After:
text data bss dec hex filename
36920 81 12 37013 9095 drivers/usb/host/ehci-hcd.o
Signed-off-by: Geyslan G. Bem
---
Notes:
The comment
/* functions have the "wrong" filename when they're output... */
was removed because after changes the file remained the same size and
the symbols of the new inline functions were not created, so they were
properly inlined.
Tested by compilation only.
drivers/usb/host/ehci-dbg.c | 54 -
1 file changed, 34 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 4ba577d..db28992 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -317,23 +317,31 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
int port, u32 status)
#endif /* CONFIG_DYNAMIC_DEBUG */
-/* functions have the "wrong" filename when they're output... */
-#define dbg_status(ehci, label, status) { \
- char _buf [80]; \
- dbg_status_buf (_buf, sizeof _buf, label, status); \
- ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_status(struct ehci_hcd *ehci, const char *label, u32 status)
+{
+ char buf[80];
+
+ dbg_status_buf(buf, sizeof(buf), label, status);
+ ehci_dbg(ehci, "%s\n", buf);
}
-#define dbg_cmd(ehci, label, command) { \
- char _buf [80]; \
- dbg_command_buf (_buf, sizeof _buf, label, command); \
- ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_cmd(struct ehci_hcd *ehci, const char *label, u32 command)
+{
+ char buf[80];
+
+ dbg_command_buf(buf, sizeof(buf), label, command);
+ ehci_dbg(ehci, "%s\n", buf);
}
-#define dbg_port(ehci, label, port, status) { \
- char _buf [80]; \
- dbg_port_buf (_buf, sizeof _buf, label, port, status); \
- ehci_dbg (ehci, "%s\n", _buf); \
+static inline void
+dbg_port(struct ehci_hcd *ehci, const char *label, int port, u32 status)
+{
+ char buf[80];
+
+ dbg_port_buf(buf, sizeof(buf), label, port, status);
+ ehci_dbg(ehci, "%s\n", buf);
}
/*-*/
@@ -403,13 +411,19 @@ struct debug_buffer {
size_t alloc_size;
};
-#define speed_char(info1) ({ char tmp; \
- switch (info1 & (3 << 12)) { \
- case QH_FULL_SPEED: tmp = 'f'; break; \
- case QH_LOW_SPEED: tmp = 'l'; break; \
- case QH_HIGH_SPEED: tmp = 'h'; break; \
- default: tmp = '?'; break; \
- } tmp; })
+static inline char speed_char(u32 info1)
+{
+ switch (info1 & (3 << 12)) {
+ case QH_FULL_SPEED:
+ return 'f';
+ case QH_LOW_SPEED:
+ return 'l';
+ case QH_HIGH_SPEED:
+ return 'h';
+ default:
+ return '?';
+ }
+}
static inline char token_mark(struct ehci_hcd *ehci, __hc32 token)
{
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison
This patch fixes an unsigned comparison to less than 0.
Signed-off-by: Geyslan G. Bem
---
Notes:
I'm not sure about that comparison because in qh_lines() temp receives
the snprintf() return and thereafter occurs this comparison:
if (size < temp)
temp = size;
Is it a test of string truncation right? That possibility is being
treated. But if after some snprintf returns the temp is exactly size
minus 1 (trailing null)? Could this scenario happen? If yes, I think
size could be not counting correctly. Let me know more about it.
drivers/usb/host/ehci-dbg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 980ca55..1645120 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
next += temp;
list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
- if (size <= 0)
+ if (size == 0)
break;
qh_lines(ehci, qh, &next, &size);
}
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/17] usb: host: ehci-dbg: fix up closing parenthesis
This patch puts the closing parenthesis at the statement end removing
unnecessary "new line".
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index c409e4f..3b423e1 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -35,8 +35,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
HCS_N_PCC(params),
HCS_PORTROUTED(params) ? "" : " ordered",
HCS_PPC(params) ? "" : " !ppc",
- HCS_N_PORTS(params)
- );
+ HCS_N_PORTS(params));
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
if (HCS_PORTROUTED(params)) {
int i;
@@ -189,8 +188,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label,
u32 status)
(status & STS_FLR) ? " FLR" : "",
(status & STS_PCD) ? " PCD" : "",
(status & STS_ERR) ? " ERR" : "",
- (status & STS_INT) ? " INT" : ""
- );
+ (status & STS_INT) ? " INT" : "");
}
static int __maybe_unused
@@ -205,8 +203,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label,
u32 enable)
(enable & STS_FLR) ? " FLR" : "",
(enable & STS_PCD) ? " PCD" : "",
(enable & STS_ERR) ? " ERR" : "",
- (enable & STS_INT) ? " INT" : ""
- );
+ (enable & STS_INT) ? " INT" : "");
}
static const char *const fls_strings[] = { "1024", "512", "256", "??" };
@@ -232,8 +229,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label,
u32 command)
(command & CMD_PSE) ? " Periodic" : "",
fls_strings[(command >> 2) & 0x3],
(command & CMD_RESET) ? " Reset" : "",
- (command & CMD_RUN) ? "RUN" : "HALT"
- );
+ (command & CMD_RUN) ? "RUN" : "HALT");
}
static int
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/17] usb: host: ehci-dbg: put spaces around operators
This patch fixes coding style issues reported by checkpatch concerning
to missing spaces around operators.
There is an additional change on line 49 that removes unnecessary
parenthesis around ternary operands.
Signed-off-by: Geyslan G. Bem
---
Notes:
Tested by compilation only.
drivers/usb/host/ehci-dbg.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 3b423e1..980ca55 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -46,7 +46,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
/* FIXME MIPS won't readb() ... */
byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
- ((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
+ (i & 0x1) ? byte & 0xf : (byte >> 4) & 0xf);
strcat(buf, tmp);
}
ehci_dbg(ehci, "%s portroute %s\n", label, buf);
@@ -257,14 +257,14 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
int port, u32 status)
"%s%sport:%d status %06x %d %s%s%s%s%s%s "
"sig=%s%s%s%s%s%s%s%s%s%s%s",
label, label[0] ? " " : "", port, status,
- status>>25,/*device address */
- (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
+ status >> 25, /*device address */
+ (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ACK ?
" ACK" : "",
- (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_NYET ?
+ (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_NYET ?
" NYET" : "",
- (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_STALL ?
+ (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_STALL ?
" STALL" : "",
- (status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ERR ?
+ (status & PORT_SSTS) >> 23 == PORTSC_SUSPEND_STS_ERR ?
" ERR" : "",
(status & PORT_POWER) ? " POWER" : "",
(status & PORT_OWNER) ? " OWNER" : "",
@@ -846,7 +846,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer
*buf)
if (dev_is_pci(hcd->self.controller)) {
struct pci_dev *pdev;
u32 offset, cap, cap2;
- unsignedcount = 256/4;
+ unsignedcount = 256 / 4;
pdev = to_pci_dev(ehci_to_hcd(ehci)->self.controller);
offset = HCC_EXT_CAPS(ehci_readl(ehci,
@@ -1058,7 +1058,7 @@ static int debug_periodic_open(struct inode *inode,
struct file *file)
if (!buf)
return -ENOMEM;
- buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8)*PAGE_SIZE;
+ buf->alloc_size = (sizeof(void *) == 4 ? 6 : 8) * PAGE_SIZE;
file->private_data = buf;
return 0;
}
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/17] usb: host: ehci-dbg: use C89-style comments
This patch fixes coding style issues reported by checkpatch.
Coding style demands usage of C89-style comments and a specific format
when it's multiline.
This also removes the Free Software Foundation address because FSF can
change it again.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 52bf3fe..df9f598 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -11,16 +11,14 @@
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
/* this file is part of ehci-hcd.c */
#ifdef CONFIG_DYNAMIC_DEBUG
-/* check the values in the HCSPARAMS register
+/*
+ * check the values in the HCSPARAMS register
* (host controller _Structural_ parameters)
* see EHCI spec, Table 2-4 for each value
*/
@@ -46,7 +44,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
buf[0] = 0;
for (i = 0; i < HCS_N_PORTS(params); i++) {
- // FIXME MIPS won't readb() ...
+ /* FIXME MIPS won't readb() ... */
byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
@@ -63,10 +61,11 @@ static inline void dbg_hcs_params(struct ehci_hcd *ehci,
char *label) {}
#ifdef CONFIG_DYNAMIC_DEBUG
-/* check the values in the HCCPARAMS register
+/*
+ * check the values in the HCCPARAMS register
* (host controller _Capability_ parameters)
* see EHCI Spec, Table 2-5 for each value
- * */
+ */
static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
{
u32 params = ehci_readl(ehci, &ehci->caps->hcc_params);
@@ -515,7 +514,8 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
*next = 0;
- /* dumps a snapshot of the async schedule.
+ /*
+* dumps a snapshot of the async schedule.
* usually empty except for long-term bulk reads, or head.
* one QH per line, and TDs we know about
*/
@@ -647,7 +647,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
*buf)
size -= temp;
next += temp;
- /* dump a snapshot of the periodic schedule.
+ /*
+* dump a snapshot of the periodic schedule.
* iso changes, interrupt usually doesn't.
*/
spin_lock_irqsave(&ehci->lock, flags);
@@ -861,7 +862,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer
*buf)
}
#endif
- // FIXME interpret both types of params
+ /* FIXME interpret both types of params */
i = ehci_readl(ehci, &ehci->caps->hcs_params);
temp = scnprintf(next, size, "structural params 0x%08x\n", i);
size -= temp;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/17] usb: host: ehci-dbg: remove space before open square bracket
This patch fixes coding style issues reported by checkpatch. The only
change in this patch that isn't just removing spaces before opening
square brackets is at line 213 where the initialization of fls_strings[]
is placed in same line.
Signed-off-by: Geyslan G. Bem
---
Notes:
Before this patch there are 20 warnings about spaces before open square
bracket. After there are still 3. These lines concern to macros that
will be modified to inline functions in sequential patches.
Tested by compilation only.
drivers/usb/host/ehci-dbg.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index fcbbdfa..52bf3fe 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -42,7 +42,7 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
if (HCS_PORTROUTED(params)) {
int i;
- char buf [46], tmp [7], byte;
+ char buf[46], tmp[7], byte;
buf[0] = 0;
for (i = 0; i < HCS_N_PORTS(params); i++) {
@@ -109,8 +109,8 @@ dbg_qtd(const char *label, struct ehci_hcd *ehci, struct
ehci_qtd *qtd)
hc32_to_cpup(ehci, &qtd->hw_next),
hc32_to_cpup(ehci, &qtd->hw_alt_next),
hc32_to_cpup(ehci, &qtd->hw_token),
- hc32_to_cpup(ehci, &qtd->hw_buf [0]));
- if (qtd->hw_buf [1])
+ hc32_to_cpup(ehci, &qtd->hw_buf[0]));
+ if (qtd->hw_buf[1])
ehci_dbg(ehci, " p1=%08x p2=%08x p3=%08x p4=%08x\n",
hc32_to_cpup(ehci, &qtd->hw_buf[1]),
hc32_to_cpup(ehci, &qtd->hw_buf[2]),
@@ -179,7 +179,7 @@ dbg_status_buf(char *buf, unsigned len, const char *label,
u32 status)
{
return scnprintf(buf, len,
"%s%sstatus %04x%s%s%s%s%s%s%s%s%s%s%s",
- label, label [0] ? " " : "", status,
+ label, label[0] ? " " : "", status,
(status & STS_PPCE_MASK) ? " PPCE" : "",
(status & STS_ASS) ? " Async" : "",
(status & STS_PSS) ? " Periodic" : "",
@@ -199,7 +199,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label,
u32 enable)
{
return scnprintf(buf, len,
"%s%sintrenable %02x%s%s%s%s%s%s%s",
- label, label [0] ? " " : "", enable,
+ label, label[0] ? " " : "", enable,
(enable & STS_PPCE_MASK) ? " PPCE" : "",
(enable & STS_IAA) ? " IAA" : "",
(enable & STS_FATAL) ? " FATAL" : "",
@@ -210,8 +210,7 @@ dbg_intr_buf(char *buf, unsigned len, const char *label,
u32 enable)
);
}
-static const char *const fls_strings [] =
-{ "1024", "512", "256", "??" };
+static const char *const fls_strings[] = { "1024", "512", "256", "??" };
static int
dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
@@ -219,7 +218,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label,
u32 command)
return scnprintf(buf, len,
"%s%scommand %07x %s%s%s%s%s%s=%d ithresh=%d%s%s%s%s "
"period=%s%s %s",
- label, label [0] ? " " : "", command,
+ label, label[0] ? " " : "", command,
(command & CMD_HIRD) ? " HIRD" : "",
(command & CMD_PPCEE) ? " PPCEE" : "",
(command & CMD_FSP) ? " FSP" : "",
@@ -232,7 +231,7 @@ dbg_command_buf(char *buf, unsigned len, const char *label,
u32 command)
(command & CMD_IAAD) ? " IAAD" : "",
(command & CMD_ASE) ? " Async" : "",
(command & CMD_PSE) ? " Periodic" : "",
- fls_strings [(command >> 2) & 0x3],
+ fls_strings[(command >> 2) & 0x3],
(command & CMD_RESET) ? " Reset" : "",
(command & CMD_RUN) ? "RUN" : "HALT"
);
@@ -254,7 +253,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
int port, u32 status)
return scnprintf(buf, len,
"%s%sport:%d status %06x %d %s%s%s%s%s%s "
"sig=%s%s%s%s%s%s%s%s%s%s%s",
- label, label [0] ? " " : "", port, status,
+ label, label[0] ? " " : "", port, status,
status>>25,/*device address */
(status & PORT_SSTS)>>23 == PORTSC_SUSPEND_STS_ACK ?
" ACK" : "",
@@ -653,10 +652,10 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
*buf)
*/
spin_lock_irqsave(&ehci->lock, flags);
for (i = 0; i < ehci->periodic_size; i++) {
- p = ehci->pshadow [i];
+ p = ehci->pshadow[i];
if (likely(!p.ptr))
continue;
- tag = Q_NEXT_TYPE(ehci, ehci->periodic [i]);
+
[PATCH 01/17] usb: host: ehci-dbg: remove space before open parenthesis
This patch fixes coding style issues reported by checkpatch. The vast
majority of changes in this patch are removing spaces before opening
parenthesis, but in some cases, a few additional changes are made to fix
other coding style issues.
These additional changes are:
- Spaces around >> on line 50.
- On line 55 a call to ehci_dbg reduced to a single line.
- sizeof operands surrounded with parenthesis on lines 877, 883, 889
and 901.
Signed-off-by: Geyslan G. Bem
---
Notes:
Before this patch there are 105 warnings about spaces before opening
parenthesis. After there are still 6. These lines concern to macros that
will be modified to inline functions in sequential patches.
Tested by compilation only.
drivers/usb/host/ehci-dbg.c | 199 ++--
1 file changed, 99 insertions(+), 100 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index b7d623f..fcbbdfa 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -24,41 +24,40 @@
* (host controller _Structural_ parameters)
* see EHCI spec, Table 2-4 for each value
*/
-static void dbg_hcs_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
{
u32 params = ehci_readl(ehci, &ehci->caps->hcs_params);
- ehci_dbg (ehci,
+ ehci_dbg(ehci,
"%s hcs_params 0x%x dbg=%d%s cc=%d pcc=%d%s%s ports=%d\n",
label, params,
- HCS_DEBUG_PORT (params),
- HCS_INDICATOR (params) ? " ind" : "",
- HCS_N_CC (params),
- HCS_N_PCC (params),
- HCS_PORTROUTED (params) ? "" : " ordered",
- HCS_PPC (params) ? "" : " !ppc",
- HCS_N_PORTS (params)
+ HCS_DEBUG_PORT(params),
+ HCS_INDICATOR(params) ? " ind" : "",
+ HCS_N_CC(params),
+ HCS_N_PCC(params),
+ HCS_PORTROUTED(params) ? "" : " ordered",
+ HCS_PPC(params) ? "" : " !ppc",
+ HCS_N_PORTS(params)
);
/* Port routing, per EHCI 0.95 Spec, Section 2.2.5 */
- if (HCS_PORTROUTED (params)) {
+ if (HCS_PORTROUTED(params)) {
int i;
char buf [46], tmp [7], byte;
buf[0] = 0;
- for (i = 0; i < HCS_N_PORTS (params); i++) {
+ for (i = 0; i < HCS_N_PORTS(params); i++) {
// FIXME MIPS won't readb() ...
- byte = readb (&ehci->caps->portroute[(i>>1)]);
+ byte = readb(&ehci->caps->portroute[(i >> 1)]);
sprintf(tmp, "%d ",
((i & 0x1) ? ((byte)&0xf) : ((byte>>4)&0xf)));
strcat(buf, tmp);
}
- ehci_dbg (ehci, "%s portroute %s\n",
- label, buf);
+ ehci_dbg(ehci, "%s portroute %s\n", label, buf);
}
}
#else
-static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
#endif
@@ -68,19 +67,19 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci,
char *label) {}
* (host controller _Capability_ parameters)
* see EHCI Spec, Table 2-5 for each value
* */
-static void dbg_hcc_params (struct ehci_hcd *ehci, char *label)
+static void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
{
u32 params = ehci_readl(ehci, &ehci->caps->hcc_params);
- if (HCC_ISOC_CACHE (params)) {
- ehci_dbg (ehci,
+ if (HCC_ISOC_CACHE(params)) {
+ ehci_dbg(ehci,
"%s hcc_params %04x caching frame %s%s%s\n",
label, params,
HCC_PGM_FRAMELISTLEN(params) ? "256/512/1024" : "1024",
HCC_CANPARK(params) ? " park" : "",
HCC_64BIT_ADDR(params) ? " 64 bit addr" : "");
} else {
- ehci_dbg (ehci,
+ ehci_dbg(ehci,
"%s hcc_params %04x thresh %d uframes %s%s%s%s%s%s%s\n",
label,
params,
@@ -97,14 +96,14 @@ static void dbg_hcc_params (struct ehci_hcd *ehci, char
*label)
}
#else
-static inline void dbg_hcc_params (struct ehci_hcd *ehci, char *label) {}
+static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
#endif
#ifdef CONFIG_DYNAMIC_DEBUG
static void __maybe_unused
-dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
+dbg_qtd(const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd)
{
ehci_dbg(ehci, "%s td %p n%08x %08x t%08x p0=%08x\n", label, qtd,
hc32_to_cpup(ehci, &qtd->hw_next),
@@ -120,22 +119,22 @@ dbg_qtd (const char *label, struct ehci_hcd *ehci, struct
ehci_qtd *qtd)
[PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
This patch fixes coding style issues reported by checkpatch concerning
to unnecessary space after a cast.
Signed-off-by: Geyslan G. Bem
---
drivers/usb/host/ehci-dbg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
index 1645120..edae79e 100644
--- a/drivers/usb/host/ehci-dbg.c
+++ b/drivers/usb/host/ehci-dbg.c
@@ -123,7 +123,7 @@ dbg_qh(const char *label, struct ehci_hcd *ehci, struct
ehci_qh *qh)
ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
- dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
+ dbg_qtd("overlay", ehci, (struct ehci_qtd *)&hw->hw_qtd_next);
}
static void __maybe_unused
@@ -491,7 +491,7 @@ static void qh_lines(
(scratch >> 16) & 0x7fff,
scratch,
td->urb,
- (u32) td->qtd_dma,
+ (u32)td->qtd_dma,
hc32_to_cpup(ehci, &td->hw_buf[0]));
if (size < temp)
temp = size;
--
2.6.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: f_fs: avoid race condition with ffs_epfile_io_complete
On Tue, Dec 29 2015, changbin...@intel.com wrote:
> From: "Du, Changbin"
>
> ffs_epfile_io and ffs_epfile_io_complete runs in different context, but
> there is no synchronization between them.
>
> consider the following scenario:
> 1) ffs_epfile_io interrupted by sigal while
> wait_for_completion_interruptible
> 2) then ffs_epfile_io set ret to -EINTR
> 3) just before or during usb_ep_dequeue, the request completed
> 4) ffs_epfile_io return with -EINTR
>
> In this case, ffs_epfile_io tell caller no transfer success but actually
> it may has been done. This break the caller's pipe.
>
> Below script can help test it (adbd is the process which lies on f_fs).
> while true
> do
>pkill -19 adbd #SIGSTOP
>pkill -18 adbd #SIGCONT
>sleep 0.1
> done
>
> To avoid this, just dequeue the request first. After usb_ep_dequeue, the
> request must be done or canceled.
>
> With this change, we can ensure no race condition in f_fs driver. But
> actually I found some of the udc driver has analogical issue in its
> dequeue implementation. For example,
> 1) the dequeue function hold the controller's lock.
> 2) before driver request controller to stop transfer, a request
>completed.
> 3) the controller trigger a interrupt, but its irq handler need wait
>dequeue function to release the lock.
> 4) dequeue function give back the request with negative status, and
>release lock.
> 5) irq handler get lock but the request has already been given back.
>
> So, the dequeue implementation should take care of this case. IMO, it
> can be done as below steps to dequeue a already started request,
> 1) request controller to stop transfer on the given ep. HW know the
>actual transfer status.
> 2) after hw stop transfer, driver scan if there are any completed one.
> 3) if found, process it with real status. if no, the request can
>canceled.
>
> Signed-off-by: Du, Changbin
Acked-by: Michal Nazarewicz
While reviewing this patch I found two other bugs in ffs_epfile_io and
some more opportunities to refactor the code.
As soon as the kernel compiles I’ll send a patch set which will include
a modified version of this patch which has a smaller difference.
> ---
> drivers/usb/gadget/function/f_fs.c | 45
> --
> 1 file changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index cf43e9e..8050939 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -687,6 +687,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct
> ffs_io_data *io_data)
> struct ffs_ep *ep;
> char *data = NULL;
> ssize_t ret, data_len = -EINVAL;
> + bool interrupted = false;
> int halt;
>
> /* Are we still active? */
> @@ -829,26 +830,35 @@ static ssize_t ffs_epfile_io(struct file *file, struct
> ffs_io_data *io_data)
>
> spin_unlock_irq(&epfile->ffs->eps_lock);
>
> - if (unlikely(ret < 0)) {
> - /* nop */
> - } else if (unlikely(
> + if (unlikely(ret < 0))
> + goto error_mutex;
> +
> + if (unlikely(
> wait_for_completion_interruptible(&done))) {
> - ret = -EINTR;
> - usb_ep_dequeue(ep->ep, req);
> - } else {
> /*
> - * XXX We may end up silently droping data
> - * here. Since data_len (i.e. req->length) may
> - * be bigger than len (after being rounded up
> - * to maxpacketsize), we may end up with more
> - * data then user space has space for.
> + * To avoid race condition with
> + * ffs_epfile_io_complete, dequeue the request
> + * first then check status. usb_ep_dequeue API
> + * should guarantee no race condition with
> + * req->complete callback.
>*/
> - ret = ep->status;
> - if (io_data->read && ret > 0) {
> - ret = copy_to_iter(data, ret,
> &io_data->data);
> - if (!ret)
> - ret = -EFAULT;
> - }
> + usb_ep_dequeue(ep->ep, req);
> + interrupted = true;
> + }
> +
> + /*
> + * XXX We may end up silently droping data
> + * here. Since data_len (i.e. req->length) may
> + * be b
Re: [linux-sunxi] Re: [PATCH v2 1/3] reset: Add shared reset_control_[de]assert variants
Am Samstag, den 19.12.2015, 11:55 +0100 schrieb Hans de Goede: > On 18-12-15 12:08, Maxime Ripard wrote: [...] > > I guess we could also have something like this: > > > >* The driver gets the reference to the reset line using > > reset_control_get or its shared variant. > > > > - If you call reset_control_get on a free line, it succeeds, and > >marks the line in exclusive use. > > - If you call reset_control_get on a busy line, it fails with > >EBUSY > > > > - If you call the shared variant on a free line, it succeeds > > - If you call the shared variant on a busy exclusive line, it > >fails with EBUSY > > - If you call the shared variant on a busy !exclusive line, it > >succeeds. >> > >* The customer driver can then call reset_control_assert / deassert: > > > > - If the reset line is in exclusive use, the assertion happens > >right away, it succeeds and returns 0. > > > > - If the reset line is in a !exclusive use, but with a single > >user, the assertion happens right away, it succeeds and returns > >0. > > Ack for all of the above, this is what I had in mind at first, but I started > with a more lightweight version as I'm lazy :) If Philipp likes this > suggestion I can rework my patch-set into this. Seconded, this all sounds good to me. > > - If the reset line is in a !exclusive use with more than 1 user, > >the refcount is modified and an error is returned to notify that > >it didn't happen. > > Also ack, except for returning the error, if a driver has used > reset_control_get_shared, it should simply be aware that doing an assert > might not necessarily actually assert the line, just like doing a clk-disable > does not really necessary disable the clock, etc. If a driver is not prepared > to deal with this, it should simply not use reset_control_get_shared. > > I see returning an error if the assert did not happen due to other users / > deassert_count != 0 as inconsistent compared to how clks, regulators and phys > handle this, these all simply return success in this case. I wouldn't want drivers to have to differentiate between relevant and irrelevant error codes, so in the clock-like refcounting use case reset_assert should not return an error if it just correctly decremented the refcount. I'd still prefer to have separate API for the counted must_deassert/may_assert vs the exclusive must_assert/must_deassert use cases, but I just can't think of a good name. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison
On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> This patch fixes an unsigned comparison to less than 0.
No, it doesn't. It changes an unsigned comparison for less than or
equal to 0, which is very different from less than 0.
> Signed-off-by: Geyslan G. Bem
> ---
>
> Notes:
> I'm not sure about that comparison because in qh_lines() temp receives
> the snprintf() return and thereafter occurs this comparison:
>
> if (size < temp)
> temp = size;
>
> Is it a test of string truncation right? That possibility is being
> treated. But if after some snprintf returns the temp is exactly size
> minus 1 (trailing null)? Could this scenario happen? If yes, I think
> size could be not counting correctly. Let me know more about it.
I think the two weird code sequences in qh_lines() were written before
scnprintf existed. They should be changed to use scnprintf instead of
snprintf; then the "if (size < temp) temp = size;" things can be
removed.
> drivers/usb/host/ehci-dbg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 980ca55..1645120 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer *buf)
> next += temp;
>
> list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
> - if (size <= 0)
> + if (size == 0)
> break;
> qh_lines(ehci, qh, &next, &size);
> }
The new line does exactly the same thing as the old line. There's no
reason to make this change.
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 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> This patch fixes coding style issues reported by checkpatch concerning
> to unnecessary space after a cast.
This is a case where checkpatch is wrong, IMO. Casts should always be
followed by a space. I will not accept this patch.
This must be something recently added to checkpatch. It never used to
complain about casts, whether they were followed by a space or not.
Alan Stern
> Signed-off-by: Geyslan G. Bem
> ---
> drivers/usb/host/ehci-dbg.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 1645120..edae79e 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -123,7 +123,7 @@ dbg_qh(const char *label, struct ehci_hcd *ehci, struct
> ehci_qh *qh)
>
> ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
> qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
> - dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
> + dbg_qtd("overlay", ehci, (struct ehci_qtd *)&hw->hw_qtd_next);
> }
>
> static void __maybe_unused
> @@ -491,7 +491,7 @@ static void qh_lines(
> (scratch >> 16) & 0x7fff,
> scratch,
> td->urb,
> - (u32) td->qtd_dma,
> + (u32)td->qtd_dma,
> hc32_to_cpup(ehci, &td->hw_buf[0]));
> if (size < temp)
> temp = size;
>
--
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 09/17] usb: host: ehci-dbg: fix up function definitions
On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> Functions must have the opening brace at the beginning of the next line
> and body conforming indentation.
This isn't necessary if the function is an empty static inline void
routine.
Alan Stern
> This patch also reduces qh_lines() header definition to two lines.
>
> Signed-off-by: Geyslan G. Bem
> ---
> drivers/usb/host/ehci-dbg.c | 44 +---
> 1 file changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index edae79e..a365d9d 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -54,7 +54,9 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char
> *label)
> }
> #else
>
> -static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
> +static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
> +{
> +}
>
> #endif
>
> @@ -94,7 +96,9 @@ static void dbg_hcc_params(struct ehci_hcd *ehci, char
> *label)
> }
> #else
>
> -static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
> +static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
> +{
> +}
>
> #endif
>
> @@ -284,23 +288,32 @@ dbg_port_buf(char *buf, unsigned len, const char
> *label, int port, u32 status)
> #else
> static inline void __maybe_unused
> dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
> -{}
> +{
> +}
>
> static inline int __maybe_unused
> dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
> -{ return 0; }
> +{
> + return 0;
> +}
>
> static inline int __maybe_unused
> dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
> -{ return 0; }
> +{
> + return 0;
> +}
>
> static inline int __maybe_unused
> dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
> -{ return 0; }
> +{
> + return 0;
> +}
>
> static inline int __maybe_unused
> dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32
> status)
> -{ return 0; }
> +{
> + return 0;
> +}
>
> #endif /* CONFIG_DYNAMIC_DEBUG */
>
> @@ -327,8 +340,13 @@ dbg_port_buf(char *buf, unsigned len, const char *label,
> int port, u32 status)
>
> #ifdef STUB_DEBUG_FILES
>
> -static inline void create_debug_files(struct ehci_hcd *bus) { }
> -static inline void remove_debug_files(struct ehci_hcd *bus) { }
> +static inline void create_debug_files(struct ehci_hcd *bus)
> +{
> +}
> +
> +static inline void remove_debug_files(struct ehci_hcd *bus)
> +{
> +}
>
> #else
>
> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci,
> __hc32 token)
> return '/';
> }
>
> -static void qh_lines(
> - struct ehci_hcd *ehci,
> - struct ehci_qh *qh,
> - char **nextp,
> - unsigned *sizep
> -)
> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
> + char **nextp, unsigned *sizep)
> {
> u32 scratch;
> u32 hw_curr;
>
--
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 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function
On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> This patch fixes a coding style issue reported by checkpatch related to
> many leading tabs, removing a 'do while' loop and making use of goto tag
> instead.
This is highly questionable. It's a big amount of code churn, nearly
impossible to verify visually, just to remove one level of indentation.
It also introduces an unnecessary backwards "goto", which seems like a
bad idea.
Alan Stern
> Others changes in this patch are:
> - Some multiline statements are reduced (718, 729, 780, 786, 790).
> - A constant is moved to right on line 770.
>
> Signed-off-by: Geyslan G. Bem
> ---
>
> Notes:
> Tested by compilation only.
>
> drivers/usb/host/ehci-dbg.c | 180
> ++--
> 1 file changed, 88 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 2268756..278333d 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
> *buf)
>*/
> spin_lock_irqsave(&ehci->lock, flags);
> for (i = 0; i < ehci->periodic_size; i++) {
> + struct ehci_qh_hw *hw;
> +
> p = ehci->pshadow[i];
> if (likely(!p.ptr))
> continue;
> @@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct
> debug_buffer *buf)
> size -= temp;
> next += temp;
>
> - do {
> - struct ehci_qh_hw *hw;
> -
> - switch (hc32_to_cpu(ehci, tag)) {
> - case Q_TYPE_QH:
> - hw = p.qh->hw;
> - temp = scnprintf(next, size, " qh%d-%04x/%p",
> - p.qh->ps.period,
> - hc32_to_cpup(ehci,
> - &hw->hw_info2)
> - /* uframe masks */
> - & (QH_CMASK | QH_SMASK),
> - p.qh);
> - size -= temp;
> - next += temp;
> - /* don't repeat what follows this qh */
> - for (temp = 0; temp < seen_count; temp++) {
> - if (seen[temp].ptr != p.ptr)
> +do_loop:
> + switch (hc32_to_cpu(ehci, tag)) {
> + case Q_TYPE_QH:
> + hw = p.qh->hw;
> + temp = scnprintf(next, size, " qh%d-%04x/%p",
> + p.qh->ps.period,
> + hc32_to_cpup(ehci, &hw->hw_info2)
> + /* uframe masks */
> + & (QH_CMASK | QH_SMASK),
> + p.qh);
> + size -= temp;
> + next += temp;
> + /* don't repeat what follows this qh */
> + for (temp = 0; temp < seen_count; temp++) {
> + if (seen[temp].ptr != p.ptr)
> + continue;
> + if (p.qh->qh_next.ptr) {
> + temp = scnprintf(next, size, " ...");
> + size -= temp;
> + next += temp;
> + }
> + break;
> + }
> + /* show more info the first time around */
> + if (temp == seen_count) {
> + u32 scratch = hc32_to_cpup(ehci,
> + &hw->hw_info1);
> + struct ehci_qtd *qtd;
> + char*type = "";
> +
> + /* count tds, get ep direction */
> + temp = 0;
> + list_for_each_entry(qtd,
> + &p.qh->qtd_list,
> + qtd_list) {
> + temp++;
> + switch ((hc32_to_cpu(ehci,
> + qtd->hw_token) >> 8)
> + & 0x03) {
> + case 0:
> + type = "out";
> + continue;
> + case 1:
> + type = "in";
> continue;
> - if (p.
Re: [linux-sunxi] Re: [PATCH v2 1/3] reset: Add shared reset_control_[de]assert variants
On 4 January 2016 at 21:39, Philipp Zabel wrote: > Am Samstag, den 19.12.2015, 11:55 +0100 schrieb Hans de Goede: >> On 18-12-15 12:08, Maxime Ripard wrote: >> > - If the reset line is in a !exclusive use with more than 1 user, >> >the refcount is modified and an error is returned to notify that >> >it didn't happen. >> >> Also ack, except for returning the error, if a driver has used >> reset_control_get_shared, it should simply be aware that doing an assert >> might not necessarily actually assert the line, just like doing a clk-disable >> does not really necessary disable the clock, etc. If a driver is not prepared >> to deal with this, it should simply not use reset_control_get_shared. >> >> I see returning an error if the assert did not happen due to other users / >> deassert_count != 0 as inconsistent compared to how clks, regulators and phys >> handle this, these all simply return success in this case. > > I wouldn't want drivers to have to differentiate between relevant and > irrelevant error codes, so in the clock-like refcounting use case > reset_assert should not return an error if it just correctly decremented > the refcount. I'd still prefer to have separate API for the counted > must_deassert/may_assert vs the exclusive must_assert/must_deassert use > cases, but I just can't think of a good name. > Maybe something along the lines of assert_now or assert_sync. It should be possible to call on shared line and then get an error when the operation is blocked by other user. The driver may not really care. Depending on the hardware the line can be shared on one device and exclusive on another. The driver may just let the line go when the device is powered off. And it may require a reset cycle when it detects the device is hosed and return an error when the reset fails for whatever reason. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/17] usb: host: ehci-dbg: fix unsigned comparison
2016-01-04 17:50 GMT-03:00 Alan Stern :
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> This patch fixes an unsigned comparison to less than 0.
>
> No, it doesn't. It changes an unsigned comparison for less than or
> equal to 0, which is very different from less than 0.
You're right. The statemant is incomplete.
>
>> Signed-off-by: Geyslan G. Bem
>> ---
>>
>> Notes:
>> I'm not sure about that comparison because in qh_lines() temp receives
>> the snprintf() return and thereafter occurs this comparison:
>>
>> if (size < temp)
>> temp = size;
>>
>> Is it a test of string truncation right? That possibility is being
>> treated. But if after some snprintf returns the temp is exactly size
>> minus 1 (trailing null)? Could this scenario happen? If yes, I think
>> size could be not counting correctly. Let me know more about it.
>
> I think the two weird code sequences in qh_lines() were written before
> scnprintf existed. They should be changed to use scnprintf instead of
> snprintf; then the "if (size < temp) temp = size;" things can be
> removed.
I see. I can do another patch for that if you allow me.
>
>> drivers/usb/host/ehci-dbg.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index 980ca55..1645120 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer
>> *buf)
>> next += temp;
>>
>> list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
>> - if (size <= 0)
>> + if (size == 0)
>> break;
>> qh_lines(ehci, qh, &next, &size);
>> }
>
> The new line does exactly the same thing as the old line. There's no
> reason to make this change.
I think that the original and new logic will be the same because the
size variable has no sign. If in some previous subtraction the
subtracted value is greater than size value, this will spin (rotate),
probably, to a great positive value.
The compiled code will not change indeed. That change was only focused
on the improvement of the code reading. So if you allow me I could
change the commit message. If not let's forget it. :-)
>
> Alan Stern
>
--
Regards,
Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
2016-01-04 17:58 GMT-03:00 Alan Stern :
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> This patch fixes coding style issues reported by checkpatch concerning
>> to unnecessary space after a cast.
>
> This is a case where checkpatch is wrong, IMO. Casts should always be
> followed by a space. I will not accept this patch.
Ok. I understand.
>
> This must be something recently added to checkpatch. It never used to
> complain about casts, whether they were followed by a space or not.
I'm using the checkpatch --strict option.
>
> Alan Stern
>
>> Signed-off-by: Geyslan G. Bem
>> ---
>> drivers/usb/host/ehci-dbg.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index 1645120..edae79e 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -123,7 +123,7 @@ dbg_qh(const char *label, struct ehci_hcd *ehci, struct
>> ehci_qh *qh)
>>
>> ehci_dbg(ehci, "%s qh %p n%08x info %x %x qtd %x\n", label,
>> qh, hw->hw_next, hw->hw_info1, hw->hw_info2, hw->hw_current);
>> - dbg_qtd("overlay", ehci, (struct ehci_qtd *) &hw->hw_qtd_next);
>> + dbg_qtd("overlay", ehci, (struct ehci_qtd *)&hw->hw_qtd_next);
>> }
>>
>> static void __maybe_unused
>> @@ -491,7 +491,7 @@ static void qh_lines(
>> (scratch >> 16) & 0x7fff,
>> scratch,
>> td->urb,
>> - (u32) td->qtd_dma,
>> + (u32)td->qtd_dma,
>> hc32_to_cpup(ehci, &td->hw_buf[0]));
>> if (size < temp)
>> temp = size;
>>
>
--
Regards,
Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] usb: f_fs: fix ffs_epfile_io returning success on req alloc failure
In the AIO path, if allocating of a request failse, the function simply
goes to the error_lock path whose end result is returning value of ret.
However, at this point ret’s value is zero (assigned as return value from
ffs_mutex_lock).
Fix by adding ‘ret = -ENOMEM’ statement.
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_fs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index d1a4a86..1384220 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -793,8 +793,10 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
if (io_data->aio) {
req = usb_ep_alloc_request(ep->ep, GFP_KERNEL);
- if (unlikely(!req))
+ if (unlikely(!req)) {
+ ret = -ENOMEM;
goto error_lock;
+ }
req->buf = data;
req->length = data_len;
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] usb: f_fs: refactor ffs_epfile_io
Eliminate one of the return paths by using a ‘goto error_mutex’ and
rearrange some if-bodies which results in reduction of the indention level
and thus hopefully makes the function easier to read and reason about.
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_fs.c | 127 +
1 file changed, 58 insertions(+), 69 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index c4e6395..63fe693 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -684,6 +684,7 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep,
static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
{
struct ffs_epfile *epfile = file->private_data;
+ struct usb_request *req;
struct ffs_ep *ep;
char *data = NULL;
ssize_t ret, data_len = -EINVAL;
@@ -713,7 +714,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
if (!halt) {
/*
* if we _do_ wait above, the epfile->ffs->gadget might be NULL
-* before the waiting completes, so do not assign to 'gadget'
earlier
+* before the waiting completes, so do not assign to 'gadget'
+* earlier
*/
struct usb_gadget *gadget = epfile->ffs->gadget;
size_t copied;
@@ -755,17 +757,12 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
if (epfile->ep != ep) {
/* In the meantime, endpoint got disabled or changed. */
ret = -ESHUTDOWN;
- goto error_lock;
} else if (halt) {
/* Halt */
if (likely(epfile->ep == ep) && !WARN_ON(!ep->ep))
usb_ep_set_halt(ep->ep);
- spin_unlock_irq(&epfile->ffs->eps_lock);
ret = -EBADMSG;
- } else {
- /* Fire the request */
- struct usb_request *req;
-
+ } else if (unlikely(data_len == -EINVAL)) {
/*
* Sanity Check: even though data_len can't be used
* uninitialized at the time I write this comment, some
@@ -777,82 +774,74 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
* For such reason, we're adding this redundant sanity check
* here.
*/
- if (unlikely(data_len == -EINVAL)) {
- WARN(1, "%s: data_len == -EINVAL\n", __func__);
- ret = -EINVAL;
- goto error_lock;
- }
-
- if (io_data->aio) {
- req = usb_ep_alloc_request(ep->ep, GFP_KERNEL);
- if (unlikely(!req)) {
- ret = -ENOMEM;
- goto error_lock;
- }
-
- req->buf = data;
- req->length = data_len;
+ WARN(1, "%s: data_len == -EINVAL\n", __func__);
+ ret = -EINVAL;
+ } else if (!io_data->aio) {
+ DECLARE_COMPLETION_ONSTACK(done);
- io_data->buf = data;
- io_data->ep = ep->ep;
- io_data->req = req;
- io_data->ffs = epfile->ffs;
+ req = ep->req;
+ req->buf = data;
+ req->length = data_len;
- req->context = io_data;
- req->complete = ffs_epfile_async_io_complete;
+ req->context = &done;
+ req->complete = ffs_epfile_io_complete;
- ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
- if (unlikely(ret)) {
- usb_ep_free_request(ep->ep, req);
- goto error_lock;
- }
- ret = -EIOCBQUEUED;
+ ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
+ if (unlikely(ret < 0))
+ goto error_lock;
- spin_unlock_irq(&epfile->ffs->eps_lock);
- } else {
- DECLARE_COMPLETION_ONSTACK(done);
+ spin_unlock_irq(&epfile->ffs->eps_lock);
- req = ep->req;
- req->buf = data;
- req->length = data_len;
+ if (unlikely(wait_for_completion_interruptible(&done))) {
+ ret = -EINTR;
+ usb_ep_dequeue(ep->ep, req);
+ goto error_mutex;
+ }
- req->context = &done;
- req->complete = ffs_epfile_io_complete;
+ /*
+* XXX We may end up silen
[PATCH 1/5] usb: f_fs: fix memory leak when ep changes during transfer
In the ffs_epfile_io function, data buffer is allocated for non-halt
requests. Later, after grabing a mutex, the function checks that
epfile->ep is still ep and if it’s not, it set ret to -ESHUTDOWN and
follow a path including spin_unlock_irq (just after ‘ret = -ESHUTDOWN’),
mutex_unlock (after if-else-if-else chain) and returns ret. Noticeably,
this does not include freeing of the data buffer.
Fix by introducing a goto which moves control flow to the the end of the
function where spin_unlock_irq, mutex_unlock and kfree are all called.
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_fs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index cf43e9e..d1a4a86 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -763,7 +763,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
if (epfile->ep != ep) {
/* In the meantime, endpoint got disabled or changed. */
ret = -ESHUTDOWN;
- spin_unlock_irq(&epfile->ffs->eps_lock);
+ goto error_lock;
} else if (halt) {
/* Halt */
if (likely(epfile->ep == ep) && !WARN_ON(!ep->ep))
--
2.6.0.rc2.230.g3dd15c0
--
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 5/5] usb: f_fs: avoid race condition with ffs_epfile_io_complete
From: "Du, Changbin"
ffs_epfile_io and ffs_epfile_io_complete runs in different context, but
there is no synchronization between them.
consider the following scenario:
1) ffs_epfile_io interrupted by sigal while
wait_for_completion_interruptible
2) then ffs_epfile_io set ret to -EINTR
3) just before or during usb_ep_dequeue, the request completed
4) ffs_epfile_io return with -EINTR
In this case, ffs_epfile_io tell caller no transfer success but actually
it may has been done. This break the caller's pipe.
Below script can help test it (adbd is the process which lies on f_fs).
while true
do
pkill -19 adbd #SIGSTOP
pkill -18 adbd #SIGCONT
sleep 0.1
done
To avoid this, just dequeue the request first. After usb_ep_dequeue, the
request must be done or canceled.
With this change, we can ensure no race condition in f_fs driver. But
actually I found some of the udc driver has analogical issue in its
dequeue implementation. For example,
1) the dequeue function hold the controller's lock.
2) before driver request controller to stop transfer, a request
completed.
3) the controller trigger a interrupt, but its irq handler need wait
dequeue function to release the lock.
4) dequeue function give back the request with negative status, and
release lock.
5) irq handler get lock but the request has already been given back.
So, the dequeue implementation should take care of this case. IMO, it
can be done as below steps to dequeue a already started request,
1) request controller to stop transfer on the given ep. HW know the
actual transfer status.
2) after hw stop transfer, driver scan if there are any completed one.
3) if found, process it with real status. if no, the request can
canceled.
Signed-off-by: "Du, Changbin"
[min...@mina86.com: rebased on top of refactoring patches]
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_fs.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 63fe693..8cfce10 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -778,6 +778,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
ret = -EINVAL;
} else if (!io_data->aio) {
DECLARE_COMPLETION_ONSTACK(done);
+ bool interrupted = false;
req = ep->req;
req->buf = data;
@@ -793,9 +794,14 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
spin_unlock_irq(&epfile->ffs->eps_lock);
if (unlikely(wait_for_completion_interruptible(&done))) {
- ret = -EINTR;
+ /*
+* To avoid race condition with ffs_epfile_io_complete,
+* dequeue the request first then check
+* status. usb_ep_dequeue API should guarantee no race
+* condition with req->complete callback.
+*/
usb_ep_dequeue(ep->ep, req);
- goto error_mutex;
+ interrupted = ep->status < 0;
}
/*
@@ -804,7 +810,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
* rounded up to maxpacketsize), we may end up with more data
* then user space has space for.
*/
- ret = ep->status;
+ ret = interrupted ? -EINTR : ep->status;
if (io_data->read && ret > 0) {
ret = copy_to_iter(data, ret, &io_data->data);
if (!ret)
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] usb: f_fs: replace unnecessary goto with a return
In ffs_epfile_io error label points to a return path which includes
a kfree(data) call. However, at the beginning of the function data is
always NULL so some of the early ‘goto error’ can safely be replaced
with a trivial return statement.
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_fs.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 1384220..c4e6395 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -690,32 +690,24 @@ static ssize_t ffs_epfile_io(struct file *file, struct
ffs_io_data *io_data)
int halt;
/* Are we still active? */
- if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) {
- ret = -ENODEV;
- goto error;
- }
+ if (WARN_ON(epfile->ffs->state != FFS_ACTIVE))
+ return -ENODEV;
/* Wait for endpoint to be enabled */
ep = epfile->ep;
if (!ep) {
- if (file->f_flags & O_NONBLOCK) {
- ret = -EAGAIN;
- goto error;
- }
+ if (file->f_flags & O_NONBLOCK)
+ return -EAGAIN;
ret = wait_event_interruptible(epfile->wait, (ep = epfile->ep));
- if (ret) {
- ret = -EINTR;
- goto error;
- }
+ if (ret)
+ return -EINTR;
}
/* Do we halt? */
halt = (!io_data->read == !epfile->in);
- if (halt && epfile->isoc) {
- ret = -EINVAL;
- goto error;
- }
+ if (halt && epfile->isoc)
+ return -EINVAL;
/* Allocate & copy */
if (!halt) {
--
2.6.0.rc2.230.g3dd15c0
--
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 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
On Mon, Jan 04, 2016 at 06:40:38PM -0300, Geyslan G. Bem wrote: > 2016-01-04 17:58 GMT-03:00 Alan Stern : > > On Mon, 4 Jan 2016, Geyslan G. Bem wrote: > > > >> This patch fixes coding style issues reported by checkpatch concerning > >> to unnecessary space after a cast. > > > > This is a case where checkpatch is wrong, IMO. Casts should always be > > followed by a space. I will not accept this patch. > Ok. I understand. > > > > > This must be something recently added to checkpatch. It never used to > > complain about casts, whether they were followed by a space or not. > I'm using the checkpatch --strict option. Please never use that on existing kernel code, except for drivers/staging/ stuff, otherwise your patches will start to very quickly be ignored. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
Hello. On 01/05/2016 12:49 AM, Greg Kroah-Hartman wrote: This patch fixes coding style issues reported by checkpatch concerning to unnecessary space after a cast. This is a case where checkpatch is wrong, IMO. Casts should always be followed by a space. I will not accept this patch. Ok. I understand. This must be something recently added to checkpatch. It never used to complain about casts, whether they were followed by a space or not. I'm using the checkpatch --strict option. Please never use that on existing kernel code, except for drivers/staging/ stuff, otherwise your patches will start to very quickly be ignored. Just wanted to remind everybody that this option is forced when checking the networking code... thanks, greg k-h MBR, 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 07/17] usb: host: ehci-dbg: fix unsigned comparison
On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> 2016-01-04 17:50 GMT-03:00 Alan Stern :
> > On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
> >
> >> This patch fixes an unsigned comparison to less than 0.
> >
> > No, it doesn't. It changes an unsigned comparison for less than or
> > equal to 0, which is very different from less than 0.
> You're right. The statemant is incomplete.
>
> >
> >> Signed-off-by: Geyslan G. Bem
> >> ---
> >>
> >> Notes:
> >> I'm not sure about that comparison because in qh_lines() temp receives
> >> the snprintf() return and thereafter occurs this comparison:
> >>
> >> if (size < temp)
> >> temp = size;
> >>
> >> Is it a test of string truncation right? That possibility is being
> >> treated. But if after some snprintf returns the temp is exactly size
> >> minus 1 (trailing null)? Could this scenario happen? If yes, I think
> >> size could be not counting correctly. Let me know more about it.
> >
> > I think the two weird code sequences in qh_lines() were written before
> > scnprintf existed. They should be changed to use scnprintf instead of
> > snprintf; then the "if (size < temp) temp = size;" things can be
> > removed.
> I see. I can do another patch for that if you allow me.
Sure, go ahead.
> >> drivers/usb/host/ehci-dbg.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> >> index 980ca55..1645120 100644
> >> --- a/drivers/usb/host/ehci-dbg.c
> >> +++ b/drivers/usb/host/ehci-dbg.c
> >> @@ -542,7 +542,7 @@ static ssize_t fill_async_buffer(struct debug_buffer
> >> *buf)
> >> next += temp;
> >>
> >> list_for_each_entry(qh, &ehci->async_unlink, unlink_node) {
> >> - if (size <= 0)
> >> + if (size == 0)
> >> break;
> >> qh_lines(ehci, qh, &next, &size);
> >> }
> >
> > The new line does exactly the same thing as the old line. There's no
> > reason to make this change.
> I think that the original and new logic will be the same because the
> size variable has no sign. If in some previous subtraction the
> subtracted value is greater than size value, this will spin (rotate),
> probably, to a great positive value.
>
> The compiled code will not change indeed. That change was only focused
> on the improvement of the code reading. So if you allow me I could
> change the commit message. If not let's forget it. :-)
IMO there's no improvement in reading the code. So let's forget about
this.
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 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
2016-01-04 18:52 GMT-03:00 Sergei Shtylyov : > Hello. > > On 01/05/2016 12:49 AM, Greg Kroah-Hartman wrote: > > This patch fixes coding style issues reported by checkpatch concerning > to unnecessary space after a cast. This is a case where checkpatch is wrong, IMO. Casts should always be followed by a space. I will not accept this patch. >>> >>> Ok. I understand. >>> This must be something recently added to checkpatch. It never used to complain about casts, whether they were followed by a space or not. >>> >>> I'm using the checkpatch --strict option. >> >> >> Please never use that on existing kernel code, except for >> drivers/staging/ stuff, otherwise your patches will start to very >> quickly be ignored. Good to know. Tks. > > >Just wanted to remind everybody that this option is forced when checking > the networking code... Ditto. > >> thanks, >> >> greg k-h > > > MBR, Sergei > -- Regards, Geyslan G. Bem hackingbits.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/17] usb: host: ehci-dbg: fix up function definitions
2016-01-04 18:00 GMT-03:00 Alan Stern :
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> Functions must have the opening brace at the beginning of the next line
>> and body conforming indentation.
>
> This isn't necessary if the function is an empty static inline void
> routine.
Ok. It's more related to style. Let's forget it.
>
> Alan Stern
>
>> This patch also reduces qh_lines() header definition to two lines.
>>
>> Signed-off-by: Geyslan G. Bem
>> ---
>> drivers/usb/host/ehci-dbg.c | 44
>> +---
>> 1 file changed, 29 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index edae79e..a365d9d 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -54,7 +54,9 @@ static void dbg_hcs_params(struct ehci_hcd *ehci, char
>> *label)
>> }
>> #else
>>
>> -static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label) {}
>> +static inline void dbg_hcs_params(struct ehci_hcd *ehci, char *label)
>> +{
>> +}
>>
>> #endif
>>
>> @@ -94,7 +96,9 @@ static void dbg_hcc_params(struct ehci_hcd *ehci, char
>> *label)
>> }
>> #else
>>
>> -static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label) {}
>> +static inline void dbg_hcc_params(struct ehci_hcd *ehci, char *label)
>> +{
>> +}
>>
>> #endif
>>
>> @@ -284,23 +288,32 @@ dbg_port_buf(char *buf, unsigned len, const char
>> *label, int port, u32 status)
>> #else
>> static inline void __maybe_unused
>> dbg_qh(char *label, struct ehci_hcd *ehci, struct ehci_qh *qh)
>> -{}
>> +{
>> +}
>>
>> static inline int __maybe_unused
>> dbg_status_buf(char *buf, unsigned len, const char *label, u32 status)
>> -{ return 0; }
>> +{
>> + return 0;
>> +}
>>
>> static inline int __maybe_unused
>> dbg_command_buf(char *buf, unsigned len, const char *label, u32 command)
>> -{ return 0; }
>> +{
>> + return 0;
>> +}
>>
>> static inline int __maybe_unused
>> dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable)
>> -{ return 0; }
>> +{
>> + return 0;
>> +}
>>
>> static inline int __maybe_unused
>> dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32
>> status)
>> -{ return 0; }
>> +{
>> + return 0;
>> +}
>>
>> #endif /* CONFIG_DYNAMIC_DEBUG */
>>
>> @@ -327,8 +340,13 @@ dbg_port_buf(char *buf, unsigned len, const char
>> *label, int port, u32 status)
>>
>> #ifdef STUB_DEBUG_FILES
>>
>> -static inline void create_debug_files(struct ehci_hcd *bus) { }
>> -static inline void remove_debug_files(struct ehci_hcd *bus) { }
>> +static inline void create_debug_files(struct ehci_hcd *bus)
>> +{
>> +}
>> +
>> +static inline void remove_debug_files(struct ehci_hcd *bus)
>> +{
>> +}
>>
>> #else
>>
>> @@ -404,12 +422,8 @@ static inline char token_mark(struct ehci_hcd *ehci,
>> __hc32 token)
>> return '/';
>> }
>>
>> -static void qh_lines(
>> - struct ehci_hcd *ehci,
>> - struct ehci_qh *qh,
>> - char **nextp,
>> - unsigned *sizep
>> -)
>> +static void qh_lines(struct ehci_hcd *ehci, struct ehci_qh *qh,
>> + char **nextp, unsigned *sizep)
>> {
>> u32 scratch;
>> u32 hw_curr;
>>
>
And about that style? Should be done?
--
Regards,
Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer function
2016-01-04 18:01 GMT-03:00 Alan Stern :
> On Mon, 4 Jan 2016, Geyslan G. Bem wrote:
>
>> This patch fixes a coding style issue reported by checkpatch related to
>> many leading tabs, removing a 'do while' loop and making use of goto tag
>> instead.
>
> This is highly questionable. It's a big amount of code churn, nearly
> impossible to verify visually, just to remove one level of indentation.
> It also introduces an unnecessary backwards "goto", which seems like a
> bad idea.
After hear you I agree. I saw that others drivers uses similar
structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
would be the case in this file of moving code to a new function? If
not, please disregard this patch.
>
> Alan Stern
>
>> Others changes in this patch are:
>> - Some multiline statements are reduced (718, 729, 780, 786, 790).
>> - A constant is moved to right on line 770.
>>
>> Signed-off-by: Geyslan G. Bem
>> ---
>>
>> Notes:
>> Tested by compilation only.
>>
>> drivers/usb/host/ehci-dbg.c | 180
>> ++--
>> 1 file changed, 88 insertions(+), 92 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
>> index 2268756..278333d 100644
>> --- a/drivers/usb/host/ehci-dbg.c
>> +++ b/drivers/usb/host/ehci-dbg.c
>> @@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer
>> *buf)
>>*/
>> spin_lock_irqsave(&ehci->lock, flags);
>> for (i = 0; i < ehci->periodic_size; i++) {
>> + struct ehci_qh_hw *hw;
>> +
>> p = ehci->pshadow[i];
>> if (likely(!p.ptr))
>> continue;
>> @@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct
>> debug_buffer *buf)
>> size -= temp;
>> next += temp;
>>
>> - do {
>> - struct ehci_qh_hw *hw;
>> -
>> - switch (hc32_to_cpu(ehci, tag)) {
>> - case Q_TYPE_QH:
>> - hw = p.qh->hw;
>> - temp = scnprintf(next, size, " qh%d-%04x/%p",
>> - p.qh->ps.period,
>> - hc32_to_cpup(ehci,
>> - &hw->hw_info2)
>> - /* uframe masks */
>> - & (QH_CMASK |
>> QH_SMASK),
>> - p.qh);
>> - size -= temp;
>> - next += temp;
>> - /* don't repeat what follows this qh */
>> - for (temp = 0; temp < seen_count; temp++) {
>> - if (seen[temp].ptr != p.ptr)
>> +do_loop:
>> + switch (hc32_to_cpu(ehci, tag)) {
>> + case Q_TYPE_QH:
>> + hw = p.qh->hw;
>> + temp = scnprintf(next, size, " qh%d-%04x/%p",
>> + p.qh->ps.period,
>> + hc32_to_cpup(ehci, &hw->hw_info2)
>> + /* uframe masks */
>> + & (QH_CMASK | QH_SMASK),
>> + p.qh);
>> + size -= temp;
>> + next += temp;
>> + /* don't repeat what follows this qh */
>> + for (temp = 0; temp < seen_count; temp++) {
>> + if (seen[temp].ptr != p.ptr)
>> + continue;
>> + if (p.qh->qh_next.ptr) {
>> + temp = scnprintf(next, size, " ...");
>> + size -= temp;
>> + next += temp;
>> + }
>> + break;
>> + }
>> + /* show more info the first time around */
>> + if (temp == seen_count) {
>> + u32 scratch = hc32_to_cpup(ehci,
>> + &hw->hw_info1);
>> + struct ehci_qtd *qtd;
>> + char*type = "";
>> +
>> + /* count tds, get ep direction */
>> + temp = 0;
>> + list_for_each_entry(qtd,
>> + &p.qh->qtd_list,
>> + qtd_list) {
>> + temp++;
>> + switch ((hc32_to_cpu(ehci,
>> + qtd->hw_token) >> 8)
>> + & 0x03) {
>>
[PATCH v6 00/11] usbip: features to USB over WebSocket
Dear all, This series of patches introduces WebSocket to USB/IP. 0. Version info v6) # Added __rcu annotation to a RCU pointer to clear sparse warnings. # Corrected a copy to RCU pointer with rcu_rcu_assign_pointer(). # Added __user annotations to arguments of read/write method. # Added static to some functions which are not called from other files. # Removed unnecessary EXPORT_SYMBOLs. v5) # Added vendor/pruduct name conversion to port command. # Put initial value to pool_head in name.c. # Fixed list command exception when host option is omitted. # Fixed exception in case gai_strerror() returns NULL. # Fixed WebSocket connection close via proxy. # Fixed to stop WebSocket ping-pong on connection close. # Removed redundant usbipd daemon option. # Removed redundant SSL code had not been deleted. # Removed an unused local variable in WebSocket code. # Modified C++ reserved word in names.c as same as headers. v4) # Fixed regression of usbip list --remote v3) # Coding style for goto err labels are fixed. # Defined magic numbers for open_hc_device() argument. # Corrected include .../uapi/linux/usbip_ux.h as . # Modified parameter notation in manuals not to use '='. # Fixed inappropriate version definition in tools/.../websocket/configure.ac. # Remved unnecessary COPYING and AUTHORS fil from tools/.../websocket/. # Added -version-info to libraries in tools/.../src. v2) # Formatted patches from linux-next. # Fixed change log word wrapping. # Removed SSL patches. # Fixed a bug that vendor and product names are not shown by 'usbws list -l' because usbip_names_init() was not called in libusbip.la. 1. Features included It also includes some independent features effective in themselves. 1) Exporting devices Export request and response PDU had been defined in a header but not been used. Now it works! Also, it supports senarios, for example, connect ubiquetous devices to a Linux based cloud service. In this senario, it's needed to establish connection from a device inside of firewall to a service outside. Exporting is suit for the senario. 2) User space transmission USB/IP transfer URBs in kernel space. It's better for performance but difficult to introduce application protocols. Like fuse for file systems, it allows to transfer URBs in user space. When usbip_ux.ko is loaded, it replaces kernel_sendmsg() and kernel_recvmsg() with user spcace interface. When USB/IP utilities find usbip_ux.ko, they start threads to read/write PDUs from/to usbip_ux.ko and send/recv them. 3) Replaceable protocols Both transmission(send/receive) and connection establishment are replaceable. 4) a WebSocket implementation It's made with Poco C++. DNS and proxy client are supported. I published scripts I used while developed the patches. http://linux-usbip-additions.blogspot.jp/2015/03/scripts-to-patch-and-ma ke-locally.html http://linux-usbip-additions.blogspot.jp/2015/03/test-scripts.html 2. Why WebSocket? It allows to use USB/IP in internet. WebSocket is widely used to encapsulate packets in HTTP and to carry them through firewall using HTTP port numbers. Assumed use case is a system that service in internet serves distributes devices in home or office networks. Service may be called as cloud and devices as ubiquitous. Home/SOHO/IntranetInternet ++++ +--+ +--+ |Router, ||Internet| +|device|---|Linux |-|proxy, ||service | |+--+ +--+ |firewall||on Linux| +--+ controller++++ ex) Device Service sensors ... environment analysis cameras ... monitoring, recording ID/biometric readers .. authentication 3. Why userspace transmission? Userspace transmission and APIs provided by this series allow to apply application protocols to USB/IP. Why not use usbfs or libusb? a) Not only device(usbip-host) side, application(vhci-hcd) side must be handled. b) In device side, if using usbfs or libusb, many parts of usbip-common and usbip-host driver must be copied to userspace. It's not good for maintainability. Tunneling daemons can wrap TCP/IP with application protocol. They pass packets through loopback so this series has certain advantage regarding performance. It's important for small (IoT) devices. 4. Why exporting devices? Connection from outside firewall is usually blocked. So existing import request sent with attach command doesn't work. # usbipd (blocked)|| <- # usbip attach Firewall opens some ports, usually HTTP(80) and HTTPS(443), from inside. Then export request sent with new connect command works. # usbip connect -> # usbipa (passed) Thank you, Nobuo Iwata // *** BLURB HERE *** Nobuo Iwata (11): usbip:
[PATCH v6 02/11] usbip: readme and manuals about exporting devices
This patch adds function and usage of export to README and manuals. The wording, 'server' and 'client' is changed also. For existing attach command, the daemon runs device side machine and attach command is executed in application side machine. Then 'server' is used for device side and 'client' is for application side. For the new connect command, the daemon runs applications side machine and connect command is executed in device side machine. Now, 'server' and 'client' run in different machine than before. So, to avoid confusion, words 'device side' and 'application side' are used instead of 'client' and 'server'. Signed-off-by: Nobuo Iwata --- tools/usb/usbip/Makefile.am | 2 +- tools/usb/usbip/README | 70 +--- tools/usb/usbip/doc/usbip.8 | 82 +--- tools/usb/usbip/doc/usbipa.8 | 77 ++ tools/usb/usbip/doc/usbipd.8 | 29 +- tools/usb/usbip/libsrc/vhci_driver.c | 2 +- 6 files changed, 205 insertions(+), 57 deletions(-) diff --git a/tools/usb/usbip/Makefile.am b/tools/usb/usbip/Makefile.am index 66f8bf0..f371ed9 100644 --- a/tools/usb/usbip/Makefile.am +++ b/tools/usb/usbip/Makefile.am @@ -3,4 +3,4 @@ includedir = @includedir@/usbip include_HEADERS := $(addprefix libsrc/, \ usbip_common.h vhci_driver.h usbip_host_driver.h) -dist_man_MANS := $(addprefix doc/, usbip.8 usbipd.8) +dist_man_MANS := $(addprefix doc/, usbip.8 usbipd.8 usbipa.8) diff --git a/tools/usb/usbip/README b/tools/usb/usbip/README index 831f49f..74f4afb 100644 --- a/tools/usb/usbip/README +++ b/tools/usb/usbip/README @@ -1,7 +1,8 @@ # # README for usbip-utils # -# Copyright (C) 2011 matt mooney +# Copyright (C) 2015 Nobuo Iwata +# 2011 matt mooney # 2005-2008 Takahiro Hirofuchi @@ -36,41 +37,70 @@ [Usage] -server:# (Physically attach your USB device.) +Device-side: a machine has USB device(s). +Application-side: a machine runs an application software uses remote USB device. -server:# insmod usbip-core.ko -server:# insmod usbip-host.ko +1) Connect from application-side to device-side. -server:# usbipd -D +dev:# (Physically attach your USB device.) + +dev:# insmod usbip-core.ko +dev:# insmod usbip-host.ko + +dev:# usbipd -D - Start usbip daemon. -server:# usbip list -l - - List driver assignments for USB devices. +dev:# usbip list -l + - List driver assignments for USB devices and their busid. -server:# usbip bind --busid 1-2 - - Bind usbip-host.ko to the device with busid 1-2. - - The USB device 1-2 is now exportable to other hosts! - - Use `usbip unbind --busid 1-2' to stop exporting the device. +dev:# usbip bind --busid + - Bind usbip-host.ko to the device with . + - The USB device with is now exportable to other hosts! + - Use `usbip unbind --busid ` to stop exporting the device. -client:# insmod usbip-core.ko -client:# insmod vhci-hcd.ko +app:# insmod usbip-core.ko +app:# insmod vhci-hcd.ko -client:# usbip list --remote +app:# usbip list --remote - List exported USB devices on the . -client:# usbip attach --remote --busid 1-2 +app:# usbip attach --remote --busid - Connect the remote USB device. -client:# usbip port +app:# usbip port - Show virtual port status. -client:# usbip detach --port +app:# usbip detach --port - Detach the USB device. +2) Connect from device-side to application-side. + +app:# insmod usbip-core.ko +app:# insmod vhci-hcd.ko + +app:# usbipa -D + - Start usbip daemon. + +dev:# (Physically attach your USB device.) + +dev:# insmod usbip-core.ko +dev:# insmod usbip-host.ko + +dev:# usbip list -l + - List driver assignments for USB devices and their busid. + +dev:# usbip connect --remote --busid + - Bind usbip-host.ko to the device with . + - The USB device of is connected to remote host! + +dev:# usbip disconnect --remote --busid + - The USB device with is disconnected from remote host. + - Unbind usbip-host.ko from the device. + [Example] --- - SERVER SIDE + DEVICE SIDE --- Physically attach your USB devices to this host. @@ -131,7 +161,7 @@ Mark the device of busid 3-3.2 as exportable: ... --- - CLIENT SIDE + APPLICATION SIDE --- First, let's list available remote devices that are marked as exportable on the host. @@ -170,7 +200,7 @@ Attach a remote USB device: deux:# usbip attach --remote 10.0.0.3 --busid 1-1 port 0 attached -Show the devices attached to this client: +Show the devices attached to this machine: deux:# usbip port Port 00: at Full Speed(12Mbps) diff --git
[PATCH v6 03/11] usbip: safe completion against usb_kill_urb()
stub_shutdown_connection() : drivers/usb/usbip/stub_dev.c
stub_device_cleanup_urbs() : drivers/usb/usbip/stub_main.c
requests to kill pending URBs and clears priv lists.
stub_complete() : drivers/usb/usbip/stub_tx.c might be called with URBs
to have been requested to kill.
To avoid kernel panic, this patch ignores killed URBs linked to cleared
priv lists.
To know the killed URBs in stub_complete(), sdev->ud.tcp_socket which
cleared before stub_device_cleanup_urbs() is checked.
The critial condition will happen by unbind command before detach,
broken connection in connect command and disconnect command.
Signed-off-by: Nobuo Iwata
---
drivers/usb/usbip/stub_tx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c
index dbcabc9..f19f321 100644
--- a/drivers/usb/usbip/stub_tx.c
+++ b/drivers/usb/usbip/stub_tx.c
@@ -97,7 +97,9 @@ void stub_complete(struct urb *urb)
/* link a urb to the queue of tx. */
spin_lock_irqsave(&sdev->priv_lock, flags);
- if (priv->unlinking) {
+ if (sdev->ud.tcp_socket == NULL) {
+ dev_info(&urb->dev->dev, "discard a urb for closed connection");
+ } else if (priv->unlinking) {
stub_enqueue_ret_unlink(sdev, priv->seqnum, urb->status);
stub_free_priv_and_urb(priv);
} else {
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 01/11] usbip: exporting devices
USB/IP supports a function to import USB devices from application-side
machine by attach command.
The usage is as following.
dev:# (Physically attach your USB device.)
dev:# insmod usbip-core.ko and usbip-host.ko
dev:# usbipd -D
// Start usbip daemon.
dev:# usbip list -l
// List local USB devices and their busid.
dev:# usbip bind --busid
// Make a device exportable to other hosts.
app:# insmod usbip-core.ko and vhci-hcd.ko
app:# usbip list --remote
// List importable USB devices from the .
app:# usbip attach --remote --busid
// Import a device
By attach command, connection will be established from application-side
to device-side.
This patch introduces a function to export devices form device-side
machine to application-side machine.
The usage is as following.
app:# insmod usbip-core.ko and vhci-hcd.ko
app:# usbipa -D
// Start usbip daemon.
dev:# (Physically attach your USB device.)
dev:# insmod usbip-core.ko and usbip-host.ko
dev:# usbip list -l
// List local USB devices and their busid.
dev:# usbip connect --remote --busid
// Export a device to .
For this, export function, connection is established from device-side
machine to application-side machine.
Following use cases are supposed for the export function.
1) Server application or cloud service serves distributed ubiquitous
devices.
2) Dedicate devices to server application or cloud service.
To connect to cloud service, it needs to connect from inside of
firewall.
Probably, the export function was planned because the packets have been
defined in a header file (usbip_network.h) but it not yet used.
This patch fixes the defined packet structures (ie. int in reply to
uinit32_t) and use them.
Also, vendor/product name converion is added to port commnad as same as
list command.
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/libsrc/names.c | 4 +-
tools/usb/usbip/libsrc/usbip_host_driver.c | 16 ++
tools/usb/usbip/libsrc/usbip_host_driver.h | 1 +
tools/usb/usbip/libsrc/vhci_driver.c | 127 +--
tools/usb/usbip/libsrc/vhci_driver.h | 8 +-
tools/usb/usbip/src/Makefile.am| 9 +-
tools/usb/usbip/src/usbip.c| 17 +-
tools/usb/usbip/src/usbip.h| 11 +-
tools/usb/usbip/src/usbip_attach.c | 49 +---
tools/usb/usbip/src/usbip_bind.c | 7 +-
tools/usb/usbip/src/usbip_connect.c| 214 ++
tools/usb/usbip/src/usbip_detach.c | 13 +-
tools/usb/usbip/src/usbip_disconnect.c | 202 +
tools/usb/usbip/src/usbip_list.c | 22 +-
tools/usb/usbip/src/usbip_network.h| 5 +-
tools/usb/usbip/src/usbip_port.c | 21 +-
tools/usb/usbip/src/usbip_unbind.c | 7 +-
tools/usb/usbip/src/usbipd.c | 233 +++
tools/usb/usbip/src/usbipd_app.c | 240
tools/usb/usbip/src/usbipd_dev.c | 247 +
20 files changed, 1151 insertions(+), 302 deletions(-)
diff --git a/tools/usb/usbip/libsrc/names.c b/tools/usb/usbip/libsrc/names.c
index 81ff852..7d65d28 100644
--- a/tools/usb/usbip/libsrc/names.c
+++ b/tools/usb/usbip/libsrc/names.c
@@ -162,7 +162,7 @@ struct pool {
void *mem;
};
-static struct pool *pool_head;
+static struct pool *pool_head = NULL;
static void *my_malloc(size_t size)
{
@@ -201,6 +201,8 @@ void names_free(void)
pool = pool->next;
free(tmp);
}
+
+ pool_head = NULL;
}
static int new_vendor(const char *name, u_int16_t vendorid)
diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c
b/tools/usb/usbip/libsrc/usbip_host_driver.c
index bef08d5..de5541a 100644
--- a/tools/usb/usbip/libsrc/usbip_host_driver.c
+++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
@@ -278,3 +278,19 @@ struct usbip_exported_device *usbip_host_get_device(int
num)
return NULL;
}
+
+struct usbip_exported_device *usbip_host_find_device(char *busid)
+{
+ struct list_head *i;
+ struct usbip_exported_device *edev;
+
+ list_for_each(i, &host_driver->edev_list) {
+ edev = list_entry(i, struct usbip_exported_device, node);
+ if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE)) {
+ return edev;
+ }
+ }
+
+ return NULL;
+}
+
diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.h
b/tools/usb/usbip/libsrc/usbip_host_driver.h
index 2a31f85..69c65a6 100644
--- a/tools/usb/usbip/libsrc/usbip_host_driver.h
+++ b/tools/usb/usbip/libsrc/usbip_host_driver.h
@@ -45,5 +45,6 @@ void usbip_host_driver_close(void);
int usbip_host_refresh_device_list(void);
int usbip_host_export_device(struct usbip_exported_device *edev, int sockfd);
struct usbip_exported_device *usbip_host_get_device(int num);
+struct usbip_exported_device *usbip_host_find_device(char *busid);
#endif /* __USBIP_HOST_DRIVER_H */
diff --git a/tools/usb/usbip/libsrc/vhci_driver.c
b/tools
[PATCH v6 04/11] usbip: kernel module for userspace URBs transmission
Originally, USB/IP transmits requests and response PDUs for preparation to transfer URBs in user space, after the preparation, URBs are transmitted in kernel space. To make easy to introduce application network protocols like WebSocket and so on, the driver, usbip_ux.ko, forwards URBs to USB/IP user space utilities. It's just like fuse driver for user space file system. Then, utilities transfer URBs in user space. To do so, usbip_trx_ops makes send/receive functions pluggable. kernel_sendmsg() and kernel_recvmsg() for kernel mode transfer can be substituted by read/write methods to user space utilities. In the absence of usbip_ux.ko, original kernel space transferring is valid. usbip_ux.ko replaces usbip_trx_ops in its init routine. A) Original - kernel space URBs transfer User+---+ 1) import/export +---+ space |uspipd,|<>|usbip, | |usbip | |usbipa | +---+---+ +---+---+ | | 2)Set sockfd| |2)Set sockfd thru sysfs| | thru sysfs V V Kernel +---+4)URBs+---+ space |usbip |<>|vhci | |host.ko| |hcd.ko | +---+ +---+ 3)link to kernel trx_ops 3)link to kernel trx_ops B) New - user space URBs transfer User+---+1)import/export +---+ space |uspipd,|<>|usbip, | +---|usbip |<>|usbipa |---+ 2)Set sockfd|+--+---+6)URBs+---+--+|2)Set sockfd thru ioctl|| ^ ^ || thru ioctl (right) || |5)read/write 5)read/write| || (left) 3)Set sockfd|| +---+ +--+ ||3)Set Sockfd thru sysfs|+---+ | /dev/usbip-ux| +---+| thru sysfs (left)VV V V VV (right) Kernel +---+ +---+ +---+ +---+ space |usbip |<->|usbip | |usbip |<->|vhci | |host.ko|5)send |ux.ko | |ux.ko |5)send |hcd.ko | +---+ recv +---+ +---+ recv +---+ 4)link to user trx_ops 4)link to user trx_ops Kernel module configuration for the driver will be shown as below. USB/IP support USBIP_CORE +-- USB/IP userspace URB transmission USBIP_UX +-- VHCI hcdUSBIP_HCD +-- Debug messages for USB/IP USBIP_DEBUG The reason why the userspace transmission oter than usbfs is needed. a) Application(vhci_hcd)-side is needed Usbfs provides functions to control device. So it can be applied to device(usbip_host)-side but not to application(vhci_hcd)-side. b) Use existing kernel modules as-is To implement same functionality in userspace with interface like usbfs, almost same code to kernel modules must be copied to userspcae. Also interfaces between kernel modules and utiities (sysfs) should be changed to new one. So utilities must be modified according to the new interface too. Modifications to existing code by this patch is small and usbip_ux.c handles major part of userspace transmission. To get diff include/uapi/linux/usbip_ux.h, I used modified dontdiff.txt. Signed-off-by: Nobuo Iwata --- drivers/usb/usbip/Kconfig| 10 + drivers/usb/usbip/Makefile | 3 + drivers/usb/usbip/stub_dev.c | 16 +- drivers/usb/usbip/stub_rx.c | 3 +- drivers/usb/usbip/stub_tx.c | 5 +- drivers/usb/usbip/usbip_common.c | 79 +++- drivers/usb/usbip/usbip_common.h | 29 +- drivers/usb/usbip/usbip_ux.c | 598 +++ drivers/usb/usbip/usbip_ux.h | 82 + drivers/usb/usbip/vhci_hcd.c | 9 +- drivers/usb/usbip/vhci_rx.c | 3 +- drivers/usb/usbip/vhci_sysfs.c | 40 ++- drivers/usb/usbip/vhci_tx.c | 6 +- include/uapi/linux/usbip_ux.h| 39 ++ 14 files changed, 865 insertions(+), 57 deletions(-) diff --git a/drivers/usb/usbip/Kconfig b/drivers/usb/usbip/Kconfig index bd99e9e..e847d06 100644 --- a/drivers/usb/usbip/Kconfig +++ b/drivers/usb/usbip/Kconfig @@ -14,6 +14,16 @@ config USBIP_CORE If unsure, say N. +config USBIP_UX + tristate "USB/IP userspace URB transmission" + depends on USBIP_CORE + ---help--- + This moves USB/IP URB transmission to userspace + to apply SSL, WebSocket and etc. + + To compile this driver as a module, choose M here: the + module will be called usbip-ux. + config USBIP_VHCI_HCD tristate "VHCI hcd" depends on USBIP_CORE diff --gi
[PATCH v6 09/11] usbip: deriving functions as libraries
To utilize core parts of USB/IP to application protocol implementations, this patch derives libraries by exposing some functions of utilities and removing some unnecessary portions. Following functions are exposed. For command: - usbip_attach_device() - usbip_detach_port() - usbip_bind_device() - usbip_unbind_device() - usbip_list_imported_devices() : port command - usbip_list_importable_devices() : list --remote - usbip_list_devices() : list --local - usbip_connect_device() - usbip_disconnect_device() For daemon: - usbip_recv_pdu() - processes accepted a connection - usbip_break_connections() - breaks send/receive threads - usbip_driver_open() - open host or vhci driver - usbip_driver_close() - close host or vhci driver main() and option processing are removed. AS_LIBRARY macro is used to remove option processing. Following libraries are generated. - libusbip.la : for commnad - libusbipa.la : for application-side daemon - libusbipd.la : for device-side daemon Succeeding WebSocket patch uses these libraries. Signed-off-by: Nobuo Iwata --- tools/usb/usbip/libsrc/Makefile.am | 10 +-- tools/usb/usbip/src/Makefile.am| 22 +- tools/usb/usbip/src/usbip.h| 7 ++ tools/usb/usbip/src/usbip_attach.c | 14 ++-- tools/usb/usbip/src/usbip_bind.c | 6 ++ tools/usb/usbip/src/usbip_connect.c| 12 +++- tools/usb/usbip/src/usbip_detach.c | 15 ++-- tools/usb/usbip/src/usbip_disconnect.c | 12 +++- tools/usb/usbip/src/usbip_list.c | 70 +++--- tools/usb/usbip/src/usbip_netconn.c| 98 ++ tools/usb/usbip/src/usbip_network.c| 67 -- tools/usb/usbip/src/usbip_port.c | 9 ++- tools/usb/usbip/src/usbip_unbind.c | 6 ++ tools/usb/usbip/src/usbipd.c | 13 +--- tools/usb/usbip/src/usbipd.h | 38 ++ tools/usb/usbip/src/usbipd_app.c | 1 + tools/usb/usbip/src/usbipd_dev.c | 1 + 17 files changed, 274 insertions(+), 127 deletions(-) diff --git a/tools/usb/usbip/libsrc/Makefile.am b/tools/usb/usbip/libsrc/Makefile.am index 5754425..356a6c0 100644 --- a/tools/usb/usbip/libsrc/Makefile.am +++ b/tools/usb/usbip/libsrc/Makefile.am @@ -1,9 +1,9 @@ -libusbip_la_CPPFLAGS = -DUSBIDS_FILE='"@USBIDS_DIR@/usb.ids"' -libusbip_la_CFLAGS = @EXTRA_CFLAGS@ -libusbip_la_LDFLAGS = -version-info @LIBUSBIP_VERSION@ +libusbiplib_la_CPPFLAGS = -DUSBIDS_FILE='"@USBIDS_DIR@/usb.ids"' +libusbiplib_la_CFLAGS = @EXTRA_CFLAGS@ +libusbiplib_la_LDFLAGS = -version-info @LIBUSBIP_VERSION@ -lib_LTLIBRARIES := libusbip.la -libusbip_la_SOURCES := names.c names.h usbip_host_driver.c usbip_host_driver.h \ +lib_LTLIBRARIES := libusbiplib.la +libusbiplib_la_SOURCES := names.c names.h usbip_host_driver.c usbip_host_driver.h \ usbip_common.c usbip_common.h vhci_driver.c vhci_driver.h \ usbip_ux.c usbip_ux.h \ sysfs_utils.c sysfs_utils.h diff --git a/tools/usb/usbip/src/Makefile.am b/tools/usb/usbip/src/Makefile.am index f5697c2..780bdb3 100644 --- a/tools/usb/usbip/src/Makefile.am +++ b/tools/usb/usbip/src/Makefile.am @@ -1,14 +1,32 @@ AM_CPPFLAGS = -I$(top_srcdir)/libsrc -DUSBIDS_FILE='"@USBIDS_DIR@/usb.ids"' AM_CFLAGS = @EXTRA_CFLAGS@ -LDADD = $(top_builddir)/libsrc/libusbip.la +LDADD = $(top_builddir)/libsrc/libusbiplib.la sbin_PROGRAMS := usbip usbipd usbipa +lib_LTLIBRARIES := libusbip.la libusbipd.la libusbipa.la -usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \ +usbip_SOURCES := usbip.h utils.h usbip.c utils.c \ +usbip_network.c usbip_netconn.c\ usbip_attach.c usbip_detach.c usbip_list.c \ usbip_bind.c usbip_unbind.c usbip_port.c \ usbip_connect.c usbip_disconnect.c +usbip_CFLAGS := $(AM_CFLAGS) usbipd_SOURCES := usbip_network.h usbipd.c usbipd_dev.c usbip_network.c +usbipd_CFLAGS := $(AM_CFLAGS) usbipa_SOURCES := usbip_network.h usbipd.c usbipd_app.c usbip_network.c +usbipa_CFLAGS := $(AM_CFLAGS) + +libusbip_la_SOURCES := utils.h utils.c usbip_network.c \ +usbip_attach.c usbip_detach.c usbip_list.c \ +usbip_bind.c usbip_unbind.c usbip_port.c \ +usbip_connect.c usbip_disconnect.c +libusbip_la_CFLAGS := $(AM_CFLAGS) -DAS_LIBRARY +libusbip_la_LDFLAGS := -version-info @LIBUSBIP_VERSION@ + +libusbipd_la_SOURCES := usbipd_dev.c usbip_network.c +libusbipd_la_LDFLAGS := -version-info @LIBUSBIP_VERSION@ + +libusbipa_la_SOURCES := usbipd_app.c usbip_network.c +libusbipa_la_LDFLAGS := -version-info @LIBUSBIP_VERSION@ diff --git a/tools/usb/usbip/src/usbip.h b/tools/usb/usbip/src/usbip.h index 0875d15..1d642cc 100644 --- a/tools/usb/usbip/src/usbip.h +++ b/tools/usb/usbip/src/usbip.h @@ -34,8 +34,15 @@ int usbip_port_show(int argc, char *argv[]); int usbip_connect(int argc, char *argv[]); int usbip_disconnect(int argc, char *argv[]); +int usbip_attach_device(char *
[PATCH v6 11/11] usbip: USB over WebSocket
This patch adds utilities transmit packets via WebSocket protocol. WebSocket version of utilities as following. usbws : command usbwsa : application-side daemon usbwsd : device-side daemon The command supports all sub-command (ie. list, connect, disconnect, port, bind, unbind, attach and detach). It uses --url option to specify remote address and port number. Implementation of this patch depends on Poco C++ (http://pocoproject.org/). The tree is shown below. tools +--usb +--usbip +--src : command, daemons and their core libraries +--libsrc : common library for command and daemon +--websocket : new! WebSocket implementations +--poco : new! implementation with Poco C++ Signed-off-by: Nobuo Iwata --- tools/usb/usbip/websocket/INSTALL | 237 ++ tools/usb/usbip/websocket/Makefile.am | 3 + tools/usb/usbip/websocket/README | 184 tools/usb/usbip/websocket/autogen.sh | 9 + tools/usb/usbip/websocket/cleanup.sh | 12 + tools/usb/usbip/websocket/configure.ac| 55 +++ tools/usb/usbip/websocket/doc/usbws.8 | 192 tools/usb/usbip/websocket/doc/usbwsa.8| 101 + tools/usb/usbip/websocket/doc/usbwsd.8| 109 + tools/usb/usbip/websocket/poco/Makefile.am| 18 + .../usb/usbip/websocket/poco/USBWSCommand.cpp | 410 ++ tools/usb/usbip/websocket/poco/USBWSCommand.h | 99 + .../usb/usbip/websocket/poco/USBWSDaemon.cpp | 228 ++ tools/usb/usbip/websocket/poco/USBWSDaemon.h | 80 .../websocket/poco/USBWSRequestHandler.cpp| 90 .../websocket/poco/USBWSRequestHandler.h | 49 +++ .../poco/USBWSRequestHandlerFactory.cpp | 49 +++ .../poco/USBWSRequestHandlerFactory.h | 48 ++ tools/usb/usbip/websocket/poco/USBWSUtil.h| 52 +++ .../usbip/websocket/poco/USBWSWebSocket.cpp | 204 + .../usb/usbip/websocket/poco/USBWSWebSocket.h | 69 +++ 21 files changed, 2298 insertions(+) diff --git a/tools/usb/usbip/websocket/INSTALL b/tools/usb/usbip/websocket/INSTALL new file mode 100644 index 000..d3c5b40 --- /dev/null +++ b/tools/usb/usbip/websocket/INSTALL @@ -0,0 +1,237 @@ +Installation Instructions +* + +Copyright (C) 1994, 1995, 1996, 1999, 2000, 2001, 2002, 2004, 2005, +2006, 2007 Free Software Foundation, Inc. + +This file is free documentation; the Free Software Foundation gives +unlimited permission to copy, distribute and modify it. + +Basic Installation +== + +Briefly, the shell commands `./configure; make; make install' should +configure, build, and install this package. The following +more-detailed instructions are generic; see the `README' file for +instructions specific to this package. + + The `configure' shell script attempts to guess correct values for +various system-dependent variables used during compilation. It uses +those values to create a `Makefile' in each directory of the package. +It may also create one or more `.h' files containing system-dependent +definitions. Finally, it creates a shell script `config.status' that +you can run in the future to recreate the current configuration, and a +file `config.log' containing compiler output (useful mainly for +debugging `configure'). + + It can also use an optional file (typically called `config.cache' +and enabled with `--cache-file=config.cache' or simply `-C') that saves +the results of its tests to speed up reconfiguring. Caching is +disabled by default to prevent problems with accidental use of stale +cache files. + + If you need to do unusual things to compile the package, please try +to figure out how `configure' could check whether to do them, and mail +diffs or instructions to the address given in the `README' so they can +be considered for the next release. If you are using the cache, and at +some point `config.cache' contains results you don't want to keep, you +may remove or edit it. + + The file `configure.ac' (or `configure.in') is used to create +`configure' by a program called `autoconf'. You need `configure.ac' if +you want to change it or regenerate `configure' using a newer version +of `autoconf'. + +The simplest way to compile this package is: + + 1. `cd' to the directory containing the package's source code and type + `./configure' to configure the package for your system. + + Running `configure' might take a while. While running, it prints + some messages telling which features it is checking for. + + 2. Type `make' to compile the package. + + 3. Optionally, type `make check' to run any self-tests that come with + the package. + + 4. Type `make install' to install the programs and any data files and + documentation. + + 5. You can remove the program binaries and object files from the + source code directory by typing `make clean'. To also remove the + files that `
[PATCH v6 10/11] usbip: added const qualifier to arguments of some functions
This patch adds 'const' qualifier to 'char*' arguments of library
interfaces to make acceptable std::string.c_str(). Essentially, these
qualifiers are better to be used even if not to use C++. Although, I
just added to functions related to previous patch.
Also, it changes C++ reserved words (ie. new and class) in list.h.
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/libsrc/list.h | 24 --
tools/usb/usbip/libsrc/names.c | 15 +++---
tools/usb/usbip/libsrc/usbip_common.c | 16 +++
tools/usb/usbip/libsrc/usbip_common.h | 6 +++---
tools/usb/usbip/libsrc/usbip_host_driver.c | 10 +
tools/usb/usbip/libsrc/usbip_host_driver.h | 2 +-
tools/usb/usbip/libsrc/vhci_driver.c | 11 ++
tools/usb/usbip/libsrc/vhci_driver.h | 6 --
tools/usb/usbip/src/usbip.h| 16 ---
tools/usb/usbip/src/usbip_attach.c | 4 ++--
tools/usb/usbip/src/usbip_bind.c | 6 +++---
tools/usb/usbip/src/usbip_connect.c| 4 ++--
tools/usb/usbip/src/usbip_detach.c | 2 +-
tools/usb/usbip/src/usbip_disconnect.c | 5 +++--
tools/usb/usbip/src/usbip_list.c | 7 +++
tools/usb/usbip/src/usbip_netconn.c| 2 +-
tools/usb/usbip/src/usbip_network.c| 12 +++
tools/usb/usbip/src/usbip_unbind.c | 2 +-
tools/usb/usbip/src/usbipd.c | 4 ++--
tools/usb/usbip/src/usbipd.h | 2 +-
tools/usb/usbip/src/usbipd_app.c | 9
tools/usb/usbip/src/usbipd_dev.c | 2 +-
tools/usb/usbip/src/utils.c| 2 +-
tools/usb/usbip/src/utils.h| 2 +-
24 files changed, 94 insertions(+), 77 deletions(-)
diff --git a/tools/usb/usbip/libsrc/list.h b/tools/usb/usbip/libsrc/list.h
index 5eaaa78..b46a98f 100644
--- a/tools/usb/usbip/libsrc/list.h
+++ b/tools/usb/usbip/libsrc/list.h
@@ -36,14 +36,14 @@ static inline void INIT_LIST_HEAD(struct list_head *list)
* This is only for internal list manipulation where we know
* the prev/next entries already!
*/
-static inline void __list_add(struct list_head *new,
+static inline void __list_add(struct list_head *neo,
struct list_head *prev,
struct list_head *next)
{
- next->prev = new;
- new->next = next;
- new->prev = prev;
- prev->next = new;
+ next->prev = neo;
+ neo->next = next;
+ neo->prev = prev;
+ prev->next = neo;
}
/**
@@ -54,9 +54,9 @@ static inline void __list_add(struct list_head *new,
* Insert a new entry after the specified head.
* This is good for implementing stacks.
*/
-static inline void list_add(struct list_head *new, struct list_head *head)
+static inline void list_add(struct list_head *neo, struct list_head *head)
{
- __list_add(new, head, head->next);
+ __list_add(neo, head, head->next);
}
/*
@@ -73,8 +73,8 @@ static inline void __list_del(struct list_head * prev, struct
list_head * next)
}
#define POISON_POINTER_DELTA 0
-#define LIST_POISON1 ((void *) 0x00100100 + POISON_POINTER_DELTA)
-#define LIST_POISON2 ((void *) 0x00200200 + POISON_POINTER_DELTA)
+#define LIST_POISON1 ((char *) 0x00100100 + POISON_POINTER_DELTA)
+#define LIST_POISON2 ((char *) 0x00200200 + POISON_POINTER_DELTA)
/**
* list_del - deletes entry from list.
@@ -90,8 +90,8 @@ static inline void __list_del_entry(struct list_head *entry)
static inline void list_del(struct list_head *entry)
{
__list_del(entry->prev, entry->next);
- entry->next = LIST_POISON1;
- entry->prev = LIST_POISON2;
+ entry->next = (struct list_head *)LIST_POISON1;
+ entry->prev = (struct list_head *)LIST_POISON2;
}
/**
@@ -120,7 +120,9 @@ static inline void list_del(struct list_head *entry)
for (pos = (head)->next, n = pos->next; pos != (head); \
pos = n, n = pos->next)
+#ifndef offsetof
#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+#endif
/**
* container_of - cast a member of a structure out to the containing structure
diff --git a/tools/usb/usbip/libsrc/names.c b/tools/usb/usbip/libsrc/names.c
index 7d65d28..656005f 100644
--- a/tools/usb/usbip/libsrc/names.c
+++ b/tools/usb/usbip/libsrc/names.c
@@ -23,6 +23,8 @@
*
* Copyright (C) 2005 Takahiro Hirofuchi
* - names_deinit() is added.
+ * Copyright (C) 2015 Nobuo Iwata
+ * - some modifications for portability.
*
*/
@@ -38,7 +40,6 @@
#include
#include "names.h"
-#include "usbip_common.h"
struct vendor {
struct vendor *next;
@@ -52,8 +53,8 @@ struct product {
char name[1];
};
-struct class {
- struct class *next;
+struct clazz {
+ struct clazz *next;
u_int8_t classid;
char name[1];
};
@@ -94,7 +95,7 @@ static unsigned int hashnum(unsigned int num)
static struct vendor *vendors[HA
[PATCH v6 06/11] usbip: readme about user space URBs transmission
Addition to README regarding user space URBs transmission. Signed-off-by: Nobuo Iwata --- tools/usb/usbip/README | 22 ++ 1 file changed, 22 insertions(+) diff --git a/tools/usb/usbip/README b/tools/usb/usbip/README index 74f4afb..6b61da5 100644 --- a/tools/usb/usbip/README +++ b/tools/usb/usbip/README @@ -98,6 +98,28 @@ Application-side: a machine runs an application software uses remote USB device. - Unbind usbip-host.ko from the device. +[Userspace Transmission] + +In usage shown above, once USB devices are imported or exported, USP/IP drivers send and receive URBs in kernel space. The usbip_ux.ko kernel module alternates the route to user space by forwarding USBs through USB/IP utilities (ie. usbip, usbipd, usbipa). When userspace transmission enabled, usbip attach and connect will continue executing until usbip detach or disconnect is exeuted. + +app:# insmod usbip-core.ko +app:# insmod usbip-ux.ko +app:# insmod vhci-hcd.ko + +app:# usbipa -D + +dev:# insmod usbip-core.ko +dev:# insmod usbip-ux.ko +dev:# insmod usbip-host.ko + +dev:# usbip connect --remote --busid + - Continue running. + - Until disconnect command is executed in other terminal window. + +dev:# usbip disconnect --remote --busid + - Stops transission, quits connect command and disconnect device. + + [Example] --- DEVICE SIDE -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 08/11] usbip: letting connection establishment replaceable
To introduce some application protocols like WebSocket, this patch
allows to substitute connection establishment and termination. In
combination with previous patch, both connection and transmission can
be replaced.
usbip_connection_operations_t includes open and close operation. Open
method returns usbip_sock_t which includes send, receive and close
method. Then, transmission methods are replaced at the same time.
Succeeding WebSocket patch uses this feature.
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/libsrc/usbip_common.c | 10 ++
tools/usb/usbip/libsrc/usbip_common.h | 11 +++
tools/usb/usbip/src/usbip.c| 1 +
tools/usb/usbip/src/usbip_attach.c | 6 +++---
tools/usb/usbip/src/usbip_connect.c| 6 +++---
tools/usb/usbip/src/usbip_disconnect.c | 6 +++---
tools/usb/usbip/src/usbip_list.c | 6 +++---
tools/usb/usbip/src/usbip_network.c| 9 +++--
tools/usb/usbip/src/usbip_network.h| 3 +--
9 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/tools/usb/usbip/libsrc/usbip_common.c
b/tools/usb/usbip/libsrc/usbip_common.c
index dc0712c..54efa10 100644
--- a/tools/usb/usbip/libsrc/usbip_common.c
+++ b/tools/usb/usbip/libsrc/usbip_common.c
@@ -297,3 +297,13 @@ void usbip_sock_init(usbip_sock_t *sock, int fd, void *arg,
sock->shutdown = shutdown;
}
+usbip_connection_operations_t usbip_conn_ops = {NULL, NULL};
+
+void usbip_conn_init(
+ usbip_sock_t *(*open)(char *host, char *port),
+ void (*close)(usbip_sock_t *sock))
+{
+ usbip_conn_ops.open = open;
+ usbip_conn_ops.close = close;
+}
+
diff --git a/tools/usb/usbip/libsrc/usbip_common.h
b/tools/usb/usbip/libsrc/usbip_common.h
index 0dcbd99..07c411f 100644
--- a/tools/usb/usbip/libsrc/usbip_common.h
+++ b/tools/usb/usbip/libsrc/usbip_common.h
@@ -148,4 +148,15 @@ void usbip_sock_init(usbip_sock_t *sock, int fd, void *arg,
ssize_t (*recv)(void *arg, void *buf, size_t len, int wait_all),
void (*shutdown)(void *arg));
+typedef struct usbip_connection_operations {
+ usbip_sock_t *(*open)(char *host, char *port);
+ void (*close)(usbip_sock_t *sock);
+} usbip_connection_operations_t;
+
+extern usbip_connection_operations_t usbip_conn_ops;
+
+void usbip_conn_init(
+ usbip_sock_t *(*open)(char *host, char *port),
+ void (*close)(usbip_sock_t *sock));
+
#endif /* __USBIP_COMMON_H */
diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c
index 9d1468f..505e384 100644
--- a/tools/usb/usbip/src/usbip.c
+++ b/tools/usb/usbip/src/usbip.c
@@ -202,6 +202,7 @@ int main(int argc, char *argv[])
argc -= optind;
argv += optind;
optind = 0;
+ usbip_net_tcp_conn_init();
rc = run_command(&cmds[i], argc, argv);
goto out;
}
diff --git a/tools/usb/usbip/src/usbip_attach.c
b/tools/usb/usbip/src/usbip_attach.c
index ae0ca6e..c67e3d2 100644
--- a/tools/usb/usbip/src/usbip_attach.c
+++ b/tools/usb/usbip/src/usbip_attach.c
@@ -135,7 +135,7 @@ static int attach_device(char *host, char *busid)
int rc;
int rhport;
- sock = usbip_net_tcp_connect(host, usbip_port_string);
+ sock = usbip_conn_ops.open(host, usbip_port_string);
if (!sock) {
err("tcp connect");
goto err_out;
@@ -164,13 +164,13 @@ static int attach_device(char *host, char *busid)
usbip_ux_join(ux);
}
usbip_ux_cleanup(&ux);
- usbip_net_tcp_close(sock);
+ usbip_conn_ops.close(sock);
return 0;
err_cleanup_ux:
usbip_ux_cleanup(&ux);
err_close_conn:
- usbip_net_tcp_close(sock);
+ usbip_conn_ops.close(sock);
err_out:
return -1;
}
diff --git a/tools/usb/usbip/src/usbip_connect.c
b/tools/usb/usbip/src/usbip_connect.c
index 5f9505c..8dabd0b 100644
--- a/tools/usb/usbip/src/usbip_connect.c
+++ b/tools/usb/usbip/src/usbip_connect.c
@@ -150,7 +150,7 @@ static int connect_device(char *host, char *busid)
goto err_out;
}
- sock = usbip_net_tcp_connect(host, usbip_port_string);
+ sock = usbip_conn_ops.open(host, usbip_port_string);
if (!sock) {
err("tcp connect");
goto err_unbind_device;
@@ -174,13 +174,13 @@ static int connect_device(char *host, char *busid)
usbip_unbind_device(busid);
}
usbip_ux_cleanup(&ux);
- usbip_net_tcp_close(sock);
+ usbip_conn_ops.close(sock);
return 0;
err_cleanup_ux:
usbip_ux_cleanup(&ux);
err_close_conn:
- usbip_net_tcp_close(sock);
+ usbip_conn_ops.close(sock);
err_unbind_device:
usbip_unbind_device(busid);
err_out:
diff --git a/tools/usb/usbip/src/usbip_disconnect.c
b/tools/usb/usbip/src/usbip_disconnect.c
index 8da
[PATCH v6 05/11] usbip: tools for userspace URBs transmission
Originally, USB/IP transmits requests and response PDUs for preparation
to transfer URBs in user space, after completion of the preparation,
URBs are transmitted in kernel space.
To make easy to introduce application network protocols like WebSocket,
the driver, usbip_ux.ko, forwards URBs to USB/IP user space utilities.
Then, the utilities exchange URBs in userspace.
To do so, tools/usb/usbip/libsrc/usbip_ux.c includes tx/rx threads to
read/wite URBs from usbip_ux.ko and transfer URBs in userspace. When
usbip_ux.ko is installed, /dev/usbip-ux will be found, then the threads
will be started. Otherwise, threads will not be started and original
kernel space transmission is valid.
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/libsrc/Makefile.am | 1 +
tools/usb/usbip/libsrc/usbip_ux.c | 244 +
tools/usb/usbip/libsrc/usbip_ux.h | 30 +++
tools/usb/usbip/libsrc/vhci_driver.c | 10 +-
tools/usb/usbip/src/usbip_attach.c | 30 ++-
tools/usb/usbip/src/usbip_connect.c| 18 +-
tools/usb/usbip/src/usbip_disconnect.c | 11 +-
tools/usb/usbip/src/usbipd.c | 5 +-
tools/usb/usbip/src/usbipd_app.c | 14 ++
tools/usb/usbip/src/usbipd_dev.c | 30 ++-
10 files changed, 370 insertions(+), 23 deletions(-)
diff --git a/tools/usb/usbip/libsrc/Makefile.am
b/tools/usb/usbip/libsrc/Makefile.am
index 7c8f8a4..5754425 100644
--- a/tools/usb/usbip/libsrc/Makefile.am
+++ b/tools/usb/usbip/libsrc/Makefile.am
@@ -5,4 +5,5 @@ libusbip_la_LDFLAGS = -version-info @LIBUSBIP_VERSION@
lib_LTLIBRARIES := libusbip.la
libusbip_la_SOURCES := names.c names.h usbip_host_driver.c usbip_host_driver.h
\
usbip_common.c usbip_common.h vhci_driver.c
vhci_driver.h \
+ usbip_ux.c usbip_ux.h \
sysfs_utils.c sysfs_utils.h
diff --git a/tools/usb/usbip/libsrc/usbip_ux.c
b/tools/usb/usbip/libsrc/usbip_ux.c
new file mode 100644
index 000..3ac45a72
--- /dev/null
+++ b/tools/usb/usbip/libsrc/usbip_ux.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright (C) 2015 Nobuo Iwata
+ *
+ * USB/IP URB transmission in userspace.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include "usbip_common.h"
+#include "usbip_ux.h"
+
+#undef PROGNAME
+#define PROGNAME "libusbip"
+
+#define DEVNAME "/dev/" USBIP_UX_DEV_NAME
+
+#define BLEN 1500
+
+#ifdef DEBUG
+void dump_buff(char *buff, size_t bufflen, char *label)
+{
+#define DUMP_BUFF 80
+#define WORK_BUFF 16
+ size_t i = 0, j;
+ char b[DUMP_BUFF];
+ char bb[WORK_BUFF];
+
+ dbg("dump %s for %zd bytes", label, bufflen);
+ for(i=0;isockfd, buf, BLEN, 0);
+ if (received == 0) {
+ dbg("connection closed on sock:%p", ux->kaddr.sock);
+ break;
+ } else if (received < 0) {
+ dbg("receive error on sock:%p", ux->kaddr.sock);
+ break;
+ }
+ dump_buff(buf, received, "ux received");
+ written = 0;
+ while(written < received) {
+ ret = write(ux->devfd, buf+written, received-written);
+ if (ret < 0) {
+ dbg("write error for sock:%p", ux->kaddr.sock);
+ good = 0;
+ break;
+ }
+ written += ret;
+ }
+ }
+ dbg("end of ux-rx for sock:%p", ux->kaddr.sock);
+ ioctl(ux->devfd, USBIP_UX_IOCINTR);
+ return 0;
+}
+
+static void *usbip_ux_tx(void *arg)
+{
+ usbip_ux_t *ux = (usbip_ux_t*)arg;
+ ssize_t sent, reads;
+ char buf[BLEN];
+
+ for(;;) {
+ reads = read(ux->devfd, buf, BLEN);
+ if (reads == 0) {
+#ifdef DEBUG
+ dbg("end of read on sock:%p continue.", ux->kaddr.sock);
+#endif
+ sched_yield();
+ continue;
+ } else if (reads < 0) {
+ dbg("read error on sock:%p", ux->kaddr.sock);
+ break;
+ }
+ dump_buff(buf, reads, "ux sending");
+ sent = send(ux->sockfd, buf, reads, 0);
+ if (sent < 0) {
+ dbg("connection closed on sock:%p", ux->kaddr.sock);
+ break;
+ } else if (sent < reads) {
+ dbg("send error on sock:%p %zd < %zd", ux->kaddr.sock,
+ sent, reads);
+ break;
+ }
+ }
+ dbg("end of ux-tx for sock:%p", ux->kaddr.sock);
+ shutdown(ux->sockfd, SHUT_RDWR);
+ return 0;
+}
+
+/*
+ * Setup user space mode.
+ * Null will be set in ux if usbip_ux.ko is not installed.
+ */
+int usbip_ux_setup(int sockfd, usbip_ux_t **uxp)
+{
+ usbip_ux_t *ux;
+ int fd, ret;
+
+ *
[PATCH v6 07/11] usbip: letting send and receive replaceable
This patch allows to substitute send, receive and shutdown routines for
both a) request/response PDUs among utilities and b) user space URBs
transmission.
usbip_sock_t is introduced instead of sockfd. it includes function
pointers of send/receive/shutdown routines, an argument for the
routines, and a sockfd. The argument is needed for the routines. The
sockfd is needed to bind connection to USB device.
Succeeding SSL and WebSocket patch use this feature.
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/libsrc/usbip_common.c | 14 +
tools/usb/usbip/libsrc/usbip_common.h | 14 +
tools/usb/usbip/libsrc/usbip_ux.c | 24 ++--
tools/usb/usbip/libsrc/usbip_ux.h | 4 +-
tools/usb/usbip/src/usbip_attach.c | 30 +-
tools/usb/usbip/src/usbip_connect.c| 30 +-
tools/usb/usbip/src/usbip_disconnect.c | 26 -
tools/usb/usbip/src/usbip_list.c | 27 +
tools/usb/usbip/src/usbip_network.c| 78 +++---
tools/usb/usbip/src/usbip_network.h| 12 ++--
tools/usb/usbip/src/usbipd.c | 14 +++--
tools/usb/usbip/src/usbipd_app.c | 36 ++--
tools/usb/usbip/src/usbipd_dev.c | 40 ++---
13 files changed, 214 insertions(+), 135 deletions(-)
diff --git a/tools/usb/usbip/libsrc/usbip_common.c
b/tools/usb/usbip/libsrc/usbip_common.c
index ac73710..dc0712c 100644
--- a/tools/usb/usbip/libsrc/usbip_common.c
+++ b/tools/usb/usbip/libsrc/usbip_common.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2005-2007 Takahiro Hirofuchi
+ * Copyright (C) 2015 Nobuo Iwata
*/
#include
@@ -283,3 +284,16 @@ void usbip_names_get_class(char *buff, size_t size,
uint8_t class,
snprintf(buff, size, "%s / %s / %s (%02x/%02x/%02x)", c, s, p, class,
subclass, protocol);
}
+
+void usbip_sock_init(usbip_sock_t *sock, int fd, void *arg,
+ ssize_t (*send)(void *arg, void *buf, size_t len),
+ ssize_t (*recv)(void *arg, void *buf, size_t len, int wait_all),
+ void (*shutdown)(void *arg))
+{
+ sock->fd = fd;
+ sock->arg = arg;
+ sock->send = send;
+ sock->recv = recv;
+ sock->shutdown = shutdown;
+}
+
diff --git a/tools/usb/usbip/libsrc/usbip_common.h
b/tools/usb/usbip/libsrc/usbip_common.h
index 15fe792..0dcbd99 100644
--- a/tools/usb/usbip/libsrc/usbip_common.h
+++ b/tools/usb/usbip/libsrc/usbip_common.h
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2005-2007 Takahiro Hirofuchi
+ * Copyright (C) 2015 Nobuo Iwata
*/
#ifndef __USBIP_COMMON_H
@@ -134,4 +135,17 @@ void usbip_names_get_product(char *buff, size_t size,
uint16_t vendor,
void usbip_names_get_class(char *buff, size_t size, uint8_t class,
uint8_t subclass, uint8_t protocol);
+typedef struct usbip_sock {
+ int fd;
+ void *arg;
+ ssize_t (*send)(void *arg, void *buf, size_t len);
+ ssize_t (*recv)(void *arg, void *buf, size_t len, int wait_all);
+ void (*shutdown)(void *arg);
+} usbip_sock_t;
+
+void usbip_sock_init(usbip_sock_t *sock, int fd, void *arg,
+ ssize_t (*send)(void *arg, void *buf, size_t len),
+ ssize_t (*recv)(void *arg, void *buf, size_t len, int wait_all),
+ void (*shutdown)(void *arg));
+
#endif /* __USBIP_COMMON_H */
diff --git a/tools/usb/usbip/libsrc/usbip_ux.c
b/tools/usb/usbip/libsrc/usbip_ux.c
index 3ac45a72..aa3d863 100644
--- a/tools/usb/usbip/libsrc/usbip_ux.c
+++ b/tools/usb/usbip/libsrc/usbip_ux.c
@@ -57,7 +57,11 @@ static void *usbip_ux_rx(void *arg)
char buf[BLEN];
while(good) {
- received = recv(ux->sockfd, buf, BLEN, 0);
+ if (ux->sock->recv) {
+ received = ux->sock->recv(ux->sock->arg, buf, BLEN, 0);
+ } else {
+ received = recv(ux->sock->fd, buf, BLEN, 0);
+ }
if (received == 0) {
dbg("connection closed on sock:%p", ux->kaddr.sock);
break;
@@ -101,7 +105,11 @@ static void *usbip_ux_tx(void *arg)
break;
}
dump_buff(buf, reads, "ux sending");
- sent = send(ux->sockfd, buf, reads, 0);
+ if (ux->sock->send) {
+ sent = ux->sock->send(ux->sock->arg, buf, reads);
+ } else {
+ sent = send(ux->sock->fd, buf, reads, 0);
+ }
if (sent < 0) {
dbg("connection closed on sock:%p", ux->kaddr.sock);
break;
@@ -112,7 +120,11 @@ static void *usbip_ux_tx(void *arg)
}
}
dbg("end of ux-tx for sock:%p", ux->kaddr.sock);
- shutdown(ux->sockfd, SHUT_RDWR);
+ if (ux->sock->shutdown) {
+ ux->sock->shutdown(ux->sock->arg);
+ } else {
+ shutdown(ux->sock->fd, SHUT_RDWR);
+ }
return 0;
}
@@ -120,7 +132,7 @@ static void *usbip_ux_tx(void *arg)
* Se
[PATCH v2 0/2] usbip: vhci number of ports extension
This series of patches extends number of ports limitaion in application (vhci) side. 0. Version info v2) # Added static to some functions and variables not called from other files. 1. Overview This series conatins 2 patches. 1/2: Extends number of ports using multiple host controllers. 'num_controllers=N' module parameter denotes the number. The default is 1. Number of ports per controller are extended from 8 to USB_MAXCHILDREN(31). It can be altered with -DVHCI_NPORTS=n at compile time. 2/2: Event handling threads are used to be created for each port. This patch aggregates them to one thread. Rewritten with workqueue. Assumed use case is a system that service in internet serves distributes devices in home or office. In the use case, application side might be needed to support more ports than 31. Home/SOHO/Enterprise Intranet/Internet ++ +--+ +--+ |Service | +|device|---|Linux |---|on | |+--+ +--+ |Linux | +--+ controller ++ ex) Device Service sensors ... environment analysis cameras ... monitoring, recording ID/biometric readers .. authentication To increase number of ports, existing implementation has an overhead that event handing kernel threads are started for each port. The second patch eliminates the overhead. NOTE: This series depends on "USB/IP over WebSocket" patches. *** BLURB HERE *** Nobuo Iwata (2): usbip: vhci number of ports extension usbip: single thread event handler drivers/usb/usbip/README | 3 + drivers/usb/usbip/stub_dev.c | 3 +- drivers/usb/usbip/usbip_common.c | 7 + drivers/usb/usbip/usbip_common.h | 4 +- drivers/usb/usbip/usbip_event.c | 174 ++--- drivers/usb/usbip/usbip_ux.c | 2 +- drivers/usb/usbip/vhci.h | 41 +- drivers/usb/usbip/vhci_hcd.c | 259 - drivers/usb/usbip/vhci_rx.c | 21 +- drivers/usb/usbip/vhci_sysfs.c | 298 +++ tools/usb/usbip/libsrc/vhci_driver.c | 548 ++- tools/usb/usbip/libsrc/vhci_driver.h | 38 +- tools/usb/usbip/src/usbip_attach.c | 8 +- tools/usb/usbip/src/usbip_port.c | 13 +- tools/usb/usbip/src/usbipd_app.c | 56 ++- 15 files changed, 916 insertions(+), 559 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] usbip: single thread event handler
This patch reduces number of event handling threads to one.
In existing implementation, event kernel threads are created for each
port. The functions of the threads are terminationg connection and
error handling. It's too expensive to have to each port.
With this patch, event handler is a single thread workqueue
[usbip_event].
Both application (vhci) and device (stub) side are replaced.
Signed-off-by: Nobuo Iwata
---
drivers/usb/usbip/stub_dev.c | 3 +-
drivers/usb/usbip/usbip_common.c | 7 ++
drivers/usb/usbip/usbip_common.h | 4 +-
drivers/usb/usbip/usbip_event.c | 174 +++
drivers/usb/usbip/vhci_sysfs.c | 2 +-
5 files changed, 142 insertions(+), 48 deletions(-)
diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index d8d3add..9ec002d 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -380,7 +380,6 @@ err_files:
err_port:
dev_set_drvdata(&udev->dev, NULL);
usb_put_dev(udev);
- kthread_stop_put(sdev->ud.eh);
busid_priv->sdev = NULL;
stub_device_free(sdev);
@@ -441,7 +440,7 @@ static void stub_disconnect(struct usb_device *udev)
}
/* If usb reset is called from event handler */
- if (busid_priv->sdev->ud.eh == current)
+ if (usbip_in_eh(current))
return;
/* shutdown the current connection */
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index 963d8db..8c9fb19 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -807,12 +807,19 @@ EXPORT_SYMBOL_GPL(usbip_trx_ops);
static int __init usbip_core_init(void)
{
+ int ret;
+
pr_info(DRIVER_DESC " v" USBIP_VERSION "\n");
+ ret = usbip_init_eh();
+ if (ret) {
+ return ret;
+ }
return 0;
}
static void __exit usbip_core_exit(void)
{
+ usbip_finish_eh();
return;
}
diff --git a/drivers/usb/usbip/usbip_common.h b/drivers/usb/usbip/usbip_common.h
index ffed6fe..5e56598 100644
--- a/drivers/usb/usbip/usbip_common.h
+++ b/drivers/usb/usbip/usbip_common.h
@@ -275,7 +275,6 @@ struct usbip_device {
struct task_struct *tcp_tx;
unsigned long event;
- struct task_struct *eh;
wait_queue_head_t eh_waitq;
struct eh_ops {
@@ -321,10 +320,13 @@ void usbip_pad_iso(struct usbip_device *ud, struct urb
*urb);
int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb);
/* usbip_event.c */
+int usbip_init_eh(void);
+void usbip_finish_eh(void);
int usbip_start_eh(struct usbip_device *ud);
void usbip_stop_eh(struct usbip_device *ud);
void usbip_event_add(struct usbip_device *ud, unsigned long event);
int usbip_event_happened(struct usbip_device *ud);
+int usbip_in_eh(struct task_struct *task);
static inline int interface_to_busnum(struct usb_interface *interface)
{
diff --git a/drivers/usb/usbip/usbip_event.c b/drivers/usb/usbip/usbip_event.c
index 64933b9..9bbdf1d 100644
--- a/drivers/usb/usbip/usbip_event.c
+++ b/drivers/usb/usbip/usbip_event.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2003-2008 Takahiro Hirofuchi
+ * Copyright (C) 2015 Nobuo Iwata
*
* This is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -19,17 +20,68 @@
#include
#include
+#include
+#include
#include "usbip_common.h"
-static int event_handler(struct usbip_device *ud)
+struct usbip_event {
+ struct list_head node;
+ struct usbip_device *ud;
+};
+
+static DEFINE_SPINLOCK(event_lock);
+static LIST_HEAD(event_list);
+
+static void set_event(struct usbip_device *ud, unsigned long event)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ud->lock, flags);
+ ud->event |= event;
+ spin_unlock_irqrestore(&ud->lock, flags);
+}
+
+static void unset_event(struct usbip_device *ud, unsigned long event)
{
- usbip_dbg_eh("enter\n");
+ unsigned long flags;
+
+ spin_lock_irqsave(&ud->lock, flags);
+ ud->event &= ~event;
+ spin_unlock_irqrestore(&ud->lock, flags);
+}
+
+static struct usbip_device *get_event(void)
+{
+ struct usbip_event *ue = NULL;
+ struct usbip_device *ud = NULL;
+ unsigned long flags;
- /*
-* Events are handled by only this thread.
-*/
- while (usbip_event_happened(ud)) {
+ spin_lock_irqsave(&event_lock, flags);
+ if (!list_empty(&event_list)) {
+ ue = list_first_entry(&event_list, struct usbip_event, node);
+ list_del(&ue->node);
+ }
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ if (ue) {
+ ud = ue->ud;
+ kfree(ue);
+ }
+ return ud;
+}
+
+static struct task_struct *worker_context = NULL;
+
+static void event_handler(struct work_struct *work)
+{
+ struct usbip_device *ud;
+
+ if (worker_context == NULL
[PATCH v2 1/2] usbip: vhci number of ports extension
This patch extends number of ports limitation in application (vhci)
side.
To do so, vhci driver supports multiple host controllers. The number of
controllers can be specified as a module parameter 'num_controllers'.
The default is 1.
ex) # insmod vhci_hcd.ko num_controllers=4
Also, ports per controller is changed from 8 to USB_MAXCHILDREN (31).
It can be modified with VHCI_NPORTS flag at module compilation.
So number of ports supported by vhci is 'num_controllers' * 31.
Sysfs structure is changes as following.
BEFORE:
/sys/devices/platform
+-- vhci
+-- status
+-- attach
+-- detach
+-- usbip_debug
AFTER: example for num_controllers=4
/sys/devices/platform
+-- vhci.0
| +-- nports
| +-- status.0
| +-- status.1
| +-- status.2
| +-- status.3
| +-- attach
| +-- detach
| +-- usbip_debug
+-- vhci.1
+-- vhci.2
+-- vhci.3
vhci.N is shown for each host controller kobj. vhch.1, vhci.2, ... are
shown only when num_controllers is more than 1. Only vhci.0 has user
space interfaces. 'nports' is newly added to give ports-per-controller
and number of controlles. Before that, number of ports is acquired by
counting status lines. Status is divided for each controller to avoid
page size (4KB) limitation.
Variable wording relating port has been corrected. 'port' represents id
across multiple controllers. 'rhport (root hub port)' represents id
within a controller.
Some unimportant info level messages are changed to debug level because
they are too busy when using many ports.
NOTE: Syslog error messages "systemd-udevd[390]: error opening USB
device 'descriptors' file" may be shown. They are not caused by this
patch. It seems to be a systemd problem.
Signed-off-by: Nobuo Iwata
---
drivers/usb/usbip/README | 3 +
drivers/usb/usbip/usbip_ux.c | 2 +-
drivers/usb/usbip/vhci.h | 41 +-
drivers/usb/usbip/vhci_hcd.c | 259 -
drivers/usb/usbip/vhci_rx.c | 21 +-
drivers/usb/usbip/vhci_sysfs.c | 298 +++
tools/usb/usbip/libsrc/vhci_driver.c | 548 ++-
tools/usb/usbip/libsrc/vhci_driver.h | 38 +-
tools/usb/usbip/src/usbip_attach.c | 8 +-
tools/usb/usbip/src/usbip_port.c | 13 +-
tools/usb/usbip/src/usbipd_app.c | 56 ++-
11 files changed, 775 insertions(+), 512 deletions(-)
diff --git a/drivers/usb/usbip/README b/drivers/usb/usbip/README
index 41a2cf2..fce3d7f 100644
--- a/drivers/usb/usbip/README
+++ b/drivers/usb/usbip/README
@@ -1,3 +1,6 @@
+MODULE PARAMS:
+ - num_controllers : number of controllers. Default is 1.
+
TODO:
- more discussion about the protocol
- testing
diff --git a/drivers/usb/usbip/usbip_ux.c b/drivers/usb/usbip/usbip_ux.c
index 97e6385..4abf0b2 100644
--- a/drivers/usb/usbip/usbip_ux.c
+++ b/drivers/usb/usbip/usbip_ux.c
@@ -396,7 +396,7 @@ static int usbip_ux_unlink(struct usbip_device *ud)
rcu_read_lock();
ux = rcu_dereference(ud->ux);
if (ux == NULL) {
- pr_err("Unlink to unlinked ux.\n");
+ usbip_dbg_ux("Unlink to unlinked ux.\n");
rcu_read_unlock();
return -EINVAL;
}
diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index a863a98..6c34075 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2003-2008 Takahiro Hirofuchi
+ * Copyright (C) 2015 Nobuo Iwata
*
* This is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -72,7 +73,11 @@ struct vhci_unlink {
};
/* Number of supported ports. Value has an upperbound of USB_MAXCHILDREN */
-#define VHCI_NPORTS 8
+#ifndef VHCI_NPORTS
+#define VHCI_NPORTS USB_MAXCHILDREN
+#endif
+
+#define MAX_STATUS_NAME 16
/* for usb_bus.hcpriv */
struct vhci_hcd {
@@ -93,11 +98,16 @@ struct vhci_hcd {
struct vhci_device vdev[VHCI_NPORTS];
};
-extern struct vhci_hcd *the_controller;
-extern const struct attribute_group dev_attr_group;
+extern int num_controllers;
+extern struct platform_device **the_pdevs;
+extern struct attribute_group dev_attr_group;
/* vhci_hcd.c */
-void rh_port_connect(int rhport, enum usb_device_speed speed);
+void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed);
+
+/* vhci_sysfs.c */
+int vhci_init_attr_group(void);
+void vhci_finish_attr_group(void);
/* vhci_rx.c */
struct urb *pickup_urb_and_free_priv(struct vhci_device *vdev, __u32 seqnum);
@@ -106,9 +116,14 @@ int vhci_rx_loop(void *data);
/* vhci_tx.c */
int vhci_tx_loop(void *data);
-static inline struct vhci_device *port_to_vdev(__u32 port)
+static inline __u32 port_to_rhport(__u32 port)
+{
+ return port % VHCI_NPORTS;
+}
+
+static inline int port_to_pdev_nr(__u3
RE: [PATCH v5 04/11] usbip: kernel module for userspace URBs transmission
Dear Oliver, > You cannot honor GFP_NOIO GFP_NOIO is used in original kernel space transmission relating sk_buf. It is not used for userspace transmission. I added 'if' statement around the block to skip the block where GFP_NOIO is used for kernel space transmission. > the cleanest solution would be an additional flag to match requiring > a kernel space driver Other allocations are regular GFP_KERNEL or GFP_ATOMIC. Thank you, n.iwata // > -Original Message- > From: linux-usb-ow...@vger.kernel.org > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Oliver Neukum > Sent: Sunday, January 03, 2016 2:07 AM > To: fx IWATA NOBUO > Cc: valentina.mane...@gmail.com; shuah...@samsung.com; > gre...@linuxfoundation.org; linux-usb@vger.kernel.org; fx MICHIMURA > TADAO; Alan Stern > Subject: Re: [PATCH v5 04/11] usbip: kernel module for userspace URBs > transmission > > On Wed, 2015-12-30 at 15:10 +0900, Nobuo Iwata wrote: > > Originally, USB/IP transmits requests and response PDUs for > > preparation to transfer URBs in user space, after the preparation, > > URBs are transmitted in kernel space. > > > > To make easy to introduce application network protocols like WebSocket > > and so on, the driver, usbip_ux.ko, forwards URBs to USB/IP user space > > utilities. It's just like fuse driver for user space file system. > > Then, utilities transfer URBs in user space. > > Hi, > > this raises fundamental questions. Just like fuse you are importing > limitations this way. Fuse cannot do shared writable memory mappings because > the VM can deadlock if you drop the GFP_NOFS requirement. That is inevitable > if you go to user space, as user space is paged memory. Putting a HCD into > user space the problem becomes even worse. You cannot honor GFP_NOIO. > That means that essentially no block device can run without the risk of > a deadlock. > > It seems to me that the cleanest solution would be an additional flag to > match requiring a kernel space driver would be the cleanest solution. > > Regards > Oliver > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH v11 0/4] Allow USB devices to remain runtime-suspended when sleeping
On Mon, Nov 02, 2015 at 02:50:40AM +0100, Rafael J. Wysocki wrote: > > I've queued up this series for the second half of the v4.4 merge window. > > Thanks, > Rafael > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel What's the status of this patch series? I haven't seen the patches posted for v4.4, plus there's the issue that Dan found that needs to be addressed. Is there a new revision of the patch series being worked on? -- 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 08/17] usb: host: ehci-dbg: remove unnecessary space after cast
On Mon, 2016-01-04 at 19:07 -0300, Geyslan G. Bem wrote:
> 2016-01-04 18:52 GMT-03:00 Sergei Shtylyov :
> > > > > > This patch fixes coding style issues reported by checkpatch
> > > > > > concerning
> > > > > > to unnecessary space after a cast.
> > > > > This is a case where checkpatch is wrong, IMO. Casts should always be
> > > > > followed by a space. I will not accept this patch.
Your choice, but most kernel code disagrees with you.
measuring only kernel casts to a pointer, (because there are
too many false positives otherwise) casts without a space
are preferred ~3:1 over casts followed by a space.
(without space)
$ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)\w+" * |
\
sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
36612
(with space)
$ grep -rP --include=*.[ch] -oh "\(\s*(\w{3,}\s+){0,2}\w{3,}\s*\*+\s*\)[
\t]\w+" * | \
sort|cut -f1 -d")"| sed 's/$/)/' | wc -l
13233
--
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 net] r8152: add reset_resume function
From: Hayes Wang Date: Mon, 4 Jan 2016 14:38:46 +0800 > When the reset_resume() is called, the flag of SELECTIVE_SUSPEND should be > cleared and reinitialize the device, whether the SELECTIVE_SUSPEND is set > or not. If reset_resume() is called, it means the power supply is cut or the > device is reset. That is, the device wouldn't be in runtime suspend state and > the reinitialization is necessary. > > Signed-off-by: Hayes Wang Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: f_fs: avoid race condition with ffs_epfile_io_complete
On Tue, Dec 29, 2015 at 02:36:58PM +0800, changbin...@intel.com wrote:
> From: "Du, Changbin"
>
> ffs_epfile_io and ffs_epfile_io_complete runs in different context, but
> there is no synchronization between them.
>
> consider the following scenario:
> 1) ffs_epfile_io interrupted by sigal while
> wait_for_completion_interruptible
> 2) then ffs_epfile_io set ret to -EINTR
> 3) just before or during usb_ep_dequeue, the request completed
> 4) ffs_epfile_io return with -EINTR
>
> In this case, ffs_epfile_io tell caller no transfer success but actually
> it may has been done. This break the caller's pipe.
>
> Below script can help test it (adbd is the process which lies on f_fs).
> while true
> do
>pkill -19 adbd #SIGSTOP
>pkill -18 adbd #SIGCONT
>sleep 0.1
> done
>
> To avoid this, just dequeue the request first. After usb_ep_dequeue, the
> request must be done or canceled.
>
> With this change, we can ensure no race condition in f_fs driver. But
> actually I found some of the udc driver has analogical issue in its
> dequeue implementation. For example,
> 1) the dequeue function hold the controller's lock.
> 2) before driver request controller to stop transfer, a request
>completed.
> 3) the controller trigger a interrupt, but its irq handler need wait
>dequeue function to release the lock.
> 4) dequeue function give back the request with negative status, and
>release lock.
> 5) irq handler get lock but the request has already been given back.
>
get unlock?
During the interrupt handler, it should only handle the "data complete"
interrupt on queued request; if the "data complete" interrupt occurs, but
it belongs to nobody, it will handle noop.
> So, the dequeue implementation should take care of this case. IMO, it
> can be done as below steps to dequeue a already started request,
> 1) request controller to stop transfer on the given ep. HW know the
>actual transfer status.
> 2) after hw stop transfer, driver scan if there are any completed one.
> 3) if found, process it with real status. if no, the request can
>canceled.
>
> Signed-off-by: Du, Changbin
> ---
> drivers/usb/gadget/function/f_fs.c | 45
> --
> 1 file changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index cf43e9e..8050939 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -687,6 +687,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct
> ffs_io_data *io_data)
> struct ffs_ep *ep;
> char *data = NULL;
> ssize_t ret, data_len = -EINVAL;
> + bool interrupted = false;
> int halt;
>
> /* Are we still active? */
> @@ -829,26 +830,35 @@ static ssize_t ffs_epfile_io(struct file *file, struct
> ffs_io_data *io_data)
>
> spin_unlock_irq(&epfile->ffs->eps_lock);
>
> - if (unlikely(ret < 0)) {
> - /* nop */
> - } else if (unlikely(
> + if (unlikely(ret < 0))
> + goto error_mutex;
> +
> + if (unlikely(
> wait_for_completion_interruptible(&done))) {
> - ret = -EINTR;
> - usb_ep_dequeue(ep->ep, req);
> - } else {
> /*
> - * XXX We may end up silently droping data
> - * here. Since data_len (i.e. req->length) may
> - * be bigger than len (after being rounded up
> - * to maxpacketsize), we may end up with more
> - * data then user space has space for.
> + * To avoid race condition with
> + * ffs_epfile_io_complete, dequeue the request
> + * first then check status. usb_ep_dequeue API
> + * should guarantee no race condition with
> + * req->complete callback.
>*/
> - ret = ep->status;
> - if (io_data->read && ret > 0) {
> - ret = copy_to_iter(data, ret,
> &io_data->data);
> - if (!ret)
> - ret = -EFAULT;
> - }
> + usb_ep_dequeue(ep->ep, req);
> + interrupted = true;
> + }
> +
> + /*
> + * XXX We may end up silently droping data
> + * here. Since data_len (i.e. req->length) may
> + * be bigger than len (after being rounded up
> +
RE: [PATCH] usb: f_fs: avoid race condition with ffs_epfile_io_complete
> > To avoid this, just dequeue the request first. After usb_ep_dequeue, the > > request must be done or canceled. > > > > With this change, we can ensure no race condition in f_fs driver. But > > actually I found some of the udc driver has analogical issue in its > > dequeue implementation. For example, > > 1) the dequeue function hold the controller's lock. > > 2) before driver request controller to stop transfer, a request > >completed. > > 3) the controller trigger a interrupt, but its irq handler need wait > >dequeue function to release the lock. > > 4) dequeue function give back the request with negative status, and > >release lock. > > 5) irq handler get lock but the request has already been given back. > > > > get unlock? > > During the interrupt handler, it should only handle the "data complete" > interrupt on queued request; if the "data complete" interrupt occurs, but > it belongs to nobody, it will handle noop. > > > Best Regards, > Peter Chen You are right, but the problem is the request->status is wrong. If the data send out but report caller as -EINTR, it will introduce duplicate-send issue. Regards, Du, Changbin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Charity/Donation
Hi, My name is Jeffrey Skoll, a philanthropist and the founder of one of the largest private foundations in the world. I believe strongly in ‘giving while living.’ I had one idea that never changed in my mind — that you should use your wealth to help people and I have decided to secretly give USD2.498 Million to a randomly selected individual. On receipt of this email, you should count yourself as the individual. Kindly get back to me at your earliest convenience, so I know your email address is valid. Visit the web page to know more about me: http://www.theglobeandmail.com/news/national/meet-the-canadian-billionaire-whos-giving-it-all-away/article4209888/ or you can read an article of me on Wikipedia. Regards, Jeffrey Skoll. -- 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: f_fs: avoid race condition with ffs_epfile_io_complete
On Tue, Jan 05, 2016 at 04:09:47AM +, Du, Changbin wrote: > > > To avoid this, just dequeue the request first. After usb_ep_dequeue, the > > > request must be done or canceled. > > > > > > With this change, we can ensure no race condition in f_fs driver. But > > > actually I found some of the udc driver has analogical issue in its > > > dequeue implementation. For example, > > > 1) the dequeue function hold the controller's lock. > > > 2) before driver request controller to stop transfer, a request > > >completed. > > > 3) the controller trigger a interrupt, but its irq handler need wait > > >dequeue function to release the lock. > > > 4) dequeue function give back the request with negative status, and > > >release lock. > > > 5) irq handler get lock but the request has already been given back. > > > > > > > get unlock? > > > > During the interrupt handler, it should only handle the "data complete" > > interrupt on queued request; if the "data complete" interrupt occurs, but > > it belongs to nobody, it will handle noop. > > > > > > Best Regards, > > Peter Chen > > You are right, but the problem is the request->status is wrong. If the data > send out but report caller as -EINTR, it will introduce duplicate-send > issue. > Why -EINTR, the kernel-doc said it should return -ECONNRESET for active request, see include/linux/usb/gadget.h. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: f_fs: avoid race condition with ffs_epfile_io_complete
> > > > You are right, but the problem is the request->status is wrong. If the data > > send out but report caller as -EINTR, it will introduce duplicate-send > > issue. > > > > Why -EINTR, the kernel-doc said it should return -ECONNRESET for active > request, see include/linux/usb/gadget.h. > > -- > > Best Regards, > Peter Chen F_fs return -EINTER in its dequeuer case, not udc driver. What I want to say is driver should return the right status for each usb request. -- 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
