Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1650501 > unrolled thread

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

Started bySudeep Holla <sudeep.holla@arm.com>
First post2017-05-25 15:30 +0200
Last post2017-06-02 11:40 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support  ARM MHU doorbells Sudeep Holla <sudeep.holla@arm.com> - 2017-05-25 15:30 +0200
    Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to  support ARM MHU doorbells Rob Herring <robh@kernel.org> - 2017-05-31 19:20 +0200
      Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support  ARM MHU doorbells Sudeep Holla <sudeep.holla@arm.com> - 2017-05-31 19:20 +0200
      Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support  ARM MHU doorbells Jassi Brar <jassisinghbrar@gmail.com> - 2017-06-02 07:50 +0200
        Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support  ARM MHU doorbells Sudeep Holla <sudeep.holla@arm.com> - 2017-06-02 11:40 +0200

#1650501 — Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-05-25 15:30 +0200
SubjectRe: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
Message-ID<tL0ZY-iP-7@gated-at.bofh.it>

On 25/05/17 14:22, Jassi Brar wrote:
> On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla <sudeep.holla@arm.com> 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 doorbells.
>>
>> Since the first version of this binding can't support doorbells,
>> this patch extends the existing binding to support them.
>>
>> Cc: Alexey Klimov <alexey.klimov@arm.com>
>> Cc: Jassi Brar <jaswinder.singh@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  .../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.

-- 
Regards,
Sudeep

[toc] | [next] | [standalone]


#1654470 — Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells

FromRob Herring <robh@kernel.org>
Date2017-05-31 19:20 +0200
SubjectRe: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
Message-ID<tNfrQ-7Ij-5@gated-at.bofh.it>
In reply to#1650501
On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote:
> 
> 
> On 25/05/17 14:22, Jassi Brar wrote:
> > On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla <sudeep.holla@arm.com> 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 doorbells.
> >>
> >> Since the first version of this binding can't support doorbells,
> >> this patch extends the existing binding to support them.
> >>
> >> Cc: Alexey Klimov <alexey.klimov@arm.com>
> >> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Cc: devicetree@vger.kernel.org
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >> ---
> >>  .../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 
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.

Rob

[toc] | [prev] | [next] | [standalone]


#1654476

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-05-31 19:20 +0200
Message-ID<tNfrR-7Ij-23@gated-at.bofh.it>
In reply to#1654470

On 31/05/17 18:08, Rob Herring wrote:
> On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote:
>>
>>
>> On 25/05/17 14:22, Jassi Brar wrote:

[...]

>>> 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 
> single cell usage should be deprecated. Maybe you'll have to encode the 
> 2nd cell when not used as 0 means bit 0?
> 

Instead of having special encoding, I like your below suggestion on
using #mbox-cells to distinguish the usage modes.

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

Ah good point, yes we can distinguish with #mbox-cells. I will drop the
new compatible.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]


#1655910

FromJassi Brar <jassisinghbrar@gmail.com>
Date2017-06-02 07:50 +0200
Message-ID<tNNDb-4UD-3@gated-at.bofh.it>
In reply to#1654470
Hi Rob,

On Wed, May 31, 2017 at 10:38 PM, Rob Herring <robh@kernel.org> 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.

Instead of having his client driver pass BIT(x) like other MHU
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.

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.

Cheers!

[toc] | [prev] | [next] | [standalone]


#1656068

FromSudeep Holla <sudeep.holla@arm.com>
Date2017-06-02 11:40 +0200
Message-ID<tNRdM-7Cl-9@gated-at.bofh.it>
In reply to#1655910

On 02/06/17 06:45, Jassi Brar wrote:
> Hi Rob,
> 
> On Wed, May 31, 2017 at 10:38 PM, Rob Herring <robh@kernel.org> 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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web