i3c: dw: Disable IBI IRQ depends on hot-join and SIR enabling

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

Posted by Dylan Hung 1 year, 11 months ago

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")

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
---
 drivers/i3c/master/dw-i3c-master.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index ef5751e91cc9..276153e10f5a 100644
--- 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);
 
-- 
2.25.1

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

Posted by Jeremy Kerr 1 year, 7 months ago

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);
>  

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).

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.

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

Cheers,


Jeremy

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

Posted by andy.shevchenko@gmail.com 1 year, 10 months ago

Fri, Jan 19, 2024 at 01:45:47PM +0800, Dylan Hung kirjoitti:
> 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")
> 
> Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>

Tag block is not supposed to have blank lines.

...

> @@ -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);

'!!()' is redundant.

> +
>  		reg |= BIT(idx);
> +		global = (reg == 0xffffffff) && hj_rejected;
>  	}

Moreover, you can refactor a bit both branches, like

	bool hj_rejected = true;
	...
	if (...) {
		...
	} else {
		hj_rejected = readl(master->regs + DEVICE_CTRL) & DEV_CTRL_HOT_JOIN_NACK;
		...
	}
	global = (reg == 0xffffffff) && hj_rejected;


-- 
With Best Regards,
Andy Shevchenko

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

Posted by Alexandre Belloni 1 year, 10 months ago

On Fri, 19 Jan 2024 13:45:47 +0800, Dylan Hung wrote:
> 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")
> 
> 

Applied, thanks!

[1/1] i3c: dw: Disable IBI IRQ depends on hot-join and SIR enabling
      https://git.kernel.org/abelloni/c/10201396ef64

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com