linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support
@ 2021-01-22 16:40 Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 01/13] mfd: arizona: Drop arizona-extcon cells Hans de Goede
                   ` (12 more replies)
  0 siblings, 13 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:40 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Hi all,

Here is v3 om my series to rework the arizona codec jack-detect support
to use the snd_soc_jack helpers instead of direct extcon reporting.

This is done by reworking the extcon driver into an arizona-jackdet
library and then modifying the codec drivers to use that directly,
replacing the old separate extcon child-devices and extcon-driver.

This brings the arizona-codec jack-detect handling inline with how
all other ASoC codec driver do this. This was developed and tested on
a Lenovo Yoga Tablet 1051L with a WM5102 codec.

There are various interdependencies between the patches in this
series, so IMHO it would be best if this entire series would be merged
through the MFD tree.

Note this series applies on top of my "[PATCH v4 0/5] MFD/ASoC: Add
support for Intel Bay Trail boards with WM5102 codec" series.

Changes in v3:
-Move the bugfix patches to earlier in the series so that they
 apply to drivers/extcon/extcon-arizona.c so that they can be
 cherry-picked into the stable series
-Split runtime_pm_get -> runtime_pm_get_sync changes out into their
 own patch
-Simply move drivers/extcon/extcon-arizona.c to
 sound/soc/codecs/arizona-jack.c instead of first adding arizona-jack.c
 as a copy and then later removing extcon-arizona.c
-Some other small tweaks, see individual patch changelogs

Regards,

Hans


Hans de Goede (13):
  mfd: arizona: Drop arizona-extcon cells
  extcon: arizona: Fix some issues when HPDET IRQ fires after the jack
    has been unplugged
  extcon: arizona: Fix various races on driver unbind
  extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol")
    call
  extcon: arizona: Always use pm_runtime_get_sync() when we need the
    device to be awake
  ASoC/extcon: arizona: Move arizona jack code to
    sound/soc/codecs/arizona-jack.c
  ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv
  ASoC: arizona-jack: Use arizona->dev for runtime-pm
  ASoC: arizona-jack: convert into a helper library for codec drivers
  ASoC: arizona-jack: Use snd_soc_jack to report jack events
  ASoC: arizona-jack: Cleanup logging
  ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use
    the new jack library
  ASoC: Intel: bytcr_wm5102: Add jack detect support

 MAINTAINERS                                   |   1 -
 drivers/extcon/Kconfig                        |   8 -
 drivers/extcon/Makefile                       |   1 -
 drivers/mfd/arizona-core.c                    |  20 -
 sound/soc/codecs/Makefile                     |   2 +-
 .../soc/codecs/arizona-jack.c                 | 539 +++++++-----------
 sound/soc/codecs/arizona.h                    |  44 ++
 sound/soc/codecs/wm5102.c                     |  12 +-
 sound/soc/codecs/wm5110.c                     |  12 +-
 sound/soc/codecs/wm8997.c                     |  14 +-
 sound/soc/codecs/wm8998.c                     |   9 +
 sound/soc/intel/boards/bytcr_wm5102.c         |  28 +-
 12 files changed, 307 insertions(+), 383 deletions(-)
 rename drivers/extcon/extcon-arizona.c => sound/soc/codecs/arizona-jack.c (77%)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 01/13] mfd: arizona: Drop arizona-extcon cells
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
@ 2021-01-22 16:40 ` Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged Hans de Goede
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:40 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

