On 9/9/24 4:47 AM, Tinaco, Mariel wrote:
>
>
...
>>> + *val = get_unaligned_le16(state->spi_tx_buf);
>>
>> With spi_tx_buf changed to __le16, this can use le16_to_cpu() instead of
>> get_unaligned_le16().
>>
>
> I suppose we use the cpu_to_le16 for the set_hvdac_word function?
>
> static int ad8460_set_hvdac_word(struct ad8460_state *state, int index, int val)
> {
> state->spi_tx_buf = cpu_to_le16(FIELD_PREP(AD8460_DATA_BYTE_FULL_MSK, val));
>
> return regmap_bulk_write(state->regmap, AD8460_HVDAC_DATA_WORD(index),
> &state->spi_tx_buf, AD8460_DATA_BYTE_WORD_LENGTH);
> }
>
Yes, that looks correct.
>>> +static ssize_t ad8460_write_toggle_en(struct iio_dev *indio_dev, uintptr_t
>> private,
>>> + const struct iio_chan_spec *chan,
>>> + const char *buf, size_t len) {
>>> + struct ad8460_state *state = iio_priv(indio_dev);
>>> + bool toggle_en;
>>> + int ret;
>>> +
>>> + ret = kstrtobool(buf, &toggle_en);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
>>> + return ad8460_enable_apg_mode(state, toggle_en);
>>> + unreachable();
>>> +}
>>
>> Hmm... do we need to make an unscoped version of
>> iio_device_claim_direct_scoped()?
>>
>
> So iio_device_claim_direct_scoped is used here because the buffer enable/disable
> accesses this enable_apg_mode function. Is it also a standard practice to put the
> kstrobool() util inside the scope?
>
Since this is at the end of a function with nothing after it, it
would be nice if we could avoid the indent and unreachable();
The idea would be to write the last 3 lines like this instead:
guard_cond(iio_device_claim_direct, return -EBUSY, indio_dev);
But I didn't see a `guard_cond()` analog of `guard()` in
linux/cleanup.h. So this is probably fine for now and adding
`guard_cond()` (if it is actually a good idea in the first
place) can be a job for another time.
