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

[Johan Hovold]

On Wed, May 02, 2018 at 08:16:11AM -0500, Rob Herring wrote:
> On Wed, May 2, 2018 at 3:16 AM, Johan Hovold <johan@xxxxxxxxxx> wrote:
> > On Tue, May 01, 2018 at 09:05:42AM -0500, Rob Herring wrote:
> >> On Thu, Apr 26, 2018 at 4:10 AM, Johan Hovold <johan@xxxxxxxxxx> wrote:
> >> > 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@xxxxxxxxxx> wrote:
> >> >> > Add binding for u-blox GNSS receivers.

> >> >> > +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?

> > ...my point was that in case there's no backup battery, you don't want
> > to enable vcc (via v_bckp) at probe (and instead have the device cold
> > boot whenever it's used).
> 
> Wouldn't that result in very long acquisition times? I guess I was
> thinking Vcc would be always on when running and V_bckp was just for
> suspend.

Yes, the acquisition times would certainly be longer, but for a GNSS
receiver which is rarely used that might still be preferred. And since
I'm using runtime pm to manage the supply, this policy decision can be
deferred to user space and controlled through the power/control
attribute.

> > Hence, the driver would need to check if the v_bckp-supply is identical
> > to vcc and not enable the former at probe in that case (i.e. similar to
> > if no v_bckp had been specified and we considered it optional).
> 
> I guess if that's the intended operation, then making it optional is fine.

Ok.

> >> > 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).
> >>
> >> Yes, I think that's good.
> >>
> >> Though with the enable-gpios I was debating the name for sirfstar a
> >> bit because it isn't the normal drive it active to enable, but rather
> >> a pulse to enable or disable.
> >
> > I had some concerns along those lines as well, and if you agree I'll
> > change the property name to on_off-gpios (or onoff-gpios) which matches
> > the vendor data sheets for this pin, and which I think would be better.
> 
> Okay, just add a vendor prefix. And note that using '_' is discouraged.

Ok, I'll name it "sirf,onoff-gpios".

Thanks,
Johan