* [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver
@ 2022-02-16 22:52 Daniel Scally
2022-02-16 22:52 ` [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev() Daniel Scally
` (7 more replies)
0 siblings, 8 replies; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:52 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
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.
Hans - this (plus a series to media [1]) adds support for the Surface Go 2's
IR camera...the regulator settings for the Go1/2/3 world facing camera are the
same, so I'd expect them to match for the IR sensor too, which means it should
enable support for your Go too.
Thanks
Dan
[1] https://lore.kernel.org/linux-media/20220215230737.1870630-1-djrscally@gmail.com/
Daniel Scally (6):
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: Add terminator to gpiod_lookup_table
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 | 47 +++++++++--
drivers/clk/clk-tps68470.c | 13 +++-
drivers/platform/x86/intel/int3472/tps68470.c | 77 +++++++++++++++----
drivers/platform/x86/intel/int3472/tps68470.h | 3 +-
.../x86/intel/int3472/tps68470_board_data.c | 58 +++++++++++++-
include/acpi/acpi_bus.h | 14 ++++
include/linux/platform_data/tps68470.h | 7 +-
7 files changed, 188 insertions(+), 31 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev()
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
@ 2022-02-16 22:52 ` Daniel Scally
2022-02-21 9:50 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 2/6] ACPI: bus: Add iterator for dependent devices Daniel Scally
` (6 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:52 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
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.
Extend the functionality by adding a new function that fetches the
next consumer of a supplier device. We can then simplify the original
function by simply calling the new one.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
drivers/acpi/scan.c | 47 ++++++++++++++++++++++++++++++++++-------
include/acpi/acpi_bus.h | 2 ++
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 4463c2eda61e..b3ed664ee1cb 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2256,9 +2256,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) {
@@ -2389,23 +2401,42 @@ 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.
*/
-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_next_consumer_dev);
+
+/**
+ * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier
+ * @supplier: Pointer to the dependee device
+ *
+ * Returns the first &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.
+ */
+struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier)
+{
+ return acpi_dev_get_next_consumer_dev(supplier, NULL);
}
EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ca88c4706f2b..8b06fef04722 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -720,6 +720,8 @@ 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_next_consumer_dev(struct acpi_device *supplier,
+ struct acpi_device *start);
struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier);
struct acpi_device *
acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/6] ACPI: bus: Add iterator for dependent devices
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
2022-02-16 22:52 ` [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev() Daniel Scally
@ 2022-02-16 22:53 ` Daniel Scally
2022-02-21 9:52 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers Daniel Scally
` (5 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:53 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
Add a helper macro to iterate over ACPI devices that are flagged
as consumers of an initial supplier ACPI device.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
include/acpi/acpi_bus.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 8b06fef04722..72103bcdd5b3 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -723,6 +723,18 @@ 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);
struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier);
+
+/**
+ * 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_first_consumer_dev(supplier); \
+ 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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
2022-02-16 22:52 ` [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev() Daniel Scally
2022-02-16 22:53 ` [PATCH 2/6] ACPI: bus: Add iterator for dependent devices Daniel Scally
@ 2022-02-16 22:53 ` Daniel Scally
2022-02-17 3:22 ` kernel test robot
2022-02-21 9:59 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table Daniel Scally
` (4 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:53 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
At present, the int3472-tps68470 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.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
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..b535564712bb 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;
+ unsigned int n_consumers;
struct mfd_cell *cells;
struct regmap *regmap;
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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
` (2 preceding siblings ...)
2022-02-16 22:53 ` [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers Daniel Scally
@ 2022-02-16 22:53 ` Daniel Scally
2022-02-21 10:00 ` Hans de Goede
2022-02-21 13:46 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 5/6] platform/x86: int3472: Support multiple gpio lookups in board data Daniel Scally
` (3 subsequent siblings)
7 siblings, 2 replies; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:53 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
Without the terminator, if a con_id is passed to gpio_find() that
does not exist in the lookup table the function will not stop looping
correctly, and eventually cause an oops.
Fixes: 1596ef1251b5 ("platform/x86: int3472: Pass tps68470_regulator_platform_data to the tps68470-regulator MFD-cell")
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
drivers/platform/x86/intel/int3472/tps68470_board_data.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
index f93d437fd192..525f09a3b5ff 100644
--- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
+++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
@@ -100,7 +100,8 @@ static struct gpiod_lookup_table surface_go_tps68470_gpios = {
.dev_id = "i2c-INT347A:00",
.table = {
GPIO_LOOKUP("tps68470-gpio", 9, "reset", GPIO_ACTIVE_LOW),
- GPIO_LOOKUP("tps68470-gpio", 7, "powerdown", GPIO_ACTIVE_LOW)
+ GPIO_LOOKUP("tps68470-gpio", 7, "powerdown", GPIO_ACTIVE_LOW),
+ { }
}
};
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 5/6] platform/x86: int3472: Support multiple gpio lookups in board data
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
` (3 preceding siblings ...)
2022-02-16 22:53 ` [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table Daniel Scally
@ 2022-02-16 22:53 ` Daniel Scally
2022-02-21 10:03 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 6/6] platform/x86: int3472: Add board data for Surface Go2 IR camera Daniel Scally
` (2 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:53 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
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.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
drivers/platform/x86/intel/int3472/tps68470.c | 18 ++++++++++-----
drivers/platform/x86/intel/int3472/tps68470.h | 3 ++-
.../x86/intel/int3472/tps68470_board_data.c | 22 ++++++++++++++++---
3 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index b535564712bb..736480961ec3 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)
struct regmap *regmap;
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,12 @@ 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..442a8a2de224 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,32 @@ 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 = 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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 6/6] platform/x86: int3472: Add board data for Surface Go2 IR camera
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
` (4 preceding siblings ...)
2022-02-16 22:53 ` [PATCH 5/6] platform/x86: int3472: Support multiple gpio lookups in board data Daniel Scally
@ 2022-02-16 22:53 ` Daniel Scally
2022-02-21 10:05 ` Hans de Goede
2022-02-17 9:55 ` [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Hans de Goede
2022-02-21 10:07 ` Hans de Goede
7 siblings, 1 reply; 26+ messages in thread
From: Daniel Scally @ 2022-02-16 22:53 UTC (permalink / raw)
To: linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, hdegoede, markgross, robert.moore
Add the board data describing the regulators for the Microsoft
Surface Go line's IR camera.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
.../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 442a8a2de224..49a3591c6d85 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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-16 22:53 ` [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers Daniel Scally
@ 2022-02-17 3:22 ` kernel test robot
2022-02-21 9:59 ` Hans de Goede
1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2022-02-17 3:22 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: kbuild-all, rafael, lenb, mturquette, sboyd, hdegoede, markgross,
robert.moore
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
[cannot apply to platform-drivers-x86/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
142
143 static int skl_int3472_tps68470_probe(struct i2c_client *client)
144 {
145 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
146 const struct int3472_tps68470_board_data *board_data;
147 struct tps68470_clk_platform_data *clk_pdata;
148 unsigned int n_consumers;
149 struct mfd_cell *cells;
150 struct regmap *regmap;
151 int device_type;
152 int ret;
153
154 n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> 155 if (n_consumers < 0)
156 return n_consumers;
157
158 regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
159 if (IS_ERR(regmap)) {
160 dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
161 return PTR_ERR(regmap);
162 }
163
164 i2c_set_clientdata(client, regmap);
165
166 ret = tps68470_chip_init(&client->dev, regmap);
167 if (ret < 0) {
168 dev_err(&client->dev, "TPS68470 init error %d\n", ret);
169 return ret;
170 }
171
172 device_type = skl_int3472_tps68470_calc_type(adev);
173 switch (device_type) {
174 case DESIGNED_FOR_WINDOWS:
175 board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
176 if (!board_data)
177 return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
178
179 cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
180 if (!cells)
181 return -ENOMEM;
182
183 /*
184 * The order of the cells matters here! The clk must be first
185 * because the regulator depends on it. The gpios must be last,
186 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
187 * the clk + regulators must be ready when this happens.
188 */
189 cells[0].name = "tps68470-clk";
190 cells[0].platform_data = clk_pdata;
191 cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
192 cells[1].name = "tps68470-regulator";
193 cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
194 cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
195 cells[2].name = "tps68470-gpio";
196
197 gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
198
199 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
200 cells, TPS68470_WIN_MFD_CELL_COUNT,
201 NULL, 0, NULL);
202 kfree(cells);
203
204 if (ret)
205 gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
206
207 break;
208 case DESIGNED_FOR_CHROMEOS:
209 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
210 tps68470_cros, ARRAY_SIZE(tps68470_cros),
211 NULL, 0, NULL);
212 break;
213 default:
214 dev_err(&client->dev, "Failed to add MFD devices\n");
215 return device_type;
216 }
217
218 /*
219 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
220 * for the GPIO cell already does this.
221 */
222
223 return ret;
224 }
225
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
@ 2022-02-17 3:22 ` kernel test robot
0 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2022-02-17 3:22 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4523 bytes --]
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
[cannot apply to platform-drivers-x86/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
142
143 static int skl_int3472_tps68470_probe(struct i2c_client *client)
144 {
145 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
146 const struct int3472_tps68470_board_data *board_data;
147 struct tps68470_clk_platform_data *clk_pdata;
148 unsigned int n_consumers;
149 struct mfd_cell *cells;
150 struct regmap *regmap;
151 int device_type;
152 int ret;
153
154 n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> 155 if (n_consumers < 0)
156 return n_consumers;
157
158 regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
159 if (IS_ERR(regmap)) {
160 dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
161 return PTR_ERR(regmap);
162 }
163
164 i2c_set_clientdata(client, regmap);
165
166 ret = tps68470_chip_init(&client->dev, regmap);
167 if (ret < 0) {
168 dev_err(&client->dev, "TPS68470 init error %d\n", ret);
169 return ret;
170 }
171
172 device_type = skl_int3472_tps68470_calc_type(adev);
173 switch (device_type) {
174 case DESIGNED_FOR_WINDOWS:
175 board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
176 if (!board_data)
177 return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
178
179 cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
180 if (!cells)
181 return -ENOMEM;
182
183 /*
184 * The order of the cells matters here! The clk must be first
185 * because the regulator depends on it. The gpios must be last,
186 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
187 * the clk + regulators must be ready when this happens.
188 */
189 cells[0].name = "tps68470-clk";
190 cells[0].platform_data = clk_pdata;
191 cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
192 cells[1].name = "tps68470-regulator";
193 cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
194 cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
195 cells[2].name = "tps68470-gpio";
196
197 gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
198
199 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
200 cells, TPS68470_WIN_MFD_CELL_COUNT,
201 NULL, 0, NULL);
202 kfree(cells);
203
204 if (ret)
205 gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
206
207 break;
208 case DESIGNED_FOR_CHROMEOS:
209 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
210 tps68470_cros, ARRAY_SIZE(tps68470_cros),
211 NULL, 0, NULL);
212 break;
213 default:
214 dev_err(&client->dev, "Failed to add MFD devices\n");
215 return device_type;
216 }
217
218 /*
219 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
220 * for the GPIO cell already does this.
221 */
222
223 return ret;
224 }
225
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
` (5 preceding siblings ...)
2022-02-16 22:53 ` [PATCH 6/6] platform/x86: int3472: Add board data for Surface Go2 IR camera Daniel Scally
@ 2022-02-17 9:55 ` Hans de Goede
2022-02-18 8:45 ` Daniel Scally
2022-02-21 10:07 ` Hans de Goede
7 siblings, 1 reply; 26+ messages in thread
From: Hans de Goede @ 2022-02-17 9:55 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi Dan,
On 2/16/22 23:52, Daniel Scally wrote:
> 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.
This sounds great, thank you for your work on this. I have a bit of a backlog wrt
pdx86 patches to review and I try to through those in FIFO order to keep things fair,
so it may be a while (approx. 1-2 weeks) before I get around to reviewing
these.
Regards,
Hans
>
> Hans - this (plus a series to media [1]) adds support for the Surface Go 2's
> IR camera...the regulator settings for the Go1/2/3 world facing camera are the
> same, so I'd expect them to match for the IR sensor too, which means it should
> enable support for your Go too.
>
> Thanks
> Dan
>
>
> [1] https://lore.kernel.org/linux-media/20220215230737.1870630-1-djrscally@gmail.com/
>
> Daniel Scally (6):
> 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: Add terminator to gpiod_lookup_table
> 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 | 47 +++++++++--
> drivers/clk/clk-tps68470.c | 13 +++-
> drivers/platform/x86/intel/int3472/tps68470.c | 77 +++++++++++++++----
> drivers/platform/x86/intel/int3472/tps68470.h | 3 +-
> .../x86/intel/int3472/tps68470_board_data.c | 58 +++++++++++++-
> include/acpi/acpi_bus.h | 14 ++++
> include/linux/platform_data/tps68470.h | 7 +-
> 7 files changed, 188 insertions(+), 31 deletions(-)
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver
2022-02-17 9:55 ` [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Hans de Goede
@ 2022-02-18 8:45 ` Daniel Scally
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Scally @ 2022-02-18 8:45 UTC (permalink / raw)
To: Hans de Goede, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi Hans
On 17/02/2022 09:55, Hans de Goede wrote:
> Hi Dan,
>
> On 2/16/22 23:52, Daniel Scally wrote:
>> 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.
> This sounds great, thank you for your work on this. I have a bit of a backlog wrt
> pdx86 patches to review and I try to through those in FIFO order to keep things fair,
> so it may be a while (approx. 1-2 weeks) before I get around to reviewing
> these.
No problem, thanks very much
>
> Regards,
>
> Hans
>
>
>> Hans - this (plus a series to media [1]) adds support for the Surface Go 2's
>> IR camera...the regulator settings for the Go1/2/3 world facing camera are the
>> same, so I'd expect them to match for the IR sensor too, which means it should
>> enable support for your Go too.
>>
>> Thanks
>> Dan
>>
>>
>> [1] https://lore.kernel.org/linux-media/20220215230737.1870630-1-djrscally@gmail.com/
>>
>> Daniel Scally (6):
>> 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: Add terminator to gpiod_lookup_table
>> 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 | 47 +++++++++--
>> drivers/clk/clk-tps68470.c | 13 +++-
>> drivers/platform/x86/intel/int3472/tps68470.c | 77 +++++++++++++++----
>> drivers/platform/x86/intel/int3472/tps68470.h | 3 +-
>> .../x86/intel/int3472/tps68470_board_data.c | 58 +++++++++++++-
>> include/acpi/acpi_bus.h | 14 ++++
>> include/linux/platform_data/tps68470.h | 7 +-
>> 7 files changed, 188 insertions(+), 31 deletions(-)
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev()
2022-02-16 22:52 ` [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev() Daniel Scally
@ 2022-02-21 9:50 ` Hans de Goede
2022-02-21 9:51 ` Hans de Goede
0 siblings, 1 reply; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 9:50 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:52, Daniel Scally 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.
>
> Extend the functionality by adding a new function that fetches the
> next consumer of a supplier device. We can then simplify the original
> function by simply calling the new one.
>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> drivers/acpi/scan.c | 47 ++++++++++++++++++++++++++++++++++-------
> include/acpi/acpi_bus.h | 2 ++
> 2 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 4463c2eda61e..b3ed664ee1cb 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2256,9 +2256,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) {
> @@ -2389,23 +2401,42 @@ 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.
This bit of the help text seems to not be entirely correct, since the reference to
start gets consumed by this, so the caller only needs to put the device when it
does NOT pass it as start to another acpi_dev_get_next_consumer_dev call.
> */
> -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_next_consumer_dev);
> +
> +/**
> + * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier
> + * @supplier: Pointer to the dependee device
> + *
> + * Returns the first &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.
> + */
> +struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier)
> +{
> + return acpi_dev_get_next_consumer_dev(supplier, NULL);
> }
> EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
The only caller of this is skl_int3472_get_sensor_adev_and_name() IMHO it would
be better to just move that over to acpi_dev_get_next_consumer_dev(..., NULL);
in this same patch and just drop acpi_dev_get_first_consumer_dev() all together.
I expect this entire series to get merged through the pdx86 tree, so from
that pov doing this should be fine..
Regards,
Hans
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index ca88c4706f2b..8b06fef04722 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -720,6 +720,8 @@ 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_next_consumer_dev(struct acpi_device *supplier,
> + struct acpi_device *start);
> struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier);
> struct acpi_device *
> acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev()
2022-02-21 9:50 ` Hans de Goede
@ 2022-02-21 9:51 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 9:51 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/21/22 10:50, Hans de Goede wrote:
> Hi,
>
> On 2/16/22 23:52, Daniel Scally 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.
>>
>> Extend the functionality by adding a new function that fetches the
>> next consumer of a supplier device. We can then simplify the original
>> function by simply calling the new one.
>>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>> ---
>> drivers/acpi/scan.c | 47 ++++++++++++++++++++++++++++++++++-------
>> include/acpi/acpi_bus.h | 2 ++
>> 2 files changed, 41 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 4463c2eda61e..b3ed664ee1cb 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -2256,9 +2256,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) {
>> @@ -2389,23 +2401,42 @@ 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.
>
> This bit of the help text seems to not be entirely correct, since the reference to
> start gets consumed by this, so the caller only needs to put the device when it
> does NOT pass it as start to another acpi_dev_get_next_consumer_dev call.
>
>
>
>> */
>> -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_next_consumer_dev);
>> +
>> +/**
>> + * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier
>> + * @supplier: Pointer to the dependee device
>> + *
>> + * Returns the first &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.
>> + */
>> +struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier)
>> +{
>> + return acpi_dev_get_next_consumer_dev(supplier, NULL);
>> }
>> EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
>
> The only caller of this is skl_int3472_get_sensor_adev_and_name() IMHO it would
> be better to just move that over to acpi_dev_get_next_consumer_dev(..., NULL);
> in this same patch and just drop acpi_dev_get_first_consumer_dev() all together.
>
> I expect this entire series to get merged through the pdx86 tree, so from
> that pov doing this should be fine..
I forgot to add: that otherwise this looks good to me, so with the above
addressed you may add my:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>> index ca88c4706f2b..8b06fef04722 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -720,6 +720,8 @@ 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_next_consumer_dev(struct acpi_device *supplier,
>> + struct acpi_device *start);
>> struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier);
>> struct acpi_device *
>> acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/6] ACPI: bus: Add iterator for dependent devices
2022-02-16 22:53 ` [PATCH 2/6] ACPI: bus: Add iterator for dependent devices Daniel Scally
@ 2022-02-21 9:52 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 9:52 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:53, Daniel Scally wrote:
> Add a helper macro to iterate over ACPI devices that are flagged
> as consumers of an initial supplier ACPI device.
>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> include/acpi/acpi_bus.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 8b06fef04722..72103bcdd5b3 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -723,6 +723,18 @@ 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);
> struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier);
> +
> +/**
> + * 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_first_consumer_dev(supplier); \
If we drop acpi_dev_get_first_consumer_dev() this needs to be updated to:
for (consumer = acpi_dev_get_next_consumer_dev(supplier, NULL); \
Otherwise this looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> + 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 *
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-16 22:53 ` [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers Daniel Scally
2022-02-17 3:22 ` kernel test robot
@ 2022-02-21 9:59 ` Hans de Goede
2022-02-25 0:49 ` Stephen Boyd
1 sibling, 1 reply; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 9:59 UTC (permalink / raw)
To: Daniel Scally, Michael Turquette
Cc: rafael, lenb, sboyd, markgross, robert.moore, linux-acpi,
linux-clk, platform-driver-x86
Hi,
On 2/16/22 23:53, Daniel Scally wrote:
> At present, the int3472-tps68470 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.
>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Michael, I plan to merge this entire series through the platform-drivers-x86 git
tree, may I have your ack for merging the clk bits from this ?
Regards,
Hans
> ---
> 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..b535564712bb 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;
> + unsigned int n_consumers;
> struct mfd_cell *cells;
> struct regmap *regmap;
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table
2022-02-16 22:53 ` [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table Daniel Scally
@ 2022-02-21 10:00 ` Hans de Goede
2022-02-21 13:46 ` Hans de Goede
1 sibling, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 10:00 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:53, Daniel Scally wrote:
> Without the terminator, if a con_id is passed to gpio_find() that
> does not exist in the lookup table the function will not stop looping
> correctly, and eventually cause an oops.
>
> Fixes: 1596ef1251b5 ("platform/x86: int3472: Pass tps68470_regulator_platform_data to the tps68470-regulator MFD-cell")
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> drivers/platform/x86/intel/int3472/tps68470_board_data.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> index f93d437fd192..525f09a3b5ff 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> @@ -100,7 +100,8 @@ static struct gpiod_lookup_table surface_go_tps68470_gpios = {
> .dev_id = "i2c-INT347A:00",
> .table = {
> GPIO_LOOKUP("tps68470-gpio", 9, "reset", GPIO_ACTIVE_LOW),
> - GPIO_LOOKUP("tps68470-gpio", 7, "powerdown", GPIO_ACTIVE_LOW)
> + GPIO_LOOKUP("tps68470-gpio", 7, "powerdown", GPIO_ACTIVE_LOW),
> + { }
> }
> };
>
Oops, I'll got and apply this to pdx86/for-next and pdx86/fixes right away. Please drop this
from the next version of the series.
Regards,
Hans
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 5/6] platform/x86: int3472: Support multiple gpio lookups in board data
2022-02-16 22:53 ` [PATCH 5/6] platform/x86: int3472: Support multiple gpio lookups in board data Daniel Scally
@ 2022-02-21 10:03 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 10:03 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:53, Daniel Scally wrote:
> 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.
>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> drivers/platform/x86/intel/int3472/tps68470.c | 18 ++++++++++-----
> drivers/platform/x86/intel/int3472/tps68470.h | 3 ++-
> .../x86/intel/int3472/tps68470_board_data.c | 22 ++++++++++++++++---
> 3 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index b535564712bb..736480961ec3 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)
> struct regmap *regmap;
> 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,12 @@ 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;
Please keep an empty line between variable declarations and statements
(I believe checkpatch should have warned about this?)
> 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..442a8a2de224 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,32 @@ 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 = 2,
> + .tps68470_gpio_lookup_tables = {
> + &surface_go_int347a_gpios,
> + &surface_go_int347e_gpios,
> + }
> };
>
> static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
Otherwise this looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 6/6] platform/x86: int3472: Add board data for Surface Go2 IR camera
2022-02-16 22:53 ` [PATCH 6/6] platform/x86: int3472: Add board data for Surface Go2 IR camera Daniel Scally
@ 2022-02-21 10:05 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 10:05 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:53, Daniel Scally wrote:
> Add the board data describing the regulators for the Microsoft
> Surface Go line's IR camera.
>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> .../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 442a8a2de224..49a3591c6d85 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,
> },
> };
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
` (6 preceding siblings ...)
2022-02-17 9:55 ` [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Hans de Goede
@ 2022-02-21 10:07 ` Hans de Goede
7 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 10:07 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86,
Rafael J. Wysocki
Cc: lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:52, Daniel Scally wrote:
> 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.
>
> Hans - this (plus a series to media [1]) adds support for the Surface Go 2's
> IR camera...the regulator settings for the Go1/2/3 world facing camera are the
> same, so I'd expect them to match for the IR sensor too, which means it should
> enable support for your Go too.
Thank you, this mostly looks good to me. I've added some small remarks but
nothing big stands out.
Rafael, I think it would be easiest for me to merge this entire series through
pdx86/for-next. May I have your ack for patches 1+2 to merge them through
the pdx86 tree?
Regards,
Hans
> [1] https://lore.kernel.org/linux-media/20220215230737.1870630-1-djrscally@gmail.com/
>
> Daniel Scally (6):
> 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: Add terminator to gpiod_lookup_table
> 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 | 47 +++++++++--
> drivers/clk/clk-tps68470.c | 13 +++-
> drivers/platform/x86/intel/int3472/tps68470.c | 77 +++++++++++++++----
> drivers/platform/x86/intel/int3472/tps68470.h | 3 +-
> .../x86/intel/int3472/tps68470_board_data.c | 58 +++++++++++++-
> include/acpi/acpi_bus.h | 14 ++++
> include/linux/platform_data/tps68470.h | 7 +-
> 7 files changed, 188 insertions(+), 31 deletions(-)
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table
2022-02-16 22:53 ` [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table Daniel Scally
2022-02-21 10:00 ` Hans de Goede
@ 2022-02-21 13:46 ` Hans de Goede
1 sibling, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-21 13:46 UTC (permalink / raw)
To: Daniel Scally, linux-acpi, linux-clk, platform-driver-x86
Cc: rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/16/22 23:53, Daniel Scally wrote:
> Without the terminator, if a con_id is passed to gpio_find() that
> does not exist in the lookup table the function will not stop looping
> correctly, and eventually cause an oops.
>
> Fixes: 1596ef1251b5 ("platform/x86: int3472: Pass tps68470_regulator_platform_data to the tps68470-regulator MFD-cell")
This fixes tag is wrong, that sha does not exist? See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/platform/x86/intel/int3472/tps68470_board_data.c
I've updated the hash to 19d8d6e36b4b while merging this.
Regards,
Hans
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> drivers/platform/x86/intel/int3472/tps68470_board_data.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> index f93d437fd192..525f09a3b5ff 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> @@ -100,7 +100,8 @@ static struct gpiod_lookup_table surface_go_tps68470_gpios = {
> .dev_id = "i2c-INT347A:00",
> .table = {
> GPIO_LOOKUP("tps68470-gpio", 9, "reset", GPIO_ACTIVE_LOW),
> - GPIO_LOOKUP("tps68470-gpio", 7, "powerdown", GPIO_ACTIVE_LOW)
> + GPIO_LOOKUP("tps68470-gpio", 7, "powerdown", GPIO_ACTIVE_LOW),
> + { }
> }
> };
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-17 3:22 ` kernel test robot
@ 2022-02-22 14:44 ` Hans de Goede
-1 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-22 14:44 UTC (permalink / raw)
To: kernel test robot, Daniel Scally, linux-acpi, linux-clk,
platform-driver-x86
Cc: kbuild-all, rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi,
On 2/17/22 04:22, kernel test robot wrote:
> Hi Daniel,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on rafael-pm/linux-next]
> [also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
> [cannot apply to platform-drivers-x86/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
Right this needs to be an int, not an unsigned int. Daniel, please fix this for v2.
Regards,
Hans
>
> vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
>
> 142
> 143 static int skl_int3472_tps68470_probe(struct i2c_client *client)
> 144 {
> 145 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> 146 const struct int3472_tps68470_board_data *board_data;
> 147 struct tps68470_clk_platform_data *clk_pdata;
> 148 unsigned int n_consumers;
> 149 struct mfd_cell *cells;
> 150 struct regmap *regmap;
> 151 int device_type;
> 152 int ret;
> 153
> 154 n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> > 155 if (n_consumers < 0)
> 156 return n_consumers;
> 157
> 158 regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
> 159 if (IS_ERR(regmap)) {
> 160 dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
> 161 return PTR_ERR(regmap);
> 162 }
> 163
> 164 i2c_set_clientdata(client, regmap);
> 165
> 166 ret = tps68470_chip_init(&client->dev, regmap);
> 167 if (ret < 0) {
> 168 dev_err(&client->dev, "TPS68470 init error %d\n", ret);
> 169 return ret;
> 170 }
> 171
> 172 device_type = skl_int3472_tps68470_calc_type(adev);
> 173 switch (device_type) {
> 174 case DESIGNED_FOR_WINDOWS:
> 175 board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
> 176 if (!board_data)
> 177 return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
> 178
> 179 cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
> 180 if (!cells)
> 181 return -ENOMEM;
> 182
> 183 /*
> 184 * The order of the cells matters here! The clk must be first
> 185 * because the regulator depends on it. The gpios must be last,
> 186 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
> 187 * the clk + regulators must be ready when this happens.
> 188 */
> 189 cells[0].name = "tps68470-clk";
> 190 cells[0].platform_data = clk_pdata;
> 191 cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
> 192 cells[1].name = "tps68470-regulator";
> 193 cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
> 194 cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
> 195 cells[2].name = "tps68470-gpio";
> 196
> 197 gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
> 198
> 199 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> 200 cells, TPS68470_WIN_MFD_CELL_COUNT,
> 201 NULL, 0, NULL);
> 202 kfree(cells);
> 203
> 204 if (ret)
> 205 gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
> 206
> 207 break;
> 208 case DESIGNED_FOR_CHROMEOS:
> 209 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> 210 tps68470_cros, ARRAY_SIZE(tps68470_cros),
> 211 NULL, 0, NULL);
> 212 break;
> 213 default:
> 214 dev_err(&client->dev, "Failed to add MFD devices\n");
> 215 return device_type;
> 216 }
> 217
> 218 /*
> 219 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
> 220 * for the GPIO cell already does this.
> 221 */
> 222
> 223 return ret;
> 224 }
> 225
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
@ 2022-02-22 14:44 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-22 14:44 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4917 bytes --]
Hi,
On 2/17/22 04:22, kernel test robot wrote:
> Hi Daniel,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on rafael-pm/linux-next]
> [also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
> [cannot apply to platform-drivers-x86/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp(a)intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
Right this needs to be an int, not an unsigned int. Daniel, please fix this for v2.
Regards,
Hans
>
> vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
>
> 142
> 143 static int skl_int3472_tps68470_probe(struct i2c_client *client)
> 144 {
> 145 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> 146 const struct int3472_tps68470_board_data *board_data;
> 147 struct tps68470_clk_platform_data *clk_pdata;
> 148 unsigned int n_consumers;
> 149 struct mfd_cell *cells;
> 150 struct regmap *regmap;
> 151 int device_type;
> 152 int ret;
> 153
> 154 n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> > 155 if (n_consumers < 0)
> 156 return n_consumers;
> 157
> 158 regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
> 159 if (IS_ERR(regmap)) {
> 160 dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
> 161 return PTR_ERR(regmap);
> 162 }
> 163
> 164 i2c_set_clientdata(client, regmap);
> 165
> 166 ret = tps68470_chip_init(&client->dev, regmap);
> 167 if (ret < 0) {
> 168 dev_err(&client->dev, "TPS68470 init error %d\n", ret);
> 169 return ret;
> 170 }
> 171
> 172 device_type = skl_int3472_tps68470_calc_type(adev);
> 173 switch (device_type) {
> 174 case DESIGNED_FOR_WINDOWS:
> 175 board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
> 176 if (!board_data)
> 177 return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
> 178
> 179 cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
> 180 if (!cells)
> 181 return -ENOMEM;
> 182
> 183 /*
> 184 * The order of the cells matters here! The clk must be first
> 185 * because the regulator depends on it. The gpios must be last,
> 186 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
> 187 * the clk + regulators must be ready when this happens.
> 188 */
> 189 cells[0].name = "tps68470-clk";
> 190 cells[0].platform_data = clk_pdata;
> 191 cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
> 192 cells[1].name = "tps68470-regulator";
> 193 cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
> 194 cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
> 195 cells[2].name = "tps68470-gpio";
> 196
> 197 gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
> 198
> 199 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> 200 cells, TPS68470_WIN_MFD_CELL_COUNT,
> 201 NULL, 0, NULL);
> 202 kfree(cells);
> 203
> 204 if (ret)
> 205 gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
> 206
> 207 break;
> 208 case DESIGNED_FOR_CHROMEOS:
> 209 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> 210 tps68470_cros, ARRAY_SIZE(tps68470_cros),
> 211 NULL, 0, NULL);
> 212 break;
> 213 default:
> 214 dev_err(&client->dev, "Failed to add MFD devices\n");
> 215 return device_type;
> 216 }
> 217
> 218 /*
> 219 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
> 220 * for the GPIO cell already does this.
> 221 */
> 222
> 223 return ret;
> 224 }
> 225
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-22 14:44 ` Hans de Goede
@ 2022-02-22 14:47 ` Daniel Scally
-1 siblings, 0 replies; 26+ messages in thread
From: Daniel Scally @ 2022-02-22 14:47 UTC (permalink / raw)
To: Hans de Goede, kernel test robot, linux-acpi, linux-clk,
platform-driver-x86
Cc: kbuild-all, rafael, lenb, mturquette, sboyd, markgross, robert.moore
Hi Hans
On 22/02/2022 14:44, Hans de Goede wrote:
> Hi,
>
> On 2/17/22 04:22, kernel test robot wrote:
>> Hi Daniel,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on rafael-pm/linux-next]
>> [also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
>> [cannot apply to platform-drivers-x86/for-next]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
>> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp@intel.com/config)
>> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> smatch warnings:
>> drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
> Right this needs to be an int, not an unsigned int. Daniel, please fix this for v2.
Will do! And thanks for your comments / tags on the rest of the series;
I'll post a new version some time this week.
Dan
>
> Regards,
>
> Hans
>
>
>
>> vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
>>
>> 142
>> 143 static int skl_int3472_tps68470_probe(struct i2c_client *client)
>> 144 {
>> 145 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
>> 146 const struct int3472_tps68470_board_data *board_data;
>> 147 struct tps68470_clk_platform_data *clk_pdata;
>> 148 unsigned int n_consumers;
>> 149 struct mfd_cell *cells;
>> 150 struct regmap *regmap;
>> 151 int device_type;
>> 152 int ret;
>> 153
>> 154 n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
>> > 155 if (n_consumers < 0)
>> 156 return n_consumers;
>> 157
>> 158 regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>> 159 if (IS_ERR(regmap)) {
>> 160 dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
>> 161 return PTR_ERR(regmap);
>> 162 }
>> 163
>> 164 i2c_set_clientdata(client, regmap);
>> 165
>> 166 ret = tps68470_chip_init(&client->dev, regmap);
>> 167 if (ret < 0) {
>> 168 dev_err(&client->dev, "TPS68470 init error %d\n", ret);
>> 169 return ret;
>> 170 }
>> 171
>> 172 device_type = skl_int3472_tps68470_calc_type(adev);
>> 173 switch (device_type) {
>> 174 case DESIGNED_FOR_WINDOWS:
>> 175 board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
>> 176 if (!board_data)
>> 177 return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
>> 178
>> 179 cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
>> 180 if (!cells)
>> 181 return -ENOMEM;
>> 182
>> 183 /*
>> 184 * The order of the cells matters here! The clk must be first
>> 185 * because the regulator depends on it. The gpios must be last,
>> 186 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
>> 187 * the clk + regulators must be ready when this happens.
>> 188 */
>> 189 cells[0].name = "tps68470-clk";
>> 190 cells[0].platform_data = clk_pdata;
>> 191 cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
>> 192 cells[1].name = "tps68470-regulator";
>> 193 cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
>> 194 cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
>> 195 cells[2].name = "tps68470-gpio";
>> 196
>> 197 gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
>> 198
>> 199 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> 200 cells, TPS68470_WIN_MFD_CELL_COUNT,
>> 201 NULL, 0, NULL);
>> 202 kfree(cells);
>> 203
>> 204 if (ret)
>> 205 gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
>> 206
>> 207 break;
>> 208 case DESIGNED_FOR_CHROMEOS:
>> 209 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> 210 tps68470_cros, ARRAY_SIZE(tps68470_cros),
>> 211 NULL, 0, NULL);
>> 212 break;
>> 213 default:
>> 214 dev_err(&client->dev, "Failed to add MFD devices\n");
>> 215 return device_type;
>> 216 }
>> 217
>> 218 /*
>> 219 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
>> 220 * for the GPIO cell already does this.
>> 221 */
>> 222
>> 223 return ret;
>> 224 }
>> 225
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
@ 2022-02-22 14:47 ` Daniel Scally
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Scally @ 2022-02-22 14:47 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]
Hi Hans
On 22/02/2022 14:44, Hans de Goede wrote:
> Hi,
>
> On 2/17/22 04:22, kernel test robot wrote:
>> Hi Daniel,
>>
>> I love your patch! Perhaps something to improve:
>>
>> [auto build test WARNING on rafael-pm/linux-next]
>> [also build test WARNING on clk/clk-next linus/master v5.17-rc4 next-20220216]
>> [cannot apply to platform-drivers-x86/for-next]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Add-multiple-consumer-support-to-int3472-tps68470-driver/20220217-065452
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
>> config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220217/202202171110.7EOaTUJH-lkp(a)intel.com/config)
>> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> smatch warnings:
>> drivers/platform/x86/intel/int3472/tps68470.c:155 skl_int3472_tps68470_probe() warn: unsigned 'n_consumers' is never less than zero.
> Right this needs to be an int, not an unsigned int. Daniel, please fix this for v2.
Will do! And thanks for your comments / tags on the rest of the series;
I'll post a new version some time this week.
Dan
>
> Regards,
>
> Hans
>
>
>
>> vim +/n_consumers +155 drivers/platform/x86/intel/int3472/tps68470.c
>>
>> 142
>> 143 static int skl_int3472_tps68470_probe(struct i2c_client *client)
>> 144 {
>> 145 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
>> 146 const struct int3472_tps68470_board_data *board_data;
>> 147 struct tps68470_clk_platform_data *clk_pdata;
>> 148 unsigned int n_consumers;
>> 149 struct mfd_cell *cells;
>> 150 struct regmap *regmap;
>> 151 int device_type;
>> 152 int ret;
>> 153
>> 154 n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
>> > 155 if (n_consumers < 0)
>> 156 return n_consumers;
>> 157
>> 158 regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>> 159 if (IS_ERR(regmap)) {
>> 160 dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
>> 161 return PTR_ERR(regmap);
>> 162 }
>> 163
>> 164 i2c_set_clientdata(client, regmap);
>> 165
>> 166 ret = tps68470_chip_init(&client->dev, regmap);
>> 167 if (ret < 0) {
>> 168 dev_err(&client->dev, "TPS68470 init error %d\n", ret);
>> 169 return ret;
>> 170 }
>> 171
>> 172 device_type = skl_int3472_tps68470_calc_type(adev);
>> 173 switch (device_type) {
>> 174 case DESIGNED_FOR_WINDOWS:
>> 175 board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
>> 176 if (!board_data)
>> 177 return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
>> 178
>> 179 cells = kcalloc(TPS68470_WIN_MFD_CELL_COUNT, sizeof(*cells), GFP_KERNEL);
>> 180 if (!cells)
>> 181 return -ENOMEM;
>> 182
>> 183 /*
>> 184 * The order of the cells matters here! The clk must be first
>> 185 * because the regulator depends on it. The gpios must be last,
>> 186 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
>> 187 * the clk + regulators must be ready when this happens.
>> 188 */
>> 189 cells[0].name = "tps68470-clk";
>> 190 cells[0].platform_data = clk_pdata;
>> 191 cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
>> 192 cells[1].name = "tps68470-regulator";
>> 193 cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
>> 194 cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
>> 195 cells[2].name = "tps68470-gpio";
>> 196
>> 197 gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_table);
>> 198
>> 199 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> 200 cells, TPS68470_WIN_MFD_CELL_COUNT,
>> 201 NULL, 0, NULL);
>> 202 kfree(cells);
>> 203
>> 204 if (ret)
>> 205 gpiod_remove_lookup_table(board_data->tps68470_gpio_lookup_table);
>> 206
>> 207 break;
>> 208 case DESIGNED_FOR_CHROMEOS:
>> 209 ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> 210 tps68470_cros, ARRAY_SIZE(tps68470_cros),
>> 211 NULL, 0, NULL);
>> 212 break;
>> 213 default:
>> 214 dev_err(&client->dev, "Failed to add MFD devices\n");
>> 215 return device_type;
>> 216 }
>> 217
>> 218 /*
>> 219 * No acpi_dev_clear_dependencies() here, since the acpi_gpiochip_add()
>> 220 * for the GPIO cell already does this.
>> 221 */
>> 222
>> 223 return ret;
>> 224 }
>> 225
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-21 9:59 ` Hans de Goede
@ 2022-02-25 0:49 ` Stephen Boyd
2022-02-25 9:22 ` Hans de Goede
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Boyd @ 2022-02-25 0:49 UTC (permalink / raw)
To: Daniel Scally, Hans de Goede, Michael Turquette
Cc: rafael, lenb, markgross, robert.moore, linux-acpi, linux-clk,
platform-driver-x86
Quoting Hans de Goede (2022-02-21 01:59:09)
> Hi,
>
> On 2/16/22 23:53, Daniel Scally wrote:
> > At present, the int3472-tps68470 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.
> >
> > Signed-off-by: Daniel Scally <djrscally@gmail.com>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> Michael, I plan to merge this entire series through the platform-drivers-x86 git
> tree, may I have your ack for merging the clk bits from this ?
>
With the type fix
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers
2022-02-25 0:49 ` Stephen Boyd
@ 2022-02-25 9:22 ` Hans de Goede
0 siblings, 0 replies; 26+ messages in thread
From: Hans de Goede @ 2022-02-25 9:22 UTC (permalink / raw)
To: Stephen Boyd, Daniel Scally, Michael Turquette
Cc: rafael, lenb, markgross, robert.moore, linux-acpi, linux-clk,
platform-driver-x86
Hi Stephen,
On 2/25/22 01:49, Stephen Boyd wrote:
> Quoting Hans de Goede (2022-02-21 01:59:09)
>> Hi,
>>
>> On 2/16/22 23:53, Daniel Scally wrote:
>>> At present, the int3472-tps68470 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.
>>>
>>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>>
>> Thanks, patch looks good to me:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> Michael, I plan to merge this entire series through the platform-drivers-x86 git
>> tree, may I have your ack for merging the clk bits from this ?
>>
>
> With the type fix
>
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> Acked-by: Stephen Boyd <sboyd@kernel.org>
Thanks, and sorry for not addressing you, I should have seen that
there are 2 clk subsys maintainers (and I know from experience
that you are the most active maintainer recently).
Regards,
Hans
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-02-25 9:22 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-16 22:52 [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Daniel Scally
2022-02-16 22:52 ` [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev() Daniel Scally
2022-02-21 9:50 ` Hans de Goede
2022-02-21 9:51 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 2/6] ACPI: bus: Add iterator for dependent devices Daniel Scally
2022-02-21 9:52 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 3/6] platform/x86: int3472: Support multiple clock consumers Daniel Scally
2022-02-17 3:22 ` kernel test robot
2022-02-17 3:22 ` kernel test robot
2022-02-22 14:44 ` Hans de Goede
2022-02-22 14:44 ` Hans de Goede
2022-02-22 14:47 ` Daniel Scally
2022-02-22 14:47 ` Daniel Scally
2022-02-21 9:59 ` Hans de Goede
2022-02-25 0:49 ` Stephen Boyd
2022-02-25 9:22 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 4/6] platform/x86: int3472: Add terminator to gpiod_lookup_table Daniel Scally
2022-02-21 10:00 ` Hans de Goede
2022-02-21 13:46 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 5/6] platform/x86: int3472: Support multiple gpio lookups in board data Daniel Scally
2022-02-21 10:03 ` Hans de Goede
2022-02-16 22:53 ` [PATCH 6/6] platform/x86: int3472: Add board data for Surface Go2 IR camera Daniel Scally
2022-02-21 10:05 ` Hans de Goede
2022-02-17 9:55 ` [PATCH 0/6] Add multiple-consumer support to int3472-tps68470 driver Hans de Goede
2022-02-18 8:45 ` Daniel Scally
2022-02-21 10:07 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.