[PATCH] phy: zynqmp: Add serdes/psgtr driver

Michal Simek michal.simek at xilinx.com
Wed Nov 24 15:52:11 CET 2021



On 11/22/21 22:53, Sean Anderson wrote:
> 
> 
> On 11/18/21 7:30 AM, Michal Simek wrote:
>> Add PSGTR driver for Xilinx ZynqMP.
>> The most of configurations are taken from Linux kernel psgtr driver.
>>
>> USB3.0 and SGMII configurations are tested on SOM. In SGMII case also
>> IOU_SLCR reg is updated to get proper clock setup and signal detection
>> configuration.
> 
> Are USB3 and SGMII all that's been tested? I noticed that the kernel
> driver has DP- and SATA-specific stuff which has been left out.
> Presumably they are not supported?

I have tested USB3 and SGMII. I didn't test DP/SATA and there is missing 
some code for it.
DP will be tested with u-boot driver which we will develop.

> 
> Perhaps also note that the termination fix is not implemented.

Terminanation fix was for v1 silicon which none is really using now 
that's why this code is not needed for newly written SW.

> 
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>>   MAINTAINERS              |   1 +
>>   drivers/phy/Kconfig      |   7 +
>>   drivers/phy/Makefile     |   1 +
>>   drivers/phy/phy-zynqmp.c | 690 +++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 699 insertions(+)
>>   create mode 100644 drivers/phy/phy-zynqmp.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1eb71cbdad12..d1e9fbd4a279 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -611,6 +611,7 @@ F:    drivers/mmc/zynq_sdhci.c
>>   F:    drivers/mtd/nand/raw/zynq_nand.c
>>   F:    drivers/net/phy/xilinx_phy.c
>>   F:    drivers/net/zynq_gem.c
>> +F:    drivers/phy/phy-zynqmp.c
>>   F:    drivers/serial/serial_zynq.c
>>   F:    drivers/reset/reset-zynqmp.c
>>   F:    drivers/rtc/zynqmp_rtc.c
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 4767d215f337..d79798429b18 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -281,6 +281,13 @@ config PHY_IMX8MQ_USB
>>       help
>>         Support the USB3.0 PHY in NXP i.MX8MQ SoC
>>
>> +config PHY_XILINX_ZYNQMP
>> +    tristate "Xilinx ZynqMP PHY driver"
>> +    depends on PHY && ARCH_ZYNQMP
>> +    help
>> +      Enable this to support ZynqMP High Speed Gigabit Transceiver
>> +      that is part of ZynqMP SoC.
>> +
>>   source "drivers/phy/rockchip/Kconfig"
>>   source "drivers/phy/cadence/Kconfig"
>>   source "drivers/phy/ti/Kconfig"
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 13a8ade8919f..bf9b40932fe3 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -38,5 +38,6 @@ obj-$(CONFIG_MT76X8_USB_PHY) += mt76x8-usb-phy.o
>>   obj-$(CONFIG_PHY_DA8XX_USB) += phy-da8xx-usb.o
>>   obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
>>   obj-$(CONFIG_PHY_IMX8MQ_USB) += phy-imx8mq-usb.o
>> +obj-$(CONFIG_PHY_XILINX_ZYNQMP) += phy-zynqmp.o
>>   obj-y += cadence/
>>   obj-y += ti/
>> diff --git a/drivers/phy/phy-zynqmp.c b/drivers/phy/phy-zynqmp.c
>> new file mode 100644
>> index 000000000000..d6fe8dcef74e
>> --- /dev/null
>> +++ b/drivers/phy/phy-zynqmp.c
>> @@ -0,0 +1,690 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * phy-zynqmp.c - PHY driver for Xilinx ZynqMP GT.
>> + *
>> + * Copyright (C) 2018-2021 Xilinx Inc.
>> + *
>> + * Author: Anurag Kumar Vulisha <anuragku at xilinx.com>
>> + * Author: Subbaraya Sundeep <sundeep.lkml at gmail.com>
>> + * Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <clk-uclass.h>
>> +#include <dm.h>
>> +#include <log.h>
>> +#include <dm/device.h>
>> +#include <dm/device_compat.h>
>> +#include <dm/lists.h>
>> +#include <dt-bindings/phy/phy.h>
>> +#include <generic-phy.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/sys_proto.h>
>> +#include <power-domain.h>
>> +#include <regmap.h>
>> +#include <syscon.h>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +
>> +#include <dt-bindings/phy/phy.h>
>> +
>> +#include <asm/arch/hardware.h>
>> +
>> +/*
>> + * Lane Registers
>> + */
>> +
>> +/* TX De-emphasis parameters */
>> +#define L0_TX_ANA_TM_18            0x0048
>> +#define L0_TX_ANA_TM_118        0x01d8
>> +#define L0_TX_ANA_TM_118_FORCE_17_0    BIT(0)
>> +
>> +/* DN Resistor calibration code parameters */
>> +#define L0_TXPMA_ST_3            0x0b0c
>> +#define L0_DN_CALIB_CODE        0x3f
>> +
>> +/* PMA control parameters */
>> +#define L0_TXPMD_TM_45            0x0cb4
>> +#define L0_TXPMD_TM_48            0x0cc0
>> +#define L0_TXPMD_TM_45_OVER_DP_MAIN    BIT(0)
>> +#define L0_TXPMD_TM_45_ENABLE_DP_MAIN    BIT(1)
>> +#define L0_TXPMD_TM_45_OVER_DP_POST1    BIT(2)
>> +#define L0_TXPMD_TM_45_ENABLE_DP_POST1    BIT(3)
>> +#define L0_TXPMD_TM_45_OVER_DP_POST2    BIT(4)
>> +#define L0_TXPMD_TM_45_ENABLE_DP_POST2    BIT(5)
>> +
>> +/* PCS control parameters */
>> +#define L0_TM_DIG_6            0x106c
>> +#define L0_TM_DIS_DESCRAMBLE_DECODER    0x0f
>> +#define L0_TX_DIG_61            0x00f4
>> +#define L0_TM_DISABLE_SCRAMBLE_ENCODER    0x0f
>> +
>> +/* PLL Test Mode register parameters */
>> +#define L0_TM_PLL_DIG_37        0x2094
>> +#define L0_TM_COARSE_CODE_LIMIT        0x10
>> +
>> +/* PLL SSC step size offsets */
>> +#define L0_PLL_SS_STEPS_0_LSB        0x2368
>> +#define L0_PLL_SS_STEPS_1_MSB        0x236c
>> +#define L0_PLL_SS_STEP_SIZE_0_LSB    0x2370
>> +#define L0_PLL_SS_STEP_SIZE_1        0x2374
>> +#define L0_PLL_SS_STEP_SIZE_2        0x2378
>> +#define L0_PLL_SS_STEP_SIZE_3_MSB    0x237c
>> +#define L0_PLL_STATUS_READ_1        0x23e4
>> +
>> +/* SSC step size parameters */
>> +#define STEP_SIZE_0_MASK        0xff
>> +#define STEP_SIZE_1_MASK        0xff
>> +#define STEP_SIZE_2_MASK        0xff
>> +#define STEP_SIZE_3_MASK        0x3
>> +#define STEP_SIZE_SHIFT            8
>> +#define FORCE_STEP_SIZE            0x10
>> +#define FORCE_STEPS            0x20
>> +#define STEPS_0_MASK            0xff
>> +#define STEPS_1_MASK            0x07
>> +
>> +/* Reference clock selection parameters */
>> +#define L0_Ln_REF_CLK_SEL(n)        (0x2860 + (n) * 4)
>> +#define L0_REF_CLK_SEL_MASK        0x8f
>> +
>> +/* Calibration digital logic parameters */
>> +#define L3_TM_CALIB_DIG19        0xec4c
>> +#define L3_CALIB_DONE_STATUS        0xef14
>> +#define L3_TM_CALIB_DIG18        0xec48
>> +#define L3_TM_CALIB_DIG19_NSW        0x07
>> +#define L3_TM_CALIB_DIG18_NSW        0xe0
>> +#define L3_TM_OVERRIDE_NSW_CODE         0x20
>> +#define L3_CALIB_DONE            0x02
>> +#define L3_NSW_SHIFT            5
>> +#define L3_NSW_PIPE_SHIFT        4
>> +#define L3_NSW_CALIB_SHIFT        3
>> +
>> +#define PHY_REG_OFFSET            0x4000
>> +
>> +/*
>> + * Global Registers
>> + */
>> +
>> +/* Refclk selection parameters */
>> +#define PLL_REF_SEL(n)            (0x10000 + (n) * 4)
>> +#define PLL_FREQ_MASK            0x1f
>> +#define PLL_STATUS_LOCKED        0x10
>> +
>> +/* Inter Connect Matrix parameters */
>> +#define ICM_CFG0            0x10010
>> +#define ICM_CFG1            0x10014
>> +#define ICM_CFG0_L0_MASK        0x07
>> +#define ICM_CFG0_L1_MASK        0x70
>> +#define ICM_CFG1_L2_MASK        0x07
>> +#define ICM_CFG2_L3_MASK        0x70
>> +#define ICM_CFG_SHIFT            4
>> +
>> +/* Inter Connect Matrix allowed protocols */
>> +#define ICM_PROTOCOL_PD            0x0
>> +#define ICM_PROTOCOL_PCIE        0x1
>> +#define ICM_PROTOCOL_SATA        0x2
>> +#define ICM_PROTOCOL_USB        0x3
>> +#define ICM_PROTOCOL_DP            0x4
>> +#define ICM_PROTOCOL_SGMII        0x5
>> +
>> +/* Test Mode common reset control  parameters */
>> +#define TM_CMN_RST            0x10018
>> +#define TM_CMN_RST_EN            0x1
>> +#define TM_CMN_RST_SET            0x2
>> +#define TM_CMN_RST_MASK            0x3
>> +
>> +/* Bus width parameters */
>> +#define TX_PROT_BUS_WIDTH        0x10040
>> +#define RX_PROT_BUS_WIDTH        0x10044
>> +#define PROT_BUS_WIDTH_10        0x0
>> +#define PROT_BUS_WIDTH_20        0x1
>> +#define PROT_BUS_WIDTH_40        0x2
>> +#define PROT_BUS_WIDTH_MASK        0x3
>> +#define PROT_BUS_WIDTH_SHIFT        2
>> +
>> +/* Number of GT lanes */
>> +#define NUM_LANES            4
>> +
>> +/* SIOU SATA control register */
>> +#define SATA_CONTROL_OFFSET        0x0100
>> +
>> +/* Total number of controllers */
>> +#define CONTROLLERS_PER_LANE        5
>> +
>> +/* Protocol Type parameters */
>> +#define XPSGTR_TYPE_USB0        0  /* USB controller 0 */
>> +#define XPSGTR_TYPE_USB1        1  /* USB controller 1 */
>> +#define XPSGTR_TYPE_SATA_0        2  /* SATA controller lane 0 */
>> +#define XPSGTR_TYPE_SATA_1        3  /* SATA controller lane 1 */
>> +#define XPSGTR_TYPE_PCIE_0        4  /* PCIe controller lane 0 */
>> +#define XPSGTR_TYPE_PCIE_1        5  /* PCIe controller lane 1 */
>> +#define XPSGTR_TYPE_PCIE_2        6  /* PCIe controller lane 2 */
>> +#define XPSGTR_TYPE_PCIE_3        7  /* PCIe controller lane 3 */
>> +#define XPSGTR_TYPE_DP_0        8  /* Display Port controller lane 0 */
>> +#define XPSGTR_TYPE_DP_1        9  /* Display Port controller lane 1 */
>> +#define XPSGTR_TYPE_SGMII0        10 /* Ethernet SGMII controller 0 */
>> +#define XPSGTR_TYPE_SGMII1        11 /* Ethernet SGMII controller 1 */
>> +#define XPSGTR_TYPE_SGMII2        12 /* Ethernet SGMII controller 2 */
>> +#define XPSGTR_TYPE_SGMII3        13 /* Ethernet SGMII controller 3 */
>> +
>> +/* Timeout values */
>> +#define TIMEOUT_US            1000
>> +
>> +#define IOU_SLCR_GEM_CLK_CTRL        0x308
>> +#define GEM_CTRL_GEM_SGMII_MODE        BIT(2)
>> +#define GEM_CTRL_GEM_REF_SRC_SEL    BIT(1)
>> +
>> +#define IOU_SLCR_GEM_CTRL        0x360
>> +#define GEM_CTRL_GEM_SGMII_SD        BIT(0)
>> +
>> +/**
>> + * struct xpsgtr_ssc - structure to hold SSC settings for a lane
>> + * @refclk_rate: PLL reference clock frequency
>> + * @pll_ref_clk: value to be written to register for corresponding 
>> ref clk rate
>> + * @steps: number of steps of SSC (Spread Spectrum Clock)
>> + * @step_size: step size of each step
>> + */
>> +struct xpsgtr_ssc {
>> +    u32 refclk_rate;
>> +    u8  pll_ref_clk;
>> +    u32 steps;
>> +    u32 step_size;
>> +};
>> +
>> +/**
>> + * struct xpsgtr_phy - representation of a lane
>> + * @dev: pointer to the xpsgtr_dev instance
>> + * @refclk: reference clock index
>> + * @type: controller which uses this lane
>> + * @lane: lane number
>> + * @protocol: protocol in which the lane operates
>> + */
>> +struct xpsgtr_phy {
>> +    struct xpsgtr_dev *dev;
>> +    unsigned int refclk;
>> +    u8 type;
>> +    u8 lane;
>> +    u8 protocol;
>> +};
>> +
>> +/**
>> + * struct xpsgtr_dev - representation of a ZynMP GT device
>> + * @dev: pointer to device
>> + * @serdes: serdes base address
>> + * @siou: siou base address
>> + * @phys: PHY lanes
>> + * @refclk_sscs: spread spectrum settings for the reference clocks
>> + * @clk: reference clocks
>> + */
>> +struct xpsgtr_dev {
>> +    struct udevice *dev;
>> +    u8 *serdes;
>> +    u8 *siou;
>> +    struct xpsgtr_phy phys[NUM_LANES];
>> +    const struct xpsgtr_ssc *refclk_sscs[NUM_LANES];
>> +    struct clk clk[NUM_LANES];
>> +};
>> +
>> +/*
>> + * Configuration Data
>> + */
>> +/* lookup table to hold all settings needed for a ref clock frequency */
>> +static const struct xpsgtr_ssc ssc_lookup[] = {
>> +    {  19200000, 0x05,  608, 264020 },
>> +    {  20000000, 0x06,  634, 243454 },
>> +    {  24000000, 0x07,  760, 168973 },
>> +    {  26000000, 0x08,  824, 143860 },
>> +    {  27000000, 0x09,  856,  86551 },
>> +    {  38400000, 0x0a, 1218,  65896 },
>> +    {  40000000, 0x0b,  634, 243454 },
>> +    {  52000000, 0x0c,  824, 143860 },
>> +    { 100000000, 0x0d, 1058,  87533 },
>> +    { 108000000, 0x0e,  856,  86551 },
>> +    { 125000000, 0x0f,  992, 119497 },
>> +    { 135000000, 0x10, 1070,  55393 },
>> +    { 150000000, 0x11,  792, 187091 }
>> +};
>> +
>> +/*
>> + * I/O Accessors
>> + */
>> +
>> +static inline u32 xpsgtr_read(struct xpsgtr_dev *gtr_dev, u32 reg)
>> +{
>> +    return readl(gtr_dev->serdes + reg);
>> +}
>> +
>> +static inline void xpsgtr_write(struct xpsgtr_dev *gtr_dev, u32 reg, 
>> u32 value)
>> +{
>> +    writel(value, gtr_dev->serdes + reg);
>> +}
>> +
>> +static inline void xpsgtr_clr_set(struct xpsgtr_dev *gtr_dev, u32 reg,
>> +                  u32 clr, u32 set)
>> +{
>> +    u32 value = xpsgtr_read(gtr_dev, reg);
>> +
>> +    value &= ~clr;
>> +    value |= set;
>> +    xpsgtr_write(gtr_dev, reg, value);
>> +}
>> +
>> +static inline u32 xpsgtr_read_phy(struct xpsgtr_phy *gtr_phy, u32 reg)
>> +{
>> +    void __iomem *addr = gtr_phy->dev->serdes
>> +               + gtr_phy->lane * PHY_REG_OFFSET + reg;
>> +
>> +    return readl(addr);
>> +}
>> +
>> +static inline void xpsgtr_write_phy(struct xpsgtr_phy *gtr_phy,
>> +                    u32 reg, u32 value)
>> +{
>> +    void __iomem *addr = gtr_phy->dev->serdes
>> +               + gtr_phy->lane * PHY_REG_OFFSET + reg;
>> +
>> +    writel(value, addr);
>> +}
>> +
>> +static inline void xpsgtr_clr_set_phy(struct xpsgtr_phy *gtr_phy,
>> +                      u32 reg, u32 clr, u32 set)
>> +{
>> +    void __iomem *addr = gtr_phy->dev->serdes
>> +               + gtr_phy->lane * PHY_REG_OFFSET + reg;
>> +
>> +    writel((readl(addr) & ~clr) | set, addr);
>> +}
>> +
>> +/* Configure PLL and spread-sprectrum clock. */
>> +static void xpsgtr_configure_pll(struct xpsgtr_phy *gtr_phy)
>> +{
>> +    const struct xpsgtr_ssc *ssc;
>> +    u32 step_size;
>> +
>> +    ssc = gtr_phy->dev->refclk_sscs[gtr_phy->refclk];
>> +    step_size = ssc->step_size;
>> +
>> +    xpsgtr_clr_set(gtr_phy->dev, PLL_REF_SEL(gtr_phy->lane),
>> +               PLL_FREQ_MASK, ssc->pll_ref_clk);
>> +
>> +    /* Enable lane clock sharing, if required */
>> +    if (gtr_phy->refclk != gtr_phy->lane) {
>> +        /* Lane3 Ref Clock Selection Register */
>> +        xpsgtr_clr_set(gtr_phy->dev, L0_Ln_REF_CLK_SEL(gtr_phy->lane),
>> +                   L0_REF_CLK_SEL_MASK, 1 << gtr_phy->refclk);
>> +    }
>> +
>> +    /* SSC step size [7:0] */
>> +    xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_0_LSB,
>> +               STEP_SIZE_0_MASK, step_size & STEP_SIZE_0_MASK);
>> +
>> +    /* SSC step size [15:8] */
>> +    step_size >>= STEP_SIZE_SHIFT;
>> +    xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_1,
>> +               STEP_SIZE_1_MASK, step_size & STEP_SIZE_1_MASK);
>> +
>> +    /* SSC step size [23:16] */
>> +    step_size >>= STEP_SIZE_SHIFT;
>> +    xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_2,
>> +               STEP_SIZE_2_MASK, step_size & STEP_SIZE_2_MASK);
>> +
>> +    /* SSC steps [7:0] */
>> +    xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEPS_0_LSB,
>> +               STEPS_0_MASK, ssc->steps & STEPS_0_MASK);
>> +
>> +    /* SSC steps [10:8] */
>> +    xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEPS_1_MSB,
>> +               STEPS_1_MASK,
>> +               (ssc->steps >> STEP_SIZE_SHIFT) & STEPS_1_MASK);
>> +
>> +    /* SSC step size [24:25] */
>> +    step_size >>= STEP_SIZE_SHIFT;
>> +    xpsgtr_clr_set_phy(gtr_phy, L0_PLL_SS_STEP_SIZE_3_MSB,
>> +               STEP_SIZE_3_MASK, (step_size & STEP_SIZE_3_MASK) |
>> +               FORCE_STEP_SIZE | FORCE_STEPS);
>> +}
>> +
>> +/* Configure the lane protocol. */
>> +static void xpsgtr_lane_set_protocol(struct xpsgtr_phy *gtr_phy)
>> +{
>> +    struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>> +    u8 protocol = gtr_phy->protocol;
>> +
>> +    switch (gtr_phy->lane) {
>> +    case 0:
>> +        xpsgtr_clr_set(gtr_dev, ICM_CFG0, ICM_CFG0_L0_MASK, protocol);
>> +        break;
>> +    case 1:
>> +        xpsgtr_clr_set(gtr_dev, ICM_CFG0, ICM_CFG0_L1_MASK,
>> +                   protocol << ICM_CFG_SHIFT);
>> +        break;
>> +    case 2:
>> +        xpsgtr_clr_set(gtr_dev, ICM_CFG1, ICM_CFG0_L0_MASK, protocol);
>> +        break;
>> +    case 3:
>> +        xpsgtr_clr_set(gtr_dev, ICM_CFG1, ICM_CFG0_L1_MASK,
>> +                   protocol << ICM_CFG_SHIFT);
>> +        break;
>> +    default:
>> +        /* We already checked 0 <= lane <= 3 */
>> +        break;
>> +    }
>> +}
>> +
>> +/* Bypass (de)scrambler and 8b/10b decoder and encoder. */
>> +static void xpsgtr_bypass_scrambler_8b10b(struct xpsgtr_phy *gtr_phy)
>> +{
>> +    xpsgtr_write_phy(gtr_phy, L0_TM_DIG_6, 
>> L0_TM_DIS_DESCRAMBLE_DECODER);
>> +    xpsgtr_write_phy(gtr_phy, L0_TX_DIG_61, 
>> L0_TM_DISABLE_SCRAMBLE_ENCODER);
>> +}
>> +
>> +/* SGMII-specific initialization. */
>> +static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy *gtr_phy)
>> +{
>> +    struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
>> +    u32 shift = gtr_phy->lane * PROT_BUS_WIDTH_SHIFT;
>> +
>> +    /* Set SGMII protocol TX and RX bus width to 10 bits. */
>> +    xpsgtr_clr_set(gtr_dev, TX_PROT_BUS_WIDTH, PROT_BUS_WIDTH_MASK << 
>> shift,
>> +               PROT_BUS_WIDTH_10 << shift);
>> +
>> +    xpsgtr_clr_set(gtr_dev, RX_PROT_BUS_WIDTH, PROT_BUS_WIDTH_MASK << 
>> shift,
>> +               PROT_BUS_WIDTH_10 << shift);
>> +
>> +    xpsgtr_bypass_scrambler_8b10b(gtr_phy);
>> +
>> +    /*
>> +     * Below code is just temporary solution till we have a way how to
>> +     * do it via firmware interface in sync with Linux. Till that happen
>> +     * this is the most sensible thing to do here.
>> +     */
>> +    /* GEM I/O Clock Control */
>> +    clrsetbits_le32(ZYNQMP_IOU_SLCR_BASEADDR + IOU_SLCR_GEM_CLK_CTRL,
>> +            0xf << shift,
>> +            (GEM_CTRL_GEM_SGMII_MODE | GEM_CTRL_GEM_REF_SRC_SEL) <<
>> +            shift);
>> +
>> +    /* Setup signal detect */
>> +    clrsetbits_le32(ZYNQMP_IOU_SLCR_BASEADDR + IOU_SLCR_GEM_CTRL,
>> +            PROT_BUS_WIDTH_MASK << shift,
>> +            GEM_CTRL_GEM_SGMII_SD << shift);
>> +}
>> +
>> +static int xpsgtr_init(struct phy *x)
>> +{
>> +    struct xpsgtr_dev *gtr_dev = dev_get_priv(x->dev);
>> +    struct xpsgtr_phy *gtr_phy;
>> +    u32 phy_lane = x->id;
>> +
>> +    gtr_phy = &gtr_dev->phys[phy_lane];
>> +
>> +    /* Enable coarse code saturation limiting logic. */
>> +    xpsgtr_write_phy(gtr_phy, L0_TM_PLL_DIG_37, 
>> L0_TM_COARSE_CODE_LIMIT);
>> +
>> +    /*
>> +     * Configure the PLL, the lane protocol, and perform 
>> protocol-specific
>> +     * initialization.
>> +     */
>> +    xpsgtr_configure_pll(gtr_phy);
>> +    xpsgtr_lane_set_protocol(gtr_phy);
>> +
>> +    switch (gtr_phy->protocol) {
>> +    case ICM_PROTOCOL_SGMII:
>> +        xpsgtr_phy_init_sgmii(gtr_phy);
>> +        break;
>> +    case ICM_PROTOCOL_DP:
>> +    case ICM_PROTOCOL_SATA:
>> +        return -EINVAL;
>> +    }
>> +    return 0;
>> +}
>> +
>> +/*
>> + * OF Xlate Support
>> + */
>> +
>> +/* Set the lane type and protocol based on the PHY type and instance 
>> number. */
>> +static int xpsgtr_set_lane_type(struct xpsgtr_phy *gtr_phy, u8 phy_type,
>> +                unsigned int phy_instance)
>> +{
>> +    unsigned int num_phy_types;
>> +    const int *phy_types;
>> +
>> +    switch (phy_type) {
>> +    case PHY_TYPE_SATA: {
>> +        static const int types[] = {
>> +            XPSGTR_TYPE_SATA_0,
>> +            XPSGTR_TYPE_SATA_1,
>> +        };
>> +
>> +        phy_types = types;
>> +        num_phy_types = ARRAY_SIZE(types);
>> +        gtr_phy->protocol = ICM_PROTOCOL_SATA;
>> +        break;
>> +    }
>> +    case PHY_TYPE_USB3: {
>> +        static const int types[] = {
>> +            XPSGTR_TYPE_USB0,
>> +            XPSGTR_TYPE_USB1,
>> +        };
>> +
>> +        phy_types = types;
>> +        num_phy_types = ARRAY_SIZE(types);
>> +        gtr_phy->protocol = ICM_PROTOCOL_USB;
>> +        break;
>> +    }
>> +    case PHY_TYPE_DP: {
>> +        static const int types[] = {
>> +            XPSGTR_TYPE_DP_0,
>> +            XPSGTR_TYPE_DP_1,
>> +        };
>> +
>> +        phy_types = types;
>> +        num_phy_types = ARRAY_SIZE(types);
>> +        gtr_phy->protocol = ICM_PROTOCOL_DP;
>> +        break;
>> +    }
>> +    case PHY_TYPE_PCIE: {
>> +        static const int types[] = {
>> +            XPSGTR_TYPE_PCIE_0,
>> +            XPSGTR_TYPE_PCIE_1,
>> +            XPSGTR_TYPE_PCIE_2,
>> +            XPSGTR_TYPE_PCIE_3,
>> +        };
>> +
>> +        phy_types = types;
>> +        num_phy_types = ARRAY_SIZE(types);
>> +        gtr_phy->protocol = ICM_PROTOCOL_PCIE;
>> +        break;
>> +    }
>> +    case PHY_TYPE_SGMII: {
>> +        static const int types[] = {
>> +            XPSGTR_TYPE_SGMII0,
>> +            XPSGTR_TYPE_SGMII1,
>> +            XPSGTR_TYPE_SGMII2,
>> +            XPSGTR_TYPE_SGMII3,
>> +        };
>> +
>> +        phy_types = types;
>> +        num_phy_types = ARRAY_SIZE(types);
>> +        gtr_phy->protocol = ICM_PROTOCOL_SGMII;
>> +        break;
>> +    }
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (phy_instance >= num_phy_types)
>> +        return -EINVAL;
>> +
>> +    gtr_phy->type = phy_types[phy_instance];
>> +    return 0;
>> +}
>> +
>> +/*
>> + * Valid combinations of controllers and lanes (Interconnect Matrix).
>> + */
>> +static const unsigned int icm_matrix[NUM_LANES][CONTROLLERS_PER_LANE] 
>> = {
>> +    { XPSGTR_TYPE_PCIE_0, XPSGTR_TYPE_SATA_0, XPSGTR_TYPE_USB0,
>> +        XPSGTR_TYPE_DP_1, XPSGTR_TYPE_SGMII0 },
>> +    { XPSGTR_TYPE_PCIE_1, XPSGTR_TYPE_SATA_1, XPSGTR_TYPE_USB0,
>> +        XPSGTR_TYPE_DP_0, XPSGTR_TYPE_SGMII1 },
>> +    { XPSGTR_TYPE_PCIE_2, XPSGTR_TYPE_SATA_0, XPSGTR_TYPE_USB0,
>> +        XPSGTR_TYPE_DP_1, XPSGTR_TYPE_SGMII2 },
>> +    { XPSGTR_TYPE_PCIE_3, XPSGTR_TYPE_SATA_1, XPSGTR_TYPE_USB1,
>> +        XPSGTR_TYPE_DP_0, XPSGTR_TYPE_SGMII3 }
>> +};
>> +
>> +/* Translate OF phandle and args to PHY instance. */
>> +static int xpsgtr_of_xlate(struct phy *x,
>> +               struct ofnode_phandle_args *args)
>> +{
>> +    struct xpsgtr_dev *gtr_dev = dev_get_priv(x->dev);
>> +    struct xpsgtr_phy *gtr_phy;
>> +    struct udevice *dev = x->dev;
>> +    unsigned int phy_instance;
>> +    unsigned int phy_lane;
>> +    unsigned int phy_type;
>> +    unsigned int refclk;
>> +    unsigned int i;
>> +    int ret;
>> +
>> +    if (args->args_count != 4) {
>> +        dev_err(dev, "Invalid number of cells in 'phy' property\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /*
>> +     * Get the PHY parameters from the OF arguments and derive the lane
>> +     * type.
>> +     */
>> +    phy_lane = args->args[0];
>> +    if (phy_lane >= NUM_LANES) {
>> +        dev_err(dev, "Invalid lane number %u\n", phy_lane);
>> +        return -ENODEV;
>> +    }
>> +
>> +    gtr_phy = &gtr_dev->phys[phy_lane];
>> +    phy_type = args->args[1];
>> +    phy_instance = args->args[2];
>> +
>> +    ret = xpsgtr_set_lane_type(gtr_phy, phy_type, phy_instance);
>> +    if (ret < 0) {
>> +        dev_err(dev, "Invalid PHY type and/or instance\n");
>> +        return ret;
>> +    }
>> +
>> +    refclk = args->args[3];
>> +    if (refclk >= ARRAY_SIZE(gtr_dev->refclk_sscs) ||
>> +        !gtr_dev->refclk_sscs[refclk]) {
>> +        dev_err(dev, "Invalid reference clock number %u\n", refclk);
>> +        return -EINVAL;
>> +    }
>> +
>> +    gtr_phy->refclk = refclk;
>> +
>> +    /* This is difference compare to Linux */
>> +    gtr_phy->dev = gtr_dev;
>> +    gtr_phy->lane = phy_lane;
> 
> Why don't we set these in probe()?


Because lane number is not the part of psgtr node but the part of node 
which is requesting phy.


> 
>> +
>> +    /*
>> +     * Ensure that the Interconnect Matrix is obeyed, i.e a given 
>> lane type
>> +     * is allowed to operate on the lane.
>> +     */
>> +    for (i = 0; i < CONTROLLERS_PER_LANE; i++) {
>> +        if (icm_matrix[phy_lane][i] == gtr_phy->type) {
>> +            x->id = phy_lane;
>> +            return 0;
>> +        }
>> +    }
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +/*
>> + * Probe & Platform Driver
>> + */
>> +static int xpsgtr_get_ref_clocks(struct udevice *dev)
>> +{
>> +    unsigned int refclk;
>> +    struct xpsgtr_dev *gtr_dev = dev_get_priv(dev);
>> +    int ret;
>> +
>> +    for (refclk = 0; refclk < 4; ++refclk) {
> 
> refclk < ARRAY_SIZE(gtr_dev->clk)? refclk < NUM_LANES?

Will fix.

> 
>> +        int i;
>> +        u32 rate;
>> +        char name[8];
>> +        struct clk *clk = &gtr_dev->clk[refclk];
>> +
>> +        snprintf(name, sizeof(name), "ref%u", refclk);
>> +        dev_dbg(dev, "Checking name: %s\n", name);
>> +        ret = clk_get_by_name(dev, name, clk);
>> +        if (ret)
> 
> This should be
> 
>      if (ret == -ENODATA) {
>          continue;
>      } else if (ret) {
>          dev_dbg(dev, "couldn't get clock %s (err %d)", name, ret);
>          return ret;
>      }

Nope. I don't want to fail if I can't access that clock. The reason is 
that not all clocks are wired. If clock is not there that's completely 
fine and I want to continue not end. It is really just for the record 
which clocks are used. The same logic is in linux driver too.


> 
> although we really should have a clk_get_by_name_optional wrapper in
> U-Boot. I wonder if we need to clean up clocks as well...
> 
>> +            continue;
>> +
>> +        rate = clk_get_rate(clk);
>> +
>> +        dev_dbg(dev, "clk rate %d\n", rate);
>> +
>> +        ret = clk_enable(clk);
>> +        if (ret) {
>> +            dev_err(dev, "failed to enable refclk %d clock\n",
>> +                refclk);
>> +            return ret;
>> +        }
>> +
>> +        for (i = 0 ; i < ARRAY_SIZE(ssc_lookup); i++) {
>> +            if (rate == ssc_lookup[i].refclk_rate) {
>> +                gtr_dev->refclk_sscs[refclk] = &ssc_lookup[i];
>> +                dev_dbg(dev, "Found rate %d\n", i);
>> +                break;
>> +            }
>> +        }
>> +
>> +        if (i == ARRAY_SIZE(ssc_lookup)) {
>> +            dev_err(dev,
>> +                "Invalid rate %u for reference clock %u\n",
>> +                rate, refclk);
>> +            return -EINVAL;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int xpsgtr_probe(struct udevice *dev)
>> +{
>> +    struct xpsgtr_dev *gtr_dev = dev_get_priv(dev);
>> +
>> +    gtr_dev->serdes = (u8 *)dev_read_addr_name(dev, "serdes");
> 
> Why not dev_remap_addr_name?

will use.

> 
>> +    if (!gtr_dev->serdes)
> 
> And this check is wrong because on failure you will get back
> FDT_ADDR_T_NONE, not NULL.

Using dev_remap_addr_name should solve this problem.

> 
>> +        return -ENODEV;
>> +
>> +    gtr_dev->siou = (u8 *)dev_read_addr_name(dev, "siou");
>> +    if (!gtr_dev->siou)
>> +        return -ENODEV;
> 
> ditto

Will fix.

> 
>> +    gtr_dev->dev = dev;
>> +
>> +    return xpsgtr_get_ref_clocks(dev);
>> +}
>> +
>> +static const struct udevice_id xpsgtr_phy_ids[] = {
>> +    { .compatible = "xlnx,zynqmp-psgtr-v1.1", },
>> +    { }
>> +};
>> +
>> +static const struct phy_ops xpsgtr_phy_ops = {
>> +    .init = xpsgtr_init,
> 
> Why don't we need power_on? In Linux that calls xpsgtr_wait_pll_lock,
> which seems important.

In the case we have PLL are already locked all the time but that doesn't 
mean we can't add this checking. It should just go through on the first 
pass and never timeout.
Will look.

Thanks,
Michal






More information about the U-Boot mailing list