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 24D4CC5472D for ; Sat, 24 Aug 2024 18:51:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:CC:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=cTXwDdH5fYMans6AL7WAaQCqOrfwUgTk5ue5mciCuB8=; b=H3bOKrzQ4IDRFSQAuIP/KP9W/X /HxfPJM9XQD8ccL953RwDhAXFWpYbi6av/9a2JdW4qD2YzuahlhnDCBDtVGa0H+UlL2nMvtPg6eqH sEC+vtrk7yftxYvXBPSTvzShxUlLKCcXMnfdN+hXneuLDq98/NAztPrs9S5o8CpwGxJqRV8m12jVx HBolmGkI42rL2kqkoWwczp7xUy+iT/ykp3ROcFwO0C8FWYpGu0tOgw8OHsmV+i4hqOUjI/21LzXsu 28vbKhdisQHxDuMxlAYMkt9+EhnihF3LKdHrHwdZ8EfX5fg0UrfaGCzIEAcNdL64PCr15KcLoEUa4 dfnelgCw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1shvqj-00000002ixt-0nMz; Sat, 24 Aug 2024 18:50:57 +0000 Received: from lelv0143.ext.ti.com ([198.47.23.248]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1shvpw-00000002isK-2Z4y for linux-arm-kernel@lists.infradead.org; Sat, 24 Aug 2024 18:50:10 +0000 Received: from lelv0265.itg.ti.com ([10.180.67.224]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id 47OInoe7017964; Sat, 24 Aug 2024 13:49:50 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1724525390; bh=cTXwDdH5fYMans6AL7WAaQCqOrfwUgTk5ue5mciCuB8=; h=Date:From:To:CC:Subject:References:In-Reply-To; b=oaPBxPAHBTQs5YHBvtyPZkI51brtncAdx1gbOdoUJPeX9gF5Aux1/tgXuqeV3+pVB UiNQvy+tQ9YdYVOwr9sqyxQveuSsRe3kaftjv1H9KYwBkPNck6sFkrMjqbg1nxoy8X 0EljoV8twax77k79IG+RozjzY1dCexuJHuleul74= Received: from DFLE109.ent.ti.com (dfle109.ent.ti.com [10.64.6.30]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 47OInoC2023438 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Sat, 24 Aug 2024 13:49:50 -0500 Received: from DFLE108.ent.ti.com (10.64.6.29) by DFLE109.ent.ti.com (10.64.6.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Sat, 24 Aug 2024 13:49:50 -0500 Received: from lelvsmtp5.itg.ti.com (10.180.75.250) by DFLE108.ent.ti.com (10.64.6.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Sat, 24 Aug 2024 13:49:50 -0500 Received: from localhost (uda0133052.dhcp.ti.com [128.247.81.232]) by lelvsmtp5.itg.ti.com (8.15.2/8.15.2) with ESMTP id 47OInosZ088292; Sat, 24 Aug 2024 13:49:50 -0500 Date: Sat, 24 Aug 2024 13:49:50 -0500 From: Nishanth Menon To: Kousik Sanagavarapu CC: Jonathan Cameron , Santosh Shilimkar , Nathan Chancellor , Julia Lawall , Shuah Khan , Javier Carrasco , , Subject: Re: [PATCH v3 1/4] soc: ti: pruss: factor out memories setup Message-ID: <20240824184950.gzsgdawt2ujjt6ky@subgroup> References: <20240707055341.3656-1-five231003@gmail.com> <20240707055341.3656-2-five231003@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20240707055341.3656-2-five231003@gmail.com> X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240824_115009_487894_0AEE3244 X-CRM114-Status: GOOD ( 35.66 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10:44-20240707, Kousik Sanagavarapu wrote: > Factor out memories setup code from probe() into a new function > pruss_of_setup_memories(). This sets the stage for introducing auto > cleanup of the device node (done in the subsequent patch), since the > clean up depends on the scope of the pointer and factoring out > code into a seperate function obviously limits the scope of the various typo s/seperate/separate - use --codespell with checkpatch to catch :) A follow on patch has the same problem as well. > variables used in that function. > > Apart from the above, this change also has the advantage of making the > code look more neat. > > While at it, use dev_err_probe() instead of plain dev_err() as this new > function is called by the probe(). > > Signed-off-by: Kousik Sanagavarapu > --- > drivers/soc/ti/pruss.c | 111 ++++++++++++++++++++++------------------- > 1 file changed, 61 insertions(+), 50 deletions(-) > > diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c > index 24a42e0b645c..a3c55a291b0b 100644 > --- a/drivers/soc/ti/pruss.c > +++ b/drivers/soc/ti/pruss.c > @@ -415,6 +415,63 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node) > return ret; > } > > +static int pruss_of_setup_memories(struct device *dev, struct pruss *pruss) > +{ > + struct device_node *np = dev_of_node(dev); > + struct device_node *child; > + const struct pruss_private_data *data = of_device_get_match_data(dev); > + const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" }; > + int i; > + > + child = of_get_child_by_name(np, "memories"); > + if (!child) > + return dev_err_probe(dev, -ENODEV, > + "%pOF is missing its 'memories' node\n", > + child); > + > + for (i = 0; i < PRUSS_MEM_MAX; i++) { > + struct resource res; > + int index; > + > + /* > + * On AM437x one of two PRUSS units don't contain Shared RAM, > + * skip it > + */ > + if (data && data->has_no_sharedram && i == PRUSS_MEM_SHRD_RAM2) > + continue; > + > + index = of_property_match_string(child, "reg-names", > + mem_names[i]); > + if (index < 0) { > + of_node_put(child); > + return index; > + } > + > + if (of_address_to_resource(child, index, &res)) { > + of_node_put(child); > + return -EINVAL; > + } > + > + pruss->mem_regions[i].va = devm_ioremap(dev, res.start, > + resource_size(&res)); > + if (!pruss->mem_regions[i].va) { > + of_node_put(child); > + return dev_err_probe(dev, -ENOMEM, > + "failed to parse and map memory resource %d %s\n", > + i, mem_names[i]); > + } > + pruss->mem_regions[i].pa = res.start; > + pruss->mem_regions[i].size = resource_size(&res); > + > + dev_dbg(dev, "memory %8s: pa %pa size 0x%zx va %pK\n", > + mem_names[i], &pruss->mem_regions[i].pa, > + pruss->mem_regions[i].size, pruss->mem_regions[i].va); > + } > + of_node_put(child); > + > + return 0; > +} > + > static struct regmap_config regmap_conf = { > .reg_bits = 32, > .val_bits = 32, > @@ -471,15 +528,8 @@ static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss) > static int pruss_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > - struct device_node *np = dev_of_node(dev); > - struct device_node *child; > struct pruss *pruss; > - struct resource res; > - int ret, i, index; > - const struct pruss_private_data *data; > - const char *mem_names[PRUSS_MEM_MAX] = { "dram0", "dram1", "shrdram2" }; > - > - data = of_device_get_match_data(&pdev->dev); > + int ret; > > ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); > if (ret) { > @@ -494,48 +544,9 @@ static int pruss_probe(struct platform_device *pdev) > pruss->dev = dev; > mutex_init(&pruss->lock); > > - child = of_get_child_by_name(np, "memories"); > - if (!child) { > - dev_err(dev, "%pOF is missing its 'memories' node\n", child); > - return -ENODEV; > - } > - > - for (i = 0; i < PRUSS_MEM_MAX; i++) { > - /* > - * On AM437x one of two PRUSS units don't contain Shared RAM, > - * skip it > - */ > - if (data && data->has_no_sharedram && i == PRUSS_MEM_SHRD_RAM2) > - continue; > - > - index = of_property_match_string(child, "reg-names", > - mem_names[i]); > - if (index < 0) { > - of_node_put(child); > - return index; > - } > - > - if (of_address_to_resource(child, index, &res)) { > - of_node_put(child); > - return -EINVAL; > - } > - > - pruss->mem_regions[i].va = devm_ioremap(dev, res.start, > - resource_size(&res)); > - if (!pruss->mem_regions[i].va) { > - dev_err(dev, "failed to parse and map memory resource %d %s\n", > - i, mem_names[i]); > - of_node_put(child); > - return -ENOMEM; > - } > - pruss->mem_regions[i].pa = res.start; > - pruss->mem_regions[i].size = resource_size(&res); > - > - dev_dbg(dev, "memory %8s: pa %pa size 0x%zx va %pK\n", > - mem_names[i], &pruss->mem_regions[i].pa, > - pruss->mem_regions[i].size, pruss->mem_regions[i].va); > - } > - of_node_put(child); > + ret = pruss_of_setup_memories(dev, pruss); > + if (ret < 0) > + goto rpm_put; Why? We have not called pm_runtime_enable at this point. > > platform_set_drvdata(pdev, pruss); > > -- > 2.45.2.561.g66ac6e4bcd > -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D