Re: [RFC PATCH 7/7] i2c: core: hand over reserved devices when requesting ancillary addresses

[Wolfram Sang]

> > @@ -984,7 +986,21 @@ struct i2c_client *i2c_new_ancillary_device(struct i2c_client *client,
> >                         of_property_read_u32_index(np, "reg", i, &addr);
> >         }
> >
> > -       dev_dbg(&client->adapter->dev, "Address for %s : 0x%x\n", name, addr);
> > +       dev_info(adapter_dev, "Address for %s : 0x%x\n", name, addr);
> > +
> > +       /* No need to scan muxes, siblings must sit on the same adapter */
> > +       reserved_dev = device_find_child(adapter_dev, &addr, __i2c_check_addr_busy);
> > +       reserved_client = i2c_verify_client(reserved_dev);
> > +
> > +       if (reserved_client) {
> > +               if (reserved_client->dev.of_node != np ||
> > +                   strcmp(reserved_client->name, I2C_RESERVED_DRV_NAME) != 0)
> > +                       return ERR_PTR(-EBUSY);
> 
> Missing put_device(reserved_dev).

Actually, I think the code could even be like this:

	struct i2c_client *reserved_client = NULL;

	...

	reserved_dev = device_find_child(adapter_dev, &addr, __i2c_check_addr_busy);
	if (reserved_dev) {
		reserved_np = reserved_dev->of_node;
		reserved_client = i2c_verify_client(reserved_dev);
		put_device(reserved_dev);
	}

	if (reserved_client) {
		if (reserved_np != np ||
		    strcmp(reserved_client->name, I2C_RESERVED_DRV_NAME) != 0)
			return ERR_PTR(-EBUSY);

		strlcpy(reserved_client->name, I2C_DUMMY_DRV_NAME, sizeof(client->name));
		return reserved_client;
	}

	return i2c_new_dummy_device(client->adapter, addr);

We put the device early - as soon we don't access the struct anymore. I
think we don't need the refcnt any further because what we are doing
here is to hand over the initial refcnt from the core to the requesting
driver. We turn the device from "reserved" (internally managed) to
"dummy" (managed by the driver). So, I think the code is okay regarding
the struct device. I will have a second look when it comes to
concurrency problems regarding the struct i2c_client, though.

> (perhaps i2c_verify_client() checking dev was not such a great idea, as
>  callers need to act on dev && !verified anyway?)

Yeah, since I refactored the ACPI code as well, patch 1 from this series
can probably go.

Thanks again for your review, Geert!

Attachment: signature.asc
Description: PGP signature