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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 5 Dec 2022 14:24:03 +1300
From:   Daniel Beer <daniel.beer@...rinstitute.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     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 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.
> 
> Lots of devices are a "fake HID" device as other operating systems make
> it easy to write userspace drivers that way.  Linux included.  What
> userspace programs are going to want to interact with this device and
> what api are they going to use?

Hi Greg,

The application I'm looking at uses it as a debug console, so personally
I'd like to be able to use it with picocom and other terminal programs.

> > > > --- a/include/uapi/linux/major.h
> > > > +++ b/include/uapi/linux/major.h
> > > > @@ -175,4 +175,6 @@
> > > >  #define BLOCK_EXT_MAJOR		259
> > > >  #define SCSI_OSD_MAJOR		260	/* open-osd's OSD scsi device */
> > > >  
> > > > +#define FT260_MAJOR		261
> > > 
> > > 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?

Cheers,
Daniel

-- 
Daniel Beer
Firmware Engineer at Igor Institute
daniel.beer@...rinstitute.com or +64-27-420-8101
Offices in Seattle, San Francisco, and Vancouver BC or (206) 494-3312