On 10/24/24 8:27 AM, Nuno Sá wrote: > On Wed, 2024-10-23 at 15:59 -0500, David Lechner wrote: >> Add the basic infrastructure to support SPI offload providers and >> consumers. >> ... >> +struct spi_offload *devm_spi_offload_get(struct device *dev, >> + struct spi_device *spi, >> + const struct spi_offload_config *config) >> +{ >> + struct spi_offload *offload; >> + int ret; >> + >> + if (!spi || !config) >> + return ERR_PTR(-EINVAL); >> + >> + if (!spi->controller->get_offload) >> + return ERR_PTR(-ENODEV); >> + >> + offload = spi->controller->get_offload(spi, config); >> + if (IS_ERR(offload)) >> + return offload; >> + >> + if (offload->spi) >> + return ERR_PTR(-EBUSY); >> + >> + offload->spi = spi; >> + get_device(offload->provider_dev); > > Isn't this redundant? From what I can tell, we're assuming that the spi controller > (of the spi device) is the offload provider. Therefore, getting an extra reference > for it does not really seems necessary. The device cannot go away without under the > spi_device feet. If that could happen, then we would also need to take care about > callback access and things like that. Going this way, it would also be arguable to > have a try_module_get(). > > - Nuno Sá > > Yes, you are right that we don't really need to take a reference to the device. This was left over from when I made an implementation that assumed the offload provider could be anything, not just a SPI controller.