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

Rene Herman
2008-06-14T10:40:14Z

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.