On Tue, May 12, 2009 at 01:13:10PM -0700, Elina Pasheva wrote:
> Subject: [PATCH 004/004] USB: serial: sierra driver interface blacklisting
> From: Elina Pasheva <epasheva@xxxxxxxxxxxxxxxxxx>
>
> The following is summary of changes we have made to sierra.c driver in
> [PATCH 004/004] dealing with interface blacklisting support:
> - Modified sierra_probe() and sierra_calc_interface() to handle blacklisted
> interfaces accordingly
> - Added data list structure and function to support blacklisting
But you never use this structure. Why add it?
> - Improved comments in id_table
> Signed-off-by: Elina Pasheva <epasheva@xxxxxxxxxxxxxxxxxx>
> ---
>
> drivers/usb/serial/sierra.c | 45 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 3 deletions(-)
>
> --- a/drivers/usb/serial/sierra.c 2009-05-01 13:17:14.000000000 -0700
> +++ b/drivers/usb/serial/sierra.c 2009-05-01 17:30:05.000000000 -0700
> @@ -47,6 +47,12 @@
> static int debug;
> static int nmea;
>
> +/* list of interface numbers - used for constructing interface blacklists */
> +struct list {
> + const u32 listlen; /* number of interface numbers on list */
> + const u8 *list; /* pointer to the array holding the numbers */
> +};
The kernel has built in list functions, I would recommend using them
instead of creating something so generic as "struct list" in your
driver.
And how is this interface list used to create a blacklist? I see you do
create one:
> +static const u8 direct_ip_non_serial_ifaces[] = { 7, 8, 9 };
> +static const struct list direct_ip_interface_blacklist = {
> + .listlen = ARRAY_SIZE(direct_ip_non_serial_ifaces),
> + .list = direct_ip_non_serial_ifaces,
> +};
But then you never refer to it in the code.
Am I missing something here?
confused,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
