Re: [PATCH v1 1/2] dt-bindings: power: supply: Document Maxim MAX8971 charger

[Krzysztof Kozlowski <krzk@kernel.org>]

On 23/02/2025 15:28, Svyatoslav Ryhel wrote:
>>>>> +  extcon:
>>>>> +    description:
>>>>> +      Special device used to detect type of plug.
>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>>>
>>>> You rather miss proper connector or ports. Which device pins are
>>>> describbed here?
>>>>
>>>
>>> This is an optional phandle to extcon, which can detect plug type. If
>>
>> I know what is this, you just wrote. extcon property is not allowed anymore.
>>
> 
> There is no helper for obtaining extcon via graph endpoint I am aware
> of. Can you provide an example of graph parsing extcon helper?

Most of recent drivers have the same problem and they solved it somehow
without introducing 'extcon' property.

> 
>>> extcon is provided, charger then can change its setup to better
>>> provide supply to the battery. If no extcon is provided, device itself
>>> can detect only the fact of charger without details about type.
>>
>>
>>>
>>>>
>>>>> +
>>>>> +  maxim,fcharge-current-limit-microamp:
>>>>> +    description:
>>>>> +      Fast-Charge current limit
>>>>> +    minimum: 250000
>>>>> +    default: 500000
>>>>> +    maximum: 1550000
>>>>> +
>>>>> +  maxim,fcharge-timer-hours:
>>>>> +    description: |
>>>>> +      Fast-Charge timer in hours. Setting this value 3 and lower or 11 and
>>>>> +      higher will disable Fast-Charge timer.
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    default: 5
>>>>> +
>>>>> +  maxim,fcharge-rst-threshold-high:
>>>>> +    description:
>>>>> +      Set Fast-Charge reset threshold to -100 mV
>>>>> +    type: boolean
>>>>> +
>>>>> +  maxim,in-current-limit-microamp:
>>>>> +    description:
>>>>> +      Input current limit
>>>>> +    minimum: 100000
>>>>> +    default: 500000
>>>>> +    maximum: 1500000
>>>>> +
>>>>> +  maxim,topoff-timer-minutes:
>>>>> +    description:
>>>>> +      Top-Off timer minutes
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [0, 10, 20, 30, 40, 50, 60, 70]
>>>>> +    default: 30
>>>>> +
>>>>> +  maxim,topoff-current-threshold-microamp:
>>>>> +    description:
>>>>> +      Top-Off current threshold
>>>>> +    enum: [50000, 100000, 150000, 200000]
>>>>> +    default: 50000
>>>>> +
>>>>> +  maxim,fcharge-usb-current-limit-microamp:
>>>>> +    description:
>>>>> +      Fast-Charge USB current limit
>>>>> +    minimum: 100000
>>>>> +    default: 500000
>>>>> +    maximum: 1500000
>>>>> +
>>>>> +  maxim,fcharge-ac-current-limit-microamp:
>>>>> +    description:
>>>>> +      Fast-Charge AC current limit
>>>>> +    minimum: 100000
>>>>> +    default: 500000
>>>>> +    maximum: 1500000
>>>>> +
>>>>> +  maxim,usb-in-current-limit-microamp:
>>>>> +    description:
>>>>> +      USB Input current limit
>>>>> +    minimum: 100000
>>>>> +    default: 500000
>>>>> +    maximum: 1500000
>>>>> +
>>>>> +  maxim,ac-in-current-limit-microamp:
>>>>> +    description:
>>>>> +      AC Input current limit
>>>>> +    minimum: 100000
>>>>> +    default: 500000
>>>>> +    maximum: 1500000
>>>>
>>>> For all or most of these you miss monitored batter.
>>>>
>>>
>>> This is a charger, it does not monitor battery. There is a fuel gauge
>>> for that or a battery monitor.
>>>
>> What does charger charge? Battery or something else?
>>
> 
> Anything it is linked to. Moreover ref to power supply is not needed

Not true. I opened datasheet and it clearly says:
"1-Cell Li+ charger".

It cannot charge other things properly, so it cannot charge "anything".
It only charges batteries.

"The MAX8971 is a compact, high-frequency, high-effi-
ciency switch-mode charger for a one-cell lithium-ion (Li+)
battery. "

> since properties from there are not relevant to this driver.

We talk about hardware here.

> monitored-battery refers to a simple battery cell, which has nothing
> to do with charger and power-supplies is irrelevant since this device

battery cell has everything to do with charger because it describes the
characteristic of what device you are charging: e.g. maximum input
current or top-off currents, which you define here as well.

You are duplicating some properties and adding some which are not DT at
all (all timers). Look how other devices do it.


Best regards,
Krzysztof