Message ID | 1529716721-28400-1-git-send-email-linux@roeck-us.net (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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; >
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;
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(+)