lkml.org 
[lkml]   [2016]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface
From
Date

> +static int ucsi_run_cmd(struct ucsi *ucsi, void *data, size_t size)
> +{
> + int status;
> + int ret;
> +
> + dev_vdbg(ucsi->dev, "%s control 0x%llx\n", __func__,
> + ucsi->ppm->data->control);
> +
> + ret = ucsi->ppm->cmd(ucsi->ppm);
> + if (ret)
> + return ret;
> +
> + /* REVISIT: We may need to set UCSI_CCI_CMD_COMPLETE flag here */
> + wait_for_completion(&ucsi->complete);
> +
> + status = ucsi->status;
> + if (status != UCSI_ERROR && size)
> + memcpy(data, ucsi->ppm->data->message_in, size);
> +
> + ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> + if (ret)
> + goto out;
> +
> + if (status == UCSI_ERROR) {
> + u16 error;
> +
> + ucsi->ppm->data->control = UCSI_GET_ERROR_STATUS;
> + ret = ucsi->ppm->cmd(ucsi->ppm);
> + if (ret)
> + goto out;
> +
> + wait_for_completion(&ucsi->complete);
> +
> + /* Something has really gone wrong */
> + if (ucsi->status == UCSI_ERROR) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + memcpy(&error, ucsi->ppm->data->message_in, sizeof(error));
> +
> + ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> + if (ret)
> + goto out;
> +
> + switch (error) {
> + case UCSI_ERROR_INVALID_CON_NUM:
> + ret = -ENXIO;
> + break;
> + case UCSI_ERROR_INCOMPATIBLE_PARTNER:
> + case UCSI_ERROR_CC_COMMUNICATION_ERR:
> + case UCSI_ERROR_CONTRACT_NEGOTIATION_FAIL:
> + ret = -EIO;
> + break;

This looks like you mapped all the interesting error condition
on a single error code and gave out other error codes to
conditions of lesser importance.

> + case UCSI_ERROR_DEAD_BATTERY:
> + dev_warn(ucsi->dev, "Dead Battery Condition!\n");
> + ret = -EPERM;
> + break;
> + case UCSI_ERROR_UNREGONIZED_CMD:
> + case UCSI_ERROR_INVALID_CMD_ARGUMENT:
> + default:
> + ret = -EINVAL;
> + break;
> + }
> + }
> +out:
> + ucsi->ppm->data->control = 0;
> + return ret;
> +}

[..]

> +/**
> + * ucsi_init - Initialize an UCSI Interface
> + * @ucsi: The UCSI Interface
> + *
> + * Registers all the USB Type-C ports governed by the PPM of @ucsi and enables
> + * all the notifications from the PPM.
> + */
> +int ucsi_init(struct ucsi *ucsi)
> +{
> + struct ucsi_control *ctrl = (void *)&ucsi->ppm->data->control;
> + struct ucsi_connector *con;
> + int ret;
> + int i;
> +
> + /* Enable basic notifications */
> + ctrl->cmd = UCSI_SET_NOTIFICATION_ENABLE;
> + ctrl->data = UCSI_ENABLE_NTFY_CMD_COMPLETE | UCSI_ENABLE_NTFY_ERROR;
> + ret = ucsi_run_cmd(ucsi, NULL, 0);
> + if (ret)
> + return ret;
> +
> + /* Get PPM capabilities */
> + ctrl->cmd = UCSI_GET_CAPABILITY;
> + ret = ucsi_run_cmd(ucsi, &ucsi->cap, sizeof(ucsi->cap));
> + if (ret)
> + return ret;
> +
> + ucsi->connector = kcalloc(ucsi->cap.num_connectors,
> + sizeof(struct ucsi_connector), GFP_KERNEL);
> + if (!ucsi->connector)
> + return -ENOMEM;
> +
> + for (i = 0, con = ucsi->connector; i < ucsi->cap.num_connectors;
> + i++, con++) {
> + struct typec_capability *cap = &con->typec_cap;
> + struct ucsi_connector_status constat;
> +
> + /* Get connector capability */
> + ctrl->cmd = UCSI_GET_CONNECTOR_CAPABILITY;
> + ctrl->data = i + 1;
> + ret = ucsi_run_cmd(ucsi, &con->cap, sizeof(con->cap));
> + if (ret)
> + goto err;
> +
> + /* Register the connector */
> +
> + if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP)
> + cap->type = TYPEC_PORT_DRP;
> + else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DFP)
> + cap->type = TYPEC_PORT_DFP;
> + else if (con->cap.op_mode & UCSI_CONCAP_OPMODE_UFP)
> + cap->type = TYPEC_PORT_UFP;
> +
> + cap->usb_pd = !!(ucsi->cap.attributes &
> + UCSI_CAP_ATTR_USB_PD);
> + cap->audio_accessory = !!(con->cap.op_mode &
> + UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY);
> + cap->debug_accessory = !!(con->cap.op_mode &
> + UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY);
> +
> + /* TODO: Alt modes */
> +
> + cap->dr_swap = ucsi_dr_swap;
> + cap->pr_swap = ucsi_pr_swap;
> +
> + con->port = typec_register_port(ucsi->dev, cap);
> + if (IS_ERR(con->port)) {
> + ret = PTR_ERR(con->port);
> + goto err;
> + }
> +
> + con->num = i + 1;
> + con->ucsi = ucsi;
> + INIT_WORK(&con->work, ucsi_connector_change);

You init the work later. Does this depend on the firmware
generating no events before they are enabled? Depending on firmware
is a bad idea.

> +
> + /* Check if the connector is connected */
> + if (WARN_ON(ucsi_get_constat(con, &constat) != 0))
> + continue;
> +
> + if (constat.connected)
> + ucsi_connect(con, &constat);
> + }
> +
> + /* Enable all notifications */
> + ctrl->cmd = UCSI_SET_NOTIFICATION_ENABLE;
> + ctrl->data = UCSI_ENABLE_NTFY_ALL;
> + ret = ucsi_run_cmd(ucsi, NULL, 0);
> + if (ret)
> + goto err;
> +
> + return 0;
> +err:
> + if (i > 0)
> + for (; i >= 0; i--, con--)
> + typec_unregister_port(con->port);
> +
> + kfree(ucsi->connector);
> + return ret;
> +}
> +EXPORT_SYMBOL(ucsi_init);
> +

\
 
 \ /
  Last update: 2016-02-10 12:41    [W:0.266 / U:2.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site