[PATCH v2 00/14] Hookup typec power-negotation to the PMIC and charger

[PATCH v2 00/14] Hookup typec power-negotation to the PMIC and charger

[Hans de Goede]

Hi All,

This series implements a number of typec changes discussed a while back:

- It exports the negotiated voltage and max-current in the form of a
  power-supply class device which represents the USB Type-C power-brick
  (adapter/charger)
- It adds a power_supply_set_input_current_limit_from_supplier helper
  function which charger drivers can use to get the max-current from
  their supplier
- It adds regulator support to the charger IC on the device I've. The
  exported regulator controls the 5v boost convertor which generates the
  5V USB vbus which gets output when the Type-C port is in host / power-src
  mode
- It adds a bunch of misc. related fixes and glue code to tie everything
  together

One thing which was undecided in the previous discussion was how to make
port-controller drivers hookup to external ICs (e.g. a non Type-C aware PMIC)
to decect the input-current-limit for USB2 power-sources (through e.g. BC1.2
detection). Since a number of existing drivers, including the one for the
PMIC used on the 2 mini laptops I'm working on, already use the extcon
framework to communicate the detected USB2 charger-type, I've decided to
simply hook into this existing code. As this patch set shows this can be
done with zero changes to the existing PMIC/extcon drivers.

With this series the GPD win and GPD pocket mini laptops both fully
support any type of Type-C charging. When hooked up with:
-A -> C cable and plugged into a regular port they charge at 5V 0.5A
-A -> C cable and plugged into a dedictaed charger they charge at 5V 2A
-C -> C cable and plugged into a fixed 5V 3A charger, at 5V 3A
-C -> C cable and plugged into a PD capable charger, which delivers max 12V, 2A
 they charge at 12V, 2A

And when a Type-C to USB-A receptacle (so host mode) cable gets plugged in
the port correctly supplies 5V to any plugged in USB-A peripherals.

This is v2 of this series, which has the following changes (see
changelog inside individual patches for details):

-Add "i2c: Allow overriding dev_name through board_info" patch, this is
 necessary for getting stable dev_names which are necessary for specifying
 regulator-mappings through regulator_init_data
-Use regulator_init_data to specify mapping,  drop "staging: typec:
 fusb302: Add support for fcs,vbus-regulator-name device-property" patch
-Merged helper code for port-c related extcon / power_supply handling
 directly into the fusb302 patches using the code, rather then trying
 to add generic helpers even though there is only 1 user

Patches 1-12 can be merged directly into their resp. subsystems,
patches 13 and 14 need to wait for the others to be merged first.

Regards,

Hans

[PATCH v2 01/14] 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 1d28454..22dc8ba 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 701ad26..d3655cf 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.9.4

[PATCH v2 01/14] 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 1d28454..22dc8ba 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 701ad26..d3655cf 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.9.4

[PATCH v2 02/14] staging: typec: tcpm: Add get_current_limit tcpc_dev callback

A Rp signalling the default current limit indicates that we're possibly
connected to an USB2 power-source. In some cases the type-c port-controller
may provide the capability to detect the current-limit in this case,
through e.g. BC1.2 detection.

This commit adds an optional get_current_limit tcpc_dev callback which
allows the port-controller to provide current-limit detection for when
the CC pin is pulled up with Rp.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-s/get_usb2_current_limit/get_current_limit/
---
 drivers/staging/typec/tcpm.c | 5 ++++-
 drivers/staging/typec/tcpm.h | 7 +++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4eb..8e823af 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -660,7 +660,10 @@ static u32 tcpm_get_current_limit(struct tcpm_port *port)
 		break;
 	case TYPEC_CC_RP_DEF:
 	default:
-		limit = 0;
+		if (port->tcpc->get_current_limit)
+			limit = port->tcpc->get_current_limit(port->tcpc);
+		else
+			limit = 0;
 		break;
 	}
 
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 19c307d..1465668 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -108,6 +108,13 @@ struct tcpc_dev {
 
 	int (*init)(struct tcpc_dev *dev);
 	int (*get_vbus)(struct tcpc_dev *dev);
+	/*
+	 * This optional callback gets called by the tcpm core when configured
+	 * as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
+	 * current-limit detection method for the cc=Rp-def case. E.g. some
+	 * tcpcs may include BC1.2 charger detection and use that in this case.
+	 */
+	int (*get_current_limit)(struct tcpc_dev *dev);
 	int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
 	int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
 		      enum typec_cc_status *cc2);
-- 
2.9.4

[PATCH v2 02/14] staging: typec: tcpm: Add get_current_limit tcpc_dev callback

[Hans de Goede]

A Rp signalling the default current limit indicates that we're possibly
connected to an USB2 power-source. In some cases the type-c port-controller
may provide the capability to detect the current-limit in this case,
through e.g. BC1.2 detection.

This commit adds an optional get_current_limit tcpc_dev callback which
allows the port-controller to provide current-limit detection for when
the CC pin is pulled up with Rp.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-s/get_usb2_current_limit/get_current_limit/
---
 drivers/staging/typec/tcpm.c | 5 ++++-
 drivers/staging/typec/tcpm.h | 7 +++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4eb..8e823af 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -660,7 +660,10 @@ static u32 tcpm_get_current_limit(struct tcpm_port *port)
 		break;
 	case TYPEC_CC_RP_DEF:
 	default:
-		limit = 0;
+		if (port->tcpc->get_current_limit)
+			limit = port->tcpc->get_current_limit(port->tcpc);
+		else
+			limit = 0;
 		break;
 	}
 
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 19c307d..1465668 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -108,6 +108,13 @@ struct tcpc_dev {
 
 	int (*init)(struct tcpc_dev *dev);
 	int (*get_vbus)(struct tcpc_dev *dev);
+	/*
+	 * This optional callback gets called by the tcpm core when configured
+	 * as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
+	 * current-limit detection method for the cc=Rp-def case. E.g. some
+	 * tcpcs may include BC1.2 charger detection and use that in this case.
+	 */
+	int (*get_current_limit)(struct tcpc_dev *dev);
 	int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
 	int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
 		      enum typec_cc_status *cc2);
-- 
2.9.4

Re: [PATCH v2 02/14] staging: typec: tcpm: Add get_current_limit tcpc_dev callback

[Guenter Roeck]

On 08/15/2017 01:04 PM, Hans de Goede wrote:
> A Rp signalling the default current limit indicates that we're possibly
> connected to an USB2 power-source. In some cases the type-c port-controller
> may provide the capability to detect the current-limit in this case,
> through e.g. BC1.2 detection.
> 
> This commit adds an optional get_current_limit tcpc_dev callback which
> allows the port-controller to provide current-limit detection for when
> the CC pin is pulled up with Rp.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v2:
> -s/get_usb2_current_limit/get_current_limit/
> ---
>   drivers/staging/typec/tcpm.c | 5 ++++-
>   drivers/staging/typec/tcpm.h | 7 +++++++
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 20eb4eb..8e823af 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -660,7 +660,10 @@ static u32 tcpm_get_current_limit(struct tcpm_port *port)
>   		break;
>   	case TYPEC_CC_RP_DEF:
>   	default:
> -		limit = 0;
> +		if (port->tcpc->get_current_limit)
> +			limit = port->tcpc->get_current_limit(port->tcpc);
> +		else
> +			limit = 0;
>   		break;
>   	}
>   
> diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
> index 19c307d..1465668 100644
> --- a/drivers/staging/typec/tcpm.h
> +++ b/drivers/staging/typec/tcpm.h
> @@ -108,6 +108,13 @@ struct tcpc_dev {
>   
>   	int (*init)(struct tcpc_dev *dev);
>   	int (*get_vbus)(struct tcpc_dev *dev);
> +	/*
> +	 * This optional callback gets called by the tcpm core when configured
> +	 * as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
> +	 * current-limit detection method for the cc=Rp-def case. E.g. some
> +	 * tcpcs may include BC1.2 charger detection and use that in this case.
> +	 */
> +	int (*get_current_limit)(struct tcpc_dev *dev);
>   	int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
>   	int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
>   		      enum typec_cc_status *cc2);
> 

[PATCH v2 03/14] staging: typec: fusb302: Set max supply voltage to 5V

[Hans de Goede]

