Hi > -----Original Message----- > From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter Roeck > Sent: Tuesday, September 26, 2017 9:50 PM > To: Jun Li <jun.li@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; > mark.rutland@xxxxxxx; heikki.krogerus@xxxxxxxxxxxxxxx > Cc: yueyao@xxxxxxxxxx; o_leveque@xxxxxxxxx; Peter Chen > <peter.chen@xxxxxxx>; A.s. Dong <aisheng.dong@xxxxxxx>; linux- > usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 10/12] staging: typec: tcpci: update set_cc for different > state > > On 09/25/2017 05:45 PM, Li Jun wrote: > > As we should keep the disconnected cc line to be open when attached, > > so update the set_cc interface accordingly for it. > > > > Signed-off-by: Li Jun <jun.li@xxxxxxx> > > --- > > drivers/staging/typec/tcpci.c | 37 +++++++++++++++++++++---------------- > > 1 file changed, 21 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/staging/typec/tcpci.c > > b/drivers/staging/typec/tcpci.c index cea67f9..c7c45da 100644 > > --- a/drivers/staging/typec/tcpci.c > > +++ b/drivers/staging/typec/tcpci.c > > @@ -57,10 +57,11 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int > reg, u16 val) > > return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16)); > > } > > > > -static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status > > cc) > > +static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc, > > + bool attached, enum typec_cc_polarity polarity) > > { > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc); > > - unsigned int reg; > > + unsigned int reg = 0, reg_cc1 = 0, reg_cc2 = 0; > > int ret; > > > > switch (cc) { > > @@ -69,26 +70,23 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum > typec_cc_status cc) > > (TCPC_ROLE_CTRL_CC_RA << > TCPC_ROLE_CTRL_CC2_SHIFT); > > break; > > case TYPEC_CC_RD: > > - reg = (TCPC_ROLE_CTRL_CC_RD << > TCPC_ROLE_CTRL_CC1_SHIFT) | > > - (TCPC_ROLE_CTRL_CC_RD << > TCPC_ROLE_CTRL_CC2_SHIFT); > > + reg_cc1 = TCPC_ROLE_CTRL_CC_RD << > TCPC_ROLE_CTRL_CC1_SHIFT; > > + reg_cc2 = TCPC_ROLE_CTRL_CC_RD << > TCPC_ROLE_CTRL_CC2_SHIFT; > > break; > > case TYPEC_CC_RP_DEF: > > - reg = (TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC1_SHIFT) | > > - (TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC2_SHIFT) | > > - (TCPC_ROLE_CTRL_RP_VAL_DEF << > > - TCPC_ROLE_CTRL_RP_VAL_SHIFT); > > + reg_cc1 = TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC1_SHIFT; > > + reg_cc2 = TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC2_SHIFT; > > + reg = TCPC_ROLE_CTRL_RP_VAL_DEF << > TCPC_ROLE_CTRL_RP_VAL_SHIFT; > > break; > > case TYPEC_CC_RP_1_5: > > - reg = (TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC1_SHIFT) | > > - (TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC2_SHIFT) | > > - (TCPC_ROLE_CTRL_RP_VAL_1_5 << > > - TCPC_ROLE_CTRL_RP_VAL_SHIFT); > > + reg_cc1 = TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC1_SHIFT; > > + reg_cc2 = TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC2_SHIFT; > > + reg = TCPC_ROLE_CTRL_RP_VAL_1_5 << > TCPC_ROLE_CTRL_RP_VAL_SHIFT; > > break; > > case TYPEC_CC_RP_3_0: > > - reg = (TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC1_SHIFT) | > > - (TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC2_SHIFT) | > > - (TCPC_ROLE_CTRL_RP_VAL_3_0 << > > - TCPC_ROLE_CTRL_RP_VAL_SHIFT); > > + reg_cc1 = TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC1_SHIFT; > > + reg_cc2 = TCPC_ROLE_CTRL_CC_RP << > TCPC_ROLE_CTRL_CC2_SHIFT; > > + reg = TCPC_ROLE_CTRL_RP_VAL_3_0 << > TCPC_ROLE_CTRL_RP_VAL_SHIFT; > > break; > > case TYPEC_CC_OPEN: > > default: > > @@ -97,6 +95,13 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum > typec_cc_status cc) > > break; > > } > > > > + if (!attached) > > + reg |= reg_cc1 | reg_cc2; > > + else if (polarity == TYPEC_POLARITY_CC1) > > + reg |= reg_cc1; > > + else > > + reg |= reg_cc2; > > + > > I think this is wrong. The value of CC pins should not depend on the state of the > state machine. What if the state machine knows the polarity but is in a transient > state ?. > Agree with you this API change will make set_cc somehow independent. tcpci_set_polarity() maybe the right place to cover this. > I am also not sure if the chip should ever set both CC lines at the same time. To look for connection, both CC lines should be set at the same time for possible contact. > > Guenter > > > ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > > if (ret < 0) > > return ret; > > ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
