Hi Michal, On Wed, 15 Dec 2021 at 02:58, Michal Simek <michal.si...@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.si...@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 <s...@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 <anura...@xilinx.com> > + * Author: Subbaraya Sundeep <sundeep.l...@gmail.com> > + * Author: Laurent Pinchart <laurent.pinch...@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 = >r_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
