usb: typec: ucsi: Simplified registration and I/O API

Adding more simplified API for interface registration and
read and write operations.

The registration is split into separate creation and
registration phases. That allows the drivers to properly
initialize the interface before registering it if necessary.

The read and write operations are supplied in a completely
separate struct ucsi_operations that is passed to the
ucsi_register() function during registration. The new read
and write operations will work more traditionally so that
the read callback function reads a requested amount of data
from an offset, and the write callback functions write the
given data to the offset. The drivers will have to support
both non-blocking writing and blocking writing. In blocking
writing the driver itself is responsible of waiting for the
completion event.

The new API makes it possible for the drivers to perform
tasks also independently of the core ucsi.c, and that should
allow for example quirks to be handled completely in the
drivers without the need to touch ucsi.c.

The old API is kept until all drivers have been converted to
the new API.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Ajay Gupta <ajayg@nvidia.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20191104142435.29960-12-heikki.krogerus@linux.intel.comSigned-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/usb/typec/ucsi/ucsi.c

@@ -98,6 +98,101 @@ static int ucsi_ack(struct ucsi *ucsi, u8 ack)
 	return ret;
 }
 
+static int ucsi_acknowledge_command(struct ucsi *ucsi)
+{
+	u64 ctrl;
+
+	ctrl = UCSI_ACK_CC_CI;
+	ctrl |= UCSI_ACK_COMMAND_COMPLETE;
+
+	return ucsi->ops->sync_write(ucsi, UCSI_CONTROL, &ctrl, sizeof(ctrl));
+}
+
+static int ucsi_acknowledge_connector_change(struct ucsi *ucsi)
+{
+	u64 ctrl;
+
+	ctrl = UCSI_ACK_CC_CI;
+	ctrl |= UCSI_ACK_CONNECTOR_CHANGE;
+
+	return ucsi->ops->async_write(ucsi, UCSI_CONTROL, &ctrl, sizeof(ctrl));
+}
+
+static int ucsi_exec_command(struct ucsi *ucsi, u64 command);
+
+static int ucsi_read_error(struct ucsi *ucsi)
+{
+	u16 error;
+	int ret;
+
+	/* Acknowlege the command that failed */
+	ret = ucsi_acknowledge_command(ucsi);
+	if (ret)
+		return ret;
+
+	ret = ucsi_exec_command(ucsi, UCSI_GET_ERROR_STATUS);
+	if (ret < 0)
+		return ret;
+
+	ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, &error, sizeof(error));
+	if (ret)
+		return ret;
+
+	switch (error) {
+	case UCSI_ERROR_INCOMPATIBLE_PARTNER:
+		return -EOPNOTSUPP;
+	case UCSI_ERROR_CC_COMMUNICATION_ERR:
+		return -ECOMM;
+	case UCSI_ERROR_CONTRACT_NEGOTIATION_FAIL:
+		return -EPROTO;
+	case UCSI_ERROR_DEAD_BATTERY:
+		dev_warn(ucsi->dev, "Dead battery condition!\n");
+		return -EPERM;
+	/* The following mean a bug in this driver */
+	case UCSI_ERROR_INVALID_CON_NUM:
+	case UCSI_ERROR_UNREGONIZED_CMD:
+	case UCSI_ERROR_INVALID_CMD_ARGUMENT:
+		dev_err(ucsi->dev, "possible UCSI driver bug (0x%x)\n", error);
+		return -EINVAL;
+	default:
+		dev_err(ucsi->dev, "%s: error without status\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)
+{
+	u32 cci;
+	int ret;
+
+	ret = ucsi->ops->sync_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd));
+	if (ret)
+		return ret;
+
+	ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+	if (ret)
+		return ret;
+
+	if (cci & UCSI_CCI_BUSY)
+		return -EBUSY;
+
+	if (!(cci & UCSI_CCI_COMMAND_COMPLETE))
+		return -EIO;
+
+	if (cci & UCSI_CCI_NOT_SUPPORTED)
+		return -EOPNOTSUPP;
+
+	if (cci & UCSI_CCI_ERROR) {
+		if (cmd == UCSI_GET_ERROR_STATUS)
+			return -EIO;
+		return ucsi_read_error(ucsi);
+	}
+
+	return UCSI_CCI_LENGTH(cci);
+}
+
 static int ucsi_run_command(struct ucsi *ucsi, struct ucsi_control *ctrl,
 			    void *data, size_t size)
 {
@@ -106,6 +201,26 @@ static int ucsi_run_command(struct ucsi *ucsi, struct ucsi_control *ctrl,
 	u16 error;
 	int ret;
 
+	if (ucsi->ops) {
+		ret = ucsi_exec_command(ucsi, ctrl->raw_cmd);
+		if (ret < 0)
+			return ret;
+
+		data_length = ret;
+
+		if (data) {
+			ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, data, size);
+			if (ret)
+				return ret;
+		}
+
+		ret = ucsi_acknowledge_command(ucsi);
+		if (ret)
+			return ret;
+
+		return data_length;
+	}
+
 	ret = ucsi_command(ucsi, ctrl);
 	if (ret)
 		goto err;
@@ -518,7 +633,7 @@ static void ucsi_partner_change(struct ucsi_connector *con)
 		ucsi_altmode_update_active(con);
 }
 
-static void ucsi_connector_change(struct work_struct *work)
+static void ucsi_handle_connector_change(struct work_struct *work)
 {
 	struct ucsi_connector *con = container_of(work, struct ucsi_connector,
 						  work);
@@ -580,7 +695,10 @@ static void ucsi_connector_change(struct work_struct *work)
 	if (con->status.change & UCSI_CONSTAT_PARTNER_CHANGE)
 		ucsi_partner_change(con);
 
-	ret = ucsi_ack(ucsi, UCSI_ACK_EVENT);
+	if (ucsi->ops)
+		ret = ucsi_acknowledge_connector_change(ucsi);
+	else
+		ret = ucsi_ack(ucsi, UCSI_ACK_EVENT);
 	if (ret)
 		dev_err(ucsi->dev, "%s: ACK failed (%d)", __func__, ret);
 
@@ -591,6 +709,20 @@ static void ucsi_connector_change(struct work_struct *work)
 	mutex_unlock(&con->lock);
 }
 
+/**
+ * ucsi_connector_change - Process Connector Change Event
+ * @ucsi: UCSI Interface
+ * @num: Connector number
+ */
+void ucsi_connector_change(struct ucsi *ucsi, u8 num)
+{
+	struct ucsi_connector *con = &ucsi->connector[num - 1];
+
+	if (!test_and_set_bit(EVENT_PENDING, &ucsi->flags))
+		schedule_work(&con->work);
+}
+EXPORT_SYMBOL_GPL(ucsi_connector_change);
+
 /**
  * ucsi_notify - PPM notification handler
  * @ucsi: Source UCSI Interface for the notifications
@@ -647,6 +779,40 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
 	unsigned long tmo;
 	int ret;
 
+	if (ucsi->ops) {
+		u64 command = UCSI_PPM_RESET;
+		u32 cci;
+
+		ret = ucsi->ops->async_write(ucsi, UCSI_CONTROL, &command,
+					     sizeof(command));
+		if (ret < 0)
+			return ret;
+
+		tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS);
+
+		do {
+			if (time_is_before_jiffies(tmo))
+				return -ETIMEDOUT;
+
+			ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+			if (ret)
+				return ret;
+
+			/* If the PPM is still doing something else, reset it again. */
+			if (cci & ~UCSI_CCI_RESET_COMPLETE) {
+				ret = ucsi->ops->async_write(ucsi, UCSI_CONTROL,
+							     &command,
+							     sizeof(command));
+				if (ret < 0)
+					return ret;
+			}
+
+			msleep(20);
+		} while (!(cci & UCSI_CCI_RESET_COMPLETE));
+
+		return 0;
+	}
+
 	ctrl.raw_cmd = 0;
 	ctrl.cmd.cmd = UCSI_PPM_RESET;
 	trace_ucsi_command(&ctrl);
