Re: [PATCH v4 1/2] platform/chrome: cros_ec_typec: Use workqueue for port update
From: Enric Balletbo i Serra
Date: Wed Jul 01 2020 - 05:11:20 EST
Hi Prashant,
Thank you for the patch. And thanks to Guenter and Heikki for reviewing it.
On 29/6/20 23:13, Prashant Malani wrote:
> Use a work queue to call the port update routines, instead of doing it
> directly in the PD notifier callback. This will prevent other drivers
> with PD notifier callbacks from being blocked on the port update routine
> completing.
>
> Signed-off-by: Prashant Malani <pmalani@xxxxxxxxxxxx>
Applied for 5.9 both patches with the corresponding reviewed tags.
> ---
>
> Changes in v4:
> - Removed extra newline.
> - Moved both variable declarations into one line.
>
> Changes in v3:
> - Use new 100 character line length limit.
>
> Changes in v2:
> - No changes.
>
> drivers/platform/chrome/cros_ec_typec.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 0c041b79cbba..69c4118e280c 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -58,6 +58,7 @@ struct cros_typec_data {
> /* Array of ports, indexed by port number. */
> struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
> struct notifier_block nb;
> + struct work_struct port_work;
> };
>
> static int cros_typec_parse_port_props(struct typec_capability *cap,
> @@ -619,18 +620,24 @@ static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
> return 0;
> }
>
> -static int cros_ec_typec_event(struct notifier_block *nb,
> - unsigned long host_event, void *_notify)
> +static void cros_typec_port_work(struct work_struct *work)
> {
> - struct cros_typec_data *typec = container_of(nb, struct cros_typec_data,
> - nb);
> - int ret, i;
> + struct cros_typec_data *typec = container_of(work, struct cros_typec_data, port_work);
> + int i, ret;
>
> for (i = 0; i < typec->num_ports; i++) {
> ret = cros_typec_port_update(typec, i);
> if (ret < 0)
> dev_warn(typec->dev, "Update failed for port: %d\n", i);
> }
> +}
> +
> +static int cros_ec_typec_event(struct notifier_block *nb,
> + unsigned long host_event, void *_notify)
> +{
> + struct cros_typec_data *typec = container_of(nb, struct cros_typec_data, nb);
> +
> + schedule_work(&typec->port_work);
>
> return NOTIFY_OK;
> }
> @@ -689,6 +696,12 @@ static int cros_typec_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> + INIT_WORK(&typec->port_work, cros_typec_port_work);
> +
> + /*
> + * Safe to call port update here, since we haven't registered the
> + * PD notifier yet.
> + */
> for (i = 0; i < typec->num_ports; i++) {
> ret = cros_typec_port_update(typec, i);
> if (ret < 0)
>