Anything higher then 5V may damage hardware not capable of it, so
the only sane default here is 5V. If a board is able to handle a
higher voltage that should come from board specific data such as
device-tree and not be hard coded into the fusb302 code.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/fusb302/fusb302.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index 03a3809..6baed06 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1187,9 +1187,9 @@ static const struct tcpc_config fusb302_tcpc_config = {
 	.nr_src_pdo = ARRAY_SIZE(src_pdo),
 	.snk_pdo = snk_pdo,
 	.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
-	.max_snk_mv = 9000,
+	.max_snk_mv = 5000,
 	.max_snk_ma = 3000,
-	.max_snk_mw = 27000,
+	.max_snk_mw = 15000,
 	.operating_snk_mw = 2500,
 	.type = TYPEC_PORT_DRP,
 	.default_role = TYPEC_SINK,
-- 
2.9.4

[PATCH v2 03/14] staging: typec: fusb302: Set max supply voltage to 5V

[Hans de Goede]

Anything higher then 5V may damage hardware not capable of it, so
the only sane default here is 5V. If a board is able to handle a
higher voltage that should come from board specific data such as
device-tree and not be hard coded into the fusb302 code.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/fusb302/fusb302.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index 03a3809..6baed06 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1187,9 +1187,9 @@ static const struct tcpc_config fusb302_tcpc_config = {
 	.nr_src_pdo = ARRAY_SIZE(src_pdo),
 	.snk_pdo = snk_pdo,
 	.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
-	.max_snk_mv = 9000,
+	.max_snk_mv = 5000,
 	.max_snk_ma = 3000,
-	.max_snk_mw = 27000,
+	.max_snk_mw = 15000,
 	.operating_snk_mw = 2500,
 	.type = TYPEC_PORT_DRP,
 	.default_role = TYPEC_SINK,
-- 
2.9.4

[PATCH v2 04/14] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Hans de Goede]

This is board specific info so it should come from board config, such
as devicetree.

I've chosen to prefix these with "fcs," treating them as fusb302 driver
specific for now. We may want to revisit this and replace these with
properties which are part of a (to be written) generic type-c controller
devicetree binding.

Since this commit adds new dt-properties it also adds devicetree-bindings
documentation (which so far was absent for the fusb302 driver).

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use micro... instead of mili...
-Add devicetree bindings documentation
---
 .../devicetree/bindings/usb/fcs,fusb302.txt        | 29 ++++++++++++++++++++++
 drivers/staging/typec/fusb302/TODO                 |  4 +++
 drivers/staging/typec/fusb302/fusb302.c            | 18 +++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/fcs,fusb302.txt

diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
new file mode 100644
index 0000000..ffc6c87
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -0,0 +1,29 @@
+Fairchild FUSB302 Type-C Port controllers
+
+Required properties :
+- compatible            : "fcs,fusb302"
+- reg                   : I2C slave address
+- interrupts            : Interrupt specifier
+
+Optional properties :
+- fcs,max-snk-microvolt : Maximum voltage to negotiate when configured as sink
+- fcs,max-snk-microamp  : Maximum current to negotiate when configured as sink
+- fcs,max-snk-microwatt : Maximum power to negotiate when configured as sink
+			  If this is less then max-snk-microvolt *
+			  max-snk-microamp then the configured current will
+			  be clamped.
+- fcs,operating-snk-microwatt :
+                          Minimum amount of power accepted from a sink
+			  when negotiating
+
+Example:
+
+fusb302: typec-portc@54 {
+	compatible = "fcs,fusb302";
+	reg = <0x54>;
+	interrupt-parent = <&nmi_intc>;
+	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	fcs,max-snk-microvolt = <12000000>;
+	fcs,max-snk-microamp = <3000000>;
+	fcs,max-snk-microwatt = <36000000>;
+};
diff --git a/drivers/staging/typec/fusb302/TODO b/drivers/staging/typec/fusb302/TODO
index 4933a1d..19b466e 100644
--- a/drivers/staging/typec/fusb302/TODO
+++ b/drivers/staging/typec/fusb302/TODO
@@ -4,3 +4,7 @@ fusb302:
 - Find a non-hacky way to coordinate between PM and I2C access
 - Documentation? The FUSB302 datasheet provides information on the chip to help
   understand the code. But it may still be helpful to have a documentation.
+- We may want to replace the  "fcs,max-snk-microvolt", "fcs,max-snk-microamp",
+  "fcs,max-snk-microwatt" and "fcs,operating-snk-microwatt" device(tree)
+  properties with properties which are part of a generic type-c controller
+  devicetree binding.
diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index 6baed06..e1efc67 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -90,6 +90,7 @@ struct fusb302_chip {
 	struct i2c_client *i2c_client;
 	struct tcpm_port *tcpm_port;
 	struct tcpc_dev tcpc_dev;
+	struct tcpc_config tcpc_config;
 
 	struct regulator *vbus;
 
@@ -1198,7 +1199,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
 
 static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 {
-	fusb302_tcpc_dev->config = &fusb302_tcpc_config;
 	fusb302_tcpc_dev->init = tcpm_init;
 	fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
 	fusb302_tcpc_dev->set_cc = tcpm_set_cc;
@@ -1684,7 +1684,9 @@ static int fusb302_probe(struct i2c_client *client,
 {
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter;
+	struct device *dev = &client->dev;
 	int ret = 0;
+	u32 val;
 
 	adapter = to_i2c_adapter(client->dev.parent);
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -1699,8 +1701,22 @@ static int fusb302_probe(struct i2c_client *client,
 	chip->i2c_client = client;
 	i2c_set_clientdata(client, chip);
 	chip->dev = &client->dev;
+	chip->tcpc_config = fusb302_tcpc_config;
+	chip->tcpc_dev.config = &chip->tcpc_config;
 	mutex_init(&chip->lock);
 
+	if (!device_property_read_u32(dev, "fcs,max-snk-microvolt", &val))
+		chip->tcpc_config.max_snk_mv = val / 1000;
+
+	if (!device_property_read_u32(dev, "fcs,max-snk-microamp", &val))
+		chip->tcpc_config.max_snk_ma = val / 1000;
+
+	if (!device_property_read_u32(dev, "fcs,max-snk-microwatt", &val))
+		chip->tcpc_config.max_snk_mw = val / 1000;
+
+	if (!device_property_read_u32(dev, "fcs,operating-snk-microwatt", &val))
+		chip->tcpc_config.operating_snk_mw = val / 1000;
+
 	ret = fusb302_debugfs_init(chip);
 	if (ret < 0)
 		return ret;
-- 
2.9.4

[PATCH v2 04/14] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Hans de Goede]

This is board specific info so it should come from board config, such
as devicetree.

I've chosen to prefix these with "fcs," treating them as fusb302 driver
specific for now. We may want to revisit this and replace these with
properties which are part of a (to be written) generic type-c controller
devicetree binding.

Since this commit adds new dt-properties it also adds devicetree-bindings
documentation (which so far was absent for the fusb302 driver).

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use micro... instead of mili...
-Add devicetree bindings documentation
---
 .../devicetree/bindings/usb/fcs,fusb302.txt        | 29 ++++++++++++++++++++++
 drivers/staging/typec/fusb302/TODO                 |  4 +++
 drivers/staging/typec/fusb302/fusb302.c            | 18 +++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/usb/fcs,fusb302.txt

diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
new file mode 100644
index 0000000..ffc6c87
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -0,0 +1,29 @@
+Fairchild FUSB302 Type-C Port controllers
+
+Required properties :
+- compatible            : "fcs,fusb302"
+- reg                   : I2C slave address
+- interrupts            : Interrupt specifier
+
+Optional properties :
+- fcs,max-snk-microvolt : Maximum voltage to negotiate when configured as sink
+- fcs,max-snk-microamp  : Maximum current to negotiate when configured as sink
+- fcs,max-snk-microwatt : Maximum power to negotiate when configured as sink
+			  If this is less then max-snk-microvolt *
+			  max-snk-microamp then the configured current will
+			  be clamped.
+- fcs,operating-snk-microwatt :
+                          Minimum amount of power accepted from a sink
+			  when negotiating
+
+Example:
+
+fusb302: typec-portc@54 {
+	compatible = "fcs,fusb302";
+	reg = <0x54>;
+	interrupt-parent = <&nmi_intc>;
+	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	fcs,max-snk-microvolt = <12000000>;
+	fcs,max-snk-microamp = <3000000>;
+	fcs,max-snk-microwatt = <36000000>;
+};
diff --git a/drivers/staging/typec/fusb302/TODO b/drivers/staging/typec/fusb302/TODO
index 4933a1d..19b466e 100644
--- a/drivers/staging/typec/fusb302/TODO
+++ b/drivers/staging/typec/fusb302/TODO
@@ -4,3 +4,7 @@ fusb302:
 - Find a non-hacky way to coordinate between PM and I2C access
 - Documentation? The FUSB302 datasheet provides information on the chip to help
   understand the code. But it may still be helpful to have a documentation.
+- We may want to replace the  "fcs,max-snk-microvolt", "fcs,max-snk-microamp",
+  "fcs,max-snk-microwatt" and "fcs,operating-snk-microwatt" device(tree)
+  properties with properties which are part of a generic type-c controller
+  devicetree binding.
diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index 6baed06..e1efc67 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -90,6 +90,7 @@ struct fusb302_chip {
 	struct i2c_client *i2c_client;
 	struct tcpm_port *tcpm_port;
 	struct tcpc_dev tcpc_dev;
+	struct tcpc_config tcpc_config;
 
 	struct regulator *vbus;
 
@@ -1198,7 +1199,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
 
 static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 {
-	fusb302_tcpc_dev->config = &fusb302_tcpc_config;
 	fusb302_tcpc_dev->init = tcpm_init;
 	fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
 	fusb302_tcpc_dev->set_cc = tcpm_set_cc;
@@ -1684,7 +1684,9 @@ static int fusb302_probe(struct i2c_client *client,
 {
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter;
+	struct device *dev = &client->dev;
 	int ret = 0;
+	u32 val;
 
 	adapter = to_i2c_adapter(client->dev.parent);
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -1699,8 +1701,22 @@ static int fusb302_probe(struct i2c_client *client,
 	chip->i2c_client = client;
 	i2c_set_clientdata(client, chip);
 	chip->dev = &client->dev;
+	chip->tcpc_config = fusb302_tcpc_config;
+	chip->tcpc_dev.config = &chip->tcpc_config;
 	mutex_init(&chip->lock);
 
+	if (!device_property_read_u32(dev, "fcs,max-snk-microvolt", &val))
+		chip->tcpc_config.max_snk_mv = val / 1000;
+
+	if (!device_property_read_u32(dev, "fcs,max-snk-microamp", &val))
+		chip->tcpc_config.max_snk_ma = val / 1000;
+
+	if (!device_property_read_u32(dev, "fcs,max-snk-microwatt", &val))
+		chip->tcpc_config.max_snk_mw = val / 1000;
+
+	if (!device_property_read_u32(dev, "fcs,operating-snk-microwatt", &val))
+		chip->tcpc_config.operating_snk_mw = val / 1000;
+
 	ret = fusb302_debugfs_init(chip);
 	if (ret < 0)
 		return ret;
-- 
2.9.4

Re: [PATCH v2 04/14] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Rob Herring]

On Tue, Aug 15, 2017 at 10:04:52PM +0200, Hans de Goede wrote:
> This is board specific info so it should come from board config, such
> as devicetree.
> 
> I've chosen to prefix these with "fcs," treating them as fusb302 driver
> specific for now. We may want to revisit this and replace these with
> properties which are part of a (to be written) generic type-c controller
> devicetree binding.
> 
> Since this commit adds new dt-properties it also adds devicetree-bindings
> documentation (which so far was absent for the fusb302 driver).
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: devicetree@vger.kernel.org
> Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Use micro... instead of mili...
> -Add devicetree bindings documentation
> ---
>  .../devicetree/bindings/usb/fcs,fusb302.txt        | 29 ++++++++++++++++++++++
>  drivers/staging/typec/fusb302/TODO                 |  4 +++
>  drivers/staging/typec/fusb302/fusb302.c            | 18 +++++++++++++-
>  3 files changed, 50 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> new file mode 100644
> index 0000000..ffc6c87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> @@ -0,0 +1,29 @@
> +Fairchild FUSB302 Type-C Port controllers
> +
> +Required properties :
> +- compatible            : "fcs,fusb302"
> +- reg                   : I2C slave address
> +- interrupts            : Interrupt specifier
> +
> +Optional properties :
> +- fcs,max-snk-microvolt : Maximum voltage to negotiate when configured as sink
> +- fcs,max-snk-microamp  : Maximum current to negotiate when configured as sink
> +- fcs,max-snk-microwatt : Maximum power to negotiate when configured as sink
> +			  If this is less then max-snk-microvolt *
> +			  max-snk-microamp then the configured current will
> +			  be clamped.
> +- fcs,operating-snk-microwatt :

Might as well spell out sink.

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

> +                          Minimum amount of power accepted from a sink
> +			  when negotiating
> +
> +Example:
> +
> +fusb302: typec-portc@54 {
> +	compatible = "fcs,fusb302";
> +	reg = <0x54>;
> +	interrupt-parent = <&nmi_intc>;
> +	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +	fcs,max-snk-microvolt = <12000000>;
> +	fcs,max-snk-microamp = <3000000>;
> +	fcs,max-snk-microwatt = <36000000>;
> +};

Re: [PATCH v2 04/14] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Rob Herring]

On Tue, Aug 15, 2017 at 10:04:52PM +0200, Hans de Goede wrote:
> This is board specific info so it should come from board config, such
> as devicetree.
> 
> I've chosen to prefix these with "fcs," treating them as fusb302 driver
> specific for now. We may want to revisit this and replace these with
> properties which are part of a (to be written) generic type-c controller
> devicetree binding.
> 
> Since this commit adds new dt-properties it also adds devicetree-bindings
> documentation (which so far was absent for the fusb302 driver).
> 
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: "Yueyao (Nathan) Zhu" <yueyao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Use micro... instead of mili...
> -Add devicetree bindings documentation
> ---
>  .../devicetree/bindings/usb/fcs,fusb302.txt        | 29 ++++++++++++++++++++++
>  drivers/staging/typec/fusb302/TODO                 |  4 +++
>  drivers/staging/typec/fusb302/fusb302.c            | 18 +++++++++++++-
>  3 files changed, 50 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> new file mode 100644
> index 0000000..ffc6c87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> @@ -0,0 +1,29 @@
> +Fairchild FUSB302 Type-C Port controllers
> +
> +Required properties :
> +- compatible            : "fcs,fusb302"
> +- reg                   : I2C slave address
> +- interrupts            : Interrupt specifier
> +
> +Optional properties :
> +- fcs,max-snk-microvolt : Maximum voltage to negotiate when configured as sink
> +- fcs,max-snk-microamp  : Maximum current to negotiate when configured as sink
> +- fcs,max-snk-microwatt : Maximum power to negotiate when configured as sink
> +			  If this is less then max-snk-microvolt *
> +			  max-snk-microamp then the configured current will
> +			  be clamped.
> +- fcs,operating-snk-microwatt :

Might as well spell out sink.

Otherwise,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> +                          Minimum amount of power accepted from a sink
> +			  when negotiating
> +
> +Example:
> +
> +fusb302: typec-portc@54 {
> +	compatible = "fcs,fusb302";
> +	reg = <0x54>;
> +	interrupt-parent = <&nmi_intc>;
> +	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +	fcs,max-snk-microvolt = <12000000>;
> +	fcs,max-snk-microamp = <3000000>;
> +	fcs,max-snk-microwatt = <36000000>;
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/14] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Hans de Goede]

Hi,

On 17-08-17 23:41, Rob Herring wrote:
> On Tue, Aug 15, 2017 at 10:04:52PM +0200, Hans de Goede wrote:
>> This is board specific info so it should come from board config, such
>> as devicetree.
>>
>> I've chosen to prefix these with "fcs," treating them as fusb302 driver
>> specific for now. We may want to revisit this and replace these with
>> properties which are part of a (to be written) generic type-c controller
>> devicetree binding.
>>
>> Since this commit adds new dt-properties it also adds devicetree-bindings
>> documentation (which so far was absent for the fusb302 driver).
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Frank Rowand <frowand.list@gmail.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Use micro... instead of mili...
>> -Add devicetree bindings documentation
>> ---
>>   .../devicetree/bindings/usb/fcs,fusb302.txt        | 29 ++++++++++++++++++++++
>>   drivers/staging/typec/fusb302/TODO                 |  4 +++
>>   drivers/staging/typec/fusb302/fusb302.c            | 18 +++++++++++++-
>>   3 files changed, 50 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/devicetree/bindings/usb/fcs,fusb302.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
>> new file mode 100644
>> index 0000000..ffc6c87
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
>> @@ -0,0 +1,29 @@
>> +Fairchild FUSB302 Type-C Port controllers
>> +
>> +Required properties :
>> +- compatible            : "fcs,fusb302"
>> +- reg                   : I2C slave address
>> +- interrupts            : Interrupt specifier
>> +
>> +Optional properties :
>> +- fcs,max-snk-microvolt : Maximum voltage to negotiate when configured as sink
>> +- fcs,max-snk-microamp  : Maximum current to negotiate when configured as sink
>> +- fcs,max-snk-microwatt : Maximum power to negotiate when configured as sink
>> +			  If this is less then max-snk-microvolt *
>> +			  max-snk-microamp then the configured current will
>> +			  be clamped.
>> +- fcs,operating-snk-microwatt :
> 
> Might as well spell out sink.

Fixed for v3.

> Otherwise,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thank you, added to v3 of this patch-set.

Regards,

Hans

> 
>> +                          Minimum amount of power accepted from a sink
>> +			  when negotiating
>> +
>> +Example:
>> +
>> +fusb302: typec-portc@54 {
>> +	compatible = "fcs,fusb302";
>> +	reg = <0x54>;
>> +	interrupt-parent = <&nmi_intc>;
>> +	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>> +	fcs,max-snk-microvolt = <12000000>;
>> +	fcs,max-snk-microamp = <3000000>;
>> +	fcs,max-snk-microwatt = <36000000>;
>> +};

[PATCH v2 05/14] staging: typec: fusb302: Use client->irq as irq if set

[Hans de Goede]

The fusb302 is also used on x86 systems where the platform code sets
the irq in client->irq and there is no gpio named fcs,int_n.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/fusb302/fusb302.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index e1efc67..dea88fd 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1735,9 +1735,13 @@ static int fusb302_probe(struct i2c_client *client,
 		goto destroy_workqueue;
 	}
 
-	ret = init_gpio(chip);
-	if (ret < 0)
-		goto destroy_workqueue;
+	if (client->irq) {
+		chip->gpio_int_n_irq = client->irq;
+	} else {
+		ret = init_gpio(chip);
+		if (ret < 0)
+			goto destroy_workqueue;
+	}
 
 	chip->tcpm_port = tcpm_register_port(&client->dev, &chip->tcpc_dev);
 	if (IS_ERR(chip->tcpm_port)) {
-- 
2.9.4

[PATCH v2 05/14] staging: typec: fusb302: Use client->irq as irq if set

[Hans de Goede]

The fusb302 is also used on x86 systems where the platform code sets
the irq in client->irq and there is no gpio named fcs,int_n.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/fusb302/fusb302.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index e1efc67..dea88fd 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1735,9 +1735,13 @@ static int fusb302_probe(struct i2c_client *client,
 		goto destroy_workqueue;
 	}
 
-	ret = init_gpio(chip);
-	if (ret < 0)
-		goto destroy_workqueue;
+	if (client->irq) {
+		chip->gpio_int_n_irq = client->irq;
+	} else {
+		ret = init_gpio(chip);
+		if (ret < 0)
+			goto destroy_workqueue;
+	}
 
 	chip->tcpm_port = tcpm_register_port(&client->dev, &chip->tcpc_dev);
 	if (IS_ERR(chip->tcpm_port)) {
-- 
2.9.4

[PATCH v2 06/14] staging: typec: fusb302: Add support for USB2 charger detection through extcon

[Hans de Goede]

The fusb302 port-controller relies on an external device doing USB2
charger-type detection.

The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
X86/ACPI platforms already has a charger-type detection driver which
uses extcon to communicate the detected charger-type.

This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
USB2 charger detection on these systems. Note that the "fcs,extcon-name"
property name is only for kernel internal use by X86/ACPI platform code
and as such is NOT documented in the devicetree bindings.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/fusb302/fusb302.c | 49 +++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index dea88fd..870f8078 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -17,6 +17,7 @@
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/extcon.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -96,6 +97,7 @@ struct fusb302_chip {
 
 	int gpio_int_n;
 	int gpio_int_n_irq;
+	struct extcon_dev *extcon;
 
 	struct workqueue_struct *wq;
 	struct delayed_work bc_lvl_handler;
@@ -516,6 +518,38 @@ static int tcpm_get_vbus(struct tcpc_dev *dev)
 	return ret;
 }
 
+static int tcpm_get_current_limit(struct tcpc_dev *dev)
+{
+	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
+						 tcpc_dev);
+	int current_limit = 0;
+	unsigned long timeout;
+
+	if (!chip->extcon)
+		return 0;
+
+	/*
+	 * USB2 Charger detection may still be in progress when we get here,
+	 * this can take upto 600ms, wait 800ms max.
+	 */
+	timeout = jiffies + msecs_to_jiffies(800);
+	do {
+		if (extcon_get_state(chip->extcon, EXTCON_CHG_USB_SDP) == 1)
+			current_limit = 500;
+
+		if (extcon_get_state(chip->extcon, EXTCON_CHG_USB_CDP) == 1 ||
+		    extcon_get_state(chip->extcon, EXTCON_CHG_USB_ACA) == 1)
+			current_limit = 1500;
+
+		if (extcon_get_state(chip->extcon, EXTCON_CHG_USB_DCP) == 1)
+			current_limit = 2000;
+
+		msleep(50);
+	} while (current_limit == 0 && time_before(jiffies, timeout));
+
+	return current_limit;
+}
+
 static int fusb302_set_cc_pull(struct fusb302_chip *chip,
 			       bool pull_up, bool pull_down)
 {
@@ -1201,6 +1235,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 {
 	fusb302_tcpc_dev->init = tcpm_init;
 	fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
+	fusb302_tcpc_dev->get_current_limit = tcpm_get_current_limit;
 	fusb302_tcpc_dev->set_cc = tcpm_set_cc;
 	fusb302_tcpc_dev->get_cc = tcpm_get_cc;
 	fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
@@ -1685,6 +1720,7 @@ static int fusb302_probe(struct i2c_client *client,
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter;
 	struct device *dev = &client->dev;
+	const char *name;
 	int ret = 0;
 	u32 val;
 
@@ -1717,6 +1753,19 @@ static int fusb302_probe(struct i2c_client *client,
 	if (!device_property_read_u32(dev, "fcs,operating-snk-microwatt", &val))
 		chip->tcpc_config.operating_snk_mw = val / 1000;
 
+	/*
+	 * Devicetree platforms should get extcon via phandle (not yet
+	 * supported). On ACPI platforms, we get the name from a device prop.
+	 * This device prop is for kernel internal use only and is expected
+	 * to be set by the platform code which also registers the i2c client
+	 * for the fusb302.
+	 */
+	if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {
+		chip->extcon = extcon_get_extcon_dev(name);
+		if (!chip->extcon)
+			return -EPROBE_DEFER;
+	}
+
 	ret = fusb302_debugfs_init(chip);
 	if (ret < 0)
 		return ret;
-- 
2.9.4

[PATCH v2 07/14] staging: typec: fusb302: Export current-limit through a power_supply class dev

[Hans de Goede]

The fusb302 Type-C port-controller cannot control the current-limit
directly, so we need to exported the limit so that another driver
(e.g. the charger driver) can pick the limit up and configure the
system accordingly.

The power-supply subsys already provides infrastructure for this,
power-supply devices have the notion of being supplied by another
power-supply and have properties through which we can export the
current-limit.

Register a power_supply and export the current-limit through the
power_supply's current-max property.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Put the psy class device code directly in fusb302.c rather then introducing
 helpers which are only used by fusb302.c
-Add an online property to the psy so that upower does not mistake it for a
 second battery in the system
---
 drivers/staging/typec/fusb302/Kconfig   |  2 +-
 drivers/staging/typec/fusb302/fusb302.c | 63 +++++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/fusb302/Kconfig b/drivers/staging/typec/fusb302/Kconfig
index fce099f..48a4f2f 100644
--- a/drivers/staging/typec/fusb302/Kconfig
+++ b/drivers/staging/typec/fusb302/Kconfig
@@ -1,6 +1,6 @@
 config TYPEC_FUSB302
 	tristate "Fairchild FUSB302 Type-C chip driver"
-	depends on I2C
+	depends on I2C && POWER_SUPPLY
 	help
 	  The Fairchild FUSB302 Type-C chip driver that works with
 	  Type-C Port Controller Manager to provide USB PD and USB
diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index 870f8078..36fde36 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/power_supply.h>
 #include <linux/proc_fs.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sched/clock.h>
@@ -108,6 +109,11 @@ struct fusb302_chip {
 	/* lock for sharing chip states */
 	struct mutex lock;
 
+	/* psy + psy status */
+	struct power_supply *psy;
+	u32 current_limit;
+	u32 supply_voltage;
+
 	/* chip status */
 	enum toggling_mode toggling_mode;
 	enum src_current_status src_current_status;
@@ -876,11 +882,13 @@ static int tcpm_set_vbus(struct tcpc_dev *dev, bool on, bool charge)
 		chip->vbus_on = on;
 		fusb302_log(chip, "vbus := %s", on ? "On" : "Off");
 	}
-	if (chip->charge_on == charge)
+	if (chip->charge_on == charge) {
 		fusb302_log(chip, "charge is already %s",
 			    charge ? "On" : "Off");
-	else
+	} else {
 		chip->charge_on = charge;
+		power_supply_changed(chip->psy);
+	}
 
 done:
 	mutex_unlock(&chip->lock);
@@ -896,6 +904,11 @@ static int tcpm_set_current_limit(struct tcpc_dev *dev, u32 max_ma, u32 mv)
 	fusb302_log(chip, "current limit: %d ma, %d mv (not implemented)",
 		    max_ma, mv);
 
+	chip->supply_voltage = mv;
+	chip->current_limit = max_ma;
+
+	power_supply_changed(chip->psy);
+
 	return 0;
 }
 
@@ -1681,6 +1694,43 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int fusb302_psy_get_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    union power_supply_propval *val)
+{
+	struct fusb302_chip *chip = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = chip->charge_on;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = chip->supply_voltage * 1000; /* mV -> µV */
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		val->intval = chip->current_limit * 1000; /* mA -> µA */
+		break;
+	default:
+		return -ENODATA;
+	}
+
+	return 0;
+}
+
+static enum power_supply_property fusb302_psy_properties[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
+};
+
+const struct power_supply_desc fusb302_psy_desc = {
+	.name		= "fusb302-typec-source",
+	.type		= POWER_SUPPLY_TYPE_USB_TYPE_C,
+	.properties	= fusb302_psy_properties,
+	.num_properties	= ARRAY_SIZE(fusb302_psy_properties),
+	.get_property	= fusb302_psy_get_property,
+};
+
 static int init_gpio(struct fusb302_chip *chip)
 {
 	struct device_node *node;
@@ -1720,6 +1770,7 @@ static int fusb302_probe(struct i2c_client *client,
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter;
 	struct device *dev = &client->dev;
+	struct power_supply_config cfg = {};
 	const char *name;
 	int ret = 0;
 	u32 val;
@@ -1766,6 +1817,14 @@ static int fusb302_probe(struct i2c_client *client,
 			return -EPROBE_DEFER;
 	}
 
+	cfg.drv_data = chip;
+	chip->psy = devm_power_supply_register(dev, &fusb302_psy_desc, &cfg);
+	if (IS_ERR(chip->psy)) {
+		ret = PTR_ERR(chip->psy);
+		dev_err(chip->dev, "Error registering power-supply: %d\n", ret);
+		return ret;
+	}
+
 	ret = fusb302_debugfs_init(chip);
 	if (ret < 0)
 		return ret;
-- 
2.9.4

[PATCH v2 07/14] staging: typec: fusb302: Export current-limit through a power_supply class dev

[Hans de Goede]

The fusb302 Type-C port-controller cannot control the current-limit
directly, so we need to exported the limit so that another driver
(e.g. the charger driver) can pick the limit up and configure the
system accordingly.

The power-supply subsys already provides infrastructure for this,
power-supply devices have the notion of being supplied by another
power-supply and have properties through which we can export the
current-limit.

Register a power_supply and export the current-limit through the
power_supply's current-max property.

Cc: "Yueyao (Nathan) Zhu" <yueyao@google.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Put the psy class device code directly in fusb302.c rather then introducing
 helpers which are only used by fusb302.c
-Add an online property to the psy so that upower does not mistake it for a
 second battery in the system
---
 drivers/staging/typec/fusb302/Kconfig   |  2 +-
 drivers/staging/typec/fusb302/fusb302.c | 63 +++++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/fusb302/Kconfig b/drivers/staging/typec/fusb302/Kconfig
index fce099f..48a4f2f 100644
--- a/drivers/staging/typec/fusb302/Kconfig
+++ b/drivers/staging/typec/fusb302/Kconfig
@@ -1,6 +1,6 @@
 config TYPEC_FUSB302
 	tristate "Fairchild FUSB302 Type-C chip driver"
-	depends on I2C
+	depends on I2C && POWER_SUPPLY
 	help
 	  The Fairchild FUSB302 Type-C chip driver that works with
 	  Type-C Port Controller Manager to provide USB PD and USB
diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
index 870f8078..36fde36 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/power_supply.h>
 #include <linux/proc_fs.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sched/clock.h>
@@ -108,6 +109,11 @@ struct fusb302_chip {
 	/* lock for sharing chip states */
 	struct mutex lock;
 
+	/* psy + psy status */
+	struct power_supply *psy;
+	u32 current_limit;
+	u32 supply_voltage;
+
 	/* chip status */
 	enum toggling_mode toggling_mode;
 	enum src_current_status src_current_status;
@@ -876,11 +882,13 @@ static int tcpm_set_vbus(struct tcpc_dev *dev, bool on, bool charge)
 		chip->vbus_on = on;
 		fusb302_log(chip, "vbus := %s", on ? "On" : "Off");
 	}
-	if (chip->charge_on == charge)
+	if (chip->charge_on == charge) {
 		fusb302_log(chip, "charge is already %s",
 			    charge ? "On" : "Off");
-	else
+	} else {
 		chip->charge_on = charge;
+		power_supply_changed(chip->psy);
+	}
 
 done:
 	mutex_unlock(&chip->lock);
@@ -896,6 +904,11 @@ static int tcpm_set_current_limit(struct tcpc_dev *dev, u32 max_ma, u32 mv)
 	fusb302_log(chip, "current limit: %d ma, %d mv (not implemented)",
 		    max_ma, mv);
 
+	chip->supply_voltage = mv;
+	chip->current_limit = max_ma;
+
+	power_supply_changed(chip->psy);
+
 	return 0;
 }
 
@@ -1681,6 +1694,43 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int fusb302_psy_get_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    union power_supply_propval *val)
+{
+	struct fusb302_chip *chip = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = chip->charge_on;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = chip->supply_voltage * 1000; /* mV -> µV */
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		val->intval = chip->current_limit * 1000; /* mA -> µA */
+		break;
+	default:
+		return -ENODATA;
+	}
+
+	return 0;
+}
+
+static enum power_supply_property fusb302_psy_properties[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
+};
+
+const struct power_supply_desc fusb302_psy_desc = {
+	.name		= "fusb302-typec-source",
+	.type		= POWER_SUPPLY_TYPE_USB_TYPE_C,
+	.properties	= fusb302_psy_properties,
+	.num_properties	= ARRAY_SIZE(fusb302_psy_properties),
+	.get_property	= fusb302_psy_get_property,
+};
+
 static int init_gpio(struct fusb302_chip *chip)
 {
 	struct device_node *node;
@@ -1720,6 +1770,7 @@ static int fusb302_probe(struct i2c_client *client,
 	struct fusb302_chip *chip;
 	struct i2c_adapter *adapter;
 	struct device *dev = &client->dev;
+	struct power_supply_config cfg = {};
 	const char *name;
 	int ret = 0;
 	u32 val;
@@ -1766,6 +1817,14 @@ static int fusb302_probe(struct i2c_client *client,
 			return -EPROBE_DEFER;
 	}
 
+	cfg.drv_data = chip;
+	chip->psy = devm_power_supply_register(dev, &fusb302_psy_desc, &cfg);
+	if (IS_ERR(chip->psy)) {
+		ret = PTR_ERR(chip->psy);
+		dev_err(chip->dev, "Error registering power-supply: %d\n", ret);
+		return ret;
+	}
+
 	ret = fusb302_debugfs_init(chip);
 	if (ret < 0)
 		return ret;
-- 
2.9.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Hans de Goede]

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds a power_supply_set_input_current_limit_from_supplier
helper function which charger power-supply drivers can call to get
the max-current from their supplier and have this applied
through their set_property call-back to their input-current-limit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_core.c | 41 ++++++++++++++++++++++++++++++++
 include/linux/power_supply.h             |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 0741fce..3f92574 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -375,6 +375,47 @@ int power_supply_is_system_supplied(void)
 }
 EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
 
+static int __power_supply_get_supplier_max_current(struct device *dev,
+						   void *data)
+{
+	union power_supply_propval ret = {0,};
+	struct power_supply *epsy = dev_get_drvdata(dev);
+	struct power_supply *psy = data;
+
+	if (__power_supply_is_supplied_by(epsy, psy))
+		if (!epsy->desc->get_property(epsy,
+					      POWER_SUPPLY_PROP_CURRENT_MAX,
+					      &ret))
+			return ret.intval;
+
+	return 0;
+}
+
+int power_supply_set_input_current_limit_from_supplier(struct power_supply *psy)
+{
+	union power_supply_propval val = {0,};
+	int curr;
+
+	if (!psy->desc->set_property)
+		return -EINVAL;
+
+	/*
+	 * This function is not intended for use with a supply with multiple
+	 * suppliers, we simply pick the first supply to report a non 0
+	 * max-current.
+	 */
+	curr = class_for_each_device(power_supply_class, NULL, psy,
+				      __power_supply_get_supplier_max_current);
+	if (curr <= 0)
+		return (curr == 0) ? -ENODEV : curr;
+
+	val.intval = curr;
+
+	return psy->desc->set_property(psy,
+				POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, &val);
+}
+EXPORT_SYMBOL_GPL(power_supply_set_input_current_limit_from_supplier);
+
 int power_supply_set_battery_charged(struct power_supply *psy)
 {
 	if (atomic_read(&psy->use_cnt) >= 0 &&
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index de89066..79e90b3 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -332,6 +332,8 @@ extern int power_supply_get_battery_info(struct power_supply *psy,
 					 struct power_supply_battery_info *info);
 extern void power_supply_changed(struct power_supply *psy);
 extern int power_supply_am_i_supplied(struct power_supply *psy);
+extern int power_supply_set_input_current_limit_from_supplier(
+					 struct power_supply *psy);
 extern int power_supply_set_battery_charged(struct power_supply *psy);
 
 #ifdef CONFIG_POWER_SUPPLY
-- 
2.9.4

[PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Hans de Goede]

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds a power_supply_set_input_current_limit_from_supplier
helper function which charger power-supply drivers can call to get
the max-current from their supplier and have this applied
through their set_property call-back to their input-current-limit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_core.c | 41 ++++++++++++++++++++++++++++++++
 include/linux/power_supply.h             |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 0741fce..3f92574 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -375,6 +375,47 @@ int power_supply_is_system_supplied(void)
 }
 EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
 
+static int __power_supply_get_supplier_max_current(struct device *dev,
+						   void *data)
+{
+	union power_supply_propval ret = {0,};
+	struct power_supply *epsy = dev_get_drvdata(dev);
+	struct power_supply *psy = data;
+
+	if (__power_supply_is_supplied_by(epsy, psy))
+		if (!epsy->desc->get_property(epsy,
+					      POWER_SUPPLY_PROP_CURRENT_MAX,
+					      &ret))
+			return ret.intval;
+
+	return 0;
+}
+
+int power_supply_set_input_current_limit_from_supplier(struct power_supply *psy)
+{
+	union power_supply_propval val = {0,};
+	int curr;
+
+	if (!psy->desc->set_property)
+		return -EINVAL;
+
+	/*
+	 * This function is not intended for use with a supply with multiple
+	 * suppliers, we simply pick the first supply to report a non 0
+	 * max-current.
+	 */
+	curr = class_for_each_device(power_supply_class, NULL, psy,
+				      __power_supply_get_supplier_max_current);
+	if (curr <= 0)
+		return (curr == 0) ? -ENODEV : curr;
+
+	val.intval = curr;
+
+	return psy->desc->set_property(psy,
+				POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, &val);
+}
+EXPORT_SYMBOL_GPL(power_supply_set_input_current_limit_from_supplier);
+
 int power_supply_set_battery_charged(struct power_supply *psy)
 {
 	if (atomic_read(&psy->use_cnt) >= 0 &&
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index de89066..79e90b3 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -332,6 +332,8 @@ extern int power_supply_get_battery_info(struct power_supply *psy,
 					 struct power_supply_battery_info *info);
 extern void power_supply_changed(struct power_supply *psy);
 extern int power_supply_am_i_supplied(struct power_supply *psy);
+extern int power_supply_set_input_current_limit_from_supplier(
+					 struct power_supply *psy);
 extern int power_supply_set_battery_charged(struct power_supply *psy);
 
 #ifdef CONFIG_POWER_SUPPLY
-- 
2.9.4

Re: [PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Tony Lindgren]

* Hans de Goede <hdegoede@redhat.com> [170815 13:06]:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
> 
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
> 
> This commit adds a power_supply_set_input_current_limit_from_supplier
> helper function which charger power-supply drivers can call to get
> the max-current from their supplier and have this applied
> through their set_property call-back to their input-current-limit.

Hmm so can this also be used for the USB gadget subsystem
to tell charge controller when it's OK to enable 500mA
charging after enumeration?

FYI, that's controlled by the bq24190 pin named OTG that should
be only set high after enumeration. Any ideas how that is wired
on your device? Does it connect to the USB PHY or to a GPIO
line?

Regards,

Tony

Re: [PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Hans de Goede]

Hi,

On 16-08-17 17:54, Tony Lindgren wrote:
> * Hans de Goede <hdegoede@redhat.com> [170815 13:06]:
>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>> done by a separate port-controller IC, while the current limit is
>> controlled through another (charger) IC.
>>
>> It has been decided to model this by modelling the external Type-C
>> power brick (adapter/charger) as a power-supply class device which
>> supplies the charger-IC, with its voltage-now and current-max representing
>> the negotiated voltage and max current draw.
>>
>> This commit adds a power_supply_set_input_current_limit_from_supplier
>> helper function which charger power-supply drivers can call to get
>> the max-current from their supplier and have this applied
>> through their set_property call-back to their input-current-limit.
> 
> Hmm so can this also be used for the USB gadget subsystem
> to tell charge controller when it's OK to enable 500mA
> charging after enumeration?

I'm not sure that that would be best modeled this way. Perhaps
the phy-driver can directly control the gpio you have for that,
that seems better then trying to solve this with cross subsystem
calls which are always tricky.

> FYI, that's controlled by the bq24190 pin named OTG that should
> be only set high after enumeration. Any ideas how that is wired
> on your device? Does it connect to the USB PHY or to a GPIO
> line?

I believe it is just hardwired to be logical high all the time
on my board.

Regards,

Hans

Re: [PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Hans de Goede]

Hi,

On 16-08-17 17:54, Tony Lindgren wrote:
> * Hans de Goede <hdegoede@redhat.com> [170815 13:06]:
>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>> done by a separate port-controller IC, while the current limit is
>> controlled through another (charger) IC.
>>
>> It has been decided to model this by modelling the external Type-C
>> power brick (adapter/charger) as a power-supply class device which
>> supplies the charger-IC, with its voltage-now and current-max representing
>> the negotiated voltage and max current draw.
>>
>> This commit adds a power_supply_set_input_current_limit_from_supplier
>> helper function which charger power-supply drivers can call to get
>> the max-current from their supplier and have this applied
>> through their set_property call-back to their input-current-limit.
> 
> Hmm so can this also be used for the USB gadget subsystem
> to tell charge controller when it's OK to enable 500mA
> charging after enumeration?

I'm not sure that that would be best modeled this way. Perhaps
the phy-driver can directly control the gpio you have for that,
that seems better then trying to solve this with cross subsystem
calls which are always tricky.

> FYI, that's controlled by the bq24190 pin named OTG that should
> be only set high after enumeration. Any ideas how that is wired
> on your device? Does it connect to the USB PHY or to a GPIO
> line?

I believe it is just hardwired to be logical high all the time
on my board.

Regards,

Hans

Re: [PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Tony Lindgren]

* Hans de Goede <hdegoede@redhat.com> [170816 10:38]:
> Hi,
> 
> On 16-08-17 17:54, Tony Lindgren wrote:
> > * Hans de Goede <hdegoede@redhat.com> [170815 13:06]:
> > > On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> > > done by a separate port-controller IC, while the current limit is
> > > controlled through another (charger) IC.
> > > 
> > > It has been decided to model this by modelling the external Type-C
> > > power brick (adapter/charger) as a power-supply class device which
> > > supplies the charger-IC, with its voltage-now and current-max representing
> > > the negotiated voltage and max current draw.
> > > 
> > > This commit adds a power_supply_set_input_current_limit_from_supplier
> > > helper function which charger power-supply drivers can call to get
> > > the max-current from their supplier and have this applied
> > > through their set_property call-back to their input-current-limit.
> > 
> > Hmm so can this also be used for the USB gadget subsystem
> > to tell charge controller when it's OK to enable 500mA
> > charging after enumeration?
> 
> I'm not sure that that would be best modeled this way. Perhaps
> the phy-driver can directly control the gpio you have for that,
> that seems better then trying to solve this with cross subsystem
> calls which are always tricky.

I don't think the phy driver knows either when the system
has enumerated as a gadget..

> > FYI, that's controlled by the bq24190 pin named OTG that should
> > be only set high after enumeration. Any ideas how that is wired
> > on your device? Does it connect to the USB PHY or to a GPIO
> > line?
> 
> I believe it is just hardwired to be logical high all the time
> on my board.

OK thanks for checking.

Tony

Re: [PATCH v2 08/14] power: supply: Add power_supply_set_input_current_limit_from_supplier helper

[Sebastian Reichel]

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

Hi,

On Tue, Aug 15, 2017 at 10:04:56PM +0200, Hans de Goede wrote:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
> 
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
> 
> This commit adds a power_supply_set_input_current_limit_from_supplier
> helper function which charger power-supply drivers can call to get
> the max-current from their supplier and have this applied
> through their set_property call-back to their input-current-limit.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks, queued.

-- Sebastian

>  drivers/power/supply/power_supply_core.c | 41 ++++++++++++++++++++++++++++++++
>  include/linux/power_supply.h             |  2 ++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 0741fce..3f92574 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -375,6 +375,47 @@ int power_supply_is_system_supplied(void)
>  }
>  EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
>  
> +static int __power_supply_get_supplier_max_current(struct device *dev,
> +						   void *data)
> +{
> +	union power_supply_propval ret = {0,};
> +	struct power_supply *epsy = dev_get_drvdata(dev);
> +	struct power_supply *psy = data;
> +
> +	if (__power_supply_is_supplied_by(epsy, psy))
> +		if (!epsy->desc->get_property(epsy,
> +					      POWER_SUPPLY_PROP_CURRENT_MAX,
> +					      &ret))
> +			return ret.intval;
> +
> +	return 0;
> +}
> +
> +int power_supply_set_input_current_limit_from_supplier(struct power_supply *psy)
> +{
> +	union power_supply_propval val = {0,};
> +	int curr;
> +
> +	if (!psy->desc->set_property)
> +		return -EINVAL;
> +
> +	/*
> +	 * This function is not intended for use with a supply with multiple
> +	 * suppliers, we simply pick the first supply to report a non 0
> +	 * max-current.
> +	 */
> +	curr = class_for_each_device(power_supply_class, NULL, psy,
> +				      __power_supply_get_supplier_max_current);
> +	if (curr <= 0)
> +		return (curr == 0) ? -ENODEV : curr;
> +
> +	val.intval = curr;
> +
> +	return psy->desc->set_property(psy,
> +				POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, &val);
> +}
> +EXPORT_SYMBOL_GPL(power_supply_set_input_current_limit_from_supplier);
> +
>  int power_supply_set_battery_charged(struct power_supply *psy)
>  {
>  	if (atomic_read(&psy->use_cnt) >= 0 &&
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index de89066..79e90b3 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -332,6 +332,8 @@ extern int power_supply_get_battery_info(struct power_supply *psy,
>  					 struct power_supply_battery_info *info);
>  extern void power_supply_changed(struct power_supply *psy);
>  extern int power_supply_am_i_supplied(struct power_supply *psy);
> +extern int power_supply_set_input_current_limit_from_supplier(
> +					 struct power_supply *psy);
>  extern int power_supply_set_battery_charged(struct power_supply *psy);
>  
>  #ifdef CONFIG_POWER_SUPPLY
> -- 
> 2.9.4
> 

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

[PATCH v2 09/14] power: supply: bq24190_charger: Export 5V boost converter as regulator

[Hans de Goede]

Register the 5V boost converter as a regulator named "usb_otg_vbus".

This commit also adds support for bq24190_platform_data, through which
non device-tree platforms can pass the regulator_init_data (containing
mappings for the consumer amongst other things).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use "usb_otg_vbus" as default name for the regulator
-Add support for passing regulator_init_data for non device-tree platforms
-Register the regulator later, to avoid it showing up and shortly later
 disappearing again on probe errors (e.g. -EPROBE_DEFER).
---
 drivers/power/supply/bq24190_charger.c | 126 +++++++++++++++++++++++++++++++++
 include/linux/power/bq24190_charger.h  |  18 +++++
 2 files changed, 144 insertions(+)
 create mode 100644 include/linux/power/bq24190_charger.h

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index d5a707e..073cd9d 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -16,6 +16,9 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/power_supply.h>
+#include <linux/power/bq24190_charger.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
 #include <linux/workqueue.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
@@ -504,6 +507,125 @@ static int bq24190_sysfs_create_group(struct bq24190_dev_info *bdi)
 static inline void bq24190_sysfs_remove_group(struct bq24190_dev_info *bdi) {}
 #endif
 
+#ifdef CONFIG_REGULATOR
+static int bq24190_vbus_enable(struct regulator_dev *dev)
+{
+	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(bdi->dev);
+	if (ret < 0) {
+		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
+		pm_runtime_put_noidle(bdi->dev);
+		return ret;
+	}
+
+	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
+				 BQ24190_REG_POC_CHG_CONFIG_MASK,
+				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
+				 BQ24190_REG_POC_CHG_CONFIG_OTG);
+
+	pm_runtime_mark_last_busy(bdi->dev);
+	pm_runtime_put_autosuspend(bdi->dev);
+
+	return ret;
+}
+
+static int bq24190_vbus_disable(struct regulator_dev *dev)
+{
+	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(bdi->dev);
+	if (ret < 0) {
+		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
+		pm_runtime_put_noidle(bdi->dev);
+		return ret;
+	}
+
+	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
+				 BQ24190_REG_POC_CHG_CONFIG_MASK,
+				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
+				 BQ24190_REG_POC_CHG_CONFIG_CHARGE);
+
+	pm_runtime_mark_last_busy(bdi->dev);
+	pm_runtime_put_autosuspend(bdi->dev);
+
+	return ret;
+}
+
+static int bq24190_vbus_is_enabled(struct regulator_dev *dev)
+{
+	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
+	int ret;
+	u8 val;
+
+	ret = pm_runtime_get_sync(bdi->dev);
+	if (ret < 0) {
+		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
+		pm_runtime_put_noidle(bdi->dev);
+		return ret;
+	}
+
+	ret = bq24190_read_mask(bdi, BQ24190_REG_POC,
+				BQ24190_REG_POC_CHG_CONFIG_MASK,
+				BQ24190_REG_POC_CHG_CONFIG_SHIFT, &val);
+
+	pm_runtime_mark_last_busy(bdi->dev);
+	pm_runtime_put_autosuspend(bdi->dev);
+
+	return ret ? ret : val == BQ24190_REG_POC_CHG_CONFIG_OTG;
+}
+
+static const struct regulator_ops bq24190_vbus_ops = {
+	.enable = bq24190_vbus_enable,
+	.disable = bq24190_vbus_disable,
+	.is_enabled = bq24190_vbus_is_enabled,
+};
+
+static const struct regulator_desc bq24190_vbus_desc = {
+	.name = "usb_otg_vbus",
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+	.ops = &bq24190_vbus_ops,
+	.fixed_uV = 5000000,
+	.n_voltages = 1,
+};
+
+static const struct regulator_init_data bq24190_vbus_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+};
+
+static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
+{
+	struct bq24190_platform_data *pdata = bdi->dev->platform_data;
+	struct regulator_config cfg = { };
+	struct regulator_dev *reg;
+	int ret = 0;
+
+	cfg.dev = bdi->dev;
+	if (pdata && pdata->regulator_init_data)
+		cfg.init_data = pdata->regulator_init_data;
+	else
+		cfg.init_data = &bq24190_vbus_init_data;
+	cfg.driver_data = bdi;
+	reg = devm_regulator_register(bdi->dev, &bq24190_vbus_desc, &cfg);
+	if (IS_ERR(reg)) {
+		ret = PTR_ERR(reg);
+		dev_err(bdi->dev, "Can't register regulator: %d\n", ret);
+	}
+
+	return ret;
+}
+#else
+static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
+{
+	return 0;
+}
+#endif
+
 /*
  * According to the "Host Mode and default Mode" section of the
  * manual, a write to any register causes the bq24190 to switch
@@ -1577,6 +1699,10 @@ static int bq24190_probe(struct i2c_client *client,
 		goto out_sysfs;
 	}
 
+	ret = bq24190_register_vbus_regulator(bdi);
+	if (ret < 0)
+		goto out_sysfs;
+
 	if (bdi->extcon) {
 		INIT_DELAYED_WORK(&bdi->extcon_work, bq24190_extcon_work);
 		bdi->extcon_nb.notifier_call = bq24190_extcon_event;
diff --git a/include/linux/power/bq24190_charger.h b/include/linux/power/bq24190_charger.h
new file mode 100644
index 0000000..45ce7f1
--- /dev/null
+++ b/include/linux/power/bq24190_charger.h
@@ -0,0 +1,18 @@
+/*
+ * Platform data for the TI bq24190 battery charger driver.
+ *
+ * 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.
+ */
+
+#ifndef _BQ24190_CHARGER_H_
+#define _BQ24190_CHARGER_H_
+
+#include <linux/regulator/machine.h>
+
+struct bq24190_platform_data {
+	const struct regulator_init_data *regulator_init_data;
+};
+
+#endif
-- 
2.9.4

[PATCH v2 09/14] power: supply: bq24190_charger: Export 5V boost converter as regulator

[Hans de Goede]

Register the 5V boost converter as a regulator named "usb_otg_vbus".

This commit also adds support for bq24190_platform_data, through which
non device-tree platforms can pass the regulator_init_data (containing
mappings for the consumer amongst other things).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use "usb_otg_vbus" as default name for the regulator
-Add support for passing regulator_init_data for non device-tree platforms
-Register the regulator later, to avoid it showing up and shortly later
 disappearing again on probe errors (e.g. -EPROBE_DEFER).
---
 drivers/power/supply/bq24190_charger.c | 126 +++++++++++++++++++++++++++++++++
 include/linux/power/bq24190_charger.h  |  18 +++++
 2 files changed, 144 insertions(+)
 create mode 100644 include/linux/power/bq24190_charger.h

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index d5a707e..073cd9d 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -16,6 +16,9 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/power_supply.h>
+#include <linux/power/bq24190_charger.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
 #include <linux/workqueue.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
@@ -504,6 +507,125 @@ static int bq24190_sysfs_create_group(struct bq24190_dev_info *bdi)
 static inline void bq24190_sysfs_remove_group(struct bq24190_dev_info *bdi) {}
 #endif
 
+#ifdef CONFIG_REGULATOR
+static int bq24190_vbus_enable(struct regulator_dev *dev)
+{
+	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(bdi->dev);
+	if (ret < 0) {
+		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
+		pm_runtime_put_noidle(bdi->dev);
+		return ret;
+	}
+
+	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
+				 BQ24190_REG_POC_CHG_CONFIG_MASK,
+				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
+				 BQ24190_REG_POC_CHG_CONFIG_OTG);
+
+	pm_runtime_mark_last_busy(bdi->dev);
+	pm_runtime_put_autosuspend(bdi->dev);
+
+	return ret;
+}
+
+static int bq24190_vbus_disable(struct regulator_dev *dev)
+{
+	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(bdi->dev);
+	if (ret < 0) {
+		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
+		pm_runtime_put_noidle(bdi->dev);
+		return ret;
+	}
+
+	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
+				 BQ24190_REG_POC_CHG_CONFIG_MASK,
+				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
+				 BQ24190_REG_POC_CHG_CONFIG_CHARGE);
+
+	pm_runtime_mark_last_busy(bdi->dev);
+	pm_runtime_put_autosuspend(bdi->dev);
+
+	return ret;
+}
+
+static int bq24190_vbus_is_enabled(struct regulator_dev *dev)
+{
+	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
+	int ret;
+	u8 val;
+
+	ret = pm_runtime_get_sync(bdi->dev);
+	if (ret < 0) {
+		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
+		pm_runtime_put_noidle(bdi->dev);
+		return ret;
+	}
+
+	ret = bq24190_read_mask(bdi, BQ24190_REG_POC,
+				BQ24190_REG_POC_CHG_CONFIG_MASK,
+				BQ24190_REG_POC_CHG_CONFIG_SHIFT, &val);
+
+	pm_runtime_mark_last_busy(bdi->dev);
+	pm_runtime_put_autosuspend(bdi->dev);
+
+	return ret ? ret : val == BQ24190_REG_POC_CHG_CONFIG_OTG;
+}
+
+static const struct regulator_ops bq24190_vbus_ops = {
+	.enable = bq24190_vbus_enable,
+	.disable = bq24190_vbus_disable,
+	.is_enabled = bq24190_vbus_is_enabled,
+};
+
+static const struct regulator_desc bq24190_vbus_desc = {
+	.name = "usb_otg_vbus",
+	.type = REGULATOR_VOLTAGE,
+	.owner = THIS_MODULE,
+	.ops = &bq24190_vbus_ops,
+	.fixed_uV = 5000000,
+	.n_voltages = 1,
+};
+
+static const struct regulator_init_data bq24190_vbus_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+};
+
+static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
+{
+	struct bq24190_platform_data *pdata = bdi->dev->platform_data;
+	struct regulator_config cfg = { };
+	struct regulator_dev *reg;
+	int ret = 0;
+
+	cfg.dev = bdi->dev;
+	if (pdata && pdata->regulator_init_data)
+		cfg.init_data = pdata->regulator_init_data;
+	else
+		cfg.init_data = &bq24190_vbus_init_data;
+	cfg.driver_data = bdi;
+	reg = devm_regulator_register(bdi->dev, &bq24190_vbus_desc, &cfg);
+	if (IS_ERR(reg)) {
+		ret = PTR_ERR(reg);
+		dev_err(bdi->dev, "Can't register regulator: %d\n", ret);
+	}
+
+	return ret;
+}
+#else
+static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
+{
+	return 0;
+}
+#endif
+
 /*
  * According to the "Host Mode and default Mode" section of the
  * manual, a write to any register causes the bq24190 to switch
@@ -1577,6 +1699,10 @@ static int bq24190_probe(struct i2c_client *client,
 		goto out_sysfs;
 	}
 
+	ret = bq24190_register_vbus_regulator(bdi);
+	if (ret < 0)
+		goto out_sysfs;
+
 	if (bdi->extcon) {
 		INIT_DELAYED_WORK(&bdi->extcon_work, bq24190_extcon_work);
 		bdi->extcon_nb.notifier_call = bq24190_extcon_event;
diff --git a/include/linux/power/bq24190_charger.h b/include/linux/power/bq24190_charger.h
new file mode 100644
index 0000000..45ce7f1
--- /dev/null
+++ b/include/linux/power/bq24190_charger.h
@@ -0,0 +1,18 @@
+/*
+ * Platform data for the TI bq24190 battery charger driver.
+ *
+ * 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.
+ */
+
+#ifndef _BQ24190_CHARGER_H_
+#define _BQ24190_CHARGER_H_
+
+#include <linux/regulator/machine.h>
+
+struct bq24190_platform_data {
+	const struct regulator_init_data *regulator_init_data;
+};
+
+#endif
-- 
2.9.4

Re: [PATCH v2 09/14] power: supply: bq24190_charger: Export 5V boost converter as regulator

[Sebastian Reichel]

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

Hi,

On Tue, Aug 15, 2017 at 10:04:57PM +0200, Hans de Goede wrote:
> Register the 5V boost converter as a regulator named "usb_otg_vbus".
> 
> This commit also adds support for bq24190_platform_data, through which
> non device-tree platforms can pass the regulator_init_data (containing
> mappings for the consumer amongst other things).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Use "usb_otg_vbus" as default name for the regulator
> -Add support for passing regulator_init_data for non device-tree platforms
> -Register the regulator later, to avoid it showing up and shortly later
>  disappearing again on probe errors (e.g. -EPROBE_DEFER).
> ---
>  drivers/power/supply/bq24190_charger.c | 126 +++++++++++++++++++++++++++++++++
>  include/linux/power/bq24190_charger.h  |  18 +++++
>  2 files changed, 144 insertions(+)
>  create mode 100644 include/linux/power/bq24190_charger.h
> 
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index d5a707e..073cd9d 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -16,6 +16,9 @@
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/power_supply.h>
> +#include <linux/power/bq24190_charger.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
>  #include <linux/workqueue.h>
>  #include <linux/gpio.h>
>  #include <linux/i2c.h>
> @@ -504,6 +507,125 @@ static int bq24190_sysfs_create_group(struct bq24190_dev_info *bdi)
>  static inline void bq24190_sysfs_remove_group(struct bq24190_dev_info *bdi) {}
>  #endif
>  
> +#ifdef CONFIG_REGULATOR
> +
> +static int bq24190_vbus_enable(struct regulator_dev *dev)
> +{
> +	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(bdi->dev);
> +	if (ret < 0) {
> +		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> +		pm_runtime_put_noidle(bdi->dev);
> +		return ret;
> +	}
> +
> +	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
> +				 BQ24190_REG_POC_CHG_CONFIG_MASK,
> +				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
> +				 BQ24190_REG_POC_CHG_CONFIG_OTG);
> +
> +	pm_runtime_mark_last_busy(bdi->dev);
> +	pm_runtime_put_autosuspend(bdi->dev);
> +
> +	return ret;
> +}
> +
> +static int bq24190_vbus_disable(struct regulator_dev *dev)
> +{
> +	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(bdi->dev);
> +	if (ret < 0) {
> +		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> +		pm_runtime_put_noidle(bdi->dev);
> +		return ret;
> +	}
> +
> +	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
> +				 BQ24190_REG_POC_CHG_CONFIG_MASK,
> +				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
> +				 BQ24190_REG_POC_CHG_CONFIG_CHARGE);
> +
> +	pm_runtime_mark_last_busy(bdi->dev);
> +	pm_runtime_put_autosuspend(bdi->dev);
> +
> +	return ret;
> +}

Let's reduce code duplication:

static int bq24190_vbus_set(dev, val) {
    ...
}

static int bq24190_vbus_enable(dev) {
    return bq24190_vbus_set(dev, BQ24190_REG_POC_CHG_CONFIG_OTG);
}

static int bq24190_vbus_disable(dev) {
    return bq24190_vbus_set(dev, BQ24190_REG_POC_CHG_CONFIG_CHARGE);
}

> +static int bq24190_vbus_is_enabled(struct regulator_dev *dev)
> +{
> +	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
> +	int ret;
> +	u8 val;
> +
> +	ret = pm_runtime_get_sync(bdi->dev);
> +	if (ret < 0) {
> +		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> +		pm_runtime_put_noidle(bdi->dev);
> +		return ret;
> +	}
> +
> +	ret = bq24190_read_mask(bdi, BQ24190_REG_POC,
> +				BQ24190_REG_POC_CHG_CONFIG_MASK,
> +				BQ24190_REG_POC_CHG_CONFIG_SHIFT, &val);
> +
> +	pm_runtime_mark_last_busy(bdi->dev);
> +	pm_runtime_put_autosuspend(bdi->dev);
> +
> +	return ret ? ret : val == BQ24190_REG_POC_CHG_CONFIG_OTG;
> +}
> +
> +static const struct regulator_ops bq24190_vbus_ops = {
> +	.enable = bq24190_vbus_enable,
> +	.disable = bq24190_vbus_disable,
> +	.is_enabled = bq24190_vbus_is_enabled,
> +};
> +
> +static const struct regulator_desc bq24190_vbus_desc = {
> +	.name = "usb_otg_vbus",
> +	.type = REGULATOR_VOLTAGE,
> +	.owner = THIS_MODULE,
> +	.ops = &bq24190_vbus_ops,
> +	.fixed_uV = 5000000,
> +	.n_voltages = 1,
> +};
> +
> +static const struct regulator_init_data bq24190_vbus_init_data = {
> +	.constraints = {
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +};
> +
> +static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
> +{
> +	struct bq24190_platform_data *pdata = bdi->dev->platform_data;
> +	struct regulator_config cfg = { };
> +	struct regulator_dev *reg;
> +	int ret = 0;
> +
> +	cfg.dev = bdi->dev;
> +	if (pdata && pdata->regulator_init_data)
> +		cfg.init_data = pdata->regulator_init_data;
> +	else
> +		cfg.init_data = &bq24190_vbus_init_data;
> +	cfg.driver_data = bdi;
> +	reg = devm_regulator_register(bdi->dev, &bq24190_vbus_desc, &cfg);
> +	if (IS_ERR(reg)) {
> +		ret = PTR_ERR(reg);
> +		dev_err(bdi->dev, "Can't register regulator: %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +#else
> +static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * According to the "Host Mode and default Mode" section of the
>   * manual, a write to any register causes the bq24190 to switch
> @@ -1577,6 +1699,10 @@ static int bq24190_probe(struct i2c_client *client,
>  		goto out_sysfs;
>  	}
>  
> +	ret = bq24190_register_vbus_regulator(bdi);
> +	if (ret < 0)
> +		goto out_sysfs;
> +
>  	if (bdi->extcon) {
>  		INIT_DELAYED_WORK(&bdi->extcon_work, bq24190_extcon_work);
>  		bdi->extcon_nb.notifier_call = bq24190_extcon_event;
> diff --git a/include/linux/power/bq24190_charger.h b/include/linux/power/bq24190_charger.h
> new file mode 100644
> index 0000000..45ce7f1
> --- /dev/null
> +++ b/include/linux/power/bq24190_charger.h
> @@ -0,0 +1,18 @@
> +/*
> + * Platform data for the TI bq24190 battery charger driver.
> + *
> + * 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.
> + */
> +
> +#ifndef _BQ24190_CHARGER_H_
> +#define _BQ24190_CHARGER_H_
> +
> +#include <linux/regulator/machine.h>
> +
> +struct bq24190_platform_data {
> +	const struct regulator_init_data *regulator_init_data;
> +};
> +
> +#endif
> -- 
> 2.9.4
> 

Otherwise looks fine to me.

-- Sebastian

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

Re: [PATCH v2 09/14] power: supply: bq24190_charger: Export 5V boost converter as regulator

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 6711 bytes --]

Hi,

On Tue, Aug 15, 2017 at 10:04:57PM +0200, Hans de Goede wrote:
> Register the 5V boost converter as a regulator named "usb_otg_vbus".
> 
> This commit also adds support for bq24190_platform_data, through which
> non device-tree platforms can pass the regulator_init_data (containing
> mappings for the consumer amongst other things).
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Use "usb_otg_vbus" as default name for the regulator
> -Add support for passing regulator_init_data for non device-tree platforms
> -Register the regulator later, to avoid it showing up and shortly later
>  disappearing again on probe errors (e.g. -EPROBE_DEFER).
> ---
>  drivers/power/supply/bq24190_charger.c | 126 +++++++++++++++++++++++++++++++++
>  include/linux/power/bq24190_charger.h  |  18 +++++
>  2 files changed, 144 insertions(+)
>  create mode 100644 include/linux/power/bq24190_charger.h
> 
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index d5a707e..073cd9d 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -16,6 +16,9 @@
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/power_supply.h>
> +#include <linux/power/bq24190_charger.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
>  #include <linux/workqueue.h>
>  #include <linux/gpio.h>
>  #include <linux/i2c.h>
> @@ -504,6 +507,125 @@ static int bq24190_sysfs_create_group(struct bq24190_dev_info *bdi)
>  static inline void bq24190_sysfs_remove_group(struct bq24190_dev_info *bdi) {}
>  #endif
>  
> +#ifdef CONFIG_REGULATOR
> +
> +static int bq24190_vbus_enable(struct regulator_dev *dev)
> +{
> +	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(bdi->dev);
> +	if (ret < 0) {
> +		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> +		pm_runtime_put_noidle(bdi->dev);
> +		return ret;
> +	}
> +
> +	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
> +				 BQ24190_REG_POC_CHG_CONFIG_MASK,
> +				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
> +				 BQ24190_REG_POC_CHG_CONFIG_OTG);
> +
> +	pm_runtime_mark_last_busy(bdi->dev);
> +	pm_runtime_put_autosuspend(bdi->dev);
> +
> +	return ret;
> +}
> +
> +static int bq24190_vbus_disable(struct regulator_dev *dev)
> +{
> +	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(bdi->dev);
> +	if (ret < 0) {
> +		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> +		pm_runtime_put_noidle(bdi->dev);
> +		return ret;
> +	}
> +
> +	ret = bq24190_write_mask(bdi, BQ24190_REG_POC,
> +				 BQ24190_REG_POC_CHG_CONFIG_MASK,
> +				 BQ24190_REG_POC_CHG_CONFIG_SHIFT,
> +				 BQ24190_REG_POC_CHG_CONFIG_CHARGE);
> +
> +	pm_runtime_mark_last_busy(bdi->dev);
> +	pm_runtime_put_autosuspend(bdi->dev);
> +
> +	return ret;
> +}

Let's reduce code duplication:

static int bq24190_vbus_set(dev, val) {
    ...
}

static int bq24190_vbus_enable(dev) {
    return bq24190_vbus_set(dev, BQ24190_REG_POC_CHG_CONFIG_OTG);
}

static int bq24190_vbus_disable(dev) {
    return bq24190_vbus_set(dev, BQ24190_REG_POC_CHG_CONFIG_CHARGE);
}

> +static int bq24190_vbus_is_enabled(struct regulator_dev *dev)
> +{
> +	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
> +	int ret;
> +	u8 val;
> +
> +	ret = pm_runtime_get_sync(bdi->dev);
> +	if (ret < 0) {
> +		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> +		pm_runtime_put_noidle(bdi->dev);
> +		return ret;
> +	}
> +
> +	ret = bq24190_read_mask(bdi, BQ24190_REG_POC,
> +				BQ24190_REG_POC_CHG_CONFIG_MASK,
> +				BQ24190_REG_POC_CHG_CONFIG_SHIFT, &val);
> +
> +	pm_runtime_mark_last_busy(bdi->dev);
> +	pm_runtime_put_autosuspend(bdi->dev);
> +
> +	return ret ? ret : val == BQ24190_REG_POC_CHG_CONFIG_OTG;
> +}
> +
> +static const struct regulator_ops bq24190_vbus_ops = {
> +	.enable = bq24190_vbus_enable,
> +	.disable = bq24190_vbus_disable,
> +	.is_enabled = bq24190_vbus_is_enabled,
> +};
> +
> +static const struct regulator_desc bq24190_vbus_desc = {
> +	.name = "usb_otg_vbus",
> +	.type = REGULATOR_VOLTAGE,
> +	.owner = THIS_MODULE,
> +	.ops = &bq24190_vbus_ops,
> +	.fixed_uV = 5000000,
> +	.n_voltages = 1,
> +};
> +
> +static const struct regulator_init_data bq24190_vbus_init_data = {
> +	.constraints = {
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +};
> +
> +static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
> +{
> +	struct bq24190_platform_data *pdata = bdi->dev->platform_data;
> +	struct regulator_config cfg = { };
> +	struct regulator_dev *reg;
> +	int ret = 0;
> +
> +	cfg.dev = bdi->dev;
> +	if (pdata && pdata->regulator_init_data)
> +		cfg.init_data = pdata->regulator_init_data;
> +	else
> +		cfg.init_data = &bq24190_vbus_init_data;
> +	cfg.driver_data = bdi;
> +	reg = devm_regulator_register(bdi->dev, &bq24190_vbus_desc, &cfg);
> +	if (IS_ERR(reg)) {
> +		ret = PTR_ERR(reg);
> +		dev_err(bdi->dev, "Can't register regulator: %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +#else
> +static int bq24190_register_vbus_regulator(struct bq24190_dev_info *bdi)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * According to the "Host Mode and default Mode" section of the
>   * manual, a write to any register causes the bq24190 to switch
> @@ -1577,6 +1699,10 @@ static int bq24190_probe(struct i2c_client *client,
>  		goto out_sysfs;
>  	}
>  
> +	ret = bq24190_register_vbus_regulator(bdi);
> +	if (ret < 0)
> +		goto out_sysfs;
> +
>  	if (bdi->extcon) {
>  		INIT_DELAYED_WORK(&bdi->extcon_work, bq24190_extcon_work);
>  		bdi->extcon_nb.notifier_call = bq24190_extcon_event;
> diff --git a/include/linux/power/bq24190_charger.h b/include/linux/power/bq24190_charger.h
> new file mode 100644
> index 0000000..45ce7f1
> --- /dev/null
> +++ b/include/linux/power/bq24190_charger.h
> @@ -0,0 +1,18 @@
> +/*
> + * Platform data for the TI bq24190 battery charger driver.
> + *
> + * 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.
> + */
> +
> +#ifndef _BQ24190_CHARGER_H_
> +#define _BQ24190_CHARGER_H_
> +
> +#include <linux/regulator/machine.h>
> +
> +struct bq24190_platform_data {
> +	const struct regulator_init_data *regulator_init_data;
> +};
> +
> +#endif
> -- 
> 2.9.4
> 

Otherwise looks fine to me.

-- Sebastian

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

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 10/14] power: supply: bq24190_charger: Add input_current_limit property

[Hans de Goede]

Export the input current limit of the charger as a
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property on the charger
power_supply class device.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq24190_charger.c | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 073cd9d..f13f892 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -987,6 +987,33 @@ static int bq24190_charger_set_voltage(struct bq24190_dev_info *bdi,
 			ARRAY_SIZE(bq24190_cvc_vreg_values), val->intval);
 }
 
+static int bq24190_charger_get_iinlimit(struct bq24190_dev_info *bdi,
+		union power_supply_propval *val)
+{
+	int iinlimit, ret;
+
+	ret = bq24190_get_field_val(bdi, BQ24190_REG_ISC,
+			BQ24190_REG_ISC_IINLIM_MASK,
+			BQ24190_REG_ISC_IINLIM_SHIFT,
+			bq24190_isc_iinlim_values,
+			ARRAY_SIZE(bq24190_isc_iinlim_values), &iinlimit);
+	if (ret < 0)
+		return ret;
+
+	val->intval = iinlimit;
+	return 0;
+}
+
+static int bq24190_charger_set_iinlimit(struct bq24190_dev_info *bdi,
+		const union power_supply_propval *val)
+{
+	return bq24190_set_field_val(bdi, BQ24190_REG_ISC,
+			BQ24190_REG_ISC_IINLIM_MASK,
+			BQ24190_REG_ISC_IINLIM_SHIFT,
+			bq24190_isc_iinlim_values,
+			ARRAY_SIZE(bq24190_isc_iinlim_values), val->intval);
+}
+
 static int bq24190_charger_get_property(struct power_supply *psy,
 		enum power_supply_property psp, union power_supply_propval *val)
 {
@@ -1027,6 +1054,9 @@ static int bq24190_charger_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
 		ret = bq24190_charger_get_voltage_max(bdi, val);
 		break;
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = bq24190_charger_get_iinlimit(bdi, val);
+		break;
 	case POWER_SUPPLY_PROP_SCOPE:
 		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
 		ret = 0;
@@ -1078,6 +1108,9 @@ static int bq24190_charger_set_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
 		ret = bq24190_charger_set_voltage(bdi, val);
 		break;
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = bq24190_charger_set_iinlimit(bdi, val);
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -1099,6 +1132,7 @@ static int bq24190_charger_property_is_writeable(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CHARGE_TYPE:
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
 		ret = 1;
 		break;
 	default:
@@ -1118,6 +1152,7 @@ static enum power_supply_property bq24190_charger_properties[] = {
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
 	POWER_SUPPLY_PROP_SCOPE,
 	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_MANUFACTURER,
-- 
2.9.4

Re: [PATCH v2 10/14] power: supply: bq24190_charger: Add input_current_limit property

[Sebastian Reichel]

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

Hi,

On Tue, Aug 15, 2017 at 10:04:58PM +0200, Hans de Goede wrote:
> Export the input current limit of the charger as a
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property on the charger
> power_supply class device.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/bq24190_charger.c | 35 ++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index 073cd9d..f13f892 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -987,6 +987,33 @@ static int bq24190_charger_set_voltage(struct bq24190_dev_info *bdi,
>  			ARRAY_SIZE(bq24190_cvc_vreg_values), val->intval);
>  }
>  
> +static int bq24190_charger_get_iinlimit(struct bq24190_dev_info *bdi,
> +		union power_supply_propval *val)
> +{
> +	int iinlimit, ret;
> +
> +	ret = bq24190_get_field_val(bdi, BQ24190_REG_ISC,
> +			BQ24190_REG_ISC_IINLIM_MASK,
> +			BQ24190_REG_ISC_IINLIM_SHIFT,
> +			bq24190_isc_iinlim_values,
> +			ARRAY_SIZE(bq24190_isc_iinlim_values), &iinlimit);
> +	if (ret < 0)
> +		return ret;
> +
> +	val->intval = iinlimit;
> +	return 0;
> +}
> +
> +static int bq24190_charger_set_iinlimit(struct bq24190_dev_info *bdi,
> +		const union power_supply_propval *val)
> +{
> +	return bq24190_set_field_val(bdi, BQ24190_REG_ISC,
> +			BQ24190_REG_ISC_IINLIM_MASK,
> +			BQ24190_REG_ISC_IINLIM_SHIFT,
> +			bq24190_isc_iinlim_values,
> +			ARRAY_SIZE(bq24190_isc_iinlim_values), val->intval);
> +}
> +
>  static int bq24190_charger_get_property(struct power_supply *psy,
>  		enum power_supply_property psp, union power_supply_propval *val)
>  {
> @@ -1027,6 +1054,9 @@ static int bq24190_charger_get_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>  		ret = bq24190_charger_get_voltage_max(bdi, val);
>  		break;
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = bq24190_charger_get_iinlimit(bdi, val);
> +		break;
>  	case POWER_SUPPLY_PROP_SCOPE:
>  		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
>  		ret = 0;
> @@ -1078,6 +1108,9 @@ static int bq24190_charger_set_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>  		ret = bq24190_charger_set_voltage(bdi, val);
>  		break;
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = bq24190_charger_set_iinlimit(bdi, val);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -1099,6 +1132,7 @@ static int bq24190_charger_property_is_writeable(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_CHARGE_TYPE:
>  	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
>  	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>  		ret = 1;
>  		break;
>  	default:
> @@ -1118,6 +1152,7 @@ static enum power_supply_property bq24190_charger_properties[] = {
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
>  	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>  	POWER_SUPPLY_PROP_SCOPE,
>  	POWER_SUPPLY_PROP_MODEL_NAME,
>  	POWER_SUPPLY_PROP_MANUFACTURER,
> -- 
> 2.9.4
> 

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

[PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Hans de Goede]

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds support for this to the bq24190_charger driver by calling
power_supply_set_input_current_limit_from_supplier helper if the
"input-current-limit-from-supplier" device-property is set.

Note this replaces the functionality to get the current-limit from an
extcon device, which will be removed in a follow-up commit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Wait a bit before applying current-max from our supplier as input-current-limit
 the bq24190 may still be in its power-good wait-state while our supplier is
 done negotating current-max and if we apply the limit to early then the
 input-current-limit will be reset to 0.5A by the bq24190 after its
 power-good wait is done.
---
 drivers/power/supply/bq24190_charger.c | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index f13f892..6f75c8e 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -159,9 +159,11 @@ struct bq24190_dev_info {
 	struct extcon_dev		*extcon;
 	struct notifier_block		extcon_nb;
 	struct delayed_work		extcon_work;
+	struct delayed_work		input_current_limit_work;
 	char				model_name[I2C_NAME_SIZE];
 	bool				initialized;
 	bool				irq_event;
+	bool				input_current_limit_from_supplier;
 	struct mutex			f_reg_lock;
 	u8				f_reg;
 	u8				ss_reg;
@@ -1142,6 +1144,32 @@ static int bq24190_charger_property_is_writeable(struct power_supply *psy,
 	return ret;
 }
 
+static void bq24190_input_current_limit_work(struct work_struct *work)
+{
+	struct bq24190_dev_info *bdi =
+		container_of(work, struct bq24190_dev_info,
+			     input_current_limit_work.work);
+
+	power_supply_set_input_current_limit_from_supplier(bdi->charger);
+}
+
+static void bq24190_charger_external_power_changed(struct power_supply *psy)
+{
+	struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
+
+	/*
+	 * The Power-Good detection may take up to 220ms, sometimes
+	 * the external charger detection is quicker, and the bq24190 will
+	 * reset to iinlim based on its own charger detection (which is not
+	 * hooked up when using external charger detection) resulting in a
+	 * too low default 500mA iinlim. Delay setting the input-current-limit
+	 * for 300ms to avoid this.
+	 */
+	if (bdi->input_current_limit_from_supplier)
+		queue_delayed_work(system_wq, &bdi->input_current_limit_work,
+				   msecs_to_jiffies(300));
+}
+
 static enum power_supply_property bq24190_charger_properties[] = {
 	POWER_SUPPLY_PROP_CHARGE_TYPE,
 	POWER_SUPPLY_PROP_HEALTH,
@@ -1170,6 +1198,7 @@ static const struct power_supply_desc bq24190_charger_desc = {
 	.get_property		= bq24190_charger_get_property,
 	.set_property		= bq24190_charger_set_property,
 	.property_is_writeable	= bq24190_charger_property_is_writeable,
+	.external_power_changed	= bq24190_charger_external_power_changed,
 };
 
 /* Battery power supply property routines */
@@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
 	mutex_init(&bdi->f_reg_lock);
 	bdi->f_reg = 0;
 	bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
+	INIT_DELAYED_WORK(&bdi->input_current_limit_work,
+			  bq24190_input_current_limit_work);
 
 	i2c_set_clientdata(client, bdi);
 
@@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
+	bdi->input_current_limit_from_supplier =
+		device_property_read_bool(dev,
+					  "input-current-limit-from-supplier");
+
 	/*
 	 * Devicetree platforms should get extcon via phandle (not yet supported).
 	 * On ACPI platforms, extcon clients may invoke us with:
-- 
2.9.4

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Liam Breck]

Hi Hans,

On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
>
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
>
> This commit adds support for this to the bq24190_charger driver by calling
> power_supply_set_input_current_limit_from_supplier helper if the
> "input-current-limit-from-supplier" device-property is set.
>
> Note this replaces the functionality to get the current-limit from an
> extcon device, which will be removed in a follow-up commit.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Wait a bit before applying current-max from our supplier as input-current-limit
>  the bq24190 may still be in its power-good wait-state while our supplier is
>  done negotating current-max and if we apply the limit to early then the
>  input-current-limit will be reset to 0.5A by the bq24190 after its
>  power-good wait is done.
> ---
>  drivers/power/supply/bq24190_charger.c | 35 ++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index f13f892..6f75c8e 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>         struct extcon_dev               *extcon;
>         struct notifier_block           extcon_nb;
>         struct delayed_work             extcon_work;
> +       struct delayed_work             input_current_limit_work;
>         char                            model_name[I2C_NAME_SIZE];
>         bool                            initialized;
>         bool                            irq_event;
> +       bool                            input_current_limit_from_supplier;
>         struct mutex                    f_reg_lock;
>         u8                              f_reg;
>         u8                              ss_reg;
> @@ -1142,6 +1144,32 @@ static int bq24190_charger_property_is_writeable(struct power_supply *psy,
>         return ret;
>  }
>
> +static void bq24190_input_current_limit_work(struct work_struct *work)
> +{
> +       struct bq24190_dev_info *bdi =
> +               container_of(work, struct bq24190_dev_info,
> +                            input_current_limit_work.work);
> +
> +       power_supply_set_input_current_limit_from_supplier(bdi->charger);
> +}
> +
> +static void bq24190_charger_external_power_changed(struct power_supply *psy)
> +{
> +       struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
> +
> +       /*
> +        * The Power-Good detection may take up to 220ms, sometimes
> +        * the external charger detection is quicker, and the bq24190 will
> +        * reset to iinlim based on its own charger detection (which is not
> +        * hooked up when using external charger detection) resulting in a
> +        * too low default 500mA iinlim. Delay setting the input-current-limit
> +        * for 300ms to avoid this.
> +        */
> +       if (bdi->input_current_limit_from_supplier)
> +               queue_delayed_work(system_wq, &bdi->input_current_limit_work,
> +                                  msecs_to_jiffies(300));
> +}
> +
>  static enum power_supply_property bq24190_charger_properties[] = {
>         POWER_SUPPLY_PROP_CHARGE_TYPE,
>         POWER_SUPPLY_PROP_HEALTH,
> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc bq24190_charger_desc = {
>         .get_property           = bq24190_charger_get_property,
>         .set_property           = bq24190_charger_set_property,
>         .property_is_writeable  = bq24190_charger_property_is_writeable,
> +       .external_power_changed = bq24190_charger_external_power_changed,
>  };
>
>  /* Battery power supply property routines */
> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>         mutex_init(&bdi->f_reg_lock);
>         bdi->f_reg = 0;
>         bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
> +       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
> +                         bq24190_input_current_limit_work);
>
>         i2c_set_clientdata(client, bdi);
>
> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client *client,
>                 return -EINVAL;
>         }
>
> +       bdi->input_current_limit_from_supplier =
> +               device_property_read_bool(dev,
> +                                         "input-current-limit-from-supplier");
> +

Maybe
  if (device_property_read_bool(dev,
"linux,input-current-limit-from-supplier")) {
       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
                         bq24190_input_current_limit_work);
      bdi->input_current_limit_from_supplier = true; // or use a field
in bdi->input_current_limit_work as the flag?
   }

Also should document property for DT use assuming Sebastian is ok with
new power_supply method.

And can you rebase to my pending series in next pass? There's nothing
controversial in it :-)

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Hans de Goede]

Hi,

On 16-08-17 22:28, Liam Breck wrote:
> Hi Hans,
> 
> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>> done by a separate port-controller IC, while the current limit is
>> controlled through another (charger) IC.
>>
>> It has been decided to model this by modelling the external Type-C
>> power brick (adapter/charger) as a power-supply class device which
>> supplies the charger-IC, with its voltage-now and current-max representing
>> the negotiated voltage and max current draw.
>>
>> This commit adds support for this to the bq24190_charger driver by calling
>> power_supply_set_input_current_limit_from_supplier helper if the
>> "input-current-limit-from-supplier" device-property is set.
>>
>> Note this replaces the functionality to get the current-limit from an
>> extcon device, which will be removed in a follow-up commit.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Wait a bit before applying current-max from our supplier as input-current-limit
>>   the bq24190 may still be in its power-good wait-state while our supplier is
>>   done negotating current-max and if we apply the limit to early then the
>>   input-current-limit will be reset to 0.5A by the bq24190 after its
>>   power-good wait is done.
>> ---
>>   drivers/power/supply/bq24190_charger.c | 35 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
>> index f13f892..6f75c8e 100644
>> --- a/drivers/power/supply/bq24190_charger.c
>> +++ b/drivers/power/supply/bq24190_charger.c
>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>          struct extcon_dev               *extcon;
>>          struct notifier_block           extcon_nb;
>>          struct delayed_work             extcon_work;
>> +       struct delayed_work             input_current_limit_work;
>>          char                            model_name[I2C_NAME_SIZE];
>>          bool                            initialized;
>>          bool                            irq_event;
>> +       bool                            input_current_limit_from_supplier;
>>          struct mutex                    f_reg_lock;
>>          u8                              f_reg;
>>          u8                              ss_reg;
>> @@ -1142,6 +1144,32 @@ static int bq24190_charger_property_is_writeable(struct power_supply *psy,
>>          return ret;
>>   }
>>
>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>> +{
>> +       struct bq24190_dev_info *bdi =
>> +               container_of(work, struct bq24190_dev_info,
>> +                            input_current_limit_work.work);
>> +
>> +       power_supply_set_input_current_limit_from_supplier(bdi->charger);
>> +}
>> +
>> +static void bq24190_charger_external_power_changed(struct power_supply *psy)
>> +{
>> +       struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>> +
>> +       /*
>> +        * The Power-Good detection may take up to 220ms, sometimes
>> +        * the external charger detection is quicker, and the bq24190 will
>> +        * reset to iinlim based on its own charger detection (which is not
>> +        * hooked up when using external charger detection) resulting in a
>> +        * too low default 500mA iinlim. Delay setting the input-current-limit
>> +        * for 300ms to avoid this.
>> +        */
>> +       if (bdi->input_current_limit_from_supplier)
>> +               queue_delayed_work(system_wq, &bdi->input_current_limit_work,
>> +                                  msecs_to_jiffies(300));
>> +}
>> +
>>   static enum power_supply_property bq24190_charger_properties[] = {
>>          POWER_SUPPLY_PROP_CHARGE_TYPE,
>>          POWER_SUPPLY_PROP_HEALTH,
>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc bq24190_charger_desc = {
>>          .get_property           = bq24190_charger_get_property,
>>          .set_property           = bq24190_charger_set_property,
>>          .property_is_writeable  = bq24190_charger_property_is_writeable,
>> +       .external_power_changed = bq24190_charger_external_power_changed,
>>   };
>>
>>   /* Battery power supply property routines */
>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>          mutex_init(&bdi->f_reg_lock);
>>          bdi->f_reg = 0;
>>          bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
>> +       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>> +                         bq24190_input_current_limit_work);
>>
>>          i2c_set_clientdata(client, bdi);
>>
>> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client *client,
>>                  return -EINVAL;
>>          }
>>
>> +       bdi->input_current_limit_from_supplier =
>> +               device_property_read_bool(dev,
>> +                                         "input-current-limit-from-supplier");
>> +
> 
> Maybe
>    if (device_property_read_bool(dev,
> "linux,input-current-limit-from-supplier")) {
>         INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>                           bq24190_input_current_limit_work);
>        bdi->input_current_limit_from_supplier = true; // or use a field
> in bdi->input_current_limit_work as the flag?
>     }

Done, except for making the flag a field in input_current_limit_work,
input_current_limit_work is of type struct delayed_work so I cannot just
add a field.

> Also should document property for DT use assuming Sebastian is ok with
> new power_supply method.

Also done for v3 of this patch.

> And can you rebase to my pending series in next pass? There's nothing
> controversial in it :-)

I've cherry-picked v3 of the patch adding the dt-binding-doc to my tree so
that I could add the documentation for the property on top. I will make sure
to rebase on top of the rest once the rest is merged by Sebastian (otherwise
I need to rebase after each revision of the patch-set).

Regards,

Hans

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Liam Breck]

Hi Hans,

On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 16-08-17 22:28, Liam Breck wrote:
>>
>> Hi Hans,
>>
>> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>>> done by a separate port-controller IC, while the current limit is
>>> controlled through another (charger) IC.
>>>
>>> It has been decided to model this by modelling the external Type-C
>>> power brick (adapter/charger) as a power-supply class device which
>>> supplies the charger-IC, with its voltage-now and current-max
>>> representing
>>> the negotiated voltage and max current draw.
>>>
>>> This commit adds support for this to the bq24190_charger driver by
>>> calling
>>> power_supply_set_input_current_limit_from_supplier helper if the
>>> "input-current-limit-from-supplier" device-property is set.
>>>
>>> Note this replaces the functionality to get the current-limit from an
>>> extcon device, which will be removed in a follow-up commit.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> -Wait a bit before applying current-max from our supplier as
>>> input-current-limit
>>>   the bq24190 may still be in its power-good wait-state while our
>>> supplier is
>>>   done negotating current-max and if we apply the limit to early then the
>>>   input-current-limit will be reset to 0.5A by the bq24190 after its
>>>   power-good wait is done.
>>> ---
>>>   drivers/power/supply/bq24190_charger.c | 35
>>> ++++++++++++++++++++++++++++++++++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/power/supply/bq24190_charger.c
>>> b/drivers/power/supply/bq24190_charger.c
>>> index f13f892..6f75c8e 100644
>>> --- a/drivers/power/supply/bq24190_charger.c
>>> +++ b/drivers/power/supply/bq24190_charger.c
>>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>>          struct extcon_dev               *extcon;
>>>          struct notifier_block           extcon_nb;
>>>          struct delayed_work             extcon_work;
>>> +       struct delayed_work             input_current_limit_work;
>>>          char                            model_name[I2C_NAME_SIZE];
>>>          bool                            initialized;
>>>          bool                            irq_event;
>>> +       bool
>>> input_current_limit_from_supplier;
>>>          struct mutex                    f_reg_lock;
>>>          u8                              f_reg;
>>>          u8                              ss_reg;
>>> @@ -1142,6 +1144,32 @@ static int
>>> bq24190_charger_property_is_writeable(struct power_supply *psy,
>>>          return ret;
>>>   }
>>>
>>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>>> +{
>>> +       struct bq24190_dev_info *bdi =
>>> +               container_of(work, struct bq24190_dev_info,
>>> +                            input_current_limit_work.work);
>>> +
>>> +       power_supply_set_input_current_limit_from_supplier(bdi->charger);
>>> +}
>>> +
>>> +static void bq24190_charger_external_power_changed(struct power_supply
>>> *psy)
>>> +{
>>> +       struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>>> +
>>> +       /*
>>> +        * The Power-Good detection may take up to 220ms, sometimes
>>> +        * the external charger detection is quicker, and the bq24190
>>> will
>>> +        * reset to iinlim based on its own charger detection (which is
>>> not
>>> +        * hooked up when using external charger detection) resulting in
>>> a
>>> +        * too low default 500mA iinlim. Delay setting the
>>> input-current-limit
>>> +        * for 300ms to avoid this.
>>> +        */
>>> +       if (bdi->input_current_limit_from_supplier)
>>> +               queue_delayed_work(system_wq,
>>> &bdi->input_current_limit_work,
>>> +                                  msecs_to_jiffies(300));
>>> +}
>>> +
>>>   static enum power_supply_property bq24190_charger_properties[] = {
>>>          POWER_SUPPLY_PROP_CHARGE_TYPE,
>>>          POWER_SUPPLY_PROP_HEALTH,
>>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc
>>> bq24190_charger_desc = {
>>>          .get_property           = bq24190_charger_get_property,
>>>          .set_property           = bq24190_charger_set_property,
>>>          .property_is_writeable  = bq24190_charger_property_is_writeable,
>>> +       .external_power_changed = bq24190_charger_external_power_changed,
>>>   };
>>>
>>>   /* Battery power supply property routines */
>>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>>          mutex_init(&bdi->f_reg_lock);
>>>          bdi->f_reg = 0;
>>>          bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
>>> */
>>> +       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>> +                         bq24190_input_current_limit_work);
>>>
>>>          i2c_set_clientdata(client, bdi);
>>>
>>> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client
>>> *client,
>>>                  return -EINVAL;
>>>          }
>>>
>>> +       bdi->input_current_limit_from_supplier =
>>> +               device_property_read_bool(dev,
>>> +
>>> "input-current-limit-from-supplier");
>>> +
>>
>>
>> Maybe
>>    if (device_property_read_bool(dev,
>> "linux,input-current-limit-from-supplier")) {
>>         INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>                           bq24190_input_current_limit_work);
>>        bdi->input_current_limit_from_supplier = true; // or use a field
>> in bdi->input_current_limit_work as the flag?
>>     }
>
>
> Done, except for making the flag a field in input_current_limit_work,
> input_current_limit_work is of type struct delayed_work so I cannot just
> add a field.

What I meant was you could check an existing field in delayed_work to
see if it was init'd.

>> Also should document property for DT use assuming Sebastian is ok with
>> new power_supply method.
>
> Also done for v3 of this patch.
>
>> And can you rebase to my pending series in next pass? There's nothing
>> controversial in it :-)
>
>
> I've cherry-picked v3 of the patch adding the dt-binding-doc to my tree so
> that I could add the documentation for the property on top. I will make sure
> to rebase on top of the rest once the rest is merged by Sebastian (otherwise
> I need to rebase after each revision of the patch-set).

Sounds good.

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Liam Breck]

Hi Hans, I sent too soon...

On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 16-08-17 22:28, Liam Breck wrote:
>>
>> Hi Hans,
>>
>> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>>> done by a separate port-controller IC, while the current limit is
>>> controlled through another (charger) IC.
>>>
>>> It has been decided to model this by modelling the external Type-C
>>> power brick (adapter/charger) as a power-supply class device which
>>> supplies the charger-IC, with its voltage-now and current-max
>>> representing
>>> the negotiated voltage and max current draw.
>>>
>>> This commit adds support for this to the bq24190_charger driver by
>>> calling
>>> power_supply_set_input_current_limit_from_supplier helper if the
>>> "input-current-limit-from-supplier" device-property is set.
>>>
>>> Note this replaces the functionality to get the current-limit from an
>>> extcon device, which will be removed in a follow-up commit.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> -Wait a bit before applying current-max from our supplier as
>>> input-current-limit
>>>   the bq24190 may still be in its power-good wait-state while our
>>> supplier is
>>>   done negotating current-max and if we apply the limit to early then the
>>>   input-current-limit will be reset to 0.5A by the bq24190 after its
>>>   power-good wait is done.
>>> ---
>>>   drivers/power/supply/bq24190_charger.c | 35
>>> ++++++++++++++++++++++++++++++++++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/power/supply/bq24190_charger.c
>>> b/drivers/power/supply/bq24190_charger.c
>>> index f13f892..6f75c8e 100644
>>> --- a/drivers/power/supply/bq24190_charger.c
>>> +++ b/drivers/power/supply/bq24190_charger.c
>>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>>          struct extcon_dev               *extcon;
>>>          struct notifier_block           extcon_nb;
>>>          struct delayed_work             extcon_work;
>>> +       struct delayed_work             input_current_limit_work;
>>>          char                            model_name[I2C_NAME_SIZE];
>>>          bool                            initialized;
>>>          bool                            irq_event;
>>> +       bool
>>> input_current_limit_from_supplier;
>>>          struct mutex                    f_reg_lock;
>>>          u8                              f_reg;
>>>          u8                              ss_reg;
>>> @@ -1142,6 +1144,32 @@ static int
>>> bq24190_charger_property_is_writeable(struct power_supply *psy,
>>>          return ret;
>>>   }
>>>
>>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>>> +{
>>> +       struct bq24190_dev_info *bdi =
>>> +               container_of(work, struct bq24190_dev_info,
>>> +                            input_current_limit_work.work);
>>> +
>>> +       power_supply_set_input_current_limit_from_supplier(bdi->charger);
>>> +}
>>> +
>>> +static void bq24190_charger_external_power_changed(struct power_supply
>>> *psy)
>>> +{
>>> +       struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>>> +
>>> +       /*
>>> +        * The Power-Good detection may take up to 220ms, sometimes
>>> +        * the external charger detection is quicker, and the bq24190
>>> will
>>> +        * reset to iinlim based on its own charger detection (which is
>>> not
>>> +        * hooked up when using external charger detection) resulting in
>>> a
>>> +        * too low default 500mA iinlim. Delay setting the
>>> input-current-limit
>>> +        * for 300ms to avoid this.
>>> +        */
>>> +       if (bdi->input_current_limit_from_supplier)
>>> +               queue_delayed_work(system_wq,
>>> &bdi->input_current_limit_work,
>>> +                                  msecs_to_jiffies(300));
>>> +}
>>> +
>>>   static enum power_supply_property bq24190_charger_properties[] = {
>>>          POWER_SUPPLY_PROP_CHARGE_TYPE,
>>>          POWER_SUPPLY_PROP_HEALTH,
>>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc
>>> bq24190_charger_desc = {
>>>          .get_property           = bq24190_charger_get_property,
>>>          .set_property           = bq24190_charger_set_property,
>>>          .property_is_writeable  = bq24190_charger_property_is_writeable,
>>> +       .external_power_changed = bq24190_charger_external_power_changed,
>>>   };
>>>
>>>   /* Battery power supply property routines */
>>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>>          mutex_init(&bdi->f_reg_lock);
>>>          bdi->f_reg = 0;
>>>          bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
>>> */
>>> +       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>> +                         bq24190_input_current_limit_work);
>>>
>>>          i2c_set_clientdata(client, bdi);
>>>
>>> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client
>>> *client,
>>>                  return -EINVAL;
>>>          }
>>>
>>> +       bdi->input_current_limit_from_supplier =
>>> +               device_property_read_bool(dev,
>>> +
>>> "input-current-limit-from-supplier");
>>> +
>>
>>
>> Maybe
>>    if (device_property_read_bool(dev,
>> "linux,input-current-limit-from-supplier")) {
>>         INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>                           bq24190_input_current_limit_work);
>>        bdi->input_current_limit_from_supplier = true; // or use a field
>> in bdi->input_current_limit_work as the flag?
>>     }
>
>
> Done, except for making the flag a field in input_current_limit_work,
> input_current_limit_work is of type struct delayed_work so I cannot just
> add a field.
>
>> Also should document property for DT use assuming Sebastian is ok with
>> new power_supply method.
>
>
> Also done for v3 of this patch.
>
>> And can you rebase to my pending series in next pass? There's nothing
>> controversial in it :-)
>
>
> I've cherry-picked v3 of the patch adding the dt-binding-doc to my tree so
> that I could add the documentation for the property on top. I will make sure
> to rebase on top of the rest once the rest is merged by Sebastian (otherwise
> I need to rebase after each revision of the patch-set).

If the property is linux,input-current-limit-from-supplier should it
also be documented elsewhere in DT bindings?

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Liam Breck]

