Thread (35 messages) 35 messages, 7 authors, 2026-02-25

Re: [PATCH v2 0/4] iio: adc: ad4080: add support for AD4880 dual-channel ADC

From: Jonathan Cameron <jic23@kernel.org>
Date: 2026-02-18 19:08:45
Also in: linux-iio, linux-spi, lkml

On Tue, 17 Feb 2026 16:55:56 -0600
David Lechner [off-list ref] wrote:
On 2/17/26 2:28 AM, Andy Shevchenko wrote:
quoted
On Mon, Feb 16, 2026 at 12:53:10PM -0600, David Lechner wrote:  
quoted
On 2/16/26 1:14 AM, Andy Shevchenko wrote:  
quoted
On Sun, Feb 15, 2026 at 05:16:47PM -0600, David Lechner wrote:  
quoted
On 2/15/26 2:03 AM, Andy Shevchenko wrote:  
quoted
On Sat, Feb 14, 2026 at 12:31:12PM -0600, David Lechner wrote:  
quoted
On 2/14/26 12:11 PM, Andy Shevchenko wrote:  
quoted
On Sat, Feb 14, 2026 at 04:08:52PM +0000, Jonathan Cameron wrote:  
quoted
On Sun, 8 Feb 2026 14:50:23 +0200
Andy Shevchenko [off-list ref] wrote:  
quoted
On Fri, Feb 06, 2026 at 06:07:12PM +0200, Antoniu Miclaus wrote:  
...
  
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
I believe there is a better approach, what you need is rather a flag
to SPI core to tell that this is the device with shared CS.  
Antoniu, this comment from Andy needs addressing before we move
on. It seems fairly fundamental and I'm not seeing a reply to it on list.

I'm not entirely sure what Andy is suggesting will work but this
is perhaps a mismatch in really understanding what is going on here.
Andy, how would a flag work given they seem to be separately addressable
SPI buses. I think this isn't a shared SPI CS, but rather a device
with two entirely separate SPI buses. I think the only reason
we are bothering to implement it as a single device at all is the
shared backend.  
My understanding that there are two devices that for whatever reason share  
It is the opposite. It is a _single_ device with _two_ CS lines.  
Don't we have already support for that? This changes the picture even more towards
NAKing this. See below why.  
Yes, spi_new_ancillary_device() was introduced exactly for this sort
of thing, which is why I think it makes sense to use it.
 
quoted
quoted
adc@0 {
	reg = <0>, <1>;
	...
};
 
quoted
the same CS line. Yes, I probably misread the idea behind, but I meant
some flag for SPI device that tells SPI core that the CS it wants is shared
(maybe a high bit in the cs field or so), then CS core won't complain on
validation about using the same cs number which is "already in use".  
There was one existing user in the kernel of spi_new_ancillary_device()
that looked like this, so it seemed the right way to approach it. However,
code was added later that caused the primary SPI device to "claim" both
CS lines for itself and probably broke the one existing user of
spi_new_ancillary_device() (hard to tell without hardware to test).

The idea here was to unbreak that so we could use spi_new_ancillary_device()
just as in the existing use case.

The patch for that could have been a bit more strict to only allow the
spi_new_ancillary_device() to take CS 1 and fail otherwise, but users
are going to notice if it isn't working right anyway, so I didn't ask
for more checking.  
 
quoted
quoted
quoted
There is an argument that maybe we should be looking at how
to do data muxing backends to support the more general case of two
separate chips feeding into a single buffer, but that's a complex
beast and I'm not sure if it is something we actually need.  
I think it would actually be quite similar to what is done in this
series.  
TBH, the change sounds to me like a hack. It doesn't cover other potential ways
of the multi-cs devices come into play. Given that SPI core supports multi-cs
I don't see a good justification for this patch.

What did I miss?  
As far as I can tell, other than the one existing user of
spi_new_ancillary_device(), other SPI multi-CS stuff is only used
by SPI flash memory devices, not general SPI devices. There code
that is being modified here was introduced to support the SPI
flash memory devices, so that use case is already covered by
existing code.  
Right. And obvious question why can't we apply the same approach
to any SPI device? Like extending existing code to cover generic
cases.  
spi_new_ancillary_device() was already accepted in the kernel as the
solution for this sort of use case, so isn't it already the generic
approach?  
I don't think the single user functionality is considered generic.
  
quoted
I can see that it could possibly be nice if the SPI core saw that
there was more than one CS and called spi_new_ancillary_device()
automatically and somehow passed that along with the main SPI device
to the driver probe function. But since this is only the second user
of spi_new_ancillary_device(), I don't think we have enough data
points to be able to say if this is really what all peripheral drivers
would want.  
Also, if that one designed for the case, why is needed patching?  
Because the multi-CS stuff for SPI flash memory was added later
and broke it. There is only one obscure user, so it is not entirely
surprising if no one noticed yet.
quoted
...

The  mentioned approach predates the SPI memory chip support being
integrated into SPI core. I think we should consider to kill
spi_new_ancillary_device() in favour of using the same mechanism
as being used for SPI mem chips.
  
I'm not sure the SPI mem work ever actually got finished. In the code, see:

	if ((of_property_present(nc, "parallel-memories")) &&
	    (!(ctlr->flags & SPI_CONTROLLER_MULTI_CS))) {
		dev_err(&ctlr->dev, "SPI controller doesn't support multi CS\n");
		return -EINVAL;
	}

But there is no SPI controller that has that flag. So I'm not sure if
anyone is actually using this yet. And anyway I think the aim there was
to be able to assert two CS at the same time, which is not what we are
aiming to do here.

And the other potential user of multi-cs is stacked-memories, but this
is only mentioned in dt-bindings docs and nowhere else.

There doesn't seem to be any other code besides the validation that is
done when the SPI device is added that makes use of more than one CS line.

I would like to agree with you that there should be a better way, but I
still don't see an obvious way to do it if there is one (other than the
suggestion I already gave that probe should somehow give you two spi
devices instead of one).
I wonder a bit if a single SPI device but with explicit control of which
CS index in the spi_messages or similar would work?  The disadvantage is you'd
probably want a lot of helpers to have variants with a selection parameter.

Would end up smelling like paged registers, just with a chip select to pick
the page rather than a page register.

If this isn't common, may not be worth the effort.

Jonathan
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help