linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] [PATCH 1/3] dt-bindings: connector: Add self-powered property
@ 2018-09-28 23:47 Badhri Jagan Sridharan
  2018-09-28 23:47 ` [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
  2018-09-28 23:47 ` [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset Badhri Jagan Sridharan
  0 siblings, 2 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2018-09-28 23:47 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heikki Krogerus, Greg Kroah-Hartman,
	Chanwoo Choi, Andrzej Hajda, Li Jun, linux
  Cc: devicetree, linux-kernel, linux-usb, Badhri Jagan Sridharan

From USB_PD_R3_0
7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset
(see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the
loss of their power source.

Therefore it is necessary to know whether the port belongs to
a device which is self powered or bus powered. This change
adds "self-powered" flag to the connector class which present indicates
that the port belongs to a device that is self powered. Else it is
bus powered usb device.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 Documentation/devicetree/bindings/connector/usb-connector.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
index 8855bfcfd7785..8f7a36ae29bf7 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.txt
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -14,6 +14,8 @@ Optional properties:
 - label: symbolic name for the connector,
 - type: size of the connector, should be specified in case of USB-A, USB-B
   non-fullsize connectors: "mini", "micro".
+- self-powered: Set this property if the usb device that has its own power
+  source.
 
 Optional properties for usb-c-connector:
 - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
-- 
2.19.0.605.g01d371f741-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
  2018-09-28 23:47 [PATCH 1/3] [PATCH 1/3] dt-bindings: connector: Add self-powered property Badhri Jagan Sridharan
@ 2018-09-28 23:47 ` Badhri Jagan Sridharan
  2018-09-30 22:04   ` Guenter Roeck
  2018-10-01  9:53   ` Heikki Krogerus
  2018-09-28 23:47 ` [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset Badhri Jagan Sridharan
  1 sibling, 2 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2018-09-28 23:47 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heikki Krogerus, Greg Kroah-Hartman,
	Chanwoo Choi, Andrzej Hajda, Li Jun, linux
  Cc: devicetree, linux-kernel, linux-usb, Badhri Jagan Sridharan

During HARD_RESET the data link is disconnected.
For self powered device, the spec is advising against doing that.

From USB_PD_R3_0
7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset
(see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the
loss of their power source.

Tackle this by letting TCPM know whether the device is self or bus powered.

This overcomes unnecessary port disconnections from hard reset.
Also, speeds up the enumeration time when connected to Type-A ports.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---------
Version history:

V2:
Based on feedback from heikki.krogerus@linux.intel.com
- self_powered added to the struct tcpm_port which is populated from
  a. "connector" node of the device tree in tcpm_fw_get_caps()
  b. "self_powered" node of the tcpc_config in tcpm_copy_caps

Based on feedbase from linux@roeck-us.net
- Code was refactored
- SRC_HARD_RESET_VBUS_OFF sets the link state to false based
  on self_powered flag

V1 located here:
https://lkml.org/lkml/2018/9/13/94
---
 drivers/usb/typec/tcpm.c | 12 ++++++++++--
 include/linux/usb/tcpm.h |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 4f1f4215f3d6e..c3ac0e46106b4 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -317,6 +317,9 @@ struct tcpm_port {
 	/* Deadline in jiffies to exit src_try_wait state */
 	unsigned long max_wait;
 
+	/* port belongs to a self powered device */
+	bool self_powered;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
 	case SRC_HARD_RESET_VBUS_OFF:
 		tcpm_set_vconn(port, true);
 		tcpm_set_vbus(port, false);
-		tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
+		tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
+			       TYPEC_HOST);
 		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
 		break;
 	case SRC_HARD_RESET_VBUS_ON:
@@ -3270,7 +3274,8 @@ static void run_state_machine(struct tcpm_port *port)
 		memset(&port->pps_data, 0, sizeof(port->pps_data));
 		tcpm_set_vconn(port, false);
 		tcpm_set_charge(port, false);
-		tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
+		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
+			       TYPEC_DEVICE);
 		/*
 		 * VBUS may or may not toggle, depending on the adapter.
 		 * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
@@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
 		return -EINVAL;
 	port->operating_snk_mw = mw / 1000;
 
+	port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
+
 	return 0;
 }
 
@@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
 	port->typec_caps.prefer_role = tcfg->default_role;
 	port->typec_caps.type = tcfg->type;
 	port->typec_caps.data = tcfg->data;
+	port->self_powered = port->tcpc->config->self_powered;
 
 	return 0;
 }
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 7e7fbfb84e8e3..50c74a77db55c 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -89,6 +89,7 @@ struct tcpc_config {
 	enum typec_port_data data;
 	enum typec_role default_role;
 	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
+	bool self_powered;	/* port belongs to a self powered device */
 
 	const struct typec_altmode_desc *alt_modes;
 };
-- 
2.19.0.605.g01d371f741-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset
  2018-09-28 23:47 [PATCH 1/3] [PATCH 1/3] dt-bindings: connector: Add self-powered property Badhri Jagan Sridharan
  2018-09-28 23:47 ` [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
@ 2018-09-28 23:47 ` Badhri Jagan Sridharan
  2018-09-28 23:48   ` Badhri Jagan Sridharan
  2018-09-30 22:05   ` Guenter Roeck
  1 sibling, 2 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2018-09-28 23:47 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Heikki Krogerus, Greg Kroah-Hartman,
	Chanwoo Choi, Andrzej Hajda, Li Jun, linux
  Cc: devicetree, linux-kernel, linux-usb, Badhri Jagan Sridharan

During the initial connect to a non-pd port, sink would hard reset
twice before deeming that the port partner is non-pd. TCPM sets the
the charge path to false during the hard reset. This causes unnecessary
connects/disconnects of charge path and makes port take longer to
charge from the non-pd ports. Avoid this by not setting the charge path
to false unless the partner has already identified to be pd capable.

When partner is a pd port, set the charge path to false in
SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based
of CC pull up and resume the charge path when port enters
SNK_HARD_RESET_SINK_ON.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

--------
Changes in V2:
Based on feedback of jackp@codeaurora.org
- vsafe_5v_hard_reset flag from tcpc_config is removed
- Patch only differentiates between pd port partner and non-pd port
partner

V1 version of the patch is here:
https://lkml.org/lkml/2018/9/14/11
---
 drivers/usb/typec/tcpm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index c3ac0e46106b4..c25a69922ee6a 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -3273,7 +3273,8 @@ static void run_state_machine(struct tcpm_port *port)
 	case SNK_HARD_RESET_SINK_OFF:
 		memset(&port->pps_data, 0, sizeof(port->pps_data));
 		tcpm_set_vconn(port, false);
-		tcpm_set_charge(port, false);
+		if (port->pd_capable)
+			tcpm_set_charge(port, false);
 		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
 			       TYPEC_DEVICE);
 		/*
@@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
 		 * Similar, dual-mode ports in source mode should transition
 		 * to PE_SNK_Transition_to_default.
 		 */
+		if (port->pd_capable) {
+			tcpm_set_current_limit(port,
+					       tcpm_get_current_limit(port),
+					       5000);
+			tcpm_set_charge(port, true);
+		}
 		tcpm_set_attached_state(port, true);
 		tcpm_set_state(port, SNK_STARTUP, 0);
 		break;
-- 
2.19.0.605.g01d371f741-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset
  2018-09-28 23:47 ` [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset Badhri Jagan Sridharan
@ 2018-09-28 23:48   ` Badhri Jagan Sridharan
  2018-09-30 22:05   ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2018-09-28 23:48 UTC (permalink / raw)
  To: robh+dt, mark.rutland, Heikki Krogerus, Greg Kroah-Hartman,
	cw00.choi, a.hajda, jun.li, Guenter Roeck, jackp
  Cc: devicetree, LKML, USB

+ jackp@codeaurora.org
On Fri, Sep 28, 2018 at 4:47 PM Badhri Jagan Sridharan
<badhri@google.com> wrote:
>
> During the initial connect to a non-pd port, sink would hard reset
> twice before deeming that the port partner is non-pd. TCPM sets the
> the charge path to false during the hard reset. This causes unnecessary
> connects/disconnects of charge path and makes port take longer to
> charge from the non-pd ports. Avoid this by not setting the charge path
> to false unless the partner has already identified to be pd capable.
>
> When partner is a pd port, set the charge path to false in
> SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based
> of CC pull up and resume the charge path when port enters
> SNK_HARD_RESET_SINK_ON.
>
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
>
> --------
> Changes in V2:
> Based on feedback of jackp@codeaurora.org
> - vsafe_5v_hard_reset flag from tcpc_config is removed
> - Patch only differentiates between pd port partner and non-pd port
> partner
>
> V1 version of the patch is here:
> https://lkml.org/lkml/2018/9/14/11
> ---
>  drivers/usb/typec/tcpm.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index c3ac0e46106b4..c25a69922ee6a 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -3273,7 +3273,8 @@ static void run_state_machine(struct tcpm_port *port)
>         case SNK_HARD_RESET_SINK_OFF:
>                 memset(&port->pps_data, 0, sizeof(port->pps_data));
>                 tcpm_set_vconn(port, false);
> -               tcpm_set_charge(port, false);
> +               if (port->pd_capable)
> +                       tcpm_set_charge(port, false);
>                 tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
>                                TYPEC_DEVICE);
>                 /*
> @@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
>                  * Similar, dual-mode ports in source mode should transition
>                  * to PE_SNK_Transition_to_default.
>                  */
> +               if (port->pd_capable) {
> +                       tcpm_set_current_limit(port,
> +                                              tcpm_get_current_limit(port),
> +                                              5000);
> +                       tcpm_set_charge(port, true);
> +               }
>                 tcpm_set_attached_state(port, true);
>                 tcpm_set_state(port, SNK_STARTUP, 0);
>                 break;
> --
> 2.19.0.605.g01d371f741-goog
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
  2018-09-28 23:47 ` [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
@ 2018-09-30 22:04   ` Guenter Roeck
  2018-10-01  9:53   ` Heikki Krogerus
  1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2018-09-30 22:04 UTC (permalink / raw)
  To: Badhri Jagan Sridharan, Rob Herring, Mark Rutland,
	Heikki Krogerus, Greg Kroah-Hartman, Chanwoo Choi, Andrzej Hajda,
	Li Jun
  Cc: devicetree, linux-kernel, linux-usb

On 09/28/2018 04:47 PM, Badhri Jagan Sridharan wrote:
> During HARD_RESET the data link is disconnected.
> For self powered device, the spec is advising against doing that.
> 
>>From USB_PD_R3_0
> 7.1.5 Response to Hard Resets
> Device operation during and after a Hard Reset is defined as follows:
> Self-powered devices Should Not disconnect from USB during a Hard Reset
> (see Section 9.1.2).
> Bus powered devices will disconnect from USB during a Hard Reset due to the
> loss of their power source.
> 
> Tackle this by letting TCPM know whether the device is self or bus powered.
> 
> This overcomes unnecessary port disconnections from hard reset.
> Also, speeds up the enumeration time when connected to Type-A ports.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---------
> Version history:
> 
> V2:
> Based on feedback from heikki.krogerus@linux.intel.com
> - self_powered added to the struct tcpm_port which is populated from
>    a. "connector" node of the device tree in tcpm_fw_get_caps()
>    b. "self_powered" node of the tcpc_config in tcpm_copy_caps
> 
> Based on feedbase from linux@roeck-us.net
> - Code was refactored
> - SRC_HARD_RESET_VBUS_OFF sets the link state to false based
>    on self_powered flag
> 
> V1 located here:
> https://lkml.org/lkml/2018/9/13/94
> ---
>   drivers/usb/typec/tcpm.c | 12 ++++++++++--
>   include/linux/usb/tcpm.h |  1 +
>   2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 4f1f4215f3d6e..c3ac0e46106b4 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -317,6 +317,9 @@ struct tcpm_port {
>   	/* Deadline in jiffies to exit src_try_wait state */
>   	unsigned long max_wait;
>   
> +	/* port belongs to a self powered device */
> +	bool self_powered;
> +
>   #ifdef CONFIG_DEBUG_FS
>   	struct dentry *dentry;
>   	struct mutex logbuffer_lock;	/* log buffer access lock */
> @@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
>   	case SRC_HARD_RESET_VBUS_OFF:
>   		tcpm_set_vconn(port, true);
>   		tcpm_set_vbus(port, false);
> -		tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
> +		tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
> +			       TYPEC_HOST);
>   		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
>   		break;
>   	case SRC_HARD_RESET_VBUS_ON:
> @@ -3270,7 +3274,8 @@ static void run_state_machine(struct tcpm_port *port)
>   		memset(&port->pps_data, 0, sizeof(port->pps_data));
>   		tcpm_set_vconn(port, false);
>   		tcpm_set_charge(port, false);
> -		tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> +		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
> +			       TYPEC_DEVICE);
>   		/*
>   		 * VBUS may or may not toggle, depending on the adapter.
>   		 * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
> @@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>   		return -EINVAL;
>   	port->operating_snk_mw = mw / 1000;
>   
> +	port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
> +
>   	return 0;
>   }
>   
> @@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
>   	port->typec_caps.prefer_role = tcfg->default_role;
>   	port->typec_caps.type = tcfg->type;
>   	port->typec_caps.data = tcfg->data;
> +	port->self_powered = port->tcpc->config->self_powered;
>   
>   	return 0;
>   }
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 7e7fbfb84e8e3..50c74a77db55c 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -89,6 +89,7 @@ struct tcpc_config {
>   	enum typec_port_data data;
>   	enum typec_role default_role;
>   	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
> +	bool self_powered;	/* port belongs to a self powered device */
>   
>   	const struct typec_altmode_desc *alt_modes;
>   };
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset
  2018-09-28 23:47 ` [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset Badhri Jagan Sridharan
  2018-09-28 23:48   ` Badhri Jagan Sridharan
@ 2018-09-30 22:05   ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2018-09-30 22:05 UTC (permalink / raw)
  To: Badhri Jagan Sridharan, Rob Herring, Mark Rutland,
	Heikki Krogerus, Greg Kroah-Hartman, Chanwoo Choi, Andrzej Hajda,
	Li Jun
  Cc: devicetree, linux-kernel, linux-usb

On 09/28/2018 04:47 PM, Badhri Jagan Sridharan wrote:
> During the initial connect to a non-pd port, sink would hard reset
> twice before deeming that the port partner is non-pd. TCPM sets the
> the charge path to false during the hard reset. This causes unnecessary
> connects/disconnects of charge path and makes port take longer to
> charge from the non-pd ports. Avoid this by not setting the charge path
> to false unless the partner has already identified to be pd capable.
> 
> When partner is a pd port, set the charge path to false in
> SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based
> of CC pull up and resume the charge path when port enters
> SNK_HARD_RESET_SINK_ON.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> 
> --------
> Changes in V2:
> Based on feedback of jackp@codeaurora.org
> - vsafe_5v_hard_reset flag from tcpc_config is removed
> - Patch only differentiates between pd port partner and non-pd port
> partner
> 
> V1 version of the patch is here:
> https://lkml.org/lkml/2018/9/14/11
> ---
>   drivers/usb/typec/tcpm.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index c3ac0e46106b4..c25a69922ee6a 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -3273,7 +3273,8 @@ static void run_state_machine(struct tcpm_port *port)
>   	case SNK_HARD_RESET_SINK_OFF:
>   		memset(&port->pps_data, 0, sizeof(port->pps_data));
>   		tcpm_set_vconn(port, false);
> -		tcpm_set_charge(port, false);
> +		if (port->pd_capable)
> +			tcpm_set_charge(port, false);
>   		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
>   			       TYPEC_DEVICE);
>   		/*
> @@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
>   		 * Similar, dual-mode ports in source mode should transition
>   		 * to PE_SNK_Transition_to_default.
>   		 */
> +		if (port->pd_capable) {
> +			tcpm_set_current_limit(port,
> +					       tcpm_get_current_limit(port),
> +					       5000);
> +			tcpm_set_charge(port, true);
> +		}
>   		tcpm_set_attached_state(port, true);
>   		tcpm_set_state(port, SNK_STARTUP, 0);
>   		break;
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
  2018-09-28 23:47 ` [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
  2018-09-30 22:04   ` Guenter Roeck
@ 2018-10-01  9:53   ` Heikki Krogerus
  2018-10-01 19:45     ` Badhri Jagan Sridharan
  1 sibling, 1 reply; 8+ messages in thread
From: Heikki Krogerus @ 2018-10-01  9:53 UTC (permalink / raw)
  To: Badhri Jagan Sridharan
  Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Chanwoo Choi,
	Andrzej Hajda, Li Jun, linux, devicetree, linux-kernel,
	linux-usb

On Fri, Sep 28, 2018 at 04:47:02PM -0700, Badhri Jagan Sridharan wrote:
> During HARD_RESET the data link is disconnected.
> For self powered device, the spec is advising against doing that.
> 
> >From USB_PD_R3_0
> 7.1.5 Response to Hard Resets
> Device operation during and after a Hard Reset is defined as follows:
> Self-powered devices Should Not disconnect from USB during a Hard Reset
> (see Section 9.1.2).
> Bus powered devices will disconnect from USB during a Hard Reset due to the
> loss of their power source.
> 
> Tackle this by letting TCPM know whether the device is self or bus powered.
> 
> This overcomes unnecessary port disconnections from hard reset.
> Also, speeds up the enumeration time when connected to Type-A ports.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

Could you rebase these on top of the latest usb-next. These are not
going to apply cleanly on top of it because tcpm.c and all port
drivers are now in their own directory drivers/usb/typec/tcpm/.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---------
> Version history:
> 
> V2:
> Based on feedback from heikki.krogerus@linux.intel.com
> - self_powered added to the struct tcpm_port which is populated from
>   a. "connector" node of the device tree in tcpm_fw_get_caps()
>   b. "self_powered" node of the tcpc_config in tcpm_copy_caps
> 
> Based on feedbase from linux@roeck-us.net
> - Code was refactored
> - SRC_HARD_RESET_VBUS_OFF sets the link state to false based
>   on self_powered flag
> 
> V1 located here:
> https://lkml.org/lkml/2018/9/13/94
> ---
>  drivers/usb/typec/tcpm.c | 12 ++++++++++--
>  include/linux/usb/tcpm.h |  1 +
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 4f1f4215f3d6e..c3ac0e46106b4 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -317,6 +317,9 @@ struct tcpm_port {
>  	/* Deadline in jiffies to exit src_try_wait state */
>  	unsigned long max_wait;
>  
> +	/* port belongs to a self powered device */
> +	bool self_powered;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	struct dentry *dentry;
>  	struct mutex logbuffer_lock;	/* log buffer access lock */
> @@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
>  	case SRC_HARD_RESET_VBUS_OFF:
>  		tcpm_set_vconn(port, true);
>  		tcpm_set_vbus(port, false);
> -		tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
> +		tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
> +			       TYPEC_HOST);
>  		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
>  		break;
>  	case SRC_HARD_RESET_VBUS_ON:
> @@ -3270,7 +3274,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		memset(&port->pps_data, 0, sizeof(port->pps_data));
>  		tcpm_set_vconn(port, false);
>  		tcpm_set_charge(port, false);
> -		tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> +		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
> +			       TYPEC_DEVICE);
>  		/*
>  		 * VBUS may or may not toggle, depending on the adapter.
>  		 * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
> @@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>  		return -EINVAL;
>  	port->operating_snk_mw = mw / 1000;
>  
> +	port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
> +
>  	return 0;
>  }
>  
> @@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
>  	port->typec_caps.prefer_role = tcfg->default_role;
>  	port->typec_caps.type = tcfg->type;
>  	port->typec_caps.data = tcfg->data;
> +	port->self_powered = port->tcpc->config->self_powered;
>  
>  	return 0;
>  }
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 7e7fbfb84e8e3..50c74a77db55c 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -89,6 +89,7 @@ struct tcpc_config {
>  	enum typec_port_data data;
>  	enum typec_role default_role;
>  	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
> +	bool self_powered;	/* port belongs to a self powered device */
>  
>  	const struct typec_altmode_desc *alt_modes;
>  };

Thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
  2018-10-01  9:53   ` Heikki Krogerus
@ 2018-10-01 19:45     ` Badhri Jagan Sridharan
  0 siblings, 0 replies; 8+ messages in thread
From: Badhri Jagan Sridharan @ 2018-10-01 19:45 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: robh+dt, mark.rutland, Greg Kroah-Hartman, cw00.choi, a.hajda,
	jun.li, Guenter Roeck, devicetree, LKML, USB

Sure. Just resent the patch stack !
On Mon, Oct 1, 2018 at 2:54 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Fri, Sep 28, 2018 at 04:47:02PM -0700, Badhri Jagan Sridharan wrote:
> > During HARD_RESET the data link is disconnected.
> > For self powered device, the spec is advising against doing that.
> >
> > >From USB_PD_R3_0
> > 7.1.5 Response to Hard Resets
> > Device operation during and after a Hard Reset is defined as follows:
> > Self-powered devices Should Not disconnect from USB during a Hard Reset
> > (see Section 9.1.2).
> > Bus powered devices will disconnect from USB during a Hard Reset due to the
> > loss of their power source.
> >
> > Tackle this by letting TCPM know whether the device is self or bus powered.
> >
> > This overcomes unnecessary port disconnections from hard reset.
> > Also, speeds up the enumeration time when connected to Type-A ports.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
>
> Could you rebase these on top of the latest usb-next. These are not
> going to apply cleanly on top of it because tcpm.c and all port
> drivers are now in their own directory drivers/usb/typec/tcpm/.
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> > ---------
> > Version history:
> >
> > V2:
> > Based on feedback from heikki.krogerus@linux.intel.com
> > - self_powered added to the struct tcpm_port which is populated from
> >   a. "connector" node of the device tree in tcpm_fw_get_caps()
> >   b. "self_powered" node of the tcpc_config in tcpm_copy_caps
> >
> > Based on feedbase from linux@roeck-us.net
> > - Code was refactored
> > - SRC_HARD_RESET_VBUS_OFF sets the link state to false based
> >   on self_powered flag
> >
> > V1 located here:
> > https://lkml.org/lkml/2018/9/13/94
> > ---
> >  drivers/usb/typec/tcpm.c | 12 ++++++++++--
> >  include/linux/usb/tcpm.h |  1 +
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > index 4f1f4215f3d6e..c3ac0e46106b4 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -317,6 +317,9 @@ struct tcpm_port {
> >       /* Deadline in jiffies to exit src_try_wait state */
> >       unsigned long max_wait;
> >
> > +     /* port belongs to a self powered device */
> > +     bool self_powered;
> > +
> >  #ifdef CONFIG_DEBUG_FS
> >       struct dentry *dentry;
> >       struct mutex logbuffer_lock;    /* log buffer access lock */
> > @@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
> >       case SRC_HARD_RESET_VBUS_OFF:
> >               tcpm_set_vconn(port, true);
> >               tcpm_set_vbus(port, false);
> > -             tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
> > +             tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
> > +                            TYPEC_HOST);
> >               tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
> >               break;
> >       case SRC_HARD_RESET_VBUS_ON:
> > @@ -3270,7 +3274,8 @@ static void run_state_machine(struct tcpm_port *port)
> >               memset(&port->pps_data, 0, sizeof(port->pps_data));
> >               tcpm_set_vconn(port, false);
> >               tcpm_set_charge(port, false);
> > -             tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> > +             tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
> > +                            TYPEC_DEVICE);
> >               /*
> >                * VBUS may or may not toggle, depending on the adapter.
> >                * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
> > @@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> >               return -EINVAL;
> >       port->operating_snk_mw = mw / 1000;
> >
> > +     port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
> > +
> >       return 0;
> >  }
> >
> > @@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
> >       port->typec_caps.prefer_role = tcfg->default_role;
> >       port->typec_caps.type = tcfg->type;
> >       port->typec_caps.data = tcfg->data;
> > +     port->self_powered = port->tcpc->config->self_powered;
> >
> >       return 0;
> >  }
> > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > index 7e7fbfb84e8e3..50c74a77db55c 100644
> > --- a/include/linux/usb/tcpm.h
> > +++ b/include/linux/usb/tcpm.h
> > @@ -89,6 +89,7 @@ struct tcpc_config {
> >       enum typec_port_data data;
> >       enum typec_role default_role;
> >       bool try_role_hw;       /* try.{src,snk} implemented in hardware */
> > +     bool self_powered;      /* port belongs to a self powered device */
> >
> >       const struct typec_altmode_desc *alt_modes;
> >  };
>
> Thanks,
>
> --
> heikki

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-01 19:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-28 23:47 [PATCH 1/3] [PATCH 1/3] dt-bindings: connector: Add self-powered property Badhri Jagan Sridharan
2018-09-28 23:47 ` [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
2018-09-30 22:04   ` Guenter Roeck
2018-10-01  9:53   ` Heikki Krogerus
2018-10-01 19:45     ` Badhri Jagan Sridharan
2018-09-28 23:47 ` [PATCH 3/3] [PATCH v2 3/3] usb: typec: tcpm: charge current handling for sink during hard reset Badhri Jagan Sridharan
2018-09-28 23:48   ` Badhri Jagan Sridharan
2018-09-30 22:05   ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).