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

Simon Glass sjg at chromium.org
Fri Dec 17 17:37:06 CET 2021


Hi Michal,

On Wed, 15 Dec 2021 at 02:58, Michal Simek <michal.simek at xilinx.com> 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.
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
>
> Changes in v2:
> - Add phy power on sequence
> - Use NUM_LANES in for loop
> - Use dev_remap_addr_name
> - Fix clk handling
>
>  MAINTAINERS              |   1 +
>  drivers/phy/Kconfig      |   7 +
>  drivers/phy/Makefile     |   1 +
>  drivers/phy/phy-zynqmp.c | 760 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 769 insertions(+)
>  create mode 100644 drivers/phy/phy-zynqmp.c

Reviewed-by: Simon Glass <sjg at chromium.org>

Various nits below.

>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index de42663ac1ea..b36a4c44716b 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..1f43e6441af6
> --- /dev/null
> +++ b/drivers/phy/phy-zynqmp.c
> @@ -0,0 +1,760 @@
> +// 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>

Please check ordering

https://www.denx.de/wiki/U-Boot/CodingStyle

[..]

> +/* 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 */

Could use an enum...

> +
> +/* 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

Single-line comment style

> + */
> +
> +static inline u32 xpsgtr_read(struct xpsgtr_dev *gtr_dev, u32 reg)

You don't really need to inline these and I imagine the compiler will do it

> +{
> +       return readl(gtr_dev->serdes + reg);
> +}
> +

[..]

> +/* 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;

-EINVAL or something else

> +       }
> +
> +       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) {

if (ret)

> +               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;
> +       }
> +

[..]

> +static int xpsgtr_probe(struct udevice *dev)
> +{
> +       struct xpsgtr_dev *gtr_dev = dev_get_priv(dev);
> +
> +       gtr_dev->serdes = dev_remap_addr_name(dev, "serdes");
> +       if (!gtr_dev->serdes)
> +               return -ENODEV;

-EINVAL if you have a problem with the DT. We use -ENODEV to mean
there is, in fact, no device, which clearly is not the case here.

> +
> +       gtr_dev->siou = dev_remap_addr_name(dev, "siou");
> +       if (!gtr_dev->siou)
> +               return -ENODEV;

Same here.

> +
> +       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,
> +       .of_xlate = xpsgtr_of_xlate,
> +       .power_on = xpsgtr_power_on,
> +};
> +
> +U_BOOT_DRIVER(psgtr_phy) = {
> +       .name = "psgtr_phy",
> +       .id = UCLASS_PHY,
> +       .of_match = xpsgtr_phy_ids,
> +       .ops = &xpsgtr_phy_ops,
> +       .probe = xpsgtr_probe,
> +       .priv_auto = sizeof(struct xpsgtr_dev),
> +};
> --
> 2.34.1
>

Regards,
Simon


More information about the U-Boot mailing list