This commit improves the control of ground switches in AAD IRQ
Signed-off-by: David Rau <[email protected]>
---
sound/soc/codecs/da7219-aad.c | 60 +++++++++++++++++------------------
sound/soc/codecs/da7219-aad.h | 5 ++-
2 files changed, 31 insertions(+), 34 deletions(-)
diff --git a/sound/soc/codecs/da7219-aad.c b/sound/soc/codecs/da7219-aad.c
index e3d398b8f54e..993a0d00bc48 100644
--- a/sound/soc/codecs/da7219-aad.c
+++ b/sound/soc/codecs/da7219-aad.c
@@ -342,36 +342,17 @@ static void da7219_aad_hptest_work(struct work_struct *work)
static void da7219_aad_jack_det_work(struct work_struct *work)
{
struct da7219_aad_priv *da7219_aad =
- container_of(work, struct da7219_aad_priv, jack_det_work);
+ container_of(work, struct da7219_aad_priv, jack_det_work.work);
struct snd_soc_component *component = da7219_aad->component;
- u8 srm_st;
- mutex_lock(&da7219_aad->jack_det_mutex);
-
- srm_st = snd_soc_component_read(component, DA7219_PLL_SRM_STS) & DA7219_PLL_SRM_STS_MCLK;
- msleep(da7219_aad->gnd_switch_delay * ((srm_st == 0x0) ? 2 : 1) - 4);
/* Enable ground switch */
snd_soc_component_update_bits(component, 0xFB, 0x01, 0x01);
-
- mutex_unlock(&da7219_aad->jack_det_mutex);
}
-
/*
* IRQ
*/
-static irqreturn_t da7219_aad_pre_irq_thread(int irq, void *data)
-{
-
- struct da7219_aad_priv *da7219_aad = data;
-
- if (!da7219_aad->jack_inserted)
- schedule_work(&da7219_aad->jack_det_work);
-
- return IRQ_WAKE_THREAD;
-}
-
static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
{
struct da7219_aad_priv *da7219_aad = data;
@@ -392,6 +373,18 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
/* Read status register for jack insertion & type status */
statusa = snd_soc_component_read(component, DA7219_ACCDET_STATUS_A);
+ if (events[DA7219_AAD_IRQ_REG_A] & DA7219_E_JACK_INSERTED_MASK) {
+ u8 srm_st;
+ int delay = 0;
+
+ srm_st = snd_soc_component_read(component,
+ DA7219_PLL_SRM_STS) & DA7219_PLL_SRM_STS_MCLK;
+ delay = (da7219_aad->gnd_switch_delay * ((srm_st == 0x0) ? 2 : 1) - 2);
+ queue_delayed_work(da7219_aad->aad_wq,
+ &da7219_aad->jack_det_work,
+ msecs_to_jiffies(delay));
+ }
+
/* Clear events */
regmap_bulk_write(da7219->regmap, DA7219_ACCDET_IRQ_EVENT_A,
events, DA7219_AAD_IRQ_REG_MAX);
@@ -400,9 +393,6 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
events[DA7219_AAD_IRQ_REG_A], events[DA7219_AAD_IRQ_REG_B],
statusa);
- if (!da7219_aad->jack_inserted)
- cancel_work_sync(&da7219_aad->jack_det_work);
-
if (statusa & DA7219_JACK_INSERTION_STS_MASK) {
/* Jack Insertion */
if (events[DA7219_AAD_IRQ_REG_A] &
@@ -430,9 +420,9 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
if (statusa & DA7219_JACK_TYPE_STS_MASK) {
report |= SND_JACK_HEADSET;
mask |= SND_JACK_HEADSET | SND_JACK_LINEOUT;
- schedule_work(&da7219_aad->btn_det_work);
+ queue_work(da7219_aad->aad_wq, &da7219_aad->btn_det_work);
} else {
- schedule_work(&da7219_aad->hptest_work);
+ queue_work(da7219_aad->aad_wq, &da7219_aad->hptest_work);
}
}
@@ -465,6 +455,7 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
da7219_aad->jack_inserted = false;
/* Cancel any pending work */
+ cancel_delayed_work_sync(&da7219_aad->jack_det_work);
cancel_work_sync(&da7219_aad->btn_det_work);
cancel_work_sync(&da7219_aad->hptest_work);
@@ -964,13 +955,19 @@ int da7219_aad_init(struct snd_soc_component *component)
snd_soc_component_update_bits(component, DA7219_ACCDET_CONFIG_1,
DA7219_BUTTON_CONFIG_MASK, 0);
+ da7219_aad_handle_gnd_switch_time(component);
+
+ da7219_aad->aad_wq = create_singlethread_workqueue("da7219-aad");
+ if (!da7219_aad->aad_wq) {
+ dev_err(component->dev, "Failed to create aad workqueue\n");
+ return -ENOMEM;
+ }
+
+ INIT_DELAYED_WORK(&da7219_aad->jack_det_work, da7219_aad_jack_det_work);
INIT_WORK(&da7219_aad->btn_det_work, da7219_aad_btn_det_work);
INIT_WORK(&da7219_aad->hptest_work, da7219_aad_hptest_work);
- INIT_WORK(&da7219_aad->jack_det_work, da7219_aad_jack_det_work);
-
- mutex_init(&da7219_aad->jack_det_mutex);
- ret = request_threaded_irq(da7219_aad->irq, da7219_aad_pre_irq_thread,
+ ret = request_threaded_irq(da7219_aad->irq, NULL,
da7219_aad_irq_thread,
IRQF_TRIGGER_LOW | IRQF_ONESHOT,
"da7219-aad", da7219_aad);
@@ -984,8 +981,6 @@ int da7219_aad_init(struct snd_soc_component *component)
regmap_bulk_write(da7219->regmap, DA7219_ACCDET_IRQ_MASK_A,
&mask, DA7219_AAD_IRQ_REG_MAX);
- da7219_aad_handle_gnd_switch_time(component);
-
return 0;
}
@@ -1002,8 +997,10 @@ void da7219_aad_exit(struct snd_soc_component *component)
free_irq(da7219_aad->irq, da7219_aad);
+ cancel_delayed_work_sync(&da7219_aad->jack_det_work);
cancel_work_sync(&da7219_aad->btn_det_work);
cancel_work_sync(&da7219_aad->hptest_work);
+ destroy_workqueue(da7219_aad->aad_wq);
}
/*
@@ -1031,4 +1028,5 @@ int da7219_aad_probe(struct i2c_client *i2c)
MODULE_DESCRIPTION("ASoC DA7219 AAD Driver");
MODULE_AUTHOR("Adam Thomson <[email protected]>");
+MODULE_AUTHOR("David Rau <[email protected]>");
MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/da7219-aad.h b/sound/soc/codecs/da7219-aad.h
index be87ee47edde..fbfbf3e67918 100644
--- a/sound/soc/codecs/da7219-aad.h
+++ b/sound/soc/codecs/da7219-aad.h
@@ -197,9 +197,8 @@ struct da7219_aad_priv {
struct work_struct btn_det_work;
struct work_struct hptest_work;
- struct work_struct jack_det_work;
-
- struct mutex jack_det_mutex;
+ struct delayed_work jack_det_work;
+ struct workqueue_struct *aad_wq;
struct snd_soc_jack *jack;
bool micbias_resume_enable;
--
2.17.1
On Mon, Apr 10, 2023 at 09:26:34AM +0000, David Rau wrote:
> This commit improves the control of ground switches in AAD IRQ
In what way does it do this - what was previously unrelabile and how
does this change address that?
On Tue, Apr 11, 2023 at 4:32 AM Mark Brown <[email protected]> wrote:
>
> On Mon, Apr 10, 2023 at 09:26:34AM +0000, David Rau wrote:
>
> > This commit improves the control of ground switches in AAD IRQ
>
> In what way does it do this - what was previously unrelabile and how
> does this change address that?
One very specific problem is that da7219_aad_handle_gnd_switch_time()
is currently called after interrupts were enabled. As a result, the
delay time is not initialized if there is an interrupt before the
initialization. This results in a negative value passed to msleep().
Since the parameter to msleep() is unsigned, this causes it to sleep
forever which in turn causes a substantial number of hung task crashes
in ChromeOS. Plus, of course, the code doesn't really do anything in
this situation.
A secondary problem may be that calling msleep() with a potentially
large sleep time on a system worker isn't really a good idea, but I
didn't explore the impact further.
Guenter
On Wed, Apr 12, 2023 at 10:32:47AM +0800, Baili Deng wrote:
> The change in the patch done to address the issue Geunter mentioned is that
> da7219_aad_handle_gnd_switch_time() is now called before interrupts are
> enabled. To address the msleep() issue, the delay is now added as a delayed
> work of its own workqueue.
The point with the question was that this sort of stuff should be in the
commit messages. I can't really tell what the change is supposed to do
as things stand.
Please don't top post, reply in line with needed context. This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.
On Wed, Apr 12, 2023 at 03:27:01PM +0000, David.Rau.opensource wrote:
> Sorry for missing such needed information in the previous commit message.
> May I send another commit that includes the complete information again?
Yes, please.