Skip to contentSkip to conversation

Add support for time DT property in TCPM

RFC Superseded Amit Sunil Dhamne 4 participants 8 messages Sep 19, 2024 – Sep 21, 2024

Subsystems (4)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover Add support for time DT property in TCPM

USB PD specification defines a bunch of timers that can have a range of acceptable values instead of specific values. These values have to be tuned based on the platform. However, TCPM currently sets them to a default value without providing a mechanism to set platform specific values.

This patchset adds new DT properties per timer to allow users to define platform specific values.

Changes compared to v1:
- Defined new properties per timer that we are interested in rather than defining a single pd-timers u32 array property.
- Better description of the timer properties.
- Since subject has changed, adding link for previous patchset for

    posterity:
    https://lore.kernel.org/all/[email protected]/

Amit Sunil Dhamne (2):
dt-bindings: connector: Add properties to define time values
usb: typec: tcpm: Add support for parsing time dt properties

 .../bindings/connector/usb-connector.yaml     | 32 +++++++-
 drivers/usb/typec/tcpm/tcpm.c                 | 81 +++++++++++++++----
 2 files changed, 96 insertions(+), 17 deletions(-)

base-commit: 68d4209158f43a558c5553ea95ab0c8975eab18c

Signature · 1 line
2.46.0.792.g87dc391469-goog
#1 lore raw

RFC v2 1/2 dt-bindings: connector: Add properties to define time values

This commit adds the following properties:
* sink-wait-cap-time-ms
* ps-source-off-time-ms
* cc-debounce-time-ms

This is to enable setting of platform/board specific timer values as these timers have a range of acceptable values.

---
 .../bindings/connector/usb-connector.yaml     | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/connector/usb-connector.yaml +31 -1
index fb216ce68bb3..5efa4ea3c41c 100644
@@ -253,6 +253,33 @@ properties:
 
     additionalProperties: false
 
+  sink-wait-cap-time-ms:
+    description: Represents the max time in ms that USB Type-C port (in sink
+      role) should wait for the port partner (source role) to send source caps.
+      SinkWaitCap timer starts when port in sink role attaches to the source.
+      This timer will stop when sink receives PD source cap advertisement before
+      timeout in which case it'll move to capability negotiation stage. A
+      timeout leads to a hard reset message by the port. If no value is set for
+      this property then the driver assumes a default value of PD_T_SINK_WAIT_CAP
+      (defined in include/linux/usb/pd.h).
+
+  ps-source-off-time-ms:
+    description: Represents the max time in ms that a DRP in source role should
+      take to turn off power after the PsSourceOff timer starts. PsSourceOff
+      timer starts when a sink's PHY layer receives EOP of the GoodCRC message
+      (corresponding to an Accept message sent in response to a PR_Swap or a
+      FR_Swap request). This timer stops when last bit of GoodCRC EOP
+      corresponding to the received PS_RDY message is transmitted by the PHY
+      layer. A timeout shall lead to error recovery in the type-c port. If no
+      value is set for this property then the driver assumes a default value of
+      PD_T_PS_SOURCE_OFF (defined in include/linux/usb/pd.h).
+
+  cc-debounce-time-ms:
+    description: Represents the max time in ms that a port shall wait to
+      determine if it's attached to a partner. If no value is set for this
+      property then the driver assumes a default value of PD_T_CC_DEBOUNCE
+      (defined in include/linux/usb/pd.h).
+
 dependencies:
   sink-vdos-v1: [ sink-vdos ]
   sink-vdos: [ sink-vdos-v1 ]
@@ -380,7 +407,7 @@ examples:
     };
 
   # USB-C connector attached to a typec port controller(ptn5110), which has
-  # power delivery support and enables drp.
+  # power delivery support, explicitly defines time properties and enables drp.
   - |
     #include <dt-bindings/usb/pd.h>
     typec: ptn5110 {
@@ -393,6 +420,9 @@ examples:
             sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_USB_COMM)
                          PDO_VAR(5000, 12000, 2000)>;
             op-sink-microwatt = <10000000>;
+            sink-wait-cap-time-ms = <465>;
+            ps-source-off-time-ms = <835>;
+            cc-debounce-time-ms = <101>;
         };
     };
 
Signature · 1 line
2.46.0.792.g87dc391469-goog
maintainer
#2 lore raw

Re: [RFC v2 1/2] dt-bindings: connector: Add properties to define time values

Quoted 7 lines · Amit Sunil Dhamne

This commit adds the following properties:
* sink-wait-cap-time-ms
* ps-source-off-time-ms
* cc-debounce-time-ms

This is to enable setting of platform/board specific timer values as these timers have a range of acceptable values.

What's acceptable? Put that in constraints for the properties.

Quoted 13 lines
---
 .../bindings/connector/usb-connector.yaml     | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/connector/usb-connector.yaml +0 -0
index fb216ce68bb3..5efa4ea3c41c 100644
@@ -253,6 +253,33 @@ properties:
 
     additionalProperties: false

Documentation/devicetree/bindings/connector/usb-connector.yaml properties: · line 264

Quoted 10 lines
Documentation/devicetree/bindings/connector/usb-connector.yaml +9 -0
 
+  sink-wait-cap-time-ms:
+    description: Represents the max time in ms that USB Type-C port (in sink
+      role) should wait for the port partner (source role) to send source caps.
+      SinkWaitCap timer starts when port in sink role attaches to the source.
+      This timer will stop when sink receives PD source cap advertisement before
+      timeout in which case it'll move to capability negotiation stage. A
+      timeout leads to a hard reset message by the port. If no value is set for
+      this property then the driver assumes a default value of PD_T_SINK_WAIT_CAP
+      (defined in include/linux/usb/pd.h).

Express default as "default: N". Sorry, defines don't work here.

#3 lore raw

RFC v2 2/2 usb: typec: tcpm: Add support for parsing time dt properties

In reply to Amit Sunil Dhamne

Add support for DT time properties to allow users to define platform specific timing deadlines of certain timers rather than using hardcoded ones. For values that have not been explicitly defined in DT using this property, default values will be set therefore, making this change backward compatible.

---
 drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++-------
 1 file changed, 65 insertions(+), 16 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +65 -16
index 4b02d6474259..e6c243bc44f7 100644
@@ -310,6 +310,17 @@ struct pd_data {
 	unsigned int operating_snk_mw;
 };
 
