Re: [PATCH RESEND 0/2] usb: dwc3: gadget: Bug fixes
On 1/13/2015 12:10 PM, Amit VIRDI wrote: This is a re-submission of patches [1/4] and [2/4] from: http://www.spinics.net/lists/linux-usb/msg118841.html Commit log of both these patches has been modified for aided clarity. These patches have been rebased on Balbi's testing/next. Patches [3/4] and [4/4] were accepted as they were. Amit Virdi (2): usb: dwc3: gadget: Fix TRB preparation during SG usb: dwc3: gadget: Stop TRB preparation after limit is reached drivers/usb/dwc3/gadget.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) I was using older git version, so git failed to send patches to sta...@vger.kernel.org Please discard this patchset. I'm resending with updated git -- 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 RESEND(2) 0/2] usb: dwc3: gadget: Bug fixes
This is a re-submission of patches [1/4] and [2/4] from: http://www.spinics.net/lists/linux-usb/msg118841.html Commit log of both these patches has been modified for aided clarity. These patches have been rebased on Balbi's testing/next. Patches [3/4] and [4/4] were accepted as they were. Amit Virdi (2): usb: dwc3: gadget: Fix TRB preparation during SG usb: dwc3: gadget: Stop TRB preparation after limit is reached drivers/usb/dwc3/gadget.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) -- 1.8.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND(2) 2/2] usb: dwc3: gadget: Stop TRB preparation after limit is reached
DWC3 gadget sets up a pool of 32 TRBs for each EP during initialization. This
means, the max TRBs that can be submitted for an EP is fixed to 32. Since the
request queue for an EP is a linked list, any number of requests can be queued
to it by the gadget layer. However, the dwc3 driver must not submit TRBs more
than the pool it has created for. This limit wasn't respected when SG was used
resulting in submitting more than the max TRBs, eventually leading to
non-transfer of the TRBs submitted over the max limit.
Root cause:
When SG is used, there are two loops iterating to prepare TRBs:
- Outer loop over the request_list
- Inner loop over the SG list
The code was missing break to get out of the outer loop.
Signed-off-by: Amit Virdi
Cc: # v3.9+
---
drivers/usb/dwc3/gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cb8939134c32..6c5e344822b9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -897,6 +897,9 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool
starting)
if (last_one)
break;
}
+
+ if (last_one)
+ break;
} else {
dma = req->request.dma;
length = req->request.length;
--
1.8.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND(2) 1/2] usb: dwc3: gadget: Fix TRB preparation during SG
When scatter gather (SG) is used, multiple TRBs are prepared from one DWC3
request (dwc3_request). So while preparing TRBs, the 'last' flag should be set
only when it is the last TRB being prepared from the last dwc3_request entry.
The current implementation uses list_is_last to check if the dwc3_request is the
last entry from the request_list. However, list_is_last returns false for the
last entry too. This is because, while preparing the first TRB from a request,
the function dwc3_prepare_one_trb modifies the request's next and prev pointers
while moving the URB to req_queued. Hence, list_is_last always returns false no
matter what.
The correct way is not to access the modified pointers of dwc3_request but to
use list_empty macro instead.
Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix scatter
gather implementation"
Signed-off-by: Amit Virdi
Cc: # v3.9+
---
drivers/usb/dwc3/gadget.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4e2593993fae..cb8939134c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -879,8 +879,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool
starting)
if (i == (request->num_mapped_sgs - 1) ||
sg_is_last(s)) {
- if (list_is_last(&req->list,
- &dep->request_list))
+ if (list_empty(&dep->request_list))
last_one = true;
chain = false;
}
--
1.8.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH next] usb: gadget: uvc: to_uvcg_control_header() can be static
drivers/usb/gadget/function/uvc_configfs.c:46:28: sparse: symbol
'to_uvcg_control_header' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:138:25: sparse: symbol
'uvcg_control_header_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:164:6: sparse: symbol
'uvcg_control_header_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:721:20: sparse: symbol
'to_uvcg_format' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:798:30: sparse: symbol
'to_uvcg_streaming_header' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:950:25: sparse: symbol
'uvcg_streaming_header_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:976:6: sparse: symbol
'uvcg_streaming_header_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1020:19: sparse: symbol
'to_uvcg_frame' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1265:25: sparse: symbol
'uvcg_frame_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1315:6: sparse: symbol
'uvcg_frame_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1338:26: sparse: symbol
'to_uvcg_uncompressed' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1548:25: sparse: symbol
'uvcg_uncompressed_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1586:6: sparse: symbol
'uvcg_uncompressed_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1610:19: sparse: symbol
'to_uvcg_mjpeg' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1761:25: sparse: symbol
'uvcg_mjpeg_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1793:6: sparse: symbol
'uvcg_mjpeg_drop' was not declared. Should it be static?
Signed-off-by: Fengguang Wu
---
uvc_configfs.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc_configfs.c
b/drivers/usb/gadget/function/uvc_configfs.c
index 33d92ab..f69f47a 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -43,7 +43,7 @@ struct uvcg_control_header {
unsignedlinked;
};
-struct uvcg_control_header *to_uvcg_control_header(struct config_item *item)
+static struct uvcg_control_header *to_uvcg_control_header(struct config_item
*item)
{
return container_of(item, struct uvcg_control_header, item);
}
@@ -135,7 +135,7 @@ static struct configfs_attribute
*uvcg_control_header_attrs[] = {
NULL,
};
-struct config_item_type uvcg_control_header_type = {
+static struct config_item_type uvcg_control_header_type = {
.ct_item_ops= &uvcg_control_header_item_ops,
.ct_attrs = uvcg_control_header_attrs,
.ct_owner = THIS_MODULE,
@@ -161,7 +161,7 @@ static struct config_item *uvcg_control_header_make(struct
config_group *group,
return &h->item;
}
-void uvcg_control_header_drop(struct config_group *group,
+static void uvcg_control_header_drop(struct config_group *group,
struct config_item *item)
{
struct uvcg_control_header *h = to_uvcg_control_header(item);
@@ -718,7 +718,7 @@ struct uvcg_format {
__u8bmaControls[UVCG_STREAMING_CONTROL_SIZE];
};
-struct uvcg_format *to_uvcg_format(struct config_item *item)
+static struct uvcg_format *to_uvcg_format(struct config_item *item)
{
return container_of(to_config_group(item), struct uvcg_format, group);
}
@@ -795,7 +795,7 @@ struct uvcg_streaming_header {
unsignednum_fmt;
};
-struct uvcg_streaming_header *to_uvcg_streaming_header(struct config_item
*item)
+static struct uvcg_streaming_header *to_uvcg_streaming_header(struct
config_item *item)
{
return container_of(item, struct uvcg_streaming_header, item);
}
@@ -947,7 +947,7 @@ static struct configfs_attribute
*uvcg_streaming_header_attrs[] = {
NULL,
};
-struct config_item_type uvcg_streaming_header_type = {
+static struct config_item_type uvcg_streaming_header_type = {
.ct_item_ops= &uvcg_streaming_header_item_ops,
.ct_attrs = uvcg_streaming_header_attrs,
.ct_owner = THIS_MODULE,
@@ -973,7 +973,7 @@ static struct config_item
return &h->item;
}
-void uvcg_streaming_header_drop(struct config_group *group,
+static void uvcg_streaming_header_drop(struct config_group *group,
struct config_item *item)
{
struct uvcg_streaming_header *h = to_uvcg_streaming_header(item);
@@ -1017,7 +1017,7 @@ struct uvcg_frame {
[next:master 2818/3104] drivers/usb/gadget/function/f_uvc.c:816:41: sparse: incorrect type in assignment (different base types)
tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master head: 8fb4ab29bd8400e0060662208573d26b9d91656d commit: 46919a23ee87bbc4eeb6d958471174e26836f0e1 [2818/3104] usb: gadget: uvc: configfs support in uvc function reproduce: # apt-get install sparse git checkout 46919a23ee87bbc4eeb6d958471174e26836f0e1 make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) drivers/usb/gadget/function/f_uvc.c:50:9: sparse: advancing past deep designator drivers/usb/gadget/function/f_uvc.c:113:35: sparse: incorrect type in initializer (different base types) drivers/usb/gadget/function/f_uvc.c:113:35:expected unsigned short [unsigned] [usertype] wMaxTransferSize drivers/usb/gadget/function/f_uvc.c:113:35:got restricted __le16 [usertype] drivers/usb/gadget/function/f_uvc.c:575:42: sparse: incorrect type in assignment (different base types) drivers/usb/gadget/function/f_uvc.c:575:42:expected unsigned short [unsigned] [usertype] wTotalLength drivers/usb/gadget/function/f_uvc.c:575:42:got restricted __le16 [usertype] drivers/usb/gadget/function/f_uvc.c:589:44: sparse: incorrect type in assignment (different base types) drivers/usb/gadget/function/f_uvc.c:589:44:expected unsigned short [unsigned] [usertype] wTotalLength drivers/usb/gadget/function/f_uvc.c:589:44:got restricted __le16 [usertype] >> drivers/usb/gadget/function/f_uvc.c:816:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/f_uvc.c:816:41:expected unsigned short [unsigned] [usertype] wTerminalType drivers/usb/gadget/function/f_uvc.c:816:41:got restricted __le16 [usertype] >> drivers/usb/gadget/function/f_uvc.c:819:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/f_uvc.c:819:41:expected unsigned short [unsigned] [usertype] wObjectiveFocalLengthMin drivers/usb/gadget/function/f_uvc.c:819:41:got restricted __le16 [usertype] >> drivers/usb/gadget/function/f_uvc.c:820:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/f_uvc.c:820:41:expected unsigned short [unsigned] [usertype] wObjectiveFocalLengthMax drivers/usb/gadget/function/f_uvc.c:820:41:got restricted __le16 [usertype] >> drivers/usb/gadget/function/f_uvc.c:821:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/f_uvc.c:821:41:expected unsigned short [unsigned] [usertype] wOcularFocalLength drivers/usb/gadget/function/f_uvc.c:821:41:got restricted __le16 [usertype] >> drivers/usb/gadget/function/f_uvc.c:833:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/f_uvc.c:833:41:expected unsigned short [unsigned] [usertype] wMaxMultiplier drivers/usb/gadget/function/f_uvc.c:833:41:got restricted __le16 [usertype] >> drivers/usb/gadget/function/f_uvc.c:844:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/f_uvc.c:844:41:expected unsigned short [unsigned] [usertype] wTerminalType drivers/usb/gadget/function/f_uvc.c:844:41:got restricted __le16 [usertype] -- >> drivers/usb/gadget/function/uvc_configfs.c:46:28: sparse: symbol >> 'to_uvcg_control_header' was not declared. Should it be static? >> drivers/usb/gadget/function/uvc_configfs.c:124:1: sparse: cast to restricted >> __le16 >> drivers/usb/gadget/function/uvc_configfs.c:124:1: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/uvc_configfs.c:124:1:expected unsigned short [unsigned] [usertype] bcdUVC drivers/usb/gadget/function/uvc_configfs.c:124:1:got restricted __le16 [usertype] >> drivers/usb/gadget/function/uvc_configfs.c:127:1: sparse: cast to restricted >> __le32 >> drivers/usb/gadget/function/uvc_configfs.c:127:1: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/uvc_configfs.c:127:1:expected unsigned int [unsigned] [usertype] dwClockFrequency drivers/usb/gadget/function/uvc_configfs.c:127:1:got restricted __le32 [usertype] >> drivers/usb/gadget/function/uvc_configfs.c:138:25: sparse: symbol >> 'uvcg_control_header_type' was not declared. Should it be static? >> drivers/usb/gadget/function/uvc_configfs.c:156:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/uvc_configfs.c:156:41:expected unsigned short [unsigned] [usertype] bcdUVC drivers/usb/gadget/function/uvc_configfs.c:156:41:got restricted __le16 [usertype] >> drivers/usb/gadget/function/uvc_configfs.c:157:41: sparse: incorrect type in >> assignment (different base types) drivers/usb/gadget/function/uvc_configfs.c:157:41:expected unsigned int [unsigned] [usertype] dwClockFrequency drivers/usb/gadge
[PATCH v2 0/4] usb: udc: Unify dp control
Hi Felipe, This is the follow-up: http://marc.info/?l=linux-usb&m=140979297227434&w=2. Sorry for late. In the first two patches, we introduce two internal APIs which are used to find corresponding udc according to usb_gadget or usb_gadget_driver, it can simplify the code structure. In the 3rd patch, it maintains flag 'vbus' as vbus status, and try to connect or disconnect gadget according to vbus status. In the 4th patch, it maintains count 'deactivations' for connect or disconnect gadget according to function/user behavior. If you agree with handing dp like above way, I will have two patchset, one for udc driver for 'vbus' and another for function driver for 'deactivations'. Changes for v2: - introduce two internal APIs to simplify the code structure. - Teach patch 3/4 and 4/4 to use the new introduced API. Peter Chen (4): usb: udc: add usb_gadget_find_udc usb: udc: add udc_driver_find_udc usb: udc: add usb_udc_vbus_handler usb: udc: add usb_udc_activation_handler drivers/usb/gadget/udc/udc-core.c | 144 -- include/linux/usb/gadget.h| 9 +++ 2 files changed, 116 insertions(+), 37 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] usb: udc: add usb_udc_vbus_handler
This commit updates udc core vbus status, and try to connect
or disconnect gadget.
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc/udc-core.c | 34 +-
include/linux/usb/gadget.h| 4
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 7f8dc5b..453ff14 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -35,6 +35,8 @@
* @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
+ * @vbus - for udcs who care about vbus status, this value is real vbus status;
+ * for udcs who do not care about vbus status, this value is always true
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -44,6 +46,7 @@ struct usb_udc {
struct usb_gadget *gadget;
struct device dev;
struct list_headlist;
+ boolvbus;
};
static struct class *udc_class;
@@ -179,6 +182,34 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
/* - */
+static void usb_udc_connect_control(struct usb_udc *udc)
+{
+ if (udc->vbus)
+ usb_gadget_connect(udc->gadget);
+ else
+ usb_gadget_disconnect(udc->gadget);
+}
+
+/**
+ * usb_udc_vbus_handler - updates the udc core vbus status, and try to
+ * connect or disconnect gadget
+ * @gadget: The gadget which vbus change occurs
+ * @status: The vbus status
+ *
+ * The udc driver calls it when it wants to connect or disconnect gadget
+ * according to vbus status.
+ */
+void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
+{
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
+
+ if (udc) {
+ udc->vbus = status;
+ usb_udc_connect_control(udc);
+ }
+}
+EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
+
/**
* usb_gadget_udc_reset - notifies the udc core that bus reset occurs
* @gadget: The gadget which bus reset occurs
@@ -311,6 +342,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err4;
usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
+ udc->vbus = true;
mutex_unlock(&udc_lock);
@@ -412,7 +444,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct
usb_gadget_driver *dri
driver->unbind(udc->gadget);
goto err1;
}
- usb_gadget_connect(udc->gadget);
+ usb_udc_connect_control(udc);
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
return 0;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 70ddb39..2be007a 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1027,6 +1027,10 @@ extern void usb_gadget_udc_reset(struct usb_gadget
*gadget,
extern void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req);
+/*-*/
+
+/* utility to update vbus status for udc core, it may be scheduled */
+extern void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status);
/*-*/
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] usb: udc: add udc_driver_find_udc
This is an internal API, it can be used to find corresponding udc
according to usb_gadget_driver.
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc/udc-core.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 0156709..7f8dc5b 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -70,6 +70,22 @@ found:
return udc;
}
+static struct usb_udc *udc_driver_find_udc(struct usb_gadget_driver *driver)
+{
+ struct usb_udc *udc = NULL;
+
+ mutex_lock(&udc_lock);
+ list_for_each_entry(udc, &udc_list, list)
+ if (udc->driver == driver)
+ goto found;
+ mutex_unlock(&udc_lock);
+
+ return NULL;
+
+found:
+ mutex_unlock(&udc_lock);
+ return udc;
+}
/* - */
#ifdef CONFIG_HAS_DMA
@@ -469,17 +485,14 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver
*driver)
if (!driver || !driver->unbind)
return -EINVAL;
- mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list)
- if (udc->driver == driver) {
- usb_gadget_remove_driver(udc);
- usb_gadget_set_state(udc->gadget,
- USB_STATE_NOTATTACHED);
- ret = 0;
- break;
- }
+ udc = udc_driver_find_udc(driver);
+ if (udc) {
+ usb_gadget_remove_driver(udc);
+ usb_gadget_set_state(udc->gadget,
+ USB_STATE_NOTATTACHED);
+ ret = 0;
+ }
- mutex_unlock(&udc_lock);
return ret;
}
EXPORT_SYMBOL_GPL(usb_gadget_unregister_driver);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] usb: udc: add usb_udc_activation_handler
During this API, the deactivation count will be update, and it
will try to connect or disconnect gadget. It can be used to
enable functions for gadget.
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc/udc-core.c | 28 +++-
include/linux/usb/gadget.h| 5 +
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 453ff14..f301c5e 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -37,6 +37,7 @@
* @list - for use by the udc class driver
* @vbus - for udcs who care about vbus status, this value is real vbus status;
* for udcs who do not care about vbus status, this value is always true
+ * @deactivations - the deactivation count to connect or disconnect gadget
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -47,6 +48,7 @@ struct usb_udc {
struct device dev;
struct list_headlist;
boolvbus;
+ int deactivations;
};
static struct class *udc_class;
@@ -184,13 +186,37 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
static void usb_udc_connect_control(struct usb_udc *udc)
{
- if (udc->vbus)
+ if (udc->vbus && !udc->deactivations)
usb_gadget_connect(udc->gadget);
else
usb_gadget_disconnect(udc->gadget);
}
/**
+ * usb_udc_activation_handler - updates udc's deactivation count and
+ * try to connect or disconnect
+ *
+ * @gadget: The gadget which the function is at
+ * @active: the function needs to be active or not
+ *
+ * The composite core calls it when it wants to activate or deactivate
+ * function.
+ */
+void usb_udc_activation_handler(struct usb_gadget *gadget, bool active)
+{
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
+
+ if (udc) {
+ if (active)
+ udc->deactivations--;
+ else
+ udc->deactivations++;
+ usb_udc_connect_control(udc);
+ }
+}
+EXPORT_SYMBOL_GPL(usb_udc_activation_handler);
+
+/**
* usb_udc_vbus_handler - updates the udc core vbus status, and try to
* connect or disconnect gadget
* @gadget: The gadget which vbus change occurs
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 2be007a..4d1adea 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1034,6 +1034,11 @@ extern void usb_udc_vbus_handler(struct usb_gadget
*gadget, bool status);
/*-*/
+/* utility to activate or deactive function */
+extern void usb_udc_activation_handler(struct usb_gadget *gadget, bool active);
+
+/*-*/
+
/* utility wrapping a simple endpoint selection policy */
extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] usb: udc: add usb_gadget_find_udc
This is an internal API, and is used to find corresponding udc according
to gadget.
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc/udc-core.c | 51 +++
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index e31d574..0156709 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -52,6 +52,25 @@ static DEFINE_MUTEX(udc_lock);
/* - */
+static struct usb_udc *usb_gadget_find_udc(struct usb_gadget *gadget)
+{
+ struct usb_udc *udc = NULL;
+
+ mutex_lock(&udc_lock);
+ list_for_each_entry(udc, &udc_list, list)
+ if (udc->gadget == gadget)
+ goto found;
+ mutex_unlock(&udc_lock);
+ dev_err(gadget->dev.parent, "gadget not registered.\n");
+
+ return NULL;
+
+found:
+ mutex_unlock(&udc_lock);
+ return udc;
+}
+
+/* - */
#ifdef CONFIG_HAS_DMA
int usb_gadget_map_request(struct usb_gadget *gadget,
@@ -128,21 +147,10 @@ EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
static void usb_gadget_state_work(struct work_struct *work)
{
- struct usb_gadget *gadget = work_to_gadget(work);
- struct usb_udc *udc = NULL;
-
- mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list)
- if (udc->gadget == gadget)
- goto found;
- mutex_unlock(&udc_lock);
-
- return;
-
-found:
- mutex_unlock(&udc_lock);
+ struct usb_udc *udc = usb_gadget_find_udc(work_to_gadget(work));
- sysfs_notify(&udc->dev.kobj, NULL, "state");
+ if (udc)
+ sysfs_notify(&udc->dev.kobj, NULL, "state");
}
void usb_gadget_set_state(struct usb_gadget *gadget,
@@ -348,23 +356,14 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
*/
void usb_del_gadget_udc(struct usb_gadget *gadget)
{
- struct usb_udc *udc = NULL;
-
- mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list)
- if (udc->gadget == gadget)
- goto found;
-
- dev_err(gadget->dev.parent, "gadget not registered.\n");
- mutex_unlock(&udc_lock);
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
- return;
+ if (!udc)
+ return;
-found:
dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
list_del(&udc->list);
- mutex_unlock(&udc_lock);
if (udc->driver)
usb_gadget_remove_driver(udc);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 3/5] usb: dwc2: add generic PHY framework support for dwc2 usb controler platform driver.
Paul Zimmerman writes:
> The patch below fixes it. And it seems like the right thing to me,
> since GPIOs should be optional for a generic phy, I would think. But
> my device tree foo is very weak, so I'm not sure.
>
> CCing Robert, who touched the generic phy code last. Robert, what do
> you think?
I think your patch in [1] is correct, because
"Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt" states that
reset-gpios is optional, and because Felipe told me gpios for phy_generic are
optional.
Now the original code was written by Felipe, so better ask him first. The
original code was :
af9f51c55 phy-generic.c nop->gpio_reset = of_get_named_gpio_flags(node,
"reset-gpios",
af9f51c55 phy-generic.c 0,
&flags);
af9f51c55 phy-generic.c if (nop->gpio_reset == -EPROBE_DEFER)
af9f51c55 phy-generic.c return -EPROBE_DEFER;
Cheers.
--
Robert
[1]
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index dd05254..9a826ff 100644
> --- a/drivers/usb/phy/phy-generic.c
> +++ b/drivers/usb/phy/phy-generic.c
> @@ -218,10 +218,10 @@ int usb_phy_gen_create_phy(struct device *dev, struct
> usb_phy_generic *nop,
> clk_rate = 0;
>
> needs_vcc = of_property_read_bool(node, "vcc-supply");
> - nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
> + nop->gpiod_reset = devm_gpiod_get_optional(dev, "reset-gpios");
> err = PTR_ERR(nop->gpiod_reset);
> if (!err) {
> - nop->gpiod_vbus = devm_gpiod_get(dev,
> + nop->gpiod_vbus = devm_gpiod_get_optional(dev,
>"vbus-detect-gpio");
> err = PTR_ERR(nop->gpiod_vbus);
> }
--
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 1/4] usb: udc: add usb_gadget_find_udc
Peter Chen writes:
> @@ -348,23 +356,14 @@ static void usb_gadget_remove_driver(struct usb_udc
> *udc)
> */
> void usb_del_gadget_udc(struct usb_gadget *gadget)
> {
> - struct usb_udc *udc = NULL;
> -
> - mutex_lock(&udc_lock);
> - list_for_each_entry(udc, &udc_list, list)
> - if (udc->gadget == gadget)
> - goto found;
> -
> - dev_err(gadget->dev.parent, "gadget not registered.\n");
> - mutex_unlock(&udc_lock);
> + struct usb_udc *udc = usb_gadget_find_udc(gadget);
>
> - return;
> + if (!udc)
> + return;
>
> -found:
> dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
>
> list_del(&udc->list);
> - mutex_unlock(&udc_lock);
I believe udc->list linkage to udc_list still should be protected with
udc_lock, otherwise this will race with usb_add_gadget_udc_release().
Regards,
--
Alex
--
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 1/4] usb: udc: add usb_gadget_find_udc
> > @@ -348,23 +356,14 @@ static void usb_gadget_remove_driver(struct
> usb_udc *udc)
> > */
> > void usb_del_gadget_udc(struct usb_gadget *gadget) {
> > - struct usb_udc *udc = NULL;
> > -
> > - mutex_lock(&udc_lock);
> > - list_for_each_entry(udc, &udc_list, list)
> > - if (udc->gadget == gadget)
> > - goto found;
> > -
> > - dev_err(gadget->dev.parent, "gadget not registered.\n");
> > - mutex_unlock(&udc_lock);
> > + struct usb_udc *udc = usb_gadget_find_udc(gadget);
> >
> > - return;
> > + if (!udc)
> > + return;
> >
> > -found:
> > dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
> >
> > list_del(&udc->list);
> > - mutex_unlock(&udc_lock);
>
> I believe udc->list linkage to udc_list still should be protected with
> udc_lock,
> otherwise this will race with usb_add_gadget_udc_release().
>
Thanks, it indeed should be.
Peter
--
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/gadget/net2280: Dont use 0 as NULL pointer
Fix sparse warning Fixes: cb442ee1592d2681 (usb: gadget: udc: net2280: Re-enable dynamic debug messages) Reported-by: kbuild test robot Signed-off-by: Ricardo Ribalda Delgado --- drivers/usb/gadget/udc/net2280.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c index d2fabdc..6411ed8 100644 --- a/drivers/usb/gadget/udc/net2280.c +++ b/drivers/usb/gadget/udc/net2280.c @@ -2287,7 +2287,7 @@ static void handle_ep_small(struct net2280_ep *ep) ep->irqs++; ep_vdbg(ep->dev, "%s ack ep_stat %08x, req %p\n", - ep->ep.name, t, req ? &req->req : 0); + ep->ep.name, t, req ? &req->req : NULL); if (!ep->is_in || (ep->dev->quirks & PLX_2280)) writel(t & ~BIT(NAK_OUT_PACKETS), &ep->regs->ep_stat); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dwc2: problems with IN requests completion in linux-next
On 01/12/2015 10:35 PM, Paul Zimmerman wrote: >> From: Robert Baldyga [mailto:r.bald...@samsung.com] >> Sent: Monday, December 22, 2014 6:13 AM >> >> I have recently noticed problem with DWC2 driver in latest linux-next. I >> use it in gadget only mode at Samsung platform (Odroid U3) but I believe >> the bug can be reproduced at another platforms. >> >> While running FFS example (tools/usb/ffs-aio-example/simple/) the >> communication breaks after few seconds. It's because one of IN requests >> enqueued in DWC2 driver do not complete. At USB analyzer I see that USB >> device started to transmit data from this request, but it ended incomplete. >> >> I bisected kernel tree, and I got following patches are reason of break. >> >> 941fcce usb: dwc2: Update the gadget driver to use common dwc2_hsotg >> structure >> 11b usb: dwc2: Move gadget probe function into platform code >> bcc0607 usb: dwc2: convert to use dev_pm_ops API >> 510ffaa usb: dwc2: Initialize the USB core for peripheral mode >> db8178c usb: dwc2: Update common interrupt handler to call gadget >> interrupt handler >> 8d736d8 usb: dwc2: gadget: Do not fail probe if there isn't a clock node >> >> Patch 941fcce breaks DWC2 driver at my platform and it starts to work >> from 8d736d8 but it has described bug. >> >> I will try to localize reason of this issue. > > Hi Robert, > > I think the most likely suspect would be db8178c, since the rest appear > to be init-time things. It seems to revert cleanly, can you try > reverting just that patch and see if it helps? > Hi Paul, Thanks. It looks like you're right, commit db8178c causes the break. I have found out that if we don't call dwc2_is_controller_alive() in interrupt handler everything works well. Conclusion is that reading from GSNPSID causes the problem. BTW it's strange that this register is used for testing if controller is alive. Documentation says nothing about that as well as it does not say that reading this register can affect hardware behaviour. Avoiding to read this register in device mode seems to be the simplest solution. But the question is why does it behave this way? Best regards, Robert Baldyga -- 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 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode
Hi Felipe, On 1 December 2014 at 11:09, Kiran Raparthy wrote: > Hi Felipe, > > On 25 November 2014 at 20:15, Felipe Balbi wrote: >> On Tue, Nov 25, 2014 at 07:06:18AM +, Peter Chen wrote: >>> >>> > >>> > usb: phy: hold wakeupsource when USB is enumerated in peripheral mode >>> > >>> > Some systems require a mechanism to prevent system to enter into suspend >>> > state when USB is connected and enumerated in peripheral mode. >>> > >>> > This patch provides an interface to hold a wakeupsource to prevent >>> > suspend. >>> > PHY drivers can use this interface when USB is connected and enumerated in >>> > peripheral mode. >>> > >>> > A timed wakeupsource is temporarily held on USB disconnect events, to >>> > allow >>> > the rest of the system to react to the USB disconnection (dropping host >>> > sessions, updating charger status, etc.) prior to re-allowing suspend. >>> > >>> >>> Hi Kiran & Felipe, >>> >>> Just two questions for this series >>> >>> - Will it be the default behavior for all peripheral drivers? >>> - If the peripheral driver's PHY driver does not vbus event, how to >>> support it? >>> For example, chipidea udc driver has its vbus interface at its >>> controller register. >> >> hmm, good point. Since it's so late, I'll just go ahead and drop >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so >> we have some more time to discuss these details. > > I am just curious to know/understand why this feature needs to be > default behavior for all peripheral drivers? > If this needs to be default behavior, could you please suggest any > alternate plan/design for this feature so that i can incorporate them > in new patch. > Regards, > Kiran Just a gentle reminder to consider the patch. > >> >> cheers >> >> -- >> balbi Regards, Kiran -- 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: dwc2: problems with IN requests completion in linux-next
On 01/13/2015 11:18 AM, Robert Baldyga wrote: > On 01/12/2015 10:35 PM, Paul Zimmerman wrote: >>> From: Robert Baldyga [mailto:r.bald...@samsung.com] >>> Sent: Monday, December 22, 2014 6:13 AM >>> >>> I have recently noticed problem with DWC2 driver in latest linux-next. I >>> use it in gadget only mode at Samsung platform (Odroid U3) but I believe >>> the bug can be reproduced at another platforms. >>> >>> While running FFS example (tools/usb/ffs-aio-example/simple/) the >>> communication breaks after few seconds. It's because one of IN requests >>> enqueued in DWC2 driver do not complete. At USB analyzer I see that USB >>> device started to transmit data from this request, but it ended incomplete. >>> >>> I bisected kernel tree, and I got following patches are reason of break. >>> >>> 941fcce usb: dwc2: Update the gadget driver to use common dwc2_hsotg >>> structure >>> 11b usb: dwc2: Move gadget probe function into platform code >>> bcc0607 usb: dwc2: convert to use dev_pm_ops API >>> 510ffaa usb: dwc2: Initialize the USB core for peripheral mode >>> db8178c usb: dwc2: Update common interrupt handler to call gadget >>> interrupt handler >>> 8d736d8 usb: dwc2: gadget: Do not fail probe if there isn't a clock node >>> >>> Patch 941fcce breaks DWC2 driver at my platform and it starts to work >>> from 8d736d8 but it has described bug. >>> >>> I will try to localize reason of this issue. >> >> Hi Robert, >> >> I think the most likely suspect would be db8178c, since the rest appear >> to be init-time things. It seems to revert cleanly, can you try >> reverting just that patch and see if it helps? >> > > Hi Paul, > > Thanks. It looks like you're right, commit db8178c causes the break. > > I have found out that if we don't call dwc2_is_controller_alive() in > interrupt handler everything works well. Conclusion is that reading from > GSNPSID causes the problem. > > BTW it's strange that this register is used for testing if controller is > alive. Documentation says nothing about that as well as it does not say > that reading this register can affect hardware behaviour. > > Avoiding to read this register in device mode seems to be the simplest > solution. But the question is why does it behave this way? > It looks like calling dwc2_is_controller_alive() under spinlock solves the problem. I will prepare patch. Best regards, Robert Baldyga -- 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: dwc2: call dwc2_is_controller_alive() under spinlock
This patch fixes bug described here:
https://lkml.org/lkml/2014/12/22/185
Signed-off-by: Robert Baldyga
---
drivers/usb/dwc2/core_intr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index ad43c5b..668c8dd 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -476,13 +476,13 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
u32 gintsts;
irqreturn_t retval = IRQ_NONE;
+ spin_lock(&hsotg->lock);
+
if (!dwc2_is_controller_alive(hsotg)) {
dev_warn(hsotg->dev, "Controller is dead\n");
goto out;
}
- spin_lock(&hsotg->lock);
-
gintsts = dwc2_read_common_intr(hsotg);
if (gintsts & ~GINTSTS_PRTINT)
retval = IRQ_HANDLED;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next] r8152: replace tasklet with NAPI
Replace tasklet with NAPI.
Add rx_queue to queue the remaining rx packets if the number of the
rx packets is more than the request from poll().
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 120 ++--
1 file changed, 85 insertions(+), 35 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index b23426e..50387fe 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -27,7 +27,7 @@
#include
/* Version Information */
-#define DRIVER_VERSION "v1.07.0 (2014/10/09)"
+#define DRIVER_VERSION "v1.08.0 (2015/01/13)"
#define DRIVER_AUTHOR "Realtek linux nic maintainers "
#define DRIVER_DESC "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
#define MODULENAME "r8152"
@@ -448,6 +448,7 @@ enum rtl_register_content {
#define RTL8152_RMS(VLAN_ETH_FRAME_LEN + VLAN_HLEN)
#define RTL8153_RMSRTL8153_MAX_PACKET
#define RTL8152_TX_TIMEOUT (5 * HZ)
+#define RTL8152_NAPI_WEIGHT64
/* rtl8152 flags */
enum rtl8152_flags {
@@ -457,7 +458,7 @@ enum rtl8152_flags {
RTL8152_LINK_CHG,
SELECTIVE_SUSPEND,
PHY_RESET,
- SCHEDULE_TASKLET,
+ SCHEDULE_NAPI,
};
/* Define these values to match your device */
@@ -549,14 +550,14 @@ struct tx_agg {
struct r8152 {
unsigned long flags;
struct usb_device *udev;
- struct tasklet_struct tl;
+ struct napi_struct napi;
struct usb_interface *intf;
struct net_device *netdev;
struct urb *intr_urb;
struct tx_agg tx_info[RTL8152_MAX_TX];
struct rx_agg rx_info[RTL8152_MAX_RX];
struct list_head rx_done, tx_free;
- struct sk_buff_head tx_queue;
+ struct sk_buff_head tx_queue, rx_queue;
spinlock_t rx_lock, tx_lock;
struct delayed_work schedule;
struct mii_if_info mii;
@@ -1062,7 +1063,7 @@ static void read_bulk_callback(struct urb *urb)
spin_lock(&tp->rx_lock);
list_add_tail(&agg->list, &tp->rx_done);
spin_unlock(&tp->rx_lock);
- tasklet_schedule(&tp->tl);
+ napi_schedule(&tp->napi);
return;
case -ESHUTDOWN:
set_bit(RTL8152_UNPLUG, &tp->flags);
@@ -1126,7 +1127,7 @@ static void write_bulk_callback(struct urb *urb)
return;
if (!skb_queue_empty(&tp->tx_queue))
- tasklet_schedule(&tp->tl);
+ napi_schedule(&tp->napi);
}
static void intr_callback(struct urb *urb)
@@ -1245,6 +1246,7 @@ static int alloc_all_mem(struct r8152 *tp)
spin_lock_init(&tp->tx_lock);
INIT_LIST_HEAD(&tp->tx_free);
skb_queue_head_init(&tp->tx_queue);
+ skb_queue_head_init(&tp->rx_queue);
for (i = 0; i < RTL8152_MAX_RX; i++) {
buf = kmalloc_node(agg_buf_sz, GFP_KERNEL, node);
@@ -1649,13 +1651,32 @@ return_result:
return checksum;
}
-static void rx_bottom(struct r8152 *tp)
+static int rx_bottom(struct r8152 *tp, int budget)
{
unsigned long flags;
struct list_head *cursor, *next, rx_queue;
+ int work_done = 0;
+
+ if (!skb_queue_empty(&tp->rx_queue)) {
+ while (work_done < budget) {
+ struct sk_buff *skb = __skb_dequeue(&tp->rx_queue);
+ struct net_device *netdev = tp->netdev;
+ struct net_device_stats *stats = &netdev->stats;
+ unsigned int pkt_len;
+
+ if (!skb)
+ break;
+
+ pkt_len = skb->len;
+ napi_gro_receive(&tp->napi, skb);
+ work_done++;
+ stats->rx_packets++;
+ stats->rx_bytes += pkt_len;
+ }
+ }
if (list_empty(&tp->rx_done))
- return;
+ goto out1;
INIT_LIST_HEAD(&rx_queue);
spin_lock_irqsave(&tp->rx_lock, flags);
@@ -1708,9 +1729,14 @@ static void rx_bottom(struct r8152 *tp)
skb_put(skb, pkt_len);
skb->protocol = eth_type_trans(skb, netdev);
rtl_rx_vlan_tag(rx_desc, skb);
- netif_receive_skb(skb);
- stats->rx_packets++;
- stats->rx_bytes += pkt_len;
+ if (work_done < budget) {
+ napi_gro_receive(&tp->napi, skb);
+ work_done++;
+ stats->rx_packets++;
+ stats->rx_bytes += pkt_len;
+ } else {
+ __skb_queue_tail(&tp->rx_queue, skb);
+ }
find_next_rx:
rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
@@ -1722,6 +1748,9 @@ find_next_rx:
submit:
r8152_submit_
Re: [PATCH next] usb: gadget: uvc: to_uvcg_control_header() can be static
W dniu 13.01.2015 o 09:55, kbuild test robot pisze:
drivers/usb/gadget/function/uvc_configfs.c:46:28: sparse: symbol
'to_uvcg_control_header' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:138:25: sparse: symbol
'uvcg_control_header_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:164:6: sparse: symbol
'uvcg_control_header_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:721:20: sparse: symbol
'to_uvcg_format' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:798:30: sparse: symbol
'to_uvcg_streaming_header' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:950:25: sparse: symbol
'uvcg_streaming_header_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:976:6: sparse: symbol
'uvcg_streaming_header_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1020:19: sparse: symbol
'to_uvcg_frame' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1265:25: sparse: symbol
'uvcg_frame_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1315:6: sparse: symbol
'uvcg_frame_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1338:26: sparse: symbol
'to_uvcg_uncompressed' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1548:25: sparse: symbol
'uvcg_uncompressed_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1586:6: sparse: symbol
'uvcg_uncompressed_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1610:19: sparse: symbol
'to_uvcg_mjpeg' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1761:25: sparse: symbol
'uvcg_mjpeg_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1793:6: sparse: symbol
'uvcg_mjpeg_drop' was not declared. Should it be static?
Signed-off-by: Fengguang Wu
Reviewed-by: Andrzej Pietrasiewicz
---
uvc_configfs.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc_configfs.c
b/drivers/usb/gadget/function/uvc_configfs.c
index 33d92ab..f69f47a 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -43,7 +43,7 @@ struct uvcg_control_header {
unsignedlinked;
};
-struct uvcg_control_header *to_uvcg_control_header(struct config_item *item)
+static struct uvcg_control_header *to_uvcg_control_header(struct config_item
*item)
{
return container_of(item, struct uvcg_control_header, item);
}
@@ -135,7 +135,7 @@ static struct configfs_attribute
*uvcg_control_header_attrs[] = {
NULL,
};
-struct config_item_type uvcg_control_header_type = {
+static struct config_item_type uvcg_control_header_type = {
.ct_item_ops= &uvcg_control_header_item_ops,
.ct_attrs = uvcg_control_header_attrs,
.ct_owner = THIS_MODULE,
@@ -161,7 +161,7 @@ static struct config_item *uvcg_control_header_make(struct
config_group *group,
return &h->item;
}
-void uvcg_control_header_drop(struct config_group *group,
+static void uvcg_control_header_drop(struct config_group *group,
struct config_item *item)
{
struct uvcg_control_header *h = to_uvcg_control_header(item);
@@ -718,7 +718,7 @@ struct uvcg_format {
__u8bmaControls[UVCG_STREAMING_CONTROL_SIZE];
};
-struct uvcg_format *to_uvcg_format(struct config_item *item)
+static struct uvcg_format *to_uvcg_format(struct config_item *item)
{
return container_of(to_config_group(item), struct uvcg_format, group);
}
@@ -795,7 +795,7 @@ struct uvcg_streaming_header {
unsignednum_fmt;
};
-struct uvcg_streaming_header *to_uvcg_streaming_header(struct config_item
*item)
+static struct uvcg_streaming_header *to_uvcg_streaming_header(struct
config_item *item)
{
return container_of(item, struct uvcg_streaming_header, item);
}
@@ -947,7 +947,7 @@ static struct configfs_attribute
*uvcg_streaming_header_attrs[] = {
NULL,
};
-struct config_item_type uvcg_streaming_header_type = {
+static struct config_item_type uvcg_streaming_header_type = {
.ct_item_ops= &uvcg_streaming_header_item_ops,
.ct_attrs = uvcg_streaming_header_attrs,
.ct_owner = THIS_MODULE,
@@ -973,7 +973,7 @@ static struct config_item
return &h->item;
}
-void uvcg_streaming_header_drop(struct config_group *group,
+static void uvcg_streaming_header_drop(struct config_group *group,
struct config_item *item)
{
struct
Re: [PATCH v3 0/4] scsi: ufs & ums-* & esp_scsi: fix module reference counting
2015-01-12 18:36 GMT+09:00 Christoph Hellwig : > On Sun, Jan 11, 2015 at 10:50:02PM +0900, Akinobu Mita wrote: >> While accessing a scsi_device, the use count of the underlying LLDD module >> is incremented. The module reference is retrieved through .module field of >> struct scsi_host_template. >> >> This mapping between scsi_device and underlying LLDD module works well >> except ufs, unusual usb storage drivers, and sub drivers for esp_scsi. >> These drivers consist with core driver and actual LLDDs, and >> scsi_host_template is defined in the core driver. So the actual LLDDs can >> be unloaded even if the scsi_device is being accessed. >> >> This patch series first adds ability to adjust module reference for >> scsi host by LLDDs and then fixes actual LLDDs by adjusting module >> reference after scsi host allocation. > > Why don't we move the module into the Scsi_Host only, and use > the same macro that passes THIS_MODULE trick you are using in > the sub drivers? That seems to be a fairly common scheme in other > subsystems as well. Sure, we can take this approach. But ata drivers require more changes for it because ata drivers don't call scsi_host_alloc() directly so we need to pass THIS_MODULE to a variety of init functions() provided by libata. While looking through ata drivers, I found that libahci_platform sub-drivers and pata_of_platform driver also have module reference mismatch problem. So ata drivers need to be touched anyway. I'll update this patch series with the new approach. -- 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] tools: ffs-aio-example: use endpoint addresses from descriptors
This makes examples more platform independent and more compatible with
USB standard, as endpoint addresses in given interface may differ
between hardware platforms or even between configurations in single
USB device.
Signed-off-by: Robert Baldyga
---
tools/usb/ffs-aio-example/multibuff/host_app/test.c | 14 --
tools/usb/ffs-aio-example/simple/host_app/test.c| 17 ++---
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/tools/usb/ffs-aio-example/multibuff/host_app/test.c
b/tools/usb/ffs-aio-example/multibuff/host_app/test.c
index daa3abe..2cbcce6 100644
--- a/tools/usb/ffs-aio-example/multibuff/host_app/test.c
+++ b/tools/usb/ffs-aio-example/multibuff/host_app/test.c
@@ -33,11 +33,6 @@
#define VENDOR 0x1d6b
#define PRODUCT0x0105
-/* endpoints indexes */
-
-#define EP_BULK_IN (1 | LIBUSB_ENDPOINT_IN)
-#define EP_BULK_OUT(2 | LIBUSB_ENDPOINT_OUT)
-
#define BUF_LEN8192
/*
@@ -159,14 +154,21 @@ void test_exit(struct test_state *state)
int main(void)
{
struct test_state state;
+ struct libusb_config_descriptor *conf;
+ struct libusb_interface_descriptor const *iface;
+ unsigned char addr;
if (test_init(&state))
return 1;
+ libusb_get_config_descriptor(state.found, 0, &conf);
+ iface = &conf->interface[0].altsetting[0];
+ addr = iface->endpoint[0].bEndpointAddress;
+
while (1) {
static unsigned char buffer[BUF_LEN];
int bytes;
- libusb_bulk_transfer(state.handle, EP_BULK_IN, buffer, BUF_LEN,
+ libusb_bulk_transfer(state.handle, addr, buffer, BUF_LEN,
&bytes, 500);
}
test_exit(&state);
diff --git a/tools/usb/ffs-aio-example/simple/host_app/test.c
b/tools/usb/ffs-aio-example/simple/host_app/test.c
index acd6332..aed86ff 100644
--- a/tools/usb/ffs-aio-example/simple/host_app/test.c
+++ b/tools/usb/ffs-aio-example/simple/host_app/test.c
@@ -33,11 +33,6 @@
#define VENDOR 0x1d6b
#define PRODUCT0x0105
-/* endpoints indexes */
-
-#define EP_BULK_IN (1 | LIBUSB_ENDPOINT_IN)
-#define EP_BULK_OUT(2 | LIBUSB_ENDPOINT_OUT)
-
#define BUF_LEN8192
/*
@@ -159,16 +154,24 @@ void test_exit(struct test_state *state)
int main(void)
{
struct test_state state;
+ struct libusb_config_descriptor *conf;
+ struct libusb_interface_descriptor const *iface;
+ unsigned char in_addr, out_addr;
if (test_init(&state))
return 1;
+ libusb_get_config_descriptor(state.found, 0, &conf);
+ iface = &conf->interface[0].altsetting[0];
+ in_addr = iface->endpoint[0].bEndpointAddress;
+ out_addr = iface->endpoint[1].bEndpointAddress;
+
while (1) {
static unsigned char buffer[BUF_LEN];
int bytes;
- libusb_bulk_transfer(state.handle, EP_BULK_IN, buffer, BUF_LEN,
+ libusb_bulk_transfer(state.handle, in_addr, buffer, BUF_LEN,
&bytes, 500);
- libusb_bulk_transfer(state.handle, EP_BULK_OUT, buffer, BUF_LEN,
+ libusb_bulk_transfer(state.handle, out_addr, buffer, BUF_LEN,
&bytes, 500);
}
test_exit(&state);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: phy: mv-usb: fix usb_phy build errors
The driver was recently adapted to a core API change, but the
change was incomplete, missing out the suspend helper and
leaving an extraneous local variable around:
usb/phy/phy-mv-usb.c: In function 'mv_otg_update_state':
usb/phy/phy-mv-usb.c:341:18: warning: unused variable 'phy' [-Wunused-variable]
usb/phy/phy-mv-usb.c: In function 'mv_otg_suspend':
usb/phy/phy-mv-usb.c:861:16: error: 'struct usb_phy' has no member named 'state'
Signed-off-by: Arnd Bergmann
Fixes: e47d92545c297 ("usb: move the OTG state from the USB PHY to the OTG
structure")
---
diff --git a/drivers/usb/phy/phy-mv-usb.c b/drivers/usb/phy/phy-mv-usb.c
index 699e38c73d82..697a741a0cb1 100644
--- a/drivers/usb/phy/phy-mv-usb.c
+++ b/drivers/usb/phy/phy-mv-usb.c
@@ -338,7 +338,6 @@ static void mv_otg_update_inputs(struct mv_otg *mvotg)
static void mv_otg_update_state(struct mv_otg *mvotg)
{
struct mv_otg_ctrl *otg_ctrl = &mvotg->otg_ctrl;
- struct usb_phy *phy = &mvotg->phy;
int old_state = mvotg->phy.otg->state;
switch (old_state) {
@@ -858,10 +857,10 @@ static int mv_otg_suspend(struct platform_device *pdev,
pm_message_t state)
{
struct mv_otg *mvotg = platform_get_drvdata(pdev);
- if (mvotg->phy.state != OTG_STATE_B_IDLE) {
+ if (mvotg->phy.otg->state != OTG_STATE_B_IDLE) {
dev_info(&pdev->dev,
"OTG state is not B_IDLE, it is %d!\n",
-mvotg->phy.state);
+mvotg->phy.otg->state);
return -EAGAIN;
}
--
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
Is g_multi with g_hid possible?
Is it possible to appear as a USB gadget hid (kb + mouse), mass_storage, and ethernet at the same time? My goal is to try and create a device that when attached during boot to a PC would be able to send keys as a hid keyboard to select booting from a USB mass storage. Booting from the mass storage would then load ipxe and perform dhcp over the USB RDNIS device. The nic would probably be bridged to wifi, or at least connected to some type of boot control process. It has been necessary to unload g_multi in order to load g_hid on the Edison on 3.10.17 so far and wanted to make sure the end goal was possible at all. Cheers, chris -- 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: Is g_multi with g_hid possible?
Hi, On Tue, Jan 13, 2015 at 07:05:37AM -0800, Chris McClimans wrote: > Is it possible to appear as a USB gadget hid (kb + mouse), > mass_storage, and ethernet at the same time? yes, that's called a composite device. > My goal is to try and create a device that when attached during boot > to a PC would be able to send keys as a hid keyboard to select booting > from a USB mass storage. > > Booting from the mass storage would then load ipxe and perform dhcp > over the USB RDNIS device. The nic would probably be bridged to wifi, > or at least connected to some type of boot control process. > > It has been necessary to unload g_multi in order to load g_hid on the > Edison on 3.10.17 so far and wanted to make sure the end goal was > possible at all. Have a look at libusbg, you need to cook up your own gadget. g_multi won't help you here. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 2/4] usb: udc: add udc_driver_find_udc
On Tue, 13 Jan 2015, Peter Chen wrote:
> This is an internal API, it can be used to find corresponding udc
> according to usb_gadget_driver.
>
> Signed-off-by: Peter Chen
> ---
> drivers/usb/gadget/udc/udc-core.c | 33 +++--
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index 0156709..7f8dc5b 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -70,6 +70,22 @@ found:
> return udc;
> }
>
> +static struct usb_udc *udc_driver_find_udc(struct usb_gadget_driver *driver)
> +{
> + struct usb_udc *udc = NULL;
> +
> + mutex_lock(&udc_lock);
> + list_for_each_entry(udc, &udc_list, list)
> + if (udc->driver == driver)
> + goto found;
> + mutex_unlock(&udc_lock);
> +
> + return NULL;
> +
> +found:
> + mutex_unlock(&udc_lock);
> + return udc;
> +}
> /* -
> */
> #ifdef CONFIG_HAS_DMA
>
> @@ -469,17 +485,14 @@ int usb_gadget_unregister_driver(struct
> usb_gadget_driver *driver)
> if (!driver || !driver->unbind)
> return -EINVAL;
>
> - mutex_lock(&udc_lock);
> - list_for_each_entry(udc, &udc_list, list)
> - if (udc->driver == driver) {
> - usb_gadget_remove_driver(udc);
> - usb_gadget_set_state(udc->gadget,
> - USB_STATE_NOTATTACHED);
> - ret = 0;
> - break;
> - }
> + udc = udc_driver_find_udc(driver);
> + if (udc) {
> + usb_gadget_remove_driver(udc);
> + usb_gadget_set_state(udc->gadget,
> + USB_STATE_NOTATTACHED);
> + ret = 0;
> + }
>
> - mutex_unlock(&udc_lock);
> return ret;
> }
> EXPORT_SYMBOL_GPL(usb_gadget_unregister_driver);
This doesn't seem to be an improvement. All it does is increase the
amount of code, because the new subroutine gets used in only one place.
Also, it means that now you are calling usb_gadget_remove_driver()
without holding the udc_lock mutex, which probably isn't a good idea.
(In addition, this change will make it more difficult in the future if
we ever allow a single driver to support more than one gadget.)
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 v7 3/5] usb: dwc2: add generic PHY framework support for dwc2 usb controler platform driver.
On Tue, Jan 13, 2015 at 10:35:52AM +0100, Robert Jarzmik wrote: > Paul Zimmerman writes: > > The patch below fixes it. And it seems like the right thing to me, > > since GPIOs should be optional for a generic phy, I would think. But > > my device tree foo is very weak, so I'm not sure. > > > > CCing Robert, who touched the generic phy code last. Robert, what do > > you think? > I think your patch in [1] is correct, because > "Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt" states that > reset-gpios is optional, and because Felipe told me gpios for phy_generic are > optional. > > Now the original code was written by Felipe, so better ask him first. The > original code was : > af9f51c55 phy-generic.c nop->gpio_reset = of_get_named_gpio_flags(node, > "reset-gpios", > af9f51c55 phy-generic.c 0, > &flags); > af9f51c55 phy-generic.c if (nop->gpio_reset == -EPROBE_DEFER) > af9f51c55 phy-generic.c return -EPROBE_DEFER; yeah, at the time that was written, there was no _optional() varianto of get_gpio, if there is one now, we should use it, indeed. I can rather easily test here on BBB to make sure there are no regressions. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/4] usb: gadget: u_uac1: fix build issue
Hi, On Tue, Jan 13, 2015 at 06:51:40AM +, Peter Chen wrote: > > > See below error message: > > CC [M] drivers/gpu/drm/nouveau/core/subdev/vm/nv50.o > > CC [M] drivers/staging/lustre/lnet/selftest/module.o > > drivers/usb/gadget/function/f_uac1.c: In function ‘f_audio_free_inst’: > > drivers/usb/gadget/function/f_uac1.c:904:21: error: ‘struct f_uac1_opts’ has > > no member named ‘card’ > > gaudio_cleanup(opts->card); > > ^ > > make[4]: *** [drivers/usb/gadget/function/f_uac1.o] Error 1 > > make[3]: *** [drivers/usb/gadget/function] Error 2 > > make[2]: *** [drivers/usb/gadget] Error 2 > > make[2]: *** Waiting for unfinished jobs > > CC [M] drivers/staging/lustre/lnet/selftest/ping_test.o > > > > Signed-off-by: Huang Rui > > Cc: Peter Chen > > --- > > > > Oh, my three patches have dependencies, it needs to rebase Felipe's > fixes tree to fix this problem. > > commit 4fde6204df052bb89ba3d915ed6ed9f306f3cfa1 > Author: Peter Chen > Date: Mon Dec 1 16:09:27 2014 +0800 > > usb: gadget: f_uac1: access freed memory at f_audio_free_inst Yeah, once that gets merged by Linus, I'll merge v3.19-rc5 on my next branch, that should sort it all out. This is one great example of why you should never make new features depend on fixes. We will always sort out the merge conflicts later. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode
On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote: > Hi Felipe, > > On 1 December 2014 at 11:09, Kiran Raparthy wrote: > > Hi Felipe, > > > > On 25 November 2014 at 20:15, Felipe Balbi wrote: > >> On Tue, Nov 25, 2014 at 07:06:18AM +, Peter Chen wrote: > >>> > >>> > > >>> > usb: phy: hold wakeupsource when USB is enumerated in peripheral mode > >>> > > >>> > Some systems require a mechanism to prevent system to enter into suspend > >>> > state when USB is connected and enumerated in peripheral mode. > >>> > > >>> > This patch provides an interface to hold a wakeupsource to prevent > >>> > suspend. > >>> > PHY drivers can use this interface when USB is connected and enumerated > >>> > in > >>> > peripheral mode. > >>> > > >>> > A timed wakeupsource is temporarily held on USB disconnect events, to > >>> > allow > >>> > the rest of the system to react to the USB disconnection (dropping host > >>> > sessions, updating charger status, etc.) prior to re-allowing suspend. > >>> > > >>> > >>> Hi Kiran & Felipe, > >>> > >>> Just two questions for this series > >>> > >>> - Will it be the default behavior for all peripheral drivers? > >>> - If the peripheral driver's PHY driver does not vbus event, how to > >>> support it? > >>> For example, chipidea udc driver has its vbus interface at its > >>> controller register. > >> > >> hmm, good point. Since it's so late, I'll just go ahead and drop > >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so > >> we have some more time to discuss these details. > > > > I am just curious to know/understand why this feature needs to be > > default behavior for all peripheral drivers? > > If this needs to be default behavior, could you please suggest any > > alternate plan/design for this feature so that i can incorporate them > > in new patch. > > Regards, > > Kiran > > Just a gentle reminder to consider the patch. sorry, for the delay... man this release round has been hectic. We still haven't sorted out what should be the default behavior here, considering some PHYs will not be able to report VBUS levels, I'm not sure what we should do here. If anybody has suggestions, I'm all ears -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/3] usb: dwc3: pci: code cleanup
On Tue, Jan 13, 2015 at 02:12:00PM +0800, Huang Rui wrote: > On Mon, Jan 12, 2015 at 02:20:14PM +0200, Heikki Krogerus wrote: > > Removing a few items that are not needed anymore and > > adding separate function for quirks. > > > > Signed-off-by: Heikki Krogerus > > Cc: Huang Rui > > Looks good for me. > > Acked-by: Huang Rui I'm assuming this Ack is for the whole series ? -- balbi signature.asc Description: Digital signature
Re: Is g_multi with g_hid possible?
W dniu 13.01.2015 o 16:10, Felipe Balbi pisze: Hi, On Tue, Jan 13, 2015 at 07:05:37AM -0800, Chris McClimans wrote: Is it possible to appear as a USB gadget hid (kb + mouse), mass_storage, and ethernet at the same time? yes, that's called a composite device. My goal is to try and create a device that when attached during boot to a PC would be able to send keys as a hid keyboard to select booting from a USB mass storage. Booting from the mass storage would then load ipxe and perform dhcp over the USB RDNIS device. The nic would probably be bridged to wifi, or at least connected to some type of boot control process. It has been necessary to unload g_multi in order to load g_hid on the Edison on 3.10.17 so far and wanted to make sure the end goal was possible at all. Have a look at libusbg, you need to cook up your own gadget. g_multi won't help you here. To elaborate a bit more on Felipe's answer: g_multi is an example of a "legacy" gadget; legacy gadgets' composition of functions (like hid, mass storage, ethernet) is more or less hardcoded into them and you cannot do anything about that. If you want a composition which is not covered by any of the legacy gadgets, you need to create your own. Your options for doing that are: 1) Create another "legacy"-style gadget; it is not that difficult if you have some experience with gadgets from the kernel side. You don't get community support in this case and chances of merging another "legacy"-style gadget into upstream kernel are rather low unless you have very good arguments but I can't think of any to be honest. 2) Compose your gadget using configfs. This feature has been in the kernel since late 2012 and as of now all the functions available as drivers/usb/gadget/function/f_xyz.c are available for composition with configfs (with uvc being merged in 3.20). While composing your gadget manually (using shell) is a perfectly legal thing to do, the bare configfs interface is intended primarily for dedicated userspace tools and the tool you want to look at is libusbg: https://github.com/libusbg/libusbg Please note that often the bleeding edge of development can be found at: https://github.com/kopasiak/libusbg AP -- 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: dwc3: gadget: Stop TRB preparation after limit is reached
Hi, On Tue, Jan 13, 2015 at 10:18:20AM +0530, Amit Virdi wrote: > On 1/13/2015 12:04 AM, Felipe Balbi wrote: > >Hi, > > > >On Tue, Jan 06, 2015 at 11:44:23AM +0530, Amit Virdi wrote: > >>I can certainly provide the dwc3 specific kernel bootup logs, full > >>regdump and any loglevel you want me to, if that helps > > > >Yeah, if you can provide those, then that'll help me verifying. Full > >logs from boot to failure point with VERBOSE_DEBUG enabled (considering > >you're not running on anything recent). > > > > Okay. I'll provide full verbose logs, along with the register dump, > when I'm back to the office next week, for the working and non-working > case, but how - as attachment or as inline? > >>> > >>>Either way will do but I have a feeling mailing list attachment size > >>>will bite you, so maybe it's best to make a tarball of both logs and > >>>send that as attachment. Compressed text will be very small. > >>> > >> > >>Attached are the dwc3 verbose logs and register dump without and with the > >>fixes in this patchset. > > > >Sorry for the long delay, it has been a bit hectic. > > > > It's okay! > > >I can see from your logs that we end up with a Transfer Not Ready > >(Transfer Active) event and endpoint has BUSY flag set. I also see that > >you're queueing 31 requests and all of them will use 2 TRBs, so we > >clearly go over our TRB table. > > > >This fix is, indeed, necessary but we need to improve commit log a bit > >so it's extremely clear what the error is. Later, we can improve how we > >handle requests in this driver, but that is outside of the scope of your > >patchset. > > > >Please improve commit log and resend your series so it can be applied. > > > > Alright! I'll improve the commit messages and, also, cc stable list while > resending the patches. As I see, you have already picked patches [3/4] and > [4/4]. So, I'll resend only [1/4] and [2/4] > > >cheers > > > > Thank you for your patience and kind understanding. Alright, I just applied your patches to testing/fixes. I'll start testing today and should be able to send a pull request to Greg by the end of the week, hopefully. cheers -- balbi signature.asc Description: Digital signature
Re: Is g_multi with g_hid possible?
On Tue, Jan 13, 2015 at 05:26:34PM +0100, Andrzej Pietrasiewicz wrote: > W dniu 13.01.2015 o 16:10, Felipe Balbi pisze: > >Hi, > > > >On Tue, Jan 13, 2015 at 07:05:37AM -0800, Chris McClimans wrote: > >>Is it possible to appear as a USB gadget hid (kb + mouse), > >>mass_storage, and ethernet at the same time? > > > >yes, that's called a composite device. > > > >>My goal is to try and create a device that when attached during boot > >>to a PC would be able to send keys as a hid keyboard to select booting > >>from a USB mass storage. > >> > >>Booting from the mass storage would then load ipxe and perform dhcp > >>over the USB RDNIS device. The nic would probably be bridged to wifi, > >>or at least connected to some type of boot control process. > >> > >>It has been necessary to unload g_multi in order to load g_hid on the > >>Edison on 3.10.17 so far and wanted to make sure the end goal was > >>possible at all. > > > >Have a look at libusbg, you need to cook up your own gadget. g_multi > >won't help you here. > > > > To elaborate a bit more on Felipe's answer: > > g_multi is an example of a "legacy" gadget; legacy gadgets' > composition of functions (like hid, mass storage, ethernet) > is more or less hardcoded into them and you cannot do > anything about that. > > If you want a composition which is not covered by > any of the legacy gadgets, you need to create your own. > Your options for doing that are: > > 1) Create another "legacy"-style gadget; it is not > that difficult if you have some experience with > gadgets from the kernel side. You don't get > community support in this case and chances of > merging another "legacy"-style gadget into > upstream kernel are rather low unless you have > very good arguments but I can't think of any to > be honest. heh, they're not low, they don't exist :-) From now on we will only merge functions and rely on the configfs interface for bulding gadgets :-) > 2) Compose your gadget using configfs. This feature > has been in the kernel since late 2012 and as of now > all the functions available as > drivers/usb/gadget/function/f_xyz.c are available > for composition with configfs (with uvc being merged > in 3.20). While composing your gadget manually > (using shell) is a perfectly legal thing to do, > the bare configfs interface is intended primarily > for dedicated userspace tools and the tool you > want to look at is libusbg: > > https://github.com/libusbg/libusbg > > Please note that often the bleeding edge of development > can be found at: > > https://github.com/kopasiak/libusbg much better than my reply, thanks :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v5] usb: gadget: f_fs: add "no_disconnect" mode
Hi, On Thu, Dec 18, 2014 at 09:55:10AM +0100, Robert Baldyga wrote: > Since we can compose gadgets from many functions, there is the problem > related to gadget breakage while FunctionFS daemon being closed. FFS > function is userspace code so there is no way to know when it will close > files (it doesn't matter what is the reason of this situation, it can > be daemon logic, program breakage, process kill or any other). So when > we have another function in gadget which, for example, sends some amount > of data, does some software update or implements some real-time functionality, > we may want to keep the gadget connected despite FFS function is no longer > functional. > > We can't just remove one of functions from gadget since it has been > enumerated, so the only way to keep entire gadget working is to make > broken FFS function deactivated but still visible to host. For this > purpose this patch introduces "no_disconnect" mode. It can be enabled > by setting mount option "no_disconnect=1", and results with defering > function disconnect to the moment of reopen ep0 file or filesystem > unmount. After closing all endpoint files, FunctionFS is set to state > FFS_DEACTIVATED. > > When ffs->state == FFS_DEACTIVATED: > - function is still bound and visible to host, > - setup requests are automatically stalled, > - transfers on other endpoints are refused, > - epfiles, except ep0, are deleted from the filesystem, > - opening ep0 causes the function to be closed, and then FunctionFS > is ready for descriptors and string write, > - altsetting change causes the function to be closed - we want to keep > function alive until another functions are potentialy used, altsetting > change means that another configuration is being selected or USB cable > was unplugged, which indicates that we don't need to stay longer in > FFS_DEACTIVATED state > - unmounting of the FunctionFS instance causes the function to be closed. > > Signed-off-by: Robert Baldyga > > Changelog: > > v5: > - close function on altsetting change > > v4: https://lkml.org/lkml/2014/10/9/224 > - use ffs_data_reset() instead of ffs_data_clear() to reset ffs data > properly after ffs->ref refcount reach 0 (or under in no_disconnect > mode) in ffs_data_put() function > > v3: https://lkml.org/lkml/2014/10/9/170 > - change option name to more descriptive and less scary, > - fix cleaning up on unmount (call ffs_data_closed() in ffs_fs_kill_sb(), > and ffs_data_clear() in ffs_data_closed() if ffs->opened is negative). > > v2: https://lkml.org/lkml/2014/10/7/109 > - delete epfiles, excepting ep0, when FFS is in "zombie" mode, > - add description of FFS_ZOMBIE state, > - minor cleanups. > > v1: https://lkml.org/lkml/2014/10/6/128 This changelog should be after the tearline, I've manually editted it and also applied the small modification sugested by Michal. It should show up on my testing/next soonish. -- balbi signature.asc Description: Digital signature
Re: [PATCH 05/11] usb: gadget: at91_udc: Remove non-DT handling code
Hi, On Mon, Jan 12, 2015 at 11:53:30PM +0100, Alexandre Belloni wrote: > Hi Felipe, > > I've rebased that patch series but it depends on another one (the mfd: > syscon: part of "memory: add Atmel EBI (External Bus Interface) > driver") that is not taken yet so applying it now will break the > build. I still hope to get it merged for 3.20 and I'll resend it as > soon as possible. In that case, we either take the entire series through MFD (including the gadget patches) or we delay the gadget patches until all dependencies have been sorted out. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH] usb: gadget: composite: Provide list of registered functions
Hi, On Thu, Nov 06, 2014 at 10:08:55AM +0100, Krzysztof Opasiak wrote: > > From: Felipe Balbi [mailto:ba...@ti.com] > > Sent: Wednesday, November 05, 2014 8:43 PM > > To: Krzysztof Opasiak > > Cc: ba...@ti.com; gre...@linuxfoundation.org; > > bige...@breakpoint.cc; linux-usb@vger.kernel.org; > > s.wa...@samsung.com; k.lewando...@samsung.com; > > andrze...@samsung.com; m.szyprow...@samsung.com > > Subject: Re: [RFC PATCH] usb: gadget: composite: Provide list of > > registered functions > > > > On Mon, Oct 20, 2014 at 01:41:37PM +0200, Krzysztof Opasiak wrote: > > > Driver which provides implementation of some USB functions > > registers > > > its usb_function_driver structure in framework. > > > Function drivers are identifed using registered name. > > > > > > When gadget is composed user must know what names has been > > > > "user *must" know", why ? > > Because he has to create directory with suitable name. This means that > he needs to know that Mass storage module has been registered as > mass_storage, and that gadget zero provides functions registered as > Loopback and SourceSink. > > Let's say that you got system and kernel image. You may use uname -r to > learn about kernel version, let's say that it is 3.17. You may also > check what kernel modules has been provided with image. But how to learn > which usb function has been build into kernel? > > I see some analogy to filesystems. When you would like to use -t option > in mount you have to know how Ext4 or Function FS has been registered in > fs framework. Kernel allows you to learn about all loaded or build-in > modules. You may simply do this using /proc/filesystems where you will > get all fs names registered in kernel. So in a few steps: that's a good analogy and you're starting to convince me of the need for this interface. > 1. I have some disk image with Ext4. I would like to know if I have > support for this fs type in my kernel and what name should I use to -t > option. > 2. I check my kernel modules but Ext4 not found > 3. cat /proc/filesystems and I see that yes there is Ext4 and it has > been compiled-in my kernel and it has been registered as "ext4" > 4. Now I have gather all information and I may simply mount -t ext4 ... > > Don't you think that such scenario should be also doable with usb > functions? > > User ends up in empty functions dir of some gadget and what now? How to > allow him to ask kernel what is available? How to learn what functions > has been build-in and what names has been registered in framework? (ocf > kernel config but it's not always available) right, makes sense to me. Care to respin the patch ? -- balbi signature.asc Description: Digital signature
[PATCH] drivers/usb/serial/mos7840.c: remove unused code
From: Colin Ian King There is old, unused code that is #defined out by the use of NOTMOS7840 - this is not defined anywhere. If NOTMOS7840 is defined then the code will break on null pointer dereferences on mos7840_port. So the code is currently unused, and broken anyway, so why not just remove it. Signed-off-by: Colin Ian King --- drivers/usb/serial/mos7840.c | 29 - 1 file changed, 29 deletions(-) diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c index 220b4be..ba718ea 100644 --- a/drivers/usb/serial/mos7840.c +++ b/drivers/usb/serial/mos7840.c @@ -1309,35 +1309,6 @@ static int mos7840_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *current_position = data; unsigned char *data1; -#ifdef NOTMOS7840 - Data = 0x00; - status = mos7840_get_uart_reg(port, LINE_CONTROL_REGISTER, &Data); - mos7840_port->shadowLCR = Data; - dev_dbg(&port->dev, "%s: LINE_CONTROL_REGISTER is %x\n", __func__, Data); - dev_dbg(&port->dev, "%s: mos7840_port->shadowLCR is %x\n", __func__, mos7840_port->shadowLCR); - - /* Data = 0x03; */ - /* status = mos7840_set_uart_reg(port,LINE_CONTROL_REGISTER,Data); */ - /* mos7840_port->shadowLCR=Data;//Need to add later */ - - Data |= SERIAL_LCR_DLAB;/* data latch enable in LCR 0x80 */ - status = mos7840_set_uart_reg(port, LINE_CONTROL_REGISTER, Data); - - /* Data = 0x0c; */ - /* status = mos7840_set_uart_reg(port,DIVISOR_LATCH_LSB,Data); */ - Data = 0x00; - status = mos7840_get_uart_reg(port, DIVISOR_LATCH_LSB, &Data); - dev_dbg(&port->dev, "%s: DLL value is %x\n", __func__, Data); - - Data = 0x0; - status = mos7840_get_uart_reg(port, DIVISOR_LATCH_MSB, &Data); - dev_dbg(&port->dev, "%s: DLM value is %x\n", __func__, Data); - - Data = Data & ~SERIAL_LCR_DLAB; - dev_dbg(&port->dev, "%s: mos7840_port->shadowLCR is %x\n", __func__, mos7840_port->shadowLCR); - status = mos7840_set_uart_reg(port, LINE_CONTROL_REGISTER, Data); -#endif - if (mos7840_port_paranoia_check(port, __func__)) return -1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/11] usb: gadget: at91_udc: Remove non-DT handling code
Hi, On 13/01/2015 at 10:36:54 -0600, Felipe Balbi wrote : > > I've rebased that patch series but it depends on another one (the mfd: > > syscon: part of "memory: add Atmel EBI (External Bus Interface) > > driver") that is not taken yet so applying it now will break the > > build. I still hope to get it merged for 3.20 and I'll resend it as > > soon as possible. > > In that case, we either take the entire series through MFD (including > the gadget patches) or we delay the gadget patches until all > dependencies have been sorted out. I would prefer not to miss 3.20, can I resend the patches now so you can review and ack them? Then they could go through the mfd tree. -- 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: [RESEND PATCH] usb: gadget: at91_udc: move prepare clk into process context
On Fri, 2014-12-19 at 13:51 +, Luis Henriques wrote: > Hi Felipe, > > On Thu, Nov 20, 2014 at 01:50:49PM -0600, Felipe Balbi wrote: > > On Wed, Nov 19, 2014 at 04:37:27PM +0100, Nicolas Ferre wrote: > > > From: Ronald Wahl > > > > > > Commit 7628083227b6bc4a7e33d7c381d7a4e558424b6b (usb: gadget: at91_udc: > > > prepare clk before calling enable) added clock preparation in interrupt > > > context. This is not allowed as it might sleep. Also setting the clock > > > rate is unsafe to call from there for the same reason. Move clock > > > preparation and setting clock rate into process context (at91udc_probe). > > > > > > Signed-off-by: Ronald Wahl > > > Acked-by: Alexandre Belloni > > > Acked-by: Boris Brezillon > > > Acked-by: Nicolas Ferre > > > Cc: Felipe Balbi > > > Cc: # v3.17+ > > > --- > > > Hi Felipe, > > > > > > I forgot to answer you on this patch. So I resend it now with the proper > > > "stable" tag. You can also queue it during this -rc phase if you feel it > > > is > > > still possible. > > > > I think it's late for v3.18, so it'll go on v3.19 and get backported to > > 3.17 and 3.18. Sorry :-s > > > > Although this commit (b2ba27a5c56f "usb: gadget: at91_udc: move > prepare clk into process context") is tagged for stable v3.17+, it > seems like it could be applied to earlier kernels. > > 3.16, 3.13 and 3.12 seem to be affected by the same issue (and they > all include commit 7628083227b6 "usb: gadget: at91_udc: prepare clk > before calling enable"). Is there any reason for not applying it in > these trees? I'll queue it up for 3.13-stable. Thanks all! -Kamal -- 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 05/11] usb: gadget: at91_udc: Remove non-DT handling code
On Tue, Jan 13, 2015 at 07:05:26PM +0100, Alexandre Belloni wrote: > Hi, > > On 13/01/2015 at 10:36:54 -0600, Felipe Balbi wrote : > > > I've rebased that patch series but it depends on another one (the mfd: > > > syscon: part of "memory: add Atmel EBI (External Bus Interface) > > > driver") that is not taken yet so applying it now will break the > > > build. I still hope to get it merged for 3.20 and I'll resend it as > > > soon as possible. > > > > In that case, we either take the entire series through MFD (including > > the gadget patches) or we delay the gadget patches until all > > dependencies have been sorted out. > > I would prefer not to miss 3.20, can I resend the patches now so you can > review and ack them? Then they could go through the mfd tree. Sure, please do -- balbi signature.asc Description: Digital signature
f_hid, f_mass_storage, and f_rdnis via configfs on platform/intel-mid
Thanks for all the direction and feedback thus far, configfs is the way to go but I would need f_hid, f_mass_storage, and f_rdnis which aren't available in v3.10. Configfs f_hid is relatively recent, f_rndis + f_mass_storage where merged into v3.11, but similar functions actual entered the kernel in v3.10... which is what gives me hope that the porting may be simple. Porting the platform/intel-mid forward to a new kernel version without communication from Intel would probably result in needless duplication of effort. Would it be advisable for me to backport the drivers/usb/gadget/function/* from 3.19-rc1 and compile them in 3.10.17? I doubt it would be as simple as copying them over and modifying the Makefile, but one can hope. https://lkml.org/lkml/2014/12/14/274 - v3.19-rc1 for f_hid https://lkml.org/lkml/2013/7/1/388 - v3.11-rc1 for f_rndis and f_mass_storage https://lkml.org/lkml/2013/4/29/248 - v3.10-rc1 for f_acm (which seems to load and configure **) **root@edison-3.10.17:/# cat /sys/kernel/config/usb_gadget/gadget/functions/acm.usb0/port_num 0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: f_hid, f_mass_storage, and f_rdnis via configfs on platform/intel-mid
On Tue, Jan 13, 2015 at 11:46:11AM -0800, Chris McClimans wrote: > Thanks for all the direction and feedback thus far, configfs is the > way to go but I would need f_hid, f_mass_storage, and f_rdnis which > aren't available in v3.10. Configfs f_hid is relatively recent, then you need to backport patches yourself. The community can't really support older kernels :-) > f_rndis + f_mass_storage where merged into v3.11, but similar > functions actual entered the kernel in v3.10... which is what gives me > hope that the porting may be simple. > > Porting the platform/intel-mid forward to a new kernel version without > communication from Intel would probably result in needless duplication > of effort. Would it be advisable for me to backport the > drivers/usb/gadget/function/* from 3.19-rc1 and compile them in > 3.10.17? I doubt it would be as simple as copying them over and > modifying the Makefile, but one can hope. I guess you don't have other way but keep in mind you'll, essentially, be on your own. -- balbi signature.asc Description: Digital signature
RE: [PATCH] usb: dwc2: call dwc2_is_controller_alive() under spinlock
> From: Robert Baldyga [mailto:r.bald...@samsung.com]
> Sent: Tuesday, January 13, 2015 2:58 AM
>
> This patch fixes bug described here:
> https://lkml.org/lkml/2014/12/22/185
>
> Signed-off-by: Robert Baldyga
> ---
> drivers/usb/dwc2/core_intr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index ad43c5b..668c8dd 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -476,13 +476,13 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
> u32 gintsts;
> irqreturn_t retval = IRQ_NONE;
>
> + spin_lock(&hsotg->lock);
> +
> if (!dwc2_is_controller_alive(hsotg)) {
> dev_warn(hsotg->dev, "Controller is dead\n");
> goto out;
This isn't right, now the spinlock isn't released if we take this path.
Besides, this patch doesn't really make sense. How could taking the
spinlock affect the value returned from dwc2_is_controller_alive? All
it does is read from the GSNPSID register, and that register is never
written to.
Are you absolutely sure this is the fix?
--
Paul
> }
>
> - spin_lock(&hsotg->lock);
> -
> gintsts = dwc2_read_common_intr(hsotg);
> if (gintsts & ~GINTSTS_PRTINT)
> retval = IRQ_HANDLED;
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] usb: udc: add udc_driver_find_udc
On Tue, Jan 13, 2015 at 10:13:32AM -0500, Alan Stern wrote:
> On Tue, 13 Jan 2015, Peter Chen wrote:
>
> > This is an internal API, it can be used to find corresponding udc
> > according to usb_gadget_driver.
> >
> > Signed-off-by: Peter Chen
> > ---
> > drivers/usb/gadget/udc/udc-core.c | 33 +++--
> > 1 file changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/udc-core.c
> > b/drivers/usb/gadget/udc/udc-core.c
> > index 0156709..7f8dc5b 100644
> > --- a/drivers/usb/gadget/udc/udc-core.c
> > +++ b/drivers/usb/gadget/udc/udc-core.c
> > @@ -70,6 +70,22 @@ found:
> > return udc;
> > }
> >
> > +static struct usb_udc *udc_driver_find_udc(struct usb_gadget_driver
> > *driver)
> > +{
> > + struct usb_udc *udc = NULL;
> > +
> > + mutex_lock(&udc_lock);
> > + list_for_each_entry(udc, &udc_list, list)
> > + if (udc->driver == driver)
> > + goto found;
> > + mutex_unlock(&udc_lock);
> > +
> > + return NULL;
> > +
> > +found:
> > + mutex_unlock(&udc_lock);
> > + return udc;
> > +}
> > /*
> > - */
> > #ifdef CONFIG_HAS_DMA
> >
> > @@ -469,17 +485,14 @@ int usb_gadget_unregister_driver(struct
> > usb_gadget_driver *driver)
> > if (!driver || !driver->unbind)
> > return -EINVAL;
> >
> > - mutex_lock(&udc_lock);
> > - list_for_each_entry(udc, &udc_list, list)
> > - if (udc->driver == driver) {
> > - usb_gadget_remove_driver(udc);
> > - usb_gadget_set_state(udc->gadget,
> > - USB_STATE_NOTATTACHED);
> > - ret = 0;
> > - break;
> > - }
> > + udc = udc_driver_find_udc(driver);
> > + if (udc) {
> > + usb_gadget_remove_driver(udc);
> > + usb_gadget_set_state(udc->gadget,
> > + USB_STATE_NOTATTACHED);
> > + ret = 0;
> > + }
> >
> > - mutex_unlock(&udc_lock);
> > return ret;
> > }
> > EXPORT_SYMBOL_GPL(usb_gadget_unregister_driver);
>
> This doesn't seem to be an improvement. All it does is increase the
> amount of code, because the new subroutine gets used in only one place.
>
> Also, it means that now you are calling usb_gadget_remove_driver()
> without holding the udc_lock mutex, which probably isn't a good idea.
>
> (In addition, this change will make it more difficult in the future if
> we ever allow a single driver to support more than one gadget.)
>
Thanks, will drop this patch.
Do you have any comments for patch 3/4 and 4/4, they are major changes
in this patch set.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] usb: phy: hold wakeupsource when USB is enumerated in peripheral mode
On Tue, Jan 13, 2015 at 10:18:02AM -0600, Felipe Balbi wrote: > On Tue, Jan 13, 2015 at 04:02:20PM +0530, Kiran Raparthy wrote: > > Hi Felipe, > > > > On 1 December 2014 at 11:09, Kiran Raparthy wrote: > > > Hi Felipe, > > > > > > On 25 November 2014 at 20:15, Felipe Balbi wrote: > > >> On Tue, Nov 25, 2014 at 07:06:18AM +, Peter Chen wrote: > > >>> > > >>> > > > >>> > usb: phy: hold wakeupsource when USB is enumerated in peripheral mode > > >>> > > > >>> > Some systems require a mechanism to prevent system to enter into > > >>> > suspend > > >>> > state when USB is connected and enumerated in peripheral mode. > > >>> > > > >>> > This patch provides an interface to hold a wakeupsource to prevent > > >>> > suspend. > > >>> > PHY drivers can use this interface when USB is connected and > > >>> > enumerated in > > >>> > peripheral mode. > > >>> > > > >>> > A timed wakeupsource is temporarily held on USB disconnect events, to > > >>> > allow > > >>> > the rest of the system to react to the USB disconnection (dropping > > >>> > host > > >>> > sessions, updating charger status, etc.) prior to re-allowing suspend. > > >>> > > > >>> > > >>> Hi Kiran & Felipe, > > >>> > > >>> Just two questions for this series > > >>> > > >>> - Will it be the default behavior for all peripheral drivers? > > >>> - If the peripheral driver's PHY driver does not vbus event, how to > > >>> support it? > > >>> For example, chipidea udc driver has its vbus interface at its > > >>> controller register. > > >> > > >> hmm, good point. Since it's so late, I'll just go ahead and drop > > >> $subject from v3.20. Let's delay only $subject to v3.20 merge window so > > >> we have some more time to discuss these details. > > > > > > I am just curious to know/understand why this feature needs to be > > > default behavior for all peripheral drivers? > > > If this needs to be default behavior, could you please suggest any > > > alternate plan/design for this feature so that i can incorporate them > > > in new patch. > > > Regards, > > > Kiran > > > > Just a gentle reminder to consider the patch. > > sorry, for the delay... man this release round has been hectic. We still > haven't sorted out what should be the default behavior here, considering > some PHYs will not be able to report VBUS levels, I'm not sure what we > should do here. > My opinion is: if we need this feature, we should depend on gadget's state, eg 'configured', not vbus level. The system suspend should be allowed if the gadget state is 'suspended' which vbus is still there. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] usb: dwc3: pci: code cleanup
On Tue, Jan 13, 2015 at 10:20:34AM -0600, Felipe Balbi wrote: > On Tue, Jan 13, 2015 at 02:12:00PM +0800, Huang Rui wrote: > > On Mon, Jan 12, 2015 at 02:20:14PM +0200, Heikki Krogerus wrote: > > > Removing a few items that are not needed anymore and > > > adding separate function for quirks. > > > > > > Signed-off-by: Heikki Krogerus > > > Cc: Huang Rui > > > > Looks good for me. > > > > Acked-by: Huang Rui > > I'm assuming this Ack is for the whole series ? > Yes, please. Thanks, Rui -- 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
BUG: xhci_queue_new_dequeue_state dereference a NULL pointer
I have an x86 PC, it oops, and I took a screenshot: http://babyaries.org/mirror/picture/2015-01-13-165845_1600x900_scrot.png Then I browsed the xhci source code, compared with the oops machine code: http://babyaries.org/mirror/picture/2015-01-14-113248_1600x900_scrot.png I found that it's because deq_state->new_deq_seg is an invalid pointer that makes my PC oops. -- 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: dwc2: call dwc2_is_controller_alive() under spinlock
On 01/13/2015 10:12 PM, Paul Zimmerman wrote:
>> From: Robert Baldyga [mailto:r.bald...@samsung.com]
>> Sent: Tuesday, January 13, 2015 2:58 AM
>>
>> This patch fixes bug described here:
>> https://lkml.org/lkml/2014/12/22/185
>>
>> Signed-off-by: Robert Baldyga
>> ---
>> drivers/usb/dwc2/core_intr.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
>> index ad43c5b..668c8dd 100644
>> --- a/drivers/usb/dwc2/core_intr.c
>> +++ b/drivers/usb/dwc2/core_intr.c
>> @@ -476,13 +476,13 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
>> u32 gintsts;
>> irqreturn_t retval = IRQ_NONE;
>>
>> +spin_lock(&hsotg->lock);
>> +
>> if (!dwc2_is_controller_alive(hsotg)) {
>> dev_warn(hsotg->dev, "Controller is dead\n");
>> goto out;
>
> This isn't right, now the spinlock isn't released if we take this path.
>
> Besides, this patch doesn't really make sense. How could taking the
> spinlock affect the value returned from dwc2_is_controller_alive? All
> it does is read from the GSNPSID register, and that register is never
> written to.
>
> Are you absolutely sure this is the fix?
>
I also don't know how is it possible that reading from GSNPSID can break
data transfer, but it looks like it does.
The problem is not about value taken from GSNPSID. This value is always
proper, and dwc2_is_controller_alive() always returns true. The problem
is that the operation of reading this register affects data transmission
process, if it's not done under spinlock. I have no idea why can it be.
I will fix this patch to release spinlock in all cases.
Thanks,
Robert Baldyga
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: dwc2: call dwc2_is_controller_alive() under spinlock
This patch fixes bug described here:
https://lkml.org/lkml/2014/12/22/185
Signed-off-by: Robert Baldyga
---
Changelog:
v2:
- fixed comment from Paul Zimmerman
v1: https://lkml.org/lkml/2015/1/13/186
drivers/usb/dwc2/core_intr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index ad43c5b..02e3e2d 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -476,13 +476,13 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
u32 gintsts;
irqreturn_t retval = IRQ_NONE;
+ spin_lock(&hsotg->lock);
+
if (!dwc2_is_controller_alive(hsotg)) {
dev_warn(hsotg->dev, "Controller is dead\n");
goto out;
}
- spin_lock(&hsotg->lock);
-
gintsts = dwc2_read_common_intr(hsotg);
if (gintsts & ~GINTSTS_PRTINT)
retval = IRQ_HANDLED;
@@ -515,8 +515,8 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
}
}
- spin_unlock(&hsotg->lock);
out:
+ spin_unlock(&hsotg->lock);
return retval;
}
EXPORT_SYMBOL_GPL(dwc2_handle_common_intr);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
