On Thu, Apr 30, 2026 at 03:39:51PM +0800, Ze Huang wrote:
> On Thu Apr 30, 2026 at 10:28 AM CST, Inochi Amaoto wrote:
> > The comb PHY on K3 requires to configure a syscon device for the
> > right mux configuration. And it requires calibration before any
> > usage.
> >
> > Add USB3/PCIe comb PHY driver for Spacemit K3.
> >
> > Signed-off-by: Inochi Amaoto <[email protected]>
> > ---
> > drivers/phy/spacemit/Kconfig | 16 ++
> > drivers/phy/spacemit/Makefile | 2 +
> > drivers/phy/spacemit/phy-k3-combphy.c | 250 ++++++++++++++++
> > drivers/phy/spacemit/phy-k3-common.c | 398 ++++++++++++++++++++++++++
> > drivers/phy/spacemit/phy-k3-common.h | 27 ++
> > 5 files changed, 693 insertions(+)
> > create mode 100644 drivers/phy/spacemit/phy-k3-combphy.c
> > create mode 100644 drivers/phy/spacemit/phy-k3-common.c
> > create mode 100644 drivers/phy/spacemit/phy-k3-common.h
> >
> > diff --git a/drivers/phy/spacemit/Kconfig b/drivers/phy/spacemit/Kconfig
> > index 50b0005acf66..5fdf18fce499 100644
> > --- a/drivers/phy/spacemit/Kconfig
> > +++ b/drivers/phy/spacemit/Kconfig
> > @@ -23,3 +23,19 @@ config PHY_SPACEMIT_K1_USB2
> > help
> > Enable this to support K1 USB 2.0 PHY driver. This driver takes care
> > of
> > enabling and clock setup and will be used by K1 udc/ehci/otg/xhci
> > driver.
> > +
> > +config PHY_SPACEMIT_K3_COMMON_OPS
> > + tristate
> > + select MFD_SYSCON
> > + select GENERIC_PHY
> > +
> > +config PHY_SPACEMIT_K3_COMBO_PHY
> > + tristate "SpacemiT K3 USB3/PCIe PHY support"
> > + depends on (ARCH_SPACEMIT || COMPILE_TEST) && OF
> > + depends on COMMON_CLK
> > + select PHY_SPACEMIT_K3_COMMON_OPS
> > + help
> > + Enable this to support K3 USB3/PCIe combo PHY driver. This
> > + driver takes care of enabling and clock setup and will be used
> > + by K3 dwc3 driver.
> > + If unsure, say N.
> > diff --git a/drivers/phy/spacemit/Makefile b/drivers/phy/spacemit/Makefile
> > index a821a21d6142..41be7b0388da 100644
> > --- a/drivers/phy/spacemit/Makefile
> > +++ b/drivers/phy/spacemit/Makefile
> > @@ -1,3 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > obj-$(CONFIG_PHY_SPACEMIT_K1_PCIE) += phy-k1-pcie.o
> > obj-$(CONFIG_PHY_SPACEMIT_K1_USB2) += phy-k1-usb2.o
> > +obj-$(CONFIG_PHY_SPACEMIT_K3_COMBO_PHY) += phy-k3-combphy.o
> > +obj-$(CONFIG_PHY_SPACEMIT_K3_COMMON_OPS) += phy-k3-common.o
> > diff --git a/drivers/phy/spacemit/phy-k3-combphy.c
> > b/drivers/phy/spacemit/phy-k3-combphy.c
> > new file mode 100644
> > index 000000000000..66fa6330ad6e
> > --- /dev/null
> > +++ b/drivers/phy/spacemit/phy-k3-combphy.c
> > @@ -0,0 +1,250 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * phy-k3-usb3.c - SpacemiT K3 Type-C Orientation Switch Driver
> > + *
> > + * Copyright (c) 2025 SpacemiT Technology Co. Ltd
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/io.h>
>
> ...
>
> > +
> > + phy->apb_spare = syscon_regmap_lookup_by_phandle(node,
> > "spacemit,apb-spare");
> > + if (IS_ERR(phy->apb_spare))
> > + return dev_err_probe(dev, PTR_ERR(phy->apb_spare),
> > + "Failed to fine APB SPARE syscon");
>
> typo, s/fine/find
>
> > +
> > + apmu = syscon_regmap_lookup_by_phandle_args(node, "spacemit,apmu", 1,
> > &config);
> > + if (IS_ERR(apmu))
> > + return dev_err_probe(dev, PTR_ERR(phy->apb_spare),
> > + "Failed to fine APMU syscon");
>
> 1. typo, s/fine/find
> 2. PTR_ERR(phy->apb_spare) should be PTR_ERR(apmu)
>
Thanks for pointing that
> > +
> > + ret = k3_comb_phy_update_config(apmu, config);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Failed to set lane
> > configuration");
> > +
> > + phy->dev = dev;
> > + platform_set_drvdata(pdev, phy);
> > +
> > + ret = k3_phy_calibrate(phy->apb_spare);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Failed to calibrate phy");
> > +
> > + ret = k3_comb_phy_init_lanes(phy, config);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret, "Failed to init lanes");
> > +
> > + provider = devm_of_phy_provider_register(dev, k3_comb_phy_xlate);
> > + if (IS_ERR(provider))
> > + return dev_err_probe(dev, PTR_ERR(provider),
> > + "Failed to register provider\n");
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id k3_comb_phy_of_match[] = {
> > + { .compatible = "spacemit,k3-comb-phy" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, k3_comb_phy_of_match);
> > +
> > +static struct platform_driver k3_comb_phy_driver = {
> > + .probe = k3_comb_phy_probe,
> > + .driver = {
> > + .name = "spacemit,k3-comb-phy",
> > + .of_match_table = k3_comb_phy_of_match,
> > + },
> > +};
> > +module_platform_driver(k3_comb_phy_driver);
> > +
> > +MODULE_DESCRIPTION("SpacemiT K3 USB3/PCIe comb PHY driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/phy/spacemit/phy-k3-common.c
> > b/drivers/phy/spacemit/phy-k3-common.c
> > new file mode 100644
> > index 000000000000..77c4b4073b96
> > --- /dev/null
> > +++ b/drivers/phy/spacemit/phy-k3-common.c
> > @@ -0,0 +1,398 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/usb.h>
> > +
> > +#include <dt-bindings/phy/phy.h>
> > +
> > +#include "phy-k3-common.h"
> > +
> > +/* PHY Registers */
> > +#define PHY_VERSION 0x0
> > +
> > +#define PHY_RESET_CFG 0x04
> > +
> > +#define PHY_RESET_RXBUF_RST BIT(0)
> > +#define PHY_RESET_SOFT_RST_PCS BIT(1)
> > +#define PHY_RESET_SOFT_RST_AHB BIT(2)
> > +#define PHY_RESET_EN_SD_AFTER_LOCK BIT(6)
> > +
> > +#define PHY_CLK_CFG 0x08
> > +
> > +#define PHY_CLK_PLL_READY BIT(0)
> > +#define PHY_CLK_TXCLK_INV BIT(2)
> > +#define PHY_CLK_RXCLK_EN BIT(3)
> > +#define PHY_CLK_TXCLK_EN BIT(4)
> > +#define PHY_CLK_PCLK_EN BIT(5)
> > +#define PHY_CLK_PIPE_PCLK_EN BIT(6)
> > +#define PHY_CLK_REFCLK_FREQ GENMASK(10, 7)
> > +#define PHY_CLK_REFCLK_24M 2
> > +#define PHY_CLK_SW_INIT_DONE BIT(11)
> > +#define PHY_CLK_PU_SSC_OUT BIT(23)
> > +
> > +#define PHY_MODE_CFG 0x0C
> > +
> > +#define PHY_MODE_PCIE_INT_EN BIT(0)
> > +#define PHY_MODE_LFPS_TPERIOD GENMASK(9, 8)
> > +#define PHY_MODE_LFPS_TPERIOD_USB 3
> > +
> > +#define PHY_PU_SEL 0x40
> > +
> > +#define PHY_PU_CFG_STATUS BIT(9)
> > +#define PHY_PU_OVRD_STATUS BIT(10)
> > +
> > +#define PHY_PU_CK_REG 0x54
> > +
> > +#define PHY_PU_REFCLK_100 BIT(25)
> > +
> > +#define PHY_PLL_REG1 0x58
> > +
> > +#define PHY_PLL_FREF_SEL GENMASK(15, 13)
> > +#define PHY_PLL_FREF_24M 0x1
> > +#define PHY_PLL_SSC_DEP_SEL GENMASK(27, 24)
> > +#define PHY_PLL_SSC_5000PPM 0xa
> > +#define PHY_PLL_SSC_MODE GENMASK(29, 28)
> > +#define PHY_PLL_SSC_MODE_CENTER_SPREAD 0
> > +#define PHY_PLL_SSC_MODE_UP_SPREAD 1
> > +#define PHY_PLL_SSC_MODE_DOWN_SPREAD 2
> > +#define PHY_PLL_SSC_MODE_DOWN_SPREAD1 3
> > +
> > +#define PHY_PLL_REG2 0x5c
> > +
> > +#define PHY_PLL_SEL_REF100 BIT(21)
> > +
> > +/* PHY RX Register Definitions */
> > +#define PHY_RX_REG_A 0x60
> > +
> > +#define PHY_RX_REG0_RLOAD BIT(4)
> > +#define PHY_RX_REG1_RTERM GENMASK(11, 8)
> > +#define PHY_RX_REG1_RC_CALI GENMASK(15, 12)
> > +#define PHY_RX_REG2_CSEL GENMASK(19, 16)
> > +#define PHY_RX_REG2_FORCE_CSEL BIT(20)
> > +#define PHY_RX_REG2_PSEL GENMASK(23, 21)
> > +#define PHY_RX_REG3_I_LOAD GENMASK(26, 24)
> > +#define PHY_RX_REG3_SEL_CBOOST_CODE BIT(27)
> > +#define PHY_RX_REG3_ADJ_BIAS GENMASK(29, 28)
> > +#define PHY_RX_REG3_RDEG1 GENMASK(31, 30)
> > +
> > +#define PHY_RX_REG_B 0x64
> > +
> > +#define PHY_RX_REGB_MASK GENMASK(23, 0)
> > +
> > +#define PHY_RX_REG4_RDEG2 GENMASK(2, 1)
> > +#define PHY_RX_REG4_ENVOS BIT(4)
> > +#define PHY_RX_REG4_RTERM_SEL BIT(5)
> > +#define PHY_RX_REG4_MANUAL_CFG BIT(7)
> > +#define PHY_RX_REG5_RCELL_VCM GENMASK(11, 8)
> > +#define PHY_RX_REG5_RCELL_BIAS GENMASK(15, 12)
> > +#define PHY_RX_REG6_H1_REG GENMASK(19, 16)
> > +#define PHY_RX_REG6_ADAPT_GAIN GENMASK(21, 20)
> > +#define PHY_RX_REG6_BYPASS_ADPT BIT(22)
> > +
> > +#define PHY_ADPT_CFG0 0x140
> > +#define PHY_ADPT_AFE_RST_OVRD_EN BIT(1)
> > +#define PHY_ADPT_AFE_RST_OVRD_VAL BIT(4)
> > +
> > +#define PHY_RXEQ_TIME 0xb4
> > +#define PHY_RXEQ_TIME_OVRD_POST_C_SOC BIT(21)
> > +#define PHY_RXEQ_TIME_CFG_AMP_SOC GENMASK(23, 22)
> > +#define PHY_RXEQ_TIME_AMP_SOC_650M 0
> > +#define PHY_RXEQ_TIME_AMP_SOC_800M 1
> > +#define PHY_RXEQ_TIME_AMP_SOC_870M 2
> > +#define PHY_RXEQ_TIME_AMP_SOC_900M 3
> > +#define PHY_RXEQ_TIME_OVRD_AMP_SOC BIT(24)
> > +
> > +#define PCIE_PU_ADDR_CLK_CFG 0x0008
> > +#define PHY_CLK_PLL_READY BIT(0)
> > +#define PCIE_INITAL_TIMER GENMASK(6, 3)
> > +#define CFG_INTERNAL_TIMER_ADJ GENMASK(10, 7)
> > +#define CFG_SW_PHY_INIT_DONE BIT(11)
> > +
> > +/* Lane RX/TX configuration (per‑lane, at lane_base) */
> > +#define PCIE_RX_REG1 0x050
> > +#define PCIE_TX_REG1 0x064
> > +
> > +#define PCIE_PLL_TIMEOUT 500000
> > +#define PCIE_POLL_DELAY 500
> > +
> > +
>
> ...
>
> > +static int k3_pcie_phy_init(struct phy *phy)
> > +{
> > + struct k3_lane_group *lg = phy_get_drvdata(phy);
> > + void __iomem *phy_base = lg->base + lg->data->offsets[0];
> > + u32 val;
> > + int ret;
> > + int i;
> > +
> > + val = readl(phy_base + PHY_PLL_REG1);
> > + val = u32_replace_bits(val, 0x2, GENMASK(15, 12));
> > + writel(val, phy_base + PHY_PLL_REG1);
> > +
> > + val = readl(phy_base + PHY_PLL_REG2);
> > + val = u32_replace_bits(val, 0, BIT(21));
> > + writel(val, phy_base + PHY_PLL_REG2);
> > +
> > + for (i = 0; i < lg->data->lanes; i++) {
> > + void __iomem *lane_base = lg->base + lg->data->offsets[i];
> > +
>
> > + val = readl(lane_base + PCIE_RX_REG1);
> > + val = u32_replace_bits(val, 0, 0x3);
> > + writel(val, phy_base + PCIE_RX_REG1);
>
> This looks like a copy-paste bug.
>
> Read from lane_base but write the modified value to phy_base.
>
Yes, it should be the lane_base.
> > + }
> > +
> > + val = readl(phy_base + PHY_PLL_REG2);
> > + val |= BIT(20);
> > + writel(val, phy_base + PHY_PLL_REG2);
> > +
>
> > + writel(0x00006505, phy_base + PCIE_RX_REG1);
>
> Is it intentional? The loop above configured PCIE_RX_REG1, while the
> hard-coded 0x00006505 overwrites what's done for lane0.
>
Yes, the logic is coming from vendor.
> > +
> > + /* pll_reg1 of lane0, disable SSC: pll_reg4[3:0] = 0 */
> > + val = readl(phy_base + PHY_PLL_REG1);
> > + val = u32_replace_bits(val, 0, GENMASK(27, 24));
> > + writel(val, phy_base + PHY_PLL_REG1);
>
> A little confusing here, comment says "pll_reg4[3:0] = 0" but the code is
> modifying PHY_PLL_REG1[27:24]
>
I think this is is a mistake from the vendor code. IIRC, the
reg4 may could be the [31:24]. As it is divided into four parts.
> > +
> > + for (i = 0; i < lg->data->lanes; i++) {
> > + void __iomem *lane_base = lg->base + lg->data->offsets[i];
> > +
> > + /* set cfg_tx_send_dummy_data to be 1'b1 for disable dash data
> > */
> > + val = readl(lane_base + PHY_PU_SEL);
> > + val = u32_replace_bits(val, 1, BIT(13));
> > + writel(val, lane_base + PHY_PU_SEL);
> > +
> > + /* disable en_sample_data_after_cdr_locked */
> > + val = readl(lane_base + PHY_RESET_CFG);
> > + val = u32_replace_bits(val, 0, BIT(6));
> > + writel(val, lane_base + PHY_RESET_CFG);
> > +
> > + /* Dynamic Lock */
> > + val = readl(lane_base + PHY_MODE_CFG);
> > + val = u32_replace_bits(val, 1, BIT(2));
> > + writel(val, lane_base + PHY_MODE_CFG);
> > +
> > + val = FIELD_PREP(GENMASK(7, 0), 0x10) |
> > + FIELD_PREP(GENMASK(15, 8), 0x78) |
> > + FIELD_PREP(GENMASK(23, 16), 0x98) |
> > + FIELD_PREP(GENMASK(31, 24), 0xdf);
> > + writel(val, lane_base + PHY_RX_REG_A);
> > +
> > + val = readl(lane_base + PHY_RX_REG_B);
> > + val &= ~PHY_RX_REGB_MASK;
> > + val |= FIELD_PREP(GENMASK(7, 0), 0xb4) |
> > + FIELD_PREP(GENMASK(15, 8), 0x88) |
> > + FIELD_PREP(GENMASK(23, 16), 0x28);
> > + writel(val, lane_base + PHY_RX_REG_B);
>
> Can we define macros for these values? Just like you did for
> PHY_CLK_CFG.
>
We can, but I guess this makes no sense. As no detailed document
about these magic numbers, there is no meaningful name for
these numbers and fields.....
Regards,
Inochi
> > +
> > + /* Set init done */
> > + val = readl(lane_base + PCIE_PU_ADDR_CLK_CFG);
> > + val = u32_replace_bits(val, 1, CFG_SW_PHY_INIT_DONE);
> > + writel(val, lane_base + PCIE_PU_ADDR_CLK_CFG);
> > + }
> > +
> > + ret = readl_poll_timeout(phy_base + PCIE_PU_ADDR_CLK_CFG, val,
> > + (val & PHY_CLK_PLL_READY), PCIE_POLL_DELAY,
> > + PCIE_PLL_TIMEOUT);
> > + if (ret) {
> > + dev_err(&lg->phy->dev, "PHY PLL lock timeout\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
