2022-06-06 02:45:43

by Wangseok Lee

[permalink] [raw]
Subject: [PATCH v2 0/5] Add support for Axis, ARTPEC-8 PCIe driver

This v2 patchset is improvement several review comments received from patchset v1.

Main changes since v1 [1]:
-'make dt_binding_check' result improvement
-Add the missing property list
-improvement review comment of Krzysztof on driver code
-change folder name of phy driver to axis from artpec

[1] https://lore.kernel.org/lkml/20220328014430epcms2p7063834feb0abdf2f38a62723c96c9ff1@epcms2p7/

--------------------------------------------------------------
This series patches include newly PCIe support for Axis ARTPEC-8 SoC.
ARTPEC-8 is the SoC platform of Axis Communications.
PCIe controller driver and phy driver have been newly added.
There is also a new MAINTAINER in the addition of phy driver.
PCIe controller is designed based on Design-Ware PCIe controller IP
and PCIe phy is desinged based on SAMSUNG PHY IP.
It also includes modifications to the Design-Ware controller driver to
run the 64bit-based ARTPEC-8 PCIe controller driver.
It consists of 6 patches in total.

This series has been tested on AXIS SW bring-up board
with ARTPEC-8 chipset.
--------------------------------------------------------------

Wangseok Lee (5):
dt-bindings: pci: Add ARTPEC-8 PCIe controller
dt-bindings: phy: Add ARTPEC-8 PCIe phy
PCI: axis: Add ARTPEC-8 PCIe controller driver
phy: Add ARTPEC-8 PCIe PHY driver
MAINTAINERS: Add maintainer for Axis ARTPEC-8 PCIe PHY driver

.../bindings/pci/axis,artpec8-pcie-ep.yaml | 108 +++
.../devicetree/bindings/pci/axis,artpec8-pcie.yaml | 123 +++
.../bindings/phy/axis,artpec8-pcie-phy.yaml | 70 ++
MAINTAINERS | 2 +
drivers/pci/controller/dwc/Kconfig | 31 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-artpec8.c | 864 +++++++++++++++++++++
drivers/phy/Kconfig | 1 +
drivers/phy/Makefile | 1 +
drivers/phy/axis/Kconfig | 9 +
drivers/phy/axis/Makefile | 2 +
drivers/phy/axis/phy-artpec8-pcie.c | 806 +++++++++++++++++++
12 files changed, 2018 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie-ep.yaml
create mode 100644 Documentation/devicetree/bindings/pci/axis,artpec8-pcie.yaml
create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
create mode 100644 drivers/pci/controller/dwc/pcie-artpec8.c
create mode 100644 drivers/phy/axis/Kconfig
create mode 100644 drivers/phy/axis/Makefile
create mode 100644 drivers/phy/axis/phy-artpec8-pcie.c

--
2.9.5


2022-06-06 05:26:22

by Wangseok Lee

