linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] Introduce intel_skl_int3472 module
@ 2021-06-03 22:40 Daniel Scally
  2021-06-03 22:40 ` [PATCH v5 1/6] ACPI: scan: Extend acpi_walk_dep_device_list() Daniel Scally
                   ` (6 more replies)
  0 siblings, 7 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

Hello all

Bit longer than hoped but here's v5.

v4:
https://lore.kernel.org/lkml/20210520140928.3252671-1-djrscally@gmail.com/

v3
https://lore.kernel.org/lkml/20210222130735.1313443-1-djrscally@gmail.com/

v2
https://lore.kernel.org/platform-driver-x86/20210118003428.568892-1-djrscally@gmail.com/

v1
https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67

The only changes are the dropped patches, renamed functions in 2/6 and most of
Andy's suggestions on 5/6 - I didn't hit them all yet but didn't want to delay
this any more.

Series level changelog:

	- Dropped all but the essential patches to simplify merge plan - thanks
	Hans.

Daniel Scally (6):
  ACPI: scan: Extend acpi_walk_dep_device_list()
  ACPI: scan: Add function to fetch dependent of acpi device
  gpiolib: acpi: Export acpi_get_gpiod()
  gpiolib: acpi: Add acpi_gpio_get_io_resource()
  platform/x86: Add intel_skl_int3472 driver
  mfd: tps68470: Remove tps68470 MFD driver

 MAINTAINERS                                   |   5 +
 drivers/acpi/ec.c                             |   2 +-
 drivers/acpi/pmic/Kconfig                     |   2 +-
 drivers/acpi/pmic/intel_pmic_chtdc_ti.c       |   2 +-
 drivers/acpi/scan.c                           | 104 ++++-
 drivers/gpio/Kconfig                          |   2 +-
 drivers/gpio/gpiolib-acpi.c                   |  61 ++-
 drivers/i2c/i2c-core-acpi.c                   |   8 +-
 drivers/mfd/Kconfig                           |  18 -
 drivers/mfd/Makefile                          |   1 -
 drivers/mfd/tps68470.c                        |  97 ----
 drivers/platform/surface/aggregator/core.c    |   6 +-
 drivers/platform/surface/surface3_power.c     |  22 +-
 .../platform/surface/surface_acpi_notify.c    |   7 +-
 drivers/platform/x86/Kconfig                  |   2 +
 drivers/platform/x86/Makefile                 |   1 +
 drivers/platform/x86/intel-int3472/Kconfig    |  30 ++
 drivers/platform/x86/intel-int3472/Makefile   |   5 +
 .../intel_skl_int3472_clk_and_regulator.c     | 196 ++++++++
 .../intel-int3472/intel_skl_int3472_common.c  | 106 +++++
 .../intel-int3472/intel_skl_int3472_common.h  | 118 +++++
 .../intel_skl_int3472_discrete.c              | 417 ++++++++++++++++++
 .../intel_skl_int3472_tps68470.c              | 137 ++++++
 include/acpi/acpi_bus.h                       |   8 +
 include/linux/acpi.h                          |  11 +-
 include/linux/gpio/consumer.h                 |   2 +
 26 files changed, 1205 insertions(+), 165 deletions(-)
 delete mode 100644 drivers/mfd/tps68470.c
 create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
 create mode 100644 drivers/platform/x86/intel-int3472/Makefile
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v5 1/6] ACPI: scan: Extend acpi_walk_dep_device_list()
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
@ 2021-06-03 22:40 ` Daniel Scally
  2021-06-03 22:40 ` [PATCH v5 2/6] ACPI: scan: Add function to fetch dependent of acpi device Daniel Scally
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

The acpi_walk_dep_device_list() is not as generalisable as its name
implies, serving only to decrement the dependency count for each
dependent device of the input. Extend the function to instead accept
a callback which can be applied to all the dependencies in acpi_dep_list.
Replace all existing calls to the function with calls to a wrapper, passing
a callback that applies the same dependency reduction.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Maximilian Luz <luzmaximilian@gmail.com>  # for platform/surface parts
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v4:
	- None

 drivers/acpi/ec.c                             |  2 +-
 drivers/acpi/pmic/intel_pmic_chtdc_ti.c       |  2 +-
 drivers/acpi/scan.c                           | 69 ++++++++++++++-----
 drivers/gpio/gpiolib-acpi.c                   | 10 +--
 drivers/i2c/i2c-core-acpi.c                   |  8 +--
 drivers/platform/surface/aggregator/core.c    |  6 +-
 drivers/platform/surface/surface3_power.c     | 22 +++---
 .../platform/surface/surface_acpi_notify.c    |  7 +-
 include/acpi/acpi_bus.h                       |  7 ++
 include/linux/acpi.h                          |  4 +-
 10 files changed, 90 insertions(+), 47 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 13565629ce0a..3f7680a007a3 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1627,7 +1627,7 @@ static int acpi_ec_add(struct acpi_device *device)
 	WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr);
 
 	/* Reprobe devices depending on the EC */
-	acpi_walk_dep_device_list(ec->handle);
+	acpi_dev_clear_dependencies(device);
 
 	acpi_handle_debug(ec->handle, "enumerated.\n");
 	return 0;
diff --git a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
index a5101b07611a..fef7831d0d63 100644
--- a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
+++ b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c
@@ -117,7 +117,7 @@ static int chtdc_ti_pmic_opregion_probe(struct platform_device *pdev)
 		return err;
 
 	/* Re-enumerate devices depending on PMIC */
-	acpi_walk_dep_device_list(ACPI_HANDLE(pdev->dev.parent));
+	acpi_dev_clear_dependencies(ACPI_COMPANION(pdev->dev.parent));
 	return 0;
 }
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 453eff8ec8c3..195635c3462b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -47,12 +47,6 @@ static DEFINE_MUTEX(acpi_hp_context_lock);
  */
 static u64 spcr_uart_addr;
 
-struct acpi_dep_data {
-	struct list_head node;
-	acpi_handle supplier;
-	acpi_handle consumer;
-};
-
 void acpi_scan_lock_acquire(void)
 {
 	mutex_lock(&acpi_scan_lock);
@@ -2111,30 +2105,69 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass)
 		device->handler->hotplug.notify_online(device);
 }
 
-void acpi_walk_dep_device_list(acpi_handle handle)
+static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data)
 {
-	struct acpi_dep_data *dep, *tmp;
 	struct acpi_device *adev;
 
+	acpi_bus_get_device(dep->consumer, &adev);
+
+	if (adev) {
+		adev->dep_unmet--;
+		if (!adev->dep_unmet)
+			acpi_bus_attach(adev, true);
+	}
+
+	list_del(&dep->node);
+	kfree(dep);
+
+	return 0;
+}
+
+/**
+ * acpi_walk_dep_device_list - Apply a callback to every entry in acpi_dep_list
+ * @handle:	The ACPI handle of the supplier device
+ * @callback:	Pointer to the callback function to apply
+ * @data:	Pointer to some data to pass to the callback
+ *
+ * The return value of the callback determines this function's behaviour. If 0
+ * is returned we continue to iterate over acpi_dep_list. If a positive value
+ * is returned then the loop is broken but this function returns 0. If a
+ * negative value is returned by the callback then the loop is broken and that
+ * value is returned as the final error.
+ */
+int acpi_walk_dep_device_list(acpi_handle handle,
+			      int (*callback)(struct acpi_dep_data *, void *),
+			      void *data)
+{
+	struct acpi_dep_data *dep, *tmp;
+	int ret;
+
 	mutex_lock(&acpi_dep_list_lock);
 	list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) {
 		if (dep->supplier == handle) {
-			acpi_bus_get_device(dep->consumer, &adev);
-
-			if (adev) {
-				adev->dep_unmet--;
-				if (!adev->dep_unmet)
-					acpi_bus_attach(adev, true);
-			}
-
-			list_del(&dep->node);
-			kfree(dep);
+			ret = callback(dep, data);
+			if (ret)
+				break;
 		}
 	}
 	mutex_unlock(&acpi_dep_list_lock);
+
+	return ret > 0 ? 0 : ret;
 }
 EXPORT_SYMBOL_GPL(acpi_walk_dep_device_list);
 
+/**
+ * acpi_dev_clear_dependencies - Inform consumers that the device is now active
+ * @supplier: Pointer to the supplier &struct acpi_device
+ *
+ * Clear dependencies on the given device.
+ */
+void acpi_dev_clear_dependencies(struct acpi_device *supplier)
+{
+	acpi_walk_dep_device_list(supplier->handle, acpi_scan_clear_dep, NULL);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies);
+
 /**
  * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
  * @handle: Root of the namespace scope to scan.
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 3ef22a3c104d..5b4111e4be3f 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1233,14 +1233,14 @@ static void acpi_gpiochip_scan_gpios(struct acpi_gpio_chip *achip)
 void acpi_gpiochip_add(struct gpio_chip *chip)
 {
 	struct acpi_gpio_chip *acpi_gpio;
-	acpi_handle handle;
+	struct acpi_device *adev;
 	acpi_status status;
 
 	if (!chip || !chip->parent)
 		return;
 
-	handle = ACPI_HANDLE(chip->parent);
-	if (!handle)
+	adev = ACPI_COMPANION(chip->parent);
+	if (!adev)
 		return;
 
 	acpi_gpio = kzalloc(sizeof(*acpi_gpio), GFP_KERNEL);
@@ -1254,7 +1254,7 @@ void acpi_gpiochip_add(struct gpio_chip *chip)
 	INIT_LIST_HEAD(&acpi_gpio->events);
 	INIT_LIST_HEAD(&acpi_gpio->deferred_req_irqs_list_entry);
 
-	status = acpi_attach_data(handle, acpi_gpio_chip_dh, acpi_gpio);
+	status = acpi_attach_data(adev->handle, acpi_gpio_chip_dh, acpi_gpio);
 	if (ACPI_FAILURE(status)) {
 		dev_err(chip->parent, "Failed to attach ACPI GPIO chip\n");
 		kfree(acpi_gpio);
@@ -1263,7 +1263,7 @@ void acpi_gpiochip_add(struct gpio_chip *chip)
 
 	acpi_gpiochip_request_regions(acpi_gpio);
 	acpi_gpiochip_scan_gpios(acpi_gpio);
-	acpi_walk_dep_device_list(handle);
+	acpi_dev_clear_dependencies(adev);
 }
 
 void acpi_gpiochip_remove(struct gpio_chip *chip)
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index 8ceaa88dd78f..6f0aa0ed3241 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -259,8 +259,8 @@ static acpi_status i2c_acpi_add_device(acpi_handle handle, u32 level,
  */
 void i2c_acpi_register_devices(struct i2c_adapter *adap)
 {
+	struct acpi_device *adev;
 	acpi_status status;
-	acpi_handle handle;
 
 	if (!has_acpi_companion(&adap->dev))
 		return;
@@ -275,11 +275,11 @@ void i2c_acpi_register_devices(struct i2c_adapter *adap)
 	if (!adap->dev.parent)
 		return;
 
-	handle = ACPI_HANDLE(adap->dev.parent);
-	if (!handle)
+	adev = ACPI_COMPANION(adap->dev.parent);
+	if (!adev)
 		return;
 
-	acpi_walk_dep_device_list(handle);
+	acpi_dev_clear_dependencies(adev);
 }
 
 static const struct acpi_device_id i2c_acpi_force_400khz_device_ids[] = {
diff --git a/drivers/platform/surface/aggregator/core.c b/drivers/platform/surface/aggregator/core.c
index 8dc2c267bcd6..517f774a6e60 100644
--- a/drivers/platform/surface/aggregator/core.c
+++ b/drivers/platform/surface/aggregator/core.c
@@ -621,8 +621,8 @@ static const struct acpi_gpio_mapping ssam_acpi_gpios[] = {
 
 static int ssam_serial_hub_probe(struct serdev_device *serdev)
 {
+	struct acpi_device *ssh = ACPI_COMPANION(&serdev->dev);
 	struct ssam_controller *ctrl;
-	acpi_handle *ssh = ACPI_HANDLE(&serdev->dev);
 	acpi_status astatus;
 	int status;
 
@@ -652,7 +652,7 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev)
 	if (status)
 		goto err_devopen;
 
-	astatus = ssam_serdev_setup_via_acpi(ssh, serdev);
+	astatus = ssam_serdev_setup_via_acpi(ssh->handle, serdev);
 	if (ACPI_FAILURE(astatus)) {
 		status = -ENXIO;
 		goto err_devinit;
@@ -706,7 +706,7 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev)
 	 *       For now let's thus default power/wakeup to false.
 	 */
 	device_set_wakeup_capable(&serdev->dev, true);
-	acpi_walk_dep_device_list(ssh);
+	acpi_dev_clear_dependencies(ssh);
 
 	return 0;
 
diff --git a/drivers/platform/surface/surface3_power.c b/drivers/platform/surface/surface3_power.c
index cc4f9cba6856..dea82aa1abd4 100644
--- a/drivers/platform/surface/surface3_power.c
+++ b/drivers/platform/surface/surface3_power.c
@@ -446,12 +446,12 @@ mshw0011_space_handler(u32 function, acpi_physical_address command,
 
 static int mshw0011_install_space_handler(struct i2c_client *client)
 {
-	acpi_handle handle;
+	struct acpi_device *adev;
 	struct mshw0011_handler_data *data;
 	acpi_status status;
 
-	handle = ACPI_HANDLE(&client->dev);
-	if (!handle)
+	adev = ACPI_COMPANION(&client->dev);
+	if (!adev)
 		return -ENODEV;
 
 	data = kzalloc(sizeof(struct mshw0011_handler_data),
@@ -460,25 +460,25 @@ static int mshw0011_install_space_handler(struct i2c_client *client)
 		return -ENOMEM;
 
 	data->client = client;
-	status = acpi_bus_attach_private_data(handle, (void *)data);
+	status = acpi_bus_attach_private_data(adev->handle, (void *)data);
 	if (ACPI_FAILURE(status)) {
 		kfree(data);
 		return -ENOMEM;
 	}
 
-	status = acpi_install_address_space_handler(handle,
-				ACPI_ADR_SPACE_GSBUS,
-				&mshw0011_space_handler,
-				NULL,
-				data);
+	status = acpi_install_address_space_handler(adev->handle,
+						    ACPI_ADR_SPACE_GSBUS,
+						    &mshw0011_space_handler,
+						    NULL,
+						    data);
 	if (ACPI_FAILURE(status)) {
 		dev_err(&client->dev, "Error installing i2c space handler\n");
-		acpi_bus_detach_private_data(handle);
+		acpi_bus_detach_private_data(adev->handle);
 		kfree(data);
 		return -ENOMEM;
 	}
 
-	acpi_walk_dep_device_list(handle);
+	acpi_dev_clear_dependencies(adev);
 	return 0;
 }
 
diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c
index ef9c1f8e8336..8339988d95c1 100644
--- a/drivers/platform/surface/surface_acpi_notify.c
+++ b/drivers/platform/surface/surface_acpi_notify.c
@@ -798,7 +798,7 @@ static int san_consumer_links_setup(struct platform_device *pdev)
 
 static int san_probe(struct platform_device *pdev)
 {
-	acpi_handle san = ACPI_HANDLE(&pdev->dev);
+	struct acpi_device *san = ACPI_COMPANION(&pdev->dev);
 	struct ssam_controller *ctrl;
 	struct san_data *data;
 	acpi_status astatus;
@@ -821,7 +821,8 @@ static int san_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, data);
 
-	astatus = acpi_install_address_space_handler(san, ACPI_ADR_SPACE_GSBUS,
+	astatus = acpi_install_address_space_handler(san->handle,
+						     ACPI_ADR_SPACE_GSBUS,
 						     &san_opreg_handler, NULL,
 						     &data->info);
 	if (ACPI_FAILURE(astatus))
@@ -835,7 +836,7 @@ static int san_probe(struct platform_device *pdev)
 	if (status)
 		goto err_install_dev;
 
-	acpi_walk_dep_device_list(san);
+	acpi_dev_clear_dependencies(san);
 	return 0;
 
 err_install_dev:
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 3a82faac5767..0b2c4f170f4d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -280,6 +280,12 @@ struct acpi_device_power {
 	struct acpi_device_power_state states[ACPI_D_STATE_COUNT];	/* Power states (D0-D3Cold) */
 };
 
+struct acpi_dep_data {
+	struct list_head node;
+	acpi_handle supplier;
+	acpi_handle consumer;
+};
+
 /* Performance Management */
 
 struct acpi_device_perf_flags {
@@ -685,6 +691,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
 
 bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
 
+void acpi_dev_clear_dependencies(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);
 struct acpi_device *
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c60745f657e9..170b9bebdb2b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -666,7 +666,9 @@ extern bool acpi_driver_match_device(struct device *dev,
 				     const struct device_driver *drv);
 int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
 int acpi_device_modalias(struct device *, char *, int);
-void acpi_walk_dep_device_list(acpi_handle handle);
+int acpi_walk_dep_device_list(acpi_handle handle,
+			      int (*callback)(struct acpi_dep_data *, void *),
+			      void *data);
 
 struct platform_device *acpi_create_platform_device(struct acpi_device *,
 						    struct property_entry *);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 2/6] ACPI: scan: Add function to fetch dependent of acpi device
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
  2021-06-03 22:40 ` [PATCH v5 1/6] ACPI: scan: Extend acpi_walk_dep_device_list() Daniel Scally
@ 2021-06-03 22:40 ` Daniel Scally
  2021-06-07 14:52   ` Rafael J. Wysocki
  2021-06-03 22:40 ` [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod() Daniel Scally
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham, Andy Shevchenko

In some ACPI tables we encounter, devices use the _DEP method to assert
a dependence on other ACPI devices as opposed to the OpRegions that the
specification intends. We need to be able to find those devices "from"
the dependee, so add a callback and a wrapper to walk over the
acpi_dep_list and return the dependent ACPI device.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v5:

	- Functions renamed

 drivers/acpi/scan.c     | 35 +++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 195635c3462b..9af64c34e286 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2105,6 +2105,20 @@ 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)
+{
+	struct acpi_device *adev;
+
+	adev = acpi_bus_get_acpi_device(dep->consumer);
+	if (!adev)
+		/* If we don't find an adev then we want to continue parsing */
+		return 0;
+
+	*(struct acpi_device **)data = adev;
+
+	return 1;
+}
+
 static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data)
 {
 	struct acpi_device *adev;
@@ -2168,6 +2182,27 @@ void acpi_dev_clear_dependencies(struct acpi_device *supplier)
 }
 EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies);
 
+/**
+ * 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)
+{
+	struct acpi_device *adev = NULL;
+
+	acpi_walk_dep_device_list(supplier->handle,
+				  acpi_dev_get_first_consumer_dev_cb, &adev);
+
+	return adev;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
+
 /**
  * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
  * @handle: Root of the namespace scope to scan.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 0b2c4f170f4d..4bed30e61c5b 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -692,6 +692,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
 bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
 
 void acpi_dev_clear_dependencies(struct acpi_device *supplier);
+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);
 struct acpi_device *
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod()
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
  2021-06-03 22:40 ` [PATCH v5 1/6] ACPI: scan: Extend acpi_walk_dep_device_list() Daniel Scally
  2021-06-03 22:40 ` [PATCH v5 2/6] ACPI: scan: Add function to fetch dependent of acpi device Daniel Scally
@ 2021-06-03 22:40 ` Daniel Scally
  2021-06-04 12:59   ` Andy Shevchenko
  2021-06-03 22:40 ` [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource() Daniel Scally
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham, Andy Shevchenko

We need to be able to translate GPIO resources in an ACPI device's _CRS
into GPIO descriptor array. Those are represented in _CRS as a pathname
to a GPIO device plus the pin's index number: the acpi_get_gpiod()
function is perfect for that purpose.

As it's currently only used internally within the GPIO layer, provide and
export a wrapper function that additionally holds a reference to the GPIO
device.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v4:
	- None

 drivers/gpio/gpiolib-acpi.c   | 28 ++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 5b4111e4be3f..684ddb35d83b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -128,6 +128,34 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
 	return gpiochip_get_desc(chip, pin);
 }
 
+/**
+ * acpi_get_and_request_gpiod() - Translate ACPI GPIO pin to GPIO descriptor
+ *                               and hold a refcount to the GPIO device.
+ * @path:      ACPI GPIO controller full path name, (e.g. "\\_SB.GPO1")
+ * @pin:       ACPI GPIO pin number (0-based, controller-relative)
+ * @label:     Label to pass to gpiod_request()
+ *
+ * This function is a simple pass-through to acpi_get_gpiod(), except that
+ * as it is intended for use outside of the GPIO layer (in a similar fashion to
+ * gpiod_get_index() for example) it also holds a reference to the GPIO device.
+ */
+struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label)
+{
+	struct gpio_desc *gpio;
+	int ret;
+
+	gpio = acpi_get_gpiod(path, pin);
+	if (IS_ERR(gpio))
+		return gpio;
+
+	ret = gpiod_request(gpio, label);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return gpio;
+}
+EXPORT_SYMBOL_GPL(acpi_get_and_request_gpiod);
+
 static irqreturn_t acpi_gpio_irq_handler(int irq, void *data)
 {
 	struct acpi_gpio_event *event = data;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index c73b25bc9213..566feb56601f 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -692,6 +692,8 @@ int devm_acpi_dev_add_driver_gpios(struct device *dev,
 				   const struct acpi_gpio_mapping *gpios);
 void devm_acpi_dev_remove_driver_gpios(struct device *dev);
 
+struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label);
+
 #else  /* CONFIG_GPIOLIB && CONFIG_ACPI */
 
 struct acpi_device;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource()
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
                   ` (2 preceding siblings ...)
  2021-06-03 22:40 ` [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod() Daniel Scally
@ 2021-06-03 22:40 ` Daniel Scally
  2021-06-04 13:01   ` Andy Shevchenko
  2021-06-03 22:40 ` [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver Daniel Scally
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

Add a function to verify that a given acpi_resource represents an IO
type GPIO resource, and return it if so.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v4:
	- None

 drivers/gpio/gpiolib-acpi.c | 23 +++++++++++++++++++++++
 include/linux/acpi.h        |  7 +++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 684ddb35d83b..9887bb684575 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -196,6 +196,29 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
 }
 EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource);
 
+/**
+ * acpi_gpio_get_io_resource - Fetch details of an ACPI resource if it is a GPIO
+ *			       I/O resource or return False if not.
+ * @ares:	Pointer to the ACPI resource to fetch
+ * @agpio:	Pointer to a &struct acpi_resource_gpio to store the output pointer
+ */
+bool acpi_gpio_get_io_resource(struct acpi_resource *ares,
+			       struct acpi_resource_gpio **agpio)
+{
+	struct acpi_resource_gpio *gpio;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
+		return false;
+
+	gpio = &ares->data.gpio;
+	if (gpio->connection_type != ACPI_RESOURCE_GPIO_TYPE_IO)
+		return false;
+
+	*agpio = gpio;
+	return true;
+}
+EXPORT_SYMBOL_GPL(acpi_gpio_get_io_resource);
+
 static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
 				      struct acpi_gpio_event *event)
 {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 170b9bebdb2b..e8ba7063c000 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1098,6 +1098,8 @@ void __acpi_handle_debug(struct _ddebug *descriptor, acpi_handle handle, const c
 #if defined(CONFIG_ACPI) && defined(CONFIG_GPIOLIB)
 bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
 				struct acpi_resource_gpio **agpio);
+bool acpi_gpio_get_io_resource(struct acpi_resource *ares,
+			       struct acpi_resource_gpio **agpio);
 int acpi_dev_gpio_irq_get_by(struct acpi_device *adev, const char *name, int index);
 #else
 static inline bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
@@ -1105,6 +1107,11 @@ static inline bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
 {
 	return false;
 }
+static inline bool acpi_gpio_get_io_resource(struct acpi_resource *ares,
+					     struct acpi_resource_gpio **agpio)
+{
+	return false;
+}
 static inline int acpi_dev_gpio_irq_get_by(struct acpi_device *adev,
 					   const char *name, int index)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
                   ` (3 preceding siblings ...)
  2021-06-03 22:40 ` [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource() Daniel Scally
@ 2021-06-03 22:40 ` Daniel Scally
  2021-06-04  1:31   ` kernel test robot
                     ` (2 more replies)
  2021-06-03 22:40 ` [PATCH v5 6/6] mfd: tps68470: Remove tps68470 MFD driver Daniel Scally
  2021-06-08  9:00 ` [PATCH v5 0/6] Introduce intel_skl_int3472 module Hans de Goede
  6 siblings, 3 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

ACPI devices with _HID INT3472 are currently matched to the tps68470
driver, however this does not cover all situations in which that _HID
occurs. We've encountered three possibilities:

1. On Chrome OS devices, an ACPI device with _HID INT3472 (representing
a physical TPS68470 device) that requires a GPIO and OpRegion driver
2. On devices designed for Windows, an ACPI device with _HID INT3472
(again representing a physical TPS68470 device) which requires GPIO,
Clock and Regulator drivers.
3. On other devices designed for Windows, an ACPI device with _HID
INT3472 which does **not** represent a physical TPS68470, and is instead
used as a dummy device to group some system GPIO lines which are meant
to be consumed by the sensor that is dependent on this entry.

This commit adds a new module, registering a platform driver to deal
with the 3rd scenario plus an i2c driver to deal with #1 and #2, by
querying the CLDB buffer found against INT3472 entries to determine
which is most appropriate.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v4:
	- Some formatting and commenting tidy up
	- Cleared up some function exit paths to avoid gotos
	- Refactored skl_int3472_get_sensor_module_config() and also
	skl_int3472_tps68470_probe(), which involved adding a new function.

Andy - I didn't look at the reg-fixed-voltage suggestion yet, but I think I
covered the rest of your suggestions from v4.

 MAINTAINERS                                   |   5 +
 drivers/platform/x86/Kconfig                  |   2 +
 drivers/platform/x86/Makefile                 |   1 +
 drivers/platform/x86/intel-int3472/Kconfig    |  30 ++
 drivers/platform/x86/intel-int3472/Makefile   |   5 +
 .../intel_skl_int3472_clk_and_regulator.c     | 196 ++++++++
 .../intel-int3472/intel_skl_int3472_common.c  | 106 +++++
 .../intel-int3472/intel_skl_int3472_common.h  | 118 +++++
 .../intel_skl_int3472_discrete.c              | 417 ++++++++++++++++++
 .../intel_skl_int3472_tps68470.c              | 137 ++++++
 10 files changed, 1017 insertions(+)
 create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
 create mode 100644 drivers/platform/x86/intel-int3472/Makefile
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 008fcad7ac00..8dcc7e005ace 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9378,6 +9378,11 @@ S:	Maintained
 F:	arch/x86/include/asm/intel_scu_ipc.h
 F:	drivers/platform/x86/intel_scu_*
 
+INTEL SKYLAKE INT3472 ACPI DEVICE DRIVER
+M:	Daniel Scally <djrscally@gmail.com>
+S:	Maintained
+F:	drivers/platform/x86/intel-int3472/
+
 INTEL SPEED SELECT TECHNOLOGY
 M:	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
 L:	platform-driver-x86@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 2714f7c3843e..1263b8efcd50 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -697,6 +697,8 @@ config INTEL_CHT_INT33FE
 	  device and CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m
 	  for Type-C device.
 
+source "drivers/platform/x86/intel-int3472/Kconfig"
+
 config INTEL_HID_EVENT
 	tristate "INTEL HID Event"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index dcc8cdb95b4d..c0612c02d037 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_INTEL_HID_EVENT)		+= intel-hid.o
 obj-$(CONFIG_INTEL_INT0002_VGPIO)	+= intel_int0002_vgpio.o
 obj-$(CONFIG_INTEL_MENLOW)		+= intel_menlow.o
 obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
+obj-$(CONFIG_INTEL_SKL_INT3472)		+= intel-int3472/
 obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
 
 # MSI
diff --git a/drivers/platform/x86/intel-int3472/Kconfig b/drivers/platform/x86/intel-int3472/Kconfig
new file mode 100644
index 000000000000..c112878e833b
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/Kconfig
@@ -0,0 +1,30 @@
+config INTEL_SKL_INT3472
+	tristate "Intel SkyLake ACPI INT3472 Driver"
+	depends on ACPI
+	depends on COMMON_CLK && CLKDEV_LOOKUP
+	depends on I2C
+	depends on GPIOLIB
+	depends on REGULATOR
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  This driver adds power controller support for the Intel SkyCam
+	  devices found on the Intel SkyLake platforms.
+
+	  The INT3472 is a camera power controller, a logical device found on
+	  Intel Skylake-based systems that can map to different hardware
+	  devices depending on the platform. On machines designed for Chrome OS
+	  it maps to a TPS68470 camera PMIC. On machines designed for Windows,
+	  it maps to either a TP68470 camera PMIC, a uP6641Q sensor PMIC, or a
+	  set of discrete GPIOs and power gates.
+
+	  If your device was designed for Chrome OS, this driver will provide
+	  an ACPI OpRegion, which must be available before any of the devices
+	  using it are probed. For this reason, you should select Y if your
+	  device was designed for ChromeOS. For the same reason the
+	  I2C_DESIGNWARE_PLATFORM option must be set to Y too.
+
+	  Say Y or M here if you have a SkyLake device designed for use
+	  with Windows or ChromeOS. Say N here if you are not sure.
+
+	  The module will be named "intel-skl-int3472".
diff --git a/drivers/platform/x86/intel-int3472/Makefile b/drivers/platform/x86/intel-int3472/Makefile
new file mode 100644
index 000000000000..48bd97f0a04e
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/Makefile
@@ -0,0 +1,5 @@
+obj-$(CONFIG_INTEL_SKL_INT3472)		+= intel_skl_int3472.o
+intel_skl_int3472-objs			:= intel_skl_int3472_common.o \
+					   intel_skl_int3472_discrete.o \
+					   intel_skl_int3472_tps68470.o \
+					   intel_skl_int3472_clk_and_regulator.o
diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
new file mode 100644
index 000000000000..ceee860e2c07
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Author: Dan Scally <djrscally@gmail.com> */
+
+#include <linux/acpi.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/slab.h>
+
+#include "intel_skl_int3472_common.h"
+
+/*
+ * The regulators have to have .ops to be valid, but the only ops we actually
+ * support are .enable and .disable which are handled via .ena_gpiod. Pass an
+ * empty struct to clear the check without lying about capabilities.
+ */
+static const struct regulator_ops int3472_gpio_regulator_ops;
+
+static int skl_int3472_clk_prepare(struct clk_hw *hw)
+{
+	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
+
+	gpiod_set_value_cansleep(clk->ena_gpio, 1);
+	gpiod_set_value_cansleep(clk->led_gpio, 1);
+
+	return 0;
+}
+
+static void skl_int3472_clk_unprepare(struct clk_hw *hw)
+{
+	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
+
+	gpiod_set_value_cansleep(clk->ena_gpio, 0);
+	gpiod_set_value_cansleep(clk->led_gpio, 0);
+}
+
+static int skl_int3472_clk_enable(struct clk_hw *hw)
+{
+	/*
+	 * We're just turning a GPIO on to enable the clock, which operation
+	 * has the potential to sleep. Given .enable() cannot sleep, but
+	 * .prepare() can, we toggle the GPIO in .prepare() instead. Thus,
+	 * nothing to do here.
+	 */
+	return 0;
+}
+
+static void skl_int3472_clk_disable(struct clk_hw *hw)
+{
+	/* Likewise, nothing to do here... */
+}
+
+static unsigned int skl_int3472_get_clk_frequency(struct int3472_discrete_device *int3472)
+{
+	union acpi_object *obj;
+	unsigned int freq;
+
+	obj = skl_int3472_get_acpi_buffer(int3472->sensor, "SSDB");
+	if (IS_ERR(obj))
+		return 0; /* report rate as 0 on error */
+
+	if (obj->buffer.length < CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET + sizeof(u32)) {
+		dev_err(int3472->dev, "The buffer is too small\n");
+		kfree(obj);
+		return 0;
+	}
+
+	freq = *(u32 *)(obj->buffer.pointer + CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET);
+
+	kfree(obj);
+	return freq;
+}
+
+static unsigned long skl_int3472_clk_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
+
+	return clk->frequency;
+}
+
+static const struct clk_ops skl_int3472_clock_ops = {
+	.prepare = skl_int3472_clk_prepare,
+	.unprepare = skl_int3472_clk_unprepare,
+	.enable = skl_int3472_clk_enable,
+	.disable = skl_int3472_clk_disable,
+	.recalc_rate = skl_int3472_clk_recalc_rate,
+};
+
+int skl_int3472_register_clock(struct int3472_discrete_device *int3472)
+{
+	struct clk_init_data init = {
+		.ops = &skl_int3472_clock_ops,
+		.flags = CLK_GET_RATE_NOCACHE,
+	};
+	int ret;
+
+	init.name = kasprintf(GFP_KERNEL, "%s-clk",
+			      acpi_dev_name(int3472->adev));
+	if (!init.name)
+		return -ENOMEM;
+
+	int3472->clock.frequency = skl_int3472_get_clk_frequency(int3472);
+
+	int3472->clock.clk_hw.init = &init;
+	int3472->clock.clk = clk_register(&int3472->adev->dev,
+					  &int3472->clock.clk_hw);
+	if (IS_ERR(int3472->clock.clk)) {
+		ret = PTR_ERR(int3472->clock.clk);
+		goto out_free_init_name;
+	}
+
+	int3472->clock.cl = clkdev_create(int3472->clock.clk, NULL,
+					  int3472->sensor_name);
+	if (!int3472->clock.cl) {
+		ret = -ENOMEM;
+		goto err_unregister_clk;
+	}
+
+	kfree(init.name);
+	return 0;
+
+err_unregister_clk:
+	clk_unregister(int3472->clock.clk);
+out_free_init_name:
+	kfree(init.name);
+
+	return ret;
+}
+
+int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
+				   struct acpi_resource *ares)
+{
+	char *path = ares->data.gpio.resource_source.string_ptr;
+	const struct int3472_sensor_config *sensor_config;
+	struct regulator_consumer_supply supply_map;
+	struct regulator_init_data init_data = { };
+	struct regulator_config cfg = { };
+	int ret;
+
+	sensor_config = int3472->sensor_config;
+	if (IS_ERR(sensor_config)) {
+		dev_err(int3472->dev, "No sensor module config\n");
+		return PTR_ERR(sensor_config);
+	}
+
+	if (!sensor_config->supply_map.supply) {
+		dev_err(int3472->dev, "No supply name defined\n");
+		return -ENODEV;
+	}
+
+	init_data.constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
+	init_data.num_consumer_supplies = 1;
+	supply_map = sensor_config->supply_map;
+	supply_map.dev_name = int3472->sensor_name;
+	init_data.consumer_supplies = &supply_map;
+
+	snprintf(int3472->regulator.regulator_name,
+		 sizeof(int3472->regulator.regulator_name), "%s-regulator",
+		 acpi_dev_name(int3472->adev));
+	snprintf(int3472->regulator.supply_name,
+		 GPIO_REGULATOR_SUPPLY_NAME_LENGTH, "supply-0");
+
+	int3472->regulator.rdesc = INT3472_REGULATOR(
+						int3472->regulator.regulator_name,
+						int3472->regulator.supply_name,
+						&int3472_gpio_regulator_ops);
+
+	int3472->regulator.gpio = acpi_get_and_request_gpiod(path,
+							     ares->data.gpio.pin_table[0],
+							     "int3472,regulator");
+	if (IS_ERR(int3472->regulator.gpio)) {
+		dev_err(int3472->dev, "Failed to get regulator GPIO line\n");
+		return PTR_ERR(int3472->regulator.gpio);
+	}
+
+	cfg.dev = &int3472->adev->dev;
+	cfg.init_data = &init_data;
+	cfg.ena_gpiod = int3472->regulator.gpio;
+
+	int3472->regulator.rdev = regulator_register(&int3472->regulator.rdesc,
+						     &cfg);
+	if (IS_ERR(int3472->regulator.rdev)) {
+		ret = PTR_ERR(int3472->regulator.rdev);
+		goto err_free_gpio;
+	}
+
+	return 0;
+
+err_free_gpio:
+	gpiod_put(int3472->regulator.gpio);
+
+	return ret;
+}
diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
new file mode 100644
index 000000000000..497e74fba75f
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Author: Dan Scally <djrscally@gmail.com> */
+
+#include <linux/acpi.h>
+#include <linux/i2c.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "intel_skl_int3472_common.h"
+
+union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev, char *id)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_handle handle = adev->handle;
+	union acpi_object *obj;
+	acpi_status status;
+
+	status = acpi_evaluate_object(handle, id, NULL, &buffer);
+	if (ACPI_FAILURE(status))
+		return ERR_PTR(-ENODEV);
+
+	obj = buffer.pointer;
+	if (!obj)
+		return ERR_PTR(-ENODEV);
+
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		acpi_handle_err(handle, "%s object is not an ACPI buffer\n", id);
+		kfree(obj);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return obj;
+}
+
+int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
+{
+	union acpi_object *obj;
+	int ret;
+
+	obj = skl_int3472_get_acpi_buffer(adev, "CLDB");
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
+
+	if (obj->buffer.length > sizeof(*cldb)) {
+		acpi_handle_err(adev->handle, "The CLDB buffer is too large\n");
+		ret = -EINVAL;
+		goto out_free_obj;
+	}
+
+	memcpy(cldb, obj->buffer.pointer, obj->buffer.length);
+	ret = 0;
+
+out_free_obj:
+	kfree(obj);
+	return ret;
+}
+
+static const struct acpi_device_id int3472_device_id[] = {
+	{ "INT3472", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, int3472_device_id);
+
+static struct platform_driver int3472_discrete = {
+	.driver = {
+		.name = "int3472-discrete",
+		.acpi_match_table = int3472_device_id,
+	},
+	.probe = skl_int3472_discrete_probe,
+	.remove = skl_int3472_discrete_remove,
+};
+
+static struct i2c_driver int3472_tps68470 = {
+	.driver = {
+		.name = "int3472-tps68470",
+		.acpi_match_table = int3472_device_id,
+	},
+	.probe_new = skl_int3472_tps68470_probe,
+};
+
+static int skl_int3472_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&int3472_discrete);
+	if (ret)
+		return ret;
+
+	ret = i2c_register_driver(THIS_MODULE, &int3472_tps68470);
+	if (ret)
+		platform_driver_unregister(&int3472_discrete);
+
+	return ret;
+}
+module_init(skl_int3472_init);
+
+static void skl_int3472_exit(void)
+{
+	platform_driver_unregister(&int3472_discrete);
+	i2c_del_driver(&int3472_tps68470);
+}
+module_exit(skl_int3472_exit);
+
+MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Device Driver");
+MODULE_AUTHOR("Daniel Scally <djrscally@gmail.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
new file mode 100644
index 000000000000..6fdf78584219
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Author: Dan Scally <djrscally@gmail.com> */
+
+#ifndef _INTEL_SKL_INT3472_H
+#define _INTEL_SKL_INT3472_H
+
+#include <linux/clk-provider.h>
+#include <linux/gpio/machine.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/types.h>
+
+/* FIXME drop this once the I2C_DEV_NAME_FORMAT macro has been added to include/linux/i2c.h */
+#ifndef I2C_DEV_NAME_FORMAT
+#define I2C_DEV_NAME_FORMAT					"i2c-%s"
+#endif
+
+/* PMIC GPIO Types */
+#define INT3472_GPIO_TYPE_RESET					0x00
+#define INT3472_GPIO_TYPE_POWERDOWN				0x01
+#define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
+#define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
+#define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
+
+#define INT3472_PDEV_MAX_NAME_LEN				23
+#define INT3472_MAX_SENSOR_GPIOS				3
+
+#define GPIO_REGULATOR_NAME_LENGTH				21
+#define GPIO_REGULATOR_SUPPLY_NAME_LENGTH			9
+
+#define CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET			86
+
+#define INT3472_REGULATOR(_name, _supply, _ops)			\
+	(const struct regulator_desc) {				\
+		.name = _name,					\
+		.supply_name = _supply,				\
+		.type = REGULATOR_VOLTAGE,			\
+		.ops = _ops,					\
+		.owner = THIS_MODULE,				\
+	}
+
+#define to_int3472_clk(hw)					\
+	container_of(hw, struct int3472_gpio_clock, clk_hw)
+
+#define to_int3472_device(clk)					\
+	container_of(clk, struct int3472_discrete_device, clock)
+
+struct acpi_device;
+struct i2c_client;
+struct platform_device;
+
+struct int3472_cldb {
+	u8 version;
+	/*
+	 * control logic type
+	 * 0: UNKNOWN
+	 * 1: DISCRETE(CRD-D)
+	 * 2: PMIC TPS68470
+	 * 3: PMIC uP6641
+	 */
+	u8 control_logic_type;
+	u8 control_logic_id;
+	u8 sensor_card_sku;
+	u8 reserved[28];
+};
+
+struct int3472_gpio_function_remap {
+	const char *documented;
+	const char *actual;
+};
+
+struct int3472_sensor_config {
+	const char *sensor_module_name;
+	struct regulator_consumer_supply supply_map;
+	const struct int3472_gpio_function_remap *function_maps;
+};
+
+struct int3472_discrete_device {
+	struct acpi_device *adev;
+	struct device *dev;
+	struct acpi_device *sensor;
+	const char *sensor_name;
+
+	const struct int3472_sensor_config *sensor_config;
+
+	struct int3472_gpio_regulator {
+		char regulator_name[GPIO_REGULATOR_NAME_LENGTH];
+		char supply_name[GPIO_REGULATOR_SUPPLY_NAME_LENGTH];
+		struct gpio_desc *gpio;
+		struct regulator_dev *rdev;
+		struct regulator_desc rdesc;
+	} regulator;
+
+	struct int3472_gpio_clock {
+		struct clk *clk;
+		struct clk_hw clk_hw;
+		struct clk_lookup *cl;
+		struct gpio_desc *ena_gpio;
+		struct gpio_desc *led_gpio;
+		u32 frequency;
+	} clock;
+
+	unsigned int ngpios; /* how many GPIOs have we seen */
+	unsigned int n_sensor_gpios; /* how many have we mapped to sensor */
+	struct gpiod_lookup_table gpios;
+};
+
+int skl_int3472_discrete_probe(struct platform_device *pdev);
+int skl_int3472_discrete_remove(struct platform_device *pdev);
+int skl_int3472_tps68470_probe(struct i2c_client *client);
+union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev,
+					       char *id);
+int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb);
+int skl_int3472_register_clock(struct int3472_discrete_device *int3472);
+int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
+				   struct acpi_resource *ares);
+
+#endif
diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
new file mode 100644
index 000000000000..8c18dbff1c43
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
@@ -0,0 +1,417 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Author: Dan Scally <djrscally@gmail.com> */
+
+#include <linux/acpi.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/uuid.h>
+
+#include "intel_skl_int3472_common.h"
+
+/*
+ * 79234640-9e10-4fea-a5c1-b5aa8b19756f
+ * This _DSM GUID returns information about the GPIO lines mapped to a
+ * discrete INT3472 device. Function number 1 returns a count of the GPIO
+ * lines that are mapped. Subsequent functions return 32 bit ints encoding
+ * information about the GPIO line, including its purpose.
+ */
+static const guid_t int3472_gpio_guid =
+	GUID_INIT(0x79234640, 0x9e10, 0x4fea,
+		  0xa5, 0xc1, 0xb5, 0xaa, 0x8b, 0x19, 0x75, 0x6f);
+
+/*
+ * 822ace8f-2814-4174-a56b-5f029fe079ee
+ * This _DSM GUID returns a string from the sensor device, which acts as a
+ * module identifier.
+ */
+static const guid_t cio2_sensor_module_guid =
+	GUID_INIT(0x822ace8f, 0x2814, 0x4174,
+		  0xa5, 0x6b, 0x5f, 0x02, 0x9f, 0xe0, 0x79, 0xee);
+
+/*
+ * Here follows platform specific mapping information that we can pass to
+ * the functions mapping resources to the sensors. Where the sensors have
+ * a power enable pin defined in DSDT we need to provide a supply name so
+ * the sensor drivers can find the regulator. The device name will be derived
+ * from the sensor's ACPI device within the code. Optionally, we can provide a
+ * NULL terminated array of function name mappings to deal with any platform
+ * specific deviations from the documented behaviour of GPIOs.
+ *
+ * Map a GPIO function name to NULL to prevent the driver from mapping that
+ * GPIO at all.
+ */
+
+static const struct int3472_gpio_function_remap ov2680_gpio_function_remaps[] = {
+	{ "reset", NULL },
+	{ "powerdown", "reset" },
+	{ }
+};
+
+static const struct int3472_sensor_config int3472_sensor_configs[] = {
+	/* Lenovo Miix 510-12ISK - OV2680, Front */
+	{ "GNDF140809R", { 0 }, ov2680_gpio_function_remaps },
+	/* Lenovo Miix 510-12ISK - OV5648, Rear */
+	{ "GEFF150023R", REGULATOR_SUPPLY("avdd", NULL), NULL },
+	/* Surface Go 1&2 - OV5693, Front */
+	{ "YHCU", REGULATOR_SUPPLY("avdd", NULL), NULL },
+};
+
+static const struct int3472_sensor_config *
+skl_int3472_get_sensor_module_config(struct int3472_discrete_device *int3472)
+{
+	union acpi_object *obj;
+	unsigned int i;
+
+	obj = acpi_evaluate_dsm_typed(int3472->sensor->handle,
+				      &cio2_sensor_module_guid, 0x00,
+				      0x01, NULL, ACPI_TYPE_STRING);
+
+	if (!obj) {
+		dev_err(int3472->dev,
+			"Failed to get sensor module string from _DSM\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	if (obj->string.type != ACPI_TYPE_STRING) {
+		dev_err(int3472->dev,
+			"Sensor _DSM returned a non-string value\n");
+
+		ACPI_FREE(obj);
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(int3472_sensor_configs); i++) {
+		if (!strcmp(int3472_sensor_configs[i].sensor_module_name,
+			    obj->string.pointer))
+			break;
+	}
+
+	ACPI_FREE(obj);
+
+	if (i >= ARRAY_SIZE(int3472_sensor_configs))
+		return ERR_PTR(-EINVAL);
+
+	return &int3472_sensor_configs[i];
+}
+
+static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int3472,
+					  struct acpi_resource *ares,
+					  const char *func, u32 polarity)
+{
+	char *path = ares->data.gpio.resource_source.string_ptr;
+	const struct int3472_sensor_config *sensor_config;
+	struct gpiod_lookup *table_entry;
+	struct acpi_device *adev;
+	acpi_handle handle;
+	acpi_status status;
+	int ret;
+
+	if (int3472->n_sensor_gpios >= INT3472_MAX_SENSOR_GPIOS) {
+		dev_warn(int3472->dev, "Too many GPIOs mapped\n");
+		return -EINVAL;
+	}
+
+	sensor_config = int3472->sensor_config;
+	if (!IS_ERR(sensor_config) && sensor_config->function_maps) {
+		const struct int3472_gpio_function_remap *remap;
+
+		for (remap = sensor_config->function_maps; remap->documented; remap++) {
+			if (!strcmp(func, remap->documented)) {
+				func = remap->actual;
+				break;
+			}
+		}
+	}
+
+	/* Functions mapped to NULL should not be mapped to the sensor */
+	if (!func)
+		return 0;
+
+	status = acpi_get_handle(NULL, path, &handle);
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+
+	ret = acpi_bus_get_device(handle, &adev);
+	if (ret)
+		return -ENODEV;
+
+	table_entry = &int3472->gpios.table[int3472->n_sensor_gpios];
+	table_entry->key = acpi_dev_name(adev);
+	table_entry->chip_hwnum = ares->data.gpio.pin_table[0];
+	table_entry->con_id = func;
+	table_entry->idx = 0;
+	table_entry->flags = polarity;
+
+	int3472->n_sensor_gpios++;
+
+	return 0;
+}
+
+static int skl_int3472_map_gpio_to_clk(struct int3472_discrete_device *int3472,
+				       struct acpi_resource *ares, u8 type)
+{
+	char *path = ares->data.gpio.resource_source.string_ptr;
+	struct gpio_desc *gpio;
+
+	switch (type) {
+	case INT3472_GPIO_TYPE_CLK_ENABLE:
+		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
+						  "int3472,clk-enable");
+		if (IS_ERR(gpio))
+			return (PTR_ERR(gpio));
+
+		int3472->clock.ena_gpio = gpio;
+		break;
+	case INT3472_GPIO_TYPE_PRIVACY_LED:
+		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
+						  "int3472,privacy-led");
+		if (IS_ERR(gpio))
+			return (PTR_ERR(gpio));
+
+		int3472->clock.led_gpio = gpio;
+		break;
+	default:
+		dev_err(int3472->dev, "Invalid GPIO type 0x%02x for clock\n", type);
+		break;
+	}
+
+	return 0;
+}
+
+/**
+ * skl_int3472_handle_gpio_resources: Map PMIC resources to consuming sensor
+ * @ares: A pointer to a &struct acpi_resource
+ * @data: A pointer to a &struct int3472_discrete_device
+ *
+ * This function handles GPIO resources that are against an INT3472
+ * ACPI device, by checking the value of the corresponding _DSM entry.
+ * This will return a 32bit int, where the lowest byte represents the
+ * function of the GPIO pin:
+ *
+ * 0x00 Reset
+ * 0x01 Power down
+ * 0x0b Power enable
+ * 0x0c Clock enable
+ * 0x0d Privacy LED
+ *
+ * There are some known platform specific quirks where that does not quite
+ * hold up; for example where a pin with type 0x01 (Power down) is mapped to
+ * a sensor pin that performs a reset function or entries in _CRS and _DSM that
+ * do not actually correspond to a physical connection. These will be handled
+ * by the mapping sub-functions.
+ *
+ * GPIOs will either be mapped directly to the sensor device or else used
+ * to create clocks and regulators via the usual frameworks.
+ *
+ * Return:
+ * * 1		- To continue the loop
+ * * 0		- When all resources found are handled properly.
+ * * -EINVAL	- If the resource is not a GPIO IO resource
+ * * -ENODEV	- If the resource has no corresponding _DSM entry
+ * * -Other	- Errors propagated from one of the sub-functions.
+ */
+static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
+					     void *data)
+{
+	struct int3472_discrete_device *int3472 = data;
+	struct acpi_resource_gpio *agpio;
+	union acpi_object *obj;
+	const char *err_msg;
+	int ret;
+	u8 type;
+
+	if (!acpi_gpio_get_io_resource(ares, &agpio))
+		return 1;
+
+	/*
+	 * ngpios + 2 because the index of this _DSM function is 1-based and
+	 * the first function is just a count.
+	 */
+	obj = acpi_evaluate_dsm_typed(int3472->adev->handle,
+				      &int3472_gpio_guid, 0x00,
+				      int3472->ngpios + 2,
+				      NULL, ACPI_TYPE_INTEGER);
+
+	if (!obj) {
+		dev_warn(int3472->dev, "No _DSM entry for GPIO pin %u\n",
+			 ares->data.gpio.pin_table[0]);
+		return 1;
+	}
+
+	type = obj->integer.value & 0xff;
+
+	switch (type) {
+	case INT3472_GPIO_TYPE_RESET:
+		ret = skl_int3472_map_gpio_to_sensor(int3472, ares, "reset",
+						     GPIO_ACTIVE_LOW);
+		if (ret)
+			err_msg = "Failed to map reset pin to sensor\n";
+
+		break;
+	case INT3472_GPIO_TYPE_POWERDOWN:
+		ret = skl_int3472_map_gpio_to_sensor(int3472, ares,
+						     "powerdown",
+						     GPIO_ACTIVE_LOW);
+		if (ret)
+			err_msg = "Failed to map powerdown pin to sensor\n";
+
+		break;
+	case INT3472_GPIO_TYPE_CLK_ENABLE:
+	case INT3472_GPIO_TYPE_PRIVACY_LED:
+		ret = skl_int3472_map_gpio_to_clk(int3472, ares, type);
+		if (ret)
+			err_msg = "Failed to map GPIO to clock\n";
+
+		break;
+	case INT3472_GPIO_TYPE_POWER_ENABLE:
+		ret = skl_int3472_register_regulator(int3472, ares);
+		if (ret)
+			err_msg = "Failed to map regulator to sensor\n";
+
+		break;
+	default:
+		dev_warn(int3472->dev,
+			 "GPIO type 0x%02x unknown; the sensor may not work\n",
+			 type);
+		ret = 1;
+		break;
+	}
+
+	int3472->ngpios++;
+	ACPI_FREE(obj);
+
+	if (ret)
+		return dev_err_probe(int3472->dev, ret, err_msg);
+
+	return 0;
+}
+
+static int skl_int3472_parse_crs(struct int3472_discrete_device *int3472)
+{
+	LIST_HEAD(resource_list);
+	int ret;
+
+	/*
+	 * No error check, because not having a sensor config is not necessarily
+	 * a failure mode.
+	 */
+	int3472->sensor_config = skl_int3472_get_sensor_module_config(int3472);
+
+	ret = acpi_dev_get_resources(int3472->adev, &resource_list,
+				     skl_int3472_handle_gpio_resources,
+				     int3472);
+	if (ret)
+		goto out_free_res_list;
+
+	/*
+	 * If we find no clock enable GPIO pin then the privacy LED won't work.
+	 * We've never seen that situation, but it's possible. Warn the user so
+	 * it's clear what's happened.
+	 */
+	if (int3472->clock.ena_gpio) {
+		ret = skl_int3472_register_clock(int3472);
+		if (ret)
+			goto out_free_res_list;
+	} else {
+		if (int3472->clock.led_gpio)
+			dev_warn(int3472->dev,
+				 "No clk GPIO. The privacy LED won't work\n");
+	}
+
+	int3472->gpios.dev_id = int3472->sensor_name;
+	gpiod_add_lookup_table(&int3472->gpios);
+
+out_free_res_list:
+	acpi_dev_free_resource_list(&resource_list);
+
+	return ret;
+}
+
+int skl_int3472_discrete_probe(struct platform_device *pdev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
+	struct int3472_discrete_device *int3472;
+	struct int3472_cldb cldb;
+	int ret;
+
+	ret = skl_int3472_fill_cldb(adev, &cldb);
+	if (ret) {
+		dev_err(&pdev->dev, "Couldn't fill CLDB structure\n");
+		return ret;
+	}
+
+	if (cldb.control_logic_type != 1) {
+		dev_err(&pdev->dev, "Unsupported control logic type %u\n",
+			cldb.control_logic_type);
+		return -EINVAL;
+	}
+
+	/* Max num GPIOs we've seen plus a terminator */
+	int3472 = devm_kzalloc(&pdev->dev, struct_size(int3472, gpios.table,
+			       INT3472_MAX_SENSOR_GPIOS + 1), GFP_KERNEL);
+	if (!int3472)
+		return -ENOMEM;
+
+	int3472->adev = adev;
+	int3472->dev = &pdev->dev;
+	platform_set_drvdata(pdev, int3472);
+
+	int3472->sensor = acpi_dev_get_first_consumer_dev(adev);
+	if (!int3472->sensor) {
+		dev_err(&pdev->dev, "INT3472 seems to have no dependents.\n");
+		return -ENODEV;
+	}
+
+	int3472->sensor_name = devm_kasprintf(int3472->dev, GFP_KERNEL,
+					      I2C_DEV_NAME_FORMAT,
+					      acpi_dev_name(int3472->sensor));
+	if (!int3472->sensor_name) {
+		ret = -ENOMEM;
+		goto err_put_sensor;
+	}
+
+	/*
+	 * Initialising this list means we can call gpiod_remove_lookup_table()
+	 * in failure paths without issue.
+	 */
+	INIT_LIST_HEAD(&int3472->gpios.list);
+
+	ret = skl_int3472_parse_crs(int3472);
+	if (ret) {
+		skl_int3472_discrete_remove(pdev);
+		return ret;
+	}
+
+	return 0;
+
+err_put_sensor:
+	acpi_dev_put(int3472->sensor);
+
+	return ret;
+}
+
+int skl_int3472_discrete_remove(struct platform_device *pdev)
+{
+	struct int3472_discrete_device *int3472 = platform_get_drvdata(pdev);
+
+	gpiod_remove_lookup_table(&int3472->gpios);
+	regulator_unregister(int3472->regulator.rdev);
+	clk_unregister(int3472->clock.clk);
+
+	if (int3472->clock.cl)
+		clkdev_drop(int3472->clock.cl);
+
+	gpiod_put(int3472->regulator.gpio);
+	gpiod_put(int3472->clock.ena_gpio);
+	gpiod_put(int3472->clock.led_gpio);
+
+	return 0;
+}
diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
new file mode 100644
index 000000000000..6306ba24e0bc
--- /dev/null
+++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Author: Dan Scally <djrscally@gmail.com> */
+
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/tps68470.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "intel_skl_int3472_common.h"
+
+#define DESIGNED_FOR_CHROMEOS		1
+#define DESIGNED_FOR_WINDOWS		2
+
+static const struct mfd_cell tps68470_cros[] = {
+	{ .name = "tps68470-gpio" },
+	{ .name = "tps68470_pmic_opregion" },
+};
+
+static const struct mfd_cell tps68470_win[] = {
+	{ .name = "tps68470-gpio" },
+	{ .name = "tps68470-clk" },
+	{ .name = "tps68470-regulator" },
+};
+
+static const struct regmap_config tps68470_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = TPS68470_REG_MAX,
+};
+
+static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
+{
+	unsigned int version;
+	int ret;
+
+	/* Force software reset */
+	ret = regmap_write(regmap, TPS68470_REG_RESET, TPS68470_REG_RESET_MASK);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
+	if (ret) {
+		dev_err(dev, "Failed to read revision register: %d\n", ret);
+		return ret;
+	}
+
+	dev_info(dev, "TPS68470 REVID: 0x%02x\n", version);
+
+	return 0;
+}
+
+/** skl_int3472_tps68470_calc_type: Check what platform a device is designed for
+ * @adev: A pointer to a &struct acpi_device
+ *
+ * Check CLDB buffer against the PMIC's adev. If present, then we check
+ * the value of control_logic_type field and follow one of the
+ * following scenarios:
+ *
+ *	1. No CLDB - likely ACPI tables designed for ChromeOS. We
+ *	create platform devices for the GPIOs and OpRegion drivers.
+ *
+ *	2. CLDB, with control_logic_type = 2 - probably ACPI tables
+ *	made for Windows 2-in-1 platforms. Register pdevs for GPIO,
+ *	Clock and Regulator drivers to bind to.
+ *
+ *	3. Any other value in control_logic_type, we should never have
+ *	gotten to this point; fail probe and return.
+ *
+ * Return:
+ * * 1		Device intended for ChromeOS
+ * * 2		Device intended for Windows
+ * * -EINVAL	Where @adev has an object named CLDB but it does not conform to
+ *		our expectations
+ */
+int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
+{
+	struct int3472_cldb cldb = { 0 };
+	int ret;
+
+	/*
+	 * A CLDB buffer that exists, but which does not match our expectations
+	 * should trigger an error so we don't blindly continue.
+	 */
+	ret = skl_int3472_fill_cldb(adev, &cldb);
+	if (ret && ret != -ENODEV)
+		return ret;
+
+	if (ret)
+		return DESIGNED_FOR_CHROMEOS;
+
+	if (cldb.control_logic_type != 2)
+		return -EINVAL;
+
+	return DESIGNED_FOR_WINDOWS;
+}
+
+int skl_int3472_tps68470_probe(struct i2c_client *client)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
+	struct regmap *regmap;
+	int device_type;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	i2c_set_clientdata(client, regmap);
+
+	ret = tps68470_chip_init(&client->dev, regmap);
+	if (ret < 0) {
+		dev_err(&client->dev, "TPS68470 init error %d\n", ret);
+		return ret;
+	}
+
+	device_type = skl_int3472_tps68470_calc_type(adev);
+	switch (device_type) {
+	case DESIGNED_FOR_WINDOWS:
+		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
+					   tps68470_win, ARRAY_SIZE(tps68470_win),
+					   NULL, 0, NULL);
+		break;
+	case DESIGNED_FOR_CHROMEOS:
+		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
+					   tps68470_cros, ARRAY_SIZE(tps68470_cros),
+					   NULL, 0, NULL);
+		break;
+	default:
+		dev_err(&client->dev, "Failed to add MFD devices\n");
+		return device_type;
+	}
+
+	return ret;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 6/6] mfd: tps68470: Remove tps68470 MFD driver
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
                   ` (4 preceding siblings ...)
  2021-06-03 22:40 ` [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver Daniel Scally
@ 2021-06-03 22:40 ` Daniel Scally
  2021-06-17 11:14   ` Hans de Goede
  2021-06-08  9:00 ` [PATCH v5 0/6] Introduce intel_skl_int3472 module Hans de Goede
  6 siblings, 1 reply; 24+ messages in thread
From: Daniel Scally @ 2021-06-03 22:40 UTC (permalink / raw)
  To: Rafael J . Wysocki, Andy Shevchenko, Daniel Scally, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c, platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham, Andy Shevchenko

This driver only covered one scenario in which ACPI devices with _HID
INT3472 are found, and its functionality has been taken over by the
intel-skl-int3472 module, so remove it.

Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since v4:
	- None

 drivers/acpi/pmic/Kconfig |  2 +-
 drivers/gpio/Kconfig      |  2 +-
 drivers/mfd/Kconfig       | 18 --------
 drivers/mfd/Makefile      |  1 -
 drivers/mfd/tps68470.c    | 97 ---------------------------------------
 5 files changed, 2 insertions(+), 118 deletions(-)
 delete mode 100644 drivers/mfd/tps68470.c

diff --git a/drivers/acpi/pmic/Kconfig b/drivers/acpi/pmic/Kconfig
index 56bbcb2ce61b..f84b8f6038dc 100644
--- a/drivers/acpi/pmic/Kconfig
+++ b/drivers/acpi/pmic/Kconfig
@@ -52,7 +52,7 @@ endif	# PMIC_OPREGION
 
 config TPS68470_PMIC_OPREGION
 	bool "ACPI operation region support for TPS68470 PMIC"
-	depends on MFD_TPS68470
+	depends on INTEL_SKL_INT3472
 	help
 	  This config adds ACPI operation region support for TI TPS68470 PMIC.
 	  TPS68470 device is an advanced power management unit that powers
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1dd0ec6727fd..10228abeee56 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1367,7 +1367,7 @@ config GPIO_TPS65912
 
 config GPIO_TPS68470
 	bool "TPS68470 GPIO"
-	depends on MFD_TPS68470
+	depends on INTEL_SKL_INT3472
 	help
 	  Select this option to enable GPIO driver for the TPS68470
 	  chip family.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 5c7f2b100191..99c4e1a80ae0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1499,24 +1499,6 @@ config MFD_TPS65217
 	  This driver can also be built as a module.  If so, the module
 	  will be called tps65217.
 
-config MFD_TPS68470
-	bool "TI TPS68470 Power Management / LED chips"
-	depends on ACPI && PCI && I2C=y
-	depends on I2C_DESIGNWARE_PLATFORM=y
-	select MFD_CORE
-	select REGMAP_I2C
-	help
-	  If you say yes here you get support for the TPS68470 series of
-	  Power Management / LED chips.
-
-	  These include voltage regulators, LEDs and other features
-	  that are often used in portable devices.
-
-	  This option is a bool as it provides an ACPI operation
-	  region, which must be available before any of the devices
-	  using this are probed. This option also configures the
-	  designware-i2c driver to be built-in, for the same reason.
-
 config MFD_TI_LP873X
 	tristate "TI LP873X Power Management IC"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 4f6d2b8a5f76..8b322d89a0c5 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -105,7 +105,6 @@ obj-$(CONFIG_MFD_TPS65910)	+= tps65910.o
 obj-$(CONFIG_MFD_TPS65912)	+= tps65912-core.o
 obj-$(CONFIG_MFD_TPS65912_I2C)	+= tps65912-i2c.o
 obj-$(CONFIG_MFD_TPS65912_SPI)  += tps65912-spi.o
-obj-$(CONFIG_MFD_TPS68470)	+= tps68470.o
 obj-$(CONFIG_MFD_TPS80031)	+= tps80031.o
 obj-$(CONFIG_MENELAUS)		+= menelaus.o
 
diff --git a/drivers/mfd/tps68470.c b/drivers/mfd/tps68470.c
deleted file mode 100644
index 4a4df4ffd18c..000000000000
--- a/drivers/mfd/tps68470.c
+++ /dev/null
@@ -1,97 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * TPS68470 chip Parent driver
- *
- * Copyright (C) 2017 Intel Corporation
- *
- * Authors:
- *	Rajmohan Mani <rajmohan.mani@intel.com>
- *	Tianshu Qiu <tian.shu.qiu@intel.com>
- *	Jian Xu Zheng <jian.xu.zheng@intel.com>
- *	Yuning Pu <yuning.pu@intel.com>
- */
-
-#include <linux/acpi.h>
-#include <linux/delay.h>
-#include <linux/i2c.h>
-#include <linux/init.h>
-#include <linux/mfd/core.h>
-#include <linux/mfd/tps68470.h>
-#include <linux/regmap.h>
-
-static const struct mfd_cell tps68470s[] = {
-	{ .name = "tps68470-gpio" },
-	{ .name = "tps68470_pmic_opregion" },
-};
-
-static const struct regmap_config tps68470_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 8,
-	.max_register = TPS68470_REG_MAX,
-};
-
-static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
-{
-	unsigned int version;
-	int ret;
-
-	/* Force software reset */
-	ret = regmap_write(regmap, TPS68470_REG_RESET, TPS68470_REG_RESET_MASK);
-	if (ret)
-		return ret;
-
-	ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
-	if (ret) {
-		dev_err(dev, "Failed to read revision register: %d\n", ret);
-		return ret;
-	}
-
-	dev_info(dev, "TPS68470 REVID: 0x%x\n", version);
-
-	return 0;
-}
-
-static int tps68470_probe(struct i2c_client *client)
-{
-	struct device *dev = &client->dev;
-	struct regmap *regmap;
-	int ret;
-
-	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
-	if (IS_ERR(regmap)) {
-		dev_err(dev, "devm_regmap_init_i2c Error %ld\n",
-			PTR_ERR(regmap));
-		return PTR_ERR(regmap);
-	}
-
-	i2c_set_clientdata(client, regmap);
-
-	ret = tps68470_chip_init(dev, regmap);
-	if (ret < 0) {
-		dev_err(dev, "TPS68470 Init Error %d\n", ret);
-		return ret;
-	}
-
-	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, tps68470s,
-			      ARRAY_SIZE(tps68470s), NULL, 0, NULL);
-	if (ret < 0) {
-		dev_err(dev, "devm_mfd_add_devices failed: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
-}
-
-static const struct acpi_device_id tps68470_acpi_ids[] = {
-	{"INT3472"},
-	{},
-};
-
-static struct i2c_driver tps68470_driver = {
-	.driver = {
-		   .name = "tps68470",
-		   .acpi_match_table = tps68470_acpi_ids,
-	},
-	.probe_new = tps68470_probe,
-};
-builtin_i2c_driver(tps68470_driver);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-03 22:40 ` [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver Daniel Scally
@ 2021-06-04  1:31   ` kernel test robot
  2021-06-05 23:07     ` Daniel Scally
  2021-06-04  6:49   ` kernel test robot
  2021-06-16 17:58   ` Hans de Goede
  2 siblings, 1 reply; 24+ messages in thread
From: kernel test robot @ 2021-06-04  1:31 UTC (permalink / raw)
  To: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c
  Cc: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3486 bytes --]

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pm/linux-next]
[also build test WARNING on lee-mfd/for-mfd-next linus/master v5.13-rc4 next-20210603]
[cannot apply to gpio/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/Introduce-intel_skl_int3472-module/20210604-064345
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/3edcad8c200f211063a35d125e9fd350a2efeb40
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Scally/Introduce-intel_skl_int3472-module/20210604-064345
        git checkout 3edcad8c200f211063a35d125e9fd350a2efeb40
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c:76:5: warning: no previous prototype for 'skl_int3472_tps68470_calc_type' [-Wmissing-prototypes]
      76 | int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/skl_int3472_tps68470_calc_type +76 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c

    52	
    53	/** skl_int3472_tps68470_calc_type: Check what platform a device is designed for
    54	 * @adev: A pointer to a &struct acpi_device
    55	 *
    56	 * Check CLDB buffer against the PMIC's adev. If present, then we check
    57	 * the value of control_logic_type field and follow one of the
    58	 * following scenarios:
    59	 *
    60	 *	1. No CLDB - likely ACPI tables designed for ChromeOS. We
    61	 *	create platform devices for the GPIOs and OpRegion drivers.
    62	 *
    63	 *	2. CLDB, with control_logic_type = 2 - probably ACPI tables
    64	 *	made for Windows 2-in-1 platforms. Register pdevs for GPIO,
    65	 *	Clock and Regulator drivers to bind to.
    66	 *
    67	 *	3. Any other value in control_logic_type, we should never have
    68	 *	gotten to this point; fail probe and return.
    69	 *
    70	 * Return:
    71	 * * 1		Device intended for ChromeOS
    72	 * * 2		Device intended for Windows
    73	 * * -EINVAL	Where @adev has an object named CLDB but it does not conform to
    74	 *		our expectations
    75	 */
  > 76	int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
    77	{
    78		struct int3472_cldb cldb = { 0 };
    79		int ret;
    80	
    81		/*
    82		 * A CLDB buffer that exists, but which does not match our expectations
    83		 * should trigger an error so we don't blindly continue.
    84		 */
    85		ret = skl_int3472_fill_cldb(adev, &cldb);
    86		if (ret && ret != -ENODEV)
    87			return ret;
    88	
    89		if (ret)
    90			return DESIGNED_FOR_CHROMEOS;
    91	
    92		if (cldb.control_logic_type != 2)
    93			return -EINVAL;
    94	
    95		return DESIGNED_FOR_WINDOWS;
    96	}
    97	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65661 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-03 22:40 ` [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver Daniel Scally
  2021-06-04  1:31   ` kernel test robot
@ 2021-06-04  6:49   ` kernel test robot
  2021-06-16 17:58   ` Hans de Goede
  2 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2021-06-04  6:49 UTC (permalink / raw)
  To: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Hans de Goede, Maximilian Luz, linux-kernel, linux-acpi,
	linux-gpio, linux-i2c
  Cc: kbuild-all, clang-built-linux

[-- Attachment #1: Type: text/plain, Size: 4113 bytes --]

Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pm/linux-next]
[also build test WARNING on lee-mfd/for-mfd-next linus/master v5.13-rc4 next-20210603]
[cannot apply to gpio/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/Introduce-intel_skl_int3472-module/20210604-064345
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-r012-20210604 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 5c0d1b2f902aa6a9cf47cc7e42c5b83bb2217cf9)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/3edcad8c200f211063a35d125e9fd350a2efeb40
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Scally/Introduce-intel_skl_int3472-module/20210604-064345
        git checkout 3edcad8c200f211063a35d125e9fd350a2efeb40
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c:76:5: warning: no previous prototype for function 'skl_int3472_tps68470_calc_type' [-Wmissing-prototypes]
   int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
       ^
   drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c:76:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
   ^
   static 
   1 warning generated.


vim +/skl_int3472_tps68470_calc_type +76 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c

    52	
    53	/** skl_int3472_tps68470_calc_type: Check what platform a device is designed for
    54	 * @adev: A pointer to a &struct acpi_device
    55	 *
    56	 * Check CLDB buffer against the PMIC's adev. If present, then we check
    57	 * the value of control_logic_type field and follow one of the
    58	 * following scenarios:
    59	 *
    60	 *	1. No CLDB - likely ACPI tables designed for ChromeOS. We
    61	 *	create platform devices for the GPIOs and OpRegion drivers.
    62	 *
    63	 *	2. CLDB, with control_logic_type = 2 - probably ACPI tables
    64	 *	made for Windows 2-in-1 platforms. Register pdevs for GPIO,
    65	 *	Clock and Regulator drivers to bind to.
    66	 *
    67	 *	3. Any other value in control_logic_type, we should never have
    68	 *	gotten to this point; fail probe and return.
    69	 *
    70	 * Return:
    71	 * * 1		Device intended for ChromeOS
    72	 * * 2		Device intended for Windows
    73	 * * -EINVAL	Where @adev has an object named CLDB but it does not conform to
    74	 *		our expectations
    75	 */
  > 76	int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
    77	{
    78		struct int3472_cldb cldb = { 0 };
    79		int ret;
    80	
    81		/*
    82		 * A CLDB buffer that exists, but which does not match our expectations
    83		 * should trigger an error so we don't blindly continue.
    84		 */
    85		ret = skl_int3472_fill_cldb(adev, &cldb);
    86		if (ret && ret != -ENODEV)
    87			return ret;
    88	
    89		if (ret)
    90			return DESIGNED_FOR_CHROMEOS;
    91	
    92		if (cldb.control_logic_type != 2)
    93			return -EINVAL;
    94	
    95		return DESIGNED_FOR_WINDOWS;
    96	}
    97	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35052 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod()
  2021-06-03 22:40 ` [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod() Daniel Scally
@ 2021-06-04 12:59   ` Andy Shevchenko
  2021-06-04 14:09     ` Daniel Scally
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-06-04 12:59 UTC (permalink / raw)
  To: Daniel Scally
  Cc: Rafael J . Wysocki, Lee Jones, Hans de Goede, Maximilian Luz,
	linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, laurent.pinchart, kieran.bingham

On Thu, Jun 03, 2021 at 11:40:04PM +0100, Daniel Scally wrote:
> We need to be able to translate GPIO resources in an ACPI device's _CRS
> into GPIO descriptor array. Those are represented in _CRS as a pathname
> to a GPIO device plus the pin's index number: the acpi_get_gpiod()
> function is perfect for that purpose.
> 
> As it's currently only used internally within the GPIO layer, provide and
> export a wrapper function that additionally holds a reference to the GPIO
> device.

The subject is wrong, it should be "Introduce acpi_get_and_request_gpiod()
helper" or so. I can fix when applying.

Btw, do I understand correctly that I may push GPIO ACPI patches independently
(of the ACPI changes)?

> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes since v4:
> 	- None
> 
>  drivers/gpio/gpiolib-acpi.c   | 28 ++++++++++++++++++++++++++++
>  include/linux/gpio/consumer.h |  2 ++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 5b4111e4be3f..684ddb35d83b 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -128,6 +128,34 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
>  	return gpiochip_get_desc(chip, pin);
>  }
>  
> +/**
> + * acpi_get_and_request_gpiod() - Translate ACPI GPIO pin to GPIO descriptor
> + *                               and hold a refcount to the GPIO device.
> + * @path:      ACPI GPIO controller full path name, (e.g. "\\_SB.GPO1")
> + * @pin:       ACPI GPIO pin number (0-based, controller-relative)
> + * @label:     Label to pass to gpiod_request()
> + *
> + * This function is a simple pass-through to acpi_get_gpiod(), except that
> + * as it is intended for use outside of the GPIO layer (in a similar fashion to
> + * gpiod_get_index() for example) it also holds a reference to the GPIO device.
> + */
> +struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label)
> +{
> +	struct gpio_desc *gpio;
> +	int ret;
> +
> +	gpio = acpi_get_gpiod(path, pin);
> +	if (IS_ERR(gpio))
> +		return gpio;
> +
> +	ret = gpiod_request(gpio, label);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return gpio;
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_and_request_gpiod);
> +
>  static irqreturn_t acpi_gpio_irq_handler(int irq, void *data)
>  {
>  	struct acpi_gpio_event *event = data;
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index c73b25bc9213..566feb56601f 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -692,6 +692,8 @@ int devm_acpi_dev_add_driver_gpios(struct device *dev,
>  				   const struct acpi_gpio_mapping *gpios);
>  void devm_acpi_dev_remove_driver_gpios(struct device *dev);
>  
> +struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label);
> +
>  #else  /* CONFIG_GPIOLIB && CONFIG_ACPI */
>  
>  struct acpi_device;
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource()
  2021-06-03 22:40 ` [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource() Daniel Scally
@ 2021-06-04 13:01   ` Andy Shevchenko
  2021-06-04 14:09     ` Daniel Scally
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-06-04 13:01 UTC (permalink / raw)
  To: Daniel Scally
  Cc: Rafael J . Wysocki, Lee Jones, Hans de Goede, Maximilian Luz,
	linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, laurent.pinchart, kieran.bingham

On Thu, Jun 03, 2021 at 11:40:05PM +0100, Daniel Scally wrote:
> Add a function to verify that a given acpi_resource represents an IO
> type GPIO resource, and return it if so.

I would rephrase this to something like:

"Add a function to verify that a given ACPI resource represents a GpioIo() type
of resource, and return it if so."

I can amend when applying to my branch.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod()
  2021-06-04 12:59   ` Andy Shevchenko
@ 2021-06-04 14:09     ` Daniel Scally
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-04 14:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J . Wysocki, Lee Jones, Hans de Goede, Maximilian Luz,
	linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, laurent.pinchart, kieran.bingham

Hi Andy

On 04/06/2021 13:59, Andy Shevchenko wrote:
> On Thu, Jun 03, 2021 at 11:40:04PM +0100, Daniel Scally wrote:
>> We need to be able to translate GPIO resources in an ACPI device's _CRS
>> into GPIO descriptor array. Those are represented in _CRS as a pathname
>> to a GPIO device plus the pin's index number: the acpi_get_gpiod()
>> function is perfect for that purpose.
>>
>> As it's currently only used internally within the GPIO layer, provide and
>> export a wrapper function that additionally holds a reference to the GPIO
>> device.
> The subject is wrong, it should be "Introduce acpi_get_and_request_gpiod()
> helper" or so. I can fix when applying.


Ah! That was sloppy, sorry.

> Btw, do I understand correctly that I may push GPIO ACPI patches independently
> (of the ACPI changes)?


They're independent yes.

>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>> ---
>> Changes since v4:
>> 	- None
>>
>>  drivers/gpio/gpiolib-acpi.c   | 28 ++++++++++++++++++++++++++++
>>  include/linux/gpio/consumer.h |  2 ++
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
>> index 5b4111e4be3f..684ddb35d83b 100644
>> --- a/drivers/gpio/gpiolib-acpi.c
>> +++ b/drivers/gpio/gpiolib-acpi.c
>> @@ -128,6 +128,34 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
>>  	return gpiochip_get_desc(chip, pin);
>>  }
>>  
>> +/**
>> + * acpi_get_and_request_gpiod() - Translate ACPI GPIO pin to GPIO descriptor
>> + *                               and hold a refcount to the GPIO device.
>> + * @path:      ACPI GPIO controller full path name, (e.g. "\\_SB.GPO1")
>> + * @pin:       ACPI GPIO pin number (0-based, controller-relative)
>> + * @label:     Label to pass to gpiod_request()
>> + *
>> + * This function is a simple pass-through to acpi_get_gpiod(), except that
>> + * as it is intended for use outside of the GPIO layer (in a similar fashion to
>> + * gpiod_get_index() for example) it also holds a reference to the GPIO device.
>> + */
>> +struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label)
>> +{
>> +	struct gpio_desc *gpio;
>> +	int ret;
>> +
>> +	gpio = acpi_get_gpiod(path, pin);
>> +	if (IS_ERR(gpio))
>> +		return gpio;
>> +
>> +	ret = gpiod_request(gpio, label);
>> +	if (ret)
>> +		return ERR_PTR(ret);
>> +
>> +	return gpio;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_get_and_request_gpiod);
>> +
>>  static irqreturn_t acpi_gpio_irq_handler(int irq, void *data)
>>  {
>>  	struct acpi_gpio_event *event = data;
>> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
>> index c73b25bc9213..566feb56601f 100644
>> --- a/include/linux/gpio/consumer.h
>> +++ b/include/linux/gpio/consumer.h
>> @@ -692,6 +692,8 @@ int devm_acpi_dev_add_driver_gpios(struct device *dev,
>>  				   const struct acpi_gpio_mapping *gpios);
>>  void devm_acpi_dev_remove_driver_gpios(struct device *dev);
>>  
>> +struct gpio_desc *acpi_get_and_request_gpiod(char *path, int pin, char *label);
>> +
>>  #else  /* CONFIG_GPIOLIB && CONFIG_ACPI */
>>  
>>  struct acpi_device;
>> -- 
>> 2.25.1
>>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource()
  2021-06-04 13:01   ` Andy Shevchenko
@ 2021-06-04 14:09     ` Daniel Scally
  2021-06-04 14:33       ` Andy Shevchenko
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel Scally @ 2021-06-04 14:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J . Wysocki, Lee Jones, Hans de Goede, Maximilian Luz,
	linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, laurent.pinchart, kieran.bingham


On 04/06/2021 14:01, Andy Shevchenko wrote:
> On Thu, Jun 03, 2021 at 11:40:05PM +0100, Daniel Scally wrote:
>> Add a function to verify that a given acpi_resource represents an IO
>> type GPIO resource, and return it if so.
> I would rephrase this to something like:
>
> "Add a function to verify that a given ACPI resource represents a GpioIo() type
> of resource, and return it if so."
>
> I can amend when applying to my branch.
>
That wording is perfectly fine by me

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource()
  2021-06-04 14:09     ` Daniel Scally
@ 2021-06-04 14:33       ` Andy Shevchenko
  2021-06-04 16:54         ` Daniel Scally
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-06-04 14:33 UTC (permalink / raw)
  To: Daniel Scally
  Cc: Rafael J . Wysocki, Lee Jones, Hans de Goede, Maximilian Luz,
	linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, laurent.pinchart, kieran.bingham

On Fri, Jun 04, 2021 at 03:09:43PM +0100, Daniel Scally wrote:
> On 04/06/2021 14:01, Andy Shevchenko wrote:
> > On Thu, Jun 03, 2021 at 11:40:05PM +0100, Daniel Scally wrote:
> >> Add a function to verify that a given acpi_resource represents an IO
> >> type GPIO resource, and return it if so.
> > I would rephrase this to something like:
> >
> > "Add a function to verify that a given ACPI resource represents a GpioIo() type
> > of resource, and return it if so."
> >
> > I can amend when applying to my branch.
> >
> That wording is perfectly fine by me

These two patches (with mentioned amendments) pushed to my review and testing
queue, thanks!

I'll send PR next week to all stakeholders.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource()
  2021-06-04 14:33       ` Andy Shevchenko
@ 2021-06-04 16:54         ` Daniel Scally
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-04 16:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J . Wysocki, Lee Jones, Hans de Goede, Maximilian Luz,
	linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, laurent.pinchart, kieran.bingham


On 04/06/2021 15:33, Andy Shevchenko wrote:
> On Fri, Jun 04, 2021 at 03:09:43PM +0100, Daniel Scally wrote:
>> On 04/06/2021 14:01, Andy Shevchenko wrote:
>>> On Thu, Jun 03, 2021 at 11:40:05PM +0100, Daniel Scally wrote:
>>>> Add a function to verify that a given acpi_resource represents an IO
>>>> type GPIO resource, and return it if so.
>>> I would rephrase this to something like:
>>>
>>> "Add a function to verify that a given ACPI resource represents a GpioIo() type
>>> of resource, and return it if so."
>>>
>>> I can amend when applying to my branch.
>>>
>> That wording is perfectly fine by me
> These two patches (with mentioned amendments) pushed to my review and testing
> queue, thanks!
>
> I'll send PR next week to all stakeholders.
>
Great - thanks very much!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-04  1:31   ` kernel test robot
@ 2021-06-05 23:07     ` Daniel Scally
  0 siblings, 0 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-05 23:07 UTC (permalink / raw)
  To: kernel test robot, Rafael J . Wysocki, Andy Shevchenko,
	Lee Jones, Hans de Goede, Maximilian Luz, linux-kernel,
	linux-acpi, linux-gpio, linux-i2c
  Cc: kbuild-all

Ah - forgot to make the function static, let me sent another of this patch.

On 04/06/2021 02:31, kernel test robot wrote:
> Hi Daniel,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on pm/linux-next]
> [also build test WARNING on lee-mfd/for-mfd-next linus/master v5.13-rc4 next-20210603]
> [cannot apply to gpio/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/Introduce-intel_skl_int3472-module/20210604-064345
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: x86_64-allyesconfig (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         # https://github.com/0day-ci/linux/commit/3edcad8c200f211063a35d125e9fd350a2efeb40
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Daniel-Scally/Introduce-intel_skl_int3472-module/20210604-064345
>         git checkout 3edcad8c200f211063a35d125e9fd350a2efeb40
>         # save the attached .config to linux build tree
>         make W=1 ARCH=x86_64 
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c:76:5: warning: no previous prototype for 'skl_int3472_tps68470_calc_type' [-Wmissing-prototypes]
>       76 | int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
>          |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> vim +/skl_int3472_tps68470_calc_type +76 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
>
>     52	
>     53	/** skl_int3472_tps68470_calc_type: Check what platform a device is designed for
>     54	 * @adev: A pointer to a &struct acpi_device
>     55	 *
>     56	 * Check CLDB buffer against the PMIC's adev. If present, then we check
>     57	 * the value of control_logic_type field and follow one of the
>     58	 * following scenarios:
>     59	 *
>     60	 *	1. No CLDB - likely ACPI tables designed for ChromeOS. We
>     61	 *	create platform devices for the GPIOs and OpRegion drivers.
>     62	 *
>     63	 *	2. CLDB, with control_logic_type = 2 - probably ACPI tables
>     64	 *	made for Windows 2-in-1 platforms. Register pdevs for GPIO,
>     65	 *	Clock and Regulator drivers to bind to.
>     66	 *
>     67	 *	3. Any other value in control_logic_type, we should never have
>     68	 *	gotten to this point; fail probe and return.
>     69	 *
>     70	 * Return:
>     71	 * * 1		Device intended for ChromeOS
>     72	 * * 2		Device intended for Windows
>     73	 * * -EINVAL	Where @adev has an object named CLDB but it does not conform to
>     74	 *		our expectations
>     75	 */
>   > 76	int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
>     77	{
>     78		struct int3472_cldb cldb = { 0 };
>     79		int ret;
>     80	
>     81		/*
>     82		 * A CLDB buffer that exists, but which does not match our expectations
>     83		 * should trigger an error so we don't blindly continue.
>     84		 */
>     85		ret = skl_int3472_fill_cldb(adev, &cldb);
>     86		if (ret && ret != -ENODEV)
>     87			return ret;
>     88	
>     89		if (ret)
>     90			return DESIGNED_FOR_CHROMEOS;
>     91	
>     92		if (cldb.control_logic_type != 2)
>     93			return -EINVAL;
>     94	
>     95		return DESIGNED_FOR_WINDOWS;
>     96	}
>     97	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 2/6] ACPI: scan: Add function to fetch dependent of acpi device
  2021-06-03 22:40 ` [PATCH v5 2/6] ACPI: scan: Add function to fetch dependent of acpi device Daniel Scally
@ 2021-06-07 14:52   ` Rafael J. Wysocki
  0 siblings, 0 replies; 24+ messages in thread
From: Rafael J. Wysocki @ 2021-06-07 14:52 UTC (permalink / raw)
  To: Daniel Scally
  Cc: Rafael J . Wysocki, Andy Shevchenko, Lee Jones, Hans de Goede,
	Maximilian Luz, Linux Kernel Mailing List,
	ACPI Devel Maling List, open list:GPIO SUBSYSTEM, linux-i2c,
	Platform Driver, open list:ACPI COMPONENT ARCHITECTURE (ACPICA),
	Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	Laurent Pinchart, Kieran Bingham, Andy Shevchenko

On Fri, Jun 4, 2021 at 12:41 AM Daniel Scally <djrscally@gmail.com> wrote:
>
> In some ACPI tables we encounter, devices use the _DEP method to assert
> a dependence on other ACPI devices as opposed to the OpRegions that the
> specification intends. We need to be able to find those devices "from"
> the dependee, so add a callback and a wrapper to walk over the
> acpi_dep_list and return the dependent ACPI device.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes since v5:
>
>         - Functions renamed
>
>  drivers/acpi/scan.c     | 35 +++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h |  1 +
>  2 files changed, 36 insertions(+)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 195635c3462b..9af64c34e286 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2105,6 +2105,20 @@ 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)
> +{
> +       struct acpi_device *adev;
> +
> +       adev = acpi_bus_get_acpi_device(dep->consumer);
> +       if (!adev)
> +               /* If we don't find an adev then we want to continue parsing */
> +               return 0;
> +
> +       *(struct acpi_device **)data = adev;
> +
> +       return 1;
> +}
> +
>  static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data)
>  {
>         struct acpi_device *adev;
> @@ -2168,6 +2182,27 @@ void acpi_dev_clear_dependencies(struct acpi_device *supplier)
>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies);
>
> +/**
> + * 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)
> +{
> +       struct acpi_device *adev = NULL;
> +
> +       acpi_walk_dep_device_list(supplier->handle,
> +                                 acpi_dev_get_first_consumer_dev_cb, &adev);
> +
> +       return adev;
> +}
> +EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
> +
>  /**
>   * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
>   * @handle: Root of the namespace scope to scan.
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 0b2c4f170f4d..4bed30e61c5b 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -692,6 +692,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>  bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>
>  void acpi_dev_clear_dependencies(struct acpi_device *supplier);
> +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);
>  struct acpi_device *
> --

Applied as 5.14 material along the [1/6].

I'll expose a branch containing the 2 patches to pull from later this week.

Thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 0/6] Introduce intel_skl_int3472 module
  2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
                   ` (5 preceding siblings ...)
  2021-06-03 22:40 ` [PATCH v5 6/6] mfd: tps68470: Remove tps68470 MFD driver Daniel Scally
@ 2021-06-08  9:00 ` Hans de Goede
  2021-06-08  9:33   ` Laurent Pinchart
  6 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-06-08  9:00 UTC (permalink / raw)
  To: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

Hi,

On 6/4/21 12:40 AM, Daniel Scally wrote:
> Hello all
> 
> Bit longer than hoped but here's v5.
> 
> v4:
> https://lore.kernel.org/lkml/20210520140928.3252671-1-djrscally@gmail.com/
> 
> v3
> https://lore.kernel.org/lkml/20210222130735.1313443-1-djrscally@gmail.com/
> 
> v2
> https://lore.kernel.org/platform-driver-x86/20210118003428.568892-1-djrscally@gmail.com/
> 
> v1
> https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
> 
> The only changes are the dropped patches, renamed functions in 2/6 and most of
> Andy's suggestions on 5/6 - I didn't hit them all yet but didn't want to delay
> this any more.
> 
> Series level changelog:
> 
> 	- Dropped all but the essential patches to simplify merge plan - thanks
> 	Hans.

Thank you. Andy has already sends me a pull-req for the gpiolib-acpi changes
and I expect Rafael to send me a pull-req (from an immutable branch) for the
ACPI bits soon-ish.

When I merge those both into pdx86 I should be able to pick-up 5/6.

Given the long time this has been in the making I'm tempted to do that
(pick up 5/6 as is) and then we can do further cleanups like looking into
using the existing fixed-regulator code later. My mean reason to do this
would be to get this code into the hands of users starting with the 5.14
kernel.

So question, how usable is this (from the kernel pov, I know userspace
needs work too) once I merge 5/6. Is the kernel-side support for the cameras
on some Surface devices then complete or are more patches necessary in
other subsystems ?

Note another advantage of just merging 5/6 as is and doing the fixed-regulator
bits on top, is that having those in a separate commit makes it easier to
see if these indeed result in a nice cleanup (vs sticking with the current code).

Regards,

Hans






> 
> Daniel Scally (6):
>   ACPI: scan: Extend acpi_walk_dep_device_list()
>   ACPI: scan: Add function to fetch dependent of acpi device
>   gpiolib: acpi: Export acpi_get_gpiod()
>   gpiolib: acpi: Add acpi_gpio_get_io_resource()
>   platform/x86: Add intel_skl_int3472 driver
>   mfd: tps68470: Remove tps68470 MFD driver
> 
>  MAINTAINERS                                   |   5 +
>  drivers/acpi/ec.c                             |   2 +-
>  drivers/acpi/pmic/Kconfig                     |   2 +-
>  drivers/acpi/pmic/intel_pmic_chtdc_ti.c       |   2 +-
>  drivers/acpi/scan.c                           | 104 ++++-
>  drivers/gpio/Kconfig                          |   2 +-
>  drivers/gpio/gpiolib-acpi.c                   |  61 ++-
>  drivers/i2c/i2c-core-acpi.c                   |   8 +-
>  drivers/mfd/Kconfig                           |  18 -
>  drivers/mfd/Makefile                          |   1 -
>  drivers/mfd/tps68470.c                        |  97 ----
>  drivers/platform/surface/aggregator/core.c    |   6 +-
>  drivers/platform/surface/surface3_power.c     |  22 +-
>  .../platform/surface/surface_acpi_notify.c    |   7 +-
>  drivers/platform/x86/Kconfig                  |   2 +
>  drivers/platform/x86/Makefile                 |   1 +
>  drivers/platform/x86/intel-int3472/Kconfig    |  30 ++
>  drivers/platform/x86/intel-int3472/Makefile   |   5 +
>  .../intel_skl_int3472_clk_and_regulator.c     | 196 ++++++++
>  .../intel-int3472/intel_skl_int3472_common.c  | 106 +++++
>  .../intel-int3472/intel_skl_int3472_common.h  | 118 +++++
>  .../intel_skl_int3472_discrete.c              | 417 ++++++++++++++++++
>  .../intel_skl_int3472_tps68470.c              | 137 ++++++
>  include/acpi/acpi_bus.h                       |   8 +
>  include/linux/acpi.h                          |  11 +-
>  include/linux/gpio/consumer.h                 |   2 +
>  26 files changed, 1205 insertions(+), 165 deletions(-)
>  delete mode 100644 drivers/mfd/tps68470.c
>  create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
>  create mode 100644 drivers/platform/x86/intel-int3472/Makefile
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 0/6] Introduce intel_skl_int3472 module
  2021-06-08  9:00 ` [PATCH v5 0/6] Introduce intel_skl_int3472 module Hans de Goede
@ 2021-06-08  9:33   ` Laurent Pinchart
  0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2021-06-08  9:33 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel, Len Brown, Mika Westerberg,
	Linus Walleij, Bartosz Golaszewski, Wolfram Sang, Mark Gross,
	Robert Moore, Erik Kaneda, kieran.bingham

Hi Hans,

On Tue, Jun 08, 2021 at 11:00:24AM +0200, Hans de Goede wrote:
> On 6/4/21 12:40 AM, Daniel Scally wrote:
> > Hello all
> > 
> > Bit longer than hoped but here's v5.
> > 
> > v4:
> > https://lore.kernel.org/lkml/20210520140928.3252671-1-djrscally@gmail.com/
> > 
> > v3
> > https://lore.kernel.org/lkml/20210222130735.1313443-1-djrscally@gmail.com/
> > 
> > v2
> > https://lore.kernel.org/platform-driver-x86/20210118003428.568892-1-djrscally@gmail.com/
> > 
> > v1
> > https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
> > 
> > The only changes are the dropped patches, renamed functions in 2/6 and most of
> > Andy's suggestions on 5/6 - I didn't hit them all yet but didn't want to delay
> > this any more.
> > 
> > Series level changelog:
> > 
> > 	- Dropped all but the essential patches to simplify merge plan - thanks
> > 	Hans.
> 
> Thank you. Andy has already sends me a pull-req for the gpiolib-acpi changes
> and I expect Rafael to send me a pull-req (from an immutable branch) for the
> ACPI bits soon-ish.
> 
> When I merge those both into pdx86 I should be able to pick-up 5/6.
> 
> Given the long time this has been in the making I'm tempted to do that
> (pick up 5/6 as is) and then we can do further cleanups like looking into
> using the existing fixed-regulator code later. My mean reason to do this
> would be to get this code into the hands of users starting with the 5.14
> kernel.

That would be great !

> So question, how usable is this (from the kernel pov, I know userspace
> needs work too) once I merge 5/6. Is the kernel-side support for the cameras
> on some Surface devices then complete or are more patches necessary in
> other subsystems ?

Camera sensor drivers are the other piece of the puzzle that is
required. Daniel is working on an ov5693 driver which can be found at
https://github.com/djrscally/media_tree/tree/ov5693-v2. It's nearing
completion too.

With those two pieces, we have support for the Microsoft Surface Go 2 in
libcamera, as well as a few other devices whose exact model escapes me
at this minute.

> Note another advantage of just merging 5/6 as is and doing the fixed-regulator
> bits on top, is that having those in a separate commit makes it easier to
> see if these indeed result in a nice cleanup (vs sticking with the current code).
> 
> > Daniel Scally (6):
> >   ACPI: scan: Extend acpi_walk_dep_device_list()
> >   ACPI: scan: Add function to fetch dependent of acpi device
> >   gpiolib: acpi: Export acpi_get_gpiod()
> >   gpiolib: acpi: Add acpi_gpio_get_io_resource()
> >   platform/x86: Add intel_skl_int3472 driver
> >   mfd: tps68470: Remove tps68470 MFD driver
> > 
> >  MAINTAINERS                                   |   5 +
> >  drivers/acpi/ec.c                             |   2 +-
> >  drivers/acpi/pmic/Kconfig                     |   2 +-
> >  drivers/acpi/pmic/intel_pmic_chtdc_ti.c       |   2 +-
> >  drivers/acpi/scan.c                           | 104 ++++-
> >  drivers/gpio/Kconfig                          |   2 +-
> >  drivers/gpio/gpiolib-acpi.c                   |  61 ++-
> >  drivers/i2c/i2c-core-acpi.c                   |   8 +-
> >  drivers/mfd/Kconfig                           |  18 -
> >  drivers/mfd/Makefile                          |   1 -
> >  drivers/mfd/tps68470.c                        |  97 ----
> >  drivers/platform/surface/aggregator/core.c    |   6 +-
> >  drivers/platform/surface/surface3_power.c     |  22 +-
> >  .../platform/surface/surface_acpi_notify.c    |   7 +-
> >  drivers/platform/x86/Kconfig                  |   2 +
> >  drivers/platform/x86/Makefile                 |   1 +
> >  drivers/platform/x86/intel-int3472/Kconfig    |  30 ++
> >  drivers/platform/x86/intel-int3472/Makefile   |   5 +
> >  .../intel_skl_int3472_clk_and_regulator.c     | 196 ++++++++
> >  .../intel-int3472/intel_skl_int3472_common.c  | 106 +++++
> >  .../intel-int3472/intel_skl_int3472_common.h  | 118 +++++
> >  .../intel_skl_int3472_discrete.c              | 417 ++++++++++++++++++
> >  .../intel_skl_int3472_tps68470.c              | 137 ++++++
> >  include/acpi/acpi_bus.h                       |   8 +
> >  include/linux/acpi.h                          |  11 +-
> >  include/linux/gpio/consumer.h                 |   2 +
> >  26 files changed, 1205 insertions(+), 165 deletions(-)
> >  delete mode 100644 drivers/mfd/tps68470.c
> >  create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
> >  create mode 100644 drivers/platform/x86/intel-int3472/Makefile
> >  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
> >  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
> >  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
> >  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
> >  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-03 22:40 ` [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver Daniel Scally
  2021-06-04  1:31   ` kernel test robot
  2021-06-04  6:49   ` kernel test robot
@ 2021-06-16 17:58   ` Hans de Goede
  2021-06-16 18:08     ` Andy Shevchenko
  2021-06-16 21:42     ` Daniel Scally
  2 siblings, 2 replies; 24+ messages in thread
From: Hans de Goede @ 2021-06-16 17:58 UTC (permalink / raw)
  To: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

Hi,

On 6/4/21 12:40 AM, Daniel Scally wrote:
> ACPI devices with _HID INT3472 are currently matched to the tps68470
> driver, however this does not cover all situations in which that _HID
> occurs. We've encountered three possibilities:
> 
> 1. On Chrome OS devices, an ACPI device with _HID INT3472 (representing
> a physical TPS68470 device) that requires a GPIO and OpRegion driver
> 2. On devices designed for Windows, an ACPI device with _HID INT3472
> (again representing a physical TPS68470 device) which requires GPIO,
> Clock and Regulator drivers.
> 3. On other devices designed for Windows, an ACPI device with _HID
> INT3472 which does **not** represent a physical TPS68470, and is instead
> used as a dummy device to group some system GPIO lines which are meant
> to be consumed by the sensor that is dependent on this entry.
> 
> This commit adds a new module, registering a platform driver to deal
> with the 3rd scenario plus an i2c driver to deal with #1 and #2, by
> querying the CLDB buffer found against INT3472 entries to determine
> which is most appropriate.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

I've fixed up the missing static marking of skl_int3472_tps68470_calc_type()
spotted by lkp@intel.com while applying the patch to my tree.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans




> ---
> Changes since v4:
> 	- Some formatting and commenting tidy up
> 	- Cleared up some function exit paths to avoid gotos
> 	- Refactored skl_int3472_get_sensor_module_config() and also
> 	skl_int3472_tps68470_probe(), which involved adding a new function.
> 
> Andy - I didn't look at the reg-fixed-voltage suggestion yet, but I think I
> covered the rest of your suggestions from v4.
> 
>  MAINTAINERS                                   |   5 +
>  drivers/platform/x86/Kconfig                  |   2 +
>  drivers/platform/x86/Makefile                 |   1 +
>  drivers/platform/x86/intel-int3472/Kconfig    |  30 ++
>  drivers/platform/x86/intel-int3472/Makefile   |   5 +
>  .../intel_skl_int3472_clk_and_regulator.c     | 196 ++++++++
>  .../intel-int3472/intel_skl_int3472_common.c  | 106 +++++
>  .../intel-int3472/intel_skl_int3472_common.h  | 118 +++++
>  .../intel_skl_int3472_discrete.c              | 417 ++++++++++++++++++
>  .../intel_skl_int3472_tps68470.c              | 137 ++++++
>  10 files changed, 1017 insertions(+)
>  create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
>  create mode 100644 drivers/platform/x86/intel-int3472/Makefile
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 008fcad7ac00..8dcc7e005ace 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9378,6 +9378,11 @@ S:	Maintained
>  F:	arch/x86/include/asm/intel_scu_ipc.h
>  F:	drivers/platform/x86/intel_scu_*
>  
> +INTEL SKYLAKE INT3472 ACPI DEVICE DRIVER
> +M:	Daniel Scally <djrscally@gmail.com>
> +S:	Maintained
> +F:	drivers/platform/x86/intel-int3472/
> +
>  INTEL SPEED SELECT TECHNOLOGY
>  M:	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>  L:	platform-driver-x86@vger.kernel.org
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 2714f7c3843e..1263b8efcd50 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -697,6 +697,8 @@ config INTEL_CHT_INT33FE
>  	  device and CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m
>  	  for Type-C device.
>  
> +source "drivers/platform/x86/intel-int3472/Kconfig"
> +
>  config INTEL_HID_EVENT
>  	tristate "INTEL HID Event"
>  	depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index dcc8cdb95b4d..c0612c02d037 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_INTEL_HID_EVENT)		+= intel-hid.o
>  obj-$(CONFIG_INTEL_INT0002_VGPIO)	+= intel_int0002_vgpio.o
>  obj-$(CONFIG_INTEL_MENLOW)		+= intel_menlow.o
>  obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
> +obj-$(CONFIG_INTEL_SKL_INT3472)		+= intel-int3472/
>  obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
>  
>  # MSI
> diff --git a/drivers/platform/x86/intel-int3472/Kconfig b/drivers/platform/x86/intel-int3472/Kconfig
> new file mode 100644
> index 000000000000..c112878e833b
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/Kconfig
> @@ -0,0 +1,30 @@
> +config INTEL_SKL_INT3472
> +	tristate "Intel SkyLake ACPI INT3472 Driver"
> +	depends on ACPI
> +	depends on COMMON_CLK && CLKDEV_LOOKUP
> +	depends on I2C
> +	depends on GPIOLIB
> +	depends on REGULATOR
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  This driver adds power controller support for the Intel SkyCam
> +	  devices found on the Intel SkyLake platforms.
> +
> +	  The INT3472 is a camera power controller, a logical device found on
> +	  Intel Skylake-based systems that can map to different hardware
> +	  devices depending on the platform. On machines designed for Chrome OS
> +	  it maps to a TPS68470 camera PMIC. On machines designed for Windows,
> +	  it maps to either a TP68470 camera PMIC, a uP6641Q sensor PMIC, or a
> +	  set of discrete GPIOs and power gates.
> +
> +	  If your device was designed for Chrome OS, this driver will provide
> +	  an ACPI OpRegion, which must be available before any of the devices
> +	  using it are probed. For this reason, you should select Y if your
> +	  device was designed for ChromeOS. For the same reason the
> +	  I2C_DESIGNWARE_PLATFORM option must be set to Y too.
> +
> +	  Say Y or M here if you have a SkyLake device designed for use
> +	  with Windows or ChromeOS. Say N here if you are not sure.
> +
> +	  The module will be named "intel-skl-int3472".
> diff --git a/drivers/platform/x86/intel-int3472/Makefile b/drivers/platform/x86/intel-int3472/Makefile
> new file mode 100644
> index 000000000000..48bd97f0a04e
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/Makefile
> @@ -0,0 +1,5 @@
> +obj-$(CONFIG_INTEL_SKL_INT3472)		+= intel_skl_int3472.o
> +intel_skl_int3472-objs			:= intel_skl_int3472_common.o \
> +					   intel_skl_int3472_discrete.o \
> +					   intel_skl_int3472_tps68470.o \
> +					   intel_skl_int3472_clk_and_regulator.o
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
> new file mode 100644
> index 000000000000..ceee860e2c07
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +
> +#include <linux/acpi.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/slab.h>
> +
> +#include "intel_skl_int3472_common.h"
> +
> +/*
> + * The regulators have to have .ops to be valid, but the only ops we actually
> + * support are .enable and .disable which are handled via .ena_gpiod. Pass an
> + * empty struct to clear the check without lying about capabilities.
> + */
> +static const struct regulator_ops int3472_gpio_regulator_ops;
> +
> +static int skl_int3472_clk_prepare(struct clk_hw *hw)
> +{
> +	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
> +
> +	gpiod_set_value_cansleep(clk->ena_gpio, 1);
> +	gpiod_set_value_cansleep(clk->led_gpio, 1);
> +
> +	return 0;
> +}
> +
> +static void skl_int3472_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
> +
> +	gpiod_set_value_cansleep(clk->ena_gpio, 0);
> +	gpiod_set_value_cansleep(clk->led_gpio, 0);
> +}
> +
> +static int skl_int3472_clk_enable(struct clk_hw *hw)
> +{
> +	/*
> +	 * We're just turning a GPIO on to enable the clock, which operation
> +	 * has the potential to sleep. Given .enable() cannot sleep, but
> +	 * .prepare() can, we toggle the GPIO in .prepare() instead. Thus,
> +	 * nothing to do here.
> +	 */
> +	return 0;
> +}
> +
> +static void skl_int3472_clk_disable(struct clk_hw *hw)
> +{
> +	/* Likewise, nothing to do here... */
> +}
> +
> +static unsigned int skl_int3472_get_clk_frequency(struct int3472_discrete_device *int3472)
> +{
> +	union acpi_object *obj;
> +	unsigned int freq;
> +
> +	obj = skl_int3472_get_acpi_buffer(int3472->sensor, "SSDB");
> +	if (IS_ERR(obj))
> +		return 0; /* report rate as 0 on error */
> +
> +	if (obj->buffer.length < CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET + sizeof(u32)) {
> +		dev_err(int3472->dev, "The buffer is too small\n");
> +		kfree(obj);
> +		return 0;
> +	}
> +
> +	freq = *(u32 *)(obj->buffer.pointer + CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET);
> +
> +	kfree(obj);
> +	return freq;
> +}
> +
> +static unsigned long skl_int3472_clk_recalc_rate(struct clk_hw *hw,
> +						 unsigned long parent_rate)
> +{
> +	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
> +
> +	return clk->frequency;
> +}
> +
> +static const struct clk_ops skl_int3472_clock_ops = {
> +	.prepare = skl_int3472_clk_prepare,
> +	.unprepare = skl_int3472_clk_unprepare,
> +	.enable = skl_int3472_clk_enable,
> +	.disable = skl_int3472_clk_disable,
> +	.recalc_rate = skl_int3472_clk_recalc_rate,
> +};
> +
> +int skl_int3472_register_clock(struct int3472_discrete_device *int3472)
> +{
> +	struct clk_init_data init = {
> +		.ops = &skl_int3472_clock_ops,
> +		.flags = CLK_GET_RATE_NOCACHE,
> +	};
> +	int ret;
> +
> +	init.name = kasprintf(GFP_KERNEL, "%s-clk",
> +			      acpi_dev_name(int3472->adev));
> +	if (!init.name)
> +		return -ENOMEM;
> +
> +	int3472->clock.frequency = skl_int3472_get_clk_frequency(int3472);
> +
> +	int3472->clock.clk_hw.init = &init;
> +	int3472->clock.clk = clk_register(&int3472->adev->dev,
> +					  &int3472->clock.clk_hw);
> +	if (IS_ERR(int3472->clock.clk)) {
> +		ret = PTR_ERR(int3472->clock.clk);
> +		goto out_free_init_name;
> +	}
> +
> +	int3472->clock.cl = clkdev_create(int3472->clock.clk, NULL,
> +					  int3472->sensor_name);
> +	if (!int3472->clock.cl) {
> +		ret = -ENOMEM;
> +		goto err_unregister_clk;
> +	}
> +
> +	kfree(init.name);
> +	return 0;
> +
> +err_unregister_clk:
> +	clk_unregister(int3472->clock.clk);
> +out_free_init_name:
> +	kfree(init.name);
> +
> +	return ret;
> +}
> +
> +int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
> +				   struct acpi_resource *ares)
> +{
> +	char *path = ares->data.gpio.resource_source.string_ptr;
> +	const struct int3472_sensor_config *sensor_config;
> +	struct regulator_consumer_supply supply_map;
> +	struct regulator_init_data init_data = { };
> +	struct regulator_config cfg = { };
> +	int ret;
> +
> +	sensor_config = int3472->sensor_config;
> +	if (IS_ERR(sensor_config)) {
> +		dev_err(int3472->dev, "No sensor module config\n");
> +		return PTR_ERR(sensor_config);
> +	}
> +
> +	if (!sensor_config->supply_map.supply) {
> +		dev_err(int3472->dev, "No supply name defined\n");
> +		return -ENODEV;
> +	}
> +
> +	init_data.constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
> +	init_data.num_consumer_supplies = 1;
> +	supply_map = sensor_config->supply_map;
> +	supply_map.dev_name = int3472->sensor_name;
> +	init_data.consumer_supplies = &supply_map;
> +
> +	snprintf(int3472->regulator.regulator_name,
> +		 sizeof(int3472->regulator.regulator_name), "%s-regulator",
> +		 acpi_dev_name(int3472->adev));
> +	snprintf(int3472->regulator.supply_name,
> +		 GPIO_REGULATOR_SUPPLY_NAME_LENGTH, "supply-0");
> +
> +	int3472->regulator.rdesc = INT3472_REGULATOR(
> +						int3472->regulator.regulator_name,
> +						int3472->regulator.supply_name,
> +						&int3472_gpio_regulator_ops);
> +
> +	int3472->regulator.gpio = acpi_get_and_request_gpiod(path,
> +							     ares->data.gpio.pin_table[0],
> +							     "int3472,regulator");
> +	if (IS_ERR(int3472->regulator.gpio)) {
> +		dev_err(int3472->dev, "Failed to get regulator GPIO line\n");
> +		return PTR_ERR(int3472->regulator.gpio);
> +	}
> +
> +	cfg.dev = &int3472->adev->dev;
> +	cfg.init_data = &init_data;
> +	cfg.ena_gpiod = int3472->regulator.gpio;
> +
> +	int3472->regulator.rdev = regulator_register(&int3472->regulator.rdesc,
> +						     &cfg);
> +	if (IS_ERR(int3472->regulator.rdev)) {
> +		ret = PTR_ERR(int3472->regulator.rdev);
> +		goto err_free_gpio;
> +	}
> +
> +	return 0;
> +
> +err_free_gpio:
> +	gpiod_put(int3472->regulator.gpio);
> +
> +	return ret;
> +}
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
> new file mode 100644
> index 000000000000..497e74fba75f
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "intel_skl_int3472_common.h"
> +
> +union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev, char *id)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	acpi_handle handle = adev->handle;
> +	union acpi_object *obj;
> +	acpi_status status;
> +
> +	status = acpi_evaluate_object(handle, id, NULL, &buffer);
> +	if (ACPI_FAILURE(status))
> +		return ERR_PTR(-ENODEV);
> +
> +	obj = buffer.pointer;
> +	if (!obj)
> +		return ERR_PTR(-ENODEV);
> +
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		acpi_handle_err(handle, "%s object is not an ACPI buffer\n", id);
> +		kfree(obj);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return obj;
> +}
> +
> +int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
> +{
> +	union acpi_object *obj;
> +	int ret;
> +
> +	obj = skl_int3472_get_acpi_buffer(adev, "CLDB");
> +	if (IS_ERR(obj))
> +		return PTR_ERR(obj);
> +
> +	if (obj->buffer.length > sizeof(*cldb)) {
> +		acpi_handle_err(adev->handle, "The CLDB buffer is too large\n");
> +		ret = -EINVAL;
> +		goto out_free_obj;
> +	}
> +
> +	memcpy(cldb, obj->buffer.pointer, obj->buffer.length);
> +	ret = 0;
> +
> +out_free_obj:
> +	kfree(obj);
> +	return ret;
> +}
> +
> +static const struct acpi_device_id int3472_device_id[] = {
> +	{ "INT3472", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, int3472_device_id);
> +
> +static struct platform_driver int3472_discrete = {
> +	.driver = {
> +		.name = "int3472-discrete",
> +		.acpi_match_table = int3472_device_id,
> +	},
> +	.probe = skl_int3472_discrete_probe,
> +	.remove = skl_int3472_discrete_remove,
> +};
> +
> +static struct i2c_driver int3472_tps68470 = {
> +	.driver = {
> +		.name = "int3472-tps68470",
> +		.acpi_match_table = int3472_device_id,
> +	},
> +	.probe_new = skl_int3472_tps68470_probe,
> +};
> +
> +static int skl_int3472_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&int3472_discrete);
> +	if (ret)
> +		return ret;
> +
> +	ret = i2c_register_driver(THIS_MODULE, &int3472_tps68470);
> +	if (ret)
> +		platform_driver_unregister(&int3472_discrete);
> +
> +	return ret;
> +}
> +module_init(skl_int3472_init);
> +
> +static void skl_int3472_exit(void)
> +{
> +	platform_driver_unregister(&int3472_discrete);
> +	i2c_del_driver(&int3472_tps68470);
> +}
> +module_exit(skl_int3472_exit);
> +
> +MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Device Driver");
> +MODULE_AUTHOR("Daniel Scally <djrscally@gmail.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
> new file mode 100644
> index 000000000000..6fdf78584219
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
> @@ -0,0 +1,118 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +
> +#ifndef _INTEL_SKL_INT3472_H
> +#define _INTEL_SKL_INT3472_H
> +
> +#include <linux/clk-provider.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/types.h>
> +
> +/* FIXME drop this once the I2C_DEV_NAME_FORMAT macro has been added to include/linux/i2c.h */
> +#ifndef I2C_DEV_NAME_FORMAT
> +#define I2C_DEV_NAME_FORMAT					"i2c-%s"
> +#endif
> +
> +/* PMIC GPIO Types */
> +#define INT3472_GPIO_TYPE_RESET					0x00
> +#define INT3472_GPIO_TYPE_POWERDOWN				0x01
> +#define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
> +#define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
> +#define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
> +
> +#define INT3472_PDEV_MAX_NAME_LEN				23
> +#define INT3472_MAX_SENSOR_GPIOS				3
> +
> +#define GPIO_REGULATOR_NAME_LENGTH				21
> +#define GPIO_REGULATOR_SUPPLY_NAME_LENGTH			9
> +
> +#define CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET			86
> +
> +#define INT3472_REGULATOR(_name, _supply, _ops)			\
> +	(const struct regulator_desc) {				\
> +		.name = _name,					\
> +		.supply_name = _supply,				\
> +		.type = REGULATOR_VOLTAGE,			\
> +		.ops = _ops,					\
> +		.owner = THIS_MODULE,				\
> +	}
> +
> +#define to_int3472_clk(hw)					\
> +	container_of(hw, struct int3472_gpio_clock, clk_hw)
> +
> +#define to_int3472_device(clk)					\
> +	container_of(clk, struct int3472_discrete_device, clock)
> +
> +struct acpi_device;
> +struct i2c_client;
> +struct platform_device;
> +
> +struct int3472_cldb {
> +	u8 version;
> +	/*
> +	 * control logic type
> +	 * 0: UNKNOWN
> +	 * 1: DISCRETE(CRD-D)
> +	 * 2: PMIC TPS68470
> +	 * 3: PMIC uP6641
> +	 */
> +	u8 control_logic_type;
> +	u8 control_logic_id;
> +	u8 sensor_card_sku;
> +	u8 reserved[28];
> +};
> +
> +struct int3472_gpio_function_remap {
> +	const char *documented;
> +	const char *actual;
> +};
> +
> +struct int3472_sensor_config {
> +	const char *sensor_module_name;
> +	struct regulator_consumer_supply supply_map;
> +	const struct int3472_gpio_function_remap *function_maps;
> +};
> +
> +struct int3472_discrete_device {
> +	struct acpi_device *adev;
> +	struct device *dev;
> +	struct acpi_device *sensor;
> +	const char *sensor_name;
> +
> +	const struct int3472_sensor_config *sensor_config;
> +
> +	struct int3472_gpio_regulator {
> +		char regulator_name[GPIO_REGULATOR_NAME_LENGTH];
> +		char supply_name[GPIO_REGULATOR_SUPPLY_NAME_LENGTH];
> +		struct gpio_desc *gpio;
> +		struct regulator_dev *rdev;
> +		struct regulator_desc rdesc;
> +	} regulator;
> +
> +	struct int3472_gpio_clock {
> +		struct clk *clk;
> +		struct clk_hw clk_hw;
> +		struct clk_lookup *cl;
> +		struct gpio_desc *ena_gpio;
> +		struct gpio_desc *led_gpio;
> +		u32 frequency;
> +	} clock;
> +
> +	unsigned int ngpios; /* how many GPIOs have we seen */
> +	unsigned int n_sensor_gpios; /* how many have we mapped to sensor */
> +	struct gpiod_lookup_table gpios;
> +};
> +
> +int skl_int3472_discrete_probe(struct platform_device *pdev);
> +int skl_int3472_discrete_remove(struct platform_device *pdev);
> +int skl_int3472_tps68470_probe(struct i2c_client *client);
> +union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev,
> +					       char *id);
> +int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb);
> +int skl_int3472_register_clock(struct int3472_discrete_device *int3472);
> +int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
> +				   struct acpi_resource *ares);
> +
> +#endif
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
> new file mode 100644
> index 000000000000..8c18dbff1c43
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +
> +#include <linux/acpi.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/uuid.h>
> +
> +#include "intel_skl_int3472_common.h"
> +
> +/*
> + * 79234640-9e10-4fea-a5c1-b5aa8b19756f
> + * This _DSM GUID returns information about the GPIO lines mapped to a
> + * discrete INT3472 device. Function number 1 returns a count of the GPIO
> + * lines that are mapped. Subsequent functions return 32 bit ints encoding
> + * information about the GPIO line, including its purpose.
> + */
> +static const guid_t int3472_gpio_guid =
> +	GUID_INIT(0x79234640, 0x9e10, 0x4fea,
> +		  0xa5, 0xc1, 0xb5, 0xaa, 0x8b, 0x19, 0x75, 0x6f);
> +
> +/*
> + * 822ace8f-2814-4174-a56b-5f029fe079ee
> + * This _DSM GUID returns a string from the sensor device, which acts as a
> + * module identifier.
> + */
> +static const guid_t cio2_sensor_module_guid =
> +	GUID_INIT(0x822ace8f, 0x2814, 0x4174,
> +		  0xa5, 0x6b, 0x5f, 0x02, 0x9f, 0xe0, 0x79, 0xee);
> +
> +/*
> + * Here follows platform specific mapping information that we can pass to
> + * the functions mapping resources to the sensors. Where the sensors have
> + * a power enable pin defined in DSDT we need to provide a supply name so
> + * the sensor drivers can find the regulator. The device name will be derived
> + * from the sensor's ACPI device within the code. Optionally, we can provide a
> + * NULL terminated array of function name mappings to deal with any platform
> + * specific deviations from the documented behaviour of GPIOs.
> + *
> + * Map a GPIO function name to NULL to prevent the driver from mapping that
> + * GPIO at all.
> + */
> +
> +static const struct int3472_gpio_function_remap ov2680_gpio_function_remaps[] = {
> +	{ "reset", NULL },
> +	{ "powerdown", "reset" },
> +	{ }
> +};
> +
> +static const struct int3472_sensor_config int3472_sensor_configs[] = {
> +	/* Lenovo Miix 510-12ISK - OV2680, Front */
> +	{ "GNDF140809R", { 0 }, ov2680_gpio_function_remaps },
> +	/* Lenovo Miix 510-12ISK - OV5648, Rear */
> +	{ "GEFF150023R", REGULATOR_SUPPLY("avdd", NULL), NULL },
> +	/* Surface Go 1&2 - OV5693, Front */
> +	{ "YHCU", REGULATOR_SUPPLY("avdd", NULL), NULL },
> +};
> +
> +static const struct int3472_sensor_config *
> +skl_int3472_get_sensor_module_config(struct int3472_discrete_device *int3472)
> +{
> +	union acpi_object *obj;
> +	unsigned int i;
> +
> +	obj = acpi_evaluate_dsm_typed(int3472->sensor->handle,
> +				      &cio2_sensor_module_guid, 0x00,
> +				      0x01, NULL, ACPI_TYPE_STRING);
> +
> +	if (!obj) {
> +		dev_err(int3472->dev,
> +			"Failed to get sensor module string from _DSM\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	if (obj->string.type != ACPI_TYPE_STRING) {
> +		dev_err(int3472->dev,
> +			"Sensor _DSM returned a non-string value\n");
> +
> +		ACPI_FREE(obj);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(int3472_sensor_configs); i++) {
> +		if (!strcmp(int3472_sensor_configs[i].sensor_module_name,
> +			    obj->string.pointer))
> +			break;
> +	}
> +
> +	ACPI_FREE(obj);
> +
> +	if (i >= ARRAY_SIZE(int3472_sensor_configs))
> +		return ERR_PTR(-EINVAL);
> +
> +	return &int3472_sensor_configs[i];
> +}
> +
> +static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int3472,
> +					  struct acpi_resource *ares,
> +					  const char *func, u32 polarity)
> +{
> +	char *path = ares->data.gpio.resource_source.string_ptr;
> +	const struct int3472_sensor_config *sensor_config;
> +	struct gpiod_lookup *table_entry;
> +	struct acpi_device *adev;
> +	acpi_handle handle;
> +	acpi_status status;
> +	int ret;
> +
> +	if (int3472->n_sensor_gpios >= INT3472_MAX_SENSOR_GPIOS) {
> +		dev_warn(int3472->dev, "Too many GPIOs mapped\n");
> +		return -EINVAL;
> +	}
> +
> +	sensor_config = int3472->sensor_config;
> +	if (!IS_ERR(sensor_config) && sensor_config->function_maps) {
> +		const struct int3472_gpio_function_remap *remap;
> +
> +		for (remap = sensor_config->function_maps; remap->documented; remap++) {
> +			if (!strcmp(func, remap->documented)) {
> +				func = remap->actual;
> +				break;
> +			}
> +		}
> +	}
> +
> +	/* Functions mapped to NULL should not be mapped to the sensor */
> +	if (!func)
> +		return 0;
> +
> +	status = acpi_get_handle(NULL, path, &handle);
> +	if (ACPI_FAILURE(status))
> +		return -EINVAL;
> +
> +	ret = acpi_bus_get_device(handle, &adev);
> +	if (ret)
> +		return -ENODEV;
> +
> +	table_entry = &int3472->gpios.table[int3472->n_sensor_gpios];
> +	table_entry->key = acpi_dev_name(adev);
> +	table_entry->chip_hwnum = ares->data.gpio.pin_table[0];
> +	table_entry->con_id = func;
> +	table_entry->idx = 0;
> +	table_entry->flags = polarity;
> +
> +	int3472->n_sensor_gpios++;
> +
> +	return 0;
> +}
> +
> +static int skl_int3472_map_gpio_to_clk(struct int3472_discrete_device *int3472,
> +				       struct acpi_resource *ares, u8 type)
> +{
> +	char *path = ares->data.gpio.resource_source.string_ptr;
> +	struct gpio_desc *gpio;
> +
> +	switch (type) {
> +	case INT3472_GPIO_TYPE_CLK_ENABLE:
> +		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
> +						  "int3472,clk-enable");
> +		if (IS_ERR(gpio))
> +			return (PTR_ERR(gpio));
> +
> +		int3472->clock.ena_gpio = gpio;
> +		break;
> +	case INT3472_GPIO_TYPE_PRIVACY_LED:
> +		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
> +						  "int3472,privacy-led");
> +		if (IS_ERR(gpio))
> +			return (PTR_ERR(gpio));
> +
> +		int3472->clock.led_gpio = gpio;
> +		break;
> +	default:
> +		dev_err(int3472->dev, "Invalid GPIO type 0x%02x for clock\n", type);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * skl_int3472_handle_gpio_resources: Map PMIC resources to consuming sensor
> + * @ares: A pointer to a &struct acpi_resource
> + * @data: A pointer to a &struct int3472_discrete_device
> + *
> + * This function handles GPIO resources that are against an INT3472
> + * ACPI device, by checking the value of the corresponding _DSM entry.
> + * This will return a 32bit int, where the lowest byte represents the
> + * function of the GPIO pin:
> + *
> + * 0x00 Reset
> + * 0x01 Power down
> + * 0x0b Power enable
> + * 0x0c Clock enable
> + * 0x0d Privacy LED
> + *
> + * There are some known platform specific quirks where that does not quite
> + * hold up; for example where a pin with type 0x01 (Power down) is mapped to
> + * a sensor pin that performs a reset function or entries in _CRS and _DSM that
> + * do not actually correspond to a physical connection. These will be handled
> + * by the mapping sub-functions.
> + *
> + * GPIOs will either be mapped directly to the sensor device or else used
> + * to create clocks and regulators via the usual frameworks.
> + *
> + * Return:
> + * * 1		- To continue the loop
> + * * 0		- When all resources found are handled properly.
> + * * -EINVAL	- If the resource is not a GPIO IO resource
> + * * -ENODEV	- If the resource has no corresponding _DSM entry
> + * * -Other	- Errors propagated from one of the sub-functions.
> + */
> +static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
> +					     void *data)
> +{
> +	struct int3472_discrete_device *int3472 = data;
> +	struct acpi_resource_gpio *agpio;
> +	union acpi_object *obj;
> +	const char *err_msg;
> +	int ret;
> +	u8 type;
> +
> +	if (!acpi_gpio_get_io_resource(ares, &agpio))
> +		return 1;
> +
> +	/*
> +	 * ngpios + 2 because the index of this _DSM function is 1-based and
> +	 * the first function is just a count.
> +	 */
> +	obj = acpi_evaluate_dsm_typed(int3472->adev->handle,
> +				      &int3472_gpio_guid, 0x00,
> +				      int3472->ngpios + 2,
> +				      NULL, ACPI_TYPE_INTEGER);
> +
> +	if (!obj) {
> +		dev_warn(int3472->dev, "No _DSM entry for GPIO pin %u\n",
> +			 ares->data.gpio.pin_table[0]);
> +		return 1;
> +	}
> +
> +	type = obj->integer.value & 0xff;
> +
> +	switch (type) {
> +	case INT3472_GPIO_TYPE_RESET:
> +		ret = skl_int3472_map_gpio_to_sensor(int3472, ares, "reset",
> +						     GPIO_ACTIVE_LOW);
> +		if (ret)
> +			err_msg = "Failed to map reset pin to sensor\n";
> +
> +		break;
> +	case INT3472_GPIO_TYPE_POWERDOWN:
> +		ret = skl_int3472_map_gpio_to_sensor(int3472, ares,
> +						     "powerdown",
> +						     GPIO_ACTIVE_LOW);
> +		if (ret)
> +			err_msg = "Failed to map powerdown pin to sensor\n";
> +
> +		break;
> +	case INT3472_GPIO_TYPE_CLK_ENABLE:
> +	case INT3472_GPIO_TYPE_PRIVACY_LED:
> +		ret = skl_int3472_map_gpio_to_clk(int3472, ares, type);
> +		if (ret)
> +			err_msg = "Failed to map GPIO to clock\n";
> +
> +		break;
> +	case INT3472_GPIO_TYPE_POWER_ENABLE:
> +		ret = skl_int3472_register_regulator(int3472, ares);
> +		if (ret)
> +			err_msg = "Failed to map regulator to sensor\n";
> +
> +		break;
> +	default:
> +		dev_warn(int3472->dev,
> +			 "GPIO type 0x%02x unknown; the sensor may not work\n",
> +			 type);
> +		ret = 1;
> +		break;
> +	}
> +
> +	int3472->ngpios++;
> +	ACPI_FREE(obj);
> +
> +	if (ret)
> +		return dev_err_probe(int3472->dev, ret, err_msg);
> +
> +	return 0;
> +}
> +
> +static int skl_int3472_parse_crs(struct int3472_discrete_device *int3472)
> +{
> +	LIST_HEAD(resource_list);
> +	int ret;
> +
> +	/*
> +	 * No error check, because not having a sensor config is not necessarily
> +	 * a failure mode.
> +	 */
> +	int3472->sensor_config = skl_int3472_get_sensor_module_config(int3472);
> +
> +	ret = acpi_dev_get_resources(int3472->adev, &resource_list,
> +				     skl_int3472_handle_gpio_resources,
> +				     int3472);
> +	if (ret)
> +		goto out_free_res_list;
> +
> +	/*
> +	 * If we find no clock enable GPIO pin then the privacy LED won't work.
> +	 * We've never seen that situation, but it's possible. Warn the user so
> +	 * it's clear what's happened.
> +	 */
> +	if (int3472->clock.ena_gpio) {
> +		ret = skl_int3472_register_clock(int3472);
> +		if (ret)
> +			goto out_free_res_list;
> +	} else {
> +		if (int3472->clock.led_gpio)
> +			dev_warn(int3472->dev,
> +				 "No clk GPIO. The privacy LED won't work\n");
> +	}
> +
> +	int3472->gpios.dev_id = int3472->sensor_name;
> +	gpiod_add_lookup_table(&int3472->gpios);
> +
> +out_free_res_list:
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	return ret;
> +}
> +
> +int skl_int3472_discrete_probe(struct platform_device *pdev)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> +	struct int3472_discrete_device *int3472;
> +	struct int3472_cldb cldb;
> +	int ret;
> +
> +	ret = skl_int3472_fill_cldb(adev, &cldb);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Couldn't fill CLDB structure\n");
> +		return ret;
> +	}
> +
> +	if (cldb.control_logic_type != 1) {
> +		dev_err(&pdev->dev, "Unsupported control logic type %u\n",
> +			cldb.control_logic_type);
> +		return -EINVAL;
> +	}
> +
> +	/* Max num GPIOs we've seen plus a terminator */
> +	int3472 = devm_kzalloc(&pdev->dev, struct_size(int3472, gpios.table,
> +			       INT3472_MAX_SENSOR_GPIOS + 1), GFP_KERNEL);
> +	if (!int3472)
> +		return -ENOMEM;
> +
> +	int3472->adev = adev;
> +	int3472->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, int3472);
> +
> +	int3472->sensor = acpi_dev_get_first_consumer_dev(adev);
> +	if (!int3472->sensor) {
> +		dev_err(&pdev->dev, "INT3472 seems to have no dependents.\n");
> +		return -ENODEV;
> +	}
> +
> +	int3472->sensor_name = devm_kasprintf(int3472->dev, GFP_KERNEL,
> +					      I2C_DEV_NAME_FORMAT,
> +					      acpi_dev_name(int3472->sensor));
> +	if (!int3472->sensor_name) {
> +		ret = -ENOMEM;
> +		goto err_put_sensor;
> +	}
> +
> +	/*
> +	 * Initialising this list means we can call gpiod_remove_lookup_table()
> +	 * in failure paths without issue.
> +	 */
> +	INIT_LIST_HEAD(&int3472->gpios.list);
> +
> +	ret = skl_int3472_parse_crs(int3472);
> +	if (ret) {
> +		skl_int3472_discrete_remove(pdev);
> +		return ret;
> +	}
> +
> +	return 0;
> +
> +err_put_sensor:
> +	acpi_dev_put(int3472->sensor);
> +
> +	return ret;
> +}
> +
> +int skl_int3472_discrete_remove(struct platform_device *pdev)
> +{
> +	struct int3472_discrete_device *int3472 = platform_get_drvdata(pdev);
> +
> +	gpiod_remove_lookup_table(&int3472->gpios);
> +	regulator_unregister(int3472->regulator.rdev);
> +	clk_unregister(int3472->clock.clk);
> +
> +	if (int3472->clock.cl)
> +		clkdev_drop(int3472->clock.cl);
> +
> +	gpiod_put(int3472->regulator.gpio);
> +	gpiod_put(int3472->clock.ena_gpio);
> +	gpiod_put(int3472->clock.led_gpio);
> +
> +	return 0;
> +}
> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
> new file mode 100644
> index 000000000000..6306ba24e0bc
> --- /dev/null
> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/tps68470.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "intel_skl_int3472_common.h"
> +
> +#define DESIGNED_FOR_CHROMEOS		1
> +#define DESIGNED_FOR_WINDOWS		2
> +
> +static const struct mfd_cell tps68470_cros[] = {
> +	{ .name = "tps68470-gpio" },
> +	{ .name = "tps68470_pmic_opregion" },
> +};
> +
> +static const struct mfd_cell tps68470_win[] = {
> +	{ .name = "tps68470-gpio" },
> +	{ .name = "tps68470-clk" },
> +	{ .name = "tps68470-regulator" },
> +};
> +
> +static const struct regmap_config tps68470_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = TPS68470_REG_MAX,
> +};
> +
> +static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
> +{
> +	unsigned int version;
> +	int ret;
> +
> +	/* Force software reset */
> +	ret = regmap_write(regmap, TPS68470_REG_RESET, TPS68470_REG_RESET_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> +	if (ret) {
> +		dev_err(dev, "Failed to read revision register: %d\n", ret);
> +		return ret;
> +	}
> +
> +	dev_info(dev, "TPS68470 REVID: 0x%02x\n", version);
> +
> +	return 0;
> +}
> +
> +/** skl_int3472_tps68470_calc_type: Check what platform a device is designed for
> + * @adev: A pointer to a &struct acpi_device
> + *
> + * Check CLDB buffer against the PMIC's adev. If present, then we check
> + * the value of control_logic_type field and follow one of the
> + * following scenarios:
> + *
> + *	1. No CLDB - likely ACPI tables designed for ChromeOS. We
> + *	create platform devices for the GPIOs and OpRegion drivers.
> + *
> + *	2. CLDB, with control_logic_type = 2 - probably ACPI tables
> + *	made for Windows 2-in-1 platforms. Register pdevs for GPIO,
> + *	Clock and Regulator drivers to bind to.
> + *
> + *	3. Any other value in control_logic_type, we should never have
> + *	gotten to this point; fail probe and return.
> + *
> + * Return:
> + * * 1		Device intended for ChromeOS
> + * * 2		Device intended for Windows
> + * * -EINVAL	Where @adev has an object named CLDB but it does not conform to
> + *		our expectations
> + */
> +int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
> +{
> +	struct int3472_cldb cldb = { 0 };
> +	int ret;
> +
> +	/*
> +	 * A CLDB buffer that exists, but which does not match our expectations
> +	 * should trigger an error so we don't blindly continue.
> +	 */
> +	ret = skl_int3472_fill_cldb(adev, &cldb);
> +	if (ret && ret != -ENODEV)
> +		return ret;
> +
> +	if (ret)
> +		return DESIGNED_FOR_CHROMEOS;
> +
> +	if (cldb.control_logic_type != 2)
> +		return -EINVAL;
> +
> +	return DESIGNED_FOR_WINDOWS;
> +}
> +
> +int skl_int3472_tps68470_probe(struct i2c_client *client)
> +{
> +	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> +	struct regmap *regmap;
> +	int device_type;
> +	int ret;
> +
> +	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
> +		return PTR_ERR(regmap);
> +	}
> +
> +	i2c_set_clientdata(client, regmap);
> +
> +	ret = tps68470_chip_init(&client->dev, regmap);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "TPS68470 init error %d\n", ret);
> +		return ret;
> +	}
> +
> +	device_type = skl_int3472_tps68470_calc_type(adev);
> +	switch (device_type) {
> +	case DESIGNED_FOR_WINDOWS:
> +		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> +					   tps68470_win, ARRAY_SIZE(tps68470_win),
> +					   NULL, 0, NULL);
> +		break;
> +	case DESIGNED_FOR_CHROMEOS:
> +		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> +					   tps68470_cros, ARRAY_SIZE(tps68470_cros),
> +					   NULL, 0, NULL);
> +		break;
> +	default:
> +		dev_err(&client->dev, "Failed to add MFD devices\n");
> +		return device_type;
> +	}
> +
> +	return ret;
> +}
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-16 17:58   ` Hans de Goede
@ 2021-06-16 18:08     ` Andy Shevchenko
  2021-06-16 18:13       ` Hans de Goede
  2021-06-16 21:42     ` Daniel Scally
  1 sibling, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-06-16 18:08 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, Linux Kernel Mailing List,
	ACPI Devel Maling List, open list:GPIO SUBSYSTEM, linux-i2c,
	Platform Driver, open list:ACPI COMPONENT ARCHITECTURE (ACPICA),
	Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	Laurent Pinchart, kieran.bingham

On Wed, Jun 16, 2021 at 8:58 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 6/4/21 12:40 AM, Daniel Scally wrote:
> > ACPI devices with _HID INT3472 are currently matched to the tps68470
> > driver, however this does not cover all situations in which that _HID
> > occurs. We've encountered three possibilities:
> >
> > 1. On Chrome OS devices, an ACPI device with _HID INT3472 (representing
> > a physical TPS68470 device) that requires a GPIO and OpRegion driver
> > 2. On devices designed for Windows, an ACPI device with _HID INT3472
> > (again representing a physical TPS68470 device) which requires GPIO,
> > Clock and Regulator drivers.
> > 3. On other devices designed for Windows, an ACPI device with _HID
> > INT3472 which does **not** represent a physical TPS68470, and is instead
> > used as a dummy device to group some system GPIO lines which are meant
> > to be consumed by the sensor that is dependent on this entry.
> >
> > This commit adds a new module, registering a platform driver to deal
> > with the 3rd scenario plus an i2c driver to deal with #1 and #2, by
> > querying the CLDB buffer found against INT3472 entries to determine
> > which is most appropriate.
> >
> > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Daniel Scally <djrscally@gmail.com>
>
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> I've fixed up the missing static marking of skl_int3472_tps68470_calc_type()
> spotted by lkp@intel.com while applying the patch to my tree.

