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@xxxxxxxxx>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
-- 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
>
Attachment:
signature.asc
Description: PGP signature
