* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-10 16:58 David R. Bild
0 siblings, 0 replies; 7+ messages in thread
From: David R. Bild @ 2018-01-10 16:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: David R. Bild, linux-usb
This commit adds a driver for the Xaptum ENF Access Card, a TPM2.0
hardware module for authenticating IoT devices and gateways.
The card consists of a SPI TPM 2.0 chip and a USB-SPI bridge. This
driver configures the bridge, registers the bridge as an SPI
controller, and adds the TPM 2.0 as an SPI device. The in-kernel TPM
2.0 driver is then automatically loaded to configure the TPM and
expose it to userspace.
Signed-off-by: David R. Bild <david.bild@xaptum.com>
---
MAINTAINERS | 6 +
drivers/usb/misc/Kconfig | 2 +
drivers/usb/misc/Makefile | 1 +
drivers/usb/misc/xapea00x/Kconfig | 16 ++
drivers/usb/misc/xapea00x/Makefile | 7 +
drivers/usb/misc/xapea00x/xapea00x-bridge.c | 399 ++++++++++++++++++++++++++
drivers/usb/misc/xapea00x/xapea00x-core.c | 429 ++++++++++++++++++++++++++++
drivers/usb/misc/xapea00x/xapea00x-spi.c | 209 ++++++++++++++
drivers/usb/misc/xapea00x/xapea00x.h | 75 +++++
9 files changed, 1144 insertions(+)
create mode 100644 drivers/usb/misc/xapea00x/Kconfig
create mode 100644 drivers/usb/misc/xapea00x/Makefile
create mode 100644 drivers/usb/misc/xapea00x/xapea00x-bridge.c
create mode 100644 drivers/usb/misc/xapea00x/xapea00x-core.c
create mode 100644 drivers/usb/misc/xapea00x/xapea00x-spi.c
create mode 100644 drivers/usb/misc/xapea00x/xapea00x.h
diff --git a/MAINTAINERS b/MAINTAINERS
index a6e86e20761e..8d16f34b8603 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14300,6 +14300,12 @@ L: linux-wireless@vger.kernel.org
S: Maintained
F: drivers/net/wireless/rndis_wlan.c
+USB XAPEA00X DRIVER
+M: David R. Bild <david.bild@xaptum.com>
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/misc/xapea00x/
+
USB XHCI DRIVER
M: Mathias Nyman <mathias.nyman@intel.com>
L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 68d2f2cd17dd..747d7f03fb84 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -275,3 +275,5 @@ config USB_CHAOSKEY
To compile this driver as a module, choose M here: the
module will be called chaoskey.
+
+source "drivers/usb/misc/xapea00x/Kconfig"
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 109f54f5b9aa..f3583501547c 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_USB_HSIC_USB4604) += usb4604.o
obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o
obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/
+obj-$(CONFIG_USB_XAPEA00X) += xapea00x/
obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o
diff --git a/drivers/usb/misc/xapea00x/Kconfig b/drivers/usb/misc/xapea00x/Kconfig
new file mode 100644
index 000000000000..27e87bc1b4b2
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/Kconfig
@@ -0,0 +1,16 @@
+config USB_XAPEA00X
+ tristate "Xaptum ENF Access card support (XAP-EA-00x)"
+ depends on USB_SUPPORT
+ select SPI
+ select TCG_TPM
+ select TCG_TIS_SPI
+ ---help---
+ Say Y here if you want to support the Xaptum ENF Access
+ modules (XAP-EA-00x) in the USB or Mini PCI-e form
+ factors. The XAP-EA-00x module exposes a TPM 2.0 as
+ /dev/tpmX to use for authenticating with the Xaptum ENF.
+
+ To compile this driver as a module, choose M here. The
+ module will be called xapea00x.
+
+ If unsure, say M.
diff --git a/drivers/usb/misc/xapea00x/Makefile b/drivers/usb/misc/xapea00x/Makefile
new file mode 100644
index 000000000000..c4bcd7524c31
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the xapea00x driver.
+#
+obj-$(CONFIG_USB_XAPEA00X) += xapea00x.o
+
+xapea00x-y += xapea00x-core.o xapea00x-bridge.o
diff --git a/drivers/usb/misc/xapea00x/xapea00x-bridge.c b/drivers/usb/misc/xapea00x/xapea00x-bridge.c
new file mode 100644
index 000000000000..3efa697f216a
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-bridge.c
@@ -0,0 +1,399 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ * TPM 2.0-based hardware module for authenticating IoT devices and
+ * gateways.
+ *
+ * Copyright (c) 2017 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+#define XAPEA00X_BR_CMD_READ 0x00
+#define XAPEA00X_BR_CMD_WRITE 0x01
+#define XAPEA00X_BR_CMD_WRITEREAD 0x02
+
+#define XAPEA00X_BR_BREQTYP_SET 0x40
+
+#define XAPEA00X_BR_BREQ_SET_GPIO_VALUES 0x21
+#define XAPEA00X_BR_BREQ_SET_GPIO_CS 0x25
+#define XAPEA00X_BR_BREQ_SET_SPI_WORD 0x31
+
+#define XAPEA00X_BR_USB_TIMEOUT 1000 // msecs
+
+#define XAPEA00X_BR_CS_DISABLED 0x00
+
+/*******************************************************************************
+ * Bridge USB transfers
+ */
+
+struct xapea00x_br_bulk_command {
+ __be16 reserved1;
+ __u8 command;
+ __u8 reserved2;
+ __le32 length;
+} __attribute__((__packed__));
+
+/**
+ * xapea00x_br_prep_bulk_command - Prepares the bulk command header with
+ * the supplied values.
+ * @hdr: pointer to header to prepare
+ * @command: the command id for the command
+ * @length: length in bytes of the command data
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static void xapea00x_br_prep_bulk_command(struct xapea00x_br_bulk_command *hdr,
+ u8 command, int length)
+{
+ hdr->reserved1 = 0;
+ hdr->command = command;
+ hdr->reserved2 = 0;
+ hdr->length = __cpu_to_le32(length);
+}
+
+/**
+ * xapea00x_br_bulk_write - Issues a builk write to the bridge chip.
+ * @dev: pointer to the device to write to
+ * @command: the command started by this write (WRITE, READ, WRITE_READ)
+ * @data: pointer to the data to write. Must be DMA capable (e.g.,
+ * kmalloc-ed, not stack).
+ * @len: length in bytes of the data to write
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_br_bulk_write(struct xapea00x_device *dev,
+ struct xapea00x_br_bulk_command *header,
+ const void *data, int len)
+{
+ u8 *buf;
+ unsigned int pipe;
+ int buf_len, actual_len, retval;
+
+ buf_len = sizeof(struct xapea00x_br_bulk_command) + len;
+ buf = kzalloc(buf_len, GFP_KERNEL);
+ if (!buf) {
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ memcpy(buf, header, sizeof(struct xapea00x_br_bulk_command));
+ memcpy(buf + sizeof(struct xapea00x_br_bulk_command), data, len);
+
+ pipe = usb_sndbulkpipe(dev->udev, dev->bulk_out->bEndpointAddress);
+ retval = usb_bulk_msg(dev->udev, pipe, buf, buf_len, &actual_len,
+ XAPEA00X_BR_USB_TIMEOUT);
+ if (retval) {
+ dev_warn(&dev->interface->dev,
+ "%s: usb_bulk_msg() failed with %d\n",
+ __func__, retval);
+ goto free_buf;
+ }
+
+ retval = 0;
+
+free_buf:
+ kzfree(buf);
+
+out:
+ return retval;
+}
+
+/**
+ * xapea00x_br_bulk_read - Issues a bulk read to the bridge chip.
+ * @dev: pointer to the device to read from
+ * @data: pointer to the data read. Must be DMA capable (e.g.,
+ * kmalloc-ed, not stack).
+ * @len: length in bytes of the data to read
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_br_bulk_read(struct xapea00x_device *dev, void *data,
+ int len)
+{
+ unsigned int pipe;
+ void *buf;
+ int actual_len, retval;
+
+ buf = kzalloc(len, GFP_KERNEL);
+ if (!buf) {
+ retval = -ENOMEM;
+ goto out;
+ }
+
+ pipe = usb_rcvbulkpipe(dev->udev, dev->bulk_in->bEndpointAddress);
+ retval = usb_bulk_msg(dev->udev, pipe, buf, len, &actual_len,
+ XAPEA00X_BR_USB_TIMEOUT);
+
+ if (retval) {
+ dev_warn(&dev->interface->dev,
+ "%s: usb_bulk_msg() failed with %d\n",
+ __func__, retval);
+ goto free_buf;
+ }
+
+ memcpy(data, buf, actual_len);
+ retval = 0;
+
+free_buf:
+ kzfree(buf);
+
+out:
+ return retval;
+}
+
+/**
+ * xapea00x_br_ctrl_write - Issues a send control transfer to the bridge
+ * chip.
+ * @dev: pointer to the device to write to
+ * @bRequest: the command
+ * @wValue: the command value
+ * @wIndex: the command index
+ * @data: pointer to the command data
+ * @len: length in bytes of the command data
+ *
+ * The possible bRequest, wValue, and wIndex values and data format
+ * are specified in the hardware datasheet.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error code.
+ */
+static int xapea00x_br_ctrl_write(struct xapea00x_device *dev, u8 bRequest,
+ u16 wValue, u16 wIndex, u8 *data, u16 len)
+{
+ unsigned int pipe;
+ void *buf;
+ int retval;
+
+ /* control_msg buffer must be dma-capable (e.g.k kmalloc-ed,
+ * not stack). Copy data into such buffer here, so we can use
+ * simpler stack allocation in the callers - we have no
+ * performance concerns given the small buffers and low
+ * throughputs of this device.
+ */
+ buf = kzalloc(len, GFP_KERNEL);
+ if (!buf) {
+ retval = -ENOMEM;
+ goto out;
+ }
+ memcpy(buf, data, len);
+
+ pipe = usb_sndctrlpipe(dev->udev, 0);
+ retval = usb_control_msg(dev->udev, pipe, bRequest,
+ XAPEA00X_BR_BREQTYP_SET, wValue, wIndex,
+ buf, len, XAPEA00X_BR_USB_TIMEOUT);
+ if (retval < 0) {
+ dev_warn(&dev->interface->dev,
+ "usb_control_msg() failed with %d\n", retval);
+ goto free_buf;
+ }
+
+ retval = 0;
+
+free_buf:
+ kzfree(buf);
+
+out:
+ return retval;
+}
+
+/*******************************************************************************
+ * Bridge configuration commands
+ */
+
+/**
+ * xapea00x_br_set_gpio_value - Sets the value on the specified pin of
+ * the bridge chip.
+ * @dev: pointer to the device containing the bridge whose pin to set
+ * @pin: the number of the pin to set
+ * @value: the value to set the pin to, 0 or 1
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_br_set_gpio_value(struct xapea00x_device *dev, u8 pin,
+ u8 value)
+{
+ u8 data[4] = { 0, 0, 0, 0 };
+
+ switch (pin) {
+ case 10:
+ case 9:
+ case 8:
+ case 7:
+ case 6:
+ data[0] = value << (pin - 4);
+ data[2] = 1 << (pin - 4);
+ break;
+ case 5:
+ data[0] = value;
+ data[2] = 1;
+ break;
+ case 4:
+ case 3:
+ case 2:
+ case 1:
+ case 0:
+ data[1] = value << (pin + 3);
+ data[3] = 1 << (pin + 3);
+ }
+ return xapea00x_br_ctrl_write(dev, XAPEA00X_BR_BREQ_SET_GPIO_VALUES,
+ 0, 0, data, 4);
+}
+
+/**
+ * xapea00x_br_set_gpio_cs - Sets the chip select control on the specified
+ * pin of the bridge chip.
+ * @dev: pointer to the device containing the bridge whose cs to set
+ * @pin: the number of the pin to set
+ * @control: the chip select control value for the pin, 0, 1, or 2
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_br_set_gpio_cs(struct xapea00x_device *dev, u8 pin,
+ u8 control)
+{
+ u8 data[2] = { pin, control };
+
+ return xapea00x_br_ctrl_write(dev, XAPEA00X_BR_BREQ_SET_GPIO_CS,
+ 0, 0, data, 2);
+}
+
+/*******************************************************************************
+ * Bridge configuration commands
+ */
+/**
+ * xapea00x_br_disable_cs - disable the built-in chip select
+ * capability of the specified channel. It does not support holding
+ * the CS active between SPI transfers, a feature required for the
+ * TPM. Instead, we manually control the CS pin as a GPIO.
+ * @dev: pointer to the device containing the bridge whose cs to disable
+ * @channel: the SPI channel whose cs to disable
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful 0. Otherwise a negative error number.
+ */
+int xapea00x_br_disable_cs(struct xapea00x_device *dev, u8 channel)
+{
+ return xapea00x_br_set_gpio_cs(dev, channel,
+ XAPEA00X_BR_CS_DISABLED);
+}
+
+/**
+ * xapea00x_br_assert_cs - assert the chip select pin for the
+ * specified channel.
+ * @dev: pointer to the device containing the bridge who cs to assert
+ * @channel: the SPI channel whose cs to assert
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful 0. Otherwise a negative error number.
+ */
+int xapea00x_br_assert_cs(struct xapea00x_device *dev, u8 channel)
+{
+ return xapea00x_br_set_gpio_value(dev, channel, 0);
+}
+
+/**
+ * xapea00x_br_deassert_cs - deassert the chip select pin for the
+ * specified channel.
+ * @dev: pointer to the device containing the bridge who cs to deassert
+ * @channel: the SPI channel whose cs to deassert
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful 0. Otherwise a negative error number.
+ */
+int xapea00x_br_deassert_cs(struct xapea00x_device *dev, u8 channel)
+{
+ return xapea00x_br_set_gpio_value(dev, channel, 1);
+}
+
+/*******************************************************************************
+ * Bridge SPI reads and writes
+ */
+/**
+ * xeapea00x_spi_read - Performs a read from the active channel
+ * @dev: pointer to the device to perform the read
+ * @rx_buf: pointer to the buffer to read the data into. Must be
+ * DMA-capable (e.g., kmalloc-ed, not stack).
+ * @len: length in bytes of the data to read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_br_spi_read(struct xapea00x_device *dev, void *rx_buf, int len)
+{
+ struct xapea00x_br_bulk_command header;
+ int retval;
+
+ xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_READ, len);
+ retval = xapea00x_br_bulk_write(dev, &header, NULL, 0);
+ if (retval)
+ goto out;
+
+ retval = xapea00x_br_bulk_read(dev, rx_buf, len);
+
+out:
+ return retval;
+}
+
+/**
+ *xapea00x_br_spi_write - Performs a write to the active channel
+ * @dev: pointer to the device to perform the write
+ * @tx_buf: pointer to the data to write. Must be DMA-capable (e.g.,
+ * kmalloc-ed, not stack).
+ * @len: length in bytes of the data to write
+ */
+int xapea00x_br_spi_write(struct xapea00x_device *dev, const void *tx_buf,
+ int len)
+{
+ struct xapea00x_br_bulk_command header;
+ int retval;
+
+ xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_WRITE, len);
+ retval = xapea00x_br_bulk_write(dev, &header, tx_buf, len);
+
+ return retval;
+}
+
+/**
+ * xapea00x_br_spi_write_read - Performs a simultaneous write and read on
+ * the active channel
+ * @dev: pointer to the device to perform the write/read
+ * @tx_buf: pointer to the data to write. Must be DMA-capable (e.g.,
+ * kmalloc-ed, not stack).
+ * @rx_buf: pointer to the buffer to read the data into. Must be
+ * DMA-capable (e.g., kmalloc-ed, not stack).
+ * @len: length in bytes of the data to write/read
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_br_spi_write_read(struct xapea00x_device *dev, const void *tx_buf,
+ void *rx_buf, int len)
+{
+ struct xapea00x_br_bulk_command header;
+ int retval;
+
+ xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_WRITEREAD, len);
+ retval = xapea00x_br_bulk_write(dev, &header, tx_buf, len);
+ if (retval)
+ goto out;
+
+ retval = xapea00x_br_bulk_read(dev, rx_buf, len);
+
+out:
+ return retval;
+}
diff --git a/drivers/usb/misc/xapea00x/xapea00x-core.c b/drivers/usb/misc/xapea00x/xapea00x-core.c
new file mode 100644
index 000000000000..ad62c40d724b
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-core.c
@@ -0,0 +1,429 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ * TPM 2.0-based hardware module for authenticating IoT devices and
+ * gateways.
+ *
+ * Copyright (c) 2017 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+#define XAPEA00X_TPM_MODALIAS "tpm_tis_spi"
+
+#define kref_to_xapea00x(k) container_of(k, struct xapea00x_device, kref)
+
+static void xapea00x_delete(struct kref *kref)
+{
+ struct xapea00x_device *dev = kref_to_xapea00x(kref);
+
+ usb_put_dev(dev->udev);
+ kfree(dev);
+}
+
+/*******************************************************************************
+ * SPI master functions
+ */
+
+/**
+ * xapea00x_spi_setup - Setup the SPI channel for the TPM.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_setup(struct spi_device *spi)
+{
+ struct xapea00x_device *dev;
+ int retval;
+
+ dev = spi_master_get_devdata(spi->master);
+
+ mutex_lock(&dev->usb_mutex);
+ if (!dev->interface) {
+ retval = -ENODEV;
+ goto out;
+ }
+
+ /* Verify that this is the TPM device */
+ if (spi->chip_select != 0) {
+ retval = -EINVAL;
+ goto err;
+ }
+
+ /*
+ * Disable auto chip select for the TPM channel.
+ * Must be done after setting the SPI parameters.
+ */
+ retval = xapea00x_br_disable_cs(dev, 0);
+ if (retval)
+ goto err;
+
+ /* De-assert chip select for the TPM channel. */
+ retval = xapea00x_br_deassert_cs(dev, 0);
+ if (retval)
+ goto err;
+
+ dev_dbg(&dev->interface->dev, "configured spi channel for tpm\n");
+ retval = 0;
+ goto out;
+
+err:
+ dev_err(&dev->interface->dev,
+ "configuring SPI channel failed with %d\n", retval);
+
+out:
+ mutex_unlock(&dev->usb_mutex);
+ return retval;
+}
+
+/**
+ * xapea00x_spi_cleanup
+ *
+ * Context: !in_interrupt()
+ */
+static void xapea00x_spi_cleanup(struct spi_device *spi)
+{
+ dev_dbg(&spi->dev, "%s\n", __func__);
+}
+
+/**
+ * xapea00x_spi_transfer - Execute a single SPI transfer.
+ * @dev: pointer to the device to do the transfer on
+ * @tx_buf: pointer to the data to send, if not NULL
+ * @rx_buf: pointer to the buffer to store the received data, if not NULL
+ * @len: length in bytes of the data to send/receive
+ * @cs_hold: If non-zero, the chip select will remain asserted
+ * @delay_usecs: If nonzero, how long to delay after last bit transfer
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_spi_transfer(struct xapea00x_device *dev,
+ const void *tx_buf, void *rx_buf, u32 len,
+ int cs_hold, u16 delay_usecs)
+{
+ int retval;
+
+ /* Assert chip select */
+ retval = xapea00x_br_assert_cs(dev, 0);
+ if (retval)
+ goto out;
+
+ /* empty transfer */
+ if (!tx_buf && !rx_buf)
+ retval = 0;
+ /* read transfer */
+ else if (!tx_buf)
+ retval = xapea00x_br_spi_read(dev, rx_buf, len);
+ /* write transfer */
+ else if (!rx_buf)
+ retval = xapea00x_br_spi_write(dev, tx_buf, len);
+ /* write_read transfer */
+ else
+ retval = xapea00x_br_spi_write_read(dev, tx_buf, rx_buf, len);
+
+ /* Deassert chip select, if requested */
+ if (!cs_hold)
+ retval = xapea00x_br_deassert_cs(dev, 0);
+
+ /* Delay for the requested time */
+ udelay(delay_usecs);
+
+out:
+ return retval;
+}
+
+/**
+ * xapea00x_spi_transfer_one_message - Execute a full SPI message.
+ * @master: The SPI master on which to execute the message.
+ * @msg: The SPI message to execute.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative erorr number.
+ */
+static int xapea00x_spi_transfer_one_message(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct xapea00x_device *dev;
+ struct spi_transfer *xfer;
+ int is_last, retval;
+
+ dev = spi_master_get_devdata(master);
+
+ mutex_lock(&dev->usb_mutex);
+ if (!dev->interface) {
+ retval = -ENODEV;
+ goto out;
+ }
+
+ /* perform all transfers */
+ list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+ is_last = list_is_last(&xfer->transfer_list, &msg->transfers);
+
+ /* Transfer message */
+ retval = xapea00x_spi_transfer(dev, xfer->tx_buf,
+ xfer->rx_buf, xfer->len,
+ is_last == xfer->cs_change,
+ xfer->delay_usecs);
+ if (retval)
+ goto out;
+
+ msg->actual_length += xfer->len;
+ }
+
+ retval = 0;
+
+out:
+ msg->status = retval;
+ spi_finalize_current_message(master);
+
+ mutex_unlock(&dev->usb_mutex);
+ return retval;
+}
+
+/**
+ * xapea00x_spi_probe - Register and configure the SPI master.
+ * @dev: the device whose SPI master to register
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_probe(struct xapea00x_device *dev)
+{
+ struct spi_master *spi_master;
+ int retval;
+
+ spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
+ if (!spi_master) {
+ retval = -ENOMEM;
+ goto err_out;
+ }
+
+ spi_master_set_devdata(spi_master, dev);
+
+ spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
+ spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
+
+ spi_master->bus_num = -1; /* dynamically assigned */
+ spi_master->num_chipselect = 1;
+ spi_master->mode_bits = SPI_MODE_0;
+
+ spi_master->flags = 0;
+ spi_master->setup = xapea00x_spi_setup;
+ spi_master->cleanup = xapea00x_spi_cleanup;
+ spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
+
+ retval = spi_register_master(spi_master);
+
+ if (retval)
+ goto free_spi;
+
+ dev->spi_master = spi_master;
+ dev_dbg(&dev->interface->dev, "registered SPI master\n");
+
+ return 0;
+
+free_spi:
+ spi_master_put(spi_master);
+ dev->spi_master = NULL;
+
+err_out:
+ return retval;
+}
+
+struct xapea00x_async_probe {
+ struct work_struct work;
+ struct xapea00x_device *dev;
+};
+
+#define work_to_probe(w) container_of(w, struct xapea00x_async_probe, work)
+
+/**
+ * xapea00x_init_async_probe - initialize an async probe with the
+ * specified values.
+ * @probe: pointer to the async_probe to initialize
+ * @dev: pointer to the device to probe
+ * @f: pointer to the probe function
+ */
+static void xapea00x_init_async_probe(struct xapea00x_async_probe *probe,
+ struct xapea00x_device *dev,
+ void (*f)(struct work_struct *work))
+{
+ INIT_WORK(&probe->work, f);
+ probe->dev = dev;
+
+ kref_get(&dev->kref);
+ spi_master_get(dev->spi_master);
+}
+
+/**
+ * xapea00x_free_async_probe - clean up the internals of the async
+ * probe. Call this method after the probe has completed.
+ *
+ * The caller is responsible for freeing the probe itself, if
+ * dynamically allocated.
+ *
+ * @probe: pointer to the async_probe to clean up
+ */
+static void xapea00x_cleanup_async_probe(struct xapea00x_async_probe *probe)
+{
+ spi_master_put(probe->dev->spi_master);
+ kref_put(&probe->dev->kref, xapea00x_delete);
+}
+
+static struct spi_board_info tpm_board_info = {
+ .modalias = XAPEA00X_TPM_MODALIAS,
+ .max_speed_hz = 43 * 1000 * 1000, // Hz
+ .chip_select = 0,
+ .mode = SPI_MODE_0,
+ .platform_data = NULL,
+ .controller_data = NULL,
+};
+
+/**
+ * xapea00x_tpm_probe - Register and initialize the TPM device
+ * @work: the work struct contained by the xapea00x device
+ *
+ * Context: !in_interrupt()
+ */
+static void xapea00x_tpm_probe(struct work_struct *work)
+{
+ struct xapea00x_async_probe *probe = work_to_probe(work);
+ struct xapea00x_device *dev = probe->dev;
+ struct spi_master *spi_master = dev->spi_master;
+ struct spi_device *tpm;
+ int retval;
+
+ tpm = spi_new_device(spi_master, &tpm_board_info);
+ mutex_lock(&dev->usb_mutex);
+ if (!dev->interface) {
+ retval = -ENODEV;
+ goto out;
+ }
+ if (!tpm) {
+ retval = -ENODEV;
+ dev_err(&dev->interface->dev,
+ "unable to add spi device for TPM\n");
+ goto err;
+ }
+
+ dev->tpm = tpm;
+ dev_info(&dev->interface->dev, "TPM initialization complete\n");
+
+ retval = 0;
+ goto out;
+
+err:
+ dev_err(&dev->interface->dev,
+ "TPM initialization failed with %d\n", retval);
+
+out:
+ mutex_unlock(&dev->usb_mutex);
+ xapea00x_cleanup_async_probe(probe);
+ kzfree(probe);
+}
+
+/*******************************************************************************
+ * USB driver structs and functions
+ */
+
+static const struct usb_device_id xapea00x_devices[] = {
+ { USB_DEVICE(USB_VENDOR_ID_SILABS, USB_PRODUCT_ID_XAPEA001) },
+ { USB_DEVICE(USB_VENDOR_ID_XAPTUM, USB_PRODUCT_ID_XAPEA002) },
+ { USB_DEVICE(USB_VENDOR_ID_XAPTUM, USB_PRODUCT_ID_XAPEA003) },
+ { }
+};
+MODULE_DEVICE_TABLE(usb, xapea00x_devices);
+
+static int xapea00x_probe(struct usb_interface *interface,
+ const struct usb_device_id *id)
+{
+ struct xapea00x_device *dev;
+ struct xapea00x_async_probe *probe;
+ int retval;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ kref_init(&dev->kref);
+ mutex_init(&dev->usb_mutex);
+
+ /* ---------------------- USB ------------------------ */
+ dev->interface = interface;
+ dev->udev = usb_get_dev(interface_to_usbdev(interface));
+
+ dev->vid = __le16_to_cpu(dev->udev->descriptor.idVendor);
+ dev->pid = __le16_to_cpu(dev->udev->descriptor.idProduct);
+
+ retval = usb_find_common_endpoints(interface->cur_altsetting,
+ &dev->bulk_in, &dev->bulk_out,
+ NULL, NULL);
+ if (retval) {
+ dev_err(&interface->dev,
+ "could not find both bulk-in and bulk-out endpoints\n");
+ goto err_out;
+ }
+
+ usb_set_intfdata(interface, dev);
+
+ /* ---------------------- SPI Master ------------------------ */
+ retval = xapea00x_spi_probe(dev);
+ if (retval) {
+ dev_err(&interface->dev, "could not initialize SPI master\n");
+ goto err_out;
+ }
+
+ /* ---------------------- TPM SPI Device ------------------------ */
+ probe = kzalloc(sizeof(*probe), GFP_KERNEL);
+ xapea00x_init_async_probe(probe, dev, xapea00x_tpm_probe);
+
+ schedule_work(&probe->work);
+ dev_info(&interface->dev, "scheduled initialization of TPM\n");
+
+ /* ---------------------- Finished ------------------------ */
+ dev_info(&interface->dev, "device connected\n");
+ return 0;
+
+err_out:
+ dev_err(&interface->dev, "device failed with %d\n", retval);
+ kref_put(&dev->kref, xapea00x_delete);
+ return retval;
+}
+
+static void xapea00x_disconnect(struct usb_interface *interface)
+{
+ struct xapea00x_device *dev = usb_get_intfdata(interface);
+
+ usb_set_intfdata(interface, NULL);
+ spi_unregister_master(dev->spi_master);
+
+ mutex_lock(&dev->usb_mutex);
+ dev->interface = NULL;
+ mutex_unlock(&dev->usb_mutex);
+
+ kref_put(&dev->kref, xapea00x_delete);
+
+ dev_info(&dev->udev->dev, "device disconnected\n");
+}
+
+static struct usb_driver xapea00x_driver = {
+ .name = "xapea00x",
+ .probe = xapea00x_probe,
+ .disconnect = xapea00x_disconnect,
+ .suspend = NULL,
+ .resume = NULL,
+ .reset_resume = NULL,
+ .id_table = xapea00x_devices,
+ .supports_autosuspend = 0
+};
+
+module_usb_driver(xapea00x_driver);
+
+MODULE_AUTHOR("David R. Bild <david.bild@xaptum.com>");
+MODULE_DESCRIPTION("Xaptum XAP-EA-00x ENF Access card");
+MODULE_LICENSE("GPL");
+
+MODULE_ALIAS("xapea00x");
diff --git a/drivers/usb/misc/xapea00x/xapea00x-spi.c b/drivers/usb/misc/xapea00x/xapea00x-spi.c
new file mode 100644
index 000000000000..8e145b83f73b
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x-spi.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ * TPM 2.0-based hardware module for authenticating IoT devices and
+ * gateways.
+ *
+ * Copyright (c) 2017 Xaptum, Inc.
+ */
+
+#include "xapea00x.h"
+
+/*******************************************************************************
+ * SPI master functions
+ */
+
+/**
+ * xapea00x_spi_setup - Setup the SPI channel for the TPM.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_setup(struct spi_device *spi)
+{
+ struct xapea00x_device *dev;
+ int retval;
+
+ dev = spi_master_get_devdata(spi->master);
+
+ /* Verify that this is the TPM device */
+ if (spi->chip_select != 0) {
+ retval = -EINVAL;
+ goto err;
+ }
+
+ /* Set the SPI parameters for the TPM channel. */
+ retval = xapea00x_br_set_spi_word(dev, 0, XAPEA00X_TPM_SPI_WORD);
+ if (retval)
+ goto err;
+
+ /*
+ * Disable auto chip select for the TPM channel.
+ * Must be done after setting the SPI parameters.
+ */
+ retval = xapea00x_br_set_gpio_cs(dev, 0, XAPEA00X_GPIO_CS_DISABLED);
+ if (retval)
+ goto err;
+
+ /* De-assert chip select for the TPM channel. */
+ retval = xapea00x_br_set_gpio_value(dev, 0, 1);
+ if (retval)
+ goto err;
+
+ dev_dbg(&dev->interface->dev, "configured spi channel for tpm\n");
+ return 0;
+
+err:
+ dev_err(&dev->interface->dev,
+ "configuring SPI channel failed with %d\n", retval);
+ return retval;
+}
+
+/**
+ * xapea00x_spi_cleanup
+ *
+ * Context: !in_interrupt()
+ */
+static void xapea00x_spi_cleanup(struct spi_device *spi)
+{
+ dev_dbg(&spi->dev, "%s\n", __func__);
+}
+
+/**
+ * xapea00x_spi_transfer - Execute a single SPI transfer.
+ * @dev: pointer to the device to do the transfer on
+ * @tx_buf: pointer to the data to send, if not NULL
+ * @rx_buf: pointer to the buffer to store the received data, if not NULL
+ * @len: length in bytes of the data to send/receive
+ * @cs_hold: If non-zero, the chip select will remain asserted
+ * @delay_usecs: If nonzero, how long to delay after last bit transfer
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+int xapea00x_spi_transfer(struct xapea00x_device *dev,
+ const void *tx_buf, void *rx_buf, u32 len,
+ int cs_hold, u16 delay_usecs)
+{
+ int retval;
+
+ /* Assert chip select */
+ retval = xapea00x_br_set_gpio_value(dev, 0, 0);
+ if (retval)
+ goto out;
+
+ /* empty transfer */
+ if (!tx_buf && !rx_buf)
+ retval = 0;
+ /* read transfer */
+ else if (!tx_buf)
+ retval = xapea00x_br_spi_read(dev, rx_buf, len);
+ /* write transfer */
+ else if (!rx_buf)
+ retval = xapea00x_br_spi_write(dev, tx_buf, len);
+ /* write_read transfer */
+ else
+ retval = xapea00x_br_spi_write_read(dev, tx_buf, rx_buf, len);
+
+ /* Deassert chip select, if requested */
+ if (!cs_hold)
+ retval = xapea00x_br_set_gpio_value(dev, 0, 1);
+
+ /* Delay for the requested time */
+ udelay(delay_usecs);
+
+out:
+ return retval;
+}
+
+/**
+ * xapea00x_spi_transfer_one_message - Execute a full SPI message.
+ * @master: The SPI master on which to execute the message.
+ * @msg: The SPI message to execute.
+ *
+ * Context: !in_interrupt()
+ *
+ * Return: If successful, 0. Otherwise a negative erorr number.
+ */
+static int xapea00x_spi_transfer_one_message(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct xapea00x_device *dev;
+ struct spi_transfer *xfer;
+ int is_last, retval;
+
+ dev = spi_master_get_devdata(master);
+
+ /* perform all transfers */
+ list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+ is_last = list_is_last(&xfer->transfer_list, &msg->transfers);
+
+ /* Transfer message */
+ retval = xapea00x_spi_transfer(dev, xfer->tx_buf,
+ xfer->rx_buf, xfer->len,
+ is_last == xfer->cs_change,
+ xfer->delay_usecs);
+ if (retval)
+ goto out;
+
+ msg->actual_length += xfer->len;
+ }
+
+ retval = 0;
+
+out:
+ msg->status = retval;
+ spi_finalize_current_message(master);
+ return retval;
+}
+
+/**
+ * xapea00x_spi_probe - Register and configure the SPI master.
+ * @dev: the device whose SPI master to register
+ *
+ * Return: If successful, 0. Otherwise a negative error number.
+ */
+static int xapea00x_spi_probe(struct xapea00x_device *dev)
+{
+ struct spi_master *spi_master;
+ int retval;
+
+ spi_master = spi_alloc_master(&dev->udev->dev, sizeof(void *));
+ if (!spi_master) {
+ retval = -ENOMEM;
+ goto err_out;
+ }
+
+ spi_master_set_devdata(spi_master, dev);
+
+ spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
+ spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
+
+ spi_master->bus_num = -1; /* dynamically assigned */
+ spi_master->num_chipselect = XAPEA00X_NUM_CS;
+ spi_master->mode_bits = SPI_MODE_0;
+
+ spi_master->flags = 0;
+ spi_master->setup = xapea00x_spi_setup;
+ spi_master->cleanup = xapea00x_spi_cleanup;
+ spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
+
+ retval = spi_register_master(spi_master);
+
+ if (retval)
+ goto free_spi;
+
+ dev->spi_master = spi_master;
+ dev_dbg(&dev->interface->dev, "registered SPI master\n");
+
+ return 0;
+
+free_spi:
+ spi_master_put(spi_master);
+ dev->spi_master = NULL;
+
+err_out:
+ return retval;
+}
diff --git a/drivers/usb/misc/xapea00x/xapea00x.h b/drivers/usb/misc/xapea00x/xapea00x.h
new file mode 100644
index 000000000000..07594ec952ef
--- /dev/null
+++ b/drivers/usb/misc/xapea00x/xapea00x.h
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
+ * TPM 2.0-based hardware module for authenticating IoT devices and
+ * gateways.
+ *
+ * Copyright (c) 2017 Xaptum, Inc.
+ */
+
+#ifndef _XAPEA00X_H
+#define _XAPEA00X_H
+
+#include <linux/kernel.h>
+#include <linux/kref.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#define USB_VENDOR_ID_SILABS 0x10c4
+#define USB_VENDOR_ID_XAPTUM 0x2FE0
+
+#define USB_PRODUCT_ID_XAPEA001 0x8BDE
+#define USB_PRODUCT_ID_XAPEA002 0x8BDE
+#define USB_PRODUCT_ID_XAPEA003 0x8BEE
+
+struct xapea00x_device {
+ struct kref kref;
+
+ struct usb_device *udev;
+ /*
+ * The interface pointer will be set NULL when the device
+ * disconnects. Accessing it safe only while holding the
+ * usb_mutex.
+ */
+ struct usb_interface *interface;
+ /*
+ * Th usb_mutex must be held while synchronous USB requests are
+ * in progress. It is acquired during disconnect to be sure
+ * that there is not an outstanding request.
+ */
+ struct mutex usb_mutex;
+
+ struct usb_endpoint_descriptor *bulk_in;
+ struct usb_endpoint_descriptor *bulk_out;
+
+ u16 pid;
+ u16 vid;
+
+ struct spi_master *spi_master;
+ struct spi_device *tpm;
+};
+
+/* Public bridge functions */
+int xapea00x_br_disable_cs(struct xapea00x_device *dev, u8 channel);
+int xapea00x_br_assert_cs(struct xapea00x_device *dev, u8 channel);
+int xapea00x_br_deassert_cs(struct xapea00x_device *dev, u8 channel);
+
+int xapea00x_br_spi_read(struct xapea00x_device *dev, void *rx_buf, int len);
+int xapea00x_br_spi_write(struct xapea00x_device *dev, const void *tx_buf,
+ int len);
+int xapea00x_br_spi_write_read(struct xapea00x_device *dev, const void *tx_buf,
+ void *rx_buf, int len);
+
+/* Shared SPI function */
+int xapea00x_spi_transfer(struct xapea00x_device *dev,
+ const void *tx_buf, void *rx_buf, u32 len,
+ int cs_hold, u16 delay_usecs);
+
+/* Shared TPM functions */
+int xapea00x_tpm_platform_initialize(struct xapea00x_device *dev);
+
+#endif /* _XAPEA00X_H */
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-11 10:27 Oliver Neukum
0 siblings, 0 replies; 7+ messages in thread
From: Oliver Neukum @ 2018-01-11 10:27 UTC (permalink / raw)
To: David R. Bild, Greg Kroah-Hartman; +Cc: linux-usb
Am Mittwoch, den 10.01.2018, 10:58 -0600 schrieb David R. Bild :
> This commit adds a driver for the Xaptum ENF Access Card, a TPM2.0
> hardware module for authenticating IoT devices and gateways.
>
> The card consists of a SPI TPM 2.0 chip and a USB-SPI bridge. This
> driver configures the bridge, registers the bridge as an SPI
> controller, and adds the TPM 2.0 as an SPI device. The in-kernel TPM
> 2.0 driver is then automatically loaded to configure the TPM and
> expose it to userspace.
>
> Signed-off-by: David R. Bild <david.bild@xaptum.com>
> ---
> MAINTAINERS | 6 +
> drivers/usb/misc/Kconfig | 2 +
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/xapea00x/Kconfig | 16 ++
> drivers/usb/misc/xapea00x/Makefile | 7 +
> drivers/usb/misc/xapea00x/xapea00x-bridge.c | 399 ++++++++++++++++++++++++++
> drivers/usb/misc/xapea00x/xapea00x-core.c | 429 ++++++++++++++++++++++++++++
> drivers/usb/misc/xapea00x/xapea00x-spi.c | 209 ++++++++++++++
> drivers/usb/misc/xapea00x/xapea00x.h | 75 +++++
> 9 files changed, 1144 insertions(+)
> create mode 100644 drivers/usb/misc/xapea00x/Kconfig
> create mode 100644 drivers/usb/misc/xapea00x/Makefile
> create mode 100644 drivers/usb/misc/xapea00x/xapea00x-bridge.c
> create mode 100644 drivers/usb/misc/xapea00x/xapea00x-core.c
> create mode 100644 drivers/usb/misc/xapea00x/xapea00x-spi.c
> create mode 100644 drivers/usb/misc/xapea00x/xapea00x.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6e86e20761e..8d16f34b8603 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14300,6 +14300,12 @@ L: linux-wireless@vger.kernel.org
> S: Maintained
> F: drivers/net/wireless/rndis_wlan.c
>
> +USB XAPEA00X DRIVER
> +M: David R. Bild <david.bild@xaptum.com>
> +L: linux-usb@vger.kernel.org
> +S: Maintained
> +F: drivers/usb/misc/xapea00x/
> +
> USB XHCI DRIVER
> M: Mathias Nyman <mathias.nyman@intel.com>
> L: linux-usb@vger.kernel.org
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 68d2f2cd17dd..747d7f03fb84 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -275,3 +275,5 @@ config USB_CHAOSKEY
>
> To compile this driver as a module, choose M here: the
> module will be called chaoskey.
> +
> +source "drivers/usb/misc/xapea00x/Kconfig"
> diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
> index 109f54f5b9aa..f3583501547c 100644
> --- a/drivers/usb/misc/Makefile
> +++ b/drivers/usb/misc/Makefile
> @@ -30,4 +30,5 @@ obj-$(CONFIG_USB_HSIC_USB4604) += usb4604.o
> obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o
>
> obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/
> +obj-$(CONFIG_USB_XAPEA00X) += xapea00x/
> obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o
> diff --git a/drivers/usb/misc/xapea00x/Kconfig b/drivers/usb/misc/xapea00x/Kconfig
> new file mode 100644
> index 000000000000..27e87bc1b4b2
> --- /dev/null
> +++ b/drivers/usb/misc/xapea00x/Kconfig
> @@ -0,0 +1,16 @@
> +config USB_XAPEA00X
> + tristate "Xaptum ENF Access card support (XAP-EA-00x)"
> + depends on USB_SUPPORT
> + select SPI
> + select TCG_TPM
> + select TCG_TIS_SPI
> + ---help---
> + Say Y here if you want to support the Xaptum ENF Access
> + modules (XAP-EA-00x) in the USB or Mini PCI-e form
> + factors. The XAP-EA-00x module exposes a TPM 2.0 as
> + /dev/tpmX to use for authenticating with the Xaptum ENF.
> +
> + To compile this driver as a module, choose M here. The
> + module will be called xapea00x.
> +
> + If unsure, say M.
> diff --git a/drivers/usb/misc/xapea00x/Makefile b/drivers/usb/misc/xapea00x/Makefile
> new file mode 100644
> index 000000000000..c4bcd7524c31
> --- /dev/null
> +++ b/drivers/usb/misc/xapea00x/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the xapea00x driver.
> +#
> +obj-$(CONFIG_USB_XAPEA00X) += xapea00x.o
> +
> +xapea00x-y += xapea00x-core.o xapea00x-bridge.o
> diff --git a/drivers/usb/misc/xapea00x/xapea00x-bridge.c b/drivers/usb/misc/xapea00x/xapea00x-bridge.c
> new file mode 100644
> index 000000000000..3efa697f216a
> --- /dev/null
> +++ b/drivers/usb/misc/xapea00x/xapea00x-bridge.c
> @@ -0,0 +1,399 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
> + * TPM 2.0-based hardware module for authenticating IoT devices and
> + * gateways.
> + *
> + * Copyright (c) 2017 Xaptum, Inc.
> + */
> +
> +#include "xapea00x.h"
> +
> +#define XAPEA00X_BR_CMD_READ 0x00
> +#define XAPEA00X_BR_CMD_WRITE 0x01
> +#define XAPEA00X_BR_CMD_WRITEREAD 0x02
> +
> +#define XAPEA00X_BR_BREQTYP_SET 0x40
> +
> +#define XAPEA00X_BR_BREQ_SET_GPIO_VALUES 0x21
> +#define XAPEA00X_BR_BREQ_SET_GPIO_CS 0x25
> +#define XAPEA00X_BR_BREQ_SET_SPI_WORD 0x31
> +
> +#define XAPEA00X_BR_USB_TIMEOUT 1000 // msecs
> +
> +#define XAPEA00X_BR_CS_DISABLED 0x00
> +
> +/*******************************************************************************
> + * Bridge USB transfers
> + */
> +
> +struct xapea00x_br_bulk_command {
> + __be16 reserved1;
Here BE ...
> + __u8 command;
> + __u8 reserved2;
> + __le32 length;
... there LE?
> +} __attribute__((__packed__));
> +
> +/**
> + * xapea00x_br_prep_bulk_command - Prepares the bulk command header with
> + * the supplied values.
> + * @hdr: pointer to header to prepare
> + * @command: the command id for the command
> + * @length: length in bytes of the command data
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static void xapea00x_br_prep_bulk_command(struct xapea00x_br_bulk_command *hdr,
> + u8 command, int length)
> +{
> + hdr->reserved1 = 0;
> + hdr->command = command;
> + hdr->reserved2 = 0;
> + hdr->length = __cpu_to_le32(length);
> +}
> +
> +/**
> + * xapea00x_br_bulk_write - Issues a builk write to the bridge chip.
typo
> + * @dev: pointer to the device to write to
> + * @command: the command started by this write (WRITE, READ, WRITE_READ)
> + * @data: pointer to the data to write. Must be DMA capable (e.g.,
> + * kmalloc-ed, not stack).
> + * @len: length in bytes of the data to write
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_br_bulk_write(struct xapea00x_device *dev,
> + struct xapea00x_br_bulk_command *header,
> + const void *data, int len)
> +{
> + u8 *buf;
> + unsigned int pipe;
> + int buf_len, actual_len, retval;
> +
> + buf_len = sizeof(struct xapea00x_br_bulk_command) + len;
> + buf = kzalloc(buf_len, GFP_KERNEL);
Why zeroed? You copy all over it.
> + if (!buf) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + memcpy(buf, header, sizeof(struct xapea00x_br_bulk_command));
> + memcpy(buf + sizeof(struct xapea00x_br_bulk_command), data, len);
> +
> + pipe = usb_sndbulkpipe(dev->udev, dev->bulk_out->bEndpointAddress);
> + retval = usb_bulk_msg(dev->udev, pipe, buf, buf_len, &actual_len,
> + XAPEA00X_BR_USB_TIMEOUT);
> + if (retval) {
> + dev_warn(&dev->interface->dev,
> + "%s: usb_bulk_msg() failed with %d\n",
> + __func__, retval);
> + goto free_buf;
Does this make any sense?
> + }
> +
> + retval = 0;
> +
> +free_buf:
> + kzfree(buf);
> +
> +out:
> + return retval;
> +}
> +
> +/**
> + * xapea00x_br_bulk_read - Issues a bulk read to the bridge chip.
> + * @dev: pointer to the device to read from
> + * @data: pointer to the data read. Must be DMA capable (e.g.,
> + * kmalloc-ed, not stack).
> + * @len: length in bytes of the data to read
> + *
> + * Return: If successful, 0. Otherwise a negative error code.
> + */
> +static int xapea00x_br_bulk_read(struct xapea00x_device *dev, void *data,
> + int len)
> +{
> + unsigned int pipe;
> + void *buf;
> + int actual_len, retval;
> +
> + buf = kzalloc(len, GFP_KERNEL);
> + if (!buf) {
> + retval = -ENOMEM;
> + goto out;
> + }
> +
> + pipe = usb_rcvbulkpipe(dev->udev, dev->bulk_in->bEndpointAddress);
> + retval = usb_bulk_msg(dev->udev, pipe, buf, len, &actual_len,
> + XAPEA00X_BR_USB_TIMEOUT);
> +
> + if (retval) {
> + dev_warn(&dev->interface->dev,
> + "%s: usb_bulk_msg() failed with %d\n",
> + __func__, retval);
> + goto free_buf;
> + }
> +
> + memcpy(data, buf, actual_len);
> + retval = 0;
> +
> +free_buf:
> + kzfree(buf);
> +
> +out:
> + return retval;
> +}
> +
> +/**
> + * xapea00x_br_ctrl_write - Issues a send control transfer to the bridge
> + * chip.
> + * @dev: pointer to the device to write to
> + * @bRequest: the command
> + * @wValue: the command value
> + * @wIndex: the command index
> + * @data: pointer to the command data
> + * @len: length in bytes of the command data
> + *
> + * The possible bRequest, wValue, and wIndex values and data format
> + * are specified in the hardware datasheet.
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error code.
> + */
> +static int xapea00x_br_ctrl_write(struct xapea00x_device *dev, u8 bRequest,
> + u16 wValue, u16 wIndex, u8 *data, u16 len)
Combine with xapea00x_br_bulk_write()?
> +{
> + unsigned int pipe;
> + void *buf;
> + int retval;
> +
> + /* control_msg buffer must be dma-capable (e.g.k kmalloc-ed,
> + * not stack). Copy data into such buffer here, so we can use
> + * simpler stack allocation in the callers - we have no
> + * performance concerns given the small buffers and low
> + * throughputs of this device.
> + */
> + buf = kzalloc(len, GFP_KERNEL);
Why kzalloc something you copy over as the very next thing?
> + if (!buf) {
> + retval = -ENOMEM;
> + goto out;
> + }
> + memcpy(buf, data, len);
> +
> + pipe = usb_sndctrlpipe(dev->udev, 0);
> + retval = usb_control_msg(dev->udev, pipe, bRequest,
> + XAPEA00X_BR_BREQTYP_SET, wValue, wIndex,
> + buf, len, XAPEA00X_BR_USB_TIMEOUT);
> + if (retval < 0) {
> + dev_warn(&dev->interface->dev,
> + "usb_control_msg() failed with %d\n", retval);
> + goto free_buf;
> + }
> +
> + retval = 0;
> +
> +free_buf:
> + kzfree(buf);
> +
> +out:
> + return retval;
> +}
> +
> +/*******************************************************************************
> + * Bridge configuration commands
> + */
> +
> +/**
> + * xapea00x_br_set_gpio_value - Sets the value on the specified pin of
> + * the bridge chip.
> + * @dev: pointer to the device containing the bridge whose pin to set
> + * @pin: the number of the pin to set
> + * @value: the value to set the pin to, 0 or 1
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_br_set_gpio_value(struct xapea00x_device *dev, u8 pin,
> + u8 value)
> +{
> + u8 data[4] = { 0, 0, 0, 0 };
> +
> + switch (pin) {
> + case 10:
> + case 9:
> + case 8:
> + case 7:
> + case 6:
> + data[0] = value << (pin - 4);
> + data[2] = 1 << (pin - 4);
> + break;
> + case 5:
> + data[0] = value;
> + data[2] = 1;
> + break;
> + case 4:
> + case 3:
> + case 2:
> + case 1:
> + case 0:
> + data[1] = value << (pin + 3);
> + data[3] = 1 << (pin + 3);
Add a break please
> + }
> + return xapea00x_br_ctrl_write(dev, XAPEA00X_BR_BREQ_SET_GPIO_VALUES,
> + 0, 0, data, 4);
> +}
> +
> +/**
> + * xapea00x_br_set_gpio_cs - Sets the chip select control on the specified
> + * pin of the bridge chip.
> + * @dev: pointer to the device containing the bridge whose cs to set
> + * @pin: the number of the pin to set
> + * @control: the chip select control value for the pin, 0, 1, or 2
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_br_set_gpio_cs(struct xapea00x_device *dev, u8 pin,
> + u8 control)
> +{
> + u8 data[2] = { pin, control };
> +
> + return xapea00x_br_ctrl_write(dev, XAPEA00X_BR_BREQ_SET_GPIO_CS,
> + 0, 0, data, 2);
> +}
> +
> +/*******************************************************************************
> + * Bridge configuration commands
> + */
> +/**
> + * xapea00x_br_disable_cs - disable the built-in chip select
> + * capability of the specified channel. It does not support holding
> + * the CS active between SPI transfers, a feature required for the
> + * TPM. Instead, we manually control the CS pin as a GPIO.
> + * @dev: pointer to the device containing the bridge whose cs to disable
> + * @channel: the SPI channel whose cs to disable
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful 0. Otherwise a negative error number.
> + */
> +int xapea00x_br_disable_cs(struct xapea00x_device *dev, u8 channel)
> +{
> + return xapea00x_br_set_gpio_cs(dev, channel,
> + XAPEA00X_BR_CS_DISABLED);
> +}
> +
> +/**
> + * xapea00x_br_assert_cs - assert the chip select pin for the
> + * specified channel.
> + * @dev: pointer to the device containing the bridge who cs to assert
> + * @channel: the SPI channel whose cs to assert
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful 0. Otherwise a negative error number.
> + */
> +int xapea00x_br_assert_cs(struct xapea00x_device *dev, u8 channel)
> +{
> + return xapea00x_br_set_gpio_value(dev, channel, 0);
> +}
> +
> +/**
> + * xapea00x_br_deassert_cs - deassert the chip select pin for the
> + * specified channel.
> + * @dev: pointer to the device containing the bridge who cs to deassert
> + * @channel: the SPI channel whose cs to deassert
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful 0. Otherwise a negative error number.
> + */
> +int xapea00x_br_deassert_cs(struct xapea00x_device *dev, u8 channel)
> +{
> + return xapea00x_br_set_gpio_value(dev, channel, 1);
> +}
> +
> +/*******************************************************************************
> + * Bridge SPI reads and writes
> + */
> +/**
> + * xeapea00x_spi_read - Performs a read from the active channel
> + * @dev: pointer to the device to perform the read
> + * @rx_buf: pointer to the buffer to read the data into. Must be
> + * DMA-capable (e.g., kmalloc-ed, not stack).
> + * @len: length in bytes of the data to read
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +int xapea00x_br_spi_read(struct xapea00x_device *dev, void *rx_buf, int len)
> +{
> + struct xapea00x_br_bulk_command header;
> + int retval;
> +
> + xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_READ, len);
> + retval = xapea00x_br_bulk_write(dev, &header, NULL, 0);
> + if (retval)
> + goto out;
> +
> + retval = xapea00x_br_bulk_read(dev, rx_buf, len);
> +
> +out:
> + return retval;
> +}
> +
> +/**
> + *xapea00x_br_spi_write - Performs a write to the active channel
> + * @dev: pointer to the device to perform the write
> + * @tx_buf: pointer to the data to write. Must be DMA-capable (e.g.,
> + * kmalloc-ed, not stack).
> + * @len: length in bytes of the data to write
> + */
> +int xapea00x_br_spi_write(struct xapea00x_device *dev, const void *tx_buf,
> + int len)
> +{
> + struct xapea00x_br_bulk_command header;
> + int retval;
> +
> + xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_WRITE, len);
> + retval = xapea00x_br_bulk_write(dev, &header, tx_buf, len);
> +
> + return retval;
> +}
> +
> +/**
> + * xapea00x_br_spi_write_read - Performs a simultaneous write and read on
> + * the active channel
> + * @dev: pointer to the device to perform the write/read
> + * @tx_buf: pointer to the data to write. Must be DMA-capable (e.g.,
> + * kmalloc-ed, not stack).
> + * @rx_buf: pointer to the buffer to read the data into. Must be
> + * DMA-capable (e.g., kmalloc-ed, not stack).
> + * @len: length in bytes of the data to write/read
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +int xapea00x_br_spi_write_read(struct xapea00x_device *dev, const void *tx_buf,
> + void *rx_buf, int len)
> +{
> + struct xapea00x_br_bulk_command header;
> + int retval;
> +
> + xapea00x_br_prep_bulk_command(&header, XAPEA00X_BR_CMD_WRITEREAD, len);
> + retval = xapea00x_br_bulk_write(dev, &header, tx_buf, len);
> + if (retval)
> + goto out;
> +
> + retval = xapea00x_br_bulk_read(dev, rx_buf, len);
> +
> +out:
> + return retval;
> +}
> diff --git a/drivers/usb/misc/xapea00x/xapea00x-core.c b/drivers/usb/misc/xapea00x/xapea00x-core.c
> new file mode 100644
> index 000000000000..ad62c40d724b
> --- /dev/null
> +++ b/drivers/usb/misc/xapea00x/xapea00x-core.c
> @@ -0,0 +1,429 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
> + * TPM 2.0-based hardware module for authenticating IoT devices and
> + * gateways.
> + *
> + * Copyright (c) 2017 Xaptum, Inc.
> + */
> +
> +#include "xapea00x.h"
> +
> +#define XAPEA00X_TPM_MODALIAS "tpm_tis_spi"
> +
> +#define kref_to_xapea00x(k) container_of(k, struct xapea00x_device, kref)
> +
> +static void xapea00x_delete(struct kref *kref)
> +{
> + struct xapea00x_device *dev = kref_to_xapea00x(kref);
> +
> + usb_put_dev(dev->udev);
> + kfree(dev);
> +}
> +
> +/*******************************************************************************
> + * SPI master functions
> + */
> +
> +/**
> + * xapea00x_spi_setup - Setup the SPI channel for the TPM.
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_spi_setup(struct spi_device *spi)
> +{
> + struct xapea00x_device *dev;
> + int retval;
> +
> + dev = spi_master_get_devdata(spi->master);
> +
> + mutex_lock(&dev->usb_mutex);
> + if (!dev->interface) {
> + retval = -ENODEV;
> + goto out;
> + }
> +
> + /* Verify that this is the TPM device */
> + if (spi->chip_select != 0) {
> + retval = -EINVAL;
> + goto err;
> + }
> +
> + /*
> + * Disable auto chip select for the TPM channel.
> + * Must be done after setting the SPI parameters.
> + */
> + retval = xapea00x_br_disable_cs(dev, 0);
> + if (retval)
> + goto err;
> +
> + /* De-assert chip select for the TPM channel. */
> + retval = xapea00x_br_deassert_cs(dev, 0);
> + if (retval)
> + goto err;
> +
> + dev_dbg(&dev->interface->dev, "configured spi channel for tpm\n");
> + retval = 0;
> + goto out;
The control flow is innovative.
> +
> +err:
> + dev_err(&dev->interface->dev,
> + "configuring SPI channel failed with %d\n", retval);
> +
> +out:
> + mutex_unlock(&dev->usb_mutex);
> + return retval;
> +}
> +
> +/**
> + * xapea00x_spi_cleanup
> + *
> + * Context: !in_interrupt()
> + */
> +static void xapea00x_spi_cleanup(struct spi_device *spi)
> +{
> + dev_dbg(&spi->dev, "%s\n", __func__);
> +}
> +
> +/**
> + * xapea00x_spi_transfer - Execute a single SPI transfer.
> + * @dev: pointer to the device to do the transfer on
> + * @tx_buf: pointer to the data to send, if not NULL
> + * @rx_buf: pointer to the buffer to store the received data, if not NULL
> + * @len: length in bytes of the data to send/receive
> + * @cs_hold: If non-zero, the chip select will remain asserted
> + * @delay_usecs: If nonzero, how long to delay after last bit transfer
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +int xapea00x_spi_transfer(struct xapea00x_device *dev,
> + const void *tx_buf, void *rx_buf, u32 len,
> + int cs_hold, u16 delay_usecs)
> +{
> + int retval;
> +
> + /* Assert chip select */
> + retval = xapea00x_br_assert_cs(dev, 0);
> + if (retval)
> + goto out;
> +
> + /* empty transfer */
> + if (!tx_buf && !rx_buf)
> + retval = 0;
> + /* read transfer */
> + else if (!tx_buf)
> + retval = xapea00x_br_spi_read(dev, rx_buf, len);
> + /* write transfer */
> + else if (!rx_buf)
> + retval = xapea00x_br_spi_write(dev, tx_buf, len);
> + /* write_read transfer */
> + else
> + retval = xapea00x_br_spi_write_read(dev, tx_buf, rx_buf, len);
> +
> + /* Deassert chip select, if requested */
> + if (!cs_hold)
> + retval = xapea00x_br_deassert_cs(dev, 0);
> +
> + /* Delay for the requested time */
> + udelay(delay_usecs);
Ouch. Do we really need to unconditionally delay?
How about saving the time and using udelay only when required?
> +
> +out:
> + return retval;
> +}
> +
> +/**
> + * xapea00x_spi_transfer_one_message - Execute a full SPI message.
> + * @master: The SPI master on which to execute the message.
> + * @msg: The SPI message to execute.
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative erorr number.
> + */
> +static int xapea00x_spi_transfer_one_message(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct xapea00x_device *dev;
> + struct spi_transfer *xfer;
> + int is_last, retval;
> +
> + dev = spi_master_get_devdata(master);
> +
> + mutex_lock(&dev->usb_mutex);
> + if (!dev->interface) {
> + retval = -ENODEV;
> + goto out;
> + }
> +
> + /* perform all transfers */
> + list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> + is_last = list_is_last(&xfer->transfer_list, &msg->transfers);
> +
> + /* Transfer message */
> + retval = xapea00x_spi_transfer(dev, xfer->tx_buf,
> + xfer->rx_buf, xfer->len,
> + is_last == xfer->cs_change,
> + xfer->delay_usecs);
> + if (retval)
> + goto out;
> +
> + msg->actual_length += xfer->len;
> + }
> +
> + retval = 0;
> +
> +out:
> + msg->status = retval;
> + spi_finalize_current_message(master);
> +
> + mutex_unlock(&dev->usb_mutex);
> + return retval;
> +}
> +
> +/**
> + * xapea00x_spi_probe - Register and configure the SPI master.
> + * @dev: the device whose SPI master to register
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_spi_probe(struct xapea00x_device *dev)
> +{
> + struct spi_master *spi_master;
> + int retval;
> +
> + spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
> + if (!spi_master) {
> + retval = -ENOMEM;
> + goto err_out;
> + }
> +
> + spi_master_set_devdata(spi_master, dev);
> +
> + spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
> + spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
> +
> + spi_master->bus_num = -1; /* dynamically assigned */
> + spi_master->num_chipselect = 1;
> + spi_master->mode_bits = SPI_MODE_0;
> +
> + spi_master->flags = 0;
> + spi_master->setup = xapea00x_spi_setup;
> + spi_master->cleanup = xapea00x_spi_cleanup;
> + spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> +
> + retval = spi_register_master(spi_master);
> +
> + if (retval)
> + goto free_spi;
> +
> + dev->spi_master = spi_master;
Looks like a race condition
> + dev_dbg(&dev->interface->dev, "registered SPI master\n");
> +
> + return 0;
> +
> +free_spi:
> + spi_master_put(spi_master);
> + dev->spi_master = NULL;
> +
> +err_out:
> + return retval;
> +}
> +
> +struct xapea00x_async_probe {
> + struct work_struct work;
> + struct xapea00x_device *dev;
> +};
> +
> +#define work_to_probe(w) container_of(w, struct xapea00x_async_probe, work)
> +
> +/**
> + * xapea00x_init_async_probe - initialize an async probe with the
> + * specified values.
> + * @probe: pointer to the async_probe to initialize
> + * @dev: pointer to the device to probe
> + * @f: pointer to the probe function
> + */
> +static void xapea00x_init_async_probe(struct xapea00x_async_probe *probe,
> + struct xapea00x_device *dev,
> + void (*f)(struct work_struct *work))
> +{
> + INIT_WORK(&probe->work, f);
> + probe->dev = dev;
> +
> + kref_get(&dev->kref);
> + spi_master_get(dev->spi_master);
> +}
> +
> +/**
> + * xapea00x_free_async_probe - clean up the internals of the async
> + * probe. Call this method after the probe has completed.
> + *
> + * The caller is responsible for freeing the probe itself, if
> + * dynamically allocated.
> + *
> + * @probe: pointer to the async_probe to clean up
> + */
> +static void xapea00x_cleanup_async_probe(struct xapea00x_async_probe *probe)
> +{
> + spi_master_put(probe->dev->spi_master);
> + kref_put(&probe->dev->kref, xapea00x_delete);
> +}
> +
> +static struct spi_board_info tpm_board_info = {
> + .modalias = XAPEA00X_TPM_MODALIAS,
> + .max_speed_hz = 43 * 1000 * 1000, // Hz
> + .chip_select = 0,
> + .mode = SPI_MODE_0,
> + .platform_data = NULL,
> + .controller_data = NULL,
> +};
> +
> +/**
> + * xapea00x_tpm_probe - Register and initialize the TPM device
> + * @work: the work struct contained by the xapea00x device
> + *
> + * Context: !in_interrupt()
> + */
> +static void xapea00x_tpm_probe(struct work_struct *work)
> +{
> + struct xapea00x_async_probe *probe = work_to_probe(work);
> + struct xapea00x_device *dev = probe->dev;
> + struct spi_master *spi_master = dev->spi_master;
> + struct spi_device *tpm;
> + int retval;
> +
> + tpm = spi_new_device(spi_master, &tpm_board_info);
> + mutex_lock(&dev->usb_mutex);
> + if (!dev->interface) {
> + retval = -ENODEV;
> + goto out;
> + }
> + if (!tpm) {
Why test this under a mutex?
> + retval = -ENODEV;
> + dev_err(&dev->interface->dev,
> + "unable to add spi device for TPM\n");
> + goto err;
> + }
> +
> + dev->tpm = tpm;
> + dev_info(&dev->interface->dev, "TPM initialization complete\n");
> +
> + retval = 0;
> + goto out;
> +
> +err:
> + dev_err(&dev->interface->dev,
> + "TPM initialization failed with %d\n", retval);
> +
> +out:
> + mutex_unlock(&dev->usb_mutex);
> + xapea00x_cleanup_async_probe(probe);
> + kzfree(probe);
> +}
> +
> +/*******************************************************************************
> + * USB driver structs and functions
> + */
> +
> +static const struct usb_device_id xapea00x_devices[] = {
> + { USB_DEVICE(USB_VENDOR_ID_SILABS, USB_PRODUCT_ID_XAPEA001) },
> + { USB_DEVICE(USB_VENDOR_ID_XAPTUM, USB_PRODUCT_ID_XAPEA002) },
> + { USB_DEVICE(USB_VENDOR_ID_XAPTUM, USB_PRODUCT_ID_XAPEA003) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(usb, xapea00x_devices);
> +
> +static int xapea00x_probe(struct usb_interface *interface,
> + const struct usb_device_id *id)
> +{
> + struct xapea00x_device *dev;
> + struct xapea00x_async_probe *probe;
> + int retval;
> +
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + kref_init(&dev->kref);
> + mutex_init(&dev->usb_mutex);
> +
> + /* ---------------------- USB ------------------------ */
> + dev->interface = interface;
> + dev->udev = usb_get_dev(interface_to_usbdev(interface));
> +
> + dev->vid = __le16_to_cpu(dev->udev->descriptor.idVendor);
> + dev->pid = __le16_to_cpu(dev->udev->descriptor.idProduct);
> +
> + retval = usb_find_common_endpoints(interface->cur_altsetting,
> + &dev->bulk_in, &dev->bulk_out,
> + NULL, NULL);
> + if (retval) {
> + dev_err(&interface->dev,
> + "could not find both bulk-in and bulk-out endpoints\n");
> + goto err_out;
> + }
> +
> + usb_set_intfdata(interface, dev);
> +
> + /* ---------------------- SPI Master ------------------------ */
> + retval = xapea00x_spi_probe(dev);
> + if (retval) {
> + dev_err(&interface->dev, "could not initialize SPI master\n");
> + goto err_out;
> + }
> +
> + /* ---------------------- TPM SPI Device ------------------------ */
> + probe = kzalloc(sizeof(*probe), GFP_KERNEL);
Oops in the failure case
> + xapea00x_init_async_probe(probe, dev, xapea00x_tpm_probe);
> +
> + schedule_work(&probe->work);
> + dev_info(&interface->dev, "scheduled initialization of TPM\n");
> +
> + /* ---------------------- Finished ------------------------ */
> + dev_info(&interface->dev, "device connected\n");
> + return 0;
> +
> +err_out:
> + dev_err(&interface->dev, "device failed with %d\n", retval);
> + kref_put(&dev->kref, xapea00x_delete);
> + return retval;
> +}
> +
> +static void xapea00x_disconnect(struct usb_interface *interface)
> +{
> + struct xapea00x_device *dev = usb_get_intfdata(interface);
> +
> + usb_set_intfdata(interface, NULL);
> + spi_unregister_master(dev->spi_master);
> +
> + mutex_lock(&dev->usb_mutex);
> + dev->interface = NULL;
> + mutex_unlock(&dev->usb_mutex);
> +
> + kref_put(&dev->kref, xapea00x_delete);
> +
> + dev_info(&dev->udev->dev, "device disconnected\n");
> +}
> +
> +static struct usb_driver xapea00x_driver = {
> + .name = "xapea00x",
> + .probe = xapea00x_probe,
> + .disconnect = xapea00x_disconnect,
> + .suspend = NULL,
> + .resume = NULL,
> + .reset_resume = NULL,
> + .id_table = xapea00x_devices,
> + .supports_autosuspend = 0
> +};
> +
> +module_usb_driver(xapea00x_driver);
> +
> +MODULE_AUTHOR("David R. Bild <david.bild@xaptum.com>");
> +MODULE_DESCRIPTION("Xaptum XAP-EA-00x ENF Access card");
> +MODULE_LICENSE("GPL");
> +
> +MODULE_ALIAS("xapea00x");
> diff --git a/drivers/usb/misc/xapea00x/xapea00x-spi.c b/drivers/usb/misc/xapea00x/xapea00x-spi.c
> new file mode 100644
> index 000000000000..8e145b83f73b
> --- /dev/null
> +++ b/drivers/usb/misc/xapea00x/xapea00x-spi.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
> + * TPM 2.0-based hardware module for authenticating IoT devices and
> + * gateways.
> + *
> + * Copyright (c) 2017 Xaptum, Inc.
> + */
> +
> +#include "xapea00x.h"
> +
> +/*******************************************************************************
> + * SPI master functions
> + */
> +
> +/**
> + * xapea00x_spi_setup - Setup the SPI channel for the TPM.
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_spi_setup(struct spi_device *spi)
> +{
> + struct xapea00x_device *dev;
> + int retval;
> +
> + dev = spi_master_get_devdata(spi->master);
> +
> + /* Verify that this is the TPM device */
> + if (spi->chip_select != 0) {
> + retval = -EINVAL;
> + goto err;
> + }
> +
> + /* Set the SPI parameters for the TPM channel. */
> + retval = xapea00x_br_set_spi_word(dev, 0, XAPEA00X_TPM_SPI_WORD);
> + if (retval)
> + goto err;
> +
> + /*
> + * Disable auto chip select for the TPM channel.
> + * Must be done after setting the SPI parameters.
> + */
> + retval = xapea00x_br_set_gpio_cs(dev, 0, XAPEA00X_GPIO_CS_DISABLED);
> + if (retval)
> + goto err;
> +
> + /* De-assert chip select for the TPM channel. */
> + retval = xapea00x_br_set_gpio_value(dev, 0, 1);
> + if (retval)
> + goto err;
> +
> + dev_dbg(&dev->interface->dev, "configured spi channel for tpm\n");
> + return 0;
> +
> +err:
> + dev_err(&dev->interface->dev,
> + "configuring SPI channel failed with %d\n", retval);
> + return retval;
> +}
> +
> +/**
> + * xapea00x_spi_cleanup
> + *
> + * Context: !in_interrupt()
> + */
> +static void xapea00x_spi_cleanup(struct spi_device *spi)
> +{
> + dev_dbg(&spi->dev, "%s\n", __func__);
> +}
> +
> +/**
> + * xapea00x_spi_transfer - Execute a single SPI transfer.
> + * @dev: pointer to the device to do the transfer on
> + * @tx_buf: pointer to the data to send, if not NULL
> + * @rx_buf: pointer to the buffer to store the received data, if not NULL
> + * @len: length in bytes of the data to send/receive
> + * @cs_hold: If non-zero, the chip select will remain asserted
> + * @delay_usecs: If nonzero, how long to delay after last bit transfer
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +int xapea00x_spi_transfer(struct xapea00x_device *dev,
> + const void *tx_buf, void *rx_buf, u32 len,
> + int cs_hold, u16 delay_usecs)
> +{
> + int retval;
> +
> + /* Assert chip select */
> + retval = xapea00x_br_set_gpio_value(dev, 0, 0);
> + if (retval)
> + goto out;
> +
> + /* empty transfer */
> + if (!tx_buf && !rx_buf)
> + retval = 0;
> + /* read transfer */
> + else if (!tx_buf)
> + retval = xapea00x_br_spi_read(dev, rx_buf, len);
> + /* write transfer */
> + else if (!rx_buf)
> + retval = xapea00x_br_spi_write(dev, tx_buf, len);
> + /* write_read transfer */
> + else
> + retval = xapea00x_br_spi_write_read(dev, tx_buf, rx_buf, len);
> +
> + /* Deassert chip select, if requested */
> + if (!cs_hold)
> + retval = xapea00x_br_set_gpio_value(dev, 0, 1);
> +
> + /* Delay for the requested time */
> + udelay(delay_usecs);
> +
> +out:
> + return retval;
> +}
> +
> +/**
> + * xapea00x_spi_transfer_one_message - Execute a full SPI message.
> + * @master: The SPI master on which to execute the message.
> + * @msg: The SPI message to execute.
> + *
> + * Context: !in_interrupt()
> + *
> + * Return: If successful, 0. Otherwise a negative erorr number.
> + */
> +static int xapea00x_spi_transfer_one_message(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct xapea00x_device *dev;
> + struct spi_transfer *xfer;
> + int is_last, retval;
> +
> + dev = spi_master_get_devdata(master);
> +
> + /* perform all transfers */
> + list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> + is_last = list_is_last(&xfer->transfer_list, &msg->transfers);
> +
> + /* Transfer message */
> + retval = xapea00x_spi_transfer(dev, xfer->tx_buf,
> + xfer->rx_buf, xfer->len,
> + is_last == xfer->cs_change,
> + xfer->delay_usecs);
> + if (retval)
> + goto out;
> +
> + msg->actual_length += xfer->len;
> + }
> +
> + retval = 0;
> +
> +out:
> + msg->status = retval;
> + spi_finalize_current_message(master);
> + return retval;
> +}
> +
> +/**
> + * xapea00x_spi_probe - Register and configure the SPI master.
> + * @dev: the device whose SPI master to register
> + *
> + * Return: If successful, 0. Otherwise a negative error number.
> + */
> +static int xapea00x_spi_probe(struct xapea00x_device *dev)
> +{
> + struct spi_master *spi_master;
> + int retval;
> +
> + spi_master = spi_alloc_master(&dev->udev->dev, sizeof(void *));
> + if (!spi_master) {
> + retval = -ENOMEM;
> + goto err_out;
> + }
> +
> + spi_master_set_devdata(spi_master, dev);
> +
> + spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
> + spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
> +
> + spi_master->bus_num = -1; /* dynamically assigned */
> + spi_master->num_chipselect = XAPEA00X_NUM_CS;
> + spi_master->mode_bits = SPI_MODE_0;
> +
> + spi_master->flags = 0;
> + spi_master->setup = xapea00x_spi_setup;
> + spi_master->cleanup = xapea00x_spi_cleanup;
> + spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> +
> + retval = spi_register_master(spi_master);
> +
> + if (retval)
> + goto free_spi;
> +
> + dev->spi_master = spi_master;
> + dev_dbg(&dev->interface->dev, "registered SPI master\n");
> +
> + return 0;
> +
> +free_spi:
> + spi_master_put(spi_master);
> + dev->spi_master = NULL;
> +
> +err_out:
> + return retval;
> +}
> diff --git a/drivers/usb/misc/xapea00x/xapea00x.h b/drivers/usb/misc/xapea00x/xapea00x.h
> new file mode 100644
> index 000000000000..07594ec952ef
> --- /dev/null
> +++ b/drivers/usb/misc/xapea00x/xapea00x.h
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for the XAP-EA-00x series of the Xaptum Edge Access Card, a
> + * TPM 2.0-based hardware module for authenticating IoT devices and
> + * gateways.
> + *
> + * Copyright (c) 2017 Xaptum, Inc.
> + */
> +
> +#ifndef _XAPEA00X_H
> +#define _XAPEA00X_H
> +
> +#include <linux/kernel.h>
> +#include <linux/kref.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/usb.h>
> +#include <linux/workqueue.h>
> +
> +#define USB_VENDOR_ID_SILABS 0x10c4
> +#define USB_VENDOR_ID_XAPTUM 0x2FE0
> +
> +#define USB_PRODUCT_ID_XAPEA001 0x8BDE
> +#define USB_PRODUCT_ID_XAPEA002 0x8BDE
> +#define USB_PRODUCT_ID_XAPEA003 0x8BEE
> +
> +struct xapea00x_device {
> + struct kref kref;
> +
> + struct usb_device *udev;
> + /*
> + * The interface pointer will be set NULL when the device
> + * disconnects. Accessing it safe only while holding the
> + * usb_mutex.
> + */
> + struct usb_interface *interface;
> + /*
> + * Th usb_mutex must be held while synchronous USB requests are
> + * in progress. It is acquired during disconnect to be sure
> + * that there is not an outstanding request.
> + */
> + struct mutex usb_mutex;
> +
> + struct usb_endpoint_descriptor *bulk_in;
> + struct usb_endpoint_descriptor *bulk_out;
> +
> + u16 pid;
> + u16 vid;
> +
> + struct spi_master *spi_master;
> + struct spi_device *tpm;
> +};
> +
> +/* Public bridge functions */
> +int xapea00x_br_disable_cs(struct xapea00x_device *dev, u8 channel);
> +int xapea00x_br_assert_cs(struct xapea00x_device *dev, u8 channel);
> +int xapea00x_br_deassert_cs(struct xapea00x_device *dev, u8 channel);
> +
> +int xapea00x_br_spi_read(struct xapea00x_device *dev, void *rx_buf, int len);
> +int xapea00x_br_spi_write(struct xapea00x_device *dev, const void *tx_buf,
> + int len);
> +int xapea00x_br_spi_write_read(struct xapea00x_device *dev, const void *tx_buf,
> + void *rx_buf, int len);
> +
> +/* Shared SPI function */
> +int xapea00x_spi_transfer(struct xapea00x_device *dev,
> + const void *tx_buf, void *rx_buf, u32 len,
> + int cs_hold, u16 delay_usecs);
> +
> +/* Shared TPM functions */
> +int xapea00x_tpm_platform_initialize(struct xapea00x_device *dev);
> +
> +#endif /* _XAPEA00X_H */
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-22 14:28 Greg Kroah-Hartman
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2018-01-22 14:28 UTC (permalink / raw)
To: David R. Bild; +Cc: linux-usb
On Wed, Jan 10, 2018 at 10:58:29AM -0600, David R. Bild wrote:
> +/**
> + * xapea00x_spi_cleanup
> + *
> + * Context: !in_interrupt()
> + */
No need to have kerneldoc formatting for static functions, nor the whole
Context thing, but it's your code to maintain over time, not mine :)
> +static void xapea00x_spi_cleanup(struct spi_device *spi)
> +{
> + dev_dbg(&spi->dev, "%s\n", __func__);
> +}
Why? Why is this function here at all?
And don't use dev_dbg() for function calls, that's what ftrace is for.
Shouldn't you be actually cleaning up some memory or reference count
here?
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-22 15:07 David R. Bild
0 siblings, 0 replies; 7+ messages in thread
From: David R. Bild @ 2018-01-22 15:07 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-usb
On Mon, Jan 22, 2018 at 8:28 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 10, 2018 at 10:58:29AM -0600, David R. Bild wrote:
> > +/**
> > + * xapea00x_spi_cleanup
> > + *
> > + * Context: !in_interrupt()
> > + */
>
> No need to have kerneldoc formatting for static functions, nor the whole
> Context thing, but it's your code to maintain over time, not mine :)
It's somewhat helpful for new folks on our team who need to follow the
code, although rather verbose afterwards. If you think it's too
verbose by kernel standards, I'm happy to de-kerneldoc the static
functions.
>
>
> > +static void xapea00x_spi_cleanup(struct spi_device *spi)
> > +{
> > + dev_dbg(&spi->dev, "%s\n", __func__);
> > +}
>
> Why? Why is this function here at all?
I'll remove this method in v2 when addressing Oliver's comments. The
SPI subsystem does allow a null pointer for the cleanup callback. I
didn't think it did based on some examples I'd been looking at ---
hence the no-op function.
> Shouldn't you be actually cleaning up some memory or reference count
> here?
In this case, there's nothing to free or decrement. All cleanup is
done when the USB device is unregistered.
Thanks,
David
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-22 15:15 Greg Kroah-Hartman
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2018-01-22 15:15 UTC (permalink / raw)
To: David R. Bild; +Cc: linux-usb
On Mon, Jan 22, 2018 at 09:07:34AM -0600, David R. Bild wrote:
> On Mon, Jan 22, 2018 at 8:28 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jan 10, 2018 at 10:58:29AM -0600, David R. Bild wrote:
> > > +/**
> > > + * xapea00x_spi_cleanup
> > > + *
> > > + * Context: !in_interrupt()
> > > + */
> >
> > No need to have kerneldoc formatting for static functions, nor the whole
> > Context thing, but it's your code to maintain over time, not mine :)
>
> It's somewhat helpful for new folks on our team who need to follow the
> code, although rather verbose afterwards. If you think it's too
> verbose by kernel standards, I'm happy to de-kerneldoc the static
> functions.
No, it's fine, just not necessary at all. But note if you have them,
then you need to ensure that you keep them up to date over time.
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-25 16:34 David R. Bild
0 siblings, 0 replies; 7+ messages in thread
From: David R. Bild @ 2018-01-25 16:34 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb
On Thu, Jan 11, 2018 at 4:27 AM, Oliver Neukum <oneukum@suse.com> wrote:
>
> Am Mittwoch, den 10.01.2018, 10:58 -0600 schrieb David R. Bild :
> > This commit adds a driver for the Xaptum ENF Access Card, a TPM2.0
> > hardware module for authenticating IoT devices and gateways.
> >
Thanks for the review. Fixed a couple of definite bugs.
Responses (or requests for clarification) for some of your comments
follow. The others are fixed in a v2, which I'll sent once you answer
my clarifying questions.
> > +static int xapea00x_br_bulk_write(struct xapea00x_device *dev,
> > + struct xapea00x_br_bulk_command *header,
> > + const void *data, int len)
> > +{
> > + u8 *buf;
> > + unsigned int pipe;
> > + int buf_len, actual_len, retval;
> > +
> > + buf_len = sizeof(struct xapea00x_br_bulk_command) + len;
> > + buf = kzalloc(buf_len, GFP_KERNEL);
>
> Why zeroed? You copy all over it.
It's easiest to audit the code for incorrect usages of kalloc if we
just always use kzalloc.
This device isn't used very frequently --- once shortly after boot if
used correctly and once each time the internet connection is
re-established if used poorly. Each use sends just a couple KB
between device and host. So I'd rather optimize for ease of
functional correctness and safety than saving the few cycles needed to
zero something that is immediately overwritten.
Same logic for using kzfree everywhere, even on structures or buffers
that don't contain user or other potentially sensitive data.
> > + memcpy(buf, header, sizeof(struct xapea00x_br_bulk_command));
> > + memcpy(buf + sizeof(struct xapea00x_br_bulk_command), data, len);
> > +
> > + pipe = usb_sndbulkpipe(dev->udev, dev->bulk_out->bEndpointAddress);
> > + retval = usb_bulk_msg(dev->udev, pipe, buf, buf_len, &actual_len,
> > + XAPEA00X_BR_USB_TIMEOUT);
> > + if (retval) {
> > + dev_warn(&dev->interface->dev,
> > + "%s: usb_bulk_msg() failed with %d\n",
> > + __func__, retval);
> > + goto free_buf;
>
> Does this make any sense?
It does to me. What specifically do you think is odd?
> > + }
> > +
> > + retval = 0;
> > +
> > +free_buf:
> > + kzfree(buf);
> > +
> > +out:
> > + return retval;
> > +}
> > +static int xapea00x_br_ctrl_write(struct xapea00x_device *dev, u8 bRequest,
> > + u16 wValue, u16 wIndex, u8 *data, u16 len)
>
> Combine with xapea00x_br_bulk_write()?
No. xapea00x_br_bulk_write and xapea00x_br_ctrl_write conceptually do
very different things and take very different parameters.
> > +static int xapea00x_spi_setup(struct spi_device *spi)
> > +{
> > + struct xapea00x_device *dev;
> > + int retval;
> > +
> > + dev = spi_master_get_devdata(spi->master);
> > +
> > + mutex_lock(&dev->usb_mutex);
> > + if (!dev->interface) {
> > + retval = -ENODEV;
> > + goto out;
> > + }
> > +
> > + /* Verify that this is the TPM device */
> > + if (spi->chip_select != 0) {
> > + retval = -EINVAL;
> > + goto err;
> > + }
> > +
> > + /*
> > + * Disable auto chip select for the TPM channel.
> > + * Must be done after setting the SPI parameters.
> > + */
> > + retval = xapea00x_br_disable_cs(dev, 0);
> > + if (retval)
> > + goto err;
> > +
> > + /* De-assert chip select for the TPM channel. */
> > + retval = xapea00x_br_deassert_cs(dev, 0);
> > + if (retval)
> > + goto err;
> > +
> > + dev_dbg(&dev->interface->dev, "configured spi channel for tpm\n");
> > + retval = 0;
> > + goto out;
>
> The control flow is innovative.
Thanks ;)
I've removed the "retval = 0;" as that is unnecessary. Otherwise, do
you have a better control flow that doesn't involve repeating either
the following "dev_err" or "mutex_unlock". I've found this style to
be the safest in the face of future changes.
> > +
> > +err:
> > + dev_err(&dev->interface->dev,
> > + "configuring SPI channel failed with %d\n", retval);
> > +
> > +out:
> > + mutex_unlock(&dev->usb_mutex);
> > + return retval;
> > +}
> > +
> > + /* Deassert chip select, if requested */
> > + if (!cs_hold)
> > + retval = xapea00x_br_deassert_cs(dev, 0);
> > +
> > + /* Delay for the requested time */
> > + udelay(delay_usecs);
>
> Ouch. Do we really need to unconditionally delay?
> How about saving the time and using udelay only when required?
A delay before a subsequent SPI operation is required. The exact time
is specified by the SPI chip datasheet. The bookkeeping needed to
determine if the delay was met implicitly and thus avoid the explicit
delay would be complex and error prone. No one does it this way ---
an unconditional udelay is standard for spi controller drivers in the
kernel as far as I know.
Luckily, for most devices (this one included) delay_usecs is 0, so the
udelay doesn't actually delay.
> > +static int xapea00x_spi_probe(struct xapea00x_device *dev)
> > +{
> > + struct spi_master *spi_master;
> > + int retval;
> > +
> > + spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
> > + if (!spi_master) {
> > + retval = -ENOMEM;
> > + goto err_out;
> > + }
> > +
> > + spi_master_set_devdata(spi_master, dev);
> > +
> > + spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
> > + spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
> > +
> > + spi_master->bus_num = -1; /* dynamically assigned */
> > + spi_master->num_chipselect = 1;
> > + spi_master->mode_bits = SPI_MODE_0;
> > +
> > + spi_master->flags = 0;
> > + spi_master->setup = xapea00x_spi_setup;
> > + spi_master->cleanup = xapea00x_spi_cleanup;
> > + spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> > +
> > + retval = spi_register_master(spi_master);
> > +
> > + if (retval)
> > + goto free_spi;
> > +
> > + dev->spi_master = spi_master;
>
> Looks like a race condition
>
I'm not seeing the possible race condition. Can you please elaborate?
> > + dev_dbg(&dev->interface->dev, "registered SPI master\n");
> > +
> > + return 0;
> > +
> > +free_spi:
> > + spi_master_put(spi_master);
> > + dev->spi_master = NULL;
> > +
> > +err_out:
> > + return retval;
> > +}
> > +static void xapea00x_tpm_probe(struct work_struct *work)
> > +{
> > + struct xapea00x_async_probe *probe = work_to_probe(work);
> > + struct xapea00x_device *dev = probe->dev;
> > + struct spi_master *spi_master = dev->spi_master;
> > + struct spi_device *tpm;
> > + int retval;
> > +
> > + tpm = spi_new_device(spi_master, &tpm_board_info);
> > + mutex_lock(&dev->usb_mutex);
> > + if (!dev->interface) {
> > + retval = -ENODEV;
> > + goto out;
> > + }
> > + if (!tpm) {
>
> Why test this under a mutex?
>
dev->interface being NULL/non-NULL is used as a flag to determine if
the USB device has been unregistered. So, the mutex needs to be held
when dereferencing dev->interface subsequently.
Performing the test before acquiring the lock would be a race condition.
> > + retval = -ENODEV;
> > + dev_err(&dev->interface->dev,
> > + "unable to add spi device for TPM\n");
> > + goto err;
> > + }
> > +
> > + dev->tpm = tpm;
> > + dev_info(&dev->interface->dev, "TPM initialization complete\n");
> > +
> > + retval = 0;
> > + goto out;
> > +
> > +err:
> > + dev_err(&dev->interface->dev,
> > + "TPM initialization failed with %d\n", retval);
> > +
> > +out:
> > + mutex_unlock(&dev->usb_mutex);
> > + xapea00x_cleanup_async_probe(probe);
> > + kzfree(probe);
> > +}
Thanks much,
David
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card
@ 2018-01-25 16:43 David R. Bild
0 siblings, 0 replies; 7+ messages in thread
From: David R. Bild @ 2018-01-25 16:43 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Greg Kroah-Hartman, linux-usb
On Thu, Jan 25, 2018 at 10:34 AM, David R. Bild <david.bild@xaptum.com> wrote:
> On Thu, Jan 11, 2018 at 4:27 AM, Oliver Neukum <oneukum@suse.com> wrote:
>>
>> Am Mittwoch, den 10.01.2018, 10:58 -0600 schrieb David R. Bild :
>
>> > +static void xapea00x_tpm_probe(struct work_struct *work)
>> > +{
>> > + struct xapea00x_async_probe *probe = work_to_probe(work);
>> > + struct xapea00x_device *dev = probe->dev;
>> > + struct spi_master *spi_master = dev->spi_master;
>> > + struct spi_device *tpm;
>> > + int retval;
>> > +
>> > + tpm = spi_new_device(spi_master, &tpm_board_info);
>> > + mutex_lock(&dev->usb_mutex);
>> > + if (!dev->interface) {
>> > + retval = -ENODEV;
>> > + goto out;
>> > + }
>> > + if (!tpm) {
>>
>> Why test this under a mutex?
>>
>
> dev->interface being NULL/non-NULL is used as a flag to determine if
> the USB device has been unregistered. So, the mutex needs to be held
> when dereferencing dev->interface subsequently.
>
> Performing the test before acquiring the lock would be a race condition.
>
I'm realizing I misunderstood this comment --- you're referring to
testing "!tpm", not testing "!dev->interface".
It's because the in the failure case (!tpm), I want the error message
to include the "dev->interface->dev" identifier. But that
dereferencing is only safe if "dev->interface" isn't NULL.
>> > + retval = -ENODEV;
>> > + dev_err(&dev->interface->dev,
>> > + "unable to add spi device for TPM\n");
>> > + goto err;
>> > + }
>> > +
Thanks much,
David
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-01-25 16:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-11 10:27 [1/2] usb: misc: xapea00x: add driver for Xaptum ENF Access Card Oliver Neukum
-- strict thread matches above, loose matches on Subject: below --
2018-01-25 16:43 David R. Bild
2018-01-25 16:34 David R. Bild
2018-01-22 15:15 Greg Kroah-Hartman
2018-01-22 15:07 David R. Bild
2018-01-22 14:28 Greg Kroah-Hartman
2018-01-10 16:58 David R. Bild
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).