On Mon, 9 Sep 2024 09:51:35 -0500
David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> 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.
That's a fun topic if you check the archives. Linus really
didn't like the proposal
https://lore.kernel.org/all/170905253339.2268463.9376907713092612237.stgit@xxxxxxxxxxxxxxxxxxxxxxxxx/
Mind you I don't think he much likes scoped_cond_guard() either!
Jonathan
