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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 Aug 2020 16:01:52 +0200
From:   Lukasz Stelmach <l.stelmach@...sung.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Krzysztof Kozlowski <krzk@...nel.org>,
        Kukjin Kim <kgene@...nel.org>, Andi Shyti <andi@...zian.org>,
        linux-spi@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        m.szyprowski@...sung.com, b.zolnierkie@...sung.com
Subject: Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and
 s3c64xx_enable_datapath()

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

Download attachment "signature.asc" of type "application/pgp-signature" (488 bytes)