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

[Florian Fainelli <f.fainelli@gmail.com>]

(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@gmail.com>) 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