Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath()

From: Lukasz Stelmach <l.stelmach@samsung.com>
To: Mark Brown <broonie@kernel.org>
Cc: linux-samsung-soc@vger.kernel.org, b.zolnierkie@samsung.com,
	linux-kernel@vger.kernel.org,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux-spi@vger.kernel.org, Kukjin Kim <kgene@kernel.org>,
	Andi Shyti <andi@etezian.org>,
	linux-arm-kernel@lists.infradead.org, m.szyprowski@samsung.com
Subject: Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath()
Date: Wed, 19 Aug 2020 16:01:52 +0200	[thread overview]
Message-ID: <dleftj4koy20q7.fsf%l.stelmach@samsung.com> (raw)
In-Reply-To: <20200819131635.GD5441@sirena.org.uk> (Mark Brown's message of "Wed, 19 Aug 2020 14:16:35 +0100")

[-- Attachment #1.1: Type: text/plain, Size: 1842 bytes --]

It was <2020-08-19 śro 14:16>, when Mark Brown wrote:
> On Wed, Aug 19, 2020 at 02:58:22PM +0200, Krzysztof Kozlowski wrote:
>> On Wed, Aug 19, 2020 at 02:51:27PM +0200, Lukasz Stelmach wrote:
>
>> > Honestly, I don't know and I couldn't find out why. It makes stuff
>> > work. There has been a commit like this before
>
>> >     0f5a751ace25 spi/s3c64xx: Enable GPIO /CS prior to starting hardware
>
>> > Apparently, it was lost in
>
>> >     0732a9d2a155 spi/s3c64xx: Use core message handling
>
>> Then describe at least this... maybe Mark knows why he brough back old
>> code after refactoring?
>
> I'm not sure what's being referred to as being lost in the second commit
> TBH.

Order of enable_cs() and enable_datapath(). The order 0f5a sets makes
(for a reaseon I don't know) my devices work. In the latter commit,
which rewrites "everything", enable_datapath() is called before what
later (in aa4964c4eb3e) became s3c64xx_spi_set_cs().

> The first commit is simple code motion rather than a correctness
> thing, and more related to the handling of GPIO controlled chip
> selects according to the description (which people should be using
> with that controller anyway where possible IIRC, the native chip
> select has too many assumptions about what it's doing).

Funny, but without the automatic CS control (see the next patch in this
series) my stuff does not work.

> I don't know that I ever actually used a system that used the native
> chip select as the actual chip select.  Perhaps some quirk was
> introduced where the chip select signal does something?
>
> The commit is also lacking a description of what the issues that are
> being fixed are.

On Exynos3250 DMA transfers from SPI longer than 512 fail.

-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel