2017-03-13 22:09:36

by Okash Khawaja

[permalink] [raw]
Subject: [patch 0/7] staging: speakup: introduce tty-based comms

Hi,

This patchset introduces a TTY-based way for the synths to communicate
with devices as an alternate for direct serial comms used by the synths
at the moment. It then migrates some of the synths to the TTY-based
comms. Synths migrated in this patchset are dummy, acntsa, bns and
txprt.

As such, this series is meant to start a discussion on the changes in the
first patch: "tty_port: allow a port to be opened with a tty that has no
file handle". This allows speakup to access tty port early on during boot,
making it possible to start emiting messages as early as possible. This is
demonstrated in patch 6, where it calls tty_open_by_driver, passing in NULL
for inode and filp params.

To wrap up, here is a summary of the patches that follow. Patch 1 enables
kernel access to TTY device. Patches 2 - 5 prepare the code to accomodate
TTY-based comms as an alternate to raw serial I/O for outputting data to
external device. Patch 6 builds upon the changes made in patch 1 and utilises
TTY-subsystem to communicate with external device - currently just over ttyS*
but with plans to expand it to other TTY devices soon. Finally, patch 7 does
actual migration of some of the simple synths from raw serial comms to TTY.

Okash


2017-03-13 22:14:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> Hi,
>
> This patchset introduces a TTY-based way for the synths to communicate
> with devices as an alternate for direct serial comms used by the synths
> at the moment. It then migrates some of the synths to the TTY-based
> comms. Synths migrated in this patchset are dummy, acntsa, bns and
> txprt.

What about using the serbus code that is now in the tree? That should
make this a lot easier than your patchset from what I can see.

thanks,

greg k-h

2017-03-13 22:28:00

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Hello,

Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> > This patchset introduces a TTY-based way for the synths to communicate
> > with devices as an alternate for direct serial comms used by the synths
> > at the moment. It then migrates some of the synths to the TTY-based
> > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > txprt.
>
> What about using the serbus code that is now in the tree?

I guess you mean serdev?

Well, for a start that didn't exist when Okash worked on the TTY part :)

> That should make this a lot easier than your patchset from what I can
> see.

Will serdev support modem line control? We need this for some devices.
How is the serial port chosen with serdev?

Samuel

2017-03-13 23:43:23

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

On Mon, Mar 13, 2017 at 11:26:08PM +0100, Samuel Thibault wrote:
> Hello,
>
> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> > > This patchset introduces a TTY-based way for the synths to communicate
> > > with devices as an alternate for direct serial comms used by the synths
> > > at the moment. It then migrates some of the synths to the TTY-based
> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > txprt.
> >
> > What about using the serbus code that is now in the tree?
>
> I guess you mean serdev?

Sorry, yes.

> Well, for a start that didn't exist when Okash worked on the TTY part :)

Fair enough, but it has been discussed for many months now on the
linux-serial mailing list.

> > That should make this a lot easier than your patchset from what I can
> > see.
>
> Will serdev support modem line control? We need this for some devices.
> How is the serial port chosen with serdev?

Best asked on the linux-serial mailing list :)

thanks,

greg k-h

2017-03-13 23:49:37

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Greg KH, on mar. 14 mars 2017 07:43:03 +0800, wrote:
> > Well, for a start that didn't exist when Okash worked on the TTY part :)
>
> Fair enough, but it has been discussed for many months now on the
> linux-serial mailing list.

I have not had time for years to actually work on this subject, so I've
had even less time to follow linux-serial :) (and Okash, who actually
got the time to do the work, is very new to all this).

> > > That should make this a lot easier than your patchset from what I can
> > > see.
> >
> > Will serdev support modem line control? We need this for some devices.
> > How is the serial port chosen with serdev?
>
> Best asked on the linux-serial mailing list :)

Let's discuss there, then.

Samuel

2017-03-14 00:00:34

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Samuel Thibault, on lun. 13 mars 2017 23:26:08 +0100, wrote:
> > That should make this a lot easier than your patchset from what I can
> > see.
>
> Will serdev support modem line control? We need this for some devices.

"serdev: add serdev_device_set_rts" seems to be saying yes :)

Samuel

2017-03-14 00:25:05

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Hello Okash,

