Re: MUSB dual-role on AM335x behaving weirdly
Hi Felipe, On 27/05/2015 11:42, Alexandre Belloni wrote: Hi, On 26/05/2015 at 09:51:18 -0500, Felipe Balbi wrote : On Thu, May 14, 2015 at 04:36:33PM -0500, Bin Liu wrote: Alexandre, On Thu, May 14, 2015 at 4:26 PM, Alexandre Belloni alexandre.bell...@free-electrons.com wrote: On 14/05/2015 at 16:16:12 -0500, Bin Liu wrote : I think I found the root cause of the problem: board design issue - I bet the custom board has too much cap on VBUS line. It should be 10uF. We have a custom board that exhibits the issue but it only has a 100nF cap on VBUS. Have you measured the VBUS discharging? Is there any way to share your schematics? Alexandre, any further comments ? Yeah, I have just got more info. This is the relevant part of the schematic: http://free-electrons.com/~alexandre/usb.png The total VBUS capacitance is 200nF and the USB0 pins are connected directly to the AM3358 pins. U1 is actually not fitted. We didn't measure VBUS discharging but we observe the OTG pin sensing stops when plugging an OTG cable without any device. Do you have any news about this topic? Is there something else that we can do to help solving this issue? Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
These changes allow a PHY driver to trigger a VBUS interrupt and
to provide the value of VBUS.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v2:
- vbus variables changed from int to bool.
- dev_info() changed to dev_err()
---
drivers/usb/renesas_usbhs/common.h | 2 ++
drivers/usb/renesas_usbhs/mod.c| 3 +++
drivers/usb/renesas_usbhs/mod_gadget.c | 38 ++
3 files changed, 43 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.h
b/drivers/usb/renesas_usbhs/common.h
index 8c5fc12..478700c 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -255,6 +255,8 @@ struct usbhs_priv {
struct renesas_usbhs_driver_param dparam;
struct delayed_work notify_hotplug_work;
+ bool vbus_is_indirect;
+ bool vbus_indirect_value;
struct platform_device *pdev;
struct extcon_dev *edev;
diff --git a/drivers/usb/renesas_usbhs/mod.c b/drivers/usb/renesas_usbhs/mod.c
index 9a705b1..8a2d3dd 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -42,6 +42,9 @@ static int usbhsm_autonomy_get_vbus(struct platform_device
*pdev)
{
struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+ if (priv-vbus_is_indirect)
+ return priv-vbus_indirect_value;
+
return VBSTS usbhs_read(priv, INTSTS0);
}
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c
index dc2aa32..51b63f9 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -21,6 +21,7 @@
#include linux/platform_device.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include linux/usb/otg.h
#include common.h
/*
@@ -50,6 +51,7 @@ struct usbhsg_gpriv {
int uep_size;
struct usb_gadget_driver*driver;
+ struct usb_phy *transceiver;
u32 status;
#define USBHSG_STATUS_STARTED (1 0)
@@ -882,6 +884,8 @@ static int usbhsg_gadget_start(struct usb_gadget *gadget,
{
struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct device *dev = usbhs_priv_to_dev(priv);
+ int ret;
if (!driver ||
!driver-setup ||
@@ -891,6 +895,17 @@ static int usbhsg_gadget_start(struct usb_gadget *gadget,
/* first hook up the driver ... */
gpriv-driver = driver;
+ /* connect to bus through transceiver */
+ if (!IS_ERR_OR_NULL(gpriv-transceiver)) {
+ ret = otg_set_peripheral(gpriv-transceiver-otg,
+ gpriv-gadget);
+ if (ret) {
+ dev_err(dev, %s: can't bind to transceiver\n,
+ gpriv-gadget.name);
+ return ret;
+ }
+ }
+
return usbhsg_try_start(priv, USBHSG_STATUS_REGISTERD);
}
@@ -900,6 +915,10 @@ static int usbhsg_gadget_stop(struct usb_gadget *gadget)
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
usbhsg_try_stop(priv, USBHSG_STATUS_REGISTERD);
+
+ if (!IS_ERR_OR_NULL(gpriv-transceiver))
+ (void) otg_set_peripheral(gpriv-transceiver-otg, NULL);
+
gpriv-driver = NULL;
return 0;
@@ -947,12 +966,27 @@ static int usbhsg_set_selfpowered(struct usb_gadget
*gadget, int is_self)
return 0;
}
+static int usbhsg_vbus_session(struct usb_gadget *gadget, int is_active)
+{
+ struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+ struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct platform_device *pdev = usbhs_priv_to_pdev(priv);
+
+ priv-vbus_is_indirect = 1;
+ priv-vbus_indirect_value = !!is_active;
+
+ renesas_usbhs_call_notify_hotplug(pdev);
+
+ return 0;
+}
+
static const struct usb_gadget_ops usbhsg_gadget_ops = {
.get_frame = usbhsg_get_frame,
.set_selfpowered= usbhsg_set_selfpowered,
.udc_start = usbhsg_gadget_start,
.udc_stop = usbhsg_gadget_stop,
.pullup = usbhsg_pullup,
+ .vbus_session = usbhsg_vbus_session,
};
static int usbhsg_start(struct usbhs_priv *priv)
@@ -994,6 +1028,10 @@ int usbhs_mod_gadget_probe(struct usbhs_priv *priv)
goto usbhs_mod_gadget_probe_err_gpriv;
}
+ gpriv-transceiver = usb_get_phy(USB_PHY_TYPE_UNDEFINED);
+ dev_info(dev, %s transceiver found\n,
+gpriv-transceiver ? : No);
+
/*
* CAUTION
*
--
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 v2] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
Hi Phil,
(CC'ing Morimoto-san)
Thank you for the patch.
On Thursday 02 July 2015 08:36:42 Phil Edworthy wrote:
These changes allow a PHY driver to trigger a VBUS interrupt and
to provide the value of VBUS.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v2:
- vbus variables changed from int to bool.
- dev_info() changed to dev_err()
---
drivers/usb/renesas_usbhs/common.h | 2 ++
drivers/usb/renesas_usbhs/mod.c| 3 +++
drivers/usb/renesas_usbhs/mod_gadget.c | 38 +++
3 files changed, 43 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.h
b/drivers/usb/renesas_usbhs/common.h index 8c5fc12..478700c 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -255,6 +255,8 @@ struct usbhs_priv {
struct renesas_usbhs_driver_param dparam;
struct delayed_work notify_hotplug_work;
+ bool vbus_is_indirect;
+ bool vbus_indirect_value;
struct platform_device *pdev;
struct extcon_dev *edev;
diff --git a/drivers/usb/renesas_usbhs/mod.c
b/drivers/usb/renesas_usbhs/mod.c index 9a705b1..8a2d3dd 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -42,6 +42,9 @@ static int usbhsm_autonomy_get_vbus(struct platform_device
*pdev) {
struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+ if (priv-vbus_is_indirect)
+ return priv-vbus_indirect_value;
+
Something is bothering me. The comment block right before this function states
that these functions are used if platform doesn't have external phy.
However, the mechanism you implement here is aimed at letting a PHY control
vbus. I have a feeling this isn't the right mechanism.
return VBSTS usbhs_read(priv, INTSTS0);
}
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c index dc2aa32..51b63f9 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -21,6 +21,7 @@
#include linux/platform_device.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include linux/usb/otg.h
#include common.h
/*
@@ -50,6 +51,7 @@ struct usbhsg_gpriv {
int uep_size;
struct usb_gadget_driver*driver;
+ struct usb_phy *transceiver;
u32 status;
#define USBHSG_STATUS_STARTED(1 0)
@@ -882,6 +884,8 @@ static int usbhsg_gadget_start(struct usb_gadget
*gadget, {
struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct device *dev = usbhs_priv_to_dev(priv);
+ int ret;
if (!driver ||
!driver-setup ||
@@ -891,6 +895,17 @@ static int usbhsg_gadget_start(struct usb_gadget
*gadget, /* first hook up the driver ... */
gpriv-driver = driver;
+ /* connect to bus through transceiver */
+ if (!IS_ERR_OR_NULL(gpriv-transceiver)) {
+ ret = otg_set_peripheral(gpriv-transceiver-otg,
+ gpriv-gadget);
+ if (ret) {
+ dev_err(dev, %s: can't bind to transceiver\n,
+ gpriv-gadget.name);
Shouldn't gpriv-driver be set to NULL here ? Or possibly better, this code
block moved before setting gpriv-driver ?
+ return ret;
+ }
+ }
+
return usbhsg_try_start(priv, USBHSG_STATUS_REGISTERD);
}
@@ -900,6 +915,10 @@ static int usbhsg_gadget_stop(struct usb_gadget
*gadget) struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
usbhsg_try_stop(priv, USBHSG_STATUS_REGISTERD);
+
+ if (!IS_ERR_OR_NULL(gpriv-transceiver))
+ (void) otg_set_peripheral(gpriv-transceiver-otg, NULL);
Is there really a need for (void) ?
+
gpriv-driver = NULL;
return 0;
@@ -947,12 +966,27 @@ static int usbhsg_set_selfpowered(struct usb_gadget
*gadget, int is_self) return 0;
}
+static int usbhsg_vbus_session(struct usb_gadget *gadget, int is_active)
+{
+ struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+ struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct platform_device *pdev = usbhs_priv_to_pdev(priv);
+
+ priv-vbus_is_indirect = 1;
+ priv-vbus_indirect_value = !!is_active;
+
+ renesas_usbhs_call_notify_hotplug(pdev);
+
+ return 0;
+}
+
static const struct usb_gadget_ops usbhsg_gadget_ops = {
.get_frame = usbhsg_get_frame,
.set_selfpowered= usbhsg_set_selfpowered,
.udc_start = usbhsg_gadget_start,
.udc_stop = usbhsg_gadget_stop,
.pullup = usbhsg_pullup,
+ .vbus_session = usbhsg_vbus_session,
};
static int usbhsg_start(struct usbhs_priv *priv)
@@ -994,6 +1028,10 @@ int
Re: High CPU usage when video streaming on EHCI, 3.17.8 kernel with DMA enabled
* Tom Mises tommi...@gmail.com [150701 11:24]: On Mon, May 18, 2015 at 8:59 AM, Tony Lindgren t...@atomide.com wrote: For the memcpy part with DMA this patch should help too: [PATCH] dmaengine: omap-dma: Add support for memcpy The patch should help, but not out of the box as far as I understand. My understanding is that to take advantage of this, it would be necessary to explicitly call the DMA API either by replacing memcpy() in uvc_video_decode_data() or by modyfying the memcpy() implementation. Is that correct? Yes you should be able to configure the DMA to do memory-to-memory copy where both source and destination address is looping over the desired buffer size. Ideally you would move the data from camera directly to the buffer without doing a memcpy inbetween. Regards, Tony -- 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: [RESEND PATCH 2/2] ARM: at91/dt: update udc compatible strings
Le 01/07/2015 22:35, Kevin Hilman a écrit : Nicolas Ferre nicolas.fe...@atmel.com writes: From: Boris Brezillon boris.brezil...@free-electrons.com at91sam9g45, at91sam9x5 and sama5 SoCs should not use atmel,at91sam9rl-udc for their USB device compatible property since this compatible is attached to a specific hardware bug fix. Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com Acked-by: Alexandre Belloni alexandre.bell...@free-electrons.com Tested-by: Bo Shen voice.s...@atmel.com Acked-by: Nicolas Ferre nicolas.fe...@atmel.com Cc: sta...@vger.kernel.org #4.0+ --- Hi, This patch was forgotten while dealing with the series usb: atmel_usba_udc: Rework errata handling. This patch and the previous one should be added to mainline as fixes, the soonest. In fact, the errata handling is now broken because of this desynchronization. We'll try to queue it during 4.2-rc phase for inclusion in arm-soc tree. I've picked both of these up for the arm-soc/late branch which I'll try to get in before -rc1, if not it will make it for -rc2. Brilliant! Thanks a lot Kevin. Bye, -- Nicolas Ferre -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] phy: rcar-gen2 usb: Add Host/Function switching for USB0
Hi,
On Monday 22 June 2015 08:12 PM, Phil Edworthy wrote:
Instead of statically selecting the PHY connection to either the
USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
dts to specifiy gpio pins for the vbus and id signals. Additional
gpio pins are used to control power to an external OTG device and
an override to turn vbus on/off.
Note: the R-Car USB PHY only allows this Host/Function switching
on channel 0.
This has been tested on a r8a7791 based Koelsch board, which uses
a MAX3355 device to supply vbus power when needed.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
drivers/phy/phy-rcar-gen2.c | 269
1 file changed, 247 insertions(+), 22 deletions(-)
diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 97d45f4..8564e7d 100644
--- a/drivers/phy/phy-rcar-gen2.c
+++ b/drivers/phy/phy-rcar-gen2.c
@@ -1,5 +1,5 @@
/*
- * Renesas R-Car Gen2 PHY driver
+ * Renesas R-Car Gen2 USB PHY driver
*
* Copyright (C) 2014 Renesas Solutions Corp.
* Copyright (C) 2014 Cogent Embedded, Inc.
@@ -12,11 +12,16 @@
#include linux/clk.h
#include linux/delay.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/module.h
#include linux/of.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/platform_device.h
#include linux/spinlock.h
+#include linux/usb/gadget.h
+#include linux/usb/otg.h
+#include linux/workqueue.h
#include asm/cmpxchg.h
@@ -58,6 +63,18 @@ struct rcar_gen2_channel {
struct rcar_gen2_phy phys[PHYS_PER_CHANNEL];
int selected_phy;
u32 select_mask;
+
+ /* external power enable pin */
+ int gpio_pwr;
+
+ /* Host/Function switching */
+ struct delayed_work work;
+ int use_otg;
+ int gpio_vbus;
+ int gpio_id;
+ int gpio_vbus_pwr;
+ struct usb_phy *usbphy;
Using usb_phy is a step backwards IMO. We should rather try to get the missing
functionality adding in Generic PHY.
Thanks
Kishon
+ struct usb_otg *otg;
};
struct rcar_gen2_phy_driver {
@@ -68,31 +85,50 @@ struct rcar_gen2_phy_driver {
struct rcar_gen2_channel *channels;
};
-static int rcar_gen2_phy_init(struct phy *p)
+static void rcar_gen2_phy_switch(struct rcar_gen2_channel *channel,
+ u32 select_value)
{
- struct rcar_gen2_phy *phy = phy_get_drvdata(p);
- struct rcar_gen2_channel *channel = phy-channel;
struct rcar_gen2_phy_driver *drv = channel-drv;
unsigned long flags;
u32 ugctrl2;
- /*
- * Try to acquire exclusive access to PHY. The first driver calling
- * phy_init() on a given channel wins, and all attempts to use another
- * PHY on this channel will fail until phy_exit() is called by the first
- * driver. Achieving this with cmpxcgh() should be SMP-safe.
- */
- if (cmpxchg(channel-selected_phy, -1, phy-number) != -1)
- return -EBUSY;
-
- clk_prepare_enable(drv-clk);
-
spin_lock_irqsave(drv-lock, flags);
ugctrl2 = readl(drv-base + USBHS_UGCTRL2);
ugctrl2 = ~channel-select_mask;
- ugctrl2 |= phy-select_value;
+ ugctrl2 |= select_value;
writel(ugctrl2, drv-base + USBHS_UGCTRL2);
spin_unlock_irqrestore(drv-lock, flags);
+}
+
+static int rcar_gen2_phy_init(struct phy *p)
+{
+ struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+ struct rcar_gen2_channel *channel = phy-channel;
+ struct rcar_gen2_phy_driver *drv = channel-drv;
+
+ if (!channel-use_otg) {
+ /*
+ * Static Host/Function role.
+ * Try to acquire exclusive access to PHY. The first driver
+ * calling phy_init() on a given channel wins, and all attempts
+ * to use another PHY on this channel will fail until
+ * phy_exit() is called by the first driver. Achieving this
+ * with cmpxcgh() should be SMP-safe.
+ */
+ if (cmpxchg(channel-selected_phy, -1, phy-number) != -1)
+ return -EBUSY;
+
+ clk_prepare_enable(drv-clk);
+ rcar_gen2_phy_switch(channel, phy-select_value);
+ } else {
+ /*
+ * Using Host/Function switching, so schedule work to determine
+ * which role to use.
+ */
+ clk_prepare_enable(drv-clk);
+ schedule_delayed_work(channel-work, 100);
+ }
+
return 0;
}
@@ -117,9 +153,9 @@ static int rcar_gen2_phy_power_on(struct phy *p)
u32 value;
int err = 0, i;
- /* Skip if it's not USBHS */
- if (phy-select_value != USBHS_UGCTRL2_USB0SEL_HS_USB)
- return 0;
+ /* Optional external power pin */
+ if
[PATCH v2] phy: rcar-gen2 usb: Add Host/Function switching for USB0
Instead of statically selecting the PHY connection to either the
USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
dts to specifiy gpio pins for the vbus and id signals. Additional
gpio pins are used to control power to an external OTG device and
an override to turn vbus on/off.
Note: the R-Car USB PHY only allows this Host/Function switching
on channel 0.
This has been tested on a r8a7791 based Koelsch board, which uses
a MAX3355 device to supply vbus power when needed.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
Tested with patch usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
and changes to koelsch dts.
v2:
- Added -gpio to dts prop names of GPIO pins.
- Document the new bindings.
---
.../devicetree/bindings/phy/rcar-gen2-phy.txt | 10 +
drivers/phy/phy-rcar-gen2.c| 269 +++--
2 files changed, 257 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
b/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
index 00fc52a..3a501fe 100644
--- a/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
@@ -30,6 +30,16 @@ the USB channel; see the selector meanings below:
| 2 | PCI EHCI/OHCI | xHCI |
+---+---+---+
+Optional properties:
+ - renesas,pwr-gpio: A gpio specifier that will be active when the
+ PHY is powered on.
+ - renesas,id-gpio: A gpio specifier that is read to get the USB
+ ID signal.
+ - renesas,vbus-gpio: A gpio specifier that is read to get the USB
+ VBUS signal.
+ - renesas,vbus-pwr-gpio: A gpio specifier that will be active when VBUS
+ is required to be powered.
+
Example (Lager board):
usb-phy@e6590100 {
diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 97d45f4..1e7a4d9 100644
--- a/drivers/phy/phy-rcar-gen2.c
+++ b/drivers/phy/phy-rcar-gen2.c
@@ -1,5 +1,5 @@
/*
- * Renesas R-Car Gen2 PHY driver
+ * Renesas R-Car Gen2 USB PHY driver
*
* Copyright (C) 2014 Renesas Solutions Corp.
* Copyright (C) 2014 Cogent Embedded, Inc.
@@ -12,11 +12,16 @@
#include linux/clk.h
#include linux/delay.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/module.h
#include linux/of.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/platform_device.h
#include linux/spinlock.h
+#include linux/usb/gadget.h
+#include linux/usb/otg.h
+#include linux/workqueue.h
#include asm/cmpxchg.h
@@ -58,6 +63,18 @@ struct rcar_gen2_channel {
struct rcar_gen2_phy phys[PHYS_PER_CHANNEL];
int selected_phy;
u32 select_mask;
+
+ /* external power enable pin */
+ int gpio_pwr;
+
+ /* Host/Function switching */
+ struct delayed_work work;
+ int use_otg;
+ int gpio_vbus;
+ int gpio_id;
+ int gpio_vbus_pwr;
+ struct usb_phy *usbphy;
+ struct usb_otg *otg;
};
struct rcar_gen2_phy_driver {
@@ -68,31 +85,50 @@ struct rcar_gen2_phy_driver {
struct rcar_gen2_channel *channels;
};
-static int rcar_gen2_phy_init(struct phy *p)
+static void rcar_gen2_phy_switch(struct rcar_gen2_channel *channel,
+ u32 select_value)
{
- struct rcar_gen2_phy *phy = phy_get_drvdata(p);
- struct rcar_gen2_channel *channel = phy-channel;
struct rcar_gen2_phy_driver *drv = channel-drv;
unsigned long flags;
u32 ugctrl2;
- /*
-* Try to acquire exclusive access to PHY. The first driver calling
-* phy_init() on a given channel wins, and all attempts to use another
-* PHY on this channel will fail until phy_exit() is called by the first
-* driver. Achieving this with cmpxcgh() should be SMP-safe.
-*/
- if (cmpxchg(channel-selected_phy, -1, phy-number) != -1)
- return -EBUSY;
-
- clk_prepare_enable(drv-clk);
-
spin_lock_irqsave(drv-lock, flags);
ugctrl2 = readl(drv-base + USBHS_UGCTRL2);
ugctrl2 = ~channel-select_mask;
- ugctrl2 |= phy-select_value;
+ ugctrl2 |= select_value;
writel(ugctrl2, drv-base + USBHS_UGCTRL2);
spin_unlock_irqrestore(drv-lock, flags);
+}
+
+static int rcar_gen2_phy_init(struct phy *p)
+{
+ struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+ struct rcar_gen2_channel *channel = phy-channel;
+ struct rcar_gen2_phy_driver *drv = channel-drv;
+
+ if (!channel-use_otg) {
+ /*
+* Static Host/Function role.
+* Try to acquire exclusive access to PHY. The first driver
+* calling phy_init() on a
Re: About zero-length packet design for EHCI
On Wed, Jul 01, 2015 at 10:16:12AM -0400, Alan Stern wrote: On Wed, 1 Jul 2015, Peter Chen wrote: On Tue, Jun 30, 2015 at 12:03:34PM -0400, Alan Stern wrote: On Tue, 30 Jun 2015, Peter Chen wrote: Hi Alan, When reading the code (at qh_urb_transaction) about zero-length packet for EHCI, would you please help me on below questions: - I have not found the zero-length qtd prepared for control read (eg, the transfer size is multiple of wMaxPacketSize), Am I missing something? The status stage transaction for a control-IN transfer has length 0, but I guess that's not what you mean. Control-IN transfers don't have a zero-length QTD in the data stage because IN transfers _never_ have a zero-length QTD. Then, how to cover the ch 8.5.3.2 Variable-length Data Stage: 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. By reading your answers below, does it mean neither host nor device need to prepare qtd and dtd for reading zero-length packet, the hardware can handle it, and knows the data stage is over? If a control-IN transfer has a data stage that is shorter than wLength and is a multiple of the ep0 maxpacket value, then the peripheral must send a zero-length packet to indicate the end of the data stage. Thus, the UDC driver must prepare a zero-length transaction (DTD). If one is multiple? The wLength at setup packet is 64 bytes, Maximum Packet Length at dQH is 64 bytes, and the Total Bytes at dTD is 64 bytes too, does device must prepare a zero-length packet? I would like to double confirm it since the iPhone (As host after HNP) can't work well with it if I have a zero-length packet after 64 bytes packet which I describe above, if without zero-length packet, the iPhone works well. The host hardware will recognize when this happens, because the HCD will set the appropriate bits in the data-stage qTD. For example, with EHCI the HCD will set the Alternate Next qTD Pointer. Or with UHCI, the HCD will set the SPD (Short Packet Detect) bit. I see it in the code, but it is a null pointer (EHCI_LIST_END), does it mean one qTD (with valid Next qTD Pointer) can handle one 64 byte packet with or without zero-length packet well both? Any reasons why iPhone can't handle zero-length packet well? But the HCD should never prepare a zero-length qTD for an IN transfer. Zero-length packets are the responsibility of the _sender_, and for IN transfers the sender is the peripheral, not the host. Alan Stern -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] arm: koelsch: make USB0 perform Host/Function switching
Both USB Host (pci0) and Function (USBHS) drivers are enabled.
The USB PHY driver determines which IP block should be connected
based on vbus and id signals read via gpios.
Note that switch SW5 and SW6 on Koelsch board needs to be set to
position 3 for this to work.
---
Not for upstream until the following patches have been accepted:
usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
phy: rcar-gen2 usb: Add Host/Function switching for USB0
Hence, not signed off.
v2:
- Added -gpio to dts prop names of GPIO pins.
---
arch/arm/boot/dts/r8a7791-koelsch.dts | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
b/arch/arm/boot/dts/r8a7791-koelsch.dts
index cffe33f..1bb34d0 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -615,7 +615,6 @@
pci0 {
status = okay;
- pinctrl-0 = usb0_pins;
pinctrl-names = default;
};
@@ -627,13 +626,15 @@
hsusb {
status = okay;
- pinctrl-0 = usb0_pins;
pinctrl-names = default;
- renesas,enable-gpio = gpio5 31 GPIO_ACTIVE_HIGH;
};
usbphy {
status = okay;
+ renesas,pwr-gpio = gpio2 4 GPIO_ACTIVE_HIGH;
+ renesas,id-gpio = gpio5 31 GPIO_ACTIVE_HIGH;
+ renesas,vbus-gpio = gpio7 24 GPIO_ACTIVE_HIGH;
+ renesas,vbus-pwr-gpio = gpio7 23 GPIO_ACTIVE_HIGH;
};
pcie_bus_clk {
--
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
Viewing many gadgets in error under Windows with mass storage function and configfs
Hello, When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. There is no such parameter when using configfs. What would be the correct way to fix it? I thought about enumerate how many LUN where configured through configfs and then setting the exact number using fsg_common_set_nluns. There is no problem at all under Linux so maybe it is jut the way how the gadget is exposed through USB. I tested it on the kernel 3.17 and 4.1 using the musb of the SoC AM335x. Thanks for your feedback. Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.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] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
Hello.
On 7/2/2015 10:36 AM, Phil Edworthy wrote:
These changes allow a PHY driver to trigger a VBUS interrupt and
to provide the value of VBUS.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
[...]
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c
index dc2aa32..51b63f9 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
[...]
@@ -947,12 +966,27 @@ static int usbhsg_set_selfpowered(struct usb_gadget
*gadget, int is_self)
return 0;
}
+static int usbhsg_vbus_session(struct usb_gadget *gadget, int is_active)
+{
+ struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+ struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct platform_device *pdev = usbhs_priv_to_pdev(priv);
+
+ priv-vbus_is_indirect = 1;
s/1/true/.
+ priv-vbus_indirect_value = !!is_active;
+
+ renesas_usbhs_call_notify_hotplug(pdev);
+
+ return 0;
+}
+
[...]
@@ -994,6 +1028,10 @@ int usbhs_mod_gadget_probe(struct usbhs_priv *priv)
goto usbhs_mod_gadget_probe_err_gpriv;
}
+ gpriv-transceiver = usb_get_phy(USB_PHY_TYPE_UNDEFINED);
+ dev_info(dev, %s transceiver found\n,
+gpriv-transceiver ? : No);
Hm, I told you to make this message cleaner, and you haven't. :-(
[...]
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert usb: chipidea: usbmisc_imx: delete clock information
On Wed, Jul 01, 2015 at 12:06:19PM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.este...@freescale.com
This reverts commit 73dea4a912b2bfe955305de4891018f9e71e399d.
Since commit 73dea4a912b2(usb: chipidea: usbmisc_imx: delete clock
information) it is not possible to boot a kernel on a mx27pdk:
usbcore: registered new interface driver usb-storage
Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
pgd = c0004000
[f4424600] *pgd=1452(bad)
Internal error: : 8 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
Hardware name: Freescale i.MX27 (Device Tree Support)
task: c7832b60 ti: c783e000 task.ti: c783e000
PC is at usbmisc_imx27_init+0x4c/0xbc
LR is at usbmisc_imx27_init+0x40/0xbc
pc : [c03cb5c0]lr : [c03cb5b4]psr: 6093
sp : c783fe08 ip : fp :
r10: c0576434 r9 : 009c r8 : c7a773a0
r7 : 0100 r6 : 6013 r5 : c7a776f0 r4 : c7a773f0
r3 : f4424600 r2 : r1 : 0001 r0 : 0001
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: a0004000 DAC: 0017
Process swapper (pid: 1, stack limit = 0xc783e190)
Stack: (0xc783fe08 to 0xc784)
This commit also breaks the booting of imx6q-udoo board [1], so revert
it to avoid these regressions.
[1] http://www.spinics.net/lists/linux-usb/msg125597.html
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
drivers/usb/chipidea/usbmisc_imx.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 140945c..8f4cd92 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -11,6 +11,7 @@
#include linux/module.h
#include linux/of_platform.h
+#include linux/clk.h
#include linux/err.h
#include linux/io.h
#include linux/delay.h
@@ -83,6 +84,7 @@ struct usbmisc_ops {
struct imx_usbmisc {
void __iomem *base;
spinlock_t lock;
+ struct clk *clk;
const struct usbmisc_ops *ops;
};
@@ -426,6 +428,7 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
{
struct resource *res;
struct imx_usbmisc *data;
+ int ret;
struct of_device_id *tmp_dev;
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
@@ -439,6 +442,20 @@ static int usbmisc_imx_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+ data-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(data-clk)) {
+ dev_err(pdev-dev,
+ failed to get clock, err=%ld\n, PTR_ERR(data-clk));
+ return PTR_ERR(data-clk);
+ }
+
+ ret = clk_prepare_enable(data-clk);
+ if (ret) {
+ dev_err(pdev-dev,
+ clk_prepare_enable failed, err=%d\n, ret);
+ return ret;
+ }
+
tmp_dev = (struct of_device_id *)
of_match_device(usbmisc_imx_dt_ids, pdev-dev);
data-ops = (const struct usbmisc_ops *)tmp_dev-data;
@@ -449,6 +466,8 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
static int usbmisc_imx_remove(struct platform_device *pdev)
{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(pdev-dev);
+ clk_disable_unprepare(usbmisc-clk);
return 0;
}
--
1.9.1
Sorry, I can't accept it, and it will remain the clock on, and break
runtime pm feature.
In fact, the core and non-core use the same clocks, and at imx27, it
may need two or three clocks to let the controller work for imx27.
I can accept handle three clocks like below at ci_hdrc_imx.c to fix
this issue.
arch/arm/boot/dts/imx25.dtsi
clocks = clks 9, clks 70, clks 8;
clock-names = ipg, ahb, per;
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
Hi, On 02-07-15 10:45, Oliver Neukum wrote: On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. So how is this dealt with for usbip ? Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage: ignore ZTE MF 823 in mode 0x1225
On Wed, 2015-07-01 at 22:50 +0700, Lars Melin wrote: Hi, Then I would like to see an lsusb listing for 19d2:1225 with an SD-card interface, I have only seen 19d2:1225 with a single storage interface which is for the windows install cd-rom. My dongle indeed has only one interface. But one must not jump to conclusions from that. Remember that multiple SCSI devices can be connected to a single storage interface. There are almost no 3G dongles having the SD-card interface active in default (non-switched) mode. Here is a log made with an unfixed kernel: Jul 02 11:11:02 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 6 using xhci_hcd Jul 02 11:11:02 linux-dtbq.site kernel: usb 3-10: New USB device found, idVendor=19d2, idProduct=1225 Jul 02 11:11:02 linux-dtbq.site kernel: usb 3-10: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 02 11:11:02 linux-dtbq.site kernel: usb 3-10: Product: ZTE WCDMA Technologies MSM Jul 02 11:11:02 linux-dtbq.site kernel: usb 3-10: Manufacturer: ZTE,Incorporated Jul 02 11:11:02 linux-dtbq.site kernel: usb 3-10: SerialNumber: MF8230ZTED01CP261718U46EM0SHF3BW707_8C650 Jul 02 11:11:02 linux-dtbq.site kernel: usb-storage 3-10:1.0: USB Mass Storage device detected Jul 02 11:11:02 linux-dtbq.site kernel: scsi6 : usb-storage 3-10:1.0 Jul 02 11:11:02 linux-dtbq.site kernel: usbcore: registered new interface driver usb-storage Jul 02 11:11:02 linux-dtbq.site kernel: usbcore: registered new interface driver uas Jul 02 11:11:02 linux-dtbq.site mtp-probe[2234]: checking bus 3, device 6: /sys/devices/pci:00/:00:14.0/usb3/3-10 Jul 02 11:11:02 linux-dtbq.site mtp-probe[2234]: bus: 3, device: 6 was not an MTP device Jul 02 11:11:03 linux-dtbq.site kernel: scsi 6:0:0:0: CD-ROMCWID USB SCSI CD-ROM 2.31 PQ: 0 ANSI: 2 Jul 02 11:11:03 linux-dtbq.site kernel: sr2: scsi-1 drive Jul 02 11:11:03 linux-dtbq.site kernel: sr 6:0:0:0: Attached scsi CD-ROM sr2 Jul 02 11:11:03 linux-dtbq.site kernel: sr 6:0:0:0: Attached scsi generic sg5 type 5 Jul 02 11:11:03 linux-dtbq.site kernel: scsi 6:0:0:1: Direct-Access ZTE MMC Storage 2.31 PQ: 0 ANSI: 2 Jul 02 11:11:03 linux-dtbq.site kernel: sd 6:0:0:1: Attached scsi generic sg6 type 0 Jul 02 11:11:03 linux-dtbq.site kernel: sd 6:0:0:1: [sdd] Attached SCSI removable disk As you can see, it has a CD and a card reader on different LUNs. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] usb: chipidea: Reduce ULPI PHY reset pulse to datasheet spec of 1us
From: Peter Chen Sent: 30 June 2015 03:06 On Fri, Jun 26, 2015 at 03:47:03PM +0200, Mike Looijmans wrote: The datasheet for the 334x PHY mentions that a reset can be performed: ... by bringing the pin low for a minimum of 1 microsecond and then high. A delay of 5ms to implement that seems overly long, so reduce it to just 1us. As for the delay after reset, the datasheet only mentioned that the chip will assert the DIR output. 1ms seems like a safe time to wait for that to happen, so no change there. Signed-off-by: Mike Looijmans mike.looijm...@topic.nl --- drivers/usb/chipidea/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index e970863..c865abe 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -664,7 +664,7 @@ static int ci_hdrc_create_ulpi_phy(struct device *dev, struct ci_hdrc *ci) dev_err(dev, Failed to request ULPI reset gpio: %d\n, ret); return ret; } - msleep(5); + udelay(1); If the spec says 1us I'd delay for longer just to make sure. And add a comment saying that the reset needs to be 1us long. David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH] xHCI: FSE handled cleanly by incrementing event dequeue pointer
Some undefined values, mostly null are coming in event-buffer in my case for
stop,-invalid length” transfer event.
However, in the portion of the code you suggested does not increment the
dequeue pointer of transfer event ring, for trb_comp_code = COMP_STOP_INVAL.
In handle_tx_event() cleanup: does not increment the dequeue pointer of
transfer event ring.
In xhci_handle_event():
…
case TRB_TYPE(TRB_TRANSFER):
ret = handle_tx_event(xhci, event-trans_event);
if (ret 0)
xhci-error_bitmask |= 1 9;
else
update_ptrs = 0;
…
if (update_ptrs)
/* Update SW event ring dequeue pointer */
inc_deq(xhci, xhci-event_ring);
…
As update_ptrs=0, inc_deq(xhci, xhci-event_ring) won't run.
Regards,
-Saurov
--- Original Message ---
Sender : Mathias Nymanmathias.ny...@linux.intel.com
Date : Jul 01, 2015 17:35 (GMT+05:30)
Title : Re: [PATCH] xHCI: FSE handled cleanly by incrementing event dequeue
pointer
On 01.07.2015 13:13, SAUROV KANTI SHYAM wrote:
When a transfer is in progress and a STOP command is issued to xHC,
xHC will generate 2 event TRBs on event ring:
(1) Force Stopped Event TRB on successful completion of STOP cmd.
(2) Transfer Event TRB for the TD which was in progress and
stopped in the middle (with its Completion Code set to Stopped).
Since the value of event-buffer is undefined and should be ignored,
at this point the code should return after incrementing the dequeue
pointer of event ring.
The first event should be a stop, invalid length transfer event in case we
stop
in between TD's.
The event- buffer should be valid and contain the current dequeue pointer
value
of the ring, see xhci 1.0 section 4.6.9.
It might still be part of the previous TD, but this is is already taken care of
in handle_tx_event():
/*
* Skip the Force Stopped Event. The event_trb(event_dma) of FSE
* is not in the current TD pointed by ep_ring-dequeue because
* that the hardware dequeue pointer still at the previous TRB
* of the current TD. The previous TRB maybe a Link TD or the
* last TRB of the previous TD. The command completion handle
* will take care the rest.
*/
if (!event_seg (trb_comp_code == COMP_STOP ||
trb_comp_code == COMP_STOP_INVAL)) {
ret = 0;
goto cleanup;
}
The stop,-invalid length transfer event is followed by a command completion
event, which
we handle by calling xhci_handle_cmd_stop_ep(), so no point in calling it from
handle_tx_event()
Does this patch solve any issue in your case? there might be something else
going on.
If you have some custom solution using event data TRBs on the transfer ring
then it is
possible that it stops on an event data TRB, and the event-buffer is not a
pointer to the ring.
In that case the ED (Event Data) bit in the transfer event should first be
checked.
-Mathias
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. Sorry Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
Hi, On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: Hello, When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. There is no such parameter when using configfs. What would be the correct way to fix it? Making sure you don't lie to the other side of the cable :-) I thought about enumerate how many LUN where configured through configfs and then setting the exact number using fsg_common_set_nluns. yeah, that should be the way. There is no problem at all under Linux so maybe it is jut the way how the gadget is exposed through USB. no, we shouldn't really lie, this really needs fixing. I tested it on the kernel 3.17 and 4.1 using the musb of the SoC AM335x. Thanks for your feedback. Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- balbi signature.asc Description: Digital signature
RE: [PATCH v2] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
Hi Laurent,
On 02 July 2015 09:15, Laurent wrote:
Hi Phil,
(CC'ing Morimoto-san)
Thank you for the patch.
On Thursday 02 July 2015 08:36:42 Phil Edworthy wrote:
These changes allow a PHY driver to trigger a VBUS interrupt and
to provide the value of VBUS.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v2:
- vbus variables changed from int to bool.
- dev_info() changed to dev_err()
---
drivers/usb/renesas_usbhs/common.h | 2 ++
drivers/usb/renesas_usbhs/mod.c| 3 +++
drivers/usb/renesas_usbhs/mod_gadget.c | 38
+++
3 files changed, 43 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/common.h
b/drivers/usb/renesas_usbhs/common.h index 8c5fc12..478700c 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -255,6 +255,8 @@ struct usbhs_priv {
struct renesas_usbhs_driver_param dparam;
struct delayed_work notify_hotplug_work;
+ bool vbus_is_indirect;
+ bool vbus_indirect_value;
struct platform_device *pdev;
struct extcon_dev *edev;
diff --git a/drivers/usb/renesas_usbhs/mod.c
b/drivers/usb/renesas_usbhs/mod.c index 9a705b1..8a2d3dd 100644
--- a/drivers/usb/renesas_usbhs/mod.c
+++ b/drivers/usb/renesas_usbhs/mod.c
@@ -42,6 +42,9 @@ static int usbhsm_autonomy_get_vbus(struct
platform_device
*pdev) {
struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+ if (priv-vbus_is_indirect)
+ return priv-vbus_indirect_value;
+
Something is bothering me. The comment block right before this function states
that these functions are used if platform doesn't have external phy.
However, the mechanism you implement here is aimed at letting a PHY control
vbus. I have a feeling this isn't the right mechanism.
I took the comment as though it was talking about an physically external PHY,
but
I suppose it makes no difference from the point of view of this driver. I'll
have a
look to see if there is a better way to hook this in.
return VBSTS usbhs_read(priv, INTSTS0);
}
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c index dc2aa32..51b63f9 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -21,6 +21,7 @@
#include linux/platform_device.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include linux/usb/otg.h
#include common.h
/*
@@ -50,6 +51,7 @@ struct usbhsg_gpriv {
int uep_size;
struct usb_gadget_driver*driver;
+ struct usb_phy *transceiver;
u32 status;
#define USBHSG_STATUS_STARTED (1 0)
@@ -882,6 +884,8 @@ static int usbhsg_gadget_start(struct usb_gadget
*gadget, {
struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct device *dev = usbhs_priv_to_dev(priv);
+ int ret;
if (!driver ||
!driver-setup ||
@@ -891,6 +895,17 @@ static int usbhsg_gadget_start(struct usb_gadget
*gadget, /* first hook up the driver ... */
gpriv-driver = driver;
+ /* connect to bus through transceiver */
+ if (!IS_ERR_OR_NULL(gpriv-transceiver)) {
+ ret = otg_set_peripheral(gpriv-transceiver-otg,
+ gpriv-gadget);
+ if (ret) {
+ dev_err(dev, %s: can't bind to transceiver\n,
+ gpriv-gadget.name);
Shouldn't gpriv-driver be set to NULL here ? Or possibly better, this code
block moved before setting gpriv-driver ?
Makes sense.
+ return ret;
+ }
+ }
+
return usbhsg_try_start(priv, USBHSG_STATUS_REGISTERD);
}
@@ -900,6 +915,10 @@ static int usbhsg_gadget_stop(struct usb_gadget
*gadget) struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
usbhsg_try_stop(priv, USBHSG_STATUS_REGISTERD);
+
+ if (!IS_ERR_OR_NULL(gpriv-transceiver))
+ (void) otg_set_peripheral(gpriv-transceiver-otg, NULL);
Is there really a need for (void) ?
Oops, none at all.
+
gpriv-driver = NULL;
return 0;
@@ -947,12 +966,27 @@ static int usbhsg_set_selfpowered(struct usb_gadget
*gadget, int is_self) return 0;
}
+static int usbhsg_vbus_session(struct usb_gadget *gadget, int is_active)
+{
+ struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+ struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct platform_device *pdev = usbhs_priv_to_pdev(priv);
+
+ priv-vbus_is_indirect = 1;
+ priv-vbus_indirect_value = !!is_active;
+
+ renesas_usbhs_call_notify_hotplug(pdev);
+
+ return 0;
+}
+
static const struct usb_gadget_ops usbhsg_gadget_ops = {
.get_frame = usbhsg_get_frame,
.set_selfpowered
[PATCH v3] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
These changes allow a PHY driver to trigger a VBUS interrupt and
to provide the value of VBUS.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
v3:
- Changed how indirect vbus is plumbed in.
- Removed unnecessary (void) on call to otg_set_peripheral.
- Moved code that connects to bus through transceiver so it
is before setting gpriv-driver.
v2:
- vbus variables changed from int to bool.
- dev_info() changed to dev_err()
---
drivers/usb/renesas_usbhs/common.c | 8 +--
drivers/usb/renesas_usbhs/common.h | 2 ++
drivers/usb/renesas_usbhs/mod_gadget.c | 38 ++
3 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/renesas_usbhs/common.c
b/drivers/usb/renesas_usbhs/common.c
index 0f7e850..15e67d8 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -379,7 +379,10 @@ static void usbhsc_hotplug(struct usbhs_priv *priv)
/*
* get vbus status from platform
*/
- enable = usbhs_platform_call(priv, get_vbus, pdev);
+ if (priv-vbus_is_indirect)
+ enable = priv-vbus_indirect_value;
+ else
+ enable = usbhs_platform_call(priv, get_vbus, pdev);
/*
* get id from platform
@@ -662,7 +665,8 @@ static int usbhs_probe(struct platform_device *pdev)
pm_runtime_enable(pdev-dev);
if (!usbhsc_flags_has(priv, USBHSF_RUNTIME_PWCTRL)) {
usbhsc_power_ctrl(priv, 1);
- usbhs_mod_autonomy_mode(priv);
+ if (!priv-vbus_is_indirect)
+ usbhs_mod_autonomy_mode(priv);
}
/*
diff --git a/drivers/usb/renesas_usbhs/common.h
b/drivers/usb/renesas_usbhs/common.h
index 8c5fc12..478700c 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -255,6 +255,8 @@ struct usbhs_priv {
struct renesas_usbhs_driver_param dparam;
struct delayed_work notify_hotplug_work;
+ bool vbus_is_indirect;
+ bool vbus_indirect_value;
struct platform_device *pdev;
struct extcon_dev *edev;
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c
index dc2aa32..93ad4ea 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -21,6 +21,7 @@
#include linux/platform_device.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include linux/usb/otg.h
#include common.h
/*
@@ -50,6 +51,7 @@ struct usbhsg_gpriv {
int uep_size;
struct usb_gadget_driver*driver;
+ struct usb_phy *transceiver;
u32 status;
#define USBHSG_STATUS_STARTED (1 0)
@@ -882,12 +884,25 @@ static int usbhsg_gadget_start(struct usb_gadget *gadget,
{
struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct device *dev = usbhs_priv_to_dev(priv);
+ int ret;
if (!driver ||
!driver-setup ||
driver-max_speed USB_SPEED_FULL)
return -EINVAL;
+ /* connect to bus through transceiver */
+ if (!IS_ERR_OR_NULL(gpriv-transceiver)) {
+ ret = otg_set_peripheral(gpriv-transceiver-otg,
+ gpriv-gadget);
+ if (ret) {
+ dev_err(dev, %s: can't bind to transceiver\n,
+ gpriv-gadget.name);
+ return ret;
+ }
+ }
+
/* first hook up the driver ... */
gpriv-driver = driver;
@@ -900,6 +915,10 @@ static int usbhsg_gadget_stop(struct usb_gadget *gadget)
struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
usbhsg_try_stop(priv, USBHSG_STATUS_REGISTERD);
+
+ if (!IS_ERR_OR_NULL(gpriv-transceiver))
+ otg_set_peripheral(gpriv-transceiver-otg, NULL);
+
gpriv-driver = NULL;
return 0;
@@ -947,12 +966,27 @@ static int usbhsg_set_selfpowered(struct usb_gadget
*gadget, int is_self)
return 0;
}
+static int usbhsg_vbus_session(struct usb_gadget *gadget, int is_active)
+{
+ struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+ struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct platform_device *pdev = usbhs_priv_to_pdev(priv);
+
+ priv-vbus_is_indirect = 1;
+ priv-vbus_indirect_value = !!is_active;
+
+ renesas_usbhs_call_notify_hotplug(pdev);
+
+ return 0;
+}
+
static const struct usb_gadget_ops usbhsg_gadget_ops = {
.get_frame = usbhsg_get_frame,
.set_selfpowered= usbhsg_set_selfpowered,
.udc_start = usbhsg_gadget_start,
.udc_stop = usbhsg_gadget_stop,
.pullup = usbhsg_pullup,
+
Re: [PATCH v2] arm: koelsch: make USB0 perform Host/Function switching
Hello.
On 7/2/2015 11:14 AM, Phil Edworthy wrote:
Both USB Host (pci0) and Function (USBHS) drivers are enabled.
The USB PHY driver determines which IP block should be connected
based on vbus and id signals read via gpios.
Note that switch SW5 and SW6 on Koelsch board needs to be set to
position 3 for this to work.
[...]
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
b/arch/arm/boot/dts/r8a7791-koelsch.dts
index cffe33f..1bb34d0 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -615,7 +615,6 @@
pci0 {
status = okay;
- pinctrl-0 = usb0_pins;
pinctrl-names = default;
As you're removing pinctrl-0 prop, you also should remove pinctrl-names.
};
@@ -627,13 +626,15 @@
hsusb {
status = okay;
- pinctrl-0 = usb0_pins;
Likewise.
[...]
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3] phy: rcar-gen2 usb: Add Host/Function switching for USB0
Hi Kishon,
On 02 July 2015 09:22, Kishon wrote:
Hi,
On Monday 22 June 2015 08:12 PM, Phil Edworthy wrote:
Instead of statically selecting the PHY connection to either the
USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
dts to specifiy gpio pins for the vbus and id signals. Additional
gpio pins are used to control power to an external OTG device and
an override to turn vbus on/off.
Note: the R-Car USB PHY only allows this Host/Function switching
on channel 0.
This has been tested on a r8a7791 based Koelsch board, which uses
a MAX3355 device to supply vbus power when needed.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
drivers/phy/phy-rcar-gen2.c | 269
1 file changed, 247 insertions(+), 22 deletions(-)
diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 97d45f4..8564e7d 100644
--- a/drivers/phy/phy-rcar-gen2.c
+++ b/drivers/phy/phy-rcar-gen2.c
@@ -1,5 +1,5 @@
/*
- * Renesas R-Car Gen2 PHY driver
+ * Renesas R-Car Gen2 USB PHY driver
*
* Copyright (C) 2014 Renesas Solutions Corp.
* Copyright (C) 2014 Cogent Embedded, Inc.
@@ -12,11 +12,16 @@
#include linux/clk.h
#include linux/delay.h
#include linux/io.h
+#include linux/interrupt.h
#include linux/module.h
#include linux/of.h
+#include linux/of_gpio.h
#include linux/phy/phy.h
#include linux/platform_device.h
#include linux/spinlock.h
+#include linux/usb/gadget.h
+#include linux/usb/otg.h
+#include linux/workqueue.h
#include asm/cmpxchg.h
@@ -58,6 +63,18 @@ struct rcar_gen2_channel {
struct rcar_gen2_phy phys[PHYS_PER_CHANNEL];
int selected_phy;
u32 select_mask;
+
+ /* external power enable pin */
+ int gpio_pwr;
+
+ /* Host/Function switching */
+ struct delayed_work work;
+ int use_otg;
+ int gpio_vbus;
+ int gpio_id;
+ int gpio_vbus_pwr;
+ struct usb_phy *usbphy;
Using usb_phy is a step backwards IMO. We should rather try to get the missing
functionality adding in Generic PHY.
Ok, that will take quite a bit more work. Do you know if anyone is working on
this?
Thanks
Kishon
+ struct usb_otg *otg;
};
struct rcar_gen2_phy_driver {
@@ -68,31 +85,50 @@ struct rcar_gen2_phy_driver {
struct rcar_gen2_channel *channels;
};
-static int rcar_gen2_phy_init(struct phy *p)
+static void rcar_gen2_phy_switch(struct rcar_gen2_channel *channel,
+ u32 select_value)
{
- struct rcar_gen2_phy *phy = phy_get_drvdata(p);
- struct rcar_gen2_channel *channel = phy-channel;
struct rcar_gen2_phy_driver *drv = channel-drv;
unsigned long flags;
u32 ugctrl2;
- /*
-* Try to acquire exclusive access to PHY. The first driver calling
-* phy_init() on a given channel wins, and all attempts to use another
-* PHY on this channel will fail until phy_exit() is called by the first
-* driver. Achieving this with cmpxcgh() should be SMP-safe.
-*/
- if (cmpxchg(channel-selected_phy, -1, phy-number) != -1)
- return -EBUSY;
-
- clk_prepare_enable(drv-clk);
-
spin_lock_irqsave(drv-lock, flags);
ugctrl2 = readl(drv-base + USBHS_UGCTRL2);
ugctrl2 = ~channel-select_mask;
- ugctrl2 |= phy-select_value;
+ ugctrl2 |= select_value;
writel(ugctrl2, drv-base + USBHS_UGCTRL2);
spin_unlock_irqrestore(drv-lock, flags);
+}
+
+static int rcar_gen2_phy_init(struct phy *p)
+{
+ struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+ struct rcar_gen2_channel *channel = phy-channel;
+ struct rcar_gen2_phy_driver *drv = channel-drv;
+
+ if (!channel-use_otg) {
+ /*
+* Static Host/Function role.
+* Try to acquire exclusive access to PHY. The first driver
+* calling phy_init() on a given channel wins, and all attempts
+* to use another PHY on this channel will fail until
+* phy_exit() is called by the first driver. Achieving this
+* with cmpxcgh() should be SMP-safe.
+*/
+ if (cmpxchg(channel-selected_phy, -1, phy-number) != -1)
+ return -EBUSY;
+
+ clk_prepare_enable(drv-clk);
+ rcar_gen2_phy_switch(channel, phy-select_value);
+ } else {
+ /*
+* Using Host/Function switching, so schedule work to determine
+* which role to use.
+*/
+ clk_prepare_enable(drv-clk);
+ schedule_delayed_work(channel-work, 100);
+ }
+
return 0;
}
@@ -117,9 +153,9 @@ static int rcar_gen2_phy_power_on(struct phy *p)
u32 value;
int err = 0, i;
- /* Skip if it's not USBHS */
- if
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
On Thu, Jul 02, 2015 at 02:55:34PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 02:45 PM, Michal Nazarewicz wrote: On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. On Thu, Jul 02 2015, Felipe Balbi wrote: This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. I’ve came up with [1], which you should feel free to test, but then Krzysztof came along with [2], which among other things addressed the LUN count issue, and I kind of stopped working on the issue waiting for his follow up. Sorry that it took so much time. I have been quite busy with some other things. I will send fixed version of that series in a few hours. I'm taking Michal's patch as a quick fix for the -rc though. That patch is simple enough to get in and solves the issue at hand. -- balbi signature.asc Description: Digital signature
Re: [PATCH] xHCI: FSE handled cleanly by incrementing event dequeue pointer
Hi
On 02.07.2015 13:45, SAUROV KANTI SHYAM wrote:
Some undefined values, mostly null are coming in event-buffer in my case for
stop,-invalid length” transfer event.
That doesn't sound good, what xhci vendor and version are you using?
Have you seen/tried this on other xhci hw as well?
What kernel version are you using? are there any custom xhci driver
modification in it?
However, in the portion of the code you suggested does not increment the
dequeue pointer of transfer event ring, for trb_comp_code = COMP_STOP_INVAL.
In handle_tx_event() cleanup: does not increment the dequeue pointer of
transfer event ring.
In xhci_handle_event():
…
case TRB_TYPE(TRB_TRANSFER):
ret = handle_tx_event(xhci, event-trans_event);
if (ret 0)
xhci-error_bitmask |= 1 9;
else
update_ptrs = 0;
…
if (update_ptrs)
/* Update SW event ring dequeue pointer */
inc_deq(xhci, xhci-event_ring);
…
As update_ptrs=0, inc_deq(xhci, xhci-event_ring) won't run.
Regards,
Event ring dequeue is increased either in handle_tx_event(), cleanup:
if (trb_comp_code == COMP_MISSED_INT || !ep-skip) {
inc_deq(xhci, xhci-event_ring);
Or then in xhci_handle_event() if handle_tx_event() returns error.
For example in handle_tx_event() if event-buffer is null, then
ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event-buffer)); is NULL
and the check immediately after would return with -ENODEV
-Mathias
P.S. The potion of code was not a suggestion, it was copypasted from the
upstream xhci driver, and has
looked like that since 3.17 kernel
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage: ignore ZTE MF 823 in mode 0x1225
On Wed, 2015-07-01 at 22:50 +0700, Lars Melin wrote: Then I would like to see an lsusb listing for 19d2:1225 with an SD-card interface, I have only seen 19d2:1225 with a single storage interface which is for the windows install cd-rom. There are almost no 3G dongles having the SD-card interface active in default (non-switched) mode. Though I see your point. Is the switching desirable at all for this device or do I need to block the device on the SCSI level? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
On 07/02/2015 02:45 PM, Michal Nazarewicz wrote: On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. On Thu, Jul 02 2015, Felipe Balbi wrote: This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. I’ve came up with [1], which you should feel free to test, but then Krzysztof came along with [2], which among other things addressed the LUN count issue, and I kind of stopped working on the issue waiting for his follow up. Sorry that it took so much time. I have been quite busy with some other things. I will send fixed version of that series in a few hours. -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage: ignore ZTE MF 823 in mode 0x1225
On Thu, 2015-07-02 at 20:33 +0700, Lars Melin wrote: On 2015-07-02 20:01, Oliver Neukum wrote: On Thu, 2015-07-02 at 14:43 +0200, Oliver Neukum wrote: On Wed, 2015-07-01 at 22:50 +0700, Lars Melin wrote: Then I would like to see an lsusb listing for 19d2:1225 with an SD-card interface, I have only seen 19d2:1225 with a single storage interface which is for the windows install cd-rom. There are almost no 3G dongles having the SD-card interface active in default (non-switched) mode. Though I see your point. Is the switching desirable at all for this device or do I need to block the device on the SCSI level? Regards Oliver However, when I reinstall usb_modeswitch I see that the storage functionality is unnecessary to switch this device. Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 8 using xhci_hcd Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device found, idVendor=19d2, idProduct=1225 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Product: ZTE WCDMA Technologies MSM Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Manufacturer: ZTE,Incorporated Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: SerialNumber: MF8230ZTED01CP261718U46EM0SHF3BW707_8C650 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: USB Mass Storage device detected Jul 02 14:56:10 linux-dtbq.site kernel: Vendor: 0x19d2, Product: 0x1225, Revision: 0xf0f1 Jul 02 14:56:10 linux-dtbq.site kernel: Interface Subclass: 0x06, Protocol: 0x50 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: device ignored Jul 02 14:56:10 linux-dtbq.site kernel: storage_probe() failed Jul 02 14:56:10 linux-dtbq.site kernel: -- sending exit command to thread Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: checking bus 3, device 8: /sys/devices/pci:00/:00:14.0/usb3/3-10 Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: bus: 3, device: 8 was not an MTP device Jul 02 14:56:10 linux-dtbq.site usb_modeswitch[2740]: switch device 19d2:1225 on 003/008 Jul 02 14:56:15 linux-dtbq.site kernel: usb 3-10: USB disconnect, device number 8 Jul 02 14:56:16 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 9 using xhci_hcd Regards Oliver Yes but there are other 19d2:1225 devices which don't have dual-mode So they reused the ID. Oh just great. (19d2:1403 RNDIS or 19d2:1405 ECM) they do only have one of the modes and they don't auto-flip so they need usb_modeswitch. Understood. And I just found out that the auto flip is unreliable without storage. But do they need storage interfaces to switch to the other modes? As the virtual CD does not go away I don't really see much value in retaining the original (storage only) mode, as all it offers is an opportunity to lose data. This is the first 3G dongle I have seen with multiple LUN on the same USB interface, there has always been one interface for each storage function in the past. So yes I jumped to conclusion.. Anyway, it would be good if you could block the MMC interface on LUN level while still having the virtual cd-rom working. This device is total crap. Do you know about alternative commands that allow selecting the mode the device switches to? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
Hi Sergei,
On 02 July 2015 12:17, Sergei wrote:
To: Phil Edworthy; Yoshihiro Shimoda
Hello.
On 7/2/2015 10:36 AM, Phil Edworthy wrote:
These changes allow a PHY driver to trigger a VBUS interrupt and
to provide the value of VBUS.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
[...]
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c
b/drivers/usb/renesas_usbhs/mod_gadget.c
index dc2aa32..51b63f9 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
[...]
@@ -947,12 +966,27 @@ static int usbhsg_set_selfpowered(struct usb_gadget
*gadget, int is_self)
return 0;
}
+static int usbhsg_vbus_session(struct usb_gadget *gadget, int is_active)
+{
+ struct usbhsg_gpriv *gpriv = usbhsg_gadget_to_gpriv(gadget);
+ struct usbhs_priv *priv = usbhsg_gpriv_to_priv(gpriv);
+ struct platform_device *pdev = usbhs_priv_to_pdev(priv);
+
+ priv-vbus_is_indirect = 1;
s/1/true/.
Ok.
+ priv-vbus_indirect_value = !!is_active;
+
+ renesas_usbhs_call_notify_hotplug(pdev);
+
+ return 0;
+}
+
[...]
@@ -994,6 +1028,10 @@ int usbhs_mod_gadget_probe(struct usbhs_priv
*priv)
goto usbhs_mod_gadget_probe_err_gpriv;
}
+ gpriv-transceiver = usb_get_phy(USB_PHY_TYPE_UNDEFINED);
+ dev_info(dev, %s transceiver found\n,
+gpriv-transceiver ? : No);
Hm, I told you to make this message cleaner, and you haven't. :-(
Ah, sorry, I missed that comment.
[...]
WBR, Sergei
Thanks
Phil
--
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] arm: koelsch: make USB0 perform Host/Function switching
Hi Sergei.
On 02 July 2015 12:32, Sergei wrote:
Hello.
On 7/2/2015 11:14 AM, Phil Edworthy wrote:
Both USB Host (pci0) and Function (USBHS) drivers are enabled.
The USB PHY driver determines which IP block should be connected
based on vbus and id signals read via gpios.
Note that switch SW5 and SW6 on Koelsch board needs to be set to
position 3 for this to work.
[...]
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts
b/arch/arm/boot/dts/r8a7791-koelsch.dts
index cffe33f..1bb34d0 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -615,7 +615,6 @@
pci0 {
status = okay;
- pinctrl-0 = usb0_pins;
pinctrl-names = default;
As you're removing pinctrl-0 prop, you also should remove
pinctrl-names.
Ok.
};
@@ -627,13 +626,15 @@
hsusb {
status = okay;
- pinctrl-0 = usb0_pins;
Likewise.
Sure.
[...]
WBR, Sergei
Thanks
Phil
--
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: Viewing many gadgets in error under Windows with mass storage function and configfs
Hi, On Thu, Jul 02, 2015 at 01:58:44PM +0200, Gregory CLEMENT wrote: Hi Felipe, thanks for you prompt feedback. On 02/07/2015 13:45, Felipe Balbi wrote: Hi, On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: Hello, When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. Oh sorry to not have finding the thread about it. There is no such parameter when using configfs. What would be the correct way to fix it? Making sure you don't lie to the other side of the cable :-) I thought about enumerate how many LUN where configured through configfs and then setting the exact number using fsg_common_set_nluns. yeah, that should be the way. There is no problem at all under Linux so maybe it is jut the way how the gadget is exposed through USB. no, we shouldn't really lie, this really needs fixing. Ok so don't hesitate to ask me to test or if you need any help. sure thing, here's the thread if you want to chip in: http://www.spinics.net/lists/linux-usb/msg126284.html -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb-storage: ignore ZTE MF 823 in mode 0x1225
On 2015-07-02 20:01, Oliver Neukum wrote: On Thu, 2015-07-02 at 14:43 +0200, Oliver Neukum wrote: On Wed, 2015-07-01 at 22:50 +0700, Lars Melin wrote: Then I would like to see an lsusb listing for 19d2:1225 with an SD-card interface, I have only seen 19d2:1225 with a single storage interface which is for the windows install cd-rom. There are almost no 3G dongles having the SD-card interface active in default (non-switched) mode. Though I see your point. Is the switching desirable at all for this device or do I need to block the device on the SCSI level? Regards Oliver However, when I reinstall usb_modeswitch I see that the storage functionality is unnecessary to switch this device. Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 8 using xhci_hcd Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device found, idVendor=19d2, idProduct=1225 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Product: ZTE WCDMA Technologies MSM Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Manufacturer: ZTE,Incorporated Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: SerialNumber: MF8230ZTED01CP261718U46EM0SHF3BW707_8C650 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: USB Mass Storage device detected Jul 02 14:56:10 linux-dtbq.site kernel: Vendor: 0x19d2, Product: 0x1225, Revision: 0xf0f1 Jul 02 14:56:10 linux-dtbq.site kernel: Interface Subclass: 0x06, Protocol: 0x50 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: device ignored Jul 02 14:56:10 linux-dtbq.site kernel: storage_probe() failed Jul 02 14:56:10 linux-dtbq.site kernel: -- sending exit command to thread Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: checking bus 3, device 8: /sys/devices/pci:00/:00:14.0/usb3/3-10 Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: bus: 3, device: 8 was not an MTP device Jul 02 14:56:10 linux-dtbq.site usb_modeswitch[2740]: switch device 19d2:1225 on 003/008 Jul 02 14:56:15 linux-dtbq.site kernel: usb 3-10: USB disconnect, device number 8 Jul 02 14:56:16 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 9 using xhci_hcd Regards Oliver Yes but there are other 19d2:1225 devices which don't have dual-mode (19d2:1403 RNDIS or 19d2:1405 ECM) they do only have one of the modes and they don't auto-flip so they need usb_modeswitch. This is the first 3G dongle I have seen with multiple LUN on the same USB interface, there has always been one interface for each storage function in the past. So yes I jumped to conclusion.. Anyway, it would be good if you could block the MMC interface on LUN level while still having the virtual cd-rom working. cheers /Lars -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert usb: chipidea: usbmisc_imx: delete clock information
On 02.07.2015 10:40, Peter Chen wrote:
On Wed, Jul 01, 2015 at 12:06:19PM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.este...@freescale.com
This reverts commit 73dea4a912b2bfe955305de4891018f9e71e399d.
Since commit 73dea4a912b2(usb: chipidea: usbmisc_imx: delete clock
information) it is not possible to boot a kernel on a mx27pdk:
usbcore: registered new interface driver usb-storage
Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
pgd = c0004000
[f4424600] *pgd=1452(bad)
Internal error: : 8 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
Hardware name: Freescale i.MX27 (Device Tree Support)
task: c7832b60 ti: c783e000 task.ti: c783e000
PC is at usbmisc_imx27_init+0x4c/0xbc
LR is at usbmisc_imx27_init+0x40/0xbc
pc : [c03cb5c0]lr : [c03cb5b4]psr: 6093
sp : c783fe08 ip : fp :
r10: c0576434 r9 : 009c r8 : c7a773a0
r7 : 0100 r6 : 6013 r5 : c7a776f0 r4 : c7a773f0
r3 : f4424600 r2 : r1 : 0001 r0 : 0001
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: a0004000 DAC: 0017
Process swapper (pid: 1, stack limit = 0xc783e190)
Stack: (0xc783fe08 to 0xc784)
This commit also breaks the booting of imx6q-udoo board [1], so revert
it to avoid these regressions.
[1] http://www.spinics.net/lists/linux-usb/msg125597.html
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
drivers/usb/chipidea/usbmisc_imx.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 140945c..8f4cd92 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -11,6 +11,7 @@
#include linux/module.h
#include linux/of_platform.h
+#include linux/clk.h
#include linux/err.h
#include linux/io.h
#include linux/delay.h
@@ -83,6 +84,7 @@ struct usbmisc_ops {
struct imx_usbmisc {
void __iomem *base;
spinlock_t lock;
+struct clk *clk;
const struct usbmisc_ops *ops;
};
@@ -426,6 +428,7 @@ static int usbmisc_imx_probe(struct platform_device
*pdev)
{
struct resource *res;
struct imx_usbmisc *data;
+int ret;
struct of_device_id *tmp_dev;
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
@@ -439,6 +442,20 @@ static int usbmisc_imx_probe(struct platform_device
*pdev)
if (IS_ERR(data-base))
return PTR_ERR(data-base);
+data-clk = devm_clk_get(pdev-dev, NULL);
+if (IS_ERR(data-clk)) {
+dev_err(pdev-dev,
+failed to get clock, err=%ld\n, PTR_ERR(data-clk));
+return PTR_ERR(data-clk);
+}
+
+ret = clk_prepare_enable(data-clk);
+if (ret) {
+dev_err(pdev-dev,
+clk_prepare_enable failed, err=%d\n, ret);
+return ret;
+}
+
tmp_dev = (struct of_device_id *)
of_match_device(usbmisc_imx_dt_ids, pdev-dev);
data-ops = (const struct usbmisc_ops *)tmp_dev-data;
@@ -449,6 +466,8 @@ static int usbmisc_imx_probe(struct platform_device
*pdev)
static int usbmisc_imx_remove(struct platform_device *pdev)
{
+struct imx_usbmisc *usbmisc = dev_get_drvdata(pdev-dev);
+clk_disable_unprepare(usbmisc-clk);
return 0;
}
--
1.9.1
Sorry, I can't accept it, and it will remain the clock on, and break
runtime pm feature.
In fact, the core and non-core use the same clocks, and at imx27, it
may need two or three clocks to let the controller work for imx27.
I can accept handle three clocks like below at ci_hdrc_imx.c to fix
this issue.
arch/arm/boot/dts/imx25.dtsi
clocks = clks 9, clks 70, clks 8;
clock-names = ipg, ahb, per;
This would also work for UDOO board (one clock then can be used for hub),
but there might be a problem if such clock would be disabled by
runtime PM.
The hub chip (USB2514) datasheet is silent whether the clock could
be stopped, for example during suspend, so one has to assume that
it shouldn't be done.
Best regards,
Maciej Szmigiero
--
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: Viewing many gadgets in error under Windows with mass storage function and configfs
On 07/02/2015 03:14 PM, Felipe Balbi wrote: On Thu, Jul 02, 2015 at 02:55:34PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 02:45 PM, Michal Nazarewicz wrote: On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. On Thu, Jul 02 2015, Felipe Balbi wrote: This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. I’ve came up with [1], which you should feel free to test, but then Krzysztof came along with [2], which among other things addressed the LUN count issue, and I kind of stopped working on the issue waiting for his follow up. Sorry that it took so much time. I have been quite busy with some other things. I will send fixed version of that series in a few hours. I'm taking Michal's patch as a quick fix for the -rc though. That patch is simple enough to get in and solves the issue at hand. Nope it doesn't. It may read past buffer in case of legacy gadgets please see discussion in [1] 1 - http://marc.info/?l=linux-usbm=143498343720763w=2 BR's -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote:
When I use configs to configure the mass storage function for the
gadget, and when the device is plugged under Windows, then the
partition that I expose is well managed, but I also see 7 other gadget
in the device manager with error.
This seven bogus gadget seems to be the 7 other LUN that are not
used. Indeed if I apply this dirty patch:
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..2b4ae98 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3511,7 +3511,8 @@ static struct usb_function_instance
*fsg_alloc_inst(void)
rc = PTR_ERR(opts-common);
goto release_opts;
}
- rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS);
+// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS);
+ rc = fsg_common_set_nluns(opts-common, 1);
if (rc)
goto release_opts;
Then there is no more gadget error under Windows. As the value of
FSG_MAX_LUNS is 8 and in my configuration I only use one partition,
then it makes sens that I see 7 bogus gadgets.
I also saw that in the legacy driver, it was possible to modify the
number of LUN using the module parameter file_count.
On Thu, Jul 02 2015, Felipe Balbi wrote:
This has been reported. Michal was working on a fix, but the patch
hasn't been applied yet.
I’ve came up with [1], which you should feel free to test, but then
Krzysztof came along with [2], which among other things addressed the
LUN count issue, and I kind of stopped working on the issue waiting for
his follow up.
Since merge window is about to close, perhaps the solution is to get [1]
merged quickly so it shows up in 4.2 and then get approaches described
in [3] for the future.
[1]
http://news.gmane.org/find-root.php?message_id=xa1toak7j1rl.fsf%40mina86.com,
patch also attached at the end of the message
[2]
http://news.gmane.org/find-root.php?message_id=1434979163%2d5942%2d1%2dgit%2dsend%2demail%2dk.opasiak%40samsung.com
[3]
http://news.gmane.org/find-root.php?message_id=xa1t7fqvizh0.fsf%40mina86.com
- 8 ---
From 59b6ec98aa9c638f7d140ad0b6b5a30bb2ba4145 Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz min...@mina86.com
Date: Fri, 19 Jun 2015 23:56:34 +0200
Subject: [PATCH] usb: f_mass_storage: limit number of reported LUNs
Mass storage function created via configfs always reports eight LUNs
to the hosts even if only one LUN has been configured. Adjust the
number when the USB function is allocated based on LUNs that user
has created.
Signed-off-by: Michal Nazarewicz min...@mina86.com
Cc: sta...@vger.kernel.org
---
drivers/usb/gadget/function/f_mass_storage.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..e1beb14 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2796,8 +2796,6 @@ int fsg_common_set_nluns(struct fsg_common *common, int
nluns)
common-luns = curlun;
common-nluns = nluns;
- pr_info(Number of LUNs=%d\n, common-nluns);
-
return 0;
}
EXPORT_SYMBOL_GPL(fsg_common_set_nluns);
@@ -3563,14 +3561,26 @@ static struct usb_function *fsg_alloc(struct
usb_function_instance *fi)
struct fsg_opts *opts = fsg_opts_from_func_inst(fi);
struct fsg_common *common = opts-common;
struct fsg_dev *fsg;
+ unsigned nluns, i;
fsg = kzalloc(sizeof(*fsg), GFP_KERNEL);
if (unlikely(!fsg))
return ERR_PTR(-ENOMEM);
mutex_lock(opts-lock);
+ if (!opts-refcnt) {
+ for (nluns = i = 0; i FSG_MAX_LUNS; ++i)
+ if (common-luns[i])
+ nluns = i + 1;
+ if (!nluns)
+ pr_warn(No LUNS defined, continuing anyway\n);
+ else
+ common-nluns = nluns;
+ pr_info(Number of LUNs=%u\n, common-nluns);
+ }
opts-refcnt++;
mutex_unlock(opts-lock);
+
fsg-function.name = FSG_DRIVER_DESC;
fsg-function.bind = fsg_bind;
fsg-function.unbind= fsg_unbind;
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
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-storage: ignore ZTE MF 823 in mode 0x1225
On Thu, 2015-07-02 at 14:43 +0200, Oliver Neukum wrote: On Wed, 2015-07-01 at 22:50 +0700, Lars Melin wrote: Then I would like to see an lsusb listing for 19d2:1225 with an SD-card interface, I have only seen 19d2:1225 with a single storage interface which is for the windows install cd-rom. There are almost no 3G dongles having the SD-card interface active in default (non-switched) mode. Though I see your point. Is the switching desirable at all for this device or do I need to block the device on the SCSI level? Regards Oliver However, when I reinstall usb_modeswitch I see that the storage functionality is unnecessary to switch this device. Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 8 using xhci_hcd Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device found, idVendor=19d2, idProduct=1225 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Product: ZTE WCDMA Technologies MSM Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Manufacturer: ZTE,Incorporated Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: SerialNumber: MF8230ZTED01CP261718U46EM0SHF3BW707_8C650 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: USB Mass Storage device detected Jul 02 14:56:10 linux-dtbq.site kernel: Vendor: 0x19d2, Product: 0x1225, Revision: 0xf0f1 Jul 02 14:56:10 linux-dtbq.site kernel: Interface Subclass: 0x06, Protocol: 0x50 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: device ignored Jul 02 14:56:10 linux-dtbq.site kernel: storage_probe() failed Jul 02 14:56:10 linux-dtbq.site kernel: -- sending exit command to thread Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: checking bus 3, device 8: /sys/devices/pci:00/:00:14.0/usb3/3-10 Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: bus: 3, device: 8 was not an MTP device Jul 02 14:56:10 linux-dtbq.site usb_modeswitch[2740]: switch device 19d2:1225 on 003/008 Jul 02 14:56:15 linux-dtbq.site kernel: usb 3-10: USB disconnect, device number 8 Jul 02 14:56:16 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 9 using xhci_hcd Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
Hi Felipe, On 02/07/2015 15:14, Felipe Balbi wrote: On Thu, Jul 02, 2015 at 02:55:34PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 02:45 PM, Michal Nazarewicz wrote: On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. On Thu, Jul 02 2015, Felipe Balbi wrote: This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. I’ve came up with [1], which you should feel free to test, but then Krzysztof came along with [2], which among other things addressed the LUN count issue, and I kind of stopped working on the issue waiting for his follow up. Sorry that it took so much time. I have been quite busy with some other things. I will send fixed version of that series in a few hours. I'm taking Michal's patch as a quick fix for the -rc though. That patch is simple enough to get in and solves the issue at hand. I would like to test the patch, is it the one included in the following email ? http://www.spinics.net/lists/linux-usb/msg126292.html Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.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: Viewing many gadgets in error under Windows with mass storage function and configfs
On Thu, Jul 02, 2015 at 03:52:32PM +0200, Gregory CLEMENT wrote: Hi Felipe, On 02/07/2015 15:14, Felipe Balbi wrote: On Thu, Jul 02, 2015 at 02:55:34PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 02:45 PM, Michal Nazarewicz wrote: On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. On Thu, Jul 02 2015, Felipe Balbi wrote: This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. I’ve came up with [1], which you should feel free to test, but then Krzysztof came along with [2], which among other things addressed the LUN count issue, and I kind of stopped working on the issue waiting for his follow up. Sorry that it took so much time. I have been quite busy with some other things. I will send fixed version of that series in a few hours. I'm taking Michal's patch as a quick fix for the -rc though. That patch is simple enough to get in and solves the issue at hand. I would like to test the patch, is it the one included in the following email ? http://www.spinics.net/lists/linux-usb/msg126292.html I just pushed it to my testing/fixes, if you test that, I can still add your Tested-by, thanks -- balbi signature.asc Description: Digital signature
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
Hi Felipe, thanks for you prompt feedback. On 02/07/2015 13:45, Felipe Balbi wrote: Hi, On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: Hello, When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. Oh sorry to not have finding the thread about it. There is no such parameter when using configfs. What would be the correct way to fix it? Making sure you don't lie to the other side of the cable :-) I thought about enumerate how many LUN where configured through configfs and then setting the exact number using fsg_common_set_nluns. yeah, that should be the way. There is no problem at all under Linux so maybe it is jut the way how the gadget is exposed through USB. no, we shouldn't really lie, this really needs fixing. Ok so don't hesitate to ask me to test or if you need any help. Thanks, Gregory I tested it on the kernel 3.17 and 4.1 using the musb of the SoC AM335x. Thanks for your feedback. Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.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: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Thu, 2015-07-02 at 13:35 +0200, Hans de Goede wrote: Hi, On 02-07-15 10:45, Oliver Neukum wrote: On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. So how is this dealt with for usbip ? As far as I can tell, it isn't. Running a storage device over usbip is a bit dangerous. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Viewing many gadgets in error under Windows with mass storage function and configfs
Hi Felipe, On 02/07/2015 16:01, Felipe Balbi wrote: On Thu, Jul 02, 2015 at 03:52:32PM +0200, Gregory CLEMENT wrote: Hi Felipe, On 02/07/2015 15:14, Felipe Balbi wrote: On Thu, Jul 02, 2015 at 02:55:34PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 02:45 PM, Michal Nazarewicz wrote: On Thu, Jul 02, 2015 at 12:51:54PM +0200, Gregory CLEMENT wrote: When I use configs to configure the mass storage function for the gadget, and when the device is plugged under Windows, then the partition that I expose is well managed, but I also see 7 other gadget in the device manager with error. This seven bogus gadget seems to be the 7 other LUN that are not used. Indeed if I apply this dirty patch: diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2b4ae98 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3511,7 +3511,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) rc = PTR_ERR(opts-common); goto release_opts; } - rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); +// rc = fsg_common_set_nluns(opts-common, FSG_MAX_LUNS); + rc = fsg_common_set_nluns(opts-common, 1); if (rc) goto release_opts; Then there is no more gadget error under Windows. As the value of FSG_MAX_LUNS is 8 and in my configuration I only use one partition, then it makes sens that I see 7 bogus gadgets. I also saw that in the legacy driver, it was possible to modify the number of LUN using the module parameter file_count. On Thu, Jul 02 2015, Felipe Balbi wrote: This has been reported. Michal was working on a fix, but the patch hasn't been applied yet. I’ve came up with [1], which you should feel free to test, but then Krzysztof came along with [2], which among other things addressed the LUN count issue, and I kind of stopped working on the issue waiting for his follow up. Sorry that it took so much time. I have been quite busy with some other things. I will send fixed version of that series in a few hours. I'm taking Michal's patch as a quick fix for the -rc though. That patch is simple enough to get in and solves the issue at hand. I would like to test the patch, is it the one included in the following email ? http://www.spinics.net/lists/linux-usb/msg126292.html I just pushed it to my testing/fixes, if you test that, I can still add your Tested-by, thanks I've tested the patch from your testing/fixes, and I have no more the 7 others gadget seen as bogus under Windows. You can add my Tested-by: Gregory CLEMENT gregory.clem...@free-electrons.com on a AM335x based board. Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.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] usb-storage: ignore ZTE MF 823 in mode 0x1225
On Thu, 2015-07-02 at 15:43 +0200, Oliver Neukum wrote: On Thu, 2015-07-02 at 20:33 +0700, Lars Melin wrote: On 2015-07-02 20:01, Oliver Neukum wrote: On Thu, 2015-07-02 at 14:43 +0200, Oliver Neukum wrote: On Wed, 2015-07-01 at 22:50 +0700, Lars Melin wrote: Then I would like to see an lsusb listing for 19d2:1225 with an SD-card interface, I have only seen 19d2:1225 with a single storage interface which is for the windows install cd-rom. There are almost no 3G dongles having the SD-card interface active in default (non-switched) mode. Though I see your point. Is the switching desirable at all for this device or do I need to block the device on the SCSI level? Regards Oliver However, when I reinstall usb_modeswitch I see that the storage functionality is unnecessary to switch this device. Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 8 using xhci_hcd Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device found, idVendor=19d2, idProduct=1225 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Product: ZTE WCDMA Technologies MSM Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: Manufacturer: ZTE,Incorporated Jul 02 14:56:10 linux-dtbq.site kernel: usb 3-10: SerialNumber: MF8230ZTED01CP261718U46EM0SHF3BW707_8C650 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: USB Mass Storage device detected Jul 02 14:56:10 linux-dtbq.site kernel: Vendor: 0x19d2, Product: 0x1225, Revision: 0xf0f1 Jul 02 14:56:10 linux-dtbq.site kernel: Interface Subclass: 0x06, Protocol: 0x50 Jul 02 14:56:10 linux-dtbq.site kernel: usb-storage 3-10:1.0: device ignored Jul 02 14:56:10 linux-dtbq.site kernel: storage_probe() failed Jul 02 14:56:10 linux-dtbq.site kernel: -- sending exit command to thread Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: checking bus 3, device 8: /sys/devices/pci:00/:00:14.0/usb3/3-10 Jul 02 14:56:10 linux-dtbq.site mtp-probe[2730]: bus: 3, device: 8 was not an MTP device Jul 02 14:56:10 linux-dtbq.site usb_modeswitch[2740]: switch device 19d2:1225 on 003/008 Jul 02 14:56:15 linux-dtbq.site kernel: usb 3-10: USB disconnect, device number 8 Jul 02 14:56:16 linux-dtbq.site kernel: usb 3-10: new high-speed USB device number 9 using xhci_hcd Regards Oliver Yes but there are other 19d2:1225 devices which don't have dual-mode So they reused the ID. Oh just great. Welcome to the world of USB WWAN dongles. This happens *all* the time... The worst offenders I know of are Huawei and ZTE, probably because they make so many. For example 12d1:1001 is used by like 50 devices before modeswitch (I jest, but not by much). Dan (19d2:1403 RNDIS or 19d2:1405 ECM) they do only have one of the modes and they don't auto-flip so they need usb_modeswitch. Understood. And I just found out that the auto flip is unreliable without storage. But do they need storage interfaces to switch to the other modes? As the virtual CD does not go away I don't really see much value in retaining the original (storage only) mode, as all it offers is an opportunity to lose data. This is the first 3G dongle I have seen with multiple LUN on the same USB interface, there has always been one interface for each storage function in the past. So yes I jumped to conclusion.. Anyway, it would be good if you could block the MMC interface on LUN level while still having the virtual cd-rom working. This device is total crap. Do you know about alternative commands that allow selecting the mode the device switches to? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: About zero-length packet design for EHCI
On Wed, Jul 1, 2015 at 11:00 PM, Peter Chen peter.c...@freescale.com wrote: If one is multiple? The wLength at setup packet is 64 bytes, Maximum Packet Length at dQH is 64 bytes, and the Total Bytes at dTD is 64 bytes too, does device must prepare a zero-length packet? I would like to double confirm it since the iPhone (As host after HNP) can't work well with it if I have a zero-length packet after 64 bytes packet which I describe above, if without zero-length packet, the iPhone works well. The host hardware will recognize when this happens, because the HCD will set the appropriate bits in the data-stage qTD. For example, with EHCI the HCD will set the Alternate Next qTD Pointer. Or with UHCI, the HCD will set the SPD (Short Packet Detect) bit. I see it in the code, but it is a null pointer (EHCI_LIST_END), does it mean one qTD (with valid Next qTD Pointer) can handle one 64 byte packet with or without zero-length packet well both? Any reasons why iPhone can't handle zero-length packet well? But the HCD should never prepare a zero-length qTD for an IN transfer. Zero-length packets are the responsibility of the _sender_, and for IN transfers the sender is the peripheral, not the host. I can't speak about the details of the ehci driver. But for this issue we can speak about sender and receiver, Host and gadget both send and receive during transfers. If the protocol does not specify a transfer length, then the sender MUST send a zlt if the transfer is less than expected and a multiple (1*, 2*, 3*) of maxpacketsize. If the protocol does specify a transfer length, then exactly that length should be sent and received with NO ZLT. For example USB mass storage sends multiples of maxpacketsize but does not send ZLTs. If you do an analyzer trace between windows and your gadget, then you can see what those guys think the rules for your protocol are. Regards, Steve -- 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: Configfs usb mass_storage device always reports 8 LUNs ?
Sorry, I didn’t notice this reply before. On 06/22/2015 04:24 PM, Michal Nazarewicz wrote: common-nluns is set to FSG_MAX_LUNS in fsg_alloc_inst (which is called prior to fsg_alloc) so this is not an issue. On Mon, Jun 22 2015, Krzysztof Opasiak wrote: True, but all legacy gadgets which use mass storage call fsg_common_set_nluns() after reading module parameters and after allocating function instance. As argument they pass number luns received in module params. Right. As a quick fix I would propose doing: --- 8 - diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index e1beb14..15c3071 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -2786,7 +2786,7 @@ int fsg_common_set_nluns(struct fsg_common *common, int nluns) return -EINVAL; } - curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL); + curlun = kcalloc(FSG_MAX_LUNS, sizeof(*curlun), GFP_KERNEL); if (unlikely(!curlun)) return -ENOMEM; --- 8 - Since I think we agreed that luns should just be turned into an array with static length the above won’t be too far from that goal. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- -- 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: Viewing many gadgets in error under Windows with mass storage function and configfs
On 07/02/2015 03:14 PM, Felipe Balbi wrote: I'm taking Michal's patch as a quick fix for the -rc though. That patch is simple enough to get in and solves the issue at hand. On Thu, Jul 02 2015, Krzysztof Opasiak wrote: Nope it doesn't. It may read past buffer in case of legacy gadgets please see discussion in [1] 1 - http://marc.info/?l=linux-usbm=143498343720763w=2 Sorry, I’ve missed that reply before. Let’s bring the discussion back to the original thread. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- -- 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: About zero-length packet design for EHCI
On Thu, 2 Jul 2015, Peter Chen wrote: If a control-IN transfer has a data stage that is shorter than wLength and is a multiple of the ep0 maxpacket value, then the peripheral must send a zero-length packet to indicate the end of the data stage. Thus, the UDC driver must prepare a zero-length transaction (DTD). If one is multiple? The wLength at setup packet is 64 bytes, Maximum Packet Length at dQH is 64 bytes, and the Total Bytes at dTD is 64 bytes too, does device must prepare a zero-length packet? No. Here is the exact rule for when a device must prepare a 0-length packet for a control-IN data stage. Let wlength = le16_to_cpu(setup-wLength) and let act_length = req-length be the actual data length. Then an extra 0-length packet is needed if: act_length 0 act_length wlength (act_length % maxpacket) == 0. (Note that if act_length == 0 then req already gives rise to a 0-length packet; no additional packets are needed.) Here's another way to describe the same rule: 1. Every packet before the final packet must have size equal to maxpacket. 2. If act_length wlength then the final packet must have size smaller than maxpacket. 3. If act_length == wlength and act_length 0 then the final packet must have size 0. You can check that this is equivalent to the rule above. In the example you gave, act_length == wlength so a 0-length packet isn't needed. I would like to double confirm it since the iPhone (As host after HNP) can't work well with it if I have a zero-length packet after 64 bytes packet which I describe above, if without zero-length packet, the iPhone works well. That means the iPhone is behaving properly. The host hardware will recognize when this happens, because the HCD will set the appropriate bits in the data-stage qTD. For example, with EHCI the HCD will set the Alternate Next qTD Pointer. Or with UHCI, the HCD will set the SPD (Short Packet Detect) bit. I see it in the code, but it is a null pointer (EHCI_LIST_END), does it mean one qTD (with valid Next qTD Pointer) can handle one 64 byte packet with or without zero-length packet well both? That is a tricky question, and the full answer involves the complicated details of how EHCI works. Here is a simplified answer. If a transfer requires more than one qTD, then all the qTDs but the last one will have their Alternate Next qTD Pointers set to a special value. The last qTD doesn't need this. Since ehci-hcd never uses more than one qTD for a control transfer data stage, the issue doesn't comes up. But if you look at how ehci-hcd handles large bulk transfers, you will see what I mean. Any reasons why iPhone can't handle zero-length packet well? When the iPhone receives the 64-byte packet, it thinks the data stage is finished. It moves on to the status stage, where it sends a 0-length packet to the gadget and waits for the acknowledgment. If the gadget is still trying to send a 0-length packet, it won't acknowledge the 0-length packet sent by the iPhone. The status stage will time out and the control transfer will fail. 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
musb gadget HUB traffic sniffing/forwarding
Hi all, I'm new to linux usb and would like some guidance on achieving some of the following goals. I have a beaglebone black (BBB - currently running kernel v3.12) with OTG in peripheral mode on usb bus#1 and a host port to connect devices on usb bus#2. 1. usb bus#1: connect OTG on BBB as a musb gadget (via gadgetfs) to another PC host (e.g. linux, windows, etc); usb bus#2: connect a hub to BBB host + using libusb to control the hub + relay all traffics from host-musb gadget-libusb-hub-(applicable multiple devices hot-plugged to the hub), and vice versa So far I'm able to see traffics between the PC host and the hub before plugging any device into the hub. That is, usb traffics for endpoints of the hub itself. Once I plug in a device, after certain actions on the hub port, the host will send requests (get_descriptor/set_address) to address 0, which are not forwarded to the gadhetfs user space. I'd like to know: * Are traffics for address 0 visible to musb gadget? What code in usb/musb and/or usb/gadget can be used/revised to retrieve/handle them? * If it is feasible to accomplish the above, then the PC host would enumerate devices plugged into the hub. What code usage/change would be necessary to further retrieve messages addressed to endpoints of individual devices and relay them to the HUB (so the HUB can broadcast to individual devices), and also forward messages from devices received via hub (with libusb) back to host via musb gadget? 2. usb bus#1: connect the BBB OTG to a hub attached to the PC host + using musb gadget if needed: host-hub-OTG-(gadget) + when the host sends any messages to the hub, it would then broadcast (at least for usb2.0) to all ports on the hub. + sniff broadcast traffic on the OTG usb port (on bus#1) I don't necessarily need the OTG gadget to respond anything to the hub or PC host, but hope to sniff all usb messages broadcast by the hub, which means I will be able to see messages sent by the PC host to other devices plugged into the hub. What code in usb/musb and/or usb/gadget can be used/revised to sniff such traffics? I've tried usbmon but it only monitors devices controlled by hcd (on bus#2) not the OTG in peripheral mode (on bus#1) Thanks a lot in advance, Gordon -- 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] arm: koelsch: make USB0 perform Host/Function switching
Hi Phil, when you re-spin this patch could you change the prefix to the following? ARM: shmobile: koelsch: Thanks -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: musb gadget HUB traffic sniffing/forwarding
Hi, On Thu, Jul 02, 2015 at 06:02:50PM -0700, Y. Zhang wrote: Hi all, I'm new to linux usb and would like some guidance on achieving some of the following goals. I have a beaglebone black (BBB - currently running kernel v3.12) with you need to ask for support from whoever gave you that v3.12 kernel. If you're using TI's releases, then you need to get in touch with TI's official support channels. This forum, can't really help you out unless you move to v4.1 kernel. OTG in peripheral mode on usb bus#1 and a host port to connect devices on usb bus#2. 1. usb bus#1: connect OTG on BBB as a musb gadget (via gadgetfs) to another PC host (e.g. linux, windows, etc); usb bus#2: connect a hub to BBB host + using libusb to control the hub + relay all traffics from host-musb gadget-libusb-hub-(applicable multiple devices hot-plugged to the hub), and vice versa So far I'm able to see traffics between the PC host and the hub before plugging any device into the hub. That is, usb traffics for endpoints of the hub itself. Once I plug in a device, after certain actions on the hub port, the host will send requests (get_descriptor/set_address) to address 0, which are not forwarded to the gadhetfs user space. I'd like to know: * Are traffics for address 0 visible to musb gadget? What code in usb/musb and/or usb/gadget can be used/revised to retrieve/handle them? * If it is feasible to accomplish the above, then the PC host would enumerate devices plugged into the hub. What code usage/change would be necessary to further retrieve messages addressed to endpoints of individual devices and relay them to the HUB (so the HUB can broadcast to individual devices), and also forward messages from devices received via hub (with libusb) back to host via musb gadget? 2. usb bus#1: connect the BBB OTG to a hub attached to the PC host + using musb gadget if needed: host-hub-OTG-(gadget) + when the host sends any messages to the hub, it would then broadcast (at least for usb2.0) to all ports on the hub. + sniff broadcast traffic on the OTG usb port (on bus#1) I don't necessarily need the OTG gadget to respond anything to the hub or PC host, but hope to sniff all usb messages broadcast by the hub, which means I will be able to see messages sent by the PC host to other devices plugged into the hub. What code in usb/musb and/or usb/gadget can be used/revised to sniff such traffics? I've tried usbmon but it only monitors devices controlled by hcd (on bus#2) not the OTG in peripheral mode (on bus#1) you really want a HW sniffer. Another option is to mess around with tracepoints and add support for tracing usb_requests and dump out the necessary data. cheers -- balbi signature.asc Description: Digital signature
[PATCH 1/1] usb: gadget: composite: only control read may need zero length packet
According to the USB 2.0 Spec CH5.5.3 and CH8.5.3.2, only control
read (IN) needs zero length packet.
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/composite.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 1c0c3da..c3898b5 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1744,7 +1744,8 @@ unknown:
}
req-length = value;
req-context = cdev;
- req-zero = value w_length;
+ if (ctrl-bRequestType == USB_DIR_IN)
+ req-zero = value w_length;
value = composite_ep0_queue(cdev, req, GFP_ATOMIC);
if (value 0) {
DBG(cdev, ep_queue -- %d\n, value);
@@ -1820,7 +1821,8 @@ try_fun_setup:
if (value = 0 value != USB_GADGET_DELAYED_STATUS) {
req-length = value;
req-context = cdev;
- req-zero = value w_length;
+ if (ctrl-bRequestType == USB_DIR_IN)
+ req-zero = value w_length;
value = composite_ep0_queue(cdev, req, GFP_ATOMIC);
if (value 0) {
DBG(cdev, ep_queue -- %d\n, value);
--
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: About zero-length packet design for EHCI
On Thu, Jul 02, 2015 at 10:46:36AM -0400, Alan Stern wrote: On Thu, 2 Jul 2015, Peter Chen wrote: If a control-IN transfer has a data stage that is shorter than wLength and is a multiple of the ep0 maxpacket value, then the peripheral must send a zero-length packet to indicate the end of the data stage. Thus, the UDC driver must prepare a zero-length transaction (DTD). If one is multiple? The wLength at setup packet is 64 bytes, Maximum Packet Length at dQH is 64 bytes, and the Total Bytes at dTD is 64 bytes too, does device must prepare a zero-length packet? No. Here is the exact rule for when a device must prepare a 0-length packet for a control-IN data stage. Let wlength = le16_to_cpu(setup-wLength) and let act_length = req-length be the actual data length. Then an extra 0-length packet is needed if: act_length 0 act_length wlength (act_length % maxpacket) == 0. (Note that if act_length == 0 then req already gives rise to a 0-length packet; no additional packets are needed.) Here's another way to describe the same rule: 1. Every packet before the final packet must have size equal to maxpacket. 2. If act_length wlength then the final packet must have size smaller than maxpacket. 3. If act_length == wlength and act_length 0 then the final packet must have size 0. You can check that this is equivalent to the rule above. In the example you gave, act_length == wlength so a 0-length packet isn't needed. Thanks, I think I understand it now. Maybe only the case A like below commit needs zero-length packet. usb: chipidea: udc: Disable auto ZLP generation on ep0 953c66469735aed8d2ada639a72b150f01dae605 The host hardware will recognize when this happens, because the HCD will set the appropriate bits in the data-stage qTD. For example, with EHCI the HCD will set the Alternate Next qTD Pointer. Or with UHCI, the HCD will set the SPD (Short Packet Detect) bit. I see it in the code, but it is a null pointer (EHCI_LIST_END), does it mean one qTD (with valid Next qTD Pointer) can handle one 64 byte packet with or without zero-length packet well both? That is a tricky question, and the full answer involves the complicated details of how EHCI works. Here is a simplified answer. If a transfer requires more than one qTD, then all the qTDs but the last one will have their Alternate Next qTD Pointers set to a special value. The last qTD doesn't need this. Since ehci-hcd never uses more than one qTD for a control transfer data stage, the issue doesn't comes up. But if you look at how ehci-hcd handles large bulk transfers, you will see what I mean. The case A I list above, it should uses more than one qTD, right? Any reasons why iPhone can't handle zero-length packet well? When the iPhone receives the 64-byte packet, it thinks the data stage is finished. It moves on to the status stage, where it sends a 0-length packet to the gadget and waits for the acknowledgment. If the gadget is still trying to send a 0-length packet, it won't acknowledge the 0-length packet sent by the iPhone. The status stage will time out and the control transfer will fail. Alan Stern -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On 07/02/2015 05:25 PM, Michal Nazarewicz wrote:
Since I think we agreed that luns should just be turned into an array
with static length the above won’t be too far from that goal.
On Thu, Jul 02 2015, Krzysztof Opasiak wrote:
True, just started working on it.
I just got this to compile but haven’t tested it past that. Feel free
to grab it or whatever:
From 7fd36d82c7d190224961cbdd8b3ee365fe5f5d18 Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz min...@mina86.com
Date: Thu, 2 Jul 2015 19:14:59 +0200
Subject: [PATCH] usb: f_mass_storage: convert luns pointer into an array
Simplify the code by turning fsg_commen::luns into an array of pointers
with a static size rather than having it a pointer to dynamically
allocated array. For legacy gadgets this will waste some memory, but
in configfs-based gadgets the array is allocated with maximum size
anyway.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_mass_storage.c | 52 ++--
drivers/usb/gadget/function/f_mass_storage.h | 2 --
drivers/usb/gadget/legacy/acm_ms.c | 6 ++--
drivers/usb/gadget/legacy/mass_storage.c | 6 ++--
drivers/usb/gadget/legacy/multi.c| 6 ++--
5 files changed, 17 insertions(+), 55 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 15c3071..e34c6b2 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -281,7 +281,7 @@ struct fsg_common {
unsigned intnluns;
unsigned intlun;
- struct fsg_lun **luns;
+ struct fsg_lun *luns[FSG_MAX_LUNS];
struct fsg_lun *curlun;
unsigned intbulk_out_maxpacket;
@@ -2751,11 +2751,11 @@ void fsg_common_remove_lun(struct fsg_lun *lun, bool
sysfs)
}
EXPORT_SYMBOL_GPL(fsg_common_remove_lun);
-static void _fsg_common_remove_luns(struct fsg_common *common, int n)
+void fsg_common_remove_luns(struct fsg_common *common)
{
int i;
- for (i = 0; i n; ++i)
+ for (i = 0; i FSG_MAX_LUNS; ++i)
if (common-luns[i]) {
fsg_common_remove_lun(common-luns[i], common-sysfs);
common-luns[i] = NULL;
@@ -2763,37 +2763,16 @@ static void _fsg_common_remove_luns(struct fsg_common
*common, int n)
}
EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
-void fsg_common_remove_luns(struct fsg_common *common)
-{
- _fsg_common_remove_luns(common, common-nluns);
-}
-
-void fsg_common_free_luns(struct fsg_common *common)
-{
- fsg_common_remove_luns(common);
- kfree(common-luns);
- common-luns = NULL;
-}
-EXPORT_SYMBOL_GPL(fsg_common_free_luns);
-
int fsg_common_set_nluns(struct fsg_common *common, int nluns)
{
- struct fsg_lun **curlun;
-
/* Find out how many LUNs there should be */
if (nluns 1 || nluns FSG_MAX_LUNS) {
pr_err(invalid number of LUNs: %u\n, nluns);
return -EINVAL;
}
- curlun = kcalloc(FSG_MAX_LUNS, sizeof(*curlun), GFP_KERNEL);
- if (unlikely(!curlun))
- return -ENOMEM;
-
- if (common-luns)
- fsg_common_free_luns(common);
+ fsg_common_remove_luns(common);
- common-luns = curlun;
common-nluns = nluns;
return 0;
@@ -2880,7 +2859,7 @@ int fsg_common_create_lun(struct fsg_common *common,
struct fsg_lun_config *cfg,
char *pathbuf, *p;
int rc = -ENOMEM;
- if (!common-nluns || !common-luns)
+ if (!common-nluns)
return -ENODEV;
if (common-luns[id])
@@ -2976,7 +2955,7 @@ int fsg_common_create_luns(struct fsg_common *common,
struct fsg_config *cfg)
return 0;
fail:
- _fsg_common_remove_luns(common, i);
+ fsg_common_remove_luns(common);
return rc;
}
EXPORT_SYMBOL_GPL(fsg_common_create_luns);
@@ -3020,6 +2999,7 @@ EXPORT_SYMBOL_GPL(fsg_common_run_thread);
static void fsg_common_release(struct kref *ref)
{
struct fsg_common *common = container_of(ref, struct fsg_common, ref);
+ unsigned i;
/* If the thread isn't already dead, tell it to exit now */
if (common-state != FSG_STATE_TERMINATED) {
@@ -3027,22 +3007,14 @@ static void fsg_common_release(struct kref *ref)
wait_for_completion(common-thread_notifier);
}
- if (likely(common-luns)) {
- struct fsg_lun **lun_it = common-luns;
- unsigned i = common-nluns;
-
- /* In error recovery common-nluns may be zero. */
- for (; i; --i, ++lun_it) {
- struct fsg_lun *lun = *lun_it;
- if (!lun)
- continue;
+ for (i = 0; i FSG_MAX_LUNS; ++i) {
+ struct fsg_lun *lun = common-luns[i];
+ if (lun) {
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On 07/02/2015 05:25 PM, Michal Nazarewicz wrote: Sorry, I didn’t notice this reply before. On 06/22/2015 04:24 PM, Michal Nazarewicz wrote: common-nluns is set to FSG_MAX_LUNS in fsg_alloc_inst (which is called prior to fsg_alloc) so this is not an issue. On Mon, Jun 22 2015, Krzysztof Opasiak wrote: True, but all legacy gadgets which use mass storage call fsg_common_set_nluns() after reading module parameters and after allocating function instance. As argument they pass number luns received in module params. Right. As a quick fix I would propose doing: --- 8 - diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index e1beb14..15c3071 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -2786,7 +2786,7 @@ int fsg_common_set_nluns(struct fsg_common *common, int nluns) return -EINVAL; } - curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL); + curlun = kcalloc(FSG_MAX_LUNS, sizeof(*curlun), GFP_KERNEL); if (unlikely(!curlun)) return -ENOMEM; --- 8 - Now it should be fine as a temporary solution. Since I think we agreed that luns should just be turned into an array with static length the above won’t be too far from that goal. True, just started working on it. -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/02/2015 07:10 AM, Oliver Neukum wrote: On Thu, 2015-07-02 at 13:35 +0200, Hans de Goede wrote: Hi, On 02-07-15 10:45, Oliver Neukum wrote: On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. So how is this dealt with for usbip ? As far as I can tell, it isn't. Running a storage device over usbip is a bit dangerous. I don't follow that analysis. The usbip interactions with the usb stack all seem to be atomic, and never trigger a syscall, as far as I can tell. A port reset will flip a few bits and return. A urb enqueue queues and wakes a different thread, and returns. The alternate thread performs the sendmsg. I'm not suggesting that running a storage device over usbip is especially safe, but I don't see the limit on the design. Cheers, Jeremy -- 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: MUSB dual-role on AM335x behaving weirdly
Hi, On Thu, Jul 2, 2015 at 2:16 AM, Gregory CLEMENT gregory.clem...@free-electrons.com wrote: Hi Felipe, On 27/05/2015 11:42, Alexandre Belloni wrote: Hi, On 26/05/2015 at 09:51:18 -0500, Felipe Balbi wrote : On Thu, May 14, 2015 at 04:36:33PM -0500, Bin Liu wrote: Alexandre, On Thu, May 14, 2015 at 4:26 PM, Alexandre Belloni alexandre.bell...@free-electrons.com wrote: On 14/05/2015 at 16:16:12 -0500, Bin Liu wrote : I think I found the root cause of the problem: board design issue - I bet the custom board has too much cap on VBUS line. It should be 10uF. We have a custom board that exhibits the issue but it only has a 100nF cap on VBUS. Have you measured the VBUS discharging? Is there any way to share your schematics? Alexandre, any further comments ? Yeah, I have just got more info. This is the relevant part of the schematic: http://free-electrons.com/~alexandre/usb.png The total VBUS capacitance is 200nF and the USB0 pins are connected directly to the AM3358 pins. U1 is actually not fitted. We didn't measure VBUS discharging but we observe the OTG pin sensing stops when plugging an OTG cable without any device. Do you have any news about this topic? Is there something else that we can do to help solving this issue? In the case of CONFIG_USB_MUSB_DUAL_ROLE=y and dr_mode=otg, how is the gadget driver configured? It has to be a module not built-in. Regards, -Bin. Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.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: usb:gadget:hid:Add BOOT mode support
On Wed, 27 May 2015, Golmer Palmer wrote: Hi Alan, Thank you for the new patch! However, a comment: * At page 74 of the specifications, section F.3 Boot Keyboard Requirements, (http://www.usb.org/developers/hidpage/HID1_11.pdf) is defined that: The Boot Keyboard shall, upon reset, return to the non- boot protocol which is described in its Report descriptor. That is, the Report descriptor for a Boot Keyboard does not necessarily match the boot protocol. The Report descriptor for a Boot Keyboard is the non-boot protocol descriptor. This enforces that the default mode (protocol) is REPORT, not BOOT. So why initialize in the patch to 1 for devices without boot support and 0 for devices with boot support? I suggest to always initialize to 1 (=REPORT mode). Remember that the assumption is that the DESCRIPTOR of the BOOT protocol is IDENTICAL to the descriptor of the REPORT protocol, so they are the same. This is the key point! Moreover, if a device don't supports BOOT mode, then the function Get_Protocol() needs to be unimplemented. See page 54, section 7.2.5 Get_Protocol Request. So, I suggest to maintain the old behaviour (goto stall for returning ERROR) in the Get_Protocol() in case of bInterfaceSubClass != USB_INTERFACE_SUBCLASS_BOOT. This can be in the same way as you done in the Set_Protocol function. Golmer: I sent you a revised patch incorporating this correction more than a month ago, and I haven't heard back. Have you tested the newest patch? Does it work correctly? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] usb: gadget: mass_storage: Place EXPORT_SYMBOL_GPL() after func definition
EXPORT_SYMBOL_GPL() is usually placed after function definition
not before.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_mass_storage.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 2e8f37e..48af0a6 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2761,12 +2761,12 @@ static void _fsg_common_remove_luns(struct fsg_common
*common, int n)
common-luns[i] = NULL;
}
}
-EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
void fsg_common_remove_luns(struct fsg_common *common)
{
_fsg_common_remove_luns(common, common-nluns);
}
+EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
void fsg_common_free_luns(struct fsg_common *common)
{
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] usb: gadget: mass_storage: Free buffers if create lun fails
Creation of LUN 0 may fail (for example due to ENOMEM). As fsg_common_set_num_buffers() does some memory allocation we should free it before it becomes unavailable. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com Acked-by: Michal Nazarewicz min...@mina86.com --- drivers/usb/gadget/function/f_mass_storage.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2e8f37e 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3526,6 +3526,9 @@ static struct usb_function_instance *fsg_alloc_inst(void) config.removable = true; rc = fsg_common_create_lun(opts-common, config, 0, lun.0, (const char **)opts-func_inst.group.cg_item.ci_name); + if (rc) + goto release_buffers; + opts-lun0.lun = opts-common-luns[0]; opts-lun0.lun_id = 0; config_group_init_type_name(opts-lun0.group, lun.0, fsg_lun_type); @@ -3536,6 +3539,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) return opts-func_inst; +release_buffers: + fsg_common_free_buffers(opts-common); release_luns: kfree(opts-common-luns); release_opts: -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] usb: gadget: storage-common: Set FSG_MAX_LUNS to 16
Mass storage spec allows up to 16 LUNs, so let's don't
add some more restrictive limits.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_mass_storage.c |2 +-
drivers/usb/gadget/function/storage_common.h |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 48af0a6..ad0f69b 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -54,7 +54,7 @@
* following fields:
*
* nluns Number of LUNs function have (anywhere from 1
- * to FSG_MAX_LUNS which is 8).
+ * to FSG_MAX_LUNS).
* lunsAn array of LUN configuration values. This
* should be filled for each LUN that
* function will include (ie. for nluns
diff --git a/drivers/usb/gadget/function/storage_common.h
b/drivers/usb/gadget/function/storage_common.h
index 70c8914..c3544e6 100644
--- a/drivers/usb/gadget/function/storage_common.h
+++ b/drivers/usb/gadget/function/storage_common.h
@@ -123,7 +123,7 @@ static inline bool fsg_lun_is_open(struct fsg_lun *curlun)
#define FSG_BUFLEN ((u32)16384)
/* Maximal number of LUNs supported in mass storage function */
-#define FSG_MAX_LUNS 8
+#define FSG_MAX_LUNS 16
enum fsg_buffer_state {
BUF_STATE_EMPTY = 0,
--
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: Configfs usb mass_storage device always reports 8 LUNs ?
On 07/02/2015 09:14 PM, Felipe Balbi wrote: On Thu, Jul 02, 2015 at 09:12:30PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 07:16 PM, Michal Nazarewicz wrote: On 07/02/2015 05:25 PM, Michal Nazarewicz wrote: Since I think we agreed that luns should just be turned into an array with static length the above won’t be too far from that goal. On Thu, Jul 02 2015, Krzysztof Opasiak wrote: True, just started working on it. I just got this to compile but haven’t tested it past that. Feel free to grab it or whatever: Ohh, I was quite busy with testing (and debugging) my patches and didn't notice this message, sorry... I have run through this patch and didn't find any serious issues. But in my opinion instead of editing set_nluns() we could simply drop it and simplify legacy gadgets code as we do almost nothing in this function. for the -rc cycle we need a minimal fix, though. Simplification, refactoring and whatever else, needs to be merged during merge window. Ok, sure. So the easiest way to fix problems in previously applied patch would be as Michal suggested here: [1]. Putting 1 and 2 together should be fine as a minimal fix for GET_MAX_LUNS issue. 1 - http://marc.info/?l=linux-usbm=143585072403772w=2 2 - http://marc.info/?l=linux-usbm=143584114100542w=2 Best regards, -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/5] Mass storage fixes and improvements
Hello, This series fix a few bugs in mass storage function, adds a warning message when binding function with not contiguous LUN ids and replace dynamically allocated luns array with static one what simplifies the code. This series also fix GET_MAX_LUNS request to return max id of valid lun, not some number of slots in luns array. Best regards, -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics Krzysztof Opasiak (5): usb: gadget: mass_storage: Free buffers if create lun fails usb: gadget: mass_storage: Place EXPORT_SYMBOL_GPL() after func definition usb: gadget: storage-common: Set FSG_MAX_LUNS to 16 usb: gadget: mass_storage: Use static array for luns usb: gadget: mass_storage: Warn if LUNs ids are not contiguous drivers/usb/gadget/function/f_mass_storage.c | 140 +++--- drivers/usb/gadget/function/f_mass_storage.h |4 - drivers/usb/gadget/function/storage_common.h |2 +- drivers/usb/gadget/legacy/acm_ms.c |6 -- drivers/usb/gadget/legacy/mass_storage.c |6 -- drivers/usb/gadget/legacy/multi.c|6 -- 6 files changed, 62 insertions(+), 102 deletions(-) -- 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 v3] usb: xhci: Add support for URB_ZERO_PACKET to bulk/sg transfers
This commit checks for the URB_ZERO_PACKET flag and creates an extra
zero-length td if the urb transfer length is a multiple of the endpoint's
max packet length.
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
---
drivers/usb/host/xhci-ring.c | 66 ++--
drivers/usb/host/xhci.c | 5
2 files changed, 57 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7d34cbf..31ea1b9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3038,9 +3038,11 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
struct xhci_td *td;
struct scatterlist *sg;
int num_sgs;
- int trb_buff_len, this_sg_len, running_total;
+ int trb_buff_len, this_sg_len, running_total, ret;
unsigned int total_packet_count;
+ bool zero_length_needed;
bool first_trb;
+ int last_trb_num;
u64 addr;
bool more_trbs_coming;
@@ -3056,13 +3058,27 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
total_packet_count = DIV_ROUND_UP(urb-transfer_buffer_length,
usb_endpoint_maxp(urb-ep-desc));
- trb_buff_len = prepare_transfer(xhci, xhci-devs[slot_id],
+ ret = prepare_transfer(xhci, xhci-devs[slot_id],
ep_index, urb-stream_id,
num_trbs, urb, 0, mem_flags);
- if (trb_buff_len 0)
- return trb_buff_len;
+ if (ret 0)
+ return ret;
urb_priv = urb-hcpriv;
+
+ /* Deal with URB_ZERO_PACKET - need one more td/trb */
+ zero_length_needed = urb-transfer_flags URB_ZERO_PACKET
+ urb_priv-length == 2;
+ if (zero_length_needed) {
+ num_trbs++;
+ xhci_dbg(xhci, Creating zero length td.\n);
+ ret = prepare_transfer(xhci, xhci-devs[slot_id],
+ ep_index, urb-stream_id,
+ 1, urb, 1, mem_flags);
+ if (ret 0)
+ return ret;
+ }
+
td = urb_priv-td[0];
/*
@@ -3092,6 +3108,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
trb_buff_len = urb-transfer_buffer_length;
first_trb = true;
+ last_trb_num = zero_length_needed ? 2 : 1;
/* Queue the first TRB, even if it's zero-length */
do {
u32 field = 0;
@@ -3109,12 +3126,15 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
/* Chain all the TRBs together; clear the chain bit in the last
* TRB to indicate it's the last TRB in the chain.
*/
- if (num_trbs 1) {
+ if (num_trbs last_trb_num) {
field |= TRB_CHAIN;
- } else {
- /* FIXME - add check for ZERO_PACKET flag before this */
+ } else if (num_trbs == last_trb_num) {
td-last_trb = ep_ring-enqueue;
field |= TRB_IOC;
+ } else if (zero_length_needed num_trbs == 1) {
+ trb_buff_len = 0;
+ urb_priv-td[1]-last_trb = ep_ring-enqueue;
+ field |= TRB_IOC;
}
/* Only set interrupt on short packet for IN endpoints */
@@ -3176,7 +3196,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
if (running_total + trb_buff_len urb-transfer_buffer_length)
trb_buff_len =
urb-transfer_buffer_length - running_total;
- } while (running_total urb-transfer_buffer_length);
+ } while (num_trbs 0);
check_trb_math(urb, num_trbs, running_total);
giveback_first_trb(xhci, slot_id, ep_index, urb-stream_id,
@@ -3194,7 +3214,9 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
int num_trbs;
struct xhci_generic_trb *start_trb;
bool first_trb;
+ int last_trb_num;
bool more_trbs_coming;
+ bool zero_length_needed;
int start_cycle;
u32 field, length_field;
@@ -3225,7 +3247,6 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
num_trbs++;
running_total += TRB_MAX_BUFF_SIZE;
}
- /* FIXME: this doesn't deal with URB_ZERO_PACKET - need one more */
ret = prepare_transfer(xhci, xhci-devs[slot_id],
ep_index, urb-stream_id,
@@ -3234,6 +3255,20 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
return ret;
urb_priv = urb-hcpriv;
+
+ /* Deal with URB_ZERO_PACKET - need one more td/trb */
+ zero_length_needed = urb-transfer_flags URB_ZERO_PACKET
+ urb_priv-length == 2;
+
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On 07/02/2015 01:46 PM, Oliver Neukum wrote: On Thu, 2015-07-02 at 10:57 -0500, Jeremy White wrote: On 07/02/2015 07:10 AM, Oliver Neukum wrote: On Thu, 2015-07-02 at 13:35 +0200, Hans de Goede wrote: Hi, On 02-07-15 10:45, Oliver Neukum wrote: On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. So how is this dealt with for usbip ? As far as I can tell, it isn't. Running a storage device over usbip is a bit dangerous. I don't follow that analysis. The usbip interactions with the usb stack all seem to be atomic, and never trigger a syscall, as far as I can tell. A port reset will flip a few bits and return. A urb enqueue queues and wakes a different thread, and returns. The alternate thread performs the sendmsg. I'm not suggesting that running a storage device over usbip is especially safe, but I don't see the limit on the design. Are you referring to the current code or the proposed user space pipe? I'm referring to current usbip code. But the proposed driver would have the same behavior. To be clear, I think the only tangible new proposal is the one Hans put forth, which would modify the driver I originally posted to use a netlink socket instead of a passing a file descriptor in via sysfs. That would allow the user space application responsible for initiating the request to provide TLS as desired. It comes with the expense of an extra memcpy, but I suspect Hans is right in saying the network latencies make that an irrelevant cost. Cheers, Jeremy -- 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: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Thu, 2015-07-02 at 10:57 -0500, Jeremy White wrote: On 07/02/2015 07:10 AM, Oliver Neukum wrote: On Thu, 2015-07-02 at 13:35 +0200, Hans de Goede wrote: Hi, On 02-07-15 10:45, Oliver Neukum wrote: On Wed, 2015-07-01 at 10:06 +0100, Daniel P. Berrange wrote: I don't really think it is sensible to be defining implementing new network services which can't support strong encryption and authentication. Rather than passing the file descriptor to the kernel and having it do the I/O directly, I think it would be better to dissassociate the kernel from the network transport, and thus leave all sockets layer data I/O to userspace daemons so they can layer in TLS or SASL or whatever else is appropriate for the security need. Hi, this hits a fundamental limit. Block IO must be done entirely in kernel space or the system will deadlock. The USB stack is part of the block layer and the SCSI error handling. Thus if you involve user space you cannot honor memory allocation with GFP_NOFS and you break all APIs where we pass GFP_NOIO in the USB stack. Supposed you need to reset a storage device for error handling. Your user space programm does a syscall, which allocates memory and needs to launder pages. It proceeds to write to the storage device you wish to reset. It is the same problem FUSE has with writable mmap. You cannot do block devices in user space sanely. So how is this dealt with for usbip ? As far as I can tell, it isn't. Running a storage device over usbip is a bit dangerous. I don't follow that analysis. The usbip interactions with the usb stack all seem to be atomic, and never trigger a syscall, as far as I can tell. A port reset will flip a few bits and return. A urb enqueue queues and wakes a different thread, and returns. The alternate thread performs the sendmsg. I'm not suggesting that running a storage device over usbip is especially safe, but I don't see the limit on the design. Are you referring to the current code or the proposed user space pipe? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] usb: gadget: mass_storage: Use static array for luns
This patch replace dynamicly allocated luns array with static one.
This simplifies the code of mass storage function and modules.
It also fix issue with reporting wrong number of LUNs in GET_MAX_LUN
request.
Also change the nluns to max_luns which is better name for value
stored in that place as we no longer need to store size of luns
array.
Reported-by: David Fisher david.fish...@synopsys.com
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
drivers/usb/gadget/function/f_mass_storage.c | 125 ++
drivers/usb/gadget/function/f_mass_storage.h |4 -
drivers/usb/gadget/legacy/acm_ms.c |6 --
drivers/usb/gadget/legacy/mass_storage.c |6 --
drivers/usb/gadget/legacy/multi.c|6 --
5 files changed, 48 insertions(+), 99 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index ad0f69b..befe251 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -279,9 +279,9 @@ struct fsg_common {
int cmnd_size;
u8 cmnd[MAX_COMMAND_SIZE];
- unsigned intnluns;
+ int max_lun;
unsigned intlun;
- struct fsg_lun **luns;
+ struct fsg_lun *luns[FSG_MAX_LUNS];
struct fsg_lun *curlun;
unsigned intbulk_out_maxpacket;
@@ -533,7 +533,7 @@ static int fsg_setup(struct usb_function *f,
w_length != 1)
return -EDOM;
VDBG(fsg, get max LUN\n);
- *(u8 *)req-buf = fsg-common-nluns - 1;
+ *(u8 *)req-buf = fsg-common-max_lun;
/* Respond with data/status */
req-length = min((u16)1, w_length);
@@ -2131,7 +2131,7 @@ static int received_cbw(struct fsg_dev *fsg, struct
fsg_buffhd *bh)
}
/* Is the CBW meaningful? */
- if (cbw-Lun = FSG_MAX_LUNS || cbw-Flags ~US_BULK_FLAG_IN ||
+ if (cbw-Lun common-max_lun || cbw-Flags ~US_BULK_FLAG_IN ||
cbw-Length = 0 || cbw-Length MAX_COMMAND_SIZE) {
DBG(fsg, non-meaningful CBW: lun = %u, flags = 0x%x,
cmdlen %u\n,
@@ -2159,7 +2159,7 @@ static int received_cbw(struct fsg_dev *fsg, struct
fsg_buffhd *bh)
if (common-data_size == 0)
common-data_dir = DATA_DIR_NONE;
common-lun = cbw-Lun;
- if (common-lun common-nluns)
+ if (common-lun ARRAY_SIZE(common-luns))
common-curlun = common-luns[common-lun];
else
common-curlun = NULL;
@@ -2307,7 +2307,7 @@ reset:
}
common-running = 1;
- for (i = 0; i common-nluns; ++i)
+ for (i = 0; i = common-max_lun; ++i)
if (common-luns[i])
common-luns[i]-unit_attention_data =
SS_RESET_OCCURRED;
@@ -2409,7 +2409,7 @@ static void handle_exception(struct fsg_common *common)
if (old_state == FSG_STATE_ABORT_BULK_OUT)
common-state = FSG_STATE_STATUS_PHASE;
else {
- for (i = 0; i common-nluns; ++i) {
+ for (i = 0; i = common-max_lun; ++i) {
curlun = common-luns[i];
if (!curlun)
continue;
@@ -2453,7 +2453,7 @@ static void handle_exception(struct fsg_common *common)
* a waste of time. Ditto for the INTERFACE_CHANGE and
* CONFIG_CHANGE cases.
*/
- /* for (i = 0; i common-nluns; ++i) */
+ /* for (i = 0; i = common-max_lun; ++i) */
/* if (common-luns[i]) */
/* common-luns[i]-unit_attention_data = */
/* SS_RESET_OCCURRED; */
@@ -2552,12 +2552,11 @@ static int fsg_main_thread(void *common_)
if (!common-ops || !common-ops-thread_exits
|| common-ops-thread_exits(common) 0) {
- struct fsg_lun **curlun_it = common-luns;
- unsigned i = common-nluns;
+ int i = 0;
down_write(common-filesem);
- for (; i--; ++curlun_it) {
- struct fsg_lun *curlun = *curlun_it;
+ for (; i = common-max_lun; ++i) {
+ struct fsg_lun *curlun = common-luns[i];
if (!curlun || !fsg_lun_is_open(curlun))
continue;
@@ -2676,6 +2675,8 @@ static struct fsg_common *fsg_common_setup(struct
fsg_common *common)
init_completion(common-thread_notifier);
init_waitqueue_head(common-fsg_wait);
common-state = FSG_STATE_TERMINATED;
+ memset(common-luns, 0, sizeof(common-luns));
+ common-max_lun = -1;
[PATCH v2 5/5] usb: gadget: mass_storage: Warn if LUNs ids are not contiguous
According to mass storage specification:
Logical Unit Numbers on the device shall be numbered contiguously
starting from LUN 0 to a maximum LUN of 15 (Fh)
So let's at least print a warning message that LUNs ids should be
contiguous.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
drivers/usb/gadget/function/f_mass_storage.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index befe251..08a4fdb 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3042,6 +3042,12 @@ static int fsg_bind(struct usb_configuration *c, struct
usb_function *f)
return -EINVAL;
}
+ for (i = 0; i ARRAY_SIZE(common-luns); ++i)
+ if (!common-luns[i])
+ break;
+ if (common-max_lun != i - 1)
+ pr_err(LUN ids should be contiguous.\n);
+
opts = fsg_opts_from_func_inst(f-fi);
if (!opts-no_configfs) {
ret = fsg_common_set_cdev(fsg-common, c-cdev,
--
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 v3] usb: xhci: Add support for URB_ZERO_PACKET to bulk/sg transfers
This version of the patch changes last_trb varible name to
last_trb_num and fixes code style. I have also added a td to the urb
td array. This now gets prepared properl,y with prepare_transfer(),
and is handled correctly when transferred and completed. It only calls
the urb completion callback once as there is a check in finish_td() to
ensure that all td's have been received.
If you think I should change anything else please let me know.
Thank you,
Reyad Attiyat
On Thu, Jul 2, 2015 at 1:54 PM, Reyad Attiyat reyad.atti...@gmail.com wrote:
This commit checks for the URB_ZERO_PACKET flag and creates an extra
zero-length td if the urb transfer length is a multiple of the endpoint's
max packet length.
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
---
drivers/usb/host/xhci-ring.c | 66
++--
drivers/usb/host/xhci.c | 5
2 files changed, 57 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7d34cbf..31ea1b9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3038,9 +3038,11 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
struct xhci_td *td;
struct scatterlist *sg;
int num_sgs;
- int trb_buff_len, this_sg_len, running_total;
+ int trb_buff_len, this_sg_len, running_total, ret;
unsigned int total_packet_count;
+ bool zero_length_needed;
bool first_trb;
+ int last_trb_num;
u64 addr;
bool more_trbs_coming;
@@ -3056,13 +3058,27 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
total_packet_count = DIV_ROUND_UP(urb-transfer_buffer_length,
usb_endpoint_maxp(urb-ep-desc));
- trb_buff_len = prepare_transfer(xhci, xhci-devs[slot_id],
+ ret = prepare_transfer(xhci, xhci-devs[slot_id],
ep_index, urb-stream_id,
num_trbs, urb, 0, mem_flags);
- if (trb_buff_len 0)
- return trb_buff_len;
+ if (ret 0)
+ return ret;
urb_priv = urb-hcpriv;
+
+ /* Deal with URB_ZERO_PACKET - need one more td/trb */
+ zero_length_needed = urb-transfer_flags URB_ZERO_PACKET
+ urb_priv-length == 2;
+ if (zero_length_needed) {
+ num_trbs++;
+ xhci_dbg(xhci, Creating zero length td.\n);
+ ret = prepare_transfer(xhci, xhci-devs[slot_id],
+ ep_index, urb-stream_id,
+ 1, urb, 1, mem_flags);
+ if (ret 0)
+ return ret;
+ }
+
td = urb_priv-td[0];
/*
@@ -3092,6 +3108,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
trb_buff_len = urb-transfer_buffer_length;
first_trb = true;
+ last_trb_num = zero_length_needed ? 2 : 1;
/* Queue the first TRB, even if it's zero-length */
do {
u32 field = 0;
@@ -3109,12 +3126,15 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
/* Chain all the TRBs together; clear the chain bit in the
last
* TRB to indicate it's the last TRB in the chain.
*/
- if (num_trbs 1) {
+ if (num_trbs last_trb_num) {
field |= TRB_CHAIN;
- } else {
- /* FIXME - add check for ZERO_PACKET flag before this
*/
+ } else if (num_trbs == last_trb_num) {
td-last_trb = ep_ring-enqueue;
field |= TRB_IOC;
+ } else if (zero_length_needed num_trbs == 1) {
+ trb_buff_len = 0;
+ urb_priv-td[1]-last_trb = ep_ring-enqueue;
+ field |= TRB_IOC;
}
/* Only set interrupt on short packet for IN endpoints */
@@ -3176,7 +3196,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
if (running_total + trb_buff_len
urb-transfer_buffer_length)
trb_buff_len =
urb-transfer_buffer_length - running_total;
- } while (running_total urb-transfer_buffer_length);
+ } while (num_trbs 0);
check_trb_math(urb, num_trbs, running_total);
giveback_first_trb(xhci, slot_id, ep_index, urb-stream_id,
@@ -3194,7 +3214,9 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
int num_trbs;
struct xhci_generic_trb *start_trb;
bool first_trb;
+ int last_trb_num;
bool more_trbs_coming;
+ bool zero_length_needed;
int start_cycle;
u32 field, length_field;
@@ -3225,7 +3247,6 @@ int
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On 07/02/2015 07:16 PM, Michal Nazarewicz wrote: On 07/02/2015 05:25 PM, Michal Nazarewicz wrote: Since I think we agreed that luns should just be turned into an array with static length the above won’t be too far from that goal. On Thu, Jul 02 2015, Krzysztof Opasiak wrote: True, just started working on it. I just got this to compile but haven’t tested it past that. Feel free to grab it or whatever: Ohh, I was quite busy with testing (and debugging) my patches and didn't notice this message, sorry... I have run through this patch and didn't find any serious issues. But in my opinion instead of editing set_nluns() we could simply drop it and simplify legacy gadgets code as we do almost nothing in this function. -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On Thu, Jul 02, 2015 at 09:12:30PM +0200, Krzysztof Opasiak wrote: On 07/02/2015 07:16 PM, Michal Nazarewicz wrote: On 07/02/2015 05:25 PM, Michal Nazarewicz wrote: Since I think we agreed that luns should just be turned into an array with static length the above won’t be too far from that goal. On Thu, Jul 02 2015, Krzysztof Opasiak wrote: True, just started working on it. I just got this to compile but haven’t tested it past that. Feel free to grab it or whatever: Ohh, I was quite busy with testing (and debugging) my patches and didn't notice this message, sorry... I have run through this patch and didn't find any serious issues. But in my opinion instead of editing set_nluns() we could simply drop it and simplify legacy gadgets code as we do almost nothing in this function. for the -rc cycle we need a minimal fix, though. Simplification, refactoring and whatever else, needs to be merged during merge window. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 1/5] usb: gadget: mass_storage: Free buffers if create lun fails
Felippe, Could you please take this patch as a fix now and leave the rest of this series to merge window or should I resend it separately? On 07/02/2015 08:56 PM, Krzysztof Opasiak wrote: Creation of LUN 0 may fail (for example due to ENOMEM). As fsg_common_set_num_buffers() does some memory allocation we should free it before it becomes unavailable. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com Acked-by: Michal Nazarewicz min...@mina86.com --- drivers/usb/gadget/function/f_mass_storage.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3cc109f..2e8f37e 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -3526,6 +3526,9 @@ static struct usb_function_instance *fsg_alloc_inst(void) config.removable = true; rc = fsg_common_create_lun(opts-common, config, 0, lun.0, (const char **)opts-func_inst.group.cg_item.ci_name); + if (rc) + goto release_buffers; + opts-lun0.lun = opts-common-luns[0]; opts-lun0.lun_id = 0; config_group_init_type_name(opts-lun0.group, lun.0, fsg_lun_type); @@ -3536,6 +3539,8 @@ static struct usb_function_instance *fsg_alloc_inst(void) return opts-func_inst; +release_buffers: + fsg_common_free_buffers(opts-common); release_luns: kfree(opts-common-luns); release_opts: -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] usb: gadget: mass_storage: Warn if LUNs ids are not contiguous
On Thu, Jul 02 2015, Krzysztof Opasiak wrote:
According to mass storage specification:
Logical Unit Numbers on the device shall be numbered contiguously
starting from LUN 0 to a maximum LUN of 15 (Fh)
So let's at least print a warning message that LUNs ids should be
contiguous.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_mass_storage.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index befe251..08a4fdb 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3042,6 +3042,12 @@ static int fsg_bind(struct usb_configuration *c,
struct usb_function *f)
return -EINVAL;
}
+ for (i = 0; i ARRAY_SIZE(common-luns); ++i)
+ if (!common-luns[i])
+ break;
+ if (common-max_lun != i - 1)
+ pr_err(LUN ids should be contiguous.\n);
+
opts = fsg_opts_from_func_inst(f-fi);
if (!opts-no_configfs) {
ret = fsg_common_set_cdev(fsg-common, c-cdev,
--
1.7.9.5
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
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 4/5] usb: gadget: mass_storage: Use static array for luns
On Thu, Jul 02 2015, Krzysztof Opasiak wrote:
This patch replace dynamicly allocated luns array with static one.
This simplifies the code of mass storage function and modules.
It also fix issue with reporting wrong number of LUNs in GET_MAX_LUN
request.
Also change the nluns to max_luns which is better name for value
stored in that place as we no longer need to store size of luns
array.
Reported-by: David Fisher david.fish...@synopsys.com
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
drivers/usb/gadget/function/f_mass_storage.c | 125
++
drivers/usb/gadget/function/f_mass_storage.h |4 -
drivers/usb/gadget/legacy/acm_ms.c |6 --
drivers/usb/gadget/legacy/mass_storage.c |6 --
drivers/usb/gadget/legacy/multi.c|6 --
5 files changed, 48 insertions(+), 99 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index ad0f69b..befe251 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -279,9 +279,9 @@ struct fsg_common {
int cmnd_size;
u8 cmnd[MAX_COMMAND_SIZE];
- unsigned intnluns;
+ int max_lun;
To be honest, I don’t like this change. It is more idiomatic to store
number of elements in an array rather than index of the last element.
Sooner or later someone will do for (i = 0; i common-max_lun; ++i)
out of habit.
unsigned intlun;
- struct fsg_lun **luns;
+ struct fsg_lun *luns[FSG_MAX_LUNS];
struct fsg_lun *curlun;
unsigned intbulk_out_maxpacket;
@@ -2760,48 +2767,16 @@ static void _fsg_common_remove_luns(struct fsg_common
*common, int n)
fsg_common_remove_lun(common-luns[i], common-sysfs);
common-luns[i] = NULL;
}
+
+ _fsg_common_reduce_max_lun(common);
}
void fsg_common_remove_luns(struct fsg_common *common)
{
- _fsg_common_remove_luns(common, common-nluns);
+ _fsg_common_remove_luns(common, common-max_lun);
Shouldn’t this be:
+ _fsg_common_remove_luns(common, common-max_lun + 1);
}
EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
@@ -2954,7 +2931,7 @@ error_lun:
if (common-sysfs)
device_unregister(lun-dev);
fsg_lun_close(lun);
- common-luns[id] = NULL;
You need to keep this line.
+ _fsg_common_reduce_max_lun(common);
error_sysfs:
kfree(lun);
return rc;
@@ -3305,11 +3282,9 @@ static struct config_group *fsg_lun_make(struct
config_group *group,
return ERR_PTR(ret);
fsg_opts = to_fsg_opts(group-cg_item);
- if (num = FSG_MAX_LUNS)
- return ERR_PTR(-ERANGE);
Going through all the memory allocation and mutex locking just to find
out that num is to big seems wasteful. I’d keep this check here.
mutex_lock(fsg_opts-lock);
- if (fsg_opts-refcnt || fsg_opts-common-luns[num]) {
+ if (fsg_opts-refcnt) {
ret = -EBUSY;
goto out;
}
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
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 4/5] usb: gadget: mass_storage: Use static array for luns
On 07/02/2015 09:29 PM, Michal Nazarewicz wrote:
On Thu, Jul 02 2015, Krzysztof Opasiak wrote:
This patch replace dynamicly allocated luns array with static one.
This simplifies the code of mass storage function and modules.
It also fix issue with reporting wrong number of LUNs in GET_MAX_LUN
request.
Also change the nluns to max_luns which is better name for value
stored in that place as we no longer need to store size of luns
array.
Reported-by: David Fisher david.fish...@synopsys.com
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
drivers/usb/gadget/function/f_mass_storage.c | 125 ++
drivers/usb/gadget/function/f_mass_storage.h |4 -
drivers/usb/gadget/legacy/acm_ms.c |6 --
drivers/usb/gadget/legacy/mass_storage.c |6 --
drivers/usb/gadget/legacy/multi.c|6 --
5 files changed, 48 insertions(+), 99 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index ad0f69b..befe251 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -279,9 +279,9 @@ struct fsg_common {
int cmnd_size;
u8 cmnd[MAX_COMMAND_SIZE];
- unsigned intnluns;
+ int max_lun;
To be honest, I don’t like this change. It is more idiomatic to store
number of elements in an array rather than index of the last element.
Sooner or later someone will do for (i = 0; i common-max_lun; ++i)
out of habit.
The reason why I did this was allowing not contiguous LUNs ids. As you
suggested in earlier discussion [1] we should allow user space shoot
themselves in the foot and tis is one of the consequence. What we should
really return in GET_MAX_LUN is last valid LUN id in our array, not
number of LUNs. Windows is paying attention to this value and for
examnple for luns: 0 5 6 it makes a different if you reply with nluns -1
or max_lun. In first option windows will see only one LUN while in
second all three of them.
As we increment max_lun only when adding a LUN currently it is not
possible to get out of bounds.
unsigned intlun;
- struct fsg_lun **luns;
+ struct fsg_lun *luns[FSG_MAX_LUNS];
struct fsg_lun *curlun;
unsigned intbulk_out_maxpacket;
@@ -2760,48 +2767,16 @@ static void _fsg_common_remove_luns(struct fsg_common
*common, int n)
fsg_common_remove_lun(common-luns[i], common-sysfs);
common-luns[i] = NULL;
}
+
+ _fsg_common_reduce_max_lun(common);
}
void fsg_common_remove_luns(struct fsg_common *common)
{
- _fsg_common_remove_luns(common, common-nluns);
+ _fsg_common_remove_luns(common, common-max_lun);
Shouldn’t this be:
+ _fsg_common_remove_luns(common, common-max_lun + 1);
agree, should be fixed.
}
EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
@@ -2954,7 +2931,7 @@ error_lun:
if (common-sysfs)
device_unregister(lun-dev);
fsg_lun_close(lun);
- common-luns[id] = NULL;
You need to keep this line.
This one is also true. Sorry, my mistake...
+ _fsg_common_reduce_max_lun(common);
error_sysfs:
kfree(lun);
return rc;
@@ -3305,11 +3282,9 @@ static struct config_group *fsg_lun_make(struct
config_group *group,
return ERR_PTR(ret);
fsg_opts = to_fsg_opts(group-cg_item);
- if (num = FSG_MAX_LUNS)
- return ERR_PTR(-ERANGE);
Going through all the memory allocation and mutex locking just to find
out that num is to big seems wasteful. I’d keep this check here.
Ok I will leave it as it was before
.
mutex_lock(fsg_opts-lock);
- if (fsg_opts-refcnt || fsg_opts-common-luns[num]) {
+ if (fsg_opts-refcnt) {
ret = -EBUSY;
goto out;
}
1 - http://marc.info/?l=linux-usbm=143498348620786w=2
--
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Spice-devel] [RFC PATCH 1/1] Add a usbredir kernel module to remotely connect USB devices over IP.
On Thu, 2 Jul 2015, Jeremy White wrote: I don't follow that analysis. The usbip interactions with the usb stack all seem to be atomic, and never trigger a syscall, as far as I can tell. A port reset will flip a few bits and return. A urb enqueue queues and wakes a different thread, and returns. The alternate thread performs the sendmsg. I'm not suggesting that running a storage device over usbip is especially safe, but I don't see the limit on the design. Are you referring to the current code or the proposed user space pipe? I'm referring to current usbip code. But the proposed driver would have the same behavior. To be clear, I think the only tangible new proposal is the one Hans put forth, which would modify the driver I originally posted to use a netlink socket instead of a passing a file descriptor in via sysfs. That would allow the user space application responsible for initiating the request to provide TLS as desired. It comes with the expense of an extra memcpy, but I suspect Hans is right in saying the network latencies make that an irrelevant cost. Oliver is talking about the danger of having part of the communication path for a block device run through userspace. Imagine a situation where the client uses a USB storage device provided by the server as a swap device. And suppose a userspace daemon on the client has to process USB packets as they pass between the client and the server. If the daemon is idle for some time, parts of its address space may get stored in the swap area on the server and paged out. Now consider what happens when those parts of memory need to be paged back in. The client submits a request to read from the swap area. The request is transformed into USB packets and sent through the userspace daemon for transmission to the server. But the daemon can't process the packets because it is waiting for its missing parts to be paged back! Result: deadlock. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: move assignment out of if condition
Fix four occurrences of checkpatch.pl error:
ERROR: do not use assignment in if condition
Signed-off-by: Kris Borer kbo...@gmail.com
---
drivers/usb/core/hcd.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index be5b207..3204a4d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2683,12 +2683,14 @@ int usb_add_hcd(struct usb_hcd *hcd,
* bottom up so that hcds can customize the root hubs before hub_wq
* starts talking to them. (Note, bus id is assigned early too.)
*/
- if ((retval = hcd_buffer_create(hcd)) != 0) {
+ retval = hcd_buffer_create(hcd);
+ if (retval != 0) {
dev_dbg(hcd-self.controller, pool alloc failed\n);
goto err_create_buf;
}
- if ((retval = usb_register_bus(hcd-self)) 0)
+ retval = usb_register_bus(hcd-self);
+ if (retval 0)
goto err_register_bus;
rhdev = usb_alloc_dev(NULL, hcd-self, 0);
@@ -2734,9 +2736,13 @@ int usb_add_hcd(struct usb_hcd *hcd,
/* reset is misnamed; its role is now one-time init. the controller
* should already have been reset (and boot firmware kicked off etc).
*/
- if (hcd-driver-reset (retval = hcd-driver-reset(hcd)) 0) {
- dev_err(hcd-self.controller, can't setup: %d\n, retval);
- goto err_hcd_driver_setup;
+ if (hcd-driver-reset) {
+ retval = hcd-driver-reset(hcd);
+ if (retval 0) {
+ dev_err(hcd-self.controller, can't setup: %d\n,
+ retval);
+ goto err_hcd_driver_setup;
+ }
}
hcd-rh_pollable = 1;
@@ -2766,7 +2772,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
}
/* starting here, usbcore will pay attention to this root hub */
- if ((retval = register_root_hub(hcd)) != 0)
+ retval = register_root_hub(hcd);
+ if (retval != 0)
goto err_register_root_hub;
retval = sysfs_create_group(rhdev-dev.kobj, usb_bus_attr_group);
--
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
[PATCH] usb: dwc3: core: avoid NULL pointer dereference
commit 3e10a2ce98d1 (usb: dwc3: add hsphy_interface
property) introduced a possible NULL pointer
dereference because dwc-hsphy_interface can be
NULL.
In order to fix it, all we have to do is guard
strncmp() against a NULL argument.
Fixes: 3e10a2ce98d1 (usb: dwc3: add hsphy_interface property)
Tested-by: Murali Karicheri m-kariche...@ti.com
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/core.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5c110d8e293b..ff5773c66b84 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -446,10 +446,12 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
/* Select the HS PHY interface */
switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc-hwparams.hwparams3)) {
case DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI:
- if (!strncmp(dwc-hsphy_interface, utmi, 4)) {
+ if (dwc-hsphy_interface
+ !strncmp(dwc-hsphy_interface, utmi, 4)) {
reg = ~DWC3_GUSB2PHYCFG_ULPI_UTMI;
break;
- } else if (!strncmp(dwc-hsphy_interface, ulpi, 4)) {
+ } else if (dwc-hsphy_interface
+ !strncmp(dwc-hsphy_interface, ulpi, 4)) {
reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
dwc3_writel(dwc-regs, DWC3_GUSB2PHYCFG(0), reg);
} else {
--
2.4.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
