Re: [PATCH v3 2/4] dt-bindings: gnss: add mediatek binding

From: Johan Hovold <johan@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Loys Ollivier <lollivier@baylibre.com>,
	Johan Hovold <johan@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 2/4] dt-bindings: gnss: add mediatek binding
Date: Mon, 11 Feb 2019 10:22:51 +0100	[thread overview]
Message-ID: <20190211092251.GV4686@localhost> (raw)
In-Reply-To: <20190130155016.GA26536@bogus>

On Wed, Jan 30, 2019 at 09:50:16AM -0600, Rob Herring wrote:
> On Mon, Jan 28, 2019 at 05:37:59PM +0100, Loys Ollivier wrote:
> > Add binding for Mediatek-based GNSS receivers.
> > 
> > Signed-off-by: Loys Ollivier <lollivier@baylibre.com>
> > ---
> > 
> > v3:
> > Removed the I2C and SPI interfaces references.
> > Removed the "mediatek,mt3339" compatible line.
> > Added the optional propertied (reset, fix)
> > Added the pin names for the optional properties.
> > 
> > v2:
> > Renamed bindings from Globaltop/gtop to Mediatek/mtk.
> > Moved current-speed as an optional propertie.
> > Removed the status line in the example.
> > Added "mediatek,mt3339" compatible.
> > 
> >  .../devicetree/bindings/gnss/mediatek.txt          | 37 ++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gnss/mediatek.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gnss/mediatek.txt b/Documentation/devicetree/bindings/gnss/mediatek.txt
> > new file mode 100644
> > index 000000000000..00650d81c5c8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gnss/mediatek.txt
> > @@ -0,0 +1,37 @@
> > +Mediatek-based GNSS Receiver DT binding
> > +
> > +Mediatek chipsets are used in GNSS-receiver modules produced by several
> > +vendors and can use an UART interface.

s/an/a/

> > +
> > +Please see Documentation/devicetree/bindings/gnss/gnss.txt for generic
> > +properties.
> > +
> > +Required properties:
> > +
> > +- compatible	: Must be
> > +
> > +			"globaltop,pa6h"
> > +
> > +- vcc-supply	: Main voltage regulator (pin name: VCC)
> > +
> > +Optional properties:
> > +
> > +- gtop,reset-gpios	: GPIO used to reset the device
> > +  			  (pin name: RESET, NRESET)
> 
> reset-gpios is the standard name.
> 
> > +- gtop,fix-gpios	: GPIO used to determine device position fix state
> > +			  (pin name: FIX, 3D_FIX)

I'm thinking we might need a standard name for fix-gpios as well. This
isn't a feature that is globaltop (or mediatek) specific, and other
manufacturers provide similar functionality through pins named UI_FIX,
or even LCKIND (lock indicator).

The pin is typically used to drive an indicator LED AFAIU, but the
characteristics of the signal varies from device to device (fw to fw),
for example, pulse when no lock and low otherwise, or pulse when a 2d or
3d-fix is acquired, etc.

I'm not sure how useful the pulsing would be for software, but someone
might find a use for it in some form.

Rob, is this something we want to keep in the binding, and if so, how
about using a generic name such as "gnss-fix-gpios"?

Johan

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic