diff mbox series

USB: serial: mos7840: Add a product ID for the new product

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

Commit Message

Jacky Chou Nov. 16, 2018, 6:36 a.m. UTC
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>
---
 drivers/usb/serial/mos7840.c | 45 ++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

 			       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;
-		}
-	}
 	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;
+		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;
--
2.17.1

Comments

Johan Hovold Nov. 16, 2018, 9:55 a.m. UTC | #1
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 mbox series

Patch

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,