[PATCH v3 0/5] Add multiple-consumer support to int3472-tps68470 driver

[PATCH v3 0/5] Add multiple-consumer support to int3472-tps68470 driver

[Daniel Scally]

Hello all

At the moment there are a few places in the int3472-tps68470 driver that are
limited to just working with a single consuming device dependent on the PMIC.
There are systems where multiple camera sensors share a single TPS68470, so
we need to extend the driver to support them. This requires a couple of tweaks
to the ACPI functions to fetch dependent devices, which also assumes that only
a single dependent will be found.

The v2 for this series was some time ago...it's kept falling to the back of my
to-do list so I've only just gotten round to it; sorry about that. v2 here:

https://lore.kernel.org/linux-acpi/20220327161344.50477-1-djrscally@gmail.com/

Thanks
Dan

Daniel Scally (5):
  ACPI: scan: Add acpi_dev_get_next_consumer_dev()
  ACPI: bus: Add iterator for dependent devices
  platform/x86: int3472: Support multiple clock consumers
  platform/x86: int3472: Support multiple gpio lookups in board data
  platform/x86: int3472: Add board data for Surface Go2 IR camera

 drivers/acpi/scan.c                           | 40 +++++++---
 drivers/clk/clk-tps68470.c                    | 13 +++-
 drivers/platform/x86/intel/int3472/common.c   |  2 +-
 drivers/platform/x86/intel/int3472/tps68470.c | 76 ++++++++++++++++---
 drivers/platform/x86/intel/int3472/tps68470.h |  3 +-
 .../x86/intel/int3472/tps68470_board_data.c   | 54 ++++++++++++-
 include/acpi/acpi_bus.h                       | 15 +++-
 include/linux/platform_data/tps68470.h        |  7 +-
 8 files changed, 177 insertions(+), 33 deletions(-)

-- 
2.25.1

[PATCH v3 1/5] ACPI: scan: Add acpi_dev_get_next_consumer_dev()

[Daniel Scally]

In commit b83e2b306736 ("ACPI: scan: Add function to fetch dependent
of ACPI device") we added a means of fetching the first device to
declare itself dependent on another ACPI device in the _DEP method.
One assumption in that patch was that there would only be a single
consuming device, but this has not held.

Replace that function with a new function that fetches the next consumer
of a supplier device. Where no "previous" consumer is passed in, it
behaves identically to the original function.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v2:

	- Separated adev_p and adev instead of relying on a cast (Rafael)
	- Used a separate if block to check for adev == start in
	  acpi_dev_get_next_consumer_dev() (Rafael)

 drivers/acpi/scan.c                         | 40 +++++++++++++++------
 drivers/platform/x86/intel/int3472/common.c |  2 +-
 include/acpi/acpi_bus.h                     |  4 ++-
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 42cec8120f18..12ac2f3ae5d2 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2235,9 +2235,22 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
 	return 0;
 }
 
-static int acpi_dev_get_first_consumer_dev_cb(struct acpi_dep_data *dep, void *data)
+static int acpi_dev_get_next_consumer_dev_cb(struct acpi_dep_data *dep, void *data)
 {
-	struct acpi_device *adev;
+	struct acpi_device **adev_p = data;
+	struct acpi_device *adev = *adev_p;
+
+	/*
+	 * If we're passed a 'previous' consumer device then we need to skip
+	 * any consumers until we meet the previous one, and then NULL @data
+	 * so the next one can be returned.
+	 */
+	if (adev) {
+		if (dep->consumer == adev->handle)
+			*adev_p = NULL;
+
+		return 0;
+	}
 
 	adev = acpi_bus_get_acpi_device(dep->consumer);
 	if (adev) {
@@ -2368,25 +2381,32 @@ bool acpi_dev_ready_for_enumeration(const struct acpi_device *device)
 EXPORT_SYMBOL_GPL(acpi_dev_ready_for_enumeration);
 
 /**
- * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier
+ * acpi_dev_get_next_consumer_dev - Return the next adev dependent on @supplier
  * @supplier: Pointer to the dependee device
+ * @start: Pointer to the current dependent device
  *
- * Returns the first &struct acpi_device which declares itself dependent on
+ * Returns the next &struct acpi_device which declares itself dependent on
  * @supplier via the _DEP buffer, parsed from the acpi_dep_list.
  *
- * The caller is responsible for putting the reference to adev when it is no
- * longer needed.
+ * If the returned adev is not passed as @start to this function, the caller is
+ * responsible for putting the reference to adev when it is no longer needed.
  */
-struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier)
+struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
+						   struct acpi_device *start)
 {
-	struct acpi_device *adev = NULL;
+	struct acpi_device *adev = start;
 
 	acpi_walk_dep_device_list(supplier->handle,
-				  acpi_dev_get_first_consumer_dev_cb, &adev);
+				  acpi_dev_get_next_consumer_dev_cb, &adev);
+
+	acpi_dev_put(start);
+
+	if (adev == start)
+		return NULL;
 
 	return adev;
 }
-EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
+EXPORT_SYMBOL_GPL(acpi_dev_get_next_consumer_dev);
 
 /**
  * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
diff --git a/drivers/platform/x86/intel/int3472/common.c b/drivers/platform/x86/intel/int3472/common.c
index 77cf058e4168..9db2bb0bbba4 100644
--- a/drivers/platform/x86/intel/int3472/common.c
+++ b/drivers/platform/x86/intel/int3472/common.c
@@ -62,7 +62,7 @@ int skl_int3472_get_sensor_adev_and_name(struct device *dev,
 	struct acpi_device *sensor;
 	int ret = 0;
 
-	sensor = acpi_dev_get_first_consumer_dev(adev);
+	sensor = acpi_dev_get_next_consumer_dev(adev, NULL);
 	if (!sensor) {
 		dev_err(dev, "INT3472 seems to have no dependents.\n");
 		return -ENODEV;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e7d27373ff71..bf0886dca6eb 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -736,7 +736,9 @@ bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const ch
 
 void acpi_dev_clear_dependencies(struct acpi_device *supplier);
 bool acpi_dev_ready_for_enumeration(const struct acpi_device *device);
-struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier);
+struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
+						   struct acpi_device *start);
+
 struct acpi_device *
 acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
 struct acpi_device *
-- 
2.25.1

[PATCH v3 2/5] ACPI: bus: Add iterator for dependent devices

[Daniel Scally]

Add a helper macro to iterate over ACPI devices that are flagged
as consumers of an initial supplier ACPI device.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v2:

	- None

 include/acpi/acpi_bus.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bf0886dca6eb..abce6f0219ea 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -739,6 +739,17 @@ bool acpi_dev_ready_for_enumeration(const struct acpi_device *device);
 struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
 						   struct acpi_device *start);
 
+/**
+ * for_each_acpi_consumer_dev - iterate over the consumer ACPI devices for a
+ *				given supplier
+ * @supplier: Pointer to the supplier's ACPI device
+ * @consumer: Pointer to &struct acpi_device to hold the consumer, initially NULL
+ */
+#define for_each_acpi_consumer_dev(supplier, consumer)			\
+	for (consumer = acpi_dev_get_next_consumer_dev(supplier, NULL);	\
+	     consumer;							\
+	     consumer = acpi_dev_get_next_consumer_dev(supplier, consumer))
+
 struct acpi_device *
 acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
 struct acpi_device *
-- 
2.25.1

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

[Daniel Scally]

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.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v2:

	- None

 drivers/clk/clk-tps68470.c                    | 13 ++--
 drivers/platform/x86/intel/int3472/tps68470.c | 59 ++++++++++++++++---
 include/linux/platform_data/tps68470.h        |  7 ++-
 3 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-tps68470.c b/drivers/clk/clk-tps68470.c
index e5fbefd6ac2d..38f44b5b9b1b 100644
--- a/drivers/clk/clk-tps68470.c
+++ b/drivers/clk/clk-tps68470.c
@@ -200,7 +200,9 @@ static int tps68470_clk_probe(struct platform_device *pdev)
 		.flags = CLK_SET_RATE_GATE,
 	};
 	struct tps68470_clkdata *tps68470_clkdata;
+	struct tps68470_clk_consumer *consumer;
 	int ret;
+	int i;
 
 	tps68470_clkdata = devm_kzalloc(&pdev->dev, sizeof(*tps68470_clkdata),
 					GFP_KERNEL);
@@ -223,10 +225,13 @@ static int tps68470_clk_probe(struct platform_device *pdev)
 		return ret;
 
 	if (pdata) {
-		ret = devm_clk_hw_register_clkdev(&pdev->dev,
-						  &tps68470_clkdata->clkout_hw,
-						  pdata->consumer_con_id,
-						  pdata->consumer_dev_name);
+		for (i = 0; i < pdata->n_consumers; i++) {
+			consumer = &pdata->consumers[i];
+			ret = devm_clk_hw_register_clkdev(&pdev->dev,
+							  &tps68470_clkdata->clkout_hw,
+							  consumer->consumer_con_id,
+							  consumer->consumer_dev_name);
+		}
 	}
 
 	return ret;
diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index 22f61b47f9e5..8a684030933d 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Author: Dan Scally <djrscally@gmail.com> */
 
+#include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/mfd/core.h>
@@ -95,20 +96,64 @@ static int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
 	return DESIGNED_FOR_WINDOWS;
 }
 
+/*
+ * Return the size of the flexible array member, because we'll need that later
+ * on to pass .pdata_size to cells.
+ */
+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++;
+
+	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);
+
+	return n_consumers;
+}
+
 static int skl_int3472_tps68470_probe(struct i2c_client *client)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
 	const struct int3472_tps68470_board_data *board_data;
-	struct tps68470_clk_platform_data clk_pdata = {};
+	struct tps68470_clk_platform_data *clk_pdata;
 	struct mfd_cell *cells;
 	struct regmap *regmap;
+	int n_consumers;
 	int device_type;
 	int ret;
 
-	ret = skl_int3472_get_sensor_adev_and_name(&client->dev, NULL,
-						   &clk_pdata.consumer_dev_name);
-	if (ret)
-		return ret;
+	n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
+	if (n_consumers < 0)
+		return n_consumers;
 
 	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
 	if (IS_ERR(regmap)) {
@@ -142,8 +187,8 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
 		 * the clk + regulators must be ready when this happens.
 		 */
 		cells[0].name = "tps68470-clk";
-		cells[0].platform_data = &clk_pdata;
-		cells[0].pdata_size = sizeof(clk_pdata);
+		cells[0].platform_data = clk_pdata;
+		cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
 		cells[1].name = "tps68470-regulator";
 		cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
 		cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
diff --git a/include/linux/platform_data/tps68470.h b/include/linux/platform_data/tps68470.h
index 126d082c3f2e..e605a2cab07f 100644
--- a/include/linux/platform_data/tps68470.h
+++ b/include/linux/platform_data/tps68470.h
@@ -27,9 +27,14 @@ struct tps68470_regulator_platform_data {
 	const struct regulator_init_data *reg_init_data[TPS68470_NUM_REGULATORS];
 };
 
-struct tps68470_clk_platform_data {
+struct tps68470_clk_consumer {
 	const char *consumer_dev_name;
 	const char *consumer_con_id;
 };
 
+struct tps68470_clk_platform_data {
+	unsigned int n_consumers;
+	struct tps68470_clk_consumer consumers[];
+};
+
 #endif
-- 
2.25.1

[PATCH v3 4/5] platform/x86: int3472: Support multiple gpio lookups in board data

[Daniel Scally]

Currently, we only support passing a single gpiod_lookup_table as part
of the board data for the tps68470 driver. This carries the implicit
assumption that each TPS68470 device will only support a single
sensor, which does not hold true.

Extend the code to support the possibility of multiple sensors each
having a gpiod_lookup_table, and opportunistically add the lookup
table for the Surface Go line's IR camera.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v2:

	- None

 drivers/platform/x86/intel/int3472/tps68470.c | 17 ++++++++++-----
 drivers/platform/x86/intel/int3472/tps68470.h |  3 ++-
 .../x86/intel/int3472/tps68470_board_data.c   | 21 ++++++++++++++++---
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index 8a684030933d..49fc379fe680 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -150,6 +150,7 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
 	int n_consumers;
 	int device_type;
 	int ret;
+	int i;
 
 	n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
 	if (n_consumers < 0)
@@ -194,15 +195,18 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
 		cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
 		cells[2].name = "tps68470-gpio";
 
-		gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
+		for (i = 0; i < board_data->n_gpiod_lookups; i++)
+			gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_tables[i]);
 
 		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
 					   cells, TPS68470_WIN_MFD_CELL_COUNT,
 					   NULL, 0, NULL);
 		kfree(cells);
 
-		if (ret)
-			gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
+		if (ret) {
+			for (i = 0; i < board_data->n_gpiod_lookups; i++)
+				gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_tables[i]);
+		}
 
 		break;
 	case DESIGNED_FOR_CHROMEOS:
@@ -226,10 +230,13 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
 static int skl_int3472_tps68470_remove(struct i2c_client *client)
 {
 	const struct int3472_tps68470_board_data *board_data;
+	int i;
 
 	board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
-	if (board_data)
-		gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
+	if (board_data) {
+		for (i = 0; i < board_data->n_gpiod_lookups; i++)
+			gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_tables[i]);
+	}
 
 	return 0;
 }
diff --git a/drivers/platform/x86/intel/int3472/tps68470.h b/drivers/platform/x86/intel/int3472/tps68470.h
index cfd33eb62740..35915e701593 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.h
+++ b/drivers/platform/x86/intel/int3472/tps68470.h
@@ -16,8 +16,9 @@ struct tps68470_regulator_platform_data;
 
 struct int3472_tps68470_board_data {
 	const char *dev_name;
-	struct gpiod_lookup_table *tps68470_gpio_lookup_table;
 	const struct tps68470_regulator_platform_data *tps68470_regulator_pdata;
+	unsigned int n_gpiod_lookups;
+	struct gpiod_lookup_table *tps68470_gpio_lookup_tables[];
 };
 
 const struct int3472_tps68470_board_data *int3472_tps68470_get_board_data(const char *dev_name);
diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
index 525f09a3b5ff..e6cc8f40f5af 100644
--- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
+++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
@@ -96,7 +96,7 @@ static const struct tps68470_regulator_platform_data surface_go_tps68470_pdata =
 	},
 };
 
-static struct gpiod_lookup_table surface_go_tps68470_gpios = {
+static struct gpiod_lookup_table surface_go_int347a_gpios = {
 	.dev_id = "i2c-INT347A:00",
 	.table = {
 		GPIO_LOOKUP("tps68470-gpio", 9, "reset", GPIO_ACTIVE_LOW),
@@ -105,16 +105,31 @@ static struct gpiod_lookup_table surface_go_tps68470_gpios = {
 	}
 };
 
+static struct gpiod_lookup_table surface_go_int347e_gpios = {
+	.dev_id = "i2c-INT347E:00",
+	.table = {
+		GPIO_LOOKUP("tps68470-gpio", 5, "enable", GPIO_ACTIVE_HIGH),
+		{ }
+	}
+};
+
 static const struct int3472_tps68470_board_data surface_go_tps68470_board_data = {
 	.dev_name = "i2c-INT3472:05",
-	.tps68470_gpio_lookup_table = &surface_go_tps68470_gpios,
 	.tps68470_regulator_pdata = &surface_go_tps68470_pdata,
+	.n_gpiod_lookups = 2,
+	.tps68470_gpio_lookup_tables = {
+		&surface_go_int347a_gpios,
+		&surface_go_int347e_gpios,
+	},
 };
 
 static const struct int3472_tps68470_board_data surface_go3_tps68470_board_data = {
 	.dev_name = "i2c-INT3472:01",
-	.tps68470_gpio_lookup_table = &surface_go_tps68470_gpios,
 	.tps68470_regulator_pdata = &surface_go_tps68470_pdata,
+	.n_gpiod_lookups = 1,
+	.tps68470_gpio_lookup_tables = {
+		&surface_go_int347a_gpios
+	},
 };
 
 static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
-- 
2.25.1

[PATCH v3 5/5] platform/x86: int3472: Add board data for Surface Go2 IR camera

[Daniel Scally]

Add the board data describing the regulators for the Microsoft
Surface Go line's IR camera.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v2:

	- None

 .../x86/intel/int3472/tps68470_board_data.c   | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
index e6cc8f40f5af..309eab9c0558 100644
--- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
+++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
@@ -30,6 +30,15 @@ static struct regulator_consumer_supply int347a_vcm_consumer_supplies[] = {
 static struct regulator_consumer_supply int347a_vsio_consumer_supplies[] = {
 	REGULATOR_SUPPLY("dovdd", "i2c-INT347A:00"),
 	REGULATOR_SUPPLY("vsio", "i2c-INT347A:00-VCM"),
+	REGULATOR_SUPPLY("vddd", "i2c-INT347E:00"),
+};
+
+static struct regulator_consumer_supply int347a_aux1_consumer_supplies[] = {
+	REGULATOR_SUPPLY("vdda", "i2c-INT347E:00"),
+};
+
+static struct regulator_consumer_supply int347a_aux2_consumer_supplies[] = {
+	REGULATOR_SUPPLY("vdddo", "i2c-INT347E:00"),
 };
 
 static const struct regulator_init_data surface_go_tps68470_core_reg_init_data = {
@@ -86,6 +95,28 @@ static const struct regulator_init_data surface_go_tps68470_vsio_reg_init_data =
 	.consumer_supplies = int347a_vsio_consumer_supplies,
 };
 
+static const struct regulator_init_data surface_go_tps68470_aux1_reg_init_data = {
+	.constraints = {
+		.min_uV = 2815200,
+		.max_uV = 2815200,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = ARRAY_SIZE(int347a_aux1_consumer_supplies),
+	.consumer_supplies = int347a_aux1_consumer_supplies,
+};
+
+static const struct regulator_init_data surface_go_tps68470_aux2_reg_init_data = {
+	.constraints = {
+		.min_uV = 1800600,
+		.max_uV = 1800600,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = ARRAY_SIZE(int347a_aux2_consumer_supplies),
+	.consumer_supplies = int347a_aux2_consumer_supplies,
+};
+
 static const struct tps68470_regulator_platform_data surface_go_tps68470_pdata = {
 	.reg_init_data = {
 		[TPS68470_CORE] = &surface_go_tps68470_core_reg_init_data,
@@ -93,6 +124,8 @@ static const struct tps68470_regulator_platform_data surface_go_tps68470_pdata =
 		[TPS68470_VCM]  = &surface_go_tps68470_vcm_reg_init_data,
 		[TPS68470_VIO] = &surface_go_tps68470_vio_reg_init_data,
 		[TPS68470_VSIO] = &surface_go_tps68470_vsio_reg_init_data,
+		[TPS68470_AUX1] = &surface_go_tps68470_aux1_reg_init_data,
+		[TPS68470_AUX2] = &surface_go_tps68470_aux2_reg_init_data,
 	},
 };
 
-- 
2.25.1

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

[Hans de Goede]

Hi,

On 9/22/22 01:04, 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.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Note this one already has a review + ack from Stephen Boyd for
merging this through the pdx86 tree (from v1 of the series):

https://lore.kernel.org/platform-driver-x86/20220225004943.AA8EDC340EF@smtp.kernel.org/


Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>


Regards,

Hans





> ---
> Changes since v2:
> 
> 	- None
> 
>  drivers/clk/clk-tps68470.c                    | 13 ++--
>  drivers/platform/x86/intel/int3472/tps68470.c | 59 ++++++++++++++++---
>  include/linux/platform_data/tps68470.h        |  7 ++-
>  3 files changed, 67 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/clk-tps68470.c b/drivers/clk/clk-tps68470.c
> index e5fbefd6ac2d..38f44b5b9b1b 100644
> --- a/drivers/clk/clk-tps68470.c
> +++ b/drivers/clk/clk-tps68470.c
> @@ -200,7 +200,9 @@ static int tps68470_clk_probe(struct platform_device *pdev)
>  		.flags = CLK_SET_RATE_GATE,
>  	};
>  	struct tps68470_clkdata *tps68470_clkdata;
> +	struct tps68470_clk_consumer *consumer;
>  	int ret;
> +	int i;
>  
>  	tps68470_clkdata = devm_kzalloc(&pdev->dev, sizeof(*tps68470_clkdata),
>  					GFP_KERNEL);
> @@ -223,10 +225,13 @@ static int tps68470_clk_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	if (pdata) {
> -		ret = devm_clk_hw_register_clkdev(&pdev->dev,
> -						  &tps68470_clkdata->clkout_hw,
> -						  pdata->consumer_con_id,
> -						  pdata->consumer_dev_name);
> +		for (i = 0; i < pdata->n_consumers; i++) {
> +			consumer = &pdata->consumers[i];
> +			ret = devm_clk_hw_register_clkdev(&pdev->dev,
> +							  &tps68470_clkdata->clkout_hw,
> +							  consumer->consumer_con_id,
> +							  consumer->consumer_dev_name);
> +		}
>  	}
>  
>  	return ret;
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index 22f61b47f9e5..8a684030933d 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Author: Dan Scally <djrscally@gmail.com> */
>  
> +#include <linux/acpi.h>
>  #include <linux/i2c.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/core.h>
> @@ -95,20 +96,64 @@ static int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
>  	return DESIGNED_FOR_WINDOWS;
>  }
>  
> +/*
> + * Return the size of the flexible array member, because we'll need that later
> + * on to pass .pdata_size to cells.
> + */
> +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++;
> +
> +	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);
> +
> +	return n_consumers;
> +}
> +
>  static int skl_int3472_tps68470_probe(struct i2c_client *client)
>  {
>  	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
>  	const struct int3472_tps68470_board_data *board_data;
> -	struct tps68470_clk_platform_data clk_pdata = {};
> +	struct tps68470_clk_platform_data *clk_pdata;
>  	struct mfd_cell *cells;
>  	struct regmap *regmap;
> +	int n_consumers;
>  	int device_type;
>  	int ret;
>  
> -	ret = skl_int3472_get_sensor_adev_and_name(&client->dev, NULL,
> -						   &clk_pdata.consumer_dev_name);
> -	if (ret)
> -		return ret;
> +	n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> +	if (n_consumers < 0)
> +		return n_consumers;
>  
>  	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>  	if (IS_ERR(regmap)) {
> @@ -142,8 +187,8 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
>  		 * the clk + regulators must be ready when this happens.
>  		 */
>  		cells[0].name = "tps68470-clk";
> -		cells[0].platform_data = &clk_pdata;
> -		cells[0].pdata_size = sizeof(clk_pdata);
> +		cells[0].platform_data = clk_pdata;
> +		cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
>  		cells[1].name = "tps68470-regulator";
>  		cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
>  		cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
> diff --git a/include/linux/platform_data/tps68470.h b/include/linux/platform_data/tps68470.h
> index 126d082c3f2e..e605a2cab07f 100644
> --- a/include/linux/platform_data/tps68470.h
> +++ b/include/linux/platform_data/tps68470.h
> @@ -27,9 +27,14 @@ struct tps68470_regulator_platform_data {
>  	const struct regulator_init_data *reg_init_data[TPS68470_NUM_REGULATORS];
>  };
>  
> -struct tps68470_clk_platform_data {
> +struct tps68470_clk_consumer {
>  	const char *consumer_dev_name;
>  	const char *consumer_con_id;
>  };
>  
> +struct tps68470_clk_platform_data {
> +	unsigned int n_consumers;
> +	struct tps68470_clk_consumer consumers[];
> +};
> +
>  #endif

Re: [PATCH v3 0/5] Add multiple-consumer support to int3472-tps68470 driver

[Hans de Goede]

Hi All,

On 9/22/22 01:04, Daniel Scally wrote:
> Hello all
> 
> At the moment there are a few places in the int3472-tps68470 driver that are
> limited to just working with a single consuming device dependent on the PMIC.
> There are systems where multiple camera sensors share a single TPS68470, so
> we need to extend the driver to support them. This requires a couple of tweaks
> to the ACPI functions to fetch dependent devices, which also assumes that only
> a single dependent will be found.
> 
> The v2 for this series was some time ago...it's kept falling to the back of my
> to-do list so I've only just gotten round to it; sorry about that. v2 here:
> 
> https://lore.kernel.org/linux-acpi/20220327161344.50477-1-djrscally@gmail.com/

Rafael, I would like to merge this through the pdx86 tree may I have your
ack for patches 1 + 2 for this. As a reminder (since it has been a while)
here are your review remarks to v2 of patch 1:

https://lore.kernel.org/platform-driver-x86/CAJZ5v0i2ciLHP-=8eQcZc0v0xCzhKHKpxLC=Kgv6W5E_5=HQJA@mail.gmail.com/

(which both seem to have been addressed)

AFAICT you did not have any remarks for v2 of patch 2.

Regards,

Hans

p.s.

Dan, if I want to give the IR cam a test run on my own Surface Go (version 1)
I guess I may need a sensor driver? Where can I find that sensor driver and
what do I need in userspace to test this ?



> Daniel Scally (5):
>   ACPI: scan: Add acpi_dev_get_next_consumer_dev()
>   ACPI: bus: Add iterator for dependent devices
>   platform/x86: int3472: Support multiple clock consumers
>   platform/x86: int3472: Support multiple gpio lookups in board data
>   platform/x86: int3472: Add board data for Surface Go2 IR camera
> 
>  drivers/acpi/scan.c                           | 40 +++++++---
>  drivers/clk/clk-tps68470.c                    | 13 +++-
>  drivers/platform/x86/intel/int3472/common.c   |  2 +-
>  drivers/platform/x86/intel/int3472/tps68470.c | 76 ++++++++++++++++---
>  drivers/platform/x86/intel/int3472/tps68470.h |  3 +-
>  .../x86/intel/int3472/tps68470_board_data.c   | 54 ++++++++++++-
>  include/acpi/acpi_bus.h                       | 15 +++-
>  include/linux/platform_data/tps68470.h        |  7 +-
>  8 files changed, 177 insertions(+), 33 deletions(-)
> 

Re: [PATCH v3 0/5] Add multiple-consumer support to int3472-tps68470 driver

[Daniel Scally]

Hi Hans

On 22/09/2022 09:55, Hans de Goede wrote:
> Hi All,
>
> On 9/22/22 01:04, Daniel Scally wrote:
>> Hello all
>>
>> At the moment there are a few places in the int3472-tps68470 driver that are
>> limited to just working with a single consuming device dependent on the PMIC.
>> There are systems where multiple camera sensors share a single TPS68470, so
>> we need to extend the driver to support them. This requires a couple of tweaks
>> to the ACPI functions to fetch dependent devices, which also assumes that only
>> a single dependent will be found.
>>
>> The v2 for this series was some time ago...it's kept falling to the back of my
>> to-do list so I've only just gotten round to it; sorry about that. v2 here:
>>
>> https://lore.kernel.org/linux-acpi/20220327161344.50477-1-djrscally@gmail.com/
> Rafael, I would like to merge this through the pdx86 tree may I have your
> ack for patches 1 + 2 for this. As a reminder (since it has been a while)
> here are your review remarks to v2 of patch 1:
>
> https://lore.kernel.org/platform-driver-x86/CAJZ5v0i2ciLHP-=8eQcZc0v0xCzhKHKpxLC=Kgv6W5E_5=HQJA@mail.gmail.com/
>
> (which both seem to have been addressed)
>
> AFAICT you did not have any remarks for v2 of patch 2.
>
> Regards,
>
> Hans
>
> p.s.
>
> Dan, if I want to give the IR cam a test run on my own Surface Go (version 1)
> I guess I may need a sensor driver? Where can I find that sensor driver and
> what do I need in userspace to test this ?


It's the ov7251 sensor driver - the required changes are in mainline now
actually so you should be good there. The format of the data is a bit
unconventional; it's Y10 but passed through Intel's CIO2 CSI receiver
which packs it into a different format [1]. I'm probably just going to
add support for that format to Howdy [2] which is what people want to
use this camera for, but for now you can just test by using the
ipu3-unpack utility in libcamera [3] to convert back to proper Y10 and
then raw2rgbpnm [4] to turn it into a ppm. I use this script for now:
https://paste.debian.net/1254629. Be warned that unless you are in
sunlight the image is basically just grey - the IR LED on the front of
the surface needs driving to give a clear image otherwise. I have an led
driver for the tps68470 which can do that, I'm just working on getting
it all tied in so it triggers when streaming starts.



[1]
https://www.linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/pixfmt-yuv-luma.html

[2] https://github.com/boltgolt/howdy

[3] https://git.libcamera.org/libcamera/libcamera.git/tree/utils/ipu3

[4] https://paste.debian.net/1254629

>
>
>> Daniel Scally (5):
>>   ACPI: scan: Add acpi_dev_get_next_consumer_dev()
>>   ACPI: bus: Add iterator for dependent devices
>>   platform/x86: int3472: Support multiple clock consumers
>>   platform/x86: int3472: Support multiple gpio lookups in board data
>>   platform/x86: int3472: Add board data for Surface Go2 IR camera
>>
>>  drivers/acpi/scan.c                           | 40 +++++++---
>>  drivers/clk/clk-tps68470.c                    | 13 +++-
>>  drivers/platform/x86/intel/int3472/common.c   |  2 +-
>>  drivers/platform/x86/intel/int3472/tps68470.c | 76 ++++++++++++++++---
>>  drivers/platform/x86/intel/int3472/tps68470.h |  3 +-
>>  .../x86/intel/int3472/tps68470_board_data.c   | 54 ++++++++++++-
>>  include/acpi/acpi_bus.h                       | 15 +++-
>>  include/linux/platform_data/tps68470.h        |  7 +-
>>  8 files changed, 177 insertions(+), 33 deletions(-)
>>

Re: [PATCH v3 0/5] Add multiple-consumer support to int3472-tps68470 driver

[Rafael J. Wysocki]

On Thu, Sep 22, 2022 at 10:55 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> On 9/22/22 01:04, Daniel Scally wrote:
> > Hello all
> >
> > At the moment there are a few places in the int3472-tps68470 driver that are
> > limited to just working with a single consuming device dependent on the PMIC.
> > There are systems where multiple camera sensors share a single TPS68470, so
> > we need to extend the driver to support them. This requires a couple of tweaks
> > to the ACPI functions to fetch dependent devices, which also assumes that only
> > a single dependent will be found.
> >
> > The v2 for this series was some time ago...it's kept falling to the back of my
> > to-do list so I've only just gotten round to it; sorry about that. v2 here:
> >
> > https://lore.kernel.org/linux-acpi/20220327161344.50477-1-djrscally@gmail.com/
>
> Rafael, I would like to merge this through the pdx86 tree may I have your
> ack for patches 1 + 2 for this. As a reminder (since it has been a while)
> here are your review remarks to v2 of patch 1:
>
> https://lore.kernel.org/platform-driver-x86/CAJZ5v0i2ciLHP-=8eQcZc0v0xCzhKHKpxLC=Kgv6W5E_5=HQJA@mail.gmail.com/
>
> (which both seem to have been addressed)
>
> AFAICT you did not have any remarks for v2 of patch 2.

No, I didn't.

However, because acpi_bus_get_acpi_device() becomes
acpi_get_acpi_dev() in my tree, I think it's better to route this
material through it, if that's not a problem.

I've tentatively queued it up for 6.1.

Thanks!

> p.s.
>
> Dan, if I want to give the IR cam a test run on my own Surface Go (version 1)
> I guess I may need a sensor driver? Where can I find that sensor driver and
> what do I need in userspace to test this ?
>
>
>
> > Daniel Scally (5):
> >   ACPI: scan: Add acpi_dev_get_next_consumer_dev()
> >   ACPI: bus: Add iterator for dependent devices
> >   platform/x86: int3472: Support multiple clock consumers
> >   platform/x86: int3472: Support multiple gpio lookups in board data
> >   platform/x86: int3472: Add board data for Surface Go2 IR camera
> >
> >  drivers/acpi/scan.c                           | 40 +++++++---
> >  drivers/clk/clk-tps68470.c                    | 13 +++-
> >  drivers/platform/x86/intel/int3472/common.c   |  2 +-
> >  drivers/platform/x86/intel/int3472/tps68470.c | 76 ++++++++++++++++---
> >  drivers/platform/x86/intel/int3472/tps68470.h |  3 +-
> >  .../x86/intel/int3472/tps68470_board_data.c   | 54 ++++++++++++-
> >  include/acpi/acpi_bus.h                       | 15 +++-
> >  include/linux/platform_data/tps68470.h        |  7 +-
> >  8 files changed, 177 insertions(+), 33 deletions(-)
> >
>

Re: [PATCH v3 0/5] Add multiple-consumer support to int3472-tps68470 driver

[Hans de Goede]

Hi,

On 9/24/22 19:15, Rafael J. Wysocki wrote:
> On Thu, Sep 22, 2022 at 10:55 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi All,
>>
>> On 9/22/22 01:04, Daniel Scally wrote:
>>> Hello all
>>>
>>> At the moment there are a few places in the int3472-tps68470 driver that are
>>> limited to just working with a single consuming device dependent on the PMIC.
>>> There are systems where multiple camera sensors share a single TPS68470, so
>>> we need to extend the driver to support them. This requires a couple of tweaks
>>> to the ACPI functions to fetch dependent devices, which also assumes that only
>>> a single dependent will be found.
>>>
>>> The v2 for this series was some time ago...it's kept falling to the back of my
>>> to-do list so I've only just gotten round to it; sorry about that. v2 here:
>>>
>>> https://lore.kernel.org/linux-acpi/20220327161344.50477-1-djrscally@gmail.com/
>>
>> Rafael, I would like to merge this through the pdx86 tree may I have your
>> ack for patches 1 + 2 for this. As a reminder (since it has been a while)
>> here are your review remarks to v2 of patch 1:
>>
>> https://lore.kernel.org/platform-driver-x86/CAJZ5v0i2ciLHP-=8eQcZc0v0xCzhKHKpxLC=Kgv6W5E_5=HQJA@mail.gmail.com/
>>
>> (which both seem to have been addressed)
>>
>> AFAICT you did not have any remarks for v2 of patch 2.
> 
> No, I didn't.
> 
> However, because acpi_bus_get_acpi_device() becomes
> acpi_get_acpi_dev() in my tree, I think it's better to route this
> material through it, if that's not a problem.

Routing it to your tree is fine.

> I've tentatively queued it up for 6.1.

Great, thank you!

Regards,

Hans



>> p.s.
>>
>> Dan, if I want to give the IR cam a test run on my own Surface Go (version 1)
>> I guess I may need a sensor driver? Where can I find that sensor driver and
>> what do I need in userspace to test this ?
>>
>>
>>
>>> Daniel Scally (5):
>>>   ACPI: scan: Add acpi_dev_get_next_consumer_dev()
>>>   ACPI: bus: Add iterator for dependent devices
>>>   platform/x86: int3472: Support multiple clock consumers
>>>   platform/x86: int3472: Support multiple gpio lookups in board data
>>>   platform/x86: int3472: Add board data for Surface Go2 IR camera
>>>
>>>  drivers/acpi/scan.c                           | 40 +++++++---
>>>  drivers/clk/clk-tps68470.c                    | 13 +++-
>>>  drivers/platform/x86/intel/int3472/common.c   |  2 +-
>>>  drivers/platform/x86/intel/int3472/tps68470.c | 76 ++++++++++++++++---
>>>  drivers/platform/x86/intel/int3472/tps68470.h |  3 +-
>>>  .../x86/intel/int3472/tps68470_board_data.c   | 54 ++++++++++++-
>>>  include/acpi/acpi_bus.h                       | 15 +++-
>>>  include/linux/platform_data/tps68470.h        |  7 +-
>>>  8 files changed, 177 insertions(+), 33 deletions(-)
>>>
>>
> 

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

[Andy Shevchenko]

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?

> +	return n_consumers;
> +}

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

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