+/*
+ * @sink_wait_cap_time: Deadline (in ms) for tTypeCSinkWaitCap timer
+ * @ps_src_wait_off_time: Deadline (in ms) for tPSSourceOff timer
+ * @cc_debounce_time: Deadline (in ms) for tCCDebounce timer
+ */
+struct pd_timings {
+	u32 sink_wait_cap_time;
+	u32 ps_src_off_time;
+	u32 cc_debounce_time;
+};
+
 struct tcpm_port {
 	struct device *dev;
 
@@ -552,6 +563,9 @@ struct tcpm_port {
 	 */
 	unsigned int message_id_prime;
 	unsigned int rx_msgid_prime;
+
+	/* Timer deadline values configured at runtime */
+	struct pd_timings timings;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -4639,15 +4653,15 @@ static void run_state_machine(struct tcpm_port *port)
 	case SRC_ATTACH_WAIT:
 		if (tcpm_port_is_debug(port))
 			tcpm_set_state(port, DEBUG_ACC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		else if (tcpm_port_is_audio(port))
 			tcpm_set_state(port, AUDIO_ACC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
 			tcpm_set_state(port,
 				       tcpm_try_snk(port) ? SNK_TRY
 							  : SRC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		break;
 
 	case SNK_TRY:
@@ -4698,7 +4712,7 @@ static void run_state_machine(struct tcpm_port *port)
 		}
 		break;
 	case SRC_TRYWAIT_DEBOUNCE:
-		tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, SRC_ATTACHED, port->timings.cc_debounce_time);
 		break;
 	case SRC_TRYWAIT_UNATTACHED:
 		tcpm_set_state(port, SNK_UNATTACHED, 0);
@@ -4901,7 +4915,7 @@ static void run_state_machine(struct tcpm_port *port)
 		    (port->cc1 != TYPEC_CC_OPEN &&
 		     port->cc2 == TYPEC_CC_OPEN))
 			tcpm_set_state(port, SNK_DEBOUNCED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		else if (tcpm_port_is_disconnected(port))
 			tcpm_set_state(port, SNK_UNATTACHED,
 				       PD_T_PD_DEBOUNCE);
@@ -4941,7 +4955,7 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case SNK_TRYWAIT:
 		tcpm_set_cc(port, TYPEC_CC_RD);
-		tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, SNK_TRYWAIT_VBUS, port->timings.cc_debounce_time);
 		break;
 	case SNK_TRYWAIT_VBUS:
 		/*
@@ -5014,7 +5028,7 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case SNK_DISCOVERY_DEBOUNCE:
 		tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE,
-			       PD_T_CC_DEBOUNCE);
+			       port->timings.cc_debounce_time);
 		break;
 	case SNK_DISCOVERY_DEBOUNCE_DONE:
 		if (!tcpm_port_is_disconnected(port) &&
@@ -5041,10 +5055,10 @@ static void run_state_machine(struct tcpm_port *port)
 		if (port->vbus_never_low) {
 			port->vbus_never_low = false;
 			tcpm_set_state(port, SNK_SOFT_RESET,
-				       PD_T_SINK_WAIT_CAP);
+				       port->timings.sink_wait_cap_time);
 		} else {
 			tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT,
-				       PD_T_SINK_WAIT_CAP);
+				       port->timings.sink_wait_cap_time);
 		}
 		break;
 	case SNK_WAIT_CAPABILITIES_TIMEOUT:
@@ -5066,7 +5080,8 @@ static void run_state_machine(struct tcpm_port *port)
 		if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP))
 			tcpm_set_state_cond(port, hard_reset_state(port), 0);
 		else
-			tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP);
+			tcpm_set_state(port, hard_reset_state(port),
+				       port->timings.sink_wait_cap_time);
 		break;
 	case SNK_NEGOTIATE_CAPABILITIES:
 		port->pd_capable = true;
@@ -5203,7 +5218,7 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_set_state(port, ACC_UNATTACHED, 0);
 		break;
 	case AUDIO_ACC_DEBOUNCE:
-		tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, ACC_UNATTACHED, port->timings.cc_debounce_time);
 		break;
 
 	/* Hard_Reset states */
@@ -5420,7 +5435,7 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state(port, ERROR_RECOVERY, 0);
 		break;
 	case FR_SWAP_SNK_SRC_TRANSITION_TO_OFF:
-		tcpm_set_state(port, ERROR_RECOVERY, PD_T_PS_SOURCE_OFF);
+		tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
 		break;
 	case FR_SWAP_SNK_SRC_NEW_SINK_READY:
 		if (port->vbus_source)
@@ -5475,7 +5490,7 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_cc(port, TYPEC_CC_RD);
 		/* allow CC debounce */
 		tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED,
-			       PD_T_CC_DEBOUNCE);
+			       port->timings.cc_debounce_time);
 		break;
 	case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
 		/*
@@ -5510,7 +5525,7 @@ static void run_state_machine(struct tcpm_port *port)
 						       port->pps_data.active, 0);
 		tcpm_set_charge(port, false);
 		tcpm_set_state(port, hard_reset_state(port),
-			       PD_T_PS_SOURCE_OFF);
+			       port->timings.ps_src_off_time);
 		break;
 	case PR_SWAP_SNK_SRC_SOURCE_ON:
 		tcpm_enable_auto_vbus_discharge(port, true);
@@ -5666,7 +5681,7 @@ static void run_state_machine(struct tcpm_port *port)
 	case PORT_RESET_WAIT_OFF:
 		tcpm_set_state(port,
 			       tcpm_default_state(port),
-			       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
+			       port->vbus_present ? port->timings.ps_src_off_time : 0);
 		break;
 
 	/* AMS intermediate state */
@@ -6157,7 +6172,7 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
 	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);
+				       port->timings.cc_debounce_time);
 		break;
 	case SRC_STARTUP:
 	case SRC_SEND_CAPABILITIES:
@@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port)
 	return ret;
 }
 
+static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+	int ret;
+	u32 val;
+
+	if (!fwnode)
+		return -EINVAL;
+
+	ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val);
+	if (!ret)
+		port->timings.sink_wait_cap_time = val;
+	else
+		port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP;
+
+	ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val);
+	if (!ret)
+		port->timings.ps_src_off_time = val;
+	else
+		port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF;
+
+	ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val);
+	if (!ret)
+		port->timings.cc_debounce_time = val;
+	else
+		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
+
+	return 0;
+}
+
 static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *capabilities, *child, *caps = NULL;
@@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	init_completion(&port->pps_complete);
 	tcpm_debugfs_init(port);
 
+	err = tcpm_fw_get_timings(port, tcpc->fwnode);
+	if (err < 0)
+		goto out_destroy_wq;
+
 	err = tcpm_fw_get_caps(port, tcpc->fwnode);
 	if (err < 0)
 		goto out_destroy_wq;
+
 	err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode);
 	if (err < 0)
 		goto out_destroy_wq;
Signature · 1 line
2.46.0.792.g87dc391469-goog
Acked-by on v2 2/2
#4 lore raw

