lkml.org 
[lkml]   [2011]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v11 4/7] usb:gadget: Add SuperSpeed support to the Gadget Framework
Date
Hi Sebastian

> >@@ -157,7 +167,35 @@ ep_found:
> > /* commit results */
> > _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
> > _ep->desc = chosen_desc;
> >-
> >+ _ep->comp_desc = NULL;
> >+ _ep->maxburst = 0;
> >+ _ep->mult = 0;
> >+ if (want_comp_desc) {
>
> if (!want_comp_desc)
> return 0;
>
> I have one ident level less :)
Done :)

> >+/**
> >+ * bos_desc() - prepares the BOS descriptor.
> >+ * @cdev: pointer to usb_composite device to generate the bos
> >+ * descriptor for
> >+ *
> >+ * This function generates the BOS (Binary Device Object)
> >+ * descriptor and its device capabilities descriptors. The BOS
> >+ * descriptor should be supported by a SuperSpeed device.
> >+ */
> >+static int bos_desc(struct usb_composite_dev *cdev)
> >+{
> >+ struct usb_ext_cap_descriptor *usb_ext;
> >+ struct usb_ss_cap_descriptor *ss_cap;
> >+ struct usb_dcd_config_params dcd_config_params;
> >+ struct usb_bos_descriptor *bos = cdev->req->buf;
> >+
> >+ bos->bLength = USB_DT_BOS_SIZE;
> >+ bos->bDescriptorType = USB_DT_BOS;
> >+
> >+ bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE);
> >+ bos->bNumDeviceCaps = 0;
> >+
> >+ /*
> >+ * A SuperSpeed device shall include the USB2.0 extension
> descriptor
> >+ * and shall support LPM when operating in USB2.0 HS mode.
> >+ */
> >+ usb_ext = (struct usb_ext_cap_descriptor *)
> cdev->req->buf is (void *) so you can skip that cast.
>
> >+ (cdev->req->buf+bos->wTotalLength);
> a space between + please. bos->wTotalLength is le16 so you can't simply
> do that way.
>
> What about something like
>
> usb_ext = (struct usb_ext_cap_descriptor *)(bos + 1)
>
> ?

Added the spaces and the le16_to_cpu(bos->wTotalLength).
It seems clearer to me to leave it as
usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
if that's ok with you.

> >@@ -499,6 +633,9 @@ static int set_config(struct usb_composite_dev
> *cdev,
> > case USB_SPEED_LOW: speed = "low"; break;
> > case USB_SPEED_FULL: speed = "full"; break;
> > case USB_SPEED_HIGH: speed = "high"; break;
> >+ case USB_SPEED_SUPER:
> >+ speed = "super";
> >+ break;
>
> This is not my favorite style either but please do it the way the other
> three are done.

Well here is the dilemma: if I do it the other tree were done - I get
checkpatch error.
You're right, adding this the way it's above doesn't look too good but when
I fixed the other three I was asked not to do so in this patch, which also
makes sense since it has nothing to do with SS support...
So what do I do? Submit with a checkpatch error?


Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum






\
 
 \ /
  Last update: 2011-05-22 09:21    [W:0.073 / U:0.928 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site