Re: [PATCH] scsi: ufs: core: Handle MCQ IAG events

[VAMSHI GAJJELA]

 or

On Fri, Mar 6, 2026 at 9:19 AM Peter Wang (王信友) <peter.wang@xxxxxxxxxxxx> wrote:
>
> On Thu, 2026-03-05 at 06:07 -0600, Bart Van Assche wrote
> >
> > Hi Peter,
> >
> > It is not clear to me why the above code is considered confusing?
> >
> > UFS controllers are the only storage controllers I know of
> > that generate different interrupts depending on whether or not
> > interrupt
> > aggregation is enabled. All other storage controllers I know of use
> > the
> > same completion interrupt whether or not interrupt aggregation is
> > enabled.
> >
> > To me the above code means that whether or not interrupt aggregation
> > is
> > enabled, ufshcd_handle_mcq_cq_events() is called to process the
> > pending
> > completions.
> >
> > Thanks,
> >
> > Bart.
>
> Hi Bart,
>
> Sorry, I may not have explained it clearly enough. Normally,
> the logic is to handle A when receiving A event, and handle B
> when receiving B event. But now, the code seems to be hnadle A
> when receiving B event.
> If not familiar with this hardware logic, it’s easy to
> misunderstand.
>
> Thanks
> Peter

Hi Peter and Bart, thanks for the feedback and discussion

Bart: I agree that renaming the boolean flag to reset_iag improves clarity,
and I will address this in the next version of the patch

Peter:  I understand your concern about the readability when calling
ufshcd_handle_mcq_cq_events for both the events. As said, either
MCQ_CQ_EVENT_STATUS or MCQ_IAG_EVENT_STATUS, we are
essentially handling Completion Queue events. The only extra step in
the IAG path is clearing the counters/timers.

Sticking to ufshcd_handle_mcq_cq_events and adding the agreed-upon
descriptive argument reset_iag should help the readability IMO.

Do you think a more detailed comments right before those two function
calls would be beneficial?

Regards,
Vamshi G.