From: Torii Kenichi <[email protected]>
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 <[email protected]>
[erosca: rebase against v5.5]
Signed-off-by: Eugeniu Rosca <[email protected]>
---
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
Hi Eugeniu,
On Wed, Jan 29, 2020 at 5:20 PM Eugeniu Rosca <[email protected]> wrote:
> From: Torii Kenichi <[email protected]>
>
> 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 <[email protected]>
> [erosca: rebase against v5.5]
> Signed-off-by: Eugeniu Rosca <[email protected]>
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 -- [email protected]
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
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 <[email protected]> wrote:
> > From: Torii Kenichi <[email protected]>
> >
> > 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 <[email protected]>
> > [erosca: rebase against v5.5]
> > Signed-off-by: Eugeniu Rosca <[email protected]>
>
> 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
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 <[email protected]> wrote:
> > > From: Torii Kenichi <[email protected]>
> > >
> > > 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 <[email protected]>
> > > [erosca: rebase against v5.5]
> > > Signed-off-by: Eugeniu Rosca <[email protected]>
> >
> > 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
Hi Greg,
CC man
On Mon, Feb 10, 2020 at 9:57 PM Greg Kroah-Hartman
<[email protected]> 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 <[email protected]> wrote:
> > > From: Torii Kenichi <[email protected]>
> > >
> > > 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 <[email protected]>
> > > [erosca: rebase against v5.5]
> > > Signed-off-by: Eugeniu Rosca <[email protected]>
> >
> > 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 -- [email protected]
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
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
> <[email protected]> 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 <[email protected]> wrote:
> > > > From: Torii Kenichi <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > [erosca: rebase against v5.5]
> > > > Signed-off-by: Eugeniu Rosca <[email protected]>
> > >
> > > 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
Dear Greg,
On Tue, 11 Feb 2020 05:57:35 +0900,
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 <[email protected]> wrote:
> > > From: Torii Kenichi <[email protected]>
> > >
> > > 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 <[email protected]>
> > > [erosca: rebase against v5.5]
> > > Signed-off-by: Eugeniu Rosca <[email protected]>
> >
> > 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
I saw the code above, I thought I wouldn't write such code normally.
>#include <linux/termios.h>
>
>int ioctl(int d, int request, ...);
Do application programmers have to accept this bad code?
On Thu, Mar 12, 2020 at 05:10:05AM +0000, [email protected] wrote:
> Dear Greg,
>
> On Tue, 11 Feb 2020 05:57:35 +0900,
> 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 <[email protected]> wrote:
> > > > From: Torii Kenichi <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > [erosca: rebase against v5.5]
> > > > Signed-off-by: Eugeniu Rosca <[email protected]>
> > >
> > > 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
>
> I saw the code above, I thought I wouldn't write such code normally.
Why not?
> >#include <linux/termios.h>
> >
> >int ioctl(int d, int request, ...);
>
> Do application programmers have to accept this bad code?
I do not understand what you are asking here.
greg k-h
Hi Torii-san,
On Thu, Mar 12, 2020 at 6:10 AM [email protected]
<[email protected]> wrote:
> On Tue, 11 Feb 2020 05:57:35 +0900,
> Greg Kroah-Hartman 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 <[email protected]> wrote:
> > > > From: Torii Kenichi <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > [erosca: rebase against v5.5]
> > > > Signed-off-by: Eugeniu Rosca <[email protected]>
> > >
> > > 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
>
> I saw the code above, I thought I wouldn't write such code normally.
>
> >#include <linux/termios.h>
> >
> >int ioctl(int d, int request, ...);
>
> Do application programmers have to accept this bad code?
I guess you mean the forward declaration of ioctrl()?
No, they should include <sys/ioctl.h> instead.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Dear Greg, Geert,
> I guess you mean the forward declaration of ioctrl()?
> No, they should include <sys/ioctl.h> instead.
Right.
Adding "#include <sys/ioctl.h>" to Greg's sample code causes a
compilation error.
> > I saw the code above, I thought I wouldn't write such code normally.
> Why not?
Is it normal to declare ioctl() without "#include <sys/ioctl.h>" ?
Thanks & Best Regards,
Yuusuke Ashiduka <[email protected]>
Embedded System Development Dept. Embedded System Development Div.
FUJITSU COMPUTER TECHNOLOGIES Ltd.
> -----Original Message-----
> From: Geert Uytterhoeven <[email protected]>
> Sent: Thursday, March 12, 2020 6:03 PM
> To: Torii, Kenichi/鳥居 健一 <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Ashizuka, Yuusuke/
> 芦塚 雄介 <[email protected]>; [email protected]
> Subject: Re: [PATCH] serial: sh-sci: Support custom speed setting
>
> Hi Torii-san,
>
> On Thu, Mar 12, 2020 at 6:10 AM [email protected]
> <[email protected]> wrote:
> > On Tue, 11 Feb 2020 05:57:35 +0900,
> > Greg Kroah-Hartman 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
> <[email protected]> wrote:
> > > > > From: Torii Kenichi <[email protected]>
> > > > >
> > > > > 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 <[email protected]>
> > > > > [erosca: rebase against v5.5]
> > > > > Signed-off-by: Eugeniu Rosca <[email protected]>
> > > >
> > > > 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
> >
> > I saw the code above, I thought I wouldn't write such code normally.
> >
> > >#include <linux/termios.h>
> > >
> > >int ioctl(int d, int request, ...);
> >
> > Do application programmers have to accept this bad code?
>
> I guess you mean the forward declaration of ioctrl()?
> No, they should include <sys/ioctl.h> instead.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> [email protected]
>
> 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
Dear Greg, Geert,
> Right.
> Adding "#include <sys/ioctl.h>" to Greg's sample code causes a
> compilation error.
<snip>
> Is it normal to declare ioctl() without "#include <sys/ioctl.h>" ?
I would be happy if you could give me some comments.
> http://www.panix.com/~grante/arbitrary-baud.c
We think this sample code is no good.
Should I work on glibc changes instead of kernel fixes?
Thanks & Best Regards,
Yuusuke Ashiduka <[email protected]>
Embedded System Development Dept. Embedded System Development Div.
FUJITSU COMPUTER TECHNOLOGIES Ltd.
> -----Original Message-----
> From: Ashizuka, Yuusuke/芦塚 雄介 <[email protected]>
> Sent: Thursday, March 12, 2020 6:44 PM
> To: 'Geert Uytterhoeven' <[email protected]>;
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected];
> [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Torii, Kenichi/鳥居 健一
> <[email protected]>
> Subject: RE: [PATCH] serial: sh-sci: Support custom speed setting
>
> Dear Greg, Geert,
>
> > I guess you mean the forward declaration of ioctrl()?
> > No, they should include <sys/ioctl.h> instead.
>
> Right.
> Adding "#include <sys/ioctl.h>" to Greg's sample code causes a
> compilation error.
>
> > > I saw the code above, I thought I wouldn't write such code
> normally.
> > Why not?
>
> Is it normal to declare ioctl() without "#include <sys/ioctl.h>" ?
>
> Thanks & Best Regards,
> Yuusuke Ashiduka <[email protected]>
> Embedded System Development Dept. Embedded System Development Div.
> FUJITSU COMPUTER TECHNOLOGIES Ltd.
>
> > -----Original Message-----
> > From: Geert Uytterhoeven <[email protected]>
> > Sent: Thursday, March 12, 2020 6:03 PM
> > To: Torii, Kenichi/鳥居 健一 <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected];
> [email protected];
> > [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; Ashizuka, Yuusuke/
> > 芦塚 雄介 <[email protected]>; [email protected]
> > Subject: Re: [PATCH] serial: sh-sci: Support custom speed setting
> >
> > Hi Torii-san,
> >
> > On Thu, Mar 12, 2020 at 6:10 AM [email protected]
> > <[email protected]> wrote:
> > > On Tue, 11 Feb 2020 05:57:35 +0900,
> > > Greg Kroah-Hartman 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
> > <[email protected]> wrote:
> > > > > > From: Torii Kenichi <[email protected]>
> > > > > >
> > > > > > 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 <[email protected]>
> > > > > > [erosca: rebase against v5.5]
> > > > > > Signed-off-by: Eugeniu Rosca <[email protected]>
> > > > >
> > > > > 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
> > >
> > > I saw the code above, I thought I wouldn't write such code
> normally.
> > >
> > > >#include <linux/termios.h>
> > > >
> > > >int ioctl(int d, int request, ...);
> > >
> > > Do application programmers have to accept this bad code?
> >
> > I guess you mean the forward declaration of ioctrl()?
> > No, they should include <sys/ioctl.h> instead.
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > [email protected]
> >
> > 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
Mon, Mar 30, 2020 at 07:43:09AM +0000, [email protected] kirjoitti:
> Dear Greg, Geert,
>
> > Right.
> > Adding "#include <sys/ioctl.h>" to Greg's sample code causes a
> > compilation error.
> <snip>
> > Is it normal to declare ioctl() without "#include <sys/ioctl.h>" ?
>
> I would be happy if you could give me some comments.
>
> > http://www.panix.com/~grante/arbitrary-baud.c
>
> We think this sample code is no good.
> Should I work on glibc changes instead of kernel fixes?
Side note: I hope introducing spd_cust hack hadn't made upstream.
To the point. Use BOTHER as in code excerpt. Yes, there is a problematic parts
with the headers regarding to this feature. But you may look how others solve
it.
https://github.com/npat-efault/picocom/blob/master/termios2.txt
> > Subject: RE: [PATCH] serial: sh-sci: Support custom speed setting
> >
> > Dear Greg, Geert,
> >
> > > I guess you mean the forward declaration of ioctrl()?
> > > No, they should include <sys/ioctl.h> instead.
> >
> > Right.
> > Adding "#include <sys/ioctl.h>" to Greg's sample code causes a
> > compilation error.
> >
> > > > I saw the code above, I thought I wouldn't write such code
> > normally.
> > > Why not?
> >
> > Is it normal to declare ioctl() without "#include <sys/ioctl.h>" ?
> >
> > Thanks & Best Regards,
> > Yuusuke Ashiduka <[email protected]>
> > Embedded System Development Dept. Embedded System Development Div.
> > FUJITSU COMPUTER TECHNOLOGIES Ltd.
> >
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <[email protected]>
> > > Sent: Thursday, March 12, 2020 6:03 PM
> > > To: Torii, Kenichi/鳥居 健一 <[email protected]>
> > > Cc: [email protected]; [email protected];
> > > [email protected];
> > [email protected];
> > > [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; Ashizuka, Yuusuke/
> > > 芦塚 雄介 <[email protected]>; [email protected]
> > > Subject: Re: [PATCH] serial: sh-sci: Support custom speed setting
> > >
> > > Hi Torii-san,
> > >
> > > On Thu, Mar 12, 2020 at 6:10 AM [email protected]
> > > <[email protected]> wrote:
> > > > On Tue, 11 Feb 2020 05:57:35 +0900,
> > > > Greg Kroah-Hartman 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
> > > <[email protected]> wrote:
> > > > > > > From: Torii Kenichi <[email protected]>
> > > > > > >
> > > > > > > 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 <[email protected]>
> > > > > > > [erosca: rebase against v5.5]
> > > > > > > Signed-off-by: Eugeniu Rosca <[email protected]>
> > > > > >
> > > > > > 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
> > > >
> > > > I saw the code above, I thought I wouldn't write such code
> > normally.
> > > >
> > > > >#include <linux/termios.h>
> > > > >
> > > > >int ioctl(int d, int request, ...);
> > > >
> > > > Do application programmers have to accept this bad code?
> > >
> > > I guess you mean the forward declaration of ioctrl()?
> > > No, they should include <sys/ioctl.h> instead.
> > >
> > > Gr{oetje,eeting}s,
> > >
> > > Geert
> > >
> > > --
> > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > > [email protected]
> > >
> > > 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
--
With Best Regards,
Andy Shevchenko