Fwd: [PATCH 009/111] DRVMGR: added driver manager to cpukit/libdrvmgr

Gedare Bloom gedare at rtems.org
Tue Mar 3 21:58:38 UTC 2015


On Thu, Feb 26, 2015 at 11:38 AM, Daniel Hellstrom <daniel at gaisler.com> wrote:
[...]
> diff --git a/c/src/aclocal/enable-drvmgr.m4 b/c/src/aclocal/enable-drvmgr.m4
> new file mode 100644
> index 0000000..489f60e
> --- /dev/null
> +++ b/c/src/aclocal/enable-drvmgr.m4
> @@ -0,0 +1,12 @@
> +AC_DEFUN([RTEMS_ENABLE_DRVMGR],
> +[
> +## AC_BEFORE([$0], [RTEMS_CHECK_DRVMGR_STARTUP])dnl
> +
> +AC_ARG_ENABLE(drvmgr,
> +[AS_HELP_STRING([--enable-drvmgr],[enable Driver Manager at Startup])],
> +[case "${enableval}" in
> +  yes) RTEMS_DRVMGR_STARTUP=yes ;;
> +  no) RTEMS_DRVMGR_STARTUP=no ;;
> +  *)  AC_MSG_ERROR(bad value ${enableval} for enable-drvmgr option) ;;
> +esac],[RTEMS_DRVMGR_STARTUP=yes])
> +])
Is a configure-time option the best solution for this? The more
'switches' we have, the harder it is to test RTEMS.

> diff --git a/c/src/lib/libbsp/shared/bspdriverlevelhook.c b/c/src/lib/libbsp/shared/bspdriverlevelhook.c
> new file mode 100644
> index 0000000..93406f9
> --- /dev/null
> +++ b/c/src/lib/libbsp/shared/bspdriverlevelhook.c
> @@ -0,0 +1,16 @@
> +/*
> + *  This is a dummy bsp_driver_level_hook routine.
> + *
> + *  COPYRIGHT (c) 2015.
> + *  Cobham Gaisler.
> + *
> + *  The license and distribution terms for this file may be
> + *  found in the file LICENSE in this distribution or at
> + *  http://www.rtems.org/license/LICENSE.
> + */
> +
> +#include <bsp/bootcard.h>
> +
> +void bsp_driver_level_hook( int level )
> +{
> +}
We'll need some documentation added to the BSP guide on this.

[...]

> diff --git a/cpukit/libdrvmgr/README b/cpukit/libdrvmgr/README
> new file mode 100644
> index 0000000..6e55370
> --- /dev/null
> +++ b/cpukit/libdrvmgr/README
> @@ -0,0 +1,112 @@
> +DRIVER MANAGER
> +==============
> +
> +See documentation in Aeroflex Gaisler Driver manual.
Should either have a link to a hosted version somewhere or we should
provide the document in our manuals.

> +
> +
> +INITIALIZATION
> +==============
> +The Driver Manager can be intialized in two different ways:
> + 1. during RTEMS startup
> + 2. started by user, typically in the Init task
> +
> +The driver manager is initalized during RTEMS startup in the
> +rtems_initialize_device_drivers() function when RTEMS is
> +configured with driver manager support.
> +
> +When RTEMS is not configured with the driver manager, the manager
> +may still be initialized by the user after system startup, typically
> +from the Init() task.
> +
> +The main difference between the two ways is when interrupt
> +is enabled. Interrupt is enabled for the first time by RTEMS when
> +the Init task is started. This means, for the first case, that
> +drivers can not use interrupt services until after the
> +initialization phase is over and the user request services from
> +the drivers. For the second case of initialization, this means
> +that driver must take extra care during initalization when interrupt
s/initalization/initialization

> +is enabled so that spurious interrupts are not generated and that the
> +system does not hang in an infinite IRQ loop.
> +
> +Most of the problems above are solved for the two methods by
> +specifying in which initialization levels IRQ handling is done.
> +See Level 1 and Level 2 below.
> +
> +Other differences is that IRQ, System Clock Timer, debug Console
> +and Console can be initalized by the help of the driver manager
s/initalized/initialized

> +when initialized during start up. Between Level0 and Level1 the
> +RTEMS I/O Manager drivers are initialized. The LEON3 BSP has
> +therefore two different versions of the basic drivers.
> +
What is the advantage to having the two different approaches? I don't
clearly see, and it obviously complicated the initialization code, and
seems to have led to two versions of the same drivers.

[...]

> diff --git a/cpukit/libdrvmgr/drvmgr.c b/cpukit/libdrvmgr/drvmgr.c
> new file mode 100644
> index 0000000..0471865
> --- /dev/null
> +++ b/cpukit/libdrvmgr/drvmgr.c
> @@ -0,0 +1,643 @@
> +/* Driver Manager Interface Implementation.
> + *
> + * COPYRIGHT (c) 2009.
> + * Cobham Gaisler AB.
> + *
> + * The license and distribution terms for this file may be
> + * found in the file LICENSE in this distribution or at
> + * http://www.rtems.com/license/LICENSE.
use rtems.org for the url. (it redirects there now.)

> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <drvmgr/drvmgr.h>
> +#include <drvmgr/drvmgr_confdefs.h>
> +
> +#include "drvmgr_internal.h"
> +
> +/* Enable debugging */
> +/*#define DEBUG 1*/
Consider inheriting the debug macro from --enable-rtems-debug

