Re: [RFC PATCH 6/6] hw/arm/cubieboard: Disable unsupported M-USB in device tree blob

[Niek Linnenbank]

Hello Philippe,

I have a suggestion: probably you can apply (almost) the same patch to get USB

working for A10 as I did in the Allwinner H3, in patch #4 "add USB host controller" [1]

That way you can avoid the DTB modifications and with low effort get USB working as well for this board.

As far as I can see, in Section 21.1 in the A10 user manual [2] has the same description

as for the H3. It basically has the standard EHCI and OHCI interfaces.

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg03266.html

[2] https://linux-sunxi.org/File:Allwinner_A10_User_manual_V1.5.pdf

Regards,

Niek

On Mon, Dec 30, 2019 at 12:10 PM Philippe Mathieu-Daudé <address@hidden> wrote:

We do not model the Sunxi Multipoint USB.
The Linux kernel OOPS when booting:

  ------------[ cut here ]------------
  WARNING: CPU: 0 PID: 144 at drivers/usb/musb/sunxi.c:400 sunxi_musb_ep_offset+0x39/0x3c
  sunxi_musb_ep_offset called with non 0 offset
  Modules linked in:
  CPU: 0 PID: 144 Comm: kworker/0:2 Not tainted 4.20.7-sunxi #5.75
  Hardware name: Allwinner sun4i/sun5i Families
  Workqueue: events deferred_probe_work_func
  [<c010d77d>] (unwind_backtrace) from [<c010a425>] (show_stack+0x11/0x14)
  [<c010a425>] (show_stack) from [<c08d9141>] (dump_stack+0x69/0x78)
  [<c08d9141>] (dump_stack) from [<c011b161>] (__warn+0xa1/0xb4)
  [<c011b161>] (__warn) from [<c011b1a7>] (warn_slowpath_fmt+0x33/0x48)
  [<c011b1a7>] (warn_slowpath_fmt) from [<c0725c81>] (sunxi_musb_ep_offset+0x39/0x3c)
  [<c0725c81>] (sunxi_musb_ep_offset) from [<c071b481>] (ep_config_from_hw+0x99/0x104)
  [<c071b481>] (ep_config_from_hw) from [<c071c8d9>] (musb_probe+0x765/0xa0c)
  [<c071c8d9>] (musb_probe) from [<c063fa4f>] (platform_drv_probe+0x33/0x68)
  [<c063fa4f>] (platform_drv_probe) from [<c063e4ef>] (really_probe+0x16f/0x1e0)
  [<c063e4ef>] (really_probe) from [<c063e67f>] (driver_probe_device+0x43/0x11c)
  [<c063e67f>] (driver_probe_device) from [<c063d0cf>] (bus_for_each_drv+0x37/0x70)
  [<c063d0cf>] (bus_for_each_drv) from [<c063e32f>] (__device_attach+0x83/0xc8)
  [<c063e32f>] (__device_attach) from [<c063da8b>] (bus_probe_device+0x5b/0x60)
  [<c063da8b>] (bus_probe_device) from [<c063b7a5>] (device_add+0x2f5/0x474)
  [<c063b7a5>] (device_add) from [<c063f8ef>] (platform_device_add+0xb7/0x184)
  [<c063f8ef>] (platform_device_add) from [<c06400df>] (platform_device_register_full+0xb3/0xc4)
  [<c06400df>] (platform_device_register_full) from [<c0725a2f>] (sunxi_musb_probe+0x1d7/0x2f4)
  [<c0725a2f>] (sunxi_musb_probe) from [<c063fa4f>] (platform_drv_probe+0x33/0x68)
  [<c063fa4f>] (platform_drv_probe) from [<c063e4ef>] (really_probe+0x16f/0x1e0)
  [<c063e4ef>] (really_probe) from [<c063e67f>] (driver_probe_device+0x43/0x11c)
  [<c063e67f>] (driver_probe_device) from [<c063d0cf>] (bus_for_each_drv+0x37/0x70)
  [<c063d0cf>] (bus_for_each_drv) from [<c063e32f>] (__device_attach+0x83/0xc8)
  [<c063e32f>] (__device_attach) from [<c063da8b>] (bus_probe_device+0x5b/0x60)
  [<c063da8b>] (bus_probe_device) from [<c063ddcf>] (deferred_probe_work_func+0x4b/0x6c)
  [<c063ddcf>] (deferred_probe_work_func) from [<c012e38b>] (process_one_work+0x167/0x384)
  [<c012e38b>] (process_one_work) from [<c012f07d>] (worker_thread+0x251/0x3fc)
  [<c012f07d>] (worker_thread) from [<c0132949>] (kthread+0xfd/0x104)
  [<c0132949>] (kthread) from [<c01010f9>] (ret_from_fork+0x11/0x38)
  Exception stack(0xc6999fb0 to 0xc6999ff8)
  9fa0:                                     00000000 00000000 00000000 00000000
  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
  ---[ end trace b309edbca98c7de2 ]---
  musb-sunxi 1c13000.usb: Error unknown readb offset 128
  musb-hdrc musb-hdrc.1.auto: musb_init_controller failed with status -22
  musb-hdrc: probe of musb-hdrc.1.auto failed with error -22

This is not critical but confusing. To avoid the Linux kernel to
probe this device, mark it disabled in the device tree blob.

Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
---
I'm not sure if this is a QEMU anti-pattern or bad practice.
I know we prefer to be as close to the hardware as possible,
but here the hardware is not changed, the dtb is. However
this makes the guest behave differently. At least we don't
have to manually edit the dts. If this is only annoying for
acceptance testing, we might consider manually editing the
dts in the tests setup().
---
 hw/arm/cubieboard.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 6dc2f1d6b6..dd10577696 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -22,10 +22,34 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/arm/allwinner-a10.h"
+#include <libfdt.h>
+
+static void cubieboard_modify_dtb(const struct arm_boot_info *info, void *fdt)
+{
+    static const char unsupported_compat[] = "allwinner,sun4i-a10-musb";
+    char node_path[72];
+    int offset;
+
+    offset = fdt_node_offset_by_compatible(fdt, -1, unsupported_compat);
+    while (offset >= 0) {
+        int r = fdt_get_path(fdt, offset, node_path, sizeof(node_path));
+        assert(r >= 0);
+        r = fdt_setprop_string(fdt, offset, "status", "disabled");
+        if (r < 0) {
+            error_report("%s: Couldn't disable %s: %s", __func__,
+                         unsupported_compat, fdt_strerror(r));
+            exit(1);
+        }
+        warn_report("cubieboard: disabled unsupported node %s (%s) "
+                    "in device tree", node_path, unsupported_compat);
+        offset = fdt_node_offset_by_compatible(fdt, offset, unsupported_compat);
+    }
+}

 static struct arm_boot_info cubieboard_binfo = {
     .loader_start = AW_A10_SDRAM_BASE,
     .board_id = 0x1008,
+    .modify_dtb = cubieboard_modify_dtb,
 };

 typedef struct CubieBoardState {
--
2.21.0

--

Niek Linnenbank