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

[Andy Shevchenko]

On Mon, Dec 8, 2025 at 9:25 PM Kurt Borja <kuurtb@xxxxxxxxx> 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@xxxxxxxxxx>

-- 
With Best Regards,
Andy Shevchenko