On Wed, Oct 14, 2020 at 9:04 AM Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote: > > On Wed, Oct 14, 2020 at 08:25:11AM -0500, Rob Herring wrote: > > On Tue, Oct 13, 2020 at 02:50:30PM +0200, Sascha Hauer wrote: > > > This driver is based on an earlier version from York Sun which can > > > be found here: https://lkml.org/lkml/2018/3/14/1203. > > > > > > At that time the conclusion was that this driver is not suitable for > > > mainline as it used IMPLEMENTATION DEFINED CPU registers and also > > > NXP specific SMC calls. All this was used for the error injection only, > > > for error reporting it is not needed. > > > > Have you looked at Amazon's version: > > http://lore.kernel.org/r/20200510151310.17372-2-hhhawa@xxxxxxxxxx > > No, I was not aware of that driver. It's basically the same driver, but > limited to a single SoC. Looks like at least some things are better in > that driver, read_sysreg_s(ARM_CA57_L2MERRSR_EL1) reads better than my > open coded variant. > > > > > Which is an A57 EDAC driver. Looks like it never got upstream though, > > but it's not clear why. > > > > You'll note that it doesn't have a virtual DT node either. > > Testing the SoC type in an initcall looks odd to me. Wouldn't a > dedicated node be preferred? Yes, the one with "arm,cortex-a57". But no, a virtual node isn't preferred. We discussed this at length on Amazon's version IIRC. I could perhaps be convinced to add a property in the cpu nodes that ECC is present/enabled. Then you'd just match on cpu compatible(s) and check for the property. You're still creating the device yourself, but that's the kernel's problem which shouldn't dictate how you design your bindings. Rob
