On 10/26/24 10:05 AM, Jonathan Cameron wrote:
> On Wed, 23 Oct 2024 15:59:09 -0500
> David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
...
>> +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);
>
> Why let this return an offload that is already in use?
> Maybe make that a problem for the spi controller
> Seems odd to pass it spi then set it later.
>
> I.e. have this return ERR_PTR(-EBUSY);
I would expect that to effectively be handled by the
if (IS_ERR(offload)) below. Only the controller can
know which offloads are already in use, so the callback
should return the appropriate -EBUSY in that case.
>
>
>> + if (IS_ERR(offload))
>> + return offload;
>> +
>> + if (offload->spi)
>> + return ERR_PTR(-EBUSY);
>> +
>> + offload->spi = spi;
>> + get_device(offload->provider_dev);
>> +
>> + ret = devm_add_action_or_reset(dev, spi_offload_put, offload);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + return offload;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_spi_offload_get);
