From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/amdgpu/jpeg: deduplicate jpeg_v3_0 process_interrupt Date: Thu, 23 Apr 2026 07:56:59 +1000 Message-ID: In-Reply-To: <20260422003911.33841-2-tiagodourado@usp.br> References: <20260422003911.33841-1-tiagodourado@usp.br> <20260422003911.33841-2-tiagodourado@usp.br> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness:** The deleted `jpeg_v3_0_process_interrupt` is verified iden= tical to `jpeg_v2_0_process_interrupt` =E2=80=94 both use `VCN_2_0__SRCID__= JPEG_DECODE`, `DRM_ERROR`, and `adev->jpeg.inst->ring_dec`. The deduplicati= on is valid. **Style concern =E2=80=94 macro alias is unnecessary indirection:** ```c #define jpeg_v3_0_process_interrupt jpeg_v2_0_process_interrupt ``` This macro exists solely so the struct initializer can still say `.process = =3D jpeg_v3_0_process_interrupt`. This is over-engineering =E2=80=94 it hid= es the actual function being called and makes code search harder (grepping = for the definition of `jpeg_v3_0_process_interrupt` now lands on a macro, n= ot a function). It would be simpler and more readable to just use the funct= ion directly in the struct initializer: ```c static const struct amdgpu_irq_src_funcs jpeg_v3_0_irq_funcs =3D { .set =3D jpeg_v3_0_set_interrupt_state, .process =3D jpeg_v2_0_process_interrupt, }; ``` This is the standard kernel pattern for reusing a function from another ver= sion =E2=80=94 no macro needed. **Tag ordering issue:** The Signed-off-by / Co-developed-by order is wrong = per `Documentation/process/submitting-patches.rst`. Currently: ``` Signed-off-by: Tiago Dourado Co-developed-by: Luiz Fernandes Signed-off-by: Luiz Fernandes ``` The submitter's `Signed-off-by` must be **last**, and every `Co-developed-b= y` must be immediately followed by its matching `Signed-off-by`. The correc= t order is: ``` Co-developed-by: Luiz Fernandes Signed-off-by: Luiz Fernandes Signed-off-by: Tiago Dourado ``` --- --- Generated by Claude Code Patch Reviewer