Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give users a chance to plug their headsets in #328

Closed
wants to merge 2 commits into from
Closed

Give users a chance to plug their headsets in #328

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
674379c
ASoC: cs42l84: delay headset detect to prevent misdetections
chadmed Sep 28, 2024
720992f
ASoC: cs42l84: halve headset detection delay
chadmed Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions sound/soc/codecs/cs42l84.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ static void cs42l84_detect_hs(struct cs42l84_private *cs42l84)
CS42L84_MISC_DET_CTL_PDN_MIC_LVL_DET, 0);

/* TODO: Optimize */
msleep(100);
msleep(50);

/* Connect HSBIAS in CTIA wiring */
/* TODO: Should likely be subject of detection */
Expand All @@ -745,7 +745,7 @@ static void cs42l84_detect_hs(struct cs42l84_private *cs42l84)
FIELD_PREP(CS42L84_MISC_DET_CTL_DETECT_MODE, 3));

/* TODO: Optimize */
msleep(100);
msleep(50);

regmap_read(cs42l84->regmap, CS42L84_HS_DET_STATUS2, &reg);
regmap_update_bits(cs42l84->regmap,
Expand Down Expand Up @@ -840,6 +840,8 @@ static irqreturn_t cs42l84_irq_thread(int irq, void *data)
cs42l84->plug_state = CS42L84_PLUG;
dev_dbg(cs42l84->dev, "Plug event\n");

/* Give the user time to fully seat the plug */
msleep(1500);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please just increase the ring/tip sense debounce time in cs42l84_setup_plug_detect instead. cs42l84 seems to use the same 3 bit linear scale from 0 to 1500ms. cl42l43 uses 500ms for ring sense debounce as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that. The issue is the timing after the interrupt is triggered and we try to detect what type of device is plugged in. You can still reliably reproduce the failure mode even with the long debounce because the issue is the time between detection starting and the plug being fully seated.

What we could do is shorten this delay and max out the debounce, but this delay is important to eliminate (or at least, materially reduce) the chance of misdetecting headsets as headphones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is something fundamentally wrong witht the jack detection. Aftyer actually trying this I can reproduce it but the increased delay only pampers over the the issue. The issue is that the detection routine is run when the plug is not fully seated. That detect headsets as Detected bare headphone (no mic). I would expect that there is a second irq once the jack is fully inserted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we only get interrupts on ring sense plug and unplug. I suppose tip sense plug and ing sense unplug would be more appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are only getting the interrupt on ring sense, then how is the plug detection even running? We set current_plug_status based on the tip sense register only, and the detection logic only runs if tip sense says we're plugged in...

If tip sense isn't hit, then we would just clear the interrupt without doing anything right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never mind, that was the interrupt mask.
Looking at the not fully seated plug it's possible that the tip already makes contact. So maybe we would need to re-run hs_detect on ring sense irqs. I don't think any of the other cirrus jack codecs do that.

cs42l84_detect_hs(cs42l84);

/*
Expand Down