linux-kernel - Re: [PATCH 0/3] spi: spi-imx: fix use of more than four chip selects

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, 26 Apr 2023 15:23:20 +0200
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Mark Brown <broonie@...nel.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        linux-spi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Kevin Groeneveld <kgroeneveld@...brook.com>
Subject: Re: [PATCH 0/3] spi: spi-imx: fix use of more than four chip selects

On 26/04/2023 15.10, Mark Brown wrote:
> On Wed, Apr 26, 2023 at 02:47:44PM +0200, Rasmus Villemoes wrote:
>> On 26/04/2023 14.25, Mark Brown wrote:

>> I described a problem with what is now 87c614175bbf in linux-next: If
>> one has five spi devices, the first four of which use the four native
>> chip selects, there is no way to use a gpio for the fifth, because
>> whichever "channel" you choose in the CHANNEL_SELECT field will cause
>> the ecspi IP block to drive some SSx pin low, while the spi core is also
>> driving the gpio low, so two different devices would be selected.
> 
> Sure, and therefore I'd not expect anyone to actually describe the
> hardware like that but to instead describe the hardware as using three
> or fewer of the native chip selects with the remaining chip selects
> described as GPIOs.  If the device requires that a native chip select be
> controlled the hardware simply won't work without at least one native
> chip select being unallocated.

Exactly. But the current state (as of next-20230425) of the spi-imx
driver also doesn't work if one describes the hardware using between 1
and 3 native chip selects and the rest as gpios, because the naive
masking of ->chip_select could easily hit one of those native ones.

>> It's not exactly a regression, because any chip_select >= 4 never
>> actually worked, but what I'm saying is that 87c614175bbf also isn't a
>> complete fix if one wants to support mixing native and gpio chip
>> selects. For that, one really needs the unused_native_cs to be used for
>> all gpio chip selects; in particular, one needs some unused native cs to
>> exist. IOW, what my series tries to do.
> 
> No, we only need one unused chip select to be available.

Which is exactly what I'm saying, so I think we're in agreement.

I.e., something like this 3-patch series is needed to actually support
mixing native and gpio chip selects (having the core verify that there
is an unused chip select available, and provide that in the
->unused_native_cs field in the spi_controller). I don't think there's
any textual conflict with 87c614175bbf, and the masking done by
87c614175bbf doesn't hurt, but also becomes irrelevant if this series is
applied, since we'd never pass any value > 3 to those macros.

Rasmus