Re: [RFC v2 2/2] usb: typec: tcpm: Add support for parsing time dt properties

Quoted 12 lines · Amit Sunil Dhamne

Add support for DT time properties to allow users to define platform specific timing deadlines of certain timers rather than using hardcoded ones. For values that have not been explicitly defined in DT using this property, default values will be set therefore, making this change backward compatible.

---
 drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++-------
 1 file changed, 65 insertions(+), 16 deletions(-)

drivers/usb/typec/tcpm/tcpm.c static int tcpm_port_register_pd(struct tcpm_port *port) · line 7077

Quoted 10 lines
drivers/usb/typec/tcpm/tcpm.c +7 -0
@@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port)
 	return ret;
 }
 
+static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+	int ret;
+	u32 val;
+
+	if (!fwnode)
+		return -EINVAL;

I think this check isn't really necessary here. Other than that:

Quoted 43 lines
drivers/usb/typec/tcpm/tcpm.c +27 -0
+
+	ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val);
+	if (!ret)
+		port->timings.sink_wait_cap_time = val;
+	else
+		port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP;
+
+	ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val);
+	if (!ret)
+		port->timings.ps_src_off_time = val;
+	else
+		port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF;
+
+	ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val);
+	if (!ret)
+		port->timings.cc_debounce_time = val;
+	else
+		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
+
+	return 0;
+}
+
 static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *capabilities, *child, *caps = NULL;
@@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	init_completion(&port->pps_complete);
 	tcpm_debugfs_init(port);
 
+	err = tcpm_fw_get_timings(port, tcpc->fwnode);
+	if (err < 0)
+		goto out_destroy_wq;
+
 	err = tcpm_fw_get_caps(port, tcpc->fwnode);
 	if (err < 0)
 		goto out_destroy_wq;
+
 	err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode);
 	if (err < 0)
 		goto out_destroy_wq;
Signature · 1 line
2.46.0.792.g87dc391469-goog
Signature · 2 lines
With best wishes
Dmitry
#5 lore raw

Re: [RFC v2 2/2] usb: typec: tcpm: Add support for parsing time dt properties

Hi Dmitry,

Thanks for the review!

Quoted 24 lines · Dmitry Baryshkov
Quoted 11 lines · Amit Sunil Dhamne

Add support for DT time properties to allow users to define platform specific timing deadlines of certain timers rather than using hardcoded ones. For values that have not been explicitly defined in DT using this property, default values will be set therefore, making this change backward compatible.

---
  drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++-------
  1 file changed, 65 insertions(+), 16 deletions(-)

drivers/usb/typec/tcpm/tcpm.c static int tcpm_port_register_pd(struct tcpm_port *port) · line 7077

Quoted 10 lines
drivers/usb/typec/tcpm/tcpm.c +7 -0
@@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port)
  	return ret;
  }
 
+static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+	int ret;
+	u32 val;
+
+	if (!fwnode)
+		return -EINVAL;

I think this check isn't really necessary here. Other than that:

Quoted 2 lines

Will remove this check in next rev as suggested in https://lore.kernel.org/all/[email protected]/ .

Regards,

Amit

Quoted 43 lines
Quoted 43 lines
drivers/usb/typec/tcpm/tcpm.c +27 -0
+
+	ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val);
+	if (!ret)
+		port->timings.sink_wait_cap_time = val;
+	else
+		port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP;
+
+	ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val);
+	if (!ret)
+		port->timings.ps_src_off_time = val;
+	else
+		port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF;
+
+	ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val);
+	if (!ret)
+		port->timings.cc_debounce_time = val;
+	else
+		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
+
+	return 0;
+}
+
  static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
  {
  	struct fwnode_handle *capabilities, *child, *caps = NULL;
@@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
  	init_completion(&port->pps_complete);
  	tcpm_debugfs_init(port);
 
+	err = tcpm_fw_get_timings(port, tcpc->fwnode);
+	if (err < 0)
+		goto out_destroy_wq;
+
  	err = tcpm_fw_get_caps(port, tcpc->fwnode);
  	if (err < 0)
  		goto out_destroy_wq;
+
  	err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode);
  	if (err < 0)
  		goto out_destroy_wq;
Signature · 1 line
2.46.0.792.g87dc391469-goog
maintainer
#6 lore raw

Re: [RFC v2 2/2] usb: typec: tcpm: Add support for parsing time dt properties

In reply to Amit Sunil Dhamne
Quoted 212 lines · Amit Sunil Dhamne

