On 10/15/24 1:37 AM, Nuno Sá wrote: > On Mon, 2024-10-14 at 16:15 -0500, David Lechner wrote: >> On 10/14/24 5:08 AM, Angelo Dureghello wrote: >>> From: Angelo Dureghello <adureghello@xxxxxxxxxxxx> >>> >>> Add High Speed ad3552r platform driver. >>> >> >> ... >> >>> +static int ad3552r_hs_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct ad3552r_hs_state *st = iio_priv(indio_dev); >>> + int ret; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_SAMP_FREQ: { >>> + int sclk; >>> + >>> + ret = iio_backend_read_raw(st->back, chan, &sclk, 0, >>> + IIO_CHAN_INFO_FREQUENCY); >> >> FWIW, this still seems like an odd way to get the stream mode SCLK >> rate from the backend to me. How does the backend know that we want >> the stream mode clock rate and not some other frequency value? > > In this case the backend has a dedicated compatible so sky is the limit :). But yeah, > I'm also not extremely happy with IIO_CHAN_INFO_FREQUENCY. But what do you have in > mind? Using the sampling frequency INFO or a dedicated OP? > It think it would be most straightforward to have something like a iio_backend_get_data_stream_clock_rate() callback since that is what we are getting. Re: the other recent discussions about getting too many callbacks. Instead of a dedicated function like this, we could make a set of generic functions: iio_backend_{g,s}et_property_{s,u}(8, 16, 32, 64}() that take an enum parameter for the property. This way, for each new property, we just have to add an enum member instead of creating a get/set callback pair. Unrelated to this particular case, but taking the idea even farther, we could also do the same with enable/disable functions. We talked before about cutting the number of callbacks in half by using a bool parameter instead of separate enable/disable callbacks. But we could cut it down even more by having an enum parameter for the thing we are enabling/disabling.