diff mbox series

[v1,3/3] ASoC: codecs: wcd937x: Remove separate handling for vdd-buck supply

Message ID 20240628101143.1508513-4-quic_mohs@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Fix the unbalanced pm_runtime_enable in wcd937x-sdw soundwire slave | expand

Commit Message

Mohammad Rafi Shaik June 28, 2024, 10:11 a.m. UTC 
Remove separate handling for vdd-buck regulator supply which is not
required. The vdd-buck regulator supply enabled using bulk enable.

Add the error handling in wcd937x_probe() and disable the regulators in
error case.

Replace free_irq() with devm_free_irq() because irq have been requested
wth devm_request_threaded_irq().

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 sound/soc/codecs/wcd937x.c | 67 +++++++++-----------------------------
 sound/soc/codecs/wcd937x.h |  2 +-
 2 files changed, 17 insertions(+), 52 deletions(-)

Comments

Mark Brown June 28, 2024, 12:42 p.m. UTC | #1 
On Fri, Jun 28, 2024 at 03:41:43PM +0530, Mohammad Rafi Shaik wrote:

> Replace free_irq() with devm_free_irq() because irq have been requested
> wth devm_request_threaded_irq().

This seems like an unrelated change, and TBH if explict frees are
required I'd question if devm_ should be being used in the first place.
Mohammad Rafi Shaik July 1, 2024, 2:04 p.m. UTC | #2 
On 6/28/2024 6:12 PM, Mark Brown wrote:
> On Fri, Jun 28, 2024 at 03:41:43PM +0530, Mohammad Rafi Shaik wrote:
> 
Thanks for the review.
>> Replace free_irq() with devm_free_irq() because irq have been requested
>> wth devm_request_threaded_irq().
> 
> This seems like an unrelated change, and TBH if explict frees are
> required I'd question if devm_ should be being used in the first place.

okay, will remove this specific change from patch change.

Based on review comments added devm_free_irq().

https://lore.kernel.org/all/f741630e-f9e8-45e1-b3e2-4866f0baeac2@sirena.org.uk/

Thanks and Regards,
Rafi.
diff mbox series

Patch

diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index 94282499fa33..b696305c0817 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -869,37 +869,6 @@  static int wcd937x_enable_rx3(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int wcd937x_codec_enable_vdd_buck(struct snd_soc_dapm_widget *w,
-					 struct snd_kcontrol *kcontrol,
-					 int event)
-{
-	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
-	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
-	int ret = 0;
-
-	switch (event) {
-	case SND_SOC_DAPM_PRE_PMU:
-		if (test_bit(ALLOW_BUCK_DISABLE, &wcd937x->status_mask)) {
-			dev_err(component->dev, "buck already in enabled state\n");
-			clear_bit(ALLOW_BUCK_DISABLE, &wcd937x->status_mask);
-			return 0;
-		}
-		ret = regulator_enable(wcd937x->buck_supply);
-		if (ret) {
-			dev_err(component->dev, "VDD_BUCK is not enabled\n");
-			return ret;
-		}
-		clear_bit(ALLOW_BUCK_DISABLE, &wcd937x->status_mask);
-		usleep_range(200, 250);
-		break;
-	case SND_SOC_DAPM_POST_PMD:
-		set_bit(ALLOW_BUCK_DISABLE, &wcd937x->status_mask);
-		break;
-	}
-
-	return 0;
-}
-
 static int wcd937x_get_micb_vout_ctl_val(u32 micb_mv)
 {
 	if (micb_mv < 1000 || micb_mv > 2850) {
@@ -2226,10 +2195,7 @@  static const struct snd_soc_dapm_widget wcd937x_dapm_widgets[] = {
 			    SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMU |
 			    SND_SOC_DAPM_POST_PMD),
 
-	SND_SOC_DAPM_SUPPLY("VDD_BUCK", SND_SOC_NOPM, 0, 0,
-			    wcd937x_codec_enable_vdd_buck,
-			    SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
-
+	SND_SOC_DAPM_SUPPLY("VDD_BUCK", SND_SOC_NOPM, 0, 0, NULL, 0),
 	SND_SOC_DAPM_SUPPLY_S("CLS_H_PORT", 1, SND_SOC_NOPM, 0, 0, NULL, 0),
 
 	/* RX widgets */
@@ -2614,9 +2580,9 @@  static void wcd937x_soc_codec_remove(struct snd_soc_component *component)
 	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
 
 	wcd937x_mbhc_deinit(component);
-	free_irq(wcd937x->aux_pdm_wd_int, wcd937x);
-	free_irq(wcd937x->hphl_pdm_wd_int, wcd937x);
-	free_irq(wcd937x->hphr_pdm_wd_int, wcd937x);
+	devm_free_irq(component->dev, wcd937x->aux_pdm_wd_int, wcd937x);
+	devm_free_irq(component->dev, wcd937x->hphl_pdm_wd_int, wcd937x);
+	devm_free_irq(component->dev, wcd937x->hphr_pdm_wd_int, wcd937x);
 
 	wcd_clsh_ctrl_free(wcd937x->clsh_info);
 }
@@ -2915,24 +2881,17 @@  static int wcd937x_probe(struct platform_device *pdev)
 	wcd937x->supplies[0].supply = "vdd-rxtx";
 	wcd937x->supplies[1].supply = "vdd-px";
 	wcd937x->supplies[2].supply = "vdd-mic-bias";
+	wcd937x->supplies[3].supply = "vdd-buck";
 
 	ret = devm_regulator_bulk_get(dev, WCD937X_MAX_BULK_SUPPLY, wcd937x->supplies);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to get supplies\n");
 
 	ret = regulator_bulk_enable(WCD937X_MAX_BULK_SUPPLY, wcd937x->supplies);
-	if (ret)
+	if (ret) {
+		regulator_bulk_free(WCD937X_MAX_BULK_SUPPLY, wcd937x->supplies);
 		return dev_err_probe(dev, ret, "Failed to enable supplies\n");
-
-	/* Get the buck separately, as it needs special handling */
-	wcd937x->buck_supply = devm_regulator_get(dev, "vdd-buck");
-	if (IS_ERR(wcd937x->buck_supply))
-		return dev_err_probe(dev, PTR_ERR(wcd937x->buck_supply),
-				     "Failed to get buck supply\n");
-
-	ret = regulator_enable(wcd937x->buck_supply);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to enable buck supply\n");
+	}
 
 	wcd937x_dt_parse_micbias_info(dev, wcd937x);
 
@@ -2949,13 +2908,13 @@  static int wcd937x_probe(struct platform_device *pdev)
 
 	ret = wcd937x_add_slave_components(wcd937x, dev, &match);
 	if (ret)
-		return ret;
+		goto err_disable_regulators;
 
 	wcd937x_reset(wcd937x);
 
 	ret = component_master_add_with_match(dev, &wcd937x_comp_ops, match);
 	if (ret)
-		return ret;
+		goto err_disable_regulators;
 
 	pm_runtime_set_autosuspend_delay(dev, 1000);
 	pm_runtime_use_autosuspend(dev);
@@ -2964,6 +2923,12 @@  static int wcd937x_probe(struct platform_device *pdev)
 	pm_runtime_enable(dev);
 	pm_runtime_idle(dev);
 
+	return 0;
+
+err_disable_regulators:
+	regulator_bulk_disable(WCD937X_MAX_BULK_SUPPLY, wcd937x->supplies);
+	regulator_bulk_free(WCD937X_MAX_BULK_SUPPLY, wcd937x->supplies);
+
 	return ret;
 }
 
diff --git a/sound/soc/codecs/wcd937x.h b/sound/soc/codecs/wcd937x.h
index f267c66ca959..37bff16e88dd 100644
--- a/sound/soc/codecs/wcd937x.h
+++ b/sound/soc/codecs/wcd937x.h
@@ -483,7 +483,7 @@ 
 #define WCD937X_MAX_REGISTER			(WCD937X_DIGITAL_EFUSE_REG_31)
 
 #define WCD937X_MAX_MICBIAS			3
-#define WCD937X_MAX_BULK_SUPPLY			3
+#define WCD937X_MAX_BULK_SUPPLY			4
 #define WCD937X_MAX_TX_SWR_PORTS		4
 #define WCD937X_MAX_SWR_PORTS			5
 #define WCD937X_MAX_SWR_CH_IDS			15