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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

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