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

[Krzysztof Kozlowski <krzk@kernel.org>]

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