I'd say for a start already, rebase and submit patches 2-5, since they
are refactoring that makes sense anyway. And we can discuss about serdev
while that gets integrated, and thus serdev-equivalents of patches 1 and
6 will then be immediatetly applicable.

Samuel

2017-03-14 00:47:06

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Hello,

Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> > This patchset introduces a TTY-based way for the synths to communicate
> > with devices as an alternate for direct serial comms used by the synths
> > at the moment. It then migrates some of the synths to the TTY-based
> > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > txprt.
>
> What about using the serbus code that is now in the tree? That should
> make this a lot easier than your patchset from what I can see.

Mmm... AIUI from reading tty_port_register_device_attr, one
would have to have registered a speakup serdev device driver
*before* tty_port_register_device_attr gets called, so that
serdev_tty_port_register matches the driver in the loop of
of_serdev_register_devices, and no TTY cdev is created?

That would mean that speakup can not be loaded as a module after ttyS0
initialization, that won't fly for our use needs. The line discipline
mechanism allows us to attach ourself to an existing tty. Could we
imagine a tty_port function which removes the cdev and tries to register
the tty port again to serdev?

What we basically need to be able to say on speakup module load is
e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
discuss with it".

Samuel

2017-03-14 01:18:20

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> > > This patchset introduces a TTY-based way for the synths to communicate
> > > with devices as an alternate for direct serial comms used by the synths
> > > at the moment. It then migrates some of the synths to the TTY-based
> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > txprt.
> >
> > What about using the serbus code that is now in the tree? That should
> > make this a lot easier than your patchset from what I can see.
>
> Mmm... AIUI from reading tty_port_register_device_attr, one
> would have to have registered a speakup serdev device driver
> *before* tty_port_register_device_attr gets called, so that
> serdev_tty_port_register matches the driver in the loop of
> of_serdev_register_devices, and no TTY cdev is created?
>
> That would mean that speakup can not be loaded as a module after ttyS0
> initialization, that won't fly for our use needs. The line discipline
> mechanism allows us to attach ourself to an existing tty. Could we
> imagine a tty_port function which removes the cdev and tries to register
> the tty port again to serdev?
>
> What we basically need to be able to say on speakup module load is
> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> discuss with it".

That for_each_available_child_of_node loop is really way more complex
than what we need. And what's more, it's not working without CONFIG_OF
(!)

It would really make sense to me to have a

serdev_device *tty_port_register_serdev_device(tty, device)

which unregisters the character device of the tty, and creates instead a
controler with the given device plugged to it. Really much like a line
discipline, but way simpler :)

The issue that remains is the "tty" part: we'd need there the tty_port,
the parent device, the tty driver, the idx. All we know when we load
speakup is "ttyS0". That's where going through the cdev to attach a line
discipline was simpler :)

Samuel

2017-03-15 14:46:28

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
<[email protected]> wrote:
> Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
>> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
>> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
>> > > This patchset introduces a TTY-based way for the synths to communicate
>> > > with devices as an alternate for direct serial comms used by the synths
>> > > at the moment. It then migrates some of the synths to the TTY-based
>> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
>> > > txprt.
>> >
>> > What about using the serbus code that is now in the tree? That should
>> > make this a lot easier than your patchset from what I can see.
>>
>> Mmm... AIUI from reading tty_port_register_device_attr, one
>> would have to have registered a speakup serdev device driver
>> *before* tty_port_register_device_attr gets called, so that
>> serdev_tty_port_register matches the driver in the loop of
>> of_serdev_register_devices, and no TTY cdev is created?

Not exactly. The driver doesn't have to be registered nor loaded, but
the device does have to be present so no tty cdev is created. The only
way a device is present currently is when a node is in the DT. I
expect the x86 folks will be adding ACPI support soon.

>> That would mean that speakup can not be loaded as a module after ttyS0
>> initialization, that won't fly for our use needs. The line discipline
>> mechanism allows us to attach ourself to an existing tty. Could we
>> imagine a tty_port function which removes the cdev and tries to register
>> the tty port again to serdev?
>>
>> What we basically need to be able to say on speakup module load is
>> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
>> discuss with it".
>
> That for_each_available_child_of_node loop is really way more complex
> than what we need. And what's more, it's not working without CONFIG_OF
> (!)
>
> It would really make sense to me to have a
>
> serdev_device *tty_port_register_serdev_device(tty, device)
>
> which unregisters the character device of the tty, and creates instead a
> controler with the given device plugged to it. Really much like a line
> discipline, but way simpler :)