The arizona jack-dection handling is being reworked so that the
codec-child-device drivers directly handle jack-detect themselves,
so it is no longer necessary to instantiate "arizona-extcon"
child-devices.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/arizona-core.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 75f1bc671d59..ce6fe6de34f8 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -881,11 +881,6 @@ static const char * const wm5102_supplies[] = {
 static const struct mfd_cell wm5102_devs[] = {
 	{ .name = "arizona-micsupp" },
 	{ .name = "arizona-gpio" },
-	{
-		.name = "arizona-extcon",
-		.parent_supplies = wm5102_supplies,
-		.num_parent_supplies = 1, /* We only need MICVDD */
-	},
 	{ .name = "arizona-haptics" },
 	{ .name = "arizona-pwm" },
 	{
@@ -898,11 +893,6 @@ static const struct mfd_cell wm5102_devs[] = {
 static const struct mfd_cell wm5110_devs[] = {
 	{ .name = "arizona-micsupp" },
 	{ .name = "arizona-gpio" },
-	{
-		.name = "arizona-extcon",
-		.parent_supplies = wm5102_supplies,
-		.num_parent_supplies = 1, /* We only need MICVDD */
-	},
 	{ .name = "arizona-haptics" },
 	{ .name = "arizona-pwm" },
 	{
@@ -939,11 +929,6 @@ static const char * const wm8997_supplies[] = {
 static const struct mfd_cell wm8997_devs[] = {
 	{ .name = "arizona-micsupp" },
 	{ .name = "arizona-gpio" },
-	{
-		.name = "arizona-extcon",
-		.parent_supplies = wm8997_supplies,
-		.num_parent_supplies = 1, /* We only need MICVDD */
-	},
 	{ .name = "arizona-haptics" },
 	{ .name = "arizona-pwm" },
 	{
@@ -956,11 +941,6 @@ static const struct mfd_cell wm8997_devs[] = {
 static const struct mfd_cell wm8998_devs[] = {
 	{ .name = "arizona-micsupp" },
 	{ .name = "arizona-gpio" },
-	{
-		.name = "arizona-extcon",
-		.parent_supplies = wm5102_supplies,
-		.num_parent_supplies = 1, /* We only need MICVDD */
-	},
 	{ .name = "arizona-haptics" },
 	{ .name = "arizona-pwm" },
 	{
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 01/13] mfd: arizona: Drop arizona-extcon cells Hans de Goede
@ 2021-01-22 16:40 ` Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 03/13] extcon: arizona: Fix various races on driver unbind Hans de Goede
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:40 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

When the jack is partially inserted and then removed again it may be
removed while the hpdet code is running. In this case the following
may happen:

1. The "JACKDET rise" or ""JACKDET fall" IRQ triggers
2. arizona_jackdet runs and takes info->lock
3. The "HPDET" IRQ triggers
4. arizona_hpdet_irq runs, blocks on info->lock
5. arizona_jackdet calls arizona_stop_mic() and clears info->hpdet_done
6. arizona_jackdet releases info->lock
7. arizona_hpdet_irq now can continue running and:
7.1 Calls arizona_start_mic() (if a mic was detected)
7.2 sets info->hpdet_done

Step 7 is undesirable / a bug:
7.1 causes the device to stay in a high power-state (with MICVDD enabled)
7.2 causes hpdet to not run on the next jack insertion, which in turn
    causes the EXTCON_JACK_HEADPHONE state to never get set

This fixes both issues by skipping these 2 steps when arizona_hpdet_irq
runs after the jack has been unplugged.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-arizona.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index aae82db542a5..f7ef247de46a 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -601,7 +601,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	struct arizona *arizona = info->arizona;
 	int id_gpio = arizona->pdata.hpdet_id_gpio;
 	unsigned int report = EXTCON_JACK_HEADPHONE;
-	int ret, reading;
+	int ret, reading, state;
 	bool mic = false;
 
 	mutex_lock(&info->lock);
@@ -614,12 +614,11 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	}
 
 	/* If the cable was removed while measuring ignore the result */
-	ret = extcon_get_state(info->edev, EXTCON_MECHANICAL);
-	if (ret < 0) {
-		dev_err(arizona->dev, "Failed to check cable state: %d\n",
-			ret);
+	state = extcon_get_state(info->edev, EXTCON_MECHANICAL);
+	if (state < 0) {
+		dev_err(arizona->dev, "Failed to check cable state: %d\n", state);
 		goto out;
-	} else if (!ret) {
+	} else if (!state) {
 		dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n");
 		goto done;
 	}
@@ -667,7 +666,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 		gpio_set_value_cansleep(id_gpio, 0);
 
 	/* If we have a mic then reenable MICDET */
-	if (mic || info->mic)
+	if (state && (mic || info->mic))
 		arizona_start_mic(info);
 
 	if (info->hpdet_active) {
@@ -675,7 +674,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 		info->hpdet_active = false;
 	}
 
-	info->hpdet_done = true;
+	/* Do not set hp_det done when the cable has been unplugged */
+	if (state)
+		info->hpdet_done = true;
 
 out:
 	mutex_unlock(&info->lock);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 03/13] extcon: arizona: Fix various races on driver unbind
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 01/13] mfd: arizona: Drop arizona-extcon cells Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged Hans de Goede
@ 2021-01-22 16:40 ` Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call Hans de Goede
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:40 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

We must free/disable all interrupts and cancel all pending works
before doing further cleanup.

Before this commit arizona_extcon_remove() was doing several
register writes to shut things down before disabling the IRQs
and it was cancelling only 1 of the 3 different works used.

Move all the register-writes shutting things down to after
the disabling of the IRQs and add the 2 missing
cancel_delayed_work_sync() calls.

This fixes various possible races on driver unbind. One of which
would always trigger on devices using the mic-clamp feature for
jack detection. The ARIZONA_MICD_CLAMP_MODE_MASK update was
done before disabling the IRQs, causing:
1. arizona_jackdet() to run
2. detect a jack being inserted (clamp disabled means jack inserted)
3. call arizona_start_mic() which:
3.1 Enables the MICVDD regulator
3.2 takes a pm_runtime_reference

And this was all happening after the ARIZONA_MICD_ENA bit clearing,
which would undo 3.1 and 3.2 because the ARIZONA_MICD_CLAMP_MODE_MASK
update was being done after the ARIZONA_MICD_ENA bit clearing.

So this means that arizona_extcon_remove() would exit with
1. MICVDD enabled and 2. The pm_runtime_reference being unbalanced.

MICVDD still being enabled caused the following oops when the
regulator is released by the devm framework:

[ 2850.745757] ------------[ cut here ]------------
[ 2850.745827] WARNING: CPU: 2 PID: 2098 at drivers/regulator/core.c:2123 _regulator_put.part.0+0x19f/0x1b0
[ 2850.745835] Modules linked in: extcon_arizona ...
...
[ 2850.746909] Call Trace:
[ 2850.746932]  regulator_put+0x2d/0x40
[ 2850.746946]  release_nodes+0x22a/0x260
[ 2850.746984]  __device_release_driver+0x190/0x240
[ 2850.747002]  driver_detach+0xd4/0x120
...
[ 2850.747337] ---[ end trace f455dfd7abd9781f ]---

Note this oops is just one of various theoretically possible races caused
by the wrong ordering inside arizona_extcon_remove(), this fixes the
ordering fixing all possible races, including the reported oops.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-arizona.c | 40 +++++++++++++++++----------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index f7ef247de46a..76aacbac5869 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1760,25 +1760,6 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 	bool change;
 	int ret;
 
-	ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1,
-				       ARIZONA_MICD_ENA, 0,
-				       &change);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to disable micd on remove: %d\n",
-			ret);
-	} else if (change) {
-		regulator_disable(info->micvdd);
-		pm_runtime_put(info->dev);
-	}
-
-	gpiod_put(info->micd_pol_gpio);
-
-	pm_runtime_disable(&pdev->dev);
-
-	regmap_update_bits(arizona->regmap,
-			   ARIZONA_MICD_CLAMP_CONTROL,
-			   ARIZONA_MICD_CLAMP_MODE_MASK, 0);
-
 	if (info->micd_clamp) {
 		jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
 		jack_irq_fall = ARIZONA_IRQ_MICD_CLAMP_FALL;
@@ -1794,10 +1775,31 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 	arizona_free_irq(arizona, jack_irq_rise, info);
 	arizona_free_irq(arizona, jack_irq_fall, info);
 	cancel_delayed_work_sync(&info->hpdet_work);
+	cancel_delayed_work_sync(&info->micd_detect_work);
+	cancel_delayed_work_sync(&info->micd_timeout_work);
+
+	ret = regmap_update_bits_check(arizona->regmap, ARIZONA_MIC_DETECT_1,
+				       ARIZONA_MICD_ENA, 0,
+				       &change);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to disable micd on remove: %d\n",
+			ret);
+	} else if (change) {
+		regulator_disable(info->micvdd);
+		pm_runtime_put(info->dev);
+	}
+
+	regmap_update_bits(arizona->regmap,
+			   ARIZONA_MICD_CLAMP_CONTROL,
+			   ARIZONA_MICD_CLAMP_MODE_MASK, 0);
 	regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
 			   ARIZONA_JD1_ENA, 0);
 	arizona_clk32k_disable(arizona);
 
+	gpiod_put(info->micd_pol_gpio);
+
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (2 preceding siblings ...)
  2021-01-22 16:40 ` [PATCH v3 03/13] extcon: arizona: Fix various races on driver unbind Hans de Goede
@ 2021-01-22 16:40 ` Hans de Goede
  2021-01-22 16:40 ` [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake Hans de Goede
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:40 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

The initial value of the GPIO should match the info->micd_modes[0].gpio
value. arizona_extcon_probe() already stores the necessary flag in a
mode variable, but instead of passing mode as flags to the gpiod_get()
it was using a hardcoded GPIOD_OUT_LOW.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-arizona.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 76aacbac5869..72d23b15108c 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1510,7 +1510,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 		 */
 		info->micd_pol_gpio = gpiod_get_optional(arizona->dev,
 							 "wlf,micd-pol",
-							 GPIOD_OUT_LOW);
+							 mode);
 		if (IS_ERR(info->micd_pol_gpio)) {
 			ret = PTR_ERR(info->micd_pol_gpio);
 			dev_err(arizona->dev,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (3 preceding siblings ...)
  2021-01-22 16:40 ` [PATCH v3 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call Hans de Goede
@ 2021-01-22 16:40 ` Hans de Goede
  2021-01-22 20:38   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c Hans de Goede
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:40 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Before this commit the extcon-arizona code was mixing pm_runtime_get()
and pm_runtime_get_sync() in different places.

In all places where pm_runtime_get[_sync]() is called, the code
makes use of the device immediately after the call.
This means that we should always use pm_runtime_get_sync().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- This is a new patch in v3 of this patch-set
---
 drivers/extcon/extcon-arizona.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 72d23b15108c..56d2ce05de50 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -290,7 +290,7 @@ static void arizona_start_mic(struct arizona_extcon_info *info)
 	unsigned int mode;
 
 	/* Microphone detection can't use idle mode */
-	pm_runtime_get(info->dev);
+	pm_runtime_get_sync(info->dev);
 
 	if (info->detecting) {
 		ret = regulator_allow_bypass(info->micvdd, false);
@@ -695,7 +695,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
 	dev_dbg(arizona->dev, "Starting HPDET\n");
 
 	/* Make sure we keep the device enabled during the measurement */
-	pm_runtime_get(info->dev);
+	pm_runtime_get_sync(info->dev);
 
 	info->hpdet_active = true;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (4 preceding siblings ...)
  2021-01-22 16:40 ` [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 20:40   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv Hans de Goede
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

The jack handling for arizona codecs is being refactored so that it is
done directly by the codec drivers, instead of having an extcon-driver
bind to a separate "arizona-extcon" child-device for this.

drivers/mfd/arizona-core.c has already been updated to no longer
instantiate an "arizona-extcon" child-device for the arizona codecs.

This means that the "arizona-extcon" driver is no longer useful
(there are no longer any devices for it to bind to).

This commit drops the extcon Kconfig / Makefile bits and moves
drivers/extcon/extcon-arizona.c to sound/soc/codecs/arizona-jack.c .

This is a preparation patch for converting the arizona extcon-driver into
a helper library for letting the arizona codec-drivers directly report jack
state through the standard sound/soc/soc-jack.c functions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Fold the 2 separate patches to add a copy of extcon-arizona.c as
  sound/soc/codecs/arizona-jack.c and the follow up patch to remove the
  extcon driver into 1 patch simply moving the extcon driver code.
---
 MAINTAINERS                                               | 1 -
 drivers/extcon/Kconfig                                    | 8 --------
 drivers/extcon/Makefile                                   | 1 -
 .../extcon-arizona.c => sound/soc/codecs/arizona-jack.c   | 0
 4 files changed, 10 deletions(-)
 rename drivers/extcon/extcon-arizona.c => sound/soc/codecs/arizona-jack.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 036a03c794de..270661fcdb78 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19249,7 +19249,6 @@ F:	Documentation/devicetree/bindings/sound/wlf,arizona.yaml
 F:	Documentation/hwmon/wm83??.rst
 F:	arch/arm/mach-s3c/mach-crag6410*
 F:	drivers/clk/clk-wm83*.c
-F:	drivers/extcon/extcon-arizona.c
 F:	drivers/gpio/gpio-*wm*.c
 F:	drivers/gpio/gpio-arizona.c
 F:	drivers/hwmon/wm83??-hwmon.c
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index af58ebca2bf6..e3db936becfd 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -21,14 +21,6 @@ config EXTCON_ADC_JACK
 	help
 	  Say Y here to enable extcon device driver based on ADC values.
 
-config EXTCON_ARIZONA
-	tristate "Wolfson Arizona EXTCON support"
-	depends on MFD_ARIZONA && INPUT && SND_SOC
-	help
-	  Say Y here to enable support for external accessory detection
-	  with Wolfson Arizona devices. These are audio CODECs with
-	  advanced audio accessory detection support.
-
 config EXTCON_AXP288
 	tristate "X-Power AXP288 EXTCON support"
 	depends on MFD_AXP20X && USB_SUPPORT && X86 && ACPI
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index fe10a1b7d18b..1b390d934ca9 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -6,7 +6,6 @@
 obj-$(CONFIG_EXTCON)		+= extcon-core.o
 extcon-core-objs		+= extcon.o devres.o
 obj-$(CONFIG_EXTCON_ADC_JACK)	+= extcon-adc-jack.o
-obj-$(CONFIG_EXTCON_ARIZONA)	+= extcon-arizona.o
 obj-$(CONFIG_EXTCON_AXP288)	+= extcon-axp288.o
 obj-$(CONFIG_EXTCON_FSA9480)	+= extcon-fsa9480.o
 obj-$(CONFIG_EXTCON_GPIO)	+= extcon-gpio.o
diff --git a/drivers/extcon/extcon-arizona.c b/sound/soc/codecs/arizona-jack.c
similarity index 100%
rename from drivers/extcon/extcon-arizona.c
rename to sound/soc/codecs/arizona-jack.c
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (5 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 16:41 ` [PATCH v3 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm Hans de Goede
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Move all the jack-detect variables from struct arizona_extcon_info to
struct arizona_priv.

This is part of a patch series converting the arizona extcon driver into
a helper library for letting the arizona codec-drivers directly report jack
state through the standard sound/soc/soc-jack.c functions.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/arizona-jack.c | 97 ++++++++++-----------------------
 sound/soc/codecs/arizona.h      | 36 ++++++++++++
 2 files changed, 65 insertions(+), 68 deletions(-)

diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
index 56d2ce05de50..5b40316d0d75 100644
--- a/sound/soc/codecs/arizona-jack.c
+++ b/sound/soc/codecs/arizona-jack.c
@@ -27,6 +27,8 @@
 #include <linux/mfd/arizona/registers.h>
 #include <dt-bindings/mfd/arizona.h>
 
+#include "arizona.h"
+
 #define ARIZONA_MAX_MICD_RANGE 8
 
 #define ARIZONA_MICD_CLAMP_MODE_JDL      0x4
@@ -61,47 +63,6 @@
 
 #define MICD_LVL_0_TO_8 (MICD_LVL_0_TO_7 | ARIZONA_MICD_LVL_8)
 
-struct arizona_extcon_info {
-	struct device *dev;
-	struct arizona *arizona;
-	struct mutex lock;
-	struct regulator *micvdd;
-	struct input_dev *input;
-
-	u16 last_jackdet;
-
-	int micd_mode;
-	const struct arizona_micd_config *micd_modes;
-	int micd_num_modes;
-
-	const struct arizona_micd_range *micd_ranges;
-	int num_micd_ranges;
-
-	bool micd_reva;
-	bool micd_clamp;
-
-	struct delayed_work hpdet_work;
-	struct delayed_work micd_detect_work;
-	struct delayed_work micd_timeout_work;
-
-	bool hpdet_active;
-	bool hpdet_done;
-	bool hpdet_retried;
-
-	int num_hpdet_res;
-	unsigned int hpdet_res[3];
-
-	bool mic;
-	bool detecting;
-	int jack_flips;
-
-	int hpdet_ip_version;
-
-	struct extcon_dev *edev;
-
-	struct gpio_desc *micd_pol_gpio;
-};
-
 static const struct arizona_micd_config micd_default_modes[] = {
 	{ ARIZONA_ACCDET_SRC, 1, 0 },
 	{ 0,                  2, 1 },
@@ -135,9 +96,9 @@ static const unsigned int arizona_cable[] = {
 	EXTCON_NONE,
 };
 
-static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info);
+static void arizona_start_hpdet_acc_id(struct arizona_priv *info);
 
-static void arizona_extcon_hp_clamp(struct arizona_extcon_info *info,
+static void arizona_extcon_hp_clamp(struct arizona_priv *info,
 				    bool clamp)
 {
 	struct arizona *arizona = info->arizona;
@@ -222,7 +183,7 @@ static void arizona_extcon_hp_clamp(struct arizona_extcon_info *info,
 	snd_soc_dapm_mutex_unlock(arizona->dapm);
 }
 
-static void arizona_extcon_set_mode(struct arizona_extcon_info *info, int mode)
+static void arizona_extcon_set_mode(struct arizona_priv *info, int mode)
 {
 	struct arizona *arizona = info->arizona;
 
@@ -243,7 +204,7 @@ static void arizona_extcon_set_mode(struct arizona_extcon_info *info, int mode)
 	dev_dbg(arizona->dev, "Set jack polarity to %d\n", mode);
 }
 
-static const char *arizona_extcon_get_micbias(struct arizona_extcon_info *info)
+static const char *arizona_extcon_get_micbias(struct arizona_priv *info)
 {
 	switch (info->micd_modes[0].bias) {
 	case 1:
@@ -257,7 +218,7 @@ static const char *arizona_extcon_get_micbias(struct arizona_extcon_info *info)
 	}
 }
 
-static void arizona_extcon_pulse_micbias(struct arizona_extcon_info *info)
+static void arizona_extcon_pulse_micbias(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	const char *widget = arizona_extcon_get_micbias(info);
@@ -282,7 +243,7 @@ static void arizona_extcon_pulse_micbias(struct arizona_extcon_info *info)
 	}
 }
 
-static void arizona_start_mic(struct arizona_extcon_info *info)
+static void arizona_start_mic(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	bool change;
@@ -339,7 +300,7 @@ static void arizona_start_mic(struct arizona_extcon_info *info)
 	}
 }
 
-static void arizona_stop_mic(struct arizona_extcon_info *info)
+static void arizona_stop_mic(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	const char *widget = arizona_extcon_get_micbias(info);
@@ -407,7 +368,7 @@ static struct {
 	{ 1000, 10000 },
 };
 
-static int arizona_hpdet_read(struct arizona_extcon_info *info)
+static int arizona_hpdet_read(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	unsigned int val, range;
@@ -527,7 +488,7 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 	return val;
 }
 
-static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
+static int arizona_hpdet_do_id(struct arizona_priv *info, int *reading,
 			       bool *mic)
 {
 	struct arizona *arizona = info->arizona;
@@ -597,7 +558,7 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
 
 static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 {
-	struct arizona_extcon_info *info = data;
+	struct arizona_priv *info = data;
 	struct arizona *arizona = info->arizona;
 	int id_gpio = arizona->pdata.hpdet_id_gpio;
 	unsigned int report = EXTCON_JACK_HEADPHONE;
@@ -684,7 +645,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void arizona_identify_headphone(struct arizona_extcon_info *info)
+static void arizona_identify_headphone(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	int ret;
@@ -737,7 +698,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
 	info->hpdet_active = false;
 }
 
-static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info)
+static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	int hp_reading = 32;
@@ -790,8 +751,8 @@ static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info)
 
 static void arizona_micd_timeout_work(struct work_struct *work)
 {
-	struct arizona_extcon_info *info = container_of(work,
-						struct arizona_extcon_info,
+	struct arizona_priv *info = container_of(work,
+						struct arizona_priv,
 						micd_timeout_work.work);
 
 	mutex_lock(&info->lock);
@@ -805,7 +766,7 @@ static void arizona_micd_timeout_work(struct work_struct *work)
 	mutex_unlock(&info->lock);
 }
 
-static int arizona_micd_adc_read(struct arizona_extcon_info *info)
+static int arizona_micd_adc_read(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	unsigned int val;
@@ -842,7 +803,7 @@ static int arizona_micd_adc_read(struct arizona_extcon_info *info)
 	return val;
 }
 
-static int arizona_micd_read(struct arizona_extcon_info *info)
+static int arizona_micd_read(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	unsigned int val = 0;
@@ -875,7 +836,7 @@ static int arizona_micd_read(struct arizona_extcon_info *info)
 
 static int arizona_micdet_reading(void *priv)
 {
-	struct arizona_extcon_info *info = priv;
+	struct arizona_priv *info = priv;
 	struct arizona *arizona = info->arizona;
 	int ret, val;
 
@@ -969,7 +930,7 @@ static int arizona_micdet_reading(void *priv)
 
 static int arizona_button_reading(void *priv)
 {
-	struct arizona_extcon_info *info = priv;
+	struct arizona_priv *info = priv;
 	struct arizona *arizona = info->arizona;
 	int val, key, lvl, i;
 
@@ -1017,8 +978,8 @@ static int arizona_button_reading(void *priv)
 
 static void arizona_micd_detect(struct work_struct *work)
 {
-	struct arizona_extcon_info *info = container_of(work,
-						struct arizona_extcon_info,
+	struct arizona_priv *info = container_of(work,
+						struct arizona_priv,
 						micd_detect_work.work);
 	struct arizona *arizona = info->arizona;
 	int ret;
@@ -1051,7 +1012,7 @@ static void arizona_micd_detect(struct work_struct *work)
 
 static irqreturn_t arizona_micdet(int irq, void *data)
 {
-	struct arizona_extcon_info *info = data;
+	struct arizona_priv *info = data;
 	struct arizona *arizona = info->arizona;
 	int debounce = arizona->pdata.micd_detect_debounce;
 
@@ -1075,8 +1036,8 @@ static irqreturn_t arizona_micdet(int irq, void *data)
 
 static void arizona_hpdet_work(struct work_struct *work)
 {
-	struct arizona_extcon_info *info = container_of(work,
-						struct arizona_extcon_info,
+	struct arizona_priv *info = container_of(work,
+						struct arizona_priv,
 						hpdet_work.work);
 
 	mutex_lock(&info->lock);
@@ -1084,7 +1045,7 @@ static void arizona_hpdet_work(struct work_struct *work)
 	mutex_unlock(&info->lock);
 }
 
-static int arizona_hpdet_wait(struct arizona_extcon_info *info)
+static int arizona_hpdet_wait(struct arizona_priv *info)
 {
 	struct arizona *arizona = info->arizona;
 	unsigned int val;
@@ -1120,7 +1081,7 @@ static int arizona_hpdet_wait(struct arizona_extcon_info *info)
 
 static irqreturn_t arizona_jackdet(int irq, void *data)
 {
-	struct arizona_extcon_info *info = data;
+	struct arizona_priv *info = data;
 	struct arizona *arizona = info->arizona;
 	unsigned int val, present, mask;
 	bool cancelled_hp, cancelled_mic;
@@ -1380,7 +1341,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 {
 	struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
 	struct arizona_pdata *pdata = &arizona->pdata;
-	struct arizona_extcon_info *info;
+	struct arizona_priv *info;
 	unsigned int val;
 	unsigned int clamp_mode;
 	int jack_irq_fall, jack_irq_rise;
@@ -1754,7 +1715,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 
 static int arizona_extcon_remove(struct platform_device *pdev)
 {
-	struct arizona_extcon_info *info = platform_get_drvdata(pdev);
+	struct arizona_priv *info = platform_get_drvdata(pdev);
 	struct arizona *arizona = info->arizona;
 	int jack_irq_rise, jack_irq_fall;
 	bool change;
diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
index b893d3e4c97c..d1a263a67bba 100644
--- a/sound/soc/codecs/arizona.h
+++ b/sound/soc/codecs/arizona.h
@@ -91,6 +91,42 @@ struct arizona_priv {
 	unsigned int dvfs_reqs;
 	struct mutex dvfs_lock;
 	bool dvfs_cached;
+
+	/* Variables used by arizona-jack.c code */
+	struct device *dev;
+	struct mutex lock;
+	struct delayed_work hpdet_work;
+	struct delayed_work micd_detect_work;
+	struct delayed_work micd_timeout_work;
+	struct regulator *micvdd;
+	struct input_dev *input;
+	struct extcon_dev *edev;
+	struct gpio_desc *micd_pol_gpio;
+
+	u16 last_jackdet;
+
+	int micd_mode;
+	const struct arizona_micd_config *micd_modes;
+	int micd_num_modes;
+
+	const struct arizona_micd_range *micd_ranges;
+	int num_micd_ranges;
+
+	bool micd_reva;
+	bool micd_clamp;
+
+	bool hpdet_active;
+	bool hpdet_done;
+	bool hpdet_retried;
+
+	bool mic;
+	bool detecting;
+
+	int num_hpdet_res;
+	unsigned int hpdet_res[3];
+
+	int jack_flips;
+	int hpdet_ip_version;
 };
 
 struct arizona_voice_trigger_info {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (6 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 20:43   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers Hans de Goede
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Drivers for MFD child-devices such as the arizona codec drivers
and the arizona-extcon driver can choose to either make
runtime_pm_get/_put calls on their own child-device, which will
then be propagated to their parent; or they can make them directly
on their MFD parent-device.

The arizona-extcon code was using runtime_pm_get/_put calls on
its own child-device where as the codec drivers are using
runtime_pm_get/_put calls on their parent.

The arizona-extcon MFD cell/child-device has been removed and this
commit is part of refactoring the arizona-extcon code into a library
to be used directly from the codec drivers.

Specifically this commit moves the code over to make
runtime_pm_get/_put calls on the parent device (on arizona->dev)
bringing the code inline with how the codec drivers do this.

Note this also removes the pm_runtime_enable/_disable calls
as pm_runtime support has already been enabled on the parent-device
by the arizona MFD driver.

This is part of a patch series converting the arizona extcon driver into
a helper library for letting the arizona codec-drivers directly report
jack state through the standard sound/soc/soc-jack.c functions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/arizona-jack.c | 42 ++++++++++++++-------------------
 sound/soc/codecs/arizona.h      |  1 -
 2 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
index 5b40316d0d75..a6e8071f84ab 100644
--- a/sound/soc/codecs/arizona-jack.c
+++ b/sound/soc/codecs/arizona-jack.c
@@ -251,7 +251,7 @@ static void arizona_start_mic(struct arizona_priv *info)
 	unsigned int mode;
 
 	/* Microphone detection can't use idle mode */
-	pm_runtime_get_sync(info->dev);
+	pm_runtime_get_sync(arizona->dev);
 
 	if (info->detecting) {
 		ret = regulator_allow_bypass(info->micvdd, false);
@@ -296,7 +296,7 @@ static void arizona_start_mic(struct arizona_priv *info)
 		dev_err(arizona->dev, "Failed to enable micd: %d\n", ret);
 	} else if (!change) {
 		regulator_disable(info->micvdd);
-		pm_runtime_put_autosuspend(info->dev);
+		pm_runtime_put_autosuspend(arizona->dev);
 	}
 }
 
@@ -341,8 +341,8 @@ static void arizona_stop_mic(struct arizona_priv *info)
 
 	if (change) {
 		regulator_disable(info->micvdd);
-		pm_runtime_mark_last_busy(info->dev);
-		pm_runtime_put_autosuspend(info->dev);
+		pm_runtime_mark_last_busy(arizona->dev);
+		pm_runtime_put_autosuspend(arizona->dev);
 	}
 }
 
@@ -534,7 +534,7 @@ static int arizona_hpdet_do_id(struct arizona_priv *info, int *reading,
 		info->num_hpdet_res = 0;
 		info->hpdet_retried = true;
 		arizona_start_hpdet_acc_id(info);
-		pm_runtime_put(info->dev);
+		pm_runtime_put(arizona->dev);
 		return -EAGAIN;
 	}
 
@@ -631,7 +631,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 		arizona_start_mic(info);
 
 	if (info->hpdet_active) {
-		pm_runtime_put_autosuspend(info->dev);
+		pm_runtime_put_autosuspend(arizona->dev);
 		info->hpdet_active = false;
 	}
 
@@ -656,7 +656,7 @@ static void arizona_identify_headphone(struct arizona_priv *info)
 	dev_dbg(arizona->dev, "Starting HPDET\n");
 
 	/* Make sure we keep the device enabled during the measurement */
-	pm_runtime_get_sync(info->dev);
+	pm_runtime_get_sync(arizona->dev);
 
 	info->hpdet_active = true;
 
@@ -685,7 +685,7 @@ static void arizona_identify_headphone(struct arizona_priv *info)
 
 err:
 	arizona_extcon_hp_clamp(info, false);
-	pm_runtime_put_autosuspend(info->dev);
+	pm_runtime_put_autosuspend(arizona->dev);
 
 	/* Just report headphone */
 	ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
@@ -708,7 +708,7 @@ static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
 	dev_dbg(arizona->dev, "Starting identification via HPDET\n");
 
 	/* Make sure we keep the device enabled during the measurement */
-	pm_runtime_get_sync(info->dev);
+	pm_runtime_get_sync(arizona->dev);
 
 	info->hpdet_active = true;
 
@@ -1006,7 +1006,7 @@ static void arizona_micd_detect(struct work_struct *work)
 	else
 		arizona_button_reading(info);
 
-	pm_runtime_mark_last_busy(info->dev);
+	pm_runtime_mark_last_busy(arizona->dev);
 	mutex_unlock(&info->lock);
 }
 
@@ -1090,7 +1090,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 	cancelled_hp = cancel_delayed_work_sync(&info->hpdet_work);
 	cancelled_mic = cancel_delayed_work_sync(&info->micd_timeout_work);
 
-	pm_runtime_get_sync(info->dev);
+	pm_runtime_get_sync(arizona->dev);
 
 	mutex_lock(&info->lock);
 
@@ -1110,7 +1110,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 		dev_err(arizona->dev, "Failed to read jackdet status: %d\n",
 			ret);
 		mutex_unlock(&info->lock);
-		pm_runtime_put_autosuspend(info->dev);
+		pm_runtime_put_autosuspend(arizona->dev);
 		return IRQ_NONE;
 	}
 
@@ -1210,8 +1210,8 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 
 	mutex_unlock(&info->lock);
 
-	pm_runtime_mark_last_busy(info->dev);
-	pm_runtime_put_autosuspend(info->dev);
+	pm_runtime_mark_last_busy(arizona->dev);
+	pm_runtime_put_autosuspend(arizona->dev);
 
 	return IRQ_HANDLED;
 }
@@ -1366,7 +1366,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 
 	mutex_init(&info->lock);
 	info->arizona = arizona;
-	info->dev = &pdev->dev;
 	info->last_jackdet = ~(ARIZONA_MICD_CLAMP_STS | ARIZONA_JD1_STS);
 	INIT_DELAYED_WORK(&info->hpdet_work, arizona_hpdet_work);
 	INIT_DELAYED_WORK(&info->micd_detect_work, arizona_micd_detect);
@@ -1617,9 +1616,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 
 	arizona_extcon_set_mode(info, 0);
 
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_idle(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
+	pm_runtime_get_sync(arizona->dev);
 
 	if (info->micd_clamp) {
 		jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
@@ -1689,7 +1686,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 		goto err_hpdet;
 	}
 
-	pm_runtime_put(&pdev->dev);
+	pm_runtime_put(arizona->dev);
 
 	return 0;
 
@@ -1706,8 +1703,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 err_rise:
 	arizona_free_irq(arizona, jack_irq_rise, info);
 err_pm:
-	pm_runtime_put(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put(arizona->dev);
 err_gpio:
 	gpiod_put(info->micd_pol_gpio);
 	return ret;
@@ -1747,7 +1743,7 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 			ret);
 	} else if (change) {
 		regulator_disable(info->micvdd);
-		pm_runtime_put(info->dev);
+		pm_runtime_put(arizona->dev);
 	}
 
 	regmap_update_bits(arizona->regmap,
@@ -1759,8 +1755,6 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 
 	gpiod_put(info->micd_pol_gpio);
 
-	pm_runtime_disable(&pdev->dev);
-
 	return 0;
 }
 
diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
index d1a263a67bba..7132dbc3c840 100644
--- a/sound/soc/codecs/arizona.h
+++ b/sound/soc/codecs/arizona.h
@@ -93,7 +93,6 @@ struct arizona_priv {
 	bool dvfs_cached;
 
 	/* Variables used by arizona-jack.c code */
-	struct device *dev;
 	struct mutex lock;
 	struct delayed_work hpdet_work;
 	struct delayed_work micd_detect_work;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (7 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 20:47   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Hans de Goede
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Convert the arizona extcon driver into a helper library for direct use
from the arizona codec-drivers, rather then being bound to a separate
MFD cell.

Note the probe (and remove) sequence is split into 2 parts:

1. The arizona_jack_codec_dev_probe() function inits a bunch of
jack-detect specific variables in struct arizona_priv and tries to get
a number of resources where getting them may fail with -EPROBE_DEFER.

2. Then once the machine driver has create a snd_sock_jack through
snd_soc_card_jack_new() it calls snd_soc_component_set_jack() on
the codec component, which will call the new arizona_jack_set_jack(),
which sets up jack-detection and requests the IRQs.

This split is necessary, because the IRQ handlers need access to the
arizona->dapm pointer and the snd_sock_jack which are not available
when the codec-driver's probe function runs.

Note this requires that machine-drivers for codecs which are converted
to use the new helper functions from arizona-jack.c are modified to
create a snd_soc_jack through snd_soc_card_jack_new() and register
this jack with the codec through snd_soc_component_set_jack().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- Pass dev (the codec-device) to devm_regulator_get instead of
  passing arizona->dev. This is necessary so that the regulator gets
  properly released when the coded driver unbinds.
---
 sound/soc/codecs/Makefile       |   2 +-
 sound/soc/codecs/arizona-jack.c | 125 +++++++++++++++-----------------
 sound/soc/codecs/arizona.h      |   6 ++
 3 files changed, 65 insertions(+), 68 deletions(-)

diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index d277f0366e09..2e976cfaa862 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -43,7 +43,7 @@ snd-soc-ak4642-objs := ak4642.o
 snd-soc-ak4671-objs := ak4671.o
 snd-soc-ak5386-objs := ak5386.o
 snd-soc-ak5558-objs := ak5558.o
-snd-soc-arizona-objs := arizona.o
+snd-soc-arizona-objs := arizona.o arizona-jack.o
 snd-soc-bd28623-objs := bd28623.o
 snd-soc-bt-sco-objs := bt-sco.o
 snd-soc-cpcap-objs := cpcap.o
diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
index a6e8071f84ab..e121490eb379 100644
--- a/sound/soc/codecs/arizona-jack.c
+++ b/sound/soc/codecs/arizona-jack.c
@@ -7,14 +7,12 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
-#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
@@ -1337,27 +1335,16 @@ static int arizona_extcon_device_get_pdata(struct device *dev,
 	return 0;
 }
 
-static int arizona_extcon_probe(struct platform_device *pdev)
+int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
 {
-	struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
+	struct arizona *arizona = info->arizona;
 	struct arizona_pdata *pdata = &arizona->pdata;
-	struct arizona_priv *info;
-	unsigned int val;
-	unsigned int clamp_mode;
-	int jack_irq_fall, jack_irq_rise;
-	int ret, mode, i, j;
-
-	if (!arizona->dapm || !arizona->dapm->card)
-		return -EPROBE_DEFER;
-
-	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
+	int ret, mode;
 
 	if (!dev_get_platdata(arizona->dev))
-		arizona_extcon_device_get_pdata(&pdev->dev, arizona);
+		arizona_extcon_device_get_pdata(dev, arizona);
 
-	info->micvdd = devm_regulator_get(&pdev->dev, "MICVDD");
+	info->micvdd = devm_regulator_get(dev, "MICVDD");
 	if (IS_ERR(info->micvdd)) {
 		ret = PTR_ERR(info->micvdd);
 		dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
@@ -1365,12 +1352,10 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	}
 
 	mutex_init(&info->lock);
-	info->arizona = arizona;
 	info->last_jackdet = ~(ARIZONA_MICD_CLAMP_STS | ARIZONA_JD1_STS);
 	INIT_DELAYED_WORK(&info->hpdet_work, arizona_hpdet_work);
 	INIT_DELAYED_WORK(&info->micd_detect_work, arizona_micd_detect);
 	INIT_DELAYED_WORK(&info->micd_timeout_work, arizona_micd_timeout_work);
-	platform_set_drvdata(pdev, info);
 
 	switch (arizona->type) {
 	case WM5102:
@@ -1404,20 +1389,20 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 		break;
 	}
 
-	info->edev = devm_extcon_dev_allocate(&pdev->dev, arizona_cable);
+	info->edev = devm_extcon_dev_allocate(dev, arizona_cable);
 	if (IS_ERR(info->edev)) {
-		dev_err(&pdev->dev, "failed to allocate extcon device\n");
+		dev_err(arizona->dev, "failed to allocate extcon device\n");
 		return -ENOMEM;
 	}
 
-	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
+	ret = devm_extcon_dev_register(dev, info->edev);
 	if (ret < 0) {
 		dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
 			ret);
 		return ret;
 	}
 
-	info->input = devm_input_allocate_device(&pdev->dev);
+	info->input = devm_input_allocate_device(dev);
 	if (!info->input) {
 		dev_err(arizona->dev, "Can't allocate input dev\n");
 		ret = -ENOMEM;
@@ -1448,7 +1433,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 		else
 			mode = GPIOF_OUT_INIT_LOW;
 
-		ret = devm_gpio_request_one(&pdev->dev, pdata->micd_pol_gpio,
+		ret = devm_gpio_request_one(dev, pdata->micd_pol_gpio,
 					    mode, "MICD polarity");
 		if (ret != 0) {
 			dev_err(arizona->dev, "Failed to request GPIO%d: %d\n",
@@ -1481,17 +1466,38 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	}
 
 	if (arizona->pdata.hpdet_id_gpio > 0) {
-		ret = devm_gpio_request_one(&pdev->dev,
-					    arizona->pdata.hpdet_id_gpio,
+		ret = devm_gpio_request_one(dev, arizona->pdata.hpdet_id_gpio,
 					    GPIOF_OUT_INIT_LOW,
 					    "HPDET");
 		if (ret != 0) {
 			dev_err(arizona->dev, "Failed to request GPIO%d: %d\n",
 				arizona->pdata.hpdet_id_gpio, ret);
-			goto err_gpio;
+			gpiod_put(info->micd_pol_gpio);
+			return ret;
 		}
 	}
 
+	return 0;
+}
+EXPORT_SYMBOL_GPL(arizona_jack_codec_dev_probe);
+
+int arizona_jack_codec_dev_remove(struct arizona_priv *info)
+{
+	gpiod_put(info->micd_pol_gpio);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(arizona_jack_codec_dev_remove);
+
+static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
+					   struct snd_soc_jack *jack)
+{
+	struct arizona *arizona = info->arizona;
+	struct arizona_pdata *pdata = &arizona->pdata;
+	unsigned int val;
+	unsigned int clamp_mode;
+	int jack_irq_fall, jack_irq_rise;
+	int ret, i, j;
+
 	if (arizona->pdata.micd_bias_start_time)
 		regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_1,
 				   ARIZONA_MICD_BIAS_STARTTIME_MASK,
@@ -1532,16 +1538,15 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_RANGE) {
 		dev_err(arizona->dev, "Too many MICD ranges: %d\n",
 			arizona->pdata.num_micd_ranges);
+		return -EINVAL;
 	}
 
 	if (info->num_micd_ranges > 1) {
 		for (i = 1; i < info->num_micd_ranges; i++) {
 			if (info->micd_ranges[i - 1].max >
 			    info->micd_ranges[i].max) {
-				dev_err(arizona->dev,
-					"MICD ranges must be sorted\n");
-				ret = -EINVAL;
-				goto err_gpio;
+				dev_err(arizona->dev, "MICD ranges must be sorted\n");
+				return -EINVAL;
 			}
 		}
 	}
@@ -1559,8 +1564,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 		if (j == ARIZONA_NUM_MICD_BUTTON_LEVELS) {
 			dev_err(arizona->dev, "Unsupported MICD level %d\n",
 				info->micd_ranges[i].max);
-			ret = -EINVAL;
-			goto err_gpio;
+			return -EINVAL;
 		}
 
 		dev_dbg(arizona->dev, "%d ohms for MICD threshold %d\n",
@@ -1629,43 +1633,40 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	ret = arizona_request_irq(arizona, jack_irq_rise,
 				  "JACKDET rise", arizona_jackdet, info);
 	if (ret != 0) {
-		dev_err(&pdev->dev, "Failed to get JACKDET rise IRQ: %d\n",
-			ret);
+		dev_err(arizona->dev, "Failed to get JACKDET rise IRQ: %d\n", ret);
 		goto err_pm;
 	}
 
 	ret = arizona_set_irq_wake(arizona, jack_irq_rise, 1);
 	if (ret != 0) {
-		dev_err(&pdev->dev, "Failed to set JD rise IRQ wake: %d\n",
-			ret);
+		dev_err(arizona->dev, "Failed to set JD rise IRQ wake: %d\n", ret);
 		goto err_rise;
 	}
 
 	ret = arizona_request_irq(arizona, jack_irq_fall,
 				  "JACKDET fall", arizona_jackdet, info);
 	if (ret != 0) {
-		dev_err(&pdev->dev, "Failed to get JD fall IRQ: %d\n", ret);
+		dev_err(arizona->dev, "Failed to get JD fall IRQ: %d\n", ret);
 		goto err_rise_wake;
 	}
 
 	ret = arizona_set_irq_wake(arizona, jack_irq_fall, 1);
 	if (ret != 0) {
-		dev_err(&pdev->dev, "Failed to set JD fall IRQ wake: %d\n",
-			ret);
+		dev_err(arizona->dev, "Failed to set JD fall IRQ wake: %d\n", ret);
 		goto err_fall;
 	}
 
 	ret = arizona_request_irq(arizona, ARIZONA_IRQ_MICDET,
 				  "MICDET", arizona_micdet, info);
 	if (ret != 0) {
-		dev_err(&pdev->dev, "Failed to get MICDET IRQ: %d\n", ret);
+		dev_err(arizona->dev, "Failed to get MICDET IRQ: %d\n", ret);
 		goto err_fall_wake;
 	}
 
 	ret = arizona_request_irq(arizona, ARIZONA_IRQ_HPDET,
 				  "HPDET", arizona_hpdet_irq, info);
 	if (ret != 0) {
-		dev_err(&pdev->dev, "Failed to get HPDET IRQ: %d\n", ret);
+		dev_err(arizona->dev, "Failed to get HPDET IRQ: %d\n", ret);
 		goto err_micdet;
 	}
 
@@ -1677,12 +1678,11 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 
 	ret = regulator_allow_bypass(info->micvdd, true);
 	if (ret != 0)
-		dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n",
-			 ret);
+		dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n", ret);
 
 	ret = input_register_device(info->input);
 	if (ret) {
-		dev_err(&pdev->dev, "Can't register input device: %d\n", ret);
+		dev_err(arizona->dev, "Can't register input device: %d\n", ret);
 		goto err_hpdet;
 	}
 
@@ -1704,14 +1704,11 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 	arizona_free_irq(arizona, jack_irq_rise, info);
 err_pm:
 	pm_runtime_put(arizona->dev);
-err_gpio:
-	gpiod_put(info->micd_pol_gpio);
 	return ret;
 }
 
-static int arizona_extcon_remove(struct platform_device *pdev)
+static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
 {
-	struct arizona_priv *info = platform_get_drvdata(pdev);
 	struct arizona *arizona = info->arizona;
 	int jack_irq_rise, jack_irq_fall;
 	bool change;
@@ -1739,8 +1736,7 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 				       ARIZONA_MICD_ENA, 0,
 				       &change);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to disable micd on remove: %d\n",
-			ret);
+		dev_err(arizona->dev, "Failed to disable micd on remove: %d\n", ret);
 	} else if (change) {
 		regulator_disable(info->micvdd);
 		pm_runtime_put(arizona->dev);
@@ -1753,22 +1749,17 @@ static int arizona_extcon_remove(struct platform_device *pdev)
 			   ARIZONA_JD1_ENA, 0);
 	arizona_clk32k_disable(arizona);
 
-	gpiod_put(info->micd_pol_gpio);
-
 	return 0;
 }
 
-static struct platform_driver arizona_extcon_driver = {
-	.driver		= {
-		.name	= "arizona-extcon",
-	},
-	.probe		= arizona_extcon_probe,
-	.remove		= arizona_extcon_remove,
-};
-
-module_platform_driver(arizona_extcon_driver);
+int arizona_jack_set_jack(struct snd_soc_component *component,
+			  struct snd_soc_jack *jack, void *data)
+{
+	struct arizona_priv *info = snd_soc_component_get_drvdata(component);
 
-MODULE_DESCRIPTION("Arizona Extcon driver");
-MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:extcon-arizona");
+	if (jack)
+		return arizona_jack_enable_jack_detect(info, jack);
+	else
+		return arizona_jack_disable_jack_detect(info);
+}
+EXPORT_SYMBOL_GPL(arizona_jack_set_jack);
diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
index 7132dbc3c840..fc515845a3e6 100644
--- a/sound/soc/codecs/arizona.h
+++ b/sound/soc/codecs/arizona.h
@@ -386,4 +386,10 @@ static inline int arizona_unregister_notifier(struct snd_soc_component *componen
 
 int arizona_of_get_audio_pdata(struct arizona *arizona);
 
+int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev);
+int arizona_jack_codec_dev_remove(struct arizona_priv *info);
+
+int arizona_jack_set_jack(struct snd_soc_component *component,
+			  struct snd_soc_jack *jack, void *data);
+
 #endif
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (8 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 20:50   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging Hans de Goede
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Use the snd_soc_jack code to report jack events, instead of using extcon
for reporting the cable-type + an input_dev for reporting the button
presses.

The snd_soc_jack code will report the cable-type through both input_dev
events and through ALSA controls and the button-presses through input_dev
events.

Note that this means that when the codec drivers are moved over to use
the new arizona-jack.c library code instead of having a separate MFD
extcon cell with the extcon-arizona.c driver, we will no longer report
extcon events to userspace for cable-type changes. This should not be
a problem since "standard" Linux distro userspace does not (and has
never) used the extcon class interface for this. Android does have
support for the extcon class interface, but that was introduced in
the same release as support for input_dev cable-type events, so this
should not be a problem for Android either.

Note this also reduces ARIZONA_MAX_MICD_RANGE from 8 to 6, this is
ok to do since this info is always provided through pdata (or defaults)
and cannot be overridden from devicetree. All in kernel users of the
pdata (and the fallback defaults) define 6 or less buttons/ranges.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/arizona-jack.c | 149 +++++++++-----------------------
 sound/soc/codecs/arizona.h      |   7 +-
 2 files changed, 47 insertions(+), 109 deletions(-)

diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
index e121490eb379..268d2a44d891 100644
--- a/sound/soc/codecs/arizona-jack.c
+++ b/sound/soc/codecs/arizona-jack.c
@@ -16,8 +16,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
-#include <linux/extcon-provider.h>
 
+#include <sound/jack.h>
 #include <sound/soc.h>
 
 #include <linux/mfd/arizona/core.h>
@@ -29,6 +29,12 @@
 
 #define ARIZONA_MAX_MICD_RANGE 8
 
+/*
+ * The hardware supports 8 ranges / buttons, but the snd-jack interface
+ * only supports 6 buttons (button 0-5).
+ */
+#define ARIZONA_MAX_MICD_BUTTONS 6
+
 #define ARIZONA_MICD_CLAMP_MODE_JDL      0x4
 #define ARIZONA_MICD_CLAMP_MODE_JDH      0x5
 #define ARIZONA_MICD_CLAMP_MODE_JDL_GP5H 0x9
@@ -86,14 +92,6 @@ static const int arizona_micd_levels[] = {
 	1257, 30000,
 };
 
-static const unsigned int arizona_cable[] = {
-	EXTCON_MECHANICAL,
-	EXTCON_JACK_MICROPHONE,
-	EXTCON_JACK_HEADPHONE,
-	EXTCON_JACK_LINE_OUT,
-	EXTCON_NONE,
-};
-
 static void arizona_start_hpdet_acc_id(struct arizona_priv *info);
 
 static void arizona_extcon_hp_clamp(struct arizona_priv *info,
@@ -559,8 +557,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	struct arizona_priv *info = data;
 	struct arizona *arizona = info->arizona;
 	int id_gpio = arizona->pdata.hpdet_id_gpio;
-	unsigned int report = EXTCON_JACK_HEADPHONE;
-	int ret, reading, state;
+	int ret, reading, state, report;
 	bool mic = false;
 
 	mutex_lock(&info->lock);
@@ -573,11 +570,8 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 	}
 
 	/* If the cable was removed while measuring ignore the result */
-	state = extcon_get_state(info->edev, EXTCON_MECHANICAL);
-	if (state < 0) {
-		dev_err(arizona->dev, "Failed to check cable state: %d\n", state);
-		goto out;
-	} else if (!state) {
+	state = info->jack->status & SND_JACK_MECHANICAL;
+	if (!state) {
 		dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n");
 		goto done;
 	}
@@ -603,14 +597,11 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 
 	/* Report high impedence cables as line outputs */
 	if (reading >= 5000)
-		report = EXTCON_JACK_LINE_OUT;
+		report = SND_JACK_LINEOUT;
 	else
-		report = EXTCON_JACK_HEADPHONE;
+		report = SND_JACK_HEADPHONE;
 
-	ret = extcon_set_state_sync(info->edev, report, true);
-	if (ret != 0)
-		dev_err(arizona->dev, "Failed to report HP/line: %d\n",
-			ret);
+	snd_soc_jack_report(info->jack, report, SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
 
 done:
 	/* Reset back to starting range */
@@ -686,9 +677,8 @@ static void arizona_identify_headphone(struct arizona_priv *info)
 	pm_runtime_put_autosuspend(arizona->dev);
 
 	/* Just report headphone */
-	ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
-	if (ret != 0)
-		dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
+	snd_soc_jack_report(info->jack, SND_JACK_HEADPHONE,
+			    SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
 
 	if (info->mic)
 		arizona_start_mic(info);
@@ -740,9 +730,8 @@ static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
 
 err:
 	/* Just report headphone */
-	ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
-	if (ret != 0)
-		dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
+	snd_soc_jack_report(info->jack, SND_JACK_HEADPHONE,
+			    SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
 
 	info->hpdet_active = false;
 }
@@ -863,11 +852,7 @@ static int arizona_micdet_reading(void *priv)
 
 		arizona_identify_headphone(info);
 
-		ret = extcon_set_state_sync(info->edev,
-					      EXTCON_JACK_MICROPHONE, true);
-		if (ret != 0)
-			dev_err(arizona->dev, "Headset report failed: %d\n",
-				ret);
+		snd_soc_jack_report(info->jack, SND_JACK_MICROPHONE, SND_JACK_MICROPHONE);
 
 		/* Don't need to regulate for button detection */
 		ret = regulator_allow_bypass(info->micvdd, true);
@@ -930,7 +915,7 @@ static int arizona_button_reading(void *priv)
 {
 	struct arizona_priv *info = priv;
 	struct arizona *arizona = info->arizona;
-	int val, key, lvl, i;
+	int val, key, lvl;
 
 	val = arizona_micd_read(info);
 	if (val < 0)
@@ -947,14 +932,11 @@ static int arizona_button_reading(void *priv)
 			lvl = val & ARIZONA_MICD_LVL_MASK;
 			lvl >>= ARIZONA_MICD_LVL_SHIFT;
 
-			for (i = 0; i < info->num_micd_ranges; i++)
-				input_report_key(info->input,
-						 info->micd_ranges[i].key, 0);
-
 			if (lvl && ffs(lvl) - 1 < info->num_micd_ranges) {
-				key = info->micd_ranges[ffs(lvl) - 1].key;
-				input_report_key(info->input, key, 1);
-				input_sync(info->input);
+				key = ffs(lvl) - 1;
+				snd_soc_jack_report(info->jack,
+						    SND_JACK_BTN_0 >> key,
+						    info->micd_button_mask);
 			} else {
 				dev_err(arizona->dev, "Button out of range\n");
 			}
@@ -964,10 +946,7 @@ static int arizona_button_reading(void *priv)
 		}
 	} else {
 		dev_dbg(arizona->dev, "Mic button released\n");
-		for (i = 0; i < info->num_micd_ranges; i++)
-			input_report_key(info->input,
-					 info->micd_ranges[i].key, 0);
-		input_sync(info->input);
+		snd_soc_jack_report(info->jack, 0, info->micd_button_mask);
 		arizona_extcon_pulse_micbias(info);
 	}
 
@@ -980,20 +959,13 @@ static void arizona_micd_detect(struct work_struct *work)
 						struct arizona_priv,
 						micd_detect_work.work);
 	struct arizona *arizona = info->arizona;
-	int ret;
 
 	cancel_delayed_work_sync(&info->micd_timeout_work);
 
 	mutex_lock(&info->lock);
 
 	/* If the cable was removed while measuring ignore the result */
-	ret = extcon_get_state(info->edev, EXTCON_MECHANICAL);
-	if (ret < 0) {
-		dev_err(arizona->dev, "Failed to check cable state: %d\n",
-				ret);
-		mutex_unlock(&info->lock);
-		return;
-	} else if (!ret) {
+	if (!(info->jack->status & SND_JACK_MECHANICAL)) {
 		dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n");
 		mutex_unlock(&info->lock);
 		return;
@@ -1134,12 +1106,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 
 	if (info->last_jackdet == present) {
 		dev_dbg(arizona->dev, "Detected jack\n");
-		ret = extcon_set_state_sync(info->edev,
-					      EXTCON_MECHANICAL, true);
-
-		if (ret != 0)
-			dev_err(arizona->dev, "Mechanical report failed: %d\n",
-				ret);
+		snd_soc_jack_report(info->jack, SND_JACK_MECHANICAL, SND_JACK_MECHANICAL);
 
 		info->detecting = true;
 		info->mic = false;
@@ -1170,18 +1137,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 		info->hpdet_done = false;
 		info->hpdet_retried = false;
 
-		for (i = 0; i < info->num_micd_ranges; i++)
-			input_report_key(info->input,
-					 info->micd_ranges[i].key, 0);
-		input_sync(info->input);
-
-		for (i = 0; i < ARRAY_SIZE(arizona_cable) - 1; i++) {
-			ret = extcon_set_state_sync(info->edev,
-					arizona_cable[i], false);
-			if (ret != 0)
-				dev_err(arizona->dev,
-					"Removal report failed: %d\n", ret);
-		}
+		snd_soc_jack_report(info->jack, 0, ARIZONA_JACK_MASK | info->micd_button_mask);
 
 		/*
 		 * If the jack was removed during a headphone detection we
@@ -1389,29 +1345,6 @@ int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
 		break;
 	}
 
-	info->edev = devm_extcon_dev_allocate(dev, arizona_cable);
-	if (IS_ERR(info->edev)) {
-		dev_err(arizona->dev, "failed to allocate extcon device\n");
-		return -ENOMEM;
-	}
-
-	ret = devm_extcon_dev_register(dev, info->edev);
-	if (ret < 0) {
-		dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
-			ret);
-		return ret;
-	}
-
-	info->input = devm_input_allocate_device(dev);
-	if (!info->input) {
-		dev_err(arizona->dev, "Can't allocate input dev\n");
-		ret = -ENOMEM;
-		return ret;
-	}
-
-	info->input->name = "Headset";
-	info->input->phys = "arizona/extcon";
-
 	if (!pdata->micd_timeout)
 		pdata->micd_timeout = DEFAULT_MICD_TIMEOUT;
 
@@ -1535,9 +1468,9 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
 		info->num_micd_ranges = ARRAY_SIZE(micd_default_ranges);
 	}
 
-	if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_RANGE) {
-		dev_err(arizona->dev, "Too many MICD ranges: %d\n",
-			arizona->pdata.num_micd_ranges);
+	if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_BUTTONS) {
+		dev_err(arizona->dev, "Too many MICD ranges: %d > %d\n",
+			arizona->pdata.num_micd_ranges, ARIZONA_MAX_MICD_BUTTONS);
 		return -EINVAL;
 	}
 
@@ -1571,8 +1504,11 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
 			arizona_micd_levels[j], i);
 
 		arizona_micd_set_level(arizona, i, j);
-		input_set_capability(info->input, EV_KEY,
-				     info->micd_ranges[i].key);
+
+		/* SND_JACK_BTN_# masks start with the most significant bit */
+		info->micd_button_mask |= SND_JACK_BTN_0 >> i;
+		snd_jack_set_key(jack->jack, SND_JACK_BTN_0 >> i,
+				 info->micd_ranges[i].key);
 
 		/* Enable reporting of that range */
 		regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_2,
@@ -1620,6 +1556,8 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
 
 	arizona_extcon_set_mode(info, 0);
 
+	info->jack = jack;
+
 	pm_runtime_get_sync(arizona->dev);
 
 	if (info->micd_clamp) {
@@ -1680,18 +1618,10 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
 	if (ret != 0)
 		dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n", ret);
 
-	ret = input_register_device(info->input);
-	if (ret) {
-		dev_err(arizona->dev, "Can't register input device: %d\n", ret);
-		goto err_hpdet;
-	}
-
 	pm_runtime_put(arizona->dev);
 
 	return 0;
 
-err_hpdet:
-	arizona_free_irq(arizona, ARIZONA_IRQ_HPDET, info);
 err_micdet:
 	arizona_free_irq(arizona, ARIZONA_IRQ_MICDET, info);
 err_fall_wake:
@@ -1704,6 +1634,7 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
 	arizona_free_irq(arizona, jack_irq_rise, info);
 err_pm:
 	pm_runtime_put(arizona->dev);
+	info->jack = NULL;
 	return ret;
 }
 
@@ -1714,6 +1645,9 @@ static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
 	bool change;
 	int ret;
 
+	if (!info->jack)
+		return 0;
+
 	if (info->micd_clamp) {
 		jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
 		jack_irq_fall = ARIZONA_IRQ_MICD_CLAMP_FALL;
@@ -1748,6 +1682,7 @@ static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
 	regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
 			   ARIZONA_JD1_ENA, 0);
 	arizona_clk32k_disable(arizona);
+	info->jack = NULL;
 
 	return 0;
 }
diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
index fc515845a3e6..173ebd0bf8c9 100644
--- a/sound/soc/codecs/arizona.h
+++ b/sound/soc/codecs/arizona.h
@@ -97,9 +97,8 @@ struct arizona_priv {
 	struct delayed_work hpdet_work;
 	struct delayed_work micd_detect_work;
 	struct delayed_work micd_timeout_work;
+	struct snd_soc_jack *jack;
 	struct regulator *micvdd;
-	struct input_dev *input;
-	struct extcon_dev *edev;
 	struct gpio_desc *micd_pol_gpio;
 
 	u16 last_jackdet;
@@ -108,6 +107,7 @@ struct arizona_priv {
 	const struct arizona_micd_config *micd_modes;
 	int micd_num_modes;
 
+	int micd_button_mask;
 	const struct arizona_micd_range *micd_ranges;
 	int num_micd_ranges;
 
@@ -257,6 +257,9 @@ extern unsigned int arizona_mixer_values[ARIZONA_NUM_MIXER_INPUTS];
 #define ARIZONA_RATE_ENUM_SIZE 4
 #define ARIZONA_SAMPLE_RATE_ENUM_SIZE 14
 
+/* SND_JACK_* mask for supported cable/switch types */
+#define ARIZONA_JACK_MASK  (SND_JACK_HEADSET | SND_JACK_LINEOUT | SND_JACK_MECHANICAL)
+
 extern const char * const arizona_rate_text[ARIZONA_RATE_ENUM_SIZE];
 extern const unsigned int arizona_rate_val[ARIZONA_RATE_ENUM_SIZE];
 extern const char * const arizona_sample_rate_text[ARIZONA_SAMPLE_RATE_ENUM_SIZE];
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (9 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 20:56   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library Hans de Goede
  2021-01-22 16:41 ` [PATCH v3 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support Hans de Goede
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Cleanup the use of dev_foo functions used for logging:

1. Many of these are unnecessarily split over multiple lines
2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFERRED
   return value

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
- This is a new patch in v3 of this patch-series
---
 sound/soc/codecs/arizona-jack.c | 107 ++++++++++----------------------
 1 file changed, 34 insertions(+), 73 deletions(-)

diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
index 268d2a44d891..e101976e1c14 100644
--- a/sound/soc/codecs/arizona-jack.c
+++ b/sound/soc/codecs/arizona-jack.c
@@ -124,8 +124,7 @@ static void arizona_extcon_hp_clamp(struct arizona_priv *info,
 					 ARIZONA_HP1_TST_CAP_SEL_MASK,
 					 cap_sel);
 		if (ret != 0)
-			dev_warn(arizona->dev,
-				 "Failed to set TST_CAP_SEL: %d\n", ret);
+			dev_warn(arizona->dev, "Failed to set TST_CAP_SEL: %d\n", ret);
 		break;
 	default:
 		mask = ARIZONA_RMV_SHRT_HP1L;
@@ -145,23 +144,19 @@ static void arizona_extcon_hp_clamp(struct arizona_priv *info,
 					 ARIZONA_OUT1L_ENA |
 					 ARIZONA_OUT1R_ENA, 0);
 		if (ret != 0)
-			dev_warn(arizona->dev,
-				"Failed to disable headphone outputs: %d\n",
-				 ret);
+			dev_warn(arizona->dev, "Failed to disable headphone outputs: %d\n", ret);
 	}
 
 	if (mask) {
 		ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1L,
 					 mask, val);
 		if (ret != 0)
-			dev_warn(arizona->dev, "Failed to do clamp: %d\n",
-				 ret);
+			dev_warn(arizona->dev, "Failed to do clamp: %d\n", ret);
 
 		ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1R,
 					 mask, val);
 		if (ret != 0)
-			dev_warn(arizona->dev, "Failed to do clamp: %d\n",
-				 ret);
+			dev_warn(arizona->dev, "Failed to do clamp: %d\n", ret);
 	}
 
 	/* Restore the desired state while not doing the clamp */
@@ -171,9 +166,7 @@ static void arizona_extcon_hp_clamp(struct arizona_priv *info,
 					 ARIZONA_OUT1L_ENA |
 					 ARIZONA_OUT1R_ENA, arizona->hp_ena);
 		if (ret != 0)
-			dev_warn(arizona->dev,
-				 "Failed to restore headphone outputs: %d\n",
-				 ret);
+			dev_warn(arizona->dev, "Failed to restore headphone outputs: %d\n", ret);
 	}
 
 	snd_soc_dapm_mutex_unlock(arizona->dapm);
@@ -224,16 +217,14 @@ static void arizona_extcon_pulse_micbias(struct arizona_priv *info)
 
 	ret = snd_soc_component_force_enable_pin(component, widget);
 	if (ret != 0)
-		dev_warn(arizona->dev, "Failed to enable %s: %d\n",
-			 widget, ret);
+		dev_warn(arizona->dev, "Failed to enable %s: %d\n", widget, ret);
 
 	snd_soc_dapm_sync(dapm);
 
 	if (!arizona->pdata.micd_force_micbias) {
 		ret = snd_soc_component_disable_pin(component, widget);
 		if (ret != 0)
-			dev_warn(arizona->dev, "Failed to disable %s: %d\n",
-				 widget, ret);
+			dev_warn(arizona->dev, "Failed to disable %s: %d\n", widget, ret);
 
 		snd_soc_dapm_sync(dapm);
 	}
@@ -251,18 +242,13 @@ static void arizona_start_mic(struct arizona_priv *info)
 
 	if (info->detecting) {
 		ret = regulator_allow_bypass(info->micvdd, false);
-		if (ret != 0) {
-			dev_err(arizona->dev,
-				"Failed to regulate MICVDD: %d\n",
-				ret);
-		}
+		if (ret != 0)
+			dev_err(arizona->dev, "Failed to regulate MICVDD: %d\n", ret);
 	}
 
 	ret = regulator_enable(info->micvdd);
-	if (ret != 0) {
-		dev_err(arizona->dev, "Failed to enable MICVDD: %d\n",
-			ret);
-	}
+	if (ret != 0)
+		dev_err(arizona->dev, "Failed to enable MICVDD: %d\n", ret);
 
 	if (info->micd_reva) {
 		const struct reg_sequence reva[] = {
@@ -313,9 +299,7 @@ static void arizona_stop_mic(struct arizona_priv *info)
 
 	ret = snd_soc_component_disable_pin(component, widget);
 	if (ret != 0)
-		dev_warn(arizona->dev,
-			 "Failed to disable %s: %d\n",
-			 widget, ret);
+		dev_warn(arizona->dev, "Failed to disable %s: %d\n", widget, ret);
 
 	snd_soc_dapm_sync(dapm);
 
@@ -330,10 +314,8 @@ static void arizona_stop_mic(struct arizona_priv *info)
 	}
 
 	ret = regulator_allow_bypass(info->micvdd, true);
-	if (ret != 0) {
-		dev_err(arizona->dev, "Failed to bypass MICVDD: %d\n",
-			ret);
-	}
+	if (ret != 0)
+		dev_err(arizona->dev, "Failed to bypass MICVDD: %d\n", ret);
 
 	if (change) {
 		regulator_disable(info->micvdd);
@@ -372,16 +354,14 @@ static int arizona_hpdet_read(struct arizona_priv *info)
 
 	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2, &val);
 	if (ret != 0) {
-		dev_err(arizona->dev, "Failed to read HPDET status: %d\n",
-			ret);
+		dev_err(arizona->dev, "Failed to read HPDET status: %d\n", ret);
 		return ret;
 	}
 
 	switch (info->hpdet_ip_version) {
 	case 0:
 		if (!(val & ARIZONA_HP_DONE)) {
-			dev_err(arizona->dev, "HPDET did not complete: %x\n",
-				val);
+			dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
 			return -EAGAIN;
 		}
 
@@ -390,15 +370,13 @@ static int arizona_hpdet_read(struct arizona_priv *info)
 
 	case 1:
 		if (!(val & ARIZONA_HP_DONE_B)) {
-			dev_err(arizona->dev, "HPDET did not complete: %x\n",
-				val);
+			dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
 			return -EAGAIN;
 		}
 
 		ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
 		if (ret != 0) {
-			dev_err(arizona->dev, "Failed to read HP value: %d\n",
-				ret);
+			dev_err(arizona->dev, "Failed to read HP value: %d\n", ret);
 			return -EAGAIN;
 		}
 
@@ -411,8 +389,7 @@ static int arizona_hpdet_read(struct arizona_priv *info)
 		    (val < arizona_hpdet_b_ranges[range].threshold ||
 		     val >= ARIZONA_HPDET_B_RANGE_MAX)) {
 			range++;
-			dev_dbg(arizona->dev, "Moving to HPDET range %d\n",
-				range);
+			dev_dbg(arizona->dev, "Moving to HPDET range %d\n", range);
 			regmap_update_bits(arizona->regmap,
 					   ARIZONA_HEADPHONE_DETECT_1,
 					   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
@@ -428,8 +405,7 @@ static int arizona_hpdet_read(struct arizona_priv *info)
 			return ARIZONA_HPDET_MAX;
 		}
 
-		dev_dbg(arizona->dev, "HPDET read %d in range %d\n",
-			val, range);
+		dev_dbg(arizona->dev, "HPDET read %d in range %d\n", val, range);
 
 		val = arizona_hpdet_b_ranges[range].factor_b
 			/ ((val * 100) -
@@ -438,8 +414,7 @@ static int arizona_hpdet_read(struct arizona_priv *info)
 
 	case 2:
 		if (!(val & ARIZONA_HP_DONE_B)) {
-			dev_err(arizona->dev, "HPDET did not complete: %x\n",
-				val);
+			dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
 			return -EAGAIN;
 		}
 
@@ -475,8 +450,7 @@ static int arizona_hpdet_read(struct arizona_priv *info)
 		break;
 
 	default:
-		dev_warn(arizona->dev, "Unknown HPDET IP revision %d\n",
-			 info->hpdet_ip_version);
+		dev_warn(arizona->dev, "Unknown HPDET IP revision %d\n", info->hpdet_ip_version);
 		return -EINVAL;
 	}
 
@@ -665,8 +639,7 @@ static void arizona_identify_headphone(struct arizona_priv *info)
 	ret = regmap_update_bits(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1,
 				 ARIZONA_HP_POLL, ARIZONA_HP_POLL);
 	if (ret != 0) {
-		dev_err(arizona->dev, "Can't start HPDETL measurement: %d\n",
-			ret);
+		dev_err(arizona->dev, "Can't start HPDETL measurement: %d\n", ret);
 		goto err;
 	}
 
@@ -717,9 +690,7 @@ static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
 					 ARIZONA_HEADPHONE_DETECT_1,
 					 ARIZONA_HP_POLL, ARIZONA_HP_POLL);
 		if (ret != 0) {
-			dev_err(arizona->dev,
-				"Can't start HPDETL measurement: %d\n",
-				ret);
+			dev_err(arizona->dev, "Can't start HPDETL measurement: %d\n", ret);
 			goto err;
 		}
 	} else {
@@ -765,8 +736,7 @@ static int arizona_micd_adc_read(struct arizona_priv *info)
 
 	ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_4, &val);
 	if (ret != 0) {
-		dev_err(arizona->dev,
-			"Failed to read MICDET_ADCVAL: %d\n", ret);
+		dev_err(arizona->dev, "Failed to read MICDET_ADCVAL: %d\n", ret);
 		return ret;
 	}
 
@@ -799,16 +769,14 @@ static int arizona_micd_read(struct arizona_priv *info)
 	for (i = 0; i < 10 && !(val & MICD_LVL_0_TO_8); i++) {
 		ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_3, &val);
 		if (ret != 0) {
-			dev_err(arizona->dev,
-				"Failed to read MICDET: %d\n", ret);
+			dev_err(arizona->dev, "Failed to read MICDET: %d\n", ret);
 			return ret;
 		}
 
 		dev_dbg(arizona->dev, "MICDET: %x\n", val);
 
 		if (!(val & ARIZONA_MICD_VALID)) {
-			dev_warn(arizona->dev,
-				 "Microphone detection state invalid\n");
+			dev_warn(arizona->dev, "Microphone detection state invalid\n");
 			return -EINVAL;
 		}
 	}
@@ -857,8 +825,7 @@ static int arizona_micdet_reading(void *priv)
 		/* Don't need to regulate for button detection */
 		ret = regulator_allow_bypass(info->micvdd, true);
 		if (ret != 0) {
-			dev_err(arizona->dev, "Failed to bypass MICVDD: %d\n",
-				ret);
+			dev_err(arizona->dev, "Failed to bypass MICVDD: %d\n", ret);
 		}
 
 		return 0;
@@ -941,8 +908,7 @@ static int arizona_button_reading(void *priv)
 				dev_err(arizona->dev, "Button out of range\n");
 			}
 		} else {
-			dev_warn(arizona->dev, "Button with no mic: %x\n",
-				 val);
+			dev_warn(arizona->dev, "Button with no mic: %x\n", val);
 		}
 	} else {
 		dev_dbg(arizona->dev, "Mic button released\n");
@@ -1025,8 +991,7 @@ static int arizona_hpdet_wait(struct arizona_priv *info)
 		ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2,
 				&val);
 		if (ret) {
-			dev_err(arizona->dev,
-				"Failed to read HPDET state: %d\n", ret);
+			dev_err(arizona->dev, "Failed to read HPDET state: %d\n", ret);
 			return ret;
 		}
 
@@ -1077,8 +1042,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 
 	ret = regmap_read(arizona->regmap, ARIZONA_AOD_IRQ_RAW_STATUS, &val);
 	if (ret != 0) {
-		dev_err(arizona->dev, "Failed to read jackdet status: %d\n",
-			ret);
+		dev_err(arizona->dev, "Failed to read jackdet status: %d\n", ret);
 		mutex_unlock(&info->lock);
 		pm_runtime_put_autosuspend(arizona->dev);
 		return IRQ_NONE;
@@ -1248,8 +1212,7 @@ static int arizona_extcon_device_get_pdata(struct device *dev,
 		pdata->hpdet_channel = val;
 		break;
 	default:
-		dev_err(arizona->dev,
-			"Wrong wlf,hpdet-channel DT value %d\n", val);
+		dev_err(arizona->dev, "Wrong wlf,hpdet-channel DT value %d\n", val);
 		pdata->hpdet_channel = ARIZONA_ACCDET_MODE_HPL;
 	}
 
@@ -1303,7 +1266,7 @@ int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
 	info->micvdd = devm_regulator_get(dev, "MICVDD");
 	if (IS_ERR(info->micvdd)) {
 		ret = PTR_ERR(info->micvdd);
-		dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
+		dev_err_probe(arizona->dev, ret, "getting MICVDD\n");
 		return ret;
 	}
 
@@ -1391,9 +1354,7 @@ int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
 							 mode);
 		if (IS_ERR(info->micd_pol_gpio)) {
 			ret = PTR_ERR(info->micd_pol_gpio);
-			dev_err(arizona->dev,
-				"Failed to get microphone polarity GPIO: %d\n",
-				ret);
+			dev_err_probe(arizona->dev, ret, "getting microphone polarity GPIO\n");
 			return ret;
 		}
 	}
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (10 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  2021-01-22 20:58   ` Andy Shevchenko
  2021-01-22 16:41 ` [PATCH v3 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support Hans de Goede
  12 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Make all arizona codec drivers for which drivers/mfd/arizona-core.c used
to instantiate a "arizona-extcon" child-device use the new arizona-jack.c
library for jack-detection.

This has been tested on a Lenovo Yoga Tablet 2 1051L with a WM5102 codec.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/wm5102.c | 12 +++++++++++-
 sound/soc/codecs/wm5110.c | 12 +++++++++++-
 sound/soc/codecs/wm8997.c | 14 ++++++++++++--
 sound/soc/codecs/wm8998.c |  9 +++++++++
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/wm5102.c b/sound/soc/codecs/wm5102.c
index 70d353b63fe0..b77595fb3ea8 100644
--- a/sound/soc/codecs/wm5102.c
+++ b/sound/soc/codecs/wm5102.c
@@ -2004,6 +2004,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm5102 = {
 	.remove			= wm5102_component_remove,
 	.set_sysclk		= arizona_set_sysclk,
 	.set_pll		= wm5102_set_fll,
+	.set_jack		= arizona_jack_set_jack,
 	.name			= DRV_NAME,
 	.compress_ops		= &wm5102_compress_ops,
 	.controls		= wm5102_snd_controls,
@@ -2057,6 +2058,11 @@ static int wm5102_probe(struct platform_device *pdev)
 	if (ret != 0)
 		return ret;
 
+	/* This may return -EPROBE_DEFER, so do this early on */
+	ret = arizona_jack_codec_dev_probe(&wm5102->core, &pdev->dev);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < ARRAY_SIZE(wm5102->fll); i++)
 		wm5102->fll[i].vco_mult = 1;
 
@@ -2089,7 +2095,7 @@ static int wm5102_probe(struct platform_device *pdev)
 				  wm5102);
 	if (ret != 0) {
 		dev_err(&pdev->dev, "Failed to request DSP IRQ: %d\n", ret);
-		return ret;
+		goto err_jack_codec_dev;
 	}
 
 	ret = arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 1);
@@ -2123,6 +2129,8 @@ static int wm5102_probe(struct platform_device *pdev)
 err_dsp_irq:
 	arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
 	arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5102);
