Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754891AbcK2Agc (ORCPT ); Mon, 28 Nov 2016 19:36:32 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:40736 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752654AbcK2AgV (ORCPT ); Mon, 28 Nov 2016 19:36:21 -0500 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org DEFEB61389 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=sboyd@codeaurora.org Date: Mon, 28 Nov 2016 16:35:22 -0800 From: Stephen Boyd To: Vivek Gautam Cc: kishon@ti.com, robh+dt@kernel.org, mark.rutland@arm.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, srinivas.kandagatla@linaro.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH v2 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets Message-ID: <20161129003522.GP6095@codeaurora.org> References: <1479816163-5260-1-git-send-email-vivek.gautam@codeaurora.org> <1479816163-5260-5-git-send-email-vivek.gautam@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1479816163-5260-5-git-send-email-vivek.gautam@codeaurora.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 31171 Lines: 1004 On 11/22, Vivek Gautam wrote: > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index f1dcec1..8970d9e 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -430,6 +430,15 @@ config PHY_STIH407_USB > Enable this support to enable the picoPHY device used by USB2 > and USB3 controllers on STMicroelectronics STiH407 SoC families. > > +config PHY_QCOM_QMP > + tristate "Qualcomm QMP PHY Driver" > + depends on OF && (ARCH_QCOM || COMPILE_TEST) > + select GENERIC_PHY > + select RESET_CONTROLLER Again, probably should be dropped as we're not providing resets here, just using them. > diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c > new file mode 100644 > index 0000000..f85289e > --- /dev/null > +++ b/drivers/phy/phy-qcom-qmp.c > @@ -0,0 +1,1141 @@ > +/* > + * Copyright (c) 2016, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + [...] > + > +/* set of registers with offsets different per-PHY */ > +enum qphy_reg_layout { > + /* Common block control registers */ > + QPHY_COM_SW_RESET, > + QPHY_COM_POWER_DOWN_CONTROL, > + QPHY_COM_START_CONTROL, > + QPHY_COM_PCS_READY_STATUS, > + /* PCS registers */ > + QPHY_PLL_LOCK_CHK_DLY_TIME, > + QPHY_FLL_CNTRL1, > + QPHY_FLL_CNTRL2, > + QPHY_FLL_CNT_VAL_L, > + QPHY_FLL_CNT_VAL_H_TOL, > + QPHY_FLL_MAN_CODE, > + QPHY_PCS_READY_STATUS, > +}; > + > +unsigned int pciephy_regs_layout[] = { static? const? > + [QPHY_COM_SW_RESET] = 0x400, > + [QPHY_COM_POWER_DOWN_CONTROL] = 0x404, > + [QPHY_COM_START_CONTROL] = 0x408, > + [QPHY_COM_PCS_READY_STATUS] = 0x448, > + [QPHY_PLL_LOCK_CHK_DLY_TIME] = 0xa8, > + [QPHY_FLL_CNTRL1] = 0xc4, > + [QPHY_FLL_CNTRL2] = 0xc8, > + [QPHY_FLL_CNT_VAL_L] = 0xcc, > + [QPHY_FLL_CNT_VAL_H_TOL] = 0xd0, > + [QPHY_FLL_MAN_CODE] = 0xd4, > + [QPHY_PCS_READY_STATUS] = 0x174, > +}; > + > +unsigned int usb3phy_regs_layout[] = { static? const? > + [QPHY_FLL_CNTRL1] = 0xc0, > + [QPHY_FLL_CNTRL2] = 0xc4, > + [QPHY_FLL_CNT_VAL_L] = 0xc8, > + [QPHY_FLL_CNT_VAL_H_TOL] = 0xcc, > + [QPHY_FLL_MAN_CODE] = 0xd0, > + [QPHY_PCS_READY_STATUS] = 0x17c, > +}; > + > +static struct qmp_phy_init_tbl pciephy_serdes_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x1c), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_EN, 0x42), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0xff), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x1f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORECLK_DIV, 0x0a), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x09), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x1a), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x0a), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x02), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_BUF_ENABLE, 0x1f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x04), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN1_MODE0, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x02), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0x2f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x19), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_RESCODE_DIV_NUM, 0x15), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_EP_DIV, 0x19), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_RESCODE_DIV_NUM, 0x40), > +}; > + > +static struct qmp_phy_init_tbl pciephy_tx_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06), > +}; > + > +static struct qmp_phy_init_tbl pciephy_rx_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_ENABLES, 0x1c), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xdb), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_BAND, 0x18), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN_HALF, 0x04), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x14), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x19), > +}; > + > +static struct qmp_phy_init_tbl pciephy_pcs_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QPHY_RX_IDLE_DTCT_CNTRL, 0x4c), > + QCOM_QMP_PHY_INIT_CFG(QPHY_PWRUP_RESET_DLY_TIME_AUXCLK, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QPHY_LP_WAKEUP_DLY_TIME_AUXCLK, 0x01), > + > + QCOM_QMP_PHY_INIT_CFG_L(QPHY_PLL_LOCK_CHK_DLY_TIME, 0x05), > + > + QCOM_QMP_PHY_INIT_CFG(QPHY_ENDPOINT_REFCLK_DRIVE, 0x05), > + QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x02), > + QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG4, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG1, 0xa3), > + QCOM_QMP_PHY_INIT_CFG(QPHY_TXDEEMPH_M3P5DB_V0, 0x0e), > +}; > + > +static struct qmp_phy_init_tbl usb3phy_serdes_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x14), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x08), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x30), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x04), > + /* PLL and Loop filter settings */ > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x15), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x34), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_CFG, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x0a), > + /* SSC settings */ > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0xde), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x07), > +}; > + > +static struct qmp_phy_init_tbl usb3phy_tx_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06), > +}; > + > +static struct qmp_phy_init_tbl usb3phy_rx_init_tbl[] = { const? > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x02), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4c), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xbb), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x03), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x18), > + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x16), > +}; > + > +static struct qmp_phy_init_tbl usb3phy_pcs_init_tbl[] = { const? > + /* FLL settings */ > + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL2, 0x03), > + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL1, 0x02), > + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_L, 0x09), > + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_H_TOL, 0x42), > + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_MAN_CODE, 0x85), > + > + /* Lock Det settings */ > + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG1, 0xd1), > + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG2, 0x1f), > + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG3, 0x47), > + QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG2, 0x08), > +}; > + > +/** > + * struct qmp_phy_init_cfg:- per-PHY init config. The colon and dash can't be right. One is kernel-doc, but of course there aren't any other member descriptions. > + */ > +struct qmp_phy_init_cfg { > + /* phy-type - PCIE/UFS/USB */ > + unsigned int type; > + /* number of lanes provided by phy */ > + int nlanes; > + > + /* Initialization sequence for PHY blocks - Serdes, tx, rx, pcs */ > + struct qmp_phy_init_tbl *phy_init_serdes_tbl; > + int phy_init_serdes_tbl_sz; > + struct qmp_phy_init_tbl *phy_init_tx_tbl; > + int phy_init_tx_tbl_sz; > + struct qmp_phy_init_tbl *phy_init_rx_tbl; > + int phy_init_rx_tbl_sz; > + struct qmp_phy_init_tbl *phy_init_pcs_tbl; > + int phy_init_pcs_tbl_sz; _sz makes it sound like bytes, perhaps _num instead. > + > + /* array of registers with different offsets */ > + unsigned int *regs; const? > + > + unsigned int mask_start_ctrl; > + unsigned int mask_pwr_dn_ctrl; > + /* true, if PHY has a separate PHY_COM_CNTRL block */ > + bool has_phy_com_ctrl; > + /* true, if PHY has a reset for individual lanes */ > + bool has_lane_rst; > +}; > + > +/** > + * struct qmp_phy_desc:- per-lane phy-descriptor. Also kernel-doc requests full stop is left out on one line descriptions. > + * > + * @phy: pointer to generic phy > + * @tx: pointer to iomapped memory space for PHY's tx > + * @rx: pointer to iomapped memory space for PHY's rx > + * @pcs: pointer to iomapped memory space for PHY's pcs > + * @pipe_clk: pointer to pipe lock > + * @index: lane index > + * @qphy: pointer to QMP phy to which this lane belongs > + * @lane_rst: pointer to lane's reset controller Not sure we care about "pointer to" as types can be figured out other ways. > + */ > +struct qmp_phy_desc { > + struct phy *phy; > + void __iomem *tx; > + void __iomem *rx; > + void __iomem *pcs; > + struct clk *pipe_clk; > + unsigned int index; > + struct qcom_qmp_phy *qphy; > + struct reset_control *lane_rst; > +}; > + > +/** > + * struct qcom_qmp_phy:- structure holding QMP PHY attributes. > + * > + * @dev: pointer to device > + * @serdes: pointer to iomapped memory space for phy's serdes > + * > + * @aux_clk: pointer to phy core clock > + * @cfg_ahb_clk: pointer to AHB2PHY interface clock > + * @ref_clk: pointer to reference clock > + * @ref_clk_src: pointer to source to reference clock > + * > + * @vdda_phy: vdd supply to the phy core block > + * @vdda_pll: 1.8V vdd supply to ref_clk block > + * @vddp_ref_clk: vdd supply to specific ref_clk block (Optional) > + * > + * @phy_rst: Pointer to phy reset control > + * @phycom_rst: Pointer to phy common reset control > + * @phycfg_rst: Pointer to phy ahb cfg reset control (Optional) > + * > + * @cfg: pointer to init config for each phys > + * @phys: array of pointer to per-lane phy descriptors > + * @phy_mutex: mutex lock for PHY common block initialization > + * @init_count: Phy common block initialization count > + */ > +struct qcom_qmp_phy { > + struct device *dev; > + void __iomem *serdes; > + > + struct clk *aux_clk; > + struct clk *cfg_ahb_clk; > + struct clk *ref_clk; > + struct clk *ref_clk_src; > + > + struct regulator *vdda_phy; > + struct regulator *vdda_pll; > + struct regulator *vddp_ref_clk; > + > + struct reset_control *phy_rst; > + struct reset_control *phycom_rst; > + struct reset_control *phycfg_rst; > + > + const struct qmp_phy_init_cfg *cfg; > + struct qmp_phy_desc **phys; > + > + struct mutex phy_mutex; > + int init_count; > +}; > + > +static inline void qphy_setbits(void __iomem *reg, u32 val) > +{ > + u32 reg_val; > + > + reg_val = readl_relaxed(reg); > + reg_val |= val; > + writel_relaxed(reg_val, reg); > +} > + > +static inline void qphy_clrbits(void __iomem *reg, u32 val) > +{ > + u32 reg_val; > + > + reg_val = readl_relaxed(reg); > + reg_val &= ~val; > + writel_relaxed(reg_val, reg); > +} > + > +const struct qmp_phy_init_cfg msm8996_pciephy_init_cfg = { static? > + .type = PHY_TYPE_PCIE, > + .nlanes = 3, > + > + .phy_init_serdes_tbl = pciephy_serdes_init_tbl, > + .phy_init_serdes_tbl_sz = ARRAY_SIZE(pciephy_serdes_init_tbl), > + .phy_init_tx_tbl = pciephy_tx_init_tbl, > + .phy_init_tx_tbl_sz = ARRAY_SIZE(pciephy_tx_init_tbl), > + .phy_init_rx_tbl = pciephy_rx_init_tbl, > + .phy_init_rx_tbl_sz = ARRAY_SIZE(pciephy_rx_init_tbl), > + .phy_init_pcs_tbl = pciephy_pcs_init_tbl, > + .phy_init_pcs_tbl_sz = ARRAY_SIZE(pciephy_pcs_init_tbl), > + .regs = pciephy_regs_layout, > + .mask_start_ctrl = (PHY_PCS_START | PHY_PLL_READY_GATE_EN), > + .mask_pwr_dn_ctrl = (PHY_SW_PWRDN | PHY_REFCLK_DRV_DSBL), > + > + .has_phy_com_ctrl = true, > + .has_lane_rst = true, > +}; > + > +const struct qmp_phy_init_cfg msm8996_usb3phy_init_cfg = { static? Try running sparse. > + .type = PHY_TYPE_USB3, > + .nlanes = 1, > + > + .phy_init_serdes_tbl = usb3phy_serdes_init_tbl, > + .phy_init_serdes_tbl_sz = ARRAY_SIZE(usb3phy_serdes_init_tbl), > + .phy_init_tx_tbl = usb3phy_tx_init_tbl, > + .phy_init_tx_tbl_sz = ARRAY_SIZE(usb3phy_tx_init_tbl), > + .phy_init_rx_tbl = usb3phy_rx_init_tbl, > + .phy_init_rx_tbl_sz = ARRAY_SIZE(usb3phy_rx_init_tbl), > + .phy_init_pcs_tbl = usb3phy_pcs_init_tbl, > + .phy_init_pcs_tbl_sz = ARRAY_SIZE(usb3phy_pcs_init_tbl), Do we really need phy_init as a prefix. Could just be serdes_tbl, tx_tbl, etc? > + .regs = usb3phy_regs_layout, > + .mask_start_ctrl = (PHY_SERDES_START | PHY_PCS_START), > + .mask_pwr_dn_ctrl = PHY_SW_PWRDN, > +}; > + > +static void qcom_qmp_phy_configure(void __iomem *base, > + unsigned int *regs_layout, > + struct qmp_phy_init_tbl init_tbl[], > + int init_tbl_sz) Shorter variable names would make this easier to read. > +{ > + int i; > + > + for (i = 0; i < init_tbl_sz; i++) { > + if (init_tbl[i].in_layout) > + writel_relaxed(init_tbl[i].cfg_val, > + base + regs_layout[init_tbl[i].reg_offset]); > + else > + writel_relaxed(init_tbl[i].cfg_val, > + base + init_tbl[i].reg_offset); > + } > + const struct qmp_phy_init_tbl *t = tbl; for (i = 0; i < num; i++, t++) if (t->in_layout) writel_relaxed(t->val, base + regs[t->offset]); else writel_relaxed(t->val, base + t->offset); > + /* flush buffered writes */ > + mb(); > +} > + > +static int qcom_qmp_phy_poweron(struct phy *phy) > +{ > + struct qmp_phy_desc *phydesc = phy_get_drvdata(phy); > + struct qcom_qmp_phy *qphy = phydesc->qphy; > + int ret; > + > + dev_vdbg(&phy->dev, "Powering on QMP phy\n"); > + > + ret = regulator_enable(qphy->vdda_phy); > + if (ret) { > + dev_err(qphy->dev, "%s: vdda-phy enable failed, err=%d\n", > + __func__, ret); > + return ret; > + } > + > + ret = regulator_enable(qphy->vdda_pll); > + if (ret) { > + dev_err(qphy->dev, "%s: vdda-pll enable failed, err=%d\n", > + __func__, ret); > + goto err_vdda_pll; > + } > + > + if (qphy->vddp_ref_clk) { > + ret = regulator_enable(qphy->vddp_ref_clk); > + if (ret) { > + dev_err(qphy->dev, "%s: vdda-ref-clk enable failed, err=%d\n", > + __func__, ret); > + goto err_vddp_refclk; > + } > + } > + > + clk_prepare_enable(qphy->ref_clk_src); > + clk_prepare_enable(qphy->ref_clk); > + clk_prepare_enable(phydesc->pipe_clk); And if these fail? > + > + return 0; > + > +err_vddp_refclk: > + regulator_disable(qphy->vdda_pll); > +err_vdda_pll: > + regulator_disable(qphy->vdda_phy); > + return ret; > +} > + > +static int qcom_qmp_phy_poweroff(struct phy *phy) > +{ > + struct qmp_phy_desc *phydesc = phy_get_drvdata(phy); > + struct qcom_qmp_phy *qphy = phydesc->qphy; > + > + clk_disable_unprepare(qphy->ref_clk_src); > + clk_disable_unprepare(qphy->ref_clk); > + clk_disable_unprepare(phydesc->pipe_clk); > + > + if (qphy->vddp_ref_clk) > + regulator_disable(qphy->vddp_ref_clk); > + > + regulator_disable(qphy->vdda_pll); > + regulator_disable(qphy->vdda_phy); > + > + return 0; > +} > + > +static int qcom_qmp_phy_is_ready(struct qcom_qmp_phy *qphy, > + void __iomem *pcs_status, u32 mask) > +{ > + unsigned int init_timeout; > + > + init_timeout = PHY_READY_TIMEOUT_COUNT; > + do { > + if (readl_relaxed(pcs_status) & mask) > + break; > + > + usleep_range(REFCLK_STABILIZATION_DELAY_US_MIN, > + REFCLK_STABILIZATION_DELAY_US_MAX); > + } while (--init_timeout); readl_poll_timeout? > + > + if (!init_timeout) > + return -EBUSY; > + > + return 0; > +} > + [...] > + > +/* PHY Initialization */ > +static int qcom_qmp_phy_init(struct phy *phy) > +{ > + struct qmp_phy_desc *phydesc = phy_get_drvdata(phy); > + struct qcom_qmp_phy *qphy = phydesc->qphy; > + const struct qmp_phy_init_cfg *cfg = qphy->cfg; > + void __iomem *tx = phydesc->tx; > + void __iomem *rx = phydesc->rx; > + void __iomem *pcs = phydesc->pcs; > + int ret; > + > + dev_vdbg(qphy->dev, "Initializing QMP phy\n"); > + > + /* enable interface clocks to program phy */ > + clk_prepare_enable(qphy->aux_clk); > + clk_prepare_enable(qphy->cfg_ahb_clk); > + > + ret = qcom_qmp_phy_com_init(qphy); > + if (ret) > + goto err; > + > + if (phydesc->lane_rst) { > + ret = reset_control_deassert(phydesc->lane_rst); > + if (ret) { > + dev_err(qphy->dev, "lane<%d> reset deassert failed\n", Drop the brackets? > + phydesc->index); > + goto err_lane_rst; > + } > + } > + > + /* Tx, Rx, and PCS configurations */ > + qcom_qmp_phy_configure(tx, cfg->regs, cfg->phy_init_tx_tbl, > + cfg->phy_init_tx_tbl_sz); > + qcom_qmp_phy_configure(rx, cfg->regs, cfg->phy_init_rx_tbl, > + cfg->phy_init_rx_tbl_sz); > + qcom_qmp_phy_configure(pcs, cfg->regs, cfg->phy_init_pcs_tbl, > + cfg->phy_init_pcs_tbl_sz); > + > + /* > + * Pull out PHY from POWER DOWN state: > + * This is active low enable signal to power-down PHY. > + */ > + qphy_setbits(pcs + QPHY_POWER_DOWN_CONTROL, cfg->mask_pwr_dn_ctrl); > + /* XXX: 10 us delay; given in PCIE phy programming guide only */ Is this a todo? > + usleep_range(POWER_DOWN_DELAY_US_MIN, POWER_DOWN_DELAY_US_MAX); > + > + /* start SerDes and Phy-Coding-Sublayer */ > + qphy_setbits(pcs + QPHY_START_CTRL, cfg->mask_start_ctrl); > + > + /* Pull PHY out of reset state */ > + qphy_clrbits(pcs + QPHY_SW_RESET, PHY_SW_RESET); > + /* Make sure that above writes are completed */ > + mb(); > + > + ret = qcom_qmp_phy_is_ready(qphy, pcs + > + cfg->regs[QPHY_PCS_READY_STATUS], > + MASK_PHYSTATUS); > + if (ret) { > + dev_err(qphy->dev, "phy initialization timed-out\n"); > + goto err_pcs_ready; > + } > + > + return 0; > + > +err_pcs_ready: > + if (phydesc->lane_rst) > + reset_control_assert(phydesc->lane_rst); > +err_lane_rst: > + qcom_qmp_phy_com_exit(qphy); > +err: > + clk_disable_unprepare(qphy->cfg_ahb_clk); > + clk_disable_unprepare(qphy->aux_clk); > + return ret; > +} > + > +static int qcom_qmp_phy_exit(struct phy *phy) > +{ > + struct qmp_phy_desc *phydesc = phy_get_drvdata(phy); > + struct qcom_qmp_phy *qphy = phydesc->qphy; > + const struct qmp_phy_init_cfg *cfg = qphy->cfg; > + > + /* PHY reset */ > + qphy_setbits(phydesc->pcs + QPHY_SW_RESET, PHY_SW_RESET); > + > + /* stop SerDes and Phy-Coding-Sublayer */ > + qphy_clrbits(phydesc->pcs + QPHY_START_CTRL, cfg->mask_start_ctrl); > + > + /* Put PHY into POWER DOWN state: active low */ > + qphy_clrbits(phydesc->pcs + QPHY_POWER_DOWN_CONTROL, > + cfg->mask_pwr_dn_ctrl); > + > + /* Make sure that above writes are completed */ > + mb(); > + > + if (phydesc->lane_rst) > + reset_control_assert(phydesc->lane_rst); > + > + qcom_qmp_phy_com_exit(qphy); > + > + clk_disable_unprepare(qphy->aux_clk); > + clk_disable_unprepare(qphy->cfg_ahb_clk); > + > + return 0; > +} > + > + > +static int qcom_qmp_phy_regulator_init(struct device *dev) > +{ > + struct qcom_qmp_phy *qphy = dev_get_drvdata(dev); > + int ret; > + > + qphy->vdda_phy = devm_regulator_get(dev, "vdda-phy"); > + if (IS_ERR(qphy->vdda_phy)) { > + ret = PTR_ERR(qphy->vdda_phy); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get vdda-phy, %d\n", ret); > + return ret; > + } > + > + qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll"); > + if (IS_ERR(qphy->vdda_pll)) { > + ret = PTR_ERR(qphy->vdda_pll); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get vdda-pll, %d\n", ret); > + return ret; > + } > + > + /* optional regulator */ > + qphy->vddp_ref_clk = devm_regulator_get(dev, "vddp-ref-clk"); > + if (IS_ERR(qphy->vddp_ref_clk)) { > + ret = PTR_ERR(qphy->vddp_ref_clk); > + if (ret != -EPROBE_DEFER) { > + dev_dbg(dev, "failed to get vddp-ref-clk, %d\n", ret); > + qphy->vddp_ref_clk = NULL; > + } else { > + return ret; > + } > + } Regulator framework should insert a dummy regulator if this is missing in DT. So we shouldn't need to do anything complicated here right? > + > + return 0; > +} > + > +static int qcom_qmp_phy_clk_init(struct device *dev) > +{ > + struct qcom_qmp_phy *qphy = dev_get_drvdata(dev); > + int ret; > + > + qphy->aux_clk = devm_clk_get(dev, "aux"); > + if (IS_ERR(qphy->aux_clk)) { > + ret = PTR_ERR(qphy->aux_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get aux_clk\n"); > + return ret; > + } > + > + qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb"); > + if (IS_ERR(qphy->cfg_ahb_clk)) { > + ret = PTR_ERR(qphy->cfg_ahb_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get cfg_ahb_clk\n"); > + return ret; > + } > + > + qphy->ref_clk_src = devm_clk_get(dev, "ref_clk_src"); > + if (IS_ERR(qphy->ref_clk_src)) { > + ret = PTR_ERR(qphy->ref_clk_src); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get ref_clk_src\n"); > + return ret; > + } > + > + qphy->ref_clk = devm_clk_get(dev, "ref_clk"); > + if (IS_ERR(qphy->ref_clk)) { > + ret = PTR_ERR(qphy->ref_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get ref_clk\n"); > + return ret; > + } > + > + return 0; > +} > + > +static struct phy *qcom_qmp_phy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct qcom_qmp_phy *qphy = dev_get_drvdata(dev); > + int i; > + > + if (WARN_ON(args->args[0] >= qphy->cfg->nlanes)) > + return ERR_PTR(-ENODEV); > + > + for (i = 0; i < qphy->cfg->nlanes; i++) { > + if (qphy->phys[i]->index == args->args[0]) > + break; > + } Drop the braces please. > + > + if (i == qphy->cfg->nlanes) > + return ERR_PTR(-ENODEV); > + > + return qphy->phys[i]->phy; Could be simplified: for (i = 0; i < qphy->cfg->nlanes; i++) if (qphy->phys[i]->index == args->args[0]) return qphy->phys[i]->phy; return ERR_PTR(-ENODEV); Also, isn't i == phys[i]->index so this could be a direct lookup? > +} > + > +static const struct phy_ops qcom_qmp_phy_gen_ops = { > + .init = qcom_qmp_phy_init, > + .exit = qcom_qmp_phy_exit, > + .power_on = qcom_qmp_phy_poweron, > + .power_off = qcom_qmp_phy_poweroff, > + .owner = THIS_MODULE, > +}; > + > +static const struct of_device_id qcom_qmp_phy_of_match_table[] = { > + { > + .compatible = "qcom,msm8996-qmp-pcie-phy", > + .data = &msm8996_pciephy_init_cfg, > + }, { > + .compatible = "qcom,msm8996-qmp-usb3-phy", > + .data = &msm8996_usb3phy_init_cfg, > + }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, qcom_qmp_phy_of_match_table); > + > +static int qcom_qmp_phy_probe(struct platform_device *pdev) > +{ > + struct qcom_qmp_phy *qphy; > + struct device *dev = &pdev->dev; > + struct phy_provider *phy_provider; > + struct resource *res; > + const struct of_device_id *match; > + void __iomem *base; > + int ret = 0; Shouldn't need a default assignment here. > + int id; > + > + qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL); > + if (!qphy) > + return -ENOMEM; > + > + qphy->dev = dev; > + dev_set_drvdata(dev, qphy); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(dev, res); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + /* per PHY serdes; usually located at base address */ > + qphy->serdes = base; > + > + mutex_init(&qphy->phy_mutex); > + > + /* Get the specific init parameters of QMP phy */ > + match = of_match_node(qcom_qmp_phy_of_match_table, dev->of_node); > + qphy->cfg = match->data; > + > + ret = qcom_qmp_phy_clk_init(dev); > + if (ret) { > + dev_err(dev, "clock init failed\n"); > + return ret; > + } > + > + ret = qcom_qmp_phy_regulator_init(dev); > + if (ret) { > + dev_err(dev, "regulator init failed\n"); > + return ret; > + } > + > + qphy->phy_rst = devm_reset_control_get(dev, "phy"); > + if (IS_ERR(qphy->phy_rst)) { > + dev_err(dev, "failed to get phy core reset\n"); > + return PTR_ERR(qphy->phy_rst); > + } > + > + qphy->phycom_rst = devm_reset_control_get(dev, "common"); > + if (IS_ERR(qphy->phycom_rst)) { > + dev_err(dev, "failed to get phy common reset\n"); > + return PTR_ERR(qphy->phycom_rst); > + } > + > + qphy->phycfg_rst = devm_reset_control_get(dev, "cfg"); > + if (IS_ERR(qphy->phycfg_rst)) { > + dev_dbg(dev, "failed to get phy ahb cfg reset\n"); > + qphy->phycfg_rst = NULL; > + } > + > + qphy->phys = devm_kcalloc(dev, qphy->cfg->nlanes, > + sizeof(*qphy->phys), GFP_KERNEL); > + if (!qphy->phys) > + return -ENOMEM; > + > + for (id = 0; id < qphy->cfg->nlanes; id++) { > + struct phy *generic_phy; > + struct qmp_phy_desc *phy_desc; > + char prop_name[MAX_PROP_NAME]; > + unsigned int lane_offsets[3]; > + > + /* mem resources from index 1 to N for N number of lanes */ > + res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1); > + base = devm_ioremap_resource(dev, res); > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + phy_desc = devm_kzalloc(dev, sizeof(*phy_desc), GFP_KERNEL); > + if (!phy_desc) > + return -ENOMEM; > + > + /* > + * read offsets to Tx, Rx, and PCS blocks into a u32 array: > + * ------------------------------------ > + * | tx offset | rx offset | pcs offset | > + * ------------------------------------ > + */ > + ret = of_property_read_u32_array(dev->of_node, "lane-offsets", > + lane_offsets, 3); > + if (ret) { > + dev_err(dev, > + "failed to get tx/rx/pcs offsets for lane%d\n", > + id); > + return ret; > + } > + > + phy_desc->tx = base + lane_offsets[0]; > + phy_desc->rx = base + lane_offsets[1]; > + phy_desc->pcs = base + lane_offsets[2]; > + > + /* > + * Get PHY's Pipe clock, if any; USB3 and PCIe are PIPE3 > + * based phys, so they essentially have pipe clock. So, > + * we return error in case phy is USB3 or PIPE type. > + * Otherwise, we initialize pipe clock to NULL for > + * all phys that don't need this. > + */ > + memset(&prop_name, 0, sizeof(prop_name)); > + snprintf(prop_name, MAX_PROP_NAME, "pipe%d", id); > + phy_desc->pipe_clk = devm_clk_get(dev, prop_name); > + if (IS_ERR(phy_desc->pipe_clk)) { > + if (qphy->cfg->type == PHY_TYPE_PCIE || > + qphy->cfg->type == PHY_TYPE_USB3) { > + ret = PTR_ERR(phy_desc->pipe_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, > + "failed to get lane%d pipe_clk\n", id); > + return ret; > + } else { > + phy_desc->pipe_clk = NULL; > + } Drop the else part. > + } > + > + /* Get lane reset, if any */ > + if (qphy->cfg->has_lane_rst) { > + memset(&prop_name, 0, sizeof(prop_name)); > + snprintf(prop_name, MAX_PROP_NAME, "lane%d", id); > + phy_desc->lane_rst = devm_reset_control_get(dev, > + prop_name); > + if (IS_ERR(phy_desc->lane_rst)) { > + dev_err(dev, "failed to get lane%d reset\n", > + id); > + return PTR_ERR(phy_desc->lane_rst); > + } > + } > + > + generic_phy = devm_phy_create(dev, NULL, &qcom_qmp_phy_gen_ops); > + if (IS_ERR(generic_phy)) { > + ret = PTR_ERR(generic_phy); > + dev_err(dev, "failed to create qphy %d\n", ret); > + return ret; > + } > + > + phy_desc->phy = generic_phy; > + phy_desc->index = id; > + phy_desc->qphy = qphy; > + phy_set_drvdata(generic_phy, phy_desc); > + qphy->phys[id] = phy_desc; Probably should make the above part of this loop a function that returns a phy for each lane (or an error code on failure). This probe function is long. > + } > + > + phy_provider = devm_of_phy_provider_register(dev, qcom_qmp_phy_xlate); > + if (IS_ERR(phy_provider)) { > + ret = PTR_ERR(phy_provider); > + dev_err(dev, "failed to register qphy %d\n", ret); > + } > + > + return ret; > +} -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project