Skip to content

Conversation

@ranj063
Copy link
Contributor

@ranj063 ranj063 commented Apr 3, 2023

There is no need to set this in topology as this can be derived based on whether the bit clock frequency is derived from 19.2MHz or 24.576MHz.

There is no need to set this in topology as this can be derived based on
whether the bit clock frequency is derived from 19.2MHz or 24.576MHz.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
if (!(19200000 % bclk_freq))
ssp_link_set_params(nhlt, 0);
else if (!(24576000 % bclk_freq))
ssp_link_set_params(nhlt, 1);
Copy link

Choose a reason for hiding this comment

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

Not 100% sure we want to hardcode this in the plugin (but agree this is a cleaner implementation in topology).

Choose a reason for hiding this comment

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

yeah, we should not hardcode HW specific details as this assumes the BCLK never has a fractional divider from MCLK.

Copy link
Contributor

Choose a reason for hiding this comment

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

it never does @lgirdwood, we never used M/N dividers. Integer division is the only thing we've ever used.

@plbossart
Copy link
Contributor

There is no need to set this in topology as this can be derived based on whether the bit clock frequency is derived from 19.2MHz or 24.576MHz.

@ranj063 this is a valid assumption if the SOC provides the MCLK, likely 99% of usages.

However, there's also the possibility of using an external clock, see "011b: MCLK input" so we need a escape to deal with this case as well.

Sorry to bring the exception to the rule, I just thought about it now.

If the goal is to avoid setting this all the time, I wonder if we should have a default in the topology base class and override it as needed. It might be more future proof that a change of tools required?

@ranj063
Copy link
Contributor Author

ranj063 commented Apr 5, 2023

closing this one in favor of thesofproject/sof#7370. Thanks everyone for the feedback

@ranj063 ranj063 closed this Apr 5, 2023
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