Re: [PATCH v2 1/3] bcm-vk: add bcm_vk UAPI

[Scott Branden]

Hi Greg,

On 2020-08-18 6:53 a.m., Greg Kroah-Hartman wrote:
> On Wed, Aug 05, 2020 at 05:46:29PM -0700, Scott Branden wrote:
>> Add user space api for bcm-vk driver.
>>
>> Signed-off-by: Scott Branden <scott.branden@xxxxxxxxxxxx>
>> ---
>>  include/uapi/linux/misc/bcm_vk.h | 99 ++++++++++++++++++++++++++++++++
>>  1 file changed, 99 insertions(+)
>>  create mode 100644 include/uapi/linux/misc/bcm_vk.h
>>
>> diff --git a/include/uapi/linux/misc/bcm_vk.h b/include/uapi/linux/misc/bcm_vk.h
>> new file mode 100644
>> index 000000000000..783087b7c31f
>> --- /dev/null
>> +++ b/include/uapi/linux/misc/bcm_vk.h
>> @@ -0,0 +1,99 @@
>> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) */
>> +/*
>> + * Copyright 2018-2020 Broadcom.
>> + */
>> +
>> +#ifndef __UAPI_LINUX_MISC_BCM_VK_H
>> +#define __UAPI_LINUX_MISC_BCM_VK_H
>> +
>> +#include <linux/ioctl.h>
>> +#include <linux/types.h>
>> +
>> +#define BCM_VK_MAX_FILENAME 64
>> +
>> +struct vk_image {
>> +	__u32 type; /* Type of image */
>> +#define VK_IMAGE_TYPE_BOOT1 1 /* 1st stage (load to SRAM) */
>> +#define VK_IMAGE_TYPE_BOOT2 2 /* 2nd stage (load to DDR) */
>> +	char filename[BCM_VK_MAX_FILENAME]; /* Filename of image */
>> +};
>> +
>> +struct vk_reset {
>> +	__u32 arg1;
>> +	__u32 arg2;
>> +};
>> +
>> +#define VK_MAGIC		0x5e
>> +
>> +/* Load image to Valkyrie */
>> +#define VK_IOCTL_LOAD_IMAGE	_IOW(VK_MAGIC, 0x2, struct vk_image)
>> +
>> +/* Send Reset to Valkyrie */
>> +#define VK_IOCTL_RESET		_IOW(VK_MAGIC, 0x4, struct vk_reset)
>> +
>> +/*
>> + * message block - basic unit in the message where a message's size is always
>> + *		   N x sizeof(basic_block)
>> + */
>> +struct vk_msg_blk {
>> +	__u8 function_id;
>> +#define VK_FID_TRANS_BUF	5
>> +#define VK_FID_SHUTDOWN		8
>> +	__u8 size;
>> +	__u16 trans_id; /* transport id, queue & msg_id */
>> +	__u32 context_id;
>> +	__u32 args[2];
>> +#define VK_CMD_PLANES_MASK	0x000f /* number of planes to up/download */
>> +#define VK_CMD_UPLOAD		0x0400 /* memory transfer to vk */
>> +#define VK_CMD_DOWNLOAD		0x0500 /* memory transfer from vk */
>> +#define VK_CMD_MASK		0x0f00 /* command mask */
>> +};
>> +
>> +#define VK_BAR_FWSTS			0x41c
>> +#define VK_BAR_COP_FWSTS		0x428
>> +/* VK_FWSTS definitions */
>> +#define VK_FWSTS_RELOCATION_ENTRY	BIT(0)
> <snip>
>
> I thought BIT() was not allowed in uapi .h files, this really works
> properly???
I did some investigation and it looks like a few other header files in include/uapi also use the BIT() macro:
include/uapi/misc/uacce/uacce.h
include/uapi/linux/psci.h
include/uapi/linux/v4l2-subdev.h
tools/include/uapi/linux/pkt_sched.h

It does look like we end up defining the BIT() macro in our user space app that includes the header file.

So, what is the proper thing to be done?
1) Move the BIT() macro somewhere in include/uapi and include it in the necessary header files
2) Use the _BITUL macro in include/uapi/linux/const.h instead?
3) something else?

> thanks,
>
> greg k-h
Regards,
Scott