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@16010000 {
> - 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
