linux-kernel - Re: [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wmdmqqxf.fsf@bootlin.com>
Date: Wed, 19 Feb 2025 09:28:28 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Stanley Chu <stanley.chuys@...il.com>
Cc: frank.li@....com,  alexandre.belloni@...tlin.com,
  linux-i3c@...ts.infradead.org,  linux-kernel@...r.kernel.org,
  tomer.maimon@...oton.com,  kwliu@...oton.com,  yschu@...oton.com
Subject: Re: [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue

Hello Stanley,

On 19/02/2025 at 09:10:18 +08, Stanley Chu <stanley.chuys@...il.com> wrote:

> npcm845 HW issue:
> I3C HW stalls the write transfer if the transmit FIFO becomes empty,
> when new data is written to FIFO, I3C HW resumes the transfer but
> the first transmitted data bit may have the wrong value.
> Workaround:
> Fill the FIFO in advance to prevent FIFO from becoming empty.
>
> Signed-off-by: Stanley Chu <yschu@...oton.com>
> ---

...

> @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
>  	unsigned int dev_nb = 0, last_addr = 0;
>  	u32 reg;
>  	int ret, i;
> +	u32 dyn_addr;
>  
>  	while (true) {
>  		/* clean SVC_I3C_MINT_IBIWON w1c bits */
> @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
>  		if (SVC_I3C_MSTATUS_RXPEND(reg)) {
>  			u8 data[6];
>  
> +			/*
> +			 * SVC_I3C_QUIRK_FIFO_EMPTY fix:
> +			 * The TX FIFO should be ready before the TX transmission is starting.
> +			 */
> +			dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1);
> +			if (dyn_addr < 0)
> +				return dyn_addr;
> +			writel(dyn_addr, master->regs + SVC_I3C_MWDATAB);

Maybe Frank can test, but it feels like there are chances it will break
the other controller.

Perhaps the other quirk applies to the other controller though.


Regarding your series, patch 1 does not make much sense alone, and you
should anyway write a binding patch for your new compatible (first in
the series).

Thanks,
Miquèl