Re: cs-gpios and request_gpio

[Trent Piepho <tpiepho@xxxxxxxxxx>]

On Wed, 2018-08-15 at 10:22 -0400, Jonah Petri wrote:
> > From Trent Piepho <tpiepho () impinj ! com>:
> > The SPI drivers do it.
> > If you look in the spi master driver's probe function in the most
> > up-
> > to-date drivers, there is usually a loop over every cs-gpio that
> > does a
> > devm_gpio_request_one.
> > 
> 
> What's the proper ordering of driver behavior here?  Currently,
> spi_register_master() results in gpio activity before the GPIOs are
> reserved by the core.  The stack trace looks like:
> 
> gpio_set_value
> spi_set_cs
> spi_setup
> spi_add_device
> of_register_spi_device 
> of_register_spi_devices
> spi_register_master
> 
> Where is the spi driver supposed to get in there and reserve the
> GPIOs?  Should the driver be iterating the devtree and doing a
> devm_reserve_gpio() on each of the cs-gpios before calling
> spi_register_master (or spi_bitbang_start, which itself calls
> spi_register_master)?

I think the underlying framework in the spi core is flawed here. The
GPIOs are found in spi_register_controller(), when it calls
of_spi_register_master().  But this same function also makes the spi
controller go live.

Perhaps the parsing of the cs-gpios property could be done in
spi_alloc_controller()?

Or allow spi_register_controller() to return with the controller's io
mutex locked, so that the caller can unlock it when ready.  I think the
former would be better than that.

> The spi-imx driver cs handling seems broken, at least ilocation and
> registration step.n 4.9 where I live, and code inspection suggests in
> 4.18 as well.  The gpio is configured as an output via spi_setup-
> >spi_imx_setup->gpio_direction_output, but then that setting can be
> trampled via the later call to devm_gpio_request in the driver's
> probe method.

I think the problem is the code in the probe method was intended to run
to completion before the master went live and there were any calls to
the master's methods, but it doesn't work that way.