Driver API implementation


Marcus Shawcroft <marcus.shawcroft@...>
 

Hi,

While refactoring the device driver 'config_info' implementation(s)
such that config data can be const and romable I notice a similar
issue/opportunity w.r.t the driver API mechanism.

Currently we have:

struct device {
...
void *driver_api;
};

This pointer is used to reference a device class specific struct of
function pointers that collectively implement the upward facing API to
each driver instance. Every driver (almost) provides such a
structure.

This superficially looks to me like the kind of material we ought to
be pushing from RAM to ROM. Making *driver_api above const would
allow every driver to define its API structure as const.

Is this non const design deliberate, or an oversight? Does anyone have
objections to making the driver API machinery const, or see benefits
that out weigh the benefit of making these structures romable ?

Cheers
/Marcus


Boie, Andrew P
 

On Fri, 2016-10-21 at 09:48 +0100, Marcus Shawcroft wrote:
Is this non const design deliberate, or an oversight? Does anyone have
objections to making the driver API machinery const, or see benefits
that out weigh the benefit of making these structures romable ?
I think this would be a good change. I can't think of use-cases where the API
structs need to be mutable at runtime.

As a side note, one drawback of the API struct approach in general is that it
confounds gc-sections. If a particular API is never used, the linker can't
figure this out as there is always at least one reference to the API -- the API
struct. I dont know of good alternatives however.

Andrew


Liu, Baohong
 

-----Original Message-----
From: Marcus Shawcroft [mailto:marcus.shawcroft(a)gmail.com]
Sent: Friday, October 21, 2016 1:49 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] Driver API implementation

Hi,

While refactoring the device driver 'config_info' implementation(s) such that
config data can be const and romable I notice a similar issue/opportunity
w.r.t the driver API mechanism.

Currently we have:

struct device {
...
void *driver_api;
};

This pointer is used to reference a device class specific struct of function
pointers that collectively implement the upward facing API to each driver
instance. Every driver (almost) provides such a structure.

This superficially looks to me like the kind of material we ought to be pushing
from RAM to ROM. Making *driver_api above const would allow every
driver to define its API structure as const.

Is this non const design deliberate, or an oversight? Does anyone have
objections to making the driver API machinery const, or see benefits that out
weigh the benefit of making these structures romable ?
It used to be like the following.

During individual driver boot initialization, this assignment will be done after
the driver initialization code did all other things successfully. If driver initialization
fails for any reason, the pointer will be NULL. So, even if an app can get the device
binding successfully, it can discover the failure by checking the pointer against NULL.

I did not get involved in this part for quite a while, do not know whether this is
still the case.

Cheers
/Marcus


Liu, Baohong
 

-----Original Message-----
From: Liu, Baohong [mailto:baohong.liu(a)intel.com]
Sent: Friday, October 21, 2016 10:40 AM
To: devel(a)lists.zephyrproject.org; marcus.shawcroft(a)gmail.com
Subject: [devel] Re: Driver API implementation

-----Original Message-----
From: Marcus Shawcroft [mailto:marcus.shawcroft(a)gmail.com]
Sent: Friday, October 21, 2016 1:49 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] Driver API implementation

Hi,

While refactoring the device driver 'config_info' implementation(s)
such that config data can be const and romable I notice a similar
issue/opportunity w.r.t the driver API mechanism.

Currently we have:

struct device {
...
void *driver_api;
};

This pointer is used to reference a device class specific struct of
function pointers that collectively implement the upward facing API to
each driver instance. Every driver (almost) provides such a structure.

This superficially looks to me like the kind of material we ought to
be pushing from RAM to ROM. Making *driver_api above const would
allow every driver to define its API structure as const.

Is this non const design deliberate, or an oversight? Does anyone have
objections to making the driver API machinery const, or see benefits
that out weigh the benefit of making these structures romable ?
It used to be like the following.

During individual driver boot initialization, this assignment will be done after
the driver initialization code did all other things successfully. If driver
initialization fails for any reason, the pointer will be NULL. So, even if an app
can get the device binding successfully, it can discover the failure by checking
Just found device binding is smarter now. It will check the pointer itself.
NULL pointer will make device binding fail.

the pointer against NULL.

I did not get involved in this part for quite a while, do not know whether this
is still the case.

Cheers
/Marcus


Thomas, Ramesh
 

On 10/21/2016 11:31 AM, Liu, Baohong wrote:


-----Original Message-----
From: Liu, Baohong [mailto:baohong.liu(a)intel.com]
Sent: Friday, October 21, 2016 10:40 AM
To: devel(a)lists.zephyrproject.org; marcus.shawcroft(a)gmail.com
Subject: [devel] Re: Driver API implementation

-----Original Message-----
From: Marcus Shawcroft [mailto:marcus.shawcroft(a)gmail.com]
Sent: Friday, October 21, 2016 1:49 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] Driver API implementation

Hi,

While refactoring the device driver 'config_info' implementation(s)
such that config data can be const and romable I notice a similar
issue/opportunity w.r.t the driver API mechanism.

Currently we have:

struct device {
...
void *driver_api;
};

This pointer is used to reference a device class specific struct of
function pointers that collectively implement the upward facing API to
each driver instance. Every driver (almost) provides such a structure.

This superficially looks to me like the kind of material we ought to
be pushing from RAM to ROM. Making *driver_api above const would
allow every driver to define its API structure as const.

Is this non const design deliberate, or an oversight? Does anyone have
objections to making the driver API machinery const, or see benefits
that out weigh the benefit of making these structures romable ?
It used to be like the following.

During individual driver boot initialization, this assignment will be done after
the driver initialization code did all other things successfully. If driver
initialization fails for any reason, the pointer will be NULL. So, even if an app
can get the device binding successfully, it can discover the failure by checking
Just found device binding is smarter now. It will check the pointer itself.
NULL pointer will make device binding fail.
Looks like this only serves as a debug of init failures. We can instead
use ASSERT for that purpose.

In my opinion, if there is no other valid use case for this, we should
remove the check and move the APIs to ROM.