Re: [Intel-gfx] [PATCH v8 09/16] mei: bus: export common mkhi definitions into a separate header

[Ceraolo Spurio, Daniele]

On 9/7/2022 8:58 AM, Tomas Winkler wrote:
> From: Vitaly Lubart <vitaly.lubart@xxxxxxxxx>
>
> Exported common mkhi definitions from bus-fixup.c into a separate
> header file mkhi.h for other driver usage.
>
> Signed-off-by: Vitaly Lubart <vitaly.lubart@xxxxxxxxx>
> Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
> ---
>   drivers/misc/mei/bus-fixup.c | 31 +-------------------
>   drivers/misc/mei/mkhi.h      | 57 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 58 insertions(+), 30 deletions(-)
>   create mode 100644 drivers/misc/mei/mkhi.h
>
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 344598fcf8e9..c4e527803299 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -15,6 +15,7 @@
>   
>   #include "mei_dev.h"
>   #include "client.h"
> +#include "mkhi.h"
>   
>   #define MEI_UUID_NFC_INFO UUID_LE(0xd2de1625, 0x382d, 0x417d, \
>   			0x48, 0xa4, 0xef, 0xab, 0xba, 0x8a, 0x12, 0x06)
> @@ -89,20 +90,6 @@ struct mei_os_ver {
>   	u8  reserved2;
>   } __packed;
>   
> -#define MKHI_FEATURE_PTT 0x10
> -
> -struct mkhi_rule_id {
> -	__le16 rule_type;
> -	u8 feature_id;
> -	u8 reserved;
> -} __packed;
> -
> -struct mkhi_fwcaps {
> -	struct mkhi_rule_id id;
> -	u8 len;
> -	u8 data[];
> -} __packed;
> -
>   struct mkhi_fw_ver_block {
>   	u16 minor;
>   	u8 major;
> @@ -115,22 +102,6 @@ struct mkhi_fw_ver {
>   	struct mkhi_fw_ver_block ver[MEI_MAX_FW_VER_BLOCKS];
>   } __packed;
>   
> -#define MKHI_FWCAPS_GROUP_ID 0x3
> -#define MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD 6
> -#define MKHI_GEN_GROUP_ID 0xFF
> -#define MKHI_GEN_GET_FW_VERSION_CMD 0x2
> -struct mkhi_msg_hdr {
> -	u8  group_id;
> -	u8  command;
> -	u8  reserved;
> -	u8  result;
> -} __packed;
> -
> -struct mkhi_msg {
> -	struct mkhi_msg_hdr hdr;
> -	u8 data[];
> -} __packed;
> -
>   #define MKHI_OSVER_BUF_LEN (sizeof(struct mkhi_msg_hdr) + \
>   			    sizeof(struct mkhi_fwcaps) + \
>   			    sizeof(struct mei_os_ver))
> diff --git a/drivers/misc/mei/mkhi.h b/drivers/misc/mei/mkhi.h
> new file mode 100644
> index 000000000000..43cadfb1b990
> --- /dev/null
> +++ b/drivers/misc/mei/mkhi.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2003-2022, Intel Corporation. All rights reserved.
> + * Intel Management Engine Interface (Intel MEI) Linux driver
> + */
> +
> +#ifndef _MEI_MKHI_H_
> +#define _MEI_MKHI_H_
> +
> +#include <linux/types.h>
> +
> +#define MKHI_FEATURE_PTT 0x10
> +
> +#define MKHI_FWCAPS_GROUP_ID 0x3
> +#define MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD 6
> +#define MKHI_GEN_GROUP_ID 0xFF
> +#define MKHI_GEN_GET_FW_VERSION_CMD 0x2
> +
> +#define MKHI_GROUP_ID_GFX              0x30
> +#define MKHI_GFX_RESET_WARN_CMD_REQ    0x0
> +#define MKHI_GFX_MEMORY_READY_CMD_REQ  0x1
> +
> +/* Allow transition to PXP mode without approval */
> +#define MKHI_GFX_MEM_READY_PXP_ALLOWED  0x1
> +
> +#define MKHI_GROUP_ID_GFX              0x30
> +#define MKHI_GFX_RESET_WARN_CMD_REQ    0x0
> +#define MKHI_GFX_MEMORY_READY_CMD_REQ  0x1
> +
> +/* Allow transition to PXP mode without approval */
> +#define MKHI_GFX_MEM_READY_PXP_ALLOWED  0x1

Something weird is happening here. Those defs were added by the next 
patch in the previous rev, while now they're added here twice and then 
still added once more in the next patch, so they're defined 3 times. IMO 
leaving it to the next patch is better, so they should just be dropped 
from this one.

Daniele

> +
> +struct mkhi_rule_id {
> +	__le16 rule_type;
> +	u8 feature_id;
> +	u8 reserved;
> +} __packed;
> +
> +struct mkhi_fwcaps {
> +	struct mkhi_rule_id id;
> +	u8 len;
> +	u8 data[];
> +} __packed;
> +
> +struct mkhi_msg_hdr {
> +	u8  group_id;
> +	u8  command;
> +	u8  reserved;
> +	u8  result;
> +} __packed;
> +
> +struct mkhi_msg {
> +	struct mkhi_msg_hdr hdr;
> +	u8 data[];
> +} __packed;
> +
> +#endif /* _MEI_MKHI_H_ */