All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/3 v7] mailbox: Enable BCM2835 mailbox support
Date: Thu, 30 Apr 2015 21:01:55 -0600	[thread overview]
Message-ID: <5542ECA3.8060104@wwwdotorg.org> (raw)
In-Reply-To: <CABb+yY0+Dd6RMJfmcqWrJfsSkPS8KjtpctJteoTZrj3QttJ71g@mail.gmail.com>

On 04/29/2015 11:26 PM, Jassi Brar wrote:
> On Wed, Apr 29, 2015 at 10:39 PM, Eric Anholt <eric@anholt.net> wrote:
> 
>> +
>> +struct bcm2835_mbox {
>> +       struct device *dev;
>> +       void __iomem *regs;
>> +       spinlock_t lock;
>> +       struct mbox_controller controller;
>> +};
>> +
>> +static struct bcm2835_mbox *mbox;
>> +
>> +static irqreturn_t bcm2835_mbox_irq(int irq, void *dev_id)
>> +{
>> +       struct device *dev = mbox->dev;
>> +       struct mbox_chan *link = &mbox->controller.chans[0];
>> +
> I learn from Stephen's other post that the controller could have
> multiple channels. In which case this driver is poorly setup. Actually
> if the driver was designed properly there isn't anything special to be
> done.
>  Here you choose to waste 'dev_id' and hard-code dereferencing to channel-0

Now that I look a bit harder at the registers, I think there are 2
mailbox register sets, but each is uni-directional, so a pair makes up
the typical bi-directional mailbox. There are multiple "owner" areas (or
sets of registers); I'm not quite sure what that implies.

As such, limiting this driver to a single mailbox is probably correct.

I would expect to see #mbox-cells=<0> in the DT, which would require a
custom of_xlate though (or modifying the default to work with a 0 cell
count; IIRC the default of_xlate for some other subsystems will work in
that scenario).

(As an aside, if we ever did find the need to expand the driver to cover
more mailboxes in the future, extending it and the DT to support
#mbox-cells=<0> or #mbox-cells=<1> at run-time should be trivial).

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Jassi Brar
	<jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Cc: "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Lee Jones <lee-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Devicetree List
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Craig McGeachie <slapdau-/E1597aS9LT0CCvOHzKKcA@public.gmane.org>,
	Lubomir Rintel <lkundrak-NGH9Lh4a5iE@public.gmane.org>,
	Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH 2/3 v7] mailbox: Enable BCM2835 mailbox support
Date: Thu, 30 Apr 2015 21:01:55 -0600	[thread overview]
Message-ID: <5542ECA3.8060104@wwwdotorg.org> (raw)
In-Reply-To: <CABb+yY0+Dd6RMJfmcqWrJfsSkPS8KjtpctJteoTZrj3QttJ71g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 04/29/2015 11:26 PM, Jassi Brar wrote:
> On Wed, Apr 29, 2015 at 10:39 PM, Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:
> 
>> +
>> +struct bcm2835_mbox {
>> +       struct device *dev;
>> +       void __iomem *regs;
>> +       spinlock_t lock;
>> +       struct mbox_controller controller;
>> +};
>> +
>> +static struct bcm2835_mbox *mbox;
>> +
>> +static irqreturn_t bcm2835_mbox_irq(int irq, void *dev_id)
>> +{
>> +       struct device *dev = mbox->dev;
>> +       struct mbox_chan *link = &mbox->controller.chans[0];
>> +
> I learn from Stephen's other post that the controller could have
> multiple channels. In which case this driver is poorly setup. Actually
> if the driver was designed properly there isn't anything special to be
> done.
>  Here you choose to waste 'dev_id' and hard-code dereferencing to channel-0

Now that I look a bit harder at the registers, I think there are 2
mailbox register sets, but each is uni-directional, so a pair makes up
the typical bi-directional mailbox. There are multiple "owner" areas (or
sets of registers); I'm not quite sure what that implies.

As such, limiting this driver to a single mailbox is probably correct.

I would expect to see #mbox-cells=<0> in the DT, which would require a
custom of_xlate though (or modifying the default to work with a 0 cell
count; IIRC the default of_xlate for some other subsystems will work in
that scenario).

(As an aside, if we ever did find the need to expand the driver to cover
more mailboxes in the future, extending it and the DT to support
#mbox-cells=<0> or #mbox-cells=<1> at run-time should be trivial).
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-05-01  3:01 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-27 22:27 [PATCH 0/3] BCM2835 core mailbox support Eric Anholt
2015-04-27 22:27 ` Eric Anholt
2015-04-27 22:27 ` [PATCH 1/3 v4] dt/bindings: Add binding for the BCM2835 mailbox driver Eric Anholt
2015-04-27 22:27   ` Eric Anholt
2015-04-28  5:21   ` Jassi Brar
2015-04-28  5:21     ` Jassi Brar
2015-04-28 20:44   ` [PATCH 1/3 v5] " Eric Anholt
2015-04-28 20:44     ` Eric Anholt
2015-04-29  1:30     ` Stephen Warren
2015-04-29  1:30       ` Stephen Warren
2015-04-29 16:39       ` Eric Anholt
2015-04-29 16:39         ` Eric Anholt
2015-04-29 23:41         ` Stephen Warren
2015-04-29 23:41           ` Stephen Warren
2015-04-30  4:53           ` Jassi Brar
2015-04-30  4:53             ` Jassi Brar
2015-04-27 22:27 ` [PATCH 2/3 v5] mailbox: Enable BCM2835 mailbox support Eric Anholt
2015-04-27 22:27   ` Eric Anholt
2015-04-28  5:43   ` Jassi Brar
2015-04-28  5:43     ` Jassi Brar
2015-04-28  9:50     ` Lubomir Rintel
2015-04-28  9:50       ` Lubomir Rintel
2015-04-28 19:49     ` Eric Anholt
2015-04-28 19:49       ` Eric Anholt
2015-04-29  2:07       ` Jassi Brar
2015-04-29  2:07         ` Jassi Brar
2015-04-29 17:05         ` Eric Anholt
2015-04-29 17:05           ` Eric Anholt
2015-04-28 20:54   ` [PATCH 2/3 v6] " Eric Anholt
2015-04-28 20:54     ` Eric Anholt
2015-04-29  1:39     ` Stephen Warren
2015-04-29  1:39       ` Stephen Warren
2015-04-29 17:09     ` [PATCH 2/3 v7] " Eric Anholt
2015-04-29 17:09       ` Eric Anholt
2015-04-30  5:26       ` Jassi Brar
2015-04-30  5:26         ` Jassi Brar
2015-05-01  3:01         ` Stephen Warren [this message]
2015-05-01  3:01           ` Stephen Warren
2015-05-05  2:18           ` Jassi Brar
2015-05-05  2:18             ` Jassi Brar
2015-05-05  0:57         ` Eric Anholt
2015-05-05  0:57           ` Eric Anholt
2015-05-05  2:30           ` Jassi Brar
2015-05-05  2:30             ` Jassi Brar
2015-05-05 19:32             ` Stephen Warren
2015-05-05 19:32               ` Stephen Warren
2015-04-27 22:27 ` [PATCH 3/3] ARM: bcm2835: Add the mailbox to the device tree Eric Anholt
2015-04-27 22:27   ` Eric Anholt

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=5542ECA3.8060104@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.