+err_jack_codec_dev:
+	arizona_jack_codec_dev_remove(&wm5102->core);
 
 	return ret;
 }
@@ -2141,6 +2149,8 @@ static int wm5102_remove(struct platform_device *pdev)
 	arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
 	arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5102);
 
+	arizona_jack_codec_dev_remove(&wm5102->core);
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
index 4238929b2375..ef22051a3599 100644
--- a/sound/soc/codecs/wm5110.c
+++ b/sound/soc/codecs/wm5110.c
@@ -2370,6 +2370,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm5110 = {
 	.remove			= wm5110_component_remove,
 	.set_sysclk		= arizona_set_sysclk,
 	.set_pll		= wm5110_set_fll,
+	.set_jack		= arizona_jack_set_jack,
 	.name			= DRV_NAME,
 	.compress_ops		= &wm5110_compress_ops,
 	.controls		= wm5110_snd_controls,
@@ -2424,6 +2425,11 @@ static int wm5110_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	/* This may return -EPROBE_DEFER, so do this early on */
+	ret = arizona_jack_codec_dev_probe(&wm5110->core, &pdev->dev);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < ARRAY_SIZE(wm5110->fll); i++)
 		wm5110->fll[i].vco_mult = 3;
 
@@ -2456,7 +2462,7 @@ static int wm5110_probe(struct platform_device *pdev)
 				  wm5110);
 	if (ret != 0) {
 		dev_err(&pdev->dev, "Failed to request DSP IRQ: %d\n", ret);
-		return ret;
+		goto err_jack_codec_dev;
 	}
 
 	ret = arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 1);
@@ -2490,6 +2496,8 @@ static int wm5110_probe(struct platform_device *pdev)
 err_dsp_irq:
 	arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
 	arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5110);
+err_jack_codec_dev:
+	arizona_jack_codec_dev_remove(&wm5110->core);
 
 	return ret;
 }
@@ -2510,6 +2518,8 @@ static int wm5110_remove(struct platform_device *pdev)
 	arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
 	arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5110);
 
+	arizona_jack_codec_dev_remove(&wm5110->core);
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/wm8997.c b/sound/soc/codecs/wm8997.c
index 229f2986cd96..4f5a848960e0 100644
--- a/sound/soc/codecs/wm8997.c
+++ b/sound/soc/codecs/wm8997.c
@@ -1096,6 +1096,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm8997 = {
 	.remove			= wm8997_component_remove,
 	.set_sysclk		= arizona_set_sysclk,
 	.set_pll		= wm8997_set_fll,
+	.set_jack		= arizona_jack_set_jack,
 	.controls		= wm8997_snd_controls,
 	.num_controls		= ARRAY_SIZE(wm8997_snd_controls),
 	.dapm_widgets		= wm8997_dapm_widgets,
@@ -1132,6 +1133,11 @@ static int wm8997_probe(struct platform_device *pdev)
 
 	arizona_init_dvfs(&wm8997->core);
 
+	/* This may return -EPROBE_DEFER, so do this early on */
+	ret = arizona_jack_codec_dev_probe(&wm8997->core, &pdev->dev);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < ARRAY_SIZE(wm8997->fll); i++)
 		wm8997->fll[i].vco_mult = 1;
 
