Re: [PATCH 2/6] Documentation: devicetree: add bindings to support ARM MHU subchannels

[Jassi Brar]

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.
  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.

>> +
>>  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.

Thanks.