From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: rkclock(4), rkusbphy(4): fix USB-related bitmasks for RK3399
To: David Gwynne <david@gwynne.id.au>
Cc: fercerpav@gmail.com, tech@openbsd.org, kettenis@openbsd.org
Date: Sun, 24 Nov 2024 23:48:45 +0100

> From: David Gwynne <david@gwynne.id.au>
> Date: Thu, 21 Nov 2024 21:19:09 +1000
> 
> > On 19 Nov 2024, at 08:45, Paul Fertser <fercerpav@gmail.com> 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.

I dropped the comment about OpenBSD only supporting host mode.  That
is true, but the sentence seemed garbled and we are (mostly) using the
same values as the Linux driver I'm looking at.  Although there is a
funny difference between the two PHYs for the RK3568 that I need to
look into...

Thanks!

> > ---
> > 
> > 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),
> > };
> > 
> 
>