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

Jump to bottom

ASoC: intel: bytcr_rt5651: add remove function to disable jack #1149

Merged
merged 1 commit into from
Sep 5, 2019
Merged

ASoC: intel: bytcr_rt5651: add remove function to disable jack #1149

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions sound/soc/codecs/rt5651.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1770,6 +1770,9 @@ static int rt5651_detect_headset(struct snd_soc_component *component)

static bool rt5651_support_button_press(struct rt5651_priv *rt5651)
{
if (!rt5651->hp_jack)
return false;

/* Button press support only works with internal jack-detection */
return (rt5651->hp_jack->status & SND_JACK_MICROPHONE) &&
rt5651->gpiod_hp_det == NULL;
Copy link
Member

Choose a reason for hiding this comment

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

if you look at the caller, the hp_jack is set to NULL after calling this function:

static void rt5651_disable_jack_detect(struct snd_soc_component *component)
{
	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);

	disable_irq(rt5651->irq);
	rt5651_cancel_work(rt5651);

	if (rt5651_support_button_press(rt5651)) {
		rt5651_disable_micbias1_ovcd_irq(component);
		rt5651_disable_micbias1_for_ovcd(component);
		snd_soc_jack_report(rt5651->hp_jack, 0, SND_JACK_BTN_0);
	}

	rt5651->hp_jack = NULL;
}

so something is fishy here.

Copy link
Author

Choose a reason for hiding this comment

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

@plbossart yes for sure. I guess I'm delaying something with the addition or someone is calling set_jack(NULL) twice. As I understand this will access NULL if you call it twice? Not sure of the semantics of set_jack, I guess you should be able to call it twice with NULL...

Copy link
Collaborator

Choose a reason for hiding this comment

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

@juimonen there was a recent addition in soc-core.c to call set_jack(NULL) for robustness.

linux/sound/soc/soc-core.c

Line 935 in f95e4eb

snd_soc_component_set_jack(component, NULL, NULL);

Do you think this could be the reason?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it is, I think this fix is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the commit that added this change: 3bb936f

Copy link
Author

Choose a reason for hiding this comment

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

@ranjani yeah could be... I don't have the rt5651 device to test this, usually these devices don't reboot nicely after kernel oops, so it is really hard also with remote access device.

Expand Down