linux-kernel - Re: [PATCH v7 2/2] iio: adc: Add ti-ads1018 driver

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: <CAHp75VcOVpGbb3UBm+QQrw25=yU+J624c29ptMk8yrJpNEL=jA@mail.gmail.com>
Date: Mon, 8 Dec 2025 22:19:10 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Kurt Borja <kuurtb@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Tobias Sperling <tobias.sperling@...ting.com>, David Lechner <dlechner@...libre.com>, 
	Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v7 2/2] iio: adc: Add ti-ads1018 driver

On Mon, Dec 8, 2025 at 9:25 PM Kurt Borja <kuurtb@...il.com> wrote:
>
> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
> analog-to-digital converters.
>
> These chips' MOSI pin is shared with a data-ready interrupt. Defining

Either "This", or "pins are".

> this interrupt in devicetree is optional, therefore we only create an
> IIO trigger if one is found.
>
> Handling this interrupt requires some considerations. When enabling the
> trigger the CS line is tied low (active), thus we need to hold
> spi_bus_lock() too, to avoid state corruption. This is done inside the
> set_trigger_state() callback, to let users use other triggers without
> wasting a bus lock.

...

> +/**
> + * ads1018_calc_delay - Calculates a suitable delay for a single-shot reading
> + * @hz: Sampling frequency
> + *
> + * Calculates an appropriate delay for a single shot reading given a sampling
> + * frequency.
> + *
> + * Return: Delay in microseconds (Always greater than 0).
> + */
> +static u32 ads1018_calc_delay(unsigned int hz)
> +{
> +       /*
> +        * Calculate the worst-case sampling rate by subtracting 10% error
> +        * specified in the datasheet...
> +        */
> +       hz -= DIV_ROUND_UP(hz, 10);
> +
> +       /* ...Then calculate time per sample in microseconds. */
> +       return DIV_ROUND_UP(MICROHZ_PER_HZ, hz);

If time per sample is in µs, the associated frequency is in MHz, so
the correct constant is HZ_PER_MHZ. What did I miss here?

> +}

...

> +static struct spi_driver ads1018_spi_driver = {
> +       .driver = {
> +               .name = "ads1018",
> +               .of_match_table = ads1018_of_match,
> +       },
> +       .probe = ads1018_spi_probe,
> +       .id_table = ads1018_spi_match,
> +};

> +

Unneeded blank line.

> +module_spi_driver(ads1018_spi_driver);

...

Other than above, LGTM!
Reviewed-by: Andy Shevchenko <andy@...nel.org>

-- 
With Best Regards,
Andy Shevchenko