@@ -807,7 +973,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	struct ucsi_control ctrl;
 	int ret;
 
-	INIT_WORK(&con->work, ucsi_connector_change);
+	INIT_WORK(&con->work, ucsi_handle_connector_change);
 	init_completion(&con->complete);
 	mutex_init(&con->lock);
 	con->num = index + 1;
@@ -898,9 +1064,14 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	return 0;
 }
 
-static void ucsi_init(struct work_struct *work)
+/**
+ * ucsi_init - Initialize UCSI interface
+ * @ucsi: UCSI to be initialized
+ *
+ * Registers all ports @ucsi has and enables all notification events.
+ */
+int ucsi_init(struct ucsi *ucsi)
 {
-	struct ucsi *ucsi = container_of(work, struct ucsi, work);
 	struct ucsi_connector *con;
 	struct ucsi_control ctrl;
 	int ret;
@@ -956,7 +1127,7 @@ static void ucsi_init(struct work_struct *work)
 
 	mutex_unlock(&ucsi->ppm_lock);
 
-	return;
+	return 0;
 
 err_unregister:
 	for (con = ucsi->connector; con->port; con++) {
@@ -970,49 +1141,106 @@ static void ucsi_init(struct work_struct *work)
 	ucsi_reset_ppm(ucsi);
 err:
 	mutex_unlock(&ucsi->ppm_lock);
-	dev_err(ucsi->dev, "PPM init failed (%d)\n", ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ucsi_init);
+
+static void ucsi_init_work(struct work_struct *work)
+{
+	struct ucsi *ucsi = container_of(work, struct ucsi, work);
+	int ret;
+
+	ret = ucsi_init(ucsi);
+	if (ret)
+		dev_err(ucsi->dev, "PPM init failed (%d)\n", ret);
 }
 
 /**
- * ucsi_register_ppm - Register UCSI PPM Interface
- * @dev: Device interface to the PPM
- * @ppm: The PPM interface
- *
- * Allocates UCSI instance, associates it with @ppm and returns it to the
- * caller, and schedules initialization of the interface.
+ * ucsi_get_drvdata - Return private driver data pointer
+ * @ucsi: UCSI interface
  */
-struct ucsi *ucsi_register_ppm(struct device *dev, struct ucsi_ppm *ppm)
+void *ucsi_get_drvdata(struct ucsi *ucsi)
+{
+	return ucsi->driver_data;
+}
+EXPORT_SYMBOL_GPL(ucsi_get_drvdata);
+
+/**
+ * ucsi_get_drvdata - Assign private driver data pointer
+ * @ucsi: UCSI interface
+ * @data: Private data pointer
+ */
+void ucsi_set_drvdata(struct ucsi *ucsi, void *data)
+{
+	ucsi->driver_data = data;
+}
+EXPORT_SYMBOL_GPL(ucsi_set_drvdata);
+
+/**
+ * ucsi_create - Allocate UCSI instance
+ * @dev: Device interface to the PPM (Platform Policy Manager)
+ * @ops: I/O routines
+ */
+struct ucsi *ucsi_create(struct device *dev, const struct ucsi_operations *ops)
 {
 	struct ucsi *ucsi;
 
+	if (!ops || !ops->read || !ops->sync_write || !ops->async_write)
+		return ERR_PTR(-EINVAL);
+
 	ucsi = kzalloc(sizeof(*ucsi), GFP_KERNEL);
 	if (!ucsi)
 		return ERR_PTR(-ENOMEM);
 
-	INIT_WORK(&ucsi->work, ucsi_init);
-	init_completion(&ucsi->complete);
+	INIT_WORK(&ucsi->work, ucsi_init_work);
 	mutex_init(&ucsi->ppm_lock);
-
 	ucsi->dev = dev;
-	ucsi->ppm = ppm;
+	ucsi->ops = ops;
+
+	return ucsi;
+}
+EXPORT_SYMBOL_GPL(ucsi_create);
+
+/**
+ * ucsi_destroy - Free UCSI instance
+ * @ucsi: UCSI instance to be freed
+ */
+void ucsi_destroy(struct ucsi *ucsi)
+{
+	kfree(ucsi);
+}
+EXPORT_SYMBOL_GPL(ucsi_destroy);
+
+/**
+ * ucsi_register - Register UCSI interface
+ * @ucsi: UCSI instance
+ */
+int ucsi_register(struct ucsi *ucsi)
+{
+	int ret;
+
+	ret = ucsi->ops->read(ucsi, UCSI_VERSION, &ucsi->version,
+			      sizeof(ucsi->version));
+	if (ret)
+		return ret;
+
+	if (!ucsi->version)
+		return -ENODEV;
 
-	/*
-	 * Communication with the PPM takes a lot of time. It is not reasonable
-	 * to initialize the driver here. Using a work for now.
-	 */
 	queue_work(system_long_wq, &ucsi->work);
 
-	return ucsi;
+	return 0;
 }
-EXPORT_SYMBOL_GPL(ucsi_register_ppm);
+EXPORT_SYMBOL_GPL(ucsi_register);
 
 /**
- * ucsi_unregister_ppm - Unregister UCSI PPM Interface
- * @ucsi: struct ucsi associated with the PPM
+ * ucsi_unregister - Unregister UCSI interface
+ * @ucsi: UCSI interface to be unregistered
  *
- * Unregister UCSI PPM that was created with ucsi_register().
+ * Unregister UCSI interface that was created with ucsi_register().
  */
-void ucsi_unregister_ppm(struct ucsi *ucsi)
+void ucsi_unregister(struct ucsi *ucsi)
 {
 	struct ucsi_control ctrl;
 	int i;
@@ -1035,7 +1263,51 @@ void ucsi_unregister_ppm(struct ucsi *ucsi)
 	ucsi_reset_ppm(ucsi);
 
 	kfree(ucsi->connector);
-	kfree(ucsi);
+}
+EXPORT_SYMBOL_GPL(ucsi_unregister);
+
+/**
+ * ucsi_register_ppm - Register UCSI PPM Interface
+ * @dev: Device interface to the PPM
+ * @ppm: The PPM interface
+ *
+ * Allocates UCSI instance, associates it with @ppm and returns it to the
+ * caller, and schedules initialization of the interface.
+ */
+struct ucsi *ucsi_register_ppm(struct device *dev, struct ucsi_ppm *ppm)
+{
+	struct ucsi *ucsi;
+
+	ucsi = kzalloc(sizeof(*ucsi), GFP_KERNEL);
+	if (!ucsi)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_WORK(&ucsi->work, ucsi_init_work);
+	init_completion(&ucsi->complete);
+	mutex_init(&ucsi->ppm_lock);
+
+	ucsi->dev = dev;
+	ucsi->ppm = ppm;
+
+	/*
+	 * Communication with the PPM takes a lot of time. It is not reasonable
+	 * to initialize the driver here. Using a work for now.
+	 */
+	queue_work(system_long_wq, &ucsi->work);
+
+	return ucsi;
+}
+EXPORT_SYMBOL_GPL(ucsi_register_ppm);
+
+/**
+ * ucsi_unregister_ppm - Unregister UCSI PPM Interface
+ * @ucsi: struct ucsi associated with the PPM
+ *
+ * Unregister UCSI PPM that was created with ucsi_register().
+ */
+void ucsi_unregister_ppm(struct ucsi *ucsi)
+{
+	ucsi_unregister(ucsi);
 }
 EXPORT_SYMBOL_GPL(ucsi_unregister_ppm);
 

drivers/usb/typec/ucsi/ucsi.h

@@ -10,6 +10,56 @@
 
 /* -------------------------------------------------------------------------- */
 
+struct ucsi;
+
+/* UCSI offsets (Bytes) */
+#define UCSI_VERSION			0
+#define UCSI_CCI			4
+#define UCSI_CONTROL			8
+#define UCSI_MESSAGE_IN			16
+#define UCSI_MESSAGE_OUT		32
+
+/* Command Status and Connector Change Indication (CCI) bits */
+#define UCSI_CCI_CONNECTOR(_c_)		(((_c_) & GENMASK(7, 0)) >> 1)
+#define UCSI_CCI_LENGTH(_c_)		(((_c_) & GENMASK(15, 8)) >> 8)
+#define UCSI_CCI_NOT_SUPPORTED		BIT(25)
+#define UCSI_CCI_CANCEL_COMPLETE	BIT(26)
+#define UCSI_CCI_RESET_COMPLETE		BIT(27)
+#define UCSI_CCI_BUSY			BIT(28)
+#define UCSI_CCI_ACK_COMPLETE		BIT(29)
+#define UCSI_CCI_ERROR			BIT(30)
+#define UCSI_CCI_COMMAND_COMPLETE	BIT(31)
+
+/**
+ * struct ucsi_operations - UCSI I/O operations
+ * @read: Read operation
+ * @sync_write: Blocking write operation
+ * @async_write: Non-blocking write operation
+ *
+ * Read and write routines for UCSI interface. @sync_write must wait for the
+ * Command Completion Event from the PPM before returning, and @async_write must
+ * return immediately after sending the data to the PPM.
+ */
+struct ucsi_operations {
+	int (*read)(struct ucsi *ucsi, unsigned int offset,
+		    void *val, size_t val_len);
+	int (*sync_write)(struct ucsi *ucsi, unsigned int offset,
+			  const void *val, size_t val_len);
+	int (*async_write)(struct ucsi *ucsi, unsigned int offset,
+			   const void *val, size_t val_len);
+};
+
+struct ucsi *ucsi_create(struct device *dev, const struct ucsi_operations *ops);
+void ucsi_destroy(struct ucsi *ucsi);
+int ucsi_register(struct ucsi *ucsi);
+void ucsi_unregister(struct ucsi *ucsi);
+void *ucsi_get_drvdata(struct ucsi *ucsi);
+void ucsi_set_drvdata(struct ucsi *ucsi, void *data);
+
+void ucsi_connector_change(struct ucsi *ucsi, u8 num);
+
+/* -------------------------------------------------------------------------- */
+
 /* Command Status and Connector Change Indication (CCI) data structure */
 struct ucsi_cci {
 	u8:1; /* reserved */
@@ -207,6 +257,10 @@ struct ucsi_control {
 #define UCSI_ACK_EVENT			1
 #define UCSI_ACK_CMD			2
 
+/* Bits for ACK CC or CI */
+#define UCSI_ACK_CONNECTOR_CHANGE		BIT(16)
+#define UCSI_ACK_COMMAND_COMPLETE		BIT(17)
+
 /* Bits for SET_NOTIFICATION_ENABLE command */
 #define UCSI_ENABLE_NTFY_CMD_COMPLETE		BIT(0)
 #define UCSI_ENABLE_NTFY_EXT_PWR_SRC_CHANGE	BIT(1)
@@ -383,8 +437,12 @@ enum ucsi_status {
 };
 
 struct ucsi {
+	u16 version;
 	struct device *dev;
 	struct ucsi_ppm *ppm;
+	struct driver_data *driver_data;
+
+	const struct ucsi_operations *ops;
 
 	enum ucsi_status status;
 	struct completion complete;