* Re: [PATCH v11 6/6] docs: iio: adc: ad4691: add driver documentation
From: Jonathan Cameron @ 2026-05-17 12:32 UTC (permalink / raw)
To: David Lechner
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
Bartosz Golaszewski, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <78d4bb1c-4c6d-4781-86ee-458579ac6990@baylibre.com>
On Sat, 16 May 2026 13:18:03 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 5/15/26 8:31 AM, Radu Sabau via B4 Relay wrote:
> > From: Radu Sabau <radu.sabau@analog.com>
> >
> > Add RST documentation for the AD4691 family ADC driver covering
> > supported devices, IIO channels, operating modes, oversampling,
> > reference voltage, LDO supply, reset, GP pins, SPI offload support,
> > and buffer data format.
> >
> > Signed-off-by: Radu Sabau <radu.sabau@analog.com>
FWIW I finished giving the series another look and mostly didn't have
anything to add to David's review! So subject to further discussion
on the feedback (and maybe a day or two to see if others want to enter
the fray), looking forward to v12. Fingers crossed that's good to go.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v11 3/6] iio: adc: ad4691: add triggered buffer support
From: Jonathan Cameron @ 2026-05-17 12:25 UTC (permalink / raw)
To: David Lechner
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
Bartosz Golaszewski, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <9b7986e1-6550-415d-b301-33089ba10177@baylibre.com>
On Sat, 16 May 2026 12:32:51 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 5/15/26 8:31 AM, Radu Sabau via B4 Relay wrote:
> > From: Radu Sabau <radu.sabau@analog.com>
> >
> > Add buffered capture support using the IIO triggered buffer framework.
> >
> > CNV Burst Mode: the GP pin identified by interrupt-names in the device
> > tree is configured as DATA_READY output. The IRQ handler stops
> > conversions and fires the IIO trigger; the trigger handler executes a
> > pre-built SPI message that reads all active channels from the AVG_IN
> > accumulator registers and then resets accumulator state and restarts
> > conversions for the next cycle.
> >
> > Manual Mode: CNV is tied to SPI CS so each transfer simultaneously
> > reads the previous result and starts the next conversion (pipelined
> > N+1 scheme). At preenable time a pre-built, optimised SPI message of
> > N+1 transfers is constructed (N channel reads plus one NOOP to drain
> > the pipeline). The trigger handler executes the message in a single
> > spi_sync() call and collects the results. An external trigger (e.g.
> > iio-trig-hrtimer) is required to drive the trigger at the desired
> > sample rate.
> >
> > Both modes share the same trigger handler and push a complete scan —
> > one big-endian 16-bit (__be16) slot per active channel, densely packed
> > in scan_index order, followed by a timestamp.
> >
> > The CNV Burst Mode sampling frequency (PWM period) is exposed as a
> > buffer-level attribute via IIO_DEVICE_ATTR.
> >
> > Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> > +
> > +static int ad4691_manual_buffer_preenable(struct iio_dev *indio_dev)
> > +{
> > + struct ad4691_state *st = iio_priv(indio_dev);
> > + unsigned int k, i;
> > + int ret;
> > +
> > + memset(st->scan_xfers, 0, sizeof(st->scan_xfers));
> > + memset(st->scan_tx, 0, sizeof(st->scan_tx));
> > +
> > + spi_message_init(&st->scan_msg);
> > +
> > + k = 0;
> > + iio_for_each_active_channel(indio_dev, i) {
> > + if (i >= indio_dev->num_channels - 1)
> > + break; /* skip soft timestamp */
>
> I don't think timestamp gets set in the scan mask. It is handled separately.
FWIW that is a sashiko false postive (I believe anyway!)
If we do hit this please shout as we have a core bug.
If anyone has time to look at how hard it would be to tweak
iio_for_each_active_channel to skip a last element timestamp that
would be great.
I think that iterates one too far which is what sashiko is tripping over.
I'm only keen to fix that if we can make it low cost and hid it entirely
from drivers.
Jonathan
>
> > + /*
> > + * Channel-select command occupies the first (high) byte of the
> > + * 16-bit DIN frame; the second byte is a don't-care zero pad.
> > + * put_unaligned_be16() writes [cmd, 0x00] in memory so the
> > + * SPI controller sends the command byte first on the wire.
> > + */
> > + put_unaligned_be16((u16)(AD4691_ADC_CHAN(i) << 8), &st->scan_tx[k]);
> > + st->scan_xfers[k].tx_buf = &st->scan_tx[k];
> > + /*
> > + * The pipeline means xfer[0] receives the residual from the
> > + * previous sequence, not a valid sample. Discard it (rx_buf=NULL)
> > + * to avoid aliasing vals[0] across two concurrent DMA mappings.
> > + * xfer[1] (or the NOOP when only one channel is active) writes
> > + * the real ch[0] result to vals[0]. Subsequent transfers write
> > + * into vals[k-1] so each result lands at the next dense slot.
> > + */
> > + st->scan_xfers[k].rx_buf = (k == 0) ? NULL : &st->vals[k - 1];
> > + st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
> > + st->scan_xfers[k].cs_change = 1;
> > + st->scan_xfers[k].cs_change_delay.value = AD4691_CNV_HIGH_TIME_NS;
> > + st->scan_xfers[k].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
> > + spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
> > + k++;
> > + }
> > +
> > + /* Final NOOP transfer retrieves the last channel's result. */
> > + st->scan_xfers[k].tx_buf = &st->scan_tx[k]; /* scan_tx[k] == 0 == NOOP */
> > + st->scan_xfers[k].rx_buf = &st->vals[k - 1];
> > + st->scan_xfers[k].len = sizeof(st->scan_tx[k]);
> > + spi_message_add_tail(&st->scan_xfers[k], &st->scan_msg);
> > +
> > + ret = spi_optimize_message(st->spi, &st->scan_msg);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ad4691_enter_conversion_mode(st);
> > + if (ret) {
> > + spi_unoptimize_message(&st->scan_msg);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
^ permalink raw reply
* Re: [PATCH v13 02/12] iio: kstrtox: add local _parse_integer_limit_init() helper
From: Rodrigo Alencar @ 2026-05-17 12:19 UTC (permalink / raw)
To: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc
Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
Petr Mladek, Steven Rostedt, Andy Shevchenko, Rasmus Villemoes,
Sergey Senozhatsky, Shuah Khan
In-Reply-To: <20260517-adf41513-iio-driver-v13-2-bb6e134a360f@analog.com>
On 26/05/17 10:13AM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> Add parsing helper that accepts an initial value for the accumulated
> result when parsing an 64-bit integer. It reuses current implementation
> for _parse_integer_limit(), which now consumes the new function with
> init = 0. The diff algorithm would have the documentation header and
> prototype of _parse_integer_limit() moved around so it is adjusted
> according to guidelines.
Sashiko's feedback at:
https://sashiko.dev/#/patchset/20260517-adf41513-iio-driver-v13-0-bb6e134a360f%40analog.com?part=2
...
> -/*
> - * Convert non-negative integer string representation in explicitly given radix
> - * to an integer. A maximum of max_chars characters will be converted.
> - *
> - * Return number of characters consumed maybe or-ed with overflow bit.
> - * If overflow occurs, result integer (incorrect) is still returned.
> - *
> - * Don't you dare use this function.
> - */
> -noinline
> -unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned long long *p,
> - size_t max_chars)
...
> +/**
> + * _parse_integer_limit() - Convert integer string representation to an integer
> + * limiting the number of characters parsed.
> + * @s: The start of the string.
> + * @base: The number base to use.
> + * @p: Where to write the result of the conversion.
> + * @max_chars: Maximum amount of characters to consume.
> + *
> + * Convert non-negative integer string representation in explicitly given radix
> + * to an integer. A maximum of max_chars characters will be converted.
> + *
> + * Return: Number of characters consumed maybe or-ed with overflow bit.
> + * If overflow occurs, result integer (incorrect) is still returned.
> + */
> +noinline
> +unsigned int _parse_integer_limit(const char *s, unsigned int base,
> + unsigned long long *p, size_t max_chars)
Is it safe to remove the "Don't you dare use this function." warning?
Since this function returns the number of consumed characters bitwise-ORed with
the KSTRTOX_OVERFLOW flag, turning the comment into standard kernel-doc format
makes it look like an inviting API.
If a developer uses this and directly advances a pointer by the return value
without masking the overflow bit, could an integer overflow during parsing cause
the pointer to jump out-of-bounds?
That return value merged with the overflow indication is a bit weird in the first
place. I wonder if we need a precursor patch cleaning _parse_integer_limit() and
removing this KSTRTOX_OVERFLOW completely... turning it into:
ssize_t _parse_integer_limit(const char *s,
const char **endp,
unsigned int base,
unsigned long long *res,
size_t max_chars);
so ssize_t returns the amount of converted characters, or -ERANGE if overflow occurs.
Also endp would point to the end of the conversion, so current usage (in _kstrtoull())
rv = _parse_integer(s, base, &_res);
if (rv & KSTRTOX_OVERFLOW)
return -ERANGE;
if (rv == 0)
return -EINVAL;
s += rv;
would become:
ret = _parse_integer(s, &s, base, &_res);
if (ret < 0)
return ret;
if (!ret)
return -EINVAL;
which seems more aligned to the code we see out there.
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: Jonathan Cameron @ 2026-05-17 12:19 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Uwe Kleine-König, Liam Girdwood, Mark Brown,
Linus Walleij, Bartosz Golaszewski, Philipp Zabel,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-pwm, linux-gpio, linux-doc
In-Reply-To: <20260515-ad4692-multichannel-sar-adc-driver-v11-2-eab27d852ac2@analog.com>
On Fri, 15 May 2026 16:31:31 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add support for the Analog Devices AD4691 family of high-speed,
> low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
> AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
> AD4694 (8-ch, 1 MSPS).
>
> The driver implements a custom regmap layer over raw SPI to handle the
> device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> read_raw/write_raw interface for single-channel reads.
>
> The chip idles in Autonomous Mode so that single-shot read_raw can use
> the internal oscillator without disturbing the hardware configuration.
>
> Three voltage supply domains are managed: avdd (required), vio, and a
> reference supply on either the REF pin (ref-supply, external buffer)
> or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> REFBUF_EN is set accordingly). Hardware reset is performed via
> the reset controller framework; a software reset through SPI_CONFIG_A
> is used as fallback when no hardware reset is available.
>
> Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> 16-bit transfer.
>
> IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_separate. The oscillator
> is shared hardware — writing any channel's sampling_frequency attribute
> sets it for all others — but per-channel attributes are used throughout
> the series to avoid an ABI change when per-channel oversampling ratios
> are introduced in a later commit, at which point the effective output
> rate (osc_freq / osr[N]) becomes genuinely per-channel.
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Just a couple of really trivial comments.
I might just have tweaked some of these whilst applying but seeing
as it seems you'll be doing a v12 - over to you :)
Jonathan
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> new file mode 100644
> index 000000000000..ba77e1bfef16
> --- /dev/null
> +++ b/drivers/iio/adc/ad4691.c
> +
> +#define AD4691_OSC_EN_REG 0x180
> +#define AD4691_STATE_RESET_REG 0x181
> +#define AD4691_STATE_RESET_ALL 0x01
Is this a value in a field or documented as whole register value?
I checked, it's BIT(0) - rest of register is Reserved 0
Hence cleaner to use BIT(0) for this definition.
> +#define AD4691_ADC_SETUP 0x182
> +#define AD4691_ADC_MODE_MASK GENMASK(1, 0)
> +#define AD4691_AUTONOMOUS_MODE 0x02
Personal preference would have been to fully define the field values
at this stage, but meh, you bring in the other used ones in later patches
so not critical.
> +
> +#define AD4691_CHANNEL(ch) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) \
> + | BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .info_mask_separate_available = \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE), \
> + .channel = ch, \
> + .scan_index = ch, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 16, \
> + .storagebits = 16, \
Really trivial but I suspect that scan_index and most of scan_type aren't used
until the buffered support is added. So in ideal world add that extra stuff
in that patch. .realbits is used here so fine to initialize that now.
> + }, \
> + }
> +
> +static int ad4691_single_shot_read(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val)
> +{
> + struct ad4691_state *st = iio_priv(indio_dev);
> + unsigned int reg_val, osc_idx, period_us;
> + int ret;
> +
> + guard(mutex)(&st->lock);
> +
> + /* Use AUTONOMOUS mode for single-shot reads. */
> + ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG,
> + AD4691_STATE_RESET_ALL);
This identical to line below that you don't wrap. I don't mind which
but consistency is good.
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> + BIT(chan->channel));
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4691_ACC_MASK_REG,
> + ~BIT(chan->channel) & GENMASK(15, 0));
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 1);
> + if (ret)
> + return ret;
> +
> + osc_idx = FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val);
> + /* Wait 2 oscillator periods for the conversion to complete. */
> + period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC, ad4691_osc_freqs_Hz[osc_idx]);
> + fsleep(period_us);
> +
> + ret = regmap_write(st->regmap, AD4691_OSC_EN_REG, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(st->regmap, AD4691_AVG_IN(chan->channel), ®_val);
> + if (ret)
> + return ret;
> +
> + *val = reg_val;
> +
> + ret = regmap_write(st->regmap, AD4691_STATE_RESET_REG, AD4691_STATE_RESET_ALL);
See above.
> + if (ret)
> + return ret;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static const struct of_device_id ad4691_of_match[] = {
> + { .compatible = "adi,ad4691", .data = &ad4691_chip_info },
> + { .compatible = "adi,ad4692", .data = &ad4692_chip_info },
> + { .compatible = "adi,ad4693", .data = &ad4693_chip_info },
> + { .compatible = "adi,ad4694", .data = &ad4694_chip_info },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad4691_of_match);
> +
> +static const struct spi_device_id ad4691_id[] = {
> + { "ad4691", (kernel_ulong_t)&ad4691_chip_info },
> + { "ad4692", (kernel_ulong_t)&ad4692_chip_info },
> + { "ad4693", (kernel_ulong_t)&ad4693_chip_info },
> + { "ad4694", (kernel_ulong_t)&ad4694_chip_info },
New thing to IIO, but as you are doing another spin anyway can you save
us a future patch. Seems likely that similar to I2C that Uwe is
currently working on, we will eventually move away from stashing pointers
in that ulong_t. So please use named initializers. I'm not sure
why they've been standard for years for of_device_id but not spi_device_id!
Jonathan
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, ad4691_id);
^ permalink raw reply
* Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: Jonathan Cameron @ 2026-05-17 12:14 UTC (permalink / raw)
To: David Lechner
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
Bartosz Golaszewski, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <0696b662-f478-4d1a-95e0-0338bbdb719e@baylibre.com>
On Sat, 16 May 2026 12:11:16 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 5/15/26 8:31 AM, Radu Sabau via B4 Relay wrote:
> > From: Radu Sabau <radu.sabau@analog.com>
> >
> > Add support for the Analog Devices AD4691 family of high-speed,
> > low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
> > AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
> > AD4694 (8-ch, 1 MSPS).
> >
> > The driver implements a custom regmap layer over raw SPI to handle the
> > device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> > read_raw/write_raw interface for single-channel reads.
> >
> > The chip idles in Autonomous Mode so that single-shot read_raw can use
> > the internal oscillator without disturbing the hardware configuration.
> >
> > Three voltage supply domains are managed: avdd (required), vio, and a
> > reference supply on either the REF pin (ref-supply, external buffer)
> > or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> > REFBUF_EN is set accordingly). Hardware reset is performed via
> > the reset controller framework; a software reset through SPI_CONFIG_A
> > is used as fallback when no hardware reset is available.
> >
> > Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> > an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> > 16-bit transfer.
> >
> > IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_separate. The oscillator
> > is shared hardware — writing any channel's sampling_frequency attribute
> > sets it for all others — but per-channel attributes are used throughout
> > the series to avoid an ABI change when per-channel oversampling ratios
> > are introduced in a later commit, at which point the effective output
> > rate (osc_freq / osr[N]) becomes genuinely per-channel.
> >
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > Signed-off-by: Radu Sabau <radu.sabau@analog.com>
One follow up as I was commenting on same code...
> > diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> > new file mode 100644
> > index 000000000000..ba77e1bfef16
> > --- /dev/null
> > +++ b/drivers/iio/adc/ad4691.c
> > +static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
> > +{
> > + unsigned int reg_val;
> > + int ret;
> > +
>
> No mutex lock here? Maybe without OK since it is a read.
Agreed. It's not a bug, but also not a fast path and it will save reasoning
and need for comment to just take the lock.
>
> > + /*
> > + * AD4691_OSC_FREQ_REG is non-volatile and written during
> > + * ad4691_config(), so regmap returns the cached value here without
> > + * touching the SPI bus. No lock is needed.
> > + */
> > + ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
> > + if (ret)
> > + return ret;
> > +
> > + *val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
> > + return IIO_VAL_INT;
> > +}
> > +
> > +static int ad4691_set_sampling_freq(struct iio_dev *indio_dev, int freq)
> > +{
> > + struct ad4691_state *st = iio_priv(indio_dev);
> > + unsigned int start = ad4691_samp_freq_start(st->info);
> > +
> > + IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> > + if (IIO_DEV_ACQUIRE_FAILED(claim))
> > + return -EBUSY;
> > +
> > + for (unsigned int i = start; i < ARRAY_SIZE(ad4691_osc_freqs_Hz); i++) {
> > + if (ad4691_osc_freqs_Hz[i] != freq)
> > + continue;
>
> mutex lock?
Agreed. Whilst the direct mode acquire will serialize that's an internal implementation
detail. Where a driver needs to ensure some sequences are not interrupted
(like I think for the single short read?) then it should take the local
lock.
>
> > + return regmap_update_bits(st->regmap, AD4691_OSC_FREQ_REG,
> > + AD4691_OSC_FREQ_MASK, i);
> > + }
> > +
> > + return -EINVAL;
> > +}
^ permalink raw reply
* Re: [PATCH v13 08/12] iio: frequency: adf41513: driver implementation
From: Rodrigo Alencar @ 2026-05-17 11:56 UTC (permalink / raw)
To: rodrigo.alencar, linux-kernel, linux-iio, devicetree, linux-doc
Cc: Jonathan Cameron, David Lechner, Andy Shevchenko,
Lars-Peter Clausen, Michael Hennerich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Andrew Morton,
Petr Mladek, Steven Rostedt, Andy Shevchenko, Rasmus Villemoes,
Sergey Senozhatsky, Shuah Khan
In-Reply-To: <20260517-adf41513-iio-driver-v13-8-bb6e134a360f@analog.com>
On 26/05/17 10:14AM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
>
> The driver is based on existing PLL drivers in the IIO subsystem and
> implements the following key features:
>
> - Integer-N and fractional-N (fixed/variable modulus) synthesis modes;
> - High-resolution frequency calculations using microhertz (µHz) precision
> to handle sub-Hz resolution across multi-GHz frequency ranges;
> - IIO debugfs interface for direct register access;
> - FW property parsing from devicetree including charge pump settings and
> reference path configuration;
> - Power management support with suspend/resume callbacks;
> - Lock detect GPIO monitoring.
Sashiko's feed back at:
https://sashiko.dev/#/patchset/20260517-adf41513-iio-driver-v13-0-bb6e134a360f%40analog.com?part=8
...
> +static int adf41513_calc_pfd_frequency(struct adf41513_state *st,
> + struct adf41513_pll_settings *result,
> + u64 fpfd_limit_uhz)
> +{
> + result->ref_div2 = st->data.ref_div2_en;
> + result->ref_doubler = st->data.ref_doubler_en;
> +
> + if (st->data.ref_doubler_en &&
> + st->ref_freq_hz > ADF41513_MAX_REF_FREQ_DOUBLER_HZ) {
> + result->ref_doubler = 0;
> + dev_warn(&st->spi->dev, "Disabling ref doubler due to high reference frequency\n");
Will this cause kernel log spam if an application frequently changes the
frequency? Since ref_doubler_en and ref_freq_hz are constant after probe,
could we validate this once during initialization or use dev_warn_once()
here?
Indeed, will move this check to parse_fw()
> + }
...
> +static int adf41513_calc_variable_mod(struct adf41513_state *st,
> + struct adf41513_pll_settings *result)
> +{
> + u64 freq_error_uhz, mod2;
> + u32 frac1, frac2;
> + u32 int_val = div64_u64_rem(result->target_frequency_uhz,
> + result->pfd_frequency_uhz, &freq_error_uhz);
> +
> + if (st->chip_info->has_prescaler_8_9 && int_val >= ADF41513_MIN_INT_FRAC_8_9 &&
> + int_val <= ADF41513_MAX_INT_8_9)
> + result->prescaler = 1;
> + else if (int_val >= ADF41513_MIN_INT_FRAC_4_5 && int_val <= ADF41513_MAX_INT_4_5)
> + result->prescaler = 0;
> + else
> + return -ERANGE;
> +
> + /* calculate required mod2 based on target resolution / 2 */
> + mod2 = DIV64_U64_ROUND_CLOSEST(result->pfd_frequency_uhz << 1,
> + st->data.freq_resolution_uhz * ADF41513_FIXED_MODULUS);
> + /* ensure mod2 is at least 2 for meaningful operation */
> + mod2 = clamp(mod2, 2, ADF41513_MAX_MOD2);
> +
> + /* calculate frac1 and frac2 */
> + frac1 = mul_u64_u64_div_u64(freq_error_uhz, ADF41513_FIXED_MODULUS,
> + result->pfd_frequency_uhz);
> + freq_error_uhz -= mul_u64_u32_div(result->pfd_frequency_uhz, frac1,
> + ADF41513_FIXED_MODULUS);
> + frac2 = mul_u64_u64_div_u64(freq_error_uhz, mod2 * ADF41513_FIXED_MODULUS,
> + result->pfd_frequency_uhz);
Can this calculation result in a frac2 value that exceeds mod2?
Because mul_u64_u32_div() truncates the remainder during the integer
division, the subtracted value might be smaller than the exact mathematical
contribution of frac1. This leaves freq_error_uhz artificially larger than
its theoretical bound.
When frac2 is computed by multiplying this enlarged error by
(mod2 * ADF41513_FIXED_MODULUS), couldn't the result exceed mod2? If
the hardware requires frac2 < mod2, should we cap frac2 or handle the
remainder differently?
will replace to:
/* calculate frac1 and frac2 */
frac1 = mul_u64_u64_div_u64(freq_error_uhz, ADF41513_FIXED_MODULUS,
result->pfd_frequency_uhz);
frac2 = mul_u64_u64_div_u64(freq_error_uhz, mod2 * ADF41513_FIXED_MODULUS,
result->pfd_frequency_uhz) - (u64)frac1 * mod2;
which would remove the possible error introduced by freq_error_uhz -= mul_u64_u32_div().
> +
> + /* integer part */
> + result->actual_frequency_uhz = (u64)int_val * result->pfd_frequency_uhz;
...
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: Jonathan Cameron @ 2026-05-17 11:52 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Uwe Kleine-König, Liam Girdwood, Mark Brown,
Linus Walleij, Bartosz Golaszewski, Philipp Zabel,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-pwm, linux-gpio, linux-doc
In-Reply-To: <20260515-ad4692-multichannel-sar-adc-driver-v11-2-eab27d852ac2@analog.com>
On Fri, 15 May 2026 16:31:31 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add support for the Analog Devices AD4691 family of high-speed,
> low-power multichannel SAR ADCs: AD4691 (16-ch, 500 kSPS),
> AD4692 (16-ch, 1 MSPS), AD4693 (8-ch, 500 kSPS) and
> AD4694 (8-ch, 1 MSPS).
>
> The driver implements a custom regmap layer over raw SPI to handle the
> device's mixed 1/2/3/4-byte register widths and uses the standard IIO
> read_raw/write_raw interface for single-channel reads.
>
> The chip idles in Autonomous Mode so that single-shot read_raw can use
> the internal oscillator without disturbing the hardware configuration.
>
> Three voltage supply domains are managed: avdd (required), vio, and a
> reference supply on either the REF pin (ref-supply, external buffer)
> or the REFIN pin (refin-supply, uses the on-chip reference buffer;
> REFBUF_EN is set accordingly). Hardware reset is performed via
> the reset controller framework; a software reset through SPI_CONFIG_A
> is used as fallback when no hardware reset is available.
>
> Accumulator channel masking for single-shot reads uses ACC_MASK_REG via
> an ADDR_DESCENDING SPI write, which covers both mask bytes in a single
> 16-bit transfer.
>
> IIO_CHAN_INFO_SAMP_FREQ is exposed as info_mask_separate. The oscillator
> is shared hardware — writing any channel's sampling_frequency attribute
> sets it for all others — but per-channel attributes are used throughout
> the series to avoid an ABI change when per-channel oversampling ratios
> are introduced in a later commit, at which point the effective output
> rate (osc_freq / osr[N]) becomes genuinely per-channel.
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Just to make sure it's not missed, one sashiko comment referenced below.
I agree with it that no sleep after coming out of reset is unusual. Not
impossible I suppose, but needs a comment.
> +static int ad4691_reset(struct ad4691_state *st)
> +{
> + struct device *dev = regmap_get_device(st->regmap);
> + struct reset_control *rst;
> +
> + rst = devm_reset_control_get_optional_exclusive(dev, NULL);
> + if (IS_ERR(rst))
> + return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset\n");
> +
> + if (rst) {
> + /*
> + * Assert the reset line before sleeping to guarantee a proper
> + * reset pulse on every probe, including driver reloads where
> + * the line may already be deasserted (reset_control_put() does
> + * not re-assert on release).
> + * devm_reset_control_get_optional_exclusive_deasserted() cannot
> + * be used because it deasserts immediately without delay; the
> + * datasheet (Table 5) requires a ≥300 µs reset pulse width
> + * before deassertion.
> + */
> + reset_control_assert(rst);
> + fsleep(300);
> + return reset_control_deassert(rst);
Sashiko makes the reasonable point that we'd kind of expect some time between
that pin dropping the device out of reset and it being able to respond. If it
really is that quick - then add a comment.
https://sashiko.dev/#/patchset/20260515-ad4692-multichannel-sar-adc-driver-v11-0-eab27d852ac2%40analog.com
> + }
> +
> + /* No hardware reset available, fall back to software reset. */
> + return regmap_write(st->regmap, AD4691_SPI_CONFIG_A_REG,
> + AD4691_SW_RESET);
Same applies here.
> +}
^ permalink raw reply
* Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: Jonathan Cameron @ 2026-05-17 11:50 UTC (permalink / raw)
To: David Lechner
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Uwe Kleine-König, Liam Girdwood, Mark Brown, Linus Walleij,
Bartosz Golaszewski, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <0696b662-f478-4d1a-95e0-0338bbdb719e@baylibre.com>
On Sat, 16 May 2026 12:11:16 -0500
David Lechner <dlechner@baylibre.com> wrote:
> > +static int ad4691_probe(struct spi_device *spi)
> > +{
> > + struct device *dev = &spi->dev;
> > + struct iio_dev *indio_dev;
> > + struct ad4691_state *st;
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> > + if (!indio_dev)
> > + return -ENOMEM;
> > +
> > + st = iio_priv(indio_dev);
> > + st->info = spi_get_device_match_data(spi);
> > + if (!st->info)
> > + return -ENODEV;
>
> We've recently standardized on not checking return value
> of spi_get_device_match_data().
There is a wrinkle in that plan. It's still possible to bind these
drivers to a different ID via driver_override and that will land
us in a NULL dereference.
Until we've closed that out (Andy and others are working on it)
we should probably keep these in. Once that's in place we can
cycle back to clean them out + potentially backport that feature
to ensure drivers that are currently not checking are fine.
>
> > +
> > + ret = devm_mutex_init(dev, &st->lock);
> > + if (ret)
> > + return ret;
> > +
> > + st->regmap = devm_regmap_init(dev, NULL, spi, &ad4691_regmap_config);
> > + if (IS_ERR(st->regmap))
> > + return dev_err_probe(dev, PTR_ERR(st->regmap),
> > + "Failed to initialize regmap\n");
> > +
> > + ret = ad4691_regulator_setup(st);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ad4691_reset(st);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ad4691_config(st);
> > + if (ret)
> > + return ret;
> > +
> > + indio_dev->name = st->info->name;
> > + indio_dev->info = &ad4691_info;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > + indio_dev->channels = st->info->sw_info->channels;
> > + indio_dev->num_channels = st->info->sw_info->num_channels;
> > +
> > + return devm_iio_device_register(dev, indio_dev);
> > +}
^ permalink raw reply
* Re: [PATCH RFC v4 01/10] dt-bindings: iio: frequency: add ad9910
From: Jonathan Cameron @ 2026-05-17 11:28 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-iio,
devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <ikhp5dsb4ook2cx665p4xbqg5ykmnoytiaybv2cx5khyn3wngn@j752jkewd36b>
On Sun, 17 May 2026 11:12:14 +0100
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> On 26/05/16 11:40AM, Jonathan Cameron wrote:
> > On Wed, 13 May 2026 16:09:24 +0100
> > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> >
> > > On 26/05/12 07:31PM, Jonathan Cameron wrote:
> > > > On Fri, 08 May 2026 18:00:17 +0100
> > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> > > >
> > > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > >
> > > > > DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
> > > > > configurations for clocks, DAC current, reset and basic GPIO control.
> > > >
> > > > I think this is getting close enough now that for next version you should
> > > > drop the RFC (which is probably gating DT binding folk giving it
> > > > a detailed review!)
> > > >
> > > > >
> > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > >
> > > > > +
> > > > > + adi,dac-output-current-microamp:
> > > > > + minimum: 8640
> > > > > + maximum: 31590
> > > > > + default: 20070
> > > > > + description:
> > > > > + DAC full-scale output current in microamps.
> > > > > +
> > > > Can we use generic dac.yaml defined output-range-microamp? The base will be 0 always but
> > > > that shouldn't matter.
> > > >
> > >
> > > would that be fine even if we do not have those child channel nodes in the device-tree node?
> > >
> > I think I'd rather we generalized to cover the 'one shared value' case rather
> > then went with a vendor specific binding.
>
> I can't reference dac.yaml in node level as it forces the nodename to match a pattern:
>
> dds@0 (adi,ad9910): $nodename:0: 'dds@0' does not match '^channel(@[0-9a-f]+)?$'
>
> Also, I can't reference the property only, because it ends with *-microamp:
>
> output-range-microamp:
> $ref: /schemas/iio/dac/dac.yaml#/properties/output-range-microamp
> items:
> - const: 0
> - minimum: 8640
> maximum: 31590
> default: 20070
>
> which gets me:
>
> properties:output-range-microamp: '$ref' should not be valid under {'const': '$ref'}
>
> so I will adjust it to:
>
> output-range-microamp:
> description: DAC full-scale output current in microamps.
> items:
> - const: 0
> - minimum: 8640
> maximum: 31590
> default: 20070
>
> and not reference dac.yaml at all.
>
That works. Thanks
^ permalink raw reply
* [PATCH net-next 2/2] net/mlx5: implement max_sfs parameter
From: Tariq Toukan @ 2026-05-17 11:27 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jiri Pirko, Simon Horman, Jonathan Corbet, Shuah Khan,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Vlad Dumitrescu, Aleksandr Loktionov, Daniel Zahka, David Ahern,
Nikolay Aleksandrov, netdev, linux-doc, linux-kernel, linux-rdma,
Gal Pressman, Dragos Tatulea, Jiri Pirko, Nikolay Aleksandrov
In-Reply-To: <20260517112700.343575-1-tariqt@nvidia.com>
From: Nikolay Aleksandrov <nikolay@nvidia.com>
Implement max_sfs generic parameter to allow users to control the total
light-weight NIC subfunctions that can be created using devlink instead
of external vendor tools. A value of 0 will effectively disable creation
of new subfunction devices. A warning is sent to user-space via extack
(returning extack without error code is interpreted as a warning by
user-space tools).
Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/networking/devlink/mlx5.rst | 7 +-
.../mellanox/mlx5/core/lib/nv_param.c | 83 ++++++++++++++++++-
2 files changed, 86 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/devlink/mlx5.rst b/Documentation/networking/devlink/mlx5.rst
index 4bba4d780a4a..283b93d16861 100644
--- a/Documentation/networking/devlink/mlx5.rst
+++ b/Documentation/networking/devlink/mlx5.rst
@@ -45,8 +45,13 @@ Parameters
- The range is between 1 and a device-specific max.
- Applies to each physical function (PF) independently, if the device
supports it. Otherwise, it applies symmetrically to all PFs.
+ * - ``max_sfs``
+ - permanent
+ - The range is between 0 and a device-specific max.
+ - Applies to each physical function (PF) independently.
-Note: permanent parameters such as ``enable_sriov`` and ``total_vfs`` require FW reset to take effect
+Note: permanent parameters such as ``enable_sriov``, ``total_vfs` and ``max_sfs``
+ require FW reset to take effect
.. code-block:: bash
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
index 19bb620b7436..eff3a67e4ca0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
@@ -68,7 +68,9 @@ struct mlx5_ifc_mnvda_reg_bits {
struct mlx5_ifc_nv_global_pci_conf_bits {
u8 sriov_valid[0x1];
- u8 reserved_at_1[0x10];
+ u8 reserved_at_1[0xa];
+ u8 per_pf_num_sf[0x1];
+ u8 reserved_at_c[0x5];
u8 per_pf_total_vf[0x1];
u8 reserved_at_12[0xe];
@@ -93,9 +95,11 @@ struct mlx5_ifc_nv_global_pci_cap_bits {
};
struct mlx5_ifc_nv_pf_pci_conf_bits {
- u8 reserved_at_0[0x9];
+ u8 log_sf_bar_size[0x8];
+ u8 pf_total_sf_en[0x1];
u8 pf_total_vf_en[0x1];
- u8 reserved_at_a[0x16];
+ u8 reserved_at_a[0x6];
+ u8 total_sf[0x10];
u8 reserved_at_20[0x20];
@@ -755,6 +759,76 @@ static int mlx5_devlink_total_vfs_validate(struct devlink *devlink, u32 id,
return 0;
}
+static int mlx5_devlink_max_sfs_get(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+ u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
+ void *data;
+ int err;
+
+ err = mlx5_nv_param_read_per_host_pf_conf(dev, mnvda, sizeof(mnvda));
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to read PF configuration");
+ return err;
+ }
+
+ data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
+ ctx->val.vu32 = MLX5_GET(nv_pf_pci_conf, data, total_sf);
+
+ return 0;
+}
+
+static int mlx5_devlink_max_sfs_set(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+ u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
+ void *data;
+ int err;
+
+ err = mlx5_nv_param_read_global_pci_conf(dev, mnvda, sizeof(mnvda));
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to read global PCI configuration");
+ return err;
+ }
+
+ data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
+ MLX5_SET(nv_global_pci_conf, data, per_pf_num_sf, !!ctx->val.vu32);
+
+ err = mlx5_nv_param_write(dev, mnvda, sizeof(mnvda));
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to change per_pf_num_sf global PCI configuration");
+ return err;
+ }
+
+ memset(mnvda, 0, sizeof(mnvda));
+ err = mlx5_nv_param_read_per_host_pf_conf(dev, mnvda, sizeof(mnvda));
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to read PF configuration");
+ return err;
+ }
+
+ data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
+ MLX5_SET(nv_pf_pci_conf, data, log_sf_bar_size, ctx->val.vu32 ? 12 : 0);
+ MLX5_SET(nv_pf_pci_conf, data, pf_total_sf_en, !!ctx->val.vu32);
+ MLX5_SET(nv_pf_pci_conf, data, total_sf, ctx->val.vu32);
+
+ err = mlx5_nv_param_write(dev, mnvda, sizeof(mnvda));
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to change PF PCI configuration");
+ return err;
+ }
+ NL_SET_ERR_MSG(extack, "Modifying max_sfs requires a reboot");
+
+ return 0;
+}
+
static const struct devlink_param mlx5_nv_param_devlink_params[] = {
DEVLINK_PARAM_GENERIC(ENABLE_SRIOV, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
mlx5_devlink_enable_sriov_get,
@@ -763,6 +837,9 @@ static const struct devlink_param mlx5_nv_param_devlink_params[] = {
mlx5_devlink_total_vfs_get,
mlx5_devlink_total_vfs_set,
mlx5_devlink_total_vfs_validate),
+ DEVLINK_PARAM_GENERIC(MAX_SFS, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+ mlx5_devlink_max_sfs_get,
+ mlx5_devlink_max_sfs_set, NULL),
DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_CQE_COMPRESSION_TYPE,
"cqe_compress_type", DEVLINK_PARAM_TYPE_STRING,
BIT(DEVLINK_PARAM_CMODE_PERMANENT),
--
2.44.0
^ permalink raw reply related
* [PATCH net-next 1/2] devlink: add generic device max_sfs parameter
From: Tariq Toukan @ 2026-05-17 11:26 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jiri Pirko, Simon Horman, Jonathan Corbet, Shuah Khan,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Vlad Dumitrescu, Aleksandr Loktionov, Daniel Zahka, David Ahern,
Nikolay Aleksandrov, netdev, linux-doc, linux-kernel, linux-rdma,
Gal Pressman, Dragos Tatulea, Jiri Pirko, Nikolay Aleksandrov
In-Reply-To: <20260517112700.343575-1-tariqt@nvidia.com>
From: Nikolay Aleksandrov <nikolay@nvidia.com>
Add a new generic devlink device parameter (max_sfs) to control if and
how many light-weight NIC subfunctions can be created. Subfunctions are
a light-weight network functions backed by an underlying PCI function.
Their lifecycle can already be managed by devlink, but currently users
cannot enable them in the device. They can be enabled/disabled only via
external vendor tools. This parameter allows subfunctions to be enabled
(>0) or disabled (0) via devlink. A subsequent patch will add support
for max_sfs to the mlx5 driver.
Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/networking/devlink/devlink-params.rst | 6 ++++++
include/net/devlink.h | 4 ++++
net/devlink/param.c | 5 +++++
3 files changed, 15 insertions(+)
diff --git a/Documentation/networking/devlink/devlink-params.rst b/Documentation/networking/devlink/devlink-params.rst
index ea17756dcda6..29b8a9246fb6 100644
--- a/Documentation/networking/devlink/devlink-params.rst
+++ b/Documentation/networking/devlink/devlink-params.rst
@@ -165,3 +165,9 @@ own name.
- u32
- Controls the maximum number of MAC address filters that can be assigned
to a Virtual Function (VF).
+ * - ``max_sfs``
+ - u32
+ - The maximum number of subfunctions which can be created on the device.
+ Modifying this parameter may require a device restart and PCI bus
+ rescanning because the BAR layout may change. A value of 0 disables
+ subfunction creation.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index bcd31de1f890..4ec455cfe7a4 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -546,6 +546,7 @@ enum devlink_param_generic_id {
DEVLINK_PARAM_GENERIC_ID_TOTAL_VFS,
DEVLINK_PARAM_GENERIC_ID_NUM_DOORBELLS,
DEVLINK_PARAM_GENERIC_ID_MAX_MAC_PER_VF,
+ DEVLINK_PARAM_GENERIC_ID_MAX_SFS,
/* add new param generic ids above here*/
__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -619,6 +620,9 @@ enum devlink_param_generic_id {
#define DEVLINK_PARAM_GENERIC_MAX_MAC_PER_VF_NAME "max_mac_per_vf"
#define DEVLINK_PARAM_GENERIC_MAX_MAC_PER_VF_TYPE DEVLINK_PARAM_TYPE_U32
+#define DEVLINK_PARAM_GENERIC_MAX_SFS_NAME "max_sfs"
+#define DEVLINK_PARAM_GENERIC_MAX_SFS_TYPE DEVLINK_PARAM_TYPE_U32
+
#define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \
{ \
.id = DEVLINK_PARAM_GENERIC_ID_##_id, \
diff --git a/net/devlink/param.c b/net/devlink/param.c
index cf95268da5b0..523243e49d88 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -117,6 +117,11 @@ static const struct devlink_param devlink_param_generic[] = {
.name = DEVLINK_PARAM_GENERIC_MAX_MAC_PER_VF_NAME,
.type = DEVLINK_PARAM_GENERIC_MAX_MAC_PER_VF_TYPE,
},
+ {
+ .id = DEVLINK_PARAM_GENERIC_ID_MAX_SFS,
+ .name = DEVLINK_PARAM_GENERIC_MAX_SFS_NAME,
+ .type = DEVLINK_PARAM_GENERIC_MAX_SFS_TYPE,
+ },
};
static int devlink_param_generic_verify(const struct devlink_param *param)
--
2.44.0
^ permalink raw reply related
* [PATCH net-next 0/2] devlink: add generic max_sfs parameter and mlx5 support
From: Tariq Toukan @ 2026-05-17 11:26 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jiri Pirko, Simon Horman, Jonathan Corbet, Shuah Khan,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Vlad Dumitrescu, Aleksandr Loktionov, Daniel Zahka, David Ahern,
Nikolay Aleksandrov, netdev, linux-doc, linux-kernel, linux-rdma,
Gal Pressman, Dragos Tatulea, Jiri Pirko
Hi,
This series by Nikolay introduces a new generic devlink device
parameter, max_sfs, to control the number of light-weight NIC
subfunctions (SFs) that can be created on a device.
The first patch adds the generic devlink parameter and infrastructure
support.
The second patch implements support for the parameter in the mlx5
driver.
With this addition, users can enable or disable SF creation directly via
devlink, without relying on external vendor-specific tools.
Regards,
Tariq
Nikolay Aleksandrov (2):
devlink: add generic device max_sfs parameter
net/mlx5: implement max_sfs parameter
.../networking/devlink/devlink-params.rst | 6 ++
Documentation/networking/devlink/mlx5.rst | 7 +-
.../mellanox/mlx5/core/lib/nv_param.c | 83 ++++++++++++++++++-
include/net/devlink.h | 4 +
net/devlink/param.c | 5 ++
5 files changed, 101 insertions(+), 4 deletions(-)
base-commit: 627ac78f2741e2ebd2225e2e953b6964a8a9182f
--
2.44.0
^ permalink raw reply
* [PATCH] docs/gpu: fix typo in msm-crash-dump.rst
From: Sakurai Shun @ 2026-05-17 11:06 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jonathan Corbet, Shuah Khan
Cc: Sakurai Shun, dri-devel, linux-doc, linux-kernel
Replace "uinque" with "unique"
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/gpu/msm-crash-dump.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gpu/msm-crash-dump.rst b/Documentation/gpu/msm-crash-dump.rst
index 9509cc422..474c0d265 100644
--- a/Documentation/gpu/msm-crash-dump.rst
+++ b/Documentation/gpu/msm-crash-dump.rst
@@ -70,7 +70,7 @@ ringbuffer
bo
List of buffers from the hanging submission if available.
- Each buffer object will have a uinque iova.
+ Each buffer object will have a unique iova.
iova
GPU address of the buffer object.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v13 06/12] iio: core: add decimal value formatting into 64-bit value
From: Rodrigo Alencar @ 2026-05-17 10:44 UTC (permalink / raw)
To: Andy Shevchenko, rodrigo.alencar
Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
David Lechner, Andy Shevchenko, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Andrew Morton, Petr Mladek, Steven Rostedt,
Rasmus Villemoes, Sergey Senozhatsky, Shuah Khan
In-Reply-To: <agmVYvWBmLI4A65m@ashevche-desk.local>
On 26/05/17 01:16PM, Andy Shevchenko wrote:
> On Sun, May 17, 2026 at 10:14:01AM +0100, Rodrigo Alencar via B4 Relay wrote:
>
> > Create new format types for iio values (IIO_VAL_DECIMAL64_*), which
> > defines the representation of fixed decimal point values into a single
> > 64-bit number. This new format increases the range of represented values,
> > allowing for integer parts greater than 2^32, as bits are not "wasted"
> > in the fractional part, which can be seen in IIO_VAL_INT_PLUS_MICRO and
> > IIO_VAL_INT_PLUS_NANO. Helpers are created to compose and decompose 64-bit
> > decimals into integer values used in IIO formatting interfaces, which
> > creates consistency and avoid error-prone manual assignments when using
> > wordpart macros. When doing the parsing, kstrtodec64() is used with the
> > scale defined by the specific decimal format type.
>
> ...
>
> > + {
> > + int scale = type - IIO_VAL_DECIMAL64_BASE;
>
> > + int l = 0;
>
> Perhaps make it global in the function? We have the same in
> IIO_VAL_INT_MULTIPLE case.
>
> > + s64 frac;
> > +
> > + tmp2 = div64_s64_rem(iio_val_s64_from_s32s(vals),
> > + int_pow(10, scale), &frac);
> > + if (tmp2 == 0 && frac < 0)
> > + l += sysfs_emit_at(buf, offset, "-");
> > +
> > + l += sysfs_emit_at(buf, offset + l, "%lld.%0*lld", tmp2, scale,
> > + abs(frac));
> > + return l;
> > + }
>
> ...
>
> > #ifndef _IIO_TYPES_H_
> > #define _IIO_TYPES_H_
>
> Also needs types.h now... Which makes me think if the proposed macros are
> placed in the good enough location.
That is a good point. I left it there because they are related to IIO_VAL_INT_64
and IIO_VAL_DECIMAL64_*. I had them as macros initially, but they are not exactly
"types" indeed.
>
> (Note, iio/iio.h missing actually types.h, but includes it indirectly.)
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* [PATCH] docs/mm: fix typo in process_addrs.rst
From: Sakurai Shun @ 2026-05-17 10:36 UTC (permalink / raw)
To: Andrew Morton, Suren Baghdasaryan, Liam R. Howlett,
Lorenzo Stoakes, Vlastimil Babka, Shakeel Butt, David Hildenbrand,
Mike Rapoport, Michal Hocko, Jonathan Corbet, Shuah Khan
Cc: Sakurai Shun, linux-mm, linux-doc, linux-kernel
Replace "presense" with "presence"
Signed-off-by: Sakurai Shun <ssh1326@icloud.com>
---
Documentation/mm/process_addrs.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/mm/process_addrs.rst b/Documentation/mm/process_addrs.rst
index 851680ead..042d64d72 100644
--- a/Documentation/mm/process_addrs.rst
+++ b/Documentation/mm/process_addrs.rst
@@ -775,7 +775,7 @@ lock, releasing or downgrading the mmap write lock also releases the VMA write
lock so there is no :c:func:`!vma_end_write` function.
Note that when write-locking a VMA lock, the :c:member:`!vma.vm_refcnt` is temporarily
-modified so that readers can detect the presense of a writer. The reference counter is
+modified so that readers can detect the presence of a writer. The reference counter is
restored once the vma sequence number used for serialisation is updated.
This ensures the semantics we require - VMA write locks provide exclusive write
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v13 06/12] iio: core: add decimal value formatting into 64-bit value
From: Andy Shevchenko @ 2026-05-17 10:16 UTC (permalink / raw)
To: rodrigo.alencar
Cc: linux-kernel, linux-iio, devicetree, linux-doc, Jonathan Cameron,
David Lechner, Andy Shevchenko, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Andrew Morton, Petr Mladek, Steven Rostedt,
Rasmus Villemoes, Sergey Senozhatsky, Shuah Khan
In-Reply-To: <20260517-adf41513-iio-driver-v13-6-bb6e134a360f@analog.com>
On Sun, May 17, 2026 at 10:14:01AM +0100, Rodrigo Alencar via B4 Relay wrote:
> Create new format types for iio values (IIO_VAL_DECIMAL64_*), which
> defines the representation of fixed decimal point values into a single
> 64-bit number. This new format increases the range of represented values,
> allowing for integer parts greater than 2^32, as bits are not "wasted"
> in the fractional part, which can be seen in IIO_VAL_INT_PLUS_MICRO and
> IIO_VAL_INT_PLUS_NANO. Helpers are created to compose and decompose 64-bit
> decimals into integer values used in IIO formatting interfaces, which
> creates consistency and avoid error-prone manual assignments when using
> wordpart macros. When doing the parsing, kstrtodec64() is used with the
> scale defined by the specific decimal format type.
...
> + {
> + int scale = type - IIO_VAL_DECIMAL64_BASE;
> + int l = 0;
Perhaps make it global in the function? We have the same in
IIO_VAL_INT_MULTIPLE case.
> + s64 frac;
> +
> + tmp2 = div64_s64_rem(iio_val_s64_from_s32s(vals),
> + int_pow(10, scale), &frac);
> + if (tmp2 == 0 && frac < 0)
> + l += sysfs_emit_at(buf, offset, "-");
> +
> + l += sysfs_emit_at(buf, offset + l, "%lld.%0*lld", tmp2, scale,
> + abs(frac));
> + return l;
> + }
...
> #ifndef _IIO_TYPES_H_
> #define _IIO_TYPES_H_
Also needs types.h now... Which makes me think if the proposed macros are
placed in the good enough location.
(Note, iio/iio.h missing actually types.h, but includes it indirectly.)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v4 01/10] dt-bindings: iio: frequency: add ad9910
From: Rodrigo Alencar @ 2026-05-17 10:12 UTC (permalink / raw)
To: Jonathan Cameron, Rodrigo Alencar
Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-iio,
devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <20260516114022.58949a06@jic23-huawei>
On 26/05/16 11:40AM, Jonathan Cameron wrote:
> On Wed, 13 May 2026 16:09:24 +0100
> Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
>
> > On 26/05/12 07:31PM, Jonathan Cameron wrote:
> > > On Fri, 08 May 2026 18:00:17 +0100
> > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> > >
> > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > >
> > > > DT-bindings for AD9910, a 1 GSPS DDS with 14-bit DAC. It includes
> > > > configurations for clocks, DAC current, reset and basic GPIO control.
> > >
> > > I think this is getting close enough now that for next version you should
> > > drop the RFC (which is probably gating DT binding folk giving it
> > > a detailed review!)
> > >
> > > >
> > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > >
> > > > +
> > > > + adi,dac-output-current-microamp:
> > > > + minimum: 8640
> > > > + maximum: 31590
> > > > + default: 20070
> > > > + description:
> > > > + DAC full-scale output current in microamps.
> > > > +
> > > Can we use generic dac.yaml defined output-range-microamp? The base will be 0 always but
> > > that shouldn't matter.
> > >
> >
> > would that be fine even if we do not have those child channel nodes in the device-tree node?
> >
> I think I'd rather we generalized to cover the 'one shared value' case rather
> then went with a vendor specific binding.
I can't reference dac.yaml in node level as it forces the nodename to match a pattern:
dds@0 (adi,ad9910): $nodename:0: 'dds@0' does not match '^channel(@[0-9a-f]+)?$'
Also, I can't reference the property only, because it ends with *-microamp:
output-range-microamp:
$ref: /schemas/iio/dac/dac.yaml#/properties/output-range-microamp
items:
- const: 0
- minimum: 8640
maximum: 31590
default: 20070
which gets me:
properties:output-range-microamp: '$ref' should not be valid under {'const': '$ref'}
so I will adjust it to:
output-range-microamp:
description: DAC full-scale output current in microamps.
items:
- const: 0
- minimum: 8640
maximum: 31590
default: 20070
and not reference dac.yaml at all.
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Miguel Ojeda @ 2026-05-17 9:58 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd,
gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej,
linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <ba7ec52f-c4e9-4588-9484-dc8280d55593@gmail.com>
On Sun, May 17, 2026 at 8:10 AM Demi Marie Obenour
<demiobenour@gmail.com> wrote:
>
> I think it is simpler to just inline all of this code into its
> single call-site. The safety of the code is obvious in context,
> and you can avoid checking for impossible errors. For instance,
> since all of the options have required arguments, it really is safe
> to dereference optarg without any null check.
If we are going to have unsafe code, then let's please build safe
abstractions wherever possible, just like we do elsewhere. We should
also write `// SAFETY` comments and enable the lints that catch that
etc., just like elsewhere too.
(This is not to say we should use `getopt` instead of something like
`clap` -- as soon as we start using `cargo vendor`, then it makes
sense to at least consider having a set of vetted, well-known crates
to write Rust tools in-tree, as I mentioned in v1.)
Cheers,
Miguel
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Miguel Ojeda @ 2026-05-17 9:48 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd,
gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej,
linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <18809682-aa16-4ab4-b615-cefa525c872f@gmail.com>
On Sun, May 17, 2026 at 11:33 AM Demi Marie Obenour
<demiobenour@gmail.com> wrote:
>
> That's true. Some distros (Fedora, Debian) don't like that either,
> but that's a bigger ecosystem-wide concern.
Do you mean that distributions would like to package this tool on
their own? If so, I don't see why packagers would be blocked.
Cheers,
Miguel
^ permalink raw reply
* Re: Re: [PATCH v2] dcache: add fs.dentry-limit sysctl with negative-first reaper
From: Horst Birthelmer @ 2026-05-17 9:42 UTC (permalink / raw)
To: Mateusz Guzik
Cc: Horst Birthelmer, Miklos Szeredi, Jonathan Corbet, Shuah Khan,
Alexander Viro, Christian Brauner, Jan Kara, linux-doc,
linux-kernel, linux-fsdevel, Horst Birthelmer
In-Reply-To: <5afacskoalmd2u6s525dosvyrtr3j66ajd5m4p2ylymtlgytkz@excrdfpndx37>
On Sun, May 17, 2026 at 11:15:04AM +0200, Mateusz Guzik wrote:
> On Sat, May 16, 2026 at 04:52:54PM +0200, Horst Birthelmer wrote:
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > The dcache only shrinks under memory pressure, which is rarely reached
> > on machines with ample RAM, so cached negative dentries can accumulate
> > without bound. Give administrators a soft cap they can set,
> > and a background worker that prefers negative dentries when reclaiming.
> >
> > Two new sysctls under /proc/sys/fs/:
> >
> > dentry-limit -- soft cap on nr_dentry. 0 (default)
> > disables the feature; behaviour is then
> > identical to before.
> > dentry-limit-interval-ms -- pacing for the worker while still over
> > the cap. Default 1000, minimum 1.
> >
> > When the cap is exceeded, a delayed_work runs in two phases:
> >
> > 1. iterate_supers() draining only negative dentries from every LRU.
> > Positive entries are rotated past so the walk makes progress.
> > DCACHE_REFERENCED is ignored here on purpose -- an admin-imposed
> > cap should evict even hot negatives before any positive entry.
> > 2. If still over the cap, iterate_supers() again with the same
> > isolate callback the memory-pressure shrinker uses.
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> > There was a discussion at LSFMM about servers with too many cached
> > negative dentries.
> > That gave me the idea to keep the dentries in general limited
> > if the system administrator needs it to.
> >
>
> I wrote about the negative entries problem here:
>
> https://lore.kernel.org/linux-fsdevel/f7bp3ggliqbb7adyysonxgvo6zn76mo4unroagfcuu3bfghynu@7wkgqkfb5c43/#t
>
> The mechanism as suggested here will end up evicting *useful* negative
> entries. Granted, they will be recreated soon enough so it's not a
> tragedy but it still is an avoidable perf loss.
>
> What is needed in the long run is a mechanism which aggressively
> recycles stale negative entries and recognizes which ones should be
> saved for the time being.
>
> Below some magic threshold you just allocate a new negative entry.
>
> All new entries would get a grace period where they need to get hits and
> prove useful OR get whacked. If you are at or above the threshold and
> are allocating a new entry, you can whack the oldest negative one which
> did not make it.
>
> This is just one idea, what is not up for debate is the discrepancy
> between small subset of negative entires with tons of hits vs the ones
> which get virtually no traffic at all.
I'm trying not to focus that much on the negative dentries since it has
no relevance for fuse, but was just a nice effect to solve that one, too,
and a bit of 'when you're at it' logic.
I'm more interested in throwing out the unused ones.
You are completely right in your analysis that this could remove fresh
and useful negative dentries.
>
> Whatever the mechanism it will have to take advantage of it.
>
> > This is somewhat related to [1] where it would address the same
> > symptoms but in a more unobtrusive way, by just garbage collecting
> > the negative and then the unused cache entries.
> >
> > The other effect I have seen regarding this is that FUSE
> > will not forget inodes (no FORGET call to the FUSE server)
> > even after the latest reference has been closed until much later.
> >
> > In a FUSE server that mirrors the kernel cached inodes in user space
> > because it has to keep a lot of private data for every node
> > this puts an unnecessarry memory strain on that userspace entity
> > especially if the memory is limited for its cgroup.
>
> I don't know anything about how FUSE works. In this context I presume
> you have a mount point backed by FUSE and the problematic memory usage
> stems from inodes created against such a mount point.
>
correct
> This would suggest you would be better served with a mechanism which
> allows userspace to cull some number of dentries for a given mount
> point, maybe even with an optional preference for negative entries if
> that's considered better for given fs.
>
As I mentioned in the other post, I kinda did this (not triggered by user
space, though, just by a limit negotiated during init with user space)
just for fuse and was told that this kind of limit would be useful in vfs.
> Or to put it differently, I would look into exposing sb shrinkers to
> root instead of rolling with a global scan.
This would be a cool idea.
>
> > +static enum lru_status dentry_lru_isolate_negative(struct list_head *item,
> > + struct list_lru_one *lru, void *arg)
> > +{
> > + struct list_head *freeable = arg;
> > + struct dentry *dentry = container_of(item, struct dentry, d_lru);
> > +
> > + if (!spin_trylock(&dentry->d_lock))
> > + return LRU_SKIP;
>
> If anything of the sort is to land, you definitely want to pre-check
> d_count and d_is_negative without the lock.
probably ...
I still think that a lock held is a good indicator that we can just move on.
Thanks for your time,
Horst
^ permalink raw reply
* Re: [RFC PATCH v3 2/3] Documentation: add kconfirm
From: Nathan Chancellor @ 2026-05-17 9:40 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Julian Braha, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh,
masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour,
ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <CANiq72=dKOhoLUoWRmzG9Kyv0jWY97Nx_O4rWV-UHjRtULz-jg@mail.gmail.com>
On Sun, May 17, 2026 at 08:05:30AM +0200, Miguel Ojeda wrote:
> On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote:
> > +In ``scripts/kconfirm/`` run the following to download the dependencies::
> > +
> > + cargo vendor
>
> I am not sure how important this is for `scripts/` and/or `tools/`
> (Kbuild may have a policy), but this should probably handle `O=`
> builds.
>
> In some cases, the source tree may even be read-only, i.e. we wouldn't
> be able to create `target/` there.
I guess this is kind of a weird/unique situation. I agree that the files
generated by 'cargo run' should absolutely be contained within the build
folder; at that point, $(srctree) could be read only and I would
consider it rude not to respect the user's choice of build directory.
For 'cargo vendor' however, I am not sure. They are source files and I
would expect that running 'cargo vendor' would be more considered part
of preparing the source tree, rather than the build one (so it should
not be read only).
At the same time, it might be safer for dependency updates and internal
consistency that they are confined to the build folder. I guess we would
only want to remove them with a 'distclean', rather than 'mrproper' or
'clean', in that case, to avoid requiring users to constantly run
'cargo vendor'. It might be more ergonomic for this to be a Kbuild
target ('kconfirmvendor'?) so that this could be handled automatically
based on the user's build command.
Additionally, can we detect explicitly when dependencies are not
properly vendored and error with a more helpful error message? The build
command in patch 1 just throws up its hands when the build fails and
asks if the dependencies have been set up but if we provided our own
vendoring build target, we could add some canary that says we vendored
successfully and if that is not present, error before even running the
build and say "hey, you need to explicitly run this target before you
build".
--
Cheers,
Nathan
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Demi Marie Obenour @ 2026-05-17 9:32 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd,
gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej,
linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <CANiq72=9nxRgfFf1WzWgp=TP9or=Mi=wLyME9-f2M4hti+ZNcg@mail.gmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 842 bytes --]
On 5/17/26 05:30, Miguel Ojeda wrote:
> On Sun, May 17, 2026 at 9:32 AM Demi Marie Obenour
> <demiobenour@gmail.com> wrote:
>>
>> Using a ton of vendored dependencies would make for unreviewable
>> patches.
>
> I am referring to `cargo vendor` here, not to copying in-tree -- we
> already discussed that in previous versions.
That's true. Some distros (Fedora, Debian) don't like that either,
but that's a bigger ecosystem-wide concern.
> Please note that I was arguing for avoiding actual vendoring in
> previous versions...
>
>> I'm the one who suggested using FFI here and for command-line parsing.
>> The command-line interface would also work.
>
> Yes, I suggested the CLI in v1, and then you mentioned the library in v2.
CLI would also work just as well.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Miguel Ojeda @ 2026-05-17 9:30 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd,
gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej,
linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <615113d6-7e90-4d54-ad1f-a6833474e8c9@gmail.com>
On Sun, May 17, 2026 at 9:32 AM Demi Marie Obenour
<demiobenour@gmail.com> wrote:
>
> Using a ton of vendored dependencies would make for unreviewable
> patches.
I am referring to `cargo vendor` here, not to copying in-tree -- we
already discussed that in previous versions.
Please note that I was arguing for avoiding actual vendoring in
previous versions...
> I'm the one who suggested using FFI here and for command-line parsing.
> The command-line interface would also work.
Yes, I suggested the CLI in v1, and then you mentioned the library in v2.
Cheers,
Miguel
^ permalink raw reply
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm
From: Nathan Chancellor @ 2026-05-17 9:28 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Julian Braha, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh,
masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour,
ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild
In-Reply-To: <CANiq72kr=tzvEitYj6xyT=jGnKQZK1dmekSU3us7MWGTrv0FNA@mail.gmail.com>
On Sun, May 17, 2026 at 08:28:16AM +0200, Miguel Ojeda wrote:
> On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote:
> >
> > +CARGO = cargo
>
> Question to Kbuild: would it hurt to hardcore `--offline` here?
>
> If someone within Make actually ever needs Cargo to fetch something,
> then they should be very explicit about it (in which case we could
> have another variable etc.).
No, I don't think so. I think there would need to be a very compelling
reason for connecting to the network during the build process. Although,
we would need to handle someone passing CARGO via the make command line
so that '--offline' does not get blown away.
> > - rust/libpin_init_internal.so rust/libpin_init_internal.dylib
> > + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \
>
> Spurious change?
Maybe 'scripts/kconfirm' used to be here?
Another thing I just realized: scripts/kconfirm is going to mess with
shell autocompletion for some people, as scripts/kc<tab> will currently
always complete to scripts/kconfig. Not sure if that will be that big of
a deal but I know Linus has complained about that in the past.
> > +$(TARGET):
> > + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS)
>
> This probably does not work in `O=` builds or in cases where the
> `srctree` is read-only (please see my other reply on the docs patch).
Yeah, it seems like this wants something like '--target-dir $(obj)' or
'--target-dir $(objtree)/scripts/kconfirm'? I don't find this to be
particularly readable either (I am more used to "build then run" as two
separate steps) but maybe that is because I am just not familiar with
Rust projects.
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v2] dcache: add fs.dentry-limit sysctl with negative-first reaper
From: Mateusz Guzik @ 2026-05-17 9:15 UTC (permalink / raw)
To: Horst Birthelmer
Cc: Miklos Szeredi, Jonathan Corbet, Shuah Khan, Alexander Viro,
Christian Brauner, Jan Kara, linux-doc, linux-kernel,
linux-fsdevel, Horst Birthelmer
In-Reply-To: <20260516-limit-dentries-cache-v2-1-c733a78e603b@ddn.com>
On Sat, May 16, 2026 at 04:52:54PM +0200, Horst Birthelmer wrote:
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> The dcache only shrinks under memory pressure, which is rarely reached
> on machines with ample RAM, so cached negative dentries can accumulate
> without bound. Give administrators a soft cap they can set,
> and a background worker that prefers negative dentries when reclaiming.
>
> Two new sysctls under /proc/sys/fs/:
>
> dentry-limit -- soft cap on nr_dentry. 0 (default)
> disables the feature; behaviour is then
> identical to before.
> dentry-limit-interval-ms -- pacing for the worker while still over
> the cap. Default 1000, minimum 1.
>
> When the cap is exceeded, a delayed_work runs in two phases:
>
> 1. iterate_supers() draining only negative dentries from every LRU.
> Positive entries are rotated past so the walk makes progress.
> DCACHE_REFERENCED is ignored here on purpose -- an admin-imposed
> cap should evict even hot negatives before any positive entry.
> 2. If still over the cap, iterate_supers() again with the same
> isolate callback the memory-pressure shrinker uses.
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
> There was a discussion at LSFMM about servers with too many cached
> negative dentries.
> That gave me the idea to keep the dentries in general limited
> if the system administrator needs it to.
>
I wrote about the negative entries problem here:
https://lore.kernel.org/linux-fsdevel/f7bp3ggliqbb7adyysonxgvo6zn76mo4unroagfcuu3bfghynu@7wkgqkfb5c43/#t
The mechanism as suggested here will end up evicting *useful* negative
entries. Granted, they will be recreated soon enough so it's not a
tragedy but it still is an avoidable perf loss.
What is needed in the long run is a mechanism which aggressively
recycles stale negative entries and recognizes which ones should be
saved for the time being.
Below some magic threshold you just allocate a new negative entry.
All new entries would get a grace period where they need to get hits and
prove useful OR get whacked. If you are at or above the threshold and
are allocating a new entry, you can whack the oldest negative one which
did not make it.
This is just one idea, what is not up for debate is the discrepancy
between small subset of negative entires with tons of hits vs the ones
which get virtually no traffic at all.
Whatever the mechanism it will have to take advantage of it.
> This is somewhat related to [1] where it would address the same
> symptoms but in a more unobtrusive way, by just garbage collecting
> the negative and then the unused cache entries.
>
> The other effect I have seen regarding this is that FUSE
> will not forget inodes (no FORGET call to the FUSE server)
> even after the latest reference has been closed until much later.
>
> In a FUSE server that mirrors the kernel cached inodes in user space
> because it has to keep a lot of private data for every node
> this puts an unnecessarry memory strain on that userspace entity
> especially if the memory is limited for its cgroup.
I don't know anything about how FUSE works. In this context I presume
you have a mount point backed by FUSE and the problematic memory usage
stems from inodes created against such a mount point.
This would suggest you would be better served with a mechanism which
allows userspace to cull some number of dentries for a given mount
point, maybe even with an optional preference for negative entries if
that's considered better for given fs.
Or to put it differently, I would look into exposing sb shrinkers to
root instead of rolling with a global scan.
> +static enum lru_status dentry_lru_isolate_negative(struct list_head *item,
> + struct list_lru_one *lru, void *arg)
> +{
> + struct list_head *freeable = arg;
> + struct dentry *dentry = container_of(item, struct dentry, d_lru);
> +
> + if (!spin_trylock(&dentry->d_lock))
> + return LRU_SKIP;
If anything of the sort is to land, you definitely want to pre-check
d_count and d_is_negative without the lock.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox