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

["Santino Wang (王俊傑)" <Santino.Wang@mediatek.com>]

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?

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)
> > +);
>