it66121_probe() calls regmap_read() four times to read the vendor and device ID registers, without error checking. This leads to silently continuing using invalid ID values when any of the reads fail (which has happened and is being fixed [0]).
Adding error checking on all 4 regmap_read()s would be annoying, so switch to regmap_bulk_read(). In turn a bulk read requires a differen data structure, so take that as an opportunity to make the vendor/device ID computation more explicit. [0] https://lore.kernel.org/r/[email protected] Signed-off-by: Luca Ceresoli <[email protected]> --- drivers/gpu/drm/bridge/ite-it66121.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c index 692a85e1e2e8..9b43456c67a1 100644 --- a/drivers/gpu/drm/bridge/ite-it66121.c +++ b/drivers/gpu/drm/bridge/ite-it66121.c @@ -1580,7 +1580,16 @@ static const struct it66121_chip_info it66xx_chip_info[] = { static int it66121_probe(struct i2c_client *client) { - u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 }; + union { + __u8 byte[4]; + struct { + __u16 vendor; + __u16 device; + }; + } chip_id_regs; + BUILD_BUG_ON(sizeof(chip_id_regs) != 4); + u16 vendor_id, device_id; + u32 revision_id; struct device_node *ep; int ret, i; struct it66121_ctx *ctx; @@ -1644,19 +1653,22 @@ static int it66121_probe(struct i2c_client *client) if (IS_ERR(ctx->regmap)) return PTR_ERR(ctx->regmap); - regmap_read(ctx->regmap, IT66121_VENDOR_ID0_REG, &vendor_ids[0]); - regmap_read(ctx->regmap, IT66121_VENDOR_ID1_REG, &vendor_ids[1]); - regmap_read(ctx->regmap, IT66121_DEVICE_ID0_REG, &device_ids[0]); - regmap_read(ctx->regmap, IT66121_DEVICE_ID1_REG, &device_ids[1]); + ret = regmap_bulk_read(ctx->regmap, IT66121_VENDOR_ID0_REG, + &chip_id_regs, sizeof(chip_id_regs)); + if (ret) + return dev_err_probe(dev, ret, "Error reading device IDs\n"); /* Revision is shared with DEVICE_ID1 */ - revision_id = FIELD_GET(IT66121_REVISION_MASK, device_ids[1]); - device_ids[1] &= IT66121_DEVICE_ID1_MASK; + revision_id = FIELD_GET(IT66121_REVISION_MASK, chip_id_regs.byte[3]); + chip_id_regs.byte[3] &= IT66121_DEVICE_ID1_MASK; + + vendor_id = le16_to_cpu(chip_id_regs.vendor); + device_id = le16_to_cpu(chip_id_regs.device); for (i = 0; i < ARRAY_SIZE(it66xx_chip_info); i++) { chip_info = &it66xx_chip_info[i]; - if ((vendor_ids[1] << 8 | vendor_ids[0]) == chip_info->vid && - (device_ids[1] << 8 | device_ids[0]) == chip_info->pid) { + if (vendor_id == chip_info->vid && + device_id == chip_info->pid) { ctx->id = chip_info->id; break; } -- 2.56.0
