Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1172461 > unrolled thread

Re: [PATCH v2 0/3] ARM: keystone: add ecc error interrupt handling

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 0/3] ARM: keystone: add ecc error interrupt handling Stephen Boyd <sboyd@codeaurora.org> - 2015-06-25 23:40 +0200
    Re: [PATCH v2 0/3] ARM: keystone: add ecc error interrupt handling Stephen Boyd <sboyd@codeaurora.org> - 2015-07-02 02:20 +0200

#1172461 — Re: [PATCH v2 0/3] ARM: keystone: add ecc error interrupt handling

On 06/25/2015 02:30 PM, santosh shilimkar wrote:
> On 6/25/2015 2:02 PM, Stephen Boyd wrote:
>> On 06/25/2015 08:04 AM, santosh shilimkar wrote:
>>> On 6/25/2015 7:31 AM, Vitaly Andrianov wrote:
>>>> This patch series adds support for arm L1/L2 ecc and ddr3 ecc error
>>>> handling
>>>> for Keystone devices
>>>>
>>>> Change Log
>>>>
>>>> v2:
>>>> - removing unused and sorting headers of keystone.c are moved to a
>>>> separate
>>>>     patch.
>>>> - l1l2 ecc and ddr3 ecc error handling are split it to separate
>>>> patches
>>>> - removed unused headers from keystone_ecc.c
>>>> - platsmp.c removed from the patch.
>>>> - return IRQ_HANDLED for 1 bit error in l1l2 ecc handler
>>>> - checked and handled existing echttps://lwn.net/Articles/593336/c
>>>> error before enabling ddr3 interrupt
>>>> - 1 bit ddr3 interrupt is disabled, because it is handled by hardware
>>>> and
>>>>     there is no reason to handle it by software
>>>>
>>> This version looks good to me. As already commented, I would have liked
>>> the patch 2/3(L2 ECC) code in ARM generic code so will give some more
>>> time for others to come back. Otherwise I will queue this up for next
>>> window.
>>
>> Why not make this into an edac driver? I sent out an L1/L2 error
>> detection edac driver for Krait processors a year ago, but it stalled
>> due to some DT binding stuff[1]. This looks fairly similar.
>>
> Indeed the error detection part is very similar(expected as well
> considering the same processor L2 regs). I am not sure we need
> full driver only for that but at least the IRQ error handler
> related code can reside together. Lets see what RMK thinks
> on this.
>

There's an existing one for highbank (drivers/edac/highbank_l2_edac.c)
and there was a patch set for the pl310 as well[1]. I don't think we
want any architecture specific code for this, just use the EDAC framework.

[1] https://lkml.org/lkml/2014/3/2/87

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1175626

On 06/26/2015 05:20 AM, Vitaly Andrianov wrote:
>
>
> On 06/25/2015 05:35 PM, Stephen Boyd wrote:
>>
>> There's an existing one for highbank (drivers/edac/highbank_l2_edac.c)
>> and there was a patch set for the pl310 as well[1]. I don't think we
>> want any architecture specific code for this, just use the EDAC
>> framework.
>>
>> [1] https://lkml.org/lkml/2014/3/2/87
>>
>
> Before porting that patch I was looking to implementation of the EDAC
> for L2 cache and tried to use the framework. Sorry, but I couldn't
> understand why would the Keystone platform may need it. Most likely
> because I didn't understand the framework itself :(
>
> In order the Keystone ECC works u-boot has to initialize the entire
> DDR3 and enable ECC. When kernel boots up it has to install the ECC
> interrupt handlers for Cortex-A15 L1/L2 ECC and Keystone2 DDR3 ECC. As
> far as 1-bit errors handled and corrected by hardware the software
> even doesn't need to handle those errors. We need to handle 2-bit
> errors and just reboot the board.
>
> As the ECC detection has to be enable on kernel boot time and cannot
> be disabled there is not sense to make it in a module.
>
> So, shall Keystone use the EDAC framework to install the onetime
> working interrupt handler? What are advantages to use the framework?
>

Some advantages that come to mind are user configurability and
statistics. Perhaps the user wants to know how many single bit errors
(correctable errors in EDAC language) have happened so they can figure
out if the part is going bad. Or perhaps the user wants to panic on
single bit errors to play it safe. The edac framework makes this sort of
thing standard, instead of requiring SoC specific tooling. It also
groups similar pieces of hardware support in one place and gets code out
of mach directories.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web