| Message ID | 20230712113731.3306-4-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | gnss: ubx: updates to support the Renesas KingFisher board | expand |
Hi Wolfram, On Wed, Jul 12, 2023 at 1:40 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > 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> Thanks for your patch! > --- a/drivers/gnss/ubx.c > +++ b/drivers/gnss/ubx.c > @@ -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); Please assert reset before disabling the regulator, for symmetry with ubx_set_active(). > + > 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!) */ Does it have to be active low? The description in your DT bindings suggest both active low and active high are possible. I think you just wanted to express that GPIOD_OUT_HIGH means asserted, i.e. low for the common case of an active-low reset? > + 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; 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
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;
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> --- drivers/gnss/ubx.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)