Re: [PATCH] staging: usbip: stub_main.c: Cleaning up missing null-terminate after strncpy call
On Wed, Jun 04, 2014 at 11:39:49PM +0200, Rickard Strandqvist wrote: > Added a guaranteed null-terminate after call to strncpy. > > This was partly found using a static code analysis program called cppcheck. > We already knew that the string was NUL terminated because we checked strnlen() on the lines before. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: gadget: pxa27x_udc: use devm_* helpers
Sergei Shtylyov writes:
> Hello.
>
> On 06/09/2014 11:52 PM, Robert Jarzmik wrote:
>> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +udc->regs = devm_request_and_ioremap(&pdev->dev, regs);
>
>Isn't this function deprecated in favor of devm_ioremap_resource()?
I don't know. It's not marked as deprecated. Could you give me a pointer ?
>
>> if (!regs)
>
>s/regs/udc->regs/.
Good catch.
>> if (retval != 0) {
>> dev_err(udc->dev, "%s: can't get irq %i, err %d\n",
>> driver_name, udc->irq, retval);
>> -goto err_irq;
>> +goto err;
>
>Why not just *return*?
Sure, simpler would be a return :)
>
>> }
>>
>> retval = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
>> if (retval)
>> -goto err_add_udc;
>> +goto err;
>
>Likewise.
Ditto.
Thanks for the review.
Cheers.
--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: pxa27x_udc: add devicetree support
Jingoo Han writes: > On Tuesday, June 10, 2014 4:52 AM, Robert Jarzmik wrote: >> >> Add support for device-tree device discovery. If devicetree is not >> provided, fallback to legacy platform data "discovery". >> +#include > > I don't think that this 'hardware.h' is necessary. > Let me know the reason why this header file is added. Debug leftover, good point. Will be removed in v2. >> >> #include >> #include >> #include >> +#include > > How about renaming and moving this 'udc.h' to 'include/linux/usb/' > directory, and including '#include '? > In this case, it enhances build coverage. So, regardless of the > machine dependency, this driver file 'pxa27x_udc.c' can be built. Actually, I'll change it by : #include The build coverage will be still enhanced, and the true purpose is clearer : provide the platform data structure for legacy boards Thanks for the review. Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/7] usb: gadget: core: delete udc_attach_driver API
After gadget bus introducing, the user (configfs) of this API
will use device-model's bind/unbind, so we can use delete it.
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc-core.c | 44 -
include/linux/usb/gadget.h|2 -
2 files changed, 0 insertions(+), 46 deletions(-)
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 00a04e9..47f87af 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -158,16 +158,6 @@ static int __find_driver(struct device *dev, void *data)
return 0;
}
-static int __find_udc(struct device *dev, void *data)
-{
- char *name = data;
-
- if (strcmp(name, dev_name(dev)) == 0)
- return 1;
- else
- return 0;
-}
-
/**
* usb_gadget_udc_start - tells usb device controller to start up
* @gadget: The gadget we want to get started
@@ -410,40 +400,6 @@ err1:
return ret;
}
-int udc_attach_driver(const char *name, struct usb_gadget_driver *driver)
-{
- struct usb_udc *udc;
- struct device *dev;
- int ret = -ENODEV;
- struct device_driver *drv;
-
- mutex_lock(&udc_lock);
-
- if (!driver || !driver->bind || !driver->setup) {
- ret = -EINVAL;
- goto out;
- }
-
- dev = bus_find_device(&gadget_bus_type,
- NULL, (char *)name, __find_udc);
- if (!dev)
- goto out;
-
- udc = container_of(dev, struct usb_udc, dev);
- if (udc->driver) {
- ret = -EBUSY;
- goto out;
- }
-
- drv = &driver->driver;
- drv->bus = &gadget_bus_type;
- ret = driver_register(drv);
-out:
- mutex_unlock(&udc_lock);
- return ret;
-}
-EXPORT_SYMBOL_GPL(udc_attach_driver);
-
int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
{
struct device_driver *drv;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..cd58acd 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -921,8 +921,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 udc_attach_driver(const char *name,
- struct usb_gadget_driver *driver);
/*-*/
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/7] usb: gadget: core: add implementation of gadget bus
At gadget bus, the device (udc) and the driver (gadget dirver)
will try to match between each other, the automatically
binding is designed at this patch.
The rule of auto-binding is: if the udc is not in use and the
gadget driver is not used by any udcs, it is matched successfully,
and will be binded. Each gadget driver can only be binded at one
udc. For example, there are two udcs, "udc-0" and "udc-1", and
two gadget drivers, "g_mass_storage" and "g_serial", assumed the
udcs is added to bus first, then, the "udc-0" will be binded to
"g_mass_storage", and "udc-1" will be binded to "g_seiral".
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc-core.c | 192 +++--
1 files changed, 128 insertions(+), 64 deletions(-)
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 86aeb8b..e2d26c6 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -20,10 +20,10 @@
#include
#include
#include
-#include
#include
#include
#include
+#include
#include
#include
@@ -33,7 +33,6 @@
* @driver - the gadget driver pointer. For use by gadget bus
* @dev - the child device to the actual controller
* @gadget - the gadget. For use by the gadget bus code
- * @list - for use by the gadget bus code
*
* This represents the internal data structure which is used by the gadget bus
* to hold information about udc driver and gadget together.
@@ -42,12 +41,11 @@ struct usb_udc {
struct usb_gadget_driver*driver;
struct usb_gadget *gadget;
struct device dev;
- struct list_headlist;
};
static struct bus_type gadget_bus_type;
-static LIST_HEAD(udc_list);
static DEFINE_MUTEX(udc_lock);
+static DEFINE_IDA(udc_ida);
/* - */
@@ -123,6 +121,38 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
/* - */
+static int __find_gadget(struct device *dev, void *data)
+{
+ struct usb_gadget *gadget = data;
+ struct usb_udc *udc = container_of(dev, struct usb_udc, dev);
+
+ if (udc->gadget == gadget)
+ return 1;
+ else
+ return 0;
+}
+
+static int __find_driver(struct device *dev, void *data)
+{
+ struct usb_gadget_driver *driver = data;
+ struct usb_udc *udc = container_of(dev, struct usb_udc, dev);
+
+ if (udc->driver == driver)
+ return 1;
+ else
+ return 0;
+}
+
+static int __find_udc(struct device *dev, void *data)
+{
+ char *name = data;
+
+ if (strcmp(name, dev_name(dev)) == 0)
+ return 1;
+ else
+ return 0;
+}
+
/**
* usb_gadget_udc_start - tells usb device controller to start up
* @gadget: The gadget we want to get started
@@ -188,7 +218,7 @@ static void usb_udc_nop_release(struct device *dev)
* usb_add_gadget_udc_release - adds a new gadget to the udc framework
* @parent: the parent device to this udc. Usually the controller driver's
* device.
- * @gadget: the gadget to be added to the list.
+ * @gadget: the gadget to be added to the gadget bus.
* @release: a gadget release function.
*
* Returns zero on success, negative errno otherwise.
@@ -197,7 +227,11 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
void (*release)(struct device *dev))
{
struct usb_udc *udc;
- int ret = -ENOMEM;
+ int id, ret = -ENOMEM;
+
+ id = ida_simple_get(&udc_ida, 0, 0x, GFP_KERNEL);
+ if (id < 0)
+ return id;
udc = kzalloc(sizeof(*udc), GFP_KERNEL);
if (!udc)
@@ -227,14 +261,14 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
udc->dev.bus = &gadget_bus_type;
udc->dev.groups = usb_udc_attr_groups;
udc->dev.parent = parent;
- ret = dev_set_name(&udc->dev, "%s", kobject_name(&parent->kobj));
+ udc->dev.id = id;
+ ret = dev_set_name(&udc->dev, "%s-%d", "udc", id);
if (ret)
goto err3;
udc->gadget = gadget;
mutex_lock(&udc_lock);
- list_add_tail(&udc->list, &udc_list);
ret = device_add(&udc->dev);
if (ret)
@@ -247,17 +281,15 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
return 0;
err4:
- list_del(&udc->list);
mutex_unlock(&udc_lock);
-
err3:
put_device(&udc->dev);
-
err2:
put_device(&gadget->dev);
kfree(udc);
err1:
+ ida_simple_remove(&udc_ida, id);
return ret;
}
EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
@@ -266,7 +298,7 @@ EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
* usb_add_gadget_udc - adds a new gadget to the udc framework
*
[PATCH RFC 5/7] usb: gadget: configfs: changes for gadget bus introducing
- Deciding configfs device_driver's name according to creating
sequence, it is still RFC
- Register gadget driver at gadgets_make
- Delete gadget_cdev_desc_UDC_store and gadget_cdev_desc_UDC_show
interface, we can use standard device-model bind/unbind entry to
bind specific udc to gadgetfs driver after disable gadget_bus's
drivers_autoprobe
Signed-off-by: Peter Chen
---
drivers/usb/gadget/configfs.c | 115 +++-
1 files changed, 55 insertions(+), 60 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 2ddcd63..32655da 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -3,12 +3,14 @@
#include
#include
#include
+#include
#include
#include
#include "configfs.h"
#include "u_f.h"
#include "u_os_desc.h"
+static DEFINE_IDA(gadget_ida);
int check_user_usb_string(const char *name,
struct usb_gadget_strings *stringtab_dev)
{
@@ -54,7 +56,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
@@ -63,6 +64,7 @@ struct gadget_info {
bool use_os_desc;
char b_vendor_code;
char qw_sign[OS_STRING_QW_SIGN_LEN];
+ int id;
};
struct config_usb_cfg {
@@ -228,62 +230,16 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct
gadget_info *gi,
return len;
}
-static ssize_t gadget_dev_desc_UDC_show(struct gadget_info *gi, char *page)
-{
- return sprintf(page, "%s\n", gi->udc_name ?: "");
-}
-
static int unregister_gadget(struct gadget_info *gi)
{
int ret;
- if (!gi->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;
return 0;
}
-static ssize_t gadget_dev_desc_UDC_store(struct gadget_info *gi,
- const char *page, size_t len)
-{
- char *name;
- int ret;
-
- name = kstrdup(page, GFP_KERNEL);
- if (!name)
- return -ENOMEM;
- if (name[len - 1] == '\n')
- name[len - 1] = '\0';
-
- mutex_lock(&gi->lock);
-
- if (!strlen(name)) {
- ret = unregister_gadget(gi);
- if (ret)
- goto err;
- } else {
- if (gi->udc_name) {
- ret = -EBUSY;
- goto err;
- }
- ret = udc_attach_driver(name, &gi->composite.gadget_driver);
- if (ret)
- goto err;
- gi->udc_name = name;
- }
- mutex_unlock(&gi->lock);
- return len;
-err:
- kfree(name);
- mutex_unlock(&gi->lock);
- return ret;
-}
-
GI_DEVICE_DESC_ITEM_ATTR(bDeviceClass);
GI_DEVICE_DESC_ITEM_ATTR(bDeviceSubClass);
GI_DEVICE_DESC_ITEM_ATTR(bDeviceProtocol);
@@ -292,7 +248,6 @@ GI_DEVICE_DESC_ITEM_ATTR(idVendor);
GI_DEVICE_DESC_ITEM_ATTR(idProduct);
GI_DEVICE_DESC_ITEM_ATTR(bcdDevice);
GI_DEVICE_DESC_ITEM_ATTR(bcdUSB);
-GI_DEVICE_DESC_ITEM_ATTR(UDC);
static struct configfs_attribute *gadget_root_attrs[] = {
&gadget_cdev_desc_bDeviceClass.attr,
@@ -303,7 +258,6 @@ static struct configfs_attribute *gadget_root_attrs[] = {
&gadget_cdev_desc_idProduct.attr,
&gadget_cdev_desc_bcdDevice.attr,
&gadget_cdev_desc_bcdUSB.attr,
- &gadget_cdev_desc_UDC.attr,
NULL,
};
@@ -346,6 +300,7 @@ static void gadget_info_attr_release(struct config_item
*item)
WARN_ON(!list_empty(&gi->string_list));
WARN_ON(!list_empty(&gi->available_func));
kfree(gi->composite.gadget_driver.function);
+ ida_simple_remove(&gadget_ida, gi->id);
kfree(gi);
}
@@ -438,9 +393,7 @@ static int config_usb_cfg_unlink(
* remove the function.
*/
mutex_lock(&gi->lock);
- if (gi->udc_name)
- unregister_gadget(gi);
- WARN_ON(gi->udc_name);
+ unregister_gadget(gi);
list_for_each_entry(f, &cfg->func_list, list) {
if (f->fi == fi) {
@@ -917,10 +870,8 @@ 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)
- unregister_gadget(gi);
+ unregister_gadget(gi);
cdev->os_desc_config = NULL;
- WARN_ON(gi->udc_name);
mutex_unlock(&gi->lock);
return 0;
}
@@ -1456,20 +1407,54 @@ static const struct usb_gadget_driver
configfs_driver_template = {
.max_speed = USB_SPEED_SUPER,
.driver = {
.owner = THIS_MODULE,
- .name = "configfs-gadget",
},
};
+static int configfs_register_driver(struct gadget_info *gi)
+{
+
[PATCH v2 3/7] usb: gadget: core: add manual binding
The user can bind any unused udc to specific gadget driver if there is no udc has already been binded to that gadget driver. For example: cd /sys/bus/usb_gadget/drivers echo udc-0 > g_serial echo udc-1 > g_mass_storage There are three solutions to do manual binding: 1. If the udc driver or gadget driver can be built as modules - echo 0 > /sys/bus/usb_gadget/drivers_autoprobe - load udc or gadget driver - Do manual binding 2. If both udc driver and gadget are build in - Add "udc-core.manual_binding=1" to your bootargs - After rootfs has been mounted, clear the value of manual_binding using below command: echo 0 > /sys/module/udc_core/parameters/manual_binding - Do manual binding 3. Unbind all current bindings first, then rebind the udc to driver cd /sys/bus/usb_gadget/drivers echo udc-0 > g_serial/unbind echo udc-1 > g_mass_storage/unbind echo udc-1 > g_serial/unbind echo udc-0 > g_mass_storage/unbind Signed-off-by: Peter Chen --- drivers/usb/gadget/udc-core.c | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c index e2d26c6..00a04e9 100644 --- a/drivers/usb/gadget/udc-core.c +++ b/drivers/usb/gadget/udc-core.c @@ -47,6 +47,21 @@ static struct bus_type gadget_bus_type; static DEFINE_MUTEX(udc_lock); static DEFINE_IDA(udc_ida); +/* + * We can bind any unused udc to specfic driver after setting manual_binding + * eg: + * echo udc-0 > /sys/bus/usb_gadget/drivers/g_serial + * echo udc-1 > /sys/bus/usb_gadget/drivers/g_mass_storage + * + * How to use manual_binding: + * First, set manual_binding = 1 before drivers and devices are added to bus + * Second, set manual_binding = 0 + * Third, do manual_binding like above + */ +static bool manual_binding; +module_param(manual_binding, bool, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(manual_binding, "binding udc and gadget driver manully"); + /* - */ #ifdef CONFIG_HAS_DMA @@ -584,6 +599,9 @@ static int usb_gadget_match(struct device *dev, struct device_driver *drv) container_of(drv, struct usb_gadget_driver, driver); bool dev_not_in_use = true, driver_not_in_use = true; + if (manual_binding) + return 0; + dev_dbg(dev, "%s: driver: %s\n", __func__, drv->name); udc = container_of(dev, struct usb_udc, dev); -- 1.7.8 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/7] usb: gadget: core: change udc class to gadget bus
The udc will be the device at gadget bus, and the gadget driver
will be the driver at this bus.
With gadget bus, we can support below features:
- udc driver is loaded later than gadget driver
- Binding udc and gadget driver manually
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc-core.c | 41 ++---
1 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index b0d9817..86aeb8b 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -30,12 +30,12 @@
/**
* struct usb_udc - describes one usb device controller
- * @driver - the gadget driver pointer. For use by the class code
+ * @driver - the gadget driver pointer. For use by gadget bus
* @dev - the child device to the actual controller
- * @gadget - the gadget. For use by the class code
- * @list - for use by the udc class driver
+ * @gadget - the gadget. For use by the gadget bus code
+ * @list - for use by the gadget bus code
*
- * This represents the internal data structure which is used by the UDC-class
+ * This represents the internal data structure which is used by the gadget bus
* to hold information about udc driver and gadget together.
*/
struct usb_udc {
@@ -45,7 +45,7 @@ struct usb_udc {
struct list_headlist;
};
-static struct class *udc_class;
+static struct bus_type gadget_bus_type;
static LIST_HEAD(udc_list);
static DEFINE_MUTEX(udc_lock);
@@ -128,7 +128,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
* @gadget: The gadget we want to get started
* @driver: The driver we want to bind to @gadget
*
- * This call is issued by the UDC Class driver when it's about
+ * This call is issued by the gadget bus driver when it's about
* to register a gadget driver to the device controller, before
* calling gadget driver's bind() method.
*
@@ -148,7 +148,7 @@ static inline int usb_gadget_udc_start(struct usb_gadget
*gadget,
* @gadget: The device we want to stop activity
* @driver: The driver to unbind from @gadget
*
- * This call is issued by the UDC Class driver after calling
+ * This call is issued by the gadget bus driver after calling
* gadget driver's unbind() method.
*
* The details are implementation specific, but it can go as
@@ -185,7 +185,7 @@ static void usb_udc_nop_release(struct device *dev)
}
/**
- * usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
+ * usb_add_gadget_udc_release - adds a new gadget to the udc framework
* @parent: the parent device to this udc. Usually the controller driver's
* device.
* @gadget: the gadget to be added to the list.
@@ -224,7 +224,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
device_initialize(&udc->dev);
udc->dev.release = usb_udc_release;
- udc->dev.class = udc_class;
+ udc->dev.bus = &gadget_bus_type;
udc->dev.groups = usb_udc_attr_groups;
udc->dev.parent = parent;
ret = dev_set_name(&udc->dev, "%s", kobject_name(&parent->kobj));
@@ -263,7 +263,7 @@ err1:
EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
/**
- * usb_add_gadget_udc - adds a new gadget to the udc class driver list
+ * usb_add_gadget_udc - adds a new gadget to the udc framework
* @parent: the parent device to this udc. Usually the controller
* driver's device.
* @gadget: the gadget to be added to the list
@@ -560,23 +560,26 @@ static int usb_udc_uevent(struct device *dev, struct
kobj_uevent_env *env)
return 0;
}
+static struct bus_type gadget_bus_type = {
+ .name = "usb_gadget",
+ .uevent = usb_udc_uevent,
+};
+
static int __init usb_udc_init(void)
{
- udc_class = class_create(THIS_MODULE, "udc");
- if (IS_ERR(udc_class)) {
- pr_err("failed to create udc class --> %ld\n",
- PTR_ERR(udc_class));
- return PTR_ERR(udc_class);
- }
+ int ret = bus_register(&gadget_bus_type);
+
+ if (ret)
+ pr_err("failed to register gadget bus: %d\n", ret);
+
+ return ret;
- udc_class->dev_uevent = usb_udc_uevent;
- return 0;
}
subsys_initcall(usb_udc_init);
static void __exit usb_udc_exit(void)
{
- class_destroy(udc_class);
+ bus_unregister(&gadget_bus_type);
}
module_exit(usb_udc_exit);
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] doc: usb: gadget_configfs: change for gadget bus introducing
After gadget bus introducing, the bind/unbind udc to configfs driver will use standard device-model's usage, which are the entries of bind and unbind at device driver. Signed-off-by: Peter Chen --- Documentation/usb/gadget_configfs.txt | 13 ++--- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Documentation/usb/gadget_configfs.txt b/Documentation/usb/gadget_configfs.txt index 4cf53e4..fd4119c 100644 --- a/Documentation/usb/gadget_configfs.txt +++ b/Documentation/usb/gadget_configfs.txt @@ -53,6 +53,7 @@ http://www.spinics.net/lists/linux-usb/msg76388.html) $ modprobe libcomposite $ mount none $CONFIGFS_HOME -t configfs +$ echo 0 > /sys/bus/usb_gadget/drivers_autoprobe where CONFIGFS_HOME is the mount point for configfs @@ -189,7 +190,6 @@ An example directory structure might look like this: ./functions/ncm.usb0/qmult ./functions/ncm.usb0/host_addr ./functions/ncm.usb0/dev_addr -./UDC ./bcdUSB ./bcdDevice ./idProduct @@ -203,18 +203,17 @@ An example directory structure might look like this: Such a gadget must be finally enabled so that the USB host can enumerate it. In order to enable the gadget it must be bound to a UDC (USB Device Controller). -$ echo > UDC +$ echo > /sys/bus/usb_gadget/drivers//bind -where is one of those found in /sys/class/udc/* +where is one of those found in /sys/bus/usb_gadget/devices/* +where is one of those found in /sys/bus/usb_gadget/drivers/* e.g.: - -$ echo s3c-hsotg > UDC - +$ echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/bind 6. Disabling the gadget --- -$ echo "" > UDC +$ echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/unbind 7. Cleaning up -- -- 1.7.8 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/7] Add Gadget Bus
Changes for v2: - Felipe's comments: " For example we don't want automatic binding, basically we might want to have g_mass_storage running on both ports and this series would prevent it." Currently, it is impossible to let g_mass_storage or other g_xxx gadgets run at both ports due to g_xxx gadget implementation can't re-enter, eg, the struct usb_configuration is global variable, it will be the same for two instances (udcs), the only possible way to run the same function at both udcs is to use gadget_configfs, the same function at different configuration will be treated differently. If we use gadget_configfs to let the same function running at two udcs, we can disable gadget_bus's drivers_autoprobe entry to disable auto-binding before gadget_configfs working. - Andrzej's test that the gadget_configfs can't work well. I made some changes for gadget_configfs to let it use standard device-model bind/unbind to enable/disable udc, see patch[5/7] and patch[7/7] for detail. Since we introduce the gadget bus, we had to use standard device model way to handle device driver register for gadget_configfs. I mark patch[5/7] as RFC, since I still have no good idea for device driver's name for gadget_configfs, the struct usb_gadget_driver is different for each udc, so it has to use different device driver to manage it. Andrzej, would you have some suggestion for this RFC? gadget-configfs works well for v2. Hi Felipe & Alan, To continue with topic discussed at http://www.spinics.net/lists/linux-usb/msg105279.html, I implement the gadget bus to bind udc to gadget driver follow most ideas from your two. Now this problem is really annoying, many gadget build-in platforms (Eg, Android) will fail to enable usb peripheral function due to udc is probe deferral, I hope we can fix this problem asap. The current "udc class" design can't cover auto-bind/manual bind by device model, so the first patch changes "udc class" to "gadget bus". The second patch adds the implementation of gadget bus, and the udc will bind to gadget driver automatically when they are matched. The rule of auto-binding is: if the udc is not in use and the gadget driver is not used by any udcs, it is matched successfully, and will be binded. Each gadget driver can only be binded at one udc. For example, there are two udcs, "udc-0" and "udc-1", and two gadget drivers, "g_mass_storage" and "g_serial", assumed the udcs is added to bus first, then, the "udc-0" will be binded to "g_mass_storage", and "udc-1" will be binded to "g_seiral". The third patch adds one "tricky" solution for manual bind if both udc and gadget driver are built in. The fourth patch delete the __init marker from all gadget driver's bind function, since the bind may not be executed after gadget driver has loaded due to udc is not ready. Below tests are done: One gadget driver build in and two udcs, udc devices are created first One gadget driver build in and two udcs, gadget driver is created first Two gadget driver build as modules and two udcs build in Two gadget driver build as modules and the udc driver build as module, and no matter which is loaded first. Manual binding/unbinding with different udcs: - Both udc and gadget driver are build in, udc-core.manual_binding=1 echo 0 > /sys/module/udc_core/parameters/manual_binding echo udc-0 > /sys/bus/usb_gadget/drivers/g_serial/bind echo udc-0 > /sys/bus/usb_gadget/drivers/g_serial/unbind echo udc-1 > /sys/bus/usb_gadget/drivers/g_serial/bind echo udc-1 > /sys/bus/usb_gadget/drivers/g_serial/unbind echo udc-0 > /sys/bus/usb_gadget/drivers/g_serial/bind - After auto-bind, unbind current udc and do manual bind echo udc-0 > /sys/bus/usb_gadget/drivers/g_serial/unbind echo udc-1 > /sys/bus/usb_gadget/drivers/g_serial/bind The problem still exists: Some drivers (like mass_storage) call usb_gadget_unregister_driver during their .unbind, it causes device core dead lock at below call chain if udc is unloaded first: device_del->device_lock(dev)->device_release_driver->usb_gadget_remove_driver ->.unbind->usb_gadget_unregister_driver->driver_detach->device_lock(dev) Peter Chen (7): usb: gadget: core: change udc class to gadget bus usb: gadget: core: add implementation of gadget bus usb: gadget: core: add manual binding usb: gadget: remove __init from gadget driver bind function usb: gadget: configfs: changes for gadget bus introducing usb: gadget: core: delete udc_attach_driver API doc: usb: gadget_configfs: change for gadget bus introducing Documentation/usb/gadget_configfs.txt | 13 +- drivers/usb/gadget/acm_ms.c |4 +- drivers/usb/gadget/audio.c|4 +- drivers/usb/gadget/cdc2.c |4 +- drivers/usb/gadget/configfs.c | 115 drivers/usb/gadget/dbgp.c |4 +- drivers/usb/gadget/ether.c|6 +- drivers/usb/gadget/f_hid.c|6 +-
[PATCH v2 4/7] usb: gadget: remove __init from gadget driver bind function
Since the .bind may not be called at the driver's initialization
routine due to the udc is not ready, we remove __init marker for it.
Signed-off-by: Peter Chen
---
drivers/usb/gadget/acm_ms.c |4 ++--
drivers/usb/gadget/audio.c|4 ++--
drivers/usb/gadget/cdc2.c |4 ++--
drivers/usb/gadget/dbgp.c |4 ++--
drivers/usb/gadget/ether.c|6 +++---
drivers/usb/gadget/f_hid.c|6 +++---
drivers/usb/gadget/f_midi.c | 12 ++--
drivers/usb/gadget/f_uac1.c | 12 ++--
drivers/usb/gadget/f_uac2.c |2 +-
drivers/usb/gadget/f_uvc.c| 28 ++--
drivers/usb/gadget/gmidi.c|4 ++--
drivers/usb/gadget/hid.c |6 +++---
drivers/usb/gadget/mass_storage.c |4 ++--
drivers/usb/gadget/multi.c|4 ++--
drivers/usb/gadget/ncm.c |4 ++--
drivers/usb/gadget/nokia.c|4 ++--
drivers/usb/gadget/printer.c |6 +++---
drivers/usb/gadget/serial.c |2 +-
drivers/usb/gadget/u_uac1.c |2 +-
drivers/usb/gadget/webcam.c |4 ++--
drivers/usb/gadget/zero.c |2 +-
21 files changed, 62 insertions(+), 62 deletions(-)
diff --git a/drivers/usb/gadget/acm_ms.c b/drivers/usb/gadget/acm_ms.c
index a252444..1cabaa7 100644
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/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;
diff --git a/drivers/usb/gadget/audio.c b/drivers/usb/gadget/audio.c
index 231b0ef..f2a47e9 100644
--- a/drivers/usb/gadget/audio.c
+++ b/drivers/usb/gadget/audio.c
@@ -107,7 +107,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)
{
/* FIXME alloc iConfiguration string, set it in c->strings */
@@ -133,7 +133,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)
{
int status;
diff --git a/drivers/usb/gadget/cdc2.c b/drivers/usb/gadget/cdc2.c
index e126b6b..8d3746e 100644
--- a/drivers/usb/gadget/cdc2.c
+++ b/drivers/usb/gadget/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;
diff --git a/drivers/usb/gadget/dbgp.c b/drivers/usb/gadget/dbgp.c
index 986fc51..b96e86e1 100644
--- a/drivers/usb/gadget/dbgp.c
+++ b/drivers/usb/gadget/dbgp.c
@@ -235,7 +235,7 @@ static void dbgp_unbind(struct usb_gadget *gadget)
static unsigned char tty_line;
#endif
-static int __init dbgp_configure_endpoints(struct usb_gadget *gadget)
+static int dbgp_configure_endpoints(struct usb_gadget *gadget)
{
int stp;
@@ -291,7 +291,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/ether.c b/drivers/usb/gadget/ether.c
index c1c113e..166093b 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -222,7 +222,7 @@ static struct usb_function *f_rndis;
* the first one present. That's to make Microsoft's drivers happy,
* and to follow DOCSIS 1.0 (cable modem standard).
*/
-static int __init rndis_do_config(struct usb_configuration *c)
+static int rndis_do_config(struct usb_configuration *c)
{
int status;
@@ -264,7 +264,7 @@ MODULE_PARM_DESC(use_eem, "use CDC EEM mode");
/*
* We _always
RE: [PATCH 0/9] ARM: Berlin: USB support
> > we are early in the cycle and I feel there will be a v2 of the chipidea > stub. IMHO, controlling the vbus regulator should not be business of the > phy driver, so ci will have to deal with it. > vbus handling has already in chipidea common code, it treats vbus as a regulator. So, the glue layer only needs to register and add vbus-supply to dts file, no source code change is needed. Peter > Anyways, good to know you are fine with it already! > > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: pxa27x_udc: add devicetree support
On Tuesday, June 10, 2014 4:52 AM, Robert Jarzmik wrote:
>
> Add support for device-tree device discovery. If devicetree is not
> provided, fallback to legacy platform data "discovery".
>
> Signed-off-by: Robert Jarzmik
> ---
> drivers/usb/gadget/pxa27x_udc.c | 105
>
> drivers/usb/gadget/pxa27x_udc.h | 4 ++
> 2 files changed, 90 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
> index cdf4d67..6236848 100644
> --- a/drivers/usb/gadget/pxa27x_udc.c
> +++ b/drivers/usb/gadget/pxa27x_udc.c
> @@ -24,12 +24,15 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> +
> +#include
> +#include
I don't think that this 'hardware.h' is necessary.
Let me know the reason why this header file is added.
>
> #include
> #include
> #include
> +#include
How about renaming and moving this 'udc.h' to 'include/linux/usb/'
directory, and including '#include '?
In this case, it enhances build coverage. So, regardless of the
machine dependency, this driver file 'pxa27x_udc.c' can be built.
Best regards,
Jingoo Han
>
> #include "pxa27x_udc.h"
>
> @@ -1508,16 +1511,16 @@ static struct usb_ep_ops pxa_ep_ops = {
> static void dplus_pullup(struct pxa_udc *udc, int on)
> {
> if (on) {
> - if (gpio_is_valid(udc->mach->gpio_pullup))
> - gpio_set_value(udc->mach->gpio_pullup,
> -!udc->mach->gpio_pullup_inverted);
> - if (udc->mach->udc_command)
> + if (gpio_is_valid(udc->gpio_pullup))
> + gpio_set_value(udc->gpio_pullup,
> +!udc->gpio_pullup_inverted);
> + if (udc->mach && udc->mach->udc_command)
> udc->mach->udc_command(PXA2XX_UDC_CMD_CONNECT);
> } else {
> - if (gpio_is_valid(udc->mach->gpio_pullup))
> - gpio_set_value(udc->mach->gpio_pullup,
> -udc->mach->gpio_pullup_inverted);
> - if (udc->mach->udc_command)
> + if (gpio_is_valid(udc->gpio_pullup))
> + gpio_set_value(udc->gpio_pullup,
> +udc->gpio_pullup_inverted);
> + if (udc->mach && udc->mach->udc_command)
> udc->mach->udc_command(PXA2XX_UDC_CMD_DISCONNECT);
> }
> udc->pullup_on = on;
> @@ -1609,7 +1612,8 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget,
> int is_active)
> {
> struct pxa_udc *udc = to_gadget_udc(_gadget);
>
> - if (!gpio_is_valid(udc->mach->gpio_pullup) && !udc->mach->udc_command)
> + if (!gpio_is_valid(udc->gpio_pullup)
> + && !(udc->mach && udc->mach->udc_command))
> return -EOPNOTSUPP;
>
> dplus_pullup(udc, is_active);
> @@ -2404,6 +2408,56 @@ static struct pxa_udc memory = {
> }
> };
>
> +static struct of_device_id udc_pxa_dt_ids[] = {
> + { .compatible = "mrvl,pxa27x_udc" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids);
> +
> +/**
> + * pxa_udc_probe_dt - device tree specific probe
> + * @pdev: platform data
> + * @udc: pxa_udc structure to fill
> + *
> + * Fills udc from platform data out of device tree.
> + *
> + * Returns 0 if DT found, 1 if DT not found, and <0 on error
> + */
> +bool pxa_udc_probe_dt(struct platform_device *pdev, struct pxa_udc *udc)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *of_id =
> + of_match_device(udc_pxa_dt_ids, &pdev->dev);
> + int ret;
> + u32 gpio_pullup;
> +
> + if (!np || !of_id)
> + return 1;
> + ret = of_alias_get_id(np, "udc");
> + pdev->id = (ret >= 0) ? ret : -1;
> +
> + if (!of_property_read_u32(np, "udc-pullup-gpio", &gpio_pullup))
> + udc->gpio_pullup = gpio_pullup;
> + udc->gpio_pullup_inverted =
> + of_property_read_bool(np, "udc-pullup-gpio-inverted");
> + return 0;
> +}
> +
> +/**
> + * pxa_udc_probe_pdata - legacy platform data probe
> + * @pdev: platform device
> + * @udc: pxa_udc structure to fill
> + *
> + * Simple copy of data from platform_data to udc control structure
> + */
> +static void pxa_udc_probe_pdata(struct platform_device *pdev,
> +struct pxa_udc *udc)
> +{
> + udc->mach = dev_get_platdata(&pdev->dev);
> + udc->gpio_pullup = udc->mach->gpio_pullup;
> + udc->gpio_pullup_inverted = udc->mach->gpio_pullup_inverted;
> +}
> +
> /**
> * pxa_udc_probe - probes the udc device
> * @_dev: platform device
> @@ -2415,7 +2469,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
> {
> struct resource *regs;
> struct pxa_udc *udc = &memory;
> - int retval = 0, gpio;
> + int retval = 0;
> +
> + retval = pxa_udc_probe_dt(pdev, udc);
> + if (retval < 0)
> + return retval;
Re: [PATCH] xhci: Fix sleeping with IRQs disabled in xhci_stop_device()
On Mon, Jun 9, 2014 at 9:07 AM, Mathias Nyman
wrote:
> xhci_stop_device() allocates and issues stop commands for each active
> endpoint.
> This is done with spinlock held and interrupt disabled so we can't sleep
> during
> memory allocation. Use GFP_ATOMIC instead
>
> Regressiong from commit ddba5cd0aeff5bbed92ebdf4b1223300b0541e78
> "xhci: Use command structures when queuing commands on the command ring"
> for 3.16-rc1
>
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/host/xhci-hub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 6231ce6..0373487 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -287,7 +287,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int
> slot_id, int suspend)
> if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
> struct xhci_command *command;
> command = xhci_alloc_command(xhci, false, false,
> -GFP_NOIO);
> +GFP_ATOMIC);
GFP_NOWAIT instead? Do we really need to steal from emergency pools for this?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] usb: host: xhci-plat: Add support to get PHYs
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 9ffecd5..453d89e 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1582,6 +1582,9 @@ struct xhci_hcd {
> u32 port_status_u0;
> /* Compliance Mode Timer Triggered every 2 seconds */
> #define COMP_MODE_RCVRY_MSECS 2000
> + /* phys for the controller */
> + struct phy *phy2_gen;
> + struct phy *phy3_gen;
> };
I don't think adding new variables here and restricting most of this
logic to xhci-plat.c (in the next patch) is the best way to do it.
There's no conceptual reason why other host controllers (e.g. xhci-pci
or even EHCI) could not have a similar need to tune their PHY after
reset. PHYs are universal to all host controllers.
There is already a 'phy' member in struct usb_hcd which I think is
mostly unused right now. I think it would be much less
confusing/redundant to reuse that member for this purpose (you could
still set it up from xhci_plat_probe(), and then call it from
hcd_bus_resume() or something like that). Since XHCI host controllers
already conveniently have two struct usb_hcd (one for 2.0 and one for
3.0), you can cleanly store references to your two PHYs in there.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: gadget: pxa27x_udc: use devm_* helpers
Hello.
On 06/09/2014 11:52 PM, Robert Jarzmik wrote:
Use devm_* helpers in the probe function to simplify the error path and
the remove path.
Signed-off-by: Robert Jarzmik
---
drivers/usb/gadget/pxa27x_udc.c | 51 +++--
1 file changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
index 6236848..f6843a8 100644
--- a/drivers/usb/gadget/pxa27x_udc.c
+++ b/drivers/usb/gadget/pxa27x_udc.c
@@ -2478,8 +2478,9 @@ static int pxa_udc_probe(struct platform_device *pdev)
pxa_udc_probe_pdata(pdev, udc);
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ udc->regs = devm_request_and_ioremap(&pdev->dev, regs);
Isn't this function deprecated in favor of devm_ioremap_resource()?
if (!regs)
s/regs/udc->regs/.
- return -ENXIO;
+ return -EADDRNOTAVAIL;
udc->irq = platform_get_irq(pdev, 0);
if (udc->irq < 0)
return udc->irq;
[...]
@@ -2523,34 +2517,23 @@ static int pxa_udc_probe(struct platform_device *pdev)
pxa_eps_setup(udc);
/* irq setup after old hardware state is cleaned up */
- retval = request_irq(udc->irq, pxa_udc_irq,
- IRQF_SHARED, driver_name, udc);
+ retval = devm_request_irq(&pdev->dev, udc->irq, pxa_udc_irq,
+ IRQF_SHARED, driver_name, udc);
if (retval != 0) {
dev_err(udc->dev, "%s: can't get irq %i, err %d\n",
driver_name, udc->irq, retval);
- goto err_irq;
+ goto err;
Why not just *return*?
}
retval = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
if (retval)
- goto err_add_udc;
+ goto err;
Likewise.
pxa_init_debugfs(udc);
return 0;
-err_add_udc:
- free_irq(udc->irq, udc);
-err_irq:
- iounmap(udc->regs);
-err_map:
- clk_unprepare(udc->clk);
-err_clk_prepare:
- clk_put(udc->clk);
- udc->clk = NULL;
-err_clk:
- if (gpio_is_valid(udc->gpio_pullup))
- gpio_free(udc->gpio_pullup);
+err:
return retval;
}
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: gadget: pxa27x_udc: use devm_* helpers
Use devm_* helpers in the probe function to simplify the error path and
the remove path.
Signed-off-by: Robert Jarzmik
---
drivers/usb/gadget/pxa27x_udc.c | 51 +++--
1 file changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
index 6236848..f6843a8 100644
--- a/drivers/usb/gadget/pxa27x_udc.c
+++ b/drivers/usb/gadget/pxa27x_udc.c
@@ -2478,8 +2478,9 @@ static int pxa_udc_probe(struct platform_device *pdev)
pxa_udc_probe_pdata(pdev, udc);
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ udc->regs = devm_request_and_ioremap(&pdev->dev, regs);
if (!regs)
- return -ENXIO;
+ return -EADDRNOTAVAIL;
udc->irq = platform_get_irq(pdev, 0);
if (udc->irq < 0)
return udc->irq;
@@ -2488,7 +2489,8 @@ static int pxa_udc_probe(struct platform_device *pdev)
udc->transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
if (gpio_is_valid(udc->gpio_pullup)) {
- retval = gpio_request(udc->gpio_pullup, "USB D+ pullup");
+ retval = devm_gpio_request(&pdev->dev,
+ udc->gpio_pullup, "USB D+ pullup");
if (retval == 0)
gpio_direction_output(udc->gpio_pullup,
udc->gpio_pullup_inverted);
@@ -2499,21 +2501,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
return retval;
}
- udc->clk = clk_get(&pdev->dev, NULL);
- if (IS_ERR(udc->clk)) {
- retval = PTR_ERR(udc->clk);
- goto err_clk;
- }
+ udc->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(udc->clk))
+ return PTR_ERR(udc->clk);
+
retval = clk_prepare(udc->clk);
if (retval)
- goto err_clk_prepare;
-
- retval = -ENOMEM;
- udc->regs = ioremap(regs->start, resource_size(regs));
- if (!udc->regs) {
- dev_err(&pdev->dev, "Unable to map UDC I/O memory\n");
- goto err_map;
- }
+ return retval;
udc->vbus_sensed = 0;
@@ -2523,34 +2517,23 @@ static int pxa_udc_probe(struct platform_device *pdev)
pxa_eps_setup(udc);
/* irq setup after old hardware state is cleaned up */
- retval = request_irq(udc->irq, pxa_udc_irq,
- IRQF_SHARED, driver_name, udc);
+ retval = devm_request_irq(&pdev->dev, udc->irq, pxa_udc_irq,
+ IRQF_SHARED, driver_name, udc);
if (retval != 0) {
dev_err(udc->dev, "%s: can't get irq %i, err %d\n",
driver_name, udc->irq, retval);
- goto err_irq;
+ goto err;
}
retval = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
if (retval)
- goto err_add_udc;
+ goto err;
pxa_init_debugfs(udc);
return 0;
-err_add_udc:
- free_irq(udc->irq, udc);
-err_irq:
- iounmap(udc->regs);
-err_map:
- clk_unprepare(udc->clk);
-err_clk_prepare:
- clk_put(udc->clk);
- udc->clk = NULL;
-err_clk:
- if (gpio_is_valid(udc->gpio_pullup))
- gpio_free(udc->gpio_pullup);
+err:
return retval;
}
@@ -2561,22 +2544,16 @@ err_clk:
static int pxa_udc_remove(struct platform_device *_dev)
{
struct pxa_udc *udc = platform_get_drvdata(_dev);
- int gpio = udc->mach->gpio_pullup;
usb_del_gadget_udc(&udc->gadget);
usb_gadget_unregister_driver(udc->driver);
- free_irq(udc->irq, udc);
pxa_cleanup_debugfs(udc);
- if (gpio_is_valid(gpio))
- gpio_free(gpio);
usb_put_phy(udc->transceiver);
udc->transceiver = NULL;
the_controller = NULL;
clk_unprepare(udc->clk);
- clk_put(udc->clk);
- iounmap(udc->regs);
return 0;
}
--
2.0.0.rc2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: gadget: pxa27x_udc: add devicetree support
Add support for device-tree device discovery. If devicetree is not
provided, fallback to legacy platform data "discovery".
Signed-off-by: Robert Jarzmik
---
drivers/usb/gadget/pxa27x_udc.c | 105
drivers/usb/gadget/pxa27x_udc.h | 4 ++
2 files changed, 90 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
index cdf4d67..6236848 100644
--- a/drivers/usb/gadget/pxa27x_udc.c
+++ b/drivers/usb/gadget/pxa27x_udc.c
@@ -24,12 +24,15 @@
#include
#include
#include
-#include
-#include
+#include
+
+#include
+#include
#include
#include
#include
+#include
#include "pxa27x_udc.h"
@@ -1508,16 +1511,16 @@ static struct usb_ep_ops pxa_ep_ops = {
static void dplus_pullup(struct pxa_udc *udc, int on)
{
if (on) {
- if (gpio_is_valid(udc->mach->gpio_pullup))
- gpio_set_value(udc->mach->gpio_pullup,
- !udc->mach->gpio_pullup_inverted);
- if (udc->mach->udc_command)
+ if (gpio_is_valid(udc->gpio_pullup))
+ gpio_set_value(udc->gpio_pullup,
+ !udc->gpio_pullup_inverted);
+ if (udc->mach && udc->mach->udc_command)
udc->mach->udc_command(PXA2XX_UDC_CMD_CONNECT);
} else {
- if (gpio_is_valid(udc->mach->gpio_pullup))
- gpio_set_value(udc->mach->gpio_pullup,
- udc->mach->gpio_pullup_inverted);
- if (udc->mach->udc_command)
+ if (gpio_is_valid(udc->gpio_pullup))
+ gpio_set_value(udc->gpio_pullup,
+ udc->gpio_pullup_inverted);
+ if (udc->mach && udc->mach->udc_command)
udc->mach->udc_command(PXA2XX_UDC_CMD_DISCONNECT);
}
udc->pullup_on = on;
@@ -1609,7 +1612,8 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int
is_active)
{
struct pxa_udc *udc = to_gadget_udc(_gadget);
- if (!gpio_is_valid(udc->mach->gpio_pullup) && !udc->mach->udc_command)
+ if (!gpio_is_valid(udc->gpio_pullup)
+ && !(udc->mach && udc->mach->udc_command))
return -EOPNOTSUPP;
dplus_pullup(udc, is_active);
@@ -2404,6 +2408,56 @@ static struct pxa_udc memory = {
}
};
+static struct of_device_id udc_pxa_dt_ids[] = {
+ { .compatible = "mrvl,pxa27x_udc" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, udc_pxa_dt_ids);
+
+/**
+ * pxa_udc_probe_dt - device tree specific probe
+ * @pdev: platform data
+ * @udc: pxa_udc structure to fill
+ *
+ * Fills udc from platform data out of device tree.
+ *
+ * Returns 0 if DT found, 1 if DT not found, and <0 on error
+ */
+bool pxa_udc_probe_dt(struct platform_device *pdev, struct pxa_udc *udc)
+{
+ struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *of_id =
+ of_match_device(udc_pxa_dt_ids, &pdev->dev);
+ int ret;
+ u32 gpio_pullup;
+
+ if (!np || !of_id)
+ return 1;
+ ret = of_alias_get_id(np, "udc");
+ pdev->id = (ret >= 0) ? ret : -1;
+
+ if (!of_property_read_u32(np, "udc-pullup-gpio", &gpio_pullup))
+ udc->gpio_pullup = gpio_pullup;
+ udc->gpio_pullup_inverted =
+ of_property_read_bool(np, "udc-pullup-gpio-inverted");
+ return 0;
+}
+
+/**
+ * pxa_udc_probe_pdata - legacy platform data probe
+ * @pdev: platform device
+ * @udc: pxa_udc structure to fill
+ *
+ * Simple copy of data from platform_data to udc control structure
+ */
+static void pxa_udc_probe_pdata(struct platform_device *pdev,
+ struct pxa_udc *udc)
+{
+ udc->mach = dev_get_platdata(&pdev->dev);
+ udc->gpio_pullup = udc->mach->gpio_pullup;
+ udc->gpio_pullup_inverted = udc->mach->gpio_pullup_inverted;
+}
+
/**
* pxa_udc_probe - probes the udc device
* @_dev: platform device
@@ -2415,7 +2469,13 @@ static int pxa_udc_probe(struct platform_device *pdev)
{
struct resource *regs;
struct pxa_udc *udc = &memory;
- int retval = 0, gpio;
+ int retval = 0;
+
+ retval = pxa_udc_probe_dt(pdev, udc);
+ if (retval < 0)
+ return retval;
+ if (retval > 0)
+ pxa_udc_probe_pdata(pdev, udc);
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!regs)
@@ -2425,19 +2485,17 @@ static int pxa_udc_probe(struct platform_device *pdev)
return udc->irq;
udc->dev = &pdev->dev;
- udc->mach = dev_get_platdata(&pdev->dev);
udc->transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
- gpio = udc->mach->gpio_pullup;
- if (gpio_is_valid(gpio)) {
- retval = gpio_request(gpio, "USB D+ pullup");
+ if (gpio_is_valid(udc->gpi
Re: [PATCH] USB: ftdi_sio: add GPIO support
Hello.
On 06/09/2014 06:23 PM, One Thousand Gnomes wrote:
#include
#include
+#ifdef CONFIG_GPIOLIB
+#include
+#endif
Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.
You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers
#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
I guess you meant *static* instead of *extern* here?
#endif
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Fix sleeping with IRQs disabled in xhci_stop_device()
On Mon, 2014-06-09 at 19:07 +0300, Mathias Nyman wrote:
> xhci_stop_device() allocates and issues stop commands for each active
> endpoint.
> This is done with spinlock held and interrupt disabled so we can't sleep
> during
> memory allocation. Use GFP_ATOMIC instead
>
> Regressiong from commit ddba5cd0aeff5bbed92ebdf4b1223300b0541e78
> "xhci: Use command structures when queuing commands on the command ring"
> for 3.16-rc1
>
> Signed-off-by: Mathias Nyman
> ---
> drivers/usb/host/xhci-hub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 6231ce6..0373487 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -287,7 +287,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int
> slot_id, int suspend)
> if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
> struct xhci_command *command;
> command = xhci_alloc_command(xhci, false, false,
> - GFP_NOIO);
> + GFP_ATOMIC);
Yep, I made the exact same patch today to suppress the warning while I
was debugging xhci failures (see the other thread about uvcvideo).
Tested-by: Johannes Berg
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Fix sleeping with IRQs disabled in xhci_stop_device()
On Mon, Jun 09, 2014 at 07:07:29PM +0300, Mathias Nyman wrote: > xhci_stop_device() allocates and issues stop commands for each active > endpoint. > This is done with spinlock held and interrupt disabled so we can't sleep > during > memory allocation. Use GFP_ATOMIC instead > > Regressiong from commit ddba5cd0aeff5bbed92ebdf4b1223300b0541e78 "Regressiong"? :) > "xhci: Use command structures when queuing commands on the command ring" > for 3.16-rc1 So you want a "fixes:" tag here, right? > > Signed-off-by: Mathias Nyman No "Reported-by:" line? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: Fix sleeping with IRQs disabled in xhci_stop_device()
xhci_stop_device() allocates and issues stop commands for each active endpoint.
This is done with spinlock held and interrupt disabled so we can't sleep during
memory allocation. Use GFP_ATOMIC instead
Regressiong from commit ddba5cd0aeff5bbed92ebdf4b1223300b0541e78
"xhci: Use command structures when queuing commands on the command ring"
for 3.16-rc1
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 6231ce6..0373487 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -287,7 +287,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int
slot_id, int suspend)
if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
struct xhci_command *command;
command = xhci_alloc_command(xhci, false, false,
-GFP_NOIO);
+GFP_ATOMIC);
if (!command) {
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_free_command(xhci, cmd);
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sleeping function called with IRQs disabled
On 06/08/2014 12:36 AM, Johannes Berg wrote: > Just got the below when I plugged in a new webcam. Not sure it's a bug > in xhci (using GFP_KERNEL presumably) or (less likely?) in the USB > autosuspend code. > Thanks, regression in XHCI xhci_stop_device() tries to allocate memory with GFP_KERNEL flag during spin_lock_irqsave(). I'll send a patch for it right away -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V3 0/2] usb: gadget: zero: Add support for interrupt EP
On Mon, Jun 09, 2014 at 10:44:38AM +0530, Amit Virdi wrote: > Felipe, Alan, > > On 5/23/2014 12:01 PM, Amit VIRDI wrote: > >This patchset adds support for interrupt EP and the corresponding test cases > >to > >gadget zero. The code has been rebased and tested on Kernel v3.15-rc5 > > > >V2 -> V3 > > - Rectified wMaxPacketSize for FS from 1023 to 64 > > - Modified default value of interrupt interval for FS to 1 ms > > > >V1 -> V2 > > - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 > > Bulk, 2 > >Isoc) to 2 EPs (Interrupt only) > > > >RFC -> V1 > > - Added support for configuring interrupt EP attributes from configfs > > interface > > > >Amit Virdi (2): > > usb: gadget: zero: Add support for interrupt EP > > usbtest: Add interrupt EP testcases > > > > Any comment on this patchset? Felipe is out for a few weeks, so please be patient, he will return and get to these. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hardware bug in Intel USB-2 hub?
On Mon, 9 Jun 2014, Toralf Förster wrote: > On 06/19/2013 09:03 PM, Alan Stern wrote: > > Sarah: > > > > This report surfaced in Bugzilla #59011 (see especially comments #38 > > and #39). Toralf reports, among other things, that the integrated > > "rate-matching" hub in his ThinkPad T420 (6 Series/C200 Series chipset) > > isn't behaving the way it should. > > > > The particular symptom is that when the hub is suspended, it does not > > relay wakeup requests from a downstream port to its upstream port, if > > the downstream device is connected at low speed and the downstream > > port's suspend feature isn't set. This happens with the 3.10-rc > > kernels, because commit 0aa2832dd0d9 changed system suspend to do a USB > > "global" bus suspend. None of the ports on non-SuperSpeed hubs are > > explicitly put into suspend mode; instead, everything on the bus goes > > into suspend when the root hub stops sending packets. > > > > The problem is easy to test on any system running a 3.10-rc kernel. > > Simply plug a low-speed USB keyboard (almost all keyboards are low > > speed) into a USB-2 port, suspend the system, and then see if typing on > > the keyboard will wake up the computer. > > > > I have tested a couple of external USB-2 hubs; one of them behaved > > correctly and one didn't. Regardless, it was surprising to see > > Toralf's report that an Intel hub doesn't work right. I don't have any > > machines with a comparable chipset, so I can't test one of those > > integrated hubs directly. > > > > Can somebody at Intel look into this? > > > > Alan Stern > > > > > Just tried kernel 3.15. - issue does still exists > https://bugzilla.kernel.org/show_bug.cgi?id=59011 Not surprising, since it is a bug in the hardware. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V3 0/2] usb: gadget: zero: Add support for interrupt EP
On Mon, 9 Jun 2014, Amit Virdi wrote: > Felipe, Alan, > > On 5/23/2014 12:01 PM, Amit VIRDI wrote: > > This patchset adds support for interrupt EP and the corresponding test > > cases to > > gadget zero. The code has been rebased and tested on Kernel v3.15-rc5 > > > > V2 -> V3 > > - Rectified wMaxPacketSize for FS from 1023 to 64 > > - Modified default value of interrupt interval for FS to 1 ms > > > > V1 -> V2 > > - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 > > Bulk, 2 > > Isoc) to 2 EPs (Interrupt only) > > > > RFC -> V1 > > - Added support for configuring interrupt EP attributes from configfs > > interface > > > > Amit Virdi (2): > >usb: gadget: zero: Add support for interrupt EP > >usbtest: Add interrupt EP testcases > > > > Any comment on this patchset? None from me. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: ftdi_sio: add GPIO support
> #include
> #include
> +#ifdef CONFIG_GPIOLIB
> +#include
> +#endif
Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.
You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers
#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
#endif
that keeps the code itself clean and easy to read and also ensures all
the types are checked everywhere we want.
Functionality wise nothing stands out as a problem. I would expect -EBUSY
not -ENXIO if the port was being used for GPIO so could not be used for
tty.
I would favour the new config option.
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels
Hi,
> -Original Message-
> From: Michal Nazarewicz [mailto:m...@google.com] On Behalf Of Michal
> Nazarewicz
> Sent: Monday, June 09, 2014 3:28 PM
> To: Krzysztof Opasiak; 'Felipe Balbi'; Krzysztof Opasiak
> Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCHv2 5/5] tools: ffs-test: add compatibility code
> for old kernels
>
> On Mon, Jun 09 2014, Krzysztof Opasiak wrote:
> > As this is an example which will be copy-paste by many users
> maybe you
> > should you struct usb_functionfs_descs_head and struct
> > usb_functionfs_descs_head_v2 instead of direct operations using
> > hard-coded offsets to make this function more readable?
>
> v3 uses usb_functionfs_descs_head{_v2,} instead of hard-coded
> offsets.
> I also started wondering if it would make sense to go one step
> further
> and define a temporary structure holding the headers. Something
> like:
>
> --- >8
> -
> /* Read v2 header */
> {
> const struct {
> const struct usb_functionfs_descs_head_v2 header;
> const __le32 counts[];
> } __attribute__((packed)) *const in = descriptors;
> const __le32 *counts = in->counts;
> __u32 flags;
>
> if (le32_to_cpu(in->header.magic) !=
> FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
> return 0;
> length = le32_to_cpu(in->header.length);
> if (length <= sizeof in->header)
> return 0;
> length -= sizeof in->header;
> flags = le32_to_cpu(in->header.flags);
> if (flags & ~(FUNCTIONFS_HAS_FS_DESC |
> FUNCTIONFS_HAS_HS_DESC |
> FUNCTIONFS_HAS_SS_DESC))
> return 0;
>
> #define GET_NEXT_COUNT_IF_FLAG(ret, flg) do { \
> if (!(flags & (flg))) \
> break; \
> if (length < 4) \
> return 0; \
> ret = le32_to_cpu(*counts); \
> length -= 4;\
> ++counts; \
> } while (0)
>
> GET_NEXT_COUNT_IF_FLAG(fs_count,
> FUNCTIONFS_HAS_FS_DESC);
> GET_NEXT_COUNT_IF_FLAG(hs_count,
> FUNCTIONFS_HAS_HS_DESC);
> GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);
>
> count = fs_count + hs_count;
> if (!count)
> return 0;
> descs_start = descs_end = (const void *)counts;
>
> #undef GET_NEXT_COUNT_IF_FLAG
> }
>
> --- >8
> -
>
> /* Allocate legacy descriptors and copy the data. */
> {
> struct {
> struct usb_functionfs_descs_head header;
> __u8 descriptors[];
> } __attribute__((packed)) *out;
>
> length = sizeof out->header + (descs_end -
> descs_start);
> out = malloc(length);
> out->header.magic =
> cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
> out->header.length = cpu_to_le32(length);
> out->header.fs_count = cpu_to_le32(fs_count);
> out->header.hs_count = cpu_to_le32(hs_count);
> memcpy(out->descriptors, descs_start, descs_end -
> descs_start);
> *legacy = out;
> }
> --- >8
> -
>
> Thoughts?
Looks very good to me! In my opinion this one should be used, it emphasizes
your intentions very well so it's perfect code for sample app.
--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opas...@samsung.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: ftdi_sio: add GPIO support
Most FTDI USB serial / parallel adapter chips support an asynchronous
"bitbang" mode. Make this functionality available as a GPIO controller
so they can be used with the Linux GPIO API instead of requiring
special user space software that only works with this particular
family of chips.
The chips can operate either in regular or in bitbang mode. Care was
taken to prevent using GPIOs if the serial device is in use and vice
versa.
At least the FT245RL will toggle several of its pins multiple times
during initialisation (even if connected to a dumb USB power supply,
so not a driver issue). Using synchronous bitbang mode might help
coping with this if the hardware connected to the FTDI chip supports
it. However, as synchronous mode isn't supported by the hardware used
for testing (SainSmart 8-channel USB relay card) and is not as
straightforward to use (reading the inputs requires the outputs to be
written first) it's left as a potential future enhancement.
Serial mode and GPIO mode were tested on FT232RL using a simple
loopback setup (TXD connected to RXD). In addition, GPIO output mode
was tested on FT245RL (8 channels).
Signed-off-by: Sascha Silbe
---
Is making the GPIO support conditional on CONFIG_GPIOLIB enough or
should I introduce a new configuration option USB_SERIAL_FTDI_SIO_GPIO
that depends on CONFIG_GPIOLIB?
Signed-off-by: Sascha Silbe
---
drivers/usb/serial/ftdi_sio.c | 264 +-
drivers/usb/serial/ftdi_sio.h | 26 +
2 files changed, 289 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index edf3b12..92f5c14 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -33,6 +33,9 @@
#include
#include
+#ifdef CONFIG_GPIOLIB
+#include
+#endif
#include
#include
#include
@@ -53,6 +56,7 @@
struct ftdi_private {
enum ftdi_chip_type chip_type;
+ struct usb_serial_port *port;
/* type of device, either SIO or FT8U232AM */
int baud_base; /* baud base clock for divisor setting */
int custom_divisor; /* custom_divisor kludge, this is for
@@ -77,6 +81,15 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+#ifdef CONFIG_GPIOLIB
+ struct gpio_chip gc;
+ bool serial_open;
+ unsigned char open_gpios;
+ unsigned int bitmode;
+ unsigned char gpio_direction; /* data direction in bitbang
+ * mode, 0=in / 1=out */
+ unsigned char gpo_values; /* GPIO output values */
+#endif
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -973,6 +986,7 @@ static int ftdi_sio_probe(struct usb_serial *serial,
static int ftdi_sio_port_probe(struct usb_serial_port *port);
static int ftdi_sio_port_remove(struct usb_serial_port *port);
static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port);
+static void ftdi_close(struct usb_serial_port *port);
static void ftdi_dtr_rts(struct usb_serial_port *port, int on);
static void ftdi_process_read_urb(struct urb *urb);
static int ftdi_prepare_write_buffer(struct usb_serial_port *port,
@@ -996,6 +1010,15 @@ static __u32 ftdi_232bm_baud_to_divisor(int baud);
static __u32 ftdi_2232h_baud_base_to_divisor(int baud, int base);
static __u32 ftdi_2232h_baud_to_divisor(int baud);
+#ifdef CONFIG_GPIOLIB
+static int ftdi_gpio_get(struct gpio_chip *gc, unsigned int gpio);
+static void ftdi_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val);
+static int ftdi_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio);
+static int ftdi_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val);
+static int ftdi_gpio_request(struct gpio_chip *chip, unsigned offset);
+static void ftdi_gpio_free(struct gpio_chip *chip, unsigned offset);
+#endif
+
static struct usb_serial_driver ftdi_sio_device = {
.driver = {
.owner =THIS_MODULE,
@@ -1010,6 +1033,7 @@ static struct usb_serial_driver ftdi_sio_device = {
.port_probe = ftdi_sio_port_probe,
.port_remove = ftdi_sio_port_remove,
.open = ftdi_open,
+ .close =ftdi_close,
.dtr_rts = ftdi_dtr_rts,
.throttle = usb_serial_generic_throttle,
.unthrottle = usb_serial_generic_unthrottle,
@@ -1379,6 +1403,59 @@ static int read_latency_timer(struct usb_serial_port
*port)
return rv;
}
+#ifdef CONFIG_GPIOLIB
+static int write_bitmode(struct usb_serial_port *port)
+{
+ struct ftdi_private *priv = usb_get_serial_port_data(port);
+ struct usb_device *udev = port->serial->dev;
+ int rv;
+ int bitmode = priv->bitmode;
+ int gpio_
Re: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels
On Mon, Jun 09 2014, Krzysztof Opasiak wrote:
> As this is an example which will be copy-paste by many users maybe you
> should you struct usb_functionfs_descs_head and struct
> usb_functionfs_descs_head_v2 instead of direct operations using
> hard-coded offsets to make this function more readable?
v3 uses usb_functionfs_descs_head{_v2,} instead of hard-coded offsets.
I also started wondering if it would make sense to go one step further
and define a temporary structure holding the headers. Something like:
--- >8 -
/* Read v2 header */
{
const struct {
const struct usb_functionfs_descs_head_v2 header;
const __le32 counts[];
} __attribute__((packed)) *const in = descriptors;
const __le32 *counts = in->counts;
__u32 flags;
if (le32_to_cpu(in->header.magic) !=
FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
return 0;
length = le32_to_cpu(in->header.length);
if (length <= sizeof in->header)
return 0;
length -= sizeof in->header;
flags = le32_to_cpu(in->header.flags);
if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
FUNCTIONFS_HAS_SS_DESC))
return 0;
#define GET_NEXT_COUNT_IF_FLAG(ret, flg) do { \
if (!(flags & (flg))) \
break; \
if (length < 4) \
return 0; \
ret = le32_to_cpu(*counts); \
length -= 4;\
++counts; \
} while (0)
GET_NEXT_COUNT_IF_FLAG(fs_count, FUNCTIONFS_HAS_FS_DESC);
GET_NEXT_COUNT_IF_FLAG(hs_count, FUNCTIONFS_HAS_HS_DESC);
GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);
count = fs_count + hs_count;
if (!count)
return 0;
descs_start = descs_end = (const void *)counts;
#undef GET_NEXT_COUNT_IF_FLAG
}
--- >8 -
/* Allocate legacy descriptors and copy the data. */
{
struct {
struct usb_functionfs_descs_head header;
__u8 descriptors[];
} __attribute__((packed)) *out;
length = sizeof out->header + (descs_end - descs_start);
out = malloc(length);
out->header.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
out->header.length = cpu_to_le32(length);
out->header.fs_count = cpu_to_le32(fs_count);
out->header.hs_count = cpu_to_le32(hs_count);
memcpy(out->descriptors, descs_start, descs_end - descs_start);
*legacy = out;
}
--- >8 -
Thoughts?
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ehci_orion should be builable for mvebu multi platform
Peter, Please amend your subject line to match the other commits in drivers/usb. Also, s/builable/buildable/ On Sun, Jun 08, 2014 at 09:43:57PM +0100, Peter Robinson wrote: > The ehci_orion is used by numerous mvebu SoCs and not just Orion > so enable it so it can be built as part of the mvebu multi platform > as it's used by Dove, Armada XP, Armada 370 too. > > Tested on Mirabox booting of the usb attached MMC slot > > Signed-off-by: Peter Robinson > --- > drivers/usb/host/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 3d9e540..c0f8900 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -141,7 +141,7 @@ config USB_EHCI_HCD_OMAP > > config USB_EHCI_HCD_ORION > tristate "Support for Marvell EBU on-chip EHCI USB controller" > - depends on USB_EHCI_HCD && PLAT_ORION > + depends on USB_EHCI_HCD && ( PLAT_ORION || ARCH_MVEBU ) hmmm, ARCH_MVEBU selects PLAT_ORION in mach-mvebu/Kconfig:8 over here. What are you trying to fix? thx, Jason. > default y > ---help--- > Enables support for the on-chip EHCI controller on Marvell's > -- > 2.0.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
slight regression in USB wakeup
Hi!
Commit c1db30a2a79eb59997b13b8cabf2a50bea9f04e1 ("USB: OHCI: fix problem
with global suspend on ATI controllers") makes S4 hibernation no longer
respond to wakeup signals that come from USB (such as pressing a key on the
keyboard). Other ways to wake up (such as the power button) still work.
On this piece of hardware, this very commit fixes resuming from S3 sleep.
In S3, USB wakeup signals do work.
--
Gnome 3, Windows 8, Slashdot Beta, now Firefox Ribbon^WAustralis. WTF is going
on with replacing usable interfaces with tabletized ones?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] leds: USB: Add support for MSI GT683R led panels
On Sat, Jun 07, 2014 at 01:12:39PM +0300, Janne Kanniainen wrote:
> > First of all, please reply to the original thread and make sure to not
> > drop people or lists from CC.
>
> Sorry this is my first patch and i didn't know that. Now I know.
>
> > For arrays you can use the ARRAY_SIZE() macro if that was the reason for
> > this change. I should have mentioned that when I pointed out that you
> > cannot use strlen().
>
> That wasn't the reason. I just thought it might be better to use u64
> than char[8]. I know why I can't use strlen and that was only careless
> error. And there was lot of them :( I will be more careful next time.
No problem, that's what review is for, and the second version was much
cleaner even if there were still a few issues (some hard to know about,
such as the DMA from stack issue, which is also a very common error).
> > Where did you get these (HID report) values from by the way?
>
> I got them by reverse engineering.
Traffic sniffing?
> >> +
> >> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> >> + enum led_brightness brightness)
> >> +{
> >> + struct gt683r_led *led =
> >> + container_of(led_cdev, struct gt683r_led, led_dev);
> >> +
> >> + mutex_lock(&led->lock);
> >
> > You cannot grab a mutex here since this function can be called from
> > interrupt context (if I remember correctly). Either way, you shouldn't
> > be holding the lock until the work task has finished...
>
> I thought use asked me to put some lock there:
>
> >> +
> >> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> >> + enum led_brightness brightness)
> >> +{
> >> + struct gt683r_led *led =
> >> + container_of(led_cdev, struct gt683r_led, led_dev);
> >> +
> >> + led->brightness = brightness;
> >
> > Missing locking?
I asked if locking was missing and did not specify how you should be
adding it. ;)
In fact, it seems you can get away with not adding any locking here.
Just do the (mutex) locking in gt683r_led_set (or gt683r_led_work).
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] usb: phy: add the Berlin USB PHY driver
On 09/06/2014 at 12:11:57 +0200, Sebastian Hesselbarth wrote : > > Some BG2Q boards hardwired the vbus to be always powered on, we should > > continue > > the probe if vbus gpio is missing. > > Yeah, the same applies for regulators. But with the comments above, it > should move to the controller stub instead. > We should use a regulator and in the case it is hardwired, use a fixed regulator. Then, we can stop if it is missing. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: non-working UVC device 058f:5608
On Mon, 2014-06-09 at 12:27 +0200, Johannes Berg wrote:
> Here we go - log + tracing:
> log: http://p.sipsolutions.net/d5926c43d531e3af.txt
> trace: http://johannes.sipsolutions.net/files/xhci.trace.dat.xz
Oh, and this was the kernel diff to commit
963649d735c8b6eb0f97e82c54f02426ff3f1f45:
diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index eb009a4..00621cb 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -20,6 +20,8 @@
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define DEBUG
+
#include "xhci.h"
#define XHCI_INIT_VALUE 0x0
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 6231ce6..70b09cd 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -20,6 +20,8 @@
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define DEBUG
+
#include
#include
@@ -287,7 +289,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int
slot_id, int suspend)
if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue) {
struct xhci_command *command;
command = xhci_alloc_command(xhci, false, false,
-GFP_NOIO);
+GFP_ATOMIC);
if (!command) {
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_free_command(xhci, cmd);
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8056d90..2ceed51 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -20,6 +20,8 @@
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define DEBUG
+
#include
#include
#include
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
index 1eefc98..4b289d6 100644
--- a/drivers/usb/host/xhci-mvebu.c
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -7,6 +7,8 @@
* version 2 as published by the Free Software Foundation.
*/
+#define DEBUG
+
#include
#include
#include
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index e20520f..aae5dc9 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -20,6 +20,8 @@
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define DEBUG
+
#include
#include
#include
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 29d8adb..2149b0c 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -11,6 +11,8 @@
* version 2 as published by the Free Software Foundation.
*/
+#define DEBUG
+
#include
#include
#include
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d67ff71..a7eda28 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -64,6 +64,8 @@
* endpoint rings; it generates events on the event ring for these.
*/
+#define DEBUG
+
#include
#include
#include "xhci.h"
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2b8d9a2..fd350b7 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -20,6 +20,8 @@
* Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
+#define DEBUG
+
#include
#include
#include
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: non-working UVC device 058f:5608
On Mon, 2014-06-09 at 11:59 +0200, Johannes Berg wrote: > > Johannes, could you enable USB debugging in the linus/master kernel and > > provide a kernel log ? > I'll try to get some logs (wasn't there tracing added to xhci too? will > check) Here we go - log + tracing: log: http://p.sipsolutions.net/d5926c43d531e3af.txt trace: http://johannes.sipsolutions.net/files/xhci.trace.dat.xz I plugged in the device, waited a bit, tried to run a camera application (didn't work) and then ran lsusb -t and lsusb -v. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3] tools: ffs-test: add compatibility code for old kernels
If ffs-test is used with a kernel prior to 3.14, which do not
support the new descriptors format, it will fail when trying to
write the descriptors. Add a function that converts the new
descriptors to the legacy ones and use it to retry writing the
descriptors using the legacy format.
Also add “-l” flag to ffs-test which will cause the tool to
never try the new format and instead immediatelly try the
legacy one. This should be useful to test whether parsing
of the old format still works on given 3.14+ kernel.
---
tools/usb/ffs-test.c | 105 ---
1 file changed, 100 insertions(+), 5 deletions(-)
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index 708d317..afd69ea 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -172,6 +173,82 @@ static const struct {
},
};
+static size_t descs_to_legacy(void **legacy, const void *descriptors) {
+ __u32 length, flags, fs_count = 0, hs_count = 0, count;
+ const unsigned char *descs, *usb_descs;
+
+ /* Read v2 header */
+ {
+ const struct usb_functionfs_descs_head_v2 *head = descriptors;
+
+ if (le32_to_cpu(head->magic) !=
+ FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
+ return 0;
+ length = le32_to_cpu(head->length);
+ if (length <= sizeof *head)
+ return 0;
+ flags = le32_to_cpu(head->flags);
+ descs = (const void *)(head + 1);
+ length -= sizeof *head;
+ }
+
+ /* Read v2 counts */
+#define GET_LE32(ret) do { \
+ if (length < 4) \
+ return 0; \
+ ret = le32_to_cpu(*(const __le32 *)descs); \
+ descs += 4; \
+ length -= 4;\
+ } while (0)
+
+ if (flags & FUNCTIONFS_HAS_FS_DESC)
+ GET_LE32(fs_count);
+ if (flags & FUNCTIONFS_HAS_HS_DESC)
+ GET_LE32(hs_count);
+ if (!fs_count && !hs_count)
+ return 0;
+ if (flags & FUNCTIONFS_HAS_SS_DESC)
+ /* Legacy does not support SS so this is ignored. */
+ GET_LE32(count);
+ if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
+ FUNCTIONFS_HAS_SS_DESC))
+ return 0;
+
+#undef GET_LE32
+
+ /*
+* Find the end of FS and HS USB descriptors. SS descriptors
+* are ignored since legacy format does not support them.
+*/
+ count = fs_count + hs_count;
+ usb_descs = descs;
+ do {
+ if (length < *descs)
+ return 0;
+ length -= *descs;
+ descs += *descs;
+ } while (--count);
+
+ /* Allocate legacy descriptors and copy the data. */
+ {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+ struct usb_functionfs_descs_head *head;
+#pragma GCC diagnostic pop
+
+ length = sizeof *head + (descs - usb_descs);
+ head = malloc(length);
+ head->magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
+ head->length = cpu_to_le32(length);
+ head->fs_count = cpu_to_le32(fs_count);
+ head->hs_count = cpu_to_le32(hs_count);
+ memcpy(head + 1, usb_descs, descs - usb_descs);
+ *legacy = head;
+ }
+
+ return length;
+}
+
#define STR_INTERFACE_ "Source/Sink"
@@ -491,12 +568,29 @@ ep0_consume(struct thread *ignore, const void *buf,
size_t nbytes)
return nbytes;
}
-static void ep0_init(struct thread *t)
+static void ep0_init(struct thread *t, bool legacy_descriptors)
{
+ void *legacy;
ssize_t ret;
+ size_t len;
+
+ if (legacy_descriptors) {
+ info("%s: writing descriptors\n", t->filename);
+ goto legacy;
+ }
- info("%s: writing descriptors\n", t->filename);
+ info("%s: writing descriptors (in v2 format)\n", t->filename);
ret = write(t->fd, &descriptors, sizeof descriptors);
+
+ if (ret < 0 && errno == EINVAL) {
+ warn("%s: new format rejected, trying legacy\n", t->filename);
+legacy:
+ len = descs_to_legacy(&legacy, &descriptors);
+ if (len) {
+ ret = write(t->fd, legacy, len);
+ free(legacy);
+ }
+ }
die_on(ret < 0, "%s: write: descriptors", t->filename);
info("%s: writing strings\n", t->filename);
@@ -507,14 +601,15 @@ static void ep0_init(struct thread *t)
/ Main
Re: [PATCH 0/9] ARM: Berlin: USB support
On 06/09/2014 06:30 AM, Peter Chen wrote: > On Thu, Jun 05, 2014 at 05:48:37PM +0200, Antoine Ténart wrote: >> This series adds the support for the Marvell Berlin USB controllers, >> the USB PHYs and also adds a reset controller. >> >> The reset controller is used by the USB PHY driver and shares the >> existing chip controller node with the clocks and one pin controller. >> >> The Marvell Berlin USB controllers are host only on the BG2Q and are >> compatible with USB ChipIdea. We here add a glue to use the available >> common functions for this kind of controllers. A USB PHY driver is also >> added to control the PHY. [...] > > I am fine with 6/9, 7/9, will queue them if no other objections. > Thanks Peter, we are early in the cycle and I feel there will be a v2 of the chipidea stub. IMHO, controlling the vbus regulator should not be business of the phy driver, so ci will have to deal with it. Anyways, good to know you are fine with it already! Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] usb: phy: add the Berlin USB PHY driver
On 06/09/2014 10:26 AM, Jisheng Zhang wrote:
> Dear Sebastian and Antoine,
>
> On Fri, 6 Jun 2014 03:54:06 -0700
> Sebastian Hesselbarth wrote:
>
>>> +
>>> +#define to_berlin_phy_priv(p) container_of((p), struct
>>> berlin_phy_priv, phy) +
>>> +struct berlin_phy_priv {
>>> + void __iomem*base;
>>> + struct usb_phy phy;
>>> + struct reset_control*rst_ctrl;
>>> + int pwr_gpio;
>>
>> Is the GPIO used for USB power? If so, we should not rely on
>
> The GPIO is used for vbus. Sorry for using the confusing "pwr". Do we still
> need to use regulator API?
Yes, I guess using regulator is still the way to go. Also, I think
it should be up to the controller to power on/off the device. That
way, we could make use of the dual role controller features on BG2
and BG2CD.
>> GPIO at all but use regulator API. Thinking of Chromecast which
>> is externally powered over USB, there will be no regulator nor
>> GPIO at all.
>>
>>> +};
>>> +
>>> +static int berlin_phy_init(struct usb_phy *phy)
>>> +{
>>> + struct berlin_phy_priv *priv = to_berlin_phy_priv(phy);
>>> + int ret;
>>> +
>>> + reset_control_reset(priv->rst_ctrl);
>>> +
>>> + writel(CLK_REF_DIV(0xc) | FEEDBACK_CLK_DIV(0x54),
>>> + priv->base + USB_PHY_PLL);
>>
>> @Jisheng: IIRC the dividers above are different for BG2? Can you please
>> evaluate?
>
> Yes, BG2 uses different refdiv and fbdiv. Is there any suggestions about how
> to
> handle this difference? The value is chosen after carefully tunning
I guess it depends on how many different div values you expect for
berlin2 usb PHYs. If it is just the two, we can go with different
compatibles for e.g. "berlin2-usb-phy" and "berlin2cd-usb-phy".
If you know of more PHYs with different div, a corresponding vendor-
specific property should do the trick, e.g.
marvell,pll-divider = <0x54c0>;
I am fine with both.
>>
>>> + writel(CLK_STABLE | PLL_CTRL_REG | PHASE_OFF_TOL_250 |
>>> KVC0_REG_CTRL |
>>> + CLK_BLK_EN, priv->base + USB_PHY_PLL_CONTROL);
>>> + writel(V2I_VCO_RATIO(0x5) | R_ROTATE_0 | ANA_TEST_DC_CTRL(0x5),
>>> + priv->base + USB_PHY_ANALOG);
>>> + writel(PHASE_FREEZE_DLY_4_CL | ACK_LENGTH_16_CL | SQ_LENGTH_12 |
>>> + DISCON_THRESHOLD_260 | SQ_THRESHOLD(0xa) | LPF_COEF(0x2) |
>>> + INTPL_CUR_30, priv->base + USB_PHY_RX_CTRL);
>>> +
>>> + writel(TX_VDD12_13 | TX_OUT_AMP(0x3), priv->base +
>>> USB_PHY_TX_CTRL1);
>>> + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) |
>>> EXT_RS_RCAL_DIV(0x4),
>>> + priv->base + USB_PHY_TX_CTRL0);
>>> +
>>> + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) |
>>> EXT_RS_RCAL_DIV(0x4) |
>>> + EXT_FS_RCAL_DIV(0x2), priv->base + USB_PHY_TX_CTRL0);
>>> +
>>> + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) |
>>> EXT_RS_RCAL_DIV(0x4),
>>> + priv->base + USB_PHY_TX_CTRL0);
>>> + writel(TX_CHAN_CTRL_REG(0xf) | DRV_SLEWRATE(0x3) |
>>> IMP_CAL_FS_HS_DLY_3 |
>>> + FS_DRV_EN_MASK(0xd), priv->base + USB_PHY_TX_CTRL2);
>>> +
>>> + ret = gpio_direction_output(priv->pwr_gpio, 0);
>>
>> As mentioned above, this should be using regulator API. And also, if
>> there is no dummy regulator allowed, it should be optional.
>>
>>> + if (ret)
>>> + return ret;
>>> +
>>> + gpio_set_value(priv->pwr_gpio, 1);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void berlin_phy_shutdown(struct usb_phy *phy)
>>> +{
>>> + struct berlin_phy_priv *priv = to_berlin_phy_priv(phy);
>>> +
>>> + gpio_set_value(priv->pwr_gpio, 0);
>>> +}
>>> +
>>> +static int berlin_phy_probe(struct platform_device *pdev)
>>> +{
>>> + struct device_node *np = pdev->dev.of_node;
>>> + struct berlin_phy_priv *priv;
>>> + struct resource *res;
>>> + int ret, gpio;
>>> +
>>> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + priv->base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(priv->base))
>>> + return PTR_ERR(priv->base);
>>> +
>>> + priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
>>> + if (IS_ERR(priv->rst_ctrl)) {
>>> + ret = PTR_ERR(priv->rst_ctrl);
>>> + dev_err(&pdev->dev, "cannot get reset controller: %d\n",
>>> ret);
>>
>> Hmm, considering a non arch_init call registered reset driver, it does
>> also spit out an error for -EPROBE_DEFER, does it?
>>
>>> + return ret;
>>> + }
>>> +
>>> + gpio = of_get_named_gpio(np, "power-gpio", 0);
>>> + if (!gpio_is_valid(gpio))
>>> + return gpio;
>
> Some BG2Q boards hardwired the vbus to be always powered on, we should
> continue
> the probe if vbus gpio is missing.
Yeah, the same applies for regulators. But with the comments above, it
should move to the controller stub instead.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to ma
Re: non-working UVC device 058f:5608
On Mon, 2014-06-09 at 11:25 +0200, Laurent Pinchart wrote: > > Indeed, that works! Interestingly, it works neither on a USB3 port > > directly, nor on a USB2 hub behind the USB3 port. > > I would thus be tempted to classify this as an XHCI controller issue. linux- > usb should be the right list to get help. I've CC'ed Mathias Nyman, the XHCI > maintainer. Yeah, I tend to agree. > Johannes, could you enable USB debugging in the linus/master kernel and > provide a kernel log ? Sure. Note that linus/next is having even more issues with this device, to the point where I couldn't even get the lsusb I pasted into the first email. I used 3.13 (because I had it installed on the system in question) to get that. It was also throwing an autosuspend warning: http://mid.gmane.org/1402177014.8442.1.ca...@jlt4.sipsolutions.net I'll try to get some logs (wasn't there tracing added to xhci too? will check) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hardware bug in Intel USB-2 hub?
On 06/19/2013 09:03 PM, Alan Stern wrote: > Sarah: > > This report surfaced in Bugzilla #59011 (see especially comments #38 > and #39). Toralf reports, among other things, that the integrated > "rate-matching" hub in his ThinkPad T420 (6 Series/C200 Series chipset) > isn't behaving the way it should. > > The particular symptom is that when the hub is suspended, it does not > relay wakeup requests from a downstream port to its upstream port, if > the downstream device is connected at low speed and the downstream > port's suspend feature isn't set. This happens with the 3.10-rc > kernels, because commit 0aa2832dd0d9 changed system suspend to do a USB > "global" bus suspend. None of the ports on non-SuperSpeed hubs are > explicitly put into suspend mode; instead, everything on the bus goes > into suspend when the root hub stops sending packets. > > The problem is easy to test on any system running a 3.10-rc kernel. > Simply plug a low-speed USB keyboard (almost all keyboards are low > speed) into a USB-2 port, suspend the system, and then see if typing on > the keyboard will wake up the computer. > > I have tested a couple of external USB-2 hubs; one of them behaved > correctly and one didn't. Regardless, it was surprising to see > Toralf's report that an Intel hub doesn't work right. I don't have any > machines with a comparable chipset, so I can't test one of those > integrated hubs directly. > > Can somebody at Intel look into this? > > Alan Stern > > Just tried kernel 3.15. - issue does still exists https://bugzilla.kernel.org/show_bug.cgi?id=59011 -- Toralf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: non-working UVC device 058f:5608
Hi Johannes, On Monday 09 June 2014 09:33:06 Johannes Berg wrote: > Hi Laurent, > > Thanks for the quick reply! You're welcome. > > > and then the kernel message repeats forever, while I can't even exit > > > uvccapture unless I kill it hard, at which point I get > > > > > > xhci_hcd :00:14.0: Signal while waiting for configure endpoint > > > command > > > usb 1-3.4.4.3: Not enough bandwidth for altsetting 0 > > > > > > from the kernel. > > > > This looks like low-level USB issues, CC'ing the linux-usb mailing list. > > Ok. > > > > Any thoughts? Just to rule out hardware defects I connected it to my > > > windows 7 work machine and it works fine without even installing a > > > driver. > > > > Could you try connecting it to an EHCI controller instead of XHCI on a > > Linux machine ? > > Indeed, that works! Interestingly, it works neither on a USB3 port > directly, nor on a USB2 hub behind the USB3 port. I would thus be tempted to classify this as an XHCI controller issue. linux- usb should be the right list to get help. I've CC'ed Mathias Nyman, the XHCI maintainer. Johannes, could you enable USB debugging in the linus/master kernel and provide a kernel log ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] usb: phy: add the Berlin USB PHY driver
Dear Sebastian and Antoine,
On Fri, 6 Jun 2014 03:54:06 -0700
Sebastian Hesselbarth wrote:
> > +
> > +#define to_berlin_phy_priv(p) container_of((p), struct
> > berlin_phy_priv, phy) +
> > +struct berlin_phy_priv {
> > + void __iomem*base;
> > + struct usb_phy phy;
> > + struct reset_control*rst_ctrl;
> > + int pwr_gpio;
>
> Is the GPIO used for USB power? If so, we should not rely on
The GPIO is used for vbus. Sorry for using the confusing "pwr". Do we still
need to use regulator API?
> GPIO at all but use regulator API. Thinking of Chromecast which
> is externally powered over USB, there will be no regulator nor
> GPIO at all.
>
> > +};
> > +
> > +static int berlin_phy_init(struct usb_phy *phy)
> > +{
> > + struct berlin_phy_priv *priv = to_berlin_phy_priv(phy);
> > + int ret;
> > +
> > + reset_control_reset(priv->rst_ctrl);
> > +
> > + writel(CLK_REF_DIV(0xc) | FEEDBACK_CLK_DIV(0x54),
> > + priv->base + USB_PHY_PLL);
>
> @Jisheng: IIRC the dividers above are different for BG2? Can you please
> evaluate?
Yes, BG2 uses different refdiv and fbdiv. Is there any suggestions about how to
handle this difference? The value is chosen after carefully tunning
>
> > + writel(CLK_STABLE | PLL_CTRL_REG | PHASE_OFF_TOL_250 |
> > KVC0_REG_CTRL |
> > + CLK_BLK_EN, priv->base + USB_PHY_PLL_CONTROL);
> > + writel(V2I_VCO_RATIO(0x5) | R_ROTATE_0 | ANA_TEST_DC_CTRL(0x5),
> > + priv->base + USB_PHY_ANALOG);
> > + writel(PHASE_FREEZE_DLY_4_CL | ACK_LENGTH_16_CL | SQ_LENGTH_12 |
> > + DISCON_THRESHOLD_260 | SQ_THRESHOLD(0xa) | LPF_COEF(0x2) |
> > + INTPL_CUR_30, priv->base + USB_PHY_RX_CTRL);
> > +
> > + writel(TX_VDD12_13 | TX_OUT_AMP(0x3), priv->base +
> > USB_PHY_TX_CTRL1);
> > + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) |
> > EXT_RS_RCAL_DIV(0x4),
> > + priv->base + USB_PHY_TX_CTRL0);
> > +
> > + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) |
> > EXT_RS_RCAL_DIV(0x4) |
> > + EXT_FS_RCAL_DIV(0x2), priv->base + USB_PHY_TX_CTRL0);
> > +
> > + writel(EXT_HS_RCAL_EN | IMPCAL_VTH_DIV(0x3) |
> > EXT_RS_RCAL_DIV(0x4),
> > + priv->base + USB_PHY_TX_CTRL0);
> > + writel(TX_CHAN_CTRL_REG(0xf) | DRV_SLEWRATE(0x3) |
> > IMP_CAL_FS_HS_DLY_3 |
> > + FS_DRV_EN_MASK(0xd), priv->base + USB_PHY_TX_CTRL2);
> > +
> > + ret = gpio_direction_output(priv->pwr_gpio, 0);
>
> As mentioned above, this should be using regulator API. And also, if
> there is no dummy regulator allowed, it should be optional.
>
> > + if (ret)
> > + return ret;
> > +
> > + gpio_set_value(priv->pwr_gpio, 1);
> > +
> > + return 0;
> > +}
> > +
> > +static void berlin_phy_shutdown(struct usb_phy *phy)
> > +{
> > + struct berlin_phy_priv *priv = to_berlin_phy_priv(phy);
> > +
> > + gpio_set_value(priv->pwr_gpio, 0);
> > +}
> > +
> > +static int berlin_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np = pdev->dev.of_node;
> > + struct berlin_phy_priv *priv;
> > + struct resource *res;
> > + int ret, gpio;
> > +
> > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + priv->base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(priv->base))
> > + return PTR_ERR(priv->base);
> > +
> > + priv->rst_ctrl = devm_reset_control_get(&pdev->dev, NULL);
> > + if (IS_ERR(priv->rst_ctrl)) {
> > + ret = PTR_ERR(priv->rst_ctrl);
> > + dev_err(&pdev->dev, "cannot get reset controller: %d\n",
> > ret);
>
> Hmm, considering a non arch_init call registered reset driver, it does
> also spit out an error for -EPROBE_DEFER, does it?
>
> > + return ret;
> > + }
> > +
> > + gpio = of_get_named_gpio(np, "power-gpio", 0);
> > + if (!gpio_is_valid(gpio))
> > + return gpio;
Some BG2Q boards hardwired the vbus to be always powered on, we should continue
the probe if vbus gpio is missing.
Thanks,
Jisheng
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels
Hi Michal,
> -Original Message-
> From: Michal Nazarewicz [mailto:min...@mina86.com]
> Sent: Friday, June 06, 2014 11:13 AM
> To: Felipe Balbi; Krzysztof Opasiak
> Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Michal
> Nazarewicz
> Subject: [PATCHv2 5/5] tools: ffs-test: add compatibility code for
> old kernels
(... snip ...)
> +static size_t descs_to_legacy(void **legacy, const void
> *descriptors) {
> + __u32 length, flags, fs_count = 0, hs_count = 0, count;
> + const unsigned char *descs = descriptors, *usb_descs;
> + unsigned char *out;
> +
> + if (get_unaligned_le32(descs) !=
> FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
> + return 0;
> +
> + length = get_unaligned_le32(descs + 4);
> + if (length < 8)
> + return 0;
> + descs += 8;
> + length -= 8;
> +
> +#define GET_LE32(ret) do { \
> + if (length < 4) \
> + return 0; \
> + ret = get_unaligned_le32(descs);\
> + descs += 4; \
> + length -= 4;\
> + } while (0)
> +
> + GET_LE32(flags);
> + if (flags & FUNCTIONFS_HAS_FS_DESC)
> + GET_LE32(fs_count);
> + if (flags & FUNCTIONFS_HAS_HS_DESC)
> + GET_LE32(hs_count);
> + if (!fs_count && !hs_count)
> + return 0;
> + if (flags & FUNCTIONFS_HAS_SS_DESC)
> + GET_LE32(count); /* The value is ignored later on
> anyway. */
> + if (flags)
> + return 0;
As far as I understand you are taking the flags from descriptor and then test
them with possible ffs speed flags, after getting flags your are not assigning
anything to this variable so in this place it will be != 0 unless none of the
flags has been provided. I don't think this is intended behavior.
> +
> +#undef GET_LE32
> +
> + usb_descs = descs;
> + for (count = fs_count + hs_count; count; --count) {
> + if (length < *descs)
> + return 0;
> + length -= *descs;
> + descs += *descs;
> + }
> +
> + length = 16 + (descs - usb_descs);
> + out = malloc(length);
> + put_unaligned_le32(FUNCTIONFS_DESCRIPTORS_MAGIC, out);
> + put_unaligned_le32(length, out + 4);
> + put_unaligned_le32(fs_count, out + 8);
> + put_unaligned_le32(hs_count, out + 12);
> + memcpy(out + 16, usb_descs, length - 16);
> + *legacy = out;
> + return length;
> +}
> +
As this is an example which will be copy-paste by many users maybe you should
you struct usb_functionfs_descs_head and struct usb_functionfs_descs_head_v2
instead of direct operations using hard-coded offsets to make this function
more readable?
--
BR's
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opas...@samsung.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: non-working UVC device 058f:5608
Hi Laurent, Thanks for the quick reply! > > and then the kernel message repeats forever, while I can't even exit > > uvccapture unless I kill it hard, at which point I get > > > > xhci_hcd :00:14.0: Signal while waiting for configure endpoint command > > usb 1-3.4.4.3: Not enough bandwidth for altsetting 0 > > > > from the kernel. > > This looks like low-level USB issues, CC'ing the linux-usb mailing list. Ok. > > Any thoughts? Just to rule out hardware defects I connected it to my > > windows 7 work machine and it works fine without even installing a > > driver. > > Could you try connecting it to an EHCI controller instead of XHCI on a Linux > machine ? Indeed, that works! Interestingly, it works neither on a USB3 port directly, nor on a USB2 hub behind the USB3 port. Thanks, johannes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
