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

[Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>]

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