[RFC] NULL callbacks in driver API


Maciek Borzecki <maciek.borzecki@...>
 

Hi,

I'd like to make a proposal for enhancement of driver API
callbacks, but before jumping to code I'd like to get some feedback first.

Right now, whenever a call to an API is made, we directly jump to call
the driver's implementation. This is ok for the general case, when we
know that every driver will implement all callbacks.

However, this approach has some downsides in my opinion and might
become an issue once the number of drivers grows or the APIs become
larger.

First of all, in practice, not every driver will be able to implement all
callbacks. It is my understanding that the API will cover only a subset
of common functionality. However, as an example, the PWM driver for
STM32 may not have an applicable method of setting phase. In such case,
the driver would provide a dummy callback returning DEV_NO_SUPPORT.

Secondly, the current approach requires that whenever an API is
augmented with a new callback, someone must track down all driver
implementations and add this dummy callback.

Once the errno patches hit master, we'll need to make sure that
dummy callbacks (if present) return proper error code.

Given that, I'm proposing to add checks whether the driver implements
given callback in the public API calls. This has some benefits. Firstly,
we'll always have a default code path. Someone working on a driver
does not have add a dummy implementation.

Secondly, this will help to enforce consistent return values for a
case when a device lacks certain feature/functionality. Lack of
callback may return DEV_NO_SUPPORT, or ENOSYS/ENOTSUP.

We can rely on static *_driver_api struct allocation, and just fill in
the blanks. This is important when adding new API calls. IIRC Linux
does that a lot.

Lastly, I believe that this will give a minimal size reduction if one
happens to use drivers with dummy callbacks.

An obligatory example:

<---- code --------->
struct pinmux_driver_api {
pmux_set set;
pmux_get get;
pmux_pullup pullup;
pmux_input input;
};
...

static inline uint32_t pinmux_pin_input_enable(struct device *dev,
uint32_t pin,
uint8_t func)
{
struct pinmux_driver_api *api;

api = (struct pinmux_driver_api *) dev->driver_api;

/* proposed enhacement */
if (!api->input) {
return DEV_NO_SUPPORT;
}

return api->input(dev, pin, func);
}
<------ code ------------->

A similar thing is also a part of clock_control API review:
https://gerrit.zephyrproject.org/r/#/c/712/

Cheers,
--
Maciek Borzecki

Join devel@lists.zephyrproject.org to automatically receive all group messages.