* [PATCH v1 2/3] usb: typec: tcpci: Add support to report vSafe0V
2020-12-01 1:32 [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus Badhri Jagan Sridharan
@ 2020-12-01 1:32 ` Badhri Jagan Sridharan
[not found] ` <20201201132749.GD232197@roeck-us.net>
2020-12-01 1:32 ` [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling Badhri Jagan Sridharan
2020-12-01 13:15 ` [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus Guenter Roeck
2 siblings, 1 reply; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2020-12-01 1:32 UTC (permalink / raw)
To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, Badhri Jagan Sridharan
This change adds vbus_vsafe0v which when set, makes TCPM
query for VSAFE0V by assigning the tcpc.is_vbus_vsafe0v callback.
Also enables ALERT.ExtendedStatus which is triggered when
status of EXTENDED_STATUS.vSafe0V changes.
EXTENDED_STATUS.vSafe0V is set when vbus is at vSafe0V and
cleared otherwise.
Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
drivers/usb/typec/tcpm/tcpci.c | 55 ++++++++++++++++++++++++++--------
drivers/usb/typec/tcpm/tcpci.h | 6 ++++
2 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 12d983a75510..e281b8bee4db 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -402,6 +402,19 @@ static int tcpci_get_vbus(struct tcpc_dev *tcpc)
return !!(reg & TCPC_POWER_STATUS_VBUS_PRES);
}
+static int tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc)
+{
+ struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, ®);
+ if (ret < 0)
+ return ret;
+
+ return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V);
+}
+
static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
{
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -554,12 +567,22 @@ static int tcpci_init(struct tcpc_dev *tcpc)
TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS;
if (tcpci->controls_vbus)
reg |= TCPC_ALERT_POWER_STATUS;
+ /* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */
+ if (tcpci->data->vbus_vsafe0v) {
+ reg |= TCPC_ALERT_EXTENDED_STATUS;
+ ret = regmap_write(tcpci->regmap, TCPC_EXTENDED_STATUS_MASK,
+ TCPC_EXTENDED_STATUS_VSAFE0V);
+ if (ret < 0)
+ return ret;
+ }
return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
}
irqreturn_t tcpci_irq(struct tcpci *tcpci)
{
u16 status;
+ int ret;
+ unsigned int raw;
tcpci_read16(tcpci, TCPC_ALERT, &status);
@@ -575,18 +598,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
tcpm_cc_change(tcpci->port);
if (status & TCPC_ALERT_POWER_STATUS) {
- unsigned int reg;
-
- regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, ®);
-
- /*
- * If power status mask has been reset, then the TCPC
- * has reset.
- */
- if (reg == 0xff)
- tcpm_tcpc_reset(tcpci->port);
- else
- tcpm_vbus_change(tcpci->port);
+ ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ if (ret >= 0) {
+ /*
+ * If power status mask has been reset, then the TCPC
+ * has reset.
+ */
+ if (raw == 0xff)
+ tcpm_tcpc_reset(tcpci->port);
+ else
+ tcpm_vbus_change(tcpci->port);
+ }
}
if (status & TCPC_ALERT_RX_STATUS) {
@@ -622,6 +644,12 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
tcpm_pd_receive(tcpci->port, &msg);
}
+ if (status & TCPC_ALERT_EXTENDED_STATUS) {
+ ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &raw);
+ if (ret >= 0 && (raw & TCPC_EXTENDED_STATUS_VSAFE0V))
+ tcpm_vbus_change(tcpci->port);
+ }
+
if (status & TCPC_ALERT_RX_HARD_RST)
tcpm_pd_hard_reset(tcpci->port);
@@ -699,6 +727,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
tcpci_set_auto_vbus_discharge_threshold;
}
+ if (tcpci->data->vbus_vsafe0v)
+ tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
+
err = tcpci_parse_config(tcpci);
if (err < 0)
return ERR_PTR(err);
diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
index 3fe313655f0c..116a69c85e38 100644
--- a/drivers/usb/typec/tcpm/tcpci.h
+++ b/drivers/usb/typec/tcpm/tcpci.h
@@ -49,6 +49,9 @@
#define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
#define TCPC_TCPC_CTRL_BIST_TM BIT(1)
+#define TCPC_EXTENDED_STATUS 0x20
+#define TCPC_EXTENDED_STATUS_VSAFE0V BIT(0)
+
#define TCPC_ROLE_CTRL 0x1a
#define TCPC_ROLE_CTRL_DRP BIT(6)
#define TCPC_ROLE_CTRL_RP_VAL_SHIFT 4
@@ -155,11 +158,14 @@ struct tcpci;
* is sourcing vbus.
* @auto_discharge_disconnect:
* Optional; Enables TCPC to autonously discharge vbus on disconnect.
+ * @vbus_vsafe0v:
+ * optional; Set when TCPC can detect whether vbus is at VSAFE0V.
*/
struct tcpci_data {
struct regmap *regmap;
unsigned char TX_BUF_BYTE_x_hidden:1;
unsigned char auto_discharge_disconnect:1;
+ unsigned char vbus_vsafe0v:1;
int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling
2020-12-01 1:32 [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus Badhri Jagan Sridharan
2020-12-01 1:32 ` [PATCH v1 2/3] usb: typec: tcpci: Add support to report vSafe0V Badhri Jagan Sridharan
@ 2020-12-01 1:32 ` Badhri Jagan Sridharan
2020-12-01 13:16 ` Guenter Roeck
2020-12-01 13:15 ` [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus Guenter Roeck
2 siblings, 1 reply; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2020-12-01 1:32 UTC (permalink / raw)
To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, Badhri Jagan Sridharan
Unmask EXTENDED_STATUS_MASK.vSafe0V, ALERT.Extended_Status
and set vbus_vsafe0v to enable VSAFE0V signalling.
Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
drivers/usb/typec/tcpm/tcpci_maxim.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.c b/drivers/usb/typec/tcpm/tcpci_maxim.c
index c1797239bf08..1aabe6abd819 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim.c
@@ -112,11 +112,18 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
return;
}
+ /* Enable VSAFE0V detection */
+ ret = max_tcpci_write8(chip, TCPC_EXTENDED_STATUS_MASK, TCPC_EXTENDED_STATUS_VSAFE0V);
+ if (ret < 0) {
+ dev_err(chip->dev, "Unable to unmask TCPC_EXTENDED_STATUS_VSAFE0V ret:%d\n", ret);
+ return;
+ }
+
alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
/* Enable Extended alert for detecting Fast Role Swap Signal */
- TCPC_ALERT_EXTND;
+ TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS;
ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
if (ret < 0) {
@@ -315,6 +322,12 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
}
}
+ if (status & TCPC_ALERT_EXTENDED_STATUS) {
+ ret = max_tcpci_read8(chip, TCPC_EXTENDED_STATUS, (u8 *)®_status);
+ if (ret >= 0 && (reg_status & TCPC_EXTENDED_STATUS_VSAFE0V))
+ tcpm_vbus_change(chip->port);
+ }
+
if (status & TCPC_ALERT_RX_STATUS)
process_rx(chip, status);
@@ -442,6 +455,7 @@ static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
chip->data.init = tcpci_init;
chip->data.frs_sourcing_vbus = max_tcpci_frs_sourcing_vbus;
chip->data.auto_discharge_disconnect = true;
+ chip->data.vbus_vsafe0v = 1;
max_tcpci_init_regs(chip);
chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling
2020-12-01 1:32 ` [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling Badhri Jagan Sridharan
@ 2020-12-01 13:16 ` Guenter Roeck
2020-12-02 4:10 ` Badhri Jagan Sridharan
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2020-12-01 13:16 UTC (permalink / raw)
To: Badhri Jagan Sridharan
Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel
On Mon, Nov 30, 2020 at 05:32:46PM -0800, Badhri Jagan Sridharan wrote:
> Unmask EXTENDED_STATUS_MASK.vSafe0V, ALERT.Extended_Status
> and set vbus_vsafe0v to enable VSAFE0V signalling.
>
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.c b/drivers/usb/typec/tcpm/tcpci_maxim.c
> index c1797239bf08..1aabe6abd819 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.c
> @@ -112,11 +112,18 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
> return;
> }
>
> + /* Enable VSAFE0V detection */
> + ret = max_tcpci_write8(chip, TCPC_EXTENDED_STATUS_MASK, TCPC_EXTENDED_STATUS_VSAFE0V);
> + if (ret < 0) {
> + dev_err(chip->dev, "Unable to unmask TCPC_EXTENDED_STATUS_VSAFE0V ret:%d\n", ret);
> + return;
> + }
> +
> alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
> TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
> TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
> /* Enable Extended alert for detecting Fast Role Swap Signal */
> - TCPC_ALERT_EXTND;
> + TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS;
>
> ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
> if (ret < 0) {
> @@ -315,6 +322,12 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
> }
> }
>
> + if (status & TCPC_ALERT_EXTENDED_STATUS) {
> + ret = max_tcpci_read8(chip, TCPC_EXTENDED_STATUS, (u8 *)®_status);
> + if (ret >= 0 && (reg_status & TCPC_EXTENDED_STATUS_VSAFE0V))
> + tcpm_vbus_change(chip->port);
> + }
> +
> if (status & TCPC_ALERT_RX_STATUS)
> process_rx(chip, status);
>
> @@ -442,6 +455,7 @@ static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
> chip->data.init = tcpci_init;
> chip->data.frs_sourcing_vbus = max_tcpci_frs_sourcing_vbus;
> chip->data.auto_discharge_disconnect = true;
> + chip->data.vbus_vsafe0v = 1;
Why not use true/false ?
>
> max_tcpci_init_regs(chip);
> chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
> --
> 2.29.2.454.gaff20da3a2-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling
2020-12-01 13:16 ` Guenter Roeck
@ 2020-12-02 4:10 ` Badhri Jagan Sridharan
0 siblings, 0 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2020-12-02 4:10 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Heikki Krogerus, Greg Kroah-Hartman, USB, LKML
On Tue, Dec 1, 2020 at 5:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Mon, Nov 30, 2020 at 05:32:46PM -0800, Badhri Jagan Sridharan wrote:
> > Unmask EXTENDED_STATUS_MASK.vSafe0V, ALERT.Extended_Status
> > and set vbus_vsafe0v to enable VSAFE0V signalling.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> > drivers/usb/typec/tcpm/tcpci_maxim.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.c b/drivers/usb/typec/tcpm/tcpci_maxim.c
> > index c1797239bf08..1aabe6abd819 100644
> > --- a/drivers/usb/typec/tcpm/tcpci_maxim.c
> > +++ b/drivers/usb/typec/tcpm/tcpci_maxim.c
> > @@ -112,11 +112,18 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
> > return;
> > }
> >
> > + /* Enable VSAFE0V detection */
> > + ret = max_tcpci_write8(chip, TCPC_EXTENDED_STATUS_MASK, TCPC_EXTENDED_STATUS_VSAFE0V);
> > + if (ret < 0) {
> > + dev_err(chip->dev, "Unable to unmask TCPC_EXTENDED_STATUS_VSAFE0V ret:%d\n", ret);
> > + return;
> > + }
> > +
> > alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
> > TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
> > TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
> > /* Enable Extended alert for detecting Fast Role Swap Signal */
> > - TCPC_ALERT_EXTND;
> > + TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS;
> >
> > ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
> > if (ret < 0) {
> > @@ -315,6 +322,12 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
> > }
> > }
> >
> > + if (status & TCPC_ALERT_EXTENDED_STATUS) {
> > + ret = max_tcpci_read8(chip, TCPC_EXTENDED_STATUS, (u8 *)®_status);
> > + if (ret >= 0 && (reg_status & TCPC_EXTENDED_STATUS_VSAFE0V))
> > + tcpm_vbus_change(chip->port);
> > + }
> > +
> > if (status & TCPC_ALERT_RX_STATUS)
> > process_rx(chip, status);
> >
> > @@ -442,6 +455,7 @@ static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
> > chip->data.init = tcpci_init;
> > chip->data.frs_sourcing_vbus = max_tcpci_frs_sourcing_vbus;
> > chip->data.auto_discharge_disconnect = true;
> > + chip->data.vbus_vsafe0v = 1;
>
> Why not use true/false ?
Sure. Fixing in v2.
>
> >
> > max_tcpci_init_regs(chip);
> > chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
> > --
> > 2.29.2.454.gaff20da3a2-goog
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus
2020-12-01 1:32 [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus Badhri Jagan Sridharan
2020-12-01 1:32 ` [PATCH v1 2/3] usb: typec: tcpci: Add support to report vSafe0V Badhri Jagan Sridharan
2020-12-01 1:32 ` [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling Badhri Jagan Sridharan
@ 2020-12-01 13:15 ` Guenter Roeck
2020-12-02 4:10 ` Badhri Jagan Sridharan
2 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2020-12-01 13:15 UTC (permalink / raw)
To: Badhri Jagan Sridharan
Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel
On Mon, Nov 30, 2020 at 05:32:44PM -0800, Badhri Jagan Sridharan wrote:
> TCPM at present lacks the notion of VSAFE0V. There
> are three vbus threshold levels that are critical to track:
> a. vSafe5V - VBUS “5 volts” as defined by the USB
> PD specification.
> b. vSinkDisconnect - Threshold used for transition from
> Attached.SNK to Unattached.SNK.
> c. vSafe0V - VBUS “0 volts” as defined by the USB
> PD specification.
>
> Tracking vSafe0V is crucial for entry into Try.SNK and
> Attached.SRC and turning vbus back on by the source in
> response to hard reset.
>
> From "4.5.2.2.8.2 Exiting from AttachWait.SRC State" section
> in the Type-C spec:
>
> "The port shall transition to Attached.SRC when VBUS is at
> vSafe0V and the SRC.Rd state is detected on exactly one of
> the CC1 or CC2 pins for at least tCCDebounce."
>
> "A DRP that strongly prefers the Sink role may optionally
> transition to Try.SNK instead of Attached.SRC when VBUS
> is at vSafe0V and the SRC.Rd state is detected on exactly
> one of the CC1 or CC2 pins for at least tCCDebounce."
>
> From "7.1.5 Response to Hard Resets" section in the PD spec:
>
> "After establishing the vSafe0V voltage condition on VBUS,
> the Source Shall wait tSrcRecover before re-applying VCONN
> and restoring VBUS to vSafe5V."
>
> vbus_present in the TCPM code tracks vSafe5V(vbus_present is true)
> and vSinkDisconnect(vbus_present is false).
>
> This change adds is_vbus_vsafe0v callback which when set makes
> TCPM query for vSafe0V voltage level when needed.
>
> Since not all TCPC controllers might have the capability
> to report vSafe0V, TCPM assumes that vSafe0V is same as
> vSinkDisconnect when is_vbus_vsafe0v callback is not set.
> This allows TCPM to continue to support controllers which don't
> have the support for reporting vSafe0V.
>
> Introducing vSafe0V helps fix the failure reported at
> "Step 15. CVS verifies PUT remains in AttachWait.SRC for 500ms"
> of "TD 4.7.2 Try. SNK DRP Connect DRP Test" of
> "Universal Serial Bus Type-C (USB Type-C) Functional Test
> Specification Chapters 4 and 5". Here the compliance tester
> intentionally maintains vbus at greater than vSafe0V and expects
> the Product under test to stay in AttachWait.SRC till vbus drops
> to vSafe0V.
>
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 63 +++++++++++++++++++++++++++++------
> include/linux/usb/tcpm.h | 6 ++++
> 2 files changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3bbc1f10af49..10a065eef73e 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -258,7 +258,19 @@ struct tcpm_port {
> bool attached;
> bool connected;
> enum typec_port_type port_type;
> +
> + /*
> + * Set to true when vbus is greater than VSAFE5V min.
> + * Set to false when vbus falls below vSinkDisconnect max threshold.
> + */
> bool vbus_present;
> +
> + /*
> + * Set to true when vbus is less than VSAFE0V max.
> + * Set to false when vbus is greater than VSAFE0V max.
> + */
> + bool vbus_vsafe0v;
> +
> bool vbus_never_low;
> bool vbus_source;
> bool vbus_charge;
> @@ -3094,7 +3106,7 @@ static void run_state_machine(struct tcpm_port *port)
> else if (tcpm_port_is_audio(port))
> tcpm_set_state(port, AUDIO_ACC_ATTACHED,
> PD_T_CC_DEBOUNCE);
> - else if (tcpm_port_is_source(port))
> + else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
> tcpm_set_state(port,
> tcpm_try_snk(port) ? SNK_TRY
> : SRC_ATTACHED,
> @@ -4097,6 +4109,12 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
> {
> tcpm_log_force(port, "VBUS on");
> port->vbus_present = true;
> + /*
> + * When vbus_present is true i.e. Voltage at VBUS is greater than VSAFE5V implicitly
> + * states that vbus is not at VSAFE0V, hence clear the vbus_vsafe0v flag here.
> + */
> + port->vbus_vsafe0v = false;
> +
> switch (port->state) {
> case SNK_TRANSITION_SINK_VBUS:
> port->explicit_contract = true;
> @@ -4186,16 +4204,8 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
> case SNK_HARD_RESET_SINK_OFF:
> tcpm_set_state(port, SNK_HARD_RESET_WAIT_VBUS, 0);
> break;
> - case SRC_HARD_RESET_VBUS_OFF:
> - /*
> - * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
> - * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
> - */
> - tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
> - break;
> case HARD_RESET_SEND:
> break;
> -
> case SNK_TRY:
> /* Do nothing, waiting for timeout */
> break;
> @@ -4266,6 +4276,28 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
> }
> }
>
> +static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
> +{
> + tcpm_log_force(port, "VBUS VSAFE0V");
> + port->vbus_vsafe0v = true;
> + switch (port->state) {
> + case SRC_HARD_RESET_VBUS_OFF:
> + /*
> + * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
> + * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
> + */
> + tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
> + break;
> + case SRC_ATTACH_WAIT:
> + if (tcpm_port_is_source(port))
> + tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED,
> + PD_T_CC_DEBOUNCE);
> + break;
> + default:
> + break;
> + }
> +}
> +
> static void _tcpm_pd_hard_reset(struct tcpm_port *port)
> {
> tcpm_log_force(port, "Received hard reset");
> @@ -4301,10 +4333,19 @@ static void tcpm_pd_event_handler(struct kthread_work *work)
> bool vbus;
>
> vbus = port->tcpc->get_vbus(port->tcpc);
> - if (vbus)
> + if (vbus) {
> _tcpm_pd_vbus_on(port);
> - else
> + } else {
> _tcpm_pd_vbus_off(port);
> + /*
> + * When TCPC does not support detecting vsafe0v voltage level,
> + * treat vbus absent as vsafe0v. Else invoke is_vbus_vsafe0v
> + * to see if vbus has discharge to VSAFE0V.
> + */
> + if (!port->tcpc->is_vbus_vsafe0v ||
> + port->tcpc->is_vbus_vsafe0v(port->tcpc) > 0)
> + _tcpm_pd_vbus_vsafe0v(port);
> + }
> }
> if (events & TCPM_CC_EVENT) {
> enum typec_cc_status cc1, cc2;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index e68aaa12886f..615d4532c028 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -98,6 +98,11 @@ enum tcpm_transmit_type {
> * will be turned on. requested_vbus_voltage is set to 0 when vbus
> * is going to disappear knowingly i.e. during PR_SWAP and
> * HARD_RESET etc.
> + * @is_vbus_vsafe0v:
> + * Optional; TCPCI spec based TCPC implementations are expected to
> + * detect VSAFE0V voltage level at vbus. When detection of VSAFE0V
> + * is supported by TCPC, set this callback for TCPM to query
> + * whether vbus is at VSAFE0V when needed.
Return values ?
> */
> struct tcpc_dev {
> struct fwnode_handle *fwnode;
> @@ -128,6 +133,7 @@ struct tcpc_dev {
> int (*enable_auto_vbus_discharge)(struct tcpc_dev *dev, bool enable);
> int (*set_auto_vbus_discharge_threshold)(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
> bool pps_active, u32 requested_vbus_voltage);
> + int (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
I don't really see the point of returning an int here. If an error is
returned, it should be checked. However, that is not currently the case;
an error is treated as "false" return. As such, the function may as well
return true/false (and false if an error is observed).
> };
>
> struct tcpm_port;
> --
> 2.29.2.454.gaff20da3a2-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus
2020-12-01 13:15 ` [PATCH v1 1/3] usb: typec: tcpm: Introduce vsafe0v for vbus Guenter Roeck
@ 2020-12-02 4:10 ` Badhri Jagan Sridharan
0 siblings, 0 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2020-12-02 4:10 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Heikki Krogerus, Greg Kroah-Hartman, USB, LKML
On Tue, Dec 1, 2020 at 5:15 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Mon, Nov 30, 2020 at 05:32:44PM -0800, Badhri Jagan Sridharan wrote:
> > TCPM at present lacks the notion of VSAFE0V. There
> > are three vbus threshold levels that are critical to track:
> > a. vSafe5V - VBUS “5 volts” as defined by the USB
> > PD specification.
> > b. vSinkDisconnect - Threshold used for transition from
> > Attached.SNK to Unattached.SNK.
> > c. vSafe0V - VBUS “0 volts” as defined by the USB
> > PD specification.
> >
> > Tracking vSafe0V is crucial for entry into Try.SNK and
> > Attached.SRC and turning vbus back on by the source in
> > response to hard reset.
> >
> > From "4.5.2.2.8.2 Exiting from AttachWait.SRC State" section
> > in the Type-C spec:
> >
> > "The port shall transition to Attached.SRC when VBUS is at
> > vSafe0V and the SRC.Rd state is detected on exactly one of
> > the CC1 or CC2 pins for at least tCCDebounce."
> >
> > "A DRP that strongly prefers the Sink role may optionally
> > transition to Try.SNK instead of Attached.SRC when VBUS
> > is at vSafe0V and the SRC.Rd state is detected on exactly
> > one of the CC1 or CC2 pins for at least tCCDebounce."
> >
> > From "7.1.5 Response to Hard Resets" section in the PD spec:
> >
> > "After establishing the vSafe0V voltage condition on VBUS,
> > the Source Shall wait tSrcRecover before re-applying VCONN
> > and restoring VBUS to vSafe5V."
> >
> > vbus_present in the TCPM code tracks vSafe5V(vbus_present is true)
> > and vSinkDisconnect(vbus_present is false).
> >
> > This change adds is_vbus_vsafe0v callback which when set makes
> > TCPM query for vSafe0V voltage level when needed.
> >
> > Since not all TCPC controllers might have the capability
> > to report vSafe0V, TCPM assumes that vSafe0V is same as
> > vSinkDisconnect when is_vbus_vsafe0v callback is not set.
> > This allows TCPM to continue to support controllers which don't
> > have the support for reporting vSafe0V.
> >
> > Introducing vSafe0V helps fix the failure reported at
> > "Step 15. CVS verifies PUT remains in AttachWait.SRC for 500ms"
> > of "TD 4.7.2 Try. SNK DRP Connect DRP Test" of
> > "Universal Serial Bus Type-C (USB Type-C) Functional Test
> > Specification Chapters 4 and 5". Here the compliance tester
> > intentionally maintains vbus at greater than vSafe0V and expects
> > the Product under test to stay in AttachWait.SRC till vbus drops
> > to vSafe0V.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> > drivers/usb/typec/tcpm/tcpm.c | 63 +++++++++++++++++++++++++++++------
> > include/linux/usb/tcpm.h | 6 ++++
> > 2 files changed, 58 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 3bbc1f10af49..10a065eef73e 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -258,7 +258,19 @@ struct tcpm_port {
> > bool attached;
> > bool connected;
> > enum typec_port_type port_type;
> > +
> > + /*
> > + * Set to true when vbus is greater than VSAFE5V min.
> > + * Set to false when vbus falls below vSinkDisconnect max threshold.
> > + */
> > bool vbus_present;
> > +
> > + /*
> > + * Set to true when vbus is less than VSAFE0V max.
> > + * Set to false when vbus is greater than VSAFE0V max.
> > + */
> > + bool vbus_vsafe0v;
> > +
> > bool vbus_never_low;
> > bool vbus_source;
> > bool vbus_charge;
> > @@ -3094,7 +3106,7 @@ static void run_state_machine(struct tcpm_port *port)
> > else if (tcpm_port_is_audio(port))
> > tcpm_set_state(port, AUDIO_ACC_ATTACHED,
> > PD_T_CC_DEBOUNCE);
> > - else if (tcpm_port_is_source(port))
> > + else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
> > tcpm_set_state(port,
> > tcpm_try_snk(port) ? SNK_TRY
> > : SRC_ATTACHED,
> > @@ -4097,6 +4109,12 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
> > {
> > tcpm_log_force(port, "VBUS on");
> > port->vbus_present = true;
> > + /*
> > + * When vbus_present is true i.e. Voltage at VBUS is greater than VSAFE5V implicitly
> > + * states that vbus is not at VSAFE0V, hence clear the vbus_vsafe0v flag here.
> > + */
> > + port->vbus_vsafe0v = false;
> > +
> > switch (port->state) {
> > case SNK_TRANSITION_SINK_VBUS:
> > port->explicit_contract = true;
> > @@ -4186,16 +4204,8 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
> > case SNK_HARD_RESET_SINK_OFF:
> > tcpm_set_state(port, SNK_HARD_RESET_WAIT_VBUS, 0);
> > break;
> > - case SRC_HARD_RESET_VBUS_OFF:
> > - /*
> > - * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
> > - * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
> > - */
> > - tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
> > - break;
> > case HARD_RESET_SEND:
> > break;
> > -
> > case SNK_TRY:
> > /* Do nothing, waiting for timeout */
> > break;
> > @@ -4266,6 +4276,28 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
> > }
> > }
> >
> > +static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
> > +{
> > + tcpm_log_force(port, "VBUS VSAFE0V");
> > + port->vbus_vsafe0v = true;
> > + switch (port->state) {
> > + case SRC_HARD_RESET_VBUS_OFF:
> > + /*
> > + * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
> > + * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
> > + */
> > + tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
> > + break;
> > + case SRC_ATTACH_WAIT:
> > + if (tcpm_port_is_source(port))
> > + tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED,
> > + PD_T_CC_DEBOUNCE);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > static void _tcpm_pd_hard_reset(struct tcpm_port *port)
> > {
> > tcpm_log_force(port, "Received hard reset");
> > @@ -4301,10 +4333,19 @@ static void tcpm_pd_event_handler(struct kthread_work *work)
> > bool vbus;
> >
> > vbus = port->tcpc->get_vbus(port->tcpc);
> > - if (vbus)
> > + if (vbus) {
> > _tcpm_pd_vbus_on(port);
> > - else
> > + } else {
> > _tcpm_pd_vbus_off(port);
> > + /*
> > + * When TCPC does not support detecting vsafe0v voltage level,
> > + * treat vbus absent as vsafe0v. Else invoke is_vbus_vsafe0v
> > + * to see if vbus has discharge to VSAFE0V.
> > + */
> > + if (!port->tcpc->is_vbus_vsafe0v ||
> > + port->tcpc->is_vbus_vsafe0v(port->tcpc) > 0)
> > + _tcpm_pd_vbus_vsafe0v(port);
> > + }
> > }
> > if (events & TCPM_CC_EVENT) {
> > enum typec_cc_status cc1, cc2;
> > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > index e68aaa12886f..615d4532c028 100644
> > --- a/include/linux/usb/tcpm.h
> > +++ b/include/linux/usb/tcpm.h
> > @@ -98,6 +98,11 @@ enum tcpm_transmit_type {
> > * will be turned on. requested_vbus_voltage is set to 0 when vbus
> > * is going to disappear knowingly i.e. during PR_SWAP and
> > * HARD_RESET etc.
> > + * @is_vbus_vsafe0v:
> > + * Optional; TCPCI spec based TCPC implementations are expected to
> > + * detect VSAFE0V voltage level at vbus. When detection of VSAFE0V
> > + * is supported by TCPC, set this callback for TCPM to query
> > + * whether vbus is at VSAFE0V when needed.
>
> Return values ?
Fixing in v2.
>
> > */
> > struct tcpc_dev {
> > struct fwnode_handle *fwnode;
> > @@ -128,6 +133,7 @@ struct tcpc_dev {
> > int (*enable_auto_vbus_discharge)(struct tcpc_dev *dev, bool enable);
> > int (*set_auto_vbus_discharge_threshold)(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
> > bool pps_active, u32 requested_vbus_voltage);
> > + int (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
>
> I don't really see the point of returning an int here. If an error is
> returned, it should be checked. However, that is not currently the case;
> an error is treated as "false" return. As such, the function may as well
> return true/false (and false if an error is observed).
Sure. Changing the callback return to boolean in v2.
>
> > };
> >
> > struct tcpm_port;
> > --
> > 2.29.2.454.gaff20da3a2-goog
> >
^ permalink raw reply [flat|nested] 8+ messages in thread