[U-Boot] [PATCH 2/6] MSCC: add support for VCoreIII SoCs

[Gregory CLEMENT]

Hi Daniel,

I am bout to send a new version of the series, but befor I am going to
answer to the pending point I didn't already address in my email 10 days
ago.

 On mer., sept. 26 2018, Daniel Schwierzeck <daniel.schwierzeck at gmail.com> wrote:

> Hi Gregory,
>
> On 25.09.2018 15:01, Gregory CLEMENT wrote:
>> These families of SoCs are found in the Microsemi Switches solution.
>> 
>> Currently the support for two families support is added:
>>  - Ocelot (VSC7513, VSC7514) already supported in Linux
>>  - Luton (Luton10: VSC7423, VSC7424, VSC7428 and Luton26: VSC7425,
>>    VSC7426, VSC7426, VSC7427, VSC7429)
[...]
>>  machdirs := $(patsubst %,arch/mips/mach-%/,$(machine-y))
>>  libs-y += $(machdirs)
>> diff --git a/arch/mips/mach-mscc/Kconfig b/arch/mips/mach-mscc/Kconfig
>> new file mode 100644
>> index 0000000000..20148bfe15
>> --- /dev/null
>> +++ b/arch/mips/mach-mscc/Kconfig
>> @@ -0,0 +1,101 @@
>> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +if ARCH_MSCC
>> +
>> +config SYS_DCACHE_SIZE
>> +	default 32768
>> +
>> +config SYS_DCACHE_LINE_SIZE
>> +	default 32
>> +
>> +config SYS_ICACHE_SIZE
>> +	default 32768
>> +
>> +config SYS_ICACHE_LINE_SIZE
>> +	default 32
>> +
>> +endif
>
> from the code below I assume you have a MIPS24k core? If so you should
> use the automatic cache size detection

Eventually I managed to use the automatic cache size detection.

>> +void vcoreiii_tlb_init(void)
>> +{
>> +        register int tlbix = 0;
>> +
>> +        init_tlb();
>> +
>> +        /* 0x70000000 size 32M (0x02000000) */
>> +        create_tlb(tlbix++, MSCC_IO_ORIGIN1_OFFSET, SZ_16M, MMU_REGIO_RW, MMU_REGIO_RW);
>> +#ifdef CONFIG_SOC_LUTON
>> +	create_tlb(tlbix++, MSCC_IO_ORIGIN2_OFFSET, SZ_16M, MMU_REGIO_RW, MMU_REGIO_RW);
>> +#endif
>> +        /* 0x40000000 - 0x43ffffff - NON-CACHED! */
>> +        /* Flash CS0-3, each 16M = 64M total (16 x 4 below )  */
>> +        create_tlb(tlbix++, MSCC_FLASH_TO,        SZ_16M, MMU_REGIO_RO, MMU_REGIO_RO);
>> +        create_tlb(tlbix++, MSCC_FLASH_TO+SZ_32M, SZ_16M, MMU_REGIO_RO, MMU_REGIO_RO);
>> +
>> +        /* 0x20000000 - up */
>> +#if CONFIG_SYS_SDRAM_SIZE <= SZ_64M
>> +        create_tlb(tlbix++, MSCC_DDR_TO,        SZ_64M,  MMU_REGIO_RW, MMU_REGIO_INVAL);
>> +#elif CONFIG_SYS_SDRAM_SIZE <= SZ_128M
>> +        create_tlb(tlbix++, MSCC_DDR_TO,        SZ_64M,  MMU_REGIO_RW, MMU_REGIO_RW);
>> +#elif CONFIG_SYS_SDRAM_SIZE <= SZ_256M
>> +        create_tlb(tlbix++, MSCC_DDR_TO,       SZ_256M,  MMU_REGIO_RW, MMU_REGIO_INVAL);
>> +#elif CONFIG_SYS_SDRAM_SIZE <= SZ_512M
>> +        create_tlb(tlbix++, MSCC_DDR_TO,       SZ_256M,  MMU_REGIO_RW, MMU_REGIO_RW);
>> +#else  /* 1024M */
>> +        create_tlb(tlbix++, MSCC_DDR_TO,       SZ_512M,  MMU_REGIO_RW, MMU_REGIO_RW);
>> +#endif
>
> can't you leave that to the kernel? U-Boot is only running in kernel
> mode and doesn't need MMU mappings.

Actually, U-Boot doesn't need MMU mappings, and without this chunk of
code U-Boot is running without any problem, but unlike most of the MIPS
based SoCs, the IO register address are not in KSEG0. The mainline linux
kernel built in legacy mode needs to access some of the registers very
early in the boot and make the assumption that the bootloader already
configured them, so we have to match this expectation.

So in code I only keep the tlb entry for MSCC_IO_ORIGIN1_OFFSET and
MSCC_IO_ORIGIN1_OFFSET (where the Io registers are) and I added a
comment to explain it.

>> +}
>> +
>> +int mach_cpu_init(void)
>> +{
>> +        /* Speed up NOR flash access */
>> +#ifdef CONFIG_SOC_LUTON
>> +        writel(ICPU_SPI_MST_CFG_FAST_READ_ENA +
>> +#else
>> +	writel(
>> +#endif
>> +	       ICPU_SPI_MST_CFG_CS_DESELECT_TIME(0x19) +
>> +               ICPU_SPI_MST_CFG_CLK_DIV(9), REG_CFG(ICPU_SPI_MST_CFG));
>> +
>> +        /* Disable all IRQs - map to destination map 0 */
>> +        writel(0, REG_CFG(ICPU_INTR_ENA));
>> +#ifdef CONFIG_SOC_OCELOT
>> +        writel(~0, REG_CFG(ICPU_DST_INTR_MAP(0)));
>> +        writel(0, REG_CFG(ICPU_DST_INTR_MAP(1)));
>> +        writel(0, REG_CFG(ICPU_DST_INTR_MAP(2)));
>> +        writel(0, REG_CFG(ICPU_DST_INTR_MAP(3)));
>> +#else
>> +	writel(ICPU_INTR_IRQ0_ENA_IRQ0_ENA, REG_CFG(ICPU_INTR_IRQ0_ENA));
>> +#endif
>
> do you really need to disable interrupts after a cold or warm boot?

It was a workaround for an issue in a legacy kernel that is now fixed so
indeed can remove it.

[...]

>> +
>> +static inline void init_tlb(void)
>> +{
>> +    register int i, max;
>> +
>> +    max = get_tlb_count();
>> +    for(i = 0; i < max; i++)
>> +        create_tlb(i, i * SZ_1M, SZ_4K, MMU_REGIO_INVAL, MMU_REGIO_INVAL);
>> +}
>
> again can't you leave the setup of MMU mappings to the kernel?

I removed this part

[...]

Gregory

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com