ChiYuan Huang <u0084500@xxxxxxxxx> 於 2020年10月7日 週三 上午1:39寫道: > > Jun Li <lijun.kernel@xxxxxxxxx> 於 2020年10月7日 週三 上午12:52寫道: > > > > ChiYuan Huang <u0084500@xxxxxxxxx> 于2020年10月6日周二 下午12:38写道: > > > > > > Guenter Roeck <linux@xxxxxxxxxxxx> 於 2020年10月5日 週一 下午11:30寫道: > > > > > > > > On 10/5/20 4:08 AM, Greg KH wrote: > > > > [ ... ] > > > > >>> What ever happened with this patch, is there still disagreement? > > > > >>> > > > > >> > > > > >> Yes, there is. I wouldn't have added the conditional without reason, > > > > >> and I am concerned that removing it entirely will open another problem. > > > > >> Feel free to apply, though - I can't prove that my concern is valid, > > > > >> and after all we'll get reports from the field later if it is. > > > > > > > > > > Ok, can I get an ack so I know who to come back to in the future if > > > > > there are issues? :) > > > > > > > > > > > > > Not from me, for the reasons I stated. I would be ok with something like: > > > > > > > > - if (tcpm_port_is_disconnected(port)) > > > > + if (tcpm_port_is_disconnected(port) || > > > > + (tcpm_cc_is_open(port->cc1) && tcpm_cc_is_open(port->cc2))) > > > > > > > > to narrow down the condition. > > > > > > I have tried the above comment and It doesn't work. > > > How about to change the judgement like as below > > > > > > - if (tcpm_port_is_disconnected(port)) > > > + if (tcpm_port_is_disconnected(port) || !port->vbus_present) > > > > > > The hard_reset_count not reset issue is following by the below order > > > 1. VBUS off ( at the same time, cc is still detected as attached) > > > port->attached become false and cc is not open > > > 2. After that, cc detached. > > > due to port->attached is false, tcpm_detach() directly return. > > > > If tcpm_detach() return directly, then how your patch can reset > > hard_reset_count? > > > Yes, it can. We know vbus_present change from true to false and cc > detach both trigger tcpm_detach. > My change is whenever tcpm_detach to be called, hard_reset_count will > be reset to zero. > > > I am seeing the same issue on my platform, the proposed change: > > - if (tcpm_port_is_disconnected(port)) > > - port->hard_reset_count = 0; > > + port->hard_reset_count = 0; > > can't resolve it on my platform. > > > I'm not sure what's your condition. Could you directly paste the tcpm > log for the check? > > How about reset hard_reset_count in SNK_READY? > > @@ -3325,6 +3329,7 @@ static void run_state_machine(struct tcpm_port *port) > > case SNK_READY: > > port->try_snk_count = 0; > > port->update_sink_caps = false; > > + port->hard_reset_count = 0; > > if (port->explicit_contract) { > > typec_set_pwr_opmode(port->typec_port, > > TYPEC_PWR_MODE_PD); > > > > can this resolve your problem? > I'm not sure. It need to have a try, then I can answer you. > But from USBPD spec, the hard_reset_count need to reset zero only when > 1. At src state, pe_src_send_cap and receive GoodCRC > 2. At snk state, pe_snk_evaluate_cap need to reset hard_reset_count > > > > Li Jun > > > > > > And that's why hard_reset_count is not reset to 0. I tried in snk_ready to reset hard_reset_count. At normal case, it works. But it seems still the possible fail case like as below. 200ms -> cc debounce max time 240ms -> snk_waitcap max time If the plugin/out period is between (200+240) and (200+ 2* 240)ms , and the src side plug out like as the described scenario. >From this case, hard_reset_count may still 1. And we expect the next plugin hard_reset_count is 0. But not, actually it never reset. So at next plugin, only one hard_reset will be sent and reach hard_reset_count max (2). This case is hard to reproduce. But actually it's possible. > > > > > > > > Guenter