Re: [PATCH] spi: orion: Allow specifying which HW CS to use with a GPIO CS

[Trent Piepho <tpiepho@xxxxxxxxxx>]

On Tue, 2018-02-20 at 20:02 +0100, Jan Kundrát wrote:
> I just found out that this patch only takes effect *after* the SPI slaves 
> have been probed. That's because the HW CS signal selection takes place 
> after a call to spi_register_master/spi_register_controller which adds the 
> SPI clients on its own. I now also understand why MSIOF discovers the CS in 
> such a complicated manner. Sorry. I wrote this patch for spidev where it is 
> not a problem, so I haven't seen this before.
> 
> I see three options now:
> 
> 1) Add an explicit DT property to let the user select an appropriate HW CS 
> signal. This has an advantage of being able to handle DT overlays in 
> future, as Trent pointed out.

You could also use a modulo scheme to allow specifying the hw cs to use
without adding a new property.

> 2) Split the spi_register_controller into two functions and let 
> orion_spi_probe access the cs_gpios in between them. No other drivers need 
> to be modified, but perhaps the MSIOF might switch to using these two 
> halves afterwards.
> 
> 3) Follow the code from MSIOF, consult the "cs" gpios property, manually 
> walk the CS GPIOs, and find out a free HW CS.

4)  Try to determine the HW CS to use at spi_setup() time.  This way
the master should know about all _probed_ spi slaves already.  It does
not work if the kernel does not know about a device that is attached to
a HW CS.

> Looking further, it seems that the CS GPIO signals are first manipulated 
> when probing for the corresponding client device. Depending on a platform 
> and its choice of a default value for the corresponding GPIO pin, this 
> might mean that a GPIO CS only gets initialized to inactive ("high" in 
> default SPI settings) too late. Other devices might have been already 
> probed for, and the CS GPIO of the "next" device might have been held 
> active ("low") during that time. How should I fix that?

Design the hardware so the GPIOs are high impedance out of reset and
put a weak pullup on each chip select line.

If you look at spi-imx or spi-dw-mmio, they request all GPIOs at the
time the master is probed.  They should probably also set them to de-
asserted outputs at this time too.

Other drivers, like spi-sirf, request the gpio in spi_setup(), which is
IMHO flawed and they should not do that.


��.n��������+%������w��{.n�����{����)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