Hi Paul,
On Tuesday 29 October 2013 11:29:57 Laurent Pinchart wrote:
> Rewrite the baud rate register value calculations in easier to read
> forms. The computed value isn't modified.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/sh-sci.c | 8 ++++----
> include/linux/serial_sci.h | 8 ++++----
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2fc1a0e..91ce9214b 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1813,13 +1813,13 @@ static unsigned int sci_scbrr_calc(unsigned int
> algo_id, unsigned int bps, {
> switch (algo_id) {
> case SCBRR_ALGO_1:
> - return ((freq + 16 * bps) / (16 * bps) - 1);
> + return freq / (16 * bps);
> case SCBRR_ALGO_2:
> - return ((freq + 16 * bps) / (32 * bps) - 1);
> + return DIV_ROUND_CLOSEST(freq, 32 * bps) - 1;
> case SCBRR_ALGO_3:
> - return (((freq * 2) + 16 * bps) / (16 * bps) - 1);
> + return freq / (8 * bps);
> case SCBRR_ALGO_4:
> - return (((freq * 2) + 16 * bps) / (32 * bps) - 1);
> + return DIV_ROUND_CLOSEST(freq, 16 * bps) - 1;
> }
>
> /* Warn, but use a safe default */
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index 803068d..eb787d4 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -11,10 +11,10 @@
> #define SCIx_NOT_SUPPORTED (-1)
>
> enum {
> - SCBRR_ALGO_1, /* ((clk + 16 * bps) / (16 * bps) - 1) */
> - SCBRR_ALGO_2, /* ((clk + 16 * bps) / (32 * bps) - 1) */
> - SCBRR_ALGO_3, /* (((clk * 2) + 16 * bps) / (16 * bps) - 1) */
> - SCBRR_ALGO_4, /* (((clk * 2) + 16 * bps) / (32 * bps) - 1) */
> + SCBRR_ALGO_1, /* clk / (16 * bps) */
> + SCBRR_ALGO_2, /* DIV_ROUND_CLOSEST(clk, 32 * bps) - 1 */
> + SCBRR_ALGO_3, /* clk / (8 * bps) */
> + SCBRR_ALGO_4, /* DIV_ROUND_CLOSEST(clk, 16 * bps) - 1 */
> SCBRR_ALGO_6, /* HSCIF variable sample rate algorithm */
> };
While touching this, I'd like to use it as an opportunity to clean things up.
It looks to me like algorithms 1 and 3 should use DIV_ROUND_CLOSEST() - 1
instead of rounding the value up as they currently do. I've checked the
documentation for various SuperH SoCs and they all confirm my hypothesis. As I
don't have access to any SuperH board I can't test this though.
Given that you have touched the baudrate calculation code in the past, could
you give me an insight on why algorithms 1 and 3 are implemented without
rounding to the closest value and without the - 1 ?
On the same subject, algorithm 3 is only used on sh7723 and sh7724 for the
SCIFA. I've only managed to find a user's manual for the sh7724 (the publicly
available sh7723 documentation just lists the IP cores and their features),
and the SCIFA documentation seems to imply that, when using the default 1/16
sampling rate, algorithm 4 should be used. Could you enlighten me on why the
sh-sci driver assume a sampling rate of 1/8 instead of 1/16 for baudrate
calculation for SCIFA on sh772[34] ?
--
Regards,
Laurent Pinchart
--
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
