We have a complex set of hardware components to manage our USB C data and
power. For these to work together we decided to use extcon to communicate
the system changes as various cables and devices are plugged in/out. We did
look at usb_roleswitch and the charging framework but thought it would be
preferable to keep all of the information together in one system.
The components we have in the system are:
1) TPS65982 type USB type C controller
2) dwc3 IP in the imx8mq
3) BQ25895 battery charger
I'll break this into 2 parts the data role and the power role.
For the data role the TPS65982 senses connect and disconnect as well as data
source/sink. It is also controlling the USB 3 data lanes. The display port and
USB 3 muxing is handled by a different chip and we'll submit patches for that
later on. The dwc3 controls the USB 2 data lanes.
On the power side there are even more moving pieces. The TPS65982 negotiates
the power delivery contract, the dwc3 senses the BC1.2 charging current and the
BQ25895 sets whether we are sinking or sourcing current and what the current
limit is of the sink and source.
For both the data and power roles no single chip has all of the required
information. Is using extcon the correct way of doing this and if not what
are the alternatives ?
The extcon extensions allow us to communicate the the power roles amongst
the various chips.
This patch series has been tested with the 5.9-rc4 kernel on the Purism
Librem5 HW. Assuming this is the correct way to use extcon there will be
follow on patches to the BQ25895 and dwc3 drivers.
Strictly speaking only the first 3 patches are needed for extcon support, the
forth patch decodes the power delivery contracts which makes use of the extcon
system.
Angus Ainslie (4):
extcon: Add USB VBUS properties
usb: typec: tps6589x: register as an extcon provider
usb: typec: tps6598x: Add the extcon USB chargers
usb: typec: tps6598x: Add the power delivery irq
drivers/usb/typec/tps6598x.c | 488 ++++++++++++++++++++++++++++++++++-
include/linux/extcon.h | 17 +-
2 files changed, 503 insertions(+), 2 deletions(-)
--
2.25.1
Notify extcon devcies that the charge current has changed
Signed-off-by: Angus Ainslie <[email protected]>
---
drivers/usb/typec/tps6598x.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 3b06d43c810d..d5aa58c9da0a 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -240,17 +240,25 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps,
EXTCON_PROP_USB_VBUS_SRC,
val);
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0);
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0);
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0);
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0);
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0);
+
switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
case TYPEC_PWR_MODE_USB:
- max_current = 500;
+ max_current = val.intval = 500;
extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
break;
case TYPEC_PWR_MODE_1_5A:
- max_current = 1500;
+ max_current = val.intval = 1500;
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state);
break;
case TYPEC_PWR_MODE_3_0A:
- max_current = 3000;
+ max_current = val.intval = 3000;
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state);
break;
case TYPEC_PWR_MODE_PD:
max_current = 500;
@@ -265,10 +273,14 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps,
EXTCON_PROP_USB_VBUS_CURRENT,
val);
- extcon_sync(tps->edev, EXTCON_USB);
- extcon_sync(tps->edev, EXTCON_USB_HOST);
extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
+ extcon_sync(tps->edev, EXTCON_CHG_USB_CDP);
+ extcon_sync(tps->edev, EXTCON_CHG_USB_DCP);
extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
+ extcon_sync(tps->edev, EXTCON_CHG_USB_FAST);
+ /* do these last for extcon notifiers */
+ extcon_sync(tps->edev, EXTCON_USB);
+ extcon_sync(tps->edev, EXTCON_USB_HOST);
}
#endif
@@ -562,6 +574,8 @@ static const unsigned int tps6598x_extcon_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
EXTCON_CHG_USB_SDP,
+ EXTCON_CHG_USB_CDP,
+ EXTCON_CHG_USB_DCP,
EXTCON_CHG_USB_SLOW,
EXTCON_CHG_USB_FAST,
EXTCON_NONE,
--
2.25.1
The tps6598x type C chip can negotiate the VBUS sink/source status as
well as the VBUS voltage and current. Notify the extcon consumers of
these changes.
Signed-off-by: Angus Ainslie <[email protected]>
---
drivers/usb/typec/tps6598x.c | 138 +++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 3db33bb622c3..3b06d43c810d 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -8,6 +8,8 @@
#include <linux/i2c.h>
#include <linux/acpi.h>
+#include <linux/extcon.h>
+#include <linux/extcon-provider.h>
#include <linux/module.h>
#include <linux/regmap.h>
#include <linux/interrupt.h>
@@ -95,7 +97,12 @@ struct tps6598x {
struct typec_port *port;
struct typec_partner *partner;
struct usb_pd_identity partner_identity;
+
struct usb_role_switch *role_sw;
+
+#ifdef CONFIG_EXTCON
+ struct extcon_dev *edev;
+#endif
};
/*
@@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
typec_set_data_role(tps->port, role);
}
+#ifdef CONFIG_EXTCON
+static void tps6589x_set_extcon_state(struct tps6598x *tps,
+ u32 status, u16 pwr_status, bool state)
+{
+ union extcon_property_value val;
+ int max_current;
+
+ if (TPS_STATUS_DATAROLE(status)) {
+ extcon_set_state(tps->edev, EXTCON_USB, false);
+ extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
+ } else {
+ extcon_set_state(tps->edev, EXTCON_USB, state);
+ extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
+ }
+
+ val.intval = TPS_STATUS_PORTROLE(status);
+ extcon_set_property(tps->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS_SRC,
+ val);
+
+ extcon_set_property(tps->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS_SRC,
+ val);
+
+ switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
+ case TYPEC_PWR_MODE_USB:
+ max_current = 500;
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
+ extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
+ break;
+ case TYPEC_PWR_MODE_1_5A:
+ max_current = 1500;
+ break;
+ case TYPEC_PWR_MODE_3_0A:
+ max_current = 3000;
+ break;
+ case TYPEC_PWR_MODE_PD:
+ max_current = 500;
+ break;
+ }
+
+ val.intval = max_current;
+ extcon_set_property(tps->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS_CURRENT,
+ val);
+ extcon_set_property(tps->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS_CURRENT,
+ val);
+
+ extcon_sync(tps->edev, EXTCON_USB);
+ extcon_sync(tps->edev, EXTCON_USB_HOST);
+ extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
+ extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
+}
+#endif
+
static int tps6598x_connect(struct tps6598x *tps, u32 status)
{
struct typec_partner_desc desc;
@@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
if (desc.identity)
typec_partner_set_identity(tps->partner);
+#ifdef CONFIG_EXTCON
+ tps6598x_set_extcon_state(tps, status, pwr_status, true);
+#endif
+
return 0;
}
static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
{
+ enum typec_pwr_opmode mode;
+ u16 pwr_status;
+ int ret;
+
if (!IS_ERR(tps->partner))
typec_unregister_partner(tps->partner);
tps->partner = NULL;
typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
+ typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
+
+ ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
+ if (ret < 0)
+ return;
+
+ mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
+
+ dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role %d\n",
+ __func__, mode, TPS_STATUS_PORTROLE(status),
+ TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
+
+#ifdef CONFIG_EXTCON
+ tps6598x_set_extcon_state(tps, status, 0, false);
+#endif
}
static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
@@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
goto err_unlock;
}
+ dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
+ (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF));
+
ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
if (ret) {
dev_err(tps->dev, "%s: failed to read status\n", __func__);
@@ -467,6 +556,18 @@ static const struct regmap_config tps6598x_regmap_config = {
.max_register = 0x7F,
};
+#ifdef CONFIG_EXTCON
+/* List of detectable cables */
+static const unsigned int tps6598x_extcon_cable[] = {
+ EXTCON_USB,
+ EXTCON_USB_HOST,
+ EXTCON_CHG_USB_SDP,
+ EXTCON_CHG_USB_SLOW,
+ EXTCON_CHG_USB_FAST,
+ EXTCON_NONE,
+};
+#endif
+
static int tps6598x_probe(struct i2c_client *client)
{
struct typec_capability typec_cap = { };
@@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client *client)
}
fwnode_handle_put(fwnode);
+#ifdef CONFIG_EXTCON
+ /* Allocate extcon device */
+ tps->edev = devm_extcon_dev_allocate(tps->dev, tps6598x_extcon_cable);
+ if (IS_ERR(tps->edev)) {
+ dev_err(tps->dev, "failed to allocate memory for extcon\n");
+ typec_unregister_port(tps->port);
+ return -ENOMEM;
+ }
+
+ /* Register extcon device */
+ ret = devm_extcon_dev_register(tps->dev, tps->edev);
+ if (ret) {
+ dev_err(tps->dev, "failed to register extcon device\n");
+ typec_unregister_port(tps->port);
+ return ret;
+ }
+
+ extcon_set_property_capability(tps->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS);
+ extcon_set_property_capability(tps->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS_SRC);
+ extcon_set_property_capability(tps->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS_VOLTAGE);
+ extcon_set_property_capability(tps->edev, EXTCON_USB,
+ EXTCON_PROP_USB_VBUS_CURRENT);
+ extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS);
+ extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS_SRC);
+ extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS_VOLTAGE);
+ extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+ EXTCON_PROP_USB_VBUS_CURRENT);
+#endif
+
if (status & TPS_STATUS_PLUG_PRESENT) {
ret = tps6598x_connect(tps, status);
if (ret)
dev_err(&client->dev, "failed to register partner\n");
+ } else {
+ tps6598x_disconnect(tps, status);
}
ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
--
2.25.1
Hi,
On 9/15/20 1:46 AM, Angus Ainslie wrote:
> The tps6598x type C chip can negotiate the VBUS sink/source status as
> well as the VBUS voltage and current. Notify the extcon consumers of
> these changes.
>
> Signed-off-by: Angus Ainslie <[email protected]>
> ---
> drivers/usb/typec/tps6598x.c | 138 +++++++++++++++++++++++++++++++++++
> 1 file changed, 138 insertions(+)
>
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index 3db33bb622c3..3b06d43c810d 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -8,6 +8,8 @@
>
> #include <linux/i2c.h>
> #include <linux/acpi.h>
> +#include <linux/extcon.h>
> +#include <linux/extcon-provider.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
> #include <linux/interrupt.h>
> @@ -95,7 +97,12 @@ struct tps6598x {
> struct typec_port *port;
> struct typec_partner *partner;
> struct usb_pd_identity partner_identity;
> +
> struct usb_role_switch *role_sw;
> +
> +#ifdef CONFIG_EXTCON
Is it necessary of 'ifdef CONFIG_EXTCON'?
If you just add 'select EXTCON' for this driver,
you don't need to check CONFIG_EXTCON.
If any device driver need some framework,
we can add the 'select EXTCON'.
> + struct extcon_dev *edev;
> +#endif
> };
>
> /*
> @@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
> typec_set_data_role(tps->port, role);
> }
>
> +#ifdef CONFIG_EXTCON
> +static void tps6589x_set_extcon_state(struct tps6598x *tps,
> + u32 status, u16 pwr_status, bool state)
> +{
> + union extcon_property_value val;
> + int max_current;
> +
> + if (TPS_STATUS_DATAROLE(status)) {
> + extcon_set_state(tps->edev, EXTCON_USB, false);
> + extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
> + } else {
> + extcon_set_state(tps->edev, EXTCON_USB, state);
> + extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
> + }
> +
> + val.intval = TPS_STATUS_PORTROLE(status);
> + extcon_set_property(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_SRC,
> + val);
> +
> + extcon_set_property(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_SRC,
> + val);
> +
> + switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
> + case TYPEC_PWR_MODE_USB:
> + max_current = 500;
> + extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
> + extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
> + break;
> + case TYPEC_PWR_MODE_1_5A:
> + max_current = 1500;
> + break;
> + case TYPEC_PWR_MODE_3_0A:
> + max_current = 3000;
> + break;
> + case TYPEC_PWR_MODE_PD:
> + max_current = 500;
> + break;
> + }
> +
> + val.intval = max_current;
> + extcon_set_property(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_CURRENT,
> + val);
> + extcon_set_property(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_CURRENT,
> + val);
> +
> + extcon_sync(tps->edev, EXTCON_USB);
> + extcon_sync(tps->edev, EXTCON_USB_HOST);
> + extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
> + extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
> +}
> +#endif
> +
> static int tps6598x_connect(struct tps6598x *tps, u32 status)
> {
> struct typec_partner_desc desc;
> @@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
> if (desc.identity)
> typec_partner_set_identity(tps->partner);
>
> +#ifdef CONFIG_EXTCON
> + tps6598x_set_extcon_state(tps, status, pwr_status, true);
> +#endif
> +
> return 0;
> }
>
> static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
> {
> + enum typec_pwr_opmode mode;
> + u16 pwr_status;
> + int ret;
> +
> if (!IS_ERR(tps->partner))
> typec_unregister_partner(tps->partner);
> tps->partner = NULL;
> typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
> typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
> typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
> + typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
> tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
> +
> + ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
> + if (ret < 0)
> + return;
> +
> + mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
> +
> + dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role %d\n",
> + __func__, mode, TPS_STATUS_PORTROLE(status),
> + TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
> +
> +#ifdef CONFIG_EXTCON
> + tps6598x_set_extcon_state(tps, status, 0, false);
> +#endif
> }
>
> static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
> @@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> goto err_unlock;
> }
>
> + dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
> + (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF));
> +
> ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
> if (ret) {
> dev_err(tps->dev, "%s: failed to read status\n", __func__);
> @@ -467,6 +556,18 @@ static const struct regmap_config tps6598x_regmap_config = {
> .max_register = 0x7F,
> };
>
> +#ifdef CONFIG_EXTCON
> +/* List of detectable cables */
> +static const unsigned int tps6598x_extcon_cable[] = {
> + EXTCON_USB,
> + EXTCON_USB_HOST,
> + EXTCON_CHG_USB_SDP,
> + EXTCON_CHG_USB_SLOW,
> + EXTCON_CHG_USB_FAST,
> + EXTCON_NONE,
> +};
> +#endif
> +
> static int tps6598x_probe(struct i2c_client *client)
> {
> struct typec_capability typec_cap = { };
> @@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client *client)
> }
> fwnode_handle_put(fwnode);
>
> +#ifdef CONFIG_EXTCON
> + /* Allocate extcon device */
> + tps->edev = devm_extcon_dev_allocate(tps->dev, tps6598x_extcon_cable);
> + if (IS_ERR(tps->edev)) {
> + dev_err(tps->dev, "failed to allocate memory for extcon\n");
> + typec_unregister_port(tps->port);
> + return -ENOMEM;
> + }
> +
> + /* Register extcon device */
> + ret = devm_extcon_dev_register(tps->dev, tps->edev);
> + if (ret) {
> + dev_err(tps->dev, "failed to register extcon device\n");
> + typec_unregister_port(tps->port);
> + return ret;
> + }
> +
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS);
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_SRC);
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_VOLTAGE);
> + extcon_set_property_capability(tps->edev, EXTCON_USB,
> + EXTCON_PROP_USB_VBUS_CURRENT);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_SRC);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_VOLTAGE);
> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> + EXTCON_PROP_USB_VBUS_CURRENT);
> +#endif
> +
> if (status & TPS_STATUS_PLUG_PRESENT) {
> ret = tps6598x_connect(tps, status);
> if (ret)
> dev_err(&client->dev, "failed to register partner\n");
> + } else {
> + tps6598x_disconnect(tps, status);
> }
>
> ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Hi,
On 2020-09-14 18:19, Chanwoo Choi wrote:
> Hi,
>
> On 9/15/20 1:46 AM, Angus Ainslie wrote:
>> The tps6598x type C chip can negotiate the VBUS sink/source status as
>> well as the VBUS voltage and current. Notify the extcon consumers of
>> these changes.
>>
>> Signed-off-by: Angus Ainslie <[email protected]>
>> ---
>> drivers/usb/typec/tps6598x.c | 138
>> +++++++++++++++++++++++++++++++++++
>> 1 file changed, 138 insertions(+)
>>
>> diff --git a/drivers/usb/typec/tps6598x.c
>> b/drivers/usb/typec/tps6598x.c
>> index 3db33bb622c3..3b06d43c810d 100644
>> --- a/drivers/usb/typec/tps6598x.c
>> +++ b/drivers/usb/typec/tps6598x.c
>> @@ -8,6 +8,8 @@
>>
>> #include <linux/i2c.h>
>> #include <linux/acpi.h>
>> +#include <linux/extcon.h>
>> +#include <linux/extcon-provider.h>
>> #include <linux/module.h>
>> #include <linux/regmap.h>
>> #include <linux/interrupt.h>
>> @@ -95,7 +97,12 @@ struct tps6598x {
>> struct typec_port *port;
>> struct typec_partner *partner;
>> struct usb_pd_identity partner_identity;
>> +
>> struct usb_role_switch *role_sw;
>> +
>> +#ifdef CONFIG_EXTCON
>
> Is it necessary of 'ifdef CONFIG_EXTCON'?
> If you just add 'select EXTCON' for this driver,
> you don't need to check CONFIG_EXTCON.
>
> If any device driver need some framework,
> we can add the 'select EXTCON'.
>
Sure I can change that for v2
Angus
>> + struct extcon_dev *edev;
>> +#endif
>> };
>>
>> /*
>> @@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct
>> tps6598x *tps,
>> typec_set_data_role(tps->port, role);
>> }
>>
>> +#ifdef CONFIG_EXTCON
>> +static void tps6589x_set_extcon_state(struct tps6598x *tps,
>> + u32 status, u16 pwr_status, bool state)
>> +{
>> + union extcon_property_value val;
>> + int max_current;
>> +
>> + if (TPS_STATUS_DATAROLE(status)) {
>> + extcon_set_state(tps->edev, EXTCON_USB, false);
>> + extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
>> + } else {
>> + extcon_set_state(tps->edev, EXTCON_USB, state);
>> + extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
>> + }
>> +
>> + val.intval = TPS_STATUS_PORTROLE(status);
>> + extcon_set_property(tps->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS_SRC,
>> + val);
>> +
>> + extcon_set_property(tps->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS_SRC,
>> + val);
>> +
>> + switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
>> + case TYPEC_PWR_MODE_USB:
>> + max_current = 500;
>> + extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
>> + extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
>> + break;
>> + case TYPEC_PWR_MODE_1_5A:
>> + max_current = 1500;
>> + break;
>> + case TYPEC_PWR_MODE_3_0A:
>> + max_current = 3000;
>> + break;
>> + case TYPEC_PWR_MODE_PD:
>> + max_current = 500;
>> + break;
>> + }
>> +
>> + val.intval = max_current;
>> + extcon_set_property(tps->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS_CURRENT,
>> + val);
>> + extcon_set_property(tps->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS_CURRENT,
>> + val);
>> +
>> + extcon_sync(tps->edev, EXTCON_USB);
>> + extcon_sync(tps->edev, EXTCON_USB_HOST);
>> + extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
>> + extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
>> +}
>> +#endif
>> +
>> static int tps6598x_connect(struct tps6598x *tps, u32 status)
>> {
>> struct typec_partner_desc desc;
>> @@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x
>> *tps, u32 status)
>> if (desc.identity)
>> typec_partner_set_identity(tps->partner);
>>
>> +#ifdef CONFIG_EXTCON
>> + tps6598x_set_extcon_state(tps, status, pwr_status, true);
>> +#endif
>> +
>> return 0;
>> }
>>
>> static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
>> {
>> + enum typec_pwr_opmode mode;
>> + u16 pwr_status;
>> + int ret;
>> +
>> if (!IS_ERR(tps->partner))
>> typec_unregister_partner(tps->partner);
>> tps->partner = NULL;
>> typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
>> typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
>> typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
>> + typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
>> tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
>> +
>> + ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
>> + if (ret < 0)
>> + return;
>> +
>> + mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
>> +
>> + dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role
>> %d\n",
>> + __func__, mode, TPS_STATUS_PORTROLE(status),
>> + TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
>> +
>> +#ifdef CONFIG_EXTCON
>> + tps6598x_set_extcon_state(tps, status, 0, false);
>> +#endif
>> }
>>
>> static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
>> @@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq,
>> void *data)
>> goto err_unlock;
>> }
>>
>> + dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
>> + (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 &
>> 0xFFFFFFFF));
>> +
>> ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
>> if (ret) {
>> dev_err(tps->dev, "%s: failed to read status\n", __func__);
>> @@ -467,6 +556,18 @@ static const struct regmap_config
>> tps6598x_regmap_config = {
>> .max_register = 0x7F,
>> };
>>
>> +#ifdef CONFIG_EXTCON
>> +/* List of detectable cables */
>> +static const unsigned int tps6598x_extcon_cable[] = {
>> + EXTCON_USB,
>> + EXTCON_USB_HOST,
>> + EXTCON_CHG_USB_SDP,
>> + EXTCON_CHG_USB_SLOW,
>> + EXTCON_CHG_USB_FAST,
>> + EXTCON_NONE,
>> +};
>> +#endif
>> +
>> static int tps6598x_probe(struct i2c_client *client)
>> {
>> struct typec_capability typec_cap = { };
>> @@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client
>> *client)
>> }
>> fwnode_handle_put(fwnode);
>>
>> +#ifdef CONFIG_EXTCON
>> + /* Allocate extcon device */
>> + tps->edev = devm_extcon_dev_allocate(tps->dev,
>> tps6598x_extcon_cable);
>> + if (IS_ERR(tps->edev)) {
>> + dev_err(tps->dev, "failed to allocate memory for extcon\n");
>> + typec_unregister_port(tps->port);
>> + return -ENOMEM;
>> + }
>> +
>> + /* Register extcon device */
>> + ret = devm_extcon_dev_register(tps->dev, tps->edev);
>> + if (ret) {
>> + dev_err(tps->dev, "failed to register extcon device\n");
>> + typec_unregister_port(tps->port);
>> + return ret;
>> + }
>> +
>> + extcon_set_property_capability(tps->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS_SRC);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS_VOLTAGE);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB,
>> + EXTCON_PROP_USB_VBUS_CURRENT);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS_SRC);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS_VOLTAGE);
>> + extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> + EXTCON_PROP_USB_VBUS_CURRENT);
>> +#endif
>> +
>> if (status & TPS_STATUS_PLUG_PRESENT) {
>> ret = tps6598x_connect(tps, status);
>> if (ret)
>> dev_err(&client->dev, "failed to register partner\n");
>> + } else {
>> + tps6598x_disconnect(tps, status);
>> }
>>
>> ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>>
On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote:
> We have a complex set of hardware components to manage our USB C data and
> power. For these to work together we decided to use extcon to communicate
> the system changes as various cables and devices are plugged in/out. We did
> look at usb_roleswitch and the charging framework but thought it would be
> preferable to keep all of the information together in one system.
>
> The components we have in the system are:
>
> 1) TPS65982 type USB type C controller
> 2) dwc3 IP in the imx8mq
> 3) BQ25895 battery charger
>
> I'll break this into 2 parts the data role and the power role.
>
> For the data role the TPS65982 senses connect and disconnect as well as data
> source/sink. It is also controlling the USB 3 data lanes. The display port and
> USB 3 muxing is handled by a different chip and we'll submit patches for that
> later on. The dwc3 controls the USB 2 data lanes.
>
> On the power side there are even more moving pieces. The TPS65982 negotiates
> the power delivery contract, the dwc3 senses the BC1.2 charging current and the
> BQ25895 sets whether we are sinking or sourcing current and what the current
> limit is of the sink and source.
>
> For both the data and power roles no single chip has all of the required
> information. Is using extcon the correct way of doing this and if not what
> are the alternatives ?
Do not use extcon with the Type-C drivers unless you have some really
good reason for not using the dedicated frameworks for each thing. The
reason why we even have some of the dedicated frameworks in the first
place, for example the USB role switch class, is because extcon simply
could not be made to work on every type of hardware architecture.
So you will need to register a power supply in tps6598x.c just like
the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982
does not communicated directly with the BQ25895. That can be one
of "supplied_from" (and also "supplied_to" if needed for sourcing) for
the bq25890_changer. You probable only need to implement the
external_power_changed() hook for it if it's missing in order to make
it work. You can also register a power supply in dwc3 and use it as a
second supply for bq25890 if you still really need to handle BC1.2.
The data role should already be handled for you. dwc3 already
registers an USB role switch, and tps6598x.c already configures one.
For data role you should not need any additional code.
Please note that there is also framework for the alt mode muxes.
> The extcon extensions allow us to communicate the the power roles amongst
> the various chips.
>
> This patch series has been tested with the 5.9-rc4 kernel on the Purism
> Librem5 HW. Assuming this is the correct way to use extcon there will be
> follow on patches to the BQ25895 and dwc3 drivers.
>
> Strictly speaking only the first 3 patches are needed for extcon support, the
> forth patch decodes the power delivery contracts which makes use of the extcon
> system.
>
>
> Angus Ainslie (4):
> extcon: Add USB VBUS properties
> usb: typec: tps6589x: register as an extcon provider
> usb: typec: tps6598x: Add the extcon USB chargers
> usb: typec: tps6598x: Add the power delivery irq
>
> drivers/usb/typec/tps6598x.c | 488 ++++++++++++++++++++++++++++++++++-
> include/linux/extcon.h | 17 +-
> 2 files changed, 503 insertions(+), 2 deletions(-)
>
> --
> 2.25.1
thanks,
--
heikki
Hi Heikki,
On 2020-09-21 07:37, Heikki Krogerus wrote:
> On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote:
>> We have a complex set of hardware components to manage our USB C data
>> and
>> power. For these to work together we decided to use extcon to
>> communicate
>> the system changes as various cables and devices are plugged in/out.
>> We did
>> look at usb_roleswitch and the charging framework but thought it would
>> be
>> preferable to keep all of the information together in one system.
>>
>> The components we have in the system are:
>>
>> 1) TPS65982 type USB type C controller
>> 2) dwc3 IP in the imx8mq
>> 3) BQ25895 battery charger
>>
>> I'll break this into 2 parts the data role and the power role.
>>
>> For the data role the TPS65982 senses connect and disconnect as well
>> as data
>> source/sink. It is also controlling the USB 3 data lanes. The display
>> port and
>> USB 3 muxing is handled by a different chip and we'll submit patches
>> for that
>> later on. The dwc3 controls the USB 2 data lanes.
>>
>> On the power side there are even more moving pieces. The TPS65982
>> negotiates
>> the power delivery contract, the dwc3 senses the BC1.2 charging
>> current and the
>> BQ25895 sets whether we are sinking or sourcing current and what the
>> current
>> limit is of the sink and source.
>>
>> For both the data and power roles no single chip has all of the
>> required
>> information. Is using extcon the correct way of doing this and if not
>> what
>> are the alternatives ?
>
> Do not use extcon with the Type-C drivers unless you have some really
> good reason for not using the dedicated frameworks for each thing. The
> reason why we even have some of the dedicated frameworks in the first
> place, for example the USB role switch class, is because extcon simply
> could not be made to work on every type of hardware architecture.
>
> So you will need to register a power supply in tps6598x.c just like
> the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982
> does not communicated directly with the BQ25895. That can be one
> of "supplied_from" (and also "supplied_to" if needed for sourcing) for
> the bq25890_changer. You probable only need to implement the
> external_power_changed() hook for it if it's missing in order to make
> it work. You can also register a power supply in dwc3 and use it as a
> second supply for bq25890 if you still really need to handle BC1.2.
>
> The data role should already be handled for you. dwc3 already
> registers an USB role switch, and tps6598x.c already configures one.
> For data role you should not need any additional code.
>
> Please note that there is also framework for the alt mode muxes.
>
Thanks for looking this over. I'll investigate the power supply
framework.
Angus
>
>> The extcon extensions allow us to communicate the the power roles
>> amongst
>> the various chips.
>>
>> This patch series has been tested with the 5.9-rc4 kernel on the
>> Purism
>> Librem5 HW. Assuming this is the correct way to use extcon there will
>> be
>> follow on patches to the BQ25895 and dwc3 drivers.
>>
>> Strictly speaking only the first 3 patches are needed for extcon
>> support, the
>> forth patch decodes the power delivery contracts which makes use of
>> the extcon
>> system.
>>
>>
>> Angus Ainslie (4):
>> extcon: Add USB VBUS properties
>> usb: typec: tps6589x: register as an extcon provider
>> usb: typec: tps6598x: Add the extcon USB chargers
>> usb: typec: tps6598x: Add the power delivery irq
>>
>> drivers/usb/typec/tps6598x.c | 488
>> ++++++++++++++++++++++++++++++++++-
>> include/linux/extcon.h | 17 +-
>> 2 files changed, 503 insertions(+), 2 deletions(-)
>>
>> --
>> 2.25.1
>
> thanks,