On Thu, Aug 22, 2019 at 9:25 PM Scott Branden
<scott.branden@xxxxxxxxxxxx> wrote:
>
> Add user space api for bcm-vk driver.
> +
> +struct vk_metadata {
> + /* struct version, always backwards compatible */
> + __u32 version;
> +
> + /* Version 0 fields */
> + __u32 card_status;
> +#define VK_CARD_STATUS_FASTBOOT_READY BIT(0)
> +#define VK_CARD_STATUS_FWLOADER_READY BIT(1)
> +
> + __u32 firmware_version;
> + __u32 fw_status;
> + /* End version 0 fields */
> +
> + __u64 reserved[14];
> + /* Total of 16*u64 for all versions */
> +};
I'd suggest getting rid of the API version fields, just leave the version 0
fields here and add a new structure + ioctl if you need other
fields
Versioning usually just adds complexity and is hard to get right.
> +struct vk_access {
> + __u8 barno; /* BAR number to use */
> + __u8 type; /* Type of access */
> +#define VK_ACCESS_READ 0
> +#define VK_ACCESS_WRITE 1
> + __u32 len; /* length of data */
> + __u64 offset; /* offset in BAR */
> + __u32 *data; /* where to read/write data to */
> +};
The pointer in the last member makes the structure incompatible between
32-bit and 64-bit user space. You could work around that using a __u64
member and turning that into a pointer using the u64_to_user_ptr()
macro in the driver in a portable way.
However, since this seems to be a read/write type interface, maybe
it's better to just use read/write file operations.
I also wonder if the interface should be on a higher abstraction level
here.
Arnd
