Out of tree GPL serial tty driver help?

Out of tree GPL serial tty driver help?

[Mark Hounschell]

I've been sort of maintaining a couple of Digi International serial port 
card (XP and AP) drivers for years now because, well, they just won't do 
it anymore. In any case, I'm moving from a 3.4.x kernel, that works just 
fine, to a 3.8.8 kernel, that does not. I have code that does something 
like this:

     tty_set_operations(&SerialDriver, &SerialOps);
     tty_register_driver(&SerialDriver);
     maxminor = NumBoards * 64;
     for (i = 0; i < maxminor; i++)
         tty_register_device(&SerialDriver, i, NULL);

The first call to tty_register_device barfs:

[   76.528626] BUG: unable to handle kernel NULL pointer dereference at 
   (null)
[   76.528628] IP: [<c02fa460>] cdev_init+0x30/0x90
[   76.528632] *pde = 00000000
[   76.528634] Oops: 0002 [#1] PREEMPT SMP
[   76.528636] Modules linked in: kvm snd_hda_codec_realtek 
snd_hda_intel snd_hda_codec snd_hwdep snd_pcm nvidia(PO) osst snd_seq 
dgdm(O+) snd_timer snd_seq_device eprm(O) st sr_mod sg synclink_gt snd 
crc32c_intel aesni_intel ablk_helper cryptd lrw gpiohsd(O) k10temp 
lcrsmem(O) hdlc aes_i586 3c59x firewire_ohci pcspkr xts gf128mul mxm_wmi 
shpchp soundcore i2c_piix4 firewire_core snd_page_alloc crc_itu_t cdrom 
wmi pci_hotplug r8169 fam15h_power microcode rtom(O) button edd autofs4 
ext4 jbd2 crc16 xhci_hcd scsi_dh_hp_sw scsi_dh_emc scsi_dh_alua 
scsi_dh_rdac scsi_dh ata_generic pata_atiixp aic7xxx aic79xx 
scsi_transport_spi
[   76.528661] Pid: 393, comm: systemd-udevd Tainted: P           O 
3.8.8-lcrs #8 Gigabyte Technology Co., Ltd. GA-990FXA-UD5/GA-990FXA-UD5
[   76.528662] EIP: 0060:[<c02fa460>] EFLAGS: 00010246 CPU: 4
[   76.528664] EIP is at cdev_init+0x30/0x90
[   76.528665] EAX: 00000000 EBX: 00000000 ECX: 0000000f EDX: 0000003c
[   76.528667] ESI: c06d38c0 EDI: 00000000 EBP: ed449d0c ESP: ed449d00
[   76.528668]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   76.528669] CR0: 8005003b CR2: f72f9920 CR3: 2ebc8000 CR4: 000407d0
[   76.528670] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[   76.528672] DR6: ffff0ff0 DR7: 00000400
[   76.528673] Process systemd-udevd (pid: 393, ti=ed448000 
task=ee753240 task.ti=ed448000)
[   76.528674] Stack:
[   76.528675]  fb396220 00000000 04600000 ed449d30 c04780a6 c0826710 
0000003c 00000000
[   76.528678]  00000000 fb396220 00000000 04600000 ed449dac c0478d52 
00000001 ed449d60
[   76.528681]  00000000 00000002 00000000 04600000 00000730 ed449d60 
00000000 00000000
[   76.528684] Call Trace:
[   76.528688]  [<c04780a6>] tty_cdev_add+0x56/0x80
[   76.528690]  [<c0478d52>] tty_register_device_attr+0x192/0x220
[   76.528693]  [<c02fa96d>] ? register_chrdev_region+0x4d/0xa0
[   76.528695]  [<c0478dfa>] tty_register_device+0x1a/0x20
[   76.528702]  [<fb37e701>] dgdm_tty_init+0x601/0x660 [dgdm]
[   76.528707]  [<fb375818>] init_module+0x2008/0x22f0 [dgdm]
[   76.528711]  [<c069d843>] ? notifier_call_chain+0x43/0x60
[   76.528716]  [<fb373810>] ? dgdm_boardread+0x430/0x430 [dgdm]
[   76.528718]  [<c0201222>] do_one_initcall+0x112/0x160
[   76.528721]  [<c0252707>] ? __blocking_notifier_call_chain+0x47/0x60
[   76.528723]  [<c02849d8>] load_module+0x1c58/0x2280
[   76.528729]  [<c028508d>] sys_init_module+0x8d/0xf0
[   76.528734]  [<c069ad1d>] syscall_call+0x7/0xb
[   76.528735] Code: 0c a8 01 89 5d f4 89 c3 89 75 f8 89 d6 ba 3c 00 00 
00 89 7d fc 89 c7 75 52 f7 c7 02 00 00 00 75 5a 89 d1 31 c0 c1 e9 02 f6 
c2 02 <f3> ab 74 08 66 c7 07 00 00 83 c7 02 83 e2 01 74 03 c6 07 00 8d
[   76.528753] EIP: [<c02fa460>] cdev_init+0x30/0x90 SS:ESP 0068:ed449d00
[   76.528756] CR2: 0000000000000000
[   76.528758] ---[ end trace 99a84df60dc687d6 ]---


I've put a printk in tty_io.c that shows the NULL pointer in 
driver->cdevs, that I assume. is what is causing it to barf. The first 4 
below are from Synclink-GT driver. The last is mine.

tty_cdev_add: Entered. driver = 0xeebd9900 driver->cdevs = 0xe678b800 
index = 0 name = ttySLG0
tty_cdev_add: Entered. driver = 0xeebd9900 driver->cdevs = 0xe678b800 
index = 1 name = ttySLG1
tty_cdev_add: Entered. driver = 0xeebd9900 driver->cdevs = 0xe678b800 
index = 2 name = ttySLG2
tty_cdev_add: Entered. driver = 0xeebd9900 driver->cdevs = 0xe678b800 
index = 3 name = ttySLG3
tty_cdev_add: Entered. driver = 0xf6d723a0 driver->cdevs = 0x00000000 
index = 0 name = tty_dgdm_G0

I've tried adding cdev_alloc and cdev_init into the mix but things seem 
to get much worse when I do.

Any tips or pointers would be appreciated.

Thanks in advance
Mark

Re: Out of tree GPL serial tty driver help?

[Peter Hurley]

On Wed, 2013-04-24 at 13:44 -0400, Mark Hounschell wrote:
> I've been sort of maintaining a couple of Digi International serial port 
> card (XP and AP) drivers for years now because, well, they just won't do 
> it anymore. In any case, I'm moving from a 3.4.x kernel, that works just 
> fine, to a 3.8.8 kernel, that does not. I have code that does something 
> like this:
>
>      tty_set_operations(&SerialDriver, &SerialOps);
>      tty_register_driver(&SerialDriver);
>      maxminor = NumBoards * 64;
>      for (i = 0; i < maxminor; i++)
>          tty_register_device(&SerialDriver, i, NULL);

You're correct in diagnosing the problem to cdevs == NULL.
You're missing:

	maxminor = min(num_boards * 64, 256);
	serial_driver = alloc_tty_driver(maxminor);

then,
	/* Fill in pertinent tty_driver fields, esp. */
	serial_driver->flags = TTY_DRIVER_DYNAMIC_DEV;

	tty_set_operations(serial_driver, &serial_ops);
	tty_register_driver(serial_driver);
	for (i = 0; i < maxminor; i++)
		tty_register_device(serial_driver, i, NULL);


Regards,
Peter Hurley

PS - Each board supports 64 individual serial ports??

Re: Out of tree GPL serial tty driver help?

[Mark Hounschell]

On 04/25/2013 05:41 PM, Peter Hurley wrote:
> On Wed, 2013-04-24 at 13:44 -0400, Mark Hounschell wrote:
>> I've been sort of maintaining a couple of Digi International serial port
>> card (XP and AP) drivers for years now because, well, they just won't do
>> it anymore. In any case, I'm moving from a 3.4.x kernel, that works just
>> fine, to a 3.8.8 kernel, that does not. I have code that does something
>> like this:
>>
>>       tty_set_operations(&SerialDriver, &SerialOps);
>>       tty_register_driver(&SerialDriver);
>>       maxminor = NumBoards * 64;
>>       for (i = 0; i < maxminor; i++)
>>           tty_register_device(&SerialDriver, i, NULL);
>
> You're correct in diagnosing the problem to cdevs == NULL.
> You're missing:
>
> 	maxminor = min(num_boards * 64, 256);
> 	serial_driver = alloc_tty_driver(maxminor);
>
> then,
> 	/* Fill in pertinent tty_driver fields, esp. */
> 	serial_driver->flags = TTY_DRIVER_DYNAMIC_DEV;
>
> 	tty_set_operations(serial_driver, &serial_ops);
> 	tty_register_driver(serial_driver);
> 	for (i = 0; i < maxminor; i++)
> 		tty_register_device(serial_driver, i, NULL);
>
>

Thanks for responding Peter.

Earlier in the code they do this:

static struct tty_driver SerialDriver
and things like
SerialDriver.termios = kmalloc((maxminor - 256) * sizeof(TERMIOS *), 
GFP_KERNEL);

So is the above no longer going to work and I _must_ now use 
alloc_tty_driver? If alloc_tty_driver is now a requirement, how much is 
it going to do for me? There are several things like the termios above 
that are manually allocated. How much if any of this is alloc_tty_driver 
going to do for me?

or might this work

static struct tty_driver SerialDriver
.
.
.
  	serial_driver.flags = TTY_DRIVER_DYNAMIC_DEV;

         SerialDriver.cdevs  = kcalloc(maxminor, 
sizeof(SerialDriver.cdevs), GFP_KERNEL);

  	tty_set_operations(&serial_driver, &serial_ops);
  	tty_register_driver(&serial_driver);
  	for (i = 0; i < maxminor; i++)
  		tty_register_device(&serial_driver, i, NULL);

???

>
> PS - Each board supports 64 individual serial ports??

No, this particular card comes in 4, 8, and 16 port flavors. I never did 
understand why they create so many device entries. I just figured they 
had a reason. For a single card, no matter how many ports, they create 
64 normal serial tty entries (tty_dgdm_G0 - tty_dgdm_G63), 64 serial 
printer entries (lp_dgdm_G0 - lp_dgdm_G63), and then 64 serial modem 
entries (cu_dgdm_G0 - cu_dgdm_G63). Don't know why.


Thanks again
Mark

Re: Out of tree GPL serial tty driver help?

[Peter Hurley]

On Fri, 2013-04-26 at 08:58 -0400, Mark Hounschell wrote:
> On 04/25/2013 05:41 PM, Peter Hurley wrote:
> > On Wed, 2013-04-24 at 13:44 -0400, Mark Hounschell wrote:
> >> I've been sort of maintaining a couple of Digi International serial port
> >> card (XP and AP) drivers for years now because, well, they just won't do
> >> it anymore. In any case, I'm moving from a 3.4.x kernel, that works just
> >> fine, to a 3.8.8 kernel, that does not. I have code that does something
> >> like this:
> >>
> >>       tty_set_operations(&SerialDriver, &SerialOps);
> >>       tty_register_driver(&SerialDriver);
> >>       maxminor = NumBoards * 64;
> >>       for (i = 0; i < maxminor; i++)
> >>           tty_register_device(&SerialDriver, i, NULL);
> >
> > You're correct in diagnosing the problem to cdevs == NULL.
> > You're missing:
> >
> > 	maxminor = min(num_boards * 64, 256);
> > 	serial_driver = alloc_tty_driver(maxminor);
> >
> > then,
> > 	/* Fill in pertinent tty_driver fields, esp. */
> > 	serial_driver->flags = TTY_DRIVER_DYNAMIC_DEV;
> >
> > 	tty_set_operations(serial_driver, &serial_ops);
> > 	tty_register_driver(serial_driver);
> > 	for (i = 0; i < maxminor; i++)
> > 		tty_register_device(serial_driver, i, NULL);
> >
> >
> 
> Thanks for responding Peter.
> 
> Earlier in the code they do this:
> 
> static struct tty_driver SerialDriver
> and things like
> SerialDriver.termios = kmalloc((maxminor - 256) * sizeof(TERMIOS *), 
                                  ^^^^^^^^^^^^^^
                                is this a transcription error?

> GFP_KERNEL);
> 
> So is the above no longer going to work and I _must_ now use 
> alloc_tty_driver?

Not required but functionally equivalent. alloc_tty_driver() is actually
a wrapper macro which calls __tty_alloc_driver(). You can verify your
driver behavior against that function, if you want.

> If alloc_tty_driver is now a requirement, how much is 
> it going to do for me? There are several things like the termios above 
> that are manually allocated. How much if any of this is alloc_tty_driver 
> going to do for me?

I can't answer this because I don't know what else your open-coded
method is doing.

> or might this work
> 
> static struct tty_driver SerialDriver
> .
> .
> .
>   	serial_driver.flags = TTY_DRIVER_DYNAMIC_DEV;
> 
>          SerialDriver.cdevs  = kcalloc(maxminor, 
> sizeof(SerialDriver.cdevs), GFP_KERNEL);
> 
>   	tty_set_operations(&serial_driver, &serial_ops);
>   	tty_register_driver(&serial_driver);
>   	for (i = 0; i < maxminor; i++)
>   		tty_register_device(&serial_driver, i, NULL);
> 
> ???
> 
> >
> > PS - Each board supports 64 individual serial ports??
> 
> No, this particular card comes in 4, 8, and 16 port flavors. I never did 
> understand why they create so many device entries. I just figured they 
> had a reason. For a single card, no matter how many ports, they create 
> 64 normal serial tty entries (tty_dgdm_G0 - tty_dgdm_G63), 64 serial 
> printer entries (lp_dgdm_G0 - lp_dgdm_G63), and then 64 serial modem 
> entries (cu_dgdm_G0 - cu_dgdm_G63). Don't know why.

So where does i/o go for tty_dgdm_G16?

Also, what host bus are these cards for?

Regards,
Peter Hurley

Re: Out of tree GPL serial tty driver help?

[Mark Hounschell]

On 04/26/2013 09:45 AM, Peter Hurley wrote:
> On Fri, 2013-04-26 at 08:58 -0400, Mark Hounschell wrote:
>> On 04/25/2013 05:41 PM, Peter Hurley wrote:
>>> On Wed, 2013-04-24 at 13:44 -0400, Mark Hounschell wrote:
>>>> I've been sort of maintaining a couple of Digi International serial port
>>>> card (XP and AP) drivers for years now because, well, they just won't do
>>>> it anymore. In any case, I'm moving from a 3.4.x kernel, that works just
>>>> fine, to a 3.8.8 kernel, that does not. I have code that does something
>>>> like this:
>>>>
>>>>        tty_set_operations(&SerialDriver, &SerialOps);
>>>>        tty_register_driver(&SerialDriver);
>>>>        maxminor = NumBoards * 64;
>>>>        for (i = 0; i < maxminor; i++)
>>>>            tty_register_device(&SerialDriver, i, NULL);
>>>
>>> You're correct in diagnosing the problem to cdevs == NULL.
>>> You're missing:
>>>
>>> 	maxminor = min(num_boards * 64, 256);
>>> 	serial_driver = alloc_tty_driver(maxminor);
>>>
>>> then,
>>> 	/* Fill in pertinent tty_driver fields, esp. */
>>> 	serial_driver->flags = TTY_DRIVER_DYNAMIC_DEV;
>>>
>>> 	tty_set_operations(serial_driver, &serial_ops);
>>> 	tty_register_driver(serial_driver);
>>> 	for (i = 0; i < maxminor; i++)
>>> 		tty_register_device(serial_driver, i, NULL);
>>>
>>>
>>
>> Thanks for responding Peter.
>>
>> Earlier in the code they do this:
>>
>> static struct tty_driver SerialDriver
>> and things like
>> SerialDriver.termios = kmalloc((maxminor - 256) * sizeof(TERMIOS *),
>                                    ^^^^^^^^^^^^^^
>                                  is this a transcription error?

Yes, sorry. (MIN(maxminor,256) * sizeof(TERMIOS *)

>
>> GFP_KERNEL);
>>
>> So is the above no longer going to work and I _must_ now use
>> alloc_tty_driver?
>
> Not required but functionally equivalent. alloc_tty_driver() is actually
> a wrapper macro which calls __tty_alloc_driver(). You can verify your
> driver behavior against that function, if you want.
>
>> If alloc_tty_driver is now a requirement, how much is
>> it going to do for me? There are several things like the termios above
>> that are manually allocated. How much if any of this is alloc_tty_driver
>> going to do for me?
>
> I can't answer this because I don't know what else your open-coded
> method is doing.
>

Looking at __tty_alloc_driver, it looks as though only driver, cdevs and 
ports are provided.

>> or might this work
>>
>> static struct tty_driver SerialDriver
>> .
>> .
>> .
>>    	serial_driver.flags = TTY_DRIVER_DYNAMIC_DEV;
>>
>>           SerialDriver.cdevs  = kcalloc(maxminor,
>> sizeof(SerialDriver.cdevs), GFP_KERNEL);

So might something like the above for cdevs and ports get me where I 
need to be?

>>
>>    	tty_set_operations(&serial_driver, &serial_ops);
>>    	tty_register_driver(&serial_driver);
>>    	for (i = 0; i < maxminor; i++)
>>    		tty_register_device(&serial_driver, i, NULL);
>>
>> ???
>>
>>>
>>> PS - Each board supports 64 individual serial ports??
>>
>> No, this particular card comes in 4, 8, and 16 port flavors. I never did
>> understand why they create so many device entries. I just figured they
>> had a reason. For a single card, no matter how many ports, they create
>> 64 normal serial tty entries (tty_dgdm_G0 - tty_dgdm_G63), 64 serial
>> printer entries (lp_dgdm_G0 - lp_dgdm_G63), and then 64 serial modem
>> entries (cu_dgdm_G0 - cu_dgdm_G63). Don't know why.
>
> So where does i/o go for tty_dgdm_G16?
>

I have no idea. I do know that G0 - G7 are the ones I actually use for 
ports 0-7 of an 8 port card-0, and
G64 - G71 are the ones I use for ports 0-7 of an 8 port card-1. I 
remember long ago I had some doc explaining this but can't seem to find 
it now.

> Also, what host bus are these cards for?
>

This particular driver and one other are normal PCI based cards. I also 
have one other that is PCI-e. This particular one is the only one that 
creates all these weird device entries. The other 2 are pretty straight 
forward. One entry per port. I figured I'd tackle this one first.

Regards
Mark

Re: Out of tree GPL serial tty driver help?

[Greg KH]

On Fri, Apr 26, 2013 at 10:28:01AM -0400, Mark Hounschell wrote:
> This particular driver and one other are normal PCI based cards. I
> also have one other that is PCI-e. This particular one is the only
> one that creates all these weird device entries. The other 2 are
> pretty straight forward. One entry per port. I figured I'd tackle
> this one first.

I would remove those other "weird" device entries from the driver, we
removed those types of device nodes from the kernel tree many many years
ago.  Just stick to one device node per port, and all should be fine.

Also, any reason why these drivers aren't in the main kernel tree?  If
they were, all of this work would have been done for you :)

thanks,

greg k-h

Re: Out of tree GPL serial tty driver help?

[Mark Hounschell]

On 04/26/2013 10:35 AM, Greg KH wrote:
> On Fri, Apr 26, 2013 at 10:28:01AM -0400, Mark Hounschell wrote:
>> This particular driver and one other are normal PCI based cards. I
>> also have one other that is PCI-e. This particular one is the only
>> one that creates all these weird device entries. The other 2 are
>> pretty straight forward. One entry per port. I figured I'd tackle
>> this one first.
>
> I would remove those other "weird" device entries from the driver, we
> removed those types of device nodes from the kernel tree many many years
> ago.  Just stick to one device node per port, and all should be fine.
>

Ya, you are certainly right there. First things first though.

> Also, any reason why these drivers aren't in the main kernel tree?  If
> they were, all of this work would have been done for you :)
>

Yes, I wish they were in the kernel. I've had to maintain them since the 
2.6 days because Digi just won't do it and their cards do what we need 
quite well and are still current products. Ya, you can buy a brand new 
one but the latest kernel supported is 2.6. I think Digi at one point 
long ago may have tried the "in kernel" route but you probably know more 
about that than I do.

Since 2.6, the maintenance on these hasn't been to bad, until udev came 
along. But I guess after 3.4 the tty layer is going through quite some 
changes that I have yet to understand.

Actually, we also have 3-4 of our own "out of kernel" GPL drivers that I 
would love to see in the kernel. They are mature and only require 
changes when something in the kernel changes. That's another story though.

Regards
Mark

Re: Out of tree GPL serial tty driver help?

[Greg KH]

On Fri, Apr 26, 2013 at 11:10:14AM -0400, Mark Hounschell wrote:
> >Also, any reason why these drivers aren't in the main kernel tree?  If
> >they were, all of this work would have been done for you :)
> >
> 
> Yes, I wish they were in the kernel. I've had to maintain them since
> the 2.6 days because Digi just won't do it and their cards do what
> we need quite well and are still current products. Ya, you can buy a
> brand new one but the latest kernel supported is 2.6. I think Digi
> at one point long ago may have tried the "in kernel" route but you
> probably know more about that than I do.
> 
> Since 2.6, the maintenance on these hasn't been to bad, until udev
> came along. But I guess after 3.4 the tty layer is going through
> quite some changes that I have yet to understand.

And there will be lots more changes happening to it in the future.  Why
not submit the drivers now (after you get them working)?

> Actually, we also have 3-4 of our own "out of kernel" GPL drivers
> that I would love to see in the kernel. They are mature and only
> require changes when something in the kernel changes. That's another
> story though.

Why not submit those as well?  What's keeping you from doing this?

thanks,

greg k-h

Re: Out of tree GPL serial tty driver help?

[Peter Hurley]

On Fri, 2013-04-26 at 08:19 -0700, Greg KH wrote:
> On Fri, Apr 26, 2013 at 11:10:14AM -0400, Mark Hounschell wrote:
> > >Also, any reason why these drivers aren't in the main kernel tree?  If
> > >they were, all of this work would have been done for you :)
> > >
> > 
> > Yes, I wish they were in the kernel. I've had to maintain them since
> > the 2.6 days because Digi just won't do it and their cards do what
> > we need quite well and are still current products. Ya, you can buy a
> > brand new one but the latest kernel supported is 2.6. I think Digi
> > at one point long ago may have tried the "in kernel" route but you
> > probably know more about that than I do.
> > 
> > Since 2.6, the maintenance on these hasn't been to bad, until udev
> > came along. But I guess after 3.4 the tty layer is going through
> > quite some changes that I have yet to understand.
> 
> And there will be lots more changes happening to it in the future.  Why
> not submit the drivers now (after you get them working)?

Probably not Mark's (or Compro's) code to submit.

Re: Out of tree GPL serial tty driver help?

[Greg KH]

On Fri, Apr 26, 2013 at 11:39:25AM -0400, Peter Hurley wrote:
> On Fri, 2013-04-26 at 08:19 -0700, Greg KH wrote:
> > On Fri, Apr 26, 2013 at 11:10:14AM -0400, Mark Hounschell wrote:
> > > >Also, any reason why these drivers aren't in the main kernel tree?  If
> > > >they were, all of this work would have been done for you :)
> > > >
> > > 
> > > Yes, I wish they were in the kernel. I've had to maintain them since
> > > the 2.6 days because Digi just won't do it and their cards do what
> > > we need quite well and are still current products. Ya, you can buy a
> > > brand new one but the latest kernel supported is 2.6. I think Digi
> > > at one point long ago may have tried the "in kernel" route but you
> > > probably know more about that than I do.
> > > 
> > > Since 2.6, the maintenance on these hasn't been to bad, until udev
> > > came along. But I guess after 3.4 the tty layer is going through
> > > quite some changes that I have yet to understand.
> > 
> > And there will be lots more changes happening to it in the future.  Why
> > not submit the drivers now (after you get them working)?
> 
> Probably not Mark's (or Compro's) code to submit.

If it's a GPL driver, anyone can submit it.

However, if the original copyright owner doesn't want it to be
submitted, we will be glad to take that under consideration.

Mark, what's the reason this has never been submitted?

thanks,

greg k-h

Re: Out of tree GPL serial tty driver help?

[Peter Hurley]

On Fri, 2013-04-26 at 10:28 -0400, Mark Hounschell wrote:
> On 04/26/2013 09:45 AM, Peter Hurley wrote:
> > On Fri, 2013-04-26 at 08:58 -0400, Mark Hounschell wrote:
> >> On 04/25/2013 05:41 PM, Peter Hurley wrote:
> >>> On Wed, 2013-04-24 at 13:44 -0400, Mark Hounschell wrote:
> >>>> I've been sort of maintaining a couple of Digi International serial port
> >>>> card (XP and AP) drivers for years now because, well, they just won't do
> >>>> it anymore. In any case, I'm moving from a 3.4.x kernel, that works just
> >>>> fine, to a 3.8.8 kernel, that does not. I have code that does something
> >>>> like this:
> >>>>
> >>>>        tty_set_operations(&SerialDriver, &SerialOps);
> >>>>        tty_register_driver(&SerialDriver);
> >>>>        maxminor = NumBoards * 64;
> >>>>        for (i = 0; i < maxminor; i++)
> >>>>            tty_register_device(&SerialDriver, i, NULL);
> >>>
> >>> You're correct in diagnosing the problem to cdevs == NULL.
> >>> You're missing:
> >>>
> >>> 	maxminor = min(num_boards * 64, 256);
> >>> 	serial_driver = alloc_tty_driver(maxminor);
> >>>
> >>> then,
> >>> 	/* Fill in pertinent tty_driver fields, esp. */
> >>> 	serial_driver->flags = TTY_DRIVER_DYNAMIC_DEV;
> >>>
> >>> 	tty_set_operations(serial_driver, &serial_ops);
> >>> 	tty_register_driver(serial_driver);
> >>> 	for (i = 0; i < maxminor; i++)
> >>> 		tty_register_device(serial_driver, i, NULL);
> >>>
> >>>
> >>
> >> Thanks for responding Peter.
> >>
> >> Earlier in the code they do this:
> >>
> >> static struct tty_driver SerialDriver
> >> and things like
> >> SerialDriver.termios = kmalloc((maxminor - 256) * sizeof(TERMIOS *),
> >                                    ^^^^^^^^^^^^^^
> >                                  is this a transcription error?
> 
> Yes, sorry. (MIN(maxminor,256) * sizeof(TERMIOS *)

Ok.

> >
> >> GFP_KERNEL);
> >>
>    >> So is the above no longer going to work and I _must_ now use
> >> alloc_tty_driver?
> >
> > Not required but functionally equivalent. alloc_tty_driver() is actually
> > a wrapper macro which calls __tty_alloc_driver(). You can verify your
> > driver behavior against that function, if you want.
> >
> >> If alloc_tty_driver is now a requirement, how much is
> >> it going to do for me? There are several things like the termios above
> >> that are manually allocated. How much if any of this is alloc_tty_driver
> >> going to do for me?
> >
> > I can't answer this because I don't know what else your open-coded
> > method is doing.
> >
> 
> Looking at __tty_alloc_driver, it looks as though only driver, cdevs and 
> ports are provided.
> 
> >> or might this work
> >>
> >> static struct tty_driver SerialDriver
> >> .
> >> .
> >> .
> >>    	serial_driver.flags = TTY_DRIVER_DYNAMIC_DEV;
> >>
> >>           SerialDriver.cdevs  = kcalloc(maxminor,
> >> sizeof(SerialDriver.cdevs), GFP_KERNEL);
> 
> So might something like the above for cdevs and ports get me where I 
> need to be?
> 
> >>
> >>    	tty_set_operations(&serial_driver, &serial_ops);
> >>    	tty_register_driver(&serial_driver);
> >>    	for (i = 0; i < maxminor; i++)
> >>    		tty_register_device(&serial_driver, i, NULL);
> >>
> >> ???
> >>
> >>>
> >>> PS - Each board supports 64 individual serial ports??
> >>
> >> No, this particular card comes in 4, 8, and 16 port flavors. I never did
> >> understand why they create so many device entries. I just figured they
> >> had a reason. For a single card, no matter how many ports, they create
> >> 64 normal serial tty entries (tty_dgdm_G0 - tty_dgdm_G63), 64 serial
> >> printer entries (lp_dgdm_G0 - lp_dgdm_G63), and then 64 serial modem
> >> entries (cu_dgdm_G0 - cu_dgdm_G63). Don't know why.
> >
> > So where does i/o go for tty_dgdm_G16?
> >
> 
> I have no idea. I do know that G0 - G7 are the ones I actually use for 
> ports 0-7 of an 8 port card-0, and
> G64 - G71 are the ones I use for ports 0-7 of an 8 port card-1. I 
> remember long ago I had some doc explaining this but can't seem to find 
> it now.
> 
> > Also, what host bus are these cards for?
> >
> 
> This particular driver and one other are normal PCI based cards. I also 
> have one other that is PCI-e. This particular one is the only one that 
> creates all these weird device entries. The other 2 are pretty straight 
> forward. One entry per port. I figured I'd tackle this one first.

Ok. Looking at Digi's website, I see they have external port
concentrators. That would explain the fixed 64-port allocation (although
that's not really how to do it).

These drivers weren't really current at 3.4 though, either. I'm not sure
what else you're going to find that doesn't work.

For both PCI and PCI-e, these drivers should _at a minimum_ be pci
drivers that register the tty driver at module init and register _only_
the tty devices for that particular PCI device at PCI probe time. Look
at the end of synclink_gt.c for how this is supposed to look.

Good luck,
Peter Hurley

Re: Out of tree GPL serial tty driver help?

[Mark Hounschell]

On 04/26/2013 12:03 PM, Greg KH wrote:
> On Fri, Apr 26, 2013 at 11:39:25AM -0400, Peter Hurley wrote:
>> On Fri, 2013-04-26 at 08:19 -0700, Greg KH wrote:
>>> On Fri, Apr 26, 2013 at 11:10:14AM -0400, Mark Hounschell wrote:
>>>>> Also, any reason why these drivers aren't in the main kernel tree?  If
>>>>> they were, all of this work would have been done for you :)
>>>>>
>>>>
>>>> Yes, I wish they were in the kernel. I've had to maintain them since
>>>> the 2.6 days because Digi just won't do it and their cards do what
>>>> we need quite well and are still current products. Ya, you can buy a
>>>> brand new one but the latest kernel supported is 2.6. I think Digi
>>>> at one point long ago may have tried the "in kernel" route but you
>>>> probably know more about that than I do.
>>>>
>>>> Since 2.6, the maintenance on these hasn't been to bad, until udev
>>>> came along. But I guess after 3.4 the tty layer is going through
>>>> quite some changes that I have yet to understand.
>>>
>>> And there will be lots more changes happening to it in the future.  Why
>>> not submit the drivers now (after you get them working)?
>>
>> Probably not Mark's (or Compro's) code to submit.
>
> If it's a GPL driver, anyone can submit it.
>
> However, if the original copyright owner doesn't want it to be
> submitted, we will be glad to take that under consideration.
>
> Mark, what's the reason this has never been submitted?
>

Greg,

As for the Digi card driver/drivers, I can't say why not. There is a 
remnant of the actual one this thread is pertaining to in 
./Documentation/serial/digiepca.txt. I've never submitted it or any of 
their others I have to maintain because I didn't know I could nor would 
know how to start. I thought that Digi would have had to do it.

As far as Compro's drivers go, I think we have just been a little 
wary/unknowing of what it would take and what the results of the effort 
would actually be. Would they still do what _we_ needed them to do? Are 
we kernel savvy enough to get them to a state you would accept? etc... 
We do understand that if they were in the kernel that the best qualified 
people would be the ones that "kept them current" with the latest kernel 
changes. It's certainly something that no one here would object to 
doing. Are you the man to talk to? What is the actual process?


Regards
Mark

Re: Out of tree GPL serial tty driver help?

[Mark Hounschell]

On 04/26/2013 12:37 PM, Peter Hurley wrote:
>
> On Fri, 2013-04-26 at 10:28 -0400, Mark Hounschell wrote:
>> On 04/26/2013 09:45 AM, Peter Hurley wrote:
>>> On Fri, 2013-04-26 at 08:58 -0400, Mark Hounschell wrote:
>>>> On 04/25/2013 05:41 PM, Peter Hurley wrote:
>>>>> On Wed, 2013-04-24 at 13:44 -0400, Mark Hounschell wrote:
>>>>>> I've been sort of maintaining a couple of Digi International serial port
>>>>>> card (XP and AP) drivers for years now because, well, they just won't do
>>>>>> it anymore. In any case, I'm moving from a 3.4.x kernel, that works just
>>>>>> fine, to a 3.8.8 kernel, that does not. I have code that does something
>>>>>> like this:
>>>>>>
>>>>>>         tty_set_operations(&SerialDriver, &SerialOps);
>>>>>>         tty_register_driver(&SerialDriver);
>>>>>>         maxminor = NumBoards * 64;
>>>>>>         for (i = 0; i < maxminor; i++)
>>>>>>             tty_register_device(&SerialDriver, i, NULL);
>>>>>
>>>>> You're correct in diagnosing the problem to cdevs == NULL.
>>>>> You're missing:
>>>>>
>>>>> 	maxminor = min(num_boards * 64, 256);
>>>>> 	serial_driver = alloc_tty_driver(maxminor);
>>>>>
>>>>> then,
>>>>> 	/* Fill in pertinent tty_driver fields, esp. */
>>>>> 	serial_driver->flags = TTY_DRIVER_DYNAMIC_DEV;
>>>>>
>>>>> 	tty_set_operations(serial_driver, &serial_ops);
>>>>> 	tty_register_driver(serial_driver);
>>>>> 	for (i = 0; i < maxminor; i++)
>>>>> 		tty_register_device(serial_driver, i, NULL);
>>>>>
>>>>>
>>>>
>>>> Thanks for responding Peter.
>>>>
>>>> Earlier in the code they do this:
>>>>
>>>> static struct tty_driver SerialDriver
>>>> and things like
>>>> SerialDriver.termios = kmalloc((maxminor - 256) * sizeof(TERMIOS *),
>>>                                     ^^^^^^^^^^^^^^
>>>                                   is this a transcription error?
>>
>> Yes, sorry. (MIN(maxminor,256) * sizeof(TERMIOS *)
>
> Ok.
>
>>>
>>>> GFP_KERNEL);
>>>>
>>     >> So is the above no longer going to work and I _must_ now use
>>>> alloc_tty_driver?
>>>
>>> Not required but functionally equivalent. alloc_tty_driver() is actually
>>> a wrapper macro which calls __tty_alloc_driver(). You can verify your
>>> driver behavior against that function, if you want.
>>>
>>>> If alloc_tty_driver is now a requirement, how much is
>>>> it going to do for me? There are several things like the termios above
>>>> that are manually allocated. How much if any of this is alloc_tty_driver
>>>> going to do for me?
>>>
>>> I can't answer this because I don't know what else your open-coded
>>> method is doing.
>>>
>>
>> Looking at __tty_alloc_driver, it looks as though only driver, cdevs and
>> ports are provided.
>>
>>>> or might this work
>>>>
>>>> static struct tty_driver SerialDriver
>>>> .
>>>> .
>>>> .
>>>>     	serial_driver.flags = TTY_DRIVER_DYNAMIC_DEV;
>>>>
>>>>            SerialDriver.cdevs  = kcalloc(maxminor,
>>>> sizeof(SerialDriver.cdevs), GFP_KERNEL);
>>
>> So might something like the above for cdevs and ports get me where I
>> need to be?
>>
>>>>
>>>>     	tty_set_operations(&serial_driver, &serial_ops);
>>>>     	tty_register_driver(&serial_driver);
>>>>     	for (i = 0; i < maxminor; i++)
>>>>     		tty_register_device(&serial_driver, i, NULL);
>>>>
>>>> ???
>>>>
>>>>>
>>>>> PS - Each board supports 64 individual serial ports??
>>>>
>>>> No, this particular card comes in 4, 8, and 16 port flavors. I never did
>>>> understand why they create so many device entries. I just figured they
>>>> had a reason. For a single card, no matter how many ports, they create
>>>> 64 normal serial tty entries (tty_dgdm_G0 - tty_dgdm_G63), 64 serial
>>>> printer entries (lp_dgdm_G0 - lp_dgdm_G63), and then 64 serial modem
>>>> entries (cu_dgdm_G0 - cu_dgdm_G63). Don't know why.
>>>
>>> So where does i/o go for tty_dgdm_G16?
>>>
>>
>> I have no idea. I do know that G0 - G7 are the ones I actually use for
>> ports 0-7 of an 8 port card-0, and
>> G64 - G71 are the ones I use for ports 0-7 of an 8 port card-1. I
>> remember long ago I had some doc explaining this but can't seem to find
>> it now.
>>
>>> Also, what host bus are these cards for?
>>>
>>
>> This particular driver and one other are normal PCI based cards. I also
>> have one other that is PCI-e. This particular one is the only one that
>> creates all these weird device entries. The other 2 are pretty straight
>> forward. One entry per port. I figured I'd tackle this one first.
>
> Ok. Looking at Digi's website, I see they have external port
> concentrators. That would explain the fixed 64-port allocation (although
> that's not really how to do it).
>
> These drivers weren't really current at 3.4 though, either. I'm not sure
> what else you're going to find that doesn't work.
>

No, I have kept them current, or I should say functional, to the best of 
my ability from 2.6 up to and including 3.4.x. What I have here works 
with kernels up to 3.4.x. I have not tried anything between 3.4 and 3.8. 
  As far as building against 3.8, the only issues were the change from 
*termios to termios and the "structn_tty_data" no longer in an include 
file so not easily directly accessible.

> For both PCI and PCI-e, these drivers should _at a minimum_ be pci
> drivers that register the tty driver at module init and register _only_
> the tty devices for that particular PCI device at PCI probe time. Look
> at the end of synclink_gt.c for how this is supposed to look.
>

I'll look at it some more but I have been there.

mark

Re: Out of tree GPL serial tty driver help?

[Peter Hurley]

On Fri, 2013-04-26 at 14:17 -0400, Mark Hounschell wrote:
> On 04/26/2013 12:37 PM, Peter Hurley wrote:
> > These drivers weren't really current at 3.4 though, either. I'm not sure
> > what else you're going to find that doesn't work.
> >
> 
> No, I have kept them current, or I should say functional, to the best of 
> my ability from 2.6 up to and including 3.4.x.

By 'current', I mean 'similar in structure and functionality to in-tree
drivers'.

The structure of this driver is more akin to a 2.5 driver (back when
there were separate serial and callout tty drivers).

>  What I have here works 
> with kernels up to 3.4.x. I have not tried anything between 3.4 and 3.8. 
>   As far as building against 3.8, the only issues were the change from 
> *termios to termios and the "structn_tty_data" no longer in an include 
> file so not easily directly accessible.

What is this driver accessing in N_TTY's private data?

> > For both PCI and PCI-e, these drivers should _at a minimum_ be pci
> > drivers that register the tty driver at module init and register _only_
> > the tty devices for that particular PCI device at PCI probe time. Look
> > at the end of synclink_gt.c for how this is supposed to look.
> >
> 
> I'll look at it some more but I have been there.

Specifically, review:
	struct pci_driver
	device_init()
	init_one()
	remove_one()
	sglt_init()
	sglt_exit()

Regards,
Peter Hurley

Re: Out of tree GPL serial tty driver help?

[Mark Hounschell]

On 04/26/2013 03:51 PM, Peter Hurley wrote:
> On Fri, 2013-04-26 at 14:17 -0400, Mark Hounschell wrote:
>> On 04/26/2013 12:37 PM, Peter Hurley wrote:
>>> These drivers weren't really current at 3.4 though, either. I'm not sure
>>> what else you're going to find that doesn't work.
>>>
>>
>> No, I have kept them current, or I should say functional, to the best of
>> my ability from 2.6 up to and including 3.4.x.
>
> By 'current', I mean 'similar in structure and functionality to in-tree
> drivers'.
>
> The structure of this driver is more akin to a 2.5 driver (back when
> there were separate serial and callout tty drivers).
>
>>   What I have here works
>> with kernels up to 3.4.x. I have not tried anything between 3.4 and 3.8.
>>    As far as building against 3.8, the only issues were the change from
>> *termios to termios and the "structn_tty_data" no longer in an include
>> file so not easily directly accessible.
>
> What is this driver accessing in N_TTY's private data?
>
         /* Decide how much data we can send into the tty layer */
         if(dgdm_rawreadok && tty->real_raw)
                 flip_len = MYFLIPLEN;
         else
                 flip_len = TTY_FLIPBUF_SIZE;
         len = MIN(data_len, flip_len);
         len = MIN(len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);
         dxb_return_fixed_cost(dxb, channel, 1);
         ld = tty_ldisc_ref(tty);
.
.
.
.
                 /*
                  * In high performance mode, we don't have to update
                  * flag_buf or any of the counts or pointers into flip buf.
                  */
                 if(!dgdm_rawreadok || !tty->real_raw) {
                         if (I_PARMRK(tty) || I_BRKINT(tty) || 
I_INPCK(tty)) {
                                 parity_scan(dc, myflipbuf[boardnum],
                                         myflipflagbuf[boardnum], &len);
                         } else {
                                 memset(myflipflagbuf[boardnum], 
TTY_NORMAL, len);
                         }
                 }
.
.
.
.
                 /*
                  * If we're doing raw reads, jam it right into the
                  * line disc bypassing the flip buffers.
                  * OPT OPP: do this only once per channel instead of
                  * once per dxb message.
                  */
                 if(dgdm_rawreadok && tty->real_raw) {
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,27)
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30)
                         tty->ldisc->ops->receive_buf(tty, 
myflipbuf[boardnum], NULL, len);
#else
                         tty->ldisc.ops->receive_buf(tty, 
myflipbuf[boardnum], NULL, len);
#endif
#else
                         tty->ldisc.receive_buf(tty, 
myflipbuf[boardnum], NULL, len);
#endif
                 } else {
                         len = tty_buffer_request_room(tty, len);
                         tty_insert_flip_string_flags(tty, 
myflipbuf[boardnum],
                                 myflipflagbuf[boardnum], len);

                         /* Tell the tty layer its okay to "eat" the 
data now */
                         tty_flip_buffer_push(tty);
                 }
.
.
.
         /*
          * Just go ahead and try to read some data into the flip buffer.
          * If there isn't any data available, dxb_rxbuf_read will return
          * 0 back to us right away.
          *
          * The dgdm_rawreadok case takes advantage of carnal knowldge that
          * the char_buf and the flag_buf are next to each other and
          * are each of (2 * TTY_FLIPBUF_SIZE) size.
          */

         if(dgdm_rawreadok && tty->real_raw)
                 flip_len = MYFLIPLEN;
         else
                 flip_len = TTY_FLIPBUF_SIZE;

         flip_len = MIN(flip_len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);

         len = dxb_rxbuf_read(brd->dxb, dc->dxbchan, 
myflipbuf[boardnum], flip_len);


etc...

Looks like for hopes of performance????




>>> For both PCI and PCI-e, these drivers should _at a minimum_ be pci
>>> drivers that register the tty driver at module init and register _only_
>>> the tty devices for that particular PCI device at PCI probe time. Look
>>> at the end of synclink_gt.c for how this is supposed to look.
>>>
>>
>> I'll look at it some more but I have been there.
>
> Specifically, review:
> 	struct pci_driver
> 	device_init()
> 	init_one()
> 	remove_one()
> 	sglt_init()
> 	sglt_exit()
>

Ok, I'll will look more closely at these to try to understand what 
should be done.


Here is the init_module funtion FYI.

GLOBAL int init_module(void)
{
     int rc;

         /* make sure that the globals are init'd before we do anything 
else */
         dgdm_init_globals();

         dgdm_NumBoards = 0;
         dgdm_Major_Serial_Registered = FALSE;
         dgdm_Major_Callout_Registered = FALSE;
         dgdm_Major_TransparentPrint_Registered = FALSE;
         dgdm_Major_Control_Registered = FALSE;
         dgdm_Major_ISDN_Registered = FALSE;

         APR(("%s, Digi International Part Number %s\n", DG_NAME, DG_PART));

         /*
          * Find and configure all the cards
          */

         rc = dgdm_init_pci();

         if (!rc) {
                 /*
                  * At least one card was found, register the devices
                  * with the kernel.
                  */
                 dgdm_proc_register_basic();

                 if (dgdm_tty) {
                         rc = dgdm_tty_init();  // This is the function 
that creates all the device entries
                         if (rc < 0) {
                                 dgdm_tty_uninit();
                                 APR(("Can't init tty devices (%d)\n", rc));
                                 return (rc);
                         }
                         /*
                          * Register FEP related /proc files
                          */
                         dgdm_proc_register_fep();
                 }
                 if (!dgdm_Major_Control_Registered) {
                         /*
                          * Register memory/control/debug/mangement devices
                          */
                         rc = register_chrdev(major + DGDM_CONTROL_MAJOR,
                                              "dxb", &BoardFops);
                         if (rc < 0) {
                                 APR(("Can't register ram device 
(%d)\n", rc));
                                 return (rc);
                         }
                         dgdm_Major_Control_Registered = TRUE;
                 }
                 if (dgdm_isdn && !dgdm_Major_ISDN_Registered) {
                         /*
                          * Register ISDN testing device
                          */
                         rc = register_chrdev(major + DGDM_ISDN_MAJOR,
                                              "dxbisdn", 
&dgdm_BoardIsdnFops);
                         if (rc < 0) {
                                 APR(("Can't register ISDN device 
(%d)\n", rc));
                                 return (rc);
                         }
                         dgdm_Major_ISDN_Registered = TRUE;
                 }
                 if(dgdm_NumBoards > 4) {
                         if (!dgdm_Major_Control2_Registered) {
                                 rc = 
register_chrdev(major+DGDM_CONTROL2_MAJOR,
                                                      "dxb2", &BoardFops);
                                 if (rc < 0) {
                                         APR(("Can't register high ram "
                                              "device (%d)\n", rc));
                                         return (rc);
                                 }
                                 dgdm_Major_Control2_Registered = TRUE;
                         }
                         if (dgdm_isdn && !dgdm_Major_ISDN2_Registered) {
                                 rc = register_chrdev(major + 
DGDM_ISDN2_MAJOR,
                                                      "dxbisdn2", 
&dgdm_BoardIsdnFops);
                                 if (rc < 0) {
                                         APR(("Can't register high ISDN "
                                              "devices (%d)\n", rc));
                                         return (rc);
                                 }
                                 dgdm_Major_ISDN2_Registered = TRUE;
                         }
                 }

                 //       Compro Computer Svcs udev support
                 // Need udev notification of the dgdm_mgmt device creation
                 //
                 // See config/99-dgdm.rules 
"/etc/udev/rules.d/99-dgdm.rules"
                 // and /usr/local/sbin/dgdm
                 //
                 if (alloc_chrdev_region(&dgdm_mgmt_major, 0, 1, 
"dgdm_mgmt") != 0) {
                         printk("DGDM: Unable to get MAJOR/MINOR device 
numbers\n");
                         return -ENODEV;
                 }

                 dgdm_mgmt_device = cdev_alloc();
                 if (dgdm_mgmt_device <= 0) {
                         printk("DGDM: Unable to alloc cdev struct\n");
                         return -ENODEV;
                 }

                 cdev_init(dgdm_mgmt_device, &dgdm_BoardIsdnFops);
                 dgdm_mgmt_device->owner = THIS_MODULE;

                 if (cdev_add(dgdm_mgmt_device, dgdm_mgmt_major, 1) < 0) {
                         printk("DGDM: Error Registering chrdev device\n");
                         return -ENODEV;
                 }

                 dgdm_mgmt_class = class_create(THIS_MODULE, "dgdm_mgmt");
                 if (IS_ERR(dgdm_mgmt_class)) {
                         printk("DGDM: Error creating class\n");
                         return -ENODEV;
                 }

                 device_dgdm_mgmt = device_create(dgdm_mgmt_class, NULL, 
dgdm_mgmt_major, NULL, "dgdm_mgmt");
                 if (IS_ERR(device_dgdm_mgmt)) {
                         printk("DGDM: Error creating device entry\n");
                         return -ENODEV;
                 }
         } else {
                 cleanup_module();
         }

         return (rc);
}


Mark

Re: Out of tree GPL serial tty driver help?

[Peter Hurley]

On Fri, 2013-04-26 at 16:26 -0400, Mark Hounschell wrote:
> On 04/26/2013 03:51 PM, Peter Hurley wrote:
> > On Fri, 2013-04-26 at 14:17 -0400, Mark Hounschell wrote:
> >> On 04/26/2013 12:37 PM, Peter Hurley wrote:
> >>> These drivers weren't really current at 3.4 though, either. I'm not sure
> >>> what else you're going to find that doesn't work.
> >>>
> >>
> >> No, I have kept them current, or I should say functional, to the best of
> >> my ability from 2.6 up to and including 3.4.x.
> >
> > By 'current', I mean 'similar in structure and functionality to in-tree
> > drivers'.
> >
> > The structure of this driver is more akin to a 2.5 driver (back when
> > there were separate serial and callout tty drivers).
> >
> >>   What I have here works
> >> with kernels up to 3.4.x. I have not tried anything between 3.4 and 3.8.
> >>    As far as building against 3.8, the only issues were the change from
> >> *termios to termios and the "structn_tty_data" no longer in an include
> >> file so not easily directly accessible.
> >
> > What is this driver accessing in N_TTY's private data?

Using the line discipline receive_buf() method directly is unsafe.

If you must, at least don't use read_cnt; use tty->receive_room. It's
not safer but at least it's accessible. [FYI - this will probably change
in the next couple of releases]

When using the tty buffer interface (!real_raw), instead of trying to
determine how much receive space is available, you need to compute how
much data is available to read and insert that with
tty_insert_flip_string_xxxx -- the tty_buffer_request_room() is
superfluous because the tty_insert_flip_string_xxxx interface already
does that for you.

Derive the equivalent tty->real_raw setting in the
driver's .set_termios() method. You can review n_tty_set_termios() to
see how to duplicate the value safely in the driver.

>          /* Decide how much data we can send into the tty layer */
>          if(dgdm_rawreadok && tty->real_raw)
>                  flip_len = MYFLIPLEN;
>          else
>                  flip_len = TTY_FLIPBUF_SIZE;
>          len = MIN(data_len, flip_len);
>          len = MIN(len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);
>          dxb_return_fixed_cost(dxb, channel, 1);
>          ld = tty_ldisc_ref(tty);
> .
> .
> .
> .
>                  /*
>                   * In high performance mode, we don't have to update
>                   * flag_buf or any of the counts or pointers into flip buf.
>                   */
>                  if(!dgdm_rawreadok || !tty->real_raw) {
>                          if (I_PARMRK(tty) || I_BRKINT(tty) || 
> I_INPCK(tty)) {
>                                  parity_scan(dc, myflipbuf[boardnum],
>                                          myflipflagbuf[boardnum], &len);
>                          } else {
>                                  memset(myflipflagbuf[boardnum], 
> TTY_NORMAL, len);
>                          }
>                  }
> .
> .
> .
> .
>                  /*
>                   * If we're doing raw reads, jam it right into the
>                   * line disc bypassing the flip buffers.
>                   * OPT OPP: do this only once per channel instead of
>                   * once per dxb message.
>                   */
>                  if(dgdm_rawreadok && tty->real_raw) {
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,27)
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,30)
>                          tty->ldisc->ops->receive_buf(tty, 
> myflipbuf[boardnum], NULL, len);
> #else
>                          tty->ldisc.ops->receive_buf(tty, 
> myflipbuf[boardnum], NULL, len);
> #endif
> #else
>                          tty->ldisc.receive_buf(tty, 
> myflipbuf[boardnum], NULL, len);
> #endif
>                  } else {
>                          len = tty_buffer_request_room(tty, len);
>                          tty_insert_flip_string_flags(tty, 
> myflipbuf[boardnum],
>                                  myflipflagbuf[boardnum], len);
> 
>                          /* Tell the tty layer its okay to "eat" the 
> data now */
>                          tty_flip_buffer_push(tty);
>                  }
> .
> .
> .
>          /*
>           * Just go ahead and try to read some data into the flip buffer.
>           * If there isn't any data available, dxb_rxbuf_read will return
>           * 0 back to us right away.
>           *
>           * The dgdm_rawreadok case takes advantage of carnal knowldge that
>           * the char_buf and the flag_buf are next to each other and
>           * are each of (2 * TTY_FLIPBUF_SIZE) size.
>           */
> 
>          if(dgdm_rawreadok && tty->real_raw)
>                  flip_len = MYFLIPLEN;
>          else
>                  flip_len = TTY_FLIPBUF_SIZE;
> 
>          flip_len = MIN(flip_len, (N_TTY_BUF_SIZE - 1) - tty->read_cnt);
> 
>          len = dxb_rxbuf_read(brd->dxb, dc->dxbchan, 
> myflipbuf[boardnum], flip_len);
> 
> 
> etc...
> 
> Looks like for hopes of performance????
> 
> 
> 
> 
> >>> For both PCI and PCI-e, these drivers should _at a minimum_ be pci
> >>> drivers that register the tty driver at module init and register _only_
> >>> the tty devices for that particular PCI device at PCI probe time. Look
> >>> at the end of synclink_gt.c for how this is supposed to look.
> >>>
> >>
> >> I'll look at it some more but I have been there.
> >
> > Specifically, review:
> > 	struct pci_driver
> > 	device_init()
> > 	init_one()
> > 	remove_one()
> > 	sglt_init()
> > 	sglt_exit()
> >
> 
> Ok, I'll will look more closely at these to try to understand what 
> should be done.

Don't bother; I looked over the code that's still downloadable from
digi.com (which seems to be your baseline) and for your needs, this
change would be pointless.

Regards,
Peter Hurley

Re: Out of tree GPL serial tty driver help?

[Greg KH]

On Fri, Apr 26, 2013 at 01:58:14PM -0400, Mark Hounschell wrote:
> As for the Digi card driver/drivers, I can't say why not. There is a
> remnant of the actual one this thread is pertaining to in
> ./Documentation/serial/digiepca.txt. I've never submitted it or any
> of their others I have to maintain because I didn't know I could nor
> would know how to start. I thought that Digi would have had to do
> it.
> 
> As far as Compro's drivers go, I think we have just been a little
> wary/unknowing of what it would take and what the results of the
> effort would actually be. Would they still do what _we_ needed them
> to do? Are we kernel savvy enough to get them to a state you would
> accept? etc... We do understand that if they were in the kernel that
> the best qualified people would be the ones that "kept them current"
> with the latest kernel changes. It's certainly something that no one
> here would object to doing. Are you the man to talk to? What is the
> actual process?

Yes, I'm the person to talk to about doing this, we can take it off-list
if you want.

But, for some reading material about the process, take a look at
Documentation/SubmittingDrivers and Documentation/SubmittingPatches in
the kernel source tree.

Also, if you don't want to do the work, I, and a bunch of others, will
be glad to do it for you, for free, see the site linuxdriverproject.org
for some details about that.

Hope this helps,

greg k-h