linux-kernel - Re: [PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for MediaTek SCPSYS

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: <46df4ad5-5102-b5fe-95b7-5b157fb28f01@linaro.org>
Date:   Fri, 15 Jul 2022 09:57:14 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Tinghan Shen <tinghan.shen@...iatek.com>,
        Yong Wu <yong.wu@...iatek.com>, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Lee Jones <lee.jones@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Chun-Jie Chen <chun-jie.chen@...iatek.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        MandyJH Liu <mandyjh.liu@...iatek.com>,
        Weiyi Lu <weiyi.lu@...iatek.com>
Cc:     iommu@...ts.linux.dev, linux-mediatek@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v2 03/19] dt-bindings: power: mediatek: Add bindings for
 MediaTek SCPSYS

On 14/07/2022 14:28, Tinghan Shen wrote:
> The System Control Processor System (SCPSYS) has several power
> management related tasks in the system. Add the bindings for it.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@...iatek.com>
> ---
>  .../bindings/mfd/mediatek,scpsys.yaml         | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> new file mode 100644
> index 000000000000..a8b9220f2f27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,scpsys.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/mediatek,scpsys.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek System Control Processor System
> +
> +maintainers:
> +  - MandyJH Liu <mandyjh.liu@...iatek.com>
> +
> +description:
> +  MediaTek System Control Processor System (SCPSYS) has several
> +  power management tasks. The tasks include MTCMOS power
> +  domain control, thermal measurement, DVFS, etc.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: mediatek,scpsys
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-controller:
> +    $ref: /schemas/power/mediatek,power-controller.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +
> +    syscon@...06000 {
> +        compatible = "mediatek,scpsys", "syscon", "simple-mfd";

This should be a SoC-specific compatible (and filename).

> +        reg = <0x10006000 0x100>;
> +
> +        spm: power-controller {

I think you created before less-portable, quite constrained bindings for
power controller. You now require that mt8195-power-controller is always
a child of some parent device which will share its regmap/MMIO with it.

And what if in your next block there is no scpsys block and power
controller is the scpsys alone? It's not possible with your bindings.

Wouldn't it be better to assign some address space to the
power-controller (now as an offset from scpsys)?

This is just wondering (Rockchip did the same...) and not a blocker as
power-controller bindings are done.

Best regards,
Krzysztof