Hi Hans, I sent too soon...

On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 16-08-17 22:28, Liam Breck wrote:
>>
>> Hi Hans,
>>
>> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>>> done by a separate port-controller IC, while the current limit is
>>> controlled through another (charger) IC.
>>>
>>> It has been decided to model this by modelling the external Type-C
>>> power brick (adapter/charger) as a power-supply class device which
>>> supplies the charger-IC, with its voltage-now and current-max
>>> representing
>>> the negotiated voltage and max current draw.
>>>
>>> This commit adds support for this to the bq24190_charger driver by
>>> calling
>>> power_supply_set_input_current_limit_from_supplier helper if the
>>> "input-current-limit-from-supplier" device-property is set.
>>>
>>> Note this replaces the functionality to get the current-limit from an
>>> extcon device, which will be removed in a follow-up commit.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> -Wait a bit before applying current-max from our supplier as
>>> input-current-limit
>>>   the bq24190 may still be in its power-good wait-state while our
>>> supplier is
>>>   done negotating current-max and if we apply the limit to early then the
>>>   input-current-limit will be reset to 0.5A by the bq24190 after its
>>>   power-good wait is done.
>>> ---
>>>   drivers/power/supply/bq24190_charger.c | 35
>>> ++++++++++++++++++++++++++++++++++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/power/supply/bq24190_charger.c
>>> b/drivers/power/supply/bq24190_charger.c
>>> index f13f892..6f75c8e 100644
>>> --- a/drivers/power/supply/bq24190_charger.c
>>> +++ b/drivers/power/supply/bq24190_charger.c
>>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>>          struct extcon_dev               *extcon;
>>>          struct notifier_block           extcon_nb;
>>>          struct delayed_work             extcon_work;
>>> +       struct delayed_work             input_current_limit_work;
>>>          char                            model_name[I2C_NAME_SIZE];
>>>          bool                            initialized;
>>>          bool                            irq_event;
>>> +       bool
>>> input_current_limit_from_supplier;
>>>          struct mutex                    f_reg_lock;
>>>          u8                              f_reg;
>>>          u8                              ss_reg;
>>> @@ -1142,6 +1144,32 @@ static int
>>> bq24190_charger_property_is_writeable(struct power_supply *psy,
>>>          return ret;
>>>   }
>>>
>>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>>> +{
>>> +       struct bq24190_dev_info *bdi =
>>> +               container_of(work, struct bq24190_dev_info,
>>> +                            input_current_limit_work.work);
>>> +
>>> +       power_supply_set_input_current_limit_from_supplier(bdi->charger);
>>> +}
>>> +
>>> +static void bq24190_charger_external_power_changed(struct power_supply
>>> *psy)
>>> +{
>>> +       struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>>> +
>>> +       /*
>>> +        * The Power-Good detection may take up to 220ms, sometimes
>>> +        * the external charger detection is quicker, and the bq24190
>>> will
>>> +        * reset to iinlim based on its own charger detection (which is
>>> not
>>> +        * hooked up when using external charger detection) resulting in
>>> a
>>> +        * too low default 500mA iinlim. Delay setting the
>>> input-current-limit
>>> +        * for 300ms to avoid this.
>>> +        */
>>> +       if (bdi->input_current_limit_from_supplier)
>>> +               queue_delayed_work(system_wq,
>>> &bdi->input_current_limit_work,
>>> +                                  msecs_to_jiffies(300));
>>> +}
>>> +
>>>   static enum power_supply_property bq24190_charger_properties[] = {
>>>          POWER_SUPPLY_PROP_CHARGE_TYPE,
>>>          POWER_SUPPLY_PROP_HEALTH,
>>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc
>>> bq24190_charger_desc = {
>>>          .get_property           = bq24190_charger_get_property,
>>>          .set_property           = bq24190_charger_set_property,
>>>          .property_is_writeable  = bq24190_charger_property_is_writeable,
>>> +       .external_power_changed = bq24190_charger_external_power_changed,
>>>   };
>>>
>>>   /* Battery power supply property routines */
>>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>>          mutex_init(&bdi->f_reg_lock);
>>>          bdi->f_reg = 0;
>>>          bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
>>> */
>>> +       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>> +                         bq24190_input_current_limit_work);
>>>
>>>          i2c_set_clientdata(client, bdi);
>>>
>>> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client
>>> *client,
>>>                  return -EINVAL;
>>>          }
>>>
>>> +       bdi->input_current_limit_from_supplier =
>>> +               device_property_read_bool(dev,
>>> +
>>> "input-current-limit-from-supplier");
>>> +
>>
>>
>> Maybe
>>    if (device_property_read_bool(dev,
>> "linux,input-current-limit-from-supplier")) {
>>         INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>                           bq24190_input_current_limit_work);
>>        bdi->input_current_limit_from_supplier = true; // or use a field
>> in bdi->input_current_limit_work as the flag?
>>     }
>
>
> Done, except for making the flag a field in input_current_limit_work,
> input_current_limit_work is of type struct delayed_work so I cannot just
> add a field.
>
>> Also should document property for DT use assuming Sebastian is ok with
>> new power_supply method.
>
>
> Also done for v3 of this patch.
>
>> And can you rebase to my pending series in next pass? There's nothing
>> controversial in it :-)
>
>
> I've cherry-picked v3 of the patch adding the dt-binding-doc to my tree so
> that I could add the documentation for the property on top. I will make sure
> to rebase on top of the rest once the rest is merged by Sebastian (otherwise
> I need to rebase after each revision of the patch-set).

