linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [char-misc-next 01/11] mei: consume flow control on the first chunk of writing
@ 2015-05-07 12:53 Tomas Winkler
  2015-05-07 12:53 ` [char-misc-next 02/11] mei: request autosuspend at the end of write Tomas Winkler
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:53 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Alexander Usyskin, Tomas Winkler

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

Consume the write flow control on the first chunk of the write instead
of on the buffer completion.
We can safely assume that the consequent chunks have the flow control
granted.

This addresses two issues:

1. Blocks other callbacks from the same client riding on the client's
flow control and prevents interleaving of messages as FW cannot distinguish
between two messages from the same client.

2. Fixes single buffer flow control arbitration in a clean way, without
connection/disconnection book keeping

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

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 36706705dfdc..ce88c2199b2c 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -755,9 +755,6 @@ void mei_cl_set_disconnected(struct mei_cl *cl)
 	if (!WARN_ON(cl->me_cl->connect_count == 0))
 		cl->me_cl->connect_count--;
 
-	if (cl->me_cl->connect_count == 0)
-		cl->me_cl->mei_flow_ctrl_creds = 0;
-
 	mei_me_cl_put(cl->me_cl);
 	cl->me_cl = NULL;
 }
@@ -1272,6 +1269,7 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
 	u32 msg_slots;
 	int slots;
 	int rets;
+	bool first_chunk;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -ENODEV;
@@ -1280,7 +1278,9 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
 
 	buf = &cb->buf;
 
-	rets = mei_cl_flow_ctrl_creds(cl);
+	first_chunk = cb->buf_idx == 0;
+
+	rets = first_chunk ? mei_cl_flow_ctrl_creds(cl) : 1;
 	if (rets < 0)
 		return rets;
 
@@ -1327,12 +1327,14 @@ int mei_cl_irq_write(struct mei_cl *cl, struct mei_cl_cb *cb,
 	cb->buf_idx += mei_hdr.length;
 	cb->completed = mei_hdr.msg_complete == 1;
 
-	if (mei_hdr.msg_complete) {
+	if (first_chunk) {
 		if (mei_cl_flow_ctrl_reduce(cl))
 			return -EIO;
-		list_move_tail(&cb->list, &dev->write_waiting_list.list);
 	}
 
+	if (mei_hdr.msg_complete)
+		list_move_tail(&cb->list, &dev->write_waiting_list.list);
+
 	return 0;
 }
 
@@ -1411,21 +1413,19 @@ int mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, bool blocking)
 	if (rets)
 		goto err;
 
+	rets = mei_cl_flow_ctrl_reduce(cl);
+	if (rets)
+		goto err;
+
 	cl->writing_state = MEI_WRITING;
 	cb->buf_idx = mei_hdr.length;
 	cb->completed = mei_hdr.msg_complete == 1;
 
 out:
-	if (mei_hdr.msg_complete) {
-		rets = mei_cl_flow_ctrl_reduce(cl);
-		if (rets < 0)
-			goto err;
-
+	if (mei_hdr.msg_complete)
 		list_add_tail(&cb->list, &dev->write_waiting_list.list);
-	} else {
+	else
 		list_add_tail(&cb->list, &dev->write_list.list);
-	}
-
 
 	if (blocking && cl->writing_state != MEI_WRITE_COMPLETE) {
 
-- 
2.1.0


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

* [char-misc-next 02/11] mei: request autosuspend at the end of write
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
@ 2015-05-07 12:53 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 03/11] mei: add also write waiting list to runtime pm blockers Tomas Winkler
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:53 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Alexander Usyskin, Tomas Winkler

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

On longer non-blocking write might not complete at the end of
autosuspend expiration, therefore we request autosuspend
again on the write completion.

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

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index ce88c2199b2c..7a5a6636f0fd 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -1459,12 +1459,18 @@ err:
  */
 void mei_cl_complete(struct mei_cl *cl, struct mei_cl_cb *cb)
 {
+	struct mei_device *dev = cl->dev;
+
 	switch (cb->fop_type) {
 	case MEI_FOP_WRITE:
 		mei_io_cb_free(cb);
 		cl->writing_state = MEI_WRITE_COMPLETE;
-		if (waitqueue_active(&cl->tx_wait))
+		if (waitqueue_active(&cl->tx_wait)) {
 			wake_up_interruptible(&cl->tx_wait);
+		} else {
+			pm_runtime_mark_last_busy(dev->dev);
+			pm_request_autosuspend(dev->dev);
+		}
 		break;
 
 	case MEI_FOP_READ:
-- 
2.1.0


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

* [char-misc-next 03/11] mei: add also write waiting list to runtime pm blockers
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
  2015-05-07 12:53 ` [char-misc-next 02/11] mei: request autosuspend at the end of write Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 04/11] uuid: extract macros for assigning raw arrays Tomas Winkler
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

The io callback is clear from write_waitling_list after
we receive interrupt from the hw to ack the write completion.
We need to wait for this interrupt deliver before we try
to enter low power state

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/init.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 97353cf8d9b6..94514b2c7a50 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -361,13 +361,15 @@ bool mei_write_is_idle(struct mei_device *dev)
 {
 	bool idle = (dev->dev_state == MEI_DEV_ENABLED &&
 		list_empty(&dev->ctrl_wr_list.list) &&
-		list_empty(&dev->write_list.list));
+		list_empty(&dev->write_list.list)   &&
+		list_empty(&dev->write_waiting_list.list));
 
-	dev_dbg(dev->dev, "write pg: is idle[%d] state=%s ctrl=%d write=%d\n",
+	dev_dbg(dev->dev, "write pg: is idle[%d] state=%s ctrl=%01d write=%01d wwait=%01d\n",
 		idle,
 		mei_dev_state_str(dev->dev_state),
 		list_empty(&dev->ctrl_wr_list.list),
-		list_empty(&dev->write_list.list));
+		list_empty(&dev->write_list.list),
+		list_empty(&dev->write_waiting_list.list));
 
 	return idle;
 }
-- 
2.1.0


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

