On 11.02.2025 10:53 AM, Philipp Zabel wrote: > 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> >> --- [...] >> 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? If the NOCSR (partial) reset is missing, we can still assert the "full" reset and program the hardware from the ground up. It's also needed for backwards dt compat as not all platforms described it when originally added. > 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. Good point, I forgot _optional_ was a thing in the reset framework Konrad