* [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers
@ 2015-02-16 15:33 David Howells
2015-02-17 11:57 ` Mauro Carvalho Chehab
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: David Howells @ 2015-02-16 15:33 UTC (permalink / raw)
To: mchehab; +Cc: mkrufky, linux-media
Use enum to represent table offsets rather than hard-coding numbers to avoid
problems with the numbers becoming out of sync with the table.
Signed-off-by: David Howells <dhowells@redhat.com>
---
drivers/media/usb/dvb-usb/cxusb.c | 115 +++++++++++++++++++++++--------------
1 file changed, 71 insertions(+), 44 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c
index f327c49..5bb1c5c 100644
--- a/drivers/media/usb/dvb-usb/cxusb.c
+++ b/drivers/media/usb/dvb-usb/cxusb.c
@@ -1516,28 +1516,55 @@ static void cxusb_disconnect(struct usb_interface *intf)
dvb_usb_device_exit(intf);
}
-static struct usb_device_id cxusb_table [] = {
- { USB_DEVICE(USB_VID_MEDION, USB_PID_MEDION_MD95700) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_COLD) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_WARM) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_COLD) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_WARM) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_COLD) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_WARM) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM) },
- { USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_VOLAR_A868R) },
- { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2) },
- { USB_DEVICE(USB_VID_CONEXANT, USB_PID_CONEXANT_D680_DMB) },
- { USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_D689) },
- { USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230) },
+enum cxusb_table_index {
+ ix_USB_PID_MEDION_MD95700,
+ ix_USB_PID_DVICO_BLUEBIRD_LG064F_COLD,
+ ix_USB_PID_DVICO_BLUEBIRD_LG064F_WARM,
+ ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD,
+ ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM,
+ ix_USB_PID_DVICO_BLUEBIRD_LGZ201_COLD,
+ ix_USB_PID_DVICO_BLUEBIRD_LGZ201_WARM,
+ ix_USB_PID_DVICO_BLUEBIRD_TH7579_COLD,
+ ix_USB_PID_DVICO_BLUEBIRD_TH7579_WARM,
+ ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD,
+ ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM,
+ ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD,
+ ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM,
+ ix_USB_PID_DVICO_BLUEBIRD_DUAL_4,
+ ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2,
+ ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM,
+ ix_USB_PID_AVERMEDIA_VOLAR_A868R,
+ ix_USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2,
+ ix_USB_PID_CONEXANT_D680_DMB,
+ ix_USB_PID_MYGICA_D689,
+ ix_USB_PID_MYGICA_T230,
+ NR__cxusb_table_index
+};
+
+static struct usb_device_id cxusb_table [NR__cxusb_table_index + 1] = {
+#define _(vend, prod) [ix_##prod] = { vend, prod }
+ _(USB_VID_MEDION, USB_PID_MEDION_MD95700), // 0
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_COLD),
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_WARM), // 2
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD),
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM), // 4
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_COLD),
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_WARM), // 6
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_COLD),
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_WARM), // 8
+ _(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD),
+ _(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM), // 10
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD),
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM), // 12
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4),
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2), // 14
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM),
+ _(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_VOLAR_A868R), // 16
+ _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2),
+ _(USB_VID_CONEXANT, USB_PID_CONEXANT_D680_DMB), // 18
+ _(USB_VID_CONEXANT, USB_PID_MYGICA_D689),
+ _(USB_VID_CONEXANT, USB_PID_MYGICA_T230), // 20
+#undef _
{} /* Terminating entry */
};
MODULE_DEVICE_TABLE (usb, cxusb_table);
@@ -1581,7 +1608,7 @@ static struct dvb_usb_device_properties cxusb_medion_properties = {
.devices = {
{ "Medion MD95700 (MDUSBTV-HYBRID)",
{ NULL },
- { &cxusb_table[0], NULL },
+ { &cxusb_table[ix_USB_PID_MEDION_MD95700], NULL },
},
}
};
@@ -1637,8 +1664,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_lgh064f_properties = {
.num_device_descs = 1,
.devices = {
{ "DViCO FusionHDTV5 USB Gold",
- { &cxusb_table[1], NULL },
- { &cxusb_table[2], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_LG064F_COLD], NULL },
+ { &cxusb_table[USB_PID_DVICO_BLUEBIRD_LG064F_WARM], NULL },
},
}
};
@@ -1693,16 +1720,16 @@ static struct dvb_usb_device_properties cxusb_bluebird_dee1601_properties = {
.num_device_descs = 3,
.devices = {
{ "DViCO FusionHDTV DVB-T Dual USB",
- { &cxusb_table[3], NULL },
- { &cxusb_table[4], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM], NULL },
},
{ "DigitalNow DVB-T Dual USB",
- { &cxusb_table[9], NULL },
- { &cxusb_table[10], NULL },
+ { &cxusb_table[ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD], NULL },
+ { &cxusb_table[ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM], NULL },
},
{ "DViCO FusionHDTV DVB-T Dual Digital 2",
- { &cxusb_table[11], NULL },
- { &cxusb_table[12], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM], NULL },
},
}
};
@@ -1756,8 +1783,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_lgz201_properties = {
.num_device_descs = 1,
.devices = {
{ "DViCO FusionHDTV DVB-T USB (LGZ201)",
- { &cxusb_table[5], NULL },
- { &cxusb_table[6], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_LGZ201_COLD], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_LGZ201_WARM], NULL },
},
}
};
@@ -1812,8 +1839,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_dtt7579_properties = {
.num_device_descs = 1,
.devices = {
{ "DViCO FusionHDTV DVB-T USB (TH7579)",
- { &cxusb_table[7], NULL },
- { &cxusb_table[8], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_TH7579_COLD], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_TH7579_WARM], NULL },
},
}
};
@@ -1865,7 +1892,7 @@ static struct dvb_usb_device_properties cxusb_bluebird_dualdig4_properties = {
.devices = {
{ "DViCO FusionHDTV DVB-T Dual Digital 4",
{ NULL },
- { &cxusb_table[13], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_4], NULL },
},
}
};
@@ -1918,7 +1945,7 @@ static struct dvb_usb_device_properties cxusb_bluebird_nano2_properties = {
.devices = {
{ "DViCO FusionHDTV DVB-T NANO2",
{ NULL },
- { &cxusb_table[14], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2], NULL },
},
}
};
@@ -1972,8 +1999,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_nano2_needsfirmware_prope
.num_device_descs = 1,
.devices = {
{ "DViCO FusionHDTV DVB-T NANO2 w/o firmware",
- { &cxusb_table[14], NULL },
- { &cxusb_table[15], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM], NULL },
},
}
};
@@ -2017,7 +2044,7 @@ static struct dvb_usb_device_properties cxusb_aver_a868r_properties = {
.devices = {
{ "AVerMedia AVerTVHD Volar (A868R)",
{ NULL },
- { &cxusb_table[16], NULL },
+ { &cxusb_table[ix_USB_PID_AVERMEDIA_VOLAR_A868R], NULL },
},
}
};
@@ -2071,7 +2098,7 @@ struct dvb_usb_device_properties cxusb_bluebird_dualdig4_rev2_properties = {
.devices = {
{ "DViCO FusionHDTV DVB-T Dual Digital 4 (rev 2)",
{ NULL },
- { &cxusb_table[17], NULL },
+ { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2], NULL },
},
}
};
@@ -2125,7 +2152,7 @@ static struct dvb_usb_device_properties cxusb_d680_dmb_properties = {
{
"Conexant DMB-TH Stick",
{ NULL },
- { &cxusb_table[18], NULL },
+ { &cxusb_table[ix_USB_PID_CONEXANT_D680_DMB], NULL },
},
}
};
@@ -2179,7 +2206,7 @@ static struct dvb_usb_device_properties cxusb_mygica_d689_properties = {
{
"Mygica D689 DMB-TH",
{ NULL },
- { &cxusb_table[19], NULL },
+ { &cxusb_table[ix_USB_PID_MYGICA_D689], NULL },
},
}
};
@@ -2232,7 +2259,7 @@ static struct dvb_usb_device_properties cxusb_mygica_t230_properties = {
{
"Mygica T230 DVB-T/T2/C",
{ NULL },
- { &cxusb_table[20], NULL },
+ { &cxusb_table[ix_USB_PID_MYGICA_T230], NULL },
},
}
};
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers
2015-02-16 15:33 [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers David Howells
@ 2015-02-17 11:57 ` Mauro Carvalho Chehab
2015-02-17 13:45 ` David Howells
2015-02-17 13:52 ` David Howells
2 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-02-17 11:57 UTC (permalink / raw)
To: David Howells; +Cc: mkrufky, linux-media
Em Mon, 16 Feb 2015 15:33:07 +0000
David Howells <dhowells@redhat.com> escreveu:
> Use enum to represent table offsets rather than hard-coding numbers to avoid
> problems with the numbers becoming out of sync with the table.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> drivers/media/usb/dvb-usb/cxusb.c | 115 +++++++++++++++++++++++--------------
> 1 file changed, 71 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb/cxusb.c b/drivers/media/usb/dvb-usb/cxusb.c
> index f327c49..5bb1c5c 100644
> --- a/drivers/media/usb/dvb-usb/cxusb.c
> +++ b/drivers/media/usb/dvb-usb/cxusb.c
> @@ -1516,28 +1516,55 @@ static void cxusb_disconnect(struct usb_interface *intf)
> dvb_usb_device_exit(intf);
> }
>
> -static struct usb_device_id cxusb_table [] = {
> - { USB_DEVICE(USB_VID_MEDION, USB_PID_MEDION_MD95700) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_COLD) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_WARM) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_COLD) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_WARM) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_COLD) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_WARM) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM) },
> - { USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_VOLAR_A868R) },
> - { USB_DEVICE(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2) },
> - { USB_DEVICE(USB_VID_CONEXANT, USB_PID_CONEXANT_D680_DMB) },
> - { USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_D689) },
> - { USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230) },
> +enum cxusb_table_index {
> + ix_USB_PID_MEDION_MD95700,
> + ix_USB_PID_DVICO_BLUEBIRD_LG064F_COLD,
> + ix_USB_PID_DVICO_BLUEBIRD_LG064F_WARM,
> + ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD,
> + ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM,
> + ix_USB_PID_DVICO_BLUEBIRD_LGZ201_COLD,
> + ix_USB_PID_DVICO_BLUEBIRD_LGZ201_WARM,
> + ix_USB_PID_DVICO_BLUEBIRD_TH7579_COLD,
> + ix_USB_PID_DVICO_BLUEBIRD_TH7579_WARM,
> + ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD,
> + ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM,
> + ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD,
> + ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM,
> + ix_USB_PID_DVICO_BLUEBIRD_DUAL_4,
> + ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2,
> + ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM,
> + ix_USB_PID_AVERMEDIA_VOLAR_A868R,
> + ix_USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2,
> + ix_USB_PID_CONEXANT_D680_DMB,
> + ix_USB_PID_MYGICA_D689,
> + ix_USB_PID_MYGICA_T230,
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. If you look below on your patch, even you forgot to add a "ix_"
prefix into one of the entires ;)
Just calling MEDION_MD95700..MYGICA_T230 would be enough and shorter.
> + NR__cxusb_table_index
> +};
> +
> +static struct usb_device_id cxusb_table [NR__cxusb_table_index + 1] = {
> +#define _(vend, prod) [ix_##prod] = { vend, prod }
Of course, if you change s/ix_USB_PID_//, here, you'll need to change the
macro to:
#define _(vend, prod) [prod] = { vend, USB_PID_##prod }
And maybe add some comment. Yet, I think that the best would be to just
remove the macro, an just use:
static struct usb_device_id cxusb_table [] = {
[VID_MEDION] = {USB_VID_MEDION, USB_PID_MEDION_MD95700},
...
}
> + _(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.
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_COLD),
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LG064F_WARM), // 2
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD),
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM), // 4
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_COLD),
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_LGZ201_WARM), // 6
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_COLD),
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_TH7579_WARM), // 8
> + _(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD),
> + _(USB_VID_DVICO, USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM), // 10
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD),
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM), // 12
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4),
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2), // 14
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM),
> + _(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_VOLAR_A868R), // 16
> + _(USB_VID_DVICO, USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2),
> + _(USB_VID_CONEXANT, USB_PID_CONEXANT_D680_DMB), // 18
> + _(USB_VID_CONEXANT, USB_PID_MYGICA_D689),
> + _(USB_VID_CONEXANT, USB_PID_MYGICA_T230), // 20
> +#undef _
> {} /* Terminating entry */
> };
> MODULE_DEVICE_TABLE (usb, cxusb_table);
> @@ -1581,7 +1608,7 @@ static struct dvb_usb_device_properties cxusb_medion_properties = {
> .devices = {
> { "Medion MD95700 (MDUSBTV-HYBRID)",
> { NULL },
> - { &cxusb_table[0], NULL },
> + { &cxusb_table[ix_USB_PID_MEDION_MD95700], NULL },
> },
> }
> };
> @@ -1637,8 +1664,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_lgh064f_properties = {
> .num_device_descs = 1,
> .devices = {
> { "DViCO FusionHDTV5 USB Gold",
> - { &cxusb_table[1], NULL },
> - { &cxusb_table[2], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_LG064F_COLD], NULL },
> + { &cxusb_table[USB_PID_DVICO_BLUEBIRD_LG064F_WARM], NULL },
Here, you forgot the "ix_" prefix.
> },
> }
> };
> @@ -1693,16 +1720,16 @@ static struct dvb_usb_device_properties cxusb_bluebird_dee1601_properties = {
> .num_device_descs = 3,
> .devices = {
> { "DViCO FusionHDTV DVB-T Dual USB",
> - { &cxusb_table[3], NULL },
> - { &cxusb_table[4], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_COLD], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_1_WARM], NULL },
> },
> { "DigitalNow DVB-T Dual USB",
> - { &cxusb_table[9], NULL },
> - { &cxusb_table[10], NULL },
> + { &cxusb_table[ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_COLD], NULL },
> + { &cxusb_table[ix_USB_PID_DIGITALNOW_BLUEBIRD_DUAL_1_WARM], NULL },
> },
> { "DViCO FusionHDTV DVB-T Dual Digital 2",
> - { &cxusb_table[11], NULL },
> - { &cxusb_table[12], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_COLD], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_2_WARM], NULL },
> },
> }
> };
> @@ -1756,8 +1783,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_lgz201_properties = {
> .num_device_descs = 1,
> .devices = {
> { "DViCO FusionHDTV DVB-T USB (LGZ201)",
> - { &cxusb_table[5], NULL },
> - { &cxusb_table[6], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_LGZ201_COLD], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_LGZ201_WARM], NULL },
> },
> }
> };
> @@ -1812,8 +1839,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_dtt7579_properties = {
> .num_device_descs = 1,
> .devices = {
> { "DViCO FusionHDTV DVB-T USB (TH7579)",
> - { &cxusb_table[7], NULL },
> - { &cxusb_table[8], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_TH7579_COLD], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_TH7579_WARM], NULL },
> },
> }
> };
> @@ -1865,7 +1892,7 @@ static struct dvb_usb_device_properties cxusb_bluebird_dualdig4_properties = {
> .devices = {
> { "DViCO FusionHDTV DVB-T Dual Digital 4",
> { NULL },
> - { &cxusb_table[13], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_4], NULL },
> },
> }
> };
> @@ -1918,7 +1945,7 @@ static struct dvb_usb_device_properties cxusb_bluebird_nano2_properties = {
> .devices = {
> { "DViCO FusionHDTV DVB-T NANO2",
> { NULL },
> - { &cxusb_table[14], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2], NULL },
> },
> }
> };
> @@ -1972,8 +1999,8 @@ static struct dvb_usb_device_properties cxusb_bluebird_nano2_needsfirmware_prope
> .num_device_descs = 1,
> .devices = {
> { "DViCO FusionHDTV DVB-T NANO2 w/o firmware",
> - { &cxusb_table[14], NULL },
> - { &cxusb_table[15], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DVB_T_NANO_2_NFW_WARM], NULL },
> },
> }
> };
> @@ -2017,7 +2044,7 @@ static struct dvb_usb_device_properties cxusb_aver_a868r_properties = {
> .devices = {
> { "AVerMedia AVerTVHD Volar (A868R)",
> { NULL },
> - { &cxusb_table[16], NULL },
> + { &cxusb_table[ix_USB_PID_AVERMEDIA_VOLAR_A868R], NULL },
> },
> }
> };
> @@ -2071,7 +2098,7 @@ struct dvb_usb_device_properties cxusb_bluebird_dualdig4_rev2_properties = {
> .devices = {
> { "DViCO FusionHDTV DVB-T Dual Digital 4 (rev 2)",
> { NULL },
> - { &cxusb_table[17], NULL },
> + { &cxusb_table[ix_USB_PID_DVICO_BLUEBIRD_DUAL_4_REV_2], NULL },
> },
> }
> };
> @@ -2125,7 +2152,7 @@ static struct dvb_usb_device_properties cxusb_d680_dmb_properties = {
> {
> "Conexant DMB-TH Stick",
> { NULL },
> - { &cxusb_table[18], NULL },
> + { &cxusb_table[ix_USB_PID_CONEXANT_D680_DMB], NULL },
> },
> }
> };
> @@ -2179,7 +2206,7 @@ static struct dvb_usb_device_properties cxusb_mygica_d689_properties = {
> {
> "Mygica D689 DMB-TH",
> { NULL },
> - { &cxusb_table[19], NULL },
> + { &cxusb_table[ix_USB_PID_MYGICA_D689], NULL },
> },
> }
> };
> @@ -2232,7 +2259,7 @@ static struct dvb_usb_device_properties cxusb_mygica_t230_properties = {
> {
> "Mygica T230 DVB-T/T2/C",
> { NULL },
> - { &cxusb_table[20], NULL },
> + { &cxusb_table[ix_USB_PID_MYGICA_T230], NULL },
> },
> }
> };
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers
2015-02-16 15:33 [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers David Howells
2015-02-17 11:57 ` Mauro Carvalho Chehab
@ 2015-02-17 13:45 ` David Howells
2015-02-17 13:57 ` Mauro Carvalho Chehab
2015-02-17 13:52 ` David Howells
2 siblings, 1 reply; 5+ messages in thread
From: David Howells @ 2015-02-17 13:45 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: dhowells, mkrufky, linux-media
Mauro Carvalho Chehab <mchehab@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},
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.
> > + _(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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers
2015-02-16 15:33 [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers David Howells
2015-02-17 11:57 ` Mauro Carvalho Chehab
2015-02-17 13:45 ` David Howells
@ 2015-02-17 13:52 ` David Howells
2 siblings, 0 replies; 5+ messages in thread
From: David Howells @ 2015-02-17 13:52 UTC (permalink / raw)
Cc: dhowells, Mauro Carvalho Chehab, mkrufky, linux-media
David Howells <dhowells@redhat.com> wrote:
> That should really be:
>
> [VID_MEDION_MD95700] = {USB_VID_MEDION, USB_PID_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.
In fact, they're both wrong, since there needs to be a USB_DEVICE() wrapper.
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers
2015-02-17 13:45 ` David Howells
@ 2015-02-17 13:57 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2015-02-17 13:57 UTC (permalink / raw)
To: David Howells; +Cc: mkrufky, linux-media
Em Tue, 17 Feb 2015 13:45:50 +0000
David Howells <dhowells@redhat.com> escreveu:
> Mauro Carvalho Chehab <mchehab@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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-17 13:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-16 15:33 [PATCH] cxusb: Use enum to represent table offsets rather than hard-coding numbers David Howells
2015-02-17 11:57 ` Mauro Carvalho Chehab
2015-02-17 13:45 ` David Howells
2015-02-17 13:57 ` Mauro Carvalho Chehab
2015-02-17 13:52 ` David Howells
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.