[PATCH v4 0/3] i2c: Hookup typec power-negotation to the PMIC and charger

[PATCH v4 0/3] i2c: Hookup typec power-negotation to the PMIC and charger

[Hans de Goede]

Hi Wolfram,

Almost all patches from my patch series for hooking up typec
power-negotation to the PMIC and charger drivers have been
queued for merging into 4.14, leaving only the 3 patches of
the v4 of this series.

The first 2 patches are i2c patches, if you could review and
merge these (preferably for 4.14, but 4.15 is fine too) that would
be great.

Darren, Andy, the single platform/x86 patch in here should only
be merged after the 2 i2c patches are in place, otherwise users
of the board(s) in question will end up not having any battery
monitoring. Also note that this patch applies on top of the
"[PATCH v2] platform/x86: intel_cht_int33fe: Work around BIOS bug on some devices"
patch I send out yesterday.

Regards,

Hans

[PATCH v4 1/3] i2c: Allow overriding dev_name through board_info

[Hans de Goede]

For devices not instantiated through ACPI the i2c-client's device-name
gets set to <busnr>-<addr> by default, e.g. "0-0022" this means that
the device-name is dependent on the order in which the i2c-busses are
enumerated.

In some cases having a predictable constant device-name is desirable,
for example on non device-tree platforms the link between a regulator
and its consumers is specified by the platform code by setting
regulator_init_data.consumers. This array identifies the regulator's
consumers by dev_name and supply(-name). Which requires a constant
dev_name.

This commit adds a dev_name field to i2c_board_info allowing
platform code to set a contstant dev_name so that the device can
be identified by its dev_name in other platform code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/i2c/i2c-core-base.c | 10 ++++++++--
 include/linux/i2c.h         |  2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 45fafcc88b93..5cdf3b947c23 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -672,10 +672,16 @@ static void i2c_adapter_unlock_bus(struct i2c_adapter *adapter,
 }
 
 static void i2c_dev_set_name(struct i2c_adapter *adap,
-			     struct i2c_client *client)
+			     struct i2c_client *client,
+			     struct i2c_board_info const *info)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
 
+	if (info && info->dev_name) {
+		dev_set_name(&client->dev, "i2c-%s", info->dev_name);
+		return;
+	}
+
 	if (adev) {
 		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
 		return;
@@ -772,7 +778,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	client->dev.fwnode = info->fwnode;
 
-	i2c_dev_set_name(adap, client);
+	i2c_dev_set_name(adap, client, info);
 
 	if (info->properties) {
 		status = device_add_properties(&client->dev, info->properties);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 701ad26fa6b4..d3655cfe9a3e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -310,6 +310,7 @@ static inline bool i2c_detect_slave_mode(struct device *dev) { return false; }
  * @type: chip type, to initialize i2c_client.name
  * @flags: to initialize i2c_client.flags
  * @addr: stored in i2c_client.addr
+ * @dev_name: Overrides the default <busnr>-<addr> dev_name if set
  * @platform_data: stored in i2c_client.dev.platform_data
  * @archdata: copied into i2c_client.dev.archdata
  * @of_node: pointer to OpenFirmware device node
@@ -334,6 +335,7 @@ struct i2c_board_info {
 	char		type[I2C_NAME_SIZE];
 	unsigned short	flags;
 	unsigned short	addr;
+	const char	*dev_name;
 	void		*platform_data;
 	struct dev_archdata	*archdata;
 	struct device_node *of_node;
-- 
2.13.4

[PATCH v4 2/3] i2c-cht-wc: Add device-properties for fusb302 integration

[Hans de Goede]

Add device-properties to make the bq24292i charger connected to
the bus get its input-current-limit from the fusb302 Type-C port
controller which is used on boards with the cht-wc PMIC,
as well as regulator_init_data for the 5V boost converter on
the bq24292i.

Since this means we now hook-up the bq24292i to the fusb302 Type-C port
controller add a check for the ACPI device which instantiates the fusb302.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Set board_info.dev_name
-Define and pass regulator_init_data for the bq24292i
-Add a check for the ACPI device which instantiates the fusb302
Changes in v4:
-Drop the ti,input-current-limit-from-supplier property on the bq24292i
 this is the default now
---
 drivers/i2c/busses/Kconfig      |  5 ++++
 drivers/i2c/busses/i2c-cht-wc.c | 51 +++++++++++++++++++++++++++++++++++------
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 79c33817e412..272ef10fb629 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -197,6 +197,11 @@ config I2C_CHT_WC
 	  SMBus controller found in the Intel Cherry Trail Whiskey Cove PMIC
 	  found on some Intel Cherry Trail systems.
 
+	  Note this controller is hooked up to a TI bq24292i charger-IC,
+	  combined with a FUSB302 Type-C port-controller as such it is advised
+	  to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m
+	  (the fusb302 driver currently is in drivers/staging).
+
 config I2C_NFORCE2
 	tristate "Nvidia nForce2, nForce3 and nForce4"
 	depends on PCI
diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
index 21312eed09e4..82e331db8d98 100644
--- a/drivers/i2c/busses/i2c-cht-wc.c
+++ b/drivers/i2c/busses/i2c-cht-wc.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/acpi.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
@@ -25,6 +26,7 @@
 #include <linux/mfd/intel_soc_pmic.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/power/bq24190_charger.h>
 #include <linux/slab.h>
 
 #define CHT_WC_I2C_CTRL			0x5e24
@@ -232,13 +234,35 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
 	.name			= "cht_wc_ext_chrg_irq_chip",
 };
 
+static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };
+
 static const struct property_entry bq24190_props[] = {
-	PROPERTY_ENTRY_STRING("extcon-name", "cht_wcove_pwrsrc"),
+	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq24190_suppliers),
 	PROPERTY_ENTRY_BOOL("omit-battery-class"),
 	PROPERTY_ENTRY_BOOL("disable-reset"),
 	{ }
 };
 
+static struct regulator_consumer_supply fusb302_consumer = {
+	.supply = "vbus",
+	/* Must match fusb302 dev_name in intel_cht_int33fe.c */
+	.dev_name = "i2c-fusb302",
+};
+
+static const struct regulator_init_data bq24190_vbus_init_data = {
+	.constraints = {
+		/* The name is used in intel_cht_int33fe.c do not change. */
+		.name = "cht_wc_usb_typec_vbus",
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.consumer_supplies = &fusb302_consumer,
+	.num_consumer_supplies = 1,
+};
+
+static struct bq24190_platform_data bq24190_pdata = {
+	.regulator_init_data = &bq24190_vbus_init_data,
+};
+
 static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
 {
 	struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
@@ -246,7 +270,9 @@ static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
 	struct i2c_board_info board_info = {
 		.type = "bq24190",
 		.addr = 0x6b,
+		.dev_name = "bq24190",
 		.properties = bq24190_props,
+		.platform_data = &bq24190_pdata,
 	};
 	int ret, reg, irq;
 
@@ -314,11 +340,21 @@ static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_irq_domain;
 
-	board_info.irq = adap->client_irq;
-	adap->client = i2c_new_device(&adap->adapter, &board_info);
-	if (!adap->client) {
-		ret = -ENOMEM;
-		goto del_adapter;
+	/*
+	 * Normally the Whiskey Cove PMIC is paired with a TI bq24292i charger,
+	 * connected to this i2c bus, and a max17047 fuel-gauge and a fusb302
+	 * USB Type-C controller connected to another i2c bus. In this setup
+	 * the max17047 and fusb302 devices are enumerated through an INT33FE
+	 * ACPI device. If this device is present register an i2c-client for
+	 * the TI bq24292i charger.
+	 */
+	if (acpi_dev_present("INT33FE", NULL, -1)) {
+		board_info.irq = adap->client_irq;
+		adap->client = i2c_new_device(&adap->adapter, &board_info);
+		if (!adap->client) {
+			ret = -ENOMEM;
+			goto del_adapter;
+		}
 	}
 
 	platform_set_drvdata(pdev, adap);
@@ -335,7 +371,8 @@ static int cht_wc_i2c_adap_i2c_remove(struct platform_device *pdev)
 {
 	struct cht_wc_i2c_adap *adap = platform_get_drvdata(pdev);
 
-	i2c_unregister_device(adap->client);
+	if (adap->client)
+		i2c_unregister_device(adap->client);
 	i2c_del_adapter(&adap->adapter);
 	irq_domain_remove(adap->irq_domain);
 
-- 
2.13.4

[PATCH v4 3/3] platform/x86: intel_cht_int33fe: Update fusb302 type string, add properties

[Hans de Goede]

The fusb302 driver as merged in staging uses "typec_fusb302" as i2c-id
rather then just "fusb302" and needs us to set a number of device-
properties, adjust the intel_cht_int33fe driver accordingly.

One of the properties set is max-snk-mv which makes the fusb302 driver
negotiate up to 12V charging voltage, which is a bad idea on boards
which are not setup to handle this, so this commit also adds 2 extra
sanity checks to make sure that the expected Whiskey Cove PMIC +
TI bq24292i charger combo, which can handle 12V, is present.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Set board_info.dev_name
-Adjust for changes in other patches in this patch-set
---
 drivers/platform/x86/Kconfig             |  6 ++++-
 drivers/platform/x86/intel_cht_int33fe.c | 44 +++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b87954f6dd..c5554577681a 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -793,7 +793,7 @@ config ACPI_CMPC
 
 config INTEL_CHT_INT33FE
 	tristate "Intel Cherry Trail ACPI INT33FE Driver"
-	depends on X86 && ACPI && I2C
+	depends on X86 && ACPI && I2C && REGULATOR
 	---help---
 	  This driver add support for the INT33FE ACPI device found on
 	  some Intel Cherry Trail devices.
@@ -804,6 +804,10 @@ config INTEL_CHT_INT33FE
 	  This driver instantiates i2c-clients for these, so that standard
 	  i2c drivers for these chips can bind to the them.
 
+	  If you enable this driver it is advised to also select
+	  CONFIG_CHARGER_BQ24190=m, CONFIG_BATTERY_MAX17042=m and
+	  CONFIG_TYPEC_FUSB302=m (currently in drivers/staging).
+
 config INTEL_INT0002_VGPIO
 	tristate "Intel ACPI INT0002 Virtual GPIO driver"
 	depends on GPIOLIB && ACPI
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index a9cbc4b8ca63..b2925d996613 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/regulator/consumer.h>
 #include <linux/slab.h>
 
 #define EXPECTED_PTYPE		4
@@ -77,12 +78,21 @@ static const struct property_entry max17047_props[] = {
 	{ }
 };
 
+static const struct property_entry fusb302_props[] = {
+	PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
+	PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
+	PROPERTY_ENTRY_U32("fcs,max-sink-microamp",   3000000),
+	PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 36000000),
+	{ }
+};
+
 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;
 	struct i2c_client *max17047;
+	struct regulator *regulator;
 	unsigned long long ptyp;
 	acpi_status status;
 	int ret, fusb302_irq;
@@ -100,6 +110,34 @@ static int cht_int33fe_probe(struct i2c_client *client)
 	if (ptyp != EXPECTED_PTYPE)
 		return -ENODEV;
 
+	/* Check presence of INT34D3 (hardware-rev 3) expected for ptype == 4 */
+	if (!acpi_dev_present("INT34D3", "1", 3)) {
+		dev_err(dev, "Error PTYPE == %d, but no INT34D3 device\n",
+			EXPECTED_PTYPE);
+		return -ENODEV;
+	}
+
+	/*
+	 * We expect the WC PMIC to be paired with a TI bq24292i charger-IC.
+	 * We check for the bq24292i vbus regulator here, this has 2 purposes:
+	 * 1) The bq24292i allows charging with up to 12V, setting the fusb302's
+	 *    max-snk voltage to 12V with another charger-IC is not good.
+	 * 2) For the fusb302 driver to get the bq24292i vbus regulator, the
+	 *    regulator-map, which is part of the bq24292i regulator_init_data,
+	 *    must be registered before the fusb302 is instantiated, otherwise
+	 *    it will end up with a dummy-regulator.
+	 * Note "cht_wc_usb_typec_vbus" comes from the regulator_init_data
+	 * which is defined in i2c-cht-wc.c from where the bq24292i i2c-client
+	 * gets instantiated. We use regulator_get_optional here so that we
+	 * don't end up getting a dummy-regulator ourselves.
+	 */
+	regulator = regulator_get_optional(dev, "cht_wc_usb_typec_vbus");
+	if (IS_ERR(regulator)) {
+		ret = PTR_ERR(regulator);
+		return (ret == -ENODEV) ? -EPROBE_DEFER : ret;
+	}
+	regulator_put(regulator);
+
 	/* The FUSB302 uses the irq at index 1 and is the only irq user */
 	fusb302_irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 1);
 	if (fusb302_irq < 0) {
@@ -126,6 +164,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
 	} else {
 		memset(&board_info, 0, sizeof(board_info));
 		strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
+		board_info.dev_name = "max17047";
 		board_info.properties = max17047_props;
 		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
 		if (!data->max17047)
@@ -133,7 +172,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
 	}
 
 	memset(&board_info, 0, sizeof(board_info));
-	strlcpy(board_info.type, "fusb302", I2C_NAME_SIZE);
+	strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
+	board_info.dev_name = "fusb302";
+	board_info.properties = fusb302_props;
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
@@ -141,6 +182,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
 		goto out_unregister_max17047;
 
 	memset(&board_info, 0, sizeof(board_info));
+	board_info.dev_name = "pi3usb30532";
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
-- 
2.13.4

Re: [PATCH v4 3/3] platform/x86: intel_cht_int33fe: Update fusb302 type string, add properties

[Andy Shevchenko]

On Sun, Sep 3, 2017 at 3:41 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The fusb302 driver as merged in staging uses "typec_fusb302" as i2c-id
> rather then just "fusb302" and needs us to set a number of device-
> properties, adjust the intel_cht_int33fe driver accordingly.
>
> One of the properties set is max-snk-mv which makes the fusb302 driver
> negotiate up to 12V charging voltage, which is a bad idea on boards
> which are not setup to handle this, so this commit also adds 2 extra
> sanity checks to make sure that the expected Whiskey Cove PMIC +
> TI bq24292i charger combo, which can handle 12V, is present.

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

(in case Wolfram would like to take it)

See comments below.

>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Set board_info.dev_name
> -Adjust for changes in other patches in this patch-set
> ---
>  drivers/platform/x86/Kconfig             |  6 ++++-
>  drivers/platform/x86/intel_cht_int33fe.c | 44 +++++++++++++++++++++++++++++++-
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 80b87954f6dd..c5554577681a 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -793,7 +793,7 @@ config ACPI_CMPC
>
>  config INTEL_CHT_INT33FE
>         tristate "Intel Cherry Trail ACPI INT33FE Driver"
> -       depends on X86 && ACPI && I2C
> +       depends on X86 && ACPI && I2C && REGULATOR
>         ---help---
>           This driver add support for the INT33FE ACPI device found on
>           some Intel Cherry Trail devices.
> @@ -804,6 +804,10 @@ config INTEL_CHT_INT33FE
>           This driver instantiates i2c-clients for these, so that standard
>           i2c drivers for these chips can bind to the them.
>
> +         If you enable this driver it is advised to also select
> +         CONFIG_CHARGER_BQ24190=m, CONFIG_BATTERY_MAX17042=m and
> +         CONFIG_TYPEC_FUSB302=m (currently in drivers/staging).
> +

I would put FUSB302 first since it's not obvious now that remark in
parens is related only to it. And might be better rephase the path in
terms of `make menuconfig` rather than pathname in the source tree.

>  config INTEL_INT0002_VGPIO
>         tristate "Intel ACPI INT0002 Virtual GPIO driver"
>         depends on GPIOLIB && ACPI
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
> index a9cbc4b8ca63..b2925d996613 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/regulator/consumer.h>
>  #include <linux/slab.h>
>
>  #define EXPECTED_PTYPE         4
> @@ -77,12 +78,21 @@ static const struct property_entry max17047_props[] = {
>         { }
>  };
>
> +static const struct property_entry fusb302_props[] = {
> +       PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
> +       PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
> +       PROPERTY_ENTRY_U32("fcs,max-sink-microamp",   3000000),
> +       PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 36000000),
> +       { }
> +};
> +
>  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;
>         struct i2c_client *max17047;
> +       struct regulator *regulator;
>         unsigned long long ptyp;
>         acpi_status status;
>         int ret, fusb302_irq;
> @@ -100,6 +110,34 @@ static int cht_int33fe_probe(struct i2c_client *client)
>         if (ptyp != EXPECTED_PTYPE)
>                 return -ENODEV;
>
> +       /* Check presence of INT34D3 (hardware-rev 3) expected for ptype == 4 */
> +       if (!acpi_dev_present("INT34D3", "1", 3)) {
> +               dev_err(dev, "Error PTYPE == %d, but no INT34D3 device\n",
> +                       EXPECTED_PTYPE);
> +               return -ENODEV;
> +       }
> +
> +       /*
> +        * We expect the WC PMIC to be paired with a TI bq24292i charger-IC.
> +        * We check for the bq24292i vbus regulator here, this has 2 purposes:
> +        * 1) The bq24292i allows charging with up to 12V, setting the fusb302's
> +        *    max-snk voltage to 12V with another charger-IC is not good.
> +        * 2) For the fusb302 driver to get the bq24292i vbus regulator, the
> +        *    regulator-map, which is part of the bq24292i regulator_init_data,
> +        *    must be registered before the fusb302 is instantiated, otherwise
> +        *    it will end up with a dummy-regulator.
> +        * Note "cht_wc_usb_typec_vbus" comes from the regulator_init_data
> +        * which is defined in i2c-cht-wc.c from where the bq24292i i2c-client
> +        * gets instantiated. We use regulator_get_optional here so that we
> +        * don't end up getting a dummy-regulator ourselves.
> +        */
> +       regulator = regulator_get_optional(dev, "cht_wc_usb_typec_vbus");
> +       if (IS_ERR(regulator)) {
> +               ret = PTR_ERR(regulator);
> +               return (ret == -ENODEV) ? -EPROBE_DEFER : ret;
> +       }
> +       regulator_put(regulator);
> +
>         /* The FUSB302 uses the irq at index 1 and is the only irq user */
>         fusb302_irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 1);
>         if (fusb302_irq < 0) {
> @@ -126,6 +164,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
>         } else {
>                 memset(&board_info, 0, sizeof(board_info));
>                 strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
> +               board_info.dev_name = "max17047";
>                 board_info.properties = max17047_props;
>                 data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
>                 if (!data->max17047)
> @@ -133,7 +172,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
>         }
>
>         memset(&board_info, 0, sizeof(board_info));
> -       strlcpy(board_info.type, "fusb302", I2C_NAME_SIZE);
> +       strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
> +       board_info.dev_name = "fusb302";
> +       board_info.properties = fusb302_props;
>         board_info.irq = fusb302_irq;
>
>         data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
> @@ -141,6 +182,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
>                 goto out_unregister_max17047;
>
>         memset(&board_info, 0, sizeof(board_info));
> +       board_info.dev_name = "pi3usb30532";
>         strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
>
>         data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
> --
> 2.13.4
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/3] platform/x86: intel_cht_int33fe: Update fusb302 type string, add properties

