From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv6 0/5] Add Altera peripheral memories to EDAC framework
Date: Fri, 6 Feb 2015 19:29:25 +0000 [thread overview]
Message-ID: <20150206192925.GG10324@leverpostej> (raw)
In-Reply-To: <54CA9DB2.4070109@opensource.altera.com>
On Thu, Jan 29, 2015 at 08:53:06PM +0000, Thor Thayer wrote:
> Hi Device Tree Maintainers,
Hi Thor,
Apologies for being silent for so long on this.
> On 01/08/2015 08:53 PM, tthayer at opensource.altera.com wrote:
> > From: Thor Thayer <tthayer@opensource.altera.com>
> >
> > This patch adds the L2 cache and OCRAM peripherals to the EDAC framework
> > using the EDAC device framework. The ECC is enabled early in the boot
> > process in the platform specific code.
> >
>
> The changes in this patch series revision were mainly to address device
> tree concerns. There were changes in other areas of the code to address
> these changes but I believe the other maintainers are waiting to see if
> these changes are accepted before they will review (they had approved
> the previous patch changes).
>
> How does the this patch series appear from a device tree perspective?
While this looks better than before (thank you for no longer treating
the PL310 registers as a syscon), I have a couple of quibbles with the
binding:
* It's not clear to me whether the L2 and OCRAM EDACs are fundamentally
different in interface, or whether they simply happen to be monitoring
different memories. The former would mean a common compatible string
for both EDAC block instances, and another property to determine
what the other end was.
* It relies on a single instance of OCRAM or L2 existing, rather than
describing the relationship with the particular memory explicitly. In
general it's better to be explicit -- this means it can more easily be
generalised to multiple OCRAMs as might occur in a later SoC (in
addition to the reasons laid out in the first point.
Thanks,
Mark.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Thor Thayer <tthayer@opensource.altera.com>
Cc: "robh+dt@kernel.org" <robh+dt@kernel.org>,
Pawel Moll <Pawel.Moll@arm.com>,
"ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>,
"galak@codeaurora.org" <galak@codeaurora.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"bp@alien8.de" <bp@alien8.de>,
"dougthompson@xmission.com" <dougthompson@xmission.com>,
"m.chehab@samsung.com" <m.chehab@samsung.com>,
"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
"dinguyen@opensource.altera.com" <dinguyen@opensource.altera.com>,
"grant.likely@linaro.org" <grant.likely@linaro.org>,
"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"tthayer.linux@gmail.com" <tthayer.linux@gmail.com>
Subject: Re: [PATCHv6 0/5] Add Altera peripheral memories to EDAC framework
Date: Fri, 6 Feb 2015 19:29:25 +0000 [thread overview]
Message-ID: <20150206192925.GG10324@leverpostej> (raw)
In-Reply-To: <54CA9DB2.4070109@opensource.altera.com>
On Thu, Jan 29, 2015 at 08:53:06PM +0000, Thor Thayer wrote:
> Hi Device Tree Maintainers,
Hi Thor,
Apologies for being silent for so long on this.
> On 01/08/2015 08:53 PM, tthayer@opensource.altera.com wrote:
> > From: Thor Thayer <tthayer@opensource.altera.com>
> >
> > This patch adds the L2 cache and OCRAM peripherals to the EDAC framework
> > using the EDAC device framework. The ECC is enabled early in the boot
> > process in the platform specific code.
> >
>
> The changes in this patch series revision were mainly to address device
> tree concerns. There were changes in other areas of the code to address
> these changes but I believe the other maintainers are waiting to see if
> these changes are accepted before they will review (they had approved
> the previous patch changes).
>
> How does the this patch series appear from a device tree perspective?
While this looks better than before (thank you for no longer treating
the PL310 registers as a syscon), I have a couple of quibbles with the
binding:
* It's not clear to me whether the L2 and OCRAM EDACs are fundamentally
different in interface, or whether they simply happen to be monitoring
different memories. The former would mean a common compatible string
for both EDAC block instances, and another property to determine
what the other end was.
* It relies on a single instance of OCRAM or L2 existing, rather than
describing the relationship with the particular memory explicitly. In
general it's better to be explicit -- this means it can more easily be
generalised to multiple OCRAMs as might occur in a later SoC (in
addition to the reasons laid out in the first point.
Thanks,
Mark.
next prev parent reply other threads:[~2015-02-06 19:29 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-09 2:53 [PATCHv6 0/5] Add Altera peripheral memories to EDAC framework tthayer at opensource.altera.com
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` [PATCHv6 1/5] arm: socfpga: Enable L2 Cache ECC on startup tthayer at opensource.altera.com
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx
2015-02-06 18:52 ` Mark Rutland
2015-02-06 18:52 ` Mark Rutland
2015-01-09 2:53 ` [PATCHv6 2/5] arm: socfpga: Enable OCRAM " tthayer at opensource.altera.com
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` tthayer
2015-02-06 18:45 ` Mark Rutland
2015-02-06 18:45 ` Mark Rutland
2015-02-06 22:05 ` Thor Thayer
2015-02-06 22:05 ` Thor Thayer
2015-01-09 2:53 ` [PATCHv6 3/5] edac: altera: Remove SDRAM module compile tthayer at opensource.altera.com
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` [PATCHv6 4/5] edac: altera: Add Altera L2 Cache and OCRAM EDAC Support tthayer at opensource.altera.com
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx
2015-02-06 19:17 ` Mark Rutland
2015-02-06 19:17 ` Mark Rutland
2015-02-06 22:09 ` Thor Thayer
2015-02-06 22:09 ` Thor Thayer
2015-02-07 10:02 ` Russell King - ARM Linux
2015-02-07 10:02 ` Russell King - ARM Linux
2015-01-09 2:53 ` [PATCHv6 5/5] arm: dts: Add Altera L2 Cache and OCRAM EDAC entries tthayer at opensource.altera.com
2015-01-09 2:53 ` tthayer
2015-01-09 2:53 ` tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx
2015-02-06 17:03 ` [RESEND PATCHv6 " Thor Thayer
2015-02-06 17:03 ` Thor Thayer
2015-02-06 17:03 ` Thor Thayer
2015-02-06 19:24 ` [PATCHv6 " Mark Rutland
2015-02-06 19:24 ` Mark Rutland
2015-02-06 19:24 ` Mark Rutland
2015-02-06 22:04 ` Thor Thayer
2015-02-06 22:04 ` Thor Thayer
2015-02-06 22:04 ` Thor Thayer
2015-01-29 20:53 ` [PATCHv6 0/5] Add Altera peripheral memories to EDAC framework Thor Thayer
2015-01-29 20:53 ` Thor Thayer
2015-01-29 20:53 ` Thor Thayer
2015-02-06 19:29 ` Mark Rutland [this message]
2015-02-06 19:29 ` Mark Rutland
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=20150206192925.GG10324@leverpostej \
--to=mark.rutland@arm.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.