From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Re: [PATCH v2 2/4] dt-bindings: npu: rockchip,rk3588-rknn-core: Add RK3568 support Date: Thu, 04 Jun 2026 16:18:12 +1000 Message-ID: In-Reply-To: <53c4249e-8d6c-4bf7-86ff-b8d02117338a@kernel.org> References: <26069b3b-43c0-45fa-92ee-729d270a8b6b@kernel.org> <53c4249e-8d6c-4bf7-86ff-b8d02117338a@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **1. Missing `$ref` key =E2=80=94 YAML syntax error (critical)** ```yaml rockchip,pmu: : /schemas/types.yaml#/definitions/phandle ``` The second line is `: /schemas/types.yaml#/definitions/phandle` with no key= . This should be: ```yaml rockchip,pmu: $ref: /schemas/types.yaml#/definitions/phandle ``` The `$ref` key appears to have been stripped (possibly by the email toolcha= in mangling `$`). This will cause `dt_binding_check` to fail. This is a mus= t-fix. **2. `rockchip,pmu` not conditionally required for RK3568** The commit message says the phandle is "Required on RK3568" and the driver = will fail at probe without it (`syscon_regmap_lookup_by_phandle` returns an= error). But the binding doesn't add a conditional `required` block: ```yaml if: properties: compatible: contains: const: rockchip,rk3568-rknn-core then: required: - rockchip,pmu ``` Without this, `dtbs_check` won't catch a missing `rockchip,pmu` on RK3568 b= oards. **3. The if/then for `sram-supply` looks correct** Making `sram-supply` conditional on RK3588-only is the right approach. --- --- Generated by Claude Code Patch Reviewer