Are you going to apply patch 6 as well?
IIRC it's acked by Lee.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-16 18:08     ` Andy Shevchenko
@ 2021-06-16 18:13       ` Hans de Goede
  0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-06-16 18:13 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, Linux Kernel Mailing List,
	ACPI Devel Maling List, open list:GPIO SUBSYSTEM, linux-i2c,
	Platform Driver, open list:ACPI COMPONENT ARCHITECTURE (ACPICA),
	Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	Laurent Pinchart, kieran.bingham

Hi,

On 6/16/21 8:08 PM, Andy Shevchenko wrote:
> On Wed, Jun 16, 2021 at 8:58 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 6/4/21 12:40 AM, Daniel Scally wrote:
>>> ACPI devices with _HID INT3472 are currently matched to the tps68470
>>> driver, however this does not cover all situations in which that _HID
>>> occurs. We've encountered three possibilities:
>>>
>>> 1. On Chrome OS devices, an ACPI device with _HID INT3472 (representing
>>> a physical TPS68470 device) that requires a GPIO and OpRegion driver
>>> 2. On devices designed for Windows, an ACPI device with _HID INT3472
>>> (again representing a physical TPS68470 device) which requires GPIO,
>>> Clock and Regulator drivers.
>>> 3. On other devices designed for Windows, an ACPI device with _HID
>>> INT3472 which does **not** represent a physical TPS68470, and is instead
>>> used as a dummy device to group some system GPIO lines which are meant
>>> to be consumed by the sensor that is dependent on this entry.
>>>
>>> This commit adds a new module, registering a platform driver to deal
>>> with the 3rd scenario plus an i2c driver to deal with #1 and #2, by
>>> querying the CLDB buffer found against INT3472 entries to determine
>>> which is most appropriate.
>>>
>>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>>
>> Thank you for your patch, I've applied this patch to my review-hans
>> branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>>
>> I've fixed up the missing static marking of skl_int3472_tps68470_calc_type()
>> spotted by lkp@intel.com while applying the patch to my tree.
> 
> Are you going to apply patch 6 as well?
> IIRC it's acked by Lee.

Ah you are right, it is Acked, but I'm not sure if that ack was for
Lee being ok with it being merged through the pdx86 tree. I'll coordinate
this with Lee off list.

Regards,

Hans


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver
  2021-06-16 17:58   ` Hans de Goede
  2021-06-16 18:08     ` Andy Shevchenko
@ 2021-06-16 21:42     ` Daniel Scally
  1 sibling, 0 replies; 24+ messages in thread
