All of lore.kernel.org
 help / color / mirror / Atom feed
From: dave.martin@linaro.org (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] drivers: bus: add ARM CCI support
Date: Tue, 23 Apr 2013 17:28:07 +0100	[thread overview]
Message-ID: <20130423162807.GA2461@linaro.org> (raw)
In-Reply-To: <1366725128.8473.21.camel@computer5.home>

On Tue, Apr 23, 2013 at 02:52:08PM +0100, Jon Medhurst (Tixy) wrote:
> On Thu, 2013-04-11 at 15:47 +0100, Lorenzo Pieralisi wrote:
> [...]

[...]

> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > +/*
> > + * cci_port_control()
> > + *	@port = index of the port to setup
> > + *	@enable = if true enables the port, if false disables it
> > + */
> > +static void notrace cci_port_control(unsigned int port, bool enable)
> > +{
> > +	void __iomem *base = ports[port].base;
> > +
> > +	if (!base)
> > +		return;
> > +
> > +	writel_relaxed(enable, base + CCI_PORT_CTRL);
> 
> If enable is bool (0 or 1) then this is going to set snoops as specified
> but is always going to clear DVM broadcast as that is controlled by bit1
> of the register. So, does the API need specifying different to allow the
> caller to choose DVM state as well, or does this function need to assume
> DVM and snoops state should be equal? I.e.
> 
> 	writel_relaxed(enable ? 0x3 : 0x0, base + CCI_PORT_CTRL);

Some #defines for that magic "3" would be a good idea too.  Ultimately we
may want independent control over snoops and DVM, but I think that could
be done later as an extension to the code, if needed.  We don't expect
there to be a large number of callers for this code...

Cheers
---Dave

WARNING: multiple messages have this Message-ID (diff)
From: Dave Martin <dave.martin@linaro.org>
To: "Jon Medhurst (Tixy)" <tixy@linaro.org>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	devicetree-discuss@lists.ozlabs.org,
	Rob Herring <rob.herring@calxeda.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Amit Kucheria <amit.kucheria@linaro.org>,
	Olof Johansson <olof@lixom.net>,
	linux-arm-kernel@lists.infradead.org,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [RFC PATCH] drivers: bus: add ARM CCI support
Date: Tue, 23 Apr 2013 17:28:07 +0100	[thread overview]
Message-ID: <20130423162807.GA2461@linaro.org> (raw)
In-Reply-To: <1366725128.8473.21.camel@computer5.home>

On Tue, Apr 23, 2013 at 02:52:08PM +0100, Jon Medhurst (Tixy) wrote:
> On Thu, 2013-04-11 at 15:47 +0100, Lorenzo Pieralisi wrote:
> [...]

[...]

> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > +/*
> > + * cci_port_control()
> > + *	@port = index of the port to setup
> > + *	@enable = if true enables the port, if false disables it
> > + */
> > +static void notrace cci_port_control(unsigned int port, bool enable)
> > +{
> > +	void __iomem *base = ports[port].base;
> > +
> > +	if (!base)
> > +		return;
> > +
> > +	writel_relaxed(enable, base + CCI_PORT_CTRL);
> 
> If enable is bool (0 or 1) then this is going to set snoops as specified
> but is always going to clear DVM broadcast as that is controlled by bit1
> of the register. So, does the API need specifying different to allow the
> caller to choose DVM state as well, or does this function need to assume
> DVM and snoops state should be equal? I.e.
> 
> 	writel_relaxed(enable ? 0x3 : 0x0, base + CCI_PORT_CTRL);

Some #defines for that magic "3" would be a good idea too.  Ultimately we
may want independent control over snoops and DVM, but I think that could
be done later as an extension to the code, if needed.  We don't expect
there to be a large number of callers for this code...

Cheers
---Dave

  parent reply	other threads:[~2013-04-23 16:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-11 14:47 [RFC PATCH] ARM CCI support Lorenzo Pieralisi
2013-04-11 14:47 ` Lorenzo Pieralisi
2013-04-11 14:47 ` [RFC PATCH] drivers: bus: add " Lorenzo Pieralisi
2013-04-11 14:47   ` Lorenzo Pieralisi
2013-04-18 17:20   ` Stephen Boyd
2013-04-18 17:20     ` Stephen Boyd
2013-04-18 17:54     ` Nicolas Pitre
2013-04-18 17:54       ` Nicolas Pitre
2013-04-18 18:11       ` Stephen Boyd
2013-04-18 18:11         ` Stephen Boyd
2013-04-22 14:24       ` Russell King - ARM Linux
2013-04-22 14:24         ` Russell King - ARM Linux
2013-04-22 15:18         ` Nicolas Pitre
2013-04-22 15:18           ` Nicolas Pitre
2013-04-19 11:21     ` Lorenzo Pieralisi
2013-04-19 11:21       ` Lorenzo Pieralisi
2013-04-23 13:52   ` Jon Medhurst (Tixy)
2013-04-23 13:52     ` Jon Medhurst (Tixy)
2013-04-23 14:27     ` Lorenzo Pieralisi
2013-04-23 14:27       ` Lorenzo Pieralisi
2013-04-23 16:28     ` Dave Martin [this message]
2013-04-23 16:28       ` Dave Martin

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=20130423162807.GA2461@linaro.org \
    --to=dave.martin@linaro.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.