linux-kernel - Re: [PATCH] hid-ft260: add UART support.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y6WNl6+ySy8zcSyg@hovoldconsulting.com>
Date:   Fri, 23 Dec 2022 12:14:31 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Daniel Beer <daniel.beer@...rinstitute.com>,
        Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        linux-i2c@...r.kernel.org,
        Michael Zaidman <michael.zaidman@...il.com>,
        Christina Quast <contact@...istina-quast.de>,
        linux-serial@...r.kernel.org, Jiri Slaby <jirislaby@...nel.org>
Subject: Re: [PATCH] hid-ft260: add UART support.

On Thu, Dec 08, 2022 at 11:02:36AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 05, 2022 at 02:24:03PM +1300, Daniel Beer wrote:
> > On Sun, Dec 04, 2022 at 10:39:21AM +0100, Greg Kroah-Hartman wrote:
> > > > Thanks for reviewing. This device is quite strange -- it presents itself
> > > > as a USB HID, but it provides both an I2C master and a UART. The
> > > > existing driver supports only the I2C functionality currently.

> > > > > A whole new major for just a single tty port?  Please no, use dynamic
> > > > > majors if you have to, or better yet, tie into the usb-serial
> > > > > implementation (this is a USB device, right?) and then you don't have to
> > > > > mess with this at all.
> > > > 
> > > > As far as I understand it, I don't think usb-serial is usable, due to
> > > > the fact that this is already an HID driver.
> > > 
> > > That should not be a restriction at all.  You are adding a tty device to
> > > this driver, no reason you can't interact with usb-serial instead.  That
> > > way you share the correct userspace tty name and major/minor numbers and
> > > all userspace tools should "just work" as they know that name and how to
> > > interact with it already.
> > > 
> > > Try doing that instead of your own "raw" tty device please.
> > 
> > Maybe I've misunderstood something. The reason I thought usb-serial was
> > unusable in this instance was that I couldn't see a way to create a port
> > except via usb-serial's own probe function (otherwise, the API looked
> > fine).
> > 
> > I don't know whether I'm looking at a serial or an I2C interface until
> > after it's already been probed by HID core, I have a struct hid_device
> > and I've asked what type of interface it is via an HID feature report.
> > This can't be determined otherwise, because strapping pins affect the
> > presentation of interfaces.
> > 
> > At that point, I (currently) call uart_add_one_port. I might have missed
> > it, but I didn't see anything analogous in the usb-serial API. Am I
> > going about this the wrong way?
> 
> I thought that this could be done, but I might be wrong.  Johan, any
> ideas?

It seems to me like this should be implemented as a new tty driver as
neither USB-serial or serial (core) is a good fit for such a HID device.

So this appears to be right approach in general:

	https://lore.kernel.org/all/20221207220617.116082-1-contact@christina-quast.de/

Johan