Re: [patch 18/18] PNP: convert resource options to single linked list

[Rene Herman <rene.herman@keyaccess.nl>]

On 05-06-08 00:09, Bjorn Helgaas wrote:

> ISAPNP, PNPBIOS, and ACPI describe the "possible resource settings" of

[ ... ]

Acked-by: Rene Herman <rene.herman@gmail.com>

Only minor comment:

> +static inline unsigned int pnp_independent_option(void)
> +{
> +	return 0;
> +}

I think this is a somewhat unintuitive name (the function doesn't return 
an option) and now that the pnp_dependent_option() one has been renamed 
to pnp_new_dependent_set() even the symmetry doesn't survive.

pnp_independent_option_flags? pnp_independent_flags? Or better yet, just 
literal 0? That last one unless you have some as of yet unpublished plan 
for the abstraction ofcourse but this function seems to obscure more 
than it helps any at the moment.

Even if you agree, obviously also fine as follow-up patch.

Only trivial:

> +static int pnp_assign_resources(struct pnp_dev *dev, int set)
> {

[ ... ]

> -fail:
> -	pnp_clean_resource_table(dev);
>  	mutex_unlock(&pnp_res_mutex);
> -	dbg_pnp_show_resources(dev, "after pnp_assign_resources (failed)");
> -	return 0;
> +	if (ret) {
> +		dev_dbg(&dev->dev, "pnp_assign_resources failed (%d)\n", ret);
> +		pnp_clean_resource_table(dev);
> +	} else
> +		dbg_pnp_show_resources(dev, "pnp_assign_resources succeeded");
> +	return ret;
>  }

if (ret < 0) would agree with the rest.

>  int pnp_auto_config_dev(struct pnp_dev *dev)
>  {
> -	struct pnp_option *dep;
> -	int i = 1;
> +	int i, ret = 0;

int ret; will do;

Rene.