linux-kernel - Re: [PATCH v1 14/15] iio: adc: ad7768-1: add support for Synchronization over SPI

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <c0981545-5e0e-4b6e-93d9-cb2871b583e8@baylibre.com>
Date: Tue, 7 Jan 2025 17:50:29 -0600
From: David Lechner <dlechner@...libre.com>
To: Jonathan Santos <Jonathan.Santos@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 marcelo.schmitt1@...il.com
Subject: Re: [PATCH v1 14/15] iio: adc: ad7768-1: add support for
 Synchronization over SPI

On 1/7/25 9:27 AM, Jonathan Santos wrote:
> The synchronization method using GPIO requires the generated pulse to be
> truly synchronous with the base MCLK signal. When it is not possible to
> do that in hardware, the datasheet recommends using synchronization over
> SPI, where the generated pulse is already synchronous with MCLK. This
> requires the SYNC_OUT pin to be connected to SYNC_IN pin.
> 
> Add the option to handle device synchronization over SPI.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@...log.com>
> ---

...

>  static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> @@ -697,11 +708,21 @@ static int ad7768_setup(struct ad7768_state *st)
>  	if (ret)
>  		return ret;
>  
> -	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
> -					  GPIOD_OUT_LOW);
> +	st->gpio_sync_in = devm_gpiod_get_optional(&st->spi->dev, "adi,sync-in",
> +						   GPIOD_OUT_LOW);
>  	if (IS_ERR(st->gpio_sync_in))
>  		return PTR_ERR(st->gpio_sync_in);
>  
> +	if (device_property_present(&st->spi->dev, "adi,sync-in-spi"))
> +		st->en_spi_sync = true;
> +
> +	/*
> +	 * GPIO and SPI Synchronization are mutually exclusive.
> +	 * Return error if both are enabled

Should it also be an error if we have neither? Otherwise it sounds like
decimation won't work correctly since there is a comment that says we have
to toggle this after updating the decimation rate register.

> +	 */
> +	if (st->gpio_sync_in && st->en_spi_sync)
> +		return -EINVAL;

A dev_err_probe() message would be helpful here when creating a new DT and
bringing up a new system since it is easy to forget a property or make a typo
that could lead to this error.

> +
>  	ret = ad7768_gpio_init(st);
>  	if (ret < 0)
>  		return ret;