From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jonas Mark (BT-FIR/ENG1)" Subject: Re: [PATCH 2/5] spi: implement companion-spi driver Date: Fri, 8 Jun 2018 06:56:23 +0000 Message-ID: Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: Andy Shevchenko , Wolfgang Grandegger , Marc Kleine-Budde , "linux-can@vger.kernel.org" , netdev , Linux Kernel Mailing List , Heiko Schocher , "ZHU Yi (BT-FIR/ENG1-Zhu)" , "Jonas Mark (BT-FIR/ENG1)" To: Oleksij Rempel Return-path: Received: from de-out1.bosch-org.com ([139.15.230.186]:45956 "EHLO de-out1.bosch-org.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750945AbeFHG40 (ORCPT ); Fri, 8 Jun 2018 02:56:26 -0400 Content-Language: de-DE Sender: netdev-owner@vger.kernel.org List-ID: Hi Oleksij, > > > > + if (slave_is_not_busy(priv)) > > > > + return 0; > > > > + > > > > > > > + 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-atomi= c. >=20 > 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. > you can send dummy message to set CS. > + struct spi_transfer t =3D { > + .len =3D 0, > + .cs_change =3D 1, > + }; >=20 > + /* send dummy to trigger CS */ > + reinit_completion(&priv->fc_complete); > + spi_sync_locked(spi, &m); >=20 > see my ancient unfinished code: > https://patchwork.kernel.org/patch/9418511/ We will check it out. > 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. >=20 > The actual datagram on top of your spi companion should go to > separate driver. There are similar (almost identical designs) >=20 > can :+ > char:+ > dw: + > gpio:+--->some_multi_end_mux_protocol--->spi_5wire_protocol->spi---> >=20 > 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? Greetings, Mark Building Technologies, Panel Software Fire (BT-FIR/ENG1)=20 Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY = | www.boschsecurity.com Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118=20 Aufsichtsratsvorsitzender: Stefan Hartung; Gesch=E4ftsf=FChrung: Gert van I= peren, Andreas Bartz, Thomas Quante, Bernhard Schuster