Re: [PATCH 01/10] dt/bindings: Add binding for BCM2835 mailbox driver

[Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3297 bytes --]

Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Mon, 02 Mar 2015, Eric Anholt wrote:
>
>> From: Lubomir Rintel <lkundrak-NGH9Lh4a5iE@public.gmane.org>
>> 
>> v2: Split into a separate patch for submitting to the devicetree list.
>
> When you say that you've split them, do you mean each DT doc, as I
> don't think this is the way to go.  I'm happy to listen to other
> people's opinions, but I think all of the .../mailbox/brcm,bcm2835-
> files should be amalgamated.

I just meant that I split this patch out of the patch that followed
(because of review feeback and
Documentation/devicetree/bindings/submitting-patches.txt rules).

>> ---
>>  .../devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
>> 
>> 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..f5741a0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox.txt
>
> Rename these files to conform to the current naming convention.  In
> -next we currently have 'altera-mailbox.txt' and 'omap-mailbox.txt',
> so 'bcm2835-mbox.txt' seems appropriate.

Will do.

>> @@ -0,0 +1,19 @@
>> +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.
>
> Binding documents are much easier to read if the property names and
> descriptions are seperated with tabs.
>
> - compatible		: Should be "brcm,bcm2835-mbox"
> - reg			: Specifies base physical address and size of the registers
> - interrupts 		: The interrupt number
> 			  [See ../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
>
> ... don't you think?  Also notice the consistency of no '.'s and the
> bracketing of the [See ...] statement.

The tree seems inconsistent on formatting of these files, but I like
your suggestion for nicer formatting so I'll do it.

>> +Example:
>> +
>> +mailbox: mailbox@7e00b800 {
>> +	compatible = "brcm,bcm2835-mbox";
>> +	reg = <0x7e00b880 0x40>;
>> +	interrupts = <0 1>;
>> +	#mbox-cells = <1>;
>> +};
>
> It would be good to see the client examples here as well.  Please consider
> pulling in brcm,bcm2835-mbox-power.txt and brcm,bcm2835-mbox-property.txt.

Oh, so have those two just smashed into this file as one set of
documentation for everything to do with mailbox on bcm2835?  That seems
good to me.  When I was adding the client drivers, the fact that the
other brcm file was named after the compatible string made me generate
new files under then new compatible strings, but the other drivers
already in the tree obviously aren't formatted that way.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]