On Fri, Jun 08, 2018 at 06:56:23AM +0000, Jonas Mark (BT-FIR/ENG1) wrote:

Hi Oleksij,

quoted

quoted

quoted

quoted

+               if (slave_is_not_busy(priv))
+                       return 0;
+

quoted

+               udelay(READY_POLL_US_GRAN);

Should it be atomic?
Can it use read_poll_* type of helpers instead?

Yes, it shall be atomic because we need to reduce the latency at
detecting the de-assertion of the busy signal. We accept that this will
cost CPU time.

In case the Companion itself is very busy and does not reply quickly we
are have second polling loop below which sleeps longer and is non-atomic.

I can confirm, this make huge impact on protocol performance. And this
protocol is not really the speed runner.

The challenge is that the protocol is synchronous and without back to
back transfers.

quoted

you can send dummy message to set CS.
+	struct spi_transfer t = {
+		.len = 0,
+		.cs_change = 1,
+	};

+	/* send dummy to trigger CS */
+	reinit_completion(&priv->fc_complete);
+	spi_sync_locked(spi, &m);

see my ancient unfinished code:
https://patchwork.kernel.org/patch/9418511/

We will check it out.

quoted

In general, this part of the code, can be provided as separate driver
which should be called as "SPI 5wire protocol". 3 wires for data, 1 -
chip select, 1 - busy state. Since, the slave cant fit to normal SPI
requirements, and can't be ready within predictable time, busy signal is
needed. Providing this as separate driver or part of SPI framework
should reduce the code for many different drivers.

The actual datagram on top of your spi companion should go to
separate driver. There are similar (almost identical designs)

can :+
char:+
dw:  +
gpio:+--->some_multi_end_mux_protocol--->spi_5wire_protocol->spi--->

In my knowledge, only data format on top of spi_5wire_protocol is
different. See my notes for similar topics:
https://github.com/olerem/icc
https://github.com/olerem/spi-5wire

With 5-wire protocol you are referencing to CLK, MISO, MOSI, CS (all
standard SPI signals) and an extra BUSY signal. What we implemented is
a 6-wire protocol. There is an additional REQUEST line where the SPI
slave requests a transfer. It is like a level triggered interrupt
request.

Yes, for making it more generic the code in drivers/spi/companion
could be split into a generic 6-wire protocol driver and a multiplexing
protocol on top of it.

How does a slave signal that it has data to be picked up with the
5-wire protocol?

The request and busy signals are multiplexed to one wire. So, it should
be easy to implement both, 5 and 6 wire protocol in one driver.

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |