[0/4] Add driver for globaltop GNSS receivers
diff mbox

Message ID 1545994235-8309-1-git-send-email-lollivier@baylibre.com
State Not Applicable
Headers show

Commit Message

Loys Ollivier Dec. 28, 2018, 10:50 a.m. UTC
Hello !

This patch series adds a new GNSS driver for the globaltop GNSS receivers.
These receivers transmits NMEA output sequence as soon as they have booted.
Power management can be done via the main supply and optional backup supply
as defined in the device tree.

The driver has been tested using a GlobalTop pa6h chipset on a Libretech-cc
board using the expansion header. Changes made in the board device tree can
be found below for reference and testing.

Loys

Loys Ollivier (4):
  dt-bindings: Add vendor prefix for "GlobalTop Technology, Inc."
  dt-bindings: gnss: add gtop binding
  gnss: add gtop receiver type support
  gnss: add driver for globaltop receivers

 Documentation/devicetree/bindings/gnss/gtop.txt    |  33 +++++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/gnss/Kconfig                               |  13 ++
 drivers/gnss/Makefile                              |   3 +
 drivers/gnss/core.c                                |   1 +
 drivers/gnss/gtop.c                                | 152 +++++++++++++++++++++
 include/linux/gnss.h                               |   1 +
 7 files changed, 204 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gnss/gtop.txt
 create mode 100644 drivers/gnss/gtop.c

---
2.7.4

Comments

Loys Ollivier Jan. 3, 2019, 4:04 p.m. UTC | #1
As suggested by Neil Armstrong - this GNSS device seems to behave 
like
most generic GNSS receivers.
I'll send a v2 with a generic driver that works for both GlobalTop 
and
u-blox.

Please ignore this serie.

Thanks,

Loys

Loys Ollivier writes:

> Hello !
>
> This patch series adds a new GNSS driver for the globaltop GNSS 
> receivers.
> These receivers transmits NMEA output sequence as soon as they 
> have booted.
> Power management can be done via the main supply and optional 
> backup supply
> as defined in the device tree.
>
> The driver has been tested using a GlobalTop pa6h chipset on a 
> Libretech-cc
> board using the expansion header. Changes made in the board 
> device tree can
> be found below for reference and testing.
>
> Loys
>
> Loys Ollivier (4):
>   dt-bindings: Add vendor prefix for "GlobalTop Technology, 
>   Inc."
>   dt-bindings: gnss: add gtop binding
>   gnss: add gtop receiver type support
>   gnss: add driver for globaltop receivers
>
>  Documentation/devicetree/bindings/gnss/gtop.txt    |  33 +++++
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>  drivers/gnss/Kconfig                               |  13 ++
>  drivers/gnss/Makefile                              |   3 +
>  drivers/gnss/core.c                                |   1 +
>  drivers/gnss/gtop.c                                | 152 
>  +++++++++++++++++++++
>  include/linux/gnss.h                               |   1 +
>  7 files changed, 204 insertions(+)
>  create mode 100644 
>  Documentation/devicetree/bindings/gnss/gtop.txt
>  create mode 100644 drivers/gnss/gtop.c
>
> ---
> diff --git 
> a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts 
> b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
> index 90a56af967a7..3b3d4dcc47aa 100644
> --- 
> a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
> +++ 
> b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
> @@ -17,6 +17,7 @@
>  
>         aliases {
>                 serial0 = &uart_AO;
> +               serial1 = &uart_A;
>                 ethernet0 = &ethmac;
>         };
>  
> @@ -269,7 +270,20 @@
>         pinctrl-names = "default";
>  };
>  
> +/* This is brought out on the UART_A_TX (8) and UART_A_RX (10) 
> pins: */
> +&uart_A {
> +       status = "okay";
> +       pinctrl-0 = <&uart_a_pins>;
> +       pinctrl-names = "default";
> +
> +       gnss {
> +               compatible = "globaltop,pa6h";
> +               v-bckp-supply = <&vcc_3v3>;
> +               vcc-supply = <&vcc_3v3>;
> +               current-speed = <9600>;
> +       };
> +};
> +
>  &usb0 {
>         status = "okay";
>  };
Johan Hovold Jan. 4, 2019, 9:19 a.m. UTC | #2
On Thu, Jan 03, 2019 at 05:04:35PM +0100, Loys Ollivier wrote:
> 
> As suggested by Neil Armstrong - this GNSS device seems to behave like
> most generic GNSS receivers.
> I'll send a v2 with a generic driver that works for both GlobalTop and
> u-blox.

That doesn't sound right to me. Judging from a quick look, this device
appears to be based on a mediatek chipset, so I suggest reworking
(renaming) this as a Mediatek driver (gnss type would be MTK, reflecting
the vendor protocol) even if it happens to look a lot like the current
u-blox driver.

If we ever add more features, like support for some part of the vendor
protocol or maybe 1pps, things will start diverging. As you noticed you
already have the generic serial gnss implementation to handle a lot of
the common bits.

> Loys Ollivier writes:
> 
> > Hello !
> >
> > This patch series adds a new GNSS driver for the globaltop GNSS
> > receivers.  These receivers transmits NMEA output sequence as soon
> > as they have booted.  Power management can be done via the main
> > supply and optional backup supply as defined in the device tree.
> >
> > The driver has been tested using a GlobalTop pa6h chipset on a
> > Libretech-cc board using the expansion header. Changes made in the
> > board device tree can be found below for reference and testing.

Thanks,
Johan
Rob Herring Jan. 4, 2019, 5:51 p.m. UTC | #3
On Fri, Dec 28, 2018 at 4:51 AM Loys Ollivier <lollivier@baylibre.com> wrote:
>
> Hello !
>
> This patch series adds a new GNSS driver for the globaltop GNSS receivers.
> These receivers transmits NMEA output sequence as soon as they have booted.
> Power management can be done via the main supply and optional backup supply
> as defined in the device tree.
>
> The driver has been tested using a GlobalTop pa6h chipset on a Libretech-cc
> board using the expansion header. Changes made in the board device tree can
> be found below for reference and testing.
>
> Loys
>
> Loys Ollivier (4):
>   dt-bindings: Add vendor prefix for "GlobalTop Technology, Inc."
>   dt-bindings: gnss: add gtop binding
>   gnss: add gtop receiver type support
>   gnss: add driver for globaltop receivers
>
>  Documentation/devicetree/bindings/gnss/gtop.txt    |  33 +++++
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>  drivers/gnss/Kconfig                               |  13 ++
>  drivers/gnss/Makefile                              |   3 +
>  drivers/gnss/core.c                                |   1 +
>  drivers/gnss/gtop.c                                | 152 +++++++++++++++++++++
>  include/linux/gnss.h                               |   1 +
>  7 files changed, 204 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gnss/gtop.txt
>  create mode 100644 drivers/gnss/gtop.c
>
> ---
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
> index 90a56af967a7..3b3d4dcc47aa 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
> @@ -17,6 +17,7 @@
>
>         aliases {
>                 serial0 = &uart_AO;
> +               serial1 = &uart_A;

You should not need this as the UART is not used as a tty. It could be
that the serial driver requires aliases, but that should be fixed.
There should be some examples of fixing this in other serial drivers.

>                 ethernet0 = &ethmac;
>         };
Loys Ollivier Jan. 7, 2019, 4:21 p.m. UTC | #4
On Fri 04 Jan 2019 at 09:19, Johan Hovold wrote:

> On Thu, Jan 03, 2019 at 05:04:35PM +0100, Loys Ollivier wrote:
>> 
>> As suggested by Neil Armstrong - this GNSS device seems to behave like
>> most generic GNSS receivers.
>> I'll send a v2 with a generic driver that works for both GlobalTop and
>> u-blox.
>
> That doesn't sound right to me. Judging from a quick look, this device
> appears to be based on a mediatek chipset, so I suggest reworking
> (renaming) this as a Mediatek driver (gnss type would be MTK, reflecting
> the vendor protocol) even if it happens to look a lot like the current
> u-blox driver.
>
OK, agreed - will rename the driver as Mediatek.
Would you recommend keeping any reference to Globaltop ? Such as in the
devicetree gnss bindings.
Or only keep "mediatek,mt3339" for any solution based on that chipset.
> If we ever add more features, like support for some part of the vendor
> protocol or maybe 1pps, things will start diverging. As you noticed you
> already have the generic serial gnss implementation to handle a lot of
> the common bits.
Yes, I would like indeed to look into 1pps after that patch series.
>
>> Loys Ollivier writes:
>> 
>> > Hello !
>> >
>> > This patch series adds a new GNSS driver for the globaltop GNSS
>> > receivers.  These receivers transmits NMEA output sequence as soon
>> > as they have booted.  Power management can be done via the main
>> > supply and optional backup supply as defined in the device tree.
>> >
>> > The driver has been tested using a GlobalTop pa6h chipset on a
>> > Libretech-cc board using the expansion header. Changes made in the
>> > board device tree can be found below for reference and testing.
>
> Thanks,
> Johan

Will send a v2.
Johan Hovold Jan. 7, 2019, 4:37 p.m. UTC | #5
On Mon, Jan 07, 2019 at 05:21:58PM +0100, Loys Ollivier wrote:
> 
> On Fri 04 Jan 2019 at 09:19, Johan Hovold wrote:
> 
> > On Thu, Jan 03, 2019 at 05:04:35PM +0100, Loys Ollivier wrote:
> >> 
> >> As suggested by Neil Armstrong - this GNSS device seems to behave like
> >> most generic GNSS receivers.
> >> I'll send a v2 with a generic driver that works for both GlobalTop and
> >> u-blox.
> >
> > That doesn't sound right to me. Judging from a quick look, this device
> > appears to be based on a mediatek chipset, so I suggest reworking
> > (renaming) this as a Mediatek driver (gnss type would be MTK, reflecting
> > the vendor protocol) even if it happens to look a lot like the current
> > u-blox driver.
> >
> OK, agreed - will rename the driver as Mediatek.
> Would you recommend keeping any reference to Globaltop ? Such as in the
> devicetree gnss bindings.
> Or only keep "mediatek,mt3339" for any solution based on that chipset.

Yes, I suggest keeping the globaltop compatible (cf. the sirf driver for
devices based on sirf chipsets).

Thanks,
Johan
Loys Ollivier Jan. 15, 2019, 10 a.m. UTC | #6
On Fri 04 Jan 2019 at 11:51, Rob Herring <robh+dt@kernel.org> wrote:

> On Fri, Dec 28, 2018 at 4:51 AM Loys Ollivier <lollivier@baylibre.com> wrote:
>>
>> Hello !
>>
>> This patch series adds a new GNSS driver for the globaltop GNSS receivers.
>> These receivers transmits NMEA output sequence as soon as they have booted.
>> Power management can be done via the main supply and optional backup supply
>> as defined in the device tree.
>>
>> The driver has been tested using a GlobalTop pa6h chipset on a Libretech-cc
>> board using the expansion header. Changes made in the board device tree can
>> be found below for reference and testing.
>>
>> Loys
>>
>> Loys Ollivier (4):
>>   dt-bindings: Add vendor prefix for "GlobalTop Technology, Inc."
>>   dt-bindings: gnss: add gtop binding
>>   gnss: add gtop receiver type support
>>   gnss: add driver for globaltop receivers
>>
>>  Documentation/devicetree/bindings/gnss/gtop.txt    |  33 +++++
>>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>>  drivers/gnss/Kconfig                               |  13 ++
>>  drivers/gnss/Makefile                              |   3 +
>>  drivers/gnss/core.c                                |   1 +
>>  drivers/gnss/gtop.c                                | 152 +++++++++++++++++++++
>>  include/linux/gnss.h                               |   1 +
>>  7 files changed, 204 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gnss/gtop.txt
>>  create mode 100644 drivers/gnss/gtop.c
>>
>> ---
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
>> index 90a56af967a7..3b3d4dcc47aa 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
>> @@ -17,6 +17,7 @@
>>
>>         aliases {
>>                 serial0 = &uart_AO;
>> +               serial1 = &uart_A;
>
> You should not need this as the UART is not used as a tty. It could be
> that the serial driver requires aliases, but that should be fixed.
> There should be some examples of fixing this in other serial drivers.
>

Thanks Rob,
Followed up on that [0]. 

[0]: https://lore.kernel.org/linux-amlogic/1547484866-3600-1-git-send-email-lollivier@baylibre.com/
>>                 ethernet0 = &ethmac;
>>         };
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Patch
diff mbox

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
index 90a56af967a7..3b3d4dcc47aa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts
@@ -17,6 +17,7 @@ 
 
        aliases {
                serial0 = &uart_AO;
+               serial1 = &uart_A;
                ethernet0 = &ethmac;
        };
 
@@ -269,7 +270,20 @@ 
        pinctrl-names = "default";
 };
 
+/* This is brought out on the UART_A_TX (8) and UART_A_RX (10) pins: */
+&uart_A {
+       status = "okay";
+       pinctrl-0 = <&uart_a_pins>;
+       pinctrl-names = "default";
+
+       gnss {
+               compatible = "globaltop,pa6h";
+               v-bckp-supply = <&vcc_3v3>;
+               vcc-supply = <&vcc_3v3>;
+               current-speed = <9600>;
+       };
+};
+
 &usb0 {
        status = "okay";
 };
--