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@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
