linux-kernel - Re: [PATCH 2/2] spi: omap2-mcspi: Add support for MULTI-mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZcOg3JR9YW9JNxPp@localhost.localdomain>
Date: Wed, 7 Feb 2024 16:25:16 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Mark Brown <broonie@...nel.org>
Cc: linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
	thomas.petazzoni@...tlin.com,
	Miquel Raynal <miquel.raynal@...tlin.com>, yen-mei.goh@...sight.com,
	koon-kee.lie@...sight.com
Subject: Re: [PATCH 2/2] spi: omap2-mcspi: Add support for MULTI-mode

Le 06/02/24 - 10:56, Mark Brown a écrit :
> On Tue, Feb 06, 2024 at 11:00:50AM +0100, Louis Chauvet wrote:
> 
> > Introduce support for MULTI-mode in the OMAP2 MCSPI driver. Currently, the
> > driver always uses SINGLE mode to handle the chip select (CS). With this
> > enhancement, MULTI-mode is enabled for specific messages, allowing for a
> > shorter delay between CS enable and the message (some FPGA devices are
> > sensitive to this delay).
> 
> I have no idea based on this commit message what either of these modes
> is or how this shorter delay would be achieved, these terms are specific
> to the OMAP hardware AFAICT.  Please clarify this, it's hard to follow
> what the change does.  It looks like this is just a CS per word thing?

Indeed, you're right, the wording is probably very OMAP specific, I
didn't realize that earlier. I'll try to explain better. What about
this addition following the above paragraph, would it be clearer?

  [...] this delay).

  The OMAP2 MCSPI device can use two different mode to send messages:
  SINGLE and MULTI:
  In SINGLE mode, the controller only leverages one single FIFO, and the 
  host system has to manually select the CS it wants to enable.
  In MULTI mode, each CS is bound to a FIFO, the host system then writes 
  the data to the relevant FIFO, as the hardware will take care of the CS

  The drawback [...]

> Note that you may not have to tell the hardware the same word length as
> the transfer specifies, so long as the wire result is the same it
> doesn't matter.

If I understand correclty what you want is: given a message, containing 2
transfers of 4 bits, with cs_change disabled, use the multi mode and send 
only one 8 bits transfer instead of two 4 bits transfer?

This seems very complex to implement, and will only benefit in very 
niche cases.
If I have to add this, I have to:
- detect the very particular pattern "message of multiple transfer and 
those transfer can be packed in bigger transfer"
- reimplement the transfer_one_message method to merge multiple transfer 
into one;
- manage the rx buffer to "unmerge" the answer;
- take care of timings if requested;
- probably other issues I don't see

I agree this kind of optimisation can be nice, and may benefit for this
spi controller, but I don't have the time to work on it. If someone 
want to do it, it could be a nice improvement.

I just see that I miss my rebase, I will push a v2. This v2 will also 
change the commit name for patch 1/2.

Have a nice day,
Louis Chauvet

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com