Re: [PATCH 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

From: "Alex G." <mr.nuke.me@gmail.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Vinod Koul <vkoul@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	ansuelsmth@gmail.com, robimarko@gmail.com,
	linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY
Date: Sat, 6 Apr 2024 11:48:45 -0500	[thread overview]
Message-ID: <45e1a982-fd59-48de-ace4-d6b6e821189d@gmail.com> (raw)
In-Reply-To: <CAA8EJpqYV537aCbkwTRUeZJzcn4wMXFqGxQHSdDFraGV_2zbQQ@mail.gmail.com>

On 4/6/24 09:37, Dmitry Baryshkov wrote:
> On Sat, 6 Apr 2024 at 11:47, Vinod Koul <vkoul@kernel.org> wrote:
>>
>> On 02-04-24, 14:25, Alexandru Gagniuc wrote:
>>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
>>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
>>> being reused from IPQ8074 and IPQ6018 PHYs.
>>>
>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>> ---
>>>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c      | 136 +++++++++++++++++-
>>>   .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h   |  14 ++
>>>   2 files changed, 149 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>>> index 8836bb1ff0cc..f07bd27e3b7a 100644
>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>>> @@ -487,6 +487,100 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_gen3_pcs_misc_tbl[] = {
>>>        QMP_PHY_INIT_CFG(QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_DRIVE, 0xc1),
>>>   };
>>>
>>> +static const struct qmp_phy_init_tbl ipq9574_gen3x2_pcie_serdes_tbl[] = {
>>> +     QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CLKBUFLR_EN, 0x18),
>>> +     QMP_PHY_INIT_CFG(QSERDES_PLL_BIAS_EN_CTRL_BY_PSM, 0x01),
>>> +     QMP_PHY_INIT_CFG(QSERDES_PLL_CLK_SELECT, 0x31),
>>> +     QMP_PHY_INIT_CFG(QSERDES_PLL_PLL_IVCO, 0x0F),
>>> +     QMP_PHY_INIT_CFG(QSERDES_PLL_BG_TRIM, 0x0F),
>>
>> Lower case here and everywhere please
> 
> For hex values

I will these updated in V2. Thanks!

>>>   static const struct qmp_phy_init_tbl sdm845_qmp_pcie_serdes_tbl[] = {
>>>        QMP_PHY_INIT_CFG(QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN, 0x14),
>>>        QMP_PHY_INIT_CFG(QSERDES_V3_COM_CLK_SELECT, 0x30),
>>> @@ -2448,7 +2542,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", "anoc", "snoc"
>>
>> How about older platforms which dont have these clocks
> 
> The driver uses devm_clk_bulk_get_optional(), so it should be fine.
> But the more important question should be why the platform needs
> anoc/snoc clocks here.
> 

I got the info from the downstream 5.4 kernel. While I don't know why 
these new clocks are required, they are needed. Omitting them will cause 
the boot to hang.

I could rename them to "snoc_lane" and "anoc_lane", if you think that 
makes more sense.

Alex

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy