Re: [PATCH] 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).

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

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c