Re: [PATCH 4/7] spi: bcm2835: Drop unused code for native Chip Select

[Lukas Wunner <lukas@xxxxxxxxx>]

[cc += Phil Elwell]

On Sat, Nov 10, 2018 at 10:07:38AM +0100, kernel@xxxxxxxxxxxxxxxx wrote:
> > On 08.11.2018, at 08:06, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > Commit a30a555d7435 ("spi: bcm2835: transform native-cs to gpio-cs on
> > first spi_setup") disabled the use of hardware-controlled native Chip
> > Select in favour of software-controlled GPIO Chip Select but left code
> > to support the former untouched.  Remove it to simplify the driver and
> > ease the addition of new features and further optimizations.
> 
> I believe downstream is still using native CS instead of GPIO by default.
> (Not sure if this really still applies), so they would still be using
> this code-fragment.

Back in 2016 someone reported issues caused by software-controlled CS
with an RFID reader.  Phil Elwell therefore disabled the conversion
from native CS to software-controlled CS as a temporary measure:

   "The quick fix is to push a downstream patch that restores the
    ability to use hardware CE, followed by an overlay that makes
    use of that ability. Later we could try to understand why the
    software CE doesn't work and fix it if possible."

    https://github.com/raspberrypi/linux/issues/1547#issuecomment-230043141
    https://github.com/raspberrypi/linux/commit/b6b83b7b1ef4
    https://github.com/raspberrypi/linux/commit/98eed4009d63

The root cause turned out to be a bug in user space, in the SPI-Py
library to be specific:

    https://github.com/lthiery/SPI-Py/issues/17

After the fix to the SPI-Py library was accepted, the temporary
changes in the kernel and DT should arguably have been reverted,
but unfortunately that never happened.  The commits are still
part of the rpi-4.19.y branch.

However I don't think it is upstream's responsibility to accommodate
to downstream keeping obsolete code changes around.


> IIRC the code was als kept for DT-backwards compatibility when the GPIO
> function is set to ALT0 - but I may be wrong there...

AFAICS, the pin's function is set to alt0 by *default*.

I think there is another issue though with the conversion from native CS
to GPIO CS in bcm2835_spi_setup():  If pin 7/8 as well as pin 35/36 is
configured to alt0, *both* pins should be toggled.  But that issue is
unrelated to the present series.

Thanks,

Lukas