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

[PATCH v1 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/

Amit Sunil Dhamne (3):
  dt-bindings: connector: Add time property for Sink BC12 detection
    completion
  dt-bindings: usb: maxim,max33359.yaml: 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 v1 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>
---
 .../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 v1 2/3] dt-bindings: usb: maxim,max33359.yaml: 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>
---
 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

[PATCH v1 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 conrollers (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 v1 2/3] dt-bindings: usb: maxim,max33359.yaml: add usage of sink bc12 time property

[Rob Herring (Arm)]

On Thu, 31 Oct 2024 16:59:53 -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>
> ---
>  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/20241031235957.1261244-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 v1 2/3] dt-bindings: usb: maxim,max33359.yaml: add usage of sink bc12 time property

[Krzysztof Kozlowski]

On Thu, Oct 31, 2024 at 04:59:53PM -0700, Amit Sunil Dhamne wrote:
> Add usage of "sink-bc12-completion-time-ms"  connector property to

There is no such property. You cannot add it. You did not even
test this patch before sending :/


> 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>

Your Reviewed-by tag is pretty wrong. I don't believe any review
happened here. If it did, it was pretty useless.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 01/11/2024 08:02, Krzysztof Kozlowski wrote:
> On Thu, Oct 31, 2024 at 04:59:53PM -0700, Amit Sunil Dhamne wrote:
>> Add usage of "sink-bc12-completion-time-ms"  connector property to
> 
> There is no such property. You cannot add it. You did not even
> test this patch before sending :/
> 

Sorry, now I see that's the patch 2, not 1. Somehow I thought you
started with it and I got confused.


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Thu, Oct 31, 2024 at 04:59:52PM -0700, Amit Sunil Dhamne wrote:
> 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>
> ---
>  .../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

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Dmitry Baryshkov]

On Fri, Nov 01, 2024 at 08:09:52AM +0100, Krzysztof Kozlowski wrote:
> On 01/11/2024 08:02, Krzysztof Kozlowski wrote:
> > On Thu, Oct 31, 2024 at 04:59:53PM -0700, Amit Sunil Dhamne wrote:
> >> Add usage of "sink-bc12-completion-time-ms"  connector property to
> > 
> > There is no such property. You cannot add it. You did not even
> > test this patch before sending :/
> > 
> 
> Sorry, now I see that's the patch 2, not 1. Somehow I thought you
> started with it and I got confused.

The question is why patch 2 generated the error even if it had patch 1
in front of it?

-- 
With best wishes
Dmitry

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

[Krzysztof Kozlowski]

On 01/11/2024 08:59, Dmitry Baryshkov wrote:
> On Fri, Nov 01, 2024 at 08:09:52AM +0100, Krzysztof Kozlowski wrote:
>> On 01/11/2024 08:02, Krzysztof Kozlowski wrote:
>>> On Thu, Oct 31, 2024 at 04:59:53PM -0700, Amit Sunil Dhamne wrote:
>>>> Add usage of "sink-bc12-completion-time-ms"  connector property to
>>>
>>> There is no such property. You cannot add it. You did not even
>>> test this patch before sending :/
>>>
>>
>> Sorry, now I see that's the patch 2, not 1. Somehow I thought you
>> started with it and I got confused.
> 
> The question is why patch 2 generated the error even if it had patch 1
> in front of it?

I suspect patch #1 did not apply, because it depends on USB next tree.

Best regards,
Krzysztof

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

[Amit Sunil Dhamne]

Hi Krzysztof,

On 11/1/24 1:34 AM, Krzysztof Kozlowski wrote:
> On 01/11/2024 08:59, Dmitry Baryshkov wrote:
>> On Fri, Nov 01, 2024 at 08:09:52AM +0100, Krzysztof Kozlowski wrote:
>>> On 01/11/2024 08:02, Krzysztof Kozlowski wrote:
>>>> On Thu, Oct 31, 2024 at 04:59:53PM -0700, Amit Sunil Dhamne wrote:
>>>>> Add usage of "sink-bc12-completion-time-ms"  connector property to
>>>> There is no such property. You cannot add it. You did not even
>>>> test this patch before sending :/
>>>>
>>> Sorry, now I see that's the patch 2, not 1. Somehow I thought you
>>> started with it and I got confused.
>> The question is why patch 2 generated the error even if it had patch 1
>> in front of it?
> I suspect patch #1 did not apply, because it depends on USB next tree.

Thanks for the review! That's right, as indicated in the cover, this
patch series is dependent on a patch series
(https://lore.kernel.org/all/20241022-pd-dt-time-props-v1-0-fea96f51b302@google.com/)
that's in the pipeline.


Regards,

Amit

>
> Best regards,
> Krzysztof
>