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: freescale: Add DisplayPort/HDMI Combo-PHY driver for i.MX8MQ Date: Sun, 12 Apr 2026 13:34:46 +1000 Message-ID: In-Reply-To: <20260407-dcss-hdmi-upstreaming-v21-6-4681070ab82f@oss.nxp.com> References: <20260407-dcss-hdmi-upstreaming-v21-0-4681070ab82f@oss.nxp.com> <20260407-dcss-hdmi-upstreaming-v21-6-4681070ab82f@oss.nxp.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 **Resource mapping should use `devm_platform_ioremap_resource()`:** ```c res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) return -ENODEV; cdns_phy->regs =3D devm_ioremap(dev, res->start, resource_size(res)); ``` This is the old pattern. Modern drivers use the simpler `devm_platform_iore= map_resource(pdev, 0)` which handles the NULL check and does `devm_ioremap_= resource` (which also does request_region). The same issue exists in the br= idge probe in patch 4. **`hdptx_clk_enable` error handling:** ```c ret =3D hdptx_clk_enable(cdns_phy); if (ret) return -EINVAL; ``` This discards the actual error code from `hdptx_clk_enable`. Should be `ret= urn ret;`. **Missing `phy_exit()` on error and remove paths**: `phy_init()` is not cal= led in the PHY driver itself (that's the consumer's responsibility), so thi= s is fine, but the clocks enabled in probe are never disabled =E2=80=94 `de= vm_clk_get_enabled` handles this via devm, so it's correct. **Large PLL configuration tables**: The 32-entry `pixel_clk_output_ctrl_tab= le` and 15-entry `pixel_clk_output_pll_table` are quite large. Consider whe= ther they belong in a separate data file, but this is a style preference. --- Generated by Claude Code Patch Reviewer