Re: [PATCH 08/13] gpio: sysfs: implement class.get_dependencies()

[Mark Brown]

On Wed, Jun 17, 2015 at 03:42:18PM +0200, Tomeu Vizoso wrote:

> +static bool strends(const char *str, const char *postfix)
> +{
> +	if (strlen(str) < strlen(postfix))
> +		return false;
> +
> +	return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
> +}

This is named like (and looks like) a generic fuction, shouldn't it be
in string.h or something?

> +static void add_dependency(struct fwnode_handle *fwnode,
> +			   struct list_head *list)
> +{
> +	struct fwnode_dependency *dep;
> +
> +	dep = kzalloc(sizeof(*dep), GFP_KERNEL);
> +	if (!dep)
> +		return;
> +
> +	INIT_LIST_HEAD(&dep->dependency);
> +	dep->fwnode = fwnode;
> +
> +	list_add_tail(&dep->dependency, list);
> +}

Might be worth putting this in generic code, it looks pretty generic?  I
have to say I'm unclear what frees the returned list.

> +	if (!is_of_node(fwnode))
> +		return NULL;
> +
> +	np = of_node(fwnode);
> +	if (!np)
> +		return NULL;

Presumably the first check could be dropped?

> +	list = kzalloc(sizeof(*list), GFP_KERNEL);
> +	if (!list)
> +		return NULL;

Might it make sense for the core to allocate the head of the list and
just ask the classes to add to the list?  We're going to want to merge
the dependencies from multiple subsystems and that saves allocating
heads that may never get anything added to them.

Attachment: signature.asc
Description: Digital signature