Em Tue, 17 Feb 2015 13:45:50 +0000
David Howells <dhowells@xxxxxxxxxx> escreveu:
> Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> wrote:
>
> > I would do a s/ix_USB_PID_// in the above, in order to simplify the
> > namespace and to avoid giving the false impression that those are vendor
> > IDs.
>
> Okay.
>
> > If you look below on your patch, even you forgot to add a "ix_" prefix into
> > one of the entires ;)
>
> Bah. I realised I'd forgotten and went back to try and fix them up.
>
> > Just calling MEDION_MD95700..MYGICA_T230 would be enough and shorter.
>
> True.
>
> > static struct usb_device_id cxusb_table [] = {
> > [VID_MEDION] = {USB_VID_MEDION, USB_PID_MEDION_MD95700},
> > ...
>
> That should really be:
>
> [VID_MEDION_MD95700] = {USB_VID_MEDION, USB_PID_MEDION_MD95700},
Actually, MEDION_MD95700 :)
>
> since the index number is the model, not the vendor, which brings me to:
>
> [DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM] = {USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM},
>
> which would be excessively long.
True. Perhaps:
[DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM] = {
USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM
},
Would be better, as it follows better the CodingStyle.
>
> > > + _(USB_VID_MEDION, USB_PID_MEDION_MD95700), // 0
> >
> > Please don't use c99 comments. Also, I don't think that the comments would
> > help, as the entries on this table doesn't need to follow the same order
> > as defined at the enum.
>
> Sorry, yes, I meant those as guides purely for when I was converting numbers
> to symbols.
>
> David
Thanks!
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
