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=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 80A7DC433DF for ; Tue, 7 Jul 2020 17:56:39 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 48562206E2 for ; Tue, 7 Jul 2020 17:56:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="v95lHakv"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="D7QOnmbu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 48562206E2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=hKELkpXJZCVLnzHShuJf7TVaXHaQxZRMsZXxnJJAKM8=; b=v95lHakvrCgNak8TnKseNbzEh kyDZpfLRh5c072yU5Cr9TYLNonEgnFkXpbq18KXFDTrmwAd07dIDo4mTjiBOgsirVxIMd5xihJdVb 5uHGPfAIo/P43CoOSZj/BMOQmWly6VH3Be6XpFeRxfgQcq6YErdRlFplcfFKqIsl4BZhNotg5l9P6 XCiIMcIXuVV2qrDKPd17S/sUXWJaAOlOuAkyUL7b1Scooq7vFKgARV/1KtVb4ttNWhkNw5v+7jk1b y6Yzr/pEjpEq4MWKFZLY9vioicCp5aylJZV5pjTc8eaWhKMGdGOk/HiPwUfYst0N/YuTPPXD/Z6fV HHYzAHy4g==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsroR-0004IW-3s; Tue, 07 Jul 2020 17:55:23 +0000 Received: from mail-wm1-x344.google.com ([2a00:1450:4864:20::344]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsroO-0004HX-Qe for linux-arm-kernel@lists.infradead.org; Tue, 07 Jul 2020 17:55:21 +0000 Received: by mail-wm1-x344.google.com with SMTP id o8so44264530wmh.4 for ; Tue, 07 Jul 2020 10:55:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Sc9g6wGOzRwu+DnLXFZTX5cameYasawGNk3OVO7epl8=; b=D7QOnmbuxoxljwvQGT3yaEZJaRFUBDlp64fFWQE1ERpsuzJiBMUztTHlfqlUQ9so2Z H7kFoFEJ3xOi1f0Gb89ost5aAWVhD+vA8aUCzwj2K4+uwU3UHrSHDAkKsLiHN8KfECz7 jyc+yQUOvIi3JJY9mZvnaj8Mfqu4S6MDdLTFY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Sc9g6wGOzRwu+DnLXFZTX5cameYasawGNk3OVO7epl8=; b=GdG0knyiLCQpO6DnmHVvYvUeDlIN3EnuWbccYC5GCK1IVGyVlOk7WQhvMSo4kR5kIa U0mPM8Re70lWr4GIMsmvXFYOilZKskBQ5LdZyr3rI+2/3L2wX9L5oZnnw2e83A+QK1zE aL2mqQSUHvQmbiEdrpgLKvgTw2kEGFiArQhCDvS8AUFGF3NiRAkNsWQzxMiUB76Ek5Y4 00PlM6AyctS5eUXYfjQ+UrWvNSz5RZ121HrQtq9EZg20s92BkAPUiXmQu34swNSkg6vF w0R3gnvA/Rw3VBaSh2ztEd4HYaDYJHY/fya9APZVtqFzUsvDWx4MRs9xKNVg1ThlrxSg 7yBA== X-Gm-Message-State: AOAM530c6westneon7eiqaFqRaMrfj1qRQGuPpP9IsIPUzxVNN2vAxAQ u883EP1w392LPudyq6LvMUiGng== X-Google-Smtp-Source: ABdhPJz88/m48rj8/5SglwbXt+8ksYS3fKh8+pilMr/CD6s/J2Lz8aV+iwPGIQEZ51STz91WaC41TA== X-Received: by 2002:a7b:cb92:: with SMTP id m18mr3606166wmi.94.1594144519752; Tue, 07 Jul 2020 10:55:19 -0700 (PDT) Received: from chromium.org (205.215.190.35.bc.googleusercontent.com. [35.190.215.205]) by smtp.gmail.com with ESMTPSA id f16sm1980411wmf.17.2020.07.07.10.55.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Jul 2020 10:55:19 -0700 (PDT) Date: Tue, 7 Jul 2020 17:55:17 +0000 From: Tomasz Figa To: Jonathan Bakker Subject: Re: [PATCH 03/11] media: exynos4-is: Fix nullptr when no CSIS device present Message-ID: <20200707175517.GC2621465@chromium.org> References: <20200426022650.10355-1-xc-racer2@live.ca> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200707_135520_917213_73E452AC X-CRM114-Status: GOOD ( 25.23 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, krzk@kernel.org, kyungmin.park@samsung.com, kgene@kernel.org, s.nawrocki@samsung.com, mchehab@kernel.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Jonathan, On Sat, Apr 25, 2020 at 07:26:42PM -0700, Jonathan Bakker wrote: > Not all devices use the CSIS device, some may use the FIMC directly in > which case the CSIS device isn't registered. This leads to a nullptr > exception when starting the stream as the CSIS device is always > referenced. Instead, if getting the CSIS device fails, try getting the > FIMC directly to check if we are using the subdev API > > Signed-off-by: Jonathan Bakker > --- > drivers/media/platform/exynos4-is/media-dev.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > Thank you for the patch. Please see my comments inline. > diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c > index 9aaf3b8060d5..5c32abc7251b 100644 > --- a/drivers/media/platform/exynos4-is/media-dev.c > +++ b/drivers/media/platform/exynos4-is/media-dev.c > @@ -289,11 +289,26 @@ static int __fimc_pipeline_s_stream(struct exynos_media_pipeline *ep, bool on) > { IDX_CSIS, IDX_FLITE, IDX_FIMC, IDX_SENSOR, IDX_IS_ISP }, > }; > struct fimc_pipeline *p = to_fimc_pipeline(ep); > - struct fimc_md *fmd = entity_to_fimc_mdev(&p->subdevs[IDX_CSIS]->entity); > enum fimc_subdev_index sd_id; > int i, ret = 0; > > if (p->subdevs[IDX_SENSOR] == NULL) { > + struct fimc_md *fmd; > + struct v4l2_subdev *sd = p->subdevs[IDX_CSIS]; > + > + if (!sd) > + sd = p->subdevs[IDX_FIMC]; > + > + if (!sd) { > + /* > + * If neither CSIS nor FIMC was set up, > + * it's impossible to have any sensors > + */ > + return -ENODEV; > + } > + > + fmd = entity_to_fimc_mdev(&sd->entity); > + Are you sure this is the correct thing to do here? In general, the media controller should be instantiated only if there are sensors in the system. What do you mean by using "the FIMC directly"? Do you mean using it only as an m2m image processor or with a sensor, but without the CSIS, which would be the case for parallel I/F sensors? Could you point me to the place where CSIS is always dereferenced? A quick look through the code only revealed that everywhere it seems to be guarded by a NULL check. Another thought from looking at the implementation of __fimc_pipeline_s_stream() is that it probably shouldn't call s_stream on all the subdevices included in seq[], but only on those that are actually included as a part of the pipeline. It would be quite a waste of power to enable unnecessary hardware. Best regards, Tomasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel