Discussion:
Line discipline
Okash Khawaja
2016-11-21 22:32:52 UTC
Permalink
Hi,

Just to be sure, this is what we want for speakup_dummy.

1. register ldisc structure in spk_tty_probe().
2. in spk_tty_probe(), attach line discipline - like you would for
/dev/ttyX from user space
3. in open() method of ldisc structure, take the tty_struct and cache it.
4. in spk_serial_out_tty() call tty->ops->write() instead of outb().

What have a I missed here?

Thanks,
Okash
Samuel Thibault
2016-11-21 22:36:31 UTC
Permalink
Hello,
Post by Okash Khawaja
1. register ldisc structure in spk_tty_probe().
Registering the speakup line discipline should be done on speakup module
insertion (speakup_init), since we're supposed to do it just once.
Post by Okash Khawaja
2. in spk_tty_probe(), attach line discipline - like you would for /dev/ttyX
from user space
spk_tty_probe also needs to open ttySX itsef.
Post by Okash Khawaja
3. in open() method of ldisc structure, take the tty_struct and cache it.
Yes.
Post by Okash Khawaja
4. in spk_serial_out_tty() call tty->ops->write() instead of outb().
Yes.

Samuel
Okash Khawaja
2016-11-23 14:46:59 UTC
Permalink
On Mon, Nov 21, 2016 at 10:36 PM, Samuel Thibault <
Post by Samuel Thibault
Hello,
Post by Okash Khawaja
1. register ldisc structure in spk_tty_probe().
Registering the speakup line discipline should be done on speakup module
insertion (speakup_init), since we're supposed to do it just once.
Post by Okash Khawaja
2. in spk_tty_probe(), attach line discipline - like you would for
/dev/ttyX
Post by Okash Khawaja
from user space
spk_tty_probe also needs to open ttySX itsef.
Post by Okash Khawaja
3. in open() method of ldisc structure, take the tty_struct and cache it.
Yes.
Post by Okash Khawaja
4. in spk_serial_out_tty() call tty->ops->write() instead of outb().
Yes.
Samuel
Thanks.

So I have a simple kernel module that registers a new ldisc. When userspace
attaches that ldisc to /dev/ttyS0, the kernel module is able to write to
that serial port using tty->ops->write().

Currently I'm investigating the core problem of opening /dev/ttyS0 from
kernel space and attaching our ldisc to it. Are there any existing ideas
around this? Is this worth investigating: to find correct tty without
opening /dev/ttyS0 and assigning our ldisc to it. Realistically, how early
in boot process do we want the ldisc assigned?
Samuel Thibault
2016-11-23 14:56:59 UTC
Permalink
Post by Okash Khawaja
So I have a simple kernel module that registers a new ldisc. When userspace
attaches that ldisc to /dev/ttyS0, the kernel module is able to write to that
serial port using tty->ops->write().
Ok, good for a start :)
Post by Okash Khawaja
Currently I'm investigating the core problem of opening /dev/ttyS0 from kernel
space and attaching our ldisc to it. Are there any existing ideas around this?
What I wrote before:


how can speakup open
the port? We don't have a process context or /dev/, so we can't just
use sys_open and alike. What we could use is some function which takes
a minor/major pair or a device name, and returns a filp, then we can
tty_set_ldisc(N_SPEAKUP) on file_tty(filp), but I don't know if such
thing exists? That would probably be building a struct inode (getting
inspired from fs/ramfs/), then just open it? Something like:

struct inode *inode = new_inode(sb);

init_special_inode(inode, S_IFCHR, MKDEV(major, minor));
filp = get_empty_filp();
do_dentry_open(filp, inode, NULL, NULL);
struct tty_struct *tty = file_tty(filp);
tty_set_ldisc(tty, N_SPEAKUP);

Post by Okash Khawaja
Is this worth investigating: to find correct tty without opening /dev/ttyS0 and
assigning our ldisc to it.
Realistically, how early in boot process do we want the ldisc
assigned?
As early as possible :)

That'll mean after tty initialization and after serial driver initialization

Samuel
Okash Khawaja
2016-11-27 20:41:50 UTC
Permalink
Hi,

Thanks for explanation. I have not been able to work on this recently -
pulled away for something urgent. As soon as it's over, I'll be back on it.
Probably third week of December.


