On 3/14/23 11:14, Álvaro Fernández Rojas wrote:
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
OK, so we are executing from TP1, but we knew that already.
[ 0.000000] bmips_cpu_setup: read_c0_brcm_config_0() = 0xe30e1006
bit 31: instruction cache enabled
bit 30: data cache enabled
bit 29: RAC present
bit 25: DSU power enabled
bit 24: data cache power enabled
bit 19: low-latency memory bus (LMB) present
bit 18: concurrent multi threading (CMT) present
bit 17: reserved
bit 12: split instruction cache
bit 2: number of Hi/Lo special registers - 1
bit 1: eDSP present
This seems to match the recommended and default values
[ 0.000000] bmips_cpu_setup: BMIPS_RAC_ADDRESS_RANGE = 0x277bdab0
That does not look intended, the reset value is supposed to be 0. Can
you apply the same values as the ones programmed in the CPU_BMIPS_3300 case?
[ 0.000000] bmips_cpu_setup: BMIPS_L2_CONFIG = 0x241a0008
[ 0.000000] bmips_cpu_setup: BMIPS_LMB_CONTROL = 0x0
LMB not enabled, that's OK.
[ 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...
Yes, so maybe the best way to go about is indeed to go with your change
such that if we are running on TP1, it is safe to assume that CFE has
not done any sensible initialization, and we have a non functional RAC.
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
--
Florian
