Hi Amadeusz,
>> +int intel_nhlt_ssp_mclk_mask(struct nhlt_acpi_table *nhlt, int ssp_num)
>> +{
>> + struct nhlt_endpoint *epnt;
>> + struct nhlt_fmt *fmt;
>> + struct nhlt_fmt_cfg *cfg;
>> + int mclk_mask = 0;
>> + int i, j;
>> +
>> + if (!nhlt)
>> + return 0;
>> +
>> + epnt = (struct nhlt_endpoint *)nhlt->desc;
>> + for (i = 0; i < nhlt->endpoint_count; i++) {
>> +
>> + /* we only care about endpoints connected to an audio codec
>> over SSP */
>> + if (epnt->linktype == NHLT_LINK_SSP &&
>> + epnt->device_type == NHLT_DEVICE_I2S &&
>> + epnt->virtual_bus_id == ssp_num) {
>
> if (epnt->linktype != NHLT_LINK_SSP ||
> epnt->device_type != NHLT_DEVICE_I2S ||
> epnt->virtual_bus_id != ssp_num)
> continue;
>
> and then you can remove one indentation level below?
Would that work? We still need to move the epnt pointer:
epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
and moving this in the endpoint_count loop would be ugly as well.
>> +
>> + fmt = (struct nhlt_fmt *)(epnt->config.caps +
>> epnt->config.size);
>> + cfg = fmt->fmt_config;
>> +
>> + /*
>> + * In theory all formats should use the same MCLK but it
>> doesn't hurt to
>> + * double-check that the configuration is consistent
>> + */
>> + for (j = 0; j < fmt->fmt_count; j++) {
>> + u32 *blob;
>> + int mdivc_offset;
>> +
>> + if (cfg->config.size >= SSP_BLOB_V1_0_SIZE) {
>> + blob = (u32 *)cfg->config.caps;
>> +
>> + if (blob[1] == SSP_BLOB_VER_2_0)
>> + mdivc_offset = SSP_BLOB_V2_0_MDIVC_OFFSET;
>> + else if (blob[1] == SSP_BLOB_VER_1_5)
>> + mdivc_offset = SSP_BLOB_V1_5_MDIVC_OFFSET;
>> + else
>> + mdivc_offset = SSP_BLOB_V1_0_MDIVC_OFFSET;
>> +
>> + mclk_mask |= blob[mdivc_offset] & GENMASK(1, 0);
>> + }
>> +
>> + 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.
>
>> +}
>> +EXPORT_SYMBOL(intel_nhlt_ssp_mclk_mask);
>> +
>> static struct nhlt_specific_cfg *
>> nhlt_get_specific_cfg(struct device *dev, struct nhlt_fmt *fmt, u8
>> num_ch,
>> u32 rate, u8 vbps, u8 bps)
>
