Re: [PATCH v2 2/3] net: usb: cdc_enc: New driver exposing USB CDC encapsulated protocols

Oliver Neukum [off-list ref] writes:

Am Sonntag, 15. Januar 2012, 07:40:36 schrieb Bjørn Mork:

quoted

USB CDC Ethernet devices may encapsulate vendor specific protocols inside
USB_CDC_SEND_ENCAPSULATED_COMMAND +USB_CDC_GET_ENCAPSULATED_RESPONSE
control messages. Devices based on Qualcomm MSM
chipsets are known to use this for exporting the Qualcomm
MSM Interface (QMI) protocol. Examples of such devices are
the Huawei E392 and E398 LTE modems.

The problem is that this is another version of the code already
in cdc-wdm, which is tested well.

Huh?  I thought that was something entirely different.  Isn't it?

Anyway, I see that I should look thoroughly at it as it does these
things in a much nicer way.  Thanks for the pointer.

I'd really like to unify this stuff.

That makes sense.  

[snipped a number of detailed issues which I obviously need to fix]

quoted

+
+/* per CDC_ENC interface state */
+struct cdc_enc_state {
+	struct usb_interface *intf;
+	struct urb *urb;		/* receive urb */
+	struct urb *interrupt;		/* interrupt urb */
+	unsigned char rcvbuf[CDC_ENC_BUFLEN];     /* receive buffer */

This is a violation of the DMA requirements. You must use kmalloc.

This probably only demonstrates my lack of understanding these
requirements.  I thought this was OK as long as I kmalloc'ed the whole
struct, but I see now that that isn't necessarily true.

I had a not very detailed look at this driver, but I would really prefer
if you could have a look at making unified code for this, cdc-wdm and cdc-acm.

Will do.  Thanks a lot for the fast review.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html