On 01/05/17 12:06, Yi He wrote: > I've read and run tests again and my review still stands for this v9 series: > > Reviewed-and-tested-by: Yi He <[email protected]> > > Best Regards, Yi >
You can add Yi review for next v10. But you need to correct make doxygen_doc warnings: /opt/Linaro//opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_class_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumerated_dev_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_devio_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_driver_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_class_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumerated_dev_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_devio_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_driver_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:274: warning: argument 'odpdrv_driver_t' of command @param is not found in the argument list of odpdrv_driver_register(odpdrv_driver_t *driver) /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:274: warning: The following parameters of odpdrv_driver_register(odpdrv_driver_t *driver) are not documented: parameter 'driver' /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:194: warning: Member api_version (variable) of class odpdrv_devio_t is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:200: warning: Member enumr_api_version (variable) of class odpdrv_devio_t is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:136: warning: Member api_version (variable) of class odpdrv_enumr_t is not documented. odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_class_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumerated_dev_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_devio_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_driver_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_class_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumr_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_enumerated_dev_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_devio_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:170: warning: Member odpdrv_driver_t (typedef) of group odpdrv_driver is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:274: warning: argument 'odpdrv_driver_t' of command @param is not found in the argument list of odpdrv_driver_register(odpdrv_driver_t *driver) /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:274: warning: The following parameters of odpdrv_driver_register(odpdrv_driver_t *driver) are not documented: parameter 'driver' /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:194: warning: Member api_version (variable) of class odpdrv_devio_t is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:200: warning: Member enumr_api_version (variable) of class odpdrv_devio_t is not documented. /opt/Linaro/odp3.git/include/odp/drv/spec/driver.h:136: warning: Member api_version (variable) of class odpdrv_enumr_t is not documented. > On 3 January 2017 at 00:09, Christophe Milard <[email protected]> > wrote: > >> The enumerator class, enumerator instance, devio and driver registration >> functions prototypes (and a draft of their parameters) are >> defined, the goal being to define the registration framework only. >> >> Signed-off-by: Christophe Milard <[email protected]> >> --- >> include/odp/drv/spec/driver.h | 293 ++++++++++++++++++++++++++++++ >> ++++++++++++ >> platform/Makefile.inc | 1 + >> 2 files changed, 294 insertions(+) >> create mode 100644 include/odp/drv/spec/driver.h >> >> diff --git a/include/odp/drv/spec/driver.h b/include/odp/drv/spec/driver.h >> new file mode 100644 >> index 0000000..abdd656 >> --- /dev/null >> +++ b/include/odp/drv/spec/driver.h >> @@ -0,0 +1,293 @@ >> +/* Copyright (c) 2017, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODPDRV driver >> + */ >> + >> +#ifndef ODPDRV_DRIVER_H_ >> +#define ODPDRV_DRIVER_H_ >> +#include <odp/visibility_begin.h> >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +/** >> +* @addtogroup odpdrv_driver >> +* @details >> +* enumerator and driver interface to ODP >> +* >> +* 1) ODP loads the different modules (i.e. it loads shared libraries, >> *.so). >> +* In the context of drivers, shared libraries may contain enumerators, >> +* drivers and devios. These register in step 2. >> +* >> +* 2) odpdrv_enumr_class_register(int (probe*)()...) >> +* -----------------------------------------------------------> >> +* odpdrv_driver_register(int (probe*)()...) >> +* -----------------------------------------------------------> >> +* odpdrv_devio_register() >> +* -----------------------------------------------------------> >> +* A number of device_enumerator_classes are registered at the ODP >> startup. >> +* Many classes are expected: static, ACPI, PCI, switchdev, virtual, >> DPAA2... >> +* A number of drivers also register to ODP (passing their own probe >> function). >> +* A number of device IO may also register to ODP (defining available >> devices >> +* interfaces). >> +* >> +* 3) ODP calls the probe function of each enumerator class >> +* <----------------------------------------------------------- >> +* odpdrv_emum_register(int (probe*)()...) >> +* -----------------------------------------------------------> >> +* -----------------------------------------------------------> >> +* -----------------------------------------------------------> >> +* odpdrv_devio_register(...) >> +* -----------------------------------------------------------> >> +* -----------------------------------------------------------> >> +* -----------------------------------------------------------> >> +* ODP calls the probe function of each registered enumerator_class. >> +* This result in the enumerator_class registering some >> +* enumerators (instances of the class) by calling >> +* odpdrv_emumerator_register() for each instance. >> +* A given enumerator_class may create many enumerators based on its >> platform: >> +* For instance Linux defines a number of PCI domains that can be viewed >> as >> +* multiple PCI enumerators. In addition, it could be considered that >> each PCI >> +* root of each processor socket in a NUMA environment has its own PCI >> +* enumerator. >> +* For enumerator class PCI, there could be one instance for each PCI >> +* domain. >> +* Note that the probe function of a given class may be recalled at any >> time >> +* (Hotplug) >> +* The devios delivered with their enumerator may also register at this >> stage. >> +* >> +* 4) For each enumerator instance, odp calls the probe function to >> retrieve >> +* the list of devices enumerated by the given enumerator instance. >> +* Note that the probe function of a given enumerator may be recalled at >> any >> +* time(Hotplug) >> +* >> +* 5) The driver framework calls the drivers probe(D,I) functions of the >> +* drivers, with device D and devio I as parameter, assuming that: >> +* -devio I was on the driver supported list of devio (and version >> matches) >> +* -the devio I is registered and found its enumerator interface(E) >> api >> +* (name and version) >> +* -device D was enumerated by an enumerator providing interface E. >> +* The return value of the driver probe function tells whether the driver >> +* can handle the device or not. >> +* >> +* @{ >> +*/ >> + >> +/* Forward declarations for a top down description of structures */ >> +typedef struct odpdrv_enumr_class_t odpdrv_enumr_class_t; >> +typedef struct odpdrv_enumr_t odpdrv_enumr_t; >> +typedef struct odpdrv_enumerated_dev_t odpdrv_enumerated_dev_t; >> +typedef struct odpdrv_devio_t odpdrv_devio_t; >> +typedef struct odpdrv_driver_t odpdrv_driver_t; >> + >> +/** Maximum size for driver and enumerator names: */ >> +#define ODPDRV_NAME_SIZE 32 >> + >> +/** Maximum size for the enumerator dependent address: */ >> +#define ODPDRV_NAME_ADDR_SZ 64 >> + >> +/** The maximum number of interfaces a driver may support: */ >> +#define ODPDRV_MAX_DEVIOS 3 >> + >> +/** >> +* Parameters to be given at enumerator class registration: >> +*/ >> +struct odpdrv_enumr_class_t { >> + /** Enumerator name: mostly used for debug purpose. >> + * Name must be unique (e.g. "PCI-DPAA2") >> + */ >> + const char name[ODPDRV_NAME_SIZE]; >> + >> + /** Probe function: >> + * Called by ODP to get the enumerator class instances registered >> + */ >> + int (*probe)(void); >> + >> + /** Remove function: >> + * Free whatever resource the class may have allocated. >> + */ >> + int (*remove)(void); >> +}; >> + >> +/** >> +* Parameter to be given at enumerator (instance) registration: >> +*/ >> +struct odpdrv_enumr_t { >> + /** Class name: >> + * Identifies the class of the enumerator >> + */ >> + const char class_name[ODPDRV_NAME_SIZE]; >> + >> + /** Enumerator api_name and version are used by the devio >> + * to make sure the device can be accessed: >> + * E.g. "PCI" >> + * The format of the enum_dev part for the odpdrv_enumerated_dev_t >> + * structure is identified by the api-name and version below >> + */ >> + const char api_name[ODPDRV_NAME_SIZE]; >> + uint32_t api_version; >> + >> + /** Probe function: >> + * Called by ODP to get the enumerated device list >> + */ >> + odpdrv_enumerated_dev_t* (*probe)(void); >> + >> + /** Remove function: >> + * Free the list of enumerated devices. >> + */ >> + int (*remove)(odpdrv_enumerated_dev_t *devlist); >> + >> + /** Register event notifier function for hotplug events: >> + */ >> + int (*register_notifier)(void (*event_handler) (void)); >> +}; >> + >> +/** This structure defines a generic enumerated device, or actually the >> +* common part between all devices, the enumerator specific part being >> pointed >> +* by the enum_dev field below. >> +*/ >> +struct odpdrv_enumerated_dev_t { >> + /** enumerator which enumerated the device: >> + * >> + */ >> + odpdrv_enumr_t *enumerator; >> + >> + /** Device address: >> + * An enumerator dependent string giving the device address, >> + * e.g. "0000.23.12.1" for PCI domain 0, bus 23, device 12, >> function 1. >> + * This string identifies the device uniquely. >> + */ >> + const char address[ODPDRV_NAME_ADDR_SZ]; >> + >> + /** Enumerator dependent part >> + * This part is allocated by the enumerator and is enumerator >> dependent >> + * (i.e. different devices types will have different contents for >> + * enum_dev). >> + */ >> + void *enum_dev; >> + >> + /** Next pointer for lists >> + */ >> + odpdrv_enumerated_dev_t *next; >> + >> +}; >> + >> +/** >> + * Parameter to be given at devio registration: >> + */ >> +struct odpdrv_devio_t { >> + /** Devio name >> + * Identifies devio interface implemented by this devio >> + * (i.e:many devios may have the same name, but none of those >> + * with same provided interface should refer to a common enumerator >> + * class) >> + */ >> + const char api_name[ODPDRV_NAME_SIZE]; >> + uint32_t api_version; >> + >> + /** Enumerator interface name and version >> + * The enumerator interface this devio needs. >> + */ >> + const char enumr_api_name[ODPDRV_NAME_SIZE]; >> + uint32_t enumr_api_version; >> + >> + /** Ops >> + * Pointer to a devio ops structure (specific to each devio) >> + */ >> + void *ops; >> +}; >> + >> +/** >> +* Parameter to be given at driver registration: >> +*/ >> +struct odpdrv_driver_t { >> + /** Driver name >> + * The driver name (the pair {driver-name, enum-api-name} must >> + * be unique) >> + */ >> + const char name[ODPDRV_NAME_SIZE]; >> + >> + /** Supported devios: >> + * The list of supported devio: one of the following devio >> + * (with correct version) must be available for the driver to work: >> + */ >> + struct { >> + const char api_name[ODPDRV_NAME_SIZE]; >> + uint32_t api_version; >> + } devios[ODPDRV_MAX_DEVIOS]; >> + >> + /** Probe function: >> + * Called by ODP to see if the driver can drive a given device >> + * >> + */ >> + int (*probe)(odpdrv_enumerated_dev_t *dev); >> + >> + /** Remove function: >> + * Only called with devices whose probe() returned true >> + * >> + */ >> + int (*remove)(odpdrv_enumerated_dev_t *dev); >> + >> +}; >> + >> +/** >> +* Register an enumerator class. >> +* Each enumerator class calls this function at init time. >> +* (probably using gcc/clang * __constructor__ attribute.) >> +* >> +* @param enumr_class Pointer to a enumerator registration structure. >> +* @return 0 on success, less than zero on error. On error, enumerators >> classes >> +* should release allocated resources and return. >> +*/ >> +int odpdrv_enumr_class_register(odpdrv_enumr_class_t *enumr_class); >> + >> +/** >> +* Register an enumerator. >> +* Each enumerator calls this function at init time. >> +* (probably using gcc/clang * __constructor__ attribute.) >> +* >> +* @param enumerator Pointer to a enumerator registration structure. >> +* @return 0 on success, less than zero on error. On error, enumerators >> +* should release allocated resources and return. >> +*/ >> +int odpdrv_enumr_register(odpdrv_enumr_t *enumerator); >> + >> +/** >> +* Register an devio. >> +* Each devio calls this function at init time. >> +* (probably using gcc/clang * __constructor__ attribute.) >> +* >> +* @param devio Pointer to a devio registration structure. >> +* @return 0 on success, less than zero on error. >> +*/ >> +int odpdrv_devio_register(odpdrv_devio_t *devio); >> + >> +/** >> +* Register a Driver. >> +* Each driver calls this function at init time. >> +* (probably using gcc/clang * __constructor__ attribute.) >> +* >> +* @param odpdrv_driver_t Pointer to a driver registration structure. >> +* @return 0 on success, less than zero on error. On error, drivers >> +* should release allocated resources and return. >> +*/ >> +int odpdrv_driver_register(odpdrv_driver_t *driver); >> + >> +/** >> +* @} >> +*/ >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#include <odp/visibility_end.h> >> +#endif >> diff --git a/platform/Makefile.inc b/platform/Makefile.inc >> index aefbf9a..57026d0 100644 >> --- a/platform/Makefile.inc >> +++ b/platform/Makefile.inc >> @@ -68,6 +68,7 @@ odpdrvspecinclude_HEADERS = \ >> $(top_srcdir)/include/odp/drv/spec/barrier.h \ >> $(top_srcdir)/include/odp/drv/spec/byteorder.h \ >> $(top_srcdir)/include/odp/drv/spec/compiler.h \ >> + $(top_srcdir)/include/odp/drv/spec/driver.h \ >> $(top_srcdir)/include/odp/drv/spec/shm.h \ >> $(top_srcdir)/include/odp/drv/spec/spinlock.h \ >> $(top_srcdir)/include/odp/drv/spec/std_types.h \ >> -- >> 2.7.4 >> >>