[permalink] [raw]
Subject: [PATCH v2 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

Add description to support Axis, ARTPEC-8 SoC.
ARTPEC-8 is the SoC platform of Axis Communications
and PCIe phy is designed based on SAMSUNG PHY.

changes since v1 :
-'make dt_binding_check' result improvement
-Add the missing property list
-Align the indentation of continued lines/entries

Signed-off-by: Wangseok Lee <[email protected]>
---
.../bindings/phy/axis,artpec8-pcie-phy.yaml | 70 ++++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
new file mode 100644
index 0000000..ab9766f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/axis,artpec8-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARTPEC-8 SoC PCIe PHY Device Tree Bindings
+
+maintainers:
+ - Jesper Nilsson <[email protected]>
+
+properties:
+ compatible:
+ const: axis,artpec8-pcie-phy
+
+ reg:
+ items:
+ - description: PHY registers.
+ - description: PHY coding sublayer registers.
+
+ reg-names:
+ items:
+ - const: phy
+ - const: pcs
+
+ "#phy-cells":
+ const: 0
+
+ clocks:
+ items:
+ - description: PCIe PHY reference clock
+
+ clock-names:
+ items:
+ - const: ref_clk
+
+ num-lanes:
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - "#phy-cells"
+ - clocks
+ - clock-names
+ - num-lanes
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ artpec8 {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ pcie_phy: pcie-phy@16c80000 {
+ compatible = "axis,artpec8-pcie-phy";
+ reg = <0x0 0x16c80000 0x0 0x2000>,
+ <0x0 0x16c90000 0x0 0x1000>;
+ reg-names = "phy", "pcs";
+ #phy-cells = <0>;
+ clocks = <&clock_cmu_fsys 53>;
+ clock-names = "ref_clk";
+ num-lanes = <2>;
+ };
+ };
+...
--
2.9.5

2022-06-06 05:45:18

by Wangseok Lee

[permalink] [raw]
Subject: [PATCH v2 4/5] phy: Add ARTPEC-8 PCIe PHY driver

Add support Axis, ARTPEC-8 SoC.
ARTPEC-8 is the SoC platform of Axis Communications.
This is based on arm64 and support GEN4 & 2lane.
This driver provides PHY interface for ARTPEC-8 SoC PCIe controller,
based on Samsung PCIe PHY IP.

Main changes since v1 [1]:
-change folder name of phy driver to axis from artpec

Signed-off-by: Wangseok Lee <[email protected]>
---
drivers/phy/Kconfig | 1 +
drivers/phy/Makefile | 1 +
drivers/phy/axis/Kconfig | 9 +
drivers/phy/axis/Makefile | 2 +
drivers/phy/axis/phy-artpec8-pcie.c | 806 ++++++++++++++++++++++++++++++++++++
5 files changed, 819 insertions(+)
create mode 100644 drivers/phy/axis/Kconfig
create mode 100644 drivers/phy/axis/Makefile
create mode 100644 drivers/phy/axis/phy-artpec8-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 300b0f2..92b8232 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -73,6 +73,7 @@ config PHY_CAN_TRANSCEIVER

source "drivers/phy/allwinner/Kconfig"
source "drivers/phy/amlogic/Kconfig"
+source "drivers/phy/axis/Kconfig"
source "drivers/phy/broadcom/Kconfig"
source "drivers/phy/cadence/Kconfig"
source "drivers/phy/freescale/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 01e9eff..808c055e 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
obj-y += allwinner/ \
amlogic/ \
+ axis/ \
broadcom/ \
cadence/ \
freescale/ \
diff --git a/drivers/phy/axis/Kconfig b/drivers/phy/axis/Kconfig
new file mode 100644
index 0000000..7198b93
--- /dev/null
+++ b/drivers/phy/axis/Kconfig
@@ -0,0 +1,9 @@
+config PHY_ARTPEC8_PCIE
+ bool "ARTPEC-8 PCIe PHY driver"
+ depends on OF && (ARCH_ARTPEC8 || COMPILE_TEST)
+ select GENERIC_PHY
+ help
+ Enable PCIe PHY support for ARTPEC-8 SoC.
+ This driver provides PHY interface for ARTPEC-8 SoC
+ PCIe controller.
+ This is based on Samsung PCIe PHY IP.
diff --git a/drivers/phy/axis/Makefile b/drivers/phy/axis/Makefile
new file mode 100644
index 0000000..45d853c
--- /dev/null
+++ b/drivers/phy/axis/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_ARTPEC8_PCIE) += phy-artpec8-pcie.o
diff --git a/drivers/phy/axis/phy-artpec8-pcie.c b/drivers/phy/axis/phy-artpec8-pcie.c
new file mode 100644
index 0000000..2742301
--- /dev/null
+++ b/drivers/phy/axis/phy-artpec8-pcie.c
@@ -0,0 +1,806 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PHY provider for ARTPEC-8 PCIe controller
+ *
+ * Copyright (C) 2019 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Author: Jaeho Cho <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+#include <linux/debugfs.h>
+
+/* ARTPEC-8 PCIe PHY registers */
+/* CMN registers */
+#define PCIE_PHY_CMN_REG004 0x10
+#define PCIE_PHY_CMN_REG00B 0x2C
+#define PCIE_PHY_CMN_REG016 0x58
+#define PCIE_PHY_CMN_REG01C 0x70
+#define PCIE_PHY_CMN_REG021 0x84
+#define PCIE_PHY_CMN_REG024 0x90
+#define PCIE_PHY_CMN_REG025 0x94
+#define PCIE_PHY_CMN_REG0E6 0x398
+#define PCIE_PHY_CMN_REG0E7 0x39C
+#define PCIE_PHY_CMN_REG0E8 0x3A0
+#define PCIE_PHY_CMN_REG0E9 0x3A4
+#define PCIE_PHY_CMN_REG0EA 0x3A8
+#define PCIE_PHY_CMN_REG0EB 0x3AC
+#define PCIE_PHY_CMN_REG0EC 0x3B0
+#define PCIE_PHY_CMN_REG0EE 0x3B8
+#define PCIE_PHY_CMN_REG0EF 0x3BC
+#define PCIE_PHY_CMN_REG0F1 0x3C4
+#define PCIE_PHY_CMN_REG0F3 0x3CC
+#define PCIE_PHY_CMN_REG0F4 0x3D0
+
+#define PCIE_PHY_CMN_REG101 0x404
+#define OV_I_CMN_RSTN BIT(4)
+#define OV_I_INIT_RSTN BIT(6)
+
+#define PCIE_PHY_CMN_REG131 0x4C4
+#define PCIE_PHY_CMN_REG17B 0x5EC
+#define PCIE_PHY_CMN_REG17D 0x5F4
+#define PCIE_PHY_CMN_REG190 0x640
+#define PCIE_PHY_CMN_REG191 0x644
+#define PCIE_PHY_CMN_REG192 0x648
+#define PCIE_PHY_CMN_REG1C7 0x71C
+#define PCIE_PHY_CMN_REG1DF 0x77C
+#define PCIE_PHY_CMN_REG1E0 0x780
+
+#define PCIE_PHY_CMN_REG0B1 0x2C4
+#define ANA_ROPLL_REF_DIG_CLK_SEL BIT(2)
+
+/* External clock */
+#define PCIE_PHY_CMN_REG14D 0x534
+#define PCIE_AUX_RX_MODE_EXTEND BIT(7)
+
+#define PCIE_PHY_CMN_REG0D9 0x364
+#define ANA_AUX_EXT_REF_CLK_SEL BIT(4)
+
+#define PCIE_PHY_CMN_REG10F 0x43C
+#define AUX_PLL_EN_EXTEND BIT(4)
+
+#define PCIE_PHY_CMN_REG11E 0x478
+#define AUX2_PLL_EN_EXTEND BIT(3)
+
+#define PCIE_PHY_CMN_REG0D4 0x350
+#define OV_S_ANA_AUX_EN BIT(3)
+#define OV_I_ANA_AUX_EN BIT(2)
+
+/* LANE registers */
+#define PCIE_PHY_TRSV_REG22D 0x8B4
+#define PCIE_PHY_TRSV_REG23E 0x8F8
+#define PCIE_PHY_TRSV_REG2A5 0xA94
+#define PCIE_PHY_TRSV_REG3E3 0xF8C
+#define PCIE_PHY_TRSV_REG3ED 0xFB4
+#define PCIE_PHY_TRSV_REG20B 0x82C
+#define PCIE_PHY_TRSV_REG20C 0x830
+#define PCIE_PHY_TRSV_REG234 0x8D0
+#define PCIE_PHY_TRSV_REG235 0x8D4
+#define PCIE_PHY_TRSV_REG237 0x8DC
+#define PCIE_PHY_TRSV_REG239 0x8E4
+#define PCIE_PHY_TRSV_REG23A 0x8E8
+#define PCIE_PHY_TRSV_REG23B 0x8EC
+#define PCIE_PHY_TRSV_REG24B 0x92C
+#define PCIE_PHY_TRSV_REG25D 0x974
+#define PCIE_PHY_TRSV_REG262 0x988
+#define PCIE_PHY_TRSV_REG271 0x9C4
+#define PCIE_PHY_TRSV_REG272 0x9C8
+#define PCIE_PHY_TRSV_REG27C 0x9F0
+#define PCIE_PHY_TRSV_REG27D 0x9F4
+#define PCIE_PHY_TRSV_REG27E 0x9F8
+#define PCIE_PHY_TRSV_REG284 0xA10
+#define PCIE_PHY_TRSV_REG289 0xA24
+#define PCIE_PHY_TRSV_REG28A 0xA28
+#define PCIE_PHY_TRSV_REG28B 0xA2C
+#define PCIE_PHY_TRSV_REG28C 0xA30
+#define PCIE_PHY_TRSV_REG28E 0xA38
+#define PCIE_PHY_TRSV_REG28F 0xA3C
+#define PCIE_PHY_TRSV_REG290 0xA40
+#define PCIE_PHY_TRSV_REG291 0xA44
+#define PCIE_PHY_TRSV_REG292 0xA48
+#define PCIE_PHY_TRSV_REG294 0xA50
+#define PCIE_PHY_TRSV_REG295 0xA54
+#define PCIE_PHY_TRSV_REG296 0xA58
+#define PCIE_PHY_TRSV_REG297 0xA5C
+#define PCIE_PHY_TRSV_REG298 0xA60
+#define PCIE_PHY_TRSV_REG29B 0xA6C
+#define PCIE_PHY_TRSV_REG29C 0xA70
+#define PCIE_PHY_TRSV_REG29D 0xA74
+#define PCIE_PHY_TRSV_REG29E 0xA78
+#define PCIE_PHY_TRSV_REG2AA 0xAA8
+#define PCIE_PHY_TRSV_REG2AE 0xAB8
+#define PCIE_PHY_TRSV_REG2C2 0xB08
+#define PCIE_PHY_TRSV_REG2C6 0xB18
+#define PCIE_PHY_TRSV_REG2C7 0xB1C
+#define PCIE_PHY_TRSV_REG2CB 0xB2C
+#define PCIE_PHY_TRSV_REG2CC 0xB30
+#define PCIE_PHY_TRSV_REG2CD 0xB34
+#define PCIE_PHY_TRSV_REG2CE 0xB38
+#define PCIE_PHY_TRSV_REG2D0 0xB40
+#define PCIE_PHY_TRSV_REG2CF 0xB3C
+#define PCIE_PHY_TRSV_REG2E0 0xB80
+#define PCIE_PHY_TRSV_REG2E9 0xBA4
+#define PCIE_PHY_TRSV_REG2EA 0xBA8
+#define PCIE_PHY_TRSV_REG2EB 0xBAC
+#define PCIE_PHY_TRSV_REG315 0xC54
+#define PCIE_PHY_TRSV_REG317 0xC5C
+#define PCIE_PHY_TRSV_REG319 0xC64
+#define PCIE_PHY_TRSV_REG364 0xD90
+#define PCIE_PHY_TRSV_REG36C 0xDB0
+#define PCIE_PHY_TRSV_REG36D 0xDB4
+#define PCIE_PHY_TRSV_REG37E 0xDF8
+#define PCIE_PHY_TRSV_REG37F 0xDFC
+#define PCIE_PHY_TRSV_REG38F 0xE3C
+#define PCIE_PHY_TRSV_REG391 0xE44
+#define PCIE_PHY_TRSV_REG39C 0xE70
+#define PCIE_PHY_TRSV_REG3A8 0xEA0
+#define PCIE_PHY_TRSV_REG3E0 0xF80
+#define PCIE_PHY_TRSV_REG3E1 0xF84
+#define PCIE_PHY_TRSV_REG3E7 0xF9C
+#define PCIE_PHY_TRSV_REG3E9 0xFA4
+#define PCIE_PHY_TRSV_REG3EA 0xFA8
+#define PCIE_PHY_TRSV_REG3EE 0xFB8
+#define PCIE_PHY_TRSV_REG3EF 0xFBC
+#define PCIE_PHY_TRSV_REG3F0 0xFC0
+
+#define PCIE_PHY_TRSV_REG2C0 0xB00
+#define LN_EQ_CTRL_RX_DATA_HOLD BIT(5)
+
+/* RX Preset registers */
+#define PCIE_PHY_CMN_REG17E 0x5F8
+#define PCIE_PHY_CMN_REG180 0x600
+#define PCIE_PHY_CMN_REG181 0x604
+#define PCIE_PHY_CMN_REG182 0x608
+#define PCIE_PHY_CMN_REG183 0x60C
+#define PCIE_PHY_CMN_REG184 0x610
+#define PCIE_PHY_CMN_REG185 0x614
+#define PCIE_PHY_CMN_REG186 0x618
+#define PCIE_PHY_CMN_REG187 0x61C
+
+
+/* ARTPEC-8 PCIe PCS registers */
+#define PCIE_PCS_OUT_VEC_4 0x154
+#define B1_DYNAMIC BIT(3)
+
+/* ARTPEC-8 SYS REG registers */
+#define FSYS_PCIE_CON 0x424
+#define PCIE_PHY_LCPLL_REFCLK_SEL 0x3
+#define PCIE_PHY_ROPLL_REFCLK_SEL (0x3UL << 2)
+#define ROPLL_REFCLK_NOT_AVAILABLE (0x2UL << 2)
+#define PCIE_PHY_LN0_REFCLK_PAD_EN BIT(10)
+#define PCIE_PHY_LN1_REFCLK_PAD_EN BIT(11)
+#define PCIE_PHY_PWR_OFF BIT(7)
+
+/* ARTPEC-8 Sub Controller registers */
+#define SFR_INIT_RSTN 0x1404
+#define SFR_CMN_RSTN 0x1408
+
+#define PCIE_PHY_LN0_REG_START 0x800
+#define PCIE_PHY_LN0_REG_END 0xFCC
+#define OFFSET_PER_LANE 0x800
+
+enum artpec8_pcie_phy_num_lanes {
+ LANE0 = 0,
+ LANE1,
+ LANE_MAX
+};
+
+struct artpec8_pcie_phy_data {
+ const struct phy_ops *ops;
+};
+
+struct artpec8_pcie_phy {
+ const struct artpec8_pcie_phy_data *drv_data;
+ void __iomem *phy_base;
+ void __iomem *pcs_base;
+ void __iomem *elbi_base;
+ struct clk *soc_pll_clk;
+ struct regmap *sysreg;
+ u32 lcpll_ref_clk;
+ const char *mode;
+ u32 num_lanes;
+};
+
+enum artpec8_pcie_ref_clk {
+ REF_CLK_FROM_XO = 0,
+ REF_CLK_FROM_IO,
+ REF_CLK_RESERVED,
+ REF_CLK_FROM_SOC_PLL,
+ REF_CLK_MAX
+};
+
+struct artpec8_pcie_phy_tune_reg {
+ u32 offset;
+ u32 val;
+};
+
+/* ARTPEC-8 PCIe Gen4 x2 PHY CMN register settings */
+struct artpec8_pcie_phy_tune_reg cmn_regs[] = {
+ {PCIE_PHY_CMN_REG004, 0x65},
+ {PCIE_PHY_CMN_REG00B, 0x18},
+ {PCIE_PHY_CMN_REG016, 0x0E},
+ {PCIE_PHY_CMN_REG01C, 0x4F},
+ {PCIE_PHY_CMN_REG021, 0x01},
+ {PCIE_PHY_CMN_REG024, 0x58},
+ {PCIE_PHY_CMN_REG025, 0x98},
+ {PCIE_PHY_CMN_REG0E6, 0x00},
+ {PCIE_PHY_CMN_REG0E7, 0x00},
+ {PCIE_PHY_CMN_REG0E8, 0x3F},
+ {PCIE_PHY_CMN_REG0E9, 0x3F},
+ {PCIE_PHY_CMN_REG0EA, 0xFF},
+ {PCIE_PHY_CMN_REG0EB, 0xFF},
+ {PCIE_PHY_CMN_REG0EC, 0x42},
+ {PCIE_PHY_CMN_REG0EE, 0x3F},
+ {PCIE_PHY_CMN_REG0EF, 0x7F},
+ {PCIE_PHY_CMN_REG0F1, 0x02},
+ {PCIE_PHY_CMN_REG0F3, 0xFF},
+ {PCIE_PHY_CMN_REG0F4, 0xFF},
+ {PCIE_PHY_CMN_REG131, 0x01},
+ {PCIE_PHY_CMN_REG17B, 0xC0},
+ {PCIE_PHY_CMN_REG17D, 0xAF},
+ {PCIE_PHY_CMN_REG190, 0x27},
+ {PCIE_PHY_CMN_REG191, 0x0F},
+ {PCIE_PHY_CMN_REG192, 0x3F},
+ {PCIE_PHY_CMN_REG1C7, 0x05},
+ {PCIE_PHY_CMN_REG1DF, 0x28},
+ {PCIE_PHY_CMN_REG1E0, 0x28},
+};
+
+/* ARTPEC-8 PCIe Gen4 x2 PHY lane register settings */
+struct artpec8_pcie_phy_tune_reg lane_regs[] = {
+ {PCIE_PHY_TRSV_REG22D, 0x00},
+ {PCIE_PHY_TRSV_REG23E, 0x00},
+ {PCIE_PHY_TRSV_REG2A5, 0x73},
+ {PCIE_PHY_TRSV_REG3E3, 0x7B},
+ {PCIE_PHY_TRSV_REG3ED, 0x4B},
+ {PCIE_PHY_TRSV_REG20B, 0x02},
+ {PCIE_PHY_TRSV_REG20C, 0xEA},
+ {PCIE_PHY_TRSV_REG234, 0x7A},
+ {PCIE_PHY_TRSV_REG235, 0x1C},
+ {PCIE_PHY_TRSV_REG237, 0x10},
+ {PCIE_PHY_TRSV_REG239, 0x68},
+ {PCIE_PHY_TRSV_REG23A, 0xC0},
+ {PCIE_PHY_TRSV_REG23B, 0x0B},
+ {PCIE_PHY_TRSV_REG24B, 0x00},
+ {PCIE_PHY_TRSV_REG25D, 0x07},
+ {PCIE_PHY_TRSV_REG262, 0x07},
+ {PCIE_PHY_TRSV_REG271, 0x23},
+ {PCIE_PHY_TRSV_REG272, 0x5E},
+ {PCIE_PHY_TRSV_REG27C, 0x8C},
+ {PCIE_PHY_TRSV_REG27D, 0x5B},
+ {PCIE_PHY_TRSV_REG27E, 0x2C},
+ {PCIE_PHY_TRSV_REG284, 0x33},
+ {PCIE_PHY_TRSV_REG289, 0xD4},
+ {PCIE_PHY_TRSV_REG28A, 0xCC},
+ {PCIE_PHY_TRSV_REG28B, 0xD9},
+ {PCIE_PHY_TRSV_REG28C, 0xDC},
+ {PCIE_PHY_TRSV_REG28E, 0xC6},
+ {PCIE_PHY_TRSV_REG28F, 0x90},
+ {PCIE_PHY_TRSV_REG290, 0x4D},
+ {PCIE_PHY_TRSV_REG291, 0x19},
+ {PCIE_PHY_TRSV_REG292, 0x1C},
+ {PCIE_PHY_TRSV_REG294, 0x05},
+ {PCIE_PHY_TRSV_REG295, 0x10},
+ {PCIE_PHY_TRSV_REG296, 0x0C},
+ {PCIE_PHY_TRSV_REG297, 0x19},
+ {PCIE_PHY_TRSV_REG298, 0x04},
+ {PCIE_PHY_TRSV_REG29B, 0x03},
+ {PCIE_PHY_TRSV_REG29C, 0x1B},
+ {PCIE_PHY_TRSV_REG29D, 0x1B},
+ {PCIE_PHY_TRSV_REG29E, 0x1F},
+ {PCIE_PHY_TRSV_REG2AA, 0x00},
+ {PCIE_PHY_TRSV_REG2AE, 0x1F},
+ {PCIE_PHY_TRSV_REG2C2, 0x25},
+ {PCIE_PHY_TRSV_REG2C6, 0x10},
+ {PCIE_PHY_TRSV_REG2C7, 0x06},
+ {PCIE_PHY_TRSV_REG2CB, 0x10},
+ {PCIE_PHY_TRSV_REG2CC, 0x06},
+ {PCIE_PHY_TRSV_REG2CD, 0x20},
+ {PCIE_PHY_TRSV_REG2CE, 0x27},
+ {PCIE_PHY_TRSV_REG2D0, 0x10},
+ {PCIE_PHY_TRSV_REG2CF, 0x0A},
+ {PCIE_PHY_TRSV_REG2E0, 0x01},
+ {PCIE_PHY_TRSV_REG2E9, 0x11},
+ {PCIE_PHY_TRSV_REG2EA, 0x05},
+ {PCIE_PHY_TRSV_REG2EB, 0x4C},
+ {PCIE_PHY_TRSV_REG315, 0x18},
+ {PCIE_PHY_TRSV_REG317, 0x86},
+ {PCIE_PHY_TRSV_REG319, 0x8E},
+ {PCIE_PHY_TRSV_REG364, 0x00},
+ {PCIE_PHY_TRSV_REG36C, 0x03},
+ {PCIE_PHY_TRSV_REG36D, 0x04},
+ {PCIE_PHY_TRSV_REG37E, 0x06},
+ {PCIE_PHY_TRSV_REG37F, 0x04},
+ {PCIE_PHY_TRSV_REG38F, 0x40},
+ {PCIE_PHY_TRSV_REG391, 0x8B},
+ {PCIE_PHY_TRSV_REG39C, 0xFF},
+ {PCIE_PHY_TRSV_REG3A8, 0x02},
+ {PCIE_PHY_TRSV_REG3E0, 0x93},
+ {PCIE_PHY_TRSV_REG3E1, 0x79},
+ {PCIE_PHY_TRSV_REG3E7, 0xF5},
+ {PCIE_PHY_TRSV_REG3E9, 0x75},
+ {PCIE_PHY_TRSV_REG3EA, 0x0D},
+ {PCIE_PHY_TRSV_REG3EE, 0xE2},
+ {PCIE_PHY_TRSV_REG3EF, 0x6F},
+ {PCIE_PHY_TRSV_REG3F0, 0x3D}
+};
+#define PCIE_PHY_CMN_REG17E 0x5F8
+#define PCIE_PHY_CMN_REG180 0x600
+#define PCIE_PHY_CMN_REG181 0x604
+#define PCIE_PHY_CMN_REG182 0x608
+#define PCIE_PHY_CMN_REG183 0x60C
+#define PCIE_PHY_CMN_REG184 0x610
+#define PCIE_PHY_CMN_REG185 0x614
+#define PCIE_PHY_CMN_REG186 0x618
+#define PCIE_PHY_CMN_REG187 0x61C
+
+struct artpec8_pcie_phy_tune_reg rx_preset_regs[] = {
+ /* 0 */
+ {PCIE_PHY_CMN_REG17E, 0x00},
+ {PCIE_PHY_CMN_REG180, 0x23},
+ {PCIE_PHY_CMN_REG181, 0x44},
+ {PCIE_PHY_CMN_REG182, 0x61},
+ {PCIE_PHY_CMN_REG183, 0x55},
+ {PCIE_PHY_CMN_REG184, 0x14},
+ {PCIE_PHY_CMN_REG185, 0x23},
+ {PCIE_PHY_CMN_REG186, 0x1A},
+ {PCIE_PHY_CMN_REG187, 0x04},
+ {PCIE_PHY_CMN_REG17E, 0x04},
+ {PCIE_PHY_CMN_REG17E, 0x00},
+ /* 1 */
+ {PCIE_PHY_CMN_REG17E, 0x08},
+ {PCIE_PHY_CMN_REG181, 0x42},
+ {PCIE_PHY_CMN_REG17E, 0x0C},
+ {PCIE_PHY_CMN_REG17E, 0x08},
+ /* 2 */
+ {PCIE_PHY_CMN_REG17E, 0x10},
+ {PCIE_PHY_CMN_REG181, 0x40},
+ {PCIE_PHY_CMN_REG17E, 0x14},
+ {PCIE_PHY_CMN_REG17E, 0x10},
+ /* 3 */
+ {PCIE_PHY_CMN_REG17E, 0x18},
+ {PCIE_PHY_CMN_REG181, 0x45},
+ {PCIE_PHY_CMN_REG17E, 0x1C},
+ {PCIE_PHY_CMN_REG17E, 0x18},
+ /* 4 */
+ {PCIE_PHY_CMN_REG17E, 0x20},
+ {PCIE_PHY_CMN_REG181, 0x46},
+ {PCIE_PHY_CMN_REG17E, 0x24},
+ {PCIE_PHY_CMN_REG17E, 0x20},
+ /* 5 */
+ {PCIE_PHY_CMN_REG17E, 0x28},
+ {PCIE_PHY_CMN_REG181, 0x48},
+ {PCIE_PHY_CMN_REG17E, 0x2C},
+ {PCIE_PHY_CMN_REG17E, 0x28},
+ /* 6 */
+ {PCIE_PHY_CMN_REG17E, 0x30},
+ {PCIE_PHY_CMN_REG181, 0x4A},
+ {PCIE_PHY_CMN_REG17E, 0x34},
+ {PCIE_PHY_CMN_REG17E, 0x30},
+ /* 7 */
+ {PCIE_PHY_CMN_REG17E, 0x38},
+ {PCIE_PHY_CMN_REG181, 0x4C},
+ {PCIE_PHY_CMN_REG17E, 0x3C},
+ {PCIE_PHY_CMN_REG17E, 0x38},
+ /* 8 */
+ {PCIE_PHY_CMN_REG17E, 0x40},
+ {PCIE_PHY_CMN_REG180, 0x20},
+ {PCIE_PHY_CMN_REG181, 0x20},
+ {PCIE_PHY_CMN_REG182, 0x01},
+ {PCIE_PHY_CMN_REG17E, 0x44},
+ {PCIE_PHY_CMN_REG17E, 0x40},
+ /* 9 */
+ {PCIE_PHY_CMN_REG17E, 0x48},
+ {PCIE_PHY_CMN_REG180, 0x20},
+ {PCIE_PHY_CMN_REG181, 0x21},
+ {PCIE_PHY_CMN_REG182, 0x01},
+ {PCIE_PHY_CMN_REG17E, 0x4C},
+ {PCIE_PHY_CMN_REG17E, 0x48},
+ /* 10 */
+ {PCIE_PHY_CMN_REG17E, 0x50},
+ {PCIE_PHY_CMN_REG180, 0x24},
+ {PCIE_PHY_CMN_REG181, 0x80},
+ {PCIE_PHY_CMN_REG182, 0x41},
+ {PCIE_PHY_CMN_REG183, 0xAF},
+ {PCIE_PHY_CMN_REG184, 0x26},
+ {PCIE_PHY_CMN_REG185, 0x34},
+ {PCIE_PHY_CMN_REG186, 0x24},
+ {PCIE_PHY_CMN_REG187, 0x06},
+ {PCIE_PHY_CMN_REG17E, 0x54},
+ {PCIE_PHY_CMN_REG17E, 0x50},
+ /* 11 */
+ {PCIE_PHY_CMN_REG17E, 0x58},
+ {PCIE_PHY_CMN_REG181, 0x81},
+ {PCIE_PHY_CMN_REG17E, 0x5C},
+ {PCIE_PHY_CMN_REG17E, 0x58},
+ /* 12 */
+ {PCIE_PHY_CMN_REG17E, 0x60},
+ {PCIE_PHY_CMN_REG181, 0x82},
+ {PCIE_PHY_CMN_REG17E, 0x64},
+ {PCIE_PHY_CMN_REG17E, 0x60},
+ /* 13 */
+ {PCIE_PHY_CMN_REG17E, 0x68},
+ {PCIE_PHY_CMN_REG181, 0x83},
+ {PCIE_PHY_CMN_REG17E, 0x6C},
+ {PCIE_PHY_CMN_REG17E, 0x68},
+ /* 14 */
+ {PCIE_PHY_CMN_REG17E, 0x70},
+ {PCIE_PHY_CMN_REG181, 0x84},
+ {PCIE_PHY_CMN_REG17E, 0x74},
+ {PCIE_PHY_CMN_REG17E, 0x70},
+ /* 15 */
+ {PCIE_PHY_CMN_REG17E, 0x78},
+ {PCIE_PHY_CMN_REG180, 0x24},
+ {PCIE_PHY_CMN_REG181, 0x85},
+ {PCIE_PHY_CMN_REG182, 0x80},
+ {PCIE_PHY_CMN_REG183, 0x7F},
+ {PCIE_PHY_CMN_REG184, 0x2D},
+ {PCIE_PHY_CMN_REG185, 0x34},
+ {PCIE_PHY_CMN_REG186, 0x24},
+ {PCIE_PHY_CMN_REG187, 0x05},
+ {PCIE_PHY_CMN_REG17E, 0x7C},
+ {PCIE_PHY_CMN_REG17E, 0x78},
+ /* 16 */
+ {PCIE_PHY_CMN_REG17E, 0x80},
+ {PCIE_PHY_CMN_REG181, 0x86},
+ {PCIE_PHY_CMN_REG17E, 0x84},
+ {PCIE_PHY_CMN_REG17E, 0x80},
+ /* 17 */
+ {PCIE_PHY_CMN_REG17E, 0x88},
+ {PCIE_PHY_CMN_REG181, 0x87},
+ {PCIE_PHY_CMN_REG17E, 0x8C},
+ {PCIE_PHY_CMN_REG17E, 0x88},
+ /* 18 */
+ {PCIE_PHY_CMN_REG17E, 0x90},
+ {PCIE_PHY_CMN_REG181, 0x88},
+ {PCIE_PHY_CMN_REG17E, 0x94},
+ {PCIE_PHY_CMN_REG17E, 0x90},
+ /* 19 */
+ {PCIE_PHY_CMN_REG17E, 0x98},
+ {PCIE_PHY_CMN_REG181, 0x89},
+ {PCIE_PHY_CMN_REG17E, 0x9C},
+ {PCIE_PHY_CMN_REG17E, 0x98},
+};
+
+
+static void artpec8_pcie_phy_reg_writel(void __iomem *base, u32 val, u32 reg)
+{
+ writel(val, base + reg);
+};
+
+static u32 artpec8_pcie_phy_reg_readl(void __iomem *base, u32 reg)
+{
+ return readl(base + reg);
+};
+
+static void artpec8_pcie_phy_reg_update(void __iomem *base, u32 mask,
+ u32 update, u32 reg)
+{
+ u32 val;
+
+ val = artpec8_pcie_phy_reg_readl(base, reg);
+ val &= ~(mask);
+ val |= update;
+ artpec8_pcie_phy_reg_writel(base, val, reg);
+};
+
+static void artpec8_pcie_enable_ref_clk_from_xo(struct artpec8_pcie_phy
+ *pciephy)
+{
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_LCPLL_REFCLK_SEL,
+ REF_CLK_FROM_XO);
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_ROPLL_REFCLK_SEL,
+ ROPLL_REFCLK_NOT_AVAILABLE);
+};
+
+static void artpec8_pcie_enable_ref_clk_from_io(struct artpec8_pcie_phy
+ *pciephy)
+{
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ PCIE_AUX_RX_MODE_EXTEND, 0,
+ PCIE_PHY_CMN_REG14D);
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ ANA_AUX_EXT_REF_CLK_SEL, 0,
+ PCIE_PHY_CMN_REG0D9);
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ AUX_PLL_EN_EXTEND, 0,
+ PCIE_PHY_CMN_REG10F);
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ AUX2_PLL_EN_EXTEND, 0,
+ PCIE_PHY_CMN_REG11E);
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ OV_S_ANA_AUX_EN, OV_S_ANA_AUX_EN,
+ PCIE_PHY_CMN_REG0D4);
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ OV_I_ANA_AUX_EN, OV_I_ANA_AUX_EN,
+ PCIE_PHY_CMN_REG0D4);
+
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_LCPLL_REFCLK_SEL,
+ REF_CLK_FROM_IO);
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_ROPLL_REFCLK_SEL,
+ ROPLL_REFCLK_NOT_AVAILABLE);
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_LN0_REFCLK_PAD_EN,
+ PCIE_PHY_LN0_REFCLK_PAD_EN);
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_LN1_REFCLK_PAD_EN,
+ PCIE_PHY_LN1_REFCLK_PAD_EN);
+}
+
+static void artpec8_pcie_enable_ref_clk_from_soc_pll(struct artpec8_pcie_phy
+ *pciephy)
+{
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_LCPLL_REFCLK_SEL,
+ REF_CLK_FROM_SOC_PLL);
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_ROPLL_REFCLK_SEL,
+ ROPLL_REFCLK_NOT_AVAILABLE);
+}
+
+static void artpec8_pcie_lane_control(struct phy *phy, u32 lane0_reg, u32 mask,
+ u32 val)
+{
+ struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+ u32 lanex_reg = lane0_reg;
+ int i;
+
+ if (lane0_reg < PCIE_PHY_LN0_REG_START ||
+ lane0_reg > PCIE_PHY_LN0_REG_END) {
+ return;
+ }
+
+ for (i = 0; i < pciephy->num_lanes; i++) {
+ lanex_reg += OFFSET_PER_LANE * i;
+ artpec8_pcie_phy_reg_update(pciephy->phy_base, mask, val,
+ lanex_reg);
+ }
+}
+
+static void artpec8_pcie_phy_tune(struct artpec8_pcie_phy *pciephy)
+{
+ int i, count;
+
+ /* init cmn registers */
+ count = ARRAY_SIZE(cmn_regs);
+ for (i = 0; i < count; i++) {
+ artpec8_pcie_phy_reg_writel(pciephy->phy_base,
+ cmn_regs[i].val,
+ cmn_regs[i].offset);
+ }
+
+ /* init lane registers */
+ count = ARRAY_SIZE(lane_regs);
+ for (i = 0; i < count; i++) {
+ artpec8_pcie_phy_reg_writel(pciephy->phy_base,
+ lane_regs[i].val,
+ lane_regs[i].offset);
+ artpec8_pcie_phy_reg_writel(pciephy->phy_base,
+ lane_regs[i].val,
+ lane_regs[i].offset +
+ OFFSET_PER_LANE);
+ }
+
+ /* rx preset registers */
+ count = ARRAY_SIZE(rx_preset_regs);
+ for (i = 0; i < count; i++) {
+ artpec8_pcie_phy_reg_writel(pciephy->phy_base,
+ rx_preset_regs[i].val,
+ rx_preset_regs[i].offset);
+ }
+}
+
+static int artpec8_pcie_phy_init(struct phy *phy)
+{
+ struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+ /* reset init_rstn and cmn_rstn */
+ artpec8_pcie_phy_reg_update(pciephy->phy_base,
+ OV_I_CMN_RSTN | OV_I_INIT_RSTN, 0, PCIE_PHY_CMN_REG101);
+
+ /* reference clock selection */
+ switch (pciephy->lcpll_ref_clk) {
+ case REF_CLK_FROM_XO:
+ artpec8_pcie_enable_ref_clk_from_xo(pciephy);
+ break;
+ case REF_CLK_FROM_IO:
+ artpec8_pcie_enable_ref_clk_from_io(pciephy);
+ break;
+ case REF_CLK_FROM_SOC_PLL:
+ artpec8_pcie_enable_ref_clk_from_soc_pll(pciephy);
+ break;
+ default:
+ break;
+ }
+
+ /* release i_init_rstn */
+ artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_INIT_RSTN,
+ OV_I_INIT_RSTN, PCIE_PHY_CMN_REG101);
+
+ /* phy initial settings */
+ artpec8_pcie_phy_tune(pciephy);
+
+ /* pll_en should be set to off when PM_STATE is P1.CPM */
+ if (!strncmp(pciephy->mode, "pcie_ep", strlen("pcie_ep"))) {
+ artpec8_pcie_phy_reg_update(pciephy->pcs_base, B1_DYNAMIC,
+ B1_DYNAMIC, PCIE_PCS_OUT_VEC_4);
+ }
+
+ /* disable lane eq ctrl rx data hold */
+ artpec8_pcie_lane_control(phy, PCIE_PHY_TRSV_REG2C0,
+ LN_EQ_CTRL_RX_DATA_HOLD, 0);
+
+ return 0;
+}
+
+static int artpec8_pcie_phy_exit(struct phy *phy)
+{
+ return 0;
+}
+
+static int artpec8_pcie_phy_reset(struct phy *phy)
+{
+ struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+ artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_CMN_RSTN,
+ 0, PCIE_PHY_CMN_REG101);
+ udelay(10);
+ artpec8_pcie_phy_reg_update(pciephy->phy_base, OV_I_CMN_RSTN,
+ OV_I_CMN_RSTN, PCIE_PHY_CMN_REG101);
+
+ return 0;
+}
+
+static int artpec8_pcie_phy_power_on(struct phy *phy)
+{
+ struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_PWR_OFF, 0);
+
+ return 0;
+}
+
+static int artpec8_pcie_phy_power_off(struct phy *phy)
+{
+ struct artpec8_pcie_phy *pciephy = phy_get_drvdata(phy);
+
+ regmap_update_bits(pciephy->sysreg, FSYS_PCIE_CON,
+ PCIE_PHY_PWR_OFF, PCIE_PHY_PWR_OFF);
+
+ return 0;
+}
+
+static int artpec8_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct artpec8_pcie_phy *artpec8_phy;
+ struct phy *generic_phy;
+ struct phy_provider *phy_provider;
+ const struct artpec8_pcie_phy_data *drv_data;
+
+ drv_data = of_device_get_match_data(dev);
+ if (!drv_data)
+ return -ENODEV;
+
+ artpec8_phy = devm_kzalloc(dev, sizeof(*artpec8_phy), GFP_KERNEL);
+ if (!artpec8_phy)
+ return -ENOMEM;
+
+ /* reference clock */
+ if (of_property_read_u32(dev->of_node, "lcpll-ref-clk",
+ &artpec8_phy->lcpll_ref_clk)) {
+ return -EINVAL;
+ }
+ /* PLL SOC reference clock */
+ if (artpec8_phy->lcpll_ref_clk == REF_CLK_FROM_SOC_PLL) {
+ artpec8_phy->soc_pll_clk = devm_clk_get(dev, "ref_clk");
+ if (IS_ERR(artpec8_phy->soc_pll_clk))
+ return -EINVAL;
+ clk_prepare_enable(artpec8_phy->soc_pll_clk);
+ }
+
+ /* link mode */
+ if (of_property_read_string(dev->of_node, "mode", &artpec8_phy->mode))
+ return -EINVAL;
+
+ /* number of lanes */
+ if (of_property_read_u32(dev->of_node, "num-lanes",
+ &artpec8_phy->num_lanes))
+ return -EINVAL;
+
+ if (artpec8_phy->num_lanes > LANE_MAX)
+ return -EINVAL;
+
+ /* PHY base register */
+ artpec8_phy->phy_base = devm_platform_ioremap_resource_byname(pdev, "phy");
+ if (IS_ERR(artpec8_phy->phy_base))
+ return PTR_ERR(artpec8_phy->phy_base);
+
+ /* PCS base register */
+ artpec8_phy->pcs_base = devm_platform_ioremap_resource_byname(pdev, "pcs");
+ if (IS_ERR(artpec8_phy->pcs_base))
+ return PTR_ERR(artpec8_phy->pcs_base);
+
+ /* sysreg regmap handle, need to change using smc */
+ artpec8_phy->sysreg =
+ syscon_regmap_lookup_by_phandle(dev->of_node,
+ "samsung,fsys-sysreg");
+ if (IS_ERR(artpec8_phy->sysreg))
+ return PTR_ERR(artpec8_phy->sysreg);
+
+ artpec8_phy->drv_data = drv_data;
+
+ generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
+ if (IS_ERR(generic_phy))
+ return PTR_ERR(generic_phy);
+
+ phy_set_drvdata(generic_phy, artpec8_phy);
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ return 0;
+}
+
+static int __exit artpec8_pcie_remove(struct platform_device *pdev)
+{
+ struct artpec8_pcie_phy *artpec8_phy = platform_get_drvdata(pdev);
+
+ if (artpec8_phy->soc_pll_clk)
+ clk_disable_unprepare(artpec8_phy->soc_pll_clk);
+
+ return 0;
+}
+
+static const struct phy_ops artpec8_phy_ops = {
+ .init = artpec8_pcie_phy_init,
+ .exit = artpec8_pcie_phy_exit,
+ .reset = artpec8_pcie_phy_reset,
+ .power_on = artpec8_pcie_phy_power_on,
+ .power_off = artpec8_pcie_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static const struct artpec8_pcie_phy_data artpec8_pcie_phy_data = {
+ .ops = &artpec8_phy_ops,
+};
+
+static const struct of_device_id artpec8_pcie_phy_match[] = {
+ {
+ .compatible = "axis,artpec8-pcie-phy",
+ .data = &artpec8_pcie_phy_data,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, artpec8_pcie_phy_match);
+
+static struct platform_driver artpec8_pcie_phy_driver = {
+ .probe = artpec8_pcie_phy_probe,
+ .remove = __exit_p(artpec8_pcie_phy_remove),
+ .driver = {
+ .of_match_table = artpec8_pcie_phy_match,
+ .name = "artpec8_pcie_phy",
+ }
+};
+
+module_platform_driver(artpec8_pcie_phy_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaeho Cho <[email protected]>");
--
2.9.5

2022-06-06 10:35:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

On 03/06/2022 04:31, Wangseok Lee wrote:
> Add description to support Axis, ARTPEC-8 SoC.
> ARTPEC-8 is the SoC platform of Axis Communications
> and PCIe phy is designed based on SAMSUNG PHY.

This does not look like wrapped in Linux commit style.
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586

>
> changes since v1 :
> -'make dt_binding_check' result improvement
> -Add the missing property list
> -Align the indentation of continued lines/entries
>
> Signed-off-by: Wangseok Lee <[email protected]>
> ---
> .../bindings/phy/axis,artpec8-pcie-phy.yaml | 70 ++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>
> diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> new file mode 100644
> index 0000000..ab9766f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/axis,artpec8-pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARTPEC-8 SoC PCIe PHY Device Tree Bindings

Same comment as patch #1.

> +
> +maintainers:
> + - Jesper Nilsson <[email protected]>
> +
> +properties:
> + compatible:
> + const: axis,artpec8-pcie-phy
> +
> + reg:
> + items:
> + - description: PHY registers.
> + - description: PHY coding sublayer registers.
> +
> + reg-names:
> + items:
> + - const: phy
> + - const: pcs
> +
> + "#phy-cells":
> + const: 0
> +
> + clocks:
> + items:
> + - description: PCIe PHY reference clock
> +
> + clock-names:
> + items:
> + - const: ref_clk

Same comment as patch #1.

> +
> + num-lanes:
> + const: 2
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - "#phy-cells"
> + - clocks
> + - clock-names
> + - num-lanes
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + artpec8 {

Same comment as patch #1.


> + #address-cells = <2>;
> + #size-cells = <2>;
> + pcie_phy: pcie-phy@16c80000 {
> + compatible = "axis,artpec8-pcie-phy";
> + reg = <0x0 0x16c80000 0x0 0x2000>,
> + <0x0 0x16c90000 0x0 0x1000>;
> + reg-names = "phy", "pcs";
> + #phy-cells = <0>;
> + clocks = <&clock_cmu_fsys 53>;
> + clock-names = "ref_clk";
> + num-lanes = <2>;
> + };
> + };
> +...


Best regards,
Krzysztof

2022-06-06 10:50:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] phy: Add ARTPEC-8 PCIe PHY driver

On 03/06/2022 04:38, Wangseok Lee wrote:
> Add support Axis, ARTPEC-8 SoC.
> ARTPEC-8 is the SoC platform of Axis Communications.
> This is based on arm64 and support GEN4 & 2lane.
> This driver provides PHY interface for ARTPEC-8 SoC PCIe controller,
> based on Samsung PCIe PHY IP.
>
> Main changes since v1 [1]:
> -change folder name of phy driver to axis from artpec
>
> Signed-off-by: Wangseok Lee <[email protected]>
> ---
> drivers/phy/Kconfig | 1 +
> drivers/phy/Makefile | 1 +
> drivers/phy/axis/Kconfig | 9 +
> drivers/phy/axis/Makefile | 2 +
> drivers/phy/axis/phy-artpec8-pcie.c | 806 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 819 insertions(+)
> create mode 100644 drivers/phy/axis/Kconfig
> create mode 100644 drivers/phy/axis/Makefile
> create mode 100644 drivers/phy/axis/phy-artpec8-pcie.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 300b0f2..92b8232 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -73,6 +73,7 @@ config PHY_CAN_TRANSCEIVER
>
> source "drivers/phy/allwinner/Kconfig"
> source "drivers/phy/amlogic/Kconfig"
> +source "drivers/phy/axis/Kconfig"
> source "drivers/phy/broadcom/Kconfig"
> source "drivers/phy/cadence/Kconfig"
> source "drivers/phy/freescale/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 01e9eff..808c055e 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
> obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
> obj-y += allwinner/ \
> amlogic/ \
> + axis/ \
> broadcom/ \
> cadence/ \
> freescale/ \
> diff --git a/drivers/phy/axis/Kconfig b/drivers/phy/axis/Kconfig
> new file mode 100644
> index 0000000..7198b93
> --- /dev/null
> +++ b/drivers/phy/axis/Kconfig
> @@ -0,0 +1,9 @@
> +config PHY_ARTPEC8_PCIE
> + bool "ARTPEC-8 PCIe PHY driver"
> + depends on OF && (ARCH_ARTPEC8 || COMPILE_TEST)
> + select GENERIC_PHY
> + help
> + Enable PCIe PHY support for ARTPEC-8 SoC.
> + This driver provides PHY interface for ARTPEC-8 SoC
> + PCIe controller.
> + This is based on Samsung PCIe PHY IP.
> diff --git a/drivers/phy/axis/Makefile b/drivers/phy/axis/Makefile
> new file mode 100644
> index 0000000..45d853c
> --- /dev/null
> +++ b/drivers/phy/axis/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PHY_ARTPEC8_PCIE) += phy-artpec8-pcie.o
> diff --git a/drivers/phy/axis/phy-artpec8-pcie.c b/drivers/phy/axis/phy-artpec8-pcie.c
> new file mode 100644
> index 0000000..2742301
> --- /dev/null
> +++ b/drivers/phy/axis/phy-artpec8-pcie.c
> @@ -0,0 +1,806 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PHY provider for ARTPEC-8 PCIe controller
> + *
> + * Copyright (C) 2019 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * Author: Jaeho Cho <[email protected]>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/debugfs.h>
> +
> +/* ARTPEC-8 PCIe PHY registers */
> +/* CMN registers */
> +#define PCIE_PHY_CMN_REG004 0x10
> +#define PCIE_PHY_CMN_REG00B 0x2C
> +#define PCIE_PHY_CMN_REG016 0x58
> +#define PCIE_PHY_CMN_REG01C 0x70
> +#define PCIE_PHY_CMN_REG021 0x84
> +#define PCIE_PHY_CMN_REG024 0x90
> +#define PCIE_PHY_CMN_REG025 0x94
> +#define PCIE_PHY_CMN_REG0E6 0x398
> +#define PCIE_PHY_CMN_REG0E7 0x39C
> +#define PCIE_PHY_CMN_REG0E8 0x3A0
> +#define PCIE_PHY_CMN_REG0E9 0x3A4
> +#define PCIE_PHY_CMN_REG0EA 0x3A8
> +#define PCIE_PHY_CMN_REG0EB 0x3AC
> +#define PCIE_PHY_CMN_REG0EC 0x3B0
> +#define PCIE_PHY_CMN_REG0EE 0x3B8
> +#define PCIE_PHY_CMN_REG0EF 0x3BC
> +#define PCIE_PHY_CMN_REG0F1 0x3C4
> +#define PCIE_PHY_CMN_REG0F3 0x3CC
> +#define PCIE_PHY_CMN_REG0F4 0x3D0
> +
> +#define PCIE_PHY_CMN_REG101 0x404
> +#define OV_I_CMN_RSTN BIT(4)
> +#define OV_I_INIT_RSTN BIT(6)
> +
> +#define PCIE_PHY_CMN_REG131 0x4C4
> +#define PCIE_PHY_CMN_REG17B 0x5EC
> +#define PCIE_PHY_CMN_REG17D 0x5F4
> +#define PCIE_PHY_CMN_REG190 0x640
> +#define PCIE_PHY_CMN_REG191 0x644
> +#define PCIE_PHY_CMN_REG192 0x648
> +#define PCIE_PHY_CMN_REG1C7 0x71C
> +#define PCIE_PHY_CMN_REG1DF 0x77C
> +#define PCIE_PHY_CMN_REG1E0 0x780
> +
> +#define PCIE_PHY_CMN_REG0B1 0x2C4
> +#define ANA_ROPLL_REF_DIG_CLK_SEL BIT(2)
> +
> +/* External clock */
> +#define PCIE_PHY_CMN_REG14D 0x534
> +#define PCIE_AUX_RX_MODE_EXTEND BIT(7)
> +
> +#define PCIE_PHY_CMN_REG0D9 0x364
> +#define ANA_AUX_EXT_REF_CLK_SEL BIT(4)
> +
> +#define PCIE_PHY_CMN_REG10F 0x43C
> +#define AUX_PLL_EN_EXTEND BIT(4)
> +
> +#define PCIE_PHY_CMN_REG11E 0x478
> +#define AUX2_PLL_EN_EXTEND BIT(3)
> +
> +#define PCIE_PHY_CMN_REG0D4 0x350
> +#define OV_S_ANA_AUX_EN BIT(3)
> +#define OV_I_ANA_AUX_EN BIT(2)
> +
> +/* LANE registers */
> +#define PCIE_PHY_TRSV_REG22D 0x8B4
> +#define PCIE_PHY_TRSV_REG23E 0x8F8
> +#define PCIE_PHY_TRSV_REG2A5 0xA94
> +#define PCIE_PHY_TRSV_REG3E3 0xF8C
> +#define PCIE_PHY_TRSV_REG3ED 0xFB4
> +#define PCIE_PHY_TRSV_REG20B 0x82C
> +#define PCIE_PHY_TRSV_REG20C 0x830
> +#define PCIE_PHY_TRSV_REG234 0x8D0
> +#define PCIE_PHY_TRSV_REG235 0x8D4
> +#define PCIE_PHY_TRSV_REG237 0x8DC
> +#define PCIE_PHY_TRSV_REG239 0x8E4
> +#define PCIE_PHY_TRSV_REG23A 0x8E8
> +#define PCIE_PHY_TRSV_REG23B 0x8EC
> +#define PCIE_PHY_TRSV_REG24B 0x92C
> +#define PCIE_PHY_TRSV_REG25D 0x974
> +#define PCIE_PHY_TRSV_REG262 0x988
> +#define PCIE_PHY_TRSV_REG271 0x9C4
> +#define PCIE_PHY_TRSV_REG272 0x9C8
> +#define PCIE_PHY_TRSV_REG27C 0x9F0
> +#define PCIE_PHY_TRSV_REG27D 0x9F4
> +#define PCIE_PHY_TRSV_REG27E 0x9F8
> +#define PCIE_PHY_TRSV_REG284 0xA10
> +#define PCIE_PHY_TRSV_REG289 0xA24
> +#define PCIE_PHY_TRSV_REG28A 0xA28
> +#define PCIE_PHY_TRSV_REG28B 0xA2C
> +#define PCIE_PHY_TRSV_REG28C 0xA30
> +#define PCIE_PHY_TRSV_REG28E 0xA38
> +#define PCIE_PHY_TRSV_REG28F 0xA3C
> +#define PCIE_PHY_TRSV_REG290 0xA40
> +#define PCIE_PHY_TRSV_REG291 0xA44
> +#define PCIE_PHY_TRSV_REG292 0xA48
> +#define PCIE_PHY_TRSV_REG294 0xA50
> +#define PCIE_PHY_TRSV_REG295 0xA54
> +#define PCIE_PHY_TRSV_REG296 0xA58
> +#define PCIE_PHY_TRSV_REG297 0xA5C
> +#define PCIE_PHY_TRSV_REG298 0xA60
> +#define PCIE_PHY_TRSV_REG29B 0xA6C
> +#define PCIE_PHY_TRSV_REG29C 0xA70
> +#define PCIE_PHY_TRSV_REG29D 0xA74
> +#define PCIE_PHY_TRSV_REG29E 0xA78
> +#define PCIE_PHY_TRSV_REG2AA 0xAA8
> +#define PCIE_PHY_TRSV_REG2AE 0xAB8
> +#define PCIE_PHY_TRSV_REG2C2 0xB08
> +#define PCIE_PHY_TRSV_REG2C6 0xB18
> +#define PCIE_PHY_TRSV_REG2C7 0xB1C
> +#define PCIE_PHY_TRSV_REG2CB 0xB2C
> +#define PCIE_PHY_TRSV_REG2CC 0xB30
> +#define PCIE_PHY_TRSV_REG2CD 0xB34
> +#define PCIE_PHY_TRSV_REG2CE 0xB38
> +#define PCIE_PHY_TRSV_REG2D0 0xB40
> +#define PCIE_PHY_TRSV_REG2CF 0xB3C
> +#define PCIE_PHY_TRSV_REG2E0 0xB80
> +#define PCIE_PHY_TRSV_REG2E9 0xBA4
> +#define PCIE_PHY_TRSV_REG2EA 0xBA8
> +#define PCIE_PHY_TRSV_REG2EB 0xBAC
> +#define PCIE_PHY_TRSV_REG315 0xC54
> +#define PCIE_PHY_TRSV_REG317 0xC5C
> +#define PCIE_PHY_TRSV_REG319 0xC64
> +#define PCIE_PHY_TRSV_REG364 0xD90
> +#define PCIE_PHY_TRSV_REG36C 0xDB0
> +#define PCIE_PHY_TRSV_REG36D 0xDB4
> +#define PCIE_PHY_TRSV_REG37E 0xDF8
> +#define PCIE_PHY_TRSV_REG37F 0xDFC
> +#define PCIE_PHY_TRSV_REG38F 0xE3C
> +#define PCIE_PHY_TRSV_REG391 0xE44
> +#define PCIE_PHY_TRSV_REG39C 0xE70
> +#define PCIE_PHY_TRSV_REG3A8 0xEA0
> +#define PCIE_PHY_TRSV_REG3E0 0xF80
> +#define PCIE_PHY_TRSV_REG3E1 0xF84
> +#define PCIE_PHY_TRSV_REG3E7 0xF9C
> +#define PCIE_PHY_TRSV_REG3E9 0xFA4
> +#define PCIE_PHY_TRSV_REG3EA 0xFA8
> +#define PCIE_PHY_TRSV_REG3EE 0xFB8
> +#define PCIE_PHY_TRSV_REG3EF 0xFBC
> +#define PCIE_PHY_TRSV_REG3F0 0xFC0
> +
> +#define PCIE_PHY_TRSV_REG2C0 0xB00
> +#define LN_EQ_CTRL_RX_DATA_HOLD BIT(5)
> +
> +/* RX Preset registers */
> +#define PCIE_PHY_CMN_REG17E 0x5F8
> +#define PCIE_PHY_CMN_REG180 0x600
> +#define PCIE_PHY_CMN_REG181 0x604
> +#define PCIE_PHY_CMN_REG182 0x608
> +#define PCIE_PHY_CMN_REG183 0x60C
> +#define PCIE_PHY_CMN_REG184 0x610
> +#define PCIE_PHY_CMN_REG185 0x614
> +#define PCIE_PHY_CMN_REG186 0x618
> +#define PCIE_PHY_CMN_REG187 0x61C
> +
> +
> +/* ARTPEC-8 PCIe PCS registers */
> +#define PCIE_PCS_OUT_VEC_4 0x154
> +#define B1_DYNAMIC BIT(3)
> +
> +/* ARTPEC-8 SYS REG registers */
> +#define FSYS_PCIE_CON 0x424
> +#define PCIE_PHY_LCPLL_REFCLK_SEL 0x3
> +#define PCIE_PHY_ROPLL_REFCLK_SEL (0x3UL << 2)
> +#define ROPLL_REFCLK_NOT_AVAILABLE (0x2UL << 2)
> +#define PCIE_PHY_LN0_REFCLK_PAD_EN BIT(10)
> +#define PCIE_PHY_LN1_REFCLK_PAD_EN BIT(11)
> +#define PCIE_PHY_PWR_OFF BIT(7)
> +
> +/* ARTPEC-8 Sub Controller registers */
> +#define SFR_INIT_RSTN 0x1404
> +#define SFR_CMN_RSTN 0x1408
> +
> +#define PCIE_PHY_LN0_REG_START 0x800
> +#define PCIE_PHY_LN0_REG_END 0xFCC
> +#define OFFSET_PER_LANE 0x800
> +
> +enum artpec8_pcie_phy_num_lanes {
> + LANE0 = 0,
> + LANE1,
> + LANE_MAX
> +};
> +
> +struct artpec8_pcie_phy_data {
> + const struct phy_ops *ops;

No need for indentation before "*ops". Other places do not use it.

> +};
> +
> +struct artpec8_pcie_phy {
> + const struct artpec8_pcie_phy_data *drv_data;
> + void __iomem *phy_base;
> + void __iomem *pcs_base;
> + void __iomem *elbi_base;
> + struct clk *soc_pll_clk;
> + struct regmap *sysreg;
> + u32 lcpll_ref_clk;
> + const char *mode;
> + u32 num_lanes;
> +};
> +
> +enum artpec8_pcie_ref_clk {
> + REF_CLK_FROM_XO = 0,
> + REF_CLK_FROM_IO,
> + REF_CLK_RESERVED,
> + REF_CLK_FROM_SOC_PLL,
> + REF_CLK_MAX
> +};
> +
> +struct artpec8_pcie_phy_tune_reg {
> + u32 offset;
> + u32 val;
> +};
> +
> +/* ARTPEC-8 PCIe Gen4 x2 PHY CMN register settings */
> +struct artpec8_pcie_phy_tune_reg cmn_regs[] = {

static const

> + {PCIE_PHY_CMN_REG004, 0x65},
> + {PCIE_PHY_CMN_REG00B, 0x18},
> + {PCIE_PHY_CMN_REG016, 0x0E},
> + {PCIE_PHY_CMN_REG01C, 0x4F},
> + {PCIE_PHY_CMN_REG021, 0x01},
> + {PCIE_PHY_CMN_REG024, 0x58},
> + {PCIE_PHY_CMN_REG025, 0x98},
> + {PCIE_PHY_CMN_REG0E6, 0x00},
> + {PCIE_PHY_CMN_REG0E7, 0x00},
> + {PCIE_PHY_CMN_REG0E8, 0x3F},
> + {PCIE_PHY_CMN_REG0E9, 0x3F},
> + {PCIE_PHY_CMN_REG0EA, 0xFF},
> + {PCIE_PHY_CMN_REG0EB, 0xFF},
> + {PCIE_PHY_CMN_REG0EC, 0x42},
> + {PCIE_PHY_CMN_REG0EE, 0x3F},
> + {PCIE_PHY_CMN_REG0EF, 0x7F},
> + {PCIE_PHY_CMN_REG0F1, 0x02},
> + {PCIE_PHY_CMN_REG0F3, 0xFF},
> + {PCIE_PHY_CMN_REG0F4, 0xFF},
> + {PCIE_PHY_CMN_REG131, 0x01},
> + {PCIE_PHY_CMN_REG17B, 0xC0},
> + {PCIE_PHY_CMN_REG17D, 0xAF},
> + {PCIE_PHY_CMN_REG190, 0x27},
> + {PCIE_PHY_CMN_REG191, 0x0F},
> + {PCIE_PHY_CMN_REG192, 0x3F},
> + {PCIE_PHY_CMN_REG1C7, 0x05},
> + {PCIE_PHY_CMN_REG1DF, 0x28},
> + {PCIE_PHY_CMN_REG1E0, 0x28},
> +};
> +
> +/* ARTPEC-8 PCIe Gen4 x2 PHY lane register settings */
> +struct artpec8_pcie_phy_tune_reg lane_regs[] = {

Same.

> + {PCIE_PHY_TRSV_REG22D, 0x00},
> + {PCIE_PHY_TRSV_REG23E, 0x00},
> + {PCIE_PHY_TRSV_REG2A5, 0x73},
> + {PCIE_PHY_TRSV_REG3E3, 0x7B},
> + {PCIE_PHY_TRSV_REG3ED, 0x4B},
> + {PCIE_PHY_TRSV_REG20B, 0x02},
> + {PCIE_PHY_TRSV_REG20C, 0xEA},
> + {PCIE_PHY_TRSV_REG234, 0x7A},
> + {PCIE_PHY_TRSV_REG235, 0x1C},
> + {PCIE_PHY_TRSV_REG237, 0x10},
> + {PCIE_PHY_TRSV_REG239, 0x68},
> + {PCIE_PHY_TRSV_REG23A, 0xC0},
> + {PCIE_PHY_TRSV_REG23B, 0x0B},
> + {PCIE_PHY_TRSV_REG24B, 0x00},
> + {PCIE_PHY_TRSV_REG25D, 0x07},
> + {PCIE_PHY_TRSV_REG262, 0x07},
> + {PCIE_PHY_TRSV_REG271, 0x23},
> + {PCIE_PHY_TRSV_REG272, 0x5E},
> + {PCIE_PHY_TRSV_REG27C, 0x8C},
> + {PCIE_PHY_TRSV_REG27D, 0x5B},
> + {PCIE_PHY_TRSV_REG27E, 0x2C},
> + {PCIE_PHY_TRSV_REG284, 0x33},
> + {PCIE_PHY_TRSV_REG289, 0xD4},
> + {PCIE_PHY_TRSV_REG28A, 0xCC},
> + {PCIE_PHY_TRSV_REG28B, 0xD9},
> + {PCIE_PHY_TRSV_REG28C, 0xDC},
> + {PCIE_PHY_TRSV_REG28E, 0xC6},
> + {PCIE_PHY_TRSV_REG28F, 0x90},
> + {PCIE_PHY_TRSV_REG290, 0x4D},
> + {PCIE_PHY_TRSV_REG291, 0x19},
> + {PCIE_PHY_TRSV_REG292, 0x1C},
> + {PCIE_PHY_TRSV_REG294, 0x05},
> + {PCIE_PHY_TRSV_REG295, 0x10},
> + {PCIE_PHY_TRSV_REG296, 0x0C},
> + {PCIE_PHY_TRSV_REG297, 0x19},
> + {PCIE_PHY_TRSV_REG298, 0x04},
> + {PCIE_PHY_TRSV_REG29B, 0x03},
> + {PCIE_PHY_TRSV_REG29C, 0x1B},
> + {PCIE_PHY_TRSV_REG29D, 0x1B},
> + {PCIE_PHY_TRSV_REG29E, 0x1F},
> + {PCIE_PHY_TRSV_REG2AA, 0x00},
> + {PCIE_PHY_TRSV_REG2AE, 0x1F},
> + {PCIE_PHY_TRSV_REG2C2, 0x25},
> + {PCIE_PHY_TRSV_REG2C6, 0x10},
> + {PCIE_PHY_TRSV_REG2C7, 0x06},
> + {PCIE_PHY_TRSV_REG2CB, 0x10},
> + {PCIE_PHY_TRSV_REG2CC, 0x06},
> + {PCIE_PHY_TRSV_REG2CD, 0x20},
> + {PCIE_PHY_TRSV_REG2CE, 0x27},
> + {PCIE_PHY_TRSV_REG2D0, 0x10},
> + {PCIE_PHY_TRSV_REG2CF, 0x0A},
> + {PCIE_PHY_TRSV_REG2E0, 0x01},
> + {PCIE_PHY_TRSV_REG2E9, 0x11},
> + {PCIE_PHY_TRSV_REG2EA, 0x05},
> + {PCIE_PHY_TRSV_REG2EB, 0x4C},
> + {PCIE_PHY_TRSV_REG315, 0x18},
> + {PCIE_PHY_TRSV_REG317, 0x86},
> + {PCIE_PHY_TRSV_REG319, 0x8E},
> + {PCIE_PHY_TRSV_REG364, 0x00},
> + {PCIE_PHY_TRSV_REG36C, 0x03},
> + {PCIE_PHY_TRSV_REG36D, 0x04},
> + {PCIE_PHY_TRSV_REG37E, 0x06},
> + {PCIE_PHY_TRSV_REG37F, 0x04},
> + {PCIE_PHY_TRSV_REG38F, 0x40},
> + {PCIE_PHY_TRSV_REG391, 0x8B},
> + {PCIE_PHY_TRSV_REG39C, 0xFF},
> + {PCIE_PHY_TRSV_REG3A8, 0x02},
> + {PCIE_PHY_TRSV_REG3E0, 0x93},
> + {PCIE_PHY_TRSV_REG3E1, 0x79},
> + {PCIE_PHY_TRSV_REG3E7, 0xF5},
> + {PCIE_PHY_TRSV_REG3E9, 0x75},
> + {PCIE_PHY_TRSV_REG3EA, 0x0D},
> + {PCIE_PHY_TRSV_REG3EE, 0xE2},
> + {PCIE_PHY_TRSV_REG3EF, 0x6F},
> + {PCIE_PHY_TRSV_REG3F0, 0x3D}
> +};
> +#define PCIE_PHY_CMN_REG17E 0x5F8
> +#define PCIE_PHY_CMN_REG180 0x600
> +#define PCIE_PHY_CMN_REG181 0x604
> +#define PCIE_PHY_CMN_REG182 0x608
> +#define PCIE_PHY_CMN_REG183 0x60C
> +#define PCIE_PHY_CMN_REG184 0x610
> +#define PCIE_PHY_CMN_REG185 0x614
> +#define PCIE_PHY_CMN_REG186 0x618
> +#define PCIE_PHY_CMN_REG187 0x61C

Defines go to the top, before any type declarations.

> +
> +struct artpec8_pcie_phy_tune_reg rx_preset_regs[] = {

Same.

> + /* 0 */
> + {PCIE_PHY_CMN_REG17E, 0x00},
> + {PCIE_PHY_CMN_REG180, 0x23},
> + {PCIE_PHY_CMN_REG181, 0x44},
> + {PCIE_PHY_CMN_REG182, 0x61},
> + {PCIE_PHY_CMN_REG183, 0x55},
> + {PCIE_PHY_CMN_REG184, 0x14},
> + {PCIE_PHY_CMN_REG185, 0x23},
> + {PCIE_PHY_CMN_REG186, 0x1A},
> + {PCIE_PHY_CMN_REG187, 0x04},
> + {PCIE_PHY_CMN_REG17E, 0x04},
> + {PCIE_PHY_CMN_REG17E, 0x00},
> + /* 1 */
> + {PCIE_PHY_CMN_REG17E, 0x08},
> + {PCIE_PHY_CMN_REG181, 0x42},
> + {PCIE_PHY_CMN_REG17E, 0x0C},
> + {PCIE_PHY_CMN_REG17E, 0x08},
> + /* 2 */
> + {PCIE_PHY_CMN_REG17E, 0x10},
> + {PCIE_PHY_CMN_REG181, 0x40},
> + {PCIE_PHY_CMN_REG17E, 0x14},
> + {PCIE_PHY_CMN_REG17E, 0x10},
> + /* 3 */
> + {PCIE_PHY_CMN_REG17E, 0x18},
> + {PCIE_PHY_CMN_REG181, 0x45},
> + {PCIE_PHY_CMN_REG17E, 0x1C},
> + {PCIE_PHY_CMN_REG17E, 0x18},
> + /* 4 */
> + {PCIE_PHY_CMN_REG17E, 0x20},
> + {PCIE_PHY_CMN_REG181, 0x46},
> + {PCIE_PHY_CMN_REG17E, 0x24},
> + {PCIE_PHY_CMN_REG17E, 0x20},
> + /* 5 */
> + {PCIE_PHY_CMN_REG17E, 0x28},
> + {PCIE_PHY_CMN_REG181, 0x48},
> + {PCIE_PHY_CMN_REG17E, 0x2C},
> + {PCIE_PHY_CMN_REG17E, 0x28},
> + /* 6 */
> + {PCIE_PHY_CMN_REG17E, 0x30},
> + {PCIE_PHY_CMN_REG181, 0x4A},
> + {PCIE_PHY_CMN_REG17E, 0x34},
> + {PCIE_PHY_CMN_REG17E, 0x30},
> + /* 7 */
> + {PCIE_PHY_CMN_REG17E, 0x38},
> + {PCIE_PHY_CMN_REG181, 0x4C},
> + {PCIE_PHY_CMN_REG17E, 0x3C},
> + {PCIE_PHY_CMN_REG17E, 0x38},
> + /* 8 */
> + {PCIE_PHY_CMN_REG17E, 0x40},
> + {PCIE_PHY_CMN_REG180, 0x20},
> + {PCIE_PHY_CMN_REG181, 0x20},
> + {PCIE_PHY_CMN_REG182, 0x01},
> + {PCIE_PHY_CMN_REG17E, 0x44},
> + {PCIE_PHY_CMN_REG17E, 0x40},
> + /* 9 */
> + {PCIE_PHY_CMN_REG17E, 0x48},
> + {PCIE_PHY_CMN_REG180, 0x20},
> + {PCIE_PHY_CMN_REG181, 0x21},
> + {PCIE_PHY_CMN_REG182, 0x01},
> + {PCIE_PHY_CMN_REG17E, 0x4C},
> + {PCIE_PHY_CMN_REG17E, 0x48},
> + /* 10 */
> + {PCIE_PHY_CMN_REG17E, 0x50},
> + {PCIE_PHY_CMN_REG180, 0x24},
> + {PCIE_PHY_CMN_REG181, 0x80},
> + {PCIE_PHY_CMN_REG182, 0x41},
> + {PCIE_PHY_CMN_REG183, 0xAF},
> + {PCIE_PHY_CMN_REG184, 0x26},
> + {PCIE_PHY_CMN_REG185, 0x34},
> + {PCIE_PHY_CMN_REG186, 0x24},
> + {PCIE_PHY_CMN_REG187, 0x06},
> + {PCIE_PHY_CMN_REG17E, 0x54},
> + {PCIE_PHY_CMN_REG17E, 0x50},
> + /* 11 */
> + {PCIE_PHY_CMN_REG17E, 0x58},
> + {PCIE_PHY_CMN_REG181, 0x81},
> + {PCIE_PHY_CMN_REG17E, 0x5C},
> + {PCIE_PHY_CMN_REG17E, 0x58},
> + /* 12 */
> + {PCIE_PHY_CMN_REG17E, 0x60},
> + {PCIE_PHY_CMN_REG181, 0x82},
> + {PCIE_PHY_CMN_REG17E, 0x64},
> + {PCIE_PHY_CMN_REG17E, 0x60},
> + /* 13 */
> + {PCIE_PHY_CMN_REG17E, 0x68},
> + {PCIE_PHY_CMN_REG181, 0x83},
> + {PCIE_PHY_CMN_REG17E, 0x6C},
> + {PCIE_PHY_CMN_REG17E, 0x68},
> + /* 14 */
> + {PCIE_PHY_CMN_REG17E, 0x70},
> + {PCIE_PHY_CMN_REG181, 0x84},
> + {PCIE_PHY_CMN_REG17E, 0x74},
> + {PCIE_PHY_CMN_REG17E, 0x70},
> + /* 15 */
> + {PCIE_PHY_CMN_REG17E, 0x78},
> + {PCIE_PHY_CMN_REG180, 0x24},
> + {PCIE_PHY_CMN_REG181, 0x85},
> + {PCIE_PHY_CMN_REG182, 0x80},
> + {PCIE_PHY_CMN_REG183, 0x7F},
> + {PCIE_PHY_CMN_REG184, 0x2D},
> + {PCIE_PHY_CMN_REG185, 0x34},
> + {PCIE_PHY_CMN_REG186, 0x24},
> + {PCIE_PHY_CMN_REG187, 0x05},
> + {PCIE_PHY_CMN_REG17E, 0x7C},
> + {PCIE_PHY_CMN_REG17E, 0x78},
> + /* 16 */
> + {PCIE_PHY_CMN_REG17E, 0x80},
> + {PCIE_PHY_CMN_REG181, 0x86},
> + {PCIE_PHY_CMN_REG17E, 0x84},
> + {PCIE_PHY_CMN_REG17E, 0x80},
> + /* 17 */
> + {PCIE_PHY_CMN_REG17E, 0x88},
> + {PCIE_PHY_CMN_REG181, 0x87},
> + {PCIE_PHY_CMN_REG17E, 0x8C},
> + {PCIE_PHY_CMN_REG17E, 0x88},
> + /* 18 */
> + {PCIE_PHY_CMN_REG17E, 0x90},
> + {PCIE_PHY_CMN_REG181, 0x88},
> + {PCIE_PHY_CMN_REG17E, 0x94},
> + {PCIE_PHY_CMN_REG17E, 0x90},
> + /* 19 */
> + {PCIE_PHY_CMN_REG17E, 0x98},
> + {PCIE_PHY_CMN_REG181, 0x89},
> + {PCIE_PHY_CMN_REG17E, 0x9C},
> + {PCIE_PHY_CMN_REG17E, 0x98},
> +};
> +
> +
> +static void artpec8_pcie_phy_reg_writel(void __iomem *base, u32 val, u32 reg)
> +{
> + writel(val, base + reg);

No, do not create wrappers on writel. Remove entire function.

> +};
> +
> +static u32 artpec8_pcie_phy_reg_readl(void __iomem *base, u32 reg)
> +{
> + return readl(base + reg);

Ditto

> +};
> +
> +static void artpec8_pcie_phy_reg_update(void __iomem *base, u32 mask,
> + u32 update, u32 reg)
> +{
> + u32 val;
> +
> + val = artpec8_pcie_phy_reg_readl(base, reg);
> + val &= ~(mask);
> + val |= update;
> + artpec8_pcie_phy_reg_writel(base, val, reg);
> +};
> +

(...)

> +static int artpec8_pcie_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct artpec8_pcie_phy *artpec8_phy;
> + struct phy *generic_phy;
> + struct phy_provider *phy_provider;
> + const struct artpec8_pcie_phy_data *drv_data;
> +
> + drv_data = of_device_get_match_data(dev);
> + if (!drv_data)
> + return -ENODEV;
> +
> + artpec8_phy = devm_kzalloc(dev, sizeof(*artpec8_phy), GFP_KERNEL);
> + if (!artpec8_phy)
> + return -ENOMEM;
> +
> + /* reference clock */
> + if (of_property_read_u32(dev->of_node, "lcpll-ref-clk",

No, really, no...

All properties *must* be documented in the bindings. You cannot sneak in
some stuff here...

> + &artpec8_phy->lcpll_ref_clk)) {
> + return -EINVAL;
> + }
> + /* PLL SOC reference clock */
> + if (artpec8_phy->lcpll_ref_clk == REF_CLK_FROM_SOC_PLL) {
> + artpec8_phy->soc_pll_clk = devm_clk_get(dev, "ref_clk");
> + if (IS_ERR(artpec8_phy->soc_pll_clk))
> + return -EINVAL;
> + clk_prepare_enable(artpec8_phy->soc_pll_clk);
> + }
> +
> + /* link mode */
> + if (of_property_read_string(dev->of_node, "mode", &artpec8_phy->mode))
> + return -EINVAL;
> +
> + /* number of lanes */
> + if (of_property_read_u32(dev->of_node, "num-lanes",
> + &artpec8_phy->num_lanes))
> + return -EINVAL;
> +
> + if (artpec8_phy->num_lanes > LANE_MAX)
> + return -EINVAL;
> +
> + /* PHY base register */
> + artpec8_phy->phy_base = devm_platform_ioremap_resource_byname(pdev, "phy");
> + if (IS_ERR(artpec8_phy->phy_base))
> + return PTR_ERR(artpec8_phy->phy_base);
> +
> + /* PCS base register */
> + artpec8_phy->pcs_base = devm_platform_ioremap_resource_byname(pdev, "pcs");
> + if (IS_ERR(artpec8_phy->pcs_base))
> + return PTR_ERR(artpec8_phy->pcs_base);
> +
> + /* sysreg regmap handle, need to change using smc */
> + artpec8_phy->sysreg =
> + syscon_regmap_lookup_by_phandle(dev->of_node,
> + "samsung,fsys-sysreg");

Nope. Usage of undocumented properties. Please post your DTS changes, so
we can validate the user of this driver.


Best regards,
Krzysztof

2022-06-08 08:39:17

by Wangseok Lee

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] phy: Add ARTPEC-8 PCIe PHY driver

