linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [char-misc-next 00/11]  Add MEI BUS and NFC Device
@ 2013-02-07 21:03 Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation Tomas Winkler
                   ` (10 more replies)
  0 siblings, 11 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

This patch set adds implementation of MEI BUS abstraction
over MEI device, this allows standard Linux device drivers  
to access functionality exposed by MEI device that was previously
available only to the user space through /dev/mei
 
The first exercises is to export the NFC radio

More information can be found under
Documentation/misc-devices/mei/mei-bus.txt


Samuel Ortiz (11):
  mei: bus: Initial MEI bus type implementation
  mei: bus: Implement driver registration
  mei: bus: Initial implementation for I/O routines
  mei: bus: Add bus related structures to mei_cl
  mei: bus: Call bus routines from the core code
  mei: bus: Synchronous API for the data transmission
  mei: bus: Implement bus driver data setter/getter
  mei: nfc: Initial nfc implementation
  mei: nfc: Connect also the regular ME client
  mei: nfc: Add NFC device to the MEI bus
  mei: nfc: Implement MEI bus IO ops

 Documentation/misc-devices/mei/mei-bus.txt |  137 ++++++++
 drivers/misc/mei/Kconfig                   |    7 +
 drivers/misc/mei/Makefile                  |    2 +
 drivers/misc/mei/bus.c                     |  483 ++++++++++++++++++++++++++++
 drivers/misc/mei/bus.h                     |   32 ++
 drivers/misc/mei/client.c                  |    4 +
 drivers/misc/mei/init.c                    |    1 +
 drivers/misc/mei/interrupt.c               |    2 +
 drivers/misc/mei/mei_dev.h                 |   78 +++++
 drivers/misc/mei/nfc.c                     |  460 ++++++++++++++++++++++++++
 drivers/misc/mei/nfc.h                     |  141 ++++++++
 drivers/misc/mei/pci-me.c                  |    8 +
 include/linux/mei_bus.h                    |  108 ++++++
 13 files changed, 1463 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/misc-devices/mei/mei-bus.txt
 create mode 100644 drivers/misc/mei/bus.c
 create mode 100644 drivers/misc/mei/bus.h
 create mode 100644 drivers/misc/mei/nfc.c
 create mode 100644 drivers/misc/mei/nfc.h
 create mode 100644 include/linux/mei_bus.h

-- 
1.7.4.4


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

* [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 22:29   ` Arnd Bergmann
  2013-02-07 22:41   ` Arnd Bergmann
  2013-02-07 21:03 ` [char-misc-next 02/11] mei: bus: Implement driver registration Tomas Winkler
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

mei bus will present some of the me clients
as devices for other standard subsystems

Implement the probe, remove, match and the device addtion routines.
A mei-bus.txt document describing the rationale and the API usage
is also added.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 Documentation/misc-devices/mei/mei-bus.txt |  137 ++++++++++++++++++++++++
 drivers/misc/mei/Makefile                  |    1 +
 drivers/misc/mei/bus.c                     |  155 ++++++++++++++++++++++++++++
 drivers/misc/mei/bus.h                     |   27 +++++
 drivers/misc/mei/mei_dev.h                 |   24 +++++
 include/linux/mei_bus.h                    |   91 ++++++++++++++++
 6 files changed, 435 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/misc-devices/mei/mei-bus.txt
 create mode 100644 drivers/misc/mei/bus.c
 create mode 100644 drivers/misc/mei/bus.h
 create mode 100644 include/linux/mei_bus.h

diff --git a/Documentation/misc-devices/mei/mei-bus.txt b/Documentation/misc-devices/mei/mei-bus.txt
new file mode 100644
index 0000000..dac6239
--- /dev/null
+++ b/Documentation/misc-devices/mei/mei-bus.txt
@@ -0,0 +1,137 @@
+Intel(R) Management Engine (ME) bus API
+=======================================
+
+
+Rationale
+=========
+While a misc character device is useful for applications to send and receive
+data to the many IP blocks found in Intel's ME, kernel drivers rely on the
+device model to be probed.
+By adding a kernel virtual bus abstraction on top of the MEI driver we can
+implement drivers for the various MEI features as standalone ones, found in
+their respective subsystem. Existing drivers can even potentially be re-used
+by adding an MEI bus layer to the existing code.
+
+
+MEI bus API
+===========
+A driver implementation for an MEI IP block is very similar to existing bus
+based device drivers. The driver registers itself as an MEI bus driver through
+the mei_bus_driver structure:
+
+struct mei_bus_driver {
+	struct device_driver driver;
+
+	struct mei_id id;
+
+	int (*probe)(struct mei_bus_client *client);
+	int (*remove)(struct mei_bus_client *client);
+};
+
+struct mei_id {
+	char name[MEI_NAME_SIZE];
+	uuid_le uuid;
+};
+
+The mei_id structure allows the driver to bind itself against an ME UUID and a
+device name. There typically is one ME UUID per technology and the mei_id name
+field matches a specific device name within that technology. As an example,
+the ME supports several NFC devices: All of them have the same ME UUID but the
+ME bus code will assign each of them a different name.
+
+To actually register a driver on the ME bus one must call the mei_add_driver()
+API. This is typically called at module init time.
+
+Once registered on the ME bus, a driver will typically try to do some I/O on
+this bus and this should be done through the mei_bus_send() and mei_bus_recv()
+routines. The latter is synchronous (blocks and sleeps until data shows up).
+In order for drivers to be notified of pending events waiting for them (e.g.
+an Rx event) they can register an event handler through the
+mei_bus_register_event_cb() routine. Currently only the MEI_BUS_EVENT_RX event
+will trigger an event handler call and the driver implementation is supposed
+to call mei_bus_recv() from the event handler in order to fetch the pending
+received buffers.
+
+
+Example
+=======
+As a theoretical example let's pretend the ME comes with a "contact" NFC IP.
+The driver init and exit routines for this device would look like:
+
+#define CONTACT_DRIVER_NAME "contact"
+
+#define NFC_UUID UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, 0x94, \
+			       0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
+
+static struct mei_bus_driver contact_driver = {
+	.driver = {
+		   .name = CONTAC_DRIVER_NAME,
+		  },
+	.id = {
+		.name = CONTACT_DRIVER_NAME,
+		.uuid = NFC_UUID,
+	},
+
+	.probe = contact_probe,
+	.remove = contact_remove,
+};
+
+static int contact_init(void)
+{
+	int r;
+
+	pr_debug(DRIVER_DESC ": %s\n", __func__);
+
+	r = mei_add_driver(&contact_driver);
+	if (r) {
+		pr_err(CONTACT_DRIVER_NAME ": driver registration failed\n");
+		return r;
+	}
+
+	return 0;
+}
+
+static void __exit contact_exit(void)
+{
+	mei_del_driver(&contact_driver);
+}
+
+module_init(contact_init);
+module_exit(contact_exit);
+
+And the driver's simplified probe routine would look like that:
+
+int contact_probe(struct mei_bus_client *client)
+{
+	struct contact_driver *contact;
+
+	[...]
+	mei_bus_register_event_cb(client, contact_event_cb, contact);
+
+	return 0;
+ }
+
+In the probe routine the driver basically registers an ME bus event handler
+which is as close as it can get to registering a threaded IRQ handler.
+The handler implementation will typically call some I/O routine depending on
+the pending events:
+
+#define MAX_NFC_PAYLOAD
+
+static void contact_event_cb(struct mei_bus_client *client, u32 events,
+			     void *context)
+{
+	struct contact_driver *contact = context;
+
+	if (events & BIT(MEI_BUS_EVENT_RX)) {
+		u8 payload[MAX_NFC_PAYLOAD];
+		int payload_size;
+
+		payload_size = mei_bus_recv(client, payload, MAX_NFC_PAYLOAD);
+		if (payload_size <= 0)
+			return;
+
+		/* Hook to the NFC subsystem */
+		nfc_hci_recv_frame(contact->hdev, payload, payload_size);
+	}
+}
diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 040af6c..5948621 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -10,5 +10,6 @@ mei-objs += client.o
 mei-objs += main.o
 mei-objs += amthif.o
 mei-objs += wd.o
+mei-objs += bus.o
 mei-$(CONFIG_INTEL_MEI_ME) += pci-me.o
 mei-$(CONFIG_INTEL_MEI_ME) += hw-me.o
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
new file mode 100644
index 0000000..bb96423c
--- /dev/null
+++ b/drivers/misc/mei/bus.c
@@ -0,0 +1,155 @@
+/*
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Copyright (c) 2012, 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/module.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/pci.h>
+#include <linux/mei_bus.h>
+
+#include "mei_dev.h"
+#include "bus.h"
+
+static int mei_device_match(struct device *dev, struct device_driver *drv)
+{
+	struct mei_bus_client *client = to_mei_client(dev);
+	struct mei_bus_driver *driver;
+
+	if (!client)
+		return 0;
+
+	driver = to_mei_driver(drv);
+
+	return !uuid_le_cmp(client->uuid, driver->id.uuid) &&
+		!strcmp(client->name, driver->id.name);
+}
+
+static int mei_device_probe(struct device *dev)
+{
+	struct mei_bus_client *client = to_mei_client(dev);
+	struct mei_bus_driver *driver;
+	int status;
+
+	if (!client)
+		return 0;
+
+	driver = to_mei_driver(dev->driver);
+	if (!driver->probe)
+		return -ENODEV;
+
+	client->driver = driver;
+	dev_dbg(dev, "probe\n");
+
+	status = driver->probe(client);
+	if (status)
+		client->driver = NULL;
+
+	return status;
+}
+
+static int mei_device_remove(struct device *dev)
+{
+	struct mei_bus_client *client = to_mei_client(dev);
+	struct mei_bus_driver *driver;
+	int status;
+
+	if (!client || !dev->driver)
+		return 0;
+
+	driver = to_mei_driver(dev->driver);
+	if (!driver->remove) {
+		dev->driver = NULL;
+		client->driver = NULL;
+
+		return 0;
+	}
+
+	status = driver->remove(client);
+	if (!status)
+		client->driver = NULL;
+
+	return status;
+}
+
+static void mei_device_shutdown(struct device *dev)
+{
+	return;
+}
+
+struct bus_type mei_bus_type = {
+	.name		= "mei",
+	.match		= mei_device_match,
+	.probe		= mei_device_probe,
+	.remove		= mei_device_remove,
+	.shutdown	= mei_device_shutdown,
+};
+EXPORT_SYMBOL(mei_bus_type);
+
+static void mei_client_dev_release(struct device *dev)
+{
+	kfree(to_mei_client(dev));
+}
+
+static struct device_type mei_client_type = {
+	.release	= mei_client_dev_release,
+};
+
+struct mei_bus_client *mei_add_device(struct mei_device *mei_dev,
+				      uuid_le uuid, char *name)
+{
+	struct mei_bus_client *client;
+	int status;
+
+	client = kzalloc(sizeof(struct mei_bus_client), GFP_KERNEL);
+	if (!client)
+		return NULL;
+
+	client->mei_dev = mei_dev;
+	client->uuid = uuid;
+	strlcpy(client->name, name, sizeof(client->name));
+
+	client->dev.parent = &client->mei_dev->pdev->dev;
+	client->dev.bus = &mei_bus_type;
+	client->dev.type = &mei_client_type;
+
+	dev_set_name(&client->dev, "%s", client->name);
+
+	status = device_register(&client->dev);
+	if (status)
+		goto out_err;
+
+	dev_dbg(&client->dev, "client %s registered\n", client->name);
+
+	return client;
+
+out_err:
+	dev_err(client->dev.parent, "Failed to register MEI client\n");
+
+	kfree(client);
+
+	return NULL;
+}
+EXPORT_SYMBOL(mei_add_device);
+
+void mei_remove_device(struct mei_bus_client *client)
+{
+	device_unregister(&client->dev);
+}
+EXPORT_SYMBOL(mei_remove_device);
diff --git a/drivers/misc/mei/bus.h b/drivers/misc/mei/bus.h
new file mode 100644
index 0000000..7130b67
--- /dev/null
+++ b/drivers/misc/mei/bus.h
@@ -0,0 +1,27 @@
+/*
+ *
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Copyright (c) 2003-2012, 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.
+ *
+ */
+
+#ifndef _MEI_BUS_H_
+#define _MEI_BUS_H_
+
+#define to_mei_driver(d) container_of(d, struct mei_bus_driver, driver)
+#define to_mei_client(d) container_of(d, struct mei_bus_client, dev)
+
+struct mei_bus_client *mei_add_device(struct mei_device *mei_dev,
+					uuid_le uuid, char *name);
+void mei_remove_device(struct mei_bus_client *client);
+
+#endif /* _MEI_BUS_H_ */
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index cb80166..ce19b26 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -21,6 +21,7 @@
 #include <linux/watchdog.h>
 #include <linux/poll.h>
 #include <linux/mei.h>
+#include <linux/mei_bus.h>
 
 #include "hw.h"
 #include "hw-me-regs.h"
@@ -264,6 +265,29 @@ struct mei_hw_ops {
 };
 
 /**
+ * mei_bus_client
+ *
+ * @uuid: me client uuid
+ * @name: client symbolic name
+ * @me_dev: mei device
+ * @cl: mei client
+ * @driver: mei bus driver for this client
+ * @dev: linux driver model device pointer
+ * @priv_data: client private data
+ */
+struct mei_bus_client {
+	uuid_le uuid;
+	char name[MEI_NAME_SIZE];
+
+	struct mei_device *mei_dev;
+	struct mei_cl *cl;
+	struct mei_bus_driver *driver;
+	struct device dev;
+
+	void *priv_data;
+};
+
+/**
  * struct mei_device -  MEI private device struct
 
  * @mem_addr - mem mapped base register address
diff --git a/include/linux/mei_bus.h b/include/linux/mei_bus.h
new file mode 100644
index 0000000..3a53f9e
--- /dev/null
+++ b/include/linux/mei_bus.h
@@ -0,0 +1,91 @@
+/******************************************************************************
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Intel MEI Interface Header
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2003 - 2012 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110,
+ * USA
+ *
+ * The full GNU General Public License is included in this distribution
+ * in the file called LICENSE.GPL.
+ *
+ * Contact Information:
+ *	Intel Corporation.
+ *	linux-mei@linux.intel.com
+ *	http://www.intel.com
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2003 - 2012 Intel Corporation. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  * Neither the name Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ *****************************************************************************/
+
+#ifndef _LINUX_MEI_BUS_H
+#define _LINUX_MEI_BUS_H
+
+#include <linux/device.h>
+#include <linux/uuid.h>
+
+struct mei_bus_client;
+
+#define MEI_NAME_SIZE 32
+
+struct mei_id {
+	char name[MEI_NAME_SIZE];
+	uuid_le uuid;
+};
+
+struct mei_bus_driver {
+	struct device_driver driver;
+
+	struct mei_id id;
+
+	int (*probe)(struct mei_bus_client *client);
+	int (*remove)(struct mei_bus_client *client);
+};
+
+#endif /* _LINUX_MEI_BUS_H */
-- 
1.7.4.4


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

* [char-misc-next 02/11] mei: bus: Implement driver registration
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 22:30   ` Arnd Bergmann
  2013-02-07 21:03 ` [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines Tomas Winkler
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c  |   29 +++++++++++++++++++++++++++++
 include/linux/mei_bus.h |    3 +++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index bb96423c..ea24e7c 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -153,3 +153,32 @@ void mei_remove_device(struct mei_bus_client *client)
 	device_unregister(&client->dev);
 }
 EXPORT_SYMBOL(mei_remove_device);
+
+int mei_add_driver(struct mei_bus_driver *driver)
+{
+	int err;
+
+	/* Can't register until after driver model init */
+	if (unlikely(WARN_ON(!mei_bus_type.p)))
+		return -EAGAIN;
+
+	driver->driver.owner = THIS_MODULE;
+	driver->driver.bus = &mei_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(mei_add_driver);
+
+void mei_del_driver(struct mei_bus_driver *driver)
+{
+	driver_unregister(&driver->driver);
+
+	pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
+}
+EXPORT_SYMBOL(mei_del_driver);
diff --git a/include/linux/mei_bus.h b/include/linux/mei_bus.h
index 3a53f9e..395f573 100644
--- a/include/linux/mei_bus.h
+++ b/include/linux/mei_bus.h
@@ -88,4 +88,7 @@ struct mei_bus_driver {
 	int (*remove)(struct mei_bus_client *client);
 };
 
+int mei_add_driver(struct mei_bus_driver *driver);
+void mei_del_driver(struct mei_bus_driver *driver);
+
 #endif /* _LINUX_MEI_BUS_H */
-- 
1.7.4.4


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

* [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 02/11] mei: bus: Implement driver registration Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 22:34   ` Arnd Bergmann
  2013-02-07 21:03 ` [char-misc-next 04/11] mei: bus: Add bus related structures to mei_cl Tomas Winkler
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c     |  226 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/mei/bus.h     |    3 +
 drivers/misc/mei/mei_dev.h |   14 +++
 include/linux/mei_bus.h    |   11 ++
 4 files changed, 254 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index ea24e7c..9689b95 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
+#include <linux/sched.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
@@ -25,6 +26,8 @@
 #include <linux/mei_bus.h>
 
 #include "mei_dev.h"
+#include "hw-me.h"
+#include "client.h"
 #include "bus.h"
 
 static int mei_device_match(struct device *dev, struct device_driver *drv)
@@ -73,6 +76,11 @@ static int mei_device_remove(struct device *dev)
 	if (!client || !dev->driver)
 		return 0;
 
+	if (client->event_cb) {
+		client->event_cb = NULL;
+		cancel_work_sync(&client->event_work);
+	}
+
 	driver = to_mei_driver(dev->driver);
 	if (!driver->remove) {
 		dev->driver = NULL;
@@ -182,3 +190,221 @@ void mei_del_driver(struct mei_bus_driver *driver)
 	pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
 }
 EXPORT_SYMBOL(mei_del_driver);
+
+int mei_send(struct mei_cl *cl, u8 *buf, size_t length)
+{
+	struct mei_device *dev;
+	struct mei_msg_hdr mei_hdr;
+	struct mei_cl_cb *cb;
+	int me_cl_id, err;
+
+	if (WARN_ON(!cl || !cl->dev))
+		return -ENODEV;
+
+	if (cl->state != MEI_FILE_CONNECTED)
+		return -ENODEV;
+
+	cb = mei_io_cb_init(cl, NULL);
+	if (!cb)
+		return -ENOMEM;
+
+	err = mei_io_cb_alloc_req_buf(cb, length);
+	if (err < 0) {
+		mei_io_cb_free(cb);
+		return err;
+	}
+
+	memcpy(cb->request_buffer.data, buf, length);
+	cb->fop_type = MEI_FOP_WRITE;
+
+	dev = cl->dev;
+
+	mutex_lock(&dev->device_lock);
+
+	/* Check if we have an ME client device */
+	me_cl_id = mei_me_cl_by_id(dev, cl->me_client_id);
+	if (me_cl_id == dev->me_clients_num) {
+		err = -ENODEV;
+		goto out_err;
+	}
+
+	if (length > dev->me_clients[me_cl_id].props.max_msg_length) {
+		err = -EINVAL;
+		goto out_err;
+	}
+
+	err = mei_cl_flow_ctrl_creds(cl);
+	if (err < 0)
+		goto out_err;
+
+	/* Host buffer is not ready, we queue the request */
+	if (err == 0 || !dev->hbuf_is_ready) {
+		cb->buf_idx = 0;
+		mei_hdr.msg_complete = 0;
+		cl->writing_state = MEI_WRITING;
+		list_add_tail(&cb->list, &dev->write_list.list);
+
+		mutex_unlock(&dev->device_lock);
+
+		return length;
+	}
+
+	dev->hbuf_is_ready = false;
+
+	/* Check for a maximum length */
+	if (length > mei_hbuf_max_len(dev)) {
+		mei_hdr.length = mei_hbuf_max_len(dev);
+		mei_hdr.msg_complete = 0;
+	} else {
+		mei_hdr.length = length;
+		mei_hdr.msg_complete = 1;
+	}
+
+	mei_hdr.host_addr = cl->host_client_id;
+	mei_hdr.me_addr = cl->me_client_id;
+	mei_hdr.reserved = 0;
+
+	if (mei_write_message(dev, &mei_hdr, buf)) {
+		err = -EIO;
+		goto out_err;
+	}
+
+	cl->writing_state = MEI_WRITING;
+	cb->buf_idx = mei_hdr.length;
+
+	if (!mei_hdr.msg_complete) {
+		list_add_tail(&cb->list, &dev->write_list.list);
+	} else {
+		if (mei_cl_flow_ctrl_reduce(cl)) {
+			err = -EIO;
+			goto out_err;
+		}
+
+		list_add_tail(&cb->list, &dev->write_waiting_list.list);
+	}
+
+	mutex_unlock(&dev->device_lock);
+
+	return mei_hdr.length;
+
+out_err:
+	mutex_unlock(&dev->device_lock);
+	mei_io_cb_free(cb);
+
+	return err;
+}
+
+int mei_recv(struct mei_cl *cl, u8 *buf, size_t length)
+{
+	struct mei_device *dev;
+	struct mei_cl_cb *cb;
+	size_t r_length;
+	int err;
+
+	if (WARN_ON(!cl || !cl->dev))
+		return -ENODEV;
+
+	dev = cl->dev;
+
+	mutex_lock(&dev->device_lock);
+
+	if (!cl->read_cb) {
+		err = mei_cl_read_start(cl);
+		if (err < 0) {
+			mutex_unlock(&dev->device_lock);
+			return err;
+		}
+	}
+
+	if (cl->reading_state != MEI_READ_COMPLETE &&
+	    !waitqueue_active(&cl->rx_wait)) {
+		mutex_unlock(&dev->device_lock);
+
+		if (wait_event_interruptible(cl->rx_wait,
+				(MEI_READ_COMPLETE == cl->reading_state))) {
+			if (signal_pending(current))
+				return -EINTR;
+			return -ERESTARTSYS;
+		}
+
+		mutex_lock(&dev->device_lock);
+	}
+
+	cb = cl->read_cb;
+
+	if (cl->reading_state != MEI_READ_COMPLETE) {
+		r_length = 0;
+		goto out;
+	}
+
+	r_length = min_t(size_t, length, cb->buf_idx);
+
+	memcpy(buf, cb->response_buffer.data, r_length);
+
+	mei_io_cb_free(cb);
+	cl->reading_state = MEI_IDLE;
+	cl->read_cb = NULL;
+
+out:
+	mutex_unlock(&dev->device_lock);
+
+	return r_length;
+}
+
+int mei_bus_send(struct mei_bus_client *client, u8 *buf, size_t length)
+{
+	struct mei_cl *cl = NULL;
+
+	/* TODO: hook between mei_bus_client and mei_cl */
+
+	if (client->ops && client->ops->send)
+		return client->ops->send(client, buf, length);
+
+	return mei_send(cl, buf, length);
+}
+EXPORT_SYMBOL(mei_bus_send);
+
+int mei_bus_recv(struct mei_bus_client *client, u8 *buf, size_t length)
+{
+	struct mei_cl *cl = NULL;
+
+	/* TODO: hook between mei_bus_client and mei_cl */
+
+	if (client->ops && client->ops->recv)
+		return client->ops->recv(client, buf, length);
+
+	return mei_recv(cl, buf, length);
+}
+EXPORT_SYMBOL(mei_bus_recv);
+
+static void mei_bus_event_work(struct work_struct *work)
+{
+	struct mei_bus_client *client;
+
+	client = container_of(work, struct mei_bus_client, event_work);
+
+	if (client->event_cb)
+		client->event_cb(client, client->events, client->event_context);
+
+	client->events = 0;
+
+	/* Prepare for the next read */
+	mei_cl_read_start(client->cl);
+}
+
+int mei_bus_register_event_cb(struct mei_bus_client *client,
+			      mei_bus_event_cb_t event_cb, void *context)
+{
+	if (client->event_cb)
+		return -EALREADY;
+
+	client->events = 0;
+	client->event_cb = event_cb;
+	client->event_context = context;
+	INIT_WORK(&client->event_work, mei_bus_event_work);
+
+	mei_cl_read_start(client->cl);
+
+	return 0;
+}
+EXPORT_SYMBOL(mei_bus_register_event_cb);
diff --git a/drivers/misc/mei/bus.h b/drivers/misc/mei/bus.h
index 7130b67..81789f6 100644
--- a/drivers/misc/mei/bus.h
+++ b/drivers/misc/mei/bus.h
@@ -24,4 +24,7 @@ struct mei_bus_client *mei_add_device(struct mei_device *mei_dev,
 					uuid_le uuid, char *name);
 void mei_remove_device(struct mei_bus_client *client);
 
+int mei_send(struct mei_cl *cl, u8 *buf, size_t length);
+int mei_recv(struct mei_cl *cl, u8 *buf, size_t length);
+
 #endif /* _MEI_BUS_H_ */
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index ce19b26..948f791 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -264,6 +264,13 @@ struct mei_hw_ops {
 		     unsigned char *buf, unsigned long len);
 };
 
+struct mei_bus_client;
+
+struct mei_bus_ops {
+	int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
+	int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
+};
+
 /**
  * mei_bus_client
  *
@@ -284,6 +291,13 @@ struct mei_bus_client {
 	struct mei_bus_driver *driver;
 	struct device dev;
 
+	struct mei_bus_ops *ops;
+
+	struct work_struct event_work;
+	mei_bus_event_cb_t event_cb;
+	void *event_context;
+	unsigned long events;
+
 	void *priv_data;
 };
 
diff --git a/include/linux/mei_bus.h b/include/linux/mei_bus.h
index 395f573..241325f 100644
--- a/include/linux/mei_bus.h
+++ b/include/linux/mei_bus.h
@@ -88,7 +88,18 @@ struct mei_bus_driver {
 	int (*remove)(struct mei_bus_client *client);
 };
 
+#define MEI_BUS_EVENT_RX 0
+#define MEI_BUS_EVENT_TX 1
+
 int mei_add_driver(struct mei_bus_driver *driver);
 void mei_del_driver(struct mei_bus_driver *driver);
 
+int mei_bus_send(struct mei_bus_client *client, u8 *buf, size_t length);
+int mei_bus_recv(struct mei_bus_client *client, u8 *buf, size_t length);
+
+typedef void (*mei_bus_event_cb_t)(struct mei_bus_client *client,
+				   u32 events, void *context);
+int mei_bus_register_event_cb(struct mei_bus_client *client,
+			      mei_bus_event_cb_t read_cb, void *context);
+
 #endif /* _LINUX_MEI_BUS_H */
-- 
1.7.4.4


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

* [char-misc-next 04/11] mei: bus: Add bus related structures to mei_cl
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (2 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 05/11] mei: bus: Call bus routines from the core code Tomas Winkler
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

We keep track of all MEI bus clients through a specific linked list.
We also have a mei_bus_client instance in the mei_cl structure.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c     |   47 +++++++++++++++++++++++++++++++------------
 drivers/misc/mei/client.c  |    1 +
 drivers/misc/mei/init.c    |    1 +
 drivers/misc/mei/mei_dev.h |    8 +++++++
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 9689b95..97afec6 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -119,16 +119,37 @@ static struct device_type mei_client_type = {
 	.release	= mei_client_dev_release,
 };
 
+static struct mei_cl *mei_bus_find_mei_cl_by_uuid(struct mei_device *mei_dev,
+						uuid_le uuid)
+{
+	struct mei_cl *cl, *next;
+
+	list_for_each_entry_safe(cl, next,
+				 &mei_dev->bus_client_list, bus_client_link) {
+		if (!uuid_le_cmp(uuid, cl->bus_client_uuid))
+			return cl;
+	}
+
+	return NULL;
+}
+
 struct mei_bus_client *mei_add_device(struct mei_device *mei_dev,
 				      uuid_le uuid, char *name)
 {
 	struct mei_bus_client *client;
+	struct mei_cl *cl;
 	int status;
 
+	cl = mei_bus_find_mei_cl_by_uuid(mei_dev, uuid);
+	if (cl == NULL)
+		return NULL;
+
 	client = kzalloc(sizeof(struct mei_bus_client), GFP_KERNEL);
 	if (!client)
 		return NULL;
 
+	client->cl = cl;
+
 	client->mei_dev = mei_dev;
 	client->uuid = uuid;
 	strlcpy(client->name, name, sizeof(client->name));
@@ -140,19 +161,17 @@ struct mei_bus_client *mei_add_device(struct mei_device *mei_dev,
 	dev_set_name(&client->dev, "%s", client->name);
 
 	status = device_register(&client->dev);
-	if (status)
-		goto out_err;
+	if (status) {
+		kfree(client);
+		dev_err(client->dev.parent, "Failed to register MEI client\n");
+		return NULL;
+	}
+
+	cl->client = client;
 
 	dev_dbg(&client->dev, "client %s registered\n", client->name);
 
 	return client;
-
-out_err:
-	dev_err(client->dev.parent, "Failed to register MEI client\n");
-
-	kfree(client);
-
-	return NULL;
 }
 EXPORT_SYMBOL(mei_add_device);
 
@@ -353,9 +372,10 @@ out:
 
 int mei_bus_send(struct mei_bus_client *client, u8 *buf, size_t length)
 {
-	struct mei_cl *cl = NULL;
+	struct mei_cl *cl = client->cl;
 
-	/* TODO: hook between mei_bus_client and mei_cl */
+	if (cl == NULL)
+		return -ENODEV;
 
 	if (client->ops && client->ops->send)
 		return client->ops->send(client, buf, length);
@@ -366,9 +386,10 @@ EXPORT_SYMBOL(mei_bus_send);
 
 int mei_bus_recv(struct mei_bus_client *client, u8 *buf, size_t length)
 {
-	struct mei_cl *cl = NULL;
+	struct mei_cl *cl =  client->cl;
 
-	/* TODO: hook between mei_bus_client and mei_cl */
+	if (cl == NULL)
+		return -ENODEV;
 
 	if (client->ops && client->ops->recv)
 		return client->ops->recv(client, buf, length);
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 1569afe..5724499 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -216,6 +216,7 @@ void mei_cl_init(struct mei_cl *cl, struct mei_device *dev)
 	init_waitqueue_head(&cl->rx_wait);
 	init_waitqueue_head(&cl->tx_wait);
 	INIT_LIST_HEAD(&cl->link);
+	INIT_LIST_HEAD(&cl->bus_client_link);
 	cl->reading_state = MEI_IDLE;
 	cl->writing_state = MEI_IDLE;
 	cl->dev = dev;
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 6ec5301..b812d56 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -46,6 +46,7 @@ void mei_device_init(struct mei_device *dev)
 {
 	/* setup our list array */
 	INIT_LIST_HEAD(&dev->file_list);
+	INIT_LIST_HEAD(&dev->bus_client_list);
 	mutex_init(&dev->device_lock);
 	init_waitqueue_head(&dev->wait_recvd_msg);
 	init_waitqueue_head(&dev->wait_stop_wd);
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 948f791..d9adad6 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -210,6 +210,11 @@ struct mei_cl {
 	enum mei_file_transaction_states writing_state;
 	int sm_state;
 	struct mei_cl_cb *read_cb;
+
+	/* MEI bus data */
+	struct mei_bus_client *client;
+	struct list_head bus_client_link;
+	uuid_le bus_client_uuid;
 };
 
 /** struct mei_hw_ops
@@ -403,6 +408,9 @@ struct mei_device {
 
 	struct work_struct init_work;
 
+	/* List of bus clients */
+	struct list_head bus_client_list;
+
 	const struct mei_hw_ops *ops;
 	char hw[0] __aligned(sizeof(void *));
 };
-- 
1.7.4.4


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

* [char-misc-next 05/11] mei: bus: Call bus routines from the core code
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (3 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 04/11] mei: bus: Add bus related structures to mei_cl Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 22:37   ` Arnd Bergmann
  2013-02-07 21:03 ` [char-misc-next 06/11] mei: bus: Synchronous API for the data transmission Tomas Winkler
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

Register the MEI bus type against the kernel core bus APIs and
call the bus Rx handler from interrupt.c

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c       |   22 ++++++++++++++++++++++
 drivers/misc/mei/interrupt.c |    2 ++
 drivers/misc/mei/mei_dev.h   |    4 ++++
 drivers/misc/mei/pci-me.c    |    6 ++++++
 4 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 97afec6..e2e15d1 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -429,3 +429,25 @@ int mei_bus_register_event_cb(struct mei_bus_client *client,
 	return 0;
 }
 EXPORT_SYMBOL(mei_bus_register_event_cb);
+
+void mei_bus_rx_event(struct mei_cl *cl)
+{
+	struct mei_bus_client *client = cl->client;
+
+	if (!client || !client->event_cb)
+		return;
+
+	set_bit(MEI_BUS_EVENT_RX, &client->events);
+
+	schedule_work(&client->event_work);
+}
+
+int mei_bus_init(struct pci_dev *pdev)
+{
+	return bus_register(&mei_bus_type);
+}
+
+void mei_bus_exit(void)
+{
+	bus_unregister(&mei_bus_type);
+}
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 3535b26..d0dea50 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -49,6 +49,8 @@ void mei_irq_complete_handler(struct mei_cl *cl, struct mei_cl_cb *cb_pos)
 		cl->reading_state = MEI_READ_COMPLETE;
 		if (waitqueue_active(&cl->rx_wait))
 			wake_up_interruptible(&cl->rx_wait);
+		else
+			mei_bus_rx_event(cl);
 
 	}
 }
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index d9adad6..cb1bac0 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -420,6 +420,10 @@ static inline unsigned long mei_secs_to_jiffies(unsigned long sec)
 	return msecs_to_jiffies(sec * MSEC_PER_SEC);
 }
 
+void mei_bus_rx_event(struct mei_cl *cl);
+int mei_bus_init(struct pci_dev *pdev);
+void mei_bus_exit(void);
+
 
 /*
  * mei init function prototypes
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index b40ec06..f736531 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -197,6 +197,9 @@ static int mei_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	mei_pdev = pdev;
 	pci_set_drvdata(pdev, dev);
 
+	err = mei_bus_init(mei_pdev);
+	if (err)
+		goto deregister_mei;
 
 	schedule_delayed_work(&dev->timer_work, HZ);
 
@@ -206,6 +209,8 @@ static int mei_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 
+deregister_mei:
+	mei_deregister();
 release_irq:
 	mei_disable_interrupts(dev);
 	flush_scheduled_work();
@@ -300,6 +305,7 @@ static void mei_remove(struct pci_dev *pdev)
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 
+	mei_bus_exit();
 	mei_deregister();
 
 }
-- 
1.7.4.4


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

* [char-misc-next 06/11] mei: bus: Synchronous API for the data transmission
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (4 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 05/11] mei: bus: Call bus routines from the core code Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter Tomas Winkler
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

Define a truly synchronous API for the bus Tx path by putting all pending
request to the write list and wait for the interrupt tx handler to wake
us up.
The __mei_send() out path is also slightly reworked to make it look more
like main.c:mei_write().

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c |   38 ++++++++++++++++++++++++++++----------
 drivers/misc/mei/bus.h |    2 ++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index e2e15d1..3f10d51 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -210,7 +210,7 @@ void mei_del_driver(struct mei_bus_driver *driver)
 }
 EXPORT_SYMBOL(mei_del_driver);
 
-int mei_send(struct mei_cl *cl, u8 *buf, size_t length)
+static int __mei_send(struct mei_cl *cl, u8 *buf, size_t length, bool blocking)
 {
 	struct mei_device *dev;
 	struct mei_msg_hdr mei_hdr;
@@ -261,11 +261,8 @@ int mei_send(struct mei_cl *cl, u8 *buf, size_t length)
 		cb->buf_idx = 0;
 		mei_hdr.msg_complete = 0;
 		cl->writing_state = MEI_WRITING;
-		list_add_tail(&cb->list, &dev->write_list.list);
-
-		mutex_unlock(&dev->device_lock);
 
-		return length;
+		goto out;
 	}
 
 	dev->hbuf_is_ready = false;
@@ -291,19 +288,30 @@ int mei_send(struct mei_cl *cl, u8 *buf, size_t length)
 	cl->writing_state = MEI_WRITING;
 	cb->buf_idx = mei_hdr.length;
 
-	if (!mei_hdr.msg_complete) {
-		list_add_tail(&cb->list, &dev->write_list.list);
-	} else {
+out:
+	if (mei_hdr.msg_complete) {
 		if (mei_cl_flow_ctrl_reduce(cl)) {
-			err = -EIO;
+			err = -ENODEV;
 			goto out_err;
 		}
-
 		list_add_tail(&cb->list, &dev->write_waiting_list.list);
+	} else {
+		list_add_tail(&cb->list, &dev->write_list.list);
 	}
 
 	mutex_unlock(&dev->device_lock);
 
+	if (blocking && cl->writing_state != MEI_WRITE_COMPLETE) {
+		if (wait_event_interruptible(cl->tx_wait,
+			cl->writing_state == MEI_WRITE_COMPLETE)) {
+				if (signal_pending(current))
+					err = -EINTR;
+			err = -ERESTARTSYS;
+			mutex_lock(&dev->device_lock);
+			goto out_err;
+		}
+	}
+
 	return mei_hdr.length;
 
 out_err:
@@ -370,6 +378,16 @@ out:
 	return r_length;
 }
 
+inline int mei_async_send(struct mei_cl *cl, u8 *buf, size_t length)
+{
+	return __mei_send(cl, buf, length, 0);
+}
+
+inline int mei_send(struct mei_cl *cl, u8 *buf, size_t length)
+{
+	return __mei_send(cl, buf, length, 1);
+}
+
 int mei_bus_send(struct mei_bus_client *client, u8 *buf, size_t length)
 {
 	struct mei_cl *cl = client->cl;
diff --git a/drivers/misc/mei/bus.h b/drivers/misc/mei/bus.h
index 81789f6..d7bf3b5 100644
--- a/drivers/misc/mei/bus.h
+++ b/drivers/misc/mei/bus.h
@@ -24,7 +24,9 @@ struct mei_bus_client *mei_add_device(struct mei_device *mei_dev,
 					uuid_le uuid, char *name);
 void mei_remove_device(struct mei_bus_client *client);
 
+int mei_async_send(struct mei_cl *cl, u8 *buf, size_t length);
 int mei_send(struct mei_cl *cl, u8 *buf, size_t length);
 int mei_recv(struct mei_cl *cl, u8 *buf, size_t length);
 
+
 #endif /* _MEI_BUS_H_ */
-- 
1.7.4.4


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

* [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (5 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 06/11] mei: bus: Synchronous API for the data transmission Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 22:38   ` Arnd Bergmann
  2013-02-07 21:03 ` [char-misc-next 08/11] mei: nfc: Initial nfc implementation Tomas Winkler
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

MEI bus drivers should be able to carry their private data around.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/bus.c  |   12 ++++++++++++
 include/linux/mei_bus.h |    3 +++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 3f10d51..58032cf 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -448,6 +448,18 @@ int mei_bus_register_event_cb(struct mei_bus_client *client,
 }
 EXPORT_SYMBOL(mei_bus_register_event_cb);
 
+inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
+{
+	return dev_get_drvdata(&client->dev);
+}
+EXPORT_SYMBOL(mei_bus_get_clientdata);
+
+inline void mei_bus_set_clientdata(struct mei_bus_client *client, void *data)
+{
+	dev_set_drvdata(&client->dev, data);
+}
+EXPORT_SYMBOL(mei_bus_set_clientdata);
+
 void mei_bus_rx_event(struct mei_cl *cl)
 {
 	struct mei_bus_client *client = cl->client;
diff --git a/include/linux/mei_bus.h b/include/linux/mei_bus.h
index 241325f..0e8e4e9 100644
--- a/include/linux/mei_bus.h
+++ b/include/linux/mei_bus.h
@@ -102,4 +102,7 @@ typedef void (*mei_bus_event_cb_t)(struct mei_bus_client *client,
 int mei_bus_register_event_cb(struct mei_bus_client *client,
 			      mei_bus_event_cb_t read_cb, void *context);
 
+void *mei_bus_get_clientdata(const struct mei_bus_client *client);
+void mei_bus_set_clientdata(struct mei_bus_client *client, void *data);
+
 #endif /* _LINUX_MEI_BUS_H */
-- 
1.7.4.4


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

* [char-misc-next 08/11] mei: nfc: Initial nfc implementation
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (6 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 22:26   ` Arnd Bergmann
  2013-02-07 21:03 ` [char-misc-next 09/11] mei: nfc: Connect also the regular ME client Tomas Winkler
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

NFC ME client is exported through mei bus to be consumed by the
NFC subsystem.

NFC is represented by two mei clients: An info one and the actual
NFC one. In order for correct connection we first need to retrieve the
firmware information from the info client.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/Kconfig   |    7 ++
 drivers/misc/mei/Makefile  |    1 +
 drivers/misc/mei/client.c  |    3 +
 drivers/misc/mei/mei_dev.h |   28 ++++++
 drivers/misc/mei/nfc.c     |  210 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/mei/nfc.h     |  122 +++++++++++++++++++++++++
 drivers/misc/mei/pci-me.c  |    2 +
 7 files changed, 373 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/mei/nfc.c
 create mode 100644 drivers/misc/mei/nfc.h

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index d21b4d0..66e84ef 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -35,3 +35,10 @@ config INTEL_MEI_ME
 	  82Q33 Express
 	  82X38/X48 Express
 
+config INTEL_MEI_BUS_NFC
+        bool "MEI bus NFC support"
+        depends on INTEL_MEI
+	help
+	  When selecting this option the ME NFC device will be added to the
+	  MEI bus. This is needed by the NFC kernel subsystem for sending and
+	  receiving HCI frames to and from the NFC device.
diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 5948621..644f92e 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -11,5 +11,6 @@ mei-objs += main.o
 mei-objs += amthif.o
 mei-objs += wd.o
 mei-objs += bus.o
+mei-$(CONFIG_INTEL_MEI_BUS_NFC) += nfc.o
 mei-$(CONFIG_INTEL_MEI_ME) += pci-me.o
 mei-$(CONFIG_INTEL_MEI_ME) += hw-me.o
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 5724499..e3ed1d4 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -358,6 +358,9 @@ void mei_host_client_init(struct work_struct *work)
 			mei_amthif_host_init(dev);
 		else if (!uuid_le_cmp(client_props->protocol_name, mei_wd_guid))
 			mei_wd_host_init(dev);
+		else if (!uuid_le_cmp(client_props->protocol_name, mei_nfc_guid))
+			mei_nfc_host_init(dev);
+
 	}
 
 	dev->dev_state = MEI_DEV_ENABLED;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index cb1bac0..da0ae42 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -475,6 +475,34 @@ int mei_amthif_irq_read_message(struct mei_cl_cb *complete_list,
 		struct mei_device *dev, struct mei_msg_hdr *mei_hdr);
 int mei_amthif_irq_read(struct mei_device *dev, s32 *slots);
 
+#ifdef CONFIG_INTEL_MEI_BUS_NFC
+/*
+ * NFC functions
+ */
+int mei_nfc_host_init(struct mei_device *dev);
+void mei_nfc_host_exit(void);
+
+/*
+ * NFC Client UUID
+ */
+extern const uuid_le mei_nfc_guid;
+
+#else
+
+static inline int mei_nfc_host_init(struct mei_device *dev)
+{
+	return 0;
+}
+
+static inline void mei_nfc_host_exit(void)
+{
+	return;
+}
+
+static const uuid_le mei_nfc_guid = UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50,
+					    0x94, 0xd4, 0x50, 0x26,
+					    0x67, 0x23, 0x77, 0x5c);
+#endif
 
 int mei_wd_send(struct mei_device *dev);
 int mei_wd_stop(struct mei_device *dev);
diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
new file mode 100644
index 0000000..240dd2b
--- /dev/null
+++ b/drivers/misc/mei/nfc.c
@@ -0,0 +1,210 @@
+/*
+ *
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Copyright (c) 2003-2012, 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/module.h>
+#include <linux/moduleparam.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+#include <linux/mei.h>
+#include <linux/mei_bus.h>
+
+#include "mei_dev.h"
+#include "client.h"
+#include "bus.h"
+#include "nfc.h"
+
+/** mei_nfc_bdev - nfc mei bus device
+ *
+ * @cl_info: nfc info host client
+ * @init_work: perform connection to the info client
+ * @fw_ivn: NFC Intervace Version Number
+ * @vendor_id: NFC manufacturer ID
+ * @radio_type: NFC radio type
+ */
+struct mei_bus_dev_nfc {
+	struct mei_cl *cl_info;
+	struct work_struct init_work;
+	u8 fw_ivn;
+	u8 vendor_id;
+	u8 radio_type;
+};
+
+struct mei_bus_dev_nfc nfc_bdev;
+
+/* 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);
+
+const uuid_le mei_nfc_info_guid = UUID_LE(0xd2de1625, 0x382d, 0x417d,
+					0x48, 0xa4, 0xef, 0xab,
+					0xba, 0x8a, 0x12, 0x06);
+
+static void mei_nfc_free(struct mei_bus_dev_nfc *bdev)
+{
+	if (bdev->cl_info) {
+		list_del(&bdev->cl_info->bus_client_link);
+		mei_cl_unlink(bdev->cl_info);
+		kfree(bdev->cl_info);
+	}
+}
+
+static int mei_nfc_if_version(struct mei_bus_dev_nfc *bdev)
+{
+	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 = bdev->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_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd));
+	if (ret < 0) {
+		dev_err(&dev->pdev->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_recv(cl, (u8 *)reply, if_version_length);
+	if (bytes_recv < 0 || bytes_recv < sizeof(struct mei_nfc_reply)) {
+		dev_err(&dev->pdev->dev, "Could not read IF version\n");
+		ret = -EIO;
+		goto err;
+	}
+
+	version = (struct mei_nfc_if_version *)reply->data;
+
+	bdev->fw_ivn = version->fw_ivn;
+	bdev->vendor_id = version->vendor_id;
+	bdev->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_bus_dev_nfc *bdev;
+	struct mei_cl *cl_info;
+	int ret;
+
+	bdev = container_of(work, struct mei_bus_dev_nfc, init_work);
+
+	cl_info = bdev->cl_info;
+	dev = cl_info->dev;
+
+	mutex_lock(&dev->device_lock);
+
+	if (mei_cl_connect(cl_info, NULL) < 0) {
+		mutex_unlock(&dev->device_lock);
+		dev_err(&dev->pdev->dev,
+			"Could not connect to the NFC INFO ME client");
+
+		goto err;
+	}
+
+	mutex_unlock(&dev->device_lock);
+
+	ret = mei_nfc_if_version(bdev);
+	if (ret < 0) {
+		dev_err(&dev->pdev->dev, "Could not get the NFC interfave version");
+
+		goto err;
+	}
+
+	dev_info(&dev->pdev->dev,
+		"NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
+		bdev->fw_ivn, bdev->vendor_id, bdev->radio_type);
+
+	return;
+
+err:
+	mei_nfc_free(bdev);
+
+	return;
+}
+
+
+int mei_nfc_host_init(struct mei_device *dev)
+{
+	struct mei_bus_dev_nfc *bdev = &nfc_bdev;
+	struct mei_cl *cl_info;
+	int i, ret;
+
+	/* already initialzed */
+	if (bdev->cl_info)
+		return 0;
+
+	cl_info = mei_cl_allocate(dev);
+	if (!cl_info)
+		return -ENOMEM;
+
+	/* check for valid client id */
+	i = mei_me_cl_by_uuid(dev, &mei_nfc_info_guid);
+	if (i < 0) {
+		dev_info(&dev->pdev->dev, "nfc: failed to find the client\n");
+		return -ENOENT;
+	}
+
+	cl_info->me_client_id = dev->me_clients[i].client_id;
+
+	ret = mei_cl_link(cl_info, MEI_HOST_CLIENT_ID_ANY);
+	if (ret)
+		goto err;
+
+	cl_info->bus_client_uuid = mei_nfc_info_guid;
+
+	list_add_tail(&cl_info->bus_client_link, &dev->bus_client_list);
+
+	bdev->cl_info = cl_info;
+
+	INIT_WORK(&bdev->init_work, mei_nfc_init);
+	schedule_work(&bdev->init_work);
+
+	return 0;
+
+err:
+	mei_nfc_free(bdev);
+
+	return ret;
+}
+
+void mei_nfc_host_exit(void)
+{
+	struct mei_bus_dev_nfc *bdev = &nfc_bdev;
+
+	mei_nfc_free(bdev);
+}
diff --git a/drivers/misc/mei/nfc.h b/drivers/misc/mei/nfc.h
new file mode 100644
index 0000000..59e6703
--- /dev/null
+++ b/drivers/misc/mei/nfc.h
@@ -0,0 +1,122 @@
+/******************************************************************************
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Intel MEI Interface Header
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2003 - 2012 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110,
+ * USA
+ *
+ * The full GNU General Public License is included in this distribution
+ * in the file called LICENSE.GPL.
+ *
+ * Contact Information:
+ *	Intel Corporation.
+ *	linux-mei@linux.intel.com
+ *	http://www.intel.com
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2003 - 2012 Intel Corporation. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  * Neither the name Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ *****************************************************************************/
+
+#ifndef _MEI_NFC_H
+#define _MEI_NFC_H
+
+#include <linux/types.h>
+
+struct mei_nfc_cmd {
+	uint8_t command;
+	uint8_t status;
+	uint16_t req_id;
+	uint32_t reserved;
+	uint16_t data_size;
+	uint8_t sub_command;
+	uint8_t data[];
+} __packed;
+
+struct mei_nfc_reply {
+	uint8_t command;
+	uint8_t status;
+	uint16_t req_id;
+	uint32_t reserved;
+	uint16_t data_size;
+	uint8_t sub_command;
+	uint8_t reply_status;
+	uint8_t data[];
+} __packed;
+
+struct mei_nfc_if_version {
+	uint8_t radio_version_sw[3];
+	uint8_t reserved[3];
+	uint8_t radio_version_hw[3];
+	uint8_t i2c_addr;
+	uint8_t fw_ivn;
+	uint8_t vendor_id;
+	uint8_t radio_type;
+} __packed;
+
+struct mei_nfc_connect {
+	uint8_t fw_ivn;
+	uint8_t vendor_id;
+} __packed;
+
+struct mei_nfc_connect_resp {
+	uint8_t fw_ivn;
+	uint8_t vendor_id;
+	uint16_t me_major;
+	uint16_t me_minor;
+	uint16_t me_hotfix;
+	uint16_t me_build;
+} __packed;
+
+#define MEI_NFC_CMD_MAINTENANCE 0x00
+
+#define MEI_NFC_SUBCMD_CONNECT    0x00
+#define MEI_NFC_SUBCMD_IF_VERSION 0x01
+
+#endif /* _MEI_NFC_H */
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index f736531..14730a1 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -283,6 +283,8 @@ static void mei_remove(struct pci_dev *pdev)
 		dev->open_handle_count--;
 	mei_cl_unlink(&dev->iamthif_cl);
 
+	mei_nfc_host_exit();
+
 	dev->iamthif_current_cb = NULL;
 	dev->me_clients_num = 0;
 
-- 
1.7.4.4


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

* [char-misc-next 09/11] mei: nfc: Connect also the regular ME client
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (7 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 08/11] mei: nfc: Initial nfc implementation Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops Tomas Winkler
  10 siblings, 0 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

After receiving the NFC interface version, IVN and radio type,
we can connect to the the actual nfc me client and send the
initialization (nfc connect) message.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/nfc.c |  128 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index 240dd2b..ab66944 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -29,6 +29,7 @@
 
 /** mei_nfc_bdev - nfc mei bus device
  *
+ * @cl: nfc info host client
  * @cl_info: nfc info host client
  * @init_work: perform connection to the info client
  * @fw_ivn: NFC Intervace Version Number
@@ -36,6 +37,7 @@
  * @radio_type: NFC radio type
  */
 struct mei_bus_dev_nfc {
+	struct mei_cl *cl;
 	struct mei_cl *cl_info;
 	struct work_struct init_work;
 	u8 fw_ivn;
@@ -56,6 +58,12 @@ const uuid_le mei_nfc_info_guid = UUID_LE(0xd2de1625, 0x382d, 0x417d,
 
 static void mei_nfc_free(struct mei_bus_dev_nfc *bdev)
 {
+	if (bdev->cl) {
+		list_del(&bdev->cl->bus_client_link);
+		mei_cl_unlink(bdev->cl);
+		kfree(bdev->cl);
+	}
+
 	if (bdev->cl_info) {
 		list_del(&bdev->cl_info->bus_client_link);
 		mei_cl_unlink(bdev->cl_info);
@@ -63,6 +71,73 @@ static void mei_nfc_free(struct mei_bus_dev_nfc *bdev)
 	}
 }
 
+static int mei_nfc_connect(struct mei_bus_dev_nfc *bdev)
+{
+	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 = bdev->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 = bdev->fw_ivn;
+	connect->vendor_id = bdev->vendor_id;
+
+	ret = mei_send(cl, (u8 *)cmd, connect_length);
+	if (ret < 0) {
+		dev_err(&dev->pdev->dev, "Could not send connect cmd\n");
+		goto err;
+	}
+
+	bytes_recv = mei_recv(cl, (u8 *)reply, connect_resp_length);
+	if (bytes_recv < 0) {
+		dev_err(&dev->pdev->dev, "Could not read connect response\n");
+		ret = bytes_recv;
+		goto err;
+	}
+
+	dev_info(&dev->pdev->dev, "IVN 0x%x Vendor ID 0x%x\n",
+		connect_resp->fw_ivn, connect_resp->vendor_id);
+
+	dev_info(&dev->pdev->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_bus_dev_nfc *bdev)
 {
 	struct mei_device *dev;
@@ -118,12 +193,13 @@ static void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
 	struct mei_bus_dev_nfc *bdev;
-	struct mei_cl *cl_info;
+	struct mei_cl *cl_info, *cl;
 	int ret;
 
 	bdev = container_of(work, struct mei_bus_dev_nfc, init_work);
 
 	cl_info = bdev->cl_info;
+	cl = bdev->cl;
 	dev = cl_info->dev;
 
 	mutex_lock(&dev->device_lock);
@@ -149,6 +225,24 @@ static void mei_nfc_init(struct work_struct *work)
 		"NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
 		bdev->fw_ivn, bdev->vendor_id, bdev->radio_type);
 
+	mutex_lock(&dev->device_lock);
+
+	if (mei_cl_connect(cl, NULL) < 0) {
+		mutex_unlock(&dev->device_lock);
+		dev_err(&dev->pdev->dev,
+			"Could not connect to the NFC ME client");
+
+		goto err;
+	}
+
+	mutex_unlock(&dev->device_lock);
+
+	ret = mei_nfc_connect(bdev);
+	if (ret < 0) {
+		dev_err(&dev->pdev->dev, "Could not connect to NFC");
+		return;
+	}
+
 	return;
 
 err:
@@ -161,7 +255,7 @@ err:
 int mei_nfc_host_init(struct mei_device *dev)
 {
 	struct mei_bus_dev_nfc *bdev = &nfc_bdev;
-	struct mei_cl *cl_info;
+	struct mei_cl *cl_info, *cl  = NULL;
 	int i, ret;
 
 	/* already initialzed */
@@ -169,14 +263,19 @@ int mei_nfc_host_init(struct mei_device *dev)
 		return 0;
 
 	cl_info = mei_cl_allocate(dev);
-	if (!cl_info)
-		return -ENOMEM;
+	cl = mei_cl_allocate(dev);
+
+	if (!cl || !cl_info) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	/* check for valid client id */
 	i = mei_me_cl_by_uuid(dev, &mei_nfc_info_guid);
 	if (i < 0) {
 		dev_info(&dev->pdev->dev, "nfc: failed to find the client\n");
-		return -ENOENT;
+		ret = -ENOENT;
+		goto err;
 	}
 
 	cl_info->me_client_id = dev->me_clients[i].client_id;
@@ -189,7 +288,26 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	list_add_tail(&cl_info->bus_client_link, &dev->bus_client_list);
 
+	/* check for valid client id */
+	i = mei_me_cl_by_uuid(dev, &mei_nfc_guid);
+	if (i < 0) {
+		dev_info(&dev->pdev->dev, "nfc: failed to find the client\n");
+		ret = -ENOENT;
+		goto err;
+	}
+
+	cl->me_client_id = dev->me_clients[i].client_id;
+
+	ret = mei_cl_link(cl, MEI_HOST_CLIENT_ID_ANY);
+	if (ret)
+		goto err;
+
+	cl->bus_client_uuid = mei_nfc_guid;
+
+	list_add_tail(&cl->bus_client_link, &dev->bus_client_list);
+
 	bdev->cl_info = cl_info;
+	bdev->cl = cl;
 
 	INIT_WORK(&bdev->init_work, mei_nfc_init);
 	schedule_work(&bdev->init_work);
-- 
1.7.4.4


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

* [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (8 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 09/11] mei: nfc: Connect also the regular ME client Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  2013-02-07 21:03 ` [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops Tomas Winkler
  10 siblings, 0 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

After building its bus name as a string based on its vendor id and radio
type, we can add it to the bus.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/nfc.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/mei/nfc.h |    6 +++++
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index ab66944..c65bd44 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -43,6 +43,8 @@ struct mei_bus_dev_nfc {
 	u8 fw_ivn;
 	u8 vendor_id;
 	u8 radio_type;
+
+	char *bus_name;
 };
 
 struct mei_bus_dev_nfc nfc_bdev;
@@ -71,6 +73,39 @@ static void mei_nfc_free(struct mei_bus_dev_nfc *bdev)
 	}
 }
 
+static int mei_nfc_build_bus_name(struct mei_bus_dev_nfc *bdev)
+{
+	struct mei_device *dev;
+
+	if (!bdev->cl)
+		return -ENODEV;
+
+	dev = bdev->cl->dev;
+
+	switch (bdev->vendor_id) {
+	case MEI_NFC_VENDOR_INSIDE:
+		switch (bdev->radio_type) {
+		case MEI_NFC_VENDOR_INSIDE_UREAD:
+			bdev->bus_name = "microread";
+			return 0;
+
+		default:
+			dev_err(&dev->pdev->dev, "Unknow radio type 0x%x\n",
+				bdev->radio_type);
+
+			return -EINVAL;
+		}
+
+	default:
+		dev_err(&dev->pdev->dev, "Unknow vendor ID 0x%x\n",
+			bdev->vendor_id);
+
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int mei_nfc_connect(struct mei_bus_dev_nfc *bdev)
 {
 	struct mei_device *dev;
@@ -192,6 +227,7 @@ err:
 static void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
+	struct mei_bus_client *bus_client;
 	struct mei_bus_dev_nfc *bdev;
 	struct mei_cl *cl_info, *cl;
 	int ret;
@@ -243,6 +279,23 @@ static void mei_nfc_init(struct work_struct *work)
 		return;
 	}
 
+	if (mei_nfc_build_bus_name(bdev) < 0) {
+		dev_err(&dev->pdev->dev,
+			"Could not build the bus ID name\n");
+		return;
+	}
+
+	bus_client = mei_add_device(dev, mei_nfc_guid,
+				    bdev->bus_name);
+	if (!bus_client) {
+		dev_err(&dev->pdev->dev,
+			"Could not add the NFC device to the MEI bus\n");
+
+		goto err;
+	}
+
+	bus_client->priv_data = bdev;
+
 	return;
 
 err:
@@ -324,5 +377,8 @@ void mei_nfc_host_exit(void)
 {
 	struct mei_bus_dev_nfc *bdev = &nfc_bdev;
 
+	if (bdev->cl && bdev->cl->client)
+		mei_remove_device(bdev->cl->client);
+
 	mei_nfc_free(bdev);
 }
diff --git a/drivers/misc/mei/nfc.h b/drivers/misc/mei/nfc.h
index 59e6703..4440436 100644
--- a/drivers/misc/mei/nfc.h
+++ b/drivers/misc/mei/nfc.h
@@ -119,4 +119,10 @@ struct mei_nfc_connect_resp {
 #define MEI_NFC_SUBCMD_CONNECT    0x00
 #define MEI_NFC_SUBCMD_IF_VERSION 0x01
 
+/* Vendors */
+#define MEI_NFC_VENDOR_INSIDE 0x00
+
+/* Radio types */
+#define MEI_NFC_VENDOR_INSIDE_UREAD 0x00
+
 #endif /* _MEI_NFC_H */
-- 
1.7.4.4


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

* [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops
  2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
                   ` (9 preceding siblings ...)
  2013-02-07 21:03 ` [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus Tomas Winkler
@ 2013-02-07 21:03 ` Tomas Winkler
  10 siblings, 0 replies; 37+ messages in thread
From: Tomas Winkler @ 2013-02-07 21:03 UTC (permalink / raw)
  To: gregkh, sameo; +Cc: arnd, linux-kernel, Tomas Winkler

From: Samuel Ortiz <sameo@linux.intel.com>

The send ops for NFC builds the command header, updates the request id
and then waits for an ACK.
The recv ops check if it receives data or an ACK and in the latter case
wakes the send ops up.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/nfc.c |   76 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/mei/nfc.h |   13 ++++++++
 2 files changed, 89 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index c65bd44..99246bc 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/sched.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/device.h>
@@ -40,11 +41,15 @@ struct mei_bus_dev_nfc {
 	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;
 };
 
 struct mei_bus_dev_nfc nfc_bdev;
@@ -224,6 +229,74 @@ err:
 	return ret;
 }
 
+static int mei_nfc_send(struct mei_bus_client *client, u8 *buf, size_t length)
+{
+	struct mei_device *dev;
+	struct mei_bus_dev_nfc *bdev;
+	struct mei_nfc_hci_hdr *hdr;
+	u8 *mei_buf;
+	int err;
+
+	bdev = (struct mei_bus_dev_nfc *) client->priv_data;
+	dev = bdev->cl->dev;
+
+	mei_buf = kzalloc(length + MEI_NFC_HEADER_SIZE, GFP_KERNEL);
+	if (!mei_buf)
+		return -ENOMEM;
+
+	hdr = (struct mei_nfc_hci_hdr *) mei_buf;
+	hdr->cmd = MEI_NFC_CMD_HCI_SEND;
+	hdr->status = 0;
+	hdr->req_id = bdev->req_id;
+	hdr->reserved = 0;
+	hdr->data_size = length;
+
+	memcpy(mei_buf + MEI_NFC_HEADER_SIZE, buf, length);
+
+	err = mei_send(bdev->cl, mei_buf, length + MEI_NFC_HEADER_SIZE);
+
+	kfree(mei_buf);
+
+	if (!wait_event_interruptible_timeout(bdev->send_wq,
+				bdev->recv_req_id == bdev->req_id, HZ)) {
+		dev_err(&dev->pdev->dev, "NFC MEI command timeout\n");
+		err = -ETIMEDOUT;
+	} else {
+		bdev->req_id++;
+	}
+
+	return err;
+}
+
+static int mei_nfc_recv(struct mei_bus_client *client, u8 *buf, size_t length)
+{
+	struct mei_bus_dev_nfc *bdev;
+	struct mei_nfc_hci_hdr *hci_hdr;
+	int received_length;
+
+	bdev = (struct mei_bus_dev_nfc *) client->priv_data;
+
+	received_length = mei_recv(bdev->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) {
+		bdev->recv_req_id = hci_hdr->req_id;
+		wake_up(&bdev->send_wq);
+
+		return 0;
+	}
+
+	return received_length;
+}
+
+static struct mei_bus_ops nfc_ops = {
+	.send = mei_nfc_send,
+	.recv = mei_nfc_recv,
+};
+
 static void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
@@ -295,6 +368,7 @@ static void mei_nfc_init(struct work_struct *work)
 	}
 
 	bus_client->priv_data = bdev;
+	bus_client->ops = &nfc_ops;
 
 	return;
 
@@ -361,8 +435,10 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	bdev->cl_info = cl_info;
 	bdev->cl = cl;
+	bdev->req_id = 1;
 
 	INIT_WORK(&bdev->init_work, mei_nfc_init);
+	init_waitqueue_head(&bdev->send_wq);
 	schedule_work(&bdev->init_work);
 
 	return 0;
diff --git a/drivers/misc/mei/nfc.h b/drivers/misc/mei/nfc.h
index 4440436..12e48d3 100644
--- a/drivers/misc/mei/nfc.h
+++ b/drivers/misc/mei/nfc.h
@@ -114,11 +114,24 @@ struct mei_nfc_connect_resp {
 	uint16_t 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
+#define MEI_NFC_MAX_HCI_PAYLOAD 300
+
 /* Vendors */
 #define MEI_NFC_VENDOR_INSIDE 0x00
 
-- 
1.7.4.4


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

* Re: [char-misc-next 08/11] mei: nfc: Initial nfc implementation
  2013-02-07 21:03 ` [char-misc-next 08/11] mei: nfc: Initial nfc implementation Tomas Winkler
@ 2013-02-07 22:26   ` Arnd Bergmann
  2013-02-07 22:41     ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:26 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:
> 
> From: Samuel Ortiz <sameo@linux.intel.com>
> 
> NFC ME client is exported through mei bus to be consumed by the
> NFC subsystem.
> 
> NFC is represented by two mei clients: An info one and the actual
> NFC one. In order for correct connection we first need to retrieve the
> firmware information from the info client.
> 
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Shouldn't this be moved to the drivers/nfc directory? Generally speaking,
all drivers nowadays tend to live in the directories of the subsystems
they are implementing support for, not the subsystems that they are
implemented with.

	Arnd

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

* Re: [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation
  2013-02-07 21:03 ` [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation Tomas Winkler
@ 2013-02-07 22:29   ` Arnd Bergmann
  2013-02-07 22:41   ` Arnd Bergmann
  1 sibling, 0 replies; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:29 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:
> From: Samuel Ortiz <sameo@linux.intel.com>
> 
> mei bus will present some of the me clients
> as devices for other standard subsystems
> 
> Implement the probe, remove, match and the device addtion routines.
> A mei-bus.txt document describing the rationale and the API usage
> is also added.

This looks really nice, but I also think you have reached the point where
you are outgrowing the scope of drivers/misc. How about turning mei
into a top-level subsystem along with this new bus_type?

Another option would be moving it to drivers/bus/mei/ if you don't
want to be quite at the top.

	Arnd

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

* Re: [char-misc-next 02/11] mei: bus: Implement driver registration
  2013-02-07 21:03 ` [char-misc-next 02/11] mei: bus: Implement driver registration Tomas Winkler
@ 2013-02-07 22:30   ` Arnd Bergmann
  0 siblings, 0 replies; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:30 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:

> +int mei_add_driver(struct mei_bus_driver *driver)

By convention, I think this should be called mei_driver_register(),
matching what we do on most other subsystem. Similarly
for mei_driver_unregister().

	Arnd

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-07 21:03 ` [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines Tomas Winkler
@ 2013-02-07 22:34   ` Arnd Bergmann
  2013-02-07 22:55     ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:34 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:
> +
> +struct mei_bus_ops {
> +       int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
> +       int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
> +};
> +

Can you have more than one set of mei_bus_ops in a driver? If not,
how about adding the callbacks to the mei_bus_driver structure
directly as a simplification? Never mind if that doesn't work for
some obvious reason I missed.

	Arnd

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

* Re: [char-misc-next 05/11] mei: bus: Call bus routines from the core code
  2013-02-07 21:03 ` [char-misc-next 05/11] mei: bus: Call bus routines from the core code Tomas Winkler
@ 2013-02-07 22:37   ` Arnd Bergmann
  2013-02-07 22:57     ` Winkler, Tomas
  2013-02-07 22:57     ` Samuel Ortiz
  0 siblings, 2 replies; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:37 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:
> @@ -197,6 +197,9 @@ static int mei_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         mei_pdev = pdev;
>         pci_set_drvdata(pdev, dev);
>  
> +       err = mei_bus_init(mei_pdev);
> +       if (err)
> +               goto deregister_mei;
>  
>         schedule_delayed_work(&dev->timer_work, HZ);
>  

This is fairly unusual, and will break if you ever have multiple mei devices
in one system, because you end up registering the bus type for each
device. I think it would be more logical to register/unregister
the bus_type from the module_init/exit functions of the module
that contains the bus_type object.

	Arnd

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-07 21:03 ` [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter Tomas Winkler
@ 2013-02-07 22:38   ` Arnd Bergmann
  2013-02-07 22:58     ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:38 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:
> 
> +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> +{
> +       return dev_get_drvdata(&client->dev);
> +}
> +EXPORT_SYMBOL(mei_bus_get_clientdata);
> +

Did you really mean to export an inline function?

Can you make this a static inline function in a header file instead?

	Arnd

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

* Re: [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation
  2013-02-07 21:03 ` [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation Tomas Winkler
  2013-02-07 22:29   ` Arnd Bergmann
@ 2013-02-07 22:41   ` Arnd Bergmann
  2013-02-07 22:59     ` Samuel Ortiz
  1 sibling, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 22:41 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Tomas Winkler wrote:
> +}
> +EXPORT_SYMBOL(mei_add_device);
> +
> +void mei_remove_device(struct mei_bus_client *client)
> +{
> +       device_unregister(&client->dev);
> +}
> +EXPORT_SYMBOL(mei_remove_device);

One more point: did you intentionally pick EXPORT_SYMBOL over
EXPORT_SYMBOL_GPL here? It is your decision as the copyright
holder, but the default is often EXPORT_SYMBOL_GPL these
days, to make it clear that you don't expect closed source
drivers to plug in there.

	Arnd

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

* Re: [char-misc-next 08/11] mei: nfc: Initial nfc implementation
  2013-02-07 22:26   ` Arnd Bergmann
@ 2013-02-07 22:41     ` Samuel Ortiz
  0 siblings, 0 replies; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-07 22:41 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

Hi Arnd,

On Thu, Feb 07, 2013 at 10:26:42PM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Tomas Winkler wrote:
> > 
> > From: Samuel Ortiz <sameo@linux.intel.com>
> > 
> > NFC ME client is exported through mei bus to be consumed by the
> > NFC subsystem.
> > 
> > NFC is represented by two mei clients: An info one and the actual
> > NFC one. In order for correct connection we first need to retrieve the
> > firmware information from the info client.
> > 
> > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> 
> Shouldn't this be moved to the drivers/nfc directory? Generally speaking,
> all drivers nowadays tend to live in the directories of the subsystems
> they are implementing support for, not the subsystems that they are
> implemented with.
This is a bit on the edge: This part of the MEI bus code doesn't implement
support for any NFC chipset in particular but for the MEI specific commands to
send and receive NFC HCI payloads.
So the drivers under driver/nfc call into the MEI bus I/O API which then call
this code to encapsulate the HCI payload into MEI (HECI) commands. The
microread driver in my nfc-next git.kernel.org tree uses the MEI bus API for
example.
Moreover this code actually talks to the ME at init time to understand which
exact NFC chipset is living behind the ME and add the correct device string to
the bus. So it really needs to access the MEI internal API to do so.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-07 22:34   ` Arnd Bergmann
@ 2013-02-07 22:55     ` Samuel Ortiz
  2013-02-11 11:52       ` Arnd Bergmann
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-07 22:55 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

Hi Arnd,

On Thu, Feb 07, 2013 at 10:34:44PM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Tomas Winkler wrote:
> > +
> > +struct mei_bus_ops {
> > +       int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
> > +       int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
> > +};
> > +
> 
> Can you have more than one set of mei_bus_ops in a driver? 
You can have at most one mei_bus_ops per mei_bus_client.

> If not, how about adding the callbacks to the mei_bus_driver structure
> directly as a simplification? 
I can add the ops directly to the mei_bus_client structure, yes.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* RE: [char-misc-next 05/11] mei: bus: Call bus routines from the core code
  2013-02-07 22:37   ` Arnd Bergmann
@ 2013-02-07 22:57     ` Winkler, Tomas
  2013-02-07 23:09       ` Arnd Bergmann
  2013-02-07 22:57     ` Samuel Ortiz
  1 sibling, 1 reply; 37+ messages in thread
From: Winkler, Tomas @ 2013-02-07 22:57 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: gregkh, sameo, linux-kernel



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Friday, February 08, 2013 00:38
> To: Winkler, Tomas
> Cc: gregkh@linuxfoundation.org; sameo@linux.intel.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [char-misc-next 05/11] mei: bus: Call bus routines from the core
> code
> 
> On Thursday 07 February 2013, Tomas Winkler wrote:
> > @@ -197,6 +197,9 @@ static int mei_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> >         mei_pdev = pdev;
> >         pci_set_drvdata(pdev, dev);
> >
> > +       err = mei_bus_init(mei_pdev);
> > +       if (err)
> > +               goto deregister_mei;
> >
> >         schedule_delayed_work(&dev->timer_work, HZ);
> >
> 
> This is fairly unusual, and will break if you ever have multiple mei devices in
> one system, because you end up registering the bus type for each device. I
> think it would be more logical to register/unregister the bus_type from the
> module_init/exit functions of the module that contains the bus_type object.

MEI is a singleton and it is enforced also in software in the probe 

Second why to register anything if the MEI device is not present on the system. 

Thanks
Tomas


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

* Re: [char-misc-next 05/11] mei: bus: Call bus routines from the core code
  2013-02-07 22:37   ` Arnd Bergmann
  2013-02-07 22:57     ` Winkler, Tomas
@ 2013-02-07 22:57     ` Samuel Ortiz
  1 sibling, 0 replies; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-07 22:57 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

Hi Arnd,

On Thu, Feb 07, 2013 at 10:37:30PM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Tomas Winkler wrote:
> > @@ -197,6 +197,9 @@ static int mei_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >         mei_pdev = pdev;
> >         pci_set_drvdata(pdev, dev);
> >  
> > +       err = mei_bus_init(mei_pdev);
> > +       if (err)
> > +               goto deregister_mei;
> >  
> >         schedule_delayed_work(&dev->timer_work, HZ);
> >  
> 
> This is fairly unusual, and will break if you ever have multiple mei devices
> in one system, because you end up registering the bus type for each
> device. I think it would be more logical to register/unregister
> the bus_type from the module_init/exit functions of the module
> that contains the bus_type object.
That makes sense, I'll fix that.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-07 22:38   ` Arnd Bergmann
@ 2013-02-07 22:58     ` Samuel Ortiz
  2013-02-07 23:57       ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-07 22:58 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

On Thu, Feb 07, 2013 at 10:38:44PM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Tomas Winkler wrote:
> > 
> > +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> > +{
> > +       return dev_get_drvdata(&client->dev);
> > +}
> > +EXPORT_SYMBOL(mei_bus_get_clientdata);
> > +
> 
> Did you really mean to export an inline function?
> 
> Can you make this a static inline function in a header file instead?
Sure, will do.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation
  2013-02-07 22:41   ` Arnd Bergmann
@ 2013-02-07 22:59     ` Samuel Ortiz
  2013-02-07 23:07       ` Winkler, Tomas
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-07 22:59 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

On Thu, Feb 07, 2013 at 10:41:06PM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Tomas Winkler wrote:
> > +}
> > +EXPORT_SYMBOL(mei_add_device);
> > +
> > +void mei_remove_device(struct mei_bus_client *client)
> > +{
> > +       device_unregister(&client->dev);
> > +}
> > +EXPORT_SYMBOL(mei_remove_device);
> 
> One more point: did you intentionally pick EXPORT_SYMBOL over
> EXPORT_SYMBOL_GPL here? 
No, I didn't.

> It is your decision as the copyright
> holder, but the default is often EXPORT_SYMBOL_GPL these
> days, to make it clear that you don't expect closed source
> drivers to plug in there.
I'll fix that if Tomas is ok with it.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* RE: [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation
  2013-02-07 22:59     ` Samuel Ortiz
@ 2013-02-07 23:07       ` Winkler, Tomas
  0 siblings, 0 replies; 37+ messages in thread
From: Winkler, Tomas @ 2013-02-07 23:07 UTC (permalink / raw)
  To: Samuel Ortiz, Arnd Bergmann; +Cc: gregkh, linux-kernel



> -----Original Message-----
> From: Samuel Ortiz [mailto:sameo@linux.intel.com]
> Sent: Friday, February 08, 2013 00:59
> To: Arnd Bergmann
> Cc: Winkler, Tomas; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [char-misc-next 01/11] mei: bus: Initial MEI bus type
> implementation
> 
> On Thu, Feb 07, 2013 at 10:41:06PM +0000, Arnd Bergmann wrote:
> > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > +}
> > > +EXPORT_SYMBOL(mei_add_device);
> > > +
> > > +void mei_remove_device(struct mei_bus_client *client) {
> > > +       device_unregister(&client->dev); }
> > > +EXPORT_SYMBOL(mei_remove_device);
> >
> > One more point: did you intentionally pick EXPORT_SYMBOL over
> > EXPORT_SYMBOL_GPL here?
> No, I didn't.
> 
> > It is your decision as the copyright
> > holder, but the default is often EXPORT_SYMBOL_GPL these days, to make
> > it clear that you don't expect closed source drivers to plug in there.
> I'll fix that if Tomas is ok with it.

So far I'm happy with not forcing GPL,  but I will check that issue.

Thanks
Tomas



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

* Re: [char-misc-next 05/11] mei: bus: Call bus routines from the core code
  2013-02-07 22:57     ` Winkler, Tomas
@ 2013-02-07 23:09       ` Arnd Bergmann
  0 siblings, 0 replies; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-07 23:09 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: gregkh, sameo, linux-kernel

On Thursday 07 February 2013, Winkler, Tomas wrote:
> Second why to register anything if the MEI device is not present on the system. 

Mostly to match the expectations of readers of that code. Note that no memory
is wasted if you do this, because it's a static data structure. You can
actually save a few bytes because the registration call moves into an __init
section that is discarded after boot.

	Arnd

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-07 22:58     ` Samuel Ortiz
@ 2013-02-07 23:57       ` Samuel Ortiz
  2013-02-11 14:58         ` Arnd Bergmann
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-07 23:57 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

On Thu, Feb 07, 2013 at 11:58:09PM +0100, Samuel Ortiz wrote:
> On Thu, Feb 07, 2013 at 10:38:44PM +0000, Arnd Bergmann wrote:
> > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > 
> > > +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> > > +{
> > > +       return dev_get_drvdata(&client->dev);
> > > +}
> > > +EXPORT_SYMBOL(mei_bus_get_clientdata);
> > > +
> > 
> > Did you really mean to export an inline function?
> > 
> > Can you make this a static inline function in a header file instead?
> Sure, will do.
Actually, I'd like to keep the mei_bus_client structure opaque to the MEI bus
drivers. So I'll still export those symbols without inlining them.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-07 22:55     ` Samuel Ortiz
@ 2013-02-11 11:52       ` Arnd Bergmann
  2013-02-11 12:58         ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-11 11:52 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Tomas Winkler, gregkh, linux-kernel

On Thursday 07 February 2013, Samuel Ortiz wrote:
> On Thu, Feb 07, 2013 at 10:34:44PM +0000, Arnd Bergmann wrote:
> > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > +
> > > +struct mei_bus_ops {
> > > +       int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > +       int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > +};
> > > +
> > 
> > Can you have more than one set of mei_bus_ops in a driver? 
> You can have at most one mei_bus_ops per mei_bus_client.
> 
> > If not, how about adding the callbacks to the mei_bus_driver structure
> > directly as a simplification? 
> I can add the ops directly to the mei_bus_client structure, yes.

I looked at the new version, and it's not what I assumed it would be.
I thought the operations were specific to a client driver and should
be part of the /mei_bus_driver/ structure, not the /mei_bus_client/.

Did I misunderstand what these functions do, or did you misunderstand
what I was asking for?

	Arnd

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-11 11:52       ` Arnd Bergmann
@ 2013-02-11 12:58         ` Samuel Ortiz
  2013-02-11 15:08           ` Arnd Bergmann
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-11 12:58 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

Hi Arnd,

On Mon, Feb 11, 2013 at 11:52:42AM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Samuel Ortiz wrote:
> > On Thu, Feb 07, 2013 at 10:34:44PM +0000, Arnd Bergmann wrote:
> > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > +
> > > > +struct mei_bus_ops {
> > > > +       int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > > +       int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > > +};
> > > > +
> > > 
> > > Can you have more than one set of mei_bus_ops in a driver? 
> > You can have at most one mei_bus_ops per mei_bus_client.
> > 
> > > If not, how about adding the callbacks to the mei_bus_driver structure
> > > directly as a simplification? 
> > I can add the ops directly to the mei_bus_client structure, yes.
> 
> I looked at the new version, and it's not what I assumed it would be.
> I thought the operations were specific to a client driver and should
> be part of the /mei_bus_driver/ structure, not the /mei_bus_client/.
The ops should be part of mei_bus_client as they're specific to the MEI
protocol for a given IP block on the ME. You need to have MEI and HECI
knowledge to implement those ops and drivers (defining their mei_bus_driver
structure) should not have that kind of knowledge but rather focus on the
technology they're driving.
If we take the NFC example again, the drivers/nfc/ code will send NFC payloads
to the bus I/O routines and this is where the mei_bus_client ops will add the
ME specific protocol (command and request id for the NFC block) on top of it.
In practice, this is an additional header which handles a transport layer that's
specific not only to the ME but to the NFC block of it. So each ME block can
have its own protocol to send and receive technology specific payloads, that's
what those ops implement.
That's why I think that those ops should not be defined by the drivers/nfc/ code
and in fact should be opaque to it.


> Did I misunderstand what these functions do, or did you misunderstand
> what I was asking for?
Probably both. I really thought you were looking for a structure cleanup by
directly putting the ops into the mei_bus_client structure.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-07 23:57       ` Samuel Ortiz
@ 2013-02-11 14:58         ` Arnd Bergmann
  2013-02-11 15:29           ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-11 14:58 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Tomas Winkler, gregkh, linux-kernel

On Thursday 07 February 2013, Samuel Ortiz wrote:
> 
> On Thu, Feb 07, 2013 at 11:58:09PM +0100, Samuel Ortiz wrote:
> > On Thu, Feb 07, 2013 at 10:38:44PM +0000, Arnd Bergmann wrote:
> > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > 
> > > > +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> > > > +{
> > > > +       return dev_get_drvdata(&client->dev);
> > > > +}
> > > > +EXPORT_SYMBOL(mei_bus_get_clientdata);
> > > > +
> > > 
> > > Did you really mean to export an inline function?
> > > 
> > > Can you make this a static inline function in a header file instead?
> > Sure, will do.
> Actually, I'd like to keep the mei_bus_client structure opaque to the MEI bus
> drivers. So I'll still export those symbols without inlining them.

Ok, makes sense. I assume when you say "bus driver" you mean what other
subsystems call a "device driver".

	Arnd

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-11 12:58         ` Samuel Ortiz
@ 2013-02-11 15:08           ` Arnd Bergmann
  2013-02-11 15:48             ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-11 15:08 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Tomas Winkler, gregkh, linux-kernel

On Monday 11 February 2013, Samuel Ortiz wrote:
> On Mon, Feb 11, 2013 at 11:52:42AM +0000, Arnd Bergmann wrote:
> > On Thursday 07 February 2013, Samuel Ortiz wrote:
> > > On Thu, Feb 07, 2013 at 10:34:44PM +0000, Arnd Bergmann wrote:
> > > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > > +
> > > > > +struct mei_bus_ops {
> > > > > +       int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > > > +       int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > > > +};
> > > > > +
> > > > 
> > > > Can you have more than one set of mei_bus_ops in a driver? 
> > > You can have at most one mei_bus_ops per mei_bus_client.
> > > 
> > > > If not, how about adding the callbacks to the mei_bus_driver structure
> > > > directly as a simplification? 
> > > I can add the ops directly to the mei_bus_client structure, yes.
> > 
> > I looked at the new version, and it's not what I assumed it would be.
> > I thought the operations were specific to a client driver and should
> > be part of the /mei_bus_driver/ structure, not the /mei_bus_client/.
> The ops should be part of mei_bus_client as they're specific to the MEI
> protocol for a given IP block on the ME. You need to have MEI and HECI
> knowledge to implement those ops and drivers (defining their mei_bus_driver
> structure) should not have that kind of knowledge but rather focus on the
> technology they're driving.
> If we take the NFC example again, the drivers/nfc/ code will send NFC payloads
> to the bus I/O routines and this is where the mei_bus_client ops will add the
> ME specific protocol (command and request id for the NFC block) on top of it.
> In practice, this is an additional header which handles a transport layer that's
> specific not only to the ME but to the NFC block of it. So each ME block can
> have its own protocol to send and receive technology specific payloads, that's
> what those ops implement.
> That's why I think that those ops should not be defined by the drivers/nfc/ code
> and in fact should be opaque to it.

I think I'm still confused. What I read in your description is that unlike
other subsystems that have a common bus implementation that is hooked into
by two kinds of drivers (bus drivers that provide devices like a USB host
controller, and device drivers that use those devices), you have three
components that can be mixed independently: the bus (based on which
PCI device you have), the transport and the endpoint device. Is that
correct?

If so, how do you know which transport to use?

	Arnd

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-11 14:58         ` Arnd Bergmann
@ 2013-02-11 15:29           ` Samuel Ortiz
  2013-02-11 16:03             ` Greg KH
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-11 15:29 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

Hi Arnd,

On Mon, Feb 11, 2013 at 02:58:37PM +0000, Arnd Bergmann wrote:
> On Thursday 07 February 2013, Samuel Ortiz wrote:
> > 
> > On Thu, Feb 07, 2013 at 11:58:09PM +0100, Samuel Ortiz wrote:
> > > On Thu, Feb 07, 2013 at 10:38:44PM +0000, Arnd Bergmann wrote:
> > > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > > 
> > > > > +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> > > > > +{
> > > > > +       return dev_get_drvdata(&client->dev);
> > > > > +}
> > > > > +EXPORT_SYMBOL(mei_bus_get_clientdata);
> > > > > +
> > > > 
> > > > Did you really mean to export an inline function?
> > > > 
> > > > Can you make this a static inline function in a header file instead?
> > > Sure, will do.
> > Actually, I'd like to keep the mei_bus_client structure opaque to the MEI bus
> > drivers. So I'll still export those symbols without inlining them.
> 
> Ok, makes sense. I assume when you say "bus driver" you mean what other
> subsystems call a "device driver".
That's correct, yes.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-11 15:08           ` Arnd Bergmann
@ 2013-02-11 15:48             ` Samuel Ortiz
  2013-02-11 16:12               ` Arnd Bergmann
  0 siblings, 1 reply; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-11 15:48 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Tomas Winkler, gregkh, linux-kernel

Hi Arnd,

On Mon, Feb 11, 2013 at 03:08:24PM +0000, Arnd Bergmann wrote:
> On Monday 11 February 2013, Samuel Ortiz wrote:
> > On Mon, Feb 11, 2013 at 11:52:42AM +0000, Arnd Bergmann wrote:
> > > On Thursday 07 February 2013, Samuel Ortiz wrote:
> > > > On Thu, Feb 07, 2013 at 10:34:44PM +0000, Arnd Bergmann wrote:
> > > > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > > > +
> > > > > > +struct mei_bus_ops {
> > > > > > +       int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > > > > +       int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
> > > > > > +};
> > > > > > +
> > > > > 
> > > > > Can you have more than one set of mei_bus_ops in a driver? 
> > > > You can have at most one mei_bus_ops per mei_bus_client.
> > > > 
> > > > > If not, how about adding the callbacks to the mei_bus_driver structure
> > > > > directly as a simplification? 
> > > > I can add the ops directly to the mei_bus_client structure, yes.
> > > 
> > > I looked at the new version, and it's not what I assumed it would be.
> > > I thought the operations were specific to a client driver and should
> > > be part of the /mei_bus_driver/ structure, not the /mei_bus_client/.
> > The ops should be part of mei_bus_client as they're specific to the MEI
> > protocol for a given IP block on the ME. You need to have MEI and HECI
> > knowledge to implement those ops and drivers (defining their mei_bus_driver
> > structure) should not have that kind of knowledge but rather focus on the
> > technology they're driving.
> > If we take the NFC example again, the drivers/nfc/ code will send NFC payloads
> > to the bus I/O routines and this is where the mei_bus_client ops will add the
> > ME specific protocol (command and request id for the NFC block) on top of it.
> > In practice, this is an additional header which handles a transport layer that's
> > specific not only to the ME but to the NFC block of it. So each ME block can
> > have its own protocol to send and receive technology specific payloads, that's
> > what those ops implement.
> > That's why I think that those ops should not be defined by the drivers/nfc/ code
> > and in fact should be opaque to it.
> 
> I think I'm still confused. 
It is confusing, and my explanations are probably not as good as they should 
be.

> What I read in your description is that unlike
> other subsystems that have a common bus implementation that is hooked into
> by two kinds of drivers (bus drivers that provide devices like a USB host
> controller, and device drivers that use those devices), you have three
> components that can be mixed independently: the bus (based on which
> PCI device you have), the transport and the endpoint device. Is that
> correct?
That's quite accurate, yes.


> If so, how do you know which transport to use?
Through the mei_bus_client ops. Device drivers get a mei_bus_client pointer
from their probe routine and the ops pointers there (If any) are set by
whoever creates the device. In the NFC case mei/nfc.c does that and implements
the NFC specific transport code for this technology. mei/nfc.c is also the
part of the code that actually adds the device to the bus.

So when a device driver wants e.g. to send its payload through the MEI bus, it
calls mei_bus_send() which takes the device driver mei_bus client pointer as
its first argument.
Then the payload may go through mei_bus_client->send() first which will eventually
physically sent the newly built frame through mei_send(). Some ME blocks don't require any
additional transport layer and in that case the device driver payload will go
straight to mei_send() since the mei_bus_client ops will be NULL.

Does that make more sense now ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-11 15:29           ` Samuel Ortiz
@ 2013-02-11 16:03             ` Greg KH
  2013-02-11 16:05               ` Samuel Ortiz
  0 siblings, 1 reply; 37+ messages in thread
From: Greg KH @ 2013-02-11 16:03 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Arnd Bergmann, Tomas Winkler, linux-kernel

On Mon, Feb 11, 2013 at 04:29:29PM +0100, Samuel Ortiz wrote:
> Hi Arnd,
> 
> On Mon, Feb 11, 2013 at 02:58:37PM +0000, Arnd Bergmann wrote:
> > On Thursday 07 February 2013, Samuel Ortiz wrote:
> > > 
> > > On Thu, Feb 07, 2013 at 11:58:09PM +0100, Samuel Ortiz wrote:
> > > > On Thu, Feb 07, 2013 at 10:38:44PM +0000, Arnd Bergmann wrote:
> > > > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > > > 
> > > > > > +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> > > > > > +{
> > > > > > +       return dev_get_drvdata(&client->dev);
> > > > > > +}
> > > > > > +EXPORT_SYMBOL(mei_bus_get_clientdata);
> > > > > > +
> > > > > 
> > > > > Did you really mean to export an inline function?
> > > > > 
> > > > > Can you make this a static inline function in a header file instead?
> > > > Sure, will do.
> > > Actually, I'd like to keep the mei_bus_client structure opaque to the MEI bus
> > > drivers. So I'll still export those symbols without inlining them.
> > 
> > Ok, makes sense. I assume when you say "bus driver" you mean what other
> > subsystems call a "device driver".
> That's correct, yes.

Thanks for clearing that up.  So, in your next revision, can you use the
"proper" names for everything so that we all can understand the code
easier as it will follow the normal standards for kernel driver model
interaction?

thanks,

greg k-h

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

* Re: [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter
  2013-02-11 16:03             ` Greg KH
@ 2013-02-11 16:05               ` Samuel Ortiz
  0 siblings, 0 replies; 37+ messages in thread
From: Samuel Ortiz @ 2013-02-11 16:05 UTC (permalink / raw)
  To: Greg KH; +Cc: Arnd Bergmann, Tomas Winkler, linux-kernel

On Mon, Feb 11, 2013 at 08:03:46AM -0800, Greg KH wrote:
> On Mon, Feb 11, 2013 at 04:29:29PM +0100, Samuel Ortiz wrote:
> > Hi Arnd,
> > 
> > On Mon, Feb 11, 2013 at 02:58:37PM +0000, Arnd Bergmann wrote:
> > > On Thursday 07 February 2013, Samuel Ortiz wrote:
> > > > 
> > > > On Thu, Feb 07, 2013 at 11:58:09PM +0100, Samuel Ortiz wrote:
> > > > > On Thu, Feb 07, 2013 at 10:38:44PM +0000, Arnd Bergmann wrote:
> > > > > > On Thursday 07 February 2013, Tomas Winkler wrote:
> > > > > > > 
> > > > > > > +inline void *mei_bus_get_clientdata(const struct mei_bus_client *client)
> > > > > > > +{
> > > > > > > +       return dev_get_drvdata(&client->dev);
> > > > > > > +}
> > > > > > > +EXPORT_SYMBOL(mei_bus_get_clientdata);
> > > > > > > +
> > > > > > 
> > > > > > Did you really mean to export an inline function?
> > > > > > 
> > > > > > Can you make this a static inline function in a header file instead?
> > > > > Sure, will do.
> > > > Actually, I'd like to keep the mei_bus_client structure opaque to the MEI bus
> > > > drivers. So I'll still export those symbols without inlining them.
> > > 
> > > Ok, makes sense. I assume when you say "bus driver" you mean what other
> > > subsystems call a "device driver".
> > That's correct, yes.
> 
> Thanks for clearing that up.  So, in your next revision, can you use the
> "proper" names for everything so that we all can understand the code
> easier as it will follow the normal standards for kernel driver model
> interaction?
I will certainly do that, yes.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

* Re: [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines
  2013-02-11 15:48             ` Samuel Ortiz
@ 2013-02-11 16:12               ` Arnd Bergmann
  0 siblings, 0 replies; 37+ messages in thread
From: Arnd Bergmann @ 2013-02-11 16:12 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Tomas Winkler, gregkh, linux-kernel

On Monday 11 February 2013 16:48:12 Samuel Ortiz wrote:
> 
> > If so, how do you know which transport to use?
> Through the mei_bus_client ops. Device drivers get a mei_bus_client pointer
> from their probe routine and the ops pointers there (If any) are set by
> whoever creates the device. In the NFC case mei/nfc.c does that and implements
> the NFC specific transport code for this technology. mei/nfc.c is also the
> part of the code that actually adds the device to the bus.

Ok.

> So when a device driver wants e.g. to send its payload through the MEI bus, it
> calls mei_bus_send() which takes the device driver mei_bus client pointer as
> its first argument.
> Then the payload may go through mei_bus_client->send() first which will eventually
> physically sent the newly built frame through mei_send(). Some ME blocks don't require any
> additional transport layer and in that case the device driver payload will go
> straight to mei_send() since the mei_bus_client ops will be NULL.
> 
> Does that make more sense now ?

Yes, so it's not actually as complicated as I thought, because
the operations are already known by the bus driver (in the
normal sense of that word, meaning the driver that creates
the device not the one driving it) at the time when the device is
created.

In that case, please ignore my original comment and put the
struct mei_bus_ops (mei_transport_ops?) back, ideally as a
"const" member of the struct mei_bus_device (to be called
mei_device then), so you can declare the structure statically
and constant in the driver that has the functions.

	Arnd

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

end of thread, other threads:[~2013-02-11 16:12 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-07 21:03 [char-misc-next 00/11] Add MEI BUS and NFC Device Tomas Winkler
2013-02-07 21:03 ` [char-misc-next 01/11] mei: bus: Initial MEI bus type implementation Tomas Winkler
2013-02-07 22:29   ` Arnd Bergmann
2013-02-07 22:41   ` Arnd Bergmann
2013-02-07 22:59     ` Samuel Ortiz
2013-02-07 23:07       ` Winkler, Tomas
2013-02-07 21:03 ` [char-misc-next 02/11] mei: bus: Implement driver registration Tomas Winkler
2013-02-07 22:30   ` Arnd Bergmann
2013-02-07 21:03 ` [char-misc-next 03/11] mei: bus: Initial implementation for I/O routines Tomas Winkler
2013-02-07 22:34   ` Arnd Bergmann
2013-02-07 22:55     ` Samuel Ortiz
2013-02-11 11:52       ` Arnd Bergmann
2013-02-11 12:58         ` Samuel Ortiz
2013-02-11 15:08           ` Arnd Bergmann
2013-02-11 15:48             ` Samuel Ortiz
2013-02-11 16:12               ` Arnd Bergmann
2013-02-07 21:03 ` [char-misc-next 04/11] mei: bus: Add bus related structures to mei_cl Tomas Winkler
2013-02-07 21:03 ` [char-misc-next 05/11] mei: bus: Call bus routines from the core code Tomas Winkler
2013-02-07 22:37   ` Arnd Bergmann
2013-02-07 22:57     ` Winkler, Tomas
2013-02-07 23:09       ` Arnd Bergmann
2013-02-07 22:57     ` Samuel Ortiz
2013-02-07 21:03 ` [char-misc-next 06/11] mei: bus: Synchronous API for the data transmission Tomas Winkler
2013-02-07 21:03 ` [char-misc-next 07/11] mei: bus: Implement bus driver data setter/getter Tomas Winkler
2013-02-07 22:38   ` Arnd Bergmann
2013-02-07 22:58     ` Samuel Ortiz
2013-02-07 23:57       ` Samuel Ortiz
2013-02-11 14:58         ` Arnd Bergmann
2013-02-11 15:29           ` Samuel Ortiz
2013-02-11 16:03             ` Greg KH
2013-02-11 16:05               ` Samuel Ortiz
2013-02-07 21:03 ` [char-misc-next 08/11] mei: nfc: Initial nfc implementation Tomas Winkler
2013-02-07 22:26   ` Arnd Bergmann
2013-02-07 22:41     ` Samuel Ortiz
2013-02-07 21:03 ` [char-misc-next 09/11] mei: nfc: Connect also the regular ME client Tomas Winkler
2013-02-07 21:03 ` [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus Tomas Winkler
2013-02-07 21:03 ` [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops Tomas Winkler

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