RE: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc line open

[Jun Li]

Hi
> -Original Message-
> From: Guenter Roeck [mailto:groe...@gmail.com] On Behalf Of Guenter Roeck
> Sent: 2018年3月30日 23:16
> To: Jun Li <jun...@nxp.com>; robh...@kernel.org; gre...@linuxfoundation.org;
> heikki.kroge...@linux.intel.com
> Cc: a.ha...@samsung.com; shufan_...@richtek.com; Peter Chen
> <peter.c...@nxp.com>; devicet...@vger.kernel.org;
> linux-...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>;
> de...@driverdev.osuosl.org
> Subject: Re: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting 
> cc
> line open
> 
> On 03/28/2018 09:06 AM, Li Jun wrote:
> > While set polarity, we should keep the not connecting cc line to be
> > open.
> >
> 
> The more I look at this code, the more I am confused by it.
> 
> The original code doesn't touch the CC lines. This function only sets the 
> polarity.
> Is it really appropriate to touch the CC lines in the same function ?
> 

Yes, I didn't find a more proper place to do this, either I change the
tcpc->set_cc() interface with orientation/polarity parameter to know
which cc line I should keep it open, or do it in low level driver like
this, do you have any suggestion how this can be done?(I guess
both cc lines have the same state after attached with current code
of all tcpm users, but this should be resolved as it's break PD compliance
test) 

thanks
Li Jun

> Guenter
> 
> > Signed-off-by: Li Jun <jun...@nxp.com>
> > ---
> >   drivers/staging/typec/tcpci.c | 18 ++
> >   1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index d5b4e4e..b58bd59 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -185,15 +185,25 @@ 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,
> > -  (polarity == TYPEC_POLARITY_CC2) ?
> > -  TCPC_TCPC_CTRL_ORIENTATION : 0);
> > +   /* Keep the disconnect cc line open */
> > +   ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, );
> > if (ret < 0)
> > return ret;
> >
> > -   return 0;
> > +   if (polarity == TYPEC_POLARITY_CC2)
> > +   reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
> > +   else
> > +   reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
> > +   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> > +  (polarity == TYPEC_POLARITY_CC2) ?
> > +  TCPC_TCPC_CTRL_ORIENTATION : 0);
> >   }
> >
> >   static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
> >

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc line open

[Guenter Roeck]

On 03/28/2018 09:06 AM, Li Jun wrote:

While set polarity, we should keep the not connecting cc line to be
open.



The more I look at this code, the more I am confused by it.

The original code doesn't touch the CC lines. This function only sets the 
polarity.
Is it really appropriate to touch the CC lines in the same function ?

Guenter


Signed-off-by: Li Jun 
---
  drivers/staging/typec/tcpci.c | 18 ++
  1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index d5b4e4e..b58bd59 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -185,15 +185,25 @@ 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,

-  (polarity == TYPEC_POLARITY_CC2) ?
-  TCPC_TCPC_CTRL_ORIENTATION : 0);
+   /* Keep the disconnect cc line open */
+   ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, );
if (ret < 0)
return ret;
  
-	return 0;

+   if (polarity == TYPEC_POLARITY_CC2)
+   reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
+   else
+   reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+
+   return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
+  (polarity == TYPEC_POLARITY_CC2) ?
+  TCPC_TCPC_CTRL_ORIENTATION : 0);
  }
  
  static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel