linux-kernel - Re: [RFC PATCH v3] edac: synps: 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: <20140731135652.GD4375@pd.tnic>
Date:	Thu, 31 Jul 2014 15:56:52 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Michal Simek <michal.simek@...inx.com>
Cc:	Punnaiah Choudary Kalluri <punnaiah.choudary.kalluri@...inx.com>,
	"dougthompson@...ssion.com" <dougthompson@...ssion.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"pawel.moll@....com" <pawel.moll@....com>,
	"mark.rutland@....com" <mark.rutland@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Rob Landley <rob@...dley.net>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Punnaiah Choudary <kpc528@...il.com>,
	Anirudha Sarangi <anirudh@...inx.com>,
	Srikanth Vemula <svemula@...inx.com>
Subject: Re: [RFC PATCH v3] edac: synps: Added EDAC support for zynq ddr ecc
 controller

On Thu, Jul 31, 2014 at 03:36:35PM +0200, Michal Simek wrote:
> Mixing functions for two/more different controller seems to me
> really messy.

Just stating that something is "really messy" without giving at least
one technical reason for it is not going to get you any further. So let
me give you mine:

I think it is not messy because one file contains all the EDAC code for
that system. Like every other system out there uses one EDAC driver.
Everything is nicely contained in one place, any communication between
the two memory controllers is done internally in that driver.

Having two drivers would make that much more complicated. But hey, if
you want to make your life more complicated because it is not enough
complicated, feel free to do so. Just make sure you don't impose any
restrictions on the rest of the subsystem.

> > -ENOPARSE for this sentence.
> 
> I meant by this this scenario - please correct me if I am completely wrong
> because I am not playing with this subsystem. My expectation is that
> memory controller number should be same all the time.
> 
> two memory controllers with the same edac driver.
> 
> Normal calling sequence:
> X probe - number 0
> Y probe - number 1
> 
> Different order/name (I think order is taken at the first place)
> in DTS should caused that X will have number 1 and Y number 0.
> 
> Another scenario:
> two controllers and one with external gpio reset for example.
> 
> X probe is called but gpio controller is not ready and gpio driver returns
> -EPROBE_DEFER then Y probe is called and get number 0. Then X probe is called
> again and get number 1.

Right, since you have a heterogeneous design, you'd need to account
for all that in your driver and deal with probing order. Like having a
way to ask a controller what kind of a controller it is and so on. I'm
afraid I don't know anything about your design to be of more help there.

However, doing this in one <name>_edac.c file is almost trivial than
doing it with two separate drivers. Unless you don't need to do any
meaningful communication between the PS and PL. Then it doesn't matter.

If I were you, I'd spend a more time on a proper design now so that all
my work later is easy. Adding two drivers just because it seems messy is
not a very good idea. What I would do is analyze all my use cases and
come up with a proper design then. But this is just me.

HTH.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/