Re: [PATCH] spi: bcm2835: Convert to use CS GPIO descriptors

[Martin Sperl <kernel@xxxxxxxxxxxxxxxx>]

> On 21.04.2019, at 14:27, Stefan Wahren <stefan.wahren@xxxxxxxx> wrote:
> 
>> Am 20.04.19 um 17:46 schrieb Martin Sperl:
>> 
>>> On 20.04.2019, at 13:08, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>>> 
>>> This converts the BCM2835 SPI master driver to use GPIO
>>> descriptors for chip select handling.
>>> 
>>> The BCM2835 driver was relying on the core to drive the
>>> CS high/low so very small changes were needed for this
>>> part. If it managed to request the CS from the device tree
>>> node, all is pretty straight forward.
>>> 
>>> However for native GPIOs this driver has a quite unorthodox
>>> loopback to request some GPIOs from the SoC GPIO chip by
>>> looking it up from the device tree using gpiochip_find()
>>> and then offseting hard into its numberspace. This has
>>> been augmented a bit by using gpiochip_request_own_desc()
>>> but this code really needs to be verified. If "native CS"
>>> is actually an SoC GPIO, why is it even done this way?
>>> Should this GPIO not just be defined in the device tree
>>> like any other CS GPIO? I'm confused.
>> It has been done to keep dt-backward compatibility.
>> 
>> When the driver was written dt had configured native-cs.
> 
> This is done in a very opaque way. The bcm2835-rpi.dtsi defines a pinmux
> group alt0 which init SPI pins (including native CS). So we better use
> spi0_gpio7 (including native CS) or even better create new groups
> without native CS and use them.

I remember that there was a patch of mine to achieve this but it was
turned down because it would not describe the the hw and the altx 
Pin up still needed to get at least documented that way. And that
when enabling the spi Block the corresponding  gpio Block would 
need to get configured properly.

At that point I left the discussion as the effect of using gpio mode
was implemented by the driver. I can search for the patch
 from a few years ago...

> Even worse the alt0 group is applied for all the RPi Zero, 1 and 2
> boards, but not for the RPi 3 ones.
> 
>> A native-cs implementation comes with lots of
>> limitations and some of which - especially dma mode
>> has some quirks with regards to native-cs (glitches,
>> deasserts after each finished dma transfer,...)
>> 
>> The only way to make this work properly was to use
>> Gpio.
> 
> So we better extend the current binding with cs-gpios as optional
> property in order to point the user the right direction. And gave the
> same warnings like in bcm2835-spi-aux.

The difference is that aux has different issues and this driver
Is not working properly without gpio cs giving unexpected results
 and resulting in regressions.

 Keeping gpio cs being mandatory is the only way to avoid
regressions everything else is just relying on the legacy code
and thus avoiding the discussion.

Martin