Skip to content

Commit 5e844cc

Browse files
l1kbroonie
authored andcommitted
spi: Introduce device-managed SPI controller allocation
SPI driver probing currently comprises two steps, whereas removal comprises only one step: spi_alloc_master() spi_register_controller() spi_unregister_controller() That's because spi_unregister_controller() calls device_unregister() instead of device_del(), thereby releasing the reference on the spi_controller which was obtained by spi_alloc_master(). An SPI driver's private data is contained in the same memory allocation as the spi_controller struct. Thus, once spi_unregister_controller() has been called, the private data is inaccessible. But some drivers need to access it after spi_unregister_controller() to perform further teardown steps. Introduce devm_spi_alloc_master() and devm_spi_alloc_slave(), which release a reference on the spi_controller struct only after the driver has unbound, thereby keeping the memory allocation accessible. Change spi_unregister_controller() to not release a reference if the spi_controller was allocated by one of these new devm functions. The present commit is small enough to be backportable to stable. It allows fixing drivers which use the private data in their ->remove() hook after it's been freed. It also allows fixing drivers which neglect to release a reference on the spi_controller in the probe error path. Long-term, most SPI drivers shall be moved over to the devm functions introduced herein. The few that can't shall be changed in a treewide commit to explicitly release the last reference on the controller. That commit shall amend spi_unregister_controller() to no longer release a reference, thereby completing the migration. As a result, the behaviour will be less surprising and more consistent with subsystems such as IIO, which also includes the private data in the allocation of the generic iio_dev struct, but calls device_del() in iio_device_unregister(). Signed-off-by: Lukas Wunner <lukas@wunner.de> Link: https://lore.kernel.org/r/272bae2ef08abd21388c98e23729886663d19192.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown <broonie@kernel.org>
1 parent ee4ad5d commit 5e844cc

2 files changed

Lines changed: 76 additions & 1 deletion

File tree

drivers/spi/spi.c

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2442,6 +2442,49 @@ struct spi_controller *__spi_alloc_controller(struct device *dev,
24422442
}
24432443
EXPORT_SYMBOL_GPL(__spi_alloc_controller);
24442444

2445+
static void devm_spi_release_controller(struct device *dev, void *ctlr)
2446+
{
2447+
spi_controller_put(*(struct spi_controller **)ctlr);
2448+
}
2449+
2450+
/**
2451+
* __devm_spi_alloc_controller - resource-managed __spi_alloc_controller()
2452+
* @dev: physical device of SPI controller
2453+
* @size: how much zeroed driver-private data to allocate
2454+
* @slave: whether to allocate an SPI master (false) or SPI slave (true)
2455+
* Context: can sleep
2456+
*
2457+
* Allocate an SPI controller and automatically release a reference on it
2458+
* when @dev is unbound from its driver. Drivers are thus relieved from
2459+
* having to call spi_controller_put().
2460+
*
2461+
* The arguments to this function are identical to __spi_alloc_controller().
2462+
*
2463+
* Return: the SPI controller structure on success, else NULL.
2464+
*/
2465+
struct spi_controller *__devm_spi_alloc_controller(struct device *dev,
2466+
unsigned int size,
2467+
bool slave)
2468+
{
2469+
struct spi_controller **ptr, *ctlr;
2470+
2471+
ptr = devres_alloc(devm_spi_release_controller, sizeof(*ptr),
2472+
GFP_KERNEL);
2473+
if (!ptr)
2474+
return NULL;
2475+
2476+
ctlr = __spi_alloc_controller(dev, size, slave);
2477+
if (ctlr) {
2478+
*ptr = ctlr;
2479+
devres_add(dev, ptr);
2480+
} else {
2481+
devres_free(ptr);
2482+
}
2483+
2484+
return ctlr;
2485+
}
2486+
EXPORT_SYMBOL_GPL(__devm_spi_alloc_controller);
2487+
24452488
#ifdef CONFIG_OF
24462489
static int of_spi_get_gpio_numbers(struct spi_controller *ctlr)
24472490
{
@@ -2778,6 +2821,11 @@ int devm_spi_register_controller(struct device *dev,
27782821
}
27792822
EXPORT_SYMBOL_GPL(devm_spi_register_controller);
27802823

2824+
static int devm_spi_match_controller(struct device *dev, void *res, void *ctlr)
2825+
{
2826+
return *(struct spi_controller **)res == ctlr;
2827+
}
2828+
27812829
static int __unregister(struct device *dev, void *null)
27822830
{
27832831
spi_unregister_device(to_spi_device(dev));
@@ -2819,7 +2867,15 @@ void spi_unregister_controller(struct spi_controller *ctlr)
28192867
list_del(&ctlr->list);
28202868
mutex_unlock(&board_lock);
28212869

2822-
device_unregister(&ctlr->dev);
2870+
device_del(&ctlr->dev);
2871+
2872+
/* Release the last reference on the controller if its driver
2873+
* has not yet been converted to devm_spi_alloc_master/slave().
2874+
*/
2875+
if (!devres_find(ctlr->dev.parent, devm_spi_release_controller,
2876+
devm_spi_match_controller, ctlr))
2877+
put_device(&ctlr->dev);
2878+
28232879
/* free bus id */
28242880
mutex_lock(&board_lock);
28252881
if (found == ctlr)

include/linux/spi/spi.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,25 @@ static inline struct spi_controller *spi_alloc_slave(struct device *host,
734734
return __spi_alloc_controller(host, size, true);
735735
}
736736

737+
struct spi_controller *__devm_spi_alloc_controller(struct device *dev,
738+
unsigned int size,
739+
bool slave);
740+
741+
static inline struct spi_controller *devm_spi_alloc_master(struct device *dev,
742+
unsigned int size)
743+
{
744+
return __devm_spi_alloc_controller(dev, size, false);
745+
}
746+
747+
static inline struct spi_controller *devm_spi_alloc_slave(struct device *dev,
748+
unsigned int size)
749+
{
750+
if (!IS_ENABLED(CONFIG_SPI_SLAVE))
751+
return NULL;
752+
753+
return __devm_spi_alloc_controller(dev, size, true);
754+
}
755+
737756
extern int spi_register_controller(struct spi_controller *ctlr);
738757
extern int devm_spi_register_controller(struct device *dev,
739758
struct spi_controller *ctlr);

0 commit comments

Comments
 (0)