* [PATCH v2 1/4] usb: typec: tps6598x: Add trace event for IRQ events
2021-02-12 12:04 [PATCH v2 0/4] usb: typec: tps6598x: Add IRQ flag and register tracing Guido Günther
@ 2021-02-12 12:04 ` Guido Günther
2021-02-12 12:04 ` [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register Guido Günther
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Guido Günther @ 2021-02-12 12:04 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb
Allow to get irq event information via the tracing framework. This
allows to inspect USB-C negotiation at runtime.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
drivers/usb/typec/Makefile | 3 +
drivers/usb/typec/tps6598x.c | 9 ++-
drivers/usb/typec/tps6598x.h | 64 ++++++++++++++++++++
drivers/usb/typec/tps6598x_trace.h | 97 ++++++++++++++++++++++++++++++
4 files changed, 170 insertions(+), 3 deletions(-)
create mode 100644 drivers/usb/typec/tps6598x.h
create mode 100644 drivers/usb/typec/tps6598x_trace.h
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index d03b48c4b864..27aa12129190 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,4 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+# define_trace.h needs to know how to find our header
+CFLAGS_tps6598x.o := -I$(src)
+
obj-$(CONFIG_TYPEC) += typec.o
typec-y := class.o mux.o bus.o
obj-$(CONFIG_TYPEC) += altmodes/
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 6e6ef6317523..bc34b35e909f 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -6,6 +6,8 @@
* Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
*/
+#include "tps6598x.h"
+
#include <linux/i2c.h>
#include <linux/acpi.h>
#include <linux/module.h>
@@ -15,6 +17,9 @@
#include <linux/usb/typec.h>
#include <linux/usb/role.h>
+#define CREATE_TRACE_POINTS
+#include "tps6598x_trace.h"
+
/* Register offsets */
#define TPS_REG_VID 0x00
#define TPS_REG_MODE 0x03
@@ -32,9 +37,6 @@
#define TPS_REG_POWER_STATUS 0x3f
#define TPS_REG_RX_IDENTITY_SOP 0x48
-/* TPS_REG_INT_* bits */
-#define TPS_REG_INT_PLUG_EVENT BIT(3)
-
/* TPS_REG_STATUS bits */
#define TPS_STATUS_PLUG_PRESENT BIT(0)
#define TPS_STATUS_ORIENTATION BIT(4)
@@ -428,6 +430,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
dev_err(tps->dev, "%s: failed to read events\n", __func__);
goto err_unlock;
}
+ trace_tps6598x_irq(event1, event2);
ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
if (ret) {
diff --git a/drivers/usb/typec/tps6598x.h b/drivers/usb/typec/tps6598x.h
new file mode 100644
index 000000000000..b83b8a6a1504
--- /dev/null
+++ b/drivers/usb/typec/tps6598x.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Driver for TI TPS6598x USB Power Delivery controller family
+ *
+ * Copyright (C) 2017, Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+
+#ifndef __TPS6598X_H__
+#define __TPS6598X_H__
+
+
+/* TPS_REG_INT_* bits */
+#define TPS_REG_INT_USER_VID_ALT_MODE_OTHER_VDM BIT_ULL(27+32)
+#define TPS_REG_INT_USER_VID_ALT_MODE_ATTN_VDM BIT_ULL(26+32)
+#define TPS_REG_INT_USER_VID_ALT_MODE_EXIT BIT_ULL(25+32)
+#define TPS_REG_INT_USER_VID_ALT_MODE_ENTERED BIT_ULL(24+32)
+#define TPS_REG_INT_EXIT_MODES_COMPLETE BIT_ULL(20+32)
+#define TPS_REG_INT_DISCOVER_MODES_COMPLETE BIT_ULL(19+32)
+#define TPS_REG_INT_VDM_MSG_SENT BIT_ULL(18+32)
+#define TPS_REG_INT_VDM_ENTERED_MODE BIT_ULL(17+32)
+#define TPS_REG_INT_ERROR_UNABLE_TO_SOURCE BIT_ULL(14+32)
+#define TPS_REG_INT_SRC_TRANSITION BIT_ULL(10+32)
+#define TPS_REG_INT_ERROR_DISCHARGE_FAILED BIT_ULL(9+32)
+#define TPS_REG_INT_ERROR_MESSAGE_DATA BIT_ULL(7+32)
+#define TPS_REG_INT_ERROR_PROTOCOL_ERROR BIT_ULL(6+32)
+#define TPS_REG_INT_ERROR_MISSING_GET_CAP_MESSAGE BIT_ULL(4+32)
+#define TPS_REG_INT_ERROR_POWER_EVENT_OCCURRED BIT_ULL(3+32)
+#define TPS_REG_INT_ERROR_CAN_PROVIDE_PWR_LATER BIT_ULL(2+32)
+#define TPS_REG_INT_ERROR_CANNOT_PROVIDE_PWR BIT_ULL(1+32)
+#define TPS_REG_INT_ERROR_DEVICE_INCOMPATIBLE BIT_ULL(0+32)
+#define TPS_REG_INT_CMD2_COMPLETE BIT(31)
+#define TPS_REG_INT_CMD1_COMPLETE BIT(30)
+#define TPS_REG_INT_ADC_HIGH_THRESHOLD BIT(29)
+#define TPS_REG_INT_ADC_LOW_THRESHOLD BIT(28)
+#define TPS_REG_INT_PD_STATUS_UPDATE BIT(27)
+#define TPS_REG_INT_STATUS_UPDATE BIT(26)
+#define TPS_REG_INT_DATA_STATUS_UPDATE BIT(25)
+#define TPS_REG_INT_POWER_STATUS_UPDATE BIT(24)
+#define TPS_REG_INT_PP_SWITCH_CHANGED BIT(23)
+#define TPS_REG_INT_HIGH_VOLTAGE_WARNING BIT(22)
+#define TPS_REG_INT_USB_HOST_PRESENT_NO_LONGER BIT(21)
+#define TPS_REG_INT_USB_HOST_PRESENT BIT(20)
+#define TPS_REG_INT_GOTO_MIN_RECEIVED BIT(19)
+#define TPS_REG_INT_PR_SWAP_REQUESTED BIT(17)
+#define TPS_REG_INT_SINK_CAP_MESSAGE_READY BIT(15)
+#define TPS_REG_INT_SOURCE_CAP_MESSAGE_READY BIT(14)
+#define TPS_REG_INT_NEW_CONTRACT_AS_PROVIDER BIT(13)
+#define TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER BIT(12)
+#define TPS_REG_INT_VDM_RECEIVED BIT(11)
+#define TPS_REG_INT_ATTENTION_RECEIVED BIT(10)
+#define TPS_REG_INT_OVERCURRENT BIT(9)
+#define TPS_REG_INT_BIST BIT(8)
+#define TPS_REG_INT_RDO_RECEIVED_FROM_SINK BIT(7)
+#define TPS_REG_INT_DR_SWAP_COMPLETE BIT(5)
+#define TPS_REG_INT_PR_SWAP_COMPLETE BIT(4)
+#define TPS_REG_INT_PLUG_EVENT BIT(3)
+#define TPS_REG_INT_HARD_RESET BIT(1)
+#define TPS_REG_INT_PD_SOFT_RESET BIT(0)
+
+#endif /* __TPS6598X_H__ */
diff --git a/drivers/usb/typec/tps6598x_trace.h b/drivers/usb/typec/tps6598x_trace.h
new file mode 100644
index 000000000000..4ec96e3b2c3e
--- /dev/null
+++ b/drivers/usb/typec/tps6598x_trace.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Driver for TI TPS6598x USB Power Delivery controller family
+ *
+ * Copyright (C) 2020 Purism SPC
+ * Author: Guido Günther <agx@sigxcpu.org>
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM tps6598x
+
+#if !defined(_TPS6598x_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define _TPS6598X_TRACE_H_
+
+#include "tps6598x.h"
+
+#include <linux/stringify.h>
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+#define show_irq_flags(flags) \
+ __print_flags_u64(flags, "|", \
+ { TPS_REG_INT_PD_SOFT_RESET, "PD_SOFT_RESET" }, \
+ { TPS_REG_INT_HARD_RESET, "HARD_RESET" }, \
+ { TPS_REG_INT_PLUG_EVENT, "PLUG_EVENT" }, \
+ { TPS_REG_INT_PR_SWAP_COMPLETE, "PR_SWAP_COMPLETE" }, \
+ { TPS_REG_INT_DR_SWAP_COMPLETE, "DR_SWAP_COMPLETE" }, \
+ { TPS_REG_INT_RDO_RECEIVED_FROM_SINK, "RDO_RECEIVED_FROM_SINK" }, \
+ { TPS_REG_INT_BIST, "BIST" }, \
+ { TPS_REG_INT_OVERCURRENT, "OVERCURRENT" }, \
+ { TPS_REG_INT_ATTENTION_RECEIVED, "ATTENTION_RECEIVED" }, \
+ { TPS_REG_INT_VDM_RECEIVED, "VDM_RECEIVED" }, \
+ { TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER, "NEW_CONTRACT_AS_CONSUMER" }, \
+ { TPS_REG_INT_NEW_CONTRACT_AS_PROVIDER, "NEW_CONTRACT_AS_PROVIDER" }, \
+ { TPS_REG_INT_SOURCE_CAP_MESSAGE_READY, "SOURCE_CAP_MESSAGE_READY" }, \
+ { TPS_REG_INT_SINK_CAP_MESSAGE_READY, "SINK_CAP_MESSAGE_READY" }, \
+ { TPS_REG_INT_PR_SWAP_REQUESTED, "PR_SWAP_REQUESTED" }, \
+ { TPS_REG_INT_GOTO_MIN_RECEIVED, "GOTO_MIN_RECEIVED" }, \
+ { TPS_REG_INT_USB_HOST_PRESENT, "USB_HOST_PRESENT" }, \
+ { TPS_REG_INT_USB_HOST_PRESENT_NO_LONGER, "USB_HOST_PRESENT_NO_LONGER" }, \
+ { TPS_REG_INT_HIGH_VOLTAGE_WARNING, "HIGH_VOLTAGE_WARNING" }, \
+ { TPS_REG_INT_PP_SWITCH_CHANGED, "PP_SWITCH_CHANGED" }, \
+ { TPS_REG_INT_POWER_STATUS_UPDATE, "POWER_STATUS_UPDATE" }, \
+ { TPS_REG_INT_DATA_STATUS_UPDATE, "DATA_STATUS_UPDATE" }, \
+ { TPS_REG_INT_STATUS_UPDATE, "STATUS_UPDATE" }, \
+ { TPS_REG_INT_PD_STATUS_UPDATE, "PD_STATUS_UPDATE" }, \
+ { TPS_REG_INT_ADC_LOW_THRESHOLD, "ADC_LOW_THRESHOLD" }, \
+ { TPS_REG_INT_ADC_HIGH_THRESHOLD, "ADC_HIGH_THRESHOLD" }, \
+ { TPS_REG_INT_CMD1_COMPLETE, "CMD1_COMPLETE" }, \
+ { TPS_REG_INT_CMD2_COMPLETE, "CMD2_COMPLETE" }, \
+ { TPS_REG_INT_ERROR_DEVICE_INCOMPATIBLE, "ERROR_DEVICE_INCOMPATIBLE" }, \
+ { TPS_REG_INT_ERROR_CANNOT_PROVIDE_PWR, "ERROR_CANNOT_PROVIDE_PWR" }, \
+ { TPS_REG_INT_ERROR_CAN_PROVIDE_PWR_LATER, "ERROR_CAN_PROVIDE_PWR_LATER" }, \
+ { TPS_REG_INT_ERROR_POWER_EVENT_OCCURRED, "ERROR_POWER_EVENT_OCCURRED" }, \
+ { TPS_REG_INT_ERROR_MISSING_GET_CAP_MESSAGE, "ERROR_MISSING_GET_CAP_MESSAGE" }, \
+ { TPS_REG_INT_ERROR_PROTOCOL_ERROR, "ERROR_PROTOCOL_ERROR" }, \
+ { TPS_REG_INT_ERROR_MESSAGE_DATA, "ERROR_MESSAGE_DATA" }, \
+ { TPS_REG_INT_ERROR_DISCHARGE_FAILED, "ERROR_DISCHARGE_FAILED" }, \
+ { TPS_REG_INT_SRC_TRANSITION, "SRC_TRANSITION" }, \
+ { TPS_REG_INT_ERROR_UNABLE_TO_SOURCE, "ERROR_UNABLE_TO_SOURCE" }, \
+ { TPS_REG_INT_VDM_ENTERED_MODE, "VDM_ENTERED_MODE" }, \
+ { TPS_REG_INT_VDM_MSG_SENT, "VDM_MSG_SENT" }, \
+ { TPS_REG_INT_DISCOVER_MODES_COMPLETE, "DISCOVER_MODES_COMPLETE" }, \
+ { TPS_REG_INT_EXIT_MODES_COMPLETE, "EXIT_MODES_COMPLETE" }, \
+ { TPS_REG_INT_USER_VID_ALT_MODE_ENTERED, "USER_VID_ALT_MODE_ENTERED" }, \
+ { TPS_REG_INT_USER_VID_ALT_MODE_EXIT, "USER_VID_ALT_MODE_EXIT" }, \
+ { TPS_REG_INT_USER_VID_ALT_MODE_ATTN_VDM, "USER_VID_ALT_MODE_ATTN_VDM" }, \
+ { TPS_REG_INT_USER_VID_ALT_MODE_OTHER_VDM, "USER_VID_ALT_MODE_OTHER_VDM" })
+
+TRACE_EVENT(tps6598x_irq,
+ TP_PROTO(u64 event1,
+ u64 event2),
+ TP_ARGS(event1, event2),
+
+ TP_STRUCT__entry(
+ __field(u64, event1)
+ __field(u64, event2)
+ ),
+
+ TP_fast_assign(
+ __entry->event1 = event1;
+ __entry->event2 = event2;
+ ),
+
+ TP_printk("event1=%s, event2=%s",
+ show_irq_flags(__entry->event1),
+ show_irq_flags(__entry->event2))
+);
+
+#endif /* _TPS6598X_TRACE_H_ */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_FILE tps6598x_trace
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#include <trace/define_trace.h>
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-12 12:04 [PATCH v2 0/4] usb: typec: tps6598x: Add IRQ flag and register tracing Guido Günther
2021-02-12 12:04 ` [PATCH v2 1/4] usb: typec: tps6598x: Add trace event for IRQ events Guido Günther
@ 2021-02-12 12:04 ` Guido Günther
2021-02-13 3:12 ` kernel test robot
2021-02-12 12:04 ` [PATCH v2 3/4] usb: typec: tps6598x: Add trace event for power " Guido Günther
2021-02-12 12:04 ` [PATCH v2 4/4] usb: typec: tps6598x: Add trace event for data status Guido Günther
3 siblings, 1 reply; 14+ messages in thread
From: Guido Günther @ 2021-02-12 12:04 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb
This allows to trace status information which helps to debug problems
with role switching, etc.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
drivers/usb/typec/tps6598x.c | 26 ++++-----
drivers/usb/typec/tps6598x.h | 66 +++++++++++++++++++++
drivers/usb/typec/tps6598x_trace.h | 94 ++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index bc34b35e909f..559aa175f948 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -37,13 +37,6 @@
#define TPS_REG_POWER_STATUS 0x3f
#define TPS_REG_RX_IDENTITY_SOP 0x48
-/* TPS_REG_STATUS bits */
-#define TPS_STATUS_PLUG_PRESENT BIT(0)
-#define TPS_STATUS_ORIENTATION BIT(4)
-#define TPS_STATUS_PORTROLE(s) (!!((s) & BIT(5)))
-#define TPS_STATUS_DATAROLE(s) (!!((s) & BIT(6)))
-#define TPS_STATUS_VCONN(s) (!!((s) & BIT(7)))
-
/* TPS_REG_SYSTEM_CONF bits */
#define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
@@ -258,9 +251,9 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
}
typec_set_pwr_opmode(tps->port, mode);
- typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
- typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
- tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), true);
+ typec_set_pwr_role(tps->port, TPS_STATUS_TO_TYPEC_PORTROLE(status));
+ typec_set_vconn_role(tps->port, TPS_STATUS_TO_TYPEC_VCONN(status));
+ tps6598x_set_data_role(tps, TPS_STATUS_TO_TYPEC_DATAROLE(status), true);
tps->partner = typec_register_partner(tps->port, &desc);
if (IS_ERR(tps->partner))
@@ -280,9 +273,10 @@ static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
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));
- tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
+ typec_set_pwr_role(tps->port, TPS_STATUS_TO_TYPEC_PORTROLE(status));
+ typec_set_vconn_role(tps->port, TPS_STATUS_TO_TYPEC_VCONN(status));
+ tps6598x_set_data_role(tps, TPS_STATUS_TO_TYPEC_DATAROLE(status), false);
+
power_supply_changed(tps->psy);
}
@@ -366,7 +360,7 @@ static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
if (ret)
goto out_unlock;
- if (role != TPS_STATUS_DATAROLE(status)) {
+ if (role != TPS_STATUS_TO_TYPEC_DATAROLE(status)) {
ret = -EPROTO;
goto out_unlock;
}
@@ -396,7 +390,7 @@ static int tps6598x_pr_set(struct typec_port *port, enum typec_role role)
if (ret)
goto out_unlock;
- if (role != TPS_STATUS_PORTROLE(status)) {
+ if (role != TPS_STATUS_TO_TYPEC_PORTROLE(status)) {
ret = -EPROTO;
goto out_unlock;
}
@@ -437,6 +431,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
dev_err(tps->dev, "%s: failed to read status\n", __func__);
goto err_clear_ints;
}
+ trace_tps6598x_status(status);
/* Handle plug insert or removal */
if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT) {
@@ -612,6 +607,7 @@ static int tps6598x_probe(struct i2c_client *client)
ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
if (ret < 0)
return ret;
+ trace_tps6598x_status(status);
ret = tps6598x_read32(tps, TPS_REG_SYSTEM_CONF, &conf);
if (ret < 0)
diff --git a/drivers/usb/typec/tps6598x.h b/drivers/usb/typec/tps6598x.h
index b83b8a6a1504..866fd1deb471 100644
--- a/drivers/usb/typec/tps6598x.h
+++ b/drivers/usb/typec/tps6598x.h
@@ -12,6 +12,72 @@
#ifndef __TPS6598X_H__
#define __TPS6598X_H__
+/* TPS_REG_STATUS bits */
+#define TPS_STATUS_PLUG_PRESENT BIT(0)
+#define TPS_STATUS_PLUG_UPSIDE_DOWN BIT(4)
+#define TPS_STATUS_PORTROLE BIT(5)
+#define TPS_STATUS_TO_TYPEC_PORTROLE(s) (!!((s) & TPS_STATUS_PORTROLE))
+#define TPS_STATUS_DATAROLE BIT(6)
+#define TPS_STATUS_TO_TYPEC_DATAROLE(s) (!!((s) & TPS_STATUS_DATAROLE))
+#define TPS_STATUS_VCONN BIT(7)
+#define TPS_STATUS_TO_TYPEC_VCONN(s) (!!((s) & TPS_STATUS_VCONN))
+#define TPS_STATUS_OVERCURRENT BIT(16)
+#define TPS_STATUS_GOTO_MIN_ACTIVE BIT(26)
+#define TPS_STATUS_BIST BIT(27)
+#define TPS_STATUS_HIGH_VOLAGE_WARNING BIT(28)
+#define TPS_STATUS_HIGH_LOW_VOLTAGE_WARNING BIT(29)
+
+#define TPS_STATUS_CONN_STATE_MASK GENMASK(3, 1)
+#define TPS_STATUS_CONN_STATE(x) FIELD_GET(TPS_STATUS_CONN_STATE_MASK, (x))
+#define TPS_STATUS_PP_5V0_SWITCH_MASK GENMASK(9, 8)
+#define TPS_STATUS_PP_5V0_SWITCH(x) FIELD_GET(TPS_STATUS_PP_5V0_SWITCH_MASK, (x))
+#define TPS_STATUS_PP_HV_SWITCH_MASK GENMASK(11, 10)
+#define TPS_STATUS_PP_HV_SWITCH(x) FIELD_GET(TPS_STATUS_PP_HV_SWITCH_MASK, (x))
+#define TPS_STATUS_PP_EXT_SWITCH_MASK GENMASK(13, 12)
+#define TPS_STATUS_PP_EXT_SWITCH(x) FIELD_GET(TPS_STATUS_PP_EXT_SWITCH_MASK, (x))
+#define TPS_STATUS_PP_CABLE_SWITCH_MASK GENMASK(15, 14)
+#define TPS_STATUS_PP_CABLE_SWITCH(x) FIELD_GET(TPS_STATUS_PP_CABLE_SWITCH_MASK, (x))
+#define TPS_STATUS_POWER_SOURCE_MASK GENMASK(19, 18)
+#define TPS_STATUS_POWER_SOURCE(x) FIELD_GET(TPS_STATUS_POWER_SOURCE_MASK, (x))
+#define TPS_STATUS_VBUS_STATUS_MASK GENMASK(21, 20)
+#define TPS_STATUS_VBUS_STATUS(x) FIELD_GET(TPS_STATUS_VBUS_STATUS_MASK, (x))
+#define TPS_STATUS_USB_HOST_PRESENT_MASK GENMASK(23, 22)
+#define TPS_STATUS_USB_HOST_PRESENT(x) FIELD_GET(TPS_STATUS_USB_HOST_PRESENT_MASK, (x))
+#define TPS_STATUS_LEGACY_MASK GENMASK(25, 24)
+#define TPS_STATUS_LEGACY(x) FIELD_GET(TPS_STATUS_LEGACY_MASK, (x))
+
+#define TPS_STATUS_CONN_STATE_NO_CONN 0
+#define TPS_STATUS_CONN_STATE_DISABLED 1
+#define TPS_STATUS_CONN_STATE_AUDIO_CONN 2
+#define TPS_STATUS_CONN_STATE_DEBUG_CONN 3
+#define TPS_STATUS_CONN_STATE_NO_CONN_R_A 4
+#define TPS_STATUS_CONN_STATE_RESERVED 5
+#define TPS_STATUS_CONN_STATE_CONN_NO_R_A 6
+#define TPS_STATUS_CONN_STATE_CONN_WITH_R_A 7
+
+#define TPS_STATUS_PP_SWITCH_STATE_DISABLED 0
+#define TPS_STATUS_PP_SWITCH_STATE_FAULT 1
+#define TPS_STATUS_PP_SWITCH_STATE_OUT 2
+#define TPS_STATUS_PP_SWITCH_STATE_IN 3
+
+#define TPS_STATUS_POWER_SOURCE_UNKNOWN 0
+#define TPS_STATUS_POWER_SOURCE_VIN_3P3 1
+#define TPS_STATUS_POWER_SOURCE_DEAD_BAT 2
+#define TPS_STATUS_POWER_SOURCE_VBUS 3
+
+#define TPS_STATUS_VBUS_STATUS_VSAFE0V 0
+#define TPS_STATUS_VBUS_STATUS_VSAFE5V 1
+#define TPS_STATUS_VBUS_STATUS_PD 2
+#define TPS_STATUS_VBUS_STATUS_FAULT 3
+
+#define TPS_STATUS_USB_HOST_PRESENT_NO 0
+#define TPS_STATUS_USB_HOST_PRESENT_PD_NO_USB 1
+#define TPS_STATUS_USB_HOST_PRESENT_NO_PD 2
+#define TPS_STATUS_USB_HOST_PRESENT_PD_USB 3
+
+#define TPS_STATUS_LEGACY_NO 0
+#define TPS_STATUS_LEGACY_SINK 1
+#define TPS_STATUS_LEGACY_SOURCE 2
/* TPS_REG_INT_* bits */
#define TPS_REG_INT_USER_VID_ALT_MODE_OTHER_VDM BIT_ULL(27+32)
diff --git a/drivers/usb/typec/tps6598x_trace.h b/drivers/usb/typec/tps6598x_trace.h
index 4ec96e3b2c3e..e0677b9c5c53 100644
--- a/drivers/usb/typec/tps6598x_trace.h
+++ b/drivers/usb/typec/tps6598x_trace.h
@@ -67,6 +67,73 @@
{ TPS_REG_INT_USER_VID_ALT_MODE_ATTN_VDM, "USER_VID_ALT_MODE_ATTN_VDM" }, \
{ TPS_REG_INT_USER_VID_ALT_MODE_OTHER_VDM, "USER_VID_ALT_MODE_OTHER_VDM" })
+#define TPS6598X_STATUS_FLAGS_MASK (GENMASK(31, 0) ^ (TPS_STATUS_CONN_STATE_MASK | \
+ TPS_STATUS_PP_5V0_SWITCH_MASK | \
+ TPS_STATUS_PP_HV_SWITCH_MASK | \
+ TPS_STATUS_PP_EXT_SWITCH_MASK | \
+ TPS_STATUS_PP_CABLE_SWITCH_MASK | \
+ TPS_STATUS_POWER_SOURCE_MASK | \
+ TPS_STATUS_VBUS_STATUS_MASK | \
+ TPS_STATUS_USB_HOST_PRESENT_MASK | \
+ TPS_STATUS_LEGACY_MASK))
+
+#define show_status_conn_state(status) \
+ __print_symbolic(TPS_STATUS_CONN_STATE((status)), \
+ { TPS_STATUS_CONN_STATE_CONN_WITH_R_A, "conn-Ra" }, \
+ { TPS_STATUS_CONN_STATE_CONN_NO_R_A, "conn-no-Ra" }, \
+ { TPS_STATUS_CONN_STATE_NO_CONN_R_A, "no-conn-Ra" }, \
+ { TPS_STATUS_CONN_STATE_DEBUG_CONN, "debug" }, \
+ { TPS_STATUS_CONN_STATE_AUDIO_CONN, "audio" }, \
+ { TPS_STATUS_CONN_STATE_DISABLED, "disabled" }, \
+ { TPS_STATUS_CONN_STATE_NO_CONN, "no-conn" })
+
+#define show_status_pp_switch_state(status) \
+ __print_symbolic(status, \
+ { TPS_STATUS_PP_SWITCH_STATE_IN, "in" }, \
+ { TPS_STATUS_PP_SWITCH_STATE_OUT, "out" }, \
+ { TPS_STATUS_PP_SWITCH_STATE_FAULT, "fault" }, \
+ { TPS_STATUS_PP_SWITCH_STATE_DISABLED, "off" })
+
+#define show_status_power_sources(status) \
+ __print_symbolic(TPS_STATUS_POWER_SOURCE(status), \
+ { TPS_STATUS_POWER_SOURCE_VBUS, "vbus" }, \
+ { TPS_STATUS_POWER_SOURCE_VIN_3P3, "vin-3p3" }, \
+ { TPS_STATUS_POWER_SOURCE_DEAD_BAT, "dead-battery" }, \
+ { TPS_STATUS_POWER_SOURCE_UNKNOWN, "unknown" })
+
+#define show_status_vbus_status(status) \
+ __print_symbolic(TPS_STATUS_VBUS_STATUS(status), \
+ { TPS_STATUS_VBUS_STATUS_VSAFE0V, "vSafe0V" }, \
+ { TPS_STATUS_VBUS_STATUS_VSAFE5V, "vSafe5V" }, \
+ { TPS_STATUS_VBUS_STATUS_PD, "pd" }, \
+ { TPS_STATUS_VBUS_STATUS_FAULT, "fault" })
+
+#define show_status_usb_host_present(status) \
+ __print_symbolic(TPS_STATUS_USB_HOST_PRESENT(status), \
+ { TPS_STATUS_USB_HOST_PRESENT_PD_USB, "pd-usb" }, \
+ { TPS_STATUS_USB_HOST_PRESENT_NO_PD, "no-pd" }, \
+ { TPS_STATUS_USB_HOST_PRESENT_PD_NO_USB, "pd-no-usb" }, \
+ { TPS_STATUS_USB_HOST_PRESENT_NO, "no" })
+
+#define show_status_legacy(status) \
+ __print_symbolic(TPS_STATUS_LEGACY(status), \
+ { TPS_STATUS_LEGACY_SOURCE, "source" }, \
+ { TPS_STATUS_LEGACY_SINK, "sink" }, \
+ { TPS_STATUS_LEGACY_NO, "no" })
+
+#define show_status_flags(flags) \
+ __print_flags((flags & TPS6598X_STATUS_FLAGS_MASK), "|", \
+ { TPS_STATUS_PLUG_PRESENT, "PLUG_PRESENT" }, \
+ { TPS_STATUS_PLUG_UPSIDE_DOWN, "UPSIDE_DOWN" }, \
+ { TPS_STATUS_PORTROLE, "PORTROLE" }, \
+ { TPS_STATUS_DATAROLE, "DATAROLE" }, \
+ { TPS_STATUS_VCONN, "VCONN" }, \
+ { TPS_STATUS_OVERCURRENT, "OVERCURRENT" }, \
+ { TPS_STATUS_GOTO_MIN_ACTIVE, "GOTO_MIN_ACTIVE" }, \
+ { TPS_STATUS_BIST, "BIST" }, \
+ { TPS_STATUS_HIGH_VOLAGE_WARNING, "HIGH_VOLAGE_WARNING" }, \
+ { TPS_STATUS_HIGH_LOW_VOLTAGE_WARNING, "HIGH_LOW_VOLTAGE_WARNING" })
+
TRACE_EVENT(tps6598x_irq,
TP_PROTO(u64 event1,
u64 event2),
@@ -87,6 +154,33 @@ TRACE_EVENT(tps6598x_irq,
show_irq_flags(__entry->event2))
);
+TRACE_EVENT(tps6598x_status,
+ TP_PROTO(u32 status),
+ TP_ARGS(status),
+
+ TP_STRUCT__entry(
+ __field(u32, status)
+ ),
+
+ TP_fast_assign(
+ __entry->status = status;
+ ),
+
+ TP_printk("conn: %s, pp_5v0: %s, pp_hv: %s, pp_ext: %s, pp_cable: %s, "
+ "pwr-src: %s, vbus: %s, usb-host: %s, legacy: %s, flags: %s",
+ show_status_conn_state(__entry->status),
+ show_status_pp_switch_state(TPS_STATUS_PP_5V0_SWITCH(__entry->status)),
+ show_status_pp_switch_state(TPS_STATUS_PP_HV_SWITCH(__entry->status)),
+ show_status_pp_switch_state(TPS_STATUS_PP_EXT_SWITCH(__entry->status)),
+ show_status_pp_switch_state(TPS_STATUS_PP_CABLE_SWITCH(__entry->status)),
+ show_status_power_sources(__entry->status),
+ show_status_vbus_status(__entry->status),
+ show_status_usb_host_present(__entry->status),
+ show_status_legacy(__entry->status),
+ show_status_flags(__entry->status)
+ )
+);
+
#endif /* _TPS6598X_TRACE_H_ */
/* This part must be outside protection */
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-12 12:04 ` [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register Guido Günther
@ 2021-02-13 3:12 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-02-13 3:12 UTC (permalink / raw)
To: Guido Günther, Heikki Krogerus, Greg Kroah-Hartman,
linux-kernel, linux-usb
Cc: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4310 bytes --]
Hi "Guido,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.11-rc7 next-20210211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Guido-G-nther/usb-typec-tps6598x-Add-IRQ-flag-and-register-tracing/20210212-200855
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: openrisc-randconfig-s032-20210209 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-215-g0fb77bb6-dirty
# https://github.com/0day-ci/linux/commit/ba45e1d5e1fd25b6aed8724106e6c7d5adef7a20
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Guido-G-nther/usb-typec-tps6598x-Add-IRQ-flag-and-register-tracing/20210212-200855
git checkout ba45e1d5e1fd25b6aed8724106e6c7d5adef7a20
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <rong.a.chen@intel.com>
"sparse warnings: (new ones prefixed by >>)"
drivers/usb/typec/tps6598x.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, drivers/usb/typec/tps6598x_trace.h):
>> drivers/usb/typec/./tps6598x_trace.h:157:1: sparse: sparse: too long token expansion
vim +157 drivers/usb/typec/./tps6598x_trace.h
c90c0282e4ce33 Guido Günther 2021-02-12 156
ba45e1d5e1fd25 Guido Günther 2021-02-12 @157 TRACE_EVENT(tps6598x_status,
ba45e1d5e1fd25 Guido Günther 2021-02-12 158 TP_PROTO(u32 status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 159 TP_ARGS(status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 160
ba45e1d5e1fd25 Guido Günther 2021-02-12 161 TP_STRUCT__entry(
ba45e1d5e1fd25 Guido Günther 2021-02-12 162 __field(u32, status)
ba45e1d5e1fd25 Guido Günther 2021-02-12 163 ),
ba45e1d5e1fd25 Guido Günther 2021-02-12 164
ba45e1d5e1fd25 Guido Günther 2021-02-12 165 TP_fast_assign(
ba45e1d5e1fd25 Guido Günther 2021-02-12 166 __entry->status = status;
ba45e1d5e1fd25 Guido Günther 2021-02-12 167 ),
ba45e1d5e1fd25 Guido Günther 2021-02-12 168
ba45e1d5e1fd25 Guido Günther 2021-02-12 169 TP_printk("conn: %s, pp_5v0: %s, pp_hv: %s, pp_ext: %s, pp_cable: %s, "
ba45e1d5e1fd25 Guido Günther 2021-02-12 170 "pwr-src: %s, vbus: %s, usb-host: %s, legacy: %s, flags: %s",
ba45e1d5e1fd25 Guido Günther 2021-02-12 171 show_status_conn_state(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 172 show_status_pp_switch_state(TPS_STATUS_PP_5V0_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 173 show_status_pp_switch_state(TPS_STATUS_PP_HV_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 174 show_status_pp_switch_state(TPS_STATUS_PP_EXT_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 175 show_status_pp_switch_state(TPS_STATUS_PP_CABLE_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 176 show_status_power_sources(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 177 show_status_vbus_status(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 178 show_status_usb_host_present(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 179 show_status_legacy(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 180 show_status_flags(__entry->status)
ba45e1d5e1fd25 Guido Günther 2021-02-12 181 )
ba45e1d5e1fd25 Guido Günther 2021-02-12 182 );
ba45e1d5e1fd25 Guido Günther 2021-02-12 183
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35193 bytes --]
[-- Attachment #3: Type: text/plain, Size: 149 bytes --]
_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
@ 2021-02-13 3:12 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-02-13 3:12 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4538 bytes --]
Hi "Guido,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.11-rc7 next-20210211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Guido-G-nther/usb-typec-tps6598x-Add-IRQ-flag-and-register-tracing/20210212-200855
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: openrisc-randconfig-s032-20210209 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-215-g0fb77bb6-dirty
# https://github.com/0day-ci/linux/commit/ba45e1d5e1fd25b6aed8724106e6c7d5adef7a20
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Guido-G-nther/usb-typec-tps6598x-Add-IRQ-flag-and-register-tracing/20210212-200855
git checkout ba45e1d5e1fd25b6aed8724106e6c7d5adef7a20
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <rong.a.chen@intel.com>
"sparse warnings: (new ones prefixed by >>)"
drivers/usb/typec/tps6598x.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, drivers/usb/typec/tps6598x_trace.h):
>> drivers/usb/typec/./tps6598x_trace.h:157:1: sparse: sparse: too long token expansion
vim +157 drivers/usb/typec/./tps6598x_trace.h
c90c0282e4ce33 Guido Günther 2021-02-12 156
ba45e1d5e1fd25 Guido Günther 2021-02-12 @157 TRACE_EVENT(tps6598x_status,
ba45e1d5e1fd25 Guido Günther 2021-02-12 158 TP_PROTO(u32 status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 159 TP_ARGS(status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 160
ba45e1d5e1fd25 Guido Günther 2021-02-12 161 TP_STRUCT__entry(
ba45e1d5e1fd25 Guido Günther 2021-02-12 162 __field(u32, status)
ba45e1d5e1fd25 Guido Günther 2021-02-12 163 ),
ba45e1d5e1fd25 Guido Günther 2021-02-12 164
ba45e1d5e1fd25 Guido Günther 2021-02-12 165 TP_fast_assign(
ba45e1d5e1fd25 Guido Günther 2021-02-12 166 __entry->status = status;
ba45e1d5e1fd25 Guido Günther 2021-02-12 167 ),
ba45e1d5e1fd25 Guido Günther 2021-02-12 168
ba45e1d5e1fd25 Guido Günther 2021-02-12 169 TP_printk("conn: %s, pp_5v0: %s, pp_hv: %s, pp_ext: %s, pp_cable: %s, "
ba45e1d5e1fd25 Guido Günther 2021-02-12 170 "pwr-src: %s, vbus: %s, usb-host: %s, legacy: %s, flags: %s",
ba45e1d5e1fd25 Guido Günther 2021-02-12 171 show_status_conn_state(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 172 show_status_pp_switch_state(TPS_STATUS_PP_5V0_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 173 show_status_pp_switch_state(TPS_STATUS_PP_HV_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 174 show_status_pp_switch_state(TPS_STATUS_PP_EXT_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 175 show_status_pp_switch_state(TPS_STATUS_PP_CABLE_SWITCH(__entry->status)),
ba45e1d5e1fd25 Guido Günther 2021-02-12 176 show_status_power_sources(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 177 show_status_vbus_status(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 178 show_status_usb_host_present(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 179 show_status_legacy(__entry->status),
ba45e1d5e1fd25 Guido Günther 2021-02-12 180 show_status_flags(__entry->status)
ba45e1d5e1fd25 Guido Günther 2021-02-12 181 )
ba45e1d5e1fd25 Guido Günther 2021-02-12 182 );
ba45e1d5e1fd25 Guido Günther 2021-02-12 183
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35193 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-13 3:12 ` kernel test robot
(?)
@ 2021-02-14 17:06 ` Guido Günther
2021-02-14 18:31 ` Ramsay Jones
-1 siblings, 1 reply; 14+ messages in thread
From: Guido Günther @ 2021-02-14 17:06 UTC (permalink / raw)
To: kernel test robot
Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb,
kbuild-all, linux-sparse
Hi ,
On Sat, Feb 13, 2021 at 11:12:37AM +0800, kernel test robot wrote:
> Hi "Guido,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v5.11-rc7 next-20210211]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Guido-G-nther/usb-typec-tps6598x-Add-IRQ-flag-and-register-tracing/20210212-200855
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> config: openrisc-randconfig-s032-20210209 (attached as .config)
> compiler: or1k-linux-gcc (GCC) 9.3.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # apt-get install sparse
> # sparse version: v0.6.3-215-g0fb77bb6-dirty
> # https://github.com/0day-ci/linux/commit/ba45e1d5e1fd25b6aed8724106e6c7d5adef7a20
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Guido-G-nther/usb-typec-tps6598x-Add-IRQ-flag-and-register-tracing/20210212-200855
> git checkout ba45e1d5e1fd25b6aed8724106e6c7d5adef7a20
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <rong.a.chen@intel.com>
>
>
> "sparse warnings: (new ones prefixed by >>)"
> drivers/usb/typec/tps6598x.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, drivers/usb/typec/tps6598x_trace.h):
> >> drivers/usb/typec/./tps6598x_trace.h:157:1: sparse: sparse: too long token expansion
>
I looked around but didn't find any hints how to fix this. Any pointers
I missed (added the sparse list to cc:)?
Cheers,
-- Guido
> vim +157 drivers/usb/typec/./tps6598x_trace.h
>
> c90c0282e4ce33 Guido Günther 2021-02-12 156
> ba45e1d5e1fd25 Guido Günther 2021-02-12 @157 TRACE_EVENT(tps6598x_status,
> ba45e1d5e1fd25 Guido Günther 2021-02-12 158 TP_PROTO(u32 status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 159 TP_ARGS(status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 160
> ba45e1d5e1fd25 Guido Günther 2021-02-12 161 TP_STRUCT__entry(
> ba45e1d5e1fd25 Guido Günther 2021-02-12 162 __field(u32, status)
> ba45e1d5e1fd25 Guido Günther 2021-02-12 163 ),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 164
> ba45e1d5e1fd25 Guido Günther 2021-02-12 165 TP_fast_assign(
> ba45e1d5e1fd25 Guido Günther 2021-02-12 166 __entry->status = status;
> ba45e1d5e1fd25 Guido Günther 2021-02-12 167 ),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 168
> ba45e1d5e1fd25 Guido Günther 2021-02-12 169 TP_printk("conn: %s, pp_5v0: %s, pp_hv: %s, pp_ext: %s, pp_cable: %s, "
> ba45e1d5e1fd25 Guido Günther 2021-02-12 170 "pwr-src: %s, vbus: %s, usb-host: %s, legacy: %s, flags: %s",
> ba45e1d5e1fd25 Guido Günther 2021-02-12 171 show_status_conn_state(__entry->status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 172 show_status_pp_switch_state(TPS_STATUS_PP_5V0_SWITCH(__entry->status)),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 173 show_status_pp_switch_state(TPS_STATUS_PP_HV_SWITCH(__entry->status)),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 174 show_status_pp_switch_state(TPS_STATUS_PP_EXT_SWITCH(__entry->status)),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 175 show_status_pp_switch_state(TPS_STATUS_PP_CABLE_SWITCH(__entry->status)),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 176 show_status_power_sources(__entry->status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 177 show_status_vbus_status(__entry->status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 178 show_status_usb_host_present(__entry->status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 179 show_status_legacy(__entry->status),
> ba45e1d5e1fd25 Guido Günther 2021-02-12 180 show_status_flags(__entry->status)
> ba45e1d5e1fd25 Guido Günther 2021-02-12 181 )
> ba45e1d5e1fd25 Guido Günther 2021-02-12 182 );
> ba45e1d5e1fd25 Guido Günther 2021-02-12 183
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> _______________________________________________
> kbuild mailing list -- kbuild@lists.01.org
> To unsubscribe send an email to kbuild-leave@lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-14 17:06 ` Guido Günther
@ 2021-02-14 18:31 ` Ramsay Jones
2021-02-14 19:00 ` Linus Torvalds
0 siblings, 1 reply; 14+ messages in thread
From: Ramsay Jones @ 2021-02-14 18:31 UTC (permalink / raw)
To: Guido Günther; +Cc: Luc Van Oostenryck, Sparse Mailing-list
[trimmed CC: list]
On 14/02/2021 17:06, Guido Günther wrote:
[snip]
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <rong.a.chen@intel.com>
>>
>>
>> "sparse warnings: (new ones prefixed by >>)"
>> drivers/usb/typec/tps6598x.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, drivers/usb/typec/tps6598x_trace.h):
>>>> drivers/usb/typec/./tps6598x_trace.h:157:1: sparse: sparse: too long token expansion
>>
>
> I looked around but didn't find any hints how to fix this. Any pointers
> I missed (added the sparse list to cc:)?
This is a limitation of sparse; when using the 'stringize' pre-processor
operator #, the maximum size of the resulting string is about 8k (if I
remember correctly).
It just so happens that I have a WIP patch from 2011 hanging around to
fix this - I didn't submit the patch, since it was easier to change the
original source (so that it didn't need such a huge string!). ;-)
Also, it was a very 'quick-n-dirty' patch which could probably be somewhat
improved (and may not even apply to the current codebase*).
ATB,
Ramsay Jones
*) I just tried to apply the patch but, as expected, there are conflicts.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-14 18:31 ` Ramsay Jones
@ 2021-02-14 19:00 ` Linus Torvalds
2021-02-14 19:07 ` Linus Torvalds
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Linus Torvalds @ 2021-02-14 19:00 UTC (permalink / raw)
To: Ramsay Jones, Greg Kroah-Hartman, Linux Kernel Mailing List,
linux-usb
Cc: Guido Günther, Luc Van Oostenryck, Sparse Mailing-list
On Sun, Feb 14, 2021 at 10:42 AM Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>
> >
> > I looked around but didn't find any hints how to fix this. Any pointers
> > I missed (added the sparse list to cc:)?
>
> This is a limitation of sparse; when using the 'stringize' pre-processor
> operator #, the maximum size of the resulting string is about 8k (if I
> remember correctly).
Well, yes and no.
The C89 standard actually says that a string literal can be at most
509 characters to be portable. C99 increased it to 4095 characters.
Sparse makes the limit higher, and the limit could easily be expanded
way past 8kB - but the point is that large string literals are
actually not guaranteed to be valid C.
So honestly, it really sounds like that TRACE_EVENT() thing is doing
something it shouldn't be doing.
I don't think there's any fundamental limit why sparse does 8kB as a
limit (just a few random buffers). Making sparse accept larger ones
should be as simple as just increasing MAX_STRING, but I really don't
think the kernel should encourage that kind of excessive string sizes.
I wouldn't be surprised if tracing buffers etc make such strings useless anyway.
Linus
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-14 19:00 ` Linus Torvalds
@ 2021-02-14 19:07 ` Linus Torvalds
2021-02-14 19:31 ` Ramsay Jones
2021-02-14 20:41 ` Luc Van Oostenryck
2 siblings, 0 replies; 14+ messages in thread
From: Linus Torvalds @ 2021-02-14 19:07 UTC (permalink / raw)
To: Ramsay Jones, Greg Kroah-Hartman, Linux Kernel Mailing List,
linux-usb
Cc: Guido Günther, Luc Van Oostenryck, Sparse Mailing-list
On Sun, Feb 14, 2021 at 11:00 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The C89 standard actually says that a string literal can be at most
> 509 characters to be portable. C99 increased it to 4095 characters.
>
> Sparse makes the limit higher, and the limit could easily be expanded
> way past 8kB - but the point is that large string literals are
> actually not guaranteed to be valid C.
Looking around, there's a couple of other similar cases:
drivers/infiniband/hw/hfi1/./trace_tx.h:727:1: error: too long token expansion
arch/x86/purgatory/kexec-purgatory.c:1340:9: warning: trying to
concatenate 21400-character string (8191 bytes max)
drivers/scsi/constants.c:318:9: warning: trying to concatenate
26550-character string (8191 bytes max)
kernel/trace/trace.c:5290:1: warning: trying to concatenate
10842-character string (8191 bytes max)
but those four are the only ones I see from a quick x86-64 allmodconfig build.
Please try to avoid it.
Linus
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-14 19:00 ` Linus Torvalds
2021-02-14 19:07 ` Linus Torvalds
@ 2021-02-14 19:31 ` Ramsay Jones
2021-02-14 20:41 ` Luc Van Oostenryck
2 siblings, 0 replies; 14+ messages in thread
From: Ramsay Jones @ 2021-02-14 19:31 UTC (permalink / raw)
To: Linus Torvalds, Greg Kroah-Hartman, Linux Kernel Mailing List,
linux-usb
Cc: Guido Günther, Luc Van Oostenryck, Sparse Mailing-list
On 14/02/2021 19:00, Linus Torvalds wrote:
> On Sun, Feb 14, 2021 at 10:42 AM Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>>
>>>
>>> I looked around but didn't find any hints how to fix this. Any pointers
>>> I missed (added the sparse list to cc:)?
>>
>> This is a limitation of sparse; when using the 'stringize' pre-processor
>> operator #, the maximum size of the resulting string is about 8k (if I
>> remember correctly).
>
> Well, yes and no.
>
> The C89 standard actually says that a string literal can be at most
> 509 characters to be portable. C99 increased it to 4095 characters.
>
> Sparse makes the limit higher, and the limit could easily be expanded
> way past 8kB - but the point is that large string literals are
> actually not guaranteed to be valid C.
>
> So honestly, it really sounds like that TRACE_EVENT() thing is doing
> something it shouldn't be doing.
Yep, as I said, I didn't submit the patch - rather I changed the source
so as not to need such a long string.
> I don't think there's any fundamental limit why sparse does 8kB as a
> limit (just a few random buffers). Making sparse accept larger ones
> should be as simple as just increasing MAX_STRING, but I really don't
> think the kernel should encourage that kind of excessive string sizes.
I agree, but I wiggled my patch (which doesn't increase MAX_STRING) to
apply to the current codebase, and ... it now fails two tests! ;-)
(It seems, in the intervening 9 years, the show_token_sequence() function
fixed the quoting of double-quotes in the resulting strings, which
my patch fails to do).
Sorry for the noise.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-14 19:00 ` Linus Torvalds
2021-02-14 19:07 ` Linus Torvalds
2021-02-14 19:31 ` Ramsay Jones
@ 2021-02-14 20:41 ` Luc Van Oostenryck
2021-02-15 11:32 ` Guido Günther
2 siblings, 1 reply; 14+ messages in thread
From: Luc Van Oostenryck @ 2021-02-14 20:41 UTC (permalink / raw)
To: Linus Torvalds
Cc: Ramsay Jones, Greg Kroah-Hartman, Linux Kernel Mailing List,
linux-usb, Guido Günther, Sparse Mailing-list
On Sun, Feb 14, 2021 at 11:00:48AM -0800, Linus Torvalds wrote:
> On Sun, Feb 14, 2021 at 10:42 AM Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
> >
> > >
> > > I looked around but didn't find any hints how to fix this. Any pointers
> > > I missed (added the sparse list to cc:)?
> >
> > This is a limitation of sparse; when using the 'stringize' pre-processor
> > operator #, the maximum size of the resulting string is about 8k (if I
> > remember correctly).
>
> Well, yes and no.
>
> The C89 standard actually says that a string literal can be at most
> 509 characters to be portable. C99 increased it to 4095 characters.
>
> Sparse makes the limit higher, and the limit could easily be expanded
> way past 8kB - but the point is that large string literals are
> actually not guaranteed to be valid C.
>
> So honestly, it really sounds like that TRACE_EVENT() thing is doing
> something it shouldn't be doing.
In itself, it's OKish but it does a lot of macro expansions and most
arguments are macros of macros of ... but the problem seems to be
limited to TP_printk().
In the current case, the offender is the string 'print_fmt_tps6598x_status'
which is just under 26K long especially because it expand
TPS6598X_STATUS_FLAGS_MASK but also because the arguments use FIELD_GET()
and thus __BF_FIELD_CHECK().
>
> I don't think there's any fundamental limit why sparse does 8kB as a
> limit (just a few random buffers). Making sparse accept larger ones
> should be as simple as just increasing MAX_STRING, but I really don't
> think the kernel should encourage that kind of excessive string sizes.
Like you noted, there are just a few cases in the kernel and IIRC
there is or was one case in it too.
I would tend to increase MAX_STRING to something like 32 or 64K,
in order to keep it reasonable but let sparse to continue its processing,
but add a warning when the string/token is bigger than the current 8K.
-- Luc
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register
2021-02-14 20:41 ` Luc Van Oostenryck
@ 2021-02-15 11:32 ` Guido Günther
0 siblings, 0 replies; 14+ messages in thread
From: Guido Günther @ 2021-02-15 11:32 UTC (permalink / raw)
To: Luc Van Oostenryck
Cc: Linus Torvalds, Ramsay Jones, Greg Kroah-Hartman,
Linux Kernel Mailing List, linux-usb, Sparse Mailing-list
Hi,
On Sun, Feb 14, 2021 at 09:41:27PM +0100, Luc Van Oostenryck wrote:
> On Sun, Feb 14, 2021 at 11:00:48AM -0800, Linus Torvalds wrote:
> > On Sun, Feb 14, 2021 at 10:42 AM Ramsay Jones
> > <ramsay@ramsayjones.plus.com> wrote:
> > >
> > > >
> > > > I looked around but didn't find any hints how to fix this. Any pointers
> > > > I missed (added the sparse list to cc:)?
> > >
> > > This is a limitation of sparse; when using the 'stringize' pre-processor
> > > operator #, the maximum size of the resulting string is about 8k (if I
> > > remember correctly).
> >
> > Well, yes and no.
> >
> > The C89 standard actually says that a string literal can be at most
> > 509 characters to be portable. C99 increased it to 4095 characters.
> >
> > Sparse makes the limit higher, and the limit could easily be expanded
> > way past 8kB - but the point is that large string literals are
> > actually not guaranteed to be valid C.
> >
> > So honestly, it really sounds like that TRACE_EVENT() thing is doing
> > something it shouldn't be doing.
>
> In itself, it's OKish but it does a lot of macro expansions and most
> arguments are macros of macros of ... but the problem seems to be
> limited to TP_printk().
>
> In the current case, the offender is the string 'print_fmt_tps6598x_status'
> which is just under 26K long especially because it expand
> TPS6598X_STATUS_FLAGS_MASK but also because the arguments use FIELD_GET()
> and thus __BF_FIELD_CHECK().
That was a great hint! Using a custom FIELD_GET() that drops the
__BF_FIELD_CHECK() makes things fit.
Cheers,
-- Guido
> >
> > I don't think there's any fundamental limit why sparse does 8kB as a
> > limit (just a few random buffers). Making sparse accept larger ones
> > should be as simple as just increasing MAX_STRING, but I really don't
> > think the kernel should encourage that kind of excessive string sizes.
>
> Like you noted, there are just a few cases in the kernel and IIRC
> there is or was one case in it too.
> I would tend to increase MAX_STRING to something like 32 or 64K,
> in order to keep it reasonable but let sparse to continue its processing,
> but add a warning when the string/token is bigger than the current 8K.
>
> -- Luc
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] usb: typec: tps6598x: Add trace event for power status register
2021-02-12 12:04 [PATCH v2 0/4] usb: typec: tps6598x: Add IRQ flag and register tracing Guido Günther
2021-02-12 12:04 ` [PATCH v2 1/4] usb: typec: tps6598x: Add trace event for IRQ events Guido Günther
2021-02-12 12:04 ` [PATCH v2 2/4] usb: typec: tps6598x: Add trace event for status register Guido Günther
@ 2021-02-12 12:04 ` Guido Günther
2021-02-12 12:04 ` [PATCH v2 4/4] usb: typec: tps6598x: Add trace event for data status Guido Günther
3 siblings, 0 replies; 14+ messages in thread
From: Guido Günther @ 2021-02-12 12:04 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb
Together with the PD status register this is vital for debugging power
negotiations at runtime.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
drivers/usb/typec/tps6598x.c | 19 +++++++++------
drivers/usb/typec/tps6598x.h | 19 +++++++++++++++
drivers/usb/typec/tps6598x_trace.h | 38 ++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 559aa175f948..3e6ad3ba7fc8 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -50,11 +50,6 @@ enum {
TPS_PORTINFO_SOURCE,
};
-/* TPS_REG_POWER_STATUS bits */
-#define TPS_POWER_STATUS_CONNECTION BIT(0)
-#define TPS_POWER_STATUS_SOURCESINK BIT(1)
-#define TPS_POWER_STATUS_PWROPMODE(p) (((p) & GENMASK(3, 2)) >> 2)
-
/* TPS_REG_RX_IDENTITY_SOP */
struct tps6598x_rx_identity_reg {
u8 status;
@@ -414,6 +409,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
u64 event1;
u64 event2;
u32 status;
+ u16 pwr_status;
int ret;
mutex_lock(&tps->lock);
@@ -433,6 +429,15 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
}
trace_tps6598x_status(status);
+ if ((event1 | event2) & TPS_REG_INT_POWER_STATUS_UPDATE) {
+ ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
+ if (ret < 0) {
+ dev_err(tps->dev, "failed to read power status: %d\n", ret);
+ goto err_clear_ints;
+ }
+ trace_tps6598x_power_status(pwr_status);
+ }
+
/* Handle plug insert or removal */
if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT) {
if (status & TPS_STATUS_PLUG_PRESENT) {
@@ -497,8 +502,8 @@ static int tps6598x_psy_get_online(struct tps6598x *tps,
if (ret < 0)
return ret;
- if ((pwr_status & TPS_POWER_STATUS_CONNECTION) &&
- (pwr_status & TPS_POWER_STATUS_SOURCESINK)) {
+ if (TPS_POWER_STATUS_CONNECTION(pwr_status) &&
+ TPS_POWER_STATUS_SOURCESINK(pwr_status)) {
val->intval = 1;
} else {
val->intval = 0;
diff --git a/drivers/usb/typec/tps6598x.h b/drivers/usb/typec/tps6598x.h
index 866fd1deb471..3f6503377678 100644
--- a/drivers/usb/typec/tps6598x.h
+++ b/drivers/usb/typec/tps6598x.h
@@ -127,4 +127,23 @@
#define TPS_REG_INT_HARD_RESET BIT(1)
#define TPS_REG_INT_PD_SOFT_RESET BIT(0)
+/* TPS_REG_POWER_STATUS bits */
+#define TPS_POWER_STATUS_CONNECTION(x) FIELD_GET(BIT(0), (x))
+#define TPS_POWER_STATUS_SOURCESINK(x) FIELD_GET(BIT(1), (x))
+#define TPS_POWER_STATUS_BC12_DET(x) FIELD_GET(BIT(2), (x))
+
+#define TPS_POWER_STATUS_TYPEC_CURRENT_MASK GENMASK(3, 2)
+#define TPS_POWER_STATUS_PWROPMODE(p) FIELD_GET(TPS_POWER_STATUS_TYPEC_CURRENT_MASK, (p))
+#define TPS_POWER_STATUS_BC12_STATUS_MASK GENMASK(6, 5)
+#define TPS_POWER_STATUS_BC12_STATUS(p) FIELD_GET(TPS_POWER_STATUS_BC12_STATUS_MASK, (p))
+
+#define TPS_POWER_STATUS_TYPEC_CURRENT_USB 0
+#define TPS_POWER_STATUS_TYPEC_CURRENT_1A5 1
+#define TPS_POWER_STATUS_TYPEC_CURRENT_3A0 2
+#define TPS_POWER_STATUS_TYPEC_CURRENT_PD 3
+
+#define TPS_POWER_STATUS_BC12_STATUS_SDP 0
+#define TPS_POWER_STATUS_BC12_STATUS_CDP 2
+#define TPS_POWER_STATUS_BC12_STATUS_DCP 3
+
#endif /* __TPS6598X_H__ */
diff --git a/drivers/usb/typec/tps6598x_trace.h b/drivers/usb/typec/tps6598x_trace.h
index e0677b9c5c53..78a5a6ca337b 100644
--- a/drivers/usb/typec/tps6598x_trace.h
+++ b/drivers/usb/typec/tps6598x_trace.h
@@ -134,6 +134,24 @@
{ TPS_STATUS_HIGH_VOLAGE_WARNING, "HIGH_VOLAGE_WARNING" }, \
{ TPS_STATUS_HIGH_LOW_VOLTAGE_WARNING, "HIGH_LOW_VOLTAGE_WARNING" })
+#define show_power_status_source_sink(power_status) \
+ __print_symbolic(TPS_POWER_STATUS_SOURCESINK(power_status), \
+ { 1, "sink" }, \
+ { 0, "source" })
+
+#define show_power_status_typec_status(power_status) \
+ __print_symbolic(TPS_POWER_STATUS_PWROPMODE(power_status), \
+ { TPS_POWER_STATUS_TYPEC_CURRENT_PD, "pd" }, \
+ { TPS_POWER_STATUS_TYPEC_CURRENT_3A0, "3.0A" }, \
+ { TPS_POWER_STATUS_TYPEC_CURRENT_1A5, "1.5A" }, \
+ { TPS_POWER_STATUS_TYPEC_CURRENT_USB, "usb" })
+
+#define show_power_status_bc12_status(power_status) \
+ __print_symbolic(TPS_POWER_STATUS_BC12_STATUS(power_status), \
+ { TPS_POWER_STATUS_BC12_STATUS_DCP, "dcp" }, \
+ { TPS_POWER_STATUS_BC12_STATUS_CDP, "cdp" }, \
+ { TPS_POWER_STATUS_BC12_STATUS_SDP, "sdp" })
+
TRACE_EVENT(tps6598x_irq,
TP_PROTO(u64 event1,
u64 event2),
@@ -181,6 +199,26 @@ TRACE_EVENT(tps6598x_status,
)
);
+TRACE_EVENT(tps6598x_power_status,
+ TP_PROTO(u16 power_status),
+ TP_ARGS(power_status),
+
+ TP_STRUCT__entry(
+ __field(u16, power_status)
+ ),
+
+ TP_fast_assign(
+ __entry->power_status = power_status;
+ ),
+
+ TP_printk("conn: %d, pwr-role: %s, typec: %s, bc: %s",
+ !!TPS_POWER_STATUS_CONNECTION(__entry->power_status),
+ show_power_status_source_sink(__entry->power_status),
+ show_power_status_typec_status(__entry->power_status),
+ show_power_status_bc12_status(__entry->power_status)
+ )
+);
+
#endif /* _TPS6598X_TRACE_H_ */
/* This part must be outside protection */
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 4/4] usb: typec: tps6598x: Add trace event for data status
2021-02-12 12:04 [PATCH v2 0/4] usb: typec: tps6598x: Add IRQ flag and register tracing Guido Günther
` (2 preceding siblings ...)
2021-02-12 12:04 ` [PATCH v2 3/4] usb: typec: tps6598x: Add trace event for power " Guido Günther
@ 2021-02-12 12:04 ` Guido Günther
3 siblings, 0 replies; 14+ messages in thread
From: Guido Günther @ 2021-02-12 12:04 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman, linux-kernel, linux-usb
This is useful to debug DP negotiation and pin assignment even
when the firmware does all the work.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
drivers/usb/typec/tps6598x.c | 12 ++++++-
drivers/usb/typec/tps6598x.h | 36 ++++++++++++++++++++
drivers/usb/typec/tps6598x_trace.h | 54 ++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 3e6ad3ba7fc8..a4ec8e56c2b9 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -36,6 +36,7 @@
#define TPS_REG_CTRL_CONF 0x29
#define TPS_REG_POWER_STATUS 0x3f
#define TPS_REG_RX_IDENTITY_SOP 0x48
+#define TPS_REG_DATA_STATUS 0x5f
/* TPS_REG_SYSTEM_CONF bits */
#define TPS_SYSCONF_PORTINFO(c) ((c) & 7)
@@ -408,7 +409,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
struct tps6598x *tps = data;
u64 event1;
u64 event2;
- u32 status;
+ u32 status, data_status;
u16 pwr_status;
int ret;
@@ -438,6 +439,15 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
trace_tps6598x_power_status(pwr_status);
}
+ if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE) {
+ ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &data_status);
+ if (ret < 0) {
+ dev_err(tps->dev, "failed to read data status: %d\n", ret);
+ goto err_clear_ints;
+ }
+ trace_tps6598x_data_status(data_status);
+ }
+
/* Handle plug insert or removal */
if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT) {
if (status & TPS_STATUS_PLUG_PRESENT) {
diff --git a/drivers/usb/typec/tps6598x.h b/drivers/usb/typec/tps6598x.h
index 3f6503377678..1afc22ab4dbb 100644
--- a/drivers/usb/typec/tps6598x.h
+++ b/drivers/usb/typec/tps6598x.h
@@ -146,4 +146,40 @@
#define TPS_POWER_STATUS_BC12_STATUS_CDP 2
#define TPS_POWER_STATUS_BC12_STATUS_DCP 3
+/* TPS_REG_DATA_STATUS bits */
+#define TPS_DATA_STATUS_DATA_CONNECTION BIT(0)
+#define TPS_DATA_STATUS_UPSIDE_DOWN BIT(1)
+#define TPS_DATA_STATUS_ACTIVE_CABLE BIT(2)
+#define TPS_DATA_STATUS_USB2_CONNECTION BIT(4)
+#define TPS_DATA_STATUS_USB3_CONNECTION BIT(5)
+#define TPS_DATA_STATUS_USB3_GEN2 BIT(6)
+#define TPS_DATA_STATUS_USB_DATA_ROLE BIT(7)
+#define TPS_DATA_STATUS_DP_CONNECTION BIT(8)
+#define TPS_DATA_STATUS_DP_SINK BIT(9)
+#define TPS_DATA_STATUS_TBT_CONNECTION BIT(16)
+#define TPS_DATA_STATUS_TBT_TYPE BIT(17)
+#define TPS_DATA_STATUS_OPTICAL_CABLE BIT(18)
+#define TPS_DATA_STATUS_ACTIVE_LINK_TRAIN BIT(20)
+#define TPS_DATA_STATUS_FORCE_LSX BIT(23)
+#define TPS_DATA_STATUS_POWER_MISMATCH BIT(24)
+
+#define TPS_DATA_STATUS_DP_PIN_ASSIGNMENT_MASK GENMASK(11, 10)
+#define TPS_DATA_STATUS_DP_PIN_ASSIGNMENT(x) \
+ FIELD_GET(TPS_DATA_STATUS_DP_PIN_ASSIGNMENT_MASK, (x))
+#define TPS_DATA_STATUS_TBT_CABLE_SPEED_MASK GENMASK(27, 25)
+#define TPS_DATA_STATUS_TBT_CABLE_SPEED FIELD_GET(TPS_DATA_STATUS_TBT_CABLE_SPEED_MASK, (x))
+#define TPS_DATA_STATUS_TBT_CABLE_GEN_MASK GENMASK(29, 28)
+#define TPS_DATA_STATUS_TBT_CABLE_GEN FIELD_GET(TPS_DATA_STATUS_TBT_CABLE_GEN_MASK, (x))
+
+/* Map data status to DP spec assignments */
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT(x) \
+ ((TPS_DATA_STATUS_DP_PIN_ASSIGNMENT(x) << 1) | \
+ FIELD_GET(TPS_DATA_STATUS_USB3_CONNECTION, (x)))
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_E 0
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_F BIT(0)
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_C BIT(1)
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_D (BIT(1) | BIT(0))
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_A BIT(2)
+#define TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B (BIT(2) | BIT(1))
+
#endif /* __TPS6598X_H__ */
diff --git a/drivers/usb/typec/tps6598x_trace.h b/drivers/usb/typec/tps6598x_trace.h
index 78a5a6ca337b..38bfb2f04e46 100644
--- a/drivers/usb/typec/tps6598x_trace.h
+++ b/drivers/usb/typec/tps6598x_trace.h
@@ -152,6 +152,41 @@
{ TPS_POWER_STATUS_BC12_STATUS_CDP, "cdp" }, \
{ TPS_POWER_STATUS_BC12_STATUS_SDP, "sdp" })
+#define TPS_DATA_STATUS_FLAGS_MASK (GENMASK(31, 0) ^ (TPS_DATA_STATUS_DP_PIN_ASSIGNMENT_MASK | \
+ TPS_DATA_STATUS_TBT_CABLE_SPEED_MASK | \
+ TPS_DATA_STATUS_TBT_CABLE_GEN_MASK))
+
+#define show_data_status_flags(data_status) \
+ __print_flags(data_status & TPS_DATA_STATUS_FLAGS_MASK, "|", \
+ { TPS_DATA_STATUS_DATA_CONNECTION, "DATA_CONNECTION" }, \
+ { TPS_DATA_STATUS_UPSIDE_DOWN, "DATA_UPSIDE_DOWN" }, \
+ { TPS_DATA_STATUS_ACTIVE_CABLE, "ACTIVE_CABLE" }, \
+ { TPS_DATA_STATUS_USB2_CONNECTION, "USB2_CONNECTION" }, \
+ { TPS_DATA_STATUS_USB3_CONNECTION, "USB3_CONNECTION" }, \
+ { TPS_DATA_STATUS_USB3_GEN2, "USB3_GEN2" }, \
+ { TPS_DATA_STATUS_USB_DATA_ROLE, "USB_DATA_ROLE" }, \
+ { TPS_DATA_STATUS_DP_CONNECTION, "DP_CONNECTION" }, \
+ { TPS_DATA_STATUS_DP_SINK, "DP_SINK" }, \
+ { TPS_DATA_STATUS_TBT_CONNECTION, "TBT_CONNECTION" }, \
+ { TPS_DATA_STATUS_TBT_TYPE, "TBT_TYPE" }, \
+ { TPS_DATA_STATUS_OPTICAL_CABLE, "OPTICAL_CABLE" }, \
+ { TPS_DATA_STATUS_ACTIVE_LINK_TRAIN, "ACTIVE_LINK_TRAIN" }, \
+ { TPS_DATA_STATUS_FORCE_LSX, "FORCE_LSX" }, \
+ { TPS_DATA_STATUS_POWER_MISMATCH, "POWER_MISMATCH" })
+
+#define show_data_status_dp_pin_assignment(data_status) \
+ __print_symbolic(TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT(data_status), \
+ { TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_E, "E" }, \
+ { TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_F, "F" }, \
+ { TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_C, "C" }, \
+ { TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_D, "D" }, \
+ { TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_A, "A" }, \
+ { TPS_DATA_STATUS_DP_SPEC_PIN_ASSIGNMENT_B, "B" })
+
+#define maybe_show_data_status_dp_pin_assignment(data_status) \
+ (data_status & TPS_DATA_STATUS_DP_CONNECTION ? \
+ show_data_status_dp_pin_assignment(data_status) : "")
+
TRACE_EVENT(tps6598x_irq,
TP_PROTO(u64 event1,
u64 event2),
@@ -219,6 +254,25 @@ TRACE_EVENT(tps6598x_power_status,
)
);
+TRACE_EVENT(tps6598x_data_status,
+ TP_PROTO(u32 data_status),
+ TP_ARGS(data_status),
+
+ TP_STRUCT__entry(
+ __field(u32, data_status)
+ ),
+
+ TP_fast_assign(
+ __entry->data_status = data_status;
+ ),
+
+ TP_printk("%s%s%s",
+ show_data_status_flags(__entry->data_status),
+ __entry->data_status & TPS_DATA_STATUS_DP_CONNECTION ? ", DP pinout " : "",
+ maybe_show_data_status_dp_pin_assignment(__entry->data_status)
+ )
+);
+
#endif /* _TPS6598X_TRACE_H_ */
/* This part must be outside protection */
--
2.30.0
^ permalink raw reply related [flat|nested] 14+ messages in thread