diff mbox series

[v2,4/8] dt-bindings: gnss: add u-blox binding

Message ID 20180530103242.20773-5-johan@kernel.org
State Not Applicable, archived
Headers show
Series gnss: add new GNSS subsystem | expand

Commit Message

Johan Hovold May 30, 2018, 10:32 a.m. UTC
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       | 44 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt   |  1 +
 2 files changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gnss/u-blox.txt

Comments

Rob Herring (Arm) May 31, 2018, 3:58 a.m. UTC | #1
On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold 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       | 44 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.txt   |  1 +
>  2 files changed, 45 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..caef9ace0b0c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
> @@ -0,0 +1,44 @@
> +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

mixture of space and tab here.

> +
> +			"u-blox,neo-8"
> +			"u-blox,neo-m8"
> +
> +- vcc-supply	: Main voltage regulator
> +
> +Required properties (DDC):
> +- reg		: DDC (I2C) slave address
> +
> +Required properties (SPI):
> +- reg		: SPI chip select address
> +
> +Required properties (USB):
> +- reg		: Number of the USB hub port or the USB host-controller port
> +                  to which this device is attached
> +
> +Optional properties:
> +
> +- timepulse-gpios	: Time pulse GPIO
> +- u-blox,extint-gpios	: External interrupt GPIO

This should be interrupts property instead of a gpio.

> +- v-bckp-supply	: Backup voltage regulator
> +
> +Example:
> +
> +serial@1234 {
> +	compatible = "ns16550a";
> +
> +	gnss {
> +		compatible = "u-blox,neo-8";
> +
> +		v-bckp-supply = <&gnss_v_bckp_reg>;
> +		vcc-supply = <&gnss_vcc_reg>;
> +	};
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index b5f978a4cac6..2128dfdf73f1 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -374,6 +374,7 @@ tronsmart	Tronsmart
>  truly	Truly Semiconductors Limited
>  tsd	Theobroma Systems Design und Consulting GmbH
>  tyan	Tyan Computer Corporation
> +u-blox	u-blox
>  ucrobotics	uCRobotics
>  ubnt	Ubiquiti Networks
>  udoo	Udoo
> -- 
> 2.17.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hovold May 31, 2018, 8:22 a.m. UTC | #2
On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold 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       | 44 +++++++++++++++++++
> >  .../devicetree/bindings/vendor-prefixes.txt   |  1 +
> >  2 files changed, 45 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..caef9ace0b0c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
> > @@ -0,0 +1,44 @@
> > +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
> 
> mixture of space and tab here.

Oops. Same single space character before the tab here in all three
binding docs (and in the in-tree slave_devices.txt which I think I used
as a template).

Do you want me to fix this even if this turns out to be the only thing
that needs to be addressed in a v3?

> > +
> > +			"u-blox,neo-8"
> > +			"u-blox,neo-m8"
> > +
> > +- vcc-supply	: Main voltage regulator
> > +
> > +Required properties (DDC):
> > +- reg		: DDC (I2C) slave address
> > +
> > +Required properties (SPI):
> > +- reg		: SPI chip select address
> > +
> > +Required properties (USB):
> > +- reg		: Number of the USB hub port or the USB host-controller port
> > +                  to which this device is attached
> > +
> > +Optional properties:
> > +
> > +- timepulse-gpios	: Time pulse GPIO
> > +- u-blox,extint-gpios	: External interrupt GPIO
> 
> This should be interrupts property instead of a gpio.

Contrary to what the name may suggest, this pin is actually an input
which can be used to control active power or to provide time or
frequency aiding data to the receiver (see section 1.13 in [1]).

I only added it for completeness as the driver does not use it
currently. Remove, leave as is, or add "input" to the description as in:

	- u-blox,extint-gpios	: External interrupt input GPIO

perhaps?

Thanks,
Johan

[1] https://www.u-blox.com/sites/default/files/NEO-8Q_DataSheet_%28UBX-15031913%29.pdf
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) May 31, 2018, 1:55 p.m. UTC | #3
On Thu, May 31, 2018 at 3:22 AM, Johan Hovold <johan@kernel.org> wrote:
> On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
>> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold 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       | 44 +++++++++++++++++++
>> >  .../devicetree/bindings/vendor-prefixes.txt   |  1 +
>> >  2 files changed, 45 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..caef9ace0b0c
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
>> > @@ -0,0 +1,44 @@
>> > +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
>>
>> mixture of space and tab here.
>
> Oops. Same single space character before the tab here in all three
> binding docs (and in the in-tree slave_devices.txt which I think I used
> as a template).

Who wrote that crap? ;)

> Do you want me to fix this even if this turns out to be the only thing
> that needs to be addressed in a v3?

No

