On 11/11/24 11:14 AM, David Lechner wrote:
> 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.
Just realized I said exactly what you said! Will fix this.
>
>>
>>
>>> + 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);
>
