diff mbox series

[v1] tcpm: Honour pSnkStdby requirement during negotiation

Message ID 20200804065111.3889122-1-badhri@google.com (mailing list archive)
State New, archived
Headers show
Series [v1] tcpm: Honour pSnkStdby requirement during negotiation | expand

Commit Message

Badhri Jagan Sridharan Aug. 4, 2020, 6:51 a.m. UTC
From PD Spec:
The Sink Shall transition to Sink Standby before a positive or
negative voltage transition of VBUS. During Sink Standby
the Sink Shall reduce its power draw to pSnkStdby. This allows
the Source to manage the voltage transition as well as
supply sufficient operating current to the Sink to maintain PD
operation during the transition. The Sink Shall
complete this transition to Sink Standby within tSnkStdby
after evaluating the Accept Message from the Source. The
transition when returning to Sink operation from Sink Standby
Shall be completed within tSnkNewPower. The
pSnkStdby requirement Shall only apply if the Sink power draw
is higher than this level.

The above requirement needs to be met to prevent hard resets
from port partner.

Introducing psnkstdby_after_accept flag to accommodate systems
where the input current limit loops are not fast enough to meet
tSnkStby(15 msec).

When not set, port requests PD_P_SNK_STDBY upon entering SNK_DISCOVERY and
the actual currrent limit after RX of PD_CTRL_PSRDY for PD link,
SNK_READY for non-pd link.

When set, port requests CC advertisement based current limit during
SNK_DISCOVERY, current gets limited to PD_P_SNK_STDBY during
SNK_TRANSITION_SINK, PD based current limit gets set after RX of
PD_CTRL_PSRDY. However, in this case it has to be made sure that the
tSnkStdby (15 msec) is met.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 52 +++++++++++++++++++++++++++--------
 include/linux/usb/pd.h        |  5 +++-
 2 files changed, 44 insertions(+), 13 deletions(-)

Comments

kernel test robot Aug. 4, 2020, 8:26 a.m. UTC | #1
Hi Badhri,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on next-20200803]
[cannot apply to v5.8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Badhri-Jagan-Sridharan/tcpm-Honour-pSnkStdby-requirement-during-negotiation/20200804-145301
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/typec/tcpm/tcpm.c: In function 'run_state_machine':
>> drivers/usb/typec/tcpm/tcpm.c:3339:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    3339 |   if (port->psnkstdby_after_accept)
         |      ^
   drivers/usb/typec/tcpm/tcpm.c:3343:2: note: here
    3343 |  case SNK_TRANSITION_SINK_VBUS:
         |  ^~~~
   At top level:
   drivers/usb/typec/tcpm/tcpm.c:1614:39: warning: 'tcpm_altmode_ops' defined but not used [-Wunused-const-variable=]
    1614 | static const struct typec_altmode_ops tcpm_altmode_ops = {
         |                                       ^~~~~~~~~~~~~~~~

vim +3339 drivers/usb/typec/tcpm/tcpm.c

  2943	
  2944	static void run_state_machine(struct tcpm_port *port)
  2945	{
  2946		int ret;
  2947		enum typec_pwr_opmode opmode;
  2948		unsigned int msecs;
  2949	
  2950		port->enter_state = port->state;
  2951		switch (port->state) {
  2952		case TOGGLING:
  2953			break;
  2954		/* SRC states */
  2955		case SRC_UNATTACHED:
  2956			if (!port->non_pd_role_swap)
  2957				tcpm_swap_complete(port, -ENOTCONN);
  2958			tcpm_src_detach(port);
  2959			if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
  2960				tcpm_set_state(port, TOGGLING, 0);
  2961				break;
  2962			}
  2963			tcpm_set_cc(port, tcpm_rp_cc(port));
  2964			if (port->port_type == TYPEC_PORT_DRP)
  2965				tcpm_set_state(port, SNK_UNATTACHED, PD_T_DRP_SNK);
  2966			break;
  2967		case SRC_ATTACH_WAIT:
  2968			if (tcpm_port_is_debug(port))
  2969				tcpm_set_state(port, DEBUG_ACC_ATTACHED,
  2970					       PD_T_CC_DEBOUNCE);
  2971			else if (tcpm_port_is_audio(port))
  2972				tcpm_set_state(port, AUDIO_ACC_ATTACHED,
  2973					       PD_T_CC_DEBOUNCE);
  2974			else if (tcpm_port_is_source(port))
  2975				tcpm_set_state(port,
  2976					       tcpm_try_snk(port) ? SNK_TRY
  2977								  : SRC_ATTACHED,
  2978					       PD_T_CC_DEBOUNCE);
  2979			break;
  2980	
  2981		case SNK_TRY:
  2982			port->try_snk_count++;
  2983			/*
  2984			 * Requirements:
  2985			 * - Do not drive vconn or vbus
  2986			 * - Terminate CC pins (both) to Rd
  2987			 * Action:
  2988			 * - Wait for tDRPTry (PD_T_DRP_TRY).
  2989			 *   Until then, ignore any state changes.
  2990			 */
  2991			tcpm_set_cc(port, TYPEC_CC_RD);
  2992			tcpm_set_state(port, SNK_TRY_WAIT, PD_T_DRP_TRY);
  2993			break;
  2994		case SNK_TRY_WAIT:
  2995			if (tcpm_port_is_sink(port)) {
  2996				tcpm_set_state(port, SNK_TRY_WAIT_DEBOUNCE, 0);
  2997			} else {
  2998				tcpm_set_state(port, SRC_TRYWAIT, 0);
  2999				port->max_wait = 0;
  3000			}
  3001			break;
  3002		case SNK_TRY_WAIT_DEBOUNCE:
  3003			tcpm_set_state(port, SNK_TRY_WAIT_DEBOUNCE_CHECK_VBUS,
  3004				       PD_T_PD_DEBOUNCE);
  3005			break;
  3006		case SNK_TRY_WAIT_DEBOUNCE_CHECK_VBUS:
  3007			if (port->vbus_present && tcpm_port_is_sink(port)) {
  3008				tcpm_set_state(port, SNK_ATTACHED, 0);
  3009			} else {
  3010				tcpm_set_state(port, SRC_TRYWAIT, 0);
  3011				port->max_wait = 0;
  3012			}
  3013			break;
  3014		case SRC_TRYWAIT:
  3015			tcpm_set_cc(port, tcpm_rp_cc(port));
  3016			if (port->max_wait == 0) {
  3017				port->max_wait = jiffies +
  3018						 msecs_to_jiffies(PD_T_DRP_TRY);
  3019				tcpm_set_state(port, SRC_TRYWAIT_UNATTACHED,
  3020					       PD_T_DRP_TRY);
  3021			} else {
  3022				if (time_is_after_jiffies(port->max_wait))
  3023					tcpm_set_state(port, SRC_TRYWAIT_UNATTACHED,
  3024						       jiffies_to_msecs(port->max_wait -
  3025									jiffies));
  3026				else
  3027					tcpm_set_state(port, SNK_UNATTACHED, 0);
  3028			}
  3029			break;
  3030		case SRC_TRYWAIT_DEBOUNCE:
  3031			tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE);
  3032			break;
  3033		case SRC_TRYWAIT_UNATTACHED:
  3034			tcpm_set_state(port, SNK_UNATTACHED, 0);
  3035			break;
  3036	
  3037		case SRC_ATTACHED:
  3038			ret = tcpm_src_attach(port);
  3039			tcpm_set_state(port, SRC_UNATTACHED,
  3040				       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
  3041			break;
  3042		case SRC_STARTUP:
  3043			opmode =  tcpm_get_pwr_opmode(tcpm_rp_cc(port));
  3044			typec_set_pwr_opmode(port->typec_port, opmode);
  3045			port->pwr_opmode = TYPEC_PWR_MODE_USB;
  3046			port->caps_count = 0;
  3047			port->negotiated_rev = PD_MAX_REV;
  3048			port->message_id = 0;
  3049			port->rx_msgid = -1;
  3050			port->explicit_contract = false;
  3051			tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
  3052			break;
  3053		case SRC_SEND_CAPABILITIES:
  3054			port->caps_count++;
  3055			if (port->caps_count > PD_N_CAPS_COUNT) {
  3056				tcpm_set_state(port, SRC_READY, 0);
  3057				break;
  3058			}
  3059			ret = tcpm_pd_send_source_caps(port);
  3060			if (ret < 0) {
  3061				tcpm_set_state(port, SRC_SEND_CAPABILITIES,
  3062					       PD_T_SEND_SOURCE_CAP);
  3063			} else {
  3064				/*
  3065				 * Per standard, we should clear the reset counter here.
  3066				 * However, that can result in state machine hang-ups.
  3067				 * Reset it only in READY state to improve stability.
  3068				 */
  3069				/* port->hard_reset_count = 0; */
  3070				port->caps_count = 0;
  3071				port->pd_capable = true;
  3072				tcpm_set_state_cond(port, SRC_SEND_CAPABILITIES_TIMEOUT,
  3073						    PD_T_SEND_SOURCE_CAP);
  3074			}
  3075			break;
  3076		case SRC_SEND_CAPABILITIES_TIMEOUT:
  3077			/*
  3078			 * Error recovery for a PD_DATA_SOURCE_CAP reply timeout.
  3079			 *
  3080			 * PD 2.0 sinks are supposed to accept src-capabilities with a
  3081			 * 3.0 header and simply ignore any src PDOs which the sink does
  3082			 * not understand such as PPS but some 2.0 sinks instead ignore
  3083			 * the entire PD_DATA_SOURCE_CAP message, causing contract
  3084			 * negotiation to fail.
  3085			 *
  3086			 * After PD_N_HARD_RESET_COUNT hard-reset attempts, we try
  3087			 * sending src-capabilities with a lower PD revision to
  3088			 * make these broken sinks work.
  3089			 */
  3090			if (port->hard_reset_count < PD_N_HARD_RESET_COUNT) {
  3091				tcpm_set_state(port, HARD_RESET_SEND, 0);
  3092			} else if (port->negotiated_rev > PD_REV20) {
  3093				port->negotiated_rev--;
  3094				port->hard_reset_count = 0;
  3095				tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
  3096			} else {
  3097				tcpm_set_state(port, hard_reset_state(port), 0);
  3098			}
  3099			break;
  3100		case SRC_NEGOTIATE_CAPABILITIES:
  3101			ret = tcpm_pd_check_request(port);
  3102			if (ret < 0) {
  3103				tcpm_pd_send_control(port, PD_CTRL_REJECT);
  3104				if (!port->explicit_contract) {
  3105					tcpm_set_state(port,
  3106						       SRC_WAIT_NEW_CAPABILITIES, 0);
  3107				} else {
  3108					tcpm_set_state(port, SRC_READY, 0);
  3109				}
  3110			} else {
  3111				tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
  3112				tcpm_set_state(port, SRC_TRANSITION_SUPPLY,
  3113					       PD_T_SRC_TRANSITION);
  3114			}
  3115			break;
  3116		case SRC_TRANSITION_SUPPLY:
  3117			/* XXX: regulator_set_voltage(vbus, ...) */
  3118			tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
  3119			port->explicit_contract = true;
  3120			typec_set_pwr_opmode(port->typec_port, TYPEC_PWR_MODE_PD);
  3121			port->pwr_opmode = TYPEC_PWR_MODE_PD;
  3122			tcpm_set_state_cond(port, SRC_READY, 0);
  3123			break;
  3124		case SRC_READY:
  3125	#if 1
  3126			port->hard_reset_count = 0;
  3127	#endif
  3128			port->try_src_count = 0;
  3129	
  3130			tcpm_swap_complete(port, 0);
  3131			tcpm_typec_connect(port);
  3132	
  3133			tcpm_check_send_discover(port);
  3134			/*
  3135			 * 6.3.5
  3136			 * Sending ping messages is not necessary if
  3137			 * - the source operates at vSafe5V
  3138			 * or
  3139			 * - The system is not operating in PD mode
  3140			 * or
  3141			 * - Both partners are connected using a Type-C connector
  3142			 *
  3143			 * There is no actual need to send PD messages since the local
  3144			 * port type-c and the spec does not clearly say whether PD is
  3145			 * possible when type-c is connected to Type-A/B
  3146			 */
  3147			break;
  3148		case SRC_WAIT_NEW_CAPABILITIES:
  3149			/* Nothing to do... */
  3150			break;
  3151	
  3152		/* SNK states */
  3153		case SNK_UNATTACHED:
  3154			if (!port->non_pd_role_swap)
  3155				tcpm_swap_complete(port, -ENOTCONN);
  3156			tcpm_pps_complete(port, -ENOTCONN);
  3157			tcpm_snk_detach(port);
  3158			if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
  3159				tcpm_set_state(port, TOGGLING, 0);
  3160				break;
  3161			}
  3162			tcpm_set_cc(port, TYPEC_CC_RD);
  3163			if (port->port_type == TYPEC_PORT_DRP)
  3164				tcpm_set_state(port, SRC_UNATTACHED, PD_T_DRP_SRC);
  3165			break;
  3166		case SNK_ATTACH_WAIT:
  3167			if ((port->cc1 == TYPEC_CC_OPEN &&
  3168			     port->cc2 != TYPEC_CC_OPEN) ||
  3169			    (port->cc1 != TYPEC_CC_OPEN &&
  3170			     port->cc2 == TYPEC_CC_OPEN))
  3171				tcpm_set_state(port, SNK_DEBOUNCED,
  3172					       PD_T_CC_DEBOUNCE);
  3173			else if (tcpm_port_is_disconnected(port))
  3174				tcpm_set_state(port, SNK_UNATTACHED,
  3175					       PD_T_PD_DEBOUNCE);
  3176			break;
  3177		case SNK_DEBOUNCED:
  3178			if (tcpm_port_is_disconnected(port))
  3179				tcpm_set_state(port, SNK_UNATTACHED,
  3180					       PD_T_PD_DEBOUNCE);
  3181			else if (port->vbus_present)
  3182				tcpm_set_state(port,
  3183					       tcpm_try_src(port) ? SRC_TRY
  3184								  : SNK_ATTACHED,
  3185					       0);
  3186			else
  3187				/* Wait for VBUS, but not forever */
  3188				tcpm_set_state(port, PORT_RESET, PD_T_PS_SOURCE_ON);
  3189			break;
  3190	
  3191		case SRC_TRY:
  3192			port->try_src_count++;
  3193			tcpm_set_cc(port, tcpm_rp_cc(port));
  3194			port->max_wait = 0;
  3195			tcpm_set_state(port, SRC_TRY_WAIT, 0);
  3196			break;
  3197		case SRC_TRY_WAIT:
  3198			if (port->max_wait == 0) {
  3199				port->max_wait = jiffies +
  3200						 msecs_to_jiffies(PD_T_DRP_TRY);
  3201				msecs = PD_T_DRP_TRY;
  3202			} else {
  3203				if (time_is_after_jiffies(port->max_wait))
  3204					msecs = jiffies_to_msecs(port->max_wait -
  3205								 jiffies);
  3206				else
  3207					msecs = 0;
  3208			}
  3209			tcpm_set_state(port, SNK_TRYWAIT, msecs);
  3210			break;
  3211		case SRC_TRY_DEBOUNCE:
  3212			tcpm_set_state(port, SRC_ATTACHED, PD_T_PD_DEBOUNCE);
  3213			break;
  3214		case SNK_TRYWAIT:
  3215			tcpm_set_cc(port, TYPEC_CC_RD);
  3216			tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE);
  3217			break;
  3218		case SNK_TRYWAIT_VBUS:
  3219			/*
  3220			 * TCPM stays in this state indefinitely until VBUS
  3221			 * is detected as long as Rp is not detected for
  3222			 * more than a time period of tPDDebounce.
  3223			 */
  3224			if (port->vbus_present && tcpm_port_is_sink(port)) {
  3225				tcpm_set_state(port, SNK_ATTACHED, 0);
  3226				break;
  3227			}
  3228			if (!tcpm_port_is_sink(port))
  3229				tcpm_set_state(port, SNK_TRYWAIT_DEBOUNCE, 0);
  3230			break;
  3231		case SNK_TRYWAIT_DEBOUNCE:
  3232			tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
  3233			break;
  3234		case SNK_ATTACHED:
  3235			ret = tcpm_snk_attach(port);
  3236			if (ret < 0)
  3237				tcpm_set_state(port, SNK_UNATTACHED, 0);
  3238			else
  3239				tcpm_set_state(port, SNK_STARTUP, 0);
  3240			break;
  3241		case SNK_STARTUP:
  3242			opmode =  tcpm_get_pwr_opmode(port->polarity ?
  3243						      port->cc2 : port->cc1);
  3244			typec_set_pwr_opmode(port->typec_port, opmode);
  3245			port->pwr_opmode = TYPEC_PWR_MODE_USB;
  3246			port->negotiated_rev = PD_MAX_REV;
  3247			port->message_id = 0;
  3248			port->rx_msgid = -1;
  3249			port->explicit_contract = false;
  3250			tcpm_set_state(port, SNK_DISCOVERY, 0);
  3251			break;
  3252		case SNK_DISCOVERY:
  3253			if (port->vbus_present) {
  3254				if (port->psnkstdby_after_accept || tcpm_get_current_limit(port) <=
  3255				    PD_P_SNK_STDBY_5V)
  3256					tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
  3257				else
  3258					tcpm_set_current_limit(port, PD_P_SNK_STDBY_5V, 5000);
  3259				tcpm_set_charge(port, true);
  3260				tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
  3261				break;
  3262			}
  3263			/*
  3264			 * For DRP, timeouts differ. Also, handling is supposed to be
  3265			 * different and much more complex (dead battery detection;
  3266			 * see USB power delivery specification, section 8.3.3.6.1.5.1).
  3267			 */
  3268			tcpm_set_state(port, hard_reset_state(port),
  3269				       port->port_type == TYPEC_PORT_DRP ?
  3270						PD_T_DB_DETECT : PD_T_NO_RESPONSE);
  3271			break;
  3272		case SNK_DISCOVERY_DEBOUNCE:
  3273			tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE,
  3274				       PD_T_CC_DEBOUNCE);
  3275			break;
  3276		case SNK_DISCOVERY_DEBOUNCE_DONE:
  3277			if (!tcpm_port_is_disconnected(port) &&
  3278			    tcpm_port_is_sink(port) &&
  3279			    time_is_after_jiffies(port->delayed_runtime)) {
  3280				tcpm_set_state(port, SNK_DISCOVERY,
  3281					       jiffies_to_msecs(port->delayed_runtime -
  3282								jiffies));
  3283				break;
  3284			}
  3285			tcpm_set_state(port, unattached_state(port), 0);
  3286			break;
  3287		case SNK_WAIT_CAPABILITIES:
  3288			ret = port->tcpc->set_pd_rx(port->tcpc, true);
  3289			if (ret < 0) {
  3290				tcpm_set_state(port, SNK_READY, 0);
  3291				break;
  3292			}
  3293			/*
  3294			 * If VBUS has never been low, and we time out waiting
  3295			 * for source cap, try a soft reset first, in case we
  3296			 * were already in a stable contract before this boot.
  3297			 * Do this only once.
  3298			 */
  3299			if (port->vbus_never_low) {
  3300				port->vbus_never_low = false;
  3301				tcpm_set_state(port, SOFT_RESET_SEND,
  3302					       PD_T_SINK_WAIT_CAP);
  3303			} else {
  3304				tcpm_set_state(port, hard_reset_state(port),
  3305					       PD_T_SINK_WAIT_CAP);
  3306			}
  3307			break;
  3308		case SNK_NEGOTIATE_CAPABILITIES:
  3309			port->pd_capable = true;
  3310			port->hard_reset_count = 0;
  3311			ret = tcpm_pd_send_request(port);
  3312			if (ret < 0) {
  3313				/* Let the Source send capabilities again. */
  3314				tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
  3315			} else {
  3316				tcpm_set_state_cond(port, hard_reset_state(port),
  3317						    PD_T_SENDER_RESPONSE);
  3318			}
  3319			break;
  3320		case SNK_NEGOTIATE_PPS_CAPABILITIES:
  3321			ret = tcpm_pd_send_pps_request(port);
  3322			if (ret < 0) {
  3323				port->pps_status = ret;
  3324				/*
  3325				 * If this was called due to updates to sink
  3326				 * capabilities, and pps is no longer valid, we should
  3327				 * safely fall back to a standard PDO.
  3328				 */
  3329				if (port->update_sink_caps)
  3330					tcpm_set_state(port, SNK_NEGOTIATE_CAPABILITIES, 0);
  3331				else
  3332					tcpm_set_state(port, SNK_READY, 0);
  3333			} else {
  3334				tcpm_set_state_cond(port, hard_reset_state(port),
  3335						    PD_T_SENDER_RESPONSE);
  3336			}
  3337			break;
  3338		case SNK_TRANSITION_SINK:
> 3339			if (port->psnkstdby_after_accept)
  3340				tcpm_set_current_limit(port, tcpm_get_current_limit(port) >
  3341						       PD_P_SNK_STDBY_5V ? PD_P_SNK_STDBY_5V :
  3342						       tcpm_get_current_limit(port), 5000);
  3343		case SNK_TRANSITION_SINK_VBUS:
  3344			tcpm_set_state(port, hard_reset_state(port),
  3345				       PD_T_PS_TRANSITION);
  3346			break;
  3347		case SNK_READY:
  3348			port->try_snk_count = 0;
  3349			port->update_sink_caps = false;
  3350			if (port->explicit_contract) {
  3351				typec_set_pwr_opmode(port->typec_port,
  3352						     TYPEC_PWR_MODE_PD);
  3353				port->pwr_opmode = TYPEC_PWR_MODE_PD;
  3354			}
  3355	
  3356			/* Set current limit for NON-PD link when psnkstdby_after_accept is not set*/
  3357			if (!port->pd_capable && !port->psnkstdby_after_accept)
  3358				tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
  3359	
  3360			tcpm_swap_complete(port, 0);
  3361			tcpm_typec_connect(port);
  3362			tcpm_check_send_discover(port);
  3363			tcpm_pps_complete(port, port->pps_status);
  3364	
  3365			power_supply_changed(port->psy);
  3366	
  3367			break;
  3368	
  3369		/* Accessory states */
  3370		case ACC_UNATTACHED:
  3371			tcpm_acc_detach(port);
  3372			tcpm_set_state(port, SRC_UNATTACHED, 0);
  3373			break;
  3374		case DEBUG_ACC_ATTACHED:
  3375		case AUDIO_ACC_ATTACHED:
  3376			ret = tcpm_acc_attach(port);
  3377			if (ret < 0)
  3378				tcpm_set_state(port, ACC_UNATTACHED, 0);
  3379			break;
  3380		case AUDIO_ACC_DEBOUNCE:
  3381			tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE);
  3382			break;
  3383	
  3384		/* Hard_Reset states */
  3385		case HARD_RESET_SEND:
  3386			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
  3387			tcpm_set_state(port, HARD_RESET_START, 0);
  3388			break;
  3389		case HARD_RESET_START:
  3390			port->hard_reset_count++;
  3391			port->tcpc->set_pd_rx(port->tcpc, false);
  3392			tcpm_unregister_altmodes(port);
  3393			port->send_discover = true;
  3394			if (port->pwr_role == TYPEC_SOURCE)
  3395				tcpm_set_state(port, SRC_HARD_RESET_VBUS_OFF,
  3396					       PD_T_PS_HARD_RESET);
  3397			else
  3398				tcpm_set_state(port, SNK_HARD_RESET_SINK_OFF, 0);
  3399			break;
  3400		case SRC_HARD_RESET_VBUS_OFF:
  3401			tcpm_set_vconn(port, true);
  3402			tcpm_set_vbus(port, false);
  3403			tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
  3404				       tcpm_data_role_for_source(port));
  3405			tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
  3406			break;
  3407		case SRC_HARD_RESET_VBUS_ON:
  3408			tcpm_set_vbus(port, true);
  3409			port->tcpc->set_pd_rx(port->tcpc, true);
  3410			tcpm_set_attached_state(port, true);
  3411			tcpm_set_state(port, SRC_UNATTACHED, PD_T_PS_SOURCE_ON);
  3412			break;
  3413		case SNK_HARD_RESET_SINK_OFF:
  3414			memset(&port->pps_data, 0, sizeof(port->pps_data));
  3415			tcpm_set_vconn(port, false);
  3416			if (port->pd_capable)
  3417				tcpm_set_charge(port, false);
  3418			tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
  3419				       tcpm_data_role_for_sink(port));
  3420			/*
  3421			 * VBUS may or may not toggle, depending on the adapter.
  3422			 * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
  3423			 * directly after timeout.
  3424			 */
  3425			tcpm_set_state(port, SNK_HARD_RESET_SINK_ON, PD_T_SAFE_0V);
  3426			break;
  3427		case SNK_HARD_RESET_WAIT_VBUS:
  3428			/* Assume we're disconnected if VBUS doesn't come back. */
  3429			tcpm_set_state(port, SNK_UNATTACHED,
  3430				       PD_T_SRC_RECOVER_MAX + PD_T_SRC_TURN_ON);
  3431			break;
  3432		case SNK_HARD_RESET_SINK_ON:
  3433			/* Note: There is no guarantee that VBUS is on in this state */
  3434			/*
  3435			 * XXX:
  3436			 * The specification suggests that dual mode ports in sink
  3437			 * mode should transition to state PE_SRC_Transition_to_default.
  3438			 * See USB power delivery specification chapter 8.3.3.6.1.3.
  3439			 * This would mean to to
  3440			 * - turn off VCONN, reset power supply
  3441			 * - request hardware reset
  3442			 * - turn on VCONN
  3443			 * - Transition to state PE_Src_Startup
  3444			 * SNK only ports shall transition to state Snk_Startup
  3445			 * (see chapter 8.3.3.3.8).
  3446			 * Similar, dual-mode ports in source mode should transition
  3447			 * to PE_SNK_Transition_to_default.
  3448			 */
  3449			if (port->pd_capable) {
  3450				tcpm_set_current_limit(port,
  3451						       tcpm_get_current_limit(port),
  3452						       5000);
  3453				tcpm_set_charge(port, true);
  3454			}
  3455			tcpm_set_attached_state(port, true);
  3456			tcpm_set_state(port, SNK_STARTUP, 0);
  3457			break;
  3458	
  3459		/* Soft_Reset states */
  3460		case SOFT_RESET:
  3461			port->message_id = 0;
  3462			port->rx_msgid = -1;
  3463			tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
  3464			if (port->pwr_role == TYPEC_SOURCE)
  3465				tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
  3466			else
  3467				tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
  3468			break;
  3469		case SOFT_RESET_SEND:
  3470			port->message_id = 0;
  3471			port->rx_msgid = -1;
  3472			if (tcpm_pd_send_control(port, PD_CTRL_SOFT_RESET))
  3473				tcpm_set_state_cond(port, hard_reset_state(port), 0);
  3474			else
  3475				tcpm_set_state_cond(port, hard_reset_state(port),
  3476						    PD_T_SENDER_RESPONSE);
  3477			break;
  3478	
  3479		/* DR_Swap states */
  3480		case DR_SWAP_SEND:
  3481			tcpm_pd_send_control(port, PD_CTRL_DR_SWAP);
  3482			tcpm_set_state_cond(port, DR_SWAP_SEND_TIMEOUT,
  3483					    PD_T_SENDER_RESPONSE);
  3484			break;
  3485		case DR_SWAP_ACCEPT:
  3486			tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
  3487			tcpm_set_state_cond(port, DR_SWAP_CHANGE_DR, 0);
  3488			break;
  3489		case DR_SWAP_SEND_TIMEOUT:
  3490			tcpm_swap_complete(port, -ETIMEDOUT);
  3491			tcpm_set_state(port, ready_state(port), 0);
  3492			break;
  3493		case DR_SWAP_CHANGE_DR:
  3494			if (port->data_role == TYPEC_HOST) {
  3495				tcpm_unregister_altmodes(port);
  3496				tcpm_set_roles(port, true, port->pwr_role,
  3497					       TYPEC_DEVICE);
  3498			} else {
  3499				tcpm_set_roles(port, true, port->pwr_role,
  3500					       TYPEC_HOST);
  3501				port->send_discover = true;
  3502			}
  3503			tcpm_set_state(port, ready_state(port), 0);
  3504			break;
  3505	
  3506		/* PR_Swap states */
  3507		case PR_SWAP_ACCEPT:
  3508			tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
  3509			tcpm_set_state(port, PR_SWAP_START, 0);
  3510			break;
  3511		case PR_SWAP_SEND:
  3512			tcpm_pd_send_control(port, PD_CTRL_PR_SWAP);
  3513			tcpm_set_state_cond(port, PR_SWAP_SEND_TIMEOUT,
  3514					    PD_T_SENDER_RESPONSE);
  3515			break;
  3516		case PR_SWAP_SEND_TIMEOUT:
  3517			tcpm_swap_complete(port, -ETIMEDOUT);
  3518			tcpm_set_state(port, ready_state(port), 0);
  3519			break;
  3520		case PR_SWAP_START:
  3521			if (port->pwr_role == TYPEC_SOURCE)
  3522				tcpm_set_state(port, PR_SWAP_SRC_SNK_TRANSITION_OFF,
  3523					       PD_T_SRC_TRANSITION);
  3524			else
  3525				tcpm_set_state(port, PR_SWAP_SNK_SRC_SINK_OFF, 0);
  3526			break;
  3527		case PR_SWAP_SRC_SNK_TRANSITION_OFF:
  3528			tcpm_set_vbus(port, false);
  3529			port->explicit_contract = false;
  3530			/* allow time for Vbus discharge, must be < tSrcSwapStdby */
  3531			tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF,
  3532				       PD_T_SRCSWAPSTDBY);
  3533			break;
  3534		case PR_SWAP_SRC_SNK_SOURCE_OFF:
  3535			tcpm_set_cc(port, TYPEC_CC_RD);
  3536			/* allow CC debounce */
  3537			tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED,
  3538				       PD_T_CC_DEBOUNCE);
  3539			break;
  3540		case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
  3541			/*
  3542			 * USB-PD standard, 6.2.1.4, Port Power Role:
  3543			 * "During the Power Role Swap Sequence, for the initial Source
  3544			 * Port, the Port Power Role field shall be set to Sink in the
  3545			 * PS_RDY Message indicating that the initial Source’s power
  3546			 * supply is turned off"
  3547			 */
  3548			tcpm_set_pwr_role(port, TYPEC_SINK);
  3549			if (tcpm_pd_send_control(port, PD_CTRL_PS_RDY)) {
  3550				tcpm_set_state(port, ERROR_RECOVERY, 0);
  3551				break;
  3552			}
  3553			tcpm_set_state_cond(port, SNK_UNATTACHED, PD_T_PS_SOURCE_ON);
  3554			break;
  3555		case PR_SWAP_SRC_SNK_SINK_ON:
  3556			tcpm_set_state(port, SNK_STARTUP, 0);
  3557			break;
  3558		case PR_SWAP_SNK_SRC_SINK_OFF:
  3559			tcpm_set_charge(port, false);
  3560			tcpm_set_state(port, hard_reset_state(port),
  3561				       PD_T_PS_SOURCE_OFF);
  3562			break;
  3563		case PR_SWAP_SNK_SRC_SOURCE_ON:
  3564			tcpm_set_cc(port, tcpm_rp_cc(port));
  3565			tcpm_set_vbus(port, true);
  3566			/*
  3567			 * allow time VBUS ramp-up, must be < tNewSrc
  3568			 * Also, this window overlaps with CC debounce as well.
  3569			 * So, Wait for the max of two which is PD_T_NEWSRC
  3570			 */
  3571			tcpm_set_state(port, PR_SWAP_SNK_SRC_SOURCE_ON_VBUS_RAMPED_UP,
  3572				       PD_T_NEWSRC);
  3573			break;
  3574		case PR_SWAP_SNK_SRC_SOURCE_ON_VBUS_RAMPED_UP:
  3575			/*
  3576			 * USB PD standard, 6.2.1.4:
  3577			 * "Subsequent Messages initiated by the Policy Engine,
  3578			 * such as the PS_RDY Message sent to indicate that Vbus
  3579			 * is ready, will have the Port Power Role field set to
  3580			 * Source."
  3581			 */
  3582			tcpm_set_pwr_role(port, TYPEC_SOURCE);
  3583			tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
  3584			tcpm_set_state(port, SRC_STARTUP, 0);
  3585			break;
  3586	
  3587		case VCONN_SWAP_ACCEPT:
  3588			tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
  3589			tcpm_set_state(port, VCONN_SWAP_START, 0);
  3590			break;
  3591		case VCONN_SWAP_SEND:
  3592			tcpm_pd_send_control(port, PD_CTRL_VCONN_SWAP);
  3593			tcpm_set_state(port, VCONN_SWAP_SEND_TIMEOUT,
  3594				       PD_T_SENDER_RESPONSE);
  3595			break;
  3596		case VCONN_SWAP_SEND_TIMEOUT:
  3597			tcpm_swap_complete(port, -ETIMEDOUT);
  3598			tcpm_set_state(port, ready_state(port), 0);
  3599			break;
  3600		case VCONN_SWAP_START:
  3601			if (port->vconn_role == TYPEC_SOURCE)
  3602				tcpm_set_state(port, VCONN_SWAP_WAIT_FOR_VCONN, 0);
  3603			else
  3604				tcpm_set_state(port, VCONN_SWAP_TURN_ON_VCONN, 0);
  3605			break;
  3606		case VCONN_SWAP_WAIT_FOR_VCONN:
  3607			tcpm_set_state(port, hard_reset_state(port),
  3608				       PD_T_VCONN_SOURCE_ON);
  3609			break;
  3610		case VCONN_SWAP_TURN_ON_VCONN:
  3611			tcpm_set_vconn(port, true);
  3612			tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
  3613			tcpm_set_state(port, ready_state(port), 0);
  3614			break;
  3615		case VCONN_SWAP_TURN_OFF_VCONN:
  3616			tcpm_set_vconn(port, false);
  3617			tcpm_set_state(port, ready_state(port), 0);
  3618			break;
  3619	
  3620		case DR_SWAP_CANCEL:
  3621		case PR_SWAP_CANCEL:
  3622		case VCONN_SWAP_CANCEL:
  3623			tcpm_swap_complete(port, port->swap_status);
  3624			if (port->pwr_role == TYPEC_SOURCE)
  3625				tcpm_set_state(port, SRC_READY, 0);
  3626			else
  3627				tcpm_set_state(port, SNK_READY, 0);
  3628			break;
  3629	
  3630		case BIST_RX:
  3631			switch (BDO_MODE_MASK(port->bist_request)) {
  3632			case BDO_MODE_CARRIER2:
  3633				tcpm_pd_transmit(port, TCPC_TX_BIST_MODE_2, NULL);
  3634				tcpm_set_state(port, unattached_state(port),
  3635					       PD_T_BIST_CONT_MODE);
  3636				break;
  3637			case BDO_MODE_TESTDATA:
  3638				if (port->tcpc->set_bist_data) {
  3639					tcpm_log(port, "Enable BIST MODE TESTDATA");
  3640					port->tcpc->set_bist_data(port->tcpc, true);
  3641				}
  3642				break;
  3643			default:
  3644				break;
  3645			}
  3646			break;
  3647		case GET_STATUS_SEND:
  3648			tcpm_pd_send_control(port, PD_CTRL_GET_STATUS);
  3649			tcpm_set_state(port, GET_STATUS_SEND_TIMEOUT,
  3650				       PD_T_SENDER_RESPONSE);
  3651			break;
  3652		case GET_STATUS_SEND_TIMEOUT:
  3653			tcpm_set_state(port, ready_state(port), 0);
  3654			break;
  3655		case GET_PPS_STATUS_SEND:
  3656			tcpm_pd_send_control(port, PD_CTRL_GET_PPS_STATUS);
  3657			tcpm_set_state(port, GET_PPS_STATUS_SEND_TIMEOUT,
  3658				       PD_T_SENDER_RESPONSE);
  3659			break;
  3660		case GET_PPS_STATUS_SEND_TIMEOUT:
  3661			tcpm_set_state(port, ready_state(port), 0);
  3662			break;
  3663		case ERROR_RECOVERY:
  3664			tcpm_swap_complete(port, -EPROTO);
  3665			tcpm_pps_complete(port, -EPROTO);
  3666			tcpm_set_state(port, PORT_RESET, 0);
  3667			break;
  3668		case PORT_RESET:
  3669			tcpm_reset_port(port);
  3670			tcpm_set_cc(port, TYPEC_CC_OPEN);
  3671			tcpm_set_state(port, PORT_RESET_WAIT_OFF,
  3672				       PD_T_ERROR_RECOVERY);
  3673			break;
  3674		case PORT_RESET_WAIT_OFF:
  3675			tcpm_set_state(port,
  3676				       tcpm_default_state(port),
  3677				       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
  3678			break;
  3679		default:
  3680			WARN(1, "Unexpected port state %d\n", port->state);
  3681			break;
  3682		}
  3683	}
  3684	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Guenter Roeck Aug. 4, 2020, 4:58 p.m. UTC | #2