On 06/06/2022 19:34, Krzysztof Kozlowski wrote:
> On 03/06/2022 04:38, Wangseok Lee wrote:
>> Add support Axis, ARTPEC-8 SoC.
>> ARTPEC-8 is the SoC platform of Axis Communications.
>> This is based on arm64 and support GEN4 & 2lane.
>> This driver provides PHY interface for ARTPEC-8 SoC PCIe controller,
>> based on Samsung PCIe PHY IP.
>> 
>> Main changes since v1 [1]:
>> -change folder name of phy driver to axis from artpec
>> 
>> Signed-off-by: Wangseok Lee <[email protected]>
>> ---
>>  drivers/phy/Kconfig                 |   1 +
>>  drivers/phy/Makefile                |   1 +
>>  drivers/phy/axis/Kconfig            |   9 +
>>  drivers/phy/axis/Makefile           |   2 +
>>  drivers/phy/axis/phy-artpec8-pcie.c | 806 ++++++++++++++++++++++++++++++++++++
>>  5 files changed, 819 insertions(+)
>>  create mode 100644 drivers/phy/axis/Kconfig
>>  create mode 100644 drivers/phy/axis/Makefile
>>  create mode 100644 drivers/phy/axis/phy-artpec8-pcie.c
>> 
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 300b0f2..92b8232 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -73,6 +73,7 @@ config PHY_CAN_TRANSCEIVER
>>  
>>  source "drivers/phy/allwinner/Kconfig"
>>  source "drivers/phy/amlogic/Kconfig"
>> +source "drivers/phy/axis/Kconfig"
>>  source "drivers/phy/broadcom/Kconfig"
>>  source "drivers/phy/cadence/Kconfig"
>>  source "drivers/phy/freescale/Kconfig"
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 01e9eff..808c055e 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -12,6 +12,7 @@ obj-$(CONFIG_PHY_PISTACHIO_USB)                += phy-pistachio-usb.o
>>  obj-$(CONFIG_USB_LGM_PHY)                += phy-lgm-usb.o
>>  obj-y                                        += allwinner/        \
>>                                             amlogic/        \
>> +                                           axis/                \
>>                                             broadcom/        \
>>                                             cadence/        \
>>                                             freescale/        \
>> diff --git a/drivers/phy/axis/Kconfig b/drivers/phy/axis/Kconfig
>> new file mode 100644
>> index 0000000..7198b93
>> --- /dev/null
>> +++ b/drivers/phy/axis/Kconfig
>> @@ -0,0 +1,9 @@
>> +config PHY_ARTPEC8_PCIE
>> +        bool "ARTPEC-8 PCIe PHY driver"
>> +        depends on OF && (ARCH_ARTPEC8 || COMPILE_TEST)
>> +        select GENERIC_PHY
>> +        help
>> +          Enable PCIe PHY support for ARTPEC-8 SoC.
>> +          This driver provides PHY interface for ARTPEC-8 SoC
>> +          PCIe controller.
>> +          This is based on Samsung PCIe PHY IP.
>> diff --git a/drivers/phy/axis/Makefile b/drivers/phy/axis/Makefile
>> new file mode 100644
>> index 0000000..45d853c
>> --- /dev/null
>> +++ b/drivers/phy/axis/Makefile
>> @@ -0,0 +1,2 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_PHY_ARTPEC8_PCIE)                += phy-artpec8-pcie.o
>> diff --git a/drivers/phy/axis/phy-artpec8-pcie.c b/drivers/phy/axis/phy-artpec8-pcie.c
>> new file mode 100644
>> index 0000000..2742301
>> --- /dev/null
>> +++ b/drivers/phy/axis/phy-artpec8-pcie.c
>> @@ -0,0 +1,806 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * PHY provider for ARTPEC-8 PCIe controller
>> + *
>> + * Copyright (C) 2019 Samsung Electronics Co., Ltd.
>> + *                http://www.samsung.com
>> + *
>> + * Author: Jaeho Cho <[email protected]>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/init.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/regmap.h>
>> +#include <linux/debugfs.h>
>> +
>> +/* ARTPEC-8 PCIe PHY registers */
>> +/* CMN registers */
>> +#define PCIE_PHY_CMN_REG004                0x10
>> +#define PCIE_PHY_CMN_REG00B                0x2C
>> +#define PCIE_PHY_CMN_REG016                0x58
>> +#define PCIE_PHY_CMN_REG01C                0x70
>> +#define PCIE_PHY_CMN_REG021                0x84
>> +#define PCIE_PHY_CMN_REG024                0x90
>> +#define PCIE_PHY_CMN_REG025                0x94
>> +#define PCIE_PHY_CMN_REG0E6                0x398
>> +#define PCIE_PHY_CMN_REG0E7                0x39C
>> +#define PCIE_PHY_CMN_REG0E8                0x3A0
>> +#define PCIE_PHY_CMN_REG0E9                0x3A4
>> +#define PCIE_PHY_CMN_REG0EA                0x3A8
>> +#define PCIE_PHY_CMN_REG0EB                0x3AC
>> +#define PCIE_PHY_CMN_REG0EC                0x3B0
>> +#define PCIE_PHY_CMN_REG0EE                0x3B8
>> +#define PCIE_PHY_CMN_REG0EF                0x3BC
>> +#define PCIE_PHY_CMN_REG0F1                0x3C4
>> +#define PCIE_PHY_CMN_REG0F3                0x3CC
>> +#define PCIE_PHY_CMN_REG0F4                0x3D0
>> +
>> +#define PCIE_PHY_CMN_REG101                0x404
>> +#define OV_I_CMN_RSTN                        BIT(4)
>> +#define OV_I_INIT_RSTN                        BIT(6)
>> +
>> +#define PCIE_PHY_CMN_REG131                0x4C4
>> +#define PCIE_PHY_CMN_REG17B                0x5EC
>> +#define PCIE_PHY_CMN_REG17D                0x5F4
>> +#define PCIE_PHY_CMN_REG190                0x640
>> +#define PCIE_PHY_CMN_REG191                0x644
>> +#define PCIE_PHY_CMN_REG192                0x648
>> +#define PCIE_PHY_CMN_REG1C7                0x71C
>> +#define PCIE_PHY_CMN_REG1DF                0x77C
>> +#define PCIE_PHY_CMN_REG1E0                0x780
>> +
>> +#define PCIE_PHY_CMN_REG0B1                0x2C4
>> +#define ANA_ROPLL_REF_DIG_CLK_SEL        BIT(2)
>> +
>> +/* External clock */
>> +#define PCIE_PHY_CMN_REG14D                0x534
>> +#define PCIE_AUX_RX_MODE_EXTEND                BIT(7)
>> +
>> +#define PCIE_PHY_CMN_REG0D9                0x364
>> +#define ANA_AUX_EXT_REF_CLK_SEL                BIT(4)
>> +
>> +#define PCIE_PHY_CMN_REG10F                0x43C
>> +#define AUX_PLL_EN_EXTEND                BIT(4)
>> +
>> +#define PCIE_PHY_CMN_REG11E                0x478
>> +#define AUX2_PLL_EN_EXTEND                BIT(3)
>> +
>> +#define PCIE_PHY_CMN_REG0D4                0x350
>> +#define OV_S_ANA_AUX_EN                        BIT(3)
>> +#define OV_I_ANA_AUX_EN                        BIT(2)
>> +
>> +/* LANE registers */
>> +#define PCIE_PHY_TRSV_REG22D                0x8B4
>> +#define PCIE_PHY_TRSV_REG23E                0x8F8
>> +#define PCIE_PHY_TRSV_REG2A5                0xA94
>> +#define PCIE_PHY_TRSV_REG3E3                0xF8C
>> +#define PCIE_PHY_TRSV_REG3ED                0xFB4
>> +#define PCIE_PHY_TRSV_REG20B                0x82C
>> +#define PCIE_PHY_TRSV_REG20C                0x830
>> +#define PCIE_PHY_TRSV_REG234                0x8D0
>> +#define PCIE_PHY_TRSV_REG235                0x8D4
>> +#define PCIE_PHY_TRSV_REG237                0x8DC
>> +#define PCIE_PHY_TRSV_REG239                0x8E4
>> +#define PCIE_PHY_TRSV_REG23A                0x8E8
>> +#define PCIE_PHY_TRSV_REG23B                0x8EC
>> +#define PCIE_PHY_TRSV_REG24B                0x92C
>> +#define PCIE_PHY_TRSV_REG25D                0x974
>> +#define PCIE_PHY_TRSV_REG262                0x988
>> +#define PCIE_PHY_TRSV_REG271                0x9C4
>> +#define PCIE_PHY_TRSV_REG272                0x9C8
>> +#define PCIE_PHY_TRSV_REG27C                0x9F0
>> +#define PCIE_PHY_TRSV_REG27D                0x9F4
>> +#define PCIE_PHY_TRSV_REG27E                0x9F8
>> +#define PCIE_PHY_TRSV_REG284                0xA10
>> +#define PCIE_PHY_TRSV_REG289                0xA24
>> +#define PCIE_PHY_TRSV_REG28A                0xA28
>> +#define PCIE_PHY_TRSV_REG28B                0xA2C
>> +#define PCIE_PHY_TRSV_REG28C                0xA30
>> +#define PCIE_PHY_TRSV_REG28E                0xA38
>> +#define PCIE_PHY_TRSV_REG28F                0xA3C
>> +#define PCIE_PHY_TRSV_REG290                0xA40
>> +#define PCIE_PHY_TRSV_REG291                0xA44
>> +#define PCIE_PHY_TRSV_REG292                0xA48
>> +#define PCIE_PHY_TRSV_REG294                0xA50
>> +#define PCIE_PHY_TRSV_REG295                0xA54
>> +#define PCIE_PHY_TRSV_REG296                0xA58
>> +#define PCIE_PHY_TRSV_REG297                0xA5C
>> +#define PCIE_PHY_TRSV_REG298                0xA60
>> +#define PCIE_PHY_TRSV_REG29B                0xA6C
>> +#define PCIE_PHY_TRSV_REG29C                0xA70
>> +#define PCIE_PHY_TRSV_REG29D                0xA74
>> +#define PCIE_PHY_TRSV_REG29E                0xA78
>> +#define PCIE_PHY_TRSV_REG2AA                0xAA8
>> +#define PCIE_PHY_TRSV_REG2AE                0xAB8
>> +#define PCIE_PHY_TRSV_REG2C2                0xB08
>> +#define PCIE_PHY_TRSV_REG2C6                0xB18
>> +#define PCIE_PHY_TRSV_REG2C7                0xB1C
>> +#define PCIE_PHY_TRSV_REG2CB                0xB2C
>> +#define PCIE_PHY_TRSV_REG2CC                0xB30
>> +#define PCIE_PHY_TRSV_REG2CD                0xB34
>> +#define PCIE_PHY_TRSV_REG2CE                0xB38
>> +#define PCIE_PHY_TRSV_REG2D0                0xB40
>> +#define PCIE_PHY_TRSV_REG2CF                0xB3C
>> +#define PCIE_PHY_TRSV_REG2E0                0xB80
>> +#define PCIE_PHY_TRSV_REG2E9                0xBA4
>> +#define PCIE_PHY_TRSV_REG2EA                0xBA8
>> +#define PCIE_PHY_TRSV_REG2EB                0xBAC
>> +#define PCIE_PHY_TRSV_REG315                0xC54
>> +#define PCIE_PHY_TRSV_REG317                0xC5C
>> +#define PCIE_PHY_TRSV_REG319                0xC64
>> +#define PCIE_PHY_TRSV_REG364                0xD90
>> +#define PCIE_PHY_TRSV_REG36C                0xDB0
>> +#define PCIE_PHY_TRSV_REG36D                0xDB4
>> +#define PCIE_PHY_TRSV_REG37E                0xDF8
>> +#define PCIE_PHY_TRSV_REG37F                0xDFC
>> +#define PCIE_PHY_TRSV_REG38F                0xE3C
>> +#define PCIE_PHY_TRSV_REG391                0xE44
>> +#define PCIE_PHY_TRSV_REG39C                0xE70
>> +#define PCIE_PHY_TRSV_REG3A8                0xEA0
>> +#define PCIE_PHY_TRSV_REG3E0                0xF80
>> +#define PCIE_PHY_TRSV_REG3E1                0xF84
>> +#define PCIE_PHY_TRSV_REG3E7                0xF9C
>> +#define PCIE_PHY_TRSV_REG3E9                0xFA4
>> +#define PCIE_PHY_TRSV_REG3EA                0xFA8
>> +#define PCIE_PHY_TRSV_REG3EE                0xFB8
>> +#define PCIE_PHY_TRSV_REG3EF                0xFBC
>> +#define PCIE_PHY_TRSV_REG3F0                0xFC0
>> +
>> +#define PCIE_PHY_TRSV_REG2C0                0xB00
>> +#define LN_EQ_CTRL_RX_DATA_HOLD                BIT(5)
>> +
>> +/* RX Preset registers */
>> +#define PCIE_PHY_CMN_REG17E                0x5F8
>> +#define PCIE_PHY_CMN_REG180                0x600
>> +#define PCIE_PHY_CMN_REG181                0x604
>> +#define PCIE_PHY_CMN_REG182                0x608
>> +#define PCIE_PHY_CMN_REG183                0x60C
>> +#define PCIE_PHY_CMN_REG184                0x610
>> +#define PCIE_PHY_CMN_REG185                0x614
>> +#define PCIE_PHY_CMN_REG186                0x618
>> +#define PCIE_PHY_CMN_REG187                0x61C
>> +
>> +
>> +/* ARTPEC-8 PCIe PCS registers */
>> +#define PCIE_PCS_OUT_VEC_4                0x154
>> +#define B1_DYNAMIC                        BIT(3)
>> +
>> +/* ARTPEC-8 SYS REG registers */
>> +#define FSYS_PCIE_CON                        0x424
>> +#define PCIE_PHY_LCPLL_REFCLK_SEL        0x3
>> +#define PCIE_PHY_ROPLL_REFCLK_SEL        (0x3UL << 2)
>> +#define ROPLL_REFCLK_NOT_AVAILABLE        (0x2UL << 2)
>> +#define PCIE_PHY_LN0_REFCLK_PAD_EN        BIT(10)
>> +#define PCIE_PHY_LN1_REFCLK_PAD_EN        BIT(11)
>> +#define PCIE_PHY_PWR_OFF                BIT(7)
>> +
>> +/* ARTPEC-8 Sub Controller registers */
>> +#define SFR_INIT_RSTN                        0x1404
>> +#define SFR_CMN_RSTN                        0x1408
>> +
>> +#define PCIE_PHY_LN0_REG_START                0x800
>> +#define PCIE_PHY_LN0_REG_END                0xFCC
>> +#define OFFSET_PER_LANE                        0x800
>> +
>> +enum artpec8_pcie_phy_num_lanes {
>> +        LANE0 = 0,
>> +        LANE1,
>> +        LANE_MAX
>> +};
>> +
>> +struct artpec8_pcie_phy_data {
>> +        const struct phy_ops        *ops;

> No need for indentation before "*ops". Other places do not use it.

 
Ok. remove unnecessary indentation.
 
>> +};
>> +
> +struct artpec8_pcie_phy {
>> +        const struct artpec8_pcie_phy_data *drv_data;
>> +        void __iomem *phy_base;
>> +        void __iomem *pcs_base;
>> +        void __iomem *elbi_base;
>> +        struct clk *soc_pll_clk;
>> +        struct regmap *sysreg;
>> +        u32 lcpll_ref_clk;
>> +        const char *mode;
>> +        u32 num_lanes;
>> +};
>> +
>> +enum artpec8_pcie_ref_clk {
>> +        REF_CLK_FROM_XO = 0,
>> +        REF_CLK_FROM_IO,
>> +        REF_CLK_RESERVED,
>> +        REF_CLK_FROM_SOC_PLL,
>> +        REF_CLK_MAX
>> +};
>> +
>> +struct artpec8_pcie_phy_tune_reg {
>> +        u32 offset;
>> +        u32 val;
>> +};
>> +
>> +/* ARTPEC-8 PCIe Gen4 x2 PHY CMN register settings */
>> +struct artpec8_pcie_phy_tune_reg cmn_regs[] = {

> static const

 
Ok.
 
>> +        {PCIE_PHY_CMN_REG004, 0x65},
>> +        {PCIE_PHY_CMN_REG00B, 0x18},
>> +        {PCIE_PHY_CMN_REG016, 0x0E},
>> +        {PCIE_PHY_CMN_REG01C, 0x4F},
>> +        {PCIE_PHY_CMN_REG021, 0x01},
>> +        {PCIE_PHY_CMN_REG024, 0x58},
>> +        {PCIE_PHY_CMN_REG025, 0x98},
>> +        {PCIE_PHY_CMN_REG0E6, 0x00},
>> +        {PCIE_PHY_CMN_REG0E7, 0x00},
>> +        {PCIE_PHY_CMN_REG0E8, 0x3F},
>> +        {PCIE_PHY_CMN_REG0E9, 0x3F},
>> +        {PCIE_PHY_CMN_REG0EA, 0xFF},
>> +        {PCIE_PHY_CMN_REG0EB, 0xFF},
>> +        {PCIE_PHY_CMN_REG0EC, 0x42},
>> +        {PCIE_PHY_CMN_REG0EE, 0x3F},
>> +        {PCIE_PHY_CMN_REG0EF, 0x7F},
>> +        {PCIE_PHY_CMN_REG0F1, 0x02},
>> +        {PCIE_PHY_CMN_REG0F3, 0xFF},
>> +        {PCIE_PHY_CMN_REG0F4, 0xFF},
>> +        {PCIE_PHY_CMN_REG131, 0x01},
>> +        {PCIE_PHY_CMN_REG17B, 0xC0},
>> +        {PCIE_PHY_CMN_REG17D, 0xAF},
>> +        {PCIE_PHY_CMN_REG190, 0x27},
>> +        {PCIE_PHY_CMN_REG191, 0x0F},
>> +        {PCIE_PHY_CMN_REG192, 0x3F},
>> +        {PCIE_PHY_CMN_REG1C7, 0x05},
>> +        {PCIE_PHY_CMN_REG1DF, 0x28},
>> +        {PCIE_PHY_CMN_REG1E0, 0x28},
>> +};
>> +
>> +/* ARTPEC-8 PCIe Gen4 x2 PHY lane register settings */
>> +struct artpec8_pcie_phy_tune_reg lane_regs[] = {

> Same.

 
Ok.
 
>> +        {PCIE_PHY_TRSV_REG22D, 0x00},
>> +        {PCIE_PHY_TRSV_REG23E, 0x00},
>> +        {PCIE_PHY_TRSV_REG2A5, 0x73},
>> +        {PCIE_PHY_TRSV_REG3E3, 0x7B},
>> +        {PCIE_PHY_TRSV_REG3ED, 0x4B},
>> +        {PCIE_PHY_TRSV_REG20B, 0x02},
>> +        {PCIE_PHY_TRSV_REG20C, 0xEA},
>> +        {PCIE_PHY_TRSV_REG234, 0x7A},
>> +        {PCIE_PHY_TRSV_REG235, 0x1C},
>> +        {PCIE_PHY_TRSV_REG237, 0x10},
>> +        {PCIE_PHY_TRSV_REG239, 0x68},
>> +        {PCIE_PHY_TRSV_REG23A, 0xC0},
>> +        {PCIE_PHY_TRSV_REG23B, 0x0B},
>> +        {PCIE_PHY_TRSV_REG24B, 0x00},
>> +        {PCIE_PHY_TRSV_REG25D, 0x07},
>> +        {PCIE_PHY_TRSV_REG262, 0x07},
>> +        {PCIE_PHY_TRSV_REG271, 0x23},
>> +        {PCIE_PHY_TRSV_REG272, 0x5E},
>> +        {PCIE_PHY_TRSV_REG27C, 0x8C},
>> +        {PCIE_PHY_TRSV_REG27D, 0x5B},
>> +        {PCIE_PHY_TRSV_REG27E, 0x2C},
>> +        {PCIE_PHY_TRSV_REG284, 0x33},
>> +        {PCIE_PHY_TRSV_REG289, 0xD4},
>> +        {PCIE_PHY_TRSV_REG28A, 0xCC},
>> +        {PCIE_PHY_TRSV_REG28B, 0xD9},
>> +        {PCIE_PHY_TRSV_REG28C, 0xDC},
>> +        {PCIE_PHY_TRSV_REG28E, 0xC6},
>> +        {PCIE_PHY_TRSV_REG28F, 0x90},
>> +        {PCIE_PHY_TRSV_REG290, 0x4D},
>> +        {PCIE_PHY_TRSV_REG291, 0x19},
>> +        {PCIE_PHY_TRSV_REG292, 0x1C},
>> +        {PCIE_PHY_TRSV_REG294, 0x05},
>> +        {PCIE_PHY_TRSV_REG295, 0x10},
>> +        {PCIE_PHY_TRSV_REG296, 0x0C},
>> +        {PCIE_PHY_TRSV_REG297, 0x19},
>> +        {PCIE_PHY_TRSV_REG298, 0x04},
>> +        {PCIE_PHY_TRSV_REG29B, 0x03},
>> +        {PCIE_PHY_TRSV_REG29C, 0x1B},
>> +        {PCIE_PHY_TRSV_REG29D, 0x1B},
>> +        {PCIE_PHY_TRSV_REG29E, 0x1F},
>> +        {PCIE_PHY_TRSV_REG2AA, 0x00},
>> +        {PCIE_PHY_TRSV_REG2AE, 0x1F},
>> +        {PCIE_PHY_TRSV_REG2C2, 0x25},
>> +        {PCIE_PHY_TRSV_REG2C6, 0x10},
>> +        {PCIE_PHY_TRSV_REG2C7, 0x06},
>> +        {PCIE_PHY_TRSV_REG2CB, 0x10},
>> +        {PCIE_PHY_TRSV_REG2CC, 0x06},
>> +        {PCIE_PHY_TRSV_REG2CD, 0x20},
>> +        {PCIE_PHY_TRSV_REG2CE, 0x27},
>> +        {PCIE_PHY_TRSV_REG2D0, 0x10},
>> +        {PCIE_PHY_TRSV_REG2CF, 0x0A},
>> +        {PCIE_PHY_TRSV_REG2E0, 0x01},
>> +        {PCIE_PHY_TRSV_REG2E9, 0x11},
>> +        {PCIE_PHY_TRSV_REG2EA, 0x05},
>> +        {PCIE_PHY_TRSV_REG2EB, 0x4C},
>> +        {PCIE_PHY_TRSV_REG315, 0x18},
>> +        {PCIE_PHY_TRSV_REG317, 0x86},
>> +        {PCIE_PHY_TRSV_REG319, 0x8E},
>> +        {PCIE_PHY_TRSV_REG364, 0x00},
>> +        {PCIE_PHY_TRSV_REG36C, 0x03},
>> +        {PCIE_PHY_TRSV_REG36D, 0x04},
>> +        {PCIE_PHY_TRSV_REG37E, 0x06},
>> +        {PCIE_PHY_TRSV_REG37F, 0x04},
>> +        {PCIE_PHY_TRSV_REG38F, 0x40},
>> +        {PCIE_PHY_TRSV_REG391, 0x8B},
>> +        {PCIE_PHY_TRSV_REG39C, 0xFF},
>> +        {PCIE_PHY_TRSV_REG3A8, 0x02},
>> +        {PCIE_PHY_TRSV_REG3E0, 0x93},
>> +        {PCIE_PHY_TRSV_REG3E1, 0x79},
>> +        {PCIE_PHY_TRSV_REG3E7, 0xF5},
>> +        {PCIE_PHY_TRSV_REG3E9, 0x75},
>> +        {PCIE_PHY_TRSV_REG3EA, 0x0D},
>> +        {PCIE_PHY_TRSV_REG3EE, 0xE2},
>> +        {PCIE_PHY_TRSV_REG3EF, 0x6F},
>> +        {PCIE_PHY_TRSV_REG3F0, 0x3D}
>> +};
>> +#define PCIE_PHY_CMN_REG17E                0x5F8
>> +#define PCIE_PHY_CMN_REG180                0x600
>> +#define PCIE_PHY_CMN_REG181                0x604
>> +#define PCIE_PHY_CMN_REG182                0x608
>> +#define PCIE_PHY_CMN_REG183                0x60C
>> +#define PCIE_PHY_CMN_REG184                0x610
>> +#define PCIE_PHY_CMN_REG185                0x614
>> +#define PCIE_PHY_CMN_REG186                0x618
>> +#define PCIE_PHY_CMN_REG187                0x61C

> Defines go to the top, before any type declarations.

 
Ok.
 
>> +
>> +struct artpec8_pcie_phy_tune_reg rx_preset_regs[] = {

> Same.

 
Ok, static const.
 
>> +        /* 0 */
>> +        {PCIE_PHY_CMN_REG17E, 0x00},
>> +        {PCIE_PHY_CMN_REG180, 0x23},
>> +        {PCIE_PHY_CMN_REG181, 0x44},
>> +        {PCIE_PHY_CMN_REG182, 0x61},
>> +        {PCIE_PHY_CMN_REG183, 0x55},
>> +        {PCIE_PHY_CMN_REG184, 0x14},
>> +        {PCIE_PHY_CMN_REG185, 0x23},
>> +        {PCIE_PHY_CMN_REG186, 0x1A},
>> +        {PCIE_PHY_CMN_REG187, 0x04},
>> +        {PCIE_PHY_CMN_REG17E, 0x04},
>> +        {PCIE_PHY_CMN_REG17E, 0x00},
>> +        /* 1 */
>> +        {PCIE_PHY_CMN_REG17E, 0x08},
>> +        {PCIE_PHY_CMN_REG181, 0x42},
>> +        {PCIE_PHY_CMN_REG17E, 0x0C},
>> +        {PCIE_PHY_CMN_REG17E, 0x08},
>> +        /* 2 */
>> +        {PCIE_PHY_CMN_REG17E, 0x10},
>> +        {PCIE_PHY_CMN_REG181, 0x40},
>> +        {PCIE_PHY_CMN_REG17E, 0x14},
>> +        {PCIE_PHY_CMN_REG17E, 0x10},
>> +        /* 3 */
>> +        {PCIE_PHY_CMN_REG17E, 0x18},
>> +        {PCIE_PHY_CMN_REG181, 0x45},
>> +        {PCIE_PHY_CMN_REG17E, 0x1C},
>> +        {PCIE_PHY_CMN_REG17E, 0x18},
>> +        /* 4 */
>> +        {PCIE_PHY_CMN_REG17E, 0x20},
>> +        {PCIE_PHY_CMN_REG181, 0x46},
>> +        {PCIE_PHY_CMN_REG17E, 0x24},
>> +        {PCIE_PHY_CMN_REG17E, 0x20},
>> +        /* 5 */
>> +        {PCIE_PHY_CMN_REG17E, 0x28},
>> +        {PCIE_PHY_CMN_REG181, 0x48},
>> +        {PCIE_PHY_CMN_REG17E, 0x2C},
>> +        {PCIE_PHY_CMN_REG17E, 0x28},
>> +        /* 6 */
>> +        {PCIE_PHY_CMN_REG17E, 0x30},
>> +        {PCIE_PHY_CMN_REG181, 0x4A},
>> +        {PCIE_PHY_CMN_REG17E, 0x34},
>> +        {PCIE_PHY_CMN_REG17E, 0x30},
>> +        /* 7 */
>> +        {PCIE_PHY_CMN_REG17E, 0x38},
>> +        {PCIE_PHY_CMN_REG181, 0x4C},
>> +        {PCIE_PHY_CMN_REG17E, 0x3C},
>> +        {PCIE_PHY_CMN_REG17E, 0x38},
>> +        /* 8 */
>> +        {PCIE_PHY_CMN_REG17E, 0x40},
>> +        {PCIE_PHY_CMN_REG180, 0x20},
>> +        {PCIE_PHY_CMN_REG181, 0x20},
>> +        {PCIE_PHY_CMN_REG182, 0x01},
>> +        {PCIE_PHY_CMN_REG17E, 0x44},
>> +        {PCIE_PHY_CMN_REG17E, 0x40},
>> +        /* 9 */
>> +        {PCIE_PHY_CMN_REG17E, 0x48},
>> +        {PCIE_PHY_CMN_REG180, 0x20},
>> +        {PCIE_PHY_CMN_REG181, 0x21},
>> +        {PCIE_PHY_CMN_REG182, 0x01},
>> +        {PCIE_PHY_CMN_REG17E, 0x4C},
>> +        {PCIE_PHY_CMN_REG17E, 0x48},
>> +        /* 10 */
>> +        {PCIE_PHY_CMN_REG17E, 0x50},
>> +        {PCIE_PHY_CMN_REG180, 0x24},
>> +        {PCIE_PHY_CMN_REG181, 0x80},
>> +        {PCIE_PHY_CMN_REG182, 0x41},
>> +        {PCIE_PHY_CMN_REG183, 0xAF},
>> +        {PCIE_PHY_CMN_REG184, 0x26},
>> +        {PCIE_PHY_CMN_REG185, 0x34},
>> +        {PCIE_PHY_CMN_REG186, 0x24},
>> +        {PCIE_PHY_CMN_REG187, 0x06},
>> +        {PCIE_PHY_CMN_REG17E, 0x54},
>> +        {PCIE_PHY_CMN_REG17E, 0x50},
>> +        /* 11 */
>> +        {PCIE_PHY_CMN_REG17E, 0x58},
>> +        {PCIE_PHY_CMN_REG181, 0x81},
>> +        {PCIE_PHY_CMN_REG17E, 0x5C},
>> +        {PCIE_PHY_CMN_REG17E, 0x58},
>> +        /* 12 */
>> +        {PCIE_PHY_CMN_REG17E, 0x60},
>> +        {PCIE_PHY_CMN_REG181, 0x82},
>> +        {PCIE_PHY_CMN_REG17E, 0x64},
>> +        {PCIE_PHY_CMN_REG17E, 0x60},
>> +        /* 13 */
>> +        {PCIE_PHY_CMN_REG17E, 0x68},
>> +        {PCIE_PHY_CMN_REG181, 0x83},
>> +        {PCIE_PHY_CMN_REG17E, 0x6C},
>> +        {PCIE_PHY_CMN_REG17E, 0x68},
>> +        /* 14 */
>> +        {PCIE_PHY_CMN_REG17E, 0x70},
>> +        {PCIE_PHY_CMN_REG181, 0x84},
>> +        {PCIE_PHY_CMN_REG17E, 0x74},
>> +        {PCIE_PHY_CMN_REG17E, 0x70},
>> +        /* 15 */
>> +        {PCIE_PHY_CMN_REG17E, 0x78},
>> +        {PCIE_PHY_CMN_REG180, 0x24},
>> +        {PCIE_PHY_CMN_REG181, 0x85},
>> +        {PCIE_PHY_CMN_REG182, 0x80},
>> +        {PCIE_PHY_CMN_REG183, 0x7F},
>> +        {PCIE_PHY_CMN_REG184, 0x2D},
>> +        {PCIE_PHY_CMN_REG185, 0x34},
>> +        {PCIE_PHY_CMN_REG186, 0x24},
>> +        {PCIE_PHY_CMN_REG187, 0x05},
>> +        {PCIE_PHY_CMN_REG17E, 0x7C},
>> +        {PCIE_PHY_CMN_REG17E, 0x78},
>> +        /* 16 */
>> +        {PCIE_PHY_CMN_REG17E, 0x80},
>> +        {PCIE_PHY_CMN_REG181, 0x86},
>> +        {PCIE_PHY_CMN_REG17E, 0x84},
>> +        {PCIE_PHY_CMN_REG17E, 0x80},
>> +        /* 17 */
>> +        {PCIE_PHY_CMN_REG17E, 0x88},
>> +        {PCIE_PHY_CMN_REG181, 0x87},
>> +        {PCIE_PHY_CMN_REG17E, 0x8C},
>> +        {PCIE_PHY_CMN_REG17E, 0x88},
>> +        /* 18 */
>> +        {PCIE_PHY_CMN_REG17E, 0x90},
>> +        {PCIE_PHY_CMN_REG181, 0x88},
>> +        {PCIE_PHY_CMN_REG17E, 0x94},
>> +        {PCIE_PHY_CMN_REG17E, 0x90},
>> +        /* 19 */
>> +        {PCIE_PHY_CMN_REG17E, 0x98},
>> +        {PCIE_PHY_CMN_REG181, 0x89},
>> +        {PCIE_PHY_CMN_REG17E, 0x9C},
>> +        {PCIE_PHY_CMN_REG17E, 0x98},
>> +};
>> +
>> +
>> +static void artpec8_pcie_phy_reg_writel(void __iomem *base, u32 val, u32 reg)
>> +{
>> +        writel(val, base + reg);

> No, do not create wrappers on writel. Remove entire function.
>
 
Ok, i will remove wrappers function and direct call writel.
 
>> +};
>> +
>> +static u32 artpec8_pcie_phy_reg_readl(void __iomem *base, u32 reg)
>> +{
>> +        return readl(base + reg);

> Ditto

 
Ok, i will remove wrappers function and direct call readl.
 
>> +};
>> +
>> +static void artpec8_pcie_phy_reg_update(void __iomem *base, u32 mask,
>> +                                        u32 update, u32 reg)
>> +{
>> +        u32 val;
>> +
>> +        val = artpec8_pcie_phy_reg_readl(base, reg);
>> +        val &= ~(mask);
>> +        val |= update;
>> +        artpec8_pcie_phy_reg_writel(base, val, reg);
>> +};
>> +

> (...)

>> +static int artpec8_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +        struct device *dev = &pdev->dev;
>> +        struct artpec8_pcie_phy *artpec8_phy;
>> +        struct phy *generic_phy;
>> +        struct phy_provider *phy_provider;
>> +        const struct artpec8_pcie_phy_data *drv_data;
>> +
>> +        drv_data = of_device_get_match_data(dev);
>> +        if (!drv_data)
>> +                return -ENODEV;
>> +
>> +        artpec8_phy = devm_kzalloc(dev, sizeof(*artpec8_phy), GFP_KERNEL);
>> +        if (!artpec8_phy)
>> +                return -ENOMEM;
>> +
>> +        /* reference clock */
>> +        if (of_property_read_u32(dev->of_node, "lcpll-ref-clk",

> No, really, no...

> All properties *must* be documented in the bindings. You cannot sneak in
> some stuff here...

 
Ok, sure.
I will should be documented the all property.
 
>> +                                &artpec8_phy->lcpll_ref_clk)) {
>> +                return -EINVAL;
>> +        }
>> +        /* PLL SOC reference clock */
>> +        if (artpec8_phy->lcpll_ref_clk == REF_CLK_FROM_SOC_PLL) {
>> +                artpec8_phy->soc_pll_clk = devm_clk_get(dev, "ref_clk");
>> +                if (IS_ERR(artpec8_phy->soc_pll_clk))
>> +                        return -EINVAL;
>> +                clk_prepare_enable(artpec8_phy->soc_pll_clk);
>> +        }
>> +
>> +        /* link mode */
>> +        if (of_property_read_string(dev->of_node, "mode", &artpec8_phy->mode))
>> +                return -EINVAL;
>> +
>> +        /* number of lanes */
>> +        if (of_property_read_u32(dev->of_node, "num-lanes",
>> +                                 &artpec8_phy->num_lanes))
>> +                return -EINVAL;
>> +
>> +        if (artpec8_phy->num_lanes > LANE_MAX)
>> +                return -EINVAL;
>> +
>> +        /* PHY base register */
>> +        artpec8_phy->phy_base = devm_platform_ioremap_resource_byname(pdev, "phy");
>> +        if (IS_ERR(artpec8_phy->phy_base))
>> +                return PTR_ERR(artpec8_phy->phy_base);
>> +
>> +        /* PCS base register */
>> +        artpec8_phy->pcs_base = devm_platform_ioremap_resource_byname(pdev, "pcs");
>> +        if (IS_ERR(artpec8_phy->pcs_base))
>> +                return PTR_ERR(artpec8_phy->pcs_base);
>> +
>> +        /* sysreg regmap handle, need to change using smc */
>> +        artpec8_phy->sysreg =
>> +                syscon_regmap_lookup_by_phandle(dev->of_node,
>> +                        "samsung,fsys-sysreg");

