Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
In-Reply-To: <1631223065-12607-1-git-send-email-khsieh@codeaurora.org>
References: <1631223065-12607-1-git-send-email-khsieh@codeaurora.org>
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.9.1
Date:   Mon, 13 Sep 2021 13:06:09 -0700
Message-ID: <CAE-0n50P2OR9CPku3qq7+xR56fpg2Y7JDinikkY+uX6y9XtcRA@mail.gmail.com>
Subject: Re: [PATCH v3] phy: qcom-qmp: add support for display port voltage
 and pre-emphasis swing
To:     Kuogee Hsieh <khsieh@codeaurora.org>, agross@kernel.org,
        bjorn.andersson@linaro.org, devicetree@vger.kernel.org,
        robdclark@gmail.com, robh+dt@kernel.org, sean@poorly.run,
        vkoul@kernel.org
Cc:     abhinavk@codeaurora.org, aravindh@codeaurora.org,
        freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org, kishon@ti.com, p.zabel@pengutronix.de
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Quoting Kuogee Hsieh (2021-09-09 14:31:05)
> @@ -3757,16 +3796,14 @@ static void qcom_qmp_v4_phy_dp_aux_init(struct qmp_phy *qphy)
>
>  static void qcom_qmp_v4_phy_configure_dp_tx(struct qmp_phy *qphy)
>  {
> -       /* Program default values before writing proper values */
> -       writel(0x27, qphy->tx + QSERDES_V4_TX_TX_DRV_LVL);
> -       writel(0x27, qphy->tx2 + QSERDES_V4_TX_TX_DRV_LVL);
> -
> -       writel(0x20, qphy->tx + QSERDES_V4_TX_TX_EMP_POST1_LVL);
> -       writel(0x20, qphy->tx2 + QSERDES_V4_TX_TX_EMP_POST1_LVL);
> -
> -       qcom_qmp_phy_configure_dp_swing(qphy,
> +       if (__qcom_qmp_phy_configure_dp_swing(qphy,
>                         QSERDES_V4_TX_TX_DRV_LVL,
> -                       QSERDES_V4_TX_TX_EMP_POST1_LVL);
> +                       QSERDES_V4_TX_TX_EMP_POST1_LVL,
> +                       (const u8 *)qmp_dp_v4_voltage_swing_hbr_rbr,

Surely we can pass a 2d-array to a function in C? We know the size of
the array is always a 4x4 so we could enforce that in the type so the
compiler complains if we try to pass something else.

> +                       (const u8 *)qmp_dp_v4_pre_emphasis_hbr_rbr,
> +                       (const u8 *)qmp_dp_v4_voltage_swing_hbr3_hbr2,
> +                       (const u8 *)qmp_dp_v4_pre_emphasis_hbr3_hbr2) < 0)
> +               return;

It's void and this is the end of the function. Why is there an if
condition and a return?

>  }
>
>  static int qcom_qmp_v4_phy_configure_dp_phy(struct qmp_phy *qphy)