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

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

(please don't top post)

Le 18/01/2016 01:42, Álvaro Fernández Rojas a écrit :
> 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).

These registers are always accessible AFAIR, either through KSEG3
(0xFF00_0000), or through KSEG1 (0xB000_0000) for newer SoCs, and in
arch/mips/include/asm/io.h, the first thing we do is call
plat_ioremap(), if the address returned is valid, we just bail out and
do not execute the snippet you are indicating.

> 
> 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?

I would prefer we maintain the existing logic from
arch/mips/include/asm/mach-bcm63xx/ioremap.h. If needed we could do this
in a two level step, where plat_ioremap() calls a helper function, and
this helper function has been scanning the Device Tree for the bus
register space.

Jonas' suggestion works too.

> 
> Regards,
> Álvaro.
> 
>> El 18 ene 2016, a las 7:49, Florian Fainelli <f.fainelli@gmail.com> escribió:
>>
>>> On January 17, 2016 3:28:20 AM PST, "Álvaro Fernández Rojas" <noltari@gmail.com> 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@gmail.com>
>>> ---
>>> 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


-- 
Florian