linux-kernel - Re: [PATCH 1/7] dt-bindings: arm: apple: Add apple,pmgr binding

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1c36c3e6-c1aa-0779-5f6d-8980377c45a4@canonical.com>
Date:   Wed, 6 Oct 2021 09:30:03 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>
To:     Hector Martin <marcan@...can.st>,
        linux-arm-kernel@...ts.infradead.org
Cc:     Marc Zyngier <maz@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Arnd Bergmann <arnd@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Mark Kettenis <mark.kettenis@...all.nl>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        devicetree@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
        linux-serial@...r.kernel.org
Subject: Re: [PATCH 1/7] dt-bindings: arm: apple: Add apple,pmgr binding

On 06/10/2021 08:56, Krzysztof Kozlowski wrote:
> On 05/10/2021 17:59, Hector Martin wrote:
>> The PMGR block in Apple Silicon SoCs is responsible for SoC power
>> management. There are two PMGRs in T8103, with different register
>> layouts but compatible registers. In order to support this as well
>> as future SoC generations with backwards-compatible registers, we
>> declare these blocks as syscons and bind to individual registers
>> in child nodes. Each register controls one SoC device.
>>
>> The respective apple compatibles are defined in case device-specific
>> quirks are necessary in the future, but currently these nodes are
>> expected to be bound by the generic syscon driver.
>>
>> Signed-off-by: Hector Martin <marcan@...can.st>
>> ---
>>  .../bindings/arm/apple/apple,pmgr.yaml        | 74 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 75 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml
>> new file mode 100644
>> index 000000000000..0304164e4140
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/apple/apple,pmgr.yaml
>> @@ -0,0 +1,74 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/arm/apple/apple,pmgr.yaml#
> 
> Please don't store all Apple-related bindings in bindings/arm/apple, but
> instead group per device type like in most of other bindings. In this
> case - this looks like something close to power domain controller, so it
> should be in bindings/power/
> 
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Apple SoC Power Manager (PMGR)
>> +
>> +maintainers:
>> +  - Hector Martin <marcan@...can.st>
>> +
>> +description: |
>> +  Apple SoCs include a PMGR block responsible for power management,
>> +  which can control various clocks, resets, power states, and
>> +  performance features. This node represents the PMGR as a syscon,
>> +  with sub-nodes representing individual features.
>> +
>> +  Apple SoCs may have a secondary "mini-PMGR"; it is represented
>> +  separately in the device tree, but works the same way.
>> +
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +          - apple,t8103-pmgr
>> +          - apple,t8103-minipmgr
>> +          - apple,pmgr
>> +
>> +  required:
>> +    - compatible
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^power-management@[0-9a-f]+$"
>> +
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - apple,t8103-pmgr
>> +          - apple,t8103-minipmgr
>> +      - const: apple,pmgr
>> +      - const: syscon
>> +      - const: simple-mfd
> 
> No power-domain-cells? Why? What exactly this device is going to do?
> Maybe I'll check the driver first.... :)
> 

After looking at the code, there is no device for
apple,t8103-pmgr/apple,pmgr. What is this binding about? Is there really
a central (central as in "one device for SoC") block managing power
which you want to model here?


Best regards,
Krzysztof