Re: [PATCH] mips: bmips: BCM6358: disable arch_sync_dma_for_cpu_all()

From: Florian Fainelli
Date: Mon Mar 13 2023 - 17:46:44 EST


(please don't top post)

On 3/13/23 14:39, Álvaro Fernández Rojas wrote:
Hi Florian,

I did another test changing from TP1 to TP0 and this is the result:
[ 0.000000] Linux version 5.15.98 (noltari@atlantis)
(mips-openwrt-linux-musl-gcc (OpenWrt GCC 12.2.0 r22187+1-19817fa3f5)
12.2.0, GNU ld (GNU Binutils) 2.40.0) #0 SMP Sun Mar 12 18:23:28 2023
[ 0.000000] bmips_cpu_setup: read_c0_brcm_config_0() = 0xe30e1006
[ 0.000000] bmips_cpu_setup: BMIPS_RAC_CONFIG = 0x2a00015
[ 0.000000] CPU0 revision is: 0002a010 (Broadcom BMIPS4350)

And there were no exceptions with EHCI/OHCI as opposed to TP1.
So the issue is only happening when booting from TP1.

Ah, that explains it then, I was just about to ask you which TP was the kernel booted on.

Maybe it's due to the fact that BCM6358 has a shared TLB?

I think it has to do with the fact that the BMIPS_RAC_CONFIG_1 is likely not enabling the RAC since that register pertains to TP1, could you dump its contents, and if they do not set bit 0 and/or 1, please set them and try again and see whether it works any better? The RAC provides substantial performance improvements, it would be a change to keep it disabled.


Maybe the correct way of solving the issue would be by adding the
following code at bcm6358_quirks():
if (read_c0_brcm_cmt_local() & (1 << 31))
bmips_dma_sync_disabled = 1;

BTW, if I understood it correctly, you want me to reverse the logic,
so bmips_dma_sync_disabled instead of bmips_dma_sync_enabled.
Is this correct?

Yes, I want the logic such that we need to set a variable to 1/true rather setting one to 0, less change to get it wrong IMHO.


Best regards,
Álvaro.


El lun, 13 mar 2023 a las 18:37, Florian Fainelli
(<f.fainelli@xxxxxxxxx>) escribió:

On 3/12/23 11:50, Álvaro Fernández Rojas wrote:
Hi Florian,

I tried what you suggested but it stil panics on EHCI:

[ 0.000000] Linux version 5.15.98 (noltari@atlantis)
(mips-openwrt-linux-musl-gcc (OpenWrt GCC 12.2.0 r22187+1-19817fa3f5)
12.2.0, GNU ld (GNU Binutils) 2.40.0) #0 SMP Sun Mar 12 18:23:28 2023
[ 0.000000] bmips_cpu_setup: read_c0_brcm_config_0() = 0xe30e1006
[ 0.000000] bmips_cpu_setup: cbr + BMIPS_RAC_CONFIG = 0x3c1b8041
[ 0.000000] CPU0 revision is: 0002a010 (Broadcom BMIPS4350)

It looks like bit 29 is set so RAC should be present.
And RAC_I seems to be set, but not RAC_D...

BTW, this is what I added to bmips_cpu_setup:

case CPU_BMIPS4350:
cfg = read_c0_brcm_config_0();
pr_info("bmips_cpu_setup: read_c0_brcm_config_0() = 0x%x\n", cfg);

cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG);
pr_info("bmips_cpu_setup: cbr + BMIPS_RAC_CONFIG = 0x%x\n", cfg);
__raw_writel(cfg | BIT(0) | BIT(1), cbr + BMIPS_RAC_CONFIG);
__raw_readl(cbr + BMIPS_RAC_CONFIG);
break;

Thanks for running those experiments, I cannot explain what you are
seeing, so there must be some sort of erratum applicable to the
BMIPS4380 revision used on the 6358 somehow...

If you can make the suggested change to use negative logic in order to
disable the RAC flushing, that would work for me, also maybe add a
Fixes: tag so it gets backported to stable trees?

Thanks!
--
Florian


--
Florian