Em Tue, 17 Feb 2015 13:45:50 +0000
David Howells <dhowe...@redhat.com> escreveu:

> Mauro Carvalho Chehab <mche...@osg.samsung.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to