Messages in this thread | | | Subject | Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver | From | Peter Hung <> | Date | Wed, 4 Nov 2015 16:41:54 +0800 |
| |
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
| |