From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 3/5] Documentation: Add documentation for the APM X-Gene SoC EDAC DTS binding
Date: Thu, 30 Apr 2015 11:01:43 +0200 [thread overview]
Message-ID: <7128860.xHITr0VFap@wuerfel> (raw)
In-Reply-To: <20150430084549.GB3488@pd.tnic>
On Thursday 30 April 2015 10:45:50 Borislav Petkov wrote:
> On Thu, Apr 30, 2015 at 10:31:12AM +0200, Arnd Bergmann wrote:
> > The problem with your approach is that a lot of these blocks are not
> > vendor specific. You will probably see a server chip that contains a
> > memory controller from DesignWare, a cache controller from ARM, and
> > some other device from the chip vendor, and each of them comes with
> > EDAC support. Then you get three other vendors that have various
> > combinations of the same memory controller, cache controller and
> > other EDAC devices. Not all of these chips would have ARM CPU cores,
> > some may be e.g. MIPS or PowerPC.
>
> I don't mean system-vendor specific but IP-block vendor specific. So
> doing a designware-edac, arm-edac, apm-edac and so on...
Ok, but I'd also do multiple drivers for one vendor if they have several
blocks that are clearly unrelated. My understanding was that this is the
case here, but I have not looked too closely.
Note that a lot of vendors do not like to say who they licensed IP
blocks from, or they are contractually required not to say it.
> Also, I really want for people thinking of writing EDAC drivers to think
> hard before doing so. Whether it is even worth to expose *every* RAS
> functionality in some driver. And to consider that exposing it might
> cause more trouble than benefit.
Definitely agreed on this point.
> We've had that experience on x86 with reporting innocuous DRAM ECC
> errors which were corrected by the hardware. People weren't even reading
> the error message and running around wanting to RMA their processors
> because something in dmesg screamed "Hardware Error".
>
> And now APEI does the firmware-first thing where it gets to look at the
> error first and then decide to report it up to the OS or not.
>
> Which is a good thing because in most cases it unnecessarily upsets the
> user.
Yes, that is helpful feedback, but seems unrelated to the discussion
about how to split this driver or not.
> So to get back on track: I think having the IP-block vendor stuff in one
> EDAC module would make this whole heterogeneity much more manageable.
>
> On that system above, we will load - if I can count correctly - 6 EDAC
> drivers anyway. But the EDAC pile would remain sane.
Ok, let me try to state what I think we agree on:
- For EDAC devices that are clearly unrelated (in particular, made by
different vendors, but possibly part of the same chip), there should
be one module per device type that registers one platform_driver.
- For EDAC drivers that are variants of one another (e.g. different
generations of the same memory controller), we want one module with
one platform_driver that can handle all variants.
Let me know if you disagree with the above. If that's fine, I think it
leaves the case where we have one chip that has multiple EDAC blocks
and we can't easily tell if those blocks are all inter-related or not.
For this case, I would like to rely on your judgment as subsystem
maintainer on whether the parts look related enough to have a single
driver (with matching all device variants) or one driver for each
class of device, as well as spotting cases where a part already has
a driver from a different chip vendor that was licensing the same IP
block.
Arnd
next prev parent reply other threads:[~2015-04-30 9:01 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-28 22:10 [PATCH v7 0/4] edac: Add APM X-Gene SoC EDAC driver Loc Ho
2015-04-28 22:10 ` [PATCH v7 1/5] arm64: Enable EDAC on ARM64 Loc Ho
2015-04-28 22:10 ` [PATCH v7 2/5] MAINTAINERS: Add entry for APM X-Gene SoC EDAC driver Loc Ho
2015-04-28 22:10 ` [PATCH v7 3/5] Documentation: Add documentation for the APM X-Gene SoC EDAC DTS binding Loc Ho
2015-04-28 22:10 ` [PATCH v7 4/5] edac: Add APM X-Gene SoC EDAC driver Loc Ho
2015-04-28 22:10 ` [PATCH 5/5] arm64: Add APM X-Gene SoC EDAC DTS entries Loc Ho
2015-04-29 8:49 ` [PATCH v7 4/5] edac: Add APM X-Gene SoC EDAC driver Arnd Bergmann
2015-04-29 16:57 ` Rob Herring
2015-04-29 18:23 ` Arnd Bergmann
2015-04-29 17:00 ` Rob Herring
2015-04-29 16:47 ` [PATCH v7 3/5] Documentation: Add documentation for the APM X-Gene SoC EDAC DTS binding Rob Herring
2015-04-29 21:33 ` Loc Ho
2015-04-29 21:49 ` Borislav Petkov
2015-04-29 21:56 ` Loc Ho
2015-04-29 22:08 ` Borislav Petkov
2015-04-29 22:20 ` Loc Ho
2015-04-29 23:02 ` Rob Herring
2015-04-30 8:20 ` Borislav Petkov
2015-04-30 8:31 ` Arnd Bergmann
2015-04-30 8:45 ` Borislav Petkov
2015-04-30 9:01 ` Arnd Bergmann [this message]
2015-04-30 9:41 ` Borislav Petkov
2015-04-30 10:21 ` Arnd Bergmann
2015-04-30 12:33 ` Borislav Petkov
2015-04-30 12:52 ` Arnd Bergmann
2015-04-30 10:42 ` Arnd Bergmann
2015-04-30 13:00 ` Borislav Petkov
2015-04-30 16:57 ` Loc Ho
2015-04-30 17:18 ` Borislav Petkov
2015-04-30 21:19 ` Loc Ho
2015-04-30 21:30 ` Borislav Petkov
2015-04-30 21:39 ` Loc Ho
2015-04-30 22:36 ` Rob Herring
2015-04-30 22:47 ` Arnd Bergmann
2015-05-01 6:44 ` Loc Ho
2015-04-30 22:59 ` Loc Ho
2015-05-01 19:59 ` Loc Ho
2015-05-04 22:36 ` Rob Herring
2015-05-04 23:39 ` Loc Ho
2015-04-29 22:43 ` Rob Herring
2015-04-30 0:47 ` Loc Ho
2015-04-29 14:40 ` [PATCH v7 1/5] arm64: Enable EDAC on ARM64 Catalin Marinas
2015-04-29 14:46 ` Jon Masters
2015-04-29 21:39 ` Loc Ho
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=7128860.xHITr0VFap@wuerfel \
--to=arnd@arndb.de \
--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