Hi Florian, El lun, 13 mar 2023 a las 22:46, Florian Fainelli (<f.fainelli@xxxxxxxxx>) escribió: > > (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. This is the code that I added to bmips_cpu_setup(): case CPU_BMIPS4350: cfg = read_c0_brcm_cmt_local(); pr_info("bmips_cpu_setup: read_c0_brcm_cmt_local() = 0x%x\n", cfg); 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_ADDRESS_RANGE); pr_info("bmips_cpu_setup: BMIPS_RAC_ADDRESS_RANGE = 0x%x\n", cfg); cfg = __raw_readl(cbr + BMIPS_L2_CONFIG); pr_info("bmips_cpu_setup: BMIPS_L2_CONFIG = 0x%x\n", cfg); cfg = __raw_readl(cbr + BMIPS_LMB_CONTROL); pr_info("bmips_cpu_setup: BMIPS_LMB_CONTROL = 0x%x\n", cfg); cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG); pr_info("bmips_cpu_setup: BMIPS_RAC_CONFIG = 0x%x\n", cfg); __raw_writel(cfg | BIT(0) | BIT(1), cbr + BMIPS_RAC_CONFIG); pr_info("bmips_cpu_setup: BMIPS_RAC_CONFIG = 0x%x\n", cfg); __raw_readl(cbr + BMIPS_RAC_CONFIG); cfg = __raw_readl(cbr + BMIPS_RAC_CONFIG_1); pr_info("bmips_cpu_setup: BMIPS_RAC_CONFIG_1 = 0x%x\n", cfg); __raw_writel(cfg | BIT(0) | BIT(1), cbr + BMIPS_RAC_CONFIG_1); pr_info("bmips_cpu_setup: BMIPS_RAC_CONFIG_1 = 0x%x\n", cfg); __raw_readl(cbr + BMIPS_RAC_CONFIG_1); break; And this is the result: [ 0.000000] bmips_cpu_setup: read_c0_brcm_cmt_local() = 0x80000000 [ 0.000000] bmips_cpu_setup: read_c0_brcm_config_0() = 0xe30e1006 [ 0.000000] bmips_cpu_setup: BMIPS_RAC_ADDRESS_RANGE = 0x277bdab0 [ 0.000000] bmips_cpu_setup: BMIPS_L2_CONFIG = 0x241a0008 [ 0.000000] bmips_cpu_setup: BMIPS_LMB_CONTROL = 0x0 [ 0.000000] bmips_cpu_setup: BMIPS_RAC_CONFIG = 0x3c1b8041 [ 0.000000] bmips_cpu_setup: BMIPS_RAC_CONFIG = 0x3c1b8041 [ 0.000000] bmips_cpu_setup: BMIPS_RAC_CONFIG_1 = 0x3600008 [ 0.000000] bmips_cpu_setup: BMIPS_RAC_CONFIG_1 = 0x3600008 As you can see the bit's aren't set and all the registers appear to have strange values and not the usual ones when initialized by the bootloader... > > > > > 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 > Álvaro