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. Best regards, Krzysztof