Message-ID: <20180601162645.GA30792@amd>
Date:   Fri, 1 Jun 2018 18:26:46 +0200
From:   Pavel Machek <pavel@....cz>
To:     Johan Hovold <johan@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Andreas Kemnade <andreas@...nade.info>,
        Arnd Bergmann <arnd@...db.de>,
        "H . Nikolaus Schaller" <hns@...delico.com>,
        Marcel Holtmann <marcel@...tmann.org>,
        Sebastian Reichel <sebastian.reichel@...labora.co.uk>,
        Tony Lindgren <tony@...mide.com>, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v3 0/8] gnss: add new GNSS subsystem

Hi!

> > So you have serial line + pm + protocol type. Nothing GNSS specific
> > really, it would be useful to (for example) say this is modem with AT
> > commands. Or this is QMI modem.
> 
> It's a matter of finding the right abstraction level. A user space
> location service will now have easy access to the class of devices it
> cares about, without having to go through a list of other random devices
> which happen to use a similar underlying interface (e.g. a modem or
> whatever).

udev solves device discovery pretty well; I don't think that's good
thing to optimize for.

(And.. I'd really prefer /dev/nmeaX and /dev/sirfX in that situation;
and yes that makes it _even easier_ for location service to find right devices). 

> Point is, you can't write a subsystem for everything. If it later turns
> out some part of the code can be shared internally, fine.

True. But you can pick reasonable name for the subsystem :-).

> > > Finally, the GNSS subsystem is clearly not serial (as in UART) specific
> > > and works just as fine with I2C, SPI, USB, iomem, rproc or whatever
> > > interface the GNSS uses.
> > 
> > Ok, true. It is "we pass tty-like data across". Again, it would be
> > useful for AT commands, too, and yes, they go over serials and usbs
> > and more, too. 
> 
> Modems and GNSS devices would have different characteristics for buffers
> and throughput for starters.
> 
> The GNSS interface uses a synchronous writes for commands to the
> receiver, something which may not be appropriate for an outgoing data
> channel, for example.

Well, these days AT modems really have separate channels for commands
and data.

> As mentioned in the cover letter, we may eventually want to add support
> for some kinds of configuration from within the kernel (e.g. protocol
> and line speed changes).

I believe we'll eventually want "real" GPS drivers, with common
interface. input was in this situation before...

> 
> > > > This will never handle devices like Nokia N900, where GPS is connected
> > > > over netlink.
> > > 
> > > Marcel already suggested adding a ugnss interface, which would allow
> > > feeding GNSS data through the generic GNSS interface from user space for
> > > use cases where it's not (yet) feasible to implement a kernel
> > > driver.
> > 
> > Yes, and ugnss would be at wrong layer for N900. First, lets take a
> > look at N900 gps driver:
> > https://github.com/postmarketOS/gps-nokia-n900
> > 
> > Got it? I'll eventually like to see it in the kernel, but your "GNSS"
> > subsystem is unsuitable for it, as it does not talk well-known
> > protocol.
> 
> Seriously? If you can't be arsed to summarise your use case, why would I
> bother reading your random user space code?

You are trying to push subsystem to kernel. I believe asking you to do
little research is not unexpected.

Anyway, it is trivial binary protocol over packets that are used for
modem communication. Handling it is like 40? lines of code.

> > I'd like to see "datapipe" devices (what you currently call GNSS) and
> > "gps" devices, which would have common interface to the userland.
> 
> You keep talking about this "gps" interface, which based on your
> earlier comments I assume you mean a NMEA 0183 interface? Again,
> converting a vendor-specific binary protocol may not be feasible. Please
> try to be more be specific.

I'm pretty sure we should have one protocol for communicating position
data to userland. I don't want to go into details at the moment, as
they are not important at the moment (and we could spend lot of time
discussing details).

NMEA would not be my first choice, really. I'd propose something very
similar to existing /dev/input/eventX, but with wider data types.

> > N900 would not have any datapipe devices, but would have /dev/gps0
> > exposing common GPS interface.
> > 
> > Droid4 would have /dev/datapipe0 (usb, AT protocol), /dev/datapipe1
> > (usb, QMI protocol), /dev/datapipe2 (gsmtty over serial, custom AT
> > protocol), /dev/datapipe3 (gsmtty over serial, NMEA wrapped in AT
> > protocol) (and more, it is complex hardware). And if we really wanted,
> > we'd have /dev/gps0, too, exposing common GPS interface.
> > 
> > Your devices would have /dev/datapipe0 with sirf or ublox or nmea
> > protocol. In we really wanted, we'd have /dev/gps0, too, again,
> > exposing common GPS interface.
> 
> This does not seem like the right abstraction level and doesn't appear
> to provide much more than what we currently have with ttys.

I don't see what you are saying here. I've described your proposal but
replaced /dev/gnss0 with /dev/datapipe0.

> > I believe we really want to use your code for AT commands, too. And we
> > really should keep GNSS/GPS names for future layer that actually
> > provides single interface for userland.
> 
> Specifics, please. What would such an interface look like? I'm pretty
> sure, we do not want to move every GNSS middleware protocol handler into
> the kernel.

Maybe we don't want to move _every_ GNSS protocol handler into kernel,
but I'm pretty sure we need to move _some_ of them there. It is
certainly best place for Nokia N900's protocol.

And I'm trying to make sure we have suitable names available when that
happens.

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)

Powered by blists - more mailing lists