Re: [PATCH v1 2/4] phy: qcom: qmp-pcie: Add qref and refgen regulator vote for QCS8300 and SA8775p PHY

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch mistakenly modifies `sm8450_qmp_gen4x2_pciephy_cfg` instead of `qcs8300_qmp_gen4x2_pciephy_cfg`, which breaks PCIe PHY probe on SM8450 and fails to apply the intended fix for QCS8300.
- [Medium] The driver unconditionally requires `refgen-supply` for SA8775P, but the DT binding schema explicitly omits it from the required properties for SA8775P.
--

--- Patch [2]: [PATCH v1 2/4] phy: qcom: qmp-pcie: Add qref and refgen regulator vote for QCS8300 and SA8775p PHY ---
commit c5c33c8f1974732f049e9666bff7aa68ba984073
Author: Ziyue Zhang <[email protected]>

phy: qcom: qmp-pcie: Add qref and refgen regulator vote for QCS8300 and SA8775p PHY

This commit adds a new sa8775p_qmp_phy_vreg_l regulator list containing 
additional supplies to support QCS8300 and SA8775p PCIe PHY configurations. 
It updates the regulator lists for the relevant PHY configurations to apply 
a hardware workaround without affecting SM8550.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index d3effad7a074b..b60fee762dabe 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
[ ... ]
> @@ -4394,8 +4398,8 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
>  
>  	.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),
> +	.vreg_list		= sa8775p_qmp_phy_vreg_l,
> +	.num_vregs		= ARRAY_SIZE(sa8775p_qmp_phy_vreg_l),
>  	.regs			= pciephy_v5_regs_layout,

[Severity: High]
The commit message mentions updating the QCS8300 configuration, but this
modifies sm8450_qmp_gen4x2_pciephy_cfg instead. 

Will this cause SM8450 platforms to fail probe since they will now request
5 regulators instead of the 2 normally provided by the DT? 

Also, does this mean qcs8300_qmp_gen4x2_pciephy_cfg remains unmodified and
misses the intended hardware workaround?

[ ... ]
> @@ -4563,8 +4567,8 @@ static const struct qmp_phy_cfg sa8775p_qmp_gen4x2_pciephy_cfg = {
>  
>  	.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),
> +	.vreg_list		= sa8775p_qmp_phy_vreg_l,
> +	.num_vregs		= ARRAY_SIZE(sa8775p_qmp_phy_vreg_l),
>  	.regs			= pciephy_v5_regs_layout,

[Severity: Medium]
This unconditionally requires the refgen supply for the SA8775P PHY, but
the DT binding schema explicitly omits refgen-supply from the required
properties for SA8775P.

If an SA8775P platform boots with a compliant device tree that omits the
optional refgen-supply, will the driver fail to probe here when fetching
the bulk regulators?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy