Messages in this thread | | | From | Prashant Malani <> | Date | Mon, 29 Jun 2020 14:08:31 -0700 | Subject | Re: [PATCH v3 1/2] platform/chrome: cros_ec_typec: Use workqueue for port update |
| |
Hi Guenter,
Thanks for the comments.
On Mon, Jun 29, 2020 at 2:05 PM Guenter Roeck <groeck@google.com> wrote: > > On Mon, Jun 29, 2020 at 9:38 AM Prashant Malani <pmalani@chromium.org> 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@chromium.org> > > --- > > > > Changes in v3: > > - Use new 100 character line length limit. > > > > Changes in v2: > > - No changes. > > > > drivers/platform/chrome/cros_ec_typec.c | 25 ++++++++++++++++++++----- > > 1 file changed, 20 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > > index 0c041b79cbba..0beb62bf5adf 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,26 @@ 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 ret; > > + int i; > > > > I know I am getting picky here, but this seems like a personal > preference change. There is no "one variable declaration per line" > coding style rule.
Done. > > > 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); > > } > > +} > > + > > + > > ... but anyway, there should be no double empty lines. >
Done. > > +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 +698,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) > > -- > > 2.27.0.212.ge8ba1cc988-goog > >
| |