If the property is linux,input-current-limit-from-supplier should it
also be documented elsewhere in DT bindings?

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Hans de Goede]

Hi,

On 28-08-17 20:07, Liam Breck wrote:
> Hi Hans, I sent too soon...
> 
> On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 16-08-17 22:28, Liam Breck wrote:
>>>
>>> Hi Hans,
>>>
>>> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>>>> done by a separate port-controller IC, while the current limit is
>>>> controlled through another (charger) IC.
>>>>
>>>> It has been decided to model this by modelling the external Type-C
>>>> power brick (adapter/charger) as a power-supply class device which
>>>> supplies the charger-IC, with its voltage-now and current-max
>>>> representing
>>>> the negotiated voltage and max current draw.
>>>>
>>>> This commit adds support for this to the bq24190_charger driver by
>>>> calling
>>>> power_supply_set_input_current_limit_from_supplier helper if the
>>>> "input-current-limit-from-supplier" device-property is set.
>>>>
>>>> Note this replaces the functionality to get the current-limit from an
>>>> extcon device, which will be removed in a follow-up commit.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> Changes in v2:
>>>> -Wait a bit before applying current-max from our supplier as
>>>> input-current-limit
>>>>    the bq24190 may still be in its power-good wait-state while our
>>>> supplier is
>>>>    done negotating current-max and if we apply the limit to early then the
>>>>    input-current-limit will be reset to 0.5A by the bq24190 after its
>>>>    power-good wait is done.
>>>> ---
>>>>    drivers/power/supply/bq24190_charger.c | 35
>>>> ++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 35 insertions(+)
>>>>
>>>> diff --git a/drivers/power/supply/bq24190_charger.c
>>>> b/drivers/power/supply/bq24190_charger.c
>>>> index f13f892..6f75c8e 100644
>>>> --- a/drivers/power/supply/bq24190_charger.c
>>>> +++ b/drivers/power/supply/bq24190_charger.c
>>>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>>>           struct extcon_dev               *extcon;
>>>>           struct notifier_block           extcon_nb;
>>>>           struct delayed_work             extcon_work;
>>>> +       struct delayed_work             input_current_limit_work;
>>>>           char                            model_name[I2C_NAME_SIZE];
>>>>           bool                            initialized;
>>>>           bool                            irq_event;
>>>> +       bool
>>>> input_current_limit_from_supplier;
>>>>           struct mutex                    f_reg_lock;
>>>>           u8                              f_reg;
>>>>           u8                              ss_reg;
>>>> @@ -1142,6 +1144,32 @@ static int
>>>> bq24190_charger_property_is_writeable(struct power_supply *psy,
>>>>           return ret;
>>>>    }
>>>>
>>>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>>>> +{
>>>> +       struct bq24190_dev_info *bdi =
>>>> +               container_of(work, struct bq24190_dev_info,
>>>> +                            input_current_limit_work.work);
>>>> +
>>>> +       power_supply_set_input_current_limit_from_supplier(bdi->charger);
>>>> +}
>>>> +
>>>> +static void bq24190_charger_external_power_changed(struct power_supply
>>>> *psy)
>>>> +{
>>>> +       struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>>>> +
>>>> +       /*
>>>> +        * The Power-Good detection may take up to 220ms, sometimes
>>>> +        * the external charger detection is quicker, and the bq24190
>>>> will
>>>> +        * reset to iinlim based on its own charger detection (which is
>>>> not
>>>> +        * hooked up when using external charger detection) resulting in
>>>> a
>>>> +        * too low default 500mA iinlim. Delay setting the
>>>> input-current-limit
>>>> +        * for 300ms to avoid this.
>>>> +        */
>>>> +       if (bdi->input_current_limit_from_supplier)
>>>> +               queue_delayed_work(system_wq,
>>>> &bdi->input_current_limit_work,
>>>> +                                  msecs_to_jiffies(300));
>>>> +}
>>>> +
>>>>    static enum power_supply_property bq24190_charger_properties[] = {
>>>>           POWER_SUPPLY_PROP_CHARGE_TYPE,
>>>>           POWER_SUPPLY_PROP_HEALTH,
>>>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc
>>>> bq24190_charger_desc = {
>>>>           .get_property           = bq24190_charger_get_property,
>>>>           .set_property           = bq24190_charger_set_property,
>>>>           .property_is_writeable  = bq24190_charger_property_is_writeable,
>>>> +       .external_power_changed = bq24190_charger_external_power_changed,
>>>>    };
>>>>
>>>>    /* Battery power supply property routines */
>>>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>>>           mutex_init(&bdi->f_reg_lock);
>>>>           bdi->f_reg = 0;
>>>>           bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
>>>> */
>>>> +       INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>>> +                         bq24190_input_current_limit_work);
>>>>
>>>>           i2c_set_clientdata(client, bdi);
>>>>
>>>> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client
>>>> *client,
>>>>                   return -EINVAL;
>>>>           }
>>>>
>>>> +       bdi->input_current_limit_from_supplier =
>>>> +               device_property_read_bool(dev,
>>>> +
>>>> "input-current-limit-from-supplier");
>>>> +
>>>
>>>
>>> Maybe
>>>     if (device_property_read_bool(dev,
>>> "linux,input-current-limit-from-supplier")) {
>>>          INIT_DELAYED_WORK(&bdi->input_current_limit_work,
>>>                            bq24190_input_current_limit_work);
>>>         bdi->input_current_limit_from_supplier = true; // or use a field
>>> in bdi->input_current_limit_work as the flag?
>>>      }
>>
>>
>> Done, except for making the flag a field in input_current_limit_work,
>> input_current_limit_work is of type struct delayed_work so I cannot just
>> add a field.
>>
>>> Also should document property for DT use assuming Sebastian is ok with
>>> new power_supply method.
>>
>>
>> Also done for v3 of this patch.
>>
>>> And can you rebase to my pending series in next pass? There's nothing
>>> controversial in it :-)
>>
>>
>> I've cherry-picked v3 of the patch adding the dt-binding-doc to my tree so
>> that I could add the documentation for the property on top. I will make sure
>> to rebase on top of the rest once the rest is merged by Sebastian (otherwise
>> I need to rebase after each revision of the patch-set).
> 
> If the property is linux,input-current-limit-from-supplier should it
> also be documented elsewhere in DT bindings?

