LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jason Wessel <jason.wessel@windriver.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: kgdb-bugreport@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers
Date: Tue, 21 Oct 2008 20:54:23 -0500	[thread overview]
Message-ID: <48FE87CF.4040007@windriver.com> (raw)
In-Reply-To: <20081021234728.6853beee@lxorguk.ukuu.org.uk>

Alan Cox wrote:
> On Tue, 21 Oct 2008 14:14:51 -0500
> Jason Wessel <jason.wessel@windriver.com> wrote:
>
>> +        unlock_kernel();
>> +    } else {
>> +        tty = tty_driver_lookup_tty(driver, NULL, tty_line);
>> +        if (!tty) {
>> +            tty = tty_init_dev(driver, tty_line, 0);
>> +            inc_tty = 0;
>> +        }
>> +        (*filp)->private_data = tty;
>> +        ret = tty->ops->open(tty, *filp);
>
> and with the error paths wrong
>
>
> This is to put it bluntly just horrible.
>

Yeah, I'd say it is pretty horrible too, but this the only API that
exists presently and it is not at all obvious how to use it.

I am fine with keeping it out of the tree, IE this patch is dropped
from merge consideration for now.  It is obvious that it is a moving
target because the patch to 2.6.27 looked different than 2.6.28.  Even
with it out of the tree I am interested in fixing the "error paths"
you mentioned.  Perhaps in this case the return of tty_init_dev()
needed further checking and that was what you were stating is incorrect?

> Really kgdb, console and other boot processes need an abstraction for tty
> device ports at the device (not ldisc) level and until that work is done
> there is no acceptable way to sort this out. Merging gunk like this will
> simply make the existing mess worse and provide more crap that will make
> it harder to fix properly.

If there is another API, please point me to it.  I am not aware of how
to get uart startup code to run without going through the
tty->ops->open().  The kgdboc module has only a generic high level
interface and makes use of only the high level tty calls.  The
debugger's needs are the same as the console and the user space.  The
early debug is still a special case, but in the ideal world there
would be a way to hand off from early debug into the struct tty_port
at the point that console_init() is called.

>
> All this stuff needs revisiting when every tty device side object in the
> kernel contains a struct tty_port, until then it's a nasty (perhaps
> neccessary for the moment) hack that belongs out of tree.

Let me know when we get closer to that stage, as I am interested in
figuring out how to have a uniform way to share the hardware between
the polled and interrupt drive contexts.

Thanks,
Jason.


  reply	other threads:[~2008-10-22  1:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-21 19:14 kgdb 2.6.28 merge plans Jason Wessel
2008-10-21 19:14 ` [PATCH 1/7] kgdb: Add the ability to schedule a breakpoint via a tasklet Jason Wessel
2008-10-21 19:14   ` [PATCH 2/7] tty: Fix sparse static warning for tty_driver_lookup_tty Jason Wessel
2008-10-21 19:14     ` [PATCH 3/7] kgdboc, tty: Add the rx polling call back capability Jason Wessel
2008-10-21 19:14       ` [PATCH 4/7] kgdboc, 8250: Add the rx polling hook to 8250 driver Jason Wessel
2008-10-21 19:14         ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
2008-10-21 19:14           ` [PATCH 6/7] kgdb: kgdboc console poll hooks for serial_txx9 uart Jason Wessel
2008-10-21 19:14             ` [PATCH 7/7] kgdb: Make mem access function weak in kgdb.c andkgdb.h Jason Wessel
2008-10-21 21:49           ` [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers Jason Wessel
2008-10-21 22:47           ` Alan Cox
2008-10-22  1:54             ` Jason Wessel [this message]
2008-10-21 22:49 ` kgdb 2.6.28 merge plans Alan Cox
2008-10-22  2:09   ` Jason Wessel
2008-10-22  8:34     ` Alan Cox
2008-10-22 10:07       ` Jason Wessel
2008-10-22 12:25         ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48FE87CF.4040007@windriver.com \
    --to=jason.wessel@windriver.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=kgdb-bugreport@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH 5/7] kgdboc, tty: use tty open to start low level drivers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).