[PATCH v4 0/3] gnss: ubx: support the reset pin of the Neo-M8 variant

[PATCH v4 0/3] gnss: ubx: support the reset pin of the Neo-M8 variant

[Wolfram Sang]

The Renesas KingFisher board includes a U-Blox Neo-M8 chip with its
reset pin wired to a GPIO. To support that, we need "reset-gpio" support
(patches 2+3). But first, simplify regulator handling with a new helper
(patch 1).

Changes since v3:
* rebased to 6.6
* improved commit messages for patches 2+3


Wolfram Sang (3):
  gnss: ubx: use new helper to remove open coded regulator handling
  dt-bindings: gnss: u-blox: add "reset-gpios" binding
  gnss: ubx: add support for the reset gpio

 .../bindings/gnss/u-blox,neo-6m.yaml          |  5 +++
 drivers/gnss/ubx.c                            | 35 ++++++++-----------
 2 files changed, 20 insertions(+), 20 deletions(-)

-- 
2.35.1

[PATCH v4 1/3] gnss: ubx: use new helper to remove open coded regulator handling

[Wolfram Sang]

v_bckp shall always be enabled as long as the device exists. We now have
a regulator helper for that, use it.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gnss/ubx.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
index c951be202ca2..9b76b101ba5e 100644
--- a/drivers/gnss/ubx.c
+++ b/drivers/gnss/ubx.c
@@ -17,7 +17,6 @@
 #include "serial.h"
 
 struct ubx_data {
-	struct regulator *v_bckp;
 	struct regulator *vcc;
 };
 
@@ -87,30 +86,16 @@ static int ubx_probe(struct serdev_device *serdev)
 		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 = devm_regulator_get_enable_optional(&serdev->dev, "v-bckp");
+	if (ret < 0 && ret != -ENODEV)
+		goto err_free_gserial;
 
 	ret = gnss_serial_register(gserial);
 	if (ret)
-		goto err_disable_v_bckp;
+		goto err_free_gserial;
 
 	return 0;
 
-err_disable_v_bckp:
-	if (data->v_bckp)
-		regulator_disable(data->v_bckp);
 err_free_gserial:
 	gnss_serial_free(gserial);
 
@@ -120,11 +105,8 @@ static int ubx_probe(struct serdev_device *serdev)
 static void ubx_remove(struct serdev_device *serdev)
 {
 	struct gnss_serial *gserial = serdev_device_get_drvdata(serdev);
-	struct ubx_data *data = gnss_serial_get_drvdata(gserial);
 
 	gnss_serial_deregister(gserial);
-	if (data->v_bckp)
-		regulator_disable(data->v_bckp);
 	gnss_serial_free(gserial);
 }
 
-- 
2.35.1

[PATCH v4 2/3] dt-bindings: gnss: u-blox: add "reset-gpios" binding

[Wolfram Sang]

The Renesas KingFisher board includes a U-Blox Neo-M8 chip. This chip
has a reset pin which is also wired on the board. Introduce a binding to
support this reset pin.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml b/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml
index 4835a280b3bf..8e97e475613f 100644
--- a/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml
+++ b/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml
@@ -41,6 +41,9 @@ properties:
     description: >
       Backup voltage regulator
 
+  reset-gpios:
+    maxItems: 1
+
 required:
   - compatible
   - vcc-supply
@@ -49,10 +52,12 @@ unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     serial {
         gnss {
             compatible = "u-blox,neo-8";
             v-bckp-supply = <&gnss_v_bckp_reg>;
             vcc-supply = <&gnss_vcc_reg>;
+            reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>;
         };
     };
-- 
2.35.1

[PATCH v4 3/3] gnss: ubx: add support for the reset gpio

[Wolfram Sang]

The Renesas KingFisher board includes a U-Blox Neo-M8 chip. This chip
has a reset pin which is also wired on the board. Add code to the driver
to support this reset pin. Because my GNSS device is hooked up via UART
and I2C simultaneously, I could verify functionality by opening/closing
the GNSS device using UART and see if the corresponding I2C device was
visible on the bus.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gnss/ubx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
index 9b76b101ba5e..e7d151cbc8c3 100644
--- a/drivers/gnss/ubx.c
+++ b/drivers/gnss/ubx.c
@@ -7,6 +7,7 @@
 
 #include <linux/errno.h>
 #include <linux/gnss.h>