* [char-misc-next 04/11] uuid: extract macros for assigning raw arrays
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
  2015-05-07 12:53 ` [char-misc-next 02/11] mei: request autosuspend at the end of write Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 03/11] mei: add also write waiting list to runtime pm blockers Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 05/11] mei: bus: report also uuid in module alias Tomas Winkler
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

we need 16 byte arrays for handling uuids in
device ids

Cc:linux-api@vger.kernel.org
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 include/uapi/linux/uuid.h | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 786f0773cc33..487f098c8517 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -32,27 +32,34 @@ typedef struct {
 	__u8 b[16];
 } uuid_be;
 
-#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
-((uuid_le)								\
-{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
-   (b) & 0xff, ((b) >> 8) & 0xff,					\
-   (c) & 0xff, ((c) >> 8) & 0xff,					\
-   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+#define __UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)                     \
+	{(a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff,\
+	  (b) & 0xff, ((b) >> 8) & 0xff,                                       \
+	  (c) & 0xff, ((c) >> 8) & 0xff,                                       \
+	  (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7)}
+
+#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)	\
+	((uuid_le) {__UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)})
+
+#define __UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)                     \
+	{((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff,\
+	  ((b) >> 8) & 0xff, (b) & 0xff,                                       \
+	  ((c) >> 8) & 0xff, (c) & 0xff,                                       \
+	  (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7)}
 
 #define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
-((uuid_be)								\
-{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
-   ((b) >> 8) & 0xff, (b) & 0xff,					\
-   ((c) >> 8) & 0xff, (c) & 0xff,					\
-   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+	((uuid_be) {__UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)})
+
+#define __NULL_UUID_LE							\
+	__UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
+		  0x00, 0x00, 0x00, 0x00)
 
-#define NULL_UUID_LE							\
-	UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
-		0x00, 0x00, 0x00, 0x00)
+#define NULL_UUID_LE ((uuid_le) {__NULL_UUID_LE})
 
-#define NULL_UUID_BE							\
-	UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
-		0x00, 0x00, 0x00, 0x00)
+#define __NULL_UUID_BE							\
+	__UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
+		  0x00, 0x00, 0x00, 0x00)
 
+#define NULL_UUID_BE ((uuid_be) {__NULL_UUID_BE})
 
 #endif /* _UAPI_LINUX_UUID_H_ */
-- 
2.1.0


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

* [char-misc-next 05/11] mei: bus: report also uuid in module alias
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (2 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 04/11] uuid: extract macros for assigning raw arrays Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 06/11] mei: bus: add name and uuid into device attributes Tomas Winkler
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, linux-api, Samuel Ortiz

In order to automate modules matching add device uuid
which is reported in client enumeration, keep also
the name that is needed in for nfc distinguishing radio vendor

Report mei:name:uuid

Cc: linux-api@vger.kernel.org
Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-mei |  2 +-
 drivers/misc/mei/bus.c                  | 42 ++++++++++++++++++++++++++-------
 drivers/misc/mei/mei_dev.h              |  2 ++
 drivers/nfc/mei_phy.h                   |  3 +++
 drivers/nfc/microread/mei.c             |  2 +-
 drivers/nfc/pn544/mei.c                 |  2 +-
 include/linux/mod_devicetable.h         | 13 ++++++++++
 scripts/mod/devicetable-offsets.c       |  1 +
 scripts/mod/file2alias.c                | 18 ++++++++++++--
 9 files changed, 72 insertions(+), 13 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-mei b/Documentation/ABI/testing/sysfs-bus-mei
index 2066f0bbd453..91967a70313a 100644
--- a/Documentation/ABI/testing/sysfs-bus-mei
+++ b/Documentation/ABI/testing/sysfs-bus-mei
@@ -4,4 +4,4 @@ KernelVersion:	3.10
 Contact:	Samuel Ortiz <sameo@linux.intel.com>
 		linux-mei@linux.intel.com
 Description:	Stores the same MODALIAS value emitted by uevent
-		Format: mei:<mei device name>
+		Format: mei:<mei device name>:<device uuid>:
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 1101d6efaf27..17b00baa53b1 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -30,23 +30,40 @@
 #define to_mei_cl_driver(d) container_of(d, struct mei_cl_driver, driver)
 #define to_mei_cl_device(d) container_of(d, struct mei_cl_device, dev)
 
+static inline uuid_le uuid_le_cast(const __u8 uuid[16])
+{
+	return *(uuid_le *)uuid;
+}
+
 static int mei_cl_device_match(struct device *dev, struct device_driver *drv)
 {
 	struct mei_cl_device *device = to_mei_cl_device(dev);
 	struct mei_cl_driver *driver = to_mei_cl_driver(drv);
 	const struct mei_cl_device_id *id;
+	const uuid_le *uuid;
+	const char *name;
 
 	if (!device)
 		return 0;
 
+	uuid = mei_me_cl_uuid(device->me_cl);
+	name = device->name;
+
 	if (!driver || !driver->id_table)
 		return 0;
 
 	id = driver->id_table;
 
-	while (id->name[0]) {
-		if (!strncmp(dev_name(dev), id->name, sizeof(id->name)))
-			return 1;
+	while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) {
+
+		if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) {
+			if (id->name[0]) {
+				if (!strncmp(name, id->name, sizeof(id->name)))
+					return 1;
+			} else {
+				return 1;
+			}
+		}
 
 		id++;
 	}
@@ -69,7 +86,7 @@ static int mei_cl_device_probe(struct device *dev)
 
 	dev_dbg(dev, "Device probe\n");
 
-	strlcpy(id.name, dev_name(dev), sizeof(id.name));
+	strlcpy(id.name, device->name, sizeof(id.name));
 
 	return driver->probe(device, &id);
 }
@@ -100,9 +117,12 @@ static int mei_cl_device_remove(struct device *dev)
 static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 			     char *buf)
 {
-	int len;
+	struct mei_cl_device *device = to_mei_cl_device(dev);
+	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
+	size_t len;
 
-	len = snprintf(buf, PAGE_SIZE, "mei:%s\n", dev_name(dev));
+	len = snprintf(buf, PAGE_SIZE, "mei:%s:" MEI_CL_UUID_FMT ":",
+		device->name, MEI_CL_UUID_ARGS(uuid->b));
 
 	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
 }
@@ -116,7 +136,11 @@ ATTRIBUTE_GROUPS(mei_cl_dev);
 
 static int mei_cl_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-	if (add_uevent_var(env, "MODALIAS=mei:%s", dev_name(dev)))
+	struct mei_cl_device *device = to_mei_cl_device(dev);
+	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
+
+	if (add_uevent_var(env, "MODALIAS=mei:%s:" MEI_CL_UUID_FMT ":",
+		device->name, MEI_CL_UUID_ARGS(uuid->b)))
 		return -ENOMEM;
 
 	return 0;
@@ -185,7 +209,9 @@ struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
 	device->dev.bus = &mei_cl_bus_type;
 	device->dev.type = &mei_cl_device_type;
 
-	dev_set_name(&device->dev, "%s", name);
+	strlcpy(device->name, name, sizeof(device->name));
+
+	dev_set_name(&device->dev, "mei:%s:%pUl", name, mei_me_cl_uuid(me_cl));
 
 	status = device_register(&device->dev);
 	if (status) {
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 79ab78184523..ab719e674edf 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -376,6 +376,7 @@ struct mei_cl *mei_cl_bus_find_cl_by_uuid(struct mei_device *dev, uuid_le uuid);
  * @dev: linux driver model device pointer
  * @me_cl: me client
  * @cl: mei client
+ * @name: device name
  * @ops: ME transport ops
  * @event_work: async work to execute event callback
  * @event_cb: Drivers register this callback to get asynchronous ME
@@ -389,6 +390,7 @@ struct mei_cl_device {
 
 	struct mei_me_client *me_cl;
 	struct mei_cl *cl;
+	char name[MEI_CL_NAME_SIZE];
 
 	const struct mei_cl_ops *ops;
 
diff --git a/drivers/nfc/mei_phy.h b/drivers/nfc/mei_phy.h
index d669900f8278..06608c28ff14 100644
--- a/drivers/nfc/mei_phy.h
+++ b/drivers/nfc/mei_phy.h
@@ -3,7 +3,10 @@
 
 #include <linux/mei_cl_bus.h>
 #include <net/nfc/hci.h>
+#include <linux/uuid.h>
 
+#define MEI_NFC_UUID __UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
+		0x94, 0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
 #define MEI_NFC_HEADER_SIZE 10
 #define MEI_NFC_MAX_HCI_PAYLOAD 300
 
diff --git a/drivers/nfc/microread/mei.c b/drivers/nfc/microread/mei.c
index 2d1395be64ae..f9f5fc97cdd7 100644
--- a/drivers/nfc/microread/mei.c
+++ b/drivers/nfc/microread/mei.c
@@ -67,7 +67,7 @@ static int microread_mei_remove(struct mei_cl_device *device)
 }
 
 static struct mei_cl_device_id microread_mei_tbl[] = {
-	{ MICROREAD_DRIVER_NAME },
+	{ MICROREAD_DRIVER_NAME, MEI_NFC_UUID},
 
 	/* required last entry */
 	{ }
diff --git a/drivers/nfc/pn544/mei.c b/drivers/nfc/pn544/mei.c
index 330cd4031009..101a37e12efa 100644
--- a/drivers/nfc/pn544/mei.c
+++ b/drivers/nfc/pn544/mei.c
@@ -67,7 +67,7 @@ static int pn544_mei_remove(struct mei_cl_device *device)
 }
 
 static struct mei_cl_device_id pn544_mei_tbl[] = {
-	{ PN544_DRIVER_NAME },
+	{ PN544_DRIVER_NAME, MEI_NFC_UUID},
 
 	/* required last entry */
 	{ }
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 3bfd56778c29..2d2b2b571d61 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -599,9 +599,22 @@ struct ipack_device_id {
 
 #define MEI_CL_MODULE_PREFIX "mei:"
 #define MEI_CL_NAME_SIZE 32
+#define MEI_CL_UUID_FMT "%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x"
+#define MEI_CL_UUID_ARGS(_u) \
+	_u[0], _u[1], _u[2], _u[3], _u[4], _u[5], _u[6], _u[7], \
+	_u[8], _u[9], _u[10], _u[11], _u[12], _u[13], _u[14], _u[15]
 
+/**
+ * struct mei_cl_device_id - MEI client device identifier
+ * @name: helper name
+ * @uuid: client uuid
+ * @driver_info: information used by the driver.
+ *
+ * identifies mei client device by uuid and name
+ */
 struct mei_cl_device_id {
 	char name[MEI_CL_NAME_SIZE];
+	__u8 uuid[16];
 	kernel_ulong_t driver_info;
 };
 
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index fce36d0f6898..091f6290a651 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -182,6 +182,7 @@ int main(void)
 
 	DEVID(mei_cl_device_id);
 	DEVID_FIELD(mei_cl_device_id, name);
+	DEVID_FIELD(mei_cl_device_id, uuid);
 
 	DEVID(rio_device_id);
 	DEVID_FIELD(rio_device_id, did);
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 78691d51a479..62c517f4b592 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -131,6 +131,15 @@ static inline void add_wildcard(char *str)
 		strcat(str + len, "*");
 }
 
+static inline void add_uuid(char *str, __u8 uuid[16])
+{
+	int len = strlen(str);
+	int i;
+
+	for (i = 0; i < 16; i++)
+		sprintf(str + len + (i << 1), "%02x", uuid[i]);
+}
+
 /**
  * Check that sizeof(device_id type) are consistent with size of section
  * in .o file. If in-consistent then userspace and kernel does not agree
@@ -1160,13 +1169,18 @@ static int do_cpu_entry(const char *filename, void *symval, char *alias)
 }
 ADD_TO_DEVTABLE("cpu", cpu_feature, do_cpu_entry);
 
-/* Looks like: mei:S */
+/* Looks like: mei:S:uuid */
 static int do_mei_entry(const char *filename, void *symval,
 			char *alias)
 {
 	DEF_FIELD_ADDR(symval, mei_cl_device_id, name);
+	DEF_FIELD_ADDR(symval, mei_cl_device_id, uuid);
+
+	sprintf(alias, MEI_CL_MODULE_PREFIX);
+	sprintf(alias + strlen(alias), "%s:",  (*name)[0]  ? *name : "*");
+	add_uuid(alias, *uuid);
 
-	sprintf(alias, MEI_CL_MODULE_PREFIX "%s", *name);
+	strcat(alias, ":*");
 
 	return 1;
 }
-- 
2.1.0


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

* [char-misc-next 06/11] mei: bus: add name and uuid into device attributes
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (3 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 05/11] mei: bus: report also uuid in module alias Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 07/11] NFC: mei_phy: move all nfc logic from mei driver to nfc Tomas Winkler
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, linux-api

Export name and uuid via sysfs and uevent

Cc: linux-api@vger.kernel.org
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-mei | 14 ++++++++++++++
 drivers/misc/mei/bus.c                  | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-mei b/Documentation/ABI/testing/sysfs-bus-mei
index 91967a70313a..20e4d1638bac 100644
--- a/Documentation/ABI/testing/sysfs-bus-mei
+++ b/Documentation/ABI/testing/sysfs-bus-mei
@@ -5,3 +5,17 @@ Contact:	Samuel Ortiz <sameo@linux.intel.com>
 		linux-mei@linux.intel.com
 Description:	Stores the same MODALIAS value emitted by uevent
 		Format: mei:<mei device name>:<device uuid>:
+
+What:		/sys/bus/mei/devices/.../name
+Date:		May 2015
+KernelVersion:	4.2
+Contact:	Tomas Winkler <tomas.winkler@intel.com>
+Description:	Stores mei client device name
+		Format: string
+
+What:		/sys/bus/mei/devices/.../uuid
+Date:		May 2015
+KernelVersion:	4.2
+Contact:	Tomas Winkler <tomas.winkler@intel.com>
+Description:	Stores mei client device uuid
+		Format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 17b00baa53b1..e76d94aa6e12 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -114,6 +114,31 @@ static int mei_cl_device_remove(struct device *dev)
 	return driver->remove(device);
 }
 
+static ssize_t name_show(struct device *dev, struct device_attribute *a,
+			     char *buf)
+{
+	struct mei_cl_device *device = to_mei_cl_device(dev);
+	size_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "%s", device->name);
+
+	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+}
+static DEVICE_ATTR_RO(name);
+
+static ssize_t uuid_show(struct device *dev, struct device_attribute *a,
+			     char *buf)
+{
+	struct mei_cl_device *device = to_mei_cl_device(dev);
+	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
+	size_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "%pUl", uuid);
+
+	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+}
+static DEVICE_ATTR_RO(uuid);
+
 static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 			     char *buf)
 {
@@ -129,6 +154,8 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 static DEVICE_ATTR_RO(modalias);
 
 static struct attribute *mei_cl_dev_attrs[] = {
+	&dev_attr_name.attr,
+	&dev_attr_uuid.attr,
 	&dev_attr_modalias.attr,
 	NULL,
 };
@@ -139,6 +166,12 @@ static int mei_cl_uevent(struct device *dev, struct kobj_uevent_env *env)
 	struct mei_cl_device *device = to_mei_cl_device(dev);
 	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
 
+	if (add_uevent_var(env, "MEI_CL_UUID=%pUl", uuid))
+		return -ENOMEM;
+
+	if (add_uevent_var(env, "MEI_CL_NAME=%s", device->name))
+		return -ENOMEM;
+
 	if (add_uevent_var(env, "MODALIAS=mei:%s:" MEI_CL_UUID_FMT ":",
 		device->name, MEI_CL_UUID_ARGS(uuid->b)))
 		return -ENOMEM;
-- 
2.1.0


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

* [char-misc-next 07/11] NFC: mei_phy: move all nfc logic from mei driver to nfc
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (4 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 06/11] mei: bus: add name and uuid into device attributes Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 08/11] mei: bus: kill mei_cl_ops Tomas Winkler
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, Samuel Ortiz

move nfc logic to mei_phy module, we prefer as much as
possible not to deal with a particualr client protocol
in the mei generic infrasutcutre

Cc: Samuel Ortiz <sameo@linux.intel.com> (supporter:NFC SUBSYSTEM)
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c     |  36 +-----
 drivers/misc/mei/mei_dev.h |   7 +-
 drivers/misc/mei/nfc.c     | 175 +--------------------------
 drivers/nfc/mei_phy.c      | 293 ++++++++++++++++++++++++++++++++++++++++-----
 drivers/nfc/mei_phy.h      |  35 ++++--
 5 files changed, 304 insertions(+), 242 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index e76d94aa6e12..de8fd089a8a4 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -220,8 +220,7 @@ struct mei_cl *mei_cl_bus_find_cl_by_uuid(struct mei_device *dev,
 struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
 					struct mei_me_client *me_cl,
 					struct mei_cl *cl,
-					char *name,
-					struct mei_cl_ops *ops)
+					char *name)
 {
 	struct mei_cl_device *device;
 	int status;
@@ -235,9 +234,8 @@ struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
 		kfree(device);
 		return NULL;
 	}
-	device->cl = cl;
-	device->ops = ops;
 
+	device->cl = cl;
 	device->dev.parent = dev->dev;
 	device->dev.bus = &mei_cl_bus_type;
 	device->dev.type = &mei_cl_device_type;
@@ -294,7 +292,7 @@ void mei_cl_driver_unregister(struct mei_cl_driver *driver)
 }
 EXPORT_SYMBOL_GPL(mei_cl_driver_unregister);
 
-static ssize_t ___mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
+ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 			bool blocking)
 {
 	struct mei_device *dev;
@@ -408,16 +406,6 @@ out:
 	return rets;
 }
 
-inline ssize_t __mei_cl_async_send(struct mei_cl *cl, u8 *buf, size_t length)
-{
-	return ___mei_cl_send(cl, buf, length, 0);
-}
-
-inline ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length)
-{
-	return ___mei_cl_send(cl, buf, length, 1);
-}
-
 ssize_t mei_cl_send(struct mei_cl_device *device, u8 *buf, size_t length)
 {
 	struct mei_cl *cl = device->cl;
@@ -425,23 +413,17 @@ ssize_t mei_cl_send(struct mei_cl_device *device, u8 *buf, size_t length)
 	if (cl == NULL)
 		return -ENODEV;
 
-	if (device->ops && device->ops->send)
-		return device->ops->send(device, buf, length);
-
-	return __mei_cl_send(cl, buf, length);
+	return __mei_cl_send(cl, buf, length, 1);
 }
 EXPORT_SYMBOL_GPL(mei_cl_send);
 
 ssize_t mei_cl_recv(struct mei_cl_device *device, u8 *buf, size_t length)
 {
-	struct mei_cl *cl =  device->cl;
+	struct mei_cl *cl = device->cl;
 
 	if (cl == NULL)
 		return -ENODEV;
 
-	if (device->ops && device->ops->recv)
-		return device->ops->recv(device, buf, length);
-
 	return __mei_cl_recv(cl, buf, length);
 }
 EXPORT_SYMBOL_GPL(mei_cl_recv);
@@ -522,10 +504,7 @@ int mei_cl_enable_device(struct mei_cl_device *device)
 	if (device->event_cb)
 		mei_cl_read_start(device->cl, 0, NULL);
 
-	if (!device->ops || !device->ops->enable)
-		return 0;
-
-	return device->ops->enable(device);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(mei_cl_enable_device);
 
@@ -540,9 +519,6 @@ int mei_cl_disable_device(struct mei_cl_device *device)
 
 	dev = cl->dev;
 
-	if (device->ops && device->ops->disable)
-		device->ops->disable(device);
-
 	device->event_cb = NULL;
 
 	mutex_lock(&dev->device_lock);
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index ab719e674edf..2175bff2730f 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -352,12 +352,11 @@ struct mei_cl_ops {
 struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
 					struct mei_me_client *me_cl,
 					struct mei_cl *cl,
-					char *name,
-					struct mei_cl_ops *ops);
+					char *name);
 void mei_cl_remove_device(struct mei_cl_device *device);
 
-ssize_t __mei_cl_async_send(struct mei_cl *cl, u8 *buf, size_t length);
-ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length);
+ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
+			bool blocking);
 ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
 void mei_cl_bus_rx_event(struct mei_cl *cl);
 void mei_cl_bus_remove_devices(struct mei_device *dev);
diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index e2a6ba0236c8..b983c4ecad38 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -95,28 +95,21 @@ struct mei_nfc_hci_hdr {
  * @cl: NFC host client
  * @cl_info: NFC info host client
  * @init_work: perform connection to the info client
- * @send_wq: send completion wait queue
  * @fw_ivn: NFC Interface Version Number
  * @vendor_id: NFC manufacturer ID
  * @radio_type: NFC radio type
  * @bus_name: bus name
  *
- * @req_id:  message counter
- * @recv_req_id: reception message counter
  */
 struct mei_nfc_dev {
 	struct mei_me_client *me_cl;
 	struct mei_cl *cl;
 	struct mei_cl *cl_info;
 	struct work_struct init_work;
-	wait_queue_head_t send_wq;
 	u8 fw_ivn;
 	u8 vendor_id;
 	u8 radio_type;
 	char *bus_name;
-
-	u16 req_id;
-	u16 recv_req_id;
 };
 
 /* UUIDs for NFC F/W clients */
@@ -202,73 +195,6 @@ static int mei_nfc_build_bus_name(struct mei_nfc_dev *ndev)
 	return 0;
 }
 
-static int mei_nfc_connect(struct mei_nfc_dev *ndev)
-{
-	struct mei_device *dev;
-	struct mei_cl *cl;
-	struct mei_nfc_cmd *cmd, *reply;
-	struct mei_nfc_connect *connect;
-	struct mei_nfc_connect_resp *connect_resp;
-	size_t connect_length, connect_resp_length;
-	int bytes_recv, ret;
-
-	cl = ndev->cl;
-	dev = cl->dev;
-
-	connect_length = sizeof(struct mei_nfc_cmd) +
-			sizeof(struct mei_nfc_connect);
-
-	connect_resp_length = sizeof(struct mei_nfc_cmd) +
-			sizeof(struct mei_nfc_connect_resp);
-
-	cmd = kzalloc(connect_length, GFP_KERNEL);
-	if (!cmd)
-		return -ENOMEM;
-	connect = (struct mei_nfc_connect *)cmd->data;
-
-	reply = kzalloc(connect_resp_length, GFP_KERNEL);
-	if (!reply) {
-		kfree(cmd);
-		return -ENOMEM;
-	}
-
-	connect_resp = (struct mei_nfc_connect_resp *)reply->data;
-
-	cmd->command = MEI_NFC_CMD_MAINTENANCE;
-	cmd->data_size = 3;
-	cmd->sub_command = MEI_NFC_SUBCMD_CONNECT;
-	connect->fw_ivn = ndev->fw_ivn;
-	connect->vendor_id = ndev->vendor_id;
-
-	ret = __mei_cl_send(cl, (u8 *)cmd, connect_length);
-	if (ret < 0) {
-		dev_err(dev->dev, "Could not send connect cmd\n");
-		goto err;
-	}
-
-	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, connect_resp_length);
-	if (bytes_recv < 0) {
-		dev_err(dev->dev, "Could not read connect response\n");
-		ret = bytes_recv;
-		goto err;
-	}
-
-	dev_info(dev->dev, "IVN 0x%x Vendor ID 0x%x\n",
-		 connect_resp->fw_ivn, connect_resp->vendor_id);
-
-	dev_info(dev->dev, "ME FW %d.%d.%d.%d\n",
-		connect_resp->me_major, connect_resp->me_minor,
-		connect_resp->me_hotfix, connect_resp->me_build);
-
-	ret = 0;
-
-err:
-	kfree(reply);
-	kfree(cmd);
-
-	return ret;
-}
-
 static int mei_nfc_if_version(struct mei_nfc_dev *ndev)
 {
 	struct mei_device *dev;
@@ -288,7 +214,7 @@ static int mei_nfc_if_version(struct mei_nfc_dev *ndev)
 	cmd.data_size = 1;
 	cmd.sub_command = MEI_NFC_SUBCMD_IF_VERSION;
 
-	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd));
+	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd), 1);
 	if (ret < 0) {
 		dev_err(dev->dev, "Could not send IF version cmd\n");
 		return ret;
@@ -320,100 +246,6 @@ err:
 	return ret;
 }
 
-static int mei_nfc_enable(struct mei_cl_device *cldev)
-{
-	struct mei_device *dev;
-	struct mei_nfc_dev *ndev;
-	int ret;
-
-	ndev = (struct mei_nfc_dev *)cldev->priv_data;
-	dev = ndev->cl->dev;
-
-	ret = mei_nfc_connect(ndev);
-	if (ret < 0) {
-		dev_err(dev->dev, "Could not connect to NFC");
-		return ret;
-	}
-
-	return 0;
-}
-
-static int mei_nfc_disable(struct mei_cl_device *cldev)
-{
-	return 0;
-}
-
-static int mei_nfc_send(struct mei_cl_device *cldev, u8 *buf, size_t length)
-{
-	struct mei_device *dev;
-	struct mei_nfc_dev *ndev;
-	struct mei_nfc_hci_hdr *hdr;
-	u8 *mei_buf;
-	int err;
-
-	ndev = (struct mei_nfc_dev *) cldev->priv_data;
-	dev = ndev->cl->dev;
-
-	err = -ENOMEM;
-	mei_buf = kzalloc(length + MEI_NFC_HEADER_SIZE, GFP_KERNEL);
-	if (!mei_buf)
-		goto out;
-
-	hdr = (struct mei_nfc_hci_hdr *) mei_buf;
-	hdr->cmd = MEI_NFC_CMD_HCI_SEND;
-	hdr->status = 0;
-	hdr->req_id = ndev->req_id;
-	hdr->reserved = 0;
-	hdr->data_size = length;
-
-	memcpy(mei_buf + MEI_NFC_HEADER_SIZE, buf, length);
-	err = __mei_cl_send(ndev->cl, mei_buf, length + MEI_NFC_HEADER_SIZE);
-	if (err < 0)
-		goto out;
-
-	if (!wait_event_interruptible_timeout(ndev->send_wq,
-				ndev->recv_req_id == ndev->req_id, HZ)) {
-		dev_err(dev->dev, "NFC MEI command timeout\n");
-		err = -ETIME;
-	} else {
-		ndev->req_id++;
-	}
-out:
-	kfree(mei_buf);
-	return err;
-}
-
-static int mei_nfc_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
-{
-	struct mei_nfc_dev *ndev;
-	struct mei_nfc_hci_hdr *hci_hdr;
-	int received_length;
-
-	ndev = (struct mei_nfc_dev *)cldev->priv_data;
-
-	received_length = __mei_cl_recv(ndev->cl, buf, length);
-	if (received_length < 0)
-		return received_length;
-
-	hci_hdr = (struct mei_nfc_hci_hdr *) buf;
-
-	if (hci_hdr->cmd == MEI_NFC_CMD_HCI_SEND) {
-		ndev->recv_req_id = hci_hdr->req_id;
-		wake_up(&ndev->send_wq);
-
-		return 0;
-	}
-
-	return received_length;
-}
-
-static struct mei_cl_ops nfc_ops = {
-	.enable = mei_nfc_enable,
-	.disable = mei_nfc_disable,
-	.send = mei_nfc_send,
-	.recv = mei_nfc_recv,
-};
-
 static void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
@@ -473,7 +305,7 @@ static void mei_nfc_init(struct work_struct *work)
 	}
 
 	cldev = mei_cl_add_device(dev, ndev->me_cl, ndev->cl,
-				  ndev->bus_name, &nfc_ops);
+				  ndev->bus_name);
 	if (!cldev) {
 		dev_err(dev->dev, "Could not add the NFC device to the MEI bus\n");
 
@@ -539,10 +371,7 @@ int mei_nfc_host_init(struct mei_device *dev, struct mei_me_client *me_cl)
 
 	ndev->cl = cl;
 
-	ndev->req_id = 1;
-
 	INIT_WORK(&ndev->init_work, mei_nfc_init);
-	init_waitqueue_head(&ndev->send_wq);
 	schedule_work(&ndev->init_work);
 
 	return 0;
diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 11c7cbdade66..7f1495d649bb 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -24,7 +24,52 @@
 
 #include "mei_phy.h"
 
-struct mei_nfc_hdr {
+struct mei_nfc_cmd {
+	u8 command;
+	u8 status;
+	u16 req_id;
+	u32 reserved;
+	u16 data_size;
+	u8 sub_command;
+	u8 data[];
+} __packed;
+
+struct mei_nfc_reply {
+	u8 command;
+	u8 status;
+	u16 req_id;
+	u32 reserved;
+	u16 data_size;
+	u8 sub_command;
+	u8 reply_status;
+	u8 data[];
+} __packed;
+
+struct mei_nfc_if_version {
+	u8 radio_version_sw[3];
+	u8 reserved[3];
+	u8 radio_version_hw[3];
+	u8 i2c_addr;
+	u8 fw_ivn;
+	u8 vendor_id;
+	u8 radio_type;
+} __packed;
+
+struct mei_nfc_connect {
+	u8 fw_ivn;
+	u8 vendor_id;
+} __packed;
+
+struct mei_nfc_connect_resp {
+	u8 fw_ivn;
+	u8 vendor_id;
+	u16 me_major;
+	u16 me_minor;
+	u16 me_hotfix;
+	u16 me_build;
+} __packed;
+
+struct mei_nfc_hci_hdr {
 	u8 cmd;
 	u8 status;
 	u16 req_id;
@@ -32,6 +77,16 @@ struct mei_nfc_hdr {
 	u16 data_size;
 } __packed;
 
+#define MEI_NFC_CMD_MAINTENANCE 0x00
+#define MEI_NFC_CMD_HCI_SEND 0x01
+#define MEI_NFC_CMD_HCI_RECV 0x02
+
+#define MEI_NFC_SUBCMD_CONNECT    0x00
+#define MEI_NFC_SUBCMD_IF_VERSION 0x01
+
+#define MEI_NFC_HEADER_SIZE 10
+
+
 #define MEI_NFC_MAX_READ (MEI_NFC_HEADER_SIZE + MEI_NFC_MAX_HCI_PAYLOAD)
 
 #define MEI_DUMP_SKB_IN(info, skb)				\
@@ -45,51 +100,156 @@ do {								\
 do {								\
 	pr_debug("%s:\n", info);				\
 	print_hex_dump_debug("mei out: ", DUMP_PREFIX_OFFSET,	\
-		       16, 1, (skb)->data, (skb)->len, false);	\
+			16, 1, (skb)->data, (skb)->len, false);	\
 } while (0)
 
-int nfc_mei_phy_enable(void *phy_id)
+
+static int mei_nfc_if_version(struct nfc_mei_phy *phy)
 {
-	int r;
-	struct nfc_mei_phy *phy = phy_id;
+
+	struct mei_nfc_cmd cmd;
+	struct mei_nfc_reply *reply = NULL;
+	struct mei_nfc_if_version *version;
+	size_t if_version_length;
+	int bytes_recv, r;
 
 	pr_info("%s\n", __func__);
 
-	if (phy->powered == 1)
-		return 0;
+	memset(&cmd, 0, sizeof(struct mei_nfc_cmd));
+	cmd.command = MEI_NFC_CMD_MAINTENANCE;
+	cmd.data_size = 1;
+	cmd.sub_command = MEI_NFC_SUBCMD_IF_VERSION;
 
-	r = mei_cl_enable_device(phy->device);
+	r = mei_cl_send(phy->device, (u8 *)&cmd, sizeof(struct mei_nfc_cmd));
 	if (r < 0) {
-		pr_err("Could not enable device\n");
+		pr_err("Could not send IF version cmd\n");
 		return r;
 	}
 
-	r = mei_cl_register_event_cb(phy->device, nfc_mei_event_cb, phy);
-	if (r) {
-		pr_err("Event cb registration failed\n");
-		mei_cl_disable_device(phy->device);
-		phy->powered = 0;
+	/* to be sure on the stack we alloc memory */
+	if_version_length = sizeof(struct mei_nfc_reply) +
+		sizeof(struct mei_nfc_if_version);
 
-		return r;
+	reply = kzalloc(if_version_length, GFP_KERNEL);
+	if (!reply)
+		return -ENOMEM;
+
+	bytes_recv = mei_cl_recv(phy->device, (u8 *)reply, if_version_length);
+	if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
+		pr_err("Could not read IF version\n");
+		r = -EIO;
+		goto err;
 	}
 
-	phy->powered = 1;
+	version = (struct mei_nfc_if_version *)reply->data;
 
-	return 0;
+	phy->fw_ivn = version->fw_ivn;
+	phy->vendor_id = version->vendor_id;
+	phy->radio_type = version->radio_type;
+
+err:
+	kfree(reply);
+	return r;
 }
-EXPORT_SYMBOL_GPL(nfc_mei_phy_enable);
 
-void nfc_mei_phy_disable(void *phy_id)
+static int mei_nfc_connect(struct nfc_mei_phy *phy)
 {
-	struct nfc_mei_phy *phy = phy_id;
+	struct mei_nfc_cmd *cmd, *reply;
+	struct mei_nfc_connect *connect;
+	struct mei_nfc_connect_resp *connect_resp;
+	size_t connect_length, connect_resp_length;
+	int bytes_recv, r;
 
 	pr_info("%s\n", __func__);
 
-	mei_cl_disable_device(phy->device);
+	connect_length = sizeof(struct mei_nfc_cmd) +
+			sizeof(struct mei_nfc_connect);
 
-	phy->powered = 0;
+	connect_resp_length = sizeof(struct mei_nfc_cmd) +
+			sizeof(struct mei_nfc_connect_resp);
+
+	cmd = kzalloc(connect_length, GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+	connect = (struct mei_nfc_connect *)cmd->data;
+
+	reply = kzalloc(connect_resp_length, GFP_KERNEL);
+	if (!reply) {
+		kfree(cmd);
+		return -ENOMEM;
+	}
+
+	connect_resp = (struct mei_nfc_connect_resp *)reply->data;
+
+	cmd->command = MEI_NFC_CMD_MAINTENANCE;
+	cmd->data_size = 3;
+	cmd->sub_command = MEI_NFC_SUBCMD_CONNECT;
+	connect->fw_ivn = phy->fw_ivn;
+	connect->vendor_id = phy->vendor_id;
+
+	r = mei_cl_send(phy->device, (u8 *)cmd, connect_length);
+	if (r < 0) {
+		pr_err("Could not send connect cmd %d\n", r);
+		goto err;
+	}
+
+	bytes_recv = mei_cl_recv(phy->device, (u8 *)reply, connect_resp_length);
+	if (bytes_recv < 0) {
+		r = bytes_recv;
+		pr_err("Could not read connect response %d\n", r);
+		goto err;
+	}
+
+	pr_info("IVN 0x%x Vendor ID 0x%x\n",
+		 connect_resp->fw_ivn, connect_resp->vendor_id);
+
+	pr_info("ME FW %d.%d.%d.%d\n",
+		connect_resp->me_major, connect_resp->me_minor,
+		connect_resp->me_hotfix, connect_resp->me_build);
+
+	r = 0;
+
+err:
+	kfree(reply);
+	kfree(cmd);
+
+	return r;
+}
+
+static int mei_nfc_send(struct nfc_mei_phy *phy, u8 *buf, size_t length)
+{
+	struct mei_nfc_hci_hdr *hdr;
+	u8 *mei_buf;
+	int err;
+
+	err = -ENOMEM;
+	mei_buf = kzalloc(length + MEI_NFC_HEADER_SIZE, GFP_KERNEL);
+	if (!mei_buf)
+		goto out;
+
+	hdr = (struct mei_nfc_hci_hdr *) mei_buf;
+	hdr->cmd = MEI_NFC_CMD_HCI_SEND;
+	hdr->status = 0;
+	hdr->req_id = phy->req_id;
+	hdr->reserved = 0;
+	hdr->data_size = length;
+
+	memcpy(mei_buf + MEI_NFC_HEADER_SIZE, buf, length);
+	err = mei_cl_send(phy->device, mei_buf, length + MEI_NFC_HEADER_SIZE);
+	if (err < 0)
+		goto out;
+
+	if (!wait_event_interruptible_timeout(phy->send_wq,
+				phy->recv_req_id == phy->req_id, HZ)) {
+		pr_err("NFC MEI command timeout\n");
+		err = -ETIME;
+	} else {
+		phy->req_id++;
+	}
+out:
+	kfree(mei_buf);
+	return err;
 }
-EXPORT_SYMBOL_GPL(nfc_mei_phy_disable);
 
 /*
  * Writing a frame must not return the number of written bytes.
@@ -103,14 +263,37 @@ static int nfc_mei_phy_write(void *phy_id, struct sk_buff *skb)
 
 	MEI_DUMP_SKB_OUT("mei frame sent", skb);
 
-	r = mei_cl_send(phy->device, skb->data, skb->len);
+	r = mei_nfc_send(phy, skb->data, skb->len);
 	if (r > 0)
 		r = 0;
 
 	return r;
 }
 
-void nfc_mei_event_cb(struct mei_cl_device *device, u32 events, void *context)
+static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
+{
+	struct mei_nfc_hci_hdr *hci_hdr;
+	int received_length;
+
+	received_length = mei_cl_recv(phy->device, buf, length);
+	if (received_length < 0)
+		return received_length;
+
+	hci_hdr = (struct mei_nfc_hci_hdr *) buf;
+
+	if (hci_hdr->cmd == MEI_NFC_CMD_HCI_SEND) {
+		phy->recv_req_id = hci_hdr->req_id;
+		wake_up(&phy->send_wq);
+
+		return 0;
+	}
+
+	return received_length;
+}
+
+
+static void nfc_mei_event_cb(struct mei_cl_device *device, u32 events,
+			     void *context)
 {
 	struct nfc_mei_phy *phy = context;
 
@@ -125,7 +308,7 @@ void nfc_mei_event_cb(struct mei_cl_device *device, u32 events, void *context)
 		if (!skb)
 			return;
 
-		reply_size = mei_cl_recv(device, skb->data, MEI_NFC_MAX_READ);
+		reply_size = mei_nfc_recv(phy, skb->data, MEI_NFC_MAX_READ);
 		if (reply_size < MEI_NFC_HEADER_SIZE) {
 			kfree_skb(skb);
 			return;
@@ -139,7 +322,61 @@ void nfc_mei_event_cb(struct mei_cl_device *device, u32 events, void *context)
 		nfc_hci_recv_frame(phy->hdev, skb);
 	}
 }
-EXPORT_SYMBOL_GPL(nfc_mei_event_cb);
+
+static int nfc_mei_phy_enable(void *phy_id)
+{
+	int r;
+	struct nfc_mei_phy *phy = phy_id;
+
+	pr_info("%s\n", __func__);
+
+	if (phy->powered == 1)
+		return 0;
+
+	r = mei_cl_enable_device(phy->device);
+	if (r < 0) {
+		pr_err("Could not enable device %d\n", r);
+		return r;
+	}
+
+	r = mei_nfc_if_version(phy);
+	if (r < 0) {
+		pr_err("Could not enable device %d\n", r);
+		goto err;
+	}
+
+	r = mei_nfc_connect(phy);
+	if (r < 0) {
+		pr_err("Could not connect to device %d\n", r);
+		goto err;
+	}
+
+	r = mei_cl_register_event_cb(phy->device, nfc_mei_event_cb, phy);
+	if (r) {
+		pr_err("Event cb registration failed %d\n", r);
+		goto err;
+	}
+
+	phy->powered = 1;
+
+	return 0;
+
+err:
+	phy->powered = 0;
+	mei_cl_disable_device(phy->device);
+	return r;
+}
+
+static void nfc_mei_phy_disable(void *phy_id)
+{
+	struct nfc_mei_phy *phy = phy_id;
+
+	pr_info("%s\n", __func__);
+
+	mei_cl_disable_device(phy->device);
+
+	phy->powered = 0;
+}
 
 struct nfc_phy_ops mei_phy_ops = {
 	.write = nfc_mei_phy_write,
@@ -157,6 +394,7 @@ struct nfc_mei_phy *nfc_mei_phy_alloc(struct mei_cl_device *device)
 		return NULL;
 
 	phy->device = device;
+	init_waitqueue_head(&phy->send_wq);
 	mei_cl_set_drvdata(device, phy);
 
 	return phy;
@@ -165,6 +403,7 @@ EXPORT_SYMBOL_GPL(nfc_mei_phy_alloc);
 
 void nfc_mei_phy_free(struct nfc_mei_phy *phy)
 {
+	mei_cl_disable_device(phy->device);
 	kfree(phy);
 }
 EXPORT_SYMBOL_GPL(nfc_mei_phy_free);
diff --git a/drivers/nfc/mei_phy.h b/drivers/nfc/mei_phy.h
index 06608c28ff14..a51f8f2685cc 100644
--- a/drivers/nfc/mei_phy.h
+++ b/drivers/nfc/mei_phy.h
@@ -10,23 +10,42 @@
 #define MEI_NFC_HEADER_SIZE 10
 #define MEI_NFC_MAX_HCI_PAYLOAD 300
 
+/**
+ * struct nfc_mei_phy
+ *
+ * @device: mei device
+ * @hdev:   nfc hci device
+
+ * @send_wq: send completion wait queue
+ * @fw_ivn: NFC Interface Version Number
+ * @vendor_id: NFC manufacturer ID
+ * @radio_type: NFC radio type
+ * @reserved: reserved for alignment
+ * @req_id:  message counter
+ * @recv_req_id: reception message counter
+ * @powered: the device is in powered state
+ * @hard_fault: < 0 if hardware error occurred
+ *    and prevents normal operation.
+ */
 struct nfc_mei_phy {
 	struct mei_cl_device *device;
 	struct nfc_hci_dev *hdev;
 
-	int powered;
+	wait_queue_head_t send_wq;
+	u8 fw_ivn;
+	u8 vendor_id;
+	u8 radio_type;
+	u8 reserved;
+
+	u16 req_id;
+	u16 recv_req_id;
 
-	int hard_fault;		/*
-				 * < 0 if hardware error occured
-				 * and prevents normal operation.
-				 */
+	int powered;
+	int hard_fault;
 };
 
 extern struct nfc_phy_ops mei_phy_ops;
 
-int nfc_mei_phy_enable(void *phy_id);
-void nfc_mei_phy_disable(void *phy_id);
-void nfc_mei_event_cb(struct mei_cl_device *device, u32 events, void *context);
 struct nfc_mei_phy *nfc_mei_phy_alloc(struct mei_cl_device *device);
 void nfc_mei_phy_free(struct nfc_mei_phy *phy);
 
-- 
2.1.0


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

* [char-misc-next 08/11] mei: bus: kill mei_cl_ops
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (5 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 07/11] NFC: mei_phy: move all nfc logic from mei driver to nfc Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 09/11] NFC: mei_phy: adjust mei nfc header according the spec Tomas Winkler
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, Samuel Ortiz

since we move all nfc hanling to the mei_phy module
we can kill mei_cl_ops

Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/mei_dev.h | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 2175bff2730f..609a2d2c2dba 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -328,27 +328,6 @@ struct mei_hw_ops {
 
 /* MEI bus API*/
 
-/**
- * struct mei_cl_ops - MEI CL device ops
- * This structure allows ME host clients to implement technology
- * specific operations.
- *
- * @enable: Enable an MEI CL device. Some devices require specific
- *	HECI commands to initialize completely.
- * @disable: Disable an MEI CL device.
- * @send: Tx hook for the device. This allows ME host clients to trap
- *	the device driver buffers before actually physically
- *	pushing it to the ME.
- * @recv: Rx hook for the device. This allows ME host clients to trap the
- *	ME buffers before forwarding them to the device driver.
- */
-struct mei_cl_ops {
-	int (*enable)(struct mei_cl_device *device);
-	int (*disable)(struct mei_cl_device *device);
-	int (*send)(struct mei_cl_device *device, u8 *buf, size_t length);
-	int (*recv)(struct mei_cl_device *device, u8 *buf, size_t length);
-};
-
 struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
 					struct mei_me_client *me_cl,
 					struct mei_cl *cl,
@@ -376,7 +355,6 @@ struct mei_cl *mei_cl_bus_find_cl_by_uuid(struct mei_device *dev, uuid_le uuid);
  * @me_cl: me client
  * @cl: mei client
  * @name: device name
- * @ops: ME transport ops
  * @event_work: async work to execute event callback
  * @event_cb: Drivers register this callback to get asynchronous ME
  *	events (e.g. Rx buffer pending) notifications.
@@ -391,8 +369,6 @@ struct mei_cl_device {
 	struct mei_cl *cl;
 	char name[MEI_CL_NAME_SIZE];
 
-	const struct mei_cl_ops *ops;
-
 	struct work_struct event_work;
 	mei_cl_event_cb_t event_cb;
 	void *event_context;
-- 
2.1.0


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

* [char-misc-next 09/11] NFC: mei_phy: adjust mei nfc header according the spec
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (6 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 08/11] mei: bus: kill mei_cl_ops Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 10/11] mei: export mei client device struct to external use Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 11/11] mei: revamp mei bus code Tomas Winkler
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, Samuel Ortiz

1. mei_nfc_hci_hdr and mei_nfc_hdr are just the same thing so drop one
2. use mei_nfc_hdr structure as the member of the command and the reply
instead of replicating all header fields.
3. dump the header for easier debugging

Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/nfc/mei_phy.c | 58 +++++++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 7f1495d649bb..2b77ccf77f81 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -24,22 +24,22 @@
 
 #include "mei_phy.h"
 
-struct mei_nfc_cmd {
-	u8 command;
+struct mei_nfc_hdr {
+	u8 cmd;
 	u8 status;
 	u16 req_id;
 	u32 reserved;
 	u16 data_size;
+} __packed;
+
+struct mei_nfc_cmd {
+	struct mei_nfc_hdr hdr;
 	u8 sub_command;
 	u8 data[];
 } __packed;
 
 struct mei_nfc_reply {
-	u8 command;
-	u8 status;
-	u16 req_id;
-	u32 reserved;
-	u16 data_size;
+	struct mei_nfc_hdr hdr;
 	u8 sub_command;
 	u8 reply_status;
 	u8 data[];
@@ -69,13 +69,6 @@ struct mei_nfc_connect_resp {
 	u16 me_build;
 } __packed;
 
-struct mei_nfc_hci_hdr {
-	u8 cmd;
-	u8 status;
-	u16 req_id;
-	u32 reserved;
-	u16 data_size;
-} __packed;
 
 #define MEI_NFC_CMD_MAINTENANCE 0x00
 #define MEI_NFC_CMD_HCI_SEND 0x01
@@ -84,9 +77,6 @@ struct mei_nfc_hci_hdr {
 #define MEI_NFC_SUBCMD_CONNECT    0x00
 #define MEI_NFC_SUBCMD_IF_VERSION 0x01
 
-#define MEI_NFC_HEADER_SIZE 10
-
-
 #define MEI_NFC_MAX_READ (MEI_NFC_HEADER_SIZE + MEI_NFC_MAX_HCI_PAYLOAD)
 
 #define MEI_DUMP_SKB_IN(info, skb)				\
@@ -103,6 +93,13 @@ do {								\
 			16, 1, (skb)->data, (skb)->len, false);	\
 } while (0)
 
+#define MEI_DUMP_NFC_HDR(info, _hdr)                                \
+do {                                                                \
+	pr_debug("%s:\n", info);                                    \
+	pr_debug("cmd=%02d status=%d req_id=%d rsvd=%d size=%d\n",  \
+		 (_hdr)->cmd, (_hdr)->status, (_hdr)->req_id,       \
+		 (_hdr)->reserved, (_hdr)->data_size);              \
+} while (0)
 
 static int mei_nfc_if_version(struct nfc_mei_phy *phy)
 {
@@ -116,10 +113,11 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
 	pr_info("%s\n", __func__);
 
 	memset(&cmd, 0, sizeof(struct mei_nfc_cmd));
-	cmd.command = MEI_NFC_CMD_MAINTENANCE;
-	cmd.data_size = 1;
+	cmd.hdr.cmd = MEI_NFC_CMD_MAINTENANCE;
+	cmd.hdr.data_size = 1;
 	cmd.sub_command = MEI_NFC_SUBCMD_IF_VERSION;
 
+	MEI_DUMP_NFC_HDR("version", &cmd.hdr);
 	r = mei_cl_send(phy->device, (u8 *)&cmd, sizeof(struct mei_nfc_cmd));
 	if (r < 0) {
 		pr_err("Could not send IF version cmd\n");
@@ -181,12 +179,13 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
 
 	connect_resp = (struct mei_nfc_connect_resp *)reply->data;
 
-	cmd->command = MEI_NFC_CMD_MAINTENANCE;
-	cmd->data_size = 3;
+	cmd->hdr.cmd = MEI_NFC_CMD_MAINTENANCE;
+	cmd->hdr.data_size = 3;
 	cmd->sub_command = MEI_NFC_SUBCMD_CONNECT;
 	connect->fw_ivn = phy->fw_ivn;
 	connect->vendor_id = phy->vendor_id;
 
+	MEI_DUMP_NFC_HDR("connect request", &cmd->hdr);
 	r = mei_cl_send(phy->device, (u8 *)cmd, connect_length);
 	if (r < 0) {
 		pr_err("Could not send connect cmd %d\n", r);
@@ -200,6 +199,8 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
 		goto err;
 	}
 
+	MEI_DUMP_NFC_HDR("connect reply", &reply->hdr);
+
 	pr_info("IVN 0x%x Vendor ID 0x%x\n",
 		 connect_resp->fw_ivn, connect_resp->vendor_id);
 
@@ -218,7 +219,7 @@ err:
 
 static int mei_nfc_send(struct nfc_mei_phy *phy, u8 *buf, size_t length)
 {
-	struct mei_nfc_hci_hdr *hdr;
+	struct mei_nfc_hdr *hdr;
 	u8 *mei_buf;
 	int err;
 
@@ -227,13 +228,15 @@ static int mei_nfc_send(struct nfc_mei_phy *phy, u8 *buf, size_t length)
 	if (!mei_buf)
 		goto out;
 
-	hdr = (struct mei_nfc_hci_hdr *) mei_buf;
+	hdr = (struct mei_nfc_hdr *)mei_buf;
 	hdr->cmd = MEI_NFC_CMD_HCI_SEND;
 	hdr->status = 0;
 	hdr->req_id = phy->req_id;
 	hdr->reserved = 0;
 	hdr->data_size = length;
 
+	MEI_DUMP_NFC_HDR("send", hdr);
+
 	memcpy(mei_buf + MEI_NFC_HEADER_SIZE, buf, length);
 	err = mei_cl_send(phy->device, mei_buf, length + MEI_NFC_HEADER_SIZE);
 	if (err < 0)
@@ -272,17 +275,18 @@ static int nfc_mei_phy_write(void *phy_id, struct sk_buff *skb)
 
 static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
 {
-	struct mei_nfc_hci_hdr *hci_hdr;
+	struct mei_nfc_hdr *hdr;
 	int received_length;
 
 	received_length = mei_cl_recv(phy->device, buf, length);
 	if (received_length < 0)
 		return received_length;
 
-	hci_hdr = (struct mei_nfc_hci_hdr *) buf;
+	hdr = (struct mei_nfc_hdr *) buf;
 
-	if (hci_hdr->cmd == MEI_NFC_CMD_HCI_SEND) {
-		phy->recv_req_id = hci_hdr->req_id;
+	MEI_DUMP_NFC_HDR("receive", hdr);
+	if (hdr->cmd == MEI_NFC_CMD_HCI_SEND) {
+		phy->recv_req_id = hdr->req_id;
 		wake_up(&phy->send_wq);
 
 		return 0;
-- 
2.1.0


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

* [char-misc-next 10/11] mei: export mei client device struct to external use
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (7 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 09/11] NFC: mei_phy: adjust mei nfc header according the spec Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-07 12:54 ` [char-misc-next 11/11] mei: revamp mei bus code Tomas Winkler
  9 siblings, 0 replies; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, Samuel Ortiz

Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/mei_dev.h | 35 -----------------------------------
 include/linux/mei_cl_bus.h | 38 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 609a2d2c2dba..70a644f688b0 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -343,41 +343,6 @@ int mei_cl_bus_init(void);
 void mei_cl_bus_exit(void);
 struct mei_cl *mei_cl_bus_find_cl_by_uuid(struct mei_device *dev, uuid_le uuid);
 
-
-/**
- * struct mei_cl_device - MEI device handle
- * An mei_cl_device pointer is returned from mei_add_device()
- * and links MEI bus clients to their actual ME host client pointer.
- * Drivers for MEI devices will get an mei_cl_device pointer
- * when being probed and shall use it for doing ME bus I/O.
- *
- * @dev: linux driver model device pointer
- * @me_cl: me client
- * @cl: mei client
- * @name: device name
- * @event_work: async work to execute event callback
- * @event_cb: Drivers register this callback to get asynchronous ME
- *	events (e.g. Rx buffer pending) notifications.
- * @event_context: event callback run context
- * @events: Events bitmask sent to the driver.
- * @priv_data: client private data
- */
-struct mei_cl_device {
-	struct device dev;
-
-	struct mei_me_client *me_cl;
-	struct mei_cl *cl;
-	char name[MEI_CL_NAME_SIZE];
-
-	struct work_struct event_work;
-	mei_cl_event_cb_t event_cb;
-	void *event_context;
-	unsigned long events;
-
-	void *priv_data;
-};
-
-
 /**
  * enum mei_pg_event - power gating transition events
  *
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index 0819d36a3a74..a16b1f9c1aca 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -7,6 +7,42 @@
 
 struct mei_cl_device;
 
+typedef void (*mei_cl_event_cb_t)(struct mei_cl_device *device,
+			       u32 events, void *context);
+
+/**
+ * struct mei_cl_device - MEI device handle
+ * An mei_cl_device pointer is returned from mei_add_device()
+ * and links MEI bus clients to their actual ME host client pointer.
+ * Drivers for MEI devices will get an mei_cl_device pointer
+ * when being probed and shall use it for doing ME bus I/O.
+ *
+ * @dev: linux driver model device pointer
+ * @me_cl: me client
+ * @cl: mei client
+ * @name: device name
+ * @event_work: async work to execute event callback
+ * @event_cb: Drivers register this callback to get asynchronous ME
+ *	events (e.g. Rx buffer pending) notifications.
+ * @event_context: event callback run context
+ * @events: Events bitmask sent to the driver.
+ * @priv_data: client private data
+ */
+struct mei_cl_device {
+	struct device dev;
+
+	struct mei_me_client *me_cl;
+	struct mei_cl *cl;
+	char name[MEI_CL_NAME_SIZE];
+
+	struct work_struct event_work;
+	mei_cl_event_cb_t event_cb;
+	void *event_context;
+	unsigned long events;
+
+	void *priv_data;
+};
+
 struct mei_cl_driver {
 	struct device_driver driver;
 	const char *name;
@@ -28,8 +64,6 @@ void mei_cl_driver_unregister(struct mei_cl_driver *driver);
 ssize_t mei_cl_send(struct mei_cl_device *device, u8 *buf, size_t length);
 ssize_t  mei_cl_recv(struct mei_cl_device *device, u8 *buf, size_t length);
 
-typedef void (*mei_cl_event_cb_t)(struct mei_cl_device *device,
-			       u32 events, void *context);
 int mei_cl_register_event_cb(struct mei_cl_device *device,
 			  mei_cl_event_cb_t read_cb, void *context);
 
-- 
2.1.0


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

* [char-misc-next 11/11] mei: revamp mei bus code
  2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
                   ` (8 preceding siblings ...)
  2015-05-07 12:54 ` [char-misc-next 10/11] mei: export mei client device struct to external use Tomas Winkler
@ 2015-05-07 12:54 ` Tomas Winkler
  2015-05-24 18:19   ` Greg KH
  9 siblings, 1 reply; 16+ messages in thread
From: Tomas Winkler @ 2015-05-07 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler, Samuel Ortiz

MEI bus was designed around nfc and was hard to extend.
Instead of hard coded way of adding the devices on the mei bus
we scan whole me client list and create a device for each
eligible me client; currently we support only clients with
single connection and fixed address clients.
NFC radio name detection is run as a fixup routine

Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/Makefile    |   2 +-
 drivers/misc/mei/bus-fixup.c | 306 ++++++++++++++
 drivers/misc/mei/bus.c       | 975 ++++++++++++++++++++++++++++---------------
 drivers/misc/mei/client.c    |   9 +-
 drivers/misc/mei/init.c      |   5 +-
 drivers/misc/mei/mei_dev.h   |  24 +-
 drivers/misc/mei/nfc.c       | 414 ------------------
 include/linux/mei_cl_bus.h   |  12 +
 8 files changed, 968 insertions(+), 779 deletions(-)
 create mode 100644 drivers/misc/mei/bus-fixup.c
 delete mode 100644 drivers/misc/mei/nfc.c

diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 518914a82b83..01447ca21c26 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -11,7 +11,7 @@ mei-objs += main.o
 mei-objs += amthif.o
 mei-objs += wd.o
 mei-objs += bus.o
-mei-objs += nfc.o
+mei-objs += bus-fixup.o
 mei-$(CONFIG_DEBUG_FS) += debugfs.o
 
 obj-$(CONFIG_INTEL_MEI_ME) += mei-me.o
diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
new file mode 100644
index 000000000000..f0d87289e08c
--- /dev/null
+++ b/drivers/misc/mei/bus-fixup.c
@@ -0,0 +1,306 @@
+/*
+ *
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/uuid.h>
+
+#include <linux/mei_cl_bus.h>
+
+#include "mei_dev.h"
+#include "client.h"
+
+#define  MEI_UUID_NFC_INFO UUID_LE(0xd2de1625, 0x382d, 0x417d, \
+			0x48, 0xa4, 0xef, 0xab, 0xba, 0x8a, 0x12, 0x06)
+
+#define MEI_UUID_NFC_HCI UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
+			0x94, 0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
+
+#define MEI_UUID_ANY NULL_UUID_LE
+
+static const uuid_le mei_nfc_info_guid = MEI_UUID_NFC_INFO;
+
+/**
+ * number_of_connections - determine whether an client be on the bus
+ *    according number of connections
+ *    We support only clients:
+ *       1. with single connection
+ *       2. and fixed clients (max_number_of_connections == 0)
+ *
+ * @dev: me clients device
+ */
+static void number_of_connections(struct mei_cl_device *dev)
+{
+	dev_dbg(&dev->dev, "running hook %s on %pUl\n",
+			__func__, mei_me_cl_uuid(dev->me_cl));
+
+	if (dev->me_cl->props.max_number_of_connections > 1)
+		dev->do_match = 0;
+}
+
+/**
+ * blacklist - blacklist a client from the bus
+ *
+ * @dev: me clients device
+ */
+static void blacklist(struct mei_cl_device *dev)
+{
+	dev_dbg(&dev->dev, "running hook %s on %pUl\n",
+			__func__, mei_me_cl_uuid(dev->me_cl));
+	dev->do_match = 0;
+}
+
+struct mei_nfc_cmd {
+	u8 command;
+	u8 status;
+	u16 req_id;
+	u32 reserved;
+	u16 data_size;
+	u8 sub_command;
+	u8 data[];
+} __packed;
+
+struct mei_nfc_reply {
+	u8 command;
+	u8 status;
+	u16 req_id;
+	u32 reserved;
+	u16 data_size;
+	u8 sub_command;
+	u8 reply_status;
+	u8 data[];
+} __packed;
+
+struct mei_nfc_if_version {
+	u8 radio_version_sw[3];
+	u8 reserved[3];
+	u8 radio_version_hw[3];
+	u8 i2c_addr;
+	u8 fw_ivn;
+	u8 vendor_id;
+	u8 radio_type;
+} __packed;
+
+
+#define MEI_NFC_CMD_MAINTENANCE 0x00
+#define MEI_NFC_SUBCMD_IF_VERSION 0x01
+
+/* Vendors */
+#define MEI_NFC_VENDOR_INSIDE 0x00
+#define MEI_NFC_VENDOR_NXP    0x01
+
+/* Radio types */
+#define MEI_NFC_VENDOR_INSIDE_UREAD 0x00
+#define MEI_NFC_VENDOR_NXP_PN544    0x01
+
+/**
+ * mei_nfc_if_version  - get NFC interface version
+ *
+ * @cl: host client (nfc info)
+ * @ver: NFC interface version to be filled in
+ *
+ * Return: 0 on success; < 0 otherwise
+ */
+static int mei_nfc_if_version(struct mei_cl *cl,
+			      struct mei_nfc_if_version *ver)
+{
+	struct mei_device *dev;
+	struct mei_nfc_cmd cmd = {
+		.command = MEI_NFC_CMD_MAINTENANCE,
+		.data_size = 1,
+		.sub_command = MEI_NFC_SUBCMD_IF_VERSION,
+	};
+	struct mei_nfc_reply *reply = NULL;
+	size_t if_version_length;
+	int bytes_recv, ret;
+
+	dev = cl->dev;
+
+	WARN_ON(mutex_is_locked(&dev->device_lock));
+
+	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd), 1);
+	if (ret < 0) {
+		dev_err(dev->dev, "Could not send IF version cmd\n");
+		return ret;
+	}
+
+	/* to be sure on the stack we alloc memory */
+	if_version_length = sizeof(struct mei_nfc_reply) +
+		sizeof(struct mei_nfc_if_version);
+
+	reply = kzalloc(if_version_length, GFP_KERNEL);
+	if (!reply)
+		return -ENOMEM;
+
+	ret = 0;
+	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
+	if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
+		dev_err(dev->dev, "Could not read IF version\n");
+		ret = -EIO;
+		goto err;
+	}
+
+	memcpy(ver, reply->data, sizeof(struct mei_nfc_if_version));
+
+	dev_info(dev->dev, "NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
+		ver->fw_ivn, ver->vendor_id, ver->radio_type);
+
+err:
+	kfree(reply);
+	return ret;
+}
+
+/**
+ * mei_nfc_radio_name - derive nfc radio name from the interface version
+ *
+ * @ver: NFC radio version
+ *
+ * Return: radio name string
+ */
+static const char *mei_nfc_radio_name(struct mei_nfc_if_version *ver)
+{
+
+	if (ver->vendor_id == MEI_NFC_VENDOR_INSIDE) {
+		if (ver->radio_type == MEI_NFC_VENDOR_INSIDE_UREAD)
+			return "microread";
+	}
+
+	if (ver->vendor_id == MEI_NFC_VENDOR_NXP) {
+		if (ver->radio_type == MEI_NFC_VENDOR_NXP_PN544)
+			return "pn544";
+	}
+
+	return NULL;
+}
+
+/**
+ * mei_nfc - The nfc fixup function.The function retrieves nfc radio name
+ *      and set is as device attribute so we can load
+ *      the proper device driver for it
+ *
+ * @dev: me client device (nfc)
+ */
+static void mei_nfc(struct mei_cl_device *dev)
+{
+	struct mei_device *bus;
+	struct mei_cl *cl;
+	struct mei_me_client *me_cl = NULL;
+	struct mei_nfc_if_version ver;
+	const char *radio_name = NULL;
+	int ret;
+
+	bus = dev->bus;
+
+	dev_dbg(bus->dev, "running hook %s: %pUl match=%d\n",
+		__func__, mei_me_cl_uuid(dev->me_cl), dev->do_match);
+
+	mutex_lock(&bus->device_lock);
+	/* we need to connect to INFO GUID */
+	cl = mei_cl_alloc_linked(bus, MEI_HOST_CLIENT_ID_ANY);
+	if (IS_ERR(cl)) {
+		ret = PTR_ERR(cl);
+		cl = NULL;
+		dev_err(bus->dev, "nfc hook alloc failed %d\n", ret);
+		goto out;
+	}
+
+	me_cl = mei_me_cl_by_uuid(bus, &mei_nfc_info_guid);
+	if (!me_cl) {
+		ret = -ENOTTY;
+		dev_dbg(bus->dev, "Cannot connect %d\n", ret);
+		goto out;
+	}
+
+	ret = mei_cl_connect(cl, me_cl, NULL);
+	if (ret < 0) {
+		dev_err(&dev->dev, "Can't connect to the NFC INFO ME ret = %d\n",
+			ret);
+		goto out;
+	}
+
+	mutex_unlock(&bus->device_lock);
+
+	ret = mei_nfc_if_version(cl, &ver);
+	if (ret)
+		goto disconnect;
+
+	radio_name = mei_nfc_radio_name(&ver);
+
+	if (!radio_name) {
+		ret = -ENOENT;
+		dev_err(&dev->dev, "Can't get the NFC interface version ret = %d\n",
+			ret);
+		goto disconnect;
+	}
+
+	dev_dbg(bus->dev, "nfc radio %s\n", radio_name);
+	strlcpy(dev->name, radio_name, sizeof(dev->name));
+
+disconnect:
+	mutex_lock(&bus->device_lock);
+	if (mei_cl_disconnect(cl) < 0)
+		dev_err(bus->dev, "Can't disconnect the NFC INFO ME\n");
+
+	mei_cl_flush_queues(cl, NULL);
+
+out:
+	mei_cl_unlink(cl);
+	mutex_unlock(&bus->device_lock);
+	mei_me_cl_put(me_cl);
+	kfree(cl);
+
+	if (ret)
+		dev->do_match = 0;
+
+	dev_dbg(bus->dev, "end of fixup match = %d\n", dev->do_match);
+}
+
+#define MEI_FIXUP(_uuid, _hook) { _uuid, _hook }
+
+static struct mei_fixup {
+
+	const uuid_le uuid;
+	void (*hook)(struct mei_cl_device *dev);
+} mei_fixups[] = {
+	MEI_FIXUP(MEI_UUID_ANY, number_of_connections),
+	MEI_FIXUP(MEI_UUID_NFC_INFO, blacklist),
+	MEI_FIXUP(MEI_UUID_NFC_HCI, mei_nfc),
+};
+
+/**
+ * mei_cl_dev_fixup - run fixup handlers
+ *
+ * @dev: me clients device
+ */
+void mei_cl_dev_fixup(struct mei_cl_device *dev)
+{
+	struct mei_fixup *f;
+	const uuid_le *uuid = mei_me_cl_uuid(dev->me_cl);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mei_fixups); i++) {
+
+		f = &mei_fixups[i];
+		if (uuid_le_cmp(f->uuid, MEI_UUID_ANY) == 0 ||
+		    uuid_le_cmp(f->uuid, *uuid) == 0)
+			f->hook(dev);
+	}
+}
+
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index de8fd089a8a4..ce2742aa5273 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -1,6 +1,6 @@
 /*
  * Intel Management Engine Interface (Intel MEI) Linux driver
- * Copyright (c) 2012-2013, Intel Corporation.
+ * Copyright (c) 2012-2015, Intel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -30,270 +30,18 @@
 #define to_mei_cl_driver(d) container_of(d, struct mei_cl_driver, driver)
 #define to_mei_cl_device(d) container_of(d, struct mei_cl_device, dev)
 
-static inline uuid_le uuid_le_cast(const __u8 uuid[16])
-{
-	return *(uuid_le *)uuid;
-}
-
-static int mei_cl_device_match(struct device *dev, struct device_driver *drv)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	struct mei_cl_driver *driver = to_mei_cl_driver(drv);
-	const struct mei_cl_device_id *id;
-	const uuid_le *uuid;
-	const char *name;
-
-	if (!device)
-		return 0;
-
-	uuid = mei_me_cl_uuid(device->me_cl);
-	name = device->name;
-
-	if (!driver || !driver->id_table)
-		return 0;
-
-	id = driver->id_table;
-
-	while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) {
-
-		if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) {
-			if (id->name[0]) {
-				if (!strncmp(name, id->name, sizeof(id->name)))
-					return 1;
-			} else {
-				return 1;
-			}
-		}
-
-		id++;
-	}
-
-	return 0;
-}
-
-static int mei_cl_device_probe(struct device *dev)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	struct mei_cl_driver *driver;
-	struct mei_cl_device_id id;
-
-	if (!device)
-		return 0;
-
-	driver = to_mei_cl_driver(dev->driver);
-	if (!driver || !driver->probe)
-		return -ENODEV;
-
-	dev_dbg(dev, "Device probe\n");
-
-	strlcpy(id.name, device->name, sizeof(id.name));
-
-	return driver->probe(device, &id);
-}
-
-static int mei_cl_device_remove(struct device *dev)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	struct mei_cl_driver *driver;
-
-	if (!device || !dev->driver)
-		return 0;
-
-	if (device->event_cb) {
-		device->event_cb = NULL;
-		cancel_work_sync(&device->event_work);
-	}
-
-	driver = to_mei_cl_driver(dev->driver);
-	if (!driver->remove) {
-		dev->driver = NULL;
-
-		return 0;
-	}
-
-	return driver->remove(device);
-}
-
-static ssize_t name_show(struct device *dev, struct device_attribute *a,
-			     char *buf)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	size_t len;
-
-	len = snprintf(buf, PAGE_SIZE, "%s", device->name);
-
-	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
-}
-static DEVICE_ATTR_RO(name);
-
-static ssize_t uuid_show(struct device *dev, struct device_attribute *a,
-			     char *buf)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
-	size_t len;
-
-	len = snprintf(buf, PAGE_SIZE, "%pUl", uuid);
-
-	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
-}
-static DEVICE_ATTR_RO(uuid);
-
-static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
-			     char *buf)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
-	size_t len;
-
-	len = snprintf(buf, PAGE_SIZE, "mei:%s:" MEI_CL_UUID_FMT ":",
-		device->name, MEI_CL_UUID_ARGS(uuid->b));
-
-	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
-}
-static DEVICE_ATTR_RO(modalias);
-
-static struct attribute *mei_cl_dev_attrs[] = {
-	&dev_attr_name.attr,
-	&dev_attr_uuid.attr,
-	&dev_attr_modalias.attr,
-	NULL,
-};
-ATTRIBUTE_GROUPS(mei_cl_dev);
-
-static int mei_cl_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-	const uuid_le *uuid = mei_me_cl_uuid(device->me_cl);
-
-	if (add_uevent_var(env, "MEI_CL_UUID=%pUl", uuid))
-		return -ENOMEM;
-
-	if (add_uevent_var(env, "MEI_CL_NAME=%s", device->name))
-		return -ENOMEM;
-
-	if (add_uevent_var(env, "MODALIAS=mei:%s:" MEI_CL_UUID_FMT ":",
-		device->name, MEI_CL_UUID_ARGS(uuid->b)))
-		return -ENOMEM;
-
-	return 0;
-}
-
-static struct bus_type mei_cl_bus_type = {
-	.name		= "mei",
-	.dev_groups	= mei_cl_dev_groups,
-	.match		= mei_cl_device_match,
-	.probe		= mei_cl_device_probe,
-	.remove		= mei_cl_device_remove,
-	.uevent		= mei_cl_uevent,
-};
-
-static void mei_cl_dev_release(struct device *dev)
-{
-	struct mei_cl_device *device = to_mei_cl_device(dev);
-
-	if (!device)
-		return;
-
-	mei_me_cl_put(device->me_cl);
-	kfree(device);
-}
-
-static struct device_type mei_cl_device_type = {
-	.release	= mei_cl_dev_release,
-};
-
-struct mei_cl *mei_cl_bus_find_cl_by_uuid(struct mei_device *dev,
-					 uuid_le uuid)
-{
-	struct mei_cl *cl;
-
-	list_for_each_entry(cl, &dev->device_list, device_link) {
-		if (cl->device && cl->device->me_cl &&
-		    !uuid_le_cmp(uuid, *mei_me_cl_uuid(cl->device->me_cl)))
-			return cl;
-	}
-
-	return NULL;
-}
-
-struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
-					struct mei_me_client *me_cl,
-					struct mei_cl *cl,
-					char *name)
-{
-	struct mei_cl_device *device;
-	int status;
-
-	device = kzalloc(sizeof(struct mei_cl_device), GFP_KERNEL);
-	if (!device)
-		return NULL;
-
-	device->me_cl = mei_me_cl_get(me_cl);
-	if (!device->me_cl) {
-		kfree(device);
-		return NULL;
-	}
-
-	device->cl = cl;
-	device->dev.parent = dev->dev;
-	device->dev.bus = &mei_cl_bus_type;
-	device->dev.type = &mei_cl_device_type;
-
-	strlcpy(device->name, name, sizeof(device->name));
-
-	dev_set_name(&device->dev, "mei:%s:%pUl", name, mei_me_cl_uuid(me_cl));
-
-	status = device_register(&device->dev);
-	if (status) {
-		dev_err(dev->dev, "Failed to register MEI device\n");
-		mei_me_cl_put(device->me_cl);
-		kfree(device);
-		return NULL;
-	}
-
-	cl->device = device;
-
-	dev_dbg(&device->dev, "client %s registered\n", name);
-
-	return device;
-}
-EXPORT_SYMBOL_GPL(mei_cl_add_device);
-
-void mei_cl_remove_device(struct mei_cl_device *device)
-{
-	device_unregister(&device->dev);
-}
-EXPORT_SYMBOL_GPL(mei_cl_remove_device);
-
-int __mei_cl_driver_register(struct mei_cl_driver *driver, struct module *owner)
-{
-	int err;
-
-	driver->driver.name = driver->name;
-	driver->driver.owner = owner;
-	driver->driver.bus = &mei_cl_bus_type;
-
-	err = driver_register(&driver->driver);
-	if (err)
-		return err;
-
-	pr_debug("mei: driver [%s] registered\n", driver->driver.name);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(__mei_cl_driver_register);
-
-void mei_cl_driver_unregister(struct mei_cl_driver *driver)
-{
-	driver_unregister(&driver->driver);
-
-	pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
-}
-EXPORT_SYMBOL_GPL(mei_cl_driver_unregister);
-
+/**
+ * __mei_cl_send - internal client send (write)
+ *
+ * @cl: host client
+ * @buf: buffer to send
+ * @length: buffer length
+ * @blocking: wait for write completion
+ *
+ * Return: written size bytes or < 0 on error
+ */
 ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
