> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx>
> Sent: Thursday, March 3, 2022 1:14 AM
> To: Ashish Mhetre <amhetre@xxxxxxxxxx>; robh+dt@xxxxxxxxxx;
> thierry.reding@xxxxxxxxx; Jonathan Hunter <jonathanh@xxxxxxxxxx>;
> digetx@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-tegra@xxxxxxxxxxxxxxx
> Cc: Krishna Reddy <vdumpa@xxxxxxxxxx>; Sachin Nikam
> <Snikam@xxxxxxxxxx>
> Subject: Re: [Patch v4 4/4] memory: tegra: Add MC error logging on tegra186
> onward
>
> External email: Use caution opening links or attachments
>
>
> On 02/03/2022 09:43, Ashish Mhetre wrote:
> > Add new function 'get_int_channel' in tegra_mc_soc struture which is
> > implemented by tegra SOCs which support multiple MC channels. This
> > function returns the channel which should be used to get the
> > information of interrupts.
> > Remove static from tegra30_mc_handle_irq and use it as interrupt
> > handler for MC interrupts on tegra186, tegra194 and tegra234 to log the
> errors.
> > Add error specific MC status and address register bits and use them on
> > tegra186, tegra194 and tegra234.
> > Add error logging for generalized carveout interrupt on tegra186,
> > tegra194 and tegra234.
> > Add error logging for route sanity interrupt on tegra194 an tegra234.
> > Add register for higher bits of error address which is available on
> > tegra194 and tegra234.
> > Add a boolean variable 'has_addr_hi_reg' in tegra_mc_soc struture
> > which will be true if soc has register for higher bits of memory
> > controller error address. Set it true for tegra194 and tegra234.
> >
> > Signed-off-by: Ashish Mhetre <amhetre@xxxxxxxxxx>
> > ---
> > drivers/memory/tegra/mc.c | 102
> ++++++++++++++++++++++++++++++++++------
> > drivers/memory/tegra/mc.h | 37 ++++++++++++++-
> > drivers/memory/tegra/tegra186.c | 45 ++++++++++++++++++
> > drivers/memory/tegra/tegra194.c | 44 +++++++++++++++++
> > drivers/memory/tegra/tegra234.c | 59 +++++++++++++++++++++++
> > include/soc/tegra/mc.h | 4 ++
> > 6 files changed, 275 insertions(+), 16 deletions(-)
> >
>
> (...)
>
> >
> > +static int tegra186_mc_get_channel(struct tegra_mc *mc, int
> > +*mc_channel) {
> > + u32 g_intstatus;
> > +
> > + g_intstatus = mc_ch_readl(mc, MC_BROADCAST_CHANNEL,
> > + MC_GLOBAL_INTSTATUS);
> > +
> > + switch (g_intstatus & mc->soc->int_channel_mask) {
> > + case BIT(0):
> > + *mc_channel = 0;
> > + break;
> > +
> > + case BIT(1):
> > + *mc_channel = 1;
> > + break;
> > +
> > + case BIT(2):
> > + *mc_channel = 2;
> > + break;
> > +
> > + case BIT(3):
> > + *mc_channel = 3;
> > + break;
> > +
> > + case BIT(24):
> > + *mc_channel = MC_BROADCAST_CHANNEL;
> > + break;
> > +
> > + default:
> > + pr_err("Unknown interrupt source\n");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > const struct tegra_mc_soc tegra186_mc_soc = {
> > .num_clients = ARRAY_SIZE(tegra186_mc_clients),
> > .clients = tegra186_mc_clients,
> > .num_address_bits = 40,
> > .num_channels = 4,
> > + .client_id_mask = 0xff,
> > + .intmask = MC_INT_DECERR_GENERALIZED_CARVEOUT |
> MC_INT_DECERR_MTS |
> > + MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
> > + MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
> > .ops = &tegra186_mc_ops,
> > + .int_channel_mask = 0x100000f,
> > + .get_int_channel = tegra186_mc_get_channel,
> > };
> > #endif
> > diff --git a/drivers/memory/tegra/tegra194.c
> > b/drivers/memory/tegra/tegra194.c index 9400117..bc16567 100644
> > --- a/drivers/memory/tegra/tegra194.c
> > +++ b/drivers/memory/tegra/tegra194.c
> > @@ -1343,10 +1343,54 @@ static const struct tegra_mc_client
> tegra194_mc_clients[] = {
> > },
> > };
> >
> > +static int tegra194_mc_get_channel(struct tegra_mc *mc, int
> > +*mc_channel)
>
> Looks like 'mc' could be a pointer to const.
>
> > +{
> > + u32 g_intstatus;
>
> Variable name just "status" because it looks like some hungarian-notation-
> style...
>
> The same in other places like this.
>
Okay, I will update this in next version.
>
> Best regards,
> Krzysztof
