Re: [Qemu-devel] [PATCH v2] ppc: Fix sam460ex devicetree when booting the Linux kernel

[BALATON Zoltan]

On Fri, 22 Jun 2018, Guenter Roeck wrote:
> sam4660ex (or at least this emulation) does not support the "ibm,cpm" power
> management. As a result, Linux crashes when trying to access it. Remove
> its devicetree node. Also, if/when we boot the Linux kernel directly,
> serial port clock frequencies in the devicetree file will be unset, and
> serial port initialization will fail. Add plausible frequency values to
> the serials port to be able to use it. Also set valid values for the other
> clock nodes otherwise set by u-boot.

Apart from small fixes in commit message such as:
- prefix title with sam460ex: instead of ppc: to be more specific
- sam4660ex -> sam460ex
- serials port -> serial ports
- device tree may be two words but not sure and not so important

Should we set the correct frequencies set by u-boot instead of some 
plausible value? At boot u-boot prints:

CPU:   AMCC PowerPC 460EX Rev. B at 1150 MHz (PLB=230 OPB=115 EBC=115)

Where PLB, OPB and EBC freqs are not 50MHz. I'm not sure this would cause 
any problem but previously I've seen timing issues by clocks going slower 
or faster when such clock freqs are wrong.

Could you at least #define a constant next to CPU_FREQ instead of 
hardcoding values that appear in multiple places.

Thank you,
BALATON Zoltan

> Signed-off-by: Guenter Roeck <address@hidden>
> ---
> v2: Initialize all serial nodes to match u-boot behavior more closely.
>    Use direct fdt API functions and ignore errors when clearing out
>    /cpm and for setting the serial port clocks.
>
> hw/ppc/sam460ex.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index bdc53d2..a91382c 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -37,6 +37,8 @@
> #include "hw/i2c/smbus.h"
> #include "hw/usb/hcd-ehci.h"
>
> +#include <libfdt.h>
> +
> #define BINARY_DEVICE_TREE_FILE "canyonlands.dtb"
> #define UBOOT_FILENAME "u-boot-sam460-20100605.bin"
> /* to extract the official U-Boot bin from the updater: */
> @@ -255,6 +257,7 @@ static int sam460ex_load_device_tree(hwaddr addr,
>     void *fdt;
>     uint32_t tb_freq = CPU_FREQ;
>     uint32_t clock_freq = CPU_FREQ;
> +    int offset;
>
>     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
>     if (!filename) {
> @@ -308,6 +311,26 @@ static int sam460ex_load_device_tree(hwaddr addr,
>     qemu_fdt_setprop_cell(fdt, "/cpus/address@hidden", "timebase-frequency",
>                               tb_freq);
>
> +    /* Remove cpm node if it exists (it is not emulated) */
> +    offset = fdt_path_offset(fdt, "/cpm");
> +    if (offset >= 0)
> +        fdt_nop_node(fdt, offset);
> +
> +    /* set serial port clocks */
> +    offset = fdt_node_offset_by_compatible(fdt, -1, "ns16550");
> +    while (offset >= 0) {
> +        fdt_setprop_cell(fdt, offset, "clock-frequency", 50000000);
> +        offset = fdt_node_offset_by_compatible(fdt, offset, "ns16550");
> +    }
> +
> +    /* some more clocks */
> +    qemu_fdt_setprop_cell(fdt, "/plb", "clock-frequency",
> +                              50000000);
> +    qemu_fdt_setprop_cell(fdt, "/plb/opb", "clock-frequency",
> +                              50000000);
> +    qemu_fdt_setprop_cell(fdt, "/plb/opb/ebc", "clock-frequency",
> +                              50000000);
> +
>     rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
>     g_free(fdt);
>     ret = fdt_size;