* [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-12 23:42 [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
@ 2025-03-12 23:42 ` Amit Sunil Dhamne via B4 Relay
2025-03-13 8:48 ` Krzysztof Kozlowski
2025-03-12 23:42 ` [PATCH 2/5] power: supply: core: add function to get supplies from fwnode Amit Sunil Dhamne via B4 Relay
` (4 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-03-12 23:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso, Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add a new "fixed-batteries" DT property to connector class. This
property is populated with nodes associated with battery type power
supplies powering the USB PD connector. This is needed by the Type-C
Port Manager (TCPM) to query psy properties which are used to feed
Battery_Status & Battery_Capacity AMS.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
Documentation/devicetree/bindings/connector/usb-connector.yaml | 8 ++++++++
Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
2 files changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..5e15bc060f5a2cfce842f83de738f1e8bae3ce2d 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -300,6 +300,14 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint8-array
maxItems: 4
+ fixed-batteries:
+ description: Contains references to nodes associated with battery type power
+ supplies powering the USB PD device. These batteries are fixed type and
+ not hot swappable.
+ minItems: 1
+ maxItems: 4
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+
dependencies:
sink-vdos-v1: [ sink-vdos ]
sink-vdos: [ sink-vdos-v1 ]
diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
index 3de4dc40b79192b60443421b557bd2fb18683bf7..66c99f0131f074f1c08e31d7481f555647e3b2f8 100644
--- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
+++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
@@ -75,6 +75,7 @@ examples:
PDO_FIXED(9000, 2000, 0)>;
sink-bc12-completion-time-ms = <500>;
pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
+ fixed-batteries = <&batt1 &batt2>;
};
};
};
--
2.49.0.rc0.332.g42c0ae87b1-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-12 23:42 ` [PATCH 1/5] dt-bindings: connector: add fixed-batteries property Amit Sunil Dhamne via B4 Relay
@ 2025-03-13 8:48 ` Krzysztof Kozlowski
2025-03-15 0:56 ` Amit Sunil Dhamne
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-13 8:48 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On Wed, Mar 12, 2025 at 04:42:01PM -0700, Amit Sunil Dhamne wrote:
> Add a new "fixed-batteries" DT property to connector class. This
> property is populated with nodes associated with battery type power
> supplies powering the USB PD connector. This is needed by the Type-C
> Port Manager (TCPM) to query psy properties which are used to feed
What is "psy" in terms of bindings?
> Battery_Status & Battery_Capacity AMS.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
> Documentation/devicetree/bindings/connector/usb-connector.yaml | 8 ++++++++
> Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..5e15bc060f5a2cfce842f83de738f1e8bae3ce2d 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -300,6 +300,14 @@ properties:
> $ref: /schemas/types.yaml#/definitions/uint8-array
> maxItems: 4
>
> + fixed-batteries:
> + description: Contains references to nodes associated with battery type power
> + supplies powering the USB PD device. These batteries are fixed type and
What is a "battery type power supply"? If you just link here batteries,
then we have type for it - monitored-battery - but I doubt connector has
direct connection to the battery.
If you mean chargers, the OF graph is already there for this and no need
for this patch.
> + not hot swappable.
> + minItems: 1
> + maxItems: 4
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> +
> dependencies:
> sink-vdos-v1: [ sink-vdos ]
> sink-vdos: [ sink-vdos-v1 ]
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> index 3de4dc40b79192b60443421b557bd2fb18683bf7..66c99f0131f074f1c08e31d7481f555647e3b2f8 100644
> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> @@ -75,6 +75,7 @@ examples:
> PDO_FIXED(9000, 2000, 0)>;
> sink-bc12-completion-time-ms = <500>;
> pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
> + fixed-batteries = <&batt1 &batt2>;
Two phandles, so two <>.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-13 8:48 ` Krzysztof Kozlowski
@ 2025-03-15 0:56 ` Amit Sunil Dhamne
2025-03-16 16:49 ` Krzysztof Kozlowski
2025-03-16 16:55 ` Krzysztof Kozlowski
0 siblings, 2 replies; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-03-15 0:56 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
Hi Krzysztof,
On 3/13/25 1:48 AM, Krzysztof Kozlowski wrote:
> On Wed, Mar 12, 2025 at 04:42:01PM -0700, Amit Sunil Dhamne wrote:
>> Add a new "fixed-batteries" DT property to connector class. This
>> property is populated with nodes associated with battery type power
>> supplies powering the USB PD connector. This is needed by the Type-C
>> Port Manager (TCPM) to query psy properties which are used to feed
> What is "psy" in terms of bindings?
In terms of bindings this should be a phandle to a device that
owns/manages the battery (whose driver will eventually call
devm_power_supply_register to register the battery). This could be a
fuel-guage ("sprd,sc2731-fgu", say), charger ("ti,bq24190") or a
platform device ("cw2015") containing "monitored-battery" property to
manage the simple battery.
>> Battery_Status & Battery_Capacity AMS.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 8 ++++++++
>> Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..5e15bc060f5a2cfce842f83de738f1e8bae3ce2d 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -300,6 +300,14 @@ properties:
>> $ref: /schemas/types.yaml#/definitions/uint8-array
>> maxItems: 4
>>
>> + fixed-batteries:
>> + description: Contains references to nodes associated with battery type power
>> + supplies powering the USB PD device. These batteries are fixed type and
> What is a "battery type power supply"? If you just link here batteries,
> then we have type for it - monitored-battery - but I doubt connector has
> direct connection to the battery.
Regarding "nodes associated with battery type power supplies", I meant
something like a fuel guage or a charger OR platform device with
"monitored-battery" that will manage the battery lifecycle. If I use
monitored-battery for this, I will be restricted to only querying 1
simple battery. Also, I don't mean PD connector device to be a fuel
guage or charger that manages a specific battery. It should just be able
to query any FG/Chg for the battery status to relay that info to the
connector's port partner.
The intent of the patchset & this change is for the USB Type C protocol
manager module (that consumes these bindings) to be able to get info
(such as State of charge, design capacity, etc) from drivers that manage
the battery/batteries in the system. In order for such info to propagate
I need to hook up the references of these battery manager devices (fuel
guages, etc.) to connector.
I have addressed the connector <-> battery question in the cover letter.
> If you mean chargers, the OF graph is already there for this and no need
> for this patch.
No I don't mean just chargers in this case. Also, I didn't follow you on
the OF graph. Please can you explain further?
>
>> + not hot swappable.
>> + minItems: 1
>> + maxItems: 4
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> +
>> dependencies:
>> sink-vdos-v1: [ sink-vdos ]
>> sink-vdos: [ sink-vdos-v1 ]
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> index 3de4dc40b79192b60443421b557bd2fb18683bf7..66c99f0131f074f1c08e31d7481f555647e3b2f8 100644
>> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> @@ -75,6 +75,7 @@ examples:
>> PDO_FIXED(9000, 2000, 0)>;
>> sink-bc12-completion-time-ms = <500>;
>> pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
>> + fixed-batteries = <&batt1 &batt2>;
> Two phandles, so two <>.
Ack. Will fix it in the next revision.
Thanks,
Amit
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-15 0:56 ` Amit Sunil Dhamne
@ 2025-03-16 16:49 ` Krzysztof Kozlowski
2025-03-20 19:49 ` Amit Sunil Dhamne
2025-03-16 16:55 ` Krzysztof Kozlowski
1 sibling, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-16 16:49 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 15/03/2025 01:56, Amit Sunil Dhamne wrote:
> Hi Krzysztof,
>
> On 3/13/25 1:48 AM, Krzysztof Kozlowski wrote:
>> On Wed, Mar 12, 2025 at 04:42:01PM -0700, Amit Sunil Dhamne wrote:
>>> Add a new "fixed-batteries" DT property to connector class. This
>>> property is populated with nodes associated with battery type power
>>> supplies powering the USB PD connector. This is needed by the Type-C
>>> Port Manager (TCPM) to query psy properties which are used to feed
>> What is "psy" in terms of bindings?
> In terms of bindings this should be a phandle to a device that
> owns/manages the battery (whose driver will eventually call
> devm_power_supply_register to register the battery). This could be a
So a charger? Please rephrain from putting Linux names into the bindings
description.
> fuel-guage ("sprd,sc2731-fgu", say), charger ("ti,bq24190") or a
> platform device ("cw2015") containing "monitored-battery" property to
> manage the simple battery.
>>> Battery_Status & Battery_Capacity AMS.
>>>
>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>> ---
>>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 8 ++++++++
>>> Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
>>> 2 files changed, 9 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..5e15bc060f5a2cfce842f83de738f1e8bae3ce2d 100644
>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> @@ -300,6 +300,14 @@ properties:
>>> $ref: /schemas/types.yaml#/definitions/uint8-array
>>> maxItems: 4
>>>
>>> + fixed-batteries:
>>> + description: Contains references to nodes associated with battery type power
>>> + supplies powering the USB PD device. These batteries are fixed type and
>> What is a "battery type power supply"? If you just link here batteries,
>> then we have type for it - monitored-battery - but I doubt connector has
>> direct connection to the battery.
> Regarding "nodes associated with battery type power supplies", I meant
> something like a fuel guage or a charger OR platform device with
> "monitored-battery" that will manage the battery lifecycle. If I use
> monitored-battery for this, I will be restricted to only querying 1
> simple battery. Also, I don't mean PD connector device to be a fuel
> guage or charger that manages a specific battery. It should just be able
> to query any FG/Chg for the battery status to relay that info to the
> connector's port partner.
>
> The intent of the patchset & this change is for the USB Type C protocol
> manager module (that consumes these bindings) to be able to get info
The intent should be rather to accurately describe hardware and maybe
that's the problem - you focus how to bend it for your drivers.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-16 16:49 ` Krzysztof Kozlowski
@ 2025-03-20 19:49 ` Amit Sunil Dhamne
0 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-03-20 19:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
Hi Krzysztof,
On 3/16/25 9:49 AM, Krzysztof Kozlowski wrote:
> On 15/03/2025 01:56, Amit Sunil Dhamne wrote:
>> Hi Krzysztof,
>>
>> On 3/13/25 1:48 AM, Krzysztof Kozlowski wrote:
>>> On Wed, Mar 12, 2025 at 04:42:01PM -0700, Amit Sunil Dhamne wrote:
>>>> Add a new "fixed-batteries" DT property to connector class. This
>>>> property is populated with nodes associated with battery type power
>>>> supplies powering the USB PD connector. This is needed by the Type-C
>>>> Port Manager (TCPM) to query psy properties which are used to feed
>>> What is "psy" in terms of bindings?
>> In terms of bindings this should be a phandle to a device that
>> owns/manages the battery (whose driver will eventually call
>> devm_power_supply_register to register the battery). This could be a
> So a charger? Please rephrain from putting Linux names into the bindings
> description.
Noted, thanks!
>> fuel-guage ("sprd,sc2731-fgu", say), charger ("ti,bq24190") or a
>> platform device ("cw2015") containing "monitored-battery" property to
>> manage the simple battery.
>
>>>> Battery_Status & Battery_Capacity AMS.
>>>>
>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>> ---
>>>> Documentation/devicetree/bindings/connector/usb-connector.yaml | 8 ++++++++
>>>> Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 1 +
>>>> 2 files changed, 9 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..5e15bc060f5a2cfce842f83de738f1e8bae3ce2d 100644
>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> @@ -300,6 +300,14 @@ properties:
>>>> $ref: /schemas/types.yaml#/definitions/uint8-array
>>>> maxItems: 4
>>>>
>>>> + fixed-batteries:
>>>> + description: Contains references to nodes associated with battery type power
>>>> + supplies powering the USB PD device. These batteries are fixed type and
>>> What is a "battery type power supply"? If you just link here batteries,
>>> then we have type for it - monitored-battery - but I doubt connector has
>>> direct connection to the battery.
>> Regarding "nodes associated with battery type power supplies", I meant
>> something like a fuel guage or a charger OR platform device with
>> "monitored-battery" that will manage the battery lifecycle. If I use
>> monitored-battery for this, I will be restricted to only querying 1
>> simple battery. Also, I don't mean PD connector device to be a fuel
>> guage or charger that manages a specific battery. It should just be able
>> to query any FG/Chg for the battery status to relay that info to the
>> connector's port partner.
>>
>> The intent of the patchset & this change is for the USB Type C protocol
>> manager module (that consumes these bindings) to be able to get info
> The intent should be rather to accurately describe hardware and maybe
> that's the problem - you focus how to bend it for your drivers.
Acknowledged!
Thanks,
Amit
>
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-15 0:56 ` Amit Sunil Dhamne
2025-03-16 16:49 ` Krzysztof Kozlowski
@ 2025-03-16 16:55 ` Krzysztof Kozlowski
2025-03-20 19:45 ` Amit Sunil Dhamne
1 sibling, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-16 16:55 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 15/03/2025 01:56, Amit Sunil Dhamne wrote:
> The intent of the patchset & this change is for the USB Type C protocol
> manager module (that consumes these bindings) to be able to get info
> (such as State of charge, design capacity, etc) from drivers that manage
> the battery/batteries in the system. In order for such info to propagate
> I need to hook up the references of these battery manager devices (fuel
> guages, etc.) to connector.
>
> I have addressed the connector <-> battery question in the cover letter.
>
>
>> If you mean chargers, the OF graph is already there for this and no need
>> for this patch.
>
> No I don't mean just chargers in this case. Also, I didn't follow you on
> the OF graph. Please can you explain further?
>
You are duplicating existing bindings and existing practice of
describing the actual connections via OF graph. And the binding already
has the OF graph. What to explain more? Please open the binding and look
at the ports. Maybe they are incomplete? Look how other USB and USB
Type-C connections are represented.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] dt-bindings: connector: add fixed-batteries property
2025-03-16 16:55 ` Krzysztof Kozlowski
@ 2025-03-20 19:45 ` Amit Sunil Dhamne
0 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-03-20 19:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 3/16/25 9:55 AM, Krzysztof Kozlowski wrote:
> On 15/03/2025 01:56, Amit Sunil Dhamne wrote:
>> The intent of the patchset & this change is for the USB Type C protocol
>> manager module (that consumes these bindings) to be able to get info
>> (such as State of charge, design capacity, etc) from drivers that manage
>> the battery/batteries in the system. In order for such info to propagate
>> I need to hook up the references of these battery manager devices (fuel
>> guages, etc.) to connector.
>>
>> I have addressed the connector <-> battery question in the cover letter.
>>
>>
>>> If you mean chargers, the OF graph is already there for this and no need
>>> for this patch.
>> No I don't mean just chargers in this case. Also, I didn't follow you on
>> the OF graph. Please can you explain further?
>>
> You are duplicating existing bindings and existing practice of
> describing the actual connections via OF graph. And the binding already
> has the OF graph. What to explain more? Please open the binding and look
> at the ports. Maybe they are incomplete? Look how other USB and USB
> Type-C connections are represented.
I will try to use existing bindings. I will update my patchset and drop
this property.
Thanks,
Amit
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/5] power: supply: core: add function to get supplies from fwnode
2025-03-12 23:42 [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
2025-03-12 23:42 ` [PATCH 1/5] dt-bindings: connector: add fixed-batteries property Amit Sunil Dhamne via B4 Relay
@ 2025-03-12 23:42 ` Amit Sunil Dhamne via B4 Relay
2025-03-19 13:54 ` Heikki Krogerus
2025-03-12 23:42 ` [PATCH 3/5] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
` (3 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-03-12 23:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso, Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add a new helper function power_supply_get_by_fwnode_reference_array()
to retrieve a list of power_supplies associated with the fwnode's
property. The property can contain multiple nodes where each node is
associated with a power_supply. The list of power_supply objects will be
stored in an array supplied by the caller and the return value will
indicate the size of the resulting array.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
drivers/power/supply/power_supply_core.c | 60 ++++++++++++++++++++++++++++++++
include/linux/power_supply.h | 5 +++
2 files changed, 65 insertions(+)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 76c340b38015af0a67a0d91305e6242a8646bf53..df1a52f85125748c4fdcb10687aa7ed2f626ded1 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -593,6 +593,66 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
#endif /* CONFIG_OF */
+static int power_supply_match_fwnode(struct device *dev, const void *data)
+{
+ return dev && dev->parent && dev->parent->fwnode == data;
+}
+
+/**
+ * power_supply_get_by_fwnode_reference_array() - Returns an array of power
+ * supply objects associated with each fwnode reference present in the property
+ * @fwnode: Pointer to fwnode to lookup property
+ * @property: Name of property holding references
+ * @psy: Resulting array of power_supply pointers. To be provided by the caller.
+ * @size: size of power_supply pointer array.
+ *
+ * If power supply was found, it increases reference count for the
+ * internal power supply's device. The user should power_supply_put()
+ * after usage.
+ *
+ * Return: On success returns the number of power supply objects filled
+ * in the @psy array.
+ * -EOVERFLOW when size of @psy array is not suffice.
+ * -EINVAL when @psy is NULL or @size is 0.
+ * -ENODATA when fwnode does not contain the given property
+ */
+int power_supply_get_by_fwnode_reference_array(struct fwnode_handle *fwnode,
+ const char *property,
+ struct power_supply **psy,
+ ssize_t size)
+{
+ int ret, index, count = 0;
+ struct fwnode_reference_args args;
+ struct device *dev;
+
+ if (!psy || !size)
+ return -EINVAL;
+
+ for (index = 0; index < size &&
+ !(ret = fwnode_property_get_reference_args(fwnode, property, NULL,
+ 0, index, &args));
+ ++index) {
+ dev = class_find_device(&power_supply_class, NULL, args.fwnode,
+ power_supply_match_fwnode);
+ fwnode_handle_put(args.fwnode);
+ if (!dev)
+ continue;
+
+ if (count > size)
+ return -EOVERFLOW;
+
+ psy[count] = dev_get_drvdata(dev);
+ atomic_inc(&psy[count]->use_cnt);
+ ++count;
+ }
+
+ if (ret != -ENOENT)
+ return ret;
+
+ return index ? count : -ENODATA;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_by_fwnode_reference_array);
+
int power_supply_get_battery_info(struct power_supply *psy,
struct power_supply_battery_info **info_out)
{
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 6ed53b292162469d7b357734d5589bff18a201d0..3f062607e5cd7c7f04384e34128ae0953e25d981 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -820,6 +820,11 @@ devm_power_supply_get_by_phandle(struct device *dev, const char *property)
{ return NULL; }
#endif /* CONFIG_OF */
+extern int
+power_supply_get_by_fwnode_reference_array(struct fwnode_handle *fwnode,
+ const char *property,
+ struct power_supply **psy,
+ ssize_t size);
extern const enum power_supply_property power_supply_battery_info_properties[];
extern const size_t power_supply_battery_info_properties_size;
extern int power_supply_get_battery_info(struct power_supply *psy,
--
2.49.0.rc0.332.g42c0ae87b1-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] power: supply: core: add function to get supplies from fwnode
2025-03-12 23:42 ` [PATCH 2/5] power: supply: core: add function to get supplies from fwnode Amit Sunil Dhamne via B4 Relay
@ 2025-03-19 13:54 ` Heikki Krogerus
2025-04-08 19:54 ` Amit Sunil Dhamne
0 siblings, 1 reply; 23+ messages in thread
From: Heikki Krogerus @ 2025-03-19 13:54 UTC (permalink / raw)
To: amitsd
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Rafael J. Wysocki, Len Brown, Pavel Machek, devicetree,
linux-kernel, linux-usb, linux-pm, RD Babiera, Kyle Tso
On Wed, Mar 12, 2025 at 04:42:02PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add a new helper function power_supply_get_by_fwnode_reference_array()
> to retrieve a list of power_supplies associated with the fwnode's
> property. The property can contain multiple nodes where each node is
> associated with a power_supply. The list of power_supply objects will be
> stored in an array supplied by the caller and the return value will
> indicate the size of the resulting array.
I don't think this API is necessary. If I've understood what you are
after here, the batteries should simply have the Type-C psy(s) listed
in the supplied_to and/or supplied_from.
So you just need to make sure your battery nodes have the USB Type-C
node(s) listed in the "power-supplies" property in your DT, no?
thanks,
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
> drivers/power/supply/power_supply_core.c | 60 ++++++++++++++++++++++++++++++++
> include/linux/power_supply.h | 5 +++
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 76c340b38015af0a67a0d91305e6242a8646bf53..df1a52f85125748c4fdcb10687aa7ed2f626ded1 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -593,6 +593,66 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
> EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
> #endif /* CONFIG_OF */
>
> +static int power_supply_match_fwnode(struct device *dev, const void *data)
> +{
> + return dev && dev->parent && dev->parent->fwnode == data;
> +}
> +
> +/**
> + * power_supply_get_by_fwnode_reference_array() - Returns an array of power
> + * supply objects associated with each fwnode reference present in the property
> + * @fwnode: Pointer to fwnode to lookup property
> + * @property: Name of property holding references
> + * @psy: Resulting array of power_supply pointers. To be provided by the caller.
> + * @size: size of power_supply pointer array.
> + *
> + * If power supply was found, it increases reference count for the
> + * internal power supply's device. The user should power_supply_put()
> + * after usage.
> + *
> + * Return: On success returns the number of power supply objects filled
> + * in the @psy array.
> + * -EOVERFLOW when size of @psy array is not suffice.
> + * -EINVAL when @psy is NULL or @size is 0.
> + * -ENODATA when fwnode does not contain the given property
> + */
> +int power_supply_get_by_fwnode_reference_array(struct fwnode_handle *fwnode,
> + const char *property,
> + struct power_supply **psy,
> + ssize_t size)
> +{
> + int ret, index, count = 0;
> + struct fwnode_reference_args args;
> + struct device *dev;
> +
> + if (!psy || !size)
> + return -EINVAL;
> +
> + for (index = 0; index < size &&
> + !(ret = fwnode_property_get_reference_args(fwnode, property, NULL,
> + 0, index, &args));
> + ++index) {
> + dev = class_find_device(&power_supply_class, NULL, args.fwnode,
> + power_supply_match_fwnode);
> + fwnode_handle_put(args.fwnode);
> + if (!dev)
> + continue;
> +
> + if (count > size)
> + return -EOVERFLOW;
> +
> + psy[count] = dev_get_drvdata(dev);
> + atomic_inc(&psy[count]->use_cnt);
> + ++count;
> + }
> +
> + if (ret != -ENOENT)
> + return ret;
> +
> + return index ? count : -ENODATA;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_get_by_fwnode_reference_array);
> +
> int power_supply_get_battery_info(struct power_supply *psy,
> struct power_supply_battery_info **info_out)
> {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 6ed53b292162469d7b357734d5589bff18a201d0..3f062607e5cd7c7f04384e34128ae0953e25d981 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -820,6 +820,11 @@ devm_power_supply_get_by_phandle(struct device *dev, const char *property)
> { return NULL; }
> #endif /* CONFIG_OF */
>
> +extern int
> +power_supply_get_by_fwnode_reference_array(struct fwnode_handle *fwnode,
> + const char *property,
> + struct power_supply **psy,
> + ssize_t size);
> extern const enum power_supply_property power_supply_battery_info_properties[];
> extern const size_t power_supply_battery_info_properties_size;
> extern int power_supply_get_battery_info(struct power_supply *psy,
>
> --
> 2.49.0.rc0.332.g42c0ae87b1-goog
>
--
heikki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/5] power: supply: core: add function to get supplies from fwnode
2025-03-19 13:54 ` Heikki Krogerus
@ 2025-04-08 19:54 ` Amit Sunil Dhamne
0 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-04-08 19:54 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Rafael J. Wysocki, Len Brown, Pavel Machek, devicetree,
linux-kernel, linux-usb, linux-pm, RD Babiera, Kyle Tso
On 3/19/25 6:54 AM, Heikki Krogerus wrote:
> On Wed, Mar 12, 2025 at 04:42:02PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Add a new helper function power_supply_get_by_fwnode_reference_array()
>> to retrieve a list of power_supplies associated with the fwnode's
>> property. The property can contain multiple nodes where each node is
>> associated with a power_supply. The list of power_supply objects will be
>> stored in an array supplied by the caller and the return value will
>> indicate the size of the resulting array.
> I don't think this API is necessary. If I've understood what you are
> after here, the batteries should simply have the Type-C psy(s) listed
> in the supplied_to and/or supplied_from.
>
> So you just need to make sure your battery nodes have the USB Type-C
> node(s) listed in the "power-supplies" property in your DT, no?
Thanks Heikki! I will evaluate between this approach vs OF graph
approach (as suggested in previous review) and implement it in v2.
>
> thanks,
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>> drivers/power/supply/power_supply_core.c | 60 ++++++++++++++++++++++++++++++++
>> include/linux/power_supply.h | 5 +++
>> 2 files changed, 65 insertions(+)
>>
>> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
>> index 76c340b38015af0a67a0d91305e6242a8646bf53..df1a52f85125748c4fdcb10687aa7ed2f626ded1 100644
>> --- a/drivers/power/supply/power_supply_core.c
>> +++ b/drivers/power/supply/power_supply_core.c
>> @@ -593,6 +593,66 @@ struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
>> EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
>> #endif /* CONFIG_OF */
>>
>> +static int power_supply_match_fwnode(struct device *dev, const void *data)
>> +{
>> + return dev && dev->parent && dev->parent->fwnode == data;
>> +}
>> +
>> +/**
>> + * power_supply_get_by_fwnode_reference_array() - Returns an array of power
>> + * supply objects associated with each fwnode reference present in the property
>> + * @fwnode: Pointer to fwnode to lookup property
>> + * @property: Name of property holding references
>> + * @psy: Resulting array of power_supply pointers. To be provided by the caller.
>> + * @size: size of power_supply pointer array.
>> + *
>> + * If power supply was found, it increases reference count for the
>> + * internal power supply's device. The user should power_supply_put()
>> + * after usage.
>> + *
>> + * Return: On success returns the number of power supply objects filled
>> + * in the @psy array.
>> + * -EOVERFLOW when size of @psy array is not suffice.
>> + * -EINVAL when @psy is NULL or @size is 0.
>> + * -ENODATA when fwnode does not contain the given property
>> + */
>> +int power_supply_get_by_fwnode_reference_array(struct fwnode_handle *fwnode,
>> + const char *property,
>> + struct power_supply **psy,
>> + ssize_t size)
>> +{
>> + int ret, index, count = 0;
>> + struct fwnode_reference_args args;
>> + struct device *dev;
>> +
>> + if (!psy || !size)
>> + return -EINVAL;
>> +
>> + for (index = 0; index < size &&
>> + !(ret = fwnode_property_get_reference_args(fwnode, property, NULL,
>> + 0, index, &args));
>> + ++index) {
>> + dev = class_find_device(&power_supply_class, NULL, args.fwnode,
>> + power_supply_match_fwnode);
>> + fwnode_handle_put(args.fwnode);
>> + if (!dev)
>> + continue;
>> +
>> + if (count > size)
>> + return -EOVERFLOW;
>> +
>> + psy[count] = dev_get_drvdata(dev);
>> + atomic_inc(&psy[count]->use_cnt);
>> + ++count;
>> + }
>> +
>> + if (ret != -ENOENT)
>> + return ret;
>> +
>> + return index ? count : -ENODATA;
>> +}
>> +EXPORT_SYMBOL_GPL(power_supply_get_by_fwnode_reference_array);
>> +
>> int power_supply_get_battery_info(struct power_supply *psy,
>> struct power_supply_battery_info **info_out)
>> {
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index 6ed53b292162469d7b357734d5589bff18a201d0..3f062607e5cd7c7f04384e34128ae0953e25d981 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -820,6 +820,11 @@ devm_power_supply_get_by_phandle(struct device *dev, const char *property)
>> { return NULL; }
>> #endif /* CONFIG_OF */
>>
>> +extern int
>> +power_supply_get_by_fwnode_reference_array(struct fwnode_handle *fwnode,
>> + const char *property,
>> + struct power_supply **psy,
>> + ssize_t size);
>> extern const enum power_supply_property power_supply_battery_info_properties[];
>> extern const size_t power_supply_battery_info_properties_size;
>> extern int power_supply_get_battery_info(struct power_supply *psy,
>>
>> --
>> 2.49.0.rc0.332.g42c0ae87b1-goog
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/5] usb: typec: tcpm: Add support for Battery Status response message
2025-03-12 23:42 [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
2025-03-12 23:42 ` [PATCH 1/5] dt-bindings: connector: add fixed-batteries property Amit Sunil Dhamne via B4 Relay
2025-03-12 23:42 ` [PATCH 2/5] power: supply: core: add function to get supplies from fwnode Amit Sunil Dhamne via B4 Relay
@ 2025-03-12 23:42 ` Amit Sunil Dhamne via B4 Relay
2025-03-12 23:42 ` [PATCH 4/5] power: supply: core: add vendor and product id properties Amit Sunil Dhamne via B4 Relay
` (2 subsequent siblings)
5 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-03-12 23:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso, Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add support for responding to Get_Battery_Status (extended) request with
a Battery_Status (data) msg. The requester shall request the status of
an individual battery by providing an index in Get_Battery_Status. In
case of failure to identify battery, the responder shall reply with an
appropriate message indicating so.
Battery status support is only provided for fixed batteries indexed from
0 - 3.
Support for Battery_Status message is required for sinks that contain
battery as specified in USB PD Rev3.1 v1.8
("Applicability of Data Messages" section).
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Kyle Tso <kyletso@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
---
drivers/usb/typec/tcpm/tcpm.c | 127 +++++++++++++++++++++++++++++++++++++++++-
include/linux/usb/pd.h | 34 +++++++++++
2 files changed, 159 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 6bf1a22c785aff6b1ad77a20d85e22580527f5b1..2d0dcb998608e25c308159873c6b10e178e0a7a1 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -228,6 +228,7 @@ enum pd_msg_request {
PD_MSG_DATA_SINK_CAP,
PD_MSG_DATA_SOURCE_CAP,
PD_MSG_DATA_REV,
+ PD_MSG_DATA_BATT_STATUS
};
enum adev_actions {
@@ -332,6 +333,15 @@ struct pd_timings {
u32 snk_bc12_cmpletion_time;
};
+/*
+ * As per USB PD Spec Rev 3.18 (Sec. 6.5.13.11), a sink can have a maximum
+ * of 4 fixed batteries indexed [0, 3].
+ */
+#define MAX_NUM_FIXED_BATT 4
+
+/* Convert microwatt to watt */
+#define UWH_TO_WH(pow) ((pow) / 1000000)
+
struct tcpm_port {
struct device *dev;
@@ -580,6 +590,15 @@ struct tcpm_port {
/* Indicates maximum (revision, version) supported */
struct pd_revision_info pd_rev;
+
+ struct power_supply *fixed_batt[MAX_NUM_FIXED_BATT];
+ u8 fixed_batt_cnt;
+
+ /*
+ * Variable used to store battery_ref from the Get_Battery_Status
+ * request to process Battery_Status messages.
+ */
+ u8 batt_request;
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
struct mutex logbuffer_lock; /* log buffer access lock */
@@ -1339,6 +1358,62 @@ static int tcpm_pd_send_sink_caps(struct tcpm_port *port)
return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
}
+#define BATTERY_PROPERTY_UNKNOWN 0xffff
+
+static int tcpm_pd_send_batt_status(struct tcpm_port *port)
+{
+ struct pd_message msg;
+ struct power_supply *batt;
+ u32 bsdo;
+ u32 batt_id = port->batt_request;
+ union power_supply_propval val;
+ int ret;
+ bool batt_present = false;
+ u8 charging_status = BSDO_BATTERY_INFO_RSVD;
+ u16 present_charge = BATTERY_PROPERTY_UNKNOWN;
+
+ memset(&msg, 0, sizeof(msg));
+ if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
+ batt_present = true;
+ batt = port->fixed_batt[batt_id];
+ ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_ENERGY_NOW, &val);
+ /* Battery Present Charge is reported in increments of 0.1WH */
+ if (!ret)
+ present_charge = (u16)(UWH_TO_WH(val.intval) * 10);
+
+ ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_STATUS,
+ &val);
+ if (!ret) {
+ switch (val.intval) {
+ case POWER_SUPPLY_STATUS_CHARGING:
+ case POWER_SUPPLY_STATUS_FULL:
+ charging_status = BSDO_BATTERY_INFO_CHARGING;
+ break;
+ case POWER_SUPPLY_STATUS_DISCHARGING:
+ charging_status = BSDO_BATTERY_INFO_DISCHARGING;
+ break;
+ case POWER_SUPPLY_STATUS_NOT_CHARGING:
+ charging_status = BSDO_BATTERY_INFO_IDLE;
+ break;
+ default:
+ charging_status = BSDO_BATTERY_INFO_RSVD;
+ break;
+ }
+ }
+ }
+
+ bsdo = BSDO(present_charge, batt_present ? charging_status : 0,
+ batt_present, !batt_present);
+ msg.payload[0] = cpu_to_le32(bsdo);
+ msg.header = PD_HEADER_LE(PD_DATA_BATT_STATUS,
+ port->pwr_role,
+ port->data_role,
+ port->negotiated_rev,
+ port->message_id,
+ 1);
+ return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
+}
+
static void mod_tcpm_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
{
if (delay_ms) {
@@ -3597,6 +3672,7 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
{
enum pd_ext_msg_type type = pd_header_type_le(msg->header);
unsigned int data_size = pd_ext_header_data_size_le(msg->ext_msg.header);
+ const struct pd_chunked_ext_message_data *ext_msg = &msg->ext_msg;
/* stopping VDM state machine if interrupted by other Messages */
if (tcpm_vdm_ams(port)) {
@@ -3605,7 +3681,7 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
mod_vdm_delayed_work(port, 0);
}
- if (!(le16_to_cpu(msg->ext_msg.header) & PD_EXT_HDR_CHUNKED)) {
+ if (!(le16_to_cpu(ext_msg->header) & PD_EXT_HDR_CHUNKED)) {
tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
tcpm_log(port, "Unchunked extended messages unsupported");
return;
@@ -3630,9 +3706,13 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
NONE_AMS, 0);
}
break;
+ case PD_EXT_GET_BATT_STATUS:
+ port->batt_request = ext_msg->data[0];
+ tcpm_pd_handle_msg(port, PD_MSG_DATA_BATT_STATUS,
+ GETTING_BATTERY_STATUS);
+ break;
case PD_EXT_SOURCE_CAP_EXT:
case PD_EXT_GET_BATT_CAP:
- case PD_EXT_GET_BATT_STATUS:
case PD_EXT_BATT_CAP:
case PD_EXT_GET_MANUFACTURER_INFO:
case PD_EXT_MANUFACTURER_INFO:
@@ -3833,6 +3913,14 @@ static bool tcpm_send_queued_message(struct tcpm_port *port)
ret);
tcpm_ams_finish(port);
break;
+ case PD_MSG_DATA_BATT_STATUS:
+ ret = tcpm_pd_send_batt_status(port);
+ if (ret)
+ tcpm_log(port,
+ "Failed to send battery status ret=%d",
+ ret);
+ tcpm_ams_finish(port);
+ break;
default:
break;
}
@@ -7164,6 +7252,35 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw
port->timings.snk_bc12_cmpletion_time = val;
}
+static void tcpm_fw_get_batt(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+ int i, ret;
+ enum power_supply_type psy_type;
+
+ ret = power_supply_get_by_fwnode_reference_array(fwnode,
+ "fixed-batteries",
+ port->fixed_batt,
+ MAX_NUM_FIXED_BATT);
+ if (ret < 0) {
+ tcpm_log(port,
+ "Unable to parse or find batteries property, ret=%d",
+ ret);
+ return;
+ }
+
+ port->fixed_batt_cnt = ret;
+ for (i = 0; i < port->fixed_batt_cnt; i++) {
+ if (!port->fixed_batt[i])
+ continue;
+
+ psy_type = port->fixed_batt[i]->desc->type;
+ if (psy_type != POWER_SUPPLY_TYPE_BATTERY)
+ tcpm_log(port,
+ "Wrong power supply type (%u) at idx:%d. Should be battery type.",
+ psy_type, i);
+ }
+}
+
static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
{
struct fwnode_handle *capabilities, *child, *caps = NULL;
@@ -7746,6 +7863,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
tcpm_fw_get_timings(port, tcpc->fwnode);
tcpm_fw_get_pd_revision(port, tcpc->fwnode);
+ tcpm_fw_get_batt(port, tcpc->fwnode);
port->try_role = port->typec_caps.prefer_role;
@@ -7827,6 +7945,11 @@ void tcpm_unregister_port(struct tcpm_port *port)
hrtimer_cancel(&port->vdm_state_machine_timer);
hrtimer_cancel(&port->state_machine_timer);
+ for (i = 0; i < port->fixed_batt_cnt; i++) {
+ if (port->fixed_batt[i])
+ power_supply_put(port->fixed_batt[i]);
+ }
+
tcpm_reset_port(port);
tcpm_port_unregister_pd(port);
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 3068c3084eb6176d7d9184c3959a4110282a9fa0..299e7c20127cd5b7dcdae4f24468df4b34b072b5 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -553,4 +553,38 @@ void usb_power_delivery_unlink_device(struct usb_power_delivery *pd, struct devi
#endif /* CONFIG_TYPEC */
+/* Battery Status Data Object */
+#define BSDO_PRESENT_CAPACITY_SHIFT 16
+#define BSDO_PRESENT_CAPACITY_MASK GENMASK(31, 16)
+#define BSDO_CHG_STATUS_SHIFT 10
+#define BSDO_CHG_STATUS_MASK GENMASK(11, 10)
+#define BSDO_BATTERY_PRESENT BIT(9)
+#define BSDO_INVALID_BATTERY_REFERENCE BIT(8)
+
+/*
+ * Battery Charge Status: Battery Charging Status Values as defined in
+ * "USB PD Spec Rev3.1 Ver1.8", "Table 6-46 Battery Status Data Object (BSDO)".
+ */
+#define BSDO_BATTERY_INFO_CHARGING 0x1
+#define BSDO_BATTERY_INFO_DISCHARGING 0x2
+#define BSDO_BATTERY_INFO_IDLE 0x3
+#define BSDO_BATTERY_INFO_RSVD 0x4
+
+/**
+ * BSDO() - Pack data into Battery Status Data Object format
+ * @batt_charge: Battery's present state of charge in 0.1WH increment
+ * @chg_status: Battery charge status
+ * @batt_present: When set, this indicates battery is present/attached.
+ * Otherwise:
+ * - Non hot-swappable battery: Indicates absence of battery
+ * - Hot-swappable battery: Indicates battery is unattached
+ * @invalid_ref: Set when invalid battery reference is made in
+ * Get_Battery_Status request, else 0
+ */
+#define BSDO(batt_charge, chg_status, batt_present, invalid_ref) \
+ ((((batt_charge) << BSDO_PRESENT_CAPACITY_SHIFT) & BSDO_PRESENT_CAPACITY_MASK) | \
+ (((chg_status) << BSDO_CHG_STATUS_SHIFT) & BSDO_CHG_STATUS_MASK) | \
+ ((batt_present) ? BSDO_BATTERY_PRESENT : 0) | \
+ ((invalid_ref) ? BSDO_INVALID_BATTERY_REFERENCE : 0))
+
#endif /* __LINUX_USB_PD_H */
--
2.49.0.rc0.332.g42c0ae87b1-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/5] power: supply: core: add vendor and product id properties
2025-03-12 23:42 [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
` (2 preceding siblings ...)
2025-03-12 23:42 ` [PATCH 3/5] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
@ 2025-03-12 23:42 ` Amit Sunil Dhamne via B4 Relay
2025-03-12 23:42 ` [PATCH 5/5] usb: typec: tcpm: Add support for Battery Cap response message Amit Sunil Dhamne via B4 Relay
2025-03-13 8:50 ` [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Krzysztof Kozlowski
5 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-03-12 23:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso, Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add the following properties:
* Vendor Identifier (VID): Assigned to the battery manufacturer by USB
Implementers Forum (USB-IF).
* Product Identifier (PID) assigned by the manufacturer to the
battery.
This info is required by USB Type-C PD devices containing batteries.
This enables the USB Type C devices to respond to a Battery capacity
request from the port partner by querying for the PID & VID assigned to
the batteries. Refer to "USB Power Delivery Specification Rev3.1 v1.8"
Chapter 5.5 Battery_Capabilities Message.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
Documentation/ABI/testing/sysfs-class-power | 20 ++++++++++++++++++++
Documentation/power/power_supply_class.rst | 11 +++++++++++
drivers/power/supply/power_supply_sysfs.c | 2 ++
include/linux/power_supply.h | 2 ++
4 files changed, 35 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 2a5c1a09a28f91beec6b18ca7b4492093026bc81..b2f377efbf065f7674b8ce44df1bc447c2f1223d 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -822,3 +822,23 @@ Description:
Each entry is a link to the device which registered the extension.
Access: Read
+
+What: /sys/class/power_supply/<supply_name>/usbif_vendor_id
+Date: March 2025
+Contact: linux-pm@vger.kernel.org
+Description:
+ Reports the vendor id assigned to the battery manufacturer by USB
+ Implementers Forum (USB-IF).
+
+ Access: Read
+ Valid values: 0x0-0xffff
+
+What: /sys/class/power_supply/<supply_name>/usbif_product_id
+Date: March 2025
+Contact: linux-pm@vger.kernel.org
+Description:
+ Reports the product id assigned to the battery by the manufacturer
+ (associated with usbif_vendor_id).
+
+ Access: Read
+ Valid values: 0x0-0xffff
diff --git a/Documentation/power/power_supply_class.rst b/Documentation/power/power_supply_class.rst
index da8e275a14ffb9f84bae9ae1efc4720a55ea3010..6d0a6bcf501e719fa4454845b583a8b38d371bb4 100644
--- a/Documentation/power/power_supply_class.rst
+++ b/Documentation/power/power_supply_class.rst
@@ -213,6 +213,17 @@ TIME_TO_FULL
seconds left for battery to be considered full
(i.e. while battery is charging)
+USBIF_VENDOR_ID
+ Vendor ID (VID) assigned to manufacturer or device vendor associated with the
+ battery by USB Implementers Forum (USB-IF). This property is described in
+ "USB Power Delivery Specification Rev3.1 V1.8" Chapter 6.5.5 Battery
+ Capabilities, Section 6.5.5.1 Vendor ID (VID).
+USBIF_PRODUCT_ID
+ Product ID (PID) assigned to the battery, such that if the VID belongs to the
+ manufacturer then the PID will be designated by it. Similarly if the VID
+ belongs to the device vendor then the PID will be designated by it. This
+ property is described in "USB Power Delivery Specification Rev3.1 V1.8"
+ Chapter 6.5.5 Battery Capabilities, Section 6.5.5.2 Product ID (PID).
Battery <-> external power supply interaction
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index edb058c19c9c44ad9ad97a626fc8f59e3d3735a6..534ed3cd049866fa747455bb6dae1ec2dc5e2da6 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -211,6 +211,8 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
POWER_SUPPLY_ATTR(TIME_TO_EMPTY_AVG),
POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
+ POWER_SUPPLY_ATTR(USBIF_VENDOR_ID),
+ POWER_SUPPLY_ATTR(USBIF_PRODUCT_ID),
POWER_SUPPLY_ENUM_ATTR(TYPE),
POWER_SUPPLY_ENUM_ATTR(USB_TYPE),
POWER_SUPPLY_ENUM_ATTR(SCOPE),
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 3f062607e5cd7c7f04384e34128ae0953e25d981..86be5c79db6055611153aa206981cf3de87f1381 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -165,6 +165,8 @@ enum power_supply_property {
POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+ POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
+ POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
POWER_SUPPLY_PROP_TYPE, /* use power_supply.type instead */
POWER_SUPPLY_PROP_USB_TYPE,
POWER_SUPPLY_PROP_SCOPE,
--
2.49.0.rc0.332.g42c0ae87b1-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/5] usb: typec: tcpm: Add support for Battery Cap response message
2025-03-12 23:42 [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
` (3 preceding siblings ...)
2025-03-12 23:42 ` [PATCH 4/5] power: supply: core: add vendor and product id properties Amit Sunil Dhamne via B4 Relay
@ 2025-03-12 23:42 ` Amit Sunil Dhamne via B4 Relay
2025-03-13 8:50 ` [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Krzysztof Kozlowski
5 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-03-12 23:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso, Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add support for responding to Get_Battery_Cap (extended) request with a
a Battery_Capabilities (extended) msg. The requester will request
Battery Cap for a specific battery using an index in Get_Battery_Cap. In
case of failure to identify battery, TCPM shall reply with an
appropriate message indicating so.
Support has been added only for fixed batteries and not hot swappable
ones.
As the Battery Cap Data Block size is 9 Bytes (lesser than
MaxExtendedMsgChunkLen of 26B), only a single chunk is required to
complete the AMS.
Support for Battery_Capabilities message is required for sinks that
contain battery as specified in USB PD Rev3.1 v1.8
("Applicability of Data Messages" section).
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Kyle Tso <kyletso@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
---
drivers/usb/typec/tcpm/tcpm.c | 96 +++++++++++++++++++++++++++++++++++++++++--
include/linux/usb/pd.h | 31 ++++++++++++++
2 files changed, 123 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 2d0dcb998608e25c308159873c6b10e178e0a7a1..22a5a0d463c8738d2160cbe9472207b8053f99b8 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -228,7 +228,8 @@ enum pd_msg_request {
PD_MSG_DATA_SINK_CAP,
PD_MSG_DATA_SOURCE_CAP,
PD_MSG_DATA_REV,
- PD_MSG_DATA_BATT_STATUS
+ PD_MSG_DATA_BATT_STATUS,
+ PD_MSG_EXT_BATT_CAP,
};
enum adev_actions {
@@ -595,8 +596,8 @@ struct tcpm_port {
u8 fixed_batt_cnt;
/*
- * Variable used to store battery_ref from the Get_Battery_Status
- * request to process Battery_Status messages.
+ * Variable used to store battery_ref from the Get_Battery_Status &
+ * Get_Battery_Caps request to process Battery_Status messages.
*/
u8 batt_request;
#ifdef CONFIG_DEBUG_FS
@@ -1414,6 +1415,81 @@ static int tcpm_pd_send_batt_status(struct tcpm_port *port)
return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
}
+static int tcpm_pd_send_batt_cap(struct tcpm_port *port)
+{
+ struct pd_message msg;
+ struct power_supply *batt;
+ struct batt_cap_ext_msg bcdb;
+ u32 batt_id = port->batt_request;
+ int ret;
+ union power_supply_propval val;
+ bool batt_present = false;
+ u16 batt_design_cap = BATTERY_PROPERTY_UNKNOWN;
+ u16 batt_charge_cap = BATTERY_PROPERTY_UNKNOWN;
+ u8 data_obj_cnt;
+ /*
+ * As per USB PD Rev3.1 v1.8, if battery reference is incorrect,
+ * then set the VID field to 0xffff.
+ * If VID field is set to 0xffff, always set the PID field to
+ * 0x0000.
+ */
+ u16 vid = BATTERY_PROPERTY_UNKNOWN;
+ u16 pid = 0x0;
+
+ memset(&msg, 0, sizeof(msg));
+
+ if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
+ batt_present = true;
+ batt = port->fixed_batt[batt_id];
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
+ &val);
+ if (!ret)
+ vid = val.intval;
+
+ if (vid != BATTERY_PROPERTY_UNKNOWN) {
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
+ &val);
+ if (!ret)
+ pid = val.intval;
+ }
+
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
+ &val);
+ if (!ret)
+ batt_design_cap = (u16)(UWH_TO_WH(val.intval) * 10);
+
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_ENERGY_FULL,
+ &val);
+ if (!ret)
+ batt_charge_cap = (u16)(UWH_TO_WH(val.intval) * 10);
+ }
+
+ bcdb.vid = cpu_to_le16(vid);
+ bcdb.pid = cpu_to_le16(pid);
+ bcdb.batt_design_cap = cpu_to_le16(batt_design_cap);
+ bcdb.batt_last_chg_cap = cpu_to_le16(batt_charge_cap);
+ bcdb.batt_type = !batt_present ? BATT_CAP_BATT_TYPE_INVALID_REF : 0;
+ memcpy(msg.ext_msg.data, &bcdb, sizeof(bcdb));
+ msg.ext_msg.header = PD_EXT_HDR_LE(sizeof(bcdb),
+ 0, /* Denotes if request chunk */
+ 0, /* Chunk number */
+ 1 /* Chunked */);
+
+ data_obj_cnt = count_chunked_data_objs(sizeof(bcdb));
+ msg.header = cpu_to_le16(PD_HEADER(PD_EXT_BATT_CAP,
+ port->pwr_role,
+ port->data_role,
+ port->negotiated_rev,
+ port->message_id,
+ data_obj_cnt,
+ 1 /* Denotes if ext header */));
+ return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
+}
+
static void mod_tcpm_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
{
if (delay_ms) {
@@ -3711,8 +3787,12 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
tcpm_pd_handle_msg(port, PD_MSG_DATA_BATT_STATUS,
GETTING_BATTERY_STATUS);
break;
- case PD_EXT_SOURCE_CAP_EXT:
case PD_EXT_GET_BATT_CAP:
+ port->batt_request = ext_msg->data[0];
+ tcpm_pd_handle_msg(port, PD_MSG_EXT_BATT_CAP,
+ GETTING_BATTERY_CAPABILITIES);
+ break;
+ case PD_EXT_SOURCE_CAP_EXT:
case PD_EXT_BATT_CAP:
case PD_EXT_GET_MANUFACTURER_INFO:
case PD_EXT_MANUFACTURER_INFO:
@@ -3921,6 +4001,14 @@ static bool tcpm_send_queued_message(struct tcpm_port *port)
ret);
tcpm_ams_finish(port);
break;
+ case PD_MSG_EXT_BATT_CAP:
+ ret = tcpm_pd_send_batt_cap(port);
+ if (ret)
+ tcpm_log(port,
+ "Failed to send battery cap ret=%d",
+ ret);
+ tcpm_ams_finish(port);
+ break;
default:
break;
}
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 299e7c20127cd5b7dcdae4f24468df4b34b072b5..791f58b7d1de6c4093fcf38b298e7051ebb6e98c 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -204,6 +204,37 @@ struct pd_message {
};
} __packed;
+/*
+ * count_chunked_data_objs: Helper to calculate number of Data Objects on a 4
+ * byte boundary.
+ * @size: Size of data block for extended message. Should *not* include extended
+ * header size.
+ */
+static inline u8 count_chunked_data_objs(u32 size)
+{
+ size += offsetof(struct pd_chunked_ext_message_data, data);
+ return ((size / 4) + (size % 4 ? 1 : 0));
+}
+
+/**
+ * batt_cap_ext_msg - Battery capability extended PD message
+ * @vid: Battery Vendor ID (assigned by USB-IF)
+ * @pid: Battery Product ID (assigned by battery or device vendor)
+ * @batt_design_cap: Battery design capacity in 0.1Wh
+ * @batt_last_chg_cap: Battery last full charge capacity in 0.1Wh
+ * @batt_type: Battery Type. bit0 when set indicates invalid battery reference.
+ * Rest of the bits are reserved.
+ */
+struct batt_cap_ext_msg {
+ __le16 vid;
+ __le16 pid;
+ __le16 batt_design_cap;
+ __le16 batt_last_chg_cap;
+ u8 batt_type;
+} __packed;
+
+#define BATT_CAP_BATT_TYPE_INVALID_REF BIT(0)
+
/* PDO: Power Data Object */
#define PDO_MAX_OBJECTS 7
--
2.49.0.rc0.332.g42c0ae87b1-goog
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-03-12 23:42 [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
` (4 preceding siblings ...)
2025-03-12 23:42 ` [PATCH 5/5] usb: typec: tcpm: Add support for Battery Cap response message Amit Sunil Dhamne via B4 Relay
@ 2025-03-13 8:50 ` Krzysztof Kozlowski
2025-03-15 0:49 ` Amit Sunil Dhamne
5 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-13 8:50 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
> Support for Battery Status & Battery Caps messages in response to
> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
> Applicability" section. This patchset adds support for these AMSes
> to achieve greater compliance with the spec.
Which board uses it? I would be happy to see that connection between
batteries and USB connector on the schematics of some real device. How
does it look like?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-03-13 8:50 ` [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Krzysztof Kozlowski
@ 2025-03-15 0:49 ` Amit Sunil Dhamne
2025-03-16 16:52 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-03-15 0:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
Hi Krzysztof,
Thanks for the review!
On 3/13/25 1:50 AM, Krzysztof Kozlowski wrote:
> On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
>> Support for Battery Status & Battery Caps messages in response to
>> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
>> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
>> Applicability" section. This patchset adds support for these AMSes
>> to achieve greater compliance with the spec.
> Which board uses it? I would be happy to see that connection between
> batteries and USB connector on the schematics of some real device. How
> does it look like?
Any board that uses a USB Type-C connector that supplies power into or
out of a battery while operating in sink or source mode respectively.
The VBUS is connected to the (battery + buck boost IC's CHGin/Vin) or a
companion IFPMIC connected to a battery. In our board we have USB
Connector <-> IFPMIC <-> Battery.
Thanks,
Amit
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-03-15 0:49 ` Amit Sunil Dhamne
@ 2025-03-16 16:52 ` Krzysztof Kozlowski
2025-03-20 21:11 ` Amit Sunil Dhamne
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-16 16:52 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 15/03/2025 01:49, Amit Sunil Dhamne wrote:
> Hi Krzysztof,
>
> Thanks for the review!
>
> On 3/13/25 1:50 AM, Krzysztof Kozlowski wrote:
>> On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
>>> Support for Battery Status & Battery Caps messages in response to
>>> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
>>> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
>>> Applicability" section. This patchset adds support for these AMSes
>>> to achieve greater compliance with the spec.
>> Which board uses it? I would be happy to see that connection between
>> batteries and USB connector on the schematics of some real device. How
>> does it look like?
> Any board that uses a USB Type-C connector that supplies power into or
If you keep responding like this, you will got nowhere, so let me
re-iterate:
Which upstream DTS (or upstream supported hardware) is going to use this
binding, so I can see how you are going to implement it there in the
entire system?
> out of a battery while operating in sink or source mode respectively.
> The VBUS is connected to the (battery + buck boost IC's CHGin/Vin) or a
> companion IFPMIC connected to a battery. In our board we have USB
> Connector <-> IFPMIC <-> Battery.
Which board is that?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-03-16 16:52 ` Krzysztof Kozlowski
@ 2025-03-20 21:11 ` Amit Sunil Dhamne
2025-03-21 7:51 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-03-20 21:11 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 3/16/25 9:52 AM, Krzysztof Kozlowski wrote:
> On 15/03/2025 01:49, Amit Sunil Dhamne wrote:
>> Hi Krzysztof,
>>
>> Thanks for the review!
>>
>> On 3/13/25 1:50 AM, Krzysztof Kozlowski wrote:
>>> On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
>>>> Support for Battery Status & Battery Caps messages in response to
>>>> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
>>>> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
>>>> Applicability" section. This patchset adds support for these AMSes
>>>> to achieve greater compliance with the spec.
>>> Which board uses it? I would be happy to see that connection between
>>> batteries and USB connector on the schematics of some real device. How
>>> does it look like?
>> Any board that uses a USB Type-C connector that supplies power into or
> If you keep responding like this, you will got nowhere, so let me
> re-iterate:
>
> Which upstream DTS (or upstream supported hardware) is going to use this
> binding, so I can see how you are going to implement it there in the
> entire system?
This is for maxim,max33359 Type-C controller.
This would property would have been present for the connector present in
the typec device for gs101-oriole board (that uses the max33359
controller).
However, I will be exploring existing bindings to describe the
relationship for now.
>> out of a battery while operating in sink or source mode respectively.
>> The VBUS is connected to the (battery + buck boost IC's CHGin/Vin) or a
>> companion IFPMIC connected to a battery. In our board we have USB
>> Connector <-> IFPMIC <-> Battery.
> Which board is that?
gs101-oriole board.
Thanks,
Amit
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-03-20 21:11 ` Amit Sunil Dhamne
@ 2025-03-21 7:51 ` Krzysztof Kozlowski
2025-04-03 3:41 ` Amit Sunil Dhamne
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-21 7:51 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 20/03/2025 22:11, Amit Sunil Dhamne wrote:
> On 3/16/25 9:52 AM, Krzysztof Kozlowski wrote:
>> On 15/03/2025 01:49, Amit Sunil Dhamne wrote:
>>> Hi Krzysztof,
>>>
>>> Thanks for the review!
>>>
>>> On 3/13/25 1:50 AM, Krzysztof Kozlowski wrote:
>>>> On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
>>>>> Support for Battery Status & Battery Caps messages in response to
>>>>> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
>>>>> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
>>>>> Applicability" section. This patchset adds support for these AMSes
>>>>> to achieve greater compliance with the spec.
>>>> Which board uses it? I would be happy to see that connection between
>>>> batteries and USB connector on the schematics of some real device. How
>>>> does it look like?
>>> Any board that uses a USB Type-C connector that supplies power into or
>> If you keep responding like this, you will got nowhere, so let me
>> re-iterate:
>>
>> Which upstream DTS (or upstream supported hardware) is going to use this
>> binding, so I can see how you are going to implement it there in the
>> entire system?
>
> This is for maxim,max33359 Type-C controller.
Stop deflecting the questions. max33359 is not a board. I already asked
two times.
Apparently admitting "no upstream users" is impossible, so let's state
the obvious:
There are no upstream users of this.
>
> This would property would have been present for the connector present in
> the typec device for gs101-oriole board (that uses the max33359
> controller).
But it is not.
>
> However, I will be exploring existing bindings to describe the
> relationship for now.
>
>>> out of a battery while operating in sink or source mode respectively.
>>> The VBUS is connected to the (battery + buck boost IC's CHGin/Vin) or a
>>> companion IFPMIC connected to a battery. In our board we have USB
>>> Connector <-> IFPMIC <-> Battery.
>> Which board is that?
>
> gs101-oriole board.
Then why this is not used? The board was released some years ago, so I
do not see a problem in using it.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-03-21 7:51 ` Krzysztof Kozlowski
@ 2025-04-03 3:41 ` Amit Sunil Dhamne
2025-04-03 8:00 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-04-03 3:41 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 3/21/25 12:51 AM, Krzysztof Kozlowski wrote:
> On 20/03/2025 22:11, Amit Sunil Dhamne wrote:
>> On 3/16/25 9:52 AM, Krzysztof Kozlowski wrote:
>>> On 15/03/2025 01:49, Amit Sunil Dhamne wrote:
>>>> Hi Krzysztof,
>>>>
>>>> Thanks for the review!
>>>>
>>>> On 3/13/25 1:50 AM, Krzysztof Kozlowski wrote:
>>>>> On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
>>>>>> Support for Battery Status & Battery Caps messages in response to
>>>>>> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
>>>>>> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
>>>>>> Applicability" section. This patchset adds support for these AMSes
>>>>>> to achieve greater compliance with the spec.
>>>>> Which board uses it? I would be happy to see that connection between
>>>>> batteries and USB connector on the schematics of some real device. How
>>>>> does it look like?
>>>> Any board that uses a USB Type-C connector that supplies power into or
>>> If you keep responding like this, you will got nowhere, so let me
>>> re-iterate:
>>>
>>> Which upstream DTS (or upstream supported hardware) is going to use this
>>> binding, so I can see how you are going to implement it there in the
>>> entire system?
>> This is for maxim,max33359 Type-C controller.
> Stop deflecting the questions. max33359 is not a board. I already asked
> two times.
>
> Apparently admitting "no upstream users" is impossible, so let's state
> the obvious:
>
> There are no upstream users of this.
max33359 controller has an upstream user i.e., gs101-oriole (Pixel 6)
board. Totally agree that at the moment there are no upstream
devices/drivers for the Fuel Gauge (that my patchset has a dependency
on) in gs101-oriole board. gs101-oriole uses max77759 fuel gauge device.
I see that there's an effort for upstreaming it
(https://lore.kernel.org/all/20250102-b4-gs101_max77759_fg-v2-0-87959abeb7ff@uclouvain.be/).
I will mark my patches as dependent on it + demonstrate the relationship
of the devices in the gs101-oriole board. Hope that's okay?
Thanks,
Amit
>> This would property would have been present for the connector present in
>> the typec device for gs101-oriole board (that uses the max33359
>> controller).
>
> But it is not.
>
>
>> However, I will be exploring existing bindings to describe the
>> relationship for now.
>>
>>>> out of a battery while operating in sink or source mode respectively.
>>>> The VBUS is connected to the (battery + buck boost IC's CHGin/Vin) or a
>>>> companion IFPMIC connected to a battery. In our board we have USB
>>>> Connector <-> IFPMIC <-> Battery.
>>> Which board is that?
>> gs101-oriole board.
>
> Then why this is not used? The board was released some years ago, so I
> do not see a problem in using it.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-04-03 3:41 ` Amit Sunil Dhamne
@ 2025-04-03 8:00 ` Krzysztof Kozlowski
2025-04-03 8:02 ` Krzysztof Kozlowski
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-03 8:00 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 03/04/2025 05:41, Amit Sunil Dhamne wrote:
>
> On 3/21/25 12:51 AM, Krzysztof Kozlowski wrote:
>> On 20/03/2025 22:11, Amit Sunil Dhamne wrote:
>>> On 3/16/25 9:52 AM, Krzysztof Kozlowski wrote:
>>>> On 15/03/2025 01:49, Amit Sunil Dhamne wrote:
>>>>> Hi Krzysztof,
>>>>>
>>>>> Thanks for the review!
>>>>>
>>>>> On 3/13/25 1:50 AM, Krzysztof Kozlowski wrote:
>>>>>> On Wed, Mar 12, 2025 at 04:42:00PM -0700, Amit Sunil Dhamne wrote:
>>>>>>> Support for Battery Status & Battery Caps messages in response to
>>>>>>> Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
>>>>>>> powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
>>>>>>> Applicability" section. This patchset adds support for these AMSes
>>>>>>> to achieve greater compliance with the spec.
>>>>>> Which board uses it? I would be happy to see that connection between
>>>>>> batteries and USB connector on the schematics of some real device. How
>>>>>> does it look like?
>>>>> Any board that uses a USB Type-C connector that supplies power into or
>>>> If you keep responding like this, you will got nowhere, so let me
>>>> re-iterate:
>>>>
>>>> Which upstream DTS (or upstream supported hardware) is going to use this
>>>> binding, so I can see how you are going to implement it there in the
>>>> entire system?
>>> This is for maxim,max33359 Type-C controller.
>> Stop deflecting the questions. max33359 is not a board. I already asked
>> two times.
>>
>> Apparently admitting "no upstream users" is impossible, so let's state
>> the obvious:
>>
>> There are no upstream users of this.
>
> max33359 controller has an upstream user i.e., gs101-oriole (Pixel 6)
> board. Totally agree that at the moment there are no upstream
> devices/drivers for the Fuel Gauge (that my patchset has a dependency
> on) in gs101-oriole board. gs101-oriole uses max77759 fuel gauge device.
> I see that there's an effort for upstreaming it
> (https://lore.kernel.org/all/20250102-b4-gs101_max77759_fg-v2-0-87959abeb7ff@uclouvain.be/).
> I will mark my patches as dependent on it + demonstrate the relationship
> of the devices in the gs101-oriole board. Hope that's okay?
Then please send the DTS for GS101 Oriole using this binding. I don't
understand the point of adding binding for some user and in the same
time not doing anything for that user.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-04-03 8:00 ` Krzysztof Kozlowski
@ 2025-04-03 8:02 ` Krzysztof Kozlowski
2025-04-08 19:50 ` Amit Sunil Dhamne
0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-03 8:02 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 03/04/2025 10:00, Krzysztof Kozlowski wrote:
>>>>>
>>>>> Which upstream DTS (or upstream supported hardware) is going to use this
>>>>> binding, so I can see how you are going to implement it there in the
>>>>> entire system?
>>>> This is for maxim,max33359 Type-C controller.
>>> Stop deflecting the questions. max33359 is not a board. I already asked
>>> two times.
>>>
>>> Apparently admitting "no upstream users" is impossible, so let's state
>>> the obvious:
>>>
>>> There are no upstream users of this.
>>
>> max33359 controller has an upstream user i.e., gs101-oriole (Pixel 6)
>> board. Totally agree that at the moment there are no upstream
>> devices/drivers for the Fuel Gauge (that my patchset has a dependency
>> on) in gs101-oriole board. gs101-oriole uses max77759 fuel gauge device.
>> I see that there's an effort for upstreaming it
>> (https://lore.kernel.org/all/20250102-b4-gs101_max77759_fg-v2-0-87959abeb7ff@uclouvain.be/).
>> I will mark my patches as dependent on it + demonstrate the relationship
>> of the devices in the gs101-oriole board. Hope that's okay?
>
> Then please send the DTS for GS101 Oriole using this binding. I don't
> understand the point of adding binding for some user and in the same
> time not doing anything for that user.
... and just a reminder: DTS goes to separate patchset (!) from USB
drivers one, with lore link in changelog or cover letter to the binding.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
2025-04-03 8:02 ` Krzysztof Kozlowski
@ 2025-04-08 19:50 ` Amit Sunil Dhamne
0 siblings, 0 replies; 23+ messages in thread
From: Amit Sunil Dhamne @ 2025-04-08 19:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek,
devicetree, linux-kernel, linux-usb, linux-pm, RD Babiera,
Kyle Tso
On 4/3/25 1:02 AM, Krzysztof Kozlowski wrote:
> On 03/04/2025 10:00, Krzysztof Kozlowski wrote:
>>>>>> Which upstream DTS (or upstream supported hardware) is going to use this
>>>>>> binding, so I can see how you are going to implement it there in the
>>>>>> entire system?
>>>>> This is for maxim,max33359 Type-C controller.
>>>> Stop deflecting the questions. max33359 is not a board. I already asked
>>>> two times.
>>>>
>>>> Apparently admitting "no upstream users" is impossible, so let's state
>>>> the obvious:
>>>>
>>>> There are no upstream users of this.
>>> max33359 controller has an upstream user i.e., gs101-oriole (Pixel 6)
>>> board. Totally agree that at the moment there are no upstream
>>> devices/drivers for the Fuel Gauge (that my patchset has a dependency
>>> on) in gs101-oriole board. gs101-oriole uses max77759 fuel gauge device.
>>> I see that there's an effort for upstreaming it
>>> (https://lore.kernel.org/all/20250102-b4-gs101_max77759_fg-v2-0-87959abeb7ff@uclouvain.be/).
>>> I will mark my patches as dependent on it + demonstrate the relationship
>>> of the devices in the gs101-oriole board. Hope that's okay?
>> Then please send the DTS for GS101 Oriole using this binding. I don't
>> understand the point of adding binding for some user and in the same
>> time not doing anything for that user.
>
> ... and just a reminder: DTS goes to separate patchset (!) from USB
> drivers one, with lore link in changelog or cover letter to the binding.
Sure thing!
Thanks
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread