Re: PR 60404: Beaglebone Black: nonfunctional usb wifi

[Yuri Honegger <yuri.honegger%icloud.com@localhost>]

Hi,

> Am 16.07.2026 um 16:12 schrieb Brook Milligan <brook%biology.nmsu.edu@localhost>:
> 
> Thanks for the input so far.  Here is an (I hope) improved version (see below).  (There are still some aprints that will be removed, but are informative for now.)
> 
> This patch is based upon a new driver for the control module registers, which better follows the binding specs and does not introduce too much extra stuff.  Again, I would greatly appreciate a review.
> 
> There are some questions I would appreciate answers to.
> 
> First, the location of additions to GENERIC and files.ti are pretty arbitrary.  Where would you expect to find these bits?

As far as I know, there are no strict rules. Try to match the surrounding code and what makes sense to you.

> 
> Second, should this be added to any other kernels beyond GENERIC?
> 

This driver is only for TI AM335X devices which run GENERIC, so I don’t think so.

> Thanks for your input and help.
> 
> Cheers,
> Brook
> 
> 
> Index: sys/arch/arm/ti/files.ti
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/arm/ti/files.ti,v
> retrieving revision 1.30
> diff -u -r1.30 files.ti
> --- sys/arch/arm/ti/files.ti 30 Mar 2026 08:17:29 -0000 1.30
> +++ sys/arch/arm/ti/files.ti 15 Jul 2026 16:58:02 -0000
> @@ -145,6 +145,10 @@
> attach motg at fdt with omapmusb
> file arch/arm/ti/ti_omapmusb.c omapmusb
> 
> +device tiusbctrl
> +attach tiusbctrl at fdt with ti_usbctrl
> +file arch/arm/ti/ti_usbctrl.c ti_usbctrl
> +
> # RNG
> device tirng
> attach tirng at fdt with ti_rng
> Index: sys/arch/arm/ti/ti_otgreg.h
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/arm/ti/ti_otgreg.h,v
> retrieving revision 1.1
> diff -u -r1.1 ti_otgreg.h
> --- sys/arch/arm/ti/ti_otgreg.h 27 Oct 2019 16:31:26 -0000 1.1
> +++ sys/arch/arm/ti/ti_otgreg.h 15 Jul 2026 16:58:02 -0000
> @@ -72,3 +72,10 @@
> 
> #define USB_CORE_OFFSET 0x400
> #define USB_CORE_SIZE 0x400
> +
> +/* USB control module register */
> +#define CM_USBCTRL(n) (2 * sizeof(uint32_t))
> +#define CM_USBCTRL_CM_PWRDN __BIT(0)
> +#define CM_USBCTRL_OTG_PWRDN __BIT(1)
> +#define CM_USBCTRL_OTGVDET_EN __BIT(19)
> +#define CM_USBCTRL_OTGSESSENDEN __BIT(20)
> Index: sys/arch/evbarm/conf/GENERIC
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/evbarm/conf/GENERIC,v
> retrieving revision 1.131
> diff -u -r1.131 GENERIC
> --- sys/arch/evbarm/conf/GENERIC 26 Apr 2026 12:57:03 -0000 1.131
> +++ sys/arch/evbarm/conf/GENERIC 15 Jul 2026 16:58:04 -0000
> @@ -609,6 +609,8 @@
> usb* at usbus?
> usbnode* at fdt?
> 
> +tiusbctrl* at fdt?
> +

Since you asked about placement, I’d say it makes more sense to place it next to the usb phy drivers.

> # Virtio devices
> virtio* at fdt? # Virtio MMIO device
> virtio* at pci? dev ? function ? # Virtio PCI device
> 
> 
> 
> --- /dev/null 2026-07-15 10:51:55.222314187 -0600
> +++ sys/arch/arm/ti/ti_usbctrl.c 2026-07-15 10:51:11.382655707 -0600
> @@ -0,0 +1,113 @@
> +/* $NetBSD$ */
> +/*
> + * Copyright (c) 2026 Brook Milligan.  All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <sys/cdefs.h>
> +__KERNEL_RCSID(0, "$NetBSD$");
> +
> +#include <sys/device.h>
> +#include <arm/ti/ti_otgreg.h>
> +
> +#include <dev/usb/usbhist.h>
> +#include <dev/fdt/fdtvar.h>
> +
> +#ifdef USB_DEBUG
> +#ifndef USBCTRL_DEBUG
> +#define usbctrldebug 0
> +#else
> +extern int usbctrldebug;
> +#endif /* USBCTRL_DEBUG */
> +#endif /* USB_DEBUG */
> +
> +#define DPRINTF(FMT,A,B,C,D) USBHIST_LOGN(usbctrldebug,1,FMT,A,B,C,D)
> +#define USBCTRLHIST_FUNC() USBHIST_FUNC()
> +#define USBCTRLHIST_CALLED(name) USBHIST_CALLED(usbctrldebug)

