On Wed, Mar 31, 2021 at 12:14 AM Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
>
> On 30.03.2021 14:06:03, Tim Harvey wrote:
> > On Tue, Mar 30, 2021 at 3:02 AM Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> > >
> > > Some SPI host controllers do not support full-duplex SPI transfers.
> > >
> > > The function mcp251x_spi_trans() does a full duplex transfer. It is
> > > used in several places in the driver, where a TX half duplex transfer
> > > is sufficient.
> > >
> > > To fix support for half duplex SPI host controllers, this patch
> > > introduces a new function mcp251x_spi_write() and changes all callers
> > > that do a TX half duplex transfer to use mcp251x_spi_write().
>
> > So was the issue being resolved here that there was another SPI host
> > controller that wasn't advertising that it was half duplex only
>
> I don't know which SPI host controller Gerhard uses, but I assume it has
> half duplex set, as the driver probe fails with:
>
> | [ 112.226164] mcp251x spi0.1: spi transfer failed: ret = -22
>
> The -22 is returned by the SPI framework if you have a half duplex
> controller and a transfer with both TX and RX buffer set. This is the
> case in the mcp251x_spi_trans() function.
>
> > or was something else wrong with e0e25001d088 ("can: mcp251x: add
> > support for half duplex controllers")?
>
> Your patch only converted the SPI read path to use half duplex
> transfers. My patch also converts the SPI write path.
>
> If your half duplex controller works without that patch, the controller
> driver doesn't advertise correctly that it is half duplex only. If the
> hardware is indeed half duplex only, better send a patch that sets the
> half duplex flag. If the hardware support full duplex, but the driver
> somehow doesn't implement it correctly, so that it implements half
> duplex only you should at least drop a note on the SPI mailing list.
Marc,
Thanks for the explanation!
I was surprised as the 5.4 kernel I use with the CN803x OcteonTX using
drivers/spi/spi-cavium-thunderx.c works fine but as you say it is
because the host controller does not advertise half duplex only in
that kernel. I did mainlin in e8510d43f219 ("spi: spi-cavium-thunderx:
flag controller as half duplex") which appears in 5.9.
>
> Can you test this patch and give me a Tested-by?
>
I did verify that with this patch 5.12-rc5 initializes the mcp251x on
the CN803x OcteonTx and without it we fail.
Tested on a GW6404 board with an OcteonTX SoC and MCP25625
Tested-By: Tim Harvey <tharvey@xxxxxxxxxxxxx>
By the way, I believe you were discussing at one point the possibility
of adding something in the spi core that would be able to implement
half duplex transactions for drivers written for full duplex
communication. Is that something on your list or even possible?
regards,
Tim
