Messages in this thread | | | Date | Fri, 1 Apr 2022 03:25:01 +0530 | Subject | Re: [Patch v5 2/4] memory: tegra: Add MC error logging on tegra186 onward | From | Ashish Mhetre <> |
| |
On 4/1/2022 1:19 AM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > On 3/30/22 14:22, Ashish Mhetre wrote: > ... >>>> If we are to remove this callback then how to handle unknown interrupt >>>> channel error? >>> >>> Create a common helper function that returns ID of the raised channel or >>> errorno if not bits are set. >>> >> So something like this: >> >> int status_to_channel(const struct tegra_mc *mc, u32 status, >> unsigned int *mc_channel) >> { >> if ((status & mc->soc->ch_intmask) == 0) >> return -EINVAL; >> >> *mc_channel = __ffs((status & mc->soc->ch_intmask) >> >> mc->soc->status_reg_chan_shift); >> >> return 0; >> } >> >> Correct? > > Yes > >>>> Also we want to handle interrupts on one channel at a time and then >>>> clear it from status register. There can be interrupts on multiple >>>> channel. So multiple bits from status will be set. Hence it will be >>>> hard to parameterize shift such that it gives appropriate channel. >>>> So I think current approach is fine. Please correct me if I am wrong >>>> somewhere. >>> >>> You may do the following: >>> >>> 1. find the first channel bit set in the status reg >>> 2. handle that channel >>> 3. clear only the handled status bit, don't clear the other bits >>> 4. return from interrupt >>> >>> If there are other bits set, then interrupt handler will fire again and >>> next channel will be handled. >> >> For clearing status bit after handling, we can retrieve channel bit by >> something like this: >> >> ch_bit = BIT(*mc_channel) << mc->soc->status_reg_chan_shift; >> >> Correct? > > Yes > > Perhaps using FIELD_PREP() and alike helpers could make it look nice in > the code.
I tried using FIELD_PREP() and FIELD_GET() for our use-case but compilation is failing because these macros require the mask to be compile time constant and our mask "mc->soc->ch_intmask" cannot qualify to be compile time constant.
| |