| Message ID | 20181112071605.7014-1-jackychou@asix.com.tw (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: serial: mos7840: Add a product ID for the new product | expand |
On Mon, Nov 12, 2018 at 03:16:05PM +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. > > Signed-off-by: JackyChou <jackychou@asix.com.tw> > --- > drivers/usb/serial/mos7840.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c > index b42bad85097a..7667b22fa2f7 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,7 +300,7 @@ 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 == 4 || port->serial->num_ports == 3) { Please use num_ports > 2 here. > val |= ((__u16)port->port_number + 1) << 8; > } else { > if (port->port_number == 0) { > @@ -332,7 +334,7 @@ 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 == 4 || port->serial->num_ports == 3) { Same here. > Wval = ((__u16)port->port_number + 1) << 8; > } else { > if (port->port_number == 0) { > @@ -2071,9 +2073,12 @@ 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; What about 7843 devices that use a different PID? Is there a reliable way to detect the type without relying on PID? Otherwise, there's no point in verifying GPIO0 for these ones. > + else > + device_type = MOSCHIP_DEVICE_ID_7840; > + } else if (mos7810_check(serial)) Nit: if you add braces to one of the branches you need to add it to all of them. > device_type = MOSCHIP_DEVICE_ID_7810; > else > device_type = MOSCHIP_DEVICE_ID_7820; > @@ -2091,7 +2096,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 +2154,8 @@ 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 == 4) || (serial->num_ports == 3))) { > mos7840_port->SpRegOffset = 0x8; > mos7840_port->ControlRegOffset = 0x9; > mos7840_port->DcrRegOffset = 0x16; > @@ -2154,7 +2163,8 @@ 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 == 4) || (serial->num_ports == 3))) { > mos7840_port->SpRegOffset = 0xa; > mos7840_port->ControlRegOffset = 0xb; > mos7840_port->DcrRegOffset = 0x19; This is getting rather ugly. Can't you clean up the current code so that it covers your device without the need of such changes first? From the looks of it, the only reason for the above is to map the offsets for port 2 in the 2-port case to the offsets of port 3. It would be more straight-forward to handle that particular case separately. This also relates to the set/get_uart_reg functions above, which also should handle the 2-port case specifically instead. Thanks, Johan
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c index b42bad85097a..7667b22fa2f7 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,7 +300,7 @@ 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 == 4 || port->serial->num_ports == 3) { val |= ((__u16)port->port_number + 1) << 8; } else { if (port->port_number == 0) { @@ -332,7 +334,7 @@ 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 == 4 || port->serial->num_ports == 3) { Wval = ((__u16)port->port_number + 1) << 8; } else { if (port->port_number == 0) { @@ -2071,9 +2073,12 @@ 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; + else + device_type = MOSCHIP_DEVICE_ID_7840; + } else if (mos7810_check(serial)) device_type = MOSCHIP_DEVICE_ID_7810; else device_type = MOSCHIP_DEVICE_ID_7820; @@ -2091,7 +2096,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 +2154,8 @@ 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 == 4) || (serial->num_ports == 3))) { mos7840_port->SpRegOffset = 0x8; mos7840_port->ControlRegOffset = 0x9; mos7840_port->DcrRegOffset = 0x16; @@ -2154,7 +2163,8 @@ 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 == 4) || (serial->num_ports == 3))) { mos7840_port->SpRegOffset = 0xa; mos7840_port->ControlRegOffset = 0xb; mos7840_port->DcrRegOffset = 0x19;