From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B6049C433DB for ; Tue, 16 Mar 2021 01:03:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 69DF664F17 for ; Tue, 16 Mar 2021 01:03:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233173AbhCPBCj (ORCPT ); Mon, 15 Mar 2021 21:02:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232854AbhCPBCf (ORCPT ); Mon, 15 Mar 2021 21:02:35 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B35D2C06174A; Mon, 15 Mar 2021 18:02:33 -0700 (PDT) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C38E3E7; Tue, 16 Mar 2021 02:02:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1615856550; bh=CrLVb9xQ0lmni1InzJKHFcnQtZiczJWx4UWyxiY9eeI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Phc07QlObq06SmRWGl5GEcXHP5KqKZySRKb7AXbsviTtUoCZlPLiJK8h3xx5/XptN WBKVa/roW7OfUBjCuZnjMo1rycH+Sj2Rds5ZN4JiMUWkhFikfXA2J7bXMi7d6LbDRi RSzHP8dlQSvgF1fkEpSMUHEzMXEgU1sY9DlqI9MY= Date: Tue, 16 Mar 2021 03:01:53 +0200 From: Laurent Pinchart To: Biju Das Cc: "kieran.bingham+renesas@ideasonboard.com" , Mauro Carvalho Chehab , "linux-media@vger.kernel.org" , "linux-renesas-soc@vger.kernel.org" , Geert Uytterhoeven , Chris Paterson , Biju Das , Prabhakar Mahadev Lad , Hans Verkuil Subject: Re: [PATCH 2/2] media: v4l: vsp1: Fix uif null pointer access Message-ID: References: <20210301120828.6945-1-biju.das.jz@bp.renesas.com> <20210301120828.6945-3-biju.das.jz@bp.renesas.com> <133f8b63-3b84-c60a-725d-30b8d6df35dd@ideasonboard.com> <56c2d53e-8d1f-5b84-1480-5965ae9cbae2@ideasonboard.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Biju, On Mon, Mar 15, 2021 at 08:21:38AM +0000, Biju Das wrote: > On 15 March 2021 03:43, Laurent Pinchart wrote: > > On Wed, Mar 10, 2021 at 02:50:23PM +0000, Biju Das wrote: > >>> On 10/03/2021 13:56, Biju Das wrote: > >>>> Thanks for the feedback. > >>>>> Subject: Re: [PATCH 2/2] media: v4l: vsp1: Fix uif null pointer > >>>>> access > >>>>> > >>>>> Hi Biju, > >>>>> > >>>>> On 01/03/2021 12:08, Biju Das wrote: > >>>>>> RZ/G2L SoC has no UIF. This patch fixes null pointer access, > >>>>>> when UIF module is not used. > >>>>>> > >>>>>> Fixes: 5e824f989e6e8("media: v4l: vsp1: Integrate DISCOM in > >>>>>> display pipeline") > >>>>>> Signed-off-by: Biju Das > >>>>>> --- > >>>>>> drivers/media/platform/vsp1/vsp1_drm.c | 4 ++-- > >>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>>>>> > >>>>>> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > >>>>>> b/drivers/media/platform/vsp1/vsp1_drm.c > >>>>>> index f6d2f47a4058..06f74d410973 100644 > >>>>>> --- a/drivers/media/platform/vsp1/vsp1_drm.c > >>>>>> +++ b/drivers/media/platform/vsp1/vsp1_drm.c > >>>>>> @@ -462,9 +462,9 @@ static int > >>>>>> vsp1_du_pipeline_setup_inputs(struct > >>>>>> vsp1_device *vsp1, > >>>>> > >>>>> > >>>>> This looks like it complicates these conditionals more than we > >>>>> perhaps need to. > >>>>> > >>>>> What do you think about adding something above the block comment here?: > >>>> > >>>> It is much better. > >>>> > >>>> This patch is accepted in media tree[1]. So not sure, should I > >>>> send a follow up patch as optimization or drop this patch and send new one. > >>> > >>> Oh, I didn't realise these were in already. Sorry, I didn't see any > >>> review on the list, and it was the earliest I had got to them. > >>> > >>>> Please suggest. > >>> > >>> Up to you, I don't think this would get dropped now it's integrated. > >>> It's in, so if you want to update on top I believe that's fine. > >> > >> OK, Will send follow up patch as optimization. > > > > That would be nice. > > > > I don't think this patch should have been fast-tracked as a fix, as RZ/G2L > > isn't supported in mainline yet as far as I can tell. > > Please correct me, if I am wrong. We have pluggable modules in VSP and with routing > Register we can achieve any combination with the VSP driver we have. > > If it is the case, it is a bug which is fast-tracked as a fix. > Otherwise(ie, driver doesn't have flexibility to support pluggable > feature) I am agreeing with your statement. My point was that this code is currently used only on platforms that have a UIF, so there's should be no risk of this problem being triggered. It should certainly be fixed to support RZ/G2L, but that's not upstream yet. > > Hans, next time, could we get a notification instead of a silent > > merge ? > > > >>>>> if (!drm_pipe->uif) > >>>>> return 0; > >>>>> > >>>>>> * make sure it is present in the pipeline's list of entities if it > >>>>>> * wasn't already. > >>>>>> */ > >>>>>> - if (!use_uif) { > >>>>>> + if (drm_pipe->uif && !use_uif) { > >>>>>> drm_pipe->uif->pipe = NULL; > >>>>>> - } else if (!drm_pipe->uif->pipe) { > >>>>>> + } else if (drm_pipe->uif && !drm_pipe->uif->pipe) { > >>>>>> drm_pipe->uif->pipe = pipe; > >>>>>> list_add_tail(&drm_pipe->uif->list_pipe, &pipe->entities); > >>>>>> } -- Regards, Laurent Pinchart