RE: [PATCH] i3c: dw: Disable IBI IRQ depends on hot-join and SIR enabling

[Dylan Hung]

Hi Jeremy,

> -----Original Message-----
> From: Jeremy Kerr <jk@xxxxxxxxxxxxxxxxxxxx>
> Sent: Wednesday, May 1, 2024 2:22 PM
> To: Dylan Hung <dylan_hung@xxxxxxxxxxxxxx>;
> alexandre.belloni@xxxxxxxxxxx; joel@xxxxxxxxx; u.kleine-
> koenig@xxxxxxxxxxxxxx; gustavoars@xxxxxxxxxx;
> krzysztof.kozlowski@xxxxxxxxxx; zenghuchen@xxxxxxxxxx;
> matt@xxxxxxxxxxxxxxxxxxxx; linux-i3c@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Cc: BMC-SW <BMC-SW@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH] i3c: dw: Disable IBI IRQ depends on hot-join and SIR
> enabling
> 
> Hi Dylan,
> 
> Just a question on a prior patch you sent:
> 
> > Disable IBI IRQ signal and status only when hot-join and SIR enabling
> > of all target devices attached to the bus are disabled.
> >
> > Fixes: e389b1d72a62 ("i3c: dw: Add support for in-band interrupts")
> 
> [...]
> 
> > --- a/drivers/i3c/master/dw-i3c-master.c
> > +++ b/drivers/i3c/master/dw-i3c-master.c
> > @@ -1163,8 +1163,10 @@ static void
> > dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
> >                 global = reg == 0xffffffff;
> >                 reg &= ~BIT(idx);
> >         } else {
> > -               global = reg == 0;
> > +               bool hj_rejected = !!(readl(master->regs +
> > +DEVICE_CTRL) & DEV_CTRL_HOT_JOIN_NACK);
> > +
> >                 reg |= BIT(idx);
> > +               global = (reg == 0xffffffff) && hj_rejected;
> >         }
> >         writel(reg, master->regs + IBI_SIR_REQ_REJECT);
> >
>

```
	if (global)
		dw_i3c_master_enable_sir_signal(master, enable);
```

> My interpretation of this change is that we keep the "global" IBI irq enabled if
> hot-join-nack is set (ie, always, because we don't support hot join, and
> configure the hardware to nack all hot join requests).
> 
I would like to clarify the control logic, incorporating the principle of disabling the SIR interrupt signal:

Case 1:
When `DEV_CTRL_HOT_JOIN_NACK` is set, indicating `hj_rejected` is true, it signifies the controller's non-receptiveness to the hot-join event. Consequently, we can safely disable the SIR interrupt signal if none of the target devices request SIR (reg == 0xffffffff).

Case 2:
When `DEV_CTRL_HOT_JOIN_NACK` is unset, indicating `hj_rejected` is false, this indicates the controller's readiness to engage with the hot-join event. Therefore, it's imperative to keep the SIR interrupt signal enabled, even if not all target devices request SIR. In this case, `global` is false and `enable` is false.

Here's a summary table to illustrate the logic:

| enable | reg==0xffffffff? | hj_rejected? | global | result                            |
|---         |---                          |---                   |---         |---                                  |
| false     | true                      | true                | true     | disable SIR interrupt |
| false     | false                     | true                | false     | keep SIR interrupt    |
| false     | true                      | false               | false     | keep SIR interrupt    |
| false     | false                     | false               | false     | keep SIR interrupt    |

> However, we never enable the hot-join NACK interrupt - IBI_QUEUE_CTRL bit
> 0 is never set. So I can't see how we would ever get an interrupt for the hot
> join NACK case anyway. Because of that, this patch is just keeping the IBI
> threshold interrupt always enabled for no reason.
> 

Billy recently submitted a change to implement the hot-join enabling/disabling. Therefore, it is timely to consider the hot-join functionality.
https://patchwork.kernel.org/project/linux-i3c/patch/20240429073624.256830-1-billy_tsai@xxxxxxxxxxxxxx/

> Or is something else happening here? Is there another cause for the IBI
> threshold IRQs?
> 
> Cheers,
> 
> 
> Jeremy