Hi, Sorry, I modify it to plain text mode and send again 2018-03-03 2:39 GMT+08:00 李書帆 <leechu729@xxxxxxxxx>: > > Hi Jun, > > I think this operation should be moved to vendor's operation after rechecking the specification. > > 2018-03-02 22:38 GMT+08:00 Jun Li <jun.li@xxxxxxx>: >> >> Hi >> > -----Original Message----- >> > From: shufan_lee(李書帆) [mailto:shufan_lee@xxxxxxxxxxx] >> > Sent: 2018年3月1日 19:54 >> > To: Jun Li <jun.li@xxxxxxx>; ShuFanLee <leechu729@xxxxxxxxx>; >> > heikki.krogerus@xxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx; greg@xxxxxxxxx >> > Cc: cy_huang(黃啟原) <cy_huang@xxxxxxxxxxx>; >> > linux-kernel@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx >> > <linux-imx@xxxxxxx> >> > Subject: RE: [PATCH] staging: typec: handle vendor defined part and modify >> > drp toggling flow >> > >> > Hi Jun, >> > >> > > -----Original Message----- >> > > From: Jun Li [mailto:jun.li@xxxxxxx] >> > > Sent: Thursday, March 01, 2018 6:06 PM >> > > To: shufan_lee(李書帆); ShuFanLee; heikki.krogerus@xxxxxxxxxxxxxxx; >> > > linux@xxxxxxxxxxxx; greg@xxxxxxxxx >> > > Cc: cy_huang(黃啟原); linux-kernel@xxxxxxxxxxxxxxx; >> > > linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx >> > > Subject: RE: [PATCH] staging: typec: handle vendor defined part and >> > > modify drp toggling flow >> > > >> > > Hi Shufan >> > > >> > > Please don't top posting >> > > >> > > -----Original Message----- >> > > From: shufan_lee(李書帆) [mailto:shufan_lee@xxxxxxxxxxx] >> > > Sent: 2018年3月1日 16:49 >> > > To: Jun Li <jun.li@xxxxxxx>; ShuFanLee <leechu729@xxxxxxxxx>; >> > > heikki.krogerus@xxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx; greg@xxxxxxxxx >> > > Cc: cy_huang(黃啟原) <cy_huang@xxxxxxxxxxx>; >> > > linux-kernel@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx >> > > <linux-imx@xxxxxxx> >> > > Subject: RE: [PATCH] staging: typec: handle vendor defined part and >> > > modify drp toggling flow >> > > >> > > Hi Jun, >> > > >> > > The attachment is waveform of the condition we met but I'm not sure >> > > whether you can download the attachment. >> > > I add log in RT1711H it shows as following: >> > > >> > > You don't need add log by your own. >> > > There is tcpm(./drivers/usb/typec/tcpm.c) log for debug already, which can >> > show all the events and state transitions, you can get it by below command >> > as I commented: >> > > >> > > cat /sys/kernel/debug/tcpm/xxxxx(your tcpc i2c device) >> > > >> > After applying your patch[2], TCPM log is as following: >> >> I assume you also applied my patch [1]. >> [1] https://www.spinics.net/lists/devicetree/msg216851.html >> > Yes, this patch is also applied. >> >> > >> > [ 53.050602] CC1: 0 -> 2, CC2: 0 -> 0 [state DRP_TOGGLING, polarity 0, >> > connected] >> > [ 53.050613] state change DRP_TOGGLING -> SRC_ATTACH_WAIT >> > [ 53.050678] pending state change SRC_ATTACH_WAIT -> SNK_TRY @ >> > 200 ms >> > => Rd is plugged out >> > [ 53.178804] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_ATTACH_WAIT, polarity 0, >> > disconnected] >> > [ 53.178815] state change SRC_ATTACH_WAIT -> SRC_UNATTACHED >> > => Rd is plugged in >> > [ 53.178874] Start DRP toggling >> > [ 53.188472] CC1: 0 -> 0, CC2: 0 -> 0 [state DRP_TOGGLING, polarity 0, >> > disconnected] >> >> 1. The plug out and then plug in happens in 10ms? (53.188472 - 53.178804) >> Was this done manually? Or by some special test method? > > It's done manually. If you can download the waveform attached in previous email, you can see Rd is plugged out/in within ms level. > We connect a bridge board with test points of CC1, CC2 and GND to our platform's Typec receptacle. Then we can simulate a device plugging in/out by connecting/disconnecting a 5.1k resistor between CC1 and GND. > >> >> 2. This is all tcpm log if you finally keep the Rd-device connected on typec >> port? There is no more tcpm log after 53.188472 if you plug in Rd-device >> >> and don't remove it? > > Yes, RT1711H reports open/open to TCPM in drp_toggling state and stops toggling. Currently, TCPM does not restart drp_toggling if it receives open/open in drp_toggling state. > I remember the specification of TypeC does not depict what TCPM should do if it receives open/open in drp_toggling state. > Maybe restart toggling is an option. > >> 3. If the answer of Q2 is yes, then I must ask why you tcpc chip+internal firmware >> can't report further cc change event after your drp toggling starts to present Rp(I know >> it firstly present Rd after you write to LOOK4CONNECTION, but then it should change >> to present Rp, so it should be able to detect the Rd-device finally) > > Because RT1711H's internal firmware changes to attached state when Rd is re-plugged in(cc level is default Rp (around 400mV) now). > Then, LOOK4CONNECTION is set and RT1711H changes CCx to Rd that makes it reports open/open(because cc level changes from default Rp(around 400mV) to 0mV) >> >> >> > >> > If TCPM does not enter SRC_ATTACHED state, RC.DRP will not be cleared. >> >> In this case, you don’t need clear RC.DRP, see TCPCI spec: >> "Figure 4-18. Sink Disconnect" >> TCPM sink doesn't clear it in whole sequence, just directly set it: >> Restart DRP Toggling >> PC.AutoDischargeDisconnect=0b >> Set RC.DRP=1b (DRP) >> Set RC.CC1=10b (Rd) >> Set RC.CC2=10b (Rd) >> COMMAND.Look4Connection (DRP toggle) >> >> >> > When TCPM writes Rd/Rd or Rp/Rp in the drp_toggling function, it does not >> > take effect until LOOK4CONNECTION command is set. >> > The above condition causes RT1711H reports open/open at [53.188472] >> > >> > > [ 914.937340] typec_rt1711h 2-004e: __rt1711h_irq_handler [ >> > > 914.943838] typec_rt1711h 2-004e: __tcpm_get_cc cc1 = Open, cc2 = >> > Open >> > > => Device(Rd) is plugged out >> > > >> > > [ 914.958041] typec_rt1711h 2-004e: tcpm_set_pd_rx 0 [ 914.963011] >> > > typec_rt1711h 2-004e: tcpm_set_vbus vbus = 0 [ 914.968407] >> > > typec_rt1711h >> > > 2-004e: tcpm_set_vbus chg is already 0 [ 914.974541] typec_rt1711h >> > 2-004e: >> > > tcpm_set_vconn vconn is already 0 [ 914.980921] typec_rt1711h 2-004e: >> > > tcpm_set_current_limit 0 ma, 0 mv (not implemented) [ 914.988894] >> > > typec_rt1711h 2-004e: tcpm_set_polarity Polarity_CC1 [ 915.003201] >> > > typec_rt1711h 2-004e: tcpm_set_roles Source Host [ 915.009264] >> > > typec_rt1711h 2-004e: tcpm_start_drp_toggling => state_machine_work >> > of >> > > TCPM calls start_drp_toggling function but does not set >> > > LOOK4CONNECTION command yet => (Note1) Device(Rd) is plugged in >> > > (RT1711H's internal firmware detects Rd plugged in. cc_change is >> > > triggered and it will be vRd-connected at this moment) => TCPM writes >> > > LOOK4CONNECTION command >> > > - Because RC.DRP is still 1, RT1711H will not pull cc1/cc2 to Rd while >> > > writing Rd/Rd to RC.CC1/RC.CC2. >> > > - (Note2) Right after LOOK4CONNECTION command is written, RT1711H >> > > pulls CC to Rd's level (because RC.Role is Rd/Rd), stop toggling >> > > (because >> > > device(Rd) is already connected) and CC status will be open/open now >> > > (because RT1711H presents Rd and device is connected(Rd)) >> > > >> > > [ 915.037263] typec_rt1711h 2-004e: __tcpm_get_cc cc1 = Open, cc2 = >> > > Open => Enter RT1711H's irq handler and it reports open/open >> > > >> > > I think the point is to write RC.DRP = 0 at the beginning of >> > > drp_toggling so that RT1711H will pull cc1/cc2 to Rd while writing >> > > Rd/Rd to RC.CC1/RC.CC2 This operation will make RT1711H's internal >> > > firmware restarts from disconnected state and toggles correctly. >> > > >> > > According to TCPCI spec (4.4.5.2): >> > > It is recommended the TCPM write ROLE_CONTROL.DRP=0 before writing >> > to >> > > POWER_CONTROL.AutoDischargeDisconnect and starting the DRP toggling >> > > using COMMAND.Look4Connection Restart DRP Toggling => It is >> > > recommended the TCPM write ROLE_CONTROL.DRP=0 here Set >> > > >> > > This statement is_not_ recommend you do this(RC.DRP=0) for start drp >> > toggling, Please carefully check the whole sentence: >> > > "... as shown in Figure 4-16, " >> > > If you look at "Figure 4-16. DRP Initialization and Connection Detection" >> > > It gives clear drp toggling start operations: >> > > >> > > Set TCPC to DRP >> > > - Read PS.TCPCInitializationStatus >> > > - Write ROLE_CONTROL >> > > - RC.DRP = 1b >> > > - RC.CC2=01b or 10b >> > > - RC.CC1=01b or 10b >> > > - RC.CC1=RC.CC2 >> > > - Write COMMAND.Look4ConnectionPS. >> > > >> > > Above is all operations required to start drp toggling. You also can see >> > RC.CCx = 01b or 10b, not fixed to be Rd, right? >> > Yes, this one should be like your patch[07/12]. Write Rd or Rp to RC.CCx >> > according to the cc parameter of drp_toggling function. >> > > >> > > Go on to check the Figure 4-16 >> > > After debounce, we need do following: >> > > >> > > ConnectionDetermine CC & VCONN >> > > - Write RC.CC1 & RC.CC2 per decision >> > > - Write RC.DRP=0 >> > > - Write TCPC_CONTROl.PlugOrientation >> > > - Write PC.AutoDischargeDisconnect=1 >> > > & PC.EnableVconnConnection >> > > >> > > Current existing tcpm+tcpci will not clear RC.DRP after attach, That means >> > RC.DRP clear may be done after attach, not in start_drp_toggling. >> > > I am not sure if this can resolve your problem, but I think it deserve a try, >> > you can follow above operation sequence while entering attach state, refer >> > to my patch[2]: >> > > >> > > [2] >> > > >> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw >> > ww >> > > .spinics.net%2Flists%2Fdevicetree%2Fmsg216852.html&data=02%7C01%7 >> > Cjun. >> > > >> > li%40nxp.com%7C9117425550d24ddc86d808d57f6b1b4e%7C686ea1d3bc2b >> > 4c6fa92c >> > > >> > d99c5c301635%7C0%7C0%7C636555020366483456&sdata=9%2BywYl%2BR >> > PYtk60Wg6p >> > > R63cCW2AnRXs%2BrINvvqUpqL18%3D&reserved=0 >> > > >> > > diff --git a/drivers/staging/typec/tcpci.c >> > > b/drivers/staging/typec/tcpci.c index 530a5d7..7145771 100644 >> > > --- a/drivers/staging/typec/tcpci.c >> > > +++ b/drivers/staging/typec/tcpci.c >> > > @@ -184,6 +184,7 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc, >> > > enum typec_cc_polarity polarity) { >> > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc); >> > > + unsigned int reg; >> > > int ret; >> > > >> > > ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL, @@ >> > -192,6 +193,20 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc, >> > > if (ret < 0) >> > > return ret; >> > > >> > > + ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, ®); >> > > + if (ret < 0) >> > > + return ret; >> > > + >> > > + if (polarity == TYPEC_POLARITY_CC2) >> > > + ret = TCPC_ROLE_CTRL_CC1_SHIFT; >> > > + else >> > > + ret = TCPC_ROLE_CTRL_CC2_SHIFT; >> > > + reg |= TCPC_ROLE_CTRL_CC_OPEN << ret; >> > > + reg &= ~TCPC_ROLE_CTRL_DRP; >> > > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); >> > > + if (ret < 0) >> > > + return ret; >> > > + >> > > return 0; >> > > } >> > > >> > > PC.AutoDischargeDisconnect=0b Set RC.DRP=1b (DRP) Set RC.RpValue=00b >> > > (smallest Rp to save power) Set RC.CC1=01b Set >> > > RC.CC2=01bCOMMAND.Look4ConnectionNo >> > > >> > > It seems like it is not a general case here (because it is only >> > > recommended but not necessary), we can move it to vendor_ops in the >> > next patch. >> > > >> > > The TCPM should be able to cover all cases, and we should follow the >> > recommended sequence(if what you are trying to do is really as spec says). >> > Agree! >> > My understanding is that we need to set RC.DRP to 0 every time before >> > TCPM restarts toggling but not just for attached. >> >> Actually I have no objection on adding a RC.DRP clear, the question is >> where is the right place to do this, till now I don’t see how this DRP bit keep set >> while start drp toggling is causing problem, You want to start present Rd after write >> CC1/CC2 to be Rd/Rd and before write to LOOK4CONNECTION, this is not required >> per tcpci spec. > > According to TCPCI's specification, it is recommended the TCPM write RC.DRP = 0 before changing AutoDischargeDisconnecd. > Maybe, an interface for controlling AutoDischargeDisconnecd can be added. Then clear RC.DRP in the beginning of that interface. >> >> >> -Behavior after your patch: >> //begin with Open as RC.DRP = 1 >> RC.CCx=Rd & RC.DRP = 0; //Start present Rd >> Wait 1ms; // Still Rd >> RC.CCx=Rd & RC.DRP = 1; //Open? >> Look4CONNECTION = 1; //DRP toggling continue preset Rd >> >> Is my above CC state description correct? Is this what you want? > > Correct. But the purpose of setting RC.CCx = Rd & RC.DRP = 0 is to make RT1711H's internal firmware leave attached state. > Because RT1711H is still presenting Rp after the device is plugged out, it's internal firmware enters attached state when the device re-plugged in. > When we write RC.CCx = Rd & RC.DRP = 0, RT1711H's internal firmware leaves attached state. So it will keep toggling from Rd to Rp but not report open/open when LOOK4CONNECTION is set. > After rechecking the specification of TCPCI, I think TCPC's internal firmware should start toggling from detached state when LOOK4CONNECTION is set even if RC.DRP is not cleared before > I've discussed with my colleagues, we think this should be in vendor ops. I'll update it in the patch v6. > Could I also apply your patch [1] in the patch v6? > Thank you for your time to clarify this condition. >> >> >> -Only apply my patches: >> //begin with Open as RC.DRP = 1 >> RC.CCx=Rd & RC.DRP = 1; //still Open >> Look4Connection = 1; //Start preset Rd >> >> > For RT1711H, it follows above flow. If it is not correct, this operation should >> > be moved to vendor_ops. >> > > >> > > For your question: >> > > Why RT1711H reports open/open after drp_toggling is enabled? >> > > => See Note2 above. >> > > This open/open is for you plug out the device, right? >> > > => No, see Note2 above. >> > > Why RT1711H can't report new cc change events after you plug in the >> > > device? >> > > => RT1711H can generate new cc change events after plugging in the >> > device. >> > > What cc change event tcpc will report on step 4? >> > > => See Note1 above >> > > Did you verify your change can pass the typec compliance test? >> > > => We didn't test it yet but try to make all functions work correctly first. >> > > >> > > Best Regards, >> > > ***************************** >> > > Shu-Fan Lee >> > > Richtek Technology Corporation >> > > TEL: +886-3-5526789 #2359 >> > > FAX: +886-3-5526612 >> > > ***************************** >> > > >> > >> > ************* Email Confidentiality Notice ******************** >> > >> > The information contained in this e-mail message (including any attachments) >> > may be confidential, proprietary, privileged, or otherwise exempt from >> > disclosure under applicable laws. It is intended to be conveyed only to the >> > designated recipient(s). Any use, dissemination, distribution, printing, >> > retaining or copying of this e-mail (including its attachments) by unintended >> > recipient(s) is strictly prohibited and may be unlawful. If you are not an >> > intended recipient of this e-mail, or believe that you have received this >> > e-mail in error, please notify the sender immediately (by replying to this >> > e-mail), delete any and all copies of this e-mail (including any attachments) >> > from your system, and do not disclose the content of this e-mail to any other >> > person. Thank you! > > > > > -- > Best Regards, > 書帆 -- Best Regards, 書帆 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