-			bool blocking)
+		      bool blocking)
 {
 	struct mei_device *dev;
 	struct mei_cl_cb *cb = NULL;
@@ -339,6 +87,15 @@ out:
 	return rets;
 }
 
+/**
+ * __mei_cl_recv - internal client receive (read)
+ *
+ * @cl: host client
+ * @buf: buffer to send
+ * @length: buffer length
+ *
+ * Return: read size in bytes of < 0 on error
+ */
 ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 {
 	struct mei_device *dev;
@@ -406,9 +163,18 @@ out:
 	return rets;
 }
 
-ssize_t mei_cl_send(struct mei_cl_device *device, u8 *buf, size_t length)
+/**
+ * mei_cl_send - me device send  (write)
+ *
+ * @dev: me client device
+ * @buf: buffer to send
+ * @length: buffer length
+ *
+ * Return: written size in bytes or < 0 on error
+ */
+ssize_t mei_cl_send(struct mei_cl_device *dev, u8 *buf, size_t length)
 {
-	struct mei_cl *cl = device->cl;
+	struct mei_cl *cl = dev->cl;
 
 	if (cl == NULL)
 		return -ENODEV;
@@ -417,9 +183,18 @@ ssize_t mei_cl_send(struct mei_cl_device *device, u8 *buf, size_t length)
 }
 EXPORT_SYMBOL_GPL(mei_cl_send);
 
-ssize_t mei_cl_recv(struct mei_cl_device *device, u8 *buf, size_t length)
+/**
+ * mei_cl_recv - client receive (read)
+ *
+ * @dev: me client device
+ * @buf: buffer to send
+ * @length: buffer length
+ *
+ * Return: read size in bytes of < 0 on error
+ */
+ssize_t mei_cl_recv(struct mei_cl_device *dev, u8 *buf, size_t length)
 {
-	struct mei_cl *cl = device->cl;
+	struct mei_cl *cl = dev->cl;
 
 	if (cl == NULL)
 		return -ENODEV;
@@ -428,123 +203,164 @@ ssize_t mei_cl_recv(struct mei_cl_device *device, u8 *buf, size_t length)
 }
 EXPORT_SYMBOL_GPL(mei_cl_recv);
 
+/**
+ * mei_bus_event_work  - dispatch rx event for a bus device
+ *    and schedule new work
+ *
+ * @work: work
+ */
 static void mei_bus_event_work(struct work_struct *work)
 {
-	struct mei_cl_device *device;
+	struct mei_cl_device *dev;
 
-	device = container_of(work, struct mei_cl_device, event_work);
+	dev = container_of(work, struct mei_cl_device, event_work);
 
-	if (device->event_cb)
-		device->event_cb(device, device->events, device->event_context);
+	if (dev->event_cb)
+		dev->event_cb(dev, dev->events, dev->event_context);
 
-	device->events = 0;
+	dev->events = 0;
 
 	/* Prepare for the next read */
-	mei_cl_read_start(device->cl, 0, NULL);
+	mei_cl_read_start(dev->cl, 0, NULL);
 }
 
