Re: [PATCH 1/3] dt-bindings: mfd: Add power-button option for TI TPS6594 PMIC

[Krzysztof Kozlowski <krzk@kernel.org>]

On 02/06/2025 15:07, Job Sava wrote:
> On Thu, May 29, 2025 at 5:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Fri, May 23, 2025 at 09:46:49AM GMT, Job Sava wrote:
>>> On Wed, May 21, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On Tue, May 20, 2025 at 01:43:36PM GMT, Job Sava wrote:
>>>>> The TPS6594 power-button option permits users to enter STANDBY or
>>>>> ACTIVE state by a push, release, or short push button request.
>>>>>
>>>>> Signed-off-by: Job Sava <jsava@criticallink.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/mfd/ti,tps6594.yaml | 15 +++++++++++++++
>>>>>  1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>>> index 6341b6070366..a40808fd2747 100644
>>>>> --- a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>>> @@ -37,6 +37,21 @@ properties:
>>>>>        device on the SPMI bus, and the secondary PMICs are the target devices
>>>>>        on the SPMI bus.
>>>>>
>>>>> +  ti,power-button:
>>>>> +    type: boolean
>>>>> +    description: |
>>>>> +      Optional property that sets the EN/PB/VSENSE pin to be a
>>>>> +      power-button.
>>>>> +      TPS6594 has a multipurpose pin called EN/PB/VSENSE that can be either
>>>>> +      1. EN in which case it functions as an enable pin.
>>>>> +      2. VSENSE which compares the voltages and triggers an automatic
>>>>> +      on/off request.
>>>>> +      3. PB in which case it can be configured to trigger an interrupt
>>>>> +      to the SoC.
>>>>> +      ti,power-button reflects the last one of those options
>>>>> +      where the board has a button wired to the pin and triggers
>>>>> +      an interrupt on pressing it.
>>>>
>>>> Don't you need to handle two other cases as well? I assume you copied
>>>> this from the other binding, but all three options are valid?
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> Hello Krzysztof,
>>>
>>> Thank you for your response!
>>>
>>> I agree that the other two cases are valid options. However, for this
>>> particular patch series, they may be out of scope. The primary goal of
>>> this patch is to enable push-button functionality, rather than
>>> addressing the VSENSE or EN modes.
>>
>> Binding should be complete, because if you design this as bool, it
>> cannot be later changed to three-state (enum).
>>
>> I don't know if the EN and VSENSE modes are anyhow useful, maybe people
>> interested in this hardware should say.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Hi Krzysztof,
> 
> Thanks again for the feedback.
> 
> I modeled this binding after the TPS65219 PMIC, which uses a boolean

Yeah, that's what I meant in my first reply.

> for ti,power-button, despite the same EN/PB/VSENSE options. Since this
> patch only enables PB mode, I felt a boolean was appropriate and
> consistent.

Properties should have only one type, so that would be a different
property. Someone knowing the device should come with arguments whether
other states for this are useful at all. Or not useful and then argument
that in commit msg for example.


Best regards,
Krzysztof