2021-04-21 10:30:57

by Charles Keepax

[permalink] [raw]
Subject: [PATCH] spi: Make of_register_spi_device also set the fwnode

Currently, the SPI core doesn't set the struct device fwnode pointer
when it creates a new SPI device. This means when the device is
registered the fwnode is NULL and the check in device_add which sets
the fwnode->dev pointer is skipped. This wasn't previously an issue,
however these two patches:

commit 4731210c09f5 ("gpiolib: Bind gpio_device to a driver to enable
fw_devlink=on by default")
commit ced2af419528 ("gpiolib: Don't probe gpio_device if it's not the
primary device")

Added some code to the GPIO core which relies on using that
fwnode->dev pointer to determine if a driver is bound to the fwnode
and if not bind a stub GPIO driver. This means the GPIO providers
behind SPI will get both the expected driver and this stub driver
causing the stub driver to fail if it attempts to request any pin
configuration. For example on my system:

madera-pinctrl madera-pinctrl: pin gpio5 already requested by madera-pinctrl; cannot claim for gpiochip3
madera-pinctrl madera-pinctrl: pin-4 (gpiochip3) status -22
madera-pinctrl madera-pinctrl: could not request pin 4 (gpio5) from group aif1 on device madera-pinctrl
gpio_stub_drv gpiochip3: Error applying setting, reverse things back
gpio_stub_drv: probe of gpiochip3 failed with error -22

The firmware node on the device created by the GPIO framework is set
through the of_node pointer hence things generally actually work,
however that fwnode->dev is never set, as the check was skipped at
device_add time. This fix appears to match how the I2C subsystem
handles the same situation.

Signed-off-by: Charles Keepax <[email protected]>
---
drivers/spi/spi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index cc6d43d7895f0..350c109f7547d 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2038,6 +2038,7 @@ of_register_spi_device(struct spi_controller *ctlr, struct device_node *nc)
/* Store a pointer to the node in the device structure */
of_node_get(nc);
spi->dev.of_node = nc;
+ spi->dev.fwnode = of_fwnode_handle(nc);

/* Register the new device */
rc = spi_add_device(spi);
--
2.11.0


2021-04-22 01:19:34

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] spi: Make of_register_spi_device also set the fwnode

On Wed, 21 Apr 2021 11:14:02 +0100, Charles Keepax wrote:
> Currently, the SPI core doesn't set the struct device fwnode pointer
> when it creates a new SPI device. This means when the device is
> registered the fwnode is NULL and the check in device_add which sets
> the fwnode->dev pointer is skipped. This wasn't previously an issue,
> however these two patches:
>
> commit 4731210c09f5 ("gpiolib: Bind gpio_device to a driver to enable
> fw_devlink=on by default")
> commit ced2af419528 ("gpiolib: Don't probe gpio_device if it's not the
> primary device")
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: Make of_register_spi_device also set the fwnode
commit: 0e793ba77c18382f08e440260fe72bc6fce2a3cb

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark