[char-misc-next 1/2] mei: enable to set the internal flag for client write

[char-misc-next 1/2] mei: enable to set the internal flag for client write

[Tomas Winkler]

From: Alexander Usyskin <alexander.usyskin@intel.com>

Prepare the client write functions to set the internal flag in message
header. Carry both blocking and internal modes inside the transmit cb,
and call internal bus function  __mei_cl_send() with send mode bit mask.
The Internal flag should be added only on messages generated by the
driver.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c    |  2 +-
 drivers/misc/mei/bus-fixup.c |  3 ++-
 drivers/misc/mei/bus.c       | 10 ++++++----
 drivers/misc/mei/client.c    |  6 +++---
 drivers/misc/mei/client.h    |  2 +-
 drivers/misc/mei/main.c      |  2 +-
 drivers/misc/mei/mei_dev.h   | 15 ++++++++++++++-
 7 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index 7ae89b4a21d5..466afb2611c6 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -144,7 +144,7 @@ int mei_amthif_run_next_cmd(struct mei_device *dev)
 	dev->iamthif_state = MEI_IAMTHIF_WRITING;
 	cl->fp = cb->fp;
 
-	ret = mei_cl_write(cl, cb, false);
+	ret = mei_cl_write(cl, cb);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index 75b9d4ac8b1e..9e10d86e3887 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -162,7 +162,8 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 
 	WARN_ON(mutex_is_locked(&bus->device_lock));
 
-	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd), 1);
+	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd),
+			    MEI_CL_IO_TX_BLOCKING);
 	if (ret < 0) {
 		dev_err(bus->dev, "Could not send IF version cmd\n");
 		return ret;
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 8a1e813a548d..7c075e9b25e5 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -36,12 +36,12 @@
  * @cl: host client
  * @buf: buffer to send
  * @length: buffer length
- * @blocking: wait for write completion
+ * @mode: sending mode
  *
  * Return: written size bytes or < 0 on error
  */
 ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
-			bool blocking)
+		      unsigned int mode)
 {
 	struct mei_device *bus;
 	struct mei_cl_cb *cb;
@@ -80,9 +80,11 @@ ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 		goto out;
 	}
 
+	cb->internal = !!(mode & MEI_CL_IO_TX_INTERNAL);
+	cb->blocking = !!(mode & MEI_CL_IO_TX_BLOCKING);
 	memcpy(cb->buf.data, buf, length);
 
-	rets = mei_cl_write(cl, cb, blocking);
+	rets = mei_cl_write(cl, cb);
 
 out:
 	mutex_unlock(&bus->device_lock);
@@ -188,7 +190,7 @@ ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length)
 	if (cl == NULL)
 		return -ENODEV;
 
-	return __mei_cl_send(cl, buf, length, 1);
+	return __mei_cl_send(cl, buf, length, MEI_CL_IO_TX_BLOCKING);
 }
 EXPORT_SYMBOL_GPL(mei_cldev_send);
 
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 6fe02350578d..beb942e6c248 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -1598,18 +1598,17 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
  *
  * @cl: host client
  * @cb: write callback with filled data
- * @blocking: block until completed
  *
  * Return: number of bytes sent on success, <0 on failure.
  */
-int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking)
+int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb)
 {
 	struct mei_device *dev;
 	struct mei_msg_data *buf;
 	struct mei_msg_hdr mei_hdr;
 	int size;
 	int rets;
-
+	bool blocking;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -ENODEV;
@@ -1621,6 +1620,7 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking)
 
 	buf = &cb->buf;
 	size = buf->size;
+	blocking = cb->blocking;
 
 	cl_dbg(dev, cl, "size=%d\n", size);
 
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index d2bfabecd882..f2545af9be7b 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -219,7 +219,7 @@ int mei_cl_irq_connect(struct mei_cl *cl, struct mei_cl_cb *cb,
 int mei_cl_read_start(struct mei_cl *cl, size_t length, const struct file *fp);
 int mei_cl_irq_read_msg(struct mei_cl *cl, struct mei_msg_hdr *hdr,
 			struct mei_cl_cb *cmpl_list);
-int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking);
+int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb);
 int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
 		     struct mei_cl_cb *cmpl_list);
 
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index a1484574cfa8..e1bf54481fd6 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -322,7 +322,7 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 		goto out;
 	}
 
-	rets = mei_cl_write(cl, cb, false);
+	rets = mei_cl_write(cl, cb);
 out:
 	mutex_unlock(&dev->device_lock);
 	return rets;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 1169fd9e7d02..d50f70b4a05e 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -109,6 +109,17 @@ enum mei_cb_file_ops {
 	MEI_FOP_NOTIFY_STOP,
 };
 
+/**
+ * enum mei_cl_io_mode - io mode between driver and fw
+ *
+ * @MEI_CL_IO_TX_BLOCKING: send is blocking
+ * @MEI_CL_IO_TX_INTERNAL: internal communication between driver and FW
+ */
+enum mei_cl_io_mode {
+	MEI_CL_IO_TX_BLOCKING = BIT(0),
+	MEI_CL_IO_TX_INTERNAL = BIT(1),
+};
+
 /*
  * Intel MEI message data struct
  */
@@ -169,6 +180,7 @@ struct mei_cl;
  * @fp: pointer to file structure
  * @status: io status of the cb
  * @internal: communication between driver and FW flag
+ * @blocking: transmission blocking mode
  * @completed: the transfer or reception has completed
  */
 struct mei_cl_cb {
@@ -180,6 +192,7 @@ struct mei_cl_cb {
 	const struct file *fp;
 	int status;
 	u32 internal:1;
+	u32 blocking:1;
 	u32 completed:1;
 };
 
@@ -304,7 +317,7 @@ void mei_cl_bus_rescan(struct mei_device *bus);
 void mei_cl_bus_rescan_work(struct work_struct *work);
 void mei_cl_bus_dev_fixup(struct mei_cl_device *dev);
 ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
-			bool blocking);
+		      unsigned int mode);
 ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
 bool mei_cl_bus_rx_event(struct mei_cl *cl);
 bool mei_cl_bus_notify_event(struct mei_cl *cl);
-- 
2.7.4

[char-misc-next 2/2] mei: send OS type to the FW

[Tomas Winkler]

From: Alexander Usyskin <alexander.usyskin@intel.com>

Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
is supported. This workaround was added to support other broken OS
and we need to follow here. The command is sent just once at the boot time.

Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index 9e10d86e3887..344a0c99ee44 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid = MEI_UUID_NFC_INFO;
 #define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
 			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
 
