From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by befuddled.reisers.ca (Postfix, from userid 65534) id 7A2421F0938; Mon, 15 May 2017 07:10:13 -0400 (EDT) Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) by befuddled.reisers.ca (Postfix) with ESMTPS id 797771F090E for ; Mon, 15 May 2017 07:10:11 -0400 (EDT) Received: by mail-io0-x243.google.com with SMTP id m4so11241984ioe.0 for ; Mon, 15 May 2017 04:10:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=f6LsvbKavvbq/xr4Y1w5E1tl1+j9yN933+2/f35O5MI=; b=p3PEXUAC9Gou+pz9BMgjBMOwlf1iAB1eSQBuToDuRujFR0nq2Hfnwq4wmaCpdxTXgt SIxG7TkG01qv53bJPgPpq7MY2763TcdL9RHpv1OokUaT+8Aml9JzIv5/VtCqQoeN9wwB qq+fQt4RzhCkjkNUBZWnmDnusdDAbqmBqOKbpzGUHNmnFLPF8v5tEjxTnM9B8yca/4hE QD7MW9JUUdX0haBmj0zV6uaVTMdItdfQUCsADr/WispTYw7iUFUdr2wN0ocRxG3fzpVG H93K7XISBB0lMEzaIkT8rwoQqYbLp96rF6EZ4xM/XUHGg3nZ7UPgk8Nj7tZsm5UMFBKv i3vQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=f6LsvbKavvbq/xr4Y1w5E1tl1+j9yN933+2/f35O5MI=; b=Nu1lbqExgtLNG9jSNGCvXVAZLPVmLw+E5Iyfv+hxFIwwBUNSdAohxAjlt6/kbY0lpM vTZ+49WEr2WIxM/LglLl9H24mfH3mAtouhDffbjQ+o7o5S7iLrFlKo6S3krBw5st8biS MwloACraxB4fzzi9l6KNqy3w2d8LKmvJ3LWlgRYRq6FkQfzxTbZBzkcHwYL4xJW/7hol 3miaUejIBHp23grO/TdGY1PyqqB9UaZOPTEVz2Jl19MzVFoZwBHo8fOMlT48ExIPx57n DPU4zqhfWtgJI7Q5oWqh71v7L5+EVEluBHh7Bals690Am3lsB58/9hE287M+AH2tq9et Gdgg== X-Gm-Message-State: AODbwcCumNqClknwmCsm9GszceRZKzAJK+cFsxLgISMUbaxjip5OeIke 1wIIp8+oMS6PK/WHPiSGGwOdmDg/+Q== X-Received: by 10.107.34.201 with SMTP id i192mr4965834ioi.222.1494846606727; Mon, 15 May 2017 04:10:06 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.154.142 with HTTP; Mon, 15 May 2017 04:10:06 -0700 (PDT) In-Reply-To: <20170515103118.GA10885@kroah.com> References: <20170429195257.630355823@gmail.com> <20170429195403.522579504@gmail.com> <20170515103118.GA10885@kroah.com> From: Okash Khawaja Date: Mon, 15 May 2017 12:10:06 +0100 Message-ID: Subject: Re: [patch 2/6] tty: export tty_open_by_driver To: Greg Kroah-Hartman Cc: Jiri Slaby , Samuel Thibault , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, Kirk Reiser , Chris Brannon , "Speakup is a screen review system for Linux." , Alan Cox Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 X-BeenThere: speakup@linux-speakup.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Speakup is a screen review system for Linux." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 May 2017 11:10:13 -0000 Hi, On Mon, May 15, 2017 at 11:31 AM, Greg Kroah-Hartman 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 >> Reviewed-by: Samuel Thibault >> >> 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@vger.kernel.org/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. Thanks, Okash