Re: [Patch v5 2/4] memory: tegra: Add MC error logging on tegra186 onward

Ashish Mhetre
2022-03-22T17:34:22Z

On 3/19/2022 9:44 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 16.03.2022 12:25, Ashish Mhetre пишет:
>> +static int tegra186_mc_get_channel(const struct tegra_mc *mc, int *mc_channel)
>> +{
>> + u32 status;
>> +
>> + status = mc_ch_readl(mc, MC_BROADCAST_CHANNEL, MC_GLOBAL_INTSTATUS);
>
> This mc_ch_readl(MC_GLOBAL_INTSTATUS) is replicated by every
> tegraxxx_mc_get_channel(), it should be a part of common interrupt
> handler, IMO.
>
> And then I'd rename that callback to global_intstatus_to_channel().
>
Okay, I'll do that in next version.

>> + switch (status & 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");
>
> dev_err_ratelimited("unknown interrupt channel 0x%08x\n", status) and
> should be moved to the common interrupt handler.
>
So return just error from default case and handle error in common
interrupt handler with this print, right? I'll update this in next
version.