[PATCH] USB: serial: sierra: clean up special-interface handling

[PATCH] USB: serial: sierra: clean up special-interface handling

[Johan Hovold]

Clean up the handling of special interfaces that either should be
ignored or that need a larger number of URBs.

Commit 66f092ed3b94 ("USB: serial: sierra: unify quirk handling logic")
replaced the previous is_blacklisted() and is_highmemory() helpers with
a single is_quirk() helper which made it even harder to understand what
the interface lists were used for.

Rename the interface-list struct, its members and the interface-lookup
helper and restructure the code somewhat in order to make it more
self-explanatory.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/sierra.c | 58 ++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index a862aa788a19..57fc3c31712e 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -45,10 +45,9 @@
 
 static bool nmea;
 
-/* Used in interface quirks */
-struct sierra_iface_quirk {
-	const u32 infolen;	/* number of interface numbers on the list */
-	const u8  *ifaceinfo;	/* pointer to the array holding the numbers */
+struct sierra_iface_list {
+	const u8 *nums;		/* array of interface numbers */
+	size_t count;		/* number of elements in array */
 };
 
 struct sierra_intf_private {
@@ -101,20 +100,19 @@ static int sierra_calc_num_ports(struct usb_serial *serial,
 	return num_ports;
 }
 
-static int is_quirk(const u8 ifnum, const struct sierra_iface_quirk *quirk)
+static bool is_listed(const u8 ifnum, const struct sierra_iface_list *list)
 {
-	const u8  *info;
 	int i;
 
-	if (quirk) {
-		info = quirk->ifaceinfo;
+	if (!list)
+		return false;
 
-		for (i = 0; i < quirk->infolen; i++) {
-			if (info[i] == ifnum)
-				return 1;
-		}
+	for (i = 0; i < list->count; i++) {
+		if (list->nums[i] == ifnum)
+			return true;
 	}
-	return 0;
+
+	return false;
 }
 
 static u8 sierra_interface_num(struct usb_serial *serial)
@@ -125,6 +123,7 @@ static u8 sierra_interface_num(struct usb_serial *serial)
 static int sierra_probe(struct usb_serial *serial,
 			const struct usb_device_id *id)
 {
+	const struct sierra_iface_list *ignore_list;
 	int result = 0;
 	struct usb_device *udev;
 	u8 ifnum;
@@ -143,9 +142,10 @@ static int sierra_probe(struct usb_serial *serial,
 		usb_set_interface(udev, ifnum, 1);
 	}
 
-	if (is_quirk(ifnum, (struct sierra_iface_quirk *)id->driver_info)) {
-		dev_dbg(&serial->dev->dev,
-			"Ignoring interface #%d\n", ifnum);
+	ignore_list = (const struct sierra_iface_list *)id->driver_info;
+
+	if (is_listed(ifnum, ignore_list)) {
+		dev_dbg(&serial->dev->dev, "Ignoring interface #%d\n", ifnum);
 		return -ENODEV;
 	}
 
@@ -154,22 +154,22 @@ static int sierra_probe(struct usb_serial *serial,
 
 /* interfaces with higher memory requirements */
 static const u8 hi_memory_typeA_ifaces[] = { 0, 2 };
-static const struct sierra_iface_quirk typeA_interface_list = {
-	.infolen = ARRAY_SIZE(hi_memory_typeA_ifaces),
-	.ifaceinfo = hi_memory_typeA_ifaces,
+static const struct sierra_iface_list typeA_interface_list = {
+	.nums	= hi_memory_typeA_ifaces,
+	.count	= ARRAY_SIZE(hi_memory_typeA_ifaces),
 };
 
 static const u8 hi_memory_typeB_ifaces[] = { 3, 4, 5, 6 };
-static const struct sierra_iface_quirk typeB_interface_list = {
-	.infolen = ARRAY_SIZE(hi_memory_typeB_ifaces),
-	.ifaceinfo = hi_memory_typeB_ifaces,
+static const struct sierra_iface_list typeB_interface_list = {
+	.nums	= hi_memory_typeB_ifaces,
+	.count	= ARRAY_SIZE(hi_memory_typeB_ifaces),
 };
 
 /* 'ignorelist' of interfaces not served by this driver */
 static const u8 direct_ip_non_serial_ifaces[] = { 7, 8, 9, 10, 11, 19, 20 };
-static const struct sierra_iface_quirk direct_ip_interface_ignore = {
-	.infolen = ARRAY_SIZE(direct_ip_non_serial_ifaces),
-	.ifaceinfo = direct_ip_non_serial_ifaces,
+static const struct sierra_iface_list direct_ip_interface_ignore = {
+	.nums	= direct_ip_non_serial_ifaces,
+	.count	= ARRAY_SIZE(direct_ip_non_serial_ifaces),
 };
 
 static const struct usb_device_id id_table[] = {
@@ -859,7 +859,7 @@ static int sierra_port_probe(struct usb_serial_port *port)
 {
 	struct usb_serial *serial = port->serial;
 	struct sierra_port_private *portdata;
-	const struct sierra_iface_quirk *himemoryp;
+	const struct sierra_iface_list *himemory_list;
 	u8 ifnum;
 
 	portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
@@ -878,16 +878,16 @@ static int sierra_port_probe(struct usb_serial_port *port)
 	if (serial->num_ports == 1) {
 		/* Get interface number for composite device */
 		ifnum = sierra_interface_num(serial);
-		himemoryp = &typeB_interface_list;
+		himemory_list = &typeB_interface_list;
 	} else {
 		/* This is really the usb-serial port number of the interface
 		 * rather than the interface number.
 		 */
 		ifnum = port->port_number;
-		himemoryp = &typeA_interface_list;
+		himemory_list = &typeA_interface_list;
 	}
 
-	if (is_quirk(ifnum, himemoryp)) {
+	if (is_listed(ifnum, himemory_list)) {
 		portdata->num_out_urbs = N_OUT_URB_HM;
 		portdata->num_in_urbs  = N_IN_URB_HM;
 	}
-- 
2.26.2

Re: [PATCH] USB: serial: sierra: clean up special-interface handling

[Greg Kroah-Hartman]

On Mon, Jul 13, 2020 at 05:39:36PM +0200, Johan Hovold wrote:
> Clean up the handling of special interfaces that either should be
> ignored or that need a larger number of URBs.
> 
> Commit 66f092ed3b94 ("USB: serial: sierra: unify quirk handling logic")
> replaced the previous is_blacklisted() and is_highmemory() helpers with
> a single is_quirk() helper which made it even harder to understand what
> the interface lists were used for.
> 
> Rename the interface-list struct, its members and the interface-lookup
> helper and restructure the code somewhat in order to make it more
> self-explanatory.

Much better, that was messy, thanks for cleaning this up.

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH] USB: serial: sierra: clean up special-interface handling

[Johan Hovold]

On Mon, Jul 13, 2020 at 05:51:27PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jul 13, 2020 at 05:39:36PM +0200, Johan Hovold wrote:
> > Clean up the handling of special interfaces that either should be
> > ignored or that need a larger number of URBs.
> > 
> > Commit 66f092ed3b94 ("USB: serial: sierra: unify quirk handling logic")
> > replaced the previous is_blacklisted() and is_highmemory() helpers with
> > a single is_quirk() helper which made it even harder to understand what
> > the interface lists were used for.
> > 
> > Rename the interface-list struct, its members and the interface-lookup
> > helper and restructure the code somewhat in order to make it more
> > self-explanatory.
> 
> Much better, that was messy, thanks for cleaning this up.
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Now applied.

Johan