diff mbox series

usb: typec: qcom: check regulator enable status before disabling it

Message ID 20230823-qcom-tcpc-v1-1-fa81a09ca056@quicinc.com (mailing list archive)
State Superseded
Headers show
Series usb: typec: qcom: check regulator enable status before disabling it | expand

Commit Message

Hui Liu via B4 Relay Aug. 23, 2023, 9:15 a.m. UTC
From: Hui Liu <quic_huliu@quicinc.com>

Check regulator enable status before disabling it to avoid
unbalanced regulator disable warnings.

Signed-off-by: Hui Liu <quic_huliu@quicinc.com>
---
 drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: bbb9e06d2c6435af9c62074ad7048910eeb2e7bc
change-id: 20230822-qcom-tcpc-d41954ac65fa

Best regards,

Comments

Bryan O'Donoghue Aug. 23, 2023, 9:53 a.m. UTC | #1
On 23/08/2023 10:15, Hui Liu via B4 Relay wrote:
> From: Hui Liu <quic_huliu@quicinc.com>
> 
> Check regulator enable status before disabling it to avoid
> unbalanced regulator disable warnings.
> 
> Signed-off-by: Hui Liu <quic_huliu@quicinc.com>
> ---
>   drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> index bb0b8479d80f..ca616b17b5b6 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> @@ -422,7 +422,8 @@ static int qcom_pmic_typec_pdphy_disable(struct pmic_typec_pdphy *pmic_typec_pdp
>   	ret = regmap_write(pmic_typec_pdphy->regmap,
>   			   pmic_typec_pdphy->base + USB_PDPHY_EN_CONTROL_REG, 0);
>   
> -	regulator_disable(pmic_typec_pdphy->vdd_pdphy);
> +	if (regulator_is_enabled(pmic_typec_pdphy->vdd_pdphy))
> +		regulator_disable(pmic_typec_pdphy->vdd_pdphy);
>   
>   	return ret;
>   }
> 
> ---
> base-commit: bbb9e06d2c6435af9c62074ad7048910eeb2e7bc
> change-id: 20230822-qcom-tcpc-d41954ac65fa
> 
> Best regards,

Is this a fix for a real bug you've seen or a hypothetical use-case fix ?

---
bod
hui liu Aug. 23, 2023, 11:06 a.m. UTC | #2
Hi Bryan:

This change is used to fix a real bug.
In qcom tcpc driver probe function, "qcom_pmic_typec_pdphy_start" 
function will be called, thus "qcom_pmic_typec_pdphy_disable" function 
was called. But at that time the regulator was not enabled, so it lead 
to unbalanced between enable and disable of regulator. Then below error 
logs were output:

