On 09/26/2017 02:53 AM, Jun Li wrote:
Hi,
-----Original Message-----
From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter Roeck
Sent: Tuesday, September 26, 2017 3:17 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 09/12] usb: typec: tcpm: only drives the connected cc line
when attached
On 09/25/2017 05:45 PM, Li Jun wrote:
As we should only drive connected cc line to be the right state when
attached, and keeps the other one to be open, so update the set_cc
interface for that.
Signed-off-by: Li Jun <jun.li@xxxxxxx>
---
drivers/usb/typec/tcpm.c | 12 +++++++++++-
include/linux/usb/tcpm.h | 3 ++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index
38a6223..c9966ee 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1857,9 +1857,15 @@ static bool tcpm_start_drp_toggling(struct
tcpm_port *port,
static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
{
+ bool attached;
+
tcpm_log(port, "cc:=%d", cc);
port->cc_req = cc;
- port->tcpc->set_cc(port->tcpc, cc);
+ if (port->state == SRC_ATTACHED || port->state == SNK_ATTACHED)
+ attached = true;
+ else
+ attached = false;
+ port->tcpc->set_cc(port->tcpc, cc, attached, port->polarity);
Polarity should be set with set_polarity(). Is it necessary to duplicate that ?
In tcpci case, set_polarity only sets Plug Orientation bit:
" 0b: When Vconn is enabled, apply it to the CC2 pin. Monitor the CC1 pin for
BMC communications if PD messaging is enabled.
1b: When Vconn is enabled, apply it to the CC1 pin. Monitor the CC2 pin for
BMC communications if PD messaging is enabled."
Yes, but the driver should know about the flag already.
I have two more concerns:
Setting CC is logically independent from the state machine "active" state.
I don't recall that the USB PD state machine talks about CC pin changes upon
entering and exiting READY states. Why is it necessary to pass the state
to the driver ?
Also, your patch changes the API, but you don't change the driver, meaning there
should be compile failures after this patch. You need to change the calling code
in the same patch to keep the build clean.
Guenter
There is no duplication for tcpci, or you think I should do this in set_polarity
of tcpci driver internally? looks better from my point view as I may don't
need change tcpm set_cc interface.
thanks
Li Jun
}
static int tcpm_init_vbus(struct tcpm_port *port) @@ -1913,6 +1919,8
@@ static int tcpm_src_attach(struct tcpm_port *port)
if (ret < 0)
return ret;
+ tcpm_set_cc(port, tcpm_rp_cc(port));
+
ret = tcpm_set_roles(port, true, TYPEC_SOURCE, TYPEC_HOST);
if (ret < 0)
return ret;
@@ -2028,6 +2036,8 @@ static int tcpm_snk_attach(struct tcpm_port *port)
if (ret < 0)
return ret;
+ tcpm_set_cc(port, TYPEC_CC_RD);
+
ret = tcpm_set_roles(port, true, TYPEC_SINK, TYPEC_DEVICE);
if (ret < 0)
return ret;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h index
a67cf77..a007e2a1 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -159,7 +159,8 @@ struct tcpc_dev {
int (*init)(struct tcpc_dev *dev);
int (*get_vbus)(struct tcpc_dev *dev);
int (*get_current_limit)(struct tcpc_dev *dev);
- int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
+ int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc,
+ bool attached, enum typec_cc_polarity polarity);
int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
enum typec_cc_status *cc2);
int (*set_polarity)(struct tcpc_dev *dev,
--
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