On 8/3/20 11:51 PM, Badhri Jagan Sridharan wrote:
>>From PD Spec:
> The Sink Shall transition to Sink Standby before a positive or
> negative voltage transition of VBUS. During Sink Standby
> the Sink Shall reduce its power draw to pSnkStdby. This allows
> the Source to manage the voltage transition as well as
> supply sufficient operating current to the Sink to maintain PD
> operation during the transition. The Sink Shall
> complete this transition to Sink Standby within tSnkStdby
> after evaluating the Accept Message from the Source. The
> transition when returning to Sink operation from Sink Standby
> Shall be completed within tSnkNewPower. The
> pSnkStdby requirement Shall only apply if the Sink power draw
> is higher than this level.
> 
> The above requirement needs to be met to prevent hard resets
> from port partner.
> 
> Introducing psnkstdby_after_accept flag to accommodate systems
> where the input current limit loops are not fast enough to meet
> tSnkStby(15 msec).
> 
> When not set, port requests PD_P_SNK_STDBY upon entering SNK_DISCOVERY and
> the actual currrent limit after RX of PD_CTRL_PSRDY for PD link,
> SNK_READY for non-pd link.
> 
> When set, port requests CC advertisement based current limit during
> SNK_DISCOVERY, current gets limited to PD_P_SNK_STDBY during
> SNK_TRANSITION_SINK, PD based current limit gets set after RX of
> PD_CTRL_PSRDY. However, in this case it has to be made sure that the
> tSnkStdby (15 msec) is met.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 52 +++++++++++++++++++++++++++--------
>  include/linux/usb/pd.h        |  5 +++-
>  2 files changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3ef37202ee37..e46da41940b9 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -293,9 +293,12 @@ struct tcpm_port {
>  	unsigned int operating_snk_mw;
>  	bool update_sink_caps;
>  
> -	/* Requested current / voltage */
> +	/* Set current / voltage */
>  	u32 current_limit;
>  	u32 supply_voltage;
> +	/* current / voltage requested to partner */
> +	u32 req_current_limit;
> +	u32 req_supply_voltage;

I don't see a clear delineation where, when, and why supply_voltage vs. req_supply_voltage
and current_limit vs. req_current_limit is used. Maybe someone else does and can give a
Reviewed-by: tag, but for my part I'll have to spend some time trying to understand
what each variable and its use now means. Overall this suggests that the code may now be
a bit fragile. If those two sets of variables are now indeed needed, I think it would help
to have a detailed explanation for each use. This would help reducing the probability
of errors if the code has to be touched again in the future.

>  
>  	/* Used to export TA voltage and current */
>  	struct power_supply *psy;
> @@ -323,13 +326,27 @@ struct tcpm_port {
>  	struct pd_mode_data mode_data;
>  	struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
>  	struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
> -
>  	/* Deadline in jiffies to exit src_try_wait state */
>  	unsigned long max_wait;
>  
>  	/* port belongs to a self powered device */
>  	bool self_powered;
>  
> +	/*
> +	 * Honour psnkstdby after accept is received.
> +	 * However, in this case it has to be made sure that the tSnkStdby (15
> +	 * msec) is met.
> +	 *
> +	 * When not set, port requests PD_P_SNK_STDBY_5V upon entering SNK_DISCOVERY and
> +	 * the actual currrent limit after RX of PD_CTRL_PSRDY for PD link,
> +	 * SNK_READY for non-pd link.
> +	 *
> +	 * When set, port requests CC advertisement based current limit during
> +	 * SNK_DISCOVERY, current gets limited to PD_P_SNK_STDBY_5V during SNK_TRANSITION_SINK,
> +	 * PD based current limit gets set after RX of PD_CTRL_PSRDY.
> +	 */
> +	bool psnkstdby_after_accept;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	struct dentry *dentry;
>  	struct mutex logbuffer_lock;	/* log buffer access lock */
> @@ -1787,9 +1804,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  		switch (port->state) {
>  		case SNK_TRANSITION_SINK:
>  			if (port->vbus_present) {
> -				tcpm_set_current_limit(port,
> -						       port->current_limit,
> -						       port->supply_voltage);
> +				tcpm_set_current_limit(port, port->req_current_limit,
> +						       port->req_supply_voltage);
>  				port->explicit_contract = true;
>  				tcpm_set_state(port, SNK_READY, 0);
>  			} else {
> @@ -1861,8 +1877,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  			break;
>  		case SNK_NEGOTIATE_PPS_CAPABILITIES:
>  			port->pps_data.active = true;
> -			port->supply_voltage = port->pps_data.out_volt;
> -			port->current_limit = port->pps_data.op_curr;
> +			port->req_supply_voltage = port->pps_data.out_volt;
> +			port->req_current_limit = port->pps_data.op_curr;
>  			tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
>  			break;
>  		case SOFT_RESET_SEND:
> @@ -2463,8 +2479,8 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
>  			 flags & RDO_CAP_MISMATCH ? " [mismatch]" : "");
>  	}
>  
> -	port->current_limit = ma;
> -	port->supply_voltage = mv;
> +	port->req_current_limit = ma;
> +	port->req_supply_voltage = mv;
>  
>  	return 0;
>  }
> @@ -3235,9 +3251,11 @@ static void run_state_machine(struct tcpm_port *port)
>  		break;
>  	case SNK_DISCOVERY:
>  		if (port->vbus_present) {
> -			tcpm_set_current_limit(port,
> -					       tcpm_get_current_limit(port),
> -					       5000);
> +			if (port->psnkstdby_after_accept || tcpm_get_current_limit(port) <=
> +			    PD_P_SNK_STDBY_5V)
> +				tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
> +			else
> +				tcpm_set_current_limit(port, PD_P_SNK_STDBY_5V, 5000);
>  			tcpm_set_charge(port, true);
>  			tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
>  			break;
> @@ -3318,6 +3336,10 @@ static void run_state_machine(struct tcpm_port *port)
>  		}
>  		break;
>  	case SNK_TRANSITION_SINK:
> +		if (port->psnkstdby_after_accept)
> +			tcpm_set_current_limit(port, tcpm_get_current_limit(port) >
> +					       PD_P_SNK_STDBY_5V ? PD_P_SNK_STDBY_5V :
> +					       tcpm_get_current_limit(port), 5000);
>  	case SNK_TRANSITION_SINK_VBUS:
>  		tcpm_set_state(port, hard_reset_state(port),
>  			       PD_T_PS_TRANSITION);
> @@ -3331,6 +3353,10 @@ static void run_state_machine(struct tcpm_port *port)
>  			port->pwr_opmode = TYPEC_PWR_MODE_PD;
>  		}
>  
> +		/* Set current limit for NON-PD link when psnkstdby_after_accept is not set*/
> +		if (!port->pd_capable && !port->psnkstdby_after_accept)
> +			tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
> +
>  		tcpm_swap_complete(port, 0);
>  		tcpm_typec_connect(port);
>  		tcpm_check_send_discover(port);
> @@ -4513,6 +4539,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>  	port->typec_caps.type = ret;
>  	port->port_type = port->typec_caps.type;
>  
> +	port->psnkstdby_after_accept = fwnode_property_read_bool(fwnode, "psnkstdby-after-accept");
> +

I think this will need to be documented.

Guenter

>  	if (port->port_type == TYPEC_PORT_SNK)
>  		goto sink;
>  
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index b6c233e79bd4..6bd70f77045e 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -483,5 +483,8 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
>  #define PD_N_HARD_RESET_COUNT	2
>  
> -#define PD_T_BIST_CONT_MODE	50 /* 30 - 60 ms */
> +#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
> +
> +#define PD_P_SNK_STDBY_5V	500	/* 2500 mw - 500mA @ 5V */
> +
>  #endif /* __LINUX_USB_PD_H */
>
Badhri Jagan Sridharan Aug. 5, 2020, 12:18 a.m. UTC | #3
Hi Guenter,

Your concern is very valid ! I was myself pondering at the
significance and meaning of
current_limit,  supply_voltage. I will pen down what I understood from
reading the code.
If it makes sense, I will add the documentation to the change as well.
In TOT code i.e. without the patch in discussion, current_limit and
supply_voltage gets
in three places.
1. tcpm_set_current_limit --------------> Actual current_limit
/voltage requested to be set
                                                             in the hardware.
2. tcpm_pd_build_request --------------> current_limit /voltage
requested to the port partner.
                                                              Not yet
accepted by port partner
3. case PD_CTRL_ACCEPT:  ----------> PPS path: new
current_limit/voltage to be set in hw &
                                                               already
