All of lore.kernel.org
 help / color / mirror / Atom feed
From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 3/4] ARM: mm: add support for HW coherent systems in PL310
Date: Wed, 14 May 2014 16:58:25 +0200	[thread overview]
Message-ID: <20140514165825.58d3bb20@free-electrons.com> (raw)
In-Reply-To: <20140514143448.GD19866@localhost>

Dear Catalin Marinas,

On Wed, 14 May 2014 15:34:48 +0100, Catalin Marinas wrote:
> On Tue, May 13, 2014 at 11:10:38AM +0100, Thomas Petazzoni wrote:
> > --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> > +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> > @@ -8,6 +8,8 @@ Required properties:
> >  
> >  - compatible : should be one of:
> >    "arm,pl310-cache"
> > +  "arm,pl310-coherent-cache", used for I/O coherent platforms using
> > +     the PL310 cache
> >    "arm,l220-cache"
> >    "arm,l210-cache"
> >    "bcm,bcm11351-a2-pl310-cache": DEPRECATED by "brcm,bcm11351-a2-pl310-cache"
> 
> The binding name kind of implies that we have a transparent PL310 cache
> (at least to me), which is not the case. I would rather have a specific
> dma-coherent property or something similar since it's not another type
> of PL310 but rather a different SoC topology.

Fine with me. A separate property or a different compatible string is
the same.

> But I recall you mentioned that you can't make this decision at the DT
> level since you don't know before whether the SoC is I/O coherent or
> not.

As of today, hardware I/O coherency can only be enabled if CONFIG_SMP
is enabled *and* the SoC is actually multi-core, due to limitations in
the ARM core code. So if you look at my PATCH 4/4 in this series, you
will see that I adjust the compatible string of the cache controller at
run time, depending on whether hardware I/O coherency is enabled or not:

+static void __init mvebu_l2x0_pl310_coherent(void)
+{
+	struct device_node *np;
+
+	if (!coherency_available())
+		return;
+
+	for_each_compatible_node(np, NULL, "arm,pl310-cache") {
+		struct property *new_compat;
+
+		new_compat = kzalloc(sizeof(*new_compat), GFP_KERNEL);
+		new_compat->name = kstrdup("compatible", GFP_KERNEL);
+		new_compat->value = kstrdup("arm,pl310-coherent-cache",
+					    GFP_KERNEL);
+		new_compat->length = strlen(new_compat->value) + 1;
+		of_update_property(np, new_compat);
+	}
+}

So, this code can just as well be changed to add a property instead of
adjusting the compatible property. It's the same thing for me.

Also, I will send (hopefully today) a series that allows to enable
hardware I/O coherency even on !SMP configurations. But I believe it
might take a while to get merged.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Lior Amsalem <alior@marvell.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Russell King <linux@arm.linux.org.uk>,
	Jason Cooper <jason@lakedaemon.net>,
	Tawfik Bayouk <tawfik@marvell.com>, Andrew Lunn <andrew@lunn.ch>,
	Will Deacon <Will.Deacon@arm.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Gregory Clement <gregory.clement@free-electrons.com>,
	Nadav Haklai <nadavh@marvell.com>,
	Rob Herring <robh+dt@kernel.org>,
	Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
	Albin Tonnerre <Albin.Tonnerre@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCHv2 3/4] ARM: mm: add support for HW coherent systems in PL310
Date: Wed, 14 May 2014 16:58:25 +0200	[thread overview]
Message-ID: <20140514165825.58d3bb20@free-electrons.com> (raw)
In-Reply-To: <20140514143448.GD19866@localhost>

Dear Catalin Marinas,

On Wed, 14 May 2014 15:34:48 +0100, Catalin Marinas wrote:
> On Tue, May 13, 2014 at 11:10:38AM +0100, Thomas Petazzoni wrote:
> > --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> > +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> > @@ -8,6 +8,8 @@ Required properties:
> >  
> >  - compatible : should be one of:
> >    "arm,pl310-cache"
> > +  "arm,pl310-coherent-cache", used for I/O coherent platforms using
> > +     the PL310 cache
> >    "arm,l220-cache"
> >    "arm,l210-cache"
> >    "bcm,bcm11351-a2-pl310-cache": DEPRECATED by "brcm,bcm11351-a2-pl310-cache"
> 
> The binding name kind of implies that we have a transparent PL310 cache
> (at least to me), which is not the case. I would rather have a specific
> dma-coherent property or something similar since it's not another type
> of PL310 but rather a different SoC topology.

