linux-kernel - Re: [PATCH v4 1/4] dt-bindings: mfd: Add TI TPS6594 PMIC

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f69d7b3a-6b30-5f30-9e72-7197a3a62a2c@linaro.org>
Date:   Tue, 28 Mar 2023 13:21:36 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Julien Panis <jpanis@...libre.com>, lee@...nel.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        corbet@....net, arnd@...db.de, gregkh@...uxfoundation.org,
        derek.kiernan@...inx.com, dragan.cvetic@...inx.com
Cc:     eric.auger@...hat.com, jgg@...pe.ca, razor@...ckwall.org,
        stephen@...workplumber.org, davem@...emloft.net,
        christian.koenig@....com, contact@...rsion.fr,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, sterzik@...com, u-kumar1@...com,
        eblanc@...libre.com, jneanne@...libre.com
Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Add TI TPS6594 PMIC

On 28/03/2023 12:45, Julien Panis wrote:
> 
> 
> On 3/28/23 08:51, Krzysztof Kozlowski wrote:
>> On 27/03/2023 17:40, Julien Panis wrote:
>>> TPS6594 is a Power Management IC which provides regulators and others
>>> features like GPIOs, RTC, watchdog, ESMs (Error Signal Monitor), and
>>> PFSM (Pre-configurable Finite State Machine) managing the state of the
>>> device.
>>> TPS6594 is the super-set device while TPS6593 and LP8764X are derivatives.
>>>
>>> Signed-off-by: Julien Panis <jpanis@...libre.com>
>>> ---
>>>   .../devicetree/bindings/mfd/ti,tps6594.yaml   | 231 ++++++++++++++++++
>>>   1 file changed, 231 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>> new file mode 100644
>>> index 000000000000..4498e6361b34
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml
>>> @@ -0,0 +1,231 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mfd/ti,tps6594.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: TI TPS6594 Power Management Integrated Circuit
>>> +
>>> +maintainers:
>>> +  - Julien Panis <jpanis@...libre.com>
>>> +
>>> +description:
>>> +  TPS6594 is a Power Management IC which provides regulators and others
>>> +  features like GPIOs, RTC, watchdog, ESMs (Error Signal Monitor), and
>>> +  PFSM (Pre-configurable Finite State Machine) managing the state of the device.
>>> +  TPS6594 is the super-set device while TPS6593 and LP8764X are derivatives.
>> LP8764X? Compatible says LP8764.
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,lp8764
>> It's confusing. If x was wildcard, didn't you remove part of model name?
> 
> OK, I will remove 'X' from model name in v5.

There is no x in compatible. What is (are) the model name(s)?

> 
>>
>>
>>> +      - ti,tps6593
>>> +      - ti,tps6594

(...)

>>> +
>>> +  rtc:
>>> +    type: object
>>> +    description: RTC provided by this controller.
>>> +    $ref: /schemas/rtc/rtc.yaml#
>> I doubt that you can have here any RTC and any watchdog (below). This
>> should be specific binding instead. Or list of compatibles if you have 3
>> or more possible bindings.
>>
>> Additionally, judging by your DTS you do not have any resources in rtc
>> and watchdog, so these should not be nodes by themself in such case.
> 
> It seems that I can't figure out what you and Rob mean by saying that
> "binding must be complete" and that "RTC and watchdog may or may not
> need binding changes".
> What does "specific binding" mean ?

Specific means not loose, not generic, precise with some accurate
properties.

> Should we add some specific property
> for RTC/WDG provided by the PMIC ?

You know ask me to know what is in your device. I don't know. You should
know.

> Should we write another yaml for both
> of them ? 

Depends. Pretty often yes, but think what do you want to put there?

> Why shouldn't they use the generic rtc/watchdog yaml ? 

There are no properties in these nodes, so you do not need nodes. Or if
you have properties then you need specific binding, not generic one.

> I don't
> understand why they would need some "binding changes". Any example
> I could refer to ? (I might have not looked at the relevant ones for my case
> before sending this v4)

git grep $ref | grep rtc.yaml


Best regards,
Krzysztof