accepted by the port partner.
                switch (port->state) {
                case SNK_NEGOTIATE_CAPABILITIES:
                        port->pps_data.active = false;
                        tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
                        break;
                case SNK_NEGOTIATE_PPS_CAPABILITIES:
                        port->pps_data.active = true;
                        port->supply_voltage = port->pps_data.out_volt;
                        port->current_limit = port->pps_data.op_curr;
                        tcpm_set_state(port, SNK_TRANSITION_SINK, 0);

PPS path safely differentiates between new current_limit &
supply_voltage requested
to the port-partner and current_limit & supply_voltage  requested to
be set in the
hardware by caching the new requested values to partner in
port->pps_data.op_curr/_volt (in tcpm_pd_select_pps_apdo) and updates
port->supply_voltage, port->current_limit only when the port-partner responds
with an accept packet (as pointed in the above code stub).

However, when NOT IN PPS, TCPM code updates  port->current_limit &
port->supply_voltage directly in tcpm_pd_build_request where the partner can
still REJECT the request and port->current_limit & port->supply_voltage
might go out of sync. So, there is already a need to introduce new variables to
cache the requested values to port partners.

Now coming to addressing  pSnkStdby, Once the partner accepts the new request,
PD spec wants the sink to limit the power consumption to pSnkStdby (2.5W) till
source sends the PS_RDY. i.e. while TCPM waits in SNK_TRANSITION_SINK,
the sink's power consumption has to be limited to 2.5W. Sink can enforce or draw
new partner accepted current & voltage at the hardware level only
after PS_RDY is received from the port-partner.

With the patch that I am submitting, I am hoping to cleanly
differentiate between the
current limit/voltage requested to be set in hardware(
tcpm_set_current_limit) Vs
the current limit/voltage requested to the port partner.

 current_limit, supply_voltage -> Requested to be set in hardware.
 req_current_limit, req_supply_voltage -> Newly requested value to the
port partner.

Perhaps doing the following alleviates current_limit/supply_voltage ambiguity:
1. renaming current_limit, supply_voltage to hw_current_limit, hw_supply_voltage
2. making a comment saying hw_current_limit, hw_supply_voltage should only
be set in tcpm_set_current_limit.
3. Instead of calling requested values to port-partner as
req_current_limit/req_supply_voltage, maybe partner_req_current_limit,
partner_req_supply_voltage is a better name.
4. Adding a comment to state significance of hw_current_limit,
hw_supply_voltage,
partner_req_current_limit, partner_req_supply_voltage in tcpm struct.

Let me know if it makes sense I will update the patch (may be even
split if it makes
sense) and resend.

Thanks,
Badhri




On Tue, Aug 4, 2020 at 9:59 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 8/3/20 11:51 PM, Badhri Jagan Sridharan wrote:
> >>From PD Spec:
> > The Sink Shall transition to Sink Standby before a positive or
> > negative voltage transition of VBUS. During Sink Standby
> > the Sink Shall reduce its power draw to pSnkStdby. This allows
> > the Source to manage the voltage transition as well as
> > supply sufficient operating current to the Sink to maintain PD
> > operation during the transition. The Sink Shall
> > complete this transition to Sink Standby within tSnkStdby
> > after evaluating the Accept Message from the Source. The
> > transition when returning to Sink operation from Sink Standby
> > Shall be completed within tSnkNewPower. The
> > pSnkStdby requirement Shall only apply if the Sink power draw
> > is higher than this level.
> >
> > The above requirement needs to be met to prevent hard resets
> > from port partner.
> >
> > Introducing psnkstdby_after_accept flag to accommodate systems
> > where the input current limit loops are not fast enough to meet
> > tSnkStby(15 msec).
> >
> > When not set, port requests PD_P_SNK_STDBY upon entering SNK_DISCOVERY and
> > the actual currrent limit after RX of PD_CTRL_PSRDY for PD link,
> > SNK_READY for non-pd link.
> >
> > When set, port requests CC advertisement based current limit during
> > SNK_DISCOVERY, current gets limited to PD_P_SNK_STDBY during
> > SNK_TRANSITION_SINK, PD based current limit gets set after RX of
> > PD_CTRL_PSRDY. However, in this case it has to be made sure that the
> > tSnkStdby (15 msec) is met.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> >  drivers/usb/typec/tcpm/tcpm.c | 52 +++++++++++++++++++++++++++--------
> >  include/linux/usb/pd.h        |  5 +++-
> >  2 files changed, 44 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 3ef37202ee37..e46da41940b9 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -293,9 +293,12 @@ struct tcpm_port {
> >       unsigned int operating_snk_mw;
> >       bool update_sink_caps;
> >
> > -     /* Requested current / voltage */
> > +     /* Set current / voltage */
> >       u32 current_limit;
> >       u32 supply_voltage;
> > +     /* current / voltage requested to partner */
> > +     u32 req_current_limit;
> > +     u32 req_supply_voltage;
>
> I don't see a clear delineation where, when, and why supply_voltage vs. req_supply_voltage
> and current_limit vs. req_current_limit is used. Maybe someone else does and can give a
> Reviewed-by: tag, but for my part I'll have to spend some time trying to understand
> what each variable and its use now means. Overall this suggests that the code may now be
> a bit fragile. If those two sets of variables are now indeed needed, I think it would help
> to have a detailed explanation for each use. This would help reducing the probability
> of errors if the code has to be touched again in the future.
>
> >
> >       /* Used to export TA voltage and current */
> >       struct power_supply *psy;
> > @@ -323,13 +326,27 @@ struct tcpm_port {
> >       struct pd_mode_data mode_data;
> >       struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
> >       struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
> > -
> >       /* Deadline in jiffies to exit src_try_wait state */
> >       unsigned long max_wait;
> >
> >       /* port belongs to a self powered device */
> >       bool self_powered;
> >
> > +     /*
> > +      * Honour psnkstdby after accept is received.
> > +      * However, in this case it has to be made sure that the tSnkStdby (15
> > +      * msec) is met.
> > +      *
> > +      * When not set, port requests PD_P_SNK_STDBY_5V upon entering SNK_DISCOVERY and
> > +      * the actual currrent limit after RX of PD_CTRL_PSRDY for PD link,
> > +      * SNK_READY for non-pd link.
> > +      *
> > +      * When set, port requests CC advertisement based current limit during
> > +      * SNK_DISCOVERY, current gets limited to PD_P_SNK_STDBY_5V during SNK_TRANSITION_SINK,
> > +      * PD based current limit gets set after RX of PD_CTRL_PSRDY.
> > +      */
> > +     bool psnkstdby_after_accept;
> > +
> >  #ifdef CONFIG_DEBUG_FS
> >       struct dentry *dentry;
> >       struct mutex logbuffer_lock;    /* log buffer access lock */
> > @@ -1787,9 +1804,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> >               switch (port->state) {
> >               case SNK_TRANSITION_SINK:
> >                       if (port->vbus_present) {
> > -                             tcpm_set_current_limit(port,
> > -                                                    port->current_limit,
> > -                                                    port->supply_voltage);
> > +                             tcpm_set_current_limit(port, port->req_current_limit,
> > +                                                    port->req_supply_voltage);
> >                               port->explicit_contract = true;
> >                               tcpm_set_state(port, SNK_READY, 0);
> >                       } else {
> > @@ -1861,8 +1877,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> >                       break;
> >               case SNK_NEGOTIATE_PPS_CAPABILITIES:
> >                       port->pps_data.active = true;
> > -                     port->supply_voltage = port->pps_data.out_volt;
> > -                     port->current_limit = port->pps_data.op_curr;
> > +                     port->req_supply_voltage = port->pps_data.out_volt;
> > +                     port->req_current_limit = port->pps_data.op_curr;
> >                       tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> >                       break;
> >               case SOFT_RESET_SEND:
> > @@ -2463,8 +2479,8 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
> >                        flags & RDO_CAP_MISMATCH ? " [mismatch]" : "");
> >       }
> >
> > -     port->current_limit = ma;
> > -     port->supply_voltage = mv;
> > +     port->req_current_limit = ma;
> > +     port->req_supply_voltage = mv;
> >
> >       return 0;
> >  }
> > @@ -3235,9 +3251,11 @@ static void run_state_machine(struct tcpm_port *port)
> >               break;
> >       case SNK_DISCOVERY:
> >               if (port->vbus_present) {
> > -                     tcpm_set_current_limit(port,
> > -                                            tcpm_get_current_limit(port),
> > -                                            5000);
> > +                     if (port->psnkstdby_after_accept || tcpm_get_current_limit(port) <=
> > +                         PD_P_SNK_STDBY_5V)
> > +                             tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
> > +                     else
> > +                             tcpm_set_current_limit(port, PD_P_SNK_STDBY_5V, 5000);
> >                       tcpm_set_charge(port, true);
> >                       tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
> >                       break;
> > @@ -3318,6 +3336,10 @@ static void run_state_machine(struct tcpm_port *port)
> >               }
> >               break;
> >       case SNK_TRANSITION_SINK:
> > +             if (port->psnkstdby_after_accept)
> > +                     tcpm_set_current_limit(port, tcpm_get_current_limit(port) >
> > +                                            PD_P_SNK_STDBY_5V ? PD_P_SNK_STDBY_5V :
> > +                                            tcpm_get_current_limit(port), 5000);
> >       case SNK_TRANSITION_SINK_VBUS:
> >               tcpm_set_state(port, hard_reset_state(port),
> >                              PD_T_PS_TRANSITION);
> > @@ -3331,6 +3353,10 @@ static void run_state_machine(struct tcpm_port *port)
> >                       port->pwr_opmode = TYPEC_PWR_MODE_PD;
> >               }
> >
> > +             /* Set current limit for NON-PD link when psnkstdby_after_accept is not set*/
> > +             if (!port->pd_capable && !port->psnkstdby_after_accept)
> > +                     tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
> > +
> >               tcpm_swap_complete(port, 0);
> >               tcpm_typec_connect(port);
> >               tcpm_check_send_discover(port);
> > @@ -4513,6 +4539,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> >       port->typec_caps.type = ret;
> >       port->port_type = port->typec_caps.type;
> >
> > +     port->psnkstdby_after_accept = fwnode_property_read_bool(fwnode, "psnkstdby-after-accept");
> > +
>
> I think this will need to be documented.
>
> Guenter
>
> >       if (port->port_type == TYPEC_PORT_SNK)
> >               goto sink;
> >
> > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> > index b6c233e79bd4..6bd70f77045e 100644
> > --- a/include/linux/usb/pd.h
> > +++ b/include/linux/usb/pd.h
> > @@ -483,5 +483,8 @@ static inline unsigned int rdo_max_power(u32 rdo)
> >  #define PD_N_CAPS_COUNT              (PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
> >  #define PD_N_HARD_RESET_COUNT        2
> >
> > -#define PD_T_BIST_CONT_MODE  50 /* 30 - 60 ms */
> > +#define PD_T_BIST_CONT_MODE  50      /* 30 - 60 ms */
> > +
> > +#define PD_P_SNK_STDBY_5V    500     /* 2500 mw - 500mA @ 5V */
> > +
> >  #endif /* __LINUX_USB_PD_H */
> >
>
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3ef37202ee37..e46da41940b9 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -293,9 +293,12 @@  struct tcpm_port {
 	unsigned int operating_snk_mw;
 	bool update_sink_caps;
 
-	/* Requested current / voltage */
+	/* Set current / voltage */
 	u32 current_limit;
 	u32 supply_voltage;
+	/* current / voltage requested to partner */
+	u32 req_current_limit;
+	u32 req_supply_voltage;
 
 	/* Used to export TA voltage and current */
 	struct power_supply *psy;
@@ -323,13 +326,27 @@  struct tcpm_port {
 	struct pd_mode_data mode_data;
 	struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
 	struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
-
 	/* Deadline in jiffies to exit src_try_wait state */
 	unsigned long max_wait;
 
 	/* port belongs to a self powered device */
 	bool self_powered;
 
+	/*
+	 * Honour psnkstdby after accept is received.
+	 * However, in this case it has to be made sure that the tSnkStdby (15
+	 * msec) is met.
+	 *
+	 * When not set, port requests PD_P_SNK_STDBY_5V upon entering SNK_DISCOVERY and
+	 * the actual currrent limit after RX of PD_CTRL_PSRDY for PD link,
+	 * SNK_READY for non-pd link.
+	 *
+	 * When set, port requests CC advertisement based current limit during
+	 * SNK_DISCOVERY, current gets limited to PD_P_SNK_STDBY_5V during SNK_TRANSITION_SINK,
+	 * PD based current limit gets set after RX of PD_CTRL_PSRDY.
+	 */
+	bool psnkstdby_after_accept;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -1787,9 +1804,8 @@  static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 		switch (port->state) {
 		case SNK_TRANSITION_SINK:
 			if (port->vbus_present) {
-				tcpm_set_current_limit(port,
-						       port->current_limit,
-						       port->supply_voltage);
+				tcpm_set_current_limit(port, port->req_current_limit,
+						       port->req_supply_voltage);
 				port->explicit_contract = true;
 				tcpm_set_state(port, SNK_READY, 0);
 			} else {
@@ -1861,8 +1877,8 @@  static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 			break;
 		case SNK_NEGOTIATE_PPS_CAPABILITIES:
 			port->pps_data.active = true;
-			port->supply_voltage = port->pps_data.out_volt;
-			port->current_limit = port->pps_data.op_curr;
+			port->req_supply_voltage = port->pps_data.out_volt;
+			port->req_current_limit = port->pps_data.op_curr;
 			tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
 			break;
 		case SOFT_RESET_SEND:
@@ -2463,8 +2479,8 @@  static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
 			 flags & RDO_CAP_MISMATCH ? " [mismatch]" : "");
 	}
 
-	port->current_limit = ma;
-	port->supply_voltage = mv;
+	port->req_current_limit = ma;
+	port->req_supply_voltage = mv;
 
 	return 0;
 }
@@ -3235,9 +3251,11 @@  static void run_state_machine(struct tcpm_port *port)
 		break;
 	case SNK_DISCOVERY:
 		if (port->vbus_present) {
-			tcpm_set_current_limit(port,
-					       tcpm_get_current_limit(port),
-					       5000);
+			if (port->psnkstdby_after_accept || tcpm_get_current_limit(port) <=
+			    PD_P_SNK_STDBY_5V)
+				tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
+			else
+				tcpm_set_current_limit(port, PD_P_SNK_STDBY_5V, 5000);
 			tcpm_set_charge(port, true);
 			tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
 			break;
@@ -3318,6 +3336,10 @@  static void run_state_machine(struct tcpm_port *port)
 		}
 		break;
 	case SNK_TRANSITION_SINK:
+		if (port->psnkstdby_after_accept)
+			tcpm_set_current_limit(port, tcpm_get_current_limit(port) >
+					       PD_P_SNK_STDBY_5V ? PD_P_SNK_STDBY_5V :
+					       tcpm_get_current_limit(port), 5000);
 	case SNK_TRANSITION_SINK_VBUS:
 		tcpm_set_state(port, hard_reset_state(port),
 			       PD_T_PS_TRANSITION);
@@ -3331,6 +3353,10 @@  static void run_state_machine(struct tcpm_port *port)
 			port->pwr_opmode = TYPEC_PWR_MODE_PD;
 		}
 
+		/* Set current limit for NON-PD link when psnkstdby_after_accept is not set*/
+		if (!port->pd_capable && !port->psnkstdby_after_accept)
+			tcpm_set_current_limit(port, tcpm_get_current_limit(port), 5000);
+
 		tcpm_swap_complete(port, 0);
 		tcpm_typec_connect(port);
 		tcpm_check_send_discover(port);
@@ -4513,6 +4539,8 @@  static int tcpm_fw_get_caps(struct tcpm_port *port,
 	port->typec_caps.type = ret;
 	port->port_type = port->typec_caps.type;
 
+	port->psnkstdby_after_accept = fwnode_property_read_bool(fwnode, "psnkstdby-after-accept");
+
 	if (port->port_type == TYPEC_PORT_SNK)
 		goto sink;
 
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index b6c233e79bd4..6bd70f77045e 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -483,5 +483,8 @@  static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
 #define PD_N_HARD_RESET_COUNT	2
 
-#define PD_T_BIST_CONT_MODE	50 /* 30 - 60 ms */
+#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
+
+#define PD_P_SNK_STDBY_5V	500	/* 2500 mw - 500mA @ 5V */
+
 #endif /* __LINUX_USB_PD_H */