| Message ID | 20201013125033.4749-1-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add L1 and L2 error detection for A53 and A57 | expand |
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@amazon.com 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. > This is another try to get this driver to mainline. All error injection > code has been removed (though it has initially been used to test this > driver on an i.MX8 SoC), what's left is unfortunately not testable, but > also doesn't contain none of the doubtful code anymore. > > Changes since v1: > - Split dt-binding into separate patch > - Sort local function variables in reverse-xmas tree order > - drop unnecessary comparison and make variable bool > > Sascha Hauer (2): > dt-bindings: edac: Add binding for L1/L2 error detection for Cortex > A53/57 > drivers/edac: Add L1 and L2 error detection for A53 and A57 > > York Sun (1): > arm64: dts: ls104x: Add L1/L2 cache edac node > > .../bindings/edac/arm,cortex-a5x-edac.yaml | 32 +++ > .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 5 + > .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 5 + > drivers/edac/Kconfig | 6 + > drivers/edac/Makefile | 1 + > drivers/edac/cortex_arm64_l1_l2.c | 208 ++++++++++++++++++ > 6 files changed, 257 insertions(+) > create mode 100644 Documentation/devicetree/bindings/edac/arm,cortex-a5x-edac.yaml > create mode 100644 drivers/edac/cortex_arm64_l1_l2.c > > -- > 2.28.0 >
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@amazon.com 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? Sascha
On Wed, Oct 14, 2020 at 9:04 AM Sascha Hauer <s.hauer@pengutronix.de> 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@amazon.com > > 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
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. This is another try to get this driver to mainline. All error injection code has been removed (though it has initially been used to test this driver on an i.MX8 SoC), what's left is unfortunately not testable, but also doesn't contain none of the doubtful code anymore. Changes since v1: - Split dt-binding into separate patch - Sort local function variables in reverse-xmas tree order - drop unnecessary comparison and make variable bool Sascha Hauer (2): dt-bindings: edac: Add binding for L1/L2 error detection for Cortex A53/57 drivers/edac: Add L1 and L2 error detection for A53 and A57 York Sun (1): arm64: dts: ls104x: Add L1/L2 cache edac node .../bindings/edac/arm,cortex-a5x-edac.yaml | 32 +++ .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 5 + .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 5 + drivers/edac/Kconfig | 6 + drivers/edac/Makefile | 1 + drivers/edac/cortex_arm64_l1_l2.c | 208 ++++++++++++++++++ 6 files changed, 257 insertions(+) create mode 100644 Documentation/devicetree/bindings/edac/arm,cortex-a5x-edac.yaml create mode 100644 drivers/edac/cortex_arm64_l1_l2.c