+#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
+			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
+
 #define MEI_UUID_ANY NULL_UUID_LE
 
 /**
@@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
 	cldev->do_match = 0;
 }
 
+#define OSTYPE_LINUX    2
+struct mei_os_ver {
+	u16 build;
+	u16 reserved1;
+	u8  os_type;
+	u8  major;
+	u8  minor;
+	u8  reserved2;
+} __packed;
+
+#define MKHI_FEATURE_PTT 0x10
+
+struct mkhi_rule_id {
+	u16 rule_type;
+	u8 feature_id;
+	u8 reserved;
+} __packed;
+
+struct mkhi_fwcaps {
+	struct mkhi_rule_id id;
+	u8 len;
+	u8 data[0];
+} __packed;
+
+#define MKHI_FWCAPS_GROUP_ID 0x3
+#define MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD 6
+struct mkhi_msg_hdr {
+	u32  group_id    :8;
+	u32  command     :7;
+	u32  is_response :1;
+	u32  reserved    :8;
+	u32  result      :8;
+};
+
+struct mkhi_msg {
+	struct mkhi_msg_hdr hdr;
+	u8 data[0];
+} __packed;
+
+static int mei_osver(struct mei_cl_device *cldev)
+{
+	int ret;
+	const size_t size = sizeof(struct mkhi_msg_hdr) +
+			    sizeof(struct mkhi_fwcaps) +
+			    sizeof(struct mei_os_ver);
+	size_t length = 8;
+	char buf[size];
+	struct mkhi_msg *req;
+	struct mkhi_fwcaps *fwcaps;
+	struct mei_os_ver *os_ver;
+	unsigned int mode = MEI_CL_IO_TX_BLOCKING | MEI_CL_IO_TX_INTERNAL;
+
+	memset(buf, 0, size);
+
+	req = (struct mkhi_msg *)buf;
+	req->hdr.group_id = MKHI_FWCAPS_GROUP_ID;
+	req->hdr.command = MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD;
+
+	fwcaps = (struct mkhi_fwcaps *)req->data;
+
+	fwcaps->id.rule_type = 0x0;
+	fwcaps->id.feature_id = MKHI_FEATURE_PTT;
+	fwcaps->len = sizeof(*os_ver);
+	os_ver = (struct mei_os_ver *)fwcaps->data;
+	os_ver->os_type = OSTYPE_LINUX;
+
+	ret = __mei_cl_send(cldev->cl, buf, size, mode);
+	if (ret < 0)
+		return ret;
+
+	ret = __mei_cl_recv(cldev->cl, buf, length);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void mei_mkhi_fix(struct mei_cl_device *cldev)
+{
+	int ret;
+
+	dev_dbg(&cldev->dev, "running hook %s\n", __func__);
+
+	ret = mei_cldev_enable(cldev);
+	if (ret)
+		return;
+
+	ret = mei_osver(cldev);
+	if (ret)
+		dev_info(&cldev->dev, "OS version command failed\n");
+
+	mei_cldev_disable(cldev);
+}
+
 /**
  * mei_wd - wd client on the bus, change protocol version
  *   as the API has changed.
@@ -310,6 +407,7 @@ static struct mei_fixup {
 	MEI_FIXUP(MEI_UUID_NFC_INFO, blacklist),
 	MEI_FIXUP(MEI_UUID_NFC_HCI, mei_nfc),
 	MEI_FIXUP(MEI_UUID_WD, mei_wd),
+	MEI_FIXUP(MEI_UUID_MKHIF_FIX, mei_mkhi_fix),
 };
 
 /**
-- 
2.7.4

Re: [char-misc-next 2/2] mei: send OS type to the FW

[Jarkko Sakkinen]

On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
> is supported. This workaround was added to support other broken OS
> and we need to follow here. The command is sent just once at the boot time.
> 
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>

Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/misc/mei/bus-fixup.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 9e10d86e3887..344a0c99ee44 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid = MEI_UUID_NFC_INFO;
>  #define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
>  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
>  
> +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> +
>  #define MEI_UUID_ANY NULL_UUID_LE
>  
>  /**
> @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
>  	cldev->do_match = 0;
>  }
>  
> +#define OSTYPE_LINUX    2
> +struct mei_os_ver {
> +	u16 build;
> +	u16 reserved1;
> +	u8  os_type;
> +	u8  major;
> +	u8  minor;
> +	u8  reserved2;
> +} __packed;
> +
> +#define MKHI_FEATURE_PTT 0x10
> +
> +struct mkhi_rule_id {
> +	u16 rule_type;
> +	u8 feature_id;
> +	u8 reserved;
> +} __packed;
> +
> +struct mkhi_fwcaps {
> +	struct mkhi_rule_id id;
> +	u8 len;
> +	u8 data[0];
> +} __packed;
> +
> +#define MKHI_FWCAPS_GROUP_ID 0x3
> +#define MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD 6
> +struct mkhi_msg_hdr {
> +	u32  group_id    :8;
> +	u32  command     :7;
> +	u32  is_response :1;
> +	u32  reserved    :8;
> +	u32  result      :8;
> +};
> +
> +struct mkhi_msg {
> +	struct mkhi_msg_hdr hdr;
> +	u8 data[0];
> +} __packed;
> +
> +static int mei_osver(struct mei_cl_device *cldev)
> +{
> +	int ret;
> +	const size_t size = sizeof(struct mkhi_msg_hdr) +
> +			    sizeof(struct mkhi_fwcaps) +
> +			    sizeof(struct mei_os_ver);
> +	size_t length = 8;
> +	char buf[size];
> +	struct mkhi_msg *req;
> +	struct mkhi_fwcaps *fwcaps;
> +	struct mei_os_ver *os_ver;
> +	unsigned int mode = MEI_CL_IO_TX_BLOCKING | MEI_CL_IO_TX_INTERNAL;
> +
> +	memset(buf, 0, size);
> +
> +	req = (struct mkhi_msg *)buf;
> +	req->hdr.group_id = MKHI_FWCAPS_GROUP_ID;
> +	req->hdr.command = MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD;
> +
> +	fwcaps = (struct mkhi_fwcaps *)req->data;
> +
> +	fwcaps->id.rule_type = 0x0;
> +	fwcaps->id.feature_id = MKHI_FEATURE_PTT;
> +	fwcaps->len = sizeof(*os_ver);
> +	os_ver = (struct mei_os_ver *)fwcaps->data;
> +	os_ver->os_type = OSTYPE_LINUX;
> +
> +	ret = __mei_cl_send(cldev->cl, buf, size, mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __mei_cl_recv(cldev->cl, buf, length);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void mei_mkhi_fix(struct mei_cl_device *cldev)
> +{
> +	int ret;
> +
> +	dev_dbg(&cldev->dev, "running hook %s\n", __func__);
> +
> +	ret = mei_cldev_enable(cldev);
> +	if (ret)
> +		return;
> +
> +	ret = mei_osver(cldev);
> +	if (ret)
> +		dev_info(&cldev->dev, "OS version command failed\n");
> +
> +	mei_cldev_disable(cldev);
> +}
> +
>  /**
>   * mei_wd - wd client on the bus, change protocol version
>   *   as the API has changed.
> @@ -310,6 +407,7 @@ static struct mei_fixup {
>  	MEI_FIXUP(MEI_UUID_NFC_INFO, blacklist),
>  	MEI_FIXUP(MEI_UUID_NFC_HCI, mei_nfc),
>  	MEI_FIXUP(MEI_UUID_WD, mei_wd),
> +	MEI_FIXUP(MEI_UUID_MKHIF_FIX, mei_mkhi_fix),
>  };
>  
>  /**
> -- 
> 2.7.4
> 

Re: [char-misc-next 2/2] mei: send OS type to the FW

[Greg Kroah-Hartman]

On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
> is supported. This workaround was added to support other broken OS
> and we need to follow here. The command is sent just once at the boot time.
> 
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 9e10d86e3887..344a0c99ee44 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid = MEI_UUID_NFC_INFO;
>  #define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
>  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
>  
> +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> +
>  #define MEI_UUID_ANY NULL_UUID_LE
>  
>  /**
> @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
>  	cldev->do_match = 0;
>  }
>  
> +#define OSTYPE_LINUX    2
> +struct mei_os_ver {
> +	u16 build;
> +	u16 reserved1;

Don't you need to specify the endian-type of these (well for build), as
they get written to hardware?

thanks,

greg k-h

RE: [char-misc-next 2/2] mei: send OS type to the FW

[Winkler, Tomas]

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, November 10, 2016 09:23
> To: Winkler, Tomas <tomas.winkler@intel.com>
> Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> kernel@vger.kernel.org; Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> 
> On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Tell the FW that we are running a sane OS and TPM2_ChangeEPS() is
> > supported. This workaround was added to support other broken OS and we
> > need to follow here. The command is sent just once at the boot time.
> >
> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >
> > diff --git a/drivers/misc/mei/bus-fixup.c
> > b/drivers/misc/mei/bus-fixup.c index 9e10d86e3887..344a0c99ee44 100644
> > --- a/drivers/misc/mei/bus-fixup.c
> > +++ b/drivers/misc/mei/bus-fixup.c
> > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> 0x4628, 0x4D7F, \
> >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
> >
> > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > +
> >  #define MEI_UUID_ANY NULL_UUID_LE
> >
> >  /**
> > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
> >  	cldev->do_match = 0;
> >  }
> >
> > +#define OSTYPE_LINUX    2
> > +struct mei_os_ver {
> > +	u16 build;
> > +	u16 reserved1;
> 
> Don't you need to specify the endian-type of these (well for build), as they get
> written to hardware?

This is really x86 stuff only (depends on X86), the device lives in PCH or SoC,  you cannot really plug it into PowerPC.  So we are consciously not using endian-types in the HW interface.
Thanks
Tomas

Re: [char-misc-next 2/2] mei: send OS type to the FW

[Greg Kroah-Hartman]

On Thu, Nov 10, 2016 at 12:00:29PM +0000, Winkler, Tomas wrote:
> 
> 
> > -----Original Message-----
> > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > Sent: Thursday, November 10, 2016 09:23
> > To: Winkler, Tomas <tomas.winkler@intel.com>
> > Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> > kernel@vger.kernel.org; Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> > 
> > On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > >
> > > Tell the FW that we are running a sane OS and TPM2_ChangeEPS() is
> > > supported. This workaround was added to support other broken OS and we
> > > need to follow here. The command is sent just once at the boot time.
> > >
> > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 98 insertions(+)
> > >
> > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > b/drivers/misc/mei/bus-fixup.c index 9e10d86e3887..344a0c99ee44 100644
> > > --- a/drivers/misc/mei/bus-fixup.c
> > > +++ b/drivers/misc/mei/bus-fixup.c
> > > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> > 0x4628, 0x4D7F, \
> > >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
> > >
> > > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> > > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > > +
> > >  #define MEI_UUID_ANY NULL_UUID_LE
> > >
> > >  /**
> > > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
> > >  	cldev->do_match = 0;
> > >  }
> > >
> > > +#define OSTYPE_LINUX    2
> > > +struct mei_os_ver {
> > > +	u16 build;
> > > +	u16 reserved1;
> > 
> > Don't you need to specify the endian-type of these (well for build), as they get
> > written to hardware?
> 
> This is really x86 stuff only (depends on X86), the device lives in
> PCH or SoC,  you cannot really plug it into PowerPC.  So we are
> consciously not using endian-types in the HW interface.

Then just mark them all as little endian and be done with it :)

thanks,

greg k-h

RE: [char-misc-next 2/2] mei: send OS type to the FW

[Winkler, Tomas]

> 
> On Thu, Nov 10, 2016 at 12:00:29PM +0000, Winkler, Tomas wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > Sent: Thursday, November 10, 2016 09:23
> > > To: Winkler, Tomas <tomas.winkler@intel.com>
> > > Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> > > kernel@vger.kernel.org; Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com>
> > > Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> > >
> > > On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > >
> > > > Tell the FW that we are running a sane OS and TPM2_ChangeEPS() is
> > > > supported. This workaround was added to support other broken OS
> > > > and we need to follow here. The command is sent just once at the boot
> time.
> > > >
> > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > > > ++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 98 insertions(+)
> > > >
> > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > b/drivers/misc/mei/bus-fixup.c index 9e10d86e3887..344a0c99ee44
> > > > 100644
> > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > > > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> > > 0x4628, 0x4D7F, \
> > > >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
> > > >
> > > > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> > > > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > > > +
> > > >  #define MEI_UUID_ANY NULL_UUID_LE
> > > >
> > > >  /**
> > > > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
> > > >  	cldev->do_match = 0;
> > > >  }
> > > >
> > > > +#define OSTYPE_LINUX    2
> > > > +struct mei_os_ver {
> > > > +	u16 build;
> > > > +	u16 reserved1;
> > >
> > > Don't you need to specify the endian-type of these (well for build),
> > > as they get written to hardware?
> >
> > This is really x86 stuff only (depends on X86), the device lives in
> > PCH or SoC,  you cannot really plug it into PowerPC.  So we are
> > consciously not using endian-types in the HW interface.
> 
> Then just mark them all as little endian and be done with it :)

Then I will have to run the leX_to_cpu all over the code, which will really do nothing. I  think we already had this discussion in the past,  need to search the archive. 

Thanks
Tomas 

Re: [char-misc-next 2/2] mei: send OS type to the FW

[Greg Kroah-Hartman]

On Thu, Nov 10, 2016 at 12:19:06PM +0000, Winkler, Tomas wrote:
> > 
> > On Thu, Nov 10, 2016 at 12:00:29PM +0000, Winkler, Tomas wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Thursday, November 10, 2016 09:23
> > > > To: Winkler, Tomas <tomas.winkler@intel.com>
> > > > Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> > > > kernel@vger.kernel.org; Jarkko Sakkinen
> > > > <jarkko.sakkinen@linux.intel.com>
> > > > Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> > > >
> > > > On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > >
> > > > > Tell the FW that we are running a sane OS and TPM2_ChangeEPS() is
> > > > > supported. This workaround was added to support other broken OS
> > > > > and we need to follow here. The command is sent just once at the boot
> > time.
> > > > >
> > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > > > > ++++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 98 insertions(+)
> > > > >
> > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > b/drivers/misc/mei/bus-fixup.c index 9e10d86e3887..344a0c99ee44
> > > > > 100644
> > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > > > > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> > > > 0x4628, 0x4D7F, \
> > > > >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
> > > > >
> > > > > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> > > > > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > > > > +
> > > > >  #define MEI_UUID_ANY NULL_UUID_LE
> > > > >
> > > > >  /**
> > > > > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
> > > > >  	cldev->do_match = 0;
> > > > >  }
> > > > >
> > > > > +#define OSTYPE_LINUX    2
> > > > > +struct mei_os_ver {
> > > > > +	u16 build;
> > > > > +	u16 reserved1;
> > > >
> > > > Don't you need to specify the endian-type of these (well for build),
> > > > as they get written to hardware?
> > >
> > > This is really x86 stuff only (depends on X86), the device lives in
> > > PCH or SoC,  you cannot really plug it into PowerPC.  So we are
> > > consciously not using endian-types in the HW interface.
> > 
> > Then just mark them all as little endian and be done with it :)
> 
> Then I will have to run the leX_to_cpu all over the code, which will
> really do nothing.

It's not "nothing", it is explicitly stating what the hardware expects.
You write code in C for developers, the compiler will make it all go
away, but then the developer knows what is going on here, and it allows
them to understand the logic a lot better.

thanks,

greg k-h

RE: [char-misc-next 2/2] mei: send OS type to the FW

[Winkler, Tomas]

> 
> On Thu, Nov 10, 2016 at 12:19:06PM +0000, Winkler, Tomas wrote:
> > >
> > > On Thu, Nov 10, 2016 at 12:00:29PM +0000, Winkler, Tomas wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Thursday, November 10, 2016 09:23
> > > > > To: Winkler, Tomas <tomas.winkler@intel.com>
> > > > > Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> > > > > kernel@vger.kernel.org; Jarkko Sakkinen
> > > > > <jarkko.sakkinen@linux.intel.com>
> > > > > Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> > > > >
> > > > > On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > > > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > >
> > > > > > Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
> > > > > > is supported. This workaround was added to support other
> > > > > > broken OS and we need to follow here. The command is sent just
> > > > > > once at the boot
> > > time.
> > > > > >
> > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > > > > > ++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 98 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > > b/drivers/misc/mei/bus-fixup.c index
> > > > > > 9e10d86e3887..344a0c99ee44
> > > > > > 100644
> > > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > > > > > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> > > > > 0x4628, 0x4D7F, \
> > > > > >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32,
> 0xAB)
> > > > > >
> > > > > > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29,
> 0x4916, \
> > > > > > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > > > > > +
> > > > > >  #define MEI_UUID_ANY NULL_UUID_LE
> > > > > >
> > > > > >  /**
> > > > > > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device
> *cldev)
> > > > > >  	cldev->do_match = 0;
> > > > > >  }
> > > > > >
> > > > > > +#define OSTYPE_LINUX    2
> > > > > > +struct mei_os_ver {
> > > > > > +	u16 build;
> > > > > > +	u16 reserved1;
> > > > >
> > > > > Don't you need to specify the endian-type of these (well for
> > > > > build), as they get written to hardware?
> > > >
> > > > This is really x86 stuff only (depends on X86), the device lives
> > > > in PCH or SoC,  you cannot really plug it into PowerPC.  So we are
> > > > consciously not using endian-types in the HW interface.
> > >
> > > Then just mark them all as little endian and be done with it :)
> >
> > Then I will have to run the leX_to_cpu all over the code, which will
> > really do nothing.
> 
> It's not "nothing", it is explicitly stating what the hardware expects.

In this specific case this serves as annotation, so I suggest to solves this by adding documentation.  And anyhow fixing just this patch will just create more confusion. 

> You write code in C for developers, the compiler will make it all go away, but
> then the developer knows what is going on here, and it allows them to
> understand the logic a lot better.

I really wound 't care (and if you look back I was the one who put endian awareness  to iwlwifi) but I have some vague memory that we agreed on that when we matured the driver into 3.5 then this is end to end x86 only code and we won't annotate the device interface.

Thanks
Tomas

Re: [char-misc-next 2/2] mei: send OS type to the FW

[Greg Kroah-Hartman]

On Thu, Nov 10, 2016 at 01:00:54PM +0000, Winkler, Tomas wrote:
> > 
> > On Thu, Nov 10, 2016 at 12:19:06PM +0000, Winkler, Tomas wrote:
> > > >
> > > > On Thu, Nov 10, 2016 at 12:00:29PM +0000, Winkler, Tomas wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > > > > Sent: Thursday, November 10, 2016 09:23
> > > > > > To: Winkler, Tomas <tomas.winkler@intel.com>
> > > > > > Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> > > > > > kernel@vger.kernel.org; Jarkko Sakkinen
> > > > > > <jarkko.sakkinen@linux.intel.com>
> > > > > > Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> > > > > >
> > > > > > On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > > > > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > > >
> > > > > > > Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
> > > > > > > is supported. This workaround was added to support other
> > > > > > > broken OS and we need to follow here. The command is sent just
> > > > > > > once at the boot
> > > > time.
> > > > > > >
> > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > > > > > > ++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 98 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > > > b/drivers/misc/mei/bus-fixup.c index
> > > > > > > 9e10d86e3887..344a0c99ee44
> > > > > > > 100644
> > > > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > > > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > > > > > > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> > > > > > 0x4628, 0x4D7F, \
> > > > > > >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32,
> > 0xAB)
> > > > > > >
> > > > > > > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29,
> > 0x4916, \
> > > > > > > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > > > > > > +
> > > > > > >  #define MEI_UUID_ANY NULL_UUID_LE
> > > > > > >
> > > > > > >  /**
> > > > > > > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device
> > *cldev)
> > > > > > >  	cldev->do_match = 0;
> > > > > > >  }
> > > > > > >
> > > > > > > +#define OSTYPE_LINUX    2
> > > > > > > +struct mei_os_ver {
> > > > > > > +	u16 build;
> > > > > > > +	u16 reserved1;
> > > > > >
> > > > > > Don't you need to specify the endian-type of these (well for
> > > > > > build), as they get written to hardware?
> > > > >
> > > > > This is really x86 stuff only (depends on X86), the device lives
> > > > > in PCH or SoC,  you cannot really plug it into PowerPC.  So we are
> > > > > consciously not using endian-types in the HW interface.
> > > >
> > > > Then just mark them all as little endian and be done with it :)
> > >
> > > Then I will have to run the leX_to_cpu all over the code, which will
> > > really do nothing.
> > 
> > It's not "nothing", it is explicitly stating what the hardware expects.
> 
> In this specific case this serves as annotation, so I suggest to solves this by adding documentation.  And anyhow fixing just this patch will just create more confusion. 

<nit> please wrap your email lines properly </nit>

Maybe, you have to start somewhere :)

Anyway, you have to redo this patch anyway...

thanks,

greg k-h

Re: [char-misc-next 2/2] mei: send OS type to the FW

[Greg Kroah-Hartman]

On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Tell the FW that we are running a sane OS and TPM2_ChangeEPS()
> is supported. This workaround was added to support other broken OS
> and we need to follow here. The command is sent just once at the boot time.
> 
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 9e10d86e3887..344a0c99ee44 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid = MEI_UUID_NFC_INFO;
>  #define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
>  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
>  
> +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> +
>  #define MEI_UUID_ANY NULL_UUID_LE
>  
>  /**
> @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
>  	cldev->do_match = 0;
>  }
>  
> +#define OSTYPE_LINUX    2
> +struct mei_os_ver {
> +	u16 build;
> +	u16 reserved1;
> +	u8  os_type;
> +	u8  major;
> +	u8  minor;
> +	u8  reserved2;
> +} __packed;
> +
> +#define MKHI_FEATURE_PTT 0x10
> +
> +struct mkhi_rule_id {
> +	u16 rule_type;

Endian?

> +	u8 feature_id;
> +	u8 reserved;
> +} __packed;
> +
> +struct mkhi_fwcaps {
> +	struct mkhi_rule_id id;
> +	u8 len;
> +	u8 data[0];
> +} __packed;
> +
> +#define MKHI_FWCAPS_GROUP_ID 0x3
> +#define MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD 6
> +struct mkhi_msg_hdr {
> +	u32  group_id    :8;
> +	u32  command     :7;
> +	u32  is_response :1;
> +	u32  reserved    :8;
> +	u32  result      :8;

Oh that's scary, does this really work?  It's usually better to use
bitmasks to get the values out properly.

> +};
> +
> +struct mkhi_msg {
> +	struct mkhi_msg_hdr hdr;
> +	u8 data[0];
> +} __packed;
> +
> +static int mei_osver(struct mei_cl_device *cldev)
> +{
> +	int ret;
> +	const size_t size = sizeof(struct mkhi_msg_hdr) +
> +			    sizeof(struct mkhi_fwcaps) +
> +			    sizeof(struct mei_os_ver);
> +	size_t length = 8;
> +	char buf[size];
> +	struct mkhi_msg *req;
> +	struct mkhi_fwcaps *fwcaps;
> +	struct mei_os_ver *os_ver;
> +	unsigned int mode = MEI_CL_IO_TX_BLOCKING | MEI_CL_IO_TX_INTERNAL;
> +
> +	memset(buf, 0, size);
> +
> +	req = (struct mkhi_msg *)buf;
> +	req->hdr.group_id = MKHI_FWCAPS_GROUP_ID;
> +	req->hdr.command = MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD;
> +
> +	fwcaps = (struct mkhi_fwcaps *)req->data;
> +
> +	fwcaps->id.rule_type = 0x0;
> +	fwcaps->id.feature_id = MKHI_FEATURE_PTT;
> +	fwcaps->len = sizeof(*os_ver);
> +	os_ver = (struct mei_os_ver *)fwcaps->data;
> +	os_ver->os_type = OSTYPE_LINUX;
> +
> +	ret = __mei_cl_send(cldev->cl, buf, size, mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __mei_cl_recv(cldev->cl, buf, length);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void mei_mkhi_fix(struct mei_cl_device *cldev)
> +{
> +	int ret;
> +
> +	dev_dbg(&cldev->dev, "running hook %s\n", __func__);

ftrace can provide you this, right?

> +
> +	ret = mei_cldev_enable(cldev);
> +	if (ret)
> +		return;
> +
> +	ret = mei_osver(cldev);
> +	if (ret)
> +		dev_info(&cldev->dev, "OS version command failed\n");

dev_err()?

And what about the error value?

thanks,

greg k-h

RE: [char-misc-next 2/2] mei: send OS type to the FW

[Winkler, Tomas]

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, November 10, 2016 09:25
> To: Winkler, Tomas <tomas.winkler@intel.com>
> Cc: Usyskin, Alexander <alexander.usyskin@intel.com>; linux-
> kernel@vger.kernel.org; Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Subject: Re: [char-misc-next 2/2] mei: send OS type to the FW
> 
> On Tue, Nov 08, 2016 at 06:26:09PM +0200, Tomas Winkler wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Tell the FW that we are running a sane OS and TPM2_ChangeEPS() is
> > supported. This workaround was added to support other broken OS and we
> > need to follow here. The command is sent just once at the boot time.
> >
> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.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 | 98
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >
> > diff --git a/drivers/misc/mei/bus-fixup.c
> > b/drivers/misc/mei/bus-fixup.c index 9e10d86e3887..344a0c99ee44 100644
> > --- a/drivers/misc/mei/bus-fixup.c
> > +++ b/drivers/misc/mei/bus-fixup.c
> > @@ -38,6 +38,9 @@ static const uuid_le mei_nfc_info_guid =
> > MEI_UUID_NFC_INFO;  #define MEI_UUID_WD UUID_LE(0x05B79A6F,
> 0x4628, 0x4D7F, \
> >  			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
> >
> > +#define MEI_UUID_MKHIF_FIX UUID_LE(0x55213584, 0x9a29, 0x4916, \
> > +			0xba, 0xdf, 0xf, 0xb7, 0xed, 0x68, 0x2a, 0xeb)
> > +
> >  #define MEI_UUID_ANY NULL_UUID_LE
> >
> >  /**
> > @@ -69,6 +72,100 @@ static void blacklist(struct mei_cl_device *cldev)
> >  	cldev->do_match = 0;
> >  }
> >
> > +#define OSTYPE_LINUX    2
> > +struct mei_os_ver {
> > +	u16 build;
> > +	u16 reserved1;
> > +	u8  os_type;
> > +	u8  major;
> > +	u8  minor;
> > +	u8  reserved2;
> > +} __packed;
> > +
> > +#define MKHI_FEATURE_PTT 0x10
> > +
> > +struct mkhi_rule_id {
> > +	u16 rule_type;
> 
> Endian?

Device is x86 only burned to PCH/SoC. 

> > +	u8 feature_id;
> > +	u8 reserved;
> > +} __packed;
> > +
> > +struct mkhi_fwcaps {
> > +	struct mkhi_rule_id id;
> > +	u8 len;
> > +	u8 data[0];
> > +} __packed;
> > +
> > +#define MKHI_FWCAPS_GROUP_ID 0x3
> > +#define MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD 6 struct
> mkhi_msg_hdr {
> > +	u32  group_id    :8;
> > +	u32  command     :7;
> > +	u32  is_response :1;
> > +	u32  reserved    :8;
> > +	u32  result      :8;
> 
> Oh that's scary, does this really work?  It's usually better to use bitmasks to get
> the values out properly.

Yes, it works, but yes I may fix it with bit mask. 

> > +};
> > +
> > +struct mkhi_msg {
> > +	struct mkhi_msg_hdr hdr;
> > +	u8 data[0];
> > +} __packed;
> > +
> > +static int mei_osver(struct mei_cl_device *cldev) {
> > +	int ret;
> > +	const size_t size = sizeof(struct mkhi_msg_hdr) +
> > +			    sizeof(struct mkhi_fwcaps) +
> > +			    sizeof(struct mei_os_ver);
> > +	size_t length = 8;
> > +	char buf[size];
> > +	struct mkhi_msg *req;
> > +	struct mkhi_fwcaps *fwcaps;
> > +	struct mei_os_ver *os_ver;
> > +	unsigned int mode = MEI_CL_IO_TX_BLOCKING |
> MEI_CL_IO_TX_INTERNAL;
> > +
> > +	memset(buf, 0, size);
> > +
> > +	req = (struct mkhi_msg *)buf;
> > +	req->hdr.group_id = MKHI_FWCAPS_GROUP_ID;
> > +	req->hdr.command = MKHI_FWCAPS_SET_OS_VER_APP_RULE_CMD;
> > +
> > +	fwcaps = (struct mkhi_fwcaps *)req->data;
> > +
> > +	fwcaps->id.rule_type = 0x0;
> > +	fwcaps->id.feature_id = MKHI_FEATURE_PTT;
> > +	fwcaps->len = sizeof(*os_ver);
> > +	os_ver = (struct mei_os_ver *)fwcaps->data;
> > +	os_ver->os_type = OSTYPE_LINUX;
> > +
> > +	ret = __mei_cl_send(cldev->cl, buf, size, mode);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = __mei_cl_recv(cldev->cl, buf, length);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static void mei_mkhi_fix(struct mei_cl_device *cldev) {
> > +	int ret;
> > +
> > +	dev_dbg(&cldev->dev, "running hook %s\n", __func__);
> 
> ftrace can provide you this, right?

Sure it can, I'm just old chap working with printks. Will remove. 
 
> > +
> > +	ret = mei_cldev_enable(cldev);
> > +	if (ret)
> > +		return;
> > +
> > +	ret = mei_osver(cldev);
> > +	if (ret)
> > +		dev_info(&cldev->dev, "OS version command failed\n");
> 
> dev_err()?

It's not really fata value 
> And what about the error value?

Will add 

Thanks
Tomas