Cheers,
Okash

On Wed, Nov 23, 2016 at 2:56 PM, Samuel Thibault <
Post by Okash Khawaja
Post by Okash Khawaja
So I have a simple kernel module that registers a new ldisc. When
userspace
Post by Okash Khawaja
attaches that ldisc to /dev/ttyS0, the kernel module is able to write to
that
Post by Okash Khawaja
serial port using tty->ops->write().
Ok, good for a start :)
Post by Okash Khawaja
Currently I'm investigating the core problem of opening /dev/ttyS0 from
kernel
Post by Okash Khawaja
space and attaching our ldisc to it. Are there any existing ideas around
this?

how can speakup open
the port? We don't have a process context or /dev/, so we can't just
use sys_open and alike. What we could use is some function which takes
a minor/major pair or a device name, and returns a filp, then we can
tty_set_ldisc(N_SPEAKUP) on file_tty(filp), but I don't know if such
thing exists? That would probably be building a struct inode (getting
struct inode *inode = new_inode(sb);
init_special_inode(inode, S_IFCHR, MKDEV(major, minor));
filp = get_empty_filp();
do_dentry_open(filp, inode, NULL, NULL);
struct tty_struct *tty = file_tty(filp);
tty_set_ldisc(tty, N_SPEAKUP);

Post by Okash Khawaja
Is this worth investigating: to find correct tty without opening
/dev/ttyS0 and
Post by Okash Khawaja
assigning our ldisc to it.
Realistically, how early in boot process do we want the ldisc
assigned?
As early as possible :)
That'll mean after tty initialization and after serial driver
initialization
Samuel
Samuel Thibault
2016-12-11 20:22:44 UTC
Permalink
Hello,
Thanks for explanation. I have not been able to work on this recently - pulled
away for something urgent. As soon as it's over, I'll be back on it. Probably
third week of December.
I've taken some time to have a look, I have attached the result:

- tty_NULL is a patch against the kernel to make tty functions accept
being called with filp == NULL
- mymodule.tgz is a dumb module which opens ttyS0 (major 4 minor 64),
and writes to it through the tty write operation.

So this, combined with your work on the line discipline, should get
something working, and opening other serial ports (e.g. ttyUSB0) is a
matter of changing the major/minor pair (e.g. 188, 0).

Samuel
Okash Khawaja
2016-12-11 22:33:57 UTC
Permalink
Hi Samuel,

That's great. Let me get back on this soon as I can.

Thanks,
Okash
Post by Samuel Thibault
Hello,
Thanks for explanation. I have not been able to work on this recently - pulled
away for something urgent. As soon as it's over, I'll be back on it. Probably
third week of December.
- tty_NULL is a patch against the kernel to make tty functions accept
being called with filp == NULL
- mymodule.tgz is a dumb module which opens ttyS0 (major 4 minor 64),
and writes to it through the tty write operation.
So this, combined with your work on the line discipline, should get
something working, and opening other serial ports (e.g. ttyUSB0) is a
matter of changing the major/minor pair (e.g. 188, 0).
Samuel
<mymodule.tgz>
<tty_NULL>
Okash Khawaja
2016-12-16 06:52:46 UTC
Permalink
Also, the exit function speakup_test_cleanup() needs to contain code from
your module's exit function, if it is used as a kernel module, rather than
being built into kernel image.
Hi,
I quickly put together speakup-test.c and compiled it into the kernel
binary. Loading the kernel outputs test messages to ttyS0 and logs the
event in kernel log. "got tty ffff8fcfe74b0000" message appears at time
3.258357. Both speakup-test.c and dmesg.log are attached. N_SPEAKUP is 26
in /include/uapi/linux/tty.h.
Also tested this as kernel module which seemed to work fine too. That
required EXPORT_SYMBOL(tty_set_ldisc); in /drivers/tty/tty_ldisc.c. Will be
able to spend more time over the weekend.
Thanks!
Okash
Post by Okash Khawaja
Hi Samuel,
That's great. Let me get back on this soon as I can.
Thanks,
Okash
Post by Samuel Thibault
Hello,
Post by Okash Khawaja
Thanks for explanation. I have not been able to work on this recently
- pulled
Post by Samuel Thibault
Post by Okash Khawaja
away for something urgent. As soon as it's over, I'll be back on it.
Probably
Post by Samuel Thibault
Post by Okash Khawaja
third week of December.
- tty_NULL is a patch against the kernel to make tty functions accept
being called with filp == NULL
- mymodule.tgz is a dumb module which opens ttyS0 (major 4 minor 64),
and writes to it through the tty write operation.
So this, combined with your work on the line discipline, should get
something working, and opening other serial ports (e.g. ttyUSB0) is a
matter of changing the major/minor pair (e.g. 188, 0).
Samuel
<mymodule.tgz>
<tty_NULL>
Okash Khawaja
2016-12-18 11:24:11 UTC
Permalink
Hi,

Looks like my previous email didn't get through due to large attachment
size.

I've combined your module with my code and created speakup-test.c module (
http://pastebin.com/pXqRgyF1). When compiled into the kernel binary, it is
able to communicate during the boot process. Relevant part of kernel log
reproduced below. I've posted full log here: http://pastebin.com/UqMAktFm

[ 3.382675] GHES: HEST is not enabled!
[ 3.383142] ACPI: Battery Slot [BAT0] (battery present)
[ 3.383161] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[ 3.404736] 00:03: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a
16550A
[ 3.405554] Linux agpgart interface v0.103
[ 3.405616] got tty ffff97ed674b0000
[ 3.405805] speakup_test_open(): tty->name: ttyS0;
tty->ldisc->ops->name: speakup_test_ldisc
[ 3.405894] speakup_test_open(): done writing. rv=9
[ 3.407209] brd: module loaded
[ 3.407722] loop: module loaded
[ 3.407789] ata_piix 0000:00:01.1: version 2.13
[ 3.408060] scsi host0: ata_piix

Comms work fine both as a LKM and when built in.

Now I see three items to address, listed below in no particular order.

1. Supplying major and minor dev numbers, instead of hardcoding.
2. Integrating the changes into speakup_dummy and testing it.
3. Strategy for kernel patch. Do we try to have it accepted? Not sure if
there is a standard way of addressing it.

Thanks,
Okash
Post by Okash Khawaja
Hi,
Just to be sure, this is what we want for speakup_dummy.
1. register ldisc structure in spk_tty_probe().
2. in spk_tty_probe(), attach line discipline - like you would for
/dev/ttyX from user space
3. in open() method of ldisc structure, take the tty_struct and cache it.
4. in spk_serial_out_tty() call tty->ops->write() instead of outb().
What have a I missed here?
Thanks,
Okash
Samuel Thibault
2016-12-20 01:08:36 UTC
Permalink
Post by Okash Khawaja
Now I see three items to address, listed below in no particular order.
1. Supplying major and minor dev numbers, instead of hardcoding.
This could be a mere module parameter string that a function turns into
major/minor. Not a big deal :)
Post by Okash Khawaja
2. Integrating the changes into speakup_dummy and testing it.
Yep!
Post by Okash Khawaja
3. Strategy for kernel patch. Do we try to have it accepted? Not sure if there
is a standard way of addressing it.
It will never be accepted before step 2. is done. In the end we'll want
to get it accepted, sure, but we have to make speakup able to use it
first, otherwise the kernel patch will be rejected.

Thanks!
Samuel
Okash Khawaja
2016-12-26 16:02:23 UTC
Permalink
Thanks. It seems relatively straightforward from here on. Job switch combined with an unexpectedly busy holiday period means that I haven't been able to work on this recently. Things will settle next week when I will be back on it.

Merry Christmas and happy new year!

Okash
Post by Samuel Thibault
Post by Okash Khawaja
Now I see three items to address, listed below in no particular order.
1. Supplying major and minor dev numbers, instead of hardcoding.
This could be a mere module parameter string that a function turns into
major/minor. Not a big deal :)
Post by Okash Khawaja
2. Integrating the changes into speakup_dummy and testing it.
Yep!
Post by Okash Khawaja
3. Strategy for kernel patch. Do we try to have it accepted? Not sure if there
is a standard way of addressing it.
It will never be accepted before step 2. is done. In the end we'll want
to get it accepted, sure, but we have to make speakup able to use it
first, otherwise the kernel patch will be rejected.
Thanks!
Samuel
Okash Khawaja
2017-01-24 06:16:46 UTC
Permalink
Hi Samuel,

