Re: [PATCH v2] drm/mediatek: dp: Support flexible length of DP calibration data

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

Hi, Fei:

On Wed, 2024-12-04 at 22:25 +0800, Fei Shao wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The DP calibration data is stored in nvmem cells, and the data layout is
> described in the `mtk_dp_efuse_fmt` arrays for each platform.
> 
> There is no guarantee that the data is always a 4-length u32 cell array.
> For example, MT8188 has a data length of 3, preventing it from passing
> the preliminary check and undergoing calibration.
> 
> Update the logic to support flexible data lengths. Specifically, we
> validate the length returned from `nvmem_cell_read()` against the
> platform-specific efuse format. If out-of-bound access is detected, fall
> back to the default calibration values. This likely indicates an error
> in either the efuse data length described in DT or the efuse format
> within the driver.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
> 
> Changes in v2:
> - use %zu identifier for size_t in dev_warn()
> 
>  drivers/gpu/drm/mediatek/mtk_dp.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 36713c176cfc..55671701459a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1165,17 +1165,25 @@ static void mtk_dp_get_calibration_data(struct mtk_dp *mtk_dp)
>         buf = (u32 *)nvmem_cell_read(cell, &len);
>         nvmem_cell_put(cell);
> 
> -       if (IS_ERR(buf) || ((len / sizeof(u32)) != 4)) {
> +       if (IS_ERR(buf)) {
>                 dev_warn(dev, "Failed to read nvmem_cell_read\n");
> -
> -               if (!IS_ERR(buf))
> -                       kfree(buf);
> -
>                 goto use_default_val;
>         }
> 
> +       /* The cell length is in bytes. Convert it to be compatible with u32 buffer. */
> +       len /= sizeof(u32);
> +
>         for (i = 0; i < MTK_DP_CAL_MAX; i++) {
>                 fmt = &mtk_dp->data->efuse_fmt[i];
> +
> +               if (fmt->idx >= len) {
> +                       dev_warn(mtk_dp->dev,
> +                                "Out-of-bound efuse data access, fmt idx = %d, buf len = %zu\n",
> +                                fmt->idx, len);
> +                       kfree(buf);
> +                       goto use_default_val;
> +               }
> +
>                 cal_data[i] = (buf[fmt->idx] >> fmt->shift) & fmt->mask;
> 
>                 if (cal_data[i] < fmt->min_val || cal_data[i] > fmt->max_val) {
> --
> 2.47.0.338.g60cca15819-goog
>