Re: [PATCH] i3c: dw: Fix IBI intr signal programming

From: Jeremy Kerr <jk@codeconstruct.com.au>
To: Aniket <aniketmaurya@google.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Joel Stanley <joel@jms.id.au>,
	Billy Tsai <billy_tsai@aspeedtech.com>
Cc: linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] i3c: dw: Fix IBI intr signal programming
Date: Fri, 07 Jun 2024 10:28:42 +0800	[thread overview]
Message-ID: <3c6c319f3aaa60428fd28f4d95c71dc9a8150081.camel@codeconstruct.com.au> (raw)
In-Reply-To: <20240606124816.723630-1-aniketmaurya@google.com>

Hi Aniket,

> IBI_SIR_REQ_REJECT register is not present if the IP
> has IC_HAS_IBI_DATA = 1 set.

I don't have any access to the IP itself, but I understand there are a
few different configuration settings in the IP that may affect the
register interface.

I think we're OK in this case (just not reading the value out of the
SIR_REQ_REJECT register), but any thoughts on adding corresponding
switches in the driver so we can support those configurations? These
would be represented as DT config of the specific hardware instance - at
the most granular, just by the specific compatible string.

> dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
>  
>         reg = readl(master->regs + IBI_SIR_REQ_REJECT);
>         if (enable) {
> -               global = reg == 0xffffffff;
> +               global = !master->sir_en_cnt++;
>                 reg &= ~BIT(idx);
>         } else {
>                 bool hj_rejected = !!(readl(master->regs + DEVICE_CTRL) & DEV_CTRL_HOT_JOIN_NACK);
>  
>                 reg |= BIT(idx);
> -               global = (reg == 0xffffffff) && hj_rejected;
> +               global = (!--master->sir_en_cnt) && hj_rejected;
>         }

Could we use the SIR mask for this, but just read it from a field in the
struct dw_i3c_master, instead of IBI_SIR_REQ_REJECT?

This would mean that there's no possibility of the counter going out of
sync from the SIR settings - say, on underflow if we get a spurious
disable.

Cheers,

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