lkml.org 
[lkml]   [2020]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 7/7] i2c: core: hand over reserved devices when requesting ancillary addresses

> > @@ -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!

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2020-03-13 13:42    [W:0.091 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site