Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Dead code in `cec_enamods()` error check allows overwriting the register configuration when `cec_read()` fails. -- --- Patch [1]: [PATCH] drm/bridge: tda998x: Handle short CEC register reads --- commit 2bfd5ce7155e5841764b8ec506e16365cd7190b6 Author: Ruoyu Wang <[email protected]> drm/bridge: tda998x: Handle short CEC register reads This commit updates the error handling in cec_read() to ensure both messages of the I2C transfer complete successfully before using the returned byte. If the transfer is short or fails, it preserves the 0 fallback value to avoid returning uninitialized stack memory. > diff --git a/drivers/gpu/drm/bridge/tda998x_drv.c > b/drivers/gpu/drm/bridge/tda998x_drv.c > index 0b966c9844e70..fb414584962b7 100644 > --- a/drivers/gpu/drm/bridge/tda998x_drv.c > +++ b/drivers/gpu/drm/bridge/tda998x_drv.c > @@ -438,7 +438,7 @@ cec_read(struct tda998x_priv *priv, u8 addr) > int ret; > > ret = i2c_transfer(priv->hdmi->adapter, msg, ARRAY_SIZE(msg)); > - if (ret < 0) { > + if (ret != ARRAY_SIZE(msg)) { > dev_err(&priv->hdmi->dev, "Error %d reading from cec:0x%x\n", > ret, addr); > val = 0; > } > > return val; [Severity: High] This is a pre-existing issue, but since cec_read() returns a u8, does the error handling in callers like cec_enamods() work correctly? For example, in drivers/gpu/drm/bridge/tda998x_drv.c:cec_enamods(): int val = cec_read(priv, REG_CEC_ENAMODS); if (val < 0) return; if (enable) val |= mods; else val &= ~mods; cec_write(priv, REG_CEC_ENAMODS, val); Because cec_read() returns an unsigned 8-bit integer, val will be zero-extended and can never be negative. If a read fails due to an I2C error, it returns 0, which bypasses the val < 0 check. Could this result in writing back a modified 0 to REG_CEC_ENAMODS and wiping out other enable bits, potentially causing the CEC module or clocks to be disabled unexpectedly until a full reset occurs? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
