Date: Fri, 7 Apr 2023 03:59:40 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: Ganbold Tsagaankhuu <ganbold@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 4720afaffe7e - main - Improve RK3568 pcie phy handling codes a bit. Message-ID: <F22E57B0-AB76-4532-BC56-21E3CA2E17DD@freebsd.org> In-Reply-To: <202304070255.3372tIg6078531@gitrepo.freebsd.org> References: <202304070255.3372tIg6078531@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7 Apr 2023, at 03:55, Ganbold Tsagaankhuu <ganbold@FreeBSD.org> = wrote: >=20 > The branch main has been updated by ganbold: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D4720afaffe7e17e44ee0f8f3ab66da2f= d2b0d5da >=20 > commit 4720afaffe7e17e44ee0f8f3ab66da2fd2b0d5da > Author: Ganbold Tsagaankhuu <ganbold@FreeBSD.org> > AuthorDate: 2023-04-07 02:54:13 +0000 > Commit: Ganbold Tsagaankhuu <ganbold@FreeBSD.org> > CommitDate: 2023-04-07 02:54:13 +0000 >=20 > Improve RK3568 pcie phy handling codes a bit. >=20 > Move phy bifurcation code to a separate function > that can be called during the attach phase. > Also initialize both pcie lanes accordingly. No attempt at code review, just like all your recent commits? > --- > sys/arm64/rockchip/rk3568_pciephy.c | 87 = +++++++++++++++++++++++-------------- > 1 file changed, 55 insertions(+), 32 deletions(-) >=20 > diff --git a/sys/arm64/rockchip/rk3568_pciephy.c = b/sys/arm64/rockchip/rk3568_pciephy.c > index 5320f30e31bd..d56675521841 100644 > --- a/sys/arm64/rockchip/rk3568_pciephy.c > +++ b/sys/arm64/rockchip/rk3568_pciephy.c > @@ -56,9 +56,12 @@ > #define GRF_PCIE30PHY_CON4 0x10 > #define GRF_PCIE30PHY_CON5 0x14 > #define GRF_PCIE30PHY_CON6 0x18 > +#define GRF_BIFURCATION_LANE_1 0 > +#define GRF_BIFURCATION_LANE_2 1 > #define GRF_PCIE30PHY_WR_EN (0xf << 16) > #define GRF_PCIE30PHY_CON9 0x24 > -#define GRF_PCIE30PHY_DA_OCM ((1 << 15) | (1 << (15 + = 16))) > +#define GRF_PCIE30PHY_DA_OCM_MASK (1 << (15 + 16)) > +#define GRF_PCIE30PHY_DA_OCM ((1 << 15) | = GRF_PCIE30PHY_DA_OCM_MASK) > #define GRF_PCIE30PHY_STATUS0 0x80 > #define SRAM_INIT_DONE (1 << 14) >=20 > @@ -80,46 +83,42 @@ struct rk3568_pciephy_softc { > }; >=20 >=20 > +static void > +rk3568_pciephy_bifurcate(device_t dev, int control, uint32_t lane) > +{ > + struct rk3568_pciephy_softc *sc =3D device_get_softc(dev); > + > + switch (lane) { > + case 0: > + SYSCON_WRITE_4(sc->phy_grf, control, = GRF_PCIE30PHY_WR_EN); > + return; > + case 1: > + SYSCON_WRITE_4(sc->phy_grf, control, > + GRF_PCIE30PHY_WR_EN | GRF_BIFURCATION_LANE_1); > + break; > + case 2: > + SYSCON_WRITE_4(sc->phy_grf, control, > + GRF_PCIE30PHY_WR_EN | GRF_BIFURCATION_LANE_2); > + break; > + default: > + device_printf(dev, "Illegal lane %d\n", lane); > + return; This doesn=E2=80=99t report failure?.. > + } > + if (bootverbose) > + device_printf(dev, "lane %d @ pcie3x%d\n", lane, > + (control =3D=3D GRF_PCIE30PHY_CON5) ? 1 : 2); > +} > + > /* PHY class and methods */ > static int > rk3568_pciephy_enable(struct phynode *phynode, bool enable) > { > device_t dev =3D phynode_get_device(phynode); > struct rk3568_pciephy_softc *sc =3D device_get_softc(dev); > - uint32_t data_lanes[2] =3D { 0, 0 }; > int count; >=20 > if (enable) { > - /* Deassert PCIe PMA output clamp mode */ > - SYSCON_WRITE_4(sc->phy_grf, GRF_PCIE30PHY_CON9, > - GRF_PCIE30PHY_DA_OCM); > - > - /* Set bifurcation according to DT entry */ > - if (OF_hasprop(sc->node, "data-lanes")) { > - OF_getencprop(sc->node, "data-lanes", = data_lanes, > - sizeof(data_lanes)); > - if (data_lanes[0] > 0) { > - SYSCON_WRITE_4(sc->phy_grf, = GRF_PCIE30PHY_CON5, > - GRF_PCIE30PHY_WR_EN | (data_lanes[0] = - 1)); > - device_printf(dev, "pcie3x1 1 lane\n"); > - } > - if (data_lanes[1] > 0) { > - SYSCON_WRITE_4(sc->phy_grf, = GRF_PCIE30PHY_CON6, > - GRF_PCIE30PHY_WR_EN | (data_lanes[1] = - 1)); > - device_printf(dev, "pcie3x2 1 lane\n"); > - } > - if (data_lanes[0] > 1 || data_lanes[1] > 1) > - SYSCON_WRITE_4(sc->phy_grf, = GRF_PCIE30PHY_CON1, > - GRF_PCIE30PHY_DA_OCM); > - > - } else { > - SYSCON_WRITE_4(sc->phy_grf, GRF_PCIE30PHY_CON5, > - GRF_PCIE30PHY_WR_EN); > - SYSCON_WRITE_4(sc->phy_grf, GRF_PCIE30PHY_CON6, > - GRF_PCIE30PHY_WR_EN); > - device_printf(dev, "pcie3 2 lanes\n"); > - } > - > + /* Pull PHY out of reset */ > hwreset_deassert(sc->phy_reset); >=20 > /* Poll for SRAM loaded and ready */ > @@ -165,6 +164,7 @@ rk3568_pciephy_attach(device_t dev) > struct rk3568_pciephy_softc *sc =3D device_get_softc(dev); > struct phynode_init_def phy_init; > struct phynode *phynode; > + uint32_t data_lanes[2] =3D { 0, 0 }; > int rid =3D 0; >=20 > sc->dev =3D dev; > @@ -212,6 +212,29 @@ rk3568_pciephy_attach(device_t dev) >=20 > /* Set RC/EP mode not implemented yet (RC mode only) */ >=20 > + /* Set bifurcation according to "data-lanes" entry */ > + if (OF_hasprop(sc->node, "data-lanes")) { > + OF_getencprop(sc->node, "data-lanes", data_lanes, > + sizeof(data_lanes)); > + } > + else This does not conform to style(9). > + if (bootverbose) > + device_printf(dev, "lane 1 & 2 @pcie3x2\n"); These seems like messy printfs. Jess > + > + /* Deassert PCIe PMA output clamp mode */ > + SYSCON_WRITE_4(sc->phy_grf, GRF_PCIE30PHY_CON9, = GRF_PCIE30PHY_DA_OCM); > + > + /* Configure PHY HW accordingly */ > + rk3568_pciephy_bifurcate(dev, GRF_PCIE30PHY_CON5, = data_lanes[0]); > + rk3568_pciephy_bifurcate(dev, GRF_PCIE30PHY_CON6, = data_lanes[1]); > + > + if (data_lanes[0] || data_lanes[1]) > + SYSCON_WRITE_4(sc->phy_grf, GRF_PCIE30PHY_CON1, > + GRF_PCIE30PHY_DA_OCM); > + else > + SYSCON_WRITE_4(sc->phy_grf, GRF_PCIE30PHY_CON1, > + GRF_PCIE30PHY_DA_OCM_MASK); > + > bzero(&phy_init, sizeof(phy_init)); > phy_init.id =3D PHY_NONE; > phy_init.ofw_node =3D sc->node;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F22E57B0-AB76-4532-BC56-21E3CA2E17DD>