From: Jonathan Cameron <jic23@kernel.org>
To: Rodrigo Alencar <455.rodrigo.alencar@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Petr Mladek <pmladek@suse.com>,
rodrigo.alencar@analog.com, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-doc@vger.kernel.org, David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
Andrew Morton <akpm@linux-foundation.org>,
Steven Rostedt <rostedt@goodmis.org>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v9 2/9] lib: vsprintf: export simple_strntoull() in a safe prototype
Date: Sun, 12 Apr 2026 16:05:55 +0100 [thread overview]
Message-ID: <20260412160555.4f0ac419@jic23-huawei> (raw)
In-Reply-To: <mnz7d2zd27x6h2qa24rajgrbhkhsypybadkqz2fi43rg7bvjvj@oufys7xs25t4>
On Tue, 31 Mar 2026 14:01:04 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/03/30 01:49PM, Rodrigo Alencar wrote:
> > On 26/03/27 03:17PM, Rodrigo Alencar wrote:
> > > On 26/03/27 12:21PM, Andy Shevchenko wrote:
> > > > On Fri, Mar 27, 2026 at 10:11:56AM +0000, Rodrigo Alencar wrote:
> > > > > On 26/03/27 11:17AM, Andy Shevchenko wrote:
> > > > > > On Fri, Mar 27, 2026 at 09:45:17AM +0100, Petr Mladek wrote:
> > > > > > > On Fri 2026-03-20 16:27:27, Rodrigo Alencar via B4 Relay wrote:
> >
> > ...
> >
> > > > > > Maybe we want to have kstrtof32() and kstrtof64() for these two cases?
> > > > > >
> > > > > > With that we will always consider the fraction part as 32- or 64-bit,
> > > > > > imply floor() on the fraction for the sake of simplicity and require
> > > > > > it to be NUL-terminated with possible trailing '\n'.
> > > > >
> > > > > I think this is a good idea, but calling it float or fixed point itself
> > > > > is a bit confusing as float often refers to the IEEE 754 standard and
> > > > > fixed point types is often expressed in Q-format.
> > > >
> > > > Yeah... I am lack of better naming.
> > >
> > > decimals is the name, but they are often represented as:
> > >
> > > DECIMAL = INT * 10^X + FRAC
> > >
> > > in a single 64-bit number, which would be fine for my end use case.
> > > However IIO decimal fixed point parsing is out there for quite some time a
> > > lot of drivers use that. The interface often relies on breaking parsed values
> > > into an integer array (for standard attributes int val and int val2 are expected).
> >
> > Thinking about this again and in IIO drivers we end up doing something like:
> >
> > val64 = (u64)val * MICRO + val2;
> >
> > so that drivers often work with scaled versions of the decimal value.
> > then, would it make sense to have a function that already outputs such value?
> > That would allow to have more freedom over the 64-bit split between integer
> > and fractional parts.
> > As a draft:
> >
> > static int _kstrtodec64(const char *s, unsigned int scale, u64 *res)
> > {
> > u64 _res = 0, _frac = 0;
> > unsigned int rv;
> >
> > if (*s != '.') {
> > rv = _parse_integer(s, 10, &_res);
> > if (rv & KSTRTOX_OVERFLOW)
> > return -ERANGE;
> > if (rv == 0)
> > return -EINVAL;
> > s += rv;
> > }
> >
> > if (*s == '.') {
> > s++;
> > rv = _parse_integer_limit(s, 10, &_frac, scale);
> > if (rv & KSTRTOX_OVERFLOW)
> > return -ERANGE;
> > if (rv == 0)
> > return -EINVAL;
> > s += rv;
> > if (rv < scale)
> > _frac *= int_pow(10, scale - rv);
> > while (isdigit(*s)) /* truncate */
> > s++;
> > }
> >
> > if (*s == '\n')
> > s++;
> > if (*s)
> > return -EINVAL;
> >
> > if (check_mul_overflow(_res, int_pow(10, scale), &_res) ||
> > check_add_overflow(_res, _frac, &_res))
> > return -ERANGE;
> >
> > *res = _res;
> > return 0;
> > }
> >
> > noinline
> > int kstrtoudec64(const char *s, unsigned int scale, u64 *res)
> > {
> > if (s[0] == '+')
> > s++;
> > return _kstrtodec64(s, scale, res);
> > }
> > EXPORT_SYMBOL(kstrtoudec64);
> >
> > noinline
> > int kstrtosdec64(const char *s, unsigned int scale, s64 *res)
> > {
> > u64 tmp;
> > int rv;
> >
> > if (s[0] == '-') {
> > rv = _kstrtodec64(s + 1, scale, &tmp);
> > if (rv < 0)
> > return rv;
> > if ((s64)-tmp > 0)
> > return -ERANGE;
> > *res = -tmp;
> > } else {
> > rv = kstrtoudec64(s, scale, &tmp);
> > if (rv < 0)
> > return rv;
> > if ((s64)tmp < 0)
> > return -ERANGE;
> > *res = tmp;
> > }
> > return 0;
> > }
> > EXPORT_SYMBOL(kstrtosdec64);
> >
> > e.g., kstrtosdec64() or kstrtoudec64() parses "3.1415" with scale 3 into 3141
>
> Hi Jonathan,
>
> developing more on that, I wouldn't need to create a iio_str_to_fixpoint64(),
> what do you think on new format types:
>
> #define IIO_VAL_DECIMAL64_1 101
> #define IIO_VAL_DECIMAL64_2 102
> #define IIO_VAL_DECIMAL64_3 103
> #define IIO_VAL_DECIMAL64_4 104
> #define IIO_VAL_DECIMAL64_5 105
> #define IIO_VAL_DECIMAL64_6 106
> #define IIO_VAL_DECIMAL64_7 107
> #define IIO_VAL_DECIMAL64_8 108
> #define IIO_VAL_DECIMAL64_9 109
> #define IIO_VAL_DECIMAL64_10 110
> #define IIO_VAL_DECIMAL64_11 111
> #define IIO_VAL_DECIMAL64_12 112
> #define IIO_VAL_DECIMAL64_13 113
> #define IIO_VAL_DECIMAL64_14 114
> #define IIO_VAL_DECIMAL64_15 115
Seems unlikely more than a few of these would ever be used.
If you want to keep the offsets define them in terms
of first one + whatever makes sense (maybe with a base value
to make that easier - then MICRO is BASE + 6 etc)
>
> #define IIO_VAL_DECIMAL64_MILLI IIO_VAL_DECIMAL64_3
> #define IIO_VAL_DECIMAL64_MICRO IIO_VAL_DECIMAL64_6
> #define IIO_VAL_DECIMAL64_NANO IIO_VAL_DECIMAL64_9
> #define IIO_VAL_DECIMAL64_PICO IIO_VAL_DECIMAL64_12
> #define IIO_VAL_DECIMAL64_FEMTO IIO_VAL_DECIMAL64_15
>
> which gets stored as 64-bit, and represent the decimal scaled value.
> That would also work for the PLL driver (using IIO_VAL_DECIMAL64_MICRO):
> - It supports frequency range from 1 to 26 GHz with micro Hz resolution
> - In the driver a 64-bit value: (val * MICRO + val2) is already created
> anyways.
> I would leverage something like kstrtodec64() in iio_write_channel_info().
>
> That way, I would drop the changes on the iio fixpoint parse, which I think
> it would do better with something like kstrntoull() to be able to handle that
> "dB" suffix.
>
> So for now, I may have the following approaches:
> - new kstrntoull() function: to have control over the parsing, whithout
> requiring NUL-termination, avoiding unecessary string scanning or copying.
> covered in v8.
> - expose a "safe" simple_strntoull(): minimal changes to vsprintf.c, this
> is covered by this patch series (v9), and it similar solution to kstrntoull().
> - new kstrtodec64() function: parse decimal numbers as 64-bit with NUL-termination.
> Might be covered in a v10, if it is a good idea.
>
> let me know your thoughts.
For string parsing I'm not a particular expert, so I only really care
about useability of the end result. Having types would work, but I'd kind
of want an 'odd ones' to stand out. Hence only defining them for parsing
cases we already support (but with 64 bit values) would be my preference
if you got ahead with this new approach.
Jonathan
>
next prev parent reply other threads:[~2026-04-12 15:06 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-20 16:27 [PATCH v9 0/9] ADF41513/ADF41510 PLL frequency synthesizers Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 1/9] dt-bindings: iio: frequency: add adf41513 Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 2/9] lib: vsprintf: export simple_strntoull() in a safe prototype Rodrigo Alencar via B4 Relay
2026-03-27 8:45 ` Petr Mladek
2026-03-27 9:17 ` Andy Shevchenko
2026-03-27 10:11 ` Rodrigo Alencar
2026-03-27 10:21 ` Andy Shevchenko
2026-03-27 15:17 ` Rodrigo Alencar
2026-03-30 12:49 ` Rodrigo Alencar
2026-03-31 13:01 ` Rodrigo Alencar
2026-04-12 15:05 ` Jonathan Cameron [this message]
2026-04-01 12:22 ` Petr Mladek
2026-04-01 13:16 ` Rodrigo Alencar
2026-03-27 10:44 ` David Laight
2026-03-27 10:57 ` Andy Shevchenko
2026-03-27 13:28 ` David Laight
2026-03-27 9:24 ` Rodrigo Alencar
2026-03-20 16:27 ` [PATCH v9 3/9] iio: core: add fixed point parsing with 64-bit parts Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 4/9] iio: test: add kunit test for fixed-point parsing Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 5/9] iio: frequency: adf41513: driver implementation Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 6/9] iio: frequency: adf41513: handle LE synchronization feature Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 7/9] iio: frequency: adf41513: features on frequency change Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 8/9] docs: iio: add documentation for adf41513 driver Rodrigo Alencar via B4 Relay
2026-03-20 16:27 ` [PATCH v9 9/9] Documentation: ABI: testing: add common ABI file for iio/frequency Rodrigo Alencar via B4 Relay
2026-03-22 11:33 ` [PATCH v9 0/9] ADF41513/ADF41510 PLL frequency synthesizers Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260412160555.4f0ac419@jic23-huawei \
--to=jic23@kernel.org \
--cc=455.rodrigo.alencar@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=corbet@lwn.net \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-doc@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=pmladek@suse.com \
--cc=robh@kernel.org \
--cc=rodrigo.alencar@analog.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox