linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [char-misc-next 0/7] mei minor fixes and cleanups
@ 2013-09-16 20:44 Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 1/7] mei: fix function names in debug prints Tomas Winkler
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler


Alexander Usyskin (1):
  mei: fix function names in debug prints

Tomas Winkler (6):
  mei: propagate error from write routines instead of ENODEV
  mei: push credentials inside the irq write handler
  mei: mei_cl_unlink: no need to loop over dev list
  mei: simplify mei_open error handling
  mei: revamp open handler counts
  mei: amthif: mei_amthif_host_init: propagate errors from called
    functions

 drivers/misc/mei/amthif.c    | 49 +++++++++++++++++----------
 drivers/misc/mei/client.c    | 81 +++++++++++++++++++++++++++++++-------------
 drivers/misc/mei/hbm.c       |  2 +-
 drivers/misc/mei/init.c      |  5 ---
 drivers/misc/mei/interrupt.c | 31 +++++++++--------
 drivers/misc/mei/main.c      | 37 ++++++++------------
 drivers/misc/mei/mei_dev.h   |  1 +
 7 files changed, 122 insertions(+), 84 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [char-misc-next 1/7] mei: fix function names in debug prints
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  2013-09-26 15:40   ` Greg KH
  2013-09-16 20:44 ` [char-misc-next 2/7] mei: propagate error from write routines instead of ENODEV Tomas Winkler
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Alexander Usyskin, Tomas Winkler

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

Fix calling function names in debug prints.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/interrupt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index d27804e..be42c70 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -343,14 +343,14 @@ int mei_irq_read_handler(struct mei_device *dev,
 
 	/* decide where to read the message too */
 	if (!mei_hdr->host_addr) {
-		dev_dbg(&dev->pdev->dev, "call mei_irq_thread_read_bus_message.\n");
+		dev_dbg(&dev->pdev->dev, "call mei_hbm_dispatch.\n");
 		mei_hbm_dispatch(dev, mei_hdr);
-		dev_dbg(&dev->pdev->dev, "end mei_irq_thread_read_bus_message.\n");
+		dev_dbg(&dev->pdev->dev, "end mei_hbm_dispatch.\n");
 	} else if (mei_hdr->host_addr == dev->iamthif_cl.host_client_id &&
 		   (MEI_FILE_CONNECTED == dev->iamthif_cl.state) &&
 		   (dev->iamthif_state == MEI_IAMTHIF_READING)) {
 
-		dev_dbg(&dev->pdev->dev, "call mei_irq_thread_read_iamthif_message.\n");
+		dev_dbg(&dev->pdev->dev, "call mei_amthif_irq_read_msg.\n");
 		dev_dbg(&dev->pdev->dev, MEI_HDR_FMT, MEI_HDR_PRM(mei_hdr));
 
 		ret = mei_amthif_irq_read_msg(dev, mei_hdr, cmpl_list);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [char-misc-next 2/7] mei: propagate error from write routines instead of ENODEV
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 1/7] mei: fix function names in debug prints Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 3/7] mei: push credentials inside the irq write handler Tomas Winkler
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

ENODEV will cause application to try to reconnect since
it assumes that device went through the reset
write errors are not always fatal it can happen due to
resource contention

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c    | 24 +++++++++++++-----------
 drivers/misc/mei/client.c    | 17 +++++++++--------
 drivers/misc/mei/hbm.c       |  2 +-
 drivers/misc/mei/interrupt.c | 20 +++++++++++++-------
 4 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index f6ff711..e4a4e2e 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -313,13 +313,13 @@ static int mei_amthif_send_cmd(struct mei_device *dev, struct mei_cl_cb *cb)
 		mei_hdr.me_addr = dev->iamthif_cl.me_client_id;
 		mei_hdr.reserved = 0;
 		dev->iamthif_msg_buf_index += mei_hdr.length;
-		if (mei_write_message(dev, &mei_hdr,
-					(unsigned char *)dev->iamthif_msg_buf))
-			return -ENODEV;
+		ret = mei_write_message(dev, &mei_hdr, dev->iamthif_msg_buf);
+		if (ret)
+			return ret;
 
 		if (mei_hdr.msg_complete) {
 			if (mei_cl_flow_ctrl_reduce(&dev->iamthif_cl))
-				return -ENODEV;
+				return -EIO;
 			dev->iamthif_flow_control_pending = true;
 			dev->iamthif_state = MEI_IAMTHIF_FLOW_CONTROL;
 			dev_dbg(&dev->pdev->dev, "add amthif cb to write waiting list\n");
@@ -459,6 +459,7 @@ int mei_amthif_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 	struct mei_msg_hdr mei_hdr;
 	size_t len = dev->iamthif_msg_buf_size - dev->iamthif_msg_buf_index;
 	u32 msg_slots = mei_data2slots(len);
+	int rets;
 
 	mei_hdr.host_addr = cl->host_client_id;
 	mei_hdr.me_addr = cl->me_client_id;
@@ -481,16 +482,17 @@ int mei_amthif_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 	dev_dbg(&dev->pdev->dev, MEI_HDR_FMT,  MEI_HDR_PRM(&mei_hdr));
 
 	*slots -=  msg_slots;
-	if (mei_write_message(dev, &mei_hdr,
-		dev->iamthif_msg_buf + dev->iamthif_msg_buf_index)) {
-			dev->iamthif_state = MEI_IAMTHIF_IDLE;
-			cl->status = -ENODEV;
-			list_del(&cb->list);
-			return -ENODEV;
+	rets = mei_write_message(dev, &mei_hdr,
+			dev->iamthif_msg_buf + dev->iamthif_msg_buf_index);
+	if (rets) {
+		dev->iamthif_state = MEI_IAMTHIF_IDLE;
+		cl->status = rets;
+		list_del(&cb->list);
+		return rets;
 	}
 
 	if (mei_cl_flow_ctrl_reduce(cl))
-		return -ENODEV;
+		return -EIO;
 
 	dev->iamthif_msg_buf_index += mei_hdr.length;
 	cl->status = 0;
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 77b8d9b..c59c8a2 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -700,6 +700,7 @@ int mei_cl_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 	struct mei_msg_hdr mei_hdr;
 	size_t len = cb->request_buffer.size - cb->buf_idx;
 	u32 msg_slots = mei_data2slots(len);
+	int rets;
 
 	mei_hdr.host_addr = cl->host_client_id;
 	mei_hdr.me_addr = cl->me_client_id;
@@ -723,11 +724,12 @@ int mei_cl_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 			cb->request_buffer.size, cb->buf_idx);
 
 	*slots -=  msg_slots;
-	if (mei_write_message(dev, &mei_hdr,
-			cb->request_buffer.data + cb->buf_idx)) {
-		cl->status = -ENODEV;
+	rets = mei_write_message(dev, &mei_hdr,
+			cb->request_buffer.data + cb->buf_idx);
+	if (rets) {
+		cl->status = rets;
 		list_move_tail(&cb->list, &cmpl_list->list);
-		return -ENODEV;
+		return rets;
 	}
 
 	cl->status = 0;
@@ -736,7 +738,7 @@ int mei_cl_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 
 	if (mei_hdr.msg_complete) {
 		if (mei_cl_flow_ctrl_reduce(cl))
-			return -ENODEV;
+			return -EIO;
 		list_move_tail(&cb->list, &dev->write_waiting_list.list);
 	}
 
@@ -805,10 +807,9 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking)
 	mei_hdr.reserved = 0;
 
 
-	if (mei_write_message(dev, &mei_hdr, buf->data)) {
-		rets = -EIO;
+	rets = mei_write_message(dev, &mei_hdr, buf->data);
+	if (rets)
 		goto err;
-	}
 
 	cl->writing_state = MEI_WRITING;
 	cb->buf_idx = mei_hdr.length;
diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 52afe1e..9b3a0fb 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -174,7 +174,7 @@ int mei_hbm_start_req(struct mei_device *dev)
 		dev_err(&dev->pdev->dev, "version message write failed\n");
 		dev->dev_state = MEI_DEV_RESETTING;
 		mei_reset(dev, 1);
-		return -ENODEV;
+		return -EIO;
 	}
 	dev->hbm_state = MEI_HBM_START;
 	dev->init_clients_timer = MEI_CLIENTS_INIT_TIMEOUT;
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index be42c70..e4bb9ae 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -216,9 +216,11 @@ static int mei_cl_irq_read(struct mei_cl *cl, struct mei_cl_cb *cb,
 			   s32 *slots, struct mei_cl_cb *cmpl_list)
 {
 	struct mei_device *dev = cl->dev;
-
 	u32 msg_slots = mei_data2slots(sizeof(struct hbm_flow_control));
 
+	int ret;
+
+
 	if (*slots < msg_slots) {
 		/* return the cancel routine */
 		list_del(&cb->list);
@@ -227,12 +229,14 @@ static int mei_cl_irq_read(struct mei_cl *cl, struct mei_cl_cb *cb,
 
 	*slots -= msg_slots;
 
-	if (mei_hbm_cl_flow_control_req(dev, cl)) {
-		cl->status = -ENODEV;
+	ret = mei_hbm_cl_flow_control_req(dev, cl);
+	if (ret) {
+		cl->status = ret;
 		cb->buf_idx = 0;
 		list_move_tail(&cb->list, &cmpl_list->list);
-		return -ENODEV;
+		return ret;
 	}
+
 	list_move_tail(&cb->list, &dev->read_list.list);
 
 	return 0;
@@ -254,6 +258,7 @@ static int mei_cl_irq_ioctl(struct mei_cl *cl, struct mei_cl_cb *cb,
 			   s32 *slots, struct mei_cl_cb *cmpl_list)
 {
 	struct mei_device *dev = cl->dev;
+	int ret;
 
 	u32 msg_slots =
 		mei_data2slots(sizeof(struct hbm_client_connect_request));
@@ -268,11 +273,12 @@ static int mei_cl_irq_ioctl(struct mei_cl *cl, struct mei_cl_cb *cb,
 
 	cl->state = MEI_FILE_CONNECTING;
 
-	if (mei_hbm_cl_connect_req(dev, cl)) {
-		cl->status = -ENODEV;
+	ret = mei_hbm_cl_connect_req(dev, cl);
+	if (ret) {
+		cl->status = ret;
 		cb->buf_idx = 0;
 		list_del(&cb->list);
-		return -ENODEV;
+		return ret;
 	}
 
 	list_move_tail(&cb->list, &dev->ctrl_rd_list.list);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [char-misc-next 3/7] mei: push credentials inside the irq write handler
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 1/7] mei: fix function names in debug prints Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 2/7] mei: propagate error from write routines instead of ENODEV Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 4/7] mei: mei_cl_unlink: no need to loop over dev list Tomas Winkler
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

this eventually allows as use a single write queue
both for control and data messages and removing possible
race

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c    |  9 +++++++++
 drivers/misc/mei/client.c    | 30 +++++++++++++++++++++++++-----
 drivers/misc/mei/interrupt.c |  5 -----
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index e4a4e2e..226c3f3 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -461,6 +461,15 @@ int mei_amthif_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 	u32 msg_slots = mei_data2slots(len);
 	int rets;
 
+	rets = mei_cl_flow_ctrl_creds(cl);
+	if (rets < 0)
+		return rets;
+
+	if (rets == 0) {
+		cl_dbg(dev, cl, "No flow control credentials: not sending.\n");
+		return 0;
+	}
+
 	mei_hdr.host_addr = cl->host_client_id;
 	mei_hdr.me_addr = cl->me_client_id;
 	mei_hdr.reserved = 0;
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index c59c8a2..88bcde4 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -696,12 +696,33 @@ err:
 int mei_cl_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 				     s32 *slots, struct mei_cl_cb *cmpl_list)
 {
-	struct mei_device *dev = cl->dev;
+	struct mei_device *dev;
+	struct mei_msg_data *buf;
 	struct mei_msg_hdr mei_hdr;
-	size_t len = cb->request_buffer.size - cb->buf_idx;
-	u32 msg_slots = mei_data2slots(len);
+	size_t len;
+	u32 msg_slots;
 	int rets;
 
+
+	if (WARN_ON(!cl || !cl->dev))
+		return -ENODEV;
+
+	dev = cl->dev;
+
+	buf = &cb->request_buffer;
+
+	rets = mei_cl_flow_ctrl_creds(cl);
+	if (rets < 0)
+		return rets;
+
+	if (rets == 0) {
+		cl_dbg(dev, cl,	"No flow control credentials: not sending.\n");
+		return 0;
+	}
+
+	len = buf->size - cb->buf_idx;
+	msg_slots = mei_data2slots(len);
+
 	mei_hdr.host_addr = cl->host_client_id;
 	mei_hdr.me_addr = cl->me_client_id;
 	mei_hdr.reserved = 0;
@@ -724,8 +745,7 @@ int mei_cl_irq_write_complete(struct mei_cl *cl, struct mei_cl_cb *cb,
 			cb->request_buffer.size, cb->buf_idx);
 
 	*slots -=  msg_slots;
-	rets = mei_write_message(dev, &mei_hdr,
-			cb->request_buffer.data + cb->buf_idx);
+	rets = mei_write_message(dev, &mei_hdr, buf->data + cb->buf_idx);
 	if (rets) {
 		cl->status = rets;
 		list_move_tail(&cb->list, &cmpl_list->list);
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index e4bb9ae..7a95c07 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -513,11 +513,6 @@ int mei_irq_write_handler(struct mei_device *dev, struct mei_cl_cb *cmpl_list)
 		cl = cb->cl;
 		if (cl == NULL)
 			continue;
-		if (mei_cl_flow_ctrl_creds(cl) <= 0) {
-			cl_dbg(dev, cl, "No flow control credentials, not sending.\n");
-			continue;
-		}
-
 		if (cl == &dev->iamthif_cl)
 			ret = mei_amthif_irq_write_complete(cl, cb,
 						&slots, cmpl_list);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [char-misc-next 4/7] mei: mei_cl_unlink: no need to loop over dev list
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
                   ` (2 preceding siblings ...)
  2013-09-16 20:44 ` [char-misc-next 3/7] mei: push credentials inside the irq write handler Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 5/7] mei: simplify mei_open error handling Tomas Winkler
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

we can call list_del_init regardless the client is
linked or not it is always properly initialized

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/client.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 88bcde4..653b318 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -312,7 +312,6 @@ int mei_cl_link(struct mei_cl *cl, int id)
 int mei_cl_unlink(struct mei_cl *cl)
 {
 	struct mei_device *dev;
-	struct mei_cl *pos, *next;
 
 	/* don't shout on error exit path */
 	if (!cl)
@@ -324,14 +323,10 @@ int mei_cl_unlink(struct mei_cl *cl)
 
 	dev = cl->dev;
 
-	list_for_each_entry_safe(pos, next, &dev->file_list, link) {
-		if (cl->host_client_id == pos->host_client_id) {
-			cl_dbg(dev, cl, "remove host client = %d, ME client = %d\n",
-				pos->host_client_id, pos->me_client_id);
-			list_del_init(&pos->link);
-			break;
-		}
-	}
+	cl_dbg(dev, cl, "unlink client");
+
+	list_del_init(&cl->link);
+
 	return 0;
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [char-misc-next 5/7] mei: simplify mei_open error handling
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
                   ` (3 preceding siblings ...)
  2013-09-16 20:44 ` [char-misc-next 4/7] mei: mei_cl_unlink: no need to loop over dev list Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 6/7] mei: revamp open handler counts Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 7/7] mei: amthif: mei_amthif_host_init: propagate errors from called functions Tomas Winkler
  6 siblings, 0 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

1. Perform simple checks first and only then attempt to allocate cl structure.
2. Remove open_handler_count test, this is already checked in mei_cl_link function
3. return -EMFILE instead of -ENOENT as expected by user space

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/client.c |  8 +++++++-
 drivers/misc/mei/main.c   | 31 ++++++++++++++-----------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 653b318..f62bf76 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -288,7 +288,13 @@ int mei_cl_link(struct mei_cl *cl, int id)
 
 	if (id >= MEI_CLIENTS_MAX) {
 		dev_err(&dev->pdev->dev, "id exceded %d", MEI_CLIENTS_MAX) ;
-		return -ENOENT;
+		return -EMFILE;
+	}
+
+	if (dev->open_handle_count >= MEI_MAX_OPEN_HANDLE_COUNT) {
+		dev_err(&dev->pdev->dev, "open_handle_count exceded %d",
+			MEI_MAX_OPEN_HANDLE_COUNT);
+		return -EMFILE;
 	}
 
 	dev->open_handle_count++;
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index c71420e..87ab5ca 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -60,48 +60,45 @@ static int mei_open(struct inode *inode, struct file *file)
 
 	int err;
 
-	err = -ENODEV;
 	if (!misc->parent)
-		goto out;
+		return -ENODEV;
 
 	pdev = container_of(misc->parent, struct pci_dev, dev);
 
 	dev = pci_get_drvdata(pdev);
 	if (!dev)
-		goto out;
+		return -ENODEV;
 
 	mutex_lock(&dev->device_lock);
-	err = -ENOMEM;
-	cl = mei_cl_allocate(dev);
-	if (!cl)
-		goto out_unlock;
+
+	cl = NULL;
 
 	err = -ENODEV;
 	if (dev->dev_state != MEI_DEV_ENABLED) {
 		dev_dbg(&dev->pdev->dev, "dev_state != MEI_ENABLED  dev_state = %s\n",
 		    mei_dev_state_str(dev->dev_state));
-		goto out_unlock;
-	}
-	err = -EMFILE;
-	if (dev->open_handle_count >= MEI_MAX_OPEN_HANDLE_COUNT) {
-		dev_err(&dev->pdev->dev, "open_handle_count exceded %d",
-			MEI_MAX_OPEN_HANDLE_COUNT);
-		goto out_unlock;
+		goto err_unlock;
 	}
 
+	err = -ENOMEM;
+	cl = mei_cl_allocate(dev);
+	if (!cl)
+		goto err_unlock;
+
+	/* open_handle_count check is handled in the mei_cl_link */
 	err = mei_cl_link(cl, MEI_HOST_CLIENT_ID_ANY);
 	if (err)
-		goto out_unlock;
+		goto err_unlock;
 
 	file->private_data = cl;
+
 	mutex_unlock(&dev->device_lock);
 
 	return nonseekable_open(inode, file);
 
-out_unlock:
+err_unlock:
 	mutex_unlock(&dev->device_lock);
 	kfree(cl);
-out:
 	return err;
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [char-misc-next 6/7] mei: revamp open handler counts
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
                   ` (4 preceding siblings ...)
  2013-09-16 20:44 ` [char-misc-next 5/7] mei: simplify mei_open error handling Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  2013-09-16 20:44 ` [char-misc-next 7/7] mei: amthif: mei_amthif_host_init: propagate errors from called functions Tomas Winkler
  6 siblings, 0 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

Make open counter to be incremented and decremented
from mei_cl_link and mei_cl_unlik function respectively

Nfc was assuming symmetric linking API and thus open handler
count was never decreased. This patch fixes that.
We need to add separate open hander count for amthif which
is handled out of link/unlink functions and doesn't break
the symmetric API.

Last we do not waste clients slots if amthif or wd are not present
in the device. we don't need to allocates slots ahead
it is all covered by link/unlink before the devices is responding
to user space connection and thus not racing on allocation

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c  |  5 +++--
 drivers/misc/mei/client.c  | 19 +++++++++++++++----
 drivers/misc/mei/init.c    |  5 -----
 drivers/misc/mei/main.c    |  6 +-----
 drivers/misc/mei/mei_dev.h |  1 +
 5 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index 226c3f3..4f259d4 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -58,6 +58,7 @@ void mei_amthif_reset_params(struct mei_device *dev)
 	dev->iamthif_state = MEI_IAMTHIF_IDLE;
 	dev->iamthif_timer = 0;
 	dev->iamthif_stall_timer = 0;
+	dev->iamthif_open_count = 0;
 }
 
 /**
@@ -731,8 +732,8 @@ static bool mei_clear_lists(struct mei_device *dev, struct file *file)
 */
 int mei_amthif_release(struct mei_device *dev, struct file *file)
 {
-	if (dev->open_handle_count > 0)
-		dev->open_handle_count--;
+	if (dev->iamthif_open_count > 0)
+		dev->iamthif_open_count--;
 
 	if (dev->iamthif_file_object == file &&
 	    dev->iamthif_state != MEI_IAMTHIF_IDLE) {
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index f62bf76..37ed142 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -275,6 +275,7 @@ struct mei_cl_cb *mei_cl_find_read_cb(struct mei_cl *cl)
 int mei_cl_link(struct mei_cl *cl, int id)
 {
 	struct mei_device *dev;
+	long open_handle_count;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -EINVAL;
@@ -291,7 +292,8 @@ int mei_cl_link(struct mei_cl *cl, int id)
 		return -EMFILE;
 	}
 
-	if (dev->open_handle_count >= MEI_MAX_OPEN_HANDLE_COUNT) {
+	open_handle_count = dev->open_handle_count + dev->iamthif_open_count;
+	if (open_handle_count >= MEI_MAX_OPEN_HANDLE_COUNT) {
 		dev_err(&dev->pdev->dev, "open_handle_count exceded %d",
 			MEI_MAX_OPEN_HANDLE_COUNT);
 		return -EMFILE;
@@ -331,6 +333,17 @@ int mei_cl_unlink(struct mei_cl *cl)
 
 	cl_dbg(dev, cl, "unlink client");
 
+	if (dev->open_handle_count > 0)
+		dev->open_handle_count--;
+
+	/* never clear the 0 bit */
+	if (cl->host_client_id)
+		clear_bit(cl->host_client_id, dev->host_clients_map);
+
+	list_del_init(&cl->link);
+
+	cl->state = MEI_FILE_INITIALIZING;
+
 	list_del_init(&cl->link);
 
 	return 0;
@@ -352,10 +365,8 @@ void mei_host_client_init(struct work_struct *work)
 	/*
 	 * Reserving the first three client IDs
 	 * 0: Reserved for MEI Bus Message communications
-	 * 1: Reserved for Watchdog
-	 * 2: Reserved for AMTHI
 	 */
-	bitmap_set(dev->host_clients_map, 0, 3);
+	bitmap_set(dev->host_clients_map, 0, 1);
 
 	for (i = 0; i < dev->me_clients_num; i++) {
 		client_props = &dev->me_clients[i].props;
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index eca24df..bcb4a6b 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -166,12 +166,7 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 		/* remove entry if already in list */
 		dev_dbg(&dev->pdev->dev, "remove iamthif and wd from the file list.\n");
 		mei_cl_unlink(&dev->wd_cl);
-		if (dev->open_handle_count > 0)
-			dev->open_handle_count--;
 		mei_cl_unlink(&dev->iamthif_cl);
-		if (dev->open_handle_count > 0)
-			dev->open_handle_count--;
-
 		mei_amthif_reset_params(dev);
 		memset(&dev->wr_ext_msg, 0, sizeof(dev->wr_ext_msg));
 	}
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 87ab5ca..9661a81 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -141,10 +141,6 @@ static int mei_release(struct inode *inode, struct file *file)
 	    cl->host_client_id,
 	    cl->me_client_id);
 
-	if (dev->open_handle_count > 0) {
-		clear_bit(cl->host_client_id, dev->host_clients_map);
-		dev->open_handle_count--;
-	}
 	mei_cl_unlink(cl);
 
 
@@ -498,11 +494,11 @@ static int mei_ioctl_connect_client(struct file *file,
 			rets = -ENODEV;
 			goto end;
 		}
-		clear_bit(cl->host_client_id, dev->host_clients_map);
 		mei_cl_unlink(cl);
 
 		kfree(cl);
 		cl = NULL;
+		dev->iamthif_open_count++;
 		file->private_data = &dev->iamthif_cl;
 
 		client = &data->out_client_properties;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 456b322..406f68e 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -414,6 +414,7 @@ struct mei_device {
 	struct file *iamthif_file_object;
 	struct mei_cl iamthif_cl;
 	struct mei_cl_cb *iamthif_current_cb;
+	long iamthif_open_count;
 	int iamthif_mtu;
 	unsigned long iamthif_timer;
 	u32 iamthif_stall_timer;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [char-misc-next 7/7] mei: amthif: mei_amthif_host_init: propagate errors from called functions
  2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
                   ` (5 preceding siblings ...)
  2013-09-16 20:44 ` [char-misc-next 6/7] mei: revamp open handler counts Tomas Winkler
@ 2013-09-16 20:44 ` Tomas Winkler
  6 siblings, 0 replies; 9+ messages in thread
From: Tomas Winkler @ 2013-09-16 20:44 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

propagate error codes from called functions, they are correct.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/amthif.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index 4f259d4..d22c686 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -79,8 +79,10 @@ int mei_amthif_host_init(struct mei_device *dev)
 
 	i = mei_me_cl_by_uuid(dev, &mei_amthif_guid);
 	if (i < 0) {
-		dev_info(&dev->pdev->dev, "amthif: failed to find the client\n");
-		return -ENOENT;
+		ret = i;
+		dev_info(&dev->pdev->dev,
+			"amthif: failed to find the client %d\n", ret);
+		return ret;
 	}
 
 	cl->me_client_id = dev->me_clients[i].client_id;
@@ -107,8 +109,9 @@ int mei_amthif_host_init(struct mei_device *dev)
 	ret = mei_cl_link(cl, MEI_IAMTHIF_HOST_CLIENT_ID);
 
 	if (ret < 0) {
-		dev_err(&dev->pdev->dev, "amthif: failed link client\n");
-		return -ENOENT;
+		dev_err(&dev->pdev->dev,
+			"amthif: failed link client %d\n", ret);
+		return ret;
 	}
 
 	cl->state = MEI_FILE_CONNECTING;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [char-misc-next 1/7] mei: fix function names in debug prints
  2013-09-16 20:44 ` [char-misc-next 1/7] mei: fix function names in debug prints Tomas Winkler
@ 2013-09-26 15:40   ` Greg KH
  0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2013-09-26 15:40 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: arnd, linux-kernel, Alexander Usyskin

On Mon, Sep 16, 2013 at 11:44:42PM +0300, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Fix calling function names in debug prints.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/interrupt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
> index d27804e..be42c70 100644
> --- a/drivers/misc/mei/interrupt.c
> +++ b/drivers/misc/mei/interrupt.c
> @@ -343,14 +343,14 @@ int mei_irq_read_handler(struct mei_device *dev,
>  
>  	/* decide where to read the message too */
>  	if (!mei_hdr->host_addr) {
> -		dev_dbg(&dev->pdev->dev, "call mei_irq_thread_read_bus_message.\n");
> +		dev_dbg(&dev->pdev->dev, "call mei_hbm_dispatch.\n");
>  		mei_hbm_dispatch(dev, mei_hdr);
> -		dev_dbg(&dev->pdev->dev, "end mei_irq_thread_read_bus_message.\n");
> +		dev_dbg(&dev->pdev->dev, "end mei_hbm_dispatch.\n");

Stuff like this can just be deleted, use the in-kernel tracing
functionality if you really care about this type of thing.

I'll take this for now, but in the future, just remove these lines
please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-09-26 15:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-16 20:44 [char-misc-next 0/7] mei minor fixes and cleanups Tomas Winkler
2013-09-16 20:44 ` [char-misc-next 1/7] mei: fix function names in debug prints Tomas Winkler
2013-09-26 15:40   ` Greg KH
2013-09-16 20:44 ` [char-misc-next 2/7] mei: propagate error from write routines instead of ENODEV Tomas Winkler
2013-09-16 20:44 ` [char-misc-next 3/7] mei: push credentials inside the irq write handler Tomas Winkler
2013-09-16 20:44 ` [char-misc-next 4/7] mei: mei_cl_unlink: no need to loop over dev list Tomas Winkler
2013-09-16 20:44 ` [char-misc-next 5/7] mei: simplify mei_open error handling Tomas Winkler
2013-09-16 20:44 ` [char-misc-next 6/7] mei: revamp open handler counts Tomas Winkler
2013-09-16 20:44 ` [char-misc-next 7/7] mei: amthif: mei_amthif_host_init: propagate errors from called functions Tomas Winkler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).