linux-kernel - Re: [PATCH v2 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp related property

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: <mnbc33u3ohncqmhyftag26sttqif24mjadnlg2btx7tc3dmowj@6bfh2lb7ekud>
Date: Wed, 20 Nov 2024 08:45:46 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Friday Yang <friday.yang@...iatek.com>
Cc: Yong Wu <yong.wu@...iatek.com>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Philipp Zabel <p.zabel@...gutronix.de>, 
	linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v2 1/2] dt-bindings: memory: mediatek: Add SMI reset and
 clamp related property

On Wed, Nov 20, 2024 at 02:36:38PM +0800, Friday Yang wrote:
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> index 2381660b324c..302c0f93b49d 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> @@ -69,6 +69,18 @@ properties:
>      description: the hardware id of this larb. It's only required when this
>        hardware id is not consecutive from its M4U point of view.
> 
> +  resets:
> +    maxItems: 1
> +    description: This contains a phandle to the reset controller node and an index
> +      to a reset signal. SMI larbs need to get the reset controller by the node.

First sentence is 100% redundant. Arguments depend on the reset-cells,
not this binding.

> +      SMI could get the reset signal by the index number defined in the header
> +      include/dt-bindings/reset/mt8188-resets.h.

What? How this can depend on consumer? Drop entire description, it is
useless.

> +
> +  reset-names:
> +    const: larb
> +    description: The name of reset controller. SMI driver need to obtain the
> +      reset controller based on this.

Drop description, useless.

> +
>  required:
>    - compatible
>    - reg
> @@ -125,19 +137,38 @@ allOf:
>        required:
>          - mediatek,larb-id
> 
> +  - if:  # only for camera and image subsys
> +      properties:
> +        mediatek,smi:
> +            contains:

Never tested.

> +              enum:
> +                - smi_sub_common_img0_4x1
> +                - smi_sub_common_img1_4x1
> +                - smi_sub_common_cam_5x1
> +                - smi_sub_common_cam_8x1

Does not work. Test your code before you send it. No clue what you want
to achieve, so not sure how to help.


> +
> +    then:
> +      required:
> +        - resets
> +        - reset-names
> +
>  additionalProperties: false
> 
>  examples:
>    - |+
> -    #include <dt-bindings/clock/mt8173-clk.h>
> -    #include <dt-bindings/power/mt8173-power.h>
> -
> -    larb1: larb@...10000 {
> -      compatible = "mediatek,mt8173-smi-larb";
> -      reg = <0x16010000 0x1000>;
> -      mediatek,smi = <&smi_common>;
> -      power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
> -      clocks = <&vdecsys CLK_VDEC_CKEN>,
> -               <&vdecsys CLK_VDEC_LARB_CKEN>;
> -      clock-names = "apb", "smi";
> +    #include <dt-bindings/clock/mediatek,mt8188-clk.h>
> +    #include <dt-bindings/power/mediatek,mt8188-power.h>
> +    #include <dt-bindings/reset/mt8188-resets.h>

This is some total mess. Never tested, not correct. Sorry, run it
internally through some sort of review or internal checklist which will
ask you to test the code before sending.

Best regards,
Krzysztof