[PATCH v2 10/11] arm: Add Aspeed machine

[Benjamin Herrenschmidt]

On Thu, 2016-04-21 at 10:35 +0200, Arnd Bergmann wrote:
> 
> > +#define SCU_PASSWORD	0x1688A8A8
> > +
> > +static void __init aspeed_init_early(void)
> > +{
> > +	u32 reg;
> > +
> > +	/*
> > +	 * Unlock SCU
> > +	 */
> > +	writel(SCU_PASSWORD, AST_IO(AST_BASE_SCU));
> > +
> > +	/* We enable the UART clock divisor in the SCU's misc control
> > +	 * register, as the baud rates in aspeed.dtb all assume that the
> > +	 * divisor is active
> > +	 */
> > +	reg = readl(AST_IO(AST_BASE_SCU | 0x2c));
> > +	writel(reg | 0x00001000, AST_IO(AST_BASE_SCU | 0x2c));

> Can you explain a bit more about this? I would assume that the UART
> that is used for the console is working at the point that the bootloader
> hands over to the kernel, while the other uarts don't need to be
> active this early. Why do you need to do this at such an early stage

It may or may not be already correct, you don't necessarily have a
serial enabled bootloader or it might be using a UART driver that
doesn't use the 'standard' divisors etc... This just sanitizes it.

This register contains misc controls. That specific bit controls a
clock divisor by 13 from the main 24Mhz source, which provides a
reasonably "standard" 1.84 Mhz input to the UARTs.

> > 
> > +	/*
> > +	 * Disable the watchdogs
> > +	 */
> > +	writel(0, AST_IO(AST_BASE_WDT | 0x0c));
> > +	writel(0, AST_IO(AST_BASE_WDT | 0x2c));
> > +}
> Similarly here: why so early? Is the initial timeout too short to wait
> for the watchdog driver to come up? I think it makes sense to require
> the watchdog driver to be loaded if a watchdog is enabled during boot,
> and that keeps the register access in one place.

Cheers
Ben.