From: David Gwynne Subject: Re: rkclock(4), rkusbphy(4): fix USB-related bitmasks for RK3399 To: Paul Fertser Cc: OpenBSD Tech , Mark Kettenis Date: Thu, 21 Nov 2024 21:19:09 +1000 > On 19 Nov 2024, at 08:45, Paul Fertser wrote: > > On Mon, Nov 18, 2024 at 02:56:42AM +0300, Paul Fertser wrote: >> However with changes to rkusbphy(4) added back it manages to attach >> and continues to boot so they must be doing something important but >> not quite right, full speed device attached to xHCI hubs >> (TYPEC{0,1}_D* signals) works, for EHCI and OHCI (HOST{0,1}_D*) >> there's zero reaction. This can be fixed later. > > Later happened to be today, and with the attached diff I confirm all > ports work in all modes both after booting from a modern U-Boot with > "usb start" and after booting from the stock OpenBSD U-Boot which > doesn't touch USB at all. this looks right to me. > > --- > > diff --git sys/dev/fdt/rkclock.c sys/dev/fdt/rkclock.c > index 8721604f7746..82e6d42d2d6d 100644 > --- sys/dev/fdt/rkclock.c > +++ sys/dev/fdt/rkclock.c > @@ -2998,22 +2998,22 @@ rk3399_enable(void *cookie, uint32_t *cells, int on) > > switch (idx) { > case RK3399_CLK_USB2PHY0_REF: > - HWRITE4(sc, RK3399_CRU_CLKGATE_CON(6), (5 << 0) << 16); > + HWRITE4(sc, RK3399_CRU_CLKGATE_CON(6), (1 << 5) << 16); > break; > case RK3399_CLK_USB2PHY1_REF: > - HWRITE4(sc, RK3399_CRU_CLKGATE_CON(6), (6 << 0) << 16); > + HWRITE4(sc, RK3399_CRU_CLKGATE_CON(6), (1 << 6) << 16); > break; > case RK3399_CLK_UPHY0_TCPDPHY_REF: > - HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (4 << 0) << 16); > + HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (1 << 4) << 16); > break; > case RK3399_CLK_UPHY0_TCPDCORE: > - HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (5 << 0) << 16); > + HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (1 << 5) << 16); > break; > case RK3399_CLK_UPHY1_TCPDPHY_REF: > - HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (6 << 0) << 16); > + HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (1 << 6) << 16); > break; > case RK3399_CLK_UPHY1_TCPDCORE: > - HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (7 << 0) << 16); > + HWRITE4(sc, RK3399_CRU_CLKGATE_CON(13), (1 << 7) << 16); > break; > case RK3399_ACLK_GMAC: > HWRITE4(sc, RK3399_CRU_CLKGATE_CON(32), (1 << 0) << 16); > diff --git sys/dev/fdt/rkusbphy.c sys/dev/fdt/rkusbphy.c > index 3807be0d6748..b7614a37173e 100644 > --- sys/dev/fdt/rkusbphy.c > +++ sys/dev/fdt/rkusbphy.c > @@ -61,6 +61,54 @@ struct rkusbphy_chip { > const struct rkusbphy_regs *c_regs; > }; > > +/* > + * Since OpenBSD only supports USB ports in host mode, this driver > + * actually powers down the OTG part of the PHY when it is enabled. > + * This is why the OTG values in the tables below differ from this in > + * (for example) Linux. > + */ > + > +/* > + * RK3399 has two USB2PHY nodes that share a GRF. > + */ > + > +static const struct rkusbphy_regs rkusbphy_rk3399_usb0_regs = { > + /* shift, mask, set */ > + .clk_enable = { 0xe450, 4, 0x1, 0x0 }, > + > + .otg = { > + .phy_enable = { 0xe454, 0, 0x3, 0x2 }, > + }, > + > + .host = { > + .phy_enable = { 0xe458, 0, 0x3, 0x2 }, > + }, > +}; > + > +static const struct rkusbphy_regs rkusbphy_rk3399_usb1_regs = { > + /* shift, mask, set */ > + .clk_enable = { 0xe460, 4, 0x1, 0x0 }, > + > + .otg = { > + .phy_enable = { 0xe464, 0, 0x3, 0x2 }, > + }, > + > + .host = { > + .phy_enable = { 0xe468, 0, 0x3, 0x2 }, > + }, > + }; > + > +static const struct rkusbphy_chip rkusbphy_rk3399[] = { > + { > + .c_base_addr = 0xe450, > + .c_regs = &rkusbphy_rk3399_usb0_regs, > + }, > + { > + .c_base_addr = 0xe460, > + .c_regs = &rkusbphy_rk3399_usb1_regs, > + }, > +}; > + > /* > * RK3568 has two USB2PHY nodes that have a GRF each. Each GRF has > * the same register layout. > @@ -190,6 +238,7 @@ struct rkusbphy_id { > #define RKUSBPHY_ID(_n, _c) { _n, _c, nitems(_c) } > > static const struct rkusbphy_id rkusbphy_ids[] = { > + RKUSBPHY_ID("rockchip,rk3399-usb2phy", rkusbphy_rk3399), > RKUSBPHY_ID("rockchip,rk3568-usb2phy", rkusbphy_rk3568), > RKUSBPHY_ID("rockchip,rk3588-usb2phy", rkusbphy_rk3588), > }; >