From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758168AbdJMNmS (ORCPT ); Fri, 13 Oct 2017 09:42:18 -0400 Received: from foss.arm.com ([217.140.101.70]:33346 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753258AbdJMNmQ (ORCPT ); Fri, 13 Oct 2017 09:42:16 -0400 Cc: Sudeep Holla , Jassi Brar , Arnd Bergmann , ALKML , LKML , DTML , Roy Franz , Harb Abdulhamid , Nishanth Menon , Loc Ho , Alexey Klimov , Ryan Harkin Subject: Re: [PATCH v3 16/22] firmware: arm_scmi: add arm_mhu specific mailbox interface To: Bjorn Andersson References: <1506604306-20739-1-git-send-email-sudeep.holla@arm.com> <1506604306-20739-17-git-send-email-sudeep.holla@arm.com> <25023d1a-a221-b131-5a1e-7d909b006d1b@arm.com> From: Sudeep Holla Organization: ARM Message-ID: <0b65a3fd-32ff-f708-7dce-8e3a5cabf908@arm.com> Date: Fri, 13 Oct 2017 14:42:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Bjorn, Thanks for taking a look at this. Much appreciated. On 12/10/17 22:03, Bjorn Andersson wrote: > On Fri, Oct 6, 2017 at 6:51 AM, Sudeep Holla wrote: >> >> >> On 06/10/17 14:47, Jassi Brar wrote: >>> On Fri, Oct 6, 2017 at 7:02 PM, Sudeep Holla wrote: > [..] >>>> Again that's not the point, doorbell is more common feature and that can >>>> be supported. As SCMI expects doorbell feature in the specification, it >>>> just need to support that class of controllers. >>>> >>> NO. All SCMI expects is SHMEM and a signal reaching the other end. >>> The signal mechanism need not necessarily be "doorbell". >>> >> >> Agreed, but creating an abstraction ro do something as generic as >> doorbell and writing shim layer for each controller to use SCMI also >> sounds bad. >> > > In the Qualcomm platform we have a single register that exposes 32 > doorbells, wired to interrupts on the various processors/co-processors > in the SoC. > It's exactly same even on ARM MHU controller. The hardware provides 32 independent bits in a single register termed as a single physical channel. We may have 2 -3 instances of it in a platform(secure only, high priority and a low priority). However the single register is being used to pass 32-bit data on some platforms. So we may need to support both modes. One way to deal with that is to add doorbell support in the driver as I tried previously. But the mailbox maintainer has expressed concerns on that and hence I am trying to achieve that with the abstraction as in this patch. > There is a handful of different clients each using these doorbells to > inform the other side that something has happened (often that some > piece of shared memory has been filled with data). Over the years > (platforms) this register has moved around as such multiple > implementations exists. > > You can see an example of this in > drivers/mailbox/qcom-apcs-ipc-mailbox.c and e.g. > drivers/rpmsg/qcom_glink_native.c (qcom_glink_rpm.c prior to v4.14). > > Thanks for the pointer, I have already looked at these implementations. > > I'm struggling to figure out exactly how your case looks like, but if > your doorbell exists in only one instance and there is only one client > then I would suggest that it's overkill to create another driver for > it. > While I agree with your opinion, the maintainer has concerns on trying to make this a generic solution. -- Regards, Sudeep From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sudeep Holla Subject: Re: [PATCH v3 16/22] firmware: arm_scmi: add arm_mhu specific mailbox interface Date: Fri, 13 Oct 2017 14:42:11 +0100 Message-ID: <0b65a3fd-32ff-f708-7dce-8e3a5cabf908@arm.com> References: <1506604306-20739-1-git-send-email-sudeep.holla@arm.com> <1506604306-20739-17-git-send-email-sudeep.holla@arm.com> <25023d1a-a221-b131-5a1e-7d909b006d1b@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Bjorn Andersson Cc: Sudeep Holla , Jassi Brar , Arnd Bergmann , ALKML , LKML , DTML , Roy Franz , Harb Abdulhamid , Nishanth Menon , Loc Ho , Alexey Klimov , Ryan Harkin List-Id: devicetree@vger.kernel.org Hi Bjorn, Thanks for taking a look at this. Much appreciated. On 12/10/17 22:03, Bjorn Andersson wrote: > On Fri, Oct 6, 2017 at 6:51 AM, Sudeep Holla wrote: >> >> >> On 06/10/17 14:47, Jassi Brar wrote: >>> On Fri, Oct 6, 2017 at 7:02 PM, Sudeep Holla wrote: > [..] >>>> Again that's not the point, doorbell is more common feature and that can >>>> be supported. As SCMI expects doorbell feature in the specification, it >>>> just need to support that class of controllers. >>>> >>> NO. All SCMI expects is SHMEM and a signal reaching the other end. >>> The signal mechanism need not necessarily be "doorbell". >>> >> >> Agreed, but creating an abstraction ro do something as generic as >> doorbell and writing shim layer for each controller to use SCMI also >> sounds bad. >> > > In the Qualcomm platform we have a single register that exposes 32 > doorbells, wired to interrupts on the various processors/co-processors > in the SoC. > It's exactly same even on ARM MHU controller. The hardware provides 32 independent bits in a single register termed as a single physical channel. We may have 2 -3 instances of it in a platform(secure only, high priority and a low priority). However the single register is being used to pass 32-bit data on some platforms. So we may need to support both modes. One way to deal with that is to add doorbell support in the driver as I tried previously. But the mailbox maintainer has expressed concerns on that and hence I am trying to achieve that with the abstraction as in this patch. > There is a handful of different clients each using these doorbells to > inform the other side that something has happened (often that some > piece of shared memory has been filled with data). Over the years > (platforms) this register has moved around as such multiple > implementations exists. > > You can see an example of this in > drivers/mailbox/qcom-apcs-ipc-mailbox.c and e.g. > drivers/rpmsg/qcom_glink_native.c (qcom_glink_rpm.c prior to v4.14). > > Thanks for the pointer, I have already looked at these implementations. > > I'm struggling to figure out exactly how your case looks like, but if > your doorbell exists in only one instance and there is only one client > then I would suggest that it's overkill to create another driver for > it. > While I agree with your opinion, the maintainer has concerns on trying to make this a generic solution. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: sudeep.holla@arm.com (Sudeep Holla) Date: Fri, 13 Oct 2017 14:42:11 +0100 Subject: [PATCH v3 16/22] firmware: arm_scmi: add arm_mhu specific mailbox interface In-Reply-To: References: <1506604306-20739-1-git-send-email-sudeep.holla@arm.com> <1506604306-20739-17-git-send-email-sudeep.holla@arm.com> <25023d1a-a221-b131-5a1e-7d909b006d1b@arm.com> Message-ID: <0b65a3fd-32ff-f708-7dce-8e3a5cabf908@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Bjorn, Thanks for taking a look at this. Much appreciated. On 12/10/17 22:03, Bjorn Andersson wrote: > On Fri, Oct 6, 2017 at 6:51 AM, Sudeep Holla wrote: >> >> >> On 06/10/17 14:47, Jassi Brar wrote: >>> On Fri, Oct 6, 2017 at 7:02 PM, Sudeep Holla wrote: > [..] >>>> Again that's not the point, doorbell is more common feature and that can >>>> be supported. As SCMI expects doorbell feature in the specification, it >>>> just need to support that class of controllers. >>>> >>> NO. All SCMI expects is SHMEM and a signal reaching the other end. >>> The signal mechanism need not necessarily be "doorbell". >>> >> >> Agreed, but creating an abstraction ro do something as generic as >> doorbell and writing shim layer for each controller to use SCMI also >> sounds bad. >> > > In the Qualcomm platform we have a single register that exposes 32 > doorbells, wired to interrupts on the various processors/co-processors > in the SoC. > It's exactly same even on ARM MHU controller. The hardware provides 32 independent bits in a single register termed as a single physical channel. We may have 2 -3 instances of it in a platform(secure only, high priority and a low priority). However the single register is being used to pass 32-bit data on some platforms. So we may need to support both modes. One way to deal with that is to add doorbell support in the driver as I tried previously. But the mailbox maintainer has expressed concerns on that and hence I am trying to achieve that with the abstraction as in this patch. > There is a handful of different clients each using these doorbells to > inform the other side that something has happened (often that some > piece of shared memory has been filled with data). Over the years > (platforms) this register has moved around as such multiple > implementations exists. > > You can see an example of this in > drivers/mailbox/qcom-apcs-ipc-mailbox.c and e.g. > drivers/rpmsg/qcom_glink_native.c (qcom_glink_rpm.c prior to v4.14). > > Thanks for the pointer, I have already looked at these implementations. > > I'm struggling to figure out exactly how your case looks like, but if > your doorbell exists in only one instance and there is only one client > then I would suggest that it's overkill to create another driver for > it. > While I agree with your opinion, the maintainer has concerns on trying to make this a generic solution. -- Regards, Sudeep