[char-misc-next 0/3 V6] Support NFC Device on MEI CL Bus

[char-misc-next 0/3 V6] Support NFC Device on MEI CL Bus

[Tomas Winkler]

v5 -> v6
 1. include/linux/uapi/mei/nfc.h - provides API also for pure
    user space implementation as found under Android.
 2. Removed INTEL_MEI_BUS_NFC Kconfig option. 
    The NFC info client is disconnected as soon as we
    get the FW info and the regular client is connected only when
    mei_cl_enable_device() is explicitly called from an nfc driver.
    The mei cl bus now doesn't connect NFC  exclusively.
 3. Added pn544 to the possibly detected NFC chipsets.

Depends on: 
	mei: bus: Add device enabling and disabling API

Samuel Ortiz (3):
  mei: nfc: Initial nfc implementation
  mei: nfc: Add NFC device to the MEI bus
  mei: nfc: Implement MEI bus ops

 drivers/misc/mei/Makefile     |   1 +
 drivers/misc/mei/client.c     |   3 +
 drivers/misc/mei/init.c       |   2 +
 drivers/misc/mei/mei_dev.h    |  10 +
 drivers/misc/mei/nfc.c        | 493 ++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/Kbuild     |   1 +
 include/uapi/linux/mei/Kbuild |   2 +
 include/uapi/linux/mei/nfc.h  | 135 ++++++++++++
 8 files changed, 647 insertions(+)
 create mode 100644 drivers/misc/mei/nfc.c
 create mode 100644 include/uapi/linux/mei/Kbuild
 create mode 100644 include/uapi/linux/mei/nfc.h

-- 
1.8.1.2

[char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Tomas Winkler]

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

NFC ME device is exported through the 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 to properly build the ME id we first need to retrieve
the firmware information from the info client and then disconnect from it.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/Makefile     |   1 +
 drivers/misc/mei/client.c     |   3 +
 drivers/misc/mei/init.c       |   2 +
 drivers/misc/mei/mei_dev.h    |  10 ++
 drivers/misc/mei/nfc.c        | 250 ++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/Kbuild     |   1 +
 include/uapi/linux/mei/Kbuild |   2 +
 include/uapi/linux/mei/nfc.h  | 135 +++++++++++++++++++++++
 8 files changed, 404 insertions(+)
 create mode 100644 drivers/misc/mei/nfc.c
 create mode 100644 include/uapi/linux/mei/Kbuild
 create mode 100644 include/uapi/linux/mei/nfc.h

diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 3612d57..08698a4 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -11,6 +11,7 @@ mei-objs += main.o
 mei-objs += amthif.o
 mei-objs += wd.o
 mei-objs += bus.o
+mei-objs += nfc.o
 mei-$(CONFIG_DEBUG_FS) += debugfs.o
 
 obj-$(CONFIG_INTEL_MEI_ME) += mei-me.o
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index ecadd00..9541aa9 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/init.c b/drivers/misc/mei/init.c
index 54b51c0..4e102ad 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -219,6 +219,8 @@ void mei_stop(struct mei_device *dev)
 
 	mei_wd_stop(dev);
 