>
>> > +
>> > +                   "u-blox,neo-8"
>> > +                   "u-blox,neo-m8"
>> > +
>> > +- vcc-supply       : Main voltage regulator
>> > +
>> > +Required properties (DDC):
>> > +- reg              : DDC (I2C) slave address
>> > +
>> > +Required properties (SPI):
>> > +- reg              : SPI chip select address
>> > +
>> > +Required properties (USB):
>> > +- reg              : Number of the USB hub port or the USB host-controller port
>> > +                  to which this device is attached
>> > +
>> > +Optional properties:
>> > +
>> > +- timepulse-gpios  : Time pulse GPIO
>> > +- u-blox,extint-gpios      : External interrupt GPIO
>>
>> This should be interrupts property instead of a gpio.
>
> Contrary to what the name may suggest, this pin is actually an input
> which can be used to control active power or to provide time or
> frequency aiding data to the receiver (see section 1.13 in [1]).
>
> I only added it for completeness as the driver does not use it
> currently. Remove, leave as is, or add "input" to the description as in:
>
>         - u-blox,extint-gpios   : External interrupt input GPIO

Yes. You should also define the active level.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hovold May 31, 2018, 2:34 p.m. UTC | #4
On Thu, May 31, 2018 at 08:55:10AM -0500, Rob Herring wrote:
> On Thu, May 31, 2018 at 3:22 AM, Johan Hovold <johan@kernel.org> wrote:
> > On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
> >> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold 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       | 44 +++++++++++++++++++
> >> >  .../devicetree/bindings/vendor-prefixes.txt   |  1 +
> >> >  2 files changed, 45 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..caef9ace0b0c
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
> >> > @@ -0,0 +1,44 @@
> >> > +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
> >>
> >> mixture of space and tab here.
> >
> > Oops. Same single space character before the tab here in all three
> > binding docs (and in the in-tree slave_devices.txt which I think I used
> > as a template).
> 
> Who wrote that crap? ;)

Heh.

> >
> >> > +
> >> > +                   "u-blox,neo-8"
> >> > +                   "u-blox,neo-m8"
> >> > +
> >> > +- vcc-supply       : Main voltage regulator
> >> > +
> >> > +Required properties (DDC):
> >> > +- reg              : DDC (I2C) slave address
> >> > +
> >> > +Required properties (SPI):
> >> > +- reg              : SPI chip select address
> >> > +
> >> > +Required properties (USB):
> >> > +- reg              : Number of the USB hub port or the USB host-controller port
> >> > +                  to which this device is attached
> >> > +
> >> > +Optional properties:
> >> > +
> >> > +- timepulse-gpios  : Time pulse GPIO
> >> > +- u-blox,extint-gpios      : External interrupt GPIO
> >>
> >> This should be interrupts property instead of a gpio.
> >
> > Contrary to what the name may suggest, this pin is actually an input
> > which can be used to control active power or to provide time or
> > frequency aiding data to the receiver (see section 1.13 in [1]).
> >
> > I only added it for completeness as the driver does not use it
> > currently. Remove, leave as is, or add "input" to the description as in:
> >
> >         - u-blox,extint-gpios   : External interrupt input GPIO
> 
> Yes. You should also define the active level.

The active level appears to be runtime configurable and dependent on the
selected function. For power control it can be used to control force-on
(active high), force-off (active low) or both; and for time aiding
sampling on falling or rising edge is also configurable. And who knows
what else this pin is used for next time they update the firmware. ;)

Shall I remove the property, or just add "input" as suggested above?

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) May 31, 2018, 3:58 p.m. UTC | #5
On Thu, May 31, 2018 at 9:34 AM, Johan Hovold <johan@kernel.org> wrote:
> On Thu, May 31, 2018 at 08:55:10AM -0500, Rob Herring wrote:
>> On Thu, May 31, 2018 at 3:22 AM, Johan Hovold <johan@kernel.org> wrote:
>> > On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
>> >> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold 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       | 44 +++++++++++++++++++
>> >> >  .../devicetree/bindings/vendor-prefixes.txt   |  1 +
>> >> >  2 files changed, 45 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..caef9ace0b0c
>> >> > --- /dev/null
>> >> > +++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
>> >> > @@ -0,0 +1,44 @@
>> >> > +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
>> >>
>> >> mixture of space and tab here.
>> >
>> > Oops. Same single space character before the tab here in all three
>> > binding docs (and in the in-tree slave_devices.txt which I think I used
>> > as a template).
>>
>> Who wrote that crap? ;)
>
> Heh.
>
>> >
>> >> > +
>> >> > +                   "u-blox,neo-8"
>> >> > +                   "u-blox,neo-m8"
>> >> > +
>> >> > +- vcc-supply       : Main voltage regulator
>> >> > +
>> >> > +Required properties (DDC):
>> >> > +- reg              : DDC (I2C) slave address
>> >> > +
>> >> > +Required properties (SPI):
>> >> > +- reg              : SPI chip select address
>> >> > +
>> >> > +Required properties (USB):
>> >> > +- reg              : Number of the USB hub port or the USB host-controller port
>> >> > +                  to which this device is attached
>> >> > +
>> >> > +Optional properties:
>> >> > +
>> >> > +- timepulse-gpios  : Time pulse GPIO
>> >> > +- u-blox,extint-gpios      : External interrupt GPIO
>> >>
>> >> This should be interrupts property instead of a gpio.
>> >
>> > Contrary to what the name may suggest, this pin is actually an input
>> > which can be used to control active power or to provide time or
>> > frequency aiding data to the receiver (see section 1.13 in [1]).
>> >
>> > I only added it for completeness as the driver does not use it
>> > currently. Remove, leave as is, or add "input" to the description as in:
>> >
>> >         - u-blox,extint-gpios   : External interrupt input GPIO
>>
>> Yes. You should also define the active level.
>
> The active level appears to be runtime configurable and dependent on the
> selected function. For power control it can be used to control force-on
> (active high), force-off (active low) or both; and for time aiding
> sampling on falling or rising edge is also configurable. And who knows
> what else this pin is used for next time they update the firmware. ;)

