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

Guenter Roeck posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1529716721-28400-1-git-send-email-linux@roeck-us.net
Test checkpatch failed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
hw/ppc/sam460ex.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
[Qemu-devel] [PATCH v2] ppc: Fix sam460ex devicetree when booting the Linux kernel
Posted by Guenter Roeck 5 years, 9 months ago
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.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
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/cpu@0", "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;
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2] ppc: Fix sam460ex devicetree when booting the Linux kernel
Posted by no-reply@patchew.org 5 years, 9 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1529716721-28400-1-git-send-email-linux@roeck-us.net
Subject: [Qemu-devel] [PATCH v2] ppc: Fix sam460ex devicetree when booting the Linux kernel

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
55b21e6a4f ppc: Fix sam460ex devicetree when booting the Linux kernel

=== OUTPUT BEGIN ===
Checking PATCH 1/1: ppc: Fix sam460ex devicetree when booting the Linux kernel...
ERROR: braces {} are necessary for all arms of this statement
#44: FILE: hw/ppc/sam460ex.c:316:
+    if (offset >= 0)
[...]

total: 1 errors, 0 warnings, 41 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH v2] ppc: Fix sam460ex devicetree when booting the Linux kernel
Posted by BALATON Zoltan 5 years, 9 months ago
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 <linux@roeck-us.net>
> ---
> 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/cpu@0", "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;
>