Make sure to release the lane reset controller in case of a late probe error (e.g. probe deferral).
Note that due to the reset controller being defined in devicetree in (questionable) "lane" child nodes, devm_reset_control_get_exclusive() cannot be used (and we shouldn't add devres helpers for the legacy reset controller API).
Fixes: e78f3d15e115 ("phy: qcom-qmp: new qmp phy driver for qcom-chipsets") Cc: stable@vger.kernel.org # 4.12 Cc: Vivek Gautam vivek.gautam@codeaurora.org Signed-off-by: Johan Hovold johan+linaro@kernel.org --- drivers/phy/qualcomm/phy-qcom-qmp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index a84f7d1fc9b7..3f77830921f5 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -6005,6 +6005,11 @@ static const struct phy_ops qcom_qmp_pcie_ufs_ops = { .owner = THIS_MODULE, };
+static void qcom_qmp_reset_control_put(void *data) +{ + reset_control_put(data); +} + static int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id, void __iomem *serdes, const struct qmp_phy_cfg *cfg) @@ -6099,6 +6104,10 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id, dev_err(dev, "failed to get lane%d reset\n", id); return PTR_ERR(qphy->lane_rst); } + ret = devm_add_action_or_reset(dev, qcom_qmp_reset_control_put, + qphy->lane_rst); + if (ret) + return ret; }
if (cfg->type == PHY_TYPE_UFS || cfg->type == PHY_TYPE_PCIE)
Hi Johan,
On Fr, 2022-04-22 at 15:09 +0200, Johan Hovold wrote:
Make sure to release the lane reset controller in case of a late probe error (e.g. probe deferral).
Right. grepping for "of_reset_control_get", there seem to be are a few other drivers that might share the same issue...
Note that due to the reset controller being defined in devicetree in (questionable) "lane" child nodes, devm_reset_control_get_exclusive() cannot be used (and we shouldn't add devres helpers for the legacy reset controller API).
Do you mean of_reset_control_get()? Maybe you could switch to of_reset_control_get_exclusive() while at it?
That one might warrant a devres helper if other drivers were to adopt the same pattern.
The patch itself looks fine to me,
Reviewed-by: Philipp Zabel p.zabel@pengutronix.de
regards Philipp
On Mon, Apr 25, 2022 at 11:45:29AM +0200, Philipp Zabel wrote:
Hi Johan,
On Fr, 2022-04-22 at 15:09 +0200, Johan Hovold wrote:
Make sure to release the lane reset controller in case of a late probe error (e.g. probe deferral).
Right. grepping for "of_reset_control_get", there seem to be are a few other drivers that might share the same issue...
Yeah, I'm sure there are more of these.
Note that due to the reset controller being defined in devicetree in (questionable) "lane" child nodes, devm_reset_control_get_exclusive() cannot be used (and we shouldn't add devres helpers for the legacy reset controller API).
Do you mean of_reset_control_get()? Maybe you could switch to of_reset_control_get_exclusive() while at it?
Right, I was referring to of_reset_control_get() but obviously of_reset_control_get_exclusive() could still get a devres version so that sentence in parenthesis doesn't make much sense.
I must have mistakingly imagined that the latter also retrieved the struct device_node from a struct device like the current devres helpers do.
That one might warrant a devres helper if other drivers were to adopt the same pattern.
Right.
The patch itself looks fine to me,
Reviewed-by: Philipp Zabel p.zabel@pengutronix.de
Thanks for reviewing. I'll send a v2 with an updated commit message and switch to the new API in a new follow-on patch.
Johan
linux-stable-mirror@lists.linaro.org