> Nope. Usage of undocumented properties. Please post your DTS changes, so
> we can validate the user of this driver.

 
I will should be documented the all property.
 
> Best regards,
> Krzysztof


Thank you for kindness reivew.
I will return with v3 patchset which is applied review comments.

Best regards,
Wangseok Lee

2022-06-08 08:55:11

by Wangseok Lee

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: phy: Add ARTPEC-8 PCIe phy

On 06/06/2022 19:16, Krzysztof Kozlowski wrote:
> On 03/06/2022 04:31, Wangseok Lee wrote:
>> Add description to support Axis, ARTPEC-8 SoC.
>> ARTPEC-8 is the SoC platform of Axis Communications
>> and PCIe phy is designed based on SAMSUNG PHY.

> This does not look like wrapped in Linux commit style.
https://protect2.fireeye.com/v1/url?k=c73ae309-a6470b4e-c73b6846-74fe485fff30-d00b7249c2c0a970&q=1&e=b8b33895-af3d-42ce-80ac-4835057078e7&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.18-rc4%2Fsource%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L586

 
Ok, i will fix it.
 
>> 
>> changes since v1 :
>> -'make dt_binding_check' result improvement
>> -Add the missing property list
>> -Align the indentation of continued lines/entries
>> 
>> Signed-off-by: Wangseok Lee <[email protected]>
>> ---
>>  .../bindings/phy/axis,artpec8-pcie-phy.yaml        | 70 ++++++++++++++++++++++
>>  1 file changed, 70 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>> new file mode 100644
>> index 0000000..ab9766f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/axis,artpec8-pcie-phy.yaml
>> @@ -0,0 +1,70 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/v1/url?k=8784225c-e6f9ca1b-8785a913-74fe485fff30-b4af51b9b3670f43&q=1&e=b8b33895-af3d-42ce-80ac-4835057078e7&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Faxis%2Cartpec8-pcie-phy.yaml%23
>> +$schema: https://protect2.fireeye.com/v1/url?k=f78efaf3-96f312b4-f78f71bc-74fe485fff30-583950d45c073877&q=1&e=b8b33895-af3d-42ce-80ac-4835057078e7&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>> +
>> +title: ARTPEC-8 SoC PCIe PHY Device Tree Bindings

> Same comment as patch #1.

 
I will remove 'Device Tree Bindings'.
 
>> +
>> +maintainers:
>> +  - Jesper Nilsson <[email protected]>
>> +
>> +properties:
>> +  compatible:
>> +    const: axis,artpec8-pcie-phy
>> +
>> +  reg:
>> +    items:
>> +      - description: PHY registers.
>> +      - description: PHY coding sublayer registers.
>> +
>> +  reg-names:
>> +    items:
>> +      - const: phy
>> +      - const: pcs
>> +
>> +  "#phy-cells":
>> +    const: 0
>> +
>> +  clocks:
>> +    items:
>> +      - description: PCIe PHY reference clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ref_clk

> Same comment as patch #1.

 
Ok, remove _clk.
 
>> +
>> +  num-lanes:
>> +    const: 2
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - "#phy-cells"
>> +  - clocks
>> +  - clock-names
>> +  - num-lanes
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    artpec8 {

> Same comment as patch #1.


 
Ok, modity to 'soc'.
 
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +        pcie_phy: pcie-phy@16c80000 {
>> +            compatible = "axis,artpec8-pcie-phy";
>> +            reg = <0x0 0x16c80000 0x0 0x2000>,
>> +                  <0x0 0x16c90000 0x0 0x1000>;
>> +            reg-names = "phy", "pcs";
>> +            #phy-cells = <0>;
>> +            clocks = <&clock_cmu_fsys 53>;
>> +            clock-names = "ref_clk";
>> +            num-lanes = <2>;
>> +        };
>> +    };
>> +...


> Best regards,
> Krzysztof

Thank you for kindness reivew.

Best regards,
Wangseok Lee