lkml.org 
[lkml]   [2019]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V1 3/6] USB: serial: f81232: Add generator for F81534A
From
Date
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@gmail.com>
>
> 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

\
 
 \ /
  Last update: 2019-09-02 05:00    [W:0.055 / U:0.536 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site