From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: "Guido Günther" <agx@sigxcpu.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH v3 4/4] usb: typec: tps6598x: Add trace event for data status
Date: Mon, 1 Mar 2021 17:12:55 +0200 [thread overview]
Message-ID: <YD0Ed4TUlAlZmOhz@kuha.fi.intel.com> (raw)
In-Reply-To: <1125497fb83eac13fa1ee532759b91ce03770572.1613389531.git.agx@sigxcpu.org>
On Mon, Feb 15, 2021 at 12:46:45PM +0100, Guido Günther wrote:
> 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>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> 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(-)
>
> 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 9a34c020f3e5..003a577be216 100644
> --- a/drivers/usb/typec/tps6598x.h
> +++ b/drivers/usb/typec/tps6598x.h
> @@ -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__ */
> 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
thanks,
--
heikki
prev parent reply other threads:[~2021-03-01 15:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-15 11:46 [PATCH v3 0/4] usb: typec: tps6598x: Add IRQ flag and register tracing Guido Günther
2021-02-15 11:46 ` [PATCH v3 1/4] usb: typec: tps6598x: Add trace event for IRQ events Guido Günther
2021-03-01 15:09 ` Heikki Krogerus
2021-02-15 11:46 ` [PATCH v3 2/4] usb: typec: tps6598x: Add trace event for status register Guido Günther
2021-03-01 15:10 ` Heikki Krogerus
2021-02-15 11:46 ` [PATCH v3 3/4] usb: typec: tps6598x: Add trace event for power " Guido Günther
2021-03-01 15:11 ` Heikki Krogerus
2021-02-15 11:46 ` [PATCH v3 4/4] usb: typec: tps6598x: Add trace event for data status Guido Günther
2021-03-01 15:12 ` Heikki Krogerus [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YD0Ed4TUlAlZmOhz@kuha.fi.intel.com \
--to=heikki.krogerus@linux.intel.com \
--cc=agx@sigxcpu.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).