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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 4 Oct 2017 10:12:51 +0200
From:   Richard Leitner <richard.leitner@...data.com>
To:     Serge Semin <fancer.lancer@...il.com>,
        Rob Herring <robh@...nel.org>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mark Rutland <mark.rutland@....com>,
        <Sergey.Semin@...latforms.ru>,
        Linux USB List <linux-usb@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Richard Leitner <richard.leitner@...data.com>
Subject: Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property
 support


On 09/21/2017 07:10 PM, Serge Semin wrote:
> On Thu, Sep 21, 2017 at 11:26:04AM -0500, Rob Herring <robh@...nel.org> wrote:
>> On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin <fancer.lancer@...il.com> 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...

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?

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

regards,
Richard.L