| Message ID | 002201d47d76$c5a45080$50ecf180$@asix.com.tw (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: serial: mos7840: Add a product ID for the new product | expand |
On Fri, Nov 16, 2018 at 02:36:56PM +0800, JackyChou wrote: > From: JackyChou <jackychou@asix.com.tw> > > Add a new PID 0x7843 to the driver. > Let the new products be able to set up 3 serial ports with the driver. > > Because the development of new product is based on 4 serial ports, > but some users only need 3 serial ports. There is no way to set it from > the hardware, so let the driver set 3 serial ports with PID 0x7843. > > Signed-off-by: JackyChou <jackychou@asix.com.tw> > --- Thanks for the update. Always include a short changelog here so we know what has changed since earlier versions. Also include the patch revision in the Subject (i.e. "[PATCH v2] USB: ..."). > drivers/usb/serial/mos7840.c | 45 ++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c > index b42bad85097a..57ef25a2f7bb 100644 > --- a/drivers/usb/serial/mos7840.c > +++ b/drivers/usb/serial/mos7840.c > @@ -94,6 +94,7 @@ > /* The native mos7840/7820 component */ > #define USB_VENDOR_ID_MOSCHIP 0x9710 > #define MOSCHIP_DEVICE_ID_7840 0x7840 > +#define MOSCHIP_DEVICE_ID_7843 0x7843 > #define MOSCHIP_DEVICE_ID_7820 0x7820 > #define MOSCHIP_DEVICE_ID_7810 0x7810 > /* The native component can have its vendor/device id's overridden > @@ -176,6 +177,7 @@ enum mos7840_flag { > > static const struct usb_device_id id_table[] = { > {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)}, > + {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7843)}, > {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)}, > {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)}, > {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)}, > @@ -298,15 +300,10 @@ static int mos7840_set_uart_reg(struct usb_serial_port > *port, __u16 reg, > val = val & 0x00ff; > /* For the UART control registers, the application number need > to be Or'ed */ > - if (port->serial->num_ports == 4) { > + if (port->serial->num_ports == 2 && port->port_number != 0) > + val |= ((__u16)port->port_number + 2) << 8; > + else > val |= ((__u16)port->port_number + 1) << 8; > - } else { > - if (port->port_number == 0) { > - val |= ((__u16)port->port_number + 1) << 8; > - } else { > - val |= ((__u16)port->port_number + 2) << 8; > - } > - } This looks good, but please to this in a separate preparatory patch as this is an independent change from adding support for you new device. > dev_dbg(&port->dev, "%s application number is %x\n", __func__, val); > return usb_control_msg(dev, usb_sndctrlpipe(dev, 0), MCS_WRREQ, > MCS_WR_RTYPE, val, reg, NULL, 0, > @@ -332,15 +329,10 @@ static int mos7840_get_uart_reg(struct usb_serial_port > *port, __u16 reg, > return -ENOMEM; > > /* Wval is same as application number */ > - if (port->serial->num_ports == 4) { > + if (port->serial->num_ports == 2 && port->port_number != 0) > + Wval = ((__u16)port->port_number + 2) << 8; > + else > Wval = ((__u16)port->port_number + 1) << 8; > - } else { > - if (port->port_number == 0) { > - Wval = ((__u16)port->port_number + 1) << 8; > - } else { > - Wval = ((__u16)port->port_number + 2) << 8; > - } > - } Same here. > dev_dbg(&port->dev, "%s application number is %x\n", __func__, > Wval); > ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ, > MCS_RD_RTYPE, Wval, reg, buf, > VENDOR_READ_LENGTH, > @@ -2071,12 +2063,16 @@ static int mos7840_probe(struct usb_serial *serial, > VENDOR_READ_LENGTH, MOS_WDR_TIMEOUT); > > /* For a MCS7840 device GPIO0 must be set to 1 */ > - if (buf[0] & 0x01) > - device_type = MOSCHIP_DEVICE_ID_7840; > - else if (mos7810_check(serial)) > + if (buf[0] & 0x01) { > + if (product == MOSCHIP_DEVICE_ID_7843) > + device_type = MOSCHIP_DEVICE_ID_7843; And as I mentioned in my previous comments, if you're going to match in PID, there's no need to check GPIO0. Just add code to handle 7843 to the start of the function. > + else > + device_type = MOSCHIP_DEVICE_ID_7840; > + } else if (mos7810_check(serial)) { > device_type = MOSCHIP_DEVICE_ID_7810; > - else > + } else { > device_type = MOSCHIP_DEVICE_ID_7820; > + } > > kfree(buf); > out: > @@ -2091,7 +2087,10 @@ static int mos7840_calc_num_ports(struct usb_serial > *serial, > int device_type = (unsigned long)usb_get_serial_data(serial); > int num_ports; > > - num_ports = (device_type >> 4) & 0x000F; > + if (device_type == MOSCHIP_DEVICE_ID_7843) > + num_ports = 3; > + else > + num_ports = (device_type >> 4) & 0x000F; > > /* > * num_ports is currently never zero as device_type is one of > @@ -2146,7 +2145,7 @@ static int mos7840_port_probe(struct usb_serial_port > *port) > mos7840_port->SpRegOffset = 0x0; > mos7840_port->ControlRegOffset = 0x1; > mos7840_port->DcrRegOffset = 0x4; > - } else if ((mos7840_port->port_num == 2) && (serial->num_ports == > 4)) { > + } else if ((mos7840_port->port_num == 2) && (serial->num_ports > 2)) > { > mos7840_port->SpRegOffset = 0x8; > mos7840_port->ControlRegOffset = 0x9; > mos7840_port->DcrRegOffset = 0x16; > @@ -2154,7 +2153,7 @@ static int mos7840_port_probe(struct usb_serial_port > *port) > mos7840_port->SpRegOffset = 0xa; > mos7840_port->ControlRegOffset = 0xb; > mos7840_port->DcrRegOffset = 0x19; > - } else if ((mos7840_port->port_num == 3) && (serial->num_ports == > 4)) { > + } else if ((mos7840_port->port_num == 3) && (serial->num_ports > 2)) > { > mos7840_port->SpRegOffset = 0xa; > mos7840_port->ControlRegOffset = 0xb; > mos7840_port->DcrRegOffset = 0x19; Can't this be handled similarly to set_uart_reg() above? Looks like you can calculate the offsets (and remove the if-else clause) and only make sure to map port 2 in the two-port case to physical port 3. This would also go in the preparatory first patch of a two-part series. Thanks, Johan
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c index b42bad85097a..57ef25a2f7bb 100644 --- a/drivers/usb/serial/mos7840.c +++ b/drivers/usb/serial/mos7840.c @@ -94,6 +94,7 @@ /* The native mos7840/7820 component */ #define USB_VENDOR_ID_MOSCHIP 0x9710 #define MOSCHIP_DEVICE_ID_7840 0x7840 +#define MOSCHIP_DEVICE_ID_7843 0x7843 #define MOSCHIP_DEVICE_ID_7820 0x7820 #define MOSCHIP_DEVICE_ID_7810 0x7810 /* The native component can have its vendor/device id's overridden @@ -176,6 +177,7 @@ enum mos7840_flag { static const struct usb_device_id id_table[] = { {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7840)}, + {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7843)}, {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7820)}, {USB_DEVICE(USB_VENDOR_ID_MOSCHIP, MOSCHIP_DEVICE_ID_7810)}, {USB_DEVICE(USB_VENDOR_ID_BANDB, BANDB_DEVICE_ID_USO9ML2_2)}, @@ -298,15 +300,10 @@ static int mos7840_set_uart_reg(struct usb_serial_port *port, __u16 reg, val = val & 0x00ff; /* For the UART control registers, the application number need to be Or'ed */ - if (port->serial->num_ports == 4) { + if (port->serial->num_ports == 2 && port->port_number != 0) + val |= ((__u16)port->port_number + 2) << 8; + else val |= ((__u16)port->port_number + 1) << 8; - } else { - if (port->port_number == 0) { - val |= ((__u16)port->port_number + 1) << 8; - } else { - val |= ((__u16)port->port_number + 2) << 8; - } - } dev_dbg(&port->dev, "%s application number is %x\n", __func__, val); return usb_control_msg(dev, usb_sndctrlpipe(dev, 0), MCS_WRREQ,