TTY layer discussion about generic FIFO depth and Rx iddle timeout control

TTY layer discussion about generic FIFO depth and Rx iddle timeout control

[Pavel Pisa]

Dear Wander and Greg,

[resend on base of email-bot of Greg Kroah-Hartman's inbox]

I have noticed that you have sent enhancements
to the TTY layer.

I have worked on architecture of automotive LIN-bus
support for Linux UARTs. The SocketCAN API was idea
of Oliver Hartkopp and we have designed internals
to implement actual protocol. Rostislav Lisovy
was main author at our university in 2011. The code
has been used and is used by more people and I have
helped its integration to local Volkswagen subsidiary
projects. I have helped to maintain it for years
even that I have actually no use for it or contract.
But is seems usable...

I am not sure if it can reach quality standards
for mainline but I have tried to consolidate
many forks and copies from our original GIT
server which can be found on GitHub and united
project under

  https://github.com/lin-bus

Kernel part - slLIN TTY discipline - can be found there

  https://github.com/lin-bus/linux-lin/tree/master/sllin

Documentation

  https://github.com/lin-bus/linux-lin/wiki/

The main obstacle to have version which can be used
with different UARTs seamlessly is missing internal low
level kernel API which would allow to control Rx trig
level.

I have not checked your changes yet but I would be happy
if some API is available for this control. Please see
issue 

  https://github.com/lin-bus/linux-lin/issues/13

Please suggest where to discuss the proposal/solutions
or if you plan to implement something like that.

I would be happy to work on that myself or with my students
but I personally do not get to that probably earlier
than in summer. I have to finish project for European Space
Agency at PiKRON company. We have quite lot of work
to switch our Computer Architectures classes and corresponding
QtMips/QtRvSim simulator to RISC-V etc...
Mainlining CTU CAN FD driver has higher priority than LIN for
me as well.

So my actual motivation is to document the need and get some
feedback if some such solution is on the horizon
and what should API look like if I get to it ourselves etc..

Best wishes,

                Pavel
--
                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@cmp.felk.cvut.cz
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://dce.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
    Open Technologies Research Education and Exchange Services
    https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home

Re: TTY layer discussion about generic FIFO depth and Rx iddle timeout control

[Greg Kroah-Hartman]

On Mon, Jan 17, 2022 at 12:06:31AM +0100, Pavel Pisa wrote:
> Dear Wander and Greg,
> 
> [resend on base of email-bot of Greg Kroah-Hartman's inbox]
> 
> I have noticed that you have sent enhancements
> to the TTY layer.
> 
> I have worked on architecture of automotive LIN-bus
> support for Linux UARTs. The SocketCAN API was idea
> of Oliver Hartkopp and we have designed internals
> to implement actual protocol. Rostislav Lisovy
> was main author at our university in 2011. The code
> has been used and is used by more people and I have
> helped its integration to local Volkswagen subsidiary
> projects. I have helped to maintain it for years
> even that I have actually no use for it or contract.
> But is seems usable...
> 
> I am not sure if it can reach quality standards
> for mainline but I have tried to consolidate
> many forks and copies from our original GIT
> server which can be found on GitHub and united
> project under
> 
>   https://github.com/lin-bus
> 
> Kernel part - slLIN TTY discipline - can be found there
> 
>   https://github.com/lin-bus/linux-lin/tree/master/sllin

So it's just a 2000 line kernel module?  That should be easy to turn
into a patch and submit for review, right?

Odds are it can be made much smaller based on an initial glance at it.
Review comments can help show what to do.

> Documentation
> 
>   https://github.com/lin-bus/linux-lin/wiki/
> 
> The main obstacle to have version which can be used
> with different UARTs seamlessly is missing internal low
> level kernel API which would allow to control Rx trig
> level.
> 
> I have not checked your changes yet but I would be happy
> if some API is available for this control. Please see
> issue 
> 
>   https://github.com/lin-bus/linux-lin/issues/13
> 
> Please suggest where to discuss the proposal/solutions
> or if you plan to implement something like that.

Discuss it here by submitting patches please.  Links to random github
repos do not do much as we can do nothing with them, sorry.

thanks,

greg k-h

Re: TTY layer discussion about generic FIFO depth and Rx iddle timeout control

[Wander Costa]

On Sun, Jan 16, 2022 at 8:06 PM Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
>
> Dear Wander and Greg,
>

Hi there,

Notice that by no means I am an expert either on the TTY or CAN subsystems.

> [resend on base of email-bot of Greg Kroah-Hartman's inbox]
>
> I have noticed that you have sent enhancements
> to the TTY layer.
>
> I have worked on architecture of automotive LIN-bus
> support for Linux UARTs. The SocketCAN API was idea
> of Oliver Hartkopp and we have designed internals
> to implement actual protocol. Rostislav Lisovy
> was main author at our university in 2011. The code
> has been used and is used by more people and I have
> helped its integration to local Volkswagen subsidiary
> projects. I have helped to maintain it for years
> even that I have actually no use for it or contract.
> But is seems usable...
>
> I am not sure if it can reach quality standards
> for mainline but I have tried to consolidate
> many forks and copies from our original GIT
> server which can be found on GitHub and united
> project under
>
>   https://github.com/lin-bus
>
> Kernel part - slLIN TTY discipline - can be found there
>
>   https://github.com/lin-bus/linux-lin/tree/master/sllin
>
> Documentation
>
>   https://github.com/lin-bus/linux-lin/wiki/
>
> The main obstacle to have version which can be used
> with different UARTs seamlessly is missing internal low
> level kernel API which would allow to control Rx trig
> level.
>

Do you mean the electric wire-level or Rx buffer threshold?

> I have not checked your changes yet but I would be happy
> if some API is available for this control. Please see
> issue
>
>   https://github.com/lin-bus/linux-lin/issues/13
>

From what I read I believe you are talking about the FIFO threshold.
One approach Is adding a new function to serdev_controller_ops for
supported (and tested) controllers, I think.

> Please suggest where to discuss the proposal/solutions
> or if you plan to implement something like that.
>
> I would be happy to work on that myself or with my students
> but I personally do not get to that probably earlier
> than in summer. I have to finish project for European Space
> Agency at PiKRON company. We have quite lot of work
> to switch our Computer Architectures classes and corresponding
> QtMips/QtRvSim simulator to RISC-V etc...
> Mainlining CTU CAN FD driver has higher priority than LIN for
> me as well.
>
> So my actual motivation is to document the need and get some
> feedback if some such solution is on the horizon
> and what should API look like if I get to it ourselves etc..
>

As Greg said, it is easier to organize the work in a series of patches
and send them for review.


> Best wishes,
>
>                 Pavel
> --
>                 Pavel Pisa
>     phone:      +420 603531357
>     e-mail:     pisa@cmp.felk.cvut.cz
>     Department of Control Engineering FEE CVUT
>     Karlovo namesti 13, 121 35, Prague 2
>     university: http://dce.fel.cvut.cz/
>     personal:   http://cmp.felk.cvut.cz/~pisa
>     projects:   https://www.openhub.net/accounts/ppisa
>     CAN related:http://canbus.pages.fel.cvut.cz/
>     Open Technologies Research Education and Exchange Services
>     https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home
>
>

Re: TTY layer discussion about generic FIFO depth and Rx iddle timeout control

[Pavel Pisa]

Dear Greg,

thanks for the reply.

On Monday 17 of January 2022 14:03:18 Greg Kroah-Hartman wrote:
> On Mon, Jan 17, 2022 at 12:06:31AM +0100, Pavel Pisa wrote:
> >   https://github.com/lin-bus
> >
> > Kernel part - slLIN TTY discipline - can be found there
> >
> >   https://github.com/lin-bus/linux-lin/tree/master/sllin
>
> So it's just a 2000 line kernel module?  That should be easy to turn
> into a patch and submit for review, right?
>
> Odds are it can be made much smaller based on an initial glance at it.
> Review comments can help show what to do.

Thanks for encouragement for mainlining or at least review on the list.
I agree that it can shrink when patch for mainline without sections
providing compatibility with old kernels is prepared.
Generally, I think that it is doable and important is feedback
from the user base that there is interrest... and time...

I think that resolution of APO for the trigger/FIFO control
is critical for thinking about mainlining. Rest is the usual
work...

> >   https://github.com/lin-bus/linux-lin/issues/13
> >
> Discuss it here by submitting patches please.  Links to random github
> repos do not do much as we can do nothing with them, sorry.

Yes, I understand but I would like to hear some suggestion
the first where/into which object operations structure
should be the function added.

There is required functionality in 8250 driver linux/drivers/tty/serial/8250/8250_port.c

     do_set_rxtrig(struct tty_port *port, unsigned char bytes)
     do_serial8250_set_rxtrig(...)
     serial8250_set_attr_rx_trig_bytes(...)
     DEVICE_ATTR_RW(rx_trig_bytes)

But to make slLIN generally usable, we would need to have functionality
reachable from the line discipline

Do you agree that right place is struct uart_ops?

  https://elixir.bootlin.com/linux/latest/source/include/linux/serial_core.h#L38

What should be a prototype?

In general, it would worth to have possibility to set Rx trigger level
and Rx iddle timeout. It would be ideal if the set function would
adjust value down to the first equal or smaller level supported
by uart HW and in the case of lack of support can switch FIFO off.
The iddle time can be specified in microseconds, nanoseconds and or
number of character times.

So questions:

Is more preferred to add two functions, one for iddle time, one
for trigger level or combined function is better?

How should work reporting of the supported values?

Should there be more functions etc...

I can imagine generic API which would provide all functionality
by single function to not inflate struct uart_ops too much.

  int (*rx_trigger)(struct uart_port *, int mode, int *rx_trigger_bytes,
                  int *rx_trigger_iddle_time)

The rx_trigger_bytes equal to 0 would mean switch FIFO off.
Trigger time can be in nanoseconds??? and would be recomputed
to number of character for some, most of the HW???

The mode could have

  UART_RX_TRIGGER_MODE_SET
  UART_RX_TRIGGER_MODE_CHECK_ROUND_DOWN
  UART_RX_TRIGGER_MODE_CHECK_ROUND_UP

When specified parameters cannot be set, the function returns
error. When called with ROUND_UP/DOWN it modifies parameters
to the first available value in given direction
and next call to UART_RX_TRIGGER_MODE_SET would be guaranteed
to succeed.

Other option is to combine functionality to set_termios
call, but it is quite convoluted by its history and connection
to user space API. Not that maintaining trigger levels from
userspace is not usefull, but...

I would be happy if you or somebody other with linux
kernel and serial port style taste would comment.

Best wishes ant thanks for your time and work,


                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@cmp.felk.cvut.cz
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://dce.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
    Open Technologies Research Education and Exchange Services
    https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home

Re: TTY layer discussion about generic FIFO depth and Rx iddle timeout control

[Greg Kroah-Hartman]

On Wed, Jan 19, 2022 at 02:46:28PM +0100, Pavel Pisa wrote:
> Dear Greg,
> 
> thanks for the reply.
> 
> On Monday 17 of January 2022 14:03:18 Greg Kroah-Hartman wrote:
> > On Mon, Jan 17, 2022 at 12:06:31AM +0100, Pavel Pisa wrote:
> > >   https://github.com/lin-bus
> > >
> > > Kernel part - slLIN TTY discipline - can be found there
> > >
> > >   https://github.com/lin-bus/linux-lin/tree/master/sllin
> >
> > So it's just a 2000 line kernel module?  That should be easy to turn
> > into a patch and submit for review, right?
> >
> > Odds are it can be made much smaller based on an initial glance at it.
> > Review comments can help show what to do.
> 
> Thanks for encouragement for mainlining or at least review on the list.
> I agree that it can shrink when patch for mainline without sections
> providing compatibility with old kernels is prepared.
> Generally, I think that it is doable and important is feedback
> from the user base that there is interrest... and time...
> 
> I think that resolution of APO for the trigger/FIFO control
> is critical for thinking about mainlining. Rest is the usual
> work...
> 
> > >   https://github.com/lin-bus/linux-lin/issues/13
> > >
> > Discuss it here by submitting patches please.  Links to random github
> > repos do not do much as we can do nothing with them, sorry.
> 
> Yes, I understand but I would like to hear some suggestion
> the first where/into which object operations structure
> should be the function added.
> 
> There is required functionality in 8250 driver linux/drivers/tty/serial/8250/8250_port.c
> 
>      do_set_rxtrig(struct tty_port *port, unsigned char bytes)
>      do_serial8250_set_rxtrig(...)
>      serial8250_set_attr_rx_trig_bytes(...)
>      DEVICE_ATTR_RW(rx_trig_bytes)
> 
> But to make slLIN generally usable, we would need to have functionality
> reachable from the line discipline
> 
> Do you agree that right place is struct uart_ops?
> 
>   https://elixir.bootlin.com/linux/latest/source/include/linux/serial_core.h#L38
> 
> What should be a prototype?

For all of these questions, I do not know.  Try it out yourself first
and see what you feel works best.  We will be glad to review working
patches, but to discuss options before that is difficult and not
something we normally worry about.

thanks,

greg k-h

Re: TTY layer discussion about generic FIFO depth and Rx iddle timeout control

[Wander Costa]

On Wed, Jan 26, 2022 at 5:55 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 19, 2022 at 02:46:28PM +0100, Pavel Pisa wrote:
> > Dear Greg,
> >
> > thanks for the reply.
> >
> > On Monday 17 of January 2022 14:03:18 Greg Kroah-Hartman wrote:
> > > On Mon, Jan 17, 2022 at 12:06:31AM +0100, Pavel Pisa wrote:
> > > >   https://github.com/lin-bus
> > > >
> > > > Kernel part - slLIN TTY discipline - can be found there
> > > >
> > > >   https://github.com/lin-bus/linux-lin/tree/master/sllin
> > >
> > > So it's just a 2000 line kernel module?  That should be easy to turn
> > > into a patch and submit for review, right?
> > >
> > > Odds are it can be made much smaller based on an initial glance at it.
> > > Review comments can help show what to do.
> >
> > Thanks for encouragement for mainlining or at least review on the list.
> > I agree that it can shrink when patch for mainline without sections
> > providing compatibility with old kernels is prepared.
> > Generally, I think that it is doable and important is feedback
> > from the user base that there is interrest... and time...
> >
> > I think that resolution of APO for the trigger/FIFO control
> > is critical for thinking about mainlining. Rest is the usual
> > work...
> >
> > > >   https://github.com/lin-bus/linux-lin/issues/13
> > > >
> > > Discuss it here by submitting patches please.  Links to random github
> > > repos do not do much as we can do nothing with them, sorry.
> >
> > Yes, I understand but I would like to hear some suggestion
> > the first where/into which object operations structure
> > should be the function added.
> >
> > There is required functionality in 8250 driver linux/drivers/tty/serial/8250/8250_port.c
> >
> >      do_set_rxtrig(struct tty_port *port, unsigned char bytes)
> >      do_serial8250_set_rxtrig(...)
> >      serial8250_set_attr_rx_trig_bytes(...)
> >      DEVICE_ATTR_RW(rx_trig_bytes)
> >
> > But to make slLIN generally usable, we would need to have functionality
> > reachable from the line discipline
> >
> > Do you agree that right place is struct uart_ops?
> >
> >   https://elixir.bootlin.com/linux/latest/source/include/linux/serial_core.h#L38
> >
> > What should be a prototype?
>
> For all of these questions, I do not know.  Try it out yourself first
> and see what you feel works best.  We will be glad to review working
> patches, but to discuss options before that is difficult and not
> something we normally worry about.
>

That gives me some time to find more machines to test the fixes.

> thanks,
>
> greg k-h
>