On 2017-08-31 17:22, Hans de Goede wrote: > class_find_device already does a get_device on the returned device. > So the device returned by of_find_mux_chip_by_node is already referenced > and we should not reference it again (and unref it on error).
Oops, yes that's right. > Also rename of_find_mux_chip_by_node to of_get_mux_chip_by_node to > make it clear that it returns a reference. By that logic, you sign up for a lot of churn, these are but a few examples: s/of_find_device_by_node/of_get_device_by_node/ s/of_find_regulator_by_node/of_get_regulator_by_node/ s/of_find_spi_controller_by_node/of_get_spi_controller_by_node/ s/of_find_i2c_device_by_node/of_get_i2c_device_by_node/ s/of_find_i2c_adapter_by_node/of_get_i2c_adapter_by_node/ But there's already an of_get_i2c_adapter_by_node (that has slightly different semantics) -> get ready for pushback. So, I don't like the rename and would be much happier with a comment above the function that the returned ref is supposed to be put away when done. Cheers, Peter > Signed-off-by: Hans de Goede <[email protected]> > --- > drivers/mux/core.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/mux/core.c b/drivers/mux/core.c > index 7aa656ac8fb5..8864cc745506 100644 > --- a/drivers/mux/core.c > +++ b/drivers/mux/core.c > @@ -433,7 +433,7 @@ static int of_dev_node_match(struct device *dev, const > void *data) > return dev->of_node == data; > } > > -static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np) > +static struct mux_chip *of_get_mux_chip_by_node(struct device_node *np) > { > struct device *dev; > > @@ -471,7 +471,7 @@ static struct mux_control *of_mux_control_get(struct > device *dev, > return ERR_PTR(ret); > } > > - mux_chip = of_find_mux_chip_by_node(args.np); > + mux_chip = of_get_mux_chip_by_node(args.np); > of_node_put(args.np); > if (!mux_chip) > return ERR_PTR(-EPROBE_DEFER); > @@ -480,6 +480,7 @@ static struct mux_control *of_mux_control_get(struct > device *dev, > (!args.args_count && (mux_chip->controllers > 1))) { > dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n", > np, args.np); > + put_device(&mux_chip->dev); > return ERR_PTR(-EINVAL); > } > > @@ -490,10 +491,10 @@ static struct mux_control *of_mux_control_get(struct > device *dev, > if (controller >= mux_chip->controllers) { > dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n", > np, controller, args.np); > + put_device(&mux_chip->dev); > return ERR_PTR(-EINVAL); > } > > - get_device(&mux_chip->dev); > return &mux_chip->mux[controller]; > } > >
