Re: [PATCH V1 3/6] USB: serial: f81232: Add generator for F81534A

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johan,

Johan Hovold 於 2019/8/28 下午 11:02 寫道:
On Thu, Jun 06, 2019 at 10:54:13AM +0800, Ji-Ze Hong (Peter Hong) wrote:
The Fintek F81534A series is contains 1 HUB / 1 GPIO device / n UARTs,
but the UART is default disable and need enabled by GPIO device(2c42/16F8).
When F81534A plug to host, we can only see 1 HUB & 1 GPIO device, add
GPIO device USB interface to device_list and trigger generate worker,
f81534a_generate_worker to run f81534a_ctrl_generate_ports().

The operation in f81534a_ctrl_generate_ports() as following:
	1: Write 0x8fff to F81534A_CMD_ENABLE_PORT register for enable all
	   UART device.

	2: Read port existence & current status from F81534A_CMD_PORT_STATUS
	   register. the higher 16bit will indicate the UART existence. If the
	   UART is existence, we'll check it GPIO mode as long as not default
	   value (default is all input mode).

	3: 1 GPIO device will check with max 15s and check next GPIO device when
	   timeout. (F81534A_CTRL_RETRY * F81534A_CTRL_TIMER)

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@xxxxxxxxx>

This is all looks crazy... Please better describe how the device works,
and you want to implement support.

I'll try to refactor more simply for first add into kernel.

---
  drivers/usb/serial/f81232.c | 356 +++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 355 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 75dfc0b9ef30..e9470fb0d691 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -41,6 +41,12 @@ static const struct usb_device_id id_table[] = {
  };
  MODULE_DEVICE_TABLE(usb, id_table);
+static const struct usb_device_id f81534a_ctrl_id_table[] = {
+	{ USB_DEVICE(0x2c42, 0x16f8) },		/* Global control device */
+	{ }					/* Terminating entry */
+};
+MODULE_DEVICE_TABLE(usb, f81534a_ctrl_id_table);

You can only have one MODULE_DEVICE_TABLE()...

I had a question about this. In this file, we'll need support 3 sets of
id f81232(1)/f81534a(9)/f81534a_ctrl(1). So I will refactor the code
about id section to the below due to the id table will use more than
once:

=======================================================================
#define F81232_ID		\
	{ USB_DEVICE(0x1934, 0x0706) }	/* 1 port UART device */

#define F81534A_SERIES_ID	\
	{ USB_DEVICE(0x2c42, 0x1602) },	/* In-Box 2 port UART device */	\
	{ USB_DEVICE(0x2c42, 0x1604) },	/* In-Box 4 port UART device */	\
	{ USB_DEVICE(0x2c42, 0x1605) },	/* In-Box 8 port UART device */	\
	{ USB_DEVICE(0x2c42, 0x1606) },	/* In-Box 12 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1608) },	/* Non-Flash type */ \
	{ USB_DEVICE(0x2c42, 0x1632) },	/* 2 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1634) },	/* 4 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1635) },	/* 8 port UART device */ \
	{ USB_DEVICE(0x2c42, 0x1636) }	/* 12 port UART device */

#define F81534A_CTRL_ID		\
	{ USB_DEVICE(0x2c42, 0x16f8) }	/* Global control device */

static const struct usb_device_id id_table[] = {
	F81232_ID,
	{ }					/* Terminating entry */
};

static const struct usb_device_id f81534a_id_table[] = {
	F81534A_SERIES_ID,
	{ }					/* Terminating entry */
};

static const struct usb_device_id f81534a_ctrl_id_table[] = {
	F81534A_CTRL_ID,
	{ }					/* Terminating entry */
};

static const struct usb_device_id all_serial_id_table[] = {
	F81232_ID,
	F81534A_SERIES_ID,
	{ }					/* Terminating entry */
};
MODULE_DEVICE_TABLE(usb, all_serial_id_table);
=======================================================================

but the checkpatch.pl give me the warning below:
ERROR: Macros with complex values should be enclosed in parentheses
#42: FILE: f81232.c:28:
+#define F81534A_SERIES_ID      \
+       { USB_DEVICE(0x2c42, 0x1602) }, /* In-Box 2 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1604) }, /* In-Box 4 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1605) }, /* In-Box 8 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1606) }, /* In-Box 12 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1608) }, /* Non-Flash type */ \
+       { USB_DEVICE(0x2c42, 0x1632) }, /* 2 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1634) }, /* 4 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1635) }, /* 8 port UART device */ \
+       { USB_DEVICE(0x2c42, 0x1636) }  /* 12 port UART device */

Is any suggestion ?

Thanks
--
With Best Regards,
Peter Hong



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux