mbox series

[v3,0/4] usb: typec: tps6598x: Add IRQ flag and register tracing

Message ID cover.1613389531.git.agx@sigxcpu.org
Headers show
Series usb: typec: tps6598x: Add IRQ flag and register tracing | expand

Message

Guido Günther Feb. 15, 2021, 11:46 a.m. UTC
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/202102120644.IXu75GmJ-lkp@intel.com/
  https://lore.kernel.org/linux-usb/202102120159.Lyh8AGhQ-lkp@intel.com/

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

Comments

Heikki Krogerus March 1, 2021, 3:09 p.m. UTC | #1
On Mon, Feb 15, 2021 at 12:46:42PM +0100, Guido Günther wrote:
> 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>


Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


> ---

>  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


thanks,

-- 
heikki
Heikki Krogerus March 1, 2021, 3:10 p.m. UTC | #2
On Mon, Feb 15, 2021 at 12:46:43PM +0100, Guido Günther wrote:
> 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 <agx@sigxcpu.org>


Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


> ---

>  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(-)

> 

> 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..621fb336c15d 100644

> --- a/drivers/usb/typec/tps6598x.h

> +++ b/drivers/usb/typec/tps6598x.h

> @@ -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)

> 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


thanks,

-- 
heikki