lkml.org 
[lkml]   [2015]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 1/5] Implement an ioctl to support the USMTMC-USB488 READ_STATUS_BYTE operation.
Hi Andy,
On Sun, Nov 15, 2015 at 10:04:10PM +0200, Andy Shevchenko wrote:
> On Sun, Nov 15, 2015 at 8:39 PM, Dave Penkler <dpenkler@gmail.com> wrote:

snip

> > +
>
> Redundant empty line.
>

ok

>
> > + data->iin_bTag = 2;
>
> Hmm??? Why 2?
> A-ha, below I found a comment. Something might be good to have here as well.
>

Added comment

> > +
>
> Redundant empty line.
>

ok

> > +
> > + if (data->iin_buffer[0] & 0x80) {
> > + /* check for valid STB notification */
> > + if ((data->iin_buffer[0] & 0x7f) > 1) {
>
> It's the same as
> if (data->iin_buffer[0] & 0x7e) {
>

Yes but when reading the spec and the code it is more obvious that here
we are testing for the value in bits D6..D0 to be a valid iin_bTag return.
(See Table 7 in the USBTMC-USB488 spec.)

> > + dev_dbg(&data->intf->dev,
> > + "%s - urb terminated, status: %d\n",
>
> I heard that dynamic debug adds function name.
>
> > + __func__, status);

I checked in device.h and could not find any trace of it. I'm on 4.4.0-rc1.

> > +static void usbtmc_free_int(struct usbtmc_device_data *data)
> > +{
> > + if (data->iin_ep_present) {
> > + if (data->iin_urb) {
>
> Why not
>
> if (!data->iin_ep_present || !data->iin_urb)
> return;
>
> ?
>

OK

Thanks,
-Dave


\
 
 \ /
  Last update: 2015-11-18 09:41    [W:0.065 / U:5.016 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site