Re: [PATCH 4/7] dt-bindings: gnss: add u-blox binding

[Johan Hovold <johan@kernel.org>]

On Wed, Apr 25, 2018 at 01:16:58PM -0500, Rob Herring wrote:
> On Tue, Apr 24, 2018 at 11:34 AM, Johan Hovold <johan@kernel.org> wrote:
> > Add binding for u-blox GNSS receivers.
> >
> > Note that the u-blox product names encodes form factor (e.g. "neo"),
> > chipset (e.g. "8") and variant (e.g. "q"), but that only formfactor and
> > chipset is used for the compatible strings (for now).
> >
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  .../devicetree/bindings/gnss/u-blox.txt       | 31 +++++++++++++++++++
> >  .../devicetree/bindings/vendor-prefixes.txt   |  1 +
> >  2 files changed, 32 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gnss/u-blox.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gnss/u-blox.txt b/Documentation/devicetree/bindings/gnss/u-blox.txt
> > new file mode 100644
> > index 000000000000..bb54b83a177f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
> > @@ -0,0 +1,31 @@
> > +u-blox GNSS Receiver DT binding
> > +
> > +The u-blox GNSS receivers can use UART, DDC (I2C), SPI and USB interfaces.
> > +
> > +Please see Documentation/devicetree/bindings/gnss/gnss.txt for generic
> > +properties.
> > +
> > +Required Properties:
> > +
> > +- compatible   : Must be one of
> > +
> > +                       "u-blox,neo-8"
> > +                       "u-blox,neo-m8"
> > +
> > +- vcc-supply   : Main voltage regulator (VCC)
> 
> What about V_BCKP?

That's the backup supply for for the RTC and batter-backed RAM. In
configurations where a battery is not used it should be connected to
VCC.

How would you model that? I can enable a vbckp regulator at probe, but
what if someone then accurately describes the corresponding pin as being
connected to VCC? I guess we can check if the regulators are identical,
and then just have the driver ignore V_BKUP. Knowing whether there is
a (hopefully charged) battery connected could be useful.

I can't seem to find any other bindings that describe battery supplies,
but I'll add it here.

> > +
> > +Optional Properties:
> 
> reg is required if using I2C, SPI, or USB.

I'll add that (even if there is no driver support for these yet).

> Datasheet also shows an interrupt pin.

Not used currently, but I'll add it to the binding as well.

> > +
> > +- timepulse-gpios      : Timepulse (e.g. 1PPS) GPIO (TIMEPULSE)
> 
> Why the 3rd "TIMEPULSE"?

That's the pin name, which in this case is identical to the property
name, so I'll drop it here.

Take a look at the sirf binding; vendors use different names for their
timepulse pins and in that case I added the actual pin names (1PPS, TM)
in parenthesis after the description.

Note that I mentioned "timepulse-gpios" in the generic binding with the
intent of trying to enforce a generic name for pins with such a
function (similarly for "enable-gpios", which I guess is already
established).

Thanks,
Johan