+	mei_nfc_host_exit();
+
 	dev->dev_state = MEI_DEV_POWER_DOWN;
 	mei_reset(dev, 0);
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index c02967d..cd5b6d4 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -509,6 +509,16 @@ struct mei_cl_cb *mei_amthif_find_read_list_entry(struct mei_device *dev,
 
 void mei_amthif_run_next_cmd(struct mei_device *dev);
 
+/*
+ * 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;
 
 int mei_amthif_irq_write_complete(struct mei_device *dev, s32 *slots,
 			struct mei_cl_cb *cb, struct mei_cl_cb *cmpl_list);
diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
new file mode 100644
index 0000000..29a4e03
--- /dev/null
+++ b/drivers/misc/mei/nfc.c
@@ -0,0 +1,250 @@
+/*
+ *
+ * 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/nfc.h>
+#include <linux/mei_cl_bus.h>
+
+#include "mei_dev.h"
+#include "client.h"
+
+/** mei_nfc_dev - NFC mei device
+ *
+ * @cl: NFC host client
+ * @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_nfc_dev {
+	struct mei_cl *cl;
+	struct mei_cl *cl_info;
+	struct work_struct init_work;
+	u8 fw_ivn;
+	u8 vendor_id;
+	u8 radio_type;
+};
+
+static struct mei_nfc_dev nfc_dev;
+
+/* UUIDs for NFC F/W clients */
+const uuid_le mei_nfc_guid = UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50,
+				     0x94, 0xd4, 0x50, 0x26,
+				     0x67, 0x23, 0x77, 0x5c);
+
+static const uuid_le mei_nfc_info_guid = UUID_LE(0xd2de1625, 0x382d, 0x417d,
+					0x48, 0xa4, 0xef, 0xab,
+					0xba, 0x8a, 0x12, 0x06);
+
+static void mei_nfc_free(struct mei_nfc_dev *ndev)
+{
+	if (ndev->cl) {
+		list_del(&ndev->cl->device_link);
+		mei_cl_unlink(ndev->cl);
+		kfree(ndev->cl);
+	}
+
+	if (ndev->cl_info) {
+		list_del(&ndev->cl_info->device_link);
+		mei_cl_unlink(ndev->cl_info);
+		kfree(ndev->cl_info);
+	}
+}
+
+static int mei_nfc_if_version(struct mei_nfc_dev *ndev)
+{
+	struct mei_device *dev;
+	struct mei_cl *cl;
+
+	struct mei_nfc_cmd cmd;
+	struct mei_nfc_reply *reply = NULL;
+	struct mei_nfc_if_version *version;
+	size_t if_version_length;
+	int bytes_recv, ret;
+
+	cl = ndev->cl_info;
+	dev = cl->dev;
+
+	memset(&cmd, 0, sizeof(struct mei_nfc_cmd));
+	cmd.command = MEI_NFC_CMD_MAINTENANCE;
+	cmd.data_size = 1;
+	cmd.sub_command = MEI_NFC_SUBCMD_IF_VERSION;
+
+	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(struct mei_nfc_cmd));
+	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_cl_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;
+
+	ndev->fw_ivn = version->fw_ivn;
+	ndev->vendor_id = version->vendor_id;
+	ndev->radio_type = version->radio_type;
+
+err:
+	kfree(reply);
+	return ret;
+}
+
+static void mei_nfc_init(struct work_struct *work)
+{
+	struct mei_device *dev;
+	struct mei_nfc_dev *ndev;
+	struct mei_cl *cl_info;
+
+	ndev = container_of(work, struct mei_nfc_dev, init_work);
+
+	cl_info = ndev->cl_info;
+	dev = cl_info->dev;
+
+	mutex_lock(&dev->device_lock);
+
+	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);
+
+	if (mei_nfc_if_version(ndev) < 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",
+		ndev->fw_ivn, ndev->vendor_id, ndev->radio_type);
+
+	mutex_lock(&dev->device_lock);
+
+	if (mei_cl_disconnect(cl_info) < 0) {
+		mutex_unlock(&dev->device_lock);
+		dev_err(&dev->pdev->dev,
+			"Could not disconnect the NFC INFO ME client");
+
+		goto err;
+	}
+
+	mutex_unlock(&dev->device_lock);
+
+	return;
+
+err:
+	mei_nfc_free(ndev);
+
+	return;
+}
+
+
+int mei_nfc_host_init(struct mei_device *dev)
+{
+	struct mei_nfc_dev *ndev = &nfc_dev;
+	struct mei_cl *cl_info, *cl = NULL;
+	int i, ret;
+
+	/* already initialzed */
+	if (ndev->cl_info)
+		return 0;
+
+	cl_info = mei_cl_allocate(dev);
+	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");
+		ret = -ENOENT;
+		goto err;
+	}
+
+	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->device_uuid = mei_nfc_info_guid;
+
+	list_add_tail(&cl_info->device_link, &dev->device_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->device_uuid = mei_nfc_guid;
+
+	list_add_tail(&cl->device_link, &dev->device_list);
+
+	ndev->cl_info = cl_info;
+	ndev->cl = cl;
+
+	INIT_WORK(&ndev->init_work, mei_nfc_init);
+	schedule_work(&ndev->init_work);
+
+	return 0;
+
+err:
+	mei_nfc_free(ndev);
+
+	return ret;
+}
+
+void mei_nfc_host_exit(void)
+{
+	struct mei_nfc_dev *ndev = &nfc_dev;
+
+	mei_nfc_free(ndev);
+}
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 5c8a1d2..d329d63 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -6,6 +6,7 @@ header-y += dvb/
 header-y += hdlc/
 header-y += hsi/
 header-y += isdn/
+header-y += mei/
 header-y += mmc/
 header-y += nfsd/
 header-y += raid/
diff --git a/include/uapi/linux/mei/Kbuild b/include/uapi/linux/mei/Kbuild
new file mode 100644
index 0000000..a20a693
--- /dev/null
+++ b/include/uapi/linux/mei/Kbuild
@@ -0,0 +1,2 @@
+# UAPI Header export list
+header-y += nfc.h
diff --git a/include/uapi/linux/mei/nfc.h b/include/uapi/linux/mei/nfc.h
new file mode 100644
index 0000000..fe5d8aa
--- /dev/null
+++ b/include/uapi/linux/mei/nfc.h
@@ -0,0 +1,135 @@
+/******************************************************************************
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Intel MEI Interface NFC 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 - 2013 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_NFC_H
+#define _LINUX_MEI_NFC_H
+
+/* NFC commands and structures */
+
+struct mei_nfc_cmd {
+	__u8 command;
+	__u8 status;
+	__u16 req_id;
+	__u32 reserved;
+	__u16 data_size;
+	__u8 sub_command;
+	__u8 data[];
+} __packed;
+
+struct mei_nfc_reply {
+	__u8 command;
+	__u8 status;
+	__u16 req_id;
+	__u32 reserved;
+	__u16 data_size;
+	__u8 sub_command;
+	__u8 reply_status;
+	__u8 data[];
+} __packed;
+
+struct mei_nfc_if_version {
+	__u8 radio_version_sw[3];
+	__u8 reserved[3];
+	__u8 radio_version_hw[3];
+	__u8 i2c_addr;
+	__u8 fw_ivn;
+	__u8 vendor_id;
+	__u8 radio_type;
+} __packed;
+
+struct mei_nfc_connect {
+	__u8 fw_ivn;
+	__u8 vendor_id;
+} __packed;
+
+struct mei_nfc_connect_resp {
+	__u8 fw_ivn;
+	__u8 vendor_id;
+	__u16 me_major;
+	__u16 me_minor;
+	__u16 me_hotfix;
+	__u16 me_build;
+} __packed;
+
+struct mei_nfc_hci_hdr {
+	__u8 cmd;
+	__u8 status;
+	__u16 req_id;
+	__u32 reserved;
+	__u16 data_size;
+} __packed;
+
+#define MEI_NFC_CMD_MAINTENANCE 0x00
+#define MEI_NFC_CMD_HCI_SEND 0x01
+#define MEI_NFC_CMD_HCI_RECV 0x02
+
+#define MEI_NFC_SUBCMD_CONNECT    0x00
+#define MEI_NFC_SUBCMD_IF_VERSION 0x01
+
+#define MEI_NFC_HEADER_SIZE 10
+#define MEI_NFC_MAX_HCI_PAYLOAD 300
+
+#endif /* _LINUX_MEI_NFC_H  */
-- 
1.8.1.2

[char-misc-next 2/3] mei: nfc: Add NFC device to the MEI bus

[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 | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index 29a4e03..1846568 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -41,6 +41,7 @@ struct mei_nfc_dev {
 	u8 fw_ivn;
 	u8 vendor_id;
 	u8 radio_type;
+	char *bus_name;
 };
 
 static struct mei_nfc_dev nfc_dev;
@@ -54,6 +55,14 @@ static const uuid_le mei_nfc_info_guid = UUID_LE(0xd2de1625, 0x382d, 0x417d,
 					0x48, 0xa4, 0xef, 0xab,
 					0xba, 0x8a, 0x12, 0x06);
 
+/* Vendors */
+#define MEI_NFC_VENDOR_INSIDE 0x00
+#define MEI_NFC_VENDOR_NXP    0x01
+
+/* Radio types */
+#define MEI_NFC_VENDOR_INSIDE_UREAD 0x00
+#define MEI_NFC_VENDOR_NXP_PN544    0x01
+
 static void mei_nfc_free(struct mei_nfc_dev *ndev)
 {
 	if (ndev->cl) {
@@ -69,6 +78,51 @@ static void mei_nfc_free(struct mei_nfc_dev *ndev)
 	}
 }
 
+static int mei_nfc_build_bus_name(struct mei_nfc_dev *ndev)
+{
+	struct mei_device *dev;
+
+	if (!ndev->cl)
+		return -ENODEV;
+
+	dev = ndev->cl->dev;
+
+	switch (ndev->vendor_id) {
+	case MEI_NFC_VENDOR_INSIDE:
+		switch (ndev->radio_type) {
+		case MEI_NFC_VENDOR_INSIDE_UREAD:
+			ndev->bus_name = "microread";
+			return 0;
+
+		default:
+			dev_err(&dev->pdev->dev, "Unknow radio type 0x%x\n",
+				ndev->radio_type);
+
+			return -EINVAL;
+		}
+
+	case MEI_NFC_VENDOR_NXP:
+		switch (ndev->radio_type) {
+		case MEI_NFC_VENDOR_NXP_PN544:
+			ndev->bus_name = "pn544";
+			return 0;
+		default:
+			dev_err(&dev->pdev->dev, "Unknow radio type 0x%x\n",
+				ndev->radio_type);
+
+			return -EINVAL;
+		}
+
+	default:
+		dev_err(&dev->pdev->dev, "Unknow vendor ID 0x%x\n",
+			ndev->vendor_id);
+
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int mei_nfc_if_version(struct mei_nfc_dev *ndev)
 {
 	struct mei_device *dev;
@@ -123,6 +177,7 @@ err:
 static void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
+	struct mei_cl_device *cldev;
 	struct mei_nfc_dev *ndev;
 	struct mei_cl *cl_info;
 
@@ -165,6 +220,23 @@ static void mei_nfc_init(struct work_struct *work)
 
 	mutex_unlock(&dev->device_lock);
 
+	if (mei_nfc_build_bus_name(ndev) < 0) {
+		dev_err(&dev->pdev->dev,
+			"Could not build the bus ID name\n");
+		return;
+	}
+
+	cldev = mei_cl_add_device(dev, mei_nfc_guid, ndev->bus_name, NULL);
+	if (!cldev) {
+		dev_err(&dev->pdev->dev,
+			"Could not add the NFC device to the MEI bus\n");
+
+		goto err;
+	}
+
+	cldev->priv_data = ndev;
+
+
 	return;
 
 err:
@@ -246,5 +318,8 @@ void mei_nfc_host_exit(void)
 {
 	struct mei_nfc_dev *ndev = &nfc_dev;
 
+	if (ndev->cl && ndev->cl->device)
+		mei_cl_remove_device(ndev->cl->device);
+
 	mei_nfc_free(ndev);
 }
-- 
1.8.1.2

[char-misc-next 3/3] mei: nfc: Implement MEI bus ops

[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.
The enable ops sends the NFC HECI connect command.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/nfc.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 169 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index 1846568..046fecb 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>
@@ -38,10 +39,14 @@ struct mei_nfc_dev {
 	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;
 };
 
 static struct mei_nfc_dev nfc_dev;
@@ -123,6 +128,73 @@ static int mei_nfc_build_bus_name(struct mei_nfc_dev *ndev)
 	return 0;
 }
 
+static int mei_nfc_connect(struct mei_nfc_dev *ndev)
+{
+	struct mei_device *dev;
+	struct mei_cl *cl;
+	struct mei_nfc_cmd *cmd, *reply;
+	struct mei_nfc_connect *connect;
+	struct mei_nfc_connect_resp *connect_resp;
+	size_t connect_length, connect_resp_length;
+	int bytes_recv, ret;
+
+	cl = ndev->cl;
+	dev = cl->dev;
+
+	connect_length = sizeof(struct mei_nfc_cmd) +
+			sizeof(struct mei_nfc_connect);
+
+	connect_resp_length = sizeof(struct mei_nfc_cmd) +
+			sizeof(struct mei_nfc_connect_resp);
+
+	cmd = kzalloc(connect_length, GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+	connect = (struct mei_nfc_connect *)cmd->data;
+
+	reply = kzalloc(connect_resp_length, GFP_KERNEL);
+	if (!reply) {
+		kfree(cmd);
+		return -ENOMEM;
+	}
+
+	connect_resp = (struct mei_nfc_connect_resp *)reply->data;
+
+	cmd->command = MEI_NFC_CMD_MAINTENANCE;
+	cmd->data_size = 3;
+	cmd->sub_command = MEI_NFC_SUBCMD_CONNECT;
+	connect->fw_ivn = ndev->fw_ivn;
+	connect->vendor_id = ndev->vendor_id;
+
+	ret = __mei_cl_send(cl, (u8 *)cmd, connect_length);
+	if (ret < 0) {
+		dev_err(&dev->pdev->dev, "Could not send connect cmd\n");
+		goto err;
+	}
+
+	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, connect_resp_length);
+	if (bytes_recv < 0) {
+		dev_err(&dev->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_nfc_dev *ndev)
 {
 	struct mei_device *dev;
@@ -174,6 +246,100 @@ err:
 	return ret;
 }
 
+static int mei_nfc_enable(struct mei_cl_device *cldev)
+{
+	struct mei_device *dev;
+	struct mei_nfc_dev *ndev = &nfc_dev;
+	int ret;
+
+	dev = ndev->cl->dev;
+
+	ret = mei_nfc_connect(ndev);
+	if (ret < 0) {
+		dev_err(&dev->pdev->dev, "Could not connect to NFC");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mei_nfc_disable(struct mei_cl_device *cldev)
+{
+	return 0;
+}
+
+static int mei_nfc_send(struct mei_cl_device *cldev, u8 *buf, size_t length)
+{
+	struct mei_device *dev;
+	struct mei_nfc_dev *ndev;
+	struct mei_nfc_hci_hdr *hdr;
+	u8 *mei_buf;
+	int err;
+
+	ndev = (struct mei_nfc_dev *) cldev->priv_data;
+	dev = ndev->cl->dev;
+
+	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 = ndev->req_id;
+	hdr->reserved = 0;
+	hdr->data_size = length;
+
+	memcpy(mei_buf + MEI_NFC_HEADER_SIZE, buf, length);
+
+	err = __mei_cl_send(ndev->cl, mei_buf, length + MEI_NFC_HEADER_SIZE);
+	if (err < 0)
+		return err;
+
+	kfree(mei_buf);
+
+	if (!wait_event_interruptible_timeout(ndev->send_wq,
+				ndev->recv_req_id == ndev->req_id, HZ)) {
+		dev_err(&dev->pdev->dev, "NFC MEI command timeout\n");
+		err = -ETIMEDOUT;
+	} else {
+		ndev->req_id++;
+	}
+
+	return err;
+}
+
+static int mei_nfc_recv(struct mei_cl_device *cldev, u8 *buf, size_t length)
+{
+	struct mei_nfc_dev *ndev;
+	struct mei_nfc_hci_hdr *hci_hdr;
+	int received_length;
+
+	ndev = (struct mei_nfc_dev *)cldev->priv_data;
+
+	received_length = __mei_cl_recv(ndev->cl, buf, length);
+	if (received_length < 0)
+		return received_length;
+
+	hci_hdr = (struct mei_nfc_hci_hdr *) buf;
+
+	if (hci_hdr->cmd == MEI_NFC_CMD_HCI_SEND) {
+		ndev->recv_req_id = hci_hdr->req_id;
+		wake_up(&ndev->send_wq);
+
+		return 0;
+	}
+
+	return received_length;
+}
+
+static struct mei_cl_ops nfc_ops = {
+	.enable = mei_nfc_enable,
+	.disable = mei_nfc_disable,
+	.send = mei_nfc_send,
+	.recv = mei_nfc_recv,
+};
+
 static void mei_nfc_init(struct work_struct *work)
 {
 	struct mei_device *dev;
@@ -226,7 +392,7 @@ static void mei_nfc_init(struct work_struct *work)
 		return;
 	}
 
-	cldev = mei_cl_add_device(dev, mei_nfc_guid, ndev->bus_name, NULL);
+	cldev = mei_cl_add_device(dev, mei_nfc_guid, ndev->bus_name, &nfc_ops);
 	if (!cldev) {
 		dev_err(&dev->pdev->dev,
 			"Could not add the NFC device to the MEI bus\n");
@@ -302,8 +468,10 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	ndev->cl_info = cl_info;
 	ndev->cl = cl;
+	ndev->req_id = 1;
 
 	INIT_WORK(&ndev->init_work, mei_nfc_init);
+	init_waitqueue_head(&ndev->send_wq);
 	schedule_work(&ndev->init_work);
 
 	return 0;
-- 
1.8.1.2

Re: [char-misc-next 0/3 V6] Support NFC Device on MEI CL Bus

[Greg KH]

On Tue, Apr 09, 2013 at 02:41:32AM +0300, Tomas Winkler wrote:
> v5 -> v6
>  1. include/linux/uapi/mei/nfc.h - provides API also for pure
>     user space implementation as found under Android.
>  2. Removed INTEL_MEI_BUS_NFC Kconfig option. 
>     The NFC info client is disconnected as soon as we
>     get the FW info and the regular client is connected only when
>     mei_cl_enable_device() is explicitly called from an nfc driver.
>     The mei cl bus now doesn't connect NFC  exclusively.
>  3. Added pn544 to the possibly detected NFC chipsets.
> 
> Depends on: 
> 	mei: bus: Add device enabling and disabling API

If this is a nfc device, why isn't it using the nfc_register_device()
and nfc_unregister_device() calls?  Shouldn't you be using the in-kernel
nfc api we already have?

thanks,

greg k-h

RE: [char-misc-next 0/3 V6] Support NFC Device on MEI CL Bus

[Winkler, Tomas]

> 
> On Tue, Apr 09, 2013 at 02:41:32AM +0300, Tomas Winkler wrote:
> > v5 -> v6
> >  1. include/linux/uapi/mei/nfc.h - provides API also for pure
> >     user space implementation as found under Android.
> >  2. Removed INTEL_MEI_BUS_NFC Kconfig option.
> >     The NFC info client is disconnected as soon as we
> >     get the FW info and the regular client is connected only when
> >     mei_cl_enable_device() is explicitly called from an nfc driver.
> >     The mei cl bus now doesn't connect NFC  exclusively.
> >  3. Added pn544 to the possibly detected NFC chipsets.
> >
> > Depends on:
> > 	mei: bus: Add device enabling and disabling API
> 
> If this is a nfc device, why isn't it using the nfc_register_device() and
> nfc_unregister_device() calls?  Shouldn't you be using the in-kernel nfc api
> we already have?
> 

Adding Samuel.

Thanks
Tomas

Re: [char-misc-next 0/3 V6] Support NFC Device on MEI CL Bus

[Samuel Ortiz]

Hi Greg, Tomas,

On Tue, Apr 09, 2013 at 01:12:48PM +0000, Winkler, Tomas wrote:
> > 
> > On Tue, Apr 09, 2013 at 02:41:32AM +0300, Tomas Winkler wrote:
> > > v5 -> v6
> > >  1. include/linux/uapi/mei/nfc.h - provides API also for pure
> > >     user space implementation as found under Android.
> > >  2. Removed INTEL_MEI_BUS_NFC Kconfig option.
> > >     The NFC info client is disconnected as soon as we
> > >     get the FW info and the regular client is connected only when
> > >     mei_cl_enable_device() is explicitly called from an nfc driver.
> > >     The mei cl bus now doesn't connect NFC  exclusively.
> > >  3. Added pn544 to the possibly detected NFC chipsets.
> > >
> > > Depends on:
> > > 	mei: bus: Add device enabling and disabling API
> > 
> > If this is a nfc device, why isn't it using the nfc_register_device() and
> > nfc_unregister_device() calls?  Shouldn't you be using the in-kernel nfc api
> > we already have?
The mei/nfc.c code adds a physical device to the MEI bus, the in kernel NFC
APIs will be called by the NFC driver (drivers/nfc/) itself, once probed. The
mei/nfc.c code is not an NFC driver, it is an MEI specific layer for properly
detecting the right NFC chipsets behind the ME and abstracting the MEI secific
commands for this chipset.

This code is needed because the only think that the ME tells us at boot
time is that there is an NFC related UUID there. It can be any NFC chipset
(Although right now the firmware will only support the microread and pn544
ones) and to know exactly which one it is we need to send a few commands to
the ME firmware. Once that's done, we add a device to the MEI bus with the
proper name ("pn544" or "microread"). The actual driver living under
drivers/nfc/ will be probed and then register itself against the NFC
subsystem (through the NFC in-kernel APIs).
By doing so we keep the ME related operations under drivers/misc/mei while
drivers/nfc/ only deals with NFC related stuff.

I hope this clears things up.  If it does, we can add a similar explanation to
the first patch of this patchset.

Cheers,
Samuel.

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

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Greg KH]

On Tue, Apr 09, 2013 at 02:41:33AM +0300, Tomas Winkler wrote:
> From: Samuel Ortiz <sameo@linux.intel.com>
> 
> NFC ME device is exported through the 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 to properly build the ME id we first need to retrieve
> the firmware information from the info client and then disconnect from it.

I still don't understand why you aren't tieing this into the existing
nfc kernel api.  What am I missing?

> 
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/misc/mei/Makefile     |   1 +
>  drivers/misc/mei/client.c     |   3 +
>  drivers/misc/mei/init.c       |   2 +
>  drivers/misc/mei/mei_dev.h    |  10 ++
>  drivers/misc/mei/nfc.c        | 250 ++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/Kbuild     |   1 +
>  include/uapi/linux/mei/Kbuild |   2 +
>  include/uapi/linux/mei/nfc.h  | 135 +++++++++++++++++++++++

Based on our previous off-list emails, you said that this .h file is
needed for the userspace api.  But the nfc userspace api is through the
nfc API, which doesn't use any of the structures defined here.  So who
exactly uses this file?

confused,

greg k-h

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Samuel Ortiz]

Hi Greg,

On Wed, Apr 10, 2013 at 10:25:51AM -0700, Greg KH wrote:
> On Tue, Apr 09, 2013 at 02:41:33AM +0300, Tomas Winkler wrote:
> > From: Samuel Ortiz <sameo@linux.intel.com>
> > 
> > NFC ME device is exported through the 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 to properly build the ME id we first need to retrieve
> > the firmware information from the info client and then disconnect from it.
> 
> I still don't understand why you aren't tieing this into the existing
> nfc kernel api.  What am I missing?
We can't tie this into the existing NFC kernel API because mei/nfc.c is not an
NFC driver, it's an ME specific shim layer that does not do anything NFC (as
in NFC Forum specified) related.
At that point in the code the only thing we know is that there is an NFC
device behind the ME. As I said, it can be any NFC device, but it's not an
Intel specific or ME specific NFC hardware, it's just a regular NFC one
(pn544, microread, etc...) that is only accessible through the ME. The typical
setup is where you access the NFC hardware through i2c, SPI or USB, but here
it's "hidden" behind the ME.

The patch below sends the needed commands to the ME in order to fetch some
information (vendor ID, radio ID) and understand which exact chipset
is physically hooked to the ME (0x0, 0x0 -> microread, 0x1, 0x1 -> pn544).
Once that's done, we can add the right device on the ME bus (This is done on
patch #2), the same way you would add an NFC device to the USB bus when your
USB host controller tells you about the vendor and product IDs.
So after patch #1 and #2, we have an NFC device on the ME and this could be
e.g. a pn544. We do have a driver for pn544 under drivers/nfc/ which will hook
into the NFC kernel APIs once probed (The pn544 is missing an mei.c file that
will register itself as an mei_cl driver for "pn544", as described in
Documentation/misc-devices/mei/mei-client-bus.txt). So the NFC kernel APIs are
called from the actual NFC driver, because mei/nfc.c is not an NFC driver,
it's a NFC specific ME layer that is needed to register the right device (as
in the kernel device/driver model) on the MEI bus.



> > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> >  drivers/misc/mei/Makefile     |   1 +
> >  drivers/misc/mei/client.c     |   3 +
> >  drivers/misc/mei/init.c       |   2 +
> >  drivers/misc/mei/mei_dev.h    |  10 ++
> >  drivers/misc/mei/nfc.c        | 250 ++++++++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/Kbuild     |   1 +
> >  include/uapi/linux/mei/Kbuild |   2 +
> >  include/uapi/linux/mei/nfc.h  | 135 +++++++++++++++++++++++
> 
> Based on our previous off-list emails, you said that this .h file is
> needed for the userspace api.  But the nfc userspace api is through the
> nfc API, which doesn't use any of the structures defined here.  So who
> exactly uses this file?
You're right, an NFC application that would use the kernel NFC APIs (AF_NFC
and the NFC generic netlink one) will not need this header.
But the Android stack does not use any of the NFC kernel APIs, it implements
the whole NFC stack in userspace and typically sends raw HCI frames to a dumb
kernel driver (not upstreamed) through /dev/pn544 for example.
That works fine with the typical case where your pn544 is directly accessible
through i2c. But if it's sitting behind the ME, you will need to send
commands exported through this file to fetch the vendor and radio IDs, but
also to send those HCI frames that the vanilla Android stack builds after
encapsulating them into a struct mei_nfc_cmd. And this is all done through the
/dev/mei interface.

Cheers,
Samuel.

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

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Arnd Bergmann]

On Wednesday 10 April 2013, Samuel Ortiz wrote:
> That works fine with the typical case where your pn544 is directly accessible
> through i2c. But if it's sitting behind the ME, you will need to send
> commands exported through this file to fetch the vendor and radio IDs, but
> also to send those HCI frames that the vanilla Android stack builds after
> encapsulating them into a struct mei_nfc_cmd. And this is all done through the
> /dev/mei interface.

You don't make it sound like a good reason to actually suppor this by adding
a public header for it.

	Arnd

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Greg KH]

On Wed, Apr 10, 2013 at 09:19:45PM +0200, Samuel Ortiz wrote:
> Hi Greg,
> 
> On Wed, Apr 10, 2013 at 10:25:51AM -0700, Greg KH wrote:
> > On Tue, Apr 09, 2013 at 02:41:33AM +0300, Tomas Winkler wrote:
> > > From: Samuel Ortiz <sameo@linux.intel.com>
> > > 
> > > NFC ME device is exported through the 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 to properly build the ME id we first need to retrieve
> > > the firmware information from the info client and then disconnect from it.
> > 
> > I still don't understand why you aren't tieing this into the existing
> > nfc kernel api.  What am I missing?
> We can't tie this into the existing NFC kernel API because mei/nfc.c is not an
> NFC driver, it's an ME specific shim layer that does not do anything NFC (as
> in NFC Forum specified) related.

Then why call it "NFC" if you don't want to confuse everyone :)

> At that point in the code the only thing we know is that there is an NFC
> device behind the ME. As I said, it can be any NFC device, but it's not an
> Intel specific or ME specific NFC hardware, it's just a regular NFC one
> (pn544, microread, etc...) that is only accessible through the ME. The typical
> setup is where you access the NFC hardware through i2c, SPI or USB, but here
> it's "hidden" behind the ME.

Ok, but once you "unhide" it, it needs to talk through the in-kernel NFC
api, not some other random interface.

> The patch below sends the needed commands to the ME in order to fetch some
> information (vendor ID, radio ID) and understand which exact chipset
> is physically hooked to the ME (0x0, 0x0 -> microread, 0x1, 0x1 -> pn544).
> Once that's done, we can add the right device on the ME bus (This is done on
> patch #2), the same way you would add an NFC device to the USB bus when your
> USB host controller tells you about the vendor and product IDs.
> So after patch #1 and #2, we have an NFC device on the ME and this could be
> e.g. a pn544. We do have a driver for pn544 under drivers/nfc/ which will hook
> into the NFC kernel APIs once probed (The pn544 is missing an mei.c file that
> will register itself as an mei_cl driver for "pn544", as described in
> Documentation/misc-devices/mei/mei-client-bus.txt). So the NFC kernel APIs are
> called from the actual NFC driver, because mei/nfc.c is not an NFC driver,
> it's a NFC specific ME layer that is needed to register the right device (as
> in the kernel device/driver model) on the MEI bus.

