Re: Deadlock in spi_add_device() -- device core problem

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

Hi Mark,

On Fri, Oct 08, 2021 at 02:31:57PM +0100, Mark Brown wrote:
> On Thu, Oct 07, 2021 at 06:52:14PM +0200, Uwe Kleine-König wrote:
> > On Thu, Oct 07, 2021 at 06:19:46PM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Oct 07, 2021 at 06:05:24PM +0200, Uwe Kleine-König wrote:
> 
> > > Drivers for a bus bind to the bus, they should not be creating new
> > > devices for that same bus, as that does not seem correct.
> 
> > That's not the culprit here. We have a spi-device (spi-mux) that is a
> > bus device on the SoC's bus and a bus master for it's own bus. And
> > spi_add_device for the spi-mux device triggers the probe function for
> > the spi-mux driver which creates a new bus controller which triggers
> > spi_add_device() for the devices on the inner bus.
> 
> I think we need to be arranging for spi_add_lock to be per bus
> rather than global - putting it into the controller ought to do
> the trick.

Yeah, that's what I consider the second best option that I already
mentioned in the initial mail of this thread.

@Greg: Would you expect that it should be possible (and benificial) to
rework the code to not hold a lock at all during device_add()?

This would then need something like:

	lock() # either per controller or global
	if bus already has a device for the same chipselect:
	    error out;
	register chipselect as busy
	unlock();

	ret = device_add(...)

	if ret:
	    lock()
	    unregister chipselect
	    unlock()

Is this worth the effort?

Anyhow, will try the suggested patch next.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]