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.