This is a series developed for the Librem 5 phone, which uses TPS65982
as its USB-C controller. Implemented are Power Delivery sink contract
tracing and exporting negotiated power values as power supply properties,
fixes for data role swapping, status register caching and a debugfs entry
for querying customer use word of the firmware running on the controller.
Angus Ainslie (3):
usb: typec: tipd: set the data role on tps IRQ
usb: typec: tipd: Add trace event for SINK PD contract
usb: typec: tipd: Provide POWER_SUPPLY_PROP_{CURRENT,VOLTAGE}_MAX
Guido Günther (2):
usb: typec: tipd: Only update power status on IRQ
usb: typec: tipd: Add debugfs entries for customer use word
Sebastian Krzyszkowiak (2):
usb: typec: tipd: Provide POWER_SUPPLY_PROP_PRESENT
usb: typec: tipd: Fail probe when the controller is in BOOT mode
drivers/usb/typec/tipd/core.c | 263 ++++++++++++++++++++++++++----
drivers/usb/typec/tipd/tps6598x.h | 30 ++++
drivers/usb/typec/tipd/trace.h | 38 +++++
3 files changed, 302 insertions(+), 29 deletions(-)
--
2.35.1
From: Angus Ainslie <[email protected]>
This helps downstream supplies to adjust their input limits.
Signed-off-by: Angus Ainslie <[email protected]>
Signed-off-by: Guido Günther <[email protected]>
Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
---
drivers/usb/typec/tipd/core.c | 76 ++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 80b4a9870caf..f3e8f1183f5b 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -39,6 +39,11 @@
#define TPS_REG_RX_IDENTITY_SOP 0x48
#define TPS_REG_DATA_STATUS 0x5f
+#define TPS_USB_500mA 500000
+#define TPS_TYPEC_1500mA 1500000
+#define TPS_TYPEC_3000mA 3000000
+#define TPS_USB_5V 5000000
+
/* TPS_REG_SYSTEM_CONF bits */
#define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
@@ -103,6 +108,8 @@ struct tps6598x {
static enum power_supply_property tps6598x_psy_props[] = {
POWER_SUPPLY_PROP_USB_TYPE,
POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX,
};
static enum power_supply_usb_type tps6598x_psy_usb_types[] = {
@@ -294,6 +301,8 @@ static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
typec_set_orientation(tps->port, TYPEC_ORIENTATION_NONE);
tps6598x_set_data_role(tps, TPS_STATUS_TO_TYPEC_DATAROLE(status), false);
+ memset(&tps->terms, 0, sizeof(struct tps6598x_pdo));
+
power_supply_changed(tps->psy);
}
@@ -577,6 +586,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
u64 event1;
u64 event2;
u32 status;
+ bool psy_changed = false;
int ret;
mutex_lock(&tps->lock);
@@ -595,10 +605,13 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
if (!tps6598x_read_status(tps, &status))
goto err_clear_ints;
- if ((event1 | event2) & TPS_REG_INT_POWER_STATUS_UPDATE)
+ if ((event1 | event2) & TPS_REG_INT_POWER_STATUS_UPDATE) {
if (!tps6598x_read_power_status(tps))
goto err_clear_ints;
+ psy_changed = true;
+ }
+
if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE) {
if (!tps6598x_read_data_status(tps))
goto err_clear_ints;
@@ -612,12 +625,18 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
dev_err(tps->dev, "failed to read pd contract: %d\n", ret);
goto err_clear_ints;
}
+ psy_changed = true;
}
/* Handle plug insert or removal */
if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
tps6598x_handle_plug_event(tps, status);
+ if ((event1 | event2) & TPS_REG_INT_HARD_RESET) {
+ memset(&tps->terms, 0, sizeof(struct tps6598x_pdo));
+ psy_changed = true;
+ }
+
err_clear_ints:
tps6598x_write64(tps, TPS_REG_INT_CLEAR1, event1);
tps6598x_write64(tps, TPS_REG_INT_CLEAR2, event2);
@@ -625,6 +644,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
err_unlock:
mutex_unlock(&tps->lock);
+ if (psy_changed)
+ power_supply_changed(tps->psy);
+
if (event1 | event2)
return IRQ_HANDLED;
return IRQ_NONE;
@@ -671,6 +693,49 @@ static int tps6598x_psy_get_online(struct tps6598x *tps,
} else {
val->intval = 0;
}
+
+ return 0;
+}
+
+static int tps6598x_psy_get_max_current(struct tps6598x *tps,
+ union power_supply_propval *val)
+{
+ enum typec_pwr_opmode mode;
+
+ mode = TPS_POWER_STATUS_PWROPMODE(tps->pwr_status);
+ switch (mode) {
+ case TYPEC_PWR_MODE_1_5A:
+ val->intval = TPS_TYPEC_1500mA;
+ break;
+ case TYPEC_PWR_MODE_3_0A:
+ val->intval = TPS_TYPEC_3000mA;
+ break;
+ case TYPEC_PWR_MODE_PD:
+ val->intval = tps->terms.max_current ?: TPS_USB_500mA;
+ break;
+ default:
+ case TYPEC_PWR_MODE_USB:
+ val->intval = TPS_USB_500mA;
+ }
+ return 0;
+}
+
+static int tps6598x_psy_get_max_voltage(struct tps6598x *tps,
+ union power_supply_propval *val)
+{
+ enum typec_pwr_opmode mode;
+
+ mode = TPS_POWER_STATUS_PWROPMODE(tps->pwr_status);
+ switch (mode) {
+ case TYPEC_PWR_MODE_PD:
+ val->intval = tps->terms.max_voltage ?: TPS_USB_5V;
+ break;
+ default:
+ case TYPEC_PWR_MODE_1_5A:
+ case TYPEC_PWR_MODE_3_0A:
+ case TYPEC_PWR_MODE_USB:
+ val->intval = TPS_USB_5V;
+ }
return 0;
}
@@ -691,6 +756,12 @@ static int tps6598x_psy_get_prop(struct power_supply *psy,
case POWER_SUPPLY_PROP_ONLINE:
ret = tps6598x_psy_get_online(tps, val);
break;
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ ret = tps6598x_psy_get_max_current(tps, val);
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+ ret = tps6598x_psy_get_max_voltage(tps, val);
+ break;
default:
ret = -EINVAL;
break;
@@ -806,7 +877,8 @@ static int tps6598x_probe(struct i2c_client *client)
mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
TPS_REG_INT_DATA_STATUS_UPDATE |
TPS_REG_INT_PLUG_EVENT |
- TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER;
+ TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER |
+ TPS_REG_INT_HARD_RESET;
}
/* Make sure the controller has application firmware running */
--
2.35.1
From: Angus Ainslie <[email protected]>
Don't immediately set the data role, only set it in response to the
negotiated value notification from the tps6589x. Otherwise data role
switch fails for DRP.
We only use values cached from the IRQ instead of poking I2C all
the time.
The update is moved in a function that will become more useful in later
commits.
Fixes: 18a6c866bb19 ("usb: typec: tps6598x: Add USB role switching logic")
Signed-off-by: Angus Ainslie <[email protected]>
Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
---
drivers/usb/typec/tipd/core.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index dfbba5ae9487..f387786ff95e 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -94,6 +94,7 @@ struct tps6598x {
struct power_supply_desc psy_desc;
enum power_supply_usb_type usb_type;
+ u32 data_status;
u16 pwr_status;
};
@@ -271,6 +272,15 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
return 0;
}
+static int
+tps6598x_update_data_status(struct tps6598x *tps, u32 status)
+{
+ tps6598x_set_data_role(tps, TPS_STATUS_TO_TYPEC_DATAROLE(status),
+ !!(tps->data_status & TPS_DATA_STATUS_DATA_CONNECTION));
+ trace_tps6598x_data_status(tps->data_status);
+ return 0;
+}
+
static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
{
if (!IS_ERR(tps->partner))
@@ -370,8 +380,6 @@ static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
goto out_unlock;
}
- tps6598x_set_data_role(tps, role, true);
-
out_unlock:
mutex_unlock(&tps->lock);
@@ -437,6 +445,7 @@ static bool tps6598x_read_data_status(struct tps6598x *tps)
dev_err(tps->dev, "failed to read data status: %d\n", ret);
return false;
}
+ tps->data_status = data_status;
trace_tps6598x_data_status(data_status);
return true;
@@ -497,10 +506,13 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
if (!tps6598x_read_power_status(tps))
goto err_clear_ints;
- if (event & APPLE_CD_REG_INT_DATA_STATUS_UPDATE)
+ if (event & APPLE_CD_REG_INT_DATA_STATUS_UPDATE) {
if (!tps6598x_read_data_status(tps))
goto err_clear_ints;
+ tps6598x_update_data_status(tps, status);
+ }
+
/* Handle plug insert or removal */
if (event & APPLE_CD_REG_INT_PLUG_EVENT)
tps6598x_handle_plug_event(tps, status);
@@ -544,10 +556,13 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
if (!tps6598x_read_power_status(tps))
goto err_clear_ints;
- if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE)
+ if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE) {
if (!tps6598x_read_data_status(tps))
goto err_clear_ints;
+ tps6598x_update_data_status(tps, status);
+ }
+
/* Handle plug insert or removal */
if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
tps6598x_handle_plug_event(tps, status);
--
2.35.1
From: Angus Ainslie <[email protected]>
This allows to trace the negotiated contract as sink. It's
only updated when the contract is actually established.
Co-developed-by: Guido Günther <[email protected]>
Signed-off-by: Guido Günther <[email protected]>
Signed-off-by: Angus Ainslie <[email protected]>
Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
---
drivers/usb/typec/tipd/core.c | 63 ++++++++++++++++++++++++++++++-
drivers/usb/typec/tipd/tps6598x.h | 30 +++++++++++++++
drivers/usb/typec/tipd/trace.h | 38 +++++++++++++++++++
3 files changed, 129 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index f387786ff95e..80b4a9870caf 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -34,6 +34,7 @@
#define TPS_REG_STATUS 0x1a
#define TPS_REG_SYSTEM_CONF 0x28
#define TPS_REG_CTRL_CONF 0x29
+#define TPS_REG_ACTIVE_CONTRACT 0x34
#define TPS_REG_POWER_STATUS 0x3f
#define TPS_REG_RX_IDENTITY_SOP 0x48
#define TPS_REG_DATA_STATUS 0x5f
@@ -93,6 +94,7 @@ struct tps6598x {
struct power_supply *psy;
struct power_supply_desc psy_desc;
enum power_supply_usb_type usb_type;
+ struct tps6598x_pdo terms;
u32 data_status;
u16 pwr_status;
@@ -528,6 +530,47 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
return IRQ_NONE;
}
+static int tps6598x_get_active_pd_contract(struct tps6598x *tps)
+{
+ u64 contract;
+ int type;
+ int max_power;
+ int ret = 0;
+
+ ret = tps6598x_block_read(tps, TPS_REG_ACTIVE_CONTRACT, &contract, 6);
+ if (ret)
+ return ret;
+
+ contract &= 0xFFFFFFFFFFFF;
+ type = TPS_PDO_CONTRACT_TYPE(contract);
+ memset(&tps->terms, 0, sizeof(struct tps6598x_pdo));
+
+ /* If there's no PD it decodes to all 0 */
+ switch (type) {
+ case TPS_PDO_CONTRACT_FIXED:
+ tps->terms.max_voltage = TPS_PDO_FIXED_CONTRACT_VOLTAGE(contract);
+ tps->terms.max_current = TPS_PDO_FIXED_CONTRACT_MAX_CURRENT(contract);
+ break;
+ case TPS_PDO_CONTRACT_BATTERY:
+ tps->terms.max_voltage = TPS_PDO_BAT_CONTRACT_MAX_VOLTAGE(contract);
+ max_power = TPS_PDO_BAT_CONTRACT_MAX_POWER(contract);
+ tps->terms.max_current = 1000 * 1000 * max_power / tps->terms.max_voltage;
+ break;
+ case TPS_PDO_CONTRACT_VARIABLE:
+ tps->terms.max_voltage = TPS_PDO_VAR_CONTRACT_MAX_VOLTAGE(contract);
+ tps->terms.max_current = TPS_PDO_VAR_CONTRACT_MAX_CURRENT(contract);
+ break;
+ default:
+ dev_warn(tps->dev, "Unknown contract type: %d\n", type);
+ return -EINVAL;
+ }
+
+ tps->terms.pdo = contract;
+ trace_tps6598x_pdo(&tps->terms);
+
+ return 0;
+}
+
static irqreturn_t tps6598x_interrupt(int irq, void *data)
{
struct tps6598x *tps = data;
@@ -563,6 +606,14 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
tps6598x_update_data_status(tps, status);
}
+ if ((event1 | event2) & TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER) {
+ ret = tps6598x_get_active_pd_contract(tps);
+ if (ret) {
+ dev_err(tps->dev, "failed to read pd contract: %d\n", ret);
+ goto err_clear_ints;
+ }
+ }
+
/* Handle plug insert or removal */
if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
tps6598x_handle_plug_event(tps, status);
@@ -751,10 +802,11 @@ static int tps6598x_probe(struct i2c_client *client)
irq_handler = cd321x_interrupt;
} else {
- /* Enable power status, data status and plug event interrupts */
+ /* Enable interrupts used by this driver */
mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
TPS_REG_INT_DATA_STATUS_UPDATE |
- TPS_REG_INT_PLUG_EVENT;
+ TPS_REG_INT_PLUG_EVENT |
+ TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER;
}
/* Make sure the controller has application firmware running */
@@ -847,6 +899,13 @@ static int tps6598x_probe(struct i2c_client *client)
ret = tps6598x_connect(tps, status);
if (ret)
dev_err(&client->dev, "failed to register partner\n");
+
+ if ((TPS_POWER_STATUS_PWROPMODE(tps->pwr_status) == TYPEC_PWR_MODE_PD) &&
+ (!(status & TPS_STATUS_PORTROLE))) {
+ ret = tps6598x_get_active_pd_contract(tps);
+ if (ret)
+ dev_err(tps->dev, "failed to read pd contract: %d\n", ret);
+ }
}
ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
index 527857549d69..546cd4881f1f 100644
--- a/drivers/usb/typec/tipd/tps6598x.h
+++ b/drivers/usb/typec/tipd/tps6598x.h
@@ -199,4 +199,34 @@
#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_A BIT(2)
#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
+
+/* PDO decoding as in https://www.ti.com/lit/an/slva842/slva842.pdf */
+#define TPS_PDO_CONTRACT_TYPE(x) FIELD_GET(GENMASK(31, 30), x)
+#define TPS_PDO_CONTRACT_FIXED 0
+#define TPS_PDO_CONTRACT_BATTERY 1
+#define TPS_PDO_CONTRACT_VARIABLE 2
+
+#define TPS_PDO_FIXED_CONTRACT_DETAILS GENMASK(29, 25)
+#define TPS_PDO_FIXED_CONTRACT_DR_POWER BIT(29)
+#define TPS_PDO_FIXED_CONTRACT_USB_SUSPEND BIT(28)
+#define TPS_PDO_FIXED_CONTRACT_EXTERNAL_PWR BIT(27)
+#define TPS_PDO_FIXED_CONTRACT_USB_COMMS BIT(26)
+#define TPS_PDO_FIXED_CONTRACT_DR_DATA BIT(25)
+
+#define TPS_PDO_FIXED_CONTRACT_VOLTAGE(x) (FIELD_GET(GENMASK(19, 10), x) * 50000)
+#define TPS_PDO_FIXED_CONTRACT_MAX_CURRENT(x) (FIELD_GET(GENMASK(9, 0), x) * 10000)
+#define TPS_PDO_VAR_CONTRACT_MAX_VOLTAGE(x) (FIELD_GET(GENMASK(29, 20), x) * 50000)
+#define TPS_PDO_VAR_CONTRACT_MIN_VOLTAGE(x) (FIELD_GET(GENMASK(19, 10), x) * 50000)
+#define TPS_PDO_VAR_CONTRACT_MAX_CURRENT(x) (FIELD_GET(GENMASK(9, 0), x) * 10000)
+#define TPS_PDO_BAT_CONTRACT_MAX_VOLTAGE(x) (FIELD_GET(GENMASK(29, 20), x) * 50000)
+#define TPS_PDO_BAT_CONTRACT_MIN_VOLTAGE(x) (FIELD_GET(GENMASK(19, 10), x) * 50000)
+#define TPS_PDO_BAT_CONTRACT_MAX_POWER(x) (FIELD_GET(GENMASK(9, 0), x) * 250000)
+
+struct tps6598x_pdo {
+ u32 pdo;
+ int max_voltage; /* uV */
+ int max_current; /* uA */
+ int max_power; /* uW */
+};
+
#endif /* __TPS6598X_H__ */
diff --git a/drivers/usb/typec/tipd/trace.h b/drivers/usb/typec/tipd/trace.h
index 12cad1bde7cc..148e2ef3157b 100644
--- a/drivers/usb/typec/tipd/trace.h
+++ b/drivers/usb/typec/tipd/trace.h
@@ -194,6 +194,20 @@
(data_status & TPS_DATA_STATUS_DP_CONNECTION ? \
show_data_status_dp_pin_assignment(data_status) : "")
+#define show_pdo_contract_type(pdo) \
+ __print_symbolic(TPS_PDO_CONTRACT_TYPE(pdo), \
+ { TPS_PDO_CONTRACT_FIXED, "fixed" }, \
+ { TPS_PDO_CONTRACT_BATTERY, "battery" }, \
+ { TPS_PDO_CONTRACT_VARIABLE, "variable" })
+
+#define show_pdo_contract_details(pdo) \
+ __print_flags(pdo & TPS_PDO_FIXED_CONTRACT_DETAILS, "|", \
+ { TPS_PDO_FIXED_CONTRACT_DR_POWER, "dr-power" }, \
+ { TPS_PDO_FIXED_CONTRACT_USB_SUSPEND, "usb-suspend" }, \
+ { TPS_PDO_FIXED_CONTRACT_EXTERNAL_PWR, "ext-power" }, \
+ { TPS_PDO_FIXED_CONTRACT_USB_COMMS, "usb-comms" }, \
+ { TPS_PDO_FIXED_CONTRACT_DR_DATA, "dr-data" })
+
TRACE_EVENT(tps6598x_irq,
TP_PROTO(u64 event1,
u64 event2),
@@ -296,6 +310,30 @@ TRACE_EVENT(tps6598x_data_status,
)
);
+TRACE_EVENT(tps6598x_pdo,
+ TP_PROTO(struct tps6598x_pdo *pdo),
+ TP_ARGS(pdo),
+
+ TP_STRUCT__entry(
+ __field(u32, pdo)
+ __field(int, max_current)
+ __field(int, max_voltage)
+ ),
+
+ TP_fast_assign(
+ __entry->pdo = pdo->pdo;
+ __entry->max_current = pdo->max_current;
+ __entry->max_voltage = pdo->max_voltage;
+ ),
+
+ TP_printk("%s supply, max %duA, %duV, details: %s",
+ show_pdo_contract_type(__entry->pdo),
+ __entry->max_current,
+ __entry->max_voltage,
+ show_pdo_contract_details(__entry->pdo)
+ )
+);
+
#endif /* _TPS6598X_TRACE_H_ */
/* This part must be outside protection */
--
2.35.1
From: Guido Günther <[email protected]>
This allows to verify that a sane firmware is on the device.
Signed-off-by: Guido Günther <[email protected]>
Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
---
drivers/usb/typec/tipd/core.c | 65 +++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 874528b02a99..d3c70aaf1a0c 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -6,6 +6,7 @@
* Author: Heikki Krogerus <[email protected]>
*/
+#include <linux/debugfs.h>
#include <linux/i2c.h>
#include <linux/acpi.h>
#include <linux/module.h>
@@ -22,6 +23,7 @@
/* Register offsets */
#define TPS_REG_VID 0x00
#define TPS_REG_MODE 0x03
+#define TPS_REG_CUSTOMER_USE 0x06
#define TPS_REG_CMD1 0x08
#define TPS_REG_DATA1 0x09
#define TPS_REG_INT_EVENT1 0x14
@@ -99,10 +101,15 @@ struct tps6598x {
struct power_supply *psy;
struct power_supply_desc psy_desc;
enum power_supply_usb_type usb_type;
+
struct tps6598x_pdo terms;
u32 data_status;
u16 pwr_status;
+#ifdef CONFIG_DEBUG_FS
+ struct dentry *dev_dentry;
+ struct dentry *customer_user_dentry;
+#endif
};
static enum power_supply_property tps6598x_psy_props[] = {
@@ -239,6 +246,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
typec_set_data_role(tps->port, role);
}
+#ifdef CONFIG_DEBUG_FS
+static struct dentry *rootdir;
+
+static int tps6598x_debug_customer_use_show(struct seq_file *s, void *v)
+{
+ struct tps6598x *tps = (struct tps6598x *)s->private;
+ u64 mode64;
+ int ret;
+
+ mutex_lock(&tps->lock);
+
+ ret = tps6598x_block_read(tps, TPS_REG_CUSTOMER_USE, &mode64, sizeof(mode64));
+ if (!ret)
+ seq_printf(s, "0x%016llx\n", mode64);
+
+ mutex_unlock(&tps->lock);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(tps6598x_debug_customer_use);
+
+static void tps6598x_debugfs_init(struct tps6598x *tps)
+{
+ struct dentry *dentry;
+
+ if (!rootdir)
+ rootdir = debugfs_create_dir("tps6598x", NULL);
+
+ dentry = debugfs_create_dir(dev_name(tps->dev), rootdir);
+ if (IS_ERR(dentry))
+ return;
+ tps->dev_dentry = dentry;
+
+ dentry = debugfs_create_file("customer_use",
+ S_IFREG | 0444, tps->dev_dentry,
+ tps, &tps6598x_debug_customer_use_fops);
+ if (IS_ERR(dentry))
+ return;
+ tps->customer_user_dentry = dentry;
+}
+
+static void tps6598x_debugfs_exit(struct tps6598x *tps)
+{
+ debugfs_remove(tps->customer_user_dentry);
+ debugfs_remove(tps->dev_dentry);
+ debugfs_remove(rootdir);
+ rootdir = NULL;
+}
+
+#else
+
+static void tps6598x_debugfs_init(const struct tps6598x *tps) { }
+static void tps6598x_debugfs_exit(const struct tps6598x *tps) { }
+
+#endif
+
static int tps6598x_connect(struct tps6598x *tps, u32 status)
{
struct typec_partner_desc desc;
@@ -995,6 +1058,7 @@ static int tps6598x_probe(struct i2c_client *client)
}
i2c_set_clientdata(client, tps);
+ tps6598x_debugfs_init(tps);
return 0;
@@ -1011,6 +1075,7 @@ static int tps6598x_remove(struct i2c_client *client)
{
struct tps6598x *tps = i2c_get_clientdata(client);
+ tps6598x_debugfs_exit(tps);
tps6598x_disconnect(tps, 0);
typec_unregister_port(tps->port);
usb_role_switch_put(tps->role_sw);
--
2.35.1
This allows userspace and downstream supplies to know whether
something is plugged in.
Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
---
drivers/usb/typec/tipd/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index f3e8f1183f5b..874528b02a99 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -108,6 +108,7 @@ struct tps6598x {
static enum power_supply_property tps6598x_psy_props[] = {
POWER_SUPPLY_PROP_USB_TYPE,
POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_CURRENT_MAX,
POWER_SUPPLY_PROP_VOLTAGE_MAX,
};
@@ -756,6 +757,9 @@ static int tps6598x_psy_get_prop(struct power_supply *psy,
case POWER_SUPPLY_PROP_ONLINE:
ret = tps6598x_psy_get_online(tps, val);
break;
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = !!tps->partner;
+ break;
case POWER_SUPPLY_PROP_CURRENT_MAX:
ret = tps6598x_psy_get_max_current(tps, val);
break;
--
2.35.1
BOOT mode means that the device isn't operational because of missing
firmware, so there's no reason to try to continue in this condition
(probe will fail in a different place anyway).
Aside of that, the warning that used to be emited about "dead-battery
condition" was misleading, as dead-battery condition is a different
thing that's unrelated to operation mode.
Therefore, assume that BOOT mode is not a supported mode of operation.
Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
---
drivers/usb/typec/tipd/core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index d3c70aaf1a0c..c818cc40139d 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -729,8 +729,6 @@ static int tps6598x_check_mode(struct tps6598x *tps)
case TPS_MODE_APP:
return 0;
case TPS_MODE_BOOT:
- dev_warn(tps->dev, "dead-battery condition\n");
- return 0;
case TPS_MODE_BIST:
case TPS_MODE_DISC:
default:
--
2.35.1
Hi,
On Thu, Mar 17, 2022 at 04:45:11PM +0100, Sebastian Krzyszkowiak wrote:
> This is a series developed for the Librem 5 phone, which uses TPS65982
> as its USB-C controller. Implemented are Power Delivery sink contract
> tracing and exporting negotiated power values as power supply properties,
> fixes for data role swapping, status register caching and a debugfs entry
> for querying customer use word of the firmware running on the controller.
>
> Angus Ainslie (3):
> usb: typec: tipd: set the data role on tps IRQ
> usb: typec: tipd: Add trace event for SINK PD contract
> usb: typec: tipd: Provide POWER_SUPPLY_PROP_{CURRENT,VOLTAGE}_MAX
>
> Guido G?nther (2):
> usb: typec: tipd: Only update power status on IRQ
> usb: typec: tipd: Add debugfs entries for customer use word
>
> Sebastian Krzyszkowiak (2):
> usb: typec: tipd: Provide POWER_SUPPLY_PROP_PRESENT
> usb: typec: tipd: Fail probe when the controller is in BOOT mode
>
> drivers/usb/typec/tipd/core.c | 263 ++++++++++++++++++++++++++----
> drivers/usb/typec/tipd/tps6598x.h | 30 ++++
> drivers/usb/typec/tipd/trace.h | 38 +++++
> 3 files changed, 302 insertions(+), 29 deletions(-)
These look pretty good to me. I'll see if I can test these on Monday -
I finally have access to a machine again that actually has TI PD
controller. But I will give my comments then in any case, if there is
anything to comment.
But related to patch 3/7, there is a series in works that would expose
the PDOs in sysfs [1]. I was wondering have you guys noticed it, and
would that actually work also in your case?
[1] https://lore.kernel.org/linux-usb/[email protected]/
Br,
--
heikki
On Thu, Mar 17, 2022 at 04:45:17PM +0100, Sebastian Krzyszkowiak wrote:
> From: Guido G?nther <[email protected]>
>
> This allows to verify that a sane firmware is on the device.
>
> Signed-off-by: Guido G?nther <[email protected]>
> Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 65 +++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 874528b02a99..d3c70aaf1a0c 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -6,6 +6,7 @@
> * Author: Heikki Krogerus <[email protected]>
> */
>
> +#include <linux/debugfs.h>
> #include <linux/i2c.h>
> #include <linux/acpi.h>
> #include <linux/module.h>
> @@ -22,6 +23,7 @@
> /* Register offsets */
> #define TPS_REG_VID 0x00
> #define TPS_REG_MODE 0x03
> +#define TPS_REG_CUSTOMER_USE 0x06
> #define TPS_REG_CMD1 0x08
> #define TPS_REG_DATA1 0x09
> #define TPS_REG_INT_EVENT1 0x14
> @@ -99,10 +101,15 @@ struct tps6598x {
> struct power_supply *psy;
> struct power_supply_desc psy_desc;
> enum power_supply_usb_type usb_type;
> +
> struct tps6598x_pdo terms;
>
> u32 data_status;
> u16 pwr_status;
> +#ifdef CONFIG_DEBUG_FS
> + struct dentry *dev_dentry;
> + struct dentry *customer_user_dentry;
> +#endif
> };
>
> static enum power_supply_property tps6598x_psy_props[] = {
> @@ -239,6 +246,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
> typec_set_data_role(tps->port, role);
> }
>
> +#ifdef CONFIG_DEBUG_FS
> +static struct dentry *rootdir;
> +
> +static int tps6598x_debug_customer_use_show(struct seq_file *s, void *v)
> +{
> + struct tps6598x *tps = (struct tps6598x *)s->private;
> + u64 mode64;
> + int ret;
> +
> + mutex_lock(&tps->lock);
> +
> + ret = tps6598x_block_read(tps, TPS_REG_CUSTOMER_USE, &mode64, sizeof(mode64));
> + if (!ret)
> + seq_printf(s, "0x%016llx\n", mode64);
> +
> + mutex_unlock(&tps->lock);
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(tps6598x_debug_customer_use);
> +
> +static void tps6598x_debugfs_init(struct tps6598x *tps)
> +{
> + struct dentry *dentry;
> +
> + if (!rootdir)
> + rootdir = debugfs_create_dir("tps6598x", NULL);
> +
> + dentry = debugfs_create_dir(dev_name(tps->dev), rootdir);
> + if (IS_ERR(dentry))
> + return;
> + tps->dev_dentry = dentry;
> +
> + dentry = debugfs_create_file("customer_use",
> + S_IFREG | 0444, tps->dev_dentry,
> + tps, &tps6598x_debug_customer_use_fops);
> + if (IS_ERR(dentry))
> + return;
> + tps->customer_user_dentry = dentry;
> +}
> +
> +static void tps6598x_debugfs_exit(struct tps6598x *tps)
> +{
> + debugfs_remove(tps->customer_user_dentry);
> + debugfs_remove(tps->dev_dentry);
> + debugfs_remove(rootdir);
> + rootdir = NULL;
> +}
> +
> +#else
> +
> +static void tps6598x_debugfs_init(const struct tps6598x *tps) { }
> +static void tps6598x_debugfs_exit(const struct tps6598x *tps) { }
> +
> +#endif
> +
> static int tps6598x_connect(struct tps6598x *tps, u32 status)
> {
> struct typec_partner_desc desc;
> @@ -995,6 +1058,7 @@ static int tps6598x_probe(struct i2c_client *client)
> }
>
> i2c_set_clientdata(client, tps);
> + tps6598x_debugfs_init(tps);
>
> return 0;
>
> @@ -1011,6 +1075,7 @@ static int tps6598x_remove(struct i2c_client *client)
> {
> struct tps6598x *tps = i2c_get_clientdata(client);
>
> + tps6598x_debugfs_exit(tps);
> tps6598x_disconnect(tps, 0);
> typec_unregister_port(tps->port);
> usb_role_switch_put(tps->role_sw);
> --
> 2.35.1
--
heikki
On Thu, Mar 17, 2022 at 04:45:14PM +0100, Sebastian Krzyszkowiak wrote:
> From: Angus Ainslie <[email protected]>
>
> This allows to trace the negotiated contract as sink. It's
> only updated when the contract is actually established.
With the PDOs (and also RDO) I think we'll rely on the sysfs interface
that I mentioned once it's ready, but since this is only trace stuff, I
guess it's okay to take this in the mean time.
I in any case noticed one issue below...
> Co-developed-by: Guido G?nther <[email protected]>
> Signed-off-by: Guido G?nther <[email protected]>
> Signed-off-by: Angus Ainslie <[email protected]>
> Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 63 ++++++++++++++++++++++++++++++-
> drivers/usb/typec/tipd/tps6598x.h | 30 +++++++++++++++
> drivers/usb/typec/tipd/trace.h | 38 +++++++++++++++++++
> 3 files changed, 129 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index f387786ff95e..80b4a9870caf 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -34,6 +34,7 @@
> #define TPS_REG_STATUS 0x1a
> #define TPS_REG_SYSTEM_CONF 0x28
> #define TPS_REG_CTRL_CONF 0x29
> +#define TPS_REG_ACTIVE_CONTRACT 0x34
> #define TPS_REG_POWER_STATUS 0x3f
> #define TPS_REG_RX_IDENTITY_SOP 0x48
> #define TPS_REG_DATA_STATUS 0x5f
> @@ -93,6 +94,7 @@ struct tps6598x {
> struct power_supply *psy;
> struct power_supply_desc psy_desc;
> enum power_supply_usb_type usb_type;
> + struct tps6598x_pdo terms;
>
> u32 data_status;
> u16 pwr_status;
> @@ -528,6 +530,47 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
> return IRQ_NONE;
> }
>
> +static int tps6598x_get_active_pd_contract(struct tps6598x *tps)
> +{
> + u64 contract;
> + int type;
> + int max_power;
> + int ret = 0;
> +
> + ret = tps6598x_block_read(tps, TPS_REG_ACTIVE_CONTRACT, &contract, 6);
> + if (ret)
> + return ret;
> +
> + contract &= 0xFFFFFFFFFFFF;
> + type = TPS_PDO_CONTRACT_TYPE(contract);
> + memset(&tps->terms, 0, sizeof(struct tps6598x_pdo));
> +
> + /* If there's no PD it decodes to all 0 */
> + switch (type) {
> + case TPS_PDO_CONTRACT_FIXED:
> + tps->terms.max_voltage = TPS_PDO_FIXED_CONTRACT_VOLTAGE(contract);
> + tps->terms.max_current = TPS_PDO_FIXED_CONTRACT_MAX_CURRENT(contract);
> + break;
> + case TPS_PDO_CONTRACT_BATTERY:
> + tps->terms.max_voltage = TPS_PDO_BAT_CONTRACT_MAX_VOLTAGE(contract);
> + max_power = TPS_PDO_BAT_CONTRACT_MAX_POWER(contract);
> + tps->terms.max_current = 1000 * 1000 * max_power / tps->terms.max_voltage;
> + break;
> + case TPS_PDO_CONTRACT_VARIABLE:
> + tps->terms.max_voltage = TPS_PDO_VAR_CONTRACT_MAX_VOLTAGE(contract);
> + tps->terms.max_current = TPS_PDO_VAR_CONTRACT_MAX_CURRENT(contract);
> + break;
> + default:
> + dev_warn(tps->dev, "Unknown contract type: %d\n", type);
> + return -EINVAL;
> + }
> +
> + tps->terms.pdo = contract;
> + trace_tps6598x_pdo(&tps->terms);
> +
> + return 0;
> +}
> +
> static irqreturn_t tps6598x_interrupt(int irq, void *data)
> {
> struct tps6598x *tps = data;
> @@ -563,6 +606,14 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> tps6598x_update_data_status(tps, status);
> }
>
> + if ((event1 | event2) & TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER) {
> + ret = tps6598x_get_active_pd_contract(tps);
> + if (ret) {
> + dev_err(tps->dev, "failed to read pd contract: %d\n", ret);
> + goto err_clear_ints;
> + }
> + }
> +
> /* Handle plug insert or removal */
> if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
> tps6598x_handle_plug_event(tps, status);
> @@ -751,10 +802,11 @@ static int tps6598x_probe(struct i2c_client *client)
>
> irq_handler = cd321x_interrupt;
> } else {
> - /* Enable power status, data status and plug event interrupts */
> + /* Enable interrupts used by this driver */
> mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
> TPS_REG_INT_DATA_STATUS_UPDATE |
> - TPS_REG_INT_PLUG_EVENT;
> + TPS_REG_INT_PLUG_EVENT |
> + TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER;
> }
>
> /* Make sure the controller has application firmware running */
> @@ -847,6 +899,13 @@ static int tps6598x_probe(struct i2c_client *client)
> ret = tps6598x_connect(tps, status);
> if (ret)
> dev_err(&client->dev, "failed to register partner\n");
> +
> + if ((TPS_POWER_STATUS_PWROPMODE(tps->pwr_status) == TYPEC_PWR_MODE_PD) &&
> + (!(status & TPS_STATUS_PORTROLE))) {
> + ret = tps6598x_get_active_pd_contract(tps);
> + if (ret)
> + dev_err(tps->dev, "failed to read pd contract: %d\n", ret);
> + }
> }
>
> ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h
> index 527857549d69..546cd4881f1f 100644
> --- a/drivers/usb/typec/tipd/tps6598x.h
> +++ b/drivers/usb/typec/tipd/tps6598x.h
> @@ -199,4 +199,34 @@
> #define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_A BIT(2)
> #define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
>
> +
> +/* PDO decoding as in https://www.ti.com/lit/an/slva842/slva842.pdf */
> +#define TPS_PDO_CONTRACT_TYPE(x) FIELD_GET(GENMASK(31, 30), x)
> +#define TPS_PDO_CONTRACT_FIXED 0
> +#define TPS_PDO_CONTRACT_BATTERY 1
> +#define TPS_PDO_CONTRACT_VARIABLE 2
> +
> +#define TPS_PDO_FIXED_CONTRACT_DETAILS GENMASK(29, 25)
> +#define TPS_PDO_FIXED_CONTRACT_DR_POWER BIT(29)
> +#define TPS_PDO_FIXED_CONTRACT_USB_SUSPEND BIT(28)
> +#define TPS_PDO_FIXED_CONTRACT_EXTERNAL_PWR BIT(27)
> +#define TPS_PDO_FIXED_CONTRACT_USB_COMMS BIT(26)
> +#define TPS_PDO_FIXED_CONTRACT_DR_DATA BIT(25)
Those are already defined in include/linux/usb/pd.h
> +#define TPS_PDO_FIXED_CONTRACT_VOLTAGE(x) (FIELD_GET(GENMASK(19, 10), x) * 50000)
> +#define TPS_PDO_FIXED_CONTRACT_MAX_CURRENT(x) (FIELD_GET(GENMASK(9, 0), x) * 10000)
> +#define TPS_PDO_VAR_CONTRACT_MAX_VOLTAGE(x) (FIELD_GET(GENMASK(29, 20), x) * 50000)
> +#define TPS_PDO_VAR_CONTRACT_MIN_VOLTAGE(x) (FIELD_GET(GENMASK(19, 10), x) * 50000)
> +#define TPS_PDO_VAR_CONTRACT_MAX_CURRENT(x) (FIELD_GET(GENMASK(9, 0), x) * 10000)
> +#define TPS_PDO_BAT_CONTRACT_MAX_VOLTAGE(x) (FIELD_GET(GENMASK(29, 20), x) * 50000)
> +#define TPS_PDO_BAT_CONTRACT_MIN_VOLTAGE(x) (FIELD_GET(GENMASK(19, 10), x) * 50000)
> +#define TPS_PDO_BAT_CONTRACT_MAX_POWER(x) (FIELD_GET(GENMASK(9, 0), x) * 250000)
And I believe the same with these too.
> +struct tps6598x_pdo {
> + u32 pdo;
> + int max_voltage; /* uV */
> + int max_current; /* uA */
> + int max_power; /* uW */
> +};
> +
> #endif /* __TPS6598X_H__ */
> diff --git a/drivers/usb/typec/tipd/trace.h b/drivers/usb/typec/tipd/trace.h
> index 12cad1bde7cc..148e2ef3157b 100644
> --- a/drivers/usb/typec/tipd/trace.h
> +++ b/drivers/usb/typec/tipd/trace.h
> @@ -194,6 +194,20 @@
> (data_status & TPS_DATA_STATUS_DP_CONNECTION ? \
> show_data_status_dp_pin_assignment(data_status) : "")
>
> +#define show_pdo_contract_type(pdo) \
> + __print_symbolic(TPS_PDO_CONTRACT_TYPE(pdo), \
> + { TPS_PDO_CONTRACT_FIXED, "fixed" }, \
> + { TPS_PDO_CONTRACT_BATTERY, "battery" }, \
> + { TPS_PDO_CONTRACT_VARIABLE, "variable" })
> +
> +#define show_pdo_contract_details(pdo) \
> + __print_flags(pdo & TPS_PDO_FIXED_CONTRACT_DETAILS, "|", \
> + { TPS_PDO_FIXED_CONTRACT_DR_POWER, "dr-power" }, \
> + { TPS_PDO_FIXED_CONTRACT_USB_SUSPEND, "usb-suspend" }, \
> + { TPS_PDO_FIXED_CONTRACT_EXTERNAL_PWR, "ext-power" }, \
> + { TPS_PDO_FIXED_CONTRACT_USB_COMMS, "usb-comms" }, \
> + { TPS_PDO_FIXED_CONTRACT_DR_DATA, "dr-data" })
> +
> TRACE_EVENT(tps6598x_irq,
> TP_PROTO(u64 event1,
> u64 event2),
> @@ -296,6 +310,30 @@ TRACE_EVENT(tps6598x_data_status,
> )
> );
>
> +TRACE_EVENT(tps6598x_pdo,
> + TP_PROTO(struct tps6598x_pdo *pdo),
> + TP_ARGS(pdo),
> +
> + TP_STRUCT__entry(
> + __field(u32, pdo)
> + __field(int, max_current)
> + __field(int, max_voltage)
> + ),
> +
> + TP_fast_assign(
> + __entry->pdo = pdo->pdo;
> + __entry->max_current = pdo->max_current;
> + __entry->max_voltage = pdo->max_voltage;
> + ),
> +
> + TP_printk("%s supply, max %duA, %duV, details: %s",
> + show_pdo_contract_type(__entry->pdo),
> + __entry->max_current,
> + __entry->max_voltage,
> + show_pdo_contract_details(__entry->pdo)
> + )
> +);
> +
> #endif /* _TPS6598X_TRACE_H_ */
>
> /* This part must be outside protection */
thanks,
--
heikki
On Thu, Mar 17, 2022 at 04:45:18PM +0100, Sebastian Krzyszkowiak wrote:
> BOOT mode means that the device isn't operational because of missing
> firmware, so there's no reason to try to continue in this condition
> (probe will fail in a different place anyway).
>
> Aside of that, the warning that used to be emited about "dead-battery
> condition" was misleading, as dead-battery condition is a different
> thing that's unrelated to operation mode.
>
> Therefore, assume that BOOT mode is not a supported mode of operation.
>
> Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index d3c70aaf1a0c..c818cc40139d 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -729,8 +729,6 @@ static int tps6598x_check_mode(struct tps6598x *tps)
> case TPS_MODE_APP:
> return 0;
> case TPS_MODE_BOOT:
> - dev_warn(tps->dev, "dead-battery condition\n");
> - return 0;
> case TPS_MODE_BIST:
> case TPS_MODE_DISC:
> default:
> --
> 2.35.1
thanks,
--
heikki
Hi,
On Thu, Mar 17, 2022 at 04:45:15PM +0100, Sebastian Krzyszkowiak wrote:
> From: Angus Ainslie <[email protected]>
>
> This helps downstream supplies to adjust their input limits.
>
> Signed-off-by: Angus Ainslie <[email protected]>
> Signed-off-by: Guido G?nther <[email protected]>
> Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 76 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 80b4a9870caf..f3e8f1183f5b 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -39,6 +39,11 @@
> #define TPS_REG_RX_IDENTITY_SOP 0x48
> #define TPS_REG_DATA_STATUS 0x5f
>
> +#define TPS_USB_500mA 500000
> +#define TPS_TYPEC_1500mA 1500000
> +#define TPS_TYPEC_3000mA 3000000
> +#define TPS_USB_5V 5000000
> +
> /* TPS_REG_SYSTEM_CONF bits */
> #define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
>
> @@ -103,6 +108,8 @@ struct tps6598x {
> static enum power_supply_property tps6598x_psy_props[] = {
> POWER_SUPPLY_PROP_USB_TYPE,
> POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_CURRENT_MAX,
> + POWER_SUPPLY_PROP_VOLTAGE_MAX,
> };
>
> static enum power_supply_usb_type tps6598x_psy_usb_types[] = {
> @@ -294,6 +301,8 @@ static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
> typec_set_orientation(tps->port, TYPEC_ORIENTATION_NONE);
> tps6598x_set_data_role(tps, TPS_STATUS_TO_TYPEC_DATAROLE(status), false);
>
> + memset(&tps->terms, 0, sizeof(struct tps6598x_pdo));
> +
> power_supply_changed(tps->psy);
> }
>
> @@ -577,6 +586,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> u64 event1;
> u64 event2;
> u32 status;
> + bool psy_changed = false;
> int ret;
>
> mutex_lock(&tps->lock);
> @@ -595,10 +605,13 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> if (!tps6598x_read_status(tps, &status))
> goto err_clear_ints;
>
> - if ((event1 | event2) & TPS_REG_INT_POWER_STATUS_UPDATE)
> + if ((event1 | event2) & TPS_REG_INT_POWER_STATUS_UPDATE) {
> if (!tps6598x_read_power_status(tps))
> goto err_clear_ints;
>
> + psy_changed = true;
> + }
> +
> if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE) {
> if (!tps6598x_read_data_status(tps))
> goto err_clear_ints;
> @@ -612,12 +625,18 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> dev_err(tps->dev, "failed to read pd contract: %d\n", ret);
> goto err_clear_ints;
> }
> + psy_changed = true;
Can data status change alone really indicate that the contract has
changed?
> }
>
> /* Handle plug insert or removal */
> if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
> tps6598x_handle_plug_event(tps, status);
>
> + if ((event1 | event2) & TPS_REG_INT_HARD_RESET) {
> + memset(&tps->terms, 0, sizeof(struct tps6598x_pdo));
> + psy_changed = true;
> + }
> +
> err_clear_ints:
> tps6598x_write64(tps, TPS_REG_INT_CLEAR1, event1);
> tps6598x_write64(tps, TPS_REG_INT_CLEAR2, event2);
> @@ -625,6 +644,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> err_unlock:
> mutex_unlock(&tps->lock);
>
> + if (psy_changed)
> + power_supply_changed(tps->psy);
> +
> if (event1 | event2)
> return IRQ_HANDLED;
> return IRQ_NONE;
> @@ -671,6 +693,49 @@ static int tps6598x_psy_get_online(struct tps6598x *tps,
> } else {
> val->intval = 0;
> }
> +
> + return 0;
> +}
> +
> +static int tps6598x_psy_get_max_current(struct tps6598x *tps,
> + union power_supply_propval *val)
> +{
> + enum typec_pwr_opmode mode;
> +
> + mode = TPS_POWER_STATUS_PWROPMODE(tps->pwr_status);
> + switch (mode) {
> + case TYPEC_PWR_MODE_1_5A:
> + val->intval = TPS_TYPEC_1500mA;
> + break;
> + case TYPEC_PWR_MODE_3_0A:
> + val->intval = TPS_TYPEC_3000mA;
> + break;
> + case TYPEC_PWR_MODE_PD:
> + val->intval = tps->terms.max_current ?: TPS_USB_500mA;
> + break;
> + default:
> + case TYPEC_PWR_MODE_USB:
> + val->intval = TPS_USB_500mA;
> + }
> + return 0;
> +}
> +
> +static int tps6598x_psy_get_max_voltage(struct tps6598x *tps,
> + union power_supply_propval *val)
> +{
> + enum typec_pwr_opmode mode;
> +
> + mode = TPS_POWER_STATUS_PWROPMODE(tps->pwr_status);
> + switch (mode) {
> + case TYPEC_PWR_MODE_PD:
> + val->intval = tps->terms.max_voltage ?: TPS_USB_5V;
> + break;
> + default:
> + case TYPEC_PWR_MODE_1_5A:
> + case TYPEC_PWR_MODE_3_0A:
> + case TYPEC_PWR_MODE_USB:
> + val->intval = TPS_USB_5V;
> + }
> return 0;
> }
>
> @@ -691,6 +756,12 @@ static int tps6598x_psy_get_prop(struct power_supply *psy,
> case POWER_SUPPLY_PROP_ONLINE:
> ret = tps6598x_psy_get_online(tps, val);
> break;
> + case POWER_SUPPLY_PROP_CURRENT_MAX:
> + ret = tps6598x_psy_get_max_current(tps, val);
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> + ret = tps6598x_psy_get_max_voltage(tps, val);
> + break;
> default:
> ret = -EINVAL;
> break;
> @@ -806,7 +877,8 @@ static int tps6598x_probe(struct i2c_client *client)
> mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
> TPS_REG_INT_DATA_STATUS_UPDATE |
> TPS_REG_INT_PLUG_EVENT |
> - TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER;
> + TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER |
> + TPS_REG_INT_HARD_RESET;
> }
>
> /* Make sure the controller has application firmware running */
> --
> 2.35.1
thanks,
--
heikki
On Thu, Mar 17, 2022 at 04:45:13PM +0100, Sebastian Krzyszkowiak wrote:
> From: Angus Ainslie <[email protected]>
>
> Don't immediately set the data role, only set it in response to the
> negotiated value notification from the tps6589x. Otherwise data role
> switch fails for DRP.
>
> We only use values cached from the IRQ instead of poking I2C all
> the time.
>
> The update is moved in a function that will become more useful in later
> commits.
>
> Fixes: 18a6c866bb19 ("usb: typec: tps6598x: Add USB role switching logic")
> Signed-off-by: Angus Ainslie <[email protected]>
> Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
Fixes tag but but no Cc: stable... tag?
Is there some reason why you don't have the stable tag, i.e. why
shouldn't this be added to the stable kernels?
> ---
> drivers/usb/typec/tipd/core.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index dfbba5ae9487..f387786ff95e 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -94,6 +94,7 @@ struct tps6598x {
> struct power_supply_desc psy_desc;
> enum power_supply_usb_type usb_type;
>
> + u32 data_status;
> u16 pwr_status;
> };
>
> @@ -271,6 +272,15 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
> return 0;
> }
>
> +static int
> +tps6598x_update_data_status(struct tps6598x *tps, u32 status)
> +{
> + tps6598x_set_data_role(tps, TPS_STATUS_TO_TYPEC_DATAROLE(status),
> + !!(tps->data_status & TPS_DATA_STATUS_DATA_CONNECTION));
> + trace_tps6598x_data_status(tps->data_status);
> + return 0;
> +}
> +
> static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
> {
> if (!IS_ERR(tps->partner))
> @@ -370,8 +380,6 @@ static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
> goto out_unlock;
> }
>
> - tps6598x_set_data_role(tps, role, true);
> -
> out_unlock:
> mutex_unlock(&tps->lock);
>
> @@ -437,6 +445,7 @@ static bool tps6598x_read_data_status(struct tps6598x *tps)
> dev_err(tps->dev, "failed to read data status: %d\n", ret);
> return false;
> }
> + tps->data_status = data_status;
> trace_tps6598x_data_status(data_status);
>
> return true;
> @@ -497,10 +506,13 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
> if (!tps6598x_read_power_status(tps))
> goto err_clear_ints;
>
> - if (event & APPLE_CD_REG_INT_DATA_STATUS_UPDATE)
> + if (event & APPLE_CD_REG_INT_DATA_STATUS_UPDATE) {
> if (!tps6598x_read_data_status(tps))
> goto err_clear_ints;
>
> + tps6598x_update_data_status(tps, status);
> + }
> +
> /* Handle plug insert or removal */
> if (event & APPLE_CD_REG_INT_PLUG_EVENT)
> tps6598x_handle_plug_event(tps, status);
> @@ -544,10 +556,13 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
> if (!tps6598x_read_power_status(tps))
> goto err_clear_ints;
>
> - if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE)
> + if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE) {
> if (!tps6598x_read_data_status(tps))
> goto err_clear_ints;
>
> + tps6598x_update_data_status(tps, status);
> + }
> +
> /* Handle plug insert or removal */
> if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
> tps6598x_handle_plug_event(tps, status);
> --
> 2.35.1
--
heikki
On Thu, Mar 17, 2022 at 04:45:16PM +0100, Sebastian Krzyszkowiak wrote:
> This allows userspace and downstream supplies to know whether
> something is plugged in.
>
> Signed-off-by: Sebastian Krzyszkowiak <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index f3e8f1183f5b..874528b02a99 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -108,6 +108,7 @@ struct tps6598x {
> static enum power_supply_property tps6598x_psy_props[] = {
> POWER_SUPPLY_PROP_USB_TYPE,
> POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_PRESENT,
> POWER_SUPPLY_PROP_CURRENT_MAX,
> POWER_SUPPLY_PROP_VOLTAGE_MAX,
> };
> @@ -756,6 +757,9 @@ static int tps6598x_psy_get_prop(struct power_supply *psy,
> case POWER_SUPPLY_PROP_ONLINE:
> ret = tps6598x_psy_get_online(tps, val);
> break;
> + case POWER_SUPPLY_PROP_PRESENT:
> + val->intval = !!tps->partner;
> + break;
> case POWER_SUPPLY_PROP_CURRENT_MAX:
> ret = tps6598x_psy_get_max_current(tps, val);
> break;
> --
> 2.35.1
--
heikki