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

[Ashish Mhetre <amhetre@xxxxxxxxxx>]

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.