Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220612144854.2223873-1-alvin@pqrs.dk> <20220612144854.2223873-2-alvin@pqrs.dk>
In-Reply-To: <20220612144854.2223873-2-alvin@pqrs.dk>
From:   Robert Foss <robert.foss@linaro.org>
Date:   Thu, 7 Jul 2022 12:53:05 +0200
Message-ID: <CAG3jFysYKJc3Ar99HsoSri+wxW2=KOdSwwsU9qBpypxrNCMaiQ@mail.gmail.com>
Subject: Re: [PATCH RESEND 1/2] drm: bridge: adv7511: fix CEC power down
 control register offset
To:     alvin@pqrs.dk
Cc:     Andrzej Hajda <andrzej.hajda@intel.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Jonas Karlman <jonas@kwiboo.se>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Hans Verkuil <hans.verkuil@cisco.com>,
        Archit Taneja <architt@codeaurora.org>,
        =?UTF-8?Q?Alvin_=C5=A0ipraga?= <alsi@bang-olufsen.dk>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, 12 Jun 2022 at 16:49, Alvin =C5=A0ipraga <alvin@pqrs.dk> wrote:
>
> From: Alvin =C5=A0ipraga <alsi@bang-olufsen.dk>
>
> The ADV7511_REG_CEC_CTRL =3D 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 =C5=A0ipraga <alsi@bang-olufsen.dk>
> ---
>  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/b=
ridge/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 *a=
dv7511)
>  {
> -       unsigned int offset =3D adv7511->type =3D=3D 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/d=
rm/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 adv75=
11 *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 adv75=
11 *adv7511)
>         dev_info(dev, "Initializing CEC failed with error %d, disabling C=
EC\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 =3D=3D -EPROBE_DEFER ? ret : 0;
>  }
> --
> 2.36.1
>

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