Re: [PATCH] usb: serial: Add bricked FTDI device PID
On Thu, Oct 23, 2014 at 11:39:50AM +0800, Greg Kroah-Hartman wrote: On Wed, Oct 22, 2014 at 11:31:34PM -0400, Perry Hung wrote: An official recent Windows driver from FTDI detects counterfeit devices and reprograms the internal EEPROM containing the USB PID to 0, effectively bricking the device. How sneaky. Is idProduct the only field that is rewritten? I'm curious about whether the type is still detected correctly. Add support for this VID/PID pair to correctly bind the driver on these devices. See: http://hackaday.com/2014/10/22/watch-that-windows-update-ftdi-drivers-are-killing-fake-chips/ Signed-off-by: Perry Hung ipe...@gmail.com Thanks for doing this, Johan, feel free to add: Cc: stable sta...@vger.kernel.org Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org to it when/if you apply it to your tree. Applied. 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] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
On Wed, Oct 22, 2014 at 11:02:26AM +0300, Tomi Valkeinen wrote: On 18/10/14 00:13, Jani Nikula wrote: Documentation/kbuild/kconfig-language.txt warns to use select with care, and in general use select only for non-visible symbols and for symbols with no dependencies, because select will force a symbol to a value without visiting the dependencies. Select has become particularly problematic, interdependently, with the BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO configs. For example: scripts/kconfig/conf --randconfig Kconfig KCONFIG_SEED=0x48312B00 warning: (DRM_RADEON DRM_NOUVEAU DRM_I915 DRM_GMA500 DRM_SHMOBILE DRM_TILCDC FB_BACKLIGHT FB_MX3 USB_APPLEDISPLAY FB_OLPC_DCON ASUS_LAPTOP SONY_LAPTOP THINKPAD_ACPI EEEPC_LAPTOP ACPI_CMPC SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM BACKLIGHT_LCD_SUPPORT) warning: (DRM_RADEON DRM_NOUVEAU DRM_I915 DRM_GMA500 DRM_SHMOBILE DRM_TILCDC FB_BACKLIGHT FB_MX3 USB_APPLEDISPLAY FB_OLPC_DCON ASUS_LAPTOP SONY_LAPTOP THINKPAD_ACPI EEEPC_LAPTOP ACPI_CMPC SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM BACKLIGHT_LCD_SUPPORT) With tristates it's possible to end up selecting FOO=y depending on BAR=m in the config, which gets discovered at build time, not config time, like reported in the thread referenced below. Do the following to fix the dependencies: * Depend on instead of select BACKLIGHT_CLASS_DEVICE everywhere. Drop select BACKLIGHT_LCD_SUPPORT in such cases, as it's a dependency of BACKLIGHT_CLASS_DEVICE. * Remove config FB_BACKLIGHT altogether, and replace it with a dependency on BACKLIGHT_CLASS_DEVICE. All configs selecting FB_BACKLIGHT select or depend on FB anyway, so we can simplify. * Depend on (ACPI ACPI_VIDEO) || ACPI=n in several places instead of selecting ACPI_VIDEO and a number of its dependencies if ACPI is enabled. This is tied to backlight, as ACPI_VIDEO depends on BACKLIGHT_CLASS_DEVICE. * Replace a couple of select INPUT/VT with depends as it seemed to be necessary. While doing 'depends on' instead of 'select' is an easy fix for this, I do dislike it quite a bit. It's a major pain to go around the kernel config, trying to find all the dependencies that a particular driver wants. If I need fb-foobar, I should just be able to enable it, instead of first searching and selecting its minor dependencies individually. So, not a NACK, but a isn't there an another way to fix this?. Looking at backlight... BACKLIGHT_LCD_SUPPORT seems to be a meta option, it only enables a Kconfig submenu. So I think we could just remove the whole BACKLIGHT_LCD_SUPPORT option. But if we do that, all the items in drivers/video/backlight/Kconfig with default 'y' or 'm' would get enabled by default, so I think we should remove the 'default's from that file. That makes sense in any case, as I don't see why HP Jornada 700 series LCD Driver should be default y. BACKLIGHT_CLASS_DEVICE doesn't depend on anything except BACKLIGHT_LCD_SUPPORT, so after removing BACKLIGHT_LCD_SUPPORT it should be safe to 'select' BACKLIGHT_CLASS_DEVICE. BACKLIGHT_CLASS_DEVICE could be made a hidden option, and the drivers in drivers/video/backlight/Kconfig which are under BACKLIGHT_CLASS_DEVICE could be made to select BACKLIGHT_CLASS_DEVICE instead. That doesn't exactly fix anything, but I think it makes sense as BACKLIGHT_CLASS_DEVICE is something that's selected from all around the kernel, so it should be a selectable library instead of a Kconfig menu option. I didn't look at the ACPI_VIDEO side, so no idea how messy that is. If we want to make BACKLIGHT_CLASS_DEVICE into a library thing then I guess we could do that, but we must then also drag it out of all the other meta options to make sure it's always available. No need I think to ditch the entire BACKLIGHT_LCD_SUPPORT meta option. And then everyone could select it. The problem is if you mix depends and select Kconfig falls over and starts to see loops where there are none. So you really can't ever mix them for the same symbol. I think if we get the BACKLIGHT_CLASS_DEVICE out of the picture with that ACPI_VIDEO should be fixable with some select ACPI_VIDEO if ACPI or so. Currently that doesn't work because of the backlight class knob and select being broken. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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: Add bricked FTDI device PID
On Thu, Oct 23, 2014 at 09:58:20AM +0200, Johan Hovold wrote: On Thu, Oct 23, 2014 at 11:39:50AM +0800, Greg Kroah-Hartman wrote: On Wed, Oct 22, 2014 at 11:31:34PM -0400, Perry Hung wrote: An official recent Windows driver from FTDI detects counterfeit devices and reprograms the internal EEPROM containing the USB PID to 0, effectively bricking the device. How sneaky. Is idProduct the only field that is rewritten? According to all of the reports I've read, yes, this is what happens, as there are api calls to the device that allow anyone to reprogram the idProduct field. I'm curious about whether the type is still detected correctly. What type are you referring to? 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] usb: serial: Add bricked FTDI device PID
On Thu, Oct 23, 2014 at 04:10:54PM +0800, Greg Kroah-Hartman wrote: On Thu, Oct 23, 2014 at 09:58:20AM +0200, Johan Hovold wrote: On Thu, Oct 23, 2014 at 11:39:50AM +0800, Greg Kroah-Hartman wrote: On Wed, Oct 22, 2014 at 11:31:34PM -0400, Perry Hung wrote: An official recent Windows driver from FTDI detects counterfeit devices and reprograms the internal EEPROM containing the USB PID to 0, effectively bricking the device. How sneaky. Is idProduct the only field that is rewritten? According to all of the reports I've read, yes, this is what happens, as there are api calls to the device that allow anyone to reprogram the idProduct field. I'm curious about whether the type is still detected correctly. What type are you referring to? We try to detect the type during probe (encoded as an enum ftdi_chip_type), which is later used when setting the baud rate (but also affects the packet layout for legacy SIO devices). It's done based on bNumInterfaces and bcdDevice. 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: Add bricked FTDI device PID
On Thu, Oct 23, 2014 at 10:17:03AM +0200, Johan Hovold wrote: On Thu, Oct 23, 2014 at 04:10:54PM +0800, Greg Kroah-Hartman wrote: On Thu, Oct 23, 2014 at 09:58:20AM +0200, Johan Hovold wrote: On Thu, Oct 23, 2014 at 11:39:50AM +0800, Greg Kroah-Hartman wrote: On Wed, Oct 22, 2014 at 11:31:34PM -0400, Perry Hung wrote: An official recent Windows driver from FTDI detects counterfeit devices and reprograms the internal EEPROM containing the USB PID to 0, effectively bricking the device. How sneaky. Is idProduct the only field that is rewritten? According to all of the reports I've read, yes, this is what happens, as there are api calls to the device that allow anyone to reprogram the idProduct field. I'm curious about whether the type is still detected correctly. What type are you referring to? We try to detect the type during probe (encoded as an enum ftdi_chip_type), which is later used when setting the baud rate (but also affects the packet layout for legacy SIO devices). It's done based on bNumInterfaces and bcdDevice. Ah, ok, I don't think those fields can be changed through the api. I could be totally wrong, I'm going to try to dig and find a clone device to test it out with, but for now, odds are, this patch should be all that is needed. 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] usb: serial: Add bricked FTDI device PID
On Thu, Oct 23, 2014 at 04:27:50PM +0800, Perry Hung wrote: It doesn't seem so, there's an IDA pro dump from somebody who decompiled the driver: https://twitter.com/marcan42/status/525126731431038977/photo/1 A picture of the source code created by disassembling a binary driver that was written to brick a cloned USB controller device. Reverse engineering inception layers... thanks for that link, very cool. 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
[PATCH] usb: ehci/ohci: use SIMPLE_DEV_PM_OPS to support hibernation
ehci/ohci just define .suspend/.resume functions for dev_pm_ops, but
in order to support both STR(suspend-to-ram) and hibernation, other
callbacks such as .freeze/.thaw are also required.
Registering all required callbacks for both STR and hibernation can
be done by SIMPLE_DEV_PM_OPS macro function.
Signed-off-by: Wonhong Kwon wonhong.k...@lge.com
---
drivers/usb/host/ehci-platform.c | 13 +
drivers/usb/host/ohci-platform.c | 13 +
2 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 2f5b9ce..9aee03f 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -311,8 +311,7 @@ static int ehci_platform_remove(struct platform_device *dev)
return 0;
}
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
static int ehci_platform_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -349,10 +348,10 @@ static int ehci_platform_resume(struct device *dev)
return 0;
}
-#else /* !CONFIG_PM */
+#else /* !CONFIG_PM_SLEEP */
#define ehci_platform_suspend NULL
#define ehci_platform_resume NULL
-#endif /* CONFIG_PM */
+#endif /* CONFIG_PM_SLEEP */
static const struct of_device_id vt8500_ehci_ids[] = {
{ .compatible = via,vt8500-ehci, },
@@ -368,10 +367,8 @@ static const struct platform_device_id
ehci_platform_table[] = {
};
MODULE_DEVICE_TABLE(platform, ehci_platform_table);
-static const struct dev_pm_ops ehci_platform_pm_ops = {
- .suspend= ehci_platform_suspend,
- .resume = ehci_platform_resume,
-};
+static SIMPLE_DEV_PM_OPS(ehci_platform_pm_ops, ehci_platform_suspend,
+ ehci_platform_resume);
static struct platform_driver ehci_platform_driver = {
.id_table = ehci_platform_table,
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 4369299..70b912f 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -298,8 +298,7 @@ static int ohci_platform_remove(struct platform_device *dev)
return 0;
}
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
static int ohci_platform_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -336,10 +335,10 @@ static int ohci_platform_resume(struct device *dev)
return 0;
}
-#else /* !CONFIG_PM */
+#else /* !CONFIG_PM_SLEEP */
#define ohci_platform_suspend NULL
#define ohci_platform_resume NULL
-#endif /* CONFIG_PM */
+#endif /* CONFIG_PM_SLEEP */
static const struct of_device_id ohci_platform_ids[] = {
{ .compatible = generic-ohci, },
@@ -353,10 +352,8 @@ static const struct platform_device_id
ohci_platform_table[] = {
};
MODULE_DEVICE_TABLE(platform, ohci_platform_table);
-static const struct dev_pm_ops ohci_platform_pm_ops = {
- .suspend= ohci_platform_suspend,
- .resume = ohci_platform_resume,
-};
+static SIMPLE_DEV_PM_OPS(ohci_platform_pm_ops, ohci_platform_suspend,
+ ohci_platform_resume);
static struct platform_driver ohci_platform_driver = {
.id_table = ohci_platform_table,
--
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] usb: serial: Perform verification for FTDI FT232R devices
From: Russ Dill russ.d...@gmail.com
This patch provides the FTDI genuine product verification steps
as contained within the new 2.12.00 official release. It ensures
that counterfeiters don't exploit engineering investment made
by FTDI. Counterfeit ICs are destroying innovation in the
industry.
FTDI recommends that to guarantee genuine FTDI products
please purchase either from FTDI directly or an authorised
distributor.
This is definitely not targeting end users - if you're unsure if
ICs are genuine then please don't use the drivers.
Signed-off-by: Russ Dill russ.d...@gmail.com
---
drivers/usb/serial/ftdi_sio.c | 111 +-
drivers/usb/serial/ftdi_sio.h | 41
2 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index dc72b924c399..ef8b0dd632d3 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1377,6 +1377,48 @@ static int read_latency_timer(struct usb_serial_port
*port)
return rv;
}
+static int write_eeprom(struct usb_serial_port *port, u8 addr, u16 data)
+{
+ struct usb_device *udev = port-serial-dev;
+ int rv;
+
+ rv = usb_control_msg(udev,
+usb_sndctrlpipe(udev, 0),
+FTDI_SIO_WRITE_EEPROM_REQUEST,
+FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
+data, addr,
+NULL, 0, WDR_TIMEOUT);
+ if (rv 0)
+ dev_err(port-dev, Unable to write EEPROM: %i\n, rv);
+ return rv;
+}
+
+static int read_eeprom(struct usb_serial_port *port, u8 addr, u16 *data)
+{
+ struct usb_device *udev = port-serial-dev;
+ u16 *buf;
+ int rv;
+
+ buf = kmalloc(2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ rv = usb_control_msg(udev,
+usb_rcvctrlpipe(udev, 0),
+FTDI_SIO_GET_LATENCY_TIMER_REQUEST,
+FTDI_SIO_GET_LATENCY_TIMER_REQUEST_TYPE,
+0, addr,
+buf, 2, WDR_TIMEOUT);
+ if (rv 0)
+ dev_err(port-dev, Unable to read from EEPROM: %i\n, rv);
+ else
+ *data = *buf;
+
+ kfree(buf);
+
+ return rv;
+}
+
static int get_serial_info(struct usb_serial_port *port,
struct serial_struct __user *retinfo)
{
@@ -1723,12 +1765,78 @@ static int ftdi_sio_probe(struct usb_serial *serial,
return 0;
}
+static u16 ftdi_checksum(u16 *data, int n)
+{
+ u16 checksum;
+ int i;
+
+ checksum = 0x;
+ for (i = 0; i n - 1; i++) {
+ checksum ^= be16_to_cpu(data[i]);
+ checksum = (checksum 1) | (checksum 15);
+ }
+
+ return cpu_to_be16(checksum);
+}
+
+static void ftdi_verify(struct usb_serial_port *port)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ u16 *eeprom_data;
+ u16 checksum;
+ int eeprom_size;
+ int i;
+
+ switch (priv-chip_type) {
+ case FT232RL:
+ eeprom_size = 0x40;
+ break;
+ default:
+ /* Unsupported for verification */
+ return;
+ }
+
+ /* Latency timer needs to be 0x77 to unlock EEPROM programming */
+ if (priv-latency != 0x77) {
+ int orig_latency = priv-latency;
+ priv-latency = 0x77;
+ write_latency_timer(port);
+ priv-latency = orig_latency;
+ }
+
+ eeprom_data = kzalloc(eeprom_size * 2, GFP_KERNEL);
+ if (!eeprom_data)
+ return;
+
+ /* Read in EEPROM */
+ for (i = 0; i eeprom_size; i++)
+ if (read_eeprom(port, i, eeprom_data + i) 0)
+ goto end_verify;
+
+ /* Verify EEPROM is valid */
+ checksum = ftdi_checksum(eeprom_data, eeprom_size);
+ if (checksum != eeprom_data[eeprom_size - 1])
+ goto end_verify;
+
+ /* Attempt to set Vendor ID to 0 */
+ eeprom_data[1] = 0;
+
+ /* Calculate new checksum to avoid bricking devices */
+ checksum = ftdi_checksum(eeprom_data, eeprom_size);
+
+ /* Verify EEPROM programming behavior/nonbehavior */
+ write_eeprom(port, 1, 0);
+ write_eeprom(port, eeprom_size - 1, checksum);
+
+end_verify:
+ kfree(eeprom_data);
+}
+
static int ftdi_sio_port_probe(struct usb_serial_port *port)
{
struct ftdi_private *priv;
struct ftdi_sio_quirk *quirk = usb_get_serial_data(port-serial);
-
priv = kzalloc(sizeof(struct ftdi_private), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -1746,6 +1854,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port
*port)
ftdi_set_max_packet_size(port);
if (read_latency_timer(port) 0)
Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
On Thu, Oct 23, 2014 at 01:52:24AM -0700, russ.d...@gmail.com wrote: From: Russ Dill russ.d...@gmail.com This patch provides the FTDI genuine product verification steps as contained within the new 2.12.00 official release. It ensures that counterfeiters don't exploit engineering investment made by FTDI. Counterfeit ICs are destroying innovation in the industry. FTDI recommends that to guarantee genuine FTDI products please purchase either from FTDI directly or an authorised distributor. This is definitely not targeting end users - if you're unsure if ICs are genuine then please don't use the drivers. Signed-off-by: Russ Dill russ.d...@gmail.com --- drivers/usb/serial/ftdi_sio.c | 111 +- drivers/usb/serial/ftdi_sio.h | 41 2 files changed, 151 insertions(+), 1 deletion(-) Funny patch, you should have saved it for April 1, otherwise people might have actually taken this seriously :) Patches as performance art, now I've seen everything... 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] usb: serial: Perform verification for FTDI FT232R devices
On Thu, Oct 23, 2014 at 11:40 AM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Oct 23, 2014 at 01:52:24AM -0700, russ.d...@gmail.com wrote: From: Russ Dill russ.d...@gmail.com This patch provides the FTDI genuine product verification steps as contained within the new 2.12.00 official release. It ensures that counterfeiters don't exploit engineering investment made by FTDI. Counterfeit ICs are destroying innovation in the industry. FTDI recommends that to guarantee genuine FTDI products please purchase either from FTDI directly or an authorised distributor. This is definitely not targeting end users - if you're unsure if ICs are genuine then please don't use the drivers. Signed-off-by: Russ Dill russ.d...@gmail.com --- drivers/usb/serial/ftdi_sio.c | 111 +- drivers/usb/serial/ftdi_sio.h | 41 2 files changed, 151 insertions(+), 1 deletion(-) Funny patch, you should have saved it for April 1, otherwise people might have actually taken this seriously :) Patches as performance art, now I've seen everything... This just made my day :) -- 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/TRIVIAL 4/4] uwb: Remove bogus colon after newline from debug message
Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be Cc: linux-usb@vger.kernel.org --- drivers/uwb/rsv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/uwb/rsv.c b/drivers/uwb/rsv.c index 0887ae982783b7df..536ad42b0a4b1d7e 100644 --- a/drivers/uwb/rsv.c +++ b/drivers/uwb/rsv.c @@ -213,7 +213,7 @@ void uwb_rsv_backoff_win_timer(unsigned long arg) bow-total_expired = 0; bow-window = UWB_DRP_BACKOFF_WIN_MIN 1; } - dev_dbg(dev, backoff_win_timer total_expired=%d, n=%d\n: , bow-total_expired, bow-n); + dev_dbg(dev, backoff_win_timer total_expired=%d, n=%d\n, bow-total_expired, bow-n); /* try to relocate all the to be moved relocations */ uwb_rsv_handle_drp_avail_change(rc); @@ -234,7 +234,7 @@ void uwb_rsv_backoff_win_increment(struct uwb_rc *rc) bow-window = 1; bow-n = prandom_u32() (bow-window - 1); - dev_dbg(dev, new_window=%d, n=%d\n: , bow-window, bow-n); + dev_dbg(dev, new_window=%d, n=%d\n, bow-window, bow-n); /* reset the timer associated variables */ timeout_us = bow-n * UWB_SUPERFRAME_LENGTH_US; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: dwc2: gadget: sparse warning of context imbalance
On Fri, Oct 17, 2014 at 07:05:19PM +, Paul Zimmerman wrote:
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: Friday, October 17, 2014 11:52 AM
On Fri, Oct 17, 2014 at 06:50:19PM +, Paul Zimmerman wrote:
From: Sudip Mukherjee [mailto:sudipm.mukher...@gmail.com]
Sent: Thursday, October 16, 2014 9:44 PM
sparse was giving the following warning:
warning: context imbalance in 's3c_hsotg_ep_enable'
- different lock contexts for basic block
we were returning ENOMEM while still holding the spinlock.
The sparse warning was fixed by releasing the spinlock before return.
Signed-off-by: Sudip Mukherjee su...@vectorindia.org
---
drivers/usb/dwc2/gadget.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856f..7f25527 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2561,8 +2561,10 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
hs_ep-fifo_size = val;
break;
}
- if (i == 8)
- return -ENOMEM;
+ if (i == 8) {
+ ret = -ENOMEM;
+ goto error;
+ }
}
/* for non control endpoints, set PID to D0 */
@@ -2579,6 +2581,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
/* enable the endpoint interrupt */
s3c_hsotg_ctrl_epint(hsotg, index, dir_in, 1);
+error:
spin_unlock_irqrestore(hsotg-lock, flags);
return ret;
}
Acked-by: Paul Zimmerman pa...@synopsys.com
v3.18-rc or v3.19 ?
v3.18-rc, since it's a bugfix. And I forgot, this should be marked for
3.17 stable too.
hi,
this is not yet added to linux-next , is something pending from my side?
thanks
sudip
--
Paul
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: function: Fixed the return value on error path
On 10/22/2014 04:03 PM, Pavitrakumar Managutte wrote: Fixed the return value on failure. status variable is set to 0 at usb_assign_descriptors call and the same is returned on error which is incorrect. Signed-off-by: Pavitrakumar Managutte pavitra1...@gmail.com Acked-by: Sebastian Andrzej Siewior bige...@linutronix.de I spotted two wrong error paths, this one. Do you look after the second one? Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] usb: dwc2: gadget: sparse warning of context imbalance
On Thu, Oct 23, 2014 at 03:37:14PM +0530, Sudip Mukherjee wrote:
On Fri, Oct 17, 2014 at 07:05:19PM +, Paul Zimmerman wrote:
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: Friday, October 17, 2014 11:52 AM
On Fri, Oct 17, 2014 at 06:50:19PM +, Paul Zimmerman wrote:
From: Sudip Mukherjee [mailto:sudipm.mukher...@gmail.com]
Sent: Thursday, October 16, 2014 9:44 PM
sparse was giving the following warning:
warning: context imbalance in 's3c_hsotg_ep_enable'
- different lock contexts for basic block
we were returning ENOMEM while still holding the spinlock.
The sparse warning was fixed by releasing the spinlock before return.
Signed-off-by: Sudip Mukherjee su...@vectorindia.org
---
drivers/usb/dwc2/gadget.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856f..7f25527 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2561,8 +2561,10 @@ static int s3c_hsotg_ep_enable(struct usb_ep
*ep,
hs_ep-fifo_size = val;
break;
}
- if (i == 8)
- return -ENOMEM;
+ if (i == 8) {
+ ret = -ENOMEM;
+ goto error;
+ }
}
/* for non control endpoints, set PID to D0 */
@@ -2579,6 +2581,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep
*ep,
/* enable the endpoint interrupt */
s3c_hsotg_ctrl_epint(hsotg, index, dir_in, 1);
+error:
spin_unlock_irqrestore(hsotg-lock, flags);
return ret;
}
Acked-by: Paul Zimmerman pa...@synopsys.com
v3.18-rc or v3.19 ?
v3.18-rc, since it's a bugfix. And I forgot, this should be marked for
3.17 stable too.
hi,
this is not yet added to linux-next , is something pending from my side?
Patience.
--
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: Perform verification for FTDI FT232R devices
drivers/usb/serial/ftdi_sio.c | 111 +- drivers/usb/serial/ftdi_sio.h | 41 2 files changed, 151 insertions(+), 1 deletion(-) Funny patch, you should have saved it for April 1, otherwise people might have actually taken this seriously :) Patches as performance art, now I've seen everything... Chuckle. Sillyness aside a pure detection version of that patch might be useful so it can warn users Running Windows may damage your adapter 8) Is the 0x, 0x0401 they end up with consistent - can we add that to the default table so end users can at least make use of devices that have been attacked by malware ? Alan -- 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: Perform verification for FTDI FT232R devices
On Thu, Oct 23, 2014 at 12:18:15PM +0100, One Thousand Gnomes wrote: drivers/usb/serial/ftdi_sio.c | 111 +- drivers/usb/serial/ftdi_sio.h | 41 2 files changed, 151 insertions(+), 1 deletion(-) Funny patch, you should have saved it for April 1, otherwise people might have actually taken this seriously :) Patches as performance art, now I've seen everything... Chuckle. Sillyness aside a pure detection version of that patch might be useful so it can warn users Running Windows may damage your adapter 8) Is the 0x, 0x0401 they end up with consistent - can we add that to the default table so end users can at least make use of devices that have been attacked by malware ? Way ahead of you. ;) http://marc.info/?l=linux-usbm=141405129201389w=2 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] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
On 23/10/14 11:10, Daniel Vetter wrote: If we want to make BACKLIGHT_CLASS_DEVICE into a library thing then I guess we could do that, but we must then also drag it out of all the other meta options to make sure it's always available. No need I think to ditch BACKLIGHT_CLASS_DEVICE only depends on HAS_IOMEM and BACKLIGHT_LCD_SUPPORT so there are no other meta options to avoid. HAS_IOMEM comes from drivers/video/Kconfig's Graphics support, and I guess we can ignore it. the entire BACKLIGHT_LCD_SUPPORT meta option. And then everyone could select it. I don't quite understand what purpose does BACKLIGHT_LCD_SUPPORT serve. It doesn't enable any code, it just opens up new Kconfig options. Why can't the Kconfig options be always available? It's just another option to 'select', without any reason I can see. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
On 23.10.2014 12:52, russ.d...@gmail.com wrote: From: Russ Dill russ.d...@gmail.com This patch provides the FTDI genuine product verification steps as contained within the new 2.12.00 official release. It ensures that counterfeiters don't exploit engineering investment made by FTDI. Counterfeit ICs are destroying innovation in the industry. FTDI recommends that to guarantee genuine FTDI products please purchase either from FTDI directly or an authorised distributor. This is definitely not targeting end users - if you're unsure if ICs are genuine then please don't use the drivers. Signed-off-by: Russ Dill russ.d...@gmail.com By the way, have you tested it? -- 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] uas: Add US_FL_NO_ATA_1X quirk for 2 more Seagate models
These drives hang when receiving ATA12 commands, so set the US_FL_NO_ATA_1X quirk to filter these out. Cc: sta...@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/usb/storage/unusual_uas.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index a801850..44ab3289 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -54,6 +54,20 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x, 0x, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* Reported-by: Hans de Goede hdego...@redhat.com */ +UNUSUAL_DEV(0x0bc2, 0x3320, 0x, 0x, + Seagate, + Expansion Desk, + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + +/* Reported-by: Bogdan Mihalcea bogdan.mihal...@infim.ro */ +UNUSUAL_DEV(0x0bc2, 0xa003, 0x, 0x, + Seagate, + Backup Plus, + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* https://bbs.archlinux.org/viewtopic.php?id=183190 */ UNUSUAL_DEV(0x0bc2, 0xab20, 0x, 0x, Seagate, -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
On Thu, Oct 23, 2014 at 12:18:15PM +0100, One Thousand Gnomes wrote: drivers/usb/serial/ftdi_sio.c | 111 +- drivers/usb/serial/ftdi_sio.h | 41 2 files changed, 151 insertions(+), 1 deletion(-) Funny patch, you should have saved it for April 1, otherwise people might have actually taken this seriously :) Patches as performance art, now I've seen everything... Chuckle. Sillyness aside a pure detection version of that patch might be useful so it can warn users Running Windows may damage your adapter 8) The EEPROM read function might also be interesting too, especially with a userspace interface for reading it back. signature.asc Description: Digital signature
TI Tiva Launchpad Luminary ICDI Bug
Hi! With a recent Update from Kernel 3.16.14-1-x86_64 to linux-3.17.1-1-x86_64 my machine isn't able anymore to recognise my TI Tiva Lauchpad. lsusb normally has the following entry: Bus 004 Device 010: ID 1cbe:00fd Luminary Micro Inc. In-Circuit Debug Interface It vanishes with 3.17. I downgraded to 3.16 and it works again. Bye pgp9gbQtDq2EB.pgp Description: PGP signature
Re: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
On Wed, Oct 22, 2014 at 10:14:54AM -0400, Alan Stern wrote:
On Wed, 22 Oct 2014, Anton Tikhomirov wrote:
That's right, and it's true for USB-2 as well. A ZLP is needed only in
cases where the host otherwise wouldn't know the transfer is over,
i.e., when the transfer length is a nonzero multiple of the maxpacket
size and is smaller than wLength.
Shall we use/check struct usb_request's zero flag for this?
Of course; we have to. There's no other way for the UDC driver to know
whether the transfer is shorter than the host expects.
alright, so I take it this incremental diff is enough ?
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 0a34e71..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -830,7 +830,8 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
} else {
dwc3_gadget_giveback(ep0, r, 0);
- if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)) {
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-zero) {
int ret;
dwc-ep0_next_event = DWC3_EP0_COMPLETE;
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi,
On Thu, Oct 23, 2014 at 08:48:08AM -0500, Felipe Balbi wrote:
On Wed, Oct 22, 2014 at 10:14:54AM -0400, Alan Stern wrote:
On Wed, 22 Oct 2014, Anton Tikhomirov wrote:
That's right, and it's true for USB-2 as well. A ZLP is needed only in
cases where the host otherwise wouldn't know the transfer is over,
i.e., when the transfer length is a nonzero multiple of the maxpacket
size and is smaller than wLength.
Shall we use/check struct usb_request's zero flag for this?
Of course; we have to. There's no other way for the UDC driver to know
whether the transfer is shorter than the host expects.
alright, so I take it this incremental diff is enough ?
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 0a34e71..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -830,7 +830,8 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
} else {
dwc3_gadget_giveback(ep0, r, 0);
- if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)) {
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-zero) {
int ret;
dwc-ep0_next_event = DWC3_EP0_COMPLETE;
here's v2:
8--
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00 2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
-* handle the case where we have to send a zero packet. This
-* seems to be case when req.length maxpacket. Could it be?
-*/
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
+
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-zero) {
+ int ret;
+
+ dwc-ep0_next_event = DWC3_EP0_COMPLETE;
+
+ ret = dwc3_ep0_start_trans(dwc, epnum,
+ dwc-ctrl_req_addr, 0,
+ DWC3_TRBCTL_CONTROL_DATA);
+ WARN_ON(ret 0);
+ }
}
}
--
2.1.0.GIT
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 67/67] usb: gadget: remove unnecessary 'driver' argument
On Mon, Oct 20, 2014 at 08:45:02AM +0200, Robert Baldyga wrote: Hi Felipe, On 10/17/2014 07:17 PM, Felipe Balbi wrote: now that no UDC driver relies on the extra 'driver' argument to -udc_stop(), we can safely remove it. Signed-off-by: Felipe Balbi ba...@ti.com I have already sent patch doing the same thing: http://markmail.org/message/x5zneg4xea4zntab hmm... I had forgotten about that. If you can fix the bugs, I'll take your version. Look at what you do for mv_udc, for instance, this change: - stop_activity(udc, driver); + stop_activity(udc, udc-driver); Is completely bogus. What happens now is that this UDC driver will always pass a valid, non-NULL driver argument to stop_activity() which means that when the UDC is being stopped, it will call gadget driver's -disconnect(). In fact, you did that blind change to all UDC drivers without considering what stop_activity() was really doing and when it was called. I guess I'll just note on my commit that it's based on your patch but with quite a few bugs being fixed. -- balbi signature.asc Description: Digital signature
Re: [PATCH 00/67] usb: pending changes for next merge window
On Mon, Oct 20, 2014 at 11:02:44AM +0800, Peter Chen wrote: On Fri, Oct 17, 2014 at 12:16:07PM -0500, Felipe Balbi wrote: Hi folks, here's a giant set of patches which I already have pending for v3.19 merge window. They still sit in my testing/next and will only be moved to next once all fixes reach v3.18-rc. Until then, I will keep rebasing them. Note that for this set to properly work, we are still pending a fix being reworked by Marek Szyprowski. As long as you don't build dwc2, everything should work fine, however. Yes, if build all, has below error: /home/peter/work/projects/usb/drivers/usb/dwc2/gadget.c: In function ‘s3c_hsotg_udc_stop’: /home/peter/work/projects/usb/drivers/usb/dwc2/gadget.c:2939:7: error: ‘driver’ undeclared (first use in this function) /home/peter/work/projects/usb/drivers/usb/dwc2/gadget.c:2939:7: note: each undeclared identifier is reported only once for each function it appears in fixed that. Please test. I mean it. Don't want nobody whinning after these patches are moved to my 'next' branch; so please go ahead and test. Tested on chipidea hardware, it works ok. For chipidea change: Acked-by: Peter Chen peter.c...@freescale.com added this. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: gadget: Properly initialize LINK TRB
On Tue, Oct 21, 2014 at 04:31:10PM -0700, Jack Pham wrote: On ISOC endpoints the last trb_pool entry used as a LINK TRB is not getting zeroed out correctly due to memset being called incorrectly and in the wrong place. If pool allocated from DMA was not zero-initialized to begin with this will result in the size and ctrl values being random garbage. Call memset correctly after assignment of the trb_link pointer. Fixes: f6bafc6a1c (usb: dwc3: convert TRBs into bitshifts) Signed-off-by: Jack Pham ja...@codeaurora.org Cc: sta...@vger.kernel.org you should add a # v3.4+ after stable email, I'll add that myself this time. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: ehci/ohci: use SIMPLE_DEV_PM_OPS to support hibernation
On Thu, 23 Oct 2014, Wonhong Kwon wrote: ehci/ohci just define .suspend/.resume functions for dev_pm_ops, but in order to support both STR(suspend-to-ram) and hibernation, other callbacks such as .freeze/.thaw are also required. Registering all required callbacks for both STR and hibernation can be done by SIMPLE_DEV_PM_OPS macro function. Signed-off-by: Wonhong Kwon wonhong.k...@lge.com The patch itself looks good, but the description and the title should be updated. The patch affects ehci_platform and ohci_platform, not ehci and ohci. If you fix that up, you can add: Acked-by: Alan Stern st...@rowland.harvard.edu 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 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
On Thu, 23 Oct 2014, Felipe Balbi wrote:
here's v2:
8--
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00 2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
- * handle the case where we have to send a zero packet. This
- * seems to be case when req.length maxpacket. Could it be?
- */
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
Don't you want to wait until the ZLP has completed before doing the
giveback?
In fact, shouldn't all this ZLP code run when the transfer is
submitted, rather than when it completes? The idea is that you get a
request, you queue up all the necessary TRBs or whatever, and if an
extra ZLP is needed then you queue up an extra TRB.
+
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-zero) {
Is this correct in the case where ur-length is 0? When that happens,
there should be only one ZLP, not two.
+ int ret;
+
+ dwc-ep0_next_event = DWC3_EP0_COMPLETE;
+
+ ret = dwc3_ep0_start_trans(dwc, epnum,
+ dwc-ctrl_req_addr, 0,
+ DWC3_TRBCTL_CONTROL_DATA);
+ WARN_ON(ret 0);
+ }
}
}
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ehci/ohci: use SIMPLE_DEV_PM_OPS to support hibernation
On Thu, Oct 23, 2014 at 6:48 AM, Wonhong Kwon wonhongk...@gmail.com wrote: -#else /* !CONFIG_PM */ +#else /* !CONFIG_PM_SLEEP */ #define ehci_platform_suspend NULL #define ehci_platform_resume NULL No need for these NULL defines anymore as you switched to SIMPLE_DEV_PM_OPS. -#else /* !CONFIG_PM */ +#else /* !CONFIG_PM_SLEEP */ #define ohci_platform_suspend NULL #define ohci_platform_resume NULL Same here. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi,
On Thu, Oct 23, 2014 at 10:18:27AM -0400, Alan Stern wrote:
On Thu, 23 Oct 2014, Felipe Balbi wrote:
here's v2:
8--
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00 2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned
to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
-* handle the case where we have to send a zero packet. This
-* seems to be case when req.length maxpacket. Could it be?
-*/
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
Don't you want to wait until the ZLP has completed before doing the
giveback?
In fact, shouldn't all this ZLP code run when the transfer is
submitted, rather than when it completes? The idea is that you get a
request, you queue up all the necessary TRBs or whatever, and if an
extra ZLP is needed then you queue up an extra TRB.
yeah, this is desirable but it would require a bigger rework of how we
handle TRBs in dwc3. Currently, for ep0, we have a single TRB and adding
another wouldn't fit into an -rc release. I'll see what I can do to make
this better but as of now we need this bug fix.
+
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-zero) {
Is this correct in the case where ur-length is 0? When that happens,
there should be only one ZLP, not two.
and here I was assuming IS_ALIGNED() would handle that case. Here's v3.
8--
From 36f84ffb45c75ff10442a9f8f2fd91dcf6c6f5dd Mon Sep 17 00:00:00 2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..711b230 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
-* handle the case where we have to send a zero packet. This
-* seems to be case when req.length maxpacket. Could it be?
-*/
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
+
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-length ur-zero) {
+ int ret;
+
+
Re: [PATCH v2 04/10] usb: dwc2/gadget: disable phy before turning off power regulators
On Mon, Oct 20, 2014 at 12:45:34PM +0200, Marek Szyprowski wrote: This patch fixes probe function to match the pattern used elsewhere in the driver, where power regulators are turned off as the last element in the device shutdown procedure. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Paul, can I have your Acked-by here so I can send this to v3.18-rc1 ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 03/10] usb: dwc2/gadget: fix gadget unregistration in udc_stop() function
On Mon, Oct 20, 2014 at 12:45:33PM +0200, Marek Szyprowski wrote: udc_stop() should clear -driver pointer unconditionally to let the UDC framework to work correctly with both registering/unregistering gadgets and enabling/disabling gadgets by writing to /sys/class/udc/*hsotg/soft_connect interface. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Also here. In fact, this is much more important :-) -- balbi signature.asc Description: Digital signature
Re: [resend PATCH 1/3] ACPI / platform: provide default DMA mask
On Wed, Oct 22, 2014 at 04:19:45PM +0200, Rafael J. Wysocki wrote: On Wednesday, October 22, 2014 10:33:08 AM Adrian Hunter wrote: On 21/10/14 16:13, Adrian Hunter wrote: On 21/10/14 16:08, Rafael J. Wysocki wrote: On Tuesday, October 21, 2014 03:27:45 PM Adrian Hunter wrote: On 24/09/14 16:49, Rafael J. Wysocki wrote: On Wednesday, September 24, 2014 11:00:37 AM Heikki Krogerus wrote: Most devices are configured for 32-bit DMA addresses. Setting the mask to 32-bit here removes the need for the drivers to do it separately. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: Rafael J. Wysocki r...@rjwysocki.net ACK Hi I need this for another driver. Did this patch go anywhere? It's not in 3.18-rc1 as far as I can say. Note Heikki is away at the moment. That's OK. I guess I can queue up this one for you. When do you need this to get into the Linus' tree? My patches are aiming for 3.19 Actually it would be easier for me if Heikki's patch is already in 3.18, but all I need to know is when it will hit? I've applied the Heiki's patch and I'm going to push it for 3.18-rc2. I had this queued for v3.19 with your Ack. It didn't feel like it should be in v3.18-rc as it's not really fixing any bug. In any case, fine by me; I'll drop it from my queue. -- balbi signature.asc Description: Digital signature
Re: TI Tiva Launchpad Luminary ICDI Bug
Moritz wrote: With a recent Update from Kernel 3.16.14-1-x86_64 to linux-3.17.1-1-x86_64 my machine isn't able anymore to recognise my TI Tiva Lauchpad. lsusb normally has the following entry: Bus 004 Device 010: ID 1cbe:00fd Luminary Micro Inc. In-Circuit Debug Interface It vanishes with 3.17. I downgraded to 3.16 and it works again. Please tell us more about your machine, in particular which USB host controller it has. It would also be helpful if you could try to reproduce the problem with the actual upstream 3.17.1 kernel, instead of whatever modifications your distribution includes. It's unfortunately impossible to say much about this problem without more information from you. //Peter pgpgJxfvabiPK.pgp Description: PGP signature
Re: [PATCH v8 1/4] mfd: add support for Diolan DLN-2 devices
On Wed, Oct 15, 2014 at 05:48:08PM +0300, Octavian Purdila wrote:
Here's some late feedback due to travels. You managed to get two updates
in there so commenting on the diff from v6.
+struct dln2_event_cb_entry {
+ struct list_head list;
+ u16 id;
+ struct platform_device *pdev;
+ dln2_event_cb_t callback;
+};
+
+int dln2_register_event_cb(struct platform_device *pdev, u16 id,
+dln2_event_cb_t rx_cb)
+{
+ struct dln2_dev *dln2 = dev_get_drvdata(pdev-dev.parent);
+ struct dln2_event_cb_entry *i, *new_cb;
Can you use a name which does not have the same suffix as the actual
callback function (i.e. _cb). Just call it entry or something.
+ unsigned long flags;
+ int ret = 0;
+
+ new_cb = kmalloc(sizeof(*new_cb), GFP_KERNEL);
Use kzalloc here.
+ if (!new_cb)
+ return -ENOMEM;
+
+ new_cb-id = id;
+ new_cb-callback = rx_cb;
+ new_cb-pdev = pdev;
+
+ spin_lock_irqsave(dln2-event_cb_lock, flags);
+
+ list_for_each_entry(i, dln2-event_cb_list, list) {
+ if (i-id == id) {
+ ret = -EBUSY;
+ break;
+ }
+ }
+
+ if (!ret)
+ list_add_rcu(new_cb-list, dln2-event_cb_list);
+
+ spin_unlock_irqrestore(dln2-event_cb_lock, flags);
+
+ if (ret)
+ kfree(new_cb);
+
+ return ret;
+}
+EXPORT_SYMBOL(dln2_register_event_cb);
+
+void dln2_unregister_event_cb(struct platform_device *pdev, u16 id)
+{
+ struct dln2_dev *dln2 = dev_get_drvdata(pdev-dev.parent);
+ struct dln2_event_cb_entry *i;
+ unsigned long flags;
+ bool found = false;
+
+ spin_lock_irqsave(dln2-event_cb_lock, flags);
+
+ list_for_each_entry(i, dln2-event_cb_list, list) {
+ if (i-id == id) {
+ list_del_rcu(i-list);
+ found = true;
+ break;
+ }
+ }
+
+ spin_unlock_irqrestore(dln2-event_cb_lock, flags);
+
+ if (found) {
+ synchronize_rcu();
+ kfree(i);
+ }
+}
+EXPORT_SYMBOL(dln2_unregister_event_cb);
+
Please add a comment describing the return value (i.e. when the urb had
been saved and shouldn't be resubmitted).
Also could rename this helper so it doesn't appear to be a variant of
dln2_transfer (e.g. something with complete in the name).
+static bool dln2_rx_transfer(struct dln2_dev *dln2, struct urb *urb,
+ u16 handle, u16 rx_slot)
+{
+ struct dln2_mod_rx_slots *rxs = dln2-mod_rx_slots[handle];
+ struct dln2_rx_context *rxc;
+ bool ret = false;
+
+ rxc = rxs-slots[rx_slot];
+
+ /*
+ * No need to disable interrupts as this lock is not taken in
+ * interrupt context elsewhere in this driver and this
+ * function (or its callers) are not exported to other
+ * modules.
Why do you break comment lines already at 70 chars?
+ */
+ spin_lock(rxs-lock);
+ if (rxc-in_use !rxc-urb) {
+ rxc-urb = urb;
+ complete(rxc-done);
+ ret = true;
+ }
+ spin_unlock(rxs-lock);
+
+ return ret;
+}
+
+static void dln2_run_event_callbacks(struct dln2_dev *dln2, u16 id, u16 echo,
+ void *data, int len)
+{
+ struct dln2_event_cb_entry *i;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(i, dln2-event_cb_list, list)
+ if (i-id == id)
+ i-callback(i-pdev, echo, data, len);
+
+ rcu_read_unlock();
+}
+
+static void dln2_rx(struct urb *urb)
+{
+ struct dln2_dev *dln2 = urb-context;
+ struct dln2_header *hdr = urb-transfer_buffer;
+ struct device *dev = dln2-interface-dev;
+ u16 id, echo, handle, size;
+ u8 *data;
+ int len;
+ int err;
+
+ switch (urb-status) {
+ case 0:
+ /* success */
+ break;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ case -EPIPE:
+ /* this urb is terminated, clean up */
+ dev_dbg(dev, urb shutting down with status %d\n, urb-status);
+ return;
+ default:
+ dev_dbg(dev, nonzero urb status received %d\n, urb-status);
+ goto out;
+ }
+
+ if (urb-actual_length sizeof(struct dln2_header)) {
+ dev_err(dev, short response: %d\n, urb-actual_length);
+ goto out;
+ }
+
+ handle = le16_to_cpu(hdr-handle);
+ id = le16_to_cpu(hdr-id);
+ echo = le16_to_cpu(hdr-echo);
+ size = le16_to_cpu(hdr-size);
+
+ if (size != urb-actual_length) {
+ dev_err(dev, size mismatch: handle %x cmd %x echo %x size %d
actual %d\n,
+ handle, id, echo, size, urb-actual_length);
+ goto out;
+ }
+
+ if (handle = DLN2_HANDLES) {
+ dev_warn(dev,
Re: [PATCH v8 2/4] i2c: add support for Diolan DLN-2 USB-I2C adapter
On Wed, Oct 15, 2014 at 05:48:09PM +0300, Octavian Purdila wrote:
+static int dln2_i2c_xfer(struct i2c_adapter *adapter,
+ struct i2c_msg *msgs, int num)
+{
+ struct dln2_i2c *dln2 = i2c_get_adapdata(adapter);
+ struct i2c_msg *pmsg;
+ struct device *dev = dln2-pdev-dev;
You want to use the i2c-adapter device here (not the platform device).
+ int i;
+
+ for (i = 0; i num; i++) {
+ int ret;
+
+ pmsg = msgs[i];
+
+ if (pmsg-len DLN2_I2C_MAX_XFER_SIZE) {
+ dev_warn(dev, maximum transfer size exceeded\n);
+ return -EOPNOTSUPP;
+ }
+
+ if (pmsg-flags I2C_M_RD) {
+ ret = dln2_i2c_read(dln2, pmsg-addr, pmsg-buf,
+ pmsg-len);
+ if (ret 0)
+ return ret;
+
+ pmsg-len = ret;
+ } else {
+ ret = dln2_i2c_write(dln2, pmsg-addr, pmsg-buf,
+ pmsg-len);
+ if (ret != pmsg-len)
+ return -EPROTO;
+ }
+ }
+
+ return num;
+}
[...]
+static int dln2_i2c_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct dln2_i2c *dln2;
+ struct device *dev = pdev-dev;
+ struct dln2_platform_data *pdata = dev_get_platdata(pdev-dev);
+
+ dln2 = devm_kzalloc(dev, sizeof(*dln2), GFP_KERNEL);
+ if (!dln2)
+ return -ENOMEM;
+
+ dln2-buf = devm_kmalloc(dev, DLN2_I2C_BUF_SIZE, GFP_KERNEL);
+ if (!dln2-buf)
+ return -ENOMEM;
+
+ dln2-pdev = pdev;
+ dln2-port = pdata-port;
+
+ /* setup i2c adapter description */
+ dln2-adapter.owner = THIS_MODULE;
+ dln2-adapter.class = I2C_CLASS_HWMON;
+ dln2-adapter.algo = dln2_i2c_usb_algorithm;
+ dln2-adapter.dev.parent = dev;
+ i2c_set_adapdata(dln2-adapter, dln2);
+ snprintf(dln2-adapter.name, sizeof(dln2-adapter.name), %s-%s-%d,
+ dln2-i2c, dev_name(pdev-dev.parent), dln2-port);
+
+ platform_set_drvdata(pdev, dln2);
+
+ /* initialize the i2c interface */
No longer initialisation since you dropped the frequency setup?
+ ret = dln2_i2c_enable(dln2, true);
+ if (ret 0) {
+ dev_err(dev, failed to initialize adapter: %d\n, ret);
Same here.
+ return ret;
+ }
+
+ /* and finally attach to i2c layer */
+ ret = i2c_add_adapter(dln2-adapter);
+ if (ret 0) {
+ dev_err(dev, failed to add I2C adapter: %d\n, ret);
+ goto out_disable;
+ }
+
+ return 0;
+
+out_disable:
+ dln2_i2c_enable(dln2, false);
+
+ return ret;
+}
Looks good otherwise.
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: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role
On 10/22/2014 03:27 PM, Paul Bolle wrote: On Tue, 2014-10-21 at 15:47 -0500, Dinh Nguyen wrote: On 10/20/2014 02:42 PM, Paul Bolle wrote: (Side note: drivers/usb/dwc2/Kconfig is sourced (in drivers/usb/Kconfig) even if USB is _not_ set. But USB_DCW2 still depends on USB. Why is that?) Because USB is for Host-Side support. DWC2 driver should only get built for when USB is enabled. I think you're getting USB and USB_SUPPORT mixed up. No, I don't think I did. Because in drivers/usb/Kconfig we see if USB [...] endif [...] source drivers/usb/dwc2/Kconfig [...] Well, CONFIG_USB enables the host stack that is needed by the DWC2 driver, without CONFIG_USB, the DWC2 driver will not be able to build. In addition, thanks to your comment, I realized that DWC2 should also be available to build when USB_GADGET is enabled. A patch has been sent: http://marc.info/?l=linux-usbm=141392377124818w=2 I haven't checked, but doesn't this mean USB_DWC2 could just depend on USB_SUPPORT? I don't think so because USB_SUPPORT will not select USB or USB_GAGDGET, and the DWC2 driver will need either 1 or both for it to build correctly. Dinh -- 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: dwc3: gadget: Properly initialize LINK TRB
On Thu, Oct 23, 2014 at 09:10:03AM -0500, Felipe Balbi wrote: Fixes: f6bafc6a1c (usb: dwc3: convert TRBs into bitshifts) Signed-off-by: Jack Pham ja...@codeaurora.org Cc: sta...@vger.kernel.org you should add a # v3.4+ after stable email, I'll add that myself this time. Good to know for next time. Thanks Felipe! -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role
On Thu, 2014-10-23 at 10:05 -0500, Dinh Nguyen wrote: On 10/22/2014 03:27 PM, Paul Bolle wrote: On Tue, 2014-10-21 at 15:47 -0500, Dinh Nguyen wrote: Well, CONFIG_USB enables the host stack that is needed by the DWC2 driver, without CONFIG_USB, the DWC2 driver will not be able to build. In addition, thanks to your comment, I realized that DWC2 should also be available to build when USB_GADGET is enabled. A patch has been sent: http://marc.info/?l=linux-usbm=141392377124818w=2 Doesn't that patch contradict your statement? It allows USB_DWC2 to be set even if USB is not set. I haven't checked, but doesn't this mean USB_DWC2 could just depend on USB_SUPPORT? I don't think so because USB_SUPPORT will not select USB or USB_GAGDGET, and the DWC2 driver will need either 1 or both for it to build correctly. My comment was not well thought through. It was a waste of your time. Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 04/10] usb: dwc2/gadget: disable phy before turning off power regulators
From: Felipe Balbi [mailto:ba...@ti.com] Sent: Thursday, October 23, 2014 8:01 AM On Mon, Oct 20, 2014 at 12:45:34PM +0200, Marek Szyprowski wrote: This patch fixes probe function to match the pattern used elsewhere in the driver, where power regulators are turned off as the last element in the device shutdown procedure. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Paul, can I have your Acked-by here so I can send this to v3.18-rc1 ? Sorry, I've been working on a really hot issue at $work and didn't have time yet to review this series. Acked-by: Paul Zimmerman pa...@synopsys.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 v2 03/10] usb: dwc2/gadget: fix gadget unregistration in udc_stop() function
From: Felipe Balbi [mailto:ba...@ti.com] Sent: Thursday, October 23, 2014 8:02 AM On Mon, Oct 20, 2014 at 12:45:33PM +0200, Marek Szyprowski wrote: udc_stop() should clear -driver pointer unconditionally to let the UDC framework to work correctly with both registering/unregistering gadgets and enabling/disabling gadgets by writing to /sys/class/udc/*hsotg/soft_connect interface. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Also here. In fact, this is much more important :-) Acked-by: Paul Zimmerman pa...@synopsys.com Are there any more patches in this series that need immediate attention? Otherwise I plan to finish reviewing the series on Friday or so. -- Paul -- 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: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role
From: Bartlomiej Zolnierkiewicz [mailto:b.zolnier...@samsung.com] Sent: Wednesday, October 22, 2014 5:26 AM On Monday, October 20, 2014 01:52:06 PM dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com config USB_DWC2_PLATFORM bool DWC2 Platform - depends on USB_DWC2_HOST default USB_DWC2_HOST It should be default USB_DWC2_HOST || USB_DWC2_PERIPHERAL because USB_DWC2_PLATFORM replaces current USB_DWC2_PERIPHERAL functionality. Additionaly USB_DWC2_PLATFORM should be changed to tristate (USB_DWC2_PERIPHERAL was a tristeate before your changes). Dinh, I think this is a good point. Is there any reason why USB_DWC2_PLATFORM (and USB_DWC2_PCI for that matter) can't be tristate? That's what DWC3 does. -- Paul -- 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: [PATCHv5 7/7] usb: dwc2: Update Kconfig to support dual-role
On 10/23/2014 01:28 PM, Paul Zimmerman wrote: From: Bartlomiej Zolnierkiewicz [mailto:b.zolnier...@samsung.com] Sent: Wednesday, October 22, 2014 5:26 AM On Monday, October 20, 2014 01:52:06 PM dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com config USB_DWC2_PLATFORM bool DWC2 Platform - depends on USB_DWC2_HOST default USB_DWC2_HOST It should be default USB_DWC2_HOST || USB_DWC2_PERIPHERAL because USB_DWC2_PLATFORM replaces current USB_DWC2_PERIPHERAL functionality. Additionaly USB_DWC2_PLATFORM should be changed to tristate (USB_DWC2_PERIPHERAL was a tristeate before your changes). Dinh, I think this is a good point. Is there any reason why USB_DWC2_PLATFORM (and USB_DWC2_PCI for that matter) can't be tristate? That's what DWC3 does. Yes, in my v6, I have made this change. Thanks, Dinh -- 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 03/10] usb: dwc2/gadget: fix gadget unregistration in udc_stop() function
On Thu, Oct 23, 2014 at 06:18:51PM +, Paul Zimmerman wrote: From: Felipe Balbi [mailto:ba...@ti.com] Sent: Thursday, October 23, 2014 8:02 AM On Mon, Oct 20, 2014 at 12:45:33PM +0200, Marek Szyprowski wrote: udc_stop() should clear -driver pointer unconditionally to let the UDC framework to work correctly with both registering/unregistering gadgets and enabling/disabling gadgets by writing to /sys/class/udc/*hsotg/soft_connect interface. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Also here. In fact, this is much more important :-) Acked-by: Paul Zimmerman pa...@synopsys.com Are there any more patches in this series that need immediate attention? Otherwise I plan to finish reviewing the series on Friday or so. no, this is the most important one. Everything else can wait until Friday, thanks. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 04/10] usb: dwc2/gadget: disable phy before turning off power regulators
On Thu, Oct 23, 2014 at 06:15:57PM +, Paul Zimmerman wrote: From: Felipe Balbi [mailto:ba...@ti.com] Sent: Thursday, October 23, 2014 8:01 AM On Mon, Oct 20, 2014 at 12:45:34PM +0200, Marek Szyprowski wrote: This patch fixes probe function to match the pattern used elsewhere in the driver, where power regulators are turned off as the last element in the device shutdown procedure. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Paul, can I have your Acked-by here so I can send this to v3.18-rc1 ? Sorry, I've been working on a really hot issue at $work and didn't have time yet to review this series. don't worry, we've all been there :-) thanks -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: gadget: Properly initialize LINK TRB
On Thu, Oct 23, 2014 at 09:36:14AM -0700, Jack Pham wrote: On Thu, Oct 23, 2014 at 09:10:03AM -0500, Felipe Balbi wrote: Fixes: f6bafc6a1c (usb: dwc3: convert TRBs into bitshifts) Signed-off-by: Jack Pham ja...@codeaurora.org Cc: sta...@vger.kernel.org you should add a # v3.4+ after stable email, I'll add that myself this time. Good to know for next time. Thanks Felipe! no problem ;-) -- balbi signature.asc Description: Digital signature
Re: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code
On Wed, Oct 22, 2014 at 08:54:00PM +, Paul Zimmerman wrote: From: Bartlomiej Zolnierkiewicz [mailto:b.zolnier...@samsung.com] Sent: Wednesday, October 22, 2014 4:16 AM On Monday, October 20, 2014 01:52:01 PM dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com This patch will aggregate the probing of gadget/hcd driver into platform.c. The gadget probe funtion is converted into gadget_init that is now only responsible for gadget only initialization. All the gadget resources is now handled by platform.c Since the host workqueue will not get initialized if the driver is configured for peripheral mode only. Thus we need to check for wq_otg before calling queue_work(). Also, we move spin_lock_init to common location for both host and gadget that is either in platform.c or pci.c. We also ove suspend/resume code to common platform code, and update it to use the new PM API (struct dev_pm_ops). Lastly, move the samsung,s3c6400-hsotg binding into dwc2_of_match_table. This patch seems to break bisectability. It moves all the gadget probing to platform.c but Kconfig/Makefile are not updated (platform.c will be compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2). It doesn't break the compile, I already tested it. It does break the operation of the driver until patch #7 is applied, but I think that's OK in the middle of a patch series. I think it's a bit much to expect the driver to keep working at each step of a patch series like this. It's your driver and, at the end of the day, your headache; but the very day you need to run a git bisect and you end up in the middle of one of these commits, you'll regret this statement :-) There's usually a way to make sure things continue to work even if it means duplicating some code until the conversion is completed, or adding temporary flags which get added and removed within the same series, etc. my 2 cents -- balbi signature.asc Description: Digital signature
[PATCH] omap2430: use MUSB_DEVCTL_BDEVICE
The OMAP2+ MUSB glue layer still uses a bare number for the DEVCTL.B-Device bit
in one place, while there's #define MUSB_DEVCTL_BDEVICE for that.
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
---
The patch is against the 'next' branch of Felipe Balbi's 'usb.git' repo.
drivers/usb/musb/omap2430.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: usb/drivers/usb/musb/omap2430.c
===
--- usb.orig/drivers/usb/musb/omap2430.c
+++ usb/drivers/usb/musb/omap2430.c
@@ -162,7 +162,8 @@ static void omap2430_musb_set_vbus(struc
* Wait for the musb to set as A device to enable the
* VBUS
*/
- while (musb_readb(musb-mregs, MUSB_DEVCTL) 0x80) {
+ while (musb_readb(musb-mregs, MUSB_DEVCTL)
+ MUSB_DEVCTL_BDEVICE) {
mdelay(5);
cpu_relax();
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel 3.17.1: fail to use USB3 device after resuming from suspend
On Thursday, October 23, 2014 10:15:50 PM Francis Moreau wrote: Hello, After resuming from a suspend (to RAM), I can't use an external USB hard drive anymore, the kernel seems to fail to detect it. Here is the kernel log when doing a suspend/resume cycle. [Oct23 22:03] wlp2s0: deauthenticating from 92:23:b1:f9:54:e4 by local choice (Reason: 3=DEAUTH_LEAVING) [ +0.025152] cfg80211: Calling CRDA to update world regulatory domain [ +0.052636] IPv6: ADDRCONF(NETDEV_UP): wlp2s0: link is not ready [ +0.071175] PM: Syncing filesystems ... done. [ +0.099223] PM: Preparing system for mem sleep [ +0.000347] Freezing user space processes ... (elapsed 0.001 seconds) done. [ +0.001406] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ +0.001083] PM: Entering mem sleep [ +0.17] Suspending console(s) (use no_console_suspend to debug) [ +0.000264] sd 4:0:0:0: [sdb] Synchronizing SCSI cache [ +0.36] sd 4:0:0:0: [sdb] Stopping disk [ +0.04] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ +0.49] sd 0:0:0:0: [sda] Stopping disk [ +1.058316] PM: suspend of devices complete after 1057.429 msecs [ +0.013434] PM: late suspend of devices complete after 13.410 msecs [ +0.000875] ehci-pci :00:1d.0: System wakeup enabled by ACPI [ +0.13] r8169 :03:00.2: System wakeup enabled by ACPI [ +0.000101] ehci-pci :00:1a.0: System wakeup enabled by ACPI [ +0.47] xhci_hcd :00:14.0: System wakeup enabled by ACPI [ +0.012356] PM: noirq suspend of devices complete after 13.373 msecs [ +0.000401] ACPI: Preparing to enter system sleep state S3 [ +0.002149] PM: Saving platform NVS memory [ +0.06] Disabling non-boot CPUs ... [ +0.75] intel_pstate CPU 1 exiting [ +0.001363] kvm: disabling virtualization on CPU1 [ +0.28] smpboot: CPU 1 is now offline [ +0.000436] intel_pstate CPU 2 exiting [ +0.001348] kvm: disabling virtualization on CPU2 [ +0.100773] smpboot: CPU 2 is now offline [ +0.000322] intel_pstate CPU 3 exiting [ +0.001260] kvm: disabling virtualization on CPU3 [ +0.101855] smpboot: CPU 3 is now offline [ +0.000246] intel_pstate CPU 4 exiting [ +0.001181] kvm: disabling virtualization on CPU4 [ +0.102021] smpboot: CPU 4 is now offline [ +0.000396] intel_pstate CPU 5 exiting [ +0.001242] kvm: disabling virtualization on CPU5 [ +0.101801] smpboot: CPU 5 is now offline [ +0.000292] intel_pstate CPU 6 exiting [ +0.001301] kvm: disabling virtualization on CPU6 [ +0.101880] smpboot: CPU 6 is now offline [ +0.000496] intel_pstate CPU 7 exiting [ +0.001265] kvm: disabling virtualization on CPU7 [ +0.101649] smpboot: CPU 7 is now offline [ +0.002022] ACPI: Low-level resume complete [ +0.43] PM: Restoring platform NVS memory [ +0.000342] Enabling non-boot CPUs ... [ +0.47] x86: Booting SMP configuration: [ +0.02] smpboot: Booting Node 0 Processor 1 APIC 0x2 [ +0.011516] kvm: enabling virtualization on CPU1 [ +0.002301] CPU1 is up [ +0.25] smpboot: Booting Node 0 Processor 2 APIC 0x4 [ +0.011466] kvm: enabling virtualization on CPU2 [ +0.002307] CPU2 is up [ +0.22] smpboot: Booting Node 0 Processor 3 APIC 0x6 [ +0.011469] kvm: enabling virtualization on CPU3 [ +0.002305] CPU3 is up [ +0.22] smpboot: Booting Node 0 Processor 4 APIC 0x1 [ +0.011483] kvm: enabling virtualization on CPU4 [ +0.002298] CPU4 is up [ +0.18] smpboot: Booting Node 0 Processor 5 APIC 0x3 [ +0.011437] kvm: enabling virtualization on CPU5 [ +0.002305] CPU5 is up [ +0.17] smpboot: Booting Node 0 Processor 6 APIC 0x5 [ +0.011550] kvm: enabling virtualization on CPU6 [ +0.002297] CPU6 is up [ +0.17] smpboot: Booting Node 0 Processor 7 APIC 0x7 [ +0.011457] kvm: enabling virtualization on CPU7 [ +0.002312] CPU7 is up [ +0.006813] ACPI: Waking up from system sleep state S3 [ +0.046539] ehci-pci :00:1d.0: System wakeup disabled by ACPI [ +0.000267] ehci-pci :00:1a.0: System wakeup disabled by ACPI [ +0.000123] xhci_hcd :00:14.0: System wakeup disabled by ACPI [ +0.53] PM: noirq resume of devices complete after 13.005 msecs [ +0.000523] PM: early resume of devices complete after 0.479 msecs [ +0.000120] mei_me :00:16.0: irq 28 for MSI/MSI-X [ +0.47] r8169 :03:00.2: System wakeup disabled by ACPI [ +0.97] snd_hda_intel :00:1b.0: irq 29 for MSI/MSI-X [ +0.003073] rtc_cmos 00:02: System wakeup disabled by ACPI [ +0.008776] sd 4:0:0:0: [sdb] Starting disk [ +0.09] sd 0:0:0:0: [sda] Starting disk [ +0.053104] r8169 :03:00.2 enp3s0f2: link down [ +0.254016] usb 1-4: reset full-speed USB device number 2 using xhci_hcd [ +0.16] xhci_hcd :00:14.0: Setup ERROR: setup context command for slot 1. [ +0.03] usb 1-4: hub failed to enable device, error -22 [ +0.019787] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300) [ +0.002347] ata3.00: configured for
Re: [resend PATCH 1/3] ACPI / platform: provide default DMA mask
On Thursday, October 23, 2014 10:06:05 AM Felipe Balbi wrote: --HSQ3hISbU3Um6hch Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 22, 2014 at 04:19:45PM +0200, Rafael J. Wysocki wrote: On Wednesday, October 22, 2014 10:33:08 AM Adrian Hunter wrote: On 21/10/14 16:13, Adrian Hunter wrote: On 21/10/14 16:08, Rafael J. Wysocki wrote: On Tuesday, October 21, 2014 03:27:45 PM Adrian Hunter wrote: On 24/09/14 16:49, Rafael J. Wysocki wrote: On Wednesday, September 24, 2014 11:00:37 AM Heikki Krogerus wrote: Most devices are configured for 32-bit DMA addresses. Setting the mask to 32-bit here removes the need for the drivers to do it separately. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: Rafael J. Wysocki r...@rjwysocki.net ACK Hi I need this for another driver. Did this patch go anywhere? It's not in 3.18-rc1 as far as I can say. Note Heikki is away at the moment. That's OK. I guess I can queue up this one for you. When do you need this to get into the Linus' tree? =20 My patches are aiming for 3.19 =20 Actually it would be easier for me if Heikki's patch is already in 3.18, but all I need to know is when it will hit? =20 I've applied the Heiki's patch and I'm going to push it for 3.18-rc2. I had this queued for v3.19 with your Ack. It didn't feel like it should be in v3.18-rc as it's not really fixing any bug. In any case, fine by me; I'll drop it from my queue. OK, thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi,
On Thu, 23 Oct 2014, Alan Stern wrote:
On Thu, 23 Oct 2014, Felipe Balbi wrote:
here's v2:
8--
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00
2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes
aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3
*dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
-* handle the case where we have to send a zero packet.
This
-* seems to be case when req.length maxpacket. Could it
be?
-*/
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
Don't you want to wait until the ZLP has completed before doing the
giveback?
In fact, shouldn't all this ZLP code run when the transfer is
submitted, rather than when it completes? The idea is that you get a
request, you queue up all the necessary TRBs or whatever, and if an
extra ZLP is needed then you queue up an extra TRB.
You may want to check my patch one more time. I sent it for review 10
monthes ago:
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
It works just fine for us in our custom kernel.
+
+ if (IS_ALIGNED(ur-length, ep0-endpoint.maxpacket)
+ ur-zero) {
Is this correct in the case where ur-length is 0? When that happens,
there should be only one ZLP, not two.
+ int ret;
+
+ dwc-ep0_next_event = DWC3_EP0_COMPLETE;
+
+ ret = dwc3_ep0_start_trans(dwc, epnum,
+ dwc-ctrl_req_addr, 0,
+ DWC3_TRBCTL_CONTROL_DATA);
+ WARN_ON(ret 0);
+ }
}
}
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
HI,
On Fri, Oct 24, 2014 at 10:14:19AM +0900, Anton Tikhomirov wrote:
Hi,
On Thu, 23 Oct 2014, Alan Stern wrote:
On Thu, 23 Oct 2014, Felipe Balbi wrote:
here's v2:
8--
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00
2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes
aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3
*dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
- * handle the case where we have to send a zero packet.
This
- * seems to be case when req.length maxpacket. Could it
be?
- */
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
Don't you want to wait until the ZLP has completed before doing the
giveback?
In fact, shouldn't all this ZLP code run when the transfer is
submitted, rather than when it completes? The idea is that you get a
request, you queue up all the necessary TRBs or whatever, and if an
extra ZLP is needed then you queue up an extra TRB.
You may want to check my patch one more time. I sent it for review 10
monthes ago:
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
It works just fine for us in our custom kernel.
you also said you'd send another version (see [1]) and never did. 10
months passed and I had even forgotten about this issue until I decided
to run all gadget drivers through USB[23]0CV and found that g_ncm falls
into this same bug, so I wrote the patch above.
considering you never sent another version even after 10 months, I'll
just go ahead with this one and work on improving TRB handling on this
driver so that when req-zero is true we can actually allocate a
separate TRB (as has been discussed on this and previous threads).
I'll add a note to commit log stating that you provided the original
patch but failed to provide a follow up.
[1] http://www.spinics.net/lists/linux-usb/msg99809.html
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: serial: Add bricked FTDI device PID
On Thu, Oct 23, 2014 at 04:27:50PM +0800, Perry Hung wrote: It doesn't seem so, there's an IDA pro dump from somebody who decompiled the driver: https://twitter.com/marcan42/status/525126731431038977/photo/1 A picture of the source code created by disassembling a binary driver that was written to brick a cloned USB controller device. Reverse engineering inception layers... And I used a Windows disassembler/decompiler... running on Wine, which itself is a clone of the Windows API. :-) FWIW, here's a more accurate analysis, after I had time to figure out exactly what's going on: https://marcan.st/transf/ftdi_evil.png The code only reprograms the PID to 0, but leave the VID alone. FTDI's driver's INF file only registers it for devices under FTDI's own VID, so it should be safe to say that this update will only bork devices with VID 0403, and so we only need to add 0403/ to the ID list. FTDI's code also only touches devices with bcdDevice 0xFF00 == 0x600 (because, amusingly, the code will actually brick other legitimate FTDI devices, just not FT232RL due to its specific EEPROM quirk): https://marcan.st/transf/checkbrick.png Amusing note: they're evidently missing a return statement for non-232RL devices, which ended up returning the dev pointer as garbage, which confused the decompiler as to the type of the return value. It's ignored anyway, but it seems they don't care about compiler warnings! If some manufacturer incorporates this update into their own driver package, with a custom INF and VID, and they are also shipping counterfeit/clone chips, then we could end up with another VID also being affected, but this is probably relatively unlikely (hopefully those shipping their own customized drivers are aware of this saga and will either not ship this build or will make damn sure they haven't sold any devices with clones). (I hope I got the In-Reply-To header right this time around...) -- Hector Martin (hec...@marcansoft.com) Public Key: http://www.marcansoft.com/marcan.asc -- 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 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
On Thu, Oct 23, 2014 at 10:18:41PM -0500, Felipe Balbi wrote:
HI,
On Fri, Oct 24, 2014 at 10:14:19AM +0900, Anton Tikhomirov wrote:
Hi,
On Thu, 23 Oct 2014, Alan Stern wrote:
On Thu, 23 Oct 2014, Felipe Balbi wrote:
here's v2:
8--
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00
2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes
aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3
*dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
-* handle the case where we have to send a zero packet.
This
-* seems to be case when req.length maxpacket. Could
it
be?
-*/
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
Don't you want to wait until the ZLP has completed before doing the
giveback?
In fact, shouldn't all this ZLP code run when the transfer is
submitted, rather than when it completes? The idea is that you get a
request, you queue up all the necessary TRBs or whatever, and if an
extra ZLP is needed then you queue up an extra TRB.
You may want to check my patch one more time. I sent it for review 10
monthes ago:
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
It works just fine for us in our custom kernel.
you also said you'd send another version (see [1]) and never did. 10
months passed and I had even forgotten about this issue until I decided
to run all gadget drivers through USB[23]0CV and found that g_ncm falls
into this same bug, so I wrote the patch above.
considering you never sent another version even after 10 months, I'll
just go ahead with this one and work on improving TRB handling on this
driver so that when req-zero is true we can actually allocate a
separate TRB (as has been discussed on this and previous threads).
I'll add a note to commit log stating that you provided the original
patch but failed to provide a follow up.
actually, I can't do that anymore as I have already moved this commit to
my 'fixes' branch.
--
balbi
signature.asc
Description: Digital signature
RE: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize
Hi,
On Thu, Oct 23, 2014 at 10:18:41PM -0500, Felipe Balbi wrote:
HI,
On Fri, Oct 24, 2014 at 10:14:19AM +0900, Anton Tikhomirov wrote:
Hi,
On Thu, 23 Oct 2014, Alan Stern wrote:
On Thu, 23 Oct 2014, Felipe Balbi wrote:
here's v2:
8-
-
From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17
00:00:00
2001
From: Felipe Balbi ba...@ti.com
Date: Tue, 30 Sep 2014 10:39:14 -0500
Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer
sizes
aligned to
wMaxPacketSize
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..ce6b0c7 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct
dwc3
*dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
- * handle the case where we have to send a zero
packet.
This
- * seems to be case when req.length maxpacket.
Could it
be?
- */
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
Don't you want to wait until the ZLP has completed before doing
the
giveback?
In fact, shouldn't all this ZLP code run when the transfer is
submitted, rather than when it completes? The idea is that you
get a
request, you queue up all the necessary TRBs or whatever, and if
an
extra ZLP is needed then you queue up an extra TRB.
You may want to check my patch one more time. I sent it for review
10
monthes ago:
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
It works just fine for us in our custom kernel.
you also said you'd send another version (see [1]) and never did. 10
months passed and I had even forgotten about this issue until I
decided
to run all gadget drivers through USB[23]0CV and found that g_ncm
falls
into this same bug, so I wrote the patch above.
I'm sorry about this. I was busy with current project at that time and
finally forgot about this issue too.
considering you never sent another version even after 10 months, I'll
just go ahead with this one and work on improving TRB handling on
this
driver so that when req-zero is true we can actually allocate a
separate TRB (as has been discussed on this and previous threads).
I'll add a note to commit log stating that you provided the original
patch but failed to provide a follow up.
actually, I can't do that anymore as I have already moved this commit
to
my 'fixes' branch.
It's ok
--
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] ARM: shmobile: add USB3.0 device node on r8a7791
On Wed, Oct 08, 2014 at 03:27:55PM +0900, Yoshihiro Shimoda wrote: (2014/10/08 15:24), Yoshihiro Shimoda wrote: This patch is based on Simon's renesas.git branch and renesas-devel-20141007-v3.17 tag. Since koelsch and henninger doesn't have a USB3.0 connector, I submit a patch for r8a7791.dtsi only. Yoshihiro Shimoda (1): ARM: shmobile: r8a7791: add USB3.0 device node arch/arm/boot/dts/r8a7791.dtsi | 10 ++ 1 file changed, 10 insertions(+) Oops, this patch means [PATCH 0/1]... Also the [PATCH] ARM: shmobile: r8a7791: add USB3.0 device node email means [PATCH 1/1]. Hi Shimoda-san, could you update me on the status of this patch. Should I queue it up? -- 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 0/2] ARM: shmobile: add USB3.0 device node on r8a7790
On Wed, Oct 08, 2014 at 03:24:28PM +0900, Yoshihiro Shimoda wrote: This series is based on Simon's renesas.git branch and renesas-devel-20141007-v3.17 tag. If we use the generic phy driver for R-Car Gen2 (drivers/phy/phy-rcar-gen2.c), we can use the USB3.0 on lager. Yoshihiro Shimoda (2): ARM: shmobile: r8a7790: add USB3.0 device node ARM: shmobile: lager: enable USB3.0 arch/arm/boot/dts/r8a7790-lager.dts |6 ++ arch/arm/boot/dts/r8a7790.dtsi | 10 ++ 2 files changed, 16 insertions(+) Hi Shimoda-san, could you update me on the status of this patch. Should I queue it up? -- 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 0/2] ARM: shmobile: add USB3.0 device node on r8a7790
Hi Simon-san, On Wed, Oct 08, 2014 at 03:24:28PM +0900, Yoshihiro Shimoda wrote: This series is based on Simon's renesas.git branch and renesas-devel-20141007-v3.17 tag. If we use the generic phy driver for R-Car Gen2 (drivers/phy/phy-rcar-gen2.c), we can use the USB3.0 on lager. Yoshihiro Shimoda (2): ARM: shmobile: r8a7790: add USB3.0 device node ARM: shmobile: lager: enable USB3.0 arch/arm/boot/dts/r8a7790-lager.dts |6 ++ arch/arm/boot/dts/r8a7790.dtsi | 10 ++ 2 files changed, 16 insertions(+) Hi Shimoda-san, could you update me on the status of this patch. Should I queue it up? Yes. So, should I rebase this patch serias on the current your repository? Best regards, Yoshihiro Shimoda -- 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] ARM: shmobile: add USB3.0 device node on r8a7791
Hi Simon-san, On Wed, Oct 08, 2014 at 03:27:55PM +0900, Yoshihiro Shimoda wrote: (2014/10/08 15:24), Yoshihiro Shimoda wrote: This patch is based on Simon's renesas.git branch and renesas-devel-20141007-v3.17 tag. Since koelsch and henninger doesn't have a USB3.0 connector, I submit a patch for r8a7791.dtsi only. Yoshihiro Shimoda (1): ARM: shmobile: r8a7791: add USB3.0 device node arch/arm/boot/dts/r8a7791.dtsi | 10 ++ 1 file changed, 10 insertions(+) Oops, this patch means [PATCH 0/1]... Also the [PATCH] ARM: shmobile: r8a7791: add USB3.0 device node email means [PATCH 1/1]. Hi Shimoda-san, could you update me on the status of this patch. Should I queue it up? Yes. So, should I rebase this patch on the current your repository? Best regards, Yoshihiro Shimoda -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: ehci/ohci-platform: use SIMPLE_DEV_PM_OPS to support hibernation
ehci/ohci-platform just define .suspend/.resume functions for dev_pm_ops,
but in order to support both STR(suspend-to-ram) and hibernation, other
callbacks such as .freeze/.thaw are also required.
Registering all required callbacks for both STR and hibernation can
be done by SIMPLE_DEV_PM_OPS macro function.
Signed-off-by: Wonhong Kwon wonhong.k...@lge.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/host/ehci-platform.c | 15 ---
drivers/usb/host/ohci-platform.c | 15 ---
2 files changed, 8 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 2f5b9ce..da056fe 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -311,8 +311,7 @@ static int ehci_platform_remove(struct platform_device *dev)
return 0;
}
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
static int ehci_platform_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -348,11 +347,7 @@ static int ehci_platform_resume(struct device *dev)
ehci_resume(hcd, false);
return 0;
}
-
-#else /* !CONFIG_PM */
-#define ehci_platform_suspend NULL
-#define ehci_platform_resume NULL
-#endif /* CONFIG_PM */
+#endif /* CONFIG_PM_SLEEP */
static const struct of_device_id vt8500_ehci_ids[] = {
{ .compatible = via,vt8500-ehci, },
@@ -368,10 +363,8 @@ static const struct platform_device_id
ehci_platform_table[] = {
};
MODULE_DEVICE_TABLE(platform, ehci_platform_table);
-static const struct dev_pm_ops ehci_platform_pm_ops = {
- .suspend= ehci_platform_suspend,
- .resume = ehci_platform_resume,
-};
+static SIMPLE_DEV_PM_OPS(ehci_platform_pm_ops, ehci_platform_suspend,
+ ehci_platform_resume);
static struct platform_driver ehci_platform_driver = {
.id_table = ehci_platform_table,
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 4369299..c6f5e03 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -298,8 +298,7 @@ static int ohci_platform_remove(struct platform_device *dev)
return 0;
}
-#ifdef CONFIG_PM
-
+#ifdef CONFIG_PM_SLEEP
static int ohci_platform_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -335,11 +334,7 @@ static int ohci_platform_resume(struct device *dev)
ohci_resume(hcd, false);
return 0;
}
-
-#else /* !CONFIG_PM */
-#define ohci_platform_suspend NULL
-#define ohci_platform_resume NULL
-#endif /* CONFIG_PM */
+#endif /* CONFIG_PM_SLEEP */
static const struct of_device_id ohci_platform_ids[] = {
{ .compatible = generic-ohci, },
@@ -353,10 +348,8 @@ static const struct platform_device_id
ohci_platform_table[] = {
};
MODULE_DEVICE_TABLE(platform, ohci_platform_table);
-static const struct dev_pm_ops ohci_platform_pm_ops = {
- .suspend= ohci_platform_suspend,
- .resume = ohci_platform_resume,
-};
+static SIMPLE_DEV_PM_OPS(ohci_platform_pm_ops, ohci_platform_suspend,
+ ohci_platform_resume);
static struct platform_driver ohci_platform_driver = {
.id_table = ohci_platform_table,
--
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 0/2] ARM: shmobile: add USB3.0 device node on r8a7790
On Fri, Oct 24, 2014 at 04:44:28AM +, yoshihiro shimoda wrote: Hi Simon-san, On Wed, Oct 08, 2014 at 03:24:28PM +0900, Yoshihiro Shimoda wrote: This series is based on Simon's renesas.git branch and renesas-devel-20141007-v3.17 tag. If we use the generic phy driver for R-Car Gen2 (drivers/phy/phy-rcar-gen2.c), we can use the USB3.0 on lager. Yoshihiro Shimoda (2): ARM: shmobile: r8a7790: add USB3.0 device node ARM: shmobile: lager: enable USB3.0 arch/arm/boot/dts/r8a7790-lager.dts |6 ++ arch/arm/boot/dts/r8a7790.dtsi | 10 ++ 2 files changed, 16 insertions(+) Hi Shimoda-san, could you update me on the status of this patch. Should I queue it up? Yes. So, should I rebase this patch serias on the current your repository? Yes, please rebase it on renesas-devel-20141024-v3.18-rc1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: shmobile: add USB3.0 device node on r8a7791
On Fri, Oct 24, 2014 at 04:43:55AM +, yoshihiro shimoda wrote: Hi Simon-san, On Wed, Oct 08, 2014 at 03:27:55PM +0900, Yoshihiro Shimoda wrote: (2014/10/08 15:24), Yoshihiro Shimoda wrote: This patch is based on Simon's renesas.git branch and renesas-devel-20141007-v3.17 tag. Since koelsch and henninger doesn't have a USB3.0 connector, I submit a patch for r8a7791.dtsi only. Yoshihiro Shimoda (1): ARM: shmobile: r8a7791: add USB3.0 device node arch/arm/boot/dts/r8a7791.dtsi | 10 ++ 1 file changed, 10 insertions(+) Oops, this patch means [PATCH 0/1]... Also the [PATCH] ARM: shmobile: r8a7791: add USB3.0 device node email means [PATCH 1/1]. Hi Shimoda-san, could you update me on the status of this patch. Should I queue it up? Yes. So, should I rebase this patch on the current your repository? Yes, please rebase it on renesas-devel-20141024-v3.18-rc1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] Add HS-USB device tree support for R8A7790/Lager board
On Wed, Oct 08, 2014 at 11:53:11PM +0400, Sergei Shtylyov wrote: Hello. Here's the set of 2 patches against Simon Horman's 'renesas.git' repo, 'renesas-devel-20141007-v3.17' tag. Here we add the HS-USB device tree support on the R8A7790/Lager reference board. The patchset requires the USB PHY driver (already merged by Kishon and Greg) and the generic PHY support patches for the HS-USB driver posted by Yoshihiro Shimoda in order to work... [1/2] ARM: shmobile: r8a7790: add HS-USB device node [2/2] ARM: shmobile: lager: enable HS-USB Now that v3.18-rc1 is present in my devel branch should I queue-up these patches? If so, please rebase them on renesas-devel-20141024-v3.18-rc1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] Add HS-USB device tree support for R8A7790/Lager board
Hi Simon-san, (2014/10/24 14:09), Simon Horman wrote: On Wed, Oct 08, 2014 at 11:53:11PM +0400, Sergei Shtylyov wrote: Hello. Here's the set of 2 patches against Simon Horman's 'renesas.git' repo, 'renesas-devel-20141007-v3.17' tag. Here we add the HS-USB device tree support on the R8A7790/Lager reference board. The patchset requires the USB PHY driver (already merged by Kishon and Greg) and the generic PHY support patches for the HS-USB driver posted by Yoshihiro Shimoda in order to work... [1/2] ARM: shmobile: r8a7790: add HS-USB device node [2/2] ARM: shmobile: lager: enable HS-USB Now that v3.18-rc1 is present in my devel branch should I queue-up these patches? If so, please rebase them on renesas-devel-20141024-v3.18-rc1 Since the first patch set was made from me, I will rebase this. Best regards, Yoshihiro Shimoda -- To unsubscribe from this list: send the line unsubscribe linux-sh 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 v3 1/3] Add HS-USB device tree support for R8A7791/Koelsch/Henninger board
Hi Simon-san, (2014/10/09 2:59), Sergei Shtylyov wrote: Hello. Here's the set of 3 patches against Simon Horman's 'renesas.git' repo, 'renesas-devel-20141007-v3.17' tag. Here we add the HS-USB device tree support on the R8A7791/Koelsch/Henninger reference boards. The patchset requires the USB PHY driver (already merged by Kishon and Greg) and the generic PHY support patches for the HS-USB driver posted by Yoshihiro Shimoda in order to work... [1/3] ARM: shmobile: r8a7791: add HS-USB device node [2/3] ARM: shmobile: koelsch: enable HS-USB [3/3] ARM: shmobile: henninger: enable HS-USB I will also rebase this patch set. Best regards, Yoshihiro Shimoda WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
