Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

[Rob Herring]

On Wed, Oct 4, 2017 at 3:12 AM, Richard Leitner
<richard.leitner@xxxxxxxxxxx> wrote:
>
> On 09/21/2017 07:10 PM, Serge Semin wrote:
>> On Thu, Sep 21, 2017 at 11:26:04AM -0500, Rob Herring <robh@xxxxxxxxxx> wrote:
>>> On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
>
> ...
>
>>>> These are different parameters of the device. They got different configuration
>>>> registers and descriptions:
>>>> max_power* - ... This value also includes the power consumption of a
>>>> permanently attached peripheral if the hub is configured as a compound
>>>> device, and the embedded peripheral reports 0mA in its descriptors.
>>>> max_current* - ... This value does NOT include the power consumption of a
>>>> permanently attached peripheral if the hub is configured as a compound
>>>> device.
>>>
>>> Then the names here should somehow reflect the above. Perhaps
>>> "composite-current" and "hub-current" or something like that.
>>>
>>
>> I left the naming in accordance with the device datasheet. I thought it would be
>> better since the driver user would still need to consult with the device
>> documentation to properly set them. I don't really get how the difference is reflected
>> with the naming declared there though. So what naming would you prefer then? Might be
>> something like:
>> {sp,bp}-max-total-current - for so named {sp,bp}-max-power, since it includes all the
>> permanently attached peripherals.
>> {sp,bp}-max-removable-current - for so named {sp,bp}-max-current, since it doesn't
>> include the permanently attached peripherals.
>>
>> Or is it better to leave it in compliance with the documentation naming?
>
> I'd prefer the naming to be in accordance with the device datasheet too.
> Changing it will IMHO generate avoidable misunderstandings...

Okay, then add a vendor prefix.

> But if we should go with Robs proposal please make sure the name from
> the device datasheet is mentioned somewhere in the description of the
> binding.
>
>>
>>>>
>>>> Additionally as you can see, they both are measured in "mA", so it isn't
>>>> a real physical power.
>>>
>>> Well, I can't because there's no units.
>>>
>>
>> What this line means then?
>> - sp-max-{power,current} : ... The value is given in mA in a 0 - 100 range (default is 1mA).
>> - bp-max-{power,current} : ... The value is given in mA in a 0 - 510 range (default is 100mA).
>>
>> Maybe I don't know something and the description line should state the units somehow
>> clearer?

The reason we have units in the name is so we don't have to go lookup
the documentation to find the units and so people don't use the same
property name with different units.

> Append the unit to the binding name, just like in "power-on-time-ms". As
> there's no "Standard Unit Suffix" for mA stated in the documentation
> (Documentation/devicetree/bindings/property-units.txt) I think it should
> be "-milliamp"?

The reason being is we align around microamps and try to avoid
everyone picking their own units (celliamps anyone?). So unless
microamps is not enough range for you, I would stick with that.

If you have both units and vendor prefix, then I care less about the
poorly written datasheet naming used.

Rob