diff mbox series

[API-NEXT,10/21] linux-gen: drv: driver: adding device querry function

Message ID 1487768164-43184-11-git-send-email-christophe.milard@linaro.org
State Superseded
Headers show
Series driver items registration and probing | expand

Commit Message

Christophe Milard Feb. 22, 2017, 12:55 p.m. UTC
Implementation of the device query function for the linux-gen ODP.

Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

---
 platform/linux-generic/drv_driver.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

-- 
2.7.4

Comments

Bill Fischofer Feb. 22, 2017, 10:38 p.m. UTC | #1
On Wed, Feb 22, 2017 at 6:55 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

> Implementation of the device query function for the linux-gen ODP.

>

> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

> ---

>  platform/linux-generic/drv_driver.c | 37 ++++++++++++++++++++++++++++++

> +++++++

>  1 file changed, 37 insertions(+)

>

> diff --git a/platform/linux-generic/drv_driver.c

> b/platform/linux-generic/drv_driver.c

> index 48a90a2..517a3c6 100644

> --- a/platform/linux-generic/drv_driver.c

> +++ b/platform/linux-generic/drv_driver.c

> @@ -376,6 +376,43 @@ static void device_destroy_terminate(odpdrv_device_t

> drv_device)

>         _odp_ishm_pool_free(list_elt_pool, device);

>  }

>

> +odpdrv_device_t *odpdrv_device_query(odpdrv_enumr_t enumr, const char

> *address)

> +{

> +       _odpdrv_device_t *dev;

> +       odpdrv_device_t *res;

> +       int index = 0;

> +

> +       int size = sizeof(odpdrv_device_t); /* for the

> ODPDRV_DEVICE_INVALID */

> +

> +       /* parse the list of device a first time to determine the size of

> +        * the memory to be allocated:

> +        */

> +       dev_list_read_lock();

> +       dev = device_lst.head;

> +       while (dev) {

> +               if ((dev->param.enumerator == enumr) &&

> +                   ((address == NULL) ||

> +                    (strcmp(dev->param.address, address) == 0)))

> +                       size += sizeof(odpdrv_device_t);

> +               dev = dev->next;

> +       }

> +

> +       /* then fill the list: */

> +       res = (odpdrv_device_t *)malloc(size);

> +       dev = device_lst.head;

> +       while (dev) {

> +               if ((dev->param.enumerator == enumr) &&

> +                   ((address == NULL) ||

> +                    (strcmp(dev->param.address, address) == 0)))

> +                       res[index++] = (odpdrv_device_t)dev;

> +               dev = dev->next;

> +       }

> +       dev_list_read_unlock();

> +       res[index++] = ODPDRV_DEVICE_INVALID;

> +

> +       return res; /* must be freed by caller! */

>


Most other ODP APIs that return a variable-number of return parameters have
the caller supply a return array and size and the routine fills that in and
returns the number of elements returned. Why is that model not suitable
here? The concern about doing malloc() calls within ODP code is that
constrains the caller as to the memory model being used. If the caller
supplies the return array then it can obtain that memory from wherever it
wishes.


> +}

> +

>  odpdrv_devio_t odpdrv_devio_register(odpdrv_devio_param_t *param)

