Re: [PATCH V6 1/1] usb:serial: Add Fintek F81532/534 driver

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

 



Hi

Andy Shevchenko 於 2015/11/3 下午 05:45 寫道:
On Tue, Nov 3, 2015 at 5:51 AM, Peter Hung <hpeter@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

  Powered by Linux