Re: [PATCH v6 3/8] usb: typec: ucsi: Implement ChromeOS UCSI driver

Dmitry Baryshkov
2024-09-12T12:58:17Z

On Tue, Sep 10, 2024 at 10:15:22AM GMT, Łukasz Bartosik wrote:
> From: Pavan Holla <pholla@chromium.org>
>
> Implementation of a UCSI transport driver for ChromeOS.
> This driver will be loaded if the ChromeOS EC implements a PPM.
>
> Signed-off-by: Pavan Holla <pholla@chromium.org>
> Co-developed-by: Łukasz Bartosik <ukaszb@chromium.org>
> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> ---
> MAINTAINERS | 7 +
> drivers/usb/typec/ucsi/Kconfig | 13 ++
> drivers/usb/typec/ucsi/Makefile | 1 +
> drivers/usb/typec/ucsi/cros_ec_ucsi.c | 273 ++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+)
> create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi.c
>

[...]

> +
> +static int cros_ucsi_sync_control(struct ucsi *ucsi, u64 cmd)
> +{
> + struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
> + bool ack = UCSI_COMMAND(cmd) == UCSI_ACK_CC_CI;
> + int ret;
> +
> + if (ack)
> + set_bit(ACK_PENDING, &udata->flags);
> + else
> + set_bit(COMMAND_PENDING, &udata->flags);
> +
> + ret = cros_ucsi_async_control(ucsi, cmd);
> + if (ret)
> + goto out;
> +
> + if (!wait_for_completion_timeout(&udata->complete, WRITE_TMO_MS))
> + ret = -ETIMEDOUT;
> +
> +out:
> + if (ack)
> + clear_bit(ACK_PENDING, &udata->flags);
> + else
> + clear_bit(COMMAND_PENDING, &udata->flags);
> + return ret;
> +}
> +
> +struct ucsi_operations cros_ucsi_ops = {
> + .read_version = cros_ucsi_read_version,
> + .read_cci = cros_ucsi_read_cci,
> + .read_message_in = cros_ucsi_read_message_in,
> + .async_control = cros_ucsi_async_control,
> + .sync_control = cros_ucsi_sync_control,

Please use ucsi_sync_control_common instead.

> +};
> +
> +static void cros_ucsi_work(struct work_struct *work)
> +{
> + struct cros_ucsi_data *udata = container_of(work, struct cros_ucsi_data, work);
> + u32 cci;
> +
> + if (cros_ucsi_read_cci(udata->ucsi, &cci))
> + return;
> +
> + if (UCSI_CCI_CONNECTOR(cci))
> + ucsi_connector_change(udata->ucsi, UCSI_CCI_CONNECTOR(cci));
> +
> + if (cci & UCSI_CCI_ACK_COMPLETE &&
> + test_and_clear_bit(ACK_PENDING, &udata->flags))
> + complete(&udata->complete);
> + if (cci & UCSI_CCI_COMMAND_COMPLETE &&
> + test_and_clear_bit(COMMAND_PENDING, &udata->flags))
> + complete(&udata->complete);

ucsi_notify_common(). Why are you ignoring these functions?

--
With best wishes
Dmitry