Re: [PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode

From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Sergey Suloev <ssuloev@orpaltech.com>
Cc: Mark Brown <broonie@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-spi@vger.kernel.org
Subject: Re: [PATCH v3 3/6] spi: sun6i: restrict transfer length in PIO-mode
Date: Mon, 9 Apr 2018 13:36:03 +0200	[thread overview]
Message-ID: <20180409113603.2iexkqvyeqmysp5e@flea> (raw)
In-Reply-To: <67c2006b-17f2-2459-e3c9-e91e3c694d8c@orpaltech.com>

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

On Mon, Apr 09, 2018 at 02:10:40PM +0300, Sergey Suloev wrote:
> On 04/09/2018 01:50 PM, Mark Brown wrote:
> > On Mon, Apr 09, 2018 at 01:26:23PM +0300, Sergey Suloev wrote:
> > > On 04/09/2018 12:27 PM, Maxime Ripard wrote:
> > > > On Fri, Apr 06, 2018 at 06:48:23PM +0300, Sergey Suloev wrote:
> > > > > On 04/06/2018 10:34 AM, Maxime Ripard wrote:
> > > > > According to what you said the driver must implement
> > > > > "transfer_one_message" instead of "transfer_one"
> > > > I'm not sure what makes you think that I said that.
> > > Because current implementation tries to send more than FIFO-depth of data in
> > > a single call to "transfer_one" which is wrong.
> > No, that's absolutely not the case.  All any of these functions has to
> > do is transfer whatever they were asked to, how they do it is not at all
> > important to the framework.
> 
> I think you don't fully understand the issue. Let's talk about sun4i and 
> sun6i SPI  drivers separately.
> 
> sun4i
> 
> 1)it is correctly declaring max_transfer_size=FIFO depth for PIO mode  but
> transfer_one() function doesn't follow the declaration allowing PIO
> transfers longer than FIFO depth  by just refilling FIFO using 3/4 FIFO
> empty interrupt. I can definitely state here that long transfers WON'T WORK
> on real hardware.

Surely the original author of the patch allowing to do just that
disagrees with you. And it's not about the hardware itself, it's about
how the driver operates as well.

> I tested it and that's why I can say that.

Then it must be fixed, and not silently reverted.

> But as soon as sun4i SPI driver  is correctly declaring
> max_transfer_size then "smart" clients will work well by limiting a
> single transfer size to FIFO depth. I tested it with real hardware,
> again.

This is really not my point. What would prevent you from doing
multiple transfers in that case, and filling the FIFO entirely,
waiting for it to be done, then resuming until you have sent the right
number of bytes?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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