Re: [PATCH RESEND 1/2] drm: bridge: adv7511: fix CEC power down control register offset

From: Robert Foss
Date: Thu Jul 07 2022 - 06:53:23 EST


On Sun, 12 Jun 2022 at 16:49, Alvin Šipraga <alvin@xxxxxxx> wrote:
>
> From: Alvin Šipraga <alsi@xxxxxxxxxxxxxxx>
>
> The ADV7511_REG_CEC_CTRL = 0xE2 register is part of the main register
> map - not the CEC register map. As such, we shouldn't apply an offset to
> the register address. Doing so will cause us to address a bogus register
> for chips with a CEC register map offset (e.g. ADV7533).
>
> Fixes: 3b1b975003e4 ("drm: adv7511/33: add HDMI CEC support")
> Signed-off-by: Alvin Šipraga <alsi@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 5 +----
> drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 4 ++--
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> index 9e3bb8a8ee40..46ae6c0d9aa5 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -406,10 +406,7 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
> #else
> static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
> {
> - unsigned int offset = adv7511->type == ADV7533 ?
> - ADV7533_REG_CEC_OFFSET : 0;
> -
> - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
> ADV7511_CEC_CTRL_POWER_DOWN);
> return 0;
> }
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> index 399f625a50c8..6d067fa54bea 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> @@ -347,7 +347,7 @@ int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
> goto err_cec_alloc;
> }
>
> - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, 0);
> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, 0);
> /* cec soft reset */
> regmap_write(adv7511->regmap_cec,
> ADV7511_REG_CEC_SOFT_RESET + offset, 0x01);
> @@ -374,7 +374,7 @@ int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
> dev_info(dev, "Initializing CEC failed with error %d, disabling CEC\n",
> ret);
> err_cec_parse_dt:
> - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset,
> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
> ADV7511_CEC_CTRL_POWER_DOWN);
> return ret == -EPROBE_DEFER ? ret : 0;
> }
> --
> 2.36.1
>

Reviewed-by: Robert Foss <robert.foss@xxxxxxxxxx>