Re: uftdi.c code review request (simple)

In article <[email protected]>,
Claus Andersen  <[email protected]> wrote:
>-=-=-=-=-=-
>
>Hello!
>
>I have made some changes to the uftdi driver on my 3.0.1 system and 
>thought it might be worth contributing them back. This is however a first 
>for me so I would highly appreciate any and all feedback. I have attached 
>my "diff -u".
>
>1) I changed USB_MATCH to use usb_lookup like the rest of the u* drivers 
>do. I think it looks cleaner and is easier to maintain?
>
>2) I added the following devices as they where known in Free/OpenBSD:
> 	B&B Electronics uLinks RS-422/485
> 	Falcom Twist GSM/GPRS modem
> 	Falcom Samba 55/56 GSM/GPRS modem
> 	Future Technology Devices KW
> 	Future Technology Devices YS
> 	Future Technology Devices Y6
> 	Future Technology Devices Y8
> 	Future Technology Devices IC
> 	Future Technology Devices DB9
> 	Future Technology Devices RS232
> 	Future Technology Devices Y9
> 	Future Technology Devices / Coastal ChipWorks TNC-X
> 	Future Technology Devices / Matrix Orbital MX200 Series LCD
> 	Future Technology Devices / Matrix Orbital LK202-24 LCD
> 	Future Technology Devices / Matrix Orbital LK204-24 LCD
> 	Future Technology Devices / Crystalfontz CFA-632 LCD
> 	Future Technology Devices / Crystalfontz CFA-634 LCD
> 	Future Technology Devices / Crystalfontz CFA-633 LCD
> 	Interpid Control Systems ValueCAN
> 	Interpid Control Systems NeoVI Blue
> 	SIIG SIIG2 US2308 Serial
>(Some of these where already listed in usbdevs but not in uftdi)
>(I do only have a Falcom Samba to test against)
>
>3) I added the "switch (uaa->vendor)" in USB_ATTACH to avoid clashing 
>product id's across vendors. The switch was choosen rather than if to make 
>additions easier. Switch is faster than if (Suboptimizing is still 
>optimizing ;-)) OK?
>
>4) The "switch (uaa->product)" in USB_ATTACH seemed bloated. Only one 
>device is UFTDI_TYPE_SIO - the rest are UFTDI_TYPE_8U232AM. The previous 
>"default:" had a /* Can't happen */ comment as we should only see devices 
>matched by USB_MATCH. So I thought it reasonable and less prone to error 
>when adding devices to make UFTDI_TYPE_8U232AM the default?
>
>5) If I'm wrong regarding to 3 or 4 and the devices should be handled 
>explicitly in USB_ATTACH would it not be nicer with a static struct á la 
>"usb_lookup"?
>
>6) It says "The ucom layer needs to be extended first" to handle more 
>ports. Has this happened?

I don't know about 6, but I agree with 1-5.

I committed your changes as you posted them. Thanks. In the future, please
send patches using send-pr so that they don't get lost!

Best,

christos