Topics

[RFC PATCH 0/6] Refactor device configuration


Vlad Dogaru <vlad.dogaru@...>
 

Hi everyone,

I'd like to propose we revisit the way that devices are configured in
Zephyr. This is very RFC, please poke holes wherever you see fit.

Currently, most drivers define one or more device instances which can be
activated and configured from Kconfig. This has several drawbacks:

- It limits the number of devices which can be defined. If you have 3
DW I2C controllers, you need to add one more set of config variables
and structures.

- It makes interaction with external tools difficult. We are working
on a hardware configuration tool that will enable the user to choose
components from a catalog and generate a Zephyr config based on that.
We can generate Kconfig, but it's not a very robust approach. Given
an I2C device, its master may be called CONFIG_MYDEV_I2C_BUS,
CONFIG_MYDEV_0_I2C_MASTER or anything else.

- It makes the config files verbose.

In the new model, device definition is done exclusively in C. Some
macros are provided to make interacting with outside tools easier.
Driver authors will be required to use these macros in order to ensure
compatibility.

We keep from existing drivers the idea that devices are defined by a
struct device and an associated config, but remove the ability to
configure said devices via Kconfig. This enables us to define an
arbitrary number of devices. We also better control the way that
configurations are described through wrapper macros.

I chose a sensor driver to convert to the proposed API because it is
what I am most familiar with, but this is not limited to sensors.

Patches 1-4 introduce the needed wrappers and some fixes. Patch 5 is
the actual driver conversion. The diffstat is pretty consistent, but
this is because we are doing both Kconfig removal _and_ multiple device
support in a single patch. I tried to split this up, but ended with
largely unusable intermediate results. I believe that the final trigger
code is easier to read as it relies on C rather than preprocessor to
make decisions.

Patch 6 converts the MCP9808 sample to the new device description
method. It showcases how users can write their own C file, which
contains configuration structures. This file will ideally be generated
by a more user friendly hardware configuration tool. Only power users
should edit these files by hand, and even then each driver would provide
a sample.

These application-specific hardware description bits will be
supplemented by platform-specific ones. The Zephyr tree will include
these files, similarly to how Kconfig defaults are now defined for
boards. So, instead of a board config defining CONFIG_I2C_DW_0_BASE_ADDR
and friends, we would have a struct i2c_dw_config in a C file.

The wrapper macros ensure that driver writers will follow the
convention for their own device config structures. Of course, complex
devices will also need to add custom parameters to their config struct.
How the hardware configuration tool will handle these is an open
question.

The series uses a (by now stale, I think) version of Tomasz's GPIO
multi-callback patches, which I needed to test that triggers work for
both MCP9808 devices. It also lacks a good deal of Doxygen comments,
which I will provide for the final version. For now I hope to gather
feedback on the result, which is devices.c in patch 6/6.



Vlad Dogaru (6):
sensor: make delayed work visible in menuconfig
sensor: add device config helpers
i2c: add device config helpers
gpio: add device config helpers
sensor: mcp9808: support multiple devices
samples: mcp9808: support two devices

drivers/sensor/Kconfig | 3 +-
drivers/sensor/Kconfig.mcp9808 | 78 ++---------------------------
drivers/sensor/sample.c | 32 ++++++++++++
drivers/sensor/sensor_mcp9808.c | 12 ++---
drivers/sensor/sensor_mcp9808.h | 29 +++++++----
drivers/sensor/sensor_mcp9808_trigger.c | 89 ++++++++++++++++-----------------
include/gpio.h | 30 +++++++++++
include/i2c.h | 17 +++++++
include/sensor.h | 77 +++++++++++++++++++++++-----
samples/sensor/mcp9808/Makefile | 3 +-
samples/sensor/mcp9808/prj.conf | 4 +-
samples/sensor/mcp9808/src/Makefile | 2 +-
samples/sensor/mcp9808/src/devices.c | 30 +++++++++++
samples/sensor/mcp9808/src/main.c | 66 +++++++++++++++---------
14 files changed, 294 insertions(+), 178 deletions(-)
create mode 100644 drivers/sensor/sample.c
create mode 100644 samples/sensor/mcp9808/src/devices.c

--
1.9.1


Vlad Dogaru <vlad.dogaru@...>
 

On Fri, Apr 22, 2016 at 01:15:57AM +0300, Vlad Dogaru wrote:
Hi everyone,

I'd like to propose we revisit the way that devices are configured in
Zephyr. This is very RFC, please poke holes wherever you see fit.
You can also find the patches on Gerrit, under the dev_config topic:
https://gerrit.zephyrproject.org/r/#/q/topic:dev_config

Currently, most drivers define one or more device instances which can be
activated and configured from Kconfig. This has several drawbacks:

- It limits the number of devices which can be defined. If you have 3
DW I2C controllers, you need to add one more set of config variables
and structures.

- It makes interaction with external tools difficult. We are working
on a hardware configuration tool that will enable the user to choose
components from a catalog and generate a Zephyr config based on that.
We can generate Kconfig, but it's not a very robust approach. Given
an I2C device, its master may be called CONFIG_MYDEV_I2C_BUS,
CONFIG_MYDEV_0_I2C_MASTER or anything else.

- It makes the config files verbose.

In the new model, device definition is done exclusively in C. Some
macros are provided to make interacting with outside tools easier.
Driver authors will be required to use these macros in order to ensure
compatibility.

We keep from existing drivers the idea that devices are defined by a
struct device and an associated config, but remove the ability to
configure said devices via Kconfig. This enables us to define an
arbitrary number of devices. We also better control the way that
configurations are described through wrapper macros.

I chose a sensor driver to convert to the proposed API because it is
what I am most familiar with, but this is not limited to sensors.

Patches 1-4 introduce the needed wrappers and some fixes. Patch 5 is
the actual driver conversion. The diffstat is pretty consistent, but
this is because we are doing both Kconfig removal _and_ multiple device
support in a single patch. I tried to split this up, but ended with
largely unusable intermediate results. I believe that the final trigger
code is easier to read as it relies on C rather than preprocessor to
make decisions.

Patch 6 converts the MCP9808 sample to the new device description
method. It showcases how users can write their own C file, which
contains configuration structures. This file will ideally be generated
by a more user friendly hardware configuration tool. Only power users
should edit these files by hand, and even then each driver would provide
a sample.

These application-specific hardware description bits will be
supplemented by platform-specific ones. The Zephyr tree will include
these files, similarly to how Kconfig defaults are now defined for
boards. So, instead of a board config defining CONFIG_I2C_DW_0_BASE_ADDR
and friends, we would have a struct i2c_dw_config in a C file.

The wrapper macros ensure that driver writers will follow the
convention for their own device config structures. Of course, complex
devices will also need to add custom parameters to their config struct.
How the hardware configuration tool will handle these is an open
question.

The series uses a (by now stale, I think) version of Tomasz's GPIO
multi-callback patches, which I needed to test that triggers work for
both MCP9808 devices. It also lacks a good deal of Doxygen comments,
which I will provide for the final version. For now I hope to gather
feedback on the result, which is devices.c in patch 6/6.



Vlad Dogaru (6):
sensor: make delayed work visible in menuconfig
sensor: add device config helpers
i2c: add device config helpers
gpio: add device config helpers
sensor: mcp9808: support multiple devices
samples: mcp9808: support two devices

drivers/sensor/Kconfig | 3 +-
drivers/sensor/Kconfig.mcp9808 | 78 ++---------------------------
drivers/sensor/sample.c | 32 ++++++++++++
drivers/sensor/sensor_mcp9808.c | 12 ++---
drivers/sensor/sensor_mcp9808.h | 29 +++++++----
drivers/sensor/sensor_mcp9808_trigger.c | 89 ++++++++++++++++-----------------
include/gpio.h | 30 +++++++++++
include/i2c.h | 17 +++++++
include/sensor.h | 77 +++++++++++++++++++++++-----
samples/sensor/mcp9808/Makefile | 3 +-
samples/sensor/mcp9808/prj.conf | 4 +-
samples/sensor/mcp9808/src/Makefile | 2 +-
samples/sensor/mcp9808/src/devices.c | 30 +++++++++++
samples/sensor/mcp9808/src/main.c | 66 +++++++++++++++---------
14 files changed, 294 insertions(+), 178 deletions(-)
create mode 100644 drivers/sensor/sample.c
create mode 100644 samples/sensor/mcp9808/src/devices.c

--
1.9.1


Tomasz Bursztyka
 

Hi Vlad,
Hi everyone,

I'd like to propose we revisit the way that devices are configured in
Zephyr. This is very RFC, please poke holes wherever you see fit.

Currently, most drivers define one or more device instances which can be
activated and configured from Kconfig. This has several drawbacks:

- It limits the number of devices which can be defined. If you have 3
DW I2C controllers, you need to add one more set of config variables
and structures.

- It makes interaction with external tools difficult. We are working
on a hardware configuration tool that will enable the user to choose
components from a catalog and generate a Zephyr config based on that.
I think you will need to clarify that before anything. I guess we are
all curious to see what it
will look like and how much it's affecting Zephyr. You are, afaik in
your patches, moving away
from Kconfig as we used to do. I am not saying it's good or bad, I just
lack the final goal: this tool
your are talking about. Would be great to see what it is all about.

We can generate Kconfig, but it's not a very robust approach. Given
an I2C device, its master may be called CONFIG_MYDEV_I2C_BUS,
CONFIG_MYDEV_0_I2C_MASTER or anything else.

Patches 1-4 introduce the needed wrappers and some fixes.
Fixes can go already to gerrit I think.

Tomasz