lkml.org 
[lkml]   [2015]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver
From
Date
Hi

Andy Shevchenko 於 2015/11/3 下午 05:45 寫道:
> On Tue, Nov 3, 2015 at 5:51 AM, Peter Hung <hpeter@gmail.com> wrote:
>> + * Please reference https://bitbucket.org/hpeter/fintek-general/src/
>> + * with f81534/tools to get set_gpio.c & set_mode.c. Please use it
>> + * carefully.
>
> Would it be good to have this under Documentation ?

I had some documents in source code. The link is the demo program to
implement switch UART/PIN funcion. I'll try to add for document with the
repository.

>> +static void f81534_dtr_rts(struct usb_serial_port *port, int on);
>> +
>> +static int f81534_set_port_mode(struct usb_serial_port *port,
>> + enum uart_mode eMode);
>> +
>> +static int f81534_save_configure_data(struct usb_serial_port *port);
>> +
>> +static int f81534_switch_gpio_mode(struct usb_serial_port *port, u8 mode);
>> +
>> +static void f81534_wakeup_all_port(struct usb_serial *serial);
>> +
>> +static void f81534_compare_msr(struct usb_serial_port *port, u8 *msr,
>> + bool is_port_open);
>> +
>> +static int f81534_prepare_write_buffer(struct usb_serial_port *port,
>> + void *dest, size_t size);
>> +
>> +static int f81534_submit_writer(struct usb_serial_port *port, gfp_t mem_flags);
>
> Would it be possible to reshuffle code to reduce amount of forward declarations?
>

I'll try to reduce it.

>
>> + if ((size <= F81534_MAX_DATA_BLOCK) &&
>> + (read_size == (count + 1))) {
>
> No need to have internal parens.
>

In my opinion, It makes more readable. Should I remove it?


>> + if (baudrate <= 115200) {
>> + value = 0x01; /* 1.846m fixed */
>> + divisor = f81534_calc_baud_divisor(baudrate, 115200, NULL);
>> + port_priv->current_baud_base = 115200;
>> + } else {
>> + for (count = 0; count < ARRAY_SIZE(baudrate_table) ; ++count) {
>> + baud_base = baudrate_table[count];
>> + divisor = f81534_calc_baud_divisor(baudrate, baud_base,
>> + &rem);
>> + if (!rem) {
>> + dev_dbg(&port->dev, "%s: found clockbase %d\n",
>> + __func__,
>> + baudrate_table[count]);
>> + value = clock_table[count];
>> + port_priv->current_baud_base = baud_base;
>> + break;
>> + }
>> + }
>
> Can you calculate baud rates more precisely? Any link to datasheet
> where it's described?
>

I'll try to comment it within source code.

>> +static int f81534_ioctl_get_rs485(struct usb_serial_port *port,
>> + struct serial_rs485 __user *arg)
>> +{
>> + int status;
>> + struct serial_rs485 data;
>> + struct f81534_port_private *port_priv = usb_get_serial_port_data(port);
>> + struct f81534_serial_private *serial_priv =
>> + usb_get_serial_data(port->serial);
>
> One line?
>

It's over 80 character with a tab, It cant be one line.

I'll fix other issue that you mention.

Thanks for your advices.
--
With Best Regards,
Peter Hung


\
 
 \ /
  Last update: 2015-11-04 10:01    [W:0.083 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site