Re: [PATCH 10/12] staging: typec: tcpci: update set_cc for different state
From: Guenter Roeck <hidden>
Date: 2017-09-26 13:49:30
On 09/25/2017 05:45 PM, Li Jun wrote:
quoted hunk
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 <redacted> --- 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 ?. I am also not sure if the chip should ever set both CC lines at the same time. Guenter
ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); if (ret < 0) return ret;
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html