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

This is take 2 on the MEI bus + NFC Device patches addressing Arnd's comments

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


V2:
1. Rename mei_add_driver to mei_driver_register and mei_del_driver to
 mei_driver_unregister.
2. Do not inline the exported bus driver data setter and getter functions.
3. Include the bus ops pointers directly into mei_bus_client.
4. Move the mei_bus_init/exit call from probe to module init time.

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                 |   79 +++++
 drivers/misc/mei/nfc.c                     |  456 ++++++++++++++++++++++++++
 drivers/misc/mei/nfc.h                     |  141 ++++++++
 drivers/misc/mei/pci-me.c                  |   23 ++-
 include/linux/mei_bus.h                    |  108 ++++++
 13 files changed, 1473 insertions(+), 2 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] 25+ messages in thread

* [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 23:53   ` Greg KH
  2013-02-08 12:28 ` [char-misc-next 02/11 V2] mei: bus: Implement driver registration Tomas Winkler
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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] 25+ messages in thread

* [char-misc-next 02/11 V2] mei: bus: Implement driver registration
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 13:36   ` Arnd Bergmann
  2013-02-08 23:55   ` Greg KH
  2013-02-08 12:28 ` [char-misc-next 03/11 V2] mei: bus: Initial implementation for I/O routines Tomas Winkler
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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..0a5e624 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_driver_register(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_driver_register);
+
+void mei_driver_unregister(struct mei_bus_driver *driver)
+{
+	driver_unregister(&driver->driver);
+
+	pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
+}
+EXPORT_SYMBOL(mei_driver_unregister);
diff --git a/include/linux/mei_bus.h b/include/linux/mei_bus.h
index 3a53f9e..0bd1189c 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_driver_register(struct mei_bus_driver *driver);
+void mei_driver_unregister(struct mei_bus_driver *driver);
+
 #endif /* _LINUX_MEI_BUS_H */
-- 
1.7.4.4


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

* [char-misc-next 03/11 V2] mei: bus: Initial implementation for I/O routines
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 02/11 V2] mei: bus: Implement driver registration Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 04/11 V2] mei: bus: Add bus related structures to mei_cl Tomas Winkler
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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 |   15 +++
 include/linux/mei_bus.h    |   11 ++
 4 files changed, 255 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 0a5e624..97e1988 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_driver_unregister(struct mei_bus_driver *driver)
 	pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
 }
 EXPORT_SYMBOL(mei_driver_unregister);
+
+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->send)
+		return client->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->recv)
+		return client->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..e95d6e1 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -273,6 +273,13 @@ struct mei_hw_ops {
  * @cl: mei client
  * @driver: mei bus driver for this client
  * @dev: linux driver model device pointer
+ * @send: Tx hook for the bus client. This allows clients to trap
+ *	the driver buffers before actually pushing it to the ME.
+ * @recv: Rx hook for the bus client. This allows clients to trap the
+ *	ME buffers before forwarding them to the driver.
+ * @event_cb: Drivers register this callback to get asynchronous ME
+	events (e.g. Rx buffer pending) notifications.
+ * @events: Events bitmask sent to the driver.
  * @priv_data: client private data
  */
 struct mei_bus_client {
@@ -284,6 +291,14 @@ struct mei_bus_client {
 	struct mei_bus_driver *driver;
 	struct device dev;
 
+	int (*send)(struct mei_bus_client *client, u8 *buf, size_t length);
+	int (*recv)(struct mei_bus_client *client, u8 *buf, size_t length);
+
+	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 0bd1189c..738e073 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_driver_register(struct mei_bus_driver *driver);
 void mei_driver_unregister(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] 25+ messages in thread

* [char-misc-next 04/11 V2] mei: bus: Add bus related structures to mei_cl
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (2 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 03/11 V2] mei: bus: Initial implementation for I/O routines Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 05/11 V2] mei: bus: Call bus routines from the core code Tomas Winkler
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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 97e1988..2e12928 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->send)
 		return client->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->recv)
 		return client->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 e95d6e1..4e1daf2 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
@@ -404,6 +409,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] 25+ messages in thread

* [char-misc-next 05/11 V2] mei: bus: Call bus routines from the core code
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (3 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 04/11 V2] mei: bus: Add bus related structures to mei_cl Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 13:34   ` Arnd Bergmann
  2013-02-08 12:28 ` [char-misc-next 06/11 V2] mei: bus: Synchronous API for the data transmission Tomas Winkler
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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    |   21 +++++++++++++++++++--
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 2e12928..1048cd4 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 __init mei_bus_init(struct pci_dev *pdev)
+{
+	return bus_register(&mei_bus_type);
+}
+
+void __exit 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 4e1daf2..a9419a8 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -421,6 +421,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..5daaa0c 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -197,7 +197,6 @@ static int mei_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	mei_pdev = pdev;
 	pci_set_drvdata(pdev, dev);
 
-
 	schedule_delayed_work(&dev->timer_work, HZ);
 
 	mutex_unlock(&mei_mutex);
@@ -389,7 +388,25 @@ static struct pci_driver mei_driver = {
 	.driver.pm = MEI_PM_OPS,
 };
 
-module_pci_driver(mei_driver);
+static int __init mei_init(void)
+{
+	int err;
+
+	err = mei_bus_init(mei_pdev);
+	if (err)
+		return err;
+
+	return pci_register_driver(&mei_driver);
+}
+
+static void __exit mei_exit(void)
+{
+	pci_unregister_driver(&mei_driver);
+	mei_bus_exit();
+}
+
+module_init(mei_init);
+module_exit(mei_exit);
 
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Intel(R) Management Engine Interface");
-- 
1.7.4.4


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

* [char-misc-next 06/11 V2] mei: bus: Synchronous API for the data transmission
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (4 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 05/11 V2] mei: bus: Call bus routines from the core code Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 07/11 V2] mei: bus: Implement bus driver data setter/getter Tomas Winkler
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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 1048cd4..5a700cd 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -210,7 +210,7 @@ void mei_driver_unregister(struct mei_bus_driver *driver)
 }
 EXPORT_SYMBOL(mei_driver_unregister);
 
-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] 25+ messages in thread

* [char-misc-next 07/11 V2] mei: bus: Implement bus driver data setter/getter
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (5 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 06/11 V2] mei: bus: Synchronous API for the data transmission Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 08/11 V2] mei: nfc: Initial nfc implementation Tomas Winkler
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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 5a700cd..a787e48 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);
 
+void *mei_bus_get_clientdata(const struct mei_bus_client *client)
+{
+	return dev_get_drvdata(&client->dev);
+}
+EXPORT_SYMBOL(mei_bus_get_clientdata);
+
+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 738e073..912c84e 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] 25+ messages in thread

* [char-misc-next 08/11 V2] mei: nfc: Initial nfc implementation
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (6 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 07/11 V2] mei: bus: Implement bus driver data setter/getter Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 09/11 V2] mei: nfc: Connect also the regular ME client Tomas Winkler
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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 a9419a8..39ddfe9 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -476,6 +476,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 5daaa0c..96c13c4 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -277,6 +277,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] 25+ messages in thread

* [char-misc-next 09/11 V2] mei: nfc: Connect also the regular ME client
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (7 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 08/11 V2] mei: nfc: Initial nfc implementation Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops Tomas Winkler
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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] 25+ messages in thread

* [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (8 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 09/11 V2] mei: nfc: Connect also the regular ME client Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  2013-02-08 12:28 ` [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops Tomas Winkler
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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] 25+ messages in thread

* [char-misc-next 11/11] mei: nfc: Implement MEI bus IO ops
  2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
                   ` (9 preceding siblings ...)
  2013-02-08 12:28 ` [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus Tomas Winkler
@ 2013-02-08 12:28 ` Tomas Winkler
  10 siblings, 0 replies; 25+ messages in thread
From: Tomas Winkler @ 2013-02-08 12:28 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 |   72 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/mei/nfc.h |   13 ++++++++
 2 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index c65bd44..86f7e47 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,69 @@ 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 void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
@@ -295,6 +363,8 @@ static void mei_nfc_init(struct work_struct *work)
 	}
 
 	bus_client->priv_data = bdev;
+	bus_client->send = mei_nfc_send;
+	bus_client->recv = mei_nfc_recv;
 
 	return;
 
@@ -361,8 +431,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] 25+ messages in thread

* Re: [char-misc-next 05/11 V2] mei: bus: Call bus routines from the core code
  2013-02-08 12:28 ` [char-misc-next 05/11 V2] mei: bus: Call bus routines from the core code Tomas Winkler
@ 2013-02-08 13:34   ` Arnd Bergmann
  0 siblings, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2013-02-08 13:34 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Friday 08 February 2013 14:28:18 Tomas Winkler wrote:
> 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>

Acked-by: Arnd Bergmann <arnd@arndb.de>

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

* Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
  2013-02-08 12:28 ` [char-misc-next 02/11 V2] mei: bus: Implement driver registration Tomas Winkler
@ 2013-02-08 13:36   ` Arnd Bergmann
  2013-02-08 23:55   ` Greg KH
  1 sibling, 0 replies; 25+ messages in thread
From: Arnd Bergmann @ 2013-02-08 13:36 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: gregkh, sameo, linux-kernel

On Friday 08 February 2013 14:28:15 Tomas Winkler wrote:
> 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>

Acked-by: Arnd Bergmann <arnd@arndb.de>

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-08 12:28 ` [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation Tomas Winkler
@ 2013-02-08 23:53   ` Greg KH
  2013-02-10  3:25     ` Samuel Ortiz
  0 siblings, 1 reply; 25+ messages in thread
From: Greg KH @ 2013-02-08 23:53 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: sameo, arnd, linux-kernel

On Fri, Feb 08, 2013 at 02:28:14PM +0200, 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.
> 
> 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,
> +		  },

Can't you put a name field in your mei_bus_driver structure and then
copy it to the version in the driver model?  That's what other bus
drivers do and it makes more sense.

> +	.id = {
> +		.name = CONTACT_DRIVER_NAME,
> +		.uuid = NFC_UUID,
> +	},

Drivers usually only have a pointer to a list of device ids, specific to
their bus type.  Don't force them to be listed in this manner, otherwise
you can not do automatic module loading (through the MODULE_DEVICE()
macro.)


> +
> +	.probe = contact_probe,
> +	.remove = contact_remove,
> +};
> +
> +static int contact_init(void)
> +{
> +	int r;
> +
> +	pr_debug(DRIVER_DESC ": %s\n", __func__);

Don't encourage people to write "noisy" kernel modules please, this
doesn't need to be here.

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

Please pass back the id that the device is being matched on, to give the
driver a chance to do something with any specific values it needs to.

Again, like other busses (PCI and USB).

> --- /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"

Why create bus.h?  Why not just put it all in mei_dev.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;
> +}

If you don't do anything here, no need to provide it at all.

> +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);

Why is this exported?

And please, EXPORT_SYMBOL_GPL() for new functions that are busses using
the driver model.

> +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,
> +};

Thank you for getting this right, it makes me happy.

> +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);

EXPORT_SYMBOL_GPL() please.

> +void mei_remove_device(struct mei_bus_client *client)
> +{
> +	device_unregister(&client->dev);
> +}
> +EXPORT_SYMBOL(mei_remove_device);

_GPL() please.


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

Why are these in this file and not just in bus.c?

> +
> +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);

These should go in mei_dev.h


> +
> +#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

I don't really understand this structure, please explain it better.

> + *
> + * @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];

This isn't needed, use the struct device name instead.

> +	struct mei_device *mei_dev;

What is this for?

> +	struct mei_cl *cl;
> +	struct mei_bus_driver *driver;

Why is this needed?

> +	struct device dev;
> +
> +	void *priv_data;

Why is this needed?  What's wrong with the one build into 'struct
device'?

> +struct mei_bus_driver {
> +	struct device_driver driver;

Add a:
	char *name;
field here please.

> +
> +	struct mei_id id;

This should be a list of ids, NULL terminated.

thanks,

greg k-h

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

* Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
  2013-02-08 12:28 ` [char-misc-next 02/11 V2] mei: bus: Implement driver registration Tomas Winkler
  2013-02-08 13:36   ` Arnd Bergmann
@ 2013-02-08 23:55   ` Greg KH
  2013-02-10  3:32     ` Samuel Ortiz
  1 sibling, 1 reply; 25+ messages in thread
From: Greg KH @ 2013-02-08 23:55 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: sameo, arnd, linux-kernel

On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> 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>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  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..0a5e624 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_driver_register(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;

No, you really don't know what 'p' is, so don't assume you do.  You have
fields you can test to see if your driver model is up and registered,
use them instead.  Don't you think that something called 'p' is not
there for you to use?

> +	driver->driver.owner = THIS_MODULE;

Nope, this should be the module that owns the driver, not this one.  It
needs to be passed in with the call, or as part of the driver structure.

> +	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_driver_register);

_GPL() please?

> +
> +void mei_driver_unregister(struct mei_bus_driver *driver)
> +{
> +	driver_unregister(&driver->driver);
> +
> +	pr_debug("mei: driver [%s] unregistered\n", driver->driver.name);
> +}
> +EXPORT_SYMBOL(mei_driver_unregister);

Same here.

thanks,

greg k-h

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-08 23:53   ` Greg KH
@ 2013-02-10  3:25     ` Samuel Ortiz
  2013-02-11 11:50       ` Arnd Bergmann
  0 siblings, 1 reply; 25+ messages in thread
From: Samuel Ortiz @ 2013-02-10  3:25 UTC (permalink / raw)
  To: Greg KH; +Cc: Tomas Winkler, arnd, linux-kernel

Hi Greg,

On Fri, Feb 08, 2013 at 03:53:41PM -0800, Greg KH wrote:
> > +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,
> > +		  },
> 
> Can't you put a name field in your mei_bus_driver structure and then
> copy it to the version in the driver model?  That's what other bus
> drivers do and it makes more sense.
Sure.

 
> > +	.id = {
> > +		.name = CONTACT_DRIVER_NAME,
> > +		.uuid = NFC_UUID,
> > +	},
> 
> Drivers usually only have a pointer to a list of device ids, specific to
> their bus type.  Don't force them to be listed in this manner, otherwise
> you can not do automatic module loading (through the MODULE_DEVICE()
> macro.)
I'll do that, yes.


> 
> > +
> > +	.probe = contact_probe,
> > +	.remove = contact_remove,
> > +};
> > +
> > +static int contact_init(void)
> > +{
> > +	int r;
> > +
> > +	pr_debug(DRIVER_DESC ": %s\n", __func__);
> 
> Don't encourage people to write "noisy" kernel modules please, this
> doesn't need to be here.
Ok, will fix.


> > +
> > +	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)
> 
> Please pass back the id that the device is being matched on, to give the
> driver a chance to do something with any specific values it needs to.
> 
> Again, like other busses (PCI and USB).
Makes sense, I'll do that.


> > --- /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"
> 
> Why create bus.h?  Why not just put it all in mei_dev.h?
I was trying not to pollute mei_dev.h too much, but I'll rm bus.h and put the
whole bus API into mei_dev.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;
> > +}
> 
> If you don't do anything here, no need to provide it at all.
I'll remove it.

 
> > +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);
> 
> Why is this exported?
It shouldn't, I'll fix that.


> And please, EXPORT_SYMBOL_GPL() for new functions that are busses using
> the driver model.
Sure.


> > +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,
> > +};
> 
> Thank you for getting this right, it makes me happy.
> 
> > +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);
> 
> EXPORT_SYMBOL_GPL() please.
> 
> > +void mei_remove_device(struct mei_bus_client *client)
> > +{
> > +	device_unregister(&client->dev);
> > +}
> > +EXPORT_SYMBOL(mei_remove_device);
> 
> _GPL() please.
> 
> 
> > --- /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)
> 
> Why are these in this file and not just in bus.c?
Yep, I'll put them there.


> > +
> > +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);
> 
> These should go in mei_dev.h
> 
> 
> > +
> > +#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
> 
> I don't really understand this structure, please explain it better.
This is a structure that links the MEI bus client pointer passed to the driver
with the actual ME client. It also allows the ME driver to implement
technology specific ME protocol through the send/recv hooks.

 
> > + *
> > + * @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];
> 
> This isn't needed, use the struct device name instead.
Yes, will fix.

> > +	struct mei_device *mei_dev;
> 
> What is this for?
Not needed, I'll remove it.


> > +	struct mei_cl *cl;
> > +	struct mei_bus_driver *driver;
> 
> Why is this needed?
It shouldn't be there, I'll remove it as well.


> > +	struct device dev;
> > +
> > +	void *priv_data;
> 
> Why is this needed?  What's wrong with the one build into 'struct
> device'?
The latter is used by drivers registered on the MEI bus through the
mei_bus_ge/sett_clientdata routines. The former is for technology specific
parts of the ME driver to retrieve their private data.
For example the NFC specific part of the ME driver will get IO requests from
mei_bus_client pointers. The priv_data field from these pointers will give
this part of the code their private structure back.



> > +struct mei_bus_driver {
> > +	struct device_driver driver;
> 
> Add a:
> 	char *name;
> field here please.
Yes, I'll ad it.


> > +
> > +	struct mei_id id;
> 
> This should be a list of ids, NULL terminated.
Yes, will do as well.

Thanks a lot for the comments, I'll send a v3 soon.

Cheers,
Samuel.

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

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

* Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
  2013-02-08 23:55   ` Greg KH
@ 2013-02-10  3:32     ` Samuel Ortiz
  2013-02-10 16:36       ` Greg KH
  0 siblings, 1 reply; 25+ messages in thread
From: Samuel Ortiz @ 2013-02-10  3:32 UTC (permalink / raw)
  To: Greg KH; +Cc: Tomas Winkler, arnd, linux-kernel

Hi Greg,

On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
> On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> > 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>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  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..0a5e624 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_driver_register(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;
> 
> No, you really don't know what 'p' is, so don't assume you do.  You have
> fields you can test to see if your driver model is up and registered,
> use them instead.  
Unless I'm missing something, I would actually need to track if the bus type
is registered or not from bus.c. I can't find any field from bus_type that
tells me for sure if my bus is registered or not, except for p.
I actually think I don't need that check anyway. IIRC, this is something I
took from i2c-core.c but in the ME case it's most likely not needed.


> > +	driver->driver.owner = THIS_MODULE;
> 
> Nope, this should be the module that owns the driver, not this one.  It
> needs to be passed in with the call, or as part of the driver structure.
That's right, I'll fix that.

Cheers,
Samuel.

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

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

* Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
  2013-02-10  3:32     ` Samuel Ortiz
@ 2013-02-10 16:36       ` Greg KH
  2013-02-11 11:03         ` Samuel Ortiz
  0 siblings, 1 reply; 25+ messages in thread
From: Greg KH @ 2013-02-10 16:36 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Tomas Winkler, arnd, linux-kernel

On Sun, Feb 10, 2013 at 04:32:18AM +0100, Samuel Ortiz wrote:
> Hi Greg,
> 
> On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
> > On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> > > 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>
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  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..0a5e624 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_driver_register(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;
> > 
> > No, you really don't know what 'p' is, so don't assume you do.  You have
> > fields you can test to see if your driver model is up and registered,
> > use them instead.  
> Unless I'm missing something, I would actually need to track if the bus type
> is registered or not from bus.c. I can't find any field from bus_type that
> tells me for sure if my bus is registered or not, except for p.
> I actually think I don't need that check anyway. IIRC, this is something I
> took from i2c-core.c but in the ME case it's most likely not needed.

It's your bus, if you need to check this, then have your own flag, don't
poke in the driver core internals.  As you point out, I don't think you
need to check this at all.

greg k-h

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

* Re: [char-misc-next 02/11 V2] mei: bus: Implement driver registration
  2013-02-10 16:36       ` Greg KH
@ 2013-02-11 11:03         ` Samuel Ortiz
  0 siblings, 0 replies; 25+ messages in thread
From: Samuel Ortiz @ 2013-02-11 11:03 UTC (permalink / raw)
  To: Greg KH; +Cc: Tomas Winkler, arnd, linux-kernel

On Sun, Feb 10, 2013 at 08:36:23AM -0800, Greg KH wrote:
> On Sun, Feb 10, 2013 at 04:32:18AM +0100, Samuel Ortiz wrote:
> > Hi Greg,
> > 
> > On Fri, Feb 08, 2013 at 03:55:24PM -0800, Greg KH wrote:
> > > On Fri, Feb 08, 2013 at 02:28:15PM +0200, Tomas Winkler wrote:
> > > > 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>
> > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > >  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..0a5e624 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_driver_register(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;
> > > 
> > > No, you really don't know what 'p' is, so don't assume you do.  You have
> > > fields you can test to see if your driver model is up and registered,
> > > use them instead.  
> > Unless I'm missing something, I would actually need to track if the bus type
> > is registered or not from bus.c. I can't find any field from bus_type that
> > tells me for sure if my bus is registered or not, except for p.
> > I actually think I don't need that check anyway. IIRC, this is something I
> > took from i2c-core.c but in the ME case it's most likely not needed.
> 
> It's your bus, if you need to check this, then have your own flag, don't
> poke in the driver core internals.  As you point out, I don't think you
> need to check this at all.
Yes, I removed the check. Tomas should sent v3 later today with all your comments
adressed.
Thanks for the review.

Cheers,
Samuel.

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

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-10  3:25     ` Samuel Ortiz
@ 2013-02-11 11:50       ` Arnd Bergmann
  2013-02-11 13:46         ` Samuel Ortiz
  0 siblings, 1 reply; 25+ messages in thread
From: Arnd Bergmann @ 2013-02-11 11:50 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Greg KH, Tomas Winkler, linux-kernel

On Sunday 10 February 2013, Samuel Ortiz wrote:
> > >  
> > >  /**
> > > + * mei_bus_client
> > 
> > I don't really understand this structure, please explain it better.
> This is a structure that links the MEI bus client pointer passed to the driver
> with the actual ME client. It also allows the ME driver to implement
> technology specific ME protocol through the send/recv hooks.

I think part of the confusion is that this is what in other subsystems
is called a device, not a client. I believe I'm still confused in the
same way that Greg is.

You already have a 'struct mei_device', which refers to the PCI device
that owns the bus, and has clients attached to it. While it may be
a little confusing to people that already worked with the current
mei code, I think it would help to rename the existing 'mei_device'
to 'mei_host' or something else that feels appropriate, and introduce
the new structure as 'mei_device' derived from 'struct device', again
matching what most other subsystems do.

Similarly, you can then rename 'mei_bus_driver' to 'mei_driver' to fit
that logic, since I would consider a 'bus_driver' to be something
that is responsible for the entire bus, not just for one device.

	Arnd

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-11 11:50       ` Arnd Bergmann
@ 2013-02-11 13:46         ` Samuel Ortiz
  2013-02-11 14:30           ` Greg KH
  0 siblings, 1 reply; 25+ messages in thread
From: Samuel Ortiz @ 2013-02-11 13:46 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Greg KH, Tomas Winkler, linux-kernel

Hi Arnd,

On Mon, Feb 11, 2013 at 11:50:26AM +0000, Arnd Bergmann wrote:
> On Sunday 10 February 2013, Samuel Ortiz wrote:
> > > >  
> > > >  /**
> > > > + * mei_bus_client
> > > 
> > > I don't really understand this structure, please explain it better.
> > This is a structure that links the MEI bus client pointer passed to the driver
> > with the actual ME client. It also allows the ME driver to implement
> > technology specific ME protocol through the send/recv hooks.
> 
> I think part of the confusion is that this is what in other subsystems
> is called a device, not a client. I believe I'm still confused in the
> same way that Greg is.
Ok, I understand where the confusion comes from now. Yes, for most of the
other subsystems, this is a device. Initially I tried to keep the MEI bus code
as little intrusive as possible and I didn't want to rename mei_device to
something else.

 
> You already have a 'struct mei_device', which refers to the PCI device
> that owns the bus, and has clients attached to it. While it may be
> a little confusing to people that already worked with the current
> mei code, I think it would help to rename the existing 'mei_device'
> to 'mei_host' or something else that feels appropriate, and introduce
> the new structure as 'mei_device' derived from 'struct device', again
> matching what most other subsystems do.
I understand, and I agree it would make sense. As we're aiming at having this
patchset merged during the next merge window, would it be ok to have this
renaming phase as a follow up patch ?

> Similarly, you can then rename 'mei_bus_driver' to 'mei_driver' to fit
> that logic, since I would consider a 'bus_driver' to be something
> that is responsible for the entire bus, not just for one device.
That would make sense as well, and I can have this done through patchset v4.

Cheers,
Samuel.

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

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-11 13:46         ` Samuel Ortiz
@ 2013-02-11 14:30           ` Greg KH
  2013-02-11 15:58             ` Samuel Ortiz
  0 siblings, 1 reply; 25+ messages in thread
From: Greg KH @ 2013-02-11 14:30 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Arnd Bergmann, Tomas Winkler, linux-kernel

On Mon, Feb 11, 2013 at 02:46:12PM +0100, Samuel Ortiz wrote:
> > You already have a 'struct mei_device', which refers to the PCI device
> > that owns the bus, and has clients attached to it. While it may be
> > a little confusing to people that already worked with the current
> > mei code, I think it would help to rename the existing 'mei_device'
> > to 'mei_host' or something else that feels appropriate, and introduce
> > the new structure as 'mei_device' derived from 'struct device', again
> > matching what most other subsystems do.
> I understand, and I agree it would make sense. As we're aiming at having this
> patchset merged during the next merge window, would it be ok to have this
> renaming phase as a follow up patch ?

Do you mean for 3.9?  Sorry, my trees are now closed for new stuff like
this for 3.9, this will all have to wait for 3.10, so you have plenty of
time to get it right.  No need to rush.

thanks,

greg k-h

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-11 14:30           ` Greg KH
@ 2013-02-11 15:58             ` Samuel Ortiz
  2013-02-11 16:05               ` Greg KH
  0 siblings, 1 reply; 25+ messages in thread
From: Samuel Ortiz @ 2013-02-11 15:58 UTC (permalink / raw)
  To: Greg KH; +Cc: Arnd Bergmann, Tomas Winkler, linux-kernel

On Mon, Feb 11, 2013 at 06:30:51AM -0800, Greg KH wrote:
> On Mon, Feb 11, 2013 at 02:46:12PM +0100, Samuel Ortiz wrote:
> > > You already have a 'struct mei_device', which refers to the PCI device
> > > that owns the bus, and has clients attached to it. While it may be
> > > a little confusing to people that already worked with the current
> > > mei code, I think it would help to rename the existing 'mei_device'
> > > to 'mei_host' or something else that feels appropriate, and introduce
> > > the new structure as 'mei_device' derived from 'struct device', again
> > > matching what most other subsystems do.
> > I understand, and I agree it would make sense. As we're aiming at having this
> > patchset merged during the next merge window, would it be ok to have this
> > renaming phase as a follow up patch ?
> 
> Do you mean for 3.9?  
That's what I meant, yes.

> Sorry, my trees are now closed for new stuff like this for 3.9, 
Out of curiosity, when do you close your trees for new stuff ? Is there a
fixed timeframe ?

Cheers,
Samuel.

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

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

* Re: [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation
  2013-02-11 15:58             ` Samuel Ortiz
@ 2013-02-11 16:05               ` Greg KH
  0 siblings, 0 replies; 25+ messages in thread
From: Greg KH @ 2013-02-11 16:05 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Arnd Bergmann, Tomas Winkler, linux-kernel

On Mon, Feb 11, 2013 at 04:58:54PM +0100, Samuel Ortiz wrote:
> On Mon, Feb 11, 2013 at 06:30:51AM -0800, Greg KH wrote:
> > On Mon, Feb 11, 2013 at 02:46:12PM +0100, Samuel Ortiz wrote:
> > > > You already have a 'struct mei_device', which refers to the PCI device
> > > > that owns the bus, and has clients attached to it. While it may be
> > > > a little confusing to people that already worked with the current
> > > > mei code, I think it would help to rename the existing 'mei_device'
> > > > to 'mei_host' or something else that feels appropriate, and introduce
> > > > the new structure as 'mei_device' derived from 'struct device', again
> > > > matching what most other subsystems do.
> > > I understand, and I agree it would make sense. As we're aiming at having this
> > > patchset merged during the next merge window, would it be ok to have this
> > > renaming phase as a follow up patch ?
> > 
> > Do you mean for 3.9?  
> That's what I meant, yes.
> 
> > Sorry, my trees are now closed for new stuff like this for 3.9, 
> Out of curiosity, when do you close your trees for new stuff ? Is there a
> fixed timeframe ?

Normally around the -rc7 or -rc8 timeframe.

thanks,

greg k-h

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

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

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-08 12:28 [char-misc-next 00/11 V2] Add MEI BUS and NFC Device Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 01/11 V2] mei: bus: Initial MEI bus type implementation Tomas Winkler
2013-02-08 23:53   ` Greg KH
2013-02-10  3:25     ` Samuel Ortiz
2013-02-11 11:50       ` Arnd Bergmann
2013-02-11 13:46         ` Samuel Ortiz
2013-02-11 14:30           ` Greg KH
2013-02-11 15:58             ` Samuel Ortiz
2013-02-11 16:05               ` Greg KH
2013-02-08 12:28 ` [char-misc-next 02/11 V2] mei: bus: Implement driver registration Tomas Winkler
2013-02-08 13:36   ` Arnd Bergmann
2013-02-08 23:55   ` Greg KH
2013-02-10  3:32     ` Samuel Ortiz
2013-02-10 16:36       ` Greg KH
2013-02-11 11:03         ` Samuel Ortiz
2013-02-08 12:28 ` [char-misc-next 03/11 V2] mei: bus: Initial implementation for I/O routines Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 04/11 V2] mei: bus: Add bus related structures to mei_cl Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 05/11 V2] mei: bus: Call bus routines from the core code Tomas Winkler
2013-02-08 13:34   ` Arnd Bergmann
2013-02-08 12:28 ` [char-misc-next 06/11 V2] mei: bus: Synchronous API for the data transmission Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 07/11 V2] mei: bus: Implement bus driver data setter/getter Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 08/11 V2] mei: nfc: Initial nfc implementation Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 09/11 V2] mei: nfc: Connect also the regular ME client Tomas Winkler
2013-02-08 12:28 ` [char-misc-next 10/11] mei: nfc: Add NFC device to the MEI bus Tomas Winkler
2013-02-08 12:28 ` [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).