| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aIcFmhQqLuUlLZb_@pie>
Date: Mon, 28 Jul 2025 05:07:38 +0000
From: Yao Zi <ziyao@...root.org>
To: Jonas Karlman <jonas@...boo.se>
Cc: Kishon Vijay Abraham I <kishon@...nel.org>,
Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Heiko Stuebner <heiko@...ech.de>,
Detlev Casanova <detlev.casanova@...labora.com>,
Frank Wang <frank.wang@...k-chips.com>,
linux-rockchip@...ts.infradead.org,
Neil Armstrong <neil.armstrong@...aro.org>,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
Shresth Prasad <shresthprasad7@...il.com>,
Vinod Koul <vkoul@...nel.org>, Chukun Pan <amadeus@....edu.cn>,
linux-arm-kernel@...ts.infradead.org,
Andy Yan <andy.yan@...k-chips.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
devicetree@...r.kernel.org
Subject: Re: [PATCH v4 5/6] phy: rockchip: naneng-combphy: Add RK3528 support
On Thu, Jul 24, 2025 at 05:23:55AM +0000, Yao Zi wrote:
> On Wed, Jul 23, 2025 at 04:51:15PM +0200, Jonas Karlman wrote:
> > Hi Yao Zi,
> >
> > On 6/24/2025 5:37 AM, Yao Zi wrote:
> > > Rockchip RK3528 integrates one naneng-combphy that is able to operate in
> > > PCIe and USB3 mode. The control logic is similar to previous variants of
> > > naneng-combphy but the register layout is apperantly different from the
> > > RK3568 one.
> > >
> > > Signed-off-by: Yao Zi <ziyao@...root.org>
> > > Reviewed-by: Heiko Stuebner <heiko@...ech.de>
> > > Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
> > > ---
> > > .../rockchip/phy-rockchip-naneng-combphy.c | 186 +++++++++++++++++-
> > > 1 file changed, 185 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > index 1d1c7723584b..bf00a85a113b 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
...
> > > @@ -398,6 +437,147 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
> > > return PTR_ERR_OR_ZERO(phy_provider);
> > > }
> > >
> > > +static int rk3528_combphy_cfg(struct rockchip_combphy_priv *priv)
> > > +{
> > > + const struct rockchip_combphy_grfcfg *cfg = priv->cfg->grfcfg;
> > > + unsigned long rate;
> > > + u32 val;
> > > +
> > > + /* Set SSC downward spread spectrum */
> > > + val = FIELD_PREP(RK3528_PHYREG6_SSC_DIR, RK3528_PHYREG6_SSC_DOWNWARD);
> > > + rockchip_combphy_updatel(priv, RK3528_PHYREG6_SSC_DIR, val, RK3528_PHYREG6);
> > > +
> > > + switch (priv->type) {
> > > + case PHY_TYPE_PCIE:
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->con0_for_pcie, true);
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> > > + break;
> > > + case PHY_TYPE_USB3:
> > > + /* Enable adaptive CTLE for USB3.0 Rx */
> > > + rockchip_combphy_updatel(priv, RK3528_PHYREG80_CTLE_EN, RK3528_PHYREG80_CTLE_EN,
> > > + RK3528_PHYREG80);
> > > +
> > > + /* Set slow slew rate control for PI */
> > > + val = FIELD_PREP(RK3528_PHYREG81_SLEW_RATE_CTRL,
> > > + RK3528_PHYREG81_SLEW_RATE_CTRL_SLOW);
> > > + rockchip_combphy_updatel(priv, RK3528_PHYREG81_SLEW_RATE_CTRL, val,
> > > + RK3528_PHYREG81);
> > > +
> > > + /* Set CDR phase path with 2x gain */
> > > + rockchip_combphy_updatel(priv, RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X,
> > > + RK3528_PHYREG81_CDR_PHASE_PATH_GAIN_2X, RK3528_PHYREG81);
> > > +
> > > + /* Set Rx squelch input filler bandwidth */
> > > + val = FIELD_PREP(RK3528_PHYREG83_RX_SQUELCH, RK3528_PHYREG83_RX_SQUELCH_VALUE);
> > > + rockchip_combphy_updatel(priv, RK3528_PHYREG83_RX_SQUELCH, val, RK3528_PHYREG83);
> > > +
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txcomp_sel, false);
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->pipe_txelec_sel, false);
> > > + rockchip_combphy_param_write(priv->phy_grf, &cfg->usb_mode_set, true);
> >
> > I suggest we add something like following here:
> >
> > rockchip_combphy_param_write(priv->pipe_grf, &cfg->u3otg0_port_en, true);
> >
> > to ensure that U3 is enabled in case boot firmware disable the U3 port.
>
> Thanks for the hint, I'm willing to adapt it. Should we handle the case
> that USB is enabled by firmware but PCIe is going to be used in kernel,
> too? It's desirable to make fewer assumptions about the state set by
> firmware.
>
> P.S., I'm assuming the register should be written as "disabled" value
> whenever PCIe is used, and "enabled" whenever USB is used, as
> the LSB of USB_GRF_USB3OTG0_CON1 is said to be "USB 3.0 SS Port Disable
> control" according to RK3588's TRM, which doesn't look like something
> compatible with PCIe mode when setting to 1'b0 (port enabled). Please
> correct me if I'm wrong.
I've read through the manual and done some tests today, and it seems I
misunderstood the purpose of USB3OTG0_CON1. This register has only
something to do with USB3, but not PCIe. Writing either "disabled" or
"enabled" value to it doesn't affect PCIe functionality. Thus for the
naneng-combphy driver, it should be enough to only write the "enabled"
value to u3otg0_port_en if USB-3 mode is used.
Anyway, thanks for your remind on this register :) Its reset value
allows USB-3 to function thus I just forgot about it during clean-up.
I'll send v5 soon.
...
> > > +static const struct rockchip_combphy_grfcfg rk3528_combphy_grfcfgs = {
> > > + /* pipe-phy-grf */
> > > + .pcie_mode_set = { 0x0000, 5, 0, 0x00, 0x11 },
> > > + .usb_mode_set = { 0x0000, 5, 0, 0x00, 0x04 },
> > > + .pipe_rxterm_set = { 0x0000, 12, 12, 0x00, 0x01 },
> > > + .pipe_txelec_set = { 0x0004, 1, 1, 0x00, 0x01 },
> > > + .pipe_txcomp_set = { 0x0004, 4, 4, 0x00, 0x01 },
> > > + .pipe_clk_24m = { 0x0004, 14, 13, 0x00, 0x00 },
> > > + .pipe_clk_100m = { 0x0004, 14, 13, 0x00, 0x02 },
> > > + .pipe_rxterm_sel = { 0x0008, 8, 8, 0x00, 0x01 },
> > > + .pipe_txelec_sel = { 0x0008, 12, 12, 0x00, 0x01 },
> > > + .pipe_txcomp_sel = { 0x0008, 15, 15, 0x00, 0x01 },
> > > + .pipe_clk_ext = { 0x000c, 9, 8, 0x02, 0x01 },
> > > + .pipe_phy_status = { 0x0034, 6, 6, 0x01, 0x00 },
> > > + .con0_for_pcie = { 0x0000, 15, 0, 0x00, 0x110 },
> > > + .con1_for_pcie = { 0x0004, 15, 0, 0x00, 0x00 },
> > > + .con2_for_pcie = { 0x0008, 15, 0, 0x00, 0x101 },
> > > + .con3_for_pcie = { 0x000c, 15, 0, 0x00, 0x0200 },
> >
> > And adding something like this:
> >
> > /* pipe-grf */
> > .u3otg0_port_en = { 0x0044, 15, 0, 0x0181, 0x1100 },
> >
> > Should be possible with ("phy: rockchip: naneng-combphy: Enable U3 OTG
> > port for RK3568") [1].
> >
> > Most RK3528 boards I have come across this far seem to use PCIe instead
> > of USB3, so having boot firmware disable U3 early (to help support USB
> > gadget in boot firmware) and instead having this PHY driver re-enable U3
> > when needed seem most logical to me.
Thank you for the U-Boot patch! While reading through, I saw commit
"rockchip: rk3528: Disable USB3OTG U3 port early" states,
> Some board designs may not use the COMBPHY for USB3 purpose. For these
> board to use USB OTG the input clock source must change to use UTMI
> clk instead of PIPE clk.
Does this mean we should ideally add similar handling for USB3OTG in the
kernel's usb2phy driver, too? Otherwise if the firmware doesn't handle
clock stuff well, the kernel'll fail to operate in OTG mode, either.
> >
> > I will push an updated U-Boot rk3528 branch [2] where I include such
> > early U3 port disable once source.denx.de is back online again.
> >
> > [1] https://lore.kernel.org/r/20250723072324.2246498-1-jonas@kwiboo.se
> > [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528
> >
> > Regards,
> > Jonas
Thanks,
Yao Zi
Powered by blists - more mailing lists