From: Daniel Scally @ 2021-06-16 21:42 UTC (permalink / raw)
  To: Hans de Goede, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham

Hi Hans

On 16/06/2021 18:58, Hans de Goede wrote:
> Hi,
>
> On 6/4/21 12:40 AM, Daniel Scally wrote:
>> ACPI devices with _HID INT3472 are currently matched to the tps68470
>> driver, however this does not cover all situations in which that _HID
>> occurs. We've encountered three possibilities:
>>
>> 1. On Chrome OS devices, an ACPI device with _HID INT3472 (representing
>> a physical TPS68470 device) that requires a GPIO and OpRegion driver
>> 2. On devices designed for Windows, an ACPI device with _HID INT3472
>> (again representing a physical TPS68470 device) which requires GPIO,
>> Clock and Regulator drivers.
>> 3. On other devices designed for Windows, an ACPI device with _HID
>> INT3472 which does **not** represent a physical TPS68470, and is instead
>> used as a dummy device to group some system GPIO lines which are meant
>> to be consumed by the sensor that is dependent on this entry.
>>
>> This commit adds a new module, registering a platform driver to deal
>> with the 3rd scenario plus an i2c driver to deal with #1 and #2, by
>> querying the CLDB buffer found against INT3472 entries to determine
>> which is most appropriate.
>>
>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> I've fixed up the missing static marking of skl_int3472_tps68470_calc_type()
> spotted by lkp@intel.com while applying the patch to my tree.



