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

[Alexandre Belloni]

On 27/09/2018 12:14:14+0200, Gregory CLEMENT wrote:
> Hi Daniel,
> 
> First thanks for you prompt review, it is much appreciate. :)
> 
> This week I am at kernel recipes conference, so I won't be able to fully
> address your comments but I will do it next week.
> 
> However, here are some answers:
> 
>  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)
> >
> > Is this some polished version of the original vendor U-Boot?
> 
> No the original vendor version was RedBoot
> 
> > Could you maybe add Ocelot and Luton in separate patches?
> 
> Yes sure the intent to have a uniq patch was to justify the common code
> between both SoC.
> 
> >
> >> 
> >> Signed-off-by: Gregory CLEMENT <gregory.clement at bootlin.com>
> >> ---
> [..]
> 
> >> +endif
> >
> > from the code below I assume you have a MIPS24k core? If so you should
> > use the automatic cache size detection
> 
> Yes it is a MIPS24k core. I will have a look on the automatic cache size
> detection.
> 
> >
> >> +
> >> +menu "MSCC VCore-III platforms"
> >> +	depends on ARCH_MSCC
> >> +
> >> +config SOC_VCOREIII
> >> +	select SUPPORTS_LITTLE_ENDIAN
> >> +	select SUPPORTS_BIG_ENDIAN
> >> +	select SUPPORTS_CPU_MIPS32_R1
> >> +	select SUPPORTS_CPU_MIPS32_R2
> >> +	select ROM_EXCEPTION_VECTORS
> >> +	select MIPS_TUNE_24KC
> >> +	bool
> >
> > sort this alpahetically
> 
> OK
> 
> >
> >> +
> [...]
> 
> >> +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.
> 
> You should be right, I will check it.
> 

At least MSCC_IO_ORIGIN1_OFFSET is necessary to get earlyprintk working
because the SoC registers are not in kseg0.

> >> +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?
> 
> I think it is needed, but I will check.
> 

I would think this was done to avoid the spi driver crash. The interrupt
mapping should stay though.

> 
> >> +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 will check again
> 

No, we need those mappings for earlyprintk and SoC detection. They are
needed to boot legacy kernels.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com