So when will you send the patch for the pn544 driver?

> > >  include/uapi/linux/mei/nfc.h  | 135 +++++++++++++++++++++++
> > 
> > Based on our previous off-list emails, you said that this .h file is
> > needed for the userspace api.  But the nfc userspace api is through the
> > nfc API, which doesn't use any of the structures defined here.  So who
> > exactly uses this file?
> You're right, an NFC application that would use the kernel NFC APIs (AF_NFC
> and the NFC generic netlink one) will not need this header.
> But the Android stack does not use any of the NFC kernel APIs, it implements
> the whole NFC stack in userspace and typically sends raw HCI frames to a dumb
> kernel driver (not upstreamed) through /dev/pn544 for example.

That's horrible, use the in-kernel apis, don't create your own, and
Android should be using the correct apis as well.  If it isn't, we don't
have to support it here, in fact, I would strongly suggest that we not,
so as it gets properly changed.

In fact, why don't you change it?

> That works fine with the typical case where your pn544 is directly accessible
> through i2c. But if it's sitting behind the ME, you will need to send
> commands exported through this file to fetch the vendor and radio IDs, but
> also to send those HCI frames that the vanilla Android stack builds after
> encapsulating them into a struct mei_nfc_cmd. And this is all done through the
> /dev/mei interface.

No NFC data should be going through /dev/mei, use the proper kernel apis
please.

greg k-h

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Samuel Ortiz]

On Wed, Apr 10, 2013 at 01:03:12PM -0700, Greg KH wrote:
> On Wed, Apr 10, 2013 at 09:19:45PM +0200, Samuel Ortiz wrote:
> > On Wed, Apr 10, 2013 at 10:25:51AM -0700, Greg KH wrote:
> > > On Tue, Apr 09, 2013 at 02:41:33AM +0300, Tomas Winkler wrote:
> > > > From: Samuel Ortiz <sameo@linux.intel.com>
> > > > 
> > > > NFC ME device is exported through the 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 to properly build the ME id we first need to retrieve
> > > > the firmware information from the info client and then disconnect from it.
> > > 
> > > I still don't understand why you aren't tieing this into the existing
> > > nfc kernel api.  What am I missing?
> > We can't tie this into the existing NFC kernel API because mei/nfc.c is not an
> > NFC driver, it's an ME specific shim layer that does not do anything NFC (as
> > in NFC Forum specified) related.
> 
> Then why call it "NFC" if you don't want to confuse everyone :)
I couldn't come up with a better name for code that handles the NFC quirks for
the MEI driver...

 
> > At that point in the code the only thing we know is that there is an NFC
> > device behind the ME. As I said, it can be any NFC device, but it's not an
> > Intel specific or ME specific NFC hardware, it's just a regular NFC one
> > (pn544, microread, etc...) that is only accessible through the ME. The typical
> > setup is where you access the NFC hardware through i2c, SPI or USB, but here
> > it's "hidden" behind the ME.
> 
> Ok, but once you "unhide" it, it needs to talk through the in-kernel NFC
> api, 
I think there's still some misunderstanding here, so let's take an analogy:
Imagine that we could have a 802.11 chipset behind the ME, and that it
could be an Intel iwlwifi one, or an Atheros ath9k one, depending on the reply
to a specific ME command that we send at init time. Would you then be
suggesting that I'd have to implement the 802.11 drivers in mei/mei_wifi.c,
while all the logic for those drivers are already implemented under
drivers/net/wireless ? If I use the in-kernel NFC APIs, this is exactly what I
would have to do.


> not some other random interface.
Which random interface ? This code adds a device on the MEI bus and the
corresponding NFC driver (drivers/nfc) will be using the MEI bus API to send
the NFC frames it gets from the in-kernel NFC stack.



> > The patch below sends the needed commands to the ME in order to fetch some
> > information (vendor ID, radio ID) and understand which exact chipset
> > is physically hooked to the ME (0x0, 0x0 -> microread, 0x1, 0x1 -> pn544).
> > Once that's done, we can add the right device on the ME bus (This is done on
> > patch #2), the same way you would add an NFC device to the USB bus when your
> > USB host controller tells you about the vendor and product IDs.
> > So after patch #1 and #2, we have an NFC device on the ME and this could be
> > e.g. a pn544. We do have a driver for pn544 under drivers/nfc/ which will hook
> > into the NFC kernel APIs once probed (The pn544 is missing an mei.c file that
> > will register itself as an mei_cl driver for "pn544", as described in
> > Documentation/misc-devices/mei/mei-client-bus.txt). So the NFC kernel APIs are
> > called from the actual NFC driver, because mei/nfc.c is not an NFC driver,
> > it's a NFC specific ME layer that is needed to register the right device (as
> > in the kernel device/driver model) on the MEI bus.
> 
> So when will you send the patch for the pn544 driver?
I did for the microread one, check drivers/nfc/microread/mei.c. Actually, this
could help clearing some of the confusion.


> > > >  include/uapi/linux/mei/nfc.h  | 135 +++++++++++++++++++++++
> > > 
> > > Based on our previous off-list emails, you said that this .h file is
> > > needed for the userspace api.  But the nfc userspace api is through the
> > > nfc API, which doesn't use any of the structures defined here.  So who
> > > exactly uses this file?
> > You're right, an NFC application that would use the kernel NFC APIs (AF_NFC
> > and the NFC generic netlink one) will not need this header.
> > But the Android stack does not use any of the NFC kernel APIs, it implements
> > the whole NFC stack in userspace and typically sends raw HCI frames to a dumb
> > kernel driver (not upstreamed) through /dev/pn544 for example.
> 
> That's horrible, 
You're preaching to the choir here. I did start the whole NFC kernel
subsystem precisely because I found the whole Android NFC architecture
horrible.

> use the in-kernel apis, don't create your own, 
I am _not_ using my own APIs, I maintain the NFC daemon (neard) that precisely
uses those APIs.


> and
> Android should be using the correct apis as well.
Yes, but that's not happening. The Android folks even moved away from the
kernel Bluetooth APIs by switching to bluedroid.


> If it isn't, we don't
> have to support it here, in fact, I would strongly suggest that we not,
> so as it gets properly changed.
This was our initial intention when putting all those structures and commands
under drivers/misc/mei/nfc.h but you were against it for technical reasons.
Once I explained to you that:

"Because the Android NFC stacks running on platforms where the NFC chip is
sitting behind the ME will send data buffer containing those specific
structures to /dev/mei directly. So those will cross the user/kernel
boundary."

You agreed that this should be placed under uapi. I'm fine moving it back to
mei/nfc.h, but not under a GPL licensed mei/nfc.c.

> In fact, why don't you change it?
Why don't I change the Android NFC architecture ? Not being a Google employee
would be a good reason I guess. And Google choosing to externalize all of
their NFC development to an NFC manufacturer (That obviously has business
reasons to lock Android OEMs into its own stack) would second that...

FWIW, I did replace the initial drivers/nfc/pn544.c code that was pushed for
supporting the Android stack with a proper driver that's entirely based on the
kernel NFC APIs.

> > That works fine with the typical case where your pn544 is directly accessible
> > through i2c. But if it's sitting behind the ME, you will need to send
> > commands exported through this file to fetch the vendor and radio IDs, but
> > also to send those HCI frames that the vanilla Android stack builds after
> > encapsulating them into a struct mei_nfc_cmd. And this is all done through the
> > /dev/mei interface.
> 
> No NFC data should be going through /dev/mei, use the proper kernel apis
> please.
Not my choice, I'm sorry. And I'm not the one who's going to implement the
adaptation layer for the Android stack to properly work over /dev/mei, other
folks at Intel will.
If an Android OEM decides he wants a pn544 NFC chipset behind an x86 ME, then
NFC data will go through /dev/mei. I don't like it, but this is a business
decision I have no control over.

Cheers,
Samuel.

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

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Arnd Bergmann]

On Wednesday 10 April 2013, Samuel Ortiz wrote:
> > > That works fine with the typical case where your pn544 is directly accessible
> > > through i2c. But if it's sitting behind the ME, you will need to send
> > > commands exported through this file to fetch the vendor and radio IDs, but
> > > also to send those HCI frames that the vanilla Android stack builds after
> > > encapsulating them into a struct mei_nfc_cmd. And this is all done through the
> > > /dev/mei interface.
> > 
> > No NFC data should be going through /dev/mei, use the proper kernel apis
> > please.
> Not my choice, I'm sorry. And I'm not the one who's going to implement the
> adaptation layer for the Android stack to properly work over /dev/mei, other
> folks at Intel will.
> If an Android OEM decides he wants a pn544 NFC chipset behind an x86 ME, then
> NFC data will go through /dev/mei. I don't like it, but this is a business
> decision I have no control over.

Welcome to the crap that we have to deal with on ARM all the time. Seriously,
you may not be able to stop people from doing something stupid, but you
should not pave their way. Exporting a header file to user space, when the
only possible use of that header is to do the wrong thing helps nobody.

If someone seriously wants to use /dev/mei in that way, they can easily
provide the header file themselves, or patch the kernel in any way necessary.

	Arnd

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Samuel Ortiz]

Hi Arnd,

On Thu, Apr 11, 2013 at 12:29:38AM +0200, Arnd Bergmann wrote:
> On Wednesday 10 April 2013, Samuel Ortiz wrote:
> > > > That works fine with the typical case where your pn544 is directly accessible
> > > > through i2c. But if it's sitting behind the ME, you will need to send
> > > > commands exported through this file to fetch the vendor and radio IDs, but
> > > > also to send those HCI frames that the vanilla Android stack builds after
> > > > encapsulating them into a struct mei_nfc_cmd. And this is all done through the
> > > > /dev/mei interface.
> > > 
> > > No NFC data should be going through /dev/mei, use the proper kernel apis
> > > please.
> > Not my choice, I'm sorry. And I'm not the one who's going to implement the
> > adaptation layer for the Android stack to properly work over /dev/mei, other
> > folks at Intel will.
> > If an Android OEM decides he wants a pn544 NFC chipset behind an x86 ME, then
> > NFC data will go through /dev/mei. I don't like it, but this is a business
> > decision I have no control over.
> 
> Welcome to the crap that we have to deal with on ARM all the time. Seriously,
> you may not be able to stop people from doing something stupid, but you
> should not pave their way. Exporting a header file to user space, when the
> only possible use of that header is to do the wrong thing helps nobody.
Although I'm pretty sure the reasons for Android not to use the kernel NFC APIs
and stack are all but technical ones, I agree with your statement here.


> If someone seriously wants to use /dev/mei in that way, they can easily
> provide the header file themselves, or patch the kernel in any way necessary.
I'm fine with moving nfc.h back to drivers/misc/mei/

Cheers,
Samuel.

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

Re: [char-misc-next 1/3] mei: nfc: Initial nfc implementation

[Greg KH]

On Thu, Apr 11, 2013 at 12:52:05AM +0200, Samuel Ortiz wrote:
> > If someone seriously wants to use /dev/mei in that way, they can easily
> > provide the header file themselves, or patch the kernel in any way necessary.
> I'm fine with moving nfc.h back to drivers/misc/mei/

Great, then please move it into the .c file, as it doesn't need to be a
.h file.

thanks,

greg k-h