Add support for DT time properties to allow users to define platform specific timing deadlines of certain timers rather than using hardcoded ones. For values that have not been explicitly defined in DT using this property, default values will be set therefore, making this change backward compatible.

---
 drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++-------
 1 file changed, 65 insertions(+), 16 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +60 -16
index 4b02d6474259..e6c243bc44f7 100644
@@ -310,6 +310,17 @@ struct pd_data {
 	unsigned int operating_snk_mw;
 };
 
+/*
+ * @sink_wait_cap_time: Deadline (in ms) for tTypeCSinkWaitCap timer
+ * @ps_src_wait_off_time: Deadline (in ms) for tPSSourceOff timer
+ * @cc_debounce_time: Deadline (in ms) for tCCDebounce timer
+ */
+struct pd_timings {
+	u32 sink_wait_cap_time;
+	u32 ps_src_off_time;
+	u32 cc_debounce_time;
+};
+
 struct tcpm_port {
 	struct device *dev;
 
@@ -552,6 +563,9 @@ struct tcpm_port {
 	 */
 	unsigned int message_id_prime;
 	unsigned int rx_msgid_prime;
+
+	/* Timer deadline values configured at runtime */
+	struct pd_timings timings;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -4639,15 +4653,15 @@ static void run_state_machine(struct tcpm_port *port)
 	case SRC_ATTACH_WAIT:
 		if (tcpm_port_is_debug(port))
 			tcpm_set_state(port, DEBUG_ACC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		else if (tcpm_port_is_audio(port))
 			tcpm_set_state(port, AUDIO_ACC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
 			tcpm_set_state(port,
 				       tcpm_try_snk(port) ? SNK_TRY
 							  : SRC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		break;
 
 	case SNK_TRY:
@@ -4698,7 +4712,7 @@ static void run_state_machine(struct tcpm_port *port)
 		}
 		break;
 	case SRC_TRYWAIT_DEBOUNCE:
-		tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, SRC_ATTACHED, port->timings.cc_debounce_time);
 		break;
 	case SRC_TRYWAIT_UNATTACHED:
 		tcpm_set_state(port, SNK_UNATTACHED, 0);
@@ -4901,7 +4915,7 @@ static void run_state_machine(struct tcpm_port *port)
 		    (port->cc1 != TYPEC_CC_OPEN &&
 		     port->cc2 == TYPEC_CC_OPEN))
 			tcpm_set_state(port, SNK_DEBOUNCED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
 		else if (tcpm_port_is_disconnected(port))
 			tcpm_set_state(port, SNK_UNATTACHED,
 				       PD_T_PD_DEBOUNCE);
@@ -4941,7 +4955,7 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case SNK_TRYWAIT:
 		tcpm_set_cc(port, TYPEC_CC_RD);
-		tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, SNK_TRYWAIT_VBUS, port->timings.cc_debounce_time);
 		break;
 	case SNK_TRYWAIT_VBUS:
 		/*
@@ -5014,7 +5028,7 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case SNK_DISCOVERY_DEBOUNCE:
 		tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE,
-			       PD_T_CC_DEBOUNCE);
+			       port->timings.cc_debounce_time);
 		break;
 	case SNK_DISCOVERY_DEBOUNCE_DONE:
 		if (!tcpm_port_is_disconnected(port) &&
@@ -5041,10 +5055,10 @@ static void run_state_machine(struct tcpm_port *port)
 		if (port->vbus_never_low) {
 			port->vbus_never_low = false;
 			tcpm_set_state(port, SNK_SOFT_RESET,
-				       PD_T_SINK_WAIT_CAP);
+				       port->timings.sink_wait_cap_time);
 		} else {
 			tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT,
-				       PD_T_SINK_WAIT_CAP);
+				       port->timings.sink_wait_cap_time);
 		}
 		break;
 	case SNK_WAIT_CAPABILITIES_TIMEOUT:
@@ -5066,7 +5080,8 @@ static void run_state_machine(struct tcpm_port *port)
 		if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP))
 			tcpm_set_state_cond(port, hard_reset_state(port), 0);
 		else
-			tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP);
+			tcpm_set_state(port, hard_reset_state(port),
+				       port->timings.sink_wait_cap_time);
 		break;
 	case SNK_NEGOTIATE_CAPABILITIES:
 		port->pd_capable = true;
@@ -5203,7 +5218,7 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_set_state(port, ACC_UNATTACHED, 0);
 		break;
 	case AUDIO_ACC_DEBOUNCE:
-		tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, ACC_UNATTACHED, port->timings.cc_debounce_time);
 		break;
 
 	/* Hard_Reset states */
@@ -5420,7 +5435,7 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state(port, ERROR_RECOVERY, 0);
 		break;
 	case FR_SWAP_SNK_SRC_TRANSITION_TO_OFF:
-		tcpm_set_state(port, ERROR_RECOVERY, PD_T_PS_SOURCE_OFF);
+		tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
 		break;
 	case FR_SWAP_SNK_SRC_NEW_SINK_READY:
 		if (port->vbus_source)
@@ -5475,7 +5490,7 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_cc(port, TYPEC_CC_RD);
 		/* allow CC debounce */
 		tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED,
-			       PD_T_CC_DEBOUNCE);
+			       port->timings.cc_debounce_time);
 		break;
 	case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
 		/*
@@ -5510,7 +5525,7 @@ static void run_state_machine(struct tcpm_port *port)
 						       port->pps_data.active, 0);
 		tcpm_set_charge(port, false);
 		tcpm_set_state(port, hard_reset_state(port),
-			       PD_T_PS_SOURCE_OFF);
+			       port->timings.ps_src_off_time);
 		break;
 	case PR_SWAP_SNK_SRC_SOURCE_ON:
 		tcpm_enable_auto_vbus_discharge(port, true);
@@ -5666,7 +5681,7 @@ static void run_state_machine(struct tcpm_port *port)
 	case PORT_RESET_WAIT_OFF:
 		tcpm_set_state(port,
 			       tcpm_default_state(port),
-			       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
+			       port->vbus_present ? port->timings.ps_src_off_time : 0);
 		break;
 
 	/* AMS intermediate state */
@@ -6157,7 +6172,7 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
 	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);
+				       port->timings.cc_debounce_time);
 		break;
 	case SRC_STARTUP:
 	case SRC_SEND_CAPABILITIES:
@@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port)
 	return ret;
 }
 
+static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+	int ret;
+	u32 val;
+
+	if (!fwnode)
+		return -EINVAL;
+
+	ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val);
+	if (!ret)
+		port->timings.sink_wait_cap_time = val;
+	else
+		port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP;
+
+	ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val);
+	if (!ret)
+		port->timings.ps_src_off_time = val;
+	else
+		port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF;
+
+	ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val);
+	if (!ret)
+		port->timings.cc_debounce_time = val;
+	else
+		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
+
+	return 0;
+}
+
 static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *capabilities, *child, *caps = NULL;

drivers/usb/typec/tcpm/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 7657

Quoted 6 lines
drivers/usb/typec/tcpm/tcpm.c +3 -0
@@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	init_completion(&port->pps_complete);
 	tcpm_debugfs_init(port);
 
+	err = tcpm_fw_get_timings(port, tcpc->fwnode);
+	if (err < 0)
+		goto out_destroy_wq;

This is somehow wrong. You are using default values in case of failure, so this should not be a reason to fail port registration under any circumstance. That function should just return void.

I would also just call it after tcpm_fw_get_caps() (or maybe even from tcpm_fw_get_caps()), because tcpm_fw_get_caps() checks fwnode in any case.

drivers/usb/typec/tcpm/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 7665

Quoted 7 lines
drivers/usb/typec/tcpm/tcpm.c +1 -0
 	err = tcpm_fw_get_caps(port, tcpc->fwnode);
 	if (err < 0)
 		goto out_destroy_wq;
+
 	err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode);
 	if (err < 0)
 		goto out_destroy_wq;

thanks,

Signature · 1 line
heikki
#7 lore raw

Re: [RFC v2 2/2] usb: typec: tcpm: Add support for parsing time dt properties

Hi Heikki,

Quoted 223 lines · Heikki Krogerus
Quoted 212 lines · Amit Sunil Dhamne

Add support for DT time properties to allow users to define platform specific timing deadlines of certain timers rather than using hardcoded ones. For values that have not been explicitly defined in DT using this property, default values will be set therefore, making this change backward compatible.

---
  drivers/usb/typec/tcpm/tcpm.c | 81 ++++++++++++++++++++++++++++-------
  1 file changed, 65 insertions(+), 16 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +60 -16
index 4b02d6474259..e6c243bc44f7 100644
@@ -310,6 +310,17 @@ struct pd_data {
  	unsigned int operating_snk_mw;
  };
 
+/*
+ * @sink_wait_cap_time: Deadline (in ms) for tTypeCSinkWaitCap timer
+ * @ps_src_wait_off_time: Deadline (in ms) for tPSSourceOff timer
+ * @cc_debounce_time: Deadline (in ms) for tCCDebounce timer
+ */
+struct pd_timings {
+	u32 sink_wait_cap_time;
+	u32 ps_src_off_time;
+	u32 cc_debounce_time;
+};
+
  struct tcpm_port {
  	struct device *dev;
 
@@ -552,6 +563,9 @@ struct tcpm_port {
  	 */
  	unsigned int message_id_prime;
  	unsigned int rx_msgid_prime;
+
+	/* Timer deadline values configured at runtime */
+	struct pd_timings timings;
  #ifdef CONFIG_DEBUG_FS
  	struct dentry *dentry;
  	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -4639,15 +4653,15 @@ static void run_state_machine(struct tcpm_port *port)
  	case SRC_ATTACH_WAIT:
  		if (tcpm_port_is_debug(port))
  			tcpm_set_state(port, DEBUG_ACC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
  		else if (tcpm_port_is_audio(port))
  			tcpm_set_state(port, AUDIO_ACC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
  		else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
  			tcpm_set_state(port,
  				       tcpm_try_snk(port) ? SNK_TRY
  							  : SRC_ATTACHED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
  		break;
 
  	case SNK_TRY:
@@ -4698,7 +4712,7 @@ static void run_state_machine(struct tcpm_port *port)
  		}
  		break;
  	case SRC_TRYWAIT_DEBOUNCE:
-		tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, SRC_ATTACHED, port->timings.cc_debounce_time);
  		break;
  	case SRC_TRYWAIT_UNATTACHED:
  		tcpm_set_state(port, SNK_UNATTACHED, 0);
@@ -4901,7 +4915,7 @@ static void run_state_machine(struct tcpm_port *port)
  		    (port->cc1 != TYPEC_CC_OPEN &&
  		     port->cc2 == TYPEC_CC_OPEN))
  			tcpm_set_state(port, SNK_DEBOUNCED,
-				       PD_T_CC_DEBOUNCE);
+				       port->timings.cc_debounce_time);
  		else if (tcpm_port_is_disconnected(port))
  			tcpm_set_state(port, SNK_UNATTACHED,
  				       PD_T_PD_DEBOUNCE);
@@ -4941,7 +4955,7 @@ static void run_state_machine(struct tcpm_port *port)
  		break;
  	case SNK_TRYWAIT:
  		tcpm_set_cc(port, TYPEC_CC_RD);
-		tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, SNK_TRYWAIT_VBUS, port->timings.cc_debounce_time);
  		break;
  	case SNK_TRYWAIT_VBUS:
  		/*
@@ -5014,7 +5028,7 @@ static void run_state_machine(struct tcpm_port *port)
  		break;
  	case SNK_DISCOVERY_DEBOUNCE:
  		tcpm_set_state(port, SNK_DISCOVERY_DEBOUNCE_DONE,
-			       PD_T_CC_DEBOUNCE);
+			       port->timings.cc_debounce_time);
  		break;
  	case SNK_DISCOVERY_DEBOUNCE_DONE:
  		if (!tcpm_port_is_disconnected(port) &&
@@ -5041,10 +5055,10 @@ static void run_state_machine(struct tcpm_port *port)
  		if (port->vbus_never_low) {
  			port->vbus_never_low = false;
  			tcpm_set_state(port, SNK_SOFT_RESET,
-				       PD_T_SINK_WAIT_CAP);
+				       port->timings.sink_wait_cap_time);
  		} else {
  			tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT,
-				       PD_T_SINK_WAIT_CAP);
+				       port->timings.sink_wait_cap_time);
  		}
  		break;
  	case SNK_WAIT_CAPABILITIES_TIMEOUT:
@@ -5066,7 +5080,8 @@ static void run_state_machine(struct tcpm_port *port)
  		if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP))
  			tcpm_set_state_cond(port, hard_reset_state(port), 0);
  		else
-			tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP);
+			tcpm_set_state(port, hard_reset_state(port),
+				       port->timings.sink_wait_cap_time);
  		break;
  	case SNK_NEGOTIATE_CAPABILITIES:
  		port->pd_capable = true;
@@ -5203,7 +5218,7 @@ static void run_state_machine(struct tcpm_port *port)
  			tcpm_set_state(port, ACC_UNATTACHED, 0);
  		break;
  	case AUDIO_ACC_DEBOUNCE:
-		tcpm_set_state(port, ACC_UNATTACHED, PD_T_CC_DEBOUNCE);
+		tcpm_set_state(port, ACC_UNATTACHED, port->timings.cc_debounce_time);
  		break;
 
  	/* Hard_Reset states */
