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>] [day] [month] [year] [list]
Message-ID: <5c399e76-42e4-4f8e-852e-91d88b229ae7@TX2EHSMHS006.ehs.local>
Date:	Thu, 13 Mar 2014 17:21:09 +0000
From:	Punnaiah Choudary Kalluri <punnaiah.choudary.kalluri@...inx.com>
To:	Michal Simek <michals@...inx.com>,
	Soren Brinkmann <sorenb@...inx.com>
CC:	Michal Simek <michals@...inx.com>,
	"monstr@...str.eu" <monstr@...str.eu>,
	Mark Rutland <mark.rutland@....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>,
	"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>
Subject: RE: [RFC PATCH] edac: zynq: Added EDAC support for zynq ddr ecc
 controller



>-----Original Message-----
>From: Michal Simek [mailto:michal.simek@...inx.com]
>Sent: Wednesday, March 12, 2014 10:20 PM
>To: Soren Brinkmann
>Cc: Michal Simek; monstr@...str.eu; Mark Rutland; Punnaiah Choudary
>Kalluri; dougthompson@...ssion.com; devicetree@...r.kernel.org; linux-
>doc@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
>kernel@...r.kernel.org; linux-edac@...r.kernel.org; robh+dt@...nel.org;
>Pawel Moll; ijc+devicetree@...lion.org.uk; galak@...eaurora.org;
>rob@...dley.net; kpc528@...il.com; kalluripunnaiahchoudary@...il.com;
>Punnaiah Choudary Kalluri
>Subject: Re: [RFC PATCH] edac: zynq: Added EDAC support for zynq ddr ecc
>controller
>
>On 03/12/2014 05:13 PM, Sören Brinkmann wrote:
>> On Wed, 2014-03-12 at 11:27AM +0100, Michal Simek wrote:
>>> On 03/11/2014 04:50 PM, Sören Brinkmann wrote:
>>>> On Tue, 2014-03-11 at 09:32AM +0100, Michal Simek wrote:
>>>>> 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.
>>>>
>>>> Well, but you refer to Zynq which has silicon revisions you can use to
>>>> refer to a specific Zynq version. That matches data sheets. Zynq-1.00.a
>>>> is just completely made up and wouldn't be found in any data sheet.
>>>
>>> I didn't reply this one but we discussed this with Soren over phone.
>>> No problem to use Synopsys version A07 according TRM.
>>>
>>> Then compatible string is
>>> xlnx,zynq-ddrc-A07 and file name is synopsys_edac.c
>>
>> This seems to make sense. I'm never sure whether leading zeros are
>> actually needed and tend to remove them, but in general this looks like
>> a reasonable string to identify Zynq's DDR controller.
>
>A07 I found in Zynq TRM and probably has any conection to synopsys version
>scheme.
I have discussed with the design team and confirmed that the synopsis IP
version is 1.4 and looks like it is wrongly updated in TRM as A07.
So, I am going to use compatible string as xlnx,zynq-ddrc-1.4 and file name
as synopsys_edac.c and send you the updated patch soon.
Thanks for the review and suggestions

Regards,
Punnaiah
>
>Thanks,
>Michal
>
>
>



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.