Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] serial: sh-sci: Support custom speed setting
@ 2020-01-29 16:19 Eugeniu Rosca
  2020-01-30 12:32 ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Eugeniu Rosca @ 2020-01-29 16:19 UTC (permalink / raw)
  To: Geert Uytterhoeven, linux-serial, linux-renesas-soc
  Cc: Wolfram Sang, Yoshihiro Shimoda, Ulrich Hecht, George G . Davis,
	Andrew Gabbasov, Jiada Wang, Yuichi Kusakabe, Yasushi Asano,
	linux-kernel, Greg Kroah-Hartman, Jiri Slaby, Fukui Yohhei,
	Torii Kenichi, Eugeniu Rosca

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>
---
 drivers/tty/serial/sh-sci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 87ca6294de0e..dd468909b2c4 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2405,6 +2405,10 @@ static void sci_set_termios(struct uart_port *port, struct ktermios *termios,
 	if (!baud)
 		goto done;
 
+	if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST &&
+	    port->custom_divisor)
+		baud = port->uartclk / port->custom_divisor;
+
 	/*
 	 * There can be multiple sources for the sampling clock.  Find the one
 	 * that gives us the smallest deviation from the desired baud rate.
-- 
2.25.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sh-sci: Support custom speed setting
  2020-01-29 16:19 [PATCH] serial: sh-sci: Support custom speed setting Eugeniu Rosca
@ 2020-01-30 12:32 ` Geert Uytterhoeven
  2020-02-10 20:57   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2020-01-30 12:32 UTC (permalink / raw)
  To: Eugeniu Rosca
  Cc: open list:SERIAL DRIVERS, Linux-Renesas, Wolfram Sang,
	Yoshihiro Shimoda, Ulrich Hecht, George G . Davis,
	Andrew Gabbasov, Jiada Wang, Yuichi Kusakabe, Yasushi Asano,
	Linux Kernel Mailing List, Greg Kroah-Hartman, Jiri Slaby,
	Fukui Yohhei, Torii Kenichi, Magnus Damm

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sh-sci: Support custom speed setting
  2020-01-30 12:32 ` Geert Uytterhoeven
@ 2020-02-10 20:57   ` Greg Kroah-Hartman
  2020-02-10 21:44     ` Eugeniu Rosca
  2020-02-11  8:15     ` Geert Uytterhoeven
  0 siblings, 2 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-10 20:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Eugeniu Rosca, open list:SERIAL DRIVERS, Linux-Renesas,
	Wolfram Sang, Yoshihiro Shimoda, Ulrich Hecht, George G . Davis,
	Andrew Gabbasov, Jiada Wang, Yuichi Kusakabe, Yasushi Asano,
	Linux Kernel Mailing List, Jiri Slaby, Fukui Yohhei,
	Torii Kenichi, Magnus Damm

On Thu, Jan 30, 2020 at 01:32:50PM +0100, Geert Uytterhoeven wrote:
> 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?

Yes, see:
	http://www.panix.com/~grante/arbitrary-baud.c


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sh-sci: Support custom speed setting
  2020-02-10 20:57   ` Greg Kroah-Hartman
@ 2020-02-10 21:44     ` Eugeniu Rosca
  2020-02-11  8:15     ` Geert Uytterhoeven
  1 sibling, 0 replies; 6+ messages in thread
From: Eugeniu Rosca @ 2020-02-10 21:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Geert Uytterhoeven
  Cc: open list:SERIAL DRIVERS, Linux-Renesas, Wolfram Sang,
	Yoshihiro Shimoda, Ulrich Hecht, George G . Davis,
	Andrew Gabbasov, Jiada Wang, Yuichi Kusakabe, Yasushi Asano,
	Linux Kernel Mailing List, Jiri Slaby, Fukui Yohhei,
	Torii Kenichi, Magnus Damm, Eugeniu Rosca, Eugeniu Rosca

Dear Geert and Greg,

On Mon, Feb 10, 2020 at 12:57:35PM -0800, Greg Kroah-Hartman wrote:
> On Thu, Jan 30, 2020 at 01:32:50PM +0100, Geert Uytterhoeven wrote:
> > 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?
> 
> Yes, see:
> 	http://www.panix.com/~grante/arbitrary-baud.c

This looks like a compelling piece of evidence users should stay away
from implementing and using the kludge (38400 baud) mechanism?

Unless the author and the users of this patch (CC-ed in this thread)
have a different opinion, I consider this topic closed. Thanks!

-- 
Best Regards
Eugeniu Rosca

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sh-sci: Support custom speed setting
  2020-02-10 20:57   ` Greg Kroah-Hartman
  2020-02-10 21:44     ` Eugeniu Rosca
@ 2020-02-11  8:15     ` Geert Uytterhoeven
  2020-02-11 12:30       ` Greg Kroah-Hartman
  1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2020-02-11  8:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Eugeniu Rosca, open list:SERIAL DRIVERS, Linux-Renesas,
	Wolfram Sang, Yoshihiro Shimoda, Ulrich Hecht, George G . Davis,
	Andrew Gabbasov, Jiada Wang, Yuichi Kusakabe, Yasushi Asano,
	Linux Kernel Mailing List, Jiri Slaby, Fukui Yohhei,
	Torii Kenichi, Magnus Damm, Michael Kerrisk (man-pages),
	linux-man

Hi Greg,

CC man

On Mon, Feb 10, 2020 at 9:57 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 30, 2020 at 01:32:50PM +0100, Geert Uytterhoeven wrote:
> > 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?
>
> Yes, see:
>         http://www.panix.com/~grante/arbitrary-baud.c

Thanks a lot!!
This must be one of the most guarded secrets of serial port programming ;-)

Implemented since 2006, commit edc6afc5496875a6 ("[PATCH] tty: switch to
ktermios and new framework"), not documented in today's man-pages.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] serial: sh-sci: Support custom speed setting
  2020-02-11  8:15     ` Geert Uytterhoeven
@ 2020-02-11 12:30       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-11 12:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Eugeniu Rosca, open list:SERIAL DRIVERS, Linux-Renesas,
	Wolfram Sang, Yoshihiro Shimoda, Ulrich Hecht, George G . Davis,
	Andrew Gabbasov, Jiada Wang, Yuichi Kusakabe, Yasushi Asano,
	Linux Kernel Mailing List, Jiri Slaby, Fukui Yohhei,
	Torii Kenichi, Magnus Damm, Michael Kerrisk (man-pages),
	linux-man

On Tue, Feb 11, 2020 at 09:15:02AM +0100, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> CC man
> 
> On Mon, Feb 10, 2020 at 9:57 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 30, 2020 at 01:32:50PM +0100, Geert Uytterhoeven wrote:
> > > 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?
> >
> > Yes, see:
> >         http://www.panix.com/~grante/arbitrary-baud.c
> 
> Thanks a lot!!
> This must be one of the most guarded secrets of serial port programming ;-)

It really is, I have to look it up each time it comes up :(

> Implemented since 2006, commit edc6afc5496875a6 ("[PATCH] tty: switch to
> ktermios and new framework"), not documented in today's man-pages.

yeah, serial port control really needs to be documented better, there's
all sorts of nice ways of controlling them that people don't seem to
know about.  I used to have a link to a bunch of online examples, but
can't seem to find that anymore either.  Ugh, another thing for the long
TODO file...

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 16:19 [PATCH] serial: sh-sci: Support custom speed setting Eugeniu Rosca
2020-01-30 12:32 ` Geert Uytterhoeven
2020-02-10 20:57   ` Greg Kroah-Hartman
2020-02-10 21:44     ` Eugeniu Rosca
2020-02-11  8:15     ` Geert Uytterhoeven
2020-02-11 12:30       ` Greg Kroah-Hartman

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git