[PATCH v5 0/4] i2c-core improvements for acpi devices + ACPI INT33FE driver

[PATCH v5 0/4] i2c-core improvements for acpi devices + ACPI INT33FE driver

[Hans de Goede]

Hi All,

Here is v5 of my patch-set for my i2c-core improvements for acpi
devices + the ACPI INT33FE platform driver.

New in this version are some minor fixed to the INT33FE driver.

The INT33FE driver depends on the i2c-core changes, so if
it is ok with Darren / Andy I believe it would be best for
this driver to be merged through the i2c subsys tree (with
their ack).

Regards,

Hans

[PATCH v5 1/4] i2c: core: Allow getting ACPI info by index

[Hans de Goede]

Modify struct i2c_acpi_lookup and i2c_acpi_fill_info() to allow
using them to get the info from a certain index in the ACPI-resource
list rather then taking the first I2cSerialBus resource.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v2:
-No changes
Changes in v3:
-Increment number of found i2c busses for lookup-by-index after checking
 the acpi-resource is an i2c bus rather then before
Changes in v4:
-No changes
---
 drivers/i2c/i2c-core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d2402bb..f7faa99 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -112,6 +112,8 @@ struct i2c_acpi_lookup {
 	acpi_handle adapter_handle;
 	acpi_handle device_handle;
 	acpi_handle search_handle;
+	int n;
+	int index;
 	u32 speed;
 	u32 min_speed;
 };
@@ -130,6 +132,9 @@ static int i2c_acpi_fill_info(struct acpi_resource *ares, void *data)
 	if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C)
 		return 1;
 
+	if (lookup->index != -1 && lookup->n++ != lookup->index)
+		return 1;
+
 	status = acpi_get_handle(lookup->device_handle,
 				 sb->resource_source.string_ptr,
 				 &lookup->adapter_handle);
@@ -182,6 +187,7 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
 
 	memset(&lookup, 0, sizeof(lookup));
 	lookup.info = info;
+	lookup.index = -1;
 
 	ret = i2c_acpi_do_lookup(adev, &lookup);
 	if (ret)
@@ -328,6 +334,7 @@ u32 i2c_acpi_find_bus_speed(struct device *dev)
 	lookup.search_handle = ACPI_HANDLE(dev);
 	lookup.min_speed = UINT_MAX;
 	lookup.info = &dummy;
+	lookup.index = -1;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
 				     I2C_ACPI_MAX_SCAN_DEPTH,
-- 
2.9.3

Re: [PATCH v5 1/4] i2c: core: Allow getting ACPI info by index

[Wolfram Sang]

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

On Wed, Apr 05, 2017 at 12:03:32AM +0200, Hans de Goede wrote:
> Modify struct i2c_acpi_lookup and i2c_acpi_fill_info() to allow
> using them to get the info from a certain index in the ACPI-resource
> list rather then taking the first I2cSerialBus resource.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 2/4] i2c: core: Add new i2c_acpi_new_device helper function

[Hans de Goede]

By default the i2c subsys creates an i2c-client for the first I2cSerialBus
resource of an acpi_device, but some acpi_devices have multiple
I2cSerialBus resources and we may want to instantiate i2c-clients for
the others.

This commit adds a new i2c_acpi_new_device function which can be used to
create an i2c-client for any I2cSerialBus resource of an acpi_device.

Note that the other resources may even be on a different i2c bus, so just
retrieving the client address is not enough.

Here is an example DSDT excerpt from such a device:

Device (WIDR)
{
    Name (_HID, "INT33FE" /* XPOWER Battery Device */)
    Name (_CID, "INT33FE" /* XPOWER Battery Device */)
    Name (_DDN, "WC PMIC Battery Device")
<snip>
    Name (RBUF, ResourceTemplate ()
    {
        I2cSerialBusV2 (0x005E, ControllerInitiated, 0x000186A0,
            AddressingMode7Bit, "\\_SB.PCI0.I2C7",
            0x00, ResourceConsumer, , Exclusive,
            )
        I2cSerialBusV2 (0x0036, ControllerInitiated, 0x000186A0,
            AddressingMode7Bit, "\\_SB.PCI0.I2C1",
            0x00, ResourceConsumer, , Exclusive,
            )
        I2cSerialBusV2 (0x0022, ControllerInitiated, 0x00061A80,
            AddressingMode7Bit, "\\_SB.PCI0.I2C1",
            0x00, ResourceConsumer, , Exclusive,
            )
        I2cSerialBusV2 (0x0054, ControllerInitiated, 0x00061A80,
            AddressingMode7Bit, "\\_SB.PCI0.I2C1",
            0x00, ResourceConsumer, , Exclusive,
            )
        GpioInt (Level, ActiveLow, Exclusive, PullNone, 0x0000,
            "\\_SB.PCI0.I2C7.PMI5", 0x00, ResourceConsumer, ,
            )
            {   // Pin list
        0x0012
            }
        GpioInt (Edge, ActiveLow, ExclusiveAndWake, PullNone, 0x0000,
            "\\_SB.GPO1", 0x00, ResourceConsumer, ,
            )
            {   // Pin list
        0x0005
            }
        GpioInt (Level, ActiveLow, Exclusive, PullNone, 0x0000,
            "\\_SB.PCI0.I2C7.PMI5", 0x00, ResourceConsumer, ,
            )
            {   // Pin list
        0x0013
            }
    })
    Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
    {
        Return (RBUF) /* \_SB_.PCI0.I2C7.WIDR.RBUF */
    }
<snip>
}

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v2:
-No changes
Changes in v3:
-Added example DSDT snippet to the commit msg
Changes in v4:
-Add a board_info argument so that the caller can set type, irq, properties,
 etc.
---
 drivers/i2c/i2c-core.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    |  7 +++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f7faa99..00c4cef 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -421,6 +421,55 @@ static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value,
 static struct notifier_block i2c_acpi_notifier = {
 	.notifier_call = i2c_acpi_notify,
 };
+
+/**
+ * i2c_acpi_new_device - Create i2c-client for the Nth I2cSerialBus resource
+ * @dev:     Device owning the ACPI resources to get the client from
+ * @index:   Index of ACPI resource to get
+ * @info:    describes the I2C device; note this is modified (addr gets set)
+ * Context: can sleep
+ *
+ * By default the i2c subsys creates an i2c-client for the first I2cSerialBus
+ * resource of an acpi_device, but some acpi_devices have multiple I2cSerialBus
+ * resources, in that case this function can be used to create an i2c-client
+ * for other I2cSerialBus resources in the Current Resource Settings table.
+ *
+ * Also see i2c_new_device, which this function calls to create the i2c-client.
+ *
+ * Returns a pointer to the new i2c-client, or NULL if the adapter is not found.
+ */
+struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
+				       struct i2c_board_info *info)
+{
+	struct i2c_acpi_lookup lookup;
+	struct i2c_adapter *adapter;
+	struct acpi_device *adev;
+	LIST_HEAD(resource_list);
+	int ret;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return NULL;
+
+	memset(&lookup, 0, sizeof(lookup));
+	lookup.info = info;
+	lookup.device_handle = acpi_device_handle(adev);
+	lookup.index = index;
+
+	ret = acpi_dev_get_resources(adev, &resource_list,
+				     i2c_acpi_fill_info, &lookup);
+	acpi_dev_free_resource_list(&resource_list);
+
+	if (ret < 0 || !info->addr)
+		return NULL;
+
+	adapter = i2c_acpi_find_adapter_by_handle(lookup.adapter_handle);
+	if (!adapter)
+		return NULL;
+
+	return i2c_new_device(adapter, info);
+}
+EXPORT_SYMBOL_GPL(i2c_acpi_new_device);
 #else /* CONFIG_ACPI */
 static inline void i2c_acpi_register_devices(struct i2c_adapter *adap) { }
 extern struct notifier_block i2c_acpi_notifier;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 6b18352..53fa50f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -824,11 +824,18 @@ static inline const struct of_device_id
 
 #if IS_ENABLED(CONFIG_ACPI)
 u32 i2c_acpi_find_bus_speed(struct device *dev);
+struct i2c_client *i2c_acpi_new_device(struct device *dev, int index,
+				       struct i2c_board_info *info);
 #else
 static inline u32 i2c_acpi_find_bus_speed(struct device *dev)
 {
 	return 0;
 }
+static inline struct i2c_client *i2c_acpi_new_device(struct device *dev,
+					int index, struct i2c_board_info *info)
+{
+	return NULL;
+}
 #endif /* CONFIG_ACPI */
 
 #endif /* _LINUX_I2C_H */
-- 
2.9.3

Re: [PATCH v5 2/4] i2c: core: Add new i2c_acpi_new_device helper function

[Wolfram Sang]

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

On Wed, Apr 05, 2017 at 12:03:33AM +0200, Hans de Goede wrote:
> By default the i2c subsys creates an i2c-client for the first I2cSerialBus
> resource of an acpi_device, but some acpi_devices have multiple
> I2cSerialBus resources and we may want to instantiate i2c-clients for
> the others.
> 
> This commit adds a new i2c_acpi_new_device function which can be used to
> create an i2c-client for any I2cSerialBus resource of an acpi_device.
> 
> Note that the other resources may even be on a different i2c bus, so just
> retrieving the client address is not enough.
> 
> Here is an example DSDT excerpt from such a device:
> 
> Device (WIDR)
> {
>     Name (_HID, "INT33FE" /* XPOWER Battery Device */)
>     Name (_CID, "INT33FE" /* XPOWER Battery Device */)
>     Name (_DDN, "WC PMIC Battery Device")
> <snip>
>     Name (RBUF, ResourceTemplate ()
>     {
>         I2cSerialBusV2 (0x005E, ControllerInitiated, 0x000186A0,
>             AddressingMode7Bit, "\\_SB.PCI0.I2C7",
>             0x00, ResourceConsumer, , Exclusive,
>             )
>         I2cSerialBusV2 (0x0036, ControllerInitiated, 0x000186A0,
>             AddressingMode7Bit, "\\_SB.PCI0.I2C1",
>             0x00, ResourceConsumer, , Exclusive,
>             )
>         I2cSerialBusV2 (0x0022, ControllerInitiated, 0x00061A80,
>             AddressingMode7Bit, "\\_SB.PCI0.I2C1",
>             0x00, ResourceConsumer, , Exclusive,
>             )
>         I2cSerialBusV2 (0x0054, ControllerInitiated, 0x00061A80,
>             AddressingMode7Bit, "\\_SB.PCI0.I2C1",
>             0x00, ResourceConsumer, , Exclusive,
>             )
>         GpioInt (Level, ActiveLow, Exclusive, PullNone, 0x0000,
>             "\\_SB.PCI0.I2C7.PMI5", 0x00, ResourceConsumer, ,
>             )
>             {   // Pin list
>         0x0012
>             }
>         GpioInt (Edge, ActiveLow, ExclusiveAndWake, PullNone, 0x0000,
>             "\\_SB.GPO1", 0x00, ResourceConsumer, ,
>             )
>             {   // Pin list
>         0x0005
>             }
>         GpioInt (Level, ActiveLow, Exclusive, PullNone, 0x0000,
>             "\\_SB.PCI0.I2C7.PMI5", 0x00, ResourceConsumer, ,
>             )
>             {   // Pin list
>         0x0013
>             }
>     })
>     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>     {
>         Return (RBUF) /* \_SB_.PCI0.I2C7.WIDR.RBUF */
>     }
> <snip>
> }
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 3/4] i2c: core: Allow drivers to disable i2c-core irq mapping

[Hans de Goede]

By default the i2c-core will try to get an irq with index 0 on ACPI / of
instantiated devices. This is troublesome on some ACPI systems where the
irq info at index 0 in the CRS table may contain nonsense and/or point
to an irqchip for which there is no Linux driver.

If this happens then before this commit the driver's probe method would
never get called because i2c_device_probe will try to get an irq by
calling acpi_dev_gpio_irq_get which will always return -EPROBE in this
case, as it waits for a matching irqchip driver to load. Thus causing
the driver to not get a chance to bind.

This commit adds a new disable_i2c_core_irq_mapping flag to struct
i2c_driver which a driver can set to tell the core to skip irq mapping.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Actually also use the irq_index for of interrupts
Changes in v3:
-Add kernel doc for new i2c_driver irq_index member
-Remove duplicate assignment of driver in i2c_device_probe
Changes in v4:
-Add a disable_i2c_core_irq_mapping flag to i2c_driver instead of
 an irq_index member (effectively a rewrite of the patch, dropped
 the Reviewed-by-s)
---
 drivers/i2c/i2c-core.c | 6 +++---
 include/linux/i2c.h    | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 00c4cef..7a065c4 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -985,7 +985,9 @@ static int i2c_device_probe(struct device *dev)
 	if (!client)
 		return 0;
 
-	if (!client->irq) {
+	driver = to_i2c_driver(dev->driver);
+
+	if (!client->irq && !driver->disable_i2c_core_irq_mapping) {
 		int irq = -ENOENT;
 
 		if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
@@ -1007,8 +1009,6 @@ static int i2c_device_probe(struct device *dev)
 		client->irq = irq;
 	}
 
-	driver = to_i2c_driver(dev->driver);
-
 	/*
 	 * An I2C ID table is not mandatory, if and only if, a suitable Device
 	 * Tree match table entry is supplied for the probing device.
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 53fa50f..3a57e3d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -149,6 +149,7 @@ enum i2c_alert_protocol {
  * @detect: Callback for device detection
  * @address_list: The I2C addresses to probe (for detect)
  * @clients: List of detected clients we created (for i2c-core use only)
+ * @disable_i2c_core_irq_mapping: Tell the i2c-core to not do irq-mapping
  *
  * The driver.owner field should be set to the module owner of this driver.
  * The driver.name field should be set to the name of this driver.
@@ -212,6 +213,8 @@ struct i2c_driver {
 	int (*detect)(struct i2c_client *, struct i2c_board_info *);
 	const unsigned short *address_list;
 	struct list_head clients;
+
+	bool disable_i2c_core_irq_mapping;
 };
 #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
 
-- 
2.9.3

Re: [PATCH v5 3/4] i2c: core: Allow drivers to disable i2c-core irq mapping

[Andy Shevchenko]

On Wed, 2017-04-05 at 00:03 +0200, Hans de Goede wrote:
> By default the i2c-core will try to get an irq with index 0 on ACPI /
> of
> instantiated devices. This is troublesome on some ACPI systems where
> the
> irq info at index 0 in the CRS table may contain nonsense and/or point
> to an irqchip for which there is no Linux driver.
> 
> If this happens then before this commit the driver's probe method
> would
> never get called because i2c_device_probe will try to get an irq by
> calling acpi_dev_gpio_irq_get which will always return -EPROBE in this
> case, as it waits for a matching irqchip driver to load. Thus causing
> the driver to not get a chance to bind.
> 
> This commit adds a new disable_i2c_core_irq_mapping flag to struct
> i2c_driver which a driver can set to tell the core to skip irq
> mapping.

FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Actually also use the irq_index for of interrupts
> Changes in v3:
> -Add kernel doc for new i2c_driver irq_index member
> -Remove duplicate assignment of driver in i2c_device_probe
> Changes in v4:
> -Add a disable_i2c_core_irq_mapping flag to i2c_driver instead of
>  an irq_index member (effectively a rewrite of the patch, dropped
>  the Reviewed-by-s)
> ---
>  drivers/i2c/i2c-core.c | 6 +++---
>  include/linux/i2c.h    | 3 +++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 00c4cef..7a065c4 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -985,7 +985,9 @@ static int i2c_device_probe(struct device *dev)
>  	if (!client)
>  		return 0;
>  
> -	if (!client->irq) {
> +	driver = to_i2c_driver(dev->driver);
> +
> +	if (!client->irq && !driver->disable_i2c_core_irq_mapping) {
>  		int irq = -ENOENT;
>  
>  		if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
> @@ -1007,8 +1009,6 @@ static int i2c_device_probe(struct device *dev)
>  		client->irq = irq;
>  	}
>  
> -	driver = to_i2c_driver(dev->driver);
> -
>  	/*
>  	 * An I2C ID table is not mandatory, if and only if, a
> suitable Device
>  	 * Tree match table entry is supplied for the probing device.
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 53fa50f..3a57e3d 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -149,6 +149,7 @@ enum i2c_alert_protocol {
>   * @detect: Callback for device detection
>   * @address_list: The I2C addresses to probe (for detect)
>   * @clients: List of detected clients we created (for i2c-core use
> only)
> + * @disable_i2c_core_irq_mapping: Tell the i2c-core to not do irq-
> mapping
>   *
>   * The driver.owner field should be set to the module owner of this
> driver.
>   * The driver.name field should be set to the name of this driver.
> @@ -212,6 +213,8 @@ struct i2c_driver {
>  	int (*detect)(struct i2c_client *, struct i2c_board_info *);
>  	const unsigned short *address_list;
>  	struct list_head clients;
> +
> +	bool disable_i2c_core_irq_mapping;
>  };
>  #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v5 3/4] i2c: core: Allow drivers to disable i2c-core irq mapping

[Wolfram Sang]

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

On Wed, Apr 05, 2017 at 12:03:34AM +0200, Hans de Goede wrote:
> By default the i2c-core will try to get an irq with index 0 on ACPI / of
> instantiated devices. This is troublesome on some ACPI systems where the
> irq info at index 0 in the CRS table may contain nonsense and/or point
> to an irqchip for which there is no Linux driver.
> 
> If this happens then before this commit the driver's probe method would
> never get called because i2c_device_probe will try to get an irq by
> calling acpi_dev_gpio_irq_get which will always return -EPROBE in this
> case, as it waits for a matching irqchip driver to load. Thus causing
> the driver to not get a chance to bind.
> 
> This commit adds a new disable_i2c_core_irq_mapping flag to struct
> i2c_driver which a driver can set to tell the core to skip irq mapping.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v5 4/4] platform/x86: Add Intel Cherry Trail ACPI INT33FE device driver

[Hans de Goede]

The INT33FE ACPI device has a CRS table with I2cSerialBusV2 resources for
3 devices: Maxim MAX17047 Fuel Gauge Controller, FUSB300C USB Type-C
Controller and PI3USB30532 USB switch.

This commit adds a driver for this ACPI device which instantiates
i2c-clients for these, so that the standard i2c drivers for these chips
can bind to the them.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v4:
-This is a new patch in v4 of this patch-set replacing the
 cht_wc_fuel_gauge driver which was binding to the INT33FE device before
 I figured out this is some sort of meta device describing 3 devices
 and that the fuel-guage really is just a Maxim MAX17047. The
 cht_wc_fuel_gauge driver will be replaced by a max17047 driver which I
 will submit seperately
Changes in v5:
-Remove not needed (and not upstream) #include <linux/power/acpi.h>
-Sort variable declarations in upside-down Chistmas tree order
---
 drivers/platform/x86/Kconfig             |  13 +++
 drivers/platform/x86/Makefile            |   1 +
 drivers/platform/x86/intel_cht_int33fe.c | 143 +++++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 drivers/platform/x86/intel_cht_int33fe.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 6a5b79c..57f7c1d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -772,6 +772,19 @@ config ACPI_CMPC
 	  keys as input device, backlight device, tablet and accelerometer
 	  devices.
 
+config INTEL_CHT_INT33FE
+	tristate "Intel Cherry Trail ACPI INT33FE Driver"
+	depends on X86 && ACPI
+	---help---
+	  This driver add support for the INT33FE ACPI device found on
+	  some Intel Cherry Trail devices.
+
+	  The INT33FE ACPI device has a CRS table with I2cSerialBusV2
+	  resources for 3 devices: Maxim MAX17047 Fuel Gauge Controller,
+	  FUSB300C USB Type-C Controller and PI3USB30532 USB switch.
+	  This driver instantiates i2c-clients for these, so that standard
+	  i2c drivers for these chips can bind to the them.
+
 config INTEL_HID_EVENT
 	tristate "INTEL HID Event"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 6d4b01a..6731893 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
 obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
 obj-$(CONFIG_TOSHIBA_HAPS)	+= toshiba_haps.o
 obj-$(CONFIG_TOSHIBA_WMI)	+= toshiba-wmi.o
+obj-$(CONFIG_INTEL_CHT_INT33FE)	+= intel_cht_int33fe.o
 obj-$(CONFIG_INTEL_HID_EVENT)	+= intel-hid.o
 obj-$(CONFIG_INTEL_VBTN)	+= intel-vbtn.o
 obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
new file mode 100644
index 0000000..df53657
--- /dev/null
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -0,0 +1,143 @@
+/*
+ * Intel Cherry Trail ACPI INT33FE pseudo device driver
+ *
+ * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Some Intel Cherry Trail based device which ship with Windows 10, have
+ * this weird INT33FE ACPI device with a CRS table with 4 I2cSerialBusV2
+ * resources, for 4 different chips attached to various i2c busses:
+ * 1. The Whiskey Cove pmic, which is also described by the INT34D3 ACPI device
+ * 2. Maxim MAX17047 Fuel Gauge Controller
+ * 3. FUSB300C USB Type-C Controller
+ * 4. PI3USB30532 USB switch
+ *
+ * So this driver is a stub / pseudo driver whose only purpose is to
+ * instantiate i2c-clients for chips 2 - 4, so that standard i2c drivers
+ * for these chips can bind to the them.
+ */
+
+#include <linux/acpi.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#define EXPECTED_PTYPE		4
+
+struct cht_int33fe_data {
+	struct i2c_client *max17047;
+	struct i2c_client *fusb300c;
+	struct i2c_client *pi3usb30532;
+};
+
+static int cht_int33fe_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct i2c_board_info board_info;
+	struct cht_int33fe_data *data;
+	unsigned long long ptyp;
+	acpi_status status;
+	int fusb300c_irq;
+
+	status = acpi_evaluate_integer(ACPI_HANDLE(dev), "PTYP", NULL, &ptyp);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "Error getting PTYPE\n");
+		return -ENODEV;
+	}
+
+	/*
+	 * The same ACPI HID is used for different configurations check PTYP
+	 * to ensure that we are dealing with the expected config.
+	 */
+	if (ptyp != EXPECTED_PTYPE)
+		return -ENODEV;
+
+	/* The FUSB300C uses the irq at index 1 and is the only irq user */
+	fusb300c_irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 1);
+	if (fusb300c_irq < 0) {
+		if (fusb300c_irq != -EPROBE_DEFER)
+			dev_err(dev, "Error getting FUSB300C irq\n");
+		return fusb300c_irq;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	memset(&board_info, 0, sizeof(board_info));
+	strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
+
+	data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
+	if (!data->max17047)
+		return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+
+	memset(&board_info, 0, sizeof(board_info));
+	strlcpy(board_info.type, "fusb300c", I2C_NAME_SIZE);
+	board_info.irq = fusb300c_irq;
+
+	data->fusb300c = i2c_acpi_new_device(dev, 2, &board_info);
+	if (!data->fusb300c)
+		goto out_unregister_max17047;
+
+	memset(&board_info, 0, sizeof(board_info));
+	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
+
+	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
+	if (!data->pi3usb30532)
+		goto out_unregister_fusb300c;
+
+	i2c_set_clientdata(client, data);
+
+	return 0;
+
+out_unregister_fusb300c:
+	i2c_unregister_device(data->fusb300c);
+
+out_unregister_max17047:
+	i2c_unregister_device(data->max17047);
+
+	return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
+}
+
+static int cht_int33fe_remove(struct i2c_client *i2c)
+{
+	struct cht_int33fe_data *data = i2c_get_clientdata(i2c);
+
+	i2c_unregister_device(data->pi3usb30532);
+	i2c_unregister_device(data->fusb300c);
+	i2c_unregister_device(data->max17047);
+
+	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 = {
+	.driver	= {
+		.name = "Intel Cherry Trail ACPI INT33FE driver",
+		.acpi_match_table = ACPI_PTR(cht_int33fe_acpi_ids),
+	},
+	.probe_new = 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_DESCRIPTION("Intel Cherry Trail ACPI INT33FE pseudo device driver");
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_LICENSE("GPL");
-- 
2.9.3

Re: [PATCH v5 4/4] platform/x86: Add Intel Cherry Trail ACPI INT33FE device driver

[Andy Shevchenko]

On Wed, 2017-04-05 at 00:03 +0200, Hans de Goede wrote:
> The INT33FE ACPI device has a CRS table with I2cSerialBusV2 resources
> for
> 3 devices: Maxim MAX17047 Fuel Gauge Controller, FUSB300C USB Type-C
> Controller and PI3USB30532 USB switch.
> 
> This commit adds a driver for this ACPI device which instantiates
> i2c-clients for these, so that the standard i2c drivers for these
> chips
> can bind to the them.
> 

Looks like we need something such as MFD for I2C bus clients...

FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v4:
> -This is a new patch in v4 of this patch-set replacing the
>  cht_wc_fuel_gauge driver which was binding to the INT33FE device
> before
>  I figured out this is some sort of meta device describing 3 devices
>  and that the fuel-guage really is just a Maxim MAX17047. The
>  cht_wc_fuel_gauge driver will be replaced by a max17047 driver which
> I
>  will submit seperately
> Changes in v5:
> -Remove not needed (and not upstream) #include <linux/power/acpi.h>
> -Sort variable declarations in upside-down Chistmas tree order
> ---
>  drivers/platform/x86/Kconfig             |  13 +++
>  drivers/platform/x86/Makefile            |   1 +
>  drivers/platform/x86/intel_cht_int33fe.c | 143
> +++++++++++++++++++++++++++++++
>  3 files changed, 157 insertions(+)
>  create mode 100644 drivers/platform/x86/intel_cht_int33fe.c
> 
> diff --git a/drivers/platform/x86/Kconfig
> b/drivers/platform/x86/Kconfig
> index 6a5b79c..57f7c1d 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -772,6 +772,19 @@ config ACPI_CMPC
>  	  keys as input device, backlight device, tablet and
> accelerometer
>  	  devices.
>  
> +config INTEL_CHT_INT33FE
> +	tristate "Intel Cherry Trail ACPI INT33FE Driver"
> +	depends on X86 && ACPI
> +	---help---
> +	  This driver add support for the INT33FE ACPI device found
> on
> +	  some Intel Cherry Trail devices.
> +
> +	  The INT33FE ACPI device has a CRS table with I2cSerialBusV2
> +	  resources for 3 devices: Maxim MAX17047 Fuel Gauge
> Controller,
> +	  FUSB300C USB Type-C Controller and PI3USB30532 USB switch.
> +	  This driver instantiates i2c-clients for these, so that
> standard
> +	  i2c drivers for these chips can bind to the them.
> +
>  config INTEL_HID_EVENT
>  	tristate "INTEL HID Event"
>  	depends on ACPI
> diff --git a/drivers/platform/x86/Makefile
> b/drivers/platform/x86/Makefile
> index 6d4b01a..6731893 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
>  obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
>  obj-$(CONFIG_TOSHIBA_HAPS)	+= toshiba_haps.o
>  obj-$(CONFIG_TOSHIBA_WMI)	+= toshiba-wmi.o
> +obj-$(CONFIG_INTEL_CHT_INT33FE)	+= intel_cht_int33fe.o
>  obj-$(CONFIG_INTEL_HID_EVENT)	+= intel-hid.o
>  obj-$(CONFIG_INTEL_VBTN)	+= intel-vbtn.o
>  obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c
> b/drivers/platform/x86/intel_cht_int33fe.c
> new file mode 100644
> index 0000000..df53657
> --- /dev/null
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -0,0 +1,143 @@
> +/*
> + * Intel Cherry Trail ACPI INT33FE pseudo device driver
> + *
> + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Some Intel Cherry Trail based device which ship with Windows 10,
> have
> + * this weird INT33FE ACPI device with a CRS table with 4
> I2cSerialBusV2
> + * resources, for 4 different chips attached to various i2c busses:
> + * 1. The Whiskey Cove pmic, which is also described by the INT34D3
> ACPI device
> + * 2. Maxim MAX17047 Fuel Gauge Controller
> + * 3. FUSB300C USB Type-C Controller
> + * 4. PI3USB30532 USB switch
> + *
> + * So this driver is a stub / pseudo driver whose only purpose is to
> + * instantiate i2c-clients for chips 2 - 4, so that standard i2c
> drivers
> + * for these chips can bind to the them.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#define EXPECTED_PTYPE		4
> +
> +struct cht_int33fe_data {
> +	struct i2c_client *max17047;
> +	struct i2c_client *fusb300c;
> +	struct i2c_client *pi3usb30532;
> +};
> +
> +static int cht_int33fe_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct i2c_board_info board_info;
> +	struct cht_int33fe_data *data;
> +	unsigned long long ptyp;
> +	acpi_status status;
> +	int fusb300c_irq;
> +
> +	status = acpi_evaluate_integer(ACPI_HANDLE(dev), "PTYP",
> NULL, &ptyp);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(dev, "Error getting PTYPE\n");
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * The same ACPI HID is used for different configurations
> check PTYP
> +	 * to ensure that we are dealing with the expected config.
> +	 */
> +	if (ptyp != EXPECTED_PTYPE)
> +		return -ENODEV;
> +
> +	/* The FUSB300C uses the irq at index 1 and is the only irq
> user */
> +	fusb300c_irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 1);
> +	if (fusb300c_irq < 0) {
> +		if (fusb300c_irq != -EPROBE_DEFER)
> +			dev_err(dev, "Error getting FUSB300C irq\n");
> +		return fusb300c_irq;
> +	}
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	memset(&board_info, 0, sizeof(board_info));
> +	strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
> +
> +	data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
> +	if (!data->max17047)
> +		return -EPROBE_DEFER; /* Wait for the i2c-adapter to
> load */
> +
> +	memset(&board_info, 0, sizeof(board_info));
> +	strlcpy(board_info.type, "fusb300c", I2C_NAME_SIZE);
> +	board_info.irq = fusb300c_irq;
> +
> +	data->fusb300c = i2c_acpi_new_device(dev, 2, &board_info);
> +	if (!data->fusb300c)
> +		goto out_unregister_max17047;
> +
> +	memset(&board_info, 0, sizeof(board_info));
> +	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
> +
> +	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
> +	if (!data->pi3usb30532)
> +		goto out_unregister_fusb300c;
> +
> +	i2c_set_clientdata(client, data);
> +
> +	return 0;
> +
> +out_unregister_fusb300c:
> +	i2c_unregister_device(data->fusb300c);
> +
> +out_unregister_max17047:
> +	i2c_unregister_device(data->max17047);
> +
> +	return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
> +}
> +
> +static int cht_int33fe_remove(struct i2c_client *i2c)
> +{
> +	struct cht_int33fe_data *data = i2c_get_clientdata(i2c);
> +
> +	i2c_unregister_device(data->pi3usb30532);
> +	i2c_unregister_device(data->fusb300c);
> +	i2c_unregister_device(data->max17047);
> +
> +	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 = {
> +	.driver	= {
> +		.name = "Intel Cherry Trail ACPI INT33FE driver",
> +		.acpi_match_table = ACPI_PTR(cht_int33fe_acpi_ids),
> +	},
> +	.probe_new = 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_DESCRIPTION("Intel Cherry Trail ACPI INT33FE pseudo device
> driver");
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_LICENSE("GPL");

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v5 4/4] platform/x86: Add Intel Cherry Trail ACPI INT33FE device driver

[Darren Hart]

On Wed, Apr 05, 2017 at 04:52:26PM +0300, Andy Shevchenko wrote:
> On Wed, 2017-04-05 at 00:03 +0200, Hans de Goede wrote:
> > The INT33FE ACPI device has a CRS table with I2cSerialBusV2 resources
> > for
> > 3 devices: Maxim MAX17047 Fuel Gauge Controller, FUSB300C USB Type-C
> > Controller and PI3USB30532 USB switch.
> > 
> > This commit adds a driver for this ACPI device which instantiates
> > i2c-clients for these, so that the standard i2c drivers for these
> > chips
> > can bind to the them.
> > 
> 
> Looks like we need something such as MFD for I2C bus clients...
> 
> FWIW:
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Wolfram, will you be making an immutable i2c branch available for us to merge
into our platform driver x86 for-next branch in order to apply this patch?

Alternatively you could send this in on our behalf, but that complicates any
fixes between now and rc1 for this new driver. We'd prefer the immutable branch
and pull this in ourselves.

Either way,

Reviewed-by: Darren Hart (VMware) <dvhart@infradead.org>

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v5 4/4] platform/x86: Add Intel Cherry Trail ACPI INT33FE device driver

[Wolfram Sang]

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

On Wed, Apr 05, 2017 at 08:41:46AM -0700, Darren Hart wrote:
> On Wed, Apr 05, 2017 at 04:52:26PM +0300, Andy Shevchenko wrote:
> > On Wed, 2017-04-05 at 00:03 +0200, Hans de Goede wrote:
> > > The INT33FE ACPI device has a CRS table with I2cSerialBusV2 resources
> > > for
> > > 3 devices: Maxim MAX17047 Fuel Gauge Controller, FUSB300C USB Type-C
> > > Controller and PI3USB30532 USB switch.
> > > 
> > > This commit adds a driver for this ACPI device which instantiates
> > > i2c-clients for these, so that the standard i2c drivers for these
> > > chips
> > > can bind to the them.
> > > 
> > 
> > Looks like we need something such as MFD for I2C bus clients...
> > 
> > FWIW:
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Wolfram, will you be making an immutable i2c branch available for us to merge
> into our platform driver x86 for-next branch in order to apply this patch?
> 
> Alternatively you could send this in on our behalf, but that complicates any
> fixes between now and rc1 for this new driver. We'd prefer the immutable branch
> and pull this in ourselves.

Fine with me, here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-INT33FE


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 4/4] platform/x86: Add Intel Cherry Trail ACPI INT33FE device driver

[Darren Hart]

On Sun, Apr 16, 2017 at 10:08:10PM +0200, Wolfram Sang wrote:
> On Wed, Apr 05, 2017 at 08:41:46AM -0700, Darren Hart wrote:
> > On Wed, Apr 05, 2017 at 04:52:26PM +0300, Andy Shevchenko wrote:
> > > On Wed, 2017-04-05 at 00:03 +0200, Hans de Goede wrote:
> > > > The INT33FE ACPI device has a CRS table with I2cSerialBusV2 resources
> > > > for
> > > > 3 devices: Maxim MAX17047 Fuel Gauge Controller, FUSB300C USB Type-C
> > > > Controller and PI3USB30532 USB switch.
> > > > 
> > > > This commit adds a driver for this ACPI device which instantiates
> > > > i2c-clients for these, so that the standard i2c drivers for these
> > > > chips
> > > > can bind to the them.
> > > > 
> > > 
> > > Looks like we need something such as MFD for I2C bus clients...
> > > 
> > > FWIW:
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > Wolfram, will you be making an immutable i2c branch available for us to merge
> > into our platform driver x86 for-next branch in order to apply this patch?
> > 
> > Alternatively you could send this in on our behalf, but that complicates any
> > fixes between now and rc1 for this new driver. We'd prefer the immutable branch
> > and pull this in ourselves.
> 
> Fine with me, here it is:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-INT33FE
> 

Excellent, thank you.

Hans, v6 queued to testing.


-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v5 0/4] i2c-core improvements for acpi devices + ACPI INT33FE driver

[Darren Hart]

On Wed, Apr 05, 2017 at 12:03:31AM +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is v5 of my patch-set for my i2c-core improvements for acpi
> devices + the ACPI INT33FE platform driver.
> 
> New in this version are some minor fixed to the INT33FE driver.
> 
> The INT33FE driver depends on the i2c-core changes, so if
> it is ok with Darren / Andy I believe it would be best for
> this driver to be merged through the i2c subsys tree (with
> their ack).

Is there an immutable i2c branch I can merge with these changes in order to add
the Cherry Trail driver on top of those?

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v5 0/4] i2c-core improvements for acpi devices + ACPI INT33FE driver

[Hans de Goede]

Hi,

On 13-04-17 21:09, Darren Hart wrote:
> On Wed, Apr 05, 2017 at 12:03:31AM +0200, Hans de Goede wrote:
>> Hi All,
>>
>> Here is v5 of my patch-set for my i2c-core improvements for acpi
>> devices + the ACPI INT33FE platform driver.
>>
>> New in this version are some minor fixed to the INT33FE driver.
>>
>> The INT33FE driver depends on the i2c-core changes, so if
>> it is ok with Darren / Andy I believe it would be best for
>> this driver to be merged through the i2c subsys tree (with
>> their ack).
>
> Is there an immutable i2c branch I can merge with these changes in order to add
> the Cherry Trail driver on top of those?

The i2c-core patches have been reviewed, but to the best of my
knowledge they have not been merged yet.

Wolfram, what is the status of this ?

Regards,

Hans