@@ -1163,10 +1169,10 @@ static int wm8997_probe(struct platform_device *pdev)
 
 	ret = arizona_init_vol_limit(arizona);
 	if (ret < 0)
-		return ret;
+		goto err_jack_codec_dev;
 	ret = arizona_init_spk_irqs(arizona);
 	if (ret < 0)
-		return ret;
+		goto err_jack_codec_dev;
 
 	ret = devm_snd_soc_register_component(&pdev->dev,
 					      &soc_component_dev_wm8997,
@@ -1181,6 +1187,8 @@ static int wm8997_probe(struct platform_device *pdev)
 
 err_spk_irqs:
 	arizona_free_spk_irqs(arizona);
+err_jack_codec_dev:
+	arizona_jack_codec_dev_remove(&wm8997->core);
 
 	return ret;
 }
@@ -1194,6 +1202,8 @@ static int wm8997_remove(struct platform_device *pdev)
 
 	arizona_free_spk_irqs(arizona);
 
+	arizona_jack_codec_dev_remove(&wm8997->core);
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/wm8998.c b/sound/soc/codecs/wm8998.c
index 5413254295b7..f74af1c46933 100644
--- a/sound/soc/codecs/wm8998.c
+++ b/sound/soc/codecs/wm8998.c
@@ -1316,6 +1316,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm8998 = {
 	.remove			= wm8998_component_remove,
 	.set_sysclk		= arizona_set_sysclk,
 	.set_pll		= wm8998_set_fll,
+	.set_jack		= arizona_jack_set_jack,
 	.controls		= wm8998_snd_controls,
 	.num_controls		= ARRAY_SIZE(wm8998_snd_controls),
 	.dapm_widgets		= wm8998_dapm_widgets,
@@ -1350,6 +1351,11 @@ static int wm8998_probe(struct platform_device *pdev)
 	wm8998->core.arizona = arizona;
 	wm8998->core.num_inputs = 3;	/* IN1L, IN1R, IN2 */
 
+	/* This may return -EPROBE_DEFER, so do this early on */
+	ret = arizona_jack_codec_dev_probe(&wm8998->core, &pdev->dev);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < ARRAY_SIZE(wm8998->fll); i++)
 		wm8998->fll[i].vco_mult = 1;
 
@@ -1392,6 +1398,7 @@ static int wm8998_probe(struct platform_device *pdev)
 	arizona_free_spk_irqs(arizona);
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
+	arizona_jack_codec_dev_remove(&wm8998->core);
 
 	return ret;
 }
@@ -1405,6 +1412,8 @@ static int wm8998_remove(struct platform_device *pdev)
 
 	arizona_free_spk_irqs(arizona);
 
+	arizona_jack_codec_dev_remove(&wm8998->core);
+
 	return 0;
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support
  2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
                   ` (11 preceding siblings ...)
  2021-01-22 16:41 ` [PATCH v3 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library Hans de Goede
@ 2021-01-22 16:41 ` Hans de Goede
  12 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 16:41 UTC (permalink / raw)
  To: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown
  Cc: Hans de Goede, patches, linux-kernel, Andy Shevchenko,
	Charles Keepax, alsa-devel

Add jack detect support by creating a jack and calling
snd_soc_component_set_jack to register the created jack
with the codec.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_wm5102.c | 28 ++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bytcr_wm5102.c b/sound/soc/intel/boards/bytcr_wm5102.c
index f38850eb2eaf..cdfe203ed9fa 100644
--- a/sound/soc/intel/boards/bytcr_wm5102.c
+++ b/sound/soc/intel/boards/bytcr_wm5102.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+#include <sound/jack.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -31,6 +32,7 @@
 #define WM5102_MAX_SYSCLK_11025	45158400 /* max sysclk for 11.025K family */
 
 struct byt_wm5102_private {
+	struct snd_soc_jack jack;
 	struct clk *mclk;
 	struct gpio_desc *spkvdd_en_gpio;
 };
@@ -177,11 +179,23 @@ static const struct snd_kcontrol_new byt_wm5102_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Speaker"),
 };
 
+static struct snd_soc_jack_pin byt_wm5102_pins[] = {
+	{
+		.pin	= "Headphone",
+		.mask	= SND_JACK_HEADPHONE,
+	},
+	{
+		.pin	= "Headset Mic",
+		.mask	= SND_JACK_MICROPHONE,
+	},
+};
+
 static int byt_wm5102_init(struct snd_soc_pcm_runtime *runtime)
 {
 	struct snd_soc_card *card = runtime->card;
 	struct byt_wm5102_private *priv = snd_soc_card_get_drvdata(card);
-	int ret;
+	struct snd_soc_component *component = asoc_rtd_to_codec(runtime, 0)->component;
+	int ret, jack_type;
 
 	card->dapm.idle_bias_off = true;
 
@@ -210,6 +224,18 @@ static int byt_wm5102_init(struct snd_soc_pcm_runtime *runtime)
 		return ret;
 	}
 
+	jack_type = ARIZONA_JACK_MASK | SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+		    SND_JACK_BTN_2 | SND_JACK_BTN_3;
+	ret = snd_soc_card_jack_new(card, "Headset", jack_type,
+				    &priv->jack, byt_wm5102_pins,
+				    ARRAY_SIZE(byt_wm5102_pins));
+	if (ret) {
+		dev_err(card->dev, "Error creating jack: %d\n", ret);
+		return ret;
+	}
+
+	snd_soc_component_set_jack(component, &priv->jack, NULL);
+
 	return 0;
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake
  2021-01-22 16:40 ` [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake Hans de Goede
@ 2021-01-22 20:38   ` Andy Shevchenko
  2021-01-22 20:47     ` Hans de Goede
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:38 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Before this commit the extcon-arizona code was mixing pm_runtime_get()
> and pm_runtime_get_sync() in different places.
>
> In all places where pm_runtime_get[_sync]() is called, the code
> makes use of the device immediately after the call.
> This means that we should always use pm_runtime_get_sync().

I think it implies the non-atomic (may sleep) context in the below functions.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> - This is a new patch in v3 of this patch-set
> ---
>  drivers/extcon/extcon-arizona.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index 72d23b15108c..56d2ce05de50 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -290,7 +290,7 @@ static void arizona_start_mic(struct arizona_extcon_info *info)
>         unsigned int mode;
>
>         /* Microphone detection can't use idle mode */
> -       pm_runtime_get(info->dev);
> +       pm_runtime_get_sync(info->dev);
>
>         if (info->detecting) {
>                 ret = regulator_allow_bypass(info->micvdd, false);
> @@ -695,7 +695,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
>         dev_dbg(arizona->dev, "Starting HPDET\n");
>
>         /* Make sure we keep the device enabled during the measurement */
> -       pm_runtime_get(info->dev);
> +       pm_runtime_get_sync(info->dev);
>
>         info->hpdet_active = true;
>
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c
  2021-01-22 16:41 ` [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c Hans de Goede
@ 2021-01-22 20:40   ` Andy Shevchenko
  2021-01-22 21:17     ` Hans de Goede
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:40 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The jack handling for arizona codecs is being refactored so that it is
> done directly by the codec drivers, instead of having an extcon-driver
> bind to a separate "arizona-extcon" child-device for this.
>
> drivers/mfd/arizona-core.c has already been updated to no longer
> instantiate an "arizona-extcon" child-device for the arizona codecs.
>
> This means that the "arizona-extcon" driver is no longer useful
> (there are no longer any devices for it to bind to).
>
> This commit drops the extcon Kconfig / Makefile bits and moves
> drivers/extcon/extcon-arizona.c to sound/soc/codecs/arizona-jack.c .
>
> This is a preparation patch for converting the arizona extcon-driver into
> a helper library for letting the arizona codec-drivers directly report jack
> state through the standard sound/soc/soc-jack.c functions.

...

>  MAINTAINERS                                               | 1 -

> -F:     drivers/extcon/extcon-arizona.c

Commit message doesn't shed a light if we need to move this actually
to another record in MAINTAINERS database.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm
  2021-01-22 16:41 ` [PATCH v3 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm Hans de Goede
@ 2021-01-22 20:43   ` Andy Shevchenko
  0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:43 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Drivers for MFD child-devices such as the arizona codec drivers
> and the arizona-extcon driver can choose to either make
> runtime_pm_get/_put calls on their own child-device, which will
> then be propagated to their parent; or they can make them directly
> on their MFD parent-device.
>
> The arizona-extcon code was using runtime_pm_get/_put calls on
> its own child-device where as the codec drivers are using
> runtime_pm_get/_put calls on their parent.
>
> The arizona-extcon MFD cell/child-device has been removed and this
> commit is part of refactoring the arizona-extcon code into a library
> to be used directly from the codec drivers.
>
> Specifically this commit moves the code over to make
> runtime_pm_get/_put calls on the parent device (on arizona->dev)
> bringing the code inline with how the codec drivers do this.
>
> Note this also removes the pm_runtime_enable/_disable calls
> as pm_runtime support has already been enabled on the parent-device
> by the arizona MFD driver.
>
> This is part of a patch series converting the arizona extcon driver into
> a helper library for letting the arizona codec-drivers directly report
> jack state through the standard sound/soc/soc-jack.c functions.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  sound/soc/codecs/arizona-jack.c | 42 ++++++++++++++-------------------
>  sound/soc/codecs/arizona.h      |  1 -
>  2 files changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
> index 5b40316d0d75..a6e8071f84ab 100644
> --- a/sound/soc/codecs/arizona-jack.c
> +++ b/sound/soc/codecs/arizona-jack.c
> @@ -251,7 +251,7 @@ static void arizona_start_mic(struct arizona_priv *info)
>         unsigned int mode;
>
>         /* Microphone detection can't use idle mode */
> -       pm_runtime_get_sync(info->dev);
> +       pm_runtime_get_sync(arizona->dev);
>
>         if (info->detecting) {
>                 ret = regulator_allow_bypass(info->micvdd, false);
> @@ -296,7 +296,7 @@ static void arizona_start_mic(struct arizona_priv *info)
>                 dev_err(arizona->dev, "Failed to enable micd: %d\n", ret);
>         } else if (!change) {
>                 regulator_disable(info->micvdd);
> -               pm_runtime_put_autosuspend(info->dev);
> +               pm_runtime_put_autosuspend(arizona->dev);
>         }
>  }
>
> @@ -341,8 +341,8 @@ static void arizona_stop_mic(struct arizona_priv *info)
>
>         if (change) {
>                 regulator_disable(info->micvdd);
> -               pm_runtime_mark_last_busy(info->dev);
> -               pm_runtime_put_autosuspend(info->dev);
> +               pm_runtime_mark_last_busy(arizona->dev);
> +               pm_runtime_put_autosuspend(arizona->dev);
>         }
>  }
>
> @@ -534,7 +534,7 @@ static int arizona_hpdet_do_id(struct arizona_priv *info, int *reading,
>                 info->num_hpdet_res = 0;
>                 info->hpdet_retried = true;
>                 arizona_start_hpdet_acc_id(info);
> -               pm_runtime_put(info->dev);
> +               pm_runtime_put(arizona->dev);
>                 return -EAGAIN;
>         }
>
> @@ -631,7 +631,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>                 arizona_start_mic(info);
>
>         if (info->hpdet_active) {
> -               pm_runtime_put_autosuspend(info->dev);
> +               pm_runtime_put_autosuspend(arizona->dev);
>                 info->hpdet_active = false;
>         }
>
> @@ -656,7 +656,7 @@ static void arizona_identify_headphone(struct arizona_priv *info)
>         dev_dbg(arizona->dev, "Starting HPDET\n");
>
>         /* Make sure we keep the device enabled during the measurement */
> -       pm_runtime_get_sync(info->dev);
> +       pm_runtime_get_sync(arizona->dev);
>
>         info->hpdet_active = true;
>
> @@ -685,7 +685,7 @@ static void arizona_identify_headphone(struct arizona_priv *info)
>
>  err:
>         arizona_extcon_hp_clamp(info, false);
> -       pm_runtime_put_autosuspend(info->dev);
> +       pm_runtime_put_autosuspend(arizona->dev);
>
>         /* Just report headphone */
>         ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
> @@ -708,7 +708,7 @@ static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
>         dev_dbg(arizona->dev, "Starting identification via HPDET\n");
>
>         /* Make sure we keep the device enabled during the measurement */
> -       pm_runtime_get_sync(info->dev);
> +       pm_runtime_get_sync(arizona->dev);
>
>         info->hpdet_active = true;
>
> @@ -1006,7 +1006,7 @@ static void arizona_micd_detect(struct work_struct *work)
>         else
>                 arizona_button_reading(info);
>
> -       pm_runtime_mark_last_busy(info->dev);
> +       pm_runtime_mark_last_busy(arizona->dev);
>         mutex_unlock(&info->lock);
>  }
>
> @@ -1090,7 +1090,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>         cancelled_hp = cancel_delayed_work_sync(&info->hpdet_work);
>         cancelled_mic = cancel_delayed_work_sync(&info->micd_timeout_work);
>
> -       pm_runtime_get_sync(info->dev);
> +       pm_runtime_get_sync(arizona->dev);
>
>         mutex_lock(&info->lock);
>
> @@ -1110,7 +1110,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>                 dev_err(arizona->dev, "Failed to read jackdet status: %d\n",
>                         ret);
>                 mutex_unlock(&info->lock);
> -               pm_runtime_put_autosuspend(info->dev);
> +               pm_runtime_put_autosuspend(arizona->dev);
>                 return IRQ_NONE;
>         }
>
> @@ -1210,8 +1210,8 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>
>         mutex_unlock(&info->lock);
>
> -       pm_runtime_mark_last_busy(info->dev);
> -       pm_runtime_put_autosuspend(info->dev);
> +       pm_runtime_mark_last_busy(arizona->dev);
> +       pm_runtime_put_autosuspend(arizona->dev);
>
>         return IRQ_HANDLED;
>  }
> @@ -1366,7 +1366,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>
>         mutex_init(&info->lock);
>         info->arizona = arizona;
> -       info->dev = &pdev->dev;
>         info->last_jackdet = ~(ARIZONA_MICD_CLAMP_STS | ARIZONA_JD1_STS);
>         INIT_DELAYED_WORK(&info->hpdet_work, arizona_hpdet_work);
>         INIT_DELAYED_WORK(&info->micd_detect_work, arizona_micd_detect);
> @@ -1617,9 +1616,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>
>         arizona_extcon_set_mode(info, 0);
>
> -       pm_runtime_enable(&pdev->dev);
> -       pm_runtime_idle(&pdev->dev);
> -       pm_runtime_get_sync(&pdev->dev);
> +       pm_runtime_get_sync(arizona->dev);
>
>         if (info->micd_clamp) {
>                 jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
> @@ -1689,7 +1686,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>                 goto err_hpdet;
>         }
>
> -       pm_runtime_put(&pdev->dev);
> +       pm_runtime_put(arizona->dev);
>
>         return 0;
>
> @@ -1706,8 +1703,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>  err_rise:
>         arizona_free_irq(arizona, jack_irq_rise, info);
>  err_pm:
> -       pm_runtime_put(&pdev->dev);
> -       pm_runtime_disable(&pdev->dev);
> +       pm_runtime_put(arizona->dev);
>  err_gpio:
>         gpiod_put(info->micd_pol_gpio);
>         return ret;
> @@ -1747,7 +1743,7 @@ static int arizona_extcon_remove(struct platform_device *pdev)
>                         ret);
>         } else if (change) {
>                 regulator_disable(info->micvdd);
> -               pm_runtime_put(info->dev);
> +               pm_runtime_put(arizona->dev);
>         }
>
>         regmap_update_bits(arizona->regmap,
> @@ -1759,8 +1755,6 @@ static int arizona_extcon_remove(struct platform_device *pdev)
>
>         gpiod_put(info->micd_pol_gpio);
>
> -       pm_runtime_disable(&pdev->dev);
> -
>         return 0;
>  }
>
> diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
> index d1a263a67bba..7132dbc3c840 100644
> --- a/sound/soc/codecs/arizona.h
> +++ b/sound/soc/codecs/arizona.h
> @@ -93,7 +93,6 @@ struct arizona_priv {
>         bool dvfs_cached;
>
>         /* Variables used by arizona-jack.c code */
> -       struct device *dev;
>         struct mutex lock;
>         struct delayed_work hpdet_work;
>         struct delayed_work micd_detect_work;
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake
  2021-01-22 20:38   ` Andy Shevchenko
@ 2021-01-22 20:47     ` Hans de Goede
  0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 20:47 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

Hi,

On 1/22/21 9:38 PM, Andy Shevchenko wrote:
> On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Before this commit the extcon-arizona code was mixing pm_runtime_get()
>> and pm_runtime_get_sync() in different places.
>>
>> In all places where pm_runtime_get[_sync]() is called, the code
>> makes use of the device immediately after the call.
>> This means that we should always use pm_runtime_get_sync().
> 
> I think it implies the non-atomic (may sleep) context in the below functions.

Right, but there were always called with the info->lock mutex held anyways,
so no change there.

> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you.

Regards,

Hans



> 
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v3:
>> - This is a new patch in v3 of this patch-set
>> ---
>>  drivers/extcon/extcon-arizona.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>> index 72d23b15108c..56d2ce05de50 100644
>> --- a/drivers/extcon/extcon-arizona.c
>> +++ b/drivers/extcon/extcon-arizona.c
>> @@ -290,7 +290,7 @@ static void arizona_start_mic(struct arizona_extcon_info *info)
>>         unsigned int mode;
>>
>>         /* Microphone detection can't use idle mode */
>> -       pm_runtime_get(info->dev);
>> +       pm_runtime_get_sync(info->dev);
>>
>>         if (info->detecting) {
>>                 ret = regulator_allow_bypass(info->micvdd, false);
>> @@ -695,7 +695,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
>>         dev_dbg(arizona->dev, "Starting HPDET\n");
>>
>>         /* Make sure we keep the device enabled during the measurement */
>> -       pm_runtime_get(info->dev);
>> +       pm_runtime_get_sync(info->dev);
>>
>>         info->hpdet_active = true;
>>
>> --
>> 2.28.0
>>
> 
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers
  2021-01-22 16:41 ` [PATCH v3 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers Hans de Goede
@ 2021-01-22 20:47   ` Andy Shevchenko
  0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:47 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Convert the arizona extcon driver into a helper library for direct use
> from the arizona codec-drivers, rather then being bound to a separate
> MFD cell.
>
> Note the probe (and remove) sequence is split into 2 parts:
>
> 1. The arizona_jack_codec_dev_probe() function inits a bunch of
> jack-detect specific variables in struct arizona_priv and tries to get
> a number of resources where getting them may fail with -EPROBE_DEFER.
>
> 2. Then once the machine driver has create a snd_sock_jack through
> snd_soc_card_jack_new() it calls snd_soc_component_set_jack() on
> the codec component, which will call the new arizona_jack_set_jack(),
> which sets up jack-detection and requests the IRQs.
>
> This split is necessary, because the IRQ handlers need access to the
> arizona->dapm pointer and the snd_sock_jack which are not available
> when the codec-driver's probe function runs.
>
> Note this requires that machine-drivers for codecs which are converted
> to use the new helper functions from arizona-jack.c are modified to
> create a snd_soc_jack through snd_soc_card_jack_new() and register
> this jack with the codec through snd_soc_component_set_jack().

Okay, it seems that messages are on behalf of the parent while managed
resources are being attached to the actual device.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> - Pass dev (the codec-device) to devm_regulator_get instead of
>   passing arizona->dev. This is necessary so that the regulator gets
>   properly released when the coded driver unbinds.
> ---
>  sound/soc/codecs/Makefile       |   2 +-
>  sound/soc/codecs/arizona-jack.c | 125 +++++++++++++++-----------------
>  sound/soc/codecs/arizona.h      |   6 ++
>  3 files changed, 65 insertions(+), 68 deletions(-)
>
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index d277f0366e09..2e976cfaa862 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -43,7 +43,7 @@ snd-soc-ak4642-objs := ak4642.o
>  snd-soc-ak4671-objs := ak4671.o
>  snd-soc-ak5386-objs := ak5386.o
>  snd-soc-ak5558-objs := ak5558.o
> -snd-soc-arizona-objs := arizona.o
> +snd-soc-arizona-objs := arizona.o arizona-jack.o
>  snd-soc-bd28623-objs := bd28623.o
>  snd-soc-bt-sco-objs := bt-sco.o
>  snd-soc-cpcap-objs := cpcap.o
> diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
> index a6e8071f84ab..e121490eb379 100644
> --- a/sound/soc/codecs/arizona-jack.c
> +++ b/sound/soc/codecs/arizona-jack.c
> @@ -7,14 +7,12 @@
>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/err.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio.h>
>  #include <linux/input.h>
> -#include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
> @@ -1337,27 +1335,16 @@ static int arizona_extcon_device_get_pdata(struct device *dev,
>         return 0;
>  }
>
> -static int arizona_extcon_probe(struct platform_device *pdev)
> +int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
>  {
> -       struct arizona *arizona = dev_get_drvdata(pdev->dev.parent);
> +       struct arizona *arizona = info->arizona;
>         struct arizona_pdata *pdata = &arizona->pdata;
> -       struct arizona_priv *info;
> -       unsigned int val;
> -       unsigned int clamp_mode;
> -       int jack_irq_fall, jack_irq_rise;
> -       int ret, mode, i, j;
> -
> -       if (!arizona->dapm || !arizona->dapm->card)
> -               return -EPROBE_DEFER;
> -
> -       info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> -       if (!info)
> -               return -ENOMEM;
> +       int ret, mode;
>
>         if (!dev_get_platdata(arizona->dev))
> -               arizona_extcon_device_get_pdata(&pdev->dev, arizona);
> +               arizona_extcon_device_get_pdata(dev, arizona);
>
> -       info->micvdd = devm_regulator_get(&pdev->dev, "MICVDD");
> +       info->micvdd = devm_regulator_get(dev, "MICVDD");
>         if (IS_ERR(info->micvdd)) {
>                 ret = PTR_ERR(info->micvdd);
>                 dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
> @@ -1365,12 +1352,10 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>         }
>
>         mutex_init(&info->lock);
> -       info->arizona = arizona;
>         info->last_jackdet = ~(ARIZONA_MICD_CLAMP_STS | ARIZONA_JD1_STS);
>         INIT_DELAYED_WORK(&info->hpdet_work, arizona_hpdet_work);
>         INIT_DELAYED_WORK(&info->micd_detect_work, arizona_micd_detect);
>         INIT_DELAYED_WORK(&info->micd_timeout_work, arizona_micd_timeout_work);
> -       platform_set_drvdata(pdev, info);
>
>         switch (arizona->type) {
>         case WM5102:
> @@ -1404,20 +1389,20 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>                 break;
>         }
>
> -       info->edev = devm_extcon_dev_allocate(&pdev->dev, arizona_cable);
> +       info->edev = devm_extcon_dev_allocate(dev, arizona_cable);
>         if (IS_ERR(info->edev)) {
> -               dev_err(&pdev->dev, "failed to allocate extcon device\n");
> +               dev_err(arizona->dev, "failed to allocate extcon device\n");
>                 return -ENOMEM;
>         }
>
> -       ret = devm_extcon_dev_register(&pdev->dev, info->edev);
> +       ret = devm_extcon_dev_register(dev, info->edev);
>         if (ret < 0) {
>                 dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
>                         ret);
>                 return ret;
>         }
>
> -       info->input = devm_input_allocate_device(&pdev->dev);
> +       info->input = devm_input_allocate_device(dev);
>         if (!info->input) {
>                 dev_err(arizona->dev, "Can't allocate input dev\n");
>                 ret = -ENOMEM;
> @@ -1448,7 +1433,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>                 else
>                         mode = GPIOF_OUT_INIT_LOW;
>
> -               ret = devm_gpio_request_one(&pdev->dev, pdata->micd_pol_gpio,
> +               ret = devm_gpio_request_one(dev, pdata->micd_pol_gpio,
>                                             mode, "MICD polarity");
>                 if (ret != 0) {
>                         dev_err(arizona->dev, "Failed to request GPIO%d: %d\n",
> @@ -1481,17 +1466,38 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>         }
>
>         if (arizona->pdata.hpdet_id_gpio > 0) {
> -               ret = devm_gpio_request_one(&pdev->dev,
> -                                           arizona->pdata.hpdet_id_gpio,
> +               ret = devm_gpio_request_one(dev, arizona->pdata.hpdet_id_gpio,
>                                             GPIOF_OUT_INIT_LOW,
>                                             "HPDET");
>                 if (ret != 0) {
>                         dev_err(arizona->dev, "Failed to request GPIO%d: %d\n",
>                                 arizona->pdata.hpdet_id_gpio, ret);
> -                       goto err_gpio;
> +                       gpiod_put(info->micd_pol_gpio);
> +                       return ret;
>                 }
>         }
>
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(arizona_jack_codec_dev_probe);
> +
> +int arizona_jack_codec_dev_remove(struct arizona_priv *info)
> +{
> +       gpiod_put(info->micd_pol_gpio);
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(arizona_jack_codec_dev_remove);
> +
> +static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
> +                                          struct snd_soc_jack *jack)
> +{
> +       struct arizona *arizona = info->arizona;
> +       struct arizona_pdata *pdata = &arizona->pdata;
> +       unsigned int val;
> +       unsigned int clamp_mode;
> +       int jack_irq_fall, jack_irq_rise;
> +       int ret, i, j;
> +
>         if (arizona->pdata.micd_bias_start_time)
>                 regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_1,
>                                    ARIZONA_MICD_BIAS_STARTTIME_MASK,
> @@ -1532,16 +1538,15 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>         if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_RANGE) {
>                 dev_err(arizona->dev, "Too many MICD ranges: %d\n",
>                         arizona->pdata.num_micd_ranges);
> +               return -EINVAL;
>         }
>
>         if (info->num_micd_ranges > 1) {
>                 for (i = 1; i < info->num_micd_ranges; i++) {
>                         if (info->micd_ranges[i - 1].max >
>                             info->micd_ranges[i].max) {
> -                               dev_err(arizona->dev,
> -                                       "MICD ranges must be sorted\n");
> -                               ret = -EINVAL;
> -                               goto err_gpio;
> +                               dev_err(arizona->dev, "MICD ranges must be sorted\n");
> +                               return -EINVAL;
>                         }
>                 }
>         }
> @@ -1559,8 +1564,7 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>                 if (j == ARIZONA_NUM_MICD_BUTTON_LEVELS) {
>                         dev_err(arizona->dev, "Unsupported MICD level %d\n",
>                                 info->micd_ranges[i].max);
> -                       ret = -EINVAL;
> -                       goto err_gpio;
> +                       return -EINVAL;
>                 }
>
>                 dev_dbg(arizona->dev, "%d ohms for MICD threshold %d\n",
> @@ -1629,43 +1633,40 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>         ret = arizona_request_irq(arizona, jack_irq_rise,
>                                   "JACKDET rise", arizona_jackdet, info);
>         if (ret != 0) {
> -               dev_err(&pdev->dev, "Failed to get JACKDET rise IRQ: %d\n",
> -                       ret);
> +               dev_err(arizona->dev, "Failed to get JACKDET rise IRQ: %d\n", ret);
>                 goto err_pm;
>         }
>
>         ret = arizona_set_irq_wake(arizona, jack_irq_rise, 1);
>         if (ret != 0) {
> -               dev_err(&pdev->dev, "Failed to set JD rise IRQ wake: %d\n",
> -                       ret);
> +               dev_err(arizona->dev, "Failed to set JD rise IRQ wake: %d\n", ret);
>                 goto err_rise;
>         }
>
>         ret = arizona_request_irq(arizona, jack_irq_fall,
>                                   "JACKDET fall", arizona_jackdet, info);
>         if (ret != 0) {
> -               dev_err(&pdev->dev, "Failed to get JD fall IRQ: %d\n", ret);
> +               dev_err(arizona->dev, "Failed to get JD fall IRQ: %d\n", ret);
>                 goto err_rise_wake;
>         }
>
>         ret = arizona_set_irq_wake(arizona, jack_irq_fall, 1);
>         if (ret != 0) {
> -               dev_err(&pdev->dev, "Failed to set JD fall IRQ wake: %d\n",
> -                       ret);
> +               dev_err(arizona->dev, "Failed to set JD fall IRQ wake: %d\n", ret);
>                 goto err_fall;
>         }
>
>         ret = arizona_request_irq(arizona, ARIZONA_IRQ_MICDET,
>                                   "MICDET", arizona_micdet, info);
>         if (ret != 0) {
> -               dev_err(&pdev->dev, "Failed to get MICDET IRQ: %d\n", ret);
> +               dev_err(arizona->dev, "Failed to get MICDET IRQ: %d\n", ret);
>                 goto err_fall_wake;
>         }
>
>         ret = arizona_request_irq(arizona, ARIZONA_IRQ_HPDET,
>                                   "HPDET", arizona_hpdet_irq, info);
>         if (ret != 0) {
> -               dev_err(&pdev->dev, "Failed to get HPDET IRQ: %d\n", ret);
> +               dev_err(arizona->dev, "Failed to get HPDET IRQ: %d\n", ret);
>                 goto err_micdet;
>         }
>
> @@ -1677,12 +1678,11 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>
>         ret = regulator_allow_bypass(info->micvdd, true);
>         if (ret != 0)
> -               dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n",
> -                        ret);
> +               dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n", ret);
>
>         ret = input_register_device(info->input);
>         if (ret) {
> -               dev_err(&pdev->dev, "Can't register input device: %d\n", ret);
> +               dev_err(arizona->dev, "Can't register input device: %d\n", ret);
>                 goto err_hpdet;
>         }
>
> @@ -1704,14 +1704,11 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>         arizona_free_irq(arizona, jack_irq_rise, info);
>  err_pm:
>         pm_runtime_put(arizona->dev);
> -err_gpio:
> -       gpiod_put(info->micd_pol_gpio);
>         return ret;
>  }
>
> -static int arizona_extcon_remove(struct platform_device *pdev)
> +static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
>  {
> -       struct arizona_priv *info = platform_get_drvdata(pdev);
>         struct arizona *arizona = info->arizona;
>         int jack_irq_rise, jack_irq_fall;
>         bool change;
> @@ -1739,8 +1736,7 @@ static int arizona_extcon_remove(struct platform_device *pdev)
>                                        ARIZONA_MICD_ENA, 0,
>                                        &change);
>         if (ret < 0) {
> -               dev_err(&pdev->dev, "Failed to disable micd on remove: %d\n",
> -                       ret);
> +               dev_err(arizona->dev, "Failed to disable micd on remove: %d\n", ret);
>         } else if (change) {
>                 regulator_disable(info->micvdd);
>                 pm_runtime_put(arizona->dev);
> @@ -1753,22 +1749,17 @@ static int arizona_extcon_remove(struct platform_device *pdev)
>                            ARIZONA_JD1_ENA, 0);
>         arizona_clk32k_disable(arizona);
>
> -       gpiod_put(info->micd_pol_gpio);
> -
>         return 0;
>  }
>
> -static struct platform_driver arizona_extcon_driver = {
> -       .driver         = {
> -               .name   = "arizona-extcon",
> -       },
> -       .probe          = arizona_extcon_probe,
> -       .remove         = arizona_extcon_remove,
> -};
> -
> -module_platform_driver(arizona_extcon_driver);
> +int arizona_jack_set_jack(struct snd_soc_component *component,
> +                         struct snd_soc_jack *jack, void *data)
> +{
> +       struct arizona_priv *info = snd_soc_component_get_drvdata(component);
>
> -MODULE_DESCRIPTION("Arizona Extcon driver");
> -MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:extcon-arizona");
> +       if (jack)
> +               return arizona_jack_enable_jack_detect(info, jack);
> +       else
> +               return arizona_jack_disable_jack_detect(info);
> +}
> +EXPORT_SYMBOL_GPL(arizona_jack_set_jack);
> diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
> index 7132dbc3c840..fc515845a3e6 100644
> --- a/sound/soc/codecs/arizona.h
> +++ b/sound/soc/codecs/arizona.h
> @@ -386,4 +386,10 @@ static inline int arizona_unregister_notifier(struct snd_soc_component *componen
>
>  int arizona_of_get_audio_pdata(struct arizona *arizona);
>
> +int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev);
> +int arizona_jack_codec_dev_remove(struct arizona_priv *info);
> +
> +int arizona_jack_set_jack(struct snd_soc_component *component,
> +                         struct snd_soc_jack *jack, void *data);
> +
>  #endif
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events
  2021-01-22 16:41 ` [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Hans de Goede
@ 2021-01-22 20:50   ` Andy Shevchenko
  0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:50 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Use the snd_soc_jack code to report jack events, instead of using extcon
> for reporting the cable-type + an input_dev for reporting the button
> presses.
>
> The snd_soc_jack code will report the cable-type through both input_dev
> events and through ALSA controls and the button-presses through input_dev
> events.
>
> Note that this means that when the codec drivers are moved over to use
> the new arizona-jack.c library code instead of having a separate MFD
> extcon cell with the extcon-arizona.c driver, we will no longer report
> extcon events to userspace for cable-type changes. This should not be
> a problem since "standard" Linux distro userspace does not (and has
> never) used the extcon class interface for this. Android does have
> support for the extcon class interface, but that was introduced in
> the same release as support for input_dev cable-type events, so this
> should not be a problem for Android either.
>
> Note this also reduces ARIZONA_MAX_MICD_RANGE from 8 to 6, this is
> ok to do since this info is always provided through pdata (or defaults)
> and cannot be overridden from devicetree. All in kernel users of the

in-kernel

> pdata (and the fallback defaults) define 6 or less buttons/ranges.
>

It makes a lot of sense to me. It might be that we can optimize
arizona_button_reading() even more in the future.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmal.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  sound/soc/codecs/arizona-jack.c | 149 +++++++++-----------------------
>  sound/soc/codecs/arizona.h      |   7 +-
>  2 files changed, 47 insertions(+), 109 deletions(-)
>
> diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
> index e121490eb379..268d2a44d891 100644
> --- a/sound/soc/codecs/arizona-jack.c
> +++ b/sound/soc/codecs/arizona-jack.c
> @@ -16,8 +16,8 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
> -#include <linux/extcon-provider.h>
>
> +#include <sound/jack.h>
>  #include <sound/soc.h>
>
>  #include <linux/mfd/arizona/core.h>
> @@ -29,6 +29,12 @@
>
>  #define ARIZONA_MAX_MICD_RANGE 8
>
> +/*
> + * The hardware supports 8 ranges / buttons, but the snd-jack interface
> + * only supports 6 buttons (button 0-5).
> + */
> +#define ARIZONA_MAX_MICD_BUTTONS 6
> +
>  #define ARIZONA_MICD_CLAMP_MODE_JDL      0x4
>  #define ARIZONA_MICD_CLAMP_MODE_JDH      0x5
>  #define ARIZONA_MICD_CLAMP_MODE_JDL_GP5H 0x9
> @@ -86,14 +92,6 @@ static const int arizona_micd_levels[] = {
>         1257, 30000,
>  };
>
> -static const unsigned int arizona_cable[] = {
> -       EXTCON_MECHANICAL,
> -       EXTCON_JACK_MICROPHONE,
> -       EXTCON_JACK_HEADPHONE,
> -       EXTCON_JACK_LINE_OUT,
> -       EXTCON_NONE,
> -};
> -
>  static void arizona_start_hpdet_acc_id(struct arizona_priv *info);
>
>  static void arizona_extcon_hp_clamp(struct arizona_priv *info,
> @@ -559,8 +557,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>         struct arizona_priv *info = data;
>         struct arizona *arizona = info->arizona;
>         int id_gpio = arizona->pdata.hpdet_id_gpio;
> -       unsigned int report = EXTCON_JACK_HEADPHONE;
> -       int ret, reading, state;
> +       int ret, reading, state, report;
>         bool mic = false;
>
>         mutex_lock(&info->lock);
> @@ -573,11 +570,8 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>         }
>
>         /* If the cable was removed while measuring ignore the result */
> -       state = extcon_get_state(info->edev, EXTCON_MECHANICAL);
> -       if (state < 0) {
> -               dev_err(arizona->dev, "Failed to check cable state: %d\n", state);
> -               goto out;
> -       } else if (!state) {
> +       state = info->jack->status & SND_JACK_MECHANICAL;
> +       if (!state) {
>                 dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n");
>                 goto done;
>         }
> @@ -603,14 +597,11 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>
>         /* Report high impedence cables as line outputs */
>         if (reading >= 5000)
> -               report = EXTCON_JACK_LINE_OUT;
> +               report = SND_JACK_LINEOUT;
>         else
> -               report = EXTCON_JACK_HEADPHONE;
> +               report = SND_JACK_HEADPHONE;
>
> -       ret = extcon_set_state_sync(info->edev, report, true);
> -       if (ret != 0)
> -               dev_err(arizona->dev, "Failed to report HP/line: %d\n",
> -                       ret);
> +       snd_soc_jack_report(info->jack, report, SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
>
>  done:
>         /* Reset back to starting range */
> @@ -686,9 +677,8 @@ static void arizona_identify_headphone(struct arizona_priv *info)
>         pm_runtime_put_autosuspend(arizona->dev);
>
>         /* Just report headphone */
> -       ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
> -       if (ret != 0)
> -               dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
> +       snd_soc_jack_report(info->jack, SND_JACK_HEADPHONE,
> +                           SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
>
>         if (info->mic)
>                 arizona_start_mic(info);
> @@ -740,9 +730,8 @@ static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
>
>  err:
>         /* Just report headphone */
> -       ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
> -       if (ret != 0)
> -               dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
> +       snd_soc_jack_report(info->jack, SND_JACK_HEADPHONE,
> +                           SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
>
>         info->hpdet_active = false;
>  }
> @@ -863,11 +852,7 @@ static int arizona_micdet_reading(void *priv)
>
>                 arizona_identify_headphone(info);
>
> -               ret = extcon_set_state_sync(info->edev,
> -                                             EXTCON_JACK_MICROPHONE, true);
> -               if (ret != 0)
> -                       dev_err(arizona->dev, "Headset report failed: %d\n",
> -                               ret);
> +               snd_soc_jack_report(info->jack, SND_JACK_MICROPHONE, SND_JACK_MICROPHONE);
>
>                 /* Don't need to regulate for button detection */
>                 ret = regulator_allow_bypass(info->micvdd, true);
> @@ -930,7 +915,7 @@ static int arizona_button_reading(void *priv)
>  {
>         struct arizona_priv *info = priv;
>         struct arizona *arizona = info->arizona;
> -       int val, key, lvl, i;
> +       int val, key, lvl;
>
>         val = arizona_micd_read(info);
>         if (val < 0)
> @@ -947,14 +932,11 @@ static int arizona_button_reading(void *priv)
>                         lvl = val & ARIZONA_MICD_LVL_MASK;
>                         lvl >>= ARIZONA_MICD_LVL_SHIFT;
>
> -                       for (i = 0; i < info->num_micd_ranges; i++)
> -                               input_report_key(info->input,
> -                                                info->micd_ranges[i].key, 0);
> -
>                         if (lvl && ffs(lvl) - 1 < info->num_micd_ranges) {
> -                               key = info->micd_ranges[ffs(lvl) - 1].key;
> -                               input_report_key(info->input, key, 1);
> -                               input_sync(info->input);
> +                               key = ffs(lvl) - 1;
> +                               snd_soc_jack_report(info->jack,
> +                                                   SND_JACK_BTN_0 >> key,
> +                                                   info->micd_button_mask);
>                         } else {
>                                 dev_err(arizona->dev, "Button out of range\n");
>                         }
> @@ -964,10 +946,7 @@ static int arizona_button_reading(void *priv)
>                 }
>         } else {
>                 dev_dbg(arizona->dev, "Mic button released\n");
> -               for (i = 0; i < info->num_micd_ranges; i++)
> -                       input_report_key(info->input,
> -                                        info->micd_ranges[i].key, 0);
> -               input_sync(info->input);
> +               snd_soc_jack_report(info->jack, 0, info->micd_button_mask);
>                 arizona_extcon_pulse_micbias(info);
>         }
>
> @@ -980,20 +959,13 @@ static void arizona_micd_detect(struct work_struct *work)
>                                                 struct arizona_priv,
>                                                 micd_detect_work.work);
>         struct arizona *arizona = info->arizona;
> -       int ret;
>
>         cancel_delayed_work_sync(&info->micd_timeout_work);
>
>         mutex_lock(&info->lock);
>
>         /* If the cable was removed while measuring ignore the result */
> -       ret = extcon_get_state(info->edev, EXTCON_MECHANICAL);
> -       if (ret < 0) {
> -               dev_err(arizona->dev, "Failed to check cable state: %d\n",
> -                               ret);
> -               mutex_unlock(&info->lock);
> -               return;
> -       } else if (!ret) {
> +       if (!(info->jack->status & SND_JACK_MECHANICAL)) {
>                 dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n");
>                 mutex_unlock(&info->lock);
>                 return;
> @@ -1134,12 +1106,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>
>         if (info->last_jackdet == present) {
>                 dev_dbg(arizona->dev, "Detected jack\n");
> -               ret = extcon_set_state_sync(info->edev,
> -                                             EXTCON_MECHANICAL, true);
> -
> -               if (ret != 0)
> -                       dev_err(arizona->dev, "Mechanical report failed: %d\n",
> -                               ret);
> +               snd_soc_jack_report(info->jack, SND_JACK_MECHANICAL, SND_JACK_MECHANICAL);
>
>                 info->detecting = true;
>                 info->mic = false;
> @@ -1170,18 +1137,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>                 info->hpdet_done = false;
>                 info->hpdet_retried = false;
>
> -               for (i = 0; i < info->num_micd_ranges; i++)
> -                       input_report_key(info->input,
> -                                        info->micd_ranges[i].key, 0);
> -               input_sync(info->input);
> -
> -               for (i = 0; i < ARRAY_SIZE(arizona_cable) - 1; i++) {
> -                       ret = extcon_set_state_sync(info->edev,
> -                                       arizona_cable[i], false);
> -                       if (ret != 0)
> -                               dev_err(arizona->dev,
> -                                       "Removal report failed: %d\n", ret);
> -               }
> +               snd_soc_jack_report(info->jack, 0, ARIZONA_JACK_MASK | info->micd_button_mask);
>
>                 /*
>                  * If the jack was removed during a headphone detection we
> @@ -1389,29 +1345,6 @@ int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
>                 break;
>         }
>
> -       info->edev = devm_extcon_dev_allocate(dev, arizona_cable);
> -       if (IS_ERR(info->edev)) {
> -               dev_err(arizona->dev, "failed to allocate extcon device\n");
> -               return -ENOMEM;
> -       }
> -
> -       ret = devm_extcon_dev_register(dev, info->edev);
> -       if (ret < 0) {
> -               dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
> -                       ret);
> -               return ret;
> -       }
> -
> -       info->input = devm_input_allocate_device(dev);
> -       if (!info->input) {
> -               dev_err(arizona->dev, "Can't allocate input dev\n");
> -               ret = -ENOMEM;
> -               return ret;
> -       }
> -
> -       info->input->name = "Headset";
> -       info->input->phys = "arizona/extcon";
> -
>         if (!pdata->micd_timeout)
>                 pdata->micd_timeout = DEFAULT_MICD_TIMEOUT;
>
> @@ -1535,9 +1468,9 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>                 info->num_micd_ranges = ARRAY_SIZE(micd_default_ranges);
>         }
>
> -       if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_RANGE) {
> -               dev_err(arizona->dev, "Too many MICD ranges: %d\n",
> -                       arizona->pdata.num_micd_ranges);
> +       if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_BUTTONS) {
> +               dev_err(arizona->dev, "Too many MICD ranges: %d > %d\n",
> +                       arizona->pdata.num_micd_ranges, ARIZONA_MAX_MICD_BUTTONS);
>                 return -EINVAL;
>         }
>
> @@ -1571,8 +1504,11 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>                         arizona_micd_levels[j], i);
>
>                 arizona_micd_set_level(arizona, i, j);
> -               input_set_capability(info->input, EV_KEY,
> -                                    info->micd_ranges[i].key);
> +
> +               /* SND_JACK_BTN_# masks start with the most significant bit */
> +               info->micd_button_mask |= SND_JACK_BTN_0 >> i;
> +               snd_jack_set_key(jack->jack, SND_JACK_BTN_0 >> i,
> +                                info->micd_ranges[i].key);
>
>                 /* Enable reporting of that range */
>                 regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_2,
> @@ -1620,6 +1556,8 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>
>         arizona_extcon_set_mode(info, 0);
>
> +       info->jack = jack;
> +
>         pm_runtime_get_sync(arizona->dev);
>
>         if (info->micd_clamp) {
> @@ -1680,18 +1618,10 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>         if (ret != 0)
>                 dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n", ret);
>
> -       ret = input_register_device(info->input);
> -       if (ret) {
> -               dev_err(arizona->dev, "Can't register input device: %d\n", ret);
> -               goto err_hpdet;
> -       }
> -
>         pm_runtime_put(arizona->dev);
>
>         return 0;
>
> -err_hpdet:
> -       arizona_free_irq(arizona, ARIZONA_IRQ_HPDET, info);
>  err_micdet:
>         arizona_free_irq(arizona, ARIZONA_IRQ_MICDET, info);
>  err_fall_wake:
> @@ -1704,6 +1634,7 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>         arizona_free_irq(arizona, jack_irq_rise, info);
>  err_pm:
>         pm_runtime_put(arizona->dev);
> +       info->jack = NULL;
>         return ret;
>  }
>
> @@ -1714,6 +1645,9 @@ static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
>         bool change;
>         int ret;
>
> +       if (!info->jack)
> +               return 0;
> +
>         if (info->micd_clamp) {
>                 jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
>                 jack_irq_fall = ARIZONA_IRQ_MICD_CLAMP_FALL;
> @@ -1748,6 +1682,7 @@ static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
>         regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
>                            ARIZONA_JD1_ENA, 0);
>         arizona_clk32k_disable(arizona);
> +       info->jack = NULL;
>
>         return 0;
>  }
> diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
> index fc515845a3e6..173ebd0bf8c9 100644
> --- a/sound/soc/codecs/arizona.h
> +++ b/sound/soc/codecs/arizona.h
> @@ -97,9 +97,8 @@ struct arizona_priv {
>         struct delayed_work hpdet_work;
>         struct delayed_work micd_detect_work;
>         struct delayed_work micd_timeout_work;
> +       struct snd_soc_jack *jack;
>         struct regulator *micvdd;
> -       struct input_dev *input;
> -       struct extcon_dev *edev;
>         struct gpio_desc *micd_pol_gpio;
>
>         u16 last_jackdet;
> @@ -108,6 +107,7 @@ struct arizona_priv {
>         const struct arizona_micd_config *micd_modes;
>         int micd_num_modes;
>
> +       int micd_button_mask;
>         const struct arizona_micd_range *micd_ranges;
>         int num_micd_ranges;
>
> @@ -257,6 +257,9 @@ extern unsigned int arizona_mixer_values[ARIZONA_NUM_MIXER_INPUTS];
>  #define ARIZONA_RATE_ENUM_SIZE 4
>  #define ARIZONA_SAMPLE_RATE_ENUM_SIZE 14
>
> +/* SND_JACK_* mask for supported cable/switch types */
> +#define ARIZONA_JACK_MASK  (SND_JACK_HEADSET | SND_JACK_LINEOUT | SND_JACK_MECHANICAL)
> +
>  extern const char * const arizona_rate_text[ARIZONA_RATE_ENUM_SIZE];
>  extern const unsigned int arizona_rate_val[ARIZONA_RATE_ENUM_SIZE];
>  extern const char * const arizona_sample_rate_text[ARIZONA_SAMPLE_RATE_ENUM_SIZE];
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging
  2021-01-22 16:41 ` [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging Hans de Goede
@ 2021-01-22 20:56   ` Andy Shevchenko
  2021-01-22 21:33     ` Hans de Goede
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:56 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Cleanup the use of dev_foo functions used for logging:
>
> 1. Many of these are unnecessarily split over multiple lines
> 2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFERRED

s/RED$//

>    return value

...

> +               if (ret != 0)

Since you are touching it if (ret) would work already. Ditto for the
similar cases below.

...

>         if (IS_ERR(info->micvdd)) {
>                 ret = PTR_ERR(info->micvdd);
> -               dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
> +               dev_err_probe(arizona->dev, ret, "getting MICVDD\n");
>                 return ret;
>         }

Seems like your first dev_err_probe use :-)
Can be even more optimized, i.e.

  if (IS_ERR(info->micvdd))
    return dev_err_probe(arizona->dev, PTR_ERR(info->micvdd), "getting
MICVDD\n");

...

>                 if (IS_ERR(info->micd_pol_gpio)) {
>                         ret = PTR_ERR(info->micd_pol_gpio);
> -                       dev_err(arizona->dev,
> -                               "Failed to get microphone polarity GPIO: %d\n",
> -                               ret);
> +                       dev_err_probe(arizona->dev, ret, "getting microphone polarity GPIO\n");
>                         return ret;
>                 }

Ditto.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library
  2021-01-22 16:41 ` [PATCH v3 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library Hans de Goede
@ 2021-01-22 20:58   ` Andy Shevchenko
  0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2021-01-22 20:58 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Make all arizona codec drivers for which drivers/mfd/arizona-core.c used
> to instantiate a "arizona-extcon" child-device use the new arizona-jack.c
> library for jack-detection.
>
> This has been tested on a Lenovo Yoga Tablet 2 1051L with a WM5102 codec.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  sound/soc/codecs/wm5102.c | 12 +++++++++++-
>  sound/soc/codecs/wm5110.c | 12 +++++++++++-
>  sound/soc/codecs/wm8997.c | 14 ++++++++++++--
>  sound/soc/codecs/wm8998.c |  9 +++++++++
>  4 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/codecs/wm5102.c b/sound/soc/codecs/wm5102.c
> index 70d353b63fe0..b77595fb3ea8 100644
> --- a/sound/soc/codecs/wm5102.c
> +++ b/sound/soc/codecs/wm5102.c
> @@ -2004,6 +2004,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm5102 = {
>         .remove                 = wm5102_component_remove,
>         .set_sysclk             = arizona_set_sysclk,
>         .set_pll                = wm5102_set_fll,
> +       .set_jack               = arizona_jack_set_jack,
>         .name                   = DRV_NAME,
>         .compress_ops           = &wm5102_compress_ops,
>         .controls               = wm5102_snd_controls,
> @@ -2057,6 +2058,11 @@ static int wm5102_probe(struct platform_device *pdev)
>         if (ret != 0)
>                 return ret;
>
> +       /* This may return -EPROBE_DEFER, so do this early on */
> +       ret = arizona_jack_codec_dev_probe(&wm5102->core, &pdev->dev);
> +       if (ret)
> +               return ret;
> +
>         for (i = 0; i < ARRAY_SIZE(wm5102->fll); i++)
>                 wm5102->fll[i].vco_mult = 1;
>
> @@ -2089,7 +2095,7 @@ static int wm5102_probe(struct platform_device *pdev)
>                                   wm5102);
>         if (ret != 0) {
>                 dev_err(&pdev->dev, "Failed to request DSP IRQ: %d\n", ret);
> -               return ret;
> +               goto err_jack_codec_dev;
>         }
>
>         ret = arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 1);
> @@ -2123,6 +2129,8 @@ static int wm5102_probe(struct platform_device *pdev)
>  err_dsp_irq:
>         arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
>         arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5102);
> +err_jack_codec_dev:
> +       arizona_jack_codec_dev_remove(&wm5102->core);
>
>         return ret;
>  }
> @@ -2141,6 +2149,8 @@ static int wm5102_remove(struct platform_device *pdev)
>         arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
>         arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5102);
>
> +       arizona_jack_codec_dev_remove(&wm5102->core);
> +
>         return 0;
>  }
>
> diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c
> index 4238929b2375..ef22051a3599 100644
> --- a/sound/soc/codecs/wm5110.c
> +++ b/sound/soc/codecs/wm5110.c
> @@ -2370,6 +2370,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm5110 = {
>         .remove                 = wm5110_component_remove,
>         .set_sysclk             = arizona_set_sysclk,
>         .set_pll                = wm5110_set_fll,
> +       .set_jack               = arizona_jack_set_jack,
>         .name                   = DRV_NAME,
>         .compress_ops           = &wm5110_compress_ops,
>         .controls               = wm5110_snd_controls,
> @@ -2424,6 +2425,11 @@ static int wm5110_probe(struct platform_device *pdev)
>                         return ret;
>         }
>
> +       /* This may return -EPROBE_DEFER, so do this early on */
> +       ret = arizona_jack_codec_dev_probe(&wm5110->core, &pdev->dev);
> +       if (ret)
> +               return ret;
> +
>         for (i = 0; i < ARRAY_SIZE(wm5110->fll); i++)
>                 wm5110->fll[i].vco_mult = 3;
>
> @@ -2456,7 +2462,7 @@ static int wm5110_probe(struct platform_device *pdev)
>                                   wm5110);
>         if (ret != 0) {
>                 dev_err(&pdev->dev, "Failed to request DSP IRQ: %d\n", ret);
> -               return ret;
> +               goto err_jack_codec_dev;
>         }
>
>         ret = arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 1);
> @@ -2490,6 +2496,8 @@ static int wm5110_probe(struct platform_device *pdev)
>  err_dsp_irq:
>         arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
>         arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5110);
> +err_jack_codec_dev:
> +       arizona_jack_codec_dev_remove(&wm5110->core);
>
>         return ret;
>  }
> @@ -2510,6 +2518,8 @@ static int wm5110_remove(struct platform_device *pdev)
>         arizona_set_irq_wake(arizona, ARIZONA_IRQ_DSP_IRQ1, 0);
>         arizona_free_irq(arizona, ARIZONA_IRQ_DSP_IRQ1, wm5110);
>
> +       arizona_jack_codec_dev_remove(&wm5110->core);
> +
>         return 0;
>  }
>
> diff --git a/sound/soc/codecs/wm8997.c b/sound/soc/codecs/wm8997.c
> index 229f2986cd96..4f5a848960e0 100644
> --- a/sound/soc/codecs/wm8997.c
> +++ b/sound/soc/codecs/wm8997.c
> @@ -1096,6 +1096,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm8997 = {
>         .remove                 = wm8997_component_remove,
>         .set_sysclk             = arizona_set_sysclk,
>         .set_pll                = wm8997_set_fll,
> +       .set_jack               = arizona_jack_set_jack,
>         .controls               = wm8997_snd_controls,
>         .num_controls           = ARRAY_SIZE(wm8997_snd_controls),
>         .dapm_widgets           = wm8997_dapm_widgets,
> @@ -1132,6 +1133,11 @@ static int wm8997_probe(struct platform_device *pdev)
>
>         arizona_init_dvfs(&wm8997->core);
>
> +       /* This may return -EPROBE_DEFER, so do this early on */
> +       ret = arizona_jack_codec_dev_probe(&wm8997->core, &pdev->dev);
> +       if (ret)
> +               return ret;
> +
>         for (i = 0; i < ARRAY_SIZE(wm8997->fll); i++)
>                 wm8997->fll[i].vco_mult = 1;
>
> @@ -1163,10 +1169,10 @@ static int wm8997_probe(struct platform_device *pdev)
>
>         ret = arizona_init_vol_limit(arizona);
>         if (ret < 0)
> -               return ret;
> +               goto err_jack_codec_dev;
>         ret = arizona_init_spk_irqs(arizona);
>         if (ret < 0)
> -               return ret;
> +               goto err_jack_codec_dev;
>
>         ret = devm_snd_soc_register_component(&pdev->dev,
>                                               &soc_component_dev_wm8997,
> @@ -1181,6 +1187,8 @@ static int wm8997_probe(struct platform_device *pdev)
>
>  err_spk_irqs:
>         arizona_free_spk_irqs(arizona);
> +err_jack_codec_dev:
> +       arizona_jack_codec_dev_remove(&wm8997->core);
>
>         return ret;
>  }
> @@ -1194,6 +1202,8 @@ static int wm8997_remove(struct platform_device *pdev)
>
>         arizona_free_spk_irqs(arizona);
>
> +       arizona_jack_codec_dev_remove(&wm8997->core);
> +
>         return 0;
>  }
>
> diff --git a/sound/soc/codecs/wm8998.c b/sound/soc/codecs/wm8998.c
> index 5413254295b7..f74af1c46933 100644
> --- a/sound/soc/codecs/wm8998.c
> +++ b/sound/soc/codecs/wm8998.c
> @@ -1316,6 +1316,7 @@ static const struct snd_soc_component_driver soc_component_dev_wm8998 = {
>         .remove                 = wm8998_component_remove,
>         .set_sysclk             = arizona_set_sysclk,
>         .set_pll                = wm8998_set_fll,
> +       .set_jack               = arizona_jack_set_jack,
>         .controls               = wm8998_snd_controls,
>         .num_controls           = ARRAY_SIZE(wm8998_snd_controls),
>         .dapm_widgets           = wm8998_dapm_widgets,
> @@ -1350,6 +1351,11 @@ static int wm8998_probe(struct platform_device *pdev)
>         wm8998->core.arizona = arizona;
>         wm8998->core.num_inputs = 3;    /* IN1L, IN1R, IN2 */
>
> +       /* This may return -EPROBE_DEFER, so do this early on */
> +       ret = arizona_jack_codec_dev_probe(&wm8998->core, &pdev->dev);
> +       if (ret)
> +               return ret;
> +
>         for (i = 0; i < ARRAY_SIZE(wm8998->fll); i++)
>                 wm8998->fll[i].vco_mult = 1;
>
> @@ -1392,6 +1398,7 @@ static int wm8998_probe(struct platform_device *pdev)
>         arizona_free_spk_irqs(arizona);
>  err_pm_disable:
>         pm_runtime_disable(&pdev->dev);
> +       arizona_jack_codec_dev_remove(&wm8998->core);
>
>         return ret;
>  }
> @@ -1405,6 +1412,8 @@ static int wm8998_remove(struct platform_device *pdev)
>
>         arizona_free_spk_irqs(arizona);
>
> +       arizona_jack_codec_dev_remove(&wm8998->core);
> +
>         return 0;
>  }
>
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c
  2021-01-22 20:40   ` Andy Shevchenko
@ 2021-01-22 21:17     ` Hans de Goede
  0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 21:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

Hi,

On 1/22/21 9:40 PM, Andy Shevchenko wrote:
> On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The jack handling for arizona codecs is being refactored so that it is
>> done directly by the codec drivers, instead of having an extcon-driver
>> bind to a separate "arizona-extcon" child-device for this.
>>
>> drivers/mfd/arizona-core.c has already been updated to no longer
>> instantiate an "arizona-extcon" child-device for the arizona codecs.
>>
>> This means that the "arizona-extcon" driver is no longer useful
>> (there are no longer any devices for it to bind to).
>>
>> This commit drops the extcon Kconfig / Makefile bits and moves
>> drivers/extcon/extcon-arizona.c to sound/soc/codecs/arizona-jack.c .
>>
>> This is a preparation patch for converting the arizona extcon-driver into
>> a helper library for letting the arizona codec-drivers directly report jack
>> state through the standard sound/soc/soc-jack.c functions.
> 
> ...
> 
>>  MAINTAINERS                                               | 1 -
> 
>> -F:     drivers/extcon/extcon-arizona.c
> 
> Commit message doesn't shed a light if we need to move this actually
> to another record in MAINTAINERS database.

Ah, good call, yes we should. I'll fix this for v4.

Regards,

Hans


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging
  2021-01-22 20:56   ` Andy Shevchenko
@ 2021-01-22 21:33     ` Hans de Goede
  0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2021-01-22 21:33 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood,
	Jie Yang, Mark Brown, patches, Linux Kernel Mailing List,
	Charles Keepax, ALSA Development Mailing List

HI,

On 1/22/21 9:56 PM, Andy Shevchenko wrote:
> On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Cleanup the use of dev_foo functions used for logging:
>>
>> 1. Many of these are unnecessarily split over multiple lines
>> 2. Use dev_err_probe() in cases where we might get a -EPROBE_DEFERRED
> 
> s/RED$//

Ack, will fix for v4.


>>    return value
> 
> ...
> 
>> +               if (ret != 0)
> 
> Since you are touching it if (ret) would work already. Ditto for the
> similar cases below.

Ack.

> ...
> 
>>         if (IS_ERR(info->micvdd)) {
>>                 ret = PTR_ERR(info->micvdd);
>> -               dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
>> +               dev_err_probe(arizona->dev, ret, "getting MICVDD\n");
>>                 return ret;
>>         }
> 
> Seems like your first dev_err_probe use :-)

Erm, nope. I did this on purpose.

> Can be even more optimized, i.e.
> 
>   if (IS_ERR(info->micvdd))
>     return dev_err_probe(arizona->dev, PTR_ERR(info->micvdd), "getting
> MICVDD\n");

Ok, so that works here, but I deliberately kept it as is because it does
not work below and I wanted to be consistent.

On second thought. That is not really a good reason, so I've
made this a 1-lines as you suggest for v4.

> 
> ...
> 
>>                 if (IS_ERR(info->micd_pol_gpio)) {
>>                         ret = PTR_ERR(info->micd_pol_gpio);
>> -                       dev_err(arizona->dev,
>> -                               "Failed to get microphone polarity GPIO: %d\n",
>> -                               ret);
>> +                       dev_err_probe(arizona->dev, ret, "getting microphone polarity GPIO\n");

This new line is 96 chars as-is if I turn this into a one-liner it goes significantly
over the 100 chars line-length limit.

So I've kept this as is for v4.

Regards,

Hans


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-01-22 21:47 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 16:40 [PATCH v3 00/13] MFD/extcon/ASoC: Rework arizona codec jack-detect support Hans de Goede
2021-01-22 16:40 ` [PATCH v3 01/13] mfd: arizona: Drop arizona-extcon cells Hans de Goede
2021-01-22 16:40 ` [PATCH v3 02/13] extcon: arizona: Fix some issues when HPDET IRQ fires after the jack has been unplugged Hans de Goede
2021-01-22 16:40 ` [PATCH v3 03/13] extcon: arizona: Fix various races on driver unbind Hans de Goede
2021-01-22 16:40 ` [PATCH v3 04/13] extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call Hans de Goede
2021-01-22 16:40 ` [PATCH v3 05/13] extcon: arizona: Always use pm_runtime_get_sync() when we need the device to be awake Hans de Goede
2021-01-22 20:38   ` Andy Shevchenko
2021-01-22 20:47     ` Hans de Goede
2021-01-22 16:41 ` [PATCH v3 06/13] ASoC/extcon: arizona: Move arizona jack code to sound/soc/codecs/arizona-jack.c Hans de Goede
2021-01-22 20:40   ` Andy Shevchenko
2021-01-22 21:17     ` Hans de Goede
2021-01-22 16:41 ` [PATCH v3 07/13] ASoC: arizona-jack: Move jack-detect variables to struct arizona_priv Hans de Goede
2021-01-22 16:41 ` [PATCH v3 08/13] ASoC: arizona-jack: Use arizona->dev for runtime-pm Hans de Goede
2021-01-22 20:43   ` Andy Shevchenko
2021-01-22 16:41 ` [PATCH v3 09/13] ASoC: arizona-jack: convert into a helper library for codec drivers Hans de Goede
2021-01-22 20:47   ` Andy Shevchenko
2021-01-22 16:41 ` [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events Hans de Goede
2021-01-22 20:50   ` Andy Shevchenko
2021-01-22 16:41 ` [PATCH v3 11/13] ASoC: arizona-jack: Cleanup logging Hans de Goede
2021-01-22 20:56   ` Andy Shevchenko
2021-01-22 21:33     ` Hans de Goede
2021-01-22 16:41 ` [PATCH v3 12/13] ASoC: arizona: Make the wm5102, wm5110, wm8997 and wm8998 drivers use the new jack library Hans de Goede
2021-01-22 20:58   ` Andy Shevchenko
2021-01-22 16:41 ` [PATCH v3 13/13] ASoC: Intel: bytcr_wm5102: Add jack detect support Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).