Skip to content

Conversation

@brentlu
Copy link
Contributor

@brentlu brentlu commented May 4, 2021

Add support for cs42l42 running on GLK boards. There is a 10 ms BCLK
delay in the master branch version to avoid the noise on TX path.
However, this feature is not supported on GLK production branch yet so
we remove it for now.

Signed-off-by: Brent Lu brent.lu@intel.com

@brentlu brentlu requested a review from ranj063 as a code owner May 4, 2021 11:13
@brentlu brentlu requested review from lgirdwood and plbossart May 4, 2021 11:18
@brentlu
Copy link
Contributor Author

brentlu commented May 4, 2021

Backport #4121 to GLK production branch

@plbossart
Copy link
Member

the change is fine, but I am not sure the 'glk production branch' is actually maintained by anyone. Shouldn't those devices transition to the regular 'stable' branches after the initial release? I don't think anyone has a burning desire to keep maintaining one branch per device, that's quickly going to be problematic.
@cujomalainey thoughts?

@cujomalainey
Copy link
Contributor

@plbossart i agree that is the plan long term once we begin kernel uprevs, but these devices are still pinned at 4.14 and we are getting new codec support so I think for the sake of handling ABI changes, I think merging to this branch is probably the best solution for integration BUT it should land on main first.

@plbossart
Copy link
Member

@plbossart i agree that is the plan long term once we begin kernel uprevs, but these devices are still pinned at 4.14 and we are getting new codec support so I think for the sake of handling ABI changes, I think merging to this branch is probably the best solution for integration BUT it should land on main first.

ok, it's already on main, I wasn't sure who maintains this 'glk-012-stable' branch. it's not really stable by any stretch of imagination...

@lgirdwood
Copy link
Member

@slawblauciak are you to merge into glk-012 ?

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.

I have to take back my approval since this is not the same patch as what was submitted to the SOF main branch, and I have no idea how this solution might work without the 10ms delay.

Add support for cs42l42 running on GLK boards.

Signed-off-by: Brent Lu <brent.lu@intel.com>
@brentlu
Copy link
Contributor Author

brentlu commented May 26, 2021

The 10ms delay will be no longer needed since COE team is working on enabling bclk earlier in hw_params. I will update the commit message.

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.

this looks good but then we will have to modify this topology immediately to add the bclk early start. So either this is merged first, and other PRs such as #4219 will have to rebase. Or we drop this PR and add the changes in #4219. please clarify how you want to proceed with the merges.

@plbossart
Copy link
Member

this looks good but then we will have to modify this topology immediately to add the bclk early start. So either this is merged first, and other PRs such as #4219 will have to rebase. Or we drop this PR and add the changes in #4219. please clarify how you want to proceed with the merges.

Sorry, it was meant to be #4249 since you are requesting a merge into a product branch, not main.

@lgirdwood
Copy link
Member

@plbossart probably best for you to also merge FW if you need to orchestrate between kernel, FW and topologies.

@plbossart
Copy link
Member

@plbossart probably best for you to also merge FW if you need to orchestrate between kernel, FW and topologies.

Actually I don't even think the kernel PR is required for testing since it does nothing other than exposing bitfields we don't use.
I would like one SOF PR with firmware+topology changes, so far my tests are not successful. see #4219 (comment)

@brentlu
Copy link
Contributor Author

brentlu commented May 27, 2021

I thins we can drop this PR and use #4249 instead. Thanks.

@brentlu brentlu closed this May 27, 2021
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.

5 participants