Okay.

> Shall I remove the property, or just add "input" as suggested above?

Just add "input".

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hovold June 1, 2018, 8:15 a.m. UTC | #6
On Thu, May 31, 2018 at 10:58:59AM -0500, Rob Herring wrote:
> On Thu, May 31, 2018 at 9:34 AM, Johan Hovold <johan@kernel.org> wrote:
> > On Thu, May 31, 2018 at 08:55:10AM -0500, Rob Herring wrote:
> >> On Thu, May 31, 2018 at 3:22 AM, Johan Hovold <johan@kernel.org> wrote:
> >> > On Wed, May 30, 2018 at 10:58:05PM -0500, Rob Herring wrote:
> >> >> On Wed, May 30, 2018 at 12:32:38PM +0200, Johan Hovold wrote:
> >> >> > Add binding for u-blox GNSS receivers.

> >> >> > +Optional properties:
> >> >> > +
> >> >> > +- timepulse-gpios  : Time pulse GPIO
> >> >> > +- u-blox,extint-gpios      : External interrupt GPIO
> >> >>
> >> >> This should be interrupts property instead of a gpio.
> >> >
> >> > Contrary to what the name may suggest, this pin is actually an input
> >> > which can be used to control active power or to provide time or
> >> > frequency aiding data to the receiver (see section 1.13 in [1]).
> >> >
> >> > I only added it for completeness as the driver does not use it
> >> > currently. Remove, leave as is, or add "input" to the description as in:
> >> >
> >> >         - u-blox,extint-gpios   : External interrupt input GPIO
> >>
> >> Yes. You should also define the active level.
> >
> > The active level appears to be runtime configurable and dependent on the
> > selected function. For power control it can be used to control force-on
> > (active high), force-off (active low) or both; and for time aiding
> > sampling on falling or rising edge is also configurable. And who knows
> > what else this pin is used for next time they update the firmware. ;)
> 
> Okay.
> 
> > Shall I remove the property, or just add "input" as suggested above?
> 
> Just add "input".

I ended up rewording this as

	u-blox,extint-gpios  : GPIO connected to the "external
			       interrupt" input pin

to avoid any confusion about the direction of the GPIO.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gnss/u-blox.txt b/Documentation/devicetree/bindings/gnss/u-blox.txt
new file mode 100644
index 000000000000..caef9ace0b0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
@@ -0,0 +1,44 @@ 
+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
+
+Required properties (DDC):
+- reg		: DDC (I2C) slave address
+
+Required properties (SPI):
+- reg		: SPI chip select address
+
+Required properties (USB):
+- reg		: Number of the USB hub port or the USB host-controller port
+                  to which this device is attached
+
+Optional properties:
+
+- timepulse-gpios	: Time pulse GPIO
+- u-blox,extint-gpios	: External interrupt GPIO
+- v-bckp-supply	: Backup voltage regulator
+
+Example:
+
+serial@1234 {
+	compatible = "ns16550a";
+
+	gnss {
+		compatible = "u-blox,neo-8";
+
+		v-bckp-supply = <&gnss_v_bckp_reg>;
+		vcc-supply = <&gnss_vcc_reg>;
+	};
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b5f978a4cac6..2128dfdf73f1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -374,6 +374,7 @@  tronsmart	Tronsmart
 truly	Truly Semiconductors Limited
 tsd	Theobroma Systems Design und Consulting GmbH
 tyan	Tyan Computer Corporation
+u-blox	u-blox
 ucrobotics	uCRobotics
 ubnt	Ubiquiti Networks
 udoo	Udoo