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.

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: codecs: add support for ES8326 #3508

Closed
wants to merge 1 commit into from
Closed

ASoC: codecs: add support for ES8326 #3508

wants to merge 1 commit into from

Conversation

yangxiaohua2009
Copy link

The ES8326 codec is not compatible with ES8316 and requires a dedicated driver.

Signed-off-by: David Yang yangxiaohua@everest-semi.com

@sofci
Copy link
Collaborator

sofci commented Mar 10, 2022

Can one of the admins verify this patch?

reply test this please to run this test once

@yangxiaohua2009
Copy link
Author

test this please

@yangxiaohua2009
Copy link
Author

@plbossart compile issue fixed. The codec driver may need to be rewrited to be compatible with ucm sof-essx8336.

@plbossart
Copy link
Member

test this please

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

The main issue is the delay in the interrupt thread, and several other cosmetic problems to fix.

amic2-src = [19];
jack-pol = [0e];
};
};
Copy link
Member

Choose a reason for hiding this comment

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

Usually dt-bindings are in a separate patch (they need to be acked by DT maintainers)

Copy link
Author

Choose a reason for hiding this comment

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

Can I submit the dt-binding patch later?

Copy link
Member

Choose a reason for hiding this comment

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

I personally don't care, but keep in mind this will be asked for upstream so you have to work on this.

sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
sound/soc/codecs/es8326.c Show resolved Hide resolved
sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
@plbossart
Copy link
Member

test this please

@plbossart
Copy link
Member

SOFCI TEST

sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
sound/soc/codecs/es8326.c Show resolved Hide resolved
if (ret == 0)
disable_irq(es8326->irq);
else
dev_warn(&i2c->dev, "Getting irq failed.");
Copy link
Member

Choose a reason for hiding this comment

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

can this driver work without the IRQ? if no, should an error be returned?

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

it's good that you added a workqueue for jack detection, but that needs to be handled in a .remove function.
also please split the DT bindings in a separate patch.

With those changes, I think this is starting to be ready for upstream in the next kernel cycle. Thanks @yangxiaohua2009

unsigned int sysclk;
};

static void es8326_jack_detect_handler(struct work_struct *work){
Copy link
Member

Choose a reason for hiding this comment

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

brace should be at start of line.

sound/soc/codecs/es8326.c Show resolved Hide resolved
jack-pol = [0e];
};
};

Copy link
Member

Choose a reason for hiding this comment

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

this should be a separate patch, so that DT reviewers can take a look without having to look at the driver code.

Copy link
Author

Choose a reason for hiding this comment

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

separate patch in #3546

sound/soc/codecs/es8326.c Show resolved Hide resolved
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

need to fix compilation errors, and you're still missing the separate patch for DT bindings.

sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
sound/soc/codecs/es8326.c Outdated Show resolved Hide resolved
@plbossart
Copy link
Member

@yangxiaohua2009 This looks good enough to me, please send this patch and the device-tree one to alsa-devel@alsa-project.org and CC: maintainers
Thanks.

The ES8326 codec is not compatible with ES8316 and requires a dedicated driver.

Signed-off-by: David Yang <yangxiaohua@everest-semi.com>
@plbossart
Copy link
Member

closing, this will be handled with an upstream merge.

@plbossart plbossart closed this Apr 4, 2022
asheplyakov pushed a commit to altlinux/linux-arm that referenced this pull request Sep 7, 2022
Based on ALSA SoF development branch.
Refer to:
Link: thesofproject#3393
Link: thesofproject#3508

Signed-off-by: Vasily Vinogradov <v.vinogradov@aq.ru>
Signed-off-by: Nikolai Kostrigin <nickel@altlinux.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants