usb: typec: tps6598x: Add IRQ flag and register tracing
Subsystems (2)
v3 cover usb: typec: tps6598x: Add IRQ flag and register tracing
This series adds tracing events for the chips IRQ and registers that are useful to figure out the current data and power status. This came about since diagnosing why a certain usb-c hub or dp-alt-mode adapter fails is hard with the information in /sys/class/typec alone since this does not have a timeline of events (and we don't want every typec user having to also buy a PD analyzer). With this series debugging these kinds of things starts to become fun:
# echo 1 > /sys/kernel/debug/tracing/events/tps6598x/enable # cat /sys/kernel/debug/tracing/trace_pipe irq/79-0-003f-526 [003] .... 512.717871: tps6598x_irq: event1=PLUG_EVENT|DATA_STATUS_UPDATE|STATUS_UPDATE, event2= irq/79-0-003f-526 [003] .... 512.722408: tps6598x_status: conn: conn-Ra, pp_5v0: off, pp_hv: off, pp_ext: off, pp_cable: off, pwr-src: vin-3p3, vbus: vSafe0V, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE irq/79-0-003f-526 [003] .... 512.727127: tps6598x_data_status: DATA_CONNECTION|USB2_CONNECTION|USB3_CONNECTION irq/79-0-003f-526 [003] .... 512.769571: tps6598x_irq: event1=PP_SWITCH_CHANGED|STATUS_UPDATE, event2= irq/79-0-003f-526 [003] .... 512.773380: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: vSafe0V, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 512.872450: tps6598x_irq: event1=POWER_STATUS_UPDATE|PD_STATUS_UPDATE, event2= irq/79-0-003f-526 [003] .... 512.876311: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: vSafe0V, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 512.880237: tps6598x_power_status: conn: 1, pwr-role: source, typec: usb, bc: sdp irq/79-0-003f-526 [003] .... 513.072682: tps6598x_irq: event1=STATUS_UPDATE, event2= irq/79-0-003f-526 [003] .... 513.076390: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: vSafe5V, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 513.090676: tps6598x_irq: event1=ERROR_CANNOT_PROVIDE_PWR, event2= irq/79-0-003f-526 [003] .... 513.094368: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: vSafe5V, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 513.109606: tps6598x_irq: event1=NEW_CONTRACT_AS_PROVIDER|POWER_STATUS_UPDATE|STATUS_UPDATE|SRC_TRANSITION, event2= irq/79-0-003f-526 [003] .... 513.113777: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: pd, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 513.117475: tps6598x_power_status: conn: 1, pwr-role: source, typec: pd, bc: sdp irq/79-0-003f-526 [003] .... 513.137469: tps6598x_irq: event1=VDM_RECEIVED, event2= irq/79-0-003f-526 [003] .... 513.141570: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: pd, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 513.281926: tps6598x_irq: event1=VDM_RECEIVED, event2= irq/79-0-003f-526 [003] .... 513.285638: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: pd, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 513.300515: tps6598x_irq: event1=VDM_RECEIVED|DATA_STATUS_UPDATE, event2= irq/79-0-003f-526 [003] .... 513.304226: tps6598x_status: conn: conn-Ra, pp_5v0: out, pp_hv: off, pp_ext: off, pp_cable: in, pwr-src: vin-3p3, vbus: pd, usb-host: no, legacy: no, flags: PLUG_PRESENT|PORTROLE|DATAROLE|VCONN irq/79-0-003f-526 [003] .... 513.308302: tps6598x_data_status: DATA_CONNECTION|USB2_CONNECTION|USB3_CONNECTION|DP_CONNECTION, DP pinout D
It should not impose any problems for firmwares that don't have IRQs for the registers enabled. The trace will then just be missing those events.
Patch is against next-20210210.
changes from v1:
- Fix issues on 32bit arches and missing include spotted by kbuild (build testes on mips)
https://lore.kernel.org/linux-usb/[email protected]/
https://lore.kernel.org/linux-usb/[email protected]/
changes from v2:
- Reduce macro expansion size by using a custom FIELD_GET. This avoids a sparse warning.
https://lore.kernel.org/linux-usb/20210213031237.GP219708@shao2-debian/
Guido Günther (4):
usb: typec: tps6598x: Add trace event for IRQ events
usb: typec: tps6598x: Add trace event for status register
usb: typec: tps6598x: Add trace event for power status register
usb: typec: tps6598x: Add trace event for data status
drivers/usb/typec/Makefile | 3 + drivers/usb/typec/tps6598x.c | 66 ++++--- drivers/usb/typec/tps6598x.h | 189 +++++++++++++++++++ drivers/usb/typec/tps6598x_trace.h | 283 +++++++++++++++++++++++++++++ 4 files changed, 515 insertions(+), 26 deletions(-) create mode 100644 drivers/usb/typec/tps6598x.h create mode 100644 drivers/usb/typec/tps6598x_trace.h
Signature · 1 line
2.30.0
PATCH v3 1/4 usb: typec: tps6598x: Add trace event for IRQ events
Allow to get irq event information via the tracing framework. This allows to inspect USB-C negotiation at runtime.
--- 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
drivers/usb/typec/Makefile +3 -0
@@ -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/
drivers/usb/typec/tps6598x.c +6 -3
@@ -6,6 +6,8 @@* Author: Heikki Krogerus <[email protected]> */ +#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) {
drivers/usb/typec/tps6598x.h new file +64 -0
64 lines not on this page · raw message
drivers/usb/typec/tps6598x_trace.h new file +97 -0
97 lines not on this page · raw message
Signature · 1 line
2.30.0
Re: [PATCH v3 1/4] usb: typec: tps6598x: Add trace event for IRQ events
Quoted 4 lines · Guido Günther
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 <[email protected]>
Quoted 237 lines
--- 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.hdrivers/usb/typec/Makefile +3
-0@@ -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/drivers/usb/typec/tps6598x.c +6
-3@@ -6,6 +6,8 @@* Author: Heikki Krogerus <[email protected]> */ +#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) {drivers/usb/typec/tps6598x.h new file +64
-0@@ -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 <[email protected]> +*/ + +#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__ */ drivers/usb/typec/tps6598x_trace.h new file +97
-0@@ -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 <[email protected]> +*/ + +#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> Signature · 1 line
2.30.0
thanks,
Signature · 1 line
heikki
PATCH v3 2/4 usb: typec: tps6598x: Add trace event for status register
In reply to Guido GüntherThis allows to trace status information which helps to debug problems with role switching, etc.
We don't use the generic FIELD_GET() to reduce the macro size since we otherwise trip up sparse.
--- drivers/usb/typec/tps6598x.c | 26 ++++----- drivers/usb/typec/tps6598x.h | 68 +++++++++++++++++++++ drivers/usb/typec/tps6598x_trace.h | 94 ++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 15 deletions(-)
drivers/usb/typec/tps6598x.c +11 -15
@@ -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 )
drivers/usb/typec/tps6598x.h +68 -0
74 lines not on this page · raw message
drivers/usb/typec/tps6598x_trace.h +94 -0
106 lines not on this page · raw message
Signature · 1 line
2.30.0
Re: [PATCH v3 2/4] usb: typec: tps6598x: Add trace event for status register
Quoted 7 lines · Guido Günther
This allows to trace status information which helps to debug problems with role switching, etc.
We don't use the generic
FIELD_GET()to reduce the macro size since we otherwise trip up sparse.Signed-off-by Guido Günther <[email protected]>
Quoted 278 lines
--- drivers/usb/typec/tps6598x.c | 26 ++++----- drivers/usb/typec/tps6598x.h | 68 +++++++++++++++++++++ drivers/usb/typec/tps6598x_trace.h | 94 ++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 15 deletions(-)drivers/usb/typec/tps6598x.c +11
-15@@ -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 )drivers/usb/typec/tps6598x.h +68
-0@@ -12,6 +12,74 @@#ifndef __TPS6598X_H__#define __TPS6598X_H__ +#define TPS_FIELD_GET (_mask, _reg) ((typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask))) + +/* 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) TPS_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) TPS_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) TPS_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) TPS_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) TPS_FIELD_GET(TPS_STATUS_PP_CABLE_SWITCH_MASK, (x)) +#define TPS_STATUS_POWER_SOURCE_MASK GENMASK(19, 18) +#define TPS_STATUS_POWER_SOURCE (x) TPS_FIELD_GET(TPS_STATUS_POWER_SOURCE_MASK, (x)) +#define TPS_STATUS_VBUS_STATUS_MASK GENMASK(21, 20) +#define TPS_STATUS_VBUS_STATUS (x) TPS_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) TPS_FIELD_GET(TPS_STATUS_USB_HOST_PRESENT_MASK, (x)) +#define TPS_STATUS_LEGACY_MASK GENMASK(25, 24) +#define TPS_STATUS_LEGACY (x) TPS_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)drivers/usb/typec/tps6598x_trace.h +94
-0@@ -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 */ Signature · 1 line
2.30.0
thanks,
Signature · 1 line
heikki
PATCH v3 3/4 usb: typec: tps6598x: Add trace event for power status register
In reply to Guido GüntherTogether with the PD status register this is vital for debugging power negotiations at runtime.
--- 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(-)
drivers/usb/typec/tps6598x.c +12 -7
@@ -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 ;
drivers/usb/typec/tps6598x.h +19 -0
@@ -129,4 +129,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) TPS_FIELD_GET(BIT(0), (x)) +#define TPS_POWER_STATUS_SOURCESINK (x) TPS_FIELD_GET(BIT(1), (x)) +#define TPS_POWER_STATUS_BC12_DET (x) TPS_FIELD_GET(BIT(2), (x)) + +#define TPS_POWER_STATUS_TYPEC_CURRENT_MASK GENMASK(3, 2) +#define TPS_POWER_STATUS_PWROPMODE (p) TPS_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) TPS_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__ */
drivers/usb/typec/tps6598x_trace.h +38 -0
@@ -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 */
Signature · 1 line
2.30.0
Re: [PATCH v3 3/4] usb: typec: tps6598x: Add trace event for power status register
Quoted 4 lines · Guido Günther
Together with the PD status register this is vital for debugging power negotiations at runtime.
Signed-off-by Guido Günther <[email protected]>
Quoted 143 lines
--- 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(-)drivers/usb/typec/tps6598x.c +12
-7@@ -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 ;drivers/usb/typec/tps6598x.h +19
-0@@ -129,4 +129,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) TPS_FIELD_GET(BIT(0), (x)) +#define TPS_POWER_STATUS_SOURCESINK (x) TPS_FIELD_GET(BIT(1), (x)) +#define TPS_POWER_STATUS_BC12_DET (x) TPS_FIELD_GET(BIT(2), (x)) + +#define TPS_POWER_STATUS_TYPEC_CURRENT_MASK GENMASK(3, 2) +#define TPS_POWER_STATUS_PWROPMODE (p) TPS_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) TPS_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__ */ drivers/usb/typec/tps6598x_trace.h +38
-0@@ -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 */ Signature · 1 line
2.30.0
thanks,
Signature · 1 line
heikki
PATCH v3 4/4 usb: typec: tps6598x: Add trace event for data status
In reply to Guido GüntherThis is useful to debug DP negotiation and pin assignment even when the firmware does all the work.
--- drivers/usb/typec/tps6598x.c | 12 ++++++- drivers/usb/typec/tps6598x.h | 38 +++++++++++++++++++++ drivers/usb/typec/tps6598x_trace.h | 54 ++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-)
drivers/usb/typec/tps6598x.c +11 -1
@@ -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 ) {
drivers/usb/typec/tps6598x.h +38 -0
@@ -148,4 +148,42 @@#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) \ + TPS_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 \ + TPS_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 \ + TPS_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) | \ + TPS_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__ */
drivers/usb/typec/tps6598x_trace.h +54 -0
66 lines not on this page · raw message
Signature · 1 line
2.30.0
Re: [PATCH v3 4/4] usb: typec: tps6598x: Add trace event for data status
Quoted 4 lines · Guido Günther
This is useful to debug DP negotiation and pin assignment even when the firmware does all the work.
Signed-off-by Guido Günther <[email protected]>
Quoted 164 lines
--- drivers/usb/typec/tps6598x.c | 12 ++++++- drivers/usb/typec/tps6598x.h | 38 +++++++++++++++++++++ drivers/usb/typec/tps6598x_trace.h | 54 ++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-)drivers/usb/typec/tps6598x.c +11
-1@@ -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 ) {drivers/usb/typec/tps6598x.h +38
-0@@ -148,4 +148,42 @@#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) \ + TPS_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 \ + TPS_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 \ + TPS_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) | \ + TPS_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__ */ drivers/usb/typec/tps6598x_trace.h +54
-0@@ -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 */ Signature · 1 line
2.30.0
thanks,
Signature · 1 line
heikki