linux, just means that it is Linux specific, it can still be a
per-device binding, although maybe we should make it a generic
power-supply consumer property? And at it to:

Documentation/devicetree/bindings/power/supply/power_supply.txt

Since that binding does not appear to be Linux specific, I've
gone with ti,input-current-limit-from-supplier for now and I've
documented it in the bq24190 bindings for now.

I've also added the device-tree binding maintainers to the Cc,
for the still to be send v3 of the patch-set, so lets just wait
and see what they have to say.

Regards,

Hans

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Sebastian Reichel]

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

Hi,

On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
> 
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
> 
> This commit adds support for this to the bq24190_charger driver by calling
> power_supply_set_input_current_limit_from_supplier helper if the
> "input-current-limit-from-supplier" device-property is set.
> 
> Note this replaces the functionality to get the current-limit from an
> extcon device, which will be removed in a follow-up commit.

I'm fine with the general approach, but ...

> [...]
> +	bdi->input_current_limit_from_supplier =
> +		device_property_read_bool(dev,
> +					  "input-current-limit-from-supplier");
> [...]

I wonder if we actually need this. I think we can just enable it
unconditionally when we have a parent power supply providing the
information.

-- Sebastian

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

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Hans de Goede]

Hi,

Thank you for your reviews / queuing!

On 29-08-17 13:40, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:
>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>> done by a separate port-controller IC, while the current limit is
>> controlled through another (charger) IC.
>>
>> It has been decided to model this by modelling the external Type-C
>> power brick (adapter/charger) as a power-supply class device which
>> supplies the charger-IC, with its voltage-now and current-max representing
>> the negotiated voltage and max current draw.
>>
>> This commit adds support for this to the bq24190_charger driver by calling
>> power_supply_set_input_current_limit_from_supplier helper if the
>> "input-current-limit-from-supplier" device-property is set.
>>
>> Note this replaces the functionality to get the current-limit from an
>> extcon device, which will be removed in a follow-up commit.
> 
> I'm fine with the general approach, but ...
> 
>> [...]
>> +	bdi->input_current_limit_from_supplier =
>> +		device_property_read_bool(dev,
>> +					  "input-current-limit-from-supplier");
>> [...]
> 
> I wonder if we actually need this. I think we can just enable it
> unconditionally when we have a parent power supply providing the
> information.

I was thinking the same when implementing this, so this is fine with
me. I think it is best to just unconditionally call
power_supply_set_input_current_limit_from_supplier from the
external_power_changed callback, that will only get called if we've
a parent supply and that function will check that the parent has
a current-max property itself.

Please let me know if just unconditionally calling
power_supply_set_input_current_limit_from_supplier from the
external_power_changed callback is ok with you then I will do that
for v3 of the patch-set (from which I will drop the patches you've
already queued).

