diff mbox series

[04/11] ASoC: codecs: rt274: Move irq registration and cleanup

Message ID 20220609133541.3984886-5-amadeuszx.slawinski@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series ASoC: codecs: Series of fixes for realtek codecs used on RVPs | expand

Commit Message

Amadeusz Sławiński June 9, 2022, 1:35 p.m. UTC 
Currently irq is registered when i2c driver is loaded, it is unnecessary
when component is unused. Initialize irq only when we probe ASoC
component.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/codecs/rt274.c | 44 ++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

Comments

Mark Brown June 9, 2022, 2:17 p.m. UTC | #1 
On Thu, Jun 09, 2022 at 03:35:34PM +0200, Amadeusz Sławiński wrote:
> Currently irq is registered when i2c driver is loaded, it is unnecessary
> when component is unused. Initialize irq only when we probe ASoC
> component.

No, this makes things worse - we should acquire resources in the device
level probe so that we handle deferred probe more gracefully, triggering
a defer from the device trying to acquire the resource makes it clearer
what's going on and reduces the amount of work we do on deferred probe
attempts.  Having an interrupt registered has no meaningful cost.
diff mbox series

Patch

diff --git a/sound/soc/codecs/rt274.c b/sound/soc/codecs/rt274.c
index a5615e94ec7d..144a6f775c21 100644
--- a/sound/soc/codecs/rt274.c
+++ b/sound/soc/codecs/rt274.c
@@ -978,13 +978,22 @@  static irqreturn_t rt274_irq(int irq, void *data)
 static int rt274_probe(struct snd_soc_component *component)
 {
 	struct rt274_priv *rt274 = snd_soc_component_get_drvdata(component);
+	int ret;
 
 	rt274->component = component;
 	INIT_DELAYED_WORK(&rt274->jack_detect_work, rt274_jack_detect_work);
 
-	if (rt274->i2c->irq)
-		schedule_delayed_work(&rt274->jack_detect_work,
-				      msecs_to_jiffies(1250));
+	if (rt274->i2c->irq) {
+		ret = request_threaded_irq(rt274->i2c->irq, NULL, rt274_irq,
+					   IRQF_TRIGGER_HIGH | IRQF_ONESHOT, "rt274", rt274);
+		if (ret) {
+			dev_err(&rt274->i2c->dev, "Failed to reguest IRQ: %d\n", ret);
+			return ret;
+		}
+
+		schedule_delayed_work(&rt274->jack_detect_work, msecs_to_jiffies(1250));
+	}
+
 	return 0;
 }
 
@@ -992,7 +1001,12 @@  static void rt274_remove(struct snd_soc_component *component)
 {
 	struct rt274_priv *rt274 = snd_soc_component_get_drvdata(component);
 
-	cancel_delayed_work_sync(&rt274->jack_detect_work);
+	if (rt274->i2c->irq) {
+		cancel_delayed_work_sync(&rt274->jack_detect_work);
+
+		disable_irq(rt274->i2c->irq);
+		free_irq(rt274->i2c->irq, rt274);
+	}
 }
 
 #ifdef CONFIG_PM
@@ -1187,16 +1201,6 @@  static int rt274_i2c_probe(struct i2c_client *i2c)
 	regmap_write(rt274->regmap, RT274_UNSOLICITED_HP_OUT, 0x81);
 	regmap_write(rt274->regmap, RT274_UNSOLICITED_MIC, 0x82);
 
-	if (rt274->i2c->irq) {
-		ret = request_threaded_irq(rt274->i2c->irq, NULL, rt274_irq,
-			IRQF_TRIGGER_HIGH | IRQF_ONESHOT, "rt274", rt274);
-		if (ret != 0) {
-			dev_err(&i2c->dev,
-				"Failed to reguest IRQ: %d\n", ret);
-			return ret;
-		}
-	}
-
 	ret = devm_snd_soc_register_component(&i2c->dev,
 				     &soc_component_dev_rt274,
 				     rt274_dai, ARRAY_SIZE(rt274_dai));
@@ -1204,17 +1208,6 @@  static int rt274_i2c_probe(struct i2c_client *i2c)
 	return ret;
 }
 
-static int rt274_i2c_remove(struct i2c_client *i2c)
-{
-	struct rt274_priv *rt274 = i2c_get_clientdata(i2c);
-
-	if (i2c->irq)
-		free_irq(i2c->irq, rt274);
-
-	return 0;
-}
-
-
 static struct i2c_driver rt274_i2c_driver = {
 	.driver = {
 		   .name = "rt274",
@@ -1224,7 +1217,6 @@  static struct i2c_driver rt274_i2c_driver = {
 #endif
 		   },
 	.probe_new = rt274_i2c_probe,
-	.remove = rt274_i2c_remove,
 	.id_table = rt274_i2c_id,
 };