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

Groups > linux.kernel > #1378933 > unrolled thread

Re: [PATCH] Add EDAC peripheral init functions & Ethernet EDAC.

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] Add EDAC peripheral init functions & Ethernet EDAC. Rob Herring <robh@kernel.org> - 2016-04-14 16:40 +0200
    Re: [PATCH] Add EDAC peripheral init functions & Ethernet EDAC. Mauro Carvalho Chehab <m.chehab@samsung.com> - 2016-04-15 12:00 +0200
      Re: [PATCH] Add EDAC peripheral init functions & Ethernet EDAC. Borislav Petkov <bp@alien8.de> - 2016-04-15 23:50 +0200
        Re: [PATCH] Add EDAC peripheral init functions & Ethernet EDAC. Borislav Petkov <bp@alien8.de> - 2016-04-18 22:10 +0200
        Re: [PATCH] Add EDAC peripheral init functions & Ethernet EDAC. Borislav Petkov <bp@alien8.de> - 2016-04-18 22:10 +0200

#1378933 — Re: [PATCH] Add EDAC peripheral init functions & Ethernet EDAC.

On Tue, Apr 12, 2016 at 05:12:55PM -0500, tthayer@opensource.altera.com wrote:
> This patch set adds the memory initialization functions for Altera's
> Arria10 peripherals, the first of which is the Ethernet EDAC. The
> first 3 patches add the memory initialization functionality. The
> last 3 patches add Ethernet EDAC support.

The ethernet part seems a bit strange to me to put under EDAC as EDAC 
is primarily memory controller ECC (and caches to some extent). Also you 
would not halt the system in case of an UC, but rather just drop the 
frame. This would need to be part of the ethernet driver in that case.
 
Of course, given that ethernet frames already have a CRC, ECC of the 
FIFO seems a bit redundant.

Rob

[toc] | [next] | [standalone]

#1379655

Em Thu, 14 Apr 2016 09:35:01 -0500
Rob Herring <robh@kernel.org> escreveu:

> On Tue, Apr 12, 2016 at 05:12:55PM -0500, tthayer@opensource.altera.com wrote:
> > This patch set adds the memory initialization functions for Altera's
> > Arria10 peripherals, the first of which is the Ethernet EDAC. The
> > first 3 patches add the memory initialization functionality. The
> > last 3 patches add Ethernet EDAC support.  
> 
> The ethernet part seems a bit strange to me to put under EDAC as EDAC 
> is primarily memory controller ECC (and caches to some extent). Also you 
> would not halt the system in case of an UC, but rather just drop the 
> frame. This would need to be part of the ethernet driver in that case.
>  
> Of course, given that ethernet frames already have a CRC, ECC of the 
> FIFO seems a bit redundant.

Actually, EDAC was conceived to be a way to report hardware errors, and,
although the main use case is for memory and CPU errors, there are a few
drivers that report errors at PCI bus. So, I don't see much problems using
it to report other hardware errors, like the ones associated with the
Ethernet hardware.

That's said, things like Ethernet frame errors are better handled via the
network drivers. I would report via EDAC only errors associated with the
Ethernet hardware that would cause the hardware to malfunction.

Btw, an UC error won't cause the system to halt, except if a UC memory
error happens and the EDAC core is loaded with an special modprobe
parameter (edac_mc_panic_on_ue = 1).

-- 
Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]

#1380294

On Fri, Apr 15, 2016 at 10:27:54AM -0500, Thor Thayer wrote:
> I'll update this patch to only count errors.

... and also think about what that counting is going to bring. If it is
only going to be there to show how many network errors happened and we
can't do anything about it except stare at that number, then adding all
that code is probably waste of time an electrons...

I'm just sayin'.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply. Srsly.

[toc] | [prev] | [next] | [standalone]

#1382024

On Mon, Apr 18, 2016 at 10:02:27PM +0200, Borislav Petkov wrote:
> >  the number of uncorrectable errors is useful from a system point of
> > view.

I forgot: so altr_edac_a10_ecc_irq() panics on uncorrectable errors. Do we want
to do that even for UEs coming from the network...?

Seems a bit overboard to me...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply. Srsly.

[toc] | [prev] | [next] | [standalone]

#1382025

On Mon, Apr 18, 2016 at 09:27:16AM -0500, Thor Thayer wrote:
> We're still getting the single bit correction

By that you mean, you get that by enabling ECC on the FIFO block?

> which makes the entire system more stable and the ability to see both
> single bit errors corrected and the number of uncorrectable errors is
> useful from a system point of view.

If so, that makes sense, yes.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply. Srsly.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web