Current-Users archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
PR 60404: Beaglebone Black: nonfunctional usb wifi
> Am 10.07.2026 um 16:23 schrieb Brook Milligan <brook%biology.nmsu.edu@localhost>:
>
>
>> On Jul 10, 2026, at 05:10, Yuri Honegger <yuri.honegger%icloud.com@localhost> wrote:
>>
>> I missed the IRC conversation about figuring out how to get whether we are usb0/1 for the BBB usb controller.
>
> No worries. As you said, IRC has its advantages and disadvantages.
>
>> We cannot go by the attach order of the driver. There is no guarantee that usb0 and usb1 always attach in the same order (in practice, it is deterministic, but the order is undefined and could change).
>
> I understand that probe order is undefined. For the moment, I ended up keying off the faa_name part of the attach arguments. I’m not sure how it gets there, but it relates to the node name in the device tree, so I expect it to be relatively stable.
It is stable until it isn’t. Names are only cosmetic. In the past, linux has changed names of properties to standardize them a bit.
>
> Please take a look at my current solution, verbose as it is for the moment [1]. Also see the follow-ups. Without knowing what I’m talking about or what that entails, I like the idea of adding a phy driver as being the cleanest solution. Done right, though, it would have to include some conditional for whether the address base is in the tree with a fallback if not, at least if we want to interact with Linux dts and until they add the address base. I have no idea how to do any of that.
Adding a phy driver doesn’t make it much more cleaner imo. The phy driver would have to attach on "ti,am335x-usb-phy" so we can use fdt_phy. It also doesn’t help with the issue of determining the usb controller instance. I’m also not sure if the amount of boilerplate a new driver adds compared to how much work it does is worth it. That said, the approach with a phy driver is not wrong. I’m fine with either.
As for how to do it: This commit contains all changes required to add a usb controller including a phy driver: https://github.com/recursivetree/netbsd-src/commit/94182f2117e75b21e36b57f282ea8248e1d6c4b9 Perhaps it helps you to figure out what is needed. Your configuration is GENERIC, not GENERIC_V5.
>
> Any thoughts are welcome. I really appreciate your interest.
>
> Additionally, if you have thoughts on the follow-on problems with wifi (ioctl invalid argument and panic) mentioned in PR 60404 [2], please let me know.
No, I haven’t got around to look at it yet.
>
> Cheers,
> Brook
>
> [1] https://mail-index.netbsd.org/current-users/2026/07/10/msg047718.html
> [2] https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=60404
>
Index: ti_motg.c
===================================================================
RCS file: /cvsroot/src/sys/arch/arm/ti/ti_motg.c,v
retrieving revision 1.5
diff -u -r1.5 ti_motg.c
--- ti_motg.c 2 Feb 2024 22:14:04 -0000 1.5
+++ ti_motg.c 10 Jul 2026 02:36:15 -0000
@@ -165,6 +165,79 @@
aprint_normal_dev(self, "interrupting on %s\n", intrstr);
+/*
+ * XXX - some of these constants should be read from the device tree
+ */
+
+/*
+ * control module
+ *
+ * see Spruh TRM Table 2-2. L4_WKUP Peripheral Memory Map, page 180 and
+ * sys/external/gpl2/dts/dist/arch/arm/boot/dts/am33xx-l4.dtsi, line 281
+ * segment@200000:target-module@10000
+ */
+#define CTRL_MODULE_BASE 0x44e10000
+
+/* see sys/external/gpl2/dts/dist/arch/arm/boot/dts/am33xx-l4.dtsi, line 327: usb_ctrl_mod:control@620 */
+#define USB_CTRL_MOD_PHY_CTRL_BASE 0x620
+#define USB_CTRL_MOD_PHY_CTRL_SIZE 0x10
The #define for a registers should go to ti_otgreg.h
+
+/*
+ * control register bits
+ * see Spruh TRM section 9.3.1.20, pages 1483-1484 and section 9.3.1.22, pages 1486-1487
+ */
+#define USBPHY_CM_PWRDN (1 << 0)
+#define USBPHY_OTG_PWRDN (1 << 1)
+#define USBPHY_OTG_VDET_EN (1 << 19)
+#define USBPHY_OTG_SESSENDEN (1 << 20)
same as above
+
+/*
+ * names based on USB control registers
+ * see Spruh TRM Table 2-1, page 178
+ */
+#define USB0_BASE "usb@47401000"
+#define USB1_BASE "usb@47401800"
+
+ int usb_index = strcmp(faa->faa_name, USB0_BASE) == 0 ? 0
+ : strcmp(faa->faa_name, USB1_BASE) == 0 ? 1
+ : -1;
+ if (usb_index < 0) {
+ return;
+ }
No chance anything like this could get it. The idea behind the device tree is that it is the sole, authoritative source about the memory map so that drivers work even if addresses change, like for example if they release a new variant of the same SoC. If you really want to work with a usb_index, go the linux route with the alias.
+
+ aprint_normal("\n");
+ aprint_normal("===> ti_motg::attach(): %s (usb%d)\n", faa->faa_name, usb_index);
+
+ bus_space_handle_t handle;
+ bus_addr_t ctrl_base_addr = CTRL_MODULE_BASE + USB_CTRL_MOD_PHY_CTRL_BASE; /* control registers base address */
+ bus_size_t ctrl_size = USB_CTRL_MOD_PHY_CTRL_SIZE; /* control registers size */
You should get this from the device tree. A while ago, I provided the skeleton for this:
phy_phandle = fdtbus_get_phandle(phandle, "phys");
cm_phandle = fdtbus_get_phandle(phy_phandle, "ti,ctrl_mod");
fdtbus_get_reg_byname(cm_phandle, "phy_ctrl", &addr, &size);
Have you tried that?
+ bus_size_t ctrl_offset = ctrl_base_addr + usb_index * 2 * sizeof(uint32_t); /* control register offset */
usually you do something like usb_index * USB_CTRL_SIZE; where USB_CTRL_SIZE is a #define. (The name is just an example)
+
+ if (bus_space_map(faa->faa_bst, ctrl_base_addr, ctrl_size, 0, &handle) != 0) {
+ aprint_error(": couldn't map USB control registers\n");
+ return;
+ }
+
+ uint32_t usb_ctrl = bus_space_read_4(faa->faa_bst, handle, ctrl_offset - ctrl_base_addr);
+ aprint_normal("0x%08x: usb_ctrl%d: 0x%08x (upper 8 bits=0x3c)\n", (uint32_t)ctrl_offset, usb_index, usb_ctrl);
+ aprint_normal(" 0: %x (0?)\n", (usb_ctrl & USBPHY_CM_PWRDN) != 0);
+ aprint_normal(" 1: %x (0?)\n", (usb_ctrl & USBPHY_OTG_PWRDN) != 0);
+ aprint_normal(" 19: %x (1?)\n", (usb_ctrl & USBPHY_OTG_VDET_EN) != 0);
+ aprint_normal(" 20: %x (1?)\n", (usb_ctrl & USBPHY_OTG_SESSENDEN) != 0);
+
+ aprint_normal("===> ti_motg::attach(): writing usb_ctrl%d\n", usb_index);
+ usb_ctrl &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN);
+ usb_ctrl |= (USBPHY_OTG_VDET_EN | USBPHY_OTG_SESSENDEN);
+ bus_space_write_4(faa->faa_bst, handle, ctrl_offset - ctrl_base_addr, usb_ctrl);
+
+ usb_ctrl = bus_space_read_4(faa->faa_bst, handle, ctrl_offset - ctrl_base_addr);
+ aprint_normal("0x%08x: usb_ctrl%d: 0x%08x (upper 8 bits=0x3c)\n", (uint32_t)ctrl_offset, usb_index, usb_ctrl);
+ aprint_normal(" 0: %x (0?)\n", (usb_ctrl & USBPHY_CM_PWRDN) != 0);
+ aprint_normal(" 1: %x (0?)\n", (usb_ctrl & USBPHY_OTG_PWRDN) != 0);
+ aprint_normal(" 19: %x (1?)\n", (usb_ctrl & USBPHY_OTG_VDET_EN) != 0);
+ aprint_normal(" 20: %x (1?)\n", (usb_ctrl & USBPHY_OTG_SESSENDEN) != 0);
+ aprint_normal("\n");
+
sc->sc_motg.sc_iot = faa->faa_bst;
if (bus_space_map(sc->sc_motg.sc_iot, addr[0], size[0], 0,
&sc->sc_motg.sc_ioh) != 0) {
Home |
Main Index |
Thread Index |
Old Index