Combined the refactor changes with your tty code to test speakup_dummy. It
seems okay except for problem when unloading speakup.ko - it is in use so
can't be unloaded. I have just got this so investigating it.

One question. Using your code, we can obtain tty, cache it and use it
directly for all subsequent writes. Why then use ldisc?

Thanks,
Okash
Post by Okash Khawaja
Thanks. It seems relatively straightforward from here on. Job switch
combined with an unexpectedly busy holiday period means that I haven't been
able to work on this recently. Things will settle next week when I will be
back on it.
Merry Christmas and happy new year!
Okash
Post by Samuel Thibault
Post by Okash Khawaja
Now I see three items to address, listed below in no particular order.
1. Supplying major and minor dev numbers, instead of hardcoding.
This could be a mere module parameter string that a function turns into
major/minor. Not a big deal :)
Post by Okash Khawaja
2. Integrating the changes into speakup_dummy and testing it.
Yep!
Post by Okash Khawaja
3. Strategy for kernel patch. Do we try to have it accepted? Not sure
if there
Post by Samuel Thibault
Post by Okash Khawaja
is a standard way of addressing it.
It will never be accepted before step 2. is done. In the end we'll want
to get it accepted, sure, but we have to make speakup able to use it
first, otherwise the kernel patch will be rejected.
Thanks!
Samuel
Samuel Thibault
2017-01-24 08:30:45 UTC
Permalink
Post by Okash Khawaja
Combined the refactor changes with your tty code to test speakup_dummy. It
seems okay
Good :)
Post by Okash Khawaja
except for problem when unloading speakup.ko - it is in use so can't
be unloaded. I have just got this so investigating it.
I guess it's a release issue on the tty or such. Could you share your
code so we can investigate too?
Post by Okash Khawaja
One question. Using your code, we can obtain tty, cache it and use it directly
for all subsequent writes. Why then use ldisc?
To be able to read characters. There is no "read" operation in the tty.
We'll receive characters through the receive_buf callback.

Samuel
Okash Khawaja
2017-01-24 21:52:26 UTC
Permalink
Hi,

On Tue, Jan 24, 2017 at 8:30 AM, Samuel Thibault
Post by Samuel Thibault
Post by Okash Khawaja
Combined the refactor changes with your tty code to test speakup_dummy. It
seems okay
Good :)
Post by Okash Khawaja
except for problem when unloading speakup.ko - it is in use so can't
be unloaded. I have just got this so investigating it.
I guess it's a release issue on the tty or such. Could you share your
code so we can investigate too?
Post by Okash Khawaja
One question. Using your code, we can obtain tty, cache it and use it directly
for all subsequent writes. Why then use ldisc?
To be able to read characters. There is no "read" operation in the tty.
We'll receive characters through the receive_buf callback.
Thanks for clarification.
Post by Samuel Thibault
Samuel
I have attached the relevant diff. This is applied after the refactor
changes which allow us to use tty version of spk_serial_out(). Also
I've just hacked it together to test speakup_dummy - other synths
won't work after this change. Please not that since I haven't been
able to look into it yet, it might be a trivial error on my part.
Samuel Thibault
2017-01-26 00:18:01 UTC
Permalink
Hello,
Post by Okash Khawaja
except for problem when unloading speakup.ko - it is in use so can't
be unloaded. I have just got this so investigating it.
Very probably a missing thing at speakup_dummy removal time.
+static void speakup_ldisc_close(struct tty_struct *tty)
+{
+ pr_warn("speakup_test_close()\n");
Does it get called at speakup_dummy removal time?
+static void release_ldisc(void)
+{
Does it get called at speakup_dummy removal time?
@@ -421,6 +533,10 @@ void synth_release(void)
struct var_t *var;
unsigned long flags;
+ // okashTODO: maybe speakup_tty should be part of synth.
+
+ release_ldisc();
Mmm, I believe calling release_ldisc() rather belongs to the release
@@ -432,7 +548,7 @@ void synth_release(void)
sysfs_remove_group(speakup_kobj, &synth->attributes);
for (var = synth->vars; var->var_id != MAXVARS; var++)
speakup_unregister_var(var->var_id);
- spk_stop_serial_interrupt();
+// spk_stop_serial_interrupt();
synth->release();
The spk_stop_serial_interrupt() call should be moved into
spk_serial_release, accent_release, dtpc_release, dtlk_release, and
keynote_release, and we'd have another function like spk_serial_release
for the tty case.

Samuel
Okash Khawaja
2017-01-26 08:24:29 UTC
Permalink
Hi,

On Thu, Jan 26, 2017 at 12:18 AM, Samuel Thibault
Post by Samuel Thibault
Hello,
Post by Okash Khawaja
except for problem when unloading speakup.ko - it is in use so can't
be unloaded. I have just got this so investigating it.
Very probably a missing thing at speakup_dummy removal time.
+static void speakup_ldisc_close(struct tty_struct *tty)
+{
+ pr_warn("speakup_test_close()\n");
Does it get called at speakup_dummy removal time?
No
Post by Samuel Thibault
+static void release_ldisc(void)
+{
Does it get called at speakup_dummy removal time?
Yes
Post by Samuel Thibault
@@ -421,6 +533,10 @@ void synth_release(void)
struct var_t *var;
unsigned long flags;
+ // okashTODO: maybe speakup_tty should be part of synth.
+
+ release_ldisc();
Mmm, I believe calling release_ldisc() rather belongs to the release
@@ -432,7 +548,7 @@ void synth_release(void)
sysfs_remove_group(speakup_kobj, &synth->attributes);
for (var = synth->vars; var->var_id != MAXVARS; var++)
speakup_unregister_var(var->var_id);
- spk_stop_serial_interrupt();
+// spk_stop_serial_interrupt();
synth->release();
The spk_stop_serial_interrupt() call should be moved into
spk_serial_release, accent_release, dtpc_release, dtlk_release, and
keynote_release, and we'd have another function like spk_serial_release
for the tty case.
Sure, absolutely. I hacked this together just to test it in action so
it's not final yet :)
Post by Samuel Thibault
Samuel
Samuel Thibault
2017-01-26 08:33:12 UTC
Permalink
Post by Samuel Thibault
+static void speakup_ldisc_close(struct tty_struct *tty)
+{
+ pr_warn("speakup_test_close()\n");
Does it get called at speakup_dummy removal time?
No
Ok. I guess what we are missing is some part of the rest of
tty_release(). We notably seem to be lacking tty_ldisc_release(tty);
between tty_unlock and tty_flush_works.

Samuel
Okash Khawaja
2017-01-28 17:49:01 UTC
Permalink
Hi,

On Thu, Jan 26, 2017 at 8:33 AM, Samuel Thibault
Post by Samuel Thibault
Post by Samuel Thibault
+static void speakup_ldisc_close(struct tty_struct *tty)
+{
+ pr_warn("speakup_test_close()\n");
Does it get called at speakup_dummy removal time?
No
Ok. I guess what we are missing is some part of the rest of
tty_release(). We notably seem to be lacking tty_ldisc_release(tty);
between tty_unlock and tty_flush_works.
Added tty_ldisc_release() which resolved the problem. Also added
tty_ldisc_flush() before tty_ldisc_release(). Wouldn't it be safer to
add those functions before tty_unlock()?

The only output I get from speakup_dummy is:

Speakup
RATE 8
PITCH 8
VOL 8
TONE 8
Dummy found

That's same before and after this ldisc change.

I looked around for tty release code, specifically tty_release() in
drivers/tty/tty_io.c. Not sure if there's something else missing here.
The modules seem to load and unload (multiple times) okay. Kernel log
doesn't show anything odd. I'll still do some more tests and make the
changes you suggested above before sharing the patch.
Post by Samuel Thibault
Samuel
Thanks,
Okash
Samuel Thibault
2017-01-28 17:57:09 UTC
Permalink
Post by Okash Khawaja
Added tty_ldisc_release() which resolved the problem.
Ok, good.
Post by Okash Khawaja
Also added tty_ldisc_flush() before tty_ldisc_release().
I don't think it's really needed since the user is asking for speakup to
go away anyway, but it shouldn't hurt.
Post by Okash Khawaja
Wouldn't it be safer to add those functions before tty_unlock()?
See callers of these, they don't lock the tty.
Post by Okash Khawaja
Speakup
RATE 8
PITCH 8
VOL 8
TONE 8
Dummy found
But are you working on the linux text console?

Samuel
Okash Khawaja
2017-01-28 18:29:03 UTC
Permalink
On Sat, Jan 28, 2017 at 5:57 PM, Samuel Thibault
Post by Samuel Thibault
Post by Okash Khawaja
Added tty_ldisc_release() which resolved the problem.
Ok, good.
Post by Okash Khawaja
Also added tty_ldisc_flush() before tty_ldisc_release().
I don't think it's really needed since the user is asking for speakup to
go away anyway, but it shouldn't hurt.
Post by Okash Khawaja
Wouldn't it be safer to add those functions before tty_unlock()?
See callers of these, they don't lock the tty.
Post by Okash Khawaja
Speakup
RATE 8
PITCH 8
VOL 8
TONE 8
Dummy found
But are you working on the linux text console?
It's SSH into Linux VM which runs speakup_dummy, so running pseudo
terminal. Should be okay?
Post by Samuel Thibault
Samuel
Samuel Thibault
2017-01-28 18:30:39 UTC
Permalink
Post by Okash Khawaja
It's SSH into Linux VM which runs speakup_dummy, so running pseudo
terminal. Should be okay?
Yes, but then don't be surprised that speakup doesn't print anything:
nothing is happening on the linux text console :) You need to log into
the linux text console and do stuff there to get speakup to speak it.

Samuel
Okash Khawaja
2017-01-28 18:56:02 UTC
Permalink
On Sat, Jan 28, 2017 at 6:30 PM, Samuel Thibault
Post by Samuel Thibault
Post by Okash Khawaja
It's SSH into Linux VM which runs speakup_dummy, so running pseudo
terminal. Should be okay?
nothing is happening on the linux text console :) You need to log into
the linux text console and do stuff there to get speakup to speak it.
My mistake. For some reason I thought espeakup was working with my
xterm which also uses pseudo terminal which led me to (misguided)
belief that it should work for SSH connection too. Thanks, it's
looking better now :)
Post by Samuel Thibault
Samuel
Okash Khawaja
2017-02-03 00:02:11 UTC
Permalink
Hi,
Post by Okash Khawaja
On Sat, Jan 28, 2017 at 6:30 PM, Samuel Thibault
Post by Samuel Thibault
Post by Okash Khawaja
It's SSH into Linux VM which runs speakup_dummy, so running pseudo
terminal. Should be okay?
nothing is happening on the linux text console :) You need to log into
the linux text console and do stuff there to get speakup to speak it.
My mistake. For some reason I thought espeakup was working with my
xterm which also uses pseudo terminal which led me to (misguided)
belief that it should work for SSH connection too. Thanks, it's
looking better now :)
Post by Samuel Thibault
Samuel
I have attached the patch which uses tty for speakup_dummy. It applies
without conflicts to 4.10.x kernel code but showed conflicts for
4.8.6. Please note that major and minor device numbers are hard coded
as 4 and 64 inside spk_ttyio_initialise() function. I have changed
names slightly, so there is spk_ttyio_* prefix which identifies
functions related to tty-based comms. The related code is grouped
inside spk_ttyio.c file.
I noticed that `startup` member of synth_dummy instance was set to
SYNTH_START which meant that probe function for dummy won't be called
when built into kernel image. May be I didn't understand it fully, but
for now I have changed to always be the literal 1.
Tested dummy and also soft synth/espeakup for regressions. Both seemed
okay but I didn't fully test them. Any ideas for testing will be
useful.
Thanks,
Okash
Attaching gzipped patch due to attachment size limits on the mailing list.
Samuel Thibault
2017-02-03 00:11:44 UTC
Permalink
Hello,
I have attached the patch which uses tty for speakup_dummy.
It looks good, cool :)
It applies without conflicts to 4.10.x kernel code but showed
conflicts for 4.8.6.
No problem, it'll take some time to get to a point where it is
submittable, so no need to target old kernels.
Please note that major and minor device numbers are hard coded as 4
and 64 inside spk_ttyio_initialise() function.
Sure, that's fine for now.
I have changed names slightly, so there is spk_ttyio_* prefix which
identifies functions related to tty-based comms. The related code is
grouped inside spk_ttyio.c file.
Good :)
I noticed that `startup` member of synth_dummy instance was set to
SYNTH_START which meant that probe function for dummy won't be called
when built into kernel image.
Yes, AIUI that is on purpose, so that one can build a kernel with all
drivers compiled in, and use e.g. the synth=dummy kernel parameter to
make the dummy driver started. So please do not change that part, leave
it as it is.
Tested dummy and also soft synth/espeakup for regressions. Both seemed
okay but I didn't fully test them. Any ideas for testing will be
useful.
From looking at the patch, I wouldn't expect any regression anyway.
Ideally you'd do some daily work like hacking source code, compiling,
etc. with the dummy driver enabled, to exercice speakup in harsh
conditions.

Last but not least, you will need to maintain a patch queue, so that the
whole thing is easy to review. So before developping more, please learn
about quilt, and split what you have into several parts already, which
can be easily reviewed and make sense separately, I'd say five patches
in the patch queue:

- the drivers/tty/*+include/linux/tty.h
- the addition of the serial_out method, ploggued onto spk_serial_out
for all drivers
- the move of spk_stop_serial_interrupt into spk_serial_release
- the addition of spk_ttyio and N_SPEAKUP
- making dummy use ttyio methods instead of serial

The idea is that when applying patches one after the other, everything
should be working fine at each step. Each patch needs a changelog to
explain what it does, see patch submissions on the linux kernel mailing
list for examples of changelogs.

Then you'll be able to insert a patch for the serial_in method, and
another patch that would add a parameter to be parsed to produce
e.g. 4,64 in the ttyS0 case, 188,0 in the ttyUSB0 case, etc. (see
Documentation/devices.txt), and a patch to make other drivers use it.

Samuel
Samuel Thibault
2017-02-03 00:13:55 UTC
Permalink
see patch submissions on the linux kernel mailing list for examples of
changelogs.
And of course Documentation/SubmittingPatches.

But really for now, learn about quilt, so that you get at ease with
changing code in the different patches of your series.

Samuel
Okash Khawaja
2017-02-03 00:20:44 UTC
Permalink
Post by Samuel Thibault
see patch submissions on the linux kernel mailing list for examples of
changelogs.
And of course Documentation/SubmittingPatches.
But really for now, learn about quilt, so that you get at ease with
changing code in the different patches of your series.
Samuel
Thanks all that is very helpful. I'll find out about quilt.
Okash Khawaja
2017-02-08 22:42:06 UTC
Permalink
Post by Okash Khawaja
Post by Samuel Thibault
see patch submissions on the linux kernel mailing list for examples of
changelogs.
And of course Documentation/SubmittingPatches.
But really for now, learn about quilt, so that you get at ease with
changing code in the different patches of your series.
Samuel
Thanks all that is very helpful. I'll find out about quilt.
About to send this out as series of patches from quilt. That still
means major and minor numbers are fixed at 4, 64 and serial_in is
still the same. Those changes will be the next step.

Thanks,
Okash

Samuel Thibault
2017-01-25 00:51:03 UTC
Permalink
Post by Okash Khawaja
Combined the refactor changes with your tty code to test speakup_dummy. It
seems okay except for problem when unloading speakup.ko - it is in use so can't
be unloaded. I have just got this so investigating it.
Just to make sure: it's expected that the speakup module is busy while
speakup_dummy is still loaded, since there is a dependency between the
two, you need to rmmod speakup_dummy before speakup.

Samuel
Okash Khawaja
2017-01-25 05:40:13 UTC
Permalink
Hi,
Post by Samuel Thibault
Post by Okash Khawaja
Combined the refactor changes with your tty code to test speakup_dummy. It
seems okay except for problem when unloading speakup.ko - it is in use so can't
be unloaded. I have just got this so investigating it.
Just to make sure: it's expected that the speakup module is busy while
speakup_dummy is still loaded, since there is a dependency between the
two, you need to rmmod speakup_dummy before speakup.
Samuel
Yes. So I'm using insmod and rmmod instead of modprobe. Here are the steps:

insmod speakup.ko
insmod speakup_dummy.ko
rmmod speakup_dummy
rmmod speakup # this fails with message that speakup is in use

Also speakup module, on it own, can be loaded and removed without problem.
Loading...