+#include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -18,6 +19,7 @@
 
 struct ubx_data {
 	struct regulator *vcc;
+	struct gpio_desc *reset_gpio;
 };
 
 static int ubx_set_active(struct gnss_serial *gserial)
@@ -29,6 +31,8 @@ static int ubx_set_active(struct gnss_serial *gserial)
 	if (ret)
 		return ret;
 
+	gpiod_set_value_cansleep(data->reset_gpio, 0);
+
 	return 0;
 }
 
@@ -37,6 +41,8 @@ static int ubx_set_standby(struct gnss_serial *gserial)
 	struct ubx_data *data = gnss_serial_get_drvdata(gserial);
 	int ret;
 
+	gpiod_set_value_cansleep(data->reset_gpio, 1);
+
 	ret = regulator_disable(data->vcc);
 	if (ret)
 		return ret;
@@ -90,6 +96,13 @@ static int ubx_probe(struct serdev_device *serdev)
 	if (ret < 0 && ret != -ENODEV)
 		goto err_free_gserial;
 
+	/* Start with reset asserted */
+	data->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(data->reset_gpio)) {
+		ret = PTR_ERR(data->reset_gpio);
+		goto err_free_gserial;
+	}
+
 	ret = gnss_serial_register(gserial);
 	if (ret)
 		goto err_free_gserial;
-- 
2.35.1

Re: [PATCH v4 3/3] gnss: ubx: add support for the reset gpio

[Johan Hovold]

On Fri, Nov 03, 2023 at 11:56:00PM +0100, Wolfram Sang wrote:
> The Renesas KingFisher board includes a U-Blox Neo-M8 chip. This chip
> has a reset pin which is also wired on the board. Add code to the driver
> to support this reset pin. Because my GNSS device is hooked up via UART
> and I2C simultaneously, I could verify functionality by opening/closing
> the GNSS device using UART and see if the corresponding I2C device was
> visible on the bus.

>  static int ubx_set_active(struct gnss_serial *gserial)
> @@ -29,6 +31,8 @@ static int ubx_set_active(struct gnss_serial *gserial)
>  	if (ret)
>  		return ret;
>  
> +	gpiod_set_value_cansleep(data->reset_gpio, 0);
> +
>  	return 0;
>  }
>  
> @@ -37,6 +41,8 @@ static int ubx_set_standby(struct gnss_serial *gserial)
>  	struct ubx_data *data = gnss_serial_get_drvdata(gserial);
>  	int ret;
>  
> +	gpiod_set_value_cansleep(data->reset_gpio, 1);
> +
>  	ret = regulator_disable(data->vcc);
>  	if (ret)
>  		return ret;
> @@ -90,6 +96,13 @@ static int ubx_probe(struct serdev_device *serdev)
>  	if (ret < 0 && ret != -ENODEV)
>  		goto err_free_gserial;
>  
> +	/* Start with reset asserted */
> +	data->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(data->reset_gpio)) {
> +		ret = PTR_ERR(data->reset_gpio);
> +		goto err_free_gserial;
> +	}
> +
>  	ret = gnss_serial_register(gserial);
>  	if (ret)
>  		goto err_free_gserial;

So as I just replied to you v3, the hardware integration manual for
NEO-M8 and the datasheets for some of the other modules explicitly says
that the RESET_N pin should not be used this way:

	1.5 I/O pins
	RESET_N: Reset input
	Driving RESET_N low activates a hardware reset of the system.
	Use this pin only to reset the module. Do not use RESET_N to
	turn the module on and off, since the reset state increases
	power consumption.

(and AFAIU you should generally not try to use reset this way unless it
is explicitly said to be supported).

Johan

Re: [PATCH v4 3/3] gnss: ubx: add support for the reset gpio

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]


> 	1.5 I/O pins
> 	RESET_N: Reset input
> 	Driving RESET_N low activates a hardware reset of the system.
> 	Use this pin only to reset the module. Do not use RESET_N to
> 	turn the module on and off, since the reset state increases
> 	power consumption.
> 
> (and AFAIU you should generally not try to use reset this way unless it
> is explicitly said to be supported).

Oh! That's the opposite of my intention :/ Okay, today I learnt
something. Thank you for pointing this out. I will remember this and
double check reset handling in the future.

That means I only need to de-assert reset in probe() for now, right?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]