Re: [PATCH] can: mcp251x: fix support for half duplex SPI host controllers - Marc Kleine-Budde

From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Tim Harvey <tharvey@gateworks.com>
Cc: Gerhard Bertelsmann <info@gerhard-bertelsmann.de>,
	linux-can@vger.kernel.org
Subject: Re: [PATCH] can: mcp251x: fix support for half duplex SPI host controllers
Date: Wed, 31 Mar 2021 17:05:59 +0200	[thread overview]
Message-ID: <20210331150559.5fbus7itrotjwvxs@pengutronix.de> (raw)
In-Reply-To: <CAJ+vNU10c5NtLaG64VhcYO8E9sA3di6v5_MnNHVTaFrfkrpxyQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]

On 31.03.2021 07:54:19, Tim Harvey wrote:
> > 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.

[...]

> 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.

Thanks for mainlining the half duplex flag and clarifying the issue
here, I was to lazy to look up which controller you're using and if the
patch was mainlined.

> > 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@gateworks.com>

Thanks a lot!

> 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?

Yes I had this discussion, but the only thing the SPI core can do is to
aggregate several half duplex transfers into one full duplex one. This
requires a lot of memory handling/copying and I decided to not work on
this.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]