-
Notifications
You must be signed in to change notification settings - Fork 130
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.
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
ASoC: intel: bytcr_rt5651: add remove function to disable jack #1149
Conversation
a long shot following rt5682 fix... |
SOFCI TEST |
4622638
to
f37b0df
Compare
@juimonen you may want to work with Hans De Goede (@jwrdegoede), he's done quite a bit of rt5651 support for baytrail tablets and might have devices you can test this patch on. Though those devices are usually BYT-CR and we don't support them well with SOF so far due to this stupid SSP2-SSP0 swap. |
FYI, I've a list of all devices / codecs I can test on here:
https://github.com/jwrdegoede/sunxi-fedora-scripts/blob/master/x86-codec-info
Never mind the repo-name, that is a leftover from when I was working on Allwinner based tablets instead of on x86 tablets.
The VIOS LTH17 listed there is a Cherry Trail device with rt5651. I can run some tests if necessary, but I've been sorta swamped with work lately, so expect a slow turnaround time on testing requests and please spell out what exactly you want tested (microphone, or speakers or ... what behavior to check for / expect).
|
@jwrdegoede ok thanks! I'll try still to do couple of more iterations internally, I'll then think what to do... |
When removing sof module the support_button_press function will oops because hp_jack pointer is not checked for NULL. So add a check to fix this. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
@plbossart in rt5651 the headset detection work seems to be cancelled when the module is removed. However if I add a null check in the support_button_press function we don't get oops in module removal anymore. Don't know if this is a correct fix though... @jwrdegoede any thoughts? |
@@ -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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juimonen @plbossart this fix looks right to me.
This PR was not tested by SOF CI, so has anyone actually tried the code on a platform with rt5651? |
@plbossart only @Jiangxinx tested on device with rt5651: #1059 (comment) |
ok, thanks for the pointer. |
When removing sof module the rt5651 jack handler will oops
if jack detection is not disabled. So add remove function,
which disables the jack detection.
Signed-off-by: Jaska Uimonen jaska.uimonen@intel.com