[PATCH v2] i3c: dw-i3c-master: Fix IBI count register selection for versalnet

Shubhrajyoti Datta posted 1 patch 12 hours ago
drivers/i3c/master/dw-i3c-master.c | 21 ++++++++++++++++++++-
drivers/i3c/master/dw-i3c-master.h |  1 +
2 files changed, 21 insertions(+), 1 deletion(-)
[PATCH v2] i3c: dw-i3c-master: Fix IBI count register selection for versalnet
Posted by Shubhrajyoti Datta 12 hours ago
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().

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
---

Changes in v2:
Remove the fixes tag


 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 d6bdb32397fb..e43bdc9abeb1 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)
 		return;
 
@@ -1586,6 +1590,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;
 
@@ -1645,6 +1650,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.44.4
Re: [PATCH v2] i3c: dw-i3c-master: Fix IBI count register selection for versalnet
Posted by Jeremy Kerr 11 hours ago
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).
> 
> 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().
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> ---
> 
> Changes in v2:
> Remove the fixes tag

The base context looks better now, thanks.

One other question though:

> +       /*
> +        * 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);

How are you binding the driver to this device? Are you using a unique
OF compatible string, or something ACPI-based?

... and if that can be specific to this hardware instance, would that be
an effective mechanism to select the IBI read method instead?

Cheers,


Jeremy