Re: [typec] pd: fixed source capability: peak_current field

Re: [typec] pd: fixed source capability: peak_current field

[Heikki Krogerus]

+linux-usb ml, Guenter and others.

On Sun, Sep 10, 2023 at 09:29:04PM -0400, Douglas Gilbert wrote:
> Hi,
> The Peak current 2 bit field has been present in the fixed source PDO
> since PD 2.0 . It is commented out in typec/pd.c with a note to revisit
> as it needs the Operational current field from the active RDO.
> 
> Not sure I agree with that: the 2 bit field is unit-less and could easily
> be displayed as such. At the moment the active RDO is not directly
> available through sysfs and would certainly be a useful addition for
> many reasons.
> 
> So my vote (not that I get one) would be to output peak_current in
> sysfs with no unit. Then in the ABI peak_current could be explained
> by reference to a table in the spec (e.g. "USB Power Delivery
> Specification Revision 3.1, Version 1.8, table 6-10 Fixed Power
> Source Peak Current Capability"). Then the issue of what it means
> is handed off to the user space :-)
> 
> Also of note is that the Peak current field is B21..B20 which is
> below B23 (obviously) so all fixed source PDOs have that field
> (and thus those unit-less values could differ), not just
> PDO #1 .

I agree with you. I don't think it makes sense to try to interpret the
peak current value in kernel, so we could just expose the value as-is.

But first giving the others heads-up on this, just in case somebody
disagrees.

thanks,

-- 
heikki

Re: [typec] pd: fixed source capability: peak_current field

[Douglas Gilbert]

On 2023-09-11 09:45, Heikki Krogerus wrote:
> +linux-usb ml, Guenter and others.
> 
> On Sun, Sep 10, 2023 at 09:29:04PM -0400, Douglas Gilbert wrote:
>> Hi,
>> The Peak current 2 bit field has been present in the fixed source PDO
>> since PD 2.0 . It is commented out in typec/pd.c with a note to revisit
>> as it needs the Operational current field from the active RDO.
>>
>> Not sure I agree with that: the 2 bit field is unit-less and could easily
>> be displayed as such. At the moment the active RDO is not directly
>> available through sysfs and would certainly be a useful addition for
>> many reasons.
>>
>> So my vote (not that I get one) would be to output peak_current in
>> sysfs with no unit. Then in the ABI peak_current could be explained
>> by reference to a table in the spec (e.g. "USB Power Delivery
>> Specification Revision 3.1, Version 1.8, table 6-10 Fixed Power
>> Source Peak Current Capability"). Then the issue of what it means
>> is handed off to the user space :-)
>>
>> Also of note is that the Peak current field is B21..B20 which is
>> below B23 (obviously) so all fixed source PDOs have that field
>> (and thus those unit-less values could differ), not just
>> PDO #1 .
> 
> I agree with you. I don't think it makes sense to try to interpret the
> peak current value in kernel, so we could just expose the value as-is.
> 
> But first giving the others heads-up on this, just in case somebody
> disagrees.

Similar situation with fast_role_swap in the fixed sink PDO. It is not really a
current but an index to a table embedded in the r31_v18's table 6-16.
Importantly the value 0 implies the sink does not support FRS. So again it
seems to be unit-less so the suffix "_current" in its sysfs name is
questionable. The spec does help much here formally naming it: the "Fast Role
Swap USB Type-C® Current" field in section 6.4.1.3.1.6 . The show function
for the fast_role_swap_current is defined in typec/pd.c but I didn't see any
invocations.

Doug Gilbert