Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220319151016.983348-1-alvin@pqrs.dk> <20220319151016.983348-2-alvin@pqrs.dk>
In-Reply-To: <20220319151016.983348-2-alvin@pqrs.dk>
From:   Robert Foss <robert.foss@linaro.org>
Date:   Fri, 22 Apr 2022 16:25:22 +0200
Message-ID: <CAG3jFyuZuf-yoy42iSbsdaxVXQvV69-XVyf-6bT-7LDpRpQhjQ@mail.gmail.com>
Subject: Re: [PATCH 1/2] drm: bridge: adv7511: enable CEC support for ADV7535
To:     =?UTF-8?Q?Alvin_=C5=A0ipraga?= <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>,
        =?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 Sat, 19 Mar 2022 at 16:10, Alvin =C5=A0ipraga <alvin@pqrs.dk> wrote:
>
> From: Alvin =C5=A0ipraga <alsi@bang-olufsen.dk>
>
> Like the ADV7533, the ADV7535 has an offset for the CEC register map,
> and it is the same value (ADV7533_REG_CEC_OFFSET =3D 0x70).
>
> Rather than testing for numerous chip types in the offset calculations
> throughout the driver, just compute it during driver probe and put it in
> the private adv7511 data structure.
>
> Signed-off-by: Alvin =C5=A0ipraga <alsi@bang-olufsen.dk>
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  1 +
>  drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 18 ++++++------------
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  5 +++--
>  3 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/b=
ridge/adv7511/adv7511.h
> index 6a882891d91c..da6d8ee2cd84 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -335,6 +335,7 @@ struct adv7511 {
>
>         struct regmap *regmap;
>         struct regmap *regmap_cec;
> +       unsigned int reg_cec_offset;
>         enum drm_connector_status status;
>         bool powered;
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c b/drivers/gpu/d=
rm/bridge/adv7511/adv7511_cec.c
> index 28d9becc939c..1f619389e201 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
> @@ -21,8 +21,7 @@
>
>  static void adv_cec_tx_raw_status(struct adv7511 *adv7511, u8 tx_raw_sta=
tus)
>  {
> -       unsigned int offset =3D adv7511->type =3D=3D ADV7533 ?
> -                                       ADV7533_REG_CEC_OFFSET : 0;
> +       unsigned int offset =3D adv7511->reg_cec_offset;
>         unsigned int val;
>
>         if (regmap_read(adv7511->regmap_cec,
> @@ -73,8 +72,7 @@ static void adv_cec_tx_raw_status(struct adv7511 *adv75=
11, u8 tx_raw_status)
>
>  void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
>  {
> -       unsigned int offset =3D adv7511->type =3D=3D ADV7533 ?
> -                                       ADV7533_REG_CEC_OFFSET : 0;
> +       unsigned int offset =3D adv7511->reg_cec_offset;
>         const u32 irq_tx_mask =3D ADV7511_INT1_CEC_TX_READY |
>                                 ADV7511_INT1_CEC_TX_ARBIT_LOST |
>                                 ADV7511_INT1_CEC_TX_RETRY_TIMEOUT;
> @@ -118,8 +116,7 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511,=
 unsigned int irq1)
>  static int adv7511_cec_adap_enable(struct cec_adapter *adap, bool enable=
)
>  {
>         struct adv7511 *adv7511 =3D cec_get_drvdata(adap);
> -       unsigned int offset =3D adv7511->type =3D=3D ADV7533 ?
> -                                       ADV7533_REG_CEC_OFFSET : 0;
> +       unsigned int offset =3D adv7511->reg_cec_offset;
>
>         if (adv7511->i2c_cec =3D=3D NULL)
>                 return -EIO;
> @@ -165,8 +162,7 @@ static int adv7511_cec_adap_enable(struct cec_adapter=
 *adap, bool enable)
>  static int adv7511_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
>  {
>         struct adv7511 *adv7511 =3D cec_get_drvdata(adap);
> -       unsigned int offset =3D adv7511->type =3D=3D ADV7533 ?
> -                                       ADV7533_REG_CEC_OFFSET : 0;
> +       unsigned int offset =3D adv7511->reg_cec_offset;
>         unsigned int i, free_idx =3D ADV7511_MAX_ADDRS;
>
>         if (!adv7511->cec_enabled_adap)
> @@ -235,8 +231,7 @@ static int adv7511_cec_adap_transmit(struct cec_adapt=
er *adap, u8 attempts,
>                                      u32 signal_free_time, struct cec_msg=
 *msg)
>  {
>         struct adv7511 *adv7511 =3D cec_get_drvdata(adap);
> -       unsigned int offset =3D adv7511->type =3D=3D ADV7533 ?
> -                                       ADV7533_REG_CEC_OFFSET : 0;
> +       unsigned int offset =3D adv7511->reg_cec_offset;
>         u8 len =3D msg->len;
>         unsigned int i;
>
> @@ -289,8 +284,7 @@ static int adv7511_cec_parse_dt(struct device *dev, s=
truct adv7511 *adv7511)
>
>  int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
>  {
> -       unsigned int offset =3D adv7511->type =3D=3D ADV7533 ?
> -                                               ADV7533_REG_CEC_OFFSET : =
0;
> +       unsigned int offset =3D adv7511->reg_cec_offset;
>         int ret =3D adv7511_cec_parse_dt(dev, adv7511);
>
>         if (ret)
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/d=
rm/bridge/adv7511/adv7511_drv.c
> index 005bf18682ff..0be65a1ffc47 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1027,8 +1027,7 @@ static bool adv7511_cec_register_volatile(struct de=
vice *dev, unsigned int reg)
>         struct i2c_client *i2c =3D to_i2c_client(dev);
>         struct adv7511 *adv7511 =3D i2c_get_clientdata(i2c);
>
> -       if (adv7511->type =3D=3D ADV7533 || adv7511->type =3D=3D ADV7535)
> -               reg -=3D ADV7533_REG_CEC_OFFSET;
> +       reg -=3D adv7511->reg_cec_offset;
>
>         switch (reg) {
>         case ADV7511_REG_CEC_RX_FRAME_HDR:
> @@ -1073,6 +1072,8 @@ static int adv7511_init_cec_regmap(struct adv7511 *=
adv)
>                 ret =3D adv7533_patch_cec_registers(adv);
>                 if (ret)
>                         goto err;
> +
> +               adv->reg_cec_offset =3D ADV7533_REG_CEC_OFFSET;
>         }
>
>         return 0;

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