Re: [PATCH] ACPI: Do not fail acpi_bind_one() if device is already bound correctly

[Rafael J. Wysocki]

On Friday, August 02, 2013 04:38:38 PM Toshi Kani wrote:
> On Fri, 2013-08-02 at 00:33 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > 
> > Modify acpi_bind_one() so that it doesn't fail if the device
> > represented by its first argument has already been bound to the
> > given ACPI handle (second argument), because that is not a good
> > enough reason for returning an error code.
> 
> While it seems reasonable to allow such case, I do not think we will hit
> this case under the normal scenarios.  So, I do not think we need to
> make this change now unless it actually solves Yasuaki's issue (which I
> am guessing not).

In theory it should be possible to call acpi_bind_one() twice in a row
for the same dev and the same handle without failure, that simply is
logical.  The patch may not fix any problems visible now, but returning an
error code in such a case is simply incorrect.

Thanks,
Rafael


> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > ---
> >  drivers/acpi/glue.c |   15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > Index: linux-pm/drivers/acpi/glue.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/glue.c
> > +++ linux-pm/drivers/acpi/glue.c
> > @@ -143,7 +143,10 @@ int acpi_bind_one(struct device *dev, ac
> >  	list_for_each_entry(pn, &acpi_dev->physical_node_list, node)
> >  		if (pn->dev == dev) {
> >  			dev_warn(dev, "Already associated with ACPI node\n");
> > -			goto err_free;
> > +			if (ACPI_HANDLE(dev) == handle)
> > +				retval = 0;
> > +
> > +			goto out_free;
> >  		}
> >  
> >  	/* allocate physical node id according to physical_node_id_bitmap */
> > @@ -152,7 +155,7 @@ int acpi_bind_one(struct device *dev, ac
> >  		ACPI_MAX_PHYSICAL_NODE);
> >  	if (physical_node->node_id >= ACPI_MAX_PHYSICAL_NODE) {
> >  		retval = -ENOSPC;
> > -		goto err_free;
> > +		goto out_free;
> >  	}
> >  
> >  	set_bit(physical_node->node_id, acpi_dev->physical_node_id_bitmap);
> > @@ -185,10 +188,14 @@ int acpi_bind_one(struct device *dev, ac
> >  	put_device(dev);
> >  	return retval;
> >  
> > - err_free:
> > + out_free:
> >  	mutex_unlock(&acpi_dev->physical_node_lock);
> >  	kfree(physical_node);
> > -	goto err;
> > +	if (retval)
> > +		goto err;
> > +
> > +	put_device(dev);
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(acpi_bind_one);
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/