Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311137 > unrolled thread

[PATCH 1/2] bmips: add BCM6358 support

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] bmips: add BCM6358 support Álvaro Fernández Rojas   <noltari@gmail.com> - 2016-01-17 12:30 +0100
    Re: [PATCH 1/2] bmips: add BCM6358 support Florian Fainelli <f.fainelli@gmail.com> - 2016-01-18 07:50 +0100
      Re: [PATCH 1/2] bmips: add BCM6358 support Álvaro Fernández Rojas <noltari@gmail.com> - 2016-01-18 10:50 +0100
        Re: [PATCH 1/2] bmips: add BCM6358 support Jonas Gorski <jogo@openwrt.org> - 2016-01-18 14:40 +0100
        Re: [PATCH 1/2] bmips: add BCM6358 support Florian Fainelli <f.fainelli@gmail.com> - 2016-01-18 20:30 +0100
        Re: [PATCH 1/2] bmips: add BCM6358 support Florian Fainelli <f.fainelli@gmail.com> - 2016-01-19 01:10 +0100

#1311137 — [PATCH 1/2] bmips: add BCM6358 support

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;
+}
+
 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			},
 	{ },
diff --git a/arch/mips/include/asm/mach-bmips/ioremap.h b/arch/mips/include/asm/mach-bmips/ioremap.h
new file mode 100644
index 0000000..b1a75f8
--- /dev/null
+++ b/arch/mips/include/asm/mach-bmips/ioremap.h
@@ -0,0 +1,33 @@
+#ifndef __ASM_MACH_BMIPS_IOREMAP_H
+#define __ASM_MACH_BMIPS_IOREMAP_H
+
+#include <linux/types.h>
+
+static inline phys_addr_t fixup_bigphys_addr(phys_addr_t phys_addr, phys_addr_t size)
+{
+	return phys_addr;
+}
+
+static inline int is_bmips_internal_registers(phys_addr_t offset)
+{
+	if (offset >= 0xfff00000)
+		return 1;
+
+	return 0;
+}
+
+static inline void __iomem *plat_ioremap(phys_addr_t offset, unsigned long size,
+					 unsigned long flags)
+{
+	if (is_bmips_internal_registers(offset))
+		return (void __iomem *)offset;
+
+	return NULL;
+}
+
+static inline int plat_iounmap(const volatile void __iomem *addr)
+{
+	return is_bmips_internal_registers((unsigned long)addr);
+}
+
+#endif /* __ASM_MACH_BMIPS_IOREMAP_H */
-- 
1.9.1

[toc] | [next] | [standalone]

#1311340

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

[toc] | [prev] | [next] | [standalone]

#1311426

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

[toc] | [prev] | [next] | [standalone]

#1311556

Hi,

On 18 January 2016 at 10:42, Álvaro Fernández Rojas <noltari@gmail.com> wrote:
> 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?

How about using the arm approach [1] and adding support for
registering static mappings? That way we avoid adding a custom
plat_ioremap() and keep the amount of bmips code to a minimum.


Jonas

[1]:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ed8fd2186a4e4f3b98434093b56f9b793d48443e
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=101eeda38c0ab8a4f916176e325d9e036d981a24

[toc] | [prev] | [next] | [standalone]

#1311767

(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

[toc] | [prev] | [next] | [standalone]

#1311861

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).

I see what you mean by that now, we can indeed drop these registers from
plat_ioremap() since the fallback already takes care of that for us.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web