What would trigger calling this function? I could imagine people
wanting to do DT overlays to add a device which could trigger it. Or
did you have some other mechanism in mind.

>
> The issue that remains is the "tty" part: we'd need there the tty_port,
> the parent device, the tty driver, the idx. All we know when we load
> speakup is "ttyS0". That's where going through the cdev to attach a line
> discipline was simpler :)
>
> Samuel

2017-03-15 15:03:28

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> <[email protected]> wrote:
> > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> >> > > This patchset introduces a TTY-based way for the synths to communicate
> >> > > with devices as an alternate for direct serial comms used by the synths
> >> > > at the moment. It then migrates some of the synths to the TTY-based
> >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> >> > > txprt.
> >> >
> >> > What about using the serbus code that is now in the tree? That should
> >> > make this a lot easier than your patchset from what I can see.
> >>
> >> Mmm... AIUI from reading tty_port_register_device_attr, one
> >> would have to have registered a speakup serdev device driver
> >> *before* tty_port_register_device_attr gets called, so that
> >> serdev_tty_port_register matches the driver in the loop of
> >> of_serdev_register_devices, and no TTY cdev is created?
>
> Not exactly. The driver doesn't have to be registered nor loaded, but
> the device does have to be present so no tty cdev is created. The only
> way a device is present currently is when a node is in the DT. I
> expect the x86 folks will be adding ACPI support soon.

Ok, but in our case there is no hardware device that the system can
see/probe, it's just plugged externally.

> >> That would mean that speakup can not be loaded as a module after ttyS0
> >> initialization, that won't fly for our use needs. The line discipline
> >> mechanism allows us to attach ourself to an existing tty. Could we
> >> imagine a tty_port function which removes the cdev and tries to register
> >> the tty port again to serdev?
> >>
> >> What we basically need to be able to say on speakup module load is
> >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> >> discuss with it".
> >
> > That for_each_available_child_of_node loop is really way more complex
> > than what we need. And what's more, it's not working without CONFIG_OF
> > (!)
> >
> > It would really make sense to me to have a
> >
> > serdev_device *tty_port_register_serdev_device(tty, device)
> >
> > which unregisters the character device of the tty, and creates instead a
> > controler with the given device plugged to it. Really much like a line
> > discipline, but way simpler :)
>
> What would trigger calling this function?

>From the user point of view, loading the speakup module for the external
device, typically, with "ttyS0" as module parameter. Then the speakup
init function can do whatever it needs to achieve this :)

Samuel

2017-03-16 09:26:13

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Hello,

Also, could serdev also provide an send_xchar function similar to the
send_xchar tty driver function? We need this to send high-priority
characters.

Samuel

2017-03-22 00:05:31

by Samuel Thibault

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Hello,

So Rob, how do you see this going? Shall we introduce a serdev_device
*tty_port_register_serdev_device(tty, device)? Will you work on it or
should I give it a try?

Samuel

Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
> Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> > On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> > <[email protected]> wrote:
> > > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> > >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> > >> > > This patchset introduces a TTY-based way for the synths to communicate
> > >> > > with devices as an alternate for direct serial comms used by the synths
> > >> > > at the moment. It then migrates some of the synths to the TTY-based
> > >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > >> > > txprt.
> > >> >
> > >> > What about using the serbus code that is now in the tree? That should
> > >> > make this a lot easier than your patchset from what I can see.
> > >>
> > >> Mmm... AIUI from reading tty_port_register_device_attr, one
> > >> would have to have registered a speakup serdev device driver
> > >> *before* tty_port_register_device_attr gets called, so that
> > >> serdev_tty_port_register matches the driver in the loop of
> > >> of_serdev_register_devices, and no TTY cdev is created?
> >
> > Not exactly. The driver doesn't have to be registered nor loaded, but
> > the device does have to be present so no tty cdev is created. The only
> > way a device is present currently is when a node is in the DT. I
> > expect the x86 folks will be adding ACPI support soon.
>
> Ok, but in our case there is no hardware device that the system can
> see/probe, it's just plugged externally.
>
> > >> That would mean that speakup can not be loaded as a module after ttyS0
> > >> initialization, that won't fly for our use needs. The line discipline
> > >> mechanism allows us to attach ourself to an existing tty. Could we
> > >> imagine a tty_port function which removes the cdev and tries to register
> > >> the tty port again to serdev?
> > >>
> > >> What we basically need to be able to say on speakup module load is
> > >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> > >> discuss with it".
> > >
> > > That for_each_available_child_of_node loop is really way more complex
> > > than what we need. And what's more, it's not working without CONFIG_OF
> > > (!)
> > >
> > > It would really make sense to me to have a
> > >
> > > serdev_device *tty_port_register_serdev_device(tty, device)
> > >
> > > which unregisters the character device of the tty, and creates instead a
> > > controler with the given device plugged to it. Really much like a line
> > > discipline, but way simpler :)
> >
> > What would trigger calling this function?
>
> From the user point of view, loading the speakup module for the external
> device, typically, with "ttyS0" as module parameter. Then the speakup
> init function can do whatever it needs to achieve this :)

2017-04-09 16:54:50

by Okash Khawaja

[permalink] [raw]
Subject: Re: [patch 0/7] staging: speakup: introduce tty-based comms

Hi,

Any updates on this? We can start working in
tty_port_register_serdev_device() if that's okay?

Thanks,
Okash

On Wed, Mar 22, 2017 at 01:05:24AM +0100, Samuel Thibault wrote:
> Hello,
>
> So Rob, how do you see this going? Shall we introduce a serdev_device
> *tty_port_register_serdev_device(tty, device)? Will you work on it or
> should I give it a try?
>
> Samuel
>
> Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
> > Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> > > On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> > > <[email protected]> wrote:
> > > > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> > > >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > > >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, [email protected] wrote:
> > > >> > > This patchset introduces a TTY-based way for the synths to communicate
> > > >> > > with devices as an alternate for direct serial comms used by the synths
> > > >> > > at the moment. It then migrates some of the synths to the TTY-based
> > > >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > >> > > txprt.
> > > >> >
> > > >> > What about using the serbus code that is now in the tree? That should
> > > >> > make this a lot easier than your patchset from what I can see.
> > > >>
> > > >> Mmm... AIUI from reading tty_port_register_device_attr, one
> > > >> would have to have registered a speakup serdev device driver
> > > >> *before* tty_port_register_device_attr gets called, so that
> > > >> serdev_tty_port_register matches the driver in the loop of
> > > >> of_serdev_register_devices, and no TTY cdev is created?
> > >
> > > Not exactly. The driver doesn't have to be registered nor loaded, but
> > > the device does have to be present so no tty cdev is created. The only
> > > way a device is present currently is when a node is in the DT. I
> > > expect the x86 folks will be adding ACPI support soon.
> >
> > Ok, but in our case there is no hardware device that the system can
> > see/probe, it's just plugged externally.
> >
> > > >> That would mean that speakup can not be loaded as a module after ttyS0
> > > >> initialization, that won't fly for our use needs. The line discipline
> > > >> mechanism allows us to attach ourself to an existing tty. Could we
> > > >> imagine a tty_port function which removes the cdev and tries to register
> > > >> the tty port again to serdev?
> > > >>
> > > >> What we basically need to be able to say on speakup module load is
> > > >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> > > >> discuss with it".
> > > >
> > > > That for_each_available_child_of_node loop is really way more complex
> > > > than what we need. And what's more, it's not working without CONFIG_OF
> > > > (!)
> > > >
> > > > It would really make sense to me to have a
> > > >
> > > > serdev_device *tty_port_register_serdev_device(tty, device)
> > > >
> > > > which unregisters the character device of the tty, and creates instead a
> > > > controler with the given device plugged to it. Really much like a line
> > > > discipline, but way simpler :)
> > >
> > > What would trigger calling this function?
> >
> > From the user point of view, loading the speakup module for the external
> > device, typically, with "ttyS0" as module parameter. Then the speakup
> > init function can do whatever it needs to achieve this :)