Re: [PATCH 72/80] tty: fix up gigaset a bit

[Tilman Schmidt]

Scripsit Alan Cox die 17.10.2008 13:40:
>>>  	/* prevent other callers from entering ldisc methods */
>>> +	/* FIXME: should use the tty state flags */
>>>  	tty->disc_data = NULL;
>>>  
>>>  	if (!cs->hw.ser)
>> 
>> Do you know of an example line discipline that has got this right?
>> My model for this code was drivers/net/ppp_async.c but now it seems
>> that this was not as exemplary as I had hoped.
> 
> If you want to know if the tty has been closed for further I/O then you
> can use test_bit(TTY_IO_ERROR, &tty->flags). This gets set at the right
> points on close/hangup/etc.

Ok, but where do you see a need for such a test? AFAICS I have covered
all the eventualities without resorting to an explicit tty->flags test,
and looking around among my LD brethren it looks like they came to the
same conclusion. (Not that I'd want to pull the "a million flies can't
be wrong" card. ;-) Can you point me to a place or situation where an
additional test_bit(TTY_IO_ERROR, &tty->flags) would catch a case that
would otherwise be handled incorrectly?

>> Looking around, I see that many LDs don't even provide a poll method.
>> So I'm thinking of just dropping this one. Would that be ok?
> 
> PPP doesn't route characters via the tty interface while you seem to do
> so for the AT emulation so you do need it.

Actually I don't. The AT command interface is accessed through a
separate tty interface created by the main module of the Gigaset driver
in drivers/isdn/gigaset/interface.c.

(The reason for that is that ser_gigaset is just one of three hardware
backends for accessing Gigaset devices, and the only one with an
existing tty device. I have to create my own AT command interface
device for the other two, anyway. Specialcasing ser_gigaset in order to
route that back to the tty device of the serial port would just
complicate things needlessly.)

So I think removing the poll method is the right thing to do. If you
have no objections, I'll submit a patch doing just that. (Hoping to
still make it for the .28 merge window.)

Thanks,
Tilman

Attachment: signature.asc
Description: OpenPGP digital signature