Hi Kishon, >-----Original Message----- >From: Kishon Vijay Abraham I [mailto:kishon@xxxxxx] >Sent: Tuesday, October 09, 2018 1:18 PM >To: Anurag Kumar Vulisha <anuragku@xxxxxxxxxx>; robh+dt@xxxxxxxxxx; >mark.rutland@xxxxxxx; Michal Simek <michals@xxxxxxxxxx>; >vivek.gautam@xxxxxxxxxxxxxx >Cc: v.anuragkumar@xxxxxxxxx; Ajay Yugalkishore Pandey <APANDEY@xxxxxxxxxx>; >devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- >kernel@xxxxxxxxxxxxxxx >Subject: Re: [PATCH v4 2/2] phy: zynqmp: Add phy driver for xilinx zynqmp phy core > >Hi Anurag, > >On Wednesday 12 September 2018 09:52 PM, Anurag Kumar Vulisha wrote: >> ZynqMP SoC has a Gigabit Transceiver with four lanes. All the high speed >> peripherals such as USB, SATA, PCIE, Display Port and Ethernet SGMII can >> rely on any of the four GT lanes for PHY layer. This patch adds driver >> for that ZynqMP GT core. >> >> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xxxxxxxxxx> >> --- >> Changes in v4: >> 1. Moved include/dt-bindings/phy/phy.h into patch 1 as suggested by >> "Rob Herring" >> >> Changes in v3: >> 1. Corrected the Documentation as suggested by "Vivek Gautam" >> >> Changes in v2: >> 1. Fixed the compilation error when compiled phy-zynqmp.c as a module >> 2. Added CONFIG_PM macro in phy-zynqmp.c driver >> --- >> drivers/phy/Kconfig | 8 + >> drivers/phy/Makefile | 1 + >> drivers/phy/phy-zynqmp.c | 1581 >++++++++++++++++++++++++++++++++++++++++ >> include/linux/phy/phy-zynqmp.h | 52 ++ >> 4 files changed, 1642 insertions(+) >> create mode 100644 drivers/phy/phy-zynqmp.c >> create mode 100644 include/linux/phy/phy-zynqmp.h >> >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index 5c8d452..14cf3330 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig >> @@ -40,6 +40,14 @@ config PHY_XGENE >> help >> This option enables support for APM X-Gene SoC multi-purpose PHY. >> >> +config PHY_XILINX_ZYNQMP >> + tristate "Xilinx ZynqMP PHY driver" >> + depends on ARCH_ZYNQMP >> + select GENERIC_PHY >> + help >> + Enable this to support ZynqMP High Speed Gigabit Transceiver >> + that is part of ZynqMP SoC. >> + >> source "drivers/phy/allwinner/Kconfig" >> source "drivers/phy/amlogic/Kconfig" >> source "drivers/phy/broadcom/Kconfig" >> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile >> index 84e3bd9..f2a8d27 100644 >> --- a/drivers/phy/Makefile >> +++ b/drivers/phy/Makefile >> @@ -7,6 +7,7 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o >> obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o >> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o >> obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o >> +obj-$(CONFIG_PHY_XILINX_ZYNQMP) += phy-zynqmp.o >> obj-$(CONFIG_ARCH_SUNXI) += allwinner/ >> obj-$(CONFIG_ARCH_MESON) += amlogic/ >> obj-$(CONFIG_LANTIQ) += lantiq/ >> diff --git a/drivers/phy/phy-zynqmp.c b/drivers/phy/phy-zynqmp.c >> new file mode 100644 >> index 0000000..497862d >> --- /dev/null >> +++ b/drivers/phy/phy-zynqmp.c >> @@ -0,0 +1,1581 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * phy-zynqmp.c - PHY driver for Xilinx ZynqMP GT. >> + * >> + * Copyright (C) 2018 Xilinx Inc. >> + * >> + * Author: Anurag Kumar Vulisha <anuragku@xxxxxxxxxx> >> + * >> + * This driver is tested for USB, SATA and Display Port currently. >> + * Other controllers PCIe and SGMII should also work but that is >> + * experimental as of now. >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_platform.h> >> +#include <linux/of_address.h> >> +#include <linux/phy/phy.h> >> +#include <linux/phy/phy-zynqmp.h> >> +#include <linux/platform_device.h> >> +#include <linux/delay.h> >> +#include <dt-bindings/phy/phy.h> >> +#include <linux/reset.h> >> +#include <linux/list.h> >> +#include <linux/slab.h> >> + >> +/* 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 >> + >> +/* Refclk selection parameters */ >> +#define PLL_REF_SEL0 0x10000 >> +#define PLL_REF_OFFSET 0x4 >> +#define PLL_FREQ_MASK 0x1F >> +#define PLL_STATUS_READ_OFFSET 0x4000 >> +#define PLL_STATUS_LOCKED 0x10 >> + >> +/* PLL SSC step size offsets */ >> +#define L0_L0_REF_CLK_SEL 0x2860 >> +#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_OFFSET 0x4000 >> +#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_OFFSET 0x4000 >> +#define STEPS_0_MASK 0xFF >> +#define STEPS_1_MASK 0x07 >> + >> +/* BG calibration parameters */ >> +#define BGCAL_REF_SEL 0x10028 >> +#define BGCAL_REF_VALUE 0x0C >> + >> +/* 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 >> + >> +/* DN Resistor calibration code parameters */ >> +#define L0_TXPMA_ST_3 0x0B0C >> +#define L0_DN_CALIB_CODE 0x3F >> + >> +/* PLL Test Mode register parameters */ >> +#define L0_TM_PLL_DIG_37 0x2094 >> +#define L0_TM_PLL_DIG_37_OFFSET 0x4000 >> +#define L0_TM_COARSE_CODE_LIMIT 0x10 >> + >> +/* PCS control parameters */ >> +#define L0_TM_DIG_6 0x106C >> +#define L0_TX_DIG_61 0x00F4 >> +#define L0_TM_DIG_6_OFFSET 0x4000 >> +#define L0_TX_DIG_61_OFFSET 0x4000 >> +#define L0_TM_DIS_DESCRAMBLE_DECODER 0x0F >> +#define L0_TM_DISABLE_SCRAMBLE_ENCODER 0x0F >> + >> +/* TX De-emphasis parameters */ >> +#define L0_TX_ANA_TM_18 0x0048 >> +#define L0_TX_ANA_TM_118 0x01D8 >> +#define L0_TX_ANA_TM_18_OFFSET 0x4000 >> +#define L0_TX_ANA_TM_118_OFFSET 0x4000 >> +#define L0_TX_ANA_TM_118_FORCE_17_0 BIT(0) >> + >> +/* PMA control parameters */ >> +#define L0_TXPMD_TM_45 0x0CB4 >> +#define L0_TXPMD_TM_48 0x0CC0 >> +#define L0_TXPMD_TM_45_OFFSET 0x4000 >> +#define L0_TXPMD_TM_48_OFFSET 0x4000 >> +#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) >> + >> +/* 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_SHIFT 2 >> + >> +/* Max number of GT lanes */ >> +#define MAX_LANES 4 >> + >> +/* Max Allowed refclk frequencies */ >> +#define MAX_REFCLK 13 >> + >> +/* Lane CLK sharing mask */ >> +#define LANE_CLK_SHARE_MASK 0x8F >> + >> +/* SIOU SATA control register */ >> +#define SATA_CONTROL_OFFSET 0x0100 >> + >> +/* Total number of controllers */ >> +#define CONTROLLERS_PER_LANE 5 >> + >> +/* USB pipe control parameters */ >> +#define PIPE_CLK_OFFSET 0x7c >> +#define PIPE_POWER_OFFSET 0x80 >> +#define PIPE_CLK_ON 1 >> +#define PIPE_CLK_OFF 0 >> +#define PIPE_POWER_ON 1 >> +#define PIPE_POWER_OFF 0 >> + >> +/* Protocol Type Pparameters */ >> +#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 RST_TIMEOUT_MS 1000 >> +#define TIMEOUT_US 1000 >> + >> +/* >> + * This table holds the valid combinations of controllers and >> + * lanes(Interconnect Matrix). >> + */ >> +static unsigned int icm_matrix[MAX_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 } >> +}; >> + >> +/* Allowed PLL reference clock frequencies */ >> +enum pll_frequencies { >> + REF_19_2M = 0, >> + REF_20M, >> + REF_24M, >> + REF_26M, >> + REF_27M, >> + REF_38_4M, >> + REF_40M, >> + REF_52M, >> + REF_100M, >> + REF_108M, >> + REF_125M, >> + REF_135M, >> + REF_150M, >> +}; >> + >> +/** >> + * struct xpsgtr_phy - representation of a lane >> + * @phy: pointer to the kernel PHY device >> + * @type: controller which uses this lane >> + * @lane: lane number >> + * @protocol: protocol in which the lane operates >> + * @ref_clk: enum of allowed ref clock rates for this lane PLL >> + * @pll_lock: PLL status >> + * @skip_phy_init: skip phy_init() if true >> + * @data: pointer to hold private data >> + * @refclk_rate: PLL reference clock frequency >> + * @share_laneclk: lane number of the clock to be shared >> + */ >> +struct xpsgtr_phy { >> + struct phy *phy; >> + u8 type; >> + u8 lane; >> + u8 protocol; >> + enum pll_frequencies ref_clk; >> + bool pll_lock; >> + bool skip_phy_init; >> + void *data; >> + u32 refclk_rate; >> + u32 share_laneclk; >> +}; >> + >> +/** >> + * 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; >> +}; >> + >> +/* lookup table to hold all settings needed for a ref clock frequency */ >> +static struct xpsgtr_ssc ssc_lookup[MAX_REFCLK] = { >> + {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} >> +}; >> + >> +/** >> + * struct xpsgtr_dev - representation of a ZynMP GT device >> + * @dev: pointer to device >> + * @serdes: serdes base address >> + * @siou: siou base address >> + * @gtr_mutex: mutex for locking >> + * @phys: pointer to all the lanes >> + * @tx_term_fix: fix for GT issue >> + * @saved_icm_cfg0: stored value of ICM CFG0 register >> + * @saved_icm_cfg1: stored value of ICM CFG1 register >> + * @sata_rst: a reset control for SATA >> + * @dp_rst: a reset control for DP >> + * @usb0_crst: a reset control for usb0 core >> + * @usb1_crst: a reset control for usb1 core >> + * @usb0_hibrst: a reset control for usb0 hibernation module >> + * @usb1_hibrst: a reset control for usb1 hibernation module >> + * @usb0_apbrst: a reset control for usb0 apb bus >> + * @usb1_apbrst: a reset control for usb1 apb bus >> + * @gem0_rst: a reset control for gem0 >> + * @gem1_rst: a reset control for gem1 >> + * @gem2_rst: a reset control for gem2 >> + * @gem3_rst: a reset control for gem3 >> + */ >> +struct xpsgtr_dev { >> + struct device *dev; >> + void __iomem *serdes; >> + void __iomem *siou; >> + struct mutex gtr_mutex; /* mutex for locking */ >> + struct xpsgtr_phy **phys; >> + bool tx_term_fix; >> + unsigned int saved_icm_cfg0; >> + unsigned int saved_icm_cfg1; >> + struct reset_control *sata_rst; >> + struct reset_control *dp_rst; >> + struct reset_control *usb0_crst; >> + struct reset_control *usb1_crst; >> + struct reset_control *usb0_hibrst; >> + struct reset_control *usb1_hibrst; >> + struct reset_control *usb0_apbrst; >> + struct reset_control *usb1_apbrst; >> + struct reset_control *gem0_rst; >> + struct reset_control *gem1_rst; >> + struct reset_control *gem2_rst; >> + struct reset_control *gem3_rst; >> +}; >> + >> +/** >> + * xpsgtr_override_deemph - override PIPE TX de-emphasis >> + * @phy: pointer to phy >> + * @plvl: pre-emphasis level >> + * @vlvl: voltage swing level >> + * >> + * Return: None >> + */ >> +void xpsgtr_override_deemph(struct phy *phy, u8 plvl, u8 vlvl) >> +{ >> + struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy); >> + struct xpsgtr_dev *gtr_dev = gtr_phy->data; >> + static u8 pe[4][4] = { { 0x2, 0x2, 0x2, 0x2 }, >> + { 0x1, 0x1, 0x1, 0xFF }, >> + { 0x0, 0x0, 0xFF, 0xFF }, >> + { 0xFF, 0xFF, 0xFF, 0xFF } }; > >Here the consumer driver directly passes the index of a local array as >argument. How is the consumer driver supposed to this index. What happens if >the same consumer is connected to a different PHY? > Thanks a lot for spending your time in reviewing this patch. The consumer of this particular function is a Display Port and it is Xilinx specific IP. I am not very sure of the information if any other Display Port drivers need to handle this similar kind of runtime adjustment of the de-emphasis or voltage swing values, but the Xilinx DP has to do it at runtime as a part of training sequence. You may consider this as an implementation requirement. The Xilinx DP is expected to work only with this zynqmp phy and these plvl & vlvl values are predefined and there is an one to one mapping of these values between Xilinx DP driver and phy-zynqmp driver. These values are calculated by the DP driver based on the DPCD data read from sink and passed as an arguments to xpsgtr_override_deemph() function. Hope this answers your query. Please correct me, if I am missing something or my understanding is wrong Thanks, Anurag Kumar Vulisha
