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 8297DC6FD1F for ; Thu, 16 Mar 2023 12:20:31 +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:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+q7+E/FIOmDjBV4b4LFqPbio65mZgjJ+1CtVYYI+6ec=; b=PU5QZqef6xrggM FLnH0s9TI56VLV+OeSMcvbAJtDpoU+5GZU+GFH2bHiI3l/pi1QZZT8m2byn6P8O3gbGRc/QWKNo4v UpluvGEU3/H9TTM1JKB1uwW6R6yXpgaTzNo6H4KNlQVr5GXfKLDua16DkCZFqHNiIFWVheoJDZ+PR sEgRckLDrvz4hWWzWXEnak5QU4wYmyd2brrX93xiw6hDGLFLjFZcH++14XySvJld6tjEY6ZdUsDmH grK4ZqlecnnIMP+iif1dvmbF9jYgL31WbkJanGAiRTdJUXNlTTNKhh/emwv0L8ZbelDc/7VNuBRQO 0Nw5/5pah90eygaWeYyg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pcmZo-00GMnW-2z; Thu, 16 Mar 2023 12:19:24 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pcmZl-00GMmB-1s for linux-arm-kernel@lists.infradead.org; Thu, 16 Mar 2023 12:19:23 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id CED54B82140; Thu, 16 Mar 2023 12:19:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D91E0C4339B; Thu, 16 Mar 2023 12:19:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1678969156; bh=hbKOvEwKovlGoJYefBxKQZ367EL/i6RYcMcuImKZkZQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=kR5QVIrBJ9m5dBdeldQYjOL2yMiKiZUlNhv5gk9sB198hjvyqoPeU9hCjqXS1PrCc VBFFzoQCLhoVoxMbW72ErLDOkGrhi+J+CtDKVBJ0BUATLbpbWVBb6E2qmkfw1q26PU uJebGUsfmdNChhnmO4fI9IPRl13qgrT7F96xM/BaiXoTEpx5jZshRPPu8mR5J1wWQw vUHT+gh+eDHVerZzinN5jz+gAtAHUw2gKIBO4uZCn78vIfkemghMj/FBPn5sScAlsj hfyHVkwze0tYmegXxq/x9dsw391XQsWFmE6UqXHG2dj8KqIfi/EsXjyMjuAWPiYoZy DI052+tp2x+Hw== Message-ID: Date: Thu, 16 Mar 2023 14:19:10 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v4 4/5] soc: ti: pruss: Add helper functions to set GPI mode, MII_RT_event and XFR Content-Language: en-US To: Md Danish Anwar , MD Danish Anwar , "Andrew F. Davis" , Suman Anna , Vignesh Raghavendra , Mathieu Poirier , Bjorn Andersson , Santosh Shilimkar , Nishanth Menon Cc: linux-remoteproc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, srk@ti.com, devicetree@vger.kernel.org, netdev@vger.kernel.org References: <20230313111127.1229187-1-danishanwar@ti.com> <20230313111127.1229187-5-danishanwar@ti.com> <60e73395-f670-6eaa-0eb7-389553320a71@kernel.org> <20718115-7606-a77b-7e4d-511ca9c1d798@ti.com> From: Roger Quadros In-Reply-To: <20718115-7606-a77b-7e4d-511ca9c1d798@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230316_051921_916164_36BCD28D X-CRM114-Status: GOOD ( 28.69 ) 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 16/03/2023 13:44, Md Danish Anwar wrote: > > On 16/03/23 17:06, Roger Quadros wrote: >> Hi, >> >> On 16/03/2023 13:05, Md Danish Anwar wrote: >>> Hi Roger, >>> >>> On 15/03/23 17:52, Roger Quadros wrote: >>>> >>>> >>>> On 13/03/2023 13:11, MD Danish Anwar wrote: >>>>> From: Suman Anna >>>>> >>>>> The PRUSS CFG module is represented as a syscon node and is currently >>>>> managed by the PRUSS platform driver. Add easy accessor functions to set >>>>> GPI mode, MII_RT event enable/disable and XFR (XIN XOUT) enable/disable >>>>> to enable the PRUSS Ethernet usecase. These functions reuse the generic >>>>> pruss_cfg_update() API function. >>>>> >>>>> Signed-off-by: Suman Anna >>>>> Co-developed-by: Grzegorz Jaszczyk >>>>> Signed-off-by: Grzegorz Jaszczyk >>>>> Signed-off-by: Puranjay Mohan >>>>> Signed-off-by: MD Danish Anwar >>>>> --- >>>>> drivers/soc/ti/pruss.c | 60 ++++++++++++++++++++++++++++++++ >>>>> include/linux/remoteproc/pruss.h | 22 ++++++++++++ >>>>> 2 files changed, 82 insertions(+) >>>>> >>>>> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c >>>>> index 26d8129b515c..2f04b7922ddb 100644 >>>>> --- a/drivers/soc/ti/pruss.c >>>>> +++ b/drivers/soc/ti/pruss.c >>>>> @@ -203,6 +203,66 @@ static int pruss_cfg_update(struct pruss *pruss, unsigned int reg, >>>>> return regmap_update_bits(pruss->cfg_regmap, reg, mask, val); >>>>> } >>>>> >>>>> +/** >>>>> + * pruss_cfg_gpimode() - set the GPI mode of the PRU >>>>> + * @pruss: the pruss instance handle >>>>> + * @pru_id: id of the PRU core within the PRUSS >>>>> + * @mode: GPI mode to set >>>>> + * >>>>> + * Sets the GPI mode for a given PRU by programming the >>>>> + * corresponding PRUSS_CFG_GPCFGx register >>>>> + * >>>>> + * Return: 0 on success, or an error code otherwise >>>>> + */ >>>>> +int pruss_cfg_gpimode(struct pruss *pruss, enum pruss_pru_id pru_id, >>>>> + enum pruss_gpi_mode mode) >>>>> +{ >>>>> + if (pru_id < 0 || pru_id >= PRUSS_NUM_PRUS) >>>>> + return -EINVAL; >>>>> + >>>>> + if (mode < 0 || mode > PRUSS_GPI_MODE_MAX) >>>>> + return -EINVAL; >>>>> + >>>>> + return pruss_cfg_update(pruss, PRUSS_CFG_GPCFG(pru_id), >>>>> + PRUSS_GPCFG_PRU_GPI_MODE_MASK, >>>>> + mode << PRUSS_GPCFG_PRU_GPI_MODE_SHIFT); >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(pruss_cfg_gpimode); >>>>> + >>>>> +/** >>>>> + * pruss_cfg_miirt_enable() - Enable/disable MII RT Events >>>>> + * @pruss: the pruss instance >>>>> + * @enable: enable/disable >>>>> + * >>>>> + * Enable/disable the MII RT Events for the PRUSS. >>>>> + * >>>>> + * Return: 0 on success, or an error code otherwise >>>>> + */ >>>>> +int pruss_cfg_miirt_enable(struct pruss *pruss, bool enable) >>>>> +{ >>>>> + u32 set = enable ? PRUSS_MII_RT_EVENT_EN : 0; >>>>> + >>>>> + return pruss_cfg_update(pruss, PRUSS_CFG_MII_RT, >>>>> + PRUSS_MII_RT_EVENT_EN, set); >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(pruss_cfg_miirt_enable); >>>>> + >>>>> +/** >>>>> + * pruss_cfg_xfr_enable() - Enable/disable XIN XOUT shift functionality >>>>> + * @pruss: the pruss instance >>>>> + * @enable: enable/disable >>>>> + * @mask: Mask for PRU / RTU >>>> >>>> You should not expect the user to provide the mask but only >>>> the core type e.g. >>>> >>>> enum pru_type { >>>> PRU_TYPE_PRU = 0, >>>> PRU_TYPE_RTU, >>>> PRU_TYPE_TX_PRU, >>>> PRU_TYPE_MAX, >>>> }; >>>> >>>> Then you figure out the mask in the function. >>>> Also check for invalid pru_type and return error if so. >>>> >>> >>> Sure Roger, I will create a enum and take it as parameter in API. Based on >>> these enum I will calculate mask and do XFR shifting inside the API >>> pruss_cfg_xfr_enable(). >>> >>> There are two registers for XFR shift. >>> >>> #define PRUSS_SPP_XFER_SHIFT_EN BIT(1) >>> #define PRUSS_SPP_RTU_XFR_SHIFT_EN BIT(3) >>> >>> For PRU XFR shifting, the mask should be PRUSS_SPP_XFER_SHIFT_EN, >>> for RTU shifting mask should be PRUSS_SPP_RTU_XFR_SHIFT_EN and for PRU and RTU >>> shifting mask should be (PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN) >>> >>> So the enum would be something like this. >>> >>> /** >>> * enum xfr_shift_type - XFR shift type >>> * @XFR_SHIFT_PRU: Enables XFR shift for PRU >>> * @XFR_SHIFT_RTU: Enables XFR shift for RTU >>> * @XFR_SHIFT_PRU_RTU: Enables XFR shift for both PRU and RTU >> >> This is not required. User can call the API twice. once for PRU and once for RTU. >> >>> * @XFR_SHIFT_MAX: Total number of XFR shift types available. >>> * >>> */ >>> >>> enum xfr_shift_type { >>> XFR_SHIFT_PRU = 0, >>> XFR_SHIFT_RTU, >>> XFR_SHIFT_PRU_RTU, >>> XFR_SHIFT_MAX, >>> }; >> >> Why do you need this new enum definition? >> We already have pru_type defined somewhere. You can move it to a public header >> if not there yet. >> >> enum pru_type { >> PRU_TYPE_PRU = 0, >> PRU_TYPE_RTU, >> PRU_TYPE_TX_PRU, >> PRU_TYPE_MAX, >> }; >> > > This enum is present in drivers/remoteproc/pru_rproc.c file. But the problem > with this enum is that in [1] we need to enable XFR shift for both PRU and RTU > for which the mask will be OR of PRUSS_SPP_XFER_SHIFT_EN (mask for PRU) and > PRUSS_SPP_RTU_XFR_SHIFT_EN (mask of RTU). Is there any limitation that you have to enable both simultaneously? The driver can first do enable for PRU and then later for RTU. As you will do a read modify write, the previous enable state of register shouldn't be affected. > > Now this enum doesn't have a field for both PRU and RTU. Also we don't need > need the XFR shift for PRU_TYPE_TX_PRU as only two XFR shift register bits are > defined. That is OK. You can return error if not RTU or PRU. > > That is why I thought of introducing new enum. > > [1] drivers/net/ethernet/ti/icssg_config.c > > /* enable XFR shift for PRU and RTU */ > mask = PRUSS_SPP_XFER_SHIFT_EN | PRUSS_SPP_RTU_XFR_SHIFT_EN; Driver can do like so pruss_cfg_xfr_enable(pruss, PRU_TYPE_PRU, true); pruss_cfg_xfr_enable(pruss, PRU_TYPE_RTU, true); The second call should not disable the PRU XFR as you will do a read-modify-write only affecting the RTU bit. cheers, -roger _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel