Re: [PATCH 2/2]linux-usb:optimize to match the Huawei USB storagedevices and support new switch command

[Matthew Dharm]

On Wed, Dec 19, 2012 at 12:34 AM, Sebastian Andrzej Siewior
<sebastian@xxxxxxxxxxxxx> wrote:
> On Wed, Dec 19, 2012 at 03:13:32AM +0000, Fangxiaozhi (Franko) wrote:
>> > And shouldn't you read something from the us->recv_bulk_pipe after
>> > that?
>>       Well, because our device will re-connect to switch the ports if it receives the command.
>>       So it is not necessary to read the response of the command.
>
> Hmm. I guess this for Matthew / Greg to decide, I don't insist on anything.
> Maybe a comment would be nice because now it looks, atleast to me, that
> something is missing.

I think an unusual situation like that deserves a comment that
explains that the device is about to disconnect / reconnect, so
reading status is not necessary.

I am also concerned about the error of using &bcbw instead of bcbw.  I
doubt this code would have worked with that typo in place.  How was
this patch tested?

Also, the dongles_pid function is really just a different
implementation of the unusual_devs.h table.  I think that it is much
easier for people to add new entries to the table, rather than edit
your code, when new dongles are released.  BUT, your code includes
many more PIDs than the table did.  Again, how was this tested for the
new PIDs covered?  At a minimum, some comment in dongles_pid is
required to highlight this area of code for possible future expansion
as new devices are released.

Matt

-- 
Matthew Dharm
Maintainer, USB Mass Storage driver for Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/