[Hans de Goede]

Hi,

On 04-09-17 08:27, Andy Shevchenko wrote:
> On Sun, Sep 3, 2017 at 3:41 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The fusb302 driver as merged in staging uses "typec_fusb302" as i2c-id
>> rather then just "fusb302" and needs us to set a number of device-
>> properties, adjust the intel_cht_int33fe driver accordingly.
>>
>> One of the properties set is max-snk-mv which makes the fusb302 driver
>> negotiate up to 12V charging voltage, which is a bad idea on boards
>> which are not setup to handle this, so this commit also adds 2 extra
>> sanity checks to make sure that the expected Whiskey Cove PMIC +
>> TI bq24292i charger combo, which can handle 12V, is present.
> 
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> (in case Wolfram would like to take it)
> 
> See comments below.
> 
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Set board_info.dev_name
>> -Adjust for changes in other patches in this patch-set
>> ---
>>   drivers/platform/x86/Kconfig             |  6 ++++-
>>   drivers/platform/x86/intel_cht_int33fe.c | 44 +++++++++++++++++++++++++++++++-
>>   2 files changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 80b87954f6dd..c5554577681a 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -793,7 +793,7 @@ config ACPI_CMPC
>>
>>   config INTEL_CHT_INT33FE
>>          tristate "Intel Cherry Trail ACPI INT33FE Driver"
>> -       depends on X86 && ACPI && I2C
>> +       depends on X86 && ACPI && I2C && REGULATOR
>>          ---help---
>>            This driver add support for the INT33FE ACPI device found on
>>            some Intel Cherry Trail devices.
>> @@ -804,6 +804,10 @@ config INTEL_CHT_INT33FE
>>            This driver instantiates i2c-clients for these, so that standard
>>            i2c drivers for these chips can bind to the them.
>>
>> +         If you enable this driver it is advised to also select
>> +         CONFIG_CHARGER_BQ24190=m, CONFIG_BATTERY_MAX17042=m and
>> +         CONFIG_TYPEC_FUSB302=m (currently in drivers/staging).
>> +
> 
> I would put FUSB302 first since it's not obvious now that remark in
> parens is related only to it. And might be better rephase the path in
> terms of `make menuconfig` rather than pathname in the source tree.

Ok, fixed for v5, which I will send right away.

Regards,

Hans



>>   config INTEL_INT0002_VGPIO
>>          tristate "Intel ACPI INT0002 Virtual GPIO driver"
>>          depends on GPIOLIB && ACPI
>> diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
>> index a9cbc4b8ca63..b2925d996613 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/regulator/consumer.h>
>>   #include <linux/slab.h>
>>
>>   #define EXPECTED_PTYPE         4
>> @@ -77,12 +78,21 @@ static const struct property_entry max17047_props[] = {
>>          { }
>>   };
>>
>> +static const struct property_entry fusb302_props[] = {
>> +       PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
>> +       PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
>> +       PROPERTY_ENTRY_U32("fcs,max-sink-microamp",   3000000),
>> +       PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 36000000),
>> +       { }
>> +};
>> +
>>   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;
>>          struct i2c_client *max17047;
>> +       struct regulator *regulator;
>>          unsigned long long ptyp;
>>          acpi_status status;
>>          int ret, fusb302_irq;
>> @@ -100,6 +110,34 @@ static int cht_int33fe_probe(struct i2c_client *client)
>>          if (ptyp != EXPECTED_PTYPE)
>>                  return -ENODEV;
>>
>> +       /* Check presence of INT34D3 (hardware-rev 3) expected for ptype == 4 */
>> +       if (!acpi_dev_present("INT34D3", "1", 3)) {
>> +               dev_err(dev, "Error PTYPE == %d, but no INT34D3 device\n",
>> +                       EXPECTED_PTYPE);
>> +               return -ENODEV;
>> +       }
>> +
>> +       /*
>> +        * We expect the WC PMIC to be paired with a TI bq24292i charger-IC.
>> +        * We check for the bq24292i vbus regulator here, this has 2 purposes:
>> +        * 1) The bq24292i allows charging with up to 12V, setting the fusb302's
>> +        *    max-snk voltage to 12V with another charger-IC is not good.
>> +        * 2) For the fusb302 driver to get the bq24292i vbus regulator, the
>> +        *    regulator-map, which is part of the bq24292i regulator_init_data,
>> +        *    must be registered before the fusb302 is instantiated, otherwise
>> +        *    it will end up with a dummy-regulator.
>> +        * Note "cht_wc_usb_typec_vbus" comes from the regulator_init_data
>> +        * which is defined in i2c-cht-wc.c from where the bq24292i i2c-client
>> +        * gets instantiated. We use regulator_get_optional here so that we
>> +        * don't end up getting a dummy-regulator ourselves.
>> +        */
>> +       regulator = regulator_get_optional(dev, "cht_wc_usb_typec_vbus");
>> +       if (IS_ERR(regulator)) {
>> +               ret = PTR_ERR(regulator);
>> +               return (ret == -ENODEV) ? -EPROBE_DEFER : ret;
>> +       }
>> +       regulator_put(regulator);
>> +
>>          /* The FUSB302 uses the irq at index 1 and is the only irq user */
>>          fusb302_irq = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), 1);
>>          if (fusb302_irq < 0) {
>> @@ -126,6 +164,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
>>          } else {
>>                  memset(&board_info, 0, sizeof(board_info));
>>                  strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
>> +               board_info.dev_name = "max17047";
>>                  board_info.properties = max17047_props;
>>                  data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
>>                  if (!data->max17047)
>> @@ -133,7 +172,9 @@ static int cht_int33fe_probe(struct i2c_client *client)
>>          }
>>
>>          memset(&board_info, 0, sizeof(board_info));
>> -       strlcpy(board_info.type, "fusb302", I2C_NAME_SIZE);
>> +       strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
>> +       board_info.dev_name = "fusb302";
>> +       board_info.properties = fusb302_props;
>>          board_info.irq = fusb302_irq;
>>
>>          data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
>> @@ -141,6 +182,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
>>                  goto out_unregister_max17047;
>>
>>          memset(&board_info, 0, sizeof(board_info));
>> +       board_info.dev_name = "pi3usb30532";
>>          strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
>>
>>          data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
>> --
>> 2.13.4
>>
> 
> 
> 

Re: [PATCH v4 0/3] i2c: Hookup typec power-negotation to the PMIC and charger

[Wolfram Sang]

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

Hi,

> The first 2 patches are i2c patches, if you could review and
> merge these (preferably for 4.14, but 4.15 is fine too) that would
> be great.

Definately for v4.15 and I very likely won't be able to review them
before rc1 or rc2 time, if even that. Sorry, but I2C core changes need
extra careful review and I2C maintenance is largely done in my limited
spare time. If you could get other people to review/tag the patches,
this would be very helpful.

> Darren, Andy, the single platform/x86 patch in here should only
> be merged after the 2 i2c patches are in place, otherwise users
> of the board(s) in question will end up not having any battery
> monitoring. Also note that this patch applies on top of the
> "[PATCH v2] platform/x86: intel_cht_int33fe: Work around BIOS bug on some devices"
> patch I send out yesterday.

Is that dependency for v4.14? Would it be an idea if I take the platform
patch via the i2c tree then?

Regards,

   Wolfram


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

Re: [PATCH v4 0/3] i2c: Hookup typec power-negotation to the PMIC and charger

[Hans de Goede]

Hi,

On 04-09-17 06:56, Wolfram Sang wrote:
> Hi,
> 
>> The first 2 patches are i2c patches, if you could review and
>> merge these (preferably for 4.14, but 4.15 is fine too) that would
>> be great.
> 
> Definately for v4.15 and I very likely won't be able to review them
> before rc1 or rc2 time, if even that. Sorry, but I2C core changes need
> extra careful review and I2C maintenance is largely done in my limited
> spare time.

Ok, I understand.

> If you could get other people to review/tag the patches,
> this would be very helpful.

FWIW althought the first patch is a core change it is not really a big
one and won't impact any existing drivers, but I fully understand
you still want to take your time to review it.

Andy (Shevchenko) can you perhaps take a look at the first patch
in this series and give your opinion on it ?

>> Darren, Andy, the single platform/x86 patch in here should only
>> be merged after the 2 i2c patches are in place, otherwise users
>> of the board(s) in question will end up not having any battery
>> monitoring. Also note that this patch applies on top of the
>> "[PATCH v2] platform/x86: intel_cht_int33fe: Work around BIOS bug on some devices"
>> patch I send out yesterday.
> 
> Is that dependency for v4.14?

Yes that dep should make 4.14.

> Would it be an idea if I take the platform
> patch via the i2c tree then?

I've some other patches pending which require further changes to the
intel_cht_int33fe driver (*), so I think it would be best if the last
patch got merged through the platform/x86 tree. The dependencies
between patch 2-3 are runtime only (and not fatal if missing, just
inconvenient) so merging this to 2 separate trees should be fine
as long as both patches get merged for 4.15.

Regards,

Hans


*) The intel_cht_int33fe and i2c/busses/i2c-cht-wc drivers together
instantiate 4 i2c devices which together make all the "magic" behind
the Type-C connector on these boards happen. The patch-series
completed by these 3 patches ties 3 of the 4 drivers together so
that charging and battery monitoring fully works. Ideally all 4
i2c-clients would be instantiated from a single place, e.g.
intel_cht_int33fe.c, but the charger is hooked up through a "child"
i2c bus on the pmic, which is where i2c/busses/i2c-cht-wc.c
comes into play.

The next series adds support for the 4th chip, the type-c cross-switch/
mux which is necessary to get superspeed usb and/or displayport working
over the Type-C connector.

Re: [PATCH v4 0/3] i2c: Hookup typec power-negotation to the PMIC and charger

[Wolfram Sang]

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


> I've some other patches pending which require further changes to the
> intel_cht_int33fe driver (*), so I think it would be best if the last
> patch got merged through the platform/x86 tree. The dependencies
> between patch 2-3 are runtime only (and not fatal if missing, just
> inconvenient) so merging this to 2 separate trees should be fine
> as long as both patches get merged for 4.15.

OK. So, I'll just look at the first two patches. You'll know when I done
this.


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