[bug report] ASoC: Intel: KMB: Enable TDM audio capture

Pierre-Louis Bossart pierre-louis.bossart at linux.intel.com
Tue Aug 25 20:23:11 CEST 2020


>>>>>>   506          switch (config->chan_nr) {
>>>>>>   507          case 8:
>>>>>>   508          case 4:
>>>>>>   509                  /*
>>>>>>   510                   * Platform is not capable of providing clocks for
>>>>>>   511                   * multi channel audio
>>>>>>   512                   */
>>>>>>   513                  if (kmb_i2s->master)
>>>>>>   514                          return -EINVAL;
>>>>>>   515
>>>>>>   516                  write_val = ((config->chan_nr / 2) << TDM_CHANNEL_CONFIG_BIT) |
>>>>>>   517                                  (config->data_width << DATA_WIDTH_CONFIG_BIT) |
>>>>>>   518                                  !MASTER_MODE | TDM_OPERATION;
>>>>>>                                        ^^^^^^^^^^^^
>>>>>> MASTER_MODE is BIT(13).  It's unclear what this is supposed to be.  My
>>>>>> best guess is that the ! should just be deleted.
>>>>>
>>>>> This ! is intentional because it is meant to be Slave mode. Would a better approach be to create another #define for slave mode?
>>>>
>>>> In my opinion, it's better to just leave it out.  ORing with zero causes
>>>> a different static checker warning on my unreleased checks...  Is it
>>>> 0 << 13?  I feel like ORing with zero just makes things more confusing.
>>>>
>>> It is 0<<13, in the event it was previously configured to Master I would need to unset the bit
>>
>> You are assigning the result to write_val, so there's no memory of what was configured before?
> 
> Yea you are right, would leaving this !MASTER_MODE out the best solution?

Sounds good to me. Thanks Dan for the report!


More information about the Alsa-devel mailing list