On Wed, Mar 24, 2021 at 08:41:07AM +0100, Mauro Carvalho Chehab wrote: > XR21V1412 and XR21V1414 have exactly the same interface, but > they support multiple 2 and 4 ports, respectively. > > On such devices, the "CDC Union" field shows how they're > grouped, as can be seen on those lsusb -v outputs: > > https://linux-usb.vger.kernel.narkive.com/YaTYwHkM/usb-uart-device-from-exar-co-not-working-with-cdc-acm-but-usbserial > https://jquery.developreference.com/article/22043997/udev+rule+with+bInterfaceNumber+doesn't+work+%5bclosed%5d > > So, for instance, on XR21V1414, (0x04e2:0x1414), the 3rd > port is: > > CDC Union: > bMasterInterface 4 > bSlaveInterface 5 > > CDC Call Management: > bmCapabilities 0x01 > call management > bDataInterface 5 > > In other words, the control interface is an even number, > and the data interface is the next odd number. > > The logic to get the proper register for an specific channel > came from this patch: > > https://lore.kernel.org/linux-usb/20180404070634.nhspvmxcjwfgjkcv@advantechmxl-desktop > > Add support for them. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > --- > drivers/usb/serial/xr_serial.c | 30 ++++++++++++++++++++++++++++-- > 1 file changed, 28 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c > index 518c4725431a..f161394d9b2d 100644 > --- a/drivers/usb/serial/xr_serial.c > +++ b/drivers/usb/serial/xr_serial.c > @@ -145,6 +145,7 @@ static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = { > > struct xr_port_private { > enum xr_model model; > + unsigned int channel; > }; > > static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val) > @@ -153,6 +154,14 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val) > struct usb_serial *serial = port->serial; > int ret; > > + switch (port_priv->model) { > + case XR21V141X: > + if (port_priv->channel) > + reg |= (port_priv->channel - 1) << 8; reg is u8 so you're simply discarding the channel index here and effectively only the first port will work as intended after this patch. > + break; > + default: > + return -EINVAL; > + }; > ret = usb_control_msg(serial->dev, > usb_sndctrlpipe(serial->dev, 0), > xr_hal_table[port_priv->model][REQ_SET], > static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id) > { > + struct usb_interface *intf = serial->interface; > + struct usb_endpoint_descriptor *data_ep; > struct xr_port_private *port_priv; > + int ifnum; > > - /* Don't bind to control interface */ > - if (serial->interface->cur_altsetting->desc.bInterfaceNumber == 0) > + /* Attach only data interfaces */ > + ifnum = intf->cur_altsetting->desc.bInterfaceNumber; > + if (!(ifnum % 2)) > return -ENODEV; > > port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL); > if (!port_priv) > return -ENOMEM; > > + data_ep = &intf->cur_altsetting->endpoint[0].desc; > + > port_priv->model = id->driver_info; > + port_priv->channel = data_ep->bEndpointAddress; This creative but it seems cleaner to simply use the interface number of the control interface divided by two. That gives you a zero-based index which doesn't require the "channel--" added at various places by the rest of the series. Johan
