Re: [PATCH v3 10/15] serial: stm32-usart: Add STM32 USART Driver

[Maxime Coquelin]

Hi Peter,

2015-03-19 18:35 GMT+01:00 Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>:
> 2015-03-19 15:58 GMT+01:00 Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>:
>> On 03/19/2015 09:55 AM, Maxime Coquelin wrote:
>>>>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>>>>> +                           struct ktermios *old)
>> [...]
>>>>>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>>>>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>>>>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>>>>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>>>>>> +               fraction = 0;
>>>>>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>>>>>> +       }
>> [...]
>>> Really, I would prefer keeping this fractional divider as it is
>>> implemented today.
>>
>> You have to admit that's basically an unintelligible mess;
>> how would anyone ever be able to refactor and replace that with a
>> common divider implementation?
>>
>> At the very least, please comment on the formula and format.
>
> Ok, I will refactor the implementation, and comment it.

The implementation was indeed a mess.
I found some time to refactor the code, and also added support for 8
times oversampling (16 by default).
It will allow to achieve higher speeds, with the side effect of being
less tolerant to clock deviations.

What do you think about the code below?


    usartdiv = DIV_ROUND_CLOSEST(port->uartclk, baud);

    /*
     * The USART supports 16 or 8 times oversampling.
     * By default we prefer 16 times oversampling, so that the receiver
     * has a better tolerance to clock deviations.
     * 8 times oversampling is only used to achieve higher speeds.
     */
    if (usartdiv < 16) {
        oversampling = 8;
        stm32_set_bits(port, USART_CR1, USART_CR1_OVER8);
    } else {
        oversampling = 16;
        stm32_clr_bits(port, USART_CR1, USART_CR1_OVER8);
    }

    mantissa = (usartdiv / oversampling) << USART_BRR_DIV_M_SHIFT;
    fraction = usartdiv % oversampling;
    writel_relaxed(mantissa | fraction, port->membase + USART_BRR)

Thanks,
Maxime
>
> Regards,
> Maxime
>
>>
>> Regards,
>> Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/