+ }
+ cfg = (struct nhlt_fmt_cfg *)(cfg->config.caps + cfg->config.size); + } + } + epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length); + }
+ return mclk_mask;
Although I understand that it is relegated to the caller, but if both mclk being set is considered an error maybe add some kind of check here instead and free callers from having to remember about it?
if (hweight_long(mclk_mask) != 1) return -EINVAL;
return mclk_mask;
I went back and forth multiple times on this one. I can't figure out if this would be a bug or a feature, it could be e.g. a test capability and it's supported in hardware. I decided to make the decision in the caller rather than a lower level in the library.
If the tools used to generate NHLT don't support this multi-MCLK mode then we could indeed move the test here.
Considering comment I added above I've asked Czarek to also check this series. I'm not sure it even makes sense to name the field "_mask" when it is one bit...
it's two bits, see above.
So I've spend a bit talking with FW team, and you are right, I got confused by one of the tables and some code that specified it as 1 bit field and rest as reserved, while other documents do specify it as a variable range of bits.
Yeah, I had to ask multiple times as well. It's far from self-explanatory and all the findings were based on NHLT shared with me.
See https://github.com/thesofproject/linux/issues/3336#issuecomment-1206176141 for two examples with MCLK0 and MCLK1 used.
Going back to return value, the tool I have access to only has support for MCLK0. I guess we can make the assumption for now that everyone connects codec to one clock source and if someone later implements HW where somehow 2 different clocks are used (depending on format) we can refine the check later?
Indeed it seems that depending on tools versions and targeted silicon, MCLK1 may or may not be supported.
I guess it'd be fine to throw a big fat error in case this two-MCLK configuration ever shows-up. That way we'll know for sure what was deployed.
Thanks for the feedback, I'll update this shortly.