Re: [PATCH v11 03/10] spi: Add multi-cs memories support in SPI core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On 1/21/24 10:06, Michael Walle wrote:
FWIW, the problem is due to

+#define SPI_CS_CNT_MAX 4

in the offending patch, but apeed2400 FMC supports up to 5 SPI chip selects.

  static const struct aspeed_spi_data ast2400_fmc_data = {
         .max_cs        = 5,
    ^^^^^^^^^^^^^^^^^^^
         .hastype       = true,

Limiting .max_cs to 4 or increasing SPI_CS_CNT_MAX to 5 fixes the problem,
though of course I don't know if increasing SPI_CS_CNT_MAX has other side
effects.

Yeah, I was coming to a similar conclusion myself - the limit is just
too low.  I can't see any problem with increasing it.

It would cost a bit of memory and somewhat affect performance sine many
of the newly introduced loops are bound by SPI_CS_CNT_MAX and not by
num_chipselect.

It also might make sense to document the new limit somewhere. Prior
to this commit it was not limited at all.
Documentation/devicetree/bindings/spi/spi-davinci.txt lists 5 chip
selects in its example for the use of cs-gpios.
Documentation/devicetree/bindings/spi/spi-controller.yaml also does not
list a limit.

Given that, that the rest of this series is under discussion (and esp. whether
it is the correct way to do it) it might make sense to just revert the picked
patches.


I can't really comment on that, but I found that there is another
affected devicetree property: num-cs. Its range isn't limited in
Documentation/devicetree/bindings/spi/spi-controller.yaml. Various
dts/dtsi files use numbers as large as 8. The range is limited in some
bindings files, but not all of them.
Documentation/devicetree/bindings/spi/spi-lantiq-ssc.txt, for example,
says

  "num-cs: see spi-bus.txt, set to 8 if unset"

Various Broadcom dtsi files set it to 8.

So I guess 8 would be the absolute minimum to re-enable support for
affected systems.

Guenter





[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux