On Tue, 2024-10-22 at 09:31 +0300, Alexandru Ardelean wrote: > On Mon, Oct 21, 2024 at 10:31 PM David Lechner <dlechner@xxxxxxxxxxxx> wrote: > > > > On 10/21/24 2:03 PM, David Lechner wrote: > > > On 10/21/24 8:02 AM, Alexandru Ardelean wrote: > > > > There's a small issue with setting oversampling-ratio that seems to have > > > > been there since the driver was in staging. > > > > Trying to set an oversampling value of '2' will set an oversampling > > > > value > > > > of '1'. This is because find_closest() does an average + rounding of 1 + > > > > 2, > > > > and we get '1'. > > > > > > > > This is the only issue with find_closest(), at least in this setup. The > > > > other values (above 2) work reasonably well. Setting 3, rounds to 2, so > > > > a > > > > quick fix is to round 'val' to 3 (if userspace provides 2). > > > > > > This sounds like a bug in find_closest() instead of in this driver. > > > > > Adding Bart (the original author of find_closest()). > > > > If there is an exact match in the list, it seems reasonable to expect > > > that the exact match is returned by find_closest(). > > > > > > > Likely also affected by this bug since they have values 1, 2 in the list: > > > > * rtq6056_adc_set_average() > > * si1133_scale_to_swgain() > > Yeah. > I forgot to mention this sooner. > But this patch is more of an RFC patch about how to handle this > situation with find_closest(). > > For monotonic values with an increment of 1, find_closest() is a bit buggy. > Will try to fix find_closest() > > > FWIW, I'm not a fan of using find_closest() in this situation. We have an available attr wich outputs the supported values. To me, -EINVAL is the way to go if some user writes an invalid value. I feel the usage of find_closest() in these case is likely to make the code easier. Maybe an helper analogous to match_string() would be seen with good eyes (like match_value()). But yeah, I guess that changing the behavior now could break some userspace users. - Nuno Sá
