On Mon, May 8, 2017 at 10:37 PM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote: > > > On 08/05/17 17:46, Jassi Brar wrote: >> On Mon, May 8, 2017 at 9:40 PM, Rob Herring <robh@xxxxxxxxxx> wrote: >>> +Bjorn >>> >>> On Tue, May 02, 2017 at 02:55:49PM +0100, Sudeep Holla wrote: >>>> The ARM MHU has mechanism to assert interrupt signals to facilitate >>>> inter-processor message based communication. It drives the signal using >>>> a 32-bit register, with all 32-bits logically ORed together. It also >>>> enables software to set, clear and check the status of each of the bits >>>> of this register independently. Each bit of the register can be >>>> associated with a type of event that can contribute to raising the >>>> interrupt thereby allowing it to be used as independent subchannels. >>>> >>>> Since the first version of this binding can't support sub-channels, >>>> this patch extends the existing binding to support them. >>>> >>>> Cc: Alexey Klimov <alexey.klimov@xxxxxxx> >>>> Cc: Jassi Brar <jaswinder.singh@xxxxxxxxxx> >>>> Cc: Rob Herring <robh+dt@xxxxxxxxxx> >>>> Cc: devicetree@xxxxxxxxxxxxxxx >>>> Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx> >>>> --- >>>> .../devicetree/bindings/mailbox/arm-mhu.txt | 44 ++++++++++++++++++++-- >>>> 1 file changed, 41 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >>>> index 4971f03f0b33..86a66f7918e2 100644 >>>> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >>>> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >>>> @@ -10,21 +10,40 @@ 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. >>> >>> Sounds like a doorbell? (i.e. a single bit mailbox). Bjorn is doing >>> something similar for QCom h/w. I guess the difference here is you have >>> 32 sources and 1 output. It seems to me these should be described >>> similarly. >>> >> Yes, QCom controller triggers different interrupt for each bit of a >> 32bits register i.e, each signal is associated with 1bit information. >> Whereas MHU signals 32bits at a time to the target cpu. > > Agreed. I had a look at Qcom driver, not entirely clear if each bit as > interrupt as I don't see any interrupt support there. Also, it just adds > all the 32 channels which I am trying to avoid as at-most 4-5 will be > used while we end up creating 64 channels. > OK, so you just need to use 4 singles bits. That is, 4 different commands to remote. #define SCMI_CMD_1 BIT(a) #define SCMI_CMD_2 BIT(b) #define SCMI_CMD_3 BIT(c) #define SCMI_CMD_4 BIT(d) >> Both these cases are already supported by mailbox framework, so >> Bjorn has implemented QCom's 'doorbell' driver over mailbox api. And >> we can do without this "arm,mhu-v2" driver. I believe Sudeep already >> knows well how to use the MHU driver as such to get what his client >> drivers need. >> > > As I mentioned above one reason for adding the complexity is avoiding > creation of 32*2 channels. Secondly we still need a way to distinguish > between the 2 use-cases(existing and new one). Any thoughts ? > I say, your usecase is an instance of the supported usecases by the existing driver. Just send the BIT(x) as a 32-bit value. Remote doesn't even need to find which bit is set, it can simply switch-case the value it got against SCMI_CMD_[1,4] >>>> + >>>> 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 >>>> + subchannels >>>> + "arm,mhu-v2" - if the controller supports subchannels >>> >>> How do I know if I have v2? This correlates to an IP version or >>> IP configuration or ? >>> >> This is purely a software concept - virtual channel. There are only 3 >> physical channels and that are managed by existing version of driver & >> bindings. This is another reason I am against this patchset. >> > > I understand your concern. Please suggest alternative if we need to use > each bit in the single set register as a different doorbell ? > Your commands are encoded as a simple BIT(x) which is an instance of u32 If you want to send 2 commands together to remote, even that is just as simple... send BIT(a) | BIT(b). The remote will figure which bits are set and take action priority wise. [[ BTW, today you may need only 4bits because you have only 4 different commands. What if the command set grows beyond 32bits? The perfectly capable h/w will be rendered useless just because of s/w design decisions. So instead of assigning BIT(x) type command codes, please consider using full range of u32. Remote does not need it to be BIT(x) but just a unique number. The same "doorbell" will ring and the remote will use switch-case to figure who is it. ]] > We need > this from DT as we need to specify each bit as a channel for different > client. > The client DT node could carry the command code (as a u32 value) that it is going to work with. >The specification clearly states each bit can be used as a > doorbell. > Doorbell and Mailbox are the same thing. Its just that we are used to pass data packets over shared-memory that we think mailbox is different. It is perfectly possible to not need shared-memory, if command+data can be encoded within 32bits. In that case you would call Mailbox a 32bit Doorbell :) For example, PL320 has 8 32bit registers that can carry data for remote. If it is still not clear, please share your client driver. I will adapt that to work with existing MHU driver & bindings. Cheers! -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
