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;
Date:   Tue, 25 Jul 2023 16:44:08 -0400
From:   =?utf-8?B?TsOtY29sYXMgRi4gUi4gQS4=?= Prado 
        <nfraprado@collabora.com>
To:     Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>, kernel@collabora.com,
        Andrew-CT Chen <andrew-ct.chen@mediatek.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Tiffany Lin <tiffany.lin@mediatek.com>,
        Yunfei Dong <yunfei.dong@mediatek.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org, linux-mediatek@lists.infradead.org,
        Matthias Brugger <matthias.bgg@gmail.com>
Subject: Re: [PATCH v5 5/7] media: mediatek: vcodec: Read HW active status
 from syscon
Message-ID: <b5ba26b8-7c03-4ead-97ff-8ee3ae0efa26@notapiano>
References: <20230630151436.155586-1-nfraprado@collabora.com>
 <20230630151436.155586-6-nfraprado@collabora.com>
 <b5fd2dff-14a5-3ad8-9698-d1a50f4516fa@xs4all.nl>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b5fd2dff-14a5-3ad8-9698-d1a50f4516fa@xs4all.nl>
Precedence: bulk

On Tue, Jul 25, 2023 at 12:15:03PM +0200, Hans Verkuil wrote:
> Hi Nicolas,
> 
> On 30/06/2023 17:14, N?colas F. R. A. Prado wrote:
> > Remove the requirement of a VDEC_SYS reg iospace for both MT8173 and
> > MT8183. To achieve that, rely on a vdecsys syscon to be passed through
> > the DT, and use it to directly read the VDEC_HW_ACTIVE bit during IRQ
> > handling to check whether the HW is active. Also update the VP8 stateful
> > decoder to use the syscon, if present, for writes to VDEC_SYS.
> > 
> > The old behavior is still present when reg-names aren't supplied, as
> > to keep backward compatibility.
> > 
> > Signed-off-by: N?colas F. R. A. Prado <nfraprado@collabora.com>
> > 
> > ---
> > 
> > Changes in v5:
> > - Added explicit linux/bitfield.h include for FIELD_GET(), following
> >   0day report
> > 
> > Changes in v4:
> > - Added new helper and updated VP8 stateful decoder to use it, so the
> >   syscon can also be used by mt8173
> > - Made handling cleaner
> > - Reworded commit
> > 
> > Changes in v3:
> > - Switched handling of VDEC_HW_ACTIVE to use a syscon instead of the
> >   'active' clock
> > - Reworded commit
> > - Removed changes to subdev part of driver, since they aren't used by
> >   MT8183
> > 
> >  .../mediatek/vcodec/mtk_vcodec_dec_drv.c      | 77 ++++++++++++++++---
> >  .../platform/mediatek/vcodec/mtk_vcodec_drv.h |  1 +
> >  .../mediatek/vcodec/mtk_vcodec_util.c         | 15 ++++
> >  .../mediatek/vcodec/mtk_vcodec_util.h         |  2 +
> >  .../mediatek/vcodec/vdec/vdec_vp8_if.c        | 10 +--
> >  5 files changed, 87 insertions(+), 18 deletions(-)
> 
> This patch introduced this new smatch error:
> 
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c:143 mtk_vcodec_get_reg_bases() error: buffer overflow 'mtk_dec_reg_names' 11 <= 11
> 
> I think it is due to:
> 
> if (reg_num <= 0 || reg_num > NUM_MAX_VDEC_REG_BASE) {
> 
> in mtk_vcodec_get_reg_bases(): the '>' should probably be '>='.
> 
> Can you post a follow-up patch fixing this?

Hi Hans,

sorry about that, and thanks for noticing it.

I've just sent the fix:
https://lore.kernel.org/all/20230725204043.569799-1-nfraprado@collabora.com

Thanks,
N?colas