diff mbox series

[5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration

Message ID 1534859756-6955-5-git-send-email-loic.poulain@linaro.org (mailing list archive)
State New, archived
Headers show
Series [1/6] usb: chipidea: Add dynamic pinctrl selection | expand

Commit Message

Loic Poulain Aug. 21, 2018, 1:55 p.m. UTC
Phy power on/off cycle can happen several times during device life.
We then need to balance the extcon notifier registration accordingly.

Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-usb-hs.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Bjorn Andersson Aug. 23, 2018, 5:27 p.m. UTC | #1
On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote:

> Phy power on/off cycle can happen several times during device life.
> We then need to balance the extcon notifier registration accordingly.
> 
> Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-usb-hs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> index 2d0c70b..92e9d94 100644
> --- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
> @@ -181,6 +181,12 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
>  {
>  	struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
>  
> +	if (uphy->vbus_edev) {
> +		devm_extcon_unregister_notifier(&uphy->ulpi->dev,
> +						uphy->vbus_edev, EXTCON_USB,
> +						&uphy->vbus_notify);

As I presume the power_on should always be matched with a power_off I
wonder if it really is appropriate to use the devres version of this
API.

Should we drop the devm_ from registration in power_on as well?

> +	}
> +
>  	regulator_disable(uphy->v3p3);
>  	regulator_disable(uphy->v1p8);
>  	clk_disable_unprepare(uphy->sleep_clk);

Regards,
Bjorn
Loic Poulain Aug. 24, 2018, 9:17 a.m. UTC | #2
On 23 August 2018 at 19:27, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> On Tue 21 Aug 06:55 PDT 2018, Loic Poulain wrote:
>
>> Phy power on/off cycle can happen several times during device life.
>> We then need to balance the extcon notifier registration accordingly.
>>
>> Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
>> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>> ---
>>  drivers/phy/qualcomm/phy-qcom-usb-hs.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> index 2d0c70b..92e9d94 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
>> @@ -181,6 +181,12 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
>>  {
>>       struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
>>
>> +     if (uphy->vbus_edev) {
>> +             devm_extcon_unregister_notifier(&uphy->ulpi->dev,
>> +                                             uphy->vbus_edev, EXTCON_USB,
>> +                                             &uphy->vbus_notify);
>
> As I presume the power_on should always be matched with a power_off I
> wonder if it really is appropriate to use the devres version of this
> API.
>
> Should we drop the devm_ from registration in power_on as well?

Yes, thanks, I'll drop the devres version.

Regards,
Loic
diff mbox series

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 2d0c70b..92e9d94 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -181,6 +181,12 @@  static int qcom_usb_hs_phy_power_off(struct phy *phy)
 {
 	struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
 
+	if (uphy->vbus_edev) {
+		devm_extcon_unregister_notifier(&uphy->ulpi->dev,
+						uphy->vbus_edev, EXTCON_USB,
+						&uphy->vbus_notify);
+	}
+
 	regulator_disable(uphy->v3p3);
 	regulator_disable(uphy->v1p8);
 	clk_disable_unprepare(uphy->sleep_clk);