Re: [PATCH v2 4/5] power: supply: core: add vendor and product id properties

[Amit Sunil Dhamne <amitsd@google.com>]

Hi Sebastian,

On 6/23/25 2:44 PM, Sebastian Reichel wrote:
> Hi,
>
> On Wed, May 07, 2025 at 06:00:25PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
>> 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 | 19 +++++++++++++++----
>>   Documentation/power/power_supply_class.rst  | 11 +++++++++++
>>   drivers/power/supply/power_supply_sysfs.c   |  2 ++
>>   include/linux/power_supply.h                |  2 ++
>>   4 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
>> index 2a5c1a09a28f91beec6b18ca7b4492093026bc81..5495e82885b2294cdfd5ace0e7e5fcbeadfccb5f 100644
>> --- a/Documentation/ABI/testing/sysfs-class-power
>> +++ b/Documentation/ABI/testing/sysfs-class-power
>> @@ -814,11 +814,22 @@ Description:
>>   		Access: Read
>>   		Valid values: 1-31
>>   
>> -What:		/sys/class/power_supply/<supply_name>/extensions/<extension_name>
>> -Date:		March 2025
> Why does this remove existing Documentation?

Oops. I think that probably happened while resolving merge conflicts  I 
guess. Will fix.


>
>> +What:		/sys/class/power_supply/<supply_name>/usbif_vendor_id
> I think we can use USB_VENDOR_ID and USB_PRODUCT_ID for this like
> everyone else?

Agreed.


>
>> +Date:		May 2025
>>   Contact:	linux-pm@vger.kernel.org
>>   Description:
>> -		Reports the extensions registered to the power supply.
>> -		Each entry is a link to the device which registered the extension.
>> +		Reports the vendor id assigned to the battery manufacturer by USB
>> +		Implementers Forum (USB-IF).
>>   
>>   		Access: Read
>> +		Valid values: 0x0-0xffff
> If I haven't missed something the formatting will be in decimal. I
> think the hex format is more sensible, so this needs some extra
> handling in power_supply_format_property() in power_supply_sysfs.c.

I see. I will add special handling for these in 
power_supply_format_property().


>
>> +
>> +What:		/sys/class/power_supply/<supply_name>/usbif_product_id
>> +Date:		May 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 a35b08bd368e9305554e1a608dc8e526983cfa12..100eb559dcede938595ffbf83bc5ef3645a5a172 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,
> Neither this series, nor the Pixel 6 one seems to have any user for
> these new properties? This becomes part of the kernel ABI, so we do
> not add new properties without a user.

I realize that there's no current driver that supplies these values atm. 
I plan to add it for 
https://lore.kernel.org/all/20250523-b4-gs101_max77759_fg-v4-0-b49904e35a34@uclouvain.be/ 
driver once it gets accepted.


Thanks,

Amit

>
> Greetings,
>
> -- Sebastian