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

[Jeremy Kerr <jk@codeconstruct.com.au>]

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