Regards,

Hans

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Hans de Goede]

Hi,

Thank you for your reviews / queuing!

On 29-08-17 13:40, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:
>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>> done by a separate port-controller IC, while the current limit is
>> controlled through another (charger) IC.
>>
>> It has been decided to model this by modelling the external Type-C
>> power brick (adapter/charger) as a power-supply class device which
>> supplies the charger-IC, with its voltage-now and current-max representing
>> the negotiated voltage and max current draw.
>>
>> This commit adds support for this to the bq24190_charger driver by calling
>> power_supply_set_input_current_limit_from_supplier helper if the
>> "input-current-limit-from-supplier" device-property is set.
>>
>> Note this replaces the functionality to get the current-limit from an
>> extcon device, which will be removed in a follow-up commit.
> 
> I'm fine with the general approach, but ...
> 
>> [...]
>> +	bdi->input_current_limit_from_supplier =
>> +		device_property_read_bool(dev,
>> +					  "input-current-limit-from-supplier");
>> [...]
> 
> I wonder if we actually need this. I think we can just enable it
> unconditionally when we have a parent power supply providing the
> information.

I was thinking the same when implementing this, so this is fine with
me. I think it is best to just unconditionally call
power_supply_set_input_current_limit_from_supplier from the
external_power_changed callback, that will only get called if we've
a parent supply and that function will check that the parent has
a current-max property itself.

