diff mbox series

[3/6] phy: qcom: qmp-pcie: Add X1P42100 Gen4x4 PHY

Message ID 20250125-topic-x1p4_dts-v1-3-02659a08b044@oss.qualcomm.com
State New
Headers show
Series X1P42100 DT and PCIe PHY bits | expand

Commit Message

Konrad Dybcio Jan. 25, 2025, 3:31 a.m. UTC
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Add a new, common configuration for Gen4x4 V6 PHYs without an init
sequence.

The bootloader configures the hardware once and the OS retains that
configuration by using the NOCSR reset line (which doesn't drop
register state on assert) in place of the "full reset" one.

Use this new configuration for X1P42100's Gen4x4 PHY.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Manivannan Sadhasivam Jan. 26, 2025, 7:29 a.m. UTC | #1
On January 25, 2025 11:00:23 PM GMT+05:30, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
>On Sat, Jan 25, 2025 at 04:31:19AM +0100, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> 
>> Add a new, common configuration for Gen4x4 V6 PHYs without an init
>> sequence.
>> 
>> The bootloader configures the hardware once and the OS retains that
>> configuration by using the NOCSR reset line (which doesn't drop
>> register state on assert) in place of the "full reset" one.
>
>I know your opinion, but my 2c would still be for not depending on the
>bootloader. I think that was the rule for ages for many possible
>reasons.
>

But if Linux or other OS can trust the bootloader, then it makes perfect sense to rely on them. Obviously, the question here is that on which platforms this level of trust should be established. And the answer I got was starting from the compute platforms (atleast X1E).

So let's take it on an experimental basis and see how it goes? If at all any problem arises, we can always resort to in kernel sequences.

- Mani

>> 
>> Use this new configuration for X1P42100's Gen4x4 PHY.
>> 
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>

மணிவண்ணன் சதாசிவம்
Manivannan Sadhasivam Jan. 26, 2025, 4:32 p.m. UTC | #2
On Sun, Jan 26, 2025 at 01:39:05PM +0200, Dmitry Baryshkov wrote:
> On Sun, Jan 26, 2025 at 12:59:52PM +0530, Manivannan Sadhasivam wrote:
> > 
> > 
> > On January 25, 2025 11:00:23 PM GMT+05:30, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
> > >On Sat, Jan 25, 2025 at 04:31:19AM +0100, Konrad Dybcio wrote:
> > >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > >> 
> > >> Add a new, common configuration for Gen4x4 V6 PHYs without an init
> > >> sequence.
> > >> 
> > >> The bootloader configures the hardware once and the OS retains that
> > >> configuration by using the NOCSR reset line (which doesn't drop
> > >> register state on assert) in place of the "full reset" one.
> > >
> > >I know your opinion, but my 2c would still be for not depending on the
> > >bootloader. I think that was the rule for ages for many possible
> > >reasons.
> > >
> > 
> > But if Linux or other OS can trust the bootloader, then it makes perfect sense to rely on them. Obviously, the question here is that on which platforms this level of trust should be established. And the answer I got was starting from the compute platforms (atleast X1E).
> 
> Is there any way how those values can be lost that we still might want
> to support ? The GDSC going to the OFF state? Some deep sleep state or a
> power collapse? Actual suspend to RAM (instead of current S2Idle)?
> 

As per Konrad's reply to my identical question, PHY register state is supposed
to be maintained by MX domain even during CX PC. This seem to be case on X1E
based platforms (compute).

> > 
> > So let's take it on an experimental basis and see how it goes? If at all any problem arises, we can always resort to in kernel sequences.
> 
> Sounds like a good proposal. Can possibly have a corresponding 'do not
> merge' patch with actual init tables?
> 

I don't find it really required. If the init sequences are really needed, we
know where to find them.

- Mani
diff mbox series

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 58103e87540ad84faca708debf61d79fe9f9ac54..68befe2901944b7f39e5adc12208c4b5578d94b1 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -4150,6 +4150,21 @@  static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
 	.phy_status		= PHYSTATUS_4_20,
 };
 
+static const struct qmp_phy_cfg qmp_v6_gen4x4_pciephy_cfg = {
+	.lanes = 4,
+
+	.offsets                = &qmp_pcie_offsets_v6_20,
+
+	.reset_list             = sdm845_pciephy_reset_l,
+	.num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
+	.vreg_list              = qmp_phy_vreg_l,
+	.num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
+	.regs                   = pciephy_v6_regs_layout,
+
+	.pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
+	.phy_status             = PHYSTATUS_4_20,
+};
+
 static void qmp_pcie_init_port_b(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tbls *tbls)
 {
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
@@ -4981,6 +4996,9 @@  static const struct of_device_id qmp_pcie_of_match_table[] = {
 	}, {
 		.compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy",
 		.data = &x1e80100_qmp_gen4x8_pciephy_cfg,
+	}, {
+		.compatible = "qcom,x1p42100-qmp-gen4x4-pcie-phy",
+		.data = &qmp_v6_gen4x4_pciephy_cfg,
 	},
 	{ },
 };