linux-kernel - Re: [PATCH 2/3] dt-bindings: chrome: add new binding google,cros-ec-chrage-state

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]

Message-ID: <caafbc83-22de-4699-a126-e5df13866f1b@kernel.org>
Date: Wed, 20 Nov 2024 17:35:13 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: "Sung-Chi, Li" <lschyi@...omium.org>, Rob Herring <robh@...nel.org>
Cc: Benson Leung <bleung@...omium.org>, Tzung-Bi Shih <tzungbi@...nel.org>,
 Guenter Roeck <groeck@...omium.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Lee Jones <lee@...nel.org>, linux-kernel@...r.kernel.org,
 chrome-platform@...ts.linux.dev, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: chrome: add new binding
 google,cros-ec-chrage-state

On 19/11/2024 03:23, Sung-Chi, Li wrote:
> 
>>> +
>>> +  type:
>>
>> Too generic. Property types are global. You need a vendor prefix for 
>> starters.
>>
> 
> Thank you, I will use a more specific name in the following patches.
> 
>>> +    description: current limit type.
>>> +    enum:
>>> +      - charge
>>> +      - input
>>
>> What if you need to describe both?
>>
> 
> We need to declare different DTS nods for each. This node is representing the
> constraint, not the charge chip itself.

Looks like you are re-implementing charger manager. Even title says: driver.

Use standard psy properties including battery. All this is supposed to
describe hardware, not your driver.

> The voltage, min and max milliamp on each current type are different on a single
> charge chip. For example, I have a device that uses the charge chip rt9490, and
> it has the following set up:

> 
> - Input current
>   - min-milliamp: 100
>   - max-milliamp: 3300
> - Charge current
>   - min-milliamp: 150
>   - max-milliamp: 5000
> 
> I cannot find a clean way to merge different current type, max, and min milliamp
> just in a single DTS node.

Well, all other bindings were able, so I really do not get why this one
is so special.

> Also, we need to split different constraints into its own DTS node. It is
> because the a cooling device in the thermal framework need its own DTS node, so
> we can use it in the trip section.

So fix thermal framework.

Best regards,
Krzysztof