[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2d3f3da1-713e-4378-b87d-11f10f0f9590@kernel.org>
Date: Tue, 27 Aug 2024 12:33:02 +0200
From: Konrad Dybcio <konradybcio@...nel.org>
To: Qiang Yu <quic_qianyu@...cinc.com>, manivannan.sadhasivam@...aro.org,
vkoul@...nel.org, kishon@...nel.org, robh@...nel.org, andersson@...nel.org,
konradybcio@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
mturquette@...libre.com, sboyd@...nel.org, abel.vesa@...aro.org,
quic_msarkar@...cinc.com, quic_devipriy@...cinc.com
Cc: dmitry.baryshkov@...aro.org, kw@...ux.com, lpieralisi@...nel.org,
neil.armstrong@...aro.org, linux-arm-msm@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
linux-clk@...r.kernel.org
Subject: Re: [PATCH 3/8] phy: qcom: qmp: Add phy register and clk setting for
x1e80100 PCIe3
On 27.08.2024 8:36 AM, Qiang Yu wrote:
> Currently driver supports only x4 lane based functionality using tx/rx and
> tx2/rx2 pair of register sets. To support 8 lane functionality with PCIe3,
> PCIe3 related QMP PHY provides additional programming which are available
> as txz and rxz based register set. Hence adds txz and rxz based registers
> usage and programming sequences. Phy register setting for txz and rxz will
> be applied to all 8 lanes. Some lanes may have different settings on
> several registers than txz/rxz, these registers should be programmed after
> txz/rxz programming sequences completing.
>
> Besides, PCIe3 related QMP PHY also requires addtional clk, which is named
> as clkref_en. Hence, add this clk into qmp_pciephy_clk_l so that it can be
> easily parsed from devicetree during init.
>
> Signed-off-by: Qiang Yu <quic_qianyu@...cinc.com>
> ---
[...]
> +static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_rx_tbl[] = {
> + QMP_PHY_INIT_CFG_LANE(QSERDES_V6_20_RX_DFE_CTLE_POST_CAL_OFFSET, 0x3a, 1),
1 -> BIT(0)
[...]
> + /* Set to true for programming all 8 lanes using txz/rxz registers */
> + bool lane_broadcasting;
This is unnecessary because you call qmp_configure_lane conditionally,
but that function has a nullcheck built in
> +
> /* resets to be requested */
> const char * const *reset_list;
> int num_resets;
> @@ -2655,6 +2815,8 @@ struct qmp_pcie {
> void __iomem *rx;
> void __iomem *tx2;
> void __iomem *rx2;
> + void __iomem *txz;
> + void __iomem *rxz;
> void __iomem *ln_shrd;
>
> void __iomem *port_b;
> @@ -2700,7 +2862,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
>
> /* list of clocks required by phy */
> static const char * const qmp_pciephy_clk_l[] = {
> - "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> + "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "clkref_en",
Why not just put in TCSR_PCIE_8L_CLKREF_EN as "ref"? It's downstream
of the XO anyway.
[...]
> const struct qmp_phy_cfg *cfg = qmp->cfg;
> @@ -3700,6 +3907,11 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
>
> qmp_configure(qmp->dev, serdes, tbls->serdes, tbls->serdes_num);
>
> + if (cfg->lane_broadcasting) {
All these ifs can be unconditional
Konrad
Powered by blists - more mailing lists