Re: [PATCH V2 1/1] usb:serial:f81534 Add F81532/534 Driver
Hi Peter, On Thu, Jun 25, 2015 at 01:16:56PM +0800, Peter Hung wrote: Hello Johan, Peter Hung 於 2015/6/15 上午 09:54 寫道: This driver is for Fintek F81532/F81534 USB to Serial Ports IC. Features: 1. F81534 is 1-to-4 F81532 is 1-to-2 serial ports IC 2. Support Baudrate from B50 to B150 (excluding B100). 3. The RTS signal can be transformed their behavior with configuration for transceiver (for RS232/RS485/RS422) (/sys/class/ttyUSBx/uart_mode) 4. There are 4x3 output-only GPIOs to control transceiver mode. It's can be controlled via sysfs (/sys/class/ttyUSBx/gpio) Do you receive my patch? Yes, I did. I just haven't had time to review it yet. Are there anything should I do to improve it ? There are, including - your custom read and write implementations look odd, you should be able to reuse a lot more of the generic framework - you'll also need to implement open/and close in some way (enable/disable in hardware or flag in software) as you should not push data to a closed tty - stack allocated buffers used for DMA (register accessors) - DMA buffers allocated as part of larger struct (read/write buffers) - lots of magic constants (e.g. use defines for the registers) - As Greg already mentioned, you need to implement gpio support using gpiolib, not a custom sysfs interface I don't have time to look closer at the architectural bits until next week I'm afraid, but perhaps you could start with the above. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
On 06/25/2015 10:53 PM, Mathias Nyman wrote: On 09.05.2015 04:15, Lu Baolu wrote: There is no need to call xhci_stop_device() and xhci_ring_device() in hub control and bus suspend functions since all device suspend and resume have been notified through device_suspend/device_resume interfaces. I was looking through this code again before sending it forward, and it occurred to me that this might be breaking the PORT_SUSPEND and PORT_SET_LINK_STATE port features for xhci root hub. In normal use these requests are called by usb core in usb_port_suspend(), which also now notifies xhci, which makes sure xhci_stop_device() is called. But I don't think there is anything preventing an URB to be sent to the xhci roothub with a PORT_SUSPEND or PORT_SET_LINK_STATE port feature request. In this case the usb_port_suspend() is not called, and no notify will stop the device. For example hub validation tests might do this. If that, we can drop this patch. It doesn't impact the other two patches in this patch series. Thanks, Baolu -Mathias -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
Hi Oliver.
Thank you for your patience, and review. I apreciated it very much.
On Thu, 25 Jun 2015, Oliver Neukum wrote:
Date: Thu, 25 Jun 2015 11:49:29
From: Oliver Neukum oneu...@suse.com
To: Enrico Mioso mrkiko...@gmail.com
Cc: linux-usb@vger.kernel.org, net...@vger.kernel.org
Subject: Re: [PATCH RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
On Tue, 2015-06-23 at 00:32 +0200, Enrico Mioso wrote:
This patch introduces a new NCM tx engine, able to operate in standard-
and huawei-style mode.
In the first case, the NDP is disposed after the initial headers and
before any datagram.
What works:
- is able to communicate with compliant NCM devices:
I tested this with a board running the Linux g_ncm gadget driver.
What doesn't work:
- After some packets I start gettint LOTS of EVENT_RX_MEMORY from usbnet,
which fails to allocate an RX SKB in rx_submit(). Don't understand why,
any suggestion would be very welcome.
The tx_fixup function given here, even if actually working, should be
considered as an example: the NCM manager is used here simulating the
cdc_ncm.c behaviour.
Signed-off-by: Enrico Mioso mrkiko...@gmail.com
---
drivers/net/usb/huawei_cdc_ncm.c | 187 ++-
1 file changed, 185 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index 735f7da..217802a 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -29,6 +29,35 @@
#include linux/usb/cdc-wdm.h
#include linux/usb/cdc_ncm.h
+/* NCM management operations: */
+
+/* NCM_INIT_FRAME: prepare for a new frame.
+ * NTH16 header is written to output SKB, NDP data is reset and last
+ * committed NDP pointer set to NULL.
+ * Now, data may be added to this NCM package.
+ */
+#define NCM_INIT_FRAME 1
+
+/* NCM_UPDATE_NDP: adds data to an NDP structure, hence updating it.
+ * Some checks are performed to be sure data fits in, respecting device and
+ * spec constrains.
+ * Normally the NDP is kept in memory and committed to the SKB only when
+ * requested. However, calling this method after NCM_COMMIT_NDP, causes it to
+ * work directly on the already committed SKB copy. this allows for flexibility
+ * in frame ordering.
+ */
+#define NCM_UPDATE_NDP 2
+
+/* Write NDP: commits NDP to output SKB.
+ * This method should be called only once per frame.
+ */
+#define NCM_COMMIT_NDP 3
+
+/* Finalizes NTH16 header: to be called when working in
+ * update-already-committed mode.
+ */
+#define NCM_FINALIZE_NTH 5
+
/* Driver data */
struct huawei_cdc_ncm_state {
struct cdc_ncm_ctx *ctx;
@@ -36,6 +65,16 @@ struct huawei_cdc_ncm_state {
struct usb_driver *subdriver;
struct usb_interface *control;
struct usb_interface *data;
+
+ /* Keeps track of where data starts and ends in SKBs. */
+ int data_start;
+ int data_len;
+
+ /* Last committed NDP for post-commit operations. */
+ struct usb_cdc_ncm_ndp16 *skb_ndp;
+
+ /* Non-committed NDP */
+ struct usb_cdc_ncm_ndp16 *ndp;
};
static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
@@ -53,6 +92,149 @@ static int huawei_cdc_ncm_manage_power(struct usbnet
*usbnet_dev, int on)
return 0;
}
+/* huawei_ncm_mgmt: flexible TX NCM manager.
+ *
+ * Once a non-zero status value is rturned, current frame should be discarded
+ * and operations restarted from scratch.
+ */
Is there any advantage in keeping this in a single function?
I did this choice in the light of the fact I think the tx_fixup function will
become more complex than it is now, when aggregating frames.
I answer here your other message to make it more convenient to read: my
intention for the tx_fixup function would be to:
- aggregate frames
- send them out when:
- a timer expires
OR
- we have enough data in the aggregate, and cannot add more.
This is something done in cdc_ncm.c for example.
But here I have a question: by reading the comment in file
drivers/net/usb/rndis_host.c at line 572, there seem to be different opinions
in this matter.
What to do ?
+int
+huawei_ncm_mgmt(struct usbnet *dev,
+ struct huawei_cdc_ncm_state *drvstate, struct sk_buff *skb_out,
int mode) {
+ struct usb_cdc_ncm_nth16 *nth16 = (struct usb_cdc_ncm_nth16
*)skb_out-data;
+ struct cdc_ncm_ctx *ctx = drvstate-ctx;
+ struct usb_cdc_ncm_ndp16 *ndp16 = NULL;
+ int ret = -EINVAL;
+ u16 ndplen, index;
+
+ switch (mode) {
+ case NCM_INIT_FRAME:
+
+ /* Write a new NTH16 header */
+ nth16 = (struct usb_cdc_ncm_nth16 *)memset(skb_put(skb_out,
sizeof(struct usb_cdc_ncm_nth16)), 0, sizeof(struct usb_cdc_ncm_nth16));
+ if (!nth16) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ /* NTH16 signature and header length
Re: [PATCH RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
On Thu, 2015-06-25 at 13:44 +0200, Enrico Mioso wrote:
On Thu, 25 Jun 2015, Oliver Neukum wrote:
Is there any advantage in keeping this in a single function?
I did this choice in the light of the fact I think the tx_fixup function will
become more complex than it is now, when aggregating frames.
Yes, but that is a reason to split the helpers up not the opposite.
I answer here your other message to make it more convenient to read: my
intention for the tx_fixup function would be to:
- aggregate frames
- send them out when:
- a timer expires
How would you do that in tx_fixup()? If a timer is required then you
need a separate function.
OR
- we have enough data in the aggregate, and cannot add more.
Yes.
You need a third case:
- the interface is taken down.
But in general the logic for that is already there. So can you explain
what additional goals you have?
This is something done in cdc_ncm.c for example.
But here I have a question: by reading the comment in file
drivers/net/usb/rndis_host.c at line 572, there seem to be different opinions
in this matter.
That is a very old comment written for much slower devices.
rndis_host doesn't get much love nowadays.
What to do ?
+int
+huawei_ncm_mgmt(struct usbnet *dev,
+ struct huawei_cdc_ncm_state *drvstate, struct sk_buff *skb_out,
int mode) {
+ struct usb_cdc_ncm_nth16 *nth16 = (struct usb_cdc_ncm_nth16
*)skb_out-data;
+ struct cdc_ncm_ctx *ctx = drvstate-ctx;
+ struct usb_cdc_ncm_ndp16 *ndp16 = NULL;
+ int ret = -EINVAL;
+ u16 ndplen, index;
+
+ switch (mode) {
+ case NCM_INIT_FRAME:
+
+ /* Write a new NTH16 header */
+ nth16 = (struct usb_cdc_ncm_nth16 *)memset(skb_put(skb_out,
sizeof(struct usb_cdc_ncm_nth16)), 0, sizeof(struct usb_cdc_ncm_nth16));
+ if (!nth16) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ /* NTH16 signature and header length are known a-priori. */
+ nth16-dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN);
+ nth16-wHeaderLength = cpu_to_le16(sizeof(struct
usb_cdc_ncm_nth16));
+
+ /* TX sequence numbering */
+ nth16-wSequence = cpu_to_le16(ctx-tx_seq++);
+
+ /* Forget about previous SKB NDP */
+ drvstate-skb_ndp = NULL;
This is probably better done after you know you cannot fail.
Sure. Thank you.
+
+ /* Allocate a new NDP */
+ ndp16 = kzalloc(ctx-max_ndp_size, GFP_NOIO);
Where is this freed?
The intention wqas to free it in the NCM_COMMIT_NDP case.
Infact after allocating the pointer, I make a copy of it in the driver state
(drvstate) variable, and get back to it later.
Is this wrong?
Well, no, but it supposes a matched commit phase. Can you guarantee
that? I was under the oppression that in that phase you want to actually
give a frame over to the hardware.
--
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 RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
On Tue, 2015-06-23 at 00:32 +0200, Enrico Mioso wrote:
This patch introduces a new NCM tx engine, able to operate in standard-
and huawei-style mode.
In the first case, the NDP is disposed after the initial headers and
before any datagram.
What works:
- is able to communicate with compliant NCM devices:
I tested this with a board running the Linux g_ncm gadget driver.
What doesn't work:
- After some packets I start gettint LOTS of EVENT_RX_MEMORY from usbnet,
which fails to allocate an RX SKB in rx_submit(). Don't understand why,
any suggestion would be very welcome.
The tx_fixup function given here, even if actually working, should be
considered as an example: the NCM manager is used here simulating the
cdc_ncm.c behaviour.
Signed-off-by: Enrico Mioso mrkiko...@gmail.com
---
drivers/net/usb/huawei_cdc_ncm.c | 187
++-
1 file changed, 185 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/huawei_cdc_ncm.c
b/drivers/net/usb/huawei_cdc_ncm.c
index 735f7da..217802a 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -29,6 +29,35 @@
#include linux/usb/cdc-wdm.h
#include linux/usb/cdc_ncm.h
+/* NCM management operations: */
+
+/* NCM_INIT_FRAME: prepare for a new frame.
+ * NTH16 header is written to output SKB, NDP data is reset and last
+ * committed NDP pointer set to NULL.
+ * Now, data may be added to this NCM package.
+ */
+#define NCM_INIT_FRAME 1
+
+/* NCM_UPDATE_NDP: adds data to an NDP structure, hence updating it.
+ * Some checks are performed to be sure data fits in, respecting device and
+ * spec constrains.
+ * Normally the NDP is kept in memory and committed to the SKB only when
+ * requested. However, calling this method after NCM_COMMIT_NDP, causes it
to
+ * work directly on the already committed SKB copy. this allows for
flexibility
+ * in frame ordering.
+ */
+#define NCM_UPDATE_NDP 2
+
+/* Write NDP: commits NDP to output SKB.
+ * This method should be called only once per frame.
+ */
+#define NCM_COMMIT_NDP 3
+
+/* Finalizes NTH16 header: to be called when working in
+ * update-already-committed mode.
+ */
+#define NCM_FINALIZE_NTH 5
+
/* Driver data */
struct huawei_cdc_ncm_state {
struct cdc_ncm_ctx *ctx;
@@ -36,6 +65,16 @@ struct huawei_cdc_ncm_state {
struct usb_driver *subdriver;
struct usb_interface *control;
struct usb_interface *data;
+
+ /* Keeps track of where data starts and ends in SKBs. */
+ int data_start;
+ int data_len;
+
+ /* Last committed NDP for post-commit operations. */
+ struct usb_cdc_ncm_ndp16 *skb_ndp;
+
+ /* Non-committed NDP */
+ struct usb_cdc_ncm_ndp16 *ndp;
};
static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
@@ -53,6 +92,149 @@ static int huawei_cdc_ncm_manage_power(struct usbnet
*usbnet_dev, int on)
return 0;
}
+/* huawei_ncm_mgmt: flexible TX NCM manager.
+ *
+ * Once a non-zero status value is rturned, current frame should be discarded
+ * and operations restarted from scratch.
+ */
Is there any advantage in keeping this in a single function?
+int
+huawei_ncm_mgmt(struct usbnet *dev,
+ struct huawei_cdc_ncm_state *drvstate, struct sk_buff *skb_out,
int mode) {
+ struct usb_cdc_ncm_nth16 *nth16 = (struct usb_cdc_ncm_nth16
*)skb_out-data;
+ struct cdc_ncm_ctx *ctx = drvstate-ctx;
+ struct usb_cdc_ncm_ndp16 *ndp16 = NULL;
+ int ret = -EINVAL;
+ u16 ndplen, index;
+
+ switch (mode) {
+ case NCM_INIT_FRAME:
+
+ /* Write a new NTH16 header */
+ nth16 = (struct usb_cdc_ncm_nth16 *)memset(skb_put(skb_out,
sizeof(struct usb_cdc_ncm_nth16)), 0, sizeof(struct usb_cdc_ncm_nth16));
+ if (!nth16) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ /* NTH16 signature and header length are known a-priori. */
+ nth16-dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN);
+ nth16-wHeaderLength = cpu_to_le16(sizeof(struct
usb_cdc_ncm_nth16));
+
+ /* TX sequence numbering */
+ nth16-wSequence = cpu_to_le16(ctx-tx_seq++);
+
+ /* Forget about previous SKB NDP */
+ drvstate-skb_ndp = NULL;
This is probably better done after you know you cannot fail.
+
+ /* Allocate a new NDP */
+ ndp16 = kzalloc(ctx-max_ndp_size, GFP_NOIO);
Where is this freed?
+ if (!ndp16)
+ return ret;
+
+ /* Prepare a new NDP to add data on subsequent calls. */
+ drvstate-ndp = memset(ndp16, 0, ctx-max_ndp_size);
Either kzalloc() or memset(). Using both never makes sense.
+
+ /* Initial NDP length */
+ ndp16-wLength =
Re: [PATCH RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
On Tue, 2015-06-23 at 00:32 +0200, Enrico Mioso wrote:
+/* XXX rewrite, not multipacket */
Can you explain what you want to do here?
+struct sk_buff *
+huawei_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb_in, gfp_t flags)
{
+ struct huawei_cdc_ncm_state *drvstate = (void *)dev-data;
+ struct cdc_ncm_ctx *ctx = drvstate-ctx;
+ struct sk_buff *skb_out;
+ int status;
+
+ skb_out = alloc_skb(ctx-tx_max, GFP_ATOMIC);
You must test this for NULL
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] usb: xhci: remove stop device and ring doorbell in hub control and bus suspend
On 09.05.2015 04:15, Lu Baolu wrote: There is no need to call xhci_stop_device() and xhci_ring_device() in hub control and bus suspend functions since all device suspend and resume have been notified through device_suspend/device_resume interfaces. I was looking through this code again before sending it forward, and it occurred to me that this might be breaking the PORT_SUSPEND and PORT_SET_LINK_STATE port features for xhci root hub. In normal use these requests are called by usb core in usb_port_suspend(), which also now notifies xhci, which makes sure xhci_stop_device() is called. But I don't think there is anything preventing an URB to be sent to the xhci roothub with a PORT_SUSPEND or PORT_SET_LINK_STATE port feature request. In this case the usb_port_suspend() is not called, and no notify will stop the device. For example hub validation tests might do this -Mathias -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
Hi Oliver! And thank you again.
I like / recommend the usage of open messaging standards: my preferred XMPP ID
(JID) is: mrk...@jit.si.
On Thu, 25 Jun 2015, Oliver Neukum wrote:
Date: Thu, 25 Jun 2015 15:38:46
From: Oliver Neukum oneu...@suse.de
To: Enrico Mioso mrkiko...@gmail.com
Cc: linux-usb@vger.kernel.org, net...@vger.kernel.org
Subject: Re: [PATCH RFC] 2/2 huawei_cdc_ncm: introduce new TX ncm stack
On Thu, 2015-06-25 at 13:44 +0200, Enrico Mioso wrote:
On Thu, 25 Jun 2015, Oliver Neukum wrote:
Is there any advantage in keeping this in a single function?
I did this choice in the light of the fact I think the tx_fixup function will
become more complex than it is now, when aggregating frames.
Yes, but that is a reason to split the helpers up not the opposite.
Right - I understood only now your observation.
the only reason to write the manager that way was that it shouldn't become very
complex - it should simply do things to frames, helping out in building valid
NCM packages.
I answer here your other message to make it more convenient to read: my
intention for the tx_fixup function would be to:
- aggregate frames
- send them out when:
- a timer expires
How would you do that in tx_fixup()? If a timer is required then you
need a separate function.
Sure. I meant: I will adapt it in case needed, and expectin the code to become
a little bit more convoluted.
OR
- we have enough data in the aggregate, and cannot add more.
Yes.
You need a third case:
- the interface is taken down.
But in general the logic for that is already there. So can you explain
what additional goals you have?
regarding the timer logic I saw it in cdc_ncm.c, but I should study it in
more detail to understand it and implement it here where needed in case.
And sure, the interface goes down case is important: didn't think about it.
Thanks for the point.
the only other additional goal is to use the manager in such a way that NDP
will be disposed after frames.
I think that this split between NCM management and tx_fixup makes things more
flexible in general: this is the reason for re-writing it.
This is something done in cdc_ncm.c for example.
But here I have a question: by reading the comment in file
drivers/net/usb/rndis_host.c at line 572, there seem to be different opinions
in this matter.
That is a very old comment written for much slower devices.
rndis_host doesn't get much love nowadays.
Fine.
What to do ?
+int
+huawei_ncm_mgmt(struct usbnet *dev,
+ struct huawei_cdc_ncm_state *drvstate, struct sk_buff *skb_out,
int mode) {
+ struct usb_cdc_ncm_nth16 *nth16 = (struct usb_cdc_ncm_nth16
*)skb_out-data;
+ struct cdc_ncm_ctx *ctx = drvstate-ctx;
+ struct usb_cdc_ncm_ndp16 *ndp16 = NULL;
+ int ret = -EINVAL;
+ u16 ndplen, index;
+
+ switch (mode) {
+ case NCM_INIT_FRAME:
+
+ /* Write a new NTH16 header */
+ nth16 = (struct usb_cdc_ncm_nth16 *)memset(skb_put(skb_out,
sizeof(struct usb_cdc_ncm_nth16)), 0, sizeof(struct usb_cdc_ncm_nth16));
+ if (!nth16) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ /* NTH16 signature and header length are known a-priori. */
+ nth16-dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN);
+ nth16-wHeaderLength = cpu_to_le16(sizeof(struct
usb_cdc_ncm_nth16));
+
+ /* TX sequence numbering */
+ nth16-wSequence = cpu_to_le16(ctx-tx_seq++);
+
+ /* Forget about previous SKB NDP */
+ drvstate-skb_ndp = NULL;
This is probably better done after you know you cannot fail.
Sure. Thank you.
+
+ /* Allocate a new NDP */
+ ndp16 = kzalloc(ctx-max_ndp_size, GFP_NOIO);
Where is this freed?
The intention wqas to free it in the NCM_COMMIT_NDP case.
Infact after allocating the pointer, I make a copy of it in the driver state
(drvstate) variable, and get back to it later.
Is this wrong?
Well, no, but it supposes a matched commit phase. Can you guarantee
that? I was under the oppression that in that phase you want to actually
give a frame over to the hardware.
No. When Italk about committing, I think about writing things to out skb.
another reason why i found confortable writing the code this way was to
maintain a kind of statefullness in a more clean way.
But I understand this is kind of agruable, and I can if needed break it up as
desired.
Regarding the commit phase - I am not sure I understand your comment, sorry
about that.
However, my intention would be to allow the caller to do calls in sequences
like:
- init frame
- ncm update
- ncm update
- ncm update
...
- ncm update
- ncm commit
(to work in huawei mode)
OR
- ncm init frame
- ncm commit
- ncm update
- ncm update
- ncm update
- ncm update
- finalize nth
(to work in cdc_ncm.c-mode)..
But to prevent
64 byte EP0 OUT data transfer issue on Chipidea highspeed dual role controller
I am developing a custom USB device on a iMX6q platform (Wandboard) Chipidea HDRC (highspeed dual role controller). The HID interface consists of a single Interrupt IN ep and ep0. It is required to send HID reports from Host to Gadget over ep0 (with set_report cmd on hidraw interface) in OUT direction. The size of each HID report is 64 bytes. The Chipidea HDRC is unable to receive/ process the 64 bytes of data over ep0 (OUT). The setup stage is fine. No UDC interrupt is raised by controller to indicate completion of transaction. The same data transfer works fine for 64 bytes (63, 63 etc) and 64 bytes (65, 66 etc) where the transfers are split as 64+n. Analyzer logs attached. 1. Is there an issue with Chipidea HDRC receiving wMaxPacketSize (i.e. 64 byte) aligned data transfers on ep0 in OUT direction? 2. Anything that needs to be configured/ added in descriptor or source to have completion interrupt hit on 64 bytes data transfer on ep0 OUT? THank you!! File C:\Program Files (x86)\Lecroy\USBTracer\data.usb. From Packet #33198 to Packet #38355. Packet# ___|___ Packet(33198) Dir(--) H(S) SETUP(0xB4) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(8) ___| Time Stamp(1.3535 6645) ___|___ Packet(33199) Dir(--) H(S) DATA0(0xC3) Data(8 bytes) CRC16(0xB504) Pkt Len(16) ___| Time Stamp(1.3535 6669) ___|___ Packet(33200) Dir(--) H(S) ACK(0x4B) Pkt Len(6) Time Stamp(1.3535 6705) ___|___ Packet(38351) Dir(--) H(S) PING(0x2D) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(10) ___| Time Stamp(1.3663 3101) ___|___ Packet(38352) Dir(--) H(S) ACK(0x4B) Pkt Len(8) Time Stamp(1.3663 3129) ___|___ Packet(38353) Dir(--) H(S) OUT(0x87) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(10) ___| Time Stamp(1.3663 3275) ___|___ Packet(38354) Dir(--) H(S) DATA1(0xD2) Data(64 bytes) CRC16(0x9A52) Pkt Len(74) ___| Time Stamp(1.3663 3299) ___|___ Packet(38355) Dir(--) H(S) NYET(0x69) Pkt Len(8) Time Stamp(1.3663 3391) ___|___ File C:\Program Files (x86)\Lecroy\USBTracer\data.usb. From Packet #29969 to Packet #87486. Packet# ___|___ Packet(29969) Dir(--) H(S) SETUP(0xB4) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(8) ___| Time Stamp(1.0957 5739) ___|___ Packet(29970) Dir(--) H(S) DATA0(0xC3) Data(8 bytes) CRC16(0x3101) Pkt Len(16) ___| Time Stamp(1.0957 5763) ___|___ Packet(29971) Dir(--) H(S) ACK(0x4B) Pkt Len(8) Time Stamp(1.0957 5797) ___|___ Packet(35124) Dir(--) H(S) PING(0x2D) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(8) ___| Time Stamp(1.1085 2569) ___|___ Packet(35125) Dir(--) H(S) ACK(0x4B) Pkt Len(6) Time Stamp(1.1085 2597) ___|___ Packet(35126) Dir(--) H(S) OUT(0x87) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(8) ___| Time Stamp(1.1085 2729) ___|___ Packet(35127) Dir(--) H(S) DATA1(0xD2) Data(62 bytes) CRC16(0xADB3) Pkt Len(70) ___| Time Stamp(1.1085 2753) ___|___ Packet(35128) Dir(--) H(S) NYET(0x69) Pkt Len(6) Time Stamp(1.1085 2843) ___|___ Packet(87484) Dir(--) H(S) IN(0x96) ADDR(2) ENDP(0) CRC5(0x15) Pkt Len(10) ___| Time Stamp(1.2386 3931) ___|___ Packet(87485) Dir(--) H(S) DATA1(0xD2) Data(0 bytes) CRC16(0x) Pkt Len(10) ___| Time Stamp(1.2386 3959) ___|___ Packet(87486) Dir(--) H(S) ACK(0x4B) Pkt Len(8) Time Stamp(1.2386 3987) ___|___ File C:\Program Files (x86)\Lecroy\USBTracer\data.usb. From Packet #37091 to Packet #87527. Packet# ___|___ Packet(37091)
[PATCH 1/1] usb: usleep_range is preferred over udelay where wakeup is flexible
According to Documentation/timers/timers-howto.txt
udelay() is only called once from a place where sleeping is allowed.
We can replace it with a call to usleep_range()
with a reasonable upper limit.
Signed-off-by: Sunny Kumar sunny.kumar@gmail.com
---
drivers/usb/storage/transport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 540add2..5e67f63 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -,7 +,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct
us_data *us)
* command phase and the data phase. Some devices need a little
* more than that, probably because of clock rate inaccuracies. */
if (unlikely(us-fflags US_FL_GO_SLOW))
- udelay(125);
+ usleep_range(125, 150);
if (transfer_length) {
unsigned int pipe = srb-sc_data_direction == DMA_FROM_DEVICE ?
--
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: 64 byte EP0 OUT data transfer issue on Chipidea highspeed dual role controller
cc Alexander Thanks On Thu, Jun 25, 2015 at 7:41 PM, Jayan John jayanjoh...@gmail.com wrote: I am developing a custom USB device on a iMX6q platform (Wandboard) Chipidea HDRC (highspeed dual role controller). The HID interface consists of a single Interrupt IN ep and ep0. It is required to send HID reports from Host to Gadget over ep0 (with set_report cmd on hidraw interface) in OUT direction. The size of each HID report is 64 bytes. The Chipidea HDRC is unable to receive/ process the 64 bytes of data over ep0 (OUT). The setup stage is fine. No UDC interrupt is raised by controller to indicate completion of transaction. The same data transfer works fine for 64 bytes (63, 63 etc) and 64 bytes (65, 66 etc) where the transfers are split as 64+n. Analyzer logs attached. 1. Is there an issue with Chipidea HDRC receiving wMaxPacketSize (i.e. 64 byte) aligned data transfers on ep0 in OUT direction? 2. Anything that needs to be configured/ added in descriptor or source to have completion interrupt hit on 64 bytes data transfer on ep0 OUT? THank you!! -- 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 TRIVIAL] drivers/usb/gadget/composite.c: i18n is not an acronym
Ping? On Sun, May 31, 2015 at 3:52 PM, Diego Viola diego.vi...@gmail.com wrote: Signed-off-by: Diego Viola diego.vi...@gmail.com --- drivers/usb/gadget/composite.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 4e3447b..79a3ae0 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -896,7 +896,7 @@ void usb_remove_config(struct usb_composite_dev *cdev, /* We support strings in multiple languages ... string descriptor zero * says which languages are supported. The typical case will be that - * only one language (probably English) is used, with I18N handled on + * only one language (probably English) is used, with i18n handled on * the host side. */ @@ -949,7 +949,7 @@ static int get_string(struct usb_composite_dev *cdev, struct usb_function *f; int len; - /* Yes, not only is USB's I18N support probably more than most + /* Yes, not only is USB's i18n support probably more than most * folk will ever care about ... also, it's all supported here. * (Except for UTF8 support for Unicode's Astral Planes.) */ -- 2.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: cp210x: add ID for Aruba Networks controllers
Add the USB serial console device ID for Aruba Networks 7xxx series
controllers which have a USB port for their serial console.
Signed-off-by: Peter Sanford pe...@sanford.io
---
drivers/usb/serial/cp210x.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index ffd739e..eac7cca 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -187,6 +187,7 @@ static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x1FB9, 0x0602) }, /* Lake Shore Model 648 Magnet Power
Supply */
{ USB_DEVICE(0x1FB9, 0x0700) }, /* Lake Shore Model 737 VSM Controller
*/
{ USB_DEVICE(0x1FB9, 0x0701) }, /* Lake Shore Model 776 Hall Matrix */
+ { USB_DEVICE(0x2626, 0xEA60) }, /* Aruba Networks 7xxx USB Serial
Console */
{ USB_DEVICE(0x3195, 0xF190) }, /* Link Instruments MSO-19 */
{ USB_DEVICE(0x3195, 0xF280) }, /* Link Instruments MSO-28 */
{ USB_DEVICE(0x3195, 0xF281) }, /* Link Instruments MSO-28 */
--
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: urb-interval differently interpreted via xhci for 2.0
(quoting fixed) Bernd Porr wrote: Alan Stern wrote: On Wed, 24 Jun 2015, Bernd Porr wrote: I use urb-interval to control the sampling rate of these devices. That works fine with the ehci driver. When I use the xhci driver it seems to be interpreting the urb-interval in a different way and/or ignoring it? As you have seen from the source code, xhci-hcd ignores the urb-interval value provided by the driver and instead uses the value from the endpoint descriptor. And the xHCI specification requires this (section 6.2.3.6): | For [isochronous] enpoint types, system software shall translate the | bInterval field in the USB Endpoint Descriptor to the appropriate | value for this field. Currently, I think the only way to do what you want is to set ep-desc.bInterval to the desired value and then call usb_set_interface(). I guess the cleanest solution is to write additional code in both the firmware and the driver for xhci and then once the older drivers change roll that over. The best approach is (I think but correct me) to ditch ISO for now just for xhci and then much later also for the other drivers. Please note that you can implement Alan's suggestion in a mostly backwards-compatible way by having the device offering multiple alternate settings; an old driver (using EHCI) would still be able to use a different interval with the 'wrong' alternate setting. Regards, Clemens -- 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: urb-interval differently interpreted via xhci for 2.0
Hi Clemens, thanks for the reply. I'm using the Cypress chip which offers a fixed set of alternative settings and I use the one offering iso transfer so I cannot easily do that. There are loads of sigma boards out there and because of the linux firmware and the drivers not really into sync I need to do something that keeps it intact. The problem with setting the bInterval is that this happens at the init of the driver but if the user wants to switch between different sampling rates later on that this might upset other transfers. That really needs to be on the level of a single endpoint and not resetting in the worst case the whole setup (which switching between alternate settings). Here's what I'll do: for xhci I'll do a soft interval in my firmware and only send a packet back once a counter has counted to zero. Otherwise I just send back a zero length packet. I keep bInterval=1 (125us) and then just send back data from the firmware at the sampling rate requested by the driver. That obviously is exactly the opposite what the xhci people have intended but in my case I need to init the endpoints in a way that I can transfer at 8kHz if I want to. It's important that the user can switch between different sampling rates at any moment and keeping the latency always as low as possible. At the end we use it for robot control. So, I'll waste a bit of bandwidth to keep it as responsive as possible. /Bernd Clemens Ladisch wrote: (quoting fixed) Bernd Porr wrote: Alan Stern wrote: On Wed, 24 Jun 2015, Bernd Porr wrote: I use urb-interval to control the sampling rate of these devices. That works fine with the ehci driver. When I use the xhci driver it seems to be interpreting the urb-interval in a different way and/or ignoring it? As you have seen from the source code, xhci-hcd ignores the urb-interval value provided by the driver and instead uses the value from the endpoint descriptor. And the xHCI specification requires this (section 6.2.3.6): | For [isochronous] enpoint types, system software shall translate the | bInterval field in the USB Endpoint Descriptor to the appropriate | value for this field. Currently, I think the only way to do what you want is to set ep-desc.bInterval to the desired value and then call usb_set_interface(). I guess the cleanest solution is to write additional code in both the firmware and the driver for xhci and then once the older drivers change roll that over. The best approach is (I think but correct me) to ditch ISO for now just for xhci and then much later also for the other drivers. Please note that you can implement Alan's suggestion in a mostly backwards-compatible way by having the device offering multiple alternate settings; an old driver (using EHCI) would still be able to use a different interval with the 'wrong' alternate setting. Regards, Clemens -- http://www.berndporr.me.uk http://www.imdb.com/name/nm3293421/ http://www.facebook.com/bernd.porr +44 (0)7840 340069 -- 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/1] usb:serial:f81534 Add F81532/534 Driver
On Thu, Jun 25, 2015 at 05:21:01PM +0800, Peter Hung wrote: Hi Johan, Johan Hovold 於 2015/6/25 下午 04:06 寫道: - As Greg already mentioned, you need to implement gpio support using gpiolib, not a custom sysfs interface I don't have time to look closer at the architectural bits until next week I'm afraid, but perhaps you could start with the above. Thanks for your advices, I'll try to improve it again. But I had something need to clarify. 1. The sysfs interface of the driver provide limited output pin control. only support output 0/1, not support input mode. Should I implement it as gpiolib? Yes. 2. If it still recommends to implement with gpiolib, Could I implement it and preserve the sysfs interface for legacy? No, I'm afraid not. We don't want to have to maintain out-of-tree legacy interfaces. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