Looking at other drivers for the skeleton is fine, but you can throw out the stuff you don’t use.

> +
> +static const struct device_compatible_entry compat_data[] = {
> + { .compat = "ti,am335x-usb-ctrl-module" },
> + DEVICE_COMPAT_EOL
> +};
> +
> +/*
> + * usbctrl device attachment and driver,
> + * for the USB control module registers, which
> + * must be initialized prior to the ti_motg
> + * driver attachment
> + */
> +
> +struct ti_usbctrl_softc {
> + bus_space_tag_t sc_ctrliot;
> + bus_space_handle_t sc_ctrlioh;
> +};
> +
> +static int ti_usbctrl_match(device_t, cfdata_t, void *);
> +static void ti_usbctrl_attach(device_t, device_t, void *);
> +
> +CFATTACH_DECL_NEW(ti_usbctrl, sizeof(struct ti_usbctrl_softc),
> +    ti_usbctrl_match, ti_usbctrl_attach, NULL, NULL);
> +
> +static int
> +ti_usbctrl_match(device_t parent, cfdata_t match, void *aux)
> +{
> + struct fdt_attach_args * const faa = aux;
> +
> + return of_compatible_match(faa->faa_phandle, compat_data);
> +}
> +
> +static void
> +ti_usbctrl_attach(device_t parent, device_t self, void *aux)
> +{
> + struct ti_usbctrl_softc *sc = device_private(self);
> + struct fdt_attach_args * const faa = aux;
> + const int phandle = faa->faa_phandle;
> + bus_addr_t addr[2];
> + bus_size_t size[2];
> +
> + USBCTRLHIST_FUNC(); USBCTRLHIST_CALLED();
Are you actually using this to debug?
> +
> + aprint_normal("\n===> ti_usbctrl_attach(): %s\n", of_compatible_lookup(phandle, compat_data)->compat);
> +
> + if (fdtbus_get_reg_byname(phandle, "phy_ctrl", &addr[0], &size[0]) != 0 ||
> +    fdtbus_get_reg_byname(phandle, "wakeup", &addr[1], &size[1])) {
> + aprint_error_dev(self, "couldn't get registers\n");
> + return;
> + }

if you are only using phy_ctrl, you don’t have to read the wakeup property.

> + aprint_normal("===> phy_ctrl: 0x%08lx 0x%08lx\n", addr[0], size[0]);
> + aprint_normal("===> wakeup:   0x%08lx 0x%08lx\n", addr[1], size[1]);
> +
> + sc->sc_ctrliot = faa->faa_bst;
> + if (bus_space_map(sc->sc_ctrliot, addr[0], size[0], 0, &sc->sc_ctrlioh) != 0) {
> + aprint_error_dev(self, "couldn't map registers\n");
> + return;
> + }
> +
> + for (int i = 0; i < 2; ++i) {
> + uint32_t usb_ctrl = bus_space_read_4(sc->sc_ctrliot, sc->sc_ctrlioh, CM_USBCTRL(i));
> + aprint_normal("===> reading usb_ctrl%d: 0x%04x\n", i, usb_ctrl);
> + usb_ctrl &= ~(CM_USBCTRL_CM_PWRDN | CM_USBCTRL_OTG_PWRDN);
> + usb_ctrl |= (CM_USBCTRL_OTGVDET_EN | CM_USBCTRL_OTGSESSENDEN);
> + aprint_normal("===> writing usb_ctrl%d: 0x%04x\n", i, usb_ctrl);
> + bus_space_write_4(sc->sc_ctrliot, sc->sc_ctrlioh, CM_USBCTRL(i), usb_ctrl);
> + }
> +}
> 

A general remark: This driver only writes in attach. For that, you don’t need to store the bus_space_ object in your softcore. In fact, you could have an empty softcore and keep them on the stack in attach. You can even bus_space_unmap after writing the registers.

Yuri