[  150.485973] unbalanced disables for smb2352_dummy_reg
[  150.486037] ------------[ cut here ]------------
[  150.490020] WARNING: CPU: 1 PID: 2476 at 
/local/mnt3/workspace/qcs405-087/build-qti-distro-fullstack-noselinux-debug/tmp-glibc/work-shared/qcs405-pine/kernel-source/drivers/regulator/core.c:2285 
_regulatdisable+0x15c/0x164
[  150.494751] Modules linked in: qcom_pmic_tcpm(+)
[  150.514577] CPU: 1 PID: 2476 Comm: insmod Tainted: G        W 
4.14.162 #1
[  150.519268] Hardware name: Qualcomm Technologies, Inc. QCS405 Turbox 
C405 DVT IOT WITH PINE (DT)
[  150.526307] task: 000000004869024f task.stack: 00000000db4a9141
[  150.535329] pc : _regulator_disable+0x15c/0x164
[  150.540962] lr : _regulator_disable+0x158/0x164
[  150.545475] sp : ffffff80109d3920 pstate : 60400145
[  150.549988] x29: ffffff80109d3940 x28: ffffff80109d3e18
[  150.554854] x27: ffffffd16adac0e8 x26: 0000000000000001
[  150.560407] x25: 0000000000000001 x24: ffffffd171913680
[  150.565702] x23: 0000000000000000 x22: ffffffd16ad16500
[  150.570997] x21: ffffffd1718f1f10 x20: 00000000fffffffb
[  150.576293] x19: ffffffd1718f1e80 x18: 00000000000000d8
[  150.581588] x17: 00000000000000d8 x16: 0000000000000036
[  150.586883] x15: ffffff91dfb54328 x14: 0000000000003733
[  150.592178] x13: 0000000000000004 x12: 00000000ac3774ff
[  150.597474] x11: 00000000ac3774ff x10: 0000000000000015
[  150.602770] x9 : b5bcc964516bcc00 x8 : b5bcc964516bcc00
[  150.608064] x7 : 5f32353332626d73 x6 : ffffff91e06e6171
[  150.613359] x5 : 0000000000000000 x4 : 0000000000000008
[  150.618654] x3 : 0000000000fd7195 x2 : ffffffd177487c78
[  150.623949] x1 : ffffff91dff905bc x0 : 0000000000000029
[  150.629244]
[  150.629244] PC: 0xffffff91df3dda0c:
[  150.634539] da0c  f84307f5 d65f03c0 f9421a68 b4000068 f9400101 
b50000e1 f9400268 d0006129
[  150.639496] da2c  9127c529 f9400108 f100011f 9a880121 90005900 
9126d800 97ed1ccd 321d7bf4
[  150.647568] da4c  d4210000 17ffffec f81c0ff7 a90157f6 a9024ff4 
a9037bfd 9100c3fd f9402c17
[  150.655729] da6c  aa0003f4 910242f3 aa1303e0 941df710 f9402e96 
52808401 aa1f03e2 b9001e9f
[  150.663890]
[  150.663890] LR: 0xffffff91df3dda08:
[  150.672039] da08  a9414ff4 f84307f5 d65f03c0 f9421a68 b4000068 
f9400101 b50000e1 f9400268
[  150.677082] da28  d0006129 9127c529 f9400108 f100011f 9a880121 
90005900 9126d800 97ed1ccd
[  150.685155] da48  321d7bf4 d4210000 17ffffec f81c0ff7 a90157f6 
a9024ff4 a9037bfd 9100c3fd
[  150.693316] da68  f9402c17 aa0003f4 910242f3 aa1303e0 941df710 
f9402e96 52808401 aa1f03e2
[  150.701475]
[  150.701475] SP: 0xffffff80109d38e0:
[  150.709624] 38e0  df3dda4c ffffff91 60400145 00000000 ffffffc8 
ffffff80 516bcc00 b5bcc964
[  150.714669] 3900  ffffffff 0000007f df512f6c ffffff91 109d3940 
ffffff80 df3dda4c ffffff91
[  150.722743] 3920  718f1f10 ffffffd1 dfb5b6dc ffffff91 6ffed980 
ffffffd1 718f1e80 ffffffd1
[  150.730903] 3940  109d3970 ffffff80 df3dd8a4 ffffff91 6ad16500 
ffffffd1 00000000 00000000
[  150.739062]
[  150.747202] Call trace:
[  150.748771]  _regulator_disable+0x15c/0x164
[  150.750943]  regulator_disable+0x34/0x80
[  150.755118]  qcom_pmic_typec_pdphy_start+0x40/0x144 [qcom_pmic_tcpm]
[  150.759289]  qcom_pmic_typec_probe+0x2e8/0x300 [qcom_pmic_tcpm]
[  150.765619]  platform_drv_probe+0x5c/0xb0
[  150.771256]  driver_probe_device+0x2e8/0x41c
[  150.775425]  __driver_attach+0x90/0x114
[  150.779762]  bus_for_each_dev+0x80/0xc8
[  150.783321]  driver_attach+0x20/0x28
[  150.787140]  bus_add_driver+0x138/0x240
[  150.790960]  driver_register+0x8c/0xd8
[  150.794519]  __platform_driver_register+0x40/0x48
[  150.798346]  init_module+0x2c/0x1000 [qcom_pmic_tcpm]
[  150.803119]  do_one_initcall+0xdc/0x1b0
[  150.808147]  do_init_module+0x60/0x1c4
[  150.811793]  load_module+0x23cc/0x2790
[  150.815613]  SyS_finit_module+0xbc/0x110
[  150.819345]  el0_svc_naked+0x34/0x38
[  150.823424] ---[ end trace b3846aa77ad5b668 ]---

