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

[Mark Brown]

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