Re: [PATCH v2 1/2] phy: spacemit: add K1 USB 2.0 PHY driver

[Yixun Lan <dlan@kernel.org>]

Hi Dave,

On 12:32 Tue 06 Oct     , Dave Klotz wrote:
> The UTMI PHY at c0a30000 ("spacemit,k1-usb2-phy") feeds the K1's DWC3
> USB 3 controller. Port the Linux driver: enable the PHY clock, program
> the PLL divider for the 24 MHz reference, wait for the PLL, then release
> the PHY's resets and enable its clocks, the HS TX clock and the
> automatic host-disconnect clear.
> 
..
> The divider has to be written before the PLL is polled. On the OrangePi
Right, the upstream kernel driver also did this way

> R2S under FreeBSD, a driver that polled first and never wrote it left
> the PLL stopped: the clock enables did not latch (0x60ef read back as
Sorry, I've not fully understood here, "polled first and never write"..
Could you elaborate more on this?

> 0x60e9), the PHY gave the controller no UTMI clock and the xHCI reset
> (USBCMD.HCRST) never completed. Warn if the enables do not latch.
Will comment at code below

> 
> Also apply the vendor's HS DAC current setting (+15 %), which the
> FreeBSD driver that runs the R2S port does.
> 
Any actual problem without above settings? I'm just curious,
as upstream code miss this part

> Signed-off-by: Dave Klotz <DaveWK225@protonmail.com>
> ---
>  MAINTAINERS                        |   1 +
>  drivers/phy/spacemit/Kconfig       |   9 ++
>  drivers/phy/spacemit/Makefile      |   1 +
>  drivers/phy/spacemit/phy-k1-usb2.c | 173 +++++++++++++++++++++++++++++
>  4 files changed, 184 insertions(+)
>  create mode 100644 drivers/phy/spacemit/phy-k1-usb2.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e09f9506c..0af42acfbc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1517,6 +1517,7 @@ M:	Yixun Lan <dlan@kernel.org>
>  S:	Maintained
>  T:	git https://git.u-boot-project.org/u-boot/custodians/u-boot-spacemit.git
>  F:	arch/riscv/dts/k1-*-u-boot.dtsi
> +F:	drivers/phy/spacemit/phy-k1-usb2.c
>  N:	spacemit
>  K:	spacemit
>  
> diff --git a/drivers/phy/spacemit/Kconfig b/drivers/phy/spacemit/Kconfig
> index 3c47601c1f..7146965d92 100644
> --- a/drivers/phy/spacemit/Kconfig
> +++ b/drivers/phy/spacemit/Kconfig
> @@ -9,3 +9,12 @@ config PHY_SPACEMIT_K1_PCIE
>  	  ports B and C.  The combo PHY must be enabled even if port A
>  	  is not used for PCIe, because it provides calibration values
>  	  required by the other ports.
> +
> +config PHY_SPACEMIT_K1_USB2
> +	bool "SpacemiT K1 USB 2.0 PHY driver"
> +	depends on PHY && CLK
> +	help
> +	  Enable this to support the USB 2.0 (UTMI) PHY of the SpacemiT K1
> +	  SoC, used by its DWC3 USB 3 controller. The driver configures the
> +	  PLL for the 24 MHz reference clock, releases the internal resets
> +	  and enables the PHY clocks before the controller is started.
> diff --git a/drivers/phy/spacemit/Makefile b/drivers/phy/spacemit/Makefile
> index 3496c099cd..cad47d11bf 100644
> --- a/drivers/phy/spacemit/Makefile
> +++ b/drivers/phy/spacemit/Makefile
> @@ -1 +1,2 @@
>  obj-$(CONFIG_PHY_SPACEMIT_K1_PCIE) += phy-k1-pcie.o
> +obj-$(CONFIG_PHY_SPACEMIT_K1_USB2) += phy-k1-usb2.o
> diff --git a/drivers/phy/spacemit/phy-k1-usb2.c b/drivers/phy/spacemit/phy-k1-usb2.c
> new file mode 100644
> index 0000000000..c4b438219b
> --- /dev/null
> +++ b/drivers/phy/spacemit/phy-k1-usb2.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * SpacemiT K1 USB 2.0 (UTMI) PHY driver
> + *
> + * Ported from the Linux SpacemiT K1 USB 2.0 PHY driver:
> + * Copyright (C) 2025 SpacemiT (Hangzhou) Technology Co. Ltd
> + * Copyright (C) 2025 Ze Huang <huang.ze@linux.dev>
> + *
> + * The HS DAC current setting and the clock-enable check follow the FreeBSD
> + * spacemit_usb2phy driver (BSD-2-Clause), which brought USB up on the
> + * OrangePi R2S:
> + * Copyright (c) 2026 Daniel Shue <dgshue@gmail.com>
> + */
> +
> +#include <clk.h>
> +#include <dm.h>
> +#include <generic-phy.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +
> +#define PHY_RST_MODE_CTRL		0x04
> +#define  PHY_PLL_RDY			BIT(0)
> +#define  PHY_CLK_CDR_EN			BIT(1)
> +#define  PHY_CLK_PLL_EN			BIT(2)
> +#define  PHY_CLK_MAC_EN			BIT(3)
> +#define  PHY_MAC_RSTN			BIT(5)
> +#define  PHY_CDR_RSTN			BIT(6)
> +#define  PHY_PLL_RSTN			BIT(7)
> +#define  PHY_HS_LINE_TX_MODE		BIT(13)
> +#define  PHY_FS_LINE_TX_MODE		BIT(14)
> +
> +#define  PHY_INIT_MODE_BITS		(PHY_FS_LINE_TX_MODE | PHY_HS_LINE_TX_MODE)
> +#define  PHY_CLK_ENABLE_BITS		(PHY_CLK_PLL_EN | PHY_CLK_CDR_EN | \
> +					 PHY_CLK_MAC_EN)
> +#define  PHY_DEASSERT_RST_BITS		(PHY_PLL_RSTN | PHY_CDR_RSTN | \
> +					 PHY_MAC_RSTN)
> +
> +#define PHY_TX_HOST_CTRL		0x10
> +#define  PHY_HST_DISC_AUTO_CLR		BIT(2)
> +
> +#define PHY_HSTXP_HW_CTRL		0x34
> +#define  PHY_HSTXP_RSTN			BIT(2)
> +#define  PHY_CLK_HSTXP_EN		BIT(3)
> +#define  PHY_HSTXP_MODE			BIT(4)
> +
> +#define PHY_PLL_DIV_CFG			0x98
> +#define  PHY_FDIV_FRACT_8_15		GENMASK(7, 0)
> +#define  PHY_FDIV_FRACT_16_19		GENMASK(11, 8)
> +#define  PHY_FDIV_FRACT_20_21		BIT(12)
> +#define  PHY_FDIV_FRACT_0_1		GENMASK(14, 13)
> +#define  PHY_DIV_LOCAL_EN		BIT(15)
> +
> +#define  PHY_SEL_FREQ_24MHZ		0x01
> +#define  FDIV_REG_MASK			(PHY_FDIV_FRACT_20_21 | PHY_FDIV_FRACT_16_19 | \
> +					 PHY_FDIV_FRACT_8_15)
> +#define  FDIV_REG_VAL			0x1ec4
> +
> +/* Vendor HS DAC current setting: +15 % drive, current regulator on */
> +#define PHY_ANALOG_REG14_13		0xa4
> +#define  PHY_HSDAC_ISEL			GENMASK(3, 0)
> +#define  PHY_HSDAC_ISEL_15_INC		0xc
> +#define  PHY_HSDAC_IREG_EN		BIT(4)
> +
> +#define K1_USB2PHY_PLL_TIMEOUT_US	50000
> +
> +struct spacemit_usb2phy {
> +	void __iomem *base;
> +	struct clk clk;
> +};
> +
> +static int spacemit_usb2phy_init(struct phy *phy)
> +{
> +	struct spacemit_usb2phy *sphy = dev_get_priv(phy->dev);
> +	void __iomem *base = sphy->base;
> +	u32 val;
> +	int ret;
> +
> +	ret = clk_enable(&sphy->clk);
> +	if (ret) {
> +		dev_err(phy->dev, "failed to enable clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Let any controller reset settle before touching the PHY */
> +	udelay(200);
> +
> +	/*
> +	 * Program the PLL divider for the 24 MHz reference first. Until it is
> +	 * written the PLL does not run, the clock enables below do not latch
> +	 * and the PHY gives the DWC3 no UTMI clock, so the xHCI reset
> +	 * (USBCMD.HCRST) never completes. Found on the OrangePi R2S under
> +	 * FreeBSD, whose earlier driver polled for the PLL before this write.
> +	 */
> +	val = FIELD_PREP(FDIV_REG_MASK, FDIV_REG_VAL) |
> +	      FIELD_PREP(PHY_FDIV_FRACT_0_1, PHY_SEL_FREQ_24MHZ) |
> +	      PHY_DIV_LOCAL_EN;
> +	writel(val, base + PHY_PLL_DIV_CFG);
> +
> +	ret = readl_poll_timeout(base + PHY_RST_MODE_CTRL, val,
> +				 val & PHY_PLL_RDY, K1_USB2PHY_PLL_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(phy->dev, "PLL lock timeout\n");
> +		clk_disable(&sphy->clk);
> +		return ret;
> +	}
> +
> +	/* Release the PHY's internal resets and enable its clocks */
> +	writel(PHY_INIT_MODE_BITS | PHY_CLK_ENABLE_BITS | PHY_DEASSERT_RST_BITS,
> +	       base + PHY_RST_MODE_CTRL);
..
> +	val = readl(base + PHY_RST_MODE_CTRL);
> +	if ((val & PHY_CLK_ENABLE_BITS) != PHY_CLK_ENABLE_BITS)
> +		dev_warn(phy->dev, "clock enables did not latch (%#x)\n", val);
So read back and check.. if this situation is happening, a warning
message won't save life.

But, I need to check if the "enable bits" actually reflect the clock
enabling status from the hardware perspective.. but even leave that alone,
I think using a poll() instead of a readl() is a more proper way to go?
usually it will take few time for the hardware to settle down..

> +
> +	writel(PHY_HSTXP_RSTN | PHY_CLK_HSTXP_EN | PHY_HSTXP_MODE,
> +	       base + PHY_HSTXP_HW_CTRL);
> +
> +	clrsetbits_le32(base + PHY_ANALOG_REG14_13, PHY_HSDAC_ISEL,
> +			FIELD_PREP(PHY_HSDAC_ISEL, PHY_HSDAC_ISEL_15_INC) |
> +			PHY_HSDAC_IREG_EN);
> +
> +	/* Auto-clear the HS host disconnect state on reconnect */
> +	setbits_le32(base + PHY_TX_HOST_CTRL, PHY_HST_DISC_AUTO_CLR);
> +
> +	return 0;
> +}

-- 
Yixun Lan (dlan)