Message-ID: <20180605214752.GB28143@amd>
Date:   Tue, 5 Jun 2018 23:47:52 +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!

> > udev solves device discovery pretty well; I don't think that's good
> > thing to optimize for.
> 
> It's about grouping related devices together, devices which share some
> common functionality. In this case, providing location data from some
> satellite system. I really don't understand how you can find having a
> class type named "gnss" for this to be controversial in any way.

We normally group devices by interface, not by functionality.

> > (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). 
> 
> As I've already explained, you may not know which protocol is currently
> active when the device start and you typically switch from NMEA to a
> vendor protocol during runtime (e.g. for configuration or to access raw
> GNSS data).
> 
> Trying to encode the GNSS protocol in the character-device node name is
> just a bad idea.

I thought idea was to switch to the "best" protocol in kernel.

> > > 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 :-).
> 
> I find naming a subsystem for GNSS receivers "gnss" to be very
> reasonable. ;)

I don't. I'll explain why below.

> > 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...
> 
> As we also already discussed, there's nothing preventing you from trying
> to move gpsd into the kernel later. I doubt this is something we want,
> and it may turn out not to be feasible for other reasons.

Well -- I believe we want "gpsd in kernel". If you take /dev/gnss0 and
drivers/gnss now, where do I put them?

> And as you already acknowledged, this series does solve a problem we
> have today.

Yes. I'm not disputing that.

> > Anyway, it is trivial binary protocol over packets that are used for
> > modem communication. Handling it is like 40? lines of code.
> 
> Ok, so I did read the damn thing along with the overview of the N900 GPS
> hardware and reverse-engineered protocol (why didn't you point me to
> those instead?) and I'm still not sure what the point you're trying to
> make is.

Umm. Where did you find the hardware overview/protocol overview?

> The n900 service you link to above, parses phonet data, does some
> *floating point* calculations and generates NMEA sentences which it
> feeds to a pseudo terminal whose slave side is opened by gpsd.
> 
> That NMEA data could just as easily be fed through a different kernel
> subsystem, namely gnss instead of tty, where it could be accessed
> through a common interface (for now, a raw gnss interface, with some
> associated meta data). [ And from what I can tell, ugnss would also
> allow you to get rid of some hacks related to finding out when the GNSS
> is opened and needs to be powered on. ]
> 
> So the ugnss interface looks like it will work for N900 just as it will
> for other phones.

Not NMEA please. First, NMEA has strange format of latitude/longitude
-- that's those calculations IIRC. NMEA has other problems, too --
like not atomically providing speeds and accuracies. Plus it is crazy
text protoco with CRCs.

> > NMEA would not be my first choice, really. I'd propose something very
> > similar to existing /dev/input/eventX, but with wider data types.
> 
> Fine, you pursue that idea if you want then. I can see many problems
> with trying to so, but the point is, this series doesn't prevent you
> from doing so.

> If you think you'll one day be able to parse and repackage the various
> vendor protocols within the kernel, you can consider the raw gnss
> interface as an intermediate step (which may continue to exist in
> parallel just as say hidraw).
> 
> As I've already mentioned, I considered naming the device nodes
> /dev/gnssraw0 partly because it would leave /dev/gnss namespace free for
> any such future development. I ultimately found that idea to be too
> implausible for it to be worth the potential confusion arising from the
> fact that "raw" GNSS data is already has an established meaning.
> 
> But in the end, this is just name bike shedding.

So I agree /dev/gnssraw is not great. But /dev/gnss is even worse. And
yes, it is "just" a naming, but it will be impossible to fix later.

/dev/serdev? /dev/gnssproto?

> > 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.
> 
> If that's the concern, we could reconsider naming the raw protocol
> interface /dev/gnnsraw0 as mentioned above.

I prfer /dev/gnssraw0 to /dev/gnss0.

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