linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver
@ 2020-03-11 10:32 Laurent Pinchart
  2020-03-11 10:32 ` [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY Laurent Pinchart
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-11 10:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kishon Vijay Abraham I, Anurag Kumar Vulisha, Michal Simek

Hello,

The patch series adds a PHY driver for the Xilinx ZynqMP gigabit serial
transceivers (PS-GTR). The PS-GTR is a set of 4 PHYs that can be used by
the PCIe, USB 3.0, DisplayPort, SATA and Ethernet controllers that are
part of the Serial I/O Unit (SIOU).

The code is based on a previous version sent by Anurag Kumar Vulisha and
available at [1]. The DT bindings have been converted to YAML, and both
the bindings and the driver have been considerably reworked (and
simplified). The most notable changes is the removal of manual handling
of the reset lines of the PHY users (which belongs to the PHY users
themselves), and moving to the standard PHY .power_on() and .configure()
operations to replace functions that were previously exported by the
driver. Please see individual patches for a more detailed changelog.

The code is based on v5.6-rc4 and has been tested with DisplayPort on
the Xilinx ZC106 board.

[1] https://patchwork.kernel.org/cover/10735681/

Anurag Kumar Vulisha (2):
  dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver

Laurent Pinchart (1):
  arm64: dts: zynqmp: Add GTR transceivers

 .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++
 MAINTAINERS                                   |   9 +
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |  10 +
 drivers/phy/Kconfig                           |   8 +
 drivers/phy/Makefile                          |   1 +
 drivers/phy/phy-zynqmp.c                      | 995 ++++++++++++++++++
 include/dt-bindings/phy/phy.h                 |   1 +
 7 files changed, 1128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
 create mode 100644 drivers/phy/phy-zynqmp.c

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-11 10:32 [PATCH v6 0/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
@ 2020-03-11 10:32 ` Laurent Pinchart
  2020-03-13 11:14   ` Kishon Vijay Abraham I
  2020-03-20  2:35   ` Rob Herring
  2020-03-11 10:32 ` [PATCH v6 2/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
  2020-03-11 10:32 ` [PATCH v6 3/3] arm64: dts: zynqmp: Add GTR transceivers Laurent Pinchart
  2 siblings, 2 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-11 10:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kishon Vijay Abraham I, Anurag Kumar Vulisha, Michal Simek, devicetree

From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>

Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
Processing System Gigabit Transceiver which provides PHY capabilities to
USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.

Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v5:

- Document clocks and clock-names properties
- Document resets and reset-names properties
- Replace subnodes with an additional entry in the PHY cells
- Drop lane frequency PHY cell, replaced by reference clock phandle
- Convert bindings to YAML
- Reword the subject line
- Drop Rob's R-b as the bindings have significantly changed
- Drop resets and reset-names properties
---
 .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
 include/dt-bindings/phy/phy.h                 |   1 +
 2 files changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml

diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
new file mode 100644
index 000000000000..9948e4a60e45
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+description: |
+  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
+  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
+  Ethernet SGMII controllers.
+
+properties:
+  "#phy-cells":
+    const: 4
+    description: |
+      The cells contain the following arguments.
+
+      - description: The GTR lane
+        minimum: 0
+        maximum: 3
+      - description: The PHY type
+        enum:
+          - PHY_TYPE_DP
+          - PHY_TYPE_PCIE
+          - PHY_TYPE_SATA
+          - PHY_TYPE_SGMII
+          - PHY_TYPE_USB
+      - description: The PHY instance
+        minimum: 0
+        maximum: 1 # for DP, SATA or USB
+        maximum: 3 # for PCIE or SGMII
+      - description: The reference clock number
+        minimum: 0
+        maximum: 3
+
+  compatible:
+    enum:
+      - xlnx,zynqmp-psgtr-v1.1
+      - xlnx,zynqmp-psgtr
+
+  clocks:
+    minItems: 1
+    maxItems: 4
+    description: |
+      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
+      inputs shall not have an entry.
+
+  clock-names:
+    minItems: 1
+    maxItems: 4
+    items:
+      pattern: "^ref[0-3]$"
+
+  reg:
+    items:
+      - description: SERDES registers block
+      - description: SIOU registers block
+
+  reg-names:
+    items:
+      - const: serdes
+      - const: siou
+
+required:
+  - "#phy-cells"
+  - compatible
+  - reg
+  - reg-names
+
+if:
+  properties:
+    compatible:
+      const: xlnx,zynqmp-psgtr
+
+then:
+  properties:
+    xlnx,tx-termination-fix:
+      description: |
+        Include this for fixing functional issue with the TX termination
+        resistance in GT, which can be out of spec for the XCZU9EG silicon
+        version.
+      type: boolean
+
+additionalProperties: false
+
+examples:
+  - |
+    phy: phy@fd400000 {
+      compatible = "xlnx,zynqmp-psgtr-v1.1";
+      reg = <0x0 0xfd400000 0x0 0x40000>,
+            <0x0 0xfd3d0000 0x0 0x1000>;
+      reg-names = "serdes", "siou";
+      clocks = <&refclks 3>, <&refclks 2>, <&refclks 0>;
+      clock-names = "ref1", "ref2", "ref3";
+      #phy-cells = <4>;
+      status = "okay";
+    };
+
+...
diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
index 1f3f866fae7b..f6bc83b66ae9 100644
--- a/include/dt-bindings/phy/phy.h
+++ b/include/dt-bindings/phy/phy.h
@@ -17,5 +17,6 @@
 #define PHY_TYPE_USB3		4
 #define PHY_TYPE_UFS		5
 #define PHY_TYPE_DP		6
