Re: [PATCH 00/21] On-demand device registration
On 28 May 2015 at 06:33, Rob Herring robherri...@gmail.com wrote: On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso tomeu.viz...@collabora.com wrote: Hello, I have a problem with the panel on my Tegra Chromebook taking longer than expected to be ready during boot (Stéphane Marchesin reported what is basically the same issue in [0]), and have looked into ordered probing as a better way of solving this than moving nodes around in the DT or playing with initcall levels. While reading the thread [1] that Alexander Holler started with his series to make probing order deterministic, it occurred to me that it should be possible to achieve the same by registering devices as they are referenced by other devices. I like the concept and novel approach. This basically reuses the information that is already implicit in the probe() implementations, saving us from refactoring existing drivers or adding information to DTBs. Something I'm not completely happy with is that I have had to move the call to of_platform_populate after all platform drivers have been registered. Otherwise I don't see how I could register drivers on demand as we don't have yet each driver's compatible strings. Yeah, this is the opposite of what we'd really like. Can you elaborate on the reasons why we would like to have devices registered before built-in drivers finish registering, even if we don't probe them yet? Ideally, we would have a solution that works for modules too. However, we're no worse off. We pretty much build-in dependencies to avoid module ordering problems. Nod, I haven't looked yet at requesting modules on-demand, but I guess it should be doable. Modules that have dependencies described in the firmware should get them probed automatically already though. Perhaps we need to make the probing on-demand rather than simply on device-driver match occurring. I'm afraid that too much old code depends on that. For example, Rafael pointed out to the PNP subsystem, which registers a driver that will probe devices with the EISA ID PNP0c02 to reserve memory regions for devices that will be probed later. http://lxr.free-electrons.com/source/drivers/pnp/system.c My understanding is that probing of PNP0c02 devices must happen before the actual devices that depend on those regions are probed, so if we decoupled the probing from the driver/device registration, we would be breaking that assumption. For machs that don't move of_platform_populate() to a later point, these patches shouldn't cause any problems but it's not guaranteed that we'll avoid all the deferred probes as some drivers may not be registered yet. Ideally, of_platform_populate is not explicitly called by each platform. So I think we need to make this work for the default case. The problem is that some platforms will need fixing because some initcalls assume that some devices will have been registered already, or even probed. I think removing those assumptions shouldn't be problematic because I haven't had much trouble with this on the four platforms I have tested with, but I cannot test every board that is supported upstream. I can ask though the KernelCI folks to boot my branch in all their boards and make sure that those work when of_platform_populate is called in late_initcall. http://kernelci.org/boot/all/job/next/kernel/next-20150619/ I have tested this on boards with Tegra, iMX.6 and Exynos SoCs, and these patches were enough to eliminate all the deferred probes. With this series I get the kernel to output to the panel in 0.5s, instead of 2.8s. That's certainly compelling. Have to say that those numbers are with the serial console enabled (without, it's 0.5s vs 1.5s), but on machines that take longer to boot we should see bigger gains because we won't be sending devices to the end of the queue when their probe is deferred. Regards, Tomeu Rob Regards, Tomeu [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html [1] https://lkml.org/lkml/2014/5/12/452 Tomeu Vizoso (21): regulator: core: Reduce critical area in _regulator_get ARM: tegra: Add gpio-ranges property ARM: tegra: Register drivers before devices ARM: EXYNOS: Register drivers before devices ARM i.MX6q: Register drivers before devices of/platform: Add of_platform_device_ensure() of/platform: Ensure device registration on lookup gpio: Probe GPIO drivers on demand gpio: Probe pinctrl devices on demand regulator: core: Probe regulators on demand drm: Probe panels on demand drm/tegra: Probe dpaux devices on demand i2c: core: Probe i2c master devices on demand pwm: Probe PWM chip devices on demand backlight: Probe backlight devices on demand usb: phy: Probe phy devices on demand clk: Probe clk providers on demand pinctrl: Probe pinctrl devices on demand phy: core: Probe phy providers on demand dma: of: Probe DMA controllers on demand power-supply: Probe power supplies on
Re: ax88179_178a: ethernet to usb dongle disconnect crash
On Mon, Jun 22, 2015 at 04:09:05PM +0530, Vivek Bhagat wrote: Hello Greg, Please find comments inline below- On Jun 17, 2015 6:23 PM, Greg KH gre...@linuxfoundation.org wrote: On Wed, Jun 17, 2015 at 10:31:40AM +0530, Vivek Bhagat wrote: Hi All, I have connected my pc and TV board as below - PC (network i/f) ethernet to usb dongle --- usb port of TV board When I power off my board, i get a kernel crash. Please have a look at log attached here.I debug and found that unregister_netdev() in usbnet_disconnect() clears net_device object and still skb is under processing which tries to access net_device object and leads to crash. 3.10 is really old, please try something more modern like 4.0 at the very least to see if the issue is still there. Our linux 3.10 is having all patches included from stable version 4.0. I doubt that. Otherwise it would just be 4.0 :) Due to integration issue i can not replace vanilla 4.0 kernel in my setup. Then you are stuck getting support from the company that is forcing you to stick with that kernel version, sorry but the community can not help you out here. best of luck, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
On Mon, 22 Jun 2015, James Bottomley wrote: I'm not sure I entirely like this: we are back again treating data corruption problems silently. However, I also believe treating a single flush failure as a critical filesystem error is also wrong: The data's all there correctly; all it does is introduce a potential window were the FS could get corrupted in the unlikely event the system crashed. Obviously, for a disk with a writeback cache that can't do flush, that window is much wider and the real solution should be to try to switch the cache to write through. I agree. Doing the switch manually (by writing to the cache_type attribute file) works, but it's a nuisance to do this when you have a portable USB drive that gets moved among a bunch of machines. How about something like this patch? It transforms FS FLUSH into a log warning from an error but preserves the error on any other path. You'll still get a fairly continuous dump of warnings for one of these devices, though ... do they respond to mode selects turning off the writeback? I would be very surprised if any of those drives support MODE SELECT at all. Maybe your patch will be acceptable, though. We'll have to hear from Markus and Matt. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: On Mon, 22 Jun 2015, James Bottomley wrote: I'm not sure I entirely like this: we are back again treating data corruption problems silently. However, I also believe treating a single flush failure as a critical filesystem error is also wrong: The data's all there correctly; all it does is introduce a potential window were the FS could get corrupted in the unlikely event the system crashed. Obviously, for a disk with a writeback cache that can't do flush, that window is much wider and the real solution should be to try to switch the cache to write through. I agree. Doing the switch manually (by writing to the cache_type attribute file) works, but it's a nuisance to do this when you have a portable USB drive that gets moved among a bunch of machines. Perhaps it might be wise to do this to every USB device ... for external devices, the small performance gain doesn't really make up for the potential data loss. How about something like this patch? It transforms FS FLUSH into a log warning from an error but preserves the error on any other path. You'll still get a fairly continuous dump of warnings for one of these devices, though ... do they respond to mode selects turning off the writeback? I would be very surprised if any of those drives support MODE SELECT at all. I assume the cache type attribute file you refer to above is just pretending their cache is write through rather than actually setting it to be so? The original IDE device had no way of turning their cache types to write through either, but the manufacturers were eventually convinced of the error of their ways. Maybe your patch will be acceptable, though. We'll have to hear from Markus and Matt. We'll probably have to take this to fsdevel as well ... they might have opinions about whether the FS wants to be informed about flush failure. James Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-scsi in -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 2/3] phy: rcar-gen2 usb: Add Host/Function switching for USB0
+Laurent
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.
Looks like you are touching the part which Laurent had a problem with, so
looping him.
Thanks
Kishon
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;
+ 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
Re: [PATCH 1/3] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS
Hello.
On 06/22/2015 05:42 PM, 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
---
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..94a7aeb 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;
+ int vbus_is_indirect;
s/int/bool/.
+ int vbus_indirect_value;
Likewise.
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..2cfdb50 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
[...]
@@ -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_info(dev, %s: can't bind to transceiver\n,
dev_err().
[...]
@@ -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);
dev_info(dev, %stransceiver found\n,
gpriv-transceiver ? : no );
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: Locking issues w/ functionfs gadget and aio?
On Sat, Jun 20, 2015 at 10:24 PM, Al Viro v...@zeniv.linux.org.uk wrote: On Fri, Jun 12, 2015 at 05:51:12PM -0700, John Stultz wrote: I'm not super sure what the right fix is, but if do something like the following (sorry, whitespace corrupted via copy/paste), I don't seem to run into the problem. Looks sane. Which tree would you prefer it to go through, vfs or usb? BTW, in either case you'd need Signed-off-by: on that patch... Heh. I assumed my hack would be the wrong thing, but if there's no better suggestion, I'm happy to submit it. I have no strong preference of which tree it goes through, but I'd like for Andrzej to weigh in to make sure he agrees. I'll write up a proper changelog and resubmit here shortly. thanks for the review! -john -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 0/2] FTDI CBUS GPIO support
On Sun, Jun 21, 2015 at 12:12:55AM +0200, Stefan Agner wrote: Yet another FTDI GPIO patchset. Yet somewhat different to previous implementations... There are three GPIO modes supported by FTDI devices: 1. Asynchronous Bit Bang Mode (used in Sacha's patch) 2. Synchronous Bit Bang Mode (used in Philipp's patch) 3. CBUS Bit Bang Mode (used in Philipp's patch and this patchset) Previous implementations: - Philipp Hachtmann (https://lkml.org/lkml/2014/5/31/181) - Sascha Silbe (https://lkml.org/lkml/2014/6/9/406) The first two modes allow to control the serial pins and use the USB standard data transfer (write/read) to set the GPIO output values. Hence these modes interference with the standard serial mode of the devices, but are fast. The third option uses the USB control transfer to set GPIOs (which makes bit banging slower), and allows to control only 4 pins. The controllable pins are predefined per device type (in FT232R CBUS0-3, in FT232H ACBUS5-9) and are not required for standard UART/serial communication. However, the default configuration is set to auxiliary functions such as TX/RXLED. Hence to make use of them in CBUS Bit Bang mode, the pins need to be reprogrammed to I/O mode first (EEPROM). All three modes are supported from userspace by libftdi afaik. Is there a way to retrieve the settings from eeprom and only register the gpio chip based on the configuration? In my use case I would like to use the additional GPIOs to control an embedded board (power off/reset etc.) and use the serial communication alongside. Using libftdi to use the CBUS Bit Bang mode is cumbersome, since libftdi requires to detach the kernel driver to get access to the device. The user needs then to reconnect the serial terminal every time a GPIO has been used. Hence, if any of these modes, I see most value in supporting the CBUS mode through the kernel's gpiolib API. However, since some functions are shared (e.g. set_bitmode to enable the different bit modes), this patchset is does some ground work for the other modes too, in case anybody wants to do further work on them. I agree, the usb-serial driver should only provide access to the four cbus pins if available (and use gpiolib). This patchset currently supports FT232R type of devices and has been tested using a FT232RL device. I think the FT232H (and probably later) types of devices should work too (at least the Table 3.5 in the FT232H data sheet mentions the ACBUS Signal Option I/O mode). However, I don't have such a device to test at my disposal. On the implementation side, I created a distinct GPIO driver in drivers/gpio and create that platform device directly from within the ftdi_sio driver. I understand that the mfd subsystem would be the way to go, however it seems to me quite a big change... At least all USB device IDs would need to be moved to the mfd core device since the mfd device would be registered as a USB driver. I guess the ftdi_sio driver would become a platform device and still live under drivers/usb/serial/...? I just saw that recent discussion by Grant and Linus did not approve this approach...? Using the platform bus -- directly as you do or via MFD -- allows for some (arguably contrived) abstraction but I think we should avoid it nonetheless. USB (serial) does not use it as you already noted, and there's not much to gain from creating a single-cell-mfd child device to the USB interface. Instead, hang the gpio chip directly off the usb interface (not the port), add a new config option, and keep the gpio implementation under drivers/usb/serial (possibly in its own file ftdi_sio-gpio.c). Note that your current implementation fails to remove the gpio chip on device disconnect, leaks resources in error paths, and lacks locking for the gpio state. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
On Mon, 2015-06-22 at 10:55 -0400, Alan Stern wrote:
Some USB mass-storage devices claim to have a write-back cache but
don't support the SYNCHRONIZE CACHE command, which means there is no
way to tell these devices to flush their caches out to permanent
storage. Unfortunately, there is nothing we can do about this.
Until recently this deficiency did not cause any noticeable problems.
But following commit 89fb4cd1f717 (scsi: handle flush errors
properly), the errors are propagated to userspace where they get
reported as failures.
As a workaround, this patch adds a quirks flag for these devices to
the usb-storage driver, and a corresponding SCSI device flag, to
indicate the device can't handle SYNCHRONIZE CACHE. If the flag is
set, the sd driver will override the cache settings reported by the
device, so that the device appears to be write-through. This will
prevent the unsupported command from being sent.
This addresses Bugzilla #89511.
I'm not sure I entirely like this: we are back again treating data
corruption problems silently.
However, I also believe treating a single flush failure as a critical
filesystem error is also wrong: The data's all there correctly; all it
does is introduce a potential window were the FS could get corrupted in
the unlikely event the system crashed.
Obviously, for a disk with a writeback cache that can't do flush, that
window is much wider and the real solution should be to try to switch
the cache to write through.
How about something like this patch? It transforms FS FLUSH into a log
warning from an error but preserves the error on any other path. You'll
still get a fairly continuous dump of warnings for one of these devices,
though ... do they respond to mode selects turning off the writeback?
James
---
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 20badd7..bb9f9f3 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -173,10 +173,21 @@ static bool blk_flush_complete_seq(struct request *rq,
BUG_ON(rq-flush.seq seq);
rq-flush.seq |= seq;
- if (likely(!error))
+ if (likely(!error)) {
seq = blk_flush_cur_seq(rq);
- else
+ } else {
seq = REQ_FSEQ_DONE;
+ printk_ratelimited(KERN_ERR %s: flush failed: data integrity
problem\n,
+ rq-rq_disk ? rq-rq_disk-disk_name : ?);
+ /*
+* returning an error to the FS is wrong: the data is all
+* there, it just might not be written out in the expected
+* order and thus have a window where the integrity is suspect
+* in a crash. Given the small likelihood of actually
+* crashing, we should just log a warning here.
+*/
+ error = 0;
+ }
switch (seq) {
case REQ_FSEQ_PREFLUSH:
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 2/3] 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
---
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;
+ 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 (gpio_is_valid(phy-channel-gpio_pwr))
+ gpio_direction_output(phy-channel-gpio_pwr, 1);
spin_lock_irqsave(drv-lock, flags);
@@ -160,9 +196,9 @@ static int
[PATCH 1/3] 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
---
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..94a7aeb 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;
+ int vbus_is_indirect;
+ int 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..2cfdb50 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_info(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
[PATCH 3/3] 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.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
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..8f394be 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 = gpio2 4 GPIO_ACTIVE_HIGH;
+ renesas,id = gpio5 31 GPIO_ACTIVE_HIGH;
+ renesas,vbus = gpio7 24 GPIO_ACTIVE_HIGH;
+ renesas,vbus-pwr = gpio7 23 GPIO_ACTIVE_HIGH;
};
pcie_bus_clk {
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 2/5] usb: gadget: mass_storage: Enforce contiguous LUN ids
On Mon, 22 Jun 2015, Michal Nazarewicz wrote: On Mon, Jun 22 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 don't allow to bind ms function unless we have at least LUN0 and LUNs ids are contiguous. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com Acked-by: Michal Nazarewicz min...@mina86.com but then again I think that we should let user space shoot themselves in the foot if they want to, especially as letting them to that is less code. ;) How about logging a warning message if the LUNs aren't contiguous but then continuing on? Like we do if the serial number string isn't provided. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
Some USB mass-storage devices claim to have a write-back cache but
don't support the SYNCHRONIZE CACHE command, which means there is no
way to tell these devices to flush their caches out to permanent
storage. Unfortunately, there is nothing we can do about this.
Until recently this deficiency did not cause any noticeable problems.
But following commit 89fb4cd1f717 (scsi: handle flush errors
properly), the errors are propagated to userspace where they get
reported as failures.
As a workaround, this patch adds a quirks flag for these devices to
the usb-storage driver, and a corresponding SCSI device flag, to
indicate the device can't handle SYNCHRONIZE CACHE. If the flag is
set, the sd driver will override the cache settings reported by the
device, so that the device appears to be write-through. This will
prevent the unsupported command from being sent.
This addresses Bugzilla #89511.
Signed-off-by: Alan Stern st...@rowland.harvard.edu
Reported-by: Jun Itou itou_...@infoseek.jp
Reported-by: Markus Rathgeb maggu2...@gmail.com
Reported-by: Matt vick...@hotmail.com
Tested-by: Markus Rathgeb maggu2...@gmail.com
Tested-by: Matt vick...@hotmail.com
Fixes: 89fb4cd1f717a871ef79fa7debbe840e3225cd54
CC: James Bottomley jbottom...@parallels.com
CC: sta...@vger.kernel.org
---
As far as I'm concerned, this can be merged by either Greg or James
(once the current merge window is over, of course).
Alan Stern
[as1779]
drivers/scsi/sd.c |7 +++
drivers/usb/storage/scsiglue.c |4
drivers/usb/storage/unusual_devs.h | 21 +
include/linux/usb_usual.h |2 ++
include/scsi/scsi_device.h |1 +
5 files changed, 35 insertions(+)
Index: usb-4.0/include/scsi/scsi_device.h
===
--- usb-4.0.orig/include/scsi/scsi_device.h
+++ usb-4.0/include/scsi/scsi_device.h
@@ -174,6 +174,7 @@ struct scsi_device {
unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
unsigned broken_fua:1; /* Don't set FUA bit */
unsigned lun_in_cdb:1; /* Store LUN bits in CDB[1] */
+ unsigned broken_synch_cache:1; /* SYNCHRONIZE_CACHE is broken */
atomic_t disk_events_disable_depth; /* disable depth for disk events */
Index: usb-4.0/drivers/scsi/sd.c
===
--- usb-4.0.orig/drivers/scsi/sd.c
+++ usb-4.0/drivers/scsi/sd.c
@@ -2929,6 +2929,13 @@ static void sd_probe_async(void *data, a
sdkp-first_scan = 1;
sdkp-max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
+ /*
+* Some buggy USB bridges don't implement SYNCHRONIZE_CACHE
+* but still claim to be write-back. Override them.
+*/
+ if (sdp-broken_synch_cache)
+ sdkp-cache_override = 1;
+
sd_revalidate_disk(gd);
gd-driverfs_dev = sdp-sdev_gendev;
Index: usb-4.0/include/linux/usb_usual.h
===
--- usb-4.0.orig/include/linux/usb_usual.h
+++ usb-4.0/include/linux/usb_usual.h
@@ -79,6 +79,8 @@
/* Cannot handle MI_REPORT_SUPPORTED_OPERATION_CODES */ \
US_FLAG(MAX_SECTORS_240,0x0800) \
/* Sets max_sectors to 240 */ \
+ US_FLAG(NO_SYNCHRONIZE_CACHE, 0x1000) \
+ /* Cannot handle SYNCHRONIZE CACHE */ \
#define US_FLAG(name, value) US_FL_##name = value ,
enum { US_DO_ALL_FLAGS };
Index: usb-4.0/drivers/usb/storage/scsiglue.c
===
--- usb-4.0.orig/drivers/usb/storage/scsiglue.c
+++ usb-4.0/drivers/usb/storage/scsiglue.c
@@ -260,6 +260,10 @@ static int slave_configure(struct scsi_d
if (us-fflags US_FL_BROKEN_FUA)
sdev-broken_fua = 1;
+ /* Likewise for SYNCHRONIZE CACHE */
+ if (us-fflags US_FL_NO_SYNCHRONIZE_CACHE)
+ sdev-broken_synch_cache = 1;
+
} else {
/* Non-disk-type devices don't need to blacklist any pages
Index: usb-4.0/drivers/usb/storage/unusual_devs.h
===
--- usb-4.0.orig/drivers/usb/storage/unusual_devs.h
+++ usb-4.0/drivers/usb/storage/unusual_devs.h
@@ -392,6 +392,13 @@ UNUSUAL_DEV( 0x04b8, 0x0602, 0x0110, 0x
785EPX Storage,
USB_SC_SCSI, USB_PR_BULK, NULL, US_FL_SINGLE_LUN),
+/* Reported by Jun Itou itou_...@infoseek.jp */
+UNUSUAL_DEV( 0x04bb, 0x0109, 0x0100, 0x0100,
+ I-O DATA DEVICE INC.,
+ I-O DATA HDZ-UES,
+ USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+ US_FL_NO_SYNCHRONIZE_CACHE),
+
/* Not sure who reported this originally but
* Pavel Machek pa...@ucw.cz reported that the extra US_FL_SINGLE_LUN
* flag be added
Re: [PATCH 5/5] usb: gadget: mass_storage: Send correct number of LUNs to host
On 06/22/2015 04:34 PM, Michal Nazarewicz wrote: On Mon, Jun 22 2015, Krzysztof Opasiak wrote: GET_MAX_LUN request should return number of realy created LUNs not the size of preallocated array. This patch changes fsg_common_set_nluns() to fsg_common_set_max_luns() which now only allocates an empty array for luns and set nluns to 0. While LUNS are create we simply count them and store result in nluns which is send later to host. Reported-by: David Fisher david.fish...@synopsys.com Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com At this point I would just change common-luns to be an array rather than a pointer. This would remove need for max_luns all together. Sounds like a good idea and I also though about it but I gave up due to memory usage. Most legacy gadgets reduce size of allocated luns array to number of received luns in module params. Adding array with static size will make this impossible. If we may don't care about this ~80 bytes I will be happy make common-luns array as it simplifies the code. BR's -- Krzysztof Opasiak Samsung RD Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 0/3] R-Car Gen2 USB0 Host/Function switching
This patch set adds support for USB Host/Function switching using external gpios to get the vbus and id signals. I am aware that the dt binding for the USB phy will need updating, but wanted to get this patch set out first to see whether this is the best way to implement this. Phil Phil Edworthy (3): usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS phy: rcar-gen2 usb: Add Host/Function switching for USB0 arm: koelsch: make USB0 perform Host/Function switching arch/arm/boot/dts/r8a7791-koelsch.dts | 7 +- drivers/phy/phy-rcar-gen2.c| 269 ++--- drivers/usb/renesas_usbhs/common.h | 2 + drivers/usb/renesas_usbhs/mod.c| 3 + drivers/usb/renesas_usbhs/mod_gadget.c | 38 + 5 files changed, 294 insertions(+), 25 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
On Mon, 22 Jun 2015, Markus Rathgeb wrote: Maybe your patch will be acceptable, though. We'll have to hear from Markus and Matt. We'll probably have to take this to fsdevel as well ... they might have opinions about whether the FS wants to be informed about flush failure. So, it is okay to wait for the end of that discussion before I start further testing or should I test the patch above? Please test it now. I'd like to know if it fixes your problem, regardless of how the discussion goes. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH v5 4/5] usb: gadget: legacy: don't use __init/__exit attributes for bind/unbind path
In order to prepare to independent gadgets and
gadget drivers registration in udc-core, some of the
functions can't have __init/__exit attributes (almost
only bind/unbind callbacks are affected)
Tested-by: Maxime Ripard maxime.rip...@free-electrons.com
Signed-off-by: Ruslan Bilovol ruslan.bilo...@gmail.com
---
drivers/usb/gadget/legacy/acm_ms.c | 6 +++---
drivers/usb/gadget/legacy/audio.c| 6 +++---
drivers/usb/gadget/legacy/cdc2.c | 6 +++---
drivers/usb/gadget/legacy/dbgp.c | 2 +-
drivers/usb/gadget/legacy/ether.c| 8
drivers/usb/gadget/legacy/gmidi.c| 6 +++---
drivers/usb/gadget/legacy/hid.c | 6 +++---
drivers/usb/gadget/legacy/mass_storage.c | 4 ++--
drivers/usb/gadget/legacy/multi.c| 16
drivers/usb/gadget/legacy/ncm.c | 6 +++---
drivers/usb/gadget/legacy/nokia.c| 6 +++---
drivers/usb/gadget/legacy/printer.c | 6 +++---
drivers/usb/gadget/legacy/serial.c | 2 +-
drivers/usb/gadget/legacy/webcam.c | 4 ++--
drivers/usb/gadget/legacy/zero.c | 2 +-
15 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/gadget/legacy/acm_ms.c
b/drivers/usb/gadget/legacy/acm_ms.c
index c30b7b5..3a48aab 100644
--- a/drivers/usb/gadget/legacy/acm_ms.c
+++ b/drivers/usb/gadget/legacy/acm_ms.c
@@ -121,7 +121,7 @@ static struct usb_function *f_msg;
/*
* We _always_ have both ACM and mass storage functions.
*/
-static int __init acm_ms_do_config(struct usb_configuration *c)
+static int acm_ms_do_config(struct usb_configuration *c)
{
struct fsg_opts *opts;
int status;
@@ -174,7 +174,7 @@ static struct usb_configuration acm_ms_config_driver = {
/*-*/
-static int __init acm_ms_bind(struct usb_composite_dev *cdev)
+static int acm_ms_bind(struct usb_composite_dev *cdev)
{
struct usb_gadget *gadget = cdev-gadget;
struct fsg_opts *opts;
@@ -249,7 +249,7 @@ fail_get_msg:
return status;
}
-static int __exit acm_ms_unbind(struct usb_composite_dev *cdev)
+static int acm_ms_unbind(struct usb_composite_dev *cdev)
{
usb_put_function(f_msg);
usb_put_function_instance(fi_msg);
diff --git a/drivers/usb/gadget/legacy/audio.c
b/drivers/usb/gadget/legacy/audio.c
index f46a395..ba95518 100644
--- a/drivers/usb/gadget/legacy/audio.c
+++ b/drivers/usb/gadget/legacy/audio.c
@@ -167,7 +167,7 @@ static const struct usb_descriptor_header *otg_desc[] = {
/*-*/
-static int __init audio_do_config(struct usb_configuration *c)
+static int audio_do_config(struct usb_configuration *c)
{
int status;
@@ -216,7 +216,7 @@ static struct usb_configuration audio_config_driver = {
/*-*/
-static int __init audio_bind(struct usb_composite_dev *cdev)
+static int audio_bind(struct usb_composite_dev *cdev)
{
#ifndef CONFIG_GADGET_UAC1
struct f_uac2_opts *uac2_opts;
@@ -276,7 +276,7 @@ fail:
return status;
}
-static int __exit audio_unbind(struct usb_composite_dev *cdev)
+static int audio_unbind(struct usb_composite_dev *cdev)
{
#ifdef CONFIG_GADGET_UAC1
if (!IS_ERR_OR_NULL(f_uac1))
diff --git a/drivers/usb/gadget/legacy/cdc2.c b/drivers/usb/gadget/legacy/cdc2.c
index 2e85d94..8d1985c 100644
--- a/drivers/usb/gadget/legacy/cdc2.c
+++ b/drivers/usb/gadget/legacy/cdc2.c
@@ -104,7 +104,7 @@ static struct usb_function_instance *fi_ecm;
/*
* We _always_ have both CDC ECM and CDC ACM functions.
*/
-static int __init cdc_do_config(struct usb_configuration *c)
+static int cdc_do_config(struct usb_configuration *c)
{
int status;
@@ -153,7 +153,7 @@ static struct usb_configuration cdc_config_driver = {
/*-*/
-static int __init cdc_bind(struct usb_composite_dev *cdev)
+static int cdc_bind(struct usb_composite_dev *cdev)
{
struct usb_gadget *gadget = cdev-gadget;
struct f_ecm_opts *ecm_opts;
@@ -211,7 +211,7 @@ fail:
return status;
}
-static int __exit cdc_unbind(struct usb_composite_dev *cdev)
+static int cdc_unbind(struct usb_composite_dev *cdev)
{
usb_put_function(f_acm);
usb_put_function_instance(fi_serial);
diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 633683a..7c42b01 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -284,7 +284,7 @@ fail_1:
return -ENODEV;
}
-static int __init dbgp_bind(struct usb_gadget *gadget,
+static int dbgp_bind(struct usb_gadget *gadget,
struct usb_gadget_driver *driver)
{
int err, stp;
diff --git a/drivers/usb/gadget/legacy/ether.c
[PATCH v5 5/5] usb: gadget: udc-core: independent registration of gadgets and gadget drivers
Change behavior during registration of gadgets and
gadget drivers in udc-core. Instead of previous
approach when for successful probe of usb gadget driver
at least one usb gadget should be already registered
use another one where gadget drivers and gadgets
can be registered in udc-core independently.
Independent registration of gadgets and gadget drivers
is useful for built-in into kernel gadget and gadget
driver case - because it's possible that gadget is
really probed only on late_init stage (due to deferred
probe) whereas gadget driver's probe is silently failed
on module_init stage due to no any UDC added.
Also it is useful for modules case - now there is no
difference what module to insert first: gadget module
or gadget driver one.
Tested-by: Maxime Ripard maxime.rip...@free-electrons.com
Signed-off-by: Ruslan Bilovol ruslan.bilo...@gmail.com
---
drivers/usb/gadget/udc/udc-core.c | 49 ---
include/linux/usb/gadget.h| 2 ++
2 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 441877d..5a764ea 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -51,8 +51,12 @@ struct usb_udc {
static struct class *udc_class;
static LIST_HEAD(udc_list);
+static LIST_HEAD(gadget_driver_pending_list);
static DEFINE_MUTEX(udc_lock);
+static int udc_bind_to_driver(struct usb_udc *udc,
+ struct usb_gadget_driver *driver);
+
/* - */
#ifdef CONFIG_HAS_DMA
@@ -264,6 +268,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
void (*release)(struct device *dev))
{
struct usb_udc *udc;
+ struct usb_gadget_driver *driver;
int ret = -ENOMEM;
udc = kzalloc(sizeof(*udc), GFP_KERNEL);
@@ -311,6 +316,18 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
udc-vbus = true;
+ /* pick up one of pending gadget drivers */
+ list_for_each_entry(driver, gadget_driver_pending_list, pending) {
+ if (!driver-udc_name || strcmp(driver-udc_name,
+ dev_name(udc-dev)) == 0) {
+ ret = udc_bind_to_driver(udc, driver);
+ if (ret)
+ goto err4;
+ list_del(driver-pending);
+ break;
+ }
+ }
+
mutex_unlock(udc_lock);
return 0;
@@ -382,9 +399,16 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
list_del(udc-list);
mutex_unlock(udc_lock);
- if (udc-driver)
+ if (udc-driver) {
+ struct usb_gadget_driver *driver = udc-driver;
+
usb_gadget_remove_driver(udc);
+ mutex_lock(udc_lock);
+ list_add(driver-pending, gadget_driver_pending_list);
+ mutex_unlock(udc_lock);
+ }
+
kobject_uevent(udc-dev.kobj, KOBJ_REMOVE);
flush_work(gadget-work);
device_unregister(udc-dev);
@@ -442,11 +466,7 @@ int usb_gadget_probe_driver(struct usb_gadget_driver
*driver)
if (!ret)
break;
}
- if (ret)
- ret = -ENODEV;
- else if (udc-driver)
- ret = -EBUSY;
- else
+ if (!ret !udc-driver)
goto found;
} else {
list_for_each_entry(udc, udc_list, list) {
@@ -456,9 +476,12 @@ int usb_gadget_probe_driver(struct usb_gadget_driver
*driver)
}
}
- pr_debug(couldn't find an available UDC\n);
+ list_add_tail(driver-pending, gadget_driver_pending_list);
+ pr_info(udc-core: couldn't find an available UDC
+ - added [%s] to list of pending drivers\n,
+ driver-function);
mutex_unlock(udc_lock);
- return ret;
+ return 0;
found:
ret = udc_bind_to_driver(udc, driver);
mutex_unlock(udc_lock);
@@ -484,6 +507,16 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver
*driver)
break;
}
+ if (ret) {
+ struct usb_gadget_driver *tmp;
+
+ list_for_each_entry(tmp, gadget_driver_pending_list, pending)
+ if (tmp == driver) {
+ list_del(driver-pending);
+ ret = 0;
+ break;
+ }
+ }
mutex_unlock(udc_lock);
return ret;
}
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 5482d13..9c6fe22
[PATCH v5 3/5] usb: gadget: udc-core: remove unused usb_udc_attach_driver()
Now when last user of usb_udc_attach_driver() is switched
to passing UDC name via usb_gadget_driver struct, it's safe
to remove this function
Tested-by: Maxime Ripard maxime.rip...@free-electrons.com
Signed-off-by: Ruslan Bilovol ruslan.bilo...@gmail.com
---
drivers/usb/gadget/udc/udc-core.c | 26 --
include/linux/usb/gadget.h| 2 --
2 files changed, 28 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index b431129..441877d 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -427,32 +427,6 @@ err1:
return ret;
}
-int usb_udc_attach_driver(const char *name, struct usb_gadget_driver *driver)
-{
- struct usb_udc *udc = NULL;
- int ret = -ENODEV;
-
- mutex_lock(udc_lock);
- list_for_each_entry(udc, udc_list, list) {
- ret = strcmp(name, dev_name(udc-dev));
- if (!ret)
- break;
- }
- if (ret) {
- ret = -ENODEV;
- goto out;
- }
- if (udc-driver) {
- ret = -EBUSY;
- goto out;
- }
- ret = udc_bind_to_driver(udc, driver);
-out:
- mutex_unlock(udc_lock);
- return ret;
-}
-EXPORT_SYMBOL_GPL(usb_udc_attach_driver);
-
int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
{
struct usb_udc *udc = NULL;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 8bba379..5482d13 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -933,8 +933,6 @@ extern int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget, void (*release)(struct device *dev));
extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget
*gadget);
extern void usb_del_gadget_udc(struct usb_gadget *gadget);
-extern int usb_udc_attach_driver(const char *name,
- struct usb_gadget_driver *driver);
/*-*/
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH v5 2/5] usb: gadget: configfs: pass UDC name via usb_gadget_driver struct
Now when udc-core supports binding to specific UDC by passing
its name via 'udc_name' member of usb_gadget_driver struct,
switch to this generic approach.
Tested-by: Maxime Ripard maxime.rip...@free-electrons.com
Signed-off-by: Ruslan Bilovol ruslan.bilo...@gmail.com
---
drivers/usb/gadget/configfs.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index c42765b..efad021 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -54,7 +54,6 @@ struct gadget_info {
struct list_head string_list;
struct list_head available_func;
- const char *udc_name;
#ifdef CONFIG_USB_OTG
struct usb_otg_descriptor otg;
#endif
@@ -230,21 +229,21 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct
gadget_info *gi,
static ssize_t gadget_dev_desc_UDC_show(struct gadget_info *gi, char *page)
{
- return sprintf(page, %s\n, gi-udc_name ?: );
+ return sprintf(page, %s\n, gi-composite.gadget_driver.udc_name ?:
);
}
static int unregister_gadget(struct gadget_info *gi)
{
int ret;
- if (!gi-udc_name)
+ if (!gi-composite.gadget_driver.udc_name)
return -ENODEV;
ret = usb_gadget_unregister_driver(gi-composite.gadget_driver);
if (ret)
return ret;
- kfree(gi-udc_name);
- gi-udc_name = NULL;
+ kfree(gi-composite.gadget_driver.udc_name);
+ gi-composite.gadget_driver.udc_name = NULL;
return 0;
}
@@ -267,14 +266,16 @@ static ssize_t gadget_dev_desc_UDC_store(struct
gadget_info *gi,
if (ret)
goto err;
} else {
- if (gi-udc_name) {
+ if (gi-composite.gadget_driver.udc_name) {
ret = -EBUSY;
goto err;
}
- ret = usb_udc_attach_driver(name, gi-composite.gadget_driver);
- if (ret)
+ gi-composite.gadget_driver.udc_name = name;
+ ret = usb_gadget_probe_driver(gi-composite.gadget_driver);
+ if (ret) {
+ gi-composite.gadget_driver.udc_name = NULL;
goto err;
- gi-udc_name = name;
+ }
}
mutex_unlock(gi-lock);
return len;
@@ -438,9 +439,9 @@ static int config_usb_cfg_unlink(
* remove the function.
*/
mutex_lock(gi-lock);
- if (gi-udc_name)
+ if (gi-composite.gadget_driver.udc_name)
unregister_gadget(gi);
- WARN_ON(gi-udc_name);
+ WARN_ON(gi-composite.gadget_driver.udc_name);
list_for_each_entry(f, cfg-func_list, list) {
if (f-fi == fi) {
@@ -917,10 +918,10 @@ static int os_desc_unlink(struct config_item *os_desc_ci,
struct usb_composite_dev *cdev = gi-cdev;
mutex_lock(gi-lock);
- if (gi-udc_name)
+ if (gi-composite.gadget_driver.udc_name)
unregister_gadget(gi);
cdev-os_desc_config = NULL;
- WARN_ON(gi-udc_name);
+ WARN_ON(gi-composite.gadget_driver.udc_name);
mutex_unlock(gi-lock);
return 0;
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
This patch introduces a new NCM tx engine, able to operate in standard-
and huawei-style mode.
In the first case, the NDP is disposed after the initial headers and
before any datagram.
What works:
- is able to communicate with compliant NCM devices:
I tested this with a board running the Linux g_ncm gadget driver.
What doesn't work:
- After some packets I start gettint LOTS of EVENT_RX_MEMORY from usbnet,
which fails to allocate an RX SKB in rx_submit(). Don't understand why,
any suggestion would be very welcome.
The tx_fixup function given here, even if actually working, should be
considered as an example: the NCM manager is used here simulating the
cdc_ncm.c behaviour.
Signed-off-by: Enrico Mioso mrkiko...@gmail.com
---
drivers/net/usb/huawei_cdc_ncm.c | 187 ++-
1 file changed, 185 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index 735f7da..217802a 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -29,6 +29,35 @@
#include linux/usb/cdc-wdm.h
#include linux/usb/cdc_ncm.h
+/* NCM management operations: */
+
+/* NCM_INIT_FRAME: prepare for a new frame.
+ * NTH16 header is written to output SKB, NDP data is reset and last
+ * committed NDP pointer set to NULL.
+ * Now, data may be added to this NCM package.
+ */
+#define NCM_INIT_FRAME 1
+
+/* NCM_UPDATE_NDP: adds data to an NDP structure, hence updating it.
+ * Some checks are performed to be sure data fits in, respecting device and
+ * spec constrains.
+ * Normally the NDP is kept in memory and committed to the SKB only when
+ * requested. However, calling this method after NCM_COMMIT_NDP, causes it to
+ * work directly on the already committed SKB copy. this allows for flexibility
+ * in frame ordering.
+ */
+#define NCM_UPDATE_NDP 2
+
+/* Write NDP: commits NDP to output SKB.
+ * This method should be called only once per frame.
+ */
+#define NCM_COMMIT_NDP 3
+
+/* Finalizes NTH16 header: to be called when working in
+ * update-already-committed mode.
+ */
+#define NCM_FINALIZE_NTH 5
+
/* Driver data */
struct huawei_cdc_ncm_state {
struct cdc_ncm_ctx *ctx;
@@ -36,6 +65,16 @@ struct huawei_cdc_ncm_state {
struct usb_driver *subdriver;
struct usb_interface *control;
struct usb_interface *data;
+
+ /* Keeps track of where data starts and ends in SKBs. */
+ int data_start;
+ int data_len;
+
+ /* Last committed NDP for post-commit operations. */
+ struct usb_cdc_ncm_ndp16 *skb_ndp;
+
+ /* Non-committed NDP */
+ struct usb_cdc_ncm_ndp16 *ndp;
};
static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
@@ -53,6 +92,149 @@ static int huawei_cdc_ncm_manage_power(struct usbnet
*usbnet_dev, int on)
return 0;
}
+/* huawei_ncm_mgmt: flexible TX NCM manager.
+ *
+ * Once a non-zero status value is rturned, current frame should be discarded
+ * and operations restarted from scratch.
+ */
+int
+huawei_ncm_mgmt(struct usbnet *dev,
+ struct huawei_cdc_ncm_state *drvstate, struct sk_buff *skb_out,
int mode) {
+ struct usb_cdc_ncm_nth16 *nth16 = (struct usb_cdc_ncm_nth16
*)skb_out-data;
+ struct cdc_ncm_ctx *ctx = drvstate-ctx;
+ struct usb_cdc_ncm_ndp16 *ndp16 = NULL;
+ int ret = -EINVAL;
+ u16 ndplen, index;
+
+ switch (mode) {
+ case NCM_INIT_FRAME:
+
+ /* Write a new NTH16 header */
+ nth16 = (struct usb_cdc_ncm_nth16 *)memset(skb_put(skb_out,
sizeof(struct usb_cdc_ncm_nth16)), 0, sizeof(struct usb_cdc_ncm_nth16));
+ if (!nth16) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ /* NTH16 signature and header length are known a-priori. */
+ nth16-dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN);
+ nth16-wHeaderLength = cpu_to_le16(sizeof(struct
usb_cdc_ncm_nth16));
+
+ /* TX sequence numbering */
+ nth16-wSequence = cpu_to_le16(ctx-tx_seq++);
+
+ /* Forget about previous SKB NDP */
+ drvstate-skb_ndp = NULL;
+
+ /* Allocate a new NDP */
+ ndp16 = kzalloc(ctx-max_ndp_size, GFP_NOIO);
+ if (!ndp16)
+ return ret;
+
+ /* Prepare a new NDP to add data on subsequent calls. */
+ drvstate-ndp = memset(ndp16, 0, ctx-max_ndp_size);
+
+ /* Initial NDP length */
+ ndp16-wLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_ndp16) +
sizeof(struct usb_cdc_ncm_dpe16));
+
+ /* Frame signature: to be reworked in general. */
+ ndp16-dwSignature =
cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN);
+
+ ret = 0;
+ break;
+
+ case NCM_UPDATE_NDP:
+
Re: [PATCH 00/21] On-demand device registration
On Mon, Jun 22, 2015 at 10:23 AM, Tomeu Vizoso tomeu.viz...@collabora.com wrote: On 28 May 2015 at 06:33, Rob Herring robherri...@gmail.com wrote: On Mon, May 25, 2015 at 9:53 AM, Tomeu Vizoso tomeu.viz...@collabora.com wrote: Hello, I have a problem with the panel on my Tegra Chromebook taking longer than expected to be ready during boot (Stéphane Marchesin reported what is basically the same issue in [0]), and have looked into ordered probing as a better way of solving this than moving nodes around in the DT or playing with initcall levels. While reading the thread [1] that Alexander Holler started with his series to make probing order deterministic, it occurred to me that it should be possible to achieve the same by registering devices as they are referenced by other devices. I like the concept and novel approach. This basically reuses the information that is already implicit in the probe() implementations, saving us from refactoring existing drivers or adding information to DTBs. Something I'm not completely happy with is that I have had to move the call to of_platform_populate after all platform drivers have been registered. Otherwise I don't see how I could register drivers on demand as we don't have yet each driver's compatible strings. Yeah, this is the opposite of what we'd really like. Can you elaborate on the reasons why we would like to have devices registered before built-in drivers finish registering, even if we don't probe them yet? My main thought was for modules we will almost always have devices appearing first. More generally, we can have devices and drivers coming or going at any point in time and should not put restrictions on ordering. Also, I think all the probe ordering and dependency tracking should be done within the driver core (i.e. dependencies are a list of struct devices). At some level it has to become firmware specific, but we want to minimize that part. I could be convinced otherwise and you have put more thought into this problem than I have. Ideally, we would have a solution that works for modules too. However, we're no worse off. We pretty much build-in dependencies to avoid module ordering problems. Nod, I haven't looked yet at requesting modules on-demand, but I guess it should be doable. Modules that have dependencies described in the firmware should get them probed automatically already though. Perhaps we need to make the probing on-demand rather than simply on device-driver match occurring. I'm afraid that too much old code depends on that. For example, Rafael pointed out to the PNP subsystem, which registers a driver that will probe devices with the EISA ID PNP0c02 to reserve memory regions for devices that will be probed later. http://lxr.free-electrons.com/source/drivers/pnp/system.c My understanding is that probing of PNP0c02 devices must happen before the actual devices that depend on those regions are probed, so if we decoupled the probing from the driver/device registration, we would be breaking that assumption. That shouldn't matter as PNP matching is PNP specific. We already have different ways of matching with device/driver name or of_match_table for example. Changing how and when OF matching occurs would not affect PNP matching. We do matching on device and driver add currently. For the when part, we would need to add what I'll call async matching or deferred matching which in addition to matching on the of_match_table also matches on the dependency list having probed. Your last series essentially does this, but the difference is yours is not OF specific and I think it needs to be. I mean it is OF specific only in the aspect that matching already is. From a driver and subsystem standpoint, it should not be OF specific much like deferred probe is not OF specific, but in reality only occurs (currently) on OF probed drivers. For machs that don't move of_platform_populate() to a later point, these patches shouldn't cause any problems but it's not guaranteed that we'll avoid all the deferred probes as some drivers may not be registered yet. Ideally, of_platform_populate is not explicitly called by each platform. So I think we need to make this work for the default case. The problem is that some platforms will need fixing because some initcalls assume that some devices will have been registered already, or even probed. I think removing those assumptions shouldn't be problematic because I haven't had much trouble with this on the four platforms I have tested with, but I cannot test every board that is supported upstream. I can ask though the KernelCI folks to boot my branch in all their boards and make sure that those work when of_platform_populate is called in late_initcall. I'd imagine Kevin would be happy to. That is still a subset of h/w, so we'd need a way to disable any solution. Rob -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH v5 0/5] usb/gadget: independent registration of gadgets and gadget drivers
This patchset adds independent registration of gadgets and gadget drivers to udc-core. This is very useful for built-in modules into kernel case since it's possible situation that gadget driver is probing at a time when no gadgets are registered in udc-core. In this case instead of silently failing without of any attempt to recover, with independent registration of gadgets and gadget drivers there is no matter in which order gadgets and gadget drivers are probed/registered. This patch has side-effect on gadget drivers that had __init/__exit attributes on some paths like bind/unbind and (since bind/unbind may happen at any time) should not use them now. This is covered by forth patch = v5: - this set of patches (that I already forgot about) has been successfully tested by Maxime Ripard, who sent me confirmation via personal email, so his Tested-by tag is added to the patches - rebased onto latest 'next' branch of Felipe Balbi's tree v4: - misc fixes - addressed Alan's and Sergei's comments - rebased onto latest 'next' branch of Felipe Balbi's tree v3: - addressed Alan's comments - now UDC name and pending gadget drivers list is a part of struct usb_gadget_driver. - removed usb_udc_attach_driver() function that became unused and not needed now v2: - changed first patch to have only deferred probe part (because Gadget Bus seems to be better variant when some more complicated behavior will be required) - rebased to latest 'next' branch of Felipe Balbi's tree Ruslan Bilovol (5): usb: gadget: bind UDC by name passed via usb_gadget_driver structure usb: gadget: configfs: pass UDC name via usb_gadget_driver struct usb: gadget: udc-core: remove unused usb_udc_attach_driver() usb: gadget: legacy: don't use __init/__exit attributes for bind/unbind path usb: gadget: udc-core: independent registration of gadgets and gadget drivers drivers/usb/gadget/configfs.c| 27 +- drivers/usb/gadget/legacy/acm_ms.c | 6 +-- drivers/usb/gadget/legacy/audio.c| 6 +-- drivers/usb/gadget/legacy/cdc2.c | 6 +-- drivers/usb/gadget/legacy/dbgp.c | 2 +- drivers/usb/gadget/legacy/ether.c| 8 +-- drivers/usb/gadget/legacy/gmidi.c| 6 +-- drivers/usb/gadget/legacy/hid.c | 6 +-- drivers/usb/gadget/legacy/mass_storage.c | 4 +- drivers/usb/gadget/legacy/multi.c| 16 +++--- drivers/usb/gadget/legacy/ncm.c | 6 +-- drivers/usb/gadget/legacy/nokia.c| 6 +-- drivers/usb/gadget/legacy/printer.c | 6 +-- drivers/usb/gadget/legacy/serial.c | 2 +- drivers/usb/gadget/legacy/webcam.c | 4 +- drivers/usb/gadget/legacy/zero.c | 2 +- drivers/usb/gadget/udc/udc-core.c| 87 include/linux/usb/gadget.h | 8 ++- 18 files changed, 117 insertions(+), 91 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
If the 'snps,need-phy-for-wake' is set in the device tree then:
- We know that we can wakeup, so call device_set_wakeup_capable().
The USB core will use this knowledge to enable wakeup by default.
- We know that we should keep the PHY on during suspend if something
on our root hub needs remote wakeup. This requires the patch (USB:
Export usb_wakeup_enabled_descendants()). Note that we don't keep
the PHY on at suspend time if it's not needed because it would be a
power draw.
If we later find some users of dwc2 that can support wakeup without
keeping the PHY on we may want to add a way to call
device_set_wakeup_capable() without keeping the PHY on at suspend
time.
Signed-off-by: Chris Zhong z...@rock-chips.com
Signed-off-by: Douglas Anderson diand...@chromium.org
---
drivers/usb/dwc2/core.h | 2 ++
drivers/usb/dwc2/platform.c | 45 +++--
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 53b8de0..b60a1e8 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -687,6 +687,8 @@ struct dwc2_hsotg {
enum usb_dr_mode dr_mode;
unsigned int hcd_enabled:1;
unsigned int gadget_enabled:1;
+ unsigned int need_phy_for_wake:1;
+ unsigned int phy_off_for_suspend:1;
struct phy *phy;
struct usb_phy *uphy;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 9093530..38fce75 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -42,7 +42,9 @@
#include linux/of_device.h
#include linux/mutex.h
#include linux/platform_device.h
+#include linux/usb.h
+#include linux/usb/hcd.h
#include linux/usb/of.h
#include core.h
@@ -222,6 +224,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg-dr_mode = of_usb_get_dr_mode(dev-dev.of_node);
+ hsotg-need_phy_for_wake =
+ of_property_read_bool(dev-dev.of_node,
+ snps,need-phy-for-wake);
+
/*
* Attempt to find a generic PHY, then look for an old style
* USB PHY
@@ -265,6 +271,14 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg-gadget_enabled = 1;
}
+ /*
+* If we need PHY for wakeup we must be wakeup capable.
+* When we have a device that can wake without the PHY we
+* can adjust this condition.
+*/
+ if (hsotg-need_phy_for_wake)
+ device_set_wakeup_capable(dev-dev, true);
+
if (hsotg-dr_mode != USB_DR_MODE_PERIPHERAL) {
retval = dwc2_hcd_init(hsotg, irq);
if (retval) {
@@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
return retval;
}
+static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
+{
+ struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)-self.root_hub;
+
+ if (dwc2-lx_state == DWC2_L0)
+ return false;
+
+ /* If the controller isn't allowed to wakeup then we can power off. */
+ if (!device_may_wakeup(dwc2-dev))
+ return true;
+
+ /*
+* We don't want to power off the PHY if something under the
+* root hub has wakeup enabled.
+*/
+ if (usb_wakeup_enabled_descendants(root_hub))
+ return false;
+
+ /* No reason to keep the PHY powered, so allow poweroff */
+ return true;
+}
+
static int __maybe_unused dwc2_suspend(struct device *dev)
{
struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
@@ -290,8 +326,10 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
if (dwc2_is_device_mode(dwc2)) {
ret = s3c_hsotg_suspend(dwc2);
} else {
- if (dwc2-lx_state == DWC2_L0)
+ if (!dwc2_can_poweroff_phy(dwc2))
return 0;
+
+ dwc2-phy_off_for_suspend = true;
phy_exit(dwc2-phy);
phy_power_off(dwc2-phy);
@@ -307,9 +345,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
if (dwc2_is_device_mode(dwc2)) {
ret = s3c_hsotg_resume(dwc2);
} else {
+ if (!dwc2-phy_off_for_suspend)
+ return ret;
+
phy_power_on(dwc2-phy);
phy_init(dwc2-phy);
-
+ dwc2-phy_off_for_suspend = false;
}
return ret;
}
--
2.4.3.573.g4eafbef
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
This series of patches, together with https://patchwork.kernel.org/patch/6652341/ from Chris Zhong and a dts change allow us to wake up from a USB device on rk3288 boards. The patches were tested on rk3288-jerry in the chromeos-3.14 kernel. The chromeos-3.14 kernel tested included a full set of dwc2 backports from upstream, so this is expected to function upstream once we get everything setup there. Douglas Anderson (3): USB: Export usb_wakeup_enabled_descendants() Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled Documentation/devicetree/bindings/usb/dwc2.txt | 4 +++ drivers/usb/core/hub.c | 7 ++-- drivers/usb/dwc2/core.h| 2 ++ drivers/usb/dwc2/platform.c| 45 -- include/linux/usb/hcd.h| 5 +++ 5 files changed, 58 insertions(+), 5 deletions(-) -- 2.4.3.573.g4eafbef -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 1/3] USB: Export usb_wakeup_enabled_descendants()
In (e583d9d USB: global suspend and remote wakeup don't mix) we
introduced wakeup_enabled_descendants() as a static function. We'd
like to use this function in USB controller drivers to know if we
should keep the controller on during suspend time, since doing so has
a power impact.
Signed-off-by: Douglas Anderson diand...@chromium.org
---
drivers/usb/core/hub.c | 7 ---
include/linux/usb/hcd.h | 5 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 43cb2f2..fdc59db 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3034,13 +3034,14 @@ static int usb_disable_remote_wakeup(struct usb_device
*udev)
}
/* Count of wakeup-enabled devices at or below udev */
-static unsigned wakeup_enabled_descendants(struct usb_device *udev)
+unsigned usb_wakeup_enabled_descendants(struct usb_device *udev)
{
struct usb_hub *hub = usb_hub_to_struct_hub(udev);
return udev-do_remote_wakeup +
(hub ? hub-wakeup_enabled_descendants : 0);
}
+EXPORT_SYMBOL_GPL(usb_wakeup_enabled_descendants);
/*
* usb_port_suspend - suspend a usb device's upstream port
@@ -3149,7 +3150,7 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
* Therefore we will turn on the suspend feature if udev or any of its
* descendants is enabled for remote wakeup.
*/
- else if (PMSG_IS_AUTO(msg) || wakeup_enabled_descendants(udev) 0)
+ else if (PMSG_IS_AUTO(msg) || usb_wakeup_enabled_descendants(udev) 0)
status = set_port_feature(hub-hdev, port1,
USB_PORT_FEAT_SUSPEND);
else {
@@ -3548,7 +3549,7 @@ static int hub_suspend(struct usb_interface *intf,
pm_message_t msg)
}
if (udev)
hub-wakeup_enabled_descendants +=
- wakeup_enabled_descendants(udev);
+ usb_wakeup_enabled_descendants(udev);
}
if (hdev-do_remote_wakeup hub-quirk_check_port_auto_suspend) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..30d74c9 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -626,11 +626,16 @@ extern wait_queue_head_t usb_kill_urb_queue;
#define usb_endpoint_out(ep_dir) (!((ep_dir) USB_DIR_IN))
#ifdef CONFIG_PM
+extern unsigned usb_wakeup_enabled_descendants(struct usb_device *udev);
extern void usb_root_hub_lost_power(struct usb_device *rhdev);
extern int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg);
extern int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg);
extern void usb_hcd_resume_root_hub(struct usb_hcd *hcd);
#else
+static inline unsigned usb_wakeup_enabled_descendants(struct usb_device *udev)
+{
+ return 0;
+}
static inline void usb_hcd_resume_root_hub(struct usb_hcd *hcd)
{
return;
--
2.4.3.573.g4eafbef
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH v5 1/5] usb: gadget: bind UDC by name passed via usb_gadget_driver structure
Introduce new 'udc_name' member to usb_gadget_driver structure.
The 'udc_name' is a name of UDC that usb_gadget_driver should
be bound to. If udc_name is NULL, it will be bound to any
available UDC.
Tested-by: Maxime Ripard maxime.rip...@free-electrons.com
Signed-off-by: Ruslan Bilovol ruslan.bilo...@gmail.com
---
drivers/usb/gadget/udc/udc-core.c | 24 +++-
include/linux/usb/gadget.h| 4
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index d69c355..b431129 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -456,21 +456,35 @@ EXPORT_SYMBOL_GPL(usb_udc_attach_driver);
int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
{
struct usb_udc *udc = NULL;
- int ret;
+ int ret = -ENODEV;
if (!driver || !driver-bind || !driver-setup)
return -EINVAL;
mutex_lock(udc_lock);
- list_for_each_entry(udc, udc_list, list) {
- /* For now we take the first one */
- if (!udc-driver)
+ if (driver-udc_name) {
+ list_for_each_entry(udc, udc_list, list) {
+ ret = strcmp(driver-udc_name, dev_name(udc-dev));
+ if (!ret)
+ break;
+ }
+ if (ret)
+ ret = -ENODEV;
+ else if (udc-driver)
+ ret = -EBUSY;
+ else
goto found;
+ } else {
+ list_for_each_entry(udc, udc_list, list) {
+ /* For now we take the first one */
+ if (!udc-driver)
+ goto found;
+ }
}
pr_debug(couldn't find an available UDC\n);
mutex_unlock(udc_lock);
- return -ENODEV;
+ return ret;
found:
ret = udc_bind_to_driver(udc, driver);
mutex_unlock(udc_lock);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 4f3dfb7..8bba379 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -824,6 +824,8 @@ static inline int usb_gadget_disconnect(struct usb_gadget
*gadget)
* @reset: Invoked on USB bus reset. It is mandatory for all gadget drivers
* and should be called in_interrupt.
* @driver: Driver model state for this driver.
+ * @udc_name: A name of UDC this driver should be bound to. If udc_name is
NULL,
+ * this driver will be bound to any available UDC.
*
* Devices are disabled till a gadget driver successfully bind()s, which
* means the driver will handle setup() requests needed to enumerate (and
@@ -884,6 +886,8 @@ struct usb_gadget_driver {
/* FIXME support safe rmmod */
struct device_driverdriver;
+
+ char*udc_name;
};
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v5 00/22] usb: third series of updates for dwc2 driver
Hi, On Thu, Jun 4, 2015 at 6:12 AM, Kaukab, Yousaf yousaf.kau...@intel.com wrote: Tested-by: Heiko Stuebner he...@sntech.de -- 8 -- [ 19.799200] BUG: sleeping function called from invalid context at mm/slab.c:2863 Will I see a patch for fixing this ? I am currently on a business trip and can't look into this for at least a couple of weeks. John, since this is your driver, could you fix it up ? Hi Felipe, I've been out of the office for the past 2 weeks and am catching up on stuff now. I'll take a look at it later this week if I don't hear anything from Yousaf. I have patches to fix this issue (and another one). I will send them out hopefully tomorrow. I noticed that (33ad261 usb: dwc2: host: spinlock urb_enqueue) is now in linuxnext, so I pulled it (and many other) changes into the chromeos-3.14 kernel for testing. I still see the problem that Heiko reported. Note that aside from the warning, this causes all sorts of spinlock recursion issues and an eventual freeze on my system. Until Yousaf's patch has landed it seems as if we should do a revert of (33ad261 usb: dwc2: host: spinlock urb_enqueue) to avoid regressing existing systems. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH RFC] 1/2 cdc_ncm: export cdc_ncm_align_tail
Export this function to allow for reusing it in other drivers needing NCM
frames alignment, such as the huawei_cdc_ncm one.
Signed-off-by: Enrico Mioso mrkiko...@gmail.com
---
drivers/net/usb/cdc_ncm.c | 3 ++-
include/linux/usb/cdc_ncm.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8067b8f..1a8b619 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -967,7 +967,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct
usb_interface *intf)
return ret;
}
-static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t
remainder, size_t max)
+void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder,
size_t max)
{
size_t align = ALIGN(skb-len, modulus) - skb-len + remainder;
@@ -976,6 +976,7 @@ static void cdc_ncm_align_tail(struct sk_buff *skb, size_t
modulus, size_t remai
if (align skb_tailroom(skb) = align)
memset(skb_put(skb, align), 0, align);
}
+EXPORT_SYMBOL_GPL(cdc_ncm_align_tail);
/* return a pointer to a valid struct usb_cdc_ncm_ndp16 of type sign, possibly
* allocating a new one within skb
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 7c9b484..8fe7ef0 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -141,5 +141,6 @@ int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int
ndpoffset);
struct sk_buff *
cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);
+void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder,
size_t max);
#endif /* __LINUX_USB_CDC_NCM_H */
--
2.4.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH RFC 0/2] huawei_cdc_ncm: new NCM TX stack for Huawei-style NCM
This is an ALFA version of my new NCM TX engine, built with huawei-style NCM and flexibility in mind. The main goal is to allow foreasily re-ordering NCM frame content. Basic communication works - but an usbnet EVENT_RX_MEMORY will stop the process after some packets. Any idea or comment about any aspect of this code is really apreciated - including suggestions for style and over-long lines problematic to break down, at least for me. Lots of lines longer than 80 chars seemed not easily breakable to me: any suggestion is welcome. Enrico Mioso (2): cdc_ncm: export cdc_ncm_align_tail huawei_cdc_ncm: introduce new TX ncm stack drivers/net/usb/cdc_ncm.c| 3 +- drivers/net/usb/huawei_cdc_ncm.c | 187 ++- include/linux/usb/cdc_ncm.h | 1 + 3 files changed, 188 insertions(+), 3 deletions(-) -- 2.4.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [Patch v1] ax88179_178a: add reset functionality in reset_resume
On Sun, 2015-06-21 at 11:46 +0530, Vivek Bhagat wrote:
Without reset functionality in reset_resume, iperf connection does not
establish after suspend/resume however ping works at the same time.
iperf connection fails with wrong checksum error shown by tcpdump.
reset function inside reset_resume solves above bug. We have verified
this issue on ASIX based ST Lab, Cadyce dongle.
Well, you the experiment rules. This highlights, however, a lot
of duplication in the code paths for reset and bind.
Does this patch on top of yours work for you?
Regards
Oliver
From 0ae01a2eacf4db10d5d299c6567fd3272ad9a1c2 Mon Sep 17 00:00:00 2001
From: Oliver Neukum oneu...@suse.de
Date: Mon, 22 Jun 2015 12:28:55 +0200
Subject: [PATCH] ax88179: unify common code in bind and reset
In both code paths the device is set up anew.
So let's unify them.
Signed-off-by: Oliver Neukum oneu...@suse.com
---
drivers/net/usb/ax88179_178a.c | 152 ++---
1 file changed, 52 insertions(+), 100 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 00928c0..02c9f5c 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1220,21 +1220,15 @@ static int ax88179_led_setting(struct usbnet *dev)
return 0;
}
-static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
+static void setup_chip(struct usbnet *dev, bool phy_auto_detach)
{
u8 buf[5];
u16 *tmp16;
u8 *tmp;
- struct ax88179_data *ax179_data = (struct ax88179_data *)dev-data;
- struct ethtool_eee eee_data;
-
- usbnet_get_endpoints(dev, intf);
tmp16 = (u16 *)buf;
tmp = (u8 *)buf;
- memset(ax179_data, 0, sizeof(*ax179_data));
-
/* Power up ethernet PHY */
*tmp16 = 0;
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PHYPWR_RSTCTL, 2, 2, tmp16);
@@ -1246,6 +1240,11 @@ static int ax88179_bind(struct usbnet *dev, struct
usb_interface *intf)
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_CLK_SELECT, 1, 1, tmp);
msleep(100);
+ if (phy_auto_detach) {
+ /* Ethernet PHY Auto Detach*/
+ ax88179_auto_detach(dev, 0);
+ }
+
ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
ETH_ALEN, dev-net-dev_addr);
memcpy(dev-net-perm_addr, dev-net-dev_addr, ETH_ALEN);
@@ -1254,27 +1253,24 @@ static int ax88179_bind(struct usbnet *dev, struct
usb_interface *intf)
memcpy(tmp, AX88179_BULKIN_SIZE[0], 5);
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_BULKIN_QCTRL, 5, 5, tmp);
- dev-rx_urb_size = 1024 * 20;
-
*tmp = 0x34;
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PAUSE_WATERLVL_LOW, 1, 1, tmp);
*tmp = 0x52;
ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PAUSE_WATERLVL_HIGH,
1, 1, tmp);
+}
- dev-net-netdev_ops = ax88179_netdev_ops;
- dev-net-ethtool_ops = ax88179_ethtool_ops;
- dev-net-needed_headroom = 8;
+static void start_chip(struct usbnet *dev)
+{
+ u8 buf[5];
+ u16 *tmp16;
+ u8 *tmp;
+ struct ethtool_eee eee_data;
+ struct ax88179_data *ax179_data = (struct ax88179_data *)dev-data;
- /* Initialize MII structure */
- dev-mii.dev = dev-net;
- dev-mii.mdio_read = ax88179_mdio_read;
- dev-mii.mdio_write = ax88179_mdio_write;
- dev-mii.phy_id_mask = 0xff;
- dev-mii.reg_num_mask = 0xff;
- dev-mii.phy_id = 0x03;
- dev-mii.supports_gmii = 1;
+ tmp16 = (u16 *)buf;
+ tmp = (u8 *)buf;
dev-net-features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
NETIF_F_RXCSUM;
@@ -1322,6 +1318,40 @@ static int ax88179_bind(struct usbnet *dev, struct
usb_interface *intf)
mii_nway_restart(dev-mii);
usbnet_link_change(dev, 0, 0);
+}
+
+static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ u8 buf[5];
+ u16 *tmp16;
+ u8 *tmp;
+ struct ax88179_data *ax179_data = (struct ax88179_data *)dev-data;
+
+ usbnet_get_endpoints(dev, intf);
+
+ tmp16 = (u16 *)buf;
+ tmp = (u8 *)buf;
+
+ memset(ax179_data, 0, sizeof(*ax179_data));
+
+ setup_chip(dev, false);
+
+ dev-rx_urb_size = 1024 * 20;
+
+ dev-net-netdev_ops = ax88179_netdev_ops;
+ dev-net-ethtool_ops = ax88179_ethtool_ops;
+ dev-net-needed_headroom = 8;
+
+ /* Initialize MII structure */
+ dev-mii.dev = dev-net;
+ dev-mii.mdio_read = ax88179_mdio_read;
+ dev-mii.mdio_write = ax88179_mdio_write;
+ dev-mii.phy_id_mask = 0xff;
+ dev-mii.reg_num_mask = 0xff;
+ dev-mii.phy_id = 0x03;
+ dev-mii.supports_gmii = 1;
+
+ start_chip(dev);
return 0;
}
@@ -1529,90 +1559,12 @@ static int ax88179_reset(struct usbnet *dev)
u8 buf[5];
u16 *tmp16;
u8 *tmp;
-
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Mon, Jun 22, 2015 at 12:41:22PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 21:37:04 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 03:07:48PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that it
appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct
usb_otg_caps *otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
Right. Plus controller drivers that are not updated to use these otg_caps
are also legacy.
Did not understand the Plus case.
Some of 4 properties is present, but its driver dose not use otg_caps?
yes that is what I meant.
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
I didn't understand this point. If a controller driver is not updated
to use otg_caps it is legacy and gadget-otg_caps will be NULL.
Some of existing chipdea platforms work fine on HNP and SRP , which did
not use any new flags and properties, after my patch, those platform should
still enable HNP and SRP without any DT change.
Agreed.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
If controller drivers don't support OTG. gadget-is_otg and
gadget-otg_caps will not be set by them and they don't have to use
of_usb_set_otg_caps().
But after my patch, some time later, this driver adds OTG functions on
it new platform, it should disable any OTG feature for its legacy
platforms (none of properties is passed).
That can be decided in of_usb_update_otg_caps().
Controller sets whatever it supports in otg_caps.
of_usb_updade_otg_caps() checks if it is legacy DT (i.e. no otg-rev) and then
overrides to legacy otg_caps.
So I didn't understand why the decision can't be taken here
for non-legacy controller drivers with legacy DT.
They will set gadget-otg_caps and gadget-is_otg.
If none of the 4 DT flags are passed then we know it is legacy DT
and we can limit to legacy behaviour.
legacy behaviour number is 2, not only one legacy behaviour.
That's why I leave the otg caps decided by controller drivers.
I'm only suggesting that it be decided at one place i.e. in
of_usb_updade_otg_caps() instead of every controller.
Do you see any problems with that approach?
The problem is the override policy for legacy platforms.
For case 3) above, we should enable HNP and SRP,
For case 4) above, after add OTG HNPSRP support, it should disable HNP and SRP.
How I make one decision in of_usb_updade_otg_caps()
for above 2 cases?(the otg-rev is 0 for both).
Li Jun
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
RE: Configfs usb mass_storage device always reports 8 LUNs ?
This works for ok for me.
Can we also remove the misleading (too early) instances of :
pr_info(Number of LUNs=%d\n, common-nluns);
in fsg_common_set_luns() and fsg_common_create_lun()
Thanks,
David
-Original Message-
From: Mike Nazarewicz [mailto:m...@google.com] On Behalf Of Michal Nazarewicz
Sent: 19 June 2015 23:18
To: Felipe Balbi
Cc: David Fisher; linux-usb@vger.kernel.org; Andrew Sinclair
Subject: Re: Configfs usb mass_storage device always reports 8 LUNs ?
On Fri, Jun 19 2015, Felipe Balbi wrote:
the fact is that this needs to be configurable from configfs. If user
sets up a single lun, then this should be as you said, however, if 2
luns are configured, I still want to have those 2 working.
From a user perspective, configfs should support N luns just fine as
long as we have enough memory available.
Yes, sorry, you’re right. I didn’t fallow closely migration to configfs based
configuration and didn’t have the full picture.
After taking another look, I think the best solution is to have a loop in
fsg_alloc, something like the following (beware that this has not been tested
in any way):
From 19ec2796009f8650c7db4358f7e16931ba96e4ee 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
---
drivers/usb/gadget/function/f_mass_storage.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..8e26a12 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3563,14 +3563,28 @@ 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 if (nluns != common-nluns) {
+ pr_info(Adjusting number of LUNs=%u (was %u)\n,
+ nluns, common-nluns);
+ common-nluns = nluns;
+ }
+ }
opts-refcnt++;
mutex_unlock(opts-lock);
+
fsg-function.name = FSG_DRIVER_DESC;
fsg-function.bind = fsg_bind;
fsg-function.unbind= fsg_unbind;
--
2.4.3.573.g4eafbef
--
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--
linux-4.1/drivers/usb/serial/io_edgeport.c: 7 * redundant conditions ?
Hello there,
1.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1049]: (style) Redundant condition:
edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
if ((!edge_serial-is_epic) ||
((edge_serial-is_epic)
(edge_serial-epic_descriptor.Supports.IOSPChase))) {
Maybe
if ((!edge_serial-is_epic) ||
edge_serial-epic_descriptor.Supports.IOSPChase) {
2.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1064]: (style) Redundant condition:
edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
if ((!edge_serial-is_epic) ||
((edge_serial-is_epic)
(edge_serial-epic_descriptor.Supports.IOSPClose))) {
3.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1615]: (style) Redundant condition:
edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
More of the same at lines 1631, 2468, 2497, 2501
Regards
David Binderman
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Mon, Jun 22, 2015 at 12:43:37PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that it
appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct usb_otg_caps
*otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
OK this was our difference in understanding. I was thinking that for non
legacy code otg-rev _must_ be passed. without otg-rev the disable flags
will be ignored. It makes life much easier no?
I have to consider ID pint detect case, this is a most common usage,
after controller driver use otg_caps and enable OTG HNP/SRP, some
platform still just want ID pin detect, no more otg feature required,
it need disable OTG HNP/SRP/ADP, the dt should be:
dr_mode = otg;
adp-disable;
hnp-disable;
srp-didable;
in this case, we cannot require DT user still pass a otg-rev, right?
why do you want otg-rev to be optional for non-legacy DT?
For above ID pin detect case.
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v6 3/3] USB: io_ti: Add heartbeat to keep idle Edgeport ports from disconnecting
On Thu, Jun 18, 2015 at 06:43:36AM -0500, Peter E. Berger wrote:
From: Peter E. Berger pber...@brimson.com
When using newer Edgeport devices such as the EP/416, idle ports are
automatically bounced (disconnected and then reconnected) approximately
every 60 seconds. This breaks programs (e.g: minicom) where idle periods
are common, normal and expected.
I confirmed with the manufacturer (Digi International) that some Edgeports
now ship from the factory with firmware that expects periodic heartbeat
queries from the driver to keep idle ports alive. This patch implements
heartbeat support using the mechanism Digi suggested (periodically
requesting an I2C descriptor address) that appears effective on Edgeports
running the newer firmware (that require it) and benign on Edgeport
devices running older firmware. Since we know that Edgeport firmware
version 4.80 (the version distributed in /lib/firmware/down3.bin and used
for Edgeports that are either running still older versions or have no
onboard non-volatile firmware image) does not require heartbeat support,
this patch schedules heartbeats only on devices running firmware versions
newer than 4.80.
Signed-off-by: Peter E. Berger pber...@brimson.com
---
drivers/usb/serial/io_ti.c | 78
+-
1 file changed, 77 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index c76820b..b7c1ea1 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -101,6 +101,7 @@ struct edgeport_serial {
struct mutex es_lock;
int num_ports_open;
struct usb_serial *serial;
+ struct delayed_work heartbeat_work;
int fw_version;
};
@@ -206,6 +207,18 @@ static void edge_send(struct usb_serial_port *port,
struct tty_struct *tty);
static int edge_create_sysfs_attrs(struct usb_serial_port *port);
static int edge_remove_sysfs_attrs(struct usb_serial_port *port);
+/*
+ * Some release of Edgeport firmware down3.bin after version 4.80
+ * introduced code to automatically disconnect idle devices after periods
+ * of inactivity, typically ~60 seconds. This occurs without regard to
+ * whether ports on the device are open or not. Digi International Tech
+ * Support suggested adding driver heartbeat code to reset the firmware
+ * timer by requesting a descriptor record every 15 seconds, which should be
+ * effective with newer firmware versions that require it, and benign with
+ * older versions that do not. In practice 40 seconds seems often enough.
+ */
+#define FW_HEARTBEAT_VERSION_CUTOFF ((4 8) + 80)
+#define FW_HEARTBEAT_SECS 40
/* Timeouts in msecs: firmware downloads take longer */
#define TI_VSEND_TIMEOUT_DEFAULT 1000
@@ -2354,6 +2367,36 @@ static void edge_break(struct tty_struct *tty, int
break_state)
__func__, status);
}
+static void edge_heartbeat_work(struct work_struct *work)
+{
+ struct edgeport_serial *serial;
+ struct ti_i2c_desc *rom_desc;
+
+ serial = container_of(work, struct edgeport_serial,
+ heartbeat_work.work);
+
+ rom_desc = kmalloc(sizeof(*rom_desc), GFP_KERNEL);
+
+ /* Descriptor address request is enough to reset the firmware timer */
+ if (!rom_desc || !get_descriptor_addr(serial, I2C_DESC_TYPE_ION,
+ rom_desc)) {
+ dev_err(serial-serial-interface-dev,
+ %s - Incomplete heartbeat\n, __func__);
+ }
+ kfree(rom_desc);
+
+ schedule_delayed_work(serial-heartbeat_work, FW_HEARTBEAT_SECS * HZ);
Use the helper here as well.
+}
+
+static inline void edge_heartbeat_reschedule(struct edgeport_serial
+ *edge_serial)
Odd line break, rename edge_serial or break after void instead if needed.
Perhaps just edge_heartbeat_schedule would have been a better name.
+{
+ if (edge_serial-fw_version FW_HEARTBEAT_VERSION_CUTOFF) {
Return if = cutoff instead.
+ schedule_delayed_work(edge_serial-heartbeat_work,
+ FW_HEARTBEAT_SECS * HZ);
+ }
+}
Looks good otherwise.
Thanks,
Johan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
RE: linux-4.1/drivers/usb/serial/io_edgeport.c: 7 * redundant conditions ?
Hello there Johan, Yes, that is a redundant test. Thanks for the confirmation. Care to fix these instances and submit a proper patch that we can apply? My success rate with kernel patches is 0.0%, despite many attempts over the years. I am happy for someone else to claim the victory of submitting a proper patch. Regards David Binderman -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that it
appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct usb_otg_caps
*otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
OK this was our difference in understanding. I was thinking that for non
legacy code otg-rev _must_ be passed. without otg-rev the disable flags
will be ignored. It makes life much easier no?
why do you want otg-rev to be optional for non-legacy DT?
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v6 1/3] USB: io_ti: Increase insufficient timeout for firmware downloads
On Thu, Jun 18, 2015 at 06:43:34AM -0500, Peter E. Berger wrote: From: Peter E. Berger pber...@brimson.com The io_ti driver fails to download firmware to Edgeport devices such as the EP/416. One of the problems is that the default 1 second timeout in ti_vsend_sync() is insufficient for download operations. This patch increases the download timeout to 10 seconds. Patch looks good now. What happens when download fails? Does the device still work (e.g. this is only needed to support newer on-disk firmware)? Perhaps you can mention that in the commit log as well. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH v6 2/3] USB: io_ti: Move request_firmware() calls out of download_fw()
On Thu, Jun 18, 2015 at 06:43:35AM -0500, Peter E. Berger wrote:
From: Peter E. Berger pber...@brimson.com
The io_ti driver fails to download firmware to Edgeport
devices such as the EP/416, even when the on-disk firmware image
(/lib/firmware/edgeport/down3.bin) is more current than the version
on the EP/416. The current download code is broken in a few ways.
Notably it mis-uses global variables OperationalMajorVersion and
OperationalMinorVersion (reading their values before they've been
properly initialized and subsequently initializing them multiple times
without synchronization). This patch drops the global variables and
replaces the redundant calls to request_firmware()/release_firmware()
in download_fw() with a single call pair in edge_startup(); the firmware
image pointer is then passed to download_fw() and build_i2c_fw_hdr().
Also, the firmware has a build_number field, though it apparently is
unused (according to observations of the three firmware images I have
seen and confirmed by Digi Tech Support). This comment describes its
structure, in case it is populated in a future release.
Signed-off-by: Peter E. Berger pber...@brimson.com
---
drivers/usb/serial/io_ti.c | 100
+++--
1 file changed, 51 insertions(+), 49 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 69378a7..c76820b 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -101,6 +101,7 @@ struct edgeport_serial {
struct mutex es_lock;
int num_ports_open;
struct usb_serial *serial;
+ int fw_version;
};
@@ -187,10 +188,6 @@ static const struct usb_device_id id_table_combined[] = {
MODULE_DEVICE_TABLE(usb, id_table_combined);
-static unsigned char OperationalMajorVersion;
-static unsigned char OperationalMinorVersion;
-static unsigned short OperationalBuildNumber;
-
static int closing_wait = EDGE_CLOSING_WAIT;
static bool ignore_cpu_rev;
static int default_uart_mode;/* RS232 */
@@ -751,18 +748,18 @@ exit:
}
/* Build firmware header used for firmware update */
-static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
+static int build_i2c_fw_hdr(u8 *header, struct device *dev,
+ const struct firmware *fw)
{
__u8 *buffer;
int buffer_size;
int i;
- int err;
__u8 cs = 0;
struct ti_i2c_desc *i2c_header;
struct ti_i2c_image_header *img_header;
struct ti_i2c_firmware_rec *firmware_rec;
- const struct firmware *fw;
- const char *fw_name = edgeport/down3.bin;
+ u8 fw_major_version = fw-data[0];
+ u8 fw_minor_version = fw-data[1];
/* In order to update the I2C firmware we must change the type 2 record
* to type 0xF2. This will force the UMP to come up in Boot Mode.
@@ -785,24 +782,11 @@ static int build_i2c_fw_hdr(__u8 *header, struct device
*dev)
// Set entire image of 0xffs
memset(buffer, 0xff, buffer_size);
- err = request_firmware(fw, fw_name, dev);
- if (err) {
- dev_err(dev, Failed to load image \%s\ err %d\n,
- fw_name, err);
- kfree(buffer);
- return err;
- }
-
- /* Save Download Version Number */
- OperationalMajorVersion = fw-data[0];
- OperationalMinorVersion = fw-data[1];
- OperationalBuildNumber = fw-data[2] | (fw-data[3] 8);
-
/* Copy version number into firmware record */
firmware_rec = (struct ti_i2c_firmware_rec *)buffer;
- firmware_rec-Ver_Major = OperationalMajorVersion;
- firmware_rec-Ver_Minor = OperationalMinorVersion;
+ firmware_rec-Ver_Major = fw_major_version;
+ firmware_rec-Ver_Minor = fw_minor_version;
/* Pointer to fw_down memory image */
img_header = (struct ti_i2c_image_header *)fw-data[4];
Note that sanity checks on the firmware size are missing here; you could
fix that as a follow up.
@@ -811,8 +795,6 @@ static int build_i2c_fw_hdr(__u8 *header, struct device
*dev)
fw-data[4 + sizeof(struct ti_i2c_image_header)],
le16_to_cpu(img_header-Length));
- release_firmware(fw);
-
for (i=0; i buffer_size; i++) {
cs = (__u8)(cs + buffer[i]);
}
@@ -826,8 +808,8 @@ static int build_i2c_fw_hdr(__u8 *header, struct device
*dev)
i2c_header-Type= I2C_DESC_TYPE_FIRMWARE_BLANK;
i2c_header-Size= cpu_to_le16(buffer_size);
i2c_header-CheckSum= cs;
- firmware_rec-Ver_Major = OperationalMajorVersion;
- firmware_rec-Ver_Minor = OperationalMinorVersion;
+ firmware_rec-Ver_Major = fw_major_version;
+ firmware_rec-Ver_Minor = fw_minor_version;
return 0;
}
@@ -934,7 +916,8 @@ static int ti_cpu_rev(struct edge_ti_manuf_descriptor
*desc)
* This routine downloads the main operating code into the TI5052,
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Thu, 18 Jun 2015 21:37:04 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 03:07:48PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that it
appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct usb_otg_caps
*otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
Right. Plus controller drivers that are not updated to use these otg_caps
are also legacy.
Did not understand the Plus case.
Some of 4 properties is present, but its driver dose not use otg_caps?
yes that is what I meant.
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
I didn't understand this point. If a controller driver is not updated
to use otg_caps it is legacy and gadget-otg_caps will be NULL.
Some of existing chipdea platforms work fine on HNP and SRP , which did
not use any new flags and properties, after my patch, those platform should
still enable HNP and SRP without any DT change.
Agreed.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
If controller drivers don't support OTG. gadget-is_otg and
gadget-otg_caps will not be set by them and they don't have to use
of_usb_set_otg_caps().
But after my patch, some time later, this driver adds OTG functions on
it new platform, it should disable any OTG feature for its legacy
platforms (none of properties is passed).
That can be decided in of_usb_update_otg_caps().
Controller sets whatever it supports in otg_caps.
of_usb_updade_otg_caps() checks if it is legacy DT (i.e. no otg-rev) and then
overrides to legacy otg_caps.
So I didn't understand why the decision can't be taken here
for non-legacy controller drivers with legacy DT.
They will set gadget-otg_caps and gadget-is_otg.
If none of the 4 DT flags are passed then we know it is legacy DT
and we can limit to legacy behaviour.
legacy behaviour number is 2, not only one legacy behaviour.
That's why I leave the otg caps decided by controller drivers.
I'm only suggesting that it be decided at one place i.e. in
of_usb_updade_otg_caps() instead of every controller.
Do you see any problems with that approach?
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: linux-4.1/drivers/usb/serial/io_edgeport.c: 7 * redundant conditions ?
On Mon, Jun 22, 2015 at 08:40:51AM +, David Binderman wrote:
Hello there,
1.
[linux-4.1/drivers/usb/serial/io_edgeport.c:1049]: (style) Redundant
condition: edge_serial.is_epic. 'A (!A || B)' is equivalent to 'A || B'
if ((!edge_serial-is_epic) ||
((edge_serial-is_epic)
(edge_serial-epic_descriptor.Supports.IOSPChase))) {
Maybe
if ((!edge_serial-is_epic) ||
edge_serial-epic_descriptor.Supports.IOSPChase) {
Yes, that is a redundant test.
Care to fix these instances and submit a proper patch that we can apply?
You can drop the parentheses around the negation as well.
Thanks,
Johan
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 3/3] arm: koelsch: make USB0 perform Host/Function switching
Hello.
On 06/22/2015 05:42 PM, 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.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
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..8f394be 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
[...]
@@ -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 = gpio2 4 GPIO_ACTIVE_HIGH;
+ renesas,id = gpio5 31 GPIO_ACTIVE_HIGH;
+ renesas,vbus = gpio7 24 GPIO_ACTIVE_HIGH;
+ renesas,vbus-pwr = gpio7 23 GPIO_ACTIVE_HIGH;
The prop names should end with -gpio or even -gpios, according to the
GPIO bindings.
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
Maybe your patch will be acceptable, though. We'll have to hear from Markus and Matt. We'll probably have to take this to fsdevel as well ... they might have opinions about whether the FS wants to be informed about flush failure. So, it is okay to wait for the end of that discussion before I start further testing or should I test the patch above? -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
On Mon, 2015-06-22 at 13:48 -0400, Alan Stern wrote: On Mon, 22 Jun 2015, James Bottomley wrote: On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: On Mon, 22 Jun 2015, James Bottomley wrote: I'm not sure I entirely like this: we are back again treating data corruption problems silently. However, I also believe treating a single flush failure as a critical filesystem error is also wrong: The data's all there correctly; all it does is introduce a potential window were the FS could get corrupted in the unlikely event the system crashed. Obviously, for a disk with a writeback cache that can't do flush, that window is much wider and the real solution should be to try to switch the cache to write through. I agree. Doing the switch manually (by writing to the cache_type attribute file) works, but it's a nuisance to do this when you have a portable USB drive that gets moved among a bunch of machines. Perhaps it might be wise to do this to every USB device ... for external devices, the small performance gain doesn't really make up for the potential data loss. How about something like this patch? It transforms FS FLUSH into a log warning from an error but preserves the error on any other path. You'll still get a fairly continuous dump of warnings for one of these devices, though ... do they respond to mode selects turning off the writeback? I would be very surprised if any of those drives support MODE SELECT at all. I assume the cache type attribute file you refer to above is just pretending their cache is write through rather than actually setting it to be so? Yes; I'm referring to cache_type_store() in sd.c, and writing temporary write through, which does not issue a MODE SELECT command. It would be easy enough for people to try leaving out the temporary, but I don't expect it to work. The original IDE device had no way of turning their cache types to write through either, but the manufacturers were eventually convinced of the error of their ways. In this case the stupidity resides in the USB-ATA bridge. You can see the gory details at https://bugzilla.kernel.org/show_bug.cgi?id=89511#c19 OK, so that says the SAT in the bridge doesn't know what to do with MODE_SELECT (probably unsurprising given that it's a usb bridge). the SATA disk should respond to the ATA command SET FEATURES, though. Presuming we can get it through the bridge. You can try it with hdparm -W 0 dev optionally with --prefer_ata_12 to do the 12 instead of 16 byte encapsulation and see if that makes a difference. James -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
Please test it now. I'd like to know if it fixes your problem, regardless of how the discussion goes. Seems to be working, I also attached the kernel log. The flush failed will grow with disc activity (but that was to be expected). Jun 22 23:24:50 m3800 kernel: usb 1-3: new high-speed USB device number 6 using xhci_hcd Jun 22 23:24:51 m3800 kernel: usb 1-3: New USB device found, idVendor=04cf, idProduct=8818 Jun 22 23:24:51 m3800 kernel: usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jun 22 23:24:51 m3800 kernel: usb 1-3: Product: USB Mass Storage Device Jun 22 23:24:51 m3800 kernel: usb 1-3: Manufacturer: Myson Century, Inc. Jun 22 23:24:51 m3800 kernel: usb 1-3: SerialNumber: 100 Jun 22 23:24:51 m3800 kernel: usb-storage 1-3:1.0: USB Mass Storage device detected Jun 22 23:24:51 m3800 kernel: scsi host6: usb-storage 1-3:1.0 Jun 22 23:24:51 m3800 kernel: usbcore: registered new interface driver usb-storage Jun 22 23:24:52 m3800 kernel: scsi 6:0:0:0: Direct-Access SAMSUNG MP0804H UE10 PQ: 0 ANSI: 0 CCS Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: Attached scsi generic sg1 type 0 Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] 156368016 512-byte logical blocks: (80.0 GB/74.5 GiB) Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Write Protect is off Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Mode Sense: 00 14 00 00 Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA Jun 22 23:24:52 m3800 kernel: sdb: sdb1 sdb2 Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Attached SCSI disk Jun 22 23:24:52 m3800 kernel: BTRFS: device label USB_BILDER devid 1 transid 9 /dev/sdb2 Jun 22 23:24:52 m3800 kernel: BTRFS info (device sdb2): disk space caching is enabled Jun 22 23:24:52 m3800 kernel: BTRFS: has skinny extents Jun 22 23:24:52 m3800 kernel: FAT-fs (sdb1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck. Jun 22 23:25:28 m3800 kernel: sdb: flush failed: data integrity problem Jun 22 23:25:28 m3800 kernel: sdb: flush failed: data integrity problem Jun 22 23:26:55 m3800 kernel: sdb: flush failed: data integrity problem Jun 22 23:26:55 m3800 kernel: sdb: flush failed: data integrity problem -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] USB: storage: add no SYNCHRONIZE CACHE quirk
On Mon, 22 Jun 2015, James Bottomley wrote: On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: On Mon, 22 Jun 2015, James Bottomley wrote: I'm not sure I entirely like this: we are back again treating data corruption problems silently. However, I also believe treating a single flush failure as a critical filesystem error is also wrong: The data's all there correctly; all it does is introduce a potential window were the FS could get corrupted in the unlikely event the system crashed. Obviously, for a disk with a writeback cache that can't do flush, that window is much wider and the real solution should be to try to switch the cache to write through. I agree. Doing the switch manually (by writing to the cache_type attribute file) works, but it's a nuisance to do this when you have a portable USB drive that gets moved among a bunch of machines. Perhaps it might be wise to do this to every USB device ... for external devices, the small performance gain doesn't really make up for the potential data loss. How about something like this patch? It transforms FS FLUSH into a log warning from an error but preserves the error on any other path. You'll still get a fairly continuous dump of warnings for one of these devices, though ... do they respond to mode selects turning off the writeback? I would be very surprised if any of those drives support MODE SELECT at all. I assume the cache type attribute file you refer to above is just pretending their cache is write through rather than actually setting it to be so? Yes; I'm referring to cache_type_store() in sd.c, and writing temporary write through, which does not issue a MODE SELECT command. It would be easy enough for people to try leaving out the temporary, but I don't expect it to work. The original IDE device had no way of turning their cache types to write through either, but the manufacturers were eventually convinced of the error of their ways. In this case the stupidity resides in the USB-ATA bridge. You can see the gory details at https://bugzilla.kernel.org/show_bug.cgi?id=89511#c19 Maybe your patch will be acceptable, though. We'll have to hear from Markus and Matt. We'll probably have to take this to fsdevel as well ... they might have opinions about whether the FS wants to be informed about flush failure. Okay. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 0/2] FTDI CBUS GPIO support
On 2015-06-22 19:26, Johan Hovold wrote: On Sun, Jun 21, 2015 at 12:12:55AM +0200, Stefan Agner wrote: snip Bit Bang mode, the pins need to be reprogrammed to I/O mode first (EEPROM). All three modes are supported from userspace by libftdi afaik. Is there a way to retrieve the settings from eeprom and only register the gpio chip based on the configuration? Afaik, there is. Not sure if that is somewhat standardized between the different controllers, will have a look at it. In my use case I would like to use the additional GPIOs to control an embedded board (power off/reset etc.) and use the serial communication alongside. Using libftdi to use the CBUS Bit Bang mode is cumbersome, since libftdi requires to detach the kernel driver to get access to the device. The user needs then to reconnect the serial terminal every time a GPIO has been used. Hence, if any of these modes, I see most value in supporting the CBUS mode through the kernel's gpiolib API. However, since some functions are shared (e.g. set_bitmode to enable the different bit modes), this patchset is does some ground work for the other modes too, in case anybody wants to do further work on them. I agree, the usb-serial driver should only provide access to the four cbus pins if available (and use gpiolib). This patchset currently supports FT232R type of devices and has been tested using a FT232RL device. I think the FT232H (and probably later) types of devices should work too (at least the Table 3.5 in the FT232H data sheet mentions the ACBUS Signal Option I/O mode). However, I don't have such a device to test at my disposal. On the implementation side, I created a distinct GPIO driver in drivers/gpio and create that platform device directly from within the ftdi_sio driver. I understand that the mfd subsystem would be the way to go, however it seems to me quite a big change... At least all USB device IDs would need to be moved to the mfd core device since the mfd device would be registered as a USB driver. I guess the ftdi_sio driver would become a platform device and still live under drivers/usb/serial/...? I just saw that recent discussion by Grant and Linus did not approve this approach...? Using the platform bus -- directly as you do or via MFD -- allows for some (arguably contrived) abstraction but I think we should avoid it nonetheless. USB (serial) does not use it as you already noted, and there's not much to gain from creating a single-cell-mfd child device to the USB interface. Instead, hang the gpio chip directly off the usb interface (not the port), add a new config option, and keep the gpio implementation under drivers/usb/serial (possibly in its own file ftdi_sio-gpio.c). Agreed sounds like a good plan. Will try this approach in v2. Except I don't think hanging it directly to the USB interface is the right thing to do. Looking at the block diagram of FT232R or FT232H, the CBUS pins seem to be part of the UART/FIFO controller. And I think the dual UART FT2232D actually supports controlling the CBUS pins of the two UART controllers individually, at least the block diagram thereof suggests so. Note that your current implementation fails to remove the gpio chip on device disconnect, leaks resources in error paths, and lacks locking for the gpio state. Oh true. I was aware that it is somewhat rough, should probably have sent the patch as RFC... -- Stefan -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 3/3] arm: koelsch: make USB0 perform Host/Function switching
Hello.
On 06/22/2015 08:43 PM, Sergei Shtylyov 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.
Signed-off-by: Phil Edworthy phil.edwor...@renesas.com
---
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..8f394be 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
[...]
@@ -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 = gpio2 4 GPIO_ACTIVE_HIGH;
+renesas,id = gpio5 31 GPIO_ACTIVE_HIGH;
+renesas,vbus = gpio7 24 GPIO_ACTIVE_HIGH;
+renesas,vbus-pwr = gpio7 23 GPIO_ACTIVE_HIGH;
The prop names should end with -gpio or even -gpios, according to the
GPIO bindings.
Oh, and you didn't document the props in the previous patch.
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH] functionfs: Avoid aio locking problem
The functionfs aio logic seems broken. When using functionfs, I was seeing frequent hangs, and enabling spinlock debugging, I got: g_ffs gadget: g_ffs ready ci_hdrc ci_hdrc.0: CI_HDRC_CONTROLLER_RESET_EVENT received BUG: spinlock lockup suspected on CPU#0, adbd/2791 lock: 0xe7764880, .magic: e7764880, .owner: none/-1, .owner_cpu: -407539900 CPU: 0 PID: 2791 Comm: adbd Not tainted 4.1.0-rc1-00032-g359b12f #147 Hardware name: Qualcomm (Flattened Device Tree) [c0216ac8] (unwind_backtrace) from [c02136a8] (show_stack+0x10/0x14) [c02136a8] (show_stack) from [c075d9fc] (dump_stack+0x70/0xbc) [c075d9fc] (dump_stack) from [c026ef90] (do_raw_spin_lock+0x114/0x1a0) [c026ef90] (do_raw_spin_lock) from [c0764cb8] (_raw_spin_lock_irqsave+0x50/0x5c) [c0764cb8] (_raw_spin_lock_irqsave) from [c037c1a0] (kiocb_set_cancel_fn+0x1c/0x60) [c037c1a0] (kiocb_set_cancel_fn) from [c05ae568] (ffs_epfile_read_iter+0x8c/0x140) [c05ae568] (ffs_epfile_read_iter) from [c0332018] (__vfs_read+0xb0/0xd4) [c0332018] (__vfs_read) from [c0332ef8] (vfs_read+0x7c/0x100) [c0332ef8] (vfs_read) from [c0332fbc] (SyS_read+0x40/0x8c) [c0332fbc] (SyS_read) from [c020ff20] (ret_fast_syscall+0x0/0x4c) INFO: rcu_preempt detected stalls on CPUs/tasks: 0: (1 GPs behind) idle=805/140/0 softirq=7187/7189 fqs=2601 (detected by 3, t=2603 jiffies, g=3028, c=3027, q=474) Task dump for CPU 0: adbdR running 0 2791 1 0x0002 [c075f234] (__schedule) from [] (0x) Looking at the code, the __vfs_read() calls new_sync_read(), which allocates a struct kiocb kiocb on the stack and passes it to the ffs_epfile_read_iter() funciton. That then calls kiocb_set_cancel_fn() passing a pointer to that kiocb. However, kiocb_set_cancel_fn() assumes the kiocb is a sub-element of a struct aio_kiocb, and it tries to grab the kioctx from that parent structure. However it seems there is no aio_kiocb structure here, so the spin_lock_irqsave hangs trying to lock random data on the stack. This patch avoids the issue, by only calling kiocb_set_cancel_fn if the aio flag is set. Cc: Felipe Balbi ba...@ti.com Cc: Al Viro v...@zeniv.linux.org.uk Cc: Andrzej Pietrasiewicz andrze...@samsung.com Cc: Krzysztof Opasiak k.opas...@samsung.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Michal Nazarewicz min...@mina86.com Cc: Robert Baldyga r.bald...@samsung.com Cc: linux-usb@vger.kernel.org Signed-off-by: John Stultz john.stu...@linaro.org --- drivers/usb/gadget/function/f_fs.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 3507f88..d2434c9 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -924,7 +924,8 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, struct iov_iter *from) kiocb-private = p; - kiocb_set_cancel_fn(kiocb, ffs_aio_cancel); + if (p-aio) + kiocb_set_cancel_fn(kiocb, ffs_aio_cancel); res = ffs_epfile_io(kiocb-ki_filp, p); if (res == -EIOCBQUEUED) @@ -968,7 +969,8 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to) kiocb-private = p; - kiocb_set_cancel_fn(kiocb, ffs_aio_cancel); + if (p-aio) + kiocb_set_cancel_fn(kiocb, ffs_aio_cancel); res = ffs_epfile_io(kiocb-ki_filp, p); if (res == -EIOCBQUEUED) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH] usb: ci_hdrc_imx: add optional hub clock
On Mon, Jun 22, 2015 at 12:54:14AM +0800, Maciej S. Szmigiero wrote:
This patch adds ability to define optional clock of connected
USB hub to ChipIdea i.MX usb controller driver.
This is needed for example for UDOO board.
Previously, this board DT file used a fact that non-core registers
of this USB controller have a separate driver (usbmisc_imx) which
did allow defining a separate clock.
Because the non-core registers are in fact using the same clock as
main controller this allowed to use one of such clock definitions
to enable USB hub clock instead.
However, since commit 73dea4a912b2
(usb: chipidea: usbmisc_imx: delete clock information) this
possibility no longer exists and loading USB support on this board
results in a hard SoC lockup.
Note that this is not specific to particular USB hub chip used,
rather than to a particular board.
Since this is a DT-based board there is no board platform file to
put such clock enable.
Also, USB bus devices aren't instantiated in DT file since it is an
enumerable bus.
NOP PHY also can't be used as clock consumer since this
USB controller needs a true MXS phy definition, which accepts only
one clock (different from USB controller one).
Based on a patch previously submitted by Fabio Estevam, with consent.
Signed-off-by: Maciej Szmigiero m...@maciej.szmigiero.name
diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
b/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
index 38a5480..fc65f1c 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-imx.txt
@@ -5,6 +5,8 @@ Required properties:
- reg: Should contain registers location and length
- interrupts: Should contain controller interrupt
- fsl,usbphy: phandle of usb phy that connects to the port
+- clocks: phandles of clocks that drive the controller and optionally
+ an USB hub connected to it
Recommended properies:
- phy_type: the type of the phy connected to the core. Should be one
@@ -20,12 +22,14 @@ Optional properties:
- external-vbus-divider: enables off-chip resistor divider for Vbus
- maximum-speed: limit the maximum connection speed to full-speed.
- tpl-support: TPL (Targeted Peripheral List) feature for targeted hosts
+- clock-names: must be default, hub if optional USB hub clock is used
Examples:
usb@02184000 { /* USB OTG */
compatible = fsl,imx6q-usb, fsl,imx27-usb;
reg = 0x02184000 0x200;
interrupts = 0 43 0x04;
+ clocks = clks IMX6QDL_CLK_USBOH3;
fsl,usbphy = usbphy1;
fsl,usbmisc = usbmisc 0;
disable-over-current;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 389f0e0..bb7f859 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -65,6 +65,7 @@ struct ci_hdrc_imx_data {
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
+ struct clk *clk_hub;
struct imx_usbmisc_data *usbmisc_data;
bool supports_runtime_pm;
bool in_lpm;
@@ -196,6 +197,16 @@ static int ci_hdrc_imx_probe(struct platform_device
*pdev)
goto disable_device;
}
+ data-clk_hub = devm_clk_get(pdev-dev, hub);
+ if (!IS_ERR(data-clk_hub)) {
+ ret = clk_prepare_enable(data-clk_hub);
+ if (ret) {
+ dev_err(pdev-dev,
+ Failed to enable clk_hub: %d\n, ret);
+ goto disable_device;
+ }
+ }
+
Hi Maciej,
As I said before, the USB HUB is just an USB peripheral, we should
not put a peripheral's clock information to controller driver, I think
hub driver should take responsibilities for it, just like other usb
pheripheral drivers, like usb modem, etc. You can talk it with Alan
Stern about it.
platform_set_drvdata(pdev, data);
if (data-supports_runtime_pm) {
@@ -223,6 +234,8 @@ static int ci_hdrc_imx_remove(struct platform_device
*pdev)
pm_runtime_disable(pdev-dev);
pm_runtime_put_noidle(pdev-dev);
}
+ if (!IS_ERR(data-clk_hub))
+ clk_disable_unprepare(data-clk_hub);
ci_hdrc_remove_device(data-ci_pdev);
clk_disable_unprepare(data-clk);
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH] USB: atm: cxacru: fix blank line after declaration
Fixed a coding style issue. Adds blank lines after declarations.
Signed-off-by: Aaron Raimist aaronraim...@riseup.net
---
drivers/usb/atm/cxacru.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index 813d4d3..1173f9c 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -270,6 +270,7 @@ static ssize_t cxacru_sysfs_showattr_dB(s16 value, char
*buf)
static ssize_t cxacru_sysfs_showattr_bool(u32 value, char *buf)
{
static char *str[] = { no, yes };
+
if (unlikely(value = ARRAY_SIZE(str)))
return snprintf(buf, PAGE_SIZE, %u\n, value);
return snprintf(buf, PAGE_SIZE, %s\n, str[value]);
@@ -278,6 +279,7 @@ static ssize_t cxacru_sysfs_showattr_bool(u32 value, char
*buf)
static ssize_t cxacru_sysfs_showattr_LINK(u32 value, char *buf)
{
static char *str[] = { NULL, not connected, connected, lost };
+
if (unlikely(value = ARRAY_SIZE(str) || str[value] == NULL))
return snprintf(buf, PAGE_SIZE, %u\n, value);
return snprintf(buf, PAGE_SIZE, %s\n, str[value]);
@@ -702,6 +704,7 @@ static int cxacru_cm_get_array(struct cxacru_data
*instance, enum cxacru_cm_requ
len = ret / 4;
for (offb = 0; offb len; ) {
int l = le32_to_cpu(buf[offb++]);
+
if (l 0 || l stride || l (len - offb) / 2) {
if (printk_ratelimit())
usb_err(instance-usbatm, invalid data length
from cm %#x: %d\n,
@@ -732,6 +735,7 @@ cleanup:
static int cxacru_card_status(struct cxacru_data *instance)
{
int ret = cxacru_cm(instance, CM_REQUEST_CARD_GET_STATUS, NULL, 0,
NULL, 0);
+
if (ret 0) { /* firmware not loaded */
usb_dbg(instance-usbatm, cxacru_adsl_start: CARD_GET_STATUS
returned %d\n, ret);
return ret;
@@ -945,6 +949,7 @@ static int cxacru_fw(struct usb_device *usb_dev, enum
cxacru_fw_request fw,
offb = offd = 0;
do {
int l = min_t(int, stride, size - offd);
+
buf[offb++] = fw;
buf[offb++] = l;
buf[offb++] = code1;
@@ -1091,8 +1096,8 @@ static int cxacru_heavy_init(struct usbatm_data
*usbatm_instance,
{
const struct firmware *fw, *bp;
struct cxacru_data *instance = usbatm_instance-driver_data;
-
int ret = cxacru_find_firmware(instance, fw, fw);
+
if (ret) {
usb_warn(usbatm_instance, firmware (cxacru-fw.bin) unavailable
(system misconfigured?)\n);
return ret;
--
2.4.3
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On 06/20/2015 12:18 AM, Michal Nazarewicz wrote:
On Fri, Jun 19 2015, Felipe Balbi wrote:
the fact is that this needs to be configurable from configfs. If user
sets up a single lun, then this should be as you said, however, if 2
luns are configured, I still want to have those 2 working.
From a user perspective, configfs should support N luns just fine as
long as we have enough memory available.
Yes, sorry, you’re right. I didn’t fallow closely migration to configfs
based configuration and didn’t have the full picture.
Well, not exactly. MS spec says that there should be no more that 16 LUNs.
After taking another look, I think the best solution is to have a loop
in fsg_alloc, something like the following (beware that this has not
been tested in any way):
From 19ec2796009f8650c7db4358f7e16931ba96e4ee 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
---
drivers/usb/gadget/function/f_mass_storage.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..8e26a12 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3563,14 +3563,28 @@ 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 if (nluns != common-nluns) {
+ pr_info(Adjusting number of LUNs=%u (was %u)\n,
+ nluns, common-nluns);
+ common-nluns = nluns;
+ }
+ }
opts-refcnt++;
mutex_unlock(opts-lock);
+
fsg-function.name = FSG_DRIVER_DESC;
fsg-function.bind = fsg_bind;
fsg-function.unbind = fsg_unbind;
Looks simple but we will still get comment number of LUNs=8. Moreover
it may cause some trouble if we set nluns == 0, because in create lun we
check whether nluns 0. In addition if we use for example
fsg_common_set_nluns(1) or anything other what FSG_MAX_LUNS this loop
will read past allocated buffer.
I have prepared a series which fix several smaller issues and also this one:
http://article.gmane.org/gmane.linux.usb.general/127209
Best regards,
--
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 2/5] usb: gadget: mass_storage: Enforce contiguous LUN ids
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 don't allow to bind ms function unless we have at least LUN0
and LUNs ids are contiguous.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
drivers/usb/gadget/function/f_mass_storage.c | 29 ++
1 file changed, 29 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 2e8f37e..19b31d7 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3065,6 +3065,35 @@ static int fsg_bind(struct usb_configuration *c, struct
usb_function *f)
int ret;
struct fsg_opts *opts;
+ /*
+* Don't allow to bind if we don't have at least one LUN
+* or LUNs ids are not contiguous.
+*/
+ if (likely(common-luns)) {
+ bool found_null = false;
+
+ for (i = 0; i common-nluns; ++i) {
+ if (!common-luns[i]) {
+ found_null = true;
+ continue;
+ }
+
+ if (!found_null) {
+ continue;
+ } else {
+ pr_err(LUN ids should be contiguous.\n);
+ return -EINVAL;
+ }
+ }
+
+ if (i == 0 || !common-luns[i]) {
+ pr_err(There should be at least one LUN.\n);
+ return -EINVAL;
+ }
+ } else {
+ return -EINVAL;
+ }
+
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
[PATCH 0/5] Mass storage fixes and improvements
Hello, This series fix a few bugs in mass storage function and disallows to bind function with not contiguous LUN ids. Last patch in this series fix GET_MAX_LUNS request to return actual number of luns, not the number of prealocated space. 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: Enforce contiguous LUN ids 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: Send correct number of LUNs to host drivers/usb/gadget/function/f_mass_storage.c | 72 -- drivers/usb/gadget/function/f_mass_storage.h |2 +- 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, 67 insertions(+), 27 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH 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 Cc: sta...@vger.kernel.org # 3.13+ --- 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
[PATCH 3/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
---
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 19b31d7..4257238 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
[PATCH 5/5] usb: gadget: mass_storage: Send correct number of LUNs to host
GET_MAX_LUN request should return number of realy created LUNs
not the size of preallocated array.
This patch changes fsg_common_set_nluns() to fsg_common_set_max_luns()
which now only allocates an empty array for luns and set nluns
to 0. While LUNS are create we simply count them and store result
in nluns which is send later to host.
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 | 69 ++
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, 48 insertions(+), 41 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 4257238..7e37070 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -280,6 +280,7 @@ struct fsg_common {
u8 cmnd[MAX_COMMAND_SIZE];
unsigned intnluns;
+ unsigned intmax_luns;
unsigned intlun;
struct fsg_lun **luns;
struct fsg_lun *curlun;
@@ -2131,7 +2132,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-nluns || 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,
@@ -2411,8 +2412,7 @@ static void handle_exception(struct fsg_common *common)
else {
for (i = 0; i common-nluns; ++i) {
curlun = common-luns[i];
- if (!curlun)
- continue;
+ WARN_ON(!curlun);
curlun-prevent_medium_removal = 0;
curlun-sense_data = SS_NO_SENSE;
curlun-unit_attention_data = SS_NO_SENSE;
@@ -2558,7 +2558,9 @@ static int fsg_main_thread(void *common_)
down_write(common-filesem);
for (; i--; ++curlun_it) {
struct fsg_lun *curlun = *curlun_it;
- if (!curlun || !fsg_lun_is_open(curlun))
+
+ WARN_ON(!curlun);
+ if (!fsg_lun_is_open(curlun))
continue;
fsg_lun_close(curlun);
@@ -2759,6 +2761,8 @@ static void _fsg_common_remove_luns(struct fsg_common
*common, int n)
if (common-luns[i]) {
fsg_common_remove_lun(common-luns[i], common-sysfs);
common-luns[i] = NULL;
+ WARN_ON(common-nluns == 0);
+ common-nluns--;
}
}
@@ -2773,20 +2777,22 @@ void fsg_common_free_luns(struct fsg_common *common)
fsg_common_remove_luns(common);
kfree(common-luns);
common-luns = NULL;
+ common-max_luns = 0;
+ common-nluns = 0;
}
EXPORT_SYMBOL_GPL(fsg_common_free_luns);
-int fsg_common_set_nluns(struct fsg_common *common, int nluns)
+int fsg_common_set_max_luns(struct fsg_common *common, int max_luns)
{
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);
+ if (max_luns 1 || max_luns FSG_MAX_LUNS) {
+ pr_err(invalid number of LUNs: %u\n, max_luns);
return -EINVAL;
}
- curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL);
+ curlun = kcalloc(max_luns, sizeof(*curlun), GFP_KERNEL);
if (unlikely(!curlun))
return -ENOMEM;
@@ -2794,13 +2800,15 @@ int fsg_common_set_nluns(struct fsg_common *common, int
nluns)
fsg_common_free_luns(common);
common-luns = curlun;
- common-nluns = nluns;
+ common-max_luns = max_luns;
+ common-nluns = 0;
- pr_info(Number of LUNs=%d\n, common-nluns);
+ pr_info(Number of LUNs=%d, max number of LUNs=%d\n,
+ common-nluns, common-max_luns);
return 0;
}
-EXPORT_SYMBOL_GPL(fsg_common_set_nluns);
+EXPORT_SYMBOL_GPL(fsg_common_set_max_luns);
void fsg_common_set_ops(struct fsg_common *common,
const struct fsg_operations *ops)
@@ -2882,7 +2890,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-max_luns || !common-luns)
[PATCH 4/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
---
drivers/usb/gadget/function/storage_common.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Mon, 22 Jun 2015 18:45:37 +0800
Li Jun b47...@freescale.com wrote:
On Mon, Jun 22, 2015 at 12:41:22PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 21:37:04 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 03:07:48PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that
it appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct
usb_otg_caps *otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
Right. Plus controller drivers that are not updated to use these
otg_caps
are also legacy.
Did not understand the Plus case.
Some of 4 properties is present, but its driver dose not use otg_caps?
yes that is what I meant.
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
I didn't understand this point. If a controller driver is not updated
to use otg_caps it is legacy and gadget-otg_caps will be NULL.
Some of existing chipdea platforms work fine on HNP and SRP , which did
not use any new flags and properties, after my patch, those platform
should
still enable HNP and SRP without any DT change.
Agreed.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
If controller drivers don't support OTG. gadget-is_otg and
gadget-otg_caps will not be set by them and they don't have to use
of_usb_set_otg_caps().
But after my patch, some time later, this driver adds OTG functions on
it new platform, it should disable any OTG feature for its legacy
platforms (none of properties is passed).
That can be decided in of_usb_update_otg_caps().
Controller sets whatever it supports in otg_caps.
of_usb_updade_otg_caps() checks if it is legacy DT (i.e. no otg-rev) and
then
overrides to legacy otg_caps.
So I didn't understand why the decision can't be taken here
for non-legacy controller drivers with legacy DT.
They will set gadget-otg_caps and gadget-is_otg.
If none of the 4 DT flags are passed then we know it is legacy DT
and we can limit to legacy behaviour.
legacy behaviour number is 2, not only one legacy behaviour.
That's why I leave the otg caps decided by controller drivers.
I'm only suggesting that it be decided at one place i.e. in
of_usb_updade_otg_caps() instead of every controller.
Do you see any problems with that approach?
The problem is the override policy for legacy platforms.
For case 3) above, we should enable HNP and SRP,
case 3 is controller is supporting new flags but DT is not and we want
legacy OTG enabled, right?
Can't we detect if none of the new otg flags are present then it is legacy DT
so use legacy OTG mode?
For case 4) above, after add OTG HNPSRP support, it should disable HNP and
SRP.
case 4 is controller was not supporting OTG at all before and now is updated to
support
OTG. But for such cases isn't dr_mode = peripheral in the DT?
one example is dwc3 controller.
If the dr_mode was otg for such case and we want OTG disabled then it is
really the DT fault.
We don't need to tackle this case. Just fix up the DT to dr_mode = peripheral
if
OTG behaviour is not needed.
How I make one decision in of_usb_updade_otg_caps()
for above 2 cases?(the otg-rev is 0 for both).
For case 3. otg-rev passed by controller is not 0. otg-rev is just not present
in DT.
cheers,
-roger
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Mon, 22 Jun 2015 18:56:18 +0800
Li Jun b47...@freescale.com wrote:
On Mon, Jun 22, 2015 at 12:43:37PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that it
appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct usb_otg_caps
*otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
OK this was our difference in understanding. I was thinking that for non
legacy code otg-rev _must_ be passed. without otg-rev the disable flags
will be ignored. It makes life much easier no?
I have to consider ID pint detect case, this is a most common usage,
after controller driver use otg_caps and enable OTG HNP/SRP, some
platform still just want ID pin detect, no more otg feature required,
it need disable OTG HNP/SRP/ADP, the dt should be:
dr_mode = otg;
adp-disable;
hnp-disable;
srp-didable;
in this case, we cannot require DT user still pass a otg-rev, right?
Right. Although I'm beginning to think if we should add drd-only flag
to explicitly state DRD feature as it might be more common than OTG.
But for current patches I think we can safely assume that
if the 3 flags are not passed and otg-rev is not passed then it is
legacy DT requiring OTG with HNP/SRP.
why do you want otg-rev to be optional for non-legacy DT?
For above ID pin detect case.
Got it.
cheers,
-roger
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On Mon, Jun 22 2015, David Fisher wrote:
This works for ok for me.
Can we also remove the misleading (too early) instances of :
pr_info(Number of LUNs=%d\n, common-nluns);
in fsg_common_set_luns() and fsg_common_create_lun()
I’m assuming you mean fsg_common_set_nluns and fsg_common_create_luns,
right?
The pr_info should remain in at least one of those; otherwise legacy
gadgets would not get the information printed. I guess print in the
former may be removed since all legacy gadgets call the latter soon
afterwards. Then of course, ‘adjusting’ would no longer be valid so the
end commit would look something like:
From 3f949477f3060b3522e1f0c41f34b0ff9de96e10 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
---
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;
--
2.4.3.573.g4eafbef
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
On 06/20/2015 12:18 AM, Michal Nazarewicz wrote:
On Fri, Jun 19 2015, Felipe Balbi wrote:
the fact is that this needs to be configurable from configfs. If user
sets up a single lun, then this should be as you said, however, if 2
luns are configured, I still want to have those 2 working.
From a user perspective, configfs should support N luns just fine as
long as we have enough memory available.
Yes, sorry, you’re right. I didn’t fallow closely migration to configfs
based configuration and didn’t have the full picture.
Well, not exactly. MS spec says that there should be no more that 16 LUNs.
Yeah. fsg_lun_make limits LUN to FSG_MAX_LUNS which is eight so we’re
fine.
After taking another look, I think the best solution is to have a loop
in fsg_alloc, something like the following (beware that this has not
been tested in any way):
From 19ec2796009f8650c7db4358f7e16931ba96e4ee 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
---
drivers/usb/gadget/function/f_mass_storage.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..8e26a12 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3563,14 +3563,28 @@ 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 if (nluns != common-nluns) {
+pr_info(Adjusting number of LUNs=%u (was %u)\n,
+nluns, common-nluns);
+common-nluns = nluns;
+}
+}
opts-refcnt++;
mutex_unlock(opts-lock);
+
fsg-function.name = FSG_DRIVER_DESC;
fsg-function.bind = fsg_bind;
fsg-function.unbind= fsg_unbind;
Looks simple but we will still get comment number of LUNs=8. Moreover
it may cause some trouble if we set nluns == 0,
Which is why the code leaves common-nluns == 8 if nluns == 0.
because in create lun we check whether nluns 0. In addition if we
use for example fsg_common_set_nluns(1) or anything other what
FSG_MAX_LUNS this loop will read past allocated buffer.
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.
I have prepared a series which fix several smaller issues and also this one:
http://article.gmane.org/gmane.linux.usb.general/127209
Best regards,
--
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
--
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
Re: [PATCH 1/5] usb: gadget: mass_storage: Free buffers if create lun fails
On Mon, Jun 22 2015, 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 Cc: sta...@vger.kernel.org # 3.13+ 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 -- 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
Re: [PATCH 3/5] usb: gadget: mass_storage: Place EXPORT_SYMBOL_GPL() after func definition
On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
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 19b31d7..4257238 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
--
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
Re: optimal io size / custom alignment
On Sun, 21 Jun 2015, Tom Yan wrote: I was not saying RAIDs are virtual devices. I just mentioned it because I saw things like virtio-blk or zram use blk_queue_io_opt(). I know they all use VPDs, but the main point is whether those hardware RAIDs or so are handled by sd_mod, and whether those transfer lengths info are still important when it's just a simple drive. To me they look like to be of different nature. That's why I think it's inappropraite that they use the same variable / file to report because that makes tools like fdisk have trouble determining when does those values really matters. In fact, (maybe I am just unlucky :P) VPDs of all my devices are to some extent broken. I just found out today my Intel 530 SSD connecting directly to SATA also reports totally garbage values for TRIM : ( To be honest the UAS thing doesn't really affect me a lot, I mostly use gdisk now (which doesn't care about i/o size AFAIK). I can also disable uas with the quirk so that VPDs are skipped when I really need fdisk for msdos/mbr. It's just I think that it kind of reveal a problem that has to be dealt with sooner or later, though you can optimistically think that vendors would do better on VPDs in the future. Regardless of all these issues, it is clear that a lot of devices don't implement the VPD data correctly. Therefore the information in the kernel will often be wrong. And consequently, fdisk needs to offer the user an option to override the default partition-alignment setting. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in
[PATCH v2] usb: class: Use USB_CLASS_PRINTER instead of number 7
Kernel provides very nice defines for USB device class
so it's a good idea to use them in suitable places.
It is much easier to grep for such define instead of 7.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
Changes since v1:
- Fix also all other occurences
drivers/usb/class/usblp.c | 66 -
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 0924ee4..2786b83 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -57,6 +57,7 @@
#include linux/mutex.h
#undef DEBUG
#include linux/usb.h
+#include linux/usb/ch9.h
#include linux/ratelimit.h
/*
@@ -79,12 +80,20 @@
#define IOCNR_SOFT_RESET 7
/* Get device_id string: */
#define LPIOC_GET_DEVICE_ID(len) _IOC(_IOC_READ, 'P', IOCNR_GET_DEVICE_ID, len)
-/* The following ioctls were added for http://hpoj.sourceforge.net: */
-/* Get two-int array:
- * [0]=current protocol (1=7/1/1, 2=7/1/2, 3=7/1/3),
- * [1]=supported protocol mask (mask(1n)!=0 means 7/1/n supported): */
+/* The following ioctls were added for http://hpoj.sourceforge.net:
+ * Get two-int array:
+ * [0]=current protocol
+ * (1=USB_CLASS_PRINTER/1/1, 2=USB_CLASS_PRINTER/1/2,
+ * 3=USB_CLASS_PRINTER/1/3),
+ * [1]=supported protocol mask (mask(1n)!=0 means
+ * USB_CLASS_PRINTER/1/n supported):
+ */
#define LPIOC_GET_PROTOCOLS(len) _IOC(_IOC_READ, 'P', IOCNR_GET_PROTOCOLS, len)
-/* Set protocol (arg: 1=7/1/1, 2=7/1/2, 3=7/1/3): */
+/*
+ * Set protocol
+ * (arg: 1=USB_CLASS_PRINTER/1/1, 2=USB_CLASS_PRINTER/1/2,
+ * 3=USB_CLASS_PRINTER/1/3):
+ */
#define LPIOC_SET_PROTOCOL _IOC(_IOC_WRITE, 'P', IOCNR_SET_PROTOCOL, 0)
/* Set channel number (HP Vendor-specific command): */
#define LPIOC_HP_SET_CHANNEL _IOC(_IOC_WRITE, 'P', IOCNR_HP_SET_CHANNEL, 0)
@@ -146,8 +155,10 @@ struct usblp {
int readcount; /* Counter for reads */
int ifnum; /* Interface number */
struct usb_interface*intf; /* The interface */
- /* Alternate-setting numbers and endpoints for each protocol
-* (7/1/{index=1,2,3}) that the device supports: */
+ /*
+* Alternate-setting numbers and endpoints for each protocol
+* (USB_CLASS_PRINTER/1/{index=1,2,3}) that the device supports:
+*/
struct {
int alt_setting;
struct usb_endpoint_descriptor *epwrite;
@@ -1201,19 +1212,23 @@ abort_ret:
* but our requirements are too intricate for simple match to handle.
*
* The proto_bias option may be used to specify the preferred protocol
- * for all USB printers (1=7/1/1, 2=7/1/2, 3=7/1/3). If the device
- * supports the preferred protocol, then we bind to it.
+ * for all USB printers (1=USB_CLASS_PRINTER/1/1, 2=USB_CLASS_PRINTER/1/2,
+ * 3=USB_CLASS_PRINTER/1/3). If the device supports the preferred protocol,
+ * then we bind to it.
*
- * The best interface for us is 7/1/2, because it is compatible
- * with a stream of characters. If we find it, we bind to it.
+ * The best interface for us is USB_CLASS_PRINTER/1/2, because it
+ * is compatible with a stream of characters. If we find it, we bind to it.
*
* Note that the people from hpoj.sourceforge.net need to be able to
- * bind to 7/1/3 (MLC/1284.4), so we provide them ioctls for this purpose.
+ * bind to USB_CLASS_PRINTER/1/3 (MLC/1284.4), so we provide them ioctls
+ * for this purpose.
*
- * Failing 7/1/2, we look for 7/1/3, even though it's probably not
- * stream-compatible, because this matches the behaviour of the old code.
+ * Failing USB_CLASS_PRINTER/1/2, we look for USB_CLASS_PRINTER/1/3,
+ * even though it's probably not stream-compatible, because this matches
+ * the behaviour of the old code.
*
- * If nothing else, we bind to 7/1/1 - the unidirectional interface.
+ * If nothing else, we bind to USB_CLASS_PRINTER/1/1
+ * - the unidirectional interface.
*/
static int usblp_select_alts(struct usblp *usblp)
{
@@ -1231,7 +1246,8 @@ static int usblp_select_alts(struct usblp *usblp)
for (i = 0; i if_alt-num_altsetting; i++) {
ifd = if_alt-altsetting[i];
- if (ifd-desc.bInterfaceClass != 7 ||
ifd-desc.bInterfaceSubClass != 1)
+ if (ifd-desc.bInterfaceClass != USB_CLASS_PRINTER ||
+ ifd-desc.bInterfaceSubClass != 1)
if (!(usblp-quirks USBLP_QUIRK_BAD_CLASS))
continue;
@@ -1257,8 +1273,10 @@ static int usblp_select_alts(struct usblp *usblp)
if (!epwrite || (ifd-desc.bInterfaceProtocol 1 !epread))
continue;
- /* Turn off reads for 7/1/1 (unidirectional) interfaces
-* and buggy bidirectional printers. */
+ /*
+* Turn off reads
Re: Configfs usb mass_storage device always reports 8 LUNs ?
On 06/22/2015 04:24 PM, Michal Nazarewicz wrote:
On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
On 06/20/2015 12:18 AM, Michal Nazarewicz wrote:
On Fri, Jun 19 2015, Felipe Balbi wrote:
the fact is that this needs to be configurable from configfs. If user
sets up a single lun, then this should be as you said, however, if 2
luns are configured, I still want to have those 2 working.
From a user perspective, configfs should support N luns just fine as
long as we have enough memory available.
Yes, sorry, you’re right. I didn’t fallow closely migration to configfs
based configuration and didn’t have the full picture.
Well, not exactly. MS spec says that there should be no more that 16 LUNs.
Yeah. fsg_lun_make limits LUN to FSG_MAX_LUNS which is eight so we’re
fine.
After taking another look, I think the best solution is to have a loop
in fsg_alloc, something like the following (beware that this has not
been tested in any way):
From 19ec2796009f8650c7db4358f7e16931ba96e4ee 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
---
drivers/usb/gadget/function/f_mass_storage.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 3cc109f..8e26a12 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3563,14 +3563,28 @@ 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 if (nluns != common-nluns) {
+ pr_info(Adjusting number of LUNs=%u (was %u)\n,
+ nluns, common-nluns);
+ common-nluns = nluns;
+ }
+ }
opts-refcnt++;
mutex_unlock(opts-lock);
+
fsg-function.name = FSG_DRIVER_DESC;
fsg-function.bind = fsg_bind;
fsg-function.unbind = fsg_unbind;
Looks simple but we will still get comment number of LUNs=8. Moreover
it may cause some trouble if we set nluns == 0,
Which is why the code leaves common-nluns == 8 if nluns == 0.
because in create lun we check whether nluns 0. In addition if we
use for example fsg_common_set_nluns(1) or anything other what
FSG_MAX_LUNS this loop will read past allocated buffer.
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.
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.
BR's
--
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
Re: [PATCH 4/5] usb: gadget: storage-common: Set FSG_MAX_LUNS to 16
On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
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/storage_common.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
--
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
Re: [PATCH 5/5] usb: gadget: mass_storage: Send correct number of LUNs to host
On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
GET_MAX_LUN request should return number of realy created LUNs
not the size of preallocated array.
This patch changes fsg_common_set_nluns() to fsg_common_set_max_luns()
which now only allocates an empty array for luns and set nluns
to 0. While LUNS are create we simply count them and store result
in nluns which is send later to host.
Reported-by: David Fisher david.fish...@synopsys.com
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
At this point I would just change common-luns to be an array rather
than a pointer. This would remove need for max_luns all together.
---
drivers/usb/gadget/function/f_mass_storage.c | 69
++
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, 48 insertions(+), 41 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 4257238..7e37070 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -280,6 +280,7 @@ struct fsg_common {
u8 cmnd[MAX_COMMAND_SIZE];
unsigned intnluns;
+ unsigned intmax_luns;
unsigned intlun;
struct fsg_lun **luns;
struct fsg_lun *curlun;
@@ -2131,7 +2132,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-nluns || 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,
@@ -2411,8 +2412,7 @@ static void handle_exception(struct fsg_common *common)
else {
for (i = 0; i common-nluns; ++i) {
curlun = common-luns[i];
- if (!curlun)
- continue;
+ WARN_ON(!curlun);
curlun-prevent_medium_removal = 0;
curlun-sense_data = SS_NO_SENSE;
curlun-unit_attention_data = SS_NO_SENSE;
@@ -2558,7 +2558,9 @@ static int fsg_main_thread(void *common_)
down_write(common-filesem);
for (; i--; ++curlun_it) {
struct fsg_lun *curlun = *curlun_it;
- if (!curlun || !fsg_lun_is_open(curlun))
+
+ WARN_ON(!curlun);
+ if (!fsg_lun_is_open(curlun))
continue;
fsg_lun_close(curlun);
@@ -2759,6 +2761,8 @@ static void _fsg_common_remove_luns(struct fsg_common
*common, int n)
if (common-luns[i]) {
fsg_common_remove_lun(common-luns[i], common-sysfs);
common-luns[i] = NULL;
+ WARN_ON(common-nluns == 0);
+ common-nluns--;
}
}
@@ -2773,20 +2777,22 @@ void fsg_common_free_luns(struct fsg_common *common)
fsg_common_remove_luns(common);
kfree(common-luns);
common-luns = NULL;
+ common-max_luns = 0;
+ common-nluns = 0;
}
EXPORT_SYMBOL_GPL(fsg_common_free_luns);
-int fsg_common_set_nluns(struct fsg_common *common, int nluns)
+int fsg_common_set_max_luns(struct fsg_common *common, int max_luns)
{
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);
+ if (max_luns 1 || max_luns FSG_MAX_LUNS) {
+ pr_err(invalid number of LUNs: %u\n, max_luns);
return -EINVAL;
}
- curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL);
+ curlun = kcalloc(max_luns, sizeof(*curlun), GFP_KERNEL);
if (unlikely(!curlun))
return -ENOMEM;
@@ -2794,13 +2800,15 @@ int fsg_common_set_nluns(struct fsg_common *common,
int nluns)
fsg_common_free_luns(common);
common-luns = curlun;
- common-nluns = nluns;
+ common-max_luns = max_luns;
+ common-nluns = 0;
- pr_info(Number of LUNs=%d\n, common-nluns);
+ pr_info(Number of LUNs=%d, max number of LUNs=%d\n,
+ common-nluns, common-max_luns);
return 0;
}
-EXPORT_SYMBOL_GPL(fsg_common_set_nluns);
+EXPORT_SYMBOL_GPL(fsg_common_set_max_luns);
void fsg_common_set_ops(struct fsg_common *common,
const struct fsg_operations *ops)
@@ -2882,7 +2890,7 @@ int fsg_common_create_lun(struct fsg_common
Re: [PATCH v6] usb: common: add API to set usb otg capabilities by device tree
On Mon, Jun 22, 2015 at 04:32:56PM +0300, Roger Quadros wrote:
On Mon, 22 Jun 2015 18:45:37 +0800
Li Jun b47...@freescale.com wrote:
On Mon, Jun 22, 2015 at 12:41:22PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 21:37:04 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 03:07:48PM +0300, Roger Quadros wrote:
On Thu, 18 Jun 2015 16:47:48 +0800
Li Jun b47...@freescale.com wrote:
On Thu, Jun 18, 2015 at 10:36:50AM +0300, Roger Quadros wrote:
Lin,
You can use --in-reply-to message id of v5 of this path so that
it appears together
with the other patches in peoples mailboxes.
+ * the passed properties in DT.
+ * @np: Pointer to the given device_node
+ * @otg_caps: Pointer to the target usb_otg_caps to be set
+ *
+ * The function gets and sets the otg capabilities
+ */
+void of_usb_set_otg_caps(struct device_node *np, struct
usb_otg_caps *otg_caps)
+{
+ u32 otg_rev;
+
+ if (!otg_caps)
+ return;
+
+ if (!of_property_read_u32(np, otg-rev, otg_rev))
+ otg_caps-otg_rev = otg_rev;
should we check if otg_rev is in correct format?
anything non BCD and greater than 0x is invalid.
Also if otg-rev is not passed then we need to treat it as legacy
platform right? How is this taken care of?
Missed this comment
This handling rely on controller driver, cannot decided here.
There are several cases we need take care:
1) otg-rev is not passed, but all 3 disable flags passed, this is
valid, means user want to disable whole OTG, so only otg-rev
not passed, cannot treat as legacy platform.
2) Legacy platform means: none of 4 properties is present.
Right. Plus controller drivers that are not updated to use these
otg_caps
are also legacy.
Did not understand the Plus case.
Some of 4 properties is present, but its driver dose not use otg_caps?
yes that is what I meant.
3) Some controller drivers already support OTG HNP/SRP, then change
to utilize those new flags, still should support OTG HNP/SRP w/o
any dt change, so OTG caps should be enabled for legacy platforms.
I didn't understand this point. If a controller driver is not updated
to use otg_caps it is legacy and gadget-otg_caps will be NULL.
Some of existing chipdea platforms work fine on HNP and SRP , which did
not use any new flags and properties, after my patch, those platform
should
still enable HNP and SRP without any DT change.
Agreed.
4) Some controller drivers does not support any OTG, after add OTG
functions and utilize those new flags, should keep OTG disabled
for legacy platforms.
If controller drivers don't support OTG. gadget-is_otg and
gadget-otg_caps will not be set by them and they don't have to use
of_usb_set_otg_caps().
But after my patch, some time later, this driver adds OTG functions on
it new platform, it should disable any OTG feature for its legacy
platforms (none of properties is passed).
That can be decided in of_usb_update_otg_caps().
Controller sets whatever it supports in otg_caps.
of_usb_updade_otg_caps() checks if it is legacy DT (i.e. no otg-rev) and
then
overrides to legacy otg_caps.
So I didn't understand why the decision can't be taken here
for non-legacy controller drivers with legacy DT.
They will set gadget-otg_caps and gadget-is_otg.
If none of the 4 DT flags are passed then we know it is legacy DT
and we can limit to legacy behaviour.
legacy behaviour number is 2, not only one legacy behaviour.
That's why I leave the otg caps decided by controller drivers.
I'm only suggesting that it be decided at one place i.e. in
of_usb_updade_otg_caps() instead of every controller.
Do you see any problems with that approach?
The problem is the override policy for legacy platforms.
For case 3) above, we should enable HNP and SRP,
case 3 is controller is supporting new flags but DT is not and we want
legacy OTG enabled, right?
Can't we detect if none of the new otg flags are present then it is legacy DT
so use legacy OTG mode?
Yes, we can. The legacy OTG mode is HNPSRP is enabled.
For case 4) above, after add OTG HNPSRP support, it should disable HNP and
SRP.
case 4 is controller was not supporting OTG at all before and now is updated
to support
OTG. But for such cases isn't dr_mode = peripheral in the DT?
one example is dwc3 controller.
If the dr_mode was otg for such case and we want OTG disabled then it is
really the DT fault.
It's ID pin detect for dual role switch as
Re: [PATCH 2/5] usb: gadget: mass_storage: Enforce contiguous LUN ids
On Mon, Jun 22 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 don't allow to bind ms function unless we have at least LUN0
and LUNs ids are contiguous.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
but then again I think that we should let user space shoot themselves in
the foot if they want to, especially as letting them to that is less
code. ;)
---
drivers/usb/gadget/function/f_mass_storage.c | 29
++
1 file changed, 29 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index 2e8f37e..19b31d7 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3065,6 +3065,35 @@ static int fsg_bind(struct usb_configuration *c,
struct usb_function *f)
int ret;
struct fsg_opts *opts;
+ /*
+ * Don't allow to bind if we don't have at least one LUN
+ * or LUNs ids are not contiguous.
+ */
+ if (likely(common-luns)) {
+ bool found_null = false;
+
+ for (i = 0; i common-nluns; ++i) {
+ if (!common-luns[i]) {
+ found_null = true;
+ continue;
+ }
+
+ if (!found_null) {
+ continue;
+ } else {
+ pr_err(LUN ids should be contiguous.\n);
+ return -EINVAL;
+ }
+ }
+
+ if (i == 0 || !common-luns[i]) {
+ pr_err(There should be at least one LUN.\n);
+ return -EINVAL;
+ }
+ } else {
+ return -EINVAL;
+ }
+
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
