On Mon, May 15, 2017 at 12:10:06PM +0100, Okash Khawaja wrote:
> Hi,
>
> On Mon, May 15, 2017 at 11:31 AM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Sat, Apr 29, 2017 at 08:52:59PM +0100, Okash Khawaja wrote:
> >> This applies on top of the changes already in staging-next branch which allow
> >> kernel access to TTY dev.
> >>
> >> Signe-doff-by: Okash Khawaja <okash.khawaja@xxxxxxxxx>
> >> Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
> >>
> >> Index: linux-staging/drivers/tty/tty_io.c
> >> ===================================================================
> >> --- linux-staging.orig/drivers/tty/tty_io.c
> >> +++ linux-staging/drivers/tty/tty_io.c
> >> @@ -1369,7 +1369,10 @@ static struct tty_struct *tty_driver_loo
> >> struct tty_struct *tty;
> >>
> >> if (driver->ops->lookup)
> >> - tty = driver->ops->lookup(driver, file, idx);
> >> + if (!file)
> >> + tty = ERR_PTR(-EIO);
> >> + else
> >> + tty = driver->ops->lookup(driver, file, idx);
> >
> > Why make this change? Shouldn't the lookup function allow a NULL file
> > pointer? Or is the problem that they do not?
> >
> >> else
> >> tty = driver->ttys[idx];
> >>
> >> @@ -2001,7 +2004,7 @@ static struct tty_driver *tty_lookup_dri
> >> struct tty_driver *console_driver = console_device(index);
> >> if (console_driver) {
> >> driver = tty_driver_kref_get(console_driver);
> >> - if (driver) {
> >> + if (driver && filp) {
> >
> > Why change this too?
> >
> > Your changelog does not explain any of this, please do so.
> >
> > thanks,
> >
> > greg k-h
>
>
> Sorry, I should have been more descriptive here. The changes which
> check file pointer for null are basically from Alan Cox's patch here:
> http://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1215095.html.
> The description from that patch is quoted below:
>
> "[RFC] tty_port: allow a port to be opened with a tty that has no file handle
>
> Let us create tty objects entirely in kernel space. Untested proposal to
> show why all the ideas around rewriting half the uart stack are not needed.
>
> With this a kernel created non file backed tty object could be used to
> handle
> data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> particular has to work back to the fs/tty layer.
>
> The tty_port code is however otherwise clean of file handles as far as I can
> tell as is the low level tty port write path used by the ldisc, the
> configuration low level interfaces and most of the ldiscs.
>
> Currently you don't have any exposure to see tty hangups because those are
> built around the file layer. However a) it's a fixed port so you probably
> don't care about that b) if you do we can add a callback and c) you almost
> certainly don't want the userspace tear down/rebuild behaviour anyway.
>
> This should however be sufficient if we wanted for example to enumerate all
> the bluetooth bound fixed ports via ACPI and make them directly available.
>
> It doesn't deal with the case of a user opening a port that's also kernel
> opened and that would need some locking out (so it returned EBUSY if bound
> to a kernel device of some kind). That needs resolving along with how you
> "up" or "down" your new bluetooth device, or enumerate it while providing
> the existing tty API to avoid regressions (and to debug)."
>
> With this patchset tty_open_by_driver is now called from inside kernel
> with file pointer set to null. I can resend this patch with above
> description.
Please fix that up and resend the whole series.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
