Re: [PATCH v2 01/13] intel_gna: add driver module

[Maciej Kwapulinski <maciej.kwapulinski@linux.intel.com>]

Andy Shevchenko <andy.shevchenko@gmail.com> writes:

> On Wed, Mar 24, 2021 at 8:38 PM Maciej Kwapulinski
> <maciej.kwapulinski@linux.intel.com> wrote:
>>
....
>> diff --git a/include/uapi/misc/intel/gna.h b/include/uapi/misc/intel/gna.h
>> new file mode 100644
>> index 000000000000..a7e435b74a0a
>> --- /dev/null
>> +++ b/include/uapi/misc/intel/gna.h
>> @@ -0,0 +1,155 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
>> +/* Copyright(c) 2017-2021 Intel Corporation */
>> +
>> +#ifndef _UAPI_GNA_H_
>> +#define _UAPI_GNA_H_
>> +
>> +#if defined(__cplusplus)
>> +extern "C" {
>> +#endif
>
>> +#include <linux/types.h>
>> +#include <linux/ioctl.h>
>> +#include <linux/const.h>
>
> Ordered?
>
What do You mean?

>>
......
>> +struct gna_compute_cfg {
>> +       __u32 layer_base;
>> +       __u32 layer_count;
>> +
>> +       /* List of GNA memory buffers */
>> +       __u64 buffers_ptr;
>> +       __u64 buffer_count;
>> +
>> +       __u8 active_list_on;
>> +       __u8 gna_mode;
>> +       __u8 hw_perf_encoding;
>> +       __u8 pad[5];
>> +};
>> +
>> +union gna_parameter {
>> +       struct {
>> +               __u64 id;
>> +       } in;
>> +
>> +       struct {
>> +               __u64 value;
>> +       } out;
>> +};
>> +
>> +union gna_memory_map {
>> +       struct {
>> +               __u64 address;
>> +               __u32 size;
>> +               __u32 pad;
>> +       } in;
>> +
>> +       struct {
>> +               __u64 memory_id;
>> +       } out;
>> +};
>> +
>> +union gna_compute {
>> +       struct {
>> +               struct gna_compute_cfg config;
>> +       } in;
>> +
>> +       struct {
>> +               __u64 request_id;
>> +       } out;
>> +};
>> +
>> +union gna_wait {
>> +       struct {
>> +               __u64 request_id;
>> +               __u32 timeout;
>> +               __u32 pad;
>> +       } in;
>> +
>> +       struct {
>> +               __u32 hw_status;
>> +               __u32 pad;
>> +               struct gna_drv_perf drv_perf;
>> +               struct gna_hw_perf hw_perf;
>> +       } out;
>> +};
>
> For all unions:
> How do you know which branch is used (out, in)? What field and where
> in the ABI points to that?

each of the unions above plays the role of in/out argument to its
corresponding ioctl call.

'in' part is used when ioctl() is called by client (userland
application) - data is written by app.

'out' part is read by app on exit from ioctl(), but only when ioctl()
retuns 0.

do You suggest adding the comment to gna.h for the above?

> .....