@@ -5420,7 +5435,7 @@ static void run_state_machine(struct tcpm_port *port)
  		tcpm_set_state(port, ERROR_RECOVERY, 0);
  		break;
  	case FR_SWAP_SNK_SRC_TRANSITION_TO_OFF:
-		tcpm_set_state(port, ERROR_RECOVERY, PD_T_PS_SOURCE_OFF);
+		tcpm_set_state(port, ERROR_RECOVERY, port->timings.ps_src_off_time);
  		break;
  	case FR_SWAP_SNK_SRC_NEW_SINK_READY:
  		if (port->vbus_source)
@@ -5475,7 +5490,7 @@ static void run_state_machine(struct tcpm_port *port)
  		tcpm_set_cc(port, TYPEC_CC_RD);
  		/* allow CC debounce */
  		tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED,
-			       PD_T_CC_DEBOUNCE);
+			       port->timings.cc_debounce_time);
  		break;
  	case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
  		/*
@@ -5510,7 +5525,7 @@ static void run_state_machine(struct tcpm_port *port)
  						       port->pps_data.active, 0);
  		tcpm_set_charge(port, false);
  		tcpm_set_state(port, hard_reset_state(port),
-			       PD_T_PS_SOURCE_OFF);
+			       port->timings.ps_src_off_time);
  		break;
  	case PR_SWAP_SNK_SRC_SOURCE_ON:
  		tcpm_enable_auto_vbus_discharge(port, true);
@@ -5666,7 +5681,7 @@ static void run_state_machine(struct tcpm_port *port)
  	case PORT_RESET_WAIT_OFF:
  		tcpm_set_state(port,
  			       tcpm_default_state(port),
-			       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
+			       port->vbus_present ? port->timings.ps_src_off_time : 0);
  		break;
 
  	/* AMS intermediate state */
@@ -6157,7 +6172,7 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
  	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);
+				       port->timings.cc_debounce_time);
  		break;
  	case SRC_STARTUP:
  	case SRC_SEND_CAPABILITIES:
@@ -7053,6 +7068,35 @@ static int tcpm_port_register_pd(struct tcpm_port *port)
  	return ret;
  }
 
+static int tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+	int ret;
+	u32 val;
+
+	if (!fwnode)
+		return -EINVAL;
+
+	ret = fwnode_property_read_u32(fwnode, "sink-wait-cap-time-ms", &val);
+	if (!ret)
+		port->timings.sink_wait_cap_time = val;
+	else
+		port->timings.sink_wait_cap_time = PD_T_SINK_WAIT_CAP;
+
+	ret = fwnode_property_read_u32(fwnode, "ps-source-off-time-ms", &val);
+	if (!ret)
+		port->timings.ps_src_off_time = val;
+	else
+		port->timings.ps_src_off_time = PD_T_PS_SOURCE_OFF;
+
+	ret = fwnode_property_read_u32(fwnode, "cc-debounce-time-ms", &val);
+	if (!ret)
+		port->timings.cc_debounce_time = val;
+	else
+		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
+
+	return 0;
+}
+
  static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
  {
  	struct fwnode_handle *capabilities, *child, *caps = NULL;

drivers/usb/typec/tcpm/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 7657

Quoted 6 lines
drivers/usb/typec/tcpm/tcpm.c +3 -0
@@ -7608,9 +7652,14 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
  	init_completion(&port->pps_complete);
  	tcpm_debugfs_init(port);
 
+	err = tcpm_fw_get_timings(port, tcpc->fwnode);
+	if (err < 0)
+		goto out_destroy_wq;

This is somehow wrong. You are using default values in case of failure, so this should not be a reason to fail port registration under any circumstance. That function should just return void.

Quoted 3 lines

I would also just call it after tcpm_fw_get_caps() (or maybe even from tcpm_fw_get_caps()), because tcpm_fw_get_caps() checks fwnode in any case.

Thanks for the review! I will update in the next revision with this change. Will send the next revision once I get review from DT maintainers for DT patch.

Regards,

Amit

Quoted 9 lines

drivers/usb/typec/tcpm/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 7665

Quoted 7 lines
drivers/usb/typec/tcpm/tcpm.c +1 -0
  	err = tcpm_fw_get_caps(port, tcpc->fwnode);
  	if (err < 0)
  		goto out_destroy_wq;
+
  	err = tcpm_fw_get_snk_vdos(port, tcpc->fwnode);
  	if (err < 0)
  		goto out_destroy_wq;

thanks,