Claude review: drm/bridge: ite-it6263: Move chip initialization code from probe to atomic_enable

[Claude Code Review Bot <claude-review@example.com>]

Overall Series Review

Subject: drm/bridge: ite-it6263: Move chip initialization code from probe to atomic_enable
Author: Biju <biju.das.au@gmail.com>
Patches: 1
Reviewed: 2026-05-05T09:39:38.356348

---

This is a single-patch series that moves IT6263 bridge chip initialization (hardware reset, I2C address configuration, LVDS config, HDMI config) from `probe()` to `atomic_enable()`. The motivation is sound: on the RZ/G3L SMARC EVK, suspend-to-RAM powers down the IT6263, and the display controller's PM callbacks invoke `atomic_disable`/`atomic_enable`, so the chip must be fully reinitialized on each enable cycle.

The approach is reasonable and follows the pattern used by other bridge drivers that need to survive power loss. The patch is small and well-scoped. However, there are a few concerns worth raising:

**Concern 1 — Error handling regression in `atomic_enable`.** In `probe()`, the original `it6263_lvds_set_i2c_addr()` failure was fatal and returned an error via `dev_err_probe()`. In `atomic_enable()`, the failure is downgraded to a `dev_err()` log and execution continues. If the I2C address setup fails, subsequent writes to the LVDS regmap will go to the wrong address, likely causing silent corruption or a hang. The `atomic_enable` callback is void-returning so a direct return isn't possible, but the function should at least bail out early rather than proceeding with configuration that depends on the I2C address being set.

**Concern 2 — `it6263_detect()` / EDID reads before `atomic_enable`.** The `it6263_detect()` and `it6263_read_edid()` functions access the HDMI regmap to read HPD status and EDID. After this patch, the chip is no longer initialized in `probe()`, so these functions may be called against an uninitialized chip (e.g., during connector probing at boot). The `devm_regulator_bulk_get_enable` still powers the chip in probe, and the GPIO is requested as `GPIOD_OUT_LOW` (de-asserted reset on active-low), so the chip *may* come up in a usable-enough state for I2C register reads. But the HDMI config registers (e.g., `HDMI_REG_DDC_MASTER_CTRL`) are never written until `atomic_enable` now. This deserves a closer look at whether detect/EDID work reliably without `it6263_hdmi_config()` being called first.

**Concern 3 — No `atomic_disable` counterpart.** The `atomic_disable` only mutes audio/video and powers down the AFE driver. It does not assert the reset GPIO. This means on a normal disable/enable cycle (not involving a full power loss), the chip gets a gratuitous hardware reset pulse followed by full reinitialization. This should be harmless but adds ~46ms of latency to every enable.

---

---
Generated by Claude Code Patch Reviewer