Re: [PATCH 4/4] phy: apple: atc: Add T8122 DisplayPort support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] phy: apple: atc: incomplete PHY reconfiguration on
  orientation change in atcphy_sw_set()
- [Medium] phy: apple: atc: missing RCAL1 poll for APPLE_ATCPHY_MODE_DP
  in atcphy_configure()
--

commit d63ba778ecb86e9dbceea7ed869d115d4530f29a
Author: Ramzi Abdiche <abdicheramzi@gmail.com>

phy: apple: atc: Add T8122 DisplayPort support

This commit adds DisplayPort AUX and lane configuration support for T8122
PHYs, introduces a DisplayPort operations table in struct atcphy_hw, and
stops active DisplayPort lanes before changing orientation or PHY mode.

> diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
> index 256a084996469..068d985b62a6d 100644
> --- a/drivers/phy/apple/atc.c
> +++ b/drivers/phy/apple/atc.c

[ ... ]

> @@ -1942,12 +2492,24 @@ static void atcphy_configure_post_tunable_t8122(struct apple_atcphy *atcphy, enu
>  	udelay(10);
>  }
>  
> +static void atcphy_configure_post_tunable_t8122(struct apple_atcphy *atcphy, enum atcphy_mode mode)
> +{
> +	if (atcphy_modes[mode].pipehandler_state == ATCPHY_PIPEHANDLER_STATE_USB3 ||
> +	    (atcphy->hw->dp && atcphy_modes[mode].enable_dp_aux))
> +		atcphy_power_on_common_t8122(atcphy);
> +}
> +
>  static int atcphy_configure(struct apple_atcphy *atcphy, enum atcphy_mode mode)
>  {
>  	int ret = 0;
>  	u32 reg;
>  
>  	lockdep_assert_held(&atcphy->lock);
> +	if (atcphy->hw->dp && atcphy->hw->dp->stop) {
> +		ret = atcphy->hw->dp->stop(atcphy);
> +		if (ret)
> +			return ret;
> +	}

[Severity: Medium]
Should the AUS_COMMON_DIG_RCAL1 poll later in atcphy_configure() also run
when (atcphy->hw->dp && atcphy_modes[mode].enable_dp_aux) is true?

atcphy_configure_post_tunable_t8122() now powers on the common block for
APPLE_ATCPHY_MODE_DP (where pipehandler_state is
ATCPHY_PIPEHANDLER_STATE_DUMMY), but atcphy_configure() only waits for
AUS_COMMON_DIG_RCAL1_ALL_CODES_DONE when pipehandler_state is
ATCPHY_PIPEHANDLER_STATE_USB3:

drivers/phy/apple/atc.c:atcphy_configure() {
    ...
	if (atcphy->hw->gen == ATCPHY_GENERATION_T8122 &&
	    atcphy_modes[mode].pipehandler_state == ATCPHY_PIPEHANDLER_STATE_USB3) {
		ret = readl_poll_timeout(atcphy->regs.core + AUS_COMMON_DIG_RCAL1, reg,
					 (reg & AUS_COMMON_DIG_RCAL1_ALL_CODES_DONE), 10, 100000);
    ...
}

In 4-lane DisplayPort mode (Pin Assignments C and E), the PHY will proceed
with AUX and link training without waiting for resistor calibration to
finish.

[ ... ]

> @@ -2257,19 +2829,31 @@ static int atcphy_probe_rcdev(struct apple_atcphy *atcphy)
>  static int atcphy_sw_set(struct typec_switch_dev *sw, enum typec_orientation orientation)
>  {
>  	struct apple_atcphy *atcphy = typec_switch_get_drvdata(sw);
> +	bool swap_lanes;
> +	int ret;
>  
>  	guard(mutex)(&atcphy->lock);
>  
>  	switch (orientation) {
>  	case TYPEC_ORIENTATION_NONE:
> -		break;
> +		return 0;
>  	case TYPEC_ORIENTATION_NORMAL:
> -		atcphy->swap_lanes = false;
> +		swap_lanes = false;
>  		break;
>  	case TYPEC_ORIENTATION_REVERSE:
> -		atcphy->swap_lanes = true;
> +		swap_lanes = true;
>  		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* Cleanup needs the old lane configuration until shutdown succeeds. */
> +	if (swap_lanes != atcphy->swap_lanes && atcphy->hw->dp && atcphy->hw->dp->stop) {
> +		ret = atcphy->hw->dp->stop(atcphy);
> +		if (ret)
> +			return ret;
>  	}
> +	atcphy->swap_lanes = swap_lanes;
>  
>  	return 0;
>  }

[Severity: Medium]
If orientation changes while atcphy->mode != APPLE_ATCPHY_MODE_OFF, does
stopping only the DisplayPort link leave the crossbar, lane mode, and
tunable registers programmed for the old orientation?

When atcphy_sw_set() flips atcphy->swap_lanes while a mode is already
active (the case where dp->stop() here has an effect, since transitioning
through TYPEC_STATE_SAFE already calls dp->stop() in atcphy_configure()),
atcphy->mode is left unchanged.  A subsequent atcphy_mux_set() for the
same mode returns early because atcphy->mode == target_mode, and
atcphy_dp_set_rate_t8122() then configures the DisplayPort PMA lanes for
the new swap_lanes while ACIOPHY_CROSSBAR, ACIOPHY_LANE_MODE, and
ATCPHY_MISC_LANE_SWAP remain set for the previous orientation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-t8122-phy-v1-0-c2783b2a53a8@chaosmail.tech?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy