[PATCHv3] USB: serial: mos7840: Add a product ID for the new product

Commit Message

Jacky Chou Nov. 22, 2018, 8 a.m. UTC

From: JackyChou <[e-mail]>

For now, pause to add PID 0x7843 in the driver.

Simplify the processes of some functions.
Such modifications will not affect the old devices and will make
the addition of new product (0x7843) more flexible in the future.

Signed-off-by: JackyChou <[e-mail]>
---
 drivers/usb/serial/mos7840.c | 48 ++++++++++++------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

 			       MCS_WR_RTYPE, val, reg, NULL, 0,
@@ -332,15 +327,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;
-		}
-	}
 	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,
@@ -2146,22 +2136,16 @@ 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)) {
-		mos7840_port->SpRegOffset = 0x8;
-		mos7840_port->ControlRegOffset = 0x9;
-		mos7840_port->DcrRegOffset = 0x16;
-	} else if ((mos7840_port->port_num == 2) && (serial->num_ports ==
2)) {
-		mos7840_port->SpRegOffset = 0xa;
-		mos7840_port->ControlRegOffset = 0xb;
-		mos7840_port->DcrRegOffset = 0x19;
-	} else if ((mos7840_port->port_num == 3) && (serial->num_ports ==
4)) {
-		mos7840_port->SpRegOffset = 0xa;
-		mos7840_port->ControlRegOffset = 0xb;
-		mos7840_port->DcrRegOffset = 0x19;
-	} else if ((mos7840_port->port_num == 4) && (serial->num_ports ==
4)) {
-		mos7840_port->SpRegOffset = 0xc;
-		mos7840_port->ControlRegOffset = 0xd;
-		mos7840_port->DcrRegOffset = 0x1c;
+	} else {
+		u8 port_offset;
+
+		if ((mos7840_port->port_num == 2) && (serial->num_ports ==
2))
+			port_offset = 1;
+		else
+			port_offset = mos7840_port->port_num - 2;
+		mos7840_port->SpRegOffset = 0x8 + (2 * port_offset);
+		mos7840_port->ControlRegOffset = 0x9 + (2 * port_offset);
+		mos7840_port->DcrRegOffset = 0x16 + (3 * port_offset);
 	}
 	mos7840_dump_serial_port(port, mos7840_port);
 	mos7840_set_port_private(port, mos7840_port);

Comments

Johan Hovold Nov. 27, 2018, 3:26 p.m. UTC | #1

On Thu, Nov 22, 2018 at 04:00:17PM +0800, JackyChou wrote:
> 
> From: JackyChou <[e-mail]>

Thanks for the update. This patch appears to have been corrupted (tabs
replaced with spaces) so it cannot be applied. Try sending the patch to
yourself first and make sure you can apply it with git am (and/or run
checkpatch on the result).

> For now, pause to add PID 0x7843 in the driver.

This doesn't belong in the commit message. But there's no reason not to
include the patch adding the PID in the same series. Just send a
two-part series with this patch as the first patch, and the PID one as
the second patch (depending on the first).

Take a look at git-format-patch and git-send-email which can simply the
process.

Also make sure each commit message is self-contained, and that the patch
summary (Subject) reflects what each patch does.
 
> Simplify the processes of some functions.

Please be a bit more specific.

> Such modifications will not affect the old devices and will make
> the addition of new product (0x7843) more flexible in the future.

This can still stay here.

> Signed-off-by: JackyChou <[e-mail]>
> ---
>  drivers/usb/serial/mos7840.c | 48 ++++++++++++------------------------
>  1 file changed, 16 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index b42bad85097a..0ca945dce377 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -298,15 +298,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;
> -		}
> -	}

Perhaps this would be more readable if you used a port offset here as
well, and only make sure to map the second port in the two-port case to
port offset three.

Thanks,
Johan

Patch

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index b42bad85097a..0ca945dce377 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -298,15 +298,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,