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

[Sit, Michael Wei Hong]

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> Sent: Wednesday, 26 August, 2020 2:23 AM
> To: Sit, Michael Wei Hong <michael.wei.hong.sit at intel.com>
> Cc: Dan Carpenter <dan.carpenter at oracle.com>; alsa-devel at alsa-
> project.org; Sia, Jee Heng <jee.heng.sia at intel.com>
> Subject: Re: [bug report] ASoC: Intel: KMB: Enable TDM audio
> capture
> 
> 
> >>>>>>   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!
Tested removing the !MASTER_MODE from the write_val, it still works as expected
Removing !MASTER_MODE sounds good to me. Thanks Dan for the report!