diff mbox series

[v6,06/12] power: supply: smb347-charger: Make smb347_set_writable() IRQ-safe

Message ID 20210731173842.19643-7-digetx@gmail.com
State New
Headers show
Series Add OTG mode support to Tegra USB PHY, SMB347 and Nexus 7 | expand

Commit Message

Dmitry Osipenko July 31, 2021, 5:38 p.m. UTC
The smb347_set_writable() is used by interrupt handler and outside of it.
The interrupt should be disabled when the function is used outside of
interrupt handler in order to prevent racing with the interrupt context.
Add new parameter to smb347_set_writable() that allows to disable IRQ.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/power/supply/smb347-charger.c | 30 +++++++++++++++++++--------
 1 file changed, 21 insertions(+), 9 deletions(-)

Comments

Sebastian Reichel Aug. 6, 2021, 9:13 p.m. UTC | #1
Hi,

On Sat, Jul 31, 2021 at 08:38:36PM +0300, Dmitry Osipenko wrote:
> The smb347_set_writable() is used by interrupt handler and outside of it.
> The interrupt should be disabled when the function is used outside of
> interrupt handler in order to prevent racing with the interrupt context.
> Add new parameter to smb347_set_writable() that allows to disable IRQ.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/smb347-charger.c | 30 +++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
> index df240420f2de..db1378b41f80 100644
> --- a/drivers/power/supply/smb347-charger.c
> +++ b/drivers/power/supply/smb347-charger.c
> @@ -671,10 +671,22 @@ static int smb347_set_temp_limits(struct smb347_charger *smb)
>   *
>   * Returns %0 on success and negative errno in case of failure.
>   */
> -static int smb347_set_writable(struct smb347_charger *smb, bool writable)
> +static int smb347_set_writable(struct smb347_charger *smb, bool writable,
> +			       bool irq_toggle)
>  {
> -	return regmap_update_bits(smb->regmap, CMD_A, CMD_A_ALLOW_WRITE,
> -				  writable ? CMD_A_ALLOW_WRITE : 0);
> +	struct i2c_client *client = to_i2c_client(smb->dev);
> +	int ret;
> +
> +	if (writable && irq_toggle && !smb->irq_unsupported)
> +		disable_irq(client->irq);
> +
> +	ret = regmap_update_bits(smb->regmap, CMD_A, CMD_A_ALLOW_WRITE,
> +				 writable ? CMD_A_ALLOW_WRITE : 0);
> +
> +	if ((!writable || ret) && irq_toggle && !smb->irq_unsupported)
> +		enable_irq(client->irq);
> +
> +	return ret;
>  }
>  
>  static int smb347_hw_init(struct smb347_charger *smb)
> @@ -682,7 +694,7 @@ static int smb347_hw_init(struct smb347_charger *smb)
>  	unsigned int val;
>  	int ret;
>  
> -	ret = smb347_set_writable(smb, true);
> +	ret = smb347_set_writable(smb, true, false);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -758,7 +770,7 @@ static int smb347_hw_init(struct smb347_charger *smb)
>  	ret = smb347_start_stop_charging(smb);
>  
>  fail:
> -	smb347_set_writable(smb, false);
> +	smb347_set_writable(smb, false, false);
>  	return ret;
>  }
>  
> @@ -866,7 +878,7 @@ static int smb347_irq_set(struct smb347_charger *smb, bool enable)
>  	if (smb->irq_unsupported)
>  		return 0;
>  
> -	ret = smb347_set_writable(smb, true);
> +	ret = smb347_set_writable(smb, true, true);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -891,7 +903,7 @@ static int smb347_irq_set(struct smb347_charger *smb, bool enable)
>  	ret = regmap_update_bits(smb->regmap, CFG_PIN, CFG_PIN_EN_CHARGER_ERROR,
>  				 enable ? CFG_PIN_EN_CHARGER_ERROR : 0);
>  fail:
> -	smb347_set_writable(smb, false);
> +	smb347_set_writable(smb, false, true);
>  	return ret;
>  }
>  
> @@ -919,7 +931,7 @@ static int smb347_irq_init(struct smb347_charger *smb,
>  	if (!client->irq)
>  		return 0;
>  
> -	ret = smb347_set_writable(smb, true);
> +	ret = smb347_set_writable(smb, true, false);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -931,7 +943,7 @@ static int smb347_irq_init(struct smb347_charger *smb,
>  				 CFG_STAT_ACTIVE_HIGH | CFG_STAT_DISABLED,
>  				 CFG_STAT_DISABLED);
>  
> -	smb347_set_writable(smb, false);
> +	smb347_set_writable(smb, false, false);
>  
>  	if (ret < 0) {
>  		dev_warn(smb->dev, "failed to initialize IRQ: %d\n", ret);
> -- 
> 2.32.0
>
diff mbox series

Patch

diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
index df240420f2de..db1378b41f80 100644
--- a/drivers/power/supply/smb347-charger.c
+++ b/drivers/power/supply/smb347-charger.c
@@ -671,10 +671,22 @@  static int smb347_set_temp_limits(struct smb347_charger *smb)
  *
  * Returns %0 on success and negative errno in case of failure.
  */
-static int smb347_set_writable(struct smb347_charger *smb, bool writable)
+static int smb347_set_writable(struct smb347_charger *smb, bool writable,
+			       bool irq_toggle)
 {
-	return regmap_update_bits(smb->regmap, CMD_A, CMD_A_ALLOW_WRITE,
-				  writable ? CMD_A_ALLOW_WRITE : 0);
+	struct i2c_client *client = to_i2c_client(smb->dev);
+	int ret;
+
+	if (writable && irq_toggle && !smb->irq_unsupported)
+		disable_irq(client->irq);
+
+	ret = regmap_update_bits(smb->regmap, CMD_A, CMD_A_ALLOW_WRITE,
+				 writable ? CMD_A_ALLOW_WRITE : 0);
+
+	if ((!writable || ret) && irq_toggle && !smb->irq_unsupported)
+		enable_irq(client->irq);
+
+	return ret;
 }
 
 static int smb347_hw_init(struct smb347_charger *smb)
@@ -682,7 +694,7 @@  static int smb347_hw_init(struct smb347_charger *smb)
 	unsigned int val;
 	int ret;
 
-	ret = smb347_set_writable(smb, true);
+	ret = smb347_set_writable(smb, true, false);
 	if (ret < 0)
 		return ret;
 
@@ -758,7 +770,7 @@  static int smb347_hw_init(struct smb347_charger *smb)
 	ret = smb347_start_stop_charging(smb);
 
 fail:
-	smb347_set_writable(smb, false);
+	smb347_set_writable(smb, false, false);
 	return ret;
 }
 
@@ -866,7 +878,7 @@  static int smb347_irq_set(struct smb347_charger *smb, bool enable)
 	if (smb->irq_unsupported)
 		return 0;
 
-	ret = smb347_set_writable(smb, true);
+	ret = smb347_set_writable(smb, true, true);
 	if (ret < 0)
 		return ret;
 
@@ -891,7 +903,7 @@  static int smb347_irq_set(struct smb347_charger *smb, bool enable)
 	ret = regmap_update_bits(smb->regmap, CFG_PIN, CFG_PIN_EN_CHARGER_ERROR,
 				 enable ? CFG_PIN_EN_CHARGER_ERROR : 0);
 fail:
-	smb347_set_writable(smb, false);
+	smb347_set_writable(smb, false, true);
 	return ret;
 }
 
@@ -919,7 +931,7 @@  static int smb347_irq_init(struct smb347_charger *smb,
 	if (!client->irq)
 		return 0;
 
-	ret = smb347_set_writable(smb, true);
+	ret = smb347_set_writable(smb, true, false);
 	if (ret < 0)
 		return ret;
 
@@ -931,7 +943,7 @@  static int smb347_irq_init(struct smb347_charger *smb,
 				 CFG_STAT_ACTIVE_HIGH | CFG_STAT_DISABLED,
 				 CFG_STAT_DISABLED);
 
-	smb347_set_writable(smb, false);
+	smb347_set_writable(smb, false, false);
 
 	if (ret < 0) {
 		dev_warn(smb->dev, "failed to initialize IRQ: %d\n", ret);