Re: [PATCH v5 08/10] iio: adc: ad_sigma_delta: Check for previous ready signals

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Tue, Dec 3, 2024 at 1:01 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> It can happen if a previous conversion was aborted the ADC pulls down
> the ̅R̅D̅Y line but the event wasn't handled before. In that case enabling

Interesting use of Unicode, but I suggest to avoid it when it can be
avoided, i.e.
using the notation of #RDY_N might be appropriate as that is how
usually the HW people refer to the active low signals.

> the irq might immediately fire (depending on the irq controller's

controller

> capabilities) and even with a rdy-gpio isn't identified as an unrelated
> one.
>
> To cure that problem check for a pending event before the measurement is
> started and clear it if needed.

...

> +       data = kzalloc(data_read_len + 1, GFP_KERNEL);

Yes, I know that's not needed right now, but would make code robust
against changes. I'm talking about using __free() here.

> +       if (!data)
> +               return -ENOMEM;
> +
> +       spi_message_init(&m);
> +       if (sigma_delta->info->has_registers) {
> +               unsigned int data_reg = sigma_delta->info->data_reg ?: AD_SD_REG_DATA;
> +
> +               data[0] = data_reg << sigma_delta->info->addr_shift;
> +               data[0] |= sigma_delta->info->read_mask;
> +               data[0] |= sigma_delta->comm;
> +               t[0].tx_buf = data;
> +               spi_message_add_tail(&t[0], &m);
> +       }
> +
> +       /*
> +        * The first transferred byte is part of the real data register,
> +        * so this doesn't need to be 0xff. In the remaining
> +        * `data_read_len - 1` bytes are less than $num_resetclks ones.
> +        */
> +       t[1].tx_buf = data + 1;
> +       data[1] = 0x00;
> +       memset(data + 2, 0xff, data_read_len - 1);
> +       spi_message_add_tail(&t[1], &m);

> +       ret = spi_sync_locked(sigma_delta->spi, &m);
> +
> +       kfree(data);
> +
> +       return ret;

With the above this will become just as

  return spi_sync_locked(...);

-- 
With Best Regards,
Andy Shevchenko