> +
> +#ifdef DEBUG
> +#define DBG(x...) printk(x)
> +#else
> +#define DBG(x...)
> +#endif
> +
> +struct rtems_driver_manager drv_mgr = {
This struct type (and name) should probably be part of the "drvmgr"
namespace? If only used in this file, the global var should be
declared 'static'.

> +       .level =                0,
> +       .initializing_objs =    0,
> +       .lock =                 0,
> +       .root_dev =             {0},
> +       .root_drv =             NULL,
> +
> +       .drivers =      LIST_INITIALIZER(struct drvmgr_drv, next),
> +
> +       .buses = {
> +               LIST_INITIALIZER(struct drvmgr_bus, next),
> +               LIST_INITIALIZER(struct drvmgr_bus, next),
> +               LIST_INITIALIZER(struct drvmgr_bus, next),
> +               LIST_INITIALIZER(struct drvmgr_bus, next),
> +               LIST_INITIALIZER(struct drvmgr_bus, next),
> +       },
> +       .buses_inactive =       LIST_INITIALIZER(struct drvmgr_bus, next),
> +
> +       .devices = {
> +               LIST_INITIALIZER(struct drvmgr_dev, next),
> +               LIST_INITIALIZER(struct drvmgr_dev, next),
> +               LIST_INITIALIZER(struct drvmgr_dev, next),
> +               LIST_INITIALIZER(struct drvmgr_dev, next),
> +               LIST_INITIALIZER(struct drvmgr_dev, next),
> +       },
> +       .devices_inactive =     LIST_INITIALIZER(struct drvmgr_dev, next),
> +};
> +
> +static int do_bus_init(
> +       struct rtems_driver_manager *mgr,
> +       struct drvmgr_bus *bus,
> +       int level);
> +static int do_dev_init(
> +       struct rtems_driver_manager *mgr,
> +       struct drvmgr_dev *dev,
> +       int level);
> +
> +/* DRIVER MANAGER */
> +
> +void _DRV_Manager_init_level(int level)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +
> +       if (mgr->level >= level)
> +               return;
> +
> +       /* Set new Level */
> +       mgr->level = level;
> +
> +       /* Initialize buses and devices into this new level */
> +       drvmgr_init_update();
> +}
> +
> +/* Initialize Data structures of the driver manager and call driver
> + * register functions configured by the user.
> + */
> +void _DRV_Manager_initialization(void)
> +{
> +       struct drvmgr_drv_reg_func *drvreg;
> +
> +       /* drv_mgr is already initialized statically by compiler except
> +        * the lock
> +        */
> +       DRVMGR_LOCK_INIT();
> +
> +       /* Call driver register functions. */
> +       drvreg = &drvmgr_drivers[0];
> +       while (drvreg->drv_reg) {
> +               /* Make driver register */
> +               drvreg->drv_reg();
> +               drvreg++;
> +       }
> +}
> +
> +/* Take ready devices and buses into the correct init level step by step.
> + * Once a bus or a device has been registered there is no turning
> + * back - they are taken to the level of the driver manager.
> + */
> +void drvmgr_init_update(void)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +       struct drvmgr_bus *bus;
> +       struct drvmgr_dev *dev;
> +       int bus_might_been_registered;
> +       int level;
> +
> +       /* "Lock" to make sure we don't use up the stack and that the lists
> +        * remain consistent.
> +        */
> +       DRVMGR_LOCK_WRITE();
> +       if (mgr->initializing_objs || (mgr->level == 0))
> +               goto out;
I'd rather see return than goto.

> +       mgr->initializing_objs = 1;
> +
> +init_registered_buses:
> +       /* Take all buses and devices ready into the same stage
> +        * as the driver manager global level.
> +        */
> +       for (level = 0; level < mgr->level; level++) {
> +
> +               bus_might_been_registered = 0;
> +
> +               /* Take buses into next level */
> +
> +               while ((bus = BUS_LIST_HEAD(&mgr->buses[level])) != NULL) {
> +
> +                       /* Remove first in the list (will be inserted in
> +                        * appropriate list by do_bus_init())
> +                        */
> +                       drvmgr_list_remove_head(&mgr->buses[level]);
> +
> +                       DRVMGR_UNLOCK();
> +
> +                       /* Initialize Bus, this will register devices on
> +                        * the bus. Take bus into next level.
> +                        */
> +                       do_bus_init(mgr, bus, level+1);
> +
> +                       DRVMGR_LOCK_WRITE();
> +               }
> +
> +               /* Take devices into next level */
> +               while ((dev = DEV_LIST_HEAD(&mgr->devices[level])) != NULL) {
> +
> +                       /* Always process first in list */
> +                       dev = DEV_LIST_HEAD(&mgr->devices[level]);
> +
> +                       /* Remove first in the list (will be inserted in
> +                        * appropriate list by do_dev_init())
> +                        */
> +                       drvmgr_list_remove_head(&mgr->devices[level]);
> +
> +                       DRVMGR_UNLOCK();
> +
> +                       /* Initialize Device, this may register a new bus */
> +                       do_dev_init(mgr, dev, level+1);
> +
> +                       DRVMGR_LOCK_WRITE();
> +
> +                       bus_might_been_registered = 1;
> +               }
> +
> +               /* Make sure all buses registered and ready are taken at
> +                * the same time into init level N.
> +                */
> +               if (bus_might_been_registered)
> +                       goto init_registered_buses;
how about level = 0 to restart the loop instead of a backward goto?

> +       }
> +
> +       /* Release bus/device initialization "Lock" */
> +       mgr->initializing_objs = 0;
> +
> +out:
> +       DRVMGR_UNLOCK();
> +}
> +
> +/* Take bus into next level */
> +static int do_bus_init(
> +       struct rtems_driver_manager *mgr,
> +       struct drvmgr_bus *bus,
> +       int level)
> +{
> +       int (*init)(struct drvmgr_bus *);
> +
> +       /* If bridge device has failed during initialization, the bus is not
> +        * initialized further.
> +        */
> +       if (bus->dev->state & DEV_STATE_INIT_FAILED) {
> +               bus->state |= BUS_STATE_DEPEND_FAILED;
> +               goto inactivate_out;
> +       }
> +
> +       if (bus->ops && (init = bus->ops->init[level-1])) {
> +               /* Note: This init1 function may register new devices */
> +               bus->error = init(bus);
> +               if (bus->error != DRVMGR_OK) {
> +                       /* An error of some kind during bus initialization.
> +                        *
> +                        * Child devices and their buses are not inactived
> +                        * directly here, instead they will all be catched by
> +                        * do_dev_init() and do_bus_init() by checking if
> +                        * parent or bridge-device failed. We know that
> +                        * initialization will happen later for those devices.
> +                        */
> +                       goto inactivate_out;
> +               }
> +       }
> +
> +       DRVMGR_LOCK_WRITE();
> +
> +       /* Bus taken into the new level */
> +       bus->level = level;
> +
> +       /* Put bus into list of buses reached level 'level'.
> +        * Put at end of bus list so that init[N+1]() calls comes
> +        * in the same order as init[N]()
> +        */
> +       drvmgr_list_add_tail(&mgr->buses[level], bus);
> +
> +       DRVMGR_UNLOCK();
> +
> +       return 0;
> +
> +inactivate_out:
> +       DRVMGR_LOCK_WRITE();
> +       bus->state |= BUS_STATE_INIT_FAILED;
> +       bus->state |= BUS_STATE_LIST_INACTIVE;
> +       drvmgr_list_add_head(&mgr->buses_inactive, bus);
> +       DRVMGR_UNLOCK();
> +
> +       DBG("do_bus_init(%d): (DEV: %s) failed\n", level, bus->dev->name);
> +
> +       return 1;
> +}
> +
> +/* Take device to initialization level 1 */
> +static int do_dev_init(
> +       struct rtems_driver_manager *mgr,
> +       struct drvmgr_dev *dev,
> +       int level)
> +{
> +       int (*init)(struct drvmgr_dev *);
> +
> +       /* Try to allocate Private Device Structure for driver if driver
> +        * requests for this feature.
> +        */
> +       if (dev->drv && dev->drv->dev_priv_size && !dev->priv) {
> +               dev->priv = malloc(dev->drv->dev_priv_size);
> +               memset(dev->priv, 0, dev->drv->dev_priv_size);
> +       }
> +
> +       /* If parent bus has failed during initialization,
> +        * the device is not initialized further.
> +        */
> +       if (dev->parent && (dev->parent->state & BUS_STATE_INIT_FAILED)) {
> +               dev->state |= DEV_STATE_DEPEND_FAILED;
> +               goto inactivate_out;
> +       }
> +
> +       /* Call Driver's Init Routine */
> +       if (dev->drv && (init = dev->drv->ops->init[level-1])) {
> +               /* Note: This init function may register new devices */
> +               dev->error = init(dev);
> +               if (dev->error != DRVMGR_OK) {
> +                       /* An error of some kind has occured in the
> +                        * driver/device, the failed device is put into the
> +                        * inactive list, this way Init2,3 and/or 4 will not
> +                        * be called for this device.
> +                        *
> +                        * The device is not removed from the bus (not
> +                        * unregistered). The driver can be used to find
> +                        * device information and debugging for example even
> +                        * if device initialization failed.
> +                        *
> +                        * Child buses and their devices are not inactived
> +                        * directly here, instead they will all be catched by
> +                        * do_dev_init() and do_bus_init() by checking if
> +                        * parent or bridge-device failed. We know that
> +                        * initialization will happen later for those devices.
> +                        */
> +                       goto inactivate_out;
> +               }
> +       }
> +
> +       DRVMGR_LOCK_WRITE();
> +       /* Dev taken into new level */
> +       dev->level = level;
> +
> +       /* Put at end of device list so that init[N+1]() calls comes
> +        * in the same order as init[N]()
> +        */
> +       drvmgr_list_add_tail(&mgr->devices[level], dev);
> +       DRVMGR_UNLOCK();
> +
> +       return 0;
> +
> +inactivate_out:
> +       DRVMGR_LOCK_WRITE();
> +       dev->state |= DEV_STATE_INIT_FAILED;
> +       dev->state |= DEV_STATE_LIST_INACTIVE;
> +       drvmgr_list_add_head(&mgr->devices_inactive, dev);
> +       DRVMGR_UNLOCK();
> +
> +       DBG("do_dev_init(%d): DRV: %s (DEV: %s) failed\n",
> +               level, dev->drv->name, dev->name);
> +
> +       return 1; /* Failed to take device into requested level */
> +}
These functions look terribly similar.

> +
> +/* Register Root device driver */
> +int drvmgr_root_drv_register(struct drvmgr_drv *drv)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +       struct drvmgr_dev *root = &mgr->root_dev;
> +
> +       if (mgr->root_drv) {
> +               /* Only possible to register root device once */
> +               return DRVMGR_FAIL;
> +       }
> +
> +       /* Set root device driver */
> +       drv->next = NULL;
> +       mgr->root_drv = drv;
> +
> +       /* Init root device non-NULL fields */
> +       root->minor_drv = -1;
> +       root->minor_bus = 0;
> +       root->businfo = mgr;
> +       root->name = "root bus";
> +       /* Custom Driver association */
> +       root->drv = mgr->root_drv;
> +
> +       /* This registers the root device and a bus */
> +       drvmgr_dev_register(root);
> +
> +       return DRVMGR_OK;
> +}
> +
> +/* Register a driver */
> +int drvmgr_drv_register(struct drvmgr_drv *drv)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +
> +       /* All drivers must have been registered before start of init,
> +        * because the manager does not scan all existing devices to find
> +        * suitable hardware for this driver, and it is not protected with
> +        * a lock therefore.
> +        */
> +       if (mgr->level > 0)
> +               return -1;
> +
> +       drv->obj_type = DRVMGR_OBJ_DRV;
> +
> +       /* Put driver into list of registered drivers */
> +       drvmgr_list_add_head(&mgr->drivers, drv);
> +
> +       /* TODO: we could scan for devices that this new driver has support
> +        *       for. However, at this stage we assume that all drivers are
> +        *       registered before devices are registered.
> +        *
> +        * LOCK: From the same assumsion locking the driver list is not needed
> +        *       either.
> +        */
> +
> +       return 0;
> +}
> +
> +/* Insert a device into a driver's device list and assign a driver minor number
> + * to the device.
> + *
> + * The devices are ordered by their minor number (sorted linked list of devices)
> + * the minor number is found by looking for a gap or at the end.
> + */
> +static void drvmgr_insert_dev_into_drv(
> +       struct drvmgr_drv *drv,
> +       struct drvmgr_dev *dev)
> +{
> +       struct drvmgr_dev *curr, **pprevnext;
> +       int minor;
> +
> +       minor = 0;
> +       pprevnext = &drv->dev;
> +       curr = drv->dev;
> +
> +       while (curr) {
> +               if (minor < curr->minor_drv) {
> +                       /* Found a gap. Insert new device between prev
> +                        * and curr. */
> +                       break;
> +               }
> +               minor++;
> +               pprevnext = &curr->next_in_drv;
> +               curr = curr->next_in_drv;
> +       }
> +       dev->next_in_drv = curr;
> +       *pprevnext = dev;
> +
> +       /* Set minor */
> +       dev->minor_drv = minor;
> +       drv->dev_cnt++;
> +}
> +
> +/* Insert a device into a bus device list and assign a bus minor number to the
> + * device.
> + *
> + * The devices are ordered by their minor number (sorted linked list of devices)
> + * and by their registeration order if not using the same driver.
> + *
> + * The minor number is found by looking for a gap or at the end.
> + */
> +static void drvmgr_insert_dev_into_bus(
> +       struct drvmgr_bus *bus,
> +       struct drvmgr_dev *dev)
> +{
> +       struct drvmgr_dev *curr, **pprevnext;
> +       int minor;
> +
> +       minor = 0;
> +       pprevnext = &bus->children;
> +       curr = bus->children;
> +
> +       while (curr) {
> +               if (dev->drv && (dev->drv == curr->drv)) {
> +                       if (minor < curr->minor_bus) {
> +                               /* Found a gap. Insert new device between prev
> +                                * and curr. */
> +                               break;
> +                       }
> +                       minor++;
> +               }
> +               pprevnext = &curr->next_in_bus;
> +               curr = curr->next_in_bus;
> +       }
> +       dev->next_in_bus = curr;
> +       *pprevnext = dev;
> +
> +       /* Set minor. Devices without driver are given -1 */
> +       if (dev->drv == NULL)
> +               minor = -1;
> +       dev->minor_bus = minor;
> +       bus->dev_cnt++;
> +}
> +
> +/* Try to find a driver for a device (unite a device with driver).
> + * a device with a driver
> + */
> +static struct drvmgr_drv *drvmgr_dev_find_drv(
> +               struct drvmgr_dev *dev)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +       struct drvmgr_drv *drv;
> +
> +       /* NOTE: No locking is needed here since Driver list is supposed to be
> +        *       initialized once during startup, we treat it as a static
> +        *       read-only list
> +        */
> +
> +       /* Try to find a driver that can handle this device */
> +       for (drv = DRV_LIST_HEAD(&mgr->drivers); drv; drv = drv->next)
> +               if (dev->parent->ops->unite(drv, dev) == 1)
> +                       break;
> +
> +       return drv;
> +}
> +
> +/* Register a device */
> +int drvmgr_dev_register(struct drvmgr_dev *dev)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +       struct drvmgr_drv *drv;
> +       struct drvmgr_bus *bus = dev->parent;
> +       struct drvmgr_key *keys;
> +       struct drvmgr_list *init_list = &mgr->devices_inactive;
> +
> +       DBG("DEV_REG: %s at bus \"%s\"\n", dev->name,
> +               bus && bus->dev && bus->dev->name ? bus->dev->name : "UNKNOWN");
> +
> +       /* Custom driver assocation? */
> +       if (dev->drv) {
> +               drv = dev->drv;
> +               DBG("CUSTOM ASSOCIATION (%s to %s)\n", dev->name, drv->name);
> +       } else {
> +               /* Try to find a driver that can handle this device */
> +               dev->drv = drv = drvmgr_dev_find_drv(dev);
> +       }
> +
> +       DRVMGR_LOCK_WRITE();
> +
> +       /* Assign Bus Minor number and put into bus device list
> +        * unless root device.
> +        */
> +       if (bus)
> +               drvmgr_insert_dev_into_bus(bus, dev);
> +
> +       if (!drv) {
> +               /* No driver found that can handle this device, put into
> +                * inactive list
> +                */
> +               dev->minor_drv = -1;
> +               dev->state |= DEV_STATE_LIST_INACTIVE;
> +       } else {
> +               /* United device with driver.
> +                * Put the device on the registered device list
> +                */
> +               dev->state |= DEV_STATE_UNITED;
> +
> +               /* Check if user want to skip this core. This is not a
> +                * normal request, however in a multi-processor system
> +                * the two(or more) RTEMS instances must not use the same
> +                * devices in a system, not reporting a device to
> +                * it's driver will effectively accomplish this. In a
> +                * non Plug & Play system one can easily avoid this
> +                * problem by not report the core, but in a Plug & Play
> +                * system the bus driver will report all found cores.
> +                *
> +                * To stop the two RTEMS instances from using the same
> +                * device the user can simply define a resource entry
> +                * for a certain device but set the keys field to NULL.
> +                */
> +               if (drvmgr_keys_get(dev, &keys) == 0 && keys == NULL) {
> +                       /* Found Driver resource entry point
> +                        * for this device, it was NULL, this
> +                        * indicates to skip the core.
> +                        *
> +                        * We put it into the inactive list
> +                        * marking it as ignored.
> +                        */
> +                       dev->state |= DEV_STATE_IGNORED;
> +               } else {
> +                       /* Assign Driver Minor number and put into driver's
> +                        * device list
> +                        */
> +                       drvmgr_insert_dev_into_drv(drv, dev);
> +
> +                       /* Just register device, it will be initialized
> +                        * later together with bus.
> +                        *
> +                        * At the end of the list (breadth first search)
> +                        */
> +                       init_list = &mgr->devices[0];
> +
> +                       DBG("Registered %s (DRV: %s) on %s\n",
> +                               dev->name, drv->name,
> +                               bus ? bus->dev->name : "NO PARENT");
> +               }
> +       }
> +
> +       drvmgr_list_add_tail(init_list, dev);
> +
> +       DRVMGR_UNLOCK();
> +
> +       /* Trigger Device initialization if not root device and
> +        * has a driver
> +        */
> +       if (bus && dev->drv)
> +               drvmgr_init_update();
> +
> +       return 0;
> +}
> +
> +/* Register a bus */
> +int drvmgr_bus_register(struct drvmgr_bus *bus)
> +{
> +       struct rtems_driver_manager *mgr = &drv_mgr;
> +       struct drvmgr_bus *bus_up;
> +
> +       /* Get bus architecture depth - the distance from root bus */
> +       bus->depth = 0;
> +       bus_up = bus->dev->parent;
> +       while (bus_up) {
> +               bus->depth++;
> +               bus_up = bus_up->dev->parent;
> +       }
> +
> +       DRVMGR_LOCK_WRITE();
> +
> +       /* Put driver into list of found buses */
> +       drvmgr_list_add_tail(&mgr->buses[0], bus);
> +
> +       DRVMGR_UNLOCK();
> +
> +       /* Take bus into level1 and so on */
> +       drvmgr_init_update();
> +
> +       return 0;
> +}
> +
> +/* Allocate memory for a Device structure */
> +int drvmgr_alloc_dev(struct drvmgr_dev **pdev, int extra)
> +{
> +       struct drvmgr_dev *dev;
> +       int size;
> +
> +       size = ((sizeof(struct drvmgr_dev) + 3) & ~0x3) + extra;
I have no idea what is happening here.

> +       dev = (struct drvmgr_dev *)calloc(size, 1);
> +       if (!dev) {
> +               /* Failed to allocate device structure - critical error */
> +               rtems_fatal_error_occurred(RTEMS_NO_MEMORY);
> +       }
> +       *pdev = dev;
> +       dev->obj_type = DRVMGR_OBJ_DEV;
> +
> +       return 0;
> +}
> +
> +/* Allocate memory for a Bus structure */
> +int drvmgr_alloc_bus(struct drvmgr_bus **pbus, int extra)
> +{
> +       struct drvmgr_bus *bus;
> +       int size;
> +
> +       size = ((sizeof(struct drvmgr_bus) + 3) & ~0x3) + extra;
> +       bus = (struct drvmgr_bus *)calloc(size, 1);
> +       if (!bus) {
> +               /* Failed to allocate device structure - critical error */
> +               rtems_fatal_error_occurred(RTEMS_NO_MEMORY);
> +       }
> +       *pbus = bus;
> +       bus->obj_type = DRVMGR_OBJ_BUS;
> +
> +       return 0;
> +}
> +
> +/* Add driver resources to a bus instance */
> +void drvmgr_bus_res_add(struct drvmgr_bus *bus,
> +                               struct drvmgr_bus_res *bres)
> +{
> +       /* insert first in bus resource list. Locking isn't needed since
> +        * resources can only be added before resource requests are made.
> +        * When bus has been registered resources are considered a read-only
> +        * tree.
> +        */
> +       bres->next = bus->reslist;
> +       bus->reslist = bres;
> +}
> diff --git a/cpukit/libdrvmgr/drvmgr.h b/cpukit/libdrvmgr/drvmgr.h
> new file mode 100644
> index 0000000..f091728
> --- /dev/null
> +++ b/cpukit/libdrvmgr/drvmgr.h
> @@ -0,0 +1,965 @@
> +/* Driver Manager Interface.
> + *
> + * COPYRIGHT (c) 2009.
> + * Cobham Gaisler AB.
> + *
> + * The license and distribution terms for this file may be
> + * found in the file LICENSE in this distribution or at
> + * http://www.rtems.com/license/LICENSE.
> + */
> +
> +#ifndef _DRIVER_MANAGER_H_
> +#define _DRIVER_MANAGER_H_
> +
> +#include <rtems.h>
> +#include <drvmgr/drvmgr_list.h>
> +#include <stdint.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/*** Configure Driver manager ***/
> +
> +/* Define the number of initialization levels of device drivers */
> +#define DRVMGR_LEVEL_MAX 4
> +
> +/* Default to use semahpores for protection. Initialization works without
> + * locks and after initialization too if devices are not removed.
> + */
> +#ifndef DRVMGR_USE_LOCKS
> +#define DRVMGR_USE_LOCKS 1
> +#endif
> +
> +struct drvmgr_dev;     /* Device */
> +struct drvmgr_bus;     /* Bus */
> +struct drvmgr_drv;     /* Driver */
> +
> +/*** List Interface shortcuts ***/
> +#define BUS_LIST_HEAD(list) LIST_HEAD(list, struct drvmgr_bus)
> +#define BUS_LIST_TAIL(list) LIST_TAIL(list, struct drvmgr_bus)
> +#define DEV_LIST_HEAD(list) LIST_HEAD(list, struct drvmgr_dev)
> +#define DEV_LIST_TAIL(list) LIST_TAIL(list, struct drvmgr_dev)
> +#define DRV_LIST_HEAD(list) LIST_HEAD(list, struct drvmgr_drv)
> +#define DRV_LIST_TAIL(list) LIST_TAIL(list, struct drvmgr_drv)
> +
> +/*** Bus indentification ***/
> +#define DRVMGR_BUS_TYPE_NONE 0         /* Not a valid bus */
> +#define DRVMGR_BUS_TYPE_ROOT 1         /* Hard coded bus */
> +#define DRVMGR_BUS_TYPE_PCI 2          /* PCI bus */
> +#define DRVMGR_BUS_TYPE_AMBAPP 3       /* AMBA Plug & Play bus */
> +#define DRVMGR_BUS_TYPE_LEON2_AMBA 4   /* LEON2 hardcoded bus */
> +#define DRVMGR_BUS_TYPE_AMBAPP_DIST 5  /* Distibuted AMBA Plug & Play bus accessed using a communication interface */
> +#define DRVMGR_BUS_TYPE_SPW_RMAP 6     /* SpaceWire Network bus */
> +#define DRVMGR_BUS_TYPE_AMBAPP_RMAP 7  /* SpaceWire RMAP accessed AMBA Plug & Play bus */
> +
> +enum {
> +       DRVMGR_OBJ_NONE = 0,
> +       DRVMGR_OBJ_DRV = 1,
> +       DRVMGR_OBJ_BUS = 2,
> +       DRVMGR_OBJ_DEV = 3,
> +};
> +
> +/*** Driver indentification ***
> + *
> + * 64-bit identification integer definition
> + *  * Bus ID 8-bit [7..0]
> + *  * Reserved 8-bit field [63..56]
> + *  * Device ID specific for bus type 48-bit [55..8]  (Different buses have
> + *    different unique identifications for hardware/driver.)
> + *
> + * ID Rules
> + *  * A root bus driver must always have device ID set to 0. There can only by
> + *    one root bus driver for a certain bus type.
> + *  * A Driver ID must identify a unique hardware core
> + *
> + */
> +
> +/* Bus ID Mask */
> +#define DRIVER_ID_BUS_MASK 0x00000000000000FFULL
> +
> +/* Reserved Mask for future use */
> +#define DRIVER_ID_RSV_MASK 0xFF00000000000000ULL
> +
> +/* Reserved Mask for future use */
> +#define DRIVER_ID_DEV_MASK 0x00FFFFFFFFFFFF00ULL
> +
> +/* Set Bus ID Mask. */
> +#define DRIVER_ID(busid, devid) ((unsigned long long) \
> +       ((((unsigned long long)(devid) << 8) & DRIVER_ID_DEV_MASK) | \
> +        ((unsigned long long)(busid) & DRIVER_ID_BUS_MASK)))
> +
> +/* Get IDs */
> +#define DRIVER_BUSID_GET(id)   ((unsigned long long)(id) & DRIVER_ID_BUS_MASK)
> +#define DRIVER_DEVID_GET(id)   (((unsigned long long)(id) & DRIVER_ID_DEV_MASK) >> 8)
> +
> +#define DRIVER_ROOTBUS_ID(bus_type)    DRIVER_ID(bus_type, 0)
> +
> +/*** Root Bus drivers ***/
> +
> +/* Generic Hard coded Root bus: Driver ID */
> +#define DRIVER_ROOT_ID         DRIVER_ROOTBUS_ID(DRVMGR_BUS_TYPE_ROOT)
> +
> +/* PCI Plug & Play bus: Driver ID */
> +#define DRIVER_PCIBUS_ID       DRIVER_ROOTBUS_ID(DRVMGR_BUS_TYPE_PCI)
> +
> +/* AMBA Plug & Play bus: Driver ID */
> +#define DRIVER_GRLIB_AMBAPP_ID DRIVER_ROOTBUS_ID(DRVMGR_BUS_TYPE_AMBAPP)
> +
> +/* AMBA Hard coded bus: Driver ID */
> +#define DRIVER_LEON2_AMBA_ID   DRIVER_ROOTBUS_ID(DRVMGR_BUS_TYPE_LEON2_AMBA)
> +
> +/* Distributed AMBA Plug & Play bus: Driver ID */
> +#define DRIVER_AMBAPP_DIST_ID  DRIVER_ROOTBUS_ID(DRVMGR_BUS_TYPE_AMBAPP_DIST)
> +
It would be good to put all these macros in the same 'macro' namespace
for consistency purposes.

