Re: [PATCH v2 7/9] i3c: mipi-i3c-hci: Program AST2700 IBI termination threshold

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] i3c: mipi-i3c-hci: lost IBI threshold in
  i3c_hci_do_reset_and_restore()
--

commit 8e158c24faa65911afb5d2dc115124fa69c5c4f7
Author: Billy Tsai <billy_tsai@aspeedtech.com>

i3c: mipi-i3c-hci: Program AST2700 IBI termination threshold

This commit adds HCI_QUIRK_IBI_PAYLOAD_THLD and programs the AST2700
ASPEED_I3C_MST_MRL register to grow and enable the IBI payload
termination threshold when payload IBIs are requested.

> diff --git a/drivers/i3c/master/mipi-i3c-hci/aspeed.c b/drivers/i3c/master/mipi-i3c-hci/aspeed.c
> index 3aa910e786f8e..4a73e0180a108 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/aspeed.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/aspeed.c
> @@ -136,6 +136,26 @@ void aspeed_i3c_write(struct i3c_hci *hci, u32 reg, u32 val)
>  	writel(val, to_aspeed_vendor_data(hci)->inhouse_regs + reg);
>  }
>  
> +void aspeed_i3c_ibi_grow_threshold(struct i3c_hci *hci, unsigned int ibi_max_len)
> +{
> +	u32 mrl, cur_len;
> +
> +	/*
> +	 * The termination threshold is controller-wide while IBI requests
> +	 * for different devices can run in parallel, so serialize the
> +	 * read-modify-write.
> +	 */
> +	guard(spinlock_irqsave)(&hci->lock);
> +
> +	mrl = aspeed_i3c_read(hci, ASPEED_I3C_MST_MRL);
> +	cur_len = FIELD_GET(ASPEED_I3C_IBI_TERMINATE_LEN, mrl);
> +
> +	ibi_max_len = max(ibi_max_len, cur_len);
> +	aspeed_i3c_write(hci, ASPEED_I3C_MST_MRL,
> +			 ASPEED_I3C_IBI_TERMINATE_EN |
> +			 FIELD_PREP(ASPEED_I3C_IBI_TERMINATE_LEN, ibi_max_len));
> +}

[Severity: Medium]
Does i3c_hci_do_reset_and_restore() lose the ASPEED_I3C_MST_MRL threshold
when DMA recovery resets the controller?

When hci_dma_recovery() -> i3c_hci_reset_and_restore() ->
i3c_hci_do_reset_and_restore() runs, it reprograms the in-house
ASPEED_I3C_CTRL and interrupt registers and restores the DAT entries
(including DAT_0_IBI_PAYLOAD), but nothing caches or restores
ASPEED_I3C_MST_MRL:

drivers/i3c/master/mipi-i3c-hci/core.c:i3c_hci_do_reset_and_restore() {
	ret = i3c_hci_reset_and_init(hci);
	if (ret)
		return -EIO;

	if (hci->quirks & HCI_QUIRK_VENDOR_BUS_SETUP)
		aspeed_i3c_bus_setup(hci);
	...
	mipi_i3c_hci_dat_v1.restore(hci);

If the reset clears the in-house registers, payload IBI termination is left
disabled for already-registered devices, and a later i3c_hci_request_ibi()
with a smaller max_payload_len will read cur_len == 0 from hardware and
program a threshold smaller than what other devices requested.  Caching the
grown threshold in struct aspeed_i3c_vendor_data and restoring
ASPEED_I3C_MST_MRL during reset recovery would preserve it.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-b4-i3c-hci-ast2700-v2-0-4d8fff5c5196@aspeedtech.com?part=7