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

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

[Shubhrajyoti Datta]

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

[Jeremy Kerr]

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

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

[Datta, Shubhrajyoti]

[Public]

> -----Original Message-----
> From: Jeremy Kerr <jk@codeconstruct.com.au>
> Sent: Wednesday, April 1, 2026 2:44 PM
> To: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>; linux-
> kernel@vger.kernel.org
> Cc: git (AMD-Xilinx) <git@amd.com>; Alexandre Belloni
> <alexandre.belloni@bootlin.com>; Frank Li <Frank.Li@nxp.com>; Joel Stanley
> <joel@jms.id.au>; linux-i3c@lists.infradead.org
> Subject: Re: [PATCH v2] i3c: dw-i3c-master: Fix IBI count register selection for
> versalnet
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> 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>
> > ---
...

> > +       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?
Hi Jeremy,

 VersalNet currently uses the generic "snps,dw-i3c-master-1.00a" compatible string — there is no unique compatible string for this hardware instance. The DTS
   entry looks like:

  compatible = "snps,dw-i3c-master-1.00a";

  So the compatible-string approach, as it stands, cannot distinguish VersalNet from other DesignWare instances.

  We could introduce a VersalNet-specific compatible with a generic fallback:

  compatible = "xlnx,versalnet-dw-i3c-master", "snps,dw-i3c-master-1.00a";

  and pass a quirk flag via of_device_id.data to select the IBI read method. However, the probe-time detection avoids having to enumerate all affected
  variants — the IC_HAS_IBI_DATA=0 configuration is a synthesizable option in the IP and may appear in other SoCs using the same core.

  Do you have a preference? If the DTS change is acceptable, I can go with the compatible + match-data approach .

However I thought that detecting it will be helpful for backward compatible.

  Thanks,
  Shubhrajyoti

>
> Cheers,
>
>
> Jeremy

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

[Jeremy Kerr]

Hi Shubhrajyoti,

> > 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?
> Hi Jeremy,
> 
> VersalNet currently uses the generic "snps,dw-i3c-master-1.00a"
> compatible string — there is no unique compatible string for this
> hardware instance. The DTS    entry looks like:
> 
>   compatible = "snps,dw-i3c-master-1.00a";

You should *always* have a unique compatible string for each device
instance, if there can be any variation in behaviours from that generic
one (which you certainly do have here).

You can still fall-back on a generic one, but using that as your only
compatible value means you can't do device-specific behaviours in your
driver, as you have just found.

>   We could introduce a VersalNet-specific compatible with a generic fallback:
> 
>   compatible = "xlnx,versalnet-dw-i3c-master", "snps,dw-i3c-master-1.00a";

Yes, you want that anyway.

>   and pass a quirk flag via of_device_id.data to select the IBI read method.

Exactly, and there is already the struct dw_i3c_drvdata to help with
this.

> However, the probe-time detection avoids having to enumerate all affected
> variants — the IC_HAS_IBI_DATA=0 configuration is a synthesizable
> option in the IP and may appear in other SoCs using the same core.
> 
> Do you have a preference? If the DTS change is acceptable, I can go
> with the compatible + match-data approach

That would certainly be my recommendation.

If it's a synthesisable option, it may even be worth adding as a flag to
the binding definition (along with the same for other options that might
be present). This would mean you don't need a-priori knowledge of the
mapping of compatible strings to their synthesis options.

I don't have any access to documentation on those options though, so
can't be of much help with doing that.

> However I thought that detecting it will be helpful for backward compatible.

Given it doesn't work at the moment, there shouldn't be any backwards
compat concerns, I think?

Cheers,


Jeremy

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

[Alexandre Belloni]

Hello,

On 02/04/2026 16:32:30+0800, Jeremy Kerr wrote:
> Hi Shubhrajyoti,
> 
> > > 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?
> > Hi Jeremy,
> > 
> > VersalNet currently uses the generic "snps,dw-i3c-master-1.00a"
> > compatible string — there is no unique compatible string for this
> > hardware instance. The DTS    entry looks like:
> > 
> >   compatible = "snps,dw-i3c-master-1.00a";
> 
> You should *always* have a unique compatible string for each device
> instance, if there can be any variation in behaviours from that generic
> one (which you certainly do have here).
> 
> You can still fall-back on a generic one, but using that as your only
> compatible value means you can't do device-specific behaviours in your
> driver, as you have just found.
> 
> >   We could introduce a VersalNet-specific compatible with a generic fallback:
> > 
> >   compatible = "xlnx,versalnet-dw-i3c-master", "snps,dw-i3c-master-1.00a";
> 
> Yes, you want that anyway.
> 

I agree you need a VersalNet-specific compatible string.

> >   and pass a quirk flag via of_device_id.data to select the IBI read method.
> 
> Exactly, and there is already the struct dw_i3c_drvdata to help with
> this.
> 
> > However, the probe-time detection avoids having to enumerate all affected
> > variants — the IC_HAS_IBI_DATA=0 configuration is a synthesizable
> > option in the IP and may appear in other SoCs using the same core.
> > 
> > Do you have a preference? If the DTS change is acceptable, I can go
> > with the compatible + match-data approach
> 
> That would certainly be my recommendation.
> 
> If it's a synthesisable option, it may even be worth adding as a flag to
> the binding definition (along with the same for other options that might
> be present). This would mean you don't need a-priori knowledge of the
> mapping of compatible strings to their synthesis options.
> 
> I don't have any access to documentation on those options though, so
> can't be of much help with doing that.
> 
> > However I thought that detecting it will be helpful for backward compatible.
> 
> Given it doesn't work at the moment, there shouldn't be any backwards
> compat concerns, I think?

I guess the proposed solution works as we can probe the HW to know how
it was synthesised. It's good to be able to avoid having device tree
properties for things we can discover.


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