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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E23B0C61DF7 for ; Thu, 23 Nov 2023 13:16:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=tg5v5hGY8DtPubggB5+efB50UNBZwhNkDMyN84tDimo=; b=IgDrVqYvk8rxpn RWgZm8zXwzIdt9QQ6kcSJEszckWGUc2R67zJxVMJdcUubahEMg25XAO9GSbONqrWyyyCn4wkxHpm0 Av96kOkxcP+5Lhka3rOO8yiVj0dm5HS3lIHpM9Il92AMf3/Hblxham4x0+DJeIyBbJtn2Y3M4XPXE A/1Hst2vxjoARw8loHYXoRiFsmpxc9QJy4f2sqONGSQ4q7kU4ie5rduBC+oFLoYqBVFZ6KM7XmLV6 WanKepgUxyLdFlH142h4tXQNCr3kNyve6I44TDBuwr4I0xhB6LEGLF5p1ci/8Xn5WmM6m8AA3ktpD z2r/nRzbTyDzX2lw/Gpw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r69Yq-004rfu-2G; Thu, 23 Nov 2023 13:16:04 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r69YT-004rXf-1d; Thu, 23 Nov 2023 13:15:42 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0EA83620E4; Thu, 23 Nov 2023 13:15:41 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BC946C433C8; Thu, 23 Nov 2023 13:15:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1700745340; bh=nH7dXM1v48hCQEwydIRgFJ5nFhGmuyX9YZPGafz3Pxw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KyPchN12hc69ZKNewTbuiSIBSITycVgAVn491PHwnXSh5qJHQYuOHLzuCgoWZ6gx9 AaN2/4tKha42UzAkK9QHYR/m3Hz4NTPNBPc6De4gPlv85M+gcpdsZdUl5K0k2osWED EKIqaoXCwstrz/mSsvMw8BO3PuruYhlmh4NNMC40= Date: Thu, 23 Nov 2023 13:02:01 +0000 From: Greg Kroah-Hartman To: Umang Jain Cc: linux-staging@lists.linux.dev, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org, Stefan Wahren , Dan Carpenter , Kieran Bingham , Laurent Pinchart , Phil Elwell , Dave Stevenson , "Ricardo B . Marliere" Subject: Re: [PATCH 2/9] staging: vc04_services: Log using pr_err() when vchiq_state is unset Message-ID: <2023112312-epic-viscosity-3848@gregkh> References: <20231107095156.365492-1-umang.jain@ideasonboard.com> <20231107095156.365492-3-umang.jain@ideasonboard.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231107095156.365492-3-umang.jain@ideasonboard.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231123_051541_586250_978DD6AD X-CRM114-Status: GOOD ( 18.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 On Tue, Nov 07, 2023 at 04:51:49AM -0500, Umang Jain wrote: > In cases, where the global vchiq state is still unset, we cannot log > to dynamic debug (access to struct device is needed, hence potential > NULL de-reference). Log using pr_err() instead. No, something is wrong here, don't do that. > In vchiq_initialise(), remove the 'goto' because it is just again > trying to log to dynamic debug. Simply return with -ENNOTCONN after > logging to pr_err(). > > In vchiq_open(), move the vchiq_log_debug() after the state pointer > null check. > > Signed-off-by: Umang Jain > Reviewed-by: Ricardo B. Marliere > --- > .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 ++---- > .../staging/vc04_services/interface/vchiq_arm/vchiq_dev.c | 7 +++---- > 2 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > index 9fb8f657cc78..9fb3e240d9de 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -687,10 +687,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out) > usleep_range(500, 600); > } > if (i == VCHIQ_INIT_RETRIES) { > - vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not initialized\n", > - __func__); > - ret = -ENOTCONN; > - goto failed; > + pr_err("%s: videocore not initialized\n", __func__); > + return -ENOTCONN; Here's a good reason to get rid of the crazy "this subsystem is special so let us use a custom logging macro" logic. Convert everything to just use real dev_*() calls so it makes it obvious what is happening and how it all is working. It will save you from doing stuff like this in the future as you will "know" that there isn't a valid device pointer here. thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel