From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: phy: qualcomm: add MSM8974 HDMI PHY support Date: Wed, 25 Mar 2026 07:14:42 +1000 Message-ID: In-Reply-To: <20260324-fd-hdmi-phy-v7-4-b41dde8d83b8@oss.qualcomm.com> References: <20260324-fd-hdmi-phy-v7-0-b41dde8d83b8@oss.qualcomm.com> <20260324-fd-hdmi-phy-v7-4-b41dde8d83b8@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review Adds PLL rate calculation and configuration for the MSM8974 HDMI PHY, build= ing on the preqmp framework from Patch 1 and the UniPHY registers from Patc= hes 2-3. **Bug: Pixel clock comparison values are in wrong units** In `qcom_hdmi_msm8974_phy_pll_set_rate()`: ```c if (pixclk =3D=3D 297000) { ... } else if (pixclk =3D=3D 268500) { ... ``` `pixclk` is `hdmi_phy->hdmi_opts.tmds_char_rate` which is in Hz (`unsigned = long long`). These comparisons against 297000 and 268500 will never be true= =E2=80=94 the values should be `297000000` (297 MHz) and `268500000` (268.= 5 MHz), or the code should use `HZ_PER_KHZ` multiplier: ```c if (pixclk =3D=3D 297 * HZ_PER_MHZ) { ``` This also applies to the second comparison at line 7074: ```c if (pixclk =3D=3D 268500) ``` **Minor: Duplicate helpers** `write16()`, `write24()`, `read24()` are defined again here =E2=80=94 same = as noted in Patch 1. In this file they could share with the `phy-qcom-hdmi-= 28lpm.c` definitions via a common header. **Minor: Unused `div` variable** In `qcom_28lpm_set_rate()`: ```c unsigned int div; ... dev_dbg(hdmi_phy->dev, "rate=3D%u, div =3D %d, vco =3D %lu", pixclk, div, = vco_freq); ``` `div` is declared and used in the debug print but never assigned a value in= this function. It will contain an indeterminate value. --- Generated by Claude Code Patch Reviewer