Re: [PATCH 1/2] bmips: add BCM6358 support

[Ãlvaro FernÃndez Rojas]

I can refine it to support a custom offset for each cpu instead of a generic one, but defining a custom offset for new SoCs such as BCM6368 or BCM6328 would actually break them, since that way the address wouldn't be remapped to 0xb0000000.
See: https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/io.h#L213
In those CPUs the remapping is done automatically (from 0x10000000 to 0xb0000000), since the registers are located in the low 512MB of address space (0x1fffffffULL).

However, the older CPUs such as BCM6358 (or BCM3368) need that custom ioremap, since those registers aren't located in the low 512MB of address space.
If you want, I can do something like this: https://gist.github.com/Noltari/bc5fe029c52cf053a454
And after that we could add other CPUs such as the BCM3368, which needs a different offset: "{ "brcm,bcm3368", 0xfff80000 }"

What do you think? Should we keep a generic offset (0xfff00000) or should we add SoC specific compatible strings with each custom offset?

Regards,
Ãlvaro.

> El 18 ene 2016, a las 7:49, Florian Fainelli <f.fainelli@xxxxxxxxx> escribiÃ:
> 
>> On January 17, 2016 3:28:20 AM PST, "Ãlvaro FernÃndez Rojas" <noltari@xxxxxxxxx> wrote:
>> BCM6358 has a shared TLB which conflicts with current SMP support, so
>> it must
>> be disabled for now.
>> BCM6358 uses >= 0xfff00000 addresses for internal registers, which need
>> to be
>> remapped (by using a simplified version of BRCM63xx ioremap.h).
>> 
>> Signed-off-by: Ãlvaro FernÃndez Rojas <noltari@xxxxxxxxx>
>> ---
>> arch/mips/bmips/setup.c                    | 10 +++++++++
>> arch/mips/include/asm/mach-bmips/ioremap.h | 33
>> ++++++++++++++++++++++++++++++
>> 2 files changed, 43 insertions(+)
>> create mode 100644 arch/mips/include/asm/mach-bmips/ioremap.h
>> 
>> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
>> index 3553528..38b5bd5 100644
>> --- a/arch/mips/bmips/setup.c
>> +++ b/arch/mips/bmips/setup.c
>> @@ -95,6 +95,15 @@ static void bcm6328_quirks(void)
>>        bcm63xx_fixup_cpu1();
>> }
>> 
>> +static void bcm6358_quirks(void)
>> +{
>> +    /*
>> +     * BCM6358 needs special handling for its shared TLB, so
>> +     * disable SMP for now
>> +     */
>> +    bmips_smp_enabled = 0;
>> +}
> 
> That part looks good.
> 
>> +
>> static void bcm6368_quirks(void)
>> {
>>    bcm63xx_fixup_cpu1();
>> @@ -104,6 +113,7 @@ static const struct bmips_quirk bmips_quirk_list[]
>> = {
>>    { "brcm,bcm3384-viper",        &bcm3384_viper_quirks        },
>>    { "brcm,bcm33843-viper",    &bcm3384_viper_quirks        },
>>    { "brcm,bcm6328",        &bcm6328_quirks            },
>> +    { "brcm,bcm6358",        &bcm6358_quirks            },
>>    { "brcm,bcm6368",        &bcm6368_quirks            },
>>    { "brcm,bcm63168",        &bcm6368_quirks            },
>>    { },
> 
> <snip>
> 
>> +
>> +static inline int is_bmips_internal_registers(phys_addr_t offset)
>> +{
>> +    if (offset >= 0xfff00000)
>> +        return 1;
>> +
>> +    return 0;
> 
> That should probably be refined to be looking at the SoC/CPU you are running on, using eventually of_machine_is_compatible on the SoC-specific compatible string. For instance, on 6368 and newer, the physical register offset moves to PA 0x1000_0000.
> 
> Thanks!
> 
> -- 
> Florian