diff mbox

[1/3,v5] dt/bindings: Add binding for the BCM2835 mailbox driver

Message ID 1430253868-11138-1-git-send-email-eric@anholt.net
State Superseded, archived
Headers show

Commit Message

Eric Anholt April 28, 2015, 8:44 p.m. UTC
From: Lubomir Rintel <lkundrak@v3.sk>

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Signed-off-by: Craig McGeachie <slapdau@yahoo.com.au>
Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---

v2: Split into a separate patch for submitting to the devicetree list.
    Consistently start node docs with a capital letter. device's
    address in the example shouldn't have "0x". Drop machine-specific
    interrupt numbers from the docs.  (changes by anholt).

v3: Move the file to just bcm2835-mbox.txt, clean up formatting
    (changes by anholt, from review by Lee Jones).

v4: Move file back by consensus from various Broadcom platform
    maintainers (changes by anholt, acked by Lee Jones).

v5: Document that the mailbox cell should be 0 in clients, and add an
    example of a client (changes by anholt, from review by Jassi).

 .../bindings/mailbox/brcm,bcm2835-mbox.txt         | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt

Comments

Stephen Warren April 29, 2015, 1:30 a.m. UTC | #1
On 04/28/2015 02:44 PM, Eric Anholt wrote:
> From: Lubomir Rintel <lkundrak@v3.sk>
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Signed-off-by: Craig McGeachie <slapdau@yahoo.com.au>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
> 
> v2: Split into a separate patch for submitting to the devicetree list.
>     Consistently start node docs with a capital letter. device's
>     address in the example shouldn't have "0x". Drop machine-specific
>     interrupt numbers from the docs.  (changes by anholt).
> 
> v3: Move the file to just bcm2835-mbox.txt, clean up formatting
>     (changes by anholt, from review by Lee Jones).
> 
> v4: Move file back by consensus from various Broadcom platform
>     maintainers (changes by anholt, acked by Lee Jones).
> 
> v5: Document that the mailbox cell should be 0 in clients, and add an
>     example of a client (changes by anholt, from review by Jassi).

Some mention of what you changed in the patch might be nice, since it
was originally authored by someone else, and there's quite a changelog.

> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt

> +- #mbox-cells:	Specifies the number of cells needed to encode a mailbox
> +		  channel. The value shall be 1.  Clients should supply a value
> +		  of 0 for the mailbox channel, because there is only one
> +		  exposed

Can't you use #mbox-cells = <0> if there's no data to provide?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Anholt April 29, 2015, 4:39 p.m. UTC | #2
Stephen Warren <swarren@wwwdotorg.org> writes:

> On 04/28/2015 02:44 PM, Eric Anholt wrote:
>> From: Lubomir Rintel <lkundrak@v3.sk>
>> 
>> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>> Signed-off-by: Craig McGeachie <slapdau@yahoo.com.au>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>> ---
>> 
>> v2: Split into a separate patch for submitting to the devicetree list.
>>     Consistently start node docs with a capital letter. device's
>>     address in the example shouldn't have "0x". Drop machine-specific
>>     interrupt numbers from the docs.  (changes by anholt).
>> 
>> v3: Move the file to just bcm2835-mbox.txt, clean up formatting
>>     (changes by anholt, from review by Lee Jones).
>> 
>> v4: Move file back by consensus from various Broadcom platform
>>     maintainers (changes by anholt, acked by Lee Jones).
>> 
>> v5: Document that the mailbox cell should be 0 in clients, and add an
>>     example of a client (changes by anholt, from review by Jassi).
>
> Some mention of what you changed in the patch might be nice, since it
> was originally authored by someone else, and there's quite a changelog.

I've been taking this changelog stuff out of the patch and putting it
below '---' because when I was submitting this series before, I got
chided for putting it in the patch!

>> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
>
>> +- #mbox-cells:	Specifies the number of cells needed to encode a mailbox
>> +		  channel. The value shall be 1.  Clients should supply a value
>> +		  of 0 for the mailbox channel, because there is only one
>> +		  exposed
>
> Can't you use #mbox-cells = <0> if there's no data to provide?

of_mbox_index_xlate looks like it's always dereferencing the first arg.
Stephen Warren April 29, 2015, 11:41 p.m. UTC | #3
On 04/29/15 10:39, Eric Anholt wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
>
>> On 04/28/2015 02:44 PM, Eric Anholt wrote:
>>> From: Lubomir Rintel <lkundrak@v3.sk>
>>>
>>> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>>> Signed-off-by: Craig McGeachie <slapdau@yahoo.com.au>
>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>>> ---
>>>
>>> v2: Split into a separate patch for submitting to the devicetree list.
>>>      Consistently start node docs with a capital letter. device's
>>>      address in the example shouldn't have "0x". Drop machine-specific
>>>      interrupt numbers from the docs.  (changes by anholt).
>>>
>>> v3: Move the file to just bcm2835-mbox.txt, clean up formatting
>>>      (changes by anholt, from review by Lee Jones).
>>>
>>> v4: Move file back by consensus from various Broadcom platform
>>>      maintainers (changes by anholt, acked by Lee Jones).
>>>
>>> v5: Document that the mailbox cell should be 0 in clients, and add an
>>>      example of a client (changes by anholt, from review by Jassi).
>>
>> Some mention of what you changed in the patch might be nice, since it
>> was originally authored by someone else, and there's quite a changelog.
>
> I've been taking this changelog stuff out of the patch and putting it
> below '---' because when I was submitting this series before, I got
> chided for putting it in the patch!

A changelog and a description of the changes you've made aren't quite 
the same thing.

There should be some mention in the commit description of what you 
changed relative to the patch that someone else authored, so they don't 
get blaimed/praised for any of your changes. The format of that 
description wouldn't usually be a detailed patch revision changelog 
though, just a description of the overall changes that were left in the 
final version.

>>> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
>>
>>> +- #mbox-cells:	Specifies the number of cells needed to encode a mailbox
>>> +		  channel. The value shall be 1.  Clients should supply a value
>>> +		  of 0 for the mailbox channel, because there is only one
>>> +		  exposed
>>
>> Can't you use #mbox-cells = <0> if there's no data to provide?
>
> of_mbox_index_xlate looks like it's always dereferencing the first arg.

Is it possible to fix that?

That said, last I looked at the HW I thought there were actually 
multiple channels possible, so perhaps having a cell for future 
expansion is reasonable.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jassi Brar April 30, 2015, 4:53 a.m. UTC | #4
On Thu, Apr 30, 2015 at 5:11 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/29/15 10:39, Eric Anholt wrote:
>>
>> Stephen Warren <swarren@wwwdotorg.org> writes:
>>
>>> On 04/28/2015 02:44 PM, Eric Anholt wrote:
>>>>
>>>> From: Lubomir Rintel <lkundrak@v3.sk>
>>>>
>>>> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>>>> Signed-off-by: Craig McGeachie <slapdau@yahoo.com.au>
>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>>>> ---
>>>>
>>>> v2: Split into a separate patch for submitting to the devicetree list.
>>>>      Consistently start node docs with a capital letter. device's
>>>>      address in the example shouldn't have "0x". Drop machine-specific
>>>>      interrupt numbers from the docs.  (changes by anholt).
>>>>
>>>> v3: Move the file to just bcm2835-mbox.txt, clean up formatting
>>>>      (changes by anholt, from review by Lee Jones).
>>>>
>>>> v4: Move file back by consensus from various Broadcom platform
>>>>      maintainers (changes by anholt, acked by Lee Jones).
>>>>
>>>> v5: Document that the mailbox cell should be 0 in clients, and add an
>>>>      example of a client (changes by anholt, from review by Jassi).
>>>
>>>
>>> Some mention of what you changed in the patch might be nice, since it
>>> was originally authored by someone else, and there's quite a changelog.
>>
>>
>> I've been taking this changelog stuff out of the patch and putting it
>> below '---' because when I was submitting this series before, I got
>> chided for putting it in the patch!
>
>
> A changelog and a description of the changes you've made aren't quite the
> same thing.
>
> There should be some mention in the commit description of what you changed
> relative to the patch that someone else authored, so they don't get
> blaimed/praised for any of your changes. The format of that description
> wouldn't usually be a detailed patch revision changelog though, just a
> description of the overall changes that were left in the final version.
>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
>>>> b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
>>>
>>>
>>>> +- #mbox-cells: Specifies the number of cells needed to encode a mailbox
>>>> +                 channel. The value shall be 1.  Clients should supply
>>>> a value
>>>> +                 of 0 for the mailbox channel, because there is only
>>>> one
>>>> +                 exposed
>>>
>>>
>>> Can't you use #mbox-cells = <0> if there's no data to provide?
>>
>>
>> of_mbox_index_xlate looks like it's always dereferencing the first arg.
>
>
> Is it possible to fix that?
>
The driver specific of_xlate() is meant to be provided during
mbox_controller_register() in which case the default 1-cell xlate
won't be used.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
new file mode 100644
index 0000000..731de34
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
@@ -0,0 +1,27 @@ 
+Broadcom BCM2835 VideoCore mailbox IPC
+
+Required properties:
+
+- compatible:	Should be "brcm,bcm2835-mbox"
+- reg:		Specifies base physical address and size of the registers
+- interrupts:	The interrupt number
+		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+- #mbox-cells:	Specifies the number of cells needed to encode a mailbox
+		  channel. The value shall be 1.  Clients should supply a value
+		  of 0 for the mailbox channel, because there is only one
+		  exposed
+
+Example:
+
+mailbox: mailbox@7e00b800 {
+	compatible = "brcm,bcm2835-mbox";
+	reg = <0x7e00b880 0x40>;
+	interrupts = <0 1>;
+	#mbox-cells = <1>;
+};
+
+firmware: firmware {
+	compatible = "raspberrypi,firmware";
+	mboxes = <&mailbox 0>;
+	#power-domain-cells = <1>;
+};