linux-kernel - Re: [RFC PATCH] edac: zynq: Added EDAC support for zynq ddr ecc controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <531ECA33.8050803@monstr.eu>
Date:	Tue, 11 Mar 2014 09:32:51 +0100
From:	Michal Simek <monstr@...str.eu>
To:	Sören Brinkmann <soren.brinkmann@...inx.com>
CC:	Mark Rutland <mark.rutland@....com>,
	Punnaiah Choudary Kalluri 
	<punnaiah.choudary.kalluri@...inx.com>,
	"dougthompson@...ssion.com" <dougthompson@...ssion.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	"michal.simek@...inx.com" <michal.simek@...inx.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	Pawel Moll <Pawel.Moll@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"galak@...eaurora.org" <galak@...eaurora.org>,
	"rob@...dley.net" <rob@...dley.net>,
	"kpc528@...il.com" <kpc528@...il.com>,
	"kalluripunnaiahchoudary@...il.com" 
	<kalluripunnaiahchoudary@...il.com>,
	"punnaia@...inx.com" <punnaia@...inx.com>
Subject: Re: [RFC PATCH] edac: zynq: Added EDAC support for zynq ddr ecc controller

On 03/10/2014 11:32 PM, Sören Brinkmann wrote:
> On Mon, 2014-03-10 at 12:58PM +0100, Michal Simek wrote:
>> On 03/10/2014 11:56 AM, Mark Rutland wrote:
>>> On Sun, Mar 09, 2014 at 02:57:16AM +0000, Punnaiah Choudary Kalluri wrote:
>>>> Added EDAC support for reporting the ecc errors of zynq ddr controller.
>>>> The ddr ecc controller corrects single bit errors and detects double bit
>>>> errors
>>>>
>>>> Signed-off-by: Punnaiah Choudary Kalluri <punnaia@...inx.com>
>>>> ---
>>>>  .../devicetree/bindings/edac/zynq_edac.txt         |   18 +
>>>>  drivers/edac/Kconfig                               |    7 +
>>>>  drivers/edac/Makefile                              |    1 +
>>>>  drivers/edac/zynq_edac.c                           |  613 ++++++++++++++++++++
>>>>  4 files changed, 639 insertions(+), 0 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/edac/zynq_edac.txt
>>>>  create mode 100644 drivers/edac/zynq_edac.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/edac/zynq_edac.txt b/Documentation/devicetree/bindings/edac/zynq_edac.txt
>>>> new file mode 100644
>>>> index 0000000..c21ff83
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/edac/zynq_edac.txt
>>>> @@ -0,0 +1,18 @@
>>>> +Zynq EDAC driver, it does reports the DDR ECC single bit errors that are
>>>> +corrected and double bit ecc errors that are detected by the DDR ECC controller.
>>>> +ECC support for DDR is available in half-bus width(16 bit) configuration only.
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should be "xlnx,ps7-ddrc" or "xlnx,ps7-ddrc-1.00.a"
>>>
>>> Is this an or or a xor?
>>
>> Compatible string should be just xlnx,zynq-ddrc-1.00.a.
>> Nothing with ps7.
> 
> Isn't this vendor IP? IMHO, this should be something completely
> different. Or if you want some Zynq-specific compat string it should
> refer to an actual version string associated with Zynq. 1.00.a is not,
> AFAIK.

I have checked with Punnaiah that this is Synopsys DDR memory controller.
Zynq is based on 1.4 version with some customization.
It means I think reasonable solution is
call it drivers/edac/synopsys_edac.c
rename zynq_ in driver to synopsys_ and
use xlnx,zynq-ddrc-1.00.a compatible string because zynq is not using
clean synopsys version.

I am not getting point why you don't like 1.00.a suffix here.
Because of historical point of view compatible strings should be different
for early silicon, silicon v1, silicon v2, silicon v3, etc
but we haven't used it at all that's why we can use xilinx scheme
which we are using for our soft IPs.

Does it sound reasonable?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Download attachment "signature.asc" of type "application/pgp-signature" (264 bytes)