Hi,
This patch series adds a new GNSS driver for the Mediatek-based GNSS receivers.
These receivers transmits NMEA output sequence after boot.
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
Version 4 changes:
- dt: bindings: use standard name "reset-gpios"
- dt: bindings: use standard name "gnss-fix-gpios"
- dt: bindings: s/an/a UART
Version 3 changes [0]:
- driver: Removed "mediatek,mt3339" compatible
- driver: moved gnss_types_names string diff in the GNSS_TYPE patch
- driver: edited [patch 3/4] commit message to remove ref to vendor protocol
- driver: retained the original copyright
- dt: removed interfaces that are not available on the globaltop SoC
- dt: Added missing pins documentation
- dt: added optional properties pin names
Version 2 changes:
- driver: Renamed driver from Globaltop/gtop to Mediatek/mtk
- driver: Added "mediatek,mt3339" compatible
- dt: Renamed bindings from Globaltop to Mediatek
- dt: Moved the current-speed property as optional
- dt: removed the status line in example
- cover-letter: removed the alias that is not needed anymore
[0]: https://lkml.kernel.org/r/[email protected]
Loys Ollivier (4):
dt-bindings: Add vendor prefix for "GlobalTop Technology, Inc."
dt-bindings: gnss: add mediatek binding
gnss: add mtk receiver type support
gnss: add driver for mediatek receivers
.../devicetree/bindings/gnss/mediatek.txt | 37 +++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/gnss/Kconfig | 13 ++
drivers/gnss/Makefile | 3 +
drivers/gnss/core.c | 1 +
drivers/gnss/mtk.c | 152 +++++++++++++++++++++
include/linux/gnss.h | 1 +
7 files changed, 208 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gnss/mediatek.txt
create mode 100644 drivers/gnss/mtk.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;
ethernet0 = ðmac;
};
@@ -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";
};
--
2.7.4
Add an MTK (Mediatek) type to the "GNSS_TYPE" attribute.
Note that MTK receivers support a subset of NMEA 0183 with vendor
extensions.
Signed-off-by: Loys Ollivier <[email protected]>
---
v4:
No changes
v3:
Moved the gnss_type_names string addition to this patch.
Edited the commit message to remove the reference to a vendor protocol.
v2:
Renamed from GTOP to MTK.
drivers/gnss/core.c | 1 +
include/linux/gnss.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gnss/core.c b/drivers/gnss/core.c
index 4291a0dd22aa..320cfca80d5f 100644
--- a/drivers/gnss/core.c
+++ b/drivers/gnss/core.c
@@ -334,6 +334,7 @@ static const char * const gnss_type_names[GNSS_TYPE_COUNT] = {
[GNSS_TYPE_NMEA] = "NMEA",
[GNSS_TYPE_SIRF] = "SiRF",
[GNSS_TYPE_UBX] = "UBX",
+ [GNSS_TYPE_MTK] = "MTK",
};
static const char *gnss_type_name(struct gnss_device *gdev)
diff --git a/include/linux/gnss.h b/include/linux/gnss.h
index 43546977098c..36968a0f33e8 100644
--- a/include/linux/gnss.h
+++ b/include/linux/gnss.h
@@ -22,6 +22,7 @@ enum gnss_type {
GNSS_TYPE_NMEA = 0,
GNSS_TYPE_SIRF,
GNSS_TYPE_UBX,
+ GNSS_TYPE_MTK,
GNSS_TYPE_COUNT
};
--
2.7.4
Add driver for serial-connected Mediatek-based GNSS receivers.
These devices typically boot transmitting vendor specific NMEA output
sequences. The serial port bit rate is read from the device tree
"current-speed".
Note that the driver uses the generic GNSS serial implementation and
therefore essentially only manages power abstracted into three power
states: ACTIVE, STANDBY, and OFF.
For mediatek receivers with a main supply and no enable-gpios, this simply
means that the main supply is disabled in STANDBY and OFF (the optional
backup supply is kept enabled while the driver is bound).
Note that the timepulse-support is left unimplemented.
Signed-off-by: Loys Ollivier <[email protected]>
---
v4:
No changes
v3:
Sorted the Kconfig and Makefile
Retained the original Copyright
Removed "mediatek,mt3339" compatible.
v2:
Renamed from gtop/Globaltop to mtk/Mediatek.
Added "mediatek,mt3339" compatible.
drivers/gnss/Kconfig | 13 +++++
drivers/gnss/Makefile | 3 +
drivers/gnss/mtk.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 168 insertions(+)
create mode 100644 drivers/gnss/mtk.c
diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
index 6abc88514512..6d8c8027e1cd 100644
--- a/drivers/gnss/Kconfig
+++ b/drivers/gnss/Kconfig
@@ -15,6 +15,19 @@ if GNSS
config GNSS_SERIAL
tristate
+config GNSS_MTK_SERIAL
+ tristate "Mediatek GNSS receiver support"
+ depends on SERIAL_DEV_BUS
+ select GNSS_SERIAL
+ help
+ Say Y here if you have a Mediatek-based GNSS receiver which uses a
+ serial interface.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gnss-mtk.
+
+ If unsure, say N.
+
config GNSS_SIRF_SERIAL
tristate "SiRFstar GNSS receiver support"
depends on SERIAL_DEV_BUS
diff --git a/drivers/gnss/Makefile b/drivers/gnss/Makefile
index 5cf0ebe0330a..451f11401ecc 100644
--- a/drivers/gnss/Makefile
+++ b/drivers/gnss/Makefile
@@ -9,6 +9,9 @@ gnss-y := core.o
obj-$(CONFIG_GNSS_SERIAL) += gnss-serial.o
gnss-serial-y := serial.o
+obj-$(CONFIG_GNSS_MTK_SERIAL) += gnss-mtk.o
+gnss-mtk-y := mtk.o
+
obj-$(CONFIG_GNSS_SIRF_SERIAL) += gnss-sirf.o
gnss-sirf-y := sirf.o
diff --git a/drivers/gnss/mtk.c b/drivers/gnss/mtk.c
new file mode 100644
index 000000000000..a5aad08fb9d7
--- /dev/null
+++ b/drivers/gnss/mtk.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mediatek GNSS receiver driver
+ *
+ * Copyright (C) 2018 Johan Hovold <[email protected]>
+ */
+
+#include <linux/errno.h>
+#include <linux/gnss.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+#include <linux/serdev.h>
+
+#include "serial.h"
+
+struct mtk_data {
+ struct regulator *v_bckp;
+ struct regulator *vcc;
+};
+
+static int mtk_set_active(struct gnss_serial *gserial)
+{
+ struct mtk_data *data = gnss_serial_get_drvdata(gserial);
+ int ret;
+
+ ret = regulator_enable(data->vcc);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mtk_set_standby(struct gnss_serial *gserial)
+{
+ struct mtk_data *data = gnss_serial_get_drvdata(gserial);
+ int ret;
+
+ ret = regulator_disable(data->vcc);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mtk_set_power(struct gnss_serial *gserial,
+ enum gnss_serial_pm_state state)
+{
+ switch (state) {
+ case GNSS_SERIAL_ACTIVE:
+ return mtk_set_active(gserial);
+ case GNSS_SERIAL_OFF:
+ case GNSS_SERIAL_STANDBY:
+ return mtk_set_standby(gserial);
+ }
+
+ return -EINVAL;
+}
+
+static const struct gnss_serial_ops mtk_gserial_ops = {
+ .set_power = mtk_set_power,
+};
+
+static int mtk_probe(struct serdev_device *serdev)
+{
+ struct gnss_serial *gserial;
+ struct mtk_data *data;
+ int ret;
+
+ gserial = gnss_serial_allocate(serdev, sizeof(*data));
+ if (IS_ERR(gserial)) {
+ ret = PTR_ERR(gserial);
+ return ret;
+ }
+
+ gserial->ops = &mtk_gserial_ops;
+
+ gserial->gdev->type = GNSS_TYPE_MTK;
+
+ data = gnss_serial_get_drvdata(gserial);
+
+ data->vcc = devm_regulator_get(&serdev->dev, "vcc");
+ if (IS_ERR(data->vcc)) {
+ ret = PTR_ERR(data->vcc);
+ goto err_free_gserial;
+ }
+
+ data->v_bckp = devm_regulator_get_optional(&serdev->dev, "v-bckp");
+ if (IS_ERR(data->v_bckp)) {
+ ret = PTR_ERR(data->v_bckp);
+ if (ret == -ENODEV)
+ data->v_bckp = NULL;
+ else
+ goto err_free_gserial;
+ }
+
+ if (data->v_bckp) {
+ ret = regulator_enable(data->v_bckp);
+ if (ret)
+ goto err_free_gserial;
+ }
+
+ ret = gnss_serial_register(gserial);
+ if (ret)
+ goto err_disable_v_bckp;
+
+ return 0;
+
+err_disable_v_bckp:
+ if (data->v_bckp)
+ regulator_disable(data->v_bckp);
+err_free_gserial:
+ gnss_serial_free(gserial);
+
+ return ret;
+}
+
+static void mtk_remove(struct serdev_device *serdev)
+{
+ struct gnss_serial *gserial = serdev_device_get_drvdata(serdev);
+ struct mtk_data *data = gnss_serial_get_drvdata(gserial);
+
+ gnss_serial_deregister(gserial);
+ if (data->v_bckp)
+ regulator_disable(data->v_bckp);
+ gnss_serial_free(gserial);
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id mtk_of_match[] = {
+ { .compatible = "globaltop,pa6h" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mtk_of_match);
+#endif
+
+static struct serdev_device_driver mtk_driver = {
+ .driver = {
+ .name = "gnss-mtk",
+ .of_match_table = of_match_ptr(mtk_of_match),
+ .pm = &gnss_serial_pm_ops,
+ },
+ .probe = mtk_probe,
+ .remove = mtk_remove,
+};
+module_serdev_device_driver(mtk_driver);
+
+MODULE_AUTHOR("Loys Ollivier <[email protected]>");
+MODULE_DESCRIPTION("Mediatek GNSS receiver driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
Add binding for Mediatek-based GNSS receivers.
Signed-off-by: Loys Ollivier <[email protected]>
---
v4:
Use standard names for reset-gpios and gnss-fix-gpios
s/an/a UART
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..12283d429baa
--- /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 a UART interface.
+
+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:
+
+- current-speed : Default UART baud rate
+- gnss-fix-gpios : GPIO used to determine device position fix state
+ (pin name: FIX, 3D_FIX)
+- reset-gpios : GPIO used to reset the device
+ (pin name: RESET, NRESET)
+- timepulse-gpios : Time pulse GPIO (pin name: PPS1, 1PPS)
+- v-bckp-supply : Backup voltage regulator
+ (pin name: VBAT, VBACKUP)
+
+Example:
+
+serial@1234 {
+ compatible = "ns16550a";
+
+ gnss {
+ compatible = "globaltop,pa6h";
+ vcc-supply = <&vcc_3v3>;
+ };
+};
--
2.7.4
Add globaltop vendor definition.
Signed-off-by: Loys Ollivier <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
v4: No changes (Reviewed-by tag added)
v3: No changes
v2: Alphabetical order
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 389508584f48..d80a70343b36 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -150,6 +150,7 @@ geniatech Geniatech, Inc.
giantec Giantec Semiconductor, Inc.
giantplus Giantplus Technology Co., Ltd.
globalscale Globalscale Technologies, Inc.
+globaltop GlobalTop Technology, Inc.
gmt Global Mixed-mode Technology, Inc.
goodix Shenzhen Huiding Technology Co., Ltd.
google Google, Inc.
--
2.7.4
On Wed, Feb 13, 2019 at 04:09:27PM +0100, Loys Ollivier wrote:
> Add binding for Mediatek-based GNSS receivers.
>
> Signed-off-by: Loys Ollivier <[email protected]>
> ---
> v4:
> Use standard names for reset-gpios and gnss-fix-gpios
> s/an/a UART
>
> 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
Reviewed-by: Rob Herring <[email protected]>
On Wed, Feb 13, 2019 at 04:09:29PM +0100, Loys Ollivier wrote:
> Add driver for serial-connected Mediatek-based GNSS receivers.
>
> These devices typically boot transmitting vendor specific NMEA output
> sequences. The serial port bit rate is read from the device tree
> "current-speed".
>
> Note that the driver uses the generic GNSS serial implementation and
> therefore essentially only manages power abstracted into three power
> states: ACTIVE, STANDBY, and OFF.
>
> For mediatek receivers with a main supply and no enable-gpios, this simply
> means that the main supply is disabled in STANDBY and OFF (the optional
> backup supply is kept enabled while the driver is bound).
>
> Note that the timepulse-support is left unimplemented.
>
> Signed-off-by: Loys Ollivier <[email protected]>
> ---
> v4:
> No changes
>
> v3:
> Sorted the Kconfig and Makefile
> Retained the original Copyright
> Removed "mediatek,mt3339" compatible.
>
> v2:
> Renamed from gtop/Globaltop to mtk/Mediatek.
> Added "mediatek,mt3339" compatible.
>
> drivers/gnss/Kconfig | 13 +++++
> drivers/gnss/Makefile | 3 +
> drivers/gnss/mtk.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 168 insertions(+)
> create mode 100644 drivers/gnss/mtk.c
>
> diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
> index 6abc88514512..6d8c8027e1cd 100644
> --- a/drivers/gnss/Kconfig
> +++ b/drivers/gnss/Kconfig
> @@ -15,6 +15,19 @@ if GNSS
> config GNSS_SERIAL
> tristate
>
> +config GNSS_MTK_SERIAL
> + tristate "Mediatek GNSS receiver support"
> + depends on SERIAL_DEV_BUS
> + select GNSS_SERIAL
> + help
> + Say Y here if you have a Mediatek-based GNSS receiver which uses a
> + serial interface.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called gnss-mtk.
> +
> + If unsure, say N.
> +
> config GNSS_SIRF_SERIAL
> tristate "SiRFstar GNSS receiver support"
> depends on SERIAL_DEV_BUS
> diff --git a/drivers/gnss/Makefile b/drivers/gnss/Makefile
> index 5cf0ebe0330a..451f11401ecc 100644
> --- a/drivers/gnss/Makefile
> +++ b/drivers/gnss/Makefile
> @@ -9,6 +9,9 @@ gnss-y := core.o
> obj-$(CONFIG_GNSS_SERIAL) += gnss-serial.o
> gnss-serial-y := serial.o
>
> +obj-$(CONFIG_GNSS_MTK_SERIAL) += gnss-mtk.o
> +gnss-mtk-y := mtk.o
> +
> obj-$(CONFIG_GNSS_SIRF_SERIAL) += gnss-sirf.o
> gnss-sirf-y := sirf.o
>
> diff --git a/drivers/gnss/mtk.c b/drivers/gnss/mtk.c
> new file mode 100644
> index 000000000000..a5aad08fb9d7
> --- /dev/null
> +++ b/drivers/gnss/mtk.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek GNSS receiver driver
> + *
> + * Copyright (C) 2018 Johan Hovold <[email protected]>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/gnss.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/serdev.h>
> +
> +#include "serial.h"
> +
> +struct mtk_data {
> + struct regulator *v_bckp;
> + struct regulator *vcc;
> +};
> +
> +static int mtk_set_active(struct gnss_serial *gserial)
> +{
> + struct mtk_data *data = gnss_serial_get_drvdata(gserial);
> + int ret;
> +
> + ret = regulator_enable(data->vcc);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
Hello
This could be simplified to return regulator_enable(data->vcc);
Furthermore, after this simplification, the function seems useless.
Same comment for mtk_set_standby()
Regards
On Thu, Feb 14, 2019 at 11:00:19AM +0100, Corentin Labbe wrote:
> On Wed, Feb 13, 2019 at 04:09:29PM +0100, Loys Ollivier wrote:
> > Add driver for serial-connected Mediatek-based GNSS receivers.
> >
> > These devices typically boot transmitting vendor specific NMEA output
> > sequences. The serial port bit rate is read from the device tree
> > "current-speed".
> >
> > Note that the driver uses the generic GNSS serial implementation and
> > therefore essentially only manages power abstracted into three power
> > states: ACTIVE, STANDBY, and OFF.
> >
> > For mediatek receivers with a main supply and no enable-gpios, this simply
> > means that the main supply is disabled in STANDBY and OFF (the optional
> > backup supply is kept enabled while the driver is bound).
> >
> > Note that the timepulse-support is left unimplemented.
> >
> > Signed-off-by: Loys Ollivier <[email protected]>
> > +static int mtk_set_active(struct gnss_serial *gserial)
> > +{
> > + struct mtk_data *data = gnss_serial_get_drvdata(gserial);
> > + int ret;
> > +
> > + ret = regulator_enable(data->vcc);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
>
> Hello
>
> This could be simplified to return regulator_enable(data->vcc);
Indeed, but I prefer this style which clearly separates the error path
from the success path while making the success return value explicit.
> Furthermore, after this simplification, the function seems useless.
Why do you think so? You still need to retrieve the regulator from the
driver data. Sure, this could be folded into mtk_set_power(), but that
would be less ideal if there are more resources that need to be managed
(e.g. an external lna supply).
Thanks,
Johan
On Thu 14 Feb 2019 at 10:12, Johan Hovold <[email protected]> wrote:
> On Thu, Feb 14, 2019 at 11:00:19AM +0100, Corentin Labbe wrote:
>> On Wed, Feb 13, 2019 at 04:09:29PM +0100, Loys Ollivier wrote:
>> > Add driver for serial-connected Mediatek-based GNSS receivers.
>> >
>> > These devices typically boot transmitting vendor specific NMEA output
>> > sequences. The serial port bit rate is read from the device tree
>> > "current-speed".
>> >
>> > Note that the driver uses the generic GNSS serial implementation and
>> > therefore essentially only manages power abstracted into three power
>> > states: ACTIVE, STANDBY, and OFF.
>> >
>> > For mediatek receivers with a main supply and no enable-gpios, this simply
>> > means that the main supply is disabled in STANDBY and OFF (the optional
>> > backup supply is kept enabled while the driver is bound).
>> >
>> > Note that the timepulse-support is left unimplemented.
>> >
>> > Signed-off-by: Loys Ollivier <[email protected]>
>
>> > +static int mtk_set_active(struct gnss_serial *gserial)
>> > +{
>> > + struct mtk_data *data = gnss_serial_get_drvdata(gserial);
>> > + int ret;
>> > +
>> > + ret = regulator_enable(data->vcc);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + return 0;
>> > +}
>>
>> Hello
>>
>> This could be simplified to return regulator_enable(data->vcc);
>
> Indeed, but I prefer this style which clearly separates the error path
> from the success path while making the success return value explicit.
>
It respects the coding style/standard and I prefer the original version.
It's also easier to debug if you want to add extra code on error.
>> Furthermore, after this simplification, the function seems useless.
>
> Why do you think so? You still need to retrieve the regulator from the
> driver data. Sure, this could be folded into mtk_set_power(), but that
> would be less ideal if there are more resources that need to be managed
> (e.g. an external lna supply).
I would be in favor to keep mtk_set_active. It clearly shows when the
regulator is enabled.
It also seems logic to have an enable/standby function in the driver.
Even if those are trivial (for now).
>
> Thanks,
> Johan
--
-L
On Wed, Feb 13, 2019 at 04:09:27PM +0100, Loys Ollivier wrote:
> Add binding for Mediatek-based GNSS receivers.
>
> Signed-off-by: Loys Ollivier <[email protected]>
> ---
> v4:
> Use standard names for reset-gpios and gnss-fix-gpios
> s/an/a UART
>
> 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..12283d429baa
> --- /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 a UART interface.
> +
> +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:
> +
> +- current-speed : Default UART baud rate
> +- gnss-fix-gpios : GPIO used to determine device position fix state
> + (pin name: FIX, 3D_FIX)
> +- reset-gpios : GPIO used to reset the device
> + (pin name: RESET, NRESET)
> +- timepulse-gpios : Time pulse GPIO (pin name: PPS1, 1PPS)
> +- v-bckp-supply : Backup voltage regulator
> + (pin name: VBAT, VBACKUP)
As we discussed, I changed this to "vbackup-supply" (here and in the
driver) before applying.
Note that git reported some whitespace errors above too (spaces before
tabs) that I fixed up. Not sure why checkpatch doesn't complain.
All now applied.
Thanks,
Johan
On Fri 15 Feb 2019 at 16:04, Johan Hovold <[email protected]> wrote:
> On Wed, Feb 13, 2019 at 04:09:27PM +0100, Loys Ollivier wrote:
>> Add binding for Mediatek-based GNSS receivers.
>>
>> Signed-off-by: Loys Ollivier <[email protected]>
>> ---
>> v4:
>> Use standard names for reset-gpios and gnss-fix-gpios
>> s/an/a UART
>>
>> 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..12283d429baa
>> --- /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 a UART interface.
>> +
>> +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:
>> +
>> +- current-speed : Default UART baud rate
>> +- gnss-fix-gpios : GPIO used to determine device position fix state
>> + (pin name: FIX, 3D_FIX)
>> +- reset-gpios : GPIO used to reset the device
>> + (pin name: RESET, NRESET)
>> +- timepulse-gpios : Time pulse GPIO (pin name: PPS1, 1PPS)
>> +- v-bckp-supply : Backup voltage regulator
>> + (pin name: VBAT, VBACKUP)
>
> As we discussed, I changed this to "vbackup-supply" (here and in the
> driver) before applying.
>
> Note that git reported some whitespace errors above too (spaces before
> tabs) that I fixed up. Not sure why checkpatch doesn't complain.
>
Oops, thanks for fixing it. I ran checkpatch again and it did not
complain. Apprently it's not checking for spaces before tabs in "txt"
files.
I'll add a step in my flow to apply the patches I create before sending
them to the list so that won't happen again ;)
> All now applied.
>
Yay ! Thanks !
Loys
> Thanks,
> Johan
--
-L
On Thu 2019-02-14 11:12:12, Johan Hovold wrote:
> On Thu, Feb 14, 2019 at 11:00:19AM +0100, Corentin Labbe wrote:
> > On Wed, Feb 13, 2019 at 04:09:29PM +0100, Loys Ollivier wrote:
> > > Add driver for serial-connected Mediatek-based GNSS receivers.
> > >
> > > These devices typically boot transmitting vendor specific NMEA output
> > > sequences. The serial port bit rate is read from the device tree
> > > "current-speed".
> > >
> > > Note that the driver uses the generic GNSS serial implementation and
> > > therefore essentially only manages power abstracted into three power
> > > states: ACTIVE, STANDBY, and OFF.
> > >
> > > For mediatek receivers with a main supply and no enable-gpios, this simply
> > > means that the main supply is disabled in STANDBY and OFF (the optional
> > > backup supply is kept enabled while the driver is bound).
> > >
> > > Note that the timepulse-support is left unimplemented.
> > >
> > > Signed-off-by: Loys Ollivier <[email protected]>
>
> > > +static int mtk_set_active(struct gnss_serial *gserial)
> > > +{
> > > + struct mtk_data *data = gnss_serial_get_drvdata(gserial);
> > > + int ret;
> > > +
> > > + ret = regulator_enable(data->vcc);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return 0;
> > > +}
> >
> > Hello
> >
> > This could be simplified to return regulator_enable(data->vcc);
>
> Indeed, but I prefer this style which clearly separates the error path
> from the success path while making the success return value explicit.
What is clear about useless code? That function can be two lines, this
only leads people to wonder "what is going on here?".
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html