Messages in this thread | | | Date | Wed, 23 Sep 2015 18:03:15 -0500 | From | Josh Cartwright <> | Subject | Re: [PATCH v11 3/4] add FPGA manager core |
| |
On Wed, Sep 23, 2015 at 12:10:13PM -0500, atull wrote: > On Tue, 22 Sep 2015, Josh Cartwright wrote: [..] > > > +struct fpga_manager *of_fpga_mgr_get(struct device_node *node) > > > +{ > > > + struct fpga_manager *mgr; > > > + struct device *dev; > > > + > > > + if (!node) > > > + return ERR_PTR(-EINVAL); > > > + > > > + dev = class_find_device(fpga_mgr_class, NULL, node, > > > + fpga_mgr_of_node_match); > > > + if (!dev) > > > + return ERR_PTR(-ENODEV); > > > + > > > + mgr = to_fpga_manager(dev); > > > + put_device(dev); > > > > Who's ensuring the FPGA manager device's lifetime between _get and _put? > > Well... get_device and put_device are not sufficient?
Sorry if I was too opaque.
You've just put_device()'d the only reference to the device you had here, so nothing is preventing from the device from being ripped away while it's being used.
You should remove the put_device() call here, and add a corresponding put_device() in fpga_mgr_put().
The module refcounting is also a bit strange, as you are acquiring/releasing a reference to THIS_MODULE, but you also should care about the lower-level driver's module refcount.
[..] > > > + dt_label = of_get_property(mgr->dev.of_node, "label", NULL); > > > + if (dt_label) > > > + ret = dev_set_name(&mgr->dev, "%s", dt_label); > > > + else > > > + ret = dev_set_name(&mgr->dev, "fpga%d", id); > > > > I'm wondering if an alias {} node is better for this. > > I could look into that. Is there an example of that you particularly > like for this?
Look at i2c's usage of the of_alias_*() functions.
Josh [unhandled content-type:application/pgp-signature] | |