Support Cirrus Logic CS42L42 headphone codec #2884

brentlu · 2021-04-30T09:18:20Z

It's a series to:

Add a machine driver, sof_cs42l42, to support Cirrus Logic CS42L42 + Maxim MAX98357A on GLK devices.
Move MAX98357A support to a common module, sof_maxim_common
Code refactory to sof_rt5682 and sof_cs42l42 to use sof_maxim_common to support Maxim MAX98357A

brentlu · 2021-04-30T09:39:45Z

The topology is here: thesofproject/sof#4121

plbossart

Thanks @brentlu it's quite good but see below a couple of suggested improvements to remove copy-paste and improve code.

I think you are missing the sound/soc/intel/common/soc-acpi..glk-match.c, this would be needed to load the machine driver.

sound/soc/intel/boards/sof_cs42l42.c

sound/soc/intel/boards/Kconfig

sound/soc/intel/boards/sof_rt5682.c

sound/soc/intel/boards/sof_cs42l42.c

Move max98357a code to this common module so it could be shared between multiple SOF machine drivers. Signed-off-by: Brent Lu <brent.lu@intel.com>

lyakh

some minor cosmetic improvement suggestions

sound/soc/intel/boards/sof_cs42l42.c

plbossart

thanks @brentlu, nice work.

plbossart · 2021-05-03T14:23:52Z

I had to manually approve @brentlu so let's wait until we see the GitHub Actions results + another approved to merge

plbossart

I have to take my approval back after looking at the topology.
It's simpler if we have first the SSP links added, then dmic and HDMI last. The topology changes [1] can be simplified a lot by using the same order as for DA7219.

[1] thesofproject/sof@ef0ebaf#diff-ef984278da4b010127ccd588daa6d53c55bff7a852c2ef42f7b39a82d4bd3951R195

brentlu · 2021-05-03T16:36:30Z

Adjust the DAI Link sequence in sof_card_dai_links_create() function to follow the order: SPK, HEADPHONE, DMIC, HDMI

plbossart · 2021-05-03T23:20:22Z

restarting SOF CI tests since we are missing the configuration for the new machine driver

plbossart · 2021-05-03T23:23:01Z

SOFCI TEST

plbossart

@brentlu I added the missing config and CI reports errors with make W=1 and also checkpatch.pl --strict has quite a few warnings. Nothing critical but let's make this PR shiny for upstream. Thanks!

bardliao

Looks quite good to me, just some minor comments.

sound/soc/intel/boards/sof_cs42l42.c

brentlu · 2021-05-04T12:35:40Z

warnings from checkpatch are fixed

brentlu · 2021-05-04T14:18:33Z

False alarm?

Commit 429ffb620dcd ("ASoC: Intel: add sof-cs42l42 machine driver")

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#74:
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 570 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit 429ffb620dcd ("ASoC: Intel: add sof-cs42l42 machine driver") has style problems, please review.

brentlu · 2021-05-04T15:11:47Z

@plbossart If there is no concern, I'd like to send the patches to alsa-dev due to the tight schedule. Is it ok?

plbossart · 2021-05-04T16:02:02Z

False alarm?

Commit 429ffb6 ("ASoC: Intel: add sof-cs42l42 machine driver")

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#74:
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 570 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

Commit 429ffb6 ("ASoC: Intel: add sof-cs42l42 machine driver") has style problems, please review.

yes, that's normal and not a problem.

plbossart · 2021-05-04T16:11:38Z

@plbossart If there is no concern, I'd like to send the patches to alsa-dev due to the tight schedule. Is it ok?

let me check with Mark Brown, there's a missing dependency upstream, some previous patches were not applied, and if you send the patches they will not apply directly. thanks!

plbossart · 2021-05-04T16:13:03Z

still a make W=1 issue @brentlu . You can fix this in your setup by running 'make sound/soc/codecs/ W=1' (the last / matters). Thanks!

plbossart · 2021-05-04T16:32:14Z

@plbossart If there is no concern, I'd like to send the patches to alsa-dev due to the tight schedule. Is it ok?

let me check with Mark Brown, there's a missing dependency upstream, some previous patches were not applied, and if you send the patches they will not apply directly. thanks!

ok, so I need to resend the previous series which was partly missed for some reason, and I'll include your PR on top once all the warnings are removed. Does this work for you @brentlu? Thanks!

brentlu · 2021-05-05T01:57:29Z

The warning is from the codec header file which we don't need it. I've removed it from machine driver and send a mail to Cirrus team to fix it.

brentlu · 2021-05-05T01:58:05Z

Thanks. I think it's the best option.

plbossart

sorry, one more fix needed, the code was inspired by an earlier version of sof-rt5682 but the .nonatomic property is not needed.

sound/soc/intel/boards/sof_cs42l42.c

The machine driver is a generic machine driver for SOF with cs42l42 I2C codec. It currently supports Maxim MAX98357A speker amp on GLK but is extensible for other apms and platforms. Signed-off-by: Brent Lu <brent.lu@intel.com>

Refactor the machine driver by using the common code in maxim-common module to support max98357a. Signed-off-by: Brent Lu <brent.lu@intel.com>

plbossart

let's merge, thanks @brentlu

plbossart · 2021-05-05T16:03:20Z

I'll use my superpowers to merge and avoid more delays

brentlu requested review from bardliao, dbaluta, kv2019i, lgirdwood, plbossart and ranj063 as code owners April 30, 2021 09:18

This was referenced Apr 30, 2021

[FEATURE] Support Cirrus Logic CS42L42 headphone codec thesofproject/sof#4122

Closed

topology: sof-glk-cs42l42: add topology for cs42l42 thesofproject/sof#4121

Merged

plbossart requested changes Apr 30, 2021

View reviewed changes

ASoC: Intel: maxim-common: support max98357a

5ebe386

Move max98357a code to this common module so it could be shared between multiple SOF machine drivers. Signed-off-by: Brent Lu <brent.lu@intel.com>

brentlu force-pushed the cs42l42 branch 2 times, most recently from 4d62b0e to ba0d2dd Compare May 3, 2021 05:28

lyakh reviewed May 3, 2021

View reviewed changes

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

brentlu force-pushed the cs42l42 branch from ba0d2dd to cb1431a Compare May 3, 2021 11:39

plbossart approved these changes May 3, 2021

View reviewed changes

plbossart requested changes May 3, 2021

View reviewed changes

brentlu force-pushed the cs42l42 branch from cb1431a to 0d2fce8 Compare May 3, 2021 16:15

plbossart approved these changes May 3, 2021

View reviewed changes

plbossart requested changes May 3, 2021

View reviewed changes

bardliao reviewed May 4, 2021

View reviewed changes

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

brentlu force-pushed the cs42l42 branch from 0d2fce8 to cb3ae28 Compare May 4, 2021 06:00

brentlu force-pushed the cs42l42 branch from cb3ae28 to 39c7009 Compare May 4, 2021 14:00

brentlu force-pushed the cs42l42 branch from 39c7009 to b92ad43 Compare May 5, 2021 01:39

plbossart requested changes May 5, 2021

View reviewed changes

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

sound/soc/intel/boards/sof_cs42l42.c Outdated Show resolved Hide resolved

brentlu added 2 commits May 5, 2021 23:32

ASoC: Intel: add sof-cs42l42 machine driver

3fc90ce

The machine driver is a generic machine driver for SOF with cs42l42 I2C codec. It currently supports Maxim MAX98357A speker amp on GLK but is extensible for other apms and platforms. Signed-off-by: Brent Lu <brent.lu@intel.com>

ASoC: Intel: sof_rt5682: code refactory for max98357a

0166f91

Refactor the machine driver by using the common code in maxim-common module to support max98357a. Signed-off-by: Brent Lu <brent.lu@intel.com>

brentlu force-pushed the cs42l42 branch from b92ad43 to 0166f91 Compare May 5, 2021 15:36

plbossart approved these changes May 5, 2021

View reviewed changes

plbossart merged commit eaa4677 into thesofproject:topic/sof-dev May 5, 2021

Support Cirrus Logic CS42L42 headphone codec #2884

Support Cirrus Logic CS42L42 headphone codec #2884

Uh oh!

Conversation

brentlu commented Apr 30, 2021

Uh oh!

brentlu commented Apr 30, 2021

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lyakh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented May 3, 2021

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

brentlu commented May 3, 2021

Uh oh!

plbossart commented May 3, 2021

Uh oh!

plbossart commented May 3, 2021

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

bardliao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brentlu commented May 4, 2021

Uh oh!

brentlu commented May 4, 2021

Commit 429ffb620dcd ("ASoC: Intel: add sof-cs42l42 machine driver")

Uh oh!

brentlu commented May 4, 2021

Uh oh!

plbossart commented May 4, 2021

Commit 429ffb6 ("ASoC: Intel: add sof-cs42l42 machine driver")

Uh oh!

plbossart commented May 4, 2021

Uh oh!

plbossart commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brentlu commented May 5, 2021

Uh oh!

brentlu commented May 5, 2021

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented May 5, 2021

Uh oh!

Reviewers

plbossart commented May 4, 2021 •

edited

Loading

plbossart commented May 4, 2021 •

edited

Loading