Re: [PATCH v3 4/5] phy: Add ARTPEC-8 PCIe PHY driver

Wangseok Lee
2022-07-06T08:11:05Z

On 05-07-22, 15:30, Vinod Koul wrote:
> On 14-06-22, 10:34, 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.
>>
>> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com>
>> Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com>
>> ---
>> v2->v3 :
>> -remove unnecessary indentation
>> -redefine local struct to statis const
>> -add static const to struct that requires static const definition
>> -remove wrappers on writel and readl
>>
>> v1->v2 :
>> -change folder name of phy driver to axis from artpec
>> ---
>> 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 | 776 ++++++++++++++++++++++++++++++++++++
>> 5 files changed, 789 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.
>
> How different is it from SS IP and why should it not be under
> phy/samsung/ then?
>

SoC platform is completely different from exynos platform and different
from exynos PCIe PHY. So the overall sysreg configuration is different
and register map is also different. The reset method and type of PHY for
initialization are different. This is not a driver that supports Samsung
SoC platform, but this PHY driver that supports only Axis, Artpec-8
platforms, so it is not added to phy/samsung.

>> 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..d21867b
>> --- /dev/null
>> +++ b/drivers/phy/axis/phy-artpec8-pcie.c
>> @@ -0,0 +1,776 @@
>> +// 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 <jaeho79.cho@samsung.com>
>> + */
>> +
>> +#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)
>
> Pls use GENMASK for this
>

Ok, i will fix it.

>> +#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 */
>> +static const 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 */
>> +static const 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}
>> +};
>> +
>> +static const 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_update(void __iomem *base, u32 mask,
>> +                                        u32 update, u32 reg)
>> +{
>> +        u32 val;
>> +
>> +        val = readl(base + reg);
>> +        val &= ~(mask);
>> +        val |= update;
>> +        writel(val, base + reg);
>> +};
>> +
>> +static void artpec8_pcie_enable_ref_clk_from_xo(struct artpec8_pcie_phy
>> +                                                *pciephy)
>
> pls use consistent formatting for second lines, this and previous
> function _do_not_ match. Hint, checkpatch recommends aligning to
> previous line open brace (--strict option will check that for you)

I will fix it, thank you for information.

>> +{
>> +        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++)
>> +                writel(cmn_regs[i].val, pciephy->>phy_base + cmn_regs[i].offset);
>> +
>> +        /* init lane registers */
>> +        count = ARRAY_SIZE(lane_regs);
>> +        for (i = 0; i < count; i++) {
>> +                writel(lane_regs[i].val, pciephy->>phy_base + lane_regs[i].offset);
>> +                writel(lane_regs[i].val, pciephy->>phy_base + lane_regs[i].offset +
>> +                                        OFFSET_PER_LANE);
>> +        }
>> +
>> +        /* rx preset registers */
>> +        count = ARRAY_SIZE(rx_preset_regs);
>> +        for (i = 0; i < count; i++)
>> +                writel(rx_preset_regs[i].val, pciephy->>phy_base + 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,
>> +};
>
> you have only _one_ ops, so why add driver data for this?
>

Maybe i made it in case data is added in the future, but I will delete it.

>> +
>> +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 <jaeho79.cho@samsung.com>>");
>> --
>> 2.9.5
>
> --
> ~Vinod
Thank you for kindness reivew.

Best regards,
Wangseok Lee