[RFC PATCH v2 0/3] gnss: updates to support the Renesas KingFisher board

[RFC PATCH v2 0/3] gnss: updates to support the Renesas KingFisher board

[Wolfram Sang]

Hi, here are some patches with which I can use the GPS receiver on the
Renesas KingFisher board. Still RFC because I have a specific question
regarding the last patch. Looking forward to comments!

   Wolfram

Wolfram Sang (3):
  gnss: ubx: use new helper to remove open coded regulator handling
  gnss: ubx: add support for the reset gpio
  arm64: dts: renesas: ulcb-kf: add node for GPS

 arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 14 ++++++++++
 drivers/gnss/ubx.c                       | 35 ++++++++++--------------
 2 files changed, 29 insertions(+), 20 deletions(-)

-- 
2.30.2

[RFC PATCH v2 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>
---

Changes since RFC v1:
* rebased because of patches dropped

 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.30.2

[RFC PATCH v2 2/3] gnss: ubx: add support for the reset gpio

[Wolfram Sang]

Tested with a Renesas KingFisher board. 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>
---
Changes since RFC v1:
* rebased because of patches dropped
* bail out correctly when requesting GPIO fails (Thanks, Dan!)

 drivers/gnss/ubx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
index 9b76b101ba5e..cb0612100644 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;
 }
 
@@ -41,6 +45,8 @@ static int ubx_set_standby(struct gnss_serial *gserial)
 	if (ret)
 		return ret;
 
+	gpiod_set_value_cansleep(data->reset_gpio, 1);
+
 	return 0;
 }
 
@@ -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 (GPIO must be active low!) */
+	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.30.2

[RFC PATCH v2 3/3] arm64: dts: renesas: ulcb-kf: add node for GPS

[Wolfram Sang]

From: Wolfram Sang <wsa@kernel.org>

Add the node for the GPS receiver and its VCC supply.

Signed-off-by: Wolfram Sang <wsa@kernel.org>
---
Changes since RFC v1:
* rebased because of patches dropped and refactored
* added static vcc-suplly

@Geert: Not sure if we need so many fixed 3v3 regulators. Most of the
other ones more or less directly derive from d_3v3. Or do you prefer it
this way?

 arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
index e62f5359f64b..dd1ffc31c874 100644
--- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
@@ -39,6 +39,13 @@ accel_3v3: regulator-acc-3v3 {
 		regulator-max-microvolt = <3300000>;
 	};
 
+	d_3v3: regulator-d-3v3 {
+		compatible = "regulator-fixed";
+		regulator-name = "d-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
 	hdmi_1v8: regulator-hdmi-1v8 {
 		compatible = "regulator-fixed";
 		regulator-name = "hdmi-1v8";
@@ -434,6 +441,13 @@ &scif1 {
 	pinctrl-names = "default";
 
 	status = "okay";
+
+	gnss {
+		compatible = "u-blox,neo-m8";
+		reset-gpios = <&gpio_exp_75 6 GPIO_ACTIVE_LOW>;
+		vcc-supply = <&d_3v3>;
+		current-speed = <9600>;
+	};
 };
 
 &sdhi3 {
-- 
2.30.2

Re: [RFC PATCH v2 3/3] arm64: dts: renesas: ulcb-kf: add node for GPS

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jun 20, 2023 at 12:39 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> From: Wolfram Sang <wsa@kernel.org>
>
> Add the node for the GPS receiver and its VCC supply.
>
> Signed-off-by: Wolfram Sang <wsa@kernel.org>
> ---
> Changes since RFC v1:
> * rebased because of patches dropped and refactored
> * added static vcc-suplly

Thanks for the update!

> @Geert: Not sure if we need so many fixed 3v3 regulators. Most of the
> other ones more or less directly derive from d_3v3. Or do you prefer it
> this way?

Probably all of them should refer to &reg_3p3v instead...

> --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> @@ -39,6 +39,13 @@ accel_3v3: regulator-acc-3v3 {
>                 regulator-max-microvolt = <3300000>;
>         };
>
> +       d_3v3: regulator-d-3v3 {

... but if you want to keep it like this for now, please name
it appropriately (s/d/gnss/g).

> +               compatible = "regulator-fixed";
> +               regulator-name = "d-3v3";
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +       };
> +
>         hdmi_1v8: regulator-hdmi-1v8 {
>                 compatible = "regulator-fixed";
>                 regulator-name = "hdmi-1v8";

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH v2 3/3] arm64: dts: renesas: ulcb-kf: add node for GPS

[Wolfram Sang]

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


> > @Geert: Not sure if we need so many fixed 3v3 regulators. Most of the
> > other ones more or less directly derive from d_3v3. Or do you prefer it
> > this way?
> 
> Probably all of them should refer to &reg_3p3v instead...

Yes, I like this. I will refactor it into a seperate series.


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