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

From: "Winkler, Tomas" <tomas.winkler@intel.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Usyskin, Alexander" <alexander.usyskin@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>,
	"Lubart, Vitaly" <vitaly.lubart@intel.com>
Subject: RE: [Intel-gfx] [PATCH v8 09/16] mei: bus: export common mkhi definitions into a separate header
Date: Wed, 7 Sep 2022 19:48:42 +0000	[thread overview]
Message-ID: <MN2PR11MB4093182E030B512E952F7384E5419@MN2PR11MB4093.namprd11.prod.outlook.com> (raw)
In-Reply-To: <d83d1cb9-961e-2f34-46d8-f46836261948@intel.com>

> 
> 
> 
> On 9/7/2022 8:58 AM, Tomas Winkler wrote:
> > From: Vitaly Lubart <vitaly.lubart@intel.com>
> >
> > 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@intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > ---
> >   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.

You are right, bad --amend :(
> 
> 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_ */