Messages in this thread | | | Date | Wed, 18 Nov 2015 09:12:47 +0100 | From | Dave Penkler <> | Subject | Re: [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
| |