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

[Krzysztof Kozlowski]

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@xxxxxxxxx>
>> ---
>>  .../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@xxxxxxxxx>
>> +
>> +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