On Di, 2025-02-11 at 17:42 +0800, Wenbin Yao wrote:
> From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
>
> Decide the in-driver logic based on whether the nocsr reset is present
> and defer checking the appropriateness of that to dt-bindings to save
> on boilerplate.
>
> Reset controller APIs are fine consuming a nullptr, so no additional
> checks are necessary there.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> Signed-off-by: Wenbin Yao <quic_wenbyao@xxxxxxxxxxx>
> Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 873f2f9844c6..ac42e4b01065 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -2793,8 +2793,6 @@ struct qmp_phy_cfg {
>
> bool skip_start_delay;
>
> - bool has_nocsr_reset;
> -
> /* QMP PHY pipe clock interface rate */
> unsigned long pipe_clock_rate;
>
> @@ -3685,7 +3683,6 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
>
> .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
> .phy_status = PHYSTATUS_4_20,
> - .has_nocsr_reset = true,
>
> /* 20MHz PHY AUX Clock */
> .aux_clock_rate = 20000000,
> @@ -3718,7 +3715,6 @@ static const struct qmp_phy_cfg sm8650_qmp_gen4x2_pciephy_cfg = {
>
> .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
> .phy_status = PHYSTATUS_4_20,
> - .has_nocsr_reset = true,
>
> /* 20MHz PHY AUX Clock */
> .aux_clock_rate = 20000000,
> @@ -3836,7 +3832,6 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x2_pciephy_cfg = {
>
> .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
> .phy_status = PHYSTATUS_4_20,
> - .has_nocsr_reset = true,
> };
>
> static const struct qmp_phy_cfg x1e80100_qmp_gen4x4_pciephy_cfg = {
> @@ -3870,7 +3865,6 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x4_pciephy_cfg = {
>
> .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
> .phy_status = PHYSTATUS_4_20,
> - .has_nocsr_reset = true,
> };
>
> static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
> @@ -3902,7 +3896,6 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
>
> .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
> .phy_status = PHYSTATUS_4_20,
> - .has_nocsr_reset = true,
> };
>
> static void qmp_pcie_init_port_b(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tbls *tbls)
> @@ -4203,11 +4196,14 @@ static int qmp_pcie_reset_init(struct qmp_pcie *qmp)
> if (ret)
> return dev_err_probe(dev, ret, "failed to get resets\n");
>
> - if (cfg->has_nocsr_reset) {
> - qmp->nocsr_reset = devm_reset_control_get_exclusive(dev, "phy_nocsr");
> - if (IS_ERR(qmp->nocsr_reset))
> + qmp->nocsr_reset = devm_reset_control_get_exclusive(dev, "phy_nocsr");
> + if (IS_ERR(qmp->nocsr_reset)) {
> + if (PTR_ERR(qmp->nocsr_reset) == -ENOENT ||
> + PTR_ERR(qmp->nocsr_reset) == -EINVAL)
Why is -EINVAL ignored here?
Without this you could just use
devm_reset_control_get_optional_exclusive(), which already turns -
ENOENT into NULL. That seems to me the correct thing to do, as from
driver point-of-view, this reset control is optional.
> + qmp->nocsr_reset = NULL;
> + else
> return dev_err_probe(dev, PTR_ERR(qmp->nocsr_reset),
> - "failed to get no-csr reset\n");
> + "failed to get no-csr reset\n");
> }
>
> return 0;
regards
Philipp
