On DesignWare I3C controllers where IC_HAS_IBI_DATA=0 (such as versalnet),
the IBI_STS_CNT field (bits [28:24] of QUEUE_STATUS_LEVEL) is hardwired
to 0. The IBI status entry count is instead reported via IBI_BUF_BLR
(bits [23:16] of the same register).

irq_handle_ibis() was unconditionally reading IBI_STS_CNT, causing it to
always see 0 pending IBIs on versalnet and return early without draining
the IBI buffer. Since INTR_IBI_THLD_STAT is level-triggered against the
buffer fill level, this left the interrupt permanently asserted.

Detect IBI data capability at probe time by writing the IBI data threshold
field in QUEUE_THLD_CTRL and reading it back. Use the result to select the
correct register field in irq_handle_ibis().

Fixes: e389b1d72a62 ("i3c: dw: Add support for in-band interrupts")
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
---

 drivers/i3c/master/dw-i3c-master.c | 21 ++++++++++++++++++++-
 drivers/i3c/master/dw-i3c-master.h |  1 +
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index f2a7f8122d9e..7a1782a69081 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1455,7 +1455,11 @@ static void dw_i3c_master_irq_handle_ibis(struct dw_i3c_master *master)
 	u32 reg;
 
 	reg = readl(master->regs + QUEUE_STATUS_LEVEL);
-	n_ibis = QUEUE_STATUS_IBI_STATUS_CNT(reg);
+	if (master->has_ibi_data)
+		n_ibis = QUEUE_STATUS_IBI_STATUS_CNT(reg);
+	else
+		n_ibis = QUEUE_STATUS_IBI_BUF_BLR(reg);
+
 	if (!n_ibis)
 		n_ibis = QUEUE_STATUS_IBI_BUF_BLR(reg);
 	if (!n_ibis)
@@ -1588,6 +1592,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 			struct platform_device *pdev)
 {
 	int ret, irq;
+	u32 thld_ctrl;
 	const struct dw_i3c_drvdata *drvdata;
 	unsigned long quirks = 0;
 
@@ -1647,6 +1652,20 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	master->maxdevs = ret >> 16;
 	master->free_pos = GENMASK(master->maxdevs - 1, 0);
 
+	/*
+	 * Detect IBI data capability (IC_HAS_IBI_DATA): write a non-zero value
+	 * to IBI_DATA_THLD and read back. On controllers like Versalnet
+	 * the field is hardwired to 0 and the write is ignored. Restore the
+	 * original register value after detection.
+	 */
+	thld_ctrl = readl(master->regs + QUEUE_THLD_CTRL);
+	ret = thld_ctrl | QUEUE_THLD_CTRL_IBI_DATA(2);
+	writel(ret, master->regs + QUEUE_THLD_CTRL);
+	ret = readl(master->regs + QUEUE_THLD_CTRL);
+	if (ret & QUEUE_THLD_CTRL_IBI_DATA_MASK)
+		master->has_ibi_data = true;
+	writel(thld_ctrl, master->regs + QUEUE_THLD_CTRL);
+
 	if (has_acpi_companion(&pdev->dev)) {
 		quirks = (unsigned long)device_get_match_data(&pdev->dev);
 	} else if (pdev->dev.of_node) {
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index c5cb695c16ab..306e25a08937 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -51,6 +51,7 @@ struct dw_i3c_master {
 	u32 i2c_fm_timing;
 	u32 i2c_fmp_timing;
 	u32 quirks;
+	bool has_ibi_data;
 	/*
 	 * Per-device hardware data, used to manage the device address table
 	 * (DAT)
-- 
2.34.1

Hi Shubhrajyoti,

> On DesignWare I3C controllers where IC_HAS_IBI_DATA=0 (such as versalnet),
> the IBI_STS_CNT field (bits [28:24] of QUEUE_STATUS_LEVEL) is hardwired
> to 0. The IBI status entry count is instead reported via IBI_BUF_BLR
> (bits [23:16] of the same register).

OK. You seem to have some odd context in your patch though:

> irq_handle_ibis() was unconditionally reading IBI_STS_CNT, causing it to
> always see 0 pending IBIs on versalnet and return early without draining
> the IBI buffer. Since INTR_IBI_THLD_STAT is level-triggered against the
> buffer fill level, this left the interrupt permanently asserted.
> 
> Detect IBI data capability at probe time by writing the IBI data threshold
> field in QUEUE_THLD_CTRL and reading it back. Use the result to select the
> correct register field in irq_handle_ibis().
> 
> Fixes: e389b1d72a62 ("i3c: dw: Add support for in-band interrupts")
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> ---
> 
>  drivers/i3c/master/dw-i3c-master.c | 21 ++++++++++++++++++++-
>  drivers/i3c/master/dw-i3c-master.h |  1 +
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index f2a7f8122d9e..7a1782a69081 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1455,7 +1455,11 @@ static void dw_i3c_master_irq_handle_ibis(struct dw_i3c_master *master)
>         u32 reg;
>  
>         reg = readl(master->regs + QUEUE_STATUS_LEVEL);
> -       n_ibis = QUEUE_STATUS_IBI_STATUS_CNT(reg);
> +       if (master->has_ibi_data)
> +               n_ibis = QUEUE_STATUS_IBI_STATUS_CNT(reg);
> +       else
> +               n_ibis = QUEUE_STATUS_IBI_BUF_BLR(reg);
> +
>         if (!n_ibis)
>                 n_ibis = QUEUE_STATUS_IBI_BUF_BLR(reg);

... where this last conditional is not present upstream? It would seem
to duplicate the purpose of this patch.

While that two-line addition could be a simpler workaround, I think your
intended change looks a bit more solid.

> Fixes: e389b1d72a62 ("i3c: dw: Add support for in-band interrupts")

I don't think this is qualifies as a fix if it enables support for new
hardware configurations.

Cheers,


Jeremy