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

[PATCH v2 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.

Series level changes in v2:

- Dropped the patch that added a terminator to the existing tps68470 board data

The v1 can be found here:

https://lore.kernel.org/platform-driver-x86/20220216225304.53911-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                           | 37 ++++++---
 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   | 49 +++++++++++-
 include/acpi/acpi_bus.h                       | 15 +++-
 include/linux/platform_data/tps68470.h        |  7 +-
 8 files changed, 169 insertions(+), 33 deletions(-)

-- 
2.25.1

[PATCH v2 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 in v2:

	- Removed acpi_dev_get_first_consumer_dev() entirely

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

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 010ef0b28374..8797e4a33674 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2215,9 +2215,21 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass)
 		device->handler->hotplug.notify_online(device);
 }
 
-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 = *(struct acpi_device **)data;
+
+	/*
+	 * 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)
+			*(struct acpi_device **)data = NULL;
+
+		return 0;
+	}
 
 	adev = acpi_bus_get_acpi_device(dep->consumer);
 	if (adev) {
@@ -2348,25 +2360,28 @@ 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);
 
-	return adev;
+	acpi_dev_put(start);
+	return adev == start ? NULL : 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 2f93ecf05dac..cdc726d251b6 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -696,7 +696,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 v2 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 in v2:

	- switched to use acpi_dev_get_next_consumer_dev() in the loop init

 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 cdc726d251b6..7fbe690a5b33 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -699,6 +699,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 v2 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 in 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 2ad0ac2f4096..c9fe1129c537 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)
 {
 	struct tps68470_clk_platform_data *pdata = pdev->dev.platform_data;
 	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);
@@ -219,10 +221,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 5232dbcd8212..907a0eb49f12 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/mfd/core.h>
 #include <linux/mfd/tps68470.h>
@@ -100,20 +101,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)) {
@@ -149,8 +194,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 v2 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 in v2:

	- Added a missing blank line after "int i;"

 drivers/platform/x86/intel/int3472/tps68470.c   | 17 ++++++++++++-----
 drivers/platform/x86/intel/int3472/tps68470.h   |  3 ++-
 .../x86/intel/int3472/tps68470_board_data.c     | 16 ++++++++++++++--
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index 907a0eb49f12..471b468a8da3 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -155,6 +155,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)
@@ -201,15 +202,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:
@@ -233,10 +237,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 f63ae7329cd2..dfb9431e4b1b 100644
--- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
+++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
@@ -85,7 +85,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),
@@ -94,10 +94,22 @@ 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 dmi_system_id int3472_tps68470_board_data_table[] = {
-- 
2.25.1

[PATCH v2 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 in 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 dfb9431e4b1b..3573c46e7993 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 = {
@@ -76,12 +85,36 @@ 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,
 		[TPS68470_ANA]  = &surface_go_tps68470_ana_reg_init_data,
 		[TPS68470_VCM]  = &surface_go_tps68470_vcm_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 v2 1/5] ACPI: scan: Add acpi_dev_get_next_consumer_dev()

[Rafael J. Wysocki]

On Sun, Mar 27, 2022 at 6:13 PM Daniel Scally <djrscally@gmail.com> wrote:
>
> 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 in v2:
>
>         - Removed acpi_dev_get_first_consumer_dev() entirely
>
>  drivers/acpi/scan.c                         | 37 +++++++++++++++------
>  drivers/platform/x86/intel/int3472/common.c |  2 +-
>  include/acpi/acpi_bus.h                     |  4 ++-
>  3 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 010ef0b28374..8797e4a33674 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2215,9 +2215,21 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass)
>                 device->handler->hotplug.notify_online(device);
>  }
>
> -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 = *(struct acpi_device **)data;

I would prefer

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)
> +                       *(struct acpi_device **)data = NULL;

                      *adev_p = NULL;

> +
> +               return 0;
> +       }
>
>         adev = acpi_bus_get_acpi_device(dep->consumer);
>         if (adev) {
> @@ -2348,25 +2360,28 @@ 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);
>
> -       return adev;
> +       acpi_dev_put(start);
> +       return adev == start ? NULL : adev;

And here

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 2f93ecf05dac..cdc726d251b6 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -696,7 +696,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
>

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

[Daniel Scally]

Hello

On 05/04/2022 13:57, Rafael J. Wysocki wrote:
> On Sun, Mar 27, 2022 at 6:13 PM Daniel Scally <djrscally@gmail.com> wrote:
>> 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 in v2:
>>
>>         - Removed acpi_dev_get_first_consumer_dev() entirely
>>
>>  drivers/acpi/scan.c                         | 37 +++++++++++++++------
>>  drivers/platform/x86/intel/int3472/common.c |  2 +-
>>  include/acpi/acpi_bus.h                     |  4 ++-
>>  3 files changed, 30 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 010ef0b28374..8797e4a33674 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -2215,9 +2215,21 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass)
>>                 device->handler->hotplug.notify_online(device);
>>  }
>>
>> -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 = *(struct acpi_device **)data;
> I would prefer
>
> struct acpi_device **adev_p = data;
> struct acpi_device *adev = *adev_p;


This and the below are fine for me; I'll send a v3

>
>> +
>> +       /*
>> +        * 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)
>> +                       *(struct acpi_device **)data = NULL;
>                       *adev_p = NULL;
>
>> +
>> +               return 0;
>> +       }
>>
>>         adev = acpi_bus_get_acpi_device(dep->consumer);
>>         if (adev) {
>> @@ -2348,25 +2360,28 @@ 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);
>>
>> -       return adev;
>> +       acpi_dev_put(start);
>> +       return adev == start ? NULL : adev;
> And here
>
> 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 2f93ecf05dac..cdc726d251b6 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -696,7 +696,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
>>