[PATCH linux dev-4.13 v2 2/2] serial: npcm: add NPCM UART driver
joel at jms.id.au
Fri Feb 2 13:51:06 AEDT 2018
On Wed, Jan 24, 2018 at 1:46 AM, Tomer Maimon <tmaimon77 at gmail.com> wrote:
> Add Nuvoton BMC NPCM UART driver.
> The NPCM7xx BMC contains four UART blocks and accessory logic.
> NPCM UART based on 8250 driver.
Nice! Much less code to review and maintain than before :)
I did some more digging, to see if we could add even less code. I'll
sent a patch to the list, but I need to clarify the divisor
> +static int npcm_uart_set_baud_rate(struct uart_port *port,
> + NPCM_UART_BAUDRATE_T baudrate)
> + struct uart_8250_port *up = up_to_u8250p(port);
> + int divisor = 0;
> + int ret = 0;
> + unsigned long flags;
> + divisor = ((int)port->uartclk / ((int)baudrate * 16)) - 2;
The 8250 core uses this calculation:
divisor = port->uartclk / (baudrate * 16).
Checking the datasheet, this is the calculation that is documented for Poleg:
divisor = port->uartclk / (baudrate * 16 + 2).
If I change the calculation to this, the UART doesn't work (due to the
precision of the integer division, there's no change from the 8250
core code). If I subtract 2 from the number, as you do in your patch,
Can you explain what is happening here?
> + /* since divisor is rounded down check
> + * if it is better when rounded up
> + */
> + if (((int)port->uartclk / (16 * (divisor + 2)) - (int)baudrate) >
> + ((int)baudrate -
> + (int)port->uartclk / (16 * ((divisor + 1) + 2))))
> + divisor++;
This bit confused me when I tried to understand what it was checking.
I couldn't find a mention of it in the datasheet either.
Can you explain to me what it is trying to do? Do we need it?
More information about the openbmc