Re: [RFC PATCH 03/11] soc: mediatek: dram-ecc: add tracepoints

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Wed, 4 Nov 2026 02:44:05 +0000
Santino Wang (王俊傑) <Santino.Wang@mediatek.com> wrote:

> Hi Jonathan,
> 
>   I checked UEFI 2.11 Appendix N.2.5 and N.2.6. Both sections
>   define Node, Card, Module and Device fields, but I could not
>   find an explicit channel or subchannel field in Memory Error
>   Section 2.
> 
>   On MT8992, these identify the DRAM channel and the subchannel
>   within that channel. Could you clarify which standard fields
>   or encoding should represent these two indices, particularly
>   the subchannel?


Ah. You are indeed correct.  My mistake!  I confused things with the CXL
spec equivalent which is fairly tightly bound to these entrees but
apparently gained a channel and sub channel over them.

It would be a straight forward addition.  These days UEFI has a code
first proposal path that would rapidly solve this gap.

The process would start with a public discussion (this one fits
that requirement) then a ticket filed on
https://github.com/tianocore/edk2/issues?q=is%3Aissue+state%3Aopen+label%3Atype%3Acode-first

Any example of a related one is:
https://github.com/tianocore/edk2/issues/13430

Alternatively you could encode it in the Device which would be
a slight abuse of the current spec, so not sure I'd recommend
doing that. Binding channel to this would be fine as a given
device is only on one channel, but that is more of a stretch
given subchannels in ddr5 and later.

I'd strong favor not inventing a new error record unless you
absolutely have to as that adds burden for all the userspace
tooling etc.

Jonathan



> 
> Thanks
> Santino
> 
> On Mon, 2026-11-02 at 11:57 -0800, Jonathan Cameron wrote:
> > 
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> > 
> >   
> > > diff --git a/drivers/soc/mediatek/dram-ecc/core/trace.c
> > > b/drivers/soc/mediatek/dram-ecc/core/trace.c
> > > new file mode 100644
> > > index 000000000000..9a8b20e48069
> > > --- /dev/null
> > > +++ b/drivers/soc/mediatek/dram-ecc/core/trace.c
> > > @@ -0,0 +1,6 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2026 MediaTek Inc.
> > > + */
> > > +#define CREATE_TRACE_POINTS
> > > +#include "trace.h"
> > > diff --git a/drivers/soc/mediatek/dram-ecc/core/trace.h
> > > b/drivers/soc/mediatek/dram-ecc/core/trace.h
> > > new file mode 100644
> > > index 000000000000..d603fc38f0a7
> > > --- /dev/null
> > > +++ b/drivers/soc/mediatek/dram-ecc/core/trace.h
> > > @@ -0,0 +1,119 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright (c) 2026 MediaTek Inc.
> > > + */
> > > +#undef TRACE_SYSTEM
> > > +#define TRACE_SYSTEM mtk_dram_ecc
> > > +
> > > +#if !defined(_TRACE_MTK_DRAM_ECC_H) ||
> > > defined(TRACE_HEADER_MULTI_READ)
> > > +#define _TRACE_MTK_DRAM_ECC_H
> > > +
> > > +#include <linux/tracepoint.h>
> > > +
> > > +/* driver lifecycle - state: 0=init, 1=exit */
> > > +
> > > +TRACE_EVENT(mtk_dram_ecc_lifecycle,
> > > +     TP_PROTO(int state),
> > > +     TP_ARGS(state),
> > > +     TP_STRUCT__entry(
> > > +             __field(int, state)
> > > +     ),
> > > +     TP_fast_assign(
> > > +             __entry->state = state;
> > > +     ),
> > > +     TP_printk("%s", __entry->state ? "driver removed" : "driver
> > > initialized")
> > > +);
> > > +
> > > +/*
> > > + * ECC events - spa/channel/subchannel/rank/bank/row/column
> > > decoded from the
> > > + * ECCERR_INFO0/1 entry that determined severity (see
> > > ghes_notifier.c),
> > > + * mirroring ATF dram_ecc/mt8992/fiq.c's decode. No agent/master
> > > ID: ATF's
> > > + * GHES vendor section doesn't compute them (see
> > > mtk_dram_ecc_event.h).
> > > + */
> > > +
> > > +TRACE_EVENT(mtk_dram_ecc_ce,  
> > 
> > Why push these through a custom tracepoint?  This look very similar
> > to what
> > is in the Memory Error Section 2 CPER.
> >   
> > > +     TP_PROTO(u64 spa, u8 channel, u8 subch, u16 rank, u16 bank,
> > > u32 row,
> > > +              u16 column),
> > > +     TP_ARGS(spa, channel, subch, rank, bank, row, column),
> > > +     TP_STRUCT__entry(
> > > +             __field(u64, spa)
> > > +             __field(u8, channel)
> > > +             __field(u8, subch)
> > > +             __field(u16, rank)
> > > +             __field(u16, bank)
> > > +             __field(u32, row)
> > > +             __field(u16, column)
> > > +     ),
> > > +     TP_fast_assign(
> > > +             __entry->spa     = spa;
> > > +             __entry->channel = channel;
> > > +             __entry->subch   = subch;
> > > +             __entry->rank    = rank;
> > > +             __entry->bank    = bank;
> > > +             __entry->row     = row;
> > > +             __entry->column  = column;
> > > +     ),
> > > +     TP_printk("spa=0x%016llx ch=%u subch=%u rank=%u bank=%u
> > > row=%u column=%u",
> > > +               __entry->spa, __entry->channel, __entry->subch,
> > > +               __entry->rank, __entry->bank, __entry->row,
> > > +               __entry->column)
> > > +);
> > > +
> > > +TRACE_EVENT(mtk_dram_ecc_uce,
> > > +     TP_PROTO(u64 spa, u8 channel, u8 subch, u16 rank, u16 bank,
> > > u32 row,
> > > +              u16 column),
> > > +     TP_ARGS(spa, channel, subch, rank, bank, row, column),
> > > +     TP_STRUCT__entry(
> > > +             __field(u64, spa)
> > > +             __field(u8, channel)
> > > +             __field(u8, subch)
> > > +             __field(u16, rank)
> > > +             __field(u16, bank)
> > > +             __field(u32, row)
> > > +             __field(u16, column)
> > > +     ),
> > > +     TP_fast_assign(
> > > +             __entry->spa     = spa;
> > > +             __entry->channel = channel;
> > > +             __entry->subch   = subch;
> > > +             __entry->rank    = rank;
> > > +             __entry->bank    = bank;
> > > +             __entry->row     = row;
> > > +             __entry->column  = column;
> > > +     ),
> > > +     TP_printk("spa=0x%016llx ch=%u subch=%u rank=%u bank=%u
> > > row=%u column=%u",
> > > +               __entry->spa, __entry->channel, __entry->subch,
> > > +               __entry->rank, __entry->bank, __entry->row,
> > > +               __entry->column)
> > > +);  
> >   
>