Re: [PATCH] extcon: arizona: Correct error handling on regmap_update_bits_check
On Wed, May 29, 2019 at 02:37:06PM +0900, Chanwoo Choi wrote: > Hi, > > On 19. 5. 29. 오전 1:50, Charles Keepax wrote: > > Ensure the case when regmap_update_bits_check fails and the change > > variable is not updated is handled correctly. > > > > Signed-off-by: Charles Keepax > > --- > > regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, > > ARIZONA_MICD_ENA, 0, > > You better to check the return value as the part of this patch. > Ok I will add it, I hadn't as it was kinda pointless the driver is being unbound and if this doesn't work the regulator stuff will flag the error anyway. But I guess one more error message to make it even more clear where the issue is can't hurt. Thanks, Charles
Re: [PATCH] extcon: arizona: Correct error handling on regmap_update_bits_check
Hi, On 19. 5. 29. 오전 1:50, Charles Keepax wrote: > Ensure the case when regmap_update_bits_check fails and the change > variable is not updated is handled correctly. > > Signed-off-by: Charles Keepax > --- > drivers/extcon/extcon-arizona.c | 22 +- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c > index 9327479c719c2..ba2d16de161f8 100644 > --- a/drivers/extcon/extcon-arizona.c > +++ b/drivers/extcon/extcon-arizona.c > @@ -335,10 +335,12 @@ static void arizona_start_mic(struct > arizona_extcon_info *info) > > arizona_extcon_pulse_micbias(info); > > - regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, > - ARIZONA_MICD_ENA, ARIZONA_MICD_ENA, > - &change); > - if (!change) { > + ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, > +ARIZONA_MICD_ENA, ARIZONA_MICD_ENA, > +&change); > + if (ret < 0) { > + dev_err(arizona->dev, "Failed to enable micd: %d\n", ret); > + } else if (!change) { > regulator_disable(info->micvdd); > pm_runtime_put_autosuspend(info->dev); > } > @@ -350,12 +352,14 @@ static void arizona_stop_mic(struct arizona_extcon_info > *info) > const char *widget = arizona_extcon_get_micbias(info); > struct snd_soc_dapm_context *dapm = arizona->dapm; > struct snd_soc_component *component = snd_soc_dapm_to_component(dapm); > - bool change; > + bool change = false; > int ret; > > - regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, > - ARIZONA_MICD_ENA, 0, > - &change); > + ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, > +ARIZONA_MICD_ENA, 0, > +&change); > + if (ret < 0) > + dev_err(arizona->dev, "Failed to disable micd: %d\n", ret); > > ret = snd_soc_component_disable_pin(component, widget); > if (ret != 0) > @@ -1726,7 +1730,7 @@ static int arizona_extcon_remove(struct platform_device > *pdev) > struct arizona_extcon_info *info = platform_get_drvdata(pdev); > struct arizona *arizona = info->arizona; > int jack_irq_rise, jack_irq_fall; > - bool change; > + bool change = false; > > regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, >ARIZONA_MICD_ENA, 0, You better to check the return value as the part of this patch. -- Best Regards, Chanwoo Choi Samsung Electronics
[PATCH] extcon: arizona: Correct error handling on regmap_update_bits_check
Ensure the case when regmap_update_bits_check fails and the change variable is not updated is handled correctly. Signed-off-by: Charles Keepax --- drivers/extcon/extcon-arizona.c | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index 9327479c719c2..ba2d16de161f8 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -335,10 +335,12 @@ static void arizona_start_mic(struct arizona_extcon_info *info) arizona_extcon_pulse_micbias(info); - regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, -ARIZONA_MICD_ENA, ARIZONA_MICD_ENA, -&change); - if (!change) { + ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, + ARIZONA_MICD_ENA, ARIZONA_MICD_ENA, + &change); + if (ret < 0) { + dev_err(arizona->dev, "Failed to enable micd: %d\n", ret); + } else if (!change) { regulator_disable(info->micvdd); pm_runtime_put_autosuspend(info->dev); } @@ -350,12 +352,14 @@ static void arizona_stop_mic(struct arizona_extcon_info *info) const char *widget = arizona_extcon_get_micbias(info); struct snd_soc_dapm_context *dapm = arizona->dapm; struct snd_soc_component *component = snd_soc_dapm_to_component(dapm); - bool change; + bool change = false; int ret; - regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, -ARIZONA_MICD_ENA, 0, -&change); + ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, + ARIZONA_MICD_ENA, 0, + &change); + if (ret < 0) + dev_err(arizona->dev, "Failed to disable micd: %d\n", ret); ret = snd_soc_component_disable_pin(component, widget); if (ret != 0) @@ -1726,7 +1730,7 @@ static int arizona_extcon_remove(struct platform_device *pdev) struct arizona_extcon_info *info = platform_get_drvdata(pdev); struct arizona *arizona = info->arizona; int jack_irq_rise, jack_irq_fall; - bool change; + bool change = false; regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1, ARIZONA_MICD_ENA, 0, -- 2.11.0