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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0A72C761A6 for ; Tue, 28 Mar 2023 05:42:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232241AbjC1Fmv (ORCPT ); Tue, 28 Mar 2023 01:42:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34748 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232220AbjC1Fmu (ORCPT ); Tue, 28 Mar 2023 01:42:50 -0400 Received: from fllv0016.ext.ti.com (fllv0016.ext.ti.com [198.47.19.142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D929D211F; Mon, 27 Mar 2023 22:42:47 -0700 (PDT) Received: from fllv0035.itg.ti.com ([10.64.41.0]) by fllv0016.ext.ti.com (8.15.2/8.15.2) with ESMTP id 32S5gZXm035724; Tue, 28 Mar 2023 00:42:35 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1679982155; bh=J46dwSfAkxaSMGmHAphy7mrLmxdhhaEryCo6bT/UqyI=; h=Date:Subject:To:CC:References:From:In-Reply-To; b=MiNcEsODHj/zmKzsqQvfMZYd/zqyUTeNuyacK2ZiD2dEFhgRjUsNjyrGt5LfMREdp KQNBwrFwjNyFTpjSJPvrbWkF0WhKOdUuvSjqsQr+/1ksWkaKWZ5UdWV6x1Vjh6P2D2 AcUb8HT6c1NOU1i+/NgjIvS2d6sEgPcvlddhE3tc= Received: from DLEE115.ent.ti.com (dlee115.ent.ti.com [157.170.170.26]) by fllv0035.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 32S5gZPQ074686 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 28 Mar 2023 00:42:35 -0500 Received: from DLEE112.ent.ti.com (157.170.170.23) by DLEE115.ent.ti.com (157.170.170.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16; Tue, 28 Mar 2023 00:42:35 -0500 Received: from fllv0040.itg.ti.com (10.64.41.20) by DLEE112.ent.ti.com (157.170.170.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16 via Frontend Transport; Tue, 28 Mar 2023 00:42:34 -0500 Received: from [10.24.69.114] (ileaxei01-snat2.itg.ti.com [10.180.69.6]) by fllv0040.itg.ti.com (8.15.2/8.15.2) with ESMTP id 32S5gUl1113131; Tue, 28 Mar 2023 00:42:30 -0500 Message-ID: <9d4c7762-615b-0fbd-76d2-87156e691928@ti.com> Date: Tue, 28 Mar 2023 11:12:29 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [EXTERNAL] Re: [PATCH v5 1/5] soc: ti: pruss: Add pruss_get()/put() API Content-Language: en-US To: Mathieu Poirier , MD Danish Anwar CC: "Andrew F. Davis" , Suman Anna , Roger Quadros , Vignesh Raghavendra , Tero Kristo , Bjorn Andersson , Santosh Shilimkar , Nishanth Menon , , , , , , , References: <20230323062451.2925996-1-danishanwar@ti.com> <20230323062451.2925996-2-danishanwar@ti.com> <20230327205841.GA3158115@p14s> From: Md Danish Anwar Organization: Texas Instruments In-Reply-To: <20230327205841.GA3158115@p14s> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org Hi Mathieu, On 28/03/23 02:28, Mathieu Poirier wrote: > Hi Danish > > On Thu, Mar 23, 2023 at 11:54:47AM +0530, MD Danish Anwar wrote: >> From: Tero Kristo >> >> Add two new get and put API, pruss_get() and pruss_put() to the >> PRUSS platform driver to allow client drivers to request a handle >> to a PRUSS device. This handle will be used by client drivers to >> request various operations of the PRUSS platform driver through >> additional API that will be added in the following patches. >> >> The pruss_get() function returns the pruss handle corresponding >> to a PRUSS device referenced by a PRU remoteproc instance. The >> pruss_put() is the complimentary function to pruss_get(). >> >> Co-developed-by: Suman Anna >> Signed-off-by: Suman Anna >> Signed-off-by: Tero Kristo >> Co-developed-by: Grzegorz Jaszczyk >> Signed-off-by: Grzegorz Jaszczyk >> Signed-off-by: Puranjay Mohan >> Signed-off-by: MD Danish Anwar >> Reviewed-by: Roger Quadros >> --- >> drivers/remoteproc/pru_rproc.c | 2 +- >> drivers/soc/ti/pruss.c | 60 ++++++++++++++++++- >> .../{pruss_driver.h => pruss_internal.h} | 7 ++- >> include/linux/remoteproc/pruss.h | 19 ++++++ >> 4 files changed, 83 insertions(+), 5 deletions(-) >> rename include/linux/{pruss_driver.h => pruss_internal.h} (90%) >> >> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c >> index b76db7fa693d..4ddd5854d56e 100644 >> --- a/drivers/remoteproc/pru_rproc.c >> +++ b/drivers/remoteproc/pru_rproc.c >> @@ -19,7 +19,7 @@ >> #include >> #include >> #include >> -#include >> +#include >> #include >> >> #include "remoteproc_internal.h" >> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >> index 6882c86b3ce5..6c2bb02a521d 100644 >> --- a/drivers/soc/ti/pruss.c >> +++ b/drivers/soc/ti/pruss.c >> @@ -6,6 +6,7 @@ >> * Author(s): >> * Suman Anna >> * Andrew F. Davis >> + * Tero Kristo >> */ >> >> #include >> @@ -16,8 +17,9 @@ >> #include >> #include >> #include >> -#include >> +#include >> #include >> +#include >> #include >> >> /** >> @@ -30,6 +32,62 @@ struct pruss_private_data { >> bool has_core_mux_clock; >> }; >> >> +/** >> + * pruss_get() - get the pruss for a given PRU remoteproc >> + * @rproc: remoteproc handle of a PRU instance >> + * >> + * Finds the parent pruss device for a PRU given the @rproc handle of the >> + * PRU remote processor. This function increments the pruss device's refcount, >> + * so always use pruss_put() to decrement it back once pruss isn't needed >> + * anymore. >> + * >> + * Return: pruss handle on success, and an ERR_PTR on failure using one >> + * of the following error values >> + * -EINVAL if invalid parameter >> + * -ENODEV if PRU device or PRUSS device is not found >> + */ >> +struct pruss *pruss_get(struct rproc *rproc) >> +{ >> + struct pruss *pruss; >> + struct device *dev; >> + struct platform_device *ppdev; >> + >> + if (IS_ERR_OR_NULL(rproc)) >> + return ERR_PTR(-EINVAL); >> + > > There is no guarantee that @rproc is valid without calling rproc_get_by_handle() > or pru_rproc_get(). > Here in this API, we are checking if rproc is NULL or not. Also we are checking is_pru_rproc() to make sure this rproc is pru-rproc only and not some other rproc. This API will be called from driver (icssg_prueth.c) which I'll post once this series is merged. In the driver we are doing, prueth->pru[slice] = pru_rproc_get(np, pru, &pruss_id); pruss = pruss_get(prueth->pru[slice]); So, before calling pruss_get() we are in fact calling pru_rproc_get() to make sure it's a valid rproc. I think in this API, these two checks (NULL check and is_pru_rproc) should be OK as the driver is already calling pru_rproc_get() before this API. The only way to get a "pru-rproc" is by calling pru_rproc_get(), now the check is_pru_rproc() will only be true if it is a "pru-rproc" implying pru_rproc_get() was called before calling this API. Please let me know if this is OK or if any change is required. >> + dev = &rproc->dev; >> + >> + /* make sure it is PRU rproc */ >> + if (!dev->parent || !is_pru_rproc(dev->parent)) >> + return ERR_PTR(-ENODEV); >> + >> + ppdev = to_platform_device(dev->parent->parent); >> + pruss = platform_get_drvdata(ppdev); >> + if (!pruss) >> + return ERR_PTR(-ENODEV); >> + >> + get_device(pruss->dev); >> + >> + return pruss; >> +} >> +EXPORT_SYMBOL_GPL(pruss_get); >> + >> +/** >> + * pruss_put() - decrement pruss device's usecount >> + * @pruss: pruss handle >> + * >> + * Complimentary function for pruss_get(). Needs to be called >> + * after the PRUSS is used, and only if the pruss_get() succeeds. >> + */ >> +void pruss_put(struct pruss *pruss) >> +{ >> + if (IS_ERR_OR_NULL(pruss)) >> + return; >> + >> + put_device(pruss->dev); >> +} >> +EXPORT_SYMBOL_GPL(pruss_put); >> + >> static void pruss_of_free_clk_provider(void *data) >> { >> struct device_node *clk_mux_np = data; >> diff --git a/include/linux/pruss_driver.h b/include/linux/pruss_internal.h >> similarity index 90% >> rename from include/linux/pruss_driver.h >> rename to include/linux/pruss_internal.h >> index ecfded30ed05..8f91cb164054 100644 >> --- a/include/linux/pruss_driver.h >> +++ b/include/linux/pruss_internal.h >> @@ -6,9 +6,10 @@ >> * Suman Anna >> */ >> >> -#ifndef _PRUSS_DRIVER_H_ >> -#define _PRUSS_DRIVER_H_ >> +#ifndef _PRUSS_INTERNAL_H_ >> +#define _PRUSS_INTERNAL_H_ >> >> +#include >> #include >> >> /* >> @@ -51,4 +52,4 @@ struct pruss { >> struct clk *iep_clk_mux; >> }; >> >> -#endif /* _PRUSS_DRIVER_H_ */ >> +#endif /* _PRUSS_INTERNAL_H_ */ >> diff --git a/include/linux/remoteproc/pruss.h b/include/linux/remoteproc/pruss.h >> index 039b50d58df2..93a98cac7829 100644 >> --- a/include/linux/remoteproc/pruss.h >> +++ b/include/linux/remoteproc/pruss.h >> @@ -4,12 +4,14 @@ >> * >> * Copyright (C) 2015-2022 Texas Instruments Incorporated - http://www.ti.com >> * Suman Anna >> + * Tero Kristo >> */ >> >> #ifndef __LINUX_PRUSS_H >> #define __LINUX_PRUSS_H >> >> #include >> +#include >> #include >> >> #define PRU_RPROC_DRVNAME "pru-rproc" >> @@ -44,6 +46,23 @@ enum pru_ctable_idx { >> >> struct device_node; >> struct rproc; >> +struct pruss; >> + >> +#if IS_ENABLED(CONFIG_TI_PRUSS) >> + >> +struct pruss *pruss_get(struct rproc *rproc); >> +void pruss_put(struct pruss *pruss); >> + >> +#else >> + >> +static inline struct pruss *pruss_get(struct rproc *rproc) >> +{ >> + return ERR_PTR(-EOPNOTSUPP); >> +} >> + >> +static inline void pruss_put(struct pruss *pruss) { } >> + >> +#endif /* CONFIG_TI_PRUSS */ >> >> #if IS_ENABLED(CONFIG_PRU_REMOTEPROC) >> >> -- >> 2.25.1 >> -- Thanks and Regards, Danish.