[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250727165534.6756b052@jic23-huawei>
Date: Sun, 27 Jul 2025 16:55:34 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>, Jean Delvare
<jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
linux-iio@...r.kernel.org, linux-hwmon@...r.kernel.org, Andy Shevchenko
<andy@...nel.org>, Nuno Sá <nuno.sa@...log.com>,
linux-kernel@...r.kernel.org, David Lechner <dlechner@...libre.com>
Subject: Re: [PATCH 2/7] iio: inkern: Add API for reading/writing events
On Thu, 17 Jul 2025 12:42:13 -0400
Sean Anderson <sean.anderson@...ux.dev> wrote:
> On 7/16/25 05:28, Andy Shevchenko wrote:
> > On Tue, Jul 15, 2025 at 11:42:05AM -0400, Sean Anderson wrote:
> >> On 7/15/25 04:18, Andy Shevchenko wrote:
> >> > On Mon, Jul 14, 2025 at 09:20:18PM -0400, Sean Anderson wrote:
> >
> > ...
> >
> >> >> +EXPORT_SYMBOL_GPL(iio_event_mode);
> >> >
> >> > Can we move this to namespace? Otherwise it will be never ending story...
> >> > Ditto for other new APIs.
> >>
> >> Never ending story of what?
> >
> > Of converting IIO core to use exported namespaces.
>
> What's the purpose?
Aim here is in general to reduce the massive exposed ABI by applying some
namespaces so that only drivers that opt in to specific functionality
can use particular symbols.
We've used it extensively for groups of related drivers and to some
libraries and the DMA buffers, but so far not pushed it into the IIO core.
I'd be fine with these new functions all being under IIO_CONSUMER or similar.
Quite a bit of feedback on this set will be of the lines of don't do it
the way we did it before as now we know better!
>
> >> >> + if (scale64 <= INT_MAX && scale64 >= INT_MIN)
> >> >> + raw64 = processed / (int)scale64;
> >> >
> >> > Do you need the casting? (I mean if the compiler is dumb enough to not see this)
> >>
> >> AIUI 64-bit division is not available on 32-bit platforms. The cast
> >> ensures we get 32-bit division.
> >
> > I put specifically a remark in the parentheses. So, the Q is if the compiler
> > doesn't recognize that. Can you confirm that 32-bit compilation without cast
> > is broken?
>
> inkern.c:(.text.iio_write_event_processed_scale+0x14c): undefined reference to `__aeabi_ldivmod'
>
> >> >> + *raw = clamp(raw64, (s64)INT_MIN, (s64)INT_MAX);
> >> >
> >> > You already have similar approach here...
> >>
> >> Well, I can spell it 0x7fffffffLL if you'd like...
> >
> > Nope, I like to have named constants instead of magics, but actually are those
> > castings needed for the clamp()?
>
> Apparently not. The checks in __clamp_once are only for matching signedness. And
> the ints are promoted to s64s when the comparison is made.
>
> --Sean
Powered by blists - more mailing lists