Re: PR 60404: Beaglebone Black: nonfunctional usb wifi
Yuri Honegger <[email protected]> Thu, 16 Jul 2026 21:50:16 +0200
| Newsgroups | gmane.os.netbsd.ports.arm |
|---|---|
| Message-ID | <[email protected]> |
Hi, > Am 16.07.2026 um 16:12 schrieb Brook Milligan = <[email protected]>: >=20 > 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.) >=20 > 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. >=20 > There are some questions I would appreciate answers to. >=20 > 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. >=20 > Second, should this be added to any other kernels beyond GENERIC? >=20 This driver is only for TI AM335X devices which run GENERIC, so I = don=E2=80=99t think so. > Thanks for your input and help. >=20 > Cheers, > Brook >=20 >=20 > Index: sys/arch/arm/ti/files.ti > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 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 >=20 > +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 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 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 @@ >=20 > #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 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 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? >=20 > +tiusbctrl* at fdt? > + Since you asked about placement, I=E2=80=99d 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 >=20 >=20 >=20 > --- /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=E2=80=99t use. > + > +static const struct device_compatible_entry compat_data[] =3D { > + { .compat =3D "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 =3D 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 =3D device_private(self); > + struct fdt_attach_args * const faa =3D aux; > + const int phandle =3D 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=3D=3D=3D> ti_usbctrl_attach(): %s\n", = of_compatible_lookup(phandle, compat_data)->compat); > + > + if (fdtbus_get_reg_byname(phandle, "phy_ctrl", &addr[0], &size[0]) = !=3D 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=E2=80=99t have to read the = wakeup property. > + aprint_normal("=3D=3D=3D> phy_ctrl: 0x%08lx 0x%08lx\n", addr[0], = size[0]); > + aprint_normal("=3D=3D=3D> wakeup: 0x%08lx 0x%08lx\n", addr[1], = size[1]); > + > + sc->sc_ctrliot =3D faa->faa_bst; > + if (bus_space_map(sc->sc_ctrliot, addr[0], size[0], 0, = &sc->sc_ctrlioh) !=3D 0) { > + aprint_error_dev(self, "couldn't map registers\n"); > + return; > + } > + > + for (int i =3D 0; i < 2; ++i) { > + uint32_t usb_ctrl =3D bus_space_read_4(sc->sc_ctrliot, = sc->sc_ctrlioh, CM_USBCTRL(i)); > + aprint_normal("=3D=3D=3D> reading usb_ctrl%d: 0x%04x\n", i, = usb_ctrl); > + usb_ctrl &=3D ~(CM_USBCTRL_CM_PWRDN | CM_USBCTRL_OTG_PWRDN); > + usb_ctrl |=3D (CM_USBCTRL_OTGVDET_EN | CM_USBCTRL_OTGSESSENDEN); > + aprint_normal("=3D=3D=3D> 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); > + } > +} >=20 A general remark: This driver only writes in attach. For that, you = don=E2=80=99t 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=