Fantastic, thanks very much.


>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.


Excellent :)


>
> Regards,
>
> Hans
>
>
>
>
>> ---
>> Changes since v4:
>> 	- Some formatting and commenting tidy up
>> 	- Cleared up some function exit paths to avoid gotos
>> 	- Refactored skl_int3472_get_sensor_module_config() and also
>> 	skl_int3472_tps68470_probe(), which involved adding a new function.
>>
>> Andy - I didn't look at the reg-fixed-voltage suggestion yet, but I think I
>> covered the rest of your suggestions from v4.
>>
>>  MAINTAINERS                                   |   5 +
>>  drivers/platform/x86/Kconfig                  |   2 +
>>  drivers/platform/x86/Makefile                 |   1 +
>>  drivers/platform/x86/intel-int3472/Kconfig    |  30 ++
>>  drivers/platform/x86/intel-int3472/Makefile   |   5 +
>>  .../intel_skl_int3472_clk_and_regulator.c     | 196 ++++++++
>>  .../intel-int3472/intel_skl_int3472_common.c  | 106 +++++
>>  .../intel-int3472/intel_skl_int3472_common.h  | 118 +++++
>>  .../intel_skl_int3472_discrete.c              | 417 ++++++++++++++++++
>>  .../intel_skl_int3472_tps68470.c              | 137 ++++++
>>  10 files changed, 1017 insertions(+)
>>  create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
>>  create mode 100644 drivers/platform/x86/intel-int3472/Makefile
>>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
>>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
>>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
>>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
>>  create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 008fcad7ac00..8dcc7e005ace 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9378,6 +9378,11 @@ S:	Maintained
>>  F:	arch/x86/include/asm/intel_scu_ipc.h
>>  F:	drivers/platform/x86/intel_scu_*
>>  
>> +INTEL SKYLAKE INT3472 ACPI DEVICE DRIVER
>> +M:	Daniel Scally <djrscally@gmail.com>
>> +S:	Maintained
>> +F:	drivers/platform/x86/intel-int3472/
>> +
>>  INTEL SPEED SELECT TECHNOLOGY
>>  M:	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>  L:	platform-driver-x86@vger.kernel.org
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 2714f7c3843e..1263b8efcd50 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -697,6 +697,8 @@ config INTEL_CHT_INT33FE
>>  	  device and CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m
>>  	  for Type-C device.
>>  
>> +source "drivers/platform/x86/intel-int3472/Kconfig"
>> +
>>  config INTEL_HID_EVENT
>>  	tristate "INTEL HID Event"
>>  	depends on ACPI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index dcc8cdb95b4d..c0612c02d037 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -76,6 +76,7 @@ obj-$(CONFIG_INTEL_HID_EVENT)		+= intel-hid.o
>>  obj-$(CONFIG_INTEL_INT0002_VGPIO)	+= intel_int0002_vgpio.o
>>  obj-$(CONFIG_INTEL_MENLOW)		+= intel_menlow.o
>>  obj-$(CONFIG_INTEL_OAKTRAIL)		+= intel_oaktrail.o
>> +obj-$(CONFIG_INTEL_SKL_INT3472)		+= intel-int3472/
>>  obj-$(CONFIG_INTEL_VBTN)		+= intel-vbtn.o
>>  
>>  # MSI
>> diff --git a/drivers/platform/x86/intel-int3472/Kconfig b/drivers/platform/x86/intel-int3472/Kconfig
>> new file mode 100644
>> index 000000000000..c112878e833b
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/Kconfig
>> @@ -0,0 +1,30 @@
>> +config INTEL_SKL_INT3472
>> +	tristate "Intel SkyLake ACPI INT3472 Driver"
>> +	depends on ACPI
>> +	depends on COMMON_CLK && CLKDEV_LOOKUP
>> +	depends on I2C
>> +	depends on GPIOLIB
>> +	depends on REGULATOR
>> +	select MFD_CORE
>> +	select REGMAP_I2C
>> +	help
>> +	  This driver adds power controller support for the Intel SkyCam
>> +	  devices found on the Intel SkyLake platforms.
>> +
>> +	  The INT3472 is a camera power controller, a logical device found on
>> +	  Intel Skylake-based systems that can map to different hardware
>> +	  devices depending on the platform. On machines designed for Chrome OS
>> +	  it maps to a TPS68470 camera PMIC. On machines designed for Windows,
>> +	  it maps to either a TP68470 camera PMIC, a uP6641Q sensor PMIC, or a
>> +	  set of discrete GPIOs and power gates.
>> +
>> +	  If your device was designed for Chrome OS, this driver will provide
>> +	  an ACPI OpRegion, which must be available before any of the devices
>> +	  using it are probed. For this reason, you should select Y if your
>> +	  device was designed for ChromeOS. For the same reason the
>> +	  I2C_DESIGNWARE_PLATFORM option must be set to Y too.
>> +
>> +	  Say Y or M here if you have a SkyLake device designed for use
>> +	  with Windows or ChromeOS. Say N here if you are not sure.
>> +
>> +	  The module will be named "intel-skl-int3472".
>> diff --git a/drivers/platform/x86/intel-int3472/Makefile b/drivers/platform/x86/intel-int3472/Makefile
>> new file mode 100644
>> index 000000000000..48bd97f0a04e
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/Makefile
>> @@ -0,0 +1,5 @@
>> +obj-$(CONFIG_INTEL_SKL_INT3472)		+= intel_skl_int3472.o
>> +intel_skl_int3472-objs			:= intel_skl_int3472_common.o \
>> +					   intel_skl_int3472_discrete.o \
>> +					   intel_skl_int3472_tps68470.o \
>> +					   intel_skl_int3472_clk_and_regulator.o
>> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
>> new file mode 100644
>> index 000000000000..ceee860e2c07
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_clk_and_regulator.c
>> @@ -0,0 +1,196 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Author: Dan Scally <djrscally@gmail.com> */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/device.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/slab.h>
>> +
>> +#include "intel_skl_int3472_common.h"
>> +
>> +/*
>> + * The regulators have to have .ops to be valid, but the only ops we actually
>> + * support are .enable and .disable which are handled via .ena_gpiod. Pass an
>> + * empty struct to clear the check without lying about capabilities.
>> + */
>> +static const struct regulator_ops int3472_gpio_regulator_ops;
>> +
>> +static int skl_int3472_clk_prepare(struct clk_hw *hw)
>> +{
>> +	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
>> +
>> +	gpiod_set_value_cansleep(clk->ena_gpio, 1);
>> +	gpiod_set_value_cansleep(clk->led_gpio, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static void skl_int3472_clk_unprepare(struct clk_hw *hw)
>> +{
>> +	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
>> +
>> +	gpiod_set_value_cansleep(clk->ena_gpio, 0);
>> +	gpiod_set_value_cansleep(clk->led_gpio, 0);
>> +}
>> +
>> +static int skl_int3472_clk_enable(struct clk_hw *hw)
>> +{
>> +	/*
>> +	 * We're just turning a GPIO on to enable the clock, which operation
>> +	 * has the potential to sleep. Given .enable() cannot sleep, but
>> +	 * .prepare() can, we toggle the GPIO in .prepare() instead. Thus,
>> +	 * nothing to do here.
>> +	 */
>> +	return 0;
>> +}
>> +
>> +static void skl_int3472_clk_disable(struct clk_hw *hw)
>> +{
>> +	/* Likewise, nothing to do here... */
>> +}
>> +
>> +static unsigned int skl_int3472_get_clk_frequency(struct int3472_discrete_device *int3472)
>> +{
>> +	union acpi_object *obj;
>> +	unsigned int freq;
>> +
>> +	obj = skl_int3472_get_acpi_buffer(int3472->sensor, "SSDB");
>> +	if (IS_ERR(obj))
>> +		return 0; /* report rate as 0 on error */
>> +
>> +	if (obj->buffer.length < CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET + sizeof(u32)) {
>> +		dev_err(int3472->dev, "The buffer is too small\n");
>> +		kfree(obj);
>> +		return 0;
>> +	}
>> +
>> +	freq = *(u32 *)(obj->buffer.pointer + CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET);
>> +
>> +	kfree(obj);
>> +	return freq;
>> +}
>> +
>> +static unsigned long skl_int3472_clk_recalc_rate(struct clk_hw *hw,
>> +						 unsigned long parent_rate)
>> +{
>> +	struct int3472_gpio_clock *clk = to_int3472_clk(hw);
>> +
>> +	return clk->frequency;
>> +}
>> +
>> +static const struct clk_ops skl_int3472_clock_ops = {
>> +	.prepare = skl_int3472_clk_prepare,
>> +	.unprepare = skl_int3472_clk_unprepare,
>> +	.enable = skl_int3472_clk_enable,
>> +	.disable = skl_int3472_clk_disable,
>> +	.recalc_rate = skl_int3472_clk_recalc_rate,
>> +};
>> +
>> +int skl_int3472_register_clock(struct int3472_discrete_device *int3472)
>> +{
>> +	struct clk_init_data init = {
>> +		.ops = &skl_int3472_clock_ops,
>> +		.flags = CLK_GET_RATE_NOCACHE,
>> +	};
>> +	int ret;
>> +
>> +	init.name = kasprintf(GFP_KERNEL, "%s-clk",
>> +			      acpi_dev_name(int3472->adev));
>> +	if (!init.name)
>> +		return -ENOMEM;
>> +
>> +	int3472->clock.frequency = skl_int3472_get_clk_frequency(int3472);
>> +
>> +	int3472->clock.clk_hw.init = &init;
>> +	int3472->clock.clk = clk_register(&int3472->adev->dev,
>> +					  &int3472->clock.clk_hw);
>> +	if (IS_ERR(int3472->clock.clk)) {
>> +		ret = PTR_ERR(int3472->clock.clk);
>> +		goto out_free_init_name;
>> +	}
>> +
>> +	int3472->clock.cl = clkdev_create(int3472->clock.clk, NULL,
>> +					  int3472->sensor_name);
>> +	if (!int3472->clock.cl) {
>> +		ret = -ENOMEM;
>> +		goto err_unregister_clk;
>> +	}
>> +
>> +	kfree(init.name);
>> +	return 0;
>> +
>> +err_unregister_clk:
>> +	clk_unregister(int3472->clock.clk);
>> +out_free_init_name:
>> +	kfree(init.name);
>> +
>> +	return ret;
>> +}
>> +
>> +int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
>> +				   struct acpi_resource *ares)
>> +{
>> +	char *path = ares->data.gpio.resource_source.string_ptr;
>> +	const struct int3472_sensor_config *sensor_config;
>> +	struct regulator_consumer_supply supply_map;
>> +	struct regulator_init_data init_data = { };
>> +	struct regulator_config cfg = { };
>> +	int ret;
>> +
>> +	sensor_config = int3472->sensor_config;
>> +	if (IS_ERR(sensor_config)) {
>> +		dev_err(int3472->dev, "No sensor module config\n");
>> +		return PTR_ERR(sensor_config);
>> +	}
>> +
>> +	if (!sensor_config->supply_map.supply) {
>> +		dev_err(int3472->dev, "No supply name defined\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	init_data.constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
>> +	init_data.num_consumer_supplies = 1;
>> +	supply_map = sensor_config->supply_map;
>> +	supply_map.dev_name = int3472->sensor_name;
>> +	init_data.consumer_supplies = &supply_map;
>> +
>> +	snprintf(int3472->regulator.regulator_name,
>> +		 sizeof(int3472->regulator.regulator_name), "%s-regulator",
>> +		 acpi_dev_name(int3472->adev));
>> +	snprintf(int3472->regulator.supply_name,
>> +		 GPIO_REGULATOR_SUPPLY_NAME_LENGTH, "supply-0");
>> +
>> +	int3472->regulator.rdesc = INT3472_REGULATOR(
>> +						int3472->regulator.regulator_name,
>> +						int3472->regulator.supply_name,
>> +						&int3472_gpio_regulator_ops);
>> +
>> +	int3472->regulator.gpio = acpi_get_and_request_gpiod(path,
>> +							     ares->data.gpio.pin_table[0],
>> +							     "int3472,regulator");
>> +	if (IS_ERR(int3472->regulator.gpio)) {
>> +		dev_err(int3472->dev, "Failed to get regulator GPIO line\n");
>> +		return PTR_ERR(int3472->regulator.gpio);
>> +	}
>> +
>> +	cfg.dev = &int3472->adev->dev;
>> +	cfg.init_data = &init_data;
>> +	cfg.ena_gpiod = int3472->regulator.gpio;
>> +
>> +	int3472->regulator.rdev = regulator_register(&int3472->regulator.rdesc,
>> +						     &cfg);
>> +	if (IS_ERR(int3472->regulator.rdev)) {
>> +		ret = PTR_ERR(int3472->regulator.rdev);
>> +		goto err_free_gpio;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_free_gpio:
>> +	gpiod_put(int3472->regulator.gpio);
>> +
>> +	return ret;
>> +}
>> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
>> new file mode 100644
>> index 000000000000..497e74fba75f
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
>> @@ -0,0 +1,106 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Author: Dan Scally <djrscally@gmail.com> */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/i2c.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +#include "intel_skl_int3472_common.h"
>> +
>> +union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev, char *id)
>> +{
>> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	acpi_handle handle = adev->handle;
>> +	union acpi_object *obj;
>> +	acpi_status status;
>> +
>> +	status = acpi_evaluate_object(handle, id, NULL, &buffer);
>> +	if (ACPI_FAILURE(status))
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	obj = buffer.pointer;
>> +	if (!obj)
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	if (obj->type != ACPI_TYPE_BUFFER) {
>> +		acpi_handle_err(handle, "%s object is not an ACPI buffer\n", id);
>> +		kfree(obj);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	return obj;
>> +}
>> +
>> +int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb)
>> +{
>> +	union acpi_object *obj;
>> +	int ret;
>> +
>> +	obj = skl_int3472_get_acpi_buffer(adev, "CLDB");
>> +	if (IS_ERR(obj))
>> +		return PTR_ERR(obj);
>> +
>> +	if (obj->buffer.length > sizeof(*cldb)) {
>> +		acpi_handle_err(adev->handle, "The CLDB buffer is too large\n");
>> +		ret = -EINVAL;
>> +		goto out_free_obj;
>> +	}
>> +
>> +	memcpy(cldb, obj->buffer.pointer, obj->buffer.length);
>> +	ret = 0;
>> +
>> +out_free_obj:
>> +	kfree(obj);
>> +	return ret;
>> +}
>> +
>> +static const struct acpi_device_id int3472_device_id[] = {
>> +	{ "INT3472", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, int3472_device_id);
>> +
>> +static struct platform_driver int3472_discrete = {
>> +	.driver = {
>> +		.name = "int3472-discrete",
>> +		.acpi_match_table = int3472_device_id,
>> +	},
>> +	.probe = skl_int3472_discrete_probe,
>> +	.remove = skl_int3472_discrete_remove,
>> +};
>> +
>> +static struct i2c_driver int3472_tps68470 = {
>> +	.driver = {
>> +		.name = "int3472-tps68470",
>> +		.acpi_match_table = int3472_device_id,
>> +	},
>> +	.probe_new = skl_int3472_tps68470_probe,
>> +};
>> +
>> +static int skl_int3472_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = platform_driver_register(&int3472_discrete);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = i2c_register_driver(THIS_MODULE, &int3472_tps68470);
>> +	if (ret)
>> +		platform_driver_unregister(&int3472_discrete);
>> +
>> +	return ret;
>> +}
>> +module_init(skl_int3472_init);
>> +
>> +static void skl_int3472_exit(void)
>> +{
>> +	platform_driver_unregister(&int3472_discrete);
>> +	i2c_del_driver(&int3472_tps68470);
>> +}
>> +module_exit(skl_int3472_exit);
>> +
>> +MODULE_DESCRIPTION("Intel SkyLake INT3472 ACPI Device Driver");
>> +MODULE_AUTHOR("Daniel Scally <djrscally@gmail.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
>> new file mode 100644
>> index 000000000000..6fdf78584219
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
>> @@ -0,0 +1,118 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Author: Dan Scally <djrscally@gmail.com> */
>> +
>> +#ifndef _INTEL_SKL_INT3472_H
>> +#define _INTEL_SKL_INT3472_H
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/gpio/machine.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/machine.h>
>> +#include <linux/types.h>
>> +
>> +/* FIXME drop this once the I2C_DEV_NAME_FORMAT macro has been added to include/linux/i2c.h */
>> +#ifndef I2C_DEV_NAME_FORMAT
>> +#define I2C_DEV_NAME_FORMAT					"i2c-%s"
>> +#endif
>> +
>> +/* PMIC GPIO Types */
>> +#define INT3472_GPIO_TYPE_RESET					0x00
>> +#define INT3472_GPIO_TYPE_POWERDOWN				0x01
>> +#define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
>> +#define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
>> +#define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
>> +
>> +#define INT3472_PDEV_MAX_NAME_LEN				23
>> +#define INT3472_MAX_SENSOR_GPIOS				3
>> +
>> +#define GPIO_REGULATOR_NAME_LENGTH				21
>> +#define GPIO_REGULATOR_SUPPLY_NAME_LENGTH			9
>> +
>> +#define CIO2_SENSOR_SSDB_MCLKSPEED_OFFSET			86
>> +
>> +#define INT3472_REGULATOR(_name, _supply, _ops)			\
>> +	(const struct regulator_desc) {				\
>> +		.name = _name,					\
>> +		.supply_name = _supply,				\
>> +		.type = REGULATOR_VOLTAGE,			\
>> +		.ops = _ops,					\
>> +		.owner = THIS_MODULE,				\
>> +	}
>> +
>> +#define to_int3472_clk(hw)					\
>> +	container_of(hw, struct int3472_gpio_clock, clk_hw)
>> +
>> +#define to_int3472_device(clk)					\
>> +	container_of(clk, struct int3472_discrete_device, clock)
>> +
>> +struct acpi_device;
>> +struct i2c_client;
>> +struct platform_device;
>> +
>> +struct int3472_cldb {
>> +	u8 version;
>> +	/*
>> +	 * control logic type
>> +	 * 0: UNKNOWN
>> +	 * 1: DISCRETE(CRD-D)
>> +	 * 2: PMIC TPS68470
>> +	 * 3: PMIC uP6641
>> +	 */
>> +	u8 control_logic_type;
>> +	u8 control_logic_id;
>> +	u8 sensor_card_sku;
>> +	u8 reserved[28];
>> +};
>> +
>> +struct int3472_gpio_function_remap {
>> +	const char *documented;
>> +	const char *actual;
>> +};
>> +
>> +struct int3472_sensor_config {
>> +	const char *sensor_module_name;
>> +	struct regulator_consumer_supply supply_map;
>> +	const struct int3472_gpio_function_remap *function_maps;
>> +};
>> +
>> +struct int3472_discrete_device {
>> +	struct acpi_device *adev;
>> +	struct device *dev;
>> +	struct acpi_device *sensor;
>> +	const char *sensor_name;
>> +
>> +	const struct int3472_sensor_config *sensor_config;
>> +
>> +	struct int3472_gpio_regulator {
>> +		char regulator_name[GPIO_REGULATOR_NAME_LENGTH];
>> +		char supply_name[GPIO_REGULATOR_SUPPLY_NAME_LENGTH];
>> +		struct gpio_desc *gpio;
>> +		struct regulator_dev *rdev;
>> +		struct regulator_desc rdesc;
>> +	} regulator;
>> +
>> +	struct int3472_gpio_clock {
>> +		struct clk *clk;
>> +		struct clk_hw clk_hw;
>> +		struct clk_lookup *cl;
>> +		struct gpio_desc *ena_gpio;
>> +		struct gpio_desc *led_gpio;
>> +		u32 frequency;
>> +	} clock;
>> +
>> +	unsigned int ngpios; /* how many GPIOs have we seen */
>> +	unsigned int n_sensor_gpios; /* how many have we mapped to sensor */
>> +	struct gpiod_lookup_table gpios;
>> +};
>> +
>> +int skl_int3472_discrete_probe(struct platform_device *pdev);
>> +int skl_int3472_discrete_remove(struct platform_device *pdev);
>> +int skl_int3472_tps68470_probe(struct i2c_client *client);
>> +union acpi_object *skl_int3472_get_acpi_buffer(struct acpi_device *adev,
>> +					       char *id);
>> +int skl_int3472_fill_cldb(struct acpi_device *adev, struct int3472_cldb *cldb);
>> +int skl_int3472_register_clock(struct int3472_discrete_device *int3472);
>> +int skl_int3472_register_regulator(struct int3472_discrete_device *int3472,
>> +				   struct acpi_resource *ares);
>> +
>> +#endif
>> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
>> new file mode 100644
>> index 000000000000..8c18dbff1c43
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
>> @@ -0,0 +1,417 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Author: Dan Scally <djrscally@gmail.com> */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/device.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/gpio/machine.h>
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/overflow.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/uuid.h>
>> +
>> +#include "intel_skl_int3472_common.h"
>> +
>> +/*
>> + * 79234640-9e10-4fea-a5c1-b5aa8b19756f
>> + * This _DSM GUID returns information about the GPIO lines mapped to a
>> + * discrete INT3472 device. Function number 1 returns a count of the GPIO
>> + * lines that are mapped. Subsequent functions return 32 bit ints encoding
>> + * information about the GPIO line, including its purpose.
>> + */
>> +static const guid_t int3472_gpio_guid =
>> +	GUID_INIT(0x79234640, 0x9e10, 0x4fea,
>> +		  0xa5, 0xc1, 0xb5, 0xaa, 0x8b, 0x19, 0x75, 0x6f);
>> +
>> +/*
>> + * 822ace8f-2814-4174-a56b-5f029fe079ee
>> + * This _DSM GUID returns a string from the sensor device, which acts as a
>> + * module identifier.
>> + */
>> +static const guid_t cio2_sensor_module_guid =
>> +	GUID_INIT(0x822ace8f, 0x2814, 0x4174,
>> +		  0xa5, 0x6b, 0x5f, 0x02, 0x9f, 0xe0, 0x79, 0xee);
>> +
>> +/*
>> + * Here follows platform specific mapping information that we can pass to
>> + * the functions mapping resources to the sensors. Where the sensors have
>> + * a power enable pin defined in DSDT we need to provide a supply name so
>> + * the sensor drivers can find the regulator. The device name will be derived
>> + * from the sensor's ACPI device within the code. Optionally, we can provide a
>> + * NULL terminated array of function name mappings to deal with any platform
>> + * specific deviations from the documented behaviour of GPIOs.
>> + *
>> + * Map a GPIO function name to NULL to prevent the driver from mapping that
>> + * GPIO at all.
>> + */
>> +
>> +static const struct int3472_gpio_function_remap ov2680_gpio_function_remaps[] = {
>> +	{ "reset", NULL },
>> +	{ "powerdown", "reset" },
>> +	{ }
>> +};
>> +
>> +static const struct int3472_sensor_config int3472_sensor_configs[] = {
>> +	/* Lenovo Miix 510-12ISK - OV2680, Front */
>> +	{ "GNDF140809R", { 0 }, ov2680_gpio_function_remaps },
>> +	/* Lenovo Miix 510-12ISK - OV5648, Rear */
>> +	{ "GEFF150023R", REGULATOR_SUPPLY("avdd", NULL), NULL },
>> +	/* Surface Go 1&2 - OV5693, Front */
>> +	{ "YHCU", REGULATOR_SUPPLY("avdd", NULL), NULL },
>> +};
>> +
>> +static const struct int3472_sensor_config *
>> +skl_int3472_get_sensor_module_config(struct int3472_discrete_device *int3472)
>> +{
>> +	union acpi_object *obj;
>> +	unsigned int i;
>> +
>> +	obj = acpi_evaluate_dsm_typed(int3472->sensor->handle,
>> +				      &cio2_sensor_module_guid, 0x00,
>> +				      0x01, NULL, ACPI_TYPE_STRING);
>> +
>> +	if (!obj) {
>> +		dev_err(int3472->dev,
>> +			"Failed to get sensor module string from _DSM\n");
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	if (obj->string.type != ACPI_TYPE_STRING) {
>> +		dev_err(int3472->dev,
>> +			"Sensor _DSM returned a non-string value\n");
>> +
>> +		ACPI_FREE(obj);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	for (i = 0; i < ARRAY_SIZE(int3472_sensor_configs); i++) {
>> +		if (!strcmp(int3472_sensor_configs[i].sensor_module_name,
>> +			    obj->string.pointer))
>> +			break;
>> +	}
>> +
>> +	ACPI_FREE(obj);
>> +
>> +	if (i >= ARRAY_SIZE(int3472_sensor_configs))
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	return &int3472_sensor_configs[i];
>> +}
>> +
>> +static int skl_int3472_map_gpio_to_sensor(struct int3472_discrete_device *int3472,
>> +					  struct acpi_resource *ares,
>> +					  const char *func, u32 polarity)
>> +{
>> +	char *path = ares->data.gpio.resource_source.string_ptr;
>> +	const struct int3472_sensor_config *sensor_config;
>> +	struct gpiod_lookup *table_entry;
>> +	struct acpi_device *adev;
>> +	acpi_handle handle;
>> +	acpi_status status;
>> +	int ret;
>> +
>> +	if (int3472->n_sensor_gpios >= INT3472_MAX_SENSOR_GPIOS) {
>> +		dev_warn(int3472->dev, "Too many GPIOs mapped\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	sensor_config = int3472->sensor_config;
>> +	if (!IS_ERR(sensor_config) && sensor_config->function_maps) {
>> +		const struct int3472_gpio_function_remap *remap;
>> +
>> +		for (remap = sensor_config->function_maps; remap->documented; remap++) {
>> +			if (!strcmp(func, remap->documented)) {
>> +				func = remap->actual;
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>> +	/* Functions mapped to NULL should not be mapped to the sensor */
>> +	if (!func)
>> +		return 0;
>> +
>> +	status = acpi_get_handle(NULL, path, &handle);
>> +	if (ACPI_FAILURE(status))
>> +		return -EINVAL;
>> +
>> +	ret = acpi_bus_get_device(handle, &adev);
>> +	if (ret)
>> +		return -ENODEV;
>> +
>> +	table_entry = &int3472->gpios.table[int3472->n_sensor_gpios];
>> +	table_entry->key = acpi_dev_name(adev);
>> +	table_entry->chip_hwnum = ares->data.gpio.pin_table[0];
>> +	table_entry->con_id = func;
>> +	table_entry->idx = 0;
>> +	table_entry->flags = polarity;
>> +
>> +	int3472->n_sensor_gpios++;
>> +
>> +	return 0;
>> +}
>> +
>> +static int skl_int3472_map_gpio_to_clk(struct int3472_discrete_device *int3472,
>> +				       struct acpi_resource *ares, u8 type)
>> +{
>> +	char *path = ares->data.gpio.resource_source.string_ptr;
>> +	struct gpio_desc *gpio;
>> +
>> +	switch (type) {
>> +	case INT3472_GPIO_TYPE_CLK_ENABLE:
>> +		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
>> +						  "int3472,clk-enable");
>> +		if (IS_ERR(gpio))
>> +			return (PTR_ERR(gpio));
>> +
>> +		int3472->clock.ena_gpio = gpio;
>> +		break;
>> +	case INT3472_GPIO_TYPE_PRIVACY_LED:
>> +		gpio = acpi_get_and_request_gpiod(path, ares->data.gpio.pin_table[0],
>> +						  "int3472,privacy-led");
>> +		if (IS_ERR(gpio))
>> +			return (PTR_ERR(gpio));
>> +
>> +		int3472->clock.led_gpio = gpio;
>> +		break;
>> +	default:
>> +		dev_err(int3472->dev, "Invalid GPIO type 0x%02x for clock\n", type);
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * skl_int3472_handle_gpio_resources: Map PMIC resources to consuming sensor
>> + * @ares: A pointer to a &struct acpi_resource
>> + * @data: A pointer to a &struct int3472_discrete_device
>> + *
>> + * This function handles GPIO resources that are against an INT3472
>> + * ACPI device, by checking the value of the corresponding _DSM entry.
>> + * This will return a 32bit int, where the lowest byte represents the
>> + * function of the GPIO pin:
>> + *
>> + * 0x00 Reset
>> + * 0x01 Power down
>> + * 0x0b Power enable
>> + * 0x0c Clock enable
>> + * 0x0d Privacy LED
>> + *
>> + * There are some known platform specific quirks where that does not quite
>> + * hold up; for example where a pin with type 0x01 (Power down) is mapped to
>> + * a sensor pin that performs a reset function or entries in _CRS and _DSM that
>> + * do not actually correspond to a physical connection. These will be handled
>> + * by the mapping sub-functions.
>> + *
>> + * GPIOs will either be mapped directly to the sensor device or else used
>> + * to create clocks and regulators via the usual frameworks.
>> + *
>> + * Return:
>> + * * 1		- To continue the loop
>> + * * 0		- When all resources found are handled properly.
>> + * * -EINVAL	- If the resource is not a GPIO IO resource
>> + * * -ENODEV	- If the resource has no corresponding _DSM entry
>> + * * -Other	- Errors propagated from one of the sub-functions.
>> + */
>> +static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>> +					     void *data)
>> +{
>> +	struct int3472_discrete_device *int3472 = data;
>> +	struct acpi_resource_gpio *agpio;
>> +	union acpi_object *obj;
>> +	const char *err_msg;
>> +	int ret;
>> +	u8 type;
>> +
>> +	if (!acpi_gpio_get_io_resource(ares, &agpio))
>> +		return 1;
>> +
>> +	/*
>> +	 * ngpios + 2 because the index of this _DSM function is 1-based and
>> +	 * the first function is just a count.
>> +	 */
>> +	obj = acpi_evaluate_dsm_typed(int3472->adev->handle,
>> +				      &int3472_gpio_guid, 0x00,
>> +				      int3472->ngpios + 2,
>> +				      NULL, ACPI_TYPE_INTEGER);
>> +
>> +	if (!obj) {
>> +		dev_warn(int3472->dev, "No _DSM entry for GPIO pin %u\n",
>> +			 ares->data.gpio.pin_table[0]);
>> +		return 1;
>> +	}
>> +
>> +	type = obj->integer.value & 0xff;
>> +
>> +	switch (type) {
>> +	case INT3472_GPIO_TYPE_RESET:
>> +		ret = skl_int3472_map_gpio_to_sensor(int3472, ares, "reset",
>> +						     GPIO_ACTIVE_LOW);
>> +		if (ret)
>> +			err_msg = "Failed to map reset pin to sensor\n";
>> +
>> +		break;
>> +	case INT3472_GPIO_TYPE_POWERDOWN:
>> +		ret = skl_int3472_map_gpio_to_sensor(int3472, ares,
>> +						     "powerdown",
>> +						     GPIO_ACTIVE_LOW);
>> +		if (ret)
>> +			err_msg = "Failed to map powerdown pin to sensor\n";
>> +
>> +		break;
>> +	case INT3472_GPIO_TYPE_CLK_ENABLE:
>> +	case INT3472_GPIO_TYPE_PRIVACY_LED:
>> +		ret = skl_int3472_map_gpio_to_clk(int3472, ares, type);
>> +		if (ret)
>> +			err_msg = "Failed to map GPIO to clock\n";
>> +
>> +		break;
>> +	case INT3472_GPIO_TYPE_POWER_ENABLE:
>> +		ret = skl_int3472_register_regulator(int3472, ares);
>> +		if (ret)
>> +			err_msg = "Failed to map regulator to sensor\n";
>> +
>> +		break;
>> +	default:
>> +		dev_warn(int3472->dev,
>> +			 "GPIO type 0x%02x unknown; the sensor may not work\n",
>> +			 type);
>> +		ret = 1;
>> +		break;
>> +	}
>> +
>> +	int3472->ngpios++;
>> +	ACPI_FREE(obj);
>> +
>> +	if (ret)
>> +		return dev_err_probe(int3472->dev, ret, err_msg);
>> +
>> +	return 0;
>> +}
>> +
>> +static int skl_int3472_parse_crs(struct int3472_discrete_device *int3472)
>> +{
>> +	LIST_HEAD(resource_list);
>> +	int ret;
>> +
>> +	/*
>> +	 * No error check, because not having a sensor config is not necessarily
>> +	 * a failure mode.
>> +	 */
>> +	int3472->sensor_config = skl_int3472_get_sensor_module_config(int3472);
>> +
>> +	ret = acpi_dev_get_resources(int3472->adev, &resource_list,
>> +				     skl_int3472_handle_gpio_resources,
>> +				     int3472);
>> +	if (ret)
>> +		goto out_free_res_list;
>> +
>> +	/*
>> +	 * If we find no clock enable GPIO pin then the privacy LED won't work.
>> +	 * We've never seen that situation, but it's possible. Warn the user so
>> +	 * it's clear what's happened.
>> +	 */
>> +	if (int3472->clock.ena_gpio) {
>> +		ret = skl_int3472_register_clock(int3472);
>> +		if (ret)
>> +			goto out_free_res_list;
>> +	} else {
>> +		if (int3472->clock.led_gpio)
>> +			dev_warn(int3472->dev,
>> +				 "No clk GPIO. The privacy LED won't work\n");
>> +	}
>> +
>> +	int3472->gpios.dev_id = int3472->sensor_name;
>> +	gpiod_add_lookup_table(&int3472->gpios);
>> +
>> +out_free_res_list:
>> +	acpi_dev_free_resource_list(&resource_list);
>> +
>> +	return ret;
>> +}
>> +
>> +int skl_int3472_discrete_probe(struct platform_device *pdev)
>> +{
>> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>> +	struct int3472_discrete_device *int3472;
>> +	struct int3472_cldb cldb;
>> +	int ret;
>> +
>> +	ret = skl_int3472_fill_cldb(adev, &cldb);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't fill CLDB structure\n");
>> +		return ret;
>> +	}
>> +
>> +	if (cldb.control_logic_type != 1) {
>> +		dev_err(&pdev->dev, "Unsupported control logic type %u\n",
>> +			cldb.control_logic_type);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Max num GPIOs we've seen plus a terminator */
>> +	int3472 = devm_kzalloc(&pdev->dev, struct_size(int3472, gpios.table,
>> +			       INT3472_MAX_SENSOR_GPIOS + 1), GFP_KERNEL);
>> +	if (!int3472)
>> +		return -ENOMEM;
>> +
>> +	int3472->adev = adev;
>> +	int3472->dev = &pdev->dev;
>> +	platform_set_drvdata(pdev, int3472);
>> +
>> +	int3472->sensor = acpi_dev_get_first_consumer_dev(adev);
>> +	if (!int3472->sensor) {
>> +		dev_err(&pdev->dev, "INT3472 seems to have no dependents.\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	int3472->sensor_name = devm_kasprintf(int3472->dev, GFP_KERNEL,
>> +					      I2C_DEV_NAME_FORMAT,
>> +					      acpi_dev_name(int3472->sensor));
>> +	if (!int3472->sensor_name) {
>> +		ret = -ENOMEM;
>> +		goto err_put_sensor;
>> +	}
>> +
>> +	/*
>> +	 * Initialising this list means we can call gpiod_remove_lookup_table()
>> +	 * in failure paths without issue.
>> +	 */
>> +	INIT_LIST_HEAD(&int3472->gpios.list);
>> +
>> +	ret = skl_int3472_parse_crs(int3472);
>> +	if (ret) {
>> +		skl_int3472_discrete_remove(pdev);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_put_sensor:
>> +	acpi_dev_put(int3472->sensor);
>> +
>> +	return ret;
>> +}
>> +
>> +int skl_int3472_discrete_remove(struct platform_device *pdev)
>> +{
>> +	struct int3472_discrete_device *int3472 = platform_get_drvdata(pdev);
>> +
>> +	gpiod_remove_lookup_table(&int3472->gpios);
>> +	regulator_unregister(int3472->regulator.rdev);
>> +	clk_unregister(int3472->clock.clk);
>> +
>> +	if (int3472->clock.cl)
>> +		clkdev_drop(int3472->clock.cl);
>> +
>> +	gpiod_put(int3472->regulator.gpio);
>> +	gpiod_put(int3472->clock.ena_gpio);
>> +	gpiod_put(int3472->clock.led_gpio);
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c b/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
>> new file mode 100644
>> index 000000000000..6306ba24e0bc
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c
>> @@ -0,0 +1,137 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Author: Dan Scally <djrscally@gmail.com> */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/mfd/tps68470.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "intel_skl_int3472_common.h"
>> +
>> +#define DESIGNED_FOR_CHROMEOS		1
>> +#define DESIGNED_FOR_WINDOWS		2
>> +
>> +static const struct mfd_cell tps68470_cros[] = {
>> +	{ .name = "tps68470-gpio" },
>> +	{ .name = "tps68470_pmic_opregion" },
>> +};
>> +
>> +static const struct mfd_cell tps68470_win[] = {
>> +	{ .name = "tps68470-gpio" },
>> +	{ .name = "tps68470-clk" },
>> +	{ .name = "tps68470-regulator" },
>> +};
>> +
>> +static const struct regmap_config tps68470_regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.max_register = TPS68470_REG_MAX,
>> +};
>> +
>> +static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
>> +{
>> +	unsigned int version;
>> +	int ret;
>> +
>> +	/* Force software reset */
>> +	ret = regmap_write(regmap, TPS68470_REG_RESET, TPS68470_REG_RESET_MASK);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to read revision register: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	dev_info(dev, "TPS68470 REVID: 0x%02x\n", version);
>> +
>> +	return 0;
>> +}
>> +
>> +/** skl_int3472_tps68470_calc_type: Check what platform a device is designed for
>> + * @adev: A pointer to a &struct acpi_device
>> + *
>> + * Check CLDB buffer against the PMIC's adev. If present, then we check
>> + * the value of control_logic_type field and follow one of the
>> + * following scenarios:
>> + *
>> + *	1. No CLDB - likely ACPI tables designed for ChromeOS. We
>> + *	create platform devices for the GPIOs and OpRegion drivers.
>> + *
>> + *	2. CLDB, with control_logic_type = 2 - probably ACPI tables
>> + *	made for Windows 2-in-1 platforms. Register pdevs for GPIO,
>> + *	Clock and Regulator drivers to bind to.
>> + *
>> + *	3. Any other value in control_logic_type, we should never have
>> + *	gotten to this point; fail probe and return.
>> + *
>> + * Return:
>> + * * 1		Device intended for ChromeOS
>> + * * 2		Device intended for Windows
>> + * * -EINVAL	Where @adev has an object named CLDB but it does not conform to
>> + *		our expectations
>> + */
>> +int skl_int3472_tps68470_calc_type(struct acpi_device *adev)
>> +{
>> +	struct int3472_cldb cldb = { 0 };
>> +	int ret;
>> +
>> +	/*
>> +	 * A CLDB buffer that exists, but which does not match our expectations
>> +	 * should trigger an error so we don't blindly continue.
>> +	 */
>> +	ret = skl_int3472_fill_cldb(adev, &cldb);
>> +	if (ret && ret != -ENODEV)
>> +		return ret;
>> +
>> +	if (ret)
>> +		return DESIGNED_FOR_CHROMEOS;
>> +
>> +	if (cldb.control_logic_type != 2)
>> +		return -EINVAL;
>> +
>> +	return DESIGNED_FOR_WINDOWS;
>> +}
>> +
>> +int skl_int3472_tps68470_probe(struct i2c_client *client)
>> +{
>> +	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
>> +	struct regmap *regmap;
>> +	int device_type;
>> +	int ret;
>> +
>> +	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>> +	if (IS_ERR(regmap)) {
>> +		dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
>> +		return PTR_ERR(regmap);
>> +	}
>> +
>> +	i2c_set_clientdata(client, regmap);
>> +
>> +	ret = tps68470_chip_init(&client->dev, regmap);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "TPS68470 init error %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	device_type = skl_int3472_tps68470_calc_type(adev);
>> +	switch (device_type) {
>> +	case DESIGNED_FOR_WINDOWS:
>> +		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> +					   tps68470_win, ARRAY_SIZE(tps68470_win),
>> +					   NULL, 0, NULL);
>> +		break;
>> +	case DESIGNED_FOR_CHROMEOS:
>> +		ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
>> +					   tps68470_cros, ARRAY_SIZE(tps68470_cros),
>> +					   NULL, 0, NULL);
>> +		break;
>> +	default:
>> +		dev_err(&client->dev, "Failed to add MFD devices\n");
>> +		return device_type;
>> +	}
>> +
>> +	return ret;
>> +}
>>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 6/6] mfd: tps68470: Remove tps68470 MFD driver
  2021-06-03 22:40 ` [PATCH v5 6/6] mfd: tps68470: Remove tps68470 MFD driver Daniel Scally
@ 2021-06-17 11:14   ` Hans de Goede
  0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-06-17 11:14 UTC (permalink / raw)
  To: Daniel Scally, Rafael J . Wysocki, Andy Shevchenko, Lee Jones,
	Maximilian Luz, linux-kernel, linux-acpi, linux-gpio, linux-i2c,
	platform-driver-x86, devel
  Cc: Len Brown, Mika Westerberg, Linus Walleij, Bartosz Golaszewski,
	Wolfram Sang, Mark Gross, Robert Moore, Erik Kaneda,
	laurent.pinchart, kieran.bingham, Andy Shevchenko

Hi,

On 6/4/21 12:40 AM, Daniel Scally wrote:
> This driver only covered one scenario in which ACPI devices with _HID
> INT3472 are found, and its functionality has been taken over by the
> intel-skl-int3472 module, so remove it.
> 
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
> Changes since v4:
> 	- None
> 
>  drivers/acpi/pmic/Kconfig |  2 +-
>  drivers/gpio/Kconfig      |  2 +-
>  drivers/mfd/Kconfig       | 18 --------
>  drivers/mfd/Makefile      |  1 -
>  drivers/mfd/tps68470.c    | 97 ---------------------------------------
>  5 files changed, 2 insertions(+), 118 deletions(-)
>  delete mode 100644 drivers/mfd/tps68470.c
> 
> diff --git a/drivers/acpi/pmic/Kconfig b/drivers/acpi/pmic/Kconfig
> index 56bbcb2ce61b..f84b8f6038dc 100644
> --- a/drivers/acpi/pmic/Kconfig
> +++ b/drivers/acpi/pmic/Kconfig
> @@ -52,7 +52,7 @@ endif	# PMIC_OPREGION
>  
>  config TPS68470_PMIC_OPREGION
>  	bool "ACPI operation region support for TPS68470 PMIC"
> -	depends on MFD_TPS68470
> +	depends on INTEL_SKL_INT3472
>  	help
>  	  This config adds ACPI operation region support for TI TPS68470 PMIC.
>  	  TPS68470 device is an advanced power management unit that powers
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1dd0ec6727fd..10228abeee56 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1367,7 +1367,7 @@ config GPIO_TPS65912
>  
>  config GPIO_TPS68470
>  	bool "TPS68470 GPIO"
> -	depends on MFD_TPS68470
> +	depends on INTEL_SKL_INT3472
>  	help
>  	  Select this option to enable GPIO driver for the TPS68470
>  	  chip family.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 5c7f2b100191..99c4e1a80ae0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1499,24 +1499,6 @@ config MFD_TPS65217
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called tps65217.
>  
> -config MFD_TPS68470
> -	bool "TI TPS68470 Power Management / LED chips"
> -	depends on ACPI && PCI && I2C=y
> -	depends on I2C_DESIGNWARE_PLATFORM=y
> -	select MFD_CORE
> -	select REGMAP_I2C
> -	help
> -	  If you say yes here you get support for the TPS68470 series of
> -	  Power Management / LED chips.
> -
> -	  These include voltage regulators, LEDs and other features
> -	  that are often used in portable devices.
> -
> -	  This option is a bool as it provides an ACPI operation
> -	  region, which must be available before any of the devices
> -	  using this are probed. This option also configures the
> -	  designware-i2c driver to be built-in, for the same reason.
> -
>  config MFD_TI_LP873X
>  	tristate "TI LP873X Power Management IC"
>  	depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 4f6d2b8a5f76..8b322d89a0c5 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -105,7 +105,6 @@ obj-$(CONFIG_MFD_TPS65910)	+= tps65910.o
>  obj-$(CONFIG_MFD_TPS65912)	+= tps65912-core.o
>  obj-$(CONFIG_MFD_TPS65912_I2C)	+= tps65912-i2c.o
>  obj-$(CONFIG_MFD_TPS65912_SPI)  += tps65912-spi.o
> -obj-$(CONFIG_MFD_TPS68470)	+= tps68470.o
>  obj-$(CONFIG_MFD_TPS80031)	+= tps80031.o
>  obj-$(CONFIG_MENELAUS)		+= menelaus.o
>  
> diff --git a/drivers/mfd/tps68470.c b/drivers/mfd/tps68470.c
> deleted file mode 100644
> index 4a4df4ffd18c..000000000000
> --- a/drivers/mfd/tps68470.c
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * TPS68470 chip Parent driver
> - *
> - * Copyright (C) 2017 Intel Corporation
> - *
> - * Authors:
> - *	Rajmohan Mani <rajmohan.mani@intel.com>
> - *	Tianshu Qiu <tian.shu.qiu@intel.com>
> - *	Jian Xu Zheng <jian.xu.zheng@intel.com>
> - *	Yuning Pu <yuning.pu@intel.com>
> - */
> -
> -#include <linux/acpi.h>
> -#include <linux/delay.h>
> -#include <linux/i2c.h>
> -#include <linux/init.h>
> -#include <linux/mfd/core.h>
> -#include <linux/mfd/tps68470.h>
> -#include <linux/regmap.h>
> -
> -static const struct mfd_cell tps68470s[] = {
> -	{ .name = "tps68470-gpio" },
> -	{ .name = "tps68470_pmic_opregion" },
> -};
> -
> -static const struct regmap_config tps68470_regmap_config = {
> -	.reg_bits = 8,
> -	.val_bits = 8,
> -	.max_register = TPS68470_REG_MAX,
> -};
> -
> -static int tps68470_chip_init(struct device *dev, struct regmap *regmap)
> -{
> -	unsigned int version;
> -	int ret;
> -
> -	/* Force software reset */
> -	ret = regmap_write(regmap, TPS68470_REG_RESET, TPS68470_REG_RESET_MASK);
> -	if (ret)
> -		return ret;
> -
> -	ret = regmap_read(regmap, TPS68470_REG_REVID, &version);
> -	if (ret) {
> -		dev_err(dev, "Failed to read revision register: %d\n", ret);
> -		return ret;
> -	}
> -
> -	dev_info(dev, "TPS68470 REVID: 0x%x\n", version);
> -
> -	return 0;
> -}
> -
> -static int tps68470_probe(struct i2c_client *client)
> -{
> -	struct device *dev = &client->dev;
> -	struct regmap *regmap;
> -	int ret;
> -
> -	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
> -	if (IS_ERR(regmap)) {
> -		dev_err(dev, "devm_regmap_init_i2c Error %ld\n",
> -			PTR_ERR(regmap));
> -		return PTR_ERR(regmap);
> -	}
> -
> -	i2c_set_clientdata(client, regmap);
> -
> -	ret = tps68470_chip_init(dev, regmap);
> -	if (ret < 0) {
> -		dev_err(dev, "TPS68470 Init Error %d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, tps68470s,
> -			      ARRAY_SIZE(tps68470s), NULL, 0, NULL);
> -	if (ret < 0) {
> -		dev_err(dev, "devm_mfd_add_devices failed: %d\n", ret);
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
> -static const struct acpi_device_id tps68470_acpi_ids[] = {
> -	{"INT3472"},
> -	{},
> -};
> -
> -static struct i2c_driver tps68470_driver = {
> -	.driver = {
> -		   .name = "tps68470",
> -		   .acpi_match_table = tps68470_acpi_ids,
> -	},
> -	.probe_new = tps68470_probe,
> -};
> -builtin_i2c_driver(tps68470_driver);
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-06-17 11:14 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03 22:40 [PATCH v5 0/6] Introduce intel_skl_int3472 module Daniel Scally
2021-06-03 22:40 ` [PATCH v5 1/6] ACPI: scan: Extend acpi_walk_dep_device_list() Daniel Scally
2021-06-03 22:40 ` [PATCH v5 2/6] ACPI: scan: Add function to fetch dependent of acpi device Daniel Scally
2021-06-07 14:52   ` Rafael J. Wysocki
2021-06-03 22:40 ` [PATCH v5 3/6] gpiolib: acpi: Export acpi_get_gpiod() Daniel Scally
2021-06-04 12:59   ` Andy Shevchenko
2021-06-04 14:09     ` Daniel Scally
2021-06-03 22:40 ` [PATCH v5 4/6] gpiolib: acpi: Add acpi_gpio_get_io_resource() Daniel Scally
2021-06-04 13:01   ` Andy Shevchenko
2021-06-04 14:09     ` Daniel Scally
2021-06-04 14:33       ` Andy Shevchenko
2021-06-04 16:54         ` Daniel Scally
2021-06-03 22:40 ` [PATCH v5 5/6] platform/x86: Add intel_skl_int3472 driver Daniel Scally
2021-06-04  1:31   ` kernel test robot
2021-06-05 23:07     ` Daniel Scally
2021-06-04  6:49   ` kernel test robot
2021-06-16 17:58   ` Hans de Goede
2021-06-16 18:08     ` Andy Shevchenko
2021-06-16 18:13       ` Hans de Goede
2021-06-16 21:42     ` Daniel Scally
2021-06-03 22:40 ` [PATCH v5 6/6] mfd: tps68470: Remove tps68470 MFD driver Daniel Scally
2021-06-17 11:14   ` Hans de Goede
2021-06-08  9:00 ` [PATCH v5 0/6] Introduce intel_skl_int3472 module Hans de Goede
2021-06-08  9:33   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).