So we add a conditional judgment before disable the regulator to fix the 
issue simply.

在 8/23/2023 5:53 PM, Bryan O'Donoghue 写道:
> On 23/08/2023 10:15, Hui Liu via B4 Relay wrote:
>> From: Hui Liu <quic_huliu@quicinc.com>
>>
>> Check regulator enable status before disabling it to avoid
>> unbalanced regulator disable warnings.
>>
>> Signed-off-by: Hui Liu <quic_huliu@quicinc.com>
>> ---
>>   drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c 
>> b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
>> index bb0b8479d80f..ca616b17b5b6 100644
>> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
>> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
>> @@ -422,7 +422,8 @@ static int qcom_pmic_typec_pdphy_disable(struct 
>> pmic_typec_pdphy *pmic_typec_pdp
>>       ret = regmap_write(pmic_typec_pdphy->regmap,
>>                  pmic_typec_pdphy->base + USB_PDPHY_EN_CONTROL_REG, 0);
>> -    regulator_disable(pmic_typec_pdphy->vdd_pdphy);
>> +    if (regulator_is_enabled(pmic_typec_pdphy->vdd_pdphy))
>> +        regulator_disable(pmic_typec_pdphy->vdd_pdphy);
>>       return ret;
>>   }
>>
>> ---
>> base-commit: bbb9e06d2c6435af9c62074ad7048910eeb2e7bc
>> change-id: 20230822-qcom-tcpc-d41954ac65fa
>>
>> Best regards,
> 
> Is this a fix for a real bug you've seen or a hypothetical use-case fix ?
> 
> ---
> bod
Bryan O'Donoghue Aug. 23, 2023, 11:13 a.m. UTC | #3
On 23/08/2023 12:06, hui liu wrote:
> Hi Bryan:
> 
> This change is used to fix a real bug.

okie dokie

Could you please repost a v2 with a

Fixes: a4422ff22142 ("usb: typec: qcom: Add Qualcomm PMIC Type-C driver")

Cc: stable@vger.kernel.org

And please add my

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod
Guenter Roeck Aug. 23, 2023, 1:45 p.m. UTC | #4
On Wed, Aug 23, 2023 at 05:15:53PM +0800, Hui Liu via B4 Relay wrote:
> From: Hui Liu <quic_huliu@quicinc.com>
> 
> Check regulator enable status before disabling it to avoid
> unbalanced regulator disable warnings.
> 
> Signed-off-by: Hui Liu <quic_huliu@quicinc.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> index bb0b8479d80f..ca616b17b5b6 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> @@ -422,7 +422,8 @@ static int qcom_pmic_typec_pdphy_disable(struct pmic_typec_pdphy *pmic_typec_pdp
>  	ret = regmap_write(pmic_typec_pdphy->regmap,
>  			   pmic_typec_pdphy->base + USB_PDPHY_EN_CONTROL_REG, 0);
>  
> -	regulator_disable(pmic_typec_pdphy->vdd_pdphy);
> +	if (regulator_is_enabled(pmic_typec_pdphy->vdd_pdphy))
> +		regulator_disable(pmic_typec_pdphy->vdd_pdphy);
>  
>  	return ret;
>  }
> 
> ---
> base-commit: bbb9e06d2c6435af9c62074ad7048910eeb2e7bc
> change-id: 20230822-qcom-tcpc-d41954ac65fa
> 
> Best regards,
> -- 
> Hui Liu <quic_huliu@quicinc.com>
>
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
index bb0b8479d80f..ca616b17b5b6 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
@@ -422,7 +422,8 @@  static int qcom_pmic_typec_pdphy_disable(struct pmic_typec_pdphy *pmic_typec_pdp
 	ret = regmap_write(pmic_typec_pdphy->regmap,
 			   pmic_typec_pdphy->base + USB_PDPHY_EN_CONTROL_REG, 0);
 
-	regulator_disable(pmic_typec_pdphy->vdd_pdphy);
+	if (regulator_is_enabled(pmic_typec_pdphy->vdd_pdphy))
+		regulator_disable(pmic_typec_pdphy->vdd_pdphy);
 
 	return ret;
 }