Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 6 Jun 2023 17:34:22 +0300 | Subject | Re: [PATCH v2 7/9] ASoC: codecs: Add support for the generic IIO auxiliary devices |
| |
On Tue, Jun 6, 2023 at 4:54 PM Herve Codina <herve.codina@bootlin.com> wrote: > On Sat, 3 Jun 2023 21:26:19 +0300 > andy.shevchenko@gmail.com wrote: > > Tue, May 23, 2023 at 05:12:21PM +0200, Herve Codina kirjoitti:
...
> > > + int max; > > > + int min; > > > > Wondering if there is already a data type for the ranges (like linear_range.h, > > but not sure it's applicable here). > > Seems not applicable here. > - IIO does not use linear_range or something similar. It just uses simple int. > - ASoC does not use linear_range or something similar. It just uses simple long. > > So, I keep the simple int min and max.
Sure.
...
> > > + return 1; /* The value changed */ > > > > Perhaps this 1 needs a definition? > > Yes but to be coherent, in ASoC code, many places need to be changed too > in order to use the newly defined value. > I don't think these modifications should be part of this series.
Yes, we are all for consistency.
...
> > > + for (i = 0; i < iio_aux->num_chans; i++) { > > > + iio_aux_chan = iio_aux->chans + i; > > > + > > > + ret = of_property_read_string_index(np, "io-channel-names", i, > > > + &iio_aux_chan->name); > > > + if (ret < 0) { > > > + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names[%d]\n", np, i); > > > + return ret; > > > > Ditto. > Will be changed in next iteration. > > > > > + } > > > > > + tmp = 0; > > > + of_property_read_u32_index(np, "snd-control-invert-range", i, &tmp); > > > > > + iio_aux_chan->is_invert_range = tmp; > > > > You can use this variable directly. > > Not sure, is_invert_range is a bool and tmp is a u32.
Ah, I see.
> In previous iteration, I wrote > iio_aux_chan->is_invert_range = !!tmp; > > > > + } > > > > Btw, can you avoid using OF APIs? It's better to have device property/fwnode > > API to be used from day 1. > > Hum, this comment was raised in the previous iteration > https://lore.kernel.org/linux-kernel/20230501162456.3448c494@jic23-huawei/ > > I didn't find any equivalent to of_property_read_u32_index() in the > device_property_read_*() function family. > I mean I did find anything available to get a value from an array using an index.
This is done by reading the entire array at once and then parsing as you wish in the code, device_property_read_u32_array() is for that.
> In the previous iteration it was concluded that keeping OF APIs in this series > seemed "reasonable".
Maybe, but consider the above.
-- With Best Regards, Andy Shevchenko
| |