Re: [PATCH v4 2/2] iio: adc: ti-ads112c04: Add support for TI ADS112C04

[David Lechner <dlechner@baylibre.com>]

On 8/11/26 10:38 PM, Kyle Hsieh wrote:
> On Tue, Aug 11, 2026 at 10:18 PM David Lechner <dlechner@baylibre.com> wrote:
>>
>> On 8/10/26 9:48 PM, Kyle Hsieh wrote:
>>> Add IIO driver support for the Texas Instruments ADS112C04 (16-bit)
>>> delta-sigma ADCs.
>>>

...

>>> +
>>> +     /* Power-On Reset (POR) delay */
>>> +     fsleep(50 * USEC_PER_MSEC);
>>> +
>>> +     /* Requesting OUT_HIGH asserts the active-low reset pin immediately */
>>> +     reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>>> +     if (IS_ERR(reset_gpio))
>>> +             return PTR_ERR(reset_gpio);
>>> +
>>> +     if (reset_gpio) {
>>> +             fsleep(1000);
>>> +             gpiod_set_value_cansleep(reset_gpio, 0);
>>> +     } else {
>>> +             ret = ads112c04_write_cmd(client, ADS112C04_CMD_RESET);
>>> +             if (ret < 0)
>>> +                     return ret;
>>> +     }
>>> +
>>> +     fsleep(1000);
>>> +
>>> +     /* Bypass PGA for now to allow full-scale single-ended measurements */
>>> +     st->config0 = ADS112C04_CONFIG0_PGA_BYPASS;
>>> +     ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG0, st->config0);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     ret = ads112c04_write_reg(client, ADS112C04_REG_CONFIG1, st->config1);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     if (client->irq > 0) {
>>> +             ret = devm_request_irq(dev, client->irq,
>>> +                                    ads112c04_irq_handler,
>>> +                                    0,
>>
>> I think I've asked twice now to move this 0 on the previous line.
>> If you don't agree with that, it is fine, but we just ask that you
>> reply to the suggestion with an explanation to state your reasoning.
>> Otherwise, it comes across as carelessness that you continue to
>> ignore suggestions. This is not the only suggestoin that has been
>> silently ignored. You might want to go back to the previous revisions
>> and see if you missed anything else.
> Clarifications on my previous mail.
> 
> IRQ: I will use devm_request_threaded_irq() with a NULL primary handler and
> IRQF_ONESHOT. disable_irq_nosync() is gone and the handler only calls
> complete(). This also removes the bare `0` you asked about on v2 and v3,
> with the arguments packed rather than one per line:
> 
> ret = devm_request_threaded_irq(dev, client->irq, NULL,
> ads112c04_irq_handler,
> IRQF_ONESHOT, indio_dev->name,
> indio_dev);
> 
> I left out the DRDY status read for now, since conversions are
> single-shot under the mutex. Happy to add it if you prefer.
>>

Completing the completion is safe in a normal IRQ handler. So if
we aren't disabling the interrupt and we aren't reading the status
bit in the IRQ handler, then we don't need a threaded interrupt
yet.

>>> +                                    indio_dev->name, indio_dev);
>>> +             if (ret)
>>> +                     return ret;
>>> +     }
>>> +
>>> +     return devm_iio_device_register(dev, indio_dev);
>>> +}
>>> +