Re: [PATCH v3 3/5] platform/x86: int3472: Support multiple clock consumers

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Sep 27, 2022 at 03:47:17PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 22, 2022 at 12:04:37AM +0100, Daniel Scally wrote:
> > At present, the tps68470.c only supports a single clock consumer when
> > passing platform data to the clock driver. In some devices multiple
> > sensors depend on the clock provided by a single TPS68470 and so all
> > need to be able to acquire the clock. Support passing multiple
> > consumers as platform data.

...

> > +static int
> > +skl_int3472_fill_clk_pdata(struct device *dev, struct tps68470_clk_platform_data **clk_pdata)
> > +{
> > +	struct acpi_device *adev = ACPI_COMPANION(dev);
> > +	struct acpi_device *consumer;
> > +	unsigned int n_consumers = 0;
> > +	const char *sensor_name;
> > +	unsigned int i = 0;
> > +
> > +	for_each_acpi_consumer_dev(adev, consumer)
> > +		n_consumers++;
> 
> Here no put for consumer (and IIUC it's correct).
> 
> > +
> 
> (Also no need to have a blank line here, the condition is tighten to
>  the for-loop.)
> 
> > +	if (!n_consumers) {
> > +		dev_err(dev, "INT3472 seems to have no dependents\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	*clk_pdata = devm_kzalloc(dev, struct_size(*clk_pdata, consumers, n_consumers),
> > +				  GFP_KERNEL);
> > +	if (!*clk_pdata)
> > +		return -ENOMEM;
> > +
> > +	(*clk_pdata)->n_consumers = n_consumers;
> > +	i = 0;
> > +
> > +	for_each_acpi_consumer_dev(adev, consumer) {
> > +		sensor_name = devm_kasprintf(dev, GFP_KERNEL, I2C_DEV_NAME_FORMAT,
> > +					     acpi_dev_name(consumer));
> > +		if (!sensor_name)
> > +			return -ENOMEM;
> > +
> > +		(*clk_pdata)->consumers[i].consumer_dev_name = sensor_name;
> > +		i++;
> > +	}
> 
> > +	acpi_dev_put(consumer);

> Why is it here?

Now I got it, you need to move it to the error path before returning from
inside the for-loop.

> > +	return n_consumers;
> > +}

-- 
With Best Regards,
Andy Shevchenko