Hi Andy,
Andy Shevchenko 於 2016/2/16 下午 05:11 寫道:
On Tue, 2016-02-16 at 14:55 +0800, Peter Hung wrote:
+static u32 baudrate_table[] = { 1500000, 1152000, 921600 };
+static u8 clock_table[] = { F81504_CLKSEL_24_MHZ,
F81504_CLKSEL_18DOT46_MHZ,
+ F81504_CLKSEL_14DOT77_MHZ };
I suggest to replace DOT by _.
ok
+/* We should do proper H/W transceiver setting before change to
RS485 mode */
+static int f81504_rs485_config(struct uart_port *port,
+ struct serial_rs485 *rs485)
+{
+ u8 setting;
+ u8 *index = (u8 *) port->private_data;
private_data is a type of void *, therefore no need to have an explicit
casting.
ok
+static int f81504_check_baudrate(u32 baud, size_t *idx)
+{
+ size_t index;
+ u32 quot, rem;
+
+ for (index = 0; index < ARRAY_SIZE(baudrate_table); ++index)
Post-increment is also okay.
{
+ /* Clock source must largeer than desire baudrate */
+ if (baud > baudrate_table[index])
+ continue;
+
+ quot = DIV_ROUND_CLOSEST(baudrate_table[index],
baud);
So, how quot is used and is it possible to set, for example, baud rate
as 1000000 or 576000?
The IC don't support B1000000 due to no 16MHz clock source.
The quot & rem is only use for compare, and it's must be not 0
when the code run to calculate DIV_ROUND_CLOSEST. So quot
is a redundancy here.
This function will find the suitable clock source for future use.
We'll pass suitable baud rate * 16 to port->uartclk for
serial8250_do_set_termios() to do advance divider operations.
I'll rewrite this section with remove quot and direct check
the "baudrate_table[index] % baud" divisible.
+ u8 tmp, *offset = (u8 *) port->private_data;
Same for provate_data as above.
ok
+ /*
+ * direct use 1.8432MHz when baudrate
smaller then or
+ * equal 115200bps
Check your style of comments in a _whole_ your series.
ok
+ /*
+ * If it can't found suitable clock source
but had old
+ * accpetable baudrate, we'll use it
Typo: acceptable.
Baudrate -> baud rate.
ok
+ port.port.private_data = data; /* save current idx */
Not sure you need to allocate memory for that at all, or maybe use a
struct with one member (for now).
We just pass the index of PCI configuration space currently. So
I just set a allocated u8 memory to private data. We'll maintain
current method.
+static SIMPLE_DEV_PM_OPS(f81504_serial_pm_ops,
f81504_serial_suspend,
+ f81504_serial_resume);
+
+static struct platform_driver f81504_serial_driver = {
+ .driver = {
+ .name = F81504_SERIAL_NAME,
+ .owner = THIS_MODULE,
You perhaps don't need this. Check the rest of the modules.
ok
+config SERIAL_8250_F81504
+ tristate "Fintek F81504/508/512 16550 PCIE device support"
if EXPERT
+ depends on SERIAL_8250 && MFD_FINTEK_F81504_CORE
+ default SERIAL_8250
+ select RATIONAL
It seems RATIONAL API is not used here.
This driver hadn't use RATIONAL API. I'll remove it.
Thanks for your advice.
--
With Best Regards,
Peter Hung
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
