[PATCH 0/1] ACPI / scan: Create platform device for INT33FE ACPI nodes

[PATCH 0/1] ACPI / scan: Create platform device for INT33FE ACPI nodes

[Hans de Goede]

Hi Rafael, Andy,

For the why and what of this patch see the (somewhat long) commit message.

The single patch in this set both touches drivers/acpi/scan.c and
drivers/platform/x86/intel_cht_int33fe.c, this is done this way to avoid
regressions when bisecting.

The main change here really is to ACPI change and intel_cht_int33fe.c is
modified to follow suit. Also I do not expect intel_cht_int33fe.c to see
much changes this cycle. As such I believe it would be best to merge this
patch through Rafael's tree (after review).

Andy is that ok with you and we have your ack for this?

Regards,

Hans

[PATCH] ACPI / scan: Create platform device for INT33FE ACPI nodes

[Hans de Goede]

Bay and Cherry Trail devices with a Dollar Cove or Whiskey Cove PMIC
have an ACPI node with a HID of INT33FE which is a "virtual" battery
device implementing a standard ACPI battery interface which depends upon
a proprietary, undocument OpRegion called BMOP. Since we do have docs
for the actual fuel-gauges used on these boards we instead use native
fuel-gauge drivers talking directly to the fuel-gauge ICs on boards which
rely on this INT33FE device for their battery monitoring.

On boards with a Dollar Cove PMIC the INT33FE device's resources (_CRS)
describe a non-existing I2C client at address 0x6b with a bus-speed of
100KHz. This is a problem on some boards since there are actual devices
on that same bus which need a speed of 400KHz to function properly.

This commit adds the INT33FE HID to the list of devices with I2C resources
which should be enumerated as a platform-device rather then letting the
i2c-core instantiate an i2c-client matching the first I2C resource,
so that its bus-speed will not influence the max speed of the I2C bus.
This fixes e.g. the touchscreen not working on the Teclast X98 II Plus.

The INT33FE device on boards with a Whiskey Cove PMIC is somewhat special.
Its first I2C resource is for a secondary I2C address of the PMIC itself,
which is already described in an ACPI device with an INT34D3 HID.

But it has 3 more I2C resources describing 3 other chips for which we do
need to instantiate I2C clients and which need device-connections added
between them for things to work properly. This special case is handled by
the drivers/platform/x86/intel_cht_int33fe.c code.

Before this commit that code was binding to the i2c-client instantiated
for the secondary I2C address of the PMIC, since we now instantiate a
platform device for the INT33FE device instead, this commit also changes
the intel_cht_int33fe driver from an i2c driver to a platform driver.

This also brings the intel_cht_int33fe drv inline with how we instantiate
multiple i2c clients from a single ACPI device in other cases, as done
by the drivers/platform/x86/i2c-multi-instantiate.c code.

Reported-and-tested-by: Alexander Meiler <alex.meiler@protonmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/scan.c                      |  1 +
 drivers/platform/x86/intel_cht_int33fe.c | 24 +++++++++---------------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e1b6231cfa1c..1dcc48b9d33c 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1550,6 +1550,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
 	 */
 	static const struct acpi_device_id i2c_multi_instantiate_ids[] = {
 		{"BSG1160", },
+		{"INT33FE", },
 		{}
 	};
 
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index a26f410800c2..f40b1c192106 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,6 +24,7 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -88,9 +89,9 @@ static const struct property_entry fusb302_props[] = {
 	{ }
 };
 
-static int cht_int33fe_probe(struct i2c_client *client)
+static int cht_int33fe_probe(struct platform_device *pdev)
 {
-	struct device *dev = &client->dev;
+	struct device *dev = &pdev->dev;
 	struct i2c_board_info board_info;
 	struct cht_int33fe_data *data;
 	struct i2c_client *max17047;
@@ -207,7 +208,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
 	if (!data->pi3usb30532)
 		goto out_unregister_fusb302;
 
-	i2c_set_clientdata(client, data);
+	platform_set_drvdata(pdev, data);
 
 	return 0;
 
@@ -223,9 +224,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
 	return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
 
-static int cht_int33fe_remove(struct i2c_client *i2c)
+static int cht_int33fe_remove(struct platform_device *pdev)
 {
-	struct cht_int33fe_data *data = i2c_get_clientdata(i2c);
+	struct cht_int33fe_data *data = platform_get_drvdata(pdev);
 
 	i2c_unregister_device(data->pi3usb30532);
 	i2c_unregister_device(data->fusb302);
@@ -237,29 +238,22 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
 	return 0;
 }
 
-static const struct i2c_device_id cht_int33fe_i2c_id[] = {
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, cht_int33fe_i2c_id);
-
 static const struct acpi_device_id cht_int33fe_acpi_ids[] = {
 	{ "INT33FE", },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, cht_int33fe_acpi_ids);
 
-static struct i2c_driver cht_int33fe_driver = {
+static struct platform_driver cht_int33fe_driver = {
 	.driver	= {
 		.name = "Intel Cherry Trail ACPI INT33FE driver",
 		.acpi_match_table = ACPI_PTR(cht_int33fe_acpi_ids),
 	},
-	.probe_new = cht_int33fe_probe,
+	.probe = cht_int33fe_probe,
 	.remove = cht_int33fe_remove,
-	.id_table = cht_int33fe_i2c_id,
-	.disable_i2c_core_irq_mapping = true,
 };
 
-module_i2c_driver(cht_int33fe_driver);
+module_platform_driver(cht_int33fe_driver);
 
 MODULE_DESCRIPTION("Intel Cherry Trail ACPI INT33FE pseudo device driver");
 MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
-- 
2.19.0

Re: [PATCH 0/1] ACPI / scan: Create platform device for INT33FE ACPI nodes

[Andy Shevchenko]

On Wed, Oct 17, 2018 at 11:59 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Rafael, Andy,
>
> For the why and what of this patch see the (somewhat long) commit message.
>
> The single patch in this set both touches drivers/acpi/scan.c and
> drivers/platform/x86/intel_cht_int33fe.c, this is done this way to avoid
> regressions when bisecting.
>
> The main change here really is to ACPI change and intel_cht_int33fe.c is
> modified to follow suit. Also I do not expect intel_cht_int33fe.c to see
> much changes this cycle. As such I believe it would be best to merge this
> patch through Rafael's tree (after review).
>
> Andy is that ok with you and we have your ack for this?

I love this patch!

Definitely,
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thanks!

>
> Regards,
>
> Hans
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/1] ACPI / scan: Create platform device for INT33FE ACPI nodes

[Rafael J. Wysocki]

On Wednesday, October 17, 2018 1:39:54 PM CEST Andy Shevchenko wrote:
> On Wed, Oct 17, 2018 at 11:59 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > Hi Rafael, Andy,
> >
> > For the why and what of this patch see the (somewhat long) commit message.
> >
> > The single patch in this set both touches drivers/acpi/scan.c and
> > drivers/platform/x86/intel_cht_int33fe.c, this is done this way to avoid
> > regressions when bisecting.
> >
> > The main change here really is to ACPI change and intel_cht_int33fe.c is
> > modified to follow suit. Also I do not expect intel_cht_int33fe.c to see
> > much changes this cycle. As such I believe it would be best to merge this
> > patch through Rafael's tree (after review).
> >
> > Andy is that ok with you and we have your ack for this?
> 
> I love this patch!
> 
> Definitely,
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Patch applied, thanks!