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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZUj3fzArgPU4kYYT@hovoldconsulting.com>
Date:   Mon, 6 Nov 2023 15:26:07 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Wolfram Sang <wsa+renesas@...g-engineering.com>
Cc:     linux-renesas-soc@...r.kernel.org,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/3] gnss: ubx: add support for the reset gpio

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