Re: [PATCH 2/3] dt-bindings: chrome: add new binding google,cros-ec-chrage-state
From: Krzysztof Kozlowski
Date: Wed Nov 20 2024 - 11:35:30 EST
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