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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 88F3FC433EF for ; Tue, 22 Mar 2022 14:02:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 084726B0071; Tue, 22 Mar 2022 10:02:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0347C6B0074; Tue, 22 Mar 2022 10:02:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E70866B0075; Tue, 22 Mar 2022 10:02:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.26]) by kanga.kvack.org (Postfix) with ESMTP id D76166B0071 for ; Tue, 22 Mar 2022 10:02:36 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id F111012181A for ; Tue, 22 Mar 2022 14:02:34 +0000 (UTC) X-FDA: 79272187470.11.9A7A40A Received: from alexa-out-sd-02.qualcomm.com (alexa-out-sd-02.qualcomm.com [199.106.114.39]) by imf05.hostedemail.com (Postfix) with ESMTP id 02FF0100031 for ; Tue, 22 Mar 2022 14:02:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1647957754; x=1679493754; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=U44Kwa8Q/jMglH7UombxJJKv6fr3bbe0K91vnRsSUjg=; b=n/3Hs00AB/hhEvXvu194v9dTh3Q7c3scYpFI4zrd5fGXFGUTEeSG8cQJ BzvuDrOzStmcSedZ22yccGZE6ZI9YPYoWs5qi9q0UokoAR9uxCdQWkQVX jjA77Zbpz+0YW+vwBmvpE7ZVKAMp2tTIRybYrJOVxyPdFqJYYvYL4qnhj I=; Received: from unknown (HELO ironmsg01-sd.qualcomm.com) ([10.53.140.141]) by alexa-out-sd-02.qualcomm.com with ESMTP; 22 Mar 2022 07:02:32 -0700 X-QCInternal: smtphost Received: from nasanex01c.na.qualcomm.com ([10.47.97.222]) by ironmsg01-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Mar 2022 07:02:31 -0700 Received: from nalasex01a.na.qualcomm.com (10.47.209.196) by nasanex01c.na.qualcomm.com (10.47.97.222) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Tue, 22 Mar 2022 07:02:31 -0700 Received: from [10.216.60.158] (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Tue, 22 Mar 2022 07:02:27 -0700 Message-ID: Date: Tue, 22 Mar 2022 19:32:16 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [linux-next:master 8752/11953] sound/soc/codecs/lpass-macro-common.c:53 lpass_macro_pds_init() warn: passing zero to 'ERR_PTR' Content-Language: en-US To: Dan Carpenter CC: , , , "Linux Memory Management List" , Mark Brown , Venkata Prasad Potturu References: <202203111754.32TAMFKD-lkp@intel.com> <20220322135413.GV336@kadam> From: Srinivasa Rao Mandadapu Organization: Qualcomm In-Reply-To: <20220322135413.GV336@kadam> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 02FF0100031 X-Rspam-User: Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=quicinc.com header.s=qcdkim header.b="n/3Hs00A"; spf=pass (imf05.hostedemail.com: domain of quic_srivasam@quicinc.com designates 199.106.114.39 as permitted sender) smtp.mailfrom=quic_srivasam@quicinc.com; dmarc=pass (policy=none) header.from=quicinc.com X-Stat-Signature: a6hiofg37up6fnsfnroyyqyhhzedq6rz X-HE-Tag: 1647957753-657619 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 3/22/2022 7:24 PM, Dan Carpenter wrote: Thanks for Your Time Dan!!! > On Tue, Mar 22, 2022 at 07:03:23PM +0530, Srinivasa Rao Mandadapu wrote: >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 14 struct lpass_macro *lpass_macro_pds_init(struct device *dev) >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 15 { >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 16 struct lpass_macro *l_pds; >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 17 int ret; >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 18 >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 19 if (!of_find_property(dev->of_node, "power-domains", NULL)) >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 20 return NULL; >>> >>> Returning NULL here will lead to a crash in tx_macro_runtime_resume() >>> >>> When a function returns a mix of NULL and error pointers, then NULL >>> means the feature is deliberately disabled. It's not an error, it's a >>> deliberate choice by the distro or sys admin. The caller has to >>> be written to allow the feature to be disabled. >>> >>> An example of this might be LEDs. Maybe people don't want LEDs so code >>> has to asume that the led->ops pointer might be NULL and check for that >>> before dereferencing it. >> Actually, it's optional here. For some targets, with lpass ADSP enabled, >> power domains are not required. >> >> So is the reason, returning NULL Here. >> > Unfortunately, the caller is not written to handle NULLs so it will > crash. Yes agree. will add error check with in lpass_macro_pds_exit(). Will that make sense? > > sound/soc/codecs/lpass-tx-macro.c > 1913 static int tx_macro_remove(struct platform_device *pdev) > 1914 { > 1915 struct tx_macro *tx = dev_get_drvdata(&pdev->dev); > 1916 > 1917 clk_disable_unprepare(tx->macro); > 1918 clk_disable_unprepare(tx->dcodec); > 1919 clk_disable_unprepare(tx->mclk); > 1920 clk_disable_unprepare(tx->npl); > 1921 clk_disable_unprepare(tx->fsgen); > 1922 > 1923 lpass_macro_pds_exit(tx->pds); > ^^^^^^^ > Boom. > > 1924 > 1925 return 0; > 1926 } > > >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 21 >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 22 l_pds = devm_kzalloc(dev, sizeof(*l_pds), GFP_KERNEL); >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 23 if (!l_pds) >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 24 return ERR_PTR(-ENOMEM); >>> >>> Good. >>> >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 25 >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 26 l_pds->macro_pd = dev_pm_domain_attach_by_name(dev, "macro"); >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 27 if (IS_ERR_OR_NULL(l_pds->macro_pd)) >>> 9e3d83c52844f95 Srinivasa Rao Mandadapu 2022-02-26 28 return NULL; >>> >>> If this feature is optional then it should be: >>> >>> if (IS_ERR_OR_NULL(l_pds->macro_pd)) >>> return ERR_CAST(l_pds->macro_pd); >>> >>> The admin deliberately chose to enable the feature so we can't just >>> ignore errors and convert them to NULL. >> Here it's not optional, if power domains feature is available then macro and >> dcodec power domains should be present. >> >> So will update it like below. >> >> if (IS_ERR_OR_NULL(l_pds->macro_pd)) { >> ret = PTR_ERR(l_pds->macro_pd); >> goto macro_err; >> } > I bet COMPILE_TEST allows this to compile without CONFIG_PM but there > isn't anything we can do about that... > > regards, > dan carpenter >