Hi, See question below. On 11/14/21 12:40 PM, Johan Jonker wrote: > Hi Yifeng, > > Some comments. Have a look if it's useful. > > On 10/25/21 10:06 AM, Yifeng Zhao wrote: >> This patch implements a combo phy driver for Rockchip SoCs >> with NaNeng IP block. This phy can be used as pcie-phy, usb3-phy, >> sata-phy or sgmii-phy. >> >> Signed-off-by: Yifeng Zhao <yifeng.zhao@xxxxxxxxxxxxxx> >> --- >> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c >> + >> +#define BIT_WRITEABLE_SHIFT 16 >> +#define REF_CLOCK_24MHz 24000000 >> +#define REF_CLOCK_25MHz 25000000 >> +#define REF_CLOCK_100MHz 100000000 >> +/* RK3568 T22 COMBO PHY REG */ >> +#define RK3568_T22_PHYREG5 0x14 >> +#define T22_PHYREG5_PLL_DIV_MASK GENMASK(7, 6) >> +#define T22_PHYREG5_PLL_DIV_SHIFT 6 >> +#define T22_PHYREG5_PLL_DIV_2 1 >> + >> +#define RK3568_T22_PHYREG6 0x18 Multi-PHY_Register007 Address: Operational Base + offset (0x0018) >> +#define T22_PHYREG6_TX_RTERM_MASK GENMASK(7, 4) >> +#define T22_PHYREG6_TX_RTERM_SHIFT 4 >> +#define T22_PHYREG6_TX_RTERM_50OHM 0x8 >> +#define T22_PHYREG6_RX_RTERM_MASK GENMASK(3, 0) >> +#define T22_PHYREG6_RX_RTERM_SHIFT 0 >> +#define T22_PHYREG6_RX_RTERM_44OHM 0xF >> + >> +#define RK3568_T22_PHYREG7 0x1C Multi-PHY_Register008 Address: Operational Base + offset (0x001C) >> +#define T22_PHYREG7_SSC_EN BIT(4) See Rockchip RK3568 TRM Part2 V1.0-20210111 page 1957-1958 It looks as if all PHYREG defines add -1 in value? I'm I correct here? === SSC_EN In U3 and SATA mode, this bit= 1, set SSC on In PCIE mode, this bit=0 set SSC off If Spread Spectrum Clocking (SSC) is used it is required that a common reference clock is used by the link partners. Most commercially available platforms with PCIe backplanes use SSC to reduce EMI. Maybe a dumb question for in a possible YAML description: Is this above text from the TRM correct? Is SSC only used in combination with PCIE instead of U3 and SATA? === Johan >> + >> +#define RK3568_T22_PHYREG10 0x28 >> +#define T22_PHYREG10_SU_TRIM_0_7 0xF0 >> + >> +#define RK3568_T22_PHYREG11 0x2C >> +#define T22_PHYREG11_PLL_LPF_ADJ 0x4 >> + >> +#define RK3568_T22_PHYREG12 0x30 Multi-PHY_Register013 Address: Operational Base + offset (0x0030) >> +#define T22_PHYREG12_RESISTER_MASK GENMASK(5, 4) >> +#define T22_PHYREG12_RESISTER_SHIFT 0x4 >> +#define T22_PHYREG12_RESISTER_HIGH_Z 0x3 >> +#define T22_PHYREG12_CKRCV_AMP0 BIT(7) >> + >> +#define RK3568_T22_PHYREG13 0x34 >> +#define T22_PHYREG13_CKRCV_AMP1 BIT(0) >> + >> +#define RK3568_T22_PHYREG14 0x38 >> +#define T22_PHYREG14_CTLE_EN BIT(0) >> +#define T22_PHYREG14_SSC_CNT_MASK GENMASK(7, 6) >> +#define T22_PHYREG14_SSC_CNT_SHIFT 6 >> +#define T22_PHYREG14_SSC_CNT_VALUE 0x1 >> + >> +#define RK3568_T22_PHYREG15 0x3C >> +#define T22_PHYREG15_SSC_CNT_VALUE 0x5f >> + >> +#define RK3568_T22_PHYREG17 0x44 >> +#define T22_PHYREG17_PLL_LOOP 0x32 >> + >> +#define RK3568_T22_PHYREG31 0x7C >> +#define T22_PHYREG31_SSC_MASK GENMASK(7, 4) >> +#define T22_PHYREG31_SSC_DIR_SHIFT 4 >> +#define T22_PHYREG31_SSC_UPWARD 0 >> +#define T22_PHYREG31_SSC_DOWNWARD 1 >> +#define T22_PHYREG31_SSC_OFFSET_SHIFT 6 >> +#define T22_PHYREG31_SSC_OFFSET_500PPM 1 >> + >> +#define RK3568_T22_PHYREG32 0x80 >> +#define T22_PHYREG32_PLL_KVCO_MASK GENMASK(4, 2) >> +#define T22_PHYREG32_PLL_KVCO_SHIFT 2 >> +#define T22_PHYREG32_PLL_KVCO_VALUE 2 >> +