From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751213AbdFBJdD (ORCPT ); Fri, 2 Jun 2017 05:33:03 -0400 Received: from foss.arm.com ([217.140.101.70]:37706 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751125AbdFBJdB (ORCPT ); Fri, 2 Jun 2017 05:33:01 -0400 Cc: Sudeep Holla , Linux Kernel Mailing List , Devicetree List , Alexey Klimov , Jassi Brar Subject: Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells To: Jassi Brar , Rob Herring References: <1495621003-4291-1-git-send-email-sudeep.holla@arm.com> <1495621003-4291-3-git-send-email-sudeep.holla@arm.com> <20170531170801.u6d4fhsfu5mmqgta@rob-hp-laptop> From: Sudeep Holla Organization: ARM Message-ID: <9a38040c-8062-870e-ab2a-beb4934428b3@arm.com> Date: Fri, 2 Jun 2017 10:32:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 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 On 02/06/17 06:45, Jassi Brar wrote: > Hi Rob, > > On Wed, May 31, 2017 at 10:38 PM, Rob Herring wrote: >> On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >>> >>>>> .../devicetree/bindings/mailbox/arm-mhu.txt | 46 ++++++++++++++++++++-- >>>>> 1 file changed, 43 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >>>>> index 4971f03f0b33..bd9a3a267caf 100644 >>>>> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >>>>> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >>>>> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read the data. >>>>> The last channel is specified to be a 'Secure' resource, hence can't be >>>>> used by Linux running NS. >>>>> >>>>> +The MHU drives the interrupt signal using a 32-bit register, with all >>>>> +32-bits logically ORed together. It provides a set of registers to >>>>> +enable software to set, clear and check the status of each of the bits >>>>> +of this register independently. The use of 32 bits per interrupt line >>>>> +enables software to provide more information about the source of the >>>>> +interrupt. For example, each bit of the register can be associated with >>>>> +a type of event that can contribute to raising the interrupt. Each of >>>>> +the 32-bits can be used as "doorbell" to alert the remote processor. >>>>> + >>>>> Mailbox Device Node: >>>>> ==================== >>>>> >>>>> Required properties: >>>>> -------------------- >>>>> -- compatible: Shall be "arm,mhu" & "arm,primecell" >>>>> +- compatible: Shall be "arm,primecell" and one of the below: >>>>> + "arm,mhu" - if the controller doesn't support >>>>> + doorbell model >>>>> + "arm,mhu-doorbell" - if the controller supports >>>>> + doorbell model >>>>> - reg: Contains the mailbox register address range (base >>>>> address and length) >>>>> -- #mbox-cells Shall be 1 - the index of the channel needed. >>>>> +- #mbox-cells Shall be 1 - the index of the channel needed when >>>>> + compatible is "arm,mhu" >>>>> + Shall be 2 - the index of the channel needed, and >>>>> + the index of the doorbell bit with the channel when >>>>> + compatible is "arm,mhu-doorbell" >>>>> - interrupts: Contains the interrupt information corresponding to >>>>> - each of the 3 links of MHU. >>>>> + each of the 3 physical channels of MHU namely low >>>>> + priority non-secure, high priority non-secure and >>>>> + secure channels. >>>>> >>>>> Example: >>>>> -------- >>>>> >>>>> +1. Controller which doesn't support doorbells >>>>> + >>>>> mhu: mailbox@2b1f0000 { >>>>> #mbox-cells = <1>; >>>>> compatible = "arm,mhu", "arm,primecell"; >>>>> @@ -41,3 +62,22 @@ used by Linux running NS. >>>>> reg = <0 0x2e000000 0x4000>; >>>>> mboxes = <&mhu 1>; /* HP-NonSecure */ >>>>> }; >>>>> + >>>>> +2. Controller which supports doorbells >>>>> + >>>>> + mhu: mailbox@2b1f0000 { >>>>> + #mbox-cells = <2>; >>>>> + compatible = "arm,mhu-doorbell", "arm,primecell"; >>>>> + reg = <0 0x2b1f0000 0x1000>; >>>>> + interrupts = <0 36 4>, /* LP-NonSecure */ >>>>> + <0 35 4>, /* HP-NonSecure */ >>>>> + <0 37 4>; /* Secure */ >>>>> + clocks = <&clock 0 2 1>; >>>>> + clock-names = "apb_pclk"; >>>>> + }; >>>>> + >>>>> + mhu_client: scb@2e000000 { >>>>> + compatible = "arm,scpi"; >>>>> + reg = <0 0x2e000000 0x200>; >>>>> + mboxes = <&mhu 1 4>; /* HP-NonSecure 5th doorbell bit */ >>>>> + }; >>>>> >>>> Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" >>>> equally fine. So you are basically smuggling a s/w feature into DT. >>>> >>> >>> I disagree, the spec clearly says each bit can be used for different >>> event and hence we need a way to specify that in DT when used as doorbell. >> >> I think the point is that you should not continue to use both. The >> > FYKI my point (here and in other threads) is that current MHU > driver/bindings can very well support Sudeep's usecase. > >> single cell usage should be deprecated. Maybe you'll have to encode the >> 2nd cell when not used as 0 means bit 0? >> >> Arguably, you don't even need a new compatible. #mbox-cells tells you >> whether to use the old or new binding. I'm fine either way though. >> > In mailbox terminology, there is a channel and command(s) that we > send/recv over it. OOB data passing is optional. > > MHU driver accepts a command as a u32. Sudeep's usecase has commands > of the form of BIT(x) ... an instance of u32. > Again read the spec, it is designed to be used as doorbell > Instead of having his client driver pass BIT(x) like other MHU Client is generic and doesn't understand which controller it's working with. It's not custom protocol. I have given enough links and details to say it's generic protocol adapted by other vendors. > clients, he wants to modify the driver and bindings to specify 'x' via > second cell of mboxes. That would have made sense (and already > implemented) if the controller could send only 1 bit at a time and > every client/protocol used exactly 1 command - both of which are not > true. Yes but the controller is designed to provide single bit doorbell capability. Please understand that instead of pushing how you think it needs to work. > > I even offered Sudeep to share his client driver and I'll adapt it for > him, but her refuses to either see my point or share his code. > You have not answered my queries, you keep telling I can help but never read and answer what I ask. Also I have told you the driver is in mainline(drivers/firmware/arm_scpi.c) and there will be another similar one. And theres are used by different platforms with different mailbox controllers. Please post patches to make 2 different protocols like SCPI work. Please note these protocols are used by other platforms which have different controllers. -- Regards, Sudeep