Re: [PATCH 4.4 38/60] USB: serial: sierra: fix bogus alternate-setting assumption

[Ben Hutchings]

On Thu, 2017-05-11 at 16:13 +0200, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Johan Hovold <johan@xxxxxxxxxx>
> 
> commit 16620b483eaf7750413bae472f4363b6b959fcaa upstream.
> 
> Interface numbers do not change when enabling alternate settings as
> comment and code in this driver suggested.
> 
> Remove the confusing comment and redundant retrieval of the interface
> number in probe, while simplifying and renaming the interface-number
> helper.

This looks like cleanup with no functional impact, so unless it's needed
as preparation for something else I think it should be dropped.

Ben.

> Fixes: 4db2299da213 ("sierra: driver interface blacklisting")
> Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> 
> ---
>  drivers/usb/serial/sierra.c |   28 +++++-----------------------
>  1 file changed, 5 insertions(+), 23 deletions(-)
> 
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -137,24 +137,9 @@ static int is_himemory(const u8 ifnum,
>  	return 0;
>  }
>  
> -static int sierra_calc_interface(struct usb_serial *serial)
> +static u8 sierra_interface_num(struct usb_serial *serial)
>  {
> -	int interface;
> -	struct usb_interface *p_interface;
> -	struct usb_host_interface *p_host_interface;
> -
> -	/* Get the interface structure pointer from the serial struct */
> -	p_interface = serial->interface;
> -
> -	/* Get a pointer to the host interface structure */
> -	p_host_interface = p_interface->cur_altsetting;
> -
> -	/* read the interface descriptor for this active altsetting
> -	 * to find out the interface number we are on
> -	*/
> -	interface = p_host_interface->desc.bInterfaceNumber;
> -
> -	return interface;
> +	return serial->interface->cur_altsetting->desc.bInterfaceNumber;
>  }
>  
>  static int sierra_probe(struct usb_serial *serial,
> @@ -165,7 +150,7 @@ static int sierra_probe(struct usb_seria
>  	u8 ifnum;
>  
>  	udev = serial->dev;
> -	ifnum = sierra_calc_interface(serial);
> +	ifnum = sierra_interface_num(serial);
>  
>  	/*
>  	 * If this interface supports more than 1 alternate
> @@ -178,9 +163,6 @@ static int sierra_probe(struct usb_seria
>  		usb_set_interface(udev, ifnum, 1);
>  	}
>  
> -	/* ifnum could have changed - by calling usb_set_interface */
> -	ifnum = sierra_calc_interface(serial);
> -
>  	if (is_blacklisted(ifnum,
>  				(struct sierra_iface_info *)id->driver_info)) {
>  		dev_dbg(&serial->dev->dev,
> @@ -342,7 +324,7 @@ static int sierra_send_setup(struct usb_
>  
>  	/* If composite device then properly report interface */
>  	if (serial->num_ports == 1) {
> -		interface = sierra_calc_interface(serial);
> +		interface = sierra_interface_num(serial);
>  		/* Control message is sent only to interfaces with
>  		 * interrupt_in endpoints
>  		 */
> @@ -916,7 +898,7 @@ static int sierra_port_probe(struct usb_
>  	/* Determine actual memory requirements */
>  	if (serial->num_ports == 1) {
>  		/* Get interface number for composite device */
> -		ifnum = sierra_calc_interface(serial);
> +		ifnum = sierra_interface_num(serial);
>  		himemoryp = &typeB_interface_list;
>  	} else {
>  		/* This is really the usb-serial port number of the interface
> 
> 
> 

-- 
Ben Hutchings
Software Developer, Codethink Ltd.