Messages in this thread | | | Date | Sat, 14 Jun 2008 12:31:54 +0200 | From | Rene Herman <> | Subject | Re: [patch 18/18] PNP: convert resource options to single linked list |
| |
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.
| |