* Re: [PATCH v11 3/6] iio: adc: ad4691: add triggered buffer support
From: Jonathan Cameron @ 2026-05-18 15: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: <c9610990-6b40-40a8-948c-fa1209242dbe@baylibre.com>
On Mon, 18 May 2026 09:36:18 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 5/18/26 9:21 AM, Jonathan Cameron wrote:
> > On Sun, 17 May 2026 14:21:30 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >
> >> On 5/17/26 7:25 AM, Jonathan Cameron wrote:
> >>> 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
> >>>
> >> This is what I came up with (totally untested).
> >>
> >> Since timestamp can never be set in scan_mask/active_scan_mask, it should
> >> be safe to exclude it from masklength without breaking existing code.
> > Probably...
> >>
> >> I didn't check all callers of masklength/iio_get_masklength() though.
> >
> > That was the bit that made me nervous. Particularly if there is an off
> > by one that is working by luck today - or someone who understood this
> > oddity and did it deliberately.
> >
> > At one point we also had a few other timestamps - the ones come from hardware.
> > I can't remember how we handled those wrt to the scan mask. I took a quick
> > look and thing they are all fine.
> > FWIW a nice precursor would be to make sure all timestamp channels are assigned
> > using the macro. There are a few that are hand crafted. I tested a few, but obviously
> > needs turning in to a proper set and cleaning up.
> >
> > diff --git a/drivers/iio/adc/ad4170-4.c b/drivers/iio/adc/ad4170-4.c
> > index 627cbf5a37b0..890e25294baa 100644
> > --- a/drivers/iio/adc/ad4170-4.c
> > +++ b/drivers/iio/adc/ad4170-4.c
> > @@ -2385,9 +2385,7 @@ static int ad4170_parse_channels(struct iio_dev *indio_dev)
> > }
> >
> > /* Add timestamp channel */
> > - struct iio_chan_spec ts_chan = IIO_CHAN_SOFT_TIMESTAMP(chan_num);
> > -
> > - st->chans[chan_num] = ts_chan;
> > + st->chans[chan_num] = IIO_CHAN_SOFT_TIMESTAMP(chan_num);
> > num_channels = num_channels + 1;
> >
> > indio_dev->num_channels = num_channels;
> > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > index 6e1930f7c65d..56baca1f5026 100644
> > --- a/drivers/iio/adc/at91_adc.c
> > +++ b/drivers/iio/adc/at91_adc.c
> > @@ -521,13 +521,7 @@ static int at91_adc_channel_init(struct iio_dev *idev)
> > }
> > timestamp = chan_array + idx;
> >
> > - timestamp->type = IIO_TIMESTAMP;
> > - timestamp->channel = -1;
> > - timestamp->scan_index = idx;
> > - timestamp->scan_type.sign = 's';
> > - timestamp->scan_type.realbits = 64;
> > - timestamp->scan_type.storagebits = 64;
> > -
> > + *timestamp = IIO_CHAN_SOFT_TIMESTAMP(idx);
> > idev->channels = chan_array;
> > return idev->num_channels;
> > }
> > diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
> > index 2c51b90b7101..d42b747325aa 100644
> > --- a/drivers/iio/adc/cc10001_adc.c
> > +++ b/drivers/iio/adc/cc10001_adc.c
> > @@ -262,7 +262,7 @@ static const struct iio_info cc10001_adc_info = {
> > static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
> > unsigned long channel_map)
> > {
> > - struct iio_chan_spec *chan_array, *timestamp;
> > + struct iio_chan_spec *chan_array;
> > unsigned int bit, idx = 0;
> >
> > indio_dev->num_channels = bitmap_weight(&channel_map,
> > @@ -289,13 +289,7 @@ static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
> > idx++;
> > }
> >
> > - timestamp = &chan_array[idx];
> > - timestamp->type = IIO_TIMESTAMP;
> > - timestamp->channel = -1;
> > - timestamp->scan_index = idx;
> > - timestamp->scan_type.sign = 's';
> > - timestamp->scan_type.realbits = 64;
> > - timestamp->scan_type.storagebits = 64;
> > + chan_array[idx] = IIO_CHAN_SOFT_TIMESTAMP(idx);
> >
> > indio_dev->channels = chan_array;
> >
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 96b05c86c325..702b2fc66326 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -353,7 +353,7 @@ static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
> > (chan->info_mask_shared_by_all_available & BIT(type));
> > }
> >
> > -#define IIO_CHAN_SOFT_TIMESTAMP(_si) { \
> > +#define IIO_CHAN_SOFT_TIMESTAMP(_si) (struct iio_chan_spec) { \
> > .type = IIO_TIMESTAMP, \
> > .channel = -1, \
> > .scan_index = _si, \
> >
> > Doing that will mean we can spot any unusual use of IIO_TIMESTAMP much more
> > easily.
> >
> > Anyhow, basic approach looks good to me.
>
> I guess you didn't see the other series cleaning up IIO_TIMESTAMP I already
> sent yet.
>
:( That's what I get for not reading all my email before starting to reply!
> >
> > Jonathan
> >
> >
> >
> >>
> >> ---
> >> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> >> index 9d66510a1d49..17f539fc23e2 100644
> >> --- a/drivers/iio/industrialio-buffer.c
> >> +++ b/drivers/iio/industrialio-buffer.c
> >> @@ -2300,8 +2300,10 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
> >> if (channels) {
> >> int ml = 0;
> >>
> >> - for (i = 0; i < indio_dev->num_channels; i++)
> >> - ml = max(ml, channels[i].scan_index + 1);
> >> + for (i = 0; i < indio_dev->num_channels; i++) {
> >> + if (channels[i].type != IIO_TIMESTAMP)
> >> + ml = max(ml, channels[i].scan_index + 1);
> >> + }
> >> ACCESS_PRIVATE(indio_dev, masklength) = ml;
> >> }
> >>
> >>
> >>
> >>
> >
>
^ permalink raw reply
* Re: [PATCH v11 4/6] iio: adc: ad4691: add SPI offload support
From: David Lechner @ 2026-05-18 15:16 UTC (permalink / raw)
To: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Sa, Nuno, 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
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <LV9PR03MB841418AEF0059E802F7A69B2F7032@LV9PR03MB8414.namprd03.prod.outlook.com>
On 5/18/26 10:14 AM, Sabau, Radu bogdan wrote:
>> -----Original Message-----
>> From: David Lechner <dlechner@baylibre.com>
>> Sent: Saturday, May 16, 2026 8:53 PM
>
> ...
>
>>> static ssize_t sampling_frequency_show(struct device *dev,
>>> struct device_attribute *attr,
>>> char *buf)
>>> @@ -880,6 +1229,9 @@ static ssize_t sampling_frequency_show(struct
>> device *dev,
>>> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> struct ad4691_state *st = iio_priv(indio_dev);
>>>
>>> + if (st->manual_mode && st->offload)
>>> + return sysfs_emit(buf, "%llu\n", READ_ONCE(st->offload-
>>> trigger_hz));
>>
>> Why do we need READ_ONCE?
>>
>
> trigger_hz is u64 and if the target is 32-bit, a 64-bit access compiles to two 32-bit
> instructions, so show() reading it without a lock and store() writing it concurrently
> can produce a torn value at the compiler level. READ_ONCE/WRITE_ONCE suppress
> the compiler transformations that would allow that splitting or caching. We could
> have st->lock in show() instead, but that felt heavier than necessary for a single
> scalar where a transiently stale-but-whole read is fine.
>
I would go with the mutex. It will be easier for people to understand.
^ permalink raw reply
* RE: [PATCH v11 4/6] iio: adc: ad4691: add SPI offload support
From: Sabau, Radu bogdan @ 2026-05-18 15:14 UTC (permalink / raw)
To: David Lechner, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Sa, Nuno, 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
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <80f61c0b-1f36-4fee-9f76-b93f63b87abe@baylibre.com>
> -----Original Message-----
> From: David Lechner <dlechner@baylibre.com>
> Sent: Saturday, May 16, 2026 8:53 PM
...
> > static ssize_t sampling_frequency_show(struct device *dev,
> > struct device_attribute *attr,
> > char *buf)
> > @@ -880,6 +1229,9 @@ static ssize_t sampling_frequency_show(struct
> device *dev,
> > struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > struct ad4691_state *st = iio_priv(indio_dev);
> >
> > + if (st->manual_mode && st->offload)
> > + return sysfs_emit(buf, "%llu\n", READ_ONCE(st->offload-
> >trigger_hz));
>
> Why do we need READ_ONCE?
>
trigger_hz is u64 and if the target is 32-bit, a 64-bit access compiles to two 32-bit
instructions, so show() reading it without a lock and store() writing it concurrently
can produce a torn value at the compiler level. READ_ONCE/WRITE_ONCE suppress
the compiler transformations that would allow that splitting or caching. We could
have st->lock in show() instead, but that felt heavier than necessary for a single
scalar where a transiently stale-but-whole read is fine.
^ permalink raw reply
* RE: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: Sabau, Radu bogdan @ 2026-05-18 15:05 UTC (permalink / raw)
To: Jonathan Cameron, Radu Sabau via B4 Relay
Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno,
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@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <20260517125241.7dbfb964@jic23-huawei>
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, May 17, 2026 2:53 PM
...
> > +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://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260515-
> ad4692-multichannel-sar-adc-driver-v11-0-
> eab27d852ac2*40analog.com__;IyU!!A3Ni8CS0y2Y!5I5rXG5US4TFIZ_cAbgcy
> gH-_Cbt6wLDZ5jVeBiPSDg5KuzEZT-CMN4Z3aFYYVXH6Kx4f2ClJcbr9w$
> > + }
> > +
> > + /* 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.
>
He is right. And also I think I misread the datasheet, the 300us should happen
after deassertion and actual RESETL time is 10ns minimum which would be
covered by the overhead itself.
I think the reason to why this was never a problem while testing is because
300us is the recommended time and the minimum one could be much lower
than this, so successful probe was nothing but luck on my end.
I will make sure to update the function accordingly.
^ permalink raw reply
* Re: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: David Lechner @ 2026-05-18 15:05 UTC (permalink / raw)
To: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Sa, Nuno, 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
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <LV9PR03MB841445D5BD1087FB3204EBD9F7032@LV9PR03MB8414.namprd03.prod.outlook.com>
On 5/18/26 9:59 AM, Sabau, Radu bogdan wrote:
>> -----Original Message-----
>> From: David Lechner <dlechner@baylibre.com>
>> Sent: Saturday, May 16, 2026 8:11 PM
>
> ...
>
>>> +static int ad4691_reg_read(void *context, unsigned int reg, unsigned int
>> *val)
>>> +{
>>> + struct spi_device *spi = context;
>>> + u8 tx[2], rx[4];
>>> + int ret;
>>> +
>>> + /* Set bit 15 to mark the operation as READ. */
>>
>> Can't we just set read_flag_mask in the regmap config?
>>
>
> As far as I can tell read_flag_mask is applied by the standard SPI regmap bus
> backend, which constructs and sends the address byte itself before reading
> the response. When using devm_regmap_init() with custom reg_read/reg_write
> callbacks, the regmap core calls those callbacks directly with the raw register
> address - it never touches read_flag_mask.
>
>>> + put_unaligned_be16(0x8000 | reg, tx);
>>> +
>>> + switch (reg) {
>>> + case 0 ... AD4691_OSC_FREQ_REG:
>>> + case AD4691_SPARE_CONTROL ... AD4691_ACC_MASK_REG - 1:
>
> ...
>
>>> +static int ad4691_write_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int val, int val2, long mask)
>>> +{
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_SAMP_FREQ:
>>
>> Should we aquire direct mode so that we can't change the rate during
>> buffered read?
>>
>
> It is in set_sampling_freq already. Do you think it would make more sense
> to move it here in order to help readability?
>
IIRC, I think it was resolved in a later patch in the series. So
could just be a problem of it not getting added in the right patch.
In general though, yes it would make it easier review if the
direct mode claim was made here.
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Shah, Tanmay @ 2026-05-18 15:01 UTC (permalink / raw)
To: Shenwei Wang, Mathieu Poirier
Cc: Arnaud POULIQUEN, Beleswar Prasad Padhi, Andrew Lunn,
Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Frank Li,
Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
dl-linux-imx, Bartosz Golaszewski
In-Reply-To: <PAXPR04MB918587A8812B51BBB2A46A2C89032@PAXPR04MB9185.eurprd04.prod.outlook.com>
On 5/18/2026 9:24 AM, Shenwei Wang wrote:
>
>
>
>>
>> On Thu, May 07, 2026 at 07:43:33PM +0000, Shenwei Wang wrote:
>>>
>>>
>>>> That was my initial approach. We don't even need an additional
>>>> "rpmsg-io-*" in rpmsg_gpio_channel_id_table[]. All we need is:
>>>>
>>>> /* rpmsg devices and drivers are matched using the service name */
>>>> static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
>>>> const struct rpmsg_device_id *id) {
>>>> + size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
>>>>
>>>> - return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
>>>> + return strncmp(id->name, rpdev->id.name, len) == 0;
>>>> }
>>>>
>>>
>>> If we encode the port index directly into ept->src, for example:
>>>
>>> ept->src = (baseaddr << 8) | port_index;
>>>
>>
>> There is no rpmsg_endpoint::src. You likely meant ept->addr. This would work
>> but not optimal on two front:
>>
>> (1) rpms_endpoint::addr is a u32 and idr_alloc() returns an 'int'. As such there is a
>> possibility of conflict. I concede the possibility is marginal, but it still exists.
>>
>
> I think there may be a misunderstanding in the implementation. In this case, we do not
> need the return value from idr_alloc.
>
> When the driver calls rpmsg_create_ept, it can pass an rpmsg_channel_info structure as an
> input parameter. This allows you to specify the source address you want to bind.
> Please refer to the definitions below:
>
> struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *rpdev,
> rpmsg_rx_cb_t cb, void *priv,
> struct rpmsg_channel_info chinfo)
>
> struct rpmsg_channel_info {
> char name[RPMSG_NAME_SIZE];
> u32 src;
> u32 dst;
> };
>
>> (2) By proceeding this way, the kernel exposes the GPIO controller it knows
>> about. It is preferrable to have the remote processor tell the kernel about the
>> GPIO controller it wants.
>>
>
> If everyone agrees with this namespace announcement approach, I will prepare the
> next revision based on it, even though it is not as clean as the source address encoding solution.
>
I have ack on the namespace announcement approach. To me it is very
simple contract between the firmware and the Linux which allows dynamic
endpoint allocation without giving up security concerns. Also this
approach can be easily extended for any other rpmsg devices like i2c,
spi etc.. With the fix in the rpmsg_core.c, this will work. I will have
to see the actual implementation in the next rev to provide further
feedback.
Thanks,
Tanmay
> Shenwei
>
>> I am done reviewing this revision. Given the amount of refactoring needed, I will
>> not look at the code. Please refer to this reply [1] for what I am expecting in the
>> next revision.
>>
>
^ permalink raw reply
* RE: [PATCH v11 2/6] iio: adc: ad4691: add initial driver for AD4691 family
From: Sabau, Radu bogdan @ 2026-05-18 14:59 UTC (permalink / raw)
To: David Lechner, Lars-Peter Clausen, Hennerich, Michael,
Jonathan Cameron, Sa, Nuno, 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
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <0696b662-f478-4d1a-95e0-0338bbdb719e@baylibre.com>
> -----Original Message-----
> From: David Lechner <dlechner@baylibre.com>
> Sent: Saturday, May 16, 2026 8:11 PM
...
> > +static int ad4691_reg_read(void *context, unsigned int reg, unsigned int
> *val)
> > +{
> > + struct spi_device *spi = context;
> > + u8 tx[2], rx[4];
> > + int ret;
> > +
> > + /* Set bit 15 to mark the operation as READ. */
>
> Can't we just set read_flag_mask in the regmap config?
>
As far as I can tell read_flag_mask is applied by the standard SPI regmap bus
backend, which constructs and sends the address byte itself before reading
the response. When using devm_regmap_init() with custom reg_read/reg_write
callbacks, the regmap core calls those callbacks directly with the raw register
address - it never touches read_flag_mask.
> > + put_unaligned_be16(0x8000 | reg, tx);
> > +
> > + switch (reg) {
> > + case 0 ... AD4691_OSC_FREQ_REG:
> > + case AD4691_SPARE_CONTROL ... AD4691_ACC_MASK_REG - 1:
...
> > +static int ad4691_write_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask)
> > +{
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SAMP_FREQ:
>
> Should we aquire direct mode so that we can't change the rate during
> buffered read?
>
It is in set_sampling_freq already. Do you think it would make more sense
to move it here in order to help readability?
^ permalink raw reply
* Re: [PATCH v11 3/6] iio: adc: ad4691: add triggered buffer support
From: David Lechner @ 2026-05-18 14:36 UTC (permalink / raw)
To: Jonathan Cameron
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: <20260518152103.4d428c1e@jic23-huawei>
On 5/18/26 9:21 AM, Jonathan Cameron wrote:
> On Sun, 17 May 2026 14:21:30 -0500
> David Lechner <dlechner@baylibre.com> wrote:
>
>> On 5/17/26 7:25 AM, Jonathan Cameron wrote:
>>> 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
>>>
>> This is what I came up with (totally untested).
>>
>> Since timestamp can never be set in scan_mask/active_scan_mask, it should
>> be safe to exclude it from masklength without breaking existing code.
> Probably...
>>
>> I didn't check all callers of masklength/iio_get_masklength() though.
>
> That was the bit that made me nervous. Particularly if there is an off
> by one that is working by luck today - or someone who understood this
> oddity and did it deliberately.
>
> At one point we also had a few other timestamps - the ones come from hardware.
> I can't remember how we handled those wrt to the scan mask. I took a quick
> look and thing they are all fine.
> FWIW a nice precursor would be to make sure all timestamp channels are assigned
> using the macro. There are a few that are hand crafted. I tested a few, but obviously
> needs turning in to a proper set and cleaning up.
>
> diff --git a/drivers/iio/adc/ad4170-4.c b/drivers/iio/adc/ad4170-4.c
> index 627cbf5a37b0..890e25294baa 100644
> --- a/drivers/iio/adc/ad4170-4.c
> +++ b/drivers/iio/adc/ad4170-4.c
> @@ -2385,9 +2385,7 @@ static int ad4170_parse_channels(struct iio_dev *indio_dev)
> }
>
> /* Add timestamp channel */
> - struct iio_chan_spec ts_chan = IIO_CHAN_SOFT_TIMESTAMP(chan_num);
> -
> - st->chans[chan_num] = ts_chan;
> + st->chans[chan_num] = IIO_CHAN_SOFT_TIMESTAMP(chan_num);
> num_channels = num_channels + 1;
>
> indio_dev->num_channels = num_channels;
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 6e1930f7c65d..56baca1f5026 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -521,13 +521,7 @@ static int at91_adc_channel_init(struct iio_dev *idev)
> }
> timestamp = chan_array + idx;
>
> - timestamp->type = IIO_TIMESTAMP;
> - timestamp->channel = -1;
> - timestamp->scan_index = idx;
> - timestamp->scan_type.sign = 's';
> - timestamp->scan_type.realbits = 64;
> - timestamp->scan_type.storagebits = 64;
> -
> + *timestamp = IIO_CHAN_SOFT_TIMESTAMP(idx);
> idev->channels = chan_array;
> return idev->num_channels;
> }
> diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
> index 2c51b90b7101..d42b747325aa 100644
> --- a/drivers/iio/adc/cc10001_adc.c
> +++ b/drivers/iio/adc/cc10001_adc.c
> @@ -262,7 +262,7 @@ static const struct iio_info cc10001_adc_info = {
> static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
> unsigned long channel_map)
> {
> - struct iio_chan_spec *chan_array, *timestamp;
> + struct iio_chan_spec *chan_array;
> unsigned int bit, idx = 0;
>
> indio_dev->num_channels = bitmap_weight(&channel_map,
> @@ -289,13 +289,7 @@ static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
> idx++;
> }
>
> - timestamp = &chan_array[idx];
> - timestamp->type = IIO_TIMESTAMP;
> - timestamp->channel = -1;
> - timestamp->scan_index = idx;
> - timestamp->scan_type.sign = 's';
> - timestamp->scan_type.realbits = 64;
> - timestamp->scan_type.storagebits = 64;
> + chan_array[idx] = IIO_CHAN_SOFT_TIMESTAMP(idx);
>
> indio_dev->channels = chan_array;
>
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 96b05c86c325..702b2fc66326 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -353,7 +353,7 @@ static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
> (chan->info_mask_shared_by_all_available & BIT(type));
> }
>
> -#define IIO_CHAN_SOFT_TIMESTAMP(_si) { \
> +#define IIO_CHAN_SOFT_TIMESTAMP(_si) (struct iio_chan_spec) { \
> .type = IIO_TIMESTAMP, \
> .channel = -1, \
> .scan_index = _si, \
>
> Doing that will mean we can spot any unusual use of IIO_TIMESTAMP much more
> easily.
>
> Anyhow, basic approach looks good to me.
I guess you didn't see the other series cleaning up IIO_TIMESTAMP I already
sent yet.
>
> Jonathan
>
>
>
>>
>> ---
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index 9d66510a1d49..17f539fc23e2 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -2300,8 +2300,10 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
>> if (channels) {
>> int ml = 0;
>>
>> - for (i = 0; i < indio_dev->num_channels; i++)
>> - ml = max(ml, channels[i].scan_index + 1);
>> + for (i = 0; i < indio_dev->num_channels; i++) {
>> + if (channels[i].type != IIO_TIMESTAMP)
>> + ml = max(ml, channels[i].scan_index + 1);
>> + }
>> ACCESS_PRIVATE(indio_dev, masklength) = ml;
>> }
>>
>>
>>
>>
>
^ permalink raw reply
* Re: [PATCH] pinctrl: meson: amlogic-a4: fix gpio output glitch
From: Neil Armstrong @ 2026-05-18 14:32 UTC (permalink / raw)
To: xianwei.zhao, Linus Walleij, Kevin Hilman, Jerome Brunet,
Martin Blumenstingl
Cc: linux-amlogic, linux-gpio, linux-arm-kernel, linux-kernel
In-Reply-To: <20260518-fix-set-value-glitch-v1-1-d350732dc934@amlogic.com>
On 5/18/26 10:26, Xianwei Zhao via B4 Relay wrote:
> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>
> When the system transitions from bootloader to kernel, the GPIO is
> expected to keep driving high.
>
> However, the Linux kernel first configures the pin direction and then
> sets the output value. This may cause a brief low-level glitch on the
> GPIO line, which can be problematic for regulator control.
>
> By configuring the output value before switching the pin direction to
> output, the glitch can be avoided.
>
> This commit fixes the issue by swapping the configuration order.
>
> Fixes: 6e9be3abb78c ("pinctrl: Add driver support for Amlogic SoCs")
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
> fix one issue when set gpio line high.
> ---
> drivers/pinctrl/meson/pinctrl-amlogic-a4.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> index 35d27626a336..1bd58fbbd26a 100644
> --- a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> +++ b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> @@ -548,11 +548,11 @@ static int aml_pinconf_set_output_drive(struct aml_pinctrl *info,
> {
> int ret;
>
> - ret = aml_pinconf_set_output(info, pin, true);
> + ret = aml_pinconf_set_drive(info, pin, high);
> if (ret)
> return ret;
>
> - return aml_pinconf_set_drive(info, pin, high);
> + return aml_pinconf_set_output(info, pin, true);
> }
>
> static int aml_pinconf_set(struct pinctrl_dev *pcdev, unsigned int pin,
> @@ -921,15 +921,14 @@ static int aml_gpio_direction_output(struct gpio_chip *chip, unsigned int gpio,
> unsigned int bit, reg;
> int ret;
>
> - aml_gpio_calc_reg_and_bit(bank, AML_REG_DIR, gpio, ®, &bit);
> - ret = regmap_update_bits(bank->reg_gpio, reg, BIT(bit), 0);
> + aml_gpio_calc_reg_and_bit(bank, AML_REG_OUT, gpio, ®, &bit);
> + ret = regmap_update_bits(bank->reg_gpio, reg, BIT(bit),
> + value ? BIT(bit) : 0);
> if (ret < 0)
> return ret;
>
> - aml_gpio_calc_reg_and_bit(bank, AML_REG_OUT, gpio, ®, &bit);
> -
> - return regmap_update_bits(bank->reg_gpio, reg, BIT(bit),
> - value ? BIT(bit) : 0);
> + aml_gpio_calc_reg_and_bit(bank, AML_REG_DIR, gpio, ®, &bit);
> + return regmap_update_bits(bank->reg_gpio, reg, BIT(bit), 0);
> }
>
> static int aml_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value)
>
> ---
> base-commit: 73d4991a6949eedb51e442d4e81415017d85975b
> change-id: 20260518-fix-set-value-glitch-f43cd366c295
>
> Best regards,
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Thanks,
Neil
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Andy Shevchenko @ 2026-05-18 14:27 UTC (permalink / raw)
To: Hardik Prakash
Cc: Bartosz Golaszewski, Mario Limonciello, linux-i2c, linux-gpio,
wsa, basavaraj.natikar, linus.walleij
In-Reply-To: <CANTFpSUX5rYhuTQH3dTTvzW+_yhW8Gs0U=A1t_8LDzKz4dzzAw@mail.gmail.com>
On Mon, May 18, 2026 at 07:53:28PM +0530, Hardik Prakash wrote:
> On Mon, May 18, 2026 at 19:35, Bartosz Golaszewski wrote:
> > What is blocking the pinctrl driver from probing? Does it return
> > -EPROBE_DEFER for some reason? Pin control core is ready at
> > core_initcall() so it should work in theory.
>
> On Mon, May 18, 2026 at 19:16, Mario Limonciello wrote:
> > Please try arch_initcall instead.
>
> Tested arch_initcall + patch 1. GPIO 157 now fires at 0.255s (earlier
> than any previous boot), but arbitration errors still occur at 2.309s:
>
> subsys_initcall + patch 1: GPIO 157 at ~0.310s, arbitration errors
> arch_initcall + patch 1: GPIO 157 at ~0.255s, arbitration errors
> patch 1 + patch 2 (v5): no arbitration errors, touchscreen works
>
> The driver is not returning -EPROBE_DEFER. The problem is that
> amd_gpio_probe() hasn't completed by the time i2c_designware fires,
> even with arch_initcall. Promoting the initcall level gets the driver
> registered earlier, but probe itself takes time, and i2c_designware
> catches it mid-probe regardless of registration timing.
>
> This is why device_is_bound() works where initcall promotion does not
> — it waits for probe completion, not just driver registration.
The alternative solution is to have a registered notifier for the device in
question. But not sure if it will be less-invasive than given solution.
> On Mon, 18 May 2026 at 19:41, Bartosz Golaszewski <brgl@kernel.org> wrote:
> > On Mon, May 18, 2026 at 4:08 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> > > On 5/18/26 09:05, Bartosz Golaszewski wrote:
> > > > On Mon, May 18, 2026 at 3:46 PM Mario Limonciello
> > > > <mario.limonciello@amd.com> wrote:
> > > >> On 5/18/26 08:40, Hardik Prakash wrote:
> > > >>> On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
> > > >>>> I'd still like to avoid a quirk if we can.
> > > >>>>
> > > >>>> I know my proposed patch to try to probe at an earlier stage didn't
> > > >>>> work, but could you perhaps try pulling pinctrl-amd even earlier?
> > > >>>>
> > > >>>> Maybe fs_initcall()?
> > > >>>
> > > >>> Tested. fs_initcall + patch 1 still produces the same arbitration
> > > >>> errors:
> > > >>>
> > > >>> subsys_initcall + patch 1: arbitration errors persist
> > > >>> fs_initcall + patch 1: arbitration errors persist
> > > >>> patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
> > > >>>
> > > >>> The initcall level does not appear to be the determining factor on
> > > >>> this hardware. i2c_designware is still probing AMDI0010:02 before
> > > >>> pinctrl-amd finishes regardless of how early pinctrl-amd registers.
> > > >>> The explicit device_is_bound() deferral in patch 2 is the only
> > > >>> approach that has worked.
> > > >>
> > > >> Please try arch_initcall instead.
> > > >
> > > > What is blocking the pinctrl driver from probing? Does it return
> > > > -EPROBE_DEFER for some reason? Pin control core is ready at
> > > > core_initcall() so it should work in theory.
> > >
> > > Currently it's module_platform_driver() IE device_initcall().
> > >
> > > That's why I think we "should" be able to move it a lot earlier.
> >
> > I mean with fs_initcall() change - what's blocking it now?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Bartosz Golaszewski @ 2026-05-18 14:26 UTC (permalink / raw)
To: Hardik Prakash
Cc: Mario Limonciello, linux-i2c, linux-gpio, wsa, andriy.shevchenko,
basavaraj.natikar, linus.walleij
In-Reply-To: <CANTFpSUX5rYhuTQH3dTTvzW+_yhW8Gs0U=A1t_8LDzKz4dzzAw@mail.gmail.com>
On Mon, May 18, 2026 at 4:23 PM Hardik Prakash
<hardikprakash.official@gmail.com> wrote:
>
> On Mon, May 18, 2026 at 19:35, Bartosz Golaszewski wrote:
> > What is blocking the pinctrl driver from probing? Does it return
> > -EPROBE_DEFER for some reason? Pin control core is ready at
> > core_initcall() so it should work in theory.
>
> On Mon, May 18, 2026 at 19:16, Mario Limonciello wrote:
> > Please try arch_initcall instead.
>
> Tested arch_initcall + patch 1. GPIO 157 now fires at 0.255s (earlier
> than any previous boot), but arbitration errors still occur at 2.309s:
>
> subsys_initcall + patch 1: GPIO 157 at ~0.310s, arbitration errors
> arch_initcall + patch 1: GPIO 157 at ~0.255s, arbitration errors
> patch 1 + patch 2 (v5): no arbitration errors, touchscreen works
>
> The driver is not returning -EPROBE_DEFER. The problem is that
> amd_gpio_probe() hasn't completed by the time i2c_designware fires,
> even with arch_initcall. Promoting the initcall level gets the driver
> registered earlier, but probe itself takes time, and i2c_designware
> catches it mid-probe regardless of registration timing.
>
> This is why device_is_bound() works where initcall promotion does not
> — it waits for probe completion, not just driver registration.
>
If you added wait_for_device_probe() right before requesting the
interrupt, does it help?
Bartosz
^ permalink raw reply
* Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
From: Shenwei Wang @ 2026-05-18 14:24 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Arnaud POULIQUEN, Beleswar Prasad Padhi, Andrew Lunn,
Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Frank Li,
Sascha Hauer, Shuah Khan, linux-gpio@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
devicetree@vger.kernel.org, linux-remoteproc@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
dl-linux-imx, Bartosz Golaszewski
In-Reply-To: <agYHzH-nJLl1HFIn@p14s>
>
> On Thu, May 07, 2026 at 07:43:33PM +0000, Shenwei Wang wrote:
> >
> >
> > > That was my initial approach. We don't even need an additional
> > > "rpmsg-io-*" in rpmsg_gpio_channel_id_table[]. All we need is:
> > >
> > > /* rpmsg devices and drivers are matched using the service name */
> > > static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
> > > const struct rpmsg_device_id *id) {
> > > + size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
> > >
> > > - return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> > > + return strncmp(id->name, rpdev->id.name, len) == 0;
> > > }
> > >
> >
> > If we encode the port index directly into ept->src, for example:
> >
> > ept->src = (baseaddr << 8) | port_index;
> >
>
> There is no rpmsg_endpoint::src. You likely meant ept->addr. This would work
> but not optimal on two front:
>
> (1) rpms_endpoint::addr is a u32 and idr_alloc() returns an 'int'. As such there is a
> possibility of conflict. I concede the possibility is marginal, but it still exists.
>
I think there may be a misunderstanding in the implementation. In this case, we do not
need the return value from idr_alloc.
When the driver calls rpmsg_create_ept, it can pass an rpmsg_channel_info structure as an
input parameter. This allows you to specify the source address you want to bind.
Please refer to the definitions below:
struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_device *rpdev,
rpmsg_rx_cb_t cb, void *priv,
struct rpmsg_channel_info chinfo)
struct rpmsg_channel_info {
char name[RPMSG_NAME_SIZE];
u32 src;
u32 dst;
};
> (2) By proceeding this way, the kernel exposes the GPIO controller it knows
> about. It is preferrable to have the remote processor tell the kernel about the
> GPIO controller it wants.
>
If everyone agrees with this namespace announcement approach, I will prepare the
next revision based on it, even though it is not as clean as the source address encoding solution.
Shenwei
> I am done reviewing this revision. Given the amount of refactoring needed, I will
> not look at the code. Please refer to this reply [1] for what I am expecting in the
> next revision.
>
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Hardik Prakash @ 2026-05-18 14:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Mario Limonciello, linux-i2c, linux-gpio, wsa, andriy.shevchenko,
basavaraj.natikar, linus.walleij
In-Reply-To: <CAMRc=MfT_WVMxPnYZW=mg52PHew0O4VQMGFrfo6G9vxZDDoArw@mail.gmail.com>
On Mon, May 18, 2026 at 19:35, Bartosz Golaszewski wrote:
> What is blocking the pinctrl driver from probing? Does it return
> -EPROBE_DEFER for some reason? Pin control core is ready at
> core_initcall() so it should work in theory.
On Mon, May 18, 2026 at 19:16, Mario Limonciello wrote:
> Please try arch_initcall instead.
Tested arch_initcall + patch 1. GPIO 157 now fires at 0.255s (earlier
than any previous boot), but arbitration errors still occur at 2.309s:
subsys_initcall + patch 1: GPIO 157 at ~0.310s, arbitration errors
arch_initcall + patch 1: GPIO 157 at ~0.255s, arbitration errors
patch 1 + patch 2 (v5): no arbitration errors, touchscreen works
The driver is not returning -EPROBE_DEFER. The problem is that
amd_gpio_probe() hasn't completed by the time i2c_designware fires,
even with arch_initcall. Promoting the initcall level gets the driver
registered earlier, but probe itself takes time, and i2c_designware
catches it mid-probe regardless of registration timing.
This is why device_is_bound() works where initcall promotion does not
— it waits for probe completion, not just driver registration.
Thanks,
Hardik
On Mon, 18 May 2026 at 19:41, Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Mon, May 18, 2026 at 4:08 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> >
> >
> > On 5/18/26 09:05, Bartosz Golaszewski wrote:
> > > On Mon, May 18, 2026 at 3:46 PM Mario Limonciello
> > > <mario.limonciello@amd.com> wrote:
> > >>
> > >>
> > >>
> > >> On 5/18/26 08:40, Hardik Prakash wrote:
> > >>> On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
> > >>>> I'd still like to avoid a quirk if we can.
> > >>>>
> > >>>> I know my proposed patch to try to probe at an earlier stage didn't
> > >>>> work, but could you perhaps try pulling pinctrl-amd even earlier?
> > >>>>
> > >>>> Maybe fs_initcall()?
> > >>>
> > >>> Tested. fs_initcall + patch 1 still produces the same arbitration
> > >>> errors:
> > >>>
> > >>> subsys_initcall + patch 1: arbitration errors persist
> > >>> fs_initcall + patch 1: arbitration errors persist
> > >>> patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
> > >>>
> > >>> The initcall level does not appear to be the determining factor on
> > >>> this hardware. i2c_designware is still probing AMDI0010:02 before
> > >>> pinctrl-amd finishes regardless of how early pinctrl-amd registers.
> > >>> The explicit device_is_bound() deferral in patch 2 is the only
> > >>> approach that has worked.
> > >>
> > >> Please try arch_initcall instead.
> > >>
> > >
> > > What is blocking the pinctrl driver from probing? Does it return
> > > -EPROBE_DEFER for some reason? Pin control core is ready at
> > > core_initcall() so it should work in theory.
> > >
> > > Bart
> >
> > Currently it's module_platform_driver() IE device_initcall().
> >
> > That's why I think we "should" be able to move it a lot earlier.
>
> I mean with fs_initcall() change - what's blocking it now?
>
> Bart
^ permalink raw reply
* Re: [PATCH v4 1/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Hardik Prakash @ 2026-05-18 14:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Bartosz Golaszewski, linux-i2c, linux-gpio, wsa,
mario.limonciello, basavaraj.natikar, linus.walleij
In-Reply-To: <agsc0NoznnVtclk0@ashevche-desk.local>
On Mon, May 18, 2026 at 19:36, Andy Shevchenko wrote:
> Don't we have already force and ignore lists for _DEP somewhere in the
> drivers/acpi/?
Yes — I found acpi_honor_dep_ids and acpi_ignore_dep_ids in
drivers/acpi/scan.c, and an arch_acpi_add_auto_dep() weak hook that
RISC-V already uses to synthesize dependencies not present in the DSDT.
However, the DSDT has no _DEP object on AMDI0010:02 at all, so adding
AMDI0030 to acpi_honor_dep_ids would not help — there is nothing to
honor. The arch_acpi_add_auto_dep() hook could synthesize the
dependency for x86, similar to how RISC-V uses it for IRQ routing, but
it would still require DMI-based hardware identification to avoid
affecting other machines. That is a different location for the quirk,
not the elimination of one.
Unless I am missing something, the driver-level deferral in patch 2
remains the least-invasive approach given the missing _DEP.
Thanks,
Hardik
On Mon, 18 May 2026 at 19:36, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, May 18, 2026 at 05:55:02PM +0530, Hardik Prakash wrote:
> > On Mon, May 18, 2026 at 16:24, Bartosz Golaszewski wrote:
> > > The patch looks fine but I too would prefer this to be handled at a
> > > higher-level. If we know which ACPI devices we're waiting for and what
> > > the ordering should be - is it possible to somehow setup the devlink
> > > for the platform devices that will be the children of these ACPI
> > > devices?
> > >
> > > If that can't be done, I'm fine with this as a workaround.
> >
> > I checked the DSDT and the kernel's ACPI dependency mechanisms. The
> > DSDT has no _DEP object linking AMDI0010:02 to AMDI0030:00, so there
> > is nothing for fw_devlink to act on. The GpioInt resource is on the
> > touchscreen device (TPNL/WACF2200), not on the I2C controller itself.
> >
> > Setting this up at the ACPI layer would require either a firmware
> > change to add _DEP, or a DMI quirk in the ACPI scan path to synthesize
> > the dependency — which would be equally quirk-based as the current
> > approach, just in a different driver.
>
> Don't we have already force and ignore lists for _DEP somewhere in the
> drivers/acpi/?
>
> > I'll send v5 with your two style fixes applied.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v11 3/6] iio: adc: ad4691: add triggered buffer support
From: Jonathan Cameron @ 2026-05-18 14:21 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: <58a66855-9fb3-48ca-8cae-ff9277f745df@baylibre.com>
On Sun, 17 May 2026 14:21:30 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 5/17/26 7:25 AM, Jonathan Cameron wrote:
> > 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
> >
> This is what I came up with (totally untested).
>
> Since timestamp can never be set in scan_mask/active_scan_mask, it should
> be safe to exclude it from masklength without breaking existing code.
Probably...
>
> I didn't check all callers of masklength/iio_get_masklength() though.
That was the bit that made me nervous. Particularly if there is an off
by one that is working by luck today - or someone who understood this
oddity and did it deliberately.
At one point we also had a few other timestamps - the ones come from hardware.
I can't remember how we handled those wrt to the scan mask. I took a quick
look and thing they are all fine.
FWIW a nice precursor would be to make sure all timestamp channels are assigned
using the macro. There are a few that are hand crafted. I tested a few, but obviously
needs turning in to a proper set and cleaning up.
diff --git a/drivers/iio/adc/ad4170-4.c b/drivers/iio/adc/ad4170-4.c
index 627cbf5a37b0..890e25294baa 100644
--- a/drivers/iio/adc/ad4170-4.c
+++ b/drivers/iio/adc/ad4170-4.c
@@ -2385,9 +2385,7 @@ static int ad4170_parse_channels(struct iio_dev *indio_dev)
}
/* Add timestamp channel */
- struct iio_chan_spec ts_chan = IIO_CHAN_SOFT_TIMESTAMP(chan_num);
-
- st->chans[chan_num] = ts_chan;
+ st->chans[chan_num] = IIO_CHAN_SOFT_TIMESTAMP(chan_num);
num_channels = num_channels + 1;
indio_dev->num_channels = num_channels;
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 6e1930f7c65d..56baca1f5026 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -521,13 +521,7 @@ static int at91_adc_channel_init(struct iio_dev *idev)
}
timestamp = chan_array + idx;
- timestamp->type = IIO_TIMESTAMP;
- timestamp->channel = -1;
- timestamp->scan_index = idx;
- timestamp->scan_type.sign = 's';
- timestamp->scan_type.realbits = 64;
- timestamp->scan_type.storagebits = 64;
-
+ *timestamp = IIO_CHAN_SOFT_TIMESTAMP(idx);
idev->channels = chan_array;
return idev->num_channels;
}
diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c
index 2c51b90b7101..d42b747325aa 100644
--- a/drivers/iio/adc/cc10001_adc.c
+++ b/drivers/iio/adc/cc10001_adc.c
@@ -262,7 +262,7 @@ static const struct iio_info cc10001_adc_info = {
static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
unsigned long channel_map)
{
- struct iio_chan_spec *chan_array, *timestamp;
+ struct iio_chan_spec *chan_array;
unsigned int bit, idx = 0;
indio_dev->num_channels = bitmap_weight(&channel_map,
@@ -289,13 +289,7 @@ static int cc10001_adc_channel_init(struct iio_dev *indio_dev,
idx++;
}
- timestamp = &chan_array[idx];
- timestamp->type = IIO_TIMESTAMP;
- timestamp->channel = -1;
- timestamp->scan_index = idx;
- timestamp->scan_type.sign = 's';
- timestamp->scan_type.realbits = 64;
- timestamp->scan_type.storagebits = 64;
+ chan_array[idx] = IIO_CHAN_SOFT_TIMESTAMP(idx);
indio_dev->channels = chan_array;
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 96b05c86c325..702b2fc66326 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -353,7 +353,7 @@ static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
(chan->info_mask_shared_by_all_available & BIT(type));
}
-#define IIO_CHAN_SOFT_TIMESTAMP(_si) { \
+#define IIO_CHAN_SOFT_TIMESTAMP(_si) (struct iio_chan_spec) { \
.type = IIO_TIMESTAMP, \
.channel = -1, \
.scan_index = _si, \
Doing that will mean we can spot any unusual use of IIO_TIMESTAMP much more
easily.
Anyhow, basic approach looks good to me.
Jonathan
>
> ---
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 9d66510a1d49..17f539fc23e2 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -2300,8 +2300,10 @@ int iio_buffers_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
> if (channels) {
> int ml = 0;
>
> - for (i = 0; i < indio_dev->num_channels; i++)
> - ml = max(ml, channels[i].scan_index + 1);
> + for (i = 0; i < indio_dev->num_channels; i++) {
> + if (channels[i].type != IIO_TIMESTAMP)
> + ml = max(ml, channels[i].scan_index + 1);
> + }
> ACCESS_PRIVATE(indio_dev, masklength) = ml;
> }
>
>
>
>
^ permalink raw reply related
* [PATCH 2/2] gpio: add kunit test cases for the GPIO subsystem
From: Bartosz Golaszewski @ 2026-05-18 14:16 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar, Linus Walleij,
Bartosz Golaszewski
Cc: linux-kselftest, kunit-dev, linux-kernel, linux-gpio,
Bartosz Golaszewski
In-Reply-To: <20260518-gpiolib-kunit-v1-0-131ec646c4df@oss.qualcomm.com>
Add a module containing kunit test cases for GPIO core. The idea is to
use it to test functionalities that can't easily be tested from
user-space with kernel selftests or GPIO character device test suites
provided by the libgpiod package.
For now add test cases that verify software node based lookup and ensure
that a GPIO provider unbinding with active consumers does not cause a
crash.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpiolib-kunit.c | 354 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 363 insertions(+)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 020e51e30317a8574638bbe31365a3cf49591641..2ed9ba8d9c12b0fec9f6fa3fe6077f5588bf719c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -102,6 +102,14 @@ config GPIO_CDEV_V1
This ABI version is deprecated.
Please use the latest ABI for new developments.
+config GPIO_KUNIT
+ tristate "Build GPIO Kunit test cases"
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Say Y here to build the module containing Kunit test cases verifying
+ the functionality of the GPIO subsystem.
+
config GPIO_GENERIC
depends on HAS_IOMEM # Only for IOMEM drivers
tristate
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index b267598b517de089cf6339d837264f1d09e275c0..b01163c6a94db1aa21d895fb897e87be62b816e4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_GPIO_ACPI) += gpiolib-acpi.o
gpiolib-acpi-y := gpiolib-acpi-core.o gpiolib-acpi-quirks.o
obj-$(CONFIG_GPIOLIB) += gpiolib-swnode.o
obj-$(CONFIG_GPIO_SHARED) += gpiolib-shared.o
+obj-$(CONFIG_GPIO_KUNIT) += gpiolib-kunit.o
# Device drivers. Generally keep list sorted alphabetically
obj-$(CONFIG_GPIO_REGMAP) += gpio-regmap.o
diff --git a/drivers/gpio/gpiolib-kunit.c b/drivers/gpio/gpiolib-kunit.c
new file mode 100644
index 0000000000000000000000000000000000000000..f45ba72362f82387d7e345fe57c6ae976265ff9c
--- /dev/null
+++ b/drivers/gpio/gpiolib-kunit.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) Qualcomm Technologies, Inc. and/or its subsidiaries
+ */
+
+#include <linux/fwnode.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
+#include <linux/gpio/property.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#include <kunit/platform_device.h>
+#include <kunit/test.h>
+
+#define GPIO_TEST_PROVIDER "gpio-test-provider"
+#define GPIO_SWNODE_TEST_CONSUMER "gpio-swnode-test-consumer"
+#define GPIO_UNBIND_TEST_CONSUMER "gpio-unbind-test-consumer"
+
+static int gpio_test_provider_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int gpio_test_provider_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+ return 0;
+}
+
+static int gpio_test_provider_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct gpio_chip *gc;
+
+ gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
+ if (!gc)
+ return -ENOMEM;
+
+ gc->base = -1;
+ gc->ngpio = 4;
+ gc->label = "gpio-swnode-consumer-test-device";
+ gc->parent = dev;
+ gc->owner = THIS_MODULE;
+
+ gc->get_direction = gpio_test_provider_get_direction;
+ gc->set = gpio_test_provider_set;
+
+ return devm_gpiochip_add_data(dev, gc, NULL);
+}
+
+static struct platform_driver gpio_test_provider_driver = {
+ .probe = gpio_test_provider_probe,
+ .driver = {
+ .name = GPIO_TEST_PROVIDER,
+ },
+};
+
+static const struct software_node gpio_test_provider_swnode = {
+ .name = "gpio-test-provider-primary",
+};
+
+struct gpio_swnode_consumer_pdata {
+ bool gpio_ok;
+};
+
+static const struct gpio_swnode_consumer_pdata gpio_swnode_pdata_template = {
+ .gpio_ok = false,
+};
+
+static int gpio_swnode_consumer_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct gpio_swnode_consumer_pdata *pdata = dev_get_platdata(dev);
+ struct gpio_desc *desc;
+
+ desc = devm_gpiod_get(dev, "foo", GPIOD_OUT_HIGH);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ pdata->gpio_ok = true;
+
+ return 0;
+}
+
+static struct platform_driver gpio_swnode_consumer_driver = {
+ .probe = gpio_swnode_consumer_probe,
+ .driver = {
+ .name = GPIO_SWNODE_TEST_CONSUMER,
+ },
+};
+
+static void gpio_swnode_lookup_by_primary(struct kunit *test)
+{
+ struct gpio_swnode_consumer_pdata *pdata;
+ struct platform_device_info pdevinfo;
+ struct property_entry properties[2];
+ struct platform_device *pdev;
+ int ret;
+
+ ret = kunit_platform_driver_register(test, &gpio_test_provider_driver);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = kunit_platform_driver_register(test, &gpio_swnode_consumer_driver);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ pdevinfo = (struct platform_device_info){
+ .name = GPIO_TEST_PROVIDER,
+ .id = PLATFORM_DEVID_NONE,
+ .swnode = &gpio_test_provider_swnode,
+ };
+
+ pdev = kunit_platform_device_register_full(test, &pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+ properties[0] = PROPERTY_ENTRY_GPIO("foo-gpios",
+ &gpio_test_provider_swnode,
+ 0, GPIO_ACTIVE_HIGH);
+ properties[1] = (struct property_entry){ };
+
+ pdevinfo = (struct platform_device_info){
+ .name = GPIO_SWNODE_TEST_CONSUMER,
+ .id = PLATFORM_DEVID_NONE,
+ .data = &gpio_swnode_pdata_template,
+ .size_data = sizeof(gpio_swnode_pdata_template),
+ .properties = properties,
+ };
+
+ pdev = kunit_platform_device_register_full(test, &pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+ wait_for_device_probe();
+ scoped_guard(device, &pdev->dev)
+ KUNIT_ASSERT_TRUE(test, device_is_bound(&pdev->dev));
+
+ pdata = dev_get_platdata(&pdev->dev);
+ KUNIT_ASSERT_TRUE(test, pdata->gpio_ok);
+}
+
+static void gpio_swnode_lookup_by_secondary(struct kunit *test)
+{
+ struct gpio_swnode_consumer_pdata *pdata;
+ struct platform_device_info pdevinfo;
+ struct property_entry properties[2];
+ struct fwnode_handle *primary;
+ struct platform_device *pdev;
+ int ret;
+
+ /*
+ * Can't live on the stack as it will still get referenced in cleanup
+ * path after this function returns.
+ */
+ primary = kunit_kzalloc(test, sizeof(*primary), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, primary);
+
+ ret = kunit_platform_driver_register(test, &gpio_test_provider_driver);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = kunit_platform_driver_register(test, &gpio_swnode_consumer_driver);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ fwnode_init(primary, NULL);
+
+ pdevinfo = (struct platform_device_info){
+ .name = GPIO_TEST_PROVIDER,
+ .id = PLATFORM_DEVID_NONE,
+ .fwnode = primary,
+ .swnode = &gpio_test_provider_swnode,
+ };
+
+ pdev = kunit_platform_device_register_full(test, &pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+ properties[0] = PROPERTY_ENTRY_GPIO("foo-gpios",
+ &gpio_test_provider_swnode,
+ 0, GPIO_ACTIVE_HIGH);
+ properties[1] = (struct property_entry){ };
+
+ pdevinfo = (struct platform_device_info){
+ .name = GPIO_SWNODE_TEST_CONSUMER,
+ .id = PLATFORM_DEVID_NONE,
+ .data = &gpio_swnode_pdata_template,
+ .size_data = sizeof(gpio_swnode_pdata_template),
+ .properties = properties,
+ };
+
+ pdev = kunit_platform_device_register_full(test, &pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+ wait_for_device_probe();
+ scoped_guard(device, &pdev->dev)
+ KUNIT_ASSERT_TRUE(test, device_is_bound(&pdev->dev));
+
+ pdata = dev_get_platdata(&pdev->dev);
+ KUNIT_ASSERT_TRUE(test, pdata->gpio_ok);
+}
+
+static struct kunit_case gpio_swnode_lookup_tests[] = {
+ KUNIT_CASE(gpio_swnode_lookup_by_primary),
+ KUNIT_CASE(gpio_swnode_lookup_by_secondary),
+ { }
+};
+
+static struct kunit_suite gpio_swnode_lookup_test_suite = {
+ .name = "gpio-swnode-lookup",
+ .test_cases = gpio_swnode_lookup_tests,
+};
+
+static BLOCKING_NOTIFIER_HEAD(gpio_unbind_notifier);
+
+struct gpio_unbind_consumer_drvdata {
+ struct device *dev;
+ struct gpio_desc *desc;
+ struct notifier_block nb;
+ int set_retval;
+};
+
+static int gpio_unbind_notify(struct notifier_block *nb, unsigned long action,
+ void *data)
+{
+ struct gpio_unbind_consumer_drvdata *drvdata =
+ container_of(nb, struct gpio_unbind_consumer_drvdata, nb);
+ struct device *dev = data;
+
+ if (dev != drvdata->dev)
+ return NOTIFY_DONE;
+
+ drvdata->set_retval = gpiod_set_value_cansleep(drvdata->desc, 0);
+
+ return NOTIFY_OK;
+}
+
+static void gpio_unbind_unregister_notifier(void *data)
+{
+ struct notifier_block *nb = data;
+
+ blocking_notifier_chain_unregister(&gpio_unbind_notifier, nb);
+}
+
+static int gpio_unbind_consumer_probe(struct platform_device *pdev)
+{
+ struct gpio_unbind_consumer_drvdata *data;
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->dev = dev;
+
+ data->desc = devm_gpiod_get(dev, "foo", GPIOD_OUT_HIGH);
+ if (IS_ERR(data->desc))
+ return PTR_ERR(data->desc);
+
+ data->nb.notifier_call = gpio_unbind_notify;
+ ret = blocking_notifier_chain_register(&gpio_unbind_notifier, &data->nb);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, gpio_unbind_unregister_notifier, &data->nb);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, data);
+
+ return 0;
+}
+
+static struct platform_driver gpio_unbind_consumer_driver = {
+ .probe = gpio_unbind_consumer_probe,
+ .driver = {
+ .name = GPIO_UNBIND_TEST_CONSUMER,
+ },
+};
+
+static void gpio_unbind_with_consumers(struct kunit *test)
+{
+ struct gpio_unbind_consumer_drvdata *cons_data;
+ struct platform_device_info pdevinfo;
+ struct property_entry properties[2];
+ struct platform_device *prvd, *cons;
+ int ret;
+
+ ret = kunit_platform_driver_register(test, &gpio_test_provider_driver);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = kunit_platform_driver_register(test, &gpio_unbind_consumer_driver);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ pdevinfo = (struct platform_device_info){
+ .name = GPIO_TEST_PROVIDER,
+ .id = PLATFORM_DEVID_NONE,
+ .swnode = &gpio_test_provider_swnode,
+ };
+
+ prvd = platform_device_register_full(&pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, prvd);
+
+ properties[0] = PROPERTY_ENTRY_GPIO("foo-gpios",
+ &gpio_test_provider_swnode,
+ 0, GPIO_ACTIVE_HIGH);
+ properties[1] = (struct property_entry){ };
+
+ pdevinfo = (struct platform_device_info){
+ .name = GPIO_UNBIND_TEST_CONSUMER,
+ .id = PLATFORM_DEVID_NONE,
+ .properties = properties,
+ };
+
+ cons = kunit_platform_device_register_full(test, &pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cons);
+
+ wait_for_device_probe();
+ scoped_guard(device, &cons->dev)
+ KUNIT_ASSERT_TRUE(test, device_is_bound(&cons->dev));
+
+ platform_device_unregister(prvd);
+
+ ret = blocking_notifier_call_chain(&gpio_unbind_notifier, 0, &cons->dev);
+ KUNIT_ASSERT_EQ(test, ret, NOTIFY_OK);
+
+ scoped_guard(device, &cons->dev) {
+ cons_data = platform_get_drvdata(cons);
+ /*
+ * We can't have KUNIT_ASSERT_EQ() under the guard. Despite
+ * the lock being released automatically, it will complain that
+ * a lock is still taken during a test abort.
+ */
+ ret = cons_data->set_retval;
+ }
+
+ KUNIT_ASSERT_EQ(test, cons_data->set_retval, -ENODEV);
+}
+
+static struct kunit_case gpio_unbind_with_consumers_tests[] = {
+ KUNIT_CASE(gpio_unbind_with_consumers),
+ { }
+};
+
+static struct kunit_suite gpio_unbind_with_consumers_test_suite = {
+ .name = "gpio-unbind-with-consumers",
+ .test_cases = gpio_unbind_with_consumers_tests,
+};
+
+kunit_test_suites(
+ &gpio_swnode_lookup_test_suite,
+ &gpio_unbind_with_consumers_test_suite,
+);
+
+MODULE_DESCRIPTION("Test module for the GPIO subsystem");
+MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related
* [PATCH 1/2] kunit: provide kunit_platform_device_register_full()
From: Bartosz Golaszewski @ 2026-05-18 14:16 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar, Linus Walleij,
Bartosz Golaszewski
Cc: linux-kselftest, kunit-dev, linux-kernel, linux-gpio,
Bartosz Golaszewski
In-Reply-To: <20260518-gpiolib-kunit-v1-0-131ec646c4df@oss.qualcomm.com>
Provide a kunit-managed variant of platform_device_register_full().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
include/kunit/platform_device.h | 4 ++++
lib/kunit/platform.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/include/kunit/platform_device.h b/include/kunit/platform_device.h
index f8236a8536f7ebcee6b0e00a7bd799a14b345c1b..8cad6e1c3e7efba862862b579089f2f317784a73 100644
--- a/include/kunit/platform_device.h
+++ b/include/kunit/platform_device.h
@@ -6,10 +6,14 @@ struct completion;
struct kunit;
struct platform_device;
struct platform_driver;
+struct platform_device_info;
struct platform_device *
kunit_platform_device_alloc(struct kunit *test, const char *name, int id);
int kunit_platform_device_add(struct kunit *test, struct platform_device *pdev);
+struct platform_device *
+kunit_platform_device_register_full(struct kunit *test,
+ const struct platform_device_info *pdevinfo);
int kunit_platform_device_prepare_wait_for_probe(struct kunit *test,
struct platform_device *pdev,
diff --git a/lib/kunit/platform.c b/lib/kunit/platform.c
index 0b518de26065d65dac3bd49dd94a4b3e7ea0634b..583b50b538c79599ebbf33e261fe2e9ced35efa9 100644
--- a/lib/kunit/platform.c
+++ b/lib/kunit/platform.c
@@ -6,6 +6,7 @@
#include <linux/completion.h>
#include <linux/device/bus.h>
#include <linux/device/driver.h>
+#include <linux/err.h>
#include <linux/platform_device.h>
#include <kunit/platform_device.h>
@@ -130,6 +131,36 @@ int kunit_platform_device_add(struct kunit *test, struct platform_device *pdev)
}
EXPORT_SYMBOL_GPL(kunit_platform_device_add);
+/**
+ * kunit_platform_device_register_full() - Register a KUnit test-managed platform
+ * device described by platform device info
+ * @test: test context
+ * @pdevinfo: platform device information describing the new device
+ *
+ * Register a test-managed platform device. The device is unregistered when the
+ * test completes.
+ *
+ * Return: New platform device on success, IS_ERR() on error.
+ */
+struct platform_device *
+kunit_platform_device_register_full(struct kunit *test,
+ const struct platform_device_info *pdevinfo)
+{
+ struct platform_device *pdev;
+ int ret;
+
+ pdev = platform_device_register_full(pdevinfo);
+ if (IS_ERR(pdev))
+ return pdev;
+
+ ret = kunit_add_action_or_reset(test, platform_device_unregister_wrapper, pdev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return pdev;
+}
+EXPORT_SYMBOL_GPL(kunit_platform_device_register_full);
+
struct kunit_platform_device_probe_nb {
struct completion *x;
struct device *dev;
--
2.47.3
^ permalink raw reply related
* [PATCH 0/2] gpio: add kunit tests for GPIO core
From: Bartosz Golaszewski @ 2026-05-18 14:16 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar, Linus Walleij,
Bartosz Golaszewski
Cc: linux-kselftest, kunit-dev, linux-kernel, linux-gpio,
Bartosz Golaszewski
This series adds a first batch of kunit tests for GPIO core. I intend to
gradually add more coverage for functionalities that can't really be
tested from user-space with the existing kernel selftests or libgpiod
tests.
Merging strategy: with an Ack from kunit maintainers, this can go
through the GPIO tree for v7.2.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Bartosz Golaszewski (2):
kunit: provide kunit_platform_device_register_full()
gpio: add kunit test cases for the GPIO subsystem
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpiolib-kunit.c | 354 ++++++++++++++++++++++++++++++++++++++++
include/kunit/platform_device.h | 4 +
lib/kunit/platform.c | 31 ++++
5 files changed, 398 insertions(+)
---
base-commit: 4a1989fb6514ca2a4b157ff4700bdcc8bdd9a978
change-id: 20260326-gpiolib-kunit-d7f1b5541ffa
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Bartosz Golaszewski @ 2026-05-18 14:10 UTC (permalink / raw)
To: Mario Limonciello
Cc: Hardik Prakash, linux-i2c, linux-gpio, wsa, andriy.shevchenko,
basavaraj.natikar, linus.walleij
In-Reply-To: <9d5da93e-bbe0-4359-9f17-e3c6b3a5cb34@amd.com>
On Mon, May 18, 2026 at 4:08 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
>
>
> On 5/18/26 09:05, Bartosz Golaszewski wrote:
> > On Mon, May 18, 2026 at 3:46 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >>
> >>
> >>
> >> On 5/18/26 08:40, Hardik Prakash wrote:
> >>> On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
> >>>> I'd still like to avoid a quirk if we can.
> >>>>
> >>>> I know my proposed patch to try to probe at an earlier stage didn't
> >>>> work, but could you perhaps try pulling pinctrl-amd even earlier?
> >>>>
> >>>> Maybe fs_initcall()?
> >>>
> >>> Tested. fs_initcall + patch 1 still produces the same arbitration
> >>> errors:
> >>>
> >>> subsys_initcall + patch 1: arbitration errors persist
> >>> fs_initcall + patch 1: arbitration errors persist
> >>> patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
> >>>
> >>> The initcall level does not appear to be the determining factor on
> >>> this hardware. i2c_designware is still probing AMDI0010:02 before
> >>> pinctrl-amd finishes regardless of how early pinctrl-amd registers.
> >>> The explicit device_is_bound() deferral in patch 2 is the only
> >>> approach that has worked.
> >>
> >> Please try arch_initcall instead.
> >>
> >
> > What is blocking the pinctrl driver from probing? Does it return
> > -EPROBE_DEFER for some reason? Pin control core is ready at
> > core_initcall() so it should work in theory.
> >
> > Bart
>
> Currently it's module_platform_driver() IE device_initcall().
>
> That's why I think we "should" be able to move it a lot earlier.
I mean with fs_initcall() change - what's blocking it now?
Bart
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Andy Shevchenko @ 2026-05-18 14:08 UTC (permalink / raw)
To: Hardik Prakash
Cc: linux-i2c, linux-gpio, wsa, mario.limonciello, brgl,
basavaraj.natikar, linus.walleij
In-Reply-To: <20260518122814.8975-1-hardikprakash.official@gmail.com>
On Mon, May 18, 2026 at 05:58:12PM +0530, Hardik Prakash wrote:
> Patch 1/2 (pinctrl-amd GPIO IRQ fix) is already in Linus Walleij's
> tree. This series contains only the i2c-designware probe ordering fix,
> based on top of that commit.
>
> The root cause: i2c_designware probes AMDI0010:02 before pinctrl-amd
> completes, so GPIO 157 (WACF2200 GpioInt per ACPI _CRS) has its
> interrupt bits cleared when the first I2C transaction is attempted,
> causing lost arbitration errors.
>
> A higher-level ACPI devlink approach was investigated in response to
> Bartosz Golaszewski's suggestion. The DSDT has no _DEP object linking
> AMDI0010:02 to AMDI0030:00, so fw_devlink has nothing to act on.
> Setting this up at the ACPI layer would require either a firmware
> change to add _DEP, or a DMI quirk in the ACPI scan path — equally
> quirk-based as the current approach.
>
> v5:
> - Add blank line before #include <linux/acpi.h> (Bartosz Golaszewski)
> - Use scoped_guard(device, gpio_dev) (Bartosz Golaszewski)
Too fast, I have a comment in v4, do you have a chance to read it?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Mario Limonciello @ 2026-05-18 14:08 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Hardik Prakash, linux-i2c, linux-gpio, wsa, andriy.shevchenko,
basavaraj.natikar, linus.walleij
In-Reply-To: <CAMRc=MdS_BVKb=FQLhky=8dpghBSoHeBhUk0LM5hROFxmJeyGQ@mail.gmail.com>
On 5/18/26 09:05, Bartosz Golaszewski wrote:
> On Mon, May 18, 2026 at 3:46 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>>
>>
>> On 5/18/26 08:40, Hardik Prakash wrote:
>>> On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
>>>> I'd still like to avoid a quirk if we can.
>>>>
>>>> I know my proposed patch to try to probe at an earlier stage didn't
>>>> work, but could you perhaps try pulling pinctrl-amd even earlier?
>>>>
>>>> Maybe fs_initcall()?
>>>
>>> Tested. fs_initcall + patch 1 still produces the same arbitration
>>> errors:
>>>
>>> subsys_initcall + patch 1: arbitration errors persist
>>> fs_initcall + patch 1: arbitration errors persist
>>> patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
>>>
>>> The initcall level does not appear to be the determining factor on
>>> this hardware. i2c_designware is still probing AMDI0010:02 before
>>> pinctrl-amd finishes regardless of how early pinctrl-amd registers.
>>> The explicit device_is_bound() deferral in patch 2 is the only
>>> approach that has worked.
>>
>> Please try arch_initcall instead.
>>
>
> What is blocking the pinctrl driver from probing? Does it return
> -EPROBE_DEFER for some reason? Pin control core is ready at
> core_initcall() so it should work in theory.
>
> Bart
Currently it's module_platform_driver() IE device_initcall().
That's why I think we "should" be able to move it a lot earlier.
^ permalink raw reply
* Re: [PATCH v4 1/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Andy Shevchenko @ 2026-05-18 14:06 UTC (permalink / raw)
To: Hardik Prakash
Cc: Bartosz Golaszewski, linux-i2c, linux-gpio, wsa,
mario.limonciello, basavaraj.natikar, linus.walleij
In-Reply-To: <CANTFpSU_Ba1c_R9wbjSSqnc1+_vBMAOKDvD=EVtct8hWU+Dxqw@mail.gmail.com>
On Mon, May 18, 2026 at 05:55:02PM +0530, Hardik Prakash wrote:
> On Mon, May 18, 2026 at 16:24, Bartosz Golaszewski wrote:
> > The patch looks fine but I too would prefer this to be handled at a
> > higher-level. If we know which ACPI devices we're waiting for and what
> > the ordering should be - is it possible to somehow setup the devlink
> > for the platform devices that will be the children of these ACPI
> > devices?
> >
> > If that can't be done, I'm fine with this as a workaround.
>
> I checked the DSDT and the kernel's ACPI dependency mechanisms. The
> DSDT has no _DEP object linking AMDI0010:02 to AMDI0030:00, so there
> is nothing for fw_devlink to act on. The GpioInt resource is on the
> touchscreen device (TPNL/WACF2200), not on the I2C controller itself.
>
> Setting this up at the ACPI layer would require either a firmware
> change to add _DEP, or a DMI quirk in the ACPI scan path to synthesize
> the dependency — which would be equally quirk-based as the current
> approach, just in a different driver.
Don't we have already force and ignore lists for _DEP somewhere in the
drivers/acpi/?
> I'll send v5 with your two style fixes applied.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Bartosz Golaszewski @ 2026-05-18 14:05 UTC (permalink / raw)
To: Mario Limonciello
Cc: Hardik Prakash, linux-i2c, linux-gpio, wsa, andriy.shevchenko,
basavaraj.natikar, linus.walleij
In-Reply-To: <7d0f0cf9-1936-4cf6-a425-228a37f83137@amd.com>
On Mon, May 18, 2026 at 3:46 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
>
>
> On 5/18/26 08:40, Hardik Prakash wrote:
> > On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
> >> I'd still like to avoid a quirk if we can.
> >>
> >> I know my proposed patch to try to probe at an earlier stage didn't
> >> work, but could you perhaps try pulling pinctrl-amd even earlier?
> >>
> >> Maybe fs_initcall()?
> >
> > Tested. fs_initcall + patch 1 still produces the same arbitration
> > errors:
> >
> > subsys_initcall + patch 1: arbitration errors persist
> > fs_initcall + patch 1: arbitration errors persist
> > patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
> >
> > The initcall level does not appear to be the determining factor on
> > this hardware. i2c_designware is still probing AMDI0010:02 before
> > pinctrl-amd finishes regardless of how early pinctrl-amd registers.
> > The explicit device_is_bound() deferral in patch 2 is the only
> > approach that has worked.
>
> Please try arch_initcall instead.
>
What is blocking the pinctrl driver from probing? Does it return
-EPROBE_DEFER for some reason? Pin control core is ready at
core_initcall() so it should work in theory.
Bart
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Mario Limonciello @ 2026-05-18 13:45 UTC (permalink / raw)
To: Hardik Prakash
Cc: linux-i2c, linux-gpio, wsa, andriy.shevchenko, brgl,
basavaraj.natikar, linus.walleij
In-Reply-To: <CANTFpSWGC7GsAY-3UvPtBZzqjNek-T5haiDb59QYRoRgwuQf1w@mail.gmail.com>
On 5/18/26 08:40, Hardik Prakash wrote:
> On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
>> I'd still like to avoid a quirk if we can.
>>
>> I know my proposed patch to try to probe at an earlier stage didn't
>> work, but could you perhaps try pulling pinctrl-amd even earlier?
>>
>> Maybe fs_initcall()?
>
> Tested. fs_initcall + patch 1 still produces the same arbitration
> errors:
>
> subsys_initcall + patch 1: arbitration errors persist
> fs_initcall + patch 1: arbitration errors persist
> patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
>
> The initcall level does not appear to be the determining factor on
> this hardware. i2c_designware is still probing AMDI0010:02 before
> pinctrl-amd finishes regardless of how early pinctrl-amd registers.
> The explicit device_is_bound() deferral in patch 2 is the only
> approach that has worked.
Please try arch_initcall instead.
>
> Thanks,
> Hardik
>
> On Mon, 18 May 2026 at 18:17, Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>>
>>
>> On 5/18/26 07:28, Hardik Prakash wrote:
>>> Patch 1/2 (pinctrl-amd GPIO IRQ fix) is already in Linus Walleij's
>>> tree. This series contains only the i2c-designware probe ordering fix,
>>> based on top of that commit.
>>>
>>> The root cause: i2c_designware probes AMDI0010:02 before pinctrl-amd
>>> completes, so GPIO 157 (WACF2200 GpioInt per ACPI _CRS) has its
>>> interrupt bits cleared when the first I2C transaction is attempted,
>>> causing lost arbitration errors.
>>>
>>> A higher-level ACPI devlink approach was investigated in response to
>>> Bartosz Golaszewski's suggestion. The DSDT has no _DEP object linking
>>> AMDI0010:02 to AMDI0030:00, so fw_devlink has nothing to act on.
>>> Setting this up at the ACPI layer would require either a firmware
>>> change to add _DEP, or a DMI quirk in the ACPI scan path — equally
>>> quirk-based as the current approach.
>>
>> I'd still like to avoid a quirk if we can.
>>
>> I know my proposed patch to try to probe at an earlier stage didn't
>> work, but could you perhaps try pulling pinctrl-amd even earlier?
>>
>> Maybe fs_initcall()?
>>
>>>
>>> v5:
>>> - Add blank line before #include <linux/acpi.h> (Bartosz Golaszewski)
>>> - Use scoped_guard(device, gpio_dev) (Bartosz Golaszewski)
>>>
>>> v4:
>>> - Rebase onto Linus Walleij's tree (patch 1 already there)
>>> - Use --base so series is correctly 1/1 (Andy Shevchenko)
>>> - Add subsys_initcall test results (Mario Limonciello)
>>>
>>> v3:
>>> - Fix variable declaration style in dw_i2c_needs_amd_gpio_dep (Andy Shevchenko)
>>> - Add BugLink tag (Andy Shevchenko)
>>> - Add -v3 subject versioning (Andy Shevchenko)
>>> - CC AMD engineers (Andy Shevchenko)
>>>
>>> v2:
>>> - Replace custom HID/UID lookup with acpi_dev_get_first_match_dev() (Andy Shevchenko)
>>> - Use acpi_get_first_physical_node() for platform device lookup
>>> - Use device_is_bound() under device_lock() with explanatory comments
>>> - Fix dev_warn to use dev_name() instead of hardcoded suffix
>>> - Fix commit message (removed incorrect "existing" reference)
>>> - Add Assisted-by tags per coding-assistants.rst
>>>
>>> Kernel bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=221494
>>> Related: https://bugzilla.kernel.org/show_bug.cgi?id=221454
>>>
>>> Hardik Prakash (1):
>>> i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7
>>> 14AGP11
>>>
>>> drivers/i2c/busses/i2c-designware-platdrv.c | 76 +++++++++++++++++++++
>>> 1 file changed, 76 insertions(+)
>>>
>>> base-commit: 3812a9e84265a5cdd90d29fe8d97a023e91fb945
>>
^ permalink raw reply
* Re: [PATCH v5 0/1] i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7 14AGP11
From: Hardik Prakash @ 2026-05-18 13:40 UTC (permalink / raw)
To: Mario Limonciello
Cc: linux-i2c, linux-gpio, wsa, andriy.shevchenko, brgl,
basavaraj.natikar, linus.walleij
In-Reply-To: <d57dd297-6be1-43a9-a38d-e40c8949e23a@amd.com>
On Mon, May 18, 2026 at 18:17, Mario Limonciello wrote:
> I'd still like to avoid a quirk if we can.
>
> I know my proposed patch to try to probe at an earlier stage didn't
> work, but could you perhaps try pulling pinctrl-amd even earlier?
>
> Maybe fs_initcall()?
Tested. fs_initcall + patch 1 still produces the same arbitration
errors:
subsys_initcall + patch 1: arbitration errors persist
fs_initcall + patch 1: arbitration errors persist
patch 1 + patch 2 (v5): clean boot, touchscreen fully functional
The initcall level does not appear to be the determining factor on
this hardware. i2c_designware is still probing AMDI0010:02 before
pinctrl-amd finishes regardless of how early pinctrl-amd registers.
The explicit device_is_bound() deferral in patch 2 is the only
approach that has worked.
Thanks,
Hardik
On Mon, 18 May 2026 at 18:17, Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
>
>
> On 5/18/26 07:28, Hardik Prakash wrote:
> > Patch 1/2 (pinctrl-amd GPIO IRQ fix) is already in Linus Walleij's
> > tree. This series contains only the i2c-designware probe ordering fix,
> > based on top of that commit.
> >
> > The root cause: i2c_designware probes AMDI0010:02 before pinctrl-amd
> > completes, so GPIO 157 (WACF2200 GpioInt per ACPI _CRS) has its
> > interrupt bits cleared when the first I2C transaction is attempted,
> > causing lost arbitration errors.
> >
> > A higher-level ACPI devlink approach was investigated in response to
> > Bartosz Golaszewski's suggestion. The DSDT has no _DEP object linking
> > AMDI0010:02 to AMDI0030:00, so fw_devlink has nothing to act on.
> > Setting this up at the ACPI layer would require either a firmware
> > change to add _DEP, or a DMI quirk in the ACPI scan path — equally
> > quirk-based as the current approach.
>
> I'd still like to avoid a quirk if we can.
>
> I know my proposed patch to try to probe at an earlier stage didn't
> work, but could you perhaps try pulling pinctrl-amd even earlier?
>
> Maybe fs_initcall()?
>
> >
> > v5:
> > - Add blank line before #include <linux/acpi.h> (Bartosz Golaszewski)
> > - Use scoped_guard(device, gpio_dev) (Bartosz Golaszewski)
> >
> > v4:
> > - Rebase onto Linus Walleij's tree (patch 1 already there)
> > - Use --base so series is correctly 1/1 (Andy Shevchenko)
> > - Add subsys_initcall test results (Mario Limonciello)
> >
> > v3:
> > - Fix variable declaration style in dw_i2c_needs_amd_gpio_dep (Andy Shevchenko)
> > - Add BugLink tag (Andy Shevchenko)
> > - Add -v3 subject versioning (Andy Shevchenko)
> > - CC AMD engineers (Andy Shevchenko)
> >
> > v2:
> > - Replace custom HID/UID lookup with acpi_dev_get_first_match_dev() (Andy Shevchenko)
> > - Use acpi_get_first_physical_node() for platform device lookup
> > - Use device_is_bound() under device_lock() with explanatory comments
> > - Fix dev_warn to use dev_name() instead of hardcoded suffix
> > - Fix commit message (removed incorrect "existing" reference)
> > - Add Assisted-by tags per coding-assistants.rst
> >
> > Kernel bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=221494
> > Related: https://bugzilla.kernel.org/show_bug.cgi?id=221454
> >
> > Hardik Prakash (1):
> > i2c: designware: fix probe ordering for AMD GPIO on Lenovo Yoga 7
> > 14AGP11
> >
> > drivers/i2c/busses/i2c-designware-platdrv.c | 76 +++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> >
> > base-commit: 3812a9e84265a5cdd90d29fe8d97a023e91fb945
>
^ 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