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

[Niek Linnenbank <nieklinnenbank@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6340 bytes --]

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é <f4bug@amsat.org>
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é <f4bug@amsat.org>
> ---
> 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

[-- Attachment #2: Type: text/html, Size: 7821 bytes --]