All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Andrew Jeffery" <andrew@aj.id.au>,
	qemu-devel@nongnu.org, qemu-arm@nongnu.org,
	"Cédric Le Goater" <clg@kaod.org>,
	"Peter Delevoryas" <pdel@fb.com>, "Joel Stanley" <joel@jms.id.au>
Subject: [PULL 12/14] hw/arm/aspeed: Initialize AST2600 UART clock selection registers
Date: Mon, 20 Sep 2021 10:09:26 +0200	[thread overview]
Message-ID: <20210920080928.1055567-13-clg@kaod.org> (raw)
In-Reply-To: <20210920080928.1055567-1-clg@kaod.org>

From: Peter Delevoryas <pdel@fb.com>

UART5 is typically used as the default debug UART on the AST2600, but
UART1 is also designed to be a debug UART. All the AST2600 UART's have
semi-configurable clock rates through registers in the System Control
Unit (SCU), but only UART5 works out of the box with zero-initialized
values. The rest of the UART's expect a few of the registers to be
initialized to non-zero values, or else the clock rate calculation will
yield zero or undefined (due to a divide-by-zero).

For reference, the U-Boot clock rate driver here shows the calculation:

    https://github.com/facebook/openbmc-uboot/blob/15f7e0dc01d8/drivers/clk/aspeed/clk_ast2600.c#L357

To summarize, UART5 allows selection from 4 rates: 24 MHz, 192 MHz, 24 /
13 MHz, and 192 / 13 MHz. The other UART's allow selecting either the
"low" rate (UARTCLK) or the "high" rate (HUARTCLK). UARTCLK and HUARTCLK
are configurable themselves:

    UARTCLK = UXCLK * R / (N * 2)
    HUARTCLK = HUXCLK * HR / (HN * 2)

UXCLK and HUXCLK are also configurable, and depend on the APLL and/or
HPLL clock rates, which also derive from complicated calculations. Long
story short, there's lots of multiplication and division from
configurable registers, and most of these registers are zero-initialized
in QEMU, which at best is unexpected and at worst causes this clock rate
driver to hang from divide-by-zero's. This can also be difficult to
diagnose, because it may cause U-Boot to hang before serial console
initialization completes, requiring intervention from gdb.

This change just initializes all of these registers with default values
from the datasheet.

To test this, I used Facebook's AST2600 OpenBMC image for "fuji", with
the following diff applied (because fuji uses UART1 for console output,
not UART5).

  @@ -323,8 +323,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
       }

      /* UART - attach an 8250 to the IO space as our UART5 */
  -    serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART5], 2,
  -                   aspeed_soc_get_irq(s, ASPEED_DEV_UART5),
  +    serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART1], 2,
  +                   aspeed_soc_get_irq(s, ASPEED_DEV_UART1),
                    38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);

       /* I2C */

Without these clock rate registers being initialized, U-Boot hangs in
the clock rate driver from a divide-by-zero, because the UART1 clock
rate register reads return zero, and there's no console output. After
initializing them with default values, fuji boots successfully.

Signed-off-by: Peter Delevoryas <pdel@fb.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
[ clg: Removed _PARAM suffix ]
Message-Id: <20210906134023.3711031-2-pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/misc/aspeed_scu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 05edebedeb46..d06e179a6e65 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -119,6 +119,8 @@
 #define AST2600_CLK_SEL3          TO_REG(0x308)
 #define AST2600_CLK_SEL4          TO_REG(0x310)
 #define AST2600_CLK_SEL5          TO_REG(0x314)
+#define AST2600_UARTCLK           TO_REG(0x338)
+#define AST2600_HUARTCLK          TO_REG(0x33C)
 #define AST2600_HW_STRAP1         TO_REG(0x500)
 #define AST2600_HW_STRAP1_CLR     TO_REG(0x504)
 #define AST2600_HW_STRAP1_PROT    TO_REG(0x508)
@@ -681,6 +683,8 @@ static const uint32_t ast2600_a3_resets[ASPEED_AST2600_SCU_NR_REGS] = {
     [AST2600_CLK_SEL3]          = 0x00000000,
     [AST2600_CLK_SEL4]          = 0xF3F40000,
     [AST2600_CLK_SEL5]          = 0x30000000,
+    [AST2600_UARTCLK]           = 0x00014506,
+    [AST2600_HUARTCLK]          = 0x000145C0,
     [AST2600_CHIP_ID0]          = 0x1234ABCD,
     [AST2600_CHIP_ID1]          = 0x88884444,
 };
-- 
2.31.1



  parent reply	other threads:[~2021-09-20  8:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-20  8:09 [PULL 00/14] aspeed queue Cédric Le Goater
2021-09-20  8:09 ` [PULL 01/14] hw: arm: aspeed: Enable eth0 interface for aspeed-ast2600-evb Cédric Le Goater
2021-09-20  8:09 ` [PULL 02/14] hw: arm: aspeed: Enable mac0/1 instead of mac1/2 for g220a Cédric Le Goater
2021-09-20  8:09 ` [PULL 03/14] watchdog: aspeed: Sanitize control register values Cédric Le Goater
2021-09-20  8:09 ` [PULL 04/14] watchdog: aspeed: Fix sequential control writes Cédric Le Goater
2021-09-20  8:09 ` [PULL 05/14] hw: aspeed_gpio: Simplify 1.8V defines Cédric Le Goater
2021-09-20  8:09 ` [PULL 06/14] hw: aspeed_gpio: Clarify GPIO controller name Cédric Le Goater
2021-09-20  8:09 ` [PULL 07/14] misc/pca9552: Fix LED status register indexing in pca955x_get_led() Cédric Le Goater
2021-09-20  8:09 ` [PULL 08/14] arm/aspeed: rainier: Add i2c eeproms and muxes Cédric Le Goater
2021-09-20  8:09 ` [PULL 09/14] aspeed: Emulate the AST2600A3 Cédric Le Goater
2021-09-20  8:09 ` [PULL 10/14] hw/misc: Add Infineon DPS310 sensor model Cédric Le Goater
2021-09-20  8:09 ` [PULL 11/14] arm/aspeed: Add DPS310 to Witherspoon and Rainier Cédric Le Goater
2021-09-20  8:09 ` Cédric Le Goater [this message]
2021-09-20  8:09 ` [PULL 13/14] hw/arm/aspeed: Allow machine to set UART default Cédric Le Goater
2021-09-20  8:09 ` [PULL 14/14] hw/arm/aspeed: Add Fuji machine type Cédric Le Goater
2021-09-21 14:07 ` [PULL 00/14] aspeed queue Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2021-09-13 16:12 Cédric Le Goater
2021-09-13 16:13 ` [PULL 12/14] hw/arm/aspeed: Initialize AST2600 UART clock selection registers Cédric Le Goater
2021-09-03 19:40 [PULL 00/14] aspeed queue Cédric Le Goater
2021-09-03 19:41 ` [PULL 12/14] hw/arm/aspeed: Initialize AST2600 UART clock selection registers Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210920080928.1055567-13-clg@kaod.org \
    --to=clg@kaod.org \
    --cc=andrew@aj.id.au \
    --cc=joel@jms.id.au \
    --cc=pdel@fb.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.