-int mei_cl_register_event_cb(struct mei_cl_device *device,
+/**
+ * mei_cl_register_event_cb - register event callback
+ *
+ * @dev: me client devices
+ * @event_cb: callback function
+ * @context: driver context data
+ *
+ * Return: 0 on success
+ *         -EALREADY if an callback is already registered
+ *         <0 on other errors
+ */
+int mei_cl_register_event_cb(struct mei_cl_device *dev,
 			  mei_cl_event_cb_t event_cb, void *context)
 {
-	if (device->event_cb)
+	int ret;
+
+	if (dev->event_cb)
 		return -EALREADY;
 
-	device->events = 0;
-	device->event_cb = event_cb;
-	device->event_context = context;
-	INIT_WORK(&device->event_work, mei_bus_event_work);
+	dev->events = 0;
+	dev->event_cb = event_cb;
+	dev->event_context = context;
+	INIT_WORK(&dev->event_work, mei_bus_event_work);
 
-	mei_cl_read_start(device->cl, 0, NULL);
+	ret = mei_cl_read_start(dev->cl, 0, NULL);
+	if (ret && ret != -EBUSY)
+		return ret;
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mei_cl_register_event_cb);
 
-void *mei_cl_get_drvdata(const struct mei_cl_device *device)
-{
-	return dev_get_drvdata(&device->dev);
-}
-EXPORT_SYMBOL_GPL(mei_cl_get_drvdata);
-
-void mei_cl_set_drvdata(struct mei_cl_device *device, void *data)
-{
-	dev_set_drvdata(&device->dev, data);
-}
-EXPORT_SYMBOL_GPL(mei_cl_set_drvdata);
-
-int mei_cl_enable_device(struct mei_cl_device *device)
+/**
+ * mei_cl_enable_device - enable me client device
+ *     create connection with me client
+ *
+ * @dev: me client device
+ *
+ * Return: 0 on success and < 0 on error
+ */
+int mei_cl_enable_device(struct mei_cl_device *dev)
 {
-	int err;
-	struct mei_device *dev;
-	struct mei_cl *cl = device->cl;
-
-	if (cl == NULL)
-		return -ENODEV;
-
-	dev = cl->dev;
-
-	mutex_lock(&dev->device_lock);
+	struct mei_device *bus = dev->bus;
+	struct mei_cl *cl;
+	int ret;
+
+	cl = dev->cl;
+
+	if (!cl) {
+		mutex_lock(&bus->device_lock);
+		cl = mei_cl_alloc_linked(bus, MEI_HOST_CLIENT_ID_ANY);
+		mutex_unlock(&bus->device_lock);
+		if (IS_ERR(cl))
+			return PTR_ERR(cl);
+		/* update pointers */
+		dev->cl = cl;
+		cl->device = dev;
+	}
 
+	mutex_lock(&bus->device_lock);
 	if (mei_cl_is_connected(cl)) {
-		mutex_unlock(&dev->device_lock);
-		dev_warn(dev->dev, "Already connected");
-		return -EBUSY;
+		ret = 0;
+		goto out;
 	}
 
-	err = mei_cl_connect(cl, device->me_cl, NULL);
-	if (err < 0) {
-		mutex_unlock(&dev->device_lock);
-		dev_err(dev->dev, "Could not connect to the ME client");
-
-		return err;
+	if (!mei_me_cl_is_active(dev->me_cl)) {
+		dev_err(&dev->dev, "me client is not active\n");
+		ret = -ENOTTY;
+		goto out;
 	}
 
-	mutex_unlock(&dev->device_lock);
+	ret = mei_cl_connect(cl, dev->me_cl, NULL);
+	if (ret < 0)
+		dev_err(&dev->dev, "cannot connect\n");
 
-	if (device->event_cb)
-		mei_cl_read_start(device->cl, 0, NULL);
+out:
+	mutex_unlock(&bus->device_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(mei_cl_enable_device);
 
-int mei_cl_disable_device(struct mei_cl_device *device)
+/**
+ * mei_cl_disable_device - disable me client device
+ *     disconnect form the me client
+ *
+ * @dev: me client device
+ *
+ * Return: 0 on success and < 0 on error
+ */
+int mei_cl_disable_device(struct mei_cl_device *dev)
 {
+	struct mei_device *bus;
+	struct mei_cl *cl;
 	int err;
-	struct mei_device *dev;
-	struct mei_cl *cl = device->cl;
 
-	if (cl == NULL)
+	if (!dev || !dev->cl)
 		return -ENODEV;
 
-	dev = cl->dev;
+	cl = dev->cl;
 
-	device->event_cb = NULL;
+	bus = dev->bus;
 
-	mutex_lock(&dev->device_lock);
+	dev->event_cb = NULL;
+
+	mutex_lock(&bus->device_lock);
 
 	if (!mei_cl_is_connected(cl)) {
-		dev_err(dev->dev, "Already disconnected");
+		dev_err(bus->dev, "Already disconnected");
 		err = 0;
 		goto out;
 	}
 
 	err = mei_cl_disconnect(cl);
 	if (err < 0) {
-		dev_err(dev->dev, "Could not disconnect from the ME client");
+		dev_err(bus->dev, "Could not disconnect from the ME client");
 		goto out;
 	}
 
+out:
 	/* Flush queues and remove any pending read */
 	mei_cl_flush_queues(cl, NULL);
+	mei_cl_unlink(cl);
 
-out:
-	mutex_unlock(&dev->device_lock);
-	return err;
+	kfree(cl);
+	dev->cl = NULL;
 
+	mutex_unlock(&bus->device_lock);
+	return err;
 }
 EXPORT_SYMBOL_GPL(mei_cl_disable_device);
 
+/**
+ * mei_cl_bus_rx_event  - schedule rx evenet
+ *
+ * @cl: host client
+ */
 void mei_cl_bus_rx_event(struct mei_cl *cl)
 {
 	struct mei_cl_device *device = cl->device;
@@ -557,22 +373,503 @@ void mei_cl_bus_rx_event(struct mei_cl *cl)
 	schedule_work(&device->event_work);
 }
 
-void mei_cl_bus_remove_devices(struct mei_device *dev)
+static inline uuid_le uuid_le_cast(const __u8 uuid[16])
+{
+	return *(uuid_le *)uuid;
+}
+
+/**
+ * mei_cl_device_match - find matching entry in the driver id table
+ *
+ * @drv: me client driver
+ * @dev: me client device
+ *
+ * Return: id on success NULL if no id was matched
+ */
+static const
+struct mei_cl_device_id *mei_cl_device_match(struct mei_cl_driver *drv,
+					     struct mei_cl_device *dev)
 {
-	struct mei_cl *cl, *next;
+	const struct mei_cl_device_id *id = drv->id_table;
+	const uuid_le *uuid = mei_me_cl_uuid(dev->me_cl);
 
-	mutex_lock(&dev->device_lock);
-	list_for_each_entry_safe(cl, next, &dev->device_list, device_link) {
-		if (cl->device)
-			mei_cl_remove_device(cl->device);
+	while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) {
 
-		list_del(&cl->device_link);
-		mei_cl_unlink(cl);
-		kfree(cl);
+		if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) {
+
+			if (!dev->name[0])
+				return id;
+
+			if (!strncmp(dev->name, id->name, sizeof(id->name)))
+				return id;
+		}
+		id++;
 	}
-	mutex_unlock(&dev->device_lock);
+	return NULL;
+}
+
+/**
+ * mei_cl_bus_match  - bus match function
+ *
+ * @device: device
+ * @driver: driver
+ *
+ * Return:  1 if matching device was found 0 otherwise
+ */
+static int mei_cl_bus_match(struct device *device,
+			    struct device_driver *driver)
+{
+	struct mei_cl_device *dev = to_mei_cl_device(device);
+	struct mei_cl_driver *drv = to_mei_cl_driver(driver);
+	const struct mei_cl_device_id *found_id;
+
+	if (!dev)
+		return 0;
+
+	if (!dev->do_match)
+		return 0;
+
+	if (!drv || !drv->id_table)
+		return 0;
+
+	found_id = mei_cl_device_match(drv, dev);
+	if (found_id)
+		return 1;
+
+	return 0;
+}
+
+/**
+ * mei_cl_device_probe - bus probe function
+ *
+ * @device: device
+ *
+ * Return:  0 on success; < 0 otherwise
+ */
+static int mei_cl_device_probe(struct device *device)
+{
+	struct mei_cl_device *dev;
+	struct mei_cl_driver *drv;
+	const struct mei_cl_device_id *id;
+
+	dev = to_mei_cl_device(device);
+	drv = to_mei_cl_driver(device->driver);
+
+	if (!drv)
+		return 0;
+
+	if (!drv || !drv->probe)
+		return -ENODEV;
+
+	id = mei_cl_device_match(drv, dev);
+	if (!id)
+		return -ENODEV;
+
+	__module_get(THIS_MODULE);
+
+	return drv->probe(dev, id);
+}
+
+/**
+ * mei_cl_device_remove - remove device from the bus
+ *
+ * @device: device
+ *
+ * Return:  0 on success; < 0 otherwise
+ */
+static int mei_cl_device_remove(struct device *device)
+{
+	struct mei_cl_device *dev = to_mei_cl_device(device);
+	struct mei_cl_driver *drv;
+	int ret = 0;
+
+	if (!dev || !device->driver)
+		return 0;
+
+	if (dev->event_cb) {
+		dev->event_cb = NULL;
+		cancel_work_sync(&dev->event_work);
+	}
+
+	drv = to_mei_cl_driver(device->driver);
+	if (drv->remove)
+		ret = drv->remove(dev);
+
+	module_put(THIS_MODULE);
+	device->driver = NULL;
+	return ret;
+
+}
+
+/**
+ * name_show - client device name sysfs entry
+ *
+ * @device: device
+ * @a: device attribute
+ * @buf: a buffer to fill
+ *
+ * Return: bytes to read or < 0 on error
+ */
+static ssize_t name_show(struct device *device,
+			 struct device_attribute *a,
+			 char *buf)
+{
+	struct mei_cl_device *dev = to_mei_cl_device(device);
+	size_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "%s", dev->name);
+
+	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+}
+static DEVICE_ATTR_RO(name);
+
+/**
+ * uuid_show - client uuid sysfs entry
+ *
+ * @device: device
+ * @a: device attribute
+ * @buf: a buffer to fill
+ *
+ * Return: bytes to read or < 0 on error
+ */
+static ssize_t uuid_show(struct device *device,
+			 struct device_attribute *a,
+			 char *buf)
+{
+	struct mei_cl_device *dev = to_mei_cl_device(device);
+	const uuid_le *uuid = mei_me_cl_uuid(dev->me_cl);
+	size_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "%pUl", uuid);
+
+	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+}
+static DEVICE_ATTR_RO(uuid);
+
+/**
+ * modalias_show - modalias sysfs entry
+ *
+ * @device: device
+ * @a: device attribute
+ * @buf: a buffer to fill
+ *
+ * Return: bytes to read or < 0 on error
+ */
+static ssize_t modalias_show(struct device *device,
+			     struct device_attribute *a,
+			     char *buf)
+{
+	struct mei_cl_device *dev = to_mei_cl_device(device);
+	const uuid_le *uuid = mei_me_cl_uuid(dev->me_cl);
+	size_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "mei:%s:" MEI_CL_UUID_FMT ":",
+		       dev->name, MEI_CL_UUID_ARGS(uuid->b));
+
+	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+}
+static DEVICE_ATTR_RO(modalias);
+
+static struct attribute *mei_cl_dev_attrs[] = {
+	&dev_attr_name.attr,
+	&dev_attr_uuid.attr,
+	&dev_attr_modalias.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(mei_cl_dev);
+
+/**
+ * mei_cl_device_uevent - me client bus uevent handler
+ *
+ * @device: device
+ * @env: uevent kobject
+ *
+ * Return: 0 on success -ENOMEM on when add_uevent_var fails
+ */
+static int mei_cl_device_uevent(struct device *device,
+				struct kobj_uevent_env *env)
+{
+	struct mei_cl_device *dev = to_mei_cl_device(device);
+	const uuid_le *uuid = mei_me_cl_uuid(dev->me_cl);
+
+	if (add_uevent_var(env, "MEI_CL_UUID=%pUl", uuid))
+		return -ENOMEM;
+
+	if (add_uevent_var(env, "MEI_CL_NAME=%s", dev->name))
+		return -ENOMEM;
+
+	if (add_uevent_var(env, "MODALIAS=mei:%s:" MEI_CL_UUID_FMT ":",
+		dev->name, MEI_CL_UUID_ARGS(uuid->b)))
+		return -ENOMEM;
+
+	return 0;
+}
+
+static struct bus_type mei_cl_bus_type = {
+	.name		= "mei",
+	.dev_groups	= mei_cl_dev_groups,
+	.match		= mei_cl_bus_match,
+	.probe		= mei_cl_device_probe,
+	.remove		= mei_cl_device_remove,
+	.uevent		= mei_cl_device_uevent,
+};
+
+static struct mei_device *mei_dev_bus_get(struct mei_device *bus)
+{
+	if (bus)
+		get_device(bus->dev);
+	return bus;
+}
+
+static void mei_dev_bus_put(struct mei_device *bus)
+{
+	if (bus)
+		put_device(bus->dev);
+}
+
+static void mei_cl_dev_release(struct device *dev)
+{
+	struct mei_cl_device *cl_dev = to_mei_cl_device(dev);
+
+	mei_me_cl_put(cl_dev->me_cl);
+	mei_dev_bus_put(cl_dev->bus);
+	kfree(cl_dev);
+}
+
+static struct device_type mei_cl_device_type = {
+	.release	= mei_cl_dev_release,
+};
+
+/**
+ * mei_cl_dev_alloc - initialize and allocate mei client device
+ *
+ * @bus: mei device
+ * @me_cl: me client
+ *
+ * Return: allocated device structur or NULL on allocation failure
+ */
+static struct mei_cl_device *mei_cl_dev_alloc(struct mei_device *bus,
+					      struct mei_me_client *me_cl)
+{
+	struct mei_cl_device *dev;
+
+	dev = kzalloc(sizeof(struct mei_cl_device), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	device_initialize(&dev->dev);
+	dev->dev.parent = bus->dev;
+	dev->dev.bus    = &mei_cl_bus_type;
+	dev->dev.type   = &mei_cl_device_type;
+	dev->bus        = mei_dev_bus_get(bus);
+	dev->me_cl      = mei_me_cl_get(me_cl);
+	dev->is_added   = 0;
+	INIT_LIST_HEAD(&dev->bus_list);
+
+	return dev;
+}
+
+/**
+ * mei_cl_dev_setup - setup me client device
+ *    run fix up routines and set the device name
+ *
+ * @bus: mei device
+ * @dev: me client device
+ *
+ * Return: true if the device is eligible for enumeration
+ */
+static bool mei_cl_dev_setup(struct mei_device *bus, struct mei_cl_device *dev)
+{
+	dev->do_match = 1;
+	mei_cl_dev_fixup(dev);
+
+	if (dev->do_match)
+		dev_set_name(&dev->dev, "mei:%s:%pUl",
+			     dev->name, mei_me_cl_uuid(dev->me_cl));
+
+	return dev->do_match == 1;
+}
+
+/**
+ * mei_cl_bus_dev_add - add me client devices
+ *
+ * @dev: me client device
+ *
+ * Return: 0 on success; < 0 on failre
+ */
+static int mei_cl_bus_dev_add(struct mei_cl_device *dev)
+{
+	int ret;
+
+	dev_dbg(dev->bus->dev, "adding %pUL\n", mei_me_cl_uuid(dev->me_cl));
+	ret = device_add(&dev->dev);
+	if (!ret)
+		dev->is_added = 1;
+
+	return ret;
+}
+
+/**
+ * mei_cl_bus_dev_stop - stop the driver
+ *
+ * @dev: me client device
+ */
+static void mei_cl_bus_dev_stop(struct mei_cl_device *dev)
+{
+	dev_dbg(dev->bus->dev, "stopping %s", dev_name(&dev->dev));
+	if (dev->is_added)
+		device_release_driver(&dev->dev);
+}
+
+/**
+ * mei_cl_bus_dev_destroy - destroy me client devices object
+ *
+ * @dev: me client device
+ */
+static void mei_cl_bus_dev_destroy(struct mei_cl_device *dev)
+{
+	if (!dev->is_added)
+		return;
+
+	device_del(&dev->dev);
+
+	mutex_lock(&dev->bus->cl_bus_lock);
+	list_del_init(&dev->bus_list);
+	mutex_unlock(&dev->bus->cl_bus_lock);
+
+	dev->is_added = 0;
+	put_device(&dev->dev);
+}
+
+/**
+ * mei_cl_bus_remove_device - remove a devices form the bus
+ *
+ * @dev: me client device
+ */
+static void mei_cl_bus_remove_device(struct mei_cl_device *dev)
+{
+	dev_dbg(dev->bus->dev, "removing %s", dev_name(&dev->dev));
+	mei_cl_bus_dev_stop(dev);
+	mei_cl_bus_dev_destroy(dev);
+}
+
+/**
+ * mei_cl_bus_remove_device - remove all devices form the bus
+ *
+ * @bus: mei device
+ */
+void mei_cl_bus_remove_devices(struct mei_device *bus)
+{
+	struct mei_cl_device *dev, *next;
+
+	list_for_each_entry_safe(dev, next, &bus->devices, bus_list)
+		mei_cl_bus_remove_device(dev);
+}
+
+
+/**
+ * mei_cl_dev_init - allocate and initializes an mei client devices based on me client
+ *
+ * @bus: mei device
+ * @me_cl: me client
+ */
+static void mei_cl_dev_init(struct mei_device *bus, struct mei_me_client *me_cl)
+{
+	struct mei_cl_device *dev;
+
+	dev_dbg(bus->dev, "initializing %pUl", mei_me_cl_uuid(me_cl));
+
+	if (me_cl->bus_added)
+		return;
+
+	dev = mei_cl_dev_alloc(bus, me_cl);
+	if (!dev)
+		return;
+
+	mutex_lock(&dev->bus->cl_bus_lock);
+	me_cl->bus_added = true;
+	list_add_tail(&dev->bus_list, &bus->devices);
+	mutex_unlock(&dev->bus->cl_bus_lock);
+
 }
 
+/**
+ * mei_cl_bus_rescan - scan me clients list and add create devices for eligible clients
+ *
+ * @bus: mei device
+ */
+void mei_cl_bus_rescan(struct mei_device *bus)
+{
+	struct mei_cl_device *dev, *n;
+	struct mei_me_client *me_cl;
+
+	down_read(&bus->me_clients_rwsem);
+	list_for_each_entry(me_cl, &bus->me_clients, list)
+		mei_cl_dev_init(bus, me_cl);
+	up_read(&bus->me_clients_rwsem);
+
+	mutex_lock(&bus->cl_bus_lock);
+	list_for_each_entry_safe(dev, n, &bus->devices, bus_list) {
+
+		if (!mei_me_cl_is_active(dev->me_cl)) {
+			mei_cl_bus_remove_device(dev);
+			continue;
+		}
+
+		if (dev->is_added)
+			continue;
+
+		if (mei_cl_dev_setup(bus, dev))
+			mei_cl_bus_dev_add(dev);
+		else {
+			list_del_init(&dev->bus_list);
+			put_device(&dev->dev);
+		}
+	}
+	mutex_unlock(&bus->cl_bus_lock);
+
+	dev_dbg(bus->dev, "rescan end");
+}
+EXPORT_SYMBOL_GPL(mei_cl_bus_rescan);
+
+int __mei_cl_driver_register(struct mei_cl_driver *drv, struct module *owner)
+{
+	int err;
+
+	drv->driver.name = drv->name;
+	drv->driver.owner = owner;
+	drv->driver.bus = &mei_cl_bus_type;
+
+	err = driver_register(&drv->driver);
+	if (err)
+		return err;
+
+	pr_debug("mei: driver [%s] registered\n", drv->driver.name);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__mei_cl_driver_register);
+
+void mei_cl_driver_unregister(struct mei_cl_driver *drv)
+{
+	driver_unregister(&drv->driver);
+
+	pr_debug("mei: driver [%s] unregistered\n", drv->driver.name);
+}
+EXPORT_SYMBOL_GPL(mei_cl_driver_unregister);
+
+void *mei_cl_get_drvdata(const struct mei_cl_device *dev)
+{
+	return dev_get_drvdata(&dev->dev);
+}
+EXPORT_SYMBOL_GPL(mei_cl_get_drvdata);
+
+void mei_cl_set_drvdata(struct mei_cl_device *dev, void *data)
+{
+	dev_set_drvdata(&dev->dev, data);
+}
+EXPORT_SYMBOL_GPL(mei_cl_set_drvdata);
+
 int __init mei_cl_bus_init(void)
 {
 	return bus_register(&mei_cl_bus_type);
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 7a5a6636f0fd..3c3245e93b22 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -558,7 +558,6 @@ void mei_cl_init(struct mei_cl *cl, struct mei_device *dev)
 	INIT_LIST_HEAD(&cl->rd_completed);
 	INIT_LIST_HEAD(&cl->rd_pending);
 	INIT_LIST_HEAD(&cl->link);
-	INIT_LIST_HEAD(&cl->device_link);
 	cl->writing_state = MEI_IDLE;
 	cl->state = MEI_FILE_INITIALIZING;
 	cl->dev = dev;
@@ -690,16 +689,12 @@ void mei_host_client_init(struct work_struct *work)
 		mei_wd_host_init(dev, me_cl);
 	mei_me_cl_put(me_cl);
 
-	me_cl = mei_me_cl_by_uuid(dev, &mei_nfc_guid);
-	if (me_cl)
-		mei_nfc_host_init(dev, me_cl);
-	mei_me_cl_put(me_cl);
-
-
 	dev->dev_state = MEI_DEV_ENABLED;
 	dev->reset_count = 0;
 	mutex_unlock(&dev->device_lock);
 
+	mei_cl_bus_rescan(dev);
+
 	pm_runtime_mark_last_busy(dev->dev);
 	dev_dbg(dev->dev, "rpm: autosuspend\n");
 	pm_runtime_autosuspend(dev->dev);
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 94514b2c7a50..94453d78b12b 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -331,8 +331,6 @@ void mei_stop(struct mei_device *dev)
 
 	mei_cancel_work(dev);
 
-	mei_nfc_host_exit(dev);
-
 	mei_cl_bus_remove_devices(dev);
 
 	mutex_lock(&dev->device_lock);
@@ -388,10 +386,11 @@ void mei_device_init(struct mei_device *dev,
 {
 	/* setup our list array */
 	INIT_LIST_HEAD(&dev->file_list);
-	INIT_LIST_HEAD(&dev->device_list);
+	INIT_LIST_HEAD(&dev->devices);
 	INIT_LIST_HEAD(&dev->me_clients);
 	mutex_init(&dev->device_lock);
 	init_rwsem(&dev->me_clients_rwsem);
+	mutex_init(&dev->cl_bus_lock);
 	init_waitqueue_head(&dev->wait_hw_ready);
 	init_waitqueue_head(&dev->wait_pg);
 	init_waitqueue_head(&dev->wait_hbm_start);
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 70a644f688b0..f8e353cde12d 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -178,7 +178,7 @@ struct mei_fw_status {
  * @client_id: me client id
  * @mei_flow_ctrl_creds: flow control credits
  * @connect_count: number connections to this client
- * @reserved: reserved
+ * @bus_added: added to bus
  */
 struct mei_me_client {
 	struct list_head list;
@@ -187,7 +187,7 @@ struct mei_me_client {
 	u8 client_id;
 	u8 mei_flow_ctrl_creds;
 	u8 connect_count;
-	u8 reserved;
+	u8 bus_added;
 };
 
 
@@ -241,7 +241,6 @@ struct mei_cl_cb {
  * @rd_completed: completed read
  *
  * @device: device on the mei client bus
- * @device_link:  link to bus clients
  */
 struct mei_cl {
 	struct list_head link;
@@ -260,9 +259,7 @@ struct mei_cl {
 	struct list_head rd_pending;
 	struct list_head rd_completed;
 
-	/* MEI CL bus data */
 	struct mei_cl_device *device;
-	struct list_head device_link;
 };
 
 /** struct mei_hw_ops
@@ -328,12 +325,9 @@ struct mei_hw_ops {
 
 /* MEI bus API*/
 
-struct mei_cl_device *mei_cl_add_device(struct mei_device *dev,
-					struct mei_me_client *me_cl,
-					struct mei_cl *cl,
-					char *name);
-void mei_cl_remove_device(struct mei_cl_device *device);
-
+void mei_cl_bus_rescan(struct mei_device *bus);
+void mei_cl_scan(struct mei_device *dev, struct mei_me_client *me_cl);
+void mei_cl_dev_fixup(struct mei_cl_device *dev);
 ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 			bool blocking);
 ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
@@ -341,7 +335,6 @@ void mei_cl_bus_rx_event(struct mei_cl *cl);
 void mei_cl_bus_remove_devices(struct mei_device *dev);
 int mei_cl_bus_init(void);
 void mei_cl_bus_exit(void);
-struct mei_cl *mei_cl_bus_find_cl_by_uuid(struct mei_device *dev, uuid_le uuid);
 
 /**
  * enum mei_pg_event - power gating transition events
@@ -440,7 +433,8 @@ const char *mei_pg_state_str(enum mei_pg_state state);
  * @init_work   : work item for the device init
  * @reset_work  : work item for the device reset
  *
- * @device_list : mei client bus list
+ * @cl_bus_lock : client bus list lock
+ * @devices     : mei client bus list
  *
  * @dbgfs_dir   : debugfs mei root directory
  *
@@ -535,8 +529,8 @@ struct mei_device {
 	struct work_struct init_work;
 	struct work_struct reset_work;
 
-	/* List of bus devices */
-	struct list_head device_list;
+	struct mutex cl_bus_lock;
+	struct list_head devices;
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 	struct dentry *dbgfs_dir;
diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
deleted file mode 100644
index b983c4ecad38..000000000000
--- a/drivers/misc/mei/nfc.c
+++ /dev/null
@@ -1,414 +0,0 @@
-/*
- *
- * Intel Management Engine Interface (Intel MEI) Linux driver
- * Copyright (c) 2003-2013, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/sched.h>
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/device.h>
-#include <linux/slab.h>
-
-#include <linux/mei_cl_bus.h>
-
-#include "mei_dev.h"
-#include "client.h"
-
-struct mei_nfc_cmd {
-	u8 command;
-	u8 status;
-	u16 req_id;
-	u32 reserved;
-	u16 data_size;
-	u8 sub_command;
-	u8 data[];
-} __packed;
-
-struct mei_nfc_reply {
-	u8 command;
-	u8 status;
-	u16 req_id;
-	u32 reserved;
-	u16 data_size;
-	u8 sub_command;
-	u8 reply_status;
-	u8 data[];
-} __packed;
-
-struct mei_nfc_if_version {
-	u8 radio_version_sw[3];
-	u8 reserved[3];
-	u8 radio_version_hw[3];
-	u8 i2c_addr;
-	u8 fw_ivn;
-	u8 vendor_id;
-	u8 radio_type;
-} __packed;
-
-struct mei_nfc_connect {
-	u8 fw_ivn;
-	u8 vendor_id;
-} __packed;
-
-struct mei_nfc_connect_resp {
-	u8 fw_ivn;
-	u8 vendor_id;
-	u16 me_major;
-	u16 me_minor;
-	u16 me_hotfix;
-	u16 me_build;
-} __packed;
-
-struct mei_nfc_hci_hdr {
-	u8 cmd;
-	u8 status;
-	u16 req_id;
-	u32 reserved;
-	u16 data_size;
-} __packed;
-
-#define MEI_NFC_CMD_MAINTENANCE 0x00
-#define MEI_NFC_CMD_HCI_SEND 0x01
-#define MEI_NFC_CMD_HCI_RECV 0x02
-
-#define MEI_NFC_SUBCMD_CONNECT    0x00
-#define MEI_NFC_SUBCMD_IF_VERSION 0x01
-
-#define MEI_NFC_HEADER_SIZE 10
-
-/**
- * struct mei_nfc_dev - NFC mei device
- *
- * @me_cl: NFC me client
- * @cl: NFC host client
- * @cl_info: NFC info host client
- * @init_work: perform connection to the info client
- * @fw_ivn: NFC Interface Version Number
- * @vendor_id: NFC manufacturer ID
- * @radio_type: NFC radio type
- * @bus_name: bus name
- *
- */
-struct mei_nfc_dev {
-	struct mei_me_client *me_cl;
-	struct mei_cl *cl;
-	struct mei_cl *cl_info;
-	struct work_struct init_work;
-	u8 fw_ivn;
-	u8 vendor_id;
-	u8 radio_type;
-	char *bus_name;
-};
-
-/* UUIDs for NFC F/W clients */
-const uuid_le mei_nfc_guid = UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50,
-				     0x94, 0xd4, 0x50, 0x26,
-				     0x67, 0x23, 0x77, 0x5c);
-
-static const uuid_le mei_nfc_info_guid = UUID_LE(0xd2de1625, 0x382d, 0x417d,
-					0x48, 0xa4, 0xef, 0xab,
-					0xba, 0x8a, 0x12, 0x06);
-
-/* Vendors */
-#define MEI_NFC_VENDOR_INSIDE 0x00
-#define MEI_NFC_VENDOR_NXP    0x01
-
-/* Radio types */
-#define MEI_NFC_VENDOR_INSIDE_UREAD 0x00
-#define MEI_NFC_VENDOR_NXP_PN544    0x01
-
-static void mei_nfc_free(struct mei_nfc_dev *ndev)
-{
-	if (!ndev)
-		return;
-
-	if (ndev->cl) {
-		list_del(&ndev->cl->device_link);
-		mei_cl_unlink(ndev->cl);
-		kfree(ndev->cl);
-	}
-
-	if (ndev->cl_info) {
-		list_del(&ndev->cl_info->device_link);
-		mei_cl_unlink(ndev->cl_info);
-		kfree(ndev->cl_info);
-	}
-
-	mei_me_cl_put(ndev->me_cl);
-	kfree(ndev);
-}
-
-static int mei_nfc_build_bus_name(struct mei_nfc_dev *ndev)
-{
-	struct mei_device *dev;
-
-	if (!ndev->cl)
-		return -ENODEV;
-
-	dev = ndev->cl->dev;
-
-	switch (ndev->vendor_id) {
-	case MEI_NFC_VENDOR_INSIDE:
-		switch (ndev->radio_type) {
-		case MEI_NFC_VENDOR_INSIDE_UREAD:
-			ndev->bus_name = "microread";
-			return 0;
-
-		default:
-			dev_err(dev->dev, "Unknown radio type 0x%x\n",
-				ndev->radio_type);
-
-			return -EINVAL;
-		}
-
-	case MEI_NFC_VENDOR_NXP:
-		switch (ndev->radio_type) {
-		case MEI_NFC_VENDOR_NXP_PN544:
-			ndev->bus_name = "pn544";
-			return 0;
-		default:
-			dev_err(dev->dev, "Unknown radio type 0x%x\n",
-				ndev->radio_type);
-
-			return -EINVAL;
-		}
-
-	default:
-		dev_err(dev->dev, "Unknown vendor ID 0x%x\n",
-			ndev->vendor_id);
-
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int mei_nfc_if_version(struct mei_nfc_dev *ndev)
-{
-	struct mei_device *dev;
-	struct mei_cl *cl;
-
-	struct mei_nfc_cmd cmd;
-	struct mei_nfc_reply *reply = NULL;
-	struct mei_nfc_if_version *version;
-	size_t if_version_length;
-	int bytes_recv, ret;
-
-	cl = ndev->cl_info;
-	dev = cl->dev;
-
-	memset(&cmd, 0, sizeof(struct mei_nfc_cmd));
-	cmd.command = MEI_NFC_CMD_MAINTENANCE;
-	cmd.data_size = 1;
-	cmd.sub_command = MEI_NFC_SUBCMD_IF_VERSION;
-
-	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd), 1);
-	if (ret < 0) {
-		dev_err(dev->dev, "Could not send IF version cmd\n");
-		return ret;
-	}
-
-	/* to be sure on the stack we alloc memory */
-	if_version_length = sizeof(struct mei_nfc_reply) +
-		sizeof(struct mei_nfc_if_version);
-
-	reply = kzalloc(if_version_length, GFP_KERNEL);
-	if (!reply)
-		return -ENOMEM;
-
-	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
-	if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
-		dev_err(dev->dev, "Could not read IF version\n");
-		ret = -EIO;
-		goto err;
-	}
-
-	version = (struct mei_nfc_if_version *)reply->data;
-
-	ndev->fw_ivn = version->fw_ivn;
-	ndev->vendor_id = version->vendor_id;
-	ndev->radio_type = version->radio_type;
-
-err:
-	kfree(reply);
-	return ret;
-}
-
-static void mei_nfc_init(struct work_struct *work)
-{
-	struct mei_device *dev;
-	struct mei_cl_device *cldev;
-	struct mei_nfc_dev *ndev;
-	struct mei_cl *cl_info;
-	struct mei_me_client *me_cl_info;
-
-	ndev = container_of(work, struct mei_nfc_dev, init_work);
-
-	cl_info = ndev->cl_info;
-	dev = cl_info->dev;
-
-	mutex_lock(&dev->device_lock);
-
-	/* check for valid client id */
-	me_cl_info = mei_me_cl_by_uuid(dev, &mei_nfc_info_guid);
-	if (!me_cl_info) {
-		mutex_unlock(&dev->device_lock);
-		dev_info(dev->dev, "nfc: failed to find the info client\n");
-		goto err;
-	}
-
-	if (mei_cl_connect(cl_info, me_cl_info, NULL) < 0) {
-		mei_me_cl_put(me_cl_info);
-		mutex_unlock(&dev->device_lock);
-		dev_err(dev->dev, "Could not connect to the NFC INFO ME client");
-
-		goto err;
-	}
-	mei_me_cl_put(me_cl_info);
-	mutex_unlock(&dev->device_lock);
-
-	if (mei_nfc_if_version(ndev) < 0) {
-		dev_err(dev->dev, "Could not get the NFC interface version");
-
-		goto err;
-	}
-
-	dev_info(dev->dev, "NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
-		ndev->fw_ivn, ndev->vendor_id, ndev->radio_type);
-
-	mutex_lock(&dev->device_lock);
-
-	if (mei_cl_disconnect(cl_info) < 0) {
-		mutex_unlock(&dev->device_lock);
-		dev_err(dev->dev, "Could not disconnect the NFC INFO ME client");
-
-		goto err;
-	}
-
-	mutex_unlock(&dev->device_lock);
-
-	if (mei_nfc_build_bus_name(ndev) < 0) {
-		dev_err(dev->dev, "Could not build the bus ID name\n");
-		return;
-	}
-
-	cldev = mei_cl_add_device(dev, ndev->me_cl, ndev->cl,
-				  ndev->bus_name);
-	if (!cldev) {
-		dev_err(dev->dev, "Could not add the NFC device to the MEI bus\n");
-
-		goto err;
-	}
-
-	cldev->priv_data = ndev;
-
-
-	return;
-
-err:
-	mutex_lock(&dev->device_lock);
-	mei_nfc_free(ndev);
-	mutex_unlock(&dev->device_lock);
-
-}
-
-
-int mei_nfc_host_init(struct mei_device *dev, struct mei_me_client *me_cl)
-{
-	struct mei_nfc_dev *ndev;
-	struct mei_cl *cl_info, *cl;
-	int ret;
-
-
-	/* in case of internal reset bail out
-	 * as the device is already setup
-	 */
-	cl = mei_cl_bus_find_cl_by_uuid(dev, mei_nfc_guid);
-	if (cl)
-		return 0;
-
-	ndev = kzalloc(sizeof(struct mei_nfc_dev), GFP_KERNEL);
-	if (!ndev) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	ndev->me_cl = mei_me_cl_get(me_cl);
-	if (!ndev->me_cl) {
-		ret = -ENODEV;
-		goto err;
-	}
-
-	cl_info = mei_cl_alloc_linked(dev, MEI_HOST_CLIENT_ID_ANY);
-	if (IS_ERR(cl_info)) {
-		ret = PTR_ERR(cl_info);
-		goto err;
-	}
-
-	list_add_tail(&cl_info->device_link, &dev->device_list);
-
-	ndev->cl_info = cl_info;
-
-	cl = mei_cl_alloc_linked(dev, MEI_HOST_CLIENT_ID_ANY);
-	if (IS_ERR(cl)) {
-		ret = PTR_ERR(cl);
-		goto err;
-	}
-
-	list_add_tail(&cl->device_link, &dev->device_list);
-
-	ndev->cl = cl;
-
-	INIT_WORK(&ndev->init_work, mei_nfc_init);
-	schedule_work(&ndev->init_work);
-
-	return 0;
-
-err:
-	mei_nfc_free(ndev);
-
-	return ret;
-}
-
-void mei_nfc_host_exit(struct mei_device *dev)
-{
-	struct mei_nfc_dev *ndev;
-	struct mei_cl *cl;
-	struct mei_cl_device *cldev;
-
-	cl = mei_cl_bus_find_cl_by_uuid(dev, mei_nfc_guid);
-	if (!cl)
-		return;
-
-	cldev = cl->device;
-	if (!cldev)
-		return;
-
-	ndev = (struct mei_nfc_dev *)cldev->priv_data;
-	if (ndev)
-		cancel_work_sync(&ndev->init_work);
-
-	cldev->priv_data = NULL;
-
-	mutex_lock(&dev->device_lock);
-	/* Need to remove the device here
-	 * since mei_nfc_free will unlink the clients
-	 */
-	mei_cl_remove_device(cldev);
-	mei_nfc_free(ndev);
-	mutex_unlock(&dev->device_lock);
-}
-
-
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index a16b1f9c1aca..905956045699 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -6,6 +6,7 @@
 #include <linux/mod_devicetable.h>
 
 struct mei_cl_device;
+struct mei_device;
 
 typedef void (*mei_cl_event_cb_t)(struct mei_cl_device *device,
 			       u32 events, void *context);
@@ -17,6 +18,8 @@ typedef void (*mei_cl_event_cb_t)(struct mei_cl_device *device,
  * Drivers for MEI devices will get an mei_cl_device pointer
  * when being probed and shall use it for doing ME bus I/O.
  *
+ * @bus_list: device on the bus list
+ * @bus: parent mei device
  * @dev: linux driver model device pointer
  * @me_cl: me client
  * @cl: mei client
@@ -26,9 +29,15 @@ typedef void (*mei_cl_event_cb_t)(struct mei_cl_device *device,
  *	events (e.g. Rx buffer pending) notifications.
  * @event_context: event callback run context
  * @events: Events bitmask sent to the driver.
+ *
+ * @do_match: wheather device can be matched with a driver
+ * @is_added: device is already scanned
  * @priv_data: client private data
  */
 struct mei_cl_device {
+	struct list_head bus_list;
+	struct mei_device *bus;
+
 	struct device dev;
 
 	struct mei_me_client *me_cl;
@@ -40,6 +49,9 @@ struct mei_cl_device {
 	void *event_context;
 	unsigned long events;
 
+	unsigned int do_match:1;
+	unsigned int is_added:1;
+
 	void *priv_data;
 };
 
-- 
2.1.0


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

* Re: [char-misc-next 11/11] mei: revamp mei bus code
  2015-05-07 12:54 ` [char-misc-next 11/11] mei: revamp mei bus code Tomas Winkler
@ 2015-05-24 18:19   ` Greg KH
  2015-05-24 21:29     ` Winkler, Tomas
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2015-05-24 18:19 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: arnd, linux-kernel, Samuel Ortiz

On Thu, May 07, 2015 at 03:54:08PM +0300, Tomas Winkler wrote:
> MEI bus was designed around nfc and was hard to extend.
> Instead of hard coded way of adding the devices on the mei bus
> we scan whole me client list and create a device for each
> eligible me client; currently we support only clients with
> single connection and fixed address clients.
> NFC radio name detection is run as a fixup routine
> 
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/Makefile    |   2 +-
>  drivers/misc/mei/bus-fixup.c | 306 ++++++++++++++
>  drivers/misc/mei/bus.c       | 975 ++++++++++++++++++++++++++++---------------
>  drivers/misc/mei/client.c    |   9 +-
>  drivers/misc/mei/init.c      |   5 +-
>  drivers/misc/mei/mei_dev.h   |  24 +-
>  drivers/misc/mei/nfc.c       | 414 ------------------
>  include/linux/mei_cl_bus.h   |  12 +
>  8 files changed, 968 insertions(+), 779 deletions(-)
>  create mode 100644 drivers/misc/mei/bus-fixup.c
>  delete mode 100644 drivers/misc/mei/nfc.c

This is a lot to do in just one patch.  Any chance you can split it up
into reviewable pieces?

thanks,

greg k-h

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

* RE: [char-misc-next 11/11] mei: revamp mei bus code
  2015-05-24 18:19   ` Greg KH
@ 2015-05-24 21:29     ` Winkler, Tomas
  2015-05-25  1:46       ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Winkler, Tomas @ 2015-05-24 21:29 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, linux-kernel, Samuel Ortiz

> 
> On Thu, May 07, 2015 at 03:54:08PM +0300, Tomas Winkler wrote:
> > MEI bus was designed around nfc and was hard to extend.
> > Instead of hard coded way of adding the devices on the mei bus
> > we scan whole me client list and create a device for each
> > eligible me client; currently we support only clients with
> > single connection and fixed address clients.
> > NFC radio name detection is run as a fixup routine
> >
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> >  drivers/misc/mei/Makefile    |   2 +-
> >  drivers/misc/mei/bus-fixup.c | 306 ++++++++++++++
> >  drivers/misc/mei/bus.c       | 975 ++++++++++++++++++++++++++++------------
> ---
> >  drivers/misc/mei/client.c    |   9 +-
> >  drivers/misc/mei/init.c      |   5 +-
> >  drivers/misc/mei/mei_dev.h   |  24 +-
> >  drivers/misc/mei/nfc.c       | 414 ------------------
> >  include/linux/mei_cl_bus.h   |  12 +
> >  8 files changed, 968 insertions(+), 779 deletions(-)
> >  create mode 100644 drivers/misc/mei/bus-fixup.c
> >  delete mode 100644 drivers/misc/mei/nfc.c
> 
> This is a lot to do in just one patch.  Any chance you can split it up
> into reviewable pieces?

I thought it would be harder to swallow but I'm not sure how to really 
split this into working pieces w/o do some artificial steps which
 I will have to validate again to keep the code bisectable. 
What could be naturally factored out is already in separate  patches in this series. 
The patch is maybe bigger because of code moves between files but what has really changes is just bus.c 
Please reconsider.
Thanks
Tomas 



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

* Re: [char-misc-next 11/11] mei: revamp mei bus code
  2015-05-24 21:29     ` Winkler, Tomas
@ 2015-05-25  1:46       ` Greg KH
  2015-05-25  4:40         ` Winkler, Tomas
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2015-05-25  1:46 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: arnd, linux-kernel, Samuel Ortiz

On Sun, May 24, 2015 at 09:29:41PM +0000, Winkler, Tomas wrote:
> > 
> > On Thu, May 07, 2015 at 03:54:08PM +0300, Tomas Winkler wrote:
> > > MEI bus was designed around nfc and was hard to extend.
> > > Instead of hard coded way of adding the devices on the mei bus
> > > we scan whole me client list and create a device for each
> > > eligible me client; currently we support only clients with
> > > single connection and fixed address clients.
> > > NFC radio name detection is run as a fixup routine
> > >
> > > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > ---
> > >  drivers/misc/mei/Makefile    |   2 +-
> > >  drivers/misc/mei/bus-fixup.c | 306 ++++++++++++++
> > >  drivers/misc/mei/bus.c       | 975 ++++++++++++++++++++++++++++------------
> > ---
> > >  drivers/misc/mei/client.c    |   9 +-
> > >  drivers/misc/mei/init.c      |   5 +-
> > >  drivers/misc/mei/mei_dev.h   |  24 +-
> > >  drivers/misc/mei/nfc.c       | 414 ------------------
> > >  include/linux/mei_cl_bus.h   |  12 +
> > >  8 files changed, 968 insertions(+), 779 deletions(-)
> > >  create mode 100644 drivers/misc/mei/bus-fixup.c
> > >  delete mode 100644 drivers/misc/mei/nfc.c
> > 
> > This is a lot to do in just one patch.  Any chance you can split it up
> > into reviewable pieces?
> 
> I thought it would be harder to swallow but I'm not sure how to really 
> split this into working pieces w/o do some artificial steps which
>  I will have to validate again to keep the code bisectable. 

That's fine, that's your job to do :)

> What could be naturally factored out is already in separate  patches in this series. 
> The patch is maybe bigger because of code moves between files but what has really changes is just bus.c 

Then do the movement of functions around in one patch, and then the
logical changes in others.  Come on, you know how this whole thing
works, don't be lazy here...

thanks,

greg k-h

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

* RE: [char-misc-next 11/11] mei: revamp mei bus code
  2015-05-25  1:46       ` Greg KH
@ 2015-05-25  4:40         ` Winkler, Tomas
  2015-05-25 16:20           ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Winkler, Tomas @ 2015-05-25  4:40 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, linux-kernel, Samuel Ortiz


> > > This is a lot to do in just one patch.  Any chance you can split it up
> > > into reviewable pieces?
> >
> > I thought it would be harder to swallow but I'm not sure how to really
> > split this into working pieces w/o do some artificial steps which
> >  I will have to validate again to keep the code bisectable.
> 
> That's fine, that's your job to do :)

I'd would prefer to do something that wasn't' already done.

> 
> > What could be naturally factored out is already in separate  patches in this
> series.
> > The patch is maybe bigger because of code moves between files but what has
> really changes is just bus.c
> 
> Then do the movement of functions around in one patch, and then the
> logical changes in others.  Come on, you know how this whole thing
> works, don't be lazy here...
> 
Yep, I know how it works, but here it just didn't fit as the logical changes moved also the code.
Okay, I will split the code but allow me one comment, I think that you were lazy too  go into details and  you didn't send us to hell when we first submitted that bus code.
Tomas



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

* Re: [char-misc-next 11/11] mei: revamp mei bus code
  2015-05-25  4:40         ` Winkler, Tomas
@ 2015-05-25 16:20           ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2015-05-25 16:20 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: arnd, linux-kernel, Samuel Ortiz

On Mon, May 25, 2015 at 04:40:50AM +0000, Winkler, Tomas wrote:
> 
> > > > This is a lot to do in just one patch.  Any chance you can split it up
> > > > into reviewable pieces?
> > >
> > > I thought it would be harder to swallow but I'm not sure how to really
> > > split this into working pieces w/o do some artificial steps which
> > >  I will have to validate again to keep the code bisectable.
> > 
> > That's fine, that's your job to do :)
> 
> I'd would prefer to do something that wasn't' already done.

Huh?  Your job is to split changes up into tiny logical pieces that are
easy to review.  That isn't "done" here.

> > > What could be naturally factored out is already in separate  patches in this
> > series.
> > > The patch is maybe bigger because of code moves between files but what has
> > really changes is just bus.c
> > 
> > Then do the movement of functions around in one patch, and then the
> > logical changes in others.  Come on, you know how this whole thing
> > works, don't be lazy here...
> > 
> Yep, I know how it works, but here it just didn't fit as the logical changes moved also the code.
> Okay, I will split the code but allow me one comment, I think that you were lazy too  go into details and  you didn't send us to hell when we first submitted that bus code.

So because I wasn't hard enough on your initial reviews of this code,
you are now blaming me for this needed rework?

I seem to recall that I gave a lot of feedback on those original
patches, but if you want me to be harder and take a much closer look at
all of your patches, I'll be glad to do so.  Be aware, that this is
going to slow down the acceptance rates of your patches, and I am going
to now be _very_ pedantic and grumpy.

Remember, you asked for it, this is going to be fun, for me, not you...

greg k-h

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

end of thread, other threads:[~2015-05-25 16:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-07 12:53 [char-misc-next 01/11] mei: consume flow control on the first chunk of writing Tomas Winkler
2015-05-07 12:53 ` [char-misc-next 02/11] mei: request autosuspend at the end of write Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 03/11] mei: add also write waiting list to runtime pm blockers Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 04/11] uuid: extract macros for assigning raw arrays Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 05/11] mei: bus: report also uuid in module alias Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 06/11] mei: bus: add name and uuid into device attributes Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 07/11] NFC: mei_phy: move all nfc logic from mei driver to nfc Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 08/11] mei: bus: kill mei_cl_ops Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 09/11] NFC: mei_phy: adjust mei nfc header according the spec Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 10/11] mei: export mei client device struct to external use Tomas Winkler
2015-05-07 12:54 ` [char-misc-next 11/11] mei: revamp mei bus code Tomas Winkler
2015-05-24 18:19   ` Greg KH
2015-05-24 21:29     ` Winkler, Tomas
2015-05-25  1:46       ` Greg KH
2015-05-25  4:40         ` Winkler, Tomas
2015-05-25 16:20           ` Greg KH

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).