>  {

>         ODP_ERR("NOT Supported yet! Driver %s Registration!\n.",

> --

> 2.7.4

>

>
Christophe Milard Feb. 27, 2017, 1:04 p.m. UTC | #2
hmmm,
Sure it is applicable here, but while most function using this
strategy would be happy to have some part of the job done, this one
won't:
for instance: receiving N packets always makes sense, even if M>N
packet are waiting to be received.
In this case, getting half of an anwer does not really make sense...
But I see your point regarding allocation.
Didn't we say that libC was always supported (hence malloc).
There is no need for speed here...

Christophe

On 22 February 2017 at 23:38, Bill Fischofer <bill.fischofer@linaro.org> wrote:
>

>

> On Wed, Feb 22, 2017 at 6:55 AM, Christophe Milard

> <christophe.milard@linaro.org> wrote:

>>

>> Implementation of the device query function for the linux-gen ODP.

>>

>> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

>> ---

>>  platform/linux-generic/drv_driver.c | 37

>> +++++++++++++++++++++++++++++++++++++

>>  1 file changed, 37 insertions(+)

>>

>> diff --git a/platform/linux-generic/drv_driver.c

>> b/platform/linux-generic/drv_driver.c

>> index 48a90a2..517a3c6 100644

>> --- a/platform/linux-generic/drv_driver.c

>> +++ b/platform/linux-generic/drv_driver.c

>> @@ -376,6 +376,43 @@ static void device_destroy_terminate(odpdrv_device_t

>> drv_device)

>>         _odp_ishm_pool_free(list_elt_pool, device);

>>  }

>>

>> +odpdrv_device_t *odpdrv_device_query(odpdrv_enumr_t enumr, const char

>> *address)

>> +{

>> +       _odpdrv_device_t *dev;

>> +       odpdrv_device_t *res;

>> +       int index = 0;

>> +

>> +       int size = sizeof(odpdrv_device_t); /* for the

>> ODPDRV_DEVICE_INVALID */

>> +

>> +       /* parse the list of device a first time to determine the size of

>> +        * the memory to be allocated:

>> +        */

>> +       dev_list_read_lock();

>> +       dev = device_lst.head;

>> +       while (dev) {

>> +               if ((dev->param.enumerator == enumr) &&

>> +                   ((address == NULL) ||

>> +                    (strcmp(dev->param.address, address) == 0)))

>> +                       size += sizeof(odpdrv_device_t);

>> +               dev = dev->next;

>> +       }

>> +

>> +       /* then fill the list: */

>> +       res = (odpdrv_device_t *)malloc(size);

>> +       dev = device_lst.head;

>> +       while (dev) {

>> +               if ((dev->param.enumerator == enumr) &&

>> +                   ((address == NULL) ||

>> +                    (strcmp(dev->param.address, address) == 0)))

>> +                       res[index++] = (odpdrv_device_t)dev;

>> +               dev = dev->next;

>> +       }

>> +       dev_list_read_unlock();

>> +       res[index++] = ODPDRV_DEVICE_INVALID;

>> +

>> +       return res; /* must be freed by caller! */

>

>

> Most other ODP APIs that return a variable-number of return parameters have

> the caller supply a return array and size and the routine fills that in and

> returns the number of elements returned. Why is that model not suitable

> here? The concern about doing malloc() calls within ODP code is that

> constrains the caller as to the memory model being used. If the caller

> supplies the return array then it can obtain that memory from wherever it

> wishes.

>

>>

>> +}

>> +

>>  odpdrv_devio_t odpdrv_devio_register(odpdrv_devio_param_t *param)

>>  {

>>         ODP_ERR("NOT Supported yet! Driver %s Registration!\n.",

>> --

>> 2.7.4

>>

>
diff mbox series

Patch

diff --git a/platform/linux-generic/drv_driver.c b/platform/linux-generic/drv_driver.c
index 48a90a2..517a3c6 100644
--- a/platform/linux-generic/drv_driver.c
+++ b/platform/linux-generic/drv_driver.c
@@ -376,6 +376,43 @@  static void device_destroy_terminate(odpdrv_device_t drv_device)
 	_odp_ishm_pool_free(list_elt_pool, device);
 }
 
+odpdrv_device_t *odpdrv_device_query(odpdrv_enumr_t enumr, const char *address)
+{
+	_odpdrv_device_t *dev;
+	odpdrv_device_t *res;
+	int index = 0;
+
+	int size = sizeof(odpdrv_device_t); /* for the ODPDRV_DEVICE_INVALID */
+
+	/* parse the list of device a first time to determine the size of
+	 * the memory to be allocated:
+	 */
+	dev_list_read_lock();
+	dev = device_lst.head;
+	while (dev) {
+		if ((dev->param.enumerator == enumr) &&
+		    ((address == NULL) ||
+		     (strcmp(dev->param.address, address) == 0)))
+			size += sizeof(odpdrv_device_t);
+		dev = dev->next;
+	}
+
+	/* then fill the list: */
+	res = (odpdrv_device_t *)malloc(size);
+	dev = device_lst.head;
+	while (dev) {
+		if ((dev->param.enumerator == enumr) &&
+		    ((address == NULL) ||
+		     (strcmp(dev->param.address, address) == 0)))
+			res[index++] = (odpdrv_device_t)dev;
+		dev = dev->next;
+	}
+	dev_list_read_unlock();
+	res[index++] = ODPDRV_DEVICE_INVALID;
+
+	return res; /* must be freed by caller! */
+}
+
 odpdrv_devio_t odpdrv_devio_register(odpdrv_devio_param_t *param)
 {
 	ODP_ERR("NOT Supported yet! Driver %s Registration!\n.",