Received-SPF: pass (google.com: domain of linux-kernel+bounces-204962-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 6 Jun 2024 16:57:52 -0300
From: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@analog.com>, lars@metafoo.de,
	Michael.Hennerich@analog.com, jic23@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	nuno.sa@analog.com, dlechner@baylibre.com,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/6] spi: Add SPI mode bit for MOSI idle state
 configuration
Message-ID: <ZmIUwHhjAUzZnfW5@debian-BULLSEYE-live-builder-AMD64>
References: <cover.1717539384.git.marcelo.schmitt@analog.com>
 <e1d5d57f7a7481c84f64a764f9898122e278739b.1717539384.git.marcelo.schmitt@analog.com>
 <0a716b10-0ae0-425f-919a-ea5d8b7975b6@sirena.org.uk>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0a716b10-0ae0-425f-919a-ea5d8b7975b6@sirena.org.uk>

Hi,

On 06/05, Mark Brown wrote:
> On Tue, Jun 04, 2024 at 07:41:47PM -0300, Marcelo Schmitt wrote:
> 
> > The behavior of an SPI controller data output line (SDO or MOSI or COPI
> > (Controller Output Peripheral Input) for disambiguation) is not specified
> > when the controller is not clocking out data on SCLK edges. However, there
> > exist SPI peripherals that require specific COPI line state when data is
> > not being clocked out of the controller.
> 
> This is an optimisation for accelerating devices that need a specific
> value, really if these devices need a value they should send it.

I see it more like an extension of SPI controller functionality.
Though I guess it might also be used for optimization if tx is known to be
always 0s or always 1s for a device.

> 
> >  #define SPI_MOSI_IDLE_LOW	_BITUL(17)	/* leave mosi line low when idle */
> > +#define SPI_MOSI_IDLE_HIGH	_BITUL(18)	/* leave mosi line high when idle */
> 
> Realistically we'll have a large set of drivers that are expecting the
> line to be held low so I'm not sure we need that option.  I would also
Yes, I also think most SPI devices, if ever requiring anything, would
expect the MOSI line to be low. But this patchset is about the exception to that. :)

> expect to have an implementation of these options in the core which
> supplies buffers with the relevant data for use with controllers that
> don't have the feature (similar to how _MUST_TX and _MUST_RX are done).
> Even without that we'd need feature detection so that drivers that try
> to use this aren't just buggy when used with a controller that doesn't
> implement it, but once you're detecting you may as well just make things
> work.

As far as I searched, the definitions for SPI protocol usually don't specify any
behavior for the MOSI line when the controller is not clocking out data.
So, I think SPI controllers that are not capable of implementing any type
of MOSI idle configuration are anyway compliant to what is usual SPI.
For those that can implement such feature, I thought peripherals could request
it by setting SPI mode bits.
If the controller can provide MOSI idle state configuration, then the controller
sets itself to act according to what peripheral asked.
If MOSI idle configuration is not supported, then we just move on and let
peripheral driver adapt to what is supported?
Guess we can't blame an SPI controller for it not supporting something that is
not specified in usual SPI protocols.

But yeah, it's not that evident what this patch set is all about and why this is
wanted so I made a wiki page to explain the reasoning for this set.
https://wiki.analog.com/software/linux/docs/spi/spi_copi_idle?rev=1717699755
Hopefully the figures with timing diagrams and transfer captures there will 
provide quicker understanding of this rather than I try to explain it with
only text.

If you still think we need feature detection for MOSI idle capability just let
me know, I'll implement what be needed.

Thanks,
Marcelo