+#define PHY_TYPE_SGMII		7
 
 #endif /* _DT_BINDINGS_PHY */
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver
  2020-03-11 10:32 [PATCH v6 0/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
  2020-03-11 10:32 ` [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY Laurent Pinchart
@ 2020-03-11 10:32 ` Laurent Pinchart
  2020-03-11 10:32 ` [PATCH v6 3/3] arm64: dts: zynqmp: Add GTR transceivers Laurent Pinchart
  2 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-11 10:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kishon Vijay Abraham I, Anurag Kumar Vulisha, Michal Simek

From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>

Xilinx ZynqMP SoCs have 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@xilinx.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v5:

- Cleanup headers
- Organize the code in sections
- Constify data tables and structures
- Allocate all PHY instances in one go
- Add I/O accessors
- Move DP-specific init to a separate function
- Use devm_platform_ioremap_resource_byname()
- Simplify acquisition of reset controllers
- Implement .configure() PHY operation for DP
- Implement .power_on() and .power_off() operations
- Wait for PLL lock for DP PHY too
- Remove USB core reset operations
- Fix SGMII bus width settings
- Update copyright notice and authors list
- Disable error messages on probe deferral
- Update reset names to new DT bindings
- Update to removal of subnodes in new DT bindings
- Handle reference clocks through CCF
- Add MAINTAINERS entry
- Drop reset handling
- Split TX term fix to separate function
- Remove unused registers
---
 MAINTAINERS              |   9 +
 drivers/phy/Kconfig      |   8 +
 drivers/phy/Makefile     |   1 +
 drivers/phy/phy-zynqmp.c | 995 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 1013 insertions(+)
 create mode 100644 drivers/phy/phy-zynqmp.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f2f0ca7df47f..fc6d8183106b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18403,6 +18403,15 @@ F:	Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
 F:	drivers/dma/xilinx/xilinx_dpdma.c
 F:	include/dt-bindings/dma/xlnx-zynqmp-dpdma.h
 
+XILINX ZYNQMP GSPTR PHY DRIVER
+M:	Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
+M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+L:	linux-kernel@vger.kernel.org
+T:	git https://github.com/Xilinx/linux-xlnx.git
+S:	Supported
+F:	Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
+F:	drivers/phy/phy-zynqmp.c
+
 XILLYBUS DRIVER
 M:	Eli Billauer <eli.billauer@gmail.com>
 L:	linux-kernel@vger.kernel.org
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index b3ed94b98d9b..b8251b9f3d87 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -49,6 +49,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 310c149a9df5..5dc7469f078b 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY)	+= phy-core-mipi-dphy.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_ARCH_MEDIATEK)		+= mediatek/
diff --git a/drivers/phy/phy-zynqmp.c b/drivers/phy/phy-zynqmp.c
new file mode 100644
index 000000000000..8ab99d6b9220
--- /dev/null
+++ b/drivers/phy/phy-zynqmp.c
@@ -0,0 +1,995 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-zynqmp.c - PHY driver for Xilinx ZynqMP GT.
+ *
+ * Copyright (C) 2018-20 Xilinx Inc.
+ *
+ * Author: Anurag Kumar Vulisha <anuragku@xilinx.com>
+ * Author: Subbaraya Sundeep <sundeep.lkml@gmail.com>
+ * Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * 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/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/phy/phy.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_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
+
+struct xpsgtr_dev;
+
+/**
+ * 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
+ * @phy: pointer to the kernel PHY device
+ * @type: controller which uses this lane
+ * @lane: lane number
+ * @protocol: protocol in which the lane operates
+ * @skip_phy_init: skip phy_init() if true
+ * @dev: pointer to the xpsgtr_dev instance
+ * @refclk: reference clock index
+ */
+struct xpsgtr_phy {
+	struct phy *phy;
+	u8 type;
+	u8 lane;
+	u8 protocol;
+	bool skip_phy_init;
+	struct xpsgtr_dev *dev;
+	unsigned int refclk;
+};
+
+/**
+ * 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: PHY lanes
+ * @refclk_sscs: spread spectrum settings for the reference clocks
+ * @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
+ */
+struct xpsgtr_dev {
+	struct device *dev;
+	void __iomem *serdes;
+	void __iomem *siou;
+	struct mutex gtr_mutex; /* mutex for locking */
+	struct xpsgtr_phy phys[NUM_LANES];
+	const struct xpsgtr_ssc *refclk_sscs[NUM_LANES];
+	bool tx_term_fix;
+	unsigned int saved_icm_cfg0;
+	unsigned int saved_icm_cfg1;
+};
+
+/* -----------------------------------------------------------------------------
+ * 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);
+}
+
+/* -----------------------------------------------------------------------------
+ * Hardware Configuration
+ */
+
+/* Wait for the PLL to lock (with a timeout). */
+static int xpsgtr_wait_pll_lock(struct phy *phy)
+{
+	struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
+	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
+	unsigned int timeout = TIMEOUT_US;
+	int ret;
+
+	dev_dbg(gtr_dev->dev, "Waiting for PLL lock\n");
+
+	while (1) {
+		u32 reg = xpsgtr_read_phy(gtr_phy, L0_PLL_STATUS_READ_1);
+
+		if ((reg & PLL_STATUS_LOCKED) == PLL_STATUS_LOCKED) {
+			ret = 0;
+			break;
+		}
+
+		if (--timeout == 0) {
+			ret = -ETIMEDOUT;
+			break;
+		}
+
+		udelay(1);
+	}
+
+	if (ret == -ETIMEDOUT)
+		dev_err(gtr_dev->dev,
+			"lane %u (type %u, protocol %u): PLL lock timeout\n",
+			gtr_phy->lane, gtr_phy->type, gtr_phy->protocol);
+
+	return ret;
+}
+
+/* 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);
+}
+
+/* DP-specific initialization. */
+static void xpsgtr_phy_init_dp(struct xpsgtr_phy *gtr_phy)
+{
+	xpsgtr_write_phy(gtr_phy, L0_TXPMD_TM_45,
+			 L0_TXPMD_TM_45_OVER_DP_MAIN |
+			 L0_TXPMD_TM_45_ENABLE_DP_MAIN |
+			 L0_TXPMD_TM_45_OVER_DP_POST1 |
+			 L0_TXPMD_TM_45_OVER_DP_POST2 |
+			 L0_TXPMD_TM_45_ENABLE_DP_POST2);
+	xpsgtr_write_phy(gtr_phy, L0_TX_ANA_TM_118,
+			 L0_TX_ANA_TM_118_FORCE_17_0);
+}
+
+/* SATA-specific initialization. */
+static void xpsgtr_phy_init_sata(struct xpsgtr_phy *gtr_phy)
+{
+	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
+
+	xpsgtr_bypass_scrambler_8b10b(gtr_phy);
+
+	writel(gtr_phy->lane, gtr_dev->siou + SATA_CONTROL_OFFSET);
+}
+
+/* SGMII-specific initialization. */
+static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy *gtr_phy)
+{
+	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
+
+	/* Set SGMII protocol TX and RX bus width to 10 bits. */
+	xpsgtr_write(gtr_dev, TX_PROT_BUS_WIDTH,
+		     PROT_BUS_WIDTH_10 << (gtr_phy->lane * PROT_BUS_WIDTH_SHIFT));
+	xpsgtr_write(gtr_dev, RX_PROT_BUS_WIDTH,
+		     PROT_BUS_WIDTH_10 << (gtr_phy->lane * PROT_BUS_WIDTH_SHIFT));
+
+	xpsgtr_bypass_scrambler_8b10b(gtr_phy);
+}
+
+/* Configure TX de-emphasis and margining for DP. */
+static void xpsgtr_phy_configure_dp(struct xpsgtr_phy *gtr_phy, unsigned int pre,
+				    unsigned int voltage)
+{
+	static const u8 voltage_swing[4][4] = {
+		{ 0x2a, 0x27, 0x24, 0x20 },
+		{ 0x27, 0x23, 0x20, 0xff },
+		{ 0x24, 0x20, 0xff, 0xff },
+		{ 0xff, 0xff, 0xff, 0xff }
+	};
+	static const u8 pre_emphasis[4][4] = {
+		{ 0x02, 0x02, 0x02, 0x02 },
+		{ 0x01, 0x01, 0x01, 0xff },
+		{ 0x00, 0x00, 0xff, 0xff },
+		{ 0xff, 0xff, 0xff, 0xff }
+	};
+
+	xpsgtr_write_phy(gtr_phy, L0_TXPMD_TM_48, voltage_swing[pre][voltage]);
+	xpsgtr_write_phy(gtr_phy, L0_TX_ANA_TM_18, pre_emphasis[pre][voltage]);
+}
+
+/* -----------------------------------------------------------------------------
+ * PHY Operations
+ */
+
+static bool xpsgtr_phy_init_required(struct xpsgtr_phy *gtr_phy)
+{
+	/*
+	 * As USB may save the snapshot of the states during hibernation, doing
+	 * phy_init() will put the USB controller into reset, resulting in the
+	 * losing of the saved snapshot. So try to avoid phy_init() for USB
+	 * except when gtr_phy->skip_phy_init is false (this happens when FPD is
+	 * shutdown during suspend or when gt lane is changed from current one)
+	 */
+	if (gtr_phy->protocol == ICM_PROTOCOL_USB && gtr_phy->skip_phy_init)
+		return false;
+	else
+		return true;
+}
+
+/*
+ * There is a functional issue in the GT. The TX termination resistance can be
+ * out of spec due to a issue in the calibration logic. This is the workaround
+ * to fix it, required for XCZU9EG silicon.
+ */
+static int xpsgtr_phy_tx_term_fix(struct xpsgtr_phy *gtr_phy)
+{
+	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
+	u32 timeout = TIMEOUT_US;
+	u32 nsw;
+
+	/* Enabling Test Mode control for CMN Rest */
+	xpsgtr_clr_set(gtr_dev, TM_CMN_RST, TM_CMN_RST_MASK, TM_CMN_RST_SET);
+
+	/* Set Test Mode reset */
+	xpsgtr_clr_set(gtr_dev, TM_CMN_RST, TM_CMN_RST_MASK, TM_CMN_RST_EN);
+
+	xpsgtr_write(gtr_dev, L3_TM_CALIB_DIG18, 0x00);
+	xpsgtr_write(gtr_dev, L3_TM_CALIB_DIG19, L3_TM_OVERRIDE_NSW_CODE);
+
+	/*
+	 * As a part of work around sequence for PMOS calibration fix,
+	 * we need to configure any lane ICM_CFG to valid protocol. This
+	 * will deassert the CMN_Resetn signal.
+	 */
+	xpsgtr_lane_set_protocol(gtr_phy);
+
+	/* Clear Test Mode reset */
+	xpsgtr_clr_set(gtr_dev, TM_CMN_RST, TM_CMN_RST_MASK, TM_CMN_RST_SET);
+
+	dev_dbg(gtr_dev->dev, "calibrating...\n");
+
+	do {
+		u32 reg = xpsgtr_read(gtr_dev, L3_CALIB_DONE_STATUS);
+
+		if ((reg & L3_CALIB_DONE) == L3_CALIB_DONE)
+			break;
+
+		if (!--timeout) {
+			dev_err(gtr_dev->dev, "calibration time out\n");
+			return -ETIMEDOUT;
+		}
+
+		udelay(1);
+	} while (timeout > 0);
+
+	dev_dbg(gtr_dev->dev, "calibration done\n");
+
+	/* Reading NMOS Register Code */
+	nsw = xpsgtr_read(gtr_dev, L0_TXPMA_ST_3) & L0_DN_CALIB_CODE;
+
+	/* Set Test Mode reset */
+	xpsgtr_clr_set(gtr_dev, TM_CMN_RST, TM_CMN_RST_MASK, TM_CMN_RST_EN);
+
+	/* Writing NMOS register values back [5:3] */
+	xpsgtr_write(gtr_dev, L3_TM_CALIB_DIG19, nsw >> L3_NSW_CALIB_SHIFT);
+
+	/* Writing NMOS register value [2:0] */
+	xpsgtr_write(gtr_dev, L3_TM_CALIB_DIG18,
+		     ((nsw & L3_TM_CALIB_DIG19_NSW) << L3_NSW_SHIFT) |
+		     (1 << L3_NSW_PIPE_SHIFT));
+
+	/* Clear Test Mode reset */
+	xpsgtr_clr_set(gtr_dev, TM_CMN_RST, TM_CMN_RST_MASK, TM_CMN_RST_SET);
+
+	return 0;
+}
+
+static int xpsgtr_phy_init(struct phy *phy)
+{
+	struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
+	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
+	int ret = 0;
+
+	mutex_lock(&gtr_dev->gtr_mutex);
+
+	/* Skip initialization if not required. */
+	if (!xpsgtr_phy_init_required(gtr_phy))
+		goto out;
+
+	if (gtr_dev->tx_term_fix) {
+		ret = xpsgtr_phy_tx_term_fix(gtr_phy);
+		if (ret < 0)
+			goto out;
+
+		gtr_dev->tx_term_fix = false;
+	}
+
+	/* 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_DP:
+		xpsgtr_phy_init_dp(gtr_phy);
+		break;
+
+	case ICM_PROTOCOL_SATA:
+		xpsgtr_phy_init_sata(gtr_phy);
+		break;
+
+	case ICM_PROTOCOL_SGMII:
+		xpsgtr_phy_init_sgmii(gtr_phy);
+		break;
+	}
+
+out:
+	mutex_unlock(&gtr_dev->gtr_mutex);
+	return ret;
+}
+
+static int xpsgtr_phy_exit(struct phy *phy)
+{
+	struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
+
+	gtr_phy->skip_phy_init = false;
+
+	return 0;
+}
+
+static int xpsgtr_phy_power_on(struct phy *phy)
+{
+	struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
+	int ret = 0;
+
+	/*
+	 * Wait for the PLL to lock. For DP, only wait on DP0 to avoid
+	 * cumulating waits for both lanes. The user is expected to initialize
+	 * lane 0 last.
+	 */
+	if (gtr_phy->protocol != ICM_PROTOCOL_DP ||
+	    gtr_phy->type == XPSGTR_TYPE_DP_0)
+		ret = xpsgtr_wait_pll_lock(phy);
+
+	return ret;
+}
+
+static int xpsgtr_phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
+
+	if (gtr_phy->protocol != ICM_PROTOCOL_DP)
+		return 0;
+
+	xpsgtr_phy_configure_dp(gtr_phy, opts->dp.pre[0], opts->dp.voltage[0]);
+
+	return 0;
+}
+
+static const struct phy_ops xpsgtr_phyops = {
+	.init		= xpsgtr_phy_init,
+	.exit		= xpsgtr_phy_exit,
+	.power_on	= xpsgtr_phy_power_on,
+	.configure	= xpsgtr_phy_configure,
+	.owner		= THIS_MODULE,
+};
+
+/* -----------------------------------------------------------------------------
+ * 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 struct phy *xpsgtr_xlate(struct device *dev,
+				struct of_phandle_args *args)
+{
+	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
+	struct xpsgtr_phy *gtr_phy;
+	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 ERR_PTR(-EINVAL);
+	}
+
+	/*
+	 * Get the PHY parameters from the OF arguments and derive the lane
+	 * type.
+	 */
+	phy_lane = args->args[0];
+	if (phy_lane >= ARRAY_SIZE(gtr_dev->phys)) {
+		dev_err(dev, "Invalid lane number %u\n", phy_lane);
+		return ERR_PTR(-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(gtr_dev->dev, "Invalid PHY type and/or instance\n");
+		return ERR_PTR(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 ERR_PTR(-EINVAL);
+	}
+
+	gtr_phy->refclk = refclk;
+
+	/*
+	 * 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)
+			return gtr_phy->phy;
+	}
+
+	return ERR_PTR(-EINVAL);
+}
+
+/* -----------------------------------------------------------------------------
+ * Power Management
+ */
+
+#ifdef CONFIG_PM
+static int xpsgtr_suspend(struct device *dev)
+{
+	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
+
+	/* Save the snapshot ICM_CFG registers. */
+	gtr_dev->saved_icm_cfg0 = xpsgtr_read(gtr_dev, ICM_CFG0);
+	gtr_dev->saved_icm_cfg1 = xpsgtr_read(gtr_dev, ICM_CFG1);
+
+	return 0;
+}
+
+static int xpsgtr_resume(struct device *dev)
+{
+	struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
+	unsigned int icm_cfg0, icm_cfg1;
+	unsigned int i;
+	bool skip_phy_init;
+
+	icm_cfg0 = xpsgtr_read(gtr_dev, ICM_CFG0);
+	icm_cfg1 = xpsgtr_read(gtr_dev, ICM_CFG1);
+
+	/* Return if no GT lanes got configured before suspend. */
+	if (!gtr_dev->saved_icm_cfg0 && !gtr_dev->saved_icm_cfg1)
+		return 0;
+
+	/* Check if the ICM configurations changed after suspend. */
+	if (icm_cfg0 == gtr_dev->saved_icm_cfg0 &&
+	    icm_cfg1 == gtr_dev->saved_icm_cfg1)
+		skip_phy_init = true;
+	else
+		skip_phy_init = false;
+
+	/* Update the skip_phy_init for all gtr_phy instances. */
+	for (i = 0; i < ARRAY_SIZE(gtr_dev->phys); i++)
+		gtr_dev->phys[i].skip_phy_init = skip_phy_init;
+
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops xpsgtr_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(xpsgtr_suspend, xpsgtr_resume)
+};
+
+/* -----------------------------------------------------------------------------
+ * Probe & Platform Driver
+ */
+
+static int xpsgtr_get_ref_clocks(struct xpsgtr_dev *gtr_dev)
+{
+	unsigned int refclk;
+
+	for (refclk = 0; refclk < ARRAY_SIZE(gtr_dev->refclk_sscs); ++refclk) {
+		unsigned long rate;
+		unsigned int i;
+		struct clk *clk;
+		char name[8];
+
+		snprintf(name, sizeof(name), "ref%u", refclk);
+		clk = devm_clk_get_optional(gtr_dev->dev, name);
+		if (IS_ERR(clk)) {
+			if (PTR_ERR(clk) != -EPROBE_DEFER)
+				dev_err(gtr_dev->dev,
+					"Failed to get reference clock %u: %ld\n",
+					refclk, PTR_ERR(clk));
+			return PTR_ERR(clk);
+		}
+
+		if (!clk)
+			continue;
+
+		/*
+		 * Get the spread spectrum (SSC) settings for the reference
+		 * clock rate.
+		 */
+		rate = clk_get_rate(clk);
+
+		for (i = 0 ; i < ARRAY_SIZE(ssc_lookup); i++) {
+			if (rate == ssc_lookup[i].refclk_rate) {
+				gtr_dev->refclk_sscs[refclk] = &ssc_lookup[i];
+				break;
+			}
+		}
+
+		if (i == ARRAY_SIZE(ssc_lookup)) {
+			dev_err(gtr_dev->dev,
+				"Invalid rate %lu for reference clock %u\n",
+				rate, refclk);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int xpsgtr_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct xpsgtr_dev *gtr_dev;
+	struct phy_provider *provider;
+	unsigned int port;
+	int ret;
+
+	gtr_dev = devm_kzalloc(&pdev->dev, sizeof(*gtr_dev), GFP_KERNEL);
+	if (!gtr_dev)
+		return -ENOMEM;
+
+	gtr_dev->dev = &pdev->dev;
+	platform_set_drvdata(pdev, gtr_dev);
+
+	mutex_init(&gtr_dev->gtr_mutex);
+
+	if (of_device_is_compatible(np, "xlnx,zynqmp-psgtr"))
+		gtr_dev->tx_term_fix =
+			of_property_read_bool(np, "xlnx,tx-termination-fix");
+
+	/* Acquire resources. */
+	gtr_dev->serdes = devm_platform_ioremap_resource_byname(pdev, "serdes");
+	if (IS_ERR(gtr_dev->serdes))
+		return PTR_ERR(gtr_dev->serdes);
+
+	gtr_dev->siou = devm_platform_ioremap_resource_byname(pdev, "siou");
+	if (IS_ERR(gtr_dev->siou))
+		return PTR_ERR(gtr_dev->siou);
+
+	ret = xpsgtr_get_ref_clocks(gtr_dev);
+	if (ret)
+		return ret;
+
+	/* Create PHYs. */
+	for (port = 0; port < ARRAY_SIZE(gtr_dev->phys); ++port) {
+		struct xpsgtr_phy *gtr_phy = &gtr_dev->phys[port];
+		struct phy *phy;
+
+		gtr_phy->lane = port;
+		gtr_phy->dev = gtr_dev;
+
+		phy = devm_phy_create(&pdev->dev, np, &xpsgtr_phyops);
+		if (IS_ERR(phy)) {
+			dev_err(&pdev->dev, "failed to create PHY\n");
+			return PTR_ERR(phy);
+		}
+
+		gtr_phy->phy = phy;
+		phy_set_drvdata(phy, gtr_phy);
+	}
+
+	/* Register the PHY provider. */
+	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(&pdev->dev, "registering provider failed\n");
+			return PTR_ERR(provider);
+	}
+	return 0;
+}
+
+static const struct of_device_id xpsgtr_of_match[] = {
+	{ .compatible = "xlnx,zynqmp-psgtr", },
+	{ .compatible = "xlnx,zynqmp-psgtr-v1.1", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xpsgtr_of_match);
+
+static struct platform_driver xpsgtr_driver = {
+	.probe = xpsgtr_probe,
+	.driver = {
+		.name = "xilinx-psgtr",
+		.of_match_table	= xpsgtr_of_match,
+		.pm =  &xpsgtr_pm_ops,
+	},
+};
+
+module_platform_driver(xpsgtr_driver);
+
+MODULE_AUTHOR("Xilinx Inc.");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Xilinx ZynqMP High speed Gigabit Transceiver");
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 3/3] arm64: dts: zynqmp: Add GTR transceivers
  2020-03-11 10:32 [PATCH v6 0/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
  2020-03-11 10:32 ` [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY Laurent Pinchart
  2020-03-11 10:32 ` [PATCH v6 2/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
@ 2020-03-11 10:32 ` Laurent Pinchart
  2020-03-13  6:54   ` Michal Simek
  2 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-11 10:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kishon Vijay Abraham I, Anurag Kumar Vulisha, Michal Simek

Add a DT node for the PS-GTR transceivers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 2e284eb8d3c1..5e06e4c19d94 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -13,6 +13,7 @@
  */
 
 #include <dt-bindings/power/xlnx-zynqmp-power.h>
+#include <dt-bindings/reset/xlnx-zynqmp-resets.h>
 
 / {
 	compatible = "xlnx,zynqmp";
@@ -564,6 +565,15 @@ pcie_intc: legacy-interrupt-controller {
 			};
 		};
 
+		psgtr: phy@fd400000 {
+			compatible = "xlnx,zynqmp-psgtr-v1.1";
+			status = "disabled";
+			reg = <0x0 0xfd400000 0x0 0x40000>,
+			      <0x0 0xfd3d0000 0x0 0x1000>;
+			reg-names = "serdes", "siou";
+			#phy-cells = <4>;
+		};
+
 		rtc: rtc@ffa60000 {
 			compatible = "xlnx,zynqmp-rtc";
 			status = "disabled";
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] arm64: dts: zynqmp: Add GTR transceivers
  2020-03-11 10:32 ` [PATCH v6 3/3] arm64: dts: zynqmp: Add GTR transceivers Laurent Pinchart
@ 2020-03-13  6:54   ` Michal Simek
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2020-03-13  6:54 UTC (permalink / raw)
  To: Laurent Pinchart, linux-kernel
  Cc: Kishon Vijay Abraham I, Anurag Kumar Vulisha, Michal Simek

On 11. 03. 20 11:32, Laurent Pinchart wrote:
> Add a DT node for the PS-GTR transceivers.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 2e284eb8d3c1..5e06e4c19d94 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <dt-bindings/power/xlnx-zynqmp-power.h>
> +#include <dt-bindings/reset/xlnx-zynqmp-resets.h>
>  
>  / {
>  	compatible = "xlnx,zynqmp";
> @@ -564,6 +565,15 @@ pcie_intc: legacy-interrupt-controller {
>  			};
>  		};
>  
> +		psgtr: phy@fd400000 {
> +			compatible = "xlnx,zynqmp-psgtr-v1.1";
> +			status = "disabled";
> +			reg = <0x0 0xfd400000 0x0 0x40000>,
> +			      <0x0 0xfd3d0000 0x0 0x1000>;
> +			reg-names = "serdes", "siou";
> +			#phy-cells = <4>;
> +		};
> +
>  		rtc: rtc@ffa60000 {
>  			compatible = "xlnx,zynqmp-rtc";
>  			status = "disabled";
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-11 10:32 ` [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY Laurent Pinchart
@ 2020-03-13 11:14   ` Kishon Vijay Abraham I
  2020-03-18 15:05     ` Laurent Pinchart
  2020-03-20  2:35   ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-13 11:14 UTC (permalink / raw)
  To: Laurent Pinchart, linux-kernel, Rob Herring
  Cc: Anurag Kumar Vulisha, Michal Simek, devicetree

+Rob

On 11/03/20 4:02 pm, Laurent Pinchart wrote:
> From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> 
> Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
> Processing System Gigabit Transceiver which provides PHY capabilities to
> USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.
> 
> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v5:
> 
> - Document clocks and clock-names properties
> - Document resets and reset-names properties
> - Replace subnodes with an additional entry in the PHY cells
> - Drop lane frequency PHY cell, replaced by reference clock phandle
> - Convert bindings to YAML
> - Reword the subject line
> - Drop Rob's R-b as the bindings have significantly changed
> - Drop resets and reset-names properties
> ---
>  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
>  include/dt-bindings/phy/phy.h                 |   1 +
>  2 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> new file mode 100644
> index 000000000000..9948e4a60e45
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |
> +  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
> +  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
> +  Ethernet SGMII controllers.
> +
> +properties:
> +  "#phy-cells":
> +    const: 4
> +    description: |
> +      The cells contain the following arguments.
> +
> +      - description: The GTR lane
> +        minimum: 0
> +        maximum: 3
> +      - description: The PHY type
> +        enum:
> +          - PHY_TYPE_DP
> +          - PHY_TYPE_PCIE
> +          - PHY_TYPE_SATA
> +          - PHY_TYPE_SGMII
> +          - PHY_TYPE_USB
> +      - description: The PHY instance
> +        minimum: 0
> +        maximum: 1 # for DP, SATA or USB
> +        maximum: 3 # for PCIE or SGMII
> +      - description: The reference clock number
> +        minimum: 0
> +        maximum: 3
> +
> +  compatible:
> +    enum:
> +      - xlnx,zynqmp-psgtr-v1.1
> +      - xlnx,zynqmp-psgtr
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
> +      inputs shall not have an entry.
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      pattern: "^ref[0-3]$"
> +
> +  reg:
> +    items:
> +      - description: SERDES registers block
> +      - description: SIOU registers block
> +
> +  reg-names:
> +    items:
> +      - const: serdes
> +      - const: siou
> +
> +required:
> +  - "#phy-cells"
> +  - compatible
> +  - reg
> +  - reg-names
> +
> +if:
> +  properties:
> +    compatible:
> +      const: xlnx,zynqmp-psgtr
> +
> +then:
> +  properties:
> +    xlnx,tx-termination-fix:
> +      description: |
> +        Include this for fixing functional issue with the TX termination
> +        resistance in GT, which can be out of spec for the XCZU9EG silicon
> +        version.
> +      type: boolean
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    phy: phy@fd400000 {
> +      compatible = "xlnx,zynqmp-psgtr-v1.1";
> +      reg = <0x0 0xfd400000 0x0 0x40000>,
> +            <0x0 0xfd3d0000 0x0 0x1000>;
> +      reg-names = "serdes", "siou";
> +      clocks = <&refclks 3>, <&refclks 2>, <&refclks 0>;
> +      clock-names = "ref1", "ref2", "ref3";
> +      #phy-cells = <4>;
> +      status = "okay";
> +    };
> +
> +...
> diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
> index 1f3f866fae7b..f6bc83b66ae9 100644
> --- a/include/dt-bindings/phy/phy.h
> +++ b/include/dt-bindings/phy/phy.h
> @@ -17,5 +17,6 @@
>  #define PHY_TYPE_USB3		4
>  #define PHY_TYPE_UFS		5
>  #define PHY_TYPE_DP		6
> +#define PHY_TYPE_SGMII		7
>  
>  #endif /* _DT_BINDINGS_PHY */
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-13 11:14   ` Kishon Vijay Abraham I
@ 2020-03-18 15:05     ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-18 15:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-kernel, Rob Herring, Anurag Kumar Vulisha, Michal Simek,
	devicetree

Hi Kishon,

On Fri, Mar 13, 2020 at 04:44:04PM +0530, Kishon Vijay Abraham I wrote:
> +Rob

Any comment regarding patch 2/3 ? :-) You mentioned in your review of v5
that the exported symbols were a no-go, and that is now fixed. The
driver uses the PHY .configure() API to configure DisplayPort
parameters, .power_on() now waits for the PHY PLL to lock, and the
USB-specific exported symbols were removed with reset support being
moved to the PHY consumers (there's no reason for the PHY driver to
reset the PHY consumers, that's a layering violation).

> On 11/03/20 4:02 pm, Laurent Pinchart wrote:
> > From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > 
> > Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
> > Processing System Gigabit Transceiver which provides PHY capabilities to
> > USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.
> > 
> > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v5:
> > 
> > - Document clocks and clock-names properties
> > - Document resets and reset-names properties
> > - Replace subnodes with an additional entry in the PHY cells
> > - Drop lane frequency PHY cell, replaced by reference clock phandle
> > - Convert bindings to YAML
> > - Reword the subject line
> > - Drop Rob's R-b as the bindings have significantly changed
> > - Drop resets and reset-names properties
> > ---
> >  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
> >  include/dt-bindings/phy/phy.h                 |   1 +
> >  2 files changed, 105 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > new file mode 100644
> > index 000000000000..9948e4a60e45
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > +
> > +description: |
> > +  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
> > +  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
> > +  Ethernet SGMII controllers.
> > +
> > +properties:
> > +  "#phy-cells":
> > +    const: 4
> > +    description: |
> > +      The cells contain the following arguments.
> > +
> > +      - description: The GTR lane
> > +        minimum: 0
> > +        maximum: 3
> > +      - description: The PHY type
> > +        enum:
> > +          - PHY_TYPE_DP
> > +          - PHY_TYPE_PCIE
> > +          - PHY_TYPE_SATA
> > +          - PHY_TYPE_SGMII
> > +          - PHY_TYPE_USB
> > +      - description: The PHY instance
> > +        minimum: 0
> > +        maximum: 1 # for DP, SATA or USB
> > +        maximum: 3 # for PCIE or SGMII
> > +      - description: The reference clock number
> > +        minimum: 0
> > +        maximum: 3
> > +
> > +  compatible:
> > +    enum:
> > +      - xlnx,zynqmp-psgtr-v1.1
> > +      - xlnx,zynqmp-psgtr
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 4
> > +    description: |
> > +      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
> > +      inputs shall not have an entry.
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 4
> > +    items:
> > +      pattern: "^ref[0-3]$"
> > +
> > +  reg:
> > +    items:
> > +      - description: SERDES registers block
> > +      - description: SIOU registers block
> > +
> > +  reg-names:
> > +    items:
> > +      - const: serdes
> > +      - const: siou
> > +
> > +required:
> > +  - "#phy-cells"
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +
> > +if:
> > +  properties:
> > +    compatible:
> > +      const: xlnx,zynqmp-psgtr
> > +
> > +then:
> > +  properties:
> > +    xlnx,tx-termination-fix:
> > +      description: |
> > +        Include this for fixing functional issue with the TX termination
> > +        resistance in GT, which can be out of spec for the XCZU9EG silicon
> > +        version.
> > +      type: boolean
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    phy: phy@fd400000 {
> > +      compatible = "xlnx,zynqmp-psgtr-v1.1";
> > +      reg = <0x0 0xfd400000 0x0 0x40000>,
> > +            <0x0 0xfd3d0000 0x0 0x1000>;
> > +      reg-names = "serdes", "siou";
> > +      clocks = <&refclks 3>, <&refclks 2>, <&refclks 0>;
> > +      clock-names = "ref1", "ref2", "ref3";
> > +      #phy-cells = <4>;
> > +      status = "okay";
> > +    };
> > +
> > +...
> > diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
> > index 1f3f866fae7b..f6bc83b66ae9 100644
> > --- a/include/dt-bindings/phy/phy.h
> > +++ b/include/dt-bindings/phy/phy.h
> > @@ -17,5 +17,6 @@
> >  #define PHY_TYPE_USB3		4
> >  #define PHY_TYPE_UFS		5
> >  #define PHY_TYPE_DP		6
> > +#define PHY_TYPE_SGMII		7
> >  
> >  #endif /* _DT_BINDINGS_PHY */
> > 

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-11 10:32 ` [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY Laurent Pinchart
  2020-03-13 11:14   ` Kishon Vijay Abraham I
@ 2020-03-20  2:35   ` Rob Herring
  2020-03-20  9:50     ` Laurent Pinchart
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-03-20  2:35 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-kernel, Kishon Vijay Abraham I, Anurag Kumar Vulisha,
	Michal Simek, devicetree

On Wed, Mar 11, 2020 at 12:32:50PM +0200, Laurent Pinchart wrote:
> From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> 
> Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
> Processing System Gigabit Transceiver which provides PHY capabilities to
> USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.
> 
> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v5:
> 
> - Document clocks and clock-names properties
> - Document resets and reset-names properties
> - Replace subnodes with an additional entry in the PHY cells
> - Drop lane frequency PHY cell, replaced by reference clock phandle
> - Convert bindings to YAML
> - Reword the subject line
> - Drop Rob's R-b as the bindings have significantly changed
> - Drop resets and reset-names properties
> ---
>  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
>  include/dt-bindings/phy/phy.h                 |   1 +
>  2 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> new file mode 100644
> index 000000000000..9948e4a60e45
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: GPL-2.0

For new bindings:

(GPL-2.0-only OR BSD-2-Clause)

Though I guess Anurag needs to agree.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |
> +  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
> +  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
> +  Ethernet SGMII controllers.
> +
> +properties:
> +  "#phy-cells":
> +    const: 4
> +    description: |
> +      The cells contain the following arguments.
> +
> +      - description: The GTR lane
> +        minimum: 0
> +        maximum: 3
> +      - description: The PHY type
> +        enum:
> +          - PHY_TYPE_DP
> +          - PHY_TYPE_PCIE
> +          - PHY_TYPE_SATA
> +          - PHY_TYPE_SGMII
> +          - PHY_TYPE_USB
> +      - description: The PHY instance
> +        minimum: 0
> +        maximum: 1 # for DP, SATA or USB
> +        maximum: 3 # for PCIE or SGMII
> +      - description: The reference clock number
> +        minimum: 0
> +        maximum: 3

Humm, interesting almost json-schema. I guess it's fine as-is.

I would like to figure out how to apply a schema like this to the 
consumer nodes. We'd have to look up the phandle, get that node's 
compatible, find the provider's schema, find #.*-cells property, and 
extract a schema from it. Actually, doesn't sound too hard.

> +
> +  compatible:
> +    enum:
> +      - xlnx,zynqmp-psgtr-v1.1
> +      - xlnx,zynqmp-psgtr
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
> +      inputs shall not have an entry.
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      pattern: "^ref[0-3]$"
> +
> +  reg:
> +    items:
> +      - description: SERDES registers block
> +      - description: SIOU registers block
> +
> +  reg-names:
> +    items:
> +      - const: serdes
> +      - const: siou
> +
> +required:
> +  - "#phy-cells"
> +  - compatible
> +  - reg
> +  - reg-names
> +
> +if:
> +  properties:
> +    compatible:
> +      const: xlnx,zynqmp-psgtr
> +
> +then:
> +  properties:
> +    xlnx,tx-termination-fix:
> +      description: |
> +        Include this for fixing functional issue with the TX termination
> +        resistance in GT, which can be out of spec for the XCZU9EG silicon
> +        version.
> +      type: boolean
> +
> +additionalProperties: false

This won't work with 'xlnx,tx-termination-fix'. You need to move it to 
the main properties section and then do:

if:
  properties:
    compatible:
      const: xlnx,zynqmp-psgtr-v1.1

then:
  properties:
    xlnx,tx-termination-fix: false

I think this would also work:

  not:
    required:
      - xlnx,tx-termination-fix
> +
> +examples:
> +  - |
> +    phy: phy@fd400000 {
> +      compatible = "xlnx,zynqmp-psgtr-v1.1";
> +      reg = <0x0 0xfd400000 0x0 0x40000>,
> +            <0x0 0xfd3d0000 0x0 0x1000>;
> +      reg-names = "serdes", "siou";
> +      clocks = <&refclks 3>, <&refclks 2>, <&refclks 0>;
> +      clock-names = "ref1", "ref2", "ref3";
> +      #phy-cells = <4>;
> +      status = "okay";

Drop status in examples.

> +    };
> +
> +...
> diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
> index 1f3f866fae7b..f6bc83b66ae9 100644
> --- a/include/dt-bindings/phy/phy.h
> +++ b/include/dt-bindings/phy/phy.h
> @@ -17,5 +17,6 @@
>  #define PHY_TYPE_USB3		4
>  #define PHY_TYPE_UFS		5
>  #define PHY_TYPE_DP		6
> +#define PHY_TYPE_SGMII		7
>  
>  #endif /* _DT_BINDINGS_PHY */
> -- 
> Regards,
> 
> Laurent Pinchart
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-20  2:35   ` Rob Herring
@ 2020-03-20  9:50     ` Laurent Pinchart
  2020-03-20  9:55       ` [PATCH v6.1 " Laurent Pinchart
  2020-03-20 16:53       ` [PATCH v6 " Rob Herring
  0 siblings, 2 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-20  9:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, Kishon Vijay Abraham I, Anurag Kumar Vulisha,
	Michal Simek, devicetree

Hi Rob,

On Thu, Mar 19, 2020 at 08:35:20PM -0600, Rob Herring wrote:
> On Wed, Mar 11, 2020 at 12:32:50PM +0200, Laurent Pinchart wrote:
> > From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > 
> > Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
> > Processing System Gigabit Transceiver which provides PHY capabilities to
> > USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.
> > 
> > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v5:
> > 
> > - Document clocks and clock-names properties
> > - Document resets and reset-names properties
> > - Replace subnodes with an additional entry in the PHY cells
> > - Drop lane frequency PHY cell, replaced by reference clock phandle
> > - Convert bindings to YAML
> > - Reword the subject line
> > - Drop Rob's R-b as the bindings have significantly changed
> > - Drop resets and reset-names properties
> > ---
> >  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
> >  include/dt-bindings/phy/phy.h                 |   1 +
> >  2 files changed, 105 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > new file mode 100644
> > index 000000000000..9948e4a60e45
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> For new bindings:
> 
> (GPL-2.0-only OR BSD-2-Clause)
> 
> Though I guess Anurag needs to agree.

There's an ongoing similar discussion regarding the DPSUB (DRM/KMS)
bindings. Hyun is checking with the Xilinx legal department. If they
agree, I'll change the license here, otherwise I'll keep it as-is.

> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > +
> > +description: |
> > +  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
> > +  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
> > +  Ethernet SGMII controllers.
> > +
> > +properties:
> > +  "#phy-cells":
> > +    const: 4
> > +    description: |
> > +      The cells contain the following arguments.
> > +
> > +      - description: The GTR lane
> > +        minimum: 0
> > +        maximum: 3
> > +      - description: The PHY type
> > +        enum:
> > +          - PHY_TYPE_DP
> > +          - PHY_TYPE_PCIE
> > +          - PHY_TYPE_SATA
> > +          - PHY_TYPE_SGMII
> > +          - PHY_TYPE_USB
> > +      - description: The PHY instance
> > +        minimum: 0
> > +        maximum: 1 # for DP, SATA or USB
> > +        maximum: 3 # for PCIE or SGMII
> > +      - description: The reference clock number
> > +        minimum: 0
> > +        maximum: 3
> 
> Humm, interesting almost json-schema. I guess it's fine as-is.
> 
> I would like to figure out how to apply a schema like this to the 
> consumer nodes. We'd have to look up the phandle, get that node's 
> compatible, find the provider's schema, find #.*-cells property, and 
> extract a schema from it. Actually, doesn't sound too hard.

That would be nice :-)

> > +
> > +  compatible:
> > +    enum:
> > +      - xlnx,zynqmp-psgtr-v1.1
> > +      - xlnx,zynqmp-psgtr
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 4
> > +    description: |
> > +      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
> > +      inputs shall not have an entry.
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 4
> > +    items:
> > +      pattern: "^ref[0-3]$"
> > +
> > +  reg:
> > +    items:
> > +      - description: SERDES registers block
> > +      - description: SIOU registers block
> > +
> > +  reg-names:
> > +    items:
> > +      - const: serdes
> > +      - const: siou
> > +
> > +required:
> > +  - "#phy-cells"
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +
> > +if:
> > +  properties:
> > +    compatible:
> > +      const: xlnx,zynqmp-psgtr
> > +
> > +then:
> > +  properties:
> > +    xlnx,tx-termination-fix:
> > +      description: |
> > +        Include this for fixing functional issue with the TX termination
> > +        resistance in GT, which can be out of spec for the XCZU9EG silicon
> > +        version.
> > +      type: boolean
> > +
> > +additionalProperties: false
> 
> This won't work with 'xlnx,tx-termination-fix'. You need to move it to 
> the main properties section and then do:
> 
> if:
>   properties:
>     compatible:
>       const: xlnx,zynqmp-psgtr-v1.1

It doesn't make a big difference as only two compatible values are
allowed, but is there a way to express the condition the other way
around, if (compatible != "xlnx,zynqmp-psgtr") ?

> 
> then:
>   properties:
>     xlnx,tx-termination-fix: false

This works.

> I think this would also work:
> 
>   not:
>     required:
>       - xlnx,tx-termination-fix

I've tested it and it works, but I'm not sure why, given that the
property isn't required required in the first place. Could you enlighten
me ?

> > +
> > +examples:
> > +  - |
> > +    phy: phy@fd400000 {
> > +      compatible = "xlnx,zynqmp-psgtr-v1.1";
> > +      reg = <0x0 0xfd400000 0x0 0x40000>,
> > +            <0x0 0xfd3d0000 0x0 0x1000>;
> > +      reg-names = "serdes", "siou";
> > +      clocks = <&refclks 3>, <&refclks 2>, <&refclks 0>;
> > +      clock-names = "ref1", "ref2", "ref3";
> > +      #phy-cells = <4>;
> > +      status = "okay";
> 
> Drop status in examples.

OK.

> > +    };
> > +
> > +...
> > diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
> > index 1f3f866fae7b..f6bc83b66ae9 100644
> > --- a/include/dt-bindings/phy/phy.h
> > +++ b/include/dt-bindings/phy/phy.h
> > @@ -17,5 +17,6 @@
> >  #define PHY_TYPE_USB3		4
> >  #define PHY_TYPE_UFS		5
> >  #define PHY_TYPE_DP		6
> > +#define PHY_TYPE_SGMII		7
> >  
> >  #endif /* _DT_BINDINGS_PHY */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6.1 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-20  9:50     ` Laurent Pinchart
@ 2020-03-20  9:55       ` Laurent Pinchart
  2020-03-20 16:53       ` [PATCH v6 " Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-03-20  9:55 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Rob Herring
  Cc: Anurag Kumar Vulisha, Michal Simek, devicetree, linux-kernel

From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>

Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
Processing System Gigabit Transceiver which provides PHY capabilities to
USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.

Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v6:

- Fixed specification of compatible-dependent xlnx,tx-termination-fix
  property
- Dropped status property from example
- Use 4 spaces to indent example

Changes since v5:

- Document clocks and clock-names properties
- Document resets and reset-names properties
- Replace subnodes with an additional entry in the PHY cells
- Drop lane frequency PHY cell, replaced by reference clock phandle
- Convert bindings to YAML
- Reword the subject line
- Drop Rob's R-b as the bindings have significantly changed
- Drop resets and reset-names properties
---
 .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 106 ++++++++++++++++++
 include/dt-bindings/phy/phy.h                 |   1 +
 2 files changed, 107 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml

diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
new file mode 100644
index 000000000000..b5d661f2813d
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+description: |
+  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
+  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
+  Ethernet SGMII controllers.
+
+properties:
+  "#phy-cells":
+    const: 4
+    description: |
+      The cells contain the following arguments.
+
+      - description: The GTR lane
+        minimum: 0
+        maximum: 3
+      - description: The PHY type
+        enum:
+          - PHY_TYPE_DP
+          - PHY_TYPE_PCIE
+          - PHY_TYPE_SATA
+          - PHY_TYPE_SGMII
+          - PHY_TYPE_USB
+      - description: The PHY instance
+        minimum: 0
+        maximum: 1 # for DP, SATA or USB
+        maximum: 3 # for PCIE or SGMII
+      - description: The reference clock number
+        minimum: 0
+        maximum: 3
+
+  compatible:
+    enum:
+      - xlnx,zynqmp-psgtr-v1.1
+      - xlnx,zynqmp-psgtr
+
+  clocks:
+    minItems: 1
+    maxItems: 4
+    description: |
+      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
+      inputs shall not have an entry.
+
+  clock-names:
+    minItems: 1
+    maxItems: 4
+    items:
+      pattern: "^ref[0-3]$"
+
+  reg:
+    items:
+      - description: SERDES registers block
+      - description: SIOU registers block
+
+  reg-names:
+    items:
+      - const: serdes
+      - const: siou
+
+  xlnx,tx-termination-fix:
+    description: |
+      Include this for fixing functional issue with the TX termination
+      resistance in GT, which can be out of spec for the XCZU9EG silicon
+      version.
+    type: boolean
+
+required:
+  - "#phy-cells"
+  - compatible
+  - reg
+  - reg-names
+
+if:
+  properties:
+    compatible:
+      const: xlnx,zynqmp-psgtr-v1.1
+
+then:
+  not:
+    required:
+      - xlnx,tx-termination-fix
+
+additionalProperties: false
+
+examples:
+  - |
+    phy: phy@fd400000 {
+        compatible = "xlnx,zynqmp-psgtr-v1.1";
+        reg = <0x0 0xfd400000 0x0 0x40000>,
+              <0x0 0xfd3d0000 0x0 0x1000>;
+        reg-names = "serdes", "siou";
+        clocks = <&refclks 3>, <&refclks 2>, <&refclks 0>;
+        clock-names = "ref1", "ref2", "ref3";
+        #phy-cells = <4>;
+    };
+
+...
diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
index 1f3f866fae7b..f6bc83b66ae9 100644
--- a/include/dt-bindings/phy/phy.h
+++ b/include/dt-bindings/phy/phy.h
@@ -17,5 +17,6 @@
 #define PHY_TYPE_USB3		4
 #define PHY_TYPE_UFS		5
 #define PHY_TYPE_DP		6
+#define PHY_TYPE_SGMII		7
 
 #endif /* _DT_BINDINGS_PHY */
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-20  9:50     ` Laurent Pinchart
  2020-03-20  9:55       ` [PATCH v6.1 " Laurent Pinchart
@ 2020-03-20 16:53       ` Rob Herring
  2020-04-01 19:25         ` Laurent Pinchart
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-03-20 16:53 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-kernel, Kishon Vijay Abraham I, Anurag Kumar Vulisha,
	Michal Simek, devicetree

On Fri, Mar 20, 2020 at 3:50 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> On Thu, Mar 19, 2020 at 08:35:20PM -0600, Rob Herring wrote:
> > On Wed, Mar 11, 2020 at 12:32:50PM +0200, Laurent Pinchart wrote:
> > > From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > >
> > > Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
> > > Processing System Gigabit Transceiver which provides PHY capabilities to
> > > USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.
> > >
> > > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > Changes since v5:
> > >
> > > - Document clocks and clock-names properties
> > > - Document resets and reset-names properties
> > > - Replace subnodes with an additional entry in the PHY cells
> > > - Drop lane frequency PHY cell, replaced by reference clock phandle
> > > - Convert bindings to YAML
> > > - Reword the subject line
> > > - Drop Rob's R-b as the bindings have significantly changed
> > > - Drop resets and reset-names properties
> > > ---
> > >  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
> > >  include/dt-bindings/phy/phy.h                 |   1 +
> > >  2 files changed, 105 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > > new file mode 100644
> > > index 000000000000..9948e4a60e45
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > > @@ -0,0 +1,104 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> >
> > For new bindings:
> >
> > (GPL-2.0-only OR BSD-2-Clause)
> >
> > Though I guess Anurag needs to agree.
>
> There's an ongoing similar discussion regarding the DPSUB (DRM/KMS)
> bindings. Hyun is checking with the Xilinx legal department. If they
> agree, I'll change the license here, otherwise I'll keep it as-is.

TBC, the choice is change it or take your toys elsewhere and play by
yourself. I don't really want to end up with whatever each submitter
desires. I don't expect there's many companies that object to a
permissive license.

> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > +
> > > +description: |
> > > +  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
> > > +  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
> > > +  Ethernet SGMII controllers.
> > > +
> > > +properties:
> > > +  "#phy-cells":
> > > +    const: 4
> > > +    description: |
> > > +      The cells contain the following arguments.
> > > +
> > > +      - description: The GTR lane
> > > +        minimum: 0
> > > +        maximum: 3
> > > +      - description: The PHY type
> > > +        enum:
> > > +          - PHY_TYPE_DP
> > > +          - PHY_TYPE_PCIE
> > > +          - PHY_TYPE_SATA
> > > +          - PHY_TYPE_SGMII
> > > +          - PHY_TYPE_USB
> > > +      - description: The PHY instance
> > > +        minimum: 0
> > > +        maximum: 1 # for DP, SATA or USB
> > > +        maximum: 3 # for PCIE or SGMII
> > > +      - description: The reference clock number
> > > +        minimum: 0
> > > +        maximum: 3
> >
> > Humm, interesting almost json-schema. I guess it's fine as-is.
> >
> > I would like to figure out how to apply a schema like this to the
> > consumer nodes. We'd have to look up the phandle, get that node's
> > compatible, find the provider's schema, find #.*-cells property, and
> > extract a schema from it. Actually, doesn't sound too hard.
>
> That would be nice :-)
>
> > > +
> > > +  compatible:
> > > +    enum:
> > > +      - xlnx,zynqmp-psgtr-v1.1
> > > +      - xlnx,zynqmp-psgtr
> > > +
> > > +  clocks:
> > > +    minItems: 1
> > > +    maxItems: 4
> > > +    description: |
> > > +      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
> > > +      inputs shall not have an entry.
> > > +
> > > +  clock-names:
> > > +    minItems: 1
> > > +    maxItems: 4
> > > +    items:
> > > +      pattern: "^ref[0-3]$"
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: SERDES registers block
> > > +      - description: SIOU registers block
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: serdes
> > > +      - const: siou
> > > +
> > > +required:
> > > +  - "#phy-cells"
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> > > +
> > > +if:
> > > +  properties:
> > > +    compatible:
> > > +      const: xlnx,zynqmp-psgtr
> > > +
> > > +then:
> > > +  properties:
> > > +    xlnx,tx-termination-fix:
> > > +      description: |
> > > +        Include this for fixing functional issue with the TX termination
> > > +        resistance in GT, which can be out of spec for the XCZU9EG silicon
> > > +        version.
> > > +      type: boolean
> > > +
> > > +additionalProperties: false
> >
> > This won't work with 'xlnx,tx-termination-fix'. You need to move it to
> > the main properties section and then do:
> >
> > if:
> >   properties:
> >     compatible:
> >       const: xlnx,zynqmp-psgtr-v1.1
>
> It doesn't make a big difference as only two compatible values are
> allowed, but is there a way to express the condition the other way
> around, if (compatible != "xlnx,zynqmp-psgtr") ?

if:
  properties:
    compatible:
      not:
        const: xlnx,zynqmp-psgtr

I think if: { not: ... } would also work. You'll have to test them out.

> > then:
> >   properties:
> >     xlnx,tx-termination-fix: false
>
> This works.
>
> > I think this would also work:
> >
> >   not:
> >     required:
> >       - xlnx,tx-termination-fix
>
> I've tested it and it works, but I'm not sure why, given that the
> property isn't required required in the first place. Could you enlighten
> me ?

'required' is true or false based on presence or absence of properties
in the list. If 'xlnx,tx-termination-fix' is present, then 'required'
evaluates to true. And the inverse is true. Then we take the inverse
of of that with 'not'.

The first case is what trips me up more because a property not present
evaluates to true. So if you look at 'select' schemas, we have to make
any properties we list (compatible typically) required.

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY
  2020-03-20 16:53       ` [PATCH v6 " Rob Herring
@ 2020-04-01 19:25         ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2020-04-01 19:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, Kishon Vijay Abraham I, Anurag Kumar Vulisha,
	Michal Simek, devicetree

Hi Rob,

On Fri, Mar 20, 2020 at 10:53:05AM -0600, Rob Herring wrote:
> On Fri, Mar 20, 2020 at 3:50 AM Laurent Pinchart wrote:
> > On Thu, Mar 19, 2020 at 08:35:20PM -0600, Rob Herring wrote:
> > > On Wed, Mar 11, 2020 at 12:32:50PM +0200, Laurent Pinchart wrote:
> > > > From: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > > >
> > > > Add DT bindings for the Xilinx ZynqMP PHY. ZynqMP SoCs have a High Speed
> > > > Processing System Gigabit Transceiver which provides PHY capabilities to
> > > > USB, SATA, PCIE, Display Port and Ehernet SGMII controllers.
> > > >
> > > > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > > Changes since v5:
> > > >
> > > > - Document clocks and clock-names properties
> > > > - Document resets and reset-names properties
> > > > - Replace subnodes with an additional entry in the PHY cells
> > > > - Drop lane frequency PHY cell, replaced by reference clock phandle
> > > > - Convert bindings to YAML
> > > > - Reword the subject line
> > > > - Drop Rob's R-b as the bindings have significantly changed
> > > > - Drop resets and reset-names properties
> > > > ---
> > > >  .../bindings/phy/xlnx,zynqmp-psgtr.yaml       | 104 ++++++++++++++++++
> > > >  include/dt-bindings/phy/phy.h                 |   1 +
> > > >  2 files changed, 105 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > > > new file mode 100644
> > > > index 000000000000..9948e4a60e45
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> > > > @@ -0,0 +1,104 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > >
> > > For new bindings:
> > >
> > > (GPL-2.0-only OR BSD-2-Clause)
> > >
> > > Though I guess Anurag needs to agree.
> >
> > There's an ongoing similar discussion regarding the DPSUB (DRM/KMS)
> > bindings. Hyun is checking with the Xilinx legal department. If they
> > agree, I'll change the license here, otherwise I'll keep it as-is.
> 
> TBC, the choice is change it or take your toys elsewhere and play by
> yourself. I don't really want to end up with whatever each submitter
> desires. I don't expect there's many companies that object to a
> permissive license.

I don't expect that either, but it's out of my control in any case.
Let's say.

I've heard quite a few times that "the preferred license for new
bindings is GPL-2.0-only OR BSD-2-Clause", but this is the first time I
hear it's a hard requirement. I have missed the decision making process,
I have nothing to question, and I'll spread that message in the future.

> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/phy/xlnx,zynqmp-psgtr.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Xilinx ZynqMP Gigabit Transceiver PHY Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > +
> > > > +description: |
> > > > +  This binding describes the Xilinx ZynqMP Gigabit Transceiver (GTR) PHY. The
> > > > +  GTR provides four lanes and is used by USB, SATA, PCIE, Display port and
> > > > +  Ethernet SGMII controllers.
> > > > +
> > > > +properties:
> > > > +  "#phy-cells":
> > > > +    const: 4
> > > > +    description: |
> > > > +      The cells contain the following arguments.
> > > > +
> > > > +      - description: The GTR lane
> > > > +        minimum: 0
> > > > +        maximum: 3
> > > > +      - description: The PHY type
> > > > +        enum:
> > > > +          - PHY_TYPE_DP
> > > > +          - PHY_TYPE_PCIE
> > > > +          - PHY_TYPE_SATA
> > > > +          - PHY_TYPE_SGMII
> > > > +          - PHY_TYPE_USB
> > > > +      - description: The PHY instance
> > > > +        minimum: 0
> > > > +        maximum: 1 # for DP, SATA or USB
> > > > +        maximum: 3 # for PCIE or SGMII
> > > > +      - description: The reference clock number
> > > > +        minimum: 0
> > > > +        maximum: 3
> > >
> > > Humm, interesting almost json-schema. I guess it's fine as-is.
> > >
> > > I would like to figure out how to apply a schema like this to the
> > > consumer nodes. We'd have to look up the phandle, get that node's
> > > compatible, find the provider's schema, find #.*-cells property, and
> > > extract a schema from it. Actually, doesn't sound too hard.
> >
> > That would be nice :-)
> >
> > > > +
> > > > +  compatible:
> > > > +    enum:
> > > > +      - xlnx,zynqmp-psgtr-v1.1
> > > > +      - xlnx,zynqmp-psgtr
> > > > +
> > > > +  clocks:
> > > > +    minItems: 1
> > > > +    maxItems: 4
> > > > +    description: |
> > > > +      Clock for each PS_MGTREFCLK[0-3] reference clock input. Unconnected
> > > > +      inputs shall not have an entry.
> > > > +
> > > > +  clock-names:
> > > > +    minItems: 1
> > > > +    maxItems: 4
> > > > +    items:
> > > > +      pattern: "^ref[0-3]$"
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: SERDES registers block
> > > > +      - description: SIOU registers block
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: serdes
> > > > +      - const: siou
> > > > +
> > > > +required:
> > > > +  - "#phy-cells"
> > > > +  - compatible
> > > > +  - reg
> > > > +  - reg-names
> > > > +
> > > > +if:
> > > > +  properties:
> > > > +    compatible:
> > > > +      const: xlnx,zynqmp-psgtr
> > > > +
> > > > +then:
> > > > +  properties:
> > > > +    xlnx,tx-termination-fix:
> > > > +      description: |
> > > > +        Include this for fixing functional issue with the TX termination
> > > > +        resistance in GT, which can be out of spec for the XCZU9EG silicon
> > > > +        version.
> > > > +      type: boolean
> > > > +
> > > > +additionalProperties: false
> > >
> > > This won't work with 'xlnx,tx-termination-fix'. You need to move it to
> > > the main properties section and then do:
> > >
> > > if:
> > >   properties:
> > >     compatible:
> > >       const: xlnx,zynqmp-psgtr-v1.1
> >
> > It doesn't make a big difference as only two compatible values are
> > allowed, but is there a way to express the condition the other way
> > around, if (compatible != "xlnx,zynqmp-psgtr") ?
> 
> if:
>   properties:
>     compatible:
>       not:
>         const: xlnx,zynqmp-psgtr
> 
> I think if: { not: ... } would also work. You'll have to test them out.

I tried both, and neither worked. No big deal, I'll keep the current
expression.

> > > then:
> > >   properties:
> > >     xlnx,tx-termination-fix: false
> >
> > This works.
> >
> > > I think this would also work:
> > >
> > >   not:
> > >     required:
> > >       - xlnx,tx-termination-fix
> >
> > I've tested it and it works, but I'm not sure why, given that the
> > property isn't required required in the first place. Could you enlighten
> > me ?
> 
> 'required' is true or false based on presence or absence of properties
> in the list. If 'xlnx,tx-termination-fix' is present, then 'required'
> evaluates to true. And the inverse is true. Then we take the inverse
> of of that with 'not'.
> 
> The first case is what trips me up more because a property not present
> evaluates to true. So if you look at 'select' schemas, we have to make
> any properties we list (compatible typically) required.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-04-01 19:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 10:32 [PATCH v6 0/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
2020-03-11 10:32 ` [PATCH v6 1/3] dt-bindings: phy: Add DT bindings for Xilinx ZynqMP PSGTR PHY Laurent Pinchart
2020-03-13 11:14   ` Kishon Vijay Abraham I
2020-03-18 15:05     ` Laurent Pinchart
2020-03-20  2:35   ` Rob Herring
2020-03-20  9:50     ` Laurent Pinchart
2020-03-20  9:55       ` [PATCH v6.1 " Laurent Pinchart
2020-03-20 16:53       ` [PATCH v6 " Rob Herring
2020-04-01 19:25         ` Laurent Pinchart
2020-03-11 10:32 ` [PATCH v6 2/3] phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver Laurent Pinchart
2020-03-11 10:32 ` [PATCH v6 3/3] arm64: dts: zynqmp: Add GTR transceivers Laurent Pinchart
2020-03-13  6:54   ` Michal Simek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).