Re: [PATCH] serial: sh-sci: Support custom speed setting

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Eugeniu Rosca <erosca@de.adit-jv.com>
Cc: "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	Ulrich Hecht <uli+renesas@fpond.eu>,
	"George G . Davis" <george_davis@mentor.com>,
	Andrew Gabbasov <andrew_gabbasov@mentor.com>,
	Jiada Wang <jiada_wang@mentor.com>,
	Yuichi Kusakabe <yuichi.kusakabe@denso-ten.com>,
	Yasushi Asano <yasano@jp.adit-jv.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	Fukui Yohhei <yohhei.fukui@denso-ten.com>,
	Torii Kenichi <torii.ken1@jp.fujitsu.com>,
	Magnus Damm <magnus.damm@gmail.com>
Subject: Re: [PATCH] serial: sh-sci: Support custom speed setting
Date: Thu, 30 Jan 2020 13:32:50 +0100	[thread overview]
Message-ID: <CAMuHMdWV0kkKq6sKOHsdz+FFGNHphzq_q7rvmYAL=U4fH2H3wQ@mail.gmail.com> (raw)
In-Reply-To: <20200129161955.30562-1-erosca@de.adit-jv.com>

Hi Eugeniu,

On Wed, Jan 29, 2020 at 5:20 PM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> From: Torii Kenichi <torii.ken1@jp.fujitsu.com>
>
> This patch is necessary to use BT module and XM module with DENSO TEN
> development board.
>
> This patch supports ASYNC_SPD_CUST flag by ioctl(TIOCSSERIAL), enables
> custom speed setting with setserial(1).
>
> The custom speed is calculated from uartclk and custom_divisor.
> If custom_divisor is zero, custom speed setting is invalid.
>
> Signed-off-by: Torii Kenichi <torii.ken1@jp.fujitsu.com>
> [erosca: rebase against v5.5]
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>

Thanks for your patch!

While this seems to work fine[*], I have a few comments/questions:
  1. This feature seems to be deprecated:

         sh-sci e6e68000.serial: setserial sets custom speed on
ttySC1. This is deprecated.

  2. As the wanted speed is specified as a divider, the resulting speed
     may be off, cfr. the example for 57600 below.
     Note that the SCIF device has multiple clock inputs, and can do
     57600 perfectly if the right crystal has been fitted.

 3. What to do with "[PATCH/RFC] serial: sh-sci: Update uartclk based
     on selected clock" (https://patchwork.kernel.org/patch/11103703/)?
     Combined with this, things become pretty complicated and
     unpredictable, as uartclk now always reflect the frequency of the
     last used base clock, which was the optimal one for the previously
     used speed....

I think it would be easier if we just had an API to specify a raw speed.
Perhaps that already exists?

BTW, what's the speed you need for your BT/XM modules?

[*] stty speed 38400 < /dev/ttySC1 followed by
    setserial /dev/ttySC1 spd_cust divisor 1128 gives 57624 bps on Koelsch.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds