mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* serial167 (was: Re: [RFC 0/5] tty: move stuff around)
@ 2011-03-17  9:59 Geert Uytterhoeven
  2011-03-17 10:46 ` Alan Cox
  0 siblings, 1 reply; 3+ messages in thread
From: Geert Uytterhoeven @ 2011-03-17  9:59 UTC (permalink / raw)
  To: Alan Cox
  Cc: Arnd Bergmann, Greg KH, Jiri Slaby, linux-kernel, Rogier Wolff,
	Michael H. Warfield, Linux/m68k

On Fri, Sep 3, 2010 at 22:32, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Fri, 3 Sep 2010 22:08:38 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
>> On Fri, Sep 3, 2010 at 17:29, Arnd Bergmann <arnd@arndb.de> wrote:
>> > amiserial,nozomi,synclink,
>> > rocket,ipwireless
>> > cyclades,moxa,mxser,
>> > isicom,bfin_jtag_comm   -> drivers/tty/
>> >
>> > stallion,istallion,
>> > epca,specialix,ip2,
>> > riscom,serial167        -> drivers/staging/tty/*
>>
>> I thought we wanted to keep serial167 alive as long as it builds?
>
> Does it build ? Is it secure ? I was under the impression the answer in
> both cases was no.

As serial167 is now in staging in Linus tree...

It still builds in 2.6.38. What is unsecure about it? If nothing,
please move it back,
as the whole MVME167 platform is pretty useless without its console driver.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: serial167 (was: Re: [RFC 0/5] tty: move stuff around)
  2011-03-17  9:59 serial167 (was: Re: [RFC 0/5] tty: move stuff around) Geert Uytterhoeven
@ 2011-03-17 10:46 ` Alan Cox
  2011-03-17 10:52   ` Geert Uytterhoeven
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Cox @ 2011-03-17 10:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Arnd Bergmann, Greg KH, Jiri Slaby, linux-kernel, Rogier Wolff,
	Michael H. Warfield, Linux/m68k

> As serial167 is now in staging in Linus tree...
> 
> It still builds in 2.6.38. What is unsecure about it? If nothing,
> please move it back,
> as the whole MVME167 platform is pretty useless without its console driver.

It has no reference counting and it doesn't use tty_port objects.

The former has been required for many releases otherwise your tty objects
are going to vanish under you and stuff, the latter is now used by all
'active' drivers and once we have total coverage will allow a lot of
simplification as it will mean a lot of structures that belong to the
physical port lifetime can be put there - like the tty input buffers.

You have other problems too - eg startup and shutdown depend upon the big
kernel lock - which is now gone from those areas.

The tty_port helpers now in the kernel will do a lot of the work for you.
They are designed to do all the locking and nastiness around
open/close/hangup and provide you with serialized simple handling for
startup/shutdown.

At minimum you want to add tty refcounting and tty port objects. If you
use the tty_port helpers for open/close/hangup and rip out all but the
tiny core bits of startup/shutdown etc that should also sort out the main
other problems.

Going via info->tty can be done via tty_port_tty_get() but in this driver
you can probably prove it's safe to take a kref when you assign the
port->tty field and drop it on shutdown at least for the IRQ paths, for
some of the none IRQ paths it looks like tty should be passed down. I am
not sure about user triggered vhangup() being ok to just keep the single
ref. tty_port_tty_get() at the start of the IRQ handler, passing the
tty reference around and putting the kref at the end would be a fraction
heavier but should be bulletproof.

Bringing it up to the 21st century might not be a bad idea either -
consts, platform device, no hardcoded board specific hackery, not
essential but if the rest of the driver is getting a much needed service
perhaps it's worth treating as it was a new submission and giving it a
full review and cleanup to modern spec ? We wouldn't accept the existing
driver as a submission today.

Alan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: serial167 (was: Re: [RFC 0/5] tty: move stuff around)
  2011-03-17 10:46 ` Alan Cox
@ 2011-03-17 10:52   ` Geert Uytterhoeven
  0 siblings, 0 replies; 3+ messages in thread
From: Geert Uytterhoeven @ 2011-03-17 10:52 UTC (permalink / raw)
  To: Alan Cox
  Cc: Arnd Bergmann, Greg KH, Jiri Slaby, linux-kernel, Rogier Wolff,
	Michael H. Warfield, Linux/m68k

Hi Alan,

On Thu, Mar 17, 2011 at 11:46, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> As serial167 is now in staging in Linus tree...
>>
>> It still builds in 2.6.38. What is unsecure about it? If nothing,
>> please move it back,
>> as the whole MVME167 platform is pretty useless without its console driver.
>
> It has no reference counting and it doesn't use tty_port objects.

[...]

Many thanks for your elaborate (and valid) explanation!
I'll let the VME users sort it out...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-03-17 10:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-17  9:59 serial167 (was: Re: [RFC 0/5] tty: move stuff around) Geert Uytterhoeven
2011-03-17 10:46 ` Alan Cox
2011-03-17 10:52   ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®