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

Andy Shevchenko
2022-09-27T12:48:49Z

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