On 11/24/24 10:32 AM, Jonathan Cameron wrote:
> On Fri, 15 Nov 2024 14:18:40 -0600
> David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
>> Add the basic infrastructure to support SPI offload providers and
>> consumers.
>>
...
>> + resource = kzalloc(sizeof(*resource), GFP_KERNEL);
>> + if (!resource)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + resource->controller = spi->controller;
>> + resource->offload = spi->controller->get_offload(spi, config);
>> + ret = PTR_ERR_OR_ZERO(resource->offload);
>> + if (ret) {
> Why not simply
> if (IS_ERR(resource->offload) {
> kfree(resource);
> return resource->offload;
> }
>> + kfree(resource);
>> + return ERR_PTR(ret);
>> + }
Hmm... maybe somewhere along the way ret was being checked again
after this, but doesn't to be the case anymore.
>> +
>> + ret = devm_add_action_or_reset(dev, spi_offload_put, resource);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + return resource->offload;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_spi_offload_get);
>
>> diff --git a/include/linux/spi/spi-offload.h b/include/linux/spi/spi-offload.h
>> new file mode 100644
>> index 000000000000..81b115fc89bf
>> --- /dev/null
>> +++ b/include/linux/spi/spi-offload.h
>
>> +
>> +MODULE_IMPORT_NS(SPI_OFFLOAD);
>
> This is rarely done in headers. (only pwm.h does it I think)
> I'd push it down into code that uses this.
Yes, it was Uwe that suggested that I put it in the header. :-)
Are there any unwanted side effects of having it in the header?
>
> It might be worth splitting the header into a spi-offload-provider.h
> and spi-offload-consumer.h with a common spi-offload-types.h included
> by both.
>