> +/*! Bus parameters used by driver interface functions to aquire information
> + * about bus. All Bus drivers should implement the operation 'get_params' so
> + * that the driver interface routines can access bus dependent information in
> + * an non-dependent way.
> + */
> +struct drvmgr_bus_params {
> +       char            *dev_prefix;            /*!< Optional name prefix */
> +};
> +
> +/* Interrupt Service Routine (ISR) */
> +typedef void (*drvmgr_isr)(void *arg);
Can you use the existing typedef for ISRs?

> +
> +/*! Bus operations */
> +struct drvmgr_bus_ops {
> +       /* Functions used internally within driver manager */
> +       int     (*init[DRVMGR_LEVEL_MAX])(struct drvmgr_bus *);
> +       int     (*remove)(struct drvmgr_bus *);
> +       int     (*unite)(struct drvmgr_drv *, struct drvmgr_dev *);     /*!< Unite Hardware Device with Driver */
> +
> +       /* Functions called indirectly from drivers */
> +       int     (*int_register)(struct drvmgr_dev *, int index, const char *info, drvmgr_isr isr, void *arg);
> +       int     (*int_unregister)(struct drvmgr_dev *, int index, drvmgr_isr isr, void *arg);
> +       int     (*int_clear)(struct drvmgr_dev *, int index);
> +       int     (*int_mask)(struct drvmgr_dev *, int index);
> +       int     (*int_unmask)(struct drvmgr_dev *, int index);
why are these all prefixed by int_?

> +
> +       /* Get Parameters */
> +       int     (*get_params)(struct drvmgr_dev *, struct drvmgr_bus_params *);
> +       /* Get Frequency of Bus */
> +       int     (*freq_get)(struct drvmgr_dev*, int, unsigned int*);
try to be consistent about using verb_object, e.g. get_params(),
get_frequency().

> +       /*! Function called to request information about a device. The bus
> +        *  driver interpret the bus-specific information about the device.
> +        */
> +       void    (*info_dev)(struct drvmgr_dev *, void (*print)(void *p, char *str), void *p);
maybe get_dev_info() ?

> +};
I'm a little confused why drvmgr_bus_ops contains a number of
functions without a drvmgr_bus* as an argument.

