linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/3] Enable BCM2835 mailbox support
Date: Tue, 01 Oct 2013 20:50:44 -0600	[thread overview]
Message-ID: <524B8A04.1020006@wwwdotorg.org> (raw)
In-Reply-To: <5232F7B7.4070206@yahoo.com.au>

On 09/13/2013 05:32 AM, Craig McGeachie wrote:
> Cherry-picked selection of commits by Lubomir Rintel [1], Suman Anna and
> Jassi Brar [2] on which to base the implementation.
> 
> Signed-off-by: Suman Ana <s-anna@ti.com>
> Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> [1] http://lists.infradead.org/pipermail/linux-rpi-kernel/2013-April/000528.html
> [2] http://lists.infradead.org/pipermail/linux-rpi-kernel/2013-May/000546.html

This needs a proper patch description. It should describe what the
mailbox HW is, what services it provides, etc.

There shouldn't be any text after the tag paragraph (i.e. the s-o-b
lines) and the --- separator.

Are you able to generate patches using "git format-patch". That would
included diffstat in the patch summary, which can be useful.

I'd prefer to see patch 1 and 2 squashed together, since the new mailbox
driver should just be implemented in one shot, rather than added with
one implementation, then reworked to be different in a separate patch.

That said, the patches need to be split up based on subsystem divisions.
The squashed patch 1 and 2, and the separate patch 3, should be split into:

a) A patch that adds the driver. This would touch just
drivers/mailbox/... (or drivers/thermal/...) and
Documentation/devicetree/bindings/...

b) A patch that adds the necessary entries to *.dts/*.dtsi

c) A patch that enables any new entries in bcm2835_defconfig.

In my initial round of review, I'll only discuss the DT bindings, and
only in response to patch 2 where they're complete rather than here.
Once the bindings are finalized and the driver adjusted to implement the
final bindings, then I'll look at the driver code.

Note that all new DT bindings should be CC'd to the DT bindings
maintainers; see the following in MAINTAINERS:

OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
M:      Rob Herring <rob.herring@calxeda.com>
M:      Pawel Moll <pawel.moll@arm.com>
M:      Mark Rutland <mark.rutland@arm.com>
M:      Stephen Warren <swarren@wwwdotorg.org>
M:      Ian Campbell <ijc+devicetree@hellion.org.uk>
L:      devicetree at vger.kernel.org
S:      Maintained
F:      Documentation/devicetree/
F:      arch/*/boot/dts/
F:      include/dt-bindings/

> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
> index 1e12aef..4fc8a68 100644
> --- a/arch/arm/boot/dts/bcm2835.dtsi
> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> @@ -29,6 +29,12 @@
>  			#interrupt-cells = <2>;
>  		};

There needs to be a blank line here.

>  +		mailbox {

Patch corruption? + is indented!

  parent reply	other threads:[~2013-10-02  2:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-13 11:32 [RFC PATCH 1/3] Enable BCM2835 mailbox support Craig McGeachie
2013-09-15  3:28 ` Craig McGeachie
2013-09-15  4:22   ` Jassi Brar
2013-10-02  2:50 ` Stephen Warren [this message]
2013-10-02  9:18   ` Craig McGeachie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=524B8A04.1020006@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).