[PATCH v2 3/3] usb: typec: tcpm: Add support for sink-bc12-completion-time-ms DT property

Amit Sunil Dhamne posted 3 patches 3 weeks ago
[PATCH v2 3/3] usb: typec: tcpm: Add support for sink-bc12-completion-time-ms DT property
Posted by Amit Sunil Dhamne 3 weeks ago
Add support for parsing DT time property "sink-bc12-completion-time-ms".
This timer is used to relax the PD state machine during Sink attach to
allow completion of Battery Charging (BC1.2) charger type detection in
TCPC before PD negotiations. BC1.2 detection is a hardware mechanism to
detect charger port type that is run by some controllers (such as
"maxim,max33359") in parallel to Type-C connection state machines.
This is to ensure that BC1.2 completes before PD is enabled as running
BC1.2 in parallel with PD negotiation results in delays violating timer
constraints in PD spec.

This is an optional timer and will not add any delay unless explicitly
set.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index b3d5d1d48937..8b325b93b5a9 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -319,6 +319,7 @@ struct pd_timings {
 	u32 sink_wait_cap_time;
 	u32 ps_src_off_time;
 	u32 cc_debounce_time;
+	u32 snk_bc12_cmpletion_time;
 };
 
 struct tcpm_port {
@@ -4978,7 +4979,16 @@ static void run_state_machine(struct tcpm_port *port)
 		if (ret < 0)
 			tcpm_set_state(port, SNK_UNATTACHED, 0);
 		else
-			tcpm_set_state(port, SNK_STARTUP, 0);
+			/*
+			 * For Type C port controllers that use Battery Charging
+			 * Detection (based on BCv1.2 spec) to detect USB
+			 * charger type, add a delay of "snk_bc12_cmpletion_time"
+			 * before transitioning to SNK_STARTUP to allow BC1.2
+			 * detection to complete before PD is eventually enabled
+			 * in later states.
+			 */
+			tcpm_set_state(port, SNK_STARTUP,
+				       port->timings.snk_bc12_cmpletion_time);
 		break;
 	case SNK_STARTUP:
 		opmode =  tcpm_get_pwr_opmode(port->polarity ?
@@ -7090,6 +7100,10 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw
 		port->timings.cc_debounce_time = val;
 	else
 		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
+
+	ret = fwnode_property_read_u32(fwnode, "sink-bc12-completion-time-ms", &val);
+	if (!ret)
+		port->timings.snk_bc12_cmpletion_time = val;
 }
 
 static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
-- 
2.47.0.199.ga7371fff76-goog
Re: [PATCH v2 3/3] usb: typec: tcpm: Add support for sink-bc12-completion-time-ms DT property
Posted by Amit Sunil Dhamne 1 week, 2 days ago
Hi Heikki,

On 11/2/24 8:43 PM, Amit Sunil Dhamne wrote:
> Add support for parsing DT time property "sink-bc12-completion-time-ms".
> This timer is used to relax the PD state machine during Sink attach to
> allow completion of Battery Charging (BC1.2) charger type detection in
> TCPC before PD negotiations. BC1.2 detection is a hardware mechanism to
> detect charger port type that is run by some controllers (such as
> "maxim,max33359") in parallel to Type-C connection state machines.
> This is to ensure that BC1.2 completes before PD is enabled as running
> BC1.2 in parallel with PD negotiation results in delays violating timer
> constraints in PD spec.
>
> This is an optional timer and will not add any delay unless explicitly
> set.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>   drivers/usb/typec/tcpm/tcpm.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index b3d5d1d48937..8b325b93b5a9 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -319,6 +319,7 @@ struct pd_timings {
>   	u32 sink_wait_cap_time;
>   	u32 ps_src_off_time;
>   	u32 cc_debounce_time;
> +	u32 snk_bc12_cmpletion_time;
>   };
>   
>   struct tcpm_port {
> @@ -4978,7 +4979,16 @@ static void run_state_machine(struct tcpm_port *port)
>   		if (ret < 0)
>   			tcpm_set_state(port, SNK_UNATTACHED, 0);
>   		else
> -			tcpm_set_state(port, SNK_STARTUP, 0);
> +			/*
> +			 * For Type C port controllers that use Battery Charging
> +			 * Detection (based on BCv1.2 spec) to detect USB
> +			 * charger type, add a delay of "snk_bc12_cmpletion_time"
> +			 * before transitioning to SNK_STARTUP to allow BC1.2
> +			 * detection to complete before PD is eventually enabled
> +			 * in later states.
> +			 */
> +			tcpm_set_state(port, SNK_STARTUP,
> +				       port->timings.snk_bc12_cmpletion_time);
>   		break;
>   	case SNK_STARTUP:
>   		opmode =  tcpm_get_pwr_opmode(port->polarity ?
> @@ -7090,6 +7100,10 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw
>   		port->timings.cc_debounce_time = val;
>   	else
>   		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
> +
> +	ret = fwnode_property_read_u32(fwnode, "sink-bc12-completion-time-ms", &val);
> +	if (!ret)
> +		port->timings.snk_bc12_cmpletion_time = val;
>   }
>   
>   static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)


I wanted to gently follow up with you on this patchset if this looks 
okay to you?


Thanks,

Amit
Re: [PATCH v2 3/3] usb: typec: tcpm: Add support for sink-bc12-completion-time-ms DT property
Posted by Heikki Krogerus 1 week, 2 days ago
On Thu, Nov 14, 2024 at 11:59:41AM -0800, Amit Sunil Dhamne wrote:
> Hi Heikki,
> 
> On 11/2/24 8:43 PM, Amit Sunil Dhamne wrote:
> > Add support for parsing DT time property "sink-bc12-completion-time-ms".
> > This timer is used to relax the PD state machine during Sink attach to
> > allow completion of Battery Charging (BC1.2) charger type detection in
> > TCPC before PD negotiations. BC1.2 detection is a hardware mechanism to
> > detect charger port type that is run by some controllers (such as
> > "maxim,max33359") in parallel to Type-C connection state machines.
> > This is to ensure that BC1.2 completes before PD is enabled as running
> > BC1.2 in parallel with PD negotiation results in delays violating timer
> > constraints in PD spec.
> > 
> > This is an optional timer and will not add any delay unless explicitly
> > set.
> > 
> > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> > Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> >   drivers/usb/typec/tcpm/tcpm.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index b3d5d1d48937..8b325b93b5a9 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -319,6 +319,7 @@ struct pd_timings {
> >   	u32 sink_wait_cap_time;
> >   	u32 ps_src_off_time;
> >   	u32 cc_debounce_time;
> > +	u32 snk_bc12_cmpletion_time;
> >   };
> >   struct tcpm_port {
> > @@ -4978,7 +4979,16 @@ static void run_state_machine(struct tcpm_port *port)
> >   		if (ret < 0)
> >   			tcpm_set_state(port, SNK_UNATTACHED, 0);
> >   		else
> > -			tcpm_set_state(port, SNK_STARTUP, 0);
> > +			/*
> > +			 * For Type C port controllers that use Battery Charging
> > +			 * Detection (based on BCv1.2 spec) to detect USB
> > +			 * charger type, add a delay of "snk_bc12_cmpletion_time"
> > +			 * before transitioning to SNK_STARTUP to allow BC1.2
> > +			 * detection to complete before PD is eventually enabled
> > +			 * in later states.
> > +			 */
> > +			tcpm_set_state(port, SNK_STARTUP,
> > +				       port->timings.snk_bc12_cmpletion_time);
> >   		break;
> >   	case SNK_STARTUP:
> >   		opmode =  tcpm_get_pwr_opmode(port->polarity ?
> > @@ -7090,6 +7100,10 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw
> >   		port->timings.cc_debounce_time = val;
> >   	else
> >   		port->timings.cc_debounce_time = PD_T_CC_DEBOUNCE;
> > +
> > +	ret = fwnode_property_read_u32(fwnode, "sink-bc12-completion-time-ms", &val);
> > +	if (!ret)
> > +		port->timings.snk_bc12_cmpletion_time = val;
> >   }
> >   static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
> 
> 
> I wanted to gently follow up with you on this patchset if this looks okay to
> you?

Sorry, this is okay by me. I thought that there's still some problem
with the device property itself, but I must have misunderstood.

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

-- 
heikki