> +#define BUS_OPS_NUM (sizeof(struct drvmgr_bus_ops)/sizeof(void (*)(void)))
> +
> +struct drvmgr_func {
> +       int funcid;
> +       void *func;
> +};
> +#define DRVMGR_FUNC(_ID_, _FUNC_) {(int)(_ID_), (void *)(_FUNC_)}
> +#define DRVMGR_FUNC_END {0, NULL}
Are these macros used anywhere?

> +
> +/*** Resource definitions ***
> + *
> + * Overview of structures:
> + *  All bus resources entries (_bus_res) are linked together per bus
> + *  (bus_info->reslist). One bus resource entry has a pointer to an array of
> + *  driver resources (_drv_res). One driver resouces is made out of an array
> + *  of keys (drvmgr_key). All keys belongs to the same driver and harwdare
> + *  device. Each key has a Name, Type ID and Data interpreted differently
> + *  depending on the Type ID (union drvmgr_key_value).
> + *
> + */
> +
> +/* Key Data Types */
> +#define KEY_TYPE_NONE          0
> +#define KEY_TYPE_INT           1
> +#define KEY_TYPE_STRING                2
> +#define KEY_TYPE_POINTER       3
> +
You might consider an enum type instead. And again use a "namespace"
especially here in public headers.

> +#define KEY_EMPTY      {NULL, KEY_TYPE_NONE, {0}}
> +#define RES_EMPTY      {0, 0, NULL}
> +#define MMAP_EMPTY     {0, 0, 0}
> +
> +/*! Union of different values */
> +union drvmgr_key_value {
> +       unsigned int            i;      /*!< Key data type UNSIGNED INTEGER */
> +       char                    *str;   /*!< Key data type STRING */
> +       void                    *ptr;   /*!< Key data type ADDRESS/POINTER */
> +};
You may like to use uintptr_t instead of unsigned int in here.
> +
> +/* One key. One Value. Holding information relevant to the driver. */
> +struct drvmgr_key {
> +       char                    *key_name;      /* Name of key */
> +       int                     key_type;       /* How to interpret key_value */
> +       union drvmgr_key_value  key_value;      /* The value or pointer to value */
> +};
> +
> +/*! Driver resource entry, Driver resources for a certain device instance,
> + *  containing a number of keys where each key hold the data of interest.
> + */
> +struct drvmgr_drv_res {
> +       uint64_t                drv_id;         /*!< Identifies the driver this resource is aiming */
> +       int                     minor_bus;      /*!< Indentifies a specfic device */
> +       struct drvmgr_key       *keys;          /*!< First key in key array, ended with KEY_EMPTY */
> +};
> +
> +/*! Bus resource list node */
> +struct drvmgr_bus_res {
> +       struct drvmgr_bus_res   *next;          /*!< Next resource node in list */
> +       struct drvmgr_drv_res   resource[];     /*!< Array of resources, one per device instance */
> +};
I think you want to use resource[RTEMS_ZERO_LENGTH_ARRAY] here.

> +
> +/*! MAP entry. Describes an linear address space translation. Untranslated
> + *  Start, Translated Start and length.
> + *
> + * Used by bus drivers to describe the address translation needed for
> + * the translation driver interface.
> + */
> +struct drvmgr_map_entry {
> +       char            *name;          /*!< Map Name */
> +       unsigned int    size;           /*!< Size of map window */
> +       char            *from_adr;      /*!< Start address of access window used
> +                                        *   to reach into remote bus */
> +       char            *to_adr;        /*!< Start address of remote system
> +                                        *   address range */
> +};
> +#define DRVMGR_TRANSLATE_ONE2ONE       NULL
> +#define DRVMGR_TRANSLATE_NO_BRIDGE     ((void *)1)  /* No bridge, error */
> +
> +/*! Bus information. Describes a bus. */
> +struct drvmgr_bus {
> +       int                     obj_type;       /*!< DRVMGR_OBJ_BUS */
> +       unsigned char           bus_type;       /*!< Type of bus */
> +       unsigned char           depth;          /*!< Bus level distance from root bus */
> +       struct drvmgr_bus       *next;          /*!< Next Bus */
> +       struct drvmgr_dev       *dev;           /*!< Bus device, the hardware... */
> +       void                    *priv;          /*!< Private data structure used by BUS driver */
> +       struct drvmgr_dev       *children;      /*!< Hardware devices on this bus */
> +       struct drvmgr_bus_ops   *ops;           /*!< Bus operations supported by this bus driver */
> +       struct drvmgr_func      *funcs;         /*!< Extra operations */
> +       int                     dev_cnt;        /*!< Number of devices this bus has */
> +       struct drvmgr_bus_res   *reslist;       /*!< Bus resources, head of a linked list of resources. */
> +       struct drvmgr_map_entry *maps_up;       /*!< Map Translation, array of address spaces upstreams to CPU */
> +       struct drvmgr_map_entry *maps_down;     /*!< Map Translation, array of address spaces downstreams to Hardware */
> +
> +       /* Bus status */
> +       int                     level;          /*!< Initialization Level of Bus */
> +       int                     state;          /*!< Init State of Bus, BUS_STATE_* */
> +       int                     error;          /*!< Return code from bus->ops->initN() */
> +};
> +
> +/* States of a bus */
> +#define BUS_STATE_INIT_FAILED  0x00000001      /* Initialization Failed */
> +#define BUS_STATE_LIST_INACTIVE        0x00001000      /* In inactive bus list */
> +#define BUS_STATE_DEPEND_FAILED        0x00000004      /* Device init failed */
> +
> +/* States of a device */
> +#define DEV_STATE_INIT_FAILED  0x00000001      /* Initialization Failed */
> +#define DEV_STATE_INIT_DONE    0x00000002      /* All init levels completed */
> +#define DEV_STATE_DEPEND_FAILED        0x00000004      /* Parent Bus init failed */
> +#define DEV_STATE_UNITED       0x00000100      /* Device United with Device Driver */
> +#define DEV_STATE_REMOVED      0x00000200      /* Device has been removed (unregistered) */
> +#define DEV_STATE_IGNORED      0x00000400      /* Device was ignored according to user's request, the device
> +                                                * was never reported to it's driver (as expected).
> +                                                */
> +#define DEV_STATE_LIST_INACTIVE        0x00001000      /* In inactive device list */
> +
> +/*! Device information */
> +struct drvmgr_dev {
> +       int                     obj_type;       /*!< DRVMGR_OBJ_DEV */
> +       struct drvmgr_dev       *next;          /*!< Next device */
> +       struct drvmgr_dev       *next_in_bus;   /*!< Next device on the same bus */
> +       struct drvmgr_dev       *next_in_drv;   /*!< Next device using the same driver */
> +
> +       struct drvmgr_drv       *drv;           /*!< The driver owning this device */
> +       struct drvmgr_bus       *parent;        /*!< Bus that this device resides on */
> +       short                   minor_drv;      /*!< Device number within driver */
> +       short                   minor_bus;      /*!< Device number on bus (for device separation) */
> +       char                    *name;          /*!< Name of Device Hardware */
> +       void                    *priv;          /*!< Pointer to driver private device structure */
> +       void                    *businfo;       /*!< Host bus specific information */
> +       struct drvmgr_bus       *bus;           /*!< Pointer to bus, set only if this is a bridge */
> +
> +       /* Device Status */
> +       unsigned int            state;          /*!< State of device, see DEV_STATE_* */
> +       int                     level;          /*!< Init Level */
> +       int                     error;          /*!< Error state returned by driver */
> +};
> +
> +/*! Driver operations, function pointers. */
> +struct drvmgr_drv_ops {
> +       int     (*init[DRVMGR_LEVEL_MAX])(struct drvmgr_dev *); /*! Function doing Init Stage 1 of a hardware device */
> +       int     (*remove)(struct drvmgr_dev *); /*! Function called when device instance is to be removed */
> +       int     (*info)(struct drvmgr_dev *, void (*print)(void *p, char *str), void *p, int, char *argv[]);/*! Function called to request information about a device or driver */
> +};
> +#define DRV_OPS_NUM (sizeof(struct drvmgr_drv_ops)/sizeof(void (*)(void)))
> +
Function names in these ops tables could be made consistent when they
have teh same purpose. Also, the macro get the num could be shared,
e.g. #define DRVMGR_OPS_NUM(x) (sizeof(x)/sizeof(void (*)(void)) where
x is an ops table would work.

> +/*! Device driver description */
> +struct drvmgr_drv {
> +       int                     obj_type;       /*!< DRVMGR_OBJ_DRV */
> +       struct drvmgr_drv       *next;          /*!< Next Driver */
> +       struct drvmgr_dev       *dev;           /*!< Devices using this driver */
> +
> +       uint64_t                drv_id;         /*!< Unique Driver ID */
> +       char                    *name;          /*!< Name of Driver */
> +       int                     bus_type;       /*!< Type of Bus this driver supports */
> +       struct drvmgr_drv_ops   *ops;           /*!< Driver operations */
> +       struct drvmgr_func      *funcs;         /*!< Extra Operations */
> +       unsigned int            dev_cnt;        /*!< Number of devices in dev */
> +       unsigned int            dev_priv_size;  /*!< If non-zero DRVMGR will allocate memory for dev->priv */
> +};
> +
> +/*! Structure defines a function pointer called when driver manager is ready
> + *  for drivers to register themselfs. Used to select drivers available to the
> + *  driver manager.
> + */
> +struct drvmgr_drv_reg_func {
> +       void (*drv_reg)(void);
> +};
In RTEMS kernel code we try to avoid abbreviations. I've been letting
it slip, but I'm having second thoughts that many of these
public-facing struct and function definitions should be re-written to
use fewer abbreviations. so this would be like struct
drvmgr_driver_register_function. Also, in this case it is just one
function, why not just use a typedef for it?

> +
> +/*** DRIVER | DEVICE | BUS FUNCTIONS ***/
> +
> +/* Return Codes */
> +enum {
> +       DRVMGR_OK = 0,          /* Sucess */
> +       DRVMGR_NOMEM = 1,       /* Memory allocation error */
> +       DRVMGR_EIO = 2,         /* I/O error */
> +       DRVMGR_EINVAL = 3,      /* Invalid parameter */
> +       DRVMGR_ENOSYS = 4,
> +       DRVMGR_TIMEDOUT = 5,    /* Operation timeout error */
> +       DRVMGR_EBUSY = 6,
> +       DRVMGR_ENORES = 7,      /* Not enough resources */
> +       DRVMGR_FAIL = -1        /* Unspecified failure */
> +};
> +
> +/*! Initialize data structures of the driver management system.
> + *  Calls predefined register driver functions so that drivers can
> + *  register themselves.
> + */
> +extern void _DRV_Manager_initialization(void);
> +
Should pick a consistent "namespace" for this, for example Drvmgr
would be consistent with drvmgr. Then this function might either be
_Drvmgr_Manager_initialization() or _Drvmgr_Initialization() depending
on how you view it.

> +/*! Take all devices into init level 'level', all devices registered later
> + *  will directly be taken into this level as well, ensuring that all
> + *  registerd devices has been taken into the level.
> + *
> + */
> +extern void _DRV_Manager_init_level(int level);
> +
> +/*! This function must be defined by the BSP when the driver manager is enabled
> + * and initialized during BSP initialization. The function is called after a
> + * init level is reached the first time by the driver manager.
> + */
> +extern void bsp_driver_level_hook(int level);
It seems wrong to have a declaration for a bsp-layer function here. It
is probably also wrong to be calling it directly below.

> +
> +/*! Init driver manager all in one go, will call _DRV_Manager_initialization(),
> + *  then _DRV_Manager_init_level([1..DRVMGR_LEVEL_MAX]).
> + *  Typically called from Init task when user wants to initilize driver
> + *  manager after startup, otherwise not used.
> + */
> +extern int drvmgr_init(void);
> +
> +/* Take registered buses and devices into the correct init level,
> + * this function is called from _init_level() so normally
> + * we don't need to call it directly.
> + */
> +extern void drvmgr_init_update(void);
> +
> +/*! Register Root Bus device driver */
> +extern int drvmgr_root_drv_register(struct drvmgr_drv *drv);
> +
> +/*! Register a driver */
> +extern int drvmgr_drv_register(struct drvmgr_drv *drv);
> +
> +/*! Register a device */
> +extern int drvmgr_dev_register(struct drvmgr_dev *dev);
Tired eyes have trouble distinguishing drvmgr_drv and drvmgr_dev.
Another point in favor of reducing the abbreviations in the drvmgr
code base..

This is all the further I got for now. Will finish later.

Gedare


More information about the devel mailing list