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

[Fangxiaozhi (Franko)]

Dear Matthew:


> -----Original Message-----
> From: Matthew Dharm [mailto:mdharm-usb@xxxxxxxxxxxxxxxxxx]
> Sent: Wednesday, December 19, 2012 11:41 PM
> To: Sebastian Andrzej Siewior
> Cc: Fangxiaozhi (Franko); linux-usb@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Xueguiying (Zihan); Linlei (Lei Lin);
> greg@xxxxxxxxx; Yili (Neil); Wangyuhua (Roger, Credit); Huqiao; balbi@xxxxxx
> Subject: Re: [PATCH 2/2]linux-usb:optimize to match the Huawei USB storage
> devices and support new switch command
> 
> 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.
	You mean that we have to add some comment in the source code, 
to explain why we don't read the response. Right?
	
> 
> 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?  
	In the dongles_pid function, we have check all the product IDs for our dongles, which is assigned for all of our Mobile Broadband products in our company. So the product ID of our new dongle in future, will also be included in this list.
	In our lab, we can configure our dongle firmware to support all of these product ID. We have test them(cover all the product ID), and this function works fine.

>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.
	As far as I know, the product ID list in dongles_pid function includes all. We will not add any other product ID for our dongle. So we need not update the product ID list in dongles_pid function in future.
	However, I also will add the comment to highlight the area of code, as your advice did.
> 
> Matt
> 
> --
> Matthew Dharm
> Maintainer, USB Mass Storage driver for Linux

Best Regards,
Franko Fang
¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_