Please let me know if just unconditionally calling
power_supply_set_input_current_limit_from_supplier from the
external_power_changed callback is ok with you then I will do that
for v3 of the patch-set (from which I will drop the patches you've
already queued).

Regards,

Hans

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[Sebastian Reichel]

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

Hi,

On Tue, Aug 29, 2017 at 01:53:24PM +0200, Hans de Goede wrote:
> Hi,
> 
> Thank you for your reviews / queuing!
> 
> On 29-08-17 13:40, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:
> > > On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> > > done by a separate port-controller IC, while the current limit is
> > > controlled through another (charger) IC.
> > > 
> > > It has been decided to model this by modelling the external Type-C
> > > power brick (adapter/charger) as a power-supply class device which
> > > supplies the charger-IC, with its voltage-now and current-max representing
> > > the negotiated voltage and max current draw.
> > > 
> > > This commit adds support for this to the bq24190_charger driver by calling
> > > power_supply_set_input_current_limit_from_supplier helper if the
> > > "input-current-limit-from-supplier" device-property is set.
> > > 
> > > Note this replaces the functionality to get the current-limit from an
> > > extcon device, which will be removed in a follow-up commit.
> > 
> > I'm fine with the general approach, but ...
> > 
> > > [...]
> > > +	bdi->input_current_limit_from_supplier =
> > > +		device_property_read_bool(dev,
> > > +					  "input-current-limit-from-supplier");
> > > [...]
> > 
> > I wonder if we actually need this. I think we can just enable it
> > unconditionally when we have a parent power supply providing the
> > information.
> 
> I was thinking the same when implementing this, so this is fine with
> me. I think it is best to just unconditionally call
> power_supply_set_input_current_limit_from_supplier from the
> external_power_changed callback, that will only get called if we've
> a parent supply and that function will check that the parent has
> a current-max property itself.
> 
> Please let me know if just unconditionally calling
> power_supply_set_input_current_limit_from_supplier from the
> external_power_changed callback is ok with you then I will do that
> for v3 of the patch-set (from which I will drop the patches you've
> already queued).

Makes sense to me.

-- Sebastian

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

[PATCH v2 12/14] power: supply: bq24190_charger: Remove extcon handling

[Hans de Goede]

Now that drivers/i2c/busses/i2c-cht-wc.c uses
"input-current-limit-from-supplier" instead of "extcon-name" the last
user of the bq24190 extcon code is gone, remove it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Move the comment with the example code for passing properties on i2c_client
 instantion to the input-current-limit-from-supplier handling to preserve
 the example code
---
 drivers/power/supply/bq24190_charger.c | 107 ++-------------------------------
 1 file changed, 5 insertions(+), 102 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 6f75c8e..3721a7f 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -11,7 +11,6 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
-#include <linux/extcon.h>
 #include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
@@ -156,9 +155,6 @@ struct bq24190_dev_info {
 	struct device			*dev;
 	struct power_supply		*charger;
 	struct power_supply		*battery;
-	struct extcon_dev		*extcon;
-	struct notifier_block		extcon_nb;
-	struct delayed_work		extcon_work;
 	struct delayed_work		input_current_limit_work;
 	char				model_name[I2C_NAME_SIZE];
 	bool				initialized;
@@ -1554,75 +1550,6 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void bq24190_extcon_work(struct work_struct *work)
-{
-	struct bq24190_dev_info *bdi =
-		container_of(work, struct bq24190_dev_info, extcon_work.work);
-	int error, iinlim = 0;
-	u8 v;
-
-	error = pm_runtime_get_sync(bdi->dev);
-	if (error < 0) {
-		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
-		pm_runtime_put_noidle(bdi->dev);
-		return;
-	}
-
-	if      (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_SDP) == 1)
-		iinlim =  500000;
-	else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_CDP) == 1 ||
-		 extcon_get_state(bdi->extcon, EXTCON_CHG_USB_ACA) == 1)
-		iinlim = 1500000;
-	else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_DCP) == 1)
-		iinlim = 2000000;
-
-	if (iinlim) {
-		error = bq24190_set_field_val(bdi, BQ24190_REG_ISC,
-					      BQ24190_REG_ISC_IINLIM_MASK,
-					      BQ24190_REG_ISC_IINLIM_SHIFT,
-					      bq24190_isc_iinlim_values,
-					      ARRAY_SIZE(bq24190_isc_iinlim_values),
-					      iinlim);
-		if (error < 0)
-			dev_err(bdi->dev, "Can't set IINLIM: %d\n", error);
-	}
-
-	/* if no charger found and in USB host mode, set OTG 5V boost, else normal */
-	if (!iinlim && extcon_get_state(bdi->extcon, EXTCON_USB_HOST) == 1)
-		v = BQ24190_REG_POC_CHG_CONFIG_OTG;
-	else
-		v = BQ24190_REG_POC_CHG_CONFIG_CHARGE;
-
-	error = bq24190_write_mask(bdi, BQ24190_REG_POC,
-				   BQ24190_REG_POC_CHG_CONFIG_MASK,
-				   BQ24190_REG_POC_CHG_CONFIG_SHIFT,
-				   v);
-	if (error < 0)
-		dev_err(bdi->dev, "Can't set CHG_CONFIG: %d\n", error);
-
-	pm_runtime_mark_last_busy(bdi->dev);
-	pm_runtime_put_autosuspend(bdi->dev);
-}
-
-static int bq24190_extcon_event(struct notifier_block *nb, unsigned long event,
-				void *param)
-{
-	struct bq24190_dev_info *bdi =
-		container_of(nb, struct bq24190_dev_info, extcon_nb);
-
-	/*
-	 * The Power-Good detection may take up to 220ms, sometimes
-	 * the external charger detection is quicker, and the bq24190 will
-	 * reset to iinlim based on its own charger detection (which is not
-	 * hooked up when using external charger detection) resulting in
-	 * a too low default 500mA iinlim. Delay applying the extcon value
-	 * for 300ms to avoid this.
-	 */
-	queue_delayed_work(system_wq, &bdi->extcon_work, msecs_to_jiffies(300));
-
-	return NOTIFY_OK;
-}
-
 static int bq24190_hw_init(struct bq24190_dev_info *bdi)
 {
 	u8 v;
@@ -1660,7 +1587,6 @@ static int bq24190_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct power_supply_config charger_cfg = {}, battery_cfg = {};
 	struct bq24190_dev_info *bdi;
-	const char *name;
 	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -1690,28 +1616,19 @@ static int bq24190_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
-	bdi->input_current_limit_from_supplier =
-		device_property_read_bool(dev,
-					  "input-current-limit-from-supplier");
-
 	/*
-	 * Devicetree platforms should get extcon via phandle (not yet supported).
-	 * On ACPI platforms, extcon clients may invoke us with:
+	 * This prop. can be passed on device instantiation from platform code:
 	 * struct property_entry pe[] =
-	 *   { PROPERTY_ENTRY_STRING("extcon-name", client_name), ... };
+	 *   { PROPERTY_ENTRY_BOOL("input-current-limit-from-supplier"), ... };
 	 * struct i2c_board_info bi =
 	 *   { .type = "bq24190", .addr = 0x6b, .properties = pe, .irq = irq };
 	 * struct i2c_adapter ad = { ... };
 	 * i2c_add_adapter(&ad);
 	 * i2c_new_device(&ad, &bi);
 	 */
-	if (device_property_read_string(dev, "extcon-name", &name) == 0) {
-		bdi->extcon = extcon_get_extcon_dev(name);
-		if (!bdi->extcon)
-			return -EPROBE_DEFER;
-
-		dev_info(bdi->dev, "using extcon device %s\n", name);
-	}
+	bdi->input_current_limit_from_supplier =
+		device_property_read_bool(dev,
+					  "input-current-limit-from-supplier");
 
 	pm_runtime_enable(dev);
 	pm_runtime_use_autosuspend(dev);
@@ -1773,20 +1690,6 @@ static int bq24190_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto out_sysfs;
 
-	if (bdi->extcon) {
-		INIT_DELAYED_WORK(&bdi->extcon_work, bq24190_extcon_work);
-		bdi->extcon_nb.notifier_call = bq24190_extcon_event;
-		ret = devm_extcon_register_notifier_all(dev, bdi->extcon,
-							&bdi->extcon_nb);
-		if (ret) {
-			dev_err(dev, "Can't register extcon\n");
-			goto out_sysfs;
-		}
-
-		/* Sync initial cable state */
-		queue_delayed_work(system_wq, &bdi->extcon_work, 0);
-	}
-
 	enable_irq_wake(client->irq);
 
 	pm_runtime_mark_last_busy(dev);
-- 
2.9.4

[PATCH v2 13/14] 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
---
 drivers/i2c/busses/Kconfig      |  5 ++++
 drivers/i2c/busses/i2c-cht-wc.c | 52 +++++++++++++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 79c3381..272ef10 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 21312ee..d1f3ec4 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,36 @@ 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("input-current-limit-from-supplier"),
 	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 +271,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 +341,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 +372,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.9.4

[PATCH v2 14/14] 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 ef73860..2e412a3 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 5f1924f..6edfbed 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
@@ -70,12 +71,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-snk-microvolt", 12000000),
+	PROPERTY_ENTRY_U32("fcs,max-snk-microamp",   3000000),
+	PROPERTY_ENTRY_U32("fcs,max-snk-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;
@@ -93,6 +103,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) {
@@ -119,6 +157,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)
@@ -126,7 +165,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);
@@ -134,6 +175,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.9.4