[PATCH v2 0/3] Add new time property for battery charger type detection

[PATCH v2 0/3] Add new time property for battery charger type detection

[Amit Sunil Dhamne]

This patchset adds a new time DT property to handle time taken by
battery charger type detection completion.

Note that BC detection is based on
"Battery Charging Specification Revision 1.2".

This patchset depends on the patch series:
 - https://lore.kernel.org/all/20241022-pd-dt-time-props-v1-0-fea96f51b302@google.com/

Changes in v1 -> v2:
  - Fixed the subject line in patch [2]:
    s/maxim,max33359.yaml/maxim,max33359
  - Fixed typo in patch [3] in body:
    s/conroller/controller


Amit Sunil Dhamne (3):
  dt-bindings: connector: Add time property for Sink BC12 detection
    completion
  dt-bindings: usb: maxim,max33359: add usage of sink bc12 time property
  usb: typec: tcpm: Add support for sink-bc12-completion-time-ms DT
    property

 .../bindings/connector/usb-connector.yaml        | 11 +++++++++++
 .../devicetree/bindings/usb/maxim,max33359.yaml  |  1 +
 drivers/usb/typec/tcpm/tcpm.c                    | 16 +++++++++++++++-
 3 files changed, 27 insertions(+), 1 deletion(-)


base-commit: 0fc810ae3ae110f9e2fcccce80fc8c8d62f97907
prerequisite-patch-id: c08696694dabcbb86b458a935a9cbbcbabb75672
prerequisite-patch-id: 634924883df820956acb33941b659e8c9ef85d1e
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 1/3] dt-bindings: connector: Add time property for Sink BC12 detection completion

[Amit Sunil Dhamne]

This commit adds a new time property for Battery charger (BC1.2) type
detection completion process (based on BCv1.2 detection spec) when
typec port connects in a potential sink role. BC1.2 detection is used
by some Type C port controllers implementations (such as
"maxim,max33359") to detect the type of charger port.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/connector/usb-connector.yaml  | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 21a0c58c65cd..67700440e23b 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -283,6 +283,16 @@ properties:
     maximum: 200
     default: 200
 
+  sink-bc12-completion-time-ms:
+    description: Represents the max time in ms that a port in sink role takes
+      to complete Battery Charger (BC1.2) Detection. BC1.2 detection is a
+      hardware mechanism, which in some TCPC implementations, can run in
+      parallel once the Type-C connection state machine reaches the "potential
+      connect as sink" state. In TCPCs where this causes delays to respond to
+      the incoming PD messages, sink-bc12-completion-time-ms is used to delay
+      PD negotiation till BC1.2 detection completes.
+    default: 0
+
 dependencies:
   sink-vdos-v1: [ sink-vdos ]
   sink-vdos: [ sink-vdos-v1 ]
@@ -426,6 +436,7 @@ examples:
             sink-wait-cap-time-ms = <465>;
             ps-source-off-time-ms = <835>;
             cc-debounce-time-ms = <101>;
+            sink-bc12-completion-time-ms = <500>;
         };
     };
 
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 2/3] dt-bindings: usb: maxim,max33359: add usage of sink bc12 time property

[Amit Sunil Dhamne]

Add usage of "sink-bc12-completion-time-ms"  connector property to
max33359 controller for delaying PD negotiation till BC1.2 detection
completes. This overcomes the occasional delays observed while
receiving PD messages where BC1.2 detection runs in parallel.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
index 276bf7554215..20b62228371b 100644
--- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
+++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
@@ -69,6 +69,7 @@ examples:
                                        PDO_FIXED_DATA_SWAP |
                                        PDO_FIXED_DUAL_ROLE)
                                        PDO_FIXED(9000, 2000, 0)>;
+                sink-bc12-completion-time-ms = <500>;
             };
         };
     };
-- 
2.47.0.199.ga7371fff76-goog

Re: [PATCH v2 2/3] dt-bindings: usb: maxim,max33359: add usage of sink bc12 time property

[Rob Herring (Arm)]

On Sat, 02 Nov 2024 20:43:29 -0700, Amit Sunil Dhamne wrote:
> Add usage of "sink-bc12-completion-time-ms"  connector property to
> max33359 controller for delaying PD negotiation till BC1.2 detection
> completes. This overcomes the occasional delays observed while
> receiving PD messages where BC1.2 detection runs in parallel.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb: maxtcpc@25: connector: Unevaluated properties are not allowed ('sink-bc12-completion-time-ms' was unexpected)
	from schema $id: http://devicetree.org/schemas/usb/maxim,max33359.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb: connector: Unevaluated properties are not allowed ('sink-bc12-completion-time-ms' was unexpected)
	from schema $id: http://devicetree.org/schemas/connector/usb-connector.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241103034402.2460252-3-amitsd@google.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 2/3] dt-bindings: usb: maxim,max33359: add usage of sink bc12 time property

[Rob Herring]

On Sun, Nov 03, 2024 at 12:22:50AM -0500, Rob Herring (Arm) wrote:
> 
> On Sat, 02 Nov 2024 20:43:29 -0700, Amit Sunil Dhamne wrote:
> > Add usage of "sink-bc12-completion-time-ms"  connector property to
> > max33359 controller for delaying PD negotiation till BC1.2 detection
> > completes. This overcomes the occasional delays observed while
> > receiving PD messages where BC1.2 detection runs in parallel.
> > 
> > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> > Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb: maxtcpc@25: connector: Unevaluated properties are not allowed ('sink-bc12-completion-time-ms' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/usb/maxim,max33359.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb: connector: Unevaluated properties are not allowed ('sink-bc12-completion-time-ms' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/connector/usb-connector.yaml#

This can be ignored as patch 1 couldn't be applied.

Rob

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

[Amit Sunil Dhamne]

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

[Amit Sunil Dhamne]

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

[Heikki Krogerus]

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