Fine with me. A separate property or a different compatible string is
the same.

> But I recall you mentioned that you can't make this decision at the DT
> level since you don't know before whether the SoC is I/O coherent or
> not.

As of today, hardware I/O coherency can only be enabled if CONFIG_SMP
is enabled *and* the SoC is actually multi-core, due to limitations in
the ARM core code. So if you look at my PATCH 4/4 in this series, you
will see that I adjust the compatible string of the cache controller at
run time, depending on whether hardware I/O coherency is enabled or not:

+static void __init mvebu_l2x0_pl310_coherent(void)
+{
+	struct device_node *np;
+
+	if (!coherency_available())
+		return;
+
+	for_each_compatible_node(np, NULL, "arm,pl310-cache") {
+		struct property *new_compat;
+
+		new_compat = kzalloc(sizeof(*new_compat), GFP_KERNEL);
+		new_compat->name = kstrdup("compatible", GFP_KERNEL);
+		new_compat->value = kstrdup("arm,pl310-coherent-cache",
+					    GFP_KERNEL);
+		new_compat->length = strlen(new_compat->value) + 1;
+		of_update_property(np, new_compat);
+	}
+}

So, this code can just as well be changed to add a property instead of
adjusting the compatible property. It's the same thing for me.

Also, I will send (hopefully today) a series that allows to enable
hardware I/O coherency even on !SMP configurations. But I believe it
might take a while to get merged.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2014-05-14 14:58 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 10:10 [PATCHv2 0/4] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock Thomas Petazzoni
2014-05-13 10:10 ` Thomas Petazzoni
2014-05-13 10:10 ` [PATCHv2 1/4] of: make of_update_property() usable earlier in the boot process Thomas Petazzoni
2014-05-13 10:10   ` Thomas Petazzoni
2014-05-13 14:00   ` Rob Herring
2014-05-13 14:00     ` Rob Herring
2014-05-13 14:30     ` Thomas Petazzoni
2014-05-13 14:30       ` Thomas Petazzoni
2014-05-13 14:54       ` Grant Likely
2014-05-13 14:54         ` Grant Likely
2014-05-13 15:30       ` Jason Cooper
2014-05-13 15:30         ` Jason Cooper
2014-05-13 15:54         ` Thomas Petazzoni
2014-05-13 15:54           ` Thomas Petazzoni
2014-05-13 16:31           ` Jason Cooper
2014-05-13 16:31             ` Jason Cooper
2014-05-13 16:58           ` Rob Herring
2014-05-13 16:58             ` Rob Herring
2014-05-13 17:00             ` Jason Cooper
2014-05-13 17:00               ` Jason Cooper
2014-05-13 10:10 ` [PATCHv2 2/4] ARM: mm: allow sub-architectures to override PCI I/O memory type Thomas Petazzoni
2014-05-13 10:10   ` Thomas Petazzoni
2014-05-14 15:01   ` Catalin Marinas
2014-05-14 15:01     ` Catalin Marinas
2014-05-13 10:10 ` [PATCHv2 3/4] ARM: mm: add support for HW coherent systems in PL310 Thomas Petazzoni
2014-05-13 10:10   ` Thomas Petazzoni
2014-05-14 14:34   ` Catalin Marinas
2014-05-14 14:34     ` Catalin Marinas
2014-05-14 14:58     ` Thomas Petazzoni [this message]
2014-05-14 14:58       ` Thomas Petazzoni
2014-05-13 10:10 ` [PATCHv2 4/4] ARM: mvebu: implement L2/PCIe deadlock workaround Thomas Petazzoni
2014-05-13 10:10   ` Thomas Petazzoni
2014-05-13 11:13   ` Arnd Bergmann
2014-05-13 11:13     ` Arnd Bergmann
2014-05-13 12:52     ` Thomas Petazzoni
2014-05-13 12:52       ` Thomas Petazzoni
2014-05-14 15:24       ` Catalin Marinas
2014-05-14 15:24         ` Catalin Marinas
2014-05-14 14:58   ` Catalin Marinas
2014-05-14 14:58     ` Catalin Marinas
2014-05-14 15:04     ` Thomas Petazzoni
2014-05-14 15:04       ` Thomas Petazzoni

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=20140514165825.58d3bb20@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --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.