All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Andreas Kemnade <andreas@kemnade.info>,
	Arnd Bergmann <arnd@arndb.de>,
	"H . Nikolaus Schaller" <hns@goldelico.com>,
	Marcel Holtmann <marcel@holtmann.org>,
	Sebastian Reichel <sebastian.reichel@collabora.co.uk>,
	Tony Lindgren <tony@atomide.com>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 0/8] gnss: add new GNSS subsystem
Date: Tue, 5 Jun 2018 23:47:52 +0200	[thread overview]
Message-ID: <20180605214752.GB28143@amd> (raw)
In-Reply-To: <20180604102238.GC13775@localhost>

[-- Attachment #1: Type: text/plain, Size: 5423 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  reply	other threads:[~2018-06-05 21:47 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-01  8:22 [PATCH v3 0/8] gnss: add new GNSS subsystem Johan Hovold
2018-06-01  8:22 ` [PATCH v3 1/8] gnss: add GNSS receiver subsystem Johan Hovold
2018-06-01  8:22 ` [PATCH v3 2/8] dt-bindings: add generic gnss binding Johan Hovold
2018-06-01  8:22 ` [PATCH v3 3/8] gnss: add generic serial driver Johan Hovold
2018-06-01  8:22 ` [PATCH v3 4/8] dt-bindings: gnss: add u-blox binding Johan Hovold
2018-06-01 14:34   ` Rob Herring
2018-06-01  8:22 ` [PATCH v3 5/8] gnss: add driver for u-blox receivers Johan Hovold
2018-06-01  8:22 ` [PATCH v3 6/8] dt-bindings: gnss: add sirfstar binding Johan Hovold
2018-06-01  8:22 ` [PATCH v3 7/8] gnss: add driver for sirfstar-based receivers Johan Hovold
2018-06-01  8:22 ` [PATCH v3 8/8] gnss: add receiver type support Johan Hovold
2018-06-01  9:33 ` [PATCH v3 0/8] gnss: add new GNSS subsystem Pavel Machek
2018-06-01  9:49   ` Johan Hovold
2018-06-01 10:26     ` Pavel Machek
2018-06-01 12:22       ` Johan Hovold
2018-06-01 16:26         ` Pavel Machek
2018-06-01 16:37           ` H. Nikolaus Schaller
2018-06-01 21:46             ` Pavel Machek
2018-06-04 10:22           ` Johan Hovold
2018-06-05 21:47             ` Pavel Machek [this message]
2018-06-13  8:21               ` Johan Hovold
2018-06-28 12:01 ` Greg Kroah-Hartman
2018-06-29  9:46   ` Pavel Machek
2018-06-29 11:46     ` Johan Hovold
2018-06-29 12:05       ` Pavel Machek
2018-06-29 12:09         ` Johan Hovold
2018-07-02 19:32           ` Pavel Machek
2018-07-03  7:20             ` Johan Hovold
2018-06-29 18:26     ` Greg Kroah-Hartman
2018-07-02 19:01       ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180605214752.GB28143@amd \
    --to=pavel@ucw.cz \
    --cc=andreas@kemnade.info \
    --cc=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hns@goldelico.com \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sebastian.reichel@collabora.co.uk \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.