lkml.org 
[lkml]   [2023]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2023-06-06 16:35    [W:0.315 / U:1.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site