mbox series

[0/4] thunderbolt: Add support for receiver lane margining

Message ID 20220823105352.56306-1-mika.westerberg@linux.intel.com
Headers show
Series thunderbolt: Add support for receiver lane margining | expand

Message

Mika Westerberg Aug. 23, 2022, 10:53 a.m. UTC
Hi all,

This series adds support for receiver lane margining. This is standard
USB4 port feature that can be used in manufacturing to check electrical
robustness and quality of given USB4 port. This is a separate Kconfig
option (CONFIG_USB4_DEBUGFS_MARGINING) that can be enabled by the
kernels used in manufacturing floor (normal distro kernels do not need
this to be enabled).

This exposes a new debugfs directory "margining" under each connected
USB4 port that can be used to run the margining test. This supports both
hardware and software lane margining although I have only tested the
former as the current Intel hardware only supports that.

Mika Westerberg (4):
  thunderbolt: Move tb_xdomain_parent() to tb.h
  thunderbolt: Move port CL state functions into correct place in switch.c
  thunderbolt: Add helpers to check if CL states are enabled on port
  thunderbolt: Add support for receiver lane margining

 drivers/thunderbolt/Kconfig   |  10 +
 drivers/thunderbolt/debugfs.c | 836 ++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/sb_regs.h |  58 +++
 drivers/thunderbolt/switch.c  | 235 +++++-----
 drivers/thunderbolt/tb.h      |  27 ++
 drivers/thunderbolt/usb4.c    | 120 +++++
 drivers/thunderbolt/xdomain.c |   9 +-
 7 files changed, 1184 insertions(+), 111 deletions(-)

Comments

Lukas Wunner Aug. 23, 2022, 2:02 p.m. UTC | #1
On Tue, Aug 23, 2022 at 01:53:51PM +0300, Mika Westerberg wrote:
> +/**
> + * tb_port_is_clx_enabled() - Is given CL state enabled
> + * @port: USB4 port to check
> + * @clx: CL state to check
> + *
> + * Returns true if given CL state is enabled for @port.
> + */
> +bool tb_port_is_clx_enabled(struct tb_port *port, enum tb_clx clx)
> +{
> +	u32 phy, mask = LANE_ADP_CS_1_CL0S_ENABLE | LANE_ADP_CS_1_CL1_ENABLE;
> +	int ret;
> +
> +	if (!tb_port_clx_supported(port, clx))
> +		return false;
> +
> +	ret = tb_port_read(port, &phy, TB_CFG_PORT,
> +			   port->cap_phy + LANE_ADP_CS_1, 1);
> +	if (ret)
> +		return false;
> +
> +	return (phy & mask) == mask;
> +}
> +
[...]
> +static inline bool tb_port_are_clx_enabled(struct tb_port *port)
> +{
> +	return tb_port_is_clx_enabled(port, TB_CL1) ||
> +	       tb_port_is_clx_enabled(port, TB_CL2);
> +}

If you change enum tb_clx to use "power of two" values (0 1 2 4 8 ...)
then you could just pass a bitmask to tb_port_is_clx_enabled()
and thus need only a single invocation in tb_port_are_clx_enabled().
Just a thought.

Thanks,

Lukas
Mika Westerberg Aug. 24, 2022, 8:33 a.m. UTC | #2
Hi Lukas,

On Tue, Aug 23, 2022 at 04:02:16PM +0200, Lukas Wunner wrote:
> On Tue, Aug 23, 2022 at 01:53:51PM +0300, Mika Westerberg wrote:
> > +/**
> > + * tb_port_is_clx_enabled() - Is given CL state enabled
> > + * @port: USB4 port to check
> > + * @clx: CL state to check
> > + *
> > + * Returns true if given CL state is enabled for @port.
> > + */
> > +bool tb_port_is_clx_enabled(struct tb_port *port, enum tb_clx clx)
> > +{
> > +	u32 phy, mask = LANE_ADP_CS_1_CL0S_ENABLE | LANE_ADP_CS_1_CL1_ENABLE;
> > +	int ret;
> > +
> > +	if (!tb_port_clx_supported(port, clx))
> > +		return false;
> > +
> > +	ret = tb_port_read(port, &phy, TB_CFG_PORT,
> > +			   port->cap_phy + LANE_ADP_CS_1, 1);
> > +	if (ret)
> > +		return false;
> > +
> > +	return (phy & mask) == mask;
> > +}
> > +
> [...]
> > +static inline bool tb_port_are_clx_enabled(struct tb_port *port)
> > +{
> > +	return tb_port_is_clx_enabled(port, TB_CL1) ||
> > +	       tb_port_is_clx_enabled(port, TB_CL2);
> > +}
> 
> If you change enum tb_clx to use "power of two" values (0 1 2 4 8 ...)
> then you could just pass a bitmask to tb_port_is_clx_enabled()
> and thus need only a single invocation in tb_port_are_clx_enabled().
> Just a thought.

Sure good point. I'll do that in v2 thanks!