Re: [PATCH v1 2/2] gpio: merrifield: Convert to use acpi_dev_get_first_match()

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Thu, 2018-07-12 at 21:18 +0200, Hans de Goede wrote:
> Hi,
> 
> On 12-07-18 19:23, Andy Shevchenko wrote:
> > acpi_dev_get_first_match_name() is deprecated because it leaks
> > a reference count.
> > 
> > Convert the driver to use acpi_dev_get_first_match() instead.

> > -static const char *mrfld_gpio_get_pinctrl_dev_name(void)
> > +static const char *mrfld_gpio_get_pinctrl_dev_name(struct
> > mrfld_gpio *priv)
> >   {
> > -	const char *dev_name =
> > acpi_dev_get_first_match_name("INTC1002", NULL, -1);
> > -	return dev_name ? dev_name : "pinctrl-merrifield";
> > +	struct acpi_device *adev;
> > +	const char *dev_name;
> > +
> > +	adev = acpi_dev_get_first_match("INTC1002", NULL, -1);
> > +	if (!adev)
> > +		return "pinctrl-merrifield";
> > +
> > +	dev_name = devm_kstrdup(priv->dev, acpi_dev_name(adev),
> > GFP_KERNEL);
> > +	put_device(&adev->dev);
> > +	return dev_name;
> >   }
> 
> Hmm, this looks complicated(ish)

Yes, I agree. Though as I told this is dictated by nature of
bus_find_device(). Moreover, we can't cover all, especially single time
used, cases by introducing yet another acpi_dev_get_first_match_*() type
of API.

>  maybe add a new
> devm_acpi_dev_get_first_match_name() which takes a dev for the
> devm_kstrdup and do all this inside there.

Yes, that would make sense because of existing users. But in many cases
this memory would be used just for one time. Basically for users you
mentioned below we don't need to do any allocations till we have a
reference to struct device.

Another possibility is to provide a buffer to the function. (I tried to
mock up and had been unsatisfied with the result, though)

>  There are
> quite a few users of acpi_dev_get_first_match_name() under
> sound/soc/intel/boards and converting them all like this will
> be cumbersome.

Yes, and I don't like to blow them up. But I don't see any better
solution for now.

>  IMHO we need a better API then the acpi_dev_get_first_match()
> you are proposing.

I'm all hears.

Take into account the flexibility of new API as well. (Locally I used to
have prototyping something which actually requires handle to be
returned)


-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html