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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1986AC352A2 for ; Fri, 7 Feb 2020 10:41:01 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id DD4492082E for ; Fri, 7 Feb 2020 10:41:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="C1B8G+RE" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DD4492082E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject: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=cDkc8YRlXjmS/27ECcQ+fwYrGv9JpcmVdXBJvorK3nY=; b=C1B8G+RE8oSWUQ FIBtd8uyrBlIowwVZzL6FRiP2n3uNqt3UjmGXgEDZC0fXHw3aW6dirY5Yq2vgztzpYStTLRZlbY20 omOPDf95OLpDJdxCX4fwIwBGgGrNHk4/HJwIClTFWKL3AaGl9KHAr9Ov7t9X65AQb1a8dkaqUF8sy K1EPxOc67laPk8mjJogsu6USgaVsElLzdh5cH0tmlYGQ3uaNz7JJU4uxJkdQLK14+Gdi2i7lJ80P1 tzbVJ9DOHQqCXvW95HLLwFojC0SURmzghDjM5pcqSN29pOfTOhLApElNBG3YrlrMsdoH+8cHWl80j orAIMN1AaCoiCNgOjHYw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1j014D-0003n3-6Y; Fri, 07 Feb 2020 10:40:57 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1j014A-0003mQ-Is for linux-arm-kernel@lists.infradead.org; Fri, 07 Feb 2020 10:40:56 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 55FDE30E; Fri, 7 Feb 2020 02:40:49 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 576A13F52E; Fri, 7 Feb 2020 02:40:48 -0800 (PST) Date: Fri, 7 Feb 2020 10:40:43 +0000 From: Sudeep Holla To: Peng Fan Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init Message-ID: <20200207104043.GA36345@bogus> References: <1580993846-17712-1-git-send-email-peng.fan@nxp.com> <1580993846-17712-2-git-send-email-peng.fan@nxp.com> <20200206143337.GC3383@bogus> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200207_024054_710579_860C00DC X-CRM114-Status: GOOD ( 26.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "f.fainelli@gmail.com" , "viresh.kumar@linaro.org" , linux-kernel , dl-linux-imx , Sudeep Holla , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Feb 07, 2020 at 02:16:04AM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init > > > > On Thu, Feb 06, 2020 at 08:57:26PM +0800, peng.fan@nxp.com wrote: > > > From: Peng Fan > > > > > > The firmware itself might not mark channel free, so let's explicitly > > > mark it free when do initialization. > > > > > > Also move struct scmi_shared_mem to common.h > > > > > > Signed-off-by: Peng Fan > > > --- > > > drivers/firmware/arm_scmi/common.h | 19 +++++++++++++++++-- > > > drivers/firmware/arm_scmi/mailbox.c | 2 ++ > > > drivers/firmware/arm_scmi/shmem.c | 18 ------------------ > > > 3 files changed, 19 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/firmware/arm_scmi/common.h > > > b/drivers/firmware/arm_scmi/common.h > > > index fd091a4ccbff..5df262a564a4 100644 > > > --- a/drivers/firmware/arm_scmi/common.h > > > +++ b/drivers/firmware/arm_scmi/common.h > > > @@ -211,8 +211,23 @@ extern const struct scmi_desc scmi_mailbox_desc; > > > void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr); > > > void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, > > > int id); > > > > > > -/* shmem related declarations */ > > > -struct scmi_shared_mem; > > > +/* > > > + * SCMI specification requires all parameters, message headers, > > > +return > > > + * arguments or any protocol data to be expressed in little endian > > > + * format only. > > > + */ > > > +struct scmi_shared_mem { > > > + __le32 reserved; > > > + __le32 channel_status; > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR BIT(1) > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE BIT(0) > > > + __le32 reserved1[2]; > > > + __le32 flags; > > > +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0) > > > + __le32 length; > > > + __le32 msg_header; > > > + u8 msg_payload[0]; > > > +}; > > > > > > void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem, > > > struct scmi_xfer *xfer); > > > diff --git a/drivers/firmware/arm_scmi/mailbox.c > > > b/drivers/firmware/arm_scmi/mailbox.c > > > index 68ed58e2a47a..2d34bf6e94e2 100644 > > > --- a/drivers/firmware/arm_scmi/mailbox.c > > > +++ b/drivers/firmware/arm_scmi/mailbox.c > > > @@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct > > scmi_chan_info *cinfo, struct device *dev, > > > cinfo->transport_info = smbox; > > > smbox->cinfo = cinfo; > > > > > > + iowrite32(BIT(0), &smbox->shmem->channel_status); > > > + > > > > +arm list > > > If we need this then we may need to put this as a function in shmem.c I am > > still not convinced if we can do this unconditionally, i.e. will that affect Rx > > channel if there's notification pending before we initialise. But we can deal > > with that later. > > Per understanding, channel is specific to an agent, it could not be shared. > So the shmem binded to the channel will not be used by others. > Yes > Since this is the initialization process, the firmware might not init the shmem. > But, is there any particular reason for firmware not to ? It means platform holds the channel and needs to release it to agent(OSPM) in this case after initialisation. > The shmem.c shmem_tx_prepare will spin until channel free, so I did the patch. > Otherwise it might spin forever. > Yes I guessed that to be reason. > I'll add a check as following > if (tx) > iowrite32(BIT(0), &smbox->shmem->channel_status); > Not yet, I need answers to above query. > I not find a good place to put this in shmem.c (: > Least of the problem :), let us first agree if we have to have it. > > > > Also what about error fields ? I would rather clear it to 0, not just BIT(0) > > Tx channel error should also be cleared, fix in v2. > OK but wait for a while before you post for the discussion to end. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 590E1C352A2 for ; Fri, 7 Feb 2020 10:40:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 373582082E for ; Fri, 7 Feb 2020 10:40:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727144AbgBGKku (ORCPT ); Fri, 7 Feb 2020 05:40:50 -0500 Received: from foss.arm.com ([217.140.110.172]:38670 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726827AbgBGKku (ORCPT ); Fri, 7 Feb 2020 05:40:50 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 55FDE30E; Fri, 7 Feb 2020 02:40:49 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 576A13F52E; Fri, 7 Feb 2020 02:40:48 -0800 (PST) Date: Fri, 7 Feb 2020 10:40:43 +0000 From: Sudeep Holla To: Peng Fan Cc: "viresh.kumar@linaro.org" , "f.fainelli@gmail.com" , dl-linux-imx , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" , linux-kernel , Sudeep Holla Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init Message-ID: <20200207104043.GA36345@bogus> References: <1580993846-17712-1-git-send-email-peng.fan@nxp.com> <1580993846-17712-2-git-send-email-peng.fan@nxp.com> <20200206143337.GC3383@bogus> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 07, 2020 at 02:16:04AM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH 2/2] firmware: arm_scmi: mark channel free when init > > > > On Thu, Feb 06, 2020 at 08:57:26PM +0800, peng.fan@nxp.com wrote: > > > From: Peng Fan > > > > > > The firmware itself might not mark channel free, so let's explicitly > > > mark it free when do initialization. > > > > > > Also move struct scmi_shared_mem to common.h > > > > > > Signed-off-by: Peng Fan > > > --- > > > drivers/firmware/arm_scmi/common.h | 19 +++++++++++++++++-- > > > drivers/firmware/arm_scmi/mailbox.c | 2 ++ > > > drivers/firmware/arm_scmi/shmem.c | 18 ------------------ > > > 3 files changed, 19 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/firmware/arm_scmi/common.h > > > b/drivers/firmware/arm_scmi/common.h > > > index fd091a4ccbff..5df262a564a4 100644 > > > --- a/drivers/firmware/arm_scmi/common.h > > > +++ b/drivers/firmware/arm_scmi/common.h > > > @@ -211,8 +211,23 @@ extern const struct scmi_desc scmi_mailbox_desc; > > > void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr); > > > void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, > > > int id); > > > > > > -/* shmem related declarations */ > > > -struct scmi_shared_mem; > > > +/* > > > + * SCMI specification requires all parameters, message headers, > > > +return > > > + * arguments or any protocol data to be expressed in little endian > > > + * format only. > > > + */ > > > +struct scmi_shared_mem { > > > + __le32 reserved; > > > + __le32 channel_status; > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR BIT(1) > > > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE BIT(0) > > > + __le32 reserved1[2]; > > > + __le32 flags; > > > +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0) > > > + __le32 length; > > > + __le32 msg_header; > > > + u8 msg_payload[0]; > > > +}; > > > > > > void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem, > > > struct scmi_xfer *xfer); > > > diff --git a/drivers/firmware/arm_scmi/mailbox.c > > > b/drivers/firmware/arm_scmi/mailbox.c > > > index 68ed58e2a47a..2d34bf6e94e2 100644 > > > --- a/drivers/firmware/arm_scmi/mailbox.c > > > +++ b/drivers/firmware/arm_scmi/mailbox.c > > > @@ -104,6 +104,8 @@ static int mailbox_chan_setup(struct > > scmi_chan_info *cinfo, struct device *dev, > > > cinfo->transport_info = smbox; > > > smbox->cinfo = cinfo; > > > > > > + iowrite32(BIT(0), &smbox->shmem->channel_status); > > > + > > > > +arm list > > > If we need this then we may need to put this as a function in shmem.c I am > > still not convinced if we can do this unconditionally, i.e. will that affect Rx > > channel if there's notification pending before we initialise. But we can deal > > with that later. > > Per understanding, channel is specific to an agent, it could not be shared. > So the shmem binded to the channel will not be used by others. > Yes > Since this is the initialization process, the firmware might not init the shmem. > But, is there any particular reason for firmware not to ? It means platform holds the channel and needs to release it to agent(OSPM) in this case after initialisation. > The shmem.c shmem_tx_prepare will spin until channel free, so I did the patch. > Otherwise it might spin forever. > Yes I guessed that to be reason. > I'll add a check as following > if (tx) > iowrite32(BIT(0), &smbox->shmem->channel_status); > Not yet, I need answers to above query. > I not find a good place to put this in shmem.c (: > Least of the problem :), let us first agree if we have to have it. > > > > Also what about error fields ? I would rather clear it to 0, not just BIT(0) > > Tx channel error should also be cleared, fix in v2. > OK but wait for a while before you post for the discussion to end. -- Regards, Sudeep