Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus

From: Mark Brown
Date: Wed Jun 17 2015 - 09:17:04 EST


On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:

> + if (status) {
> + slim_dev->driver = NULL;
> + } else if (driver->device_up) {
> + ctrl = slim_dev->ctrl;
> + queue_work(ctrl->wq, &slim_dev->wd);
> + }

Nothing ever cleans this work up if it didn't manage to run or
complete.

> +static void slim_report(struct work_struct *work)
> +{
> + struct slim_driver *sbdrv;
> + struct slim_device *sbdev = container_of(work, struct slim_device, wd);
> +
> + if (!sbdev->dev.driver)
> + return;

So we just forget about the device if we don't have a driver for it?

> + /* check if device-up or down needs to be called */
> + if ((!sbdev->reported && !sbdev->notified) ||
> + (sbdev->reported && sbdev->notified))
> + return;

No locking here?

> +/**
> + * slim_ctrl_add_boarddevs: Add devices registered by board-info
> + * @ctrl: Controller to which these devices are to be added to.
> + * This API is called by controller when it is up and running.
> + * If devices on a controller were registered before controller,
> + * this will make sure that they get probed when controller is up.
> + */
> +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)

My concerns about the split here still remain.

Attachment: signature.asc
Description: Digital signature