Re: [PATCH 0/7] convert semaphore to mutex in struct class

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

Dave Young wrote:
> This is the first one of the series about driver core changes.

Please always provide kerneldoc comments when you add new API elements;
here: exported functions.

It's unfortunate that the driver core's API isn't fully documented yet,
and you shouldn't make it worse.

That's only my personal opinion as one API user though.  But others
might agree.  Among else, things worth documenting are return values
after errors, side effects (!), constraints on the calling context if
there are any special constraints.

I assume you didn't write documentation yet because you need general
feedback first.

...
> +struct device *class_find_device(struct class *class, void *data,
> +				   int (*match)(struct device *, void *))
> +{
> +	struct device *dev;
> +
> +	if (!class)
> +		return NULL;
> +
> +	mutex_lock(&class->mutex);
> +	list_for_each_entry(dev, &class->devices, node)
> +		if (match(dev, data) && get_device(dev))
> +			break;
> +	mutex_unlock(&class->mutex);
> +
> +	return dev;
> +}

What is returned if there was no match?
What if there was a match but get_ failed?

...
> +struct class_device *class_find_child(struct class *class, void *data,
> +				   int (*match)(struct class_device *, void *))
> +{
...
> +	mutex_lock(&class->mutex);
> +	list_for_each_entry(dev, &class->children, node)
> +		if (match(dev, data) && class_device_get(dev))
> +			break;
> +	mutex_unlock(&class->mutex);
> +
> +	return dev;
> +}

Here too?
-- 
Stefan Richter
-=====-==--- ---= -=-=-
http://arcgraph.de/sr/