[char-misc-next 0/4] mei: bus: rx fixes

[char-misc-next 0/4] mei: bus: rx fixes

[Tomas Winkler]

The motivation behind this series is better support
for fixed address clients on the mei client bus.
Fixed address clients do not have flow control and hence
it is hard to work with clients especially if they have
unsolicitedreceive.
The top most patch add non blocking receive function for easier
check on data availability.


Alexander Usyskin (4):
  mei: introduce host client uninitialized state
  mei: bus: make a client pointer always available
  mei: bus: split RX and async notification callbacks
  mei: bus: enable non-blocking RX

 drivers/misc/mei/bus-fixup.c |   4 +-
 drivers/misc/mei/bus.c       | 189 +++++++++++++++++++++++++------------------
 drivers/misc/mei/client.c    |  11 ++-
 drivers/misc/mei/mei_dev.h   |  10 ++-
 drivers/nfc/mei_phy.c        |  43 +++++-----
 drivers/watchdog/mei_wdt.c   |  38 +++------
 include/linux/mei_cl_bus.h   |  35 ++++----
 7 files changed, 178 insertions(+), 152 deletions(-)

-- 
2.7.4

[char-misc-next 1/4] mei: introduce host client uninitialized state

[Tomas Winkler]

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

Introduce a new host client state, MEI_FILE_UNINITIALIZED,
to distinguish client objects that was unlinked,
but not destroyed and can be linked again.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/client.c  | 6 +++---
 drivers/misc/mei/mei_dev.h | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index c71cee8bd957..46ee9155ada6 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -571,7 +571,7 @@ void mei_cl_init(struct mei_cl *cl, struct mei_device *dev)
 	INIT_LIST_HEAD(&cl->rd_pending);
 	INIT_LIST_HEAD(&cl->link);
 	cl->writing_state = MEI_IDLE;
-	cl->state = MEI_FILE_INITIALIZING;
+	cl->state = MEI_FILE_UNINITIALIZED;
 	cl->dev = dev;
 }
 
@@ -672,7 +672,7 @@ int mei_cl_unlink(struct mei_cl *cl)
 
 	list_del_init(&cl->link);
 
-	cl->state = MEI_FILE_INITIALIZING;
+	cl->state = MEI_FILE_UNINITIALIZED;
 
 	return 0;
 }
@@ -756,7 +756,7 @@ void mei_cl_set_disconnected(struct mei_cl *cl)
 	struct mei_device *dev = cl->dev;
 
 	if (cl->state == MEI_FILE_DISCONNECTED ||
-	    cl->state == MEI_FILE_INITIALIZING)
+	    cl->state <= MEI_FILE_INITIALIZING)
 		return;
 
 	cl->state = MEI_FILE_DISCONNECTED;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 93b150315b93..82e69a00b7a1 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -55,7 +55,8 @@ extern const uuid_le mei_amthif_guid;
 
 /* File state */
 enum file_state {
-	MEI_FILE_INITIALIZING = 0,
+	MEI_FILE_UNINITIALIZED = 0,
+	MEI_FILE_INITIALIZING,
 	MEI_FILE_CONNECTING,
 	MEI_FILE_CONNECTED,
 	MEI_FILE_DISCONNECTING,
-- 
2.7.4

[char-misc-next 2/4] mei: bus: make a client pointer always available

[Tomas Winkler]

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

Change life time of the client pointer, allocate it upon client device
creation and free it upon device destruction, instead of upon
connection and disconnection.
This helps to eliminate racy NULL checks in the bus code.

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

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 7c075e9b25e5..483587f60249 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -187,9 +187,6 @@ ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length)
 {
 	struct mei_cl *cl = cldev->cl;
 
-	if (cl == NULL)
-		return -ENODEV;
-
 	return __mei_cl_send(cl, buf, length, MEI_CL_IO_TX_BLOCKING);
 }
 EXPORT_SYMBOL_GPL(mei_cldev_send);
@@ -207,9 +204,6 @@ ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
 {
 	struct mei_cl *cl = cldev->cl;
 
-	if (cl == NULL)
-		return -ENODEV;
-
 	return __mei_cl_recv(cl, buf, length);
 }
 EXPORT_SYMBOL_GPL(mei_cldev_recv);
@@ -403,7 +397,7 @@ EXPORT_SYMBOL_GPL(mei_cldev_ver);
  */
 bool mei_cldev_enabled(struct mei_cl_device *cldev)
 {
-	return cldev->cl && mei_cl_is_connected(cldev->cl);
+	return mei_cl_is_connected(cldev->cl);
 }
 EXPORT_SYMBOL_GPL(mei_cldev_enabled);
 
@@ -423,14 +417,13 @@ int mei_cldev_enable(struct mei_cl_device *cldev)
 
 	cl = cldev->cl;
 
-	if (!cl) {
+	if (cl->state == MEI_FILE_UNINITIALIZED) {
 		mutex_lock(&bus->device_lock);
-		cl = mei_cl_alloc_linked(bus);
+		ret = mei_cl_link(cl);
 		mutex_unlock(&bus->device_lock);
-		if (IS_ERR(cl))
-			return PTR_ERR(cl);
+		if (ret)
+			return ret;
 		/* update pointers */
-		cldev->cl = cl;
 		cl->cldev = cldev;
 	}
 
@@ -471,7 +464,7 @@ int mei_cldev_disable(struct mei_cl_device *cldev)
 	struct mei_cl *cl;
 	int err;
 
-	if (!cldev || !cldev->cl)
+	if (!cldev)
 		return -ENODEV;
 
 	cl = cldev->cl;
@@ -497,9 +490,6 @@ int mei_cldev_disable(struct mei_cl_device *cldev)
 	mei_cl_flush_queues(cl, NULL);
 	mei_cl_unlink(cl);
 
-	kfree(cl);
-	cldev->cl = NULL;
-
 	mutex_unlock(&bus->device_lock);
 	return err;
 }
@@ -754,6 +744,7 @@ static void mei_cl_bus_dev_release(struct device *dev)
 
 	mei_me_cl_put(cldev->me_cl);
 	mei_dev_bus_put(cldev->bus);
+	kfree(cldev->cl);
 	kfree(cldev);
 }
 
@@ -786,17 +777,25 @@ static struct mei_cl_device *mei_cl_bus_dev_alloc(struct mei_device *bus,
 						  struct mei_me_client *me_cl)
 {
 	struct mei_cl_device *cldev;
+	struct mei_cl *cl;
 
 	cldev = kzalloc(sizeof(struct mei_cl_device), GFP_KERNEL);
 	if (!cldev)
 		return NULL;
 
+	cl = mei_cl_allocate(bus);
+	if (!cl) {
+		kfree(cldev);
+		return NULL;
+	}
+
 	device_initialize(&cldev->dev);
 	cldev->dev.parent = bus->dev;
 	cldev->dev.bus    = &mei_cl_bus_type;
 	cldev->dev.type   = &mei_cl_device_type;
 	cldev->bus        = mei_dev_bus_get(bus);
 	cldev->me_cl      = mei_me_cl_get(me_cl);
+	cldev->cl         = cl;
 	mei_cl_bus_set_name(cldev);
 	cldev->is_added   = 0;
 	INIT_LIST_HEAD(&cldev->bus_list);
-- 
2.7.4

[char-misc-next 3/4] mei: bus: split RX and async notification callbacks

[Tomas Winkler]

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

Split callbacks for RX and async notification events on mei bus to
eliminate synchronization problems and to open way for RX optimizations.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c     | 141 ++++++++++++++++++++++++++-------------------
 drivers/misc/mei/client.c  |   5 ++
 drivers/nfc/mei_phy.c      |  36 +++++-------
 drivers/watchdog/mei_wdt.c |  36 ++++--------
 include/linux/mei_cl_bus.h |  32 +++++-----
 5 files changed, 128 insertions(+), 122 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 483587f60249..2fd254ecde2f 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -209,31 +209,40 @@ ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
 EXPORT_SYMBOL_GPL(mei_cldev_recv);
 
 /**
- * mei_cl_bus_event_work  - dispatch rx event for a bus device
- *    and schedule new work
+ * mei_cl_bus_rx_work - dispatch rx event for a bus device
  *
  * @work: work
  */
-static void mei_cl_bus_event_work(struct work_struct *work)
+static void mei_cl_bus_rx_work(struct work_struct *work)
 {
 	struct mei_cl_device *cldev;
 	struct mei_device *bus;
 
-	cldev = container_of(work, struct mei_cl_device, event_work);
+	cldev = container_of(work, struct mei_cl_device, rx_work);
 
 	bus = cldev->bus;
 
-	if (cldev->event_cb)
-		cldev->event_cb(cldev, cldev->events);
+	if (cldev->rx_cb)
+		cldev->rx_cb(cldev);
 
-	cldev->events = 0;
+	mutex_lock(&bus->device_lock);
+	mei_cl_read_start(cldev->cl, mei_cl_mtu(cldev->cl), NULL);
+	mutex_unlock(&bus->device_lock);
+}
 
-	/* Prepare for the next read */
-	if (cldev->events_mask & BIT(MEI_CL_EVENT_RX)) {
-		mutex_lock(&bus->device_lock);
-		mei_cl_read_start(cldev->cl, mei_cl_mtu(cldev->cl), NULL);
-		mutex_unlock(&bus->device_lock);
-	}
+/**
+ * mei_cl_bus_notif_work - dispatch FW notif event for a bus device
+ *
+ * @work: work
+ */
+static void mei_cl_bus_notif_work(struct work_struct *work)
+{
+	struct mei_cl_device *cldev;
+
+	cldev = container_of(work, struct mei_cl_device, notif_work);
+
+	if (cldev->notif_cb)
+		cldev->notif_cb(cldev);
 }
 
 /**
@@ -248,18 +257,13 @@ bool mei_cl_bus_notify_event(struct mei_cl *cl)
 {
 	struct mei_cl_device *cldev = cl->cldev;
 
-	if (!cldev || !cldev->event_cb)
-		return false;
-
-	if (!(cldev->events_mask & BIT(MEI_CL_EVENT_NOTIF)))
+	if (!cldev || !cldev->notif_cb)
 		return false;
 
 	if (!cl->notify_ev)
 		return false;
 
-	set_bit(MEI_CL_EVENT_NOTIF, &cldev->events);
-
-	schedule_work(&cldev->event_work);
+	schedule_work(&cldev->notif_work);
 
 	cl->notify_ev = false;
 
@@ -267,7 +271,7 @@ bool mei_cl_bus_notify_event(struct mei_cl *cl)
 }
 
 /**
- * mei_cl_bus_rx_event  - schedule rx event
+ * mei_cl_bus_rx_event - schedule rx event
  *
  * @cl: host client
  *
@@ -278,64 +282,81 @@ bool mei_cl_bus_rx_event(struct mei_cl *cl)
 {
 	struct mei_cl_device *cldev = cl->cldev;
 
-	if (!cldev || !cldev->event_cb)
-		return false;
-
-	if (!(cldev->events_mask & BIT(MEI_CL_EVENT_RX)))
+	if (!cldev || !cldev->rx_cb)
 		return false;
 
-	set_bit(MEI_CL_EVENT_RX, &cldev->events);
-
-	schedule_work(&cldev->event_work);
+	schedule_work(&cldev->rx_work);
 
 	return true;
 }
 
 /**
- * mei_cldev_register_event_cb - register event callback
+ * mei_cldev_register_rx_cb - register Rx event callback
  *
  * @cldev: me client devices
- * @event_cb: callback function
- * @events_mask: requested events bitmask
+ * @rx_cb: callback function
  *
  * Return: 0 on success
  *         -EALREADY if an callback is already registered
  *         <0 on other errors
  */
-int mei_cldev_register_event_cb(struct mei_cl_device *cldev,
-				unsigned long events_mask,
-				mei_cldev_event_cb_t event_cb)
+int mei_cldev_register_rx_cb(struct mei_cl_device *cldev, mei_cldev_cb_t rx_cb)
 {
 	struct mei_device *bus = cldev->bus;
 	int ret;
 
-	if (cldev->event_cb)
+	if (!rx_cb)
+		return -EINVAL;
+	if (cldev->rx_cb)
 		return -EALREADY;
 
-	cldev->events = 0;
-	cldev->events_mask = events_mask;
-	cldev->event_cb = event_cb;
-	INIT_WORK(&cldev->event_work, mei_cl_bus_event_work);
+	cldev->rx_cb = rx_cb;
+	INIT_WORK(&cldev->rx_work, mei_cl_bus_rx_work);
 
-	if (cldev->events_mask & BIT(MEI_CL_EVENT_RX)) {
-		mutex_lock(&bus->device_lock);
-		ret = mei_cl_read_start(cldev->cl, mei_cl_mtu(cldev->cl), NULL);
-		mutex_unlock(&bus->device_lock);
-		if (ret && ret != -EBUSY)
-			return ret;
-	}
+	mutex_lock(&bus->device_lock);
+	ret = mei_cl_read_start(cldev->cl, mei_cl_mtu(cldev->cl), NULL);
+	mutex_unlock(&bus->device_lock);
+	if (ret && ret != -EBUSY)
+		return ret;
 
-	if (cldev->events_mask & BIT(MEI_CL_EVENT_NOTIF)) {
-		mutex_lock(&bus->device_lock);
-		ret = mei_cl_notify_request(cldev->cl, NULL, event_cb ? 1 : 0);
-		mutex_unlock(&bus->device_lock);
-		if (ret)
-			return ret;
-	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mei_cldev_register_rx_cb);
+
+/**
+ * mei_cldev_register_notif_cb - register FW notification event callback
+ *
+ * @cldev: me client devices
+ * @notif_cb: callback function
+ *
+ * Return: 0 on success
+ *         -EALREADY if an callback is already registered
+ *         <0 on other errors
+ */
+int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
+				mei_cldev_cb_t notif_cb)
+{
+	struct mei_device *bus = cldev->bus;
+	int ret;
+
+	if (!notif_cb)
+		return -EINVAL;
+
+	if (cldev->notif_cb)
+		return -EALREADY;
+
+	cldev->notif_cb = notif_cb;
+	INIT_WORK(&cldev->notif_work, mei_cl_bus_notif_work);
+
+	mutex_lock(&bus->device_lock);
+	ret = mei_cl_notify_request(cldev->cl, NULL, 1);
+	mutex_unlock(&bus->device_lock);
+	if (ret)
+		return ret;
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(mei_cldev_register_event_cb);
+EXPORT_SYMBOL_GPL(mei_cldev_register_notif_cb);
 
 /**
  * mei_cldev_get_drvdata - driver data getter
@@ -471,8 +492,6 @@ int mei_cldev_disable(struct mei_cl_device *cldev)
 
 	bus = cldev->bus;
 
-	cldev->event_cb = NULL;
-
 	mutex_lock(&bus->device_lock);
 
 	if (!mei_cl_is_connected(cl)) {
@@ -619,9 +638,13 @@ static int mei_cl_device_remove(struct device *dev)
 	if (!cldev || !dev->driver)
 		return 0;
 
-	if (cldev->event_cb) {
-		cldev->event_cb = NULL;
-		cancel_work_sync(&cldev->event_work);
+	if (cldev->rx_cb) {
+		cancel_work_sync(&cldev->rx_work);
+		cldev->rx_cb = NULL;
+	}
+	if (cldev->notif_cb) {
+		cancel_work_sync(&cldev->notif_work);
+		cldev->notif_cb = NULL;
 	}
 
 	cldrv = to_mei_cl_driver(dev->driver);
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 46ee9155ada6..9635b14b6011 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -673,6 +673,11 @@ int mei_cl_unlink(struct mei_cl *cl)
 	list_del_init(&cl->link);
 
 	cl->state = MEI_FILE_UNINITIALIZED;
+	cl->writing_state = MEI_IDLE;
+
+	WARN_ON(!list_empty(&cl->rd_completed) ||
+		!list_empty(&cl->rd_pending) ||
+		!list_empty(&cl->link));
 
 	return 0;
 }
diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 03139c5a05e4..8a04c5e02999 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -297,9 +297,11 @@ static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
 }
 
 
-static void nfc_mei_event_cb(struct mei_cl_device *cldev, u32 events)
+static void nfc_mei_rx_cb(struct mei_cl_device *cldev)
 {
 	struct nfc_mei_phy *phy = mei_cldev_get_drvdata(cldev);
+	struct sk_buff *skb;
+	int reply_size;
 
 	if (!phy)
 		return;
@@ -307,27 +309,22 @@ static void nfc_mei_event_cb(struct mei_cl_device *cldev, u32 events)
 	if (phy->hard_fault != 0)
 		return;
 
-	if (events & BIT(MEI_CL_EVENT_RX)) {
-		struct sk_buff *skb;
-		int reply_size;
-
-		skb = alloc_skb(MEI_NFC_MAX_READ, GFP_KERNEL);
-		if (!skb)
-			return;
+	skb = alloc_skb(MEI_NFC_MAX_READ, GFP_KERNEL);
+	if (!skb)
+		return;
 
-		reply_size = mei_nfc_recv(phy, skb->data, MEI_NFC_MAX_READ);
-		if (reply_size < MEI_NFC_HEADER_SIZE) {
-			kfree_skb(skb);
-			return;
-		}
+	reply_size = mei_nfc_recv(phy, skb->data, MEI_NFC_MAX_READ);
+	if (reply_size < MEI_NFC_HEADER_SIZE) {
+		kfree_skb(skb);
+		return;
+	}
 
-		skb_put(skb, reply_size);
-		skb_pull(skb, MEI_NFC_HEADER_SIZE);
+	skb_put(skb, reply_size);
+	skb_pull(skb, MEI_NFC_HEADER_SIZE);
 
-		MEI_DUMP_SKB_IN("mei frame read", skb);
+	MEI_DUMP_SKB_IN("mei frame read", skb);
 
-		nfc_hci_recv_frame(phy->hdev, skb);
-	}
+	nfc_hci_recv_frame(phy->hdev, skb);
 }
 
 static int nfc_mei_phy_enable(void *phy_id)
@@ -358,8 +355,7 @@ static int nfc_mei_phy_enable(void *phy_id)
 		goto err;
 	}
 
-	r = mei_cldev_register_event_cb(phy->cldev, BIT(MEI_CL_EVENT_RX),
-					nfc_mei_event_cb);
+	r = mei_cldev_register_rx_cb(phy->cldev, nfc_mei_rx_cb);
 	if (r) {
 		pr_err("Event cb registration failed %d\n", r);
 		goto err;
diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index 40953fe4db86..b29c6fde7473 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -412,11 +412,11 @@ static void mei_wdt_unregister_work(struct work_struct *work)
 }
 
 /**
- * mei_wdt_event_rx - callback for data receive
+ * mei_wdt_rx - callback for data receive
  *
  * @cldev: bus device
  */
-static void mei_wdt_event_rx(struct mei_cl_device *cldev)
+static void mei_wdt_rx(struct mei_cl_device *cldev)
 {
 	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
 	struct mei_wdt_start_response res;
@@ -484,11 +484,11 @@ static void mei_wdt_event_rx(struct mei_cl_device *cldev)
 }
 
 /*
- * mei_wdt_notify_event - callback for event notification
+ * mei_wdt_notif - callback for event notification
  *
  * @cldev: bus device
  */
-static void mei_wdt_notify_event(struct mei_cl_device *cldev)
+static void mei_wdt_notif(struct mei_cl_device *cldev)
 {
 	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
 
@@ -498,21 +498,6 @@ static void mei_wdt_notify_event(struct mei_cl_device *cldev)
 	mei_wdt_register(wdt);
 }
 
-/**
- * mei_wdt_event - callback for event receive
- *
- * @cldev: bus device
- * @events: event mask
- */
-static void mei_wdt_event(struct mei_cl_device *cldev, u32 events)
-{
-	if (events & BIT(MEI_CL_EVENT_RX))
-		mei_wdt_event_rx(cldev);
-
-	if (events & BIT(MEI_CL_EVENT_NOTIF))
-		mei_wdt_notify_event(cldev);
-}
-
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 
 static ssize_t mei_dbgfs_read_activation(struct file *file, char __user *ubuf,
@@ -623,16 +608,17 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
 		goto err_out;
 	}
 
-	ret = mei_cldev_register_event_cb(wdt->cldev,
-					  BIT(MEI_CL_EVENT_RX) |
-					  BIT(MEI_CL_EVENT_NOTIF),
-					  mei_wdt_event);
+	ret = mei_cldev_register_rx_cb(wdt->cldev, mei_wdt_rx);
+	if (ret) {
+		dev_err(&cldev->dev, "Could not reg rx event ret=%d\n", ret);
+		goto err_disable;
+	}
 
+	ret = mei_cldev_register_notif_cb(wdt->cldev, mei_wdt_notif);
 	/* on legacy devices notification is not supported
-	 * this doesn't fail the registration for RX event
 	 */
 	if (ret && ret != -EOPNOTSUPP) {
-		dev_err(&cldev->dev, "Could not register event ret=%d\n", ret);
+		dev_err(&cldev->dev, "Could not reg notif event ret=%d\n", ret);
 		goto err_disable;
 	}
 
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index 4adb2e7c9f84..017f5232b3de 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -8,8 +8,7 @@
 struct mei_cl_device;
 struct mei_device;
 
-typedef void (*mei_cldev_event_cb_t)(struct mei_cl_device *cldev,
-				     u32 events);
+typedef void (*mei_cldev_cb_t)(struct mei_cl_device *cldev);
 
 /**
  * struct mei_cl_device - MEI device handle
@@ -24,11 +23,12 @@ typedef void (*mei_cldev_event_cb_t)(struct mei_cl_device *cldev,
  * @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.
- * @events_mask: Events bit mask requested by driver.
- * @events: Events bitmask sent to the driver.
+ * @rx_work: async work to execute Rx event callback
+ * @rx_cb: Drivers register this callback to get asynchronous ME
+ *	Rx buffer pending notifications.
+ * @notif_work: async work to execute FW notif event callback
+ * @notif_cb: Drivers register this callback to get asynchronous ME
+ *	FW notification pending notifications.
  *
  * @do_match: wheather device can be matched with a driver
  * @is_added: device is already scanned
@@ -43,10 +43,10 @@ struct mei_cl_device {
 	struct mei_cl *cl;
 	char name[MEI_CL_NAME_SIZE];
 
-	struct work_struct event_work;
-	mei_cldev_event_cb_t event_cb;
-	unsigned long events_mask;
-	unsigned long events;
+	struct work_struct rx_work;
+	mei_cldev_cb_t rx_cb;
+	struct work_struct notif_work;
+	mei_cldev_cb_t notif_cb;
 
 	unsigned int do_match:1;
 	unsigned int is_added:1;
@@ -88,13 +88,9 @@ void mei_cldev_driver_unregister(struct mei_cl_driver *cldrv);
 ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length);
 ssize_t  mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length);
 
-int mei_cldev_register_event_cb(struct mei_cl_device *cldev,
-				unsigned long event_mask,
-				mei_cldev_event_cb_t read_cb);
-
-#define MEI_CL_EVENT_RX 0
-#define MEI_CL_EVENT_TX 1
-#define MEI_CL_EVENT_NOTIF 2
+int mei_cldev_register_rx_cb(struct mei_cl_device *cldev, mei_cldev_cb_t rx_cb);
+int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
+				mei_cldev_cb_t notif_cb);
 
 const uuid_le *mei_cldev_uuid(const struct mei_cl_device *cldev);
 u8 mei_cldev_ver(const struct mei_cl_device *cldev);
-- 
2.7.4

[char-misc-next 4/4] mei: bus: enable non-blocking RX

[Tomas Winkler]

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

Enable non-blocking receive for drivers on mei bus, this allows checking
for data availability by mei client drivers. This is most effective for
fixed address clients, that lacks flow control.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus-fixup.c |  4 ++--
 drivers/misc/mei/bus.c       | 19 ++++++++++++++++---
 drivers/misc/mei/mei_dev.h   |  7 ++++++-
 drivers/nfc/mei_phy.c        |  7 ++++---
 drivers/watchdog/mei_wdt.c   |  2 +-
 include/linux/mei_cl_bus.h   |  3 ++-
 6 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
index 7f2cef9011ae..18e05ca7584f 100644
--- a/drivers/misc/mei/bus-fixup.c
+++ b/drivers/misc/mei/bus-fixup.c
@@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device *cldev)
 	if (ret < 0)
 		return ret;
 
-	ret = __mei_cl_recv(cldev->cl, buf, length);
+	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
 	if (ret < 0)
 		return ret;
 
@@ -272,7 +272,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 		return -ENOMEM;
 
 	ret = 0;
-	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
+	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, 0);
 	if (bytes_recv < if_version_length) {
 		dev_err(bus->dev, "Could not read IF version\n");
 		ret = -EIO;
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 2fd254ecde2f..45e937937675 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -98,15 +98,18 @@ ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
  * @cl: host client
  * @buf: buffer to receive
  * @length: buffer length
+ * @mode: io mode
  *
  * Return: read size in bytes of < 0 on error
  */
-ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
+ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
+		      unsigned int mode)
 {
 	struct mei_device *bus;
 	struct mei_cl_cb *cb;
 	size_t r_length;
 	ssize_t rets;
+	bool nonblock = !!(mode & MEI_CL_IO_RX_NONBLOCK);
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -ENODEV;
@@ -127,6 +130,11 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 	if (rets && rets != -EBUSY)
 		goto out;
 
+	if (nonblock) {
+		rets = -EAGAIN;
+		goto out;
+	}
+
 	/* wait on event only if there is no other waiter */
 	/* synchronized under device mutex */
 	if (!waitqueue_active(&cl->rx_wait)) {
@@ -197,14 +205,19 @@ EXPORT_SYMBOL_GPL(mei_cldev_send);
  * @cldev: me client device
  * @buf: buffer to receive
  * @length: buffer length
+ * @flags: read io flags [O_NONBLOCK]
  *
  * Return: read size in bytes of < 0 on error
  */
-ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
+ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
+		       unsigned int flags)
 {
 	struct mei_cl *cl = cldev->cl;
+	unsigned int mode;
+
+	mode = (flags & O_NONBLOCK) ? MEI_CL_IO_RX_NONBLOCK : 0;
 
-	return __mei_cl_recv(cl, buf, length);
+	return __mei_cl_recv(cl, buf, length, mode);
 }
 EXPORT_SYMBOL_GPL(mei_cldev_recv);
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 82e69a00b7a1..f16bd1209848 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -115,10 +115,14 @@ enum mei_cb_file_ops {
  *
  * @MEI_CL_IO_TX_BLOCKING: send is blocking
  * @MEI_CL_IO_TX_INTERNAL: internal communication between driver and FW
+ *
+ * @MEI_CL_IO_RX_NONBLOCK: recv is non-blocking
  */
 enum mei_cl_io_mode {
 	MEI_CL_IO_TX_BLOCKING = BIT(0),
 	MEI_CL_IO_TX_INTERNAL = BIT(1),
+
+	MEI_CL_IO_RX_NONBLOCK = BIT(2),
 };
 
 /*
@@ -318,7 +322,8 @@ void mei_cl_bus_rescan_work(struct work_struct *work);
 void mei_cl_bus_dev_fixup(struct mei_cl_device *dev);
 ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 		      unsigned int mode);
-ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
+ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
+		      unsigned int mode);
 bool mei_cl_bus_rx_event(struct mei_cl *cl);
 bool mei_cl_bus_notify_event(struct mei_cl *cl);
 void mei_cl_bus_remove_devices(struct mei_device *bus);
diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
index 8a04c5e02999..0e8158c5f81b 100644
--- a/drivers/nfc/mei_phy.c
+++ b/drivers/nfc/mei_phy.c
@@ -132,7 +132,8 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
 	if (!reply)
 		return -ENOMEM;
 
-	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, if_version_length);
+	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
+				    if_version_length, 0);
 	if (bytes_recv < 0 || bytes_recv < if_version_length) {
 		pr_err("Could not read IF version\n");
 		r = -EIO;
@@ -193,7 +194,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
 	}
 
 	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
-				    connect_resp_length);
+				    connect_resp_length, 0);
 	if (bytes_recv < 0) {
 		r = bytes_recv;
 		pr_err("Could not read connect response %d\n", r);
@@ -279,7 +280,7 @@ static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
 	struct mei_nfc_hdr *hdr;
 	int received_length;
 
-	received_length = mei_cldev_recv(phy->cldev, buf, length);
+	received_length = mei_cldev_recv(phy->cldev, buf, length, 0);
 	if (received_length < 0)
 		return received_length;
 
diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index b29c6fde7473..3e29af534f8e 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -423,7 +423,7 @@ static void mei_wdt_rx(struct mei_cl_device *cldev)
 	const size_t res_len = sizeof(res);
 	int ret;
 
-	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len);
+	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len, 0);
 	if (ret < 0) {
 		dev_err(&cldev->dev, "failure in recv %d\n", ret);
 		return;
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index 017f5232b3de..c3bb565f0745 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -86,7 +86,8 @@ void mei_cldev_driver_unregister(struct mei_cl_driver *cldrv);
 		      mei_cldev_driver_unregister)
 
 ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length);
-ssize_t  mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length);
+ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
+		       unsigned int flags);
 
 int mei_cldev_register_rx_cb(struct mei_cl_device *cldev, mei_cldev_cb_t rx_cb);
 int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
-- 
2.7.4

Re: [char-misc-next 4/4] mei: bus: enable non-blocking RX

[Greg Kroah-Hartman]

On Wed, Nov 16, 2016 at 10:51:30PM +0200, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> Enable non-blocking receive for drivers on mei bus, this allows checking
> for data availability by mei client drivers. This is most effective for
> fixed address clients, that lacks flow control.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/bus-fixup.c |  4 ++--
>  drivers/misc/mei/bus.c       | 19 ++++++++++++++++---
>  drivers/misc/mei/mei_dev.h   |  7 ++++++-
>  drivers/nfc/mei_phy.c        |  7 ++++---
>  drivers/watchdog/mei_wdt.c   |  2 +-
>  include/linux/mei_cl_bus.h   |  3 ++-
>  6 files changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 7f2cef9011ae..18e05ca7584f 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device *cldev)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = __mei_cl_recv(cldev->cl, buf, length);
> +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -272,7 +272,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
>  		return -ENOMEM;
>  
>  	ret = 0;
> -	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
> +	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, 0);
>  	if (bytes_recv < if_version_length) {
>  		dev_err(bus->dev, "Could not read IF version\n");
>  		ret = -EIO;
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index 2fd254ecde2f..45e937937675 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -98,15 +98,18 @@ ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
>   * @cl: host client
>   * @buf: buffer to receive
>   * @length: buffer length
> + * @mode: io mode
>   *
>   * Return: read size in bytes of < 0 on error
>   */
> -ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
> +ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
> +		      unsigned int mode)
>  {
>  	struct mei_device *bus;
>  	struct mei_cl_cb *cb;
>  	size_t r_length;
>  	ssize_t rets;
> +	bool nonblock = !!(mode & MEI_CL_IO_RX_NONBLOCK);
>  
>  	if (WARN_ON(!cl || !cl->dev))
>  		return -ENODEV;
> @@ -127,6 +130,11 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
>  	if (rets && rets != -EBUSY)
>  		goto out;
>  
> +	if (nonblock) {
> +		rets = -EAGAIN;
> +		goto out;
> +	}
> +
>  	/* wait on event only if there is no other waiter */
>  	/* synchronized under device mutex */
>  	if (!waitqueue_active(&cl->rx_wait)) {
> @@ -197,14 +205,19 @@ EXPORT_SYMBOL_GPL(mei_cldev_send);
>   * @cldev: me client device
>   * @buf: buffer to receive
>   * @length: buffer length
> + * @flags: read io flags [O_NONBLOCK]
>   *
>   * Return: read size in bytes of < 0 on error
>   */
> -ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
> +ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
> +		       unsigned int flags)
>  {
>  	struct mei_cl *cl = cldev->cl;
> +	unsigned int mode;
> +
> +	mode = (flags & O_NONBLOCK) ? MEI_CL_IO_RX_NONBLOCK : 0;
>  
> -	return __mei_cl_recv(cl, buf, length);
> +	return __mei_cl_recv(cl, buf, length, mode);
>  }
>  EXPORT_SYMBOL_GPL(mei_cldev_recv);
>  
> diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
> index 82e69a00b7a1..f16bd1209848 100644
> --- a/drivers/misc/mei/mei_dev.h
> +++ b/drivers/misc/mei/mei_dev.h
> @@ -115,10 +115,14 @@ enum mei_cb_file_ops {
>   *
>   * @MEI_CL_IO_TX_BLOCKING: send is blocking
>   * @MEI_CL_IO_TX_INTERNAL: internal communication between driver and FW
> + *
> + * @MEI_CL_IO_RX_NONBLOCK: recv is non-blocking
>   */
>  enum mei_cl_io_mode {
>  	MEI_CL_IO_TX_BLOCKING = BIT(0),
>  	MEI_CL_IO_TX_INTERNAL = BIT(1),
> +
> +	MEI_CL_IO_RX_NONBLOCK = BIT(2),
>  };
>  
>  /*
> @@ -318,7 +322,8 @@ void mei_cl_bus_rescan_work(struct work_struct *work);
>  void mei_cl_bus_dev_fixup(struct mei_cl_device *dev);
>  ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
>  		      unsigned int mode);
> -ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
> +ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
> +		      unsigned int mode);
>  bool mei_cl_bus_rx_event(struct mei_cl *cl);
>  bool mei_cl_bus_notify_event(struct mei_cl *cl);
>  void mei_cl_bus_remove_devices(struct mei_device *bus);
> diff --git a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c
> index 8a04c5e02999..0e8158c5f81b 100644
> --- a/drivers/nfc/mei_phy.c
> +++ b/drivers/nfc/mei_phy.c
> @@ -132,7 +132,8 @@ static int mei_nfc_if_version(struct nfc_mei_phy *phy)
>  	if (!reply)
>  		return -ENOMEM;
>  
> -	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply, if_version_length);
> +	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> +				    if_version_length, 0);
>  	if (bytes_recv < 0 || bytes_recv < if_version_length) {
>  		pr_err("Could not read IF version\n");
>  		r = -EIO;
> @@ -193,7 +194,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
>  	}
>  
>  	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> -				    connect_resp_length);
> +				    connect_resp_length, 0);
>  	if (bytes_recv < 0) {
>  		r = bytes_recv;
>  		pr_err("Could not read connect response %d\n", r);
> @@ -279,7 +280,7 @@ static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
>  	struct mei_nfc_hdr *hdr;
>  	int received_length;
>  
> -	received_length = mei_cldev_recv(phy->cldev, buf, length);
> +	received_length = mei_cldev_recv(phy->cldev, buf, length, 0);
>  	if (received_length < 0)
>  		return received_length;
>  
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> index b29c6fde7473..3e29af534f8e 100644
> --- a/drivers/watchdog/mei_wdt.c
> +++ b/drivers/watchdog/mei_wdt.c
> @@ -423,7 +423,7 @@ static void mei_wdt_rx(struct mei_cl_device *cldev)
>  	const size_t res_len = sizeof(res);
>  	int ret;
>  
> -	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len);
> +	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len, 0);
>  	if (ret < 0) {
>  		dev_err(&cldev->dev, "failure in recv %d\n", ret);
>  		return;
> diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
> index 017f5232b3de..c3bb565f0745 100644
> --- a/include/linux/mei_cl_bus.h
> +++ b/include/linux/mei_cl_bus.h
> @@ -86,7 +86,8 @@ void mei_cldev_driver_unregister(struct mei_cl_driver *cldrv);
>  		      mei_cldev_driver_unregister)
>  
>  ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t length);
> -ssize_t  mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length);
> +ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
> +		       unsigned int flags);

Functions like this are horrid!  You have to always go around and dig up
what "flags" means, and if 1 means this or 0 means that or whatever.

It makes it so you can not read the code that calls this function at all
and know what is going on.

Just make a new function mei_cldev_recv_async() and then call a local,
static function, that does the work with the correct flag set.  That way
the developer always knows exactly what is going on.

thanks,

greg k-h

RE: [char-misc-next 4/4] mei: bus: enable non-blocking RX

[Winkler, Tomas]

> 
> On Wed, Nov 16, 2016 at 10:51:30PM +0200, Tomas Winkler wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Enable non-blocking receive for drivers on mei bus, this allows
> > checking for data availability by mei client drivers. This is most
> > effective for fixed address clients, that lacks flow control.
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> >  drivers/misc/mei/bus-fixup.c |  4 ++--
> >  drivers/misc/mei/bus.c       | 19 ++++++++++++++++---
> >  drivers/misc/mei/mei_dev.h   |  7 ++++++-
> >  drivers/nfc/mei_phy.c        |  7 ++++---
> >  drivers/watchdog/mei_wdt.c   |  2 +-
> >  include/linux/mei_cl_bus.h   |  3 ++-
> >  6 files changed, 31 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/misc/mei/bus-fixup.c
> > b/drivers/misc/mei/bus-fixup.c index 7f2cef9011ae..18e05ca7584f 100644
> > --- a/drivers/misc/mei/bus-fixup.c
> > +++ b/drivers/misc/mei/bus-fixup.c
> > @@ -141,7 +141,7 @@ static int mei_osver(struct mei_cl_device *cldev)
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	ret = __mei_cl_recv(cldev->cl, buf, length);
> > +	ret = __mei_cl_recv(cldev->cl, buf, length, 0);
> >  	if (ret < 0)
> >  		return ret;
> >
> > @@ -272,7 +272,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
> >  		return -ENOMEM;
> >
> >  	ret = 0;
> > -	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length);
> > +	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, 0);
> >  	if (bytes_recv < if_version_length) {
> >  		dev_err(bus->dev, "Could not read IF version\n");
> >  		ret = -EIO;
> > diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c index
> > 2fd254ecde2f..45e937937675 100644
> > --- a/drivers/misc/mei/bus.c
> > +++ b/drivers/misc/mei/bus.c
> > @@ -98,15 +98,18 @@ ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf,
> size_t length,
> >   * @cl: host client
> >   * @buf: buffer to receive
> >   * @length: buffer length
> > + * @mode: io mode
> >   *
> >   * Return: read size in bytes of < 0 on error
> >   */
> > -ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
> > +ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
> > +		      unsigned int mode)
> >  {
> >  	struct mei_device *bus;
> >  	struct mei_cl_cb *cb;
> >  	size_t r_length;
> >  	ssize_t rets;
> > +	bool nonblock = !!(mode & MEI_CL_IO_RX_NONBLOCK);
> >
> >  	if (WARN_ON(!cl || !cl->dev))
> >  		return -ENODEV;
> > @@ -127,6 +130,11 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf,
> size_t length)
> >  	if (rets && rets != -EBUSY)
> >  		goto out;
> >
> > +	if (nonblock) {
> > +		rets = -EAGAIN;
> > +		goto out;
> > +	}
> > +
> >  	/* wait on event only if there is no other waiter */
> >  	/* synchronized under device mutex */
> >  	if (!waitqueue_active(&cl->rx_wait)) { @@ -197,14 +205,19 @@
> > EXPORT_SYMBOL_GPL(mei_cldev_send);
> >   * @cldev: me client device
> >   * @buf: buffer to receive
> >   * @length: buffer length
> > + * @flags: read io flags [O_NONBLOCK]
> >   *
> >   * Return: read size in bytes of < 0 on error
> >   */
> > -ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t
> > length)
> > +ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
> > +		       unsigned int flags)
> >  {
> >  	struct mei_cl *cl = cldev->cl;
> > +	unsigned int mode;
> > +
> > +	mode = (flags & O_NONBLOCK) ? MEI_CL_IO_RX_NONBLOCK : 0;
> >
> > -	return __mei_cl_recv(cl, buf, length);
> > +	return __mei_cl_recv(cl, buf, length, mode);
> >  }
> >  EXPORT_SYMBOL_GPL(mei_cldev_recv);
> >
> > diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
> > index 82e69a00b7a1..f16bd1209848 100644
> > --- a/drivers/misc/mei/mei_dev.h
> > +++ b/drivers/misc/mei/mei_dev.h
> > @@ -115,10 +115,14 @@ enum mei_cb_file_ops {
> >   *
> >   * @MEI_CL_IO_TX_BLOCKING: send is blocking
> >   * @MEI_CL_IO_TX_INTERNAL: internal communication between driver and
> > FW
> > + *
> > + * @MEI_CL_IO_RX_NONBLOCK: recv is non-blocking
> >   */
> >  enum mei_cl_io_mode {
> >  	MEI_CL_IO_TX_BLOCKING = BIT(0),
> >  	MEI_CL_IO_TX_INTERNAL = BIT(1),
> > +
> > +	MEI_CL_IO_RX_NONBLOCK = BIT(2),
> >  };
> >
> >  /*
> > @@ -318,7 +322,8 @@ void mei_cl_bus_rescan_work(struct work_struct
> > *work);  void mei_cl_bus_dev_fixup(struct mei_cl_device *dev);
> > ssize_t __mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
> >  		      unsigned int mode);
> > -ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length);
> > +ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length,
> > +		      unsigned int mode);
> >  bool mei_cl_bus_rx_event(struct mei_cl *cl);  bool
> > mei_cl_bus_notify_event(struct mei_cl *cl);  void
> > mei_cl_bus_remove_devices(struct mei_device *bus); diff --git
> > a/drivers/nfc/mei_phy.c b/drivers/nfc/mei_phy.c index
> > 8a04c5e02999..0e8158c5f81b 100644
> > --- a/drivers/nfc/mei_phy.c
> > +++ b/drivers/nfc/mei_phy.c
> > @@ -132,7 +132,8 @@ static int mei_nfc_if_version(struct nfc_mei_phy
> *phy)
> >  	if (!reply)
> >  		return -ENOMEM;
> >
> > -	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> if_version_length);
> > +	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> > +				    if_version_length, 0);
> >  	if (bytes_recv < 0 || bytes_recv < if_version_length) {
> >  		pr_err("Could not read IF version\n");
> >  		r = -EIO;
> > @@ -193,7 +194,7 @@ static int mei_nfc_connect(struct nfc_mei_phy *phy)
> >  	}
> >
> >  	bytes_recv = mei_cldev_recv(phy->cldev, (u8 *)reply,
> > -				    connect_resp_length);
> > +				    connect_resp_length, 0);
> >  	if (bytes_recv < 0) {
> >  		r = bytes_recv;
> >  		pr_err("Could not read connect response %d\n", r); @@ -279,7
> +280,7
> > @@ static int mei_nfc_recv(struct nfc_mei_phy *phy, u8 *buf, size_t length)
> >  	struct mei_nfc_hdr *hdr;
> >  	int received_length;
> >
> > -	received_length = mei_cldev_recv(phy->cldev, buf, length);
> > +	received_length = mei_cldev_recv(phy->cldev, buf, length, 0);
> >  	if (received_length < 0)
> >  		return received_length;
> >
> > diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> > index b29c6fde7473..3e29af534f8e 100644
> > --- a/drivers/watchdog/mei_wdt.c
> > +++ b/drivers/watchdog/mei_wdt.c
> > @@ -423,7 +423,7 @@ static void mei_wdt_rx(struct mei_cl_device *cldev)
> >  	const size_t res_len = sizeof(res);
> >  	int ret;
> >
> > -	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len);
> > +	ret = mei_cldev_recv(wdt->cldev, (u8 *)&res, res_len, 0);
> >  	if (ret < 0) {
> >  		dev_err(&cldev->dev, "failure in recv %d\n", ret);
> >  		return;
> > diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
> > index 017f5232b3de..c3bb565f0745 100644
> > --- a/include/linux/mei_cl_bus.h
> > +++ b/include/linux/mei_cl_bus.h
> > @@ -86,7 +86,8 @@ void mei_cldev_driver_unregister(struct mei_cl_driver
> *cldrv);
> >  		      mei_cldev_driver_unregister)
> >
> >  ssize_t mei_cldev_send(struct mei_cl_device *cldev, u8 *buf, size_t
> > length); -ssize_t  mei_cldev_recv(struct mei_cl_device *cldev, u8
> > *buf, size_t length);
> > +ssize_t mei_cldev_recv(struct mei_cl_device *cldev, u8 *buf, size_t length,
> > +		       unsigned int flags);
> 
> Functions like this are horrid!  You have to always go around and dig up what
> "flags" means, and if 1 means this or 0 means that or whatever.

I agree the flags are always mystery but In this case we tried to follow known API, basically it just pushes O_NOBLOCK. 
Second we made it similar to its send counterpart which takes few flags

> 
> It makes it so you can not read the code that calls this function at all and know
> what is going on.

True, but consistency in API  and relatively compact API are also in trade off in usefulness of an API.
 

> 
> Just make a new function mei_cldev_recv_async() and then call a local, static
> function, that does the work with the correct flag set.  That way the developer
> always knows exactly what is going on.

We can do a wrapper, but _async() is not proper here maybe _nonblock(), 

Thanks
Tomas

Re: [char-misc-next 4/4] mei: bus: enable non-blocking RX

[Greg Kroah-Hartman]

On Thu, Nov 17, 2016 at 04:22:24PM +0000, Winkler, Tomas wrote:
> > Just make a new function mei_cldev_recv_async() and then call a local, static
> > function, that does the work with the correct flag set.  That way the developer
> > always knows exactly what is going on.
> 
> We can do a wrapper, but _async() is not proper here maybe _nonblock(),

Yes, I just guessed at the name :)

RE: [char-misc-next 4/4] mei: bus: enable non-blocking RX

[Winkler, Tomas]

> 
> On Thu, Nov 17, 2016 at 04:22:24PM +0000, Winkler, Tomas wrote:
> > > Just make a new function mei_cldev_recv_async() and then call a
> > > local, static function, that does the work with the correct flag
> > > set.  That way the developer always knows exactly what is going on.
> >
> > We can do a wrapper, but _async() is not proper here maybe
> > _nonblock(),
> 
> Yes, I just guessed at the name :)

Understood, any how I believe that we should keep patch as is, there is no one
API _nonblock() function in the whole kernel unlike _async(), 
nonblock  is always passed in as a flag or Boolean. 

Tomas

Re: [char-misc-next 4/4] mei: bus: enable non-blocking RX

[Greg Kroah-Hartman]

On Fri, Nov 18, 2016 at 07:30:25PM +0000, Winkler, Tomas wrote:
> 
> > 
> > On Thu, Nov 17, 2016 at 04:22:24PM +0000, Winkler, Tomas wrote:
> > > > Just make a new function mei_cldev_recv_async() and then call a
> > > > local, static function, that does the work with the correct flag
> > > > set.  That way the developer always knows exactly what is going on.
> > >
> > > We can do a wrapper, but _async() is not proper here maybe
> > > _nonblock(),
> > 
> > Yes, I just guessed at the name :)
> 
> Understood, any how I believe that we should keep patch as is, there is no one
> API _nonblock() function in the whole kernel unlike _async(), 
> nonblock  is always passed in as a flag or Boolean. 

And I'll still argue that this is a horrible api and you can do better.
No need to duplicate the errors of our childhood :)

thanks,

greg k-h

RE: [char-misc-next 4/4] mei: bus: enable non-blocking RX

[Winkler, Tomas]

> 
> On Fri, Nov 18, 2016 at 07:30:25PM +0000, Winkler, Tomas wrote:
> >
> > >
> > > On Thu, Nov 17, 2016 at 04:22:24PM +0000, Winkler, Tomas wrote:
> > > > > Just make a new function mei_cldev_recv_async() and then call a
> > > > > local, static function, that does the work with the correct flag
> > > > > set.  That way the developer always knows exactly what is going on.
> > > >
> > > > We can do a wrapper, but _async() is not proper here maybe
> > > > _nonblock(),
> > >
> > > Yes, I just guessed at the name :)
> >
> > Understood, any how I believe that we should keep patch as is, there
> > is no one API _nonblock() function in the whole kernel unlike
> > _async(), nonblock  is always passed in as a flag or Boolean.
> 
> And I'll still argue that this is a horrible api and you can do better.
> No need to duplicate the errors of our childhood :)

Legacy is not always wrong it as is consistency, and the balance where to put change of behavior in arguments or function names is gentle one.
I'm not sure we want to end with Java function names, either.
bool doesShorterNameExistThatEquallyConvaysTheBehaviorOfTheMethod()

I'll respin the patch as it's not really worth the argument in this particular case,  but  I'